2019-09-16 21:35:34

by Manivannan Sadhasivam

[permalink] [raw]
Subject: [PATCH v5 0/8] Add Bitmain BM1880 clock driver

Hello,

This patchset adds common clock driver for Bitmain BM1880 SoC clock
controller. The clock controller consists of gate, divider, mux
and pll clocks with different compositions. Hence, the driver uses
composite clock structure in place where multiple clocking units are
combined together.

This patchset also removes UART fixed clock and sources clocks from clock
controller for Sophon Edge board where the driver has been validated.

Thanks,
Mani

Changes in v5:

* Incorporated review comments from Rob on dt binding

Changes in v4:

* Fixed devicetree binding issue
* Added ARCH_BITMAIN as the default for the clk driver

Changes in v3:

* Switched to clk_hw_{register/unregister} APIs
* Returned clk_hw from the in-driver registration helpers

Changes in v2:

* Converted the dt binding to YAML
* Incorporated review comments from Stephen (majority of change is switching
to new way of specifying clk parents)


Manivannan Sadhasivam (8):
clk: Zero init clk_init_data in helpers
clk: Warn if clk_init_data is not zero initialized
clk: Add clk_hw_unregister_composite helper function definition
dt-bindings: clock: Add devicetree binding for BM1880 SoC
arm64: dts: bitmain: Add clock controller support for BM1880 SoC
arm64: dts: bitmain: Source common clock for UART controllers
clk: Add common clock driver for BM1880 SoC
MAINTAINERS: Add entry for BM1880 SoC clock driver

.../bindings/clock/bitmain,bm1880-clk.yaml | 76 ++
MAINTAINERS | 2 +
.../boot/dts/bitmain/bm1880-sophon-edge.dts | 9 -
arch/arm64/boot/dts/bitmain/bm1880.dtsi | 28 +
drivers/clk/Kconfig | 7 +
drivers/clk/Makefile | 1 +
drivers/clk/clk-bm1880.c | 966 ++++++++++++++++++
drivers/clk/clk-composite.c | 13 +-
drivers/clk/clk-divider.c | 2 +-
drivers/clk/clk-fixed-rate.c | 2 +-
drivers/clk/clk-gate.c | 2 +-
drivers/clk/clk-mux.c | 2 +-
drivers/clk/clk.c | 8 +
include/dt-bindings/clock/bm1880-clock.h | 82 ++
14 files changed, 1186 insertions(+), 14 deletions(-)
create mode 100644 Documentation/devicetree/bindings/clock/bitmain,bm1880-clk.yaml
create mode 100644 drivers/clk/clk-bm1880.c
create mode 100644 include/dt-bindings/clock/bm1880-clock.h

--
2.17.1


2019-09-16 21:35:39

by Manivannan Sadhasivam

[permalink] [raw]
Subject: [PATCH v5 2/8] clk: Warn if clk_init_data is not zero initialized

The new implementation for determining parent map uses multiple ways
to pass parent info. The order in which it gets processed depends on
the first available member. Hence, it is necessary to zero init the
clk_init_data struct so that the expected member gets processed correctly.
So, add a warning if multiple clk_init_data members are available during
clk registration.

Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/clk/clk.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index c0990703ce54..7d6d6984c979 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3497,6 +3497,14 @@ static int clk_core_populate_parent_map(struct clk_core *core)
if (!num_parents)
return 0;

