[PATCH v2 0/4] clk: imx: scu and fracn pll update

[PATCH v2 0/4] clk: imx: scu and fracn pll update

[Peng Fan (OSS)]

Patch 1 is to resubmit [1] with comments addressed.
Patch 2 is for hdmi. Fix tag not needed.
Patch 3 and Patch 4 are for i.MX93 Fracn PLL fix.
In Patch 4:
fence_write is reusing the idea of fence_udelay in drivers/clk/tegra/clk.h

Detailed information is in commit log of each patch.

[1] https://lore.kernel.org/lkml/20240228082649.1633083-1-peng.fan@oss.nxp.com/

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
Changes in v2:
- Patch 1: use HZ_PER_MHZ, use unsigned long, add errata link
- Patch 4: Drop fence_write, just use readl
- Link to v1: https://lore.kernel.org/r/20241014-imx-clk-v1-v1-0-ee75876d3102@nxp.com

---
Peng Fan (4):
      clk: imx: lpcg-scu: SW workaround for errata (e10858)
      clk: imx: lpcg-scu: Skip HDMI LPCG clock save/restore
      clk: imx: fracn-gppll: correct PLL initialization flow
      clk: imx: fracn-gppll: fix pll power up

 drivers/clk/imx/clk-fracn-gppll.c | 10 +++++++---
 drivers/clk/imx/clk-lpcg-scu.c    | 41 +++++++++++++++++++++++++++++++++++----
 2 files changed, 44 insertions(+), 7 deletions(-)
---
base-commit: d61a00525464bfc5fe92c6ad713350988e492b88
change-id: 20241014-imx-clk-v1-24c26cfa1d79

Best regards,
-- 
Peng Fan <peng.fan@nxp.com>

[PATCH v2 1/4] clk: imx: lpcg-scu: SW workaround for errata (e10858)

[Peng Fan (OSS)]

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);
 	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

[PATCH v2 2/4] clk: imx: lpcg-scu: Skip HDMI LPCG clock save/restore

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

On i.MX8QM, HDMI LPCG clocks operation needs SCU clock "hdmi_ipg_clk"
to be ON. While during noirq suspend phase, "hdmi_ipg_clk" is disabled
by HDMI IRQ STEER driver, so SError will be triggered when accessing
the HDMI LPCG registers.

