2023-04-24 09:55:34

by Nikita Shubin

Subject: [PATCH 12/43] clk: ep93xx: add DT support for Cirrus EP93xx

This is a rewrite of EP93xx timer driver in
arch/arm/mach-ep93xx/clock.c trying to do everything
the device tree way:

- convert to syscon driver
- provide clock acces via of

Co-developed-by: Alexander Sverdlin <[email protected]>
Signed-off-by: Alexander Sverdlin <[email protected]>
Signed-off-by: Nikita Shubin <[email protected]>
---

Notes:
rfc->v0
Linus Walleij:
- renamed all __underscore

Alexander Sverdlin:
- "Logick" -> "Logic"

Changes by Alexander Sverdlin:
- remove pr_info
- DIV_ROUND_UP_ULL -> DIV_ROUND_CLOSEST
- fix zeroing bitfield in ep93xx_div_set_rate
- add sanity check for EP93XX_SYSCON_CHIPID_ID
- use bit index for DMA clock's
- ep93xx_clk_register_gate() takes bit index, not mask
- remove redundant define
- use DIV_ROUND_CLOSEST() everywhere to achieve frequencies closer to those
requested
- Add the forgotten configuration from the deleted
arch/arm/mach-ep93xx/core.c

drivers/clk/Kconfig | 8 +
drivers/clk/Makefile | 1 +
drivers/clk/clk-ep93xx.c | 880 +++++++++++++++++++++++++++++++++++++++
3 files changed, 889 insertions(+)
create mode 100644 drivers/clk/clk-ep93xx.c

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 1eef05bb1f99..d3ad975b688e 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -436,6 +436,14 @@ config COMMON_CLK_K210
help
Support for the Canaan Kendryte K210 RISC-V SoC clocks.

+config COMMON_CLK_EP93XX
+ bool "Clock driver for Cirrus Logic ep93xx SoC"
+ depends on ARCH_EP93XX || COMPILE_TEST
+ select MFD_SYSCON
+ select REGMAP
+ help
+ This driver supports the SoC clocks on the Cirrus Logic ep93xx.
+
source "drivers/clk/actions/Kconfig"
source "drivers/clk/analogbits/Kconfig"
source "drivers/clk/baikal-t1/Kconfig"
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index e3ca0d058a25..deec25ffd004 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_COMMON_CLK_CDCE706) += clk-cdce706.o
obj-$(CONFIG_COMMON_CLK_CDCE925) += clk-cdce925.o
obj-$(CONFIG_ARCH_CLPS711X) += clk-clps711x.o
obj-$(CONFIG_COMMON_CLK_CS2000_CP) += clk-cs2000-cp.o
+obj-$(CONFIG_COMMON_CLK_EP93XX) += clk-ep93xx.o
obj-$(CONFIG_ARCH_SPARX5) += clk-sparx5.o
obj-$(CONFIG_COMMON_CLK_EN7523) += clk-en7523.o
obj-$(CONFIG_COMMON_CLK_FIXED_MMIO) += clk-fixed-mmio.o
diff --git a/drivers/clk/clk-ep93xx.c b/drivers/clk/clk-ep93xx.c
new file mode 100644
index 000000000000..e5913fc4c39b
--- /dev/null
+++ b/drivers/clk/clk-ep93xx.c
@@ -0,0 +1,880 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Clock control for Cirrus EP93xx chips.
+ * Copyright (C) 2021 Nikita Shubin <[email protected]>
+ *
+ * Based on a rewrite of arch/arm/mach-ep93xx/clock.c:
+ * Copyright (C) 2006 Lennert Buytenhek <[email protected]>
+ */
+#define pr_fmt(fmt) "ep93xx " KBUILD_MODNAME ": " fmt
+
+#include <linux/kernel.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/string.h>
+#include <linux/io.h>
+#include <linux/spinlock.h>
+#include <linux/clkdev.h>
+#include <linux/clk-provider.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/mfd/syscon.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/soc/cirrus/ep93xx.h>
+#include <dt-bindings/clock/cirrus,ep93xx-clock.h>
+
+#include <asm/div64.h>
+
+#define EP93XX_EXT_CLK_RATE 14745600
+#define EP93XX_EXT_RTC_RATE 32768
+
+#define EP93XX_SYSCON_POWER_STATE 0x00
+#define EP93XX_SYSCON_PWRCNT 0x04
+#define EP93XX_SYSCON_PWRCNT_UARTBAUD BIT(29)
+#define EP93XX_SYSCON_PWRCNT_USH_EN 28
+#define EP93XX_SYSCON_PWRCNT_DMA_M2M1 27
+#define EP93XX_SYSCON_PWRCNT_DMA_M2M0 26
+#define EP93XX_SYSCON_PWRCNT_DMA_M2P8 25
+#define EP93XX_SYSCON_PWRCNT_DMA_M2P9 24
+#define EP93XX_SYSCON_PWRCNT_DMA_M2P6 23
+#define EP93XX_SYSCON_PWRCNT_DMA_M2P7 22
+#define EP93XX_SYSCON_PWRCNT_DMA_M2P4 21
+#define EP93XX_SYSCON_PWRCNT_DMA_M2P5 20
+#define EP93XX_SYSCON_PWRCNT_DMA_M2P2 19
+#define EP93XX_SYSCON_PWRCNT_DMA_M2P3 18
+#define EP93XX_SYSCON_PWRCNT_DMA_M2P0 17
+#define EP93XX_SYSCON_PWRCNT_DMA_M2P1 16
+#define EP93XX_SYSCON_CLKSET1 0x20
+#define EP93XX_SYSCON_CLKSET1_NBYP1 BIT(23)
+#define EP93XX_SYSCON_CLKSET2 0x24
+#define EP93XX_SYSCON_CLKSET2_NBYP2 BIT(19)
+#define EP93XX_SYSCON_CLKSET2_PLL2_EN BIT(18)
+#define EP93XX_SYSCON_DEVCFG 0x80
+#define EP93XX_SYSCON_DEVCFG_U3EN 24
+#define EP93XX_SYSCON_DEVCFG_U2EN 20
+#define EP93XX_SYSCON_DEVCFG_U1EN 18
+#define EP93XX_SYSCON_VIDCLKDIV 0x84
+#define EP93XX_SYSCON_CLKDIV_ENABLE 15
+#define EP93XX_SYSCON_CLKDIV_ESEL BIT(14)
+#define EP93XX_SYSCON_CLKDIV_PSEL BIT(13)
+#define EP93XX_SYSCON_CLKDIV_PDIV_SHIFT 8
+#define EP93XX_SYSCON_I2SCLKDIV 0x8c
+#define EP93XX_SYSCON_I2SCLKDIV_SENA 31
+#define EP93XX_SYSCON_I2SCLKDIV_ORIDE BIT(29)
+#define EP93XX_SYSCON_I2SCLKDIV_SPOL BIT(19)
+#define EP93XX_I2SCLKDIV_SDIV (1 << 16)
+#define EP93XX_I2SCLKDIV_LRDIV32 (0 << 17)
+#define EP93XX_I2SCLKDIV_LRDIV64 (1 << 17)
+#define EP93XX_I2SCLKDIV_LRDIV128 (2 << 17)
+#define EP93XX_I2SCLKDIV_LRDIV_MASK (3 << 17)
+#define EP93XX_SYSCON_KEYTCHCLKDIV 0x90
+#define EP93XX_SYSCON_KEYTCHCLKDIV_TSEN 31
+#define EP93XX_SYSCON_KEYTCHCLKDIV_ADIV 16
+#define EP93XX_SYSCON_KEYTCHCLKDIV_KEN 15
+#define EP93XX_SYSCON_KEYTCHCLKDIV_KDIV 0
+#define EP93XX_SYSCON_CHIPID 0x94
+#define EP93XX_SYSCON_CHIPID_ID 0x9213
+
+static DEFINE_SPINLOCK(clk_lock);
+static struct regmap *ep93xx_map;
+#define ep93xx_regmap_read(reg, val) regmap_read(ep93xx_map, reg, val)
+
+/* Keeps track of all clocks */
+static struct clk_hw_onecell_data *ep93xx_clk_data;
+
+static char fclk_divisors[] = { 1, 2, 4, 8, 16, 1, 1, 1 };
+static char hclk_divisors[] = { 1, 2, 4, 5, 6, 8, 16, 32 };
+static char pclk_divisors[] = { 1, 2, 4, 8 };
+
+static char adc_divisors[] = { 16, 4 };
+static char sclk_divisors[] = { 2, 4 };
+static char lrclk_divisors[] = { 32, 64, 128 };
+
+static const char * const mux_parents[] = {
+ "xtali",
+ "pll1",
+ "pll2"
+};
+
+/*
+ * PLL rate = 14.7456 MHz * (X1FBD + 1) * (X2FBD + 1) / (X2IPD + 1) / 2^PS
+ */
+static unsigned long calc_pll_rate(unsigned long long rate, u32 config_word)
+{
+ int i;
+
+ rate *= ((config_word >> 11) & 0x1f) + 1; /* X1FBD */
+ rate *= ((config_word >> 5) & 0x3f) + 1; /* X2FBD */
+ do_div(rate, (config_word & 0x1f) + 1); /* X2IPD */
+ for (i = 0; i < ((config_word >> 16) & 3); i++) /* PS */
+ rate >>= 1;
+
+ return (unsigned long)rate;
+}
+
+struct clk_psc {
+ struct clk_hw hw;
+ unsigned int reg;
+ u8 bit_idx;
+ u32 mask;
+ u8 shift;
+ u8 width;
+ char *div;
+ u8 num_div;
+ spinlock_t *lock;
+};
+
+#define to_clk_psc(_hw) container_of(_hw, struct clk_psc, hw)
+
+static int ep93xx_clk_is_enabled(struct clk_hw *hw)
+{
+ struct clk_psc *psc = to_clk_psc(hw);
+ u32 val;
+
+ ep93xx_regmap_read(psc->reg, &val);
+
+ return (val & BIT(psc->bit_idx)) ? 1 : 0;
+}
+
+static int ep93xx_clk_enable(struct clk_hw *hw)
+{
+ struct clk_psc *psc = to_clk_psc(hw);
+ unsigned long flags = 0;
+ u32 val;
+
+ if (psc->lock)
+ spin_lock_irqsave(psc->lock, flags);
+
+ ep93xx_regmap_read(psc->reg, &val);
+ val |= BIT(psc->bit_idx);
+
+ ep93xx_syscon_swlocked_write(val, psc->reg);
+
+ if (psc->lock)
+ spin_unlock_irqrestore(psc->lock, flags);
+
+ return 0;
+}
+
+static void ep93xx_clk_disable(struct clk_hw *hw)
+{
+ struct clk_psc *psc = to_clk_psc(hw);
+ unsigned long flags = 0;
+ u32 val;
+
+ if (psc->lock)
+ spin_lock_irqsave(psc->lock, flags);
+
+ ep93xx_regmap_read(psc->reg, &val);
+ val &= ~BIT(psc->bit_idx);
+
+ ep93xx_syscon_swlocked_write(val, psc->reg);
+
+ if (psc->lock)
+ spin_unlock_irqrestore(psc->lock, flags);
+}
+
+static const struct clk_ops clk_ep93xx_gate_ops = {
+ .enable = ep93xx_clk_enable,
+ .disable = ep93xx_clk_disable,
+ .is_enabled = ep93xx_clk_is_enabled,
+};
+
+static struct clk_hw *ep93xx_clk_register_gate(const char *name,
+ const char *parent_name,
+ unsigned int reg,
+ u8 bit_idx)
+{
+ struct clk_init_data init;
+ struct clk_psc *psc;
+ struct clk *clk;
+
+ psc = kzalloc(sizeof(*psc), GFP_KERNEL);
+ if (!psc)
+ return ERR_PTR(-ENOMEM);
+
+ init.name = name;
+ init.ops = &clk_ep93xx_gate_ops;
+ init.flags = CLK_SET_RATE_PARENT;
+ init.parent_names = (parent_name ? &parent_name : NULL);
+ init.num_parents = (parent_name ? 1 : 0);
+
+ psc->reg = reg;
+ psc->bit_idx = bit_idx;
+ psc->hw.init = &init;
+ psc->lock = &clk_lock;
+
+ clk = clk_register(NULL, &psc->hw);
+ if (IS_ERR(clk)) {
+ kfree(psc);
+ return ERR_CAST(clk);
+ }
+
+ return &psc->hw;
+}
+
+static u8 ep93xx_mux_get_parent(struct clk_hw *hw)
+{
+ struct clk_psc *psc = to_clk_psc(hw);
+ u32 val;
+
+ ep93xx_regmap_read(psc->reg, &val);
+ if (!(val & EP93XX_SYSCON_CLKDIV_ESEL))
+ return 0;
+
+ if (!(val & EP93XX_SYSCON_CLKDIV_PSEL))
+ return 1;
+
+ return 2;
+}
+
+static int ep93xx_mux_set_parent_lock(struct clk_hw *hw, u8 index)
+{
+ struct clk_psc *psc = to_clk_psc(hw);
+ unsigned long flags = 0;
+ u32 val;
+
+ if (index >= ARRAY_SIZE(mux_parents))
+ return -EINVAL;
+
+ if (psc->lock)
+ spin_lock_irqsave(psc->lock, flags);
+
+ ep93xx_regmap_read(psc->reg, &val);
+ val &= ~(EP93XX_SYSCON_CLKDIV_ESEL | EP93XX_SYSCON_CLKDIV_PSEL);
+
+ if (index != 0) {
+ val |= EP93XX_SYSCON_CLKDIV_ESEL;
+ val |= (index - 1) ? EP93XX_SYSCON_CLKDIV_PSEL : 0;
+ }
+
+ ep93xx_syscon_swlocked_write(val, psc->reg);
+
+ if (psc->lock)
+ spin_unlock_irqrestore(psc->lock, flags);
+
+ return 0;
+}
+
+static bool is_best(unsigned long rate, unsigned long now,
+ unsigned long best)
+{
+ return abs(rate - now) < abs(rate - best);
+}
+
+static int ep93xx_mux_determine_rate(struct clk_hw *hw,
+ struct clk_rate_request *req)
+{
+ unsigned long rate = req->rate;
+ struct clk *parent_best = NULL;
+ unsigned long parent_rate;
+ unsigned long best_rate = 0, actual_rate, mclk_rate;
+ unsigned long parent_rate_best;
+ int div, pdiv;
+ int i;
+
+ /*
+ * Try the two pll's and the external clock
+ * Because the valid predividers are 2, 2.5 and 3, we multiply
+ * all the clocks by 2 to avoid floating point math.
+ *
+ * This is based on the algorithm in the ep93xx raster guide:
+ * http://be-a-maverick.com/en/pubs/appNote/AN269REV1.pdf
+ *
+ */
+ for (i = 0; i < ARRAY_SIZE(mux_parents); i++) {
+ struct clk *parent = clk_get_sys(mux_parents[i], NULL);
+
+ parent_rate = clk_get_rate(parent);
+ mclk_rate = parent_rate * 2;
+
+ /* Try each predivider value */
+ for (pdiv = 4; pdiv <= 6; pdiv++) {
+ div = DIV_ROUND_CLOSEST(mclk_rate, rate * pdiv);
+ if (div < 1 || div > 127)
+ continue;
+
+ actual_rate = DIV_ROUND_CLOSEST(mclk_rate, pdiv * div);
+
+ if (is_best(rate, actual_rate, best_rate)) {
+ best_rate = actual_rate;
+ parent_rate_best = parent_rate;
+ parent_best = parent;
+ }
+ }
+ }
+
+ if (!parent_best)
+ return -EINVAL;
+
+ req->best_parent_rate = parent_rate_best;
+ req->best_parent_hw = __clk_get_hw(parent_best);
+ req->rate = best_rate;
+
+ return 0;
+}
+
+static unsigned long ep93xx_ddiv_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct clk_psc *psc = to_clk_psc(hw);
+ unsigned long rate = 0;
+ u32 val;
+ int pdiv, div;
+
+ ep93xx_regmap_read(psc->reg, &val);
+ pdiv = ((val >> EP93XX_SYSCON_CLKDIV_PDIV_SHIFT) & 0x03);
+ div = val & 0x7f;
+ if (div > 0)
+ rate = DIV_ROUND_CLOSEST(parent_rate * 2, (pdiv + 3) * div);
+
+ return rate;
+}
+
+static int ep93xx_ddiv_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct clk_psc *psc = to_clk_psc(hw);
+ int pdiv, div, npdiv, ndiv;
+ unsigned long actual_rate, mclk_rate, rate_err = -1;
+ int found = 0;
+ u32 val;
+
+ ep93xx_regmap_read(psc->reg, &val);
+ mclk_rate = parent_rate * 2;
+
+ for (pdiv = 4; pdiv <= 6; pdiv++) {
+ div = DIV_ROUND_CLOSEST(mclk_rate, rate * pdiv);
+ if (div < 1 || div > 127)
+ continue;
+
+ actual_rate = DIV_ROUND_CLOSEST(mclk_rate, pdiv * div);
+
+ if (!found || abs(actual_rate - rate) < rate_err) {
+ npdiv = pdiv - 3;
+ ndiv = div;
+ rate_err = abs(actual_rate - rate);
+ found = 1;
+ }
+ }
+
+ if (!found)
+ return -EINVAL;
+
+ /* Clear old dividers */
+ val &= ~0x37f;
+
+ /* Set the new pdiv and div bits for the new clock rate */
+ val |= (npdiv << EP93XX_SYSCON_CLKDIV_PDIV_SHIFT) | ndiv;
+
+ ep93xx_syscon_swlocked_write(val, psc->reg);
+
+ return 0;
+}
+
+static const struct clk_ops clk_ddiv_ops = {
+ .enable = ep93xx_clk_enable,
+ .disable = ep93xx_clk_disable,
+ .is_enabled = ep93xx_clk_is_enabled,
+ .get_parent = ep93xx_mux_get_parent,
+ .set_parent = ep93xx_mux_set_parent_lock,
+ .determine_rate = ep93xx_mux_determine_rate,
+ .recalc_rate = ep93xx_ddiv_recalc_rate,
+ .set_rate = ep93xx_ddiv_set_rate,
+};
+
+static struct clk_hw *clk_hw_register_ddiv(const char *name,
+ unsigned int reg,
+ u8 bit_idx)
+{
+ struct clk_init_data init;
+ struct clk_psc *psc;
+ struct clk *clk;
+
+ psc = kzalloc(sizeof(*psc), GFP_KERNEL);
+ if (!psc)
+ return ERR_PTR(-ENOMEM);
+
+ init.name = name;
+ init.ops = &clk_ddiv_ops;
+ init.flags = 0;
+ init.parent_names = mux_parents;
+ init.num_parents = ARRAY_SIZE(mux_parents);
+
+ psc->reg = reg;
+ psc->bit_idx = bit_idx;
+ psc->lock = &clk_lock;
+ psc->hw.init = &init;
+
+ clk = clk_register(NULL, &psc->hw);
+ if (IS_ERR(clk)) {
+ kfree(psc);
+ return ERR_CAST(clk);
+ }
+
+ return &psc->hw;
+}
+
+static unsigned long ep93xx_div_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct clk_psc *psc = to_clk_psc(hw);
+ u32 val;
+ u8 index;
+
+ ep93xx_regmap_read(psc->reg, &val);
+ index = (val & psc->mask) >> psc->shift;
+ if (index > psc->num_div)
+ return 0;
+
+ return DIV_ROUND_CLOSEST(parent_rate, psc->div[index]);
+}
+
+static long ep93xx_div_round_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *parent_rate)
+{
+ struct clk_psc *psc = to_clk_psc(hw);
+ unsigned long best = 0, now;
+ bool assigned = false;
+ int i;
+
+ for (i = 0; i < psc->num_div; i++) {
+ if ((rate * psc->div[i]) == *parent_rate)
+ return rate;
+
+ now = DIV_ROUND_CLOSEST(*parent_rate, psc->div[i]);
+
+ if (!assigned || is_best(rate, now, best))
+ best = now;
+ assigned = true;
+ }
+
+ return best;
+}
+
+static int ep93xx_div_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct clk_psc *psc = to_clk_psc(hw);
+ u32 val;
+ int i;
+
+ ep93xx_regmap_read(psc->reg, &val);
+ val &= ~psc->mask;
+ for (i = 0; i < psc->num_div; i++)
+ if (rate == DIV_ROUND_CLOSEST(parent_rate, psc->div[i])) {
+ val |= i << psc->shift;
+ break;
+ }
+
+ if (i == psc->num_div)
+ return -EINVAL;
+
+ ep93xx_syscon_swlocked_write(val, psc->reg);
+
+ return 0;
+}
+
+static const struct clk_ops ep93xx_div_ops = {
+ .enable = ep93xx_clk_enable,
+ .disable = ep93xx_clk_disable,
+ .is_enabled = ep93xx_clk_is_enabled,
+ .recalc_rate = ep93xx_div_recalc_rate,
+ .round_rate = ep93xx_div_round_rate,
+ .set_rate = ep93xx_div_set_rate,
+};
+
+static struct clk_hw *clk_hw_register_div(const char *name,
+ const char *parent_name,
+ unsigned int reg,
+ u8 enable_bit,
+ u8 shift,
+ u8 width,
+ char *clk_divisors,
+ u8 num_div)
+{
+ struct clk_init_data init;
+ struct clk_psc *psc;
+ struct clk *clk;
+
+ psc = kzalloc(sizeof(*psc), GFP_KERNEL);
+ if (!psc)
+ return ERR_PTR(-ENOMEM);
+
+ init.name = name;
+ init.ops = &ep93xx_div_ops;
+ init.flags = 0;
+ init.parent_names = (parent_name ? &parent_name : NULL);
+ init.num_parents = 1;
+
+ psc->reg = reg;
+ psc->bit_idx = enable_bit;
+ psc->mask = GENMASK(shift + width - 1, shift);
+ psc->shift = shift;
+ psc->div = clk_divisors;
+ psc->num_div = num_div;
+ psc->lock = &clk_lock;
+ psc->hw.init = &init;
+
+ clk = clk_register(NULL, &psc->hw);
+ if (IS_ERR(clk)) {
+ kfree(psc);
+ return ERR_CAST(clk);
+ }
+
+ return &psc->hw;
+}
+
+struct ep93xx_gate {
+ unsigned int idx;
+ unsigned int bit;
+ const char *dev_id;
+ const char *con_id;
+};
+
+static struct ep93xx_gate ep93xx_uarts[] = {
+ {EP93XX_CLK_UART1, EP93XX_SYSCON_DEVCFG_U1EN, "apb:uart1", NULL},
+ {EP93XX_CLK_UART2, EP93XX_SYSCON_DEVCFG_U2EN, "apb:uart2", NULL},
+ {EP93XX_CLK_UART3, EP93XX_SYSCON_DEVCFG_U3EN, "apb:uart3", NULL},
+};
+
+static void ep93xx_uart_clock_init(void)
+{
+ unsigned int i;
+ struct clk_hw *hw;
+ u32 val;
+ unsigned int clk_uart_div;
+
+ ep93xx_regmap_read(EP93XX_SYSCON_PWRCNT, &val);
+ if (val & EP93XX_SYSCON_PWRCNT_UARTBAUD)
+ clk_uart_div = 1;
+ else
+ clk_uart_div = 2;
+
+ hw = clk_hw_register_fixed_factor(NULL, "uart", "xtali", 0, 1, clk_uart_div);
+ ep93xx_clk_data->hws[EP93XX_CLK_UART] = hw;
+
+ /* parenting uart gate clocks to uart clock */
+ for (i = 0; i < ARRAY_SIZE(ep93xx_uarts); i++) {
+ hw = ep93xx_clk_register_gate(ep93xx_uarts[i].dev_id,
+ "uart",
+ EP93XX_SYSCON_DEVCFG,
+ ep93xx_uarts[i].bit);
+
+ ep93xx_clk_data->hws[ep93xx_uarts[i].idx] = hw;
+ }
+}
+
+static struct ep93xx_gate ep93xx_dmas[] = {
+ {EP93XX_CLK_M2P0, EP93XX_SYSCON_PWRCNT_DMA_M2P0, NULL, "m2p0"},
+ {EP93XX_CLK_M2P1, EP93XX_SYSCON_PWRCNT_DMA_M2P1, NULL, "m2p1"},
+ {EP93XX_CLK_M2P2, EP93XX_SYSCON_PWRCNT_DMA_M2P2, NULL, "m2p2"},
+ {EP93XX_CLK_M2P3, EP93XX_SYSCON_PWRCNT_DMA_M2P3, NULL, "m2p3"},
+ {EP93XX_CLK_M2P4, EP93XX_SYSCON_PWRCNT_DMA_M2P4, NULL, "m2p4"},
+ {EP93XX_CLK_M2P5, EP93XX_SYSCON_PWRCNT_DMA_M2P5, NULL, "m2p5"},
+ {EP93XX_CLK_M2P6, EP93XX_SYSCON_PWRCNT_DMA_M2P6, NULL, "m2p6"},
+ {EP93XX_CLK_M2P7, EP93XX_SYSCON_PWRCNT_DMA_M2P7, NULL, "m2p7"},
+ {EP93XX_CLK_M2P8, EP93XX_SYSCON_PWRCNT_DMA_M2P8, NULL, "m2p8"},
+ {EP93XX_CLK_M2P9, EP93XX_SYSCON_PWRCNT_DMA_M2P9, NULL, "m2p9"},
+ {EP93XX_CLK_M2M0, EP93XX_SYSCON_PWRCNT_DMA_M2M0, NULL, "m2m0"},
+ {EP93XX_CLK_M2M1, EP93XX_SYSCON_PWRCNT_DMA_M2M1, NULL, "m2m1"},
+};
+
+static void ep93xx_dma_clock_init(void __iomem *base)
+{
+ int i;
+ struct clk_hw *hw;
+ int ret;
+
+ for (i = 0; i < ARRAY_SIZE(ep93xx_dmas); i++) {
+ hw = clk_hw_register_gate(NULL, ep93xx_dmas[i].con_id,
+ "hclk", 0,
+ base + EP93XX_SYSCON_PWRCNT,
+ ep93xx_dmas[i].bit,
+ 0,
+ &clk_lock);
+
+ ret = clk_hw_register_clkdev(hw, ep93xx_dmas[i].con_id, NULL);
+ if (ret) {
+ pr_err("%s: failed to register lookup %s\n",
+ __func__, ep93xx_dmas[i].con_id);
+ continue;
+ }
+
+ ep93xx_clk_data->hws[ep93xx_dmas[i].idx] = hw;
+ }
+}
+
+static int ep93xx_clk_probe(struct platform_device *pdev)
+{
+ void __iomem *base;
+ unsigned int clk_usb_div;
+ unsigned long clk_spi_div;
+ struct clk_hw *hw;
+ struct device *dev = &pdev->dev;
+ u32 value;
+ struct resource *res;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
+ ep93xx_regmap_read(EP93XX_SYSCON_CLKSET2, &value);
+ clk_usb_div = (((value >> 28) & 0xf) + 1);
+ hw = clk_hw_register_fixed_factor(NULL, "usb_clk", "pll2", 0, 1, clk_usb_div);
+ hw = clk_hw_register_gate(NULL, "ohci-platform",
+ "usb_clk", 0,
+ base + EP93XX_SYSCON_PWRCNT,
+ EP93XX_SYSCON_PWRCNT_USH_EN,
+ 0,
+ &clk_lock);
+ clk_hw_register_clkdev(hw, NULL, "ohci-platform");
+ ep93xx_clk_data->hws[EP93XX_CLK_USB] = hw;
+
+ /*
+ * EP93xx SSP clock rate was doubled in version E2. For more information
+ * see:
+ * http://www.cirrus.com/en/pubs/appNote/AN273REV4.pdf
+ */
+ clk_spi_div = 1;
+ if (ep93xx_chip_revision() < EP93XX_CHIP_REV_E2)
+ clk_spi_div = 2;
+ hw = clk_hw_register_fixed_factor(NULL, "ep93xx-spi.0", "xtali", 0, 1, clk_spi_div);
+ clk_hw_register_clkdev(hw, NULL, "ep93xx-spi.0");
+ ep93xx_clk_data->hws[EP93XX_CLK_SPI] = hw;
+
+ /* pwm clock */
+ hw = clk_hw_register_fixed_factor(NULL, "pwm_clk", "xtali", 0, 1, 1);
+ clk_hw_register_clkdev(hw, "pwm_clk", NULL);
+ ep93xx_clk_data->hws[EP93XX_CLK_PWM] = hw;
+
+ ep93xx_uart_clock_init();
+
+ /* touchscreen/adc clock */
+ hw = clk_hw_register_div("ep93xx-adc",
+ "xtali",
+ EP93XX_SYSCON_KEYTCHCLKDIV,
+ EP93XX_SYSCON_KEYTCHCLKDIV_TSEN,
+ EP93XX_SYSCON_KEYTCHCLKDIV_ADIV,
+ 1,
+ adc_divisors,
+ ARRAY_SIZE(adc_divisors));
+
+ clk_hw_register_clkdev(hw, NULL, "ep93xx-adc");
+ ep93xx_clk_data->hws[EP93XX_CLK_ADC] = hw;
+
+ /* keypad clock */
+ hw = clk_hw_register_div("ep93xx-keypad",
+ "xtali",
+ EP93XX_SYSCON_KEYTCHCLKDIV,
+ EP93XX_SYSCON_KEYTCHCLKDIV_KEN,
+ EP93XX_SYSCON_KEYTCHCLKDIV_KDIV,
+ 1,
+ adc_divisors,
+ ARRAY_SIZE(adc_divisors));
+
+ clk_hw_register_clkdev(hw, NULL, "ep93xx-keypad");
+ ep93xx_clk_data->hws[EP93XX_CLK_KEYPAD] = hw;
+
+ /* On reset PDIV and VDIV is set to zero, while PDIV zero
+ * means clock disable, VDIV shouldn't be zero.
+ * So i set both dividers to minimum.
+ */
+ /* ENA - Enable CLK divider. */
+ /* PDIV - 00 - Disable clock */
+ /* VDIV - at least 2 */
+ /* Check and enable video clk registers */
+ ep93xx_regmap_read(EP93XX_SYSCON_VIDCLKDIV, &value);
+ value |= (1 << EP93XX_SYSCON_CLKDIV_PDIV_SHIFT) | 2;
+ ep93xx_syscon_swlocked_write(value, EP93XX_SYSCON_VIDCLKDIV);
+
+ /* check and enable i2s clk registers */
+ ep93xx_regmap_read(EP93XX_SYSCON_I2SCLKDIV, &value);
+ value |= (1 << EP93XX_SYSCON_CLKDIV_PDIV_SHIFT) | 2;
+ /*
+ * Override the SAI_MSTR_CLK_CFG from the I2S block and use the
+ * I2SClkDiv Register settings. LRCLK transitions on the falling SCLK
+ * edge.
+ */
+ value |= EP93XX_SYSCON_I2SCLKDIV_ORIDE | EP93XX_SYSCON_I2SCLKDIV_SPOL;
+ ep93xx_syscon_swlocked_write(value, EP93XX_SYSCON_I2SCLKDIV);
+
+ /* video clk */
+ hw = clk_hw_register_ddiv("ep93xx-fb",
+ EP93XX_SYSCON_VIDCLKDIV,
+ EP93XX_SYSCON_CLKDIV_ENABLE);
+
+ clk_hw_register_clkdev(hw, NULL, "ep93xx-fb");
+ ep93xx_clk_data->hws[EP93XX_CLK_VIDEO] = hw;
+
+ /* i2s clk */
+ hw = clk_hw_register_ddiv("mclk",
+ EP93XX_SYSCON_I2SCLKDIV,
+ EP93XX_SYSCON_CLKDIV_ENABLE);
+
+ clk_hw_register_clkdev(hw, "mclk", "ep93xx-i2s");
+ ep93xx_clk_data->hws[EP93XX_CLK_I2S_MCLK] = hw;
+
+ /* i2s sclk */
+#define EP93XX_I2SCLKDIV_SDIV_SHIFT 16
+#define EP93XX_I2SCLKDIV_SDIV_WIDTH 1
+ hw = clk_hw_register_div("sclk",
+ "mclk",
+ EP93XX_SYSCON_I2SCLKDIV,
+ EP93XX_SYSCON_I2SCLKDIV_SENA,
+ EP93XX_I2SCLKDIV_SDIV_SHIFT,
+ EP93XX_I2SCLKDIV_SDIV_WIDTH,
+ sclk_divisors,
+ ARRAY_SIZE(sclk_divisors));
+
+ clk_hw_register_clkdev(hw, "sclk", "ep93xx-i2s");
+ ep93xx_clk_data->hws[EP93XX_CLK_I2S_SCLK] = hw;
+
+ /* i2s lrclk */
+#define EP93XX_I2SCLKDIV_LRDIV32_SHIFT 17
+#define EP93XX_I2SCLKDIV_LRDIV32_WIDTH 2
+ hw = clk_hw_register_div("lrclk",
+ "sclk",
+ EP93XX_SYSCON_I2SCLKDIV,
+ EP93XX_SYSCON_I2SCLKDIV_SENA,
+ EP93XX_I2SCLKDIV_LRDIV32_SHIFT,
+ EP93XX_I2SCLKDIV_LRDIV32_WIDTH,
+ lrclk_divisors,
+ ARRAY_SIZE(lrclk_divisors));
+
+ clk_hw_register_clkdev(hw, "lrclk", "ep93xx-i2s");
+ ep93xx_clk_data->hws[EP93XX_CLK_I2S_LRCLK] = hw;
+
+ /* IrDa clk uses same pattern but no init code presents in original clock driver */
+ return 0;
+}
+
+static const struct of_device_id ep93xx_clk_dt_ids[] = {
+ { .compatible = "cirrus,ep9301-syscon", },
+ { /* sentinel */ },
+};
+
+static struct platform_driver ep93xx_clk_driver = {
+ .probe = ep93xx_clk_probe,
+ .driver = {
+ .name = "ep93xx-clk",
+ .of_match_table = ep93xx_clk_dt_ids,
+ .suppress_bind_attrs = true,
+ },
+};
+builtin_platform_driver(ep93xx_clk_driver);
+
+static void __init ep93xx_clock_init(struct device_node *np)
+{
+ void __iomem *base;
+ int i;
+ int ret;
+ u32 value;
+ struct clk_hw *hw;
+ unsigned long clk_pll1_rate;
+ unsigned long clk_f_rate;
+ unsigned long clk_h_rate;
+ unsigned long clk_p_rate;
+ unsigned long clk_pll2_rate;
+ unsigned int clk_f_div;
+ unsigned int clk_h_div;
+ unsigned int clk_p_div;
+
+ ep93xx_clk_data = kzalloc(struct_size(ep93xx_clk_data, hws,
+ EP93XX_NUM_CLKS),
+ GFP_KERNEL);
+
+ if (!ep93xx_clk_data)
+ return;
+
+ /*
+ * This way all clock fetched before the platform device probes,
+ * except those we assign here for early use, will be deferred.
+ */
+ for (i = 0; i < EP93XX_NUM_CLKS; i++)
+ ep93xx_clk_data->hws[i] = ERR_PTR(-EPROBE_DEFER);
+
+ base = of_iomap(np, 0);
+ /* Remap the system controller for the exclusive register */
+ if (IS_ERR(base)) {
+ pr_err("failed to map base\n");
+ return;
+ }
+
+ ep93xx_map = syscon_node_to_regmap(np);
+ if (IS_ERR(ep93xx_map)) {
+ pr_err("no syscon regmap\n");
+ return;
+ }
+
+ /*
+ * We check that the regmap works on this very first access,
+ * but as this is an MMIO-backed regmap, subsequent regmap
+ * access is not going to fail and we skip error checks from
+ * this point.
+ */
+ ret = ep93xx_regmap_read(EP93XX_SYSCON_CHIPID, &value);
+ if (ret || (value & 0xffff) != EP93XX_SYSCON_CHIPID_ID) {
+ pr_err("failed to read global status register\n");
+ return;
+ }
+
+ hw = clk_hw_register_fixed_rate(NULL, "xtali", NULL, 0, EP93XX_EXT_CLK_RATE);
+ clk_hw_register_clkdev(hw, NULL, "xtali");
+
+ /* Determine the bootloader configured pll1 rate */
+ ep93xx_regmap_read(EP93XX_SYSCON_CLKSET1, &value);
+ if (!(value & EP93XX_SYSCON_CLKSET1_NBYP1))
+ clk_pll1_rate = EP93XX_EXT_CLK_RATE;
+ else
+ clk_pll1_rate = calc_pll_rate(EP93XX_EXT_CLK_RATE, value);
+
+ hw = clk_hw_register_fixed_rate(NULL, "pll1", "xtali", 0, clk_pll1_rate);
+ clk_hw_register_clkdev(hw, NULL, "pll1");
+ ep93xx_clk_data->hws[EP93XX_CLK_PLL1] = hw;
+
+ /* Initialize the pll1 derived clocks */
+ clk_f_div = fclk_divisors[(value >> 25) & 0x7];
+ clk_h_div = hclk_divisors[(value >> 20) & 0x7];
+ clk_p_div = pclk_divisors[(value >> 18) & 0x3];
+
+ hw = clk_hw_register_fixed_factor(NULL, "fclk", "pll1", 0, 1, clk_f_div);
+ clk_f_rate = clk_get_rate(hw->clk);
+ ep93xx_clk_data->hws[EP93XX_CLK_FCLK] = hw;
+ hw = clk_hw_register_fixed_factor(NULL, "hclk", "pll1", 0, 1, clk_h_div);
+ clk_h_rate = clk_get_rate(hw->clk);
+ ep93xx_clk_data->hws[EP93XX_CLK_HCLK] = hw;
+ hw = clk_hw_register_fixed_factor(NULL, "pclk", "hclk", 0, 1, clk_p_div);
+ clk_p_rate = clk_get_rate(hw->clk);
+ ep93xx_clk_data->hws[EP93XX_CLK_PCLK] = hw;
+
+ clk_hw_register_clkdev(hw, "apb_pclk", NULL);
+
+ ep93xx_dma_clock_init(base);
+
+ /* Determine the bootloader configured pll2 rate */
+ ep93xx_regmap_read(EP93XX_SYSCON_CLKSET2, &value);
+ if (!(value & EP93XX_SYSCON_CLKSET2_NBYP2))
+ clk_pll2_rate = EP93XX_EXT_CLK_RATE;
+ else if (value & EP93XX_SYSCON_CLKSET2_PLL2_EN)
+ clk_pll2_rate = calc_pll_rate(EP93XX_EXT_CLK_RATE, value);
+ else
+ clk_pll2_rate = 0;
+
+ hw = clk_hw_register_fixed_rate(NULL, "pll2", "xtali", 0, clk_pll2_rate);
+ ep93xx_clk_data->hws[EP93XX_CLK_PLL2] = hw;
+ clk_hw_register_clkdev(hw, NULL, "pll2");
+
+ ep93xx_clk_data->num = EP93XX_NUM_CLKS;
+ of_clk_add_hw_provider(np, of_clk_hw_onecell_get, ep93xx_clk_data);
+
+ pr_info("PLL1 running at %ld MHz, PLL2 at %ld MHz\n",
+ clk_pll1_rate / 1000000, clk_pll2_rate / 1000000);
+ pr_info("FCLK %ld MHz, HCLK %ld MHz, PCLK %ld MHz\n",
+ clk_f_rate / 1000000, clk_h_rate / 1000000,
+ clk_p_rate / 1000000);
+}
+CLK_OF_DECLARE_DRIVER(ep93xx_cc, "cirrus,ep9301-syscon", ep93xx_clock_init);
--
2.39.2

