RE: [PATCH v2 1/6] dt-bindings: interrupt-controller: Add support for Realtek DHC SoCs

[James Tai [戴志峰]]

Hi Krzysztof,

>I doubt it.
>
>And bot prooves it.

My 'dtschema' is outdated, and I encountered errors after updating it.
I will fix it in next patches.

>> - Fixed code style issues
>
>Be specific - what code style issues did you fix?
>
I fixed the code style issue related to the license declaration.

>>
>>  .../realtek,rtd1319-intc.yaml                 | 79
>+++++++++++++++++++
>>  .../realtek,rtd1319d-intc.yaml                | 79
>+++++++++++++++++++
>>  .../realtek,rtd1325-intc.yaml                 | 79
>+++++++++++++++++++
>>  .../realtek,rtd1619b-intc.yaml                | 78 ++++++++++++++++++
>>  4 files changed, 315 insertions(+)
>>  create mode 100644
>> Documentation/devicetree/bindings/interrupt-controller/realtek,rtd1319
>> -intc.yaml  create mode 100644
>> Documentation/devicetree/bindings/interrupt-controller/realtek,rtd1319
>> d-intc.yaml  create mode 100644
>> Documentation/devicetree/bindings/interrupt-controller/realtek,rtd1325
>> -intc.yaml  create mode 100644
>> Documentation/devicetree/bindings/interrupt-controller/realtek,rtd1619
>> b-intc.yaml
>
>Why do you have four bindings for the same? Please explain me the differences.
>
If the bindings can be shared, I will consolidate it into one.

>>
>> diff --git
>> a/Documentation/devicetree/bindings/interrupt-controller/realtek,rtd13
>> 19-intc.yaml
>> b/Documentation/devicetree/bindings/interrupt-controller/realtek,rtd13
>> 19-intc.yaml
>> new file mode 100644
>> index 000000000000..b88f3ac07cd9
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/interrupt-controller/realtek,r
>> +++ td1319-intc.yaml
>> @@ -0,0 +1,79 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
>> +---
>> +$id:
>> +http://devicetree.org/schemas/interrupt-controller/realtek,rtd1319-in
>> +tc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Realtek DHC RTD1319 Interrupt Controller Device Tree Bindings
>> +
>> +description:
>> +  This interrupt controller is a component of Realtek DHC RTD1319 and
>> +  is designed to receive interrupts from peripheral devices.
>> +
>> +  Each DHC SoC has two sets of interrupt controllers, each capable of
>> + handling up to 32 interrupts.
>> +
>> +maintainers:
>> +  - James Tai <james.tai@xxxxxxxxxxx>
>> +
>> +allOf:
>> +  - $ref: /schemas/interrupt-controller.yaml#
>> +
>> +properties:
>> +  "#interrupt-cells":
>> +    const: 1
>
>
>compatible is first, put the cells next to other interrupt controller properties.
>
I will fix it in next patches.

>> +
>> +  compatible:
>> +    enum:
>> +      - realtek,rtd1319-intc-iso
>> +      - realtek,rtd1319-intc-misc
>> +
>> +  "#address-cells":
>> +    const: 0
>> +
>> +  interrupt-controller: true
>> +
>> +  interrupts-extended:
>
>interrupts instead.
>
>Anyway, you must describe the items. Why this is not fixed but flexible?
>Hardware has different number of pins? That's unlikely.
>
I will replace it with 'interrupts'. Since our Interrupt controller architecture doesn't involve multiple interrupt sources, using 'interrupts' should suffice.

>> +    minItems: 1
>> +    maxItems: 4
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +required:
>> +  - "#interrupt-cells"
>> +  - "#address-cells"
>> +  - compatible
>> +  - interrupt-controller
>> +  - interrupts-extended
>> +  - reg
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +
>> +    rtd1319_iso_irq: interrupt-controller@40 {
>> +      compatible = "realtek,rtd1319-intc-iso";
>> +      reg = <0x00 0x40>;
>> +      interrupts-extended = <&gic GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>,
>> +                            <&gic GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
>> +      interrupt-controller;
>> +      #address-cells = <0>;
>> +      #interrupt-cells = <1>;
>> +    };
>> +
>> +    rtd1319_misc_irq: interrupt-controller@80 {
>> +      compatible = "realtek,rtd1319-intc-misc";
>
>Drop, one example is enough. This is the same as previous.
>
I will fix it in next patches.

>> +      reg = <0x00 0x80>;
>> +      interrupts-extended = <&gic GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>,
>> +                            <&gic GIC_SPI 89
>IRQ_TYPE_LEVEL_HIGH>,
>> +                            <&gic GIC_SPI 90
>IRQ_TYPE_LEVEL_HIGH>,
>> +                            <&gic GIC_SPI 39
>IRQ_TYPE_LEVEL_HIGH>;
>> +      interrupt-controller;
>> +      #address-cells = <0>;
>> +      #interrupt-cells = <1>;
>> +    };
>> +...
>> diff --git
>> a/Documentation/devicetree/bindings/interrupt-controller/realtek,rtd13
>> 19d-intc.yaml
>> b/Documentation/devicetree/bindings/interrupt-controller/realtek,rtd13
>> 19d-intc.yaml
>> new file mode 100644
>> index 000000000000..75aba448baf7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/interrupt-controller/realtek,r
>> +++ td1319d-intc.yaml
>> @@ -0,0 +1,79 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
>> +---
>> +$id:
>> +http://devicetree.org/schemas/interrupt-controller/realtek,rtd1319d-i
>> +ntc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Realtek DHC RTD1319D Interrupt Controller Device Tree Bindings
>> +
>> +description:
>> +  This interrupt controller is a component of Realtek DHC RTD1319D
>> +and
>> +  is designed to receive interrupts from peripheral devices.
>> +
>> +  Each DHC SoC has two sets of interrupt controllers, each capable of
>> + handling up to 32 interrupts.
>> +
>> +maintainers:
>> +  - James Tai <james.tai@xxxxxxxxxxx>
>> +
>> +allOf:
>> +  - $ref: /schemas/interrupt-controller.yaml#
>> +
>> +properties:
>> +  "#interrupt-cells":
>> +    const: 1
>> +
>> +  compatible:
>> +    enum:
>> +      - realtek,rtd1319d-intc-iso
>> +      - realtek,rtd1319d-intc-misc
>
>So this is the same as the other one? Why it cannot be part of that one?

I will consolidate these parts into a single file.

Thank you for your feedback.

Regards,
James