[PATCH v2 2/5] dt-bindings: arm: msm: Add bindings for multi channel DDR in LLCC

Komal Bajaj posted 5 patches 2 years, 11 months ago
There is a newer version of this series
[PATCH v2 2/5] dt-bindings: arm: msm: Add bindings for multi channel DDR in LLCC
Posted by Komal Bajaj 2 years, 11 months ago
Add description for additional nodes needed to support
mulitple channel DDR configurations in LLCC.

Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
---
 Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
index 38efcad56dbd..9a4a76caf490 100644
--- a/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
+++ b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
@@ -37,15 +37,24 @@ properties:
     items:
       - description: LLCC base register region
       - description: LLCC broadcast base register region
+      - description: Feature register to decide which LLCC configuration
+                     to use, this is optional
 
   reg-names:
     items:
       - const: llcc_base
       - const: llcc_broadcast_base
+      - const: multi_channel_register
 
   interrupts:
     maxItems: 1
 
+  multi-ch-bit-off:
+    items:
+      - description: Specifies the offset in bits into the multi_channel_register
+                     and the number of bits used to decide which LLCC configuration
+                     to use
+
 required:
   - compatible
   - reg
-- 
2.39.1
Re: [PATCH v2 2/5] dt-bindings: arm: msm: Add bindings for multi channel DDR in LLCC
Posted by Krzysztof Kozlowski 2 years, 11 months ago
On 13/03/2023 13:40, Komal Bajaj wrote:
> Add description for additional nodes needed to support
> mulitple channel DDR configurations in LLCC.
> 
> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>

+Cc Mani,

This will conflict with:
https://lore.kernel.org/all/20230314080443.64635-3-manivannan.sadhasivam@linaro.org/

Please rebase on top of Mani's patches (assuming they are not
conflicting in principle)

> ---
>  Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
> index 38efcad56dbd..9a4a76caf490 100644
> --- a/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
> +++ b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
> @@ -37,15 +37,24 @@ properties:
>      items:

minItems: 2

>        - description: LLCC base register region
>        - description: LLCC broadcast base register region
> +      - description: Feature register to decide which LLCC configuration
> +                     to use, this is optional
>  
>    reg-names:

minItems: 2

>      items:
>        - const: llcc_base
>        - const: llcc_broadcast_base
> +      - const: multi_channel_register
>  
>    interrupts:
>      maxItems: 1
>  
> +  multi-ch-bit-off:
> +    items:
> +      - description: Specifies the offset in bits into the multi_channel_register
> +                     and the number of bits used to decide which LLCC configuration
> +                     to use

There are here few issues.
First, I don't fully understand the property. What is an LLCC
configuration? Like some fused values?

Second, don't make it a register specific, it will not scale easily to
any new version of this interface. Although how this should look like
depends on what is it.

Third, you need vendor prefix and type (unless this is a generic
property, but does not look like). Then "items" is probably wrong. Line
break after "description: "

Best regards,
Krzysztof
Re: [PATCH v2 2/5] dt-bindings: arm: msm: Add bindings for multi channel DDR in LLCC
Posted by Manivannan Sadhasivam 2 years, 11 months ago
On Wed, Mar 15, 2023 at 08:41:21AM +0100, Krzysztof Kozlowski wrote:
> On 13/03/2023 13:40, Komal Bajaj wrote:
> > Add description for additional nodes needed to support
> > mulitple channel DDR configurations in LLCC.
> > 
> > Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
> 
> +Cc Mani,
> 

Thanks, Krzysztof!

> This will conflict with:
> https://lore.kernel.org/all/20230314080443.64635-3-manivannan.sadhasivam@linaro.org/
> 
> Please rebase on top of Mani's patches (assuming they are not
> conflicting in principle)
> 
> > ---
> >  Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
> > index 38efcad56dbd..9a4a76caf490 100644
> > --- a/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
> > +++ b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
> > @@ -37,15 +37,24 @@ properties:
> >      items:
> 
> minItems: 2
> 
> >        - description: LLCC base register region
> >        - description: LLCC broadcast base register region
> > +      - description: Feature register to decide which LLCC configuration
> > +                     to use, this is optional
> >  
> >    reg-names:
> 
> minItems: 2
> 
> >      items:
> >        - const: llcc_base
> >        - const: llcc_broadcast_base
> > +      - const: multi_channel_register

Is this the actual register region or a specific register offset? We generally
try to pass the base address of the region along with the size and use the
offset inside the driver to access any specific registers.

Thanks,
Mani

> >  
> >    interrupts:
> >      maxItems: 1
> >  
> > +  multi-ch-bit-off:
> > +    items:
> > +      - description: Specifies the offset in bits into the multi_channel_register
> > +                     and the number of bits used to decide which LLCC configuration
> > +                     to use
> 
> There are here few issues.
> First, I don't fully understand the property. What is an LLCC
> configuration? Like some fused values?
> 
> Second, don't make it a register specific, it will not scale easily to
> any new version of this interface. Although how this should look like
> depends on what is it.
> 
> Third, you need vendor prefix and type (unless this is a generic
> property, but does not look like). Then "items" is probably wrong. Line
> break after "description: "
> 
> Best regards,
> Krzysztof
> 

-- 
மணிவண்ணன் சதாசிவம்
Re: [PATCH v2 2/5] dt-bindings: arm: msm: Add bindings for multi channel DDR in LLCC
Posted by Rob Herring 2 years, 11 months ago
On Mon, 13 Mar 2023 18:10:37 +0530, Komal Bajaj wrote:
> Add description for additional nodes needed to support
> mulitple channel DDR configurations in LLCC.
> 
> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
> ---
>  Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/msm/qcom,llcc.example.dtb: system-cache-controller@1100000: reg: [[17825792, 2097152], [19922944, 327680]] is too short
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/msm/qcom,llcc.example.dtb: system-cache-controller@1100000: reg-names: ['llcc_base', 'llcc_broadcast_base'] is too short
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230313124040.9463-3-quic_kbajaj@quicinc.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.