Re: [PATCH 06/10] clk: amlogic: PLL reset signal supports active-low configuration

From: Jian Hu

Date: Tue May 19 2026 - 23:37:40 EST


On 5/14/2026 11:16 PM, Jerome Brunet wrote:
[ EXTERNAL EMAIL ]

On lun. 11 mai 2026 at 20:47, Jian Hu via B4 Relay <devnull+jian.hu.amlogic.com@xxxxxxxxxx> wrote:

From: Jian Hu <jian.hu@xxxxxxxxxxx>

In the A9 design, the PLL reset signal is configured as active-low.

Add the flag 'CLK_MESON_PLL_RST_N' to indicate that the PLL reset signal
is active-low.

Signed-off-by: Jian Hu <jian.hu@xxxxxxxxxxx>
---
drivers/clk/meson/clk-pll.c | 42 +++++++++++++++++++++++++++++++-----------
drivers/clk/meson/clk-pll.h | 2 ++
2 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
index 5a0bd75f85a9..8568ad6ba7b6 100644
--- a/drivers/clk/meson/clk-pll.c
+++ b/drivers/clk/meson/clk-pll.c
@@ -295,10 +295,14 @@ static int meson_clk_pll_is_enabled(struct clk_hw *hw)
{
struct clk_regmap *clk = to_clk_regmap(hw);
struct meson_clk_pll_data *pll = meson_clk_pll_data(clk);
+ unsigned int rst;

- if (MESON_PARM_APPLICABLE(&pll->rst) &&
- meson_parm_read(clk->map, &pll->rst))
- return 0;
+ if (MESON_PARM_APPLICABLE(&pll->rst)) {
+ rst = meson_parm_read(clk->map, &pll->rst);
+ if ((rst && !(pll->flags & CLK_MESON_PLL_RST_ACTIVE_LOW)) ||
+ (!rst && (pll->flags & CLK_MESON_PLL_RST_ACTIVE_LOW)))
Again not a great usage of binary ops. What you've written above is the
verbose version of a XOR.

The code duplication remarks applies to the rest of the patch too


Ok, I will update this and the other similar instances below in the next version.

Here is the updated code for it:

    int active_low = !!(pll->flags & CLK_MESON_PLL_RST_ACTIVE_LOW);

    if (MESON_PARM_APPLICABLE(&pll->rst) &&
                (meson_parm_read(clk->map, &pll->rst) ^ active_low))

[...]


Best regards,

Jian