TWL6040 fails to initialize

TWL6040 fails to initialize

[Florian Vaussard]

Hi Peter,

I got recently to work on the DT support for an OMAP4 board [1], and I
encountered some troubles with the probe of the twl6040 audio codec on
3.14-rc kernels. On 3.13, things were working correctly. So I somewhat
managed to bisect this down to [c7f9129 mfd: twl6040: reg_defaults
support for regmap]. But looking more into it, things are less obvious.

When the init fails, here is what happens:

[    2.455749] twl6040 0-004b: Looking up vio-supply from device tree
[    2.456359] twl6040 0-004b: Looking up v2v1-supply from device tree
[    2.457061] twl6040 0-004b: Failed to set masks in 0x4: -121
[    2.463043] twl6040: probe of 0-004b failed with error -121

logically resulting in

[    2.770050] omap-abe-twl6040 sound.4: ASoC: CODEC twl6040-codec not
registered
[    2.777770] omap-abe-twl6040 sound.4: snd_soc_register_card() failed:
-517
[    2.785095] platform sound.4: Driver omap-abe-twl6040 requests probe
deferral

We get a EREMOTEIO when calling regmap_add_irq_chip() from
twl6040_probe(). regmap_add_irq_chip() will try to perform non-cached
i2c writes, where the omap-i2c driver get a NACK from the remote chip.
Strange enough, the non-cached read just before (i.e.
twl6040_reg_read(twl6040, TWL6040_REG_ASICREV)) succeeds.

After some fiddling around, I could find that delaying the call to
regmap_add_irq_chip(), let's say by msleep(1), will "solve" the problem.

As the TRM for TWL6040 is not public, and I cannot physically access the
i2c bus on the board, I ran out of hypothesis. I first suspected a
concurrent access from the McPDM bus, but the mcpdm driver does not seem
to perform command (write) access.