2023-04-24 12:09:00

by Christophe JAILLET

Subject: Re: [PATCH 12/43] clk: ep93xx: add DT support for Cirrus EP93xx

Le 24/04/2023 à 14:34, Nikita Shubin a écrit :
> This is a rewrite of EP93xx timer driver in
> arch/arm/mach-ep93xx/clock.c trying to do everything
> the device tree way:
>
> - convert to syscon driver
> - provide clock acces via of
>
> Co-developed-by: Alexander Sverdlin <[email protected]>
> Signed-off-by: Alexander Sverdlin <[email protected]>
> Signed-off-by: Nikita Shubin <[email protected]>
> ---
>
> Notes:
> rfc->v0
> Linus Walleij:
> - renamed all __underscore
>
> Alexander Sverdlin:
> - "Logick" -> "Logic"
>
> Changes by Alexander Sverdlin:
> - remove pr_info
> - DIV_ROUND_UP_ULL -> DIV_ROUND_CLOSEST
> - fix zeroing bitfield in ep93xx_div_set_rate
> - add sanity check for EP93XX_SYSCON_CHIPID_ID
> - use bit index for DMA clock's
> - ep93xx_clk_register_gate() takes bit index, not mask
> - remove redundant define
> - use DIV_ROUND_CLOSEST() everywhere to achieve frequencies closer to those
> requested
> - Add the forgotten configuration from the deleted
> arch/arm/mach-ep93xx/core.c
>
> drivers/clk/Kconfig | 8 +
> drivers/clk/Makefile | 1 +
> drivers/clk/clk-ep93xx.c | 880 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 889 insertions(+)
> create mode 100644 drivers/clk/clk-ep93xx.c
>
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 1eef05bb1f99..d3ad975b688e 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -436,6 +436,14 @@ config COMMON_CLK_K210
> help
> Support for the Canaan Kendryte K210 RISC-V SoC clocks.
>
> +config COMMON_CLK_EP93XX
> + bool "Clock driver for Cirrus Logic ep93xx SoC"
> + depends on ARCH_EP93XX || COMPILE_TEST
> + select MFD_SYSCON
> + select REGMAP
> + help
> + This driver supports the SoC clocks on the Cirrus Logic ep93xx.
> +
> source "drivers/clk/actions/Kconfig"
> source "drivers/clk/analogbits/Kconfig"
> source "drivers/clk/baikal-t1/Kconfig"
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index e3ca0d058a25..deec25ffd004 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_COMMON_CLK_CDCE706) += clk-cdce706.o
> obj-$(CONFIG_COMMON_CLK_CDCE925) += clk-cdce925.o
> obj-$(CONFIG_ARCH_CLPS711X) += clk-clps711x.o
> obj-$(CONFIG_COMMON_CLK_CS2000_CP) += clk-cs2000-cp.o
> +obj-$(CONFIG_COMMON_CLK_EP93XX) += clk-ep93xx.o
> obj-$(CONFIG_ARCH_SPARX5) += clk-sparx5.o
> obj-$(CONFIG_COMMON_CLK_EN7523) += clk-en7523.o
> obj-$(CONFIG_COMMON_CLK_FIXED_MMIO) += clk-fixed-mmio.o
> diff --git a/drivers/clk/clk-ep93xx.c b/drivers/clk/clk-ep93xx.c
> new file mode 100644
> index 000000000000..e5913fc4c39b
> --- /dev/null
> +++ b/drivers/clk/clk-ep93xx.c
> @@ -0,0 +1,880 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Clock control for Cirrus EP93xx chips.
> + * Copyright (C) 2021 Nikita Shubin <[email protected]>
> + *
> + * Based on a rewrite of arch/arm/mach-ep93xx/clock.c:
> + * Copyright (C) 2006 Lennert Buytenhek <[email protected]>
> + */
> +#define pr_fmt(fmt) "ep93xx " KBUILD_MODNAME ": " fmt
> +
> +#include <linux/kernel.h>
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/string.h>
> +#include <linux/io.h>
> +#include <linux/spinlock.h>
> +#include <linux/clkdev.h>
> +#include <linux/clk-provider.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/soc/cirrus/ep93xx.h>
> +#include <dt-bindings/clock/cirrus,ep93xx-clock.h>
> +
> +#include <asm/div64.h>
> +
> +#define EP93XX_EXT_CLK_RATE 14745600
> +#define EP93XX_EXT_RTC_RATE 32768
> +
> +#define EP93XX_SYSCON_POWER_STATE 0x00
> +#define EP93XX_SYSCON_PWRCNT 0x04
> +#define EP93XX_SYSCON_PWRCNT_UARTBAUD BIT(29)
> +#define EP93XX_SYSCON_PWRCNT_USH_EN 28
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2M1 27
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2M0 26
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P8 25
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P9 24
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P6 23
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P7 22
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P4 21
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P5 20
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P2 19
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P3 18
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P0 17
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P1 16
> +#define EP93XX_SYSCON_CLKSET1 0x20
> +#define EP93XX_SYSCON_CLKSET1_NBYP1 BIT(23)
> +#define EP93XX_SYSCON_CLKSET2 0x24
> +#define EP93XX_SYSCON_CLKSET2_NBYP2 BIT(19)
> +#define EP93XX_SYSCON_CLKSET2_PLL2_EN BIT(18)
> +#define EP93XX_SYSCON_DEVCFG 0x80
> +#define EP93XX_SYSCON_DEVCFG_U3EN 24
> +#define EP93XX_SYSCON_DEVCFG_U2EN 20
> +#define EP93XX_SYSCON_DEVCFG_U1EN 18
> +#define EP93XX_SYSCON_VIDCLKDIV 0x84
> +#define EP93XX_SYSCON_CLKDIV_ENABLE 15
> +#define EP93XX_SYSCON_CLKDIV_ESEL BIT(14)
> +#define EP93XX_SYSCON_CLKDIV_PSEL BIT(13)
> +#define EP93XX_SYSCON_CLKDIV_PDIV_SHIFT 8
> +#define EP93XX_SYSCON_I2SCLKDIV 0x8c
> +#define EP93XX_SYSCON_I2SCLKDIV_SENA 31
> +#define EP93XX_SYSCON_I2SCLKDIV_ORIDE BIT(29)
> +#define EP93XX_SYSCON_I2SCLKDIV_SPOL BIT(19)
> +#define EP93XX_I2SCLKDIV_SDIV (1 << 16)
> +#define EP93XX_I2SCLKDIV_LRDIV32 (0 << 17)
> +#define EP93XX_I2SCLKDIV_LRDIV64 (1 << 17)
> +#define EP93XX_I2SCLKDIV_LRDIV128 (2 << 17)
> +#define EP93XX_I2SCLKDIV_LRDIV_MASK (3 << 17)
> +#define EP93XX_SYSCON_KEYTCHCLKDIV 0x90
> +#define EP93XX_SYSCON_KEYTCHCLKDIV_TSEN 31
> +#define EP93XX_SYSCON_KEYTCHCLKDIV_ADIV 16
> +#define EP93XX_SYSCON_KEYTCHCLKDIV_KEN 15
> +#define EP93XX_SYSCON_KEYTCHCLKDIV_KDIV 0
> +#define EP93XX_SYSCON_CHIPID 0x94
> +#define EP93XX_SYSCON_CHIPID_ID 0x9213
> +
> +static DEFINE_SPINLOCK(clk_lock);
> +static struct regmap *ep93xx_map;
> +#define ep93xx_regmap_read(reg, val) regmap_read(ep93xx_map, reg, val)
> +
> +/* Keeps track of all clocks */
> +static struct clk_hw_onecell_data *ep93xx_clk_data;
> +
> +static char fclk_divisors[] = { 1, 2, 4, 8, 16, 1, 1, 1 };
> +static char hclk_divisors[] = { 1, 2, 4, 5, 6, 8, 16, 32 };
> +static char pclk_divisors[] = { 1, 2, 4, 8 };
> +
> +static char adc_divisors[] = { 16, 4 };
> +static char sclk_divisors[] = { 2, 4 };
> +static char lrclk_divisors[] = { 32, 64, 128 };
> +
> +static const char * const mux_parents[] = {
> + "xtali",
> + "pll1",
> + "pll2"
> +};
> +
> +/*
> + * PLL rate = 14.7456 MHz * (X1FBD + 1) * (X2FBD + 1) / (X2IPD + 1) / 2^PS
> + */
> +static unsigned long calc_pll_rate(unsigned long long rate, u32 config_word)
> +{
> + int i;
> +
> + rate *= ((config_word >> 11) & 0x1f) + 1; /* X1FBD */
> + rate *= ((config_word >> 5) & 0x3f) + 1; /* X2FBD */
> + do_div(rate, (config_word & 0x1f) + 1); /* X2IPD */
> + for (i = 0; i < ((config_word >> 16) & 3); i++) /* PS */
> + rate >>= 1;
> +
> + return (unsigned long)rate;
> +}
> +
> +struct clk_psc {
> + struct clk_hw hw;
> + unsigned int reg;
> + u8 bit_idx;
> + u32 mask;
> + u8 shift;
> + u8 width;
> + char *div;
> + u8 num_div;
> + spinlock_t *lock;
> +};
> +
> +#define to_clk_psc(_hw) container_of(_hw, struct clk_psc, hw)
> +
> +static int ep93xx_clk_is_enabled(struct clk_hw *hw)
> +{
> + struct clk_psc *psc = to_clk_psc(hw);
> + u32 val;
> +
> + ep93xx_regmap_read(psc->reg, &val);
> +
> + return (val & BIT(psc->bit_idx)) ? 1 : 0;
> +}
> +
> +static int ep93xx_clk_enable(struct clk_hw *hw)
> +{
> + struct clk_psc *psc = to_clk_psc(hw);
> + unsigned long flags = 0;
> + u32 val;
> +
> + if (psc->lock)
> + spin_lock_irqsave(psc->lock, flags);
> +
> + ep93xx_regmap_read(psc->reg, &val);
> + val |= BIT(psc->bit_idx);
> +
> + ep93xx_syscon_swlocked_write(val, psc->reg);
> +
> + if (psc->lock)
> + spin_unlock_irqrestore(psc->lock, flags);
> +
> + return 0;
> +}
> +
> +static void ep93xx_clk_disable(struct clk_hw *hw)
> +{
> + struct clk_psc *psc = to_clk_psc(hw);
> + unsigned long flags = 0;
> + u32 val;
> +
> + if (psc->lock)
> + spin_lock_irqsave(psc->lock, flags);
> +
> + ep93xx_regmap_read(psc->reg, &val);
> + val &= ~BIT(psc->bit_idx);
> +
> + ep93xx_syscon_swlocked_write(val, psc->reg);
> +
> + if (psc->lock)
> + spin_unlock_irqrestore(psc->lock, flags);
> +}
> +
> +static const struct clk_ops clk_ep93xx_gate_ops = {
> + .enable = ep93xx_clk_enable,
> + .disable = ep93xx_clk_disable,
> + .is_enabled = ep93xx_clk_is_enabled,
> +};
> +
> +static struct clk_hw *ep93xx_clk_register_gate(const char *name,
> + const char *parent_name,
> + unsigned int reg,
> + u8 bit_idx)
> +{
> + struct clk_init_data init;
> + struct clk_psc *psc;
> + struct clk *clk;
> +
> + psc = kzalloc(sizeof(*psc), GFP_KERNEL);
> + if (!psc)
> + return ERR_PTR(-ENOMEM);
> +
> + init.name = name;
> + init.ops = &clk_ep93xx_gate_ops;
> + init.flags = CLK_SET_RATE_PARENT;
> + init.parent_names = (parent_name ? &parent_name : NULL);
> + init.num_parents = (parent_name ? 1 : 0);
> +
> + psc->reg = reg;
> + psc->bit_idx = bit_idx;
> + psc->hw.init = &init;
> + psc->lock = &clk_lock;
> +
> + clk = clk_register(NULL, &psc->hw);
> + if (IS_ERR(clk)) {
> + kfree(psc);
> + return ERR_CAST(clk);
> + }
> +
> + return &psc->hw;
> +}
> +
> +static u8 ep93xx_mux_get_parent(struct clk_hw *hw)
> +{
> + struct clk_psc *psc = to_clk_psc(hw);
> + u32 val;
> +
> + ep93xx_regmap_read(psc->reg, &val);
> + if (!(val & EP93XX_SYSCON_CLKDIV_ESEL))
> + return 0;
> +
> + if (!(val & EP93XX_SYSCON_CLKDIV_PSEL))
> + return 1;
> +
> + return 2;
> +}
> +
> +static int ep93xx_mux_set_parent_lock(struct clk_hw *hw, u8 index)
> +{
> + struct clk_psc *psc = to_clk_psc(hw);
> + unsigned long flags = 0;
> + u32 val;
> +
> + if (index >= ARRAY_SIZE(mux_parents))
> + return -EINVAL;
> +
> + if (psc->lock)
> + spin_lock_irqsave(psc->lock, flags);
> +
> + ep93xx_regmap_read(psc->reg, &val);
> + val &= ~(EP93XX_SYSCON_CLKDIV_ESEL | EP93XX_SYSCON_CLKDIV_PSEL);
> +
> + if (index != 0) {
> + val |= EP93XX_SYSCON_CLKDIV_ESEL;
> + val |= (index - 1) ? EP93XX_SYSCON_CLKDIV_PSEL : 0;
> + }
> +
> + ep93xx_syscon_swlocked_write(val, psc->reg);
> +
> + if (psc->lock)
> + spin_unlock_irqrestore(psc->lock, flags);
> +
> + return 0;
> +}
> +
> +static bool is_best(unsigned long rate, unsigned long now,
> + unsigned long best)
> +{
> + return abs(rate - now) < abs(rate - best);
> +}
> +
> +static int ep93xx_mux_determine_rate(struct clk_hw *hw,
> + struct clk_rate_request *req)
> +{
> + unsigned long rate = req->rate;
> + struct clk *parent_best = NULL;
> + unsigned long parent_rate;
> + unsigned long best_rate = 0, actual_rate, mclk_rate;
> + unsigned long parent_rate_best;
> + int div, pdiv;
> + int i;
> +
> + /*
> + * Try the two pll's and the external clock
> + * Because the valid predividers are 2, 2.5 and 3, we multiply
> + * all the clocks by 2 to avoid floating point math.
> + *
> + * This is based on the algorithm in the ep93xx raster guide:
> + * http://be-a-maverick.com/en/pubs/appNote/AN269REV1.pdf
> + *
> + */
> + for (i = 0; i < ARRAY_SIZE(mux_parents); i++) {
> + struct clk *parent = clk_get_sys(mux_parents[i], NULL);
> +
> + parent_rate = clk_get_rate(parent);
> + mclk_rate = parent_rate * 2;
> +
> + /* Try each predivider value */
> + for (pdiv = 4; pdiv <= 6; pdiv++) {
> + div = DIV_ROUND_CLOSEST(mclk_rate, rate * pdiv);
> + if (div < 1 || div > 127)
> + continue;
> +
> + actual_rate = DIV_ROUND_CLOSEST(mclk_rate, pdiv * div);
> +
> + if (is_best(rate, actual_rate, best_rate)) {
> + best_rate = actual_rate;
> + parent_rate_best = parent_rate;
> + parent_best = parent;
> + }
> + }
> + }
> +
> + if (!parent_best)
> + return -EINVAL;
> +
> + req->best_parent_rate = parent_rate_best;
> + req->best_parent_hw = __clk_get_hw(parent_best);
> + req->rate = best_rate;
> +
> + return 0;
> +}
> +
> +static unsigned long ep93xx_ddiv_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct clk_psc *psc = to_clk_psc(hw);
> + unsigned long rate = 0;
> + u32 val;
> + int pdiv, div;
> +
> + ep93xx_regmap_read(psc->reg, &val);
> + pdiv = ((val >> EP93XX_SYSCON_CLKDIV_PDIV_SHIFT) & 0x03);
> + div = val & 0x7f;
> + if (div > 0)
> + rate = DIV_ROUND_CLOSEST(parent_rate * 2, (pdiv + 3) * div);
> +
> + return rate;
> +}
> +
> +static int ep93xx_ddiv_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + struct clk_psc *psc = to_clk_psc(hw);
> + int pdiv, div, npdiv, ndiv;
> + unsigned long actual_rate, mclk_rate, rate_err = -1;
> + int found = 0;
> + u32 val;
> +
> + ep93xx_regmap_read(psc->reg, &val);
> + mclk_rate = parent_rate * 2;
> +
> + for (pdiv = 4; pdiv <= 6; pdiv++) {
> + div = DIV_ROUND_CLOSEST(mclk_rate, rate * pdiv);
> + if (div < 1 || div > 127)
> + continue;
> +
> + actual_rate = DIV_ROUND_CLOSEST(mclk_rate, pdiv * div);
> +
> + if (!found || abs(actual_rate - rate) < rate_err) {
> + npdiv = pdiv - 3;
> + ndiv = div;
> + rate_err = abs(actual_rate - rate);
> + found = 1;
> + }
> + }
> +
> + if (!found)
> + return -EINVAL;
> +
> + /* Clear old dividers */
> + val &= ~0x37f;
> +
> + /* Set the new pdiv and div bits for the new clock rate */
> + val |= (npdiv << EP93XX_SYSCON_CLKDIV_PDIV_SHIFT) | ndiv;
> +
> + ep93xx_syscon_swlocked_write(val, psc->reg);
> +
> + return 0;
> +}
> +
> +static const struct clk_ops clk_ddiv_ops = {
> + .enable = ep93xx_clk_enable,
> + .disable = ep93xx_clk_disable,
> + .is_enabled = ep93xx_clk_is_enabled,
> + .get_parent = ep93xx_mux_get_parent,
> + .set_parent = ep93xx_mux_set_parent_lock,
> + .determine_rate = ep93xx_mux_determine_rate,
> + .recalc_rate = ep93xx_ddiv_recalc_rate,
> + .set_rate = ep93xx_ddiv_set_rate,
> +};
> +
> +static struct clk_hw *clk_hw_register_ddiv(const char *name,
> + unsigned int reg,
> + u8 bit_idx)
> +{
> + struct clk_init_data init;
> + struct clk_psc *psc;
> + struct clk *clk;
> +
> + psc = kzalloc(sizeof(*psc), GFP_KERNEL);
> + if (!psc)
> + return ERR_PTR(-ENOMEM);
> +
> + init.name = name;
> + init.ops = &clk_ddiv_ops;
> + init.flags = 0;
> + init.parent_names = mux_parents;
> + init.num_parents = ARRAY_SIZE(mux_parents);
> +
> + psc->reg = reg;
> + psc->bit_idx = bit_idx;
> + psc->lock = &clk_lock;
> + psc->hw.init = &init;
> +
> + clk = clk_register(NULL, &psc->hw);
> + if (IS_ERR(clk)) {
> + kfree(psc);
> + return ERR_CAST(clk);
> + }
> +
> + return &psc->hw;
> +}
> +
> +static unsigned long ep93xx_div_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct clk_psc *psc = to_clk_psc(hw);
> + u32 val;
> + u8 index;
> +
> + ep93xx_regmap_read(psc->reg, &val);
> + index = (val & psc->mask) >> psc->shift;
> + if (index > psc->num_div)
> + return 0;
> +
> + return DIV_ROUND_CLOSEST(parent_rate, psc->div[index]);
> +}
> +
> +static long ep93xx_div_round_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long *parent_rate)
> +{
> + struct clk_psc *psc = to_clk_psc(hw);
> + unsigned long best = 0, now;
> + bool assigned = false;
> + int i;
> +
> + for (i = 0; i < psc->num_div; i++) {
> + if ((rate * psc->div[i]) == *parent_rate)
> + return rate;
> +
> + now = DIV_ROUND_CLOSEST(*parent_rate, psc->div[i]);
> +
> + if (!assigned || is_best(rate, now, best))
> + best = now;
> + assigned = true;
> + }
> +
> + return best;
> +}
> +
> +static int ep93xx_div_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + struct clk_psc *psc = to_clk_psc(hw);
> + u32 val;
> + int i;
> +
> + ep93xx_regmap_read(psc->reg, &val);
> + val &= ~psc->mask;
> + for (i = 0; i < psc->num_div; i++)
> + if (rate == DIV_ROUND_CLOSEST(parent_rate, psc->div[i])) {
> + val |= i << psc->shift;
> + break;
> + }
> +
> + if (i == psc->num_div)
> + return -EINVAL;
> +
> + ep93xx_syscon_swlocked_write(val, psc->reg);
> +
> + return 0;
> +}
> +
> +static const struct clk_ops ep93xx_div_ops = {
> + .enable = ep93xx_clk_enable,
> + .disable = ep93xx_clk_disable,
> + .is_enabled = ep93xx_clk_is_enabled,
> + .recalc_rate = ep93xx_div_recalc_rate,
> + .round_rate = ep93xx_div_round_rate,
> + .set_rate = ep93xx_div_set_rate,
> +};
> +
> +static struct clk_hw *clk_hw_register_div(const char *name,
> + const char *parent_name,
> + unsigned int reg,
> + u8 enable_bit,
> + u8 shift,
> + u8 width,
> + char *clk_divisors,
> + u8 num_div)
> +{
> + struct clk_init_data init;
> + struct clk_psc *psc;
> + struct clk *clk;
> +
> + psc = kzalloc(sizeof(*psc), GFP_KERNEL);
> + if (!psc)
> + return ERR_PTR(-ENOMEM);
> +
> + init.name = name;
> + init.ops = &ep93xx_div_ops;
> + init.flags = 0;
> + init.parent_names = (parent_name ? &parent_name : NULL);
> + init.num_parents = 1;
> +
> + psc->reg = reg;
> + psc->bit_idx = enable_bit;
> + psc->mask = GENMASK(shift + width - 1, shift);
> + psc->shift = shift;
> + psc->div = clk_divisors;
> + psc->num_div = num_div;
> + psc->lock = &clk_lock;
> + psc->hw.init = &init;
> +
> + clk = clk_register(NULL, &psc->hw);
> + if (IS_ERR(clk)) {
> + kfree(psc);
> + return ERR_CAST(clk);
> + }
> +
> + return &psc->hw;
> +}
> +
> +struct ep93xx_gate {
> + unsigned int idx;
> + unsigned int bit;
> + const char *dev_id;
> + const char *con_id;
> +};
> +
> +static struct ep93xx_gate ep93xx_uarts[] = {
> + {EP93XX_CLK_UART1, EP93XX_SYSCON_DEVCFG_U1EN, "apb:uart1", NULL},
> + {EP93XX_CLK_UART2, EP93XX_SYSCON_DEVCFG_U2EN, "apb:uart2", NULL},
> + {EP93XX_CLK_UART3, EP93XX_SYSCON_DEVCFG_U3EN, "apb:uart3", NULL},
> +};
> +
> +static void ep93xx_uart_clock_init(void)
> +{
> + unsigned int i;
> + struct clk_hw *hw;
> + u32 val;
> + unsigned int clk_uart_div;
> +
> + ep93xx_regmap_read(EP93XX_SYSCON_PWRCNT, &val);
> + if (val & EP93XX_SYSCON_PWRCNT_UARTBAUD)
> + clk_uart_div = 1;
> + else
> + clk_uart_div = 2;
> +
> + hw = clk_hw_register_fixed_factor(NULL, "uart", "xtali", 0, 1, clk_uart_div);
> + ep93xx_clk_data->hws[EP93XX_CLK_UART] = hw;
> +
> + /* parenting uart gate clocks to uart clock */
> + for (i = 0; i < ARRAY_SIZE(ep93xx_uarts); i++) {
> + hw = ep93xx_clk_register_gate(ep93xx_uarts[i].dev_id,
> + "uart",
> + EP93XX_SYSCON_DEVCFG,
> + ep93xx_uarts[i].bit);
> +
> + ep93xx_clk_data->hws[ep93xx_uarts[i].idx] = hw;
> + }
> +}
> +
> +static struct ep93xx_gate ep93xx_dmas[] = {
> + {EP93XX_CLK_M2P0, EP93XX_SYSCON_PWRCNT_DMA_M2P0, NULL, "m2p0"},
> + {EP93XX_CLK_M2P1, EP93XX_SYSCON_PWRCNT_DMA_M2P1, NULL, "m2p1"},
> + {EP93XX_CLK_M2P2, EP93XX_SYSCON_PWRCNT_DMA_M2P2, NULL, "m2p2"},
> + {EP93XX_CLK_M2P3, EP93XX_SYSCON_PWRCNT_DMA_M2P3, NULL, "m2p3"},
> + {EP93XX_CLK_M2P4, EP93XX_SYSCON_PWRCNT_DMA_M2P4, NULL, "m2p4"},
> + {EP93XX_CLK_M2P5, EP93XX_SYSCON_PWRCNT_DMA_M2P5, NULL, "m2p5"},
> + {EP93XX_CLK_M2P6, EP93XX_SYSCON_PWRCNT_DMA_M2P6, NULL, "m2p6"},
> + {EP93XX_CLK_M2P7, EP93XX_SYSCON_PWRCNT_DMA_M2P7, NULL, "m2p7"},
> + {EP93XX_CLK_M2P8, EP93XX_SYSCON_PWRCNT_DMA_M2P8, NULL, "m2p8"},
> + {EP93XX_CLK_M2P9, EP93XX_SYSCON_PWRCNT_DMA_M2P9, NULL, "m2p9"},
> + {EP93XX_CLK_M2M0, EP93XX_SYSCON_PWRCNT_DMA_M2M0, NULL, "m2m0"},
> + {EP93XX_CLK_M2M1, EP93XX_SYSCON_PWRCNT_DMA_M2M1, NULL, "m2m1"},
> +};
> +
> +static void ep93xx_dma_clock_init(void __iomem *base)
> +{
> + int i;
> + struct clk_hw *hw;
> + int ret;
> +
> + for (i = 0; i < ARRAY_SIZE(ep93xx_dmas); i++) {
> + hw = clk_hw_register_gate(NULL, ep93xx_dmas[i].con_id,
> + "hclk", 0,
> + base + EP93XX_SYSCON_PWRCNT,
> + ep93xx_dmas[i].bit,
> + 0,
> + &clk_lock);
> +
> + ret = clk_hw_register_clkdev(hw, ep93xx_dmas[i].con_id, NULL);
> + if (ret) {
> + pr_err("%s: failed to register lookup %s\n",
> + __func__, ep93xx_dmas[i].con_id);
> + continue;
> + }
> +
> + ep93xx_clk_data->hws[ep93xx_dmas[i].idx] = hw;
> + }
> +}
> +
> +static int ep93xx_clk_probe(struct platform_device *pdev)
> +{
> + void __iomem *base;
> + unsigned int clk_usb_div;
> + unsigned long clk_spi_div;
> + struct clk_hw *hw;
> + struct device *dev = &pdev->dev;
> + u32 value;
> + struct resource *res;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + ep93xx_regmap_read(EP93XX_SYSCON_CLKSET2, &value);
> + clk_usb_div = (((value >> 28) & 0xf) + 1);
> + hw = clk_hw_register_fixed_factor(NULL, "usb_clk", "pll2", 0, 1, clk_usb_div);
> + hw = clk_hw_register_gate(NULL, "ohci-platform",
> + "usb_clk", 0,
> + base + EP93XX_SYSCON_PWRCNT,
> + EP93XX_SYSCON_PWRCNT_USH_EN,
> + 0,
> + &clk_lock);
> + clk_hw_register_clkdev(hw, NULL, "ohci-platform");
> + ep93xx_clk_data->hws[EP93XX_CLK_USB] = hw;
> +
> + /*
> + * EP93xx SSP clock rate was doubled in version E2. For more information
> + * see:
> + * http://www.cirrus.com/en/pubs/appNote/AN273REV4.pdf
> + */
> + clk_spi_div = 1;
> + if (ep93xx_chip_revision() < EP93XX_CHIP_REV_E2)
> + clk_spi_div = 2;
> + hw = clk_hw_register_fixed_factor(NULL, "ep93xx-spi.0", "xtali", 0, 1, clk_spi_div);
> + clk_hw_register_clkdev(hw, NULL, "ep93xx-spi.0");
> + ep93xx_clk_data->hws[EP93XX_CLK_SPI] = hw;
> +
> + /* pwm clock */
> + hw = clk_hw_register_fixed_factor(NULL, "pwm_clk", "xtali", 0, 1, 1);
> + clk_hw_register_clkdev(hw, "pwm_clk", NULL);
> + ep93xx_clk_data->hws[EP93XX_CLK_PWM] = hw;
> +
> + ep93xx_uart_clock_init();
> +
> + /* touchscreen/adc clock */
> + hw = clk_hw_register_div("ep93xx-adc",
> + "xtali",
> + EP93XX_SYSCON_KEYTCHCLKDIV,
> + EP93XX_SYSCON_KEYTCHCLKDIV_TSEN,
> + EP93XX_SYSCON_KEYTCHCLKDIV_ADIV,
> + 1,
> + adc_divisors,
> + ARRAY_SIZE(adc_divisors));
> +
> + clk_hw_register_clkdev(hw, NULL, "ep93xx-adc");
> + ep93xx_clk_data->hws[EP93XX_CLK_ADC] = hw;
> +
> + /* keypad clock */
> + hw = clk_hw_register_div("ep93xx-keypad",
> + "xtali",
> + EP93XX_SYSCON_KEYTCHCLKDIV,
> + EP93XX_SYSCON_KEYTCHCLKDIV_KEN,
> + EP93XX_SYSCON_KEYTCHCLKDIV_KDIV,
> + 1,
> + adc_divisors,
> + ARRAY_SIZE(adc_divisors));
> +
> + clk_hw_register_clkdev(hw, NULL, "ep93xx-keypad");
> + ep93xx_clk_data->hws[EP93XX_CLK_KEYPAD] = hw;
> +
> + /* On reset PDIV and VDIV is set to zero, while PDIV zero
> + * means clock disable, VDIV shouldn't be zero.
> + * So i set both dividers to minimum.
> + */
> + /* ENA - Enable CLK divider. */
> + /* PDIV - 00 - Disable clock */
> + /* VDIV - at least 2 */
> + /* Check and enable video clk registers */
> + ep93xx_regmap_read(EP93XX_SYSCON_VIDCLKDIV, &value);
> + value |= (1 << EP93XX_SYSCON_CLKDIV_PDIV_SHIFT) | 2;
> + ep93xx_syscon_swlocked_write(value, EP93XX_SYSCON_VIDCLKDIV);
> +
> + /* check and enable i2s clk registers */
> + ep93xx_regmap_read(EP93XX_SYSCON_I2SCLKDIV, &value);
> + value |= (1 << EP93XX_SYSCON_CLKDIV_PDIV_SHIFT) | 2;
> + /*
> + * Override the SAI_MSTR_CLK_CFG from the I2S block and use the
> + * I2SClkDiv Register settings. LRCLK transitions on the falling SCLK
> + * edge.
> + */
> + value |= EP93XX_SYSCON_I2SCLKDIV_ORIDE | EP93XX_SYSCON_I2SCLKDIV_SPOL;
> + ep93xx_syscon_swlocked_write(value, EP93XX_SYSCON_I2SCLKDIV);
> +
> + /* video clk */
> + hw = clk_hw_register_ddiv("ep93xx-fb",
> + EP93XX_SYSCON_VIDCLKDIV,
> + EP93XX_SYSCON_CLKDIV_ENABLE);
> +
> + clk_hw_register_clkdev(hw, NULL, "ep93xx-fb");
> + ep93xx_clk_data->hws[EP93XX_CLK_VIDEO] = hw;
> +
> + /* i2s clk */
> + hw = clk_hw_register_ddiv("mclk",
> + EP93XX_SYSCON_I2SCLKDIV,
> + EP93XX_SYSCON_CLKDIV_ENABLE);
> +
> + clk_hw_register_clkdev(hw, "mclk", "ep93xx-i2s");
> + ep93xx_clk_data->hws[EP93XX_CLK_I2S_MCLK] = hw;
> +
> + /* i2s sclk */
> +#define EP93XX_I2SCLKDIV_SDIV_SHIFT 16
> +#define EP93XX_I2SCLKDIV_SDIV_WIDTH 1
> + hw = clk_hw_register_div("sclk",
> + "mclk",
> + EP93XX_SYSCON_I2SCLKDIV,
> + EP93XX_SYSCON_I2SCLKDIV_SENA,
> + EP93XX_I2SCLKDIV_SDIV_SHIFT,
> + EP93XX_I2SCLKDIV_SDIV_WIDTH,
> + sclk_divisors,
> + ARRAY_SIZE(sclk_divisors));
> +
> + clk_hw_register_clkdev(hw, "sclk", "ep93xx-i2s");
> + ep93xx_clk_data->hws[EP93XX_CLK_I2S_SCLK] = hw;
> +
> + /* i2s lrclk */
> +#define EP93XX_I2SCLKDIV_LRDIV32_SHIFT 17
> +#define EP93XX_I2SCLKDIV_LRDIV32_WIDTH 2
> + hw = clk_hw_register_div("lrclk",
> + "sclk",
> + EP93XX_SYSCON_I2SCLKDIV,
> + EP93XX_SYSCON_I2SCLKDIV_SENA,
> + EP93XX_I2SCLKDIV_LRDIV32_SHIFT,
> + EP93XX_I2SCLKDIV_LRDIV32_WIDTH,
> + lrclk_divisors,
> + ARRAY_SIZE(lrclk_divisors));
> +
> + clk_hw_register_clkdev(hw, "lrclk", "ep93xx-i2s");
> + ep93xx_clk_data->hws[EP93XX_CLK_I2S_LRCLK] = hw;
> +
> + /* IrDa clk uses same pattern but no init code presents in original clock driver */
> + return 0;
> +}
> +
> +static const struct of_device_id ep93xx_clk_dt_ids[] = {
> + { .compatible = "cirrus,ep9301-syscon", },
> + { /* sentinel */ },
> +};
> +
> +static struct platform_driver ep93xx_clk_driver = {
> + .probe = ep93xx_clk_probe,
> + .driver = {
> + .name = "ep93xx-clk",
> + .of_match_table = ep93xx_clk_dt_ids,
> + .suppress_bind_attrs = true,
> + },
> +};
> +builtin_platform_driver(ep93xx_clk_driver);
> +
> +static void __init ep93xx_clock_init(struct device_node *np)
> +{
> + void __iomem *base;
> + int i;
> + int ret;
> + u32 value;
> + struct clk_hw *hw;
> + unsigned long clk_pll1_rate;
> + unsigned long clk_f_rate;
> + unsigned long clk_h_rate;
> + unsigned long clk_p_rate;
> + unsigned long clk_pll2_rate;
> + unsigned int clk_f_div;
> + unsigned int clk_h_div;
> + unsigned int clk_p_div;
> +
> + ep93xx_clk_data = kzalloc(struct_size(ep93xx_clk_data, hws,
> + EP93XX_NUM_CLKS),
> + GFP_KERNEL);
> +

Hi,

This is never released if an error occures below.
Missing error handling or it is useless to care about it?

> + if (!ep93xx_clk_data)
> + return;
> +
> + /*
> + * This way all clock fetched before the platform device probes,
> + * except those we assign here for early use, will be deferred.
> + */
> + for (i = 0; i < EP93XX_NUM_CLKS; i++)
> + ep93xx_clk_data->hws[i] = ERR_PTR(-EPROBE_DEFER);
> +
> + base = of_iomap(np, 0);

Same.

> + /* Remap the system controller for the exclusive register */
> + if (IS_ERR(base)) {
> + pr_err("failed to map base\n");
> + return;
> + }
> +
> + ep93xx_map = syscon_node_to_regmap(np);
> + if (IS_ERR(ep93xx_map)) {
> + pr_err("no syscon regmap\n");
> + return;
> + }
> +
> + /*
> + * We check that the regmap works on this very first access,
> + * but as this is an MMIO-backed regmap, subsequent regmap
> + * access is not going to fail and we skip error checks from
> + * this point.
> + */
> + ret = ep93xx_regmap_read(EP93XX_SYSCON_CHIPID, &value);
> + if (ret || (value & 0xffff) != EP93XX_SYSCON_CHIPID_ID) {
> + pr_err("failed to read global status register\n");
> + return;
> + }
> +
> + hw = clk_hw_register_fixed_rate(NULL, "xtali", NULL, 0, EP93XX_EXT_CLK_RATE);
> + clk_hw_register_clkdev(hw, NULL, "xtali");
> +
> + /* Determine the bootloader configured pll1 rate */
> + ep93xx_regmap_read(EP93XX_SYSCON_CLKSET1, &value);
> + if (!(value & EP93XX_SYSCON_CLKSET1_NBYP1))
> + clk_pll1_rate = EP93XX_EXT_CLK_RATE;
> + else
> + clk_pll1_rate = calc_pll_rate(EP93XX_EXT_CLK_RATE, value);
> +
> + hw = clk_hw_register_fixed_rate(NULL, "pll1", "xtali", 0, clk_pll1_rate);
> + clk_hw_register_clkdev(hw, NULL, "pll1");
> + ep93xx_clk_data->hws[EP93XX_CLK_PLL1] = hw;
> +
> + /* Initialize the pll1 derived clocks */
> + clk_f_div = fclk_divisors[(value >> 25) & 0x7];
> + clk_h_div = hclk_divisors[(value >> 20) & 0x7];
> + clk_p_div = pclk_divisors[(value >> 18) & 0x3];
> +
> + hw = clk_hw_register_fixed_factor(NULL, "fclk", "pll1", 0, 1, clk_f_div);
> + clk_f_rate = clk_get_rate(hw->clk);
> + ep93xx_clk_data->hws[EP93XX_CLK_FCLK] = hw;
> + hw = clk_hw_register_fixed_factor(NULL, "hclk", "pll1", 0, 1, clk_h_div);
> + clk_h_rate = clk_get_rate(hw->clk);
> + ep93xx_clk_data->hws[EP93XX_CLK_HCLK] = hw;
> + hw = clk_hw_register_fixed_factor(NULL, "pclk", "hclk", 0, 1, clk_p_div);
> + clk_p_rate = clk_get_rate(hw->clk);
> + ep93xx_clk_data->hws[EP93XX_CLK_PCLK] = hw;
> +
> + clk_hw_register_clkdev(hw, "apb_pclk", NULL);
> +
> + ep93xx_dma_clock_init(base);
> +
> + /* Determine the bootloader configured pll2 rate */
> + ep93xx_regmap_read(EP93XX_SYSCON_CLKSET2, &value);
> + if (!(value & EP93XX_SYSCON_CLKSET2_NBYP2))
> + clk_pll2_rate = EP93XX_EXT_CLK_RATE;
> + else if (value & EP93XX_SYSCON_CLKSET2_PLL2_EN)
> + clk_pll2_rate = calc_pll_rate(EP93XX_EXT_CLK_RATE, value);
> + else
> + clk_pll2_rate = 0;
> +
> + hw = clk_hw_register_fixed_rate(NULL, "pll2", "xtali", 0, clk_pll2_rate);
> + ep93xx_clk_data->hws[EP93XX_CLK_PLL2] = hw;
> + clk_hw_register_clkdev(hw, NULL, "pll2");
> +
> + ep93xx_clk_data->num = EP93XX_NUM_CLKS;
> + of_clk_add_hw_provider(np, of_clk_hw_onecell_get, ep93xx_clk_data);
> +
> + pr_info("PLL1 running at %ld MHz, PLL2 at %ld MHz\n",
> + clk_pll1_rate / 1000000, clk_pll2_rate / 1000000);
> + pr_info("FCLK %ld MHz, HCLK %ld MHz, PCLK %ld MHz\n",
> + clk_f_rate / 1000000, clk_h_rate / 1000000,
> + clk_p_rate / 1000000);
> +}
> +CLK_OF_DECLARE_DRIVER(ep93xx_cc, "cirrus,ep9301-syscon", ep93xx_clock_init);

