[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7959920acf004f3cb8072de1e17439fa@realtek.com>
Date: Mon, 20 Nov 2023 09:08:18 +0000
From: James Tai [戴志峰] <james.tai@...ltek.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
Thomas Gleixner <tglx@...utronix.de>,
Marc Zyngier <maz@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: RE: [PATCH v2 1/6] dt-bindings: interrupt-controller: Add support for Realtek DHC SoCs
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@...ltek.com>
>> +
>> +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@...ltek.com>
>> +
>> +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
Powered by blists - more mailing lists