As the regulators are enabled just before, could it be the twl6040 IP
which is not yet initialized, and NACK'ing writes but not reads? The
commit c7f9129 changed the regmap logic by reducing the number of
non-cached accesses, and thus changed the access timings, so it may have
trigged this behaviour. But this is pure supposition, as I cannot hack
the i2c bus :( And adding any kind of tracing before
regmap_add_irq_chip() usually delays enough to make the probe succeeds.

Any ideas?

Regards,
Florian

[1] http://thread.gmane.org/gmane.linux.ports.arm.omap/110801

TWL6040 fails to initialize

[Florian Vaussard]

Hi Peter,

I got recently to work on the DT support for an OMAP4 board [1], and I
encountered some troubles with the probe of the twl6040 audio codec on
3.14-rc kernels. On 3.13, things were working correctly. So I somewhat
managed to bisect this down to [c7f9129 mfd: twl6040: reg_defaults
support for regmap]. But looking more into it, things are less obvious.

When the init fails, here is what happens:

[    2.455749] twl6040 0-004b: Looking up vio-supply from device tree
[    2.456359] twl6040 0-004b: Looking up v2v1-supply from device tree
[    2.457061] twl6040 0-004b: Failed to set masks in 0x4: -121
[    2.463043] twl6040: probe of 0-004b failed with error -121

logically resulting in

[    2.770050] omap-abe-twl6040 sound.4: ASoC: CODEC twl6040-codec not
registered
[    2.777770] omap-abe-twl6040 sound.4: snd_soc_register_card() failed:
-517
[    2.785095] platform sound.4: Driver omap-abe-twl6040 requests probe
deferral

We get a EREMOTEIO when calling regmap_add_irq_chip() from
twl6040_probe(). regmap_add_irq_chip() will try to perform non-cached
i2c writes, where the omap-i2c driver get a NACK from the remote chip.
Strange enough, the non-cached read just before (i.e.
twl6040_reg_read(twl6040, TWL6040_REG_ASICREV)) succeeds.

After some fiddling around, I could find that delaying the call to
regmap_add_irq_chip(), let's say by msleep(1), will "solve" the problem.

As the TRM for TWL6040 is not public, and I cannot physically access the
i2c bus on the board, I ran out of hypothesis. I first suspected a
concurrent access from the McPDM bus, but the mcpdm driver does not seem
to perform command (write) access.

As the regulators are enabled just before, could it be the twl6040 IP
which is not yet initialized, and NACK'ing writes but not reads? The
commit c7f9129 changed the regmap logic by reducing the number of
non-cached accesses, and thus changed the access timings, so it may have
trigged this behaviour. But this is pure supposition, as I cannot hack
the i2c bus :( And adding any kind of tracing before
regmap_add_irq_chip() usually delays enough to make the probe succeeds.

Any ideas?

Regards,
Florian

[1] http://thread.gmane.org/gmane.linux.ports.arm.omap/110801

Re: TWL6040 fails to initialize

[Peter Ujfalusi]

Hi Florian,

On 02/25/2014 12:30 PM, Florian Vaussard wrote:
> Hi Peter,
> 
> I got recently to work on the DT support for an OMAP4 board [1], and I
> encountered some troubles with the probe of the twl6040 audio codec on
> 3.14-rc kernels. On 3.13, things were working correctly. So I somewhat
> managed to bisect this down to [c7f9129 mfd: twl6040: reg_defaults
> support for regmap]. But looking more into it, things are less obvious.

Interesting. I just booted 3.14.0-rc4 with HEAD:
7472e009a3f1 Merge branch 'for-linus' of
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security

on my PandaBoard-ES and audio comes up just fine (twl6040).

> 
> When the init fails, here is what happens:
> 
> [    2.455749] twl6040 0-004b: Looking up vio-supply from device tree
> [    2.456359] twl6040 0-004b: Looking up v2v1-supply from device tree
> [    2.457061] twl6040 0-004b: Failed to set masks in 0x4: -121
> [    2.463043] twl6040: probe of 0-004b failed with error -121
> 
> logically resulting in

Do you have the regulators in your dts file? vio and v2v1 is needed by the
twl6040.

> 
> [    2.770050] omap-abe-twl6040 sound.4: ASoC: CODEC twl6040-codec not
> registered
> [    2.777770] omap-abe-twl6040 sound.4: snd_soc_register_card() failed:
> -517
> [    2.785095] platform sound.4: Driver omap-abe-twl6040 requests probe
> deferral
> 
> We get a EREMOTEIO when calling regmap_add_irq_chip() from
> twl6040_probe(). regmap_add_irq_chip() will try to perform non-cached
> i2c writes, where the omap-i2c driver get a NACK from the remote chip.
> Strange enough, the non-cached read just before (i.e.
> twl6040_reg_read(twl6040, TWL6040_REG_ASICREV)) succeeds.

Power is not enabled? There might be missing 32K clock for twl6040, can you
check your board's schema against PandaBoard-ES?

> 
> After some fiddling around, I could find that delaying the call to
> regmap_add_irq_chip(), let's say by msleep(1), will "solve" the problem.
> 
> As the TRM for TWL6040 is not public, and I cannot physically access the
> i2c bus on the board, I ran out of hypothesis. I first suspected a
> concurrent access from the McPDM bus, but the mcpdm driver does not seem
> to perform command (write) access.
> 
> As the regulators are enabled just before, could it be the twl6040 IP
> which is not yet initialized, and NACK'ing writes but not reads? The
> commit c7f9129 changed the regmap logic by reducing the number of
> non-cached accesses, and thus changed the access timings, so it may have
> trigged this behaviour. But this is pure supposition, as I cannot hack
> the i2c bus :( And adding any kind of tracing before
> regmap_add_irq_chip() usually delays enough to make the probe succeeds.
> 
> Any ideas?
> 
> Regards,
> Florian
> 
> [1] http://thread.gmane.org/gmane.linux.ports.arm.omap/110801
> 


-- 
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

TWL6040 fails to initialize

[Peter Ujfalusi]

Hi Florian,

On 02/25/2014 12:30 PM, Florian Vaussard wrote:
> Hi Peter,
> 
> I got recently to work on the DT support for an OMAP4 board [1], and I
> encountered some troubles with the probe of the twl6040 audio codec on
> 3.14-rc kernels. On 3.13, things were working correctly. So I somewhat
> managed to bisect this down to [c7f9129 mfd: twl6040: reg_defaults
> support for regmap]. But looking more into it, things are less obvious.

Interesting. I just booted 3.14.0-rc4 with HEAD:
7472e009a3f1 Merge branch 'for-linus' of
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security

on my PandaBoard-ES and audio comes up just fine (twl6040).

> 
> When the init fails, here is what happens:
> 
> [    2.455749] twl6040 0-004b: Looking up vio-supply from device tree
> [    2.456359] twl6040 0-004b: Looking up v2v1-supply from device tree
> [    2.457061] twl6040 0-004b: Failed to set masks in 0x4: -121
> [    2.463043] twl6040: probe of 0-004b failed with error -121
> 
> logically resulting in

Do you have the regulators in your dts file? vio and v2v1 is needed by the
twl6040.

> 
> [    2.770050] omap-abe-twl6040 sound.4: ASoC: CODEC twl6040-codec not
> registered
> [    2.777770] omap-abe-twl6040 sound.4: snd_soc_register_card() failed:
> -517
> [    2.785095] platform sound.4: Driver omap-abe-twl6040 requests probe
> deferral
> 
> We get a EREMOTEIO when calling regmap_add_irq_chip() from
> twl6040_probe(). regmap_add_irq_chip() will try to perform non-cached
> i2c writes, where the omap-i2c driver get a NACK from the remote chip.
> Strange enough, the non-cached read just before (i.e.
> twl6040_reg_read(twl6040, TWL6040_REG_ASICREV)) succeeds.

Power is not enabled? There might be missing 32K clock for twl6040, can you
check your board's schema against PandaBoard-ES?

> 
> After some fiddling around, I could find that delaying the call to
> regmap_add_irq_chip(), let's say by msleep(1), will "solve" the problem.
> 
> As the TRM for TWL6040 is not public, and I cannot physically access the
> i2c bus on the board, I ran out of hypothesis. I first suspected a
> concurrent access from the McPDM bus, but the mcpdm driver does not seem
> to perform command (write) access.
> 
> As the regulators are enabled just before, could it be the twl6040 IP
> which is not yet initialized, and NACK'ing writes but not reads? The
> commit c7f9129 changed the regmap logic by reducing the number of
> non-cached accesses, and thus changed the access timings, so it may have
> trigged this behaviour. But this is pure supposition, as I cannot hack
> the i2c bus :( And adding any kind of tracing before
> regmap_add_irq_chip() usually delays enough to make the probe succeeds.
> 
> Any ideas?
> 
> Regards,
> Florian
> 
> [1] http://thread.gmane.org/gmane.linux.ports.arm.omap/110801
> 


-- 
P?ter

Re: TWL6040 fails to initialize

[Florian Vaussard]

Hi Peter,

On 02/25/2014 03:29 PM, Peter Ujfalusi wrote:
> Hi Florian,
> 
> On 02/25/2014 12:30 PM, Florian Vaussard wrote:
>> Hi Peter,
>>
>> I got recently to work on the DT support for an OMAP4 board [1], and I
>> encountered some troubles with the probe of the twl6040 audio codec on
>> 3.14-rc kernels. On 3.13, things were working correctly. So I somewhat
>> managed to bisect this down to [c7f9129 mfd: twl6040: reg_defaults
>> support for regmap]. But looking more into it, things are less obvious.
> 
> Interesting. I just booted 3.14.0-rc4 with HEAD:
> 7472e009a3f1 Merge branch 'for-linus' of
> git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security
> 
> on my PandaBoard-ES and audio comes up just fine (twl6040).
> 

I know, it is weird. It appeared with 3.14-rc1, 3.13 was working just fine.

>>
>> When the init fails, here is what happens:
>>
>> [    2.455749] twl6040 0-004b: Looking up vio-supply from device tree
>> [    2.456359] twl6040 0-004b: Looking up v2v1-supply from device tree
>> [    2.457061] twl6040 0-004b: Failed to set masks in 0x4: -121
>> [    2.463043] twl6040: probe of 0-004b failed with error -121
>>
>> logically resulting in
> 
> Do you have the regulators in your dts file? vio and v2v1 is needed by the
> twl6040.
> 

Yes, I have both regulators. Here is the relevant part of the DTS:

        twl6040: twl@4b {
                compatible = "ti,twl6040";
                reg = <0x4b>;
                interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
                interrupt-parent = <&gic>;
                ti,audpwron-gpio = <&gpio6 0 GPIO_ACTIVE_HIGH>;

                vio-supply = <&v1v8>;
                v2v1-supply = <&v2v1>;
                enable-active-high;
        };


>>
>> [    2.770050] omap-abe-twl6040 sound.4: ASoC: CODEC twl6040-codec not
>> registered
>> [    2.777770] omap-abe-twl6040 sound.4: snd_soc_register_card() failed:
>> -517
>> [    2.785095] platform sound.4: Driver omap-abe-twl6040 requests probe
>> deferral
>>
>> We get a EREMOTEIO when calling regmap_add_irq_chip() from
>> twl6040_probe(). regmap_add_irq_chip() will try to perform non-cached
>> i2c writes, where the omap-i2c driver get a NACK from the remote chip.
>> Strange enough, the non-cached read just before (i.e.
>> twl6040_reg_read(twl6040, TWL6040_REG_ASICREV)) succeeds.
> 
> Power is not enabled? There might be missing 32K clock for twl6040, can you
> check your board's schema against PandaBoard-ES?
> 

If the power was not enabled at all, I would be unable to read the
revision register, no? And delaying the probe by one millisecond would
be of no help in this case IMHO.

Unfortunately, I don't have access to the schematics of the processor
board. Only the expansion boards from Gumstix have public schematics.

For the 32K, if it was disabled, I think that I would be unable to play
with the headset output in low-power mode, right? When I put the
msleep(), I can play to HSL/HSR and everything seems to work as expected.

Regards,
Florian

TWL6040 fails to initialize

[Florian Vaussard]

Hi Peter,

On 02/25/2014 03:29 PM, Peter Ujfalusi wrote:
> Hi Florian,
> 
> On 02/25/2014 12:30 PM, Florian Vaussard wrote:
>> Hi Peter,
>>
>> I got recently to work on the DT support for an OMAP4 board [1], and I
>> encountered some troubles with the probe of the twl6040 audio codec on
>> 3.14-rc kernels. On 3.13, things were working correctly. So I somewhat
>> managed to bisect this down to [c7f9129 mfd: twl6040: reg_defaults
>> support for regmap]. But looking more into it, things are less obvious.
> 
> Interesting. I just booted 3.14.0-rc4 with HEAD:
> 7472e009a3f1 Merge branch 'for-linus' of
> git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security
> 
> on my PandaBoard-ES and audio comes up just fine (twl6040).
> 

I know, it is weird. It appeared with 3.14-rc1, 3.13 was working just fine.

>>
>> When the init fails, here is what happens:
>>
>> [    2.455749] twl6040 0-004b: Looking up vio-supply from device tree
>> [    2.456359] twl6040 0-004b: Looking up v2v1-supply from device tree
>> [    2.457061] twl6040 0-004b: Failed to set masks in 0x4: -121
>> [    2.463043] twl6040: probe of 0-004b failed with error -121
>>
>> logically resulting in
> 
> Do you have the regulators in your dts file? vio and v2v1 is needed by the
> twl6040.
> 

Yes, I have both regulators. Here is the relevant part of the DTS:

        twl6040: twl at 4b {
                compatible = "ti,twl6040";
                reg = <0x4b>;
                interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
                interrupt-parent = <&gic>;
                ti,audpwron-gpio = <&gpio6 0 GPIO_ACTIVE_HIGH>;

                vio-supply = <&v1v8>;
                v2v1-supply = <&v2v1>;
                enable-active-high;
        };


>>
>> [    2.770050] omap-abe-twl6040 sound.4: ASoC: CODEC twl6040-codec not
>> registered
>> [    2.777770] omap-abe-twl6040 sound.4: snd_soc_register_card() failed:
>> -517
>> [    2.785095] platform sound.4: Driver omap-abe-twl6040 requests probe
>> deferral
>>
>> We get a EREMOTEIO when calling regmap_add_irq_chip() from
>> twl6040_probe(). regmap_add_irq_chip() will try to perform non-cached
>> i2c writes, where the omap-i2c driver get a NACK from the remote chip.
>> Strange enough, the non-cached read just before (i.e.
>> twl6040_reg_read(twl6040, TWL6040_REG_ASICREV)) succeeds.
> 
> Power is not enabled? There might be missing 32K clock for twl6040, can you
> check your board's schema against PandaBoard-ES?
> 

If the power was not enabled at all, I would be unable to read the
revision register, no? And delaying the probe by one millisecond would
be of no help in this case IMHO.

Unfortunately, I don't have access to the schematics of the processor
board. Only the expansion boards from Gumstix have public schematics.

For the 32K, if it was disabled, I think that I would be unable to play
with the headset output in low-power mode, right? When I put the
msleep(), I can play to HSL/HSR and everything seems to work as expected.

Regards,
Florian

Re: TWL6040 fails to initialize

[Peter Ujfalusi]

On 02/25/2014 05:41 PM, Florian Vaussard wrote:
> If the power was not enabled at all, I would be unable to read the
> revision register, no? And delaying the probe by one millisecond would
> be of no help in this case IMHO.

One thing which might cause this is that if the audpwron GPIO is set high
before the twl6040 module is probing. When I request the GPIO I ask it to be
set to low. If the line was high before this means we initiate the power off
sequence.
Can you try something like this:

diff --git a/drivers/mfd/twl6040.c b/drivers/mfd/twl6040.c
index 75316fb33448..d2a0bd1539ae 100644
--- a/drivers/mfd/twl6040.c
+++ b/drivers/mfd/twl6040.c
@@ -674,6 +674,9 @@ static int twl6040_probe(struct i2c_client *client,
                                            GPIOF_OUT_INIT_LOW, "audpwron");
                if (ret)
                        goto gpio_err;
+
+               /* power-down sequence latency */
+               usleep_range(500, 700);
        }

        ret = regmap_add_irq_chip(twl6040->regmap, twl6040->irq, IRQF_ONESHOT,


-- 
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

TWL6040 fails to initialize

[Peter Ujfalusi]

On 02/25/2014 05:41 PM, Florian Vaussard wrote:
> If the power was not enabled at all, I would be unable to read the
> revision register, no? And delaying the probe by one millisecond would
> be of no help in this case IMHO.

One thing which might cause this is that if the audpwron GPIO is set high
before the twl6040 module is probing. When I request the GPIO I ask it to be
set to low. If the line was high before this means we initiate the power off
sequence.
Can you try something like this:

diff --git a/drivers/mfd/twl6040.c b/drivers/mfd/twl6040.c
index 75316fb33448..d2a0bd1539ae 100644
--- a/drivers/mfd/twl6040.c
+++ b/drivers/mfd/twl6040.c
@@ -674,6 +674,9 @@ static int twl6040_probe(struct i2c_client *client,
                                            GPIOF_OUT_INIT_LOW, "audpwron");
                if (ret)
                        goto gpio_err;
+
+               /* power-down sequence latency */
+               usleep_range(500, 700);
        }

        ret = regmap_add_irq_chip(twl6040->regmap, twl6040->irq, IRQF_ONESHOT,


-- 
P?ter

Re: TWL6040 fails to initialize

[Florian Vaussard]

Hi,

First, thanks for your help.

On 02/26/2014 08:26 AM, Peter Ujfalusi wrote:
> On 02/25/2014 05:41 PM, Florian Vaussard wrote:
>> If the power was not enabled at all, I would be unable to read the
>> revision register, no? And delaying the probe by one millisecond would
>> be of no help in this case IMHO.
> 
> One thing which might cause this is that if the audpwron GPIO is set high
> before the twl6040 module is probing. When I request the GPIO I ask it to be
> set to low. If the line was high before this means we initiate the power off
> sequence.
> Can you try something like this:
> 

I statistically checked that the sleep should be placed after the GPIO
request, so indeed this seems to be the problem, and your explanation is
plausible. Can you send a proper patch?

Now, related to this, I managed to found a part of the datasheet on the
Great Internet. Looking at the "Power-Up Sequence" section, it is written:

- NRESPWRON goes high -> plug detect and GPO are available
- V2V1 goes high -> hook-detect available by I2C programming (sleep mode)
- AUDPWRON goes high && READYINT -> ready to communicate through I2C and PDM

So, although there seems to be some contradictions on when it is
possible to access the I2C, shouldn't we enable the AUDPWRON GPIO
_before_ making any I2C access?

For the twl6040_probe, the following path would seem more correct to me:

1) enable regulators
2) request AUDPWRON
3) twl6040_power(ON) && regcache_cache_only(false)
4) wait for READYINT (or sleep if deterministic)
5) perform all required I2C accesses (read revision, etc.)
6) twl6040_power(OFF) && regcache_cache_only(true)

