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@xxxxxxxxxxxxx> > > --- > > .../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@xxxxxxxxxxxxx> > > + - Lars Povlsen <lars.povlsen@xxxxxxxxxxxxx> > > + > > +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 -- BR Steen -=-=-=-=-=-=-=-=-=-=-=-=-=-= steen.hegelund@xxxxxxxxxxxxx