[RFC PATCH v2 12/27] damengine: dw-edma: Fix MSI data values for multi-vector IMWr interrupts

Koichiro Den posted 27 patches 2 months, 1 week ago
[RFC PATCH v2 12/27] damengine: dw-edma: Fix MSI data values for multi-vector IMWr interrupts
Posted by Koichiro Den 2 months, 1 week ago
When multiple MSI vectors are allocated for the DesignWare eDMA, the
driver currently records the same MSI message for all IRQs by calling
get_cached_msi_msg() per vector. For multi-vector MSI (as opposed to
MSI-X), the cached message corresponds to vector 0 and msg.data is
supposed to be adjusted by the IRQ index.

As a result, all eDMA interrupts share the same MSI data value and the
interrupt controller cannot distinguish between them.

Introduce dw_edma_compose_msi() to construct the correct MSI message for
each vector. For MSI-X nothing changes. For multi-vector MSI, derive the
base IRQ with msi_get_virq(dev, 0) and OR in the per-vector offset into
msg.data before storing it in dw->irq[i].msi.

This makes each IMWr MSI vector use a unique MSI data value.

Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
 drivers/dma/dw-edma/dw-edma-core.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 8e5f7defa6b6..3542177a4a8e 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -839,6 +839,28 @@ static inline void dw_edma_add_irq_mask(u32 *mask, u32 alloc, u16 cnt)
 		(*mask)++;
 }
 
