[PATCH 0/3] staging: mt7621-dts: complete reset missing stuff

[PATCH 0/3] staging: mt7621-dts: complete reset missing stuff

[Sergio Paracuellos]

Hi all,

Mt7621 SoC uses reset controller from ralink rt2880.

In order to get 'mt7621.dtsi' file out of staging there are still
some nodes which are not documented at all. This patch series
add documentation for reset controller in ralink rt2880 derived
SoCs which corresponds with 'ralink,rt2880-reset' compatible.

It also align new binding documentation using defined reset bits
along the device tree file itself.

Thanks in advance for your time.

Best regards,
    Sergio Paracuellos

Sergio Paracuellos (3):
  dt-bindings: reset: add dt binding header for ralink RT2880 resets
  dt: bindings: add ralink RT2880 resets device tree binding
    documentation
  staging: mt7621-dts: align resets with binding documentation

 .../bindings/reset/ralink,rt2880-reset.yaml   | 39 ++++++++++++++++++
 drivers/staging/mt7621-dts/mt7621.dtsi        | 23 ++++++-----
 include/dt-bindings/reset/ralink-rt2880.h     | 40 +++++++++++++++++++
 3 files changed, 91 insertions(+), 11 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/reset/ralink,rt2880-reset.yaml
 create mode 100644 include/dt-bindings/reset/ralink-rt2880.h

-- 
2.25.1

[PATCH 1/3] dt-bindings: reset: add dt binding header for ralink RT2880 resets

[Sergio Paracuellos]

Adds dt binding header for 'ralink,rt2880-reset' resets.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 include/dt-bindings/reset/ralink-rt2880.h | 40 +++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 include/dt-bindings/reset/ralink-rt2880.h

diff --git a/include/dt-bindings/reset/ralink-rt2880.h b/include/dt-bindings/reset/ralink-rt2880.h
new file mode 100644
index 000000000000..266ef521a584
--- /dev/null
+++ b/include/dt-bindings/reset/ralink-rt2880.h
@@ -0,0 +1,40 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2021 Sergio Paracuellos
+ * Author: Sergio Paracuellos <sergio.paracuellos@gmail.com>
+ */
+
+#ifndef DT_BINDING_RALINK_RT2880_RESET_H
+#define DT_BINDING_RALINK_RT2880_RESET_H
+
+/*
+ * Ralink RT2880 SoC reset controller register bits.
+ */
+#define RALINK_RT2880_SYS	0
+#define RALINK_RT2880_MCM	2
+#define RALINK_RT2880_HSDMA	5
+#define RALINK_RT2880_FE	6
+#define RALINK_RT2880_SPDIFTX	7
+#define RALINK_RT2880_TIMER	8
+#define RALINK_RT2880_INT	9
+#define RALINK_RT2880_MC	10
+#define RALINK_RT2880_PCM	11
+#define RALINK_RT2880_PIO	13
+#define RALINK_RT2880_GDMA	14
+#define RALINK_RT2880_NFI	15
+#define RALINK_RT2880_I2C	16
+#define RALINK_RT2880_I2S	17
+#define RALINK_RT2880_SPI	18
+#define RALINK_RT2880_UART1	19
+#define RALINK_RT2880_UART2	20
+#define RALINK_RT2880_UART3	21
+#define RALINK_RT2880_ETH	23
+#define RALINK_RT2880_PCIE0	24
+#define RALINK_RT2880_PCIE1	25
+#define RALINK_RT2880_PCIE2	26
+#define RALINK_RT2880_AUX_STCK	28
+#define RALINK_RT2880_CRYPTO	29
+#define RALINK_RT2880_SDXC	30
+#define RALINK_RT2880_PPE	31
+
+#endif /* DT_BINDING_RALINK_RT2880_RESET_H */
-- 
2.25.1

Re: [PATCH 1/3] dt-bindings: reset: add dt binding header for ralink RT2880 resets

[Rob Herring]

