clk: rockchip: Use flexible array for clock table

[PATCH] clk: rockchip: Use flexible array for clock table

Posted by Rosen Penev 1 month ago

Store the clock lookup table in the Rockchip clock provider
allocation instead of allocating it separately.

This ties the table lifetime directly to the provider and removes a
separate allocation failure path while preserving the clk_onecell_data
lookup interface.

Assisted-by: Codex:GPT-5.5
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/clk/rockchip/clk.c | 17 ++++-------------
 drivers/clk/rockchip/clk.h |  2 ++
 2 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
index ee8c79b938d3..dbb4b6f33abb 100644
--- a/drivers/clk/rockchip/clk.c
+++ b/drivers/clk/rockchip/clk.c
@@ -359,26 +359,21 @@ static struct rockchip_clk_provider *rockchip_clk_init_base(
 		unsigned long nr_clks, bool has_late_clocks)
 {
 	struct rockchip_clk_provider *ctx;
-	struct clk **clk_table;
 	struct clk *default_clk_val;
 	int i;
 
 	default_clk_val = ERR_PTR(has_late_clocks ? -EPROBE_DEFER : -ENOENT);
 
-	ctx = kzalloc_obj(struct rockchip_clk_provider);
+	ctx = kzalloc_flex(*ctx, clk_table, nr_clks);
 	if (!ctx)
 		return ERR_PTR(-ENOMEM);
 
-	clk_table = kzalloc_objs(struct clk *, nr_clks);
-	if (!clk_table)
-		goto err_free;
-
 	for (i = 0; i < nr_clks; ++i)
-		clk_table[i] = default_clk_val;
+		ctx->clk_table[i] = default_clk_val;
 
-	ctx->reg_base = base;
-	ctx->clk_data.clks = clk_table;
 	ctx->clk_data.clk_num = nr_clks;
+	ctx->clk_data.clks = ctx->clk_table;
+	ctx->reg_base = base;
 	ctx->cru_node = np;
 	spin_lock_init(&ctx->lock);
 
@@ -388,10 +383,6 @@ static struct rockchip_clk_provider *rockchip_clk_init_base(
 						   "rockchip,grf");
 
 	return ctx;
-
-err_free:
-	kfree(ctx);
-	return ERR_PTR(-ENOMEM);
 }
 
 struct rockchip_clk_provider *rockchip_clk_init(struct device_node *np,
diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
index 9e3503e2ffc2..d4033bf750f5 100644
--- a/drivers/clk/rockchip/clk.h
+++ b/drivers/clk/rockchip/clk.h
@@ -604,6 +604,7 @@ struct rockchip_aux_grf {
  * @grf: regmap of the general-register-files syscon
  * @aux_grf_table: hashtable of auxiliary GRF regmaps, indexed by grf_type
  * @lock: maintains exclusion between callbacks for a given clock-provider.
+ * @clk_table: clock lookup table.
  */
 struct rockchip_clk_provider {
 	void __iomem *reg_base;
@@ -612,6 +613,7 @@ struct rockchip_clk_provider {
 	struct regmap *grf;
 	DECLARE_HASHTABLE(aux_grf_table, GRF_HASH_ORDER);
 	spinlock_t lock;
+	struct clk *clk_table[];
 };
 
 struct rockchip_pll_rate_table {
-- 
2.54.0

Re: [PATCH] clk: rockchip: Use flexible array for clock table

Posted by Brian Masney 1 month ago

Hi Rosen,

On Sun, May 10, 2026 at 08:22:52PM -0700, Rosen Penev wrote:
> Store the clock lookup table in the Rockchip clock provider
> allocation instead of allocating it separately.
> 
> This ties the table lifetime directly to the provider and removes a
> separate allocation failure path while preserving the clk_onecell_data
> lookup interface.
> 
> Assisted-by: Codex:GPT-5.5
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/clk/rockchip/clk.c | 17 ++++-------------
>  drivers/clk/rockchip/clk.h |  2 ++
>  2 files changed, 6 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
> index ee8c79b938d3..dbb4b6f33abb 100644
> --- a/drivers/clk/rockchip/clk.c
> +++ b/drivers/clk/rockchip/clk.c
> @@ -359,26 +359,21 @@ static struct rockchip_clk_provider *rockchip_clk_init_base(
>  		unsigned long nr_clks, bool has_late_clocks)
>  {
>  	struct rockchip_clk_provider *ctx;
> -	struct clk **clk_table;
>  	struct clk *default_clk_val;
>  	int i;
>  
>  	default_clk_val = ERR_PTR(has_late_clocks ? -EPROBE_DEFER : -ENOENT);
>  
> -	ctx = kzalloc_obj(struct rockchip_clk_provider);
> +	ctx = kzalloc_flex(*ctx, clk_table, nr_clks);
>  	if (!ctx)
>  		return ERR_PTR(-ENOMEM);
>  
> -	clk_table = kzalloc_objs(struct clk *, nr_clks);
> -	if (!clk_table)
> -		goto err_free;
> -
>  	for (i = 0; i < nr_clks; ++i)
> -		clk_table[i] = default_clk_val;
> +		ctx->clk_table[i] = default_clk_val;
>  
> -	ctx->reg_base = base;
> -	ctx->clk_data.clks = clk_table;
>  	ctx->clk_data.clk_num = nr_clks;
> +	ctx->clk_data.clks = ctx->clk_table;

Where's the __counted_by? Also struct clk_onecell_data is embedded
inside struct rockchip_clk_provider, and I'm not sure offhand how
that'll work.

Brian


> +	ctx->reg_base = base;
>  	ctx->cru_node = np;
>  	spin_lock_init(&ctx->lock);
>  
> @@ -388,10 +383,6 @@ static struct rockchip_clk_provider *rockchip_clk_init_base(
>  						   "rockchip,grf");
>  
>  	return ctx;
> -
> -err_free:
> -	kfree(ctx);
> -	return ERR_PTR(-ENOMEM);
>  }
>  
>  struct rockchip_clk_provider *rockchip_clk_init(struct device_node *np,
> diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
> index 9e3503e2ffc2..d4033bf750f5 100644
> --- a/drivers/clk/rockchip/clk.h
> +++ b/drivers/clk/rockchip/clk.h
> @@ -604,6 +604,7 @@ struct rockchip_aux_grf {
>   * @grf: regmap of the general-register-files syscon
>   * @aux_grf_table: hashtable of auxiliary GRF regmaps, indexed by grf_type
>   * @lock: maintains exclusion between callbacks for a given clock-provider.
> + * @clk_table: clock lookup table.
>   */
>  struct rockchip_clk_provider {
>  	void __iomem *reg_base;
> @@ -612,6 +613,7 @@ struct rockchip_clk_provider {
>  	struct regmap *grf;
>  	DECLARE_HASHTABLE(aux_grf_table, GRF_HASH_ORDER);
>  	spinlock_t lock;
> +	struct clk *clk_table[];
>  };
>  
>  struct rockchip_pll_rate_table {
> -- 
> 2.54.0
>

Re: [PATCH] clk: rockchip: Use flexible array for clock table

Posted by Rosen Penev 1 month ago

On Mon, May 11, 2026 at 9:06 AM Brian Masney <bmasney@redhat.com> wrote:
>
> Hi Rosen,
>
> On Sun, May 10, 2026 at 08:22:52PM -0700, Rosen Penev wrote:
> > Store the clock lookup table in the Rockchip clock provider
> > allocation instead of allocating it separately.
> >
> > This ties the table lifetime directly to the provider and removes a
> > separate allocation failure path while preserving the clk_onecell_data
> > lookup interface.
> >
> > Assisted-by: Codex:GPT-5.5
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> >  drivers/clk/rockchip/clk.c | 17 ++++-------------
> >  drivers/clk/rockchip/clk.h |  2 ++
> >  2 files changed, 6 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
> > index ee8c79b938d3..dbb4b6f33abb 100644
> > --- a/drivers/clk/rockchip/clk.c
> > +++ b/drivers/clk/rockchip/clk.c
> > @@ -359,26 +359,21 @@ static struct rockchip_clk_provider *rockchip_clk_init_base(
> >               unsigned long nr_clks, bool has_late_clocks)
> >  {
> >       struct rockchip_clk_provider *ctx;
> > -     struct clk **clk_table;
> >       struct clk *default_clk_val;
> >       int i;
> >
> >       default_clk_val = ERR_PTR(has_late_clocks ? -EPROBE_DEFER : -ENOENT);
> >
> > -     ctx = kzalloc_obj(struct rockchip_clk_provider);
> > +     ctx = kzalloc_flex(*ctx, clk_table, nr_clks);
> >       if (!ctx)
> >               return ERR_PTR(-ENOMEM);
> >
> > -     clk_table = kzalloc_objs(struct clk *, nr_clks);
> > -     if (!clk_table)
> > -             goto err_free;
> > -
> >       for (i = 0; i < nr_clks; ++i)
> > -             clk_table[i] = default_clk_val;
> > +             ctx->clk_table[i] = default_clk_val;
> >
> > -     ctx->reg_base = base;
> > -     ctx->clk_data.clks = clk_table;
> >       ctx->clk_data.clk_num = nr_clks;
> > +     ctx->clk_data.clks = ctx->clk_table;
>
> Where's the __counted_by? Also struct clk_onecell_data is embedded
> inside struct rockchip_clk_provider, and I'm not sure offhand how
> that'll work.
Not present. __counted_by does not work with embedded struct members.
No need to add one IMO.
>
> Brian
>
>
> > +     ctx->reg_base = base;
> >       ctx->cru_node = np;
> >       spin_lock_init(&ctx->lock);
> >
> > @@ -388,10 +383,6 @@ static struct rockchip_clk_provider *rockchip_clk_init_base(
> >                                                  "rockchip,grf");
> >
> >       return ctx;
> > -
> > -err_free:
> > -     kfree(ctx);
> > -     return ERR_PTR(-ENOMEM);
> >  }
> >
> >  struct rockchip_clk_provider *rockchip_clk_init(struct device_node *np,
> > diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
> > index 9e3503e2ffc2..d4033bf750f5 100644
> > --- a/drivers/clk/rockchip/clk.h
> > +++ b/drivers/clk/rockchip/clk.h
> > @@ -604,6 +604,7 @@ struct rockchip_aux_grf {
> >   * @grf: regmap of the general-register-files syscon
> >   * @aux_grf_table: hashtable of auxiliary GRF regmaps, indexed by grf_type
> >   * @lock: maintains exclusion between callbacks for a given clock-provider.
> > + * @clk_table: clock lookup table.
> >   */
> >  struct rockchip_clk_provider {
> >       void __iomem *reg_base;
> > @@ -612,6 +613,7 @@ struct rockchip_clk_provider {
> >       struct regmap *grf;
> >       DECLARE_HASHTABLE(aux_grf_table, GRF_HASH_ORDER);
> >       spinlock_t lock;
> > +     struct clk *clk_table[];
> >  };
> >
> >  struct rockchip_pll_rate_table {
> > --
> > 2.54.0
> >
>

Re: [PATCH] clk: rockchip: Use flexible array for clock table

Posted by Brian Masney 1 month ago

+ Gustavo

On Mon, May 11, 2026 at 12:08:56PM -0700, Rosen Penev wrote:
> On Mon, May 11, 2026 at 9:06 AM Brian Masney <bmasney@redhat.com> wrote:
> >
> > Hi Rosen,
> >
> > On Sun, May 10, 2026 at 08:22:52PM -0700, Rosen Penev wrote:
> > > Store the clock lookup table in the Rockchip clock provider
> > > allocation instead of allocating it separately.
> > >
> > > This ties the table lifetime directly to the provider and removes a
> > > separate allocation failure path while preserving the clk_onecell_data
> > > lookup interface.
> > >
> > > Assisted-by: Codex:GPT-5.5
> > > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > > ---
> > >  drivers/clk/rockchip/clk.c | 17 ++++-------------
> > >  drivers/clk/rockchip/clk.h |  2 ++
> > >  2 files changed, 6 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
> > > index ee8c79b938d3..dbb4b6f33abb 100644
> > > --- a/drivers/clk/rockchip/clk.c
> > > +++ b/drivers/clk/rockchip/clk.c
> > > @@ -359,26 +359,21 @@ static struct rockchip_clk_provider *rockchip_clk_init_base(
> > >               unsigned long nr_clks, bool has_late_clocks)
> > >  {
> > >       struct rockchip_clk_provider *ctx;
> > > -     struct clk **clk_table;
> > >       struct clk *default_clk_val;
> > >       int i;
> > >
> > >       default_clk_val = ERR_PTR(has_late_clocks ? -EPROBE_DEFER : -ENOENT);
> > >
> > > -     ctx = kzalloc_obj(struct rockchip_clk_provider);
> > > +     ctx = kzalloc_flex(*ctx, clk_table, nr_clks);
> > >       if (!ctx)
> > >               return ERR_PTR(-ENOMEM);
> > >
> > > -     clk_table = kzalloc_objs(struct clk *, nr_clks);
> > > -     if (!clk_table)
> > > -             goto err_free;
> > > -
> > >       for (i = 0; i < nr_clks; ++i)
> > > -             clk_table[i] = default_clk_val;
> > > +             ctx->clk_table[i] = default_clk_val;
> > >
> > > -     ctx->reg_base = base;
> > > -     ctx->clk_data.clks = clk_table;
> > >       ctx->clk_data.clk_num = nr_clks;
> > > +     ctx->clk_data.clks = ctx->clk_table;
> >
> > Where's the __counted_by? Also struct clk_onecell_data is embedded
> > inside struct rockchip_clk_provider, and I'm not sure offhand how
> > that'll work.
> Not present. __counted_by does not work with embedded struct members.
> No need to add one IMO.

I'll be honest that I'm not sure what's correct here. I added Gustavo
since I saw comments from him before about these types of changes.

Brian