driverdev-devel.linuxdriverproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v11 0/6] MIPS: ralink: add CPU clock detection and clock driver for MT7621
@ 2021-03-09  5:22 Sergio Paracuellos
  2021-03-09  5:22 ` [PATCH v11 1/6] dt-bindings: clock: add dt binding header for mt7621 clocks Sergio Paracuellos
                   ` (6 more replies)
  0 siblings, 7 replies; 15+ messages in thread
From: Sergio Paracuellos @ 2021-03-09  5:22 UTC (permalink / raw)
  To: sboyd
  Cc: devel, devicetree, tsbogend, gregkh, linux-mips, linux-kernel,
	robh+dt, john, neil, linux-clk

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. Main registers for the clocks
are in the sysc control node so this node is merged with clock properties
and will also be the clock provider for the SoC.

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 v11:
 - Collect Rob's Reviewed-by in bindings documentation patch.
 - Fix MAINTAINERS patch using file 'mediatek,mt7621-sysc.yaml'
   for documentation bindings.

Changes in v10:
 - Merge clock properties into 'sysc' system control node making
   this node a clock provider.
 - Update driver to use 'mediatek,mt7621-sysc' as compatible string.
 - Update documentation bindings and its related filename to 
   'mediatek,mt7621-sysc.yaml'.
 - Make use of 'linux/bitfields.h' header to avoid some preprocesor
   shift definitions and just use bit masks decreasing a bit LOC.

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-sysc 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-sysc.yaml  |  68 +++
 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               | 528 ++++++++++++++++++
 drivers/staging/mt7621-dts/gbpc1.dts          |  11 -
 drivers/staging/mt7621-dts/mt7621.dtsi        |  82 ++-
 include/dt-bindings/clock/mt7621-clk.h        |  41 ++
 11 files changed, 702 insertions(+), 59 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mt7621-sysc.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

-- 
2.25.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v11 1/6] dt-bindings: clock: add dt binding header for mt7621 clocks
  2021-03-09  5:22 [PATCH v11 0/6] MIPS: ralink: add CPU clock detection and clock driver for MT7621 Sergio Paracuellos
@ 2021-03-09  5:22 ` Sergio Paracuellos
  2021-04-09 18:15   ` Stephen Boyd
  2021-03-09  5:22 ` [PATCH v11 2/6] dt: bindings: add mt7621-sysc device tree binding documentation Sergio Paracuellos
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Sergio Paracuellos @ 2021-03-09  5:22 UTC (permalink / raw)
  To: sboyd
  Cc: devel, devicetree, tsbogend, Rob Herring, gregkh, linux-mips,
	linux-kernel, robh+dt, john, neil, linux-clk

Adds dt binding header for 'mediatek,mt7621-clk' clocks.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 include/dt-bindings/clock/mt7621-clk.h | 41 ++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 include/dt-bindings/clock/mt7621-clk.h

diff --git a/include/dt-bindings/clock/mt7621-clk.h b/include/dt-bindings/clock/mt7621-clk.h
new file mode 100644
index 000000000000..1422badcf9de
--- /dev/null
+++ b/include/dt-bindings/clock/mt7621-clk.h
@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Author: Sergio Paracuellos <sergio.paracuellos@gmail.com>
+ */
+
+#ifndef _DT_BINDINGS_CLK_MT7621_H
+#define _DT_BINDINGS_CLK_MT7621_H
+
+#define MT7621_CLK_XTAL		0
+#define MT7621_CLK_CPU		1
+#define MT7621_CLK_BUS		2
+#define MT7621_CLK_50M		3
+#define MT7621_CLK_125M		4
+#define MT7621_CLK_150M		5
+#define MT7621_CLK_250M		6
+#define MT7621_CLK_270M		7
+
+#define MT7621_CLK_HSDMA	8
+#define MT7621_CLK_FE		9
+#define MT7621_CLK_SP_DIVTX	10
+#define MT7621_CLK_TIMER	11
+#define MT7621_CLK_PCM		12
+#define MT7621_CLK_PIO		13
+#define MT7621_CLK_GDMA		14
+#define MT7621_CLK_NAND		15
+#define MT7621_CLK_I2C		16
+#define MT7621_CLK_I2S		17
+#define MT7621_CLK_SPI		18
+#define MT7621_CLK_UART1	19
+#define MT7621_CLK_UART2	20
+#define MT7621_CLK_UART3	21
+#define MT7621_CLK_ETH		22
+#define MT7621_CLK_PCIE0	23
+#define MT7621_CLK_PCIE1	24
+#define MT7621_CLK_PCIE2	25
+#define MT7621_CLK_CRYPTO	26
+#define MT7621_CLK_SHXC		27
+
+#define MT7621_CLK_MAX		28
+
+#endif /* _DT_BINDINGS_CLK_MT7621_H */
-- 
2.25.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v11 2/6] dt: bindings: add mt7621-sysc device tree binding documentation
  2021-03-09  5:22 [PATCH v11 0/6] MIPS: ralink: add CPU clock detection and clock driver for MT7621 Sergio Paracuellos
  2021-03-09  5:22 ` [PATCH v11 1/6] dt-bindings: clock: add dt binding header for mt7621 clocks Sergio Paracuellos
@ 2021-03-09  5:22 ` Sergio Paracuellos
  2021-04-09 18:15   ` Stephen Boyd
  2021-03-09  5:22 ` [PATCH v11 3/6] clk: ralink: add clock driver for mt7621 SoC Sergio Paracuellos
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Sergio Paracuellos @ 2021-03-09  5:22 UTC (permalink / raw)
  To: sboyd
  Cc: devel, devicetree, tsbogend, Rob Herring, gregkh, linux-mips,
	linux-kernel, robh+dt, john, neil, linux-clk

Adds device tree binding documentation for clocks in the
MT7621 SOC.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 .../bindings/clock/mediatek,mt7621-sysc.yaml  | 68 +++++++++++++++++++
 1 file changed, 68 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mt7621-sysc.yaml

diff --git a/Documentation/devicetree/bindings/clock/mediatek,mt7621-sysc.yaml b/Documentation/devicetree/bindings/clock/mediatek,mt7621-sysc.yaml
new file mode 100644
index 000000000000..915f84efd763
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/mediatek,mt7621-sysc.yaml
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/mediatek,mt7621-sysc.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>.
+
+  The clocks are provided inside a system controller node.
+
+properties:
+  compatible:
+    items:
+      - const: mediatek,mt7621-sysc
+      - const: syscon
+
+  reg:
+    maxItems: 1
+
+  "#clock-cells":
+    description:
+      The first cell indicates the clock number, see [1] for available
+      clocks.
+    const: 1
+
+  ralink,memctl:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      phandle of syscon used to control memory registers
+
+  clock-output-names:
+    maxItems: 8
+
+required:
+  - compatible
+  - reg
+  - '#clock-cells'
+  - ralink,memctl
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/mt7621-clk.h>
+
+    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";
+    };
-- 
2.25.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v11 3/6] clk: ralink: add clock driver for mt7621 SoC
  2021-03-09  5:22 [PATCH v11 0/6] MIPS: ralink: add CPU clock detection and clock driver for MT7621 Sergio Paracuellos
  2021-03-09  5:22 ` [PATCH v11 1/6] dt-bindings: clock: add dt binding header for mt7621 clocks Sergio Paracuellos
  2021-03-09  5:22 ` [PATCH v11 2/6] dt: bindings: add mt7621-sysc device tree binding documentation Sergio Paracuellos
@ 2021-03-09  5:22 ` Sergio Paracuellos
  2021-04-09 18:14   ` Stephen Boyd
  2021-03-09  5:22 ` [PATCH v11 4/6] staging: mt7621-dts: make use of new 'mt7621-clk' Sergio Paracuellos
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Sergio Paracuellos @ 2021-03-09  5:22 UTC (permalink / raw)
  To: sboyd
  Cc: devel, devicetree, tsbogend, gregkh, linux-mips, linux-kernel,
	robh+dt, john, neil, linux-clk

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.
* SYSC_REG_CPLL_CLKCFG1 - a banch of gates to enable/disable
clocks for all or some ip cores.

Looking into driver code, and some openWRT patched there are
another frequencies 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"

With this information the clk driver will provide clock and gates
functionality from a a set of hardcoded clocks allowing to define
a nice device tree without fixed clocks.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/clk/Kconfig             |   1 +
 drivers/clk/Makefile            |   1 +
 drivers/clk/ralink/Kconfig      |  15 +
 drivers/clk/ralink/Makefile     |   2 +
 drivers/clk/ralink/clk-mt7621.c | 528 ++++++++++++++++++++++++++++++++
 5 files changed, 547 insertions(+)
 create mode 100644 drivers/clk/ralink/Kconfig
 create mode 100644 drivers/clk/ralink/Makefile
 create mode 100644 drivers/clk/ralink/clk-mt7621.c

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 7c5dc348c16f..70b23da997bf 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -382,6 +382,7 @@ source "drivers/clk/mediatek/Kconfig"
 source "drivers/clk/meson/Kconfig"
 source "drivers/clk/mvebu/Kconfig"
 source "drivers/clk/qcom/Kconfig"
+source "drivers/clk/ralink/Kconfig"
 source "drivers/clk/renesas/Kconfig"
 source "drivers/clk/rockchip/Kconfig"
 source "drivers/clk/samsung/Kconfig"
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 5325847469e9..1b35ad852721 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -98,6 +98,7 @@ obj-$(CONFIG_COMMON_CLK_NXP)		+= nxp/
 obj-$(CONFIG_MACH_PISTACHIO)		+= pistachio/
 obj-$(CONFIG_COMMON_CLK_PXA)		+= pxa/
 obj-$(CONFIG_COMMON_CLK_QCOM)		+= qcom/
+obj-y					+= ralink/
 obj-y					+= renesas/
 obj-$(CONFIG_ARCH_ROCKCHIP)		+= rockchip/
 obj-$(CONFIG_COMMON_CLK_SAMSUNG)	+= samsung/