What do you think?

Thanks,
Florian

> diff --git a/drivers/mfd/twl6040.c b/drivers/mfd/twl6040.c
> index 75316fb33448..d2a0bd1539ae 100644
> --- a/drivers/mfd/twl6040.c
> +++ b/drivers/mfd/twl6040.c
> @@ -674,6 +674,9 @@ static int twl6040_probe(struct i2c_client *client,
>                                             GPIOF_OUT_INIT_LOW, "audpwron");
>                 if (ret)
>                         goto gpio_err;
> +
> +               /* power-down sequence latency */
> +               usleep_range(500, 700);
>         }
> 
>         ret = regmap_add_irq_chip(twl6040->regmap, twl6040->irq, IRQF_ONESHOT,
> 
> 

TWL6040 fails to initialize

[Florian Vaussard]

Hi,

First, thanks for your help.

On 02/26/2014 08:26 AM, Peter Ujfalusi wrote:
> On 02/25/2014 05:41 PM, Florian Vaussard wrote:
>> If the power was not enabled at all, I would be unable to read the
>> revision register, no? And delaying the probe by one millisecond would
>> be of no help in this case IMHO.
> 
> One thing which might cause this is that if the audpwron GPIO is set high
> before the twl6040 module is probing. When I request the GPIO I ask it to be
> set to low. If the line was high before this means we initiate the power off
> sequence.
> Can you try something like this:
> 

I statistically checked that the sleep should be placed after the GPIO
request, so indeed this seems to be the problem, and your explanation is
plausible. Can you send a proper patch?

Now, related to this, I managed to found a part of the datasheet on the
Great Internet. Looking at the "Power-Up Sequence" section, it is written:

- NRESPWRON goes high -> plug detect and GPO are available
- V2V1 goes high -> hook-detect available by I2C programming (sleep mode)
- AUDPWRON goes high && READYINT -> ready to communicate through I2C and PDM

So, although there seems to be some contradictions on when it is
possible to access the I2C, shouldn't we enable the AUDPWRON GPIO
_before_ making any I2C access?

For the twl6040_probe, the following path would seem more correct to me:

1) enable regulators
2) request AUDPWRON
3) twl6040_power(ON) && regcache_cache_only(false)
4) wait for READYINT (or sleep if deterministic)
5) perform all required I2C accesses (read revision, etc.)
6) twl6040_power(OFF) && regcache_cache_only(true)

