[v2] talitos: allocate channels with main struct

[PATCHv2] talitos: allocate channels with main struct

Posted by Rosen Penev 1 month, 1 week ago

Use a flexible array member to combine allocations.

Add __counted_by for extra runtime analysis.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 v2: add check for of_property_read_u32
 drivers/crypto/talitos.c | 19 +++++++------------
 drivers/crypto/talitos.h |  5 +++--
 2 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index bc61d0fe3514..e1f009684216 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -3409,14 +3409,20 @@ static int talitos_probe(struct platform_device *ofdev)
 	struct device *dev = &ofdev->dev;
 	struct device_node *np = ofdev->dev.of_node;
 	struct talitos_private *priv;
+	unsigned int num_channels;
 	int i, err;
 	int stride;
 	struct resource *res;
 
-	priv = devm_kzalloc(dev, sizeof(struct talitos_private), GFP_KERNEL);
+	if (of_property_read_u32(np, "fsl,num-channels", &num_channels))
+		num_channels = 0;
+
+	priv = devm_kzalloc(dev, struct_size(priv, chan, num_channels), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
+	priv->num_channels = num_channels;
+
 	INIT_LIST_HEAD(&priv->alg_list);
 
 	dev_set_drvdata(dev, priv);
@@ -3436,7 +3442,6 @@ static int talitos_probe(struct platform_device *ofdev)
 	}
 
 	/* get SEC version capabilities from device tree */
-	of_property_read_u32(np, "fsl,num-channels", &priv->num_channels);
 	of_property_read_u32(np, "fsl,channel-fifo-len", &priv->chfifo_len);
 	of_property_read_u32(np, "fsl,exec-units-mask", &priv->exec_units);
 	of_property_read_u32(np, "fsl,descriptor-types-mask",
@@ -3511,16 +3516,6 @@ static int talitos_probe(struct platform_device *ofdev)
 		}
 	}
 
-	priv->chan = devm_kcalloc(dev,
-				  priv->num_channels,
-				  sizeof(struct talitos_channel),
-				  GFP_KERNEL);
-	if (!priv->chan) {
-		dev_err(dev, "failed to allocate channel management space\n");
-		err = -ENOMEM;
-		goto err_out;
-	}
-
 	priv->fifo_len = roundup_pow_of_two(priv->chfifo_len);
 
 	for (i = 0; i < priv->num_channels; i++) {
diff --git a/drivers/crypto/talitos.h b/drivers/crypto/talitos.h
index 1a93ee355929..34b0b5fab7e7 100644
--- a/drivers/crypto/talitos.h
+++ b/drivers/crypto/talitos.h
@@ -139,8 +139,6 @@ struct talitos_private {
 	 */
 	unsigned int fifo_len;
 
-	struct talitos_channel *chan;
-
 	/* next channel to be assigned next incoming descriptor */
 	atomic_t last_chan ____cacheline_aligned;
 
@@ -153,6 +151,9 @@ struct talitos_private {
 	/* hwrng device */
 	struct hwrng rng;
 	bool rng_registered;
+
+	struct talitos_channel chan[] __counted_by(num_channels);
+
 };
 
 /* .features flag */
-- 
2.54.0

Re: [PATCHv2] talitos: allocate channels with main struct

Posted by Herbert Xu 1 month, 1 week ago

On Tue, May 05, 2026 at 12:37:05AM -0700, Rosen Penev wrote:
> Use a flexible array member to combine allocations.
> 
> Add __counted_by for extra runtime analysis.
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  v2: add check for of_property_read_u32
>  drivers/crypto/talitos.c | 19 +++++++------------
>  drivers/crypto/talitos.h |  5 +++--
>  2 files changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
> index bc61d0fe3514..e1f009684216 100644
> --- a/drivers/crypto/talitos.c
> +++ b/drivers/crypto/talitos.c
> @@ -3409,14 +3409,20 @@ static int talitos_probe(struct platform_device *ofdev)
>  	struct device *dev = &ofdev->dev;
>  	struct device_node *np = ofdev->dev.of_node;
>  	struct talitos_private *priv;
> +	unsigned int num_channels;
>  	int i, err;
>  	int stride;
>  	struct resource *res;
>  
> -	priv = devm_kzalloc(dev, sizeof(struct talitos_private), GFP_KERNEL);
> +	if (of_property_read_u32(np, "fsl,num-channels", &num_channels))
> +		num_channels = 0;

Does this driver still work with zero channels? It should just fail
the probe.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCHv2] talitos: allocate channels with main struct

Posted by Paul Louvel 1 month, 1 week ago

FYI, all devices supported by the talitos driver has at least one channel.
The SEC1.2 (also called the SEC Lite) on the MPC885 SoC has one and only one 
crypto channel.

On 5/5/26 10:23 AM, Herbert Xu wrote:
> On Tue, May 05, 2026 at 12:37:05AM -0700, Rosen Penev wrote:
>> Use a flexible array member to combine allocations.
>>
>> Add __counted_by for extra runtime analysis.
>>
>> Signed-off-by: Rosen Penev <rosenp@gmail.com>
>> ---
>>   v2: add check for of_property_read_u32
>>   drivers/crypto/talitos.c | 19 +++++++------------
>>   drivers/crypto/talitos.h |  5 +++--
>>   2 files changed, 10 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
>> index bc61d0fe3514..e1f009684216 100644
>> --- a/drivers/crypto/talitos.c
>> +++ b/drivers/crypto/talitos.c
>> @@ -3409,14 +3409,20 @@ static int talitos_probe(struct platform_device *ofdev)
>>   	struct device *dev = &ofdev->dev;
>>   	struct device_node *np = ofdev->dev.of_node;
>>   	struct talitos_private *priv;
>> +	unsigned int num_channels;
>>   	int i, err;
>>   	int stride;
>>   	struct resource *res;
>>   
>> -	priv = devm_kzalloc(dev, sizeof(struct talitos_private), GFP_KERNEL);
>> +	if (of_property_read_u32(np, "fsl,num-channels", &num_channels))
>> +		num_channels = 0;
> Does this driver still work with zero channels? It should just fail
> the probe.
>
> Thanks,

-- 
Paul Louvel, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCHv2] talitos: allocate channels with main struct

Posted by Rosen Penev 1 month, 1 week ago

On Tue, May 5, 2026 at 1:23 AM Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> On Tue, May 05, 2026 at 12:37:05AM -0700, Rosen Penev wrote:
> > Use a flexible array member to combine allocations.
> >
> > Add __counted_by for extra runtime analysis.
> >
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> >  v2: add check for of_property_read_u32
> >  drivers/crypto/talitos.c | 19 +++++++------------
> >  drivers/crypto/talitos.h |  5 +++--
> >  2 files changed, 10 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
> > index bc61d0fe3514..e1f009684216 100644
> > --- a/drivers/crypto/talitos.c
> > +++ b/drivers/crypto/talitos.c
> > @@ -3409,14 +3409,20 @@ static int talitos_probe(struct platform_device *ofdev)
> >       struct device *dev = &ofdev->dev;
> >       struct device_node *np = ofdev->dev.of_node;
> >       struct talitos_private *priv;
> > +     unsigned int num_channels;
> >       int i, err;
> >       int stride;
> >       struct resource *res;
> >
> > -     priv = devm_kzalloc(dev, sizeof(struct talitos_private), GFP_KERNEL);
> > +     if (of_property_read_u32(np, "fsl,num-channels", &num_channels))
> > +             num_channels = 0;
>
> Does this driver still work with zero channels? It should just fail
> the probe.
I looked through the dts files. All of them have this property. I'll
have the change.
>
> Thanks,
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt