mbox series

[v9,0/6] MIPS: ralink: add CPU clock detection and clock driver for MT7621

Message ID 20210218070709.11932-1-sergio.paracuellos@gmail.com
Headers show
Series MIPS: ralink: add CPU clock detection and clock driver for MT7621 | expand

Message

Sergio Paracuellos Feb. 18, 2021, 7:07 a.m. UTC
This patchset ports CPU clock detection for MT7621 from OpenWrt
and adds a complete clock plan for the mt7621 SOC.

The documentation for this SOC only talks about two registers
regarding to the clocks:
* SYSC_REG_CPLL_CLKCFG0 - provides some information about boostrapped
refclock. PLL and dividers used for CPU and some sort of BUS (AHB?).
* SYSC_REG_CPLL_CLKCFG1 - a banch of gates to enable/disable clocks for
all or some ip cores.

Registers needed for this driver to work are in two already mapped areas
in its platform's device tree. These are 'sysc' and 'memc' nodes. Most
of other drivers just make use of platform operations defined in
'asm/mach-ralink/ralink_regs.h' but this can be avoided declaring this
two nodes to be accesible through syscon. Since these are the only two
needed control interfaces for this clock driver that seems to be the
correct thing to do.

No documentation about a probably existent set of dividers for each ip
core is included in the datasheets. So we cannot make anything better,
AFAICT.

Looking into driver code, and some openWRT patched there are
another frequences which are used in some drivers (uart, sd...).
According to all of this information the clock plan for this
SoC is set as follows:
 - Main top clock "xtal" from where all the rest of the world is
   derived.
 - CPU clock "cpu" derived from "xtal" frequencies and a bunch of
   register reads and predividers.
 - BUS clock "bus" derived from "cpu" and with (cpu / 4) MHz.
 - Fixed clocks from "xtal":
    * "50m": 50 MHz.
    * "125m": 125 MHz.
    * "150m": 150 MHz.
    * "250m": 250 MHz.
    * "270m": 270 MHz.

We also have a buch of gate clocks with their parents:
 - "hsdma": "150m"
 - "fe": "250m"
 - "sp_divtx": "270m"
 - "timer": "50m"
 - "pcm": "270m"
 - "pio": "50m"
 - "gdma": "bus"
 - "nand": "125m"
 - "i2c": "50m"
 - "i2s": "270m"
 - "spi": "bus"
 - "uart1": "50m"
 - "uart2": "50m"
 - "uart3": "50m"
 - "eth": "50m"
 - "pcie0": "125m"
 - "pcie1": "125m"
 - "pcie2": "125m"
 - "crypto": "250m"
 - "shxc": "50m"

There was a previous attempt of doing this here[0] but the author
(Chuanhong Guo) did not wanted to make assumptions of a clock plan
for the platform that time. It seems that now he has a better idea of
how the clocks are dispossed for this SoC so he share code[1] where
some frequencies and clock parents for the gates are coded from a
real mediatek private clock plan.
                                                
I do really want this to be upstreamed so according to the comments
in previous attempt[0] from Oleksij Rempel and the frequencies in
code[1] I have tried to do this by myself.

All of this patches have been tested in a GNUBee PC1 resulting in a
working platform.

Changes in v9:
 - Set two missing ret values to its related PTR_ERR in function
   'mt7621_clk_probe' (also related with [3]).
 - Select MFC_SYSCON in Kconfig.

Changes in v8:
 - Fix kernel test robot complain about the use of 'ret' variable
   initialized: see [3]

Changes in v7:
 - Make use of CLK_OF_DECLARE_DRIVER instead of CLK_OF_DECLARE and
   register there only the top clocks that are needed in 'of_clk_init'.
   The rest of the clocks (fixed and gates) are now registered using
   a platform driver. Because we have avoid architecture dependent stuff
   now this has sense because we can enable this driver for COMPILE_TEST.
 - Convert fixed clocks and gates related function to receive a 'struct
   device' pointer instead of 'struct device_node' one.
 - Make use of dev_ APIS in stuff related with platform driver instead
   of use device_node related stuff. 
 - Add new static global 'mt7621_clk_early' to store pointers to clk_hw
   registered at 'of_clk_init' stage. Make use of this in platform device
   probe function to properly copy this into the new required 'clk_data'
   to provide a properly hierarchy clock structure.
 - Rename 'mt7621_register_top_clocks' function into a more accurate 
   name now which is 'mt7621_register_early_clocks'.
 - Enable driver for COMPILE_TEST.

