Re: [PATCH 08/20] dt-bindings: reset: add Synaptics SL261X SoCs

[Jisheng Zhang <jszhang@kernel.org>]

On Tue, Sep 29, 2026 at 08:44:13PM +0100, Conor Dooley wrote:
> On Tue, Sep 29, 2026 at 02:14:05PM +0800, Jisheng Zhang wrote:
> > Add device tree bindings for the resets on Synaptics SL261X SoCs.
> > 
> > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > ---
> >  .../bindings/reset/syna,sl261x-reset.yaml     | 40 ++++++++++
> >  include/dt-bindings/reset/syna,sl261x-reset.h | 80 +++++++++++++++++++
> >  2 files changed, 120 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/reset/syna,sl261x-reset.yaml
> >  create mode 100644 include/dt-bindings/reset/syna,sl261x-reset.h
> > 
> > diff --git a/Documentation/devicetree/bindings/reset/syna,sl261x-reset.yaml b/Documentation/devicetree/bindings/reset/syna,sl261x-reset.yaml
> > new file mode 100644
> > index 000000000000..8cfec17df800
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/reset/syna,sl261x-reset.yaml
> > @@ -0,0 +1,40 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +# Copyright (C) 2026 Synaptics Incorporated
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/reset/syna,sl261x-reset.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Synaptics SL261X reset controller
> > +
> > +maintainers:
> > +  - Jisheng Zhang <jszhang@kernel.org>
> > +
> > +description: The reset controller node must be a sub-node of the chip
> > +  controller node on SL261X SoCs.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - syna,sl261x-soc-reset
> > +      - syna,sl261x-system-reset
> > +
> > +  "#reset-cells":
> > +    const: 1
> > +
> > +required:
> > +  - compatible
> > +  - "#reset-cells"
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    chip: chip-control@f7e10000 {
> > +        reg = <0xf7e10000 0x1000>;
> > +
> > +        chip_rst: reset {
> > +            compatible = "syna,sl261x-soc-reset";
> > +            #reset-cells = <1>;
> > +        };
> 
> Why can't this just be part of the parent node?

Good question! IMHO, the reasons are:

1. the so called Gbls(Gbl means global) such as mcuGbl and
chip control Gbl contain not only reset, pinctrl and clk/plls
but also some other registers for different purposes, e.g in the
chip control gbl, there are some registers to control eth phy sel(RMII
or RGMII), and TXC 90 degree selection etc. These registers will
be used by the Ethernet driver;

While there's no pinctrl regs in avioGbl and vppGbl. In the vppgbl,
there are some registers for DPHYTX etc.

As can been, in different Gbls, there maybe different purposes registers, no
obvious patterns.

So the question here is why not split the gbl into different register
spaces, and abstract each space for each function group? Two blocking
points make this impossible and ugly: different reg space may not be
aligned at 4KB boundary, e.g reset: 0x350 ~ 0x37c; eth phy sel:0xa18;
some registers which belong to similar functionality, e.g clks, may not be
adjacent.

So the best way is to abstract these GBLs via. "syscon", "simple-mfd", and
put necessary subnodes such as reset, pinctrl and clks under the GBL
node. If the registers in GBL is misc control, pass the syscon phandle
to the main driver, e.g I planed to pass the syscon phandle to GMAC
driver to control the RMII/RGMII selection.

2.follow current bg2 and bg2q style
e.g arch/arm/boot/dts/synaptics/berlin2q.dtsi

IMHO, why bg2 and bg2q dtsi files are implemented as current
style is due to the above point 1.

Kindly let me know if there's a better solution.

> From a quick check of the dts, the node with this subnode didn't also
> have a system-reset node too, so there's no conflict or anything of that
> nature.
> 
> > +    };
> > diff --git a/include/dt-bindings/reset/syna,sl261x-reset.h b/include/dt-bindings/reset/syna,sl261x-reset.h
> > new file mode 100644
> > index 000000000000..7034f58aae5b
> > --- /dev/null
> > +++ b/include/dt-bindings/reset/syna,sl261x-reset.h
> > @@ -0,0 +1,80 @@
> > +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
> > +/*
> > + * Copyright (C) 2026 Synaptics Incorporated
> > + *
> > + * Author: Jisheng Zhang <jszhang@kernel.org>
> > + */
> > +
> > +#ifndef _DT_BINDINGS_SL261X_RESET_H
> > +#define _DT_BINDINGS_SL261X_RESET_H
> > +
> > +/* ACPU subsystem */
> > +#define RST_SOC_SDIO0		0
> > +#define RST_SOC_USB0		1
> > +#define RST_SOC_EMMC		2
> > +#define RST_SOC_GETH0		3
> > +#define RST_SOC_SDIO1		4
> > +#define RST_SOC_USB1		5
> > +#define RST_SOC_GETH1		6
> > +#define RST_SOC_USB0PHY		7
> > +#define RST_SOC_USB0CORE	8
> > +#define RST_SOC_USB0MAHB	9
> > +#define RST_SOC_USB1PHY		10
> > +#define RST_SOC_USB1CORE	11
> > +#define RST_SOC_USB1MAHB	12
> > +#define RST_SOC_UART0		13
> > +#define RST_SOC_UART1		14
> > +#define RST_SOC_UART2		15
> > +#define RST_SOC_UART3		16
> > +#define RST_SOC_I2C0		17
> > +#define RST_SOC_I2C1		18
> > +#define RST_SOC_SPI0		19
> > +#define RST_SOC_SPI1		20
> > +#define RST_SOC_SPI2		21
> > +#define RST_SOC_SPI3		22
> > +#define RST_SOC_APBTIMERS	23
> > +#define RST_SOC_APBSYSCNT	24
> > +#define RST_SOC_APBWDT		25
> > +#define RST_SOC_APBGPIO		26
> > +#define RST_SOC_APBDMA		27
> > +#define RST_SOC_GPUCORE		28
> > +#define RST_SOC_NPUCORE		29
> > +#define RST_SOC_AVIOAIOG	30
> > +#define RST_SOC_AVIOVPPG	31
> > +#define RST_SOC_AVIOVIPG	32
> > +
> > +/* system subsystem */
> > +#define RST_SM_ADCCORE		0
> > +#define RST_SM_ADCPRST		1
> > +#define RST_SM_CAN0PRST		2
> > +#define RST_SM_CAN0SRST		3
> > +#define RST_SM_CAN1PRST		4
> > +#define RST_SM_CAN1SRST		5
> > +#define RST_SM_GPIOPRST		6
> > +#define RST_SM_GPIOSRST		7
> > +#define RST_SM_I2CM0PRST	8
> > +#define RST_SM_I2CM0SRST	9
> > +#define RST_SM_I2CM1PRST	10
> > +#define RST_SM_I2CM1SRST	11
> > +#define RST_SM_I3CPRST		12
> > +#define RST_SM_I3CSRST		13
> > +#define RST_SM_PDMPRST		14
> > +#define RST_SM_PDMSRST		15
> > +#define RST_SM_PVTHSRST		16
> > +#define RST_SM_PVTPRST		17
> > +#define RST_SM_PWMPERIRST	18
> > +#define RST_SM_PWMPRST		19
> > +#define RST_SM_SPIMPRST		20
> > +#define RST_SM_SPIMSRST		21
> > +#define RST_SM_SPISPRST		22
> > +#define RST_SM_SPISSRST		23
> > +#define RST_SM_UART0PRST	24
> > +#define RST_SM_UART0SRST	25
> > +#define RST_SM_UART1PRST	26
> > +#define RST_SM_UART1SRST	27
> > +#define RST_SM_UART2PRST	28
> > +#define RST_SM_UART2SRST	29
> > +#define RST_SM_UART3PRST	30
> > +#define RST_SM_UART3SRST	31
> > +
> > +#endif /* _DT_BINDINGS_SL261X_RESET_H */
> > -- 
> > 2.53.0
> >