From: Conor Dooley <conor@kernel.org>
To: John Crispin <john@phrozen.org>
Cc: Lorenzo Bianconi <lorenzo@kernel.org>, Lee Jones <lee@kernel.org>,
Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
Linus Walleij <linusw@kernel.org>,
Bartosz Golaszewski <brgl@kernel.org>,
Ahmed Naseef <naseefkm@gmail.com>,
Benjamin Larsson <benjamin.larsson@genexis.eu>,
Christian Marangi <ansuelsmth@gmail.com>,
devicetree@vger.kernel.org, lei.dong@airoha.com,
Linqi.zhang@airoha.com, linux-gpio@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
mfd@lists.linux.dev,
Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>,
Rajeev.kumar@airoha.com, upstream@airoha.com,
Weifeng.xu@airoha.com, zhengping.zhang@airoha.com
Subject: Re: [PATCH 5/7] dt-bindings: pinctrl: airoha: add AN7589
Date: Tue, 10 Nov 2026 18:27:58 +0000 [thread overview]
Message-ID: <20261110-attribute-animosity-720536bc561a@spud> (raw)
In-Reply-To: <20261110173941.301478-6-john@phrozen.org>
[-- Attachment #1: Type: text/plain, Size: 16936 bytes --]
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
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
next prev parent reply other threads:[~2026-11-10 18:28 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-10 17:39 [PATCH 0/7] pinctrl: airoha: add AN7589 support John Crispin
2026-11-10 17:39 ` [PATCH 1/7] pinctrl: airoha: make gpio register layout per SoC John Crispin
2026-11-11 11:22 ` Lorenzo Bianconi
2026-11-10 17:39 ` [PATCH 2/7] pinctrl: airoha: add per pin mode field mux John Crispin
2026-11-10 17:39 ` [PATCH 3/7] pinctrl: airoha: make the chip SCU optional John Crispin
2026-11-11 11:27 ` Lorenzo Bianconi
2026-11-10 17:39 ` [PATCH 4/7] pinctrl: airoha: mux a requested GPIO through the IOMUX John Crispin
2026-11-10 17:39 ` [PATCH 5/7] dt-bindings: pinctrl: airoha: add AN7589 John Crispin
2026-11-10 18:27 ` Conor Dooley [this message]
2026-11-10 17:39 ` [PATCH 6/7] pinctrl: airoha: move the GPIO register offsets to the common header John Crispin
2026-11-11 11:29 ` Lorenzo Bianconi
2026-11-10 17:39 ` [PATCH 7/7] pinctrl: airoha: add support of an7589 SoC John Crispin
2026-11-11 11:32 ` Lorenzo Bianconi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261110-attribute-animosity-720536bc561a@spud \
--to=conor@kernel.org \
--cc=Linqi.zhang@airoha.com \
--cc=Rajeev.kumar@airoha.com \
--cc=Weifeng.xu@airoha.com \
--cc=ansuelsmth@gmail.com \
--cc=benjamin.larsson@genexis.eu \
--cc=brgl@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=john@phrozen.org \
--cc=krzk+dt@kernel.org \
--cc=lee@kernel.org \
--cc=lei.dong@airoha.com \
--cc=linusw@kernel.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=lorenzo@kernel.org \
--cc=mfd@lists.linux.dev \
--cc=mikhail.kshevetskiy@iopsys.eu \
--cc=naseefkm@gmail.com \
--cc=robh@kernel.org \
--cc=upstream@airoha.com \
--cc=zhengping.zhang@airoha.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.