2023-04-24 17:19:22

by kernel test robot

Subject: Re: [PATCH 12/43] clk: ep93xx: add DT support for Cirrus EP93xx

Hi Nikita,

kernel test robot noticed the following build errors:

[auto build test ERROR on soc/for-next]
[also build test ERROR on robh/for-next linusw-pinctrl/devel linusw-pinctrl/for-next linus/master v6.3]
[cannot apply to next-20230421]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Nikita-Shubin/gpio-ep93xx-split-device-in-multiple/20230424-183649
base: https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
patch link: https://lore.kernel.org/r/20230424123522.18302-13-nikita.shubin%40maquefel.me
patch subject: [PATCH 12/43] clk: ep93xx: add DT support for Cirrus EP93xx
config: sparc-allyesconfig (https://download.01.org/0day-ci/archive/20230425/[email protected]/config)
compiler: sparc64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/8621eda93e4a641c57f889ac36d2386482e92157
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Nikita-Shubin/gpio-ep93xx-split-device-in-multiple/20230424-183649
git checkout 8621eda93e4a641c57f889ac36d2386482e92157
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sparc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sparc SHELL=/bin/bash drivers/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

drivers/clk/clk-ep93xx.c: In function 'ep93xx_clk_enable':
>> drivers/clk/clk-ep93xx.c:153:9: error: implicit declaration of function 'ep93xx_syscon_swlocked_write' [-Werror=implicit-function-declaration]
153 | ep93xx_syscon_swlocked_write(val, psc->reg);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors


vim +/ep93xx_syscon_swlocked_write +153 drivers/clk/clk-ep93xx.c

140
141 static int ep93xx_clk_enable(struct clk_hw *hw)
142 {
143 struct clk_psc *psc = to_clk_psc(hw);
144 unsigned long flags = 0;
145 u32 val;
146
147 if (psc->lock)
148 spin_lock_irqsave(psc->lock, flags);
149
150 ep93xx_regmap_read(psc->reg, &val);
151 val |= BIT(psc->bit_idx);
152
> 153 ep93xx_syscon_swlocked_write(val, psc->reg);
154
155 if (psc->lock)
156 spin_unlock_irqrestore(psc->lock, flags);
157
158 return 0;
159 }
160

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

2023-04-29 01:23:21

by Stephen Boyd

Subject: Re: [PATCH 12/43] clk: ep93xx: add DT support for Cirrus EP93xx

Quoting Nikita Shubin (2023-04-24 05:34:28)
> This is a rewrite of EP93xx timer driver in
> arch/arm/mach-ep93xx/clock.c trying to do everything
> the device tree way:
>
> - convert to syscon driver
> - provide clock acces via of

But clkdev is still around?

>
> Co-developed-by: Alexander Sverdlin <[email protected]>
> Signed-off-by: Alexander Sverdlin <[email protected]>
> Signed-off-by: Nikita Shubin <[email protected]>
> ---
>
> Notes:
> rfc->v0
> Linus Walleij:
> - renamed all __underscore
>
> Alexander Sverdlin:
> - "Logick" -> "Logic"
>
> Changes by Alexander Sverdlin:
> - remove pr_info
> - DIV_ROUND_UP_ULL -> DIV_ROUND_CLOSEST
> - fix zeroing bitfield in ep93xx_div_set_rate
> - add sanity check for EP93XX_SYSCON_CHIPID_ID
> - use bit index for DMA clock's
> - ep93xx_clk_register_gate() takes bit index, not mask
> - remove redundant define
> - use DIV_ROUND_CLOSEST() everywhere to achieve frequencies closer to those
> requested
> - Add the forgotten configuration from the deleted
> arch/arm/mach-ep93xx/core.c
>
> drivers/clk/Kconfig | 8 +
> drivers/clk/Makefile | 1 +
> drivers/clk/clk-ep93xx.c | 880 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 889 insertions(+)
> create mode 100644 drivers/clk/clk-ep93xx.c
>
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 1eef05bb1f99..d3ad975b688e 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -436,6 +436,14 @@ config COMMON_CLK_K210
> help
> Support for the Canaan Kendryte K210 RISC-V SoC clocks.
>
> +config COMMON_CLK_EP93XX

Keep this sorted based on Kconfig symbol.

> + bool "Clock driver for Cirrus Logic ep93xx SoC"
> + depends on ARCH_EP93XX || COMPILE_TEST
> + select MFD_SYSCON
> + select REGMAP
> + help
> + This driver supports the SoC clocks on the Cirrus Logic ep93xx.
> +
> source "drivers/clk/actions/Kconfig"
> source "drivers/clk/analogbits/Kconfig"
> source "drivers/clk/baikal-t1/Kconfig"
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index e3ca0d058a25..deec25ffd004 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_COMMON_CLK_CDCE706) += clk-cdce706.o
> obj-$(CONFIG_COMMON_CLK_CDCE925) += clk-cdce925.o
> obj-$(CONFIG_ARCH_CLPS711X) += clk-clps711x.o
> obj-$(CONFIG_COMMON_CLK_CS2000_CP) += clk-cs2000-cp.o
> +obj-$(CONFIG_COMMON_CLK_EP93XX) += clk-ep93xx.o
> obj-$(CONFIG_ARCH_SPARX5) += clk-sparx5.o
> obj-$(CONFIG_COMMON_CLK_EN7523) += clk-en7523.o
> obj-$(CONFIG_COMMON_CLK_FIXED_MMIO) += clk-fixed-mmio.o
> diff --git a/drivers/clk/clk-ep93xx.c b/drivers/clk/clk-ep93xx.c
> new file mode 100644
> index 000000000000..e5913fc4c39b
> --- /dev/null
> +++ b/drivers/clk/clk-ep93xx.c
> @@ -0,0 +1,880 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Clock control for Cirrus EP93xx chips.
> + * Copyright (C) 2021 Nikita Shubin <[email protected]>
> + *
> + * Based on a rewrite of arch/arm/mach-ep93xx/clock.c:
> + * Copyright (C) 2006 Lennert Buytenhek <[email protected]>
> + */
> +#define pr_fmt(fmt) "ep93xx " KBUILD_MODNAME ": " fmt
> +
> +#include <linux/kernel.h>
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/string.h>
> +#include <linux/io.h>
> +#include <linux/spinlock.h>
> +#include <linux/clkdev.h>
> +#include <linux/clk-provider.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/soc/cirrus/ep93xx.h>
> +#include <dt-bindings/clock/cirrus,ep93xx-clock.h>
> +
> +#include <asm/div64.h>
> +
> +#define EP93XX_EXT_CLK_RATE 14745600
> +#define EP93XX_EXT_RTC_RATE 32768
> +
> +#define EP93XX_SYSCON_POWER_STATE 0x00
> +#define EP93XX_SYSCON_PWRCNT 0x04
> +#define EP93XX_SYSCON_PWRCNT_UARTBAUD BIT(29)
> +#define EP93XX_SYSCON_PWRCNT_USH_EN 28
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2M1 27
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2M0 26
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P8 25
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P9 24
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P6 23
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P7 22
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P4 21
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P5 20
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P2 19
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P3 18
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P0 17
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P1 16
> +#define EP93XX_SYSCON_CLKSET1 0x20
> +#define EP93XX_SYSCON_CLKSET1_NBYP1 BIT(23)
> +#define EP93XX_SYSCON_CLKSET2 0x24
> +#define EP93XX_SYSCON_CLKSET2_NBYP2 BIT(19)
> +#define EP93XX_SYSCON_CLKSET2_PLL2_EN BIT(18)
> +#define EP93XX_SYSCON_DEVCFG 0x80
> +#define EP93XX_SYSCON_DEVCFG_U3EN 24
> +#define EP93XX_SYSCON_DEVCFG_U2EN 20
> +#define EP93XX_SYSCON_DEVCFG_U1EN 18
> +#define EP93XX_SYSCON_VIDCLKDIV 0x84
> +#define EP93XX_SYSCON_CLKDIV_ENABLE 15
> +#define EP93XX_SYSCON_CLKDIV_ESEL BIT(14)
> +#define EP93XX_SYSCON_CLKDIV_PSEL BIT(13)
> +#define EP93XX_SYSCON_CLKDIV_PDIV_SHIFT 8
> +#define EP93XX_SYSCON_I2SCLKDIV 0x8c
> +#define EP93XX_SYSCON_I2SCLKDIV_SENA 31
> +#define EP93XX_SYSCON_I2SCLKDIV_ORIDE BIT(29)
> +#define EP93XX_SYSCON_I2SCLKDIV_SPOL BIT(19)
> +#define EP93XX_I2SCLKDIV_SDIV (1 << 16)
> +#define EP93XX_I2SCLKDIV_LRDIV32 (0 << 17)
> +#define EP93XX_I2SCLKDIV_LRDIV64 (1 << 17)
> +#define EP93XX_I2SCLKDIV_LRDIV128 (2 << 17)
> +#define EP93XX_I2SCLKDIV_LRDIV_MASK (3 << 17)
> +#define EP93XX_SYSCON_KEYTCHCLKDIV 0x90
> +#define EP93XX_SYSCON_KEYTCHCLKDIV_TSEN 31
> +#define EP93XX_SYSCON_KEYTCHCLKDIV_ADIV 16
> +#define EP93XX_SYSCON_KEYTCHCLKDIV_KEN 15
> +#define EP93XX_SYSCON_KEYTCHCLKDIV_KDIV 0
> +#define EP93XX_SYSCON_CHIPID 0x94
> +#define EP93XX_SYSCON_CHIPID_ID 0x9213
> +
> +static DEFINE_SPINLOCK(clk_lock);

