回复: [PATCH v5 1/3] dt-bindings: reset: add sky1 reset controller

[Gary Yang]

Hi Rob:

Thanks for your comments

> EXTERNAL EMAIL
> 
> On Thu, Jan 29, 2026 at 03:00:44PM +0800, Gary Yang wrote:
> > There are two reset controllers on Cix sky1 Soc. One is located in S0
> > domain, and the other is located in S0 and S5 domain.
> 
> The comments on v4 still apply... Repeated below...
> 

Yes, I will read carefully those comments on V3 and V4, Thanks for your kindly remind

> >
> > Signed-off-by: Gary Yang <gary.yang at cixtech.com>
> > Signed-off-by: Peter Chen <peter.chen at cixtech.com>
> > ---
> >  .../bindings/reset/cix,sky1-rst.yaml          |  41 +++++
> >  .../soc/cix/cix,sky1-system-controller.yaml   |  48 +++++
> >  include/dt-bindings/reset/cix,sky1-rst-fch.h  |  42 +++++
> >  include/dt-bindings/reset/cix,sky1-rst.h      | 164 ++++++++++++++++++
> >  4 files changed, 295 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml
> >  create mode 100644
> > Documentation/devicetree/bindings/soc/cix/cix,sky1-system-controller.y
> > aml  create mode 100644 include/dt-bindings/reset/cix,sky1-rst-fch.h
> >  create mode 100644 include/dt-bindings/reset/cix,sky1-rst.h
> >
> > diff --git a/Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml
> > b/Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml
> > new file mode 100644
> > index 000000000000..b52791e2b18e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml
> > @@ -0,0 +1,41 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/reset/cix,sky1-rst.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: CIX Sky1 Reset Controller
> > +
> > +maintainers:
> > +  - Gary Yang <gary.yang at cixtech.com>
> > +
> > +description: |
> > +  CIX Sky1 reset controller can be used to reset various set of peripherals.
> > +  There are two reset controllers, one is located in S0 domain, the
> > +other
> > +  is located in S0 and S5 domain.
> > +
> > +  See also:
> > +  - include/dt-bindings/reset/cix,sky1-rst.h
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - cix,sky1-rst
> > +          - cix,sky1-rst-fch
> > +
> > +  '#reset-cells':
> > +    const: 1
> > +
> > +required:
> > +  - compatible
> > +  - '#reset-cells'
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    reset-controller {
> > +      compatible = "cix,sky1-rst";
> > +      #reset-cells = <1>;
> > +    };
> > diff --git
> > a/Documentation/devicetree/bindings/soc/cix/cix,sky1-system-controller
> > .yaml
> > b/Documentation/devicetree/bindings/soc/cix/cix,sky1-system-controller
> > .yaml
> > new file mode 100644
> > index 000000000000..913e77fc522a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/soc/cix/cix,sky1-system-contro
> > +++ ller.yaml
> > @@ -0,0 +1,48 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause %YAML 1.2
> > +---
> > +$id:
> > +http://devicetree.org/schemas/soc/cix/cix,sky1-system-controller.yaml
> > +#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Cix Sky1 SoC system controller register region
> > +
> > +maintainers:
> > +  - Gary Yang <gary.yang at cixtech.com>
> > +
> > +description:
> > +  An wide assortment of registers of the system controller on Sky1
> > +SoC,
> > +  including resets and usb.
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - cix,sky1-system-controller
> > +          - cix,sky1-s5-system-controller
> > +      - const: syscon
> > +      - const: simple-mfd
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +patternProperties:
> > +  "^reset(-controller)?$":
> > +    type: object
> > +    $ref: /schemas/reset/cix,sky1-rst.yaml#
> 
> This is not a separate device.
> 

As Conor's suggestion in V3, we refer to microchip,mpfs-mss-top-sysreg.yaml. In this file, you can see codes below
40
41   pinctrl at 200:
42     type: object
43     $ref: /schemas/pinctrl/microchip,mpfs-pinctrl-iomux0.yaml

and in microchip,mpfs-pinctrl-iomux0.yaml, you can also see:

73       pinctrl at 200 {
74         compatible = "microchip,mpfs-pinctrl-iomux0";
75         reg = <0x200 0x4>;
76         ........
86       };

pinctrl at 200 is also a device node, all right? If not, how to describe a separate device?

> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    syscon at 16000000 {
> > +      compatible = "cix,sky1-s5-system-controller", "syscon",
> "simple-mfd";
> > +      reg = <0x16000000 0x1000>;
> > +      reset-controller {
> > +        compatible = "cix,sky1-rst";
> > +        #reset-cells = <1>;
> 
> Move #reset-cells to the parent node and drop this node. And then drop
> "simple-mfd". Or perhaps the definition for what's in the
> "cix,sky1-s5-system-controller" is incomplete? If so, please define all the
> binding.
> 
> Rob

If we move #reset-cells to the parent node, the parent node acts as a reset controller. All right?
There is not only a reset controller, and are some other modules. How do we add these modules
to the parent node in the future? If you feel confuse this scheme, Can we refer to the syscon node 
via phandle?

Best Regards
Gary