+ /*
+ * Check for non-zero initialized clk_init_data struct. This is
+ * required because, we only require one of the (parent_names/
+ * parent_data/parent_hws) to be set at a time. Otherwise, the
+ * current code would use first available member.
+ */
+ WARN_ON((parent_names && parent_data) || (parent_names && parent_hws));
+
/*
* Avoid unnecessary string look-ups of clk_core's possible parents by
* having a cache of names/clk_hw pointers to clk_core pointers.
--
2.17.1

2019-09-16 21:35:44

by Manivannan Sadhasivam

[permalink] [raw]
Subject: [PATCH v5 6/8] arm64: dts: bitmain: Source common clock for UART controllers

Remove fixed clock and source common clock for UART controllers.

Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
arch/arm64/boot/dts/bitmain/bm1880-sophon-edge.dts | 9 ---------
arch/arm64/boot/dts/bitmain/bm1880.dtsi | 12 ++++++++++++
2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/boot/dts/bitmain/bm1880-sophon-edge.dts b/arch/arm64/boot/dts/bitmain/bm1880-sophon-edge.dts
index 3e8c70778e24..7a2c7f9c2660 100644
--- a/arch/arm64/boot/dts/bitmain/bm1880-sophon-edge.dts
+++ b/arch/arm64/boot/dts/bitmain/bm1880-sophon-edge.dts
@@ -49,12 +49,6 @@
reg = <0x1 0x00000000 0x0 0x40000000>; // 1GB
};

- uart_clk: uart-clk {
- compatible = "fixed-clock";
- clock-frequency = <500000000>;
- #clock-cells = <0>;
- };
-
soc {
gpio0: gpio@50027000 {
porta: gpio-controller@0 {
@@ -173,21 +167,18 @@

&uart0 {
status = "okay";
- clocks = <&uart_clk>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_uart0_default>;
};

&uart1 {
status = "okay";
- clocks = <&uart_clk>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_uart1_default>;
};

&uart2 {
status = "okay";
- clocks = <&uart_clk>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_uart2_default>;
};
diff --git a/arch/arm64/boot/dts/bitmain/bm1880.dtsi b/arch/arm64/boot/dts/bitmain/bm1880.dtsi
index 8471662413da..fa6e6905f588 100644
--- a/arch/arm64/boot/dts/bitmain/bm1880.dtsi
+++ b/arch/arm64/boot/dts/bitmain/bm1880.dtsi
@@ -174,6 +174,9 @@
uart0: serial@58018000 {
compatible = "snps,dw-apb-uart";
reg = <0x0 0x58018000 0x0 0x2000>;
+ clocks = <&clk BM1880_CLK_UART_500M>,
+ <&clk BM1880_CLK_APB_UART>;
+ clock-names = "baudclk", "apb_pclk";
interrupts = <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>;
reg-shift = <2>;
reg-io-width = <4>;
@@ -184,6 +187,9 @@
uart1: serial@5801A000 {
compatible = "snps,dw-apb-uart";
reg = <0x0 0x5801a000 0x0 0x2000>;
+ clocks = <&clk BM1880_CLK_UART_500M>,
+ <&clk BM1880_CLK_APB_UART>;
+ clock-names = "baudclk", "apb_pclk";
interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
reg-shift = <2>;
reg-io-width = <4>;
@@ -194,6 +200,9 @@
uart2: serial@5801C000 {
compatible = "snps,dw-apb-uart";
reg = <0x0 0x5801c000 0x0 0x2000>;
+ clocks = <&clk BM1880_CLK_UART_500M>,
+ <&clk BM1880_CLK_APB_UART>;
+ clock-names = "baudclk", "apb_pclk";
interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
reg-shift = <2>;
reg-io-width = <4>;
@@ -204,6 +213,9 @@
uart3: serial@5801E000 {
compatible = "snps,dw-apb-uart";
reg = <0x0 0x5801e000 0x0 0x2000>;
+ clocks = <&clk BM1880_CLK_UART_500M>,
+ <&clk BM1880_CLK_APB_UART>;
+ clock-names = "baudclk", "apb_pclk";
interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
reg-shift = <2>;
reg-io-width = <4>;
--
2.17.1

2019-09-16 21:35:47

by Manivannan Sadhasivam

[permalink] [raw]
Subject: [PATCH v5 8/8] MAINTAINERS: Add entry for BM1880 SoC clock driver

Add MAINTAINERS entry for Bitmain BM1880 SoC clock driver.

Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
MAINTAINERS | 2 ++
1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 997a4f8fe88e..280defec35b2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1503,8 +1503,10 @@ M: Manivannan Sadhasivam <[email protected]>
L: [email protected] (moderated for non-subscribers)
S: Maintained
F: arch/arm64/boot/dts/bitmain/
+F: drivers/clk/clk-bm1880.c
F: drivers/pinctrl/pinctrl-bm1880.c
F: Documentation/devicetree/bindings/arm/bitmain.yaml
+F: Documentation/devicetree/bindings/clock/bitmain,bm1880-clk.yaml
F: Documentation/devicetree/bindings/pinctrl/bitmain,bm1880-pinctrl.txt

ARM/CALXEDA HIGHBANK ARCHITECTURE
--
2.17.1

2019-09-16 21:36:12

by Manivannan Sadhasivam

[permalink] [raw]
Subject: [PATCH v5 3/8] clk: Add clk_hw_unregister_composite helper function definition

This function has been delcared but not defined anywhere. Hence, this
commit adds definition for it.

Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/clk/clk-composite.c | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
index 4d579f9d20f6..ccca58a6d271 100644
--- a/drivers/clk/clk-composite.c
+++ b/drivers/clk/clk-composite.c
@@ -344,3 +344,14 @@ void clk_unregister_composite(struct clk *clk)
clk_unregister(clk);
kfree(composite);
}
+
+void clk_hw_unregister_composite(struct clk_hw *hw)
+{
+ struct clk_composite *composite;
+
+ composite = to_clk_composite(hw);
+
+ clk_hw_unregister(hw);
+ kfree(composite);
+}
+EXPORT_SYMBOL_GPL(clk_hw_unregister_composite);
--
2.17.1

2019-09-17 01:02:46

by Manivannan Sadhasivam

[permalink] [raw]
Subject: [PATCH v5 1/8] clk: Zero init clk_init_data in helpers

The clk_init_data struct needs to be initialized to zero for the new
parent_map implementation to work correctly. Otherwise, the member which
is available first will get processed.

Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/clk/clk-composite.c | 2 +-
drivers/clk/clk-divider.c | 2 +-
drivers/clk/clk-fixed-rate.c | 2 +-
drivers/clk/clk-gate.c | 2 +-
drivers/clk/clk-mux.c | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
index b06038b8f658..4d579f9d20f6 100644
--- a/drivers/clk/clk-composite.c
+++ b/drivers/clk/clk-composite.c
@@ -208,7 +208,7 @@ struct clk_hw *clk_hw_register_composite(struct device *dev, const char *name,
unsigned long flags)
{
struct clk_hw *hw;
- struct clk_init_data init;
+ struct clk_init_data init = { NULL };
struct clk_composite *composite;
struct clk_ops *clk_composite_ops;
int ret;
diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index 3f9ff78c4a2a..65dd8137f9ec 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -471,7 +471,7 @@ static struct clk_hw *_register_divider(struct device *dev, const char *name,
{
struct clk_divider *div;
struct clk_hw *hw;
- struct clk_init_data init;
+ struct clk_init_data init = { NULL };
int ret;

if (clk_divider_flags & CLK_DIVIDER_HIWORD_MASK) {
diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c
index a7e4aef7a376..746c3ecdc5b3 100644
--- a/drivers/clk/clk-fixed-rate.c
+++ b/drivers/clk/clk-fixed-rate.c
@@ -58,7 +58,7 @@ struct clk_hw *clk_hw_register_fixed_rate_with_accuracy(struct device *dev,
{
struct clk_fixed_rate *fixed;
struct clk_hw *hw;
- struct clk_init_data init;
+ struct clk_init_data init = { NULL };
int ret;

/* allocate fixed-rate clock */
diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
index 1b99fc962745..8ed83ec730cb 100644
--- a/drivers/clk/clk-gate.c
+++ b/drivers/clk/clk-gate.c
@@ -141,7 +141,7 @@ struct clk_hw *clk_hw_register_gate(struct device *dev, const char *name,
{
struct clk_gate *gate;
struct clk_hw *hw;
- struct clk_init_data init;
+ struct clk_init_data init = { NULL };
int ret;

if (clk_gate_flags & CLK_GATE_HIWORD_MASK) {
diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
index 66e91f740508..2caa6b2a9ee5 100644
--- a/drivers/clk/clk-mux.c
+++ b/drivers/clk/clk-mux.c
@@ -153,7 +153,7 @@ struct clk_hw *clk_hw_register_mux_table(struct device *dev, const char *name,
{
struct clk_mux *mux;
struct clk_hw *hw;
- struct clk_init_data init;
+ struct clk_init_data init = { NULL };
u8 width = 0;
int ret;

--
2.17.1

2019-09-17 01:02:44

by Manivannan Sadhasivam

[permalink] [raw]
Subject: [PATCH v5 5/8] arm64: dts: bitmain: Add clock controller support for BM1880 SoC

Add clock controller support for Bitmain BM1880 SoC.

Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
arch/arm64/boot/dts/bitmain/bm1880.dtsi | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/bitmain/bm1880.dtsi b/arch/arm64/boot/dts/bitmain/bm1880.dtsi
index d65453f99a99..8471662413da 100644
--- a/arch/arm64/boot/dts/bitmain/bm1880.dtsi
+++ b/arch/arm64/boot/dts/bitmain/bm1880.dtsi
@@ -4,6 +4,7 @@
* Author: Manivannan Sadhasivam <[email protected]>
*/

+#include <dt-bindings/clock/bm1880-clock.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/reset/bitmain,bm1880-reset.h>

@@ -66,6 +67,12 @@
<GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
};

+ osc: osc {
+ compatible = "fixed-clock";
+ clock-frequency = <25000000>;
+ #clock-cells = <0>;
+ };
+
soc {
compatible = "simple-bus";
#address-cells = <2>;
@@ -94,6 +101,15 @@
reg = <0x400 0x120>;
};

+ clk: clock-controller@e8 {
+ compatible = "bitmain,bm1880-clk";
+ reg = <0xe8 0x0c>, <0x800 0xb0>;
+ reg-names = "pll", "sys";
+ clocks = <&osc>;
+ clock-names = "osc";
+ #clock-cells = <1>;
+ };
+
rst: reset-controller@c00 {
compatible = "bitmain,bm1880-reset";
reg = <0xc00 0x8>;
--
2.17.1

2019-09-17 02:37:24

by Manivannan Sadhasivam

[permalink] [raw]
Subject: [PATCH v5 4/8] dt-bindings: clock: Add devicetree binding for BM1880 SoC

Add YAML devicetree binding for Bitmain BM1880 SoC.

Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
.../bindings/clock/bitmain,bm1880-clk.yaml | 76 +++++++++++++++++
include/dt-bindings/clock/bm1880-clock.h | 82 +++++++++++++++++++
2 files changed, 158 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/bitmain,bm1880-clk.yaml
create mode 100644 include/dt-bindings/clock/bm1880-clock.h

diff --git a/Documentation/devicetree/bindings/clock/bitmain,bm1880-clk.yaml b/Documentation/devicetree/bindings/clock/bitmain,bm1880-clk.yaml
new file mode 100644
index 000000000000..e63827399c1a
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/bitmain,bm1880-clk.yaml
@@ -0,0 +1,76 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bindings/clock/bitmain,bm1880-clk.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Bitmain BM1880 Clock Controller
+
+maintainers:
+ - Manivannan Sadhasivam <[email protected]>
+
+description: |
+ The Bitmain BM1880 clock controller generates and supplies clock to
+ various peripherals within the SoC.
+
+ This binding uses common clock bindings
+ [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+properties:
+ compatible:
+ const: bitmain,bm1880-clk
+
+ reg:
+ items:
+ - description: pll registers
+ - description: system registers
+
+ reg-names:
+ items:
+ - const: pll
+ - const: sys
+
+ clocks:
+ maxItems: 1
+
+ clock-names:
+ const: osc
+
+ '#clock-cells':
+ const: 1
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - clocks
+ - clock-names
+ - '#clock-cells'
+
+additionalProperties: false
+
+examples:
+ # Clock controller node:
+ - |
+ clk: clock-controller@e8 {
+ compatible = "bitmain,bm1880-clk";
+ reg = <0xe8 0x0c>, <0x800 0xb0>;
+ reg-names = "pll", "sys";
+ clocks = <&osc>;
+ clock-names = "osc";
+ #clock-cells = <1>;
+ };
+
+ # Example UART controller node that consumes clock generated by the clock controller:
+ - |
+ uart0: serial@58018000 {
+ compatible = "snps,dw-apb-uart";
+ reg = <0x0 0x58018000 0x0 0x2000>;
+ clocks = <&clk 45>, <&clk 46>;
+ clock-names = "baudclk", "apb_pclk";
+ interrupts = <0 9 4>;
+ reg-shift = <2>;
+ reg-io-width = <4>;
+ };
+
+...
diff --git a/include/dt-bindings/clock/bm1880-clock.h b/include/dt-bindings/clock/bm1880-clock.h
new file mode 100644
index 000000000000..b46732361b25
--- /dev/null
+++ b/include/dt-bindings/clock/bm1880-clock.h
@@ -0,0 +1,82 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Device Tree binding constants for Bitmain BM1880 SoC
+ *
+ * Copyright (c) 2019 Linaro Ltd.
+ */
+
+#ifndef __DT_BINDINGS_CLOCK_BM1880_H
+#define __DT_BINDINGS_CLOCK_BM1880_H
+
+#define BM1880_CLK_OSC 0
+#define BM1880_CLK_MPLL 1
+#define BM1880_CLK_SPLL 2
+#define BM1880_CLK_FPLL 3
+#define BM1880_CLK_DDRPLL 4
+#define BM1880_CLK_A53 5
+#define BM1880_CLK_50M_A53 6
+#define BM1880_CLK_AHB_ROM 7
+#define BM1880_CLK_AXI_SRAM 8
+#define BM1880_CLK_DDR_AXI 9
+#define BM1880_CLK_EFUSE 10
+#define BM1880_CLK_APB_EFUSE 11
+#define BM1880_CLK_AXI5_EMMC 12
+#define BM1880_CLK_EMMC 13
+#define BM1880_CLK_100K_EMMC 14
+#define BM1880_CLK_AXI5_SD 15
+#define BM1880_CLK_SD 16
+#define BM1880_CLK_100K_SD 17
+#define BM1880_CLK_500M_ETH0 18
+#define BM1880_CLK_AXI4_ETH0 19
+#define BM1880_CLK_500M_ETH1 20
+#define BM1880_CLK_AXI4_ETH1 21
+#define BM1880_CLK_AXI1_GDMA 22
+#define BM1880_CLK_APB_GPIO 23
+#define BM1880_CLK_APB_GPIO_INTR 24
+#define BM1880_CLK_GPIO_DB 25
+#define BM1880_CLK_AXI1_MINER 26
+#define BM1880_CLK_AHB_SF 27
+#define BM1880_CLK_SDMA_AXI 28
+#define BM1880_CLK_SDMA_AUD 29
+#define BM1880_CLK_APB_I2C 30
+#define BM1880_CLK_APB_WDT 31
+#define BM1880_CLK_APB_JPEG 32
+#define BM1880_CLK_JPEG_AXI 33
+#define BM1880_CLK_AXI5_NF 34
+#define BM1880_CLK_APB_NF 35
+#define BM1880_CLK_NF 36
+#define BM1880_CLK_APB_PWM 37
+#define BM1880_CLK_DIV_0_RV 38
+#define BM1880_CLK_DIV_1_RV 39
+#define BM1880_CLK_MUX_RV 40
+#define BM1880_CLK_RV 41
+#define BM1880_CLK_APB_SPI 42
+#define BM1880_CLK_TPU_AXI 43
+#define BM1880_CLK_DIV_UART_500M 44
+#define BM1880_CLK_UART_500M 45
+#define BM1880_CLK_APB_UART 46
+#define BM1880_CLK_APB_I2S 47
+#define BM1880_CLK_AXI4_USB 48
+#define BM1880_CLK_APB_USB 49
+#define BM1880_CLK_125M_USB 50
+#define BM1880_CLK_33K_USB 51
+#define BM1880_CLK_DIV_12M_USB 52
+#define BM1880_CLK_12M_USB 53
+#define BM1880_CLK_APB_VIDEO 54
+#define BM1880_CLK_VIDEO_AXI 55
+#define BM1880_CLK_VPP_AXI 56
+#define BM1880_CLK_APB_VPP 57
+#define BM1880_CLK_DIV_0_AXI1 58
+#define BM1880_CLK_DIV_1_AXI1 59
+#define BM1880_CLK_AXI1 60
+#define BM1880_CLK_AXI2 61
+#define BM1880_CLK_AXI3 62
+#define BM1880_CLK_AXI4 63
+#define BM1880_CLK_AXI5 64
+#define BM1880_CLK_DIV_0_AXI6 65
+#define BM1880_CLK_DIV_1_AXI6 66
+#define BM1880_CLK_MUX_AXI6 67
+#define BM1880_CLK_AXI6 68
+#define BM1880_NR_CLKS 69
+
+#endif /* __DT_BINDINGS_CLOCK_BM1880_H */
--
2.17.1

2019-09-17 02:39:16

by Manivannan Sadhasivam

[permalink] [raw]
Subject: [PATCH v5 7/8] clk: Add common clock driver for BM1880 SoC

Add common clock driver for Bitmain BM1880 SoC. The clock controller on
BM1880 has supplies clocks to all peripherals in the form of gate clocks
and composite clocks (fixed factor + gate).

Signed-off-by: Manivannan Sadhasivam <[email protected]>
---
drivers/clk/Kconfig | 7 +
drivers/clk/Makefile | 1 +
drivers/clk/clk-bm1880.c | 966 +++++++++++++++++++++++++++++++++++++++
3 files changed, 974 insertions(+)
create mode 100644 drivers/clk/clk-bm1880.c

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 801fa1cd0321..e70c64e43ff9 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -139,6 +139,13 @@ config COMMON_CLK_SI570
This driver supports Silicon Labs 570/571/598/599 programmable
clock generators.

+config COMMON_CLK_BM1880
+ bool "Clock driver for Bitmain BM1880 SoC"
+ depends on ARCH_BITMAIN || COMPILE_TEST
+ default ARCH_BITMAIN
+ help
+ This driver supports the clocks on Bitmain BM1880 SoC.
+
config COMMON_CLK_CDCE706
tristate "Clock driver for TI CDCE706 clock synthesizer"
depends on I2C
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 0cad76021297..2c1ae6289a78 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_MACH_ASM9260) += clk-asm9260.o
obj-$(CONFIG_COMMON_CLK_AXI_CLKGEN) += clk-axi-clkgen.o
obj-$(CONFIG_ARCH_AXXIA) += clk-axm5516.o
obj-$(CONFIG_COMMON_CLK_BD718XX) += clk-bd718x7.o
+obj-$(CONFIG_COMMON_CLK_BM1880) += clk-bm1880.o
obj-$(CONFIG_COMMON_CLK_CDCE706) += clk-cdce706.o
obj-$(CONFIG_COMMON_CLK_CDCE925) += clk-cdce925.o
obj-$(CONFIG_ARCH_CLPS711X) += clk-clps711x.o
diff --git a/drivers/clk/clk-bm1880.c b/drivers/clk/clk-bm1880.c
new file mode 100644
index 000000000000..3b10de929fd4
--- /dev/null
+++ b/drivers/clk/clk-bm1880.c
@@ -0,0 +1,966 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Bitmain BM1880 SoC clock driver
+ *
+ * Copyright (c) 2019 Linaro Ltd.
+ * Author: Manivannan Sadhasivam <[email protected]>
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#include <dt-bindings/clock/bm1880-clock.h>
+
+#define BM1880_CLK_MPLL_CTL 0x00
+#define BM1880_CLK_SPLL_CTL 0x04
+#define BM1880_CLK_FPLL_CTL 0x08
+#define BM1880_CLK_DDRPLL_CTL 0x0c
+
+#define BM1880_CLK_ENABLE0 0x00
+#define BM1880_CLK_ENABLE1 0x04
+#define BM1880_CLK_SELECT 0x20
+#define BM1880_CLK_DIV0 0x40
+#define BM1880_CLK_DIV1 0x44
+#define BM1880_CLK_DIV2 0x48
+#define BM1880_CLK_DIV3 0x4c
+#define BM1880_CLK_DIV4 0x50
+#define BM1880_CLK_DIV5 0x54
+#define BM1880_CLK_DIV6 0x58
+#define BM1880_CLK_DIV7 0x5c
+#define BM1880_CLK_DIV8 0x60
+#define BM1880_CLK_DIV9 0x64
+#define BM1880_CLK_DIV10 0x68
+#define BM1880_CLK_DIV11 0x6c
+#define BM1880_CLK_DIV12 0x70
+#define BM1880_CLK_DIV13 0x74
+#define BM1880_CLK_DIV14 0x78
+#define BM1880_CLK_DIV15 0x7c
+#define BM1880_CLK_DIV16 0x80
+#define BM1880_CLK_DIV17 0x84
+#define BM1880_CLK_DIV18 0x88
+#define BM1880_CLK_DIV19 0x8c
+#define BM1880_CLK_DIV20 0x90
+#define BM1880_CLK_DIV21 0x94
+#define BM1880_CLK_DIV22 0x98
+#define BM1880_CLK_DIV23 0x9c
+#define BM1880_CLK_DIV24 0xa0
+#define BM1880_CLK_DIV25 0xa4
+#define BM1880_CLK_DIV26 0xa8
+#define BM1880_CLK_DIV27 0xac
+#define BM1880_CLK_DIV28 0xb0
+
+#define to_bm1880_pll_clk(_hw) container_of(_hw, struct bm1880_pll_hw_clock, hw)
+#define to_bm1880_div_clk(_hw) container_of(_hw, struct bm1880_div_hw_clock, hw)
+
+static DEFINE_SPINLOCK(bm1880_clk_lock);
+
+struct bm1880_clock_data {
+ void __iomem *pll_base;
+ void __iomem *sys_base;
+ struct clk_hw_onecell_data *clk_data;
+};
+
+struct bm1880_gate_clock {
+ unsigned int id;
+ const char *name;
+ const char *parent;
+ u32 gate_reg;
+ s8 gate_shift;
+ unsigned long flags;
+};
+
+struct bm1880_mux_clock {
+ unsigned int id;
+ const char *name;
+ const char * const *parents;
+ s8 num_parents;
+ u32 reg;
+ s8 shift;
+ unsigned long flags;
+};
+
+struct bm1880_div_clock {
+ unsigned int id;
+ const char *name;
+ u32 reg;
+ u8 shift;
+ u8 width;
+ u32 initval;
+ const struct clk_div_table *table;
+ unsigned long flags;
+};
+
+struct bm1880_div_hw_clock {
+ struct bm1880_div_clock div;
+ void __iomem *base;
+ spinlock_t *lock;
+ struct clk_hw hw;
+ struct clk_init_data init;
+};
+
+struct bm1880_composite_clock {
+ unsigned int id;
+ const char *name;
+ const char *parent;
+ const char * const *parents;
+ unsigned int num_parents;
+ unsigned long flags;
+
+ u32 gate_reg;
+ u32 mux_reg;
+ u32 div_reg;
+
+ s8 gate_shift;
+ s8 mux_shift;
+ s8 div_shift;
+ s8 div_width;
+ s16 div_initval;
+ const struct clk_div_table *table;
+};
+
+struct bm1880_pll_clock {
+ unsigned int id;
+ const char *name;
+ u32 reg;
+ unsigned long flags;
+};
+
+struct bm1880_pll_hw_clock {
+ struct bm1880_pll_clock pll;
+ void __iomem *base;
+ struct clk_hw hw;
+ struct clk_init_data init;
+};
+
+static const struct clk_ops bm1880_pll_ops;
+static const struct clk_ops bm1880_clk_div_ops;
+
+#define GATE_DIV(_id, _name, _parent, _gate_reg, _gate_shift, _div_reg, \
+ _div_shift, _div_width, _div_initval, _table, \
+ _flags) { \
+ .id = _id, \
+ .parent = _parent, \
+ .name = _name, \
+ .gate_reg = _gate_reg, \
+ .gate_shift = _gate_shift, \
+ .div_reg = _div_reg, \
+ .div_shift = _div_shift, \
+ .div_width = _div_width, \
+ .div_initval = _div_initval, \
+ .table = _table, \
+ .mux_shift = -1, \
+ .flags = _flags, \
+ }
+
+#define GATE_MUX(_id, _name, _parents, _gate_reg, _gate_shift, \
+ _mux_reg, _mux_shift, _flags) { \
+ .id = _id, \
+ .parents = _parents, \
+ .num_parents = ARRAY_SIZE(_parents), \
+ .name = _name, \
+ .gate_reg = _gate_reg, \
+ .gate_shift = _gate_shift, \
+ .div_shift = -1, \
+ .mux_reg = _mux_reg, \
+ .mux_shift = _mux_shift, \
+ .flags = _flags, \
+ }
+
+#define CLK_PLL(_id, _name, _parent, _reg, _flags) { \
+ .pll.id = _id, \
+ .pll.name = _name, \
+ .pll.reg = _reg, \
+ .hw.init = CLK_HW_INIT_PARENTS_DATA(_name, _parent, \
+ &bm1880_pll_ops, \
+ _flags), \
+ }
+
+#define CLK_DIV(_id, _name, _parent, _reg, _shift, _width, _initval, \
+ _table, _flags) { \
+ .div.id = _id, \
+ .div.name = _name, \
+ .div.reg = _reg, \
+ .div.shift = _shift, \
+ .div.width = _width, \
+ .div.initval = _initval, \
+ .div.table = _table, \
+ .hw.init = CLK_HW_INIT_HW(_name, _parent, \
+ &bm1880_clk_div_ops, \
+ _flags), \
+ }
+
+static struct clk_parent_data bm1880_pll_parent[] = {
+ { .fw_name = "osc", .name = "osc" },
+};
+
+/*
+ * All PLL clocks are marked as CRITICAL, hence they are very crucial
+ * for the functioning of the SoC
+ */
+static struct bm1880_pll_hw_clock bm1880_pll_clks[] = {
+ CLK_PLL(BM1880_CLK_MPLL, "clk_mpll", bm1880_pll_parent,
+ BM1880_CLK_MPLL_CTL, CLK_IS_CRITICAL),
+ CLK_PLL(BM1880_CLK_SPLL, "clk_spll", bm1880_pll_parent,
+ BM1880_CLK_SPLL_CTL, CLK_IS_CRITICAL),
+ CLK_PLL(BM1880_CLK_FPLL, "clk_fpll", bm1880_pll_parent,
+ BM1880_CLK_FPLL_CTL, CLK_IS_CRITICAL),
+ CLK_PLL(BM1880_CLK_DDRPLL, "clk_ddrpll", bm1880_pll_parent,
+ BM1880_CLK_DDRPLL_CTL, CLK_IS_CRITICAL),
+};
+
+/*
+ * Clocks marked as CRITICAL are needed for the proper functioning
+ * of the SoC.
+ */
+static const struct bm1880_gate_clock bm1880_gate_clks[] = {
+ { BM1880_CLK_AHB_ROM, "clk_ahb_rom", "clk_mux_axi6",
+ BM1880_CLK_ENABLE0, 2, CLK_IS_CRITICAL },
+ { BM1880_CLK_AXI_SRAM, "clk_axi_sram", "clk_axi1",
+ BM1880_CLK_ENABLE0, 3, CLK_IS_CRITICAL },
+ { BM1880_CLK_DDR_AXI, "clk_ddr_axi", "clk_mux_axi6",
+ BM1880_CLK_ENABLE0, 4, CLK_IS_CRITICAL },
+ { BM1880_CLK_APB_EFUSE, "clk_apb_efuse", "clk_mux_axi6",
+ BM1880_CLK_ENABLE0, 6, CLK_IS_CRITICAL },
+ { BM1880_CLK_AXI5_EMMC, "clk_axi5_emmc", "clk_axi5",
+ BM1880_CLK_ENABLE0, 7, 0 },
+ { BM1880_CLK_AXI5_SD, "clk_axi5_sd", "clk_axi5",
+ BM1880_CLK_ENABLE0, 10, 0 },
+ { BM1880_CLK_AXI4_ETH0, "clk_axi4_eth0", "clk_axi4",
+ BM1880_CLK_ENABLE0, 14, 0 },
+ { BM1880_CLK_AXI4_ETH1, "clk_axi4_eth1", "clk_axi4",
+ BM1880_CLK_ENABLE0, 16, 0 },
+ { BM1880_CLK_AXI1_GDMA, "clk_axi1_gdma", "clk_axi1",
+ BM1880_CLK_ENABLE0, 17, 0 },
+ /* Don't gate GPIO clocks as it is not owned by the GPIO driver */
+ { BM1880_CLK_APB_GPIO, "clk_apb_gpio", "clk_mux_axi6",
+ BM1880_CLK_ENABLE0, 18, CLK_IGNORE_UNUSED },
+ { BM1880_CLK_APB_GPIO_INTR, "clk_apb_gpio_intr", "clk_mux_axi6",
+ BM1880_CLK_ENABLE0, 19, CLK_IGNORE_UNUSED },
+ { BM1880_CLK_AXI1_MINER, "clk_axi1_miner", "clk_axi1",
+ BM1880_CLK_ENABLE0, 21, 0 },
+ { BM1880_CLK_AHB_SF, "clk_ahb_sf", "clk_mux_axi6",
+ BM1880_CLK_ENABLE0, 22, 0 },
+ { BM1880_CLK_SDMA_AXI, "clk_sdma_axi", "clk_axi5",
+ BM1880_CLK_ENABLE0, 23, 0 },
+ { BM1880_CLK_APB_I2C, "clk_apb_i2c", "clk_mux_axi6",
+ BM1880_CLK_ENABLE0, 25, 0 },
+ { BM1880_CLK_APB_WDT, "clk_apb_wdt", "clk_mux_axi6",
+ BM1880_CLK_ENABLE0, 26, 0 },
+ { BM1880_CLK_APB_JPEG, "clk_apb_jpeg", "clk_axi6",
+ BM1880_CLK_ENABLE0, 27, 0 },
+ { BM1880_CLK_AXI5_NF, "clk_axi5_nf", "clk_axi5",
+ BM1880_CLK_ENABLE0, 29, 0 },
+ { BM1880_CLK_APB_NF, "clk_apb_nf", "clk_axi6",
+ BM1880_CLK_ENABLE0, 30, 0 },
+ { BM1880_CLK_APB_PWM, "clk_apb_pwm", "clk_mux_axi6",
+ BM1880_CLK_ENABLE1, 0, 0 },
+ { BM1880_CLK_RV, "clk_rv", "clk_mux_rv",
+ BM1880_CLK_ENABLE1, 1, 0 },
+ { BM1880_CLK_APB_SPI, "clk_apb_spi", "clk_mux_axi6",
+ BM1880_CLK_ENABLE1, 2, 0 },
+ { BM1880_CLK_UART_500M, "clk_uart_500m", "clk_div_uart_500m",
+ BM1880_CLK_ENABLE1, 4, 0 },
+ { BM1880_CLK_APB_UART, "clk_apb_uart", "clk_axi6",
+ BM1880_CLK_ENABLE1, 5, 0 },
+ { BM1880_CLK_APB_I2S, "clk_apb_i2s", "clk_axi6",
+ BM1880_CLK_ENABLE1, 6, 0 },
+ { BM1880_CLK_AXI4_USB, "clk_axi4_usb", "clk_axi4",
+ BM1880_CLK_ENABLE1, 7, 0 },
+ { BM1880_CLK_APB_USB, "clk_apb_usb", "clk_axi6",
+ BM1880_CLK_ENABLE1, 8, 0 },
+ { BM1880_CLK_12M_USB, "clk_12m_usb", "clk_div_12m_usb",
+ BM1880_CLK_ENABLE1, 11, 0 },
+ { BM1880_CLK_APB_VIDEO, "clk_apb_video", "clk_axi6",
+ BM1880_CLK_ENABLE1, 12, 0 },
+ { BM1880_CLK_APB_VPP, "clk_apb_vpp", "clk_axi6",
+ BM1880_CLK_ENABLE1, 15, 0 },
+ { BM1880_CLK_AXI6, "clk_axi6", "clk_mux_axi6",
+ BM1880_CLK_ENABLE1, 21, CLK_IS_CRITICAL },
+};
+
+static const char * const clk_a53_parents[] = { "clk_spll", "clk_mpll" };
+static const char * const clk_rv_parents[] = { "clk_div_1_rv", "clk_div_0_rv" };
+static const char * const clk_axi1_parents[] = { "clk_div_1_axi1", "clk_div_0_axi1" };
+static const char * const clk_axi6_parents[] = { "clk_div_1_axi6", "clk_div_0_axi6" };
+
+static const struct bm1880_mux_clock bm1880_mux_clks[] = {
+ { BM1880_CLK_MUX_RV, "clk_mux_rv", clk_rv_parents, 2,
+ BM1880_CLK_SELECT, 1, 0 },
+ { BM1880_CLK_MUX_AXI6, "clk_mux_axi6", clk_axi6_parents, 2,
+ BM1880_CLK_SELECT, 3, 0 },
+};
+
+static const struct clk_div_table bm1880_div_table_0[] = {
+ { 0, 1 }, { 1, 2 }, { 2, 3 }, { 3, 4 },
+ { 4, 5 }, { 5, 6 }, { 6, 7 }, { 7, 8 },
+ { 8, 9 }, { 9, 10 }, { 10, 11 }, { 11, 12 },
+ { 12, 13 }, { 13, 14 }, { 14, 15 }, { 15, 16 },
+ { 16, 17 }, { 17, 18 }, { 18, 19 }, { 19, 20 },
+ { 20, 21 }, { 21, 22 }, { 22, 23 }, { 23, 24 },
+ { 24, 25 }, { 25, 26 }, { 26, 27 }, { 27, 28 },
+ { 28, 29 }, { 29, 30 }, { 30, 31 }, { 31, 32 },
+ { 0, 0 }
+};
+
+static const struct clk_div_table bm1880_div_table_1[] = {
+ { 0, 1 }, { 1, 2 }, { 2, 3 }, { 3, 4 },
+ { 4, 5 }, { 5, 6 }, { 6, 7 }, { 7, 8 },
+ { 8, 9 }, { 9, 10 }, { 10, 11 }, { 11, 12 },
+ { 12, 13 }, { 13, 14 }, { 14, 15 }, { 15, 16 },
+ { 16, 17 }, { 17, 18 }, { 18, 19 }, { 19, 20 },
+ { 20, 21 }, { 21, 22 }, { 22, 23 }, { 23, 24 },
+ { 24, 25 }, { 25, 26 }, { 26, 27 }, { 27, 28 },
+ { 28, 29 }, { 29, 30 }, { 30, 31 }, { 31, 32 },
+ { 127, 128 }, { 0, 0 }
+};
+
+static const struct clk_div_table bm1880_div_table_2[] = {
+ { 0, 1 }, { 1, 2 }, { 2, 3 }, { 3, 4 },
+ { 4, 5 }, { 5, 6 }, { 6, 7 }, { 7, 8 },
+ { 8, 9 }, { 9, 10 }, { 10, 11 }, { 11, 12 },
+ { 12, 13 }, { 13, 14 }, { 14, 15 }, { 15, 16 },
+ { 16, 17 }, { 17, 18 }, { 18, 19 }, { 19, 20 },
+ { 20, 21 }, { 21, 22 }, { 22, 23 }, { 23, 24 },
+ { 24, 25 }, { 25, 26 }, { 26, 27 }, { 27, 28 },
+ { 28, 29 }, { 29, 30 }, { 30, 31 }, { 31, 32 },
+ { 127, 128 }, { 255, 256 }, { 0, 0 }
+};
+
+static const struct clk_div_table bm1880_div_table_3[] = {
+ { 0, 1 }, { 1, 2 }, { 2, 3 }, { 3, 4 },
+ { 4, 5 }, { 5, 6 }, { 6, 7 }, { 7, 8 },
+ { 8, 9 }, { 9, 10 }, { 10, 11 }, { 11, 12 },
+ { 12, 13 }, { 13, 14 }, { 14, 15 }, { 15, 16 },
+ { 16, 17 }, { 17, 18 }, { 18, 19 }, { 19, 20 },
+ { 20, 21 }, { 21, 22 }, { 22, 23 }, { 23, 24 },
+ { 24, 25 }, { 25, 26 }, { 26, 27 }, { 27, 28 },
+ { 28, 29 }, { 29, 30 }, { 30, 31 }, { 31, 32 },
+ { 127, 128 }, { 255, 256 }, { 511, 512 }, { 0, 0 }
+};
+
+static const struct clk_div_table bm1880_div_table_4[] = {
+ { 0, 1 }, { 1, 2 }, { 2, 3 }, { 3, 4 },
+ { 4, 5 }, { 5, 6 }, { 6, 7 }, { 7, 8 },
+ { 8, 9 }, { 9, 10 }, { 10, 11 }, { 11, 12 },
+ { 12, 13 }, { 13, 14 }, { 14, 15 }, { 15, 16 },
+ { 16, 17 }, { 17, 18 }, { 18, 19 }, { 19, 20 },
+ { 20, 21 }, { 21, 22 }, { 22, 23 }, { 23, 24 },
+ { 24, 25 }, { 25, 26 }, { 26, 27 }, { 27, 28 },
+ { 28, 29 }, { 29, 30 }, { 30, 31 }, { 31, 32 },
+ { 127, 128 }, { 255, 256 }, { 511, 512 }, { 65535, 65536 },
+ { 0, 0 }
+};
+
+/*
+ * Clocks marked as CRITICAL are needed for the proper functioning
+ * of the SoC.
+ */
+static struct bm1880_div_hw_clock bm1880_div_clks[] = {
+ CLK_DIV(BM1880_CLK_DIV_0_RV, "clk_div_0_rv", &bm1880_pll_clks[1].hw,
+ BM1880_CLK_DIV12, 16, 5, 1, bm1880_div_table_0, 0),
+ CLK_DIV(BM1880_CLK_DIV_1_RV, "clk_div_1_rv", &bm1880_pll_clks[2].hw,
+ BM1880_CLK_DIV13, 16, 5, 1, bm1880_div_table_0, 0),
+ CLK_DIV(BM1880_CLK_DIV_UART_500M, "clk_div_uart_500m", &bm1880_pll_clks[2].hw,
+ BM1880_CLK_DIV15, 16, 7, 3, bm1880_div_table_1, 0),
+ CLK_DIV(BM1880_CLK_DIV_0_AXI1, "clk_div_0_axi1", &bm1880_pll_clks[0].hw,
+ BM1880_CLK_DIV21, 16, 5, 2, bm1880_div_table_0,
+ CLK_IS_CRITICAL),
+ CLK_DIV(BM1880_CLK_DIV_1_AXI1, "clk_div_1_axi1", &bm1880_pll_clks[2].hw,
+ BM1880_CLK_DIV22, 16, 5, 3, bm1880_div_table_0,
+ CLK_IS_CRITICAL),
+ CLK_DIV(BM1880_CLK_DIV_0_AXI6, "clk_div_0_axi6", &bm1880_pll_clks[2].hw,
+ BM1880_CLK_DIV27, 16, 5, 15, bm1880_div_table_0,
+ CLK_IS_CRITICAL),
+ CLK_DIV(BM1880_CLK_DIV_1_AXI6, "clk_div_1_axi6", &bm1880_pll_clks[0].hw,
+ BM1880_CLK_DIV28, 16, 5, 11, bm1880_div_table_0,
+ CLK_IS_CRITICAL),
+ CLK_DIV(BM1880_CLK_DIV_12M_USB, "clk_div_12m_usb", &bm1880_pll_clks[2].hw,
+ BM1880_CLK_DIV18, 16, 7, 125, bm1880_div_table_1, 0),
+};
+
+/*
+ * Clocks marked as CRITICAL are all needed for the proper functioning
+ * of the SoC.
+ */
+static struct bm1880_composite_clock bm1880_composite_clks[] = {
+ GATE_MUX(BM1880_CLK_A53, "clk_a53", clk_a53_parents,
+ BM1880_CLK_ENABLE0, 0, BM1880_CLK_SELECT, 0,
+ CLK_IS_CRITICAL),
+ GATE_DIV(BM1880_CLK_50M_A53, "clk_50m_a53", "clk_fpll",
+ BM1880_CLK_ENABLE0, 1, BM1880_CLK_DIV0, 16, 5, 30,
+ bm1880_div_table_0, CLK_IS_CRITICAL),
+ GATE_DIV(BM1880_CLK_EFUSE, "clk_efuse", "clk_fpll",
+ BM1880_CLK_ENABLE0, 5, BM1880_CLK_DIV1, 16, 7, 60,
+ bm1880_div_table_1, 0),
+ GATE_DIV(BM1880_CLK_EMMC, "clk_emmc", "clk_fpll",
+ BM1880_CLK_ENABLE0, 8, BM1880_CLK_DIV2, 16, 5, 15,
+ bm1880_div_table_0, 0),
+ GATE_DIV(BM1880_CLK_100K_EMMC, "clk_100k_emmc", "clk_div_12m_usb",
+ BM1880_CLK_ENABLE0, 9, BM1880_CLK_DIV3, 16, 8, 120,
+ bm1880_div_table_2, 0),
+ GATE_DIV(BM1880_CLK_SD, "clk_sd", "clk_fpll",
+ BM1880_CLK_ENABLE0, 11, BM1880_CLK_DIV4, 16, 5, 15,
+ bm1880_div_table_0, 0),
+ GATE_DIV(BM1880_CLK_100K_SD, "clk_100k_sd", "clk_div_12m_usb",
+ BM1880_CLK_ENABLE0, 12, BM1880_CLK_DIV5, 16, 8, 120,
+ bm1880_div_table_2, 0),
+ GATE_DIV(BM1880_CLK_500M_ETH0, "clk_500m_eth0", "clk_fpll",
+ BM1880_CLK_ENABLE0, 13, BM1880_CLK_DIV6, 16, 5, 3,
+ bm1880_div_table_0, 0),
+ GATE_DIV(BM1880_CLK_500M_ETH1, "clk_500m_eth1", "clk_fpll",
+ BM1880_CLK_ENABLE0, 15, BM1880_CLK_DIV7, 16, 5, 3,
+ bm1880_div_table_0, 0),
+ /* Don't gate GPIO clocks as it is not owned by the GPIO driver */
+ GATE_DIV(BM1880_CLK_GPIO_DB, "clk_gpio_db", "clk_div_12m_usb",
+ BM1880_CLK_ENABLE0, 20, BM1880_CLK_DIV8, 16, 16, 120,
+ bm1880_div_table_4, CLK_IGNORE_UNUSED),
+ GATE_DIV(BM1880_CLK_SDMA_AUD, "clk_sdma_aud", "clk_fpll",
+ BM1880_CLK_ENABLE0, 24, BM1880_CLK_DIV9, 16, 7, 61,
+ bm1880_div_table_1, 0),
+ GATE_DIV(BM1880_CLK_JPEG_AXI, "clk_jpeg_axi", "clk_fpll",
+ BM1880_CLK_ENABLE0, 28, BM1880_CLK_DIV10, 16, 5, 4,
+ bm1880_div_table_0, 0),
+ GATE_DIV(BM1880_CLK_NF, "clk_nf", "clk_fpll",
+ BM1880_CLK_ENABLE0, 31, BM1880_CLK_DIV11, 16, 5, 30,
+ bm1880_div_table_0, 0),
+ GATE_DIV(BM1880_CLK_TPU_AXI, "clk_tpu_axi", "clk_spll",
+ BM1880_CLK_ENABLE1, 3, BM1880_CLK_DIV14, 16, 5, 1,
+ bm1880_div_table_0, 0),
+ GATE_DIV(BM1880_CLK_125M_USB, "clk_125m_usb", "clk_fpll",
+ BM1880_CLK_ENABLE1, 9, BM1880_CLK_DIV16, 16, 5, 12,
+ bm1880_div_table_0, 0),
+ GATE_DIV(BM1880_CLK_33K_USB, "clk_33k_usb", "clk_div_12m_usb",
+ BM1880_CLK_ENABLE1, 10, BM1880_CLK_DIV17, 16, 9, 363,
+ bm1880_div_table_3, 0),
+ GATE_DIV(BM1880_CLK_VIDEO_AXI, "clk_video_axi", "clk_fpll",
+ BM1880_CLK_ENABLE1, 13, BM1880_CLK_DIV19, 16, 5, 4,
+ bm1880_div_table_0, 0),
+ GATE_DIV(BM1880_CLK_VPP_AXI, "clk_vpp_axi", "clk_fpll",
+ BM1880_CLK_ENABLE1, 14, BM1880_CLK_DIV20, 16, 5, 4,
+ bm1880_div_table_0, 0),
+ GATE_MUX(BM1880_CLK_AXI1, "clk_axi1", clk_axi1_parents,
+ BM1880_CLK_ENABLE1, 15, BM1880_CLK_SELECT, 2,
+ CLK_IS_CRITICAL),
+ GATE_DIV(BM1880_CLK_AXI2, "clk_axi2", "clk_fpll",
+ BM1880_CLK_ENABLE1, 17, BM1880_CLK_DIV23, 16, 5, 3,
+ bm1880_div_table_0, CLK_IS_CRITICAL),
+ GATE_DIV(BM1880_CLK_AXI3, "clk_axi3", "clk_mux_rv",
+ BM1880_CLK_ENABLE1, 18, BM1880_CLK_DIV24, 16, 5, 2,
+ bm1880_div_table_0, CLK_IS_CRITICAL),
+ GATE_DIV(BM1880_CLK_AXI4, "clk_axi4", "clk_fpll",
+ BM1880_CLK_ENABLE1, 19, BM1880_CLK_DIV25, 16, 5, 6,
+ bm1880_div_table_0, CLK_IS_CRITICAL),
+ GATE_DIV(BM1880_CLK_AXI5, "clk_axi5", "clk_fpll",
+ BM1880_CLK_ENABLE1, 20, BM1880_CLK_DIV26, 16, 5, 15,
+ bm1880_div_table_0, CLK_IS_CRITICAL),
+};
+
+static unsigned long bm1880_pll_rate_calc(u32 regval, unsigned long parent_rate)
+{
+ u32 fbdiv, fref, refdiv;
+ u32 postdiv1, postdiv2;
+ unsigned long rate, numerator, denominator;
+
+ fbdiv = (regval >> 16) & 0xfff;
+ fref = parent_rate;
+ refdiv = regval & 0x1f;
+ postdiv1 = (regval >> 8) & 0x7;
+ postdiv2 = (regval >> 12) & 0x7;
+
+ numerator = parent_rate * fbdiv;
+ denominator = refdiv * postdiv1 * postdiv2;
+ do_div(numerator, denominator);
+ rate = numerator;
+
+ return rate;
+}
+
+static unsigned long bm1880_pll_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct bm1880_pll_hw_clock *pll_hw = to_bm1880_pll_clk(hw);
+ unsigned long rate;
+ u32 regval;
+
+ regval = readl(pll_hw->base + pll_hw->pll.reg);
+ rate = bm1880_pll_rate_calc(regval, parent_rate);
+
+ return rate;
+}
+
+static const struct clk_ops bm1880_pll_ops = {
+ .recalc_rate = bm1880_pll_recalc_rate,
+};
+
+static struct clk_hw *bm1880_clk_register_pll(struct bm1880_pll_hw_clock *pll_clk,
+ void __iomem *sys_base)
+{
+ struct clk_hw *hw;
+ int err;
+
+ pll_clk->base = sys_base;
+ hw = &pll_clk->hw;
+
+ err = clk_hw_register(NULL, hw);
+ if (err)
+ return ERR_PTR(err);
+
+ return hw;
+}
+
+static void bm1880_clk_unregister_pll(struct clk_hw *hw)
+{
+ struct bm1880_pll_hw_clock *pll_hw = to_bm1880_pll_clk(hw);
+
+ clk_hw_unregister(hw);
+ kfree(pll_hw);
+}
+
+static int bm1880_clk_register_plls(struct bm1880_pll_hw_clock *clks,
+ int num_clks, struct bm1880_clock_data *data)
+{
+ struct clk_hw *hw;
+ void __iomem *pll_base = data->pll_base;
+ int i;
+
+ for (i = 0; i < num_clks; i++) {
+ struct bm1880_pll_hw_clock *bm1880_clk = &clks[i];
+
+ hw = bm1880_clk_register_pll(bm1880_clk, pll_base);
+ if (IS_ERR(hw)) {
+ pr_err("%s: failed to register clock %s\n",
+ __func__, bm1880_clk->pll.name);
+ goto err_clk;
+ }
+
+ data->clk_data->hws[clks[i].pll.id] = hw;
+ }
+
+ return 0;
+
+err_clk:
+ while (i--)
+ bm1880_clk_unregister_pll(data->clk_data->hws[clks[i].pll.id]);
+
+ return PTR_ERR(hw);
+}
+
+static int bm1880_clk_register_mux(const struct bm1880_mux_clock *clks,
+ int num_clks, struct bm1880_clock_data *data)
+{
+ struct clk_hw *hw;
+ void __iomem *sys_base = data->sys_base;
+ int i;
+
+ for (i = 0; i < num_clks; i++) {
+ hw = clk_hw_register_mux(NULL, clks[i].name,
+ clks[i].parents,
+ clks[i].num_parents,
+ clks[i].flags,
+ sys_base + clks[i].reg,
+ clks[i].shift, 1, 0,
+ &bm1880_clk_lock);
+ if (IS_ERR(hw)) {
+ pr_err("%s: failed to register clock %s\n",
+ __func__, clks[i].name);
+ goto err_clk;
+ }
+
+ data->clk_data->hws[clks[i].id] = hw;
+ }
+
+ return 0;
+
+err_clk:
+ while (i--)
+ clk_hw_unregister_mux(data->clk_data->hws[clks[i].id]);
+
+ return PTR_ERR(hw);
+}
+
+static unsigned long bm1880_clk_div_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct bm1880_div_hw_clock *div_hw = to_bm1880_div_clk(hw);
+ struct bm1880_div_clock *div = &div_hw->div;
+ void __iomem *reg_addr = div_hw->base + div->reg;
+ unsigned int val;
+ unsigned long rate;
+
+ if (!(readl(reg_addr) & BIT(3))) {
+ val = div->initval;
+ } else {
+ val = readl(reg_addr) >> div->shift;
+ val &= clk_div_mask(div->width);
+ }
+
+ rate = divider_recalc_rate(hw, parent_rate, val, div->table,
+ div->flags, div->width);
+
+ return rate;
+}
+
+static long bm1880_clk_div_round_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *prate)
+{
+ struct bm1880_div_hw_clock *div_hw = to_bm1880_div_clk(hw);
+ struct bm1880_div_clock *div = &div_hw->div;
+ void __iomem *reg_addr = div_hw->base + div->reg;
+
+ if (div->flags & CLK_DIVIDER_READ_ONLY) {
+ u32 val;
+
+ val = readl(reg_addr) >> div->shift;
+ val &= clk_div_mask(div->width);
+
+ return divider_ro_round_rate(hw, rate, prate, div->table,
+ div->width, div->flags,
+ val);
+ }
+
+ return divider_round_rate(hw, rate, prate, div->table,
+ div->width, div->flags);
+}
+
+static int bm1880_clk_div_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct bm1880_div_hw_clock *div_hw = to_bm1880_div_clk(hw);
+ struct bm1880_div_clock *div = &div_hw->div;
+ void __iomem *reg_addr = div_hw->base + div->reg;
+ unsigned long flags = 0;
+ int value;
+ u32 val;
+
+ value = divider_get_val(rate, parent_rate, div->table,
+ div->width, div_hw->div.flags);
+ if (value < 0)
+ return value;
+
+ if (div_hw->lock)
+ spin_lock_irqsave(div_hw->lock, flags);
+ else
+ __acquire(div_hw->lock);
+
+ if (div->flags & CLK_DIVIDER_HIWORD_MASK) {
+ val = clk_div_mask(div->width) << (div_hw->div.shift + 16);
+ } else {
+ val = readl(reg_addr);
+ val &= ~(clk_div_mask(div->width) << div_hw->div.shift);
+ }
+ val |= (u32)value << div->shift;
+ writel(val, reg_addr);
+
+ if (div_hw->lock)
+ spin_unlock_irqrestore(div_hw->lock, flags);
+ else
+ __release(div_hw->lock);
+
+ return 0;
+}
+
+static const struct clk_ops bm1880_clk_div_ops = {
+ .recalc_rate = bm1880_clk_div_recalc_rate,
+ .round_rate = bm1880_clk_div_round_rate,
+ .set_rate = bm1880_clk_div_set_rate,
+};
+
+static struct clk_hw *bm1880_clk_register_div(struct bm1880_div_hw_clock *div_clk,
+ void __iomem *sys_base)
+{
+ struct clk_hw *hw;
+ int err;
+
+ div_clk->div.flags = CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO;
+ div_clk->base = sys_base;
+ div_clk->lock = &bm1880_clk_lock;
+
+ hw = &div_clk->hw;
+ err = clk_hw_register(NULL, hw);
+ if (err)
+ return ERR_PTR(err);
+
+ return hw;
+}
+
+static void bm1880_clk_unregister_div(struct clk_hw *hw)
+{
+ struct bm1880_div_hw_clock *div_hw = to_bm1880_div_clk(hw);
+
+ clk_hw_unregister(hw);
+ kfree(div_hw);
+}
+
+static int bm1880_clk_register_divs(struct bm1880_div_hw_clock *clks,
+ int num_clks, struct bm1880_clock_data *data)
+{
+ struct clk_hw *hw;
+ void __iomem *sys_base = data->sys_base;
+ int i;
+
+ for (i = 0; i < num_clks; i++) {
+ struct bm1880_div_hw_clock *bm1880_clk = &clks[i];
+
+ hw = bm1880_clk_register_div(bm1880_clk, sys_base);
+ if (IS_ERR(hw)) {
+ pr_err("%s: failed to register clock %s\n",
+ __func__, bm1880_clk->div.name);
+ goto err_clk;
+ }
+
+ data->clk_data->hws[clks[i].div.id] = hw;
+ }
+
+ return 0;
+
+err_clk:
+ while (i--)
+ bm1880_clk_unregister_div(data->clk_data->hws[clks[i].div.id]);
+
+ return PTR_ERR(hw);
+}
+
+static int bm1880_clk_register_gate(const struct bm1880_gate_clock *clks,
+ int num_clks, struct bm1880_clock_data *data)
+{
+ struct clk_hw *hw;
+ void __iomem *sys_base = data->sys_base;
+ int i;
+
+ for (i = 0; i < num_clks; i++) {
+ hw = clk_hw_register_gate(NULL, clks[i].name,
+ clks[i].parent,
+ clks[i].flags,
+ sys_base + clks[i].gate_reg,
+ clks[i].gate_shift,
+ 0,
+ &bm1880_clk_lock);
+ if (IS_ERR(hw)) {
+ pr_err("%s: failed to register clock %s\n",
+ __func__, clks[i].name);
+ goto err_clk;
+ }
+
+ data->clk_data->hws[clks[i].id] = hw;
+ }
+
+ return 0;
+
+err_clk:
+ while (i--)
+ clk_hw_unregister_gate(data->clk_data->hws[clks[i].id]);
+
+ return PTR_ERR(hw);
+}
+
+static struct clk_hw *bm1880_clk_register_composite(struct bm1880_composite_clock *clks,
+ void __iomem *sys_base)
+{
+ struct clk_hw *hw;
+ struct clk_mux *mux = NULL;
+ struct clk_gate *gate = NULL;
+ struct bm1880_div_hw_clock *div_hws = NULL;
+ struct clk_hw *mux_hw = NULL, *gate_hw = NULL, *div_hw = NULL;
+ const struct clk_ops *mux_ops = NULL, *gate_ops = NULL, *div_ops = NULL;
+ const char * const *parent_names;
+ const char *parent;
+ int num_parents;
+ int ret;
+
+ if (clks->mux_shift >= 0) {
+ mux = kzalloc(sizeof(*mux), GFP_KERNEL);
+ if (!mux)
+ return ERR_PTR(-ENOMEM);
+
+ mux->reg = sys_base + clks->mux_reg;
+ mux->mask = 1;
+ mux->shift = clks->mux_shift;
+ mux_hw = &mux->hw;
+ mux_ops = &clk_mux_ops;
+ mux->lock = &bm1880_clk_lock;
+
+ parent_names = clks->parents;
+ num_parents = clks->num_parents;
+ } else {
+ parent = clks->parent;
+ parent_names = &parent;
+ num_parents = 1;
+ }
+
+ if (clks->gate_shift >= 0) {
+ gate = kzalloc(sizeof(*gate), GFP_KERNEL);
+ if (!gate) {
+ ret = -ENOMEM;
+ goto err_out;
+ }
+
+ gate->reg = sys_base + clks->gate_reg;
+ gate->bit_idx = clks->gate_shift;
+ gate->lock = &bm1880_clk_lock;
+
+ gate_hw = &gate->hw;
+ gate_ops = &clk_gate_ops;
+ }
+
+ if (clks->div_shift >= 0) {
+ div_hws = kzalloc(sizeof(*div_hws), GFP_KERNEL);
+ if (!div_hws) {
+ ret = -ENOMEM;
+ goto err_out;
+ }
+
+ div_hws->base = sys_base;
+ div_hws->div.reg = clks->div_reg;
+ div_hws->div.shift = clks->div_shift;
+ div_hws->div.width = clks->div_width;
+ div_hws->div.table = clks->table;
+ div_hws->div.initval = clks->div_initval;
+ div_hws->lock = &bm1880_clk_lock;
+ div_hws->div.flags = CLK_DIVIDER_ONE_BASED |
+ CLK_DIVIDER_ALLOW_ZERO;
+
+ div_hw = &div_hws->hw;
+ div_ops = &bm1880_clk_div_ops;
+ }
+
+ hw = clk_hw_register_composite(NULL, clks->name, parent_names,
+ num_parents, mux_hw, mux_ops, div_hw,
+ div_ops, gate_hw, gate_ops,
+ clks->flags);
+
+ if (IS_ERR(hw)) {
+ ret = PTR_ERR(hw);
+ goto err_out;
+ }
+
+ return hw;
+
+err_out:
+ kfree(div_hws);
+ kfree(gate);
+ kfree(mux);
+
+ return ERR_PTR(ret);
+}
+
+static int bm1880_clk_register_composites(struct bm1880_composite_clock *clks,
+ int num_clks, struct bm1880_clock_data *data)
+{
+ struct clk_hw *hw;
+ void __iomem *sys_base = data->sys_base;
+ int i;
+
+ for (i = 0; i < num_clks; i++) {
+ struct bm1880_composite_clock *bm1880_clk = &clks[i];
+
+ hw = bm1880_clk_register_composite(bm1880_clk, sys_base);
+ if (IS_ERR(hw)) {
+ pr_err("%s: failed to register clock %s\n",
+ __func__, bm1880_clk->name);
+ goto err_clk;
+ }
+
+ data->clk_data->hws[clks[i].id] = hw;
+ }
+
+ return 0;
+
+err_clk:
+ while (i--)
+ clk_hw_unregister_composite(data->clk_data->hws[clks[i].id]);
+
+ return PTR_ERR(hw);
+}
+
+static int bm1880_clk_probe(struct platform_device *pdev)
+{
+ struct bm1880_clock_data *clk_data;
+ void __iomem *pll_base, *sys_base;
+ struct device_node *np = pdev->dev.of_node;
+ struct device *dev = &pdev->dev;
+ struct resource *res;
+ struct clk_hw_onecell_data *clk_hw_data;
+ int num_clks, i;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ pll_base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(pll_base))
+ return PTR_ERR(pll_base);
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ sys_base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(sys_base))
+ return PTR_ERR(sys_base);
+
+ clk_data = devm_kzalloc(dev, sizeof(*clk_data), GFP_KERNEL);
+ if (!clk_data)
+ return -ENOMEM;
+
+ clk_data->pll_base = pll_base;
+ clk_data->sys_base = sys_base;
+
+ num_clks = ARRAY_SIZE(bm1880_pll_clks) +
+ ARRAY_SIZE(bm1880_div_clks) +
+ ARRAY_SIZE(bm1880_mux_clks) +
+ ARRAY_SIZE(bm1880_composite_clks) +
+ ARRAY_SIZE(bm1880_gate_clks);
+
+ clk_hw_data = devm_kzalloc(&pdev->dev, struct_size(clk_hw_data, hws,
+ num_clks), GFP_KERNEL);
+ if (!clk_hw_data)
+ return -ENOMEM;
+
+ clk_data->clk_data = clk_hw_data;
+
+ for (i = 0; i < num_clks; i++)
+ clk_data->clk_data->hws[i] = ERR_PTR(-ENOENT);
+
+ clk_data->clk_data->num = num_clks;
+
+ bm1880_clk_register_plls(bm1880_pll_clks,
+ ARRAY_SIZE(bm1880_pll_clks),
+ clk_data);
+
+ bm1880_clk_register_divs(bm1880_div_clks,
+ ARRAY_SIZE(bm1880_div_clks),
+ clk_data);
+
+ bm1880_clk_register_mux(bm1880_mux_clks,
+ ARRAY_SIZE(bm1880_mux_clks),
+ clk_data);
+
+ bm1880_clk_register_composites(bm1880_composite_clks,
+ ARRAY_SIZE(bm1880_composite_clks),
+ clk_data);
+
+ bm1880_clk_register_gate(bm1880_gate_clks,
+ ARRAY_SIZE(bm1880_gate_clks),
+ clk_data);
+
+ return of_clk_add_hw_provider(np, of_clk_hw_onecell_get,
+ clk_data->clk_data);
+}
+
+static const struct of_device_id bm1880_of_match[] = {
+ { .compatible = "bitmain,bm1880-clk", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, bm1880_of_match);
+
+static struct platform_driver bm1880_clk_driver = {
+ .driver = {
+ .name = "bm1880-clk",
+ .of_match_table = bm1880_of_match,
+ },
+ .probe = bm1880_clk_probe,
+};
+module_platform_driver(bm1880_clk_driver);
+
+MODULE_AUTHOR("Manivannan Sadhasivam <[email protected]>");
+MODULE_DESCRIPTION("Clock driver for Bitmain BM1880 SoC");
+MODULE_LICENSE("GPL v2");
--
2.17.1

2019-09-17 02:57:22

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v5 4/8] dt-bindings: clock: Add devicetree binding for BM1880 SoC

On Mon, Sep 16, 2019 at 11:15 AM Manivannan Sadhasivam
<[email protected]> wrote:
>
> Add YAML devicetree binding for Bitmain BM1880 SoC.
>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
> ---
> .../bindings/clock/bitmain,bm1880-clk.yaml | 76 +++++++++++++++++
> include/dt-bindings/clock/bm1880-clock.h | 82 +++++++++++++++++++
> 2 files changed, 158 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/bitmain,bm1880-clk.yaml
> create mode 100644 include/dt-bindings/clock/bm1880-clock.h

Reviewed-by: Rob Herring <[email protected]>

2019-09-17 20:44:21

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v5 1/8] clk: Zero init clk_init_data in helpers

Quoting Manivannan Sadhasivam (2019-09-16 09:14:40)
> The clk_init_data struct needs to be initialized to zero for the new
> parent_map implementation to work correctly. Otherwise, the member which
> is available first will get processed.
>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
> ---
> drivers/clk/clk-composite.c | 2 +-
> drivers/clk/clk-divider.c | 2 +-
> drivers/clk/clk-fixed-rate.c | 2 +-
> drivers/clk/clk-gate.c | 2 +-
> drivers/clk/clk-mux.c | 2 +-
> 5 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
> index b06038b8f658..4d579f9d20f6 100644
> --- a/drivers/clk/clk-composite.c
> +++ b/drivers/clk/clk-composite.c
> @@ -208,7 +208,7 @@ struct clk_hw *clk_hw_register_composite(struct device *dev, const char *name,
> unsigned long flags)
> {
> struct clk_hw *hw;
> - struct clk_init_data init;
> + struct clk_init_data init = { NULL };

I'd prefer { } because then we don't have to worry about ordering the
struct to have a pointer vs. something else first.

> struct clk_composite *composite;
> struct clk_ops *clk_composite_ops;
> int ret;

2019-09-17 20:44:27

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v5 3/8] clk: Add clk_hw_unregister_composite helper function definition

