mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Nikita Shubin <nikita.shubin@maquefel.me>
To: Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>,
	Alexander Sverdlin <alexander.sverdlin@gmail.com>,
	Arnd Bergmann <arnd@arndb.de>
Cc: linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 03/42] clk: ep93xx: add DT support for Cirrus EP93xx
Date: Sun, 23 Jul 2023 21:37:24 +0300	[thread overview]
Message-ID: <caa702de55272effb0bee2e78e25214f8aeb64e9.camel@maquefel.me> (raw)
In-Reply-To: <3fcb760c101c5f7081235290362f5c02.sboyd@kernel.org>

Hello Stephen.

On Thu, 2023-07-20 at 16:27 -0700, Stephen Boyd wrote:
> 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?

It's not a SoC version related, so we don't know such things in
advance.

It's currently determined by querying "ep93xx_chip_revision" which
determines chip revision:

https://elixir.bootlin.com/linux/v6.5-rc2/source/arch/arm/mach-ep93xx/clock.c#L631

I decided it's a good idea to drop another header exposed function.

"soc_device_match" only uses revision:

```
static const struct soc_device_attribute ep93xx_soc_table[] = {
	{ .revision = "E2", .data = (void *)1 },
	{ /* sentinel */ }
};
```

But as there are no revisions greater than E2 - it is single match
only.

