1. Patchew
  2. linux
  3. View series

[PATCH] ata: ahci_tegra: remove kcalloc

Rosen Penev posted 1 patch 1 week, 2 days ago 
drivers/ata/ahci_tegra.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
[PATCH] ata: ahci_tegra: remove kcalloc
Posted by Rosen Penev 1 week, 2 days ago 
Combine allocations into one by using a flexible array member.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/ata/ahci_tegra.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/ata/ahci_tegra.c b/drivers/ata/ahci_tegra.c
index 44584eed6374..5972fe04ff3f 100644
--- a/drivers/ata/ahci_tegra.c
+++ b/drivers/ata/ahci_tegra.c
@@ -175,8 +175,9 @@ struct tegra_ahci_priv {
 	struct reset_control	   *sata_cold_rst;
 	/* Needs special handling, cannot use ahci_platform */
 	struct clk		   *sata_clk;
-	struct regulator_bulk_data *supplies;
 	const struct tegra_ahci_soc *soc;
+
+	struct regulator_bulk_data supplies[];
 };
 
 static void tegra_ahci_handle_quirks(struct ahci_host_priv *hpriv)
@@ -512,6 +513,7 @@ static const struct scsi_host_template ahci_platform_sht = {
 
 static int tegra_ahci_probe(struct platform_device *pdev)
 {
+	const struct tegra_ahci_soc *soc;
 	struct ahci_host_priv *hpriv;
 	struct tegra_ahci_priv *tegra;
 	struct resource *res;
@@ -521,14 +523,15 @@ static int tegra_ahci_probe(struct platform_device *pdev)
 	if (IS_ERR(hpriv))
 		return PTR_ERR(hpriv);
 
-	tegra = devm_kzalloc(&pdev->dev, sizeof(*tegra), GFP_KERNEL);
+	soc = of_device_get_match_data(&pdev->dev);
+	tegra = devm_kzalloc(&pdev->dev, struct_size(tegra, supplies, soc->num_supplies), GFP_KERNEL);
 	if (!tegra)
 		return -ENOMEM;
 
 	hpriv->plat_data = tegra;
 
 	tegra->pdev = pdev;
-	tegra->soc = of_device_get_match_data(&pdev->dev);
+	tegra->soc = soc;
 
 	tegra->sata_regs = devm_platform_ioremap_resource(pdev, 1);
 	if (IS_ERR(tegra->sata_regs))
@@ -571,12 +574,6 @@ static int tegra_ahci_probe(struct platform_device *pdev)
 		return PTR_ERR(tegra->sata_clk);
 	}
 
-	tegra->supplies = devm_kcalloc(&pdev->dev,
-				       tegra->soc->num_supplies,
-				       sizeof(*tegra->supplies), GFP_KERNEL);
-	if (!tegra->supplies)
-		return -ENOMEM;
-
 	regulator_bulk_set_supply_names(tegra->supplies,
 					tegra->soc->supply_names,
 					tegra->soc->num_supplies);
-- 
2.53.0
Re: [PATCH] ata: ahci_tegra: remove kcalloc
Posted by Niklas Cassel 1 week, 1 day ago 
Hello Rosen,

subject is a bit misleading:
"remove kcalloc"
you are removing devm_kcalloc(), so device managed.


On Tue, Mar 24, 2026 at 02:16:29PM -0700, Rosen Penev wrote:
> Combine allocations into one by using a flexible array member.
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/ata/ahci_tegra.c | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/ata/ahci_tegra.c b/drivers/ata/ahci_tegra.c
> index 44584eed6374..5972fe04ff3f 100644
> --- a/drivers/ata/ahci_tegra.c
> +++ b/drivers/ata/ahci_tegra.c
> @@ -175,8 +175,9 @@ struct tegra_ahci_priv {
>  	struct reset_control	   *sata_cold_rst;
>  	/* Needs special handling, cannot use ahci_platform */
>  	struct clk		   *sata_clk;
> -	struct regulator_bulk_data *supplies;
>  	const struct tegra_ahci_soc *soc;
> +
> +	struct regulator_bulk_data supplies[];

Personally I'm not a big fan of flexible array members, as there can be
only one. And if you use it you want to use counted_by().

Yes, there are two device managed allocations. But is that so bad?

Since it is device managed, it will get freed on device removal anyway.


Kind regards,
Niklas
Re: [PATCH] ata: ahci_tegra: remove kcalloc
Posted by Jon Hunter 1 week, 1 day ago 
On 25/03/2026 07:30, Niklas Cassel wrote:
> Hello Rosen,
> 
> subject is a bit misleading:
> "remove kcalloc"
> you are removing devm_kcalloc(), so device managed.
> 
> 
> On Tue, Mar 24, 2026 at 02:16:29PM -0700, Rosen Penev wrote:
>> Combine allocations into one by using a flexible array member.
>>
>> Signed-off-by: Rosen Penev <rosenp@gmail.com>
>> ---
>>   drivers/ata/ahci_tegra.c | 15 ++++++---------
>>   1 file changed, 6 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/ata/ahci_tegra.c b/drivers/ata/ahci_tegra.c
>> index 44584eed6374..5972fe04ff3f 100644
>> --- a/drivers/ata/ahci_tegra.c
>> +++ b/drivers/ata/ahci_tegra.c
>> @@ -175,8 +175,9 @@ struct tegra_ahci_priv {
>>   	struct reset_control	   *sata_cold_rst;
>>   	/* Needs special handling, cannot use ahci_platform */
>>   	struct clk		   *sata_clk;
>> -	struct regulator_bulk_data *supplies;
>>   	const struct tegra_ahci_soc *soc;
>> +
>> +	struct regulator_bulk_data supplies[];
> 
> Personally I'm not a big fan of flexible array members, as there can be
> only one. And if you use it you want to use counted_by().
> 
> Yes, there are two device managed allocations. But is that so bad?
> 
> Since it is device managed, it will get freed on device removal anyway.

FWIW I am not a big fan of this either. It is not an obvious bang for 
the buck for me. The one downside I see is that it does leave the door 
open for someone accidentally putting another variable after the 
flexible array member. Yes we should catch this in review, but there 
really should be at least a comment saying this must be the final member 
of the struct.

Jon

-- 
nvpublic
Re: [PATCH] ata: ahci_tegra: remove kcalloc
Posted by Rosen Penev 1 week, 1 day ago 
On Wed, Mar 25, 2026 at 3:17 AM Jon Hunter <jonathanh@nvidia.com> wrote:
>
>
> On 25/03/2026 07:30, Niklas Cassel wrote:
> > Hello Rosen,
> >
> > subject is a bit misleading:
> > "remove kcalloc"
> > you are removing devm_kcalloc(), so device managed.
> >
> >
> > On Tue, Mar 24, 2026 at 02:16:29PM -0700, Rosen Penev wrote:
> >> Combine allocations into one by using a flexible array member.
> >>
> >> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> >> ---
> >>   drivers/ata/ahci_tegra.c | 15 ++++++---------
> >>   1 file changed, 6 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/ata/ahci_tegra.c b/drivers/ata/ahci_tegra.c
> >> index 44584eed6374..5972fe04ff3f 100644
> >> --- a/drivers/ata/ahci_tegra.c
> >> +++ b/drivers/ata/ahci_tegra.c
> >> @@ -175,8 +175,9 @@ struct tegra_ahci_priv {
> >>      struct reset_control       *sata_cold_rst;
> >>      /* Needs special handling, cannot use ahci_platform */
> >>      struct clk                 *sata_clk;
> >> -    struct regulator_bulk_data *supplies;
> >>      const struct tegra_ahci_soc *soc;
> >> +
> >> +    struct regulator_bulk_data supplies[];
> >
> > Personally I'm not a big fan of flexible array members, as there can be
> > only one. And if you use it you want to use counted_by().
> >
> > Yes, there are two device managed allocations. But is that so bad?
> >
> > Since it is device managed, it will get freed on device removal anyway.
>
> FWIW I am not a big fan of this either. It is not an obvious bang for
> the buck for me. The one downside I see is that it does leave the door
> open for someone accidentally putting another variable after the
> flexible array member. Yes we should catch this in review, but there
> really should be at least a comment saying this must be the final member
> of the struct.
That will eventually become a compile time error. Currently there are
a bunch of those cases that need to get fixed before that happens.
Hardening people are working on it.
>
> Jon
>
> --
> nvpublic
>
Re: [PATCH] ata: ahci_tegra: remove kcalloc
Posted by Damien Le Moal 1 week, 2 days ago 
On 2026/03/24 14:16, Rosen Penev wrote:
> Combine allocations into one by using a flexible array member.
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/ata/ahci_tegra.c | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/ata/ahci_tegra.c b/drivers/ata/ahci_tegra.c
> index 44584eed6374..5972fe04ff3f 100644
> --- a/drivers/ata/ahci_tegra.c
> +++ b/drivers/ata/ahci_tegra.c
> @@ -175,8 +175,9 @@ struct tegra_ahci_priv {
>  	struct reset_control	   *sata_cold_rst;
>  	/* Needs special handling, cannot use ahci_platform */
>  	struct clk		   *sata_clk;
> -	struct regulator_bulk_data *supplies;
>  	const struct tegra_ahci_soc *soc;
> +
> +	struct regulator_bulk_data supplies[];

I think this needs a __counted_by() annotation, but not sure if that is possible
given that soc->num_supplies is not in this structure. Might need a copy of it.

>  };
>  
>  static void tegra_ahci_handle_quirks(struct ahci_host_priv *hpriv)
> @@ -512,6 +513,7 @@ static const struct scsi_host_template ahci_platform_sht = {
>  
>  static int tegra_ahci_probe(struct platform_device *pdev)
>  {
> +	const struct tegra_ahci_soc *soc;
>  	struct ahci_host_priv *hpriv;
>  	struct tegra_ahci_priv *tegra;
>  	struct resource *res;
> @@ -521,14 +523,15 @@ static int tegra_ahci_probe(struct platform_device *pdev)
>  	if (IS_ERR(hpriv))
>  		return PTR_ERR(hpriv);
>  
> -	tegra = devm_kzalloc(&pdev->dev, sizeof(*tegra), GFP_KERNEL);
> +	soc = of_device_get_match_data(&pdev->dev);
> +	tegra = devm_kzalloc(&pdev->dev, struct_size(tegra, supplies, soc->num_supplies), GFP_KERNEL);
>  	if (!tegra)
>  		return -ENOMEM;
>  
>  	hpriv->plat_data = tegra;
>  
>  	tegra->pdev = pdev;
> -	tegra->soc = of_device_get_match_data(&pdev->dev);
> +	tegra->soc = soc;
>  
>  	tegra->sata_regs = devm_platform_ioremap_resource(pdev, 1);
>  	if (IS_ERR(tegra->sata_regs))
> @@ -571,12 +574,6 @@ static int tegra_ahci_probe(struct platform_device *pdev)
>  		return PTR_ERR(tegra->sata_clk);
>  	}
>  
> -	tegra->supplies = devm_kcalloc(&pdev->dev,
> -				       tegra->soc->num_supplies,
> -				       sizeof(*tegra->supplies), GFP_KERNEL);
> -	if (!tegra->supplies)
> -		return -ENOMEM;
> -
>  	regulator_bulk_set_supply_names(tegra->supplies,
>  					tegra->soc->supply_names,
>  					tegra->soc->num_supplies);


-- 
Damien Le Moal
Western Digital Research
Re: [PATCH] ata: ahci_tegra: remove kcalloc
Posted by Rosen Penev 1 week, 2 days ago 
On Tue, Mar 24, 2026 at 3:10 PM Damien Le Moal <dlemoal@kernel.org> wrote:
>
> On 2026/03/24 14:16, Rosen Penev wrote:
> > Combine allocations into one by using a flexible array member.
> >
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> >  drivers/ata/ahci_tegra.c | 15 ++++++---------
> >  1 file changed, 6 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/ata/ahci_tegra.c b/drivers/ata/ahci_tegra.c
> > index 44584eed6374..5972fe04ff3f 100644
> > --- a/drivers/ata/ahci_tegra.c
> > +++ b/drivers/ata/ahci_tegra.c
> > @@ -175,8 +175,9 @@ struct tegra_ahci_priv {
> >       struct reset_control       *sata_cold_rst;
> >       /* Needs special handling, cannot use ahci_platform */
> >       struct clk                 *sata_clk;
> > -     struct regulator_bulk_data *supplies;
> >       const struct tegra_ahci_soc *soc;
> > +
> > +     struct regulator_bulk_data supplies[];
>
> I think this needs a __counted_by() annotation, but not sure if that is possible
> given that soc->num_supplies is not in this structure. Might need a copy of it.
Is it really worth it to make a copy?
>
> >  };
> >
> >  static void tegra_ahci_handle_quirks(struct ahci_host_priv *hpriv)
> > @@ -512,6 +513,7 @@ static const struct scsi_host_template ahci_platform_sht = {
> >
> >  static int tegra_ahci_probe(struct platform_device *pdev)
> >  {
> > +     const struct tegra_ahci_soc *soc;
> >       struct ahci_host_priv *hpriv;
> >       struct tegra_ahci_priv *tegra;
> >       struct resource *res;
> > @@ -521,14 +523,15 @@ static int tegra_ahci_probe(struct platform_device *pdev)
> >       if (IS_ERR(hpriv))
> >               return PTR_ERR(hpriv);
> >
> > -     tegra = devm_kzalloc(&pdev->dev, sizeof(*tegra), GFP_KERNEL);
> > +     soc = of_device_get_match_data(&pdev->dev);
> > +     tegra = devm_kzalloc(&pdev->dev, struct_size(tegra, supplies, soc->num_supplies), GFP_KERNEL);
> >       if (!tegra)
> >               return -ENOMEM;
> >
> >       hpriv->plat_data = tegra;
> >
> >       tegra->pdev = pdev;
> > -     tegra->soc = of_device_get_match_data(&pdev->dev);
> > +     tegra->soc = soc;
> >
> >       tegra->sata_regs = devm_platform_ioremap_resource(pdev, 1);
> >       if (IS_ERR(tegra->sata_regs))
> > @@ -571,12 +574,6 @@ static int tegra_ahci_probe(struct platform_device *pdev)
> >               return PTR_ERR(tegra->sata_clk);
> >       }
> >
> > -     tegra->supplies = devm_kcalloc(&pdev->dev,
> > -                                    tegra->soc->num_supplies,
> > -                                    sizeof(*tegra->supplies), GFP_KERNEL);
> > -     if (!tegra->supplies)
> > -             return -ENOMEM;
> > -
> >       regulator_bulk_set_supply_names(tegra->supplies,
> >                                       tegra->soc->supply_names,
> >                                       tegra->soc->num_supplies);
>
>
> --
> Damien Le Moal
> Western Digital Research

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.