[PATCH 00/11] enable imx6q_sabrelite sgtl5000 audio support

[PATCH 00/11] enable imx6q_sabrelite sgtl5000 audio support

[Richard Zhao]

It's based on Dong Aisheng's early pinctrl patch and Sascha's v2
convert to common clk patch.

You can also get at:
https://github.com/riczhao/kernel-imx/tree/topics/audio

Richard Zhao (11):
  dma: imx-sdma: make channel0 operations atomic
  ASoC: imx-sgtl5000: add of_node_put when probe fail.
  ASoC: fsl: add sgtl5000 clock support for imx-sgtl5000
  i2c: imx: add pinctrl support
  ARM: imx6q: move imx6q_sabrelite specific code to a dedicated
    function
  ARM: dts: imx6q-sabrelite: add ssi device
  ARM: dts: imx6q-sabrelite: add audmux device
  ASoC: imx-audmux: add pinctrl support
  ARM: imx6q: add ssi1 clk_lookup
  ARM: imx6q_sabrelite: clkdev_add cko1 for sgtl5000
  ARM: dts: imx6q-sabrelite: add sound device imx6q-sabrelite-sgtl5000

 arch/arm/boot/dts/imx6q-sabrelite.dts |   30 ++++++++++++++++
 arch/arm/boot/dts/imx6q.dtsi          |   46 +++++++++++++++++++++++--
 arch/arm/mach-imx/clk-imx6q.c         |    4 ++
 arch/arm/mach-imx/mach-imx6q.c        |   37 +++++++++++++++++++-
 drivers/dma/imx-sdma.c                |   57 ++++++++++++++++++-------------
 drivers/i2c/busses/i2c-imx.c          |    9 +++++
 sound/soc/fsl/imx-audmux.c            |    8 ++++
 sound/soc/fsl/imx-sgtl5000.c          |   61 +++++++++++++++++++++++++--------
 8 files changed, 208 insertions(+), 44 deletions(-)

Thanks
Richard
-- 
1.7.5.4

[PATCH 01/11] dma: imx-sdma: make channel0 operations atomic

[Richard Zhao]

device_prep_dma_cyclic may be call in audio trigger function which is
atomic context, so we make it atomic too.

 - change channel0 lock to spinlock.
 - Use polling to wait for channel0 finish running.

Signed-off-by: Lothar Waßmann <LW-bxm8fMRDkQLDiMYJYoSAnRvVK+yQ3ZXh@public.gmane.org>
Signed-off-by: Richard Zhao <richard.zhao-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 drivers/dma/imx-sdma.c |   57 +++++++++++++++++++++++++++--------------------
 1 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index fddccae..fc49ffa 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -24,7 +24,7 @@
 #include <linux/mm.h>
 #include <linux/interrupt.h>
 #include <linux/clk.h>
-#include <linux/wait.h>
+#include <linux/delay.h>
 #include <linux/sched.h>
 #include <linux/semaphore.h>
 #include <linux/spinlock.h>
@@ -324,7 +324,7 @@ struct sdma_engine {
 	struct dma_device		dma_device;
 	struct clk			*clk_ipg;
 	struct clk			*clk_ahb;
-	struct mutex			channel_0_lock;
+	spinlock_t		channel_0_lock;
 	struct sdma_script_start_addrs	*script_addrs;
 };
 
@@ -402,19 +402,31 @@ static void sdma_enable_channel(struct sdma_engine *sdma, int channel)
 }
 
 /*
- * sdma_run_channel - run a channel and wait till it's done
+ * sdma_run_channel0 - run a channel and wait till it's done
  */
-static int sdma_run_channel(struct sdma_channel *sdmac)
+static int sdma_run_channel0(struct sdma_channel *sdmac)
 {
 	struct sdma_engine *sdma = sdmac->sdma;
 	int channel = sdmac->channel;
 	int ret;
+	unsigned long timeout = 500;
 
-	init_completion(&sdmac->done);
-
+	if (channel)
+		return -EINVAL;
 	sdma_enable_channel(sdma, channel);
 
-	ret = wait_for_completion_timeout(&sdmac->done, HZ);
+	while (!(ret = readl_relaxed(sdma->regs + SDMA_H_INTR) & 1)) {
+		if (timeout-- <= 0)
+			break;
+		udelay(1);
+	}
+
+	if (ret) {
+		/* Clear the interrupt status */
+		writel_relaxed(ret, sdma->regs + SDMA_H_INTR);
+	} else {
+		dev_err(sdma->dev, "Timeout waiting for CH0 ready\n");
+	}
 
 	return ret ? 0 : -ETIMEDOUT;
 }
@@ -426,17 +438,17 @@ static int sdma_load_script(struct sdma_engine *sdma, void *buf, int size,
 	void *buf_virt;
 	dma_addr_t buf_phys;
 	int ret;
-
-	mutex_lock(&sdma->channel_0_lock);
+	unsigned long flags;
 
 	buf_virt = dma_alloc_coherent(NULL,
 			size,
 			&buf_phys, GFP_KERNEL);
 	if (!buf_virt) {
-		ret = -ENOMEM;
-		goto err_out;
+		return -ENOMEM;
 	}
 
+	spin_lock_irqsave(&sdma->channel_0_lock, flags);
+
 	bd0->mode.command = C0_SETPM;
 	bd0->mode.status = BD_DONE | BD_INTR | BD_WRAP | BD_EXTD;
 	bd0->mode.count = size / 2;
@@ -445,12 +457,11 @@ static int sdma_load_script(struct sdma_engine *sdma, void *buf, int size,
 
 	memcpy(buf_virt, buf, size);
 
-	ret = sdma_run_channel(&sdma->channel[0]);
+	ret = sdma_run_channel0(&sdma->channel[0]);
 
-	dma_free_coherent(NULL, size, buf_virt, buf_phys);
+	spin_unlock_irqrestore(&sdma->channel_0_lock, flags);
 
-err_out:
-	mutex_unlock(&sdma->channel_0_lock);
+	dma_free_coherent(NULL, size, buf_virt, buf_phys);
 
 	return ret;
 }
@@ -539,10 +550,6 @@ static void mxc_sdma_handle_channel(struct sdma_channel *sdmac)
 {
 	complete(&sdmac->done);
 
-	/* not interested in channel 0 interrupts */
-	if (sdmac->channel == 0)
-		return;
-
 	if (sdmac->flags & IMX_DMA_SG_LOOP)
 		sdma_handle_channel_loop(sdmac);
 	else
@@ -555,6 +562,8 @@ static irqreturn_t sdma_int_handler(int irq, void *dev_id)
 	unsigned long stat;
 
 	stat = readl_relaxed(sdma->regs + SDMA_H_INTR);
+	/* not interested in channel 0 interrupts */
+	stat &= ~1;
 	writel_relaxed(stat, sdma->regs + SDMA_H_INTR);
 
 	while (stat) {
@@ -660,6 +669,7 @@ static int sdma_load_context(struct sdma_channel *sdmac)
 	struct sdma_context_data *context = sdma->context;
 	struct sdma_buffer_descriptor *bd0 = sdma->channel[0].bd;
 	int ret;
+	unsigned long flags;
 
 	if (sdmac->direction == DMA_DEV_TO_MEM) {
 		load_address = sdmac->pc_from_device;
@@ -677,7 +687,7 @@ static int sdma_load_context(struct sdma_channel *sdmac)
 	dev_dbg(sdma->dev, "event_mask0 = 0x%08x\n", (u32)sdmac->event_mask[0]);
 	dev_dbg(sdma->dev, "event_mask1 = 0x%08x\n", (u32)sdmac->event_mask[1]);
 
-	mutex_lock(&sdma->channel_0_lock);
+	spin_lock_irqsave(&sdma->channel_0_lock, flags);
 
 	memset(context, 0, sizeof(*context));
 	context->channel_state.pc = load_address;
@@ -696,10 +706,9 @@ static int sdma_load_context(struct sdma_channel *sdmac)
 	bd0->mode.count = sizeof(*context) / 4;
 	bd0->buffer_addr = sdma->context_phys;
 	bd0->ext_buffer_addr = 2048 + (sizeof(*context) / 4) * channel;
+	ret = sdma_run_channel0(&sdma->channel[0]);
 
-	ret = sdma_run_channel(&sdma->channel[0]);
-
-	mutex_unlock(&sdma->channel_0_lock);
+	spin_unlock_irqrestore(&sdma->channel_0_lock, flags);
 
 	return ret;
 }
@@ -1305,7 +1314,7 @@ static int __init sdma_probe(struct platform_device *pdev)
 	if (!sdma)
 		return -ENOMEM;
 
-	mutex_init(&sdma->channel_0_lock);
+	spin_lock_init(&sdma->channel_0_lock);
 
 	sdma->dev = &pdev->dev;
 
-- 
1.7.5.4

Re: [PATCH 01/11] dma: imx-sdma: make channel0 operations atomic

[Sascha Hauer]

On Fri, Apr 27, 2012 at 03:02:55PM +0800, Richard Zhao wrote:
> device_prep_dma_cyclic may be call in audio trigger function which is
> atomic context, so we make it atomic too.
> 
>  - change channel0 lock to spinlock.
>  - Use polling to wait for channel0 finish running.
> 
> Signed-off-by: Lothar Waßmann <LW-bxm8fMRDkQLDiMYJYoSAnRvVK+yQ3ZXh@public.gmane.org>
> Signed-off-by: Richard Zhao <richard.zhao-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> ---
>  drivers/dma/imx-sdma.c |   57 +++++++++++++++++++++++++++--------------------
>  1 files changed, 33 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index fddccae..fc49ffa 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -24,7 +24,7 @@
>  #include <linux/mm.h>
>  #include <linux/interrupt.h>
>  #include <linux/clk.h>
> -#include <linux/wait.h>
> +#include <linux/delay.h>
>  #include <linux/sched.h>
>  #include <linux/semaphore.h>
>  #include <linux/spinlock.h>
> @@ -324,7 +324,7 @@ struct sdma_engine {
>  	struct dma_device		dma_device;
>  	struct clk			*clk_ipg;
>  	struct clk			*clk_ahb;
> -	struct mutex			channel_0_lock;
> +	spinlock_t		channel_0_lock;
>  	struct sdma_script_start_addrs	*script_addrs;
>  };
>  
> @@ -402,19 +402,31 @@ static void sdma_enable_channel(struct sdma_engine *sdma, int channel)
>  }
>  
>  /*
> - * sdma_run_channel - run a channel and wait till it's done
> + * sdma_run_channel0 - run a channel and wait till it's done
>   */
> -static int sdma_run_channel(struct sdma_channel *sdmac)
> +static int sdma_run_channel0(struct sdma_channel *sdmac)

Renaming this to sdma_run_channel0 is fine, but then the argument should
be changed to struct sdma_engine. It makes no sense to say in the
function name that this function is channel 0 only and at the same time
allow to pass in an arbitrary other channel.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 01/11] dma: imx-sdma: make channel0 operations atomic

[Richard Zhao]

On Fri, Apr 27, 2012 at 09:55:44AM +0200, Sascha Hauer wrote:
> On Fri, Apr 27, 2012 at 03:02:55PM +0800, Richard Zhao wrote:
> > device_prep_dma_cyclic may be call in audio trigger function which is
> > atomic context, so we make it atomic too.
> > 
> >  - change channel0 lock to spinlock.
> >  - Use polling to wait for channel0 finish running.
> > 
> > Signed-off-by: Lothar Waßmann <LW-bxm8fMRDkQLDiMYJYoSAnRvVK+yQ3ZXh@public.gmane.org>
> > Signed-off-by: Richard Zhao <richard.zhao-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > ---
> >  drivers/dma/imx-sdma.c |   57 +++++++++++++++++++++++++++--------------------
> >  1 files changed, 33 insertions(+), 24 deletions(-)
> > 
> > diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> > index fddccae..fc49ffa 100644
> > --- a/drivers/dma/imx-sdma.c
> > +++ b/drivers/dma/imx-sdma.c
> > @@ -24,7 +24,7 @@
> >  #include <linux/mm.h>
> >  #include <linux/interrupt.h>
> >  #include <linux/clk.h>
> > -#include <linux/wait.h>
> > +#include <linux/delay.h>
> >  #include <linux/sched.h>
> >  #include <linux/semaphore.h>
> >  #include <linux/spinlock.h>
> > @@ -324,7 +324,7 @@ struct sdma_engine {
> >  	struct dma_device		dma_device;
> >  	struct clk			*clk_ipg;
> >  	struct clk			*clk_ahb;
> > -	struct mutex			channel_0_lock;
> > +	spinlock_t		channel_0_lock;
> >  	struct sdma_script_start_addrs	*script_addrs;
> >  };
> >  
> > @@ -402,19 +402,31 @@ static void sdma_enable_channel(struct sdma_engine *sdma, int channel)
> >  }
> >  
> >  /*
> > - * sdma_run_channel - run a channel and wait till it's done
> > + * sdma_run_channel0 - run a channel and wait till it's done
> >   */
> > -static int sdma_run_channel(struct sdma_channel *sdmac)
> > +static int sdma_run_channel0(struct sdma_channel *sdmac)
> 
> Renaming this to sdma_run_channel0 is fine, but then the argument should
> be changed to struct sdma_engine. It makes no sense to say in the
> function name that this function is channel 0 only and at the same time
> allow to pass in an arbitrary other channel.
Correct. Thanks.

