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
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(-)
@@ -0,0 +1,39 @@+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)+%YAML1.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
@@ -0,0 +1,39 @@+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)+%YAML1.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
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
@@ -0,0 +1,39 @@+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)+%YAML1.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
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
@@ -0,0 +1,39 @@+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)+%YAML1.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.
quoted
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
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