[PATCH] ARM: mach-imx6q: Enable the codec clock earlier

[PATCH] ARM: mach-imx6q: Enable the codec clock earlier

[Fabio Estevam]

From: Fabio Estevam <fabio.estevam@freescale.com>

In order sgtl5000 driver to probe successfully, we need to read its ID via I2C 
,which requires that MCLK is driven prior to the I2C access.

Otherwise we get the following probe error:

sgtl5000: probe of 0-000a failed with error -5
imx-sgtl5000 sound.13: ASoC: CODEC (null) not registered
imx-sgtl5000 sound.13: snd_soc_register_card failed (-517)
platform sound.13: Driver imx-sgtl5000 requests probe deferral

Turn on MCLK sooner so that the probe can succeed.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
This issue is seen on linux-next tree

 arch/arm/mach-imx/mach-imx6q.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index 045e5e3..034f4d2 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -118,6 +118,7 @@ static void __init imx6q_sabrelite_cko1_setup(void)
 {
 	struct clk *cko1_sel, *ahb, *cko1;
 	unsigned long rate;
+	int ret;
 
 	cko1_sel = clk_get_sys(NULL, "cko1_sel");
 	ahb = clk_get_sys(NULL, "ahb");
@@ -129,6 +130,9 @@ static void __init imx6q_sabrelite_cko1_setup(void)
 	clk_set_parent(cko1_sel, ahb);
 	rate = clk_round_rate(cko1, 16000000);
 	clk_set_rate(cko1, rate);
+	ret = clk_prepare_enable(cko1);
+	if (ret)
+		pr_err("enabling clko1 failed: %d", ret);
 put_clk:
 	if (!IS_ERR(cko1_sel))
 		clk_put(cko1_sel);
-- 
1.8.1.2

[PATCH] ARM: mach-imx6q: Enable the codec clock earlier

[Russell King - ARM Linux]

On Fri, Jun 07, 2013 at 07:07:50PM -0300, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> In order sgtl5000 driver to probe successfully, we need to read its ID via I2C 
> ,which requires that MCLK is driven prior to the I2C access.
> 
> Otherwise we get the following probe error:
> 
> sgtl5000: probe of 0-000a failed with error -5
> imx-sgtl5000 sound.13: ASoC: CODEC (null) not registered
> imx-sgtl5000 sound.13: snd_soc_register_card failed (-517)
> platform sound.13: Driver imx-sgtl5000 requests probe deferral
> 
> Turn on MCLK sooner so that the probe can succeed.

Umm... doesn't that mean that imx-sgtl5000 should be getting this clock
and turning it on itself, and disabling it on device removal?

[PATCH] ARM: mach-imx6q: Enable the codec clock earlier

[Fabio Estevam]

Hi Russell,

On Fri, Jun 7, 2013 at 7:20 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:

> Umm... doesn't that mean that imx-sgtl5000 should be getting this clock
> and turning it on itself, and disabling it on device removal?

Yes, you are right.

Actually imx-sgtl500 does get this clock and turn it on itself.

The problem is that imx-sgtl5000 is getting called after the codec
sgtl500 codec is probed.

***** reading codec ID
sgtl5000: probe of 0-000a failed with error -5
***** enabling the codec clock
imx-sgtl5000 sound.13: ASoC: CODEC (null) not registered
imx-sgtl5000 sound.13: snd_soc_register_card failed (-517)
platform sound.13: Driver imx-sgtl5000 requests probe deferral


And then If I do:

--- a/sound/soc/codecs/sgtl5000.c
+++ b/sound/soc/codecs/sgtl5000.c
@@ -1525,7 +1525,7 @@ static int sgtl5000_i2c_probe(struct i2c_client *client,
        /* read chip information */
        ret = regmap_read(sgtl5000->regmap, SGTL5000_CHIP_ID, &reg);
        if (ret)
-               return ret;
+               return -EPROBE_DEFER;

        if (((reg & SGTL5000_PARTID_MASK) >> SGTL5000_PARTID_SHIFT) !=
            SGTL5000_PARTID_PART_ID) {

Then the probe is succesful on the second attemp.

I will post this to the alsa-devel list.

Thanks,

Fabio Estevam

[PATCH] ARM: mach-imx6q: Enable the codec clock earlier

[Troy Kisky]

On 6/7/2013 4:16 PM, Fabio Estevam wrote:
> Hi Russell,
>
> On Fri, Jun 7, 2013 at 7:20 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>
>> Umm... doesn't that mean that imx-sgtl5000 should be getting this clock
>> and turning it on itself, and disabling it on device removal?
> Yes, you are right.
>
> Actually imx-sgtl500 does get this clock and turn it on itself.
>
> The problem is that imx-sgtl5000 is getting called after the codec
> sgtl500 codec is probed.
>
> ***** reading codec ID
> sgtl5000: probe of 0-000a failed with error -5
> ***** enabling the codec clock
> imx-sgtl5000 sound.13: ASoC: CODEC (null) not registered
> imx-sgtl5000 sound.13: snd_soc_register_card failed (-517)
> platform sound.13: Driver imx-sgtl5000 requests probe deferral
>
>
> And then If I do:
>
> --- a/sound/soc/codecs/sgtl5000.c
> +++ b/sound/soc/codecs/sgtl5000.c
> @@ -1525,7 +1525,7 @@ static int sgtl5000_i2c_probe(struct i2c_client *client,
>          /* read chip information */
>          ret = regmap_read(sgtl5000->regmap, SGTL5000_CHIP_ID, &reg);
>          if (ret)
> -               return ret;
> +               return -EPROBE_DEFER;
>
>          if (((reg & SGTL5000_PARTID_MASK) >> SGTL5000_PARTID_SHIFT) !=
>              SGTL5000_PARTID_PART_ID) {
>
> Then the probe is succesful on the second attemp.
>
> I will post this to the alsa-devel list.
>
> Thanks,
>
> Fabio Estevam
>

Perhaps a delay is needed after the clock is turned on, to when you can 
read a register.

Using -EPROBE_DEFER seems like an abuse. Shouldn't it turn the clock 
back off before it returns anyway?

Troy

[PATCH] ARM: mach-imx6q: Enable the codec clock earlier

[Fabio Estevam]

Hi Troy,

On Fri, Jun 7, 2013 at 8:43 PM, Troy Kisky
<troy.kisky@boundarydevices.com> wrote:

> Perhaps a delay is needed after the clock is turned on, to when you can read
> a register.

The problem is that in linux-next the reading of the codec happens
before the clock is turned on and I was not able to understand yet why
this is happening .

On Linus' tree the codec reading only happens after the codec clock is on.

> Using -EPROBE_DEFER seems like an abuse. Shouldn't it turn the clock back
> off before it returns anyway?

That was done just for testing purpose.

Regards,

Fabio Estevam

[PATCH] ARM: mach-imx6q: Enable the codec clock earlier

[Troy Kisky]

On 6/7/2013 4:43 PM, Troy Kisky wrote:
> On 6/7/2013 4:16 PM, Fabio Estevam wrote:
>> Hi Russell,
>>
>> On Fri, Jun 7, 2013 at 7:20 PM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>>
>>> Umm... doesn't that mean that imx-sgtl5000 should be getting this clock
>>> and turning it on itself, and disabling it on device removal?
>> Yes, you are right.
>>
>> Actually imx-sgtl500 does get this clock and turn it on itself.
>>
>> The problem is that imx-sgtl5000 is getting called after the codec
>> sgtl500 codec is probed.
>>
>> ***** reading codec ID
>> sgtl5000: probe of 0-000a failed with error -5
>> ***** enabling the codec clock
>> imx-sgtl5000 sound.13: ASoC: CODEC (null) not registered
>> imx-sgtl5000 sound.13: snd_soc_register_card failed (-517)
>> platform sound.13: Driver imx-sgtl5000 requests probe deferral
>>
>>
>> And then If I do:
>>
>> --- a/sound/soc/codecs/sgtl5000.c
>> +++ b/sound/soc/codecs/sgtl5000.c
>> @@ -1525,7 +1525,7 @@ static int sgtl5000_i2c_probe(struct i2c_client 
>> *client,
>>          /* read chip information */
>>          ret = regmap_read(sgtl5000->regmap, SGTL5000_CHIP_ID, &reg);
>>          if (ret)
>> -               return ret;
>> +               return -EPROBE_DEFER;
>>
>>          if (((reg & SGTL5000_PARTID_MASK) >> SGTL5000_PARTID_SHIFT) !=
>>              SGTL5000_PARTID_PART_ID) {
>>
>> Then the probe is succesful on the second attemp.
>>
>> I will post this to the alsa-devel list.
>>
>> Thanks,
>>
>> Fabio Estevam
>>
>
> Perhaps a delay is needed after the clock is turned on, to when you 
> can read a register.
>
> Using -EPROBE_DEFER seems like an abuse. Shouldn't it turn the clock 
> back off before it returns anyway?
>
> Troy
>
Sorry, I read that wrong.  Maybe sgtl5000 needs to get the clock, as 
well as imx-sgtl5000.


Troy

[PATCH] ARM: mach-imx6q: Enable the codec clock earlier

[Mark Brown]

On Fri, Jun 07, 2013 at 04:49:17PM -0700, Troy Kisky wrote:

> Sorry, I read that wrong.  Maybe sgtl5000 needs to get the clock, as
> well as imx-sgtl5000.

Yes, in general we should be able to use the clock API more but it's not
present on most platforms so it's hard to use in generic code like CODEC
drivers.  This is why I keep pushing to get the generic clock API
enabled on all platforms (which unfortunately nobody seems really
interested in).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130610/bb0fbb12/attachment.sig>

[PATCH] ARM: mach-imx6q: Enable the codec clock earlier

[Mark Brown]

On Fri, Jun 07, 2013 at 04:43:12PM -0700, Troy Kisky wrote:

> Using -EPROBE_DEFER seems like an abuse. Shouldn't it turn the clock
> back off before it returns anyway?

Yes.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130610/f2a6b416/attachment.sig>