Re: [PATCH v9 2/6] dt: bindings: add mt7621-clk device tree binding documentation

Hi,
On Sat, Mar 6, 2021 at 10:54 AM Sergio Paracuellos
[off-list ref] wrote:

Hi again,

On Sat, Mar 6, 2021 at 8:12 AM Sergio Paracuellos
[off-list ref] wrote:

quoted

Hi Rob,

On Fri, Mar 5, 2021 at 11:47 PM Rob Herring [off-list ref] wrote:
[snip]

quoted

quoted

+
+  ralink,sysctl:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      phandle of syscon used to control system registers
+
+  ralink,memctl:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      phandle of syscon used to control memory registers

I assume one of these phandles are the main registers for the clocks?
Make this a child node and drop that phandle.

The 'ralink,sysctl' phandle is to read bootstrap register to be able
to derive xtal and a clk gate register for the peripherals.
The 'ralink,memctl' phandle is to read the cpu clock frequency from
the memory controller.

So there is not "main registers". I already put this as a child node
in v4 and I was told to get rid of child nodes. I need this as a
regmap to other DT node registers (sysctl, and memctl) to be able to
use the driver without specific architecture operations and properly
enable for COMPILE_TEST without dirty Makefile arch flags. Both sysctl
and memctl has no other child nodes, and I think that's why I was told
to avoid child nodes at the end. I explained here [0] current sysctl
and memctl in the mt7621 device tree and my view of the need for this
two syscons:

[0]: https://lkml.org/lkml/2021/1/2/9

So to avoid to send again "a previous version" on this patch, please
guide me in the correct thing to do. Stephen, Rob, I will be really
happy with your help :)

Since there are no other child nodes for this sysc, should merge clock
properties
with this node in the following way a valid approach:

 sysc: sysc@0 {
     compatible = "mediatek,mt7621-sysc", "syscon";
     reg = <0x0 0x100>;
     #clock-cells = <1>;
     ralink,memctl = <&memc>;
     clock-output-names = "xtal", "cpu", "bus",
                                        "50m", "125m", "150m",
                                        "250m", "270m";
};

Consumer clock:

node: node@0 {
  ...
  clocks = <&sysc MT7621_CLK_WHATEVER>;
 ...
};

I have been reviewing bindings review comments along the time and I
was already suggested to do this I am saying here (see [0]) but my
mind seems that filtered it for any reason I don't really understand.
Maybe I should sleep a bit more :).

I will send v10 with these changes that hopefully will be the correct ones.

Thanks and sorry for bothering you with already suggested things.

Best regards,
    Sergio Paracuellos

[0]: https://lkml.org/lkml/2020/12/31/206

If that is the case... and since 'sysc' is used as system control
registers for all the rest of the world, where should be the yaml file
with bindings placed?

Thanks in advance again for your help.

Best regards,
    Sergio Paracuellos

quoted

Best regards,
    Sergio Paracuellos

quoted

quoted

+
+  clock-output-names:
+    maxItems: 8
+
+required:
+  - compatible
+  - '#clock-cells'
+  - ralink,sysctl
+  - ralink,memctl
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/mt7621-clk.h>
+
+    pll {
+      compatible = "mediatek,mt7621-clk";
+      #clock-cells = <1>;
+      ralink,sysctl = <&sysc>;
+      ralink,memctl = <&memc>;
+      clock-output-names = "xtal", "cpu", "bus",
+                           "50m", "125m", "150m",
+                           "250m", "270m";
+    };
--
2.25.1