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: 9+ 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-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-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-10 17:39 ` [PATCH 7/7] pinctrl: airoha: add support of an7589 SoC John Crispin
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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®