diff mbox series

[1/2] dt-bindings: display: ti, am65x-dss: Add missing register & interrupt

Message ID 20220419070302.16502-2-a-bhatia1@ti.com (mailing list archive)
State New, archived
Headers show
Series Update register & interrupt info in am65x DSS | expand

Commit Message

Aradhya Bhatia April 19, 2022, 7:03 a.m. UTC
The DSS IP on the ti-am65x soc supports an additional register space,
named "common1". Further. the IP services a maximum number of 2
interrupts.

Add the missing register space "common1" and the additional interrupt.

Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
---
 .../devicetree/bindings/display/ti/ti,am65x-dss.yaml   | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Rob Herring (Arm) April 19, 2022, 12:12 p.m. UTC | #1
On Tue, 19 Apr 2022 12:33:01 +0530, Aradhya Bhatia wrote:
> The DSS IP on the ti-am65x soc supports an additional register space,
> named "common1". Further. the IP services a maximum number of 2
> interrupts.
> 
> Add the missing register space "common1" and the additional interrupt.
> 
> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
> ---
>  .../devicetree/bindings/display/ti/ti,am65x-dss.yaml   | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 

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:
Error: Documentation/devicetree/bindings/display/ti/ti,am65x-dss.example.dts:30.17-18 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:364: Documentation/devicetree/bindings/display/ti/ti,am65x-dss.example.dtb] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1401: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

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.
Nishanth Menon April 19, 2022, 12:40 p.m. UTC | #2
On 12:33-20220419, Aradhya Bhatia wrote:
> The DSS IP on the ti-am65x soc supports an additional register space,
> named "common1". Further. the IP services a maximum number of 2
> interrupts.
> 
> Add the missing register space "common1" and the additional interrupt.
> 
> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
> ---
>  .../devicetree/bindings/display/ti/ti,am65x-dss.yaml   | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
> index 5c7d2cbc4aac..102059e9e0d5 100644
> --- a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
> +++ b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
> @@ -26,6 +26,7 @@ properties:
>        Addresses to each DSS memory region described in the SoC's TRM.
>      items:
>        - description: common DSS register area
> +      - description: common1 DSS register area
>        - description: VIDL1 light video plane
>        - description: VID video plane
>        - description: OVR1 overlay manager for vp1
> @@ -36,6 +37,7 @@ properties:
>    reg-names:
>      items:
>        - const: common
> +      - const: common1
>        - const: vidl1
>        - const: vid
>        - const: ovr1
> @@ -64,7 +66,7 @@ properties:
>      maxItems: 3
>  
>    interrupts:
> -    maxItems: 1
> +    maxItems: 2

What are the interrupts supposed to be?

