Patchwork [3/8,v2] dma: k3dma: Upgrade k3dma driver to support hisi_asp_dma hardware

login
register
mail settings
Submitter John Stultz
Date Jan. 4, 2019, 8:56 p.m.
Message ID <1546635388-13795-4-git-send-email-john.stultz@linaro.org>
Download mbox | patch
Permalink /patch/693205/
State New
Headers show

Comments

John Stultz - Jan. 4, 2019, 8:56 p.m.
From: Youlin Wang <wwx575822@notesmail.huawei.com>

There is an new "hisi-pcm-asp-dma-1.0" device added in
"arch/arm64/boot/dts/hisilicon/hi3660.dtsi".
So we have to add a matching id in the driver file:
 .compatible = "hisilicon,hisi-pcm-asp-dma-1.0"

And also hisi-pcm-asp dma device needs no setting to the 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.

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
Signed-off-by: Youlin Wang <wwx575822@notesmail.huawei.com>
Signed-off-by: Tanglei Han <hantanglei@huawei.com>
[jstultz: Reworked to use of_match_data]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
v2:
* Reworked to use of_match_data
---
 drivers/dma/k3dma.c | 37 ++++++++++++++++++++++++++++++++-----
 1 file changed, 32 insertions(+), 5 deletions(-)
Manivannan Sadhasivam - Jan. 5, 2019, 3:41 a.m.
Hi John,

On Fri, Jan 04, 2019 at 12:56:23PM -0800, John Stultz wrote:
> From: Youlin Wang <wwx575822@notesmail.huawei.com>
> 
> There is an new "hisi-pcm-asp-dma-1.0" device added in
> "arch/arm64/boot/dts/hisilicon/hi3660.dtsi".
> So we have to add a matching id in the driver file:
>  .compatible = "hisilicon,hisi-pcm-asp-dma-1.0"
> 
> And also hisi-pcm-asp dma device needs no setting to the 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.
> 

Small description about the hardware (ASP DMAC) would be really helpful.

Thanks,
Mani

> 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
> Signed-off-by: Youlin Wang <wwx575822@notesmail.huawei.com>
> Signed-off-by: Tanglei Han <hantanglei@huawei.com>
> [jstultz: Reworked to use of_match_data]
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
> v2:
> * Reworked to use of_match_data
> ---
>  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)
> +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,
> +};
> +
> +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;
> +
>  	d->base = devm_ioremap_resource(&op->dev, iores);
>  	if (IS_ERR(d->base))
>  		return PTR_ERR(d->base);
> @@ -835,10 +860,12 @@ static int k3_dma_probe(struct platform_device *op)
>  				"dma-requests", &d->dma_requests);
>  	}
>  
> -	d->clk = devm_clk_get(&op->dev, NULL);
> -	if (IS_ERR(d->clk)) {
> -		dev_err(&op->dev, "no dma clk\n");
> -		return PTR_ERR(d->clk);
> +	if (!(soc_data->flags & K3_FLAG_NOCLK)) {
> +		d->clk = devm_clk_get(&op->dev, NULL);
> +		if (IS_ERR(d->clk)) {
> +			dev_err(&op->dev, "no dma clk\n");
> +			return PTR_ERR(d->clk);
> +		}
>  	}
>  
>  	irq = platform_get_irq(op, 0);
> -- 
> 2.7.4
>
John Stultz - Jan. 5, 2019, 5:22 a.m.
On Fri, Jan 4, 2019 at 7:42 PM Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
> On Fri, Jan 04, 2019 at 12:56:23PM -0800, John Stultz wrote:
> > From: Youlin Wang <wwx575822@notesmail.huawei.com>
> >
> > There is an new "hisi-pcm-asp-dma-1.0" device added in
> > "arch/arm64/boot/dts/hisilicon/hi3660.dtsi".
> > So we have to add a matching id in the driver file:
> >  .compatible = "hisilicon,hisi-pcm-asp-dma-1.0"
> >
> > And also hisi-pcm-asp dma device needs no setting to the 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.
> >
>
> Small description about the hardware (ASP DMAC) would be really helpful.

I've taken a shot at this (along with integrating your other feedback
- thanks again for the review!), though as I don't have direct
documentation, my knowledge is a bit second hand.

See here:
https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/hikey960-mainline-WIP&id=754a79facf1af0b59a7f8fd63050da12ebf5521e

Let me know if you have suggestions for more specific changes!
thanks
-john
Manivannan Sadhasivam - Jan. 5, 2019, 5:39 a.m.
On Fri, Jan 04, 2019 at 09:22:46PM -0800, John Stultz wrote:
> On Fri, Jan 4, 2019 at 7:42 PM Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
> > On Fri, Jan 04, 2019 at 12:56:23PM -0800, John Stultz wrote:
> > > From: Youlin Wang <wwx575822@notesmail.huawei.com>
> > >
> > > There is an new "hisi-pcm-asp-dma-1.0" device added in
> > > "arch/arm64/boot/dts/hisilicon/hi3660.dtsi".
> > > So we have to add a matching id in the driver file:
> > >  .compatible = "hisilicon,hisi-pcm-asp-dma-1.0"
> > >
> > > And also hisi-pcm-asp dma device needs no setting to the 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.
> > >
> >
> > Small description about the hardware (ASP DMAC) would be really helpful.
> 
> I've taken a shot at this (along with integrating your other feedback
> - thanks again for the review!), though as I don't have direct
> documentation, my knowledge is a bit second hand.
> 
> See here:
> https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/hikey960-mainline-WIP&id=754a79facf1af0b59a7f8fd63050da12ebf5521e
> 
> Let me know if you have suggestions for more specific changes!

