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

Vinod Koul
2019-01-20T11:12:51Z

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