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

[Bjorn Helgaas <helgaas@kernel.org>]

On Fri, Sep 05, 2025 at 03:46:59PM +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:
> - When a valid physical base address is not configured via the
>   Xilinx VSEC capability then the IP can still be used in non-LL
>   mode. The default mode for all the DMA transactions and for all
>   the DMA channels then is non-LL mode.
> - When a valid physical base address is configured but the client
>   wants to use the non-LL mode for DMA transactions then also the
>   flexibility is provided via the peripheral_config struct member of
>   dma_slave_config. In this case the channels can be individually
>   configured in non-LL mode. This use case is desirable for single
>   DMA transfer of a chunk, this saves the effort of preparing the
>   Link List.

> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -223,8 +223,28 @@ static int dw_edma_device_config(struct dma_chan *dchan,
>  				 struct dma_slave_config *config)
>  {
>  	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> +	int nollp = 0;
> +
> +	if (WARN_ON(config->peripheral_config &&
> +		    config->peripheral_size != sizeof(int)))
> +		return -EINVAL;
>  
>  	memcpy(&chan->config, config, sizeof(*config));
> +
> +	/*
> +	 * When there is no valid LLP base address available
> +	 * then the default DMA ops will use the non-LL mode.
> +	 * Cases where LL mode is enabled and client wants
> +	 * to use the non-LL mode then also client can do
> +	 * so via the providing the peripheral_config param.

s/via the/via/

> +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> @@ -224,6 +224,15 @@ static void dw_edma_pcie_get_vsec_dma_data(struct pci_dev *pdev,
>  	pdata->phys_addr = off;
>  }
>  
> +static u64 dw_edma_get_phys_addr(struct pci_dev *pdev,
> +				 struct dw_edma_pcie_data *pdata,
> +				 enum pci_barno bar)
> +{
> +	if (pdev->vendor == PCI_VENDOR_ID_XILINX)
> +		return pdata->phys_addr;
> +	return pci_bus_address(pdev, bar);

This doesn't seem right.  pci_bus_address() returns pci_bus_addr_t, so
pdata->phys_addr should also be a pci_bus_addr_t, and the function
should return pci_bus_addr_t.

A pci_bus_addr_t is not a "phys_addr"; it is an address that is valid
on the PCI side of a PCI host bridge, which may be different than the
CPU physical address on the CPU side of the bridge because of things
like IOMMUs.

Seems like the struct dw_edma_region.paddr should be renamed to
something like "bus_addr" and made into a pci_bus_addr_t.

Bjorn