Re: [PATCH v4 1/4] dt-bindings: connector: Add fsl,aud-io-slot binding

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] dt-bindings: connector: broken 2-cell interrupt-map-mask in fsl,aud-io-slot
- [Low] dt-bindings: connector: missing constraints and docs in fsl,aud-io-slot
--

commit 1ab07ded804d101088853f33b8efa1603244b3a0
Author: Chancel Liu <chancel.liu@nxp.com>

dt-bindings: connector: Add fsl,aud-io-slot binding

This commit adds the Device Tree binding schema for the NXP AUD-IO slot
connector found on i.MX95 19x19 EVK and i.MX952 EVK boards, defining
nexus mappings for GPIOs, clocks, and interrupts.

> diff --git a/Documentation/devicetree/bindings/connector/fsl,aud-io-slot.yaml b/Documentation/devicetree/bindings/connector/fsl,aud-io-slot.yaml
> new file mode 100644
> index 0000000000000..6e1e3f1b6a330
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/connector/fsl,aud-io-slot.yaml
> @@ -0,0 +1,96 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/connector/fsl,aud-io-slot.yaml#
> +$schema: http://devicetree.org/meta-schemas/base.yaml#
> +
> +title: NXP AUD-IO Slot

[ ... ]

> +  '#interrupt-cells':
> +    const: 2
> +
> +  interrupt-map:
> +    minItems: 1
> +    maxItems: 2
> +
> +  interrupt-map-mask:
> +    items:
> +      - const: 0xffffffff
> +      - const: 0x0

[Severity: Medium]
Does masking out the second interrupt cell with 0x0 in interrupt-map-mask
work as intended when #interrupt-cells is set to 2?

For GPIOs, of_parse_phandle_with_args_map() in drivers/of/base.c supports
gpio-map-pass-thru = <0x0 0xffffffff> to forward the consumer's second
cell (flags) to the parent GPIO controller while masking it during lookup.

However, interrupt-map is parsed via of_irq_parse_one() ->
of_irq_parse_raw() -> of_irq_parse_imap_parent() in drivers/of/irq.c,
which does not support an interrupt-map-pass-thru mechanism and replaces
out_irq->args entirely with the parent interrupt specifier from
interrupt-map. With #interrupt-cells = <2> and interrupt-map-mask =
<0xffffffff 0x0> (as in the example's interrupt-map = <0 0 &gpio2 27
IRQ_TYPE_LEVEL_LOW>), the consumer's second cell is required in the DT
but silently masked out and ignored during interrupt resolution.

Should #interrupt-cells be 1 with a 1-cell interrupt-map-mask for an
index-only interrupt nexus (similar to microchip,mpfs-irqmux.yaml and
renesas,rzn1-gpioirqmux.yaml), or should interrupt-map-mask avoid masking
out the second cell?

> +
> +required:
> +  - compatible
> +  - '#gpio-cells'
> +  - gpio-map
> +  - gpio-map-mask
> +  - gpio-map-pass-thru

[Severity: Low]
Since the clock nexus properties (#clock-cells and clock-map) and
interrupt nexus properties (#address-cells, #interrupt-cells,
interrupt-map, and interrupt-map-mask) are optional in required:, can
omitting dependentRequired allow incomplete nexus nodes to pass schema
validation?

If a board defines interrupt-map without #address-cells = <0> or
#interrupt-cells, of_irq_parse_raw() in drivers/of/irq.c walks up to the
parent node and inherits the parent's #address-cells, breaking the
interrupt-map lookup. Likewise, #clock-cells and clock-map both need to
be present together for of_parse_phandle_with_args_map() to translate
clocks.

Also, should $schema at the top of fsl,aud-io-slot.yaml reference
http://devicetree.org/meta-schemas/core.yaml# instead of base.yaml#, and
should gpio-map, clock-map, and interrupt-map document which physical
connector signals correspond to each index so add-on board overlays and
base boards agree on the index ABI?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105064117.277495-1-chancel.liu@oss.nxp.com?part=1