Re: [PATCH 3/8 v4] dma: k3dma: Upgrade k3dma driver to support hisi_asp_dma hardware

[Vinod Koul <vkoul@kernel.org>]

On 16-01-19, 09:10, John Stultz wrote:
> From: Youlin Wang <wwx575822@notesmail.huawei.com>
> 
> On the hi3660 hardware there are two (at least) DMA controllers,
> the DMA-P (Peripherial DMA) and the DMA-A (Audio DMA). The
                    ^^^
typo

> two blocks are similar, but have some slight differences. This
> resulted in the vendor implementing two separate drivers, which
> after review, they have been able to condense and re-use the
> existing k3dma driver.
> 
> Thus, this patch adds support for the new "hisi-pcm-asp-dma-1.0"
> compatible string in the binding.
> 
> One difference with the DMA-A controller, is that it does not
> need to initialize a clock. So we skip this by adding and using
> soc data flags.
> 
> After above this driver will support both k3 and hisi_asp dma
> hardware.

Great thanks for the effort!

> 
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Vinod Koul <vkoul@kernel.org>
> Cc: Zhuangluan Su <suzhuangluan@hisilicon.com>
> Cc: Ryan Grachek <ryan@edited.us>
> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Cc: dmaengine@vger.kernel.org
> Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Signed-off-by: Youlin Wang <wwx575822@notesmail.huawei.com>
> Signed-off-by: Tanglei Han <hantanglei@huawei.com>
> [jstultz: Reworked to use of_match_data, commit msg improvements]
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
> v2:
> * Reworked to use of_match_data
> v3:
> * Further rework of the commit message
> ---
>  drivers/dma/k3dma.c | 37 ++++++++++++++++++++++++++++++++-----
>  1 file changed, 32 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/dma/k3dma.c b/drivers/dma/k3dma.c
> index fdec2b6..df61406 100644
> --- a/drivers/dma/k3dma.c
> +++ b/drivers/dma/k3dma.c
> @@ -116,6 +116,13 @@ struct k3_dma_dev {
>  	unsigned int		irq;
>  };
>  
> +
> +#define K3_FLAG_NOCLK  (1<<0)

why not use BIT()

space between two please

> +struct k3dma_soc_data {
> +	unsigned long flags;
> +};
> +
> +
>  #define to_k3_dma(dmadev) container_of(dmadev, struct k3_dma_dev, slave)
>  
>  static int k3_dma_config_write(struct dma_chan *chan,
> @@ -790,8 +797,21 @@ static int k3_dma_transfer_resume(struct dma_chan *chan)
>  	return 0;
>  }
>  
> +static const struct k3dma_soc_data k3_v1_dma_data = {
> +	.flags = 0,
> +};

So basically this is default right, do we need to set dma_data and not
assume default..

> +
> +static const struct k3dma_soc_data asp_v1_dma_data = {
> +	.flags = K3_FLAG_NOCLK,
> +};
> +
>  static const struct of_device_id k3_pdma_dt_ids[] = {
> -	{ .compatible = "hisilicon,k3-dma-1.0", },
> +	{ .compatible = "hisilicon,k3-dma-1.0",
> +	  .data = &k3_v1_dma_data
> +	},
> +	{ .compatible = "hisilicon,hisi-pcm-asp-dma-1.0",
> +	  .data = &asp_v1_dma_data
> +	},
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, k3_pdma_dt_ids);
> @@ -810,6 +830,7 @@ static struct dma_chan *k3_of_dma_simple_xlate(struct of_phandle_args *dma_spec,
>  
>  static int k3_dma_probe(struct platform_device *op)
>  {
> +	const struct k3dma_soc_data *soc_data;
>  	struct k3_dma_dev *d;
>  	const struct of_device_id *of_id;
>  	struct resource *iores;
> @@ -823,6 +844,10 @@ static int k3_dma_probe(struct platform_device *op)
>  	if (!d)
>  		return -ENOMEM;
>  
> +	soc_data = device_get_match_data(&op->dev);
> +	if (!soc_data)
> +		return -EINVAL;

So this is not optional, either ways fine by me :)
-- 
~Vinod