linux-kernel - Re: [PATCH v3 03/15] clk: sunxi-ng: Add support for update bit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <3616088.iIbC2pHGDl@jernej-laptop>
Date: Tue, 04 Mar 2025 16:28:48 +0100
From: Jernej Škrabec <jernej.skrabec@...il.com>
To: Michael Turquette <mturquette@...libre.com>,
 Stephen Boyd <sboyd@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
 Chen-Yu Tsai <wens@...e.org>, Samuel Holland <samuel@...lland.org>,
 Andre Przywara <andre.przywara@....com>
Cc: Philipp Zabel <p.zabel@...gutronix.de>, linux-clk@...r.kernel.org,
 devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-sunxi@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 03/15] clk: sunxi-ng: Add support for update bit

Dne torek, 4. marec 2025 ob 02:27:53 Srednjeevropski standardni čas je Andre Przywara napisal(a):
> Some clocks in the Allwinner A523 SoC contain an "update bit" (bit 27),
> which must be set to apply any register changes, namely the mux
> selector, the divider and the gate bit.
> 
> Add a new CCU feature bit to mark those clocks, and set bit 27 whenever
> we are applying any changes.
> 
> Signed-off-by: Andre Przywara <andre.przywara@....com>
> ---
>  drivers/clk/sunxi-ng/ccu_common.h | 4 ++++
>  drivers/clk/sunxi-ng/ccu_div.c    | 2 ++
>  drivers/clk/sunxi-ng/ccu_gate.c   | 4 ++++
>  drivers/clk/sunxi-ng/ccu_mux.c    | 2 ++
>  4 files changed, 12 insertions(+)
> 
> diff --git a/drivers/clk/sunxi-ng/ccu_common.h b/drivers/clk/sunxi-ng/ccu_common.h
> index 50fd268329671..d41d33bdff470 100644
> --- a/drivers/clk/sunxi-ng/ccu_common.h
> +++ b/drivers/clk/sunxi-ng/ccu_common.h
> @@ -20,10 +20,14 @@
>  #define CCU_FEATURE_KEY_FIELD		BIT(8)
>  #define CCU_FEATURE_CLOSEST_RATE	BIT(9)
>  #define CCU_FEATURE_DUAL_DIV		BIT(10)
> +#define CCU_FEATURE_UPDATE_BIT27	BIT(11)

There is no reason to have "BIT27" in the name of the macro. This is similar
to KEY_FIELD, which is generic name and doesn't specify either key or position
of this key field. Maybe just CCU_FEATURE_UPDATE_BIT or something equaly
generic.

With that fixed:
Reviewed-by: Jernej Skrabec <jernej.skrabec@...il.com>

Best regards,
Jernej

>  
>  /* MMC timing mode switch bit */
>  #define CCU_MMC_NEW_TIMING_MODE		BIT(30)
>  
> +/* Some clocks need this bit to actually apply register changes */
> +#define CCU_SUNXI_UPDATE_BIT		BIT(27)
> +
>  struct device_node;
>  
>  struct ccu_common {
> diff --git a/drivers/clk/sunxi-ng/ccu_div.c b/drivers/clk/sunxi-ng/ccu_div.c
> index 7f4691f09e01f..2d8b98fe4b13a 100644
> --- a/drivers/clk/sunxi-ng/ccu_div.c
> +++ b/drivers/clk/sunxi-ng/ccu_div.c
> @@ -106,6 +106,8 @@ static int ccu_div_set_rate(struct clk_hw *hw, unsigned long rate,
>  
>  	reg = readl(cd->common.base + cd->common.reg);
>  	reg &= ~GENMASK(cd->div.width + cd->div.shift - 1, cd->div.shift);
> +	if (cd->common.features & CCU_FEATURE_UPDATE_BIT27)
> +		reg |= CCU_SUNXI_UPDATE_BIT;
>  
>  	writel(reg | (val << cd->div.shift),
>  	       cd->common.base + cd->common.reg);
> diff --git a/drivers/clk/sunxi-ng/ccu_gate.c b/drivers/clk/sunxi-ng/ccu_gate.c
> index ac52fd6bff677..0490f95781361 100644
> --- a/drivers/clk/sunxi-ng/ccu_gate.c
> +++ b/drivers/clk/sunxi-ng/ccu_gate.c
> @@ -20,6 +20,8 @@ void ccu_gate_helper_disable(struct ccu_common *common, u32 gate)
>  	spin_lock_irqsave(common->lock, flags);
>  
>  	reg = readl(common->base + common->reg);
> +	if (common->features & CCU_FEATURE_UPDATE_BIT27)
> +		reg |= CCU_SUNXI_UPDATE_BIT;
>  	writel(reg & ~gate, common->base + common->reg);
>  
>  	spin_unlock_irqrestore(common->lock, flags);
> @@ -44,6 +46,8 @@ int ccu_gate_helper_enable(struct ccu_common *common, u32 gate)
>  	spin_lock_irqsave(common->lock, flags);
>  
>  	reg = readl(common->base + common->reg);
> +	if (common->features & CCU_FEATURE_UPDATE_BIT27)
> +		reg |= CCU_SUNXI_UPDATE_BIT;
>  	writel(reg | gate, common->base + common->reg);
>  
>  	spin_unlock_irqrestore(common->lock, flags);
> diff --git a/drivers/clk/sunxi-ng/ccu_mux.c b/drivers/clk/sunxi-ng/ccu_mux.c
> index d7ffbdeee9e04..82ee21e0d3a68 100644
> --- a/drivers/clk/sunxi-ng/ccu_mux.c
> +++ b/drivers/clk/sunxi-ng/ccu_mux.c
> @@ -197,6 +197,8 @@ int ccu_mux_helper_set_parent(struct ccu_common *common,
>  	/* The key field always reads as zero. */
>  	if (common->features & CCU_FEATURE_KEY_FIELD)
>  		reg |= CCU_MUX_KEY_VALUE;
> +	if (common->features & CCU_FEATURE_UPDATE_BIT27)
> +		reg |= CCU_SUNXI_UPDATE_BIT;
>  
>  	reg &= ~GENMASK(cm->width + cm->shift - 1, cm->shift);
>  	writel(reg | (index << cm->shift), common->base + common->reg);
>