What do you think?

Thanks,
Florian

> diff --git a/drivers/mfd/twl6040.c b/drivers/mfd/twl6040.c
> index 75316fb33448..d2a0bd1539ae 100644
> --- a/drivers/mfd/twl6040.c
> +++ b/drivers/mfd/twl6040.c
> @@ -674,6 +674,9 @@ static int twl6040_probe(struct i2c_client *client,
>                                             GPIOF_OUT_INIT_LOW, "audpwron");
>                 if (ret)
>                         goto gpio_err;
> +
> +               /* power-down sequence latency */
> +               usleep_range(500, 700);
>         }
> 
>         ret = regmap_add_irq_chip(twl6040->regmap, twl6040->irq, IRQF_ONESHOT,
> 
> 

Re: TWL6040 fails to initialize

[Peter Ujfalusi]

On 02/26/2014 11:53 AM, Florian Vaussard wrote:
> On 02/26/2014 08:26 AM, Peter Ujfalusi wrote:
>> On 02/25/2014 05:41 PM, Florian Vaussard wrote:
>>> If the power was not enabled at all, I would be unable to read the
>>> revision register, no? And delaying the probe by one millisecond would
>>> be of no help in this case IMHO.
>>
>> One thing which might cause this is that if the audpwron GPIO is set high
>> before the twl6040 module is probing. When I request the GPIO I ask it to be
>> set to low. If the line was high before this means we initiate the power off
>> sequence.
>> Can you try something like this:
>>
> 
> I statistically checked that the sleep should be placed after the GPIO
> request, so indeed this seems to be the problem, and your explanation is
> plausible. Can you send a proper patch?
> 
> Now, related to this, I managed to found a part of the datasheet on the
> Great Internet. Looking at the "Power-Up Sequence" section, it is written:
> 
> - NRESPWRON goes high -> plug detect and GPO are available
> - V2V1 goes high -> hook-detect available by I2C programming (sleep mode)
> - AUDPWRON goes high && READYINT -> ready to communicate through I2C and PDM
> 
> So, although there seems to be some contradictions on when it is
> possible to access the I2C, shouldn't we enable the AUDPWRON GPIO
> _before_ making any I2C access?
> 
> For the twl6040_probe, the following path would seem more correct to me:
> 
> 1) enable regulators
> 2) request AUDPWRON
> 3) twl6040_power(ON) && regcache_cache_only(false)
> 4) wait for READYINT (or sleep if deterministic)
> 5) perform all required I2C accesses (read revision, etc.)
> 6) twl6040_power(OFF) && regcache_cache_only(true)
> 
> What do you think?