This name is not unique. Please make it unique by prefixing with ep93xx.

> +static struct regmap *ep93xx_map;

Do these need to be static globals? Doing a runtime allocation reduces
the size overhead for kernel images that don't have this hardware but
compile in this driver, i.e. distros.

> +#define ep93xx_regmap_read(reg, val) regmap_read(ep93xx_map, reg, val)
> +
> +/* Keeps track of all clocks */
> +static struct clk_hw_onecell_data *ep93xx_clk_data;

Same static global question.

> +
> +static char fclk_divisors[] = { 1, 2, 4, 8, 16, 1, 1, 1 };
> +static char hclk_divisors[] = { 1, 2, 4, 5, 6, 8, 16, 32 };
> +static char pclk_divisors[] = { 1, 2, 4, 8 };
> +
> +static char adc_divisors[] = { 16, 4 };
> +static char sclk_divisors[] = { 2, 4 };
> +static char lrclk_divisors[] = { 32, 64, 128 };

const?

> +
> +static const char * const mux_parents[] = {
> + "xtali",
> + "pll1",
> + "pll2"
> +};
> +
> +/*
> + * PLL rate = 14.7456 MHz * (X1FBD + 1) * (X2FBD + 1) / (X2IPD + 1) / 2^PS
> + */
> +static unsigned long calc_pll_rate(unsigned long long rate, u32 config_word)