Richard
> 
> Sascha
> 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 

Re: [PATCH 01/11] dma: imx-sdma: make channel0 operations atomic

[Vinod Koul]

On Fri, 2012-04-27 at 15:02 +0800, Richard Zhao wrote:
> device_prep_dma_cyclic may be call in audio trigger function which is
> atomic context, so we make it atomic too.
No this is wrong behavior. You should not call dma prepare functions in
any of the sound trigger calls. It would make sense to move this in
sound prepare callback.

-- 
~Vinod

Re: [PATCH 01/11] dma: imx-sdma: make channel0 operations atomic

[Richard Zhao]

On Fri, Apr 27, 2012 at 01:51:40PM +0530, Vinod Koul wrote:
> On Fri, 2012-04-27 at 15:02 +0800, Richard Zhao wrote:
> > device_prep_dma_cyclic may be call in audio trigger function which is
> > atomic context, so we make it atomic too.
> No this is wrong behavior. You should not call dma prepare functions in
> any of the sound trigger calls. It would make sense to move this in
> sound prepare callback.
Then, could you please doc it somewhere? I think I'm not the only one
confused.

Thanks
Richard
> 
> -- 
> ~Vinod
> 
> 

Re: [PATCH 01/11] dma: imx-sdma: make channel0 operations atomic

[Vinod Koul]

On Fri, 2012-04-27 at 16:41 +0800, Richard Zhao wrote:
> On Fri, Apr 27, 2012 at 01:51:40PM +0530, Vinod Koul wrote:
> > On Fri, 2012-04-27 at 15:02 +0800, Richard Zhao wrote:
> > > device_prep_dma_cyclic may be call in audio trigger function which is
> > > atomic context, so we make it atomic too.
> > No this is wrong behavior. You should not call dma prepare functions in
> > any of the sound trigger calls. It would make sense to move this in
> > sound prepare callback.
> Then, could you please doc it somewhere? I think I'm not the only one
> confused.
See the soc-dmaengine.c for correct behavior!
I can document only dmaengine behavior (which is already there) and not
how each subsystem should use this. People is subsystems need to see how
to use the dmaengine APIs sanely


-- 
~Vinod

Re: [PATCH 01/11] dma: imx-sdma: make channel0 operations atomic

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 326 bytes --]

On Fri, Apr 27, 2012 at 03:52:10PM +0530, Vinod Koul wrote:
> On Fri, 2012-04-27 at 16:41 +0800, Richard Zhao wrote:

> > Then, could you please doc it somewhere? I think I'm not the only one
> > confused.

> See the soc-dmaengine.c for correct behavior!

Better yet, convert your code to use it if it's not doing so already!

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 01/11] dma: imx-sdma: make channel0 operations atomic

[Richard Zhao]

On Fri, Apr 27, 2012 at 03:52:10PM +0530, Vinod Koul wrote:
> On Fri, 2012-04-27 at 16:41 +0800, Richard Zhao wrote:
> > On Fri, Apr 27, 2012 at 01:51:40PM +0530, Vinod Koul wrote:
> > > On Fri, 2012-04-27 at 15:02 +0800, Richard Zhao wrote:
> > > > device_prep_dma_cyclic may be call in audio trigger function which is
> > > > atomic context, so we make it atomic too.
> > > No this is wrong behavior. You should not call dma prepare functions in
> > > any of the sound trigger calls. It would make sense to move this in
> > > sound prepare callback.
> > Then, could you please doc it somewhere? I think I'm not the only one
> > confused.
> See the soc-dmaengine.c for correct behavior!
Do you mean sound/soc/soc-dmaengine-pcm.c ? It's where it calls dma
prepare in trigger function.
snd_dmaengine_pcm_trigger --> dmaengine_pcm_prepare_and_submit -->
dmaengine_prep_dma_cyclic

> I can document only dmaengine behavior (which is already there)
Sure, I mean, can you doc in include/linux/dmaengine.h that
dmaengine_prep_xxx may sleep?

> and not
> how each subsystem should use this. People is subsystems need to see how
> to use the dmaengine APIs sanely

Thanks
Richard
> 
> 
> -- 
> ~Vinod
> 
> 

Re: [PATCH 01/11] dma: imx-sdma: make channel0 operations atomic

[Russell King - ARM Linux]

On Fri, Apr 27, 2012 at 07:26:56PM +0800, Richard Zhao wrote:
> Sure, I mean, can you doc in include/linux/dmaengine.h that
> dmaengine_prep_xxx may sleep?

Incorrect.  They may _not_ sleep.

Re: [PATCH 01/11] dma: imx-sdma: make channel0 operations atomic

[Laxman Dewangan]

On Friday 27 April 2012 05:03 PM, Russell King - ARM Linux wrote:
> On Fri, Apr 27, 2012 at 07:26:56PM +0800, Richard Zhao wrote:
>> Sure, I mean, can you doc in include/linux/dmaengine.h that
>> dmaengine_prep_xxx may sleep?
> Incorrect.  They may _not_ sleep.

But I have seen that we are using the kzalloc in the dmaengine_prep_xxx 
and kzalloc is sleeping call.

Re: [PATCH 01/11] dma: imx-sdma: make channel0 operations atomic

[Russell King - ARM Linux]

On Fri, Apr 27, 2012 at 06:31:13PM +0530, Laxman Dewangan wrote:
> On Friday 27 April 2012 05:03 PM, Russell King - ARM Linux wrote:
>> On Fri, Apr 27, 2012 at 07:26:56PM +0800, Richard Zhao wrote:
>>> Sure, I mean, can you doc in include/linux/dmaengine.h that
>>> dmaengine_prep_xxx may sleep?
>> Incorrect.  They may _not_ sleep.
>
> But I have seen that we are using the kzalloc in the dmaengine_prep_xxx  
> and kzalloc is sleeping call.

Not with GFP_ATOMIC it isn't.

Re: [PATCH 01/11] dma: imx-sdma: make channel0 operations atomic

[Laxman Dewangan]

On Friday 27 April 2012 06:40 PM, Russell King - ARM Linux wrote:
> On Fri, Apr 27, 2012 at 06:31:13PM +0530, Laxman Dewangan wrote:
>> On Friday 27 April 2012 05:03 PM, Russell King - ARM Linux wrote:
>>> On Fri, Apr 27, 2012 at 07:26:56PM +0800, Richard Zhao wrote:
>>>> Sure, I mean, can you doc in include/linux/dmaengine.h that
>>>> dmaengine_prep_xxx may sleep?
>>> Incorrect.  They may _not_ sleep.
>> But I have seen that we are using the kzalloc in the dmaengine_prep_xxx
>> and kzalloc is sleeping call.
> Not with GFP_ATOMIC it isn't.

Yes, with GFP_ATOMIC , it will not be a sleeping calls.
I will fix my dma driver accordingly.

Re: [PATCH 01/11] dma: imx-sdma: make channel0 operations atomic

[Richard Zhao]

On Fri, Apr 27, 2012 at 02:10:10PM +0100, Russell King - ARM Linux wrote:
> On Fri, Apr 27, 2012 at 06:31:13PM +0530, Laxman Dewangan wrote:
> > On Friday 27 April 2012 05:03 PM, Russell King - ARM Linux wrote:
> >> On Fri, Apr 27, 2012 at 07:26:56PM +0800, Richard Zhao wrote:
> >>> Sure, I mean, can you doc in include/linux/dmaengine.h that
> >>> dmaengine_prep_xxx may sleep?
> >> Incorrect.  They may _not_ sleep.
Vinod, do you agree? Any way, better doc it.

Thanks
Richard
> >
> > But I have seen that we are using the kzalloc in the dmaengine_prep_xxx  
> > and kzalloc is sleeping call.
> 
> Not with GFP_ATOMIC it isn't.
> 

Re: [PATCH 01/11] dma: imx-sdma: make channel0 operations atomic

[Lothar Waßmann]

Hi,

Richard Zhao writes:
> device_prep_dma_cyclic may be call in audio trigger function which is
> atomic context, so we make it atomic too.
> 
>  - change channel0 lock to spinlock.
>  - Use polling to wait for channel0 finish running.
> 
> Signed-off-by: Lothar Waßmann <LW-bxm8fMRDkQLDiMYJYoSAnRvVK+yQ3ZXh@public.gmane.org>
>
Actually I didn't sign off the patch that I posted, because I wanted
to wait for more comments first.


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info-AvR2QvxeiV7DiMYJYoSAnRvVK+yQ3ZXh@public.gmane.org
___________________________________________________________

Re: [PATCH 01/11] dma: imx-sdma: make channel0 operations atomic

[Richard Zhao]

On Fri, Apr 27, 2012 at 11:18:31AM +0200, Lothar Waßmann wrote:
> Hi,
> 
> Richard Zhao writes:
> > device_prep_dma_cyclic may be call in audio trigger function which is
> > atomic context, so we make it atomic too.
> > 
> >  - change channel0 lock to spinlock.
> >  - Use polling to wait for channel0 finish running.
> > 
> > Signed-off-by: Lothar Waßmann <LW-bxm8fMRDkQLDiMYJYoSAnRvVK+yQ3ZXh@public.gmane.org>
> >
> Actually I didn't sign off the patch that I posted, because I wanted
> to wait for more comments first.
I send it out with slight modifications because the series highly
depend on it. Will you take it over or let me put it in next version?
Both are ok to me.

Thanks
Richard
> 
> 
> Lothar Waßmann
> -- 
> ___________________________________________________________
> 
> Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
> Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
> Geschäftsführer: Matthias Kaussen
> Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
> 
> www.karo-electronics.de | info-AvR2QvxeiV7DiMYJYoSAnRvVK+yQ3ZXh@public.gmane.org
> ___________________________________________________________
> 

Re: [PATCH 01/11] dma: imx-sdma: make channel0 operations atomic

[Lothar Waßmann]

Hi,

Richard Zhao writes:
> On Fri, Apr 27, 2012 at 11:18:31AM +0200, Lothar Waßmann wrote:
> > Hi,
> > 
> > Richard Zhao writes:
> > > device_prep_dma_cyclic may be call in audio trigger function which is
> > > atomic context, so we make it atomic too.
> > > 
> > >  - change channel0 lock to spinlock.
> > >  - Use polling to wait for channel0 finish running.
> > > 
> > > Signed-off-by: Lothar Waßmann <LW-bxm8fMRDkQLDiMYJYoSAnRvVK+yQ3ZXh@public.gmane.org>
> > >
> > Actually I didn't sign off the patch that I posted, because I wanted
> > to wait for more comments first.
> I send it out with slight modifications because the series highly
> depend on it. Will you take it over or let me put it in next version?
> Both are ok to me.
> 
I think you should keep it as part of your sound patches and I will
test your final version on our hardware.


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info-AvR2QvxeiV7DiMYJYoSAnRvVK+yQ3ZXh@public.gmane.org
___________________________________________________________

Re: [PATCH 01/11] dma: imx-sdma: make channel0 operations atomic

[Huang Shijie]

On Fri, Apr 27, 2012 at 11:13 AM, Lothar Waßmann <LW@karo-electronics.de> wrote:
> Hi,
>
> Richard Zhao writes:
>> On Fri, Apr 27, 2012 at 11:18:31AM +0200, Lothar Waßmann wrote:
>> > Hi,
>> >
>> > Richard Zhao writes:
>> > > device_prep_dma_cyclic may be call in audio trigger function which is
>> > > atomic context, so we make it atomic too.
>> > >
>> > >  - change channel0 lock to spinlock.
>> > >  - Use polling to wait for channel0 finish running.
>> > >
>> > > Signed-off-by: Lothar Waßmann <LW-bxm8fMRDkQLDiMYJYoSAnRvVK+yQ3ZXh@public.gmane.org>
>> > >
>> > Actually I didn't sign off the patch that I posted, because I wanted
>> > to wait for more comments first.
>> I send it out with slight modifications because the series highly
>> depend on it. Will you take it over or let me put it in next version?
>> Both are ok to me.
>>
> I think you should keep it as part of your sound patches and I will
> test your final version on our hardware.
>
I hope we can get a conclusion that the prep_slave_sg() can be called
in atomic context or not.
My patch "add DMA support to UART" heavily depends on it.

