On Tue, Nov 10, 2026 at 06:39:39PM +0100, John Crispin wrote:

> diff --git a/Documentation/devicetree/bindings/pinctrl/airoha,an7589-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/airoha,an7589-pinctrl.yaml
> new file mode 100644
> index 000000000000..546e333f3467
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/airoha,an7589-pinctrl.yaml
> @@ -0,0 +1,533 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/airoha,an7589-pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Airoha AN7589 Pin Controller
> +
> +maintainers:
> +  - John Crispin <john@phrozen.org>
> +
> +description:
> +  The Airoha AN7589 pin controller is used to control SoC pins. Pin
> +  functions are selected through a separate IOMUX register block, one
> +  mode field per pin. Every pin is a GPIO in mode 0.
> +
> +properties:
> +  compatible:
> +    const: airoha,an7589-pinctrl
> +
> +  airoha,iomux:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      Phandle to the IOMUX syscon holding the per pin mode fields.
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  gpio-controller: true
> +
> +  '#gpio-cells':
> +    const: 2
> +
> +  gpio-ranges:
> +    maxItems: 1
> +
> +  interrupt-controller: true
> +
> +  '#interrupt-cells':
> +    const: 2
> +
> +allOf:
> +  - $ref: pinctrl.yaml#
> +
> +required:
> +  - compatible
> +  - airoha,iomux
> +  - interrupts
> +  - gpio-controller
> +  - "#gpio-cells"
> +  - interrupt-controller
> +  - "#interrupt-cells"
> +
> +patternProperties:
> +  '-pins$':
> +    type: object
> +
> +    patternProperties:
> +      '^mux(-|$)':
> +        type: object
> +
> +        description:
> +          pinmux configuration nodes.
> +
> +        $ref: /schemas/pinctrl/pinmux-node.yaml
> +
> +        properties:
> +          function:
> +            description:
> +              A string containing the name of the function to mux to the group.
> +            enum: [uart1, uart2, uart3, uart4, uart5, uart6, i2c0, i2c1,
> +                   i2c2, i2c3, i2c4, spi, ext_spi1, ext_spi2, ext_spi3,
> +                   pcm1, pcm2, pcm_spi, i2s, mdio0, mdio1, mdio2, mdcs,
> +                   mdios, emmc, pcie0, pcie1, pcie2, pon1, pon2, sgpio0,
> +                   sgpio1, watchdog, shiftmux_spi, tod, sync_e, cpu_jtag,
> +                   npu_jtag, ibex_jtag, pma50eth_jtag, ainpu_jtag,
> +                   pma50pon_jtag, pwm]
> +
> +          groups:
> +            description:
> +              An array of strings. Each string contains the name of a group.
> +
> +        required:
> +          - function
> +          - groups
> +
> +        allOf:
> +          - if:
> +              properties:
> +                function:
> +                  const: uart1
> +            then:
> +              properties:
> +                groups:
> +                  enum: [uart1]

This stuff here is a signal to me that the functions are too detailed.
Each group here can only be one uart, so all you need is a function
called "uart"?

That said, you need to define the groups as you've done for the
functions.

> +          - if:
> +              properties:
> +                function:
> +                  const: uart2
> +            then:
> +              properties:
> +                groups:
> +                  enum: [uart2]
> +          - if:
> +              properties:
> +                function:
> +                  const: uart3
> +            then:
> +              properties:
> +                groups:
> +                  items:
> +                    enum: [uart3, uart3_alt]
> +                  maxItems: 2
> +          - if:
> +              properties:
> +                function:
> +                  const: uart4
> +            then:
> +              properties:
> +                groups:
> +                  enum: [uart4]
> +          - if:
> +              properties:
> +                function:
> +                  const: uart5
> +            then:
> +              properties:
> +                groups:
> +                  items:
> +                    enum: [uart5, uart5_alt, uart5_full]
> +                  maxItems: 3
> +          - if:
> +              properties:
> +                function:
> +                  const: uart6
> +            then:
> +              properties:
> +                groups:
> +                  items:
> +                    enum: [uart6, uart6_full]
> +                  maxItems: 2
> +          - if:
> +              properties:
> +                function:
> +                  const: i2c0
> +            then:
> +              properties:
> +                groups:
> +                  enum: [i2c0]

It's the same ehre, seemingly i2c0 can only go to the group named i2c0,
so there's no point having each i2c as a different function.


This seems to apply to every function here, so the binding can be
simplified significantly.

pw-bot: changes-requested

Thanks,
Conor.

