Message ID | 20221209-dt-binding-ufs-v1-0-8d502f0e18d5@fairphone.com |
---|---|
State | New |
Headers | show |
Series | dt-bindings: ufs: qcom: Add reg-names property for ICE | expand |
On Wed Dec 28, 2022 at 12:58 PM CET, Krzysztof Kozlowski wrote: > On 28/12/2022 12:53, Luca Weiss wrote: > > Hi Krzysztof, > > > > On Wed Dec 28, 2022 at 12:50 PM CET, Krzysztof Kozlowski wrote: > >> On 09/12/2022 15:29, Luca Weiss wrote: > >>> The code in ufs-qcom-ice.c needs the ICE reg to be named "ice". Add this > >>> in the bindings so the existing dts can validate successfully. > >>> > >>> Also sm8450 is using ICE since commit 276ee34a40c1 ("arm64: dts: qcom: > >>> sm8450: add Inline Crypto Engine registers and clock") so move the > >>> compatible to the correct if. > >>> > >>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> > >>> --- > >>> (no cover subject) > >>> > >>> The only remaining validation issues I see is the following on sc8280xp-crd.dtb > >>> and sa8540p-ride.dtb: > >>> > >> > >> Any plans on fixing the patch (after testing it) and resending? > > > > I wasn't quite sure how to fix the comments, but re-reading them this > > comment from you is how you expect it to be in v2? > > The patch fails testing, so I meant this. > > > > >> Just add it to top-level with minItems: 1 and per variant customize: > >> 1. maxItems: 1 > >> 2. minItems: 2 + required > > I tried a bit now but couldn't get it to work when using 'items' so that we have the "std" and "ice" names in there. Documentation/devicetree/bindings/ufs/qcom,ufs.yaml: allOf:2:then:properties:reg-names: 'oneOf' conditional failed, one must be fixed: [{'const': 'std'}, {'const': 'ice'}] is too long [{'const': 'std'}, {'const': 'ice'}] is too short False schema does not allow 2 1 was expected hint: "minItems" is only needed if less than the "items" list length from schema $id: http://devicetree.org/meta-schemas/items.yaml# Since I have 'minItems: 1' in top-level I seemingly cannot use 'items' in the 'if' neither alone nor with 'minItems' and/or 'maxItems', getting different errors when doing that. Can I just put 'reg-names: true' top-level and then specify either items for the ones that use ICE or for the others use the 'maxItems: 1'? Or am I supposed to ignore 'items' completely but driver expects 'ice' name so I'd rather include it. Regards Luca > > Yes. > > Best regards, > Krzysztof
On 28/12/2022 16:24, Luca Weiss wrote: > On Wed Dec 28, 2022 at 12:58 PM CET, Krzysztof Kozlowski wrote: >> On 28/12/2022 12:53, Luca Weiss wrote: >>> Hi Krzysztof, >>> >>> On Wed Dec 28, 2022 at 12:50 PM CET, Krzysztof Kozlowski wrote: >>>> On 09/12/2022 15:29, Luca Weiss wrote: >>>>> The code in ufs-qcom-ice.c needs the ICE reg to be named "ice". Add this >>>>> in the bindings so the existing dts can validate successfully. >>>>> >>>>> Also sm8450 is using ICE since commit 276ee34a40c1 ("arm64: dts: qcom: >>>>> sm8450: add Inline Crypto Engine registers and clock") so move the >>>>> compatible to the correct if. >>>>> >>>>> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> >>>>> --- >>>>> (no cover subject) >>>>> >>>>> The only remaining validation issues I see is the following on sc8280xp-crd.dtb >>>>> and sa8540p-ride.dtb: >>>>> >>>> >>>> Any plans on fixing the patch (after testing it) and resending? >>> >>> I wasn't quite sure how to fix the comments, but re-reading them this >>> comment from you is how you expect it to be in v2? >> >> The patch fails testing, so I meant this. >> >>> >>>> Just add it to top-level with minItems: 1 and per variant customize: >>>> 1. maxItems: 1 >>>> 2. minItems: 2 + required >>> > > I tried a bit now but couldn't get it to work when using 'items' so that > we have the "std" and "ice" names in there. > > Documentation/devicetree/bindings/ufs/qcom,ufs.yaml: allOf:2:then:properties:reg-names: 'oneOf' conditional failed, one must be fixed: > [{'const': 'std'}, {'const': 'ice'}] is too long > [{'const': 'std'}, {'const': 'ice'}] is too short > False schema does not allow 2 > 1 was expected > hint: "minItems" is only needed if less than the "items" list length > from schema $id: http://devicetree.org/meta-schemas/items.yaml# > > Since I have 'minItems: 1' in top-level I seemingly cannot use 'items' > in the 'if' neither alone nor with 'minItems' and/or 'maxItems', getting > different errors when doing that. top-level cannot have only minItems:1. > > Can I just put 'reg-names: true' top-level and then specify either items > for the ones that use ICE or for the others use the 'maxItems: 1'? > > Or am I supposed to ignore 'items' completely but driver expects 'ice' > name so I'd rather include it. Use the syntax like: https://elixir.bootlin.com/linux/v5.19-rc6/source/Documentation/devicetree/bindings/clock/samsung,exynos7-clock.yaml#L57 Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml index f2d6298d926c..58a2fb2c83c3 100644 --- a/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml +++ b/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml @@ -102,7 +102,6 @@ allOf: - qcom,sc8280xp-ufshc - qcom,sm8250-ufshc - qcom,sm8350-ufshc - - qcom,sm8450-ufshc then: properties: clocks: @@ -130,6 +129,7 @@ allOf: - qcom,sdm845-ufshc - qcom,sm6350-ufshc - qcom,sm8150-ufshc + - qcom,sm8450-ufshc then: properties: clocks: @@ -149,6 +149,12 @@ allOf: reg: minItems: 2 maxItems: 2 + reg-names: + items: + - const: std + - const: ice + required: + - reg-names - if: properties:
The code in ufs-qcom-ice.c needs the ICE reg to be named "ice". Add this in the bindings so the existing dts can validate successfully. Also sm8450 is using ICE since commit 276ee34a40c1 ("arm64: dts: qcom: sm8450: add Inline Crypto Engine registers and clock") so move the compatible to the correct if. Signed-off-by: Luca Weiss <luca.weiss@fairphone.com> --- (no cover subject) The only remaining validation issues I see is the following on sc8280xp-crd.dtb and sa8540p-ride.dtb: Unevaluated properties are not allowed ('required-opps', 'dma-coherent' were unexpected) Maybe someone who knows something about this can handle this? And the patch adding qcom,sm6115-ufshc hasn't been applied yet. --- Documentation/devicetree/bindings/ufs/qcom,ufs.yaml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) --- base-commit: f925116b24c0c42dc6d5ab5111c55fd7f74e8dc7 change-id: 20221209-dt-binding-ufs-2d7f64797ff2 Best regards,