+static void dw_edma_compose_msi(struct device *dev, int irq, struct msi_msg *out)
+{
+	struct msi_desc *desc = irq_get_msi_desc(irq);
+	struct msi_msg msg;
+	unsigned int base;
+
+	if (!desc)
+		return;
+
+	get_cached_msi_msg(irq, &msg);
+	if (!desc->pci.msi_attrib.is_msix) {
+		/*
+		 * For multi-vector MSI, the cached message corresponds to
+		 * vector 0. Adjust msg.data by the IRQ index so that each
+		 * vector gets a unique MSI data value for IMWr Data Register.
+		 */
+		base = msi_get_virq(dev, 0);
+		msg.data |= (irq - base);
+	}
+	*out = msg;
+}
+
 static int dw_edma_irq_request(struct dw_edma *dw,
 			       u32 *wr_alloc, u32 *rd_alloc)
 {
@@ -869,8 +891,7 @@ static int dw_edma_irq_request(struct dw_edma *dw,
 			return err;
 		}
 
-		if (irq_get_msi_desc(irq))
-			get_cached_msi_msg(irq, &dw->irq[0].msi);
+		dw_edma_compose_msi(dev, irq, &dw->irq[0].msi);
 
 		dw->nr_irqs = 1;
 	} else {
@@ -896,8 +917,7 @@ static int dw_edma_irq_request(struct dw_edma *dw,
 			if (err)
 				goto err_irq_free;
 
-			if (irq_get_msi_desc(irq))
-				get_cached_msi_msg(irq, &dw->irq[i].msi);
+			dw_edma_compose_msi(dev, irq, &dw->irq[i].msi);
 		}
 
 		dw->nr_irqs = i;
-- 
2.48.1
Re: [RFC PATCH v2 12/27] damengine: dw-edma: Fix MSI data values for multi-vector IMWr interrupts
Posted by Frank Li 2 months, 1 week ago
On Sun, Nov 30, 2025 at 01:03:50AM +0900, Koichiro Den wrote:
> When multiple MSI vectors are allocated for the DesignWare eDMA, the
> driver currently records the same MSI message for all IRQs by calling
> get_cached_msi_msg() per vector. For multi-vector MSI (as opposed to
> MSI-X), the cached message corresponds to vector 0 and msg.data is
> supposed to be adjusted by the IRQ index.
>
> As a result, all eDMA interrupts share the same MSI data value and the
> interrupt controller cannot distinguish between them.
>
> Introduce dw_edma_compose_msi() to construct the correct MSI message for
> each vector. For MSI-X nothing changes. For multi-vector MSI, derive the
> base IRQ with msi_get_virq(dev, 0) and OR in the per-vector offset into
> msg.data before storing it in dw->irq[i].msi.
>
> This makes each IMWr MSI vector use a unique MSI data value.
>
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---
>  drivers/dma/dw-edma/dw-edma-core.c | 28 ++++++++++++++++++++++++----
>  1 file changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 8e5f7defa6b6..3542177a4a8e 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -839,6 +839,28 @@ static inline void dw_edma_add_irq_mask(u32 *mask, u32 alloc, u16 cnt)
>  		(*mask)++;
>  }
>
> +static void dw_edma_compose_msi(struct device *dev, int irq, struct msi_msg *out)
> +{
> +	struct msi_desc *desc = irq_get_msi_desc(irq);
> +	struct msi_msg msg;
> +	unsigned int base;
> +
> +	if (!desc)
> +		return;
> +
> +	get_cached_msi_msg(irq, &msg);
> +	if (!desc->pci.msi_attrib.is_msix) {
> +		/*
> +		 * For multi-vector MSI, the cached message corresponds to
> +		 * vector 0. Adjust msg.data by the IRQ index so that each
> +		 * vector gets a unique MSI data value for IMWr Data Register.
> +		 */
> +		base = msi_get_virq(dev, 0);
> +		msg.data |= (irq - base);

why "|=", not "=" here?

Frank

> +	}
> +	*out = msg;
> +}
> +
>  static int dw_edma_irq_request(struct dw_edma *dw,
>  			       u32 *wr_alloc, u32 *rd_alloc)
>  {
> @@ -869,8 +891,7 @@ static int dw_edma_irq_request(struct dw_edma *dw,
>  			return err;
>  		}
>
> -		if (irq_get_msi_desc(irq))
> -			get_cached_msi_msg(irq, &dw->irq[0].msi);
> +		dw_edma_compose_msi(dev, irq, &dw->irq[0].msi);
>
>  		dw->nr_irqs = 1;
>  	} else {
> @@ -896,8 +917,7 @@ static int dw_edma_irq_request(struct dw_edma *dw,
>  			if (err)
>  				goto err_irq_free;
>
> -			if (irq_get_msi_desc(irq))
> -				get_cached_msi_msg(irq, &dw->irq[i].msi);
> +			dw_edma_compose_msi(dev, irq, &dw->irq[i].msi);
>  		}
>
>  		dw->nr_irqs = i;
> --
> 2.48.1
>
Re: [RFC PATCH v2 12/27] damengine: dw-edma: Fix MSI data values for multi-vector IMWr interrupts
Posted by Koichiro Den 2 months, 1 week ago
On Mon, Dec 01, 2025 at 02:46:43PM -0500, Frank Li wrote:
> On Sun, Nov 30, 2025 at 01:03:50AM +0900, Koichiro Den wrote:
> > When multiple MSI vectors are allocated for the DesignWare eDMA, the
> > driver currently records the same MSI message for all IRQs by calling
> > get_cached_msi_msg() per vector. For multi-vector MSI (as opposed to
> > MSI-X), the cached message corresponds to vector 0 and msg.data is
> > supposed to be adjusted by the IRQ index.
> >
> > As a result, all eDMA interrupts share the same MSI data value and the
> > interrupt controller cannot distinguish between them.
> >
> > Introduce dw_edma_compose_msi() to construct the correct MSI message for
> > each vector. For MSI-X nothing changes. For multi-vector MSI, derive the
> > base IRQ with msi_get_virq(dev, 0) and OR in the per-vector offset into
> > msg.data before storing it in dw->irq[i].msi.
> >
> > This makes each IMWr MSI vector use a unique MSI data value.
> >
> > Signed-off-by: Koichiro Den <den@valinux.co.jp>
> > ---
> >  drivers/dma/dw-edma/dw-edma-core.c | 28 ++++++++++++++++++++++++----
> >  1 file changed, 24 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> > index 8e5f7defa6b6..3542177a4a8e 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > @@ -839,6 +839,28 @@ static inline void dw_edma_add_irq_mask(u32 *mask, u32 alloc, u16 cnt)
> >  		(*mask)++;
> >  }
> >
> > +static void dw_edma_compose_msi(struct device *dev, int irq, struct msi_msg *out)
> > +{
> > +	struct msi_desc *desc = irq_get_msi_desc(irq);
> > +	struct msi_msg msg;
> > +	unsigned int base;
> > +
> > +	if (!desc)
> > +		return;
> > +
> > +	get_cached_msi_msg(irq, &msg);
> > +	if (!desc->pci.msi_attrib.is_msix) {
> > +		/*
> > +		 * For multi-vector MSI, the cached message corresponds to
> > +		 * vector 0. Adjust msg.data by the IRQ index so that each
> > +		 * vector gets a unique MSI data value for IMWr Data Register.
> > +		 */
> > +		base = msi_get_virq(dev, 0);
> > +		msg.data |= (irq - base);
> 
> why "|=", not "=" here?

"=" is better and safe here. Thanks for pointing it out, I'll fix it.

Koichiro

> 
> Frank
> 
> > +	}
> > +	*out = msg;
> > +}
> > +
> >  static int dw_edma_irq_request(struct dw_edma *dw,
> >  			       u32 *wr_alloc, u32 *rd_alloc)
> >  {
> > @@ -869,8 +891,7 @@ static int dw_edma_irq_request(struct dw_edma *dw,
> >  			return err;
> >  		}
> >
> > -		if (irq_get_msi_desc(irq))
> > -			get_cached_msi_msg(irq, &dw->irq[0].msi);
> > +		dw_edma_compose_msi(dev, irq, &dw->irq[0].msi);
> >
> >  		dw->nr_irqs = 1;
> >  	} else {
> > @@ -896,8 +917,7 @@ static int dw_edma_irq_request(struct dw_edma *dw,
> >  			if (err)
> >  				goto err_irq_free;
> >
> > -			if (irq_get_msi_desc(irq))
> > -				get_cached_msi_msg(irq, &dw->irq[i].msi);
> > +			dw_edma_compose_msi(dev, irq, &dw->irq[i].msi);
> >  		}
> >
> >  		dw->nr_irqs = i;
> > --
> > 2.48.1
> >
Re: [RFC PATCH v2 12/27] damengine: dw-edma: Fix MSI data values for multi-vector IMWr interrupts
Posted by Koichiro Den 1 month, 3 weeks ago
On Tue, Dec 02, 2025 at 03:32:49PM +0900, Koichiro Den wrote:
> On Mon, Dec 01, 2025 at 02:46:43PM -0500, Frank Li wrote:
> > On Sun, Nov 30, 2025 at 01:03:50AM +0900, Koichiro Den wrote:
> > > When multiple MSI vectors are allocated for the DesignWare eDMA, the
> > > driver currently records the same MSI message for all IRQs by calling
> > > get_cached_msi_msg() per vector. For multi-vector MSI (as opposed to
> > > MSI-X), the cached message corresponds to vector 0 and msg.data is
> > > supposed to be adjusted by the IRQ index.
> > >
> > > As a result, all eDMA interrupts share the same MSI data value and the
> > > interrupt controller cannot distinguish between them.
> > >
> > > Introduce dw_edma_compose_msi() to construct the correct MSI message for
> > > each vector. For MSI-X nothing changes. For multi-vector MSI, derive the
> > > base IRQ with msi_get_virq(dev, 0) and OR in the per-vector offset into
> > > msg.data before storing it in dw->irq[i].msi.
> > >
> > > This makes each IMWr MSI vector use a unique MSI data value.
> > >
> > > Signed-off-by: Koichiro Den <den@valinux.co.jp>
> > > ---
> > >  drivers/dma/dw-edma/dw-edma-core.c | 28 ++++++++++++++++++++++++----
> > >  1 file changed, 24 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> > > index 8e5f7defa6b6..3542177a4a8e 100644
> > > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > > @@ -839,6 +839,28 @@ static inline void dw_edma_add_irq_mask(u32 *mask, u32 alloc, u16 cnt)
> > >  		(*mask)++;
> > >  }
> > >
> > > +static void dw_edma_compose_msi(struct device *dev, int irq, struct msi_msg *out)
> > > +{
> > > +	struct msi_desc *desc = irq_get_msi_desc(irq);
> > > +	struct msi_msg msg;
> > > +	unsigned int base;
> > > +
> > > +	if (!desc)
> > > +		return;
> > > +
> > > +	get_cached_msi_msg(irq, &msg);
> > > +	if (!desc->pci.msi_attrib.is_msix) {
> > > +		/*
> > > +		 * For multi-vector MSI, the cached message corresponds to
> > > +		 * vector 0. Adjust msg.data by the IRQ index so that each
> > > +		 * vector gets a unique MSI data value for IMWr Data Register.
> > > +		 */
> > > +		base = msi_get_virq(dev, 0);
> > > +		msg.data |= (irq - base);
> > 
> > why "|=", not "=" here?
> 
> "=" is better and safe here. Thanks for pointing it out, I'll fix it.

I forgot to add a follow-up comment before sending RFC v3. Here I was too
distracted, it was intentional to apply OR to the base value, which was
cached. So [RFC PATCH v3 10/35] remains unchanged.
https://lore.kernel.org/all/20251217151609.3162665-11-den@valinux.co.jp/

Thanks,
Koichiro

> 
> Koichiro
> 
> > 
> > Frank
> > 
> > > +	}
> > > +	*out = msg;
> > > +}
> > > +
> > >  static int dw_edma_irq_request(struct dw_edma *dw,
> > >  			       u32 *wr_alloc, u32 *rd_alloc)
> > >  {
> > > @@ -869,8 +891,7 @@ static int dw_edma_irq_request(struct dw_edma *dw,
> > >  			return err;
> > >  		}
> > >
> > > -		if (irq_get_msi_desc(irq))
> > > -			get_cached_msi_msg(irq, &dw->irq[0].msi);
> > > +		dw_edma_compose_msi(dev, irq, &dw->irq[0].msi);
> > >
> > >  		dw->nr_irqs = 1;
> > >  	} else {
> > > @@ -896,8 +917,7 @@ static int dw_edma_irq_request(struct dw_edma *dw,
> > >  			if (err)
> > >  				goto err_irq_free;
> > >
> > > -			if (irq_get_msi_desc(irq))
> > > -				get_cached_msi_msg(irq, &dw->irq[i].msi);
> > > +			dw_edma_compose_msi(dev, irq, &dw->irq[i].msi);
> > >  		}
> > >
> > >  		dw->nr_irqs = i;
> > > --
> > > 2.48.1
> > >