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 BarkerAttachment:
OpenPGP_0x27F4B3459F002257.asc
Description: OpenPGP public key
Attachment:
OpenPGP_signature.asc
Description: OpenPGP digital signature