Even when the AUDPWRON signal is low we can access to registers in VIO domain,
plug detect and GPO functions. So there's no need to power on the codec just
to power it down later.


> Thanks,
> Florian
> 
>> diff --git a/drivers/mfd/twl6040.c b/drivers/mfd/twl6040.c
>> index 75316fb33448..d2a0bd1539ae 100644
>> --- a/drivers/mfd/twl6040.c
>> +++ b/drivers/mfd/twl6040.c
>> @@ -674,6 +674,9 @@ static int twl6040_probe(struct i2c_client *client,
>>                                             GPIOF_OUT_INIT_LOW, "audpwron");
>>                 if (ret)
>>                         goto gpio_err;
>> +
>> +               /* power-down sequence latency */
>> +               usleep_range(500, 700);
>>         }
>>
>>         ret = regmap_add_irq_chip(twl6040->regmap, twl6040->irq, IRQF_ONESHOT,
>>
>>


-- 
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

TWL6040 fails to initialize

[Peter Ujfalusi]

On 02/26/2014 11:53 AM, Florian Vaussard wrote:
> On 02/26/2014 08:26 AM, Peter Ujfalusi wrote:
>> On 02/25/2014 05:41 PM, Florian Vaussard wrote:
>>> If the power was not enabled at all, I would be unable to read the
>>> revision register, no? And delaying the probe by one millisecond would
>>> be of no help in this case IMHO.
>>
>> One thing which might cause this is that if the audpwron GPIO is set high
>> before the twl6040 module is probing. When I request the GPIO I ask it to be
>> set to low. If the line was high before this means we initiate the power off
>> sequence.
>> Can you try something like this:
>>
> 
> I statistically checked that the sleep should be placed after the GPIO
> request, so indeed this seems to be the problem, and your explanation is
> plausible. Can you send a proper patch?
> 
> Now, related to this, I managed to found a part of the datasheet on the
> Great Internet. Looking at the "Power-Up Sequence" section, it is written:
> 
> - NRESPWRON goes high -> plug detect and GPO are available
> - V2V1 goes high -> hook-detect available by I2C programming (sleep mode)
> - AUDPWRON goes high && READYINT -> ready to communicate through I2C and PDM
> 
> So, although there seems to be some contradictions on when it is
> possible to access the I2C, shouldn't we enable the AUDPWRON GPIO
> _before_ making any I2C access?
> 
> For the twl6040_probe, the following path would seem more correct to me:
> 
> 1) enable regulators
> 2) request AUDPWRON
> 3) twl6040_power(ON) && regcache_cache_only(false)
> 4) wait for READYINT (or sleep if deterministic)
> 5) perform all required I2C accesses (read revision, etc.)
> 6) twl6040_power(OFF) && regcache_cache_only(true)
> 
> What do you think?

Even when the AUDPWRON signal is low we can access to registers in VIO domain,
plug detect and GPO functions. So there's no need to power on the codec just
to power it down later.