Changes in v6:
 - Rewrite bindings to properly access the registers needed for the driver
   making use of syscon for two different areas: 'sysc' and 'memc'. With
   this changes architecture dependent include 'asm/mach-ralink/ralink_regs.h'
   is not needed anymore because we access this two syscons using a phandle
   through kernel's regmap APIs. Explanation of this two areas is in [2].
 - Add new 'mt7621_clk_priv' struct to store there pointers to regmap handlers
   to be able to use regmap operations from normal clock api functions. Add
   this pointer in 'mt7621_clk' and 'mt7621_clk_gate' before register its
   related clocks to make things work.
 - Add Greg's Acked-by in patches 4 and 5.
 - Rebase this series on the top of linux-next tag 'next-20210215'.

v5 RESEND notes:
 - I am resending this as I was told to do that.
 - Please, take into account Rob's comments to DT node patch and my
   reply with explanation about how are the current device tree nodes
   for this architecture being used in [2].

Changes in v5:
 - Avoid the use of syscon. All drivers of this platform are just using
   platform operations defined in 'asm/mach-ralink/ralink_regs.h'. We also
   need them for some PLL registers that are not in the sys control area.
   Hence, since we must use this dependency avoid to define clock driver
   as a child of the sysc node in the device tree and follow current
   platform code style.
 - Update bindings documentation to don't refer the syscon and make
   remove 'clock-output-names' property from required ones.
 - Use 'asm/mach-ralink/ralink_regs.h' platform read and write operations
   instead of regmap from the syscon node.
 - Remove 'mt7621_clk_provider' and directly declare 'clk_hw_onecell_data'
   pointer in 'mt7621_clk_init' and pass from there into different register
   functions. Remove pointers to 'mt7621_clk_provider' in the rest fo structs
   used in this driver.
 - Remove MHZ macro and just pass values directly in hertzs.
 - Avoid 'CLK_IGNORE_UNUSED' flag for gates and add a new function called
   'mt7621_prepare_enable_clocks' to prepare all of them to make clocks
   referenced and don't affect current driver code.
 - Remove COMPILE_TEST from Kconfig because of the use of especific arch
   stuff.
 - Fix commit message where a typo for "frequencies" word was present.
 - Make use of parent_clk_data in 'CLK_BASE' macro.
 - Remove MODULE_* macros from code since this is not a module.
 - Remove not needed includes.
 - Hardcode "xtal" as parent in FIXED macro.
 - Change 'else if' clause into 'if' clause since a return statement was
   being used in 'mt7621_xtal_recalc_rate'.

 NOTES:
   - Driver is still being declared using 'CLK_OF_DECLARE' for all the  
     clocks. I have explored the possibility to make some of them available
     afterwards using 'CLK_OF_DECLARE_DRIVER' for top clocks and the rest
     using a platform driver. The resulting code was uglier since we only want
     to use the same device tree node and the top clocks must be copied again
     for the new platform register stuff to properly have a good hierarchy.
     New globals needs to be introduced and in this particular case I don't
     really see the benefits of doing in this way. I am totally ok to have all
     the clocks registered at early stage since from other drivers perspective
     we only really need to enable gates. So, I prefer to have them in that
     way if it is not a real problem, of course.

Changes in v4:
 - Add Acked-by from Rob Herring for binding headers (PATCH 1/6).
 - Convert bindings to not use syscon phandle and declare clock as
   a child of the syscon node. Update device tree and binding doc
   accordly.
 - Make use of 'syscon_node_to_regmap' in driver code instead of
   get this using the phandle function.
 - Properly unregister clocks for the error path of the function
   'mt7621_clk_init'.
 - Include ARRAY_SIZE of fixed clocks in the 'count' to kzalloc
   of 'clk_data'.
 - Add new patch changing invalid vendor 'mtk' in favour of 'mediatek'
   which is the one listed in 'vendor-prefixes.yaml'. Update mt7621 code
   accordly. I have added this patch inside this series because clk
   binding is referring syscon node and the string for that node was
   with not listed vendor. Hence update and have all of this correct
   in the same series.

Changes in v3:
 - Fix compilation warnings reported by kernel test robot because of
   ignoring return values of 'of_clk_hw_register' in functions
   'mt7621_register_top_clocks' and 'mt7621_gate_ops_init'.
 - Fix dts file and binding documentation 'clock-output-names'.