>  
>    power-domains:
>      maxItems: 1
> @@ -122,13 +124,14 @@ examples:
>      dss: dss@4a00000 {
>              compatible = "ti,am65x-dss";
>              reg =   <0x04a00000 0x1000>, /* common */
> +            reg =   <0x04a01000 0x1000>, /* common1 */
>                      <0x04a02000 0x1000>, /* vidl1 */
>                      <0x04a06000 0x1000>, /* vid */
>                      <0x04a07000 0x1000>, /* ovr1 */
>                      <0x04a08000 0x1000>, /* ovr2 */
>                      <0x04a0a000 0x1000>, /* vp1 */
>                      <0x04a0b000 0x1000>; /* vp2 */
> -            reg-names = "common", "vidl1", "vid",
> +            reg-names = "common", "common1". "vidl1", "vid",
>                      "ovr1", "ovr2", "vp1", "vp2";
>              ti,am65x-oldi-io-ctrl = <&dss_oldi_io_ctrl>;
>              power-domains = <&k3_pds 67 TI_SCI_PD_EXCLUSIVE>;
> @@ -136,7 +139,8 @@ examples:
>                              <&k3_clks 216 1>,
>                              <&k3_clks 67 2>;
>              clock-names = "fck", "vp1", "vp2";
> -            interrupts = <GIC_SPI 166 IRQ_TYPE_EDGE_RISING>;
> +            interrupts = <GIC_SPI 166 IRQ_TYPE_EDGE_RISING>,
> +                         <GIC_SPI 167 IRQ_TYPE_EDGE_RISING>;
>              ports {
>                      #address-cells = <1>;
>                      #size-cells = <0>;
> -- 
> 2.35.3
>
Rob Herring (Arm) April 19, 2022, 2:20 p.m. UTC | #3
On Tue, Apr 19, 2022 at 12:33:01PM +0530, Aradhya Bhatia wrote:
> The DSS IP on the ti-am65x soc supports an additional register space,
> named "common1". Further. the IP services a maximum number of 2
> interrupts.
> 
> Add the missing register space "common1" and the additional interrupt.
> 
> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
> ---
>  .../devicetree/bindings/display/ti/ti,am65x-dss.yaml   | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
> index 5c7d2cbc4aac..102059e9e0d5 100644
> --- a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
> +++ b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
> @@ -26,6 +26,7 @@ properties:
>        Addresses to each DSS memory region described in the SoC's TRM.
>      items:
>        - description: common DSS register area
> +      - description: common1 DSS register area

You've just broken the ABI.

New entries have to go on the end.

>        - description: VIDL1 light video plane
>        - description: VID video plane
>        - description: OVR1 overlay manager for vp1
> @@ -36,6 +37,7 @@ properties:
>    reg-names:
>      items:
>        - const: common
> +      - const: common1
>        - const: vidl1
>        - const: vid
>        - const: ovr1
> @@ -64,7 +66,7 @@ properties:
>      maxItems: 3
>  
>    interrupts:
> -    maxItems: 1
> +    maxItems: 2

Once there is more than 1, we need to know what each entry is and the 
order.

>  
>    power-domains:
>      maxItems: 1
> @@ -122,13 +124,14 @@ examples:
>      dss: dss@4a00000 {
>              compatible = "ti,am65x-dss";
>              reg =   <0x04a00000 0x1000>, /* common */
> +            reg =   <0x04a01000 0x1000>, /* common1 */
>                      <0x04a02000 0x1000>, /* vidl1 */
>                      <0x04a06000 0x1000>, /* vid */
>                      <0x04a07000 0x1000>, /* ovr1 */
>                      <0x04a08000 0x1000>, /* ovr2 */
>                      <0x04a0a000 0x1000>, /* vp1 */
>                      <0x04a0b000 0x1000>; /* vp2 */
> -            reg-names = "common", "vidl1", "vid",
> +            reg-names = "common", "common1". "vidl1", "vid",
>                      "ovr1", "ovr2", "vp1", "vp2";
>              ti,am65x-oldi-io-ctrl = <&dss_oldi_io_ctrl>;
>              power-domains = <&k3_pds 67 TI_SCI_PD_EXCLUSIVE>;
> @@ -136,7 +139,8 @@ examples:
>                              <&k3_clks 216 1>,
>                              <&k3_clks 67 2>;
>              clock-names = "fck", "vp1", "vp2";
> -            interrupts = <GIC_SPI 166 IRQ_TYPE_EDGE_RISING>;
> +            interrupts = <GIC_SPI 166 IRQ_TYPE_EDGE_RISING>,
> +                         <GIC_SPI 167 IRQ_TYPE_EDGE_RISING>;
>              ports {
>                      #address-cells = <1>;
>                      #size-cells = <0>;
> -- 
> 2.35.3
> 
>
Tomi Valkeinen April 20, 2022, 7:05 a.m. UTC | #4
Hi,

On 19/04/2022 17:20, Rob Herring wrote:
> On Tue, Apr 19, 2022 at 12:33:01PM +0530, Aradhya Bhatia wrote:
>> The DSS IP on the ti-am65x soc supports an additional register space,
>> named "common1". Further. the IP services a maximum number of 2
>> interrupts.
>>
>> Add the missing register space "common1" and the additional interrupt.
>>
>> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
>> ---
>>   .../devicetree/bindings/display/ti/ti,am65x-dss.yaml   | 10 +++++++---
>>   1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
>> index 5c7d2cbc4aac..102059e9e0d5 100644
>> --- a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
>> +++ b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
>> @@ -26,6 +26,7 @@ properties:
>>         Addresses to each DSS memory region described in the SoC's TRM.
>>       items:
>>         - description: common DSS register area
>> +      - description: common1 DSS register area
> 
> You've just broken the ABI.
> 
> New entries have to go on the end.

I'm curious, if the 'reg-names' is a required property, as it is here, 
does this still break the ABI?

  Tomi
Rob Herring (Arm) April 20, 2022, 9:30 p.m. UTC | #5
On Wed, Apr 20, 2022 at 10:05:34AM +0300, Tomi Valkeinen wrote:
> Hi,
> 
> On 19/04/2022 17:20, Rob Herring wrote:
> > On Tue, Apr 19, 2022 at 12:33:01PM +0530, Aradhya Bhatia wrote:
> > > The DSS IP on the ti-am65x soc supports an additional register space,
> > > named "common1". Further. the IP services a maximum number of 2
> > > interrupts.
> > > 
> > > Add the missing register space "common1" and the additional interrupt.
> > > 
> > > Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
> > > ---
> > >   .../devicetree/bindings/display/ti/ti,am65x-dss.yaml   | 10 +++++++---
> > >   1 file changed, 7 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
> > > index 5c7d2cbc4aac..102059e9e0d5 100644
> > > --- a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
> > > +++ b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
> > > @@ -26,6 +26,7 @@ properties:
> > >         Addresses to each DSS memory region described in the SoC's TRM.
> > >       items:
> > >         - description: common DSS register area
> > > +      - description: common1 DSS register area
> > 
> > You've just broken the ABI.
> > 
> > New entries have to go on the end.
> 
> I'm curious, if the 'reg-names' is a required property, as it is here, does
> this still break the ABI?

Yes, the order is part of the ABI.

Sometimes we just give up with multiple optional entries or inherited 
any order allowed, but here there is no reason. Just add 'common1' to 
the end.

Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
index 5c7d2cbc4aac..102059e9e0d5 100644
--- a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
+++ b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
@@ -26,6 +26,7 @@  properties:
       Addresses to each DSS memory region described in the SoC's TRM.
     items:
       - description: common DSS register area
+      - description: common1 DSS register area
       - description: VIDL1 light video plane
       - description: VID video plane
       - description: OVR1 overlay manager for vp1
@@ -36,6 +37,7 @@  properties:
   reg-names:
     items:
       - const: common
+      - const: common1
       - const: vidl1
       - const: vid
       - const: ovr1
@@ -64,7 +66,7 @@  properties:
     maxItems: 3
 
   interrupts:
-    maxItems: 1
+    maxItems: 2
 
   power-domains:
     maxItems: 1
@@ -122,13 +124,14 @@  examples:
     dss: dss@4a00000 {
             compatible = "ti,am65x-dss";
             reg =   <0x04a00000 0x1000>, /* common */
+            reg =   <0x04a01000 0x1000>, /* common1 */
                     <0x04a02000 0x1000>, /* vidl1 */
                     <0x04a06000 0x1000>, /* vid */
                     <0x04a07000 0x1000>, /* ovr1 */
                     <0x04a08000 0x1000>, /* ovr2 */
                     <0x04a0a000 0x1000>, /* vp1 */
                     <0x04a0b000 0x1000>; /* vp2 */
-            reg-names = "common", "vidl1", "vid",
+            reg-names = "common", "common1". "vidl1", "vid",
                     "ovr1", "ovr2", "vp1", "vp2";
             ti,am65x-oldi-io-ctrl = <&dss_oldi_io_ctrl>;
             power-domains = <&k3_pds 67 TI_SCI_PD_EXCLUSIVE>;
@@ -136,7 +139,8 @@  examples:
                             <&k3_clks 216 1>,
                             <&k3_clks 67 2>;
             clock-names = "fck", "vp1", "vp2";
-            interrupts = <GIC_SPI 166 IRQ_TYPE_EDGE_RISING>;
+            interrupts = <GIC_SPI 166 IRQ_TYPE_EDGE_RISING>,
+                         <GIC_SPI 167 IRQ_TYPE_EDGE_RISING>;
             ports {
                     #address-cells = <1>;
                     #size-cells = <0>;