> Thanks,
> Florian
> 
>> diff --git a/drivers/mfd/twl6040.c b/drivers/mfd/twl6040.c
>> index 75316fb33448..d2a0bd1539ae 100644
>> --- a/drivers/mfd/twl6040.c
>> +++ b/drivers/mfd/twl6040.c
>> @@ -674,6 +674,9 @@ static int twl6040_probe(struct i2c_client *client,
>>                                             GPIOF_OUT_INIT_LOW, "audpwron");
>>                 if (ret)
>>                         goto gpio_err;
>> +
>> +               /* power-down sequence latency */
>> +               usleep_range(500, 700);
>>         }
>>
>>         ret = regmap_add_irq_chip(twl6040->regmap, twl6040->irq, IRQF_ONESHOT,
>>
>>


-- 
P?ter

Re: TWL6040 fails to initialize

[Florian Vaussard]

On 02/26/2014 11:28 AM, Peter Ujfalusi wrote:
> On 02/26/2014 11:53 AM, Florian Vaussard wrote:
>> On 02/26/2014 08:26 AM, Peter Ujfalusi wrote:
>>> On 02/25/2014 05:41 PM, Florian Vaussard wrote:
>>>> If the power was not enabled at all, I would be unable to read the
>>>> revision register, no? And delaying the probe by one millisecond would
>>>> be of no help in this case IMHO.
>>>
>>> One thing which might cause this is that if the audpwron GPIO is set high
>>> before the twl6040 module is probing. When I request the GPIO I ask it to be
>>> set to low. If the line was high before this means we initiate the power off
>>> sequence.
>>> Can you try something like this:
>>>
>>
>> I statistically checked that the sleep should be placed after the GPIO
>> request, so indeed this seems to be the problem, and your explanation is
>> plausible. Can you send a proper patch?
>>
>> Now, related to this, I managed to found a part of the datasheet on the
>> Great Internet. Looking at the "Power-Up Sequence" section, it is written:
>>
>> - NRESPWRON goes high -> plug detect and GPO are available
>> - V2V1 goes high -> hook-detect available by I2C programming (sleep mode)
>> - AUDPWRON goes high && READYINT -> ready to communicate through I2C and PDM
>>
>> So, although there seems to be some contradictions on when it is
>> possible to access the I2C, shouldn't we enable the AUDPWRON GPIO
>> _before_ making any I2C access?
>>
>> For the twl6040_probe, the following path would seem more correct to me:
>>
>> 1) enable regulators
>> 2) request AUDPWRON
>> 3) twl6040_power(ON) && regcache_cache_only(false)
>> 4) wait for READYINT (or sleep if deterministic)
>> 5) perform all required I2C accesses (read revision, etc.)
>> 6) twl6040_power(OFF) && regcache_cache_only(true)
>>
>> What do you think?
> 
> Even when the AUDPWRON signal is low we can access to registers in VIO domain,
> plug detect and GPO functions. So there's no need to power on the codec just
> to power it down later.
> 

Ok, if it is safe to do so, I am fine with the current probe. I let you
post a patch to add a delay after requesting the GPIO.

Regards,
Florian

> 
>> Thanks,
>> Florian
>>
>>> diff --git a/drivers/mfd/twl6040.c b/drivers/mfd/twl6040.c
>>> index 75316fb33448..d2a0bd1539ae 100644
>>> --- a/drivers/mfd/twl6040.c
>>> +++ b/drivers/mfd/twl6040.c
>>> @@ -674,6 +674,9 @@ static int twl6040_probe(struct i2c_client *client,
>>>                                             GPIOF_OUT_INIT_LOW, "audpwron");
>>>                 if (ret)
>>>                         goto gpio_err;
>>> +
>>> +               /* power-down sequence latency */
>>> +               usleep_range(500, 700);
>>>         }
>>>
>>>         ret = regmap_add_irq_chip(twl6040->regmap, twl6040->irq, IRQF_ONESHOT,
>>>
>>>
> 
> 

TWL6040 fails to initialize

[Florian Vaussard]

On 02/26/2014 11:28 AM, Peter Ujfalusi wrote:
> On 02/26/2014 11:53 AM, Florian Vaussard wrote:
>> On 02/26/2014 08:26 AM, Peter Ujfalusi wrote:
>>> On 02/25/2014 05:41 PM, Florian Vaussard wrote:
>>>> If the power was not enabled at all, I would be unable to read the
>>>> revision register, no? And delaying the probe by one millisecond would
>>>> be of no help in this case IMHO.
>>>
>>> One thing which might cause this is that if the audpwron GPIO is set high
>>> before the twl6040 module is probing. When I request the GPIO I ask it to be
>>> set to low. If the line was high before this means we initiate the power off
>>> sequence.
>>> Can you try something like this:
>>>
>>
>> I statistically checked that the sleep should be placed after the GPIO
>> request, so indeed this seems to be the problem, and your explanation is
>> plausible. Can you send a proper patch?
>>
>> Now, related to this, I managed to found a part of the datasheet on the
>> Great Internet. Looking at the "Power-Up Sequence" section, it is written:
>>
>> - NRESPWRON goes high -> plug detect and GPO are available
>> - V2V1 goes high -> hook-detect available by I2C programming (sleep mode)
>> - AUDPWRON goes high && READYINT -> ready to communicate through I2C and PDM
>>
>> So, although there seems to be some contradictions on when it is
>> possible to access the I2C, shouldn't we enable the AUDPWRON GPIO
>> _before_ making any I2C access?
>>
>> For the twl6040_probe, the following path would seem more correct to me:
>>
>> 1) enable regulators
>> 2) request AUDPWRON
>> 3) twl6040_power(ON) && regcache_cache_only(false)
>> 4) wait for READYINT (or sleep if deterministic)
>> 5) perform all required I2C accesses (read revision, etc.)
>> 6) twl6040_power(OFF) && regcache_cache_only(true)
>>
>> What do you think?
> 
> Even when the AUDPWRON signal is low we can access to registers in VIO domain,
> plug detect and GPO functions. So there's no need to power on the codec just
> to power it down later.
> 

Ok, if it is safe to do so, I am fine with the current probe. I let you
post a patch to add a delay after requesting the GPIO.

Regards,
Florian

