Re: [PATCH v2 2/3] dt-bindings: pinctrl: Add EcoNet EN7528 pin controller

[Conor Dooley]

On Sat, Aug 29, 2026 at 03:47:00PM +0400, Ahmed Naseef wrote:
> On Fri, Aug 28, 2026 at 04:51:27PM +0100, Conor Dooley wrote:
> > On Fri, Aug 28, 2026 at 07:35:14PM +0400, Ahmed Naseef wrote:
> > > Add the binding for the pin controller found on the EcoNet EN7528 MIPS
> > > SoC. It shares the IP with the Airoha pin controllers, but its IOMUX and
> > > pin configuration registers have a different layout, so it gets its own
> > > compatible.
> > > 
> > > The CHIP SCU holding those registers is referenced by the airoha,chip-scu
> > > phandle.
> > > 
> > > Signed-off-by: Ahmed Naseef <naseefkm@xxxxxxxxx>
> > > ---
> > >  .../pinctrl/econet,en7528-pinctrl.yaml        | 190 ++++++++++++++++++
> > >  1 file changed, 190 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/pinctrl/econet,en7528-pinctrl.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/pinctrl/econet,en7528-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/econet,en7528-pinctrl.yaml
> > > new file mode 100644
> > > index 000000000000..3ce1a5309d09
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/pinctrl/econet,en7528-pinctrl.yaml
> > > @@ -0,0 +1,190 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/pinctrl/econet,en7528-pinctrl.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: EcoNet EN7528 Pin Controller
> > > +
> > > +maintainers:
> > > +  - Lorenzo Bianconi <lorenzo@xxxxxxxxxx>
> > > +
> > > +description:
> > > +  The EcoNet EN7528 pin controller is used to control SoC pins. It shares the
> > > +  IP with the Airoha pin controllers, but its IOMUX and pin configuration
> > > +  registers have a different layout.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: econet,en7528-pinctrl
> > > +
> > > +  airoha,chip-scu:
> > > +    description: phandle to the chip SCU syscon
> > > +    $ref: /schemas/types.yaml#/definitions/phandle
> > > +
> > > +  interrupts:
> > > +    maxItems: 1
> > > +
> > > +  gpio-controller: true
> > > +
> > > +  '#gpio-cells':
> > > +    const: 2
> > > +
> > > +  gpio-ranges:
> > > +    maxItems: 1
> > > +
> > > +  interrupt-controller: true
> > > +
> > > +  '#interrupt-cells':
> > > +    const: 2
> > > +
> > > +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: [gpio, jtag, pcie_reset, pcm, pcm_spi, phy1_led0,
> > > +                   phy1_led1, phy2_led0, phy2_led1, phy3_led0, phy3_led1,
> > > +                   phy4_led0, phy4_led1, pnand, pon, pwm, sipo, spi, uart]
> > > +
> > > +          groups:
> > > +            description:
> > > +              An array of strings. Each string contains the name of a group.
> > > +            items:
> > > +              enum: [pon, sipo, sipo_rclk, uart2, uart2_cts_rts, uart3,
> > > +                     ejtag, pcm1, pcm2, spi_quad, spi_cs1, pcm_spi,
> > > +                     pcm_spi_int, pcm_spi_rst, pcm_spi_cs1, pcm_spi_cs2,
> > > +                     pcm_spi_cs3, pcm_spi_cs4, pcm_spi_cs5, pcm_spi_cs6,
> > > +                     pcm_spi_cs7, pnand, gpio0, gpio1, gpio2, gpio3, gpio4,
> > > +                     gpio5, gpio6, gpio7, gpio8, gpio9, gpio10, gpio11,
> > > +                     gpio12, gpio13, gpio14, gpio15, gpio16, gpio17,
> > > +                     gpio18, gpio19, gpio20, gpio21, gpio22, gpio23,
> > > +                     gpio24, gpio25, gpio26, gpio27, gpio28, gpio29,
> > > +                     gpio30, gpio31, gpio32, gpio33, gpio34, gpio35,
> > > +                     gpio36, gpio37, gpio38, gpio39, gpio40, gpio41,
> > > +                     pcie_reset0, pcie_reset1]
> > > +
> > > +        required:
> > > +          - function
> > > +          - groups
> > > +
> > > +        additionalProperties: false
> > > +
> > > +      '^conf(-|$)':
> > 
> > Why is this needed? Why can you not tell from the group which pins the
> > config applies to?
> 
> Not for all of them: i2c_sda, i2c_scl, uart_txd and uart_rxd are in no
> group at all.