Skip all HDMI LPCG clocks save/restore to avoid SError during
system suspend/resume, it will NOT introduce additional power consumption
as their parent clock is disabled when suspend.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/clk/imx/clk-lpcg-scu.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/clk/imx/clk-lpcg-scu.c b/drivers/clk/imx/clk-lpcg-scu.c
index cd42190233662c66f2c354f0a2eee3a2531eeb3a..07deadcf73e929841fe113c7febfb6c9b6c5e719 100644
--- a/drivers/clk/imx/clk-lpcg-scu.c
+++ b/drivers/clk/imx/clk-lpcg-scu.c
@@ -163,6 +163,9 @@ static int __maybe_unused imx_clk_lpcg_scu_suspend(struct device *dev)
 {
 	struct clk_lpcg_scu *clk = dev_get_drvdata(dev);
 
+	if (!strncmp("hdmi_lpcg", clk_hw_get_name(&clk->hw), strlen("hdmi_lpcg")))
+		return 0;
+
 	clk->state = readl_relaxed(clk->reg);
 	dev_dbg(dev, "save lpcg state 0x%x\n", clk->state);
 
@@ -173,6 +176,9 @@ static int __maybe_unused imx_clk_lpcg_scu_resume(struct device *dev)
 {
 	struct clk_lpcg_scu *clk = dev_get_drvdata(dev);
 
+	if (!strncmp("hdmi_lpcg", clk_hw_get_name(&clk->hw), strlen("hdmi_lpcg")))
+		return 0;
+
 	/*
 	 * FIXME: Sometimes writes don't work unless the CPU issues
 	 * them twice

-- 
2.37.1

[PATCH v2 3/4] clk: imx: fracn-gppll: correct PLL initialization flow

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Per i.MX93 Reference Mannual 22.4 Initialization information
1. Program appropriate value of DIV[ODIV], DIV[RDIV] and DIV[MFI]
   as per Integer mode.
2. Wait for 5 μs.
3. Program the following field in CTRL register.
   Set CTRL[POWERUP] to 1'b1 to enable PLL block.
4. Poll PLL_STATUS[PLL_LOCK] register, and wait till PLL_STATUS[PLL_LOCK]
   is 1'b1 and pll_lock output signal is 1'b1.
5. Set CTRL[CLKMUX_EN] to 1'b1 to enable PLL output clock.

So move the CLKMUX_EN operation after PLL locked.

Fixes: 1b26cb8a77a4 ("clk: imx: support fracn gppll")
Co-developed-by: Jacky Bai <ping.bai@nxp.com>
Signed-off-by: Jacky Bai <ping.bai@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/clk/imx/clk-fracn-gppll.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/imx/clk-fracn-gppll.c b/drivers/clk/imx/clk-fracn-gppll.c
index 591e0364ee5c113859a7b6271c8c11e98a0e0ffc..4749c3e0b7051cf53876664808aa28742f6861f7 100644
--- a/drivers/clk/imx/clk-fracn-gppll.c
+++ b/drivers/clk/imx/clk-fracn-gppll.c
@@ -303,13 +303,13 @@ static int clk_fracn_gppll_prepare(struct clk_hw *hw)
 	val |= POWERUP_MASK;
 	writel_relaxed(val, pll->base + PLL_CTRL);
 
-	val |= CLKMUX_EN;
-	writel_relaxed(val, pll->base + PLL_CTRL);
-
 	ret = clk_fracn_gppll_wait_lock(pll);
 	if (ret)
 		return ret;
 
+	val |= CLKMUX_EN;
+	writel_relaxed(val, pll->base + PLL_CTRL);
+
 	val &= ~CLKMUX_BYPASS;
 	writel_relaxed(val, pll->base + PLL_CTRL);
 

-- 
2.37.1

[PATCH v2 4/4] clk: imx: fracn-gppll: fix pll power up

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

To i.MX93 which features dual Cortex-A55 cores and DSU, when using
writel_relaxed to write value to PLL registers, the value might be
buffered. To make sure the value has been written into the hardware,
using readl to read back the register could achieve the goal.

current PLL power up flow can be simplified as below:
  1. writel_relaxed to set the PLL POWERUP bit;
  2. readl_poll_timeout to check the PLL lock bit:
     a). timeout = ktime_add_us(ktime_get(), timeout_us);
     b). readl the pll the lock reg;
     c). check if the pll lock bit ready
     d). check if timeout

But in some corner cases, both the write in step 1 and read in
step 2 will be blocked by other bus transaction in the SoC for a
long time, saying the value into real hardware is just before step b).
That means the timeout counting has begins for quite sometime since
step a), but value still not written into real hardware until bus
released just at a point before step b).

Then there maybe chances that the pll lock bit is not ready
when readl done but the timeout happens. readl_poll_timeout will
err return due to timeout. To avoid such unexpected failure,
read back the reg to make sure the write has been done in HW
reg.

So use readl after writel_relaxed to fix the issue.

Since we are here, to avoid udelay to run before writel_relaxed, use
readl before udelay.

Fixes: 1b26cb8a77a4 ("clk: imx: support fracn gppll")
Co-developed-by: Jacky Bai <ping.bai@nxp.com>
Signed-off-by: Jacky Bai <ping.bai@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/clk/imx/clk-fracn-gppll.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/clk/imx/clk-fracn-gppll.c b/drivers/clk/imx/clk-fracn-gppll.c
index 4749c3e0b7051cf53876664808aa28742f6861f7..85771afd4698ae6a0d8a7e82193301e187049255 100644
--- a/drivers/clk/imx/clk-fracn-gppll.c
+++ b/drivers/clk/imx/clk-fracn-gppll.c
@@ -254,9 +254,11 @@ static int clk_fracn_gppll_set_rate(struct clk_hw *hw, unsigned long drate,
 	pll_div = FIELD_PREP(PLL_RDIV_MASK, rate->rdiv) | rate->odiv |
 		FIELD_PREP(PLL_MFI_MASK, rate->mfi);
 	writel_relaxed(pll_div, pll->base + PLL_DIV);
+	readl(pll->base + PLL_DIV);
 	if (pll->flags & CLK_FRACN_GPPLL_FRACN) {
 		writel_relaxed(rate->mfd, pll->base + PLL_DENOMINATOR);
 		writel_relaxed(FIELD_PREP(PLL_MFN_MASK, rate->mfn), pll->base + PLL_NUMERATOR);
+		readl(pll->base + PLL_NUMERATOR);
 	}
 
 	/* Wait for 5us according to fracn mode pll doc */
@@ -265,6 +267,7 @@ static int clk_fracn_gppll_set_rate(struct clk_hw *hw, unsigned long drate,
 	/* Enable Powerup */
 	tmp |= POWERUP_MASK;
 	writel_relaxed(tmp, pll->base + PLL_CTRL);
+	readl(pll->base + PLL_CTRL);
 
 	/* Wait Lock */
 	ret = clk_fracn_gppll_wait_lock(pll);
@@ -302,6 +305,7 @@ static int clk_fracn_gppll_prepare(struct clk_hw *hw)
 
 	val |= POWERUP_MASK;
 	writel_relaxed(val, pll->base + PLL_CTRL);
+	readl(pll->base + PLL_CTRL);
 
 	ret = clk_fracn_gppll_wait_lock(pll);
 	if (ret)

-- 
2.37.1

Re: [PATCH v2 1/4] clk: imx: lpcg-scu: SW workaround for errata (e10858)

[Abel Vesa]

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
> 

Re: [PATCH v2 1/4] clk: imx: lpcg-scu: SW workaround for errata (e10858)

[kernel test robot]

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	

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

RE: [PATCH v2 1/4] clk: imx: lpcg-scu: SW workaround for errata (e10858)

[Peng Fan]

> 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
> >