> 
>> Thanks,
>> Florian
>>
>>> diff --git a/drivers/mfd/twl6040.c b/drivers/mfd/twl6040.c
>>> index 75316fb33448..d2a0bd1539ae 100644
>>> --- a/drivers/mfd/twl6040.c
>>> +++ b/drivers/mfd/twl6040.c
>>> @@ -674,6 +674,9 @@ static int twl6040_probe(struct i2c_client *client,
>>>                                             GPIOF_OUT_INIT_LOW, "audpwron");
>>>                 if (ret)
>>>                         goto gpio_err;
>>> +
>>> +               /* power-down sequence latency */
>>> +               usleep_range(500, 700);
>>>         }
>>>
>>>         ret = regmap_add_irq_chip(twl6040->regmap, twl6040->irq, IRQF_ONESHOT,
>>>
>>>
> 
> 

Re: TWL6040 fails to initialize

[Peter Ujfalusi]

On 02/26/2014 12:31 PM, Florian Vaussard wrote:
>>> I statistically checked that the sleep should be placed after the GPIO
>>> request, so indeed this seems to be the problem, and your explanation is
>>> plausible. Can you send a proper patch?
>>>
>>> Now, related to this, I managed to found a part of the datasheet on the
>>> Great Internet. Looking at the "Power-Up Sequence" section, it is written:
>>>
>>> - NRESPWRON goes high -> plug detect and GPO are available
>>> - V2V1 goes high -> hook-detect available by I2C programming (sleep mode)
>>> - AUDPWRON goes high && READYINT -> ready to communicate through I2C and PDM
>>>
>>> So, although there seems to be some contradictions on when it is
>>> possible to access the I2C, shouldn't we enable the AUDPWRON GPIO
>>> _before_ making any I2C access?
>>>
>>> For the twl6040_probe, the following path would seem more correct to me:
>>>
>>> 1) enable regulators
>>> 2) request AUDPWRON
>>> 3) twl6040_power(ON) && regcache_cache_only(false)
>>> 4) wait for READYINT (or sleep if deterministic)
>>> 5) perform all required I2C accesses (read revision, etc.)
>>> 6) twl6040_power(OFF) && regcache_cache_only(true)
>>>
>>> What do you think?
>>
>> Even when the AUDPWRON signal is low we can access to registers in VIO domain,
>> plug detect and GPO functions. So there's no need to power on the codec just
>> to power it down later.
>>
> 
> Ok, if it is safe to do so, I am fine with the current probe. I let you
> post a patch to add a delay after requesting the GPIO.


There's something else going on on your board.
I did some experiments on PandaBoards: in u-boot I set the GPIO127 (audpwron)
high and boot the kernel.
During the boot I see all sorts of errors, mcpdm fails to reset, i2c timeouts
towards twl6040.
For this the solution is to read the INTID register just after I request the
gpio - to clear any no longer valid interrupts before we request the IRQ. This
works fine on my boards.

The issue you have with your board is something totally different.
One thing might be that the i2c bus is set to 400KHz while the twl6040 by
default is set for normal mode (100KHz). On all of the boards I have access
this works fine but there might be some electrical issue on your board which
causes i2c to fail.

There's also another thing I just noticed: the regmap patch
regmap_register_patch() when it is applied will not update the regcache by
design. I do not like that too much. On the other hand it is handy to set the
ACCCTL register as early as possible.

So, I'm not sure about the delay and why it helps on your board.

-- 
Péter

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

TWL6040 fails to initialize

[Peter Ujfalusi]

On 02/26/2014 12:31 PM, Florian Vaussard wrote:
>>> I statistically checked that the sleep should be placed after the GPIO
>>> request, so indeed this seems to be the problem, and your explanation is
>>> plausible. Can you send a proper patch?
>>>
>>> Now, related to this, I managed to found a part of the datasheet on the
>>> Great Internet. Looking at the "Power-Up Sequence" section, it is written:
>>>
>>> - NRESPWRON goes high -> plug detect and GPO are available
>>> - V2V1 goes high -> hook-detect available by I2C programming (sleep mode)
>>> - AUDPWRON goes high && READYINT -> ready to communicate through I2C and PDM
>>>
>>> So, although there seems to be some contradictions on when it is
>>> possible to access the I2C, shouldn't we enable the AUDPWRON GPIO
>>> _before_ making any I2C access?
>>>
>>> For the twl6040_probe, the following path would seem more correct to me:
>>>
>>> 1) enable regulators
>>> 2) request AUDPWRON
>>> 3) twl6040_power(ON) && regcache_cache_only(false)
>>> 4) wait for READYINT (or sleep if deterministic)
>>> 5) perform all required I2C accesses (read revision, etc.)
>>> 6) twl6040_power(OFF) && regcache_cache_only(true)
>>>
>>> What do you think?
>>
>> Even when the AUDPWRON signal is low we can access to registers in VIO domain,
>> plug detect and GPO functions. So there's no need to power on the codec just
>> to power it down later.
>>
> 
> Ok, if it is safe to do so, I am fine with the current probe. I let you
> post a patch to add a delay after requesting the GPIO.


There's something else going on on your board.
I did some experiments on PandaBoards: in u-boot I set the GPIO127 (audpwron)
high and boot the kernel.
During the boot I see all sorts of errors, mcpdm fails to reset, i2c timeouts
towards twl6040.
For this the solution is to read the INTID register just after I request the
gpio - to clear any no longer valid interrupts before we request the IRQ. This
works fine on my boards.

The issue you have with your board is something totally different.
One thing might be that the i2c bus is set to 400KHz while the twl6040 by
default is set for normal mode (100KHz). On all of the boards I have access
this works fine but there might be some electrical issue on your board which
causes i2c to fail.

There's also another thing I just noticed: the regmap patch
regmap_register_patch() when it is applied will not update the regcache by
design. I do not like that too much. On the other hand it is handy to set the
ACCCTL register as early as possible.

So, I'm not sure about the delay and why it helps on your board.

-- 
P?ter

Re: TWL6040 fails to initialize

[Florian Vaussard]

