Re: [PATCH v15 2/2] dmaengine: dw-edma: Add non-LL mode

["Verma, Devendra" <devverma@amd.com>]

Hi Koichiro

Please check my comments inline.

Regards,
Devendra

On 17-Jun-26 08:47, Koichiro Den wrote:
> On Wed, Mar 18, 2026 at 12:34:03PM +0530, Devendra K Verma wrote:
>> AMD MDB IP supports Linked List (LL) mode as well as non-LL mode.
>> The current code does not have the mechanisms to enable the
>> DMA transactions using the non-LL mode. The following two cases
>> are added with this patch:
>> - For the AMD (Xilinx) only, when a valid physical base address of
>>    the device side DDR is not configured, then the IP can still be
>>    used in non-LL mode. For all the channels DMA transactions will
>>    be using the non-LL mode only. This, the default non-LL mode,
>>    is not applicable for Synopsys IP with the current code addition.
>>
>> - If the default mode is LL-mode, for both AMD (Xilinx) and Synosys,
>>    and if user wants to use non-LL mode then user can do so via
>>    configuring the peripheral_config param of dma_slave_config.
>>
>> Signed-off-by: Devendra K Verma <devendra.verma@amd.com>
>> Reviewed-by: Frank Li <Frank.Li@nxp.com>
>> ---
>> Changes in v15
>>     Rebased the branch
>>
> [snip]
>> +
>> +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)
> 
> Hi Devendra (cc: Frank),
> 
> Sorry for dropping a comment now that this has already landed.
> 
> I'm wondering about the lifetime of chan->non_ll. This patch lets a client
> select non-LL mode through dma_slave_config.peripheral_config for a transfer,
> but the state is stored on the channel.
> 
> We use chan->non_ll in prep to choose bursts_max, then read it again later in
> dw_hdma_v0_core_start() to choose the LL vs non-LL start path. If the channel is
> reconfigured between prep and start, or before a later chunk is started from the
> interrupt path, couldn't we start a descriptor in a different mode from the one
> it was prepared for?
> 

The mode is implemented with the intention that after prep, the 
submitted descriptor shall completed with the chosen mode. So, yes the 
mode is decided in the prep call and all the subsequent descriptors are 
completed with the chosen mode unless it is overridden by another prep call.

> (Note: Frank's not-yet-merged dma_prep_config v7 series [1] also looks at
> potential races around config+prep on the same channel from multiple process
> contexts, as I understand it. But this seems like a separate issue, since the
> state is read again at transfer start time.)
> 
> Should non_ll be snapshotted into the descriptor/chunk, maybe as
> dw_edma_desc.non_ll, or is the rule that clients must not reconfigure the
> channel while anything is pending/running?
> 
> Or was this already discussed, and there is some implicit restriction that
> clients must not mix LL and temporary non-LL requests from multiple contexts on
> the same channel?

I am not aware of any such rule which specifies that modes can not be 
mixed but it would not be a good idea to mix both. Let me give an 
example, in the non-LL mode the channels *can* utilize the LL-regions 
for data transfers. If for such a non-LL data transfer where LL-region 
is used and intended by the user then changing the mode after setting up 
the mode to another one can cause data corruption.
Eg:
Channel LL-region = ADDR
Mode set to non-LL -> DDR destination to ADDR to (ADDR + size)
First non-LL burst -> writes data to ADDR till size bytes.
Second burst configured for LL -> overwrites the data at ADDR with 
descriptor information.

This one causes the data corruption for the first burst.

> 
> [1] https://lore.kernel.org/dmaengine/20260521-dma_prep_config-v7-0-1f73f4899883@nxp.com/
> 
> Thanks,
> Koichiro
> 
>> +		dw_hdma_v0_core_non_ll_start(chunk);
>> +	else
>> +		dw_hdma_v0_core_ll_start(chunk, first);
>> +}
>> +
>>   static void dw_hdma_v0_core_ch_config(struct dw_edma_chan *chan)
>>   {
>>   	struct dw_edma *dw = chan->dw;
>> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-regs.h b/drivers/dma/dw-edma/dw-hdma-v0-regs.h
>> index eab5fd7177e5..7759ba9b4850 100644
>> --- a/drivers/dma/dw-edma/dw-hdma-v0-regs.h
>> +++ b/drivers/dma/dw-edma/dw-hdma-v0-regs.h
>> @@ -12,6 +12,7 @@
>>   #include <linux/dmaengine.h>
>>   
>>   #define HDMA_V0_MAX_NR_CH			8
>> +#define HDMA_V0_CH_EN				BIT(0)
>>   #define HDMA_V0_LOCAL_ABORT_INT_EN		BIT(6)
>>   #define HDMA_V0_REMOTE_ABORT_INT_EN		BIT(5)
>>   #define HDMA_V0_LOCAL_STOP_INT_EN		BIT(4)
>> diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
>> index 270b5458aecf..61d6064fcfed 100644
>> --- a/include/linux/dma/edma.h
>> +++ b/include/linux/dma/edma.h
>> @@ -97,6 +97,7 @@ struct dw_edma_chip {
>>   	enum dw_edma_map_format	mf;
>>   
>>   	struct dw_edma		*dw;
>> +	bool			cfg_non_ll;
>>   };
>>   
>>   /* Export to the platform drivers */
>> -- 
>> 2.43.0
>>