Quoting Manivannan Sadhasivam (2019-09-16 09:14:42)
> This function has been delcared but not defined anywhere. Hence, this
> commit adds definition for it.
>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
> ---

Can you add a fixes tag?

Fixes: 49cb392d3639 ("clk: composite: Add hw based registration APIs")

2019-09-18 00:32:23

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v5 2/8] clk: Warn if clk_init_data is not zero initialized

Quoting Manivannan Sadhasivam (2019-09-16 09:14:41)
> The new implementation for determining parent map uses multiple ways
> to pass parent info. The order in which it gets processed depends on
> the first available member. Hence, it is necessary to zero init the
> clk_init_data struct so that the expected member gets processed correctly.
> So, add a warning if multiple clk_init_data members are available during
> clk registration.
>
> Signed-off-by: Manivannan Sadhasivam <[email protected]>
> ---
> drivers/clk/clk.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index c0990703ce54..7d6d6984c979 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -3497,6 +3497,14 @@ static int clk_core_populate_parent_map(struct clk_core *core)
> if (!num_parents)
> return 0;
>
> + /*
> + * Check for non-zero initialized clk_init_data struct. This is
> + * required because, we only require one of the (parent_names/
> + * parent_data/parent_hws) to be set at a time. Otherwise, the
> + * current code would use first available member.
> + */
> + WARN_ON((parent_names && parent_data) || (parent_names && parent_hws));
> +

This will warn for many drivers because they set clk_init_data on the
stack and assign parent_names but let junk from the stack be assigned to
parent_data. The code uses parent_names first and then looks for
parent_data or parent_hws because of this fact of life that we've never
required clk_init_data to be initialized to all zero.

> /*
> * Avoid unnecessary string look-ups of clk_core's possible parents by
> * having a cache of names/clk_hw pointers to clk_core pointers.

2019-09-18 06:09:34

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v5 7/8] clk: Add common clock driver for BM1880 SoC

Quoting Manivannan Sadhasivam (2019-09-16 09:14:46)
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 801fa1cd0321..e70c64e43ff9 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -139,6 +139,13 @@ config COMMON_CLK_SI570
> This driver supports Silicon Labs 570/571/598/599 programmable
> clock generators.
>
> +config COMMON_CLK_BM1880
> + bool "Clock driver for Bitmain BM1880 SoC"

Can it be tristate?

> + depends on ARCH_BITMAIN || COMPILE_TEST
> + default ARCH_BITMAIN
> + help
> + This driver supports the clocks on Bitmain BM1880 SoC.
> +
> config COMMON_CLK_CDCE706
> tristate "Clock driver for TI CDCE706 clock synthesizer"
> depends on I2C
> diff --git a/drivers/clk/clk-bm1880.c b/drivers/clk/clk-bm1880.c
> new file mode 100644
> index 000000000000..3b10de929fd4
> --- /dev/null
> +++ b/drivers/clk/clk-bm1880.c
> @@ -0,0 +1,966 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Bitmain BM1880 SoC clock driver
> + *
> + * Copyright (c) 2019 Linaro Ltd.
> + * Author: Manivannan Sadhasivam <[email protected]>
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#include <dt-bindings/clock/bm1880-clock.h>
> +
> +#define BM1880_CLK_MPLL_CTL 0x00
> +#define BM1880_CLK_SPLL_CTL 0x04
> +#define BM1880_CLK_FPLL_CTL 0x08
> +#define BM1880_CLK_DDRPLL_CTL 0x0c
> +
> +#define BM1880_CLK_ENABLE0 0x00
> +#define BM1880_CLK_ENABLE1 0x04
> +#define BM1880_CLK_SELECT 0x20
> +#define BM1880_CLK_DIV0 0x40
> +#define BM1880_CLK_DIV1 0x44
> +#define BM1880_CLK_DIV2 0x48
> +#define BM1880_CLK_DIV3 0x4c
> +#define BM1880_CLK_DIV4 0x50
> +#define BM1880_CLK_DIV5 0x54
> +#define BM1880_CLK_DIV6 0x58
> +#define BM1880_CLK_DIV7 0x5c
> +#define BM1880_CLK_DIV8 0x60
> +#define BM1880_CLK_DIV9 0x64
> +#define BM1880_CLK_DIV10 0x68
> +#define BM1880_CLK_DIV11 0x6c
> +#define BM1880_CLK_DIV12 0x70
> +#define BM1880_CLK_DIV13 0x74
> +#define BM1880_CLK_DIV14 0x78
> +#define BM1880_CLK_DIV15 0x7c
> +#define BM1880_CLK_DIV16 0x80
> +#define BM1880_CLK_DIV17 0x84
> +#define BM1880_CLK_DIV18 0x88
> +#define BM1880_CLK_DIV19 0x8c
> +#define BM1880_CLK_DIV20 0x90
> +#define BM1880_CLK_DIV21 0x94
> +#define BM1880_CLK_DIV22 0x98
> +#define BM1880_CLK_DIV23 0x9c
> +#define BM1880_CLK_DIV24 0xa0
> +#define BM1880_CLK_DIV25 0xa4
> +#define BM1880_CLK_DIV26 0xa8
> +#define BM1880_CLK_DIV27 0xac
> +#define BM1880_CLK_DIV28 0xb0
> +
> +#define to_bm1880_pll_clk(_hw) container_of(_hw, struct bm1880_pll_hw_clock, hw)
> +#define to_bm1880_div_clk(_hw) container_of(_hw, struct bm1880_div_hw_clock, hw)
> +
> +static DEFINE_SPINLOCK(bm1880_clk_lock);
> +
> +struct bm1880_clock_data {
> + void __iomem *pll_base;
> + void __iomem *sys_base;
> + struct clk_hw_onecell_data *clk_data;

Please call it hw_data or onecell_data instead so that the code doesn't
read as clk_data->clk_data. Also, probably can just make it part of the
same struct instead of a pointer inside so that it can all be allocated
in one big chunk instead of in two.

> +};
> +
> +struct bm1880_gate_clock {
> + unsigned int id;
> + const char *name;
> + const char *parent;
> + u32 gate_reg;
> + s8 gate_shift;
> + unsigned long flags;
> +};
> +
> +struct bm1880_mux_clock {
> + unsigned int id;
> + const char *name;
> + const char * const *parents;
> + s8 num_parents;
> + u32 reg;
> + s8 shift;
> + unsigned long flags;
> +};
> +
> +struct bm1880_div_clock {
> + unsigned int id;
> + const char *name;
> + u32 reg;
> + u8 shift;
> + u8 width;
> + u32 initval;
> + const struct clk_div_table *table;
> + unsigned long flags;
> +};
> +
> +struct bm1880_div_hw_clock {
> + struct bm1880_div_clock div;
> + void __iomem *base;
> + spinlock_t *lock;
> + struct clk_hw hw;
> + struct clk_init_data init;
> +};
> +
> +struct bm1880_composite_clock {
> + unsigned int id;
> + const char *name;
> + const char *parent;
> + const char * const *parents;
> + unsigned int num_parents;
> + unsigned long flags;
> +
> + u32 gate_reg;
> + u32 mux_reg;
> + u32 div_reg;
> +
> + s8 gate_shift;
> + s8 mux_shift;
> + s8 div_shift;
> + s8 div_width;
> + s16 div_initval;
> + const struct clk_div_table *table;
> +};
> +
> +struct bm1880_pll_clock {
> + unsigned int id;
> + const char *name;
> + u32 reg;
> + unsigned long flags;
> +};
> +
> +struct bm1880_pll_hw_clock {
> + struct bm1880_pll_clock pll;
> + void __iomem *base;
> + struct clk_hw hw;
> + struct clk_init_data init;
> +};
> +
> +static const struct clk_ops bm1880_pll_ops;
> +static const struct clk_ops bm1880_clk_div_ops;
> +
> +#define GATE_DIV(_id, _name, _parent, _gate_reg, _gate_shift, _div_reg, \
> + _div_shift, _div_width, _div_initval, _table, \
> + _flags) { \
> + .id = _id, \
> + .parent = _parent, \
> + .name = _name, \
> + .gate_reg = _gate_reg, \
> + .gate_shift = _gate_shift, \
> + .div_reg = _div_reg, \
> + .div_shift = _div_shift, \
> + .div_width = _div_width, \
> + .div_initval = _div_initval, \
> + .table = _table, \
> + .mux_shift = -1, \
> + .flags = _flags, \
> + }
> +
> +#define GATE_MUX(_id, _name, _parents, _gate_reg, _gate_shift, \
> + _mux_reg, _mux_shift, _flags) { \
> + .id = _id, \
> + .parents = _parents, \
> + .num_parents = ARRAY_SIZE(_parents), \
> + .name = _name, \
> + .gate_reg = _gate_reg, \
> + .gate_shift = _gate_shift, \
> + .div_shift = -1, \
> + .mux_reg = _mux_reg, \
> + .mux_shift = _mux_shift, \
> + .flags = _flags, \
> + }
> +
> +#define CLK_PLL(_id, _name, _parent, _reg, _flags) { \
> + .pll.id = _id, \
> + .pll.name = _name, \
> + .pll.reg = _reg, \
> + .hw.init = CLK_HW_INIT_PARENTS_DATA(_name, _parent, \
> + &bm1880_pll_ops, \
> + _flags), \
> + }
> +
> +#define CLK_DIV(_id, _name, _parent, _reg, _shift, _width, _initval, \
> + _table, _flags) { \
> + .div.id = _id, \
> + .div.name = _name, \
> + .div.reg = _reg, \
> + .div.shift = _shift, \
> + .div.width = _width, \
> + .div.initval = _initval, \
> + .div.table = _table, \
> + .hw.init = CLK_HW_INIT_HW(_name, _parent, \
> + &bm1880_clk_div_ops, \
> + _flags), \
> + }
> +
> +static struct clk_parent_data bm1880_pll_parent[] = {
> + { .fw_name = "osc", .name = "osc" },
> +};
> +
> +/*
> + * All PLL clocks are marked as CRITICAL, hence they are very crucial
> + * for the functioning of the SoC

Please add more information besides crucial to function of the clk.
Basically describe what the PLLs are clocking and why those child clks
aren't enabled or marked as critical themselves. The usage of
CLK_IS_CRITICAL is too liberal in this driver so this needs to be
cleaned up. For example, clk_mpll shouldn't be marked critical, just the
a53 CPU clk that can source from it should be marked critical because
it's for the CPU running code. It's also odd that we would register gate
clks or the a53 clks if we don't expect to ever turn those clks off. Can
that be avoided so that we don't need to mark anything critical for this
path?

> + */
> +static struct bm1880_pll_hw_clock bm1880_pll_clks[] = {
> + CLK_PLL(BM1880_CLK_MPLL, "clk_mpll", bm1880_pll_parent,
> + BM1880_CLK_MPLL_CTL, CLK_IS_CRITICAL),
> + CLK_PLL(BM1880_CLK_SPLL, "clk_spll", bm1880_pll_parent,
> + BM1880_CLK_SPLL_CTL, CLK_IS_CRITICAL),
> + CLK_PLL(BM1880_CLK_FPLL, "clk_fpll", bm1880_pll_parent,
> + BM1880_CLK_FPLL_CTL, CLK_IS_CRITICAL),
> + CLK_PLL(BM1880_CLK_DDRPLL, "clk_ddrpll", bm1880_pll_parent,
> + BM1880_CLK_DDRPLL_CTL, CLK_IS_CRITICAL),
> +};
> +
> +/*
> + * Clocks marked as CRITICAL are needed for the proper functioning
> + * of the SoC.
> + */
> +static const struct bm1880_gate_clock bm1880_gate_clks[] = {
> + { BM1880_CLK_AHB_ROM, "clk_ahb_rom", "clk_mux_axi6",
> + BM1880_CLK_ENABLE0, 2, CLK_IS_CRITICAL },
> + { BM1880_CLK_AXI_SRAM, "clk_axi_sram", "clk_axi1",
> + BM1880_CLK_ENABLE0, 3, CLK_IS_CRITICAL },
> + { BM1880_CLK_DDR_AXI, "clk_ddr_axi", "clk_mux_axi6",
> + BM1880_CLK_ENABLE0, 4, CLK_IS_CRITICAL },
> + { BM1880_CLK_APB_EFUSE, "clk_apb_efuse", "clk_mux_axi6",
> + BM1880_CLK_ENABLE0, 6, CLK_IS_CRITICAL },
> + { BM1880_CLK_AXI5_EMMC, "clk_axi5_emmc", "clk_axi5",
> + BM1880_CLK_ENABLE0, 7, 0 },
> + { BM1880_CLK_AXI5_SD, "clk_axi5_sd", "clk_axi5",
> + BM1880_CLK_ENABLE0, 10, 0 },
> + { BM1880_CLK_AXI4_ETH0, "clk_axi4_eth0", "clk_axi4",
> + BM1880_CLK_ENABLE0, 14, 0 },
> + { BM1880_CLK_AXI4_ETH1, "clk_axi4_eth1", "clk_axi4",
> + BM1880_CLK_ENABLE0, 16, 0 },
> + { BM1880_CLK_AXI1_GDMA, "clk_axi1_gdma", "clk_axi1",
> + BM1880_CLK_ENABLE0, 17, 0 },
> + /* Don't gate GPIO clocks as it is not owned by the GPIO driver */
> + { BM1880_CLK_APB_GPIO, "clk_apb_gpio", "clk_mux_axi6",
> + BM1880_CLK_ENABLE0, 18, CLK_IGNORE_UNUSED },
> + { BM1880_CLK_APB_GPIO_INTR, "clk_apb_gpio_intr", "clk_mux_axi6",
> + BM1880_CLK_ENABLE0, 19, CLK_IGNORE_UNUSED },
> + { BM1880_CLK_AXI1_MINER, "clk_axi1_miner", "clk_axi1",
> + BM1880_CLK_ENABLE0, 21, 0 },
> + { BM1880_CLK_AHB_SF, "clk_ahb_sf", "clk_mux_axi6",
> + BM1880_CLK_ENABLE0, 22, 0 },
> + { BM1880_CLK_SDMA_AXI, "clk_sdma_axi", "clk_axi5",
> + BM1880_CLK_ENABLE0, 23, 0 },
> + { BM1880_CLK_APB_I2C, "clk_apb_i2c", "clk_mux_axi6",
> + BM1880_CLK_ENABLE0, 25, 0 },
> + { BM1880_CLK_APB_WDT, "clk_apb_wdt", "clk_mux_axi6",
> + BM1880_CLK_ENABLE0, 26, 0 },
> + { BM1880_CLK_APB_JPEG, "clk_apb_jpeg", "clk_axi6",
> + BM1880_CLK_ENABLE0, 27, 0 },
> + { BM1880_CLK_AXI5_NF, "clk_axi5_nf", "clk_axi5",
> + BM1880_CLK_ENABLE0, 29, 0 },
> + { BM1880_CLK_APB_NF, "clk_apb_nf", "clk_axi6",
> + BM1880_CLK_ENABLE0, 30, 0 },
> + { BM1880_CLK_APB_PWM, "clk_apb_pwm", "clk_mux_axi6",
> + BM1880_CLK_ENABLE1, 0, 0 },
> + { BM1880_CLK_RV, "clk_rv", "clk_mux_rv",
> + BM1880_CLK_ENABLE1, 1, 0 },
> + { BM1880_CLK_APB_SPI, "clk_apb_spi", "clk_mux_axi6",
> + BM1880_CLK_ENABLE1, 2, 0 },
> + { BM1880_CLK_UART_500M, "clk_uart_500m", "clk_div_uart_500m",
> + BM1880_CLK_ENABLE1, 4, 0 },
> + { BM1880_CLK_APB_UART, "clk_apb_uart", "clk_axi6",
> + BM1880_CLK_ENABLE1, 5, 0 },
> + { BM1880_CLK_APB_I2S, "clk_apb_i2s", "clk_axi6",
> + BM1880_CLK_ENABLE1, 6, 0 },
> + { BM1880_CLK_AXI4_USB, "clk_axi4_usb", "clk_axi4",
> + BM1880_CLK_ENABLE1, 7, 0 },
> + { BM1880_CLK_APB_USB, "clk_apb_usb", "clk_axi6",
> + BM1880_CLK_ENABLE1, 8, 0 },
> + { BM1880_CLK_12M_USB, "clk_12m_usb", "clk_div_12m_usb",
> + BM1880_CLK_ENABLE1, 11, 0 },
> + { BM1880_CLK_APB_VIDEO, "clk_apb_video", "clk_axi6",
> + BM1880_CLK_ENABLE1, 12, 0 },
> + { BM1880_CLK_APB_VPP, "clk_apb_vpp", "clk_axi6",
> + BM1880_CLK_ENABLE1, 15, 0 },
> + { BM1880_CLK_AXI6, "clk_axi6", "clk_mux_axi6",
> + BM1880_CLK_ENABLE1, 21, CLK_IS_CRITICAL },
> +};
> +
> +static const char * const clk_a53_parents[] = { "clk_spll", "clk_mpll" };
> +static const char * const clk_rv_parents[] = { "clk_div_1_rv", "clk_div_0_rv" };
> +static const char * const clk_axi1_parents[] = { "clk_div_1_axi1", "clk_div_0_axi1" };
> +static const char * const clk_axi6_parents[] = { "clk_div_1_axi6", "clk_div_0_axi6" };