On Sun, Sep 26, 2021 at 04:59:29PM +0200, Sergio Paracuellos wrote:
> Adds dt binding header for 'ralink,rt2880-reset' resets.
> 
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> ---
>  include/dt-bindings/reset/ralink-rt2880.h | 40 +++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>  create mode 100644 include/dt-bindings/reset/ralink-rt2880.h
> 
> diff --git a/include/dt-bindings/reset/ralink-rt2880.h b/include/dt-bindings/reset/ralink-rt2880.h
> new file mode 100644
> index 000000000000..266ef521a584
> --- /dev/null
> +++ b/include/dt-bindings/reset/ralink-rt2880.h
> @@ -0,0 +1,40 @@
> +/* SPDX-License-Identifier: GPL-2.0 */

Dual license please.

Re: [PATCH 1/3] dt-bindings: reset: add dt binding header for ralink RT2880 resets

[Sergio Paracuellos]

On Mon, Oct 4, 2021 at 8:02 PM Rob Herring <robh@kernel.org> wrote:
>
> On Sun, Sep 26, 2021 at 04:59:29PM +0200, Sergio Paracuellos wrote:
> > Adds dt binding header for 'ralink,rt2880-reset' resets.
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > ---
> >  include/dt-bindings/reset/ralink-rt2880.h | 40 +++++++++++++++++++++++
> >  1 file changed, 40 insertions(+)
> >  create mode 100644 include/dt-bindings/reset/ralink-rt2880.h
> >
> > diff --git a/include/dt-bindings/reset/ralink-rt2880.h b/include/dt-bindings/reset/ralink-rt2880.h
> > new file mode 100644
> > index 000000000000..266ef521a584
> > --- /dev/null
> > +++ b/include/dt-bindings/reset/ralink-rt2880.h
> > @@ -0,0 +1,40 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
>
> Dual license please.

Ah, ok. I thought the dual license was only for binding yaml files but
not for headers since there are a lot of already mainlined files with
only GPL-2.0. I will take into account from now that binding headers
also need dual license.

Thanks for the review and clarification.

Best regards,
    Sergio Paracuellos

Re: [PATCH 1/3] dt-bindings: reset: add dt binding header for ralink RT2880 resets

[Rob Herring]

On Mon, Oct 4, 2021 at 1:26 PM Sergio Paracuellos
<sergio.paracuellos@gmail.com> wrote:
>
> On Mon, Oct 4, 2021 at 8:02 PM Rob Herring <robh@kernel.org> wrote:
> >
> > On Sun, Sep 26, 2021 at 04:59:29PM +0200, Sergio Paracuellos wrote:
> > > Adds dt binding header for 'ralink,rt2880-reset' resets.
> > >
> > > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > > ---
> > >  include/dt-bindings/reset/ralink-rt2880.h | 40 +++++++++++++++++++++++
> > >  1 file changed, 40 insertions(+)
> > >  create mode 100644 include/dt-bindings/reset/ralink-rt2880.h
> > >
> > > diff --git a/include/dt-bindings/reset/ralink-rt2880.h b/include/dt-bindings/reset/ralink-rt2880.h
> > > new file mode 100644
> > > index 000000000000..266ef521a584
> > > --- /dev/null
> > > +++ b/include/dt-bindings/reset/ralink-rt2880.h
> > > @@ -0,0 +1,40 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> >
> > Dual license please.
>
> Ah, ok. I thought the dual license was only for binding yaml files but
> not for headers since there are a lot of already mainlined files with
> only GPL-2.0. I will take into account from now that binding headers
> also need dual license.

If you want your dts files used by other projects such as *BSD, then
they and their include files need to be licensed for that.

Rob

Re: [PATCH 1/3] dt-bindings: reset: add dt binding header for ralink RT2880 resets

[Sergio Paracuellos]