Huang Shijie

>
> Lothar Waßmann
> --
> ___________________________________________________________
>
> Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
> Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
> Geschäftsführer: Matthias Kaussen
> Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
>
> www.karo-electronics.de | info-AvR2QvxeiV7DiMYJYoSAnRvVK+yQ3ZXh@public.gmane.org
> ___________________________________________________________
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 02/11] ASoC: imx-sgtl5000: add of_node_put when probe fail.

[Richard Zhao]

Signed-off-by: Richard Zhao <richard.zhao-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 sound/soc/fsl/imx-sgtl5000.c |   29 ++++++++++++++++++-----------
 1 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/sound/soc/fsl/imx-sgtl5000.c b/sound/soc/fsl/imx-sgtl5000.c
index e1a7441..73b935e 100644
--- a/sound/soc/fsl/imx-sgtl5000.c
+++ b/sound/soc/fsl/imx-sgtl5000.c
@@ -103,24 +103,28 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev)
 	codec_np = of_parse_phandle(pdev->dev.of_node, "audio-codec", 0);
 	if (!ssi_np || !codec_np) {
 		dev_err(&pdev->dev, "phandle missing or invalid\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto fail;
 	}
 
 	ssi_pdev = of_find_device_by_node(ssi_np);
 	if (!ssi_pdev) {
 		dev_err(&pdev->dev, "failed to find SSI platform device\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto fail;
 	}
 
 	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
+	if (!data) {
+		ret = -ENOMEM;
+		goto fail;
+	}
 
 	ret = of_property_read_u32(codec_np, "clock-frequency",
 				   &data->clk_frequency);
 	if (ret) {
 		dev_err(&pdev->dev, "clock-frequency missing or invalid\n");
-		return ret;
+		goto fail;
 	}
 
 	data->dai.name = "HiFi";
@@ -136,10 +140,10 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev)
 	data->card.dev = &pdev->dev;
 	ret = snd_soc_of_parse_card_name(&data->card, "model");
 	if (ret)
-		return ret;
+		goto fail;
 	ret = snd_soc_of_parse_audio_routing(&data->card, "audio-routing");
 	if (ret)
-		return ret;
+		goto fail;
 	data->card.num_links = 1;
 	data->card.dai_link = &data->dai;
 	data->card.dapm_widgets = imx_sgtl5000_dapm_widgets;
@@ -148,14 +152,17 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev)
 	ret = snd_soc_register_card(&data->card);
 	if (ret) {
 		dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n", ret);
-		return ret;
+		goto fail;
 	}
 
 	platform_set_drvdata(pdev, data);
-	of_node_put(ssi_np);
-	of_node_put(codec_np);
+fail:
+	if (ssi_np)
+		of_node_put(ssi_np);
+	if (codec_np)
+		of_node_put(codec_np);
 
-	return 0;
+	return ret;
 }
 
 static int __devexit imx_sgtl5000_remove(struct platform_device *pdev)
-- 
1.7.5.4

Re: [PATCH 02/11] ASoC: imx-sgtl5000: add of_node_put when probe fail.

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 165 bytes --]

On Fri, Apr 27, 2012 at 03:02:56PM +0800, Richard Zhao wrote:
> Signed-off-by: Richard Zhao <richard.zhao-KZfg59tc24xl57MIdRCFDg@public.gmane.org>

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 03/11] ASoC: fsl: add sgtl5000 clock support for imx-sgtl5000

[Richard Zhao]

From: Richard Zhao <richard.zhao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

It tries to clk_get the clock. And if it failed, it assumes the clock
by default enabled.

Signed-off-by: Richard Zhao <richard.zhao-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 sound/soc/fsl/imx-sgtl5000.c |   40 ++++++++++++++++++++++++++++++++--------
 1 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/sound/soc/fsl/imx-sgtl5000.c b/sound/soc/fsl/imx-sgtl5000.c
index 73b935e..3a729ca 100644
--- a/sound/soc/fsl/imx-sgtl5000.c
+++ b/sound/soc/fsl/imx-sgtl5000.c
@@ -13,6 +13,8 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
+#include <linux/of_i2c.h>
+#include <linux/clk.h>
 #include <sound/soc.h>
 
 #include "../codecs/sgtl5000.h"
@@ -25,6 +27,7 @@ struct imx_sgtl5000_data {
 	struct snd_soc_card card;
 	char codec_dai_name[DAI_NAME_SIZE];
 	char platform_name[DAI_NAME_SIZE];
+	struct clk *codec_clk;
 	unsigned int clk_frequency;
 };
 
@@ -58,6 +61,7 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	struct device_node *ssi_np, *codec_np;
 	struct platform_device *ssi_pdev;
+	struct i2c_client *codec_dev;
 	struct imx_sgtl5000_data *data;
 	int int_port, ext_port;
 	int ret;
@@ -113,6 +117,11 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev)
 		ret = -EINVAL;
 		goto fail;
 	}
+	codec_dev = of_find_i2c_device_by_node(codec_np);
+	if (!codec_dev) {
+		dev_err(&pdev->dev, "failed to find codec platform device\n");
+		return -EINVAL;
+	}
 
 	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
 	if (!data) {
@@ -120,11 +129,20 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev)
 		goto fail;
 	}
 
-	ret = of_property_read_u32(codec_np, "clock-frequency",
-				   &data->clk_frequency);
-	if (ret) {
-		dev_err(&pdev->dev, "clock-frequency missing or invalid\n");
-		goto fail;
+	data->codec_clk = clk_get(&codec_dev->dev, NULL);
+	if (IS_ERR(data->codec_clk)) {
+		/* assuming clock enabled by default */
+		data->codec_clk = NULL;
+		ret = of_property_read_u32(codec_np, "clock-frequency",
+					&data->clk_frequency);
+		if (ret) {
+			dev_err(&codec_dev->dev,
+				"clock-frequency missing or invalid\n");
+			goto fail;
+		}
+	} else {
+		data->clk_frequency = clk_get_rate(data->codec_clk);
+		clk_prepare_enable(data->codec_clk);
 	}
 
 	data->dai.name = "HiFi";
@@ -140,10 +158,10 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev)
 	data->card.dev = &pdev->dev;
 	ret = snd_soc_of_parse_card_name(&data->card, "model");
 	if (ret)
-		goto fail;
+		goto clk_fail;
 	ret = snd_soc_of_parse_audio_routing(&data->card, "audio-routing");
 	if (ret)
-		goto fail;
+		goto clk_fail;
 	data->card.num_links = 1;
 	data->card.dai_link = &data->dai;
 	data->card.dapm_widgets = imx_sgtl5000_dapm_widgets;
@@ -152,10 +170,12 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev)
 	ret = snd_soc_register_card(&data->card);
 	if (ret) {
 		dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n", ret);
-		goto fail;
+		goto clk_fail;
 	}
 
 	platform_set_drvdata(pdev, data);
+clk_fail:
+	clk_put(data->codec_clk);
 fail:
 	if (ssi_np)
 		of_node_put(ssi_np);
@@ -169,6 +189,10 @@ static int __devexit imx_sgtl5000_remove(struct platform_device *pdev)
 {
 	struct imx_sgtl5000_data *data = platform_get_drvdata(pdev);
 
+	if (data->codec_clk) {
+		clk_disable_unprepare(data->codec_clk);
+		clk_put(data->codec_clk);
+	}
 	snd_soc_unregister_card(&data->card);
 
 	return 0;
-- 
1.7.5.4

Re: [PATCH 03/11] ASoC: fsl: add sgtl5000 clock support for imx-sgtl5000

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 231 bytes --]

On Fri, Apr 27, 2012 at 03:02:57PM +0800, Richard Zhao wrote:
> From: Richard Zhao <richard.zhao@linaro.org>
> 
> It tries to clk_get the clock. And if it failed, it assumes the clock
> by default enabled.

Applied, thanks.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 03/11] ASoC: fsl: add sgtl5000 clock support for imx-sgtl5000

[Shawn Guo]

On Fri, Apr 27, 2012 at 03:02:57PM +0800, Richard Zhao wrote:
> From: Richard Zhao <richard.zhao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> 
> It tries to clk_get the clock. And if it failed, it assumes the clock
> by default enabled.
> 
> Signed-off-by: Richard Zhao <richard.zhao-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> ---
>  sound/soc/fsl/imx-sgtl5000.c |   40 ++++++++++++++++++++++++++++++++--------
>  1 files changed, 32 insertions(+), 8 deletions(-)
> 
> diff --git a/sound/soc/fsl/imx-sgtl5000.c b/sound/soc/fsl/imx-sgtl5000.c
> index 73b935e..3a729ca 100644
> --- a/sound/soc/fsl/imx-sgtl5000.c
> +++ b/sound/soc/fsl/imx-sgtl5000.c
> @@ -13,6 +13,8 @@
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_platform.h>
> +#include <linux/of_i2c.h>
> +#include <linux/clk.h>
>  #include <sound/soc.h>
>  
>  #include "../codecs/sgtl5000.h"
> @@ -25,6 +27,7 @@ struct imx_sgtl5000_data {
>  	struct snd_soc_card card;
>  	char codec_dai_name[DAI_NAME_SIZE];
>  	char platform_name[DAI_NAME_SIZE];
> +	struct clk *codec_clk;
>  	unsigned int clk_frequency;
>  };
>  
> @@ -58,6 +61,7 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev)
>  	struct device_node *np = pdev->dev.of_node;
>  	struct device_node *ssi_np, *codec_np;
>  	struct platform_device *ssi_pdev;
> +	struct i2c_client *codec_dev;
>  	struct imx_sgtl5000_data *data;
>  	int int_port, ext_port;
>  	int ret;
> @@ -113,6 +117,11 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev)
>  		ret = -EINVAL;
>  		goto fail;
>  	}
> +	codec_dev = of_find_i2c_device_by_node(codec_np);
> +	if (!codec_dev) {
> +		dev_err(&pdev->dev, "failed to find codec platform device\n");
> +		return -EINVAL;
> +	}

What if the codec is accessed via SPI bus on some machines?