I sent some patches to make the basic clk types support the new way of
specifying parents. Can you use those patches instead of listing all
these strings?

> +
> +static const struct bm1880_mux_clock bm1880_mux_clks[] = {
> + { BM1880_CLK_MUX_RV, "clk_mux_rv", clk_rv_parents, 2,
> + BM1880_CLK_SELECT, 1, 0 },
> + { BM1880_CLK_MUX_AXI6, "clk_mux_axi6", clk_axi6_parents, 2,
> + BM1880_CLK_SELECT, 3, 0 },
> +};
> +
> +static const struct clk_div_table bm1880_div_table_0[] = {
> + { 0, 1 }, { 1, 2 }, { 2, 3 }, { 3, 4 },
> + { 4, 5 }, { 5, 6 }, { 6, 7 }, { 7, 8 },
> + { 8, 9 }, { 9, 10 }, { 10, 11 }, { 11, 12 },
> + { 12, 13 }, { 13, 14 }, { 14, 15 }, { 15, 16 },
> + { 16, 17 }, { 17, 18 }, { 18, 19 }, { 19, 20 },
> + { 20, 21 }, { 21, 22 }, { 22, 23 }, { 23, 24 },
> + { 24, 25 }, { 25, 26 }, { 26, 27 }, { 27, 28 },
> + { 28, 29 }, { 29, 30 }, { 30, 31 }, { 31, 32 },
> + { 0, 0 }
> +};
> +
> +static const struct clk_div_table bm1880_div_table_1[] = {
> + { 0, 1 }, { 1, 2 }, { 2, 3 }, { 3, 4 },
> + { 4, 5 }, { 5, 6 }, { 6, 7 }, { 7, 8 },
> + { 8, 9 }, { 9, 10 }, { 10, 11 }, { 11, 12 },
> + { 12, 13 }, { 13, 14 }, { 14, 15 }, { 15, 16 },
> + { 16, 17 }, { 17, 18 }, { 18, 19 }, { 19, 20 },
> + { 20, 21 }, { 21, 22 }, { 22, 23 }, { 23, 24 },
> + { 24, 25 }, { 25, 26 }, { 26, 27 }, { 27, 28 },
> + { 28, 29 }, { 29, 30 }, { 30, 31 }, { 31, 32 },
> + { 127, 128 }, { 0, 0 }
> +};
> +
> +static const struct clk_div_table bm1880_div_table_2[] = {
> + { 0, 1 }, { 1, 2 }, { 2, 3 }, { 3, 4 },
> + { 4, 5 }, { 5, 6 }, { 6, 7 }, { 7, 8 },
> + { 8, 9 }, { 9, 10 }, { 10, 11 }, { 11, 12 },
> + { 12, 13 }, { 13, 14 }, { 14, 15 }, { 15, 16 },
> + { 16, 17 }, { 17, 18 }, { 18, 19 }, { 19, 20 },
> + { 20, 21 }, { 21, 22 }, { 22, 23 }, { 23, 24 },
> + { 24, 25 }, { 25, 26 }, { 26, 27 }, { 27, 28 },
> + { 28, 29 }, { 29, 30 }, { 30, 31 }, { 31, 32 },
> + { 127, 128 }, { 255, 256 }, { 0, 0 }
> +};
> +
> +static const struct clk_div_table bm1880_div_table_3[] = {
> + { 0, 1 }, { 1, 2 }, { 2, 3 }, { 3, 4 },
> + { 4, 5 }, { 5, 6 }, { 6, 7 }, { 7, 8 },
> + { 8, 9 }, { 9, 10 }, { 10, 11 }, { 11, 12 },
> + { 12, 13 }, { 13, 14 }, { 14, 15 }, { 15, 16 },
> + { 16, 17 }, { 17, 18 }, { 18, 19 }, { 19, 20 },
> + { 20, 21 }, { 21, 22 }, { 22, 23 }, { 23, 24 },
> + { 24, 25 }, { 25, 26 }, { 26, 27 }, { 27, 28 },
> + { 28, 29 }, { 29, 30 }, { 30, 31 }, { 31, 32 },
> + { 127, 128 }, { 255, 256 }, { 511, 512 }, { 0, 0 }
> +};
> +
> +static const struct clk_div_table bm1880_div_table_4[] = {
> + { 0, 1 }, { 1, 2 }, { 2, 3 }, { 3, 4 },
> + { 4, 5 }, { 5, 6 }, { 6, 7 }, { 7, 8 },
> + { 8, 9 }, { 9, 10 }, { 10, 11 }, { 11, 12 },
> + { 12, 13 }, { 13, 14 }, { 14, 15 }, { 15, 16 },
> + { 16, 17 }, { 17, 18 }, { 18, 19 }, { 19, 20 },
> + { 20, 21 }, { 21, 22 }, { 22, 23 }, { 23, 24 },
> + { 24, 25 }, { 25, 26 }, { 26, 27 }, { 27, 28 },
> + { 28, 29 }, { 29, 30 }, { 30, 31 }, { 31, 32 },
> + { 127, 128 }, { 255, 256 }, { 511, 512 }, { 65535, 65536 },
> + { 0, 0 }
> +};
> +
> +/*
> + * Clocks marked as CRITICAL are needed for the proper functioning
> + * of the SoC.
> + */
> +static struct bm1880_div_hw_clock bm1880_div_clks[] = {
> + CLK_DIV(BM1880_CLK_DIV_0_RV, "clk_div_0_rv", &bm1880_pll_clks[1].hw,
> + BM1880_CLK_DIV12, 16, 5, 1, bm1880_div_table_0, 0),
> + CLK_DIV(BM1880_CLK_DIV_1_RV, "clk_div_1_rv", &bm1880_pll_clks[2].hw,
> + BM1880_CLK_DIV13, 16, 5, 1, bm1880_div_table_0, 0),
> + CLK_DIV(BM1880_CLK_DIV_UART_500M, "clk_div_uart_500m", &bm1880_pll_clks[2].hw,
> + BM1880_CLK_DIV15, 16, 7, 3, bm1880_div_table_1, 0),
> + CLK_DIV(BM1880_CLK_DIV_0_AXI1, "clk_div_0_axi1", &bm1880_pll_clks[0].hw,
> + BM1880_CLK_DIV21, 16, 5, 2, bm1880_div_table_0,
> + CLK_IS_CRITICAL),
> + CLK_DIV(BM1880_CLK_DIV_1_AXI1, "clk_div_1_axi1", &bm1880_pll_clks[2].hw,
> + BM1880_CLK_DIV22, 16, 5, 3, bm1880_div_table_0,
> + CLK_IS_CRITICAL),
> + CLK_DIV(BM1880_CLK_DIV_0_AXI6, "clk_div_0_axi6", &bm1880_pll_clks[2].hw,
> + BM1880_CLK_DIV27, 16, 5, 15, bm1880_div_table_0,
> + CLK_IS_CRITICAL),
> + CLK_DIV(BM1880_CLK_DIV_1_AXI6, "clk_div_1_axi6", &bm1880_pll_clks[0].hw,
> + BM1880_CLK_DIV28, 16, 5, 11, bm1880_div_table_0,
> + CLK_IS_CRITICAL),
> + CLK_DIV(BM1880_CLK_DIV_12M_USB, "clk_div_12m_usb", &bm1880_pll_clks[2].hw,
> + BM1880_CLK_DIV18, 16, 7, 125, bm1880_div_table_1, 0),
> +};
> +
> +/*
> + * Clocks marked as CRITICAL are all needed for the proper functioning
> + * of the SoC.
> + */
> +static struct bm1880_composite_clock bm1880_composite_clks[] = {
> + GATE_MUX(BM1880_CLK_A53, "clk_a53", clk_a53_parents,
> + BM1880_CLK_ENABLE0, 0, BM1880_CLK_SELECT, 0,
> + CLK_IS_CRITICAL),
> + GATE_DIV(BM1880_CLK_50M_A53, "clk_50m_a53", "clk_fpll",
> + BM1880_CLK_ENABLE0, 1, BM1880_CLK_DIV0, 16, 5, 30,
> + bm1880_div_table_0, CLK_IS_CRITICAL),
> + GATE_DIV(BM1880_CLK_EFUSE, "clk_efuse", "clk_fpll",
> + BM1880_CLK_ENABLE0, 5, BM1880_CLK_DIV1, 16, 7, 60,
> + bm1880_div_table_1, 0),
> + GATE_DIV(BM1880_CLK_EMMC, "clk_emmc", "clk_fpll",
> + BM1880_CLK_ENABLE0, 8, BM1880_CLK_DIV2, 16, 5, 15,
> + bm1880_div_table_0, 0),
> + GATE_DIV(BM1880_CLK_100K_EMMC, "clk_100k_emmc", "clk_div_12m_usb",
> + BM1880_CLK_ENABLE0, 9, BM1880_CLK_DIV3, 16, 8, 120,
> + bm1880_div_table_2, 0),
> + GATE_DIV(BM1880_CLK_SD, "clk_sd", "clk_fpll",
> + BM1880_CLK_ENABLE0, 11, BM1880_CLK_DIV4, 16, 5, 15,
> + bm1880_div_table_0, 0),
> + GATE_DIV(BM1880_CLK_100K_SD, "clk_100k_sd", "clk_div_12m_usb",
> + BM1880_CLK_ENABLE0, 12, BM1880_CLK_DIV5, 16, 8, 120,
> + bm1880_div_table_2, 0),
> + GATE_DIV(BM1880_CLK_500M_ETH0, "clk_500m_eth0", "clk_fpll",
> + BM1880_CLK_ENABLE0, 13, BM1880_CLK_DIV6, 16, 5, 3,
> + bm1880_div_table_0, 0),
> + GATE_DIV(BM1880_CLK_500M_ETH1, "clk_500m_eth1", "clk_fpll",
> + BM1880_CLK_ENABLE0, 15, BM1880_CLK_DIV7, 16, 5, 3,
> + bm1880_div_table_0, 0),
> + /* Don't gate GPIO clocks as it is not owned by the GPIO driver */
> + GATE_DIV(BM1880_CLK_GPIO_DB, "clk_gpio_db", "clk_div_12m_usb",
> + BM1880_CLK_ENABLE0, 20, BM1880_CLK_DIV8, 16, 16, 120,
> + bm1880_div_table_4, CLK_IGNORE_UNUSED),
> + GATE_DIV(BM1880_CLK_SDMA_AUD, "clk_sdma_aud", "clk_fpll",
> + BM1880_CLK_ENABLE0, 24, BM1880_CLK_DIV9, 16, 7, 61,
> + bm1880_div_table_1, 0),
> + GATE_DIV(BM1880_CLK_JPEG_AXI, "clk_jpeg_axi", "clk_fpll",
> + BM1880_CLK_ENABLE0, 28, BM1880_CLK_DIV10, 16, 5, 4,
> + bm1880_div_table_0, 0),
> + GATE_DIV(BM1880_CLK_NF, "clk_nf", "clk_fpll",
> + BM1880_CLK_ENABLE0, 31, BM1880_CLK_DIV11, 16, 5, 30,
> + bm1880_div_table_0, 0),
> + GATE_DIV(BM1880_CLK_TPU_AXI, "clk_tpu_axi", "clk_spll",
> + BM1880_CLK_ENABLE1, 3, BM1880_CLK_DIV14, 16, 5, 1,
> + bm1880_div_table_0, 0),
> + GATE_DIV(BM1880_CLK_125M_USB, "clk_125m_usb", "clk_fpll",
> + BM1880_CLK_ENABLE1, 9, BM1880_CLK_DIV16, 16, 5, 12,
> + bm1880_div_table_0, 0),
> + GATE_DIV(BM1880_CLK_33K_USB, "clk_33k_usb", "clk_div_12m_usb",
> + BM1880_CLK_ENABLE1, 10, BM1880_CLK_DIV17, 16, 9, 363,
> + bm1880_div_table_3, 0),
> + GATE_DIV(BM1880_CLK_VIDEO_AXI, "clk_video_axi", "clk_fpll",
> + BM1880_CLK_ENABLE1, 13, BM1880_CLK_DIV19, 16, 5, 4,
> + bm1880_div_table_0, 0),
> + GATE_DIV(BM1880_CLK_VPP_AXI, "clk_vpp_axi", "clk_fpll",
> + BM1880_CLK_ENABLE1, 14, BM1880_CLK_DIV20, 16, 5, 4,
> + bm1880_div_table_0, 0),
> + GATE_MUX(BM1880_CLK_AXI1, "clk_axi1", clk_axi1_parents,
> + BM1880_CLK_ENABLE1, 15, BM1880_CLK_SELECT, 2,
> + CLK_IS_CRITICAL),
> + GATE_DIV(BM1880_CLK_AXI2, "clk_axi2", "clk_fpll",
> + BM1880_CLK_ENABLE1, 17, BM1880_CLK_DIV23, 16, 5, 3,
> + bm1880_div_table_0, CLK_IS_CRITICAL),
> + GATE_DIV(BM1880_CLK_AXI3, "clk_axi3", "clk_mux_rv",
> + BM1880_CLK_ENABLE1, 18, BM1880_CLK_DIV24, 16, 5, 2,
> + bm1880_div_table_0, CLK_IS_CRITICAL),
> + GATE_DIV(BM1880_CLK_AXI4, "clk_axi4", "clk_fpll",
> + BM1880_CLK_ENABLE1, 19, BM1880_CLK_DIV25, 16, 5, 6,
> + bm1880_div_table_0, CLK_IS_CRITICAL),
> + GATE_DIV(BM1880_CLK_AXI5, "clk_axi5", "clk_fpll",
> + BM1880_CLK_ENABLE1, 20, BM1880_CLK_DIV26, 16, 5, 15,
> + bm1880_div_table_0, CLK_IS_CRITICAL),
> +};
> +
> +static unsigned long bm1880_pll_rate_calc(u32 regval, unsigned long parent_rate)
> +{
> + u32 fbdiv, fref, refdiv;
> + u32 postdiv1, postdiv2;
> + unsigned long rate, numerator, denominator;
> +
> + fbdiv = (regval >> 16) & 0xfff;
> + fref = parent_rate;
> + refdiv = regval & 0x1f;
> + postdiv1 = (regval >> 8) & 0x7;
> + postdiv2 = (regval >> 12) & 0x7;
> +
> + numerator = parent_rate * fbdiv;
> + denominator = refdiv * postdiv1 * postdiv2;
> + do_div(numerator, denominator);
> + rate = numerator;
> +
> + return rate;
> +}
> +
> +static unsigned long bm1880_pll_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct bm1880_pll_hw_clock *pll_hw = to_bm1880_pll_clk(hw);
> + unsigned long rate;
> + u32 regval;
> +
> + regval = readl(pll_hw->base + pll_hw->pll.reg);
> + rate = bm1880_pll_rate_calc(regval, parent_rate);
> +
> + return rate;
> +}
> +
> +static const struct clk_ops bm1880_pll_ops = {
> + .recalc_rate = bm1880_pll_recalc_rate,
> +};
> +
> +static struct clk_hw *bm1880_clk_register_pll(struct bm1880_pll_hw_clock *pll_clk,
> + void __iomem *sys_base)
> +{
> + struct clk_hw *hw;
> + int err;
> +
> + pll_clk->base = sys_base;
> + hw = &pll_clk->hw;
> +
> + err = clk_hw_register(NULL, hw);
> + if (err)
> + return ERR_PTR(err);
> +
> + return hw;
> +}
> +
> +static void bm1880_clk_unregister_pll(struct clk_hw *hw)
> +{
> + struct bm1880_pll_hw_clock *pll_hw = to_bm1880_pll_clk(hw);
> +
> + clk_hw_unregister(hw);
> + kfree(pll_hw);
> +}
> +
> +static int bm1880_clk_register_plls(struct bm1880_pll_hw_clock *clks,
> + int num_clks, struct bm1880_clock_data *data)
> +{
> + struct clk_hw *hw;
> + void __iomem *pll_base = data->pll_base;
> + int i;
> +
> + for (i = 0; i < num_clks; i++) {
> + struct bm1880_pll_hw_clock *bm1880_clk = &clks[i];
> +
> + hw = bm1880_clk_register_pll(bm1880_clk, pll_base);
> + if (IS_ERR(hw)) {
> + pr_err("%s: failed to register clock %s\n",
> + __func__, bm1880_clk->pll.name);
> + goto err_clk;
> + }
> +
> + data->clk_data->hws[clks[i].pll.id] = hw;
> + }
> +
> + return 0;
> +
> +err_clk:
> + while (i--)
> + bm1880_clk_unregister_pll(data->clk_data->hws[clks[i].pll.id]);
> +
> + return PTR_ERR(hw);
> +}
> +
> +static int bm1880_clk_register_mux(const struct bm1880_mux_clock *clks,
> + int num_clks, struct bm1880_clock_data *data)
> +{
> + struct clk_hw *hw;
> + void __iomem *sys_base = data->sys_base;
> + int i;
> +
> + for (i = 0; i < num_clks; i++) {
> + hw = clk_hw_register_mux(NULL, clks[i].name,
> + clks[i].parents,
> + clks[i].num_parents,
> + clks[i].flags,
> + sys_base + clks[i].reg,
> + clks[i].shift, 1, 0,
> + &bm1880_clk_lock);
> + if (IS_ERR(hw)) {
> + pr_err("%s: failed to register clock %s\n",
> + __func__, clks[i].name);
> + goto err_clk;
> + }
> +
> + data->clk_data->hws[clks[i].id] = hw;
> + }
> +
> + return 0;
> +
> +err_clk:
> + while (i--)
> + clk_hw_unregister_mux(data->clk_data->hws[clks[i].id]);
> +
> + return PTR_ERR(hw);
> +}
> +
> +static unsigned long bm1880_clk_div_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct bm1880_div_hw_clock *div_hw = to_bm1880_div_clk(hw);
> + struct bm1880_div_clock *div = &div_hw->div;
> + void __iomem *reg_addr = div_hw->base + div->reg;
> + unsigned int val;
> + unsigned long rate;
> +
> + if (!(readl(reg_addr) & BIT(3))) {
> + val = div->initval;
> + } else {
> + val = readl(reg_addr) >> div->shift;
> + val &= clk_div_mask(div->width);
> + }
> +
> + rate = divider_recalc_rate(hw, parent_rate, val, div->table,
> + div->flags, div->width);
> +
> + return rate;
> +}
> +
> +static long bm1880_clk_div_round_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long *prate)
> +{
> + struct bm1880_div_hw_clock *div_hw = to_bm1880_div_clk(hw);
> + struct bm1880_div_clock *div = &div_hw->div;
> + void __iomem *reg_addr = div_hw->base + div->reg;
> +
> + if (div->flags & CLK_DIVIDER_READ_ONLY) {
> + u32 val;
> +
> + val = readl(reg_addr) >> div->shift;
> + val &= clk_div_mask(div->width);
> +
> + return divider_ro_round_rate(hw, rate, prate, div->table,
> + div->width, div->flags,
> + val);
> + }
> +
> + return divider_round_rate(hw, rate, prate, div->table,
> + div->width, div->flags);
> +}
> +
> +static int bm1880_clk_div_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + struct bm1880_div_hw_clock *div_hw = to_bm1880_div_clk(hw);
> + struct bm1880_div_clock *div = &div_hw->div;
> + void __iomem *reg_addr = div_hw->base + div->reg;
> + unsigned long flags = 0;
> + int value;
> + u32 val;
> +
> + value = divider_get_val(rate, parent_rate, div->table,
> + div->width, div_hw->div.flags);
> + if (value < 0)
> + return value;
> +
> + if (div_hw->lock)
> + spin_lock_irqsave(div_hw->lock, flags);
> + else
> + __acquire(div_hw->lock);
> +
> + if (div->flags & CLK_DIVIDER_HIWORD_MASK) {

Is this used by your driver? As far as I can recall it was a
rockchip/hisilicon specific thing that hasn't happened since.

> + val = clk_div_mask(div->width) << (div_hw->div.shift + 16);
> + } else {
> + val = readl(reg_addr);
> + val &= ~(clk_div_mask(div->width) << div_hw->div.shift);
> + }
> + val |= (u32)value << div->shift;
> + writel(val, reg_addr);
> +
> + if (div_hw->lock)
> + spin_unlock_irqrestore(div_hw->lock, flags);
> + else
> + __release(div_hw->lock);
> +
> + return 0;
> +}
> +
> +static const struct clk_ops bm1880_clk_div_ops = {
> + .recalc_rate = bm1880_clk_div_recalc_rate,
> + .round_rate = bm1880_clk_div_round_rate,
> + .set_rate = bm1880_clk_div_set_rate,
> +};
> +
> +static struct clk_hw *bm1880_clk_register_div(struct bm1880_div_hw_clock *div_clk,
> + void __iomem *sys_base)
> +{
> + struct clk_hw *hw;
> + int err;
> +
> + div_clk->div.flags = CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO;
> + div_clk->base = sys_base;
> + div_clk->lock = &bm1880_clk_lock;
> +
> + hw = &div_clk->hw;
> + err = clk_hw_register(NULL, hw);
> + if (err)
> + return ERR_PTR(err);
> +
> + return hw;
> +}
> +
> +static void bm1880_clk_unregister_div(struct clk_hw *hw)
> +{
> + struct bm1880_div_hw_clock *div_hw = to_bm1880_div_clk(hw);
> +
> + clk_hw_unregister(hw);
> + kfree(div_hw);
> +}
> +
> +static int bm1880_clk_register_divs(struct bm1880_div_hw_clock *clks,
> + int num_clks, struct bm1880_clock_data *data)
> +{
> + struct clk_hw *hw;
> + void __iomem *sys_base = data->sys_base;
> + int i;
> +
> + for (i = 0; i < num_clks; i++) {
> + struct bm1880_div_hw_clock *bm1880_clk = &clks[i];
> +
> + hw = bm1880_clk_register_div(bm1880_clk, sys_base);
> + if (IS_ERR(hw)) {
> + pr_err("%s: failed to register clock %s\n",
> + __func__, bm1880_clk->div.name);
> + goto err_clk;
> + }
> +
> + data->clk_data->hws[clks[i].div.id] = hw;

This line is overly complicated. Please use local variables.

> + }
> +
> + return 0;
> +
> +err_clk:
> + while (i--)
> + bm1880_clk_unregister_div(data->clk_data->hws[clks[i].div.id]);
> +
> + return PTR_ERR(hw);
> +}
> +
> +static int bm1880_clk_register_gate(const struct bm1880_gate_clock *clks,
> + int num_clks, struct bm1880_clock_data *data)
> +{
> + struct clk_hw *hw;
> + void __iomem *sys_base = data->sys_base;
> + int i;
> +
> + for (i = 0; i < num_clks; i++) {
> + hw = clk_hw_register_gate(NULL, clks[i].name,
> + clks[i].parent,
> + clks[i].flags,
> + sys_base + clks[i].gate_reg,
> + clks[i].gate_shift,
> + 0,
> + &bm1880_clk_lock);

Weird tabs here.

> + if (IS_ERR(hw)) {
> + pr_err("%s: failed to register clock %s\n",
> + __func__, clks[i].name);
> + goto err_clk;
> + }
> +
> + data->clk_data->hws[clks[i].id] = hw;
> + }
> +
> + return 0;
> +
> +err_clk:
> + while (i--)
> + clk_hw_unregister_gate(data->clk_data->hws[clks[i].id]);
> +
> + return PTR_ERR(hw);
> +}
> +
> +static struct clk_hw *bm1880_clk_register_composite(struct bm1880_composite_clock *clks,
> + void __iomem *sys_base)
> +{
> + struct clk_hw *hw;
> + struct clk_mux *mux = NULL;
> + struct clk_gate *gate = NULL;
> + struct bm1880_div_hw_clock *div_hws = NULL;
> + struct clk_hw *mux_hw = NULL, *gate_hw = NULL, *div_hw = NULL;
> + const struct clk_ops *mux_ops = NULL, *gate_ops = NULL, *div_ops = NULL;
> + const char * const *parent_names;
> + const char *parent;
> + int num_parents;
> + int ret;
> +
> + if (clks->mux_shift >= 0) {
> + mux = kzalloc(sizeof(*mux), GFP_KERNEL);
> + if (!mux)
> + return ERR_PTR(-ENOMEM);
> +
> + mux->reg = sys_base + clks->mux_reg;
> + mux->mask = 1;
> + mux->shift = clks->mux_shift;
> + mux_hw = &mux->hw;
> + mux_ops = &clk_mux_ops;
> + mux->lock = &bm1880_clk_lock;
> +
> + parent_names = clks->parents;
> + num_parents = clks->num_parents;
> + } else {
> + parent = clks->parent;
> + parent_names = &parent;
> + num_parents = 1;
> + }
> +
> + if (clks->gate_shift >= 0) {
> + gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> + if (!gate) {
> + ret = -ENOMEM;
> + goto err_out;
> + }
> +
> + gate->reg = sys_base + clks->gate_reg;
> + gate->bit_idx = clks->gate_shift;
> + gate->lock = &bm1880_clk_lock;
> +
> + gate_hw = &gate->hw;
> + gate_ops = &clk_gate_ops;
> + }
> +
> + if (clks->div_shift >= 0) {
> + div_hws = kzalloc(sizeof(*div_hws), GFP_KERNEL);
> + if (!div_hws) {
> + ret = -ENOMEM;
> + goto err_out;
> + }
> +
> + div_hws->base = sys_base;
> + div_hws->div.reg = clks->div_reg;
> + div_hws->div.shift = clks->div_shift;
> + div_hws->div.width = clks->div_width;
> + div_hws->div.table = clks->table;
> + div_hws->div.initval = clks->div_initval;
> + div_hws->lock = &bm1880_clk_lock;
> + div_hws->div.flags = CLK_DIVIDER_ONE_BASED |
> + CLK_DIVIDER_ALLOW_ZERO;
> +
> + div_hw = &div_hws->hw;
> + div_ops = &bm1880_clk_div_ops;
> + }
> +
> + hw = clk_hw_register_composite(NULL, clks->name, parent_names,
> + num_parents, mux_hw, mux_ops, div_hw,
> + div_ops, gate_hw, gate_ops,
> + clks->flags);
> +

All of these need to be removed on probe failure. Any plans to do so?

> + if (IS_ERR(hw)) {
> + ret = PTR_ERR(hw);
> + goto err_out;
> + }
> +
> + return hw;
> +
> +err_out:
> + kfree(div_hws);
> + kfree(gate);
> + kfree(mux);
> +
> + return ERR_PTR(ret);
> +}
> +
> +static int bm1880_clk_register_composites(struct bm1880_composite_clock *clks,
> + int num_clks, struct bm1880_clock_data *data)
> +{
> + struct clk_hw *hw;
> + void __iomem *sys_base = data->sys_base;
> + int i;
> +
> + for (i = 0; i < num_clks; i++) {
> + struct bm1880_composite_clock *bm1880_clk = &clks[i];
> +
> + hw = bm1880_clk_register_composite(bm1880_clk, sys_base);
> + if (IS_ERR(hw)) {
> + pr_err("%s: failed to register clock %s\n",
> + __func__, bm1880_clk->name);
> + goto err_clk;
> + }
> +
> + data->clk_data->hws[clks[i].id] = hw;
> + }
> +
> + return 0;
> +
> +err_clk:
> + while (i--)
> + clk_hw_unregister_composite(data->clk_data->hws[clks[i].id]);
> +
> + return PTR_ERR(hw);
> +}
> +
> +static int bm1880_clk_probe(struct platform_device *pdev)
> +{
> + struct bm1880_clock_data *clk_data;
> + void __iomem *pll_base, *sys_base;
> + struct device_node *np = pdev->dev.of_node;
> + struct device *dev = &pdev->dev;
> + struct resource *res;
> + struct clk_hw_onecell_data *clk_hw_data;
> + int num_clks, i;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + pll_base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(pll_base))
> + return PTR_ERR(pll_base);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + sys_base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(sys_base))
> + return PTR_ERR(sys_base);
> +
> + clk_data = devm_kzalloc(dev, sizeof(*clk_data), GFP_KERNEL);
> + if (!clk_data)
> + return -ENOMEM;
> +
> + clk_data->pll_base = pll_base;
> + clk_data->sys_base = sys_base;
> +
> + num_clks = ARRAY_SIZE(bm1880_pll_clks) +
> + ARRAY_SIZE(bm1880_div_clks) +
> + ARRAY_SIZE(bm1880_mux_clks) +
> + ARRAY_SIZE(bm1880_composite_clks) +
> + ARRAY_SIZE(bm1880_gate_clks);
> +
> + clk_hw_data = devm_kzalloc(&pdev->dev, struct_size(clk_hw_data, hws,
> + num_clks), GFP_KERNEL);
> + if (!clk_hw_data)
> + return -ENOMEM;
> +
> + clk_data->clk_data = clk_hw_data;
> +
> + for (i = 0; i < num_clks; i++)
> + clk_data->clk_data->hws[i] = ERR_PTR(-ENOENT);
> +
> + clk_data->clk_data->num = num_clks;
> +
> + bm1880_clk_register_plls(bm1880_pll_clks,
> + ARRAY_SIZE(bm1880_pll_clks),
> + clk_data);
> +
> + bm1880_clk_register_divs(bm1880_div_clks,
> + ARRAY_SIZE(bm1880_div_clks),
> + clk_data);
> +
> + bm1880_clk_register_mux(bm1880_mux_clks,
> + ARRAY_SIZE(bm1880_mux_clks),
> + clk_data);
> +
> + bm1880_clk_register_composites(bm1880_composite_clks,
> + ARRAY_SIZE(bm1880_composite_clks),
> + clk_data);
> +
> + bm1880_clk_register_gate(bm1880_gate_clks,
> + ARRAY_SIZE(bm1880_gate_clks),
> + clk_data);
> +
> + return of_clk_add_hw_provider(np, of_clk_hw_onecell_get,

Can you use devm_of_clk_add_hw_provider()?

> + clk_data->clk_data);
> +}

2019-10-20 15:27:16

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH v5 2/8] clk: Warn if clk_init_data is not zero initialized

Hi Stephen,

On Tue, Sep 17, 2019 at 01:38:53PM -0700, Stephen Boyd wrote:
> Quoting Manivannan Sadhasivam (2019-09-16 09:14:41)
> > The new implementation for determining parent map uses multiple ways
> > to pass parent info. The order in which it gets processed depends on
> > the first available member. Hence, it is necessary to zero init the
> > clk_init_data struct so that the expected member gets processed correctly.
> > So, add a warning if multiple clk_init_data members are available during
> > clk registration.
> >
> > Signed-off-by: Manivannan Sadhasivam <[email protected]>
> > ---
> > drivers/clk/clk.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > index c0990703ce54..7d6d6984c979 100644
> > --- a/drivers/clk/clk.c
> > +++ b/drivers/clk/clk.c
> > @@ -3497,6 +3497,14 @@ static int clk_core_populate_parent_map(struct clk_core *core)
> > if (!num_parents)
> > return 0;
> >
> > + /*
> > + * Check for non-zero initialized clk_init_data struct. This is
> > + * required because, we only require one of the (parent_names/
> > + * parent_data/parent_hws) to be set at a time. Otherwise, the
> > + * current code would use first available member.
> > + */
> > + WARN_ON((parent_names && parent_data) || (parent_names && parent_hws));
> > +
>
> This will warn for many drivers because they set clk_init_data on the
> stack and assign parent_names but let junk from the stack be assigned to
> parent_data.

Yes, I agree.

> The code uses parent_names first and then looks for
> parent_data or parent_hws because of this fact of life that we've never
> required clk_init_data to be initialized to all zero.
>

Do you want me to just drop this patch or have any idea to make it better?

Thanks,
Mani

