diff mbox

[v2,4/6] ASoC: Intel: use dw_dmac autoconfiguration

Message ID 1443455828-144960-5-git-send-email-andriy.shevchenko@linux.intel.com (mailing list archive)
State Not Applicable
Headers show

Commit Message

Andy Shevchenko Sept. 28, 2015, 3:57 p.m. UTC
Instead of hardconding a platform data for dw_dmac let's use it's own
autoconfiguration feature. Thus, remove hardcoded values.

Acked-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 sound/soc/intel/common/sst-firmware.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

Comments

Vinod Koul Sept. 30, 2015, 8:10 a.m. UTC | #1
On Mon, Sep 28, 2015 at 06:57:06PM +0300, Andy Shevchenko wrote:
> Instead of hardconding a platform data for dw_dmac let's use it's own
> autoconfiguration feature. Thus, remove hardcoded values.
> 

This need ack from Mark if we carry thru dmaengine tree. You should have
cced him and ALSA

> Acked-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  sound/soc/intel/common/sst-firmware.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/sound/soc/intel/common/sst-firmware.c b/sound/soc/intel/common/sst-firmware.c
> index ebcca6d..1636a1e 100644
> --- a/sound/soc/intel/common/sst-firmware.c
> +++ b/sound/soc/intel/common/sst-firmware.c
> @@ -26,7 +26,6 @@
>  #include <linux/acpi.h>
>  
>  /* supported DMA engine drivers */
> -#include <linux/platform_data/dma-dw.h>
>  #include <linux/dma/dw.h>
>  
>  #include <asm/page.h>
> @@ -169,12 +168,6 @@ err:
>  	return ret;
>  }
>  
> -static struct dw_dma_platform_data dw_pdata = {
> -	.is_private = 1,
> -	.chan_allocation_order = CHAN_ALLOCATION_ASCENDING,
> -	.chan_priority = CHAN_PRIORITY_ASCENDING,
> -};
> -
>  static struct dw_dma_chip *dw_probe(struct device *dev, struct resource *mem,
>  	int irq)
>  {
> @@ -195,7 +188,8 @@ static struct dw_dma_chip *dw_probe(struct device *dev, struct resource *mem,
>  		return ERR_PTR(err);
>  
>  	chip->dev = dev;
> -	err = dw_dma_probe(chip, &dw_pdata);
> +
> +	err = dw_dma_probe(chip, NULL);
>  	if (err)
>  		return ERR_PTR(err);
>  
> -- 
> 2.5.1
>
Mark Brown Oct. 2, 2015, 5 p.m. UTC | #2
On Wed, Sep 30, 2015 at 01:40:49PM +0530, Vinod Koul wrote:
> On Mon, Sep 28, 2015 at 06:57:06PM +0300, Andy Shevchenko wrote:
> > Instead of hardconding a platform data for dw_dmac let's use it's own
> > autoconfiguration feature. Thus, remove hardcoded values.

> This need ack from Mark if we carry thru dmaengine tree. You should have
> cced him and ALSA

Is this part of completely removing the platform data or is it just an
incremental change?  What are the dependencies here - I've not seen the
cover letter or the rest of the series?

> > Acked-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  sound/soc/intel/common/sst-firmware.c | 10 ++--------
> >  1 file changed, 2 insertions(+), 8 deletions(-)
> > 
> > diff --git a/sound/soc/intel/common/sst-firmware.c b/sound/soc/intel/common/sst-firmware.c
> > index ebcca6d..1636a1e 100644
> > --- a/sound/soc/intel/common/sst-firmware.c
> > +++ b/sound/soc/intel/common/sst-firmware.c
> > @@ -26,7 +26,6 @@
> >  #include <linux/acpi.h>
> >  
> >  /* supported DMA engine drivers */
> > -#include <linux/platform_data/dma-dw.h>
> >  #include <linux/dma/dw.h>
> >  
> >  #include <asm/page.h>
> > @@ -169,12 +168,6 @@ err:
> >  	return ret;
> >  }
> >  
> > -static struct dw_dma_platform_data dw_pdata = {
> > -	.is_private = 1,
> > -	.chan_allocation_order = CHAN_ALLOCATION_ASCENDING,
> > -	.chan_priority = CHAN_PRIORITY_ASCENDING,
> > -};
> > -
> >  static struct dw_dma_chip *dw_probe(struct device *dev, struct resource *mem,
> >  	int irq)
> >  {
> > @@ -195,7 +188,8 @@ static struct dw_dma_chip *dw_probe(struct device *dev, struct resource *mem,
> >  		return ERR_PTR(err);
> >  
> >  	chip->dev = dev;
> > -	err = dw_dma_probe(chip, &dw_pdata);
> > +
> > +	err = dw_dma_probe(chip, NULL);
> >  	if (err)
> >  		return ERR_PTR(err);
> >  
> > -- 
> > 2.5.1
> > 
> 
> -- 
> ~Vinod
>
Andy Shevchenko Oct. 5, 2015, 11:12 a.m. UTC | #3
On Fri, 2015-10-02 at 18:00 +0100, Mark Brown wrote:
> On Wed, Sep 30, 2015 at 01:40:49PM +0530, Vinod Koul wrote:
> > On Mon, Sep 28, 2015 at 06:57:06PM +0300, Andy Shevchenko wrote:
> > > Instead of hardconding a platform data for dw_dmac let's use it's 
> > > own
> > > autoconfiguration feature. Thus, remove hardcoded values.
> 
> > This need ack from Mark if we carry thru dmaengine tree. You should 
> > have
> > cced him and ALSA
> 
> Is this part of completely removing the platform data or is it just 
> an
> incremental change?  What are the dependencies here - I've not seen 
> the
> cover letter or the rest of the series?

Since platform data is a partial here and driver relies on DMA
autoconfiguration mode, therefore it's redundant. But it will not work
until we properly do autoconfiguration in DMA driver (see [1]).

The cover letter is [2].

I'm going to resend an updated series soon.

[1] http://www.spinics.net/lists/dmaengine/msg06284.html
[2] http://www.spinics.net/lists/dmaengine/msg06288.html

> 

> > > Acked-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com
> > > >
> > > ---
> > >  sound/soc/intel/common/sst-firmware.c | 10 ++--------
> > >  1 file changed, 2 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/sound/soc/intel/common/sst-firmware.c 
> > > b/sound/soc/intel/common/sst-firmware.c
> > > index ebcca6d..1636a1e 100644
> > > --- a/sound/soc/intel/common/sst-firmware.c
> > > +++ b/sound/soc/intel/common/sst-firmware.c
> > > @@ -26,7 +26,6 @@
> > >  #include <linux/acpi.h>
> > >  
> > >  /* supported DMA engine drivers */
> > > -#include <linux/platform_data/dma-dw.h>
> > >  #include <linux/dma/dw.h>
> > >  
> > >  #include <asm/page.h>
> > > @@ -169,12 +168,6 @@ err:
> > >  	return ret;
> > >  }
> > >  
> > > -static struct dw_dma_platform_data dw_pdata = {
> > > -	.is_private = 1,
> > > -	.chan_allocation_order = CHAN_ALLOCATION_ASCENDING,
> > > -	.chan_priority = CHAN_PRIORITY_ASCENDING,
> > > -};
> > > -
> > >  static struct dw_dma_chip *dw_probe(struct device *dev, struct 
> > > resource *mem,
> > >  	int irq)
> > >  {
> > > @@ -195,7 +188,8 @@ static struct dw_dma_chip *dw_probe(struct 
> > > device *dev, struct resource *mem,
> > >  		return ERR_PTR(err);
> > >  
> > >  	chip->dev = dev;
> > > -	err = dw_dma_probe(chip, &dw_pdata);
> > > +
> > > +	err = dw_dma_probe(chip, NULL);
> > >  	if (err)
> > >  		return ERR_PTR(err);
> > >  
> > > -- 
> > > 2.5.1
> > > 
> >
Mark Brown Oct. 5, 2015, 1:57 p.m. UTC | #4
On Mon, Oct 05, 2015 at 02:12:58PM +0300, Andy Shevchenko wrote:
> On Fri, 2015-10-02 at 18:00 +0100, Mark Brown wrote:

> > Is this part of completely removing the platform data or is it just 
> > an
> > incremental change?  What are the dependencies here - I've not seen 
> > the
> > cover letter or the rest of the series?

> Since platform data is a partial here and driver relies on DMA
> autoconfiguration mode, therefore it's redundant. But it will not work
> until we properly do autoconfiguration in DMA driver (see [1]).

OK, so I may as well just apply this patch since it doesn't work anyway?
Andy Shevchenko Oct. 6, 2015, 3:50 p.m. UTC | #5
On Mon, 2015-10-05 at 14:57 +0100, Mark Brown wrote:
> On Mon, Oct 05, 2015 at 02:12:58PM +0300, Andy Shevchenko wrote:
> > On Fri, 2015-10-02 at 18:00 +0100, Mark Brown wrote:
> 
> > > Is this part of completely removing the platform data or is it 
> > > just 
> > > an
> > > incremental change?  What are the dependencies here - I've not 
> > > seen 
> > > the
> > > cover letter or the rest of the series?
> 
> > Since platform data is a partial here and driver relies on DMA
> > autoconfiguration mode, therefore it's redundant. But it will not 
> > work
> > until we properly do autoconfiguration in DMA driver (see [1]).
> 
> OK, so I may as well just apply this patch since it doesn't work > anyway?

Yes, you may do that.
diff mbox

Patch

diff --git a/sound/soc/intel/common/sst-firmware.c b/sound/soc/intel/common/sst-firmware.c
index ebcca6d..1636a1e 100644
--- a/sound/soc/intel/common/sst-firmware.c
+++ b/sound/soc/intel/common/sst-firmware.c
@@ -26,7 +26,6 @@ 
 #include <linux/acpi.h>
 
 /* supported DMA engine drivers */
-#include <linux/platform_data/dma-dw.h>
 #include <linux/dma/dw.h>
 
 #include <asm/page.h>
@@ -169,12 +168,6 @@  err:
 	return ret;
 }
 
-static struct dw_dma_platform_data dw_pdata = {
-	.is_private = 1,
-	.chan_allocation_order = CHAN_ALLOCATION_ASCENDING,
-	.chan_priority = CHAN_PRIORITY_ASCENDING,
-};
-
 static struct dw_dma_chip *dw_probe(struct device *dev, struct resource *mem,
 	int irq)
 {
@@ -195,7 +188,8 @@  static struct dw_dma_chip *dw_probe(struct device *dev, struct resource *mem,
 		return ERR_PTR(err);
 
 	chip->dev = dev;
-	err = dw_dma_probe(chip, &dw_pdata);
+
+	err = dw_dma_probe(chip, NULL);
 	if (err)
 		return ERR_PTR(err);