> +          - if:
> +              properties:
> +                function:
> +                  const: i2c1
> +            then:
> +              properties:
> +                groups:
> +                  enum: [i2c1]
> +          - if:
> +              properties:
> +                function:
> +                  const: i2c2
> +            then:
> +              properties:
> +                groups:
> +                  enum: [i2c2]
> +          - if:
> +              properties:
> +                function:
> +                  const: i2c3
> +            then:
> +              properties:
> +                groups:
> +                  enum: [i2c3]
> +          - if:
> +              properties:
> +                function:
> +                  const: i2c4
> +            then:
> +              properties:
> +                groups:
> +                  enum: [i2c4]
> +          - if:
> +              properties:
> +                function:
> +                  const: spi
> +            then:
> +              properties:
> +                groups:
> +                  items:
> +                    enum: [spi0, ext_spi0_full]
> +                  maxItems: 2
> +          - if:
> +              properties:
> +                function:
> +                  const: ext_spi1
> +            then:
> +              properties:
> +                groups:
> +                  items:
> +                    enum: [ext_spi1, ext_spi1_full]
> +                  maxItems: 2
> +          - if:
> +              properties:
> +                function:
> +                  const: ext_spi2
> +            then:
> +              properties:
> +                groups:
> +                  items:
> +                    enum: [ext_spi2, ext_spi2_full]
> +                  maxItems: 2
> +          - if:
> +              properties:
> +                function:
> +                  const: ext_spi3
> +            then:
> +              properties:
> +                groups:
> +                  items:
> +                    enum: [ext_spi3, ext_spi3_full]
> +                  maxItems: 2
> +          - if:
> +              properties:
> +                function:
> +                  const: pcm1
> +            then:
> +              properties:
> +                groups:
> +                  enum: [pcm1]
> +          - if:
> +              properties:
> +                function:
> +                  const: pcm2
> +            then:
> +              properties:
> +                groups:
> +                  enum: [pcm2]
> +          - if:
> +              properties:
> +                function:
> +                  const: pcm_spi
> +            then:
> +              properties:
> +                groups:
> +                  items:
> +                    enum: [pcm_spi, pcm_spi_cs, pcm_ctrl]
> +                  maxItems: 3
> +          - if:
> +              properties:
> +                function:
> +                  const: i2s
> +            then:
> +              properties:
> +                groups:
> +                  enum: [i2s]
> +          - if:
> +              properties:
> +                function:
> +                  const: mdio0
> +            then:
> +              properties:
> +                groups:
> +                  enum: [mdio0]
> +          - if:
> +              properties:
> +                function:
> +                  const: mdio1
> +            then:
> +              properties:
> +                groups:
> +                  enum: [mdio1]
> +          - if:
> +              properties:
> +                function:
> +                  const: mdio2
> +            then:
> +              properties:
> +                groups:
> +                  enum: [mdio2]
> +          - if:
> +              properties:
> +                function:
> +                  const: mdcs
> +            then:
> +              properties:
> +                groups:
> +                  enum: [mdcs]
> +          - if:
> +              properties:
> +                function:
> +                  const: mdios
> +            then:
> +              properties:
> +                groups:
> +                  enum: [mdios]
> +          - if:
> +              properties:
> +                function:
> +                  const: emmc
> +            then:
> +              properties:
> +                groups:
> +                  enum: [emmc]
> +          - if:
> +              properties:
> +                function:
> +                  const: pcie0
> +            then:
> +              properties:
> +                groups:
> +                  items:
> +                    enum: [pcie0_rst, pcie0_clkreq, pcie0_wake]
> +                  maxItems: 3
> +          - if:
> +              properties:
> +                function:
> +                  const: pcie1
> +            then:
> +              properties:
> +                groups:
> +                  items:
> +                    enum: [pcie1_rst, pcie1_clkreq, pcie1_wake]
> +                  maxItems: 3
> +          - if:
> +              properties:
> +                function:
> +                  const: pcie2
> +            then:
> +              properties:
> +                groups:
> +                  items:
> +                    enum: [pcie2_rst, pcie2_clkreq, pcie2_wake]
> +                  maxItems: 3
> +          - if:
> +              properties:
> +                function:
> +                  const: pon1
> +            then:
> +              properties:
> +                groups:
> +                  enum: [pon1]
> +          - if:
> +              properties:
> +                function:
> +                  const: pon2
> +            then:
> +              properties:
> +                groups:
> +                  enum: [pon2]
> +          - if:
> +              properties:
> +                function:
> +                  const: sgpio0
> +            then:
> +              properties:
> +                groups:
> +                  enum: [sgpio0]
> +          - if:
> +              properties:
> +                function:
> +                  const: sgpio1
> +            then:
> +              properties:
> +                groups:
> +                  enum: [sgpio1]
> +          - if:
> +              properties:
> +                function:
> +                  const: watchdog
> +            then:
> +              properties:
> +                groups:
> +                  enum: [watchdog]
> +          - if:
> +              properties:
> +                function:
> +                  const: shiftmux_spi
> +            then:
> +              properties:
> +                groups:
> +                  enum: [shiftmux_spi]
> +          - if:
> +              properties:
> +                function:
> +                  const: tod
> +            then:
> +              properties:
> +                groups:
> +                  enum: [tod]
> +          - if:
> +              properties:
> +                function:
> +                  const: sync_e
> +            then:
> +              properties:
> +                groups:
> +                  enum: [sync_e]
> +          - if:
> +              properties:
> +                function:
> +                  const: cpu_jtag
> +            then:
> +              properties:
> +                groups:
> +                  enum: [cpu_jtag, cpu_jtag_alt]
> +          - if:
> +              properties:
> +                function:
> +                  const: npu_jtag
> +            then:
> +              properties:
> +                groups:
> +                  enum: [npu_jtag]
> +          - if:
> +              properties:
> +                function:
> +                  const: ibex_jtag
> +            then:
> +              properties:
> +                groups:
> +                  enum: [ibex_jtag]
> +          - if:
> +              properties:
> +                function:
> +                  const: pma50eth_jtag
> +            then:
> +              properties:
> +                groups:
> +                  enum: [pma50eth_jtag]
> +          - if:
> +              properties:
> +                function:
> +                  const: ainpu_jtag
> +            then:
> +              properties:
> +                groups:
> +                  enum: [ainpu_jtag]
> +          - if:
> +              properties:
> +                function:
> +                  const: pma50pon_jtag
> +            then:
> +              properties:
> +                groups:
> +                  enum: [pma50pon_jtag]
> +          - if:
> +              properties:
> +                function:
> +                  const: pwm
> +            then:
> +              properties:
> +                groups:
> +                  items:
> +                    enum: [gpio0, gpio1, gpio2, gpio3, gpio4, gpio5, gpio6,
> +                           gpio7, gpio8, gpio9, gpio10, gpio11, gpio12,
> +                           gpio13, gpio14, gpio15]
> +
> +        additionalProperties: false
> +
> +      '^conf(-|$)':
> +        type: object
> +
> +        description:
> +          pinconf configuration nodes.
> +
> +        $ref: /schemas/pinctrl/pincfg-node.yaml
> +
> +        properties:
> +          pins:
> +            description:
> +              An array of strings. Each string contains the name of a pin.
> +            items:
> +              pattern: '^gpio([0-9]|[1-6][0-9]|7[0-3])$'
> +            minItems: 1
> +            maxItems: 74
> +
> +          input-enable: true
> +
> +          output-enable: true
> +
> +          output-low: true
> +
> +          output-high: true
> +
> +        required:
> +          - pins
> +
> +        additionalProperties: false
> +
> +    additionalProperties: false
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    pinctrl {
> +      compatible = "airoha,an7589-pinctrl";
> +      airoha,iomux = <&iomux>;
> +
> +      interrupt-parent = <&gic>;
> +      interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
> +
> +      gpio-controller;
> +      #gpio-cells = <2>;
> +
> +      interrupt-controller;
> +      #interrupt-cells = <2>;
> +
> +      gpio-ranges = <&pinctrl 0 0 74>;
> +
> +      mmc-pins {
> +        mux {
> +          function = "emmc";
> +          groups = "emmc";
> +        };
> +      };
> +
> +      pcie0-pins {
> +        mux {
> +          function = "pcie0";
> +          groups = "pcie0_rst", "pcie0_clkreq", "pcie0_wake";
> +        };
> +      };
> +
> +      uart5-pins {
> +        mux {
> +          function = "uart5";
> +          groups = "uart5", "uart5_full";
> +        };
> +      };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index aa647f5d37dd..2041a8aed28f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -22137,6 +22137,8 @@ PIN CONTROLLER - AIROHA
>  M:	Lorenzo Bianconi <lorenzo@kernel.org>
>  L:	linux-mediatek@lists.infradead.org (moderated for non-subscribers)
>  S:	Maintained
> +F:	Documentation/devicetree/bindings/mfd/airoha,an7589-gpio-sysctl.yaml
> +F:	Documentation/devicetree/bindings/pinctrl/airoha,an7589-pinctrl.yaml
>  F:	Documentation/devicetree/bindings/pinctrl/airoha,en7581-pinctrl.yaml
>  F:	drivers/pinctrl/airoha/pinctrl-airoha.c
>  
> -- 
> 2.34.1
>