RE: Re: [PATCH 2/5] dt-bindings: connector: Add fsl,io-connector binding

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s)
> to consider:
> - [Medium] Missing `#pwm-cells` property definition causes valid PWM nexus
> configurations to fail schema validation.
> - [Medium] Subsystem guide violation: Provider capabilities are missing from
> the `required` list.
> --
> 
> commit b88adcce540a59e2da49df7e20b99d2c52021962
> Author: Chancel Liu <[email protected]>
> 
> dt-bindings: connector: Add fsl,io-connector binding
> 
> The commit adds a device tree binding for the NXP I/O connector. It acts
> as a nexus node for routing GPIOs, clocks, PWMs, and interrupts through
> index-based mappings.
> 
> > diff --git a/Documentation/devicetree/bindings/connector/fsl,io-
> connector.yaml b/Documentation/devicetree/bindings/connector/fsl,io-
> connector.yaml
> > new file mode 100644
> > index 0000000000000..8b5038a2332e3
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/connector/fsl,io-connector.yaml
> 
> [ ... ]
> 
> >   pwm-map:
> >     $ref: /schemas/types.yaml#/definitions/uint32-matrix
> >
> >   pwm-map-mask:
> >     $ref: /schemas/types.yaml#/definitions/uint32-array
> >
> >   pwm-map-pass-thru:
> >     $ref: /schemas/types.yaml#/definitions/uint32-array
> >
> >   '#address-cells':
> >     const: 0
> 
> Will this fail schema validation for valid PWM nexus nodes? Since
> additionalProperties is false, omitting #pwm-cells from the properties list
> means device trees including #pwm-cells will fail validation. The #pwm-cells
> property is required by the nexus mapping logic to parse PWM specifiers.
> Should #pwm-cells be added to the properties block?
> 

Yes. The PWM nexus mapping relies on #pwm-cells. Will add in next revision.

> [ ... ]
> 
> >   interrupt-map: true
> >
> >   interrupt-map-mask: true
> >
> > required:
> >   - compatible
> 
> Are there missing required provider capabilities here? According to the
> dt-bindings subsystem guidelines, when hardware gains provider capabilities
> like a GPIO controller, interrupt controller, or clock provider, the
> corresponding standard properties like gpio-controller, #gpio-cells,
> #clock-cells, interrupt-controller, #interrupt-cells, and #pwm-cells must be
> included in the required list. Could these be added to ensure incomplete
> nodes do not accidentally pass validation?
> 

The connector can expose different sets of the provider domains
(GPIO/clock/PWM/interrupt) depending on the connector usage. So I think
it's not necessary to make all provider capability properties required.

> >
> > additionalProperties: false
> >
> > examples:
> >   - |
> >     #include <dt-bindings/interrupt-controller/irq.h>
> >
> >     connector {
> >         compatible = "fsl,io-connector";
> >         gpio-controller;
> >         #gpio-cells = <2>;
> >         gpio-map = <0 0 &gpio1 8 1>;
> >         gpio-map-mask = <0xff 0x0>;
> >         gpio-map-pass-thru = <0x0 0x1>;
> >         #clock-cells = <1>;
> >         clock-map = <0 &clk 1>;
> >         #address-cells = <0>;
> >         interrupt-controller;
> >         #interrupt-cells = <2>;
> >         interrupt-map-mask = <0xff 0x0>;
> >         interrupt-map = <0 0 &gpio2 27 IRQ_TYPE_LEVEL_LOW>;
> >     };
> 
> Should this example section include the PWM properties as well? The
> subsystem guidelines require the example to include all required properties
> to pass dt_binding_check.
> 

Will extend examples to cover PWM as well.

Regards, 
Chancel Liu