Quoting Nikita Shubin via B4 Relay (2024-04-08 01:09:54) > diff --git a/drivers/clk/clk-ep93xx.c b/drivers/clk/clk-ep93xx.c > new file mode 100644 > index 000000000000..601acb4402be > --- /dev/null > +++ b/drivers/clk/clk-ep93xx.c > @@ -0,0 +1,840 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Clock control for Cirrus EP93xx chips. > + * Copyright (C) 2021 Nikita Shubin <nikita.shubin@xxxxxxxxxxx> > + * > + * Based on a rewrite of arch/arm/mach-ep93xx/clock.c: > + * Copyright (C) 2006 Lennert Buytenhek <buytenh@xxxxxxxxxxxxxx> > + */ > +#define pr_fmt(fmt) "ep93xx " KBUILD_MODNAME ": " fmt > + > +#include <linux/bits.h> > +#include <linux/cleanup.h> > +#include <linux/clk-provider.h> > +#include <linux/clk.h> Is this include used? > +#include <linux/math.h> > +#include <linux/mfd/syscon.h> > +#include <linux/of_address.h> Is this include used? > +#include <linux/platform_device.h> > +#include <linux/regmap.h> > +#include <linux/spinlock.h> > +#include <linux/sys_soc.h> > + > +#include <linux/soc/cirrus/ep93xx.h> > +#include <dt-bindings/clock/cirrus,ep9301-syscon.h> > + > +#include <asm/div64.h> > + [...] > + > +static const struct soc_device_attribute ep93xx_soc_table[] = { > + { .revision = "E2", .data = (void *)1 }, Can we populate a different named clk auxiliary device instead? Preferably this table and logic lives in the soc driver that creates the device for this driver. > + { /* sentinel */ } > +}; > + > +static int ep93xx_clk_probe(struct auxiliary_device *adev, > + const struct auxiliary_device_id *id) > +{ > + struct ep93xx_regmap_adev *rdev = to_ep93xx_regmap_adev(adev); > + unsigned int clk_f_div, clk_h_div, clk_p_div, clk_usb_div; > + const char fclk_divisors[] = { 1, 2, 4, 8, 16, 1, 1, 1 }; > + const char hclk_divisors[] = { 1, 2, 4, 5, 6, 8, 16, 32 }; > + const char pclk_divisors[] = { 1, 2, 4, 8 }; > + struct clk_parent_data xtali = { .index = 0 }; > + struct clk_parent_data ddiv_pdata[3] = { }; > + unsigned long clk_pll1_rate, clk_pll2_rate, clk_spi_div; > + const struct soc_device_attribute *match; > + struct clk_parent_data pdata = {}; > + struct device *dev = &adev->dev; > + struct ep93xx_clk_priv *priv; > + struct ep93xx_clk *clk; > + struct clk_hw *hw, *pll1; > + unsigned int idx; > + int ret; > + u32 value; Maybe make the pll registration code a sub-function. This is a lot of local variables. > + > + priv = devm_kzalloc(dev, struct_size(priv, reg, 10), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + spin_lock_init(&priv->lock); > + priv->dev = dev; > + priv->aux_dev = rdev; > + priv->map = rdev->map; > + priv->base = rdev->base; > + > + /* Determine the bootloader configured pll1 rate */ > + regmap_read(priv->map, EP93XX_SYSCON_CLKSET1, &value); > + > + if (value & EP93XX_SYSCON_CLKSET1_NBYP1) > + clk_pll1_rate = calc_pll_rate(EP93XX_EXT_CLK_RATE, value); > + else > + clk_pll1_rate = EP93XX_EXT_CLK_RATE; > + > + pll1 = devm_clk_hw_register_fixed_rate(dev, "pll1", "xtali", 0, clk_pll1_rate); > + if (IS_ERR(pll1)) > + return PTR_ERR(pll1); > + > + priv->fixed[EP93XX_CLK_PLL1] = pll1; > + > + /* Initialize the pll1 derived clocks */ > + clk_f_div = fclk_divisors[(value >> 25) & GENMASK(2, 0)]; > + clk_h_div = hclk_divisors[(value >> 20) & GENMASK(2, 0)]; > + clk_p_div = pclk_divisors[(value >> 18) & GENMASK(1, 0)]; > + > + hw = devm_clk_hw_register_fixed_factor_parent_hw(dev, "fclk", pll1, 0, 1, clk_f_div); > + if (IS_ERR(hw)) > + return PTR_ERR(hw); > + > + priv->fixed[EP93XX_CLK_FCLK] = hw; > + > + hw = devm_clk_hw_register_fixed_factor_parent_hw(dev, "hclk", pll1, 0, 1, clk_h_div); > + if (IS_ERR(hw)) > + return PTR_ERR(hw); > + > + priv->fixed[EP93XX_CLK_HCLK] = hw; > + > + hw = devm_clk_hw_register_fixed_factor_parent_hw(dev, "pclk", hw, 0, 1, clk_p_div); > + if (IS_ERR(hw)) > + return PTR_ERR(hw); > + > + priv->fixed[EP93XX_CLK_PCLK] = hw; > + > + /* Determine the bootloader configured pll2 rate */ > + regmap_read(priv->map, 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 = devm_clk_hw_register_fixed_rate(dev, "pll2", "xtali", 0, clk_pll2_rate); > + if (IS_ERR(hw)) > + return PTR_ERR(hw); > + > + priv->fixed[EP93XX_CLK_PLL2] = hw; > + > + regmap_read(priv->map, EP93XX_SYSCON_CLKSET2, &value); > + clk_usb_div = (value >> 28 & GENMASK(3, 0)) + 1; > + hw = devm_clk_hw_register_fixed_factor(dev, "usb_clk", "pll2", 0, 1, clk_usb_div); > + if (IS_ERR(hw)) > + return PTR_ERR(hw); > + > + priv->fixed[EP93XX_CLK_USB] = hw; > + > + 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 section 6 "2x SSP (Synchronous Serial Port) Clock – Revision E2 only": > + * http://www.cirrus.com/en/pubs/appNote/AN273REV4.pdf > + */ > + clk_spi_div = 2; > + match = soc_device_match(ep93xx_soc_table); > + if (match) > + clk_spi_div = (unsigned long)match->data; > + > + hw = devm_clk_hw_register_fixed_factor(dev, "ep93xx-spi.0", "xtali", > + 0, 1, clk_spi_div); > + if (IS_ERR(hw)) > + return PTR_ERR(hw); > + > + priv->fixed[EP93XX_CLK_SPI] = hw; > + > + /* PWM clock */ > + hw = devm_clk_hw_register_fixed_factor(dev, "pwm_clk", "xtali", 0, 1, 1); > + if (IS_ERR(hw)) > + return PTR_ERR(hw); > + > + priv->fixed[EP93XX_CLK_PWM] = hw; > + > + /* USB clock */ > + hw = devm_clk_hw_register_gate(priv->dev, "ohci-platform", "usb_clk", > + 0, priv->base + EP93XX_SYSCON_PWRCNT, > + EP93XX_SYSCON_PWRCNT_USH_EN, 0, > + &priv->lock); > + if (IS_ERR(hw)) > + return PTR_ERR(hw); > + > + priv->fixed[EP93XX_CLK_USB] = hw; > + > + ddiv_pdata[0].index = 0; /* XTALI external clock */ > + ddiv_pdata[1].hw = priv->fixed[EP93XX_CLK_PLL1]; > + ddiv_pdata[2].hw = priv->fixed[EP93XX_CLK_PLL2]; > + > + /* touchscreen/ADC clock */ > + idx = EP93XX_CLK_ADC - EP93XX_CLK_UART1; > + clk = &priv->reg[idx]; > + clk->idx = idx; > + ret = clk_hw_register_div(clk, "ep93xx-adc", &xtali, Use devm? > + EP93XX_SYSCON_KEYTCHCLKDIV, > + EP93XX_SYSCON_KEYTCHCLKDIV_TSEN, > + EP93XX_SYSCON_KEYTCHCLKDIV_ADIV, > + 1, > + adc_divisors, > + ARRAY_SIZE(adc_divisors)); > + > + > + /* keypad clock */ > + idx = EP93XX_CLK_KEYPAD - EP93XX_CLK_UART1; > + clk = &priv->reg[idx]; > + clk->idx = idx; > + ret = clk_hw_register_div(clk, "ep93xx-keypad", &xtali, > + EP93XX_SYSCON_KEYTCHCLKDIV, > + EP93XX_SYSCON_KEYTCHCLKDIV_KEN, > + EP93XX_SYSCON_KEYTCHCLKDIV_KDIV, > + 1, > + adc_divisors, > + ARRAY_SIZE(adc_divisors)); > + > + /* > + * On reset PDIV and VDIV is set to zero, while PDIV zero > + * means clock disable, VDIV shouldn't be zero. > + * So we set both video and i2s dividers to minimum. > + * ENA - Enable CLK divider. > + * PDIV - 00 - Disable clock > + * VDIV - at least 2 > + */ > + > + /* Check and enable video clk registers */ > + regmap_read(priv->map, EP93XX_SYSCON_VIDCLKDIV, &value); > + value |= BIT(EP93XX_SYSCON_CLKDIV_PDIV_SHIFT) | 2; > + ep93xx_clk_write(priv, EP93XX_SYSCON_VIDCLKDIV, value); > + > + /* Check and enable i2s clk registers */ > + regmap_read(priv->map, EP93XX_SYSCON_I2SCLKDIV, &value); > + value |= BIT(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_clk_write(priv, EP93XX_SYSCON_I2SCLKDIV, value); > + > + /* video clk */ > + idx = EP93XX_CLK_VIDEO - EP93XX_CLK_UART1; > + clk = &priv->reg[idx]; > + clk->idx = idx; > + ret = clk_hw_register_ddiv(clk, "ep93xx-fb", > + ddiv_pdata, ARRAY_SIZE(ddiv_pdata), > + EP93XX_SYSCON_VIDCLKDIV, > + EP93XX_SYSCON_CLKDIV_ENABLE); > + > + /* i2s clk */ > + idx = EP93XX_CLK_I2S_MCLK - EP93XX_CLK_UART1; > + clk = &priv->reg[idx]; > + clk->idx = idx; > + ret = clk_hw_register_ddiv(clk, "mclk", > + ddiv_pdata, ARRAY_SIZE(ddiv_pdata), > + EP93XX_SYSCON_I2SCLKDIV, > + EP93XX_SYSCON_CLKDIV_ENABLE); > + > + /* i2s sclk */ > + idx = EP93XX_CLK_I2S_SCLK - EP93XX_CLK_UART1; > + clk = &priv->reg[idx]; > + clk->idx = idx; > + pdata.hw = &priv->reg[EP93XX_CLK_I2S_MCLK - EP93XX_CLK_UART1].hw; > + ret = clk_hw_register_div(clk, "sclk", &pdata, > + EP93XX_SYSCON_I2SCLKDIV, > + EP93XX_SYSCON_I2SCLKDIV_SENA, > + 16, /* EP93XX_I2SCLKDIV_SDIV_SHIFT */ > + 1, /* EP93XX_I2SCLKDIV_SDIV_WIDTH */ > + sclk_divisors, > + ARRAY_SIZE(sclk_divisors)); > + > + /* i2s lrclk */ > + idx = EP93XX_CLK_I2S_LRCLK - EP93XX_CLK_UART1; > + clk = &priv->reg[idx]; > + clk->idx = idx; > + pdata.hw = &priv->reg[EP93XX_CLK_I2S_SCLK - EP93XX_CLK_UART1].hw; > + ret = clk_hw_register_div(clk, "lrclk", &pdata, > + EP93XX_SYSCON_I2SCLKDIV, > + EP93XX_SYSCON_I2SCLKDIV_SENA, > + 17, /* EP93XX_I2SCLKDIV_LRDIV32_SHIFT */ > + 2, /* EP93XX_I2SCLKDIV_LRDIV32_WIDTH */ > + lrclk_divisors, > + ARRAY_SIZE(lrclk_divisors)); > + > + /* IrDa clk uses same pattern but no init code presents in original clock driver */ > + return devm_of_clk_add_hw_provider(priv->dev, of_clk_ep93xx_get, priv); > +} > + > +static const struct auxiliary_device_id ep93xx_clk_ids[] = { > + { > + .name = "soc_ep93xx.clk-ep93xx", > + }, This can be one line instead of three. > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(auxiliary, ep93xx_clk_ids);