Re: [PATCH 05/10] clk: amlogic: PLL l_detect signal supports active-high configuration

[Jian Hu <jian.hu@amlogic.com>]

On 5/20/2026 3:24 PM, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
> On mer. 20 mai 2026 at 11:25, Jian Hu <jian.hu@amlogic.com> wrote:
>
>> On 5/14/2026 11:13 PM, Jerome Brunet wrote:
>>> [ EXTERNAL EMAIL ]
>>>
>>> On lun. 11 mai 2026 at 20:47, Jian Hu via B4 Relay <devnull+jian.hu.amlogic.com@kernel.org> wrote:
>>>
>>>> From: Jian Hu <jian.hu@amlogic.com>
>>>>
>>>> l_detect controls the enable/disable of the PLL lock-detect module.
>>>>
>>>> For A9, the l_detect signal is active-high:
>>>> 0 -> Disable lock-detect module;
>>>> 1 -> Enable lock-detect module.
>>>>
>>>> Here, a flag CLK_MESON_PLL_L_DETECT_ACTIVE_HIGH is added to handle cases
>>>> like A9, where the signal is active-high.
>>>>
>>>> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
>>>> ---
>>>>    drivers/clk/meson/clk-pll.c | 9 +++++++--
>>>>    drivers/clk/meson/clk-pll.h | 2 ++
>>>>    2 files changed, 9 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
>>>> index 1ea6579a760f..5a0bd75f85a9 100644
>>>> --- a/drivers/clk/meson/clk-pll.c
>>>> +++ b/drivers/clk/meson/clk-pll.c
>>>> @@ -388,8 +388,13 @@ static int meson_clk_pll_enable(struct clk_hw *hw)
>>>>         }
>>>>
>>>>         if (MESON_PARM_APPLICABLE(&pll->l_detect)) {
>>>> -             meson_parm_write(clk->map, &pll->l_detect, 1);
>>>> -             meson_parm_write(clk->map, &pll->l_detect, 0);
>>>> +             if (pll->flags & CLK_MESON_PLL_L_DETECT_ACTIVE_HIGH) {
>>>> +                     meson_parm_write(clk->map, &pll->l_detect, 0);
>>>> +                     meson_parm_write(clk->map, &pll->l_detect, 1);
>>>> +             } else {
>>>> +                     meson_parm_write(clk->map, &pll->l_detect, 1);
>>>> +                     meson_parm_write(clk->map, &pll->l_detect, 0);
>>>> +             }
>>> I'm not a fan of this code duplication.
>>> Use the introduced CLK_MESON_PLL_L_DETECT_ACTIVE_HIGH to compute the
>>> first value, then flip the bit.
>>
>> Ok, I will update this in the next version.
>>
>> Here is the updated code:
>>
>>          if (MESON_PARM_APPLICABLE(&pll->l_detect)) {
>>                          meson_parm_write(clk->map, &pll->l_detect,
>>                          !(pll->flags &
>> CLK_MESON_PLL_L_DETECT_ACTIVE_HIGH));
>>                          meson_parm_write(clk->map, &pll->l_detect,
>>                          !!(pll->flags &
>> CLK_MESON_PLL_L_DETECT_ACTIVE_HIGH));
> Please use a variable. Make it clean


Ok, I will use a variable for it.

>>          }
>>
>>>>         }
>>>>
>>>>         if (meson_clk_pll_wait_lock(hw))
>>>> diff --git a/drivers/clk/meson/clk-pll.h b/drivers/clk/meson/clk-pll.h
>>>> index 949157fb7bf5..97b7c70376a3 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-high */
>>>> +#define CLK_MESON_PLL_L_DETECT_ACTIVE_HIGH   BIT(2)
>>>>
>>>>    struct meson_clk_pll_data {
>>>>         struct parm en;
>>> --
>>> Jerome
>> Best regards,
>>
>> Jian
> --
> Jerome
Best regards,

Jian

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic