* [PATCH v4 0/6] MIPS: ralink: add CPU clock detection and clock driver for MT7621
@ 2020-11-22 9:55 ` Sergio Paracuellos
0 siblings, 0 replies; 36+ messages in thread
From: Sergio Paracuellos @ 2020-11-22 9:55 UTC (permalink / raw)
To: mturquette
Cc: sboyd, robh+dt, john, tsbogend, gregkh, gch981213, hackpascal,
linux-clk, evicetree, linux-kernel, linux-mips, devel, neil
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.
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 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
Sergio Paracuellos (6):
dt-bindings: clock: add dt binding header for mt7621 clocks
dt: bindings: add mt7621-clk device tree binding documentation
clk: ralink: add clock driver for mt7621 SoC
staging: mt7621-dts: make use of new 'mt7621-clk'
staging: mt7621-dts: use valid vendor 'mediatek' instead of invalid
'mtk'
MAINTAINERS: add MT7621 CLOCK maintainer
.../bindings/clock/mediatek,mt7621-clk.yaml | 67 +++
MAINTAINERS | 6 +
arch/mips/ralink/mt7621.c | 6 +-
drivers/clk/Kconfig | 1 +
drivers/clk/Makefile | 1 +
drivers/clk/ralink/Kconfig | 14 +
drivers/clk/ralink/Makefile | 2 +
drivers/clk/ralink/clk-mt7621.c | 434 ++++++++++++++++++
drivers/staging/mt7621-dts/gbpc1.dts | 11 -
drivers/staging/mt7621-dts/mt7621.dtsi | 85 ++--
include/dt-bindings/clock/mt7621-clk.h | 41 ++
11 files changed, 609 insertions(+), 59 deletions(-)
create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
create mode 100644 drivers/clk/ralink/Kconfig
create mode 100644 drivers/clk/ralink/Makefile
create mode 100644 drivers/clk/ralink/clk-mt7621.c
create mode 100644 include/dt-bindings/clock/mt7621-clk.h
--
2.25.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v4 0/6] MIPS: ralink: add CPU clock detection and clock driver for MT7621
@ 2020-11-22 9:55 ` Sergio Paracuellos
0 siblings, 0 replies; 36+ messages in thread
From: Sergio Paracuellos @ 2020-11-22 9:55 UTC (permalink / raw)
To: mturquette
Cc: hackpascal, devel, tsbogend, sboyd, gregkh, linux-kernel,
evicetree, linux-mips, 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.
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 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
Sergio Paracuellos (6):
dt-bindings: clock: add dt binding header for mt7621 clocks
dt: bindings: add mt7621-clk device tree binding documentation
clk: ralink: add clock driver for mt7621 SoC
staging: mt7621-dts: make use of new 'mt7621-clk'
staging: mt7621-dts: use valid vendor 'mediatek' instead of invalid
'mtk'
MAINTAINERS: add MT7621 CLOCK maintainer
.../bindings/clock/mediatek,mt7621-clk.yaml | 67 +++
MAINTAINERS | 6 +
arch/mips/ralink/mt7621.c | 6 +-
drivers/clk/Kconfig | 1 +
drivers/clk/Makefile | 1 +
drivers/clk/ralink/Kconfig | 14 +
drivers/clk/ralink/Makefile | 2 +
drivers/clk/ralink/clk-mt7621.c | 434 ++++++++++++++++++
drivers/staging/mt7621-dts/gbpc1.dts | 11 -
drivers/staging/mt7621-dts/mt7621.dtsi | 85 ++--
include/dt-bindings/clock/mt7621-clk.h | 41 ++
11 files changed, 609 insertions(+), 59 deletions(-)
create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
create mode 100644 drivers/clk/ralink/Kconfig
create mode 100644 drivers/clk/ralink/Makefile
create mode 100644 drivers/clk/ralink/clk-mt7621.c
create mode 100644 include/dt-bindings/clock/mt7621-clk.h
--
2.25.1
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v4 1/6] dt-bindings: clock: add dt binding header for mt7621 clocks
2020-11-22 9:55 ` Sergio Paracuellos
@ 2020-11-22 9:55 ` Sergio Paracuellos
-1 siblings, 0 replies; 36+ messages in thread
From: Sergio Paracuellos @ 2020-11-22 9:55 UTC (permalink / raw)
To: mturquette
Cc: sboyd, robh+dt, john, tsbogend, gregkh, gch981213, hackpascal,
linux-clk, evicetree, linux-kernel, linux-mips, devel, neil,
Rob Herring
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
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v4 1/6] dt-bindings: clock: add dt binding header for mt7621 clocks
@ 2020-11-22 9:55 ` Sergio Paracuellos
0 siblings, 0 replies; 36+ messages in thread
From: Sergio Paracuellos @ 2020-11-22 9:55 UTC (permalink / raw)
To: mturquette
Cc: hackpascal, devel, tsbogend, Rob Herring, sboyd, gregkh,
linux-kernel, evicetree, linux-mips, 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] 36+ messages in thread
* [PATCH v4 2/6] dt: bindings: add mt7621-clk device tree binding documentation
2020-11-22 9:55 ` Sergio Paracuellos
@ 2020-11-22 9:55 ` Sergio Paracuellos
-1 siblings, 0 replies; 36+ messages in thread
From: Sergio Paracuellos @ 2020-11-22 9:55 UTC (permalink / raw)
To: mturquette
Cc: sboyd, robh+dt, john, tsbogend, gregkh, gch981213, hackpascal,
linux-clk, evicetree, linux-kernel, linux-mips, devel, neil
Adds device tree binding documentation for clocks in the
MT7621 SOC.
Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
.../bindings/clock/mediatek,mt7621-clk.yaml | 67 +++++++++++++++++++
1 file changed, 67 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
diff --git a/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml b/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
new file mode 100644
index 000000000000..6aca4c1a4a46
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/mediatek,mt7621-clk.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MT7621 Clock Device Tree Bindings
+
+maintainers:
+ - Sergio Paracuellos <sergio.paracuellos@gmail.com>
+
+description: |
+ The MT7621 has a PLL controller from where the cpu clock is provided
+ as well as derived clocks for the bus and the peripherals. It also
+ can gate SoC device clocks.
+
+ Each clock is assigned an identifier and client nodes use this identifier
+ to specify the clock which they consume.
+
+ All these identifiers could be found in:
+ [1]: <include/dt-bindings/clock/mt7621-clk.h>.
+
+ The mt7621 clock node should be the child of a syscon node with the
+ required property:
+
+ - compatible: Should be one of the following:
+ "mediatek,mt7621-sysc", "syscon"
+
+ Refer to the bindings described in
+ Documentation/devicetree/bindings/mfd/syscon.yaml
+
+properties:
+ compatible:
+ const: mediatek,mt7621-clk
+
+ "#clock-cells":
+ description:
+ The first cell indicates the clock gate number, see [1] for available
+ clocks.
+ const: 1
+
+ clock-output-names:
+ maxItems: 8
+
+required:
+ - compatible
+ - '#clock-cells'
+ - clock-output-names
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/mt7621-clk.h>
+
+ sysc: sysc@0 {
+ compatible = "mediatek,mt7621-sysc", "syscon";
+ reg = <0x0 0x100>;
+
+ pll {
+ compatible = "mediatek,mt7621-clk";
+ #clock-cells = <1>;
+ clock-output-names = "xtal", "cpu", "bus",
+ "50m", "125m", "150m",
+ "250m", "270m";
+ };
+ };
--
2.25.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v4 2/6] dt: bindings: add mt7621-clk device tree binding documentation
@ 2020-11-22 9:55 ` Sergio Paracuellos
0 siblings, 0 replies; 36+ messages in thread
From: Sergio Paracuellos @ 2020-11-22 9:55 UTC (permalink / raw)
To: mturquette
Cc: hackpascal, devel, tsbogend, sboyd, gregkh, linux-kernel,
evicetree, linux-mips, robh+dt, john, neil, linux-clk
Adds device tree binding documentation for clocks in the
MT7621 SOC.
Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
.../bindings/clock/mediatek,mt7621-clk.yaml | 67 +++++++++++++++++++
1 file changed, 67 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
diff --git a/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml b/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
new file mode 100644
index 000000000000..6aca4c1a4a46
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/mediatek,mt7621-clk.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MT7621 Clock Device Tree Bindings
+
+maintainers:
+ - Sergio Paracuellos <sergio.paracuellos@gmail.com>
+
+description: |
+ The MT7621 has a PLL controller from where the cpu clock is provided
+ as well as derived clocks for the bus and the peripherals. It also
+ can gate SoC device clocks.
+
+ Each clock is assigned an identifier and client nodes use this identifier
+ to specify the clock which they consume.
+
+ All these identifiers could be found in:
+ [1]: <include/dt-bindings/clock/mt7621-clk.h>.
+
+ The mt7621 clock node should be the child of a syscon node with the
+ required property:
+
+ - compatible: Should be one of the following:
+ "mediatek,mt7621-sysc", "syscon"
+
+ Refer to the bindings described in
+ Documentation/devicetree/bindings/mfd/syscon.yaml
+
+properties:
+ compatible:
+ const: mediatek,mt7621-clk
+
+ "#clock-cells":
+ description:
+ The first cell indicates the clock gate number, see [1] for available
+ clocks.
+ const: 1
+
+ clock-output-names:
+ maxItems: 8
+
+required:
+ - compatible
+ - '#clock-cells'
+ - clock-output-names
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/mt7621-clk.h>
+
+ sysc: sysc@0 {
+ compatible = "mediatek,mt7621-sysc", "syscon";
+ reg = <0x0 0x100>;
+
+ pll {
+ compatible = "mediatek,mt7621-clk";
+ #clock-cells = <1>;
+ 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] 36+ messages in thread
* [PATCH v4 3/6] clk: ralink: add clock driver for mt7621 SoC
2020-11-22 9:55 ` Sergio Paracuellos
@ 2020-11-22 9:55 ` Sergio Paracuellos
-1 siblings, 0 replies; 36+ messages in thread
From: Sergio Paracuellos @ 2020-11-22 9:55 UTC (permalink / raw)
To: mturquette
Cc: sboyd, robh+dt, john, tsbogend, gregkh, gch981213, hackpascal,
linux-clk, evicetree, linux-kernel, linux-mips, devel, neil
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 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"
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 | 14 +
drivers/clk/ralink/Makefile | 2 +
drivers/clk/ralink/clk-mt7621.c | 435 ++++++++++++++++++++++++++++++++
5 files changed, 453 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 c715d4681a0b..5f94c4329033 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -372,6 +372,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 da8fcf147eb1..6578e167b047 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -100,6 +100,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..7e8697327e0c
--- /dev/null
+++ b/drivers/clk/ralink/Kconfig
@@ -0,0 +1,14 @@
+# 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
+ 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..4e929f13fe7c
--- /dev/null
+++ b/drivers/clk/ralink/clk-mt7621.c
@@ -0,0 +1,435 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Mediatek MT7621 Clock Driver
+ * Author: Sergio Paracuellos <sergio.paracuellos@gmail.com>
+ */
+
+#include <linux/bitops.h>
+#include <linux/clk-provider.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/regmap.h>
+#include <asm/mach-ralink/ralink_regs.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 0x7
+#define XTAL_MODE_SEL_SHIFT 6
+
+#define CPU_CLK_SEL_MASK 0x3
+#define CPU_CLK_SEL_SHIFT 30
+
+#define CUR_CPU_FDIV_MASK 0x1f
+#define CUR_CPU_FDIV_SHIFT 8
+#define CUR_CPU_FFRAC_MASK 0x1f
+#define CUR_CPU_FFRAC_SHIFT 0
+
+#define CPU_PLL_PREDIV_MASK 0x3
+#define CPU_PLL_PREDIV_SHIFT 12
+#define CPU_PLL_FBDIV_MASK 0x7f
+#define CPU_PLL_FBDIV_SHIFT 4
+
+#define MHZ(x) ((x) * 1000 * 1000)
+
+struct mt7621_clk_provider {
+ struct device_node *node;
+ struct regmap *syscon_regmap;
+ struct clk_hw_onecell_data *clk_data;
+};
+
+struct mt7621_clk {
+ struct mt7621_clk_provider *clk_prov;
+ struct clk_hw hw;
+};
+
+struct mt7621_fixed_clk {
+ u8 idx;
+ const char *name;
+ const char *parent_name;
+ struct mt7621_clk_provider *clk_prov;
+ unsigned long rate;
+ struct clk_hw *hw;
+};
+
+struct mt7621_gate {
+ u8 idx;
+ const char *name;
+ const char *parent_name;
+ struct mt7621_clk_provider *clk_prov;
+ u32 bit_idx;
+ struct clk_hw hw;
+};
+
+#define GATE(_id, _name, _pname, _shift) \
+ { \
+ .idx = _id, \
+ .name = _name, \
+ .parent_name = _pname, \
+ .clk_prov = NULL, \
+ .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 *scon = clk_gate->clk_prov->syscon_regmap;
+
+ return regmap_update_bits(scon, 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 *scon = clk_gate->clk_prov->syscon_regmap;
+
+ regmap_update_bits(scon, 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 *scon = clk_gate->clk_prov->syscon_regmap;
+ unsigned int val;
+
+ if (regmap_read(scon, SYSC_REG_CLKCFG1, &val))
+ return 0;
+
+ return val & 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_node *np,
+ struct mt7621_gate *sclk)
+{
+ struct clk_init_data init = {
+ .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
+ .num_parents = 1,
+ .parent_names = &sclk->parent_name,
+ .ops = &mt7621_gate_ops,
+ .name = sclk->name,
+ };
+
+ sclk->hw.init = &init;
+ return of_clk_hw_register(np, &sclk->hw);
+}
+
+static int mt7621_register_gates(struct mt7621_clk_provider *clk_prov)
+{
+ struct clk_hw_onecell_data **clk_data = &clk_prov->clk_data;
+ 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->clk_prov = clk_prov;
+ ret = mt7621_gate_ops_init(clk_prov->node, sclk);
+ if (ret) {
+ pr_err("Couldn't register clock %s\n", sclk->name);
+ goto err_clk_unreg;
+ }
+
+ hws[sclk->idx] = &sclk->hw;
+ (*clk_data)->num++;
+ }
+
+ 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, _pname, _rate) \
+ { \
+ .idx = _id, \
+ .name = _name, \
+ .parent_name = _pname, \
+ .clk_prov = NULL, \
+ .rate = _rate \
+ }
+
+static struct mt7621_fixed_clk mt7621_fixed_clks[] = {
+ FIXED(MT7621_CLK_50M, "50m", "xtal", MHZ(50)),
+ FIXED(MT7621_CLK_125M, "125m", "xtal", MHZ(125)),
+ FIXED(MT7621_CLK_150M, "150m", "xtal", MHZ(150)),
+ FIXED(MT7621_CLK_250M, "250m", "xtal", MHZ(250)),
+ FIXED(MT7621_CLK_270M, "270m", "xtal", MHZ(270)),
+};
+
+static int mt7621_register_fixed_clocks(struct mt7621_clk_provider *clk_prov)
+{
+ struct clk_hw_onecell_data **clk_data = &clk_prov->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->clk_prov = clk_prov;
+ sclk->hw = clk_hw_register_fixed_rate(NULL, sclk->name,
+ sclk->parent_name, 0,
+ sclk->rate);
+ if (IS_ERR(sclk->hw)) {
+ pr_err("Couldn't register clock %s\n", sclk->name);
+ ret = PTR_ERR(sclk->hw);
+ goto err_clk_unreg;
+ }
+
+ hws[sclk->idx] = sclk->hw;
+ (*clk_data)->num++;
+ }
+
+ 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 *scon = clk->clk_prov->syscon_regmap;
+ u32 val;
+
+ regmap_read(scon, SYSC_REG_SYSTEM_CONFIG0, &val);
+ val = (val >> XTAL_MODE_SEL_SHIFT) & XTAL_MODE_SEL_MASK;
+
+ if (val <= 2)
+ return MHZ(20);
+ else if (val <= 5)
+ return MHZ(40);
+
+ return MHZ(25);
+}
+
+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 *scon = clk->clk_prov->syscon_regmap;
+ u32 clkcfg, clk_sel, curclk, ffiv, ffrac;
+ u32 pll, prediv, fbdiv;
+ unsigned long cpu_clk;
+
+ regmap_read(scon, SYSC_REG_CLKCFG0, &clkcfg);
+ clk_sel = (clkcfg >> CPU_CLK_SEL_SHIFT) & CPU_CLK_SEL_MASK;
+
+ regmap_read(scon, SYSC_REG_CUR_CLK_STS, &curclk);
+ ffiv = (curclk >> CUR_CPU_FDIV_SHIFT) & CUR_CPU_FDIV_MASK;
+ ffrac = (curclk >> CUR_CPU_FFRAC_SHIFT) & CUR_CPU_FFRAC_MASK;
+
+ switch (clk_sel) {
+ case 0:
+ cpu_clk = MHZ(500);
+ break;
+ case 1:
+ pll = rt_memc_r32(MEMC_REG_CPU_PLL);
+ fbdiv = (pll >> CPU_PLL_FBDIV_SHIFT) & CPU_PLL_FBDIV_MASK;
+ prediv = (pll >> CPU_PLL_PREDIV_SHIFT) & CPU_PLL_PREDIV_MASK;
+ 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_names = (const char *const[]) { _parent }, \
+ .num_parents = _parent ? 1 : 0 \
+ }, \
+}
+
+static struct mt7621_clk mt7621_clks_base[] = {
+ { NULL, CLK_BASE("xtal", NULL, mt7621_xtal_recalc_rate) },
+ { NULL, CLK_BASE("cpu", "xtal", mt7621_cpu_recalc_rate) },
+ { NULL, CLK_BASE("bus", "cpu", mt7621_bus_recalc_rate) },
+};
+
+static int mt7621_register_top_clocks(struct mt7621_clk_provider *clk_prov)
+{
+ struct clk_hw_onecell_data **clk_data = &clk_prov->clk_data;
+ struct clk_hw **hws = (*clk_data)->hws;
+ int ret, i;
+
+ for (i = 0; i < ARRAY_SIZE(mt7621_clks_base); i++) {
+ struct mt7621_clk *sclk = &mt7621_clks_base[i];
+
+ sclk->clk_prov = clk_prov;
+ ret = of_clk_hw_register(clk_prov->node, &sclk->hw);
+ if (ret) {
+ pr_err("Couldn't register top clock %i\n", i);
+ goto err_clk_unreg;
+ }
+
+ hws[i] = &sclk->hw;
+ (*clk_data)->num++;
+ }
+
+ return 0;
+
+err_clk_unreg:
+ while (--i >= 0) {
+ struct mt7621_clk *sclk = &mt7621_clks_base[i];
+
+ clk_hw_unregister(&sclk->hw);
+ }
+ return ret;
+}
+
+static void __init mt7621_clk_init(struct device_node *node)
+{
+ struct mt7621_clk_provider *clk_prov;
+ struct clk_hw_onecell_data **clk_data;
+ int i, ret, count;
+
+ clk_prov = kzalloc(sizeof(*clk_prov), GFP_KERNEL);
+ if (!clk_prov)
+ return;
+
+ clk_prov->syscon_regmap = syscon_node_to_regmap(node->parent);
+ if (IS_ERR(clk_prov->syscon_regmap)) {
+ pr_err("Could not get syscon regmap\n");
+ goto free_clk_prov;
+ }
+
+ clk_prov->node = node;
+
+ clk_data = &clk_prov->clk_data;
+ 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_prov;
+
+ ret = mt7621_register_top_clocks(clk_prov);
+ if (ret) {
+ pr_err("Couldn't register top clocks\n");
+ goto free_clk_data;
+ }
+
+ ret = mt7621_register_fixed_clocks(clk_prov);
+ if (ret) {
+ pr_err("Couldn't register fixed clocks\n");
+ goto unreg_clk_top;
+ }
+
+ ret = mt7621_register_gates(clk_prov);
+ if (ret) {
+ pr_err("Couldn't register fixed clock gates\n");
+ goto unreg_clk_fixed;
+ }
+
+ ret = of_clk_add_hw_provider(node, of_clk_hw_onecell_get,
+ clk_prov->clk_data);
+ if (ret) {
+ pr_err("Couldn't add clk hw provider\n");
+ goto unreg_clk_gates;
+ }
+
+ return;
+
+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);
+ }
+
+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_prov->clk_data);
+
+free_clk_prov:
+ kfree(clk_prov);
+}
+
+CLK_OF_DECLARE(mt7621_clk, "mediatek,mt7621-clk", mt7621_clk_init);
+
+MODULE_AUTHOR("Sergio Paracuellos <sergio.paracuellos@gmail.com>");
+MODULE_DESCRIPTION("Mediatek Mt7621 clock driver");
+MODULE_LICENSE("GPL v2");
--
2.25.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v4 3/6] clk: ralink: add clock driver for mt7621 SoC
@ 2020-11-22 9:55 ` Sergio Paracuellos
0 siblings, 0 replies; 36+ messages in thread
From: Sergio Paracuellos @ 2020-11-22 9:55 UTC (permalink / raw)
To: mturquette
Cc: hackpascal, devel, tsbogend, sboyd, gregkh, linux-kernel,
evicetree, linux-mips, 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 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"
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 | 14 +
drivers/clk/ralink/Makefile | 2 +
drivers/clk/ralink/clk-mt7621.c | 435 ++++++++++++++++++++++++++++++++
5 files changed, 453 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 c715d4681a0b..5f94c4329033 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -372,6 +372,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 da8fcf147eb1..6578e167b047 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -100,6 +100,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..7e8697327e0c
--- /dev/null
+++ b/drivers/clk/ralink/Kconfig
@@ -0,0 +1,14 @@
+# 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
+ 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..4e929f13fe7c
--- /dev/null
+++ b/drivers/clk/ralink/clk-mt7621.c
@@ -0,0 +1,435 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Mediatek MT7621 Clock Driver
+ * Author: Sergio Paracuellos <sergio.paracuellos@gmail.com>
+ */
+
+#include <linux/bitops.h>
+#include <linux/clk-provider.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/regmap.h>
+#include <asm/mach-ralink/ralink_regs.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 0x7
+#define XTAL_MODE_SEL_SHIFT 6
+
+#define CPU_CLK_SEL_MASK 0x3
+#define CPU_CLK_SEL_SHIFT 30
+
+#define CUR_CPU_FDIV_MASK 0x1f
+#define CUR_CPU_FDIV_SHIFT 8
+#define CUR_CPU_FFRAC_MASK 0x1f
+#define CUR_CPU_FFRAC_SHIFT 0
+
+#define CPU_PLL_PREDIV_MASK 0x3
+#define CPU_PLL_PREDIV_SHIFT 12
+#define CPU_PLL_FBDIV_MASK 0x7f
+#define CPU_PLL_FBDIV_SHIFT 4
+
+#define MHZ(x) ((x) * 1000 * 1000)
+
+struct mt7621_clk_provider {
+ struct device_node *node;
+ struct regmap *syscon_regmap;
+ struct clk_hw_onecell_data *clk_data;
+};
+
+struct mt7621_clk {
+ struct mt7621_clk_provider *clk_prov;
+ struct clk_hw hw;
+};
+
+struct mt7621_fixed_clk {
+ u8 idx;
+ const char *name;
+ const char *parent_name;
+ struct mt7621_clk_provider *clk_prov;
+ unsigned long rate;
+ struct clk_hw *hw;
+};
+
+struct mt7621_gate {
+ u8 idx;
+ const char *name;
+ const char *parent_name;
+ struct mt7621_clk_provider *clk_prov;
+ u32 bit_idx;
+ struct clk_hw hw;
+};
+
+#define GATE(_id, _name, _pname, _shift) \
+ { \
+ .idx = _id, \
+ .name = _name, \
+ .parent_name = _pname, \
+ .clk_prov = NULL, \
+ .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 *scon = clk_gate->clk_prov->syscon_regmap;
+
+ return regmap_update_bits(scon, 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 *scon = clk_gate->clk_prov->syscon_regmap;
+
+ regmap_update_bits(scon, 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 *scon = clk_gate->clk_prov->syscon_regmap;
+ unsigned int val;
+
+ if (regmap_read(scon, SYSC_REG_CLKCFG1, &val))
+ return 0;
+
+ return val & 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_node *np,
+ struct mt7621_gate *sclk)
+{
+ struct clk_init_data init = {
+ .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
+ .num_parents = 1,
+ .parent_names = &sclk->parent_name,
+ .ops = &mt7621_gate_ops,
+ .name = sclk->name,
+ };
+
+ sclk->hw.init = &init;
+ return of_clk_hw_register(np, &sclk->hw);
+}
+
+static int mt7621_register_gates(struct mt7621_clk_provider *clk_prov)
+{
+ struct clk_hw_onecell_data **clk_data = &clk_prov->clk_data;
+ 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->clk_prov = clk_prov;
+ ret = mt7621_gate_ops_init(clk_prov->node, sclk);
+ if (ret) {
+ pr_err("Couldn't register clock %s\n", sclk->name);
+ goto err_clk_unreg;
+ }
+
+ hws[sclk->idx] = &sclk->hw;
+ (*clk_data)->num++;
+ }
+
+ 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, _pname, _rate) \
+ { \
+ .idx = _id, \
+ .name = _name, \
+ .parent_name = _pname, \
+ .clk_prov = NULL, \
+ .rate = _rate \
+ }
+
+static struct mt7621_fixed_clk mt7621_fixed_clks[] = {
+ FIXED(MT7621_CLK_50M, "50m", "xtal", MHZ(50)),
+ FIXED(MT7621_CLK_125M, "125m", "xtal", MHZ(125)),
+ FIXED(MT7621_CLK_150M, "150m", "xtal", MHZ(150)),
+ FIXED(MT7621_CLK_250M, "250m", "xtal", MHZ(250)),
+ FIXED(MT7621_CLK_270M, "270m", "xtal", MHZ(270)),
+};
+
+static int mt7621_register_fixed_clocks(struct mt7621_clk_provider *clk_prov)
+{
+ struct clk_hw_onecell_data **clk_data = &clk_prov->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->clk_prov = clk_prov;
+ sclk->hw = clk_hw_register_fixed_rate(NULL, sclk->name,
+ sclk->parent_name, 0,
+ sclk->rate);
+ if (IS_ERR(sclk->hw)) {
+ pr_err("Couldn't register clock %s\n", sclk->name);
+ ret = PTR_ERR(sclk->hw);
+ goto err_clk_unreg;
+ }
+
+ hws[sclk->idx] = sclk->hw;
+ (*clk_data)->num++;
+ }
+
+ 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 *scon = clk->clk_prov->syscon_regmap;
+ u32 val;
+
+ regmap_read(scon, SYSC_REG_SYSTEM_CONFIG0, &val);
+ val = (val >> XTAL_MODE_SEL_SHIFT) & XTAL_MODE_SEL_MASK;
+
+ if (val <= 2)
+ return MHZ(20);
+ else if (val <= 5)
+ return MHZ(40);
+
+ return MHZ(25);
+}
+
+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 *scon = clk->clk_prov->syscon_regmap;
+ u32 clkcfg, clk_sel, curclk, ffiv, ffrac;
+ u32 pll, prediv, fbdiv;
+ unsigned long cpu_clk;
+
+ regmap_read(scon, SYSC_REG_CLKCFG0, &clkcfg);
+ clk_sel = (clkcfg >> CPU_CLK_SEL_SHIFT) & CPU_CLK_SEL_MASK;
+
+ regmap_read(scon, SYSC_REG_CUR_CLK_STS, &curclk);
+ ffiv = (curclk >> CUR_CPU_FDIV_SHIFT) & CUR_CPU_FDIV_MASK;
+ ffrac = (curclk >> CUR_CPU_FFRAC_SHIFT) & CUR_CPU_FFRAC_MASK;
+
+ switch (clk_sel) {
+ case 0:
+ cpu_clk = MHZ(500);
+ break;
+ case 1:
+ pll = rt_memc_r32(MEMC_REG_CPU_PLL);
+ fbdiv = (pll >> CPU_PLL_FBDIV_SHIFT) & CPU_PLL_FBDIV_MASK;
+ prediv = (pll >> CPU_PLL_PREDIV_SHIFT) & CPU_PLL_PREDIV_MASK;
+ 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_names = (const char *const[]) { _parent }, \
+ .num_parents = _parent ? 1 : 0 \
+ }, \
+}
+
+static struct mt7621_clk mt7621_clks_base[] = {
+ { NULL, CLK_BASE("xtal", NULL, mt7621_xtal_recalc_rate) },
+ { NULL, CLK_BASE("cpu", "xtal", mt7621_cpu_recalc_rate) },
+ { NULL, CLK_BASE("bus", "cpu", mt7621_bus_recalc_rate) },
+};
+
+static int mt7621_register_top_clocks(struct mt7621_clk_provider *clk_prov)
+{
+ struct clk_hw_onecell_data **clk_data = &clk_prov->clk_data;
+ struct clk_hw **hws = (*clk_data)->hws;
+ int ret, i;
+
+ for (i = 0; i < ARRAY_SIZE(mt7621_clks_base); i++) {
+ struct mt7621_clk *sclk = &mt7621_clks_base[i];
+
+ sclk->clk_prov = clk_prov;
+ ret = of_clk_hw_register(clk_prov->node, &sclk->hw);
+ if (ret) {
+ pr_err("Couldn't register top clock %i\n", i);
+ goto err_clk_unreg;
+ }
+
+ hws[i] = &sclk->hw;
+ (*clk_data)->num++;
+ }
+
+ return 0;
+
+err_clk_unreg:
+ while (--i >= 0) {
+ struct mt7621_clk *sclk = &mt7621_clks_base[i];
+
+ clk_hw_unregister(&sclk->hw);
+ }
+ return ret;
+}
+
+static void __init mt7621_clk_init(struct device_node *node)
+{
+ struct mt7621_clk_provider *clk_prov;
+ struct clk_hw_onecell_data **clk_data;
+ int i, ret, count;
+
+ clk_prov = kzalloc(sizeof(*clk_prov), GFP_KERNEL);
+ if (!clk_prov)
+ return;
+
+ clk_prov->syscon_regmap = syscon_node_to_regmap(node->parent);
+ if (IS_ERR(clk_prov->syscon_regmap)) {
+ pr_err("Could not get syscon regmap\n");
+ goto free_clk_prov;
+ }
+
+ clk_prov->node = node;
+
+ clk_data = &clk_prov->clk_data;
+ 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_prov;
+
+ ret = mt7621_register_top_clocks(clk_prov);
+ if (ret) {
+ pr_err("Couldn't register top clocks\n");
+ goto free_clk_data;
+ }
+
+ ret = mt7621_register_fixed_clocks(clk_prov);
+ if (ret) {
+ pr_err("Couldn't register fixed clocks\n");
+ goto unreg_clk_top;
+ }
+
+ ret = mt7621_register_gates(clk_prov);
+ if (ret) {
+ pr_err("Couldn't register fixed clock gates\n");
+ goto unreg_clk_fixed;
+ }
+
+ ret = of_clk_add_hw_provider(node, of_clk_hw_onecell_get,
+ clk_prov->clk_data);
+ if (ret) {
+ pr_err("Couldn't add clk hw provider\n");
+ goto unreg_clk_gates;
+ }
+
+ return;
+
+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);
+ }
+
+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_prov->clk_data);
+
+free_clk_prov:
+ kfree(clk_prov);
+}
+
+CLK_OF_DECLARE(mt7621_clk, "mediatek,mt7621-clk", mt7621_clk_init);
+
+MODULE_AUTHOR("Sergio Paracuellos <sergio.paracuellos@gmail.com>");
+MODULE_DESCRIPTION("Mediatek Mt7621 clock driver");
+MODULE_LICENSE("GPL v2");
--
2.25.1
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v4 4/6] staging: mt7621-dts: make use of new 'mt7621-clk'
2020-11-22 9:55 ` Sergio Paracuellos
@ 2020-11-22 9:55 ` Sergio Paracuellos
-1 siblings, 0 replies; 36+ messages in thread
From: Sergio Paracuellos @ 2020-11-22 9:55 UTC (permalink / raw)
To: mturquette
Cc: sboyd, robh+dt, john, tsbogend, gregkh, gch981213, hackpascal,
linux-clk, evicetree, linux-kernel, linux-mips, devel, neil
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.
Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
drivers/staging/mt7621-dts/gbpc1.dts | 11 ----
drivers/staging/mt7621-dts/mt7621.dtsi | 75 ++++++++++++--------------
2 files changed, 35 insertions(+), 51 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 82aa93634eda..35cfda8f6faf 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,16 @@ palmbus: palmbus@1E000000 {
#size-cells = <1>;
sysc: sysc@0 {
- compatible = "mtk,mt7621-sysc";
+ compatible = "mtk,mt7621-sysc", "syscon";
reg = <0x0 0x100>;
+
+ pll: pll {
+ compatible = "mediatek,mt7621-clk";
+ #clock-cells = <1>;
+ clock-output-names = "xtal", "cpu", "bus",
+ "50m", "125m", "150m",
+ "250m", "270m";
+ };
};
wdt: wdt@100 {
@@ -100,8 +88,8 @@ i2c: i2c@900 {
compatible = "mediatek,mt7621-i2c";
reg = <0x900 0x100>;
- clocks = <&sysclock>;
-
+ clocks = <&pll MT7621_CLK_I2C>;
+ clock-names = "i2c";
resets = <&rstctrl 16>;
reset-names = "i2c";
@@ -118,8 +106,8 @@ i2s: i2s@a00 {
compatible = "mediatek,mt7621-i2s";
reg = <0xa00 0x100>;
- clocks = <&sysclock>;
-
+ clocks = <&pll MT7621_CLK_I2S>;
+ clock-names = "i2s";
resets = <&rstctrl 17>;
reset-names = "i2s";
@@ -155,8 +143,8 @@ uartlite: uartlite@c00 {
compatible = "ns16550a";
reg = <0xc00 0x100>;
- clocks = <&sysclock>;
- clock-frequency = <50000000>;
+ clocks = <&pll MT7621_CLK_UART1>;
+ clock-names = "uart1";
interrupt-parent = <&gic>;
interrupts = <GIC_SHARED 26 IRQ_TYPE_LEVEL_HIGH>;
@@ -172,7 +160,8 @@ spi0: spi@b00 {
compatible = "ralink,mt7621-spi";
reg = <0xb00 0x100>;
- clocks = <&sysclock>;
+ clocks = <&pll MT7621_CLK_SPI>;
+ clock-names = "spi";
resets = <&rstctrl 18>;
reset-names = "spi";
@@ -188,6 +177,8 @@ gdma: gdma@2800 {
compatible = "ralink,rt3883-gdma";
reg = <0x2800 0x800>;
+ clocks = <&pll MT7621_CLK_GDMA>;
+ clock-names = "gdma";
resets = <&rstctrl 14>;
reset-names = "dma";
@@ -205,6 +196,8 @@ hsdma: hsdma@7000 {
compatible = "mediatek,mt7621-hsdma";
reg = <0x7000 0x1000>;
+ clocks = <&pll MT7621_CLK_HSDMA>;
+ clock-names = "hsdma";
resets = <&rstctrl 5>;
reset-names = "hsdma";
@@ -315,11 +308,6 @@ rstctrl: rstctrl {
#reset-cells = <1>;
};
- clkctrl: clkctrl {
- compatible = "ralink,rt2880-clock";
- #clock-cells = <1>;
- };
-
sdhci: sdhci@1E130000 {
status = "disabled";
@@ -338,7 +326,8 @@ sdhci: sdhci@1E130000 {
pinctrl-0 = <&sdhci_pins>;
pinctrl-1 = <&sdhci_pins>;
- clocks = <&mmc_clock &mmc_clock>;
+ clocks = <&pll MT7621_CLK_SHXC>,
+ <&pll MT7621_CLK_50M>;
clock-names = "source", "hclk";
interrupt-parent = <&gic>;
@@ -353,7 +342,7 @@ xhci: xhci@1E1C0000 {
0x1e1d0700 0x0100>;
reg-names = "mac", "ippc";
- clocks = <&sysclock>;
+ clocks = <&pll MT7621_CLK_XTAL>;
clock-names = "sys_ck";
interrupt-parent = <&gic>;
@@ -372,7 +361,7 @@ gic: interrupt-controller@1fbc0000 {
timer {
compatible = "mti,gic-timer";
interrupts = <GIC_LOCAL 1 IRQ_TYPE_NONE>;
- clocks = <&cpuclock>;
+ clocks = <&pll MT7621_CLK_CPU>;
};
};
@@ -385,6 +374,9 @@ nand: nand@1e003000 {
0x1e003800 0x800>;
#address-cells = <1>;
#size-cells = <1>;
+
+ clocks = <&pll MT7621_CLK_NAND>;
+ clock-names = "nand";
};
ethsys: syscon@1e000000 {
@@ -398,8 +390,9 @@ ethernet: ethernet@1e100000 {
compatible = "mediatek,mt7621-eth";
reg = <0x1e100000 0x10000>;
- clocks = <&sysclock>;
- clock-names = "ethif";
+ clocks = <&pll MT7621_CLK_FE>,
+ <&pll MT7621_CLK_ETH>;
+ clock-names = "fe", "ethif";
#address-cells = <1>;
#size-cells = <0>;
@@ -532,7 +525,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 = <&pll MT7621_CLK_PCIE0>,
+ <&pll MT7621_CLK_PCIE1>,
+ <&pll MT7621_CLK_PCIE2>;
clock-names = "pcie0", "pcie1", "pcie2";
phys = <&pcie0_phy 1>, <&pcie2_phy 0>;
phy-names = "pcie-phy0", "pcie-phy2";
--
2.25.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v4 4/6] staging: mt7621-dts: make use of new 'mt7621-clk'
@ 2020-11-22 9:55 ` Sergio Paracuellos
0 siblings, 0 replies; 36+ messages in thread
From: Sergio Paracuellos @ 2020-11-22 9:55 UTC (permalink / raw)
To: mturquette
Cc: hackpascal, devel, tsbogend, sboyd, gregkh, linux-kernel,
evicetree, linux-mips, 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.
Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
drivers/staging/mt7621-dts/gbpc1.dts | 11 ----
drivers/staging/mt7621-dts/mt7621.dtsi | 75 ++++++++++++--------------
2 files changed, 35 insertions(+), 51 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 82aa93634eda..35cfda8f6faf 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,16 @@ palmbus: palmbus@1E000000 {
#size-cells = <1>;
sysc: sysc@0 {
- compatible = "mtk,mt7621-sysc";
+ compatible = "mtk,mt7621-sysc", "syscon";
reg = <0x0 0x100>;
+
+ pll: pll {
+ compatible = "mediatek,mt7621-clk";
+ #clock-cells = <1>;
+ clock-output-names = "xtal", "cpu", "bus",
+ "50m", "125m", "150m",
+ "250m", "270m";
+ };
};
wdt: wdt@100 {
@@ -100,8 +88,8 @@ i2c: i2c@900 {
compatible = "mediatek,mt7621-i2c";
reg = <0x900 0x100>;
- clocks = <&sysclock>;
-
+ clocks = <&pll MT7621_CLK_I2C>;
+ clock-names = "i2c";
resets = <&rstctrl 16>;
reset-names = "i2c";
@@ -118,8 +106,8 @@ i2s: i2s@a00 {
compatible = "mediatek,mt7621-i2s";
reg = <0xa00 0x100>;
- clocks = <&sysclock>;
-
+ clocks = <&pll MT7621_CLK_I2S>;
+ clock-names = "i2s";
resets = <&rstctrl 17>;
reset-names = "i2s";
@@ -155,8 +143,8 @@ uartlite: uartlite@c00 {
compatible = "ns16550a";
reg = <0xc00 0x100>;
- clocks = <&sysclock>;
- clock-frequency = <50000000>;
+ clocks = <&pll MT7621_CLK_UART1>;
+ clock-names = "uart1";
interrupt-parent = <&gic>;
interrupts = <GIC_SHARED 26 IRQ_TYPE_LEVEL_HIGH>;
@@ -172,7 +160,8 @@ spi0: spi@b00 {
compatible = "ralink,mt7621-spi";
reg = <0xb00 0x100>;
- clocks = <&sysclock>;
+ clocks = <&pll MT7621_CLK_SPI>;
+ clock-names = "spi";
resets = <&rstctrl 18>;
reset-names = "spi";
@@ -188,6 +177,8 @@ gdma: gdma@2800 {
compatible = "ralink,rt3883-gdma";
reg = <0x2800 0x800>;
+ clocks = <&pll MT7621_CLK_GDMA>;
+ clock-names = "gdma";
resets = <&rstctrl 14>;
reset-names = "dma";
@@ -205,6 +196,8 @@ hsdma: hsdma@7000 {
compatible = "mediatek,mt7621-hsdma";
reg = <0x7000 0x1000>;
+ clocks = <&pll MT7621_CLK_HSDMA>;
+ clock-names = "hsdma";
resets = <&rstctrl 5>;
reset-names = "hsdma";
@@ -315,11 +308,6 @@ rstctrl: rstctrl {
#reset-cells = <1>;
};
- clkctrl: clkctrl {
- compatible = "ralink,rt2880-clock";
- #clock-cells = <1>;
- };
-
sdhci: sdhci@1E130000 {
status = "disabled";
@@ -338,7 +326,8 @@ sdhci: sdhci@1E130000 {
pinctrl-0 = <&sdhci_pins>;
pinctrl-1 = <&sdhci_pins>;
- clocks = <&mmc_clock &mmc_clock>;
+ clocks = <&pll MT7621_CLK_SHXC>,
+ <&pll MT7621_CLK_50M>;
clock-names = "source", "hclk";
interrupt-parent = <&gic>;
@@ -353,7 +342,7 @@ xhci: xhci@1E1C0000 {
0x1e1d0700 0x0100>;
reg-names = "mac", "ippc";
- clocks = <&sysclock>;
+ clocks = <&pll MT7621_CLK_XTAL>;
clock-names = "sys_ck";
interrupt-parent = <&gic>;
@@ -372,7 +361,7 @@ gic: interrupt-controller@1fbc0000 {
timer {
compatible = "mti,gic-timer";
interrupts = <GIC_LOCAL 1 IRQ_TYPE_NONE>;
- clocks = <&cpuclock>;
+ clocks = <&pll MT7621_CLK_CPU>;
};
};
@@ -385,6 +374,9 @@ nand: nand@1e003000 {
0x1e003800 0x800>;
#address-cells = <1>;
#size-cells = <1>;
+
+ clocks = <&pll MT7621_CLK_NAND>;
+ clock-names = "nand";
};
ethsys: syscon@1e000000 {
@@ -398,8 +390,9 @@ ethernet: ethernet@1e100000 {
compatible = "mediatek,mt7621-eth";
reg = <0x1e100000 0x10000>;
- clocks = <&sysclock>;
- clock-names = "ethif";
+ clocks = <&pll MT7621_CLK_FE>,
+ <&pll MT7621_CLK_ETH>;
+ clock-names = "fe", "ethif";
#address-cells = <1>;
#size-cells = <0>;
@@ -532,7 +525,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 = <&pll MT7621_CLK_PCIE0>,
+ <&pll MT7621_CLK_PCIE1>,
+ <&pll 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] 36+ messages in thread
* [PATCH v4 5/6] staging: mt7621-dts: use valid vendor 'mediatek' instead of invalid 'mtk'
2020-11-22 9:55 ` Sergio Paracuellos
@ 2020-11-22 9:55 ` Sergio Paracuellos
-1 siblings, 0 replies; 36+ messages in thread
From: Sergio Paracuellos @ 2020-11-22 9:55 UTC (permalink / raw)
To: mturquette
Cc: sboyd, robh+dt, john, tsbogend, gregkh, gch981213, hackpascal,
linux-clk, evicetree, linux-kernel, linux-mips, devel, neil
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'.
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 35cfda8f6faf..8fc311703beb 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>;
pll: pll {
@@ -69,7 +69,7 @@ pll: pll {
};
wdt: wdt@100 {
- compatible = "mtk,mt7621-wdt";
+ compatible = "mediatek,mt7621-wdt";
reg = <0x100 0x100>;
};
@@ -125,17 +125,17 @@ i2s: i2s@a00 {
};
memc: memc@5000 {
- compatible = "mtk,mt7621-memc";
+ compatible = "mediatek,mt7621-memc";
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>;
};
@@ -368,7 +368,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
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v4 5/6] staging: mt7621-dts: use valid vendor 'mediatek' instead of invalid 'mtk'
@ 2020-11-22 9:55 ` Sergio Paracuellos
0 siblings, 0 replies; 36+ messages in thread
From: Sergio Paracuellos @ 2020-11-22 9:55 UTC (permalink / raw)
To: mturquette
Cc: hackpascal, devel, tsbogend, sboyd, gregkh, linux-kernel,
evicetree, linux-mips, 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'.
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 35cfda8f6faf..8fc311703beb 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>;
pll: pll {
@@ -69,7 +69,7 @@ pll: pll {
};
wdt: wdt@100 {
- compatible = "mtk,mt7621-wdt";
+ compatible = "mediatek,mt7621-wdt";
reg = <0x100 0x100>;
};
@@ -125,17 +125,17 @@ i2s: i2s@a00 {
};
memc: memc@5000 {
- compatible = "mtk,mt7621-memc";
+ compatible = "mediatek,mt7621-memc";
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>;
};
@@ -368,7 +368,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] 36+ messages in thread
* [PATCH v4 6/6] MAINTAINERS: add MT7621 CLOCK maintainer
2020-11-22 9:55 ` Sergio Paracuellos
@ 2020-11-22 9:55 ` Sergio Paracuellos
-1 siblings, 0 replies; 36+ messages in thread
From: Sergio Paracuellos @ 2020-11-22 9:55 UTC (permalink / raw)
To: mturquette
Cc: sboyd, robh+dt, john, tsbogend, gregkh, gch981213, hackpascal,
linux-clk, evicetree, linux-kernel, linux-mips, devel, neil
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 f1f088a29bc2..30822ad6837c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11142,6 +11142,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-clk.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
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v4 6/6] MAINTAINERS: add MT7621 CLOCK maintainer
@ 2020-11-22 9:55 ` Sergio Paracuellos
0 siblings, 0 replies; 36+ messages in thread
From: Sergio Paracuellos @ 2020-11-22 9:55 UTC (permalink / raw)
To: mturquette
Cc: hackpascal, devel, tsbogend, sboyd, gregkh, linux-kernel,
evicetree, linux-mips, 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 f1f088a29bc2..30822ad6837c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11142,6 +11142,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-clk.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] 36+ messages in thread
* Re: [PATCH v4 0/6] MIPS: ralink: add CPU clock detection and clock driver for MT7621
2020-11-22 9:55 ` Sergio Paracuellos
@ 2020-12-10 6:55 ` Sergio Paracuellos
-1 siblings, 0 replies; 36+ messages in thread
From: Sergio Paracuellos @ 2020-12-10 6:55 UTC (permalink / raw)
To: Michael Turquette
Cc: Stephen Boyd, Rob Herring, John Crispin, Thomas Bogendoerfer,
Greg KH, Chuanhong Guo, Weijie Gao,
open list:COMMON CLK FRAMEWORK, linux-kernel, open list:MIPS,
open list:STAGING SUBSYSTEM, NeilBrown,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
Hi all,
On Sun, Nov 22, 2020 at 10:55 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.
>
> 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 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.
Any comments on this?? Should I resend the series to get reviewed?
Thanks in advance for your time!
Best regards,
Sergio Paracuellos
>
> 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
>
>
> Sergio Paracuellos (6):
> dt-bindings: clock: add dt binding header for mt7621 clocks
> dt: bindings: add mt7621-clk device tree binding documentation
> clk: ralink: add clock driver for mt7621 SoC
> staging: mt7621-dts: make use of new 'mt7621-clk'
> staging: mt7621-dts: use valid vendor 'mediatek' instead of invalid
> 'mtk'
> MAINTAINERS: add MT7621 CLOCK maintainer
>
> .../bindings/clock/mediatek,mt7621-clk.yaml | 67 +++
> MAINTAINERS | 6 +
> arch/mips/ralink/mt7621.c | 6 +-
> drivers/clk/Kconfig | 1 +
> drivers/clk/Makefile | 1 +
> drivers/clk/ralink/Kconfig | 14 +
> drivers/clk/ralink/Makefile | 2 +
> drivers/clk/ralink/clk-mt7621.c | 434 ++++++++++++++++++
> drivers/staging/mt7621-dts/gbpc1.dts | 11 -
> drivers/staging/mt7621-dts/mt7621.dtsi | 85 ++--
> include/dt-bindings/clock/mt7621-clk.h | 41 ++
> 11 files changed, 609 insertions(+), 59 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
> create mode 100644 drivers/clk/ralink/Kconfig
> create mode 100644 drivers/clk/ralink/Makefile
> create mode 100644 drivers/clk/ralink/clk-mt7621.c
> create mode 100644 include/dt-bindings/clock/mt7621-clk.h
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 0/6] MIPS: ralink: add CPU clock detection and clock driver for MT7621
@ 2020-12-10 6:55 ` Sergio Paracuellos
0 siblings, 0 replies; 36+ messages in thread
From: Sergio Paracuellos @ 2020-12-10 6:55 UTC (permalink / raw)
To: Michael Turquette
Cc: Weijie Gao, open list:STAGING SUBSYSTEM, Thomas Bogendoerfer,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Stephen Boyd, Greg KH, linux-kernel, open list:MIPS, Rob Herring,
John Crispin, NeilBrown, open list:COMMON CLK FRAMEWORK
Hi all,
On Sun, Nov 22, 2020 at 10:55 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.
>
> 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 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.
Any comments on this?? Should I resend the series to get reviewed?
Thanks in advance for your time!
Best regards,
Sergio Paracuellos
>
> 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
>
>
> Sergio Paracuellos (6):
> dt-bindings: clock: add dt binding header for mt7621 clocks
> dt: bindings: add mt7621-clk device tree binding documentation
> clk: ralink: add clock driver for mt7621 SoC
> staging: mt7621-dts: make use of new 'mt7621-clk'
> staging: mt7621-dts: use valid vendor 'mediatek' instead of invalid
> 'mtk'
> MAINTAINERS: add MT7621 CLOCK maintainer
>
> .../bindings/clock/mediatek,mt7621-clk.yaml | 67 +++
> MAINTAINERS | 6 +
> arch/mips/ralink/mt7621.c | 6 +-
> drivers/clk/Kconfig | 1 +
> drivers/clk/Makefile | 1 +
> drivers/clk/ralink/Kconfig | 14 +
> drivers/clk/ralink/Makefile | 2 +
> drivers/clk/ralink/clk-mt7621.c | 434 ++++++++++++++++++
> drivers/staging/mt7621-dts/gbpc1.dts | 11 -
> drivers/staging/mt7621-dts/mt7621.dtsi | 85 ++--
> include/dt-bindings/clock/mt7621-clk.h | 41 ++
> 11 files changed, 609 insertions(+), 59 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
> create mode 100644 drivers/clk/ralink/Kconfig
> create mode 100644 drivers/clk/ralink/Makefile
> create mode 100644 drivers/clk/ralink/clk-mt7621.c
> create mode 100644 include/dt-bindings/clock/mt7621-clk.h
>
> --
> 2.25.1
>
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 2/6] dt: bindings: add mt7621-clk device tree binding documentation
2020-11-22 9:55 ` Sergio Paracuellos
@ 2020-12-17 8:58 ` Stephen Boyd
-1 siblings, 0 replies; 36+ messages in thread
From: Stephen Boyd @ 2020-12-17 8:58 UTC (permalink / raw)
To: Sergio Paracuellos, mturquette
Cc: robh+dt, john, tsbogend, gregkh, gch981213, hackpascal,
linux-clk, evicetree, linux-kernel, linux-mips, devel, neil
Quoting Sergio Paracuellos (2020-11-22 01:55:52)
> Adds device tree binding documentation for clocks in the
> MT7621 SOC.
>
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> ---
> .../bindings/clock/mediatek,mt7621-clk.yaml | 67 +++++++++++++++++++
> 1 file changed, 67 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
>
Rob?
> diff --git a/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml b/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
> new file mode 100644
> index 000000000000..6aca4c1a4a46
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
> @@ -0,0 +1,67 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/mediatek,mt7621-clk.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MT7621 Clock Device Tree Bindings
> +
> +maintainers:
> + - Sergio Paracuellos <sergio.paracuellos@gmail.com>
> +
> +description: |
> + The MT7621 has a PLL controller from where the cpu clock is provided
> + as well as derived clocks for the bus and the peripherals. It also
> + can gate SoC device clocks.
> +
> + Each clock is assigned an identifier and client nodes use this identifier
> + to specify the clock which they consume.
> +
> + All these identifiers could be found in:
> + [1]: <include/dt-bindings/clock/mt7621-clk.h>.
> +
> + The mt7621 clock node should be the child of a syscon node with the
> + required property:
> +
> + - compatible: Should be one of the following:
> + "mediatek,mt7621-sysc", "syscon"
> +
> + Refer to the bindings described in
> + Documentation/devicetree/bindings/mfd/syscon.yaml
> +
> +properties:
> + compatible:
> + const: mediatek,mt7621-clk
> +
> + "#clock-cells":
> + description:
> + The first cell indicates the clock gate number, see [1] for available
> + clocks.
> + const: 1
> +
> + clock-output-names:
> + maxItems: 8
> +
> +required:
> + - compatible
> + - '#clock-cells'
> + - clock-output-names
Why is clock-output-names required? Hopefully it is not required.
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/mt7621-clk.h>
> +
> + sysc: sysc@0 {
syscon@0? I don't think sysc is a standard node name.
> + compatible = "mediatek,mt7621-sysc", "syscon";
> + reg = <0x0 0x100>;
> +
> + pll {
clock-controller? Why can't the parent device be the clk provider and
have #clock-cells?
> + compatible = "mediatek,mt7621-clk";
> + #clock-cells = <1>;
> + clock-output-names = "xtal", "cpu", "bus",
> + "50m", "125m", "150m",
> + "250m", "270m";
> + };
> + };
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 2/6] dt: bindings: add mt7621-clk device tree binding documentation
@ 2020-12-17 8:58 ` Stephen Boyd
0 siblings, 0 replies; 36+ messages in thread
From: Stephen Boyd @ 2020-12-17 8:58 UTC (permalink / raw)
To: Sergio Paracuellos, mturquette
Cc: hackpascal, devel, tsbogend, gregkh, linux-kernel, evicetree,
linux-mips, robh+dt, john, neil, linux-clk
Quoting Sergio Paracuellos (2020-11-22 01:55:52)
> Adds device tree binding documentation for clocks in the
> MT7621 SOC.
>
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> ---
> .../bindings/clock/mediatek,mt7621-clk.yaml | 67 +++++++++++++++++++
> 1 file changed, 67 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
>
Rob?
> diff --git a/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml b/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
> new file mode 100644
> index 000000000000..6aca4c1a4a46
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
> @@ -0,0 +1,67 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/mediatek,mt7621-clk.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MT7621 Clock Device Tree Bindings
> +
> +maintainers:
> + - Sergio Paracuellos <sergio.paracuellos@gmail.com>
> +
> +description: |
> + The MT7621 has a PLL controller from where the cpu clock is provided
> + as well as derived clocks for the bus and the peripherals. It also
> + can gate SoC device clocks.
> +
> + Each clock is assigned an identifier and client nodes use this identifier
> + to specify the clock which they consume.
> +
> + All these identifiers could be found in:
> + [1]: <include/dt-bindings/clock/mt7621-clk.h>.
> +
> + The mt7621 clock node should be the child of a syscon node with the
> + required property:
> +
> + - compatible: Should be one of the following:
> + "mediatek,mt7621-sysc", "syscon"
> +
> + Refer to the bindings described in
> + Documentation/devicetree/bindings/mfd/syscon.yaml
> +
> +properties:
> + compatible:
> + const: mediatek,mt7621-clk
> +
> + "#clock-cells":
> + description:
> + The first cell indicates the clock gate number, see [1] for available
> + clocks.
> + const: 1
> +
> + clock-output-names:
> + maxItems: 8
> +
> +required:
> + - compatible
> + - '#clock-cells'
> + - clock-output-names
Why is clock-output-names required? Hopefully it is not required.
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/mt7621-clk.h>
> +
> + sysc: sysc@0 {
syscon@0? I don't think sysc is a standard node name.
> + compatible = "mediatek,mt7621-sysc", "syscon";
> + reg = <0x0 0x100>;
> +
> + pll {
clock-controller? Why can't the parent device be the clk provider and
have #clock-cells?
> + compatible = "mediatek,mt7621-clk";
> + #clock-cells = <1>;
> + clock-output-names = "xtal", "cpu", "bus",
> + "50m", "125m", "150m",
> + "250m", "270m";
> + };
> + };
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 3/6] clk: ralink: add clock driver for mt7621 SoC
2020-11-22 9:55 ` Sergio Paracuellos
@ 2020-12-17 9:09 ` Stephen Boyd
-1 siblings, 0 replies; 36+ messages in thread
From: Stephen Boyd @ 2020-12-17 9:09 UTC (permalink / raw)
To: Sergio Paracuellos, mturquette
Cc: robh+dt, john, tsbogend, gregkh, gch981213, hackpascal,
linux-clk, evicetree, linux-kernel, linux-mips, devel, neil
Quoting Sergio Paracuellos (2020-11-22 01:55:53)
> 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 frequences which are used in some drivers (uart, sd...).
s/frequences/frequencies/
> 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 | 14 +
> drivers/clk/ralink/Makefile | 2 +
> drivers/clk/ralink/clk-mt7621.c | 435 ++++++++++++++++++++++++++++++++
> 5 files changed, 453 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 c715d4681a0b..5f94c4329033 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -372,6 +372,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 da8fcf147eb1..6578e167b047 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -100,6 +100,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/
Thanks for keeping it sorted!
> diff --git a/drivers/clk/ralink/Kconfig b/drivers/clk/ralink/Kconfig
> new file mode 100644
> index 000000000000..7e8697327e0c
> --- /dev/null
> +++ b/drivers/clk/ralink/Kconfig
> @@ -0,0 +1,14 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# MediaTek Mt7621 Clock Driver
> +#
> +menu "Clock driver for mediatek mt7621 SoC"
Capitalize MediaTek?
> + depends on SOC_MT7621 || COMPILE_TEST
> +
> +config CLK_MT7621
> + bool "Clock driver for MediaTek MT7621"
Like it is done here.
> + depends on SOC_MT7621 || COMPILE_TEST
> + default SOC_MT7621
> + 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..4e929f13fe7c
> --- /dev/null
> +++ b/drivers/clk/ralink/clk-mt7621.c
> @@ -0,0 +1,435 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Mediatek MT7621 Clock Driver
> + * Author: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/clk-provider.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/regmap.h>
> +#include <asm/mach-ralink/ralink_regs.h>
Is it possible to drop this include? Doing so would make this portable
and compilable on more architectures so us cross compilers can check
build stuff and make changes easily.
> +#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 0x7
> +#define XTAL_MODE_SEL_SHIFT 6
> +
> +#define CPU_CLK_SEL_MASK 0x3
> +#define CPU_CLK_SEL_SHIFT 30
> +
> +#define CUR_CPU_FDIV_MASK 0x1f
> +#define CUR_CPU_FDIV_SHIFT 8
> +#define CUR_CPU_FFRAC_MASK 0x1f
> +#define CUR_CPU_FFRAC_SHIFT 0
> +
> +#define CPU_PLL_PREDIV_MASK 0x3
> +#define CPU_PLL_PREDIV_SHIFT 12
> +#define CPU_PLL_FBDIV_MASK 0x7f
> +#define CPU_PLL_FBDIV_SHIFT 4
> +
> +#define MHZ(x) ((x) * 1000 * 1000)
I'd rather we just wrote it out in Hz and dropped this macro.
> +
> +struct mt7621_clk_provider {
> + struct device_node *node;
> + struct regmap *syscon_regmap;
> + struct clk_hw_onecell_data *clk_data;
> +};
> +
> +struct mt7621_clk {
> + struct mt7621_clk_provider *clk_prov;
> + struct clk_hw hw;
> +};
> +
> +struct mt7621_fixed_clk {
> + u8 idx;
> + const char *name;
> + const char *parent_name;
> + struct mt7621_clk_provider *clk_prov;
> + unsigned long rate;
> + struct clk_hw *hw;
> +};
> +
> +struct mt7621_gate {
> + u8 idx;
> + const char *name;
> + const char *parent_name;
> + struct mt7621_clk_provider *clk_prov;
> + u32 bit_idx;
> + struct clk_hw hw;
> +};
> +
> +#define GATE(_id, _name, _pname, _shift) \
> + { \
> + .idx = _id, \
> + .name = _name, \
> + .parent_name = _pname, \
> + .clk_prov = NULL, \
> + .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 *scon = clk_gate->clk_prov->syscon_regmap;
> +
> + return regmap_update_bits(scon, 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 *scon = clk_gate->clk_prov->syscon_regmap;
> +
> + regmap_update_bits(scon, 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 *scon = clk_gate->clk_prov->syscon_regmap;
> + unsigned int val;
> +
> + if (regmap_read(scon, SYSC_REG_CLKCFG1, &val))
> + return 0;
> +
> + return val & 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_node *np,
> + struct mt7621_gate *sclk)
> +{
> + struct clk_init_data init = {
> + .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
Why ignore unused? Are they CLK_IS_CRITICAL? Can they be enabled at
driver probe instead of here? Or left out of the kernel entirely if they
shouldn't be turned off?
> + .num_parents = 1,
> + .parent_names = &sclk->parent_name,
> + .ops = &mt7621_gate_ops,
> + .name = sclk->name,
> + };
> +
> + sclk->hw.init = &init;
> + return of_clk_hw_register(np, &sclk->hw);
> +}
> +
> +static int mt7621_register_gates(struct mt7621_clk_provider *clk_prov)
> +{
> + struct clk_hw_onecell_data **clk_data = &clk_prov->clk_data;
Why do we take a pointer of a pointer.
> + struct clk_hw **hws = (*clk_data)->hws;
And then deref that pointer?
> + int ret, i;
> +
> + for (i = 0; i < ARRAY_SIZE(mt7621_gates); i++) {
> + struct mt7621_gate *sclk = &mt7621_gates[i];
> +
> + sclk->clk_prov = clk_prov;
> + ret = mt7621_gate_ops_init(clk_prov->node, sclk);
> + if (ret) {
> + pr_err("Couldn't register clock %s\n", sclk->name);
> + goto err_clk_unreg;
> + }
> +
> + hws[sclk->idx] = &sclk->hw;
> + (*clk_data)->num++;
Just use the pointer?
> + }
> +
> + 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, _pname, _rate) \
> + { \
> + .idx = _id, \
> + .name = _name, \
> + .parent_name = _pname, \
> + .clk_prov = NULL, \
This can be dropped as NULL is the default.
> + .rate = _rate \
> + }
> +
> +static struct mt7621_fixed_clk mt7621_fixed_clks[] = {
> + FIXED(MT7621_CLK_50M, "50m", "xtal", MHZ(50)),
> + FIXED(MT7621_CLK_125M, "125m", "xtal", MHZ(125)),
> + FIXED(MT7621_CLK_150M, "150m", "xtal", MHZ(150)),
> + FIXED(MT7621_CLK_250M, "250m", "xtal", MHZ(250)),
> + FIXED(MT7621_CLK_270M, "270m", "xtal", MHZ(270)),
parent is always "xtal" so maybe just hardcode that?
> +};
> +
> +static int mt7621_register_fixed_clocks(struct mt7621_clk_provider *clk_prov)
> +{
> + struct clk_hw_onecell_data **clk_data = &clk_prov->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->clk_prov = clk_prov;
> + sclk->hw = clk_hw_register_fixed_rate(NULL, sclk->name,
> + sclk->parent_name, 0,
> + sclk->rate);
> + if (IS_ERR(sclk->hw)) {
> + pr_err("Couldn't register clock %s\n", sclk->name);
> + ret = PTR_ERR(sclk->hw);
> + goto err_clk_unreg;
> + }
> +
> + hws[sclk->idx] = sclk->hw;
> + (*clk_data)->num++;
> + }
> +
> + 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 *scon = clk->clk_prov->syscon_regmap;
> + u32 val;
> +
> + regmap_read(scon, SYSC_REG_SYSTEM_CONFIG0, &val);
> + val = (val >> XTAL_MODE_SEL_SHIFT) & XTAL_MODE_SEL_MASK;
> +
> + if (val <= 2)
> + return MHZ(20);
> + else if (val <= 5)
Replace with 'if' because it returns above.
> + return MHZ(40);
> +
> + return MHZ(25);
> +}
> +
> +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 *scon = clk->clk_prov->syscon_regmap;
> + u32 clkcfg, clk_sel, curclk, ffiv, ffrac;
> + u32 pll, prediv, fbdiv;
> + unsigned long cpu_clk;
> +
> + regmap_read(scon, SYSC_REG_CLKCFG0, &clkcfg);
> + clk_sel = (clkcfg >> CPU_CLK_SEL_SHIFT) & CPU_CLK_SEL_MASK;
> +
> + regmap_read(scon, SYSC_REG_CUR_CLK_STS, &curclk);
> + ffiv = (curclk >> CUR_CPU_FDIV_SHIFT) & CUR_CPU_FDIV_MASK;
> + ffrac = (curclk >> CUR_CPU_FFRAC_SHIFT) & CUR_CPU_FFRAC_MASK;
> +
> + switch (clk_sel) {
> + case 0:
> + cpu_clk = MHZ(500);
> + break;
> + case 1:
> + pll = rt_memc_r32(MEMC_REG_CPU_PLL);
> + fbdiv = (pll >> CPU_PLL_FBDIV_SHIFT) & CPU_PLL_FBDIV_MASK;
> + prediv = (pll >> CPU_PLL_PREDIV_SHIFT) & CPU_PLL_PREDIV_MASK;
> + 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_names = (const char *const[]) { _parent }, \
Please use clk_parent_data instead
> + .num_parents = _parent ? 1 : 0 \
> + }, \
> +}
> +
> +static struct mt7621_clk mt7621_clks_base[] = {
> + { NULL, CLK_BASE("xtal", NULL, mt7621_xtal_recalc_rate) },
> + { NULL, CLK_BASE("cpu", "xtal", mt7621_cpu_recalc_rate) },
> + { NULL, CLK_BASE("bus", "cpu", mt7621_bus_recalc_rate) },
> +};
> +
> +static int mt7621_register_top_clocks(struct mt7621_clk_provider *clk_prov)
> +{
> + struct clk_hw_onecell_data **clk_data = &clk_prov->clk_data;
> + struct clk_hw **hws = (*clk_data)->hws;
> + int ret, i;
> +
> + for (i = 0; i < ARRAY_SIZE(mt7621_clks_base); i++) {
> + struct mt7621_clk *sclk = &mt7621_clks_base[i];
> +
> + sclk->clk_prov = clk_prov;
> + ret = of_clk_hw_register(clk_prov->node, &sclk->hw);
> + if (ret) {
> + pr_err("Couldn't register top clock %i\n", i);
> + goto err_clk_unreg;
> + }
> +
> + hws[i] = &sclk->hw;
> + (*clk_data)->num++;
> + }
> +
> + return 0;
> +
> +err_clk_unreg:
> + while (--i >= 0) {
> + struct mt7621_clk *sclk = &mt7621_clks_base[i];
> +
> + clk_hw_unregister(&sclk->hw);
> + }
> + return ret;
> +}
> +
> +static void __init mt7621_clk_init(struct device_node *node)
> +{
> + struct mt7621_clk_provider *clk_prov;
> + struct clk_hw_onecell_data **clk_data;
> + int i, ret, count;
> +
> + clk_prov = kzalloc(sizeof(*clk_prov), GFP_KERNEL);
> + if (!clk_prov)
> + return;
> +
> + clk_prov->syscon_regmap = syscon_node_to_regmap(node->parent);
> + if (IS_ERR(clk_prov->syscon_regmap)) {
> + pr_err("Could not get syscon regmap\n");
> + goto free_clk_prov;
> + }
> +
> + clk_prov->node = node;
> +
> + clk_data = &clk_prov->clk_data;
> + 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_prov;
> +
> + ret = mt7621_register_top_clocks(clk_prov);
> + if (ret) {
> + pr_err("Couldn't register top clocks\n");
> + goto free_clk_data;
> + }
> +
> + ret = mt7621_register_fixed_clocks(clk_prov);
> + if (ret) {
> + pr_err("Couldn't register fixed clocks\n");
> + goto unreg_clk_top;
> + }
> +
> + ret = mt7621_register_gates(clk_prov);
> + if (ret) {
> + pr_err("Couldn't register fixed clock gates\n");
> + goto unreg_clk_fixed;
> + }
> +
> + ret = of_clk_add_hw_provider(node, of_clk_hw_onecell_get,
> + clk_prov->clk_data);
> + if (ret) {
> + pr_err("Couldn't add clk hw provider\n");
> + goto unreg_clk_gates;
> + }
> +
> + return;
> +
> +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);
> + }
> +
> +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_prov->clk_data);
> +
> +free_clk_prov:
> + kfree(clk_prov);
> +}
> +
> +CLK_OF_DECLARE(mt7621_clk, "mediatek,mt7621-clk", mt7621_clk_init);
Any reason to use this vs. a platform driver?
> +
> +MODULE_AUTHOR("Sergio Paracuellos <sergio.paracuellos@gmail.com>");
> +MODULE_DESCRIPTION("Mediatek Mt7621 clock driver");
> +MODULE_LICENSE("GPL v2");
It isn't a module, but it would be nice if it was one. If the Kconfig is
bool these shouldn't be here.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 3/6] clk: ralink: add clock driver for mt7621 SoC
@ 2020-12-17 9:09 ` Stephen Boyd
0 siblings, 0 replies; 36+ messages in thread
From: Stephen Boyd @ 2020-12-17 9:09 UTC (permalink / raw)
To: Sergio Paracuellos, mturquette
Cc: hackpascal, devel, tsbogend, gregkh, linux-kernel, evicetree,
linux-mips, robh+dt, john, neil, linux-clk
Quoting Sergio Paracuellos (2020-11-22 01:55:53)
> 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 frequences which are used in some drivers (uart, sd...).
s/frequences/frequencies/
> 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 | 14 +
> drivers/clk/ralink/Makefile | 2 +
> drivers/clk/ralink/clk-mt7621.c | 435 ++++++++++++++++++++++++++++++++
> 5 files changed, 453 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 c715d4681a0b..5f94c4329033 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -372,6 +372,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 da8fcf147eb1..6578e167b047 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -100,6 +100,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/
Thanks for keeping it sorted!
> diff --git a/drivers/clk/ralink/Kconfig b/drivers/clk/ralink/Kconfig
> new file mode 100644
> index 000000000000..7e8697327e0c
> --- /dev/null
> +++ b/drivers/clk/ralink/Kconfig
> @@ -0,0 +1,14 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# MediaTek Mt7621 Clock Driver
> +#
> +menu "Clock driver for mediatek mt7621 SoC"
Capitalize MediaTek?
> + depends on SOC_MT7621 || COMPILE_TEST
> +
> +config CLK_MT7621
> + bool "Clock driver for MediaTek MT7621"
Like it is done here.
> + depends on SOC_MT7621 || COMPILE_TEST
> + default SOC_MT7621
> + 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..4e929f13fe7c
> --- /dev/null
> +++ b/drivers/clk/ralink/clk-mt7621.c
> @@ -0,0 +1,435 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Mediatek MT7621 Clock Driver
> + * Author: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/clk-provider.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/regmap.h>
> +#include <asm/mach-ralink/ralink_regs.h>
Is it possible to drop this include? Doing so would make this portable
and compilable on more architectures so us cross compilers can check
build stuff and make changes easily.
> +#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 0x7
> +#define XTAL_MODE_SEL_SHIFT 6
> +
> +#define CPU_CLK_SEL_MASK 0x3
> +#define CPU_CLK_SEL_SHIFT 30
> +
> +#define CUR_CPU_FDIV_MASK 0x1f
> +#define CUR_CPU_FDIV_SHIFT 8
> +#define CUR_CPU_FFRAC_MASK 0x1f
> +#define CUR_CPU_FFRAC_SHIFT 0
> +
> +#define CPU_PLL_PREDIV_MASK 0x3
> +#define CPU_PLL_PREDIV_SHIFT 12
> +#define CPU_PLL_FBDIV_MASK 0x7f
> +#define CPU_PLL_FBDIV_SHIFT 4
> +
> +#define MHZ(x) ((x) * 1000 * 1000)
I'd rather we just wrote it out in Hz and dropped this macro.
> +
> +struct mt7621_clk_provider {
> + struct device_node *node;
> + struct regmap *syscon_regmap;
> + struct clk_hw_onecell_data *clk_data;
> +};
> +
> +struct mt7621_clk {
> + struct mt7621_clk_provider *clk_prov;
> + struct clk_hw hw;
> +};
> +
> +struct mt7621_fixed_clk {
> + u8 idx;
> + const char *name;
> + const char *parent_name;
> + struct mt7621_clk_provider *clk_prov;
> + unsigned long rate;
> + struct clk_hw *hw;
> +};
> +
> +struct mt7621_gate {
> + u8 idx;
> + const char *name;
> + const char *parent_name;
> + struct mt7621_clk_provider *clk_prov;
> + u32 bit_idx;
> + struct clk_hw hw;
> +};
> +
> +#define GATE(_id, _name, _pname, _shift) \
> + { \
> + .idx = _id, \
> + .name = _name, \
> + .parent_name = _pname, \
> + .clk_prov = NULL, \
> + .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 *scon = clk_gate->clk_prov->syscon_regmap;
> +
> + return regmap_update_bits(scon, 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 *scon = clk_gate->clk_prov->syscon_regmap;
> +
> + regmap_update_bits(scon, 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 *scon = clk_gate->clk_prov->syscon_regmap;
> + unsigned int val;
> +
> + if (regmap_read(scon, SYSC_REG_CLKCFG1, &val))
> + return 0;
> +
> + return val & 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_node *np,
> + struct mt7621_gate *sclk)
> +{
> + struct clk_init_data init = {
> + .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
Why ignore unused? Are they CLK_IS_CRITICAL? Can they be enabled at
driver probe instead of here? Or left out of the kernel entirely if they
shouldn't be turned off?
> + .num_parents = 1,
> + .parent_names = &sclk->parent_name,
> + .ops = &mt7621_gate_ops,
> + .name = sclk->name,
> + };
> +
> + sclk->hw.init = &init;
> + return of_clk_hw_register(np, &sclk->hw);
> +}
> +
> +static int mt7621_register_gates(struct mt7621_clk_provider *clk_prov)
> +{
> + struct clk_hw_onecell_data **clk_data = &clk_prov->clk_data;
Why do we take a pointer of a pointer.
> + struct clk_hw **hws = (*clk_data)->hws;
And then deref that pointer?
> + int ret, i;
> +
> + for (i = 0; i < ARRAY_SIZE(mt7621_gates); i++) {
> + struct mt7621_gate *sclk = &mt7621_gates[i];
> +
> + sclk->clk_prov = clk_prov;
> + ret = mt7621_gate_ops_init(clk_prov->node, sclk);
> + if (ret) {
> + pr_err("Couldn't register clock %s\n", sclk->name);
> + goto err_clk_unreg;
> + }
> +
> + hws[sclk->idx] = &sclk->hw;
> + (*clk_data)->num++;
Just use the pointer?
> + }
> +
> + 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, _pname, _rate) \
> + { \
> + .idx = _id, \
> + .name = _name, \
> + .parent_name = _pname, \
> + .clk_prov = NULL, \
This can be dropped as NULL is the default.
> + .rate = _rate \
> + }
> +
> +static struct mt7621_fixed_clk mt7621_fixed_clks[] = {
> + FIXED(MT7621_CLK_50M, "50m", "xtal", MHZ(50)),
> + FIXED(MT7621_CLK_125M, "125m", "xtal", MHZ(125)),
> + FIXED(MT7621_CLK_150M, "150m", "xtal", MHZ(150)),
> + FIXED(MT7621_CLK_250M, "250m", "xtal", MHZ(250)),
> + FIXED(MT7621_CLK_270M, "270m", "xtal", MHZ(270)),
parent is always "xtal" so maybe just hardcode that?
> +};
> +
> +static int mt7621_register_fixed_clocks(struct mt7621_clk_provider *clk_prov)
> +{
> + struct clk_hw_onecell_data **clk_data = &clk_prov->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->clk_prov = clk_prov;
> + sclk->hw = clk_hw_register_fixed_rate(NULL, sclk->name,
> + sclk->parent_name, 0,
> + sclk->rate);
> + if (IS_ERR(sclk->hw)) {
> + pr_err("Couldn't register clock %s\n", sclk->name);
> + ret = PTR_ERR(sclk->hw);
> + goto err_clk_unreg;
> + }
> +
> + hws[sclk->idx] = sclk->hw;
> + (*clk_data)->num++;
> + }
> +
> + 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 *scon = clk->clk_prov->syscon_regmap;
> + u32 val;
> +
> + regmap_read(scon, SYSC_REG_SYSTEM_CONFIG0, &val);
> + val = (val >> XTAL_MODE_SEL_SHIFT) & XTAL_MODE_SEL_MASK;
> +
> + if (val <= 2)
> + return MHZ(20);
> + else if (val <= 5)
Replace with 'if' because it returns above.
> + return MHZ(40);
> +
> + return MHZ(25);
> +}
> +
> +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 *scon = clk->clk_prov->syscon_regmap;
> + u32 clkcfg, clk_sel, curclk, ffiv, ffrac;
> + u32 pll, prediv, fbdiv;
> + unsigned long cpu_clk;
> +
> + regmap_read(scon, SYSC_REG_CLKCFG0, &clkcfg);
> + clk_sel = (clkcfg >> CPU_CLK_SEL_SHIFT) & CPU_CLK_SEL_MASK;
> +
> + regmap_read(scon, SYSC_REG_CUR_CLK_STS, &curclk);
> + ffiv = (curclk >> CUR_CPU_FDIV_SHIFT) & CUR_CPU_FDIV_MASK;
> + ffrac = (curclk >> CUR_CPU_FFRAC_SHIFT) & CUR_CPU_FFRAC_MASK;
> +
> + switch (clk_sel) {
> + case 0:
> + cpu_clk = MHZ(500);
> + break;
> + case 1:
> + pll = rt_memc_r32(MEMC_REG_CPU_PLL);
> + fbdiv = (pll >> CPU_PLL_FBDIV_SHIFT) & CPU_PLL_FBDIV_MASK;
> + prediv = (pll >> CPU_PLL_PREDIV_SHIFT) & CPU_PLL_PREDIV_MASK;
> + 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_names = (const char *const[]) { _parent }, \
Please use clk_parent_data instead
> + .num_parents = _parent ? 1 : 0 \
> + }, \
> +}
> +
> +static struct mt7621_clk mt7621_clks_base[] = {
> + { NULL, CLK_BASE("xtal", NULL, mt7621_xtal_recalc_rate) },
> + { NULL, CLK_BASE("cpu", "xtal", mt7621_cpu_recalc_rate) },
> + { NULL, CLK_BASE("bus", "cpu", mt7621_bus_recalc_rate) },
> +};
> +
> +static int mt7621_register_top_clocks(struct mt7621_clk_provider *clk_prov)
> +{
> + struct clk_hw_onecell_data **clk_data = &clk_prov->clk_data;
> + struct clk_hw **hws = (*clk_data)->hws;
> + int ret, i;
> +
> + for (i = 0; i < ARRAY_SIZE(mt7621_clks_base); i++) {
> + struct mt7621_clk *sclk = &mt7621_clks_base[i];
> +
> + sclk->clk_prov = clk_prov;
> + ret = of_clk_hw_register(clk_prov->node, &sclk->hw);
> + if (ret) {
> + pr_err("Couldn't register top clock %i\n", i);
> + goto err_clk_unreg;
> + }
> +
> + hws[i] = &sclk->hw;
> + (*clk_data)->num++;
> + }
> +
> + return 0;
> +
> +err_clk_unreg:
> + while (--i >= 0) {
> + struct mt7621_clk *sclk = &mt7621_clks_base[i];
> +
> + clk_hw_unregister(&sclk->hw);
> + }
> + return ret;
> +}
> +
> +static void __init mt7621_clk_init(struct device_node *node)
> +{
> + struct mt7621_clk_provider *clk_prov;
> + struct clk_hw_onecell_data **clk_data;
> + int i, ret, count;
> +
> + clk_prov = kzalloc(sizeof(*clk_prov), GFP_KERNEL);
> + if (!clk_prov)
> + return;
> +
> + clk_prov->syscon_regmap = syscon_node_to_regmap(node->parent);
> + if (IS_ERR(clk_prov->syscon_regmap)) {
> + pr_err("Could not get syscon regmap\n");
> + goto free_clk_prov;
> + }
> +
> + clk_prov->node = node;
> +
> + clk_data = &clk_prov->clk_data;
> + 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_prov;
> +
> + ret = mt7621_register_top_clocks(clk_prov);
> + if (ret) {
> + pr_err("Couldn't register top clocks\n");
> + goto free_clk_data;
> + }
> +
> + ret = mt7621_register_fixed_clocks(clk_prov);
> + if (ret) {
> + pr_err("Couldn't register fixed clocks\n");
> + goto unreg_clk_top;
> + }
> +
> + ret = mt7621_register_gates(clk_prov);
> + if (ret) {
> + pr_err("Couldn't register fixed clock gates\n");
> + goto unreg_clk_fixed;
> + }
> +
> + ret = of_clk_add_hw_provider(node, of_clk_hw_onecell_get,
> + clk_prov->clk_data);
> + if (ret) {
> + pr_err("Couldn't add clk hw provider\n");
> + goto unreg_clk_gates;
> + }
> +
> + return;
> +
> +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);
> + }
> +
> +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_prov->clk_data);
> +
> +free_clk_prov:
> + kfree(clk_prov);
> +}
> +
> +CLK_OF_DECLARE(mt7621_clk, "mediatek,mt7621-clk", mt7621_clk_init);
Any reason to use this vs. a platform driver?
> +
> +MODULE_AUTHOR("Sergio Paracuellos <sergio.paracuellos@gmail.com>");
> +MODULE_DESCRIPTION("Mediatek Mt7621 clock driver");
> +MODULE_LICENSE("GPL v2");
It isn't a module, but it would be nice if it was one. If the Kconfig is
bool these shouldn't be here.
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 3/6] clk: ralink: add clock driver for mt7621 SoC
2020-12-17 9:09 ` Stephen Boyd
@ 2020-12-17 9:54 ` Sergio Paracuellos
-1 siblings, 0 replies; 36+ messages in thread
From: Sergio Paracuellos @ 2020-12-17 9:54 UTC (permalink / raw)
To: Stephen Boyd
Cc: Michael Turquette, Rob Herring, John Crispin,
Thomas Bogendoerfer, Greg KH, Chuanhong Guo, Weijie Gao,
open list:COMMON CLK FRAMEWORK, evicetree, linux-kernel,
open list:MIPS, open list:STAGING SUBSYSTEM, NeilBrown
Hi Stephen,
Thanks for the review.
On Thu, Dec 17, 2020 at 10:09 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Sergio Paracuellos (2020-11-22 01:55:53)
> > 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 frequences which are used in some drivers (uart, sd...).
>
> s/frequences/frequencies/
Ok!
>
> > 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 | 14 +
> > drivers/clk/ralink/Makefile | 2 +
> > drivers/clk/ralink/clk-mt7621.c | 435 ++++++++++++++++++++++++++++++++
> > 5 files changed, 453 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 c715d4681a0b..5f94c4329033 100644
> > --- a/drivers/clk/Kconfig
> > +++ b/drivers/clk/Kconfig
> > @@ -372,6 +372,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 da8fcf147eb1..6578e167b047 100644
> > --- a/drivers/clk/Makefile
> > +++ b/drivers/clk/Makefile
> > @@ -100,6 +100,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/
>
> Thanks for keeping it sorted!
It was so clean sorted so I just followed the style there.
>
> > diff --git a/drivers/clk/ralink/Kconfig b/drivers/clk/ralink/Kconfig
> > new file mode 100644
> > index 000000000000..7e8697327e0c
> > --- /dev/null
> > +++ b/drivers/clk/ralink/Kconfig
> > @@ -0,0 +1,14 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +#
> > +# MediaTek Mt7621 Clock Driver
> > +#
> > +menu "Clock driver for mediatek mt7621 SoC"
>
> Capitalize MediaTek?
Will do.
>
> > + depends on SOC_MT7621 || COMPILE_TEST
> > +
> > +config CLK_MT7621
> > + bool "Clock driver for MediaTek MT7621"
>
> Like it is done here.
Yes, both should be properly capitalized.
>
> > + depends on SOC_MT7621 || COMPILE_TEST
> > + default SOC_MT7621
> > + 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..4e929f13fe7c
> > --- /dev/null
> > +++ b/drivers/clk/ralink/clk-mt7621.c
> > @@ -0,0 +1,435 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Mediatek MT7621 Clock Driver
> > + * Author: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > + */
> > +
> > +#include <linux/bitops.h>
> > +#include <linux/clk-provider.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/module.h>
> > +#include <linux/slab.h>
> > +#include <linux/regmap.h>
> > +#include <asm/mach-ralink/ralink_regs.h>
>
> Is it possible to drop this include? Doing so would make this portable
> and compilable on more architectures so us cross compilers can check
> build stuff and make changes easily.
No, this is not possible. This old arch makes some global functions
there to properly access different registers in the palmbus. It is not
also well documented so it is really difficult to make something
better with this.
This is needed to use 'rt_memc_r32'
(arch/mips/include/asm/mach-ralink/ralink_regs.h) for reading
MEMC_REG_CPU_PLL.
This is a not documented register and is not in the syscon related
part and we need it to derive the clock frequency for the XTAL clock.
>
> > +#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 0x7
> > +#define XTAL_MODE_SEL_SHIFT 6
> > +
> > +#define CPU_CLK_SEL_MASK 0x3
> > +#define CPU_CLK_SEL_SHIFT 30
> > +
> > +#define CUR_CPU_FDIV_MASK 0x1f
> > +#define CUR_CPU_FDIV_SHIFT 8
> > +#define CUR_CPU_FFRAC_MASK 0x1f
> > +#define CUR_CPU_FFRAC_SHIFT 0
> > +
> > +#define CPU_PLL_PREDIV_MASK 0x3
> > +#define CPU_PLL_PREDIV_SHIFT 12
> > +#define CPU_PLL_FBDIV_MASK 0x7f
> > +#define CPU_PLL_FBDIV_SHIFT 4
> > +
> > +#define MHZ(x) ((x) * 1000 * 1000)
>
> I'd rather we just wrote it out in Hz and dropped this macro.
Ok, will do.
>
> > +
> > +struct mt7621_clk_provider {
> > + struct device_node *node;
> > + struct regmap *syscon_regmap;
> > + struct clk_hw_onecell_data *clk_data;
> > +};
> > +
> > +struct mt7621_clk {
> > + struct mt7621_clk_provider *clk_prov;
> > + struct clk_hw hw;
> > +};
> > +
> > +struct mt7621_fixed_clk {
> > + u8 idx;
> > + const char *name;
> > + const char *parent_name;
> > + struct mt7621_clk_provider *clk_prov;
> > + unsigned long rate;
> > + struct clk_hw *hw;
> > +};
> > +
> > +struct mt7621_gate {
> > + u8 idx;
> > + const char *name;
> > + const char *parent_name;
> > + struct mt7621_clk_provider *clk_prov;
> > + u32 bit_idx;
> > + struct clk_hw hw;
> > +};
> > +
> > +#define GATE(_id, _name, _pname, _shift) \
> > + { \
> > + .idx = _id, \
> > + .name = _name, \
> > + .parent_name = _pname, \
> > + .clk_prov = NULL, \
> > + .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 *scon = clk_gate->clk_prov->syscon_regmap;
> > +
> > + return regmap_update_bits(scon, 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 *scon = clk_gate->clk_prov->syscon_regmap;
> > +
> > + regmap_update_bits(scon, 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 *scon = clk_gate->clk_prov->syscon_regmap;
> > + unsigned int val;
> > +
> > + if (regmap_read(scon, SYSC_REG_CLKCFG1, &val))
> > + return 0;
> > +
> > + return val & 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_node *np,
> > + struct mt7621_gate *sclk)
> > +{
> > + struct clk_init_data init = {
> > + .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
>
> Why ignore unused? Are they CLK_IS_CRITICAL? Can they be enabled at
> driver probe instead of here? Or left out of the kernel entirely if they
> shouldn't be turned off?
Because all the platform drivers are not changed to use this gates yet
and all gates are enabled by default (related registers are set to all
ones), kernel disables all the stuff because they are not being
referenced, but yes, you are right, I think I can call
clk_prepare_enable for all of them at init time and avoid this
'CLK_IGNORE_UNUSED' flag to don't break anything of the current other
upstream code.
>
> > + .num_parents = 1,
> > + .parent_names = &sclk->parent_name,
> > + .ops = &mt7621_gate_ops,
> > + .name = sclk->name,
> > + };
> > +
> > + sclk->hw.init = &init;
> > + return of_clk_hw_register(np, &sclk->hw);
> > +}
> > +
> > +static int mt7621_register_gates(struct mt7621_clk_provider *clk_prov)
> > +{
> > + struct clk_hw_onecell_data **clk_data = &clk_prov->clk_data;
>
> Why do we take a pointer of a pointer.
Yep, totally true, this double pointer is not really needed here :).
>
> > + struct clk_hw **hws = (*clk_data)->hws;
>
> And then deref that pointer?
Yes, I will change to use clk_data as a normal pointer and not a double one.
>
> > + int ret, i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(mt7621_gates); i++) {
> > + struct mt7621_gate *sclk = &mt7621_gates[i];
> > +
> > + sclk->clk_prov = clk_prov;
> > + ret = mt7621_gate_ops_init(clk_prov->node, sclk);
> > + if (ret) {
> > + pr_err("Couldn't register clock %s\n", sclk->name);
> > + goto err_clk_unreg;
> > + }
> > +
> > + hws[sclk->idx] = &sclk->hw;
> > + (*clk_data)->num++;
>
> Just use the pointer?
Will be changed.
>
> > + }
> > +
> > + 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, _pname, _rate) \
> > + { \
> > + .idx = _id, \
> > + .name = _name, \
> > + .parent_name = _pname, \
> > + .clk_prov = NULL, \
>
> This can be dropped as NULL is the default.
>
> > + .rate = _rate \
> > + }
> > +
> > +static struct mt7621_fixed_clk mt7621_fixed_clks[] = {
> > + FIXED(MT7621_CLK_50M, "50m", "xtal", MHZ(50)),
> > + FIXED(MT7621_CLK_125M, "125m", "xtal", MHZ(125)),
> > + FIXED(MT7621_CLK_150M, "150m", "xtal", MHZ(150)),
> > + FIXED(MT7621_CLK_250M, "250m", "xtal", MHZ(250)),
> > + FIXED(MT7621_CLK_270M, "270m", "xtal", MHZ(270)),
>
> parent is always "xtal" so maybe just hardcode that?
True, will hardcode this in the macro then.
>
> > +};
> > +
> > +static int mt7621_register_fixed_clocks(struct mt7621_clk_provider *clk_prov)
> > +{
> > + struct clk_hw_onecell_data **clk_data = &clk_prov->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->clk_prov = clk_prov;
> > + sclk->hw = clk_hw_register_fixed_rate(NULL, sclk->name,
> > + sclk->parent_name, 0,
> > + sclk->rate);
> > + if (IS_ERR(sclk->hw)) {
> > + pr_err("Couldn't register clock %s\n", sclk->name);
> > + ret = PTR_ERR(sclk->hw);
> > + goto err_clk_unreg;
> > + }
> > +
> > + hws[sclk->idx] = sclk->hw;
> > + (*clk_data)->num++;
> > + }
> > +
> > + 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 *scon = clk->clk_prov->syscon_regmap;
> > + u32 val;
> > +
> > + regmap_read(scon, SYSC_REG_SYSTEM_CONFIG0, &val);
> > + val = (val >> XTAL_MODE_SEL_SHIFT) & XTAL_MODE_SEL_MASK;
> > +
> > + if (val <= 2)
> > + return MHZ(20);
> > + else if (val <= 5)
>
> Replace with 'if' because it returns above.
True, good catch.
>
> > + return MHZ(40);
> > +
> > + return MHZ(25);
> > +}
> > +
> > +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 *scon = clk->clk_prov->syscon_regmap;
> > + u32 clkcfg, clk_sel, curclk, ffiv, ffrac;
> > + u32 pll, prediv, fbdiv;
> > + unsigned long cpu_clk;
> > +
> > + regmap_read(scon, SYSC_REG_CLKCFG0, &clkcfg);
> > + clk_sel = (clkcfg >> CPU_CLK_SEL_SHIFT) & CPU_CLK_SEL_MASK;
> > +
> > + regmap_read(scon, SYSC_REG_CUR_CLK_STS, &curclk);
> > + ffiv = (curclk >> CUR_CPU_FDIV_SHIFT) & CUR_CPU_FDIV_MASK;
> > + ffrac = (curclk >> CUR_CPU_FFRAC_SHIFT) & CUR_CPU_FFRAC_MASK;
> > +
> > + switch (clk_sel) {
> > + case 0:
> > + cpu_clk = MHZ(500);
> > + break;
> > + case 1:
> > + pll = rt_memc_r32(MEMC_REG_CPU_PLL);
> > + fbdiv = (pll >> CPU_PLL_FBDIV_SHIFT) & CPU_PLL_FBDIV_MASK;
> > + prediv = (pll >> CPU_PLL_PREDIV_SHIFT) & CPU_PLL_PREDIV_MASK;
> > + 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_names = (const char *const[]) { _parent }, \
>
> Please use clk_parent_data instead
parent can also be NULL here and num_parents zero, but I will search
what do you really mean with this 'clk_parent_data' :).
>
> > + .num_parents = _parent ? 1 : 0 \
> > + }, \
> > +}
> > +
> > +static struct mt7621_clk mt7621_clks_base[] = {
> > + { NULL, CLK_BASE("xtal", NULL, mt7621_xtal_recalc_rate) },
> > + { NULL, CLK_BASE("cpu", "xtal", mt7621_cpu_recalc_rate) },
> > + { NULL, CLK_BASE("bus", "cpu", mt7621_bus_recalc_rate) },
> > +};
> > +
> > +static int mt7621_register_top_clocks(struct mt7621_clk_provider *clk_prov)
> > +{
> > + struct clk_hw_onecell_data **clk_data = &clk_prov->clk_data;
> > + struct clk_hw **hws = (*clk_data)->hws;
> > + int ret, i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(mt7621_clks_base); i++) {
> > + struct mt7621_clk *sclk = &mt7621_clks_base[i];
> > +
> > + sclk->clk_prov = clk_prov;
> > + ret = of_clk_hw_register(clk_prov->node, &sclk->hw);
> > + if (ret) {
> > + pr_err("Couldn't register top clock %i\n", i);
> > + goto err_clk_unreg;
> > + }
> > +
> > + hws[i] = &sclk->hw;
> > + (*clk_data)->num++;
> > + }
> > +
> > + return 0;
> > +
> > +err_clk_unreg:
> > + while (--i >= 0) {
> > + struct mt7621_clk *sclk = &mt7621_clks_base[i];
> > +
> > + clk_hw_unregister(&sclk->hw);
> > + }
> > + return ret;
> > +}
> > +
> > +static void __init mt7621_clk_init(struct device_node *node)
> > +{
> > + struct mt7621_clk_provider *clk_prov;
> > + struct clk_hw_onecell_data **clk_data;
> > + int i, ret, count;
> > +
> > + clk_prov = kzalloc(sizeof(*clk_prov), GFP_KERNEL);
> > + if (!clk_prov)
> > + return;
> > +
> > + clk_prov->syscon_regmap = syscon_node_to_regmap(node->parent);
> > + if (IS_ERR(clk_prov->syscon_regmap)) {
> > + pr_err("Could not get syscon regmap\n");
> > + goto free_clk_prov;
> > + }
> > +
> > + clk_prov->node = node;
> > +
> > + clk_data = &clk_prov->clk_data;
> > + 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_prov;
> > +
> > + ret = mt7621_register_top_clocks(clk_prov);
> > + if (ret) {
> > + pr_err("Couldn't register top clocks\n");
> > + goto free_clk_data;
> > + }
> > +
> > + ret = mt7621_register_fixed_clocks(clk_prov);
> > + if (ret) {
> > + pr_err("Couldn't register fixed clocks\n");
> > + goto unreg_clk_top;
> > + }
> > +
> > + ret = mt7621_register_gates(clk_prov);
> > + if (ret) {
> > + pr_err("Couldn't register fixed clock gates\n");
> > + goto unreg_clk_fixed;
> > + }
> > +
> > + ret = of_clk_add_hw_provider(node, of_clk_hw_onecell_get,
> > + clk_prov->clk_data);
> > + if (ret) {
> > + pr_err("Couldn't add clk hw provider\n");
> > + goto unreg_clk_gates;
> > + }
> > +
> > + return;
> > +
> > +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);
> > + }
> > +
> > +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_prov->clk_data);
> > +
> > +free_clk_prov:
> > + kfree(clk_prov);
> > +}
> > +
> > +CLK_OF_DECLARE(mt7621_clk, "mediatek,mt7621-clk", mt7621_clk_init);
>
> Any reason to use this vs. a platform driver?
We need clocks available in 'plat_time_init' before setting up the
timer for the GIC, so to maintain all the clock driver in a simple
file and using only one device tree node and no separate the gates
into another platform driver, I think this is the only way to go, but
please correct me if I am wrong.
> > +
> > +MODULE_AUTHOR("Sergio Paracuellos <sergio.paracuellos@gmail.com>");
> > +MODULE_DESCRIPTION("Mediatek Mt7621 clock driver");
> > +MODULE_LICENSE("GPL v2");
>
> It isn't a module, but it would be nice if it was one. If the Kconfig is
> bool these shouldn't be here.
Ok, totally agree I will remove these.
Thanks again for your review. I will ger rid of all the stuff here and send v5.
Best regards,
Sergio Paracuellos
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 3/6] clk: ralink: add clock driver for mt7621 SoC
@ 2020-12-17 9:54 ` Sergio Paracuellos
0 siblings, 0 replies; 36+ messages in thread
From: Sergio Paracuellos @ 2020-12-17 9:54 UTC (permalink / raw)
To: Stephen Boyd
Cc: Weijie Gao, open list:STAGING SUBSYSTEM, Thomas Bogendoerfer,
Greg KH, Michael Turquette, linux-kernel, evicetree,
open list:MIPS, Rob Herring, John Crispin, NeilBrown,
open list:COMMON CLK FRAMEWORK
Hi Stephen,
Thanks for the review.
On Thu, Dec 17, 2020 at 10:09 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Sergio Paracuellos (2020-11-22 01:55:53)
> > 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 frequences which are used in some drivers (uart, sd...).
>
> s/frequences/frequencies/
Ok!
>
> > 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 | 14 +
> > drivers/clk/ralink/Makefile | 2 +
> > drivers/clk/ralink/clk-mt7621.c | 435 ++++++++++++++++++++++++++++++++
> > 5 files changed, 453 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 c715d4681a0b..5f94c4329033 100644
> > --- a/drivers/clk/Kconfig
> > +++ b/drivers/clk/Kconfig
> > @@ -372,6 +372,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 da8fcf147eb1..6578e167b047 100644
> > --- a/drivers/clk/Makefile
> > +++ b/drivers/clk/Makefile
> > @@ -100,6 +100,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/
>
> Thanks for keeping it sorted!
It was so clean sorted so I just followed the style there.
>
> > diff --git a/drivers/clk/ralink/Kconfig b/drivers/clk/ralink/Kconfig
> > new file mode 100644
> > index 000000000000..7e8697327e0c
> > --- /dev/null
> > +++ b/drivers/clk/ralink/Kconfig
> > @@ -0,0 +1,14 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +#
> > +# MediaTek Mt7621 Clock Driver
> > +#
> > +menu "Clock driver for mediatek mt7621 SoC"
>
> Capitalize MediaTek?
Will do.
>
> > + depends on SOC_MT7621 || COMPILE_TEST
> > +
> > +config CLK_MT7621
> > + bool "Clock driver for MediaTek MT7621"
>
> Like it is done here.
Yes, both should be properly capitalized.
>
> > + depends on SOC_MT7621 || COMPILE_TEST
> > + default SOC_MT7621
> > + 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..4e929f13fe7c
> > --- /dev/null
> > +++ b/drivers/clk/ralink/clk-mt7621.c
> > @@ -0,0 +1,435 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Mediatek MT7621 Clock Driver
> > + * Author: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > + */
> > +
> > +#include <linux/bitops.h>
> > +#include <linux/clk-provider.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/module.h>
> > +#include <linux/slab.h>
> > +#include <linux/regmap.h>
> > +#include <asm/mach-ralink/ralink_regs.h>
>
> Is it possible to drop this include? Doing so would make this portable
> and compilable on more architectures so us cross compilers can check
> build stuff and make changes easily.
No, this is not possible. This old arch makes some global functions
there to properly access different registers in the palmbus. It is not
also well documented so it is really difficult to make something
better with this.
This is needed to use 'rt_memc_r32'
(arch/mips/include/asm/mach-ralink/ralink_regs.h) for reading
MEMC_REG_CPU_PLL.
This is a not documented register and is not in the syscon related
part and we need it to derive the clock frequency for the XTAL clock.
>
> > +#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 0x7
> > +#define XTAL_MODE_SEL_SHIFT 6
> > +
> > +#define CPU_CLK_SEL_MASK 0x3
> > +#define CPU_CLK_SEL_SHIFT 30
> > +
> > +#define CUR_CPU_FDIV_MASK 0x1f
> > +#define CUR_CPU_FDIV_SHIFT 8
> > +#define CUR_CPU_FFRAC_MASK 0x1f
> > +#define CUR_CPU_FFRAC_SHIFT 0
> > +
> > +#define CPU_PLL_PREDIV_MASK 0x3
> > +#define CPU_PLL_PREDIV_SHIFT 12
> > +#define CPU_PLL_FBDIV_MASK 0x7f
> > +#define CPU_PLL_FBDIV_SHIFT 4
> > +
> > +#define MHZ(x) ((x) * 1000 * 1000)
>
> I'd rather we just wrote it out in Hz and dropped this macro.
Ok, will do.
>
> > +
> > +struct mt7621_clk_provider {
> > + struct device_node *node;
> > + struct regmap *syscon_regmap;
> > + struct clk_hw_onecell_data *clk_data;
> > +};
> > +
> > +struct mt7621_clk {
> > + struct mt7621_clk_provider *clk_prov;
> > + struct clk_hw hw;
> > +};
> > +
> > +struct mt7621_fixed_clk {
> > + u8 idx;
> > + const char *name;
> > + const char *parent_name;
> > + struct mt7621_clk_provider *clk_prov;
> > + unsigned long rate;
> > + struct clk_hw *hw;
> > +};
> > +
> > +struct mt7621_gate {
> > + u8 idx;
> > + const char *name;
> > + const char *parent_name;
> > + struct mt7621_clk_provider *clk_prov;
> > + u32 bit_idx;
> > + struct clk_hw hw;
> > +};
> > +
> > +#define GATE(_id, _name, _pname, _shift) \
> > + { \
> > + .idx = _id, \
> > + .name = _name, \
> > + .parent_name = _pname, \
> > + .clk_prov = NULL, \
> > + .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 *scon = clk_gate->clk_prov->syscon_regmap;
> > +
> > + return regmap_update_bits(scon, 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 *scon = clk_gate->clk_prov->syscon_regmap;
> > +
> > + regmap_update_bits(scon, 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 *scon = clk_gate->clk_prov->syscon_regmap;
> > + unsigned int val;
> > +
> > + if (regmap_read(scon, SYSC_REG_CLKCFG1, &val))
> > + return 0;
> > +
> > + return val & 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_node *np,
> > + struct mt7621_gate *sclk)
> > +{
> > + struct clk_init_data init = {
> > + .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
>
> Why ignore unused? Are they CLK_IS_CRITICAL? Can they be enabled at
> driver probe instead of here? Or left out of the kernel entirely if they
> shouldn't be turned off?
Because all the platform drivers are not changed to use this gates yet
and all gates are enabled by default (related registers are set to all
ones), kernel disables all the stuff because they are not being
referenced, but yes, you are right, I think I can call
clk_prepare_enable for all of them at init time and avoid this
'CLK_IGNORE_UNUSED' flag to don't break anything of the current other
upstream code.
>
> > + .num_parents = 1,
> > + .parent_names = &sclk->parent_name,
> > + .ops = &mt7621_gate_ops,
> > + .name = sclk->name,
> > + };
> > +
> > + sclk->hw.init = &init;
> > + return of_clk_hw_register(np, &sclk->hw);
> > +}
> > +
> > +static int mt7621_register_gates(struct mt7621_clk_provider *clk_prov)
> > +{
> > + struct clk_hw_onecell_data **clk_data = &clk_prov->clk_data;
>
> Why do we take a pointer of a pointer.
Yep, totally true, this double pointer is not really needed here :).
>
> > + struct clk_hw **hws = (*clk_data)->hws;
>
> And then deref that pointer?
Yes, I will change to use clk_data as a normal pointer and not a double one.
>
> > + int ret, i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(mt7621_gates); i++) {
> > + struct mt7621_gate *sclk = &mt7621_gates[i];
> > +
> > + sclk->clk_prov = clk_prov;
> > + ret = mt7621_gate_ops_init(clk_prov->node, sclk);
> > + if (ret) {
> > + pr_err("Couldn't register clock %s\n", sclk->name);
> > + goto err_clk_unreg;
> > + }
> > +
> > + hws[sclk->idx] = &sclk->hw;
> > + (*clk_data)->num++;
>
> Just use the pointer?
Will be changed.
>
> > + }
> > +
> > + 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, _pname, _rate) \
> > + { \
> > + .idx = _id, \
> > + .name = _name, \
> > + .parent_name = _pname, \
> > + .clk_prov = NULL, \
>
> This can be dropped as NULL is the default.
>
> > + .rate = _rate \
> > + }
> > +
> > +static struct mt7621_fixed_clk mt7621_fixed_clks[] = {
> > + FIXED(MT7621_CLK_50M, "50m", "xtal", MHZ(50)),
> > + FIXED(MT7621_CLK_125M, "125m", "xtal", MHZ(125)),
> > + FIXED(MT7621_CLK_150M, "150m", "xtal", MHZ(150)),
> > + FIXED(MT7621_CLK_250M, "250m", "xtal", MHZ(250)),
> > + FIXED(MT7621_CLK_270M, "270m", "xtal", MHZ(270)),
>
> parent is always "xtal" so maybe just hardcode that?
True, will hardcode this in the macro then.
>
> > +};
> > +
> > +static int mt7621_register_fixed_clocks(struct mt7621_clk_provider *clk_prov)
> > +{
> > + struct clk_hw_onecell_data **clk_data = &clk_prov->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->clk_prov = clk_prov;
> > + sclk->hw = clk_hw_register_fixed_rate(NULL, sclk->name,
> > + sclk->parent_name, 0,
> > + sclk->rate);
> > + if (IS_ERR(sclk->hw)) {
> > + pr_err("Couldn't register clock %s\n", sclk->name);
> > + ret = PTR_ERR(sclk->hw);
> > + goto err_clk_unreg;
> > + }
> > +
> > + hws[sclk->idx] = sclk->hw;
> > + (*clk_data)->num++;
> > + }
> > +
> > + 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 *scon = clk->clk_prov->syscon_regmap;
> > + u32 val;
> > +
> > + regmap_read(scon, SYSC_REG_SYSTEM_CONFIG0, &val);
> > + val = (val >> XTAL_MODE_SEL_SHIFT) & XTAL_MODE_SEL_MASK;
> > +
> > + if (val <= 2)
> > + return MHZ(20);
> > + else if (val <= 5)
>
> Replace with 'if' because it returns above.
True, good catch.
>
> > + return MHZ(40);
> > +
> > + return MHZ(25);
> > +}
> > +
> > +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 *scon = clk->clk_prov->syscon_regmap;
> > + u32 clkcfg, clk_sel, curclk, ffiv, ffrac;
> > + u32 pll, prediv, fbdiv;
> > + unsigned long cpu_clk;
> > +
> > + regmap_read(scon, SYSC_REG_CLKCFG0, &clkcfg);
> > + clk_sel = (clkcfg >> CPU_CLK_SEL_SHIFT) & CPU_CLK_SEL_MASK;
> > +
> > + regmap_read(scon, SYSC_REG_CUR_CLK_STS, &curclk);
> > + ffiv = (curclk >> CUR_CPU_FDIV_SHIFT) & CUR_CPU_FDIV_MASK;
> > + ffrac = (curclk >> CUR_CPU_FFRAC_SHIFT) & CUR_CPU_FFRAC_MASK;
> > +
> > + switch (clk_sel) {
> > + case 0:
> > + cpu_clk = MHZ(500);
> > + break;
> > + case 1:
> > + pll = rt_memc_r32(MEMC_REG_CPU_PLL);
> > + fbdiv = (pll >> CPU_PLL_FBDIV_SHIFT) & CPU_PLL_FBDIV_MASK;
> > + prediv = (pll >> CPU_PLL_PREDIV_SHIFT) & CPU_PLL_PREDIV_MASK;
> > + 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_names = (const char *const[]) { _parent }, \
>
> Please use clk_parent_data instead
parent can also be NULL here and num_parents zero, but I will search
what do you really mean with this 'clk_parent_data' :).
>
> > + .num_parents = _parent ? 1 : 0 \
> > + }, \
> > +}
> > +
> > +static struct mt7621_clk mt7621_clks_base[] = {
> > + { NULL, CLK_BASE("xtal", NULL, mt7621_xtal_recalc_rate) },
> > + { NULL, CLK_BASE("cpu", "xtal", mt7621_cpu_recalc_rate) },
> > + { NULL, CLK_BASE("bus", "cpu", mt7621_bus_recalc_rate) },
> > +};
> > +
> > +static int mt7621_register_top_clocks(struct mt7621_clk_provider *clk_prov)
> > +{
> > + struct clk_hw_onecell_data **clk_data = &clk_prov->clk_data;
> > + struct clk_hw **hws = (*clk_data)->hws;
> > + int ret, i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(mt7621_clks_base); i++) {
> > + struct mt7621_clk *sclk = &mt7621_clks_base[i];
> > +
> > + sclk->clk_prov = clk_prov;
> > + ret = of_clk_hw_register(clk_prov->node, &sclk->hw);
> > + if (ret) {
> > + pr_err("Couldn't register top clock %i\n", i);
> > + goto err_clk_unreg;
> > + }
> > +
> > + hws[i] = &sclk->hw;
> > + (*clk_data)->num++;
> > + }
> > +
> > + return 0;
> > +
> > +err_clk_unreg:
> > + while (--i >= 0) {
> > + struct mt7621_clk *sclk = &mt7621_clks_base[i];
> > +
> > + clk_hw_unregister(&sclk->hw);
> > + }
> > + return ret;
> > +}
> > +
> > +static void __init mt7621_clk_init(struct device_node *node)
> > +{
> > + struct mt7621_clk_provider *clk_prov;
> > + struct clk_hw_onecell_data **clk_data;
> > + int i, ret, count;
> > +
> > + clk_prov = kzalloc(sizeof(*clk_prov), GFP_KERNEL);
> > + if (!clk_prov)
> > + return;
> > +
> > + clk_prov->syscon_regmap = syscon_node_to_regmap(node->parent);
> > + if (IS_ERR(clk_prov->syscon_regmap)) {
> > + pr_err("Could not get syscon regmap\n");
> > + goto free_clk_prov;
> > + }
> > +
> > + clk_prov->node = node;
> > +
> > + clk_data = &clk_prov->clk_data;
> > + 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_prov;
> > +
> > + ret = mt7621_register_top_clocks(clk_prov);
> > + if (ret) {
> > + pr_err("Couldn't register top clocks\n");
> > + goto free_clk_data;
> > + }
> > +
> > + ret = mt7621_register_fixed_clocks(clk_prov);
> > + if (ret) {
> > + pr_err("Couldn't register fixed clocks\n");
> > + goto unreg_clk_top;
> > + }
> > +
> > + ret = mt7621_register_gates(clk_prov);
> > + if (ret) {
> > + pr_err("Couldn't register fixed clock gates\n");
> > + goto unreg_clk_fixed;
> > + }
> > +
> > + ret = of_clk_add_hw_provider(node, of_clk_hw_onecell_get,
> > + clk_prov->clk_data);
> > + if (ret) {
> > + pr_err("Couldn't add clk hw provider\n");
> > + goto unreg_clk_gates;
> > + }
> > +
> > + return;
> > +
> > +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);
> > + }
> > +
> > +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_prov->clk_data);
> > +
> > +free_clk_prov:
> > + kfree(clk_prov);
> > +}
> > +
> > +CLK_OF_DECLARE(mt7621_clk, "mediatek,mt7621-clk", mt7621_clk_init);
>
> Any reason to use this vs. a platform driver?
We need clocks available in 'plat_time_init' before setting up the
timer for the GIC, so to maintain all the clock driver in a simple
file and using only one device tree node and no separate the gates
into another platform driver, I think this is the only way to go, but
please correct me if I am wrong.
> > +
> > +MODULE_AUTHOR("Sergio Paracuellos <sergio.paracuellos@gmail.com>");
> > +MODULE_DESCRIPTION("Mediatek Mt7621 clock driver");
> > +MODULE_LICENSE("GPL v2");
>
> It isn't a module, but it would be nice if it was one. If the Kconfig is
> bool these shouldn't be here.
Ok, totally agree I will remove these.
Thanks again for your review. I will ger rid of all the stuff here and send v5.
Best regards,
Sergio Paracuellos
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 2/6] dt: bindings: add mt7621-clk device tree binding documentation
2020-12-17 8:58 ` Stephen Boyd
@ 2020-12-17 10:01 ` Sergio Paracuellos
-1 siblings, 0 replies; 36+ messages in thread
From: Sergio Paracuellos @ 2020-12-17 10:01 UTC (permalink / raw)
To: Stephen Boyd
Cc: Michael Turquette, Rob Herring, John Crispin,
Thomas Bogendoerfer, Greg KH, Chuanhong Guo, Weijie Gao,
open list:COMMON CLK FRAMEWORK, linux-kernel, open list:MIPS,
open list:STAGING SUBSYSTEM, NeilBrown,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
Hi Stephen,
Thanks for the review!
On Thu, Dec 17, 2020 at 9:58 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Sergio Paracuellos (2020-11-22 01:55:52)
> > Adds device tree binding documentation for clocks in the
> > MT7621 SOC.
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > ---
> > .../bindings/clock/mediatek,mt7621-clk.yaml | 67 +++++++++++++++++++
> > 1 file changed, 67 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
> >
>
> Rob?
>
> > diff --git a/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml b/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
> > new file mode 100644
> > index 000000000000..6aca4c1a4a46
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
> > @@ -0,0 +1,67 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/mediatek,mt7621-clk.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MT7621 Clock Device Tree Bindings
> > +
> > +maintainers:
> > + - Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > +
> > +description: |
> > + The MT7621 has a PLL controller from where the cpu clock is provided
> > + as well as derived clocks for the bus and the peripherals. It also
> > + can gate SoC device clocks.
> > +
> > + Each clock is assigned an identifier and client nodes use this identifier
> > + to specify the clock which they consume.
> > +
> > + All these identifiers could be found in:
> > + [1]: <include/dt-bindings/clock/mt7621-clk.h>.
> > +
> > + The mt7621 clock node should be the child of a syscon node with the
> > + required property:
> > +
> > + - compatible: Should be one of the following:
> > + "mediatek,mt7621-sysc", "syscon"
> > +
> > + Refer to the bindings described in
> > + Documentation/devicetree/bindings/mfd/syscon.yaml
> > +
> > +properties:
> > + compatible:
> > + const: mediatek,mt7621-clk
> > +
> > + "#clock-cells":
> > + description:
> > + The first cell indicates the clock gate number, see [1] for available
> > + clocks.
> > + const: 1
> > +
> > + clock-output-names:
> > + maxItems: 8
> > +
> > +required:
> > + - compatible
> > + - '#clock-cells'
> > + - clock-output-names
>
> Why is clock-output-names required? Hopefully it is not required.
Not really, can be removed from here.
>
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/clock/mt7621-clk.h>
> > +
> > + sysc: sysc@0 {
>
> syscon@0? I don't think sysc is a standard node name.
Ok, I will change this into syscon@0 in both bindings and device tree file.
>
> > + compatible = "mediatek,mt7621-sysc", "syscon";
> > + reg = <0x0 0x100>;
> > +
> > + pll {
>
> clock-controller? Why can't the parent device be the clk provider and
> have #clock-cells?
>
I don't get your point, sorry. Can you please explain this a bit more
or point to me to an example to understand the real meaning of this?
> > + compatible = "mediatek,mt7621-clk";
> > + #clock-cells = <1>;
> > + clock-output-names = "xtal", "cpu", "bus",
> > + "50m", "125m", "150m",
> > + "250m", "270m";
> > + };
> > + };
Best regards,
Sergio Paracuellos
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 2/6] dt: bindings: add mt7621-clk device tree binding documentation
@ 2020-12-17 10:01 ` Sergio Paracuellos
0 siblings, 0 replies; 36+ messages in thread
From: Sergio Paracuellos @ 2020-12-17 10:01 UTC (permalink / raw)
To: Stephen Boyd
Cc: Weijie Gao, open list:STAGING SUBSYSTEM, Thomas Bogendoerfer,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Greg KH, Michael Turquette, linux-kernel, open list:MIPS,
Rob Herring, John Crispin, NeilBrown,
open list:COMMON CLK FRAMEWORK
Hi Stephen,
Thanks for the review!
On Thu, Dec 17, 2020 at 9:58 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Sergio Paracuellos (2020-11-22 01:55:52)
> > Adds device tree binding documentation for clocks in the
> > MT7621 SOC.
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > ---
> > .../bindings/clock/mediatek,mt7621-clk.yaml | 67 +++++++++++++++++++
> > 1 file changed, 67 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
> >
>
> Rob?
>
> > diff --git a/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml b/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
> > new file mode 100644
> > index 000000000000..6aca4c1a4a46
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
> > @@ -0,0 +1,67 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/mediatek,mt7621-clk.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MT7621 Clock Device Tree Bindings
> > +
> > +maintainers:
> > + - Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > +
> > +description: |
> > + The MT7621 has a PLL controller from where the cpu clock is provided
> > + as well as derived clocks for the bus and the peripherals. It also
> > + can gate SoC device clocks.
> > +
> > + Each clock is assigned an identifier and client nodes use this identifier
> > + to specify the clock which they consume.
> > +
> > + All these identifiers could be found in:
> > + [1]: <include/dt-bindings/clock/mt7621-clk.h>.
> > +
> > + The mt7621 clock node should be the child of a syscon node with the
> > + required property:
> > +
> > + - compatible: Should be one of the following:
> > + "mediatek,mt7621-sysc", "syscon"
> > +
> > + Refer to the bindings described in
> > + Documentation/devicetree/bindings/mfd/syscon.yaml
> > +
> > +properties:
> > + compatible:
> > + const: mediatek,mt7621-clk
> > +
> > + "#clock-cells":
> > + description:
> > + The first cell indicates the clock gate number, see [1] for available
> > + clocks.
> > + const: 1
> > +
> > + clock-output-names:
> > + maxItems: 8
> > +
> > +required:
> > + - compatible
> > + - '#clock-cells'
> > + - clock-output-names
>
> Why is clock-output-names required? Hopefully it is not required.
Not really, can be removed from here.
>
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/clock/mt7621-clk.h>
> > +
> > + sysc: sysc@0 {
>
> syscon@0? I don't think sysc is a standard node name.
Ok, I will change this into syscon@0 in both bindings and device tree file.
>
> > + compatible = "mediatek,mt7621-sysc", "syscon";
> > + reg = <0x0 0x100>;
> > +
> > + pll {
>
> clock-controller? Why can't the parent device be the clk provider and
> have #clock-cells?
>
I don't get your point, sorry. Can you please explain this a bit more
or point to me to an example to understand the real meaning of this?
> > + compatible = "mediatek,mt7621-clk";
> > + #clock-cells = <1>;
> > + clock-output-names = "xtal", "cpu", "bus",
> > + "50m", "125m", "150m",
> > + "250m", "270m";
> > + };
> > + };
Best regards,
Sergio Paracuellos
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 2/6] dt: bindings: add mt7621-clk device tree binding documentation
2020-12-17 10:01 ` Sergio Paracuellos
(?)
@ 2020-12-17 10:07 ` Stephen Boyd
2020-12-17 10:14 ` Sergio Paracuellos
-1 siblings, 1 reply; 36+ messages in thread
From: Stephen Boyd @ 2020-12-17 10:07 UTC (permalink / raw)
To: Sergio Paracuellos
Cc: Weijie Gao, STAGING SUBSYSTEM, Thomas Bogendoerfer,
OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Greg KH,
Michael Turquette, linux-kernel, MIPS, Rob Herring, John Crispin,
NeilBrown, COMMON CLK FRAMEWORK
Quoting Sergio Paracuellos (2020-12-17 02:01:39)
>
> On Thu, Dec 17, 2020 at 9:58 AM Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > Quoting Sergio Paracuellos (2020-11-22 01:55:52)
> >
> > > diff --git a/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml b/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
> > > new file mode 100644
> > > index 000000000000..6aca4c1a4a46
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
> >
> > > + compatible = "mediatek,mt7621-sysc", "syscon";
> > > + reg = <0x0 0x100>;
> > > +
> > > + pll {
> >
> > clock-controller? Why can't the parent device be the clk provider and
> > have #clock-cells?
> >
>
> I don't get your point, sorry. Can you please explain this a bit more
> or point to me to an example to understand the real meaning of this?
It looks like this is a made up child node of syscon so that a driver
can probe in the kernel. It would be more DT friendly to create a
platform device from the parent node's driver, or just register the clks
with the framework directly in that driver.
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 3/6] clk: ralink: add clock driver for mt7621 SoC
2020-12-17 9:54 ` Sergio Paracuellos
(?)
@ 2020-12-17 10:12 ` Stephen Boyd
2020-12-17 10:21 ` Sergio Paracuellos
-1 siblings, 1 reply; 36+ messages in thread
From: Stephen Boyd @ 2020-12-17 10:12 UTC (permalink / raw)
To: Sergio Paracuellos
Cc: Weijie Gao, STAGING SUBSYSTEM, Thomas Bogendoerfer, Greg KH,
Michael Turquette, linux-kernel, evicetree, MIPS, Rob Herring,
John Crispin, NeilBrown, COMMON CLK FRAMEWORK
Quoting Sergio Paracuellos (2020-12-17 01:54:18)
>
> On Thu, Dec 17, 2020 at 10:09 AM Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > Quoting Sergio Paracuellos (2020-11-22 01:55:53)
> > > 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..4e929f13fe7c
> > > --- /dev/null
> > > +++ b/drivers/clk/ralink/clk-mt7621.c
> > > @@ -0,0 +1,435 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Mediatek MT7621 Clock Driver
> > > + * Author: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > > + */
> > > +
> > > +#include <linux/bitops.h>
> > > +#include <linux/clk-provider.h>
> > > +#include <linux/mfd/syscon.h>
> > > +#include <linux/module.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/regmap.h>
> > > +#include <asm/mach-ralink/ralink_regs.h>
> >
> > Is it possible to drop this include? Doing so would make this portable
> > and compilable on more architectures so us cross compilers can check
> > build stuff and make changes easily.
>
> No, this is not possible. This old arch makes some global functions
> there to properly access different registers in the palmbus. It is not
> also well documented so it is really difficult to make something
> better with this.
> This is needed to use 'rt_memc_r32'
> (arch/mips/include/asm/mach-ralink/ralink_regs.h) for reading
> MEMC_REG_CPU_PLL.
>
> This is a not documented register and is not in the syscon related
> part and we need it to derive the clock frequency for the XTAL clock.
Ok.
> > > +static int mt7621_gate_ops_init(struct device_node *np,
> > > + struct mt7621_gate *sclk)
> > > +{
> > > + struct clk_init_data init = {
> > > + .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> >
> > Why ignore unused? Are they CLK_IS_CRITICAL? Can they be enabled at
> > driver probe instead of here? Or left out of the kernel entirely if they
> > shouldn't be turned off?
>
> Because all the platform drivers are not changed to use this gates yet
> and all gates are enabled by default (related registers are set to all
> ones), kernel disables all the stuff because they are not being
> referenced, but yes, you are right, I think I can call
> clk_prepare_enable for all of them at init time and avoid this
> 'CLK_IGNORE_UNUSED' flag to don't break anything of the current other
> upstream code.
Does something crash if they're turned off? We have CLK_IS_CRITICAL for
that. The CLK_IGNORE_UNUSED flag is sort of deprecated now.
> > > +
> > > +#define CLK_BASE(_name, _parent, _recalc) { \
> > > + .init = &(struct clk_init_data) { \
> > > + .name = _name, \
> > > + .ops = &(const struct clk_ops) { \
> > > + .recalc_rate = _recalc, \
> > > + }, \
> > > + .parent_names = (const char *const[]) { _parent }, \
> >
> > Please use clk_parent_data instead
>
> parent can also be NULL here and num_parents zero, but I will search
> what do you really mean with this 'clk_parent_data' :).
Heh, 'git grep clk_parent_data -- drivers/clk/' should give some clues.
> > > +free_clk_prov:
> > > + kfree(clk_prov);
> > > +}
> > > +
> > > +CLK_OF_DECLARE(mt7621_clk, "mediatek,mt7621-clk", mt7621_clk_init);
> >
> > Any reason to use this vs. a platform driver?
>
> We need clocks available in 'plat_time_init' before setting up the
> timer for the GIC, so to maintain all the clock driver in a simple
> file and using only one device tree node and no separate the gates
> into another platform driver, I think this is the only way to go, but
> please correct me if I am wrong.
We can register the few clks like that early with
CLK_OF_DECLARE_DRIVER() and then have a platform driver register the
rest of the clks that aren't required early. This lets us hook into the
driver framework better while still getting those few clks to turn on
early enough for the timers.
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 2/6] dt: bindings: add mt7621-clk device tree binding documentation
2020-12-17 10:07 ` Stephen Boyd
@ 2020-12-17 10:14 ` Sergio Paracuellos
2020-12-17 10:32 ` Stephen Boyd
0 siblings, 1 reply; 36+ messages in thread
From: Sergio Paracuellos @ 2020-12-17 10:14 UTC (permalink / raw)
To: Stephen Boyd
Cc: Michael Turquette, Rob Herring, John Crispin,
Thomas Bogendoerfer, Greg KH, Chuanhong Guo, Weijie Gao,
COMMON CLK FRAMEWORK, linux-kernel,
open list:MIPS <linux-mips@vger.kernel.org>,
open list:STAGING SUBSYSTEM <devel@driverdev.osuosl.org>,
NeilBrown <neil@brown.name>,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
On Thu, Dec 17, 2020 at 11:07 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Sergio Paracuellos (2020-12-17 02:01:39)
> >
> > On Thu, Dec 17, 2020 at 9:58 AM Stephen Boyd <sboyd@kernel.org> wrote:
> > >
> > > Quoting Sergio Paracuellos (2020-11-22 01:55:52)
> > >
> > > > diff --git a/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml b/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
> > > > new file mode 100644
> > > > index 000000000000..6aca4c1a4a46
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
> > >
> > > > + compatible = "mediatek,mt7621-sysc", "syscon";
> > > > + reg = <0x0 0x100>;
> > > > +
> > > > + pll {
> > >
> > > clock-controller? Why can't the parent device be the clk provider and
> > > have #clock-cells?
> > >
> >
> > I don't get your point, sorry. Can you please explain this a bit more
> > or point to me to an example to understand the real meaning of this?
>
> It looks like this is a made up child node of syscon so that a driver
> can probe in the kernel. It would be more DT friendly to create a
> platform device from the parent node's driver, or just register the clks
> with the framework directly in that driver.
We cannot create a platform device because we need clocks available in
'plat_time_init' before setting up the timer for the GIC.
The only way I see to avoid this syscon and having this as a child
node is to use architecture operations in
'arch/mips/include/asm/mach-ralink/ralink_regs.h'
instead of getting a phandle using the regmap is being currently used...
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 3/6] clk: ralink: add clock driver for mt7621 SoC
2020-12-17 10:12 ` Stephen Boyd
@ 2020-12-17 10:21 ` Sergio Paracuellos
0 siblings, 0 replies; 36+ messages in thread
From: Sergio Paracuellos @ 2020-12-17 10:21 UTC (permalink / raw)
To: Stephen Boyd
Cc: Michael Turquette, Rob Herring, John Crispin,
Thomas Bogendoerfer, Greg KH, Chuanhong Guo, Weijie Gao,
COMMON CLK FRAMEWORK, evicetree, linux-kernel,
open list:MIPS <linux-mips@vger.kernel.org>,
open list:STAGING SUBSYSTEM <devel@driverdev.osuosl.org>,
NeilBrown
On Thu, Dec 17, 2020 at 11:12 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Sergio Paracuellos (2020-12-17 01:54:18)
> >
> > On Thu, Dec 17, 2020 at 10:09 AM Stephen Boyd <sboyd@kernel.org> wrote:
> > >
> > > Quoting Sergio Paracuellos (2020-11-22 01:55:53)
> > > > 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..4e929f13fe7c
> > > > --- /dev/null
> > > > +++ b/drivers/clk/ralink/clk-mt7621.c
> > > > @@ -0,0 +1,435 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Mediatek MT7621 Clock Driver
> > > > + * Author: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > > > + */
> > > > +
> > > > +#include <linux/bitops.h>
> > > > +#include <linux/clk-provider.h>
> > > > +#include <linux/mfd/syscon.h>
> > > > +#include <linux/module.h>
> > > > +#include <linux/slab.h>
> > > > +#include <linux/regmap.h>
> > > > +#include <asm/mach-ralink/ralink_regs.h>
> > >
> > > Is it possible to drop this include? Doing so would make this portable
> > > and compilable on more architectures so us cross compilers can check
> > > build stuff and make changes easily.
> >
> > No, this is not possible. This old arch makes some global functions
> > there to properly access different registers in the palmbus. It is not
> > also well documented so it is really difficult to make something
> > better with this.
> > This is needed to use 'rt_memc_r32'
> > (arch/mips/include/asm/mach-ralink/ralink_regs.h) for reading
> > MEMC_REG_CPU_PLL.
> >
> > This is a not documented register and is not in the syscon related
> > part and we need it to derive the clock frequency for the XTAL clock.
>
> Ok.
>
> > > > +static int mt7621_gate_ops_init(struct device_node *np,
> > > > + struct mt7621_gate *sclk)
> > > > +{
> > > > + struct clk_init_data init = {
> > > > + .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> > >
> > > Why ignore unused? Are they CLK_IS_CRITICAL? Can they be enabled at
> > > driver probe instead of here? Or left out of the kernel entirely if they
> > > shouldn't be turned off?
> >
> > Because all the platform drivers are not changed to use this gates yet
> > and all gates are enabled by default (related registers are set to all
> > ones), kernel disables all the stuff because they are not being
> > referenced, but yes, you are right, I think I can call
> > clk_prepare_enable for all of them at init time and avoid this
> > 'CLK_IGNORE_UNUSED' flag to don't break anything of the current other
> > upstream code.
>
> Does something crash if they're turned off? We have CLK_IS_CRITICAL for
> that. The CLK_IGNORE_UNUSED flag is sort of deprecated now.
Well, as drivers are not getting into account gates and not referenced
real hw bits are disabled by kernel because nobody requested them so
for example my uart gets down and cannot really see anything :). I
think call to 'clk_prepare_enable' should be enough since by default
all of them are setting up in registers, so call that will also
reference them...
>
> > > > +
> > > > +#define CLK_BASE(_name, _parent, _recalc) { \
> > > > + .init = &(struct clk_init_data) { \
> > > > + .name = _name, \
> > > > + .ops = &(const struct clk_ops) { \
> > > > + .recalc_rate = _recalc, \
> > > > + }, \
> > > > + .parent_names = (const char *const[]) { _parent }, \
> > >
> > > Please use clk_parent_data instead
> >
> > parent can also be NULL here and num_parents zero, but I will search
> > what do you really mean with this 'clk_parent_data' :).
>
> Heh, 'git grep clk_parent_data -- drivers/clk/' should give some clues.
Thanks, will do!
>
> > > > +free_clk_prov:
> > > > + kfree(clk_prov);
> > > > +}
> > > > +
> > > > +CLK_OF_DECLARE(mt7621_clk, "mediatek,mt7621-clk", mt7621_clk_init);
> > >
> > > Any reason to use this vs. a platform driver?
> >
> > We need clocks available in 'plat_time_init' before setting up the
> > timer for the GIC, so to maintain all the clock driver in a simple
> > file and using only one device tree node and no separate the gates
> > into another platform driver, I think this is the only way to go, but
> > please correct me if I am wrong.
>
> We can register the few clks like that early with
> CLK_OF_DECLARE_DRIVER() and then have a platform driver register the
> rest of the clks that aren't required early. This lets us hook into the
> driver framework better while still getting those few clks to turn on
> early enough for the timers.
I see. I will explore the way to do this as you are suggesting here, thanks!
Best regards,
Sergio Paracuellos
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 2/6] dt: bindings: add mt7621-clk device tree binding documentation
2020-12-17 10:14 ` Sergio Paracuellos
@ 2020-12-17 10:32 ` Stephen Boyd
2020-12-17 10:38 ` Sergio Paracuellos
0 siblings, 1 reply; 36+ messages in thread
From: Stephen Boyd @ 2020-12-17 10:32 UTC (permalink / raw)
To: Sergio Paracuellos
Cc: Michael Turquette, Rob Herring, John Crispin,
Thomas Bogendoerfer, Greg KH, Chuanhong Guo, Weijie Gao,
COMMON CLK FRAMEWORK, linux-kernel,
open list:MIPS <linux-mips@vger.kernel.org>,
open list:STAGING SUBSYSTEM <devel@driverdev.osuosl.org>,
NeilBrown <neil@brown.name>,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
Quoting Sergio Paracuellos (2020-12-17 02:14:10)
> On Thu, Dec 17, 2020 at 11:07 AM Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > Quoting Sergio Paracuellos (2020-12-17 02:01:39)
> > >
> > > On Thu, Dec 17, 2020 at 9:58 AM Stephen Boyd <sboyd@kernel.org> wrote:
> > > >
> > > > Quoting Sergio Paracuellos (2020-11-22 01:55:52)
> > > >
> > > > > diff --git a/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml b/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
> > > > > new file mode 100644
> > > > > index 000000000000..6aca4c1a4a46
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
> > > >
> > > > > + compatible = "mediatek,mt7621-sysc", "syscon";
> > > > > + reg = <0x0 0x100>;
> > > > > +
> > > > > + pll {
> > > >
> > > > clock-controller? Why can't the parent device be the clk provider and
> > > > have #clock-cells?
> > > >
> > >
> > > I don't get your point, sorry. Can you please explain this a bit more
> > > or point to me to an example to understand the real meaning of this?
> >
> > It looks like this is a made up child node of syscon so that a driver
> > can probe in the kernel. It would be more DT friendly to create a
> > platform device from the parent node's driver, or just register the clks
> > with the framework directly in that driver.
>
> We cannot create a platform device because we need clocks available in
> 'plat_time_init' before setting up the timer for the GIC.
> The only way I see to avoid this syscon and having this as a child
> node is to use architecture operations in
> 'arch/mips/include/asm/mach-ralink/ralink_regs.h'
> instead of getting a phandle using the regmap is being currently used...
Can that be done with
CLK_OF_DECLARE_DRIVER("mediatek,mt7621-sysc", my_timer_clk_init)
? Is the syscon used anywhere besides by the clk driver?
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 2/6] dt: bindings: add mt7621-clk device tree binding documentation
2020-12-17 10:32 ` Stephen Boyd
@ 2020-12-17 10:38 ` Sergio Paracuellos
2020-12-17 10:50 ` Stephen Boyd
0 siblings, 1 reply; 36+ messages in thread
From: Sergio Paracuellos @ 2020-12-17 10:38 UTC (permalink / raw)
To: Stephen Boyd
Cc: Michael Turquette, Rob Herring, John Crispin,
Thomas Bogendoerfer, Greg KH, Chuanhong Guo, Weijie Gao,
COMMON CLK FRAMEWORK, linux-kernel,
open list:MIPS <linux-mips@vger.kernel.org>,
open list:STAGING SUBSYSTEM <devel@driverdev.osuosl.org>,
NeilBrown <neil@brown.name>,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
On Thu, Dec 17, 2020 at 11:32 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Sergio Paracuellos (2020-12-17 02:14:10)
> > On Thu, Dec 17, 2020 at 11:07 AM Stephen Boyd <sboyd@kernel.org> wrote:
> > >
> > > Quoting Sergio Paracuellos (2020-12-17 02:01:39)
> > > >
> > > > On Thu, Dec 17, 2020 at 9:58 AM Stephen Boyd <sboyd@kernel.org> wrote:
> > > > >
> > > > > Quoting Sergio Paracuellos (2020-11-22 01:55:52)
> > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml b/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
> > > > > > new file mode 100644
> > > > > > index 000000000000..6aca4c1a4a46
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
> > > > >
> > > > > > + compatible = "mediatek,mt7621-sysc", "syscon";
> > > > > > + reg = <0x0 0x100>;
> > > > > > +
> > > > > > + pll {
> > > > >
> > > > > clock-controller? Why can't the parent device be the clk provider and
> > > > > have #clock-cells?
> > > > >
> > > >
> > > > I don't get your point, sorry. Can you please explain this a bit more
> > > > or point to me to an example to understand the real meaning of this?
> > >
> > > It looks like this is a made up child node of syscon so that a driver
> > > can probe in the kernel. It would be more DT friendly to create a
> > > platform device from the parent node's driver, or just register the clks
> > > with the framework directly in that driver.
> >
> > We cannot create a platform device because we need clocks available in
> > 'plat_time_init' before setting up the timer for the GIC.
> > The only way I see to avoid this syscon and having this as a child
> > node is to use architecture operations in
> > 'arch/mips/include/asm/mach-ralink/ralink_regs.h'
> > instead of getting a phandle using the regmap is being currently used...
>
> Can that be done with
>
> CLK_OF_DECLARE_DRIVER("mediatek,mt7621-sysc", my_timer_clk_init)
>
> ? Is the syscon used anywhere besides by the clk driver?
Yes, for example all the gates use them to access SYSC_REG_CLKCFG1 in
all of their 'mt7621_gate_ops' and also in all 'recalc_rate' functions
where SYSC_REG_SYSTEM_CONFIG0, is readed.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 2/6] dt: bindings: add mt7621-clk device tree binding documentation
2020-12-17 10:38 ` Sergio Paracuellos
@ 2020-12-17 10:50 ` Stephen Boyd
2020-12-17 10:54 ` Sergio Paracuellos
0 siblings, 1 reply; 36+ messages in thread
From: Stephen Boyd @ 2020-12-17 10:50 UTC (permalink / raw)
To: Sergio Paracuellos
Cc: Michael Turquette, Rob Herring, John Crispin,
Thomas Bogendoerfer, Greg KH, Chuanhong Guo, Weijie Gao,
COMMON CLK FRAMEWORK, linux-kernel,
open list:MIPS <linux-mips@vger.kernel.org>,
open list:STAGING SUBSYSTEM <devel@driverdev.osuosl.org>,
NeilBrown <neil@brown.name>,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
Quoting Sergio Paracuellos (2020-12-17 02:38:37)
> On Thu, Dec 17, 2020 at 11:32 AM Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > Quoting Sergio Paracuellos (2020-12-17 02:14:10)
> > > node is to use architecture operations in
> > > 'arch/mips/include/asm/mach-ralink/ralink_regs.h'
> > > instead of getting a phandle using the regmap is being currently used...
> >
> > Can that be done with
> >
> > CLK_OF_DECLARE_DRIVER("mediatek,mt7621-sysc", my_timer_clk_init)
> >
> > ? Is the syscon used anywhere besides by the clk driver?
>
> Yes, for example all the gates use them to access SYSC_REG_CLKCFG1 in
> all of their 'mt7621_gate_ops' and also in all 'recalc_rate' functions
> where SYSC_REG_SYSTEM_CONFIG0, is readed.
That sounds like it's only used by the clk provider/driver? Any other
code uses the syscon?
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 2/6] dt: bindings: add mt7621-clk device tree binding documentation
2020-12-17 10:50 ` Stephen Boyd
@ 2020-12-17 10:54 ` Sergio Paracuellos
0 siblings, 0 replies; 36+ messages in thread
From: Sergio Paracuellos @ 2020-12-17 10:54 UTC (permalink / raw)
To: Stephen Boyd
Cc: Michael Turquette, Rob Herring, John Crispin,
Thomas Bogendoerfer, Greg KH, Chuanhong Guo, Weijie Gao,
COMMON CLK FRAMEWORK, linux-kernel,
open list:MIPS <linux-mips@vger.kernel.org>,
open list:STAGING SUBSYSTEM <devel@driverdev.osuosl.org>,
NeilBrown <neil@brown.name>,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
On Thu, Dec 17, 2020 at 11:50 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Sergio Paracuellos (2020-12-17 02:38:37)
> > On Thu, Dec 17, 2020 at 11:32 AM Stephen Boyd <sboyd@kernel.org> wrote:
> > >
> > > Quoting Sergio Paracuellos (2020-12-17 02:14:10)
> > > > node is to use architecture operations in
> > > > 'arch/mips/include/asm/mach-ralink/ralink_regs.h'
> > > > instead of getting a phandle using the regmap is being currently used...
> > >
> > > Can that be done with
> > >
> > > CLK_OF_DECLARE_DRIVER("mediatek,mt7621-sysc", my_timer_clk_init)
> > >
> > > ? Is the syscon used anywhere besides by the clk driver?
> >
> > Yes, for example all the gates use them to access SYSC_REG_CLKCFG1 in
> > all of their 'mt7621_gate_ops' and also in all 'recalc_rate' functions
> > where SYSC_REG_SYSTEM_CONFIG0, is readed.
>
> That sounds like it's only used by the clk provider/driver? Any other
> code uses the syscon?
The only child node for the syscon for this platform is the clock
driver now, I introduced it in this series, so no other driver is
using this syscon now. All of them use global arch operations in
'arch/mips/include/asm/mach-ralink/ralink_regs.h'.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 2/6] dt: bindings: add mt7621-clk device tree binding documentation
2020-12-17 8:58 ` Stephen Boyd
@ 2020-12-17 15:04 ` Rob Herring
-1 siblings, 0 replies; 36+ messages in thread
From: Rob Herring @ 2020-12-17 15:04 UTC (permalink / raw)
To: Stephen Boyd
Cc: Sergio Paracuellos, Michael Turquette, John Crispin,
Thomas Bogendoerfer, Greg Kroah-Hartman, Chuanhong Guo,
Weijie Gao, linux-clk, evicetree, linux-kernel, open list:MIPS,
open list:STAGING SUBSYSTEM, NeilBrown
On Thu, Dec 17, 2020 at 2:58 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Sergio Paracuellos (2020-11-22 01:55:52)
> > Adds device tree binding documentation for clocks in the
> > MT7621 SOC.
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > ---
> > .../bindings/clock/mediatek,mt7621-clk.yaml | 67 +++++++++++++++++++
> > 1 file changed, 67 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
> >
>
> Rob?
Send to the DT list please.
But I agree with Stephen's comment. Either make the syscon complete
(fully describe the h/w, not just what you need ATM) to show the need
for child nodes or get rid of the child nodes.
Rob
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 2/6] dt: bindings: add mt7621-clk device tree binding documentation
@ 2020-12-17 15:04 ` Rob Herring
0 siblings, 0 replies; 36+ messages in thread
From: Rob Herring @ 2020-12-17 15:04 UTC (permalink / raw)
To: Stephen Boyd
Cc: Weijie Gao, open list:STAGING SUBSYSTEM, Thomas Bogendoerfer,
Greg Kroah-Hartman, Michael Turquette, linux-kernel, evicetree,
open list:MIPS, John Crispin, NeilBrown, linux-clk
On Thu, Dec 17, 2020 at 2:58 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Sergio Paracuellos (2020-11-22 01:55:52)
> > Adds device tree binding documentation for clocks in the
> > MT7621 SOC.
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > ---
> > .../bindings/clock/mediatek,mt7621-clk.yaml | 67 +++++++++++++++++++
> > 1 file changed, 67 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
> >
>
> Rob?
Send to the DT list please.
But I agree with Stephen's comment. Either make the syscon complete
(fully describe the h/w, not just what you need ATM) to show the need
for child nodes or get rid of the child nodes.
Rob
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 2/6] dt: bindings: add mt7621-clk device tree binding documentation
2020-12-17 15:04 ` Rob Herring
@ 2020-12-17 15:12 ` Sergio Paracuellos
-1 siblings, 0 replies; 36+ messages in thread
From: Sergio Paracuellos @ 2020-12-17 15:12 UTC (permalink / raw)
To: Rob Herring
Cc: Stephen Boyd, Michael Turquette, John Crispin,
Thomas Bogendoerfer, Greg Kroah-Hartman, Chuanhong Guo,
Weijie Gao, linux-clk, linux-kernel, open list:MIPS,
open list:STAGING SUBSYSTEM, NeilBrown,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
Hi Rob,
On Thu, Dec 17, 2020 at 4:05 PM Rob Herring <robh+dt@kernel.org> wrote:
>
> On Thu, Dec 17, 2020 at 2:58 AM Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > Quoting Sergio Paracuellos (2020-11-22 01:55:52)
> > > Adds device tree binding documentation for clocks in the
> > > MT7621 SOC.
> > >
> > > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > > ---
> > > .../bindings/clock/mediatek,mt7621-clk.yaml | 67 +++++++++++++++++++
> > > 1 file changed, 67 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
> > >
> >
> > Rob?
>
> Send to the DT list please.
Sorry, there was a typo 'evicetree@vger.kernel.org' in CC list.
>
> But I agree with Stephen's comment. Either make the syscon complete
> (fully describe the h/w, not just what you need ATM) to show the need
> for child nodes or get rid of the child nodes.
Got it. Will try to do something better and send v5.
>
> Rob
Thanks,
Sergio Paracuellos
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v4 2/6] dt: bindings: add mt7621-clk device tree binding documentation
@ 2020-12-17 15:12 ` Sergio Paracuellos
0 siblings, 0 replies; 36+ messages in thread
From: Sergio Paracuellos @ 2020-12-17 15:12 UTC (permalink / raw)
To: Rob Herring
Cc: Weijie Gao, open list:STAGING SUBSYSTEM, Thomas Bogendoerfer,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Stephen Boyd, Greg Kroah-Hartman, Michael Turquette,
linux-kernel, open list:MIPS, John Crispin, NeilBrown, linux-clk
Hi Rob,
On Thu, Dec 17, 2020 at 4:05 PM Rob Herring <robh+dt@kernel.org> wrote:
>
> On Thu, Dec 17, 2020 at 2:58 AM Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > Quoting Sergio Paracuellos (2020-11-22 01:55:52)
> > > Adds device tree binding documentation for clocks in the
> > > MT7621 SOC.
> > >
> > > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > > ---
> > > .../bindings/clock/mediatek,mt7621-clk.yaml | 67 +++++++++++++++++++
> > > 1 file changed, 67 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mt7621-clk.yaml
> > >
> >
> > Rob?
>
> Send to the DT list please.
Sorry, there was a typo 'evicetree@vger.kernel.org' in CC list.
>
> But I agree with Stephen's comment. Either make the syscon complete
> (fully describe the h/w, not just what you need ATM) to show the need
> for child nodes or get rid of the child nodes.
Got it. Will try to do something better and send v5.
>
> Rob
Thanks,
Sergio Paracuellos
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2020-12-17 15:13 UTC | newest]
Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-22 9:55 [PATCH v4 0/6] MIPS: ralink: add CPU clock detection and clock driver for MT7621 Sergio Paracuellos
2020-11-22 9:55 ` Sergio Paracuellos
2020-11-22 9:55 ` [PATCH v4 1/6] dt-bindings: clock: add dt binding header for mt7621 clocks Sergio Paracuellos
2020-11-22 9:55 ` Sergio Paracuellos
2020-11-22 9:55 ` [PATCH v4 2/6] dt: bindings: add mt7621-clk device tree binding documentation Sergio Paracuellos
2020-11-22 9:55 ` Sergio Paracuellos
2020-12-17 8:58 ` Stephen Boyd
2020-12-17 8:58 ` Stephen Boyd
2020-12-17 10:01 ` Sergio Paracuellos
2020-12-17 10:01 ` Sergio Paracuellos
2020-12-17 10:07 ` Stephen Boyd
2020-12-17 10:14 ` Sergio Paracuellos
2020-12-17 10:32 ` Stephen Boyd
2020-12-17 10:38 ` Sergio Paracuellos
2020-12-17 10:50 ` Stephen Boyd
2020-12-17 10:54 ` Sergio Paracuellos
2020-12-17 15:04 ` Rob Herring
2020-12-17 15:04 ` Rob Herring
2020-12-17 15:12 ` Sergio Paracuellos
2020-12-17 15:12 ` Sergio Paracuellos
2020-11-22 9:55 ` [PATCH v4 3/6] clk: ralink: add clock driver for mt7621 SoC Sergio Paracuellos
2020-11-22 9:55 ` Sergio Paracuellos
2020-12-17 9:09 ` Stephen Boyd
2020-12-17 9:09 ` Stephen Boyd
2020-12-17 9:54 ` Sergio Paracuellos
2020-12-17 9:54 ` Sergio Paracuellos
2020-12-17 10:12 ` Stephen Boyd
2020-12-17 10:21 ` Sergio Paracuellos
2020-11-22 9:55 ` [PATCH v4 4/6] staging: mt7621-dts: make use of new 'mt7621-clk' Sergio Paracuellos
2020-11-22 9:55 ` Sergio Paracuellos
2020-11-22 9:55 ` [PATCH v4 5/6] staging: mt7621-dts: use valid vendor 'mediatek' instead of invalid 'mtk' Sergio Paracuellos
2020-11-22 9:55 ` Sergio Paracuellos
2020-11-22 9:55 ` [PATCH v4 6/6] MAINTAINERS: add MT7621 CLOCK maintainer Sergio Paracuellos
2020-11-22 9:55 ` Sergio Paracuellos
2020-12-10 6:55 ` [PATCH v4 0/6] MIPS: ralink: add CPU clock detection and clock driver for MT7621 Sergio Paracuellos
2020-12-10 6:55 ` Sergio Paracuellos
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.