u64 rate

> +{
> + int i;
> +
> + rate *= ((config_word >> 11) & 0x1f) + 1; /* X1FBD */
> + rate *= ((config_word >> 5) & 0x3f) + 1; /* X2FBD */
> + do_div(rate, (config_word & 0x1f) + 1); /* X2IPD */
> + for (i = 0; i < ((config_word >> 16) & 3); i++) /* PS */
> + rate >>= 1;
> +
> + return (unsigned long)rate;

Drop useless cast.

> +}
> +
> +struct clk_psc {
> + struct clk_hw hw;
> + unsigned int reg;
> + u8 bit_idx;
> + u32 mask;
> + u8 shift;
> + u8 width;
> + char *div;
> + u8 num_div;
> + spinlock_t *lock;
> +};
> +
> +#define to_clk_psc(_hw) container_of(_hw, struct clk_psc, hw)
> +
> +static int ep93xx_clk_is_enabled(struct clk_hw *hw)
> +{
> + struct clk_psc *psc = to_clk_psc(hw);
> + u32 val;
> +
> + ep93xx_regmap_read(psc->reg, &val);
> +
> + return (val & BIT(psc->bit_idx)) ? 1 : 0;
> +}
> +
> +static int ep93xx_clk_enable(struct clk_hw *hw)
> +{
> + struct clk_psc *psc = to_clk_psc(hw);
> + unsigned long flags = 0;
> + u32 val;
> +
> + if (psc->lock)
> + spin_lock_irqsave(psc->lock, flags);
> +
> + ep93xx_regmap_read(psc->reg, &val);
> + val |= BIT(psc->bit_idx);
> +
> + ep93xx_syscon_swlocked_write(val, psc->reg);
> +
> + if (psc->lock)
> + spin_unlock_irqrestore(psc->lock, flags);
> +
> + return 0;
> +}
> +
> +static void ep93xx_clk_disable(struct clk_hw *hw)
> +{
> + struct clk_psc *psc = to_clk_psc(hw);
> + unsigned long flags = 0;
> + u32 val;
> +
> + if (psc->lock)
> + spin_lock_irqsave(psc->lock, flags);
> +
> + ep93xx_regmap_read(psc->reg, &val);
> + val &= ~BIT(psc->bit_idx);
> +
> + ep93xx_syscon_swlocked_write(val, psc->reg);
> +
> + if (psc->lock)
> + spin_unlock_irqrestore(psc->lock, flags);
> +}
> +
> +static const struct clk_ops clk_ep93xx_gate_ops = {
> + .enable = ep93xx_clk_enable,
> + .disable = ep93xx_clk_disable,
> + .is_enabled = ep93xx_clk_is_enabled,
> +};
> +
> +static struct clk_hw *ep93xx_clk_register_gate(const char *name,
> + const char *parent_name,
> + unsigned int reg,
> + u8 bit_idx)
> +{
> + struct clk_init_data init;
> + struct clk_psc *psc;
> + struct clk *clk;
> +
> + psc = kzalloc(sizeof(*psc), GFP_KERNEL);
> + if (!psc)
> + return ERR_PTR(-ENOMEM);
> +
> + init.name = name;
> + init.ops = &clk_ep93xx_gate_ops;
> + init.flags = CLK_SET_RATE_PARENT;
> + init.parent_names = (parent_name ? &parent_name : NULL);
> + init.num_parents = (parent_name ? 1 : 0);

Please use parent_data or parent_hw instead.

> +
> + psc->reg = reg;
> + psc->bit_idx = bit_idx;
> + psc->hw.init = &init;
> + psc->lock = &clk_lock;
> +
> + clk = clk_register(NULL, &psc->hw);
> + if (IS_ERR(clk)) {
> + kfree(psc);
> + return ERR_CAST(clk);
> + }
> +
> + return &psc->hw;
> +}
> +
> +static u8 ep93xx_mux_get_parent(struct clk_hw *hw)
> +{
> + struct clk_psc *psc = to_clk_psc(hw);
> + u32 val;
> +
> + ep93xx_regmap_read(psc->reg, &val);
> + if (!(val & EP93XX_SYSCON_CLKDIV_ESEL))
> + return 0;
> +
> + if (!(val & EP93XX_SYSCON_CLKDIV_PSEL))
> + return 1;
> +
> + return 2;
> +}
> +
> +static int ep93xx_mux_set_parent_lock(struct clk_hw *hw, u8 index)
> +{
> + struct clk_psc *psc = to_clk_psc(hw);
> + unsigned long flags = 0;
> + u32 val;
> +
> + if (index >= ARRAY_SIZE(mux_parents))
> + return -EINVAL;
> +
> + if (psc->lock)

Can you simply lock all the time?

> + spin_lock_irqsave(psc->lock, flags);
> +
> + ep93xx_regmap_read(psc->reg, &val);
> + val &= ~(EP93XX_SYSCON_CLKDIV_ESEL | EP93XX_SYSCON_CLKDIV_PSEL);
> +
> + if (index != 0) {
> + val |= EP93XX_SYSCON_CLKDIV_ESEL;
> + val |= (index - 1) ? EP93XX_SYSCON_CLKDIV_PSEL : 0;
> + }
> +
> + ep93xx_syscon_swlocked_write(val, psc->reg);
> +
> + if (psc->lock)
> + spin_unlock_irqrestore(psc->lock, flags);
> +
> + return 0;
> +}
> +
> +static bool is_best(unsigned long rate, unsigned long now,
> + unsigned long best)
> +{
> + return abs(rate - now) < abs(rate - best);
> +}
> +
> +static int ep93xx_mux_determine_rate(struct clk_hw *hw,
> + struct clk_rate_request *req)
> +{
> + unsigned long rate = req->rate;
> + struct clk *parent_best = NULL;
> + unsigned long parent_rate;
> + unsigned long best_rate = 0, actual_rate, mclk_rate;
> + unsigned long parent_rate_best;
> + int div, pdiv;
> + int i;
> +
> + /*
> + * Try the two pll's and the external clock
> + * Because the valid predividers are 2, 2.5 and 3, we multiply
> + * all the clocks by 2 to avoid floating point math.
> + *
> + * This is based on the algorithm in the ep93xx raster guide:
> + * http://be-a-maverick.com/en/pubs/appNote/AN269REV1.pdf
> + *
> + */
> + for (i = 0; i < ARRAY_SIZE(mux_parents); i++) {
> + struct clk *parent = clk_get_sys(mux_parents[i], NULL);

The possible parents can be had with clk_hw_get_parent_by_index(). Use
that so you can stop leaking .

> +
> + parent_rate = clk_get_rate(parent);
> + mclk_rate = parent_rate * 2;

The parent_rate is available via clk_hw_get_rate() or by looking at the
clk_rate_request structure 'req' if the parent is the current parent, or
by calling clk_hw_round_rate() or the poorly named
__clk_determine_rate(). In this case it looks like the parents aren't
expected to change rate, so clk_hw_get_rate() will work.

> +
> + /* Try each predivider value */
> + for (pdiv = 4; pdiv <= 6; pdiv++) {
> + div = DIV_ROUND_CLOSEST(mclk_rate, rate * pdiv);
> + if (div < 1 || div > 127)
> + continue;
> +
> + actual_rate = DIV_ROUND_CLOSEST(mclk_rate, pdiv * div);
> +
> + if (is_best(rate, actual_rate, best_rate)) {
> + best_rate = actual_rate;
> + parent_rate_best = parent_rate;
> + parent_best = parent;
> + }
> + }
> + }
> +
> + if (!parent_best)
> + return -EINVAL;
> +
> + req->best_parent_rate = parent_rate_best;
> + req->best_parent_hw = __clk_get_hw(parent_best);
> + req->rate = best_rate;
> +
> + return 0;
> +}
> +
> +static unsigned long ep93xx_ddiv_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct clk_psc *psc = to_clk_psc(hw);
> + unsigned long rate = 0;
> + u32 val;
> + int pdiv, div;
> +
> + ep93xx_regmap_read(psc->reg, &val);
> + pdiv = ((val >> EP93XX_SYSCON_CLKDIV_PDIV_SHIFT) & 0x03);
> + div = val & 0x7f;
> + if (div > 0)
> + rate = DIV_ROUND_CLOSEST(parent_rate * 2, (pdiv + 3) * div);
> +
> + return rate;
> +}
> +
> +static int ep93xx_ddiv_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + struct clk_psc *psc = to_clk_psc(hw);
> + int pdiv, div, npdiv, ndiv;
> + unsigned long actual_rate, mclk_rate, rate_err = -1;
> + int found = 0;
> + u32 val;
> +
> + ep93xx_regmap_read(psc->reg, &val);
> + mclk_rate = parent_rate * 2;
> +
> + for (pdiv = 4; pdiv <= 6; pdiv++) {
> + div = DIV_ROUND_CLOSEST(mclk_rate, rate * pdiv);
> + if (div < 1 || div > 127)
> + continue;
> +
> + actual_rate = DIV_ROUND_CLOSEST(mclk_rate, pdiv * div);
> +
> + if (!found || abs(actual_rate - rate) < rate_err) {
> + npdiv = pdiv - 3;
> + ndiv = div;
> + rate_err = abs(actual_rate - rate);
> + found = 1;
> + }
> + }
> +
> + if (!found)
> + return -EINVAL;
> +
> + /* Clear old dividers */
> + val &= ~0x37f;
> +
> + /* Set the new pdiv and div bits for the new clock rate */
> + val |= (npdiv << EP93XX_SYSCON_CLKDIV_PDIV_SHIFT) | ndiv;
> +
> + ep93xx_syscon_swlocked_write(val, psc->reg);
> +
> + return 0;
> +}
> +
> +static const struct clk_ops clk_ddiv_ops = {
> + .enable = ep93xx_clk_enable,
> + .disable = ep93xx_clk_disable,
> + .is_enabled = ep93xx_clk_is_enabled,
> + .get_parent = ep93xx_mux_get_parent,
> + .set_parent = ep93xx_mux_set_parent_lock,
> + .determine_rate = ep93xx_mux_determine_rate,
> + .recalc_rate = ep93xx_ddiv_recalc_rate,
> + .set_rate = ep93xx_ddiv_set_rate,
> +};
> +
> +static struct clk_hw *clk_hw_register_ddiv(const char *name,
> + unsigned int reg,
> + u8 bit_idx)
> +{
> + struct clk_init_data init;
> + struct clk_psc *psc;
> + struct clk *clk;
> +
> + psc = kzalloc(sizeof(*psc), GFP_KERNEL);
> + if (!psc)
> + return ERR_PTR(-ENOMEM);
> +
> + init.name = name;
> + init.ops = &clk_ddiv_ops;
> + init.flags = 0;
> + init.parent_names = mux_parents;
> + init.num_parents = ARRAY_SIZE(mux_parents);
> +
> + psc->reg = reg;
> + psc->bit_idx = bit_idx;
> + psc->lock = &clk_lock;
> + psc->hw.init = &init;
> +
> + clk = clk_register(NULL, &psc->hw);
> + if (IS_ERR(clk)) {
> + kfree(psc);
> + return ERR_CAST(clk);
> + }
> +
> + return &psc->hw;
> +}
> +
> +static unsigned long ep93xx_div_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct clk_psc *psc = to_clk_psc(hw);
> + u32 val;
> + u8 index;
> +
> + ep93xx_regmap_read(psc->reg, &val);
> + index = (val & psc->mask) >> psc->shift;
> + if (index > psc->num_div)
> + return 0;
> +
> + return DIV_ROUND_CLOSEST(parent_rate, psc->div[index]);
> +}
> +
> +static long ep93xx_div_round_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long *parent_rate)
> +{
> + struct clk_psc *psc = to_clk_psc(hw);
> + unsigned long best = 0, now;
> + bool assigned = false;
> + int i;
> +
> + for (i = 0; i < psc->num_div; i++) {
> + if ((rate * psc->div[i]) == *parent_rate)
> + return rate;
> +
> + now = DIV_ROUND_CLOSEST(*parent_rate, psc->div[i]);
> +
> + if (!assigned || is_best(rate, now, best))
> + best = now;
> + assigned = true;
> + }
> +
> + return best;
> +}
> +
> +static int ep93xx_div_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + struct clk_psc *psc = to_clk_psc(hw);
> + u32 val;
> + int i;
> +
> + ep93xx_regmap_read(psc->reg, &val);
> + val &= ~psc->mask;
> + for (i = 0; i < psc->num_div; i++)
> + if (rate == DIV_ROUND_CLOSEST(parent_rate, psc->div[i])) {
> + val |= i << psc->shift;
> + break;
> + }
> +
> + if (i == psc->num_div)
> + return -EINVAL;
> +
> + ep93xx_syscon_swlocked_write(val, psc->reg);
> +
> + return 0;
> +}
> +
> +static const struct clk_ops ep93xx_div_ops = {
> + .enable = ep93xx_clk_enable,
> + .disable = ep93xx_clk_disable,
> + .is_enabled = ep93xx_clk_is_enabled,
> + .recalc_rate = ep93xx_div_recalc_rate,
> + .round_rate = ep93xx_div_round_rate,
> + .set_rate = ep93xx_div_set_rate,
> +};
> +
> +static struct clk_hw *clk_hw_register_div(const char *name,
> + const char *parent_name,
> + unsigned int reg,
> + u8 enable_bit,
> + u8 shift,
> + u8 width,
> + char *clk_divisors,
> + u8 num_div)
> +{
> + struct clk_init_data init;
> + struct clk_psc *psc;
> + struct clk *clk;
> +
> + psc = kzalloc(sizeof(*psc), GFP_KERNEL);
> + if (!psc)
> + return ERR_PTR(-ENOMEM);
> +
> + init.name = name;
> + init.ops = &ep93xx_div_ops;
> + init.flags = 0;
> + init.parent_names = (parent_name ? &parent_name : NULL);
> + init.num_parents = 1;
> +
> + psc->reg = reg;
> + psc->bit_idx = enable_bit;
> + psc->mask = GENMASK(shift + width - 1, shift);
> + psc->shift = shift;
> + psc->div = clk_divisors;
> + psc->num_div = num_div;
> + psc->lock = &clk_lock;
> + psc->hw.init = &init;
> +
> + clk = clk_register(NULL, &psc->hw);

Use clk_hw_register()

> + if (IS_ERR(clk)) {
> + kfree(psc);
> + return ERR_CAST(clk);
> + }
> +
> + return &psc->hw;
> +}
> +
> +struct ep93xx_gate {
> + unsigned int idx;
> + unsigned int bit;
> + const char *dev_id;
> + const char *con_id;
> +};
> +
> +static struct ep93xx_gate ep93xx_uarts[] = {
> + {EP93XX_CLK_UART1, EP93XX_SYSCON_DEVCFG_U1EN, "apb:uart1", NULL},
> + {EP93XX_CLK_UART2, EP93XX_SYSCON_DEVCFG_U2EN, "apb:uart2", NULL},
> + {EP93XX_CLK_UART3, EP93XX_SYSCON_DEVCFG_U3EN, "apb:uart3", NULL},
> +};
> +
> +static void ep93xx_uart_clock_init(void)
> +{
> + unsigned int i;
> + struct clk_hw *hw;
> + u32 val;
> + unsigned int clk_uart_div;
> +
> + ep93xx_regmap_read(EP93XX_SYSCON_PWRCNT, &val);
> + if (val & EP93XX_SYSCON_PWRCNT_UARTBAUD)
> + clk_uart_div = 1;
> + else
> + clk_uart_div = 2;
> +
> + hw = clk_hw_register_fixed_factor(NULL, "uart", "xtali", 0, 1, clk_uart_div);
> + ep93xx_clk_data->hws[EP93XX_CLK_UART] = hw;
> +
> + /* parenting uart gate clocks to uart clock */
> + for (i = 0; i < ARRAY_SIZE(ep93xx_uarts); i++) {
> + hw = ep93xx_clk_register_gate(ep93xx_uarts[i].dev_id,
> + "uart",
> + EP93XX_SYSCON_DEVCFG,
> + ep93xx_uarts[i].bit);
> +
> + ep93xx_clk_data->hws[ep93xx_uarts[i].idx] = hw;
> + }
> +}
> +
> +static struct ep93xx_gate ep93xx_dmas[] = {

const?

> + {EP93XX_CLK_M2P0, EP93XX_SYSCON_PWRCNT_DMA_M2P0, NULL, "m2p0"},

Put space after { and before }

> + {EP93XX_CLK_M2P1, EP93XX_SYSCON_PWRCNT_DMA_M2P1, NULL, "m2p1"},
> + {EP93XX_CLK_M2P2, EP93XX_SYSCON_PWRCNT_DMA_M2P2, NULL, "m2p2"},
> + {EP93XX_CLK_M2P3, EP93XX_SYSCON_PWRCNT_DMA_M2P3, NULL, "m2p3"},
> + {EP93XX_CLK_M2P4, EP93XX_SYSCON_PWRCNT_DMA_M2P4, NULL, "m2p4"},
> + {EP93XX_CLK_M2P5, EP93XX_SYSCON_PWRCNT_DMA_M2P5, NULL, "m2p5"},
> + {EP93XX_CLK_M2P6, EP93XX_SYSCON_PWRCNT_DMA_M2P6, NULL, "m2p6"},
> + {EP93XX_CLK_M2P7, EP93XX_SYSCON_PWRCNT_DMA_M2P7, NULL, "m2p7"},
> + {EP93XX_CLK_M2P8, EP93XX_SYSCON_PWRCNT_DMA_M2P8, NULL, "m2p8"},
> + {EP93XX_CLK_M2P9, EP93XX_SYSCON_PWRCNT_DMA_M2P9, NULL, "m2p9"},
> + {EP93XX_CLK_M2M0, EP93XX_SYSCON_PWRCNT_DMA_M2M0, NULL, "m2m0"},
> + {EP93XX_CLK_M2M1, EP93XX_SYSCON_PWRCNT_DMA_M2M1, NULL, "m2m1"},
> +};
> +
> +static void ep93xx_dma_clock_init(void __iomem *base)
> +{
> + int i;
> + struct clk_hw *hw;
> + int ret;
> +
> + for (i = 0; i < ARRAY_SIZE(ep93xx_dmas); i++) {
> + hw = clk_hw_register_gate(NULL, ep93xx_dmas[i].con_id,
> + "hclk", 0,
> + base + EP93XX_SYSCON_PWRCNT,
> + ep93xx_dmas[i].bit,
> + 0,
> + &clk_lock);
> +
> + ret = clk_hw_register_clkdev(hw, ep93xx_dmas[i].con_id, NULL);
> + if (ret) {
> + pr_err("%s: failed to register lookup %s\n",
> + __func__, ep93xx_dmas[i].con_id);
> + continue;
> + }
> +
> + ep93xx_clk_data->hws[ep93xx_dmas[i].idx] = hw;
> + }
> +}
> +
> +static int ep93xx_clk_probe(struct platform_device *pdev)
> +{
> + void __iomem *base;
> + unsigned int clk_usb_div;
> + unsigned long clk_spi_div;
> + struct clk_hw *hw;
> + struct device *dev = &pdev->dev;
> + u32 value;
> + struct resource *res;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + ep93xx_regmap_read(EP93XX_SYSCON_CLKSET2, &value);
> + clk_usb_div = (((value >> 28) & 0xf) + 1);
> + hw = clk_hw_register_fixed_factor(NULL, "usb_clk", "pll2", 0, 1, clk_usb_div);
> + hw = clk_hw_register_gate(NULL, "ohci-platform",
> + "usb_clk", 0,
> + base + EP93XX_SYSCON_PWRCNT,
> + EP93XX_SYSCON_PWRCNT_USH_EN,
> + 0,
> + &clk_lock);
> + clk_hw_register_clkdev(hw, NULL, "ohci-platform");
> + ep93xx_clk_data->hws[EP93XX_CLK_USB] = hw;
> +
> + /*
> + * EP93xx SSP clock rate was doubled in version E2. For more information
> + * see:
> + * http://www.cirrus.com/en/pubs/appNote/AN273REV4.pdf
> + */
> + clk_spi_div = 1;
> + if (ep93xx_chip_revision() < EP93XX_CHIP_REV_E2)
> + clk_spi_div = 2;
> + hw = clk_hw_register_fixed_factor(NULL, "ep93xx-spi.0", "xtali", 0, 1, clk_spi_div);
> + clk_hw_register_clkdev(hw, NULL, "ep93xx-spi.0");
> + ep93xx_clk_data->hws[EP93XX_CLK_SPI] = hw;
> +
> + /* pwm clock */
> + hw = clk_hw_register_fixed_factor(NULL, "pwm_clk", "xtali", 0, 1, 1);
> + clk_hw_register_clkdev(hw, "pwm_clk", NULL);
> + ep93xx_clk_data->hws[EP93XX_CLK_PWM] = hw;
> +
> + ep93xx_uart_clock_init();
> +
> + /* touchscreen/adc clock */
> + hw = clk_hw_register_div("ep93xx-adc",
> + "xtali",
> + EP93XX_SYSCON_KEYTCHCLKDIV,
> + EP93XX_SYSCON_KEYTCHCLKDIV_TSEN,
> + EP93XX_SYSCON_KEYTCHCLKDIV_ADIV,
> + 1,
> + adc_divisors,
> + ARRAY_SIZE(adc_divisors));
> +
> + clk_hw_register_clkdev(hw, NULL, "ep93xx-adc");
> + ep93xx_clk_data->hws[EP93XX_CLK_ADC] = hw;
> +
> + /* keypad clock */
> + hw = clk_hw_register_div("ep93xx-keypad",
> + "xtali",
> + EP93XX_SYSCON_KEYTCHCLKDIV,
> + EP93XX_SYSCON_KEYTCHCLKDIV_KEN,
> + EP93XX_SYSCON_KEYTCHCLKDIV_KDIV,
> + 1,
> + adc_divisors,
> + ARRAY_SIZE(adc_divisors));
> +
> + clk_hw_register_clkdev(hw, NULL, "ep93xx-keypad");
> + ep93xx_clk_data->hws[EP93XX_CLK_KEYPAD] = hw;
> +
> + /* On reset PDIV and VDIV is set to zero, while PDIV zero

The /* is on a line by itself.

> + * means clock disable, VDIV shouldn't be zero.
> + * So i set both dividers to minimum.
> + */
> + /* ENA - Enable CLK divider. */
> + /* PDIV - 00 - Disable clock */
> + /* VDIV - at least 2 */
> + /* Check and enable video clk registers */
> + ep93xx_regmap_read(EP93XX_SYSCON_VIDCLKDIV, &value);
> + value |= (1 << EP93XX_SYSCON_CLKDIV_PDIV_SHIFT) | 2;
> + ep93xx_syscon_swlocked_write(value, EP93XX_SYSCON_VIDCLKDIV);
> +
> + /* check and enable i2s clk registers */
> + ep93xx_regmap_read(EP93XX_SYSCON_I2SCLKDIV, &value);
> + value |= (1 << EP93XX_SYSCON_CLKDIV_PDIV_SHIFT) | 2;
> + /*
> + * Override the SAI_MSTR_CLK_CFG from the I2S block and use the
> + * I2SClkDiv Register settings. LRCLK transitions on the falling SCLK
> + * edge.
> + */
> + value |= EP93XX_SYSCON_I2SCLKDIV_ORIDE | EP93XX_SYSCON_I2SCLKDIV_SPOL;
> + ep93xx_syscon_swlocked_write(value, EP93XX_SYSCON_I2SCLKDIV);
> +
> + /* video clk */
> + hw = clk_hw_register_ddiv("ep93xx-fb",
> + EP93XX_SYSCON_VIDCLKDIV,
> + EP93XX_SYSCON_CLKDIV_ENABLE);
> +
> + clk_hw_register_clkdev(hw, NULL, "ep93xx-fb");
> + ep93xx_clk_data->hws[EP93XX_CLK_VIDEO] = hw;
> +
> + /* i2s clk */
> + hw = clk_hw_register_ddiv("mclk",
> + EP93XX_SYSCON_I2SCLKDIV,
> + EP93XX_SYSCON_CLKDIV_ENABLE);
> +
> + clk_hw_register_clkdev(hw, "mclk", "ep93xx-i2s");
> + ep93xx_clk_data->hws[EP93XX_CLK_I2S_MCLK] = hw;
> +
> + /* i2s sclk */
> +#define EP93XX_I2SCLKDIV_SDIV_SHIFT 16
> +#define EP93XX_I2SCLKDIV_SDIV_WIDTH 1
> + hw = clk_hw_register_div("sclk",
> + "mclk",
> + EP93XX_SYSCON_I2SCLKDIV,
> + EP93XX_SYSCON_I2SCLKDIV_SENA,
> + EP93XX_I2SCLKDIV_SDIV_SHIFT,
> + EP93XX_I2SCLKDIV_SDIV_WIDTH,
> + sclk_divisors,
> + ARRAY_SIZE(sclk_divisors));
> +
> + clk_hw_register_clkdev(hw, "sclk", "ep93xx-i2s");
> + ep93xx_clk_data->hws[EP93XX_CLK_I2S_SCLK] = hw;
> +
> + /* i2s lrclk */
> +#define EP93XX_I2SCLKDIV_LRDIV32_SHIFT 17
> +#define EP93XX_I2SCLKDIV_LRDIV32_WIDTH 2

Please just inline these defines as they're only used on place.

> + hw = clk_hw_register_div("lrclk",
> + "sclk",
> + EP93XX_SYSCON_I2SCLKDIV,
> + EP93XX_SYSCON_I2SCLKDIV_SENA,
> + EP93XX_I2SCLKDIV_LRDIV32_SHIFT,
> + EP93XX_I2SCLKDIV_LRDIV32_WIDTH,
> + lrclk_divisors,
> + ARRAY_SIZE(lrclk_divisors));
> +
> + clk_hw_register_clkdev(hw, "lrclk", "ep93xx-i2s");
> + ep93xx_clk_data->hws[EP93XX_CLK_I2S_LRCLK] = hw;
> +
> + /* IrDa clk uses same pattern but no init code presents in original clock driver */
> + return 0;
> +}
> +
> +static const struct of_device_id ep93xx_clk_dt_ids[] = {
> + { .compatible = "cirrus,ep9301-syscon", },
> + { /* sentinel */ },
> +};
> +
> +static struct platform_driver ep93xx_clk_driver = {
> + .probe = ep93xx_clk_probe,
> + .driver = {
> + .name = "ep93xx-clk",
> + .of_match_table = ep93xx_clk_dt_ids,
> + .suppress_bind_attrs = true,
> + },
> +};
> +builtin_platform_driver(ep93xx_clk_driver);
> +
> +static void __init ep93xx_clock_init(struct device_node *np)
> +{
> + void __iomem *base;
> + int i;
> + int ret;
> + u32 value;
> + struct clk_hw *hw;
> + unsigned long clk_pll1_rate;
> + unsigned long clk_f_rate;
> + unsigned long clk_h_rate;
> + unsigned long clk_p_rate;
> + unsigned long clk_pll2_rate;
> + unsigned int clk_f_div;
> + unsigned int clk_h_div;
> + unsigned int clk_p_div;
> +
> + ep93xx_clk_data = kzalloc(struct_size(ep93xx_clk_data, hws,
> + EP93XX_NUM_CLKS),
> + GFP_KERNEL);
> +
> + if (!ep93xx_clk_data)
> + return;
> +
> + /*
> + * This way all clock fetched before the platform device probes,
> + * except those we assign here for early use, will be deferred.
> + */
> + for (i = 0; i < EP93XX_NUM_CLKS; i++)
> + ep93xx_clk_data->hws[i] = ERR_PTR(-EPROBE_DEFER);
> +
> + base = of_iomap(np, 0);
> + /* Remap the system controller for the exclusive register */
> + if (IS_ERR(base)) {
> + pr_err("failed to map base\n");
> + return;
> + }
> +
> + ep93xx_map = syscon_node_to_regmap(np);
> + if (IS_ERR(ep93xx_map)) {
> + pr_err("no syscon regmap\n");
> + return;
> + }
> +
> + /*
> + * We check that the regmap works on this very first access,
> + * but as this is an MMIO-backed regmap, subsequent regmap
> + * access is not going to fail and we skip error checks from
> + * this point.
> + */
> + ret = ep93xx_regmap_read(EP93XX_SYSCON_CHIPID, &value);
> + if (ret || (value & 0xffff) != EP93XX_SYSCON_CHIPID_ID) {
> + pr_err("failed to read global status register\n");
> + return;
> + }
> +
> + hw = clk_hw_register_fixed_rate(NULL, "xtali", NULL, 0, EP93XX_EXT_CLK_RATE);

Maybe this can be registered from DT itself.

> + clk_hw_register_clkdev(hw, NULL, "xtali");
> +
> + /* Determine the bootloader configured pll1 rate */
> + ep93xx_regmap_read(EP93XX_SYSCON_CLKSET1, &value);
> + if (!(value & EP93XX_SYSCON_CLKSET1_NBYP1))
> + clk_pll1_rate = EP93XX_EXT_CLK_RATE;
> + else
> + clk_pll1_rate = calc_pll_rate(EP93XX_EXT_CLK_RATE, value);
> +
> + hw = clk_hw_register_fixed_rate(NULL, "pll1", "xtali", 0, clk_pll1_rate);

And maybe this can be registered from wherever the regmap is created?

> + clk_hw_register_clkdev(hw, NULL, "pll1");

Do you need to use clkdev?

> + ep93xx_clk_data->hws[EP93XX_CLK_PLL1] = hw;
> +
> + /* Initialize the pll1 derived clocks */
> + clk_f_div = fclk_divisors[(value >> 25) & 0x7];
> + clk_h_div = hclk_divisors[(value >> 20) & 0x7];
> + clk_p_div = pclk_divisors[(value >> 18) & 0x3];
> +
> + hw = clk_hw_register_fixed_factor(NULL, "fclk", "pll1", 0, 1, clk_f_div);
> + clk_f_rate = clk_get_rate(hw->clk);
> + ep93xx_clk_data->hws[EP93XX_CLK_FCLK] = hw;
> + hw = clk_hw_register_fixed_factor(NULL, "hclk", "pll1", 0, 1, clk_h_div);
> + clk_h_rate = clk_get_rate(hw->clk);
> + ep93xx_clk_data->hws[EP93XX_CLK_HCLK] = hw;
> + hw = clk_hw_register_fixed_factor(NULL, "pclk", "hclk", 0, 1, clk_p_div);
> + clk_p_rate = clk_get_rate(hw->clk);
> + ep93xx_clk_data->hws[EP93XX_CLK_PCLK] = hw;

Try to not use clk consumer APIs from a clk provider. In this case, it
looks like some debug message, so just drop these clk_get_rate() calls
and the associated printks.

> +
> + clk_hw_register_clkdev(hw, "apb_pclk", NULL);
> +
> + ep93xx_dma_clock_init(base);

Does some interrupt or timer driver use dma? Why are these registered
early?

> +
> + /* Determine the bootloader configured pll2 rate */
> + ep93xx_regmap_read(EP93XX_SYSCON_CLKSET2, &value);
> + if (!(value & EP93XX_SYSCON_CLKSET2_NBYP2))
> + clk_pll2_rate = EP93XX_EXT_CLK_RATE;
> + else if (value & EP93XX_SYSCON_CLKSET2_PLL2_EN)
> + clk_pll2_rate = calc_pll_rate(EP93XX_EXT_CLK_RATE, value);
> + else
> + clk_pll2_rate = 0;
> +
> + hw = clk_hw_register_fixed_rate(NULL, "pll2", "xtali", 0, clk_pll2_rate);

Instead of registering a fixed rate clk here you can implemented
recalc_rate and look at the regmap value.

> + ep93xx_clk_data->hws[EP93XX_CLK_PLL2] = hw;
> + clk_hw_register_clkdev(hw, NULL, "pll2");
> +
> + ep93xx_clk_data->num = EP93XX_NUM_CLKS;
> + of_clk_add_hw_provider(np, of_clk_hw_onecell_get, ep93xx_clk_data);
> +
> + pr_info("PLL1 running at %ld MHz, PLL2 at %ld MHz\n",
> + clk_pll1_rate / 1000000, clk_pll2_rate / 1000000);
> + pr_info("FCLK %ld MHz, HCLK %ld MHz, PCLK %ld MHz\n",
> + clk_f_rate / 1000000, clk_h_rate / 1000000,
> + clk_p_rate / 1000000);
> +}
> +CLK_OF_DECLARE_DRIVER(ep93xx_cc, "cirrus,ep9301-syscon", ep93xx_clock_init);