Changes in v2:
 - Remove the following patches:
   * dt: bindings: add mt7621-pll device tree binding documentation.
   * MIPS: ralink: add clock device providing cpu/ahb/apb clock for mt7621.
 - Move all relevant clock code to 'drivers/clk/ralink/clk-mt7621.c' and
   unify there previous 'mt7621-pll' and 'mt7621-clk' into a unique driver
   and binding 'mt7621-clk'.
 - Driver is not a platform driver anymore and now make use of 'CLK_OF_DECLARE'
   because we need clocks available in 'plat_time_init' before setting up
   the timer for the GIC.
 - Use new fixed clocks as parents for different gates and deriving from 'xtal'
   using frequencies in[1].
 - Adapt dts file and bindings header and documentation for new changes.
 - Change MAINTAINERS file to only contains clk-mt7621.c code and
   mediatek,mt7621-clk.yaml file.

[0]: https://www.lkml.org/lkml/2019/7/23/1044
[1]: https://github.com/981213/linux/commit/2eca1f045e4c3db18c941135464c0d7422ad8133
[2]: https://lkml.org/lkml/2020/12/20/47
[3]: http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2021-February/150772.html

Sergio Paracuellos (6):
  dt-bindings: clock: add dt binding header for mt7621 clocks
  dt: bindings: add mt7621-clk device tree binding documentation
  clk: ralink: add clock driver for mt7621 SoC
  staging: mt7621-dts: make use of new 'mt7621-clk'
  staging: mt7621-dts: use valid vendor 'mediatek' instead of invalid
    'mtk'
  MAINTAINERS: add MT7621 CLOCK maintainer

 .../bindings/clock/mediatek,mt7621-clk.yaml   |  66 +++
 MAINTAINERS                                   |   6 +
 arch/mips/ralink/mt7621.c                     |   6 +-
 drivers/clk/Kconfig                           |   1 +
 drivers/clk/Makefile                          |   1 +
 drivers/clk/ralink/Kconfig                    |  15 +
 drivers/clk/ralink/Makefile                   |   2 +
 drivers/clk/ralink/clk-mt7621.c               | 536 ++++++++++++++++++
 drivers/staging/mt7621-dts/gbpc1.dts          |  11 -
 drivers/staging/mt7621-dts/mt7621.dtsi        |  87 ++-
 include/dt-bindings/clock/mt7621-clk.h        |  41 ++
 11 files changed, 713 insertions(+), 59 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
 create mode 100644 drivers/clk/ralink/Kconfig
 create mode 100644 drivers/clk/ralink/Makefile
 create mode 100644 drivers/clk/ralink/clk-mt7621.c
 create mode 100644 include/dt-bindings/clock/mt7621-clk.h

Comments

Rob Herring March 5, 2021, 10:47 p.m. UTC | #1
On Thu, Feb 18, 2021 at 08:07:05AM +0100, Sergio Paracuellos wrote:
> Adds device tree binding documentation for clocks in the

> MT7621 SOC.

> 

> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>

> ---

>  .../bindings/clock/mediatek,mt7621-clk.yaml   | 66 +++++++++++++++++++

>  1 file changed, 66 insertions(+)

>  create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml

> 

> diff --git a/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml b/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml

> new file mode 100644

> index 000000000000..842a0f2c9d40

> --- /dev/null

> +++ b/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml

> @@ -0,0 +1,66 @@

> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)

> +%YAML 1.2

> +---

> +$id: http://devicetree.org/schemas/clock/mediatek,mt7621-clk.yaml#

> +$schema: http://devicetree.org/meta-schemas/core.yaml#

> +

> +title: MT7621 Clock Device Tree Bindings

> +

> +maintainers:

> +  - Sergio Paracuellos <sergio.paracuellos@gmail.com>

> +

> +description: |

> +  The MT7621 has a PLL controller from where the cpu clock is provided

> +  as well as derived clocks for the bus and the peripherals. It also

> +  can gate SoC device clocks.

> +

> +  Each clock is assigned an identifier and client nodes use this identifier

> +  to specify the clock which they consume.

> +

> +  All these identifiers could be found in:

> +  [1]: <include/dt-bindings/clock/mt7621-clk.h>.

> +

> +properties:

> +  compatible:

> +    const: mediatek,mt7621-clk

> +

> +  "#clock-cells":

> +    description:

> +      The first cell indicates the clock number, see [1] for available

> +      clocks.

> +    const: 1

> +

> +  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.

> +

> +  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

>
Chuanhong Guo March 6, 2021, 1:52 a.m. UTC | #2
Hi Rob!

On Sat, Mar 6, 2021 at 6:48 AM Rob Herring <robh@kernel.org> wrote:
>

> On Thu, Feb 18, 2021 at 08:07:05AM +0100, Sergio Paracuellos wrote:

> > Adds device tree binding documentation for clocks in the

> > MT7621 SOC.

> >

> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>

> > ---

> >  .../bindings/clock/mediatek,mt7621-clk.yaml   | 66 +++++++++++++++++++

> >  1 file changed, 66 insertions(+)

> >  create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml

> >

> > diff --git a/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml b/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml

> > new file mode 100644

> > index 000000000000..842a0f2c9d40

> > --- /dev/null

> > +++ b/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml

> > @@ -0,0 +1,66 @@

> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)

> > +%YAML 1.2

> > +---

> > +$id: http://devicetree.org/schemas/clock/mediatek,mt7621-clk.yaml#

> > +$schema: http://devicetree.org/meta-schemas/core.yaml#

> > +

> > +title: MT7621 Clock Device Tree Bindings

> > +

> > +maintainers:

> > +  - Sergio Paracuellos <sergio.paracuellos@gmail.com>

> > +

> > +description: |

> > +  The MT7621 has a PLL controller from where the cpu clock is provided

> > +  as well as derived clocks for the bus and the peripherals. It also

> > +  can gate SoC device clocks.

> > +

> > +  Each clock is assigned an identifier and client nodes use this identifier

> > +  to specify the clock which they consume.

> > +

> > +  All these identifiers could be found in:

> > +  [1]: <include/dt-bindings/clock/mt7621-clk.h>.

> > +

> > +properties:

> > +  compatible:

> > +    const: mediatek,mt7621-clk

> > +

> > +  "#clock-cells":

> > +    description:

> > +      The first cell indicates the clock number, see [1] for available

> > +      clocks.

> > +    const: 1

> > +

> > +  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.


On MT7621, CPU clock can be chosen from 3 sources: crystal clock,
a fixed 500MHz clock or a clock created by the memory controller.
sysctl contains a bootstrap register to determine crystal clock, a
clock mux for choosing between the 3 sources for CPU clock, and
a clock gate register for various peripherals. The ralink,memctl
phandle here is to read the cpu clock frequency from the memory
controller.
The original implementation hides this hardware detail to avoid
splitting the driver into three just for the CPU clock.
Is this approach okay and we can put it under sysctl node,
or this driver needs to be further splitted?

-- 
Regards,
Chuanhong Guo
Sergio Paracuellos March 6, 2021, 7:12 a.m. UTC | #3
Hi Rob,

On Fri, Mar 5, 2021 at 11:47 PM Rob Herring <robh@kernel.org> wrote:
[snip]
> > +

> > +  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 :)

Best regards,
    Sergio Paracuellos
>

> > +

> > +  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

> >
Sergio Paracuellos March 6, 2021, 9:54 a.m. UTC | #4
Hi again,

On Sat, Mar 6, 2021 at 8:12 AM Sergio Paracuellos
<sergio.paracuellos@gmail.com> wrote:
>

> Hi Rob,

>

> On Fri, Mar 5, 2021 at 11:47 PM Rob Herring <robh@kernel.org> wrote:

> [snip]

> > > +

> > > +  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>;
 ...
};

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

>

> Best regards,

>     Sergio Paracuellos

> >

> > > +

> > > +  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

> > >
Sergio Paracuellos March 7, 2021, 6:27 a.m. UTC | #5
Hi,
On Sat, Mar 6, 2021 at 10:54 AM Sergio Paracuellos
<sergio.paracuellos@gmail.com> wrote:
>

> Hi again,

>

> On Sat, Mar 6, 2021 at 8:12 AM Sergio Paracuellos

> <sergio.paracuellos@gmail.com> wrote:

> >

> > Hi Rob,

> >

> > On Fri, Mar 5, 2021 at 11:47 PM Rob Herring <robh@kernel.org> wrote:

> > [snip]

> > > > +

> > > > +  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

>

> >

> > Best regards,

> >     Sergio Paracuellos

> > >

> > > > +

> > > > +  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

> > > >