>  
>  	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
>  	if (!data) {
> @@ -120,11 +129,20 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev)
>  		goto fail;
>  	}
>  
> -	ret = of_property_read_u32(codec_np, "clock-frequency",
> -				   &data->clk_frequency);
> -	if (ret) {
> -		dev_err(&pdev->dev, "clock-frequency missing or invalid\n");
> -		goto fail;
> +	data->codec_clk = clk_get(&codec_dev->dev, NULL);

It's a clock of sgtl5000 codec.  I feel it makes more sense to have
sgtl5000 device driver than machine driver to manage this clock.

Regards,
Shawn

> +	if (IS_ERR(data->codec_clk)) {
> +		/* assuming clock enabled by default */
> +		data->codec_clk = NULL;
> +		ret = of_property_read_u32(codec_np, "clock-frequency",
> +					&data->clk_frequency);
> +		if (ret) {
> +			dev_err(&codec_dev->dev,
> +				"clock-frequency missing or invalid\n");
> +			goto fail;
> +		}
> +	} else {
> +		data->clk_frequency = clk_get_rate(data->codec_clk);
> +		clk_prepare_enable(data->codec_clk);
>  	}

Re: [PATCH 03/11] ASoC: fsl: add sgtl5000 clock support for imx-sgtl5000

[Richard Zhao]

On Tue, May 01, 2012 at 09:44:48PM +0800, Shawn Guo wrote:
> On Fri, Apr 27, 2012 at 03:02:57PM +0800, Richard Zhao wrote:
> > From: Richard Zhao <richard.zhao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > 
> > It tries to clk_get the clock. And if it failed, it assumes the clock
> > by default enabled.
> > 
> > Signed-off-by: Richard Zhao <richard.zhao-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > ---
> >  sound/soc/fsl/imx-sgtl5000.c |   40 ++++++++++++++++++++++++++++++++--------
> >  1 files changed, 32 insertions(+), 8 deletions(-)
> > 
> > diff --git a/sound/soc/fsl/imx-sgtl5000.c b/sound/soc/fsl/imx-sgtl5000.c
> > index 73b935e..3a729ca 100644
> > --- a/sound/soc/fsl/imx-sgtl5000.c
> > +++ b/sound/soc/fsl/imx-sgtl5000.c
> > @@ -13,6 +13,8 @@
> >  #include <linux/module.h>
> >  #include <linux/of.h>
> >  #include <linux/of_platform.h>
> > +#include <linux/of_i2c.h>
> > +#include <linux/clk.h>
> >  #include <sound/soc.h>
> >  
> >  #include "../codecs/sgtl5000.h"
> > @@ -25,6 +27,7 @@ struct imx_sgtl5000_data {
> >  	struct snd_soc_card card;
> >  	char codec_dai_name[DAI_NAME_SIZE];
> >  	char platform_name[DAI_NAME_SIZE];
> > +	struct clk *codec_clk;
> >  	unsigned int clk_frequency;
> >  };
> >  
> > @@ -58,6 +61,7 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev)
> >  	struct device_node *np = pdev->dev.of_node;
> >  	struct device_node *ssi_np, *codec_np;
> >  	struct platform_device *ssi_pdev;
> > +	struct i2c_client *codec_dev;
> >  	struct imx_sgtl5000_data *data;
> >  	int int_port, ext_port;
> >  	int ret;
> > @@ -113,6 +117,11 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev)
> >  		ret = -EINVAL;
> >  		goto fail;
> >  	}
> > +	codec_dev = of_find_i2c_device_by_node(codec_np);
> > +	if (!codec_dev) {
> > +		dev_err(&pdev->dev, "failed to find codec platform device\n");
> > +		return -EINVAL;
> > +	}
> 
> What if the codec is accessed via SPI bus on some machines?
> 
> >  
> >  	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> >  	if (!data) {
> > @@ -120,11 +129,20 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev)
> >  		goto fail;
> >  	}
> >  
> > -	ret = of_property_read_u32(codec_np, "clock-frequency",
> > -				   &data->clk_frequency);
> > -	if (ret) {
> > -		dev_err(&pdev->dev, "clock-frequency missing or invalid\n");
> > -		goto fail;
> > +	data->codec_clk = clk_get(&codec_dev->dev, NULL);
> 
> It's a clock of sgtl5000 codec.  I feel it makes more sense to have
> sgtl5000 device driver than machine driver to manage this clock.
The ASoC machine driver handled the clock settings, I just followed the
same way, using snd_soc_dai_set_sysclk to set codec clk. But what you
said  is reasonable. I also find it's impossible to use devm_get_clk.

Mark, do you suggest get clk in ASoC machine driver or in codec driver?

Thanks
Richard
> 
> Regards,
> Shawn
> 
> > +	if (IS_ERR(data->codec_clk)) {
> > +		/* assuming clock enabled by default */
> > +		data->codec_clk = NULL;
> > +		ret = of_property_read_u32(codec_np, "clock-frequency",
> > +					&data->clk_frequency);
> > +		if (ret) {
> > +			dev_err(&codec_dev->dev,
> > +				"clock-frequency missing or invalid\n");
> > +			goto fail;
> > +		}
> > +	} else {
> > +		data->clk_frequency = clk_get_rate(data->codec_clk);
> > +		clk_prepare_enable(data->codec_clk);
> >  	}
> 

Re: [PATCH 03/11] ASoC: fsl: add sgtl5000 clock support for imx-sgtl5000

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1606 bytes --]

On Wed, May 02, 2012 at 06:50:24PM +0800, Richard Zhao wrote:
> On Tue, May 01, 2012 at 09:44:48PM +0800, Shawn Guo wrote:

> > > +	data->codec_clk = clk_get(&codec_dev->dev, NULL);

> > It's a clock of sgtl5000 codec.  I feel it makes more sense to have
> > sgtl5000 device driver than machine driver to manage this clock.

> The ASoC machine driver handled the clock settings, I just followed the
> same way, using snd_soc_dai_set_sysclk to set codec clk. But what you
> said  is reasonable. I also find it's impossible to use devm_get_clk.

> Mark, do you suggest get clk in ASoC machine driver or in codec driver?

We generally try to keep the clock API out of CODEC drivers due to the
issues with the clock API that the generic implementation is intended to
help us solve.

The major problem here is that we can't rely on having the clock API
present at all on most platforms so as soon as we start adding clock
support into things we have to do a performance with HAVE_CLK which is
at best annoying.

There's also the fact that even if the clock API is available we've no
way to register a clock consistently and since the audio clock tree
often needs management off-SoC for at least some components when the
clock API is in use you end up having to handle clock trees that go into
and out of the clock API.

The upshot of this is that while we really should be using the clock API
for clocks the API as it stands is somewhat difficult to deploy in cross
platform code.  What I'd suggest is connecting the clock to the CODEC in
the device tree but actually doing the clk_get() in the machine driver.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 03/11] ASoC: fsl: add sgtl5000 clock support for imx-sgtl5000

[Shawn Guo]

On Wed, May 02, 2012 at 12:41:22PM +0100, Mark Brown wrote:
> We generally try to keep the clock API out of CODEC drivers due to the
> issues with the clock API that the generic implementation is intended to
> help us solve.
> 
> The major problem here is that we can't rely on having the clock API
> present at all on most platforms so as soon as we start adding clock
> support into things we have to do a performance with HAVE_CLK which is
> at best annoying.
> 
Is this something that the series below tries to solve?

http://thread.gmane.org/gmane.linux.ports.arm.kernel/163652

Regards,
Shawn

> There's also the fact that even if the clock API is available we've no
> way to register a clock consistently and since the audio clock tree
> often needs management off-SoC for at least some components when the
> clock API is in use you end up having to handle clock trees that go into
> and out of the clock API.
> 
> The upshot of this is that while we really should be using the clock API
> for clocks the API as it stands is somewhat difficult to deploy in cross
> platform code.  What I'd suggest is connecting the clock to the CODEC in
> the device tree but actually doing the clk_get() in the machine driver.

Re: [PATCH 03/11] ASoC: fsl: add sgtl5000 clock support for imx-sgtl5000

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 845 bytes --]

On Wed, May 02, 2012 at 08:12:24PM +0800, Shawn Guo wrote:
> On Wed, May 02, 2012 at 12:41:22PM +0100, Mark Brown wrote:

> > The major problem here is that we can't rely on having the clock API
> > present at all on most platforms so as soon as we start adding clock
> > support into things we have to do a performance with HAVE_CLK which is
> > at best annoying.

> Is this something that the series below tries to solve?

> http://thread.gmane.org/gmane.linux.ports.arm.kernel/163652

Well, it helps building but that's only part of the issue - we've still
got registration problems and if we were using the API in anger we would
need things like clk_set_rate() to actually work.  It's not just a case
of simply enabling and disabling clocks (which can be stubbed out pretty
effectively), audio generally needs to manage the clock rates too.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH 04/11] i2c: imx: add pinctrl support

[Richard Zhao]

Signed-off-by: Richard Zhao <richard.zhao-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 arch/arm/boot/dts/imx6q-sabrelite.dts |    2 ++
 arch/arm/boot/dts/imx6q.dtsi          |   16 ++++++++++++++++
 drivers/i2c/busses/i2c-imx.c          |    9 +++++++++
 3 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts
