1.Adjust existing code to make it compatible with Ingenic X1830 SoC.
2.Add support for the clocks provided by the CGU in the Ingenic X1830
SoC, making use of the cgu code to do the heavy lifting.
3.Add USB related clock for the Ingenic X1000 SoC, and use the
"CLK_OF_DECLARE_DRIVER" instead "CLK_OF_DECLARE" like the
other CGU drivers.
Add the clock bindings for the X1830 Soc from Ingenic.
Signed-off-by: Zhou Yanjie <[email protected]>
---
.../devicetree/bindings/clock/ingenic,cgu.txt | 1 +
include/dt-bindings/clock/x1830-cgu.h | 46 ++++++++++++++++++++++
2 files changed, 47 insertions(+)
create mode 100644 include/dt-bindings/clock/x1830-cgu.h
diff --git a/Documentation/devicetree/bindings/clock/ingenic,cgu.txt b/Documentation/devicetree/bindings/clock/ingenic,cgu.txt
index 75598e6..74bfc57 100644
--- a/Documentation/devicetree/bindings/clock/ingenic,cgu.txt
+++ b/Documentation/devicetree/bindings/clock/ingenic,cgu.txt
@@ -12,6 +12,7 @@ Required properties:
* ingenic,jz4770-cgu
* ingenic,jz4780-cgu
* ingenic,x1000-cgu
+ * ingenic,x1830-cgu
- reg : The address & length of the CGU registers.
- clocks : List of phandle & clock specifiers for clocks external to the CGU.
Two such external clocks should be specified - first the external crystal
diff --git a/include/dt-bindings/clock/x1830-cgu.h b/include/dt-bindings/clock/x1830-cgu.h
new file mode 100644
index 00000000..6499170
--- /dev/null
+++ b/include/dt-bindings/clock/x1830-cgu.h
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * This header provides clock numbers for the ingenic,x1830-cgu DT binding.
+ *
+ * They are roughly ordered as:
+ * - external clocks
+ * - PLLs
+ * - muxes/dividers in the order they appear in the x1830 programmers manual
+ * - gates in order of their bit in the CLKGR* registers
+ */
+
+#ifndef __DT_BINDINGS_CLOCK_X1830_CGU_H__
+#define __DT_BINDINGS_CLOCK_X1830_CGU_H__
+
+#define X1830_CLK_EXCLK 0
+#define X1830_CLK_RTCLK 1
+#define X1830_CLK_APLL 2
+#define X1830_CLK_MPLL 3
+#define X1830_CLK_EPLL 4
+#define X1830_CLK_VPLL 5
+#define X1830_CLK_SCLKA 6
+#define X1830_CLK_CPUMUX 7
+#define X1830_CLK_CPU 8
+#define X1830_CLK_L2CACHE 9
+#define X1830_CLK_AHB0 10
+#define X1830_CLK_AHB2PMUX 11
+#define X1830_CLK_AHB2 12
+#define X1830_CLK_PCLK 13
+#define X1830_CLK_DDR 14
+#define X1830_CLK_MAC 15
+#define X1830_CLK_MSCMUX 16
+#define X1830_CLK_MSC0 17
+#define X1830_CLK_MSC1 18
+#define X1830_CLK_SSIPLL 19
+#define X1830_CLK_SSIMUX 20
+#define X1830_CLK_SSI0 21
+#define X1830_CLK_SMB0 22
+#define X1830_CLK_SMB1 23
+#define X1830_CLK_SMB2 24
+#define X1830_CLK_UART0 25
+#define X1830_CLK_UART1 26
+#define X1830_CLK_SSI1 27
+#define X1830_CLK_SFC 28
+#define X1830_CLK_PDMA 29
+
+#endif /* __DT_BINDINGS_CLOCK_X1830_CGU_H__ */
--
2.7.4
1.Adjust the PLL related code in "cgu.c" and "cgu.h" to make it
compatible with the X1830 Soc from Ingenic.
2.Adjust the code in "jz4740-cgu.c" to be compatible with the
new cgu code.
3.Adjust the code in "jz4725b-cgu.c" to be compatible with the
new cgu code.
4.Adjust the code in "jz4770-cgu.c" to be compatible with the
new cgu code.
5.Adjust the code in "jz4780-cgu.c" to be compatible with the
new cgu code.
6.Adjust the code in "x1000-cgu.c" to be compatible with the
new cgu code.
Signed-off-by: Zhou Yanjie <[email protected]>
---
drivers/clk/ingenic/cgu.c | 55 +++++++++++++++++++++++++++++----------
drivers/clk/ingenic/cgu.h | 12 ++++++++-
drivers/clk/ingenic/jz4725b-cgu.c | 3 ++-
drivers/clk/ingenic/jz4740-cgu.c | 3 ++-
drivers/clk/ingenic/jz4770-cgu.c | 6 +++--
drivers/clk/ingenic/jz4780-cgu.c | 3 ++-
drivers/clk/ingenic/x1000-cgu.c | 6 +++--
7 files changed, 66 insertions(+), 22 deletions(-)
diff --git a/drivers/clk/ingenic/cgu.c b/drivers/clk/ingenic/cgu.c
index 6e96303..c3c69a8 100644
--- a/drivers/clk/ingenic/cgu.c
+++ b/drivers/clk/ingenic/cgu.c
@@ -84,7 +84,7 @@ ingenic_pll_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
pll_info = &clk_info->pll;
spin_lock_irqsave(&cgu->lock, flags);
- ctl = readl(cgu->base + pll_info->reg);
+ ctl = readl(cgu->base + pll_info->reg[1]);
spin_unlock_irqrestore(&cgu->lock, flags);
m = (ctl >> pll_info->m_shift) & GENMASK(pll_info->m_bits - 1, 0);
@@ -93,8 +93,17 @@ ingenic_pll_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
n += pll_info->n_offset;
od_enc = ctl >> pll_info->od_shift;
od_enc &= GENMASK(pll_info->od_bits - 1, 0);
- bypass = !pll_info->no_bypass_bit &&
- !!(ctl & BIT(pll_info->bypass_bit));
+
+ if (pll_info->version >= CGU_X1830) {
+ spin_lock_irqsave(&cgu->lock, flags);
+ ctl = readl(cgu->base + pll_info->reg[0]);
+ spin_unlock_irqrestore(&cgu->lock, flags);
+
+ bypass = !pll_info->no_bypass_bit &&
+ !!(ctl & BIT(pll_info->bypass_bit));
+ } else
+ bypass = !pll_info->no_bypass_bit &&
+ !!(ctl & BIT(pll_info->bypass_bit));
if (bypass)
return parent_rate;
@@ -106,7 +115,10 @@ ingenic_pll_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
BUG_ON(od == pll_info->od_max);
od++;
- return div_u64((u64)parent_rate * m, n * od);
+ if (pll_info->version >= CGU_X1830)
+ return div_u64((u64)parent_rate * m * 2, n * od);
+ else
+ return div_u64((u64)parent_rate * m, n * od);
}
static unsigned long
@@ -139,7 +151,10 @@ ingenic_pll_calc(const struct ingenic_cgu_clk_info *clk_info,
if (pod)
*pod = od;
- return div_u64((u64)parent_rate * m, n * od);
+ if (pll_info->version >= CGU_X1830)
+ return div_u64((u64)parent_rate * m * 2, n * od);
+ else
+ return div_u64((u64)parent_rate * m, n * od);
}
static inline const struct ingenic_cgu_clk_info *to_clk_info(
@@ -183,7 +198,7 @@ ingenic_pll_set_rate(struct clk_hw *hw, unsigned long req_rate,
clk_info->name, req_rate, rate);
spin_lock_irqsave(&cgu->lock, flags);
- ctl = readl(cgu->base + pll_info->reg);
+ ctl = readl(cgu->base + pll_info->reg[1]);
ctl &= ~(GENMASK(pll_info->m_bits - 1, 0) << pll_info->m_shift);
ctl |= (m - pll_info->m_offset) << pll_info->m_shift;
@@ -194,7 +209,7 @@ ingenic_pll_set_rate(struct clk_hw *hw, unsigned long req_rate,
ctl &= ~(GENMASK(pll_info->od_bits - 1, 0) << pll_info->od_shift);
ctl |= pll_info->od_encoding[od - 1] << pll_info->od_shift;
- writel(ctl, cgu->base + pll_info->reg);
+ writel(ctl, cgu->base + pll_info->reg[1]);
spin_unlock_irqrestore(&cgu->lock, flags);
return 0;
@@ -212,16 +227,28 @@ static int ingenic_pll_enable(struct clk_hw *hw)
u32 ctl;
spin_lock_irqsave(&cgu->lock, flags);
- ctl = readl(cgu->base + pll_info->reg);
- ctl &= ~BIT(pll_info->bypass_bit);
+ if (pll_info->version >= CGU_X1830) {
+ ctl = readl(cgu->base + pll_info->reg[0]);
+
+ ctl &= ~BIT(pll_info->bypass_bit);
+
+ writel(ctl, cgu->base + pll_info->reg[0]);
+
+ ctl = readl(cgu->base + pll_info->reg[1]);
+ } else {
+ ctl = readl(cgu->base + pll_info->reg[1]);
+
+ ctl &= ~BIT(pll_info->bypass_bit);
+ }
+
ctl |= BIT(pll_info->enable_bit);
- writel(ctl, cgu->base + pll_info->reg);
+ writel(ctl, cgu->base + pll_info->reg[1]);
/* wait for the PLL to stabilise */
for (i = 0; i < timeout; i++) {
- ctl = readl(cgu->base + pll_info->reg);
+ ctl = readl(cgu->base + pll_info->reg[1]);
if (ctl & BIT(pll_info->stable_bit))
break;
mdelay(1);
@@ -245,11 +272,11 @@ static void ingenic_pll_disable(struct clk_hw *hw)
u32 ctl;
spin_lock_irqsave(&cgu->lock, flags);
- ctl = readl(cgu->base + pll_info->reg);
+ ctl = readl(cgu->base + pll_info->reg[1]);
ctl &= ~BIT(pll_info->enable_bit);
- writel(ctl, cgu->base + pll_info->reg);
+ writel(ctl, cgu->base + pll_info->reg[1]);
spin_unlock_irqrestore(&cgu->lock, flags);
}
@@ -263,7 +290,7 @@ static int ingenic_pll_is_enabled(struct clk_hw *hw)
u32 ctl;
spin_lock_irqsave(&cgu->lock, flags);
- ctl = readl(cgu->base + pll_info->reg);
+ ctl = readl(cgu->base + pll_info->reg[1]);
spin_unlock_irqrestore(&cgu->lock, flags);
return !!(ctl & BIT(pll_info->enable_bit));
diff --git a/drivers/clk/ingenic/cgu.h b/drivers/clk/ingenic/cgu.h
index 0dc8004..5f87be4 100644
--- a/drivers/clk/ingenic/cgu.h
+++ b/drivers/clk/ingenic/cgu.h
@@ -42,8 +42,18 @@
* @stable_bit: the index of the stable bit in the PLL control register
* @no_bypass_bit: if set, the PLL has no bypass functionality
*/
+enum ingenic_cgu_version {
+ CGU_JZ4740,
+ CGU_JZ4725B,
+ CGU_JZ4770,
+ CGU_JZ4780,
+ CGU_X1000,
+ CGU_X1830,
+};
+
struct ingenic_cgu_pll_info {
- unsigned reg;
+ enum ingenic_cgu_version version;
+ unsigned reg[2];
const s8 *od_encoding;
u8 m_shift, m_bits, m_offset;
u8 n_shift, n_bits, n_offset;
diff --git a/drivers/clk/ingenic/jz4725b-cgu.c b/drivers/clk/ingenic/jz4725b-cgu.c
index a3b4635..6da7b41 100644
--- a/drivers/clk/ingenic/jz4725b-cgu.c
+++ b/drivers/clk/ingenic/jz4725b-cgu.c
@@ -53,7 +53,8 @@ static const struct ingenic_cgu_clk_info jz4725b_cgu_clocks[] = {
"pll", CGU_CLK_PLL,
.parents = { JZ4725B_CLK_EXT, -1, -1, -1 },
.pll = {
- .reg = CGU_REG_CPPCR,
+ .version = CGU_JZ4725B,
+ .reg = { -1, CGU_REG_CPPCR },
.m_shift = 23,
.m_bits = 9,
.m_offset = 2,
diff --git a/drivers/clk/ingenic/jz4740-cgu.c b/drivers/clk/ingenic/jz4740-cgu.c
index 4f0e92c..3cf800d 100644
--- a/drivers/clk/ingenic/jz4740-cgu.c
+++ b/drivers/clk/ingenic/jz4740-cgu.c
@@ -68,7 +68,8 @@ static const struct ingenic_cgu_clk_info jz4740_cgu_clocks[] = {
"pll", CGU_CLK_PLL,
.parents = { JZ4740_CLK_EXT, -1, -1, -1 },
.pll = {
- .reg = CGU_REG_CPPCR,
+ .version = CGU_JZ4740,
+ .reg = { -1, CGU_REG_CPPCR },
.m_shift = 23,
.m_bits = 9,
.m_offset = 2,
diff --git a/drivers/clk/ingenic/jz4770-cgu.c b/drivers/clk/ingenic/jz4770-cgu.c
index 956dd65..a62dfb1 100644
--- a/drivers/clk/ingenic/jz4770-cgu.c
+++ b/drivers/clk/ingenic/jz4770-cgu.c
@@ -101,7 +101,8 @@ static const struct ingenic_cgu_clk_info jz4770_cgu_clocks[] = {
"pll0", CGU_CLK_PLL,
.parents = { JZ4770_CLK_EXT },
.pll = {
- .reg = CGU_REG_CPPCR0,
+ .version = CGU_JZ4770,
+ .reg = { -1, CGU_REG_CPPCR0 },
.m_shift = 24,
.m_bits = 7,
.m_offset = 1,
@@ -123,7 +124,8 @@ static const struct ingenic_cgu_clk_info jz4770_cgu_clocks[] = {
"pll1", CGU_CLK_PLL,
.parents = { JZ4770_CLK_EXT },
.pll = {
- .reg = CGU_REG_CPPCR1,
+ .version = CGU_JZ4770,
+ .reg = { -1, CGU_REG_CPPCR1 },
.m_shift = 24,
.m_bits = 7,
.m_offset = 1,
diff --git a/drivers/clk/ingenic/jz4780-cgu.c b/drivers/clk/ingenic/jz4780-cgu.c
index ea905ff..59356d1b 100644
--- a/drivers/clk/ingenic/jz4780-cgu.c
+++ b/drivers/clk/ingenic/jz4780-cgu.c
@@ -220,7 +220,8 @@ static const struct ingenic_cgu_clk_info jz4780_cgu_clocks[] = {
/* PLLs */
#define DEF_PLL(name) { \
- .reg = CGU_REG_ ## name, \
+ .version = CGU_JZ4780, \
+ .reg = { -1, CGU_REG_ ## name }, \
.m_shift = 19, \
.m_bits = 13, \
.m_offset = 1, \
diff --git a/drivers/clk/ingenic/x1000-cgu.c b/drivers/clk/ingenic/x1000-cgu.c
index b22d87b..7179b9f 100644
--- a/drivers/clk/ingenic/x1000-cgu.c
+++ b/drivers/clk/ingenic/x1000-cgu.c
@@ -57,7 +57,8 @@ static const struct ingenic_cgu_clk_info x1000_cgu_clocks[] = {
"apll", CGU_CLK_PLL,
.parents = { X1000_CLK_EXCLK, -1, -1, -1 },
.pll = {
- .reg = CGU_REG_APLL,
+ .version = CGU_X1000,
+ .reg = { -1, CGU_REG_APLL },
.m_shift = 24,
.m_bits = 7,
.m_offset = 1,
@@ -78,7 +79,8 @@ static const struct ingenic_cgu_clk_info x1000_cgu_clocks[] = {
"mpll", CGU_CLK_PLL,
.parents = { X1000_CLK_EXCLK, -1, -1, -1 },
.pll = {
- .reg = CGU_REG_MPLL,
+ .version = CGU_X1000,
+ .reg = { -1, CGU_REG_MPLL },
.m_shift = 24,
.m_bits = 7,
.m_offset = 1,
--
2.7.4
Add support for the clocks provided by the CGU in the Ingenic X1830
SoC, making use of the cgu code to do the heavy lifting.
Signed-off-by: Zhou Yanjie <[email protected]>
---
drivers/clk/ingenic/Kconfig | 10 ++
drivers/clk/ingenic/Makefile | 1 +
drivers/clk/ingenic/x1830-cgu.c | 336 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 347 insertions(+)
create mode 100644 drivers/clk/ingenic/x1830-cgu.c
diff --git a/drivers/clk/ingenic/Kconfig b/drivers/clk/ingenic/Kconfig
index fb7b399..59c6c2c 100644
--- a/drivers/clk/ingenic/Kconfig
+++ b/drivers/clk/ingenic/Kconfig
@@ -55,6 +55,16 @@ config INGENIC_CGU_X1000
If building for a X1000 SoC, you want to say Y here.
+config INGENIC_CGU_X1830
+ bool "Ingenic X1830 CGU driver"
+ default MACH_X1830
+ select INGENIC_CGU_COMMON
+ help
+ Support the clocks provided by the CGU hardware on Ingenic X1830
+ and compatible SoCs.
+
+ If building for a X1830 SoC, you want to say Y here.
+
config INGENIC_TCU_CLK
bool "Ingenic JZ47xx TCU clocks driver"
default MACH_INGENIC
diff --git a/drivers/clk/ingenic/Makefile b/drivers/clk/ingenic/Makefile
index 8b1dad9..aaa4bff 100644
--- a/drivers/clk/ingenic/Makefile
+++ b/drivers/clk/ingenic/Makefile
@@ -5,4 +5,5 @@ obj-$(CONFIG_INGENIC_CGU_JZ4725B) += jz4725b-cgu.o
obj-$(CONFIG_INGENIC_CGU_JZ4770) += jz4770-cgu.o
obj-$(CONFIG_INGENIC_CGU_JZ4780) += jz4780-cgu.o
obj-$(CONFIG_INGENIC_CGU_X1000) += x1000-cgu.o
+obj-$(CONFIG_INGENIC_CGU_X1830) += x1830-cgu.o
obj-$(CONFIG_INGENIC_TCU_CLK) += tcu.o
diff --git a/drivers/clk/ingenic/x1830-cgu.c b/drivers/clk/ingenic/x1830-cgu.c
new file mode 100644
index 00000000..946af6f
--- /dev/null
+++ b/drivers/clk/ingenic/x1830-cgu.c
@@ -0,0 +1,336 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * X1830 SoC CGU driver
+ * Copyright (c) 2019 Zhou Yanjie <[email protected]>
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/delay.h>
+#include <linux/of.h>
+#include <dt-bindings/clock/x1830-cgu.h>
+#include "cgu.h"
+#include "pm.h"
+
+/* CGU register offsets */
+#define CGU_REG_CPCCR 0x00
+#define CGU_REG_CPPCR 0x0c
+#define CGU_REG_APLL 0x10
+#define CGU_REG_MPLL 0x14
+#define CGU_REG_EPLL 0x58
+#define CGU_REG_VPLL 0xe0
+#define CGU_REG_CLKGR0 0x20
+#define CGU_REG_OPCR 0x24
+#define CGU_REG_CLKGR1 0x28
+#define CGU_REG_DDRCDR 0x2c
+#define CGU_REG_USBPCR 0x3c
+#define CGU_REG_USBRDT 0x40
+#define CGU_REG_USBVBFIL 0x44
+#define CGU_REG_USBPCR1 0x48
+#define CGU_REG_MACCDR 0x54
+#define CGU_REG_I2SCDR 0x60
+#define CGU_REG_LPCDR 0x64
+#define CGU_REG_MSC0CDR 0x68
+#define CGU_REG_I2SCDR1 0x70
+#define CGU_REG_SSICDR 0x74
+#define CGU_REG_CIMCDR 0x7c
+#define CGU_REG_MSC1CDR 0xa4
+#define CGU_REG_CMP_INTR 0xb0
+#define CGU_REG_CMP_INTRE 0xb4
+#define CGU_REG_DRCG 0xd0
+#define CGU_REG_CPCSR 0xd4
+#define CGU_REG_MACPHYC 0xe8
+
+/* bits within the OPCR register */
+#define OPCR_SPENDN0 BIT(7)
+#define OPCR_SPENDN1 BIT(6)
+
+static struct ingenic_cgu *cgu;
+
+static const s8 pll_od_encoding[64] = {
+ 0x0, 0x1, -1, 0x2, -1, -1, -1, 0x3,
+ -1, -1, -1, -1, -1, -1, -1, 0x4,
+ -1, -1, -1, -1, -1, -1, -1, -1,
+ -1, -1, -1, -1, -1, -1, -1, 0x5,
+ -1, -1, -1, -1, -1, -1, -1, -1,
+ -1, -1, -1, -1, -1, -1, -1, -1,
+ -1, -1, -1, -1, -1, -1, -1, -1,
+ -1, -1, -1, -1, -1, -1, -1, 0x6,
+};
+
+static const struct ingenic_cgu_clk_info x1830_cgu_clocks[] = {
+
+ /* External clocks */
+
+ [X1830_CLK_EXCLK] = { "ext", CGU_CLK_EXT },
+ [X1830_CLK_RTCLK] = { "rtc", CGU_CLK_EXT },
+
+ /* PLLs */
+
+ [X1830_CLK_APLL] = {
+ "apll", CGU_CLK_PLL,
+ .parents = { X1830_CLK_EXCLK, -1, -1, -1 },
+ .pll = {
+ .version = CGU_X1830,
+ .reg = { CGU_REG_CPPCR, CGU_REG_APLL },
+ .m_shift = 20,
+ .m_bits = 9,
+ .m_offset = 1,
+ .n_shift = 14,
+ .n_bits = 6,
+ .n_offset = 1,
+ .od_shift = 11,
+ .od_bits = 3,
+ .od_max = 64,
+ .od_encoding = pll_od_encoding,
+ .bypass_bit = 30,
+ .enable_bit = 0,
+ .stable_bit = 3,
+ },
+ },
+
+ [X1830_CLK_MPLL] = {
+ "mpll", CGU_CLK_PLL,
+ .parents = { X1830_CLK_EXCLK, -1, -1, -1 },
+ .pll = {
+ .version = CGU_X1830,
+ .reg = { CGU_REG_CPPCR, CGU_REG_MPLL },
+ .m_shift = 20,
+ .m_bits = 9,
+ .m_offset = 1,
+ .n_shift = 14,
+ .n_bits = 6,
+ .n_offset = 1,
+ .od_shift = 11,
+ .od_bits = 3,
+ .od_max = 64,
+ .od_encoding = pll_od_encoding,
+ .bypass_bit = 28,
+ .enable_bit = 0,
+ .stable_bit = 3,
+ },
+ },
+
+ [X1830_CLK_EPLL] = {
+ "epll", CGU_CLK_PLL,
+ .parents = { X1830_CLK_EXCLK, -1, -1, -1 },
+ .pll = {
+ .version = CGU_X1830,
+ .reg = { CGU_REG_CPPCR, CGU_REG_EPLL },
+ .m_shift = 20,
+ .m_bits = 9,
+ .m_offset = 1,
+ .n_shift = 14,
+ .n_bits = 6,
+ .n_offset = 1,
+ .od_shift = 11,
+ .od_bits = 3,
+ .od_max = 64,
+ .od_encoding = pll_od_encoding,
+ .bypass_bit = 24,
+ .enable_bit = 0,
+ .stable_bit = 3,
+ },
+ },
+
+ [X1830_CLK_VPLL] = {
+ "vpll", CGU_CLK_PLL,
+ .parents = { X1830_CLK_EXCLK, -1, -1, -1 },
+ .pll = {
+ .version = CGU_X1830,
+ .reg = { CGU_REG_CPPCR, CGU_REG_VPLL },
+ .m_shift = 20,
+ .m_bits = 9,
+ .m_offset = 1,
+ .n_shift = 14,
+ .n_bits = 6,
+ .n_offset = 1,
+ .od_shift = 11,
+ .od_bits = 3,
+ .od_max = 64,
+ .od_encoding = pll_od_encoding,
+ .bypass_bit = 26,
+ .enable_bit = 0,
+ .stable_bit = 3,
+ },
+ },
+
+ /* Muxes & dividers */
+
+ [X1830_CLK_SCLKA] = {
+ "sclk_a", CGU_CLK_MUX,
+ .parents = { -1, X1830_CLK_EXCLK, X1830_CLK_APLL, -1 },
+ .mux = { CGU_REG_CPCCR, 30, 2 },
+ },
+
+ [X1830_CLK_CPUMUX] = {
+ "cpu_mux", CGU_CLK_MUX,
+ .parents = { -1, X1830_CLK_SCLKA, X1830_CLK_MPLL, -1 },
+ .mux = { CGU_REG_CPCCR, 28, 2 },
+ },
+
+ [X1830_CLK_CPU] = {
+ "cpu", CGU_CLK_DIV,
+ .parents = { X1830_CLK_CPUMUX, -1, -1, -1 },
+ .div = { CGU_REG_CPCCR, 0, 1, 4, 22, -1, -1 },
+ },
+
+ [X1830_CLK_L2CACHE] = {
+ "l2cache", CGU_CLK_DIV,
+ .parents = { X1830_CLK_CPUMUX, -1, -1, -1 },
+ .div = { CGU_REG_CPCCR, 4, 1, 4, 22, -1, -1 },
+ },
+
+ [X1830_CLK_AHB0] = {
+ "ahb0", CGU_CLK_MUX | CGU_CLK_DIV,
+ .parents = { -1, X1830_CLK_SCLKA, X1830_CLK_MPLL, -1 },
+ .mux = { CGU_REG_CPCCR, 26, 2 },
+ .div = { CGU_REG_CPCCR, 8, 1, 4, 21, -1, -1 },
+ },
+
+ [X1830_CLK_AHB2PMUX] = {
+ "ahb2_apb_mux", CGU_CLK_MUX,
+ .parents = { -1, X1830_CLK_SCLKA, X1830_CLK_MPLL, -1 },
+ .mux = { CGU_REG_CPCCR, 24, 2 },
+ },
+
+ [X1830_CLK_AHB2] = {
+ "ahb2", CGU_CLK_DIV,
+ .parents = { X1830_CLK_AHB2PMUX, -1, -1, -1 },
+ .div = { CGU_REG_CPCCR, 12, 1, 4, 20, -1, -1 },
+ },
+
+ [X1830_CLK_PCLK] = {
+ "pclk", CGU_CLK_DIV,
+ .parents = { X1830_CLK_AHB2PMUX, -1, -1, -1 },
+ .div = { CGU_REG_CPCCR, 16, 1, 4, 20, -1, -1 },
+ },
+
+ [X1830_CLK_DDR] = {
+ "ddr", CGU_CLK_MUX | CGU_CLK_DIV | CGU_CLK_GATE,
+ .parents = { -1, X1830_CLK_SCLKA, X1830_CLK_MPLL, -1 },
+ .mux = { CGU_REG_DDRCDR, 30, 2 },
+ .div = { CGU_REG_DDRCDR, 0, 1, 4, 29, 28, 27 },
+ .gate = { CGU_REG_CLKGR0, 31 },
+ },
+
+ [X1830_CLK_MAC] = {
+ "mac", CGU_CLK_MUX | CGU_CLK_DIV | CGU_CLK_GATE,
+ .parents = { X1830_CLK_SCLKA, X1830_CLK_MPLL,
+ X1830_CLK_VPLL, X1830_CLK_EPLL },
+ .mux = { CGU_REG_MACCDR, 30, 2 },
+ .div = { CGU_REG_MACCDR, 0, 1, 8, 29, 28, 27 },
+ .gate = { CGU_REG_CLKGR1, 4 },
+ },
+
+ [X1830_CLK_MSCMUX] = {
+ "msc_mux", CGU_CLK_MUX,
+ .parents = { X1830_CLK_SCLKA, X1830_CLK_MPLL,
+ X1830_CLK_VPLL, X1830_CLK_EPLL },
+ .mux = { CGU_REG_MSC0CDR, 30, 2 },
+ },
+
+ [X1830_CLK_MSC0] = {
+ "msc0", CGU_CLK_DIV | CGU_CLK_GATE,
+ .parents = { X1830_CLK_MSCMUX, -1, -1, -1 },
+ .div = { CGU_REG_MSC0CDR, 0, 2, 8, 29, 28, 27 },
+ .gate = { CGU_REG_CLKGR0, 4 },
+ },
+
+ [X1830_CLK_MSC1] = {
+ "msc1", CGU_CLK_DIV | CGU_CLK_GATE,
+ .parents = { X1830_CLK_MSCMUX, -1, -1, -1 },
+ .div = { CGU_REG_MSC1CDR, 0, 2, 8, 29, 28, 27 },
+ .gate = { CGU_REG_CLKGR0, 5 },
+ },
+
+ [X1830_CLK_SSIPLL] = {
+ "ssi_pll", CGU_CLK_MUX | CGU_CLK_DIV,
+ .parents = { X1830_CLK_SCLKA, X1830_CLK_MPLL,
+ X1830_CLK_VPLL, X1830_CLK_EPLL },
+ .mux = { CGU_REG_SSICDR, 30, 2 },
+ .div = { CGU_REG_SSICDR, 0, 1, 8, 28, 27, 26 },
+ },
+
+ [X1830_CLK_SSIMUX] = {
+ "ssi_mux", CGU_CLK_MUX,
+ .parents = { X1830_CLK_EXCLK, X1830_CLK_SSIPLL, -1, -1 },
+ .mux = { CGU_REG_SSICDR, 29, 1 },
+ },
+
+ /* Gate-only clocks */
+
+ [X1830_CLK_SSI0] = {
+ "ssi0", CGU_CLK_GATE,
+ .parents = { X1830_CLK_SSIMUX, -1, -1, -1 },
+ .gate = { CGU_REG_CLKGR0, 6 },
+ },
+
+ [X1830_CLK_SMB0] = {
+ "smb0", CGU_CLK_GATE,
+ .parents = { X1830_CLK_PCLK, -1, -1, -1 },
+ .gate = { CGU_REG_CLKGR0, 7 },
+ },
+
+ [X1830_CLK_SMB1] = {
+ "smb1", CGU_CLK_GATE,
+ .parents = { X1830_CLK_PCLK, -1, -1, -1 },
+ .gate = { CGU_REG_CLKGR0, 8 },
+ },
+
+ [X1830_CLK_SMB2] = {
+ "smb2", CGU_CLK_GATE,
+ .parents = { X1830_CLK_PCLK, -1, -1, -1 },
+ .gate = { CGU_REG_CLKGR0, 9 },
+ },
+
+ [X1830_CLK_UART0] = {
+ "uart0", CGU_CLK_GATE,
+ .parents = { X1830_CLK_EXCLK, -1, -1, -1 },
+ .gate = { CGU_REG_CLKGR0, 14 },
+ },
+
+ [X1830_CLK_UART1] = {
+ "uart1", CGU_CLK_GATE,
+ .parents = { X1830_CLK_EXCLK, -1, -1, -1 },
+ .gate = { CGU_REG_CLKGR0, 15 },
+ },
+
+ [X1830_CLK_SSI1] = {
+ "ssi1", CGU_CLK_GATE,
+ .parents = { X1830_CLK_SSIMUX, -1, -1, -1 },
+ .gate = { CGU_REG_CLKGR0, 19 },
+ },
+
+ [X1830_CLK_SFC] = {
+ "sfc", CGU_CLK_GATE,
+ .parents = { X1830_CLK_SSIPLL, -1, -1, -1 },
+ .gate = { CGU_REG_CLKGR0, 20 },
+ },
+
+ [X1830_CLK_PDMA] = {
+ "pdma", CGU_CLK_GATE,
+ .parents = { X1830_CLK_EXCLK, -1, -1, -1 },
+ .gate = { CGU_REG_CLKGR0, 21 },
+ },
+};
+
+static void __init x1830_cgu_init(struct device_node *np)
+{
+ int retval;
+
+ cgu = ingenic_cgu_new(x1830_cgu_clocks,
+ ARRAY_SIZE(x1830_cgu_clocks), np);
+ if (!cgu) {
+ pr_err("%s: failed to initialise CGU\n", __func__);
+ return;
+ }
+
+ retval = ingenic_cgu_register_clocks(cgu);
+ if (retval) {
+ pr_err("%s: failed to register CGU Clocks\n", __func__);
+ return;
+ }
+
+ ingenic_cgu_register_syscore_ops(cgu);
+}
+CLK_OF_DECLARE_DRIVER(x1830_cgu, "ingenic,x1830-cgu", x1830_cgu_init);
--
2.7.4
Add the USB OTC clock bindings for the X1000 Soc from Ingenic.
Signed-off-by: Zhou Yanjie <[email protected]>
---
include/dt-bindings/clock/x1000-cgu.h | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/include/dt-bindings/clock/x1000-cgu.h b/include/dt-bindings/clock/x1000-cgu.h
index bbaebaf..c401fce 100644
--- a/include/dt-bindings/clock/x1000-cgu.h
+++ b/include/dt-bindings/clock/x1000-cgu.h
@@ -29,16 +29,17 @@
#define X1000_CLK_MSCMUX 14
#define X1000_CLK_MSC0 15
#define X1000_CLK_MSC1 16
-#define X1000_CLK_SSIPLL 17
-#define X1000_CLK_SSIMUX 18
-#define X1000_CLK_SFC 19
-#define X1000_CLK_I2C0 20
-#define X1000_CLK_I2C1 21
-#define X1000_CLK_I2C2 22
-#define X1000_CLK_UART0 23
-#define X1000_CLK_UART1 24
-#define X1000_CLK_UART2 25
-#define X1000_CLK_SSI 26
-#define X1000_CLK_PDMA 27
+#define X1000_CLK_OTG 17
+#define X1000_CLK_SSIPLL 18
+#define X1000_CLK_SSIMUX 19
+#define X1000_CLK_SFC 20
+#define X1000_CLK_I2C0 21
+#define X1000_CLK_I2C1 22
+#define X1000_CLK_I2C2 23
+#define X1000_CLK_UART0 24
+#define X1000_CLK_UART1 25
+#define X1000_CLK_UART2 26
+#define X1000_CLK_SSI 27
+#define X1000_CLK_PDMA 28
#endif /* __DT_BINDINGS_CLOCK_X1000_CGU_H__ */
--
2.7.4
1.Add the USB OTC clock driver for the X1000 Soc from Ingenic.
2.Use the "CLK_OF_DECLARE_DRIVER" instead "CLK_OF_DECLARE" like
the other CGU drivers.
Signed-off-by: Zhou Yanjie <[email protected]>
---
drivers/clk/ingenic/x1000-cgu.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/ingenic/x1000-cgu.c b/drivers/clk/ingenic/x1000-cgu.c
index 7179b9f..7da7c69 100644
--- a/drivers/clk/ingenic/x1000-cgu.c
+++ b/drivers/clk/ingenic/x1000-cgu.c
@@ -18,6 +18,11 @@
#define CGU_REG_CLKGR 0x20
#define CGU_REG_OPCR 0x24
#define CGU_REG_DDRCDR 0x2c
+#define CGU_REG_USBPCR 0x3c
+#define CGU_REG_USBRDT 0x40
+#define CGU_REG_USBVBFIL 0x44
+#define CGU_REG_USBPCR1 0x48
+#define CGU_REG_USBCDR 0x50
#define CGU_REG_MACCDR 0x54
#define CGU_REG_I2SCDR 0x60
#define CGU_REG_LPCDR 0x64
@@ -184,6 +189,15 @@ static const struct ingenic_cgu_clk_info x1000_cgu_clocks[] = {
.gate = { CGU_REG_CLKGR, 5 },
},
+ [X1000_CLK_OTG] = {
+ "otg", CGU_CLK_DIV | CGU_CLK_GATE | CGU_CLK_MUX,
+ .parents = { X1000_CLK_EXCLK, -1,
+ X1000_CLK_APLL, X1000_CLK_MPLL },
+ .mux = { CGU_REG_USBCDR, 30, 2 },
+ .div = { CGU_REG_USBCDR, 0, 1, 8, 29, 28, 27 },
+ .gate = { CGU_REG_CLKGR, 3 },
+ },
+
[X1000_CLK_SSIPLL] = {
"ssi_pll", CGU_CLK_MUX | CGU_CLK_DIV,
.parents = { X1000_CLK_SCLKA, X1000_CLK_MPLL, -1, -1 },
@@ -273,4 +287,4 @@ static void __init x1000_cgu_init(struct device_node *np)
ingenic_cgu_register_syscore_ops(cgu);
}
-CLK_OF_DECLARE(x1000_cgu, "ingenic,x1000-cgu", x1000_cgu_init);
+CLK_OF_DECLARE_DRIVER(x1000_cgu, "ingenic,x1000-cgu", x1000_cgu_init);
--
2.7.4
Hi Zhou,
Le mer., nov. 27, 2019 at 11:32, Zhou Yanjie <[email protected]> a
?crit :
> Add the USB OTC clock bindings for the X1000 Soc from Ingenic.
>
> Signed-off-by: Zhou Yanjie <[email protected]>
> ---
> include/dt-bindings/clock/x1000-cgu.h | 23 ++++++++++++-----------
> 1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/include/dt-bindings/clock/x1000-cgu.h
> b/include/dt-bindings/clock/x1000-cgu.h
> index bbaebaf..c401fce 100644
> --- a/include/dt-bindings/clock/x1000-cgu.h
> +++ b/include/dt-bindings/clock/x1000-cgu.h
> @@ -29,16 +29,17 @@
> #define X1000_CLK_MSCMUX 14
> #define X1000_CLK_MSC0 15
> #define X1000_CLK_MSC1 16
> -#define X1000_CLK_SSIPLL 17
> -#define X1000_CLK_SSIMUX 18
> -#define X1000_CLK_SFC 19
> -#define X1000_CLK_I2C0 20
> -#define X1000_CLK_I2C1 21
> -#define X1000_CLK_I2C2 22
> -#define X1000_CLK_UART0 23
> -#define X1000_CLK_UART1 24
> -#define X1000_CLK_UART2 25
> -#define X1000_CLK_SSI 26
> -#define X1000_CLK_PDMA 27
You can't do that. These macros are ABI now, since they are used in the
devicetree. Just use the next valid number for your OTG clock.
Cheers,
-Paul
> +#define X1000_CLK_OTG 17
> +#define X1000_CLK_SSIPLL 18
> +#define X1000_CLK_SSIMUX 19
> +#define X1000_CLK_SFC 20
> +#define X1000_CLK_I2C0 21
> +#define X1000_CLK_I2C1 22
> +#define X1000_CLK_I2C2 23
> +#define X1000_CLK_UART0 24
> +#define X1000_CLK_UART1 25
> +#define X1000_CLK_UART2 26
> +#define X1000_CLK_SSI 27
> +#define X1000_CLK_PDMA 28
>
> #endif /* __DT_BINDINGS_CLOCK_X1000_CGU_H__ */
> --
> 2.7.4
>
>
Hi Zhou,
Le mer., nov. 27, 2019 at 11:32, Zhou Yanjie <[email protected]> a
?crit :
> 1.Adjust the PLL related code in "cgu.c" and "cgu.h" to make it
> compatible with the X1830 Soc from Ingenic.
> 2.Adjust the code in "jz4740-cgu.c" to be compatible with the
> new cgu code.
> 3.Adjust the code in "jz4725b-cgu.c" to be compatible with the
> new cgu code.
> 4.Adjust the code in "jz4770-cgu.c" to be compatible with the
> new cgu code.
> 5.Adjust the code in "jz4780-cgu.c" to be compatible with the
> new cgu code.
> 6.Adjust the code in "x1000-cgu.c" to be compatible with the
> new cgu code.
>
> Signed-off-by: Zhou Yanjie <[email protected]>
> ---
> drivers/clk/ingenic/cgu.c | 55
> +++++++++++++++++++++++++++++----------
> drivers/clk/ingenic/cgu.h | 12 ++++++++-
> drivers/clk/ingenic/jz4725b-cgu.c | 3 ++-
> drivers/clk/ingenic/jz4740-cgu.c | 3 ++-
> drivers/clk/ingenic/jz4770-cgu.c | 6 +++--
> drivers/clk/ingenic/jz4780-cgu.c | 3 ++-
> drivers/clk/ingenic/x1000-cgu.c | 6 +++--
> 7 files changed, 66 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/clk/ingenic/cgu.c b/drivers/clk/ingenic/cgu.c
> index 6e96303..c3c69a8 100644
> --- a/drivers/clk/ingenic/cgu.c
> +++ b/drivers/clk/ingenic/cgu.c
> @@ -84,7 +84,7 @@ ingenic_pll_recalc_rate(struct clk_hw *hw, unsigned
> long parent_rate)
> pll_info = &clk_info->pll;
>
> spin_lock_irqsave(&cgu->lock, flags);
> - ctl = readl(cgu->base + pll_info->reg);
> + ctl = readl(cgu->base + pll_info->reg[1]);
I really don't like this patch. There is no info on what reg[1] and
reg[0] are. First, don't use hardcoded numbers, use macros with
meaningful names. Second, why not just have two fields instead of one
2-values array? That would remove a lot of the noise.
> spin_unlock_irqrestore(&cgu->lock, flags);
>
> m = (ctl >> pll_info->m_shift) & GENMASK(pll_info->m_bits - 1, 0);
> @@ -93,8 +93,17 @@ ingenic_pll_recalc_rate(struct clk_hw *hw,
> unsigned long parent_rate)
> n += pll_info->n_offset;
> od_enc = ctl >> pll_info->od_shift;
> od_enc &= GENMASK(pll_info->od_bits - 1, 0);
> - bypass = !pll_info->no_bypass_bit &&
> - !!(ctl & BIT(pll_info->bypass_bit));
> +
> + if (pll_info->version >= CGU_X1830) {
> + spin_lock_irqsave(&cgu->lock, flags);
> + ctl = readl(cgu->base + pll_info->reg[0]);
> + spin_unlock_irqrestore(&cgu->lock, flags);
Why the spinlock?
> +
> + bypass = !pll_info->no_bypass_bit &&
> + !!(ctl & BIT(pll_info->bypass_bit));
> + } else
Please comply to the kernel coding style - use brackets after the else.
> + bypass = !pll_info->no_bypass_bit &&
> + !!(ctl & BIT(pll_info->bypass_bit));
>
> if (bypass)
> return parent_rate;
> @@ -106,7 +115,10 @@ ingenic_pll_recalc_rate(struct clk_hw *hw,
> unsigned long parent_rate)
> BUG_ON(od == pll_info->od_max);
> od++;
>
> - return div_u64((u64)parent_rate * m, n * od);
> + if (pll_info->version >= CGU_X1830)
> + return div_u64((u64)parent_rate * m * 2, n * od);
Where does that *2 come from?
> + else
> + return div_u64((u64)parent_rate * m, n * od);
> }
>
> static unsigned long
> @@ -139,7 +151,10 @@ ingenic_pll_calc(const struct
> ingenic_cgu_clk_info *clk_info,
> if (pod)
> *pod = od;
>
> - return div_u64((u64)parent_rate * m, n * od);
> + if (pll_info->version >= CGU_X1830)
> + return div_u64((u64)parent_rate * m * 2, n * od);
> + else
> + return div_u64((u64)parent_rate * m, n * od);
> }
>
> static inline const struct ingenic_cgu_clk_info *to_clk_info(
> @@ -183,7 +198,7 @@ ingenic_pll_set_rate(struct clk_hw *hw, unsigned
> long req_rate,
> clk_info->name, req_rate, rate);
>
> spin_lock_irqsave(&cgu->lock, flags);
> - ctl = readl(cgu->base + pll_info->reg);
> + ctl = readl(cgu->base + pll_info->reg[1]);
>
> ctl &= ~(GENMASK(pll_info->m_bits - 1, 0) << pll_info->m_shift);
> ctl |= (m - pll_info->m_offset) << pll_info->m_shift;
> @@ -194,7 +209,7 @@ ingenic_pll_set_rate(struct clk_hw *hw, unsigned
> long req_rate,
> ctl &= ~(GENMASK(pll_info->od_bits - 1, 0) << pll_info->od_shift);
> ctl |= pll_info->od_encoding[od - 1] << pll_info->od_shift;
>
> - writel(ctl, cgu->base + pll_info->reg);
> + writel(ctl, cgu->base + pll_info->reg[1]);
> spin_unlock_irqrestore(&cgu->lock, flags);
>
> return 0;
> @@ -212,16 +227,28 @@ static int ingenic_pll_enable(struct clk_hw *hw)
> u32 ctl;
>
> spin_lock_irqsave(&cgu->lock, flags);
> - ctl = readl(cgu->base + pll_info->reg);
>
> - ctl &= ~BIT(pll_info->bypass_bit);
> + if (pll_info->version >= CGU_X1830) {
> + ctl = readl(cgu->base + pll_info->reg[0]);
> +
> + ctl &= ~BIT(pll_info->bypass_bit);
> +
> + writel(ctl, cgu->base + pll_info->reg[0]);
> +
> + ctl = readl(cgu->base + pll_info->reg[1]);
> + } else {
> + ctl = readl(cgu->base + pll_info->reg[1]);
> +
> + ctl &= ~BIT(pll_info->bypass_bit);
> + }
> +
> ctl |= BIT(pll_info->enable_bit);
>
> - writel(ctl, cgu->base + pll_info->reg);
> + writel(ctl, cgu->base + pll_info->reg[1]);
>
> /* wait for the PLL to stabilise */
> for (i = 0; i < timeout; i++) {
> - ctl = readl(cgu->base + pll_info->reg);
> + ctl = readl(cgu->base + pll_info->reg[1]);
> if (ctl & BIT(pll_info->stable_bit))
> break;
> mdelay(1);
> @@ -245,11 +272,11 @@ static void ingenic_pll_disable(struct clk_hw
> *hw)
> u32 ctl;
>
> spin_lock_irqsave(&cgu->lock, flags);
> - ctl = readl(cgu->base + pll_info->reg);
> + ctl = readl(cgu->base + pll_info->reg[1]);
>
> ctl &= ~BIT(pll_info->enable_bit);
>
> - writel(ctl, cgu->base + pll_info->reg);
> + writel(ctl, cgu->base + pll_info->reg[1]);
> spin_unlock_irqrestore(&cgu->lock, flags);
> }
>
> @@ -263,7 +290,7 @@ static int ingenic_pll_is_enabled(struct clk_hw
> *hw)
> u32 ctl;
>
> spin_lock_irqsave(&cgu->lock, flags);
> - ctl = readl(cgu->base + pll_info->reg);
> + ctl = readl(cgu->base + pll_info->reg[1]);
> spin_unlock_irqrestore(&cgu->lock, flags);
>
> return !!(ctl & BIT(pll_info->enable_bit));
> diff --git a/drivers/clk/ingenic/cgu.h b/drivers/clk/ingenic/cgu.h
> index 0dc8004..5f87be4 100644
> --- a/drivers/clk/ingenic/cgu.h
> +++ b/drivers/clk/ingenic/cgu.h
> @@ -42,8 +42,18 @@
> * @stable_bit: the index of the stable bit in the PLL control
> register
> * @no_bypass_bit: if set, the PLL has no bypass functionality
> */
> +enum ingenic_cgu_version {
> + CGU_JZ4740,
> + CGU_JZ4725B,
> + CGU_JZ4770,
> + CGU_JZ4780,
> + CGU_X1000,
> + CGU_X1830,
> +};
> +
> struct ingenic_cgu_pll_info {
> - unsigned reg;
> + enum ingenic_cgu_version version;
> + unsigned reg[2];
> const s8 *od_encoding;
> u8 m_shift, m_bits, m_offset;
> u8 n_shift, n_bits, n_offset;
> diff --git a/drivers/clk/ingenic/jz4725b-cgu.c
> b/drivers/clk/ingenic/jz4725b-cgu.c
> index a3b4635..6da7b41 100644
> --- a/drivers/clk/ingenic/jz4725b-cgu.c
> +++ b/drivers/clk/ingenic/jz4725b-cgu.c
> @@ -53,7 +53,8 @@ static const struct ingenic_cgu_clk_info
> jz4725b_cgu_clocks[] = {
> "pll", CGU_CLK_PLL,
> .parents = { JZ4725B_CLK_EXT, -1, -1, -1 },
> .pll = {
> - .reg = CGU_REG_CPPCR,
> + .version = CGU_JZ4725B,
> + .reg = { -1, CGU_REG_CPPCR },
> .m_shift = 23,
> .m_bits = 9,
> .m_offset = 2,
> diff --git a/drivers/clk/ingenic/jz4740-cgu.c
> b/drivers/clk/ingenic/jz4740-cgu.c
> index 4f0e92c..3cf800d 100644
> --- a/drivers/clk/ingenic/jz4740-cgu.c
> +++ b/drivers/clk/ingenic/jz4740-cgu.c
> @@ -68,7 +68,8 @@ static const struct ingenic_cgu_clk_info
> jz4740_cgu_clocks[] = {
> "pll", CGU_CLK_PLL,
> .parents = { JZ4740_CLK_EXT, -1, -1, -1 },
> .pll = {
> - .reg = CGU_REG_CPPCR,
> + .version = CGU_JZ4740,
> + .reg = { -1, CGU_REG_CPPCR },
> .m_shift = 23,
> .m_bits = 9,
> .m_offset = 2,
> diff --git a/drivers/clk/ingenic/jz4770-cgu.c
> b/drivers/clk/ingenic/jz4770-cgu.c
> index 956dd65..a62dfb1 100644
> --- a/drivers/clk/ingenic/jz4770-cgu.c
> +++ b/drivers/clk/ingenic/jz4770-cgu.c
> @@ -101,7 +101,8 @@ static const struct ingenic_cgu_clk_info
> jz4770_cgu_clocks[] = {
> "pll0", CGU_CLK_PLL,
> .parents = { JZ4770_CLK_EXT },
> .pll = {
> - .reg = CGU_REG_CPPCR0,
> + .version = CGU_JZ4770,
> + .reg = { -1, CGU_REG_CPPCR0 },
> .m_shift = 24,
> .m_bits = 7,
> .m_offset = 1,
> @@ -123,7 +124,8 @@ static const struct ingenic_cgu_clk_info
> jz4770_cgu_clocks[] = {
> "pll1", CGU_CLK_PLL,
> .parents = { JZ4770_CLK_EXT },
> .pll = {
> - .reg = CGU_REG_CPPCR1,
> + .version = CGU_JZ4770,
> + .reg = { -1, CGU_REG_CPPCR1 },
> .m_shift = 24,
> .m_bits = 7,
> .m_offset = 1,
> diff --git a/drivers/clk/ingenic/jz4780-cgu.c
> b/drivers/clk/ingenic/jz4780-cgu.c
> index ea905ff..59356d1b 100644
> --- a/drivers/clk/ingenic/jz4780-cgu.c
> +++ b/drivers/clk/ingenic/jz4780-cgu.c
> @@ -220,7 +220,8 @@ static const struct ingenic_cgu_clk_info
> jz4780_cgu_clocks[] = {
> /* PLLs */
>
> #define DEF_PLL(name) { \
> - .reg = CGU_REG_ ## name, \
> + .version = CGU_JZ4780, \
> + .reg = { -1, CGU_REG_ ## name }, \
> .m_shift = 19, \
> .m_bits = 13, \
> .m_offset = 1, \
> diff --git a/drivers/clk/ingenic/x1000-cgu.c
> b/drivers/clk/ingenic/x1000-cgu.c
> index b22d87b..7179b9f 100644
> --- a/drivers/clk/ingenic/x1000-cgu.c
> +++ b/drivers/clk/ingenic/x1000-cgu.c
> @@ -57,7 +57,8 @@ static const struct ingenic_cgu_clk_info
> x1000_cgu_clocks[] = {
> "apll", CGU_CLK_PLL,
> .parents = { X1000_CLK_EXCLK, -1, -1, -1 },
> .pll = {
> - .reg = CGU_REG_APLL,
> + .version = CGU_X1000,
> + .reg = { -1, CGU_REG_APLL },
> .m_shift = 24,
> .m_bits = 7,
> .m_offset = 1,
> @@ -78,7 +79,8 @@ static const struct ingenic_cgu_clk_info
> x1000_cgu_clocks[] = {
> "mpll", CGU_CLK_PLL,
> .parents = { X1000_CLK_EXCLK, -1, -1, -1 },
> .pll = {
> - .reg = CGU_REG_MPLL,
> + .version = CGU_X1000,
> + .reg = { -1, CGU_REG_MPLL },
> .m_shift = 24,
> .m_bits = 7,
> .m_offset = 1,
> --
> 2.7.4
>
>
Hi Paul,
On 2019年11月28日 01:19, Paul Cercueil wrote:
> Hi Zhou,
>
>
> Le mer., nov. 27, 2019 at 11:32, Zhou Yanjie <[email protected]> a
> écrit :
>> Add the USB OTC clock bindings for the X1000 Soc from Ingenic.
>>
>> Signed-off-by: Zhou Yanjie <[email protected]>
>> ---
>> include/dt-bindings/clock/x1000-cgu.h | 23 ++++++++++++-----------
>> 1 file changed, 12 insertions(+), 11 deletions(-)
>>
>> diff --git a/include/dt-bindings/clock/x1000-cgu.h
>> b/include/dt-bindings/clock/x1000-cgu.h
>> index bbaebaf..c401fce 100644
>> --- a/include/dt-bindings/clock/x1000-cgu.h
>> +++ b/include/dt-bindings/clock/x1000-cgu.h
>> @@ -29,16 +29,17 @@
>> #define X1000_CLK_MSCMUX 14
>> #define X1000_CLK_MSC0 15
>> #define X1000_CLK_MSC1 16
>> -#define X1000_CLK_SSIPLL 17
>> -#define X1000_CLK_SSIMUX 18
>> -#define X1000_CLK_SFC 19
>> -#define X1000_CLK_I2C0 20
>> -#define X1000_CLK_I2C1 21
>> -#define X1000_CLK_I2C2 22
>> -#define X1000_CLK_UART0 23
>> -#define X1000_CLK_UART1 24
>> -#define X1000_CLK_UART2 25
>> -#define X1000_CLK_SSI 26
>> -#define X1000_CLK_PDMA 27
>
> You can't do that. These macros are ABI now, since they are used in
> the devicetree. Just use the next valid number for your OTG clock.
>
My fault, I will fix this in v2.
> Cheers,
> -Paul
>
>> +#define X1000_CLK_OTG 17
>> +#define X1000_CLK_SSIPLL 18
>> +#define X1000_CLK_SSIMUX 19
>> +#define X1000_CLK_SFC 20
>> +#define X1000_CLK_I2C0 21
>> +#define X1000_CLK_I2C1 22
>> +#define X1000_CLK_I2C2 23
>> +#define X1000_CLK_UART0 24
>> +#define X1000_CLK_UART1 25
>> +#define X1000_CLK_UART2 26
>> +#define X1000_CLK_SSI 27
>> +#define X1000_CLK_PDMA 28
>>
>> #endif /* __DT_BINDINGS_CLOCK_X1000_CGU_H__ */
>> --
>> 2.7.4
>>
>>
>
>
Hi Paul,
On 2019年11月28日 01:37, Paul Cercueil wrote:
> Hi Zhou,
>
>
> Le mer., nov. 27, 2019 at 11:32, Zhou Yanjie <[email protected]> a
> écrit :
>> 1.Adjust the PLL related code in "cgu.c" and "cgu.h" to make it
>> compatible with the X1830 Soc from Ingenic.
>> 2.Adjust the code in "jz4740-cgu.c" to be compatible with the
>> new cgu code.
>> 3.Adjust the code in "jz4725b-cgu.c" to be compatible with the
>> new cgu code.
>> 4.Adjust the code in "jz4770-cgu.c" to be compatible with the
>> new cgu code.
>> 5.Adjust the code in "jz4780-cgu.c" to be compatible with the
>> new cgu code.
>> 6.Adjust the code in "x1000-cgu.c" to be compatible with the
>> new cgu code.
>>
>> Signed-off-by: Zhou Yanjie <[email protected]>
>> ---
>> drivers/clk/ingenic/cgu.c | 55
>> +++++++++++++++++++++++++++++----------
>> drivers/clk/ingenic/cgu.h | 12 ++++++++-
>> drivers/clk/ingenic/jz4725b-cgu.c | 3 ++-
>> drivers/clk/ingenic/jz4740-cgu.c | 3 ++-
>> drivers/clk/ingenic/jz4770-cgu.c | 6 +++--
>> drivers/clk/ingenic/jz4780-cgu.c | 3 ++-
>> drivers/clk/ingenic/x1000-cgu.c | 6 +++--
>> 7 files changed, 66 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/clk/ingenic/cgu.c b/drivers/clk/ingenic/cgu.c
>> index 6e96303..c3c69a8 100644
>> --- a/drivers/clk/ingenic/cgu.c
>> +++ b/drivers/clk/ingenic/cgu.c
>> @@ -84,7 +84,7 @@ ingenic_pll_recalc_rate(struct clk_hw *hw, unsigned
>> long parent_rate)
>> pll_info = &clk_info->pll;
>>
>> spin_lock_irqsave(&cgu->lock, flags);
>> - ctl = readl(cgu->base + pll_info->reg);
>> + ctl = readl(cgu->base + pll_info->reg[1]);
>
> I really don't like this patch. There is no info on what reg[1] and
> reg[0] are. First, don't use hardcoded numbers, use macros with
> meaningful names. Second, why not just have two fields instead of one
> 2-values array? That would remove a lot of the noise.
>
Because the X1830's PLL has been greatly changed, the bypass control is
placed in another register, so now two registers may needed to control the
PLL. If two fields are used, all the PLL related code of jz47xx-cgu.c /
x1000-cgu.c
will be more verbose. Using array methods such as:
.reg = { -1, CGU_REG_CPPCR0 },
in jz4770-cgu.c,
or :
.reg = { CGU_REG_CPPCR, CGU_REG_APLL },
in x1830-cgu.c,
can let us intuitively see that how many control registers the PLL has.
Maybe adding a corresponding comment to reg[0] and reg[1] is a better way?
>
>> spin_unlock_irqrestore(&cgu->lock, flags);
>>
>> m = (ctl >> pll_info->m_shift) & GENMASK(pll_info->m_bits - 1, 0);
>> @@ -93,8 +93,17 @@ ingenic_pll_recalc_rate(struct clk_hw *hw,
>> unsigned long parent_rate)
>> n += pll_info->n_offset;
>> od_enc = ctl >> pll_info->od_shift;
>> od_enc &= GENMASK(pll_info->od_bits - 1, 0);
>> - bypass = !pll_info->no_bypass_bit &&
>> - !!(ctl & BIT(pll_info->bypass_bit));
>> +
>> + if (pll_info->version >= CGU_X1830) {
>> + spin_lock_irqsave(&cgu->lock, flags);
>> + ctl = readl(cgu->base + pll_info->reg[0]);
>> + spin_unlock_irqrestore(&cgu->lock, flags);
>
> Why the spinlock?
>
The original code used spinlock when reading the control register,
so when reading this new control register, I think it should also
be added with spinlock.
>
>> +
>> + bypass = !pll_info->no_bypass_bit &&
>> + !!(ctl & BIT(pll_info->bypass_bit));
>> + } else
>
> Please comply to the kernel coding style - use brackets after the else.
>
My fault, I will fix this in v2.
>> + bypass = !pll_info->no_bypass_bit &&
>> + !!(ctl & BIT(pll_info->bypass_bit));
>>
>> if (bypass)
>> return parent_rate;
>> @@ -106,7 +115,10 @@ ingenic_pll_recalc_rate(struct clk_hw *hw,
>> unsigned long parent_rate)
>> BUG_ON(od == pll_info->od_max);
>> od++;
>>
>> - return div_u64((u64)parent_rate * m, n * od);
>> + if (pll_info->version >= CGU_X1830)
>> + return div_u64((u64)parent_rate * m * 2, n * od);
>
> Where does that *2 come from?
This is because the calculation method of the PLL frequency multiplication
of X1830 has changed, and a factor of 2 is added compared to the original
method (on page 381 of the X1830's PM manual).
Thanks and best regards!
>
>> + else
>> + return div_u64((u64)parent_rate * m, n * od);
>> }
>>
>> static unsigned long
>> @@ -139,7 +151,10 @@ ingenic_pll_calc(const struct
>> ingenic_cgu_clk_info *clk_info,
>> if (pod)
>> *pod = od;
>>
>> - return div_u64((u64)parent_rate * m, n * od);
>> + if (pll_info->version >= CGU_X1830)
>> + return div_u64((u64)parent_rate * m * 2, n * od);
>> + else
>> + return div_u64((u64)parent_rate * m, n * od);
>> }
>>
>> static inline const struct ingenic_cgu_clk_info *to_clk_info(
>> @@ -183,7 +198,7 @@ ingenic_pll_set_rate(struct clk_hw *hw, unsigned
>> long req_rate,
>> clk_info->name, req_rate, rate);
>>
>> spin_lock_irqsave(&cgu->lock, flags);
>> - ctl = readl(cgu->base + pll_info->reg);
>> + ctl = readl(cgu->base + pll_info->reg[1]);
>>
>> ctl &= ~(GENMASK(pll_info->m_bits - 1, 0) << pll_info->m_shift);
>> ctl |= (m - pll_info->m_offset) << pll_info->m_shift;
>> @@ -194,7 +209,7 @@ ingenic_pll_set_rate(struct clk_hw *hw, unsigned
>> long req_rate,
>> ctl &= ~(GENMASK(pll_info->od_bits - 1, 0) << pll_info->od_shift);
>> ctl |= pll_info->od_encoding[od - 1] << pll_info->od_shift;
>>
>> - writel(ctl, cgu->base + pll_info->reg);
>> + writel(ctl, cgu->base + pll_info->reg[1]);
>> spin_unlock_irqrestore(&cgu->lock, flags);
>>
>> return 0;
>> @@ -212,16 +227,28 @@ static int ingenic_pll_enable(struct clk_hw *hw)
>> u32 ctl;
>>
>> spin_lock_irqsave(&cgu->lock, flags);
>> - ctl = readl(cgu->base + pll_info->reg);
>>
>> - ctl &= ~BIT(pll_info->bypass_bit);
>> + if (pll_info->version >= CGU_X1830) {
>> + ctl = readl(cgu->base + pll_info->reg[0]);
>> +
>> + ctl &= ~BIT(pll_info->bypass_bit);
>> +
>> + writel(ctl, cgu->base + pll_info->reg[0]);
>> +
>> + ctl = readl(cgu->base + pll_info->reg[1]);
>> + } else {
>> + ctl = readl(cgu->base + pll_info->reg[1]);
>> +
>> + ctl &= ~BIT(pll_info->bypass_bit);
>> + }
>> +
>> ctl |= BIT(pll_info->enable_bit);
>>
>> - writel(ctl, cgu->base + pll_info->reg);
>> + writel(ctl, cgu->base + pll_info->reg[1]);
>>
>> /* wait for the PLL to stabilise */
>> for (i = 0; i < timeout; i++) {
>> - ctl = readl(cgu->base + pll_info->reg);
>> + ctl = readl(cgu->base + pll_info->reg[1]);
>> if (ctl & BIT(pll_info->stable_bit))
>> break;
>> mdelay(1);
>> @@ -245,11 +272,11 @@ static void ingenic_pll_disable(struct clk_hw *hw)
>> u32 ctl;
>>
>> spin_lock_irqsave(&cgu->lock, flags);
>> - ctl = readl(cgu->base + pll_info->reg);
>> + ctl = readl(cgu->base + pll_info->reg[1]);
>>
>> ctl &= ~BIT(pll_info->enable_bit);
>>
>> - writel(ctl, cgu->base + pll_info->reg);
>> + writel(ctl, cgu->base + pll_info->reg[1]);
>> spin_unlock_irqrestore(&cgu->lock, flags);
>> }
>>
>> @@ -263,7 +290,7 @@ static int ingenic_pll_is_enabled(struct clk_hw *hw)
>> u32 ctl;
>>
>> spin_lock_irqsave(&cgu->lock, flags);
>> - ctl = readl(cgu->base + pll_info->reg);
>> + ctl = readl(cgu->base + pll_info->reg[1]);
>> spin_unlock_irqrestore(&cgu->lock, flags);
>>
>> return !!(ctl & BIT(pll_info->enable_bit));
>> diff --git a/drivers/clk/ingenic/cgu.h b/drivers/clk/ingenic/cgu.h
>> index 0dc8004..5f87be4 100644
>> --- a/drivers/clk/ingenic/cgu.h
>> +++ b/drivers/clk/ingenic/cgu.h
>> @@ -42,8 +42,18 @@
>> * @stable_bit: the index of the stable bit in the PLL control register
>> * @no_bypass_bit: if set, the PLL has no bypass functionality
>> */
>> +enum ingenic_cgu_version {
>> + CGU_JZ4740,
>> + CGU_JZ4725B,
>> + CGU_JZ4770,
>> + CGU_JZ4780,
>> + CGU_X1000,
>> + CGU_X1830,
>> +};
>> +
>> struct ingenic_cgu_pll_info {
>> - unsigned reg;
>> + enum ingenic_cgu_version version;
>> + unsigned reg[2];
>> const s8 *od_encoding;
>> u8 m_shift, m_bits, m_offset;
>> u8 n_shift, n_bits, n_offset;
>> diff --git a/drivers/clk/ingenic/jz4725b-cgu.c
>> b/drivers/clk/ingenic/jz4725b-cgu.c
>> index a3b4635..6da7b41 100644
>> --- a/drivers/clk/ingenic/jz4725b-cgu.c
>> +++ b/drivers/clk/ingenic/jz4725b-cgu.c
>> @@ -53,7 +53,8 @@ static const struct ingenic_cgu_clk_info
>> jz4725b_cgu_clocks[] = {
>> "pll", CGU_CLK_PLL,
>> .parents = { JZ4725B_CLK_EXT, -1, -1, -1 },
>> .pll = {
>> - .reg = CGU_REG_CPPCR,
>> + .version = CGU_JZ4725B,
>> + .reg = { -1, CGU_REG_CPPCR },
>> .m_shift = 23,
>> .m_bits = 9,
>> .m_offset = 2,
>> diff --git a/drivers/clk/ingenic/jz4740-cgu.c
>> b/drivers/clk/ingenic/jz4740-cgu.c
>> index 4f0e92c..3cf800d 100644
>> --- a/drivers/clk/ingenic/jz4740-cgu.c
>> +++ b/drivers/clk/ingenic/jz4740-cgu.c
>> @@ -68,7 +68,8 @@ static const struct ingenic_cgu_clk_info
>> jz4740_cgu_clocks[] = {
>> "pll", CGU_CLK_PLL,
>> .parents = { JZ4740_CLK_EXT, -1, -1, -1 },
>> .pll = {
>> - .reg = CGU_REG_CPPCR,
>> + .version = CGU_JZ4740,
>> + .reg = { -1, CGU_REG_CPPCR },
>> .m_shift = 23,
>> .m_bits = 9,
>> .m_offset = 2,
>> diff --git a/drivers/clk/ingenic/jz4770-cgu.c
>> b/drivers/clk/ingenic/jz4770-cgu.c
>> index 956dd65..a62dfb1 100644
>> --- a/drivers/clk/ingenic/jz4770-cgu.c
>> +++ b/drivers/clk/ingenic/jz4770-cgu.c
>> @@ -101,7 +101,8 @@ static const struct ingenic_cgu_clk_info
>> jz4770_cgu_clocks[] = {
>> "pll0", CGU_CLK_PLL,
>> .parents = { JZ4770_CLK_EXT },
>> .pll = {
>> - .reg = CGU_REG_CPPCR0,
>> + .version = CGU_JZ4770,
>> + .reg = { -1, CGU_REG_CPPCR0 },
>> .m_shift = 24,
>> .m_bits = 7,
>> .m_offset = 1,
>> @@ -123,7 +124,8 @@ static const struct ingenic_cgu_clk_info
>> jz4770_cgu_clocks[] = {
>> "pll1", CGU_CLK_PLL,
>> .parents = { JZ4770_CLK_EXT },
>> .pll = {
>> - .reg = CGU_REG_CPPCR1,
>> + .version = CGU_JZ4770,
>> + .reg = { -1, CGU_REG_CPPCR1 },
>> .m_shift = 24,
>> .m_bits = 7,
>> .m_offset = 1,
>> diff --git a/drivers/clk/ingenic/jz4780-cgu.c
>> b/drivers/clk/ingenic/jz4780-cgu.c
>> index ea905ff..59356d1b 100644
>> --- a/drivers/clk/ingenic/jz4780-cgu.c
>> +++ b/drivers/clk/ingenic/jz4780-cgu.c
>> @@ -220,7 +220,8 @@ static const struct ingenic_cgu_clk_info
>> jz4780_cgu_clocks[] = {
>> /* PLLs */
>>
>> #define DEF_PLL(name) { \
>> - .reg = CGU_REG_ ## name, \
>> + .version = CGU_JZ4780, \
>> + .reg = { -1, CGU_REG_ ## name }, \
>> .m_shift = 19, \
>> .m_bits = 13, \
>> .m_offset = 1, \
>> diff --git a/drivers/clk/ingenic/x1000-cgu.c
>> b/drivers/clk/ingenic/x1000-cgu.c
>> index b22d87b..7179b9f 100644
>> --- a/drivers/clk/ingenic/x1000-cgu.c
>> +++ b/drivers/clk/ingenic/x1000-cgu.c
>> @@ -57,7 +57,8 @@ static const struct ingenic_cgu_clk_info
>> x1000_cgu_clocks[] = {
>> "apll", CGU_CLK_PLL,
>> .parents = { X1000_CLK_EXCLK, -1, -1, -1 },
>> .pll = {
>> - .reg = CGU_REG_APLL,
>> + .version = CGU_X1000,
>> + .reg = { -1, CGU_REG_APLL },
>> .m_shift = 24,
>> .m_bits = 7,
>> .m_offset = 1,
>> @@ -78,7 +79,8 @@ static const struct ingenic_cgu_clk_info
>> x1000_cgu_clocks[] = {
>> "mpll", CGU_CLK_PLL,
>> .parents = { X1000_CLK_EXCLK, -1, -1, -1 },
>> .pll = {
>> - .reg = CGU_REG_MPLL,
>> + .version = CGU_X1000,
>> + .reg = { -1, CGU_REG_MPLL },
>> .m_shift = 24,
>> .m_bits = 7,
>> .m_offset = 1,
>> --
>> 2.7.4
>>
>>
>
>
Hi Zhou,
>> Le mer., nov. 27, 2019 at 11:32, Zhou Yanjie <[email protected]> a
>> ?crit :
>>> 1.Adjust the PLL related code in "cgu.c" and "cgu.h" to make it
>>> compatible with the X1830 Soc from Ingenic.
>>> 2.Adjust the code in "jz4740-cgu.c" to be compatible with the
>>> new cgu code.
>>> 3.Adjust the code in "jz4725b-cgu.c" to be compatible with the
>>> new cgu code.
>>> 4.Adjust the code in "jz4770-cgu.c" to be compatible with the
>>> new cgu code.
>>> 5.Adjust the code in "jz4780-cgu.c" to be compatible with the
>>> new cgu code.
>>> 6.Adjust the code in "x1000-cgu.c" to be compatible with the
>>> new cgu code.
>>>
>>> Signed-off-by: Zhou Yanjie <[email protected]>
>>> ---
>>> drivers/clk/ingenic/cgu.c | 55
>>> +++++++++++++++++++++++++++++----------
>>> drivers/clk/ingenic/cgu.h | 12 ++++++++-
>>> drivers/clk/ingenic/jz4725b-cgu.c | 3 ++-
>>> drivers/clk/ingenic/jz4740-cgu.c | 3 ++-
>>> drivers/clk/ingenic/jz4770-cgu.c | 6 +++--
>>> drivers/clk/ingenic/jz4780-cgu.c | 3 ++-
>>> drivers/clk/ingenic/x1000-cgu.c | 6 +++--
>>> 7 files changed, 66 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/drivers/clk/ingenic/cgu.c b/drivers/clk/ingenic/cgu.c
>>> index 6e96303..c3c69a8 100644
>>> --- a/drivers/clk/ingenic/cgu.c
>>> +++ b/drivers/clk/ingenic/cgu.c
>>> @@ -84,7 +84,7 @@ ingenic_pll_recalc_rate(struct clk_hw *hw,
>>> unsigned long parent_rate)
>>> pll_info = &clk_info->pll;
>>>
>>> spin_lock_irqsave(&cgu->lock, flags);
>>> - ctl = readl(cgu->base + pll_info->reg);
>>> + ctl = readl(cgu->base + pll_info->reg[1]);
>>
>> I really don't like this patch. There is no info on what reg[1] and
>> reg[0] are. First, don't use hardcoded numbers, use macros with
>> meaningful names. Second, why not just have two fields instead of
>> one 2-values array? That would remove a lot of the noise.
>>
>
> Because the X1830's PLL has been greatly changed, the bypass control
> is
> placed in another register, so now two registers may needed to
> control the
> PLL.
That kind of info belongs in the commit message.
> If two fields are used, all the PLL related code of jz47xx-cgu.c /
> x1000-cgu.c
> will be more verbose. Using array methods such as:
> .reg = { -1, CGU_REG_CPPCR0 },
> in jz4770-cgu.c,
>
> or :
> .reg = { CGU_REG_CPPCR, CGU_REG_APLL },
> in x1830-cgu.c,
>
> can let us intuitively see that how many control registers the PLL
> has.
> Maybe adding a corresponding comment to reg[0] and reg[1] is a better
> way?
I don't see how having two fields would make the code more verbose.
From what I can see it would make this patch much smaller...
>>
>>> spin_unlock_irqrestore(&cgu->lock, flags);
>>>
>>> m = (ctl >> pll_info->m_shift) & GENMASK(pll_info->m_bits - 1,
>>> 0);
>>> @@ -93,8 +93,17 @@ ingenic_pll_recalc_rate(struct clk_hw *hw,
>>> unsigned long parent_rate)
>>> n += pll_info->n_offset;
>>> od_enc = ctl >> pll_info->od_shift;
>>> od_enc &= GENMASK(pll_info->od_bits - 1, 0);
>>> - bypass = !pll_info->no_bypass_bit &&
>>> - !!(ctl & BIT(pll_info->bypass_bit));
>>> +
>>> + if (pll_info->version >= CGU_X1830) {
>>> + spin_lock_irqsave(&cgu->lock, flags);
>>> + ctl = readl(cgu->base + pll_info->reg[0]);
>>> + spin_unlock_irqrestore(&cgu->lock, flags);
>>
>> Why the spinlock?
>>
>
> The original code used spinlock when reading the control register,
> so when reading this new control register, I think it should also
> be added with spinlock.
Well, the original code looks wrong to me. There's nothing to protect
here.
Maybe @Paul Burton can shed some light?
>>> +
>>> + bypass = !pll_info->no_bypass_bit &&
>>> + !!(ctl & BIT(pll_info->bypass_bit));
>>> + } else
>>
>> Please comply to the kernel coding style - use brackets after the
>> else.
>>
>
> My fault, I will fix this in v2.
>
>>> + bypass = !pll_info->no_bypass_bit &&
>>> + !!(ctl & BIT(pll_info->bypass_bit));
>>>
>>> if (bypass)
>>> return parent_rate;
>>> @@ -106,7 +115,10 @@ ingenic_pll_recalc_rate(struct clk_hw *hw,
>>> unsigned long parent_rate)
>>> BUG_ON(od == pll_info->od_max);
>>> od++;
>>>
>>> - return div_u64((u64)parent_rate * m, n * od);
>>> + if (pll_info->version >= CGU_X1830)
>>> + return div_u64((u64)parent_rate * m * 2, n * od);
>>
>> Where does that *2 come from?
>
> This is because the calculation method of the PLL frequency
> multiplication
> of X1830 has changed, and a factor of 2 is added compared to the
> original
> method (on page 381 of the X1830's PM manual).
Instead of adding a pll_info->version, add a pll_info->rate_multiplier,
set it to 1 everywhere and 2 on the X1830.
Cheers,
-Paul
>>
>>> + else
>>> + return div_u64((u64)parent_rate * m, n * od);
>>> }
>>>
>>> static unsigned long
>>> @@ -139,7 +151,10 @@ ingenic_pll_calc(const struct
>>> ingenic_cgu_clk_info *clk_info,
>>> if (pod)
>>> *pod = od;
>>>
>>> - return div_u64((u64)parent_rate * m, n * od);
>>> + if (pll_info->version >= CGU_X1830)
>>> + return div_u64((u64)parent_rate * m * 2, n * od);
>>> + else
>>> + return div_u64((u64)parent_rate * m, n * od);
>>> }
>>>
>>> static inline const struct ingenic_cgu_clk_info *to_clk_info(
>>> @@ -183,7 +198,7 @@ ingenic_pll_set_rate(struct clk_hw *hw,
>>> unsigned long req_rate,
>>> clk_info->name, req_rate, rate);
>>>
>>> spin_lock_irqsave(&cgu->lock, flags);
>>> - ctl = readl(cgu->base + pll_info->reg);
>>> + ctl = readl(cgu->base + pll_info->reg[1]);
>>>
>>> ctl &= ~(GENMASK(pll_info->m_bits - 1, 0) <<
>>> pll_info->m_shift);
>>> ctl |= (m - pll_info->m_offset) << pll_info->m_shift;
>>> @@ -194,7 +209,7 @@ ingenic_pll_set_rate(struct clk_hw *hw,
>>> unsigned long req_rate,
>>> ctl &= ~(GENMASK(pll_info->od_bits - 1, 0) <<
>>> pll_info->od_shift);
>>> ctl |= pll_info->od_encoding[od - 1] << pll_info->od_shift;
>>>
>>> - writel(ctl, cgu->base + pll_info->reg);
>>> + writel(ctl, cgu->base + pll_info->reg[1]);
>>> spin_unlock_irqrestore(&cgu->lock, flags);
>>>
>>> return 0;
>>> @@ -212,16 +227,28 @@ static int ingenic_pll_enable(struct clk_hw
>>> *hw)
>>> u32 ctl;
>>>
>>> spin_lock_irqsave(&cgu->lock, flags);
>>> - ctl = readl(cgu->base + pll_info->reg);
>>>
>>> - ctl &= ~BIT(pll_info->bypass_bit);
>>> + if (pll_info->version >= CGU_X1830) {
>>> + ctl = readl(cgu->base + pll_info->reg[0]);
>>> +
>>> + ctl &= ~BIT(pll_info->bypass_bit);
>>> +
>>> + writel(ctl, cgu->base + pll_info->reg[0]);
>>> +
>>> + ctl = readl(cgu->base + pll_info->reg[1]);
>>> + } else {
>>> + ctl = readl(cgu->base + pll_info->reg[1]);
>>> +
>>> + ctl &= ~BIT(pll_info->bypass_bit);
>>> + }
>>> +
>>> ctl |= BIT(pll_info->enable_bit);
>>>
>>> - writel(ctl, cgu->base + pll_info->reg);
>>> + writel(ctl, cgu->base + pll_info->reg[1]);
>>>
>>> /* wait for the PLL to stabilise */
>>> for (i = 0; i < timeout; i++) {
>>> - ctl = readl(cgu->base + pll_info->reg);
>>> + ctl = readl(cgu->base + pll_info->reg[1]);
>>> if (ctl & BIT(pll_info->stable_bit))
>>> break;
>>> mdelay(1);
>>> @@ -245,11 +272,11 @@ static void ingenic_pll_disable(struct clk_hw
>>> *hw)
>>> u32 ctl;
>>>
>>> spin_lock_irqsave(&cgu->lock, flags);
>>> - ctl = readl(cgu->base + pll_info->reg);
>>> + ctl = readl(cgu->base + pll_info->reg[1]);
>>>
>>> ctl &= ~BIT(pll_info->enable_bit);
>>>
>>> - writel(ctl, cgu->base + pll_info->reg);
>>> + writel(ctl, cgu->base + pll_info->reg[1]);
>>> spin_unlock_irqrestore(&cgu->lock, flags);
>>> }
>>>
>>> @@ -263,7 +290,7 @@ static int ingenic_pll_is_enabled(struct clk_hw
>>> *hw)
>>> u32 ctl;
>>>
>>> spin_lock_irqsave(&cgu->lock, flags);
>>> - ctl = readl(cgu->base + pll_info->reg);
>>> + ctl = readl(cgu->base + pll_info->reg[1]);
>>> spin_unlock_irqrestore(&cgu->lock, flags);
>>>
>>> return !!(ctl & BIT(pll_info->enable_bit));
>>> diff --git a/drivers/clk/ingenic/cgu.h b/drivers/clk/ingenic/cgu.h
>>> index 0dc8004..5f87be4 100644
>>> --- a/drivers/clk/ingenic/cgu.h
>>> +++ b/drivers/clk/ingenic/cgu.h
>>> @@ -42,8 +42,18 @@
>>> * @stable_bit: the index of the stable bit in the PLL control
>>> register
>>> * @no_bypass_bit: if set, the PLL has no bypass functionality
>>> */
>>> +enum ingenic_cgu_version {
>>> + CGU_JZ4740,
>>> + CGU_JZ4725B,
>>> + CGU_JZ4770,
>>> + CGU_JZ4780,
>>> + CGU_X1000,
>>> + CGU_X1830,
>>> +};
>>> +
>>> struct ingenic_cgu_pll_info {
>>> - unsigned reg;
>>> + enum ingenic_cgu_version version;
>>> + unsigned reg[2];
>>> const s8 *od_encoding;
>>> u8 m_shift, m_bits, m_offset;
>>> u8 n_shift, n_bits, n_offset;
>>> diff --git a/drivers/clk/ingenic/jz4725b-cgu.c
>>> b/drivers/clk/ingenic/jz4725b-cgu.c
>>> index a3b4635..6da7b41 100644
>>> --- a/drivers/clk/ingenic/jz4725b-cgu.c
>>> +++ b/drivers/clk/ingenic/jz4725b-cgu.c
>>> @@ -53,7 +53,8 @@ static const struct ingenic_cgu_clk_info
>>> jz4725b_cgu_clocks[] = {
>>> "pll", CGU_CLK_PLL,
>>> .parents = { JZ4725B_CLK_EXT, -1, -1, -1 },
>>> .pll = {
>>> - .reg = CGU_REG_CPPCR,
>>> + .version = CGU_JZ4725B,
>>> + .reg = { -1, CGU_REG_CPPCR },
>>> .m_shift = 23,
>>> .m_bits = 9,
>>> .m_offset = 2,
>>> diff --git a/drivers/clk/ingenic/jz4740-cgu.c
>>> b/drivers/clk/ingenic/jz4740-cgu.c
>>> index 4f0e92c..3cf800d 100644
>>> --- a/drivers/clk/ingenic/jz4740-cgu.c
>>> +++ b/drivers/clk/ingenic/jz4740-cgu.c
>>> @@ -68,7 +68,8 @@ static const struct ingenic_cgu_clk_info
>>> jz4740_cgu_clocks[] = {
>>> "pll", CGU_CLK_PLL,
>>> .parents = { JZ4740_CLK_EXT, -1, -1, -1 },
>>> .pll = {
>>> - .reg = CGU_REG_CPPCR,
>>> + .version = CGU_JZ4740,
>>> + .reg = { -1, CGU_REG_CPPCR },
>>> .m_shift = 23,
>>> .m_bits = 9,
>>> .m_offset = 2,
>>> diff --git a/drivers/clk/ingenic/jz4770-cgu.c
>>> b/drivers/clk/ingenic/jz4770-cgu.c
>>> index 956dd65..a62dfb1 100644
>>> --- a/drivers/clk/ingenic/jz4770-cgu.c
>>> +++ b/drivers/clk/ingenic/jz4770-cgu.c
>>> @@ -101,7 +101,8 @@ static const struct ingenic_cgu_clk_info
>>> jz4770_cgu_clocks[] = {
>>> "pll0", CGU_CLK_PLL,
>>> .parents = { JZ4770_CLK_EXT },
>>> .pll = {
>>> - .reg = CGU_REG_CPPCR0,
>>> + .version = CGU_JZ4770,
>>> + .reg = { -1, CGU_REG_CPPCR0 },
>>> .m_shift = 24,
>>> .m_bits = 7,
>>> .m_offset = 1,
>>> @@ -123,7 +124,8 @@ static const struct ingenic_cgu_clk_info
>>> jz4770_cgu_clocks[] = {
>>> "pll1", CGU_CLK_PLL,
>>> .parents = { JZ4770_CLK_EXT },
>>> .pll = {
>>> - .reg = CGU_REG_CPPCR1,
>>> + .version = CGU_JZ4770,
>>> + .reg = { -1, CGU_REG_CPPCR1 },
>>> .m_shift = 24,
>>> .m_bits = 7,
>>> .m_offset = 1,
>>> diff --git a/drivers/clk/ingenic/jz4780-cgu.c
>>> b/drivers/clk/ingenic/jz4780-cgu.c
>>> index ea905ff..59356d1b 100644
>>> --- a/drivers/clk/ingenic/jz4780-cgu.c
>>> +++ b/drivers/clk/ingenic/jz4780-cgu.c
>>> @@ -220,7 +220,8 @@ static const struct ingenic_cgu_clk_info
>>> jz4780_cgu_clocks[] = {
>>> /* PLLs */
>>>
>>> #define DEF_PLL(name) { \
>>> - .reg = CGU_REG_ ## name, \
>>> + .version = CGU_JZ4780, \
>>> + .reg = { -1, CGU_REG_ ## name }, \
>>> .m_shift = 19, \
>>> .m_bits = 13, \
>>> .m_offset = 1, \
>>> diff --git a/drivers/clk/ingenic/x1000-cgu.c
>>> b/drivers/clk/ingenic/x1000-cgu.c
>>> index b22d87b..7179b9f 100644
>>> --- a/drivers/clk/ingenic/x1000-cgu.c
>>> +++ b/drivers/clk/ingenic/x1000-cgu.c
>>> @@ -57,7 +57,8 @@ static const struct ingenic_cgu_clk_info
>>> x1000_cgu_clocks[] = {
>>> "apll", CGU_CLK_PLL,
>>> .parents = { X1000_CLK_EXCLK, -1, -1, -1 },
>>> .pll = {
>>> - .reg = CGU_REG_APLL,
>>> + .version = CGU_X1000,
>>> + .reg = { -1, CGU_REG_APLL },
>>> .m_shift = 24,
>>> .m_bits = 7,
>>> .m_offset = 1,
>>> @@ -78,7 +79,8 @@ static const struct ingenic_cgu_clk_info
>>> x1000_cgu_clocks[] = {
>>> "mpll", CGU_CLK_PLL,
>>> .parents = { X1000_CLK_EXCLK, -1, -1, -1 },
>>> .pll = {
>>> - .reg = CGU_REG_MPLL,
>>> + .version = CGU_X1000,
>>> + .reg = { -1, CGU_REG_MPLL },
>>> .m_shift = 24,
>>> .m_bits = 7,
>>> .m_offset = 1,
>>> --
>>> 2.7.4
>>>
>>>
>>
>>
>
>
>
On Wed, 27 Nov 2019 11:32:53 +0800, Zhou Yanjie wrote:
> Add the clock bindings for the X1830 Soc from Ingenic.
>
> Signed-off-by: Zhou Yanjie <[email protected]>
> ---
> .../devicetree/bindings/clock/ingenic,cgu.txt | 1 +
> include/dt-bindings/clock/x1830-cgu.h | 46 ++++++++++++++++++++++
> 2 files changed, 47 insertions(+)
> create mode 100644 include/dt-bindings/clock/x1830-cgu.h
>
Reviewed-by: Rob Herring <[email protected]>
Hi Paul, Zhou,
On Fri, Nov 29, 2019 at 12:23:42PM +0100, Paul Cercueil wrote:
> > > > @@ -93,8 +93,17 @@ ingenic_pll_recalc_rate(struct clk_hw *hw,
> > > > unsigned long parent_rate)
> > > > n += pll_info->n_offset;
> > > > od_enc = ctl >> pll_info->od_shift;
> > > > od_enc &= GENMASK(pll_info->od_bits - 1, 0);
> > > > - bypass = !pll_info->no_bypass_bit &&
> > > > - !!(ctl & BIT(pll_info->bypass_bit));
> > > > +
> > > > + if (pll_info->version >= CGU_X1830) {
> > > > + spin_lock_irqsave(&cgu->lock, flags);
> > > > + ctl = readl(cgu->base + pll_info->reg[0]);
> > > > + spin_unlock_irqrestore(&cgu->lock, flags);
> > >
> > > Why the spinlock?
> > >
> >
> > The original code used spinlock when reading the control register,
> > so when reading this new control register, I think it should also
> > be added with spinlock.
>
> Well, the original code looks wrong to me. There's nothing to protect here.
>
> Maybe @Paul Burton can shed some light?
I wish I could remember, but I agree it seems pointless here. The only
way I can think it could be of any use is if writes to the CGU register
we're accessing aren't atomic (ie. if we could observe a partially
completed write), but I don't believe that's the case.
So Zhou, if you want to drop the spinlock here from your X1830 path &
ideally also add a patch to remove it in the non-X1830 path that would
be great.
Thanks,
Paul