On Tue, Oct 5, 2021 at 3:29 PM Rob Herring <robh@kernel.org> wrote:
>
> On Mon, Oct 4, 2021 at 1:26 PM Sergio Paracuellos
> <sergio.paracuellos@gmail.com> wrote:
> >
> > On Mon, Oct 4, 2021 at 8:02 PM Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Sun, Sep 26, 2021 at 04:59:29PM +0200, Sergio Paracuellos wrote:
> > > > Adds dt binding header for 'ralink,rt2880-reset' resets.
> > > >
> > > > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > > > ---
> > > >  include/dt-bindings/reset/ralink-rt2880.h | 40 +++++++++++++++++++++++
> > > >  1 file changed, 40 insertions(+)
> > > >  create mode 100644 include/dt-bindings/reset/ralink-rt2880.h
> > > >
> > > > diff --git a/include/dt-bindings/reset/ralink-rt2880.h b/include/dt-bindings/reset/ralink-rt2880.h
> > > > new file mode 100644
> > > > index 000000000000..266ef521a584
> > > > --- /dev/null
> > > > +++ b/include/dt-bindings/reset/ralink-rt2880.h
> > > > @@ -0,0 +1,40 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0 */
> > >
> > > Dual license please.
> >
> > Ah, ok. I thought the dual license was only for binding yaml files but
> > not for headers since there are a lot of already mainlined files with
> > only GPL-2.0. I will take into account from now that binding headers
> > also need dual license.
>
> If you want your dts files used by other projects such as *BSD, then
> they and their include files need to be licensed for that.

Ok, it is clear now. Thanks for clarification.

Best regards,
    Sergio Paracuellos
>
> Rob

[PATCH 2/3] dt: bindings: add ralink RT2880 resets device tree binding documentation

[Sergio Paracuellos]

Adds device tree binding documentation for resets in the ralink RT2880 SoCs.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 .../bindings/reset/ralink,rt2880-reset.yaml   | 39 +++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/ralink,rt2880-reset.yaml

diff --git a/Documentation/devicetree/bindings/reset/ralink,rt2880-reset.yaml b/Documentation/devicetree/bindings/reset/ralink,rt2880-reset.yaml
new file mode 100644
index 000000000000..88eddeb4ee45
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/ralink,rt2880-reset.yaml
@@ -0,0 +1,39 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reset/ralink,rt2880-reset.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Ralink RT2880 Reset Controller Device Tree Bindings
+
+maintainers:
+  - Sergio Paracuellos <sergio.paracuellos@gmail.com>
+
+description: |
+  Ralink RT2880 reset controller driver which supports the SoC
+  system controller supplied reset registers for the various peripherals
+  of the SoC.
+
+  See also:
+  - dt-bindings/reset/ralink-rt2880.h
+
+properties:
+  compatible:
+    const: ralink,rt2880-reset
+
+  '#reset-cells':
+    const: 1
+
+required:
+  - '#reset-cells'
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/reset/ralink-rt2880.h>
+    rstctrl: reset-controller {
+      compatible = "ralink,rt2880-reset";
+      #reset-cells = <1>;
+    };
-- 
2.25.1

Re: [PATCH 2/3] dt: bindings: add ralink RT2880 resets device tree binding documentation

[Rob Herring]

