* [PATCH v2 1/5] clk: amlogic: Fix out-of-range PLL frequency setting
2025-10-30 5:24 [PATCH v2 0/5] clk: amlogic: optimize the PLL driver Chuan Liu via B4 Relay
@ 2025-10-30 5:24 ` Chuan Liu via B4 Relay
2025-10-30 5:24 ` [PATCH v2 2/5] clk: amlogic: Improve the issue of PLL lock failures Chuan Liu via B4 Relay
` (3 subsequent siblings)
4 siblings, 0 replies; 15+ messages in thread
From: Chuan Liu via B4 Relay @ 2025-10-30 5:24 UTC (permalink / raw)
To: Neil Armstrong, Jerome Brunet, Michael Turquette, Stephen Boyd,
Kevin Hilman, Martin Blumenstingl
Cc: linux-amlogic, linux-clk, linux-arm-kernel, linux-kernel,
Chuan Liu, da
From: Chuan Liu <chuan.liu@amlogic.com>
If the calculated 'm' falls into the range:
pll->range->max < m < (1 << pll->m.width)
Here an incorrect 'm' value could be obtained, so an additional
condition is added to ensure that the calculated 'm' stays within a
valid range.
Fixes: 8eed1db1adec6 ("clk: meson: pll: update driver for the g12a")
Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
---
drivers/clk/meson/clk-pll.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
index 1ea6579a760f..629f6af18ea1 100644
--- a/drivers/clk/meson/clk-pll.c
+++ b/drivers/clk/meson/clk-pll.c
@@ -191,7 +191,7 @@ static int meson_clk_get_pll_range_index(unsigned long rate,
*m = meson_clk_get_pll_range_m(rate, parent_rate, *n, pll);
/* the pre-divider gives a multiplier too big - stop */
- if (*m >= (1 << pll->m.width))
+ if (*m > pll->range->max || *m >= (1 << pll->m.width))
return -EINVAL;
return 0;
--
2.42.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v2 2/5] clk: amlogic: Improve the issue of PLL lock failures
2025-10-30 5:24 [PATCH v2 0/5] clk: amlogic: optimize the PLL driver Chuan Liu via B4 Relay
2025-10-30 5:24 ` [PATCH v2 1/5] clk: amlogic: Fix out-of-range PLL frequency setting Chuan Liu via B4 Relay
@ 2025-10-30 5:24 ` Chuan Liu via B4 Relay
2025-10-30 8:41 ` Jerome Brunet
2025-10-30 5:24 ` [PATCH v2 3/5] clk: amlogic: Add handling for PLL lock failure Chuan Liu via B4 Relay
` (2 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Chuan Liu via B4 Relay @ 2025-10-30 5:24 UTC (permalink / raw)
To: Neil Armstrong, Jerome Brunet, Michael Turquette, Stephen Boyd,
Kevin Hilman, Martin Blumenstingl
Cc: linux-amlogic, linux-clk, linux-arm-kernel, linux-kernel,
Chuan Liu, da
From: Chuan Liu <chuan.liu@amlogic.com>
Due to factors such as temperature and process variations, the
internal circuits of the PLL may require a longer time to reach a
steady state, which can result in occasional lock failures on some
SoCs under low-temperature conditions.
After enabling the PLL and releasing its reset, a 20 us delay is
added at each step to provide enough time for the internal PLL
circuit to stabilize, thus reducing the probability of PLL lock
failure.
Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
---
drivers/clk/meson/clk-pll.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
index 629f6af18ea1..f81ebf6cc981 100644
--- a/drivers/clk/meson/clk-pll.c
+++ b/drivers/clk/meson/clk-pll.c
@@ -368,11 +368,16 @@ static int meson_clk_pll_enable(struct clk_hw *hw)
/* Enable the pll */
meson_parm_write(clk->map, &pll->en, 1);
+ /* Wait for Bandgap and LDO to power up and stabilize */
+ udelay(20);
/* Take the pll out reset */
if (MESON_PARM_APPLICABLE(&pll->rst))
meson_parm_write(clk->map, &pll->rst, 0);
+ /* Wait for PLL loop stabilization */
+ udelay(20);
+
/*
* Compared with the previous SoCs, self-adaption current module
* is newly added for A1, keep the new power-on sequence to enable the
--
2.42.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v2 2/5] clk: amlogic: Improve the issue of PLL lock failures
2025-10-30 5:24 ` [PATCH v2 2/5] clk: amlogic: Improve the issue of PLL lock failures Chuan Liu via B4 Relay
@ 2025-10-30 8:41 ` Jerome Brunet
2025-10-30 9:09 ` Chuan Liu
0 siblings, 1 reply; 15+ messages in thread
From: Jerome Brunet @ 2025-10-30 8:41 UTC (permalink / raw)
To: Chuan Liu via B4 Relay
Cc: Neil Armstrong, Michael Turquette, Stephen Boyd, Kevin Hilman,
Martin Blumenstingl, chuan.liu, linux-amlogic, linux-clk,
linux-arm-kernel, linux-kernel, da
On Thu 30 Oct 2025 at 13:24, Chuan Liu via B4 Relay <devnull+chuan.liu.amlogic.com@kernel.org> wrote:
> From: Chuan Liu <chuan.liu@amlogic.com>
>
> Due to factors such as temperature and process variations, the
> internal circuits of the PLL may require a longer time to reach a
> steady state, which can result in occasional lock failures on some
> SoCs under low-temperature conditions.
>
> After enabling the PLL and releasing its reset, a 20 us delay is
> added at each step to provide enough time for the internal PLL
> circuit to stabilize, thus reducing the probability of PLL lock
> failure.
>
> Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
> ---
> drivers/clk/meson/clk-pll.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
> index 629f6af18ea1..f81ebf6cc981 100644
> --- a/drivers/clk/meson/clk-pll.c
> +++ b/drivers/clk/meson/clk-pll.c
> @@ -368,11 +368,16 @@ static int meson_clk_pll_enable(struct clk_hw *hw)
>
> /* Enable the pll */
> meson_parm_write(clk->map, &pll->en, 1);
New line
> + /* Wait for Bandgap and LDO to power up and stabilize */
> + udelay(20);
>
> /* Take the pll out reset */
> if (MESON_PARM_APPLICABLE(&pll->rst))
> meson_parm_write(clk->map, &pll->rst, 0);
>
> + /* Wait for PLL loop stabilization */
> + udelay(20);
> +
> /*
> * Compared with the previous SoCs, self-adaption current module
> * is newly added for A1, keep the new power-on sequence to enable the
--
Jerome
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/5] clk: amlogic: Improve the issue of PLL lock failures
2025-10-30 8:41 ` Jerome Brunet
@ 2025-10-30 9:09 ` Chuan Liu
0 siblings, 0 replies; 15+ messages in thread
From: Chuan Liu @ 2025-10-30 9:09 UTC (permalink / raw)
To: Jerome Brunet, Chuan Liu via B4 Relay
Cc: Neil Armstrong, Michael Turquette, Stephen Boyd, Kevin Hilman,
Martin Blumenstingl, linux-amlogic, linux-clk, linux-arm-kernel,
linux-kernel, da
Hi Jerome,
On 10/30/2025 4:41 PM, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
> On Thu 30 Oct 2025 at 13:24, Chuan Liu via B4 Relay <devnull+chuan.liu.amlogic.com@kernel.org> wrote:
>
>> From: Chuan Liu <chuan.liu@amlogic.com>
>>
>> Due to factors such as temperature and process variations, the
>> internal circuits of the PLL may require a longer time to reach a
>> steady state, which can result in occasional lock failures on some
>> SoCs under low-temperature conditions.
>>
>> After enabling the PLL and releasing its reset, a 20 us delay is
>> added at each step to provide enough time for the internal PLL
>> circuit to stabilize, thus reducing the probability of PLL lock
>> failure.
>>
>> Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
>> ---
>> drivers/clk/meson/clk-pll.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
>> index 629f6af18ea1..f81ebf6cc981 100644
>> --- a/drivers/clk/meson/clk-pll.c
>> +++ b/drivers/clk/meson/clk-pll.c
>> @@ -368,11 +368,16 @@ static int meson_clk_pll_enable(struct clk_hw *hw)
>>
>> /* Enable the pll */
>> meson_parm_write(clk->map, &pll->en, 1);
>
> New line
Ok, fix it in the next version.
>
>> + /* Wait for Bandgap and LDO to power up and stabilize */
>> + udelay(20);
>>
>> /* Take the pll out reset */
>> if (MESON_PARM_APPLICABLE(&pll->rst))
>> meson_parm_write(clk->map, &pll->rst, 0);
>>
>> + /* Wait for PLL loop stabilization */
>> + udelay(20);
>> +
>> /*
>> * Compared with the previous SoCs, self-adaption current module
>> * is newly added for A1, keep the new power-on sequence to enable the
>
> --
> Jerome
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 3/5] clk: amlogic: Add handling for PLL lock failure
2025-10-30 5:24 [PATCH v2 0/5] clk: amlogic: optimize the PLL driver Chuan Liu via B4 Relay
2025-10-30 5:24 ` [PATCH v2 1/5] clk: amlogic: Fix out-of-range PLL frequency setting Chuan Liu via B4 Relay
2025-10-30 5:24 ` [PATCH v2 2/5] clk: amlogic: Improve the issue of PLL lock failures Chuan Liu via B4 Relay
@ 2025-10-30 5:24 ` Chuan Liu via B4 Relay
2025-10-30 8:32 ` Jerome Brunet
2025-10-30 5:24 ` [PATCH v2 4/5] clk: amlogic: Optimize PLL enable timing Chuan Liu via B4 Relay
2025-10-30 5:24 ` [PATCH v2 5/5] clk: amlogic: Change the active level of l_detect Chuan Liu via B4 Relay
4 siblings, 1 reply; 15+ messages in thread
From: Chuan Liu via B4 Relay @ 2025-10-30 5:24 UTC (permalink / raw)
To: Neil Armstrong, Jerome Brunet, Michael Turquette, Stephen Boyd,
Kevin Hilman, Martin Blumenstingl
Cc: linux-amlogic, linux-clk, linux-arm-kernel, linux-kernel,
Chuan Liu, da
From: Chuan Liu <chuan.liu@amlogic.com>
If the PLL fails to lock, it should be disabled, This makes the logic
more complete, and also helps save unnecessary power consumption when
the PLL is malfunctioning.
Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
---
drivers/clk/meson/clk-pll.c | 41 +++++++++++++++++++++++------------------
1 file changed, 23 insertions(+), 18 deletions(-)
diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
index f81ebf6cc981..6c794adb8ccd 100644
--- a/drivers/clk/meson/clk-pll.c
+++ b/drivers/clk/meson/clk-pll.c
@@ -353,6 +353,23 @@ static int meson_clk_pcie_pll_enable(struct clk_hw *hw)
return -EIO;
}
+static void meson_clk_pll_disable(struct clk_hw *hw)
+{
+ struct clk_regmap *clk = to_clk_regmap(hw);
+ struct meson_clk_pll_data *pll = meson_clk_pll_data(clk);
+
+ /* Put the pll is in reset */
+ if (MESON_PARM_APPLICABLE(&pll->rst))
+ meson_parm_write(clk->map, &pll->rst, 1);
+
+ /* Disable the pll */
+ meson_parm_write(clk->map, &pll->en, 0);
+
+ /* Disable PLL internal self-adaption current module */
+ if (MESON_PARM_APPLICABLE(&pll->current_en))
+ meson_parm_write(clk->map, &pll->current_en, 0);
+}
+
static int meson_clk_pll_enable(struct clk_hw *hw)
{
struct clk_regmap *clk = to_clk_regmap(hw);
@@ -397,29 +414,17 @@ static int meson_clk_pll_enable(struct clk_hw *hw)
meson_parm_write(clk->map, &pll->l_detect, 0);
}
- if (meson_clk_pll_wait_lock(hw))
+ if (meson_clk_pll_wait_lock(hw)) {
+ /* disable PLL when PLL lock failed. */
+ meson_clk_pll_disable(hw);
+ pr_warn("%s: PLL lock failed!!!\n", clk_hw_get_name(hw));
+
return -EIO;
+ }
return 0;
}
-static void meson_clk_pll_disable(struct clk_hw *hw)
-{
- struct clk_regmap *clk = to_clk_regmap(hw);
- struct meson_clk_pll_data *pll = meson_clk_pll_data(clk);
-
- /* Put the pll is in reset */
- if (MESON_PARM_APPLICABLE(&pll->rst))
- meson_parm_write(clk->map, &pll->rst, 1);
-
- /* Disable the pll */
- meson_parm_write(clk->map, &pll->en, 0);
-
- /* Disable PLL internal self-adaption current module */
- if (MESON_PARM_APPLICABLE(&pll->current_en))
- meson_parm_write(clk->map, &pll->current_en, 0);
-}
-
static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate,
unsigned long parent_rate)
{
--
2.42.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v2 3/5] clk: amlogic: Add handling for PLL lock failure
2025-10-30 5:24 ` [PATCH v2 3/5] clk: amlogic: Add handling for PLL lock failure Chuan Liu via B4 Relay
@ 2025-10-30 8:32 ` Jerome Brunet
2025-10-30 9:15 ` Chuan Liu
0 siblings, 1 reply; 15+ messages in thread
From: Jerome Brunet @ 2025-10-30 8:32 UTC (permalink / raw)
To: Chuan Liu via B4 Relay
Cc: Neil Armstrong, Michael Turquette, Stephen Boyd, Kevin Hilman,
Martin Blumenstingl, chuan.liu, linux-amlogic, linux-clk,
linux-arm-kernel, linux-kernel, da
On Thu 30 Oct 2025 at 13:24, Chuan Liu via B4 Relay <devnull+chuan.liu.amlogic.com@kernel.org> wrote:
> From: Chuan Liu <chuan.liu@amlogic.com>
>
> If the PLL fails to lock, it should be disabled, This makes the logic
> more complete, and also helps save unnecessary power consumption when
> the PLL is malfunctioning.
>
> Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
> ---
> drivers/clk/meson/clk-pll.c | 41 +++++++++++++++++++++++------------------
> 1 file changed, 23 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
> index f81ebf6cc981..6c794adb8ccd 100644
> --- a/drivers/clk/meson/clk-pll.c
> +++ b/drivers/clk/meson/clk-pll.c
> @@ -353,6 +353,23 @@ static int meson_clk_pcie_pll_enable(struct clk_hw *hw)
> return -EIO;
> }
>
> +static void meson_clk_pll_disable(struct clk_hw *hw)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct meson_clk_pll_data *pll = meson_clk_pll_data(clk);
> +
> + /* Put the pll is in reset */
> + if (MESON_PARM_APPLICABLE(&pll->rst))
> + meson_parm_write(clk->map, &pll->rst, 1);
> +
> + /* Disable the pll */
> + meson_parm_write(clk->map, &pll->en, 0);
> +
> + /* Disable PLL internal self-adaption current module */
> + if (MESON_PARM_APPLICABLE(&pll->current_en))
> + meson_parm_write(clk->map, &pll->current_en, 0);
> +}
> +
> static int meson_clk_pll_enable(struct clk_hw *hw)
> {
> struct clk_regmap *clk = to_clk_regmap(hw);
> @@ -397,29 +414,17 @@ static int meson_clk_pll_enable(struct clk_hw *hw)
> meson_parm_write(clk->map, &pll->l_detect, 0);
> }
>
> - if (meson_clk_pll_wait_lock(hw))
> + if (meson_clk_pll_wait_lock(hw)) {
> + /* disable PLL when PLL lock failed. */
> + meson_clk_pll_disable(hw);
> + pr_warn("%s: PLL lock failed!!!\n", clk_hw_get_name(hw));
> +
This is something that's likely to spam, so pr_warn_once() (if you must warn)
and I don't think 3 "!" are necessary to convey the message.
> return -EIO;
> + }
>
> return 0;
> }
>
> -static void meson_clk_pll_disable(struct clk_hw *hw)
> -{
> - struct clk_regmap *clk = to_clk_regmap(hw);
> - struct meson_clk_pll_data *pll = meson_clk_pll_data(clk);
> -
> - /* Put the pll is in reset */
> - if (MESON_PARM_APPLICABLE(&pll->rst))
> - meson_parm_write(clk->map, &pll->rst, 1);
> -
> - /* Disable the pll */
> - meson_parm_write(clk->map, &pll->en, 0);
> -
> - /* Disable PLL internal self-adaption current module */
> - if (MESON_PARM_APPLICABLE(&pll->current_en))
> - meson_parm_write(clk->map, &pll->current_en, 0);
> -}
> -
> static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate,
> unsigned long parent_rate)
> {
--
Jerome
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2 3/5] clk: amlogic: Add handling for PLL lock failure
2025-10-30 8:32 ` Jerome Brunet
@ 2025-10-30 9:15 ` Chuan Liu
2025-10-30 11:48 ` Chuan Liu
0 siblings, 1 reply; 15+ messages in thread
From: Chuan Liu @ 2025-10-30 9:15 UTC (permalink / raw)
To: Jerome Brunet, Chuan Liu via B4 Relay
Cc: Neil Armstrong, Michael Turquette, Stephen Boyd, Kevin Hilman,
Martin Blumenstingl, linux-amlogic, linux-clk, linux-arm-kernel,
linux-kernel, da
Hi Jerome,
On 10/30/2025 4:32 PM, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
> On Thu 30 Oct 2025 at 13:24, Chuan Liu via B4 Relay <devnull+chuan.liu.amlogic.com@kernel.org> wrote:
>
>> From: Chuan Liu <chuan.liu@amlogic.com>
>>
>> If the PLL fails to lock, it should be disabled, This makes the logic
>> more complete, and also helps save unnecessary power consumption when
>> the PLL is malfunctioning.
>>
>> Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
>> ---
>> drivers/clk/meson/clk-pll.c | 41 +++++++++++++++++++++++------------------
>> 1 file changed, 23 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
>> index f81ebf6cc981..6c794adb8ccd 100644
>> --- a/drivers/clk/meson/clk-pll.c
>> +++ b/drivers/clk/meson/clk-pll.c
>> @@ -353,6 +353,23 @@ static int meson_clk_pcie_pll_enable(struct clk_hw *hw)
>> return -EIO;
>> }
>>
>> +static void meson_clk_pll_disable(struct clk_hw *hw)
>> +{
>> + struct clk_regmap *clk = to_clk_regmap(hw);
>> + struct meson_clk_pll_data *pll = meson_clk_pll_data(clk);
>> +
>> + /* Put the pll is in reset */
>> + if (MESON_PARM_APPLICABLE(&pll->rst))
>> + meson_parm_write(clk->map, &pll->rst, 1);
>> +
>> + /* Disable the pll */
>> + meson_parm_write(clk->map, &pll->en, 0);
>> +
>> + /* Disable PLL internal self-adaption current module */
>> + if (MESON_PARM_APPLICABLE(&pll->current_en))
>> + meson_parm_write(clk->map, &pll->current_en, 0);
>> +}
>> +
>> static int meson_clk_pll_enable(struct clk_hw *hw)
>> {
>> struct clk_regmap *clk = to_clk_regmap(hw);
>> @@ -397,29 +414,17 @@ static int meson_clk_pll_enable(struct clk_hw *hw)
>> meson_parm_write(clk->map, &pll->l_detect, 0);
>> }
>>
>> - if (meson_clk_pll_wait_lock(hw))
>> + if (meson_clk_pll_wait_lock(hw)) {
>> + /* disable PLL when PLL lock failed. */
>> + meson_clk_pll_disable(hw);
>> + pr_warn("%s: PLL lock failed!!!\n", clk_hw_get_name(hw));
>> +
>
> This is something that's likely to spam, so pr_warn_once() (if you must warn)
> and I don't think 3 "!" are necessary to convey the message.
>
In the next version, I'll remove the "!", and replace pr_warn to
pr_warn_once.
Some drivers that reference the clock may not check the function’s
return value (even though that's not ideal), so adding this warning
makes the issue more noticeable.
>> return -EIO;
>> + }
>>
>> return 0;
>> }
>>
>> -static void meson_clk_pll_disable(struct clk_hw *hw)
>> -{
>> - struct clk_regmap *clk = to_clk_regmap(hw);
>> - struct meson_clk_pll_data *pll = meson_clk_pll_data(clk);
>> -
>> - /* Put the pll is in reset */
>> - if (MESON_PARM_APPLICABLE(&pll->rst))
>> - meson_parm_write(clk->map, &pll->rst, 1);
>> -
>> - /* Disable the pll */
>> - meson_parm_write(clk->map, &pll->en, 0);
>> -
>> - /* Disable PLL internal self-adaption current module */
>> - if (MESON_PARM_APPLICABLE(&pll->current_en))
>> - meson_parm_write(clk->map, &pll->current_en, 0);
>> -}
>> -
>> static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate,
>> unsigned long parent_rate)
>> {
>
> --
> Jerome
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2 3/5] clk: amlogic: Add handling for PLL lock failure
2025-10-30 9:15 ` Chuan Liu
@ 2025-10-30 11:48 ` Chuan Liu
0 siblings, 0 replies; 15+ messages in thread
From: Chuan Liu @ 2025-10-30 11:48 UTC (permalink / raw)
To: Jerome Brunet, Chuan Liu via B4 Relay
Cc: Neil Armstrong, Michael Turquette, Stephen Boyd, Kevin Hilman,
Martin Blumenstingl, linux-amlogic, linux-clk, linux-arm-kernel,
linux-kernel, da
Hi Jerome,
On 10/30/2025 5:15 PM, Chuan Liu wrote:
> Hi Jerome,
>
> On 10/30/2025 4:32 PM, Jerome Brunet wrote:
>> [ EXTERNAL EMAIL ]
>>
>> On Thu 30 Oct 2025 at 13:24, Chuan Liu via B4 Relay
>> <devnull+chuan.liu.amlogic.com@kernel.org> wrote:
>>
>>> From: Chuan Liu <chuan.liu@amlogic.com>
>>>
>>> If the PLL fails to lock, it should be disabled, This makes the logic
>>> more complete, and also helps save unnecessary power consumption when
>>> the PLL is malfunctioning.
>>>
>>> Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
>>> ---
>>> drivers/clk/meson/clk-pll.c | 41 ++++++++++++++++++++++
>>> +------------------
>>> 1 file changed, 23 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
>>> index f81ebf6cc981..6c794adb8ccd 100644
>>> --- a/drivers/clk/meson/clk-pll.c
>>> +++ b/drivers/clk/meson/clk-pll.c
>>> @@ -353,6 +353,23 @@ static int meson_clk_pcie_pll_enable(struct
>>> clk_hw *hw)
>>> return -EIO;
>>> }
>>>
>>> +static void meson_clk_pll_disable(struct clk_hw *hw)
>>> +{
>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>> + struct meson_clk_pll_data *pll = meson_clk_pll_data(clk);
>>> +
>>> + /* Put the pll is in reset */
>>> + if (MESON_PARM_APPLICABLE(&pll->rst))
>>> + meson_parm_write(clk->map, &pll->rst, 1);
>>> +
>>> + /* Disable the pll */
>>> + meson_parm_write(clk->map, &pll->en, 0);
>>> +
>>> + /* Disable PLL internal self-adaption current module */
>>> + if (MESON_PARM_APPLICABLE(&pll->current_en))
>>> + meson_parm_write(clk->map, &pll->current_en, 0);
>>> +}
>>> +
>>> static int meson_clk_pll_enable(struct clk_hw *hw)
>>> {
>>> struct clk_regmap *clk = to_clk_regmap(hw);
>>> @@ -397,29 +414,17 @@ static int meson_clk_pll_enable(struct clk_hw *hw)
>>> meson_parm_write(clk->map, &pll->l_detect, 0);
>>> }
>>>
>>> - if (meson_clk_pll_wait_lock(hw))
>>> + if (meson_clk_pll_wait_lock(hw)) {
>>> + /* disable PLL when PLL lock failed. */
>>> + meson_clk_pll_disable(hw);
>>> + pr_warn("%s: PLL lock failed!!!\n", clk_hw_get_name(hw));
>>> +
>>
>> This is something that's likely to spam, so pr_warn_once() (if you
>> must warn)
Different PLLs or different timing conditions may trigger PLL lock
failures repeatedly, so using pr_warn_once() might not achieve the
intended effect.
Do you mind keeping pr_warn() here?
>> and I don't think 3 "!" are necessary to convey the message.
>>
>
> In the next version, I'll remove the "!", and replace pr_warn to
> pr_warn_once.
>
> Some drivers that reference the clock may not check the function’s
> return value (even though that's not ideal), so adding this warning
> makes the issue more noticeable.
>
>>> return -EIO;
>>> + }
>>>
>>> return 0;
>>> }
>>>
>>> -static void meson_clk_pll_disable(struct clk_hw *hw)
>>> -{
>>> - struct clk_regmap *clk = to_clk_regmap(hw);
>>> - struct meson_clk_pll_data *pll = meson_clk_pll_data(clk);
>>> -
>>> - /* Put the pll is in reset */
>>> - if (MESON_PARM_APPLICABLE(&pll->rst))
>>> - meson_parm_write(clk->map, &pll->rst, 1);
>>> -
>>> - /* Disable the pll */
>>> - meson_parm_write(clk->map, &pll->en, 0);
>>> -
>>> - /* Disable PLL internal self-adaption current module */
>>> - if (MESON_PARM_APPLICABLE(&pll->current_en))
>>> - meson_parm_write(clk->map, &pll->current_en, 0);
>>> -}
>>> -
>>> static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long
>>> rate,
>>> unsigned long parent_rate)
>>> {
>>
>> --
>> Jerome
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 4/5] clk: amlogic: Optimize PLL enable timing
2025-10-30 5:24 [PATCH v2 0/5] clk: amlogic: optimize the PLL driver Chuan Liu via B4 Relay
` (2 preceding siblings ...)
2025-10-30 5:24 ` [PATCH v2 3/5] clk: amlogic: Add handling for PLL lock failure Chuan Liu via B4 Relay
@ 2025-10-30 5:24 ` Chuan Liu via B4 Relay
2025-10-30 8:38 ` Jerome Brunet
2025-10-30 5:24 ` [PATCH v2 5/5] clk: amlogic: Change the active level of l_detect Chuan Liu via B4 Relay
4 siblings, 1 reply; 15+ messages in thread
From: Chuan Liu via B4 Relay @ 2025-10-30 5:24 UTC (permalink / raw)
To: Neil Armstrong, Jerome Brunet, Michael Turquette, Stephen Boyd,
Kevin Hilman, Martin Blumenstingl
Cc: linux-amlogic, linux-clk, linux-arm-kernel, linux-kernel,
Chuan Liu, da
From: Chuan Liu <chuan.liu@amlogic.com>
l_detect controls the enablement of the PLL lock detection module.
It should remain disabled while the internal PLL circuits are
reaching a steady state; otherwise, the lock signal may be falsely
triggered high.
Before enabling the internal power supply of the PLL, l_detect should
be disabled. After the PLL’s internal circuits have stabilized,
l_detect should be enabled to prevent false lock signal triggers.
Currently, only A1 supports both l_detect and current_en, so this
patch will only affect A1.
Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
---
drivers/clk/meson/clk-pll.c | 28 +++++++++++++++-------------
1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
index 6c794adb8ccd..c6eebde1f516 100644
--- a/drivers/clk/meson/clk-pll.c
+++ b/drivers/clk/meson/clk-pll.c
@@ -383,36 +383,38 @@ static int meson_clk_pll_enable(struct clk_hw *hw)
if (MESON_PARM_APPLICABLE(&pll->rst))
meson_parm_write(clk->map, &pll->rst, 1);
+ /* Disable the PLL lock-detect module */
+ if (MESON_PARM_APPLICABLE(&pll->l_detect))
+ meson_parm_write(clk->map, &pll->l_detect, 1);
+
/* Enable the pll */
meson_parm_write(clk->map, &pll->en, 1);
/* Wait for Bandgap and LDO to power up and stabilize */
udelay(20);
- /* Take the pll out reset */
- if (MESON_PARM_APPLICABLE(&pll->rst))
- meson_parm_write(clk->map, &pll->rst, 0);
-
- /* Wait for PLL loop stabilization */
- udelay(20);
-
/*
* Compared with the previous SoCs, self-adaption current module
* is newly added for A1, keep the new power-on sequence to enable the
* PLL. The sequence is:
- * 1. enable the pll, delay for 10us
+ * 1. enable the pll, delay for 20us
* 2. enable the pll self-adaption current module, delay for 40us
* 3. enable the lock detect module
*/
if (MESON_PARM_APPLICABLE(&pll->current_en)) {
- udelay(10);
meson_parm_write(clk->map, &pll->current_en, 1);
- udelay(40);
+ udelay(20);
}
- if (MESON_PARM_APPLICABLE(&pll->l_detect)) {
- meson_parm_write(clk->map, &pll->l_detect, 1);
+ /* Take the pll out reset */
+ if (MESON_PARM_APPLICABLE(&pll->rst))
+ meson_parm_write(clk->map, &pll->rst, 0);
+
+ /* Wait for PLL loop stabilization */
+ udelay(20);
+
+ /* Enable the lock-detect module */
+ if (MESON_PARM_APPLICABLE(&pll->l_detect))
meson_parm_write(clk->map, &pll->l_detect, 0);
- }
if (meson_clk_pll_wait_lock(hw)) {
/* disable PLL when PLL lock failed. */
--
2.42.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v2 4/5] clk: amlogic: Optimize PLL enable timing
2025-10-30 5:24 ` [PATCH v2 4/5] clk: amlogic: Optimize PLL enable timing Chuan Liu via B4 Relay
@ 2025-10-30 8:38 ` Jerome Brunet
2025-10-30 9:23 ` Chuan Liu
0 siblings, 1 reply; 15+ messages in thread
From: Jerome Brunet @ 2025-10-30 8:38 UTC (permalink / raw)
To: Chuan Liu via B4 Relay
Cc: Neil Armstrong, Michael Turquette, Stephen Boyd, Kevin Hilman,
Martin Blumenstingl, chuan.liu, linux-amlogic, linux-clk,
linux-arm-kernel, linux-kernel, da
On Thu 30 Oct 2025 at 13:24, Chuan Liu via B4 Relay <devnull+chuan.liu.amlogic.com@kernel.org> wrote:
> From: Chuan Liu <chuan.liu@amlogic.com>
>
> l_detect controls the enablement of the PLL lock detection module.
> It should remain disabled while the internal PLL circuits are
> reaching a steady state; otherwise, the lock signal may be falsely
> triggered high.
>
> Before enabling the internal power supply of the PLL, l_detect should
> be disabled. After the PLL’s internal circuits have stabilized,
> l_detect should be enabled to prevent false lock signal triggers.
You to reformat this description. It feel that both paragraph are saying
the same thing.
>
> Currently, only A1 supports both l_detect and current_en, so this
> patch will only affect A1.
>
> Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
> ---
> drivers/clk/meson/clk-pll.c | 28 +++++++++++++++-------------
> 1 file changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
> index 6c794adb8ccd..c6eebde1f516 100644
> --- a/drivers/clk/meson/clk-pll.c
> +++ b/drivers/clk/meson/clk-pll.c
> @@ -383,36 +383,38 @@ static int meson_clk_pll_enable(struct clk_hw *hw)
> if (MESON_PARM_APPLICABLE(&pll->rst))
> meson_parm_write(clk->map, &pll->rst, 1);
>
> + /* Disable the PLL lock-detect module */
> + if (MESON_PARM_APPLICABLE(&pll->l_detect))
> + meson_parm_write(clk->map, &pll->l_detect, 1);
> +
> /* Enable the pll */
> meson_parm_write(clk->map, &pll->en, 1);
> /* Wait for Bandgap and LDO to power up and stabilize */
> udelay(20);
>
> - /* Take the pll out reset */
> - if (MESON_PARM_APPLICABLE(&pll->rst))
> - meson_parm_write(clk->map, &pll->rst, 0);
Why is the reset moving around ? nothing is said in the description about
that
> -
> - /* Wait for PLL loop stabilization */
> - udelay(20);
> -
> /*
> * Compared with the previous SoCs, self-adaption current module
> * is newly added for A1, keep the new power-on sequence to enable the
> * PLL. The sequence is:
> - * 1. enable the pll, delay for 10us
> + * 1. enable the pll, delay for 20us
> * 2. enable the pll self-adaption current module, delay for 40us
> * 3. enable the lock detect module
> */
> if (MESON_PARM_APPLICABLE(&pll->current_en)) {
> - udelay(10);
> meson_parm_write(clk->map, &pll->current_en, 1);
> - udelay(40);
> + udelay(20);
> }
>
> - if (MESON_PARM_APPLICABLE(&pll->l_detect)) {
> - meson_parm_write(clk->map, &pll->l_detect, 1);
> + /* Take the pll out reset */
> + if (MESON_PARM_APPLICABLE(&pll->rst))
> + meson_parm_write(clk->map, &pll->rst, 0);
> +
> + /* Wait for PLL loop stabilization */
> + udelay(20);
> +
> + /* Enable the lock-detect module */
> + if (MESON_PARM_APPLICABLE(&pll->l_detect))
> meson_parm_write(clk->map, &pll->l_detect, 0);
> - }
>
> if (meson_clk_pll_wait_lock(hw)) {
> /* disable PLL when PLL lock failed. */
--
Jerome
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2 4/5] clk: amlogic: Optimize PLL enable timing
2025-10-30 8:38 ` Jerome Brunet
@ 2025-10-30 9:23 ` Chuan Liu
0 siblings, 0 replies; 15+ messages in thread
From: Chuan Liu @ 2025-10-30 9:23 UTC (permalink / raw)
To: Jerome Brunet, Chuan Liu via B4 Relay
Cc: Neil Armstrong, Michael Turquette, Stephen Boyd, Kevin Hilman,
Martin Blumenstingl, linux-amlogic, linux-clk, linux-arm-kernel,
linux-kernel, da
Hi Jerome,
On 10/30/2025 4:38 PM, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
> On Thu 30 Oct 2025 at 13:24, Chuan Liu via B4 Relay <devnull+chuan.liu.amlogic.com@kernel.org> wrote:
>
>> From: Chuan Liu <chuan.liu@amlogic.com>
>>
>> l_detect controls the enablement of the PLL lock detection module.
>> It should remain disabled while the internal PLL circuits are
>> reaching a steady state; otherwise, the lock signal may be falsely
>> triggered high.
>>
>> Before enabling the internal power supply of the PLL, l_detect should
>> be disabled. After the PLL’s internal circuits have stabilized,
>> l_detect should be enabled to prevent false lock signal triggers.
>
> You to reformat this description. It feel that both paragraph are saying
> the same thing.
>
Ok, drop it.
>>
>> Currently, only A1 supports both l_detect and current_en, so this
>> patch will only affect A1.
>>
>> Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
>> ---
>> drivers/clk/meson/clk-pll.c | 28 +++++++++++++++-------------
>> 1 file changed, 15 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
>> index 6c794adb8ccd..c6eebde1f516 100644
>> --- a/drivers/clk/meson/clk-pll.c
>> +++ b/drivers/clk/meson/clk-pll.c
>> @@ -383,36 +383,38 @@ static int meson_clk_pll_enable(struct clk_hw *hw)
>> if (MESON_PARM_APPLICABLE(&pll->rst))
>> meson_parm_write(clk->map, &pll->rst, 1);
>>
>> + /* Disable the PLL lock-detect module */
>> + if (MESON_PARM_APPLICABLE(&pll->l_detect))
>> + meson_parm_write(clk->map, &pll->l_detect, 1);
>> +
>> /* Enable the pll */
>> meson_parm_write(clk->map, &pll->en, 1);
>> /* Wait for Bandgap and LDO to power up and stabilize */
>> udelay(20);
>>
>> - /* Take the pll out reset */
>> - if (MESON_PARM_APPLICABLE(&pll->rst))
>> - meson_parm_write(clk->map, &pll->rst, 0);
>
> Why is the reset moving around ? nothing is said in the description about
> that
>
The function of current_en in the PLL is similar to en. It's more
reliable to assert the reset signal after current_en is set high.
I'll update the commit description accordingly in the next version.
>> -
>> - /* Wait for PLL loop stabilization */
>> - udelay(20);
>> -
>> /*
>> * Compared with the previous SoCs, self-adaption current module
>> * is newly added for A1, keep the new power-on sequence to enable the
>> * PLL. The sequence is:
>> - * 1. enable the pll, delay for 10us
>> + * 1. enable the pll, delay for 20us
>> * 2. enable the pll self-adaption current module, delay for 40us
>> * 3. enable the lock detect module
>> */
>> if (MESON_PARM_APPLICABLE(&pll->current_en)) {
>> - udelay(10);
>> meson_parm_write(clk->map, &pll->current_en, 1);
>> - udelay(40);
>> + udelay(20);
>> }
>>
>> - if (MESON_PARM_APPLICABLE(&pll->l_detect)) {
>> - meson_parm_write(clk->map, &pll->l_detect, 1);
>> + /* Take the pll out reset */
>> + if (MESON_PARM_APPLICABLE(&pll->rst))
>> + meson_parm_write(clk->map, &pll->rst, 0);
>> +
>> + /* Wait for PLL loop stabilization */
>> + udelay(20);
>> +
>> + /* Enable the lock-detect module */
>> + if (MESON_PARM_APPLICABLE(&pll->l_detect))
>> meson_parm_write(clk->map, &pll->l_detect, 0);
>> - }
>>
>> if (meson_clk_pll_wait_lock(hw)) {
>> /* disable PLL when PLL lock failed. */
>
> --
> Jerome
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 5/5] clk: amlogic: Change the active level of l_detect
2025-10-30 5:24 [PATCH v2 0/5] clk: amlogic: optimize the PLL driver Chuan Liu via B4 Relay
` (3 preceding siblings ...)
2025-10-30 5:24 ` [PATCH v2 4/5] clk: amlogic: Optimize PLL enable timing Chuan Liu via B4 Relay
@ 2025-10-30 5:24 ` Chuan Liu via B4 Relay
2025-10-30 8:40 ` Jerome Brunet
4 siblings, 1 reply; 15+ messages in thread
From: Chuan Liu via B4 Relay @ 2025-10-30 5:24 UTC (permalink / raw)
To: Neil Armstrong, Jerome Brunet, Michael Turquette, Stephen Boyd,
Kevin Hilman, Martin Blumenstingl
Cc: linux-amlogic, linux-clk, linux-arm-kernel, linux-kernel,
Chuan Liu, da
From: Chuan Liu <chuan.liu@amlogic.com>
l_detect controls the enable/disable of the PLL lock-detect module.
The enable signal is normally active-high. This design ensures that
the module remains disabled during the power-on process, preventing
power fluctuations from affecting its operating state.
For A1, the l_detect signal is active-low:
0 -> Enable lock-detect module;
1 -> Disable lock-detect module.
Here, a flag CLK_MESON_PLL_L_DETECT_N is added to handle cases like
A1, where the signal is active-low.
Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
---
drivers/clk/meson/a1-pll.c | 1 +
drivers/clk/meson/clk-pll.c | 16 ++++++++++++----
drivers/clk/meson/clk-pll.h | 2 ++
3 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/clk/meson/a1-pll.c b/drivers/clk/meson/a1-pll.c
index 1f82e9c7c14e..bfe559c71402 100644
--- a/drivers/clk/meson/a1-pll.c
+++ b/drivers/clk/meson/a1-pll.c
@@ -137,6 +137,7 @@ static struct clk_regmap a1_hifi_pll = {
.range = &a1_hifi_pll_range,
.init_regs = a1_hifi_pll_init_regs,
.init_count = ARRAY_SIZE(a1_hifi_pll_init_regs),
+ .flags = CLK_MESON_PLL_L_DETECT_N
},
.hw.init = &(struct clk_init_data){
.name = "hifi_pll",
diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
index c6eebde1f516..d729e933aa1c 100644
--- a/drivers/clk/meson/clk-pll.c
+++ b/drivers/clk/meson/clk-pll.c
@@ -384,8 +384,12 @@ static int meson_clk_pll_enable(struct clk_hw *hw)
meson_parm_write(clk->map, &pll->rst, 1);
/* Disable the PLL lock-detect module */
- if (MESON_PARM_APPLICABLE(&pll->l_detect))
- meson_parm_write(clk->map, &pll->l_detect, 1);
+ if (MESON_PARM_APPLICABLE(&pll->l_detect)) {
+ if (pll->flags & CLK_MESON_PLL_L_DETECT_N)
+ meson_parm_write(clk->map, &pll->l_detect, 1);
+ else
+ meson_parm_write(clk->map, &pll->l_detect, 0);
+ }
/* Enable the pll */
meson_parm_write(clk->map, &pll->en, 1);
@@ -413,8 +417,12 @@ static int meson_clk_pll_enable(struct clk_hw *hw)
udelay(20);
/* Enable the lock-detect module */
- if (MESON_PARM_APPLICABLE(&pll->l_detect))
- meson_parm_write(clk->map, &pll->l_detect, 0);
+ if (MESON_PARM_APPLICABLE(&pll->l_detect)) {
+ if (pll->flags & CLK_MESON_PLL_L_DETECT_N)
+ meson_parm_write(clk->map, &pll->l_detect, 0);
+ else
+ meson_parm_write(clk->map, &pll->l_detect, 1);
+ }
if (meson_clk_pll_wait_lock(hw)) {
/* disable PLL when PLL lock failed. */
diff --git a/drivers/clk/meson/clk-pll.h b/drivers/clk/meson/clk-pll.h
index 949157fb7bf5..83295a24721f 100644
--- a/drivers/clk/meson/clk-pll.h
+++ b/drivers/clk/meson/clk-pll.h
@@ -29,6 +29,8 @@ struct pll_mult_range {
#define CLK_MESON_PLL_ROUND_CLOSEST BIT(0)
#define CLK_MESON_PLL_NOINIT_ENABLED BIT(1)
+/* l_detect signal is active-low */
+#define CLK_MESON_PLL_L_DETECT_N BIT(2)
struct meson_clk_pll_data {
struct parm en;
--
2.42.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v2 5/5] clk: amlogic: Change the active level of l_detect
2025-10-30 5:24 ` [PATCH v2 5/5] clk: amlogic: Change the active level of l_detect Chuan Liu via B4 Relay
@ 2025-10-30 8:40 ` Jerome Brunet
2025-10-30 9:27 ` Chuan Liu
0 siblings, 1 reply; 15+ messages in thread
From: Jerome Brunet @ 2025-10-30 8:40 UTC (permalink / raw)
To: Chuan Liu via B4 Relay
Cc: Neil Armstrong, Michael Turquette, Stephen Boyd, Kevin Hilman,
Martin Blumenstingl, chuan.liu, linux-amlogic, linux-clk,
linux-arm-kernel, linux-kernel, da
On Thu 30 Oct 2025 at 13:24, Chuan Liu via B4 Relay <devnull+chuan.liu.amlogic.com@kernel.org> wrote:
> From: Chuan Liu <chuan.liu@amlogic.com>
>
> l_detect controls the enable/disable of the PLL lock-detect module.
>
> The enable signal is normally active-high. This design ensures that
> the module remains disabled during the power-on process, preventing
> power fluctuations from affecting its operating state.
>
> For A1, the l_detect signal is active-low:
> 0 -> Enable lock-detect module;
> 1 -> Disable lock-detect module.
>
> Here, a flag CLK_MESON_PLL_L_DETECT_N is added to handle cases like
> A1, where the signal is active-low.
Given that A1 is the only ship supporting l_detect, this change is not
necessary.
Not now at least.
>
> Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
> ---
> drivers/clk/meson/a1-pll.c | 1 +
> drivers/clk/meson/clk-pll.c | 16 ++++++++++++----
> drivers/clk/meson/clk-pll.h | 2 ++
> 3 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/clk/meson/a1-pll.c b/drivers/clk/meson/a1-pll.c
> index 1f82e9c7c14e..bfe559c71402 100644
> --- a/drivers/clk/meson/a1-pll.c
> +++ b/drivers/clk/meson/a1-pll.c
> @@ -137,6 +137,7 @@ static struct clk_regmap a1_hifi_pll = {
> .range = &a1_hifi_pll_range,
> .init_regs = a1_hifi_pll_init_regs,
> .init_count = ARRAY_SIZE(a1_hifi_pll_init_regs),
> + .flags = CLK_MESON_PLL_L_DETECT_N
> },
> .hw.init = &(struct clk_init_data){
> .name = "hifi_pll",
> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
> index c6eebde1f516..d729e933aa1c 100644
> --- a/drivers/clk/meson/clk-pll.c
> +++ b/drivers/clk/meson/clk-pll.c
> @@ -384,8 +384,12 @@ static int meson_clk_pll_enable(struct clk_hw *hw)
> meson_parm_write(clk->map, &pll->rst, 1);
>
> /* Disable the PLL lock-detect module */
> - if (MESON_PARM_APPLICABLE(&pll->l_detect))
> - meson_parm_write(clk->map, &pll->l_detect, 1);
> + if (MESON_PARM_APPLICABLE(&pll->l_detect)) {
> + if (pll->flags & CLK_MESON_PLL_L_DETECT_N)
> + meson_parm_write(clk->map, &pll->l_detect, 1);
> + else
> + meson_parm_write(clk->map, &pll->l_detect, 0);
> + }
>
> /* Enable the pll */
> meson_parm_write(clk->map, &pll->en, 1);
> @@ -413,8 +417,12 @@ static int meson_clk_pll_enable(struct clk_hw *hw)
> udelay(20);
>
> /* Enable the lock-detect module */
> - if (MESON_PARM_APPLICABLE(&pll->l_detect))
> - meson_parm_write(clk->map, &pll->l_detect, 0);
> + if (MESON_PARM_APPLICABLE(&pll->l_detect)) {
> + if (pll->flags & CLK_MESON_PLL_L_DETECT_N)
> + meson_parm_write(clk->map, &pll->l_detect, 0);
> + else
> + meson_parm_write(clk->map, &pll->l_detect, 1);
> + }
>
> if (meson_clk_pll_wait_lock(hw)) {
> /* disable PLL when PLL lock failed. */
> diff --git a/drivers/clk/meson/clk-pll.h b/drivers/clk/meson/clk-pll.h
> index 949157fb7bf5..83295a24721f 100644
> --- a/drivers/clk/meson/clk-pll.h
> +++ b/drivers/clk/meson/clk-pll.h
> @@ -29,6 +29,8 @@ struct pll_mult_range {
>
> #define CLK_MESON_PLL_ROUND_CLOSEST BIT(0)
> #define CLK_MESON_PLL_NOINIT_ENABLED BIT(1)
> +/* l_detect signal is active-low */
> +#define CLK_MESON_PLL_L_DETECT_N BIT(2)
>
> struct meson_clk_pll_data {
> struct parm en;
--
Jerome
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2 5/5] clk: amlogic: Change the active level of l_detect
2025-10-30 8:40 ` Jerome Brunet
@ 2025-10-30 9:27 ` Chuan Liu
0 siblings, 0 replies; 15+ messages in thread
From: Chuan Liu @ 2025-10-30 9:27 UTC (permalink / raw)
To: Jerome Brunet, Chuan Liu via B4 Relay
Cc: Neil Armstrong, Michael Turquette, Stephen Boyd, Kevin Hilman,
Martin Blumenstingl, linux-amlogic, linux-clk, linux-arm-kernel,
linux-kernel, da
Hi Jerome,
On 10/30/2025 4:40 PM, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
> On Thu 30 Oct 2025 at 13:24, Chuan Liu via B4 Relay <devnull+chuan.liu.amlogic.com@kernel.org> wrote:
>
>> From: Chuan Liu <chuan.liu@amlogic.com>
>>
>> l_detect controls the enable/disable of the PLL lock-detect module.
>>
>> The enable signal is normally active-high. This design ensures that
>> the module remains disabled during the power-on process, preventing
>> power fluctuations from affecting its operating state.
>>
>> For A1, the l_detect signal is active-low:
>> 0 -> Enable lock-detect module;
>> 1 -> Disable lock-detect module.
>>
>> Here, a flag CLK_MESON_PLL_L_DETECT_N is added to handle cases like
>> A1, where the signal is active-low.
>
> Given that A1 is the only ship supporting l_detect, this change is not
> necessary.
>
> Not now at least.
>
The SoCs queued for upstreaming later will depend on this patch, so I
included it in this patchset.
If that’s not appropriate, would it be acceptable for me to include
it in the A4 submission instead?
>>
>> Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
>> ---
>> drivers/clk/meson/a1-pll.c | 1 +
>> drivers/clk/meson/clk-pll.c | 16 ++++++++++++----
>> drivers/clk/meson/clk-pll.h | 2 ++
>> 3 files changed, 15 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/clk/meson/a1-pll.c b/drivers/clk/meson/a1-pll.c
>> index 1f82e9c7c14e..bfe559c71402 100644
>> --- a/drivers/clk/meson/a1-pll.c
>> +++ b/drivers/clk/meson/a1-pll.c
>> @@ -137,6 +137,7 @@ static struct clk_regmap a1_hifi_pll = {
>> .range = &a1_hifi_pll_range,
>> .init_regs = a1_hifi_pll_init_regs,
>> .init_count = ARRAY_SIZE(a1_hifi_pll_init_regs),
>> + .flags = CLK_MESON_PLL_L_DETECT_N
>> },
>> .hw.init = &(struct clk_init_data){
>> .name = "hifi_pll",
>> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
>> index c6eebde1f516..d729e933aa1c 100644
>> --- a/drivers/clk/meson/clk-pll.c
>> +++ b/drivers/clk/meson/clk-pll.c
>> @@ -384,8 +384,12 @@ static int meson_clk_pll_enable(struct clk_hw *hw)
>> meson_parm_write(clk->map, &pll->rst, 1);
>>
>> /* Disable the PLL lock-detect module */
>> - if (MESON_PARM_APPLICABLE(&pll->l_detect))
>> - meson_parm_write(clk->map, &pll->l_detect, 1);
>> + if (MESON_PARM_APPLICABLE(&pll->l_detect)) {
>> + if (pll->flags & CLK_MESON_PLL_L_DETECT_N)
>> + meson_parm_write(clk->map, &pll->l_detect, 1);
>> + else
>> + meson_parm_write(clk->map, &pll->l_detect, 0);
>> + }
>>
>> /* Enable the pll */
>> meson_parm_write(clk->map, &pll->en, 1);
>> @@ -413,8 +417,12 @@ static int meson_clk_pll_enable(struct clk_hw *hw)
>> udelay(20);
>>
>> /* Enable the lock-detect module */
>> - if (MESON_PARM_APPLICABLE(&pll->l_detect))
>> - meson_parm_write(clk->map, &pll->l_detect, 0);
>> + if (MESON_PARM_APPLICABLE(&pll->l_detect)) {
>> + if (pll->flags & CLK_MESON_PLL_L_DETECT_N)
>> + meson_parm_write(clk->map, &pll->l_detect, 0);
>> + else
>> + meson_parm_write(clk->map, &pll->l_detect, 1);
>> + }
>>
>> if (meson_clk_pll_wait_lock(hw)) {
>> /* disable PLL when PLL lock failed. */
>> diff --git a/drivers/clk/meson/clk-pll.h b/drivers/clk/meson/clk-pll.h
>> index 949157fb7bf5..83295a24721f 100644
>> --- a/drivers/clk/meson/clk-pll.h
>> +++ b/drivers/clk/meson/clk-pll.h
>> @@ -29,6 +29,8 @@ struct pll_mult_range {
>>
>> #define CLK_MESON_PLL_ROUND_CLOSEST BIT(0)
>> #define CLK_MESON_PLL_NOINIT_ENABLED BIT(1)
>> +/* l_detect signal is active-low */
>> +#define CLK_MESON_PLL_L_DETECT_N BIT(2)
>>
>> struct meson_clk_pll_data {
>> struct parm en;
>
> --
> Jerome
^ permalink raw reply [flat|nested] 15+ messages in thread