2023-05-15 10:37:09

by Nikita Shubin

Subject: Re: [PATCH 12/43] clk: ep93xx: add DT support for Cirrus EP93xx

Hello Stephen!

Thank you for your review.

Acknowledged all remarks, however, i didn't fully understood some of
the requirements:

> And maybe this can be registered from wherever the regmap is created?

Are you suggesting that all clock from init, i.e. "pll1", "pll2",
"hclk", "fclk", "pclk"

Should be moved to SoC driver instead:

https://lore.kernel.org/all/[email protected]/

?

> Does some interrupt or timer driver use dma? Why are these registered
> early?

ep93xx_dma uses subsys_initcall(ep93xx_dma_module_init)

https://elixir.bootlin.com/linux/v6.3.2/source/drivers/dma/ep93xx_dma.c#L1430

I can move clk to using arch_initcall and move all stuff to probe, i
think...

> Why is this in arm/ directory? Isn't it a clock controller?

ep93xx clocks, reboot, pinctrl use syscon regmap and some special
functions from SoC, i.e. "Syscon Software Lock Register" - so we are
able to write to some registers only after writing some magik value
there.

So all above use regmap from SoC.

Should make a separate clock controller like one i did for pinctrl ?
Then it should look like:

syscon@80930000 {
compatible = "cirrus,ep9301-syscon",
"syscon", "simple-mfd";
reg = <0x80930000 0x1000>;
#reset-cells = <1>;

clocks {
xtali: oscillator {
compatible = "fixed-clock";
#clock-cells = <0>;
clock-frequency = <14745600>;
};
};

clock-controller {
#clock-cells = <1>;
compatible = "cirrus,ep9301-clk";
clocks = <&xtali>;
};
};

Or even simply:

clocks {
xtali: oscillator {
compatible = "fixed-clock";
#clock-cells = <0>;
clock-frequency = <14745600>;
};
};

clock-controller {
#clock-cells = <1>;
compatible = "cirrus,ep9301-clk";
clocks = <&xtali>;
};

