[v1] dmaengine: ste_dma40: turn d40_base phy_chans into a flexible array

[PATCH] dmaengine: ste_dma40: turn d40_base phy_chans into a flexible array

Posted by Rosen Penev 1 week, 5 days ago

Convert the separately-offset phy_chans pointer to a C99 flexible array
member at the end of struct d40_base, and switch the allocation to
struct_size(). The log_chans and memcpy_chans slots continue to live
in the same allocation immediately after phy_chans, indexed via
base->log_chans. This removes the hand-rolled pointer fixup that
recomputed phy_chans from base + ALIGN(sizeof(struct d40_base), 4).

Assisted-by: Claude:Opus-4.7
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/ste_dma40.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 9b803c0aec25..d3e3c4cd43f1 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -602,7 +602,6 @@ struct d40_base {
 	struct dma_device		  dma_both;
 	struct dma_device		  dma_slave;
 	struct dma_device		  dma_memcpy;
-	struct d40_chan			 *phy_chans;
 	struct d40_chan			 *log_chans;
 	struct d40_chan			**lookup_log_chans;
 	struct d40_chan			**lookup_phy_chans;
@@ -621,6 +620,7 @@ struct d40_base {
 	u32				 *regs_interrupt;
 	u16				  gcc_pwr_off_mask;
 	struct d40_gen_dmac		  gen_dmac;
+	struct d40_chan			 phy_chans[];
 };
 
 static struct device *chan2dev(struct d40_chan *d40c)
@@ -3128,6 +3128,7 @@ static int __init d40_hw_detect_init(struct platform_device *pdev,
 	struct clk *clk;
 	void __iomem *virtbase;
 	struct d40_base *base;
+	size_t alloc_size;
 	int num_log_chans;
 	int num_phy_chans;
 	int num_memcpy_chans;
@@ -3197,10 +3198,9 @@ static int __init d40_hw_detect_init(struct platform_device *pdev,
 		 "hardware rev: %d with %d physical and %d logical channels\n",
 		 rev, num_phy_chans, num_log_chans);
 
-	base = devm_kzalloc(dev,
-		ALIGN(sizeof(struct d40_base), 4) +
-		(num_phy_chans + num_log_chans + num_memcpy_chans) *
-		sizeof(struct d40_chan), GFP_KERNEL);
+	alloc_size = struct_size(base, phy_chans, num_phy_chans);
+	alloc_size += sizeof(*base->log_chans) * (num_log_chans + num_memcpy_chans);
+	base = devm_kzalloc(dev, alloc_size, GFP_KERNEL);
 
 	if (!base)
 		return -ENOMEM;
@@ -3213,7 +3213,6 @@ static int __init d40_hw_detect_init(struct platform_device *pdev,
 	base->virtbase = virtbase;
 	base->plat_data = plat_data;
 	base->dev = dev;
-	base->phy_chans = ((void *)base) + ALIGN(sizeof(struct d40_base), 4);
 	base->log_chans = &base->phy_chans[num_phy_chans];
 
 	if (base->plat_data->num_of_phy_chans == 14) {
-- 
2.54.0

Re: [PATCH] dmaengine: ste_dma40: turn d40_base phy_chans into a flexible array

Posted by Linus Walleij 1 week, 5 days ago

Hi Rosen,

thanks for your patch!

On Tue, May 26, 2026 at 10:16 PM Rosen Penev <rosenp@gmail.com> wrote:

> Convert the separately-offset phy_chans pointer to a C99 flexible array
> member at the end of struct d40_base, and switch the allocation to
> struct_size(). The log_chans and memcpy_chans slots continue to live
> in the same allocation immediately after phy_chans, indexed via
> base->log_chans. This removes the hand-rolled pointer fixup that
> recomputed phy_chans from base + ALIGN(sizeof(struct d40_base), 4).
>
> Assisted-by: Claude:Opus-4.7
> Signed-off-by: Rosen Penev <rosenp@gmail.com>

OK!

Please add

unsigned int num_phy_chans

> +       struct d40_chan                  phy_chans[];

and

phy_chans[] __counted_by(num_phy_chans);


> -       base = devm_kzalloc(dev,
> -               ALIGN(sizeof(struct d40_base), 4) +
> -               (num_phy_chans + num_log_chans + num_memcpy_chans) *
> -               sizeof(struct d40_chan), GFP_KERNEL);
> +       alloc_size = struct_size(base, phy_chans, num_phy_chans);
> +       alloc_size += sizeof(*base->log_chans) * (num_log_chans + num_memcpy_chans);
> +       base = devm_kzalloc(dev, alloc_size, GFP_KERNEL);

Please describe exactly how the ALIGN(sizeof(struct d40_base), 4) requirement
is met by the new code?

The phy_chans will be read by hardware which depends on this specific
alignment otherwise the data will be corrupted.

Yours,
Linus Walleij

Re: [PATCH] dmaengine: ste_dma40: turn d40_base phy_chans into a flexible array

Posted by Rosen Penev 1 week, 4 days ago

On Wed, May 27, 2026 at 3:14 AM Linus Walleij <linusw@kernel.org> wrote:
>
> Hi Rosen,
>
> thanks for your patch!
>
> On Tue, May 26, 2026 at 10:16 PM Rosen Penev <rosenp@gmail.com> wrote:
>
> > Convert the separately-offset phy_chans pointer to a C99 flexible array
> > member at the end of struct d40_base, and switch the allocation to
> > struct_size(). The log_chans and memcpy_chans slots continue to live
> > in the same allocation immediately after phy_chans, indexed via
> > base->log_chans. This removes the hand-rolled pointer fixup that
> > recomputed phy_chans from base + ALIGN(sizeof(struct d40_base), 4).
> >
> > Assisted-by: Claude:Opus-4.7
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
>
> OK!
>
> Please add
>
> unsigned int num_phy_chans
>
> > +       struct d40_chan                  phy_chans[];
>
> and
>
> phy_chans[] __counted_by(num_phy_chans);
Not possible here. The allocation allocates space for both phy_chans
and log_chans. To do this I would need to split up allocations into
two. Not a fan of that as two kfrees and two allocs would be needed.
>
>
> > -       base = devm_kzalloc(dev,
> > -               ALIGN(sizeof(struct d40_base), 4) +
> > -               (num_phy_chans + num_log_chans + num_memcpy_chans) *
> > -               sizeof(struct d40_chan), GFP_KERNEL);
> > +       alloc_size = struct_size(base, phy_chans, num_phy_chans);
> > +       alloc_size += sizeof(*base->log_chans) * (num_log_chans + num_memcpy_chans);
> > +       base = devm_kzalloc(dev, alloc_size, GFP_KERNEL);
>
> Please describe exactly how the ALIGN(sizeof(struct d40_base), 4) requirement
> is met by the new code?
Will do.
>
> The phy_chans will be read by hardware which depends on this specific
> alignment otherwise the data will be corrupted.
>
> Yours,
> Linus Walleij