> > /*
> > * Avoid unnecessary string look-ups of clk_core's possible parents by
> > * having a cache of names/clk_hw pointers to clk_core pointers.

2019-10-20 15:28:06

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH v5 1/8] clk: Zero init clk_init_data in helpers

On Tue, Sep 17, 2019 at 01:39:56PM -0700, Stephen Boyd wrote:
> Quoting Manivannan Sadhasivam (2019-09-16 09:14:40)
> > The clk_init_data struct needs to be initialized to zero for the new
> > parent_map implementation to work correctly. Otherwise, the member which
> > is available first will get processed.
> >
> > Signed-off-by: Manivannan Sadhasivam <[email protected]>
> > ---
> > drivers/clk/clk-composite.c | 2 +-
> > drivers/clk/clk-divider.c | 2 +-
> > drivers/clk/clk-fixed-rate.c | 2 +-
> > drivers/clk/clk-gate.c | 2 +-
> > drivers/clk/clk-mux.c | 2 +-
> > 5 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c
> > index b06038b8f658..4d579f9d20f6 100644
> > --- a/drivers/clk/clk-composite.c
> > +++ b/drivers/clk/clk-composite.c
> > @@ -208,7 +208,7 @@ struct clk_hw *clk_hw_register_composite(struct device *dev, const char *name,
> > unsigned long flags)
> > {
> > struct clk_hw *hw;
> > - struct clk_init_data init;
> > + struct clk_init_data init = { NULL };
>
> I'd prefer { } because then we don't have to worry about ordering the
> struct to have a pointer vs. something else first.
>

okay. I thought having NULL would look more explicit!

Thanks,
Mani

> > struct clk_composite *composite;
> > struct clk_ops *clk_composite_ops;
> > int ret;

2019-10-20 15:30:52

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH v5 3/8] clk: Add clk_hw_unregister_composite helper function definition

On Tue, Sep 17, 2019 at 01:40:42PM -0700, Stephen Boyd wrote:
> Quoting Manivannan Sadhasivam (2019-09-16 09:14:42)
> > This function has been delcared but not defined anywhere. Hence, this
> > commit adds definition for it.
> >
> > Signed-off-by: Manivannan Sadhasivam <[email protected]>
> > ---
>
> Can you add a fixes tag?
>
> Fixes: 49cb392d3639 ("clk: composite: Add hw based registration APIs")
>

sure.

Thanks,
Mani

2019-10-20 15:41:18

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH v5 7/8] clk: Add common clock driver for BM1880 SoC

Hi Stephen,

On Tue, Sep 17, 2019 at 09:47:44PM -0700, Stephen Boyd wrote:
> Quoting Manivannan Sadhasivam (2019-09-16 09:14:46)
> > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> > index 801fa1cd0321..e70c64e43ff9 100644
> > --- a/drivers/clk/Kconfig
> > +++ b/drivers/clk/Kconfig
> > @@ -139,6 +139,13 @@ config COMMON_CLK_SI570
> > This driver supports Silicon Labs 570/571/598/599 programmable
> > clock generators.
> >
> > +config COMMON_CLK_BM1880
> > + bool "Clock driver for Bitmain BM1880 SoC"
>
> Can it be tristate?
>

I would prefer to have the common clock drivers as built-in since they
are the most fundamental parts of the system.

> > + depends on ARCH_BITMAIN || COMPILE_TEST
> > + default ARCH_BITMAIN
> > + help
> > + This driver supports the clocks on Bitmain BM1880 SoC.
> > +
> > config COMMON_CLK_CDCE706
> > tristate "Clock driver for TI CDCE706 clock synthesizer"
> > depends on I2C
> > diff --git a/drivers/clk/clk-bm1880.c b/drivers/clk/clk-bm1880.c
> > new file mode 100644
> > index 000000000000..3b10de929fd4
> > --- /dev/null
> > +++ b/drivers/clk/clk-bm1880.c
> > @@ -0,0 +1,966 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Bitmain BM1880 SoC clock driver
> > + *
> > + * Copyright (c) 2019 Linaro Ltd.
> > + * Author: Manivannan Sadhasivam <[email protected]>
> > + */
> > +
> > +#include <linux/clk-provider.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/slab.h>
> > +
> > +#include <dt-bindings/clock/bm1880-clock.h>
> > +
> > +#define BM1880_CLK_MPLL_CTL 0x00
> > +#define BM1880_CLK_SPLL_CTL 0x04
> > +#define BM1880_CLK_FPLL_CTL 0x08
> > +#define BM1880_CLK_DDRPLL_CTL 0x0c
> > +
> > +#define BM1880_CLK_ENABLE0 0x00
> > +#define BM1880_CLK_ENABLE1 0x04
> > +#define BM1880_CLK_SELECT 0x20
> > +#define BM1880_CLK_DIV0 0x40
> > +#define BM1880_CLK_DIV1 0x44
> > +#define BM1880_CLK_DIV2 0x48
> > +#define BM1880_CLK_DIV3 0x4c
> > +#define BM1880_CLK_DIV4 0x50
> > +#define BM1880_CLK_DIV5 0x54
> > +#define BM1880_CLK_DIV6 0x58
> > +#define BM1880_CLK_DIV7 0x5c
> > +#define BM1880_CLK_DIV8 0x60
> > +#define BM1880_CLK_DIV9 0x64
> > +#define BM1880_CLK_DIV10 0x68
> > +#define BM1880_CLK_DIV11 0x6c
> > +#define BM1880_CLK_DIV12 0x70
> > +#define BM1880_CLK_DIV13 0x74
> > +#define BM1880_CLK_DIV14 0x78
> > +#define BM1880_CLK_DIV15 0x7c
> > +#define BM1880_CLK_DIV16 0x80
> > +#define BM1880_CLK_DIV17 0x84
> > +#define BM1880_CLK_DIV18 0x88
> > +#define BM1880_CLK_DIV19 0x8c
> > +#define BM1880_CLK_DIV20 0x90
> > +#define BM1880_CLK_DIV21 0x94
> > +#define BM1880_CLK_DIV22 0x98
> > +#define BM1880_CLK_DIV23 0x9c
> > +#define BM1880_CLK_DIV24 0xa0
> > +#define BM1880_CLK_DIV25 0xa4
> > +#define BM1880_CLK_DIV26 0xa8
> > +#define BM1880_CLK_DIV27 0xac
> > +#define BM1880_CLK_DIV28 0xb0
> > +
> > +#define to_bm1880_pll_clk(_hw) container_of(_hw, struct bm1880_pll_hw_clock, hw)
> > +#define to_bm1880_div_clk(_hw) container_of(_hw, struct bm1880_div_hw_clock, hw)
> > +
> > +static DEFINE_SPINLOCK(bm1880_clk_lock);
> > +
> > +struct bm1880_clock_data {
> > + void __iomem *pll_base;
> > + void __iomem *sys_base;
> > + struct clk_hw_onecell_data *clk_data;
>
> Please call it hw_data or onecell_data instead so that the code doesn't
> read as clk_data->clk_data. Also, probably can just make it part of the
> same struct instead of a pointer inside so that it can all be allocated
> in one big chunk instead of in two.
>

okay.

> > +};
> > +
> > +struct bm1880_gate_clock {
> > + unsigned int id;
> > + const char *name;
> > + const char *parent;
> > + u32 gate_reg;
> > + s8 gate_shift;
> > + unsigned long flags;
> > +};
> > +
> > +struct bm1880_mux_clock {
> > + unsigned int id;
> > + const char *name;
> > + const char * const *parents;
> > + s8 num_parents;
> > + u32 reg;
> > + s8 shift;
> > + unsigned long flags;
> > +};
> > +
> > +struct bm1880_div_clock {
> > + unsigned int id;
> > + const char *name;
> > + u32 reg;
> > + u8 shift;
> > + u8 width;
> > + u32 initval;
> > + const struct clk_div_table *table;
> > + unsigned long flags;
> > +};
> > +
> > +struct bm1880_div_hw_clock {
> > + struct bm1880_div_clock div;
> > + void __iomem *base;
> > + spinlock_t *lock;
> > + struct clk_hw hw;
> > + struct clk_init_data init;
> > +};
> > +
> > +struct bm1880_composite_clock {
> > + unsigned int id;
> > + const char *name;
> > + const char *parent;
> > + const char * const *parents;
> > + unsigned int num_parents;
> > + unsigned long flags;
> > +
> > + u32 gate_reg;
> > + u32 mux_reg;
> > + u32 div_reg;
> > +
> > + s8 gate_shift;
> > + s8 mux_shift;
> > + s8 div_shift;
> > + s8 div_width;
> > + s16 div_initval;
> > + const struct clk_div_table *table;
> > +};
> > +
> > +struct bm1880_pll_clock {
> > + unsigned int id;
> > + const char *name;
> > + u32 reg;
> > + unsigned long flags;
> > +};
> > +
> > +struct bm1880_pll_hw_clock {
> > + struct bm1880_pll_clock pll;
> > + void __iomem *base;
> > + struct clk_hw hw;
> > + struct clk_init_data init;
> > +};
> > +
> > +static const struct clk_ops bm1880_pll_ops;
> > +static const struct clk_ops bm1880_clk_div_ops;
> > +
> > +#define GATE_DIV(_id, _name, _parent, _gate_reg, _gate_shift, _div_reg, \
> > + _div_shift, _div_width, _div_initval, _table, \
> > + _flags) { \
> > + .id = _id, \
> > + .parent = _parent, \
> > + .name = _name, \
> > + .gate_reg = _gate_reg, \
> > + .gate_shift = _gate_shift, \
> > + .div_reg = _div_reg, \
> > + .div_shift = _div_shift, \
> > + .div_width = _div_width, \
> > + .div_initval = _div_initval, \
> > + .table = _table, \
> > + .mux_shift = -1, \
> > + .flags = _flags, \
> > + }
> > +
> > +#define GATE_MUX(_id, _name, _parents, _gate_reg, _gate_shift, \
> > + _mux_reg, _mux_shift, _flags) { \
> > + .id = _id, \
> > + .parents = _parents, \
> > + .num_parents = ARRAY_SIZE(_parents), \
> > + .name = _name, \
> > + .gate_reg = _gate_reg, \
> > + .gate_shift = _gate_shift, \
> > + .div_shift = -1, \
> > + .mux_reg = _mux_reg, \
> > + .mux_shift = _mux_shift, \
> > + .flags = _flags, \
> > + }
> > +
> > +#define CLK_PLL(_id, _name, _parent, _reg, _flags) { \
> > + .pll.id = _id, \
> > + .pll.name = _name, \
> > + .pll.reg = _reg, \
> > + .hw.init = CLK_HW_INIT_PARENTS_DATA(_name, _parent, \
> > + &bm1880_pll_ops, \
> > + _flags), \
> > + }
> > +
> > +#define CLK_DIV(_id, _name, _parent, _reg, _shift, _width, _initval, \
> > + _table, _flags) { \
> > + .div.id = _id, \
> > + .div.name = _name, \
> > + .div.reg = _reg, \
> > + .div.shift = _shift, \
> > + .div.width = _width, \
> > + .div.initval = _initval, \
> > + .div.table = _table, \
> > + .hw.init = CLK_HW_INIT_HW(_name, _parent, \
> > + &bm1880_clk_div_ops, \
> > + _flags), \
> > + }
> > +
> > +static struct clk_parent_data bm1880_pll_parent[] = {
> > + { .fw_name = "osc", .name = "osc" },
> > +};
> > +
> > +/*
> > + * All PLL clocks are marked as CRITICAL, hence they are very crucial
> > + * for the functioning of the SoC
>
> Please add more information besides crucial to function of the clk.
> Basically describe what the PLLs are clocking and why those child clks
> aren't enabled or marked as critical themselves. The usage of
> CLK_IS_CRITICAL is too liberal in this driver so this needs to be
> cleaned up. For example, clk_mpll shouldn't be marked critical, just the
> a53 CPU clk that can source from it should be marked critical because
> it's for the CPU running code.

okay. I got into the assumption that we should explicitly mark the parent
clocks as critical also. But yeah, this doesn't makes much sense...

> It's also odd that we would register gate
> clks or the a53 clks if we don't expect to ever turn those clks off. Can
> that be avoided so that we don't need to mark anything critical for this
> path?
>

No. I would like to implement the whole logic provided by the IP as it is.
Eventhough it looks odd, that's how things are designed ;-)

> > + */
> > +static struct bm1880_pll_hw_clock bm1880_pll_clks[] = {
> > + CLK_PLL(BM1880_CLK_MPLL, "clk_mpll", bm1880_pll_parent,
> > + BM1880_CLK_MPLL_CTL, CLK_IS_CRITICAL),
> > + CLK_PLL(BM1880_CLK_SPLL, "clk_spll", bm1880_pll_parent,
> > + BM1880_CLK_SPLL_CTL, CLK_IS_CRITICAL),
> > + CLK_PLL(BM1880_CLK_FPLL, "clk_fpll", bm1880_pll_parent,
> > + BM1880_CLK_FPLL_CTL, CLK_IS_CRITICAL),
> > + CLK_PLL(BM1880_CLK_DDRPLL, "clk_ddrpll", bm1880_pll_parent,
> > + BM1880_CLK_DDRPLL_CTL, CLK_IS_CRITICAL),
> > +};
> > +
> > +/*
> > + * Clocks marked as CRITICAL are needed for the proper functioning
> > + * of the SoC.
> > + */
> > +static const struct bm1880_gate_clock bm1880_gate_clks[] = {
> > + { BM1880_CLK_AHB_ROM, "clk_ahb_rom", "clk_mux_axi6",
> > + BM1880_CLK_ENABLE0, 2, CLK_IS_CRITICAL },
> > + { BM1880_CLK_AXI_SRAM, "clk_axi_sram", "clk_axi1",
> > + BM1880_CLK_ENABLE0, 3, CLK_IS_CRITICAL },
> > + { BM1880_CLK_DDR_AXI, "clk_ddr_axi", "clk_mux_axi6",
> > + BM1880_CLK_ENABLE0, 4, CLK_IS_CRITICAL },
> > + { BM1880_CLK_APB_EFUSE, "clk_apb_efuse", "clk_mux_axi6",
> > + BM1880_CLK_ENABLE0, 6, CLK_IS_CRITICAL },
> > + { BM1880_CLK_AXI5_EMMC, "clk_axi5_emmc", "clk_axi5",
> > + BM1880_CLK_ENABLE0, 7, 0 },
> > + { BM1880_CLK_AXI5_SD, "clk_axi5_sd", "clk_axi5",
> > + BM1880_CLK_ENABLE0, 10, 0 },
> > + { BM1880_CLK_AXI4_ETH0, "clk_axi4_eth0", "clk_axi4",
> > + BM1880_CLK_ENABLE0, 14, 0 },
> > + { BM1880_CLK_AXI4_ETH1, "clk_axi4_eth1", "clk_axi4",
> > + BM1880_CLK_ENABLE0, 16, 0 },
> > + { BM1880_CLK_AXI1_GDMA, "clk_axi1_gdma", "clk_axi1",
> > + BM1880_CLK_ENABLE0, 17, 0 },
> > + /* Don't gate GPIO clocks as it is not owned by the GPIO driver */
> > + { BM1880_CLK_APB_GPIO, "clk_apb_gpio", "clk_mux_axi6",
> > + BM1880_CLK_ENABLE0, 18, CLK_IGNORE_UNUSED },
> > + { BM1880_CLK_APB_GPIO_INTR, "clk_apb_gpio_intr", "clk_mux_axi6",
> > + BM1880_CLK_ENABLE0, 19, CLK_IGNORE_UNUSED },
> > + { BM1880_CLK_AXI1_MINER, "clk_axi1_miner", "clk_axi1",
> > + BM1880_CLK_ENABLE0, 21, 0 },
> > + { BM1880_CLK_AHB_SF, "clk_ahb_sf", "clk_mux_axi6",
> > + BM1880_CLK_ENABLE0, 22, 0 },
> > + { BM1880_CLK_SDMA_AXI, "clk_sdma_axi", "clk_axi5",
> > + BM1880_CLK_ENABLE0, 23, 0 },
> > + { BM1880_CLK_APB_I2C, "clk_apb_i2c", "clk_mux_axi6",
> > + BM1880_CLK_ENABLE0, 25, 0 },
> > + { BM1880_CLK_APB_WDT, "clk_apb_wdt", "clk_mux_axi6",
> > + BM1880_CLK_ENABLE0, 26, 0 },
> > + { BM1880_CLK_APB_JPEG, "clk_apb_jpeg", "clk_axi6",
> > + BM1880_CLK_ENABLE0, 27, 0 },
> > + { BM1880_CLK_AXI5_NF, "clk_axi5_nf", "clk_axi5",
> > + BM1880_CLK_ENABLE0, 29, 0 },
> > + { BM1880_CLK_APB_NF, "clk_apb_nf", "clk_axi6",
> > + BM1880_CLK_ENABLE0, 30, 0 },
> > + { BM1880_CLK_APB_PWM, "clk_apb_pwm", "clk_mux_axi6",
> > + BM1880_CLK_ENABLE1, 0, 0 },
> > + { BM1880_CLK_RV, "clk_rv", "clk_mux_rv",
> > + BM1880_CLK_ENABLE1, 1, 0 },
> > + { BM1880_CLK_APB_SPI, "clk_apb_spi", "clk_mux_axi6",
> > + BM1880_CLK_ENABLE1, 2, 0 },
> > + { BM1880_CLK_UART_500M, "clk_uart_500m", "clk_div_uart_500m",
> > + BM1880_CLK_ENABLE1, 4, 0 },
> > + { BM1880_CLK_APB_UART, "clk_apb_uart", "clk_axi6",
> > + BM1880_CLK_ENABLE1, 5, 0 },
> > + { BM1880_CLK_APB_I2S, "clk_apb_i2s", "clk_axi6",
> > + BM1880_CLK_ENABLE1, 6, 0 },
> > + { BM1880_CLK_AXI4_USB, "clk_axi4_usb", "clk_axi4",
> > + BM1880_CLK_ENABLE1, 7, 0 },
> > + { BM1880_CLK_APB_USB, "clk_apb_usb", "clk_axi6",
> > + BM1880_CLK_ENABLE1, 8, 0 },
> > + { BM1880_CLK_12M_USB, "clk_12m_usb", "clk_div_12m_usb",
> > + BM1880_CLK_ENABLE1, 11, 0 },
> > + { BM1880_CLK_APB_VIDEO, "clk_apb_video", "clk_axi6",
> > + BM1880_CLK_ENABLE1, 12, 0 },
> > + { BM1880_CLK_APB_VPP, "clk_apb_vpp", "clk_axi6",
> > + BM1880_CLK_ENABLE1, 15, 0 },
> > + { BM1880_CLK_AXI6, "clk_axi6", "clk_mux_axi6",
> > + BM1880_CLK_ENABLE1, 21, CLK_IS_CRITICAL },
> > +};
> > +
> > +static const char * const clk_a53_parents[] = { "clk_spll", "clk_mpll" };
> > +static const char * const clk_rv_parents[] = { "clk_div_1_rv", "clk_div_0_rv" };
> > +static const char * const clk_axi1_parents[] = { "clk_div_1_axi1", "clk_div_0_axi1" };
> > +static const char * const clk_axi6_parents[] = { "clk_div_1_axi6", "clk_div_0_axi6" };
>
> I sent some patches to make the basic clk types support the new way of
> specifying parents. Can you use those patches instead of listing all
> these strings?
>

Yeah, I just noticed after sending out these patches. But I'd like to adapt
them later due to that fact that it will require some rework to this driver
and I don't have time to do it currently :(

I'll send incremental patches later once this gets in!