diff --git a/drivers/clk/ralink/Kconfig b/drivers/clk/ralink/Kconfig
new file mode 100644
index 000000000000..3e3f5cb9ad88
--- /dev/null
+++ b/drivers/clk/ralink/Kconfig
@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# MediaTek Mt7621 Clock Driver
+#
+menu "Clock driver for Mediatek mt7621 SoC"
+	depends on SOC_MT7621 || COMPILE_TEST
+
+config CLK_MT7621
+	bool "Clock driver for MediaTek MT7621"
+	depends on SOC_MT7621 || COMPILE_TEST
+	default SOC_MT7621
+	select MFD_SYSCON
+	help
+	  This driver supports MediaTek MT7621 basic clocks.
+endmenu
diff --git a/drivers/clk/ralink/Makefile b/drivers/clk/ralink/Makefile
new file mode 100644
index 000000000000..cf6f9216379d
--- /dev/null
+++ b/drivers/clk/ralink/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_CLK_MT7621) += clk-mt7621.o
diff --git a/drivers/clk/ralink/clk-mt7621.c b/drivers/clk/ralink/clk-mt7621.c
new file mode 100644
index 000000000000..6aea5accd51c
--- /dev/null
+++ b/drivers/clk/ralink/clk-mt7621.c
@@ -0,0 +1,528 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Mediatek MT7621 Clock Driver
+ * Author: Sergio Paracuellos <sergio.paracuellos@gmail.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/clk-provider.h>
+#include <linux/clk.h>
+#include <linux/mfd/syscon.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <dt-bindings/clock/mt7621-clk.h>
+
+/* Configuration registers */
+#define SYSC_REG_SYSTEM_CONFIG0         0x10
+#define SYSC_REG_SYSTEM_CONFIG1         0x14
+#define SYSC_REG_CLKCFG0		0x2c
+#define SYSC_REG_CLKCFG1		0x30
+#define SYSC_REG_CUR_CLK_STS		0x44
+#define MEMC_REG_CPU_PLL		0x648
+
+#define XTAL_MODE_SEL_MASK		GENMASK(8, 6)
+#define CPU_CLK_SEL_MASK		GENMASK(31, 30)
+#define CUR_CPU_FDIV_MASK		GENMASK(12, 8)
+#define CUR_CPU_FFRAC_MASK		GENMASK(4, 0)
+#define CPU_PLL_PREDIV_MASK		GENMASK(13, 12)
+#define CPU_PLL_FBDIV_MASK		GENMASK(10, 4)
+
+struct mt7621_clk_priv {
+	struct regmap *sysc;
+	struct regmap *memc;
+};
+
+struct mt7621_clk {
+	struct clk_hw hw;
+	struct mt7621_clk_priv *priv;
+};
+
+struct mt7621_fixed_clk {
+	u8 idx;
+	const char *name;
+	const char *parent_name;
+	unsigned long rate;
+	struct clk_hw *hw;
+};
+
+struct mt7621_gate {
+	u8 idx;
+	const char *name;
+	const char *parent_name;
+	struct mt7621_clk_priv *priv;
+	u32 bit_idx;
+	struct clk_hw hw;
+};
+
+#define GATE(_id, _name, _pname, _shift)	\
+	{					\
+		.idx		= _id,		\
+		.name		= _name,	\
+		.parent_name	= _pname,	\
+		.bit_idx	= _shift	\
+	}
+
+static struct mt7621_gate mt7621_gates[] = {
+	GATE(MT7621_CLK_HSDMA, "hsdma", "150m", BIT(5)),
+	GATE(MT7621_CLK_FE, "fe", "250m", BIT(6)),
+	GATE(MT7621_CLK_SP_DIVTX, "sp_divtx", "270m", BIT(7)),
+	GATE(MT7621_CLK_TIMER, "timer", "50m", BIT(8)),
+	GATE(MT7621_CLK_PCM, "pcm", "270m", BIT(11)),
+	GATE(MT7621_CLK_PIO, "pio", "50m", BIT(13)),
+	GATE(MT7621_CLK_GDMA, "gdma", "bus", BIT(14)),
+	GATE(MT7621_CLK_NAND, "nand", "125m", BIT(15)),
+	GATE(MT7621_CLK_I2C, "i2c", "50m", BIT(16)),
+	GATE(MT7621_CLK_I2S, "i2s", "270m", BIT(17)),
+	GATE(MT7621_CLK_SPI, "spi", "bus", BIT(18)),
+	GATE(MT7621_CLK_UART1, "uart1", "50m", BIT(19)),
+	GATE(MT7621_CLK_UART2, "uart2", "50m", BIT(20)),
+	GATE(MT7621_CLK_UART3, "uart3", "50m", BIT(21)),
+	GATE(MT7621_CLK_ETH, "eth", "50m", BIT(23)),
+	GATE(MT7621_CLK_PCIE0, "pcie0", "125m", BIT(24)),
+	GATE(MT7621_CLK_PCIE1, "pcie1", "125m", BIT(25)),
+	GATE(MT7621_CLK_PCIE2, "pcie2", "125m", BIT(26)),
+	GATE(MT7621_CLK_CRYPTO, "crypto", "250m", BIT(29)),
+	GATE(MT7621_CLK_SHXC, "shxc", "50m", BIT(30))
+};
+
+static inline struct mt7621_gate *to_mt7621_gate(struct clk_hw *hw)
+{
+	return container_of(hw, struct mt7621_gate, hw);
+}
+
+static int mt7621_gate_enable(struct clk_hw *hw)
+{
+	struct mt7621_gate *clk_gate = to_mt7621_gate(hw);
+	struct regmap *sysc = clk_gate->priv->sysc;
+
+	return regmap_update_bits(sysc, SYSC_REG_CLKCFG1,
+				  clk_gate->bit_idx, clk_gate->bit_idx);
+}
+
+static void mt7621_gate_disable(struct clk_hw *hw)
+{
+	struct mt7621_gate *clk_gate = to_mt7621_gate(hw);
+	struct regmap *sysc = clk_gate->priv->sysc;
+
+	regmap_update_bits(sysc, SYSC_REG_CLKCFG1, clk_gate->bit_idx, 0);
+}
+
+static int mt7621_gate_is_enabled(struct clk_hw *hw)
+{
+	struct mt7621_gate *clk_gate = to_mt7621_gate(hw);
+	struct regmap *sysc = clk_gate->priv->sysc;
+	u32 val;
+
+	if (regmap_read(sysc, SYSC_REG_CLKCFG1, &val))
+		return 0;
+
+	return val & BIT(clk_gate->bit_idx);
+}
+
+static const struct clk_ops mt7621_gate_ops = {
+	.enable = mt7621_gate_enable,
+	.disable = mt7621_gate_disable,
+	.is_enabled = mt7621_gate_is_enabled,
+};
+
+static int mt7621_gate_ops_init(struct device *dev,
+				struct mt7621_gate *sclk)
+{
+	struct clk_init_data init = {
+		.flags = CLK_SET_RATE_PARENT,
+		.num_parents = 1,
+		.parent_names = &sclk->parent_name,
+		.ops = &mt7621_gate_ops,
+		.name = sclk->name,
+	};
+
+	sclk->hw.init = &init;
+	return devm_clk_hw_register(dev, &sclk->hw);
+}
+
+static int mt7621_register_gates(struct device *dev,
+				 struct clk_hw_onecell_data *clk_data,
+				 struct mt7621_clk_priv *priv)
+{
+	struct clk_hw **hws = clk_data->hws;
+	int ret, i;
+
+	for (i = 0; i < ARRAY_SIZE(mt7621_gates); i++) {
+		struct mt7621_gate *sclk = &mt7621_gates[i];
+
+		sclk->priv = priv;
+		ret = mt7621_gate_ops_init(dev, sclk);
+		if (ret) {
+			dev_err(dev, "Couldn't register clock %s\n", sclk->name);
+			goto err_clk_unreg;
+		}
+
+		hws[sclk->idx] = &sclk->hw;
+	}
+
+	return 0;
+
+err_clk_unreg:
+	while (--i >= 0) {
+		struct mt7621_gate *sclk = &mt7621_gates[i];
+
+		clk_hw_unregister(&sclk->hw);
+	}
+	return ret;
+}
+
+#define FIXED(_id, _name, _rate)		\
+	{					\
+		.idx		= _id,		\
+		.name		= _name,	\
+		.parent_name	= "xtal",	\
+		.rate		= _rate		\
+	}
+
+static struct mt7621_fixed_clk mt7621_fixed_clks[] = {
+	FIXED(MT7621_CLK_50M, "50m", 50000000),
+	FIXED(MT7621_CLK_125M, "125m", 125000000),
+	FIXED(MT7621_CLK_150M, "150m", 150000000),
+	FIXED(MT7621_CLK_250M, "250m", 250000000),
+	FIXED(MT7621_CLK_270M, "270m", 270000000),
+};
+
+static int mt7621_register_fixed_clocks(struct device *dev,
+					struct clk_hw_onecell_data *clk_data)
+{
+	struct clk_hw **hws = clk_data->hws;
+	int ret, i;
+
+	for (i = 0; i < ARRAY_SIZE(mt7621_fixed_clks); i++) {
+		struct mt7621_fixed_clk *sclk = &mt7621_fixed_clks[i];
+
+		sclk->hw = clk_hw_register_fixed_rate(dev, sclk->name,
+						      sclk->parent_name, 0,
+						      sclk->rate);
+		if (IS_ERR(sclk->hw)) {
+			dev_err(dev, "Couldn't register clock %s\n", sclk->name);
+			ret = PTR_ERR(sclk->hw);
+			goto err_clk_unreg;
+		}
+
+		hws[sclk->idx] = sclk->hw;
+	}
+
+	return 0;
+
+err_clk_unreg:
+	while (--i >= 0) {
+		struct mt7621_fixed_clk *sclk = &mt7621_fixed_clks[i];
+
+		clk_hw_unregister_fixed_rate(sclk->hw);
+	}
+	return ret;
+}
+
+static inline struct mt7621_clk *to_mt7621_clk(struct clk_hw *hw)
+{
+	return container_of(hw, struct mt7621_clk, hw);
+}
+
+static unsigned long mt7621_xtal_recalc_rate(struct clk_hw *hw,
+					     unsigned long parent_rate)
+{
+	struct mt7621_clk *clk = to_mt7621_clk(hw);
+	struct regmap *sysc = clk->priv->sysc;
+	u32 val;
+
+	regmap_read(sysc, SYSC_REG_SYSTEM_CONFIG0, &val);
+	val = FIELD_GET(XTAL_MODE_SEL_MASK, val);
+
+	if (val <= 2)
+		return 20000000;
+	if (val <= 5)
+		return 40000000;
+
+	return 25000000;
+}
+
+static unsigned long mt7621_cpu_recalc_rate(struct clk_hw *hw,
+					    unsigned long xtal_clk)
+{
+	static const u32 prediv_tbl[] = { 0, 1, 2, 2 };
+	struct mt7621_clk *clk = to_mt7621_clk(hw);
+	struct regmap *sysc = clk->priv->sysc;
+	struct regmap *memc = clk->priv->memc;
+	u32 clkcfg, clk_sel, curclk, ffiv, ffrac;
+	u32 pll, prediv, fbdiv;
+	unsigned long cpu_clk;
+
+	regmap_read(sysc, SYSC_REG_CLKCFG0, &clkcfg);
+	clk_sel = FIELD_GET(CPU_CLK_SEL_MASK, clkcfg);
+
+	regmap_read(sysc, SYSC_REG_CUR_CLK_STS, &curclk);
+	ffiv = FIELD_GET(CUR_CPU_FDIV_MASK, curclk);
+	ffrac = FIELD_GET(CUR_CPU_FFRAC_MASK, curclk);
+
+	switch (clk_sel) {
+	case 0:
+		cpu_clk = 500000000;
+		break;
+	case 1:
+		regmap_read(memc, MEMC_REG_CPU_PLL, &pll);
+		fbdiv = FIELD_GET(CPU_PLL_FBDIV_MASK, pll);
+		prediv = FIELD_GET(CPU_PLL_PREDIV_MASK, pll);
+		cpu_clk = ((fbdiv + 1) * xtal_clk) >> prediv_tbl[prediv];
+		break;
+	default:
+		cpu_clk = xtal_clk;
+	}
+
+	return cpu_clk / ffiv * ffrac;
+}
+
+static unsigned long mt7621_bus_recalc_rate(struct clk_hw *hw,
+					    unsigned long parent_rate)
+{
+	return parent_rate / 4;
+}
+
+#define CLK_BASE(_name, _parent, _recalc) {				\
+	.init = &(struct clk_init_data) {				\
+		.name = _name,						\
+		.ops = &(const struct clk_ops) {			\
+			.recalc_rate = _recalc,				\
+		},							\
+		.parent_data = &(const struct clk_parent_data) {	\
+			.name = _parent,				\
+			.fw_name = _parent				\
+		},							\
+		.num_parents = _parent ? 1 : 0				\
+	},								\
+}
+
+static struct mt7621_clk mt7621_clks_base[] = {
+	{ CLK_BASE("xtal", NULL, mt7621_xtal_recalc_rate) },
+	{ CLK_BASE("cpu", "xtal", mt7621_cpu_recalc_rate) },
+	{ CLK_BASE("bus", "cpu", mt7621_bus_recalc_rate) },
+};
+
+static struct clk_hw *mt7621_clk_early[MT7621_CLK_MAX];
+
+static int mt7621_register_early_clocks(struct device_node *np,
+					struct clk_hw_onecell_data *clk_data,
+					struct mt7621_clk_priv *priv)
+{
+	struct clk_hw **hws = clk_data->hws;
+	int ret, i, j;
+
+	for (i = 0; i < ARRAY_SIZE(mt7621_clks_base); i++) {
+		struct mt7621_clk *sclk = &mt7621_clks_base[i];
+
+		sclk->priv = priv;
+		ret = of_clk_hw_register(np, &sclk->hw);
+		if (ret) {
+			pr_err("Couldn't register top clock %i\n", i);
+			goto err_clk_unreg;
+		}
+
+		hws[i] = &sclk->hw;
+		mt7621_clk_early[i] = &sclk->hw;
+	}
+
+	for (j = i; j < MT7621_CLK_MAX; j++)
+		mt7621_clk_early[j] = ERR_PTR(-EPROBE_DEFER);
+
+	return 0;
+
+err_clk_unreg:
+	while (--i >= 0) {
+		struct mt7621_clk *sclk = &mt7621_clks_base[i];
+
+		clk_hw_unregister(&sclk->hw);
+	}
+	return ret;
+}
+
+static int mt7621_prepare_enable_clocks(struct clk_hw_onecell_data *clk_data)
+{
+	int ret, i;
+
+	for (i = 0; i < MT7621_CLK_MAX; i++) {
+		ret = clk_prepare_enable(clk_data->hws[i]->clk);
+		if (ret) {
+			pr_err("failed to enable clk: %d\n", ret);
+			goto err_clk_disable;
+		}
+	}
+
+	return 0;
+
+err_clk_disable:
+	while (--i >= 0)
+		clk_disable_unprepare(clk_data->hws[i]->clk);
+	return ret;
+}
+
+static void __init mt7621_clk_init(struct device_node *node)
+{
+	struct mt7621_clk_priv *priv;
+	struct clk_hw_onecell_data *clk_data;
+	int ret, i, count;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return;
+
+	priv->sysc = syscon_node_to_regmap(node);
+	if (IS_ERR(priv->sysc)) {
+		pr_err("Could not get sysc syscon regmap\n");
+		goto free_clk_priv;
+	}
+
+	priv->memc = syscon_regmap_lookup_by_phandle(node, "ralink,memctl");
+	if (IS_ERR(priv->memc)) {
+		pr_err("Could not get memc syscon regmap\n");
+		goto free_clk_priv;
+	}
+
+	count = ARRAY_SIZE(mt7621_clks_base) +
+		ARRAY_SIZE(mt7621_fixed_clks) + ARRAY_SIZE(mt7621_gates);
+	clk_data = kzalloc(struct_size(clk_data, hws, count), GFP_KERNEL);
+	if (!clk_data)
+		goto free_clk_priv;
+
+	ret = mt7621_register_early_clocks(node, clk_data, priv);
+	if (ret) {
+		pr_err("Couldn't register top clocks\n");
+		goto free_clk_data;
+	}
+
+	clk_data->num = count;
+
+	ret = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
+	if (ret) {
+		pr_err("Couldn't add clk hw provider\n");
+		goto unreg_clk_top;
+	}
+
+	return;
+
+unreg_clk_top:
+	for (i = 0; i < ARRAY_SIZE(mt7621_clks_base); i++) {
+		struct mt7621_clk *sclk = &mt7621_clks_base[i];
+
+		clk_hw_unregister(&sclk->hw);
+	}
+
+free_clk_data:
+	kfree(clk_data);
+
+free_clk_priv:
+	kfree(priv);
+}
+CLK_OF_DECLARE_DRIVER(mt7621_clk, "mediatek,mt7621-sysc", mt7621_clk_init);
+
+static int mt7621_clk_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct clk_hw_onecell_data *clk_data;
+	struct device *dev = &pdev->dev;
+	struct mt7621_clk_priv *priv;
+	int ret, i, count;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->sysc = syscon_node_to_regmap(np);
+	if (IS_ERR(priv->sysc)) {
+		ret = PTR_ERR(priv->sysc);
+		dev_err(dev, "Could not get sysc syscon regmap\n");
+		goto free_clk_priv;
+	}
+
+	priv->memc = syscon_regmap_lookup_by_phandle(np, "ralink,memctl");
+	if (IS_ERR(priv->memc)) {
+		ret = PTR_ERR(priv->memc);
+		dev_err(dev, "Could not get memc syscon regmap\n");
+		goto free_clk_priv;
+	}
+
+	count = ARRAY_SIZE(mt7621_clks_base) +
+		ARRAY_SIZE(mt7621_fixed_clks) + ARRAY_SIZE(mt7621_gates);
+	clk_data = kzalloc(struct_size(clk_data, hws, count), GFP_KERNEL);
+	if (!clk_data) {
+		ret = -ENOMEM;
+		goto free_clk_priv;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(mt7621_clks_base); i++)
+		clk_data->hws[i] = mt7621_clk_early[i];
+
+	ret = mt7621_register_fixed_clocks(dev, clk_data);
+	if (ret) {
+		dev_err(dev, "Couldn't register fixed clocks\n");
+		goto free_clk_data;
+	}
+
+	ret = mt7621_register_gates(dev, clk_data, priv);
+	if (ret) {
+		dev_err(dev, "Couldn't register fixed clock gates\n");
+		goto unreg_clk_fixed;
+	}
+
+	clk_data->num = count;
+
+	ret = mt7621_prepare_enable_clocks(clk_data);
+	if (ret) {
+		dev_err(dev, "Couldn't register fixed clock gates\n");
+		goto unreg_clk_gates;
+	}
+
+	ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, clk_data);
+	if (ret) {
+		dev_err(dev, "Couldn't add clk hw provider\n");
+		goto disable_clks;
+	}
+
+	return 0;
+
+disable_clks:
+	for (i = 0; i < MT7621_CLK_MAX; i++)
+		clk_disable_unprepare(clk_data->hws[i]->clk);
+
+unreg_clk_gates:
+	for (i = 0; i < ARRAY_SIZE(mt7621_gates); i++) {
+		struct mt7621_gate *sclk = &mt7621_gates[i];
+
+		clk_hw_unregister(&sclk->hw);
+	}
+
+unreg_clk_fixed:
+	for (i = 0; i < ARRAY_SIZE(mt7621_fixed_clks); i++) {
+		struct mt7621_fixed_clk *sclk = &mt7621_fixed_clks[i];
+
+		clk_hw_unregister_fixed_rate(sclk->hw);
+	}
+
+free_clk_data:
+	kfree(clk_data);
+
+free_clk_priv:
+	kfree(priv);
+
+	return ret;
+}
+
+static const struct of_device_id mt7621_clk_of_match[] = {
+	{ .compatible = "mediatek,mt7621-sysc" },
+	{},
+};
+
+static struct platform_driver mt7621_clk_driver = {
+	.probe = mt7621_clk_probe,
+	.driver = {
+		.name = "mt7621-clk",
+		.of_match_table = mt7621_clk_of_match,
+	},
+};
+builtin_platform_driver(mt7621_clk_driver);
-- 
2.25.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v11 4/6] staging: mt7621-dts: make use of new 'mt7621-clk'
  2021-03-09  5:22 [PATCH v11 0/6] MIPS: ralink: add CPU clock detection and clock driver for MT7621 Sergio Paracuellos
                   ` (2 preceding siblings ...)
  2021-03-09  5:22 ` [PATCH v11 3/6] clk: ralink: add clock driver for mt7621 SoC Sergio Paracuellos