On Sun, Sep 26, 2021 at 04:59:30PM +0200, Sergio Paracuellos wrote:
> Adds device tree binding documentation for resets in the ralink RT2880 SoCs.
> 
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> ---
>  .../bindings/reset/ralink,rt2880-reset.yaml   | 39 +++++++++++++++++++
>  1 file changed, 39 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/ralink,rt2880-reset.yaml
> 
> diff --git a/Documentation/devicetree/bindings/reset/ralink,rt2880-reset.yaml b/Documentation/devicetree/bindings/reset/ralink,rt2880-reset.yaml
> new file mode 100644
> index 000000000000..88eddeb4ee45
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/ralink,rt2880-reset.yaml
> @@ -0,0 +1,39 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/reset/ralink,rt2880-reset.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Ralink RT2880 Reset Controller Device Tree Bindings
> +
> +maintainers:
> +  - Sergio Paracuellos <sergio.paracuellos@gmail.com>
> +
> +description: |
> +  Ralink RT2880 reset controller driver which supports the SoC
> +  system controller supplied reset registers for the various peripherals
> +  of the SoC.
> +
> +  See also:
> +  - dt-bindings/reset/ralink-rt2880.h
> +
> +properties:
> +  compatible:
> +    const: ralink,rt2880-reset
> +
> +  '#reset-cells':
> +    const: 1
> +
> +required:
> +  - '#reset-cells'
> +  - compatible
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/reset/ralink-rt2880.h>
> +    rstctrl: reset-controller {
> +      compatible = "ralink,rt2880-reset";
> +      #reset-cells = <1>;

How is this h/w controlled? If this is part of a system controller, then 
it needs to be documented as such. IOW, you need to document the binding 
for the whole block.

Do you really need a child node here? All you need to make a system 
controller a reset provider is add '#reset-cells' to it.

Rob

Re: [PATCH 2/3] dt: bindings: add ralink RT2880 resets device tree binding documentation

[Sergio Paracuellos]

Hi Rob,

On Mon, Oct 4, 2021 at 8:06 PM Rob Herring <robh@kernel.org> wrote:
>
> On Sun, Sep 26, 2021 at 04:59:30PM +0200, Sergio Paracuellos wrote:
> > Adds device tree binding documentation for resets in the ralink RT2880 SoCs.
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > ---
> >  .../bindings/reset/ralink,rt2880-reset.yaml   | 39 +++++++++++++++++++
> >  1 file changed, 39 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/reset/ralink,rt2880-reset.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/reset/ralink,rt2880-reset.yaml b/Documentation/devicetree/bindings/reset/ralink,rt2880-reset.yaml
> > new file mode 100644
> > index 000000000000..88eddeb4ee45
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/reset/ralink,rt2880-reset.yaml
> > @@ -0,0 +1,39 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/reset/ralink,rt2880-reset.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Ralink RT2880 Reset Controller Device Tree Bindings
> > +
> > +maintainers:
> > +  - Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > +
> > +description: |
> > +  Ralink RT2880 reset controller driver which supports the SoC
> > +  system controller supplied reset registers for the various peripherals
> > +  of the SoC.
> > +
> > +  See also:
> > +  - dt-bindings/reset/ralink-rt2880.h
> > +
> > +properties:
> > +  compatible:
> > +    const: ralink,rt2880-reset
> > +
> > +  '#reset-cells':
> > +    const: 1
> > +
> > +required:
> > +  - '#reset-cells'
> > +  - compatible
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/reset/ralink-rt2880.h>
> > +    rstctrl: reset-controller {
> > +      compatible = "ralink,rt2880-reset";
> > +      #reset-cells = <1>;
>
> How is this h/w controlled? If this is part of a system controller, then
> it needs to be documented as such. IOW, you need to document the binding
> for the whole block.
>
> Do you really need a child node here? All you need to make a system
> controller a reset provider is add '#reset-cells' to it.

I am just documenting what is already mainlined (see [0]) in order to
get mt7621-dts out of staging at some point of my life. What am I
supposed to do? Should I rewrite all already mainlined code? Because
if that is the case we need to rewrite tons of things from the ralink
platform...

I'd also like to know what we should do with those nodes already added
to the dtsi file that have not got associated compatible driver
mainlined. Can we just get rid of them?

Thanks in advance for clarification.

Best regards,
    Sergio Paracuellos

[0]: https://elixir.bootlin.com/linux/latest/source/arch/mips/ralink/reset.c

>
> Rob

Re: [PATCH 2/3] dt: bindings: add ralink RT2880 resets device tree binding documentation

[Rob Herring]

On Mon, Oct 4, 2021 at 1:23 PM Sergio Paracuellos
<sergio.paracuellos@gmail.com> wrote:
>
> Hi Rob,
>
> On Mon, Oct 4, 2021 at 8:06 PM Rob Herring <robh@kernel.org> wrote:
> >
> > On Sun, Sep 26, 2021 at 04:59:30PM +0200, Sergio Paracuellos wrote:
> > > Adds device tree binding documentation for resets in the ralink RT2880 SoCs.
> > >
> > > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > > ---
> > >  .../bindings/reset/ralink,rt2880-reset.yaml   | 39 +++++++++++++++++++
> > >  1 file changed, 39 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/reset/ralink,rt2880-reset.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/reset/ralink,rt2880-reset.yaml b/Documentation/devicetree/bindings/reset/ralink,rt2880-reset.yaml
> > > new file mode 100644
> > > index 000000000000..88eddeb4ee45
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/reset/ralink,rt2880-reset.yaml
> > > @@ -0,0 +1,39 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/reset/ralink,rt2880-reset.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Ralink RT2880 Reset Controller Device Tree Bindings
> > > +
> > > +maintainers:
> > > +  - Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > > +
> > > +description: |
> > > +  Ralink RT2880 reset controller driver which supports the SoC
> > > +  system controller supplied reset registers for the various peripherals
> > > +  of the SoC.
> > > +
> > > +  See also:
> > > +  - dt-bindings/reset/ralink-rt2880.h
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: ralink,rt2880-reset
> > > +
> > > +  '#reset-cells':
> > > +    const: 1
> > > +
> > > +required:
> > > +  - '#reset-cells'
> > > +  - compatible
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    #include <dt-bindings/reset/ralink-rt2880.h>
> > > +    rstctrl: reset-controller {
> > > +      compatible = "ralink,rt2880-reset";
> > > +      #reset-cells = <1>;
> >
> > How is this h/w controlled? If this is part of a system controller, then
> > it needs to be documented as such. IOW, you need to document the binding
> > for the whole block.
> >
> > Do you really need a child node here? All you need to make a system
> > controller a reset provider is add '#reset-cells' to it.
>
> I am just documenting what is already mainlined (see [0]) in order to
> get mt7621-dts out of staging at some point of my life. What am I
> supposed to do? Should I rewrite all already mainlined code? Because
> if that is the case we need to rewrite tons of things from the ralink
> platform...

On the flip side, am I not supposed to review bindings because the dts
is already in staging? Code dependent on DT bindings shouldn't have
been mainlined without any documented binding.

Looks like the resets are part of "mediatek,mt7621-sysc" to answer my
question. Add a #reset-cell to that node (and binding) and then change
this line to "mediatek,mt7621-sysc":

        reset_dev.of_node = of_find_compatible_node(NULL, NULL,
                                                "ralink,rt2880-reset");

That's the minimal change, but really I would move the reset code to
the clock driver as that is what handles the sysc node.


> I'd also like to know what we should do with those nodes already added
> to the dtsi file that have not got associated compatible driver
> mainlined. Can we just get rid of them?

Yes. Typically dts files start with minimal support.

A dts file in staging is odd. We shouldn't be adding them there.

Rob

Re: [PATCH 2/3] dt: bindings: add ralink RT2880 resets device tree binding documentation

[Sergio Paracuellos]

Hi Rob,

On Tue, Oct 5, 2021 at 3:27 PM Rob Herring <robh@kernel.org> wrote:
>
> On Mon, Oct 4, 2021 at 1:23 PM Sergio Paracuellos
> <sergio.paracuellos@gmail.com> wrote:
> >
> > Hi Rob,
> >
> > On Mon, Oct 4, 2021 at 8:06 PM Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Sun, Sep 26, 2021 at 04:59:30PM +0200, Sergio Paracuellos wrote:
> > > > Adds device tree binding documentation for resets in the ralink RT2880 SoCs.
> > > >
> > > > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > > > ---
> > > >  .../bindings/reset/ralink,rt2880-reset.yaml   | 39 +++++++++++++++++++
> > > >  1 file changed, 39 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/reset/ralink,rt2880-reset.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/reset/ralink,rt2880-reset.yaml b/Documentation/devicetree/bindings/reset/ralink,rt2880-reset.yaml
> > > > new file mode 100644
> > > > index 000000000000..88eddeb4ee45
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/reset/ralink,rt2880-reset.yaml
> > > > @@ -0,0 +1,39 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/reset/ralink,rt2880-reset.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Ralink RT2880 Reset Controller Device Tree Bindings
> > > > +
> > > > +maintainers:
> > > > +  - Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > > > +
> > > > +description: |
> > > > +  Ralink RT2880 reset controller driver which supports the SoC
> > > > +  system controller supplied reset registers for the various peripherals
> > > > +  of the SoC.
> > > > +
> > > > +  See also:
> > > > +  - dt-bindings/reset/ralink-rt2880.h
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    const: ralink,rt2880-reset
> > > > +
> > > > +  '#reset-cells':
> > > > +    const: 1
> > > > +
> > > > +required:
> > > > +  - '#reset-cells'
> > > > +  - compatible
> > > > +
> > > > +additionalProperties: false
> > > > +
> > > > +examples:
> > > > +  - |
> > > > +    #include <dt-bindings/reset/ralink-rt2880.h>
> > > > +    rstctrl: reset-controller {
> > > > +      compatible = "ralink,rt2880-reset";
> > > > +      #reset-cells = <1>;
> > >
> > > How is this h/w controlled? If this is part of a system controller, then
> > > it needs to be documented as such. IOW, you need to document the binding
> > > for the whole block.
> > >
> > > Do you really need a child node here? All you need to make a system
> > > controller a reset provider is add '#reset-cells' to it.
> >
> > I am just documenting what is already mainlined (see [0]) in order to
> > get mt7621-dts out of staging at some point of my life. What am I
> > supposed to do? Should I rewrite all already mainlined code? Because
> > if that is the case we need to rewrite tons of things from the ralink
> > platform...
>
> On the flip side, am I not supposed to review bindings because the dts
> is already in staging? Code dependent on DT bindings shouldn't have
> been mainlined without any documented binding.

Thanks for reviewing this. I guess I should have sent a complete
patchset with all remaining bindings and the move for the complete
binding instead of sending single binding doc patches.

>
> Looks like the resets are part of "mediatek,mt7621-sysc" to answer my
> question. Add a #reset-cell to that node (and binding) and then change
> this line to "mediatek,mt7621-sysc":
>
>         reset_dev.of_node = of_find_compatible_node(NULL, NULL,
>                                                 "ralink,rt2880-reset");
>
> That's the minimal change, but really I would move the reset code to
> the clock driver as that is what handles the sysc node.

It is not that easy since the code in reset.c is shared for all ralink
platforms and the mediatek,mt7621-sysc node is only for mt7621. So I
guess I have to "duplicate" the reset code and put it in the clock
driver for mt7621 as you are pointing out here. I have to also review
how other drivers are using the reset, using reset apis or directly
through the syscon.

>
>
> > I'd also like to know what we should do with those nodes already added
> > to the dtsi file that have not got associated compatible driver
> > mainlined. Can we just get rid of them?
>
> Yes. Typically dts files start with minimal support.
>
> A dts file in staging is odd. We shouldn't be adding them there.

Thanks for clarification.

Best regards,
    Sergio Paracuellos

>
> Rob

[PATCH 3/3] staging: mt7621-dts: align resets with binding documentation

[Sergio Paracuellos]

Binding documentation for compatible 'ralink,rt2880-reset' is now available.
Align reset related bits with binding documentation along the dtsi file.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/mt7621-dts/mt7621.dtsi | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/mt7621-dts/mt7621.dtsi b/drivers/staging/mt7621-dts/mt7621.dtsi
index eeabe9c0f4fb..40c594fdad5f 100644
--- a/drivers/staging/mt7621-dts/mt7621.dtsi
+++ b/drivers/staging/mt7621-dts/mt7621.dtsi
@@ -1,6 +1,7 @@
 #include <dt-bindings/interrupt-controller/mips-gic.h>
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/clock/mt7621-clk.h>
+#include <dt-bindings/reset/ralink-rt2880.h>
 
 / {
 	#address-cells = <1>;
@@ -88,7 +89,7 @@ i2c: i2c@900 {
 
 			clocks = <&sysc MT7621_CLK_I2C>;
 			clock-names = "i2c";
-			resets = <&rstctrl 16>;
+			resets = <&rstctrl RALINK_RT2880_I2C>;
 			reset-names = "i2c";
 
 			#address-cells = <1>;
@@ -106,7 +107,7 @@ i2s: i2s@a00 {
 
 			clocks = <&sysc MT7621_CLK_I2S>;
 			clock-names = "i2s";
-			resets = <&rstctrl 17>;
+			resets = <&rstctrl RALINK_RT2880_I2S>;
 			reset-names = "i2s";
 
 			interrupt-parent = <&gic>;
@@ -161,7 +162,7 @@ spi0: spi@b00 {
 			clocks = <&sysc MT7621_CLK_SPI>;
 			clock-names = "spi";
 
-			resets = <&rstctrl 18>;
+			resets = <&rstctrl RALINK_RT2880_SPI>;
 			reset-names = "spi";
 
 			#address-cells = <1>;
@@ -177,7 +178,7 @@ gdma: gdma@2800 {
 
 			clocks = <&sysc MT7621_CLK_GDMA>;
 			clock-names = "gdma";
-			resets = <&rstctrl 14>;
+			resets = <&rstctrl RALINK_RT2880_GDMA>;
 			reset-names = "dma";
 
 			interrupt-parent = <&gic>;
@@ -196,7 +197,7 @@ hsdma: hsdma@7000 {
 
 			clocks = <&sysc MT7621_CLK_HSDMA>;
 			clock-names = "hsdma";
-			resets = <&rstctrl 5>;
+			resets = <&rstctrl RALINK_RT2880_HSDMA>;
 			reset-names = "hsdma";
 
 			interrupt-parent = <&gic>;
@@ -296,7 +297,7 @@ pinmux {
 		};
 	};
 
-	rstctrl: rstctrl {
+	rstctrl: reset-controller {
 		compatible = "ralink,rt2880-reset";
 		#reset-cells = <1>;
 	};
@@ -383,7 +384,7 @@ ethernet: ethernet@1e100000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
 
-		resets = <&rstctrl 6 &rstctrl 23>;
+		resets = <&rstctrl RALINK_RT2880_FE &rstctrl RALINK_RT2880_ETH>;
 		reset-names = "fe", "eth";
 
 		interrupt-parent = <&gic>;
@@ -428,7 +429,7 @@ switch0: switch0@0 {
 				#size-cells = <0>;
 				reg = <0>;
 				mediatek,mcm;
-				resets = <&rstctrl 2>;
+				resets = <&rstctrl RALINK_RT2880_MCM>;
 				reset-names = "mcm";
 				interrupt-controller;
 				#interrupt-cells = <1>;
@@ -514,7 +515,7 @@ pcie@0,0 {
 			#interrupt-cells = <1>;
 			interrupt-map-mask = <0 0 0 0>;
 			interrupt-map = <0 0 0 0 &gic GIC_SHARED 4 IRQ_TYPE_LEVEL_HIGH>;
-			resets = <&rstctrl 24>;
+			resets = <&rstctrl RALINK_RT2880_PCIE0>;
 			clocks = <&sysc MT7621_CLK_PCIE0>;
 			phys = <&pcie0_phy 1>;
 			phy-names = "pcie-phy0";
@@ -529,7 +530,7 @@ pcie@1,0 {
 			#interrupt-cells = <1>;
 			interrupt-map-mask = <0 0 0 0>;
 			interrupt-map = <0 0 0 0 &gic GIC_SHARED 24 IRQ_TYPE_LEVEL_HIGH>;
-			resets = <&rstctrl 25>;
+			resets = <&rstctrl RALINK_RT2880_PCIE1>;
 			clocks = <&sysc MT7621_CLK_PCIE1>;
 			phys = <&pcie0_phy 1>;
 			phy-names = "pcie-phy1";
@@ -544,7 +545,7 @@ pcie@2,0 {
 			#interrupt-cells = <1>;
 			interrupt-map-mask = <0 0 0 0>;
 			interrupt-map = <0 0 0 0 &gic GIC_SHARED 25 IRQ_TYPE_LEVEL_HIGH>;
-			resets = <&rstctrl 26>;
+			resets = <&rstctrl RALINK_RT2880_PCIE2>;
 			clocks = <&sysc MT7621_CLK_PCIE2>;
 			phys = <&pcie2_phy 0>;
 			phy-names = "pcie-phy2";
-- 
2.25.1