Re: [PATCH v3 03/42] clk: ep93xx: add DT support for Cirrus EP93xx

Quoting Nikita Shubin via B4 Relay (2023-07-20 04:29:03)

diff --git a/drivers/clk/clk-ep93xx.c b/drivers/clk/clk-ep93xx.c new file mode 100644 index 000000000000..7b9b3a0894ab --- /dev/null +++ b/drivers/clk/clk-ep93xx.c @@ -0,0 +1,764 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/*

• • Clock control for Cirrus EP93xx chips.
• • Copyright (C) 2021 Nikita Shubin nikita.shubin@maquefel.me
• • Based on a rewrite of arch/arm/mach-ep93xx/clock.c:
• • Copyright (C) 2006 Lennert Buytenhek buytenh@wantstofly.org
• */

+#define pr_fmt(fmt) "ep93xx " KBUILD_MODNAME ": " fmt

+#include <linux/bits.h> +#include <linux/clk-provider.h> +#include <linux/clk.h> +#include <linux/clkdev.h>

Drop the unused includes.

+#include <linux/err.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/mfd/syscon.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/platform_device.h> +#include <linux/regmap.h> +#include <linux/spinlock.h> +#include <linux/string.h> +#include <linux/sys_soc.h>

+#include <linux/soc/cirrus/ep93xx.h> +#include <dt-bindings/clock/cirrus,ep93xx-clock.h>

+#include <asm/div64.h>

+#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_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_MASK GENMASK(14, 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

+/* Keeps track of all clocks */ +static const char adc_divisors[] = { 16, 4 }; +static const char sclk_divisors[] = { 2, 4 }; +static const char lrclk_divisors[] = { 32, 64, 128 };

+#define EP_PARENT(NAME) { .name = NAME, .fw_name = NAME }

+static const struct clk_parent_data ep93xx_clk_parents[] = {

•   EP_PARENT("xtali"),
  

•   EP_PARENT("pll1"),
  

•   EP_PARENT("pll2"),
  

pll2 and pll1 aren't in the DT binding, so they shouldn't be set for .fw_name here.

+};

+struct ep93xx_clk {

•   struct clk_hw hw;
  

•   u16 idx;
  

•   u16 reg;
  

•   u32 mask;
  

•   u8 bit_idx;
  

•   u8 shift;
  

•   u8 width;
  

•   u8 num_div;
  

•   const char *div;
  

+};

+struct ep93xx_clk_priv {

•   spinlock_t lock;
  

•   struct device *dev;
  

•   void __iomem *base;
  

•   struct regmap *map;
  

•   struct clk_hw *fixed[16];
  

•   struct ep93xx_clk reg[];
  

+};

+static struct ep93xx_clk *ep93xx_clk_from(struct clk_hw *hw) +{

•   return container_of(hw, struct ep93xx_clk, hw);
  

+}

+static struct ep93xx_clk_priv *ep93xx_priv_from(struct ep93xx_clk *clk) +{

•   return container_of(clk, struct ep93xx_clk_priv, reg[clk->idx]);
  

+}

+static int ep93xx_clk_is_enabled(struct clk_hw *hw) +{

•   struct ep93xx_clk *clk = ep93xx_clk_from(hw);
  

•   struct ep93xx_clk_priv *priv = ep93xx_priv_from(clk);
  

•   u32 val;
  

•   regmap_read(priv->map, clk->reg, &val);
  

•   return !!(val & BIT(clk->bit_idx));
  

+}

+static int ep93xx_clk_enable(struct clk_hw *hw) +{

•   struct ep93xx_clk *clk = ep93xx_clk_from(hw);
  

•   struct ep93xx_clk_priv *priv = ep93xx_priv_from(clk);
  

•   unsigned long flags;
  

•   u32 val;
  

•   spin_lock_irqsave(&priv->lock, flags);
  

•   regmap_read(priv->map, clk->reg, &val);
  

•   val |= BIT(clk->bit_idx);
  

•   ep93xx_syscon_swlocked_write(priv->map, clk->reg, val);
  

•   spin_unlock_irqrestore(&priv->lock, flags);
  

•   return 0;
  

+}

+static void ep93xx_clk_disable(struct clk_hw *hw) +{

•   struct ep93xx_clk *clk = ep93xx_clk_from(hw);
  

•   struct ep93xx_clk_priv *priv = ep93xx_priv_from(clk);
  

•   unsigned long flags;
  

•   u32 val;
  

•   spin_lock_irqsave(&priv->lock, flags);
  

•   regmap_read(priv->map, clk->reg, &val);
  

•   val &= ~BIT(clk->bit_idx);
  

•   ep93xx_syscon_swlocked_write(priv->map, clk->reg, val);
  

•   spin_unlock_irqrestore(&priv->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 int ep93xx_clk_register_gate(struct ep93xx_clk *clk,

•                                   const char *name,
  

•                                   const char *parent_name,
  

•                                   unsigned long flags,
  

•                                   unsigned int reg,
  

•                                   u8 bit_idx)
  

+{

•   struct ep93xx_clk_priv *priv = ep93xx_priv_from(clk);
  

•   struct clk_parent_data parent_data = { };
  

•   struct clk_init_data init = { };
  

•   init.name = name;
  

•   init.ops = &clk_ep93xx_gate_ops;
  

•   init.flags = flags;
  

•   parent_data.fw_name = parent_name;
  

•   parent_data.name = parent_name;
  

•   init.parent_data = &parent_data;
  

•   init.num_parents = 1;
  

•   clk->reg = reg;
  

•   clk->bit_idx = bit_idx;
  

•   clk->hw.init = &init;
  

•   return devm_clk_hw_register(priv->dev, &clk->hw);
  

+}

+static u8 ep93xx_mux_get_parent(struct clk_hw *hw) +{

•   struct ep93xx_clk *clk = ep93xx_clk_from(hw);
  

•   struct ep93xx_clk_priv *priv = ep93xx_priv_from(clk);
  

•   u32 val;
  

•   regmap_read(priv->map, clk->reg, &val);
  

•   val &= EP93XX_SYSCON_CLKDIV_MASK;
  

•   switch (val) {
  

•   case EP93XX_SYSCON_CLKDIV_ESEL:
  

•           return 1; // PLL1
  

•   case EP93XX_SYSCON_CLKDIV_MASK:
  

•           return 2; // PLL2
  

•   default:
  

•           break;
  

•   };
  

•   return 0; // XTALI
  

Please use /* comments like this */

+}

+static int ep93xx_mux_set_parent_lock(struct clk_hw *hw, u8 index) +{

•   struct ep93xx_clk *clk = ep93xx_clk_from(hw);
  

•   struct ep93xx_clk_priv *priv = ep93xx_priv_from(clk);
  

•   unsigned long flags;
  

•   u32 val;
  

•   if (index >= ARRAY_SIZE(ep93xx_clk_parents))
  

•           return -EINVAL;
  

•   spin_lock_irqsave(&priv->lock, flags);
  

•   regmap_read(priv->map, clk->reg, &val);
  

•   val &= ~(EP93XX_SYSCON_CLKDIV_MASK);
  

•   if (index) {
  

•           val |= EP93XX_SYSCON_CLKDIV_ESEL;
  

•           val |= (index - 1) ? EP93XX_SYSCON_CLKDIV_PSEL : 0;
  

•   }
  

•   ep93xx_syscon_swlocked_write(priv->map, clk->reg, val);
  

•   spin_unlock_irqrestore(&priv->lock, flags);
  

•   return 0;
  

+}

+static bool is_best(unsigned long rate, unsigned long now,

•                unsigned long best)
  

+{

•   return abs(rate - now) < abs(rate - best);
  

Another case where we need abs_diff() so that it doesn't get confused when trying to do signed comparison.

+}

+static int ep93xx_mux_determine_rate(struct clk_hw *hw,

•                           struct clk_rate_request *req)
  

+{

•   unsigned long best_rate = 0, actual_rate, mclk_rate;
  

•   unsigned long rate = req->rate;
  

•   struct clk_hw *parent_best = NULL;
  

•   unsigned long parent_rate_best;
  

•   unsigned long parent_rate;
  

•   int div, pdiv;
  

•   unsigned 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 < clk_hw_get_num_parents(hw); i++) {
  

•           struct clk_hw *parent = clk_hw_get_parent_by_index(hw, i);
  

•           parent_rate = clk_hw_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 = parent_best;
  

•   req->rate = best_rate;
  

•   return 0;
  

+}

+static unsigned long ep93xx_ddiv_recalc_rate(struct clk_hw *hw,

•                                           unsigned long parent_rate)
  

+{

•   struct ep93xx_clk *clk = ep93xx_clk_from(hw);
  

•   struct ep93xx_clk_priv *priv = ep93xx_priv_from(clk);
  

•   unsigned int pdiv, div;
  

•   u32 val;
  

•   regmap_read(priv->map, clk->reg, &val);
  

•   pdiv = ((val >> EP93XX_SYSCON_CLKDIV_PDIV_SHIFT) & 0x03);
  

•   div = val & GENMASK(6, 0);
  

•   if (!div)
  

•           return 0;
  

•   return DIV_ROUND_CLOSEST(parent_rate * 2, (pdiv + 3) * div);
  

+}

+static int ep93xx_ddiv_set_rate(struct clk_hw *hw, unsigned long rate,

•                           unsigned long parent_rate)
  

+{

•   struct ep93xx_clk *clk = ep93xx_clk_from(hw);
  

•   struct ep93xx_clk_priv *priv = ep93xx_priv_from(clk);
  

•   int pdiv, div, npdiv, ndiv;
  

•   unsigned long actual_rate, mclk_rate, rate_err = ULONG_MAX;
  

•   u32 val;
  

•   regmap_read(priv->map, clk->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 (abs(actual_rate - rate) < rate_err) {
  

•                   npdiv = pdiv - 3;
  

•                   ndiv = div;
  

•                   rate_err = abs(actual_rate - rate);
  

•           }
  

•   }
  

•   if (rate_err == ULONG_MAX)
  

•           return -EINVAL;
  

•   /* Clear old dividers */
  

•   val &= ~(GENMASK(9, 0) & ~BIT(7));
  

•   /* Set the new pdiv and div bits for the new clock rate */
  

•   val |= (npdiv << EP93XX_SYSCON_CLKDIV_PDIV_SHIFT) | ndiv;
  

•   ep93xx_syscon_swlocked_write(priv->map, clk->reg, val);
  

•   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 int clk_hw_register_ddiv(struct ep93xx_clk *clk,

•                                           const char *name,
  

•                                           unsigned int reg,
  

•                                           u8 bit_idx)
  

+{

•   struct ep93xx_clk_priv *priv = ep93xx_priv_from(clk);
  

•   struct clk_init_data init = { };
  

•   init.name = name;
  

•   init.ops = &clk_ddiv_ops;
  

•   init.flags = 0;
  

•   init.parent_data = ep93xx_clk_parents;
  

•   init.num_parents = ARRAY_SIZE(ep93xx_clk_parents);
  

•   clk->reg = reg;
  

•   clk->bit_idx = bit_idx;
  

•   clk->hw.init = &init;
  

•   return devm_clk_hw_register(priv->dev, &clk->hw);
  

+}

+static unsigned long ep93xx_div_recalc_rate(struct clk_hw *hw,

•                                       unsigned long parent_rate)
  

+{

•   struct ep93xx_clk *clk = ep93xx_clk_from(hw);
  

•   struct ep93xx_clk_priv *priv = ep93xx_priv_from(clk);
  

•   u32 val;
  

•   u8 index;
  

•   regmap_read(priv->map, clk->reg, &val);
  

•   index = (val & clk->mask) >> clk->shift;
  

•   if (index > clk->num_div)
  

•           return 0;
  

•   return DIV_ROUND_CLOSEST(parent_rate, clk->div[index]);
  

+}

+static long ep93xx_div_round_rate(struct clk_hw *hw, unsigned long rate,

•                              unsigned long *parent_rate)
  

+{

•   struct ep93xx_clk *clk = ep93xx_clk_from(hw);
  

•   unsigned long best = 0, now;
  

•   unsigned int i;
  

•   for (i = 0; i < clk->num_div; i++) {
  

•           if ((rate * clk->div[i]) == *parent_rate)
  

•                   return rate;
  

•           now = DIV_ROUND_CLOSEST(*parent_rate, clk->div[i]);
  

•           if (!best || is_best(rate, now, best))
  

•                   best = now;
  

•   }
  

•   return best;
  

+}

+static int ep93xx_div_set_rate(struct clk_hw *hw, unsigned long rate,

•                          unsigned long parent_rate)
  

+{

•   struct ep93xx_clk *clk = ep93xx_clk_from(hw);
  

•   struct ep93xx_clk_priv *priv = ep93xx_priv_from(clk);
  

•   unsigned int i;
  

•   u32 val;
  

•   regmap_read(priv->map, clk->reg, &val);
  

•   val &= ~clk->mask;
  

•   for (i = 0; i < clk->num_div; i++)
  

•           if (rate == DIV_ROUND_CLOSEST(parent_rate, clk->div[i])) {
  

•                   val |= i << clk->shift;
  

•                   break;
  

•           }
  

•   if (i == clk->num_div)
  

•           return -EINVAL;
  

•   ep93xx_syscon_swlocked_write(priv->map, clk->reg, val);
  

•   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 int clk_hw_register_div(struct ep93xx_clk *clk,

•                                     const char *name,
  

•                                     const char *parent_name,
  

Please try to pass a direct pointer to the parent clk_hw instead or if the clk exists outside the controller pass a DT index.

•                                     unsigned int reg,
  

•                                     u8 enable_bit,
  

•                                     u8 shift,
  

•                                     u8 width,
  

•                                     const char *clk_divisors,
  

•                                     u8 num_div)
  

+{

•   struct ep93xx_clk_priv *priv = ep93xx_priv_from(clk);
  

•   struct clk_parent_data parent_data = { };
  

•   struct clk_init_data init = { };
  

•   init.name = name;
  

•   init.ops = &ep93xx_div_ops;
  

•   init.flags = 0;
  

•   parent_data.fw_name = parent_name;
  

•   parent_data.name = parent_name;
  

•   init.parent_data = &parent_data;
  

•   init.num_parents = 1;
  

•   clk->reg = reg;
  

•   clk->bit_idx = enable_bit;
  

•   clk->mask = GENMASK(shift + width - 1, shift);
  

•   clk->shift = shift;
  

•   clk->div = clk_divisors;
  

[...]

+static int ep93xx_clk_probe(struct platform_device *pdev) +{

•   struct device *dev = &pdev->dev;
  

•   struct device_node *np = dev->of_node;
  

•   const struct soc_device_attribute *match;
  

•   struct ep93xx_clk_priv *priv;
  

•   struct ep93xx_clk *clk;
  

•   struct device_node *parent;
  

•   unsigned long clk_spi_div;
  

•   int ret;
  

•   u32 value;
  

•   priv = devm_kzalloc(dev, struct_size(priv, reg, EP93XX_CLK_UART), GFP_KERNEL);
  

•   if (!priv)
  

•           return -ENOMEM;
  

•   spin_lock_init(&priv->lock);
  

•   priv->dev = dev;
  

•   parent = of_get_parent(np);
  

•   if (!parent)
  

•           return dev_err_probe(dev, -EINVAL, "no syscon parent for clk node\n");
  

•   priv->map = syscon_node_to_regmap(parent);
  

•   if (IS_ERR(priv->map)) {
  

•           of_node_put(parent);
  

•           return dev_err_probe(dev, -EINVAL, "no syscon regmap\n");
  

•   }
  

•   priv->base = devm_of_iomap(dev, parent, 0, NULL);
  

•   of_node_put(parent);
  

•   if (IS_ERR(priv->base))
  

•           return PTR_ERR(priv->base);
  

•   ret = ep93xx_uart_clock_init(priv);
  

•   if (ret)
  

•           return ret;
  

•   ret = ep93xx_dma_clock_init(priv);
  

•   if (ret)
  

•           return ret;
  

•   /*
  

•    * 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 = 2;
  

•   match = soc_device_match(ep93xx_soc_table);
  

Why isn't this part of the compatible string for the device? We're able to introduce new bindings, correct?

•   if (match)
  

•           clk_spi_div = (unsigned long)match->data;