On Fri, 2023-04-28 at 17:58 -0700, Stephen Boyd wrote:
> Quoting Nikita Shubin (2023-04-24 05:34:28)
> > This is a rewrite of EP93xx timer driver in
> > arch/arm/mach-ep93xx/clock.c trying to do everything
> > the device tree way:
> >
> > - convert to syscon driver
> > - provide clock acces via of
>
> But clkdev is still around?
>
> >
> > Co-developed-by: Alexander Sverdlin <[email protected]>
> > Signed-off-by: Alexander Sverdlin <[email protected]>
> > Signed-off-by: Nikita Shubin <[email protected]>
> > ---
> >
> > Notes:
> >     rfc->v0
> >     Linus Walleij:
> >     - renamed all __underscore
> >    
> >     Alexander Sverdlin:
> >     - "Logick" -> "Logic"
> >    
> >     Changes by Alexander Sverdlin:
> >     - remove pr_info
> >     - DIV_ROUND_UP_ULL -> DIV_ROUND_CLOSEST
> >     - fix zeroing bitfield in ep93xx_div_set_rate
> >     - add sanity check for EP93XX_SYSCON_CHIPID_ID
> >     - use bit index for DMA clock's
> >     - ep93xx_clk_register_gate() takes bit index, not mask
> >     - remove redundant define
> >     - use DIV_ROUND_CLOSEST() everywhere to achieve frequencies
> > closer to those
> >       requested
> >     - Add the forgotten configuration from the deleted
> >       arch/arm/mach-ep93xx/core.c
> >
> >  drivers/clk/Kconfig      |   8 +
> >  drivers/clk/Makefile     |   1 +
> >  drivers/clk/clk-ep93xx.c | 880
> > +++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 889 insertions(+)
> >  create mode 100644 drivers/clk/clk-ep93xx.c
> >
> > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> > index 1eef05bb1f99..d3ad975b688e 100644
> > --- a/drivers/clk/Kconfig
> > +++ b/drivers/clk/Kconfig
> > @@ -436,6 +436,14 @@ config COMMON_CLK_K210
> >         help
> >           Support for the Canaan Kendryte K210 RISC-V SoC clocks.
> >  
> > +config COMMON_CLK_EP93XX
>
> Keep this sorted based on Kconfig symbol.
>
> > +       bool "Clock driver for Cirrus Logic ep93xx SoC"
> > +       depends on ARCH_EP93XX || COMPILE_TEST
> > +       select MFD_SYSCON
> > +       select REGMAP
> > +       help
> > +        This driver supports the SoC clocks on the Cirrus Logic
> > ep93xx.
> > +
> >  source "drivers/clk/actions/Kconfig"
> >  source "drivers/clk/analogbits/Kconfig"
> >  source "drivers/clk/baikal-t1/Kconfig"
> > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> > index e3ca0d058a25..deec25ffd004 100644
> > --- a/drivers/clk/Makefile
> > +++ b/drivers/clk/Makefile
> > @@ -29,6 +29,7 @@ obj-$(CONFIG_COMMON_CLK_CDCE706)      += clk-
> > cdce706.o
> >  obj-$(CONFIG_COMMON_CLK_CDCE925)       += clk-cdce925.o
> >  obj-$(CONFIG_ARCH_CLPS711X)            += clk-clps711x.o
> >  obj-$(CONFIG_COMMON_CLK_CS2000_CP)     += clk-cs2000-cp.o
> > +obj-$(CONFIG_COMMON_CLK_EP93XX)                += clk-ep93xx.o
> >  obj-$(CONFIG_ARCH_SPARX5)              += clk-sparx5.o
> >  obj-$(CONFIG_COMMON_CLK_EN7523)                += clk-en7523.o
> >  obj-$(CONFIG_COMMON_CLK_FIXED_MMIO)    += clk-fixed-mmio.o
> > diff --git a/drivers/clk/clk-ep93xx.c b/drivers/clk/clk-ep93xx.c
> > new file mode 100644
> > index 000000000000..e5913fc4c39b
> > --- /dev/null
> > +++ b/drivers/clk/clk-ep93xx.c
> > @@ -0,0 +1,880 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Clock control for Cirrus EP93xx chips.
> > + * Copyright (C) 2021 Nikita Shubin <[email protected]>
> > + *
> > + * Based on a rewrite of arch/arm/mach-ep93xx/clock.c:
> > + * Copyright (C) 2006 Lennert Buytenhek <[email protected]>
> > + */
> > +#define pr_fmt(fmt) "ep93xx " KBUILD_MODNAME ": " fmt
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/clk.h>
> > +#include <linux/err.h>
> > +#include <linux/module.h>
> > +#include <linux/string.h>
> > +#include <linux/io.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/clkdev.h>
> > +#include <linux/clk-provider.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> > +#include <linux/soc/cirrus/ep93xx.h>
> > +#include <dt-bindings/clock/cirrus,ep93xx-clock.h>
> > +
> > +#include <asm/div64.h>
> > +
> > +#define EP93XX_EXT_CLK_RATE            14745600
> > +#define EP93XX_EXT_RTC_RATE            32768
> > +
> > +#define EP93XX_SYSCON_POWER_STATE      0x00
> > +#define EP93XX_SYSCON_PWRCNT           0x04
> > +#define EP93XX_SYSCON_PWRCNT_UARTBAUD  BIT(29)
> > +#define EP93XX_SYSCON_PWRCNT_USH_EN    28
> > +#define EP93XX_SYSCON_PWRCNT_DMA_M2M1  27
> > +#define EP93XX_SYSCON_PWRCNT_DMA_M2M0  26
> > +#define EP93XX_SYSCON_PWRCNT_DMA_M2P8  25
> > +#define EP93XX_SYSCON_PWRCNT_DMA_M2P9  24
> > +#define EP93XX_SYSCON_PWRCNT_DMA_M2P6  23
> > +#define EP93XX_SYSCON_PWRCNT_DMA_M2P7  22
> > +#define EP93XX_SYSCON_PWRCNT_DMA_M2P4  21
> > +#define EP93XX_SYSCON_PWRCNT_DMA_M2P5  20
> > +#define EP93XX_SYSCON_PWRCNT_DMA_M2P2  19
> > +#define EP93XX_SYSCON_PWRCNT_DMA_M2P3  18
> > +#define EP93XX_SYSCON_PWRCNT_DMA_M2P0  17
> > +#define EP93XX_SYSCON_PWRCNT_DMA_M2P1  16
> > +#define EP93XX_SYSCON_CLKSET1          0x20
> > +#define EP93XX_SYSCON_CLKSET1_NBYP1    BIT(23)
> > +#define EP93XX_SYSCON_CLKSET2          0x24
> > +#define EP93XX_SYSCON_CLKSET2_NBYP2    BIT(19)
> > +#define EP93XX_SYSCON_CLKSET2_PLL2_EN  BIT(18)
> > +#define EP93XX_SYSCON_DEVCFG           0x80
> > +#define EP93XX_SYSCON_DEVCFG_U3EN      24
> > +#define EP93XX_SYSCON_DEVCFG_U2EN      20
> > +#define EP93XX_SYSCON_DEVCFG_U1EN      18
> > +#define EP93XX_SYSCON_VIDCLKDIV                0x84
> > +#define EP93XX_SYSCON_CLKDIV_ENABLE    15
> > +#define EP93XX_SYSCON_CLKDIV_ESEL      BIT(14)
> > +#define EP93XX_SYSCON_CLKDIV_PSEL      BIT(13)
> > +#define EP93XX_SYSCON_CLKDIV_PDIV_SHIFT        8
> > +#define EP93XX_SYSCON_I2SCLKDIV                0x8c
> > +#define EP93XX_SYSCON_I2SCLKDIV_SENA   31
> > +#define EP93XX_SYSCON_I2SCLKDIV_ORIDE  BIT(29)
> > +#define EP93XX_SYSCON_I2SCLKDIV_SPOL   BIT(19)
> > +#define EP93XX_I2SCLKDIV_SDIV          (1 << 16)
> > +#define EP93XX_I2SCLKDIV_LRDIV32       (0 << 17)
> > +#define EP93XX_I2SCLKDIV_LRDIV64       (1 << 17)
> > +#define EP93XX_I2SCLKDIV_LRDIV128      (2 << 17)
> > +#define EP93XX_I2SCLKDIV_LRDIV_MASK    (3 << 17)
> > +#define EP93XX_SYSCON_KEYTCHCLKDIV     0x90
> > +#define EP93XX_SYSCON_KEYTCHCLKDIV_TSEN        31
> > +#define EP93XX_SYSCON_KEYTCHCLKDIV_ADIV        16
> > +#define EP93XX_SYSCON_KEYTCHCLKDIV_KEN 15
> > +#define EP93XX_SYSCON_KEYTCHCLKDIV_KDIV        0
> > +#define EP93XX_SYSCON_CHIPID           0x94
> > +#define EP93XX_SYSCON_CHIPID_ID                0x9213
> > +
> > +static DEFINE_SPINLOCK(clk_lock);
>
> This name is not unique. Please make it unique by prefixing with
> ep93xx.
>
> > +static struct regmap *ep93xx_map;
>
> Do these need to be static globals? Doing a runtime allocation
> reduces
> the size overhead for kernel images that don't have this hardware but
> compile in this driver, i.e. distros.
>
> > +#define ep93xx_regmap_read(reg, val) regmap_read(ep93xx_map, reg,
> > val)
> > +
> > +/* Keeps track of all clocks */
> > +static struct clk_hw_onecell_data *ep93xx_clk_data;
>
> Same static global question.
>
> > +
> > +static char fclk_divisors[] = { 1, 2, 4, 8, 16, 1, 1, 1 };
> > +static char hclk_divisors[] = { 1, 2, 4, 5, 6, 8, 16, 32 };
> > +static char pclk_divisors[] = { 1, 2, 4, 8 };
> > +
> > +static char adc_divisors[] = { 16, 4 };
> > +static char sclk_divisors[] = { 2, 4 };
> > +static char lrclk_divisors[] = { 32, 64, 128 };
>
> const?
>
> > +
> > +static const char * const mux_parents[] = {
> > +       "xtali",
> > +       "pll1",
> > +       "pll2"
> > +};
> > +
> > +/*
> > + * PLL rate = 14.7456 MHz * (X1FBD + 1) * (X2FBD + 1) / (X2IPD +
> > 1) / 2^PS
> > + */
> > +static unsigned long calc_pll_rate(unsigned long long rate, u32
> > config_word)
>
> u64 rate
>
> > +{
> > +       int i;
> > +
> > +       rate *= ((config_word >> 11) & 0x1f) + 1;               /*
> > X1FBD */
> > +       rate *= ((config_word >> 5) & 0x3f) + 1;                /*
> > X2FBD */
> > +       do_div(rate, (config_word & 0x1f) + 1);                 /*
> > X2IPD */
> > +       for (i = 0; i < ((config_word >> 16) & 3); i++)         /*
> > PS */
> > +               rate >>= 1;
> > +
> > +       return (unsigned long)rate;
>
> Drop useless cast.
>
> > +}
> > +
> > +struct clk_psc {
> > +       struct clk_hw hw;
> > +       unsigned int reg;
> > +       u8 bit_idx;
> > +       u32 mask;
> > +       u8 shift;
> > +       u8 width;
> > +       char *div;
> > +       u8 num_div;
> > +       spinlock_t *lock;
> > +};
> > +
> > +#define to_clk_psc(_hw) container_of(_hw, struct clk_psc, hw)
> > +
> > +static int ep93xx_clk_is_enabled(struct clk_hw *hw)
> > +{
> > +       struct clk_psc *psc = to_clk_psc(hw);
> > +       u32 val;
> > +
> > +       ep93xx_regmap_read(psc->reg, &val);
> > +
> > +       return (val & BIT(psc->bit_idx)) ? 1 : 0;
> > +}
> > +
> > +static int ep93xx_clk_enable(struct clk_hw *hw)
> > +{
> > +       struct clk_psc *psc = to_clk_psc(hw);
> > +       unsigned long flags = 0;
> > +       u32 val;
> > +
> > +       if (psc->lock)
> > +               spin_lock_irqsave(psc->lock, flags);
> > +
> > +       ep93xx_regmap_read(psc->reg, &val);
> > +       val |= BIT(psc->bit_idx);
> > +
> > +       ep93xx_syscon_swlocked_write(val, psc->reg);
> > +
> > +       if (psc->lock)
> > +               spin_unlock_irqrestore(psc->lock, flags);
> > +
> > +       return 0;
> > +}
> > +
> > +static void ep93xx_clk_disable(struct clk_hw *hw)
> > +{
> > +       struct clk_psc *psc = to_clk_psc(hw);
> > +       unsigned long flags = 0;
> > +       u32 val;
> > +
> > +       if (psc->lock)
> > +               spin_lock_irqsave(psc->lock, flags);
> > +
> > +       ep93xx_regmap_read(psc->reg, &val);
> > +       val &= ~BIT(psc->bit_idx);
> > +
> > +       ep93xx_syscon_swlocked_write(val, psc->reg);
> > +
> > +       if (psc->lock)
> > +               spin_unlock_irqrestore(psc->lock, flags);
> > +}
> > +
> > +static const struct clk_ops clk_ep93xx_gate_ops = {
> > +       .enable = ep93xx_clk_enable,
> > +       .disable = ep93xx_clk_disable,
> > +       .is_enabled = ep93xx_clk_is_enabled,
> > +};
> > +
> > +static struct clk_hw *ep93xx_clk_register_gate(const char *name,
> > +                                   const char *parent_name,
> > +                                   unsigned int reg,
> > +                                   u8 bit_idx)
> > +{
> > +       struct clk_init_data init;
> > +       struct clk_psc *psc;
> > +       struct clk *clk;
> > +
> > +       psc = kzalloc(sizeof(*psc), GFP_KERNEL);
> > +       if (!psc)
> > +               return ERR_PTR(-ENOMEM);
> > +
> > +       init.name = name;
> > +       init.ops = &clk_ep93xx_gate_ops;
> > +       init.flags = CLK_SET_RATE_PARENT;
> > +       init.parent_names = (parent_name ? &parent_name : NULL);
> > +       init.num_parents = (parent_name ? 1 : 0);
>
> Please use parent_data or parent_hw instead.
>
> > +
> > +       psc->reg = reg;
> > +       psc->bit_idx = bit_idx;
> > +       psc->hw.init = &init;
> > +       psc->lock = &clk_lock;
> > +
> > +       clk = clk_register(NULL, &psc->hw);
> > +       if (IS_ERR(clk)) {
> > +               kfree(psc);
> > +               return ERR_CAST(clk);
> > +       }
> > +
> > +       return &psc->hw;
> > +}
> > +
> > +static u8 ep93xx_mux_get_parent(struct clk_hw *hw)
> > +{
> > +       struct clk_psc *psc = to_clk_psc(hw);
> > +       u32 val;
> > +
> > +       ep93xx_regmap_read(psc->reg, &val);
> > +       if (!(val & EP93XX_SYSCON_CLKDIV_ESEL))
> > +               return 0;
> > +
> > +       if (!(val & EP93XX_SYSCON_CLKDIV_PSEL))
> > +               return 1;
> > +
> > +       return 2;
> > +}
> > +
> > +static int ep93xx_mux_set_parent_lock(struct clk_hw *hw, u8 index)
> > +{
> > +       struct clk_psc *psc = to_clk_psc(hw);
> > +       unsigned long flags = 0;
> > +       u32 val;
> > +
> > +       if (index >= ARRAY_SIZE(mux_parents))
> > +               return -EINVAL;
> > +
> > +       if (psc->lock)
>
> Can you simply lock all the time?
>
> > +               spin_lock_irqsave(psc->lock, flags);
> > +
> > +       ep93xx_regmap_read(psc->reg, &val);
> > +       val &= ~(EP93XX_SYSCON_CLKDIV_ESEL |
> > EP93XX_SYSCON_CLKDIV_PSEL);
> > +
> > +       if (index != 0) {
> > +               val |= EP93XX_SYSCON_CLKDIV_ESEL;
> > +               val |= (index - 1) ? EP93XX_SYSCON_CLKDIV_PSEL : 0;
> > +       }
> > +
> > +       ep93xx_syscon_swlocked_write(val, psc->reg);
> > +
> > +       if (psc->lock)
> > +               spin_unlock_irqrestore(psc->lock, flags);
> > +
> > +       return 0;
> > +}
> > +
> > +static bool is_best(unsigned long rate, unsigned long now,
> > +                    unsigned long best)
> > +{
> > +       return abs(rate - now) < abs(rate - best);
> > +}
> > +
> > +static int ep93xx_mux_determine_rate(struct clk_hw *hw,
> > +                               struct clk_rate_request *req)
> > +{
> > +       unsigned long rate = req->rate;
> > +       struct clk *parent_best = NULL;
> > +       unsigned long parent_rate;
> > +       unsigned long best_rate = 0, actual_rate, mclk_rate;
> > +       unsigned long parent_rate_best;
> > +       int div, pdiv;
> > +       int i;
> > +
> > +       /*
> > +        * Try the two pll's and the external clock
> > +        * Because the valid predividers are 2, 2.5 and 3, we
> > multiply
> > +        * all the clocks by 2 to avoid floating point math.
> > +        *
> > +        * This is based on the algorithm in the ep93xx raster
> > guide:
> > +        * http://be-a-maverick.com/en/pubs/appNote/AN269REV1.pdf
> > +        *
> > +        */
> > +       for (i = 0; i < ARRAY_SIZE(mux_parents); i++) {
> > +               struct clk *parent = clk_get_sys(mux_parents[i],
> > NULL);
>
> The possible parents can be had with clk_hw_get_parent_by_index().
> Use
> that so you can stop leaking .
>
> > +
> > +               parent_rate = clk_get_rate(parent);
> > +               mclk_rate = parent_rate * 2;
>
> The parent_rate is available via clk_hw_get_rate() or by looking at
> the
> clk_rate_request structure 'req' if the parent is the current parent,
> or
> by calling clk_hw_round_rate() or the poorly named
> __clk_determine_rate(). In this case it looks like the parents aren't
> expected to change rate, so clk_hw_get_rate() will work.
>
> > +
> > +               /* Try each predivider value */
> > +               for (pdiv = 4; pdiv <= 6; pdiv++) {
> > +                       div = DIV_ROUND_CLOSEST(mclk_rate, rate *
> > pdiv);
> > +                       if (div < 1 || div > 127)
> > +                               continue;
> > +
> > +                       actual_rate = DIV_ROUND_CLOSEST(mclk_rate,
> > pdiv * div);
> > +
> > +                       if (is_best(rate, actual_rate, best_rate))
> > {
> > +                               best_rate = actual_rate;
> > +                               parent_rate_best = parent_rate;
> > +                               parent_best = parent;
> > +                       }
> > +               }
> > +       }
> > +
> > +       if (!parent_best)
> > +               return -EINVAL;
> > +
> > +       req->best_parent_rate = parent_rate_best;
> > +       req->best_parent_hw = __clk_get_hw(parent_best);
> > +       req->rate = best_rate;
> > +
> > +       return 0;
> > +}
> > +
> > +static unsigned long ep93xx_ddiv_recalc_rate(struct clk_hw *hw,
> > +                                               unsigned long
> > parent_rate)
> > +{
> > +       struct clk_psc *psc = to_clk_psc(hw);
> > +       unsigned long rate = 0;
> > +       u32 val;
> > +       int pdiv, div;
> > +
> > +       ep93xx_regmap_read(psc->reg, &val);
> > +       pdiv = ((val >> EP93XX_SYSCON_CLKDIV_PDIV_SHIFT) & 0x03);
> > +       div = val & 0x7f;
> > +       if (div > 0)
> > +               rate = DIV_ROUND_CLOSEST(parent_rate * 2, (pdiv +
> > 3) * div);
> > +
> > +       return rate;
> > +}
> > +
> > +static int ep93xx_ddiv_set_rate(struct clk_hw *hw, unsigned long
> > rate,
> > +                               unsigned long parent_rate)
> > +{
> > +       struct clk_psc *psc = to_clk_psc(hw);
> > +       int pdiv, div, npdiv, ndiv;
> > +       unsigned long actual_rate, mclk_rate, rate_err = -1;
> > +       int found = 0;
> > +       u32 val;
> > +
> > +       ep93xx_regmap_read(psc->reg, &val);
> > +       mclk_rate = parent_rate * 2;
> > +
> > +       for (pdiv = 4; pdiv <= 6; pdiv++) {
> > +               div = DIV_ROUND_CLOSEST(mclk_rate, rate * pdiv);
> > +               if (div < 1 || div > 127)
> > +                       continue;
> > +
> > +               actual_rate = DIV_ROUND_CLOSEST(mclk_rate, pdiv *
> > div);
> > +
> > +               if (!found || abs(actual_rate - rate) < rate_err) {
> > +                       npdiv = pdiv - 3;
> > +                       ndiv = div;
> > +                       rate_err = abs(actual_rate - rate);
> > +                       found = 1;
> > +               }
> > +       }
> > +
> > +       if (!found)
> > +               return -EINVAL;
> > +
> > +       /* Clear old dividers */
> > +       val &= ~0x37f;
> > +
> > +       /* Set the new pdiv and div bits for the new clock rate */
> > +       val |= (npdiv << EP93XX_SYSCON_CLKDIV_PDIV_SHIFT) | ndiv;
> > +
> > +       ep93xx_syscon_swlocked_write(val, psc->reg);
> > +
> > +       return 0;
> > +}
> > +
> > +static const struct clk_ops clk_ddiv_ops = {
> > +       .enable = ep93xx_clk_enable,
> > +       .disable = ep93xx_clk_disable,
> > +       .is_enabled = ep93xx_clk_is_enabled,
> > +       .get_parent = ep93xx_mux_get_parent,
> > +       .set_parent = ep93xx_mux_set_parent_lock,
> > +       .determine_rate = ep93xx_mux_determine_rate,
> > +       .recalc_rate = ep93xx_ddiv_recalc_rate,
> > +       .set_rate = ep93xx_ddiv_set_rate,
> > +};
> > +
> > +static struct clk_hw *clk_hw_register_ddiv(const char *name,
> > +                                         unsigned int reg,
> > +                                         u8 bit_idx)
> > +{
> > +       struct clk_init_data init;
> > +       struct clk_psc *psc;
> > +       struct clk *clk;
> > +
> > +       psc = kzalloc(sizeof(*psc), GFP_KERNEL);
> > +       if (!psc)
> > +               return ERR_PTR(-ENOMEM);
> > +
> > +       init.name = name;
> > +       init.ops = &clk_ddiv_ops;
> > +       init.flags = 0;
> > +       init.parent_names = mux_parents;
> > +       init.num_parents = ARRAY_SIZE(mux_parents);
> > +
> > +       psc->reg = reg;
> > +       psc->bit_idx = bit_idx;
> > +       psc->lock = &clk_lock;
> > +       psc->hw.init = &init;
> > +
> > +       clk = clk_register(NULL, &psc->hw);
> > +       if (IS_ERR(clk)) {
> > +               kfree(psc);
> > +               return ERR_CAST(clk);
> > +       }
> > +
> > +       return &psc->hw;
> > +}
> > +
> > +static unsigned long ep93xx_div_recalc_rate(struct clk_hw *hw,
> > +                                           unsigned long
> > parent_rate)
> > +{
> > +       struct clk_psc *psc = to_clk_psc(hw);
> > +       u32 val;
> > +       u8 index;
> > +
> > +       ep93xx_regmap_read(psc->reg, &val);
> > +       index = (val & psc->mask) >> psc->shift;
> > +       if (index > psc->num_div)
> > +               return 0;
> > +
> > +       return DIV_ROUND_CLOSEST(parent_rate, psc->div[index]);
> > +}
> > +
> > +static long ep93xx_div_round_rate(struct clk_hw *hw, unsigned long
> > rate,
> > +                                  unsigned long *parent_rate)
> > +{
> > +       struct clk_psc *psc = to_clk_psc(hw);
> > +       unsigned long best = 0, now;
> > +       bool assigned = false;
> > +       int i;
> > +
> > +       for (i = 0; i < psc->num_div; i++) {
> > +               if ((rate * psc->div[i]) == *parent_rate)
> > +                       return rate;
> > +
> > +               now = DIV_ROUND_CLOSEST(*parent_rate, psc->div[i]);
> > +
> > +               if (!assigned || is_best(rate, now, best))
> > +                       best = now;
> > +               assigned = true;
> > +       }
> > +
> > +       return best;
> > +}
> > +
> > +static int ep93xx_div_set_rate(struct clk_hw *hw, unsigned long
> > rate,
> > +                              unsigned long parent_rate)
> > +{
> > +       struct clk_psc *psc = to_clk_psc(hw);
> > +       u32 val;
> > +       int i;
> > +
> > +       ep93xx_regmap_read(psc->reg, &val);
> > +       val &= ~psc->mask;
> > +       for (i = 0; i < psc->num_div; i++)
> > +               if (rate == DIV_ROUND_CLOSEST(parent_rate, psc-
> > >div[i])) {
> > +                       val |= i << psc->shift;
> > +                       break;
> > +               }
> > +
> > +       if (i == psc->num_div)
> > +               return -EINVAL;
> > +
> > +       ep93xx_syscon_swlocked_write(val, psc->reg);
> > +
> > +       return 0;
> > +}
> > +
> > +static const struct clk_ops ep93xx_div_ops = {
> > +       .enable = ep93xx_clk_enable,
> > +       .disable = ep93xx_clk_disable,
> > +       .is_enabled = ep93xx_clk_is_enabled,
> > +       .recalc_rate = ep93xx_div_recalc_rate,
> > +       .round_rate = ep93xx_div_round_rate,
> > +       .set_rate = ep93xx_div_set_rate,
> > +};
> > +
> > +static struct clk_hw *clk_hw_register_div(const char *name,
> > +                                         const char *parent_name,
> > +                                         unsigned int reg,
> > +                                         u8 enable_bit,
> > +                                         u8 shift,
> > +                                         u8 width,
> > +                                         char *clk_divisors,
> > +                                         u8 num_div)
> > +{
> > +       struct clk_init_data init;
> > +       struct clk_psc *psc;
> > +       struct clk *clk;
> > +
> > +       psc = kzalloc(sizeof(*psc), GFP_KERNEL);
> > +       if (!psc)
> > +               return ERR_PTR(-ENOMEM);
> > +
> > +       init.name = name;
> > +       init.ops = &ep93xx_div_ops;
> > +       init.flags = 0;
> > +       init.parent_names = (parent_name ? &parent_name : NULL);
> > +       init.num_parents = 1;
> > +
> > +       psc->reg = reg;
> > +       psc->bit_idx = enable_bit;
> > +       psc->mask = GENMASK(shift + width - 1, shift);
> > +       psc->shift = shift;
> > +       psc->div = clk_divisors;
> > +       psc->num_div = num_div;
> > +       psc->lock = &clk_lock;
> > +       psc->hw.init = &init;
> > +
> > +       clk = clk_register(NULL, &psc->hw);
>
> Use clk_hw_register()
>
> > +       if (IS_ERR(clk)) {
> > +               kfree(psc);
> > +               return ERR_CAST(clk);
> > +       }
> > +
> > +       return &psc->hw;
> > +}
> > +
> > +struct ep93xx_gate {
> > +       unsigned int idx;
> > +       unsigned int bit;
> > +       const char *dev_id;
> > +       const char *con_id;
> > +};
> > +
> > +static struct ep93xx_gate ep93xx_uarts[] = {
> > +       {EP93XX_CLK_UART1, EP93XX_SYSCON_DEVCFG_U1EN, "apb:uart1",
> > NULL},
> > +       {EP93XX_CLK_UART2, EP93XX_SYSCON_DEVCFG_U2EN, "apb:uart2",
> > NULL},
> > +       {EP93XX_CLK_UART3, EP93XX_SYSCON_DEVCFG_U3EN, "apb:uart3",
> > NULL},
> > +};
> > +
> > +static void ep93xx_uart_clock_init(void)
> > +{
> > +       unsigned int i;
> > +       struct clk_hw *hw;
> > +       u32 val;
> > +       unsigned int clk_uart_div;
> > +
> > +       ep93xx_regmap_read(EP93XX_SYSCON_PWRCNT, &val);
> > +       if (val & EP93XX_SYSCON_PWRCNT_UARTBAUD)
> > +               clk_uart_div = 1;
> > +       else
> > +               clk_uart_div = 2;
> > +
> > +       hw = clk_hw_register_fixed_factor(NULL, "uart", "xtali", 0,
> > 1, clk_uart_div);
> > +       ep93xx_clk_data->hws[EP93XX_CLK_UART] = hw;
> > +
> > +       /* parenting uart gate clocks to uart clock */
> > +       for (i = 0; i < ARRAY_SIZE(ep93xx_uarts); i++) {
> > +               hw =
> > ep93xx_clk_register_gate(ep93xx_uarts[i].dev_id,
> > +                                       "uart",
> > +                                       EP93XX_SYSCON_DEVCFG,
> > +                                       ep93xx_uarts[i].bit);
> > +
> > +               ep93xx_clk_data->hws[ep93xx_uarts[i].idx] = hw;
> > +       }
> > +}
> > +
> > +static struct ep93xx_gate ep93xx_dmas[] = {
>
> const?
>
> > +       {EP93XX_CLK_M2P0, EP93XX_SYSCON_PWRCNT_DMA_M2P0, NULL,
> > "m2p0"},
>
> Put space after { and before }
>
> > +       {EP93XX_CLK_M2P1, EP93XX_SYSCON_PWRCNT_DMA_M2P1, NULL,
> > "m2p1"},
> > +       {EP93XX_CLK_M2P2, EP93XX_SYSCON_PWRCNT_DMA_M2P2, NULL,
> > "m2p2"},
> > +       {EP93XX_CLK_M2P3, EP93XX_SYSCON_PWRCNT_DMA_M2P3, NULL,
> > "m2p3"},
> > +       {EP93XX_CLK_M2P4, EP93XX_SYSCON_PWRCNT_DMA_M2P4, NULL,
> > "m2p4"},
> > +       {EP93XX_CLK_M2P5, EP93XX_SYSCON_PWRCNT_DMA_M2P5, NULL,
> > "m2p5"},
> > +       {EP93XX_CLK_M2P6, EP93XX_SYSCON_PWRCNT_DMA_M2P6, NULL,
> > "m2p6"},
> > +       {EP93XX_CLK_M2P7, EP93XX_SYSCON_PWRCNT_DMA_M2P7, NULL,
> > "m2p7"},
> > +       {EP93XX_CLK_M2P8, EP93XX_SYSCON_PWRCNT_DMA_M2P8, NULL,
> > "m2p8"},
> > +       {EP93XX_CLK_M2P9, EP93XX_SYSCON_PWRCNT_DMA_M2P9, NULL,
> > "m2p9"},
> > +       {EP93XX_CLK_M2M0, EP93XX_SYSCON_PWRCNT_DMA_M2M0, NULL,
> > "m2m0"},
> > +       {EP93XX_CLK_M2M1, EP93XX_SYSCON_PWRCNT_DMA_M2M1, NULL,
> > "m2m1"},
> > +};
> > +
> > +static void ep93xx_dma_clock_init(void __iomem *base)
> > +{
> > +       int i;
> > +       struct clk_hw *hw;
> > +       int ret;
> > +
> > +       for (i = 0; i < ARRAY_SIZE(ep93xx_dmas); i++) {
> > +               hw = clk_hw_register_gate(NULL,
> > ep93xx_dmas[i].con_id,
> > +                                       "hclk", 0,
> > +                                       base +
> > EP93XX_SYSCON_PWRCNT,
> > +                                       ep93xx_dmas[i].bit,
> > +                                       0,
> > +                                       &clk_lock);
> > +
> > +               ret = clk_hw_register_clkdev(hw,
> > ep93xx_dmas[i].con_id, NULL);
> > +               if (ret) {
> > +                       pr_err("%s: failed to register lookup
> > %s\n",
> > +                              __func__, ep93xx_dmas[i].con_id);
> > +                       continue;
> > +               }
> > +
> > +               ep93xx_clk_data->hws[ep93xx_dmas[i].idx] = hw;
> > +       }
> > +}
> > +
> > +static int ep93xx_clk_probe(struct platform_device *pdev)
> > +{
> > +       void __iomem *base;
> > +       unsigned int clk_usb_div;
> > +       unsigned long clk_spi_div;
> > +       struct clk_hw *hw;
> > +       struct device *dev = &pdev->dev;
> > +       u32 value;
> > +       struct resource *res;
> > +
> > +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +       base = devm_ioremap_resource(dev, res);
> > +       if (IS_ERR(base))
> > +               return PTR_ERR(base);
> > +
> > +       ep93xx_regmap_read(EP93XX_SYSCON_CLKSET2, &value);
> > +       clk_usb_div = (((value >> 28) & 0xf) + 1);
> > +       hw = clk_hw_register_fixed_factor(NULL, "usb_clk", "pll2",
> > 0, 1, clk_usb_div);
> > +       hw = clk_hw_register_gate(NULL, "ohci-platform",
> > +                               "usb_clk", 0,
> > +                               base + EP93XX_SYSCON_PWRCNT,
> > +                               EP93XX_SYSCON_PWRCNT_USH_EN,
> > +                               0,
> > +                               &clk_lock);
> > +       clk_hw_register_clkdev(hw, NULL, "ohci-platform");
> > +       ep93xx_clk_data->hws[EP93XX_CLK_USB] = hw;
> > +
> > +       /*
> > +        * EP93xx SSP clock rate was doubled in version E2. For
> > more information
> > +        * see:
> > +        *     http://www.cirrus.com/en/pubs/appNote/AN273REV4.pdf
> > +        */
> > +       clk_spi_div = 1;
> > +       if (ep93xx_chip_revision() < EP93XX_CHIP_REV_E2)
> > +               clk_spi_div = 2;
> > +       hw = clk_hw_register_fixed_factor(NULL, "ep93xx-spi.0",
> > "xtali", 0, 1, clk_spi_div);
> > +       clk_hw_register_clkdev(hw, NULL, "ep93xx-spi.0");
> > +       ep93xx_clk_data->hws[EP93XX_CLK_SPI] = hw;
> > +
> > +       /* pwm clock */
> > +       hw = clk_hw_register_fixed_factor(NULL, "pwm_clk", "xtali",
> > 0, 1, 1);
> > +       clk_hw_register_clkdev(hw, "pwm_clk", NULL);
> > +       ep93xx_clk_data->hws[EP93XX_CLK_PWM] = hw;
> > +
> > +       ep93xx_uart_clock_init();
> > +
> > +       /* touchscreen/adc clock */
> > +       hw = clk_hw_register_div("ep93xx-adc",
> > +                               "xtali",
> > +                               EP93XX_SYSCON_KEYTCHCLKDIV,
> > +                               EP93XX_SYSCON_KEYTCHCLKDIV_TSEN,
> > +                               EP93XX_SYSCON_KEYTCHCLKDIV_ADIV,
> > +                               1,
> > +                               adc_divisors,
> > +                               ARRAY_SIZE(adc_divisors));
> > +
> > +       clk_hw_register_clkdev(hw, NULL, "ep93xx-adc");
> > +       ep93xx_clk_data->hws[EP93XX_CLK_ADC] = hw;
> > +
> > +       /* keypad clock */
> > +       hw = clk_hw_register_div("ep93xx-keypad",
> > +                               "xtali",
> > +                               EP93XX_SYSCON_KEYTCHCLKDIV,
> > +                               EP93XX_SYSCON_KEYTCHCLKDIV_KEN,
> > +                               EP93XX_SYSCON_KEYTCHCLKDIV_KDIV,
> > +                               1,
> > +                               adc_divisors,
> > +                               ARRAY_SIZE(adc_divisors));
> > +
> > +       clk_hw_register_clkdev(hw, NULL, "ep93xx-keypad");
> > +       ep93xx_clk_data->hws[EP93XX_CLK_KEYPAD] = hw;
> > +
> > +       /* On reset PDIV and VDIV is set to zero, while PDIV zero
>
> The /* is on a line by itself.
>
> > +        * means clock disable, VDIV shouldn't be zero.
> > +        * So i set both dividers to minimum.
> > +        */
> > +       /* ENA - Enable CLK divider. */
> > +       /* PDIV - 00 - Disable clock */
> > +       /* VDIV - at least 2 */
> > +       /* Check and enable video clk registers */
> > +       ep93xx_regmap_read(EP93XX_SYSCON_VIDCLKDIV, &value);
> > +       value |= (1 << EP93XX_SYSCON_CLKDIV_PDIV_SHIFT) | 2;
> > +       ep93xx_syscon_swlocked_write(value,
> > EP93XX_SYSCON_VIDCLKDIV);
> > +
> > +       /* check and enable i2s clk registers */
> > +       ep93xx_regmap_read(EP93XX_SYSCON_I2SCLKDIV, &value);
> > +       value |= (1 << EP93XX_SYSCON_CLKDIV_PDIV_SHIFT) | 2;
> > +       /*
> > +        * Override the SAI_MSTR_CLK_CFG from the I2S block and use
> > the
> > +        * I2SClkDiv Register settings. LRCLK transitions on the
> > falling SCLK
> > +        * edge.
> > +        */
> > +       value |= EP93XX_SYSCON_I2SCLKDIV_ORIDE |
> > EP93XX_SYSCON_I2SCLKDIV_SPOL;
> > +       ep93xx_syscon_swlocked_write(value,
> > EP93XX_SYSCON_I2SCLKDIV);
> > +
> > +       /* video clk */
> > +       hw = clk_hw_register_ddiv("ep93xx-fb",
> > +                               EP93XX_SYSCON_VIDCLKDIV,
> > +                               EP93XX_SYSCON_CLKDIV_ENABLE);
> > +
> > +       clk_hw_register_clkdev(hw, NULL, "ep93xx-fb");
> > +       ep93xx_clk_data->hws[EP93XX_CLK_VIDEO] = hw;
> > +
> > +       /* i2s clk */
> > +       hw = clk_hw_register_ddiv("mclk",
> > +                               EP93XX_SYSCON_I2SCLKDIV,
> > +                               EP93XX_SYSCON_CLKDIV_ENABLE);
> > +
> > +       clk_hw_register_clkdev(hw, "mclk", "ep93xx-i2s");
> > +       ep93xx_clk_data->hws[EP93XX_CLK_I2S_MCLK] = hw;
> > +
> > +       /* i2s sclk */
> > +#define EP93XX_I2SCLKDIV_SDIV_SHIFT    16
> > +#define EP93XX_I2SCLKDIV_SDIV_WIDTH    1
> > +       hw = clk_hw_register_div("sclk",
> > +                               "mclk",
> > +                               EP93XX_SYSCON_I2SCLKDIV,
> > +                               EP93XX_SYSCON_I2SCLKDIV_SENA,
> > +                               EP93XX_I2SCLKDIV_SDIV_SHIFT,
> > +                               EP93XX_I2SCLKDIV_SDIV_WIDTH,
> > +                               sclk_divisors,
> > +                               ARRAY_SIZE(sclk_divisors));
> > +
> > +       clk_hw_register_clkdev(hw, "sclk", "ep93xx-i2s");
> > +       ep93xx_clk_data->hws[EP93XX_CLK_I2S_SCLK] = hw;
> > +
> > +       /* i2s lrclk */
> > +#define EP93XX_I2SCLKDIV_LRDIV32_SHIFT 17
> > +#define EP93XX_I2SCLKDIV_LRDIV32_WIDTH 2
>
> Please just inline these defines as they're only used on place.
>
> > +       hw = clk_hw_register_div("lrclk",
> > +                               "sclk",
> > +                               EP93XX_SYSCON_I2SCLKDIV,
> > +                               EP93XX_SYSCON_I2SCLKDIV_SENA,
> > +                               EP93XX_I2SCLKDIV_LRDIV32_SHIFT,
> > +                               EP93XX_I2SCLKDIV_LRDIV32_WIDTH,
> > +                               lrclk_divisors,
> > +                               ARRAY_SIZE(lrclk_divisors));
> > +
> > +       clk_hw_register_clkdev(hw, "lrclk", "ep93xx-i2s");
> > +       ep93xx_clk_data->hws[EP93XX_CLK_I2S_LRCLK] = hw;
> > +
> > +       /* IrDa clk uses same pattern but no init code presents in
> > original clock driver */
> > +       return 0;
> > +}
> > +
> > +static const struct of_device_id ep93xx_clk_dt_ids[] = {
> > +       { .compatible = "cirrus,ep9301-syscon", },
> > +       { /* sentinel */ },
> > +};
> > +
> > +static struct platform_driver ep93xx_clk_driver = {
> > +       .probe  = ep93xx_clk_probe,
> > +       .driver = {
> > +               .name = "ep93xx-clk",
> > +               .of_match_table = ep93xx_clk_dt_ids,
> > +               .suppress_bind_attrs = true,
> > +       },
> > +};
> > +builtin_platform_driver(ep93xx_clk_driver);
> > +
> > +static void __init ep93xx_clock_init(struct device_node *np)
> > +{
> > +       void __iomem *base;
> > +       int i;
> > +       int ret;
> > +       u32 value;
> > +       struct clk_hw *hw;
> > +       unsigned long clk_pll1_rate;
> > +       unsigned long clk_f_rate;
> > +       unsigned long clk_h_rate;
> > +       unsigned long clk_p_rate;
> > +       unsigned long clk_pll2_rate;
> > +       unsigned int clk_f_div;
> > +       unsigned int clk_h_div;
> > +       unsigned int clk_p_div;
> > +
> > +       ep93xx_clk_data = kzalloc(struct_size(ep93xx_clk_data, hws,
> > +                               EP93XX_NUM_CLKS),
> > +                               GFP_KERNEL);
> > +
> > +       if (!ep93xx_clk_data)
> > +               return;
> > +
> > +       /*
> > +        * This way all clock fetched before the platform device
> > probes,
> > +        * except those we assign here for early use, will be
> > deferred.
> > +        */
> > +       for (i = 0; i < EP93XX_NUM_CLKS; i++)
> > +               ep93xx_clk_data->hws[i] = ERR_PTR(-EPROBE_DEFER);
> > +
> > +       base = of_iomap(np, 0);
> > +       /* Remap the system controller for the exclusive register
> > */
> > +       if (IS_ERR(base)) {
> > +               pr_err("failed to map base\n");
> > +               return;
> > +       }
> > +
> > +       ep93xx_map = syscon_node_to_regmap(np);
> > +       if (IS_ERR(ep93xx_map)) {
> > +               pr_err("no syscon regmap\n");
> > +               return;
> > +       }
> > +
> > +       /*
> > +        * We check that the regmap works on this very first
> > access,
> > +        * but as this is an MMIO-backed regmap, subsequent regmap
> > +        * access is not going to fail and we skip error checks
> > from
> > +        * this point.
> > +        */
> > +       ret = ep93xx_regmap_read(EP93XX_SYSCON_CHIPID, &value);
> > +       if (ret || (value & 0xffff) != EP93XX_SYSCON_CHIPID_ID) {
> > +               pr_err("failed to read global status register\n");
> > +               return;
> > +       }
> > +
> > +       hw = clk_hw_register_fixed_rate(NULL, "xtali", NULL, 0,
> > EP93XX_EXT_CLK_RATE);
>
> Maybe this can be registered from DT itself.
>
> > +       clk_hw_register_clkdev(hw, NULL, "xtali");
> > +
> > +       /* Determine the bootloader configured pll1 rate */
> > +       ep93xx_regmap_read(EP93XX_SYSCON_CLKSET1, &value);
> > +       if (!(value & EP93XX_SYSCON_CLKSET1_NBYP1))
> > +               clk_pll1_rate = EP93XX_EXT_CLK_RATE;
> > +       else
> > +               clk_pll1_rate = calc_pll_rate(EP93XX_EXT_CLK_RATE,
> > value);
> > +
> > +       hw = clk_hw_register_fixed_rate(NULL, "pll1", "xtali", 0,
> > clk_pll1_rate);
>
> And maybe this can be registered from wherever the regmap is created?
>
> > +       clk_hw_register_clkdev(hw, NULL, "pll1");
>
> Do you need to use clkdev?
>
> > +       ep93xx_clk_data->hws[EP93XX_CLK_PLL1] = hw;
> > +
> > +       /* Initialize the pll1 derived clocks */
> > +       clk_f_div = fclk_divisors[(value >> 25) & 0x7];
> > +       clk_h_div = hclk_divisors[(value >> 20) & 0x7];
> > +       clk_p_div = pclk_divisors[(value >> 18) & 0x3];
> > +
> > +       hw = clk_hw_register_fixed_factor(NULL, "fclk", "pll1", 0,
> > 1, clk_f_div);
> > +       clk_f_rate = clk_get_rate(hw->clk);
> > +       ep93xx_clk_data->hws[EP93XX_CLK_FCLK] = hw;
> > +       hw = clk_hw_register_fixed_factor(NULL, "hclk", "pll1", 0,
> > 1, clk_h_div);
> > +       clk_h_rate = clk_get_rate(hw->clk);
> > +       ep93xx_clk_data->hws[EP93XX_CLK_HCLK] = hw;
> > +       hw = clk_hw_register_fixed_factor(NULL, "pclk", "hclk", 0,
> > 1, clk_p_div);
> > +       clk_p_rate = clk_get_rate(hw->clk);
> > +       ep93xx_clk_data->hws[EP93XX_CLK_PCLK] = hw;
>
> Try to not use clk consumer APIs from a clk provider. In this case,
> it
> looks like some debug message, so just drop these clk_get_rate()
> calls
> and the associated printks.
>
> > +
> > +       clk_hw_register_clkdev(hw, "apb_pclk", NULL);
> > +
> > +       ep93xx_dma_clock_init(base);
>
> Does some interrupt or timer driver use dma? Why are these registered
> early?
>
> > +
> > +       /* Determine the bootloader configured pll2 rate */
> > +       ep93xx_regmap_read(EP93XX_SYSCON_CLKSET2, &value);
> > +       if (!(value & EP93XX_SYSCON_CLKSET2_NBYP2))
> > +               clk_pll2_rate = EP93XX_EXT_CLK_RATE;
> > +       else if (value & EP93XX_SYSCON_CLKSET2_PLL2_EN)
> > +               clk_pll2_rate = calc_pll_rate(EP93XX_EXT_CLK_RATE,
> > value);
> > +       else
> > +               clk_pll2_rate = 0;
> > +
> > +       hw = clk_hw_register_fixed_rate(NULL, "pll2", "xtali", 0,
> > clk_pll2_rate);
>
> Instead of registering a fixed rate clk here you can implemented
> recalc_rate and look at the regmap value.
>
> > +       ep93xx_clk_data->hws[EP93XX_CLK_PLL2] = hw;
> > +       clk_hw_register_clkdev(hw, NULL, "pll2");
> > +
> > +       ep93xx_clk_data->num = EP93XX_NUM_CLKS;
> > +       of_clk_add_hw_provider(np, of_clk_hw_onecell_get,
> > ep93xx_clk_data);
> > +
> > +       pr_info("PLL1 running at %ld MHz, PLL2 at %ld MHz\n",
> > +               clk_pll1_rate / 1000000, clk_pll2_rate / 1000000);
> > +       pr_info("FCLK %ld MHz, HCLK %ld MHz, PCLK %ld MHz\n",
> > +               clk_f_rate / 1000000, clk_h_rate / 1000000,
> > +               clk_p_rate / 1000000);
> > +}
> > +CLK_OF_DECLARE_DRIVER(ep93xx_cc, "cirrus,ep9301-syscon",
> > ep93xx_clock_init);