> > +
> > +static const struct bm1880_mux_clock bm1880_mux_clks[] = {
> > + { BM1880_CLK_MUX_RV, "clk_mux_rv", clk_rv_parents, 2,
> > + BM1880_CLK_SELECT, 1, 0 },
> > + { BM1880_CLK_MUX_AXI6, "clk_mux_axi6", clk_axi6_parents, 2,
> > + BM1880_CLK_SELECT, 3, 0 },
> > +};
> > +
> > +static const struct clk_div_table bm1880_div_table_0[] = {
> > + { 0, 1 }, { 1, 2 }, { 2, 3 }, { 3, 4 },
> > + { 4, 5 }, { 5, 6 }, { 6, 7 }, { 7, 8 },
> > + { 8, 9 }, { 9, 10 }, { 10, 11 }, { 11, 12 },
> > + { 12, 13 }, { 13, 14 }, { 14, 15 }, { 15, 16 },
> > + { 16, 17 }, { 17, 18 }, { 18, 19 }, { 19, 20 },
> > + { 20, 21 }, { 21, 22 }, { 22, 23 }, { 23, 24 },
> > + { 24, 25 }, { 25, 26 }, { 26, 27 }, { 27, 28 },
> > + { 28, 29 }, { 29, 30 }, { 30, 31 }, { 31, 32 },
> > + { 0, 0 }
> > +};
> > +
> > +static const struct clk_div_table bm1880_div_table_1[] = {
> > + { 0, 1 }, { 1, 2 }, { 2, 3 }, { 3, 4 },
> > + { 4, 5 }, { 5, 6 }, { 6, 7 }, { 7, 8 },
> > + { 8, 9 }, { 9, 10 }, { 10, 11 }, { 11, 12 },
> > + { 12, 13 }, { 13, 14 }, { 14, 15 }, { 15, 16 },
> > + { 16, 17 }, { 17, 18 }, { 18, 19 }, { 19, 20 },
> > + { 20, 21 }, { 21, 22 }, { 22, 23 }, { 23, 24 },
> > + { 24, 25 }, { 25, 26 }, { 26, 27 }, { 27, 28 },
> > + { 28, 29 }, { 29, 30 }, { 30, 31 }, { 31, 32 },
> > + { 127, 128 }, { 0, 0 }
> > +};
> > +
> > +static const struct clk_div_table bm1880_div_table_2[] = {
> > + { 0, 1 }, { 1, 2 }, { 2, 3 }, { 3, 4 },
> > + { 4, 5 }, { 5, 6 }, { 6, 7 }, { 7, 8 },
> > + { 8, 9 }, { 9, 10 }, { 10, 11 }, { 11, 12 },
> > + { 12, 13 }, { 13, 14 }, { 14, 15 }, { 15, 16 },
> > + { 16, 17 }, { 17, 18 }, { 18, 19 }, { 19, 20 },
> > + { 20, 21 }, { 21, 22 }, { 22, 23 }, { 23, 24 },
> > + { 24, 25 }, { 25, 26 }, { 26, 27 }, { 27, 28 },
> > + { 28, 29 }, { 29, 30 }, { 30, 31 }, { 31, 32 },
> > + { 127, 128 }, { 255, 256 }, { 0, 0 }
> > +};
> > +
> > +static const struct clk_div_table bm1880_div_table_3[] = {
> > + { 0, 1 }, { 1, 2 }, { 2, 3 }, { 3, 4 },
> > + { 4, 5 }, { 5, 6 }, { 6, 7 }, { 7, 8 },
> > + { 8, 9 }, { 9, 10 }, { 10, 11 }, { 11, 12 },
> > + { 12, 13 }, { 13, 14 }, { 14, 15 }, { 15, 16 },
> > + { 16, 17 }, { 17, 18 }, { 18, 19 }, { 19, 20 },
> > + { 20, 21 }, { 21, 22 }, { 22, 23 }, { 23, 24 },
> > + { 24, 25 }, { 25, 26 }, { 26, 27 }, { 27, 28 },
> > + { 28, 29 }, { 29, 30 }, { 30, 31 }, { 31, 32 },
> > + { 127, 128 }, { 255, 256 }, { 511, 512 }, { 0, 0 }
> > +};
> > +
> > +static const struct clk_div_table bm1880_div_table_4[] = {
> > + { 0, 1 }, { 1, 2 }, { 2, 3 }, { 3, 4 },
> > + { 4, 5 }, { 5, 6 }, { 6, 7 }, { 7, 8 },
> > + { 8, 9 }, { 9, 10 }, { 10, 11 }, { 11, 12 },
> > + { 12, 13 }, { 13, 14 }, { 14, 15 }, { 15, 16 },
> > + { 16, 17 }, { 17, 18 }, { 18, 19 }, { 19, 20 },
> > + { 20, 21 }, { 21, 22 }, { 22, 23 }, { 23, 24 },
> > + { 24, 25 }, { 25, 26 }, { 26, 27 }, { 27, 28 },
> > + { 28, 29 }, { 29, 30 }, { 30, 31 }, { 31, 32 },
> > + { 127, 128 }, { 255, 256 }, { 511, 512 }, { 65535, 65536 },
> > + { 0, 0 }
> > +};
> > +
> > +/*
> > + * Clocks marked as CRITICAL are needed for the proper functioning
> > + * of the SoC.
> > + */
> > +static struct bm1880_div_hw_clock bm1880_div_clks[] = {
> > + CLK_DIV(BM1880_CLK_DIV_0_RV, "clk_div_0_rv", &bm1880_pll_clks[1].hw,
> > + BM1880_CLK_DIV12, 16, 5, 1, bm1880_div_table_0, 0),
> > + CLK_DIV(BM1880_CLK_DIV_1_RV, "clk_div_1_rv", &bm1880_pll_clks[2].hw,
> > + BM1880_CLK_DIV13, 16, 5, 1, bm1880_div_table_0, 0),
> > + CLK_DIV(BM1880_CLK_DIV_UART_500M, "clk_div_uart_500m", &bm1880_pll_clks[2].hw,
> > + BM1880_CLK_DIV15, 16, 7, 3, bm1880_div_table_1, 0),
> > + CLK_DIV(BM1880_CLK_DIV_0_AXI1, "clk_div_0_axi1", &bm1880_pll_clks[0].hw,
> > + BM1880_CLK_DIV21, 16, 5, 2, bm1880_div_table_0,
> > + CLK_IS_CRITICAL),
> > + CLK_DIV(BM1880_CLK_DIV_1_AXI1, "clk_div_1_axi1", &bm1880_pll_clks[2].hw,
> > + BM1880_CLK_DIV22, 16, 5, 3, bm1880_div_table_0,
> > + CLK_IS_CRITICAL),
> > + CLK_DIV(BM1880_CLK_DIV_0_AXI6, "clk_div_0_axi6", &bm1880_pll_clks[2].hw,
> > + BM1880_CLK_DIV27, 16, 5, 15, bm1880_div_table_0,
> > + CLK_IS_CRITICAL),
> > + CLK_DIV(BM1880_CLK_DIV_1_AXI6, "clk_div_1_axi6", &bm1880_pll_clks[0].hw,
> > + BM1880_CLK_DIV28, 16, 5, 11, bm1880_div_table_0,
> > + CLK_IS_CRITICAL),
> > + CLK_DIV(BM1880_CLK_DIV_12M_USB, "clk_div_12m_usb", &bm1880_pll_clks[2].hw,
> > + BM1880_CLK_DIV18, 16, 7, 125, bm1880_div_table_1, 0),
> > +};
> > +
> > +/*
> > + * Clocks marked as CRITICAL are all needed for the proper functioning
> > + * of the SoC.
> > + */
> > +static struct bm1880_composite_clock bm1880_composite_clks[] = {
> > + GATE_MUX(BM1880_CLK_A53, "clk_a53", clk_a53_parents,
> > + BM1880_CLK_ENABLE0, 0, BM1880_CLK_SELECT, 0,
> > + CLK_IS_CRITICAL),
> > + GATE_DIV(BM1880_CLK_50M_A53, "clk_50m_a53", "clk_fpll",
> > + BM1880_CLK_ENABLE0, 1, BM1880_CLK_DIV0, 16, 5, 30,
> > + bm1880_div_table_0, CLK_IS_CRITICAL),
> > + GATE_DIV(BM1880_CLK_EFUSE, "clk_efuse", "clk_fpll",
> > + BM1880_CLK_ENABLE0, 5, BM1880_CLK_DIV1, 16, 7, 60,
> > + bm1880_div_table_1, 0),
> > + GATE_DIV(BM1880_CLK_EMMC, "clk_emmc", "clk_fpll",
> > + BM1880_CLK_ENABLE0, 8, BM1880_CLK_DIV2, 16, 5, 15,
> > + bm1880_div_table_0, 0),
> > + GATE_DIV(BM1880_CLK_100K_EMMC, "clk_100k_emmc", "clk_div_12m_usb",
> > + BM1880_CLK_ENABLE0, 9, BM1880_CLK_DIV3, 16, 8, 120,
> > + bm1880_div_table_2, 0),
> > + GATE_DIV(BM1880_CLK_SD, "clk_sd", "clk_fpll",
> > + BM1880_CLK_ENABLE0, 11, BM1880_CLK_DIV4, 16, 5, 15,
> > + bm1880_div_table_0, 0),
> > + GATE_DIV(BM1880_CLK_100K_SD, "clk_100k_sd", "clk_div_12m_usb",
> > + BM1880_CLK_ENABLE0, 12, BM1880_CLK_DIV5, 16, 8, 120,
> > + bm1880_div_table_2, 0),
> > + GATE_DIV(BM1880_CLK_500M_ETH0, "clk_500m_eth0", "clk_fpll",
> > + BM1880_CLK_ENABLE0, 13, BM1880_CLK_DIV6, 16, 5, 3,
> > + bm1880_div_table_0, 0),
> > + GATE_DIV(BM1880_CLK_500M_ETH1, "clk_500m_eth1", "clk_fpll",
> > + BM1880_CLK_ENABLE0, 15, BM1880_CLK_DIV7, 16, 5, 3,
> > + bm1880_div_table_0, 0),
> > + /* Don't gate GPIO clocks as it is not owned by the GPIO driver */
> > + GATE_DIV(BM1880_CLK_GPIO_DB, "clk_gpio_db", "clk_div_12m_usb",
> > + BM1880_CLK_ENABLE0, 20, BM1880_CLK_DIV8, 16, 16, 120,
> > + bm1880_div_table_4, CLK_IGNORE_UNUSED),
> > + GATE_DIV(BM1880_CLK_SDMA_AUD, "clk_sdma_aud", "clk_fpll",
> > + BM1880_CLK_ENABLE0, 24, BM1880_CLK_DIV9, 16, 7, 61,
> > + bm1880_div_table_1, 0),
> > + GATE_DIV(BM1880_CLK_JPEG_AXI, "clk_jpeg_axi", "clk_fpll",
> > + BM1880_CLK_ENABLE0, 28, BM1880_CLK_DIV10, 16, 5, 4,
> > + bm1880_div_table_0, 0),
> > + GATE_DIV(BM1880_CLK_NF, "clk_nf", "clk_fpll",
> > + BM1880_CLK_ENABLE0, 31, BM1880_CLK_DIV11, 16, 5, 30,
> > + bm1880_div_table_0, 0),
> > + GATE_DIV(BM1880_CLK_TPU_AXI, "clk_tpu_axi", "clk_spll",
> > + BM1880_CLK_ENABLE1, 3, BM1880_CLK_DIV14, 16, 5, 1,
> > + bm1880_div_table_0, 0),
> > + GATE_DIV(BM1880_CLK_125M_USB, "clk_125m_usb", "clk_fpll",
> > + BM1880_CLK_ENABLE1, 9, BM1880_CLK_DIV16, 16, 5, 12,
> > + bm1880_div_table_0, 0),
> > + GATE_DIV(BM1880_CLK_33K_USB, "clk_33k_usb", "clk_div_12m_usb",
> > + BM1880_CLK_ENABLE1, 10, BM1880_CLK_DIV17, 16, 9, 363,
> > + bm1880_div_table_3, 0),
> > + GATE_DIV(BM1880_CLK_VIDEO_AXI, "clk_video_axi", "clk_fpll",
> > + BM1880_CLK_ENABLE1, 13, BM1880_CLK_DIV19, 16, 5, 4,
> > + bm1880_div_table_0, 0),
> > + GATE_DIV(BM1880_CLK_VPP_AXI, "clk_vpp_axi", "clk_fpll",
> > + BM1880_CLK_ENABLE1, 14, BM1880_CLK_DIV20, 16, 5, 4,
> > + bm1880_div_table_0, 0),
> > + GATE_MUX(BM1880_CLK_AXI1, "clk_axi1", clk_axi1_parents,
> > + BM1880_CLK_ENABLE1, 15, BM1880_CLK_SELECT, 2,
> > + CLK_IS_CRITICAL),
> > + GATE_DIV(BM1880_CLK_AXI2, "clk_axi2", "clk_fpll",
> > + BM1880_CLK_ENABLE1, 17, BM1880_CLK_DIV23, 16, 5, 3,
> > + bm1880_div_table_0, CLK_IS_CRITICAL),
> > + GATE_DIV(BM1880_CLK_AXI3, "clk_axi3", "clk_mux_rv",
> > + BM1880_CLK_ENABLE1, 18, BM1880_CLK_DIV24, 16, 5, 2,
> > + bm1880_div_table_0, CLK_IS_CRITICAL),
> > + GATE_DIV(BM1880_CLK_AXI4, "clk_axi4", "clk_fpll",
> > + BM1880_CLK_ENABLE1, 19, BM1880_CLK_DIV25, 16, 5, 6,
> > + bm1880_div_table_0, CLK_IS_CRITICAL),
> > + GATE_DIV(BM1880_CLK_AXI5, "clk_axi5", "clk_fpll",
> > + BM1880_CLK_ENABLE1, 20, BM1880_CLK_DIV26, 16, 5, 15,
> > + bm1880_div_table_0, CLK_IS_CRITICAL),
> > +};
> > +
> > +static unsigned long bm1880_pll_rate_calc(u32 regval, unsigned long parent_rate)
> > +{
> > + u32 fbdiv, fref, refdiv;
> > + u32 postdiv1, postdiv2;
> > + unsigned long rate, numerator, denominator;
> > +
> > + fbdiv = (regval >> 16) & 0xfff;
> > + fref = parent_rate;
> > + refdiv = regval & 0x1f;
> > + postdiv1 = (regval >> 8) & 0x7;
> > + postdiv2 = (regval >> 12) & 0x7;
> > +
> > + numerator = parent_rate * fbdiv;
> > + denominator = refdiv * postdiv1 * postdiv2;
> > + do_div(numerator, denominator);
> > + rate = numerator;
> > +
> > + return rate;
> > +}
> > +
> > +static unsigned long bm1880_pll_recalc_rate(struct clk_hw *hw,
> > + unsigned long parent_rate)
> > +{
> > + struct bm1880_pll_hw_clock *pll_hw = to_bm1880_pll_clk(hw);
> > + unsigned long rate;
> > + u32 regval;
> > +
> > + regval = readl(pll_hw->base + pll_hw->pll.reg);
> > + rate = bm1880_pll_rate_calc(regval, parent_rate);
> > +
> > + return rate;
> > +}
> > +
> > +static const struct clk_ops bm1880_pll_ops = {
> > + .recalc_rate = bm1880_pll_recalc_rate,
> > +};
> > +
> > +static struct clk_hw *bm1880_clk_register_pll(struct bm1880_pll_hw_clock *pll_clk,
> > + void __iomem *sys_base)
> > +{
> > + struct clk_hw *hw;
> > + int err;
> > +
> > + pll_clk->base = sys_base;
> > + hw = &pll_clk->hw;
> > +
> > + err = clk_hw_register(NULL, hw);
> > + if (err)
> > + return ERR_PTR(err);
> > +
> > + return hw;
> > +}
> > +
> > +static void bm1880_clk_unregister_pll(struct clk_hw *hw)
> > +{
> > + struct bm1880_pll_hw_clock *pll_hw = to_bm1880_pll_clk(hw);
> > +
> > + clk_hw_unregister(hw);
> > + kfree(pll_hw);
> > +}
> > +
> > +static int bm1880_clk_register_plls(struct bm1880_pll_hw_clock *clks,
> > + int num_clks, struct bm1880_clock_data *data)
> > +{
> > + struct clk_hw *hw;
> > + void __iomem *pll_base = data->pll_base;
> > + int i;
> > +
> > + for (i = 0; i < num_clks; i++) {
> > + struct bm1880_pll_hw_clock *bm1880_clk = &clks[i];
> > +
> > + hw = bm1880_clk_register_pll(bm1880_clk, pll_base);
> > + if (IS_ERR(hw)) {
> > + pr_err("%s: failed to register clock %s\n",
> > + __func__, bm1880_clk->pll.name);
> > + goto err_clk;
> > + }
> > +
> > + data->clk_data->hws[clks[i].pll.id] = hw;
> > + }
> > +
> > + return 0;
> > +
> > +err_clk:
> > + while (i--)
> > + bm1880_clk_unregister_pll(data->clk_data->hws[clks[i].pll.id]);
> > +
> > + return PTR_ERR(hw);
> > +}
> > +
> > +static int bm1880_clk_register_mux(const struct bm1880_mux_clock *clks,
> > + int num_clks, struct bm1880_clock_data *data)
> > +{
> > + struct clk_hw *hw;
> > + void __iomem *sys_base = data->sys_base;
> > + int i;
> > +
> > + for (i = 0; i < num_clks; i++) {
> > + hw = clk_hw_register_mux(NULL, clks[i].name,
> > + clks[i].parents,
> > + clks[i].num_parents,
> > + clks[i].flags,
> > + sys_base + clks[i].reg,
> > + clks[i].shift, 1, 0,
> > + &bm1880_clk_lock);
> > + if (IS_ERR(hw)) {
> > + pr_err("%s: failed to register clock %s\n",
> > + __func__, clks[i].name);
> > + goto err_clk;
> > + }
> > +
> > + data->clk_data->hws[clks[i].id] = hw;
> > + }
> > +
> > + return 0;
> > +
> > +err_clk:
> > + while (i--)
> > + clk_hw_unregister_mux(data->clk_data->hws[clks[i].id]);
> > +
> > + return PTR_ERR(hw);
> > +}
> > +
> > +static unsigned long bm1880_clk_div_recalc_rate(struct clk_hw *hw,
> > + unsigned long parent_rate)
> > +{
> > + struct bm1880_div_hw_clock *div_hw = to_bm1880_div_clk(hw);
> > + struct bm1880_div_clock *div = &div_hw->div;
> > + void __iomem *reg_addr = div_hw->base + div->reg;
> > + unsigned int val;
> > + unsigned long rate;
> > +
> > + if (!(readl(reg_addr) & BIT(3))) {
> > + val = div->initval;
> > + } else {
> > + val = readl(reg_addr) >> div->shift;
> > + val &= clk_div_mask(div->width);
> > + }
> > +
> > + rate = divider_recalc_rate(hw, parent_rate, val, div->table,
> > + div->flags, div->width);
> > +
> > + return rate;
> > +}
> > +
> > +static long bm1880_clk_div_round_rate(struct clk_hw *hw, unsigned long rate,
> > + unsigned long *prate)
> > +{
> > + struct bm1880_div_hw_clock *div_hw = to_bm1880_div_clk(hw);
> > + struct bm1880_div_clock *div = &div_hw->div;
> > + void __iomem *reg_addr = div_hw->base + div->reg;
> > +
> > + if (div->flags & CLK_DIVIDER_READ_ONLY) {
> > + u32 val;
> > +
> > + val = readl(reg_addr) >> div->shift;
> > + val &= clk_div_mask(div->width);
> > +
> > + return divider_ro_round_rate(hw, rate, prate, div->table,
> > + div->width, div->flags,
> > + val);
> > + }
> > +
> > + return divider_round_rate(hw, rate, prate, div->table,
> > + div->width, div->flags);
> > +}
> > +
> > +static int bm1880_clk_div_set_rate(struct clk_hw *hw, unsigned long rate,
> > + unsigned long parent_rate)
> > +{
> > + struct bm1880_div_hw_clock *div_hw = to_bm1880_div_clk(hw);
> > + struct bm1880_div_clock *div = &div_hw->div;
> > + void __iomem *reg_addr = div_hw->base + div->reg;
> > + unsigned long flags = 0;
> > + int value;
> > + u32 val;
> > +
> > + value = divider_get_val(rate, parent_rate, div->table,
> > + div->width, div_hw->div.flags);
> > + if (value < 0)
> > + return value;
> > +
> > + if (div_hw->lock)
> > + spin_lock_irqsave(div_hw->lock, flags);
> > + else
> > + __acquire(div_hw->lock);
> > +
> > + if (div->flags & CLK_DIVIDER_HIWORD_MASK) {
>
> Is this used by your driver? As far as I can recall it was a
> rockchip/hisilicon specific thing that hasn't happened since.
>

Actually, this is used by the underlying IP but not currently used in BM1880.
So, I can remove it. We can add it once the corresponding SoC is added.

> > + val = clk_div_mask(div->width) << (div_hw->div.shift + 16);
> > + } else {
> > + val = readl(reg_addr);
> > + val &= ~(clk_div_mask(div->width) << div_hw->div.shift);
> > + }
> > + val |= (u32)value << div->shift;
> > + writel(val, reg_addr);
> > +
> > + if (div_hw->lock)
> > + spin_unlock_irqrestore(div_hw->lock, flags);
> > + else
> > + __release(div_hw->lock);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct clk_ops bm1880_clk_div_ops = {
> > + .recalc_rate = bm1880_clk_div_recalc_rate,
> > + .round_rate = bm1880_clk_div_round_rate,
> > + .set_rate = bm1880_clk_div_set_rate,
> > +};
> > +
> > +static struct clk_hw *bm1880_clk_register_div(struct bm1880_div_hw_clock *div_clk,
> > + void __iomem *sys_base)
> > +{
> > + struct clk_hw *hw;
> > + int err;
> > +
> > + div_clk->div.flags = CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO;
> > + div_clk->base = sys_base;
> > + div_clk->lock = &bm1880_clk_lock;
> > +
> > + hw = &div_clk->hw;
> > + err = clk_hw_register(NULL, hw);
> > + if (err)
> > + return ERR_PTR(err);
> > +
> > + return hw;
> > +}
> > +
> > +static void bm1880_clk_unregister_div(struct clk_hw *hw)
> > +{
> > + struct bm1880_div_hw_clock *div_hw = to_bm1880_div_clk(hw);
> > +
> > + clk_hw_unregister(hw);
> > + kfree(div_hw);
> > +}
> > +
> > +static int bm1880_clk_register_divs(struct bm1880_div_hw_clock *clks,
> > + int num_clks, struct bm1880_clock_data *data)
> > +{
> > + struct clk_hw *hw;
> > + void __iomem *sys_base = data->sys_base;
> > + int i;
> > +
> > + for (i = 0; i < num_clks; i++) {
> > + struct bm1880_div_hw_clock *bm1880_clk = &clks[i];
> > +
> > + hw = bm1880_clk_register_div(bm1880_clk, sys_base);
> > + if (IS_ERR(hw)) {
> > + pr_err("%s: failed to register clock %s\n",
> > + __func__, bm1880_clk->div.name);
> > + goto err_clk;
> > + }
> > +
> > + data->clk_data->hws[clks[i].div.id] = hw;
>
> This line is overly complicated. Please use local variables.
>

Ack.

> > + }
> > +
> > + return 0;
> > +
> > +err_clk:
> > + while (i--)
> > + bm1880_clk_unregister_div(data->clk_data->hws[clks[i].div.id]);
> > +
> > + return PTR_ERR(hw);
> > +}
> > +
> > +static int bm1880_clk_register_gate(const struct bm1880_gate_clock *clks,
> > + int num_clks, struct bm1880_clock_data *data)
> > +{
> > + struct clk_hw *hw;
> > + void __iomem *sys_base = data->sys_base;
> > + int i;
> > +
> > + for (i = 0; i < num_clks; i++) {
> > + hw = clk_hw_register_gate(NULL, clks[i].name,
> > + clks[i].parent,
> > + clks[i].flags,
> > + sys_base + clks[i].gate_reg,
> > + clks[i].gate_shift,
> > + 0,
> > + &bm1880_clk_lock);
>
> Weird tabs here.
>

Ack.

> > + if (IS_ERR(hw)) {
> > + pr_err("%s: failed to register clock %s\n",
> > + __func__, clks[i].name);
> > + goto err_clk;
> > + }
> > +
> > + data->clk_data->hws[clks[i].id] = hw;
> > + }
> > +
> > + return 0;
> > +
> > +err_clk:
> > + while (i--)
> > + clk_hw_unregister_gate(data->clk_data->hws[clks[i].id]);
> > +
> > + return PTR_ERR(hw);
> > +}
> > +
> > +static struct clk_hw *bm1880_clk_register_composite(struct bm1880_composite_clock *clks,
> > + void __iomem *sys_base)
> > +{
> > + struct clk_hw *hw;
> > + struct clk_mux *mux = NULL;
> > + struct clk_gate *gate = NULL;
> > + struct bm1880_div_hw_clock *div_hws = NULL;
> > + struct clk_hw *mux_hw = NULL, *gate_hw = NULL, *div_hw = NULL;
> > + const struct clk_ops *mux_ops = NULL, *gate_ops = NULL, *div_ops = NULL;
> > + const char * const *parent_names;
> > + const char *parent;
> > + int num_parents;
> > + int ret;
> > +
> > + if (clks->mux_shift >= 0) {
> > + mux = kzalloc(sizeof(*mux), GFP_KERNEL);
> > + if (!mux)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + mux->reg = sys_base + clks->mux_reg;
> > + mux->mask = 1;
> > + mux->shift = clks->mux_shift;
> > + mux_hw = &mux->hw;
> > + mux_ops = &clk_mux_ops;
> > + mux->lock = &bm1880_clk_lock;
> > +
> > + parent_names = clks->parents;
> > + num_parents = clks->num_parents;
> > + } else {
> > + parent = clks->parent;
> > + parent_names = &parent;
> > + num_parents = 1;
> > + }
> > +
> > + if (clks->gate_shift >= 0) {
> > + gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> > + if (!gate) {
> > + ret = -ENOMEM;
> > + goto err_out;
> > + }
> > +
> > + gate->reg = sys_base + clks->gate_reg;
> > + gate->bit_idx = clks->gate_shift;
> > + gate->lock = &bm1880_clk_lock;
> > +
> > + gate_hw = &gate->hw;
> > + gate_ops = &clk_gate_ops;
> > + }
> > +
> > + if (clks->div_shift >= 0) {
> > + div_hws = kzalloc(sizeof(*div_hws), GFP_KERNEL);
> > + if (!div_hws) {
> > + ret = -ENOMEM;
> > + goto err_out;
> > + }
> > +
> > + div_hws->base = sys_base;
> > + div_hws->div.reg = clks->div_reg;
> > + div_hws->div.shift = clks->div_shift;
> > + div_hws->div.width = clks->div_width;
> > + div_hws->div.table = clks->table;
> > + div_hws->div.initval = clks->div_initval;
> > + div_hws->lock = &bm1880_clk_lock;
> > + div_hws->div.flags = CLK_DIVIDER_ONE_BASED |
> > + CLK_DIVIDER_ALLOW_ZERO;
> > +
> > + div_hw = &div_hws->hw;
> > + div_ops = &bm1880_clk_div_ops;
> > + }
> > +
> > + hw = clk_hw_register_composite(NULL, clks->name, parent_names,
> > + num_parents, mux_hw, mux_ops, div_hw,
> > + div_ops, gate_hw, gate_ops,
> > + clks->flags);
> > +
>
> All of these need to be removed on probe failure. Any plans to do so?
>

Removing the composite clocks during failure is handled one level up, in
bm1880_clk_register_composites().

> > + if (IS_ERR(hw)) {
> > + ret = PTR_ERR(hw);
> > + goto err_out;
> > + }
> > +
> > + return hw;
> > +
> > +err_out:
> > + kfree(div_hws);
> > + kfree(gate);
> > + kfree(mux);
> > +
> > + return ERR_PTR(ret);
> > +}
> > +
> > +static int bm1880_clk_register_composites(struct bm1880_composite_clock *clks,
> > + int num_clks, struct bm1880_clock_data *data)
> > +{
> > + struct clk_hw *hw;
> > + void __iomem *sys_base = data->sys_base;
> > + int i;
> > +
> > + for (i = 0; i < num_clks; i++) {
> > + struct bm1880_composite_clock *bm1880_clk = &clks[i];
> > +
> > + hw = bm1880_clk_register_composite(bm1880_clk, sys_base);
> > + if (IS_ERR(hw)) {
> > + pr_err("%s: failed to register clock %s\n",
> > + __func__, bm1880_clk->name);
> > + goto err_clk;
> > + }
> > +
> > + data->clk_data->hws[clks[i].id] = hw;
> > + }
> > +
> > + return 0;
> > +
> > +err_clk:
> > + while (i--)
> > + clk_hw_unregister_composite(data->clk_data->hws[clks[i].id]);
> > +
> > + return PTR_ERR(hw);
> > +}
> > +
> > +static int bm1880_clk_probe(struct platform_device *pdev)
> > +{
> > + struct bm1880_clock_data *clk_data;
> > + void __iomem *pll_base, *sys_base;
> > + struct device_node *np = pdev->dev.of_node;
> > + struct device *dev = &pdev->dev;
> > + struct resource *res;
> > + struct clk_hw_onecell_data *clk_hw_data;
> > + int num_clks, i;
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + pll_base = devm_ioremap_resource(&pdev->dev, res);
> > + if (IS_ERR(pll_base))
> > + return PTR_ERR(pll_base);
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> > + sys_base = devm_ioremap_resource(&pdev->dev, res);
> > + if (IS_ERR(sys_base))
> > + return PTR_ERR(sys_base);
> > +
> > + clk_data = devm_kzalloc(dev, sizeof(*clk_data), GFP_KERNEL);
> > + if (!clk_data)
> > + return -ENOMEM;
> > +
> > + clk_data->pll_base = pll_base;
> > + clk_data->sys_base = sys_base;
> > +
> > + num_clks = ARRAY_SIZE(bm1880_pll_clks) +
> > + ARRAY_SIZE(bm1880_div_clks) +
> > + ARRAY_SIZE(bm1880_mux_clks) +
> > + ARRAY_SIZE(bm1880_composite_clks) +
> > + ARRAY_SIZE(bm1880_gate_clks);
> > +
> > + clk_hw_data = devm_kzalloc(&pdev->dev, struct_size(clk_hw_data, hws,
> > + num_clks), GFP_KERNEL);
> > + if (!clk_hw_data)
> > + return -ENOMEM;
> > +
> > + clk_data->clk_data = clk_hw_data;
> > +
> > + for (i = 0; i < num_clks; i++)
> > + clk_data->clk_data->hws[i] = ERR_PTR(-ENOENT);
> > +
> > + clk_data->clk_data->num = num_clks;
> > +
> > + bm1880_clk_register_plls(bm1880_pll_clks,
> > + ARRAY_SIZE(bm1880_pll_clks),
> > + clk_data);
> > +
> > + bm1880_clk_register_divs(bm1880_div_clks,
> > + ARRAY_SIZE(bm1880_div_clks),
> > + clk_data);
> > +
> > + bm1880_clk_register_mux(bm1880_mux_clks,
> > + ARRAY_SIZE(bm1880_mux_clks),
> > + clk_data);
> > +
> > + bm1880_clk_register_composites(bm1880_composite_clks,
> > + ARRAY_SIZE(bm1880_composite_clks),
> > + clk_data);
> > +
> > + bm1880_clk_register_gate(bm1880_gate_clks,
> > + ARRAY_SIZE(bm1880_gate_clks),
> > + clk_data);
> > +
> > + return of_clk_add_hw_provider(np, of_clk_hw_onecell_get,
>
> Can you use devm_of_clk_add_hw_provider()?
>

okay.

Thanks,
Mani

> > + clk_data->clk_data);
> > +}