linux-kernel - Re: [PATCH v2 05/13] clk: renesas: Pass sub struct of cpg_mssr_priv to cpg_clk

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <000e0794-d3cd-4c2e-8eec-9216ac6a08b3@bp.renesas.com>
Date: Mon, 17 Feb 2025 11:44:10 +0000
From: Paul Barker <paul.barker.ct@...renesas.com>
To: Thierry Bultel <thierry.bultel.yh@...renesas.com>,
 thierry.bultel@...atsea.fr, Geert Uytterhoeven <geert+renesas@...der.be>,
 Michael Turquette <mturquette@...libre.com>, Stephen Boyd <sboyd@...nel.org>
Cc: linux-renesas-soc@...r.kernel.org, linux-clk@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 05/13] clk: renesas: Pass sub struct of cpg_mssr_priv
 to cpg_clk_register

Hi Thierry,

On 17/02/2025 10:52, Thierry Bultel wrote:
> In a coming evolution, the registration callback will need more parameters
> from cpg_mssr_priv (like another base address with clock controllers
> with double register block).
> Instead of adding more parameters, mode the needed parameters to a public
> sub-struct.

This is missing the Signed-off-by line.

I also think you should mention that rmw_lock is moved to the new struct
as it will be needed in the subsequent patch, since none of the
functions modified by this patch to take a `struct cpg_mssr_pub`
argument use that lock.

[snip]

> diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c b/drivers/clk/renesas/rcar-gen3-cpg.c
> index 027100e84ee4..aafeb139fdb4 100644
> --- a/drivers/clk/renesas/rcar-gen3-cpg.c
> +++ b/drivers/clk/renesas/rcar-gen3-cpg.c
> @@ -345,9 +345,11 @@ static const struct soc_device_attribute cpg_quirks_match[] __initconst = {
>  
>  struct clk * __init rcar_gen3_cpg_clk_register(struct device *dev,
>  	const struct cpg_core_clk *core, const struct cpg_mssr_info *info,
> -	struct clk **clks, void __iomem *base,
> -	struct raw_notifier_head *notifiers)
> +	struct cpg_mssr_pub *pub)
>  {
> +	struct raw_notifier_head *notifiers = &pub->notifiers;
> +	void __iomem *base = pub->base0;
> +	struct clk **clks = pub->clks;
>  	const struct clk *parent;
>  	unsigned int mult = 1;
>  	unsigned int div = 1;
> @@ -431,7 +433,7 @@ struct clk * __init rcar_gen3_cpg_clk_register(struct device *dev,
>  			}
>  
>  			writel(value, csn->reg);
> -			cpg_simple_notifier_register(notifiers, csn);
> +			cpg_simple_notifier_register(&pub->notifiers, csn);

This change isn't needed as you've added a notifiers variable at the
start of the function.

>  			break;
>  		}
>  

[snip]

> diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c
> index 79e7a90c3b1b..7d5fba3aef19 100644
> --- a/drivers/clk/renesas/renesas-cpg-mssr.c
> +++ b/drivers/clk/renesas/renesas-cpg-mssr.c
> @@ -127,7 +127,6 @@ static const u16 srstclr_for_gen4[] = {
>   *
>   * @rcdev: Optional reset controller entity
>   * @dev: CPG/MSSR device
> - * @base: CPG/MSSR register block base address
>   * @reg_layout: CPG/MSSR register layout
>   * @rmw_lock: protects RMW register accesses
>   * @np: Device node in DT for this CPG/MSSR module
> @@ -143,6 +142,7 @@ static const u16 srstclr_for_gen4[] = {
>   *                 [].val: Saved values of SMSTPCR[]
>   * @reserved_ids: Temporary used, reserved id list
>   * @num_reserved_ids: Temporary used, number of reserved id list
> + * @pub: Data passed to clock registration callback
>   * @clks: Array containing all Core and Module Clocks
>   */
>  struct cpg_mssr_priv {
> @@ -150,16 +150,13 @@ struct cpg_mssr_priv {
>  	struct reset_controller_dev rcdev;
>  #endif
>  	struct device *dev;
> -	void __iomem *base;
>  	enum clk_reg_layout reg_layout;
> -	spinlock_t rmw_lock;

You can remove the documentation line for @base in the comment above.

>  	struct device_node *np;
>  
>  	unsigned int num_core_clks;
>  	unsigned int num_mod_clks;
>  	unsigned int last_dt_core_clk;
>  
> -	struct raw_notifier_head notifiers;

As above, you can remove the @notifiers entry from the comment above.

Thanks,

-- 
Paul Barker
Download attachment "OpenPGP_0x27F4B3459F002257.asc" of type "application/pgp-keys" (3521 bytes)

Download attachment "OpenPGP_signature.asc" of type "application/pgp-signature" (237 bytes)