> 
> > +       if (match)
> > +               clk_spi_div = (unsigned long)match->data;


  parent reply	other threads:[~2023-07-23 15:37 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20230605-ep93xx-v3-0-3d63a5f1103e@maquefel.me>
2023-07-20  8:54 ` [PATCH v3 00/42] ep93xx device tree conversion Krzysztof Kozlowski
     [not found] ` <20230605-ep93xx-v3-38-3d63a5f1103e@maquefel.me>
2023-07-20  9:40   ` [PATCH v3 38/42] ata: pata_ep93xx: remove legacy pinctrl use Sergey Shtylyov
2023-07-21 14:16   ` Andy Shevchenko
     [not found] ` <20230605-ep93xx-v3-26-3d63a5f1103e@maquefel.me>
2023-07-20  9:45   ` [PATCH v3 26/42] ata: pata_ep93xx: add device tree support Sergey Shtylyov
     [not found] ` <20230605-ep93xx-v3-13-3d63a5f1103e@maquefel.me>
2023-07-20  8:54   ` [PATCH v3 13/42] watchdog: ep93xx: add DT support for Cirrus EP93xx Alexander Sverdlin
2023-07-20 13:29   ` Guenter Roeck
     [not found] ` <20230605-ep93xx-v3-32-3d63a5f1103e@maquefel.me>
2023-07-20 13:30   ` [PATCH v3 32/42] wdt: ts72xx: add DT support for ts72xx Guenter Roeck
     [not found] ` <20230605-ep93xx-v3-33-3d63a5f1103e@maquefel.me>
2023-07-20 14:49   ` [PATCH v3 33/42] gpio: ep93xx: add DT support for gpio-ep93xx Bartosz Golaszewski
     [not found] ` <20230605-ep93xx-v3-1-3d63a5f1103e@maquefel.me>
2023-07-20 14:51   ` [PATCH v3 01/42] gpio: ep93xx: split device in multiple Bartosz Golaszewski
2023-07-21 13:18   ` Andy Shevchenko
     [not found] ` <20230605-ep93xx-v3-2-3d63a5f1103e@maquefel.me>
2023-07-21 13:58   ` [PATCH v3 02/42] dt-bindings: clock: Add Cirrus EP93xx Krzysztof Kozlowski
     [not found]   ` <11dbf88d12051497ba1e3b16c0d39066.sboyd@kernel.org>
2023-07-23 18:20     ` Nikita Shubin
     [not found] ` <20230605-ep93xx-v3-4-3d63a5f1103e@maquefel.me>
2023-07-21 14:01   ` [PATCH v3 04/42] dt-bindings: pinctrl: " Krzysztof Kozlowski
     [not found] ` <20230605-ep93xx-v3-6-3d63a5f1103e@maquefel.me>
2023-07-21 14:04   ` [PATCH v3 06/42] dt-bindings: soc: " Krzysztof Kozlowski
     [not found] ` <20230605-ep93xx-v3-10-3d63a5f1103e@maquefel.me>
2023-07-21 14:07   ` [PATCH v3 10/42] dt-bindings: rtc: " Krzysztof Kozlowski
     [not found] ` <20230605-ep93xx-v3-12-3d63a5f1103e@maquefel.me>
2023-07-20 13:28   ` [PATCH v3 12/42] dt-bindings: watchdog: Add Cirrus EP93x Guenter Roeck
2023-07-21 14:08   ` Krzysztof Kozlowski
     [not found] ` <20230605-ep93xx-v3-34-3d63a5f1103e@maquefel.me>
2023-07-21 14:12   ` [PATCH v3 34/42] ARM: dts: add Cirrus EP93XX SoC .dtsi Krzysztof Kozlowski
     [not found] ` <20230605-ep93xx-v3-35-3d63a5f1103e@maquefel.me>
2023-07-21 14:15   ` [PATCH v3 35/42] ARM: dts: ep93xx: add ts7250 board Krzysztof Kozlowski
2023-07-24 13:41     ` Nikita Shubin
2023-07-24 13:48       ` Krzysztof Kozlowski
2023-07-29 20:59       ` Linus Walleij
2023-07-31 11:55         ` Nikita Shubin
     [not found] ` <20230605-ep93xx-v3-41-3d63a5f1103e@maquefel.me>
2023-07-21 14:16   ` [PATCH v3 41/42] ARM: dts: ep93xx: Add EDB9302 DT Krzysztof Kozlowski
     [not found] ` <20230605-ep93xx-v3-7-3d63a5f1103e@maquefel.me>
2023-07-21 14:13   ` [PATCH v3 07/42] soc: Add SoC driver for Cirrus ep93xx Andy Shevchenko
2023-07-28  9:28     ` Nikita Shubin
2023-07-28  9:46       ` Andy Shevchenko
2023-07-28 10:01         ` Nikita Shubin
2023-07-31 20:16           ` Andy Shevchenko
2023-11-11 21:33     ` Alexander Sverdlin
2023-11-13 10:19       ` Andy Shevchenko
2023-07-21 14:22   ` Krzysztof Kozlowski
2023-07-24 15:02     ` Nikita Shubin
2023-07-24 16:31       ` Krzysztof Kozlowski
2023-07-24 18:04         ` Arnd Bergmann
2023-07-24 19:04           ` Krzysztof Kozlowski
2023-07-25  7:32           ` Nikita Shubin
2023-08-15 20:40           ` Linus Walleij
2023-07-21 14:25 ` [PATCH v3 00/42] ep93xx device tree conversion Krzysztof Kozlowski
     [not found] ` <20230605-ep93xx-v3-5-3d63a5f1103e@maquefel.me>
2023-07-21 15:30   ` [PATCH v3 05/42] pinctrl: add a Cirrus ep93xx SoC pin controller Andy Shevchenko
     [not found] ` <20230605-ep93xx-v3-9-3d63a5f1103e@maquefel.me>
2023-07-21 15:58   ` [PATCH v3 09/42] clocksource: ep93xx: Add driver for Cirrus Logic EP93xx Andy Shevchenko
     [not found] ` <20230605-ep93xx-v3-28-3d63a5f1103e@maquefel.me>
2023-07-20 17:17   ` [PATCH v3 28/42] input: keypad: ep93xx: add DT support for Cirrus EP93xx Dmitry Torokhov
2023-07-21 16:12   ` Andy Shevchenko
     [not found] ` <20230605-ep93xx-v3-22-3d63a5f1103e@maquefel.me>
2023-07-21 16:20   ` [PATCH v3 22/42] dma: cirrus: " Andy Shevchenko
     [not found] ` <20230605-ep93xx-v3-24-3d63a5f1103e@maquefel.me>
2023-07-21 16:27   ` [PATCH v3 24/42] mtd: nand: add support for ts72xx Andy Shevchenko
2023-07-24  7:09     ` Miquel Raynal
     [not found] ` <20230605-ep93xx-v3-20-3d63a5f1103e@maquefel.me>
2023-07-21 16:32   ` [PATCH v3 20/42] net: cirrus: add DT support for Cirrus EP93xx Andy Shevchenko
     [not found] ` <20230605-ep93xx-v3-14-3d63a5f1103e@maquefel.me>
2023-07-21 16:37   ` [PATCH v3 14/42] power: reset: Add a driver for the ep93xx reset Andy Shevchenko
2023-07-28 13:53     ` Nikita Shubin
2023-07-31 20:27       ` Andy Shevchenko
2023-11-11 18:18     ` Alexander Sverdlin
2023-11-13  9:59       ` Andy Shevchenko
2023-11-13 10:04         ` Alexander Sverdlin
2023-11-13 10:07     ` Alexander Sverdlin
2023-11-13 10:22       ` Andy Shevchenko
     [not found] ` <20230605-ep93xx-v3-18-3d63a5f1103e@maquefel.me>
2023-07-21 16:42   ` [PATCH v3 18/42] spi: ep93xx: add DT support for Cirrus EP93xx Andy Shevchenko
     [not found] ` <20230605-ep93xx-v3-37-3d63a5f1103e@maquefel.me>
2023-07-28  8:23   ` [PATCH v3 37/42] pwm: ep93xx: drop legacy pinctrl Thierry Reding
     [not found] ` <20230605-ep93xx-v3-3-3d63a5f1103e@maquefel.me>
     [not found]   ` <3fcb760c101c5f7081235290362f5c02.sboyd@kernel.org>
2023-07-21 13:46     ` [PATCH v3 03/42] clk: ep93xx: add DT support for Cirrus EP93xx Andy Shevchenko
2023-07-23 18:37     ` Nikita Shubin [this message]
2023-07-28  8:38     ` Nikita Shubin
     [not found] ` <20230605-ep93xx-v3-29-3d63a5f1103e@maquefel.me>
2023-07-21 14:10   ` [PATCH v3 29/42] dt-bindings: rtc: Add ST M48T86 Krzysztof Kozlowski
2023-08-23 10:16   ` Krzysztof Kozlowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=caa702de55272effb0bee2e78e25214f8aeb64e9.camel@maquefel.me \
    --to=nikita.shubin@maquefel.me \
    --cc=alexander.sverdlin@gmail.com \
    --cc=arnd@arndb.de \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mturquette@baylibre.com \
    --cc=sboyd@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®