diff mbox series

[v4,6/9] dt-bindings: interrupt-controller: realtek,rtl-intc: Add rtl9300-intc

Message ID 20240705021520.2737568-7-chris.packham@alliedtelesis.co.nz (mailing list archive)
State New
Headers show
Series mips: Support for RTL9302C | expand

Commit Message

Chris Packham July 5, 2024, 2:15 a.m. UTC
Add a compatible string for the interrupt controller found on the
rtl930x SoCs. The interrupt controller has registers for VPE1 so these
are added as a second reg cell.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---

Notes:
    Changes in v3:
    - Add reg::minItems where required
    Changes in v3:
    - Use items to describe the regs property
    Changes in v2:
    - Set reg:maxItems to 2 to allow for VPE1 registers on the rtl9300. Add
      a condition to enforce the old limit on other SoCs.
    - Connor and Krzysztof offered acks on v1 but I think the changes here
      are big enough to void those.

 .../realtek,rtl-intc.yaml                     | 20 ++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

Comments

Krzysztof Kozlowski July 5, 2024, 6:41 a.m. UTC | #1
On 05/07/2024 04:15, Chris Packham wrote:
> Add a compatible string for the interrupt controller found on the
> rtl930x SoCs. The interrupt controller has registers for VPE1 so these
> are added as a second reg cell.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
> 
> Notes:
>     Changes in v3:
>     - Add reg::minItems where required
>     Changes in v3:
>     - Use items to describe the regs property
>     Changes in v2:
>     - Set reg:maxItems to 2 to allow for VPE1 registers on the rtl9300. Add
>       a condition to enforce the old limit on other SoCs.
>     - Connor and Krzysztof offered acks on v1 but I think the changes here
>       are big enough to void those.
> 
>  .../realtek,rtl-intc.yaml                     | 20 ++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/realtek,rtl-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/realtek,rtl-intc.yaml
> index fb5593724059..f36aaab73c01 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/realtek,rtl-intc.yaml
> +++ b/Documentation/devicetree/bindings/interrupt-controller/realtek,rtl-intc.yaml
> @@ -25,6 +25,7 @@ properties:
>        - items:
>            - enum:
>                - realtek,rtl8380-intc
> +              - realtek,rtl9300-intc
>            - const: realtek,rtl-intc
>        - const: realtek,rtl-intc
>          deprecated: true
> @@ -35,7 +36,10 @@ properties:
>      const: 1
>  
>    reg:
> -    maxItems: 1
> +    minItems: 1
> +    items:
> +      - description: vpe0 registers
> +      - description: vpe1 registers
>  
>    interrupts:
>      minItems: 1
> @@ -71,6 +75,20 @@ allOf:
>      else:
>        required:
>          - interrupts
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: realtek,rtl9300-intc
> +    then:
> +      properties:
> +        reg:
> +          minItems: 1

Hm? Why?

<form letter>
This is a friendly reminder during the review process.

It seems my or other reviewer's previous comments were not fully
addressed. Maybe the feedback got lost between the quotes, maybe you
just forgot to apply it. Please go back to the previous discussion and
either implement all requested changes or keep discussing them.

Thank you.
</form letter>

> +          maxItems: 2
Best regards,
Krzysztof
Chris Packham July 7, 2024, 9:07 p.m. UTC | #2
On 5/07/24 18:41, Krzysztof Kozlowski wrote:
> On 05/07/2024 04:15, Chris Packham wrote:
>> Add a compatible string for the interrupt controller found on the
>> rtl930x SoCs. The interrupt controller has registers for VPE1 so these
>> are added as a second reg cell.
>>
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>> ---
>>
>> Notes:
>>      Changes in v3:
>>      - Add reg::minItems where required
>>      Changes in v3:
>>      - Use items to describe the regs property
>>      Changes in v2:
>>      - Set reg:maxItems to 2 to allow for VPE1 registers on the rtl9300. Add
>>        a condition to enforce the old limit on other SoCs.
>>      - Connor and Krzysztof offered acks on v1 but I think the changes here
>>        are big enough to void those.
>>
>>   .../realtek,rtl-intc.yaml                     | 20 ++++++++++++++++++-
>>   1 file changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/interrupt-controller/realtek,rtl-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/realtek,rtl-intc.yaml
>> index fb5593724059..f36aaab73c01 100644
>> --- a/Documentation/devicetree/bindings/interrupt-controller/realtek,rtl-intc.yaml
>> +++ b/Documentation/devicetree/bindings/interrupt-controller/realtek,rtl-intc.yaml
>> @@ -25,6 +25,7 @@ properties:
>>         - items:
>>             - enum:
>>                 - realtek,rtl8380-intc
>> +              - realtek,rtl9300-intc
>>             - const: realtek,rtl-intc
>>         - const: realtek,rtl-intc
>>           deprecated: true
>> @@ -35,7 +36,10 @@ properties:
>>       const: 1
>>   
>>     reg:
>> -    maxItems: 1
>> +    minItems: 1
>> +    items:
>> +      - description: vpe0 registers
>> +      - description: vpe1 registers
>>   
>>     interrupts:
>>       minItems: 1
>> @@ -71,6 +75,20 @@ allOf:
>>       else:
>>         required:
>>           - interrupts
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: realtek,rtl9300-intc
>> +    then:
>> +      properties:
>> +        reg:
>> +          minItems: 1
> Hm? Why?
>
> <form letter>
> This is a friendly reminder during the review process.
>
> It seems my or other reviewer's previous comments were not fully
> addressed. Maybe the feedback got lost between the quotes, maybe you
> just forgot to apply it. Please go back to the previous discussion and
> either implement all requested changes or keep discussing them.
>
> Thank you.
> </form letter>

I think I probably just fluffed a copy and paste when adding minItems: 1 
to the base, sorry about that.

I've set minItems: 2 for that condition locally so it's ready for v5.

>> +          maxItems: 2
> Best regards,
> Krzysztof
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/interrupt-controller/realtek,rtl-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/realtek,rtl-intc.yaml
index fb5593724059..f36aaab73c01 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/realtek,rtl-intc.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/realtek,rtl-intc.yaml
@@ -25,6 +25,7 @@  properties:
       - items:
           - enum:
               - realtek,rtl8380-intc
+              - realtek,rtl9300-intc
           - const: realtek,rtl-intc
       - const: realtek,rtl-intc
         deprecated: true
@@ -35,7 +36,10 @@  properties:
     const: 1
 
   reg:
-    maxItems: 1
+    minItems: 1
+    items:
+      - description: vpe0 registers
+      - description: vpe1 registers
 
   interrupts:
     minItems: 1
@@ -71,6 +75,20 @@  allOf:
     else:
       required:
         - interrupts
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: realtek,rtl9300-intc
+    then:
+      properties:
+        reg:
+          minItems: 1
+          maxItems: 2
+    else:
+      properties:
+        reg:
+          maxItems: 1
 
 additionalProperties: false