Description looks good to me. But looks like you are not protecting the
other clk APIs like clk_prepare_enable and clk_disable_unprepare. Don't
they fail when the relevant clk is not found?

Thanks,
Mani

> thanks
> -john
John Stultz - Jan. 5, 2019, 5:44 a.m.
On Fri, Jan 4, 2019 at 9:39 PM Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> On Fri, Jan 04, 2019 at 09:22:46PM -0800, John Stultz wrote:
> > On Fri, Jan 4, 2019 at 7:42 PM Manivannan Sadhasivam
> > <manivannan.sadhasivam@linaro.org> wrote:
> > > On Fri, Jan 04, 2019 at 12:56:23PM -0800, John Stultz wrote:
> > > > From: Youlin Wang <wwx575822@notesmail.huawei.com>
> > > >
> > > > There is an new "hisi-pcm-asp-dma-1.0" device added in
> > > > "arch/arm64/boot/dts/hisilicon/hi3660.dtsi".
> > > > So we have to add a matching id in the driver file:
> > > >  .compatible = "hisilicon,hisi-pcm-asp-dma-1.0"
> > > >
> > > > And also hisi-pcm-asp dma device needs no setting to the 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.
> > > >
> > >
> > > Small description about the hardware (ASP DMAC) would be really helpful.
> >
> > I've taken a shot at this (along with integrating your other feedback
> > - thanks again for the review!), though as I don't have direct
> > documentation, my knowledge is a bit second hand.
> >
> > See here:
> > https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/hikey960-mainline-WIP&id=754a79facf1af0b59a7f8fd63050da12ebf5521e
> >
> > Let me know if you have suggestions for more specific changes!
>
> Description looks good to me. But looks like you are not protecting the
> other clk APIs like clk_prepare_enable and clk_disable_unprepare. Don't
> they fail when the relevant clk is not found?

No, those calls just return 0 if null clock is passed

int clk_prepare(struct clk *clk)
{
        if (!clk)
                return 0;
...

thanks
-john
Manivannan Sadhasivam - Jan. 5, 2019, 5:48 a.m.
On Fri, Jan 04, 2019 at 09:44:17PM -0800, John Stultz wrote:
> On Fri, Jan 4, 2019 at 9:39 PM Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
> >
> > On Fri, Jan 04, 2019 at 09:22:46PM -0800, John Stultz wrote:
> > > On Fri, Jan 4, 2019 at 7:42 PM Manivannan Sadhasivam
> > > <manivannan.sadhasivam@linaro.org> wrote:
> > > > On Fri, Jan 04, 2019 at 12:56:23PM -0800, John Stultz wrote:
> > > > > From: Youlin Wang <wwx575822@notesmail.huawei.com>
> > > > >
> > > > > There is an new "hisi-pcm-asp-dma-1.0" device added in
> > > > > "arch/arm64/boot/dts/hisilicon/hi3660.dtsi".
> > > > > So we have to add a matching id in the driver file:
> > > > >  .compatible = "hisilicon,hisi-pcm-asp-dma-1.0"
> > > > >
> > > > > And also hisi-pcm-asp dma device needs no setting to the 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.
> > > > >
> > > >
> > > > Small description about the hardware (ASP DMAC) would be really helpful.
> > >
> > > I've taken a shot at this (along with integrating your other feedback
> > > - thanks again for the review!), though as I don't have direct
> > > documentation, my knowledge is a bit second hand.
> > >
> > > See here:
> > > https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/hikey960-mainline-WIP&id=754a79facf1af0b59a7f8fd63050da12ebf5521e
> > >
> > > Let me know if you have suggestions for more specific changes!
> >
> > Description looks good to me. But looks like you are not protecting the
> > other clk APIs like clk_prepare_enable and clk_disable_unprepare. Don't
> > they fail when the relevant clk is not found?
> 
> No, those calls just return 0 if null clock is passed
> 
> int clk_prepare(struct clk *clk)
> {
>         if (!clk)
>                 return 0;
> ...

Ah, okay. Didn't look into the definition.

So with the change in description,

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

Thanks,
Mani

> 
> thanks
> -john

Patch

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)
+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,
+};
+
+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;
+
 	d->base = devm_ioremap_resource(&op->dev, iores);
 	if (IS_ERR(d->base))
 		return PTR_ERR(d->base);
@@ -835,10 +860,12 @@  static int k3_dma_probe(struct platform_device *op)
 				"dma-requests", &d->dma_requests);
 	}
 
-	d->clk = devm_clk_get(&op->dev, NULL);
-	if (IS_ERR(d->clk)) {
-		dev_err(&op->dev, "no dma clk\n");
-		return PTR_ERR(d->clk);
+	if (!(soc_data->flags & K3_FLAG_NOCLK)) {
+		d->clk = devm_clk_get(&op->dev, NULL);
+		if (IS_ERR(d->clk)) {
+			dev_err(&op->dev, "no dma clk\n");
+			return PTR_ERR(d->clk);
+		}
 	}
 
 	irq = platform_get_irq(op, 0);