[RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support

[RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support

[Devendra K Verma]

The HDMA IP supports the simple mode (non-linked list).
In this mode the channel registers are configured to initiate
a single DMA data transfer. The channel can be configured in
simple mode via peripheral param of dma_slave_config param.

Signed-off-by: Devendra K Verma <devverma@amd.com>
---
 drivers/dma/dw-edma/dw-edma-core.c    | 10 +++++
 drivers/dma/dw-edma/dw-edma-core.h    |  2 +
 drivers/dma/dw-edma/dw-hdma-v0-core.c | 53 ++++++++++++++++++++++++++-
 include/linux/dma/edma.h              |  8 ++++
 4 files changed, 72 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index c2b88cc99e5d..4dafd6554277 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -235,9 +235,19 @@ static int dw_edma_device_config(struct dma_chan *dchan,
 				 struct dma_slave_config *config)
 {
 	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
+	struct dw_edma_peripheral_config *pconfig = config->peripheral_config;
+	unsigned long flags;
+
+	if (WARN_ON(config->peripheral_config &&
+		    config->peripheral_size != sizeof(*pconfig)))
+		return -EINVAL;
 
+	spin_lock_irqsave(&chan->vc.lock, flags);
 	memcpy(&chan->config, config, sizeof(*config));
+
+	chan->non_ll_en = pconfig ? pconfig->non_ll_en : false;
 	chan->configured = true;
+	spin_unlock_irqrestore(&chan->vc.lock, flags);
 
 	return 0;
 }
diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
index 71894b9e0b15..c0266976aa22 100644
--- a/drivers/dma/dw-edma/dw-edma-core.h
+++ b/drivers/dma/dw-edma/dw-edma-core.h
@@ -86,6 +86,8 @@ struct dw_edma_chan {
 	u8				configured;
 
 	struct dma_slave_config		config;
+
+	bool				non_ll_en;
 };
 
 struct dw_edma_irq {
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
index e3f8db4fe909..3237c807a18e 100644
--- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
@@ -225,7 +225,7 @@ static void dw_hdma_v0_sync_ll_data(struct dw_edma_chunk *chunk)
 		readl(chunk->ll_region.vaddr.io);
 }
 
-static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
+static void dw_hdma_v0_ll_start(struct dw_edma_chunk *chunk, bool first)
 {
 	struct dw_edma_chan *chan = chunk->chan;
 	struct dw_edma *dw = chan->dw;
@@ -263,6 +263,57 @@ static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
 	SET_CH_32(dw, chan->dir, chan->id, doorbell, HDMA_V0_DOORBELL_START);
 }
 
+static void dw_hdma_v0_non_ll_start(struct dw_edma_chunk *chunk)
+{
+	struct dw_edma_chan *chan = chunk->chan;
+	struct dw_edma *dw = chan->dw;
+	struct dw_edma_burst *child;
+	u32 val;
+
+	list_for_each_entry(child, &chunk->burst->list, list) {
+		SET_CH_32(dw, chan->dir, chan->id, ch_en, BIT(0));
+
+		/* Source address */
+		SET_CH_32(dw, chan->dir, chan->id, sar.lsb, lower_32_bits(child->sar));
+		SET_CH_32(dw, chan->dir, chan->id, sar.msb, upper_32_bits(child->sar));
+
+		/* Destination address */
+		SET_CH_32(dw, chan->dir, chan->id, dar.lsb, lower_32_bits(child->dar));
+		SET_CH_32(dw, chan->dir, chan->id, dar.msb, upper_32_bits(child->dar));
+
+		/* Transfer size */
+		SET_CH_32(dw, chan->dir, chan->id, transfer_size, child->sz);
+
+		/* Interrupt setup */
+		val = GET_CH_32(dw, chan->dir, chan->id, int_setup) |
+				HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK |
+				HDMA_V0_LOCAL_STOP_INT_EN | HDMA_V0_LOCAL_ABORT_INT_EN;
+
+		if (!(dw->chip->flags & DW_EDMA_CHIP_LOCAL))
+			val |= HDMA_V0_REMOTE_STOP_INT_EN | HDMA_V0_REMOTE_ABORT_INT_EN;
+
+		SET_CH_32(dw, chan->dir, chan->id, int_setup, val);
+
+		/* Channel control setup */
+		val = GET_CH_32(dw, chan->dir, chan->id, control1);
+		val &= ~HDMA_V0_LINKLIST_EN;
+		SET_CH_32(dw, chan->dir, chan->id, control1, val);
+
+		/* Ring the doorbell */
+		SET_CH_32(dw, chan->dir, chan->id, doorbell, HDMA_V0_DOORBELL_START);
+	}
+}
+
+static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
+{
+	struct dw_edma_chan *chan = chunk->chan;
+
+	if (!chan->non_ll_en)
+		dw_hdma_v0_ll_start(chunk, first);
+	else
+		dw_hdma_v0_non_ll_start(chunk);
+}
+
 static void dw_hdma_v0_core_ch_config(struct dw_edma_chan *chan)
 {
 	struct dw_edma *dw = chan->dw;
diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
index 3080747689f6..82d808013a66 100644
--- a/include/linux/dma/edma.h
+++ b/include/linux/dma/edma.h
@@ -101,6 +101,14 @@ struct dw_edma_chip {
 	struct dw_edma		*dw;
 };
 
+/**
+ * struct dw_edma_peripheral_config - peripheral spicific configurations
+ * @non_ll_en:		 enable non-linked list mode of operations
+ */
+struct dw_edma_peripheral_config {
+	bool			non_ll_en;
+};
+
 /* Export to the platform drivers */
 #if IS_REACHABLE(CONFIG_DW_EDMA)
 int dw_edma_probe(struct dw_edma_chip *chip);
-- 
2.43.0

Re: [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support

[Vinod Koul]

On 23-06-25, 11:47, Devendra K Verma wrote:
> The HDMA IP supports the simple mode (non-linked list).
> In this mode the channel registers are configured to initiate
> a single DMA data transfer. The channel can be configured in
> simple mode via peripheral param of dma_slave_config param.
> 
> Signed-off-by: Devendra K Verma <devverma@amd.com>
> ---
>  drivers/dma/dw-edma/dw-edma-core.c    | 10 +++++
>  drivers/dma/dw-edma/dw-edma-core.h    |  2 +
>  drivers/dma/dw-edma/dw-hdma-v0-core.c | 53 ++++++++++++++++++++++++++-
>  include/linux/dma/edma.h              |  8 ++++
>  4 files changed, 72 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index c2b88cc99e5d..4dafd6554277 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -235,9 +235,19 @@ static int dw_edma_device_config(struct dma_chan *dchan,
>  				 struct dma_slave_config *config)
>  {
>  	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> +	struct dw_edma_peripheral_config *pconfig = config->peripheral_config;
> +	unsigned long flags;
> +
> +	if (WARN_ON(config->peripheral_config &&
> +		    config->peripheral_size != sizeof(*pconfig)))
> +		return -EINVAL;
>  
> +	spin_lock_irqsave(&chan->vc.lock, flags);
>  	memcpy(&chan->config, config, sizeof(*config));
> +
> +	chan->non_ll_en = pconfig ? pconfig->non_ll_en : false;
>  	chan->configured = true;
> +	spin_unlock_irqrestore(&chan->vc.lock, flags);
>  
>  	return 0;
>  }
> diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
> index 71894b9e0b15..c0266976aa22 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.h
> +++ b/drivers/dma/dw-edma/dw-edma-core.h
> @@ -86,6 +86,8 @@ struct dw_edma_chan {
>  	u8				configured;
>  
>  	struct dma_slave_config		config;
> +
> +	bool				non_ll_en;

why do you need this? What is the decision to use non ll vs ll one?

>  };
>  
>  struct dw_edma_irq {
> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> index e3f8db4fe909..3237c807a18e 100644
> --- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> @@ -225,7 +225,7 @@ static void dw_hdma_v0_sync_ll_data(struct dw_edma_chunk *chunk)
>  		readl(chunk->ll_region.vaddr.io);
>  }
>  
> -static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> +static void dw_hdma_v0_ll_start(struct dw_edma_chunk *chunk, bool first)
>  {
>  	struct dw_edma_chan *chan = chunk->chan;
>  	struct dw_edma *dw = chan->dw;
> @@ -263,6 +263,57 @@ static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
>  	SET_CH_32(dw, chan->dir, chan->id, doorbell, HDMA_V0_DOORBELL_START);
>  }
>  
> +static void dw_hdma_v0_non_ll_start(struct dw_edma_chunk *chunk)
> +{
> +	struct dw_edma_chan *chan = chunk->chan;
> +	struct dw_edma *dw = chan->dw;
> +	struct dw_edma_burst *child;
> +	u32 val;
> +
> +	list_for_each_entry(child, &chunk->burst->list, list) {
> +		SET_CH_32(dw, chan->dir, chan->id, ch_en, BIT(0));
> +
> +		/* Source address */
> +		SET_CH_32(dw, chan->dir, chan->id, sar.lsb, lower_32_bits(child->sar));
> +		SET_CH_32(dw, chan->dir, chan->id, sar.msb, upper_32_bits(child->sar));
> +
> +		/* Destination address */
> +		SET_CH_32(dw, chan->dir, chan->id, dar.lsb, lower_32_bits(child->dar));
> +		SET_CH_32(dw, chan->dir, chan->id, dar.msb, upper_32_bits(child->dar));
> +
> +		/* Transfer size */
> +		SET_CH_32(dw, chan->dir, chan->id, transfer_size, child->sz);
> +
> +		/* Interrupt setup */
> +		val = GET_CH_32(dw, chan->dir, chan->id, int_setup) |
> +				HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK |
> +				HDMA_V0_LOCAL_STOP_INT_EN | HDMA_V0_LOCAL_ABORT_INT_EN;
> +
> +		if (!(dw->chip->flags & DW_EDMA_CHIP_LOCAL))
> +			val |= HDMA_V0_REMOTE_STOP_INT_EN | HDMA_V0_REMOTE_ABORT_INT_EN;
> +
> +		SET_CH_32(dw, chan->dir, chan->id, int_setup, val);
> +
> +		/* Channel control setup */
> +		val = GET_CH_32(dw, chan->dir, chan->id, control1);
> +		val &= ~HDMA_V0_LINKLIST_EN;
> +		SET_CH_32(dw, chan->dir, chan->id, control1, val);
> +
> +		/* Ring the doorbell */
> +		SET_CH_32(dw, chan->dir, chan->id, doorbell, HDMA_V0_DOORBELL_START);
> +	}
> +}
> +
> +static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> +{
> +	struct dw_edma_chan *chan = chunk->chan;
> +
> +	if (!chan->non_ll_en)
> +		dw_hdma_v0_ll_start(chunk, first);
> +	else
> +		dw_hdma_v0_non_ll_start(chunk);
> +}
> +
>  static void dw_hdma_v0_core_ch_config(struct dw_edma_chan *chan)
>  {
>  	struct dw_edma *dw = chan->dw;
> diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> index 3080747689f6..82d808013a66 100644
> --- a/include/linux/dma/edma.h
> +++ b/include/linux/dma/edma.h
> @@ -101,6 +101,14 @@ struct dw_edma_chip {
>  	struct dw_edma		*dw;
>  };
>  
> +/**
> + * struct dw_edma_peripheral_config - peripheral spicific configurations
> + * @non_ll_en:		 enable non-linked list mode of operations
> + */
> +struct dw_edma_peripheral_config {
> +	bool			non_ll_en;
> +};
> +
>  /* Export to the platform drivers */
>  #if IS_REACHABLE(CONFIG_DW_EDMA)
>  int dw_edma_probe(struct dw_edma_chip *chip);
> -- 
> 2.43.0

-- 
~Vinod

RE: [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support

[Verma, Devendra]

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Vinod

Thanks for the review.

> -----Original Message-----
> From: Vinod Koul <vkoul@kernel.org>
> Sent: Friday, June 27, 2025 03:37
> To: Verma, Devendra <Devendra.Verma@amd.com>
> Cc: dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org; mani@kernel.org
> Subject: Re: [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On 23-06-25, 11:47, Devendra K Verma wrote:
> > The HDMA IP supports the simple mode (non-linked list).
> > In this mode the channel registers are configured to initiate a single
> > DMA data transfer. The channel can be configured in simple mode via
> > peripheral param of dma_slave_config param.
> >
> > Signed-off-by: Devendra K Verma <devverma@amd.com>
> > ---
> >  drivers/dma/dw-edma/dw-edma-core.c    | 10 +++++
> >  drivers/dma/dw-edma/dw-edma-core.h    |  2 +
> >  drivers/dma/dw-edma/dw-hdma-v0-core.c | 53
> ++++++++++++++++++++++++++-
> >  include/linux/dma/edma.h              |  8 ++++
> >  4 files changed, 72 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.c
> > b/drivers/dma/dw-edma/dw-edma-core.c
> > index c2b88cc99e5d..4dafd6554277 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > @@ -235,9 +235,19 @@ static int dw_edma_device_config(struct dma_chan
> *dchan,
> >                                struct dma_slave_config *config)  {
> >       struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> > +     struct dw_edma_peripheral_config *pconfig = config->peripheral_config;
> > +     unsigned long flags;
> > +
> > +     if (WARN_ON(config->peripheral_config &&
> > +                 config->peripheral_size != sizeof(*pconfig)))
> > +             return -EINVAL;
> >
> > +     spin_lock_irqsave(&chan->vc.lock, flags);
> >       memcpy(&chan->config, config, sizeof(*config));
> > +
> > +     chan->non_ll_en = pconfig ? pconfig->non_ll_en : false;
> >       chan->configured = true;
> > +     spin_unlock_irqrestore(&chan->vc.lock, flags);
> >
> >       return 0;
> >  }
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.h
> > b/drivers/dma/dw-edma/dw-edma-core.h
> > index 71894b9e0b15..c0266976aa22 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.h
> > +++ b/drivers/dma/dw-edma/dw-edma-core.h
> > @@ -86,6 +86,8 @@ struct dw_edma_chan {
> >       u8                              configured;
> >
> >       struct dma_slave_config         config;
> > +
> > +     bool                            non_ll_en;
>
> why do you need this? What is the decision to use non ll vs ll one?
>

The IP supports both the modes, LL mode and non-LL mode.
In the current driver code, the support for non-LL mode is not
present. This patch enables the non-LL aka simple mode support
by means of the peripheral_config option in the dmaengine_slave_config.

> >  };
> >
> >  struct dw_edma_irq {
> > diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c
> > b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> > index e3f8db4fe909..3237c807a18e 100644
> > --- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
> > +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> > @@ -225,7 +225,7 @@ static void dw_hdma_v0_sync_ll_data(struct
> dw_edma_chunk *chunk)
> >               readl(chunk->ll_region.vaddr.io);  }
> >
> > -static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool
> > first)
> > +static void dw_hdma_v0_ll_start(struct dw_edma_chunk *chunk, bool
> > +first)
> >  {
> >       struct dw_edma_chan *chan = chunk->chan;
> >       struct dw_edma *dw = chan->dw;
> > @@ -263,6 +263,57 @@ static void dw_hdma_v0_core_start(struct
> dw_edma_chunk *chunk, bool first)
> >       SET_CH_32(dw, chan->dir, chan->id, doorbell,
> > HDMA_V0_DOORBELL_START);  }
> >
> > +static void dw_hdma_v0_non_ll_start(struct dw_edma_chunk *chunk) {
> > +     struct dw_edma_chan *chan = chunk->chan;
> > +     struct dw_edma *dw = chan->dw;
> > +     struct dw_edma_burst *child;
> > +     u32 val;
> > +
> > +     list_for_each_entry(child, &chunk->burst->list, list) {
> > +             SET_CH_32(dw, chan->dir, chan->id, ch_en, BIT(0));
> > +
> > +             /* Source address */
> > +             SET_CH_32(dw, chan->dir, chan->id, sar.lsb, lower_32_bits(child->sar));
> > +             SET_CH_32(dw, chan->dir, chan->id, sar.msb,
> > + upper_32_bits(child->sar));
> > +
> > +             /* Destination address */
> > +             SET_CH_32(dw, chan->dir, chan->id, dar.lsb, lower_32_bits(child->dar));
> > +             SET_CH_32(dw, chan->dir, chan->id, dar.msb,
> > + upper_32_bits(child->dar));
> > +
> > +             /* Transfer size */
> > +             SET_CH_32(dw, chan->dir, chan->id, transfer_size,
> > + child->sz);
> > +
> > +             /* Interrupt setup */
> > +             val = GET_CH_32(dw, chan->dir, chan->id, int_setup) |
> > +                             HDMA_V0_STOP_INT_MASK |
> HDMA_V0_ABORT_INT_MASK |
> > +                             HDMA_V0_LOCAL_STOP_INT_EN |
> > + HDMA_V0_LOCAL_ABORT_INT_EN;
> > +
> > +             if (!(dw->chip->flags & DW_EDMA_CHIP_LOCAL))
> > +                     val |= HDMA_V0_REMOTE_STOP_INT_EN |
> > + HDMA_V0_REMOTE_ABORT_INT_EN;
> > +
> > +             SET_CH_32(dw, chan->dir, chan->id, int_setup, val);
> > +
> > +             /* Channel control setup */
> > +             val = GET_CH_32(dw, chan->dir, chan->id, control1);
> > +             val &= ~HDMA_V0_LINKLIST_EN;
> > +             SET_CH_32(dw, chan->dir, chan->id, control1, val);
> > +
> > +             /* Ring the doorbell */
> > +             SET_CH_32(dw, chan->dir, chan->id, doorbell,
> HDMA_V0_DOORBELL_START);
> > +     }
> > +}
> > +
> > +static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool
> > +first) {
> > +     struct dw_edma_chan *chan = chunk->chan;
> > +
> > +     if (!chan->non_ll_en)
> > +             dw_hdma_v0_ll_start(chunk, first);
> > +     else
> > +             dw_hdma_v0_non_ll_start(chunk); }
> > +
> >  static void dw_hdma_v0_core_ch_config(struct dw_edma_chan *chan)  {
> >       struct dw_edma *dw = chan->dw;
> > diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h index
> > 3080747689f6..82d808013a66 100644
> > --- a/include/linux/dma/edma.h
> > +++ b/include/linux/dma/edma.h
> > @@ -101,6 +101,14 @@ struct dw_edma_chip {
> >       struct dw_edma          *dw;
> >  };
> >
> > +/**
> > + * struct dw_edma_peripheral_config - peripheral spicific configurations
> > + * @non_ll_en:                enable non-linked list mode of operations
> > + */
> > +struct dw_edma_peripheral_config {
> > +     bool                    non_ll_en;
> > +};
> > +
> >  /* Export to the platform drivers */
> >  #if IS_REACHABLE(CONFIG_DW_EDMA)
> >  int dw_edma_probe(struct dw_edma_chip *chip);
> > --
> > 2.43.0
>
> --
> ~Vinod

RE: [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support

[Verma, Devendra]

[AMD Official Use Only - AMD Internal Distribution Only]

Gentle reminder!

Regards,
Devendra

> -----Original Message-----
> From: Verma, Devendra
> Sent: Wednesday, July 2, 2025 15:09
> To: Vinod Koul <vkoul@kernel.org>
> Cc: dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org; mani@kernel.org
> Subject: RE: [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support
>
> Hi Vinod
>
> Thanks for the review.
>
> > -----Original Message-----
> > From: Vinod Koul <vkoul@kernel.org>
> > Sent: Friday, June 27, 2025 03:37
> > To: Verma, Devendra <Devendra.Verma@amd.com>
> > Cc: dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org;
> > mani@kernel.org
> > Subject: Re: [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode
> > Support
> >
> > Caution: This message originated from an External Source. Use proper
> > caution when opening attachments, clicking links, or responding.
> >
> >
> > On 23-06-25, 11:47, Devendra K Verma wrote:
> > > The HDMA IP supports the simple mode (non-linked list).
> > > In this mode the channel registers are configured to initiate a
> > > single DMA data transfer. The channel can be configured in simple
> > > mode via peripheral param of dma_slave_config param.
> > >
> > > Signed-off-by: Devendra K Verma <devverma@amd.com>
> > > ---
> > >  drivers/dma/dw-edma/dw-edma-core.c    | 10 +++++
> > >  drivers/dma/dw-edma/dw-edma-core.h    |  2 +
> > >  drivers/dma/dw-edma/dw-hdma-v0-core.c | 53
> > ++++++++++++++++++++++++++-
> > >  include/linux/dma/edma.h              |  8 ++++
> > >  4 files changed, 72 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/dma/dw-edma/dw-edma-core.c
> > > b/drivers/dma/dw-edma/dw-edma-core.c
> > > index c2b88cc99e5d..4dafd6554277 100644
> > > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > > @@ -235,9 +235,19 @@ static int dw_edma_device_config(struct
> > > dma_chan
> > *dchan,
> > >                                struct dma_slave_config *config)  {
> > >       struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> > > +     struct dw_edma_peripheral_config *pconfig = config->peripheral_config;
> > > +     unsigned long flags;
> > > +
> > > +     if (WARN_ON(config->peripheral_config &&
> > > +                 config->peripheral_size != sizeof(*pconfig)))
> > > +             return -EINVAL;
> > >
> > > +     spin_lock_irqsave(&chan->vc.lock, flags);
> > >       memcpy(&chan->config, config, sizeof(*config));
> > > +
> > > +     chan->non_ll_en = pconfig ? pconfig->non_ll_en : false;
> > >       chan->configured = true;
> > > +     spin_unlock_irqrestore(&chan->vc.lock, flags);
> > >
> > >       return 0;
> > >  }
> > > diff --git a/drivers/dma/dw-edma/dw-edma-core.h
> > > b/drivers/dma/dw-edma/dw-edma-core.h
> > > index 71894b9e0b15..c0266976aa22 100644
> > > --- a/drivers/dma/dw-edma/dw-edma-core.h
> > > +++ b/drivers/dma/dw-edma/dw-edma-core.h
> > > @@ -86,6 +86,8 @@ struct dw_edma_chan {
> > >       u8                              configured;
> > >
> > >       struct dma_slave_config         config;
> > > +
> > > +     bool                            non_ll_en;
> >
> > why do you need this? What is the decision to use non ll vs ll one?
> >
>
> The IP supports both the modes, LL mode and non-LL mode.
> In the current driver code, the support for non-LL mode is not present. This patch
> enables the non-LL aka simple mode support by means of the peripheral_config
> option in the dmaengine_slave_config.
>
> > >  };
> > >
> > >  struct dw_edma_irq {
> > > diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c
> > > b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> > > index e3f8db4fe909..3237c807a18e 100644
> > > --- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
> > > +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> > > @@ -225,7 +225,7 @@ static void dw_hdma_v0_sync_ll_data(struct
> > dw_edma_chunk *chunk)
> > >               readl(chunk->ll_region.vaddr.io);  }
> > >
> > > -static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool
> > > first)
> > > +static void dw_hdma_v0_ll_start(struct dw_edma_chunk *chunk, bool
> > > +first)
> > >  {
> > >       struct dw_edma_chan *chan = chunk->chan;
> > >       struct dw_edma *dw = chan->dw; @@ -263,6 +263,57 @@ static
> > > void dw_hdma_v0_core_start(struct
> > dw_edma_chunk *chunk, bool first)
> > >       SET_CH_32(dw, chan->dir, chan->id, doorbell,
> > > HDMA_V0_DOORBELL_START);  }
> > >
> > > +static void dw_hdma_v0_non_ll_start(struct dw_edma_chunk *chunk) {
> > > +     struct dw_edma_chan *chan = chunk->chan;
> > > +     struct dw_edma *dw = chan->dw;
> > > +     struct dw_edma_burst *child;
> > > +     u32 val;
> > > +
> > > +     list_for_each_entry(child, &chunk->burst->list, list) {
> > > +             SET_CH_32(dw, chan->dir, chan->id, ch_en, BIT(0));
> > > +
> > > +             /* Source address */
> > > +             SET_CH_32(dw, chan->dir, chan->id, sar.lsb, lower_32_bits(child-
> >sar));
> > > +             SET_CH_32(dw, chan->dir, chan->id, sar.msb,
> > > + upper_32_bits(child->sar));
> > > +
> > > +             /* Destination address */
> > > +             SET_CH_32(dw, chan->dir, chan->id, dar.lsb, lower_32_bits(child-
> >dar));
> > > +             SET_CH_32(dw, chan->dir, chan->id, dar.msb,
> > > + upper_32_bits(child->dar));
> > > +
> > > +             /* Transfer size */
> > > +             SET_CH_32(dw, chan->dir, chan->id, transfer_size,
> > > + child->sz);
> > > +
> > > +             /* Interrupt setup */
> > > +             val = GET_CH_32(dw, chan->dir, chan->id, int_setup) |
> > > +                             HDMA_V0_STOP_INT_MASK |
> > HDMA_V0_ABORT_INT_MASK |
> > > +                             HDMA_V0_LOCAL_STOP_INT_EN |
> > > + HDMA_V0_LOCAL_ABORT_INT_EN;
> > > +
> > > +             if (!(dw->chip->flags & DW_EDMA_CHIP_LOCAL))
> > > +                     val |= HDMA_V0_REMOTE_STOP_INT_EN |
> > > + HDMA_V0_REMOTE_ABORT_INT_EN;
> > > +
> > > +             SET_CH_32(dw, chan->dir, chan->id, int_setup, val);
> > > +
> > > +             /* Channel control setup */
> > > +             val = GET_CH_32(dw, chan->dir, chan->id, control1);
> > > +             val &= ~HDMA_V0_LINKLIST_EN;
> > > +             SET_CH_32(dw, chan->dir, chan->id, control1, val);
> > > +
> > > +             /* Ring the doorbell */
> > > +             SET_CH_32(dw, chan->dir, chan->id, doorbell,
> > HDMA_V0_DOORBELL_START);
> > > +     }
> > > +}
> > > +
> > > +static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool
> > > +first) {
> > > +     struct dw_edma_chan *chan = chunk->chan;
> > > +
> > > +     if (!chan->non_ll_en)
> > > +             dw_hdma_v0_ll_start(chunk, first);
> > > +     else
> > > +             dw_hdma_v0_non_ll_start(chunk); }
> > > +
> > >  static void dw_hdma_v0_core_ch_config(struct dw_edma_chan *chan)  {
> > >       struct dw_edma *dw = chan->dw; diff --git
> > > a/include/linux/dma/edma.h b/include/linux/dma/edma.h index
> > > 3080747689f6..82d808013a66 100644
> > > --- a/include/linux/dma/edma.h
> > > +++ b/include/linux/dma/edma.h
> > > @@ -101,6 +101,14 @@ struct dw_edma_chip {
> > >       struct dw_edma          *dw;
> > >  };
> > >
> > > +/**
> > > + * struct dw_edma_peripheral_config - peripheral spicific configurations
> > > + * @non_ll_en:                enable non-linked list mode of operations
> > > + */
> > > +struct dw_edma_peripheral_config {
> > > +     bool                    non_ll_en;
> > > +};
> > > +
> > >  /* Export to the platform drivers */  #if
> > > IS_REACHABLE(CONFIG_DW_EDMA)  int dw_edma_probe(struct
> dw_edma_chip
> > > *chip);
> > > --
> > > 2.43.0
> >
> > --
> > ~Vinod

Re: [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support

[Vinod Koul]

On 02-07-25, 09:38, Verma, Devendra wrote:
> > On 23-06-25, 11:47, Devendra K Verma wrote:
> > > The HDMA IP supports the simple mode (non-linked list).
> > > In this mode the channel registers are configured to initiate a single
> > > DMA data transfer. The channel can be configured in simple mode via
> > > peripheral param of dma_slave_config param.
> > >
> > > Signed-off-by: Devendra K Verma <devverma@amd.com>
> > > ---
> > >  drivers/dma/dw-edma/dw-edma-core.c    | 10 +++++
> > >  drivers/dma/dw-edma/dw-edma-core.h    |  2 +
> > >  drivers/dma/dw-edma/dw-hdma-v0-core.c | 53
> > ++++++++++++++++++++++++++-
> > >  include/linux/dma/edma.h              |  8 ++++
> > >  4 files changed, 72 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/dma/dw-edma/dw-edma-core.c
> > > b/drivers/dma/dw-edma/dw-edma-core.c
> > > index c2b88cc99e5d..4dafd6554277 100644
> > > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > > @@ -235,9 +235,19 @@ static int dw_edma_device_config(struct dma_chan
> > *dchan,
> > >                                struct dma_slave_config *config)  {
> > >       struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> > > +     struct dw_edma_peripheral_config *pconfig = config->peripheral_config;
> > > +     unsigned long flags;
> > > +
> > > +     if (WARN_ON(config->peripheral_config &&
> > > +                 config->peripheral_size != sizeof(*pconfig)))
> > > +             return -EINVAL;
> > >
> > > +     spin_lock_irqsave(&chan->vc.lock, flags);
> > >       memcpy(&chan->config, config, sizeof(*config));
> > > +
> > > +     chan->non_ll_en = pconfig ? pconfig->non_ll_en : false;
> > >       chan->configured = true;
> > > +     spin_unlock_irqrestore(&chan->vc.lock, flags);
> > >
> > >       return 0;
> > >  }
> > > diff --git a/drivers/dma/dw-edma/dw-edma-core.h
> > > b/drivers/dma/dw-edma/dw-edma-core.h
> > > index 71894b9e0b15..c0266976aa22 100644
> > > --- a/drivers/dma/dw-edma/dw-edma-core.h
> > > +++ b/drivers/dma/dw-edma/dw-edma-core.h
> > > @@ -86,6 +86,8 @@ struct dw_edma_chan {
> > >       u8                              configured;
> > >
> > >       struct dma_slave_config         config;
> > > +
> > > +     bool                            non_ll_en;
> >
> > why do you need this? What is the decision to use non ll vs ll one?
> 
> The IP supports both the modes, LL mode and non-LL mode.
> In the current driver code, the support for non-LL mode is not
> present. This patch enables the non-LL aka simple mode support
> by means of the peripheral_config option in the dmaengine_slave_config.

That does not answer my question, what decides which mode should be
used?

-- 
~Vinod

Re: [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support

[Manivannan Sadhasivam]

On Mon, Jun 23, 2025 at 11:47:33AM GMT, Devendra K Verma wrote:
> The HDMA IP supports the simple mode (non-linked list).
> In this mode the channel registers are configured to initiate
> a single DMA data transfer. The channel can be configured in
> simple mode via peripheral param of dma_slave_config param.
> 
> Signed-off-by: Devendra K Verma <devverma@amd.com>
> ---
>  drivers/dma/dw-edma/dw-edma-core.c    | 10 +++++
>  drivers/dma/dw-edma/dw-edma-core.h    |  2 +
>  drivers/dma/dw-edma/dw-hdma-v0-core.c | 53 ++++++++++++++++++++++++++-
>  include/linux/dma/edma.h              |  8 ++++
>  4 files changed, 72 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index c2b88cc99e5d..4dafd6554277 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -235,9 +235,19 @@ static int dw_edma_device_config(struct dma_chan *dchan,
>  				 struct dma_slave_config *config)
>  {
>  	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> +	struct dw_edma_peripheral_config *pconfig = config->peripheral_config;
> +	unsigned long flags;
> +
> +	if (WARN_ON(config->peripheral_config &&
> +		    config->peripheral_size != sizeof(*pconfig)))
> +		return -EINVAL;
>  
> +	spin_lock_irqsave(&chan->vc.lock, flags);
>  	memcpy(&chan->config, config, sizeof(*config));
> +
> +	chan->non_ll_en = pconfig ? pconfig->non_ll_en : false;

Who is allocating 'dw_edma_peripheral_config' and setting 'non_ll_en' flag? We
cannot introduce a flag without anyone using it in upstream.

- Mani

>  	chan->configured = true;
> +	spin_unlock_irqrestore(&chan->vc.lock, flags);
>  
>  	return 0;
>  }
> diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
> index 71894b9e0b15..c0266976aa22 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.h
> +++ b/drivers/dma/dw-edma/dw-edma-core.h
> @@ -86,6 +86,8 @@ struct dw_edma_chan {
>  	u8				configured;
>  
>  	struct dma_slave_config		config;
> +
> +	bool				non_ll_en;
>  };
>  
>  struct dw_edma_irq {
> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> index e3f8db4fe909..3237c807a18e 100644
> --- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> @@ -225,7 +225,7 @@ static void dw_hdma_v0_sync_ll_data(struct dw_edma_chunk *chunk)
>  		readl(chunk->ll_region.vaddr.io);
>  }
>  
> -static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> +static void dw_hdma_v0_ll_start(struct dw_edma_chunk *chunk, bool first)
>  {
>  	struct dw_edma_chan *chan = chunk->chan;
>  	struct dw_edma *dw = chan->dw;
> @@ -263,6 +263,57 @@ static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
>  	SET_CH_32(dw, chan->dir, chan->id, doorbell, HDMA_V0_DOORBELL_START);
>  }
>  
> +static void dw_hdma_v0_non_ll_start(struct dw_edma_chunk *chunk)
> +{
> +	struct dw_edma_chan *chan = chunk->chan;
> +	struct dw_edma *dw = chan->dw;
> +	struct dw_edma_burst *child;
> +	u32 val;
> +
> +	list_for_each_entry(child, &chunk->burst->list, list) {
> +		SET_CH_32(dw, chan->dir, chan->id, ch_en, BIT(0));
> +
> +		/* Source address */
> +		SET_CH_32(dw, chan->dir, chan->id, sar.lsb, lower_32_bits(child->sar));
> +		SET_CH_32(dw, chan->dir, chan->id, sar.msb, upper_32_bits(child->sar));
> +
> +		/* Destination address */
> +		SET_CH_32(dw, chan->dir, chan->id, dar.lsb, lower_32_bits(child->dar));
> +		SET_CH_32(dw, chan->dir, chan->id, dar.msb, upper_32_bits(child->dar));
> +
> +		/* Transfer size */
> +		SET_CH_32(dw, chan->dir, chan->id, transfer_size, child->sz);
> +
> +		/* Interrupt setup */
> +		val = GET_CH_32(dw, chan->dir, chan->id, int_setup) |
> +				HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK |
> +				HDMA_V0_LOCAL_STOP_INT_EN | HDMA_V0_LOCAL_ABORT_INT_EN;
> +
> +		if (!(dw->chip->flags & DW_EDMA_CHIP_LOCAL))
> +			val |= HDMA_V0_REMOTE_STOP_INT_EN | HDMA_V0_REMOTE_ABORT_INT_EN;
> +
> +		SET_CH_32(dw, chan->dir, chan->id, int_setup, val);
> +
> +		/* Channel control setup */
> +		val = GET_CH_32(dw, chan->dir, chan->id, control1);
> +		val &= ~HDMA_V0_LINKLIST_EN;
> +		SET_CH_32(dw, chan->dir, chan->id, control1, val);
> +
> +		/* Ring the doorbell */
> +		SET_CH_32(dw, chan->dir, chan->id, doorbell, HDMA_V0_DOORBELL_START);
> +	}
> +}
> +
> +static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> +{
> +	struct dw_edma_chan *chan = chunk->chan;
> +
> +	if (!chan->non_ll_en)
> +		dw_hdma_v0_ll_start(chunk, first);
> +	else
> +		dw_hdma_v0_non_ll_start(chunk);
> +}
> +
>  static void dw_hdma_v0_core_ch_config(struct dw_edma_chan *chan)
>  {
>  	struct dw_edma *dw = chan->dw;
> diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> index 3080747689f6..82d808013a66 100644
> --- a/include/linux/dma/edma.h
> +++ b/include/linux/dma/edma.h
> @@ -101,6 +101,14 @@ struct dw_edma_chip {
>  	struct dw_edma		*dw;
>  };
>  
> +/**
> + * struct dw_edma_peripheral_config - peripheral spicific configurations
> + * @non_ll_en:		 enable non-linked list mode of operations
> + */
> +struct dw_edma_peripheral_config {
> +	bool			non_ll_en;
> +};
> +
>  /* Export to the platform drivers */
>  #if IS_REACHABLE(CONFIG_DW_EDMA)
>  int dw_edma_probe(struct dw_edma_chip *chip);
> -- 
> 2.43.0
> 

-- 
மணிவண்ணன் சதாசிவம்

RE: [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support

[Verma, Devendra]

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Vinod

Please check the response inline.

Regards,
Devendra

> -----Original Message-----
> From: Vinod Koul <vkoul@kernel.org>
> Sent: Wednesday, July 23, 2025 12:51
> To: Verma, Devendra <Devendra.Verma@amd.com>
> Cc: dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org; mani@kernel.org
> Subject: Re: [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On 02-07-25, 09:38, Verma, Devendra wrote:
> > > On 23-06-25, 11:47, Devendra K Verma wrote:
> > > > The HDMA IP supports the simple mode (non-linked list).
> > > > In this mode the channel registers are configured to initiate a
> > > > single DMA data transfer. The channel can be configured in simple
> > > > mode via peripheral param of dma_slave_config param.
> > > >
> > > > Signed-off-by: Devendra K Verma <devverma@amd.com>
> > > > ---
> > > >  drivers/dma/dw-edma/dw-edma-core.c    | 10 +++++
> > > >  drivers/dma/dw-edma/dw-edma-core.h    |  2 +
> > > >  drivers/dma/dw-edma/dw-hdma-v0-core.c | 53
> > > ++++++++++++++++++++++++++-
> > > >  include/linux/dma/edma.h              |  8 ++++
> > > >  4 files changed, 72 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/dma/dw-edma/dw-edma-core.c
> > > > b/drivers/dma/dw-edma/dw-edma-core.c
> > > > index c2b88cc99e5d..4dafd6554277 100644
> > > > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > > > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > > > @@ -235,9 +235,19 @@ static int dw_edma_device_config(struct
> > > > dma_chan
> > > *dchan,
> > > >                                struct dma_slave_config *config)  {
> > > >       struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> > > > +     struct dw_edma_peripheral_config *pconfig = config->peripheral_config;
> > > > +     unsigned long flags;
> > > > +
> > > > +     if (WARN_ON(config->peripheral_config &&
> > > > +                 config->peripheral_size != sizeof(*pconfig)))
> > > > +             return -EINVAL;
> > > >
> > > > +     spin_lock_irqsave(&chan->vc.lock, flags);
> > > >       memcpy(&chan->config, config, sizeof(*config));
> > > > +
> > > > +     chan->non_ll_en = pconfig ? pconfig->non_ll_en : false;
> > > >       chan->configured = true;
> > > > +     spin_unlock_irqrestore(&chan->vc.lock, flags);
> > > >
> > > >       return 0;
> > > >  }
> > > > diff --git a/drivers/dma/dw-edma/dw-edma-core.h
> > > > b/drivers/dma/dw-edma/dw-edma-core.h
> > > > index 71894b9e0b15..c0266976aa22 100644
> > > > --- a/drivers/dma/dw-edma/dw-edma-core.h
> > > > +++ b/drivers/dma/dw-edma/dw-edma-core.h
> > > > @@ -86,6 +86,8 @@ struct dw_edma_chan {
> > > >       u8                              configured;
> > > >
> > > >       struct dma_slave_config         config;
> > > > +
> > > > +     bool                            non_ll_en;
> > >
> > > why do you need this? What is the decision to use non ll vs ll one?
> >
> > The IP supports both the modes, LL mode and non-LL mode.
> > In the current driver code, the support for non-LL mode is not
> > present. This patch enables the non-LL aka simple mode support by
> > means of the peripheral_config option in the dmaengine_slave_config.
>
> That does not answer my question, what decides which mode should be used?
>

Simple HDMA mode is useful when there is no need for scatter-gather and
one big chunk of data from contiguous location is to be moved. It saves the effort of
preparing a linked list of descriptors as DMA channel registers can be programmed
directly.

> --
> ~Vinod

RE: [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support

[Verma, Devendra]

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Manivannan

Please check my response inline.

Regards,
Devendra

> -----Original Message-----
> From: Manivannan Sadhasivam <mani@kernel.org>
> Sent: Wednesday, July 23, 2025 19:26
> To: Verma, Devendra <Devendra.Verma@amd.com>
> Cc: dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org; vkoul@kernel.org
> Subject: Re: [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Mon, Jun 23, 2025 at 11:47:33AM GMT, Devendra K Verma wrote:
> > The HDMA IP supports the simple mode (non-linked list).
> > In this mode the channel registers are configured to initiate a single
> > DMA data transfer. The channel can be configured in simple mode via
> > peripheral param of dma_slave_config param.
> >
> > Signed-off-by: Devendra K Verma <devverma@amd.com>
> > ---
> >  drivers/dma/dw-edma/dw-edma-core.c    | 10 +++++
> >  drivers/dma/dw-edma/dw-edma-core.h    |  2 +
> >  drivers/dma/dw-edma/dw-hdma-v0-core.c | 53
> ++++++++++++++++++++++++++-
> >  include/linux/dma/edma.h              |  8 ++++
> >  4 files changed, 72 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.c
> > b/drivers/dma/dw-edma/dw-edma-core.c
> > index c2b88cc99e5d..4dafd6554277 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > @@ -235,9 +235,19 @@ static int dw_edma_device_config(struct dma_chan
> *dchan,
> >                                struct dma_slave_config *config)  {
> >       struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> > +     struct dw_edma_peripheral_config *pconfig = config->peripheral_config;
> > +     unsigned long flags;
> > +
> > +     if (WARN_ON(config->peripheral_config &&
> > +                 config->peripheral_size != sizeof(*pconfig)))
> > +             return -EINVAL;
> >
> > +     spin_lock_irqsave(&chan->vc.lock, flags);
> >       memcpy(&chan->config, config, sizeof(*config));
> > +
> > +     chan->non_ll_en = pconfig ? pconfig->non_ll_en : false;
>
> Who is allocating 'dw_edma_peripheral_config' and setting 'non_ll_en' flag? We
> cannot introduce a flag without anyone using it in upstream.
>
> - Mani
>

The caller of the DMA engine client APIs will be responsible for allocating
and setting the non_ll_en flag. This flag helps in setting the channel in to
non-Linked-List mode. This functionality is supported by the DMA controller.
The peripheral_config is just configuring the peripheral (DMA) in to the modes
it supports based on user discretion. As this functionality is used by the
clients to differentiate between the modes supported by the controller it
may not require the equivalent code in upstream.

> >       chan->configured = true;
> > +     spin_unlock_irqrestore(&chan->vc.lock, flags);
> >
> >       return 0;
> >  }
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.h
> > b/drivers/dma/dw-edma/dw-edma-core.h
> > index 71894b9e0b15..c0266976aa22 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.h
> > +++ b/drivers/dma/dw-edma/dw-edma-core.h
> > @@ -86,6 +86,8 @@ struct dw_edma_chan {
> >       u8                              configured;
> >
> >       struct dma_slave_config         config;
> > +
> > +     bool                            non_ll_en;
> >  };
> >
> >  struct dw_edma_irq {
> > diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c
> > b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> > index e3f8db4fe909..3237c807a18e 100644
> > --- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
> > +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> > @@ -225,7 +225,7 @@ static void dw_hdma_v0_sync_ll_data(struct
> dw_edma_chunk *chunk)
> >               readl(chunk->ll_region.vaddr.io);  }
> >
> > -static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool
> > first)
> > +static void dw_hdma_v0_ll_start(struct dw_edma_chunk *chunk, bool
> > +first)
> >  {
> >       struct dw_edma_chan *chan = chunk->chan;
> >       struct dw_edma *dw = chan->dw;
> > @@ -263,6 +263,57 @@ static void dw_hdma_v0_core_start(struct
> dw_edma_chunk *chunk, bool first)
> >       SET_CH_32(dw, chan->dir, chan->id, doorbell,
> > HDMA_V0_DOORBELL_START);  }
> >
> > +static void dw_hdma_v0_non_ll_start(struct dw_edma_chunk *chunk) {
> > +     struct dw_edma_chan *chan = chunk->chan;
> > +     struct dw_edma *dw = chan->dw;
> > +     struct dw_edma_burst *child;
> > +     u32 val;
> > +
> > +     list_for_each_entry(child, &chunk->burst->list, list) {
> > +             SET_CH_32(dw, chan->dir, chan->id, ch_en, BIT(0));
> > +
> > +             /* Source address */
> > +             SET_CH_32(dw, chan->dir, chan->id, sar.lsb, lower_32_bits(child-
> >sar));
> > +             SET_CH_32(dw, chan->dir, chan->id, sar.msb,
> > + upper_32_bits(child->sar));
> > +
> > +             /* Destination address */
> > +             SET_CH_32(dw, chan->dir, chan->id, dar.lsb, lower_32_bits(child-
> >dar));
> > +             SET_CH_32(dw, chan->dir, chan->id, dar.msb,
> > + upper_32_bits(child->dar));
> > +
> > +             /* Transfer size */
> > +             SET_CH_32(dw, chan->dir, chan->id, transfer_size,
> > + child->sz);
> > +
> > +             /* Interrupt setup */
> > +             val = GET_CH_32(dw, chan->dir, chan->id, int_setup) |
> > +                             HDMA_V0_STOP_INT_MASK |
> HDMA_V0_ABORT_INT_MASK |
> > +                             HDMA_V0_LOCAL_STOP_INT_EN |
> > + HDMA_V0_LOCAL_ABORT_INT_EN;
> > +
> > +             if (!(dw->chip->flags & DW_EDMA_CHIP_LOCAL))
> > +                     val |= HDMA_V0_REMOTE_STOP_INT_EN |
> > + HDMA_V0_REMOTE_ABORT_INT_EN;
> > +
> > +             SET_CH_32(dw, chan->dir, chan->id, int_setup, val);
> > +
> > +             /* Channel control setup */
> > +             val = GET_CH_32(dw, chan->dir, chan->id, control1);
> > +             val &= ~HDMA_V0_LINKLIST_EN;
> > +             SET_CH_32(dw, chan->dir, chan->id, control1, val);
> > +
> > +             /* Ring the doorbell */
> > +             SET_CH_32(dw, chan->dir, chan->id, doorbell,
> HDMA_V0_DOORBELL_START);
> > +     }
> > +}
> > +
> > +static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool
> > +first) {
> > +     struct dw_edma_chan *chan = chunk->chan;
> > +
> > +     if (!chan->non_ll_en)
> > +             dw_hdma_v0_ll_start(chunk, first);
> > +     else
> > +             dw_hdma_v0_non_ll_start(chunk); }
> > +
> >  static void dw_hdma_v0_core_ch_config(struct dw_edma_chan *chan)  {
> >       struct dw_edma *dw = chan->dw;
> > diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h index
> > 3080747689f6..82d808013a66 100644
> > --- a/include/linux/dma/edma.h
> > +++ b/include/linux/dma/edma.h
> > @@ -101,6 +101,14 @@ struct dw_edma_chip {
> >       struct dw_edma          *dw;
> >  };
> >
> > +/**
> > + * struct dw_edma_peripheral_config - peripheral spicific configurations
> > + * @non_ll_en:                enable non-linked list mode of operations
> > + */
> > +struct dw_edma_peripheral_config {
> > +     bool                    non_ll_en;
> > +};
> > +
> >  /* Export to the platform drivers */
> >  #if IS_REACHABLE(CONFIG_DW_EDMA)
> >  int dw_edma_probe(struct dw_edma_chip *chip);
> > --
> > 2.43.0
> >
>
> --
> மணிவண்ணன் சதாசிவம்

Re: [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support

[Manivannan Sadhasivam]

On Thu, Jul 31, 2025 at 09:32:12AM GMT, Verma, Devendra wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
> 
> Hi Manivannan
> 
> Please check my response inline.
> 
> Regards,
> Devendra
> 
> > -----Original Message-----
> > From: Manivannan Sadhasivam <mani@kernel.org>
> > Sent: Wednesday, July 23, 2025 19:26
> > To: Verma, Devendra <Devendra.Verma@amd.com>
> > Cc: dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org; vkoul@kernel.org
> > Subject: Re: [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support
> >
> > Caution: This message originated from an External Source. Use proper caution
> > when opening attachments, clicking links, or responding.
> >
> >
> > On Mon, Jun 23, 2025 at 11:47:33AM GMT, Devendra K Verma wrote:
> > > The HDMA IP supports the simple mode (non-linked list).
> > > In this mode the channel registers are configured to initiate a single
> > > DMA data transfer. The channel can be configured in simple mode via
> > > peripheral param of dma_slave_config param.
> > >
> > > Signed-off-by: Devendra K Verma <devverma@amd.com>
> > > ---
> > >  drivers/dma/dw-edma/dw-edma-core.c    | 10 +++++
> > >  drivers/dma/dw-edma/dw-edma-core.h    |  2 +
> > >  drivers/dma/dw-edma/dw-hdma-v0-core.c | 53
> > ++++++++++++++++++++++++++-
> > >  include/linux/dma/edma.h              |  8 ++++
> > >  4 files changed, 72 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/dma/dw-edma/dw-edma-core.c
> > > b/drivers/dma/dw-edma/dw-edma-core.c
> > > index c2b88cc99e5d..4dafd6554277 100644
> > > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > > @@ -235,9 +235,19 @@ static int dw_edma_device_config(struct dma_chan
> > *dchan,
> > >                                struct dma_slave_config *config)  {
> > >       struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> > > +     struct dw_edma_peripheral_config *pconfig = config->peripheral_config;
> > > +     unsigned long flags;
> > > +
> > > +     if (WARN_ON(config->peripheral_config &&
> > > +                 config->peripheral_size != sizeof(*pconfig)))
> > > +             return -EINVAL;
> > >
> > > +     spin_lock_irqsave(&chan->vc.lock, flags);
> > >       memcpy(&chan->config, config, sizeof(*config));
> > > +
> > > +     chan->non_ll_en = pconfig ? pconfig->non_ll_en : false;
> >
> > Who is allocating 'dw_edma_peripheral_config' and setting 'non_ll_en' flag? We
> > cannot introduce a flag without anyone using it in upstream.
> >
> > - Mani
> >
> 
> The caller of the DMA engine client APIs will be responsible for allocating
> and setting the non_ll_en flag. This flag helps in setting the channel in to
> non-Linked-List mode. This functionality is supported by the DMA controller.
> The peripheral_config is just configuring the peripheral (DMA) in to the modes
> it supports based on user discretion. As this functionality is used by the
> clients to differentiate between the modes supported by the controller it
> may not require the equivalent code in upstream.
> 

As I stated above, we *must not* have an unused interface in upstream drivers. I
understand that fact that your downstream driver is setting this flag and you
want to keep your driver downstream. But the flag would be unused in upstream
and that is not acceptable, sorry.

- Mani


-- 
மணிவண்ணன் சதாசிவம்