2023-06-13 21:34:31

by Stephen Boyd

Subject: Re: [PATCH 12/43] clk: ep93xx: add DT support for Cirrus EP93xx

Quoting Nikita Shubin (2023-05-15 06:31:41)
> Hello Stephen!
>
> Thank you for your review.
>
> Acknowledged all remarks, however, i didn't fully understood some of
> the requirements:

When the reply is taken out of context it is harder for me to recall
what we're talking about.

>
> > And maybe this can be registered from wherever the regmap is created?
>
> Are you suggesting that all clock from init, i.e. "pll1", "pll2",
> "hclk", "fclk", "pclk"
>
> Should be moved to SoC driver instead:
>
> https://lore.kernel.org/all/[email protected]/
>
> ?

Sure? That looks like a possibility.

>
> > Does some interrupt or timer driver use dma? Why are these registered
> > early?
>
> ep93xx_dma uses subsys_initcall(ep93xx_dma_module_init)
>
> https://elixir.bootlin.com/linux/v6.3.2/source/drivers/dma/ep93xx_dma.c#L1430
>
> I can move clk to using arch_initcall and move all stuff to probe, i
> think...

Sounds like the answer to my question is no. In which case moving to
arch_initcall() or probe will work. Please try.

>
> > Why is this in arm/ directory? Isn't it a clock controller?
>
> ep93xx clocks, reboot, pinctrl use syscon regmap and some special
> functions from SoC, i.e. "Syscon Software Lock Register" - so we are
> able to write to some registers only after writing some magik value
> there.
>
> So all above use regmap from SoC.

There can be an API in a header located in include/soc/ that implements
the magik value unlock sequence?

>
> Should make a separate clock controller like one i did for pinctrl ?
> Then it should look like:
>
> syscon@80930000 {
> compatible = "cirrus,ep9301-syscon",
> "syscon", "simple-mfd";
> reg = <0x80930000 0x1000>;
> #reset-cells = <1>;
>
> clocks {
> xtali: oscillator {
> compatible = "fixed-clock";
> #clock-cells = <0>;
> clock-frequency = <14745600>;
> };
> };
>
> clock-controller {
> #clock-cells = <1>;
> compatible = "cirrus,ep9301-clk";
> clocks = <&xtali>;
> };
> };
>
> Or even simply:
>
> clocks {
> xtali: oscillator {
> compatible = "fixed-clock";
> #clock-cells = <0>;
> clock-frequency = <14745600>;
> };
> };
>
> clock-controller {
> #clock-cells = <1>;
> compatible = "cirrus,ep9301-clk";
> clocks = <&xtali>;
> };

The DT binding shouldn't be making sub-nodes to match driver structure
of the kernel. Instead, there should be a node for a register range that
represents a device on the bus. That device may be multipurpose, in
which case it can probe as a platform driver and that platform driver
can create some number of auxiliary bus devices for the sub
functionality of the device. We shouldn't be prescribing Linux software
constructs onto the DT binding.