index 4663a4e..4e13293 100644
--- a/arch/arm/boot/dts/imx6q-sabrelite.dts
+++ b/arch/arm/boot/dts/imx6q-sabrelite.dts
@@ -50,6 +50,8 @@
 			i2c@021a0000 { /* I2C1 */
 				status = "okay";
 				clock-frequency = <100000>;
+				pinctrl-names = "default";
+				pinctrl-0 = <&pinctrl_i2c1_1>;
 
 				codec: sgtl5000@0a {
 					compatible = "fsl,sgtl5000";
diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index 2ba32e7..fe8c80d 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -390,6 +390,22 @@
 				reg = <0x020e0000 0x4000>;
 
 				/* shared pinctrl settings */
+				i2c1 {
+					pinctrl_i2c1_1: i2c1grp-1 {
+						fsl,pins =	"MX6Q_PAD_EIM_D21",
+								"MX6Q_PAD_EIM_D28";
+						fsl,hysteresis = <1>;
+						fsl,mux = <0x16 0x11>;
+						fsl,pull = <2>;
+						fsl,pue = <1>;
+						fsl,pke = <1>;
+						fsl,open-drain = <1>;
+						fsl,speed = <2>;
+						fsl,drive-strength = <6>;
+						fsl,slew-rate = <1>;
+					};
+				};
+
 				uart4 {
 					pinctrl_uart4_1: uart4grp-1 {
 						fsl,pins =	"MX6Q_PAD_KEY_COL0",
diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index dfb84b7..7a52067 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -48,6 +48,7 @@
 #include <linux/platform_device.h>
 #include <linux/clk.h>
 #include <linux/slab.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/of_i2c.h>
@@ -470,6 +471,7 @@ static int __init i2c_imx_probe(struct platform_device *pdev)
 	struct imx_i2c_struct *i2c_imx;
 	struct resource *res;
 	struct imxi2c_platform_data *pdata = pdev->dev.platform_data;
+	struct pinctrl *pct;
 	void __iomem *base;
 	resource_size_t res_size;
 	int irq, bitrate;
@@ -520,6 +522,13 @@ static int __init i2c_imx_probe(struct platform_device *pdev)
 	i2c_imx->base			= base;
 	i2c_imx->res			= res;
 
+	pct = devm_pinctrl_get_select_default(&pdev->dev);
+	if (IS_ERR(pct)) {
+		dev_err(&pdev->dev, "can't get/select pinctrl\n");
+		ret = PTR_ERR(pct);
+		goto fail3;
+	}
+
 	/* Get I2C clock */
 	i2c_imx->clk = clk_get(&pdev->dev, "i2c_clk");
 	if (IS_ERR(i2c_imx->clk)) {
-- 
1.7.5.4

Re: [PATCH 04/11] i2c: imx: add pinctrl support

[Dong Aisheng]

On Fri, Apr 27, 2012 at 03:02:58PM +0800, Richard Zhao wrote:
> Signed-off-by: Richard Zhao <richard.zhao-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> ---
>  arch/arm/boot/dts/imx6q-sabrelite.dts |    2 ++
>  arch/arm/boot/dts/imx6q.dtsi          |   16 ++++++++++++++++
>  drivers/i2c/busses/i2c-imx.c          |    9 +++++++++
>  3 files changed, 27 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts
> index 4663a4e..4e13293 100644
> --- a/arch/arm/boot/dts/imx6q-sabrelite.dts
> +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts
> @@ -50,6 +50,8 @@
>  			i2c@021a0000 { /* I2C1 */
>  				status = "okay";
>  				clock-frequency = <100000>;
> +				pinctrl-names = "default";
> +				pinctrl-0 = <&pinctrl_i2c1_1>;
>  
>  				codec: sgtl5000@0a {
>  					compatible = "fsl,sgtl5000";
> diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
> index 2ba32e7..fe8c80d 100644
> --- a/arch/arm/boot/dts/imx6q.dtsi
> +++ b/arch/arm/boot/dts/imx6q.dtsi
> @@ -390,6 +390,22 @@
>  				reg = <0x020e0000 0x4000>;
>  
>  				/* shared pinctrl settings */
> +				i2c1 {
> +					pinctrl_i2c1_1: i2c1grp-1 {
> +						fsl,pins =	"MX6Q_PAD_EIM_D21",
> +								"MX6Q_PAD_EIM_D28";
> +						fsl,hysteresis = <1>;
> +						fsl,mux = <0x16 0x11>;
> +						fsl,pull = <2>;
> +						fsl,pue = <1>;
> +						fsl,pke = <1>;
> +						fsl,open-drain = <1>;
> +						fsl,speed = <2>;
> +						fsl,drive-strength = <6>;
> +						fsl,slew-rate = <1>;
> +					};
The pinctrl binding is changed a bit since v1.
You may need to change here according to v2 or latter.

> +				};
> +
>  				uart4 {
>  					pinctrl_uart4_1: uart4grp-1 {
>  						fsl,pins =	"MX6Q_PAD_KEY_COL0",
> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> index dfb84b7..7a52067 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -48,6 +48,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/clk.h>
>  #include <linux/slab.h>
> +#include <linux/pinctrl/consumer.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
>  #include <linux/of_i2c.h>
> @@ -470,6 +471,7 @@ static int __init i2c_imx_probe(struct platform_device *pdev)
>  	struct imx_i2c_struct *i2c_imx;
>  	struct resource *res;
>  	struct imxi2c_platform_data *pdata = pdev->dev.platform_data;
> +	struct pinctrl *pct;
>  	void __iomem *base;
>  	resource_size_t res_size;
>  	int irq, bitrate;
> @@ -520,6 +522,13 @@ static int __init i2c_imx_probe(struct platform_device *pdev)
>  	i2c_imx->base			= base;
>  	i2c_imx->res			= res;
>  
> +	pct = devm_pinctrl_get_select_default(&pdev->dev);
You may want to check this change will break other platforms also using this driver.
Refer to:
http://www.spinics.net/lists/arm-kernel/msg171538.html

Regards
Dong Aisheng

Re: [PATCH 04/11] i2c: imx: add pinctrl support

[Richard Zhao]

On Fri, Apr 27, 2012 at 04:40:57PM +0800, Dong Aisheng wrote:
> On Fri, Apr 27, 2012 at 03:02:58PM +0800, Richard Zhao wrote:
> > Signed-off-by: Richard Zhao <richard.zhao-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > ---
> >  arch/arm/boot/dts/imx6q-sabrelite.dts |    2 ++
> >  arch/arm/boot/dts/imx6q.dtsi          |   16 ++++++++++++++++
> >  drivers/i2c/busses/i2c-imx.c          |    9 +++++++++
> >  3 files changed, 27 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts
> > index 4663a4e..4e13293 100644
> > --- a/arch/arm/boot/dts/imx6q-sabrelite.dts
> > +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts
> > @@ -50,6 +50,8 @@
> >  			i2c@021a0000 { /* I2C1 */
> >  				status = "okay";
> >  				clock-frequency = <100000>;
> > +				pinctrl-names = "default";
> > +				pinctrl-0 = <&pinctrl_i2c1_1>;
> >  
> >  				codec: sgtl5000@0a {
> >  					compatible = "fsl,sgtl5000";
> > diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
> > index 2ba32e7..fe8c80d 100644
> > --- a/arch/arm/boot/dts/imx6q.dtsi
> > +++ b/arch/arm/boot/dts/imx6q.dtsi
> > @@ -390,6 +390,22 @@
> >  				reg = <0x020e0000 0x4000>;
> >  
> >  				/* shared pinctrl settings */
> > +				i2c1 {
> > +					pinctrl_i2c1_1: i2c1grp-1 {
> > +						fsl,pins =	"MX6Q_PAD_EIM_D21",
> > +								"MX6Q_PAD_EIM_D28";
> > +						fsl,hysteresis = <1>;
> > +						fsl,mux = <0x16 0x11>;
> > +						fsl,pull = <2>;
> > +						fsl,pue = <1>;
> > +						fsl,pke = <1>;
> > +						fsl,open-drain = <1>;
> > +						fsl,speed = <2>;
> > +						fsl,drive-strength = <6>;
> > +						fsl,slew-rate = <1>;
> > +					};
> The pinctrl binding is changed a bit since v1.
> You may need to change here according to v2 or latter.
Yes, I'll try your v3 with SION.
> 
> > +				};
> > +
> >  				uart4 {
> >  					pinctrl_uart4_1: uart4grp-1 {
> >  						fsl,pins =	"MX6Q_PAD_KEY_COL0",
> > diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> > index dfb84b7..7a52067 100644
> > --- a/drivers/i2c/busses/i2c-imx.c
> > +++ b/drivers/i2c/busses/i2c-imx.c
> > @@ -48,6 +48,7 @@
> >  #include <linux/platform_device.h>
> >  #include <linux/clk.h>
> >  #include <linux/slab.h>
> > +#include <linux/pinctrl/consumer.h>
> >  #include <linux/of.h>
> >  #include <linux/of_device.h>
> >  #include <linux/of_i2c.h>
> > @@ -470,6 +471,7 @@ static int __init i2c_imx_probe(struct platform_device *pdev)
> >  	struct imx_i2c_struct *i2c_imx;
> >  	struct resource *res;
> >  	struct imxi2c_platform_data *pdata = pdev->dev.platform_data;
> > +	struct pinctrl *pct;
> >  	void __iomem *base;
> >  	resource_size_t res_size;
> >  	int irq, bitrate;
> > @@ -520,6 +522,13 @@ static int __init i2c_imx_probe(struct platform_device *pdev)
> >  	i2c_imx->base			= base;
> >  	i2c_imx->res			= res;
> >  
> > +	pct = devm_pinctrl_get_select_default(&pdev->dev);
> You may want to check this change will break other platforms also using this driver.
> Refer to:
> http://www.spinics.net/lists/arm-kernel/msg171538.html
I didn't expect it can go in right now. It surely depends on your dummy pinctrl.

Thanks
Richard
> 
> Regards
> Dong Aisheng

[PATCH 05/11] ARM: imx6q: move imx6q_sabrelite specific code to a dedicated function

[Richard Zhao]

It'll be easier to add other board specific code.

Signed-off-by: Richard Zhao <richard.zhao-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 arch/arm/mach-imx/mach-imx6q.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index da6c1d9..d25c5d8 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -75,11 +75,16 @@ static int ksz9021rn_phy_fixup(struct phy_device *phydev)
 	return 0;
 }
 
+static void __init imx6q_sabrelite_init(void)
+{
+	phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
+				ksz9021rn_phy_fixup);
+}
+
 static void __init imx6q_init_machine(void)
 {
 	if (of_machine_is_compatible("fsl,imx6q-sabrelite"))
-		phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
-					   ksz9021rn_phy_fixup);
+		imx6q_sabrelite_init();
 
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
 
-- 
1.7.5.4

Re: [PATCH 05/11] ARM: imx6q: move imx6q_sabrelite specific code to a dedicated function

[Shawn Guo]

On Fri, Apr 27, 2012 at 03:02:59PM +0800, Richard Zhao wrote:
> It'll be easier to add other board specific code.
> 
> Signed-off-by: Richard Zhao <richard.zhao-KZfg59tc24xl57MIdRCFDg@public.gmane.org>

Applied, thanks.

Regards,
Shawn

[PATCH 06/11] ARM: dts: imx6q-sabrelite: add ssi device

[Richard Zhao]

From: Richard Zhao <richard.zhao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Signed-off-by: Richard Zhao <richard.zhao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Richard Zhao <richard.zhao-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 arch/arm/boot/dts/imx6q-sabrelite.dts |    9 +++++++++
 arch/arm/boot/dts/imx6q.dtsi          |   18 +++++++++++++++---
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts
index 4e13293..7bd8855 100644
--- a/arch/arm/boot/dts/imx6q-sabrelite.dts
+++ b/arch/arm/boot/dts/imx6q-sabrelite.dts
@@ -22,6 +22,15 @@
 	};
 
 	soc {
+		aips-bus@02000000 { /* AIPS1 */
+			spba-bus@02000000 {
+				ssi1: ssi@02028000 {
+					fsl,mode = "i2s-slave";
+					status = "okay";
+				};
+			};
+		};
+
 		aips-bus@02100000 { /* AIPS2 */
 			enet@02188000 {
 				phy-mode = "rgmii";
diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index fe8c80d..da42fc0 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -177,19 +177,31 @@
 					interrupts = <0 51 0x04>;
 				};
 
-				ssi@02028000 { /* SSI1 */
+				ssi1: ssi@02028000 {
+					compatible = "fsl,imx6q-ssi","fsl,imx21-ssi";
 					reg = <0x02028000 0x4000>;
 					interrupts = <0 46 0x04>;
+					fsl,fifo-depth = <15>;
+					fsl,ssi-dma-events = <38 37>;
+					status = "disabled";
 				};
 
-				ssi@0202c000 { /* SSI2 */
+				ssi2: ssi@0202c000 {
+					compatible = "fsl,imx6q-ssi","fsl,imx21-ssi";
 					reg = <0x0202c000 0x4000>;
 					interrupts = <0 47 0x04>;
+					fsl,fifo-depth = <15>;
+					fsl,ssi-dma-events = <42 41>;
+					status = "disabled";
 				};
 
-				ssi@02030000 { /* SSI3 */
+				ssi3: ssi@02030000 {
+					compatible = "fsl,imx6q-ssi","fsl,imx21-ssi";
 					reg = <0x02030000 0x4000>;
 					interrupts = <0 48 0x04>;
+					fsl,fifo-depth = <15>;
+					fsl,ssi-dma-events = <46 45>;
+					status = "disabled";
 				};
 
 				asrc@02034000 {
-- 
1.7.5.4

Re: [PATCH 06/11] ARM: dts: imx6q-sabrelite: add ssi device

[Shawn Guo]

On Fri, Apr 27, 2012 at 03:03:00PM +0800, Richard Zhao wrote:
> From: Richard Zhao <richard.zhao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> 
> Signed-off-by: Richard Zhao <richard.zhao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Richard Zhao <richard.zhao-KZfg59tc24xl57MIdRCFDg@public.gmane.org>

I tried to apply patch #6 and #7, but they did not apply.

-- 
Regards,
Shawn

Re: [PATCH 06/11] ARM: dts: imx6q-sabrelite: add ssi device

[Shawn Guo]

On Tue, May 01, 2012 at 09:26:53PM +0800, Shawn Guo wrote:
> On Fri, Apr 27, 2012 at 03:03:00PM +0800, Richard Zhao wrote:
> > From: Richard Zhao <richard.zhao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > 
> > Signed-off-by: Richard Zhao <richard.zhao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > Signed-off-by: Richard Zhao <richard.zhao-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> 
> I tried to apply patch #6 and #7, but they did not apply.
> 
Just manually applied these two patches on imx/dt branch, please check
if they are applied correctly.

-- 
Regards,
Shawn

[PATCH 07/11] ARM: dts: imx6q-sabrelite: add audmux device

[Richard Zhao]

From: Richard Zhao <richard.zhao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Signed-off-by: Richard Zhao <richard.zhao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Richard Zhao <richard.zhao-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 arch/arm/boot/dts/imx6q-sabrelite.dts |    3 +++
 arch/arm/boot/dts/imx6q.dtsi          |    2 ++
 2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts
index 7bd8855..02f93bc 100644
--- a/arch/arm/boot/dts/imx6q-sabrelite.dts
+++ b/arch/arm/boot/dts/imx6q-sabrelite.dts
@@ -52,6 +52,9 @@
 				status = "okay";
 			};
 
+			audmux@021d8000 {
+				status = "okay";
+			};
 			uart2: uart@021e8000 {
 				status = "okay";
 			};
diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index da42fc0..7bf402e 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -596,7 +596,9 @@
 			};
 
 			audmux@021d8000 {
+				compatible = "fsl,imx6q-audmux", "fsl,imx31-audmux";
 				reg = <0x021d8000 0x4000>;
+				status = "disabled";
 			};
 
 			mipi@021dc000 { /* MIPI-CSI */
-- 
1.7.5.4

[PATCH 08/11] ASoC: imx-audmux: add pinctrl support

[Richard Zhao]

Signed-off-by: Richard Zhao <richard.zhao-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 arch/arm/boot/dts/imx6q-sabrelite.dts |    3 +++
 arch/arm/boot/dts/imx6q.dtsi          |   10 ++++++++++
 sound/soc/fsl/imx-audmux.c            |    8 ++++++++
 3 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts
index 02f93bc..cdae2dd 100644
--- a/arch/arm/boot/dts/imx6q-sabrelite.dts
+++ b/arch/arm/boot/dts/imx6q-sabrelite.dts
@@ -54,7 +54,10 @@
 
 			audmux@021d8000 {
 				status = "okay";
+				pinctrl-names = "default";
+				pinctrl-0 = <&pinctrl_audmux_1>;
 			};
+
 			uart2: uart@021e8000 {
 				status = "okay";
 			};
diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index 7bf402e..3c3004d 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -402,6 +402,16 @@
 				reg = <0x020e0000 0x4000>;
 
 				/* shared pinctrl settings */
+				audmux {
+					pinctrl_audmux_1: audmux-1 {
+						fsl,pins =	"MX6Q_PAD_SD2_DAT0",
+								"MX6Q_PAD_SD2_DAT3",
+								"MX6Q_PAD_SD2_DAT2",
+								"MX6Q_PAD_SD2_DAT1";
+						fsl,mux = <3 3 3 3>;
+					};
+				};
+
 				i2c1 {
 					pinctrl_i2c1_1: i2c1grp-1 {
 						fsl,pins =	"MX6Q_PAD_EIM_D21",
diff --git a/sound/soc/fsl/imx-audmux.c b/sound/soc/fsl/imx-audmux.c
index f237003..6c7dfc0 100644
--- a/sound/soc/fsl/imx-audmux.c
+++ b/sound/soc/fsl/imx-audmux.c
@@ -26,6 +26,7 @@
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
+#include <linux/pinctrl/consumer.h>
 
 #include "imx-audmux.h"
 
@@ -249,6 +250,7 @@ EXPORT_SYMBOL_GPL(imx_audmux_v2_configure_port);
 static int __devinit imx_audmux_probe(struct platform_device *pdev)
 {
 	struct resource *res;
+	struct pinctrl *pct;
 	const struct of_device_id *of_id =
 			of_match_device(imx_audmux_dt_ids, &pdev->dev);
 
@@ -257,6 +259,12 @@ static int __devinit imx_audmux_probe(struct platform_device *pdev)
 	if (!audmux_base)
 		return -EADDRNOTAVAIL;
 
+	pct = devm_pinctrl_get_select_default(&pdev->dev);
+	if (IS_ERR(pct)) {
+		dev_err(&pdev->dev, "setup pinctrl failed!");
+		return PTR_ERR(pct);
+	}
+
 	audmux_clk = clk_get(&pdev->dev, "audmux");
 	if (IS_ERR(audmux_clk)) {
 		dev_dbg(&pdev->dev, "cannot get clock: %ld\n",
-- 
1.7.5.4

[PATCH 09/11] ARM: imx6q: add ssi1 clk_lookup

[Richard Zhao]

It's used by audio drivers.

Signed-off-by: Richard Zhao <richard.zhao-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 arch/arm/mach-imx/clk-imx6q.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
index f40a35d..9a03dcc 100644
--- a/arch/arm/mach-imx/clk-imx6q.c
+++ b/arch/arm/mach-imx/clk-imx6q.c
@@ -418,6 +418,7 @@ int __init mx6q_clocks_init(void)
 	clk_register_clkdev(clk[sdma], NULL, "20ec000.sdma");
 	clk_register_clkdev(clk[dummy], NULL, "20bc000.wdog");
 	clk_register_clkdev(clk[dummy], NULL, "20c0000.wdog");
+	clk_register_clkdev(clk[ssi1], NULL, "2028000.ssi");
 
 	for (i = 0; i < ARRAY_SIZE(clks_init_on); i++) {
 		c = clk_get_sys(clks_init_on[i], NULL);
-- 
1.7.5.4

Re: [PATCH 09/11] ARM: imx6q: add ssi1 clk_lookup

[Sascha Hauer]

On Fri, Apr 27, 2012 at 03:03:03PM +0800, Richard Zhao wrote:
> It's used by audio drivers.
> 
> Signed-off-by: Richard Zhao <richard.zhao-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> ---
>  arch/arm/mach-imx/clk-imx6q.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
> index f40a35d..9a03dcc 100644
> --- a/arch/arm/mach-imx/clk-imx6q.c
> +++ b/arch/arm/mach-imx/clk-imx6q.c
> @@ -418,6 +418,7 @@ int __init mx6q_clocks_init(void)
>  	clk_register_clkdev(clk[sdma], NULL, "20ec000.sdma");
>  	clk_register_clkdev(clk[dummy], NULL, "20bc000.wdog");
>  	clk_register_clkdev(clk[dummy], NULL, "20c0000.wdog");
> +	clk_register_clkdev(clk[ssi1], NULL, "2028000.ssi");

The ssi clock needs a general cleanup on all i.MX just like I cleaned up
the other units. The SSI unit has at least a register clock and a baud
clock. What the driver requests and enables is the register clock.

The baud clock is currently unused and is needed only for master mode
(which is not implemented in the ssi driver)

So where we want to come to is:

	clk_register_clkdev(clk[ipg], "ipg", "2028000.ssi");
	clk_register_clkdev(clk[ssi1], "baud", "2028000.ssi");

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 09/11] ARM: imx6q: add ssi1 clk_lookup

[Richard Zhao]

On Fri, Apr 27, 2012 at 10:04:12AM +0200, Sascha Hauer wrote:
> On Fri, Apr 27, 2012 at 03:03:03PM +0800, Richard Zhao wrote:
> > It's used by audio drivers.
> > 
> > Signed-off-by: Richard Zhao <richard.zhao-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > ---
> >  arch/arm/mach-imx/clk-imx6q.c |    1 +
> >  1 files changed, 1 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
> > index f40a35d..9a03dcc 100644
> > --- a/arch/arm/mach-imx/clk-imx6q.c
> > +++ b/arch/arm/mach-imx/clk-imx6q.c
> > @@ -418,6 +418,7 @@ int __init mx6q_clocks_init(void)
> >  	clk_register_clkdev(clk[sdma], NULL, "20ec000.sdma");
> >  	clk_register_clkdev(clk[dummy], NULL, "20bc000.wdog");
> >  	clk_register_clkdev(clk[dummy], NULL, "20c0000.wdog");
> > +	clk_register_clkdev(clk[ssi1], NULL, "2028000.ssi");
> 
> The ssi clock needs a general cleanup on all i.MX just like I cleaned up
> the other units. The SSI unit has at least a register clock and a baud
> clock. What the driver requests and enables is the register clock.
> 
> The baud clock is currently unused and is needed only for master mode
> (which is not implemented in the ssi driver)
Are you sure for that? If I don't enable clk[ssi1], the ssi will not
work.

> 
> So where we want to come to is:
> 
> 	clk_register_clkdev(clk[ipg], "ipg", "2028000.ssi");
ssi don't have ipg gate. We can let it always on for imx6q.

Thanks
Richard
> 	clk_register_clkdev(clk[ssi1], "baud", "2028000.ssi");
> 
> Sascha
> 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 

Re: [PATCH 09/11] ARM: imx6q: add ssi1 clk_lookup

[Sascha Hauer]

On Fri, Apr 27, 2012 at 05:10:56PM +0800, Richard Zhao wrote:
> On Fri, Apr 27, 2012 at 10:04:12AM +0200, Sascha Hauer wrote:
> > On Fri, Apr 27, 2012 at 03:03:03PM +0800, Richard Zhao wrote:
> > > It's used by audio drivers.
> > > 
> > > Signed-off-by: Richard Zhao <richard.zhao-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > > ---
> > >  arch/arm/mach-imx/clk-imx6q.c |    1 +
> > >  1 files changed, 1 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
> > > index f40a35d..9a03dcc 100644
> > > --- a/arch/arm/mach-imx/clk-imx6q.c
> > > +++ b/arch/arm/mach-imx/clk-imx6q.c
> > > @@ -418,6 +418,7 @@ int __init mx6q_clocks_init(void)
> > >  	clk_register_clkdev(clk[sdma], NULL, "20ec000.sdma");
> > >  	clk_register_clkdev(clk[dummy], NULL, "20bc000.wdog");
> > >  	clk_register_clkdev(clk[dummy], NULL, "20c0000.wdog");
> > > +	clk_register_clkdev(clk[ssi1], NULL, "2028000.ssi");
> > 
> > The ssi clock needs a general cleanup on all i.MX just like I cleaned up
> > the other units. The SSI unit has at least a register clock and a baud
> > clock. What the driver requests and enables is the register clock.
> > 
> > The baud clock is currently unused and is needed only for master mode
> > (which is not implemented in the ssi driver)
> Are you sure for that? If I don't enable clk[ssi1], the ssi will not
> work.
> 
> > 
> > So where we want to come to is:
> > 
> > 	clk_register_clkdev(clk[ipg], "ipg", "2028000.ssi");
> ssi don't have ipg gate. We can let it always on for imx6q.

Can you please ask your IC guys for clarification?

For example on i.MX5 we have a ssi ipg clock and a ssi serial clock.
Both can be gated with two individual gate bits.

The i.MX6 datasheet (and also several other i.MX datasheets) is quite
nebulous. The i.MX6 has only one gate bit for each SSI unit, but
it's not clear if this bit actually gates both the ipg and serial
clock or only one of them.

My general idea is that each unit in the SoC has different input clocks.
In the driver we need to clk_get() the input clocks. If a given SoC has
no software control over some of the devices input clocks, then we need
to provide a dummy for this, because other SoCs have control over the
clock.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 09/11] ARM: imx6q: add ssi1 clk_lookup

[Richard Zhao]

On Fri, Apr 27, 2012 at 11:25:29AM +0200, Sascha Hauer wrote:
> On Fri, Apr 27, 2012 at 05:10:56PM +0800, Richard Zhao wrote:
> > On Fri, Apr 27, 2012 at 10:04:12AM +0200, Sascha Hauer wrote:
> > > On Fri, Apr 27, 2012 at 03:03:03PM +0800, Richard Zhao wrote:
> > > > It's used by audio drivers.
> > > > 
> > > > Signed-off-by: Richard Zhao <richard.zhao-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > > > ---
> > > >  arch/arm/mach-imx/clk-imx6q.c |    1 +
> > > >  1 files changed, 1 insertions(+), 0 deletions(-)
> > > > 
> > > > diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
> > > > index f40a35d..9a03dcc 100644
> > > > --- a/arch/arm/mach-imx/clk-imx6q.c
> > > > +++ b/arch/arm/mach-imx/clk-imx6q.c
> > > > @@ -418,6 +418,7 @@ int __init mx6q_clocks_init(void)
> > > >  	clk_register_clkdev(clk[sdma], NULL, "20ec000.sdma");
> > > >  	clk_register_clkdev(clk[dummy], NULL, "20bc000.wdog");
> > > >  	clk_register_clkdev(clk[dummy], NULL, "20c0000.wdog");
> > > > +	clk_register_clkdev(clk[ssi1], NULL, "2028000.ssi");
> > > 
> > > The ssi clock needs a general cleanup on all i.MX just like I cleaned up
> > > the other units. The SSI unit has at least a register clock and a baud
> > > clock. What the driver requests and enables is the register clock.
> > > 
> > > The baud clock is currently unused and is needed only for master mode
> > > (which is not implemented in the ssi driver)
> > Are you sure for that? If I don't enable clk[ssi1], the ssi will not
> > work.
> > 
> > > 
> > > So where we want to come to is:
> > > 
> > > 	clk_register_clkdev(clk[ipg], "ipg", "2028000.ssi");
> > ssi don't have ipg gate. We can let it always on for imx6q.
> 
> Can you please ask your IC guys for clarification?
> 
> For example on i.MX5 we have a ssi ipg clock and a ssi serial clock.
> Both can be gated with two individual gate bits.
> 
> The i.MX6 datasheet (and also several other i.MX datasheets) is quite
> nebulous. The i.MX6 has only one gate bit for each SSI unit, but
> it's not clear if this bit actually gates both the ipg and serial
> clock or only one of them.
You're right. ipg and serial clocks share the same gate. How do we
handle it? I think it's not the only one and won't be last one.
> 
> My general idea is that each unit in the SoC has different input clocks.
> In the driver we need to clk_get() the input clocks. If a given SoC has
> no software control over some of the devices input clocks, then we need
> to provide a dummy for this, because other SoCs have control over the
> clock.
Great.

Thanks
Richard
> 
> Sascha
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 09/11] ARM: imx6q: add ssi1 clk_lookup

[Sascha Hauer]

On Mon, Apr 30, 2012 at 10:01:46AM +0800, Richard Zhao wrote:
> > > > 
> > > > 	clk_register_clkdev(clk[ipg], "ipg", "2028000.ssi");
> > > ssi don't have ipg gate. We can let it always on for imx6q.
> > 
> > Can you please ask your IC guys for clarification?
> > 
> > For example on i.MX5 we have a ssi ipg clock and a ssi serial clock.
> > Both can be gated with two individual gate bits.
> > 
> > The i.MX6 datasheet (and also several other i.MX datasheets) is quite
> > nebulous. The i.MX6 has only one gate bit for each SSI unit, but
> > it's not clear if this bit actually gates both the ipg and serial
> > clock or only one of them.
> You're right. ipg and serial clocks share the same gate. How do we
> handle it? I think it's not the only one and won't be last one.

We don't have support for a single gate gating two clocks right now and
I don't know how this fits into the clock framework.
We could pretend that only the ipg clock is gateable because this is
needed anyway when the SSI unit is used.

It seems we need a proper solution for this later.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 09/11] ARM: imx6q: add ssi1 clk_lookup

[Richard Zhao]

On Mon, Apr 30, 2012 at 02:18:57PM +0200, Sascha Hauer wrote:
> On Mon, Apr 30, 2012 at 10:01:46AM +0800, Richard Zhao wrote:
> > > > > 
> > > > > 	clk_register_clkdev(clk[ipg], "ipg", "2028000.ssi");
> > > > ssi don't have ipg gate. We can let it always on for imx6q.
> > > 
> > > Can you please ask your IC guys for clarification?
> > > 
> > > For example on i.MX5 we have a ssi ipg clock and a ssi serial clock.
> > > Both can be gated with two individual gate bits.
> > > 
> > > The i.MX6 datasheet (and also several other i.MX datasheets) is quite
> > > nebulous. The i.MX6 has only one gate bit for each SSI unit, but
> > > it's not clear if this bit actually gates both the ipg and serial
> > > clock or only one of them.
> > You're right. ipg and serial clocks share the same gate. How do we
> > handle it? I think it's not the only one and won't be last one.
> 
> We don't have support for a single gate gating two clocks right now and
> I don't know how this fits into the clock framework.
> We could pretend that only the ipg clock is gateable because this is
> needed anyway when the SSI unit is used.
Shawn, will you add clk ssi_ipg?

Thanks
Richard
> 
> It seems we need a proper solution for this later.
> 
> Sascha
> 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 

Re: [PATCH 09/11] ARM: imx6q: add ssi1 clk_lookup

[Shawn Guo]

On Wed, May 02, 2012 at 06:34:41PM +0800, Richard Zhao wrote:
> Shawn, will you add clk ssi_ipg?
> 
Ok, will do after Sascha's clk series gets settled on arm-soc tree.

-- 
Regards,
Shawn

Re: [PATCH 09/11] ARM: imx6q: add ssi1 clk_lookup

[Sascha Hauer]

On Wed, May 02, 2012 at 10:34:17PM +0800, Shawn Guo wrote:
> On Wed, May 02, 2012 at 06:34:41PM +0800, Richard Zhao wrote:
> > Shawn, will you add clk ssi_ipg?
> > 
> Ok, will do after Sascha's clk series gets settled on arm-soc tree.

I have a branch based on Mikes clk-next as of today, I will send it
tomorrow.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 10/11] ARM: imx6q_sabrelite: clkdev_add cko1 for sgtl5000

[Richard Zhao]

Signed-off-by: Richard Zhao <richard.zhao-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 arch/arm/mach-imx/clk-imx6q.c  |    3 +++
 arch/arm/mach-imx/mach-imx6q.c |   28 ++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
index 9a03dcc..4ea0de0 100644
--- a/arch/arm/mach-imx/clk-imx6q.c
+++ b/arch/arm/mach-imx/clk-imx6q.c
@@ -419,6 +419,9 @@ int __init mx6q_clocks_init(void)
 	clk_register_clkdev(clk[dummy], NULL, "20bc000.wdog");
 	clk_register_clkdev(clk[dummy], NULL, "20c0000.wdog");
 	clk_register_clkdev(clk[ssi1], NULL, "2028000.ssi");
+	clk_register_clkdev(clk[cko1_sel], "cko1_sel", NULL);
+	clk_register_clkdev(clk[ahb], "ahb", NULL);
+	clk_register_clkdev(clk[cko1], "cko1", NULL);
 
 	for (i = 0; i < ARRAY_SIZE(clks_init_on); i++) {
 		c = clk_get_sys(clks_init_on[i], NULL);
diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index d25c5d8..e9b2522 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -10,6 +10,8 @@
  * http://www.gnu.org/copyleft/gpl.html
  */
 
+#include <linux/clkdev.h>
+#include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/init.h>
 #include <linux/io.h>
@@ -75,10 +77,36 @@ static int ksz9021rn_phy_fixup(struct phy_device *phydev)
 	return 0;
 }
 
+static void __init imx6q_cko1_setup(void)
+{
+	struct clk *cko1_sel, *ahb, *cko1;
+	unsigned long rate;
+
+	cko1_sel = clk_get_sys(NULL, "cko1_sel");
+	ahb = clk_get_sys(NULL, "ahb");
+	cko1 = clk_get_sys(NULL, "cko1");
+	if (IS_ERR(cko1_sel) || IS_ERR(ahb) || IS_ERR(cko1)) {
+		printk(KERN_ERR "cko1 setup failed!\n");
+		goto put_clk;
+	}
+	clk_set_parent(cko1_sel, ahb);
+	rate = clk_round_rate(cko1, 16000000);
+	clk_set_rate(cko1, rate);
+	clk_register_clkdev(cko1, NULL, "0-000a");
+put_clk:
+	if (!IS_ERR(cko1_sel))
+		clk_put(cko1_sel);
+	if (!IS_ERR(ahb))
+		clk_put(ahb);
+	if (!IS_ERR(cko1))
+		clk_put(cko1);
+}
+
 static void __init imx6q_sabrelite_init(void)
 {
 	phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
 				ksz9021rn_phy_fixup);
+	imx6q_cko1_setup();
 }
 
 static void __init imx6q_init_machine(void)
-- 
1.7.5.4

Re: [PATCH 10/11] ARM: imx6q_sabrelite: clkdev_add cko1 for sgtl5000

[Shawn Guo]

On Fri, Apr 27, 2012 at 03:03:04PM +0800, Richard Zhao wrote:
> Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
> ---
>  arch/arm/mach-imx/clk-imx6q.c  |    3 +++
>  arch/arm/mach-imx/mach-imx6q.c |   28 ++++++++++++++++++++++++++++
>  2 files changed, 31 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
> index 9a03dcc..4ea0de0 100644
> --- a/arch/arm/mach-imx/clk-imx6q.c
> +++ b/arch/arm/mach-imx/clk-imx6q.c
> @@ -419,6 +419,9 @@ int __init mx6q_clocks_init(void)
>  	clk_register_clkdev(clk[dummy], NULL, "20bc000.wdog");
>  	clk_register_clkdev(clk[dummy], NULL, "20c0000.wdog");
>  	clk_register_clkdev(clk[ssi1], NULL, "2028000.ssi");
> +	clk_register_clkdev(clk[cko1_sel], "cko1_sel", NULL);
> +	clk_register_clkdev(clk[ahb], "ahb", NULL);
> +	clk_register_clkdev(clk[cko1], "cko1", NULL);
>  
>  	for (i = 0; i < ARRAY_SIZE(clks_init_on); i++) {
>  		c = clk_get_sys(clks_init_on[i], NULL);
> diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
> index d25c5d8..e9b2522 100644
> --- a/arch/arm/mach-imx/mach-imx6q.c
> +++ b/arch/arm/mach-imx/mach-imx6q.c
> @@ -10,6 +10,8 @@
>   * http://www.gnu.org/copyleft/gpl.html
>   */
>  
> +#include <linux/clkdev.h>
> +#include <linux/clk.h>

Nit: I would have <linux/clk.h> put before <linux/clkdev.h>.

>  #include <linux/delay.h>
>  #include <linux/init.h>
>  #include <linux/io.h>
> @@ -75,10 +77,36 @@ static int ksz9021rn_phy_fixup(struct phy_device *phydev)
>  	return 0;
>  }
>  
> +static void __init imx6q_cko1_setup(void)

I would say this is a sabrelite specific setup so far.

> +{
> +	struct clk *cko1_sel, *ahb, *cko1;
> +	unsigned long rate;
> +
> +	cko1_sel = clk_get_sys(NULL, "cko1_sel");
> +	ahb = clk_get_sys(NULL, "ahb");
> +	cko1 = clk_get_sys(NULL, "cko1");
> +	if (IS_ERR(cko1_sel) || IS_ERR(ahb) || IS_ERR(cko1)) {
> +		printk(KERN_ERR "cko1 setup failed!\n");

pr_err

> +		goto put_clk;
> +	}
> +	clk_set_parent(cko1_sel, ahb);
> +	rate = clk_round_rate(cko1, 16000000);
> +	clk_set_rate(cko1, rate);
> +	clk_register_clkdev(cko1, NULL, "0-000a");

This dev_id looks a little strange to me.  I understand that's the
consequence of having sgtl5000 clock managed in ASoC machine driver
imx-sgtl5000.  IMO, having sgtl5000 driver manages its clock and
looking up the clock with dev_id simply as "sgtl5000" makes more
sense to me.  Will put more comment on your imx-sgtl5000 clock patch.

Regards,
Shawn

> +put_clk:
> +	if (!IS_ERR(cko1_sel))
> +		clk_put(cko1_sel);
> +	if (!IS_ERR(ahb))
> +		clk_put(ahb);
> +	if (!IS_ERR(cko1))
> +		clk_put(cko1);
> +}
> +
>  static void __init imx6q_sabrelite_init(void)
>  {
>  	phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
>  				ksz9021rn_phy_fixup);
> +	imx6q_cko1_setup();
>  }
>  
>  static void __init imx6q_init_machine(void)
> -- 
> 1.7.5.4
> 
> 

Re: [PATCH 10/11] ARM: imx6q_sabrelite: clkdev_add cko1 for sgtl5000

[Shawn Guo]

On Tue, May 01, 2012 at 08:47:10PM +0800, Shawn Guo wrote:
> > +	clk_register_clkdev(cko1, NULL, "0-000a");
> 
> This dev_id looks a little strange to me.  I understand that's the
> consequence of having sgtl5000 clock managed in ASoC machine driver
> imx-sgtl5000.
> 
Just realised it's not the case.  But shouldn't the name look like
"sgtl5000.0-000a"?

-- 
Regards,
Shawn

Re: [alsa-devel] [PATCH 10/11] ARM: imx6q_sabrelite: clkdev_add cko1 for sgtl5000

[Fabio Estevam]

On Tue, May 1, 2012 at 8:39 PM, Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Tue, May 01, 2012 at 08:47:10PM +0800, Shawn Guo wrote:
>> > +   clk_register_clkdev(cko1, NULL, "0-000a");
>>
>> This dev_id looks a little strange to me.  I understand that's the
>> consequence of having sgtl5000 clock managed in ASoC machine driver
>> imx-sgtl5000.
>>
> Just realised it's not the case.  But shouldn't the name look like
> "sgtl5000.0-000a"?

Since this is common code, we should not hardcode the I2C bus as the
codec can be connected in a I2C bus different of 0.

Re: [alsa-devel] [PATCH 10/11] ARM: imx6q_sabrelite: clkdev_add cko1 for sgtl5000

[Shawn Guo]

On Tue, May 01, 2012 at 11:47:55PM -0300, Fabio Estevam wrote:
> On Tue, May 1, 2012 at 8:39 PM, Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > On Tue, May 01, 2012 at 08:47:10PM +0800, Shawn Guo wrote:
> >> > +   clk_register_clkdev(cko1, NULL, "0-000a");
> >>
> >> This dev_id looks a little strange to me.  I understand that's the
> >> consequence of having sgtl5000 clock managed in ASoC machine driver
> >> imx-sgtl5000.
> >>
> > Just realised it's not the case.  But shouldn't the name look like
> > "sgtl5000.0-000a"?
> 
> Since this is common code, we should not hardcode the I2C bus as the
> codec can be connected in a I2C bus different of 0.

No, it's not.  It's a sabrelite specific lookup which is done in
imx6q_sabrelite_init, while imx6q_cko1_setup needs a better naming,
as what I already commented.

In the end, this type of board specific clock lookup should really
go into device tree.

-- 
Regards,
Shawn

Re: [PATCH 10/11] ARM: imx6q_sabrelite: clkdev_add cko1 for sgtl5000

[Richard Zhao]

On Tue, May 01, 2012 at 08:47:10PM +0800, Shawn Guo wrote:
> On Fri, Apr 27, 2012 at 03:03:04PM +0800, Richard Zhao wrote:
> > Signed-off-by: Richard Zhao <richard.zhao-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > ---
> >  arch/arm/mach-imx/clk-imx6q.c  |    3 +++
> >  arch/arm/mach-imx/mach-imx6q.c |   28 ++++++++++++++++++++++++++++
> >  2 files changed, 31 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
> > index 9a03dcc..4ea0de0 100644
> > --- a/arch/arm/mach-imx/clk-imx6q.c
> > +++ b/arch/arm/mach-imx/clk-imx6q.c
> > @@ -419,6 +419,9 @@ int __init mx6q_clocks_init(void)
> >  	clk_register_clkdev(clk[dummy], NULL, "20bc000.wdog");
> >  	clk_register_clkdev(clk[dummy], NULL, "20c0000.wdog");
> >  	clk_register_clkdev(clk[ssi1], NULL, "2028000.ssi");
> > +	clk_register_clkdev(clk[cko1_sel], "cko1_sel", NULL);
> > +	clk_register_clkdev(clk[ahb], "ahb", NULL);
> > +	clk_register_clkdev(clk[cko1], "cko1", NULL);
> >  
> >  	for (i = 0; i < ARRAY_SIZE(clks_init_on); i++) {
> >  		c = clk_get_sys(clks_init_on[i], NULL);
> > diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
> > index d25c5d8..e9b2522 100644
> > --- a/arch/arm/mach-imx/mach-imx6q.c
> > +++ b/arch/arm/mach-imx/mach-imx6q.c
> > @@ -10,6 +10,8 @@
> >   * http://www.gnu.org/copyleft/gpl.html
> >   */
> >  
> > +#include <linux/clkdev.h>
> > +#include <linux/clk.h>
> 
> Nit: I would have <linux/clk.h> put before <linux/clkdev.h>.
ok
> 
> >  #include <linux/delay.h>
> >  #include <linux/init.h>
> >  #include <linux/io.h>
> > @@ -75,10 +77,36 @@ static int ksz9021rn_phy_fixup(struct phy_device *phydev)
> >  	return 0;
> >  }
> >  
> > +static void __init imx6q_cko1_setup(void)
> 
> I would say this is a sabrelite specific setup so far.
ok
> 
> > +{
> > +	struct clk *cko1_sel, *ahb, *cko1;
> > +	unsigned long rate;
> > +
> > +	cko1_sel = clk_get_sys(NULL, "cko1_sel");
> > +	ahb = clk_get_sys(NULL, "ahb");
> > +	cko1 = clk_get_sys(NULL, "cko1");
> > +	if (IS_ERR(cko1_sel) || IS_ERR(ahb) || IS_ERR(cko1)) {
> > +		printk(KERN_ERR "cko1 setup failed!\n");
> 
> pr_err
ok

Thanks
Richard
> 
> > +		goto put_clk;
> > +	}
> > +	clk_set_parent(cko1_sel, ahb);
> > +	rate = clk_round_rate(cko1, 16000000);
> > +	clk_set_rate(cko1, rate);
> > +	clk_register_clkdev(cko1, NULL, "0-000a");
> 
> This dev_id looks a little strange to me.  I understand that's the
> consequence of having sgtl5000 clock managed in ASoC machine driver
> imx-sgtl5000.  IMO, having sgtl5000 driver manages its clock and
> looking up the clock with dev_id simply as "sgtl5000" makes more
> sense to me.  Will put more comment on your imx-sgtl5000 clock patch.
> 
> Regards,
> Shawn
> 
> > +put_clk:
> > +	if (!IS_ERR(cko1_sel))
> > +		clk_put(cko1_sel);
> > +	if (!IS_ERR(ahb))
> > +		clk_put(ahb);
> > +	if (!IS_ERR(cko1))
> > +		clk_put(cko1);
> > +}
> > +
> >  static void __init imx6q_sabrelite_init(void)
> >  {
> >  	phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
> >  				ksz9021rn_phy_fixup);
> > +	imx6q_cko1_setup();
> >  }
> >  
> >  static void __init imx6q_init_machine(void)
> > -- 
> > 1.7.5.4
> > 
> > 
> 

[PATCH 11/11] ARM: dts: imx6q-sabrelite: add sound device imx6q-sabrelite-sgtl5000

[Richard Zhao]

From: Richard Zhao <richard.zhao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Signed-off-by: Richard Zhao <richard.zhao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Richard Zhao <richard.zhao-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 arch/arm/boot/dts/imx6q-sabrelite.dts |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts
index cdae2dd..20aa767 100644
--- a/arch/arm/boot/dts/imx6q-sabrelite.dts
+++ b/arch/arm/boot/dts/imx6q-sabrelite.dts
@@ -97,4 +97,17 @@
 			regulator-always-on;
 		};
 	};
+
+	sound {
+		compatible = "fsl,imx6q-sabrelite-sgtl5000",
+			     "fsl,imx-audio-sgtl5000";
+		model = "imx6q-sabrelite-sgtl5000";
+		ssi-controller = <&ssi1>;
+		audio-codec = <&codec>;
+		audio-routing =
+			"MIC_IN", "Mic Jack",
+			"Headphone Jack", "HP_OUT";
+		mux-int-port = <1>;
+		mux-ext-port = <4>;
+	};
 };
-- 
1.7.5.4

Re: [PATCH 11/11] ARM: dts: imx6q-sabrelite: add sound device imx6q-sabrelite-sgtl5000

[Shawn Guo]

On Fri, Apr 27, 2012 at 03:03:05PM +0800, Richard Zhao wrote:
> From: Richard Zhao <richard.zhao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> 
> Signed-off-by: Richard Zhao <richard.zhao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Richard Zhao <richard.zhao-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> ---
>  arch/arm/boot/dts/imx6q-sabrelite.dts |   13 +++++++++++++
>  1 files changed, 13 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts
> index cdae2dd..20aa767 100644
> --- a/arch/arm/boot/dts/imx6q-sabrelite.dts
> +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts
> @@ -97,4 +97,17 @@
>  			regulator-always-on;
>  		};
>  	};
> +
> +	sound {
> +		compatible = "fsl,imx6q-sabrelite-sgtl5000",
> +			     "fsl,imx-audio-sgtl5000";
> +		model = "imx6q-sabrelite-sgtl5000";
> +		ssi-controller = <&ssi1>;
> +		audio-codec = <&codec>;
> +		audio-routing =
> +			"MIC_IN", "Mic Jack",

Just curious if you have capture working without enabling Mic Bias
power supplier.

Regards,
Shawn

> +			"Headphone Jack", "HP_OUT";
> +		mux-int-port = <1>;
> +		mux-ext-port = <4>;
> +	};
>  };
> -- 
> 1.7.5.4
> 
> 

Re: [PATCH 11/11] ARM: dts: imx6q-sabrelite: add sound device imx6q-sabrelite-sgtl5000

[Richard Zhao]

On Tue, May 01, 2012 at 08:55:14PM +0800, Shawn Guo wrote:
> On Fri, Apr 27, 2012 at 03:03:05PM +0800, Richard Zhao wrote:
> > From: Richard Zhao <richard.zhao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > 
> > Signed-off-by: Richard Zhao <richard.zhao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > Signed-off-by: Richard Zhao <richard.zhao-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> > ---
> >  arch/arm/boot/dts/imx6q-sabrelite.dts |   13 +++++++++++++
> >  1 files changed, 13 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts
> > index cdae2dd..20aa767 100644
> > --- a/arch/arm/boot/dts/imx6q-sabrelite.dts
> > +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts
> > @@ -97,4 +97,17 @@
> >  			regulator-always-on;
> >  		};
> >  	};
> > +
> > +	sound {
> > +		compatible = "fsl,imx6q-sabrelite-sgtl5000",
> > +			     "fsl,imx-audio-sgtl5000";
> > +		model = "imx6q-sabrelite-sgtl5000";
> > +		ssi-controller = <&ssi1>;
> > +		audio-codec = <&codec>;
> > +		audio-routing =
> > +			"MIC_IN", "Mic Jack",
> 
> Just curious if you have capture working without enabling Mic Bias
> power supplier.
I should add a "Mic Jack", "Mic Bias". When arecord, mic_bias_event
is called.
It's strange that it can not record any voice. debugging ...

Thanks
Richard
> 
> Regards,
> Shawn
> 
> > +			"Headphone Jack", "HP_OUT";
> > +		mux-int-port = <1>;
> > +		mux-ext-port = <4>;
> > +	};
> >  };
> > -- 
> > 1.7.5.4
> > 
> > 
> 

[PATCH 00/11] enable imx6q_sabrelite sgtl5000 audio support

[Richard Zhao]

It's based on Dong Aisheng's early pinctrl patch and Sascha's v2
convert to common clk patch.

You can also get at:
https://github.com/riczhao/kernel-imx/tree/topics/audio

Richard Zhao (11):
  dma: imx-sdma: make channel0 operations atomic
  ASoC: imx-sgtl5000: add of_node_put when probe fail.
  ASoC: fsl: add sgtl5000 clock support for imx-sgtl5000
  i2c: imx: add pinctrl support
  ARM: imx6q: move imx6q_sabrelite specific code to a dedicated
    function
  ARM: dts: imx6q-sabrelite: add ssi device
  ARM: dts: imx6q-sabrelite: add audmux device
  ASoC: imx-audmux: add pinctrl support
  ARM: imx6q: add ssi1 clk_lookup
  ARM: imx6q_sabrelite: clkdev_add cko1 for sgtl5000
  ARM: dts: imx6q-sabrelite: add sound device imx6q-sabrelite-sgtl5000

 arch/arm/boot/dts/imx6q-sabrelite.dts |   30 ++++++++++++++++
 arch/arm/boot/dts/imx6q.dtsi          |   46 +++++++++++++++++++++++--
 arch/arm/mach-imx/clk-imx6q.c         |    4 ++
 arch/arm/mach-imx/mach-imx6q.c        |   37 +++++++++++++++++++-
 drivers/dma/imx-sdma.c                |   57 ++++++++++++++++++-------------
 drivers/i2c/busses/i2c-imx.c          |    9 +++++
 sound/soc/fsl/imx-audmux.c            |    8 ++++
 sound/soc/fsl/imx-sgtl5000.c          |   61 +++++++++++++++++++++++++--------
 8 files changed, 208 insertions(+), 44 deletions(-)

Thanks
Richard
-- 
1.7.5.4

[PATCH 03/11] ASoC: fsl: add sgtl5000 clock support for imx-sgtl5000

[Richard Zhao]

From: Richard Zhao <richard.zhao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

It tries to clk_get the clock. And if it failed, it assumes the clock
by default enabled.

Signed-off-by: Richard Zhao <richard.zhao-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
 sound/soc/fsl/imx-sgtl5000.c |   40 ++++++++++++++++++++++++++++++++--------
 1 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/sound/soc/fsl/imx-sgtl5000.c b/sound/soc/fsl/imx-sgtl5000.c
index 73b935e..3a729ca 100644
--- a/sound/soc/fsl/imx-sgtl5000.c
+++ b/sound/soc/fsl/imx-sgtl5000.c
@@ -13,6 +13,8 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
+#include <linux/of_i2c.h>
+#include <linux/clk.h>
 #include <sound/soc.h>
 
 #include "../codecs/sgtl5000.h"
@@ -25,6 +27,7 @@ struct imx_sgtl5000_data {
 	struct snd_soc_card card;
 	char codec_dai_name[DAI_NAME_SIZE];
 	char platform_name[DAI_NAME_SIZE];
+	struct clk *codec_clk;
 	unsigned int clk_frequency;
 };
 
@@ -58,6 +61,7 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	struct device_node *ssi_np, *codec_np;
 	struct platform_device *ssi_pdev;
+	struct i2c_client *codec_dev;
 	struct imx_sgtl5000_data *data;
 	int int_port, ext_port;
 	int ret;
@@ -113,6 +117,11 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev)
 		ret = -EINVAL;
 		goto fail;
 	}
+	codec_dev = of_find_i2c_device_by_node(codec_np);
+	if (!codec_dev) {
+		dev_err(&pdev->dev, "failed to find codec platform device\n");
+		return -EINVAL;
+	}
 
 	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
 	if (!data) {
@@ -120,11 +129,20 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev)
 		goto fail;
 	}
 
-	ret = of_property_read_u32(codec_np, "clock-frequency",
-				   &data->clk_frequency);
-	if (ret) {
-		dev_err(&pdev->dev, "clock-frequency missing or invalid\n");
-		goto fail;
+	data->codec_clk = clk_get(&codec_dev->dev, NULL);
+	if (IS_ERR(data->codec_clk)) {
+		/* assuming clock enabled by default */
+		data->codec_clk = NULL;
+		ret = of_property_read_u32(codec_np, "clock-frequency",
+					&data->clk_frequency);
+		if (ret) {
+			dev_err(&codec_dev->dev,
+				"clock-frequency missing or invalid\n");
+			goto fail;
+		}
+	} else {
+		data->clk_frequency = clk_get_rate(data->codec_clk);
+		clk_prepare_enable(data->codec_clk);
 	}
 
 	data->dai.name = "HiFi";
@@ -140,10 +158,10 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev)
 	data->card.dev = &pdev->dev;
 	ret = snd_soc_of_parse_card_name(&data->card, "model");
 	if (ret)
-		goto fail;
+		goto clk_fail;
 	ret = snd_soc_of_parse_audio_routing(&data->card, "audio-routing");
 	if (ret)
-		goto fail;
+		goto clk_fail;
 	data->card.num_links = 1;
 	data->card.dai_link = &data->dai;
 	data->card.dapm_widgets = imx_sgtl5000_dapm_widgets;
@@ -152,10 +170,12 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev)
 	ret = snd_soc_register_card(&data->card);
 	if (ret) {
 		dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n", ret);
-		goto fail;
+		goto clk_fail;
 	}
 
 	platform_set_drvdata(pdev, data);
+clk_fail:
+	clk_put(data->codec_clk);
 fail:
 	if (ssi_np)
 		of_node_put(ssi_np);
@@ -169,6 +189,10 @@ static int __devexit imx_sgtl5000_remove(struct platform_device *pdev)
 {
 	struct imx_sgtl5000_data *data = platform_get_drvdata(pdev);
 
+	if (data->codec_clk) {
+		clk_disable_unprepare(data->codec_clk);
+		clk_put(data->codec_clk);
+	}
 	snd_soc_unregister_card(&data->card);
 
 	return 0;
-- 
1.7.5.4