Re: [PATCH RFC 1/2] dt-bindings: pinctl: amlogic,pinctrl-a4: Add gpio irq property

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The newly added `hw-irq` property is not marked as required for the GPIO banks, and the example is not updated.
- [Medium] The custom device tree property `hw-irq` lacks a vendor prefix.
- [Low] The newly added `hw-irq` property lacks a description field.
--

commit 779e29b8c4c1e37224d568880fcfcc77220c1d72
Author: Xianwei Zhao <[email protected]>

dt-bindings: pinctl: amlogic,pinctrl-a4: Add gpio irq property

Add the hw-irq property for each GPIO bank and enable interrupt-parent
for pinctrl so that gpiod_to_irq() can translate GPIO lines to IRQs.

> diff --git a/Documentation/devicetree/bindings/pinctrl/amlogic,pinctrl-a4.yaml b/Documentation/devicetree/bindings/pinctrl/amlogic,pinctrl-a4.yaml
> --- a/Documentation/devicetree/bindings/pinctrl/amlogic,pinctrl-a4.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/amlogic,pinctrl-a4.yaml

[ ... ]

> @@ -65,6 +67,9 @@ patternProperties:
>        gpio-ranges:
>          maxItems: 1
>  
> +      hw-irq:
> +        $ref: /schemas/types.yaml#/definitions/uint32

[Severity: Medium]
Should this custom property have a vendor prefix to prevent namespace
collisions, such as amlogic,hw-irq? 

[Severity: Low]
Would it be helpful to include a description field here to document the
purpose and semantic usage of this new property?

> +
>      required:
>        - reg
>        - reg-names

[Severity: Medium]
The commit message indicates this property is added for each GPIO bank.
Should hw-irq be included in this required list under the 
patternProperties:^gpio@[0-9a-f]+$ node to ensure incomplete configurations
fail schema validation?

Additionally, the example at the bottom of the file doesn't seem to include
this new property:

examples:
  ...
    gpio@4240 {
        reg = <0 0x4240 0 0x40>, <0 0x4000 0 0x8>;
        reg-names = "gpio", "mux";
        gpio-controller;
        #gpio-cells = <2>;
        gpio-ranges = <&periphs_pinctrl 0 8 10>;
    };

Does the example section need to be updated to demonstrate the usage of the
newly added property?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

_______________________________________________
linux-amlogic mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-amlogic