On 02/27/2014 01:05 PM, Peter Ujfalusi wrote:
> On 02/26/2014 12:31 PM, Florian Vaussard wrote:
>>>> I statistically checked that the sleep should be placed after the GPIO
>>>> request, so indeed this seems to be the problem, and your explanation is
>>>> plausible. Can you send a proper patch?
>>>>
>>>> Now, related to this, I managed to found a part of the datasheet on the
>>>> Great Internet. Looking at the "Power-Up Sequence" section, it is written:
>>>>
>>>> - NRESPWRON goes high -> plug detect and GPO are available
>>>> - V2V1 goes high -> hook-detect available by I2C programming (sleep mode)
>>>> - AUDPWRON goes high && READYINT -> ready to communicate through I2C and PDM
>>>>
>>>> So, although there seems to be some contradictions on when it is
>>>> possible to access the I2C, shouldn't we enable the AUDPWRON GPIO
>>>> _before_ making any I2C access?
>>>>
>>>> For the twl6040_probe, the following path would seem more correct to me:
>>>>
>>>> 1) enable regulators
>>>> 2) request AUDPWRON
>>>> 3) twl6040_power(ON) && regcache_cache_only(false)
>>>> 4) wait for READYINT (or sleep if deterministic)
>>>> 5) perform all required I2C accesses (read revision, etc.)
>>>> 6) twl6040_power(OFF) && regcache_cache_only(true)
>>>>
>>>> What do you think?
>>>
>>> Even when the AUDPWRON signal is low we can access to registers in VIO domain,
>>> plug detect and GPO functions. So there's no need to power on the codec just
>>> to power it down later.
>>>
>>
>> Ok, if it is safe to do so, I am fine with the current probe. I let you
>> post a patch to add a delay after requesting the GPIO.
> 
> 
> There's something else going on on your board.
> I did some experiments on PandaBoards: in u-boot I set the GPIO127 (audpwron)
> high and boot the kernel.
> During the boot I see all sorts of errors, mcpdm fails to reset, i2c timeouts
> towards twl6040.
> For this the solution is to read the INTID register just after I request the
> gpio - to clear any no longer valid interrupts before we request the IRQ. This
> works fine on my boards.
> 

As I said in the other thread, reading INTID is solving the issue, but
it could be a side effect.

> The issue you have with your board is something totally different.
> One thing might be that the i2c bus is set to 400KHz while the twl6040 by
> default is set for normal mode (100KHz). On all of the boards I have access
> this works fine but there might be some electrical issue on your board which
> causes i2c to fail.
> 

I should probe the I2C bus to dig the issue further, but I don't have
access to the pins. So apart drilling into the PCB, this will remain a
kind of mystery.

Your patches are somehow solving this, so I will stop my investigations
for now.

> There's also another thing I just noticed: the regmap patch
> regmap_register_patch() when it is applied will not update the regcache by
> design. I do not like that too much. On the other hand it is handy to set the
> ACCCTL register as early as possible.
> 
> So, I'm not sure about the delay and why it helps on your board.
> 

Not sure neither.

Thanks for your help on this.

Regards,
Florian

TWL6040 fails to initialize

[Florian Vaussard]

On 02/27/2014 01:05 PM, Peter Ujfalusi wrote:
> On 02/26/2014 12:31 PM, Florian Vaussard wrote:
>>>> I statistically checked that the sleep should be placed after the GPIO
>>>> request, so indeed this seems to be the problem, and your explanation is
>>>> plausible. Can you send a proper patch?
>>>>
>>>> Now, related to this, I managed to found a part of the datasheet on the
>>>> Great Internet. Looking at the "Power-Up Sequence" section, it is written:
>>>>
>>>> - NRESPWRON goes high -> plug detect and GPO are available
>>>> - V2V1 goes high -> hook-detect available by I2C programming (sleep mode)
>>>> - AUDPWRON goes high && READYINT -> ready to communicate through I2C and PDM
>>>>
>>>> So, although there seems to be some contradictions on when it is
>>>> possible to access the I2C, shouldn't we enable the AUDPWRON GPIO
>>>> _before_ making any I2C access?
>>>>
>>>> For the twl6040_probe, the following path would seem more correct to me:
>>>>
>>>> 1) enable regulators
>>>> 2) request AUDPWRON
>>>> 3) twl6040_power(ON) && regcache_cache_only(false)
>>>> 4) wait for READYINT (or sleep if deterministic)
>>>> 5) perform all required I2C accesses (read revision, etc.)
>>>> 6) twl6040_power(OFF) && regcache_cache_only(true)
>>>>
>>>> What do you think?
>>>
>>> Even when the AUDPWRON signal is low we can access to registers in VIO domain,
>>> plug detect and GPO functions. So there's no need to power on the codec just
>>> to power it down later.
>>>
>>
>> Ok, if it is safe to do so, I am fine with the current probe. I let you
>> post a patch to add a delay after requesting the GPIO.
> 
> 
> There's something else going on on your board.
> I did some experiments on PandaBoards: in u-boot I set the GPIO127 (audpwron)
> high and boot the kernel.
> During the boot I see all sorts of errors, mcpdm fails to reset, i2c timeouts
> towards twl6040.
> For this the solution is to read the INTID register just after I request the
> gpio - to clear any no longer valid interrupts before we request the IRQ. This
> works fine on my boards.
> 

As I said in the other thread, reading INTID is solving the issue, but
it could be a side effect.

> The issue you have with your board is something totally different.
> One thing might be that the i2c bus is set to 400KHz while the twl6040 by
> default is set for normal mode (100KHz). On all of the boards I have access
> this works fine but there might be some electrical issue on your board which
> causes i2c to fail.
> 

I should probe the I2C bus to dig the issue further, but I don't have
access to the pins. So apart drilling into the PCB, this will remain a
kind of mystery.

Your patches are somehow solving this, so I will stop my investigations
for now.

> There's also another thing I just noticed: the regmap patch
> regmap_register_patch() when it is applied will not update the regcache by
> design. I do not like that too much. On the other hand it is handy to set the
> ACCCTL register as early as possible.
> 
> So, I'm not sure about the delay and why it helps on your board.
> 

Not sure neither.

Thanks for your help on this.

Regards,
Florian