[v5,1/3] dt-bindings: reset: microchip sparx5 reset driver bindings

Document the Sparx5 reset device driver bindings

The driver uses two IO ranges on sparx5 for access to
the reset control and the reset status.

Signed-off-by: Steen Hegelund <steen.hegelund@microchip.com>
---
 .../bindings/reset/microchip,rst.yaml         | 55 +++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/microchip,rst.yaml

On 10/02/2021 10:19:50+0100, Steen Hegelund wrote:
> Document the Sparx5 reset device driver bindings
> 
> The driver uses two IO ranges on sparx5 for access to
> the reset control and the reset status.
> 
> Signed-off-by: Steen Hegelund <steen.hegelund@microchip.com>
> ---
>  .../bindings/reset/microchip,rst.yaml         | 55 +++++++++++++++++++
>  1 file changed, 55 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/microchip,rst.yaml
> 
> diff --git a/Documentation/devicetree/bindings/reset/microchip,rst.yaml b/Documentation/devicetree/bindings/reset/microchip,rst.yaml
> new file mode 100644
> index 000000000000..80046172c9f8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/microchip,rst.yaml
> @@ -0,0 +1,55 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/reset/microchip,rst.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Microchip Sparx5 Switch Reset Controller
> +
> +maintainers:
> +  - Steen Hegelund <steen.hegelund@microchip.com>
> +  - Lars Povlsen <lars.povlsen@microchip.com>
> +
> +description: |
> +  The Microchip Sparx5 Switch provides reset control and implements the following
> +  functions
> +    - One Time Switch Core Reset (Soft Reset)
> +
> +properties:
> +  $nodename:
> +    pattern: "^reset-controller@[0-9a-f]+$"
> +
> +  compatible:
> +    const: microchip,sparx5-switch-reset
> +
> +  reg:
> +    items:
> +      - description: cpu block registers
> +      - description: global control block registers
> +
> +  reg-names:
> +    items:
> +      - const: cpu
> +      - const: gcb
> +

I still think this is not right because then you will be mapping the
same set of register using multiple drivers without any form of
synchronisation which will work because you are mapping the region
without requesting it. But this may lead to issues later.

At least, you should make cpu start at 0x80 and of size 4. Else, you
won't be able to define and use the GPRs that are in front of
CPU_REGS:RESET.

I would still keep DEVCPU_GCB:CHIP_REGS as a syscon, especially since
you are mapping the whole set of registers.


> +  "#reset-cells":
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - "#reset-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    reset: reset-controller@0 {
> +        compatible = "microchip,sparx5-switch-reset";
> +        #reset-cells = <1>;
> +        reg = <0x0 0xd0>,
> +              <0x11010000 0x10000>;
> +        reg-names = "cpu", "gcb";
> +    };
> +
> -- 
> 2.30.0
>

Hi Alex,

On Mon, 2021-02-15 at 18:32 +0100, Alexandre Belloni wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> On 10/02/2021 10:19:50+0100, Steen Hegelund wrote:
> > Document the Sparx5 reset device driver bindings
> > 
> > The driver uses two IO ranges on sparx5 for access to
> > the reset control and the reset status.
> > 
> > Signed-off-by: Steen Hegelund <steen.hegelund@microchip.com>
> > ---
> >  .../bindings/reset/microchip,rst.yaml         | 55
> > +++++++++++++++++++
> >  1 file changed, 55 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/reset/microchip,rst.yaml
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/reset/microchip,rst.yaml
> > b/Documentation/devicetree/bindings/reset/microchip,rst.yaml
> > new file mode 100644
> > index 000000000000..80046172c9f8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/reset/microchip,rst.yaml
> > @@ -0,0 +1,55 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: "http://devicetree.org/schemas/reset/microchip,rst.yaml#"
> > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > +
> > +title: Microchip Sparx5 Switch Reset Controller
> > +
> > +maintainers:
> > +  - Steen Hegelund <steen.hegelund@microchip.com>
> > +  - Lars Povlsen <lars.povlsen@microchip.com>
> > +
> > +description: |
> > +  The Microchip Sparx5 Switch provides reset control and
> > implements the following
> > +  functions
> > +    - One Time Switch Core Reset (Soft Reset)
> > +
> > +properties:
> > +  $nodename:
> > +    pattern: "^reset-controller@[0-9a-f]+$"
> > +
> > +  compatible:
> > +    const: microchip,sparx5-switch-reset
> > +
> > +  reg:
> > +    items:
> > +      - description: cpu block registers
> > +      - description: global control block registers
> > +
> > +  reg-names:
> > +    items:
> > +      - const: cpu
> > +      - const: gcb
> > +
> 
> I still think this is not right because then you will be mapping the
> same set of register using multiple drivers without any form of
> synchronisation which will work because you are mapping the region
> without requesting it. But this may lead to issues later.
> 
> At least, you should make cpu start at 0x80 and of size 4. Else, you
> won't be able to define and use the GPRs that are in front of
> CPU_REGS:RESET.
> 
> I would still keep DEVCPU_GCB:CHIP_REGS as a syscon, especially since
> you are mapping the whole set of registers.

Ok.  I will use a syscon for the General Control Block and a very small
range for the CPU Reset register, to minimize the register footprint.

> 
> 
> > +  "#reset-cells":
> > +    const: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - reg-names
> > +  - "#reset-cells"
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    reset: reset-controller@0 {
> > +        compatible = "microchip,sparx5-switch-reset";
> > +        #reset-cells = <1>;
> > +        reg = <0x0 0xd0>,
> > +              <0x11010000 0x10000>;
> > +        reg-names = "cpu", "gcb";
> > +    };
> > +
> > --
> > 2.30.0
> > 
> 
> --
> Alexandre Belloni, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com


Thanks for your comments