Why's that? Are they just fixed output pins?

> The spi_cs0/clk/mosi/miso pads are only in pnand, which is
> 17 pins wide, so no group names them individually either.

That's easily resolved by breaking up pnand though.

Devils advocate thing here might be to say that the dedicated
configuration node should only be for the things that are fixed outputs,
and the muxable ones should get their config from a combined
pinmux/pincfg node. Personally I think that presents a nicer interface,
where someone doesn't have to go figure out what pins are in the group
they just assigned a function to to also set the config properties for
it.

Cheers,
Conor.

> > > +        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:
> > > +              enum: [gpio0, gpio1, gpio2, gpio3, gpio4, gpio5, gpio6,
> > > +                     gpio7, gpio8, gpio9, gpio10, gpio11, gpio12, gpio13,
> > > +                     gpio14, gpio15, gpio16, gpio17, gpio18, gpio19,
> > > +                     gpio20, gpio21, gpio22, gpio23, gpio24, gpio25,
> > > +                     gpio26, gpio27, gpio28, gpio29, gpio30, gpio31,
> > > +                     gpio32, gpio33, gpio34, gpio35, gpio36, gpio37,
> > > +                     gpio38, gpio39, pcie_reset0, pcie_reset1, i2c_sda,
> > > +                     i2c_scl, uart_txd, uart_rxd, spi_cs0, spi_clk,
> > > +                     spi_mosi, spi_miso]
> > > +            minItems: 1
> > > +            maxItems: 50
> > > +
> > > +          bias-disable: true
> > > +
> > > +          bias-pull-up: true
> > > +
> > > +          bias-pull-down: true
> > > +
> > > +          input-enable: true
> > > +
> > > +          output-enable: true
> > > +
> > > +          output-low: true
> > > +
> > > +          output-high: true
> > > +
> > > +          drive-strength:
> > > +            description:
> > > +              Selects the drive strength for the pin, in mA.
> > > +            enum: [2, 4]
> > > +
> > > +        required:
> > > +          - pins
> > > +
> > > +        additionalProperties: false
> > > +
> > > +    additionalProperties: false
> > > +
> > > +required:
> > > +  - compatible
> > > +  - airoha,chip-scu
> > > +  - interrupts
> > > +  - gpio-controller
> > > +  - gpio-ranges
> > > +  - "#gpio-cells"
> > > +  - interrupt-controller
> > > +  - "#interrupt-cells"
> > > +
> > > +allOf:
> > > +  - $ref: pinctrl.yaml#
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    #include <dt-bindings/interrupt-controller/mips-gic.h>
> > > +    #include <dt-bindings/interrupt-controller/irq.h>
> > > +
> > > +    pinctrl {
> > > +      compatible = "econet,en7528-pinctrl";
> > > +
> > > +      airoha,chip-scu = <&chip_scu>;
> > > +
> > > +      interrupt-parent = <&gic>;
> > > +      interrupts = <GIC_SHARED 10 IRQ_TYPE_LEVEL_HIGH>;
> > > +
> > > +      gpio-controller;
> > > +      #gpio-cells = <2>;
> > > +
> > > +      interrupt-controller;
> > > +      #interrupt-cells = <2>;
> > > +
> > > +      gpio-ranges = <&pinctrl 0 0 42>;
> > > +
> > > +      uart2-pins {
> > > +        mux {
> > > +          function = "uart";
> > > +          groups = "uart2";
> > > +        };
> > > +
> > > +        conf {
> > > +          pins = "gpio28", "gpio29";
> > > +          bias-pull-up;
> > > +        };
> > > +      };
> > > +
> > > +      pwm-pins {
> > > +        mux {
> > > +          function = "pwm";
> > > +          groups = "gpio30";
> > > +        };
> > > +      };
> > > +    };
> > > -- 
> > > 2.34.1
> > > 
> 
>

Attachment: signature.asc
Description: PGP signature