Re: [PATCH] crypto: eip93 - use struct_size() and flexible array for ring allocation

On 04/08/2026 09.18, Rosen Penev wrote:
> On Tue, Aug 4, 2026 at 12:11 AM Thomas Huth <[email protected]> wrote:
>>
>> On 04/08/2026 00.40, Rosen Penev wrote:
>>> Embed the single ring as a flexible array member in eip93_device
>>> instead of allocating it separately. This simplifies the probe path
>>> and uses struct_size() for a single allocation.
>>>
>>> Assisted-by: opencode:big-pickle
>>> Signed-off-by: Rosen Penev <[email protected]>
>>> ---
>>>    .../crypto/inside-secure/eip93/eip93-main.c   |  6 +----
>>>    .../crypto/inside-secure/eip93/eip93-main.h   | 22 +++++++++----------
>>>    2 files changed, 12 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/crypto/inside-secure/eip93/eip93-main.c b/drivers/crypto/inside-secure/eip93/eip93-main.c
>>> index 1a8dabc4ada4..e62785952b0d 100644
>>> --- a/drivers/crypto/inside-secure/eip93/eip93-main.c
>>> +++ b/drivers/crypto/inside-secure/eip93/eip93-main.c
>>> @@ -415,7 +415,7 @@ static int eip93_crypto_probe(struct platform_device *pdev)
>>>        u32 ver, algo_flags;
>>>        int ret;
>>>
>>> -     eip93 = devm_kzalloc(dev, sizeof(*eip93), GFP_KERNEL);
>>> +     eip93 = devm_kzalloc(dev, struct_size(eip93, ring, 1), GFP_KERNEL);
>>>        if (!eip93)
>>>                return -ENOMEM;
>>>
>>> @@ -436,10 +436,6 @@ static int eip93_crypto_probe(struct platform_device *pdev)
>>>        if (ret)
>>>                return ret;
>>>
>>> -     eip93->ring = devm_kcalloc(eip93->dev, 1, sizeof(*eip93->ring), GFP_KERNEL);
>>> -     if (!eip93->ring)
>>> -             return -ENOMEM;
>>> -
>>>        ret = eip93_desc_init(eip93);
>>>        if (ret)
>>>                return ret;
>>> diff --git a/drivers/crypto/inside-secure/eip93/eip93-main.h b/drivers/crypto/inside-secure/eip93/eip93-main.h
>>> index 990c2401b7ce..5f0f51081743 100644
>>> --- a/drivers/crypto/inside-secure/eip93/eip93-main.h
>>> +++ b/drivers/crypto/inside-secure/eip93/eip93-main.h
>>> @@ -92,17 +92,6 @@
>>>                                                    EIP93_HASH_SHA224 | \
>>>                                                    EIP93_HASH_SHA256))
>>>
>>> -/**
>>> - * struct eip93_device - crypto engine device structure
>>> - */
>>> -struct eip93_device {
>>> -     void __iomem            *base;
>>> -     struct device           *dev;
>>> -     struct clk              *clk;
>>> -     int                     irq;
>>> -     struct eip93_ring               *ring;
>>> -};
>>> -
>>>    struct eip93_desc_ring {
>>>        void                    *base;
>>>        void                    *base_end;
>>> @@ -131,6 +120,17 @@ struct eip93_ring {
>>>        struct idr                      crypto_async_idr;
>>>    };
>>>
>>> +/**
>>> + * struct eip93_device - crypto engine device structure
>>> + */
>>> +struct eip93_device {
>>> +     void __iomem            *base;
>>> +     struct device           *dev;
>>> +     struct clk              *clk;
>>> +     int                     irq;
>>> +     struct eip93_ring       ring[];
>>> +};
>> This looks weird, too. If there is always only one "ring", why don't you
>> embed it without the "[]" into the struct eip93_device directly?
> keeps all callers the same. -> vs .
So it's basically keeping the patch small and generating many WTFs for 
future reviewers of the code vs. having a bigger patch now and better 
understable code in the future. Not my decision (it's up to the 
maintainers), but FWIW I'd rather go with option 2.

Anyway, if you want to keep it short, wouldn't it also be possible to 
declare it as ring[1] instead and then keep the sizeof() instead of the 
struct_size() ?

  Thomas