Re: [PATCH 22/25] dmaengine: dw-edma: Replace chip ID number with device name

[Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>]

On Thu, Mar 24, 2022 at 04:48:33AM +0300, Serge Semin wrote:
> Using some abstract number as the DW eDMA chip identifier isn't really
> practical. First of all there can be more than one DW eDMA controller on
> the platform some of them can be detected as the PCIe end-points, some of
> them can be embedded into the DW PCIe Root Port/End-point controllers.
> Seeing some abstract number in for instance IRQ handlers list doesn't give
> a notion regarding their reference to the particular DMA controller.
> Secondly current DW eDMA chip id implementation doesn't provide the
> multi-eDMA platforms support for same reason of possibly having eDMA
> detected on different system buses. At the same time re-implementing
> something ida-based won't give much benefits especially seeing the DW eDMA
> chip ID is only used in the IRQ request procedure. So to speak in order to
> preserve the code simplicity and get to have the multi-eDMA platforms
> support let's just use the parental device name to create the DW eDMA
> controller name.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> ---
>  drivers/dma/dw-edma/dw-edma-core.c | 3 ++-
>  drivers/dma/dw-edma/dw-edma-core.h | 2 +-
>  drivers/dma/dw-edma/dw-edma-pcie.c | 1 -
>  include/linux/dma/edma.h           | 1 -
>  4 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index dbe1119fd1d2..72a51970bfba 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -970,7 +970,8 @@ int dw_edma_probe(struct dw_edma_chip *chip)
>  	if (!dw->chan)
>  		return -ENOMEM;
>  
> -	snprintf(dw->name, sizeof(dw->name), "dw-edma-core:%d", chip->id);
> +	snprintf(dw->name, sizeof(dw->name), "dw-edma-core:%s",
> +		 dev_name(chip->dev));
>  
>  	/* Disable eDMA, only to establish the ideal initial conditions */
>  	dw_edma_v0_core_off(dw);
> diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
> index 980adb079182..dc25798d4ba9 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.h
> +++ b/drivers/dma/dw-edma/dw-edma-core.h
> @@ -96,7 +96,7 @@ struct dw_edma_irq {
>  };
>  
>  struct dw_edma {
> -	char				name[20];
> +	char				name[30];

I'm not sure if this length is sufficient. Other than this,

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

>  
>  	struct dma_device		dma;
>  
> diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
> index f530bacfd716..3f9dadc73854 100644
> --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> @@ -222,7 +222,6 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
>  
>  	/* Data structure initialization */
>  	chip->dev = dev;
> -	chip->id = pdev->devfn;
>  
>  	chip->mf = vsec_data.mf;
>  	chip->nr_irqs = nr_irqs;
> diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> index 5cc87cfdd685..241c5a97ddf4 100644
> --- a/include/linux/dma/edma.h
> +++ b/include/linux/dma/edma.h
> @@ -73,7 +73,6 @@ enum dw_edma_map_format {
>   */
>  struct dw_edma_chip {
>  	struct device		*dev;
> -	int			id;
>  	int			nr_irqs;
>  	const struct dw_edma_core_ops   *ops;
>  	u32			flags;
> -- 
> 2.35.1
>