ARM: mach-imx6q: Enable the codec clock earlier

PATCH New Fabio Estevam 4 participants 8 messages Jun 7, 2013 – Jun 10, 2013

Subsystems (6)

Arm (32-bit) Sound Arm sub-architectures Arm i.MX and Layerscape +2 more subsystems

lore

Share & tools

lore

Fabio Estevam

author maintainer

2013-06-07 22:07 UTC #0 lore raw

PATCH 1/1 ARM: mach-imx6q: Enable the codec clock earlier

From: Fabio Estevam <[email protected]>

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 <[email protected]>

---
This issue is seen on linux-next tree

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

arch/arm/mach-imx/mach-imx6q.c +4 -0

index 045e5e3..034f4d2 100644
@@ -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);

Signature · 1 line

1.8.1.2

Russell King - ARM Linux

2013-06-07 22:20 UTC #1 lore raw

PATCH 1/1 ARM: mach-imx6q: Enable the codec clock earlier

Quoted 5 lines · Fabio Estevam

From: Fabio Estevam <[email protected]>
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.

Quoted 8 lines

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?

Fabio Estevam

author maintainer

2013-06-07 23:16 UTC #2 lore raw

PATCH 1/1 ARM: mach-imx6q: Enable the codec clock earlier

Hi Russell,

Quoted 2 lines · Russell King - ARM Linux

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:

sound/soc/codecs/sgtl5000.c +1 -1

@@ -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

Troy Kisky

2013-06-07 23:43 UTC #3 lore raw

PATCH 1/1 ARM: mach-imx6q: Enable the codec clock earlier

Quoted 36 lines · Fabio Estevam

Hi Russell,
Quoted 2 lines · Russell King - ARM Linux
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:
sound/soc/codecs/sgtl5000.c +1 -1
@@ -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) {

Quoted 8 lines

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

Fabio Estevam

author maintainer

2013-06-07 23:49 UTC #4 lore raw

PATCH 1/1 ARM: mach-imx6q: Enable the codec clock earlier

Hi Troy,

Quoted 2 lines · Troy Kisky

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.

Quoted 2 lines

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

Troy Kisky

2013-06-07 23:49 UTC #5 lore raw

PATCH 1/1 ARM: mach-imx6q: Enable the codec clock earlier

In reply to Troy Kisky

Quoted 50 lines · Troy Kisky

Quoted 37 lines · Fabio Estevam
Hi Russell,
Quoted 2 lines · Russell King - ARM Linux
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:
sound/soc/codecs/sgtl5000.c +1 -1
@@ -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) {
Quoted 8 lines
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.

Quoted 5 lines

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

Mark Brown

maintainer

2013-06-10 09:10 UTC #6 lore raw

PATCH 1/1 ARM: mach-imx6q: Enable the codec clock earlier

Quoted 2 lines · Troy Kisky

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>

Mark Brown

maintainer

2013-06-10 09:07 UTC #7 lore raw

PATCH 1/1 ARM: mach-imx6q: Enable the codec clock earlier

In reply to Troy Kisky

Quoted 2 lines · Troy Kisky

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>