@ 2021-03-09  5:22 ` Sergio Paracuellos
  2021-03-09  5:22 ` [PATCH v11 5/6] staging: mt7621-dts: use valid vendor 'mediatek' instead of invalid 'mtk' Sergio Paracuellos
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Sergio Paracuellos @ 2021-03-09  5:22 UTC (permalink / raw)
  To: sboyd
  Cc: devel, devicetree, tsbogend, gregkh, linux-mips, linux-kernel,
	robh+dt, john, neil, linux-clk

Clocks for SoC mt7621 have been properly integrated so there is
no need to declare fixed clocks at all in the device tree. Remove
all of them, add new device tree nodes for mt7621-clk and update
the rest of the nodes to use them.

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/mt7621-dts/gbpc1.dts   | 11 ----
 drivers/staging/mt7621-dts/mt7621.dtsi | 74 ++++++++++++--------------
 2 files changed, 33 insertions(+), 52 deletions(-)

diff --git a/drivers/staging/mt7621-dts/gbpc1.dts b/drivers/staging/mt7621-dts/gbpc1.dts
index a7c0d3115d72..7716d0efe524 100644
--- a/drivers/staging/mt7621-dts/gbpc1.dts
+++ b/drivers/staging/mt7621-dts/gbpc1.dts
@@ -100,17 +100,6 @@ partition@50000 {
 	};
 };
 
