| Message ID | 20241018-imx-clk-v1-v2-1-92c0b66ca970@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | clk: imx: scu and fracn pll update | expand |
On 24-10-18 18:00:55, Peng Fan (OSS) wrote: > From: Peng Fan <peng.fan@nxp.com> > > Back-to-back LPCG writes can be ignored by the LPCG register due to > a HW bug. The writes need to be separated by at least 4 cycles of > the gated clock. See https://www.nxp.com.cn/docs/en/errata/IMX8_1N94W.pdf > > The workaround is implemented as follows: > 1. For clocks running greater than or equal to 24MHz, a read > followed by the write will provide sufficient delay. > 2. For clocks running below 24MHz, add a delay of 4 clock cylces > after the write to the LPCG register. > > Fixes: 2f77296d3df9 ("clk: imx: add lpcg clock support") > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > drivers/clk/imx/clk-lpcg-scu.c | 35 +++++++++++++++++++++++++++++++---- > 1 file changed, 31 insertions(+), 4 deletions(-) > > diff --git a/drivers/clk/imx/clk-lpcg-scu.c b/drivers/clk/imx/clk-lpcg-scu.c > index dd5abd09f3e206a5073767561b517d5b3320b28c..cd42190233662c66f2c354f0a2eee3a2531eeb3a 100644 > --- a/drivers/clk/imx/clk-lpcg-scu.c > +++ b/drivers/clk/imx/clk-lpcg-scu.c > @@ -6,10 +6,12 @@ > > #include <linux/bits.h> > #include <linux/clk-provider.h> > +#include <linux/delay.h> > #include <linux/err.h> > #include <linux/io.h> > #include <linux/slab.h> > #include <linux/spinlock.h> > +#include <linux/units.h> > > #include "clk-scu.h" > > @@ -41,6 +43,31 @@ struct clk_lpcg_scu { > > #define to_clk_lpcg_scu(_hw) container_of(_hw, struct clk_lpcg_scu, hw) > > +/* e10858 -LPCG clock gating register synchronization errata */ > +static void lpcg_e10858_writel(unsigned long rate, void __iomem *reg, u32 val) > +{ > + u32 reg1; > + > + writel(val, reg); > + > + if (rate >= 24 * HZ_PER_MHZ || rate == 0) { > + /* > + * The time taken to access the LPCG registers from the AP core > + * through the interconnect is longer than the minimum delay > + * of 4 clock cycles required by the errata. > + * Adding a readl will provide sufficient delay to prevent > + * back-to-back writes. > + */ > + reg1 = readl(reg); > + } else { > + /* > + * For clocks running below 24MHz, wait a minimum of > + * 4 clock cycles. > + */ > + ndelay(4 * (DIV_ROUND_UP(1000 * HZ_PER_MHZ, rate))); > + } > +} > + > static int clk_lpcg_scu_enable(struct clk_hw *hw) > { > struct clk_lpcg_scu *clk = to_clk_lpcg_scu(hw); > @@ -57,7 +84,8 @@ static int clk_lpcg_scu_enable(struct clk_hw *hw) > val |= CLK_GATE_SCU_LPCG_HW_SEL; > > reg |= val << clk->bit_idx; > - writel(reg, clk->reg); > + > + lpcg_e10858_writel(clk_hw_get_rate(hw), clk->reg, reg); > > spin_unlock_irqrestore(&imx_lpcg_scu_lock, flags); > > @@ -74,7 +102,7 @@ static void clk_lpcg_scu_disable(struct clk_hw *hw) > > reg = readl_relaxed(clk->reg); > reg &= ~(CLK_GATE_SCU_LPCG_MASK << clk->bit_idx); > - writel(reg, clk->reg); > + lpcg_e10858_writel(clk_hw_get_rate(hw), clk->reg, reg); > > spin_unlock_irqrestore(&imx_lpcg_scu_lock, flags); > } > @@ -149,9 +177,8 @@ static int __maybe_unused imx_clk_lpcg_scu_resume(struct device *dev) > * FIXME: Sometimes writes don't work unless the CPU issues > * them twice > */ > - > - writel(clk->state, clk->reg); Now that you removed one of the writes, doesn't the comment above has to be removed as well ? > writel(clk->state, clk->reg); > + lpcg_e10858_writel(0, clk->reg, clk->state); > dev_dbg(dev, "restore lpcg state 0x%x\n", clk->state); > > return 0; > > -- > 2.37.1 >
Hi Peng, kernel test robot noticed the following build warnings: [auto build test WARNING on d61a00525464bfc5fe92c6ad713350988e492b88] url: https://github.com/intel-lab-lkp/linux/commits/Peng-Fan-OSS/clk-imx-lpcg-scu-SW-workaround-for-errata-e10858/20241018-175440 base: d61a00525464bfc5fe92c6ad713350988e492b88 patch link: https://lore.kernel.org/r/20241018-imx-clk-v1-v2-1-92c0b66ca970%40nxp.com patch subject: [PATCH v2 1/4] clk: imx: lpcg-scu: SW workaround for errata (e10858) config: arm64-randconfig-r061-20241022 (https://download.01.org/0day-ci/archive/20241023/202410230141.3xLvkclt-lkp@intel.com/config) compiler: aarch64-linux-gcc (GCC) 14.1.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241023/202410230141.3xLvkclt-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202410230141.3xLvkclt-lkp@intel.com/ All warnings (new ones prefixed by >>): drivers/clk/imx/clk-lpcg-scu.c: In function 'lpcg_e10858_writel': >> drivers/clk/imx/clk-lpcg-scu.c:49:13: warning: variable 'reg1' set but not used [-Wunused-but-set-variable] 49 | u32 reg1; | ^~~~ vim +/reg1 +49 drivers/clk/imx/clk-lpcg-scu.c 45 46 /* e10858 -LPCG clock gating register synchronization errata */ 47 static void lpcg_e10858_writel(unsigned long rate, void __iomem *reg, u32 val) 48 { > 49 u32 reg1; 50 51 writel(val, reg); 52 53 if (rate >= 24 * HZ_PER_MHZ || rate == 0) { 54 /* 55 * The time taken to access the LPCG registers from the AP core 56 * through the interconnect is longer than the minimum delay 57 * of 4 clock cycles required by the errata. 58 * Adding a readl will provide sufficient delay to prevent 59 * back-to-back writes. 60 */ 61 reg1 = readl(reg); 62 } else { 63 /* 64 * For clocks running below 24MHz, wait a minimum of 65 * 4 clock cycles. 66 */ 67 ndelay(4 * (DIV_ROUND_UP(1000 * HZ_PER_MHZ, rate))); 68 } 69 } 70
> Subject: Re: [PATCH v2 1/4] clk: imx: lpcg-scu: SW workaround for > errata (e10858) > > On 24-10-18 18:00:55, Peng Fan (OSS) wrote: > > From: Peng Fan <peng.fan@nxp.com> > > > > Back-to-back LPCG writes can be ignored by the LPCG register due to > a > > HW bug. The writes need to be separated by at least 4 cycles of the > > gated clock. See > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F > www. > > > nxp.com.cn%2Fdocs%2Fen%2Ferrata%2FIMX8_1N94W.pdf&data=05% > 7C02%7Cpeng.f > > > an%40nxp.com%7Cbe85b7f113724069f20408dcf2a5eb47%7C686ea1 > d3bc2b4c6fa92c > > > d99c5c301635%7C0%7C1%7C638652041660547699%7CUnknown%7 > CTWFpbGZsb3d8eyJW > > > IjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0% > 3D%7C0%7C% > > > 7C%7C&sdata=YYgvBob0hm1poDDzb%2FEo6uRo1G29a0jmhqzhwr%2B > Hzcg%3D&reserve > > d=0 > > > > The workaround is implemented as follows: > > 1. For clocks running greater than or equal to 24MHz, a read followed > > by the write will provide sufficient delay. > > 2. For clocks running below 24MHz, add a delay of 4 clock cylces > after > > the write to the LPCG register. > > > > Fixes: 2f77296d3df9 ("clk: imx: add lpcg clock support") > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > --- > > drivers/clk/imx/clk-lpcg-scu.c | 35 > > +++++++++++++++++++++++++++++++---- > > 1 file changed, 31 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/clk/imx/clk-lpcg-scu.c > > b/drivers/clk/imx/clk-lpcg-scu.c index > > > dd5abd09f3e206a5073767561b517d5b3320b28c..cd42190233662c6 > 6f2c354f0a2ee > > e3a2531eeb3a 100644 > > --- a/drivers/clk/imx/clk-lpcg-scu.c > > +++ b/drivers/clk/imx/clk-lpcg-scu.c > > @@ -6,10 +6,12 @@ > > > > #include <linux/bits.h> > > #include <linux/clk-provider.h> > > +#include <linux/delay.h> > > #include <linux/err.h> > > #include <linux/io.h> > > #include <linux/slab.h> > > #include <linux/spinlock.h> > > +#include <linux/units.h> > > > > #include "clk-scu.h" > > > > @@ -41,6 +43,31 @@ struct clk_lpcg_scu { > > > > #define to_clk_lpcg_scu(_hw) container_of(_hw, struct clk_lpcg_scu, > > hw) > > > > +/* e10858 -LPCG clock gating register synchronization errata */ > > +static void lpcg_e10858_writel(unsigned long rate, void __iomem > *reg, > > +u32 val) { > > + u32 reg1; > > + > > + writel(val, reg); > > + > > + if (rate >= 24 * HZ_PER_MHZ || rate == 0) { > > + /* > > + * The time taken to access the LPCG registers from > the AP core > > + * through the interconnect is longer than the > minimum delay > > + * of 4 clock cycles required by the errata. > > + * Adding a readl will provide sufficient delay to > prevent > > + * back-to-back writes. > > + */ > > + reg1 = readl(reg); > > + } else { > > + /* > > + * For clocks running below 24MHz, wait a minimum > of > > + * 4 clock cycles. > > + */ > > + ndelay(4 * (DIV_ROUND_UP(1000 * HZ_PER_MHZ, > rate))); > > + } > > +} > > + > > static int clk_lpcg_scu_enable(struct clk_hw *hw) { > > struct clk_lpcg_scu *clk = to_clk_lpcg_scu(hw); @@ -57,7 > +84,8 @@ > > static int clk_lpcg_scu_enable(struct clk_hw *hw) > > val |= CLK_GATE_SCU_LPCG_HW_SEL; > > > > reg |= val << clk->bit_idx; > > - writel(reg, clk->reg); > > + > > + lpcg_e10858_writel(clk_hw_get_rate(hw), clk->reg, reg); > > > > spin_unlock_irqrestore(&imx_lpcg_scu_lock, flags); > > > > @@ -74,7 +102,7 @@ static void clk_lpcg_scu_disable(struct clk_hw > *hw) > > > > reg = readl_relaxed(clk->reg); > > reg &= ~(CLK_GATE_SCU_LPCG_MASK << clk->bit_idx); > > - writel(reg, clk->reg); > > + lpcg_e10858_writel(clk_hw_get_rate(hw), clk->reg, reg); > > > > spin_unlock_irqrestore(&imx_lpcg_scu_lock, flags); } @@ - > 149,9 > > +177,8 @@ static int __maybe_unused > imx_clk_lpcg_scu_resume(struct device *dev) > > * FIXME: Sometimes writes don't work unless the CPU issues > > * them twice > > */ > > - > > - writel(clk->state, clk->reg); > > Now that you removed one of the writes, doesn't the comment above > has to be removed as well ? Sure, let me remove it. Thanks, Peng > > > writel(clk->state, clk->reg); > > + lpcg_e10858_writel(0, clk->reg, clk->state); > > dev_dbg(dev, "restore lpcg state 0x%x\n", clk->state); > > > > return 0; > > > > -- > > 2.37.1 > >
diff --git a/drivers/clk/imx/clk-lpcg-scu.c b/drivers/clk/imx/clk-lpcg-scu.c index dd5abd09f3e206a5073767561b517d5b3320b28c..cd42190233662c66f2c354f0a2eee3a2531eeb3a 100644 --- a/drivers/clk/imx/clk-lpcg-scu.c +++ b/drivers/clk/imx/clk-lpcg-scu.c @@ -6,10 +6,12 @@ #include <linux/bits.h> #include <linux/clk-provider.h> +#include <linux/delay.h> #include <linux/err.h> #include <linux/io.h> #include <linux/slab.h> #include <linux/spinlock.h> +#include <linux/units.h> #include "clk-scu.h" @@ -41,6 +43,31 @@ struct clk_lpcg_scu { #define to_clk_lpcg_scu(_hw) container_of(_hw, struct clk_lpcg_scu, hw) +/* e10858 -LPCG clock gating register synchronization errata */ +static void lpcg_e10858_writel(unsigned long rate, void __iomem *reg, u32 val) +{ + u32 reg1; + + writel(val, reg); + + if (rate >= 24 * HZ_PER_MHZ || rate == 0) { + /* + * The time taken to access the LPCG registers from the AP core + * through the interconnect is longer than the minimum delay + * of 4 clock cycles required by the errata. + * Adding a readl will provide sufficient delay to prevent + * back-to-back writes. + */ + reg1 = readl(reg); + } else { + /* + * For clocks running below 24MHz, wait a minimum of + * 4 clock cycles. + */ + ndelay(4 * (DIV_ROUND_UP(1000 * HZ_PER_MHZ, rate))); + } +} + static int clk_lpcg_scu_enable(struct clk_hw *hw) { struct clk_lpcg_scu *clk = to_clk_lpcg_scu(hw); @@ -57,7 +84,8 @@ static int clk_lpcg_scu_enable(struct clk_hw *hw) val |= CLK_GATE_SCU_LPCG_HW_SEL; reg |= val << clk->bit_idx; - writel(reg, clk->reg); + + lpcg_e10858_writel(clk_hw_get_rate(hw), clk->reg, reg); spin_unlock_irqrestore(&imx_lpcg_scu_lock, flags); @@ -74,7 +102,7 @@ static void clk_lpcg_scu_disable(struct clk_hw *hw) reg = readl_relaxed(clk->reg); reg &= ~(CLK_GATE_SCU_LPCG_MASK << clk->bit_idx); - writel(reg, clk->reg); + lpcg_e10858_writel(clk_hw_get_rate(hw), clk->reg, reg); spin_unlock_irqrestore(&imx_lpcg_scu_lock, flags); } @@ -149,9 +177,8 @@ static int __maybe_unused imx_clk_lpcg_scu_resume(struct device *dev) * FIXME: Sometimes writes don't work unless the CPU issues * them twice */ - - writel(clk->state, clk->reg); writel(clk->state, clk->reg); + lpcg_e10858_writel(0, clk->reg, clk->state); dev_dbg(dev, "restore lpcg state 0x%x\n", clk->state); return 0;