-&sysclock {
-			compatible = "fixed-clock";
-			/* This is normally 1/4 of cpuclock */
-			clock-frequency = <225000000>;
-};
-
-&cpuclock {
-			compatible = "fixed-clock";
-			clock-frequency = <900000000>;
-};
-
 &pcie {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pcie_pins>;
diff --git a/drivers/staging/mt7621-dts/mt7621.dtsi b/drivers/staging/mt7621-dts/mt7621.dtsi
index 16fc94f65486..b68183e7e6ad 100644
--- a/drivers/staging/mt7621-dts/mt7621.dtsi
+++ b/drivers/staging/mt7621-dts/mt7621.dtsi
@@ -1,5 +1,6 @@
 #include <dt-bindings/interrupt-controller/mips-gic.h>
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/clock/mt7621-clk.h>
 
 / {
 	#address-cells = <1>;
@@ -27,27 +28,6 @@ aliases {
 		serial0 = &uartlite;
 	};
 
-	cpuclock: cpuclock@0 {
-		#clock-cells = <0>;
-		compatible = "fixed-clock";
-
-		/* FIXME: there should be way to detect this */
-		clock-frequency = <880000000>;
-	};
-
-	sysclock: sysclock@0 {
-		#clock-cells = <0>;
-		compatible = "fixed-clock";
-
-		/* This is normally 1/4 of cpuclock */
-		clock-frequency = <220000000>;
-	};
-
-	mmc_clock: mmc_clock@0 {
-		#clock-cells = <0>;
-		compatible = "fixed-clock";
-		clock-frequency = <48000000>;
-	};
 
 	mmc_fixed_3v3: fixedregulator@0 {
 		compatible = "regulator-fixed";
@@ -76,8 +56,13 @@ palmbus: palmbus@1E000000 {
 		#size-cells = <1>;
 
 		sysc: sysc@0 {
-			compatible = "mtk,mt7621-sysc";
+			compatible = "mtk,mt7621-sysc", "syscon";
 			reg = <0x0 0x100>;
+			#clock-cells = <1>;
+			ralink,memctl = <&memc>;
+			clock-output-names = "xtal", "cpu", "bus",
+					     "50m", "125m", "150m",
+					     "250m", "270m";
 		};
 
 		wdt: wdt@100 {
@@ -101,8 +86,8 @@ i2c: i2c@900 {
 			compatible = "mediatek,mt7621-i2c";
 			reg = <0x900 0x100>;
 
-			clocks = <&sysclock>;
-
+			clocks = <&sysc MT7621_CLK_I2C>;
+			clock-names = "i2c";
 			resets = <&rstctrl 16>;
 			reset-names = "i2c";
 
@@ -119,8 +104,8 @@ i2s: i2s@a00 {
 			compatible = "mediatek,mt7621-i2s";
 			reg = <0xa00 0x100>;
 
-			clocks = <&sysclock>;
-
+			clocks = <&sysc MT7621_CLK_I2S>;
+			clock-names = "i2s";
 			resets = <&rstctrl 17>;
 			reset-names = "i2s";
 
@@ -138,7 +123,7 @@ i2s: i2s@a00 {
 		};
 
 		memc: memc@5000 {
-			compatible = "mtk,mt7621-memc";
+			compatible = "mtk,mt7621-memc", "syscon";
 			reg = <0x5000 0x1000>;
 		};
 
@@ -156,8 +141,8 @@ uartlite: uartlite@c00 {
 			compatible = "ns16550a";
 			reg = <0xc00 0x100>;
 
-			clocks = <&sysclock>;
-			clock-frequency = <50000000>;
+			clocks = <&sysc MT7621_CLK_UART1>;
+			clock-names = "uart1";
 
 			interrupt-parent = <&gic>;
 			interrupts = <GIC_SHARED 26 IRQ_TYPE_LEVEL_HIGH>;
@@ -173,7 +158,8 @@ spi0: spi@b00 {
 			compatible = "ralink,mt7621-spi";
 			reg = <0xb00 0x100>;
 
-			clocks = <&sysclock>;
+			clocks = <&sysc MT7621_CLK_SPI>;
+			clock-names = "spi";
 
 			resets = <&rstctrl 18>;
 			reset-names = "spi";
@@ -189,6 +175,8 @@ gdma: gdma@2800 {
 			compatible = "ralink,rt3883-gdma";
 			reg = <0x2800 0x800>;
 
+			clocks = <&sysc MT7621_CLK_GDMA>;
+			clock-names = "gdma";
 			resets = <&rstctrl 14>;
 			reset-names = "dma";
 
@@ -206,6 +194,8 @@ hsdma: hsdma@7000 {
 			compatible = "mediatek,mt7621-hsdma";
 			reg = <0x7000 0x1000>;
 
+			clocks = <&sysc MT7621_CLK_HSDMA>;
+			clock-names = "hsdma";
 			resets = <&rstctrl 5>;
 			reset-names = "hsdma";
 
@@ -311,11 +301,6 @@ rstctrl: rstctrl {
 		#reset-cells = <1>;
 	};
 
-	clkctrl: clkctrl {
-		compatible = "ralink,rt2880-clock";
-		#clock-cells = <1>;
-	};
-
 	sdhci: sdhci@1E130000 {
 		status = "disabled";
 
@@ -334,7 +319,8 @@ sdhci: sdhci@1E130000 {
 		pinctrl-0 = <&sdhci_pins>;
 		pinctrl-1 = <&sdhci_pins>;
 
-		clocks = <&mmc_clock &mmc_clock>;
+		clocks = <&sysc MT7621_CLK_SHXC>,
+			 <&sysc MT7621_CLK_50M>;
 		clock-names = "source", "hclk";
 
 		interrupt-parent = <&gic>;
@@ -349,7 +335,7 @@ xhci: xhci@1E1C0000 {
 		       0x1e1d0700 0x0100>;
 		reg-names = "mac", "ippc";
 
-		clocks = <&sysclock>;
+		clocks = <&sysc MT7621_CLK_XTAL>;
 		clock-names = "sys_ck";
 
 		interrupt-parent = <&gic>;
@@ -368,7 +354,7 @@ gic: interrupt-controller@1fbc0000 {
 		timer {
 			compatible = "mti,gic-timer";
 			interrupts = <GIC_LOCAL 1 IRQ_TYPE_NONE>;
-			clocks = <&cpuclock>;
+			clocks = <&sysc MT7621_CLK_CPU>;
 		};
 	};
 
@@ -381,6 +367,9 @@ nand: nand@1e003000 {
 			0x1e003800 0x800>;
 		#address-cells = <1>;
 		#size-cells = <1>;
+
+		clocks = <&sysc MT7621_CLK_NAND>;
+		clock-names = "nand";
 	};
 
 	ethsys: syscon@1e000000 {
@@ -394,8 +383,9 @@ ethernet: ethernet@1e100000 {
 		compatible = "mediatek,mt7621-eth";
 		reg = <0x1e100000 0x10000>;
 
-		clocks = <&sysclock>;
-		clock-names = "ethif";
+		clocks = <&sysc MT7621_CLK_FE>,
+			 <&sysc MT7621_CLK_ETH>;
+		clock-names = "fe", "ethif";
 
 		#address-cells = <1>;
 		#size-cells = <0>;
@@ -521,7 +511,9 @@ GIC_SHARED 24 IRQ_TYPE_LEVEL_HIGH
 
 		resets = <&rstctrl 24 &rstctrl 25 &rstctrl 26>;
 		reset-names = "pcie0", "pcie1", "pcie2";
-		clocks = <&clkctrl 24 &clkctrl 25 &clkctrl 26>;
+		clocks = <&sysc MT7621_CLK_PCIE0>,
+			 <&sysc MT7621_CLK_PCIE1>,
+			 <&sysc MT7621_CLK_PCIE2>;
 		clock-names = "pcie0", "pcie1", "pcie2";
 		phys = <&pcie0_phy 1>, <&pcie2_phy 0>;
 		phy-names = "pcie-phy0", "pcie-phy2";
-- 
2.25.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v11 5/6] staging: mt7621-dts: use valid vendor 'mediatek' instead of invalid 'mtk'
  2021-03-09  5:22 [PATCH v11 0/6] MIPS: ralink: add CPU clock detection and clock driver for MT7621 Sergio Paracuellos
                   ` (3 preceding siblings ...)
  2021-03-09  5:22 ` [PATCH v11 4/6] staging: mt7621-dts: make use of new 'mt7621-clk' Sergio Paracuellos
@ 2021-03-09  5:22 ` Sergio Paracuellos
  2021-03-09 15:07   ` Thomas Bogendoerfer
  2021-03-09  5:22 ` [PATCH v11 6/6] MAINTAINERS: add MT7621 CLOCK maintainer Sergio Paracuellos
  2021-03-23  8:13 ` [PATCH v11 0/6] MIPS: ralink: add CPU clock detection and clock driver for MT7621 Sergio Paracuellos
  6 siblings, 1 reply; 15+ messages in thread
From: Sergio Paracuellos @ 2021-03-09  5:22 UTC (permalink / raw)
  To: sboyd
  Cc: devel, devicetree, tsbogend, gregkh, linux-mips, linux-kernel,
	robh+dt, john, neil, linux-clk

Vendor listed for mediatek in kernel vendor file 'vendor-prefixes.yaml'
contains 'mediatek' as a valid vendor string. Some nodes in the device
tree are using an invalid vendor string vfor 'mtk' instead. Fix all of
them in dts file. Update also ralink mt7621 related code to properly
match new strings. Even there are used in the device tree there are
some strings that are not referred anywhere but have been also updated
with new vendor name. These are 'mtk,mt7621-wdt', 'mtk,mt7621-nand',
'mtk,mt7621-mc', and 'mtk,mt7621-cpc'.

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 arch/mips/ralink/mt7621.c              |  6 +++---
 drivers/staging/mt7621-dts/mt7621.dtsi | 12 ++++++------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/mips/ralink/mt7621.c b/arch/mips/ralink/mt7621.c
index ca0ac607b0f3..5d74fc1c96ac 100644
--- a/arch/mips/ralink/mt7621.c
+++ b/arch/mips/ralink/mt7621.c
@@ -112,8 +112,8 @@ phys_addr_t mips_cpc_default_phys_base(void)
 
 void __init ralink_of_remap(void)
 {
-	rt_sysc_membase = plat_of_remap_node("mtk,mt7621-sysc");
-	rt_memc_membase = plat_of_remap_node("mtk,mt7621-memc");
+	rt_sysc_membase = plat_of_remap_node("mediatek,mt7621-sysc");
+	rt_memc_membase = plat_of_remap_node("mediatek,mt7621-memc");
 
 	if (!rt_sysc_membase || !rt_memc_membase)
 		panic("Failed to remap core resources");
@@ -181,7 +181,7 @@ void prom_soc_init(struct ralink_soc_info *soc_info)
 
 	if (n0 == MT7621_CHIP_NAME0 && n1 == MT7621_CHIP_NAME1) {
 		name = "MT7621";
-		soc_info->compatible = "mtk,mt7621-soc";
+		soc_info->compatible = "mediatek,mt7621-soc";
 	} else {
 		panic("mt7621: unknown SoC, n0:%08x n1:%08x\n", n0, n1);
 	}
diff --git a/drivers/staging/mt7621-dts/mt7621.dtsi b/drivers/staging/mt7621-dts/mt7621.dtsi
index b68183e7e6ad..f0c9ae757bcd 100644
--- a/drivers/staging/mt7621-dts/mt7621.dtsi
+++ b/drivers/staging/mt7621-dts/mt7621.dtsi
@@ -56,7 +56,7 @@ palmbus: palmbus@1E000000 {
 		#size-cells = <1>;
 
 		sysc: sysc@0 {
-			compatible = "mtk,mt7621-sysc", "syscon";
+			compatible = "mediatek,mt7621-sysc", "syscon";
 			reg = <0x0 0x100>;
 			#clock-cells = <1>;
 			ralink,memctl = <&memc>;
@@ -66,7 +66,7 @@ sysc: sysc@0 {
 		};
 
 		wdt: wdt@100 {
-			compatible = "mtk,mt7621-wdt";
+			compatible = "mediatek,mt7621-wdt";
 			reg = <0x100 0x100>;
 		};
 
@@ -123,17 +123,17 @@ i2s: i2s@a00 {
 		};
 
 		memc: memc@5000 {
-			compatible = "mtk,mt7621-memc", "syscon";
+			compatible = "mediatek,mt7621-memc", "syscon";
 			reg = <0x5000 0x1000>;
 		};
 
 		cpc: cpc@1fbf0000 {
-			     compatible = "mtk,mt7621-cpc";
+			     compatible = "mediatek,mt7621-cpc";
 			     reg = <0x1fbf0000 0x8000>;
 		};
 
 		mc: mc@1fbf8000 {
-			    compatible = "mtk,mt7621-mc";
+			    compatible = "mediatek,mt7621-mc";
 			    reg = <0x1fbf8000 0x8000>;
 		};
 
@@ -361,7 +361,7 @@ timer {
 	nand: nand@1e003000 {
 		status = "disabled";
 
-		compatible = "mtk,mt7621-nand";
+		compatible = "mediatek,mt7621-nand";
 		bank-width = <2>;
 		reg = <0x1e003000 0x800
 			0x1e003800 0x800>;
-- 
2.25.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v11 6/6] MAINTAINERS: add MT7621 CLOCK maintainer
  2021-03-09  5:22 [PATCH v11 0/6] MIPS: ralink: add CPU clock detection and clock driver for MT7621 Sergio Paracuellos
                   ` (4 preceding siblings ...)
  2021-03-09  5:22 ` [PATCH v11 5/6] staging: mt7621-dts: use valid vendor 'mediatek' instead of invalid 'mtk' Sergio Paracuellos
@ 2021-03-09  5:22 ` Sergio Paracuellos
  2021-03-23  8:13 ` [PATCH v11 0/6] MIPS: ralink: add CPU clock detection and clock driver for MT7621 Sergio Paracuellos
  6 siblings, 0 replies; 15+ messages in thread
From: Sergio Paracuellos @ 2021-03-09  5:22 UTC (permalink / raw)
  To: sboyd
  Cc: devel, devicetree, tsbogend, gregkh, linux-mips, linux-kernel,
	robh+dt, john, neil, linux-clk

Adding myself as maintainer for mt7621 clock driver.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 809a68af5efd..ecad5d972122 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11288,6 +11288,12 @@ L:	linux-wireless@vger.kernel.org
 S:	Maintained
 F:	drivers/net/wireless/mediatek/mt7601u/
 
+MEDIATEK MT7621 CLOCK DRIVER
+M:	Sergio Paracuellos <sergio.paracuellos@gmail.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/clock/mediatek,mt7621-sysc.yaml
+F:	drivers/clk/ralink/clk-mt7621.c
+
 MEDIATEK MT7621/28/88 I2C DRIVER
 M:	Stefan Roese <sr@denx.de>
 L:	linux-i2c@vger.kernel.org
-- 
2.25.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v11 5/6] staging: mt7621-dts: use valid vendor 'mediatek' instead of invalid 'mtk'
  2021-03-09  5:22 ` [PATCH v11 5/6] staging: mt7621-dts: use valid vendor 'mediatek' instead of invalid 'mtk' Sergio Paracuellos
@ 2021-03-09 15:07   ` Thomas Bogendoerfer
  0 siblings, 0 replies; 15+ messages in thread
From: Thomas Bogendoerfer @ 2021-03-09 15:07 UTC (permalink / raw)
  To: Sergio Paracuellos
  Cc: devel, devicetree, sboyd, gregkh, linux-mips, linux-kernel,
	robh+dt, john, neil, linux-clk

On Tue, Mar 09, 2021 at 06:22:25AM +0100, Sergio Paracuellos wrote:
> Vendor listed for mediatek in kernel vendor file 'vendor-prefixes.yaml'
> contains 'mediatek' as a valid vendor string. Some nodes in the device
> tree are using an invalid vendor string vfor 'mtk' instead. Fix all of
> them in dts file. Update also ralink mt7621 related code to properly
> match new strings. Even there are used in the device tree there are
> some strings that are not referred anywhere but have been also updated
> with new vendor name. These are 'mtk,mt7621-wdt', 'mtk,mt7621-nand',
> 'mtk,mt7621-mc', and 'mtk,mt7621-cpc'.
> 
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> ---
>  arch/mips/ralink/mt7621.c              |  6 +++---

Acked-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v11 0/6] MIPS: ralink: add CPU clock detection and clock driver for MT7621
  2021-03-09  5:22 [PATCH v11 0/6] MIPS: ralink: add CPU clock detection and clock driver for MT7621 Sergio Paracuellos
                   ` (5 preceding siblings ...)
  2021-03-09  5:22 ` [PATCH v11 6/6] MAINTAINERS: add MT7621 CLOCK maintainer Sergio Paracuellos
@ 2021-03-23  8:13 ` Sergio Paracuellos
  2021-04-09 18:17   ` Stephen Boyd
  6 siblings, 1 reply; 15+ messages in thread
From: Sergio Paracuellos @ 2021-03-23  8:13 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: open list:STAGING SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Thomas Bogendoerfer, Greg KH, open list:MIPS, linux-kernel,
	Rob Herring, John Crispin, NeilBrown,
	open list:COMMON CLK FRAMEWORK

Hi Stephen,

On Tue, Mar 9, 2021 at 6:22 AM Sergio Paracuellos
<sergio.paracuellos@gmail.com> wrote:
>
> 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. Main registers for the clocks
> are in the sysc control node so this node is merged with clock properties
> and will also be the clock provider for the SoC.
>
> 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 v11:
>  - Collect Rob's Reviewed-by in bindings documentation patch.
>  - Fix MAINTAINERS patch using file 'mediatek,mt7621-sysc.yaml'
>    for documentation bindings.

Something still missing or something that is needed to be fixed to get
this series applied through your tree?

Thanks in advance for your time.

Best regards,
     Sergio Paracuellos

>
> Changes in v10:
>  - Merge clock properties into 'sysc' system control node making
>    this node a clock provider.
>  - Update driver to use 'mediatek,mt7621-sysc' as compatible string.
>  - Update documentation bindings and its related filename to
>    'mediatek,mt7621-sysc.yaml'.
>  - Make use of 'linux/bitfields.h' header to avoid some preprocesor
>    shift definitions and just use bit masks decreasing a bit LOC.
>
> 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-sysc 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-sysc.yaml  |  68 +++
>  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               | 528 ++++++++++++++++++
>  drivers/staging/mt7621-dts/gbpc1.dts          |  11 -
>  drivers/staging/mt7621-dts/mt7621.dtsi        |  82 ++-
>  include/dt-bindings/clock/mt7621-clk.h        |  41 ++
>  11 files changed, 702 insertions(+), 59 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mt7621-sysc.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
>
> --
> 2.25.1
>
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v11 3/6] clk: ralink: add clock driver for mt7621 SoC
  2021-03-09  5:22 ` [PATCH v11 3/6] clk: ralink: add clock driver for mt7621 SoC Sergio Paracuellos
@ 2021-04-09 18:14   ` Stephen Boyd
  2021-04-09 18:34     ` Sergio Paracuellos
  0 siblings, 1 reply; 15+ messages in thread
From: Stephen Boyd @ 2021-04-09 18:14 UTC (permalink / raw)
  To: Sergio Paracuellos
  Cc: devel, devicetree, tsbogend, gregkh, linux-mips, linux-kernel,
	robh+dt, john, neil, linux-clk

Quoting Sergio Paracuellos (2021-03-08 21:22:23)
> diff --git a/drivers/clk/ralink/Kconfig b/drivers/clk/ralink/Kconfig
> new file mode 100644
> index 000000000000..3e3f5cb9ad88
> --- /dev/null
> +++ b/drivers/clk/ralink/Kconfig
> @@ -0,0 +1,15 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# MediaTek Mt7621 Clock Driver
> +#
> +menu "Clock driver for Mediatek mt7621 SoC"
> +       depends on SOC_MT7621 || COMPILE_TEST

Do we need a menu and a config that says the same thing? Maybe the menu
can be dropped?

> +
> +config CLK_MT7621
> +       bool "Clock driver for MediaTek MT7621"
> +       depends on SOC_MT7621 || COMPILE_TEST
> +       default SOC_MT7621
> +       select MFD_SYSCON
> +       help
> +         This driver supports MediaTek MT7621 basic clocks.
> +endmenu
> diff --git a/drivers/clk/ralink/Makefile b/drivers/clk/ralink/Makefile
> new file mode 100644
> index 000000000000..cf6f9216379d
> --- /dev/null
> +++ b/drivers/clk/ralink/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0
> +obj-$(CONFIG_CLK_MT7621) += clk-mt7621.o
> diff --git a/drivers/clk/ralink/clk-mt7621.c b/drivers/clk/ralink/clk-mt7621.c
> new file mode 100644
> index 000000000000..6aea5accd51c
> --- /dev/null
> +++ b/drivers/clk/ralink/clk-mt7621.c
> @@ -0,0 +1,528 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Mediatek MT7621 Clock Driver
> + * Author: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bitops.h>
> +#include <linux/clk-provider.h>
> +#include <linux/clk.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <dt-bindings/clock/mt7621-clk.h>
> +
> +/* Configuration registers */
> +#define SYSC_REG_SYSTEM_CONFIG0         0x10
> +#define SYSC_REG_SYSTEM_CONFIG1         0x14
> +#define SYSC_REG_CLKCFG0               0x2c
> +#define SYSC_REG_CLKCFG1               0x30
> +#define SYSC_REG_CUR_CLK_STS           0x44
> +#define MEMC_REG_CPU_PLL               0x648
> +
> +#define XTAL_MODE_SEL_MASK             GENMASK(8, 6)
> +#define CPU_CLK_SEL_MASK               GENMASK(31, 30)
> +#define CUR_CPU_FDIV_MASK              GENMASK(12, 8)
> +#define CUR_CPU_FFRAC_MASK             GENMASK(4, 0)
> +#define CPU_PLL_PREDIV_MASK            GENMASK(13, 12)
> +#define CPU_PLL_FBDIV_MASK             GENMASK(10, 4)
> +
> +struct mt7621_clk_priv {
> +       struct regmap *sysc;
> +       struct regmap *memc;
> +};
> +
> +struct mt7621_clk {
> +       struct clk_hw hw;
> +       struct mt7621_clk_priv *priv;
> +};
> +
> +struct mt7621_fixed_clk {
> +       u8 idx;
> +       const char *name;
> +       const char *parent_name;
> +       unsigned long rate;
> +       struct clk_hw *hw;
> +};
> +
> +struct mt7621_gate {
> +       u8 idx;
> +       const char *name;
> +       const char *parent_name;
> +       struct mt7621_clk_priv *priv;
> +       u32 bit_idx;
> +       struct clk_hw hw;
> +};
> +
> +#define GATE(_id, _name, _pname, _shift)       \
> +       {                                       \
> +               .idx            = _id,          \
> +               .name           = _name,        \
> +               .parent_name    = _pname,       \
> +               .bit_idx        = _shift        \
> +       }
> +
> +static struct mt7621_gate mt7621_gates[] = {
> +       GATE(MT7621_CLK_HSDMA, "hsdma", "150m", BIT(5)),
> +       GATE(MT7621_CLK_FE, "fe", "250m", BIT(6)),
> +       GATE(MT7621_CLK_SP_DIVTX, "sp_divtx", "270m", BIT(7)),
> +       GATE(MT7621_CLK_TIMER, "timer", "50m", BIT(8)),
> +       GATE(MT7621_CLK_PCM, "pcm", "270m", BIT(11)),
> +       GATE(MT7621_CLK_PIO, "pio", "50m", BIT(13)),
> +       GATE(MT7621_CLK_GDMA, "gdma", "bus", BIT(14)),
> +       GATE(MT7621_CLK_NAND, "nand", "125m", BIT(15)),
> +       GATE(MT7621_CLK_I2C, "i2c", "50m", BIT(16)),
> +       GATE(MT7621_CLK_I2S, "i2s", "270m", BIT(17)),
> +       GATE(MT7621_CLK_SPI, "spi", "bus", BIT(18)),
> +       GATE(MT7621_CLK_UART1, "uart1", "50m", BIT(19)),
> +       GATE(MT7621_CLK_UART2, "uart2", "50m", BIT(20)),
> +       GATE(MT7621_CLK_UART3, "uart3", "50m", BIT(21)),
> +       GATE(MT7621_CLK_ETH, "eth", "50m", BIT(23)),
> +       GATE(MT7621_CLK_PCIE0, "pcie0", "125m", BIT(24)),
> +       GATE(MT7621_CLK_PCIE1, "pcie1", "125m", BIT(25)),
> +       GATE(MT7621_CLK_PCIE2, "pcie2", "125m", BIT(26)),
> +       GATE(MT7621_CLK_CRYPTO, "crypto", "250m", BIT(29)),
> +       GATE(MT7621_CLK_SHXC, "shxc", "50m", BIT(30))
> +};
> +
> +static inline struct mt7621_gate *to_mt7621_gate(struct clk_hw *hw)
> +{
> +       return container_of(hw, struct mt7621_gate, hw);
> +}
> +
> +static int mt7621_gate_enable(struct clk_hw *hw)
> +{
> +       struct mt7621_gate *clk_gate = to_mt7621_gate(hw);
> +       struct regmap *sysc = clk_gate->priv->sysc;
> +
> +       return regmap_update_bits(sysc, SYSC_REG_CLKCFG1,
> +                                 clk_gate->bit_idx, clk_gate->bit_idx);
> +}
> +
> +static void mt7621_gate_disable(struct clk_hw *hw)
> +{
> +       struct mt7621_gate *clk_gate = to_mt7621_gate(hw);
> +       struct regmap *sysc = clk_gate->priv->sysc;
> +
> +       regmap_update_bits(sysc, SYSC_REG_CLKCFG1, clk_gate->bit_idx, 0);
> +}
> +
> +static int mt7621_gate_is_enabled(struct clk_hw *hw)
> +{
> +       struct mt7621_gate *clk_gate = to_mt7621_gate(hw);
> +       struct regmap *sysc = clk_gate->priv->sysc;
> +       u32 val;
> +
> +       if (regmap_read(sysc, SYSC_REG_CLKCFG1, &val))
> +               return 0;
> +
> +       return val & BIT(clk_gate->bit_idx);
> +}
> +
> +static const struct clk_ops mt7621_gate_ops = {
> +       .enable = mt7621_gate_enable,
> +       .disable = mt7621_gate_disable,
> +       .is_enabled = mt7621_gate_is_enabled,
> +};
> +
> +static int mt7621_gate_ops_init(struct device *dev,
> +                               struct mt7621_gate *sclk)
> +{
> +       struct clk_init_data init = {
> +               .flags = CLK_SET_RATE_PARENT,
> +               .num_parents = 1,
> +               .parent_names = &sclk->parent_name,
> +               .ops = &mt7621_gate_ops,
> +               .name = sclk->name,
> +       };
> +
> +       sclk->hw.init = &init;
> +       return devm_clk_hw_register(dev, &sclk->hw);
> +}
> +
> +static int mt7621_register_gates(struct device *dev,
> +                                struct clk_hw_onecell_data *clk_data,
> +                                struct mt7621_clk_priv *priv)
> +{
> +       struct clk_hw **hws = clk_data->hws;
> +       int ret, i;
> +
> +       for (i = 0; i < ARRAY_SIZE(mt7621_gates); i++) {
> +               struct mt7621_gate *sclk = &mt7621_gates[i];
> +
> +               sclk->priv = priv;
> +               ret = mt7621_gate_ops_init(dev, sclk);
> +               if (ret) {
> +                       dev_err(dev, "Couldn't register clock %s\n", sclk->name);
> +                       goto err_clk_unreg;
> +               }
> +
> +               hws[sclk->idx] = &sclk->hw;
> +       }
> +
> +       return 0;
> +
> +err_clk_unreg:
> +       while (--i >= 0) {
> +               struct mt7621_gate *sclk = &mt7621_gates[i];
> +
> +               clk_hw_unregister(&sclk->hw);
> +       }
> +       return ret;
> +}
> +
> +#define FIXED(_id, _name, _rate)               \
> +       {                                       \
> +               .idx            = _id,          \
> +               .name           = _name,        \
> +               .parent_name    = "xtal",       \
> +               .rate           = _rate         \
> +       }
> +
> +static struct mt7621_fixed_clk mt7621_fixed_clks[] = {
> +       FIXED(MT7621_CLK_50M, "50m", 50000000),
> +       FIXED(MT7621_CLK_125M, "125m", 125000000),
> +       FIXED(MT7621_CLK_150M, "150m", 150000000),
> +       FIXED(MT7621_CLK_250M, "250m", 250000000),
> +       FIXED(MT7621_CLK_270M, "270m", 270000000),
> +};
> +
> +static int mt7621_register_fixed_clocks(struct device *dev,
> +                                       struct clk_hw_onecell_data *clk_data)
> +{
> +       struct clk_hw **hws = clk_data->hws;
> +       int ret, i;
> +
> +       for (i = 0; i < ARRAY_SIZE(mt7621_fixed_clks); i++) {
> +               struct mt7621_fixed_clk *sclk = &mt7621_fixed_clks[i];
> +
> +               sclk->hw = clk_hw_register_fixed_rate(dev, sclk->name,
> +                                                     sclk->parent_name, 0,
> +                                                     sclk->rate);
> +               if (IS_ERR(sclk->hw)) {
> +                       dev_err(dev, "Couldn't register clock %s\n", sclk->name);
> +                       ret = PTR_ERR(sclk->hw);
> +                       goto err_clk_unreg;
> +               }
> +
> +               hws[sclk->idx] = sclk->hw;
> +       }
> +
> +       return 0;
> +
> +err_clk_unreg:
> +       while (--i >= 0) {
> +               struct mt7621_fixed_clk *sclk = &mt7621_fixed_clks[i];
> +
> +               clk_hw_unregister_fixed_rate(sclk->hw);
> +       }
> +       return ret;
> +}
> +
> +static inline struct mt7621_clk *to_mt7621_clk(struct clk_hw *hw)
> +{
> +       return container_of(hw, struct mt7621_clk, hw);
> +}
> +
> +static unsigned long mt7621_xtal_recalc_rate(struct clk_hw *hw,
> +                                            unsigned long parent_rate)
> +{
> +       struct mt7621_clk *clk = to_mt7621_clk(hw);
> +       struct regmap *sysc = clk->priv->sysc;
> +       u32 val;
> +
> +       regmap_read(sysc, SYSC_REG_SYSTEM_CONFIG0, &val);
> +       val = FIELD_GET(XTAL_MODE_SEL_MASK, val);
> +
> +       if (val <= 2)
> +               return 20000000;
> +       if (val <= 5)
> +               return 40000000;
> +
> +       return 25000000;
> +}
> +
> +static unsigned long mt7621_cpu_recalc_rate(struct clk_hw *hw,
> +                                           unsigned long xtal_clk)
> +{
> +       static const u32 prediv_tbl[] = { 0, 1, 2, 2 };
> +       struct mt7621_clk *clk = to_mt7621_clk(hw);
> +       struct regmap *sysc = clk->priv->sysc;
> +       struct regmap *memc = clk->priv->memc;
> +       u32 clkcfg, clk_sel, curclk, ffiv, ffrac;
> +       u32 pll, prediv, fbdiv;
> +       unsigned long cpu_clk;
> +
> +       regmap_read(sysc, SYSC_REG_CLKCFG0, &clkcfg);
> +       clk_sel = FIELD_GET(CPU_CLK_SEL_MASK, clkcfg);
> +
> +       regmap_read(sysc, SYSC_REG_CUR_CLK_STS, &curclk);
> +       ffiv = FIELD_GET(CUR_CPU_FDIV_MASK, curclk);
> +       ffrac = FIELD_GET(CUR_CPU_FFRAC_MASK, curclk);
> +
> +       switch (clk_sel) {
> +       case 0:
> +               cpu_clk = 500000000;
> +               break;
> +       case 1:
> +               regmap_read(memc, MEMC_REG_CPU_PLL, &pll);
> +               fbdiv = FIELD_GET(CPU_PLL_FBDIV_MASK, pll);
> +               prediv = FIELD_GET(CPU_PLL_PREDIV_MASK, pll);
> +               cpu_clk = ((fbdiv + 1) * xtal_clk) >> prediv_tbl[prediv];
> +               break;
> +       default:
> +               cpu_clk = xtal_clk;
> +       }
> +
> +       return cpu_clk / ffiv * ffrac;
> +}
> +
> +static unsigned long mt7621_bus_recalc_rate(struct clk_hw *hw,
> +                                           unsigned long parent_rate)
> +{
> +       return parent_rate / 4;
> +}
> +
> +#define CLK_BASE(_name, _parent, _recalc) {                            \
> +       .init = &(struct clk_init_data) {                               \
> +               .name = _name,                                          \
> +               .ops = &(const struct clk_ops) {                        \
> +                       .recalc_rate = _recalc,                         \
> +               },                                                      \
> +               .parent_data = &(const struct clk_parent_data) {        \
> +                       .name = _parent,                                \
> +                       .fw_name = _parent                              \
> +               },                                                      \
> +               .num_parents = _parent ? 1 : 0                          \
> +       },                                                              \
> +}
> +
> +static struct mt7621_clk mt7621_clks_base[] = {
> +       { CLK_BASE("xtal", NULL, mt7621_xtal_recalc_rate) },
> +       { CLK_BASE("cpu", "xtal", mt7621_cpu_recalc_rate) },
> +       { CLK_BASE("bus", "cpu", mt7621_bus_recalc_rate) },
> +};
> +
> +static struct clk_hw *mt7621_clk_early[MT7621_CLK_MAX];
> +
> +static int mt7621_register_early_clocks(struct device_node *np,
> +                                       struct clk_hw_onecell_data *clk_data,
> +                                       struct mt7621_clk_priv *priv)
> +{
> +       struct clk_hw **hws = clk_data->hws;
> +       int ret, i, j;
> +
> +       for (i = 0; i < ARRAY_SIZE(mt7621_clks_base); i++) {
> +               struct mt7621_clk *sclk = &mt7621_clks_base[i];
> +
> +               sclk->priv = priv;
> +               ret = of_clk_hw_register(np, &sclk->hw);
> +               if (ret) {
> +                       pr_err("Couldn't register top clock %i\n", i);
> +                       goto err_clk_unreg;
> +               }
> +
> +               hws[i] = &sclk->hw;
> +               mt7621_clk_early[i] = &sclk->hw;
> +       }
> +
> +       for (j = i; j < MT7621_CLK_MAX; j++)
> +               mt7621_clk_early[j] = ERR_PTR(-EPROBE_DEFER);
> +
> +       return 0;
> +
> +err_clk_unreg:
> +       while (--i >= 0) {
> +               struct mt7621_clk *sclk = &mt7621_clks_base[i];

Please move sclk to the toplevel of this function instead of having it
twice.

> +
> +               clk_hw_unregister(&sclk->hw);
> +       }
> +       return ret;
> +}
> +
> +static int mt7621_prepare_enable_clocks(struct clk_hw_onecell_data *clk_data)
> +{
> +       int ret, i;
> +
> +       for (i = 0; i < MT7621_CLK_MAX; i++) {
> +               ret = clk_prepare_enable(clk_data->hws[i]->clk);

Are these critical clks? Why not use the CLK_IS_CRITICAL flag?

> +               if (ret) {
> +                       pr_err("failed to enable clk: %d\n", ret);
> +                       goto err_clk_disable;
> +               }
> +       }
> +
> +       return 0;
> +
> +err_clk_disable:
> +       while (--i >= 0)
> +               clk_disable_unprepare(clk_data->hws[i]->clk);
> +       return ret;
> +}
> +
> +static void __init mt7621_clk_init(struct device_node *node)
> +{
> +       struct mt7621_clk_priv *priv;
> +       struct clk_hw_onecell_data *clk_data;
> +       int ret, i, count;
> +
> +       priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> +       if (!priv)
> +               return;
> +
> +       priv->sysc = syscon_node_to_regmap(node);
> +       if (IS_ERR(priv->sysc)) {
> +               pr_err("Could not get sysc syscon regmap\n");
> +               goto free_clk_priv;
> +       }
> +
> +       priv->memc = syscon_regmap_lookup_by_phandle(node, "ralink,memctl");
> +       if (IS_ERR(priv->memc)) {
> +               pr_err("Could not get memc syscon regmap\n");
> +               goto free_clk_priv;
> +       }
> +
> +       count = ARRAY_SIZE(mt7621_clks_base) +
> +               ARRAY_SIZE(mt7621_fixed_clks) + ARRAY_SIZE(mt7621_gates);
> +       clk_data = kzalloc(struct_size(clk_data, hws, count), GFP_KERNEL);
> +       if (!clk_data)
> +               goto free_clk_priv;
> +
> +       ret = mt7621_register_early_clocks(node, clk_data, priv);
> +       if (ret) {
> +               pr_err("Couldn't register top clocks\n");
> +               goto free_clk_data;
> +       }
> +
> +       clk_data->num = count;
> +
> +       ret = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
> +       if (ret) {
> +               pr_err("Couldn't add clk hw provider\n");
> +               goto unreg_clk_top;
> +       }
> +
> +       return;
> +
> +unreg_clk_top:
> +       for (i = 0; i < ARRAY_SIZE(mt7621_clks_base); i++) {
> +               struct mt7621_clk *sclk = &mt7621_clks_base[i];
> +
> +               clk_hw_unregister(&sclk->hw);
> +       }
> +
> +free_clk_data:
> +       kfree(clk_data);
> +
> +free_clk_priv:
> +       kfree(priv);
> +}
> +CLK_OF_DECLARE_DRIVER(mt7621_clk, "mediatek,mt7621-sysc", mt7621_clk_init);
> +
> +static int mt7621_clk_probe(struct platform_device *pdev)
> +{
> +       struct device_node *np = pdev->dev.of_node;
> +       struct clk_hw_onecell_data *clk_data;
> +       struct device *dev = &pdev->dev;
> +       struct mt7621_clk_priv *priv;
> +       int ret, i, count;
> +
> +       priv = kzalloc(sizeof(*priv), GFP_KERNEL);

Can we use devm_* APIs here?

> +       if (!priv)
> +               return -ENOMEM;
> +
> +       priv->sysc = syscon_node_to_regmap(np);
> +       if (IS_ERR(priv->sysc)) {
> +               ret = PTR_ERR(priv->sysc);
> +               dev_err(dev, "Could not get sysc syscon regmap\n");
> +               goto free_clk_priv;
> +       }
> +
> +       priv->memc = syscon_regmap_lookup_by_phandle(np, "ralink,memctl");
> +       if (IS_ERR(priv->memc)) {
> +               ret = PTR_ERR(priv->memc);
> +               dev_err(dev, "Could not get memc syscon regmap\n");
> +               goto free_clk_priv;
> +       }
> +
> +       count = ARRAY_SIZE(mt7621_clks_base) +
> +               ARRAY_SIZE(mt7621_fixed_clks) + ARRAY_SIZE(mt7621_gates);
> +       clk_data = kzalloc(struct_size(clk_data, hws, count), GFP_KERNEL);
> +       if (!clk_data) {
> +               ret = -ENOMEM;
> +               goto free_clk_priv;
> +       }
> +
> +       for (i = 0; i < ARRAY_SIZE(mt7621_clks_base); i++)
> +               clk_data->hws[i] = mt7621_clk_early[i];
> +
> +       ret = mt7621_register_fixed_clocks(dev, clk_data);
> +       if (ret) {
> +               dev_err(dev, "Couldn't register fixed clocks\n");
> +               goto free_clk_data;
> +       }
> +
> +       ret = mt7621_register_gates(dev, clk_data, priv);
> +       if (ret) {
> +               dev_err(dev, "Couldn't register fixed clock gates\n");
> +               goto unreg_clk_fixed;
> +       }
> +
> +       clk_data->num = count;
> +
> +       ret = mt7621_prepare_enable_clocks(clk_data);
> +       if (ret) {
> +               dev_err(dev, "Couldn't register fixed clock gates\n");

This isn't registering fixed clock gates though?

> +               goto unreg_clk_gates;
> +       }
> +
> +       ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, clk_data);
> +       if (ret) {
> +               dev_err(dev, "Couldn't add clk hw provider\n");
> +               goto disable_clks;
> +       }
> +
> +       return 0;
> +
> +disable_clks:
> +       for (i = 0; i < MT7621_CLK_MAX; i++)
> +               clk_disable_unprepare(clk_data->hws[i]->clk);
> +
> +unreg_clk_gates:
> +       for (i = 0; i < ARRAY_SIZE(mt7621_gates); i++) {
> +               struct mt7621_gate *sclk = &mt7621_gates[i];
> +
> +               clk_hw_unregister(&sclk->hw);
> +       }
> +
> +unreg_clk_fixed:
> +       for (i = 0; i < ARRAY_SIZE(mt7621_fixed_clks); i++) {
> +               struct mt7621_fixed_clk *sclk = &mt7621_fixed_clks[i];
> +
> +               clk_hw_unregister_fixed_rate(sclk->hw);
> +       }
> +
> +free_clk_data:
> +       kfree(clk_data);
> +
> +free_clk_priv:
> +       kfree(priv);
> +
> +       return ret;
> +}
> +
> +static const struct of_device_id mt7621_clk_of_match[] = {
> +       { .compatible = "mediatek,mt7621-sysc" },
> +       {},

Nitpick: Drop the comma on the sentinel so it's a compile error if
another element is added after.

> +};
> +
> +static struct platform_driver mt7621_clk_driver = {
> +       .probe = mt7621_clk_probe,
> +       .driver = {
> +               .name = "mt7621-clk",
> +               .of_match_table = mt7621_clk_of_match,
> +       },
> +};
> +builtin_platform_driver(mt7621_clk_driver);
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v11 1/6] dt-bindings: clock: add dt binding header for mt7621 clocks
  2021-03-09  5:22 ` [PATCH v11 1/6] dt-bindings: clock: add dt binding header for mt7621 clocks Sergio Paracuellos
@ 2021-04-09 18:15   ` Stephen Boyd
  0 siblings, 0 replies; 15+ messages in thread
From: Stephen Boyd @ 2021-04-09 18:15 UTC (permalink / raw)
  To: Sergio Paracuellos
  Cc: devel, devicetree, tsbogend, Rob Herring, gregkh, linux-mips,
	linux-kernel, robh+dt, john, neil, linux-clk

Quoting Sergio Paracuellos (2021-03-08 21:22:21)
> Adds dt binding header for 'mediatek,mt7621-clk' clocks.
> 
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> ---

Applied to clk-next
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v11 2/6] dt: bindings: add mt7621-sysc device tree binding documentation
  2021-03-09  5:22 ` [PATCH v11 2/6] dt: bindings: add mt7621-sysc device tree binding documentation Sergio Paracuellos
@ 2021-04-09 18:15   ` Stephen Boyd
  0 siblings, 0 replies; 15+ messages in thread
From: Stephen Boyd @ 2021-04-09 18:15 UTC (permalink / raw)
  To: Sergio Paracuellos
  Cc: devel, devicetree, tsbogend, Rob Herring, gregkh, linux-mips,
	linux-kernel, robh+dt, john, neil, linux-clk

Quoting Sergio Paracuellos (2021-03-08 21:22:22)
> Adds device tree binding documentation for clocks in the
> MT7621 SOC.
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> ---

Applied to clk-next
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v11 0/6] MIPS: ralink: add CPU clock detection and clock driver for MT7621
  2021-03-23  8:13 ` [PATCH v11 0/6] MIPS: ralink: add CPU clock detection and clock driver for MT7621 Sergio Paracuellos
@ 2021-04-09 18:17   ` Stephen Boyd
  0 siblings, 0 replies; 15+ messages in thread
From: Stephen Boyd @ 2021-04-09 18:17 UTC (permalink / raw)
  To: Sergio Paracuellos
  Cc: STAGING SUBSYSTEM,
	OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Thomas Bogendoerfer, Greg KH, MIPS, linux-kernel, Rob Herring,
	John Crispin, NeilBrown, COMMON CLK FRAMEWORK

Quoting Sergio Paracuellos (2021-03-23 01:13:22)
> On Tue, Mar 9, 2021 at 6:22 AM Sergio Paracuellos
> <sergio.paracuellos@gmail.com> wrote:
> >
> > Changes in v11:
> >  - Collect Rob's Reviewed-by in bindings documentation patch.
> >  - Fix MAINTAINERS patch using file 'mediatek,mt7621-sysc.yaml'
> >    for documentation bindings.
> 
> Something still missing or something that is needed to be fixed to get
> this series applied through your tree?
> 
> Thanks in advance for your time.
> 

Sorry I missed this series. I thought it was going through another tree.
It can merge through clk tree. Just a few nits on the clk driver patch
but otherwise I've merged the first two patches. If you resend in the
next few days it would be great. Thanks.
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v11 3/6] clk: ralink: add clock driver for mt7621 SoC
  2021-04-09 18:14   ` Stephen Boyd
@ 2021-04-09 18:34     ` Sergio Paracuellos
  2021-04-09 18:38       ` Stephen Boyd
  0 siblings, 1 reply; 15+ messages in thread
From: Sergio Paracuellos @ 2021-04-09 18:34 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: open list:STAGING SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Thomas Bogendoerfer, Greg KH, open list:MIPS, linux-kernel,
	Rob Herring, John Crispin, NeilBrown,
	open list:COMMON CLK FRAMEWORK

Hi,

On Fri, Apr 9, 2021 at 8:14 PM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Sergio Paracuellos (2021-03-08 21:22:23)
> > diff --git a/drivers/clk/ralink/Kconfig b/drivers/clk/ralink/Kconfig
> > new file mode 100644
> > index 000000000000..3e3f5cb9ad88
> > --- /dev/null
> > +++ b/drivers/clk/ralink/Kconfig
> > @@ -0,0 +1,15 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +#
> > +# MediaTek Mt7621 Clock Driver
> > +#
> > +menu "Clock driver for Mediatek mt7621 SoC"
> > +       depends on SOC_MT7621 || COMPILE_TEST
>
> Do we need a menu and a config that says the same thing? Maybe the menu
> can be dropped?

Ok, I will drop the menu from this Kconfig.

>
> > +
> > +config CLK_MT7621
> > +       bool "Clock driver for MediaTek MT7621"
> > +       depends on SOC_MT7621 || COMPILE_TEST
> > +       default SOC_MT7621
> > +       select MFD_SYSCON
> > +       help
> > +         This driver supports MediaTek MT7621 basic clocks.
> > +endmenu
> > diff --git a/drivers/clk/ralink/Makefile b/drivers/clk/ralink/Makefile
> > new file mode 100644
> > index 000000000000..cf6f9216379d
> > --- /dev/null
> > +++ b/drivers/clk/ralink/Makefile
> > @@ -0,0 +1,2 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +obj-$(CONFIG_CLK_MT7621) += clk-mt7621.o
> > diff --git a/drivers/clk/ralink/clk-mt7621.c b/drivers/clk/ralink/clk-mt7621.c
> > new file mode 100644
> > index 000000000000..6aea5accd51c
> > --- /dev/null
> > +++ b/drivers/clk/ralink/clk-mt7621.c
> > @@ -0,0 +1,528 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Mediatek MT7621 Clock Driver
> > + * Author: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > + */
> > +
> > +#include <linux/bitfield.h>
> > +#include <linux/bitops.h>
> > +#include <linux/clk-provider.h>
> > +#include <linux/clk.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> > +#include <linux/slab.h>
> > +#include <dt-bindings/clock/mt7621-clk.h>
> > +
> > +/* Configuration registers */
> > +#define SYSC_REG_SYSTEM_CONFIG0         0x10
> > +#define SYSC_REG_SYSTEM_CONFIG1         0x14
> > +#define SYSC_REG_CLKCFG0               0x2c
> > +#define SYSC_REG_CLKCFG1               0x30
> > +#define SYSC_REG_CUR_CLK_STS           0x44
> > +#define MEMC_REG_CPU_PLL               0x648
> > +
> > +#define XTAL_MODE_SEL_MASK             GENMASK(8, 6)
> > +#define CPU_CLK_SEL_MASK               GENMASK(31, 30)
> > +#define CUR_CPU_FDIV_MASK              GENMASK(12, 8)
> > +#define CUR_CPU_FFRAC_MASK             GENMASK(4, 0)
> > +#define CPU_PLL_PREDIV_MASK            GENMASK(13, 12)
> > +#define CPU_PLL_FBDIV_MASK             GENMASK(10, 4)
> > +
> > +struct mt7621_clk_priv {
> > +       struct regmap *sysc;
> > +       struct regmap *memc;
> > +};
> > +
> > +struct mt7621_clk {
> > +       struct clk_hw hw;
> > +       struct mt7621_clk_priv *priv;
> > +};
> > +
> > +struct mt7621_fixed_clk {
> > +       u8 idx;
> > +       const char *name;
> > +       const char *parent_name;
> > +       unsigned long rate;
> > +       struct clk_hw *hw;
> > +};
> > +
> > +struct mt7621_gate {
> > +       u8 idx;
> > +       const char *name;
> > +       const char *parent_name;
> > +       struct mt7621_clk_priv *priv;
> > +       u32 bit_idx;
> > +       struct clk_hw hw;
> > +};
> > +
> > +#define GATE(_id, _name, _pname, _shift)       \
> > +       {                                       \
> > +               .idx            = _id,          \
> > +               .name           = _name,        \
> > +               .parent_name    = _pname,       \
> > +               .bit_idx        = _shift        \
> > +       }
> > +
> > +static struct mt7621_gate mt7621_gates[] = {
> > +       GATE(MT7621_CLK_HSDMA, "hsdma", "150m", BIT(5)),
> > +       GATE(MT7621_CLK_FE, "fe", "250m", BIT(6)),
> > +       GATE(MT7621_CLK_SP_DIVTX, "sp_divtx", "270m", BIT(7)),
> > +       GATE(MT7621_CLK_TIMER, "timer", "50m", BIT(8)),
> > +       GATE(MT7621_CLK_PCM, "pcm", "270m", BIT(11)),
> > +       GATE(MT7621_CLK_PIO, "pio", "50m", BIT(13)),
> > +       GATE(MT7621_CLK_GDMA, "gdma", "bus", BIT(14)),
> > +       GATE(MT7621_CLK_NAND, "nand", "125m", BIT(15)),
> > +       GATE(MT7621_CLK_I2C, "i2c", "50m", BIT(16)),
> > +       GATE(MT7621_CLK_I2S, "i2s", "270m", BIT(17)),
> > +       GATE(MT7621_CLK_SPI, "spi", "bus", BIT(18)),
> > +       GATE(MT7621_CLK_UART1, "uart1", "50m", BIT(19)),
> > +       GATE(MT7621_CLK_UART2, "uart2", "50m", BIT(20)),
> > +       GATE(MT7621_CLK_UART3, "uart3", "50m", BIT(21)),
> > +       GATE(MT7621_CLK_ETH, "eth", "50m", BIT(23)),
> > +       GATE(MT7621_CLK_PCIE0, "pcie0", "125m", BIT(24)),
> > +       GATE(MT7621_CLK_PCIE1, "pcie1", "125m", BIT(25)),
> > +       GATE(MT7621_CLK_PCIE2, "pcie2", "125m", BIT(26)),
> > +       GATE(MT7621_CLK_CRYPTO, "crypto", "250m", BIT(29)),
> > +       GATE(MT7621_CLK_SHXC, "shxc", "50m", BIT(30))
> > +};
> > +
> > +static inline struct mt7621_gate *to_mt7621_gate(struct clk_hw *hw)
> > +{
> > +       return container_of(hw, struct mt7621_gate, hw);
> > +}
> > +
> > +static int mt7621_gate_enable(struct clk_hw *hw)
> > +{
> > +       struct mt7621_gate *clk_gate = to_mt7621_gate(hw);
> > +       struct regmap *sysc = clk_gate->priv->sysc;
> > +
> > +       return regmap_update_bits(sysc, SYSC_REG_CLKCFG1,
> > +                                 clk_gate->bit_idx, clk_gate->bit_idx);
> > +}
> > +
> > +static void mt7621_gate_disable(struct clk_hw *hw)
> > +{
> > +       struct mt7621_gate *clk_gate = to_mt7621_gate(hw);
> > +       struct regmap *sysc = clk_gate->priv->sysc;
> > +
> > +       regmap_update_bits(sysc, SYSC_REG_CLKCFG1, clk_gate->bit_idx, 0);
> > +}
> > +
> > +static int mt7621_gate_is_enabled(struct clk_hw *hw)
> > +{
> > +       struct mt7621_gate *clk_gate = to_mt7621_gate(hw);
> > +       struct regmap *sysc = clk_gate->priv->sysc;
> > +       u32 val;
> > +
> > +       if (regmap_read(sysc, SYSC_REG_CLKCFG1, &val))
> > +               return 0;
> > +
> > +       return val & BIT(clk_gate->bit_idx);
> > +}
> > +
> > +static const struct clk_ops mt7621_gate_ops = {
> > +       .enable = mt7621_gate_enable,
> > +       .disable = mt7621_gate_disable,
> > +       .is_enabled = mt7621_gate_is_enabled,
> > +};
> > +
> > +static int mt7621_gate_ops_init(struct device *dev,
> > +                               struct mt7621_gate *sclk)
> > +{
> > +       struct clk_init_data init = {
> > +               .flags = CLK_SET_RATE_PARENT,
> > +               .num_parents = 1,
> > +               .parent_names = &sclk->parent_name,
> > +               .ops = &mt7621_gate_ops,
> > +               .name = sclk->name,
> > +       };
> > +
> > +       sclk->hw.init = &init;
> > +       return devm_clk_hw_register(dev, &sclk->hw);
> > +}
> > +
> > +static int mt7621_register_gates(struct device *dev,
> > +                                struct clk_hw_onecell_data *clk_data,
> > +                                struct mt7621_clk_priv *priv)
> > +{
> > +       struct clk_hw **hws = clk_data->hws;
> > +       int ret, i;
> > +
> > +       for (i = 0; i < ARRAY_SIZE(mt7621_gates); i++) {
> > +               struct mt7621_gate *sclk = &mt7621_gates[i];
> > +
> > +               sclk->priv = priv;
> > +               ret = mt7621_gate_ops_init(dev, sclk);
> > +               if (ret) {
> > +                       dev_err(dev, "Couldn't register clock %s\n", sclk->name);
> > +                       goto err_clk_unreg;
> > +               }
> > +
> > +               hws[sclk->idx] = &sclk->hw;
> > +       }
> > +
> > +       return 0;
> > +
> > +err_clk_unreg:
> > +       while (--i >= 0) {
> > +               struct mt7621_gate *sclk = &mt7621_gates[i];
> > +
> > +               clk_hw_unregister(&sclk->hw);
> > +       }
> > +       return ret;
> > +}
> > +
> > +#define FIXED(_id, _name, _rate)               \
> > +       {                                       \
> > +               .idx            = _id,          \
> > +               .name           = _name,        \
> > +               .parent_name    = "xtal",       \
> > +               .rate           = _rate         \
> > +       }
> > +
> > +static struct mt7621_fixed_clk mt7621_fixed_clks[] = {
> > +       FIXED(MT7621_CLK_50M, "50m", 50000000),
> > +       FIXED(MT7621_CLK_125M, "125m", 125000000),
> > +       FIXED(MT7621_CLK_150M, "150m", 150000000),
> > +       FIXED(MT7621_CLK_250M, "250m", 250000000),
> > +       FIXED(MT7621_CLK_270M, "270m", 270000000),
> > +};
> > +
> > +static int mt7621_register_fixed_clocks(struct device *dev,
> > +                                       struct clk_hw_onecell_data *clk_data)
> > +{
> > +       struct clk_hw **hws = clk_data->hws;
> > +       int ret, i;
> > +
> > +       for (i = 0; i < ARRAY_SIZE(mt7621_fixed_clks); i++) {
> > +               struct mt7621_fixed_clk *sclk = &mt7621_fixed_clks[i];
> > +
> > +               sclk->hw = clk_hw_register_fixed_rate(dev, sclk->name,
> > +                                                     sclk->parent_name, 0,
> > +                                                     sclk->rate);
> > +               if (IS_ERR(sclk->hw)) {
> > +                       dev_err(dev, "Couldn't register clock %s\n", sclk->name);
> > +                       ret = PTR_ERR(sclk->hw);
> > +                       goto err_clk_unreg;
> > +               }
> > +
> > +               hws[sclk->idx] = sclk->hw;
> > +       }
> > +
> > +       return 0;
> > +
> > +err_clk_unreg:
> > +       while (--i >= 0) {
> > +               struct mt7621_fixed_clk *sclk = &mt7621_fixed_clks[i];
> > +
> > +               clk_hw_unregister_fixed_rate(sclk->hw);
> > +       }
> > +       return ret;
> > +}
> > +
> > +static inline struct mt7621_clk *to_mt7621_clk(struct clk_hw *hw)
> > +{
> > +       return container_of(hw, struct mt7621_clk, hw);
> > +}
> > +
> > +static unsigned long mt7621_xtal_recalc_rate(struct clk_hw *hw,
> > +                                            unsigned long parent_rate)
> > +{
> > +       struct mt7621_clk *clk = to_mt7621_clk(hw);
> > +       struct regmap *sysc = clk->priv->sysc;
> > +       u32 val;
> > +
> > +       regmap_read(sysc, SYSC_REG_SYSTEM_CONFIG0, &val);
> > +       val = FIELD_GET(XTAL_MODE_SEL_MASK, val);
> > +
> > +       if (val <= 2)
> > +               return 20000000;
> > +       if (val <= 5)
> > +               return 40000000;
> > +
> > +       return 25000000;
> > +}
> > +
> > +static unsigned long mt7621_cpu_recalc_rate(struct clk_hw *hw,
> > +                                           unsigned long xtal_clk)
> > +{
> > +       static const u32 prediv_tbl[] = { 0, 1, 2, 2 };
> > +       struct mt7621_clk *clk = to_mt7621_clk(hw);
> > +       struct regmap *sysc = clk->priv->sysc;
> > +       struct regmap *memc = clk->priv->memc;
> > +       u32 clkcfg, clk_sel, curclk, ffiv, ffrac;
> > +       u32 pll, prediv, fbdiv;
> > +       unsigned long cpu_clk;
> > +
> > +       regmap_read(sysc, SYSC_REG_CLKCFG0, &clkcfg);
> > +       clk_sel = FIELD_GET(CPU_CLK_SEL_MASK, clkcfg);
> > +
> > +       regmap_read(sysc, SYSC_REG_CUR_CLK_STS, &curclk);
> > +       ffiv = FIELD_GET(CUR_CPU_FDIV_MASK, curclk);
> > +       ffrac = FIELD_GET(CUR_CPU_FFRAC_MASK, curclk);
> > +
> > +       switch (clk_sel) {
> > +       case 0:
> > +               cpu_clk = 500000000;
> > +               break;
> > +       case 1:
> > +               regmap_read(memc, MEMC_REG_CPU_PLL, &pll);
> > +               fbdiv = FIELD_GET(CPU_PLL_FBDIV_MASK, pll);
> > +               prediv = FIELD_GET(CPU_PLL_PREDIV_MASK, pll);
> > +               cpu_clk = ((fbdiv + 1) * xtal_clk) >> prediv_tbl[prediv];
> > +               break;
> > +       default:
> > +               cpu_clk = xtal_clk;
> > +       }
> > +
> > +       return cpu_clk / ffiv * ffrac;
> > +}
> > +
> > +static unsigned long mt7621_bus_recalc_rate(struct clk_hw *hw,
> > +                                           unsigned long parent_rate)
> > +{
> > +       return parent_rate / 4;
> > +}
> > +
> > +#define CLK_BASE(_name, _parent, _recalc) {                            \
> > +       .init = &(struct clk_init_data) {                               \
> > +               .name = _name,                                          \
> > +               .ops = &(const struct clk_ops) {                        \
> > +                       .recalc_rate = _recalc,                         \
> > +               },                                                      \
> > +               .parent_data = &(const struct clk_parent_data) {        \
> > +                       .name = _parent,                                \
> > +                       .fw_name = _parent                              \
> > +               },                                                      \
> > +               .num_parents = _parent ? 1 : 0                          \
> > +       },                                                              \
> > +}
> > +
> > +static struct mt7621_clk mt7621_clks_base[] = {
> > +       { CLK_BASE("xtal", NULL, mt7621_xtal_recalc_rate) },
> > +       { CLK_BASE("cpu", "xtal", mt7621_cpu_recalc_rate) },
> > +       { CLK_BASE("bus", "cpu", mt7621_bus_recalc_rate) },
> > +};
> > +
> > +static struct clk_hw *mt7621_clk_early[MT7621_CLK_MAX];
> > +
> > +static int mt7621_register_early_clocks(struct device_node *np,
> > +                                       struct clk_hw_onecell_data *clk_data,
> > +                                       struct mt7621_clk_priv *priv)
> > +{
> > +       struct clk_hw **hws = clk_data->hws;
> > +       int ret, i, j;
> > +
> > +       for (i = 0; i < ARRAY_SIZE(mt7621_clks_base); i++) {
> > +               struct mt7621_clk *sclk = &mt7621_clks_base[i];
> > +
> > +               sclk->priv = priv;
> > +               ret = of_clk_hw_register(np, &sclk->hw);
> > +               if (ret) {
> > +                       pr_err("Couldn't register top clock %i\n", i);
> > +                       goto err_clk_unreg;
> > +               }
> > +
> > +               hws[i] = &sclk->hw;
> > +               mt7621_clk_early[i] = &sclk->hw;
> > +       }
> > +
> > +       for (j = i; j < MT7621_CLK_MAX; j++)
> > +               mt7621_clk_early[j] = ERR_PTR(-EPROBE_DEFER);
> > +
> > +       return 0;
> > +
> > +err_clk_unreg:
> > +       while (--i >= 0) {
> > +               struct mt7621_clk *sclk = &mt7621_clks_base[i];
>
> Please move sclk to the toplevel of this function instead of having it
> twice.

Ok, no problem at all. I'll move this to void to declare twice in
success and error paths.

>
> > +
> > +               clk_hw_unregister(&sclk->hw);
> > +       }
> > +       return ret;
> > +}
> > +
> > +static int mt7621_prepare_enable_clocks(struct clk_hw_onecell_data *clk_data)
> > +{
> > +       int ret, i;
> > +
> > +       for (i = 0; i < MT7621_CLK_MAX; i++) {
> > +               ret = clk_prepare_enable(clk_data->hws[i]->clk);
>
> Are these critical clks? Why not use the CLK_IS_CRITICAL flag?

Well, Not calling clk_prepare_enable here makes the kernel to disable
all of the stuff is not being requested. Since until now no clock
driver existed, some SoC drivers  were not prepared for the clock. So,
if you prefer to avoid disabling stuff using this flag I think I can
just remove this enable_clocks function and set the flags for all the
clocks when registering them.

> > +               if (ret) {
> > +                       pr_err("failed to enable clk: %d\n", ret);
> > +                       goto err_clk_disable;
> > +               }
> > +       }
> > +
> > +       return 0;
> > +
> > +err_clk_disable:
> > +       while (--i >= 0)
> > +               clk_disable_unprepare(clk_data->hws[i]->clk);
> > +       return ret;
> > +}
> > +
> > +static void __init mt7621_clk_init(struct device_node *node)
> > +{
> > +       struct mt7621_clk_priv *priv;
> > +       struct clk_hw_onecell_data *clk_data;
> > +       int ret, i, count;
> > +
> > +       priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> > +       if (!priv)
> > +               return;
> > +
> > +       priv->sysc = syscon_node_to_regmap(node);
> > +       if (IS_ERR(priv->sysc)) {
> > +               pr_err("Could not get sysc syscon regmap\n");
> > +               goto free_clk_priv;
> > +       }
> > +
> > +       priv->memc = syscon_regmap_lookup_by_phandle(node, "ralink,memctl");
> > +       if (IS_ERR(priv->memc)) {
> > +               pr_err("Could not get memc syscon regmap\n");
> > +               goto free_clk_priv;
> > +       }
> > +
> > +       count = ARRAY_SIZE(mt7621_clks_base) +
> > +               ARRAY_SIZE(mt7621_fixed_clks) + ARRAY_SIZE(mt7621_gates);
> > +       clk_data = kzalloc(struct_size(clk_data, hws, count), GFP_KERNEL);
> > +       if (!clk_data)
> > +               goto free_clk_priv;
> > +
> > +       ret = mt7621_register_early_clocks(node, clk_data, priv);
> > +       if (ret) {
> > +               pr_err("Couldn't register top clocks\n");
> > +               goto free_clk_data;
> > +       }
> > +
> > +       clk_data->num = count;
> > +
> > +       ret = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
> > +       if (ret) {
> > +               pr_err("Couldn't add clk hw provider\n");
> > +               goto unreg_clk_top;
> > +       }
> > +
> > +       return;
> > +
> > +unreg_clk_top:
> > +       for (i = 0; i < ARRAY_SIZE(mt7621_clks_base); i++) {
> > +               struct mt7621_clk *sclk = &mt7621_clks_base[i];
> > +
> > +               clk_hw_unregister(&sclk->hw);
> > +       }
> > +
> > +free_clk_data:
> > +       kfree(clk_data);
> > +
> > +free_clk_priv:
> > +       kfree(priv);
> > +}
> > +CLK_OF_DECLARE_DRIVER(mt7621_clk, "mediatek,mt7621-sysc", mt7621_clk_init);
> > +
> > +static int mt7621_clk_probe(struct platform_device *pdev)
> > +{
> > +       struct device_node *np = pdev->dev.of_node;
> > +       struct clk_hw_onecell_data *clk_data;
> > +       struct device *dev = &pdev->dev;
> > +       struct mt7621_clk_priv *priv;
> > +       int ret, i, count;
> > +
> > +       priv = kzalloc(sizeof(*priv), GFP_KERNEL);
>
> Can we use devm_* APIs here?

Ok, I will.

>
> > +       if (!priv)
> > +               return -ENOMEM;
> > +
> > +       priv->sysc = syscon_node_to_regmap(np);
> > +       if (IS_ERR(priv->sysc)) {
> > +               ret = PTR_ERR(priv->sysc);
> > +               dev_err(dev, "Could not get sysc syscon regmap\n");
> > +               goto free_clk_priv;
> > +       }
> > +
> > +       priv->memc = syscon_regmap_lookup_by_phandle(np, "ralink,memctl");
> > +       if (IS_ERR(priv->memc)) {
> > +               ret = PTR_ERR(priv->memc);
> > +               dev_err(dev, "Could not get memc syscon regmap\n");
> > +               goto free_clk_priv;
> > +       }
> > +
> > +       count = ARRAY_SIZE(mt7621_clks_base) +
> > +               ARRAY_SIZE(mt7621_fixed_clks) + ARRAY_SIZE(mt7621_gates);
> > +       clk_data = kzalloc(struct_size(clk_data, hws, count), GFP_KERNEL);
> > +       if (!clk_data) {
> > +               ret = -ENOMEM;
> > +               goto free_clk_priv;
> > +       }
> > +
> > +       for (i = 0; i < ARRAY_SIZE(mt7621_clks_base); i++)
> > +               clk_data->hws[i] = mt7621_clk_early[i];
> > +
> > +       ret = mt7621_register_fixed_clocks(dev, clk_data);
> > +       if (ret) {
> > +               dev_err(dev, "Couldn't register fixed clocks\n");
> > +               goto free_clk_data;
> > +       }
> > +
> > +       ret = mt7621_register_gates(dev, clk_data, priv);
> > +       if (ret) {
> > +               dev_err(dev, "Couldn't register fixed clock gates\n");
> > +               goto unreg_clk_fixed;
> > +       }
> > +
> > +       clk_data->num = count;
> > +
> > +       ret = mt7621_prepare_enable_clocks(clk_data);
> > +       if (ret) {
> > +               dev_err(dev, "Couldn't register fixed clock gates\n");
>
> This isn't registering fixed clock gates though?

Copy paste is a bad thing ;). Will fix this.

>
> > +               goto unreg_clk_gates;
> > +       }
> > +
> > +       ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, clk_data);
> > +       if (ret) {
> > +               dev_err(dev, "Couldn't add clk hw provider\n");
> > +               goto disable_clks;
> > +       }
> > +
> > +       return 0;
> > +
> > +disable_clks:
> > +       for (i = 0; i < MT7621_CLK_MAX; i++)
> > +               clk_disable_unprepare(clk_data->hws[i]->clk);
> > +
> > +unreg_clk_gates:
> > +       for (i = 0; i < ARRAY_SIZE(mt7621_gates); i++) {
> > +               struct mt7621_gate *sclk = &mt7621_gates[i];
> > +
> > +               clk_hw_unregister(&sclk->hw);
> > +       }
> > +
> > +unreg_clk_fixed:
> > +       for (i = 0; i < ARRAY_SIZE(mt7621_fixed_clks); i++) {
> > +               struct mt7621_fixed_clk *sclk = &mt7621_fixed_clks[i];
> > +
> > +               clk_hw_unregister_fixed_rate(sclk->hw);
> > +       }
> > +
> > +free_clk_data:
> > +       kfree(clk_data);
> > +
> > +free_clk_priv:
> > +       kfree(priv);
> > +
> > +       return ret;
> > +}
> > +
> > +static const struct of_device_id mt7621_clk_of_match[] = {
> > +       { .compatible = "mediatek,mt7621-sysc" },
> > +       {},
>
> Nitpick: Drop the comma on the sentinel so it's a compile error if
> another element is added after.

True, will drop in next version.

>
> > +};
> > +
> > +static struct platform_driver mt7621_clk_driver = {
> > +       .probe = mt7621_clk_probe,
> > +       .driver = {
> > +               .name = "mt7621-clk",
> > +               .of_match_table = mt7621_clk_of_match,
> > +       },
> > +};
> > +builtin_platform_driver(mt7621_clk_driver);

Thanks for the review. I will resend this fixed during this weekend.

Best regards,
    Sergio Paracuellos
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v11 3/6] clk: ralink: add clock driver for mt7621 SoC
  2021-04-09 18:34     ` Sergio Paracuellos
@ 2021-04-09 18:38       ` Stephen Boyd
  0 siblings, 0 replies; 15+ messages in thread
From: Stephen Boyd @ 2021-04-09 18:38 UTC (permalink / raw)
  To: Sergio Paracuellos
  Cc: STAGING SUBSYSTEM,
	OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Thomas Bogendoerfer, Greg KH, MIPS, linux-kernel, Rob Herring,
	John Crispin, NeilBrown, COMMON CLK FRAMEWORK

Quoting Sergio Paracuellos (2021-04-09 11:34:35)
> On Fri, Apr 9, 2021 at 8:14 PM Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > Quoting Sergio Paracuellos (2021-03-08 21:22:23)
> > > diff --git a/drivers/clk/ralink/Makefile b/drivers/clk/ralink/Makefile
> > > new file mode 100644
> > > index 000000000000..cf6f9216379d
> > > --- /dev/null
> > > +++ b/drivers/clk/ralink/Makefile
> > > @@ -0,0 +1,2 @@
[...]
> >
> > > +
> > > +               clk_hw_unregister(&sclk->hw);
> > > +       }
> > > +       return ret;
> > > +}
> > > +
> > > +static int mt7621_prepare_enable_clocks(struct clk_hw_onecell_data *clk_data)
> > > +{
> > > +       int ret, i;
> > > +
> > > +       for (i = 0; i < MT7621_CLK_MAX; i++) {
> > > +               ret = clk_prepare_enable(clk_data->hws[i]->clk);
> >
> > Are these critical clks? Why not use the CLK_IS_CRITICAL flag?
> 
> Well, Not calling clk_prepare_enable here makes the kernel to disable
> all of the stuff is not being requested. Since until now no clock
> driver existed, some SoC drivers  were not prepared for the clock. So,
> if you prefer to avoid disabling stuff using this flag I think I can
> just remove this enable_clocks function and set the flags for all the
> clocks when registering them.
> 

Ok. If they're critical clks please set the flag and add a comment
indicating why they need to be enabled all the time.
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2021-04-09 18:38 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-09  5:22 [PATCH v11 0/6] MIPS: ralink: add CPU clock detection and clock driver for MT7621 Sergio Paracuellos
2021-03-09  5:22 ` [PATCH v11 1/6] dt-bindings: clock: add dt binding header for mt7621 clocks Sergio Paracuellos
2021-04-09 18:15   ` Stephen Boyd
2021-03-09  5:22 ` [PATCH v11 2/6] dt: bindings: add mt7621-sysc device tree binding documentation Sergio Paracuellos
2021-04-09 18:15   ` Stephen Boyd
2021-03-09  5:22 ` [PATCH v11 3/6] clk: ralink: add clock driver for mt7621 SoC Sergio Paracuellos
2021-04-09 18:14   ` Stephen Boyd
2021-04-09 18:34     ` Sergio Paracuellos
2021-04-09 18:38       ` Stephen Boyd
2021-03-09  5:22 ` [PATCH v11 4/6] staging: mt7621-dts: make use of new 'mt7621-clk' Sergio Paracuellos
2021-03-09  5:22 ` [PATCH v11 5/6] staging: mt7621-dts: use valid vendor 'mediatek' instead of invalid 'mtk' Sergio Paracuellos
2021-03-09 15:07   ` Thomas Bogendoerfer
2021-03-09  5:22 ` [PATCH v11 6/6] MAINTAINERS: add MT7621 CLOCK maintainer Sergio Paracuellos
2021-03-23  8:13 ` [PATCH v11 0/6] MIPS: ralink: add CPU clock detection and clock driver for MT7621 Sergio Paracuellos
2021-04-09 18:17   ` Stephen Boyd

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).