Nokia N900 - audio TPA6130A2 problems

Hello,

sometimes after rebooting Nokia N900 initializing alsa audio fails. Here output from dmesg log when it happen:

[    6.925140] tpa6130a2 2-0060: Write failed
[    6.929534] tpa6130a2 2-0060: Failed to initialize chip
[    6.935272] tpa6130a2: probe of 2-0060 failed with error -121
[    7.624237] rx51-audio n900-audio: Failed to add TPA6130A2 controls
[    7.635101] rx51-audio n900-audio: ASoC: failed to init TLV320AIC34: -19
[    7.645874] rx51-audio n900-audio: ASoC: failed to instantiate card -19
[    7.665740] rx51-audio n900-audio: snd_soc_register_card failed (-19)
[    8.063049] ALSA device list:
[    8.070343]   No soundcards found.

Any idea what to do?

Quoted 9 lines · Pali Rohár

Hello,

sometimes after rebooting Nokia N900 initializing alsa audio fails. Here output from dmesg log when it happen:

[    6.925140] tpa6130a2 2-0060: Write failed
[    6.929534] tpa6130a2 2-0060: Failed to initialize chip
[    6.935272] tpa6130a2: probe of 2-0060 failed with error -121
[    7.624237] rx51-audio n900-audio: Failed to add TPA6130A2

Quoted 8 lines

controls [ 7.635101] rx51-audio n900-audio: ASoC: failed to
init TLV320AIC34: -19 [ 7.645874] rx51-audio n900-audio: ASoC:
failed to instantiate card -19 [ 7.665740] rx51-audio
n900-audio: snd_soc_register_card failed (-19) [ 8.063049] ALSA
device list:

[    8.070343]   No soundcards found.

Any idea what to do?

Looks like the chip is not responding. Try to add a small delay after powerup to give the device to be fully ready, something like the following:

Quoted 29 lines · On Saturday 25 July 2015 15:17:13 Lars-Peter Clausen wrote:

Quoted 9 lines · Pali Rohár

Hello,

sometimes after rebooting Nokia N900 initializing alsa audio fails. Here output from dmesg log when it happen:

[    6.925140] tpa6130a2 2-0060: Write failed
[    6.929534] tpa6130a2 2-0060: Failed to initialize chip
[    6.935272] tpa6130a2: probe of 2-0060 failed with error -121
[    7.624237] rx51-audio n900-audio: Failed to add TPA6130A2

Quoted 8 lines

controls [ 7.635101] rx51-audio n900-audio: ASoC: failed to
init TLV320AIC34: -19 [ 7.645874] rx51-audio n900-audio: ASoC:
failed to instantiate card -19 [ 7.665740] rx51-audio
n900-audio: snd_soc_register_card failed (-19) [ 8.063049] ALSA
device list:

[    8.070343]   No soundcards found.

Any idea what to do?

Looks like the chip is not responding. Try to add a small delay after powerup to give the device to be fully ready, something like the following:

sound/soc/codecs/tpa6130a2.c +0 -0

@@ -152,6 +152,8 @@ static int tpa6130a2_power(u8 power)
  		if (data->power_gpio >= 0)
  			gpio_set_value(data->power_gpio, 1);
 

sound/soc/codecs/tpa6130a2.c static int tpa6130a2_power(u8 power) · line 159

Quoted 5 lines

sound/soc/codecs/tpa6130a2.c +2 -0

+		msleep(5);
+
  		data->power_state = 1;
  		ret = tpa6130a2_initialize();
  		if (ret < 0) {

Hello, your patch did not helped. Problem is still there...

[    5.962585] tpa6130a2 2-0060: Write failed
[    5.962707] tpa6130a2 2-0060: Failed to initialize chip
[    5.962860] tpa6130a2: probe of 2-0060 failed with error -121

[   28.102233] omap_i2c 48072000.i2c: controller timed out
[   29.463653] lp5523x 2-0032: lp5523 Programmable led chip found
[   30.734191] omap_i2c 48072000.i2c: controller timed out waiting for start condition to finish
[   32.142333] i2c i2c-2: SCL is stuck low, exit recovery

Hi

Quoted 32 lines · Pali Rohár

Quoted 31 lines · On Saturday 25 July 2015 15:17:13 Lars-Peter Clausen wrote:

Quoted 9 lines · Pali Rohár

Hello,

sometimes after rebooting Nokia N900 initializing alsa audio fails. Here output from dmesg log when it happen:

[    6.925140] tpa6130a2 2-0060: Write failed
[    6.929534] tpa6130a2 2-0060: Failed to initialize chip
[    6.935272] tpa6130a2: probe of 2-0060 failed with error -121
[    7.624237] rx51-audio n900-audio: Failed to add TPA6130A2

Quoted 8 lines

controls [ 7.635101] rx51-audio n900-audio: ASoC: failed to
init TLV320AIC34: -19 [ 7.645874] rx51-audio n900-audio: ASoC:
failed to instantiate card -19 [ 7.665740] rx51-audio
n900-audio: snd_soc_register_card failed (-19) [ 8.063049]
ALSA device list:

[    8.070343]   No soundcards found.

Any idea what to do?

Looks like the chip is not responding. Try to add a small delay after powerup to give the device to be fully ready, something like the following:

sound/soc/codecs/tpa6130a2.c +0 -0

@@ -152,6 +152,8 @@ static int tpa6130a2_power(u8 power)
 
  		if (data->power_gpio >= 0)
 
  			gpio_set_value(data->power_gpio, 1);
 

Quoted 8 lines

Quoted 6 lines

sound/soc/codecs/tpa6130a2.c +2 -0

+		msleep(5);
+
 

  		data->power_state = 1;
  		ret = tpa6130a2_initialize();
  		if (ret < 0) {

Hello, your patch did not helped. Problem is still there...

For me v4.2-rc5 works, i.e. TPA6130A2 can still play loudly to headphones. Don't know were there any i2c etc regression before it or how easy it would be to reproduce.

Logs below made me thinking can it be a HW issue? Although if it is an HW issue it shouldn't work sometimes I guess. Do you have any earlier well known configuration you could try is it an SW regression or something else?

Quoted 3 lines

[    5.962585] tpa6130a2 2-0060: Write failed
[    5.962707] tpa6130a2 2-0060: Failed to initialize chip
[    5.962860] tpa6130a2: probe of 2-0060 failed with error -121

-121 == EREMOTEIO which is returned from i2c-omap.c when there is no ACK from the chip.

Quoted 5 lines

[   28.102233] omap_i2c 48072000.i2c: controller timed out
[   29.463653] lp5523x 2-0032: lp5523 Programmable led chip found
[   30.734191] omap_i2c 48072000.i2c: controller timed out waiting

for start condition to finish [ 32.142333] i2c i2c-2: SCL is
stuck low, exit recovery

If SCL is really stuck it also explains why chip doesn't acknowledge.

Quoted 6 lines · On Monday 03 August 2015 20:03:16 Jarkko Nikula wrote:

Hi

Quoted 2 lines · Pali Rohár

On Saturday 25 July 2015 15:17:13 Lars-Peter Clausen wrote:
Hello, your patch did not helped. Problem is still there...

Quoted 4 lines

For me v4.2-rc5 works, i.e. TPA6130A2 can still play loudly to headphones. Don't know were there any i2c etc regression before it or how easy it would be to reproduce.

Did you tested it on Nokia N900? Or other device?

Quoted 5 lines

Logs below made me thinking can it be a HW issue? Although if it is an HW issue it shouldn't work sometimes I guess. Do you have any earlier well known configuration you could try is it an SW regression or something else?

Stock Nokia's 2.6.28 kernel works always. With that kernel I have never seen this problem. So I do not think this is HW problem.

This problem is there in more kernel versions, maybe in some older (like v3.5) is was there not so often. But do not remember correctly...

Maybe some power management problem? Something is not always initialized correctly?

I remember that there is some problem (maybe in NoLo - Nokia bootloader) that sometimes chainloaded U-Boot (booted via NoLo) is not able to initialize mmc chip (all read operation fails). In U-Boot I added some code to enable some parts in twl4030 regulator and after that mmc is working always...

So maybe something similar? Kernel expects that some PM or regulator parts are initialized, but they are only sometimes? Just speculation...

Quoted 7 lines · Pali Rohár

Quoted 6 lines · On Monday 03 August 2015 20:03:16 Jarkko Nikula wrote:

Hi

Quoted 2 lines · Pali Rohár

On Saturday 25 July 2015 15:17:13 Lars-Peter Clausen wrote:
Hello, your patch did not helped. Problem is still there...

Quoted 5 lines

Quoted 3 lines

For me v4.2-rc5 works, i.e. TPA6130A2 can still play loudly to headphones. Don't know were there any i2c etc regression before it or how easy it would be to reproduce.

Did you tested it on Nokia N900? Or other device?

N900. Seems to be only user of TPA6130A2 in mainline :-)

Quoted 8 lines

Quoted 4 lines

Logs below made me thinking can it be a HW issue? Although if it is an HW issue it shouldn't work sometimes I guess. Do you have any earlier well known configuration you could try is it an SW regression or something else?

Stock Nokia's 2.6.28 kernel works always. With that kernel I have never seen this problem. So I do not think this is HW problem.

Quoted 3 lines

This problem is there in more kernel versions, maybe in some older (like v3.5) is was there not so often. But do not remember correctly...

It is well possible that some regression got introduced to TPA6130A2 I2C communication over the years without nobody than you now notices. We used to do QA back in Meego N900 days but that was pre 3.x kernels.

Quoted 9 lines

Maybe some power management problem? Something is not always initialized correctly?

I remember that there is some problem (maybe in NoLo - Nokia bootloader) that sometimes chainloaded U-Boot (booted via NoLo) is not able to initialize mmc chip (all read operation fails). In U-Boot I added some code to enable some parts in twl4030 regulator and after that mmc is working always...

Quoted 3 lines

So maybe something similar? Kernel expects that some PM or regulator parts are initialized, but they are only sometimes? Just speculation...

I'm thinking the same. I could figure SCL could be stuck low if TPA or some other chip connected to the same I2C bus is without power and is pulling I2C signals down.

It is well possible that some regression got introduced to TPA6130A2 I2C communication over the years without nobody than you now notices. We used to do QA back in Meego N900 days but that was pre 3.x kernels.

Quoted 3 lines

So maybe something similar? Kernel expects that some PM or regulator parts are initialized, but they are only sometimes? Just speculation...

I'm thinking the same. I could figure SCL could be stuck low if TPA or some other chip connected to the same I2C bus is without power and is pulling I2C signals down.

Quoted 4 lines · Jarkko Nikula

It is well possible that some regression got introduced to TPA6130A2 I2C communication over the years without nobody than you now notices. We used to do QA back in Meego N900 days but that was pre 3.x kernels.

No major changes has been done to the tpa driver during the past years... I wanted to do some updates, like moving it to regmap, but as you said, n900 is the only user (and n9) and I do not feel comfortable to hack on a device where I do not have serial console... And I'm using the n900 time to time also.

Quoted 4 lines

Quoted 3 lines

So maybe something similar? Kernel expects that some PM or regulator parts are initialized, but they are only sometimes? Just speculation...

Quoted 3 lines

I'm thinking the same. I could figure SCL could be stuck low if TPA or some other chip connected to the same I2C bus is without power and is pulling I2C signals down.

What would happen with the SCL stuck on i2c.2 bus if you remove the tpa driver from the kernel? If you remove the other drivers for the devices on i2c.2?

Quoted 20 lines · On Tuesday 04 August 2015 09:02:39 Peter Ujfalusi wrote:

Quoted 4 lines · Jarkko Nikula

It is well possible that some regression got introduced to TPA6130A2 I2C communication over the years without nobody than you now notices. We used to do QA back in Meego N900 days but that was pre 3.x kernels.

No major changes has been done to the tpa driver during the past years... I wanted to do some updates, like moving it to regmap, but as you said, n900 is the only user (and n9) and I do not feel comfortable to hack on a device where I do not have serial console... And I'm using the n900 time to time also.

Quoted 7 lines

Quoted 3 lines

So maybe something similar? Kernel expects that some PM or regulator parts are initialized, but they are only sometimes? Just speculation...

I'm thinking the same. I could figure SCL could be stuck low if TPA or some other chip connected to the same I2C bus is without power and is pulling I2C signals down.

Quoted 3 lines

What would happen with the SCL stuck on i2c.2 bus if you remove the tpa driver from the kernel? If you remove the other drivers for the devices on i2c.2?

Hi Peter and Jarkko! Do you have some code samples for testing? Or something else which I can test? This problem is still reproducible on more N900 devices and I would like to see it fixed.

Hi Pali,

Quoted 21 lines · Pali Rohár

Quoted 20 lines · On Tuesday 04 August 2015 09:02:39 Peter Ujfalusi wrote:

Quoted 4 lines · Jarkko Nikula

It is well possible that some regression got introduced to TPA6130A2 I2C communication over the years without nobody than you now notices. We used to do QA back in Meego N900 days but that was pre 3.x kernels.

No major changes has been done to the tpa driver during the past years... I wanted to do some updates, like moving it to regmap, but as you said, n900 is the only user (and n9) and I do not feel comfortable to hack on a device where I do not have serial console... And I'm using the n900 time to time also.

Quoted 7 lines

Quoted 3 lines

So maybe something similar? Kernel expects that some PM or regulator parts are initialized, but they are only sometimes? Just speculation...

I'm thinking the same. I could figure SCL could be stuck low if TPA or some other chip connected to the same I2C bus is without power and is pulling I2C signals down.

Quoted 7 lines

Quoted 3 lines

What would happen with the SCL stuck on i2c.2 bus if you remove the tpa driver from the kernel? If you remove the other drivers for the devices on i2c.2?

Hi Peter and Jarkko! Do you have some code samples for testing? Or something else which I can test? This problem is still reproducible on more N900 devices and I would like to see it fixed.

I have not seen your error with N900, but while working on N950 I noticed similar problems when I added lp5523. I think the lp5523 reset routine locks up the omap i2c controller, since the lp5523 will stop responding in the middle of an ongoing communication:

static void lp55xx_reset_device(struct lp55xx_chip *chip)

{
	struct lp55xx_device_config *cfg = chip->cfg;
	u8 addr = cfg->reset.addr;
	u8 val  = cfg->reset.val;

	/* no error checking here because no ACK from the device after reset */
	lp55xx_write(chip, addr, val);
}

Since tpa6130a2 is on the same i2c bus, it would be affected by this. You can check this by just commenting out the call to lp55xx_reset_device() in the probe function, since it's not needed on N900 (chip reset is done via enable gpio anyways).

I'm pretty sure, there were no bus lock problems when I added lp5523 to N900 dts, so this having problems with this is probably a regression in the omap-i2c driver.

-- Sebastian

... That error occurs randomly, not
always. When I see it next time, I will try to comment that function if it happens...

Hi,

Quoted 3 lines · Pali Rohár

... That error occurs randomly, not
always. When I see it next time, I will try to comment that function if it happens...

IIRC it is easier to cause it by booting to stock kernel first and then rebooting to mainline (without power-down in between)

Hi Pali,

Quoted 21 lines · Pali Rohár

Quoted 20 lines · On Tuesday 04 August 2015 09:02:39 Peter Ujfalusi wrote:

Quoted 4 lines · Jarkko Nikula

It is well possible that some regression got introduced to TPA6130A2 I2C communication over the years without nobody than you now notices. We used to do QA back in Meego N900 days but that was pre 3.x kernels.

No major changes has been done to the tpa driver during the past years... I wanted to do some updates, like moving it to regmap, but as you said, n900 is the only user (and n9) and I do not feel comfortable to hack on a device where I do not have serial console... And I'm using the n900 time to time also.

Quoted 7 lines

Quoted 3 lines

So maybe something similar? Kernel expects that some PM or regulator parts are initialized, but they are only sometimes? Just speculation...

I'm thinking the same. I could figure SCL could be stuck low if TPA or some other chip connected to the same I2C bus is without power and is pulling I2C signals down.

Quoted 7 lines

Quoted 3 lines

What would happen with the SCL stuck on i2c.2 bus if you remove the tpa driver from the kernel? If you remove the other drivers for the devices on i2c.2?

Hi Peter and Jarkko! Do you have some code samples for testing? Or something else which I can test? This problem is still reproducible on more N900 devices and I would like to see it fixed.

I have not seen your error with N900, but while working on N950 I noticed similar problems when I added lp5523. I think the lp5523 reset routine locks up the omap i2c controller, since the lp5523 will stop responding in the middle of an ongoing communication:

static void lp55xx_reset_device(struct lp55xx_chip *chip)

{
	struct lp55xx_device_config *cfg = chip->cfg;
	u8 addr = cfg->reset.addr;
	u8 val  = cfg->reset.val;

	/* no error checking here because no ACK from the device after reset

*/ lp55xx_write(chip, addr, val);
}

Since tpa6130a2 is on the same i2c bus, it would be affected by this. You can check this by just commenting out the call to lp55xx_reset_device() in the probe function, since it's not needed on N900 (chip reset is done via enable gpio anyways).

I'm pretty sure, there were no bus lock problems when I added lp5523 to N900 dts, so this having problems with this is probably a regression in the omap-i2c driver.

-- Sebastian

Hi Sebastian! Commenting calling lp55xx_reset_device function did not helped. Still getting that error.

Tony, Peter, Jarkko: can you reproduce this problem? I'm really stucked here... do not know where is problem or how to fix it. What we know that
it happens when rebooting from stock Nokia kernel (2.6.28) to upstream.

Does the i2c communication breaks with DT _and_ non DT boot?

Does the i2c communication breaks with DT _and_ non DT boot?

Hi! We found out that tpa6130a2 device is being initialized before i2c_2 bus is initialized. So that is reason why tpa6130a2 fails...

Any idea why kernel first try to initialize one i2c device even before bus itself is initialized?

Hi,

Quoted 2 lines · Pali Rohár

Hi! We found out that tpa6130a2 device is being initialized before i2c_2 bus is initialized. So that is reason why tpa6130a2 fails...

What do you mean by initialize? A call to tpa6130a2_probe()? In that case I wonder about client->adapter. Is it NULL?

---------------

I just had another look at the driver and I think there is a race condition for tpa6130a2_add_controls() and tpa6130a2_stereo_enable().

As far as I can see both functions check for "tpa6130a2_client != NULL". tpa6130a2_client is set before the probe function has finished, though. Simplified probe:

...
tpa6130a2_client = client;
set_default_regs();
acquire_power_gpio();
acquire_regulator();
tpa6130a2_power(1); // needs tpa6130a2_client
check_device();
tpa6130a2_power(0); // needs tpa6130a2_client
...

If tpa6130a2_add_controls() or tpa6130a2_stereo_enable() is called after tpa6130a2 probe has started, but before probe has completed, tpa6130a2_client is set, but not yet initialized. The race condition can be fixed easily by moving the tpa6130a2_client assignment directly after the regulator acquisition.

Hi,

Quoted 9 lines · Sebastian Reichel

Hi,

Quoted 2 lines · Pali Rohár

Hi! We found out that tpa6130a2 device is being initialized before i2c_2 bus is initialized. So that is reason why tpa6130a2 fails...

What do you mean by initialize? A call to tpa6130a2_probe()? In that case I wonder about client->adapter. Is it NULL?

This is (a part of) the log when tpa6130a2 fails to initialize:

Jan  1 08:03:07 Nokia-N900 kernel: [    1.928344] twl 1-0048: PIH (irq 
23) chaining IRQs 340..348
Jan  1 08:03:07 Nokia-N900 kernel: [    1.934326] twl 1-0048: power (irq 
345) chaining IRQs 348..355
Jan  1 08:03:07 Nokia-N900 kernel: [    2.498504] twl4030_gpio 
twl4030-gpio: gpio (irq 340) chaining IRQs 356..373
Jan  1 08:03:07 Nokia-N900 kernel: [    2.858215] twl4030_usb 
48070000.i2c:twl@48:twl4030-usb: Initialized TWL4030 USB module
Jan  1 08:03:07 Nokia-N900 kernel: [    2.888702] input: 

twl4030_pwrbutton as /devices/platform/68000000.ocp/48070000.i2c/i2c-1/1-0048/48070000.i2c:twl@48:pwrbutton/input/input0

Jan  1 08:03:07 Nokia-N900 kernel: [    2.903594] input: TWL4030 Keypad 

as /devices/platform/68000000.ocp/48070000.i2c/i2c-1/1-0048/48070000.i2c:twl@48:keypad/input/input1

Jan  1 08:03:07 Nokia-N900 kernel: [    3.148040] 
48070000.i2c:twl@48:madc supply vusb3v1 not found, using dummy regulator
Jan  1 08:03:07 Nokia-N900 kernel: [    6.997985] omap_i2c 48070000.i2c: 
bus 1 rev3.3 at 2200 kHz
Jan  1 08:03:07 Nokia-N900 kernel: [    7.010528] tpa6130a2 2-0060: 
Write failed
Jan  1 08:03:07 Nokia-N900 kernel: [    7.015563] omap_i2c 48072000.i2c: 
bus 2 rev3.3 at 100 kHz
Jan  1 08:03:07 Nokia-N900 kernel: [    7.023742] omap_i2c 48060000.i2c: 

bus 3 rev3.3 at 400 kHz

Now, it is either tpa6130a2 probe() is called before i2c-2 is initialized or i2c driver first probes devices on the bus and only then logs successful probe ("omap_i2c 48072000.i2c: bus 2 rev3.3 at 100 kHz") in our case.

Quoted 26 lines

---------------

I just had another look at the driver and I think there is a race condition for tpa6130a2_add_controls() and tpa6130a2_stereo_enable().

As far as I can see both functions check for "tpa6130a2_client != NULL". tpa6130a2_client is set before the probe function has finished, though. Simplified probe:

...
tpa6130a2_client = client;
set_default_regs();
acquire_power_gpio();
acquire_regulator();
tpa6130a2_power(1); // needs tpa6130a2_client
check_device();
tpa6130a2_power(0); // needs tpa6130a2_client
...

If tpa6130a2_add_controls() or tpa6130a2_stereo_enable() is called after tpa6130a2 probe has started, but before probe has completed, tpa6130a2_client is set, but not yet initialized. The race condition can be fixed easily by moving the tpa6130a2_client assignment directly after the regulator acquisition.

No-no :) take a look on i2c-omap.c

	r = i2c_add_numbered_adapter(adap);

^^^^ here you see messages from tpa6130a2 (create i2c devices & probe if drivers are ready)

	if (r) {
		dev_err(omap->dev, "failure adding adapter\n");
		goto err_unuse_clocks;
	}

	dev_info(omap->dev, "bus %d rev%d.%d at %d kHz\n", adap->nr,
		 major, minor, omap->speed);

^^^^ and here "omap_i2c 48072000.i2c: bus 2 rev3.3 at 100 kHz"

so everything is ok with probe order

Jan  1 06:01:43 Nokia-N900 kernel: [    6.947998] omap_i2c 48070000.i2c: bus 1 rev3.3 at 2200 kHz
Jan  1 06:01:43 Nokia-N900 kernel: [    6.960632] tpa6130a2 2-0060: Write failed
Jan  1 06:01:43 Nokia-N900 kernel: [    6.965026] CPU: 0 PID: 6 Comm: kworker/u2:0 Not tainted 4.5.0-rc5+ #26
Jan  1 06:01:43 Nokia-N900 kernel: [    6.972106] Hardware name: Nokia RX-51 board
Jan  1 06:01:43 Nokia-N900 kernel: [    6.976684] Workqueue: deferwq deferred_probe_work_func
Jan  1 06:01:43 Nokia-N900 kernel: [    6.982299] [<c0013c18>] (unwind_backtrace) from [<c0011f38>] (show_stack+0x10/0x14)
Jan  1 06:01:43 Nokia-N900 kernel: [    6.990570] [<c0011f38>] (show_stack) from [<c0390884>] (tpa6130a2_i2c_write+0x58/0x90)
Jan  1 06:01:43 Nokia-N900 kernel: [    6.999114] [<c0390884>] (tpa6130a2_i2c_write) from [<c0390968>] (tpa6130a2_power+0xac/0x1c4)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.008239] [<c0390968>] (tpa6130a2_power) from [<c0390d80>] (tpa6130a2_probe+0x144/0x234)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.017059] [<c0390d80>] (tpa6130a2_probe) from [<c032b650>] (i2c_device_probe+0x170/0x1b8)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.025939] [<c032b650>] (i2c_device_probe) from [<c02a7bd4>] (driver_probe_device+0x120/0x2b0)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.035247] [<c02a7bd4>] (driver_probe_device) from [<c02a62c8>] (bus_for_each_drv+0x48/0x8c)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.044311] [<c02a62c8>] (bus_for_each_drv) from [<c02a7a20>] (__device_attach+0x88/0xf8)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.053009] [<c02a7a20>] (__device_attach) from [<c02a70b0>] (bus_probe_device+0x28/0x80)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.061737] [<c02a70b0>] (bus_probe_device) from [<c02a5680>] (device_add+0x3c0/0x55c)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.070159] [<c02a5680>] (device_add) from [<c032cf28>] (i2c_new_device+0xf8/0x198)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.078308] [<c032cf28>] (i2c_new_device) from [<c032d4e8>] (i2c_register_adapter+0x2d0/0x47c)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.087432] [<c032d4e8>] (i2c_register_adapter) from [<c032f084>] (omap_i2c_probe+0x54c/0x64c)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.096618] [<c032f084>] (omap_i2c_probe) from [<c02a93bc>] (platform_drv_probe+0x58/0xa0)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.105438] [<c02a93bc>] (platform_drv_probe) from [<c02a7bd4>] (driver_probe_device+0x120/0x2b0)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.114929] [<c02a7bd4>] (driver_probe_device) from [<c02a62c8>] (bus_for_each_drv+0x48/0x8c)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.123962] [<c02a62c8>] (bus_for_each_drv) from [<c02a7a20>] (__device_attach+0x88/0xf8)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.132659] [<c02a7a20>] (__device_attach) from [<c02a70b0>] (bus_probe_device+0x28/0x80)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.141357] [<c02a70b0>] (bus_probe_device) from [<c02a7508>] (deferred_probe_work_func+0x58/0x84)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.150939] [<c02a7508>] (deferred_probe_work_func) from [<c0042a50>] (process_one_work+0x1c4/0x324)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.160705] [<c0042a50>] (process_one_work) from [<c0042ef4>] (worker_thread+0x314/0x4a8)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.169433] [<c0042ef4>] (worker_thread) from [<c00474e4>] (kthread+0xcc/0xe0)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.177154] [<c00474e4>] (kthread) from [<c000f218>] (ret_from_fork+0x14/0x3c)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.184783] tpa6130a2 2-0060: Failed to initialize chip
Jan  1 06:01:43 Nokia-N900 kernel: [    7.190551] tpa6130a2: probe of 2-0060 failed with error -121
Jan  1 06:01:43 Nokia-N900 kernel: [    7.197174] omap_i2c 48072000.i2c: bus 2 rev3.3 at 100 kHz

now, the only thing I can think of remaining to test is the reset gpio set-up - I wonder if it is possible to be set in safe mode(or input, or... ?), so the reset is never deasserted.

mh both, the power gpio is turned off in tpa6130a2_power(0). I guess if you don't see the problem during probe() everything works?

I have another idea though: In opposit to the gpio, the regulator may also be referenced by something else/already enabled. I guess adding a sleep after the regulator_enable() is worth a try.

Also I wonder if the same happens, if you avoid having the module available during boot and instead load it once everything has settled. That would rule out any side-effects of other modules being probed on the same i2c bus.

Hi,

Quoted 4 lines · Sebastian Reichel

mh both, the power gpio is turned off in tpa6130a2_power(0). I guess if you don't see the problem during probe() everything works?

Quoted 8 lines

I have another idea though: In opposit to the gpio, the regulator may also be referenced by something else/already enabled. I guess adding a sleep after the regulator_enable() is worth a try.

Also I wonder if the same happens, if you avoid having the module available during boot and instead load it once everything has settled. That would rule out any side-effects of other modules being probed on the same i2c bus.

Well, I think I've figured it out - input pullups are not enabled on i2c bus pins, in stock kernel we have:

./devmem2 0x480021BC
Value at address 0x480021BC (0x4001f1bc): 0x1180118

./devmem2 0x480021C0
Value at address 0x480021C0 (0x4001f1c0): 0x1180118

in mainline

./devmem2 0x480021BC
Value at address 0x480021BC (0xb6ff01bc): 0x1000100

./devmem2 0x480021C0
Value at address 0x480021C0 (0xb6f6d1c0): 0x1000100

I wonder how i2c devices work at all :)

Will fix the board DTS file later on an will report

Quoted 2 lines · On Thursday 17 March 2016 09:56:22 Ivaylo Dimitrov wrote:

Hi,

Is camera on same bus as tpa? Maybe this is reason why camera is non-functional too?

Quoted 1 line

Will fix the board DTS file later on an will report

Thanks for investigation! Is that problem in both DTS and also legacy board code?

Hi,

Quoted 7 lines · Pali Rohár

Quoted 2 lines · On Thursday 17 March 2016 09:56:22 Ivaylo Dimitrov wrote:

Hi,

Is camera on same bus as tpa? Maybe this is reason why camera is non-functional too?

It doesn't matter, all the i2c busses are missing the pullups.

Quoted 5 lines

Quoted 1 line

Will fix the board DTS file later on an will report

Thanks for investigation! Is that problem in both DTS and also legacy board code?

I guess legacy board code does not set i2c pull-ups either, as we have that problem since the beginning of time. Anyway, let me first check if enabling pullups really solves the issue, I hope to test that in a couple of hours when I am back home.

Or you can do it yourself, it is just a matter of replacing PIN_INPUT with
PIN_INPUT_PULLUP in https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/omap3-n900.dts?id=refs/tags/v4.5#n199
for all the 3 ic2 busses

Check the schematics. If the hardware has external pull-ups on a line then don't enable the internal pull-ups. Otherwise both the external and intenal pulls are parallel the pull value will be wrong. My guess is that on n900 all the i2c lines have external pulls.

Hi,

Quoted 6 lines · Tony Lindgren

Check the schematics. If the hardware has external pull-ups on a line then don't enable the internal pull-ups. Otherwise both the external and intenal pulls are parallel the pull value will be wrong. My guess is that on n900 all the i2c lines have external pulls.

There are, 1k connected to VIO_18, but still, stock Nokia kernel enables the internal pull-ups as well. I doubt Nokia devs did that by mistake. Could it be that VIO_18 is disabled by the time TPA6130A2 is probed?

Also, what is the problem to have both internal and external pull-ups in parallel?

Seems like a bug to me. My bets are on the deferred probe related Peter posted.

You can calculate the parallel resistor value of the weak internal pull with the 1k external pull :)

If the i2c line pulls are wrong
the signal quality won't match the spec and you will be getting i2c bus errors. If you can read and write to the i2c chip, this is not the issue.

Hi,

Quoted 16 lines · Grygorii Strashko

No-no :) take a look on i2c-omap.c

    r = i2c_add_numbered_adapter(adap);

^^^^ here you see messages from tpa6130a2 (create i2c devices & probe if drivers are ready)

    if (r) {
        dev_err(omap->dev, "failure adding adapter\n");
        goto err_unuse_clocks;
    }

    dev_info(omap->dev, "bus %d rev%d.%d at %d kHz\n", adap->nr,
         major, minor, omap->speed);

Quoted 4 lines

^^^^ and here "omap_i2c 48072000.i2c: bus 2 rev3.3 at 100 kHz"

so everything is ok with probe order

Sorry for the noise then :)

here is the log with dump_stack() in tpa6130a2_i2c_write:

Jan  1 06:01:43 Nokia-N900 kernel: [    6.947998] omap_i2c 48070000.i2c: bus 1
rev3.3 at 2200 kHz
Jan  1 06:01:43 Nokia-N900 kernel: [    6.960632] tpa6130a2 2-0060: Write failed
Jan  1 06:01:43 Nokia-N900 kernel: [    6.965026] CPU: 0 PID: 6 Comm:
kworker/u2:0 Not tainted 4.5.0-rc5+ #26
Jan  1 06:01:43 Nokia-N900 kernel: [    6.972106] Hardware name: Nokia RX-51
board
Jan  1 06:01:43 Nokia-N900 kernel: [    6.976684] Workqueue: deferwq
deferred_probe_work_func
Jan  1 06:01:43 Nokia-N900 kernel: [    6.982299] [<c0013c18>]
(unwind_backtrace) from [<c0011f38>] (show_stack+0x10/0x14)
Jan  1 06:01:43 Nokia-N900 kernel: [    6.990570] [<c0011f38>] (show_stack)
from [<c0390884>] (tpa6130a2_i2c_write+0x58/0x90)
Jan  1 06:01:43 Nokia-N900 kernel: [    6.999114] [<c0390884>]
(tpa6130a2_i2c_write) from [<c0390968>] (tpa6130a2_power+0xac/0x1c4)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.008239] [<c0390968>]
(tpa6130a2_power) from [<c0390d80>] (tpa6130a2_probe+0x144/0x234)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.017059] [<c0390d80>]
(tpa6130a2_probe) from [<c032b650>] (i2c_device_probe+0x170/0x1b8)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.025939] [<c032b650>]
(i2c_device_probe) from [<c02a7bd4>] (driver_probe_device+0x120/0x2b0)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.035247] [<c02a7bd4>]
(driver_probe_device) from [<c02a62c8>] (bus_for_each_drv+0x48/0x8c)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.044311] [<c02a62c8>]
(bus_for_each_drv) from [<c02a7a20>] (__device_attach+0x88/0xf8)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.053009] [<c02a7a20>]
(__device_attach) from [<c02a70b0>] (bus_probe_device+0x28/0x80)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.061737] [<c02a70b0>]
(bus_probe_device) from [<c02a5680>] (device_add+0x3c0/0x55c)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.070159] [<c02a5680>] (device_add)
from [<c032cf28>] (i2c_new_device+0xf8/0x198)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.078308] [<c032cf28>]
(i2c_new_device) from [<c032d4e8>] (i2c_register_adapter+0x2d0/0x47c)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.087432] [<c032d4e8>]
(i2c_register_adapter) from [<c032f084>] (omap_i2c_probe+0x54c/0x64c)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.096618] [<c032f084>]
(omap_i2c_probe) from [<c02a93bc>] (platform_drv_probe+0x58/0xa0)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.105438] [<c02a93bc>]
(platform_drv_probe) from [<c02a7bd4>] (driver_probe_device+0x120/0x2b0)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.114929] [<c02a7bd4>]
(driver_probe_device) from [<c02a62c8>] (bus_for_each_drv+0x48/0x8c)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.123962] [<c02a62c8>]
(bus_for_each_drv) from [<c02a7a20>] (__device_attach+0x88/0xf8)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.132659] [<c02a7a20>]
(__device_attach) from [<c02a70b0>] (bus_probe_device+0x28/0x80)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.141357] [<c02a70b0>]
(bus_probe_device) from [<c02a7508>] (deferred_probe_work_func+0x58/0x84)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.150939] [<c02a7508>]
(deferred_probe_work_func) from [<c0042a50>] (process_one_work+0x1c4/0x324)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.160705] [<c0042a50>]
(process_one_work) from [<c0042ef4>] (worker_thread+0x314/0x4a8)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.169433] [<c0042ef4>] (worker_thread)
from [<c00474e4>] (kthread+0xcc/0xe0)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.177154] [<c00474e4>] (kthread) from
[<c000f218>] (ret_from_fork+0x14/0x3c)
Jan  1 06:01:43 Nokia-N900 kernel: [    7.184783] tpa6130a2 2-0060: Failed to
initialize chip
Jan  1 06:01:43 Nokia-N900 kernel: [    7.190551] tpa6130a2: probe of 2-0060
failed with error -121
Jan  1 06:01:43 Nokia-N900 kernel: [    7.197174] omap_i2c 48072000.i2c: bus 2

rev3.3 at 100 kHz

now, the only thing I can think of remaining to test is the reset gpio set-up - I wonder if it is possible to be set in safe mode(or input, or... ?), so the reset is never deasserted.

can you try this:

sound/soc/codecs/tpa6130a2.c +0 -0

index 11d85c5c787a..7f5881bff5d9 100644

Hi,

Quoted 11 lines

can you try this:

sound/soc/codecs/tpa6130a2.c +1 -0

index 11d85c5c787a..7f5881bff5d9 100644
@@ -386,6 +386,8 @@ static int tpa6130a2_probe(struct i2c_client *client,
                 data->power_gpio = pdata->power_gpio;
         } else if (np) {
                 data->power_gpio = of_get_named_gpio(np, "power-gpio", 0);
+               if (data->power_gpio == -EPROBE_DEFER)

sound/soc/codecs/tpa6130a2.c static int tpa6130a2_probe(struct i2c_client *client, · line 393

Quoted 5 lines

sound/soc/codecs/tpa6130a2.c +1 -0

+                       return data->power_gpio;
         } else {
                 dev_err(dev, "Platform data not set\n");
                 dump_stack();

Doesn't help :(

But enabling the pull via DT for the i2c2 works?

Quoted 1 line · On Fri Mar 18 12:33:14 2016 Peter Ujfalusi <[email protected]> wrote:

But enabling the pull via DT for the i2c2 works?

No :(. I even migrated the driver to regmap - no gain. Maybe i2c bus is blocked by another device held in reset. The next thing I am going to try is to deassert reset/power gpios on all the devices on i2c-2 bus, to see if it will make any difference. Or if you have any other ideas, please share.

Have you tried the ideas from <20160317004917.GA6750@earth> (Date:
Thu, 17 Mar 2016 01:49:18 +0100)?

Quoted 3 lines · Sebastian Reichel

Have you tried the ideas from <20160317004917.GA6750@earth> (Date:
Thu, 17 Mar 2016 01:49:18 +0100)?

To the extend I understood them :)

Regulator is V28_A, which is always-on, so it is enabled no matter what probe does. Anyway, I added a various delays after regulator_enable(), to no success.

I didn't try building the driver as a module, instead I played with bind/unbind functionality - makes no difference.

Did you by chance also test adding a delay after setting the power gpio? According to the datasheet:

Start-up time from shutdown: typical 5ms

Hi,

Quoted 6 lines · Ivaylo Dimitrov

On 18.03.2016 15:36, Sebastian Reichel wrote:

Regulator is V28_A, which is always-on, so it is enabled no matter what probe does. Anyway, I added a various delays after regulator_enable(), to no success.

Quoted 5 lines · Sebastian Reichel

Quoted 4 lines · Ivaylo Dimitrov

On 18.03.2016 15:36, Sebastian Reichel wrote:
Regulator is V28_A, which is always-on, so it is enabled no matter what probe does. Anyway, I added a various delays after regulator_enable(), to no success.

I guess we're getting closer - I put some printks in various functions in the twl-regulator.c, here is the result:

on power-up:

[    2.378601] twl4030ldo_get_voltage_sel VMMC2 vsel 0x00000008
[    2.384948] twl4030reg_enable VMMC2 grp 0x00000020
[    2.408416] twl4030ldo_get_voltage_sel VMMC2 vsel 0x00000008
[    7.196685] twl4030reg_is_enabled VMMC2 state 0x0000002e
[    7.202819] twl4030reg_is_enabled VMMC2 state 0x0000002e
[    7.209777] twl4030reg_is_enabled VMMC2 state 0x0000002e
[    7.215728] twl4030reg_is_enabled VMMC2 state 0x0000002e
[    7.223205] twl4030reg_is_enabled VMMC2 state 0x0000002e

after restart from stock kernel:

[    2.388610] twl4030ldo_get_voltage_sel VMMC2 vsel 0x0000000a
[    2.394958] twl4030reg_enable VMMC2 grp 0x00000028

[    2.418426] twl4030ldo_get_voltage_sel VMMC2 vsel 0x0000000a
[    7.186645] twl4030reg_is_enabled VMMC2 state 0x00000020
[    7.192718] twl4030reg_is_enabled VMMC2 state 0x00000020
[    7.199615] twl4030reg_is_enabled VMMC2 state 0x00000020
[    7.205535] twl4030reg_is_enabled VMMC2 state 0x00000020
[    7.212951] twl4030reg_is_enabled VMMC2 state 0x00000020

I don't see twl4030ldo_set_voltage_sel() for VMMC2(V28_A) regulator, though there are calls for VMMC1 and VAUX3.

So, it seems to me that V28_A is not enabled or correctly set-up and all devices connected to it does not function. And it looks like even after power-on VMMC2 is not correctly set-up - it is supposed to have voltage of 2.85V (10) but kernel leaves it to 2.60V (8). However my twl-fu ends here so any help is appreciated.

Hi,

Quoted 12 lines · Ivaylo Dimitrov

Quoted 5 lines · Sebastian Reichel

Quoted 4 lines · Ivaylo Dimitrov

On 18.03.2016 15:36, Sebastian Reichel wrote:
Regulator is V28_A, which is always-on, so it is enabled no matter what probe does. Anyway, I added a various delays after regulator_enable(), to no success.

I guess we're getting closer - I put some printks in various functions in the twl-regulator.c, here is the result:

on power-up:

Quoted 8 lines

[    2.378601] twl4030ldo_get_voltage_sel VMMC2 vsel 0x00000008
[    2.384948] twl4030reg_enable VMMC2 grp 0x00000020
[    2.408416] twl4030ldo_get_voltage_sel VMMC2 vsel 0x00000008
[    7.196685] twl4030reg_is_enabled VMMC2 state 0x0000002e
[    7.202819] twl4030reg_is_enabled VMMC2 state 0x0000002e
[    7.209777] twl4030reg_is_enabled VMMC2 state 0x0000002e
[    7.215728] twl4030reg_is_enabled VMMC2 state 0x0000002e
[    7.223205] twl4030reg_is_enabled VMMC2 state 0x0000002e

Ok, so normal power up results in running VMMC2 (always-on works), but voltage is not configured correctly. 2.6V is default according to the TRM. I think this is a "bug" in the regulator framework. It should setup the minimum allowed voltage before enabling the always-on regulator.

In case of the tpa6130a2/tpa6140a2 driver it may also be nice to add something like this to the driver (Vdd may be between 2.5V and 5.5V according to both datasheets):

if (regulator_can_change_voltage(data->supply))

     regulator_set_voltage(data->supply, 2500000, 5500000);

Quoted 4 lines

after restart from stock kernel:

[    2.388610] twl4030ldo_get_voltage_sel VMMC2 vsel 0x0000000a
[    2.394958] twl4030reg_enable VMMC2 grp 0x00000028

I had a quick glance at this. I think stock kernel put VMMC2 into sleep mode. Mainline kernel does not expect sleep mode being set and does not disable it.

Quoted 7 lines

[    2.418426] twl4030ldo_get_voltage_sel VMMC2 vsel 0x0000000a
[    7.186645] twl4030reg_is_enabled VMMC2 state 0x00000020
[    7.192718] twl4030reg_is_enabled VMMC2 state 0x00000020
[    7.199615] twl4030reg_is_enabled VMMC2 state 0x00000020
[    7.205535] twl4030reg_is_enabled VMMC2 state 0x00000020
[    7.212951] twl4030reg_is_enabled VMMC2 state 0x00000020

Quoted 2 lines

I don't see twl4030ldo_set_voltage_sel() for VMMC2(V28_A) regulator, though there are calls for VMMC1 and VAUX3.

I guess that's because the voltage is only configured if at least one regulator consumer requests anything specific.

Quoted 5 lines

So, it seems to me that V28_A is not enabled or correctly set-up and all devices connected to it does not function. And it looks like even after power-on VMMC2 is not correctly set-up - it is supposed to have voltage of 2.85V (10) but kernel leaves it to 2.60V (8). However my twl-fu ends here so any help is appreciated.

So in case of reboot from stock kernel voltage is already configured to 2.8V, but it does not work, because of the sleep mode.

Quoted 22 lines · Sebastian Reichel

Quoted 12 lines · Ivaylo Dimitrov

Quoted 5 lines · Sebastian Reichel

Quoted 4 lines · Ivaylo Dimitrov

On 18.03.2016 15:36, Sebastian Reichel wrote:
Regulator is V28_A, which is always-on, so it is enabled no matter what probe does. Anyway, I added a various delays after regulator_enable(), to no success.

I guess we're getting closer - I put some printks in various functions in the twl-regulator.c, here is the result:

on power-up:

Quoted 8 lines

[    2.378601] twl4030ldo_get_voltage_sel VMMC2 vsel 0x00000008
[    2.384948] twl4030reg_enable VMMC2 grp 0x00000020
[    2.408416] twl4030ldo_get_voltage_sel VMMC2 vsel 0x00000008
[    7.196685] twl4030reg_is_enabled VMMC2 state 0x0000002e
[    7.202819] twl4030reg_is_enabled VMMC2 state 0x0000002e
[    7.209777] twl4030reg_is_enabled VMMC2 state 0x0000002e
[    7.215728] twl4030reg_is_enabled VMMC2 state 0x0000002e
[    7.223205] twl4030reg_is_enabled VMMC2 state 0x0000002e

Quoted 6 lines

Ok, so normal power up results in running VMMC2 (always-on works), but voltage is not configured correctly. 2.6V is default according to the TRM. I think this is a "bug" in the regulator framework. It should setup the minimum allowed voltage before enabling the always-on regulator.

/sys/kernel/debug/regulator/regulator_summary shows 2850mV for V28_A, so I would remove the quotes. Also, always-on is because if V28_A regulator is turned off, there is a leakage through tlv320aic34 VIO. BTW one of the things I did while trying to find the problem, was to remove that always-on property from the DTS - it didn't help.

Quoted 7 lines

In case of the tpa6130a2/tpa6140a2 driver it may also be nice to add something like this to the driver (Vdd may be between 2.5V and 5.5V according to both datasheets):

if (regulator_can_change_voltage(data->supply))

    regulator_set_voltage(data->supply, 2500000, 5500000);

and add DT property for that voltage range, as max output power and harmonics depend on the supply voltage.

Quoted 5 lines

Quoted 4 lines

after restart from stock kernel:

[    2.388610] twl4030ldo_get_voltage_sel VMMC2 vsel 0x0000000a
[    2.394958] twl4030reg_enable VMMC2 grp 0x00000028

Quoted 4 lines

I had a quick glance at this. I think stock kernel put VMMC2 into sleep mode. Mainline kernel does not expect sleep mode being set and does not disable it.

Well, one would think that kernel should not have expectations on what would be the state of the hardware by the time it takes control over it, but setup everything needed instead.

Quoted 7 lines

Quoted 7 lines

[    2.418426] twl4030ldo_get_voltage_sel VMMC2 vsel 0x0000000a
[    7.186645] twl4030reg_is_enabled VMMC2 state 0x00000020
[    7.192718] twl4030reg_is_enabled VMMC2 state 0x00000020
[    7.199615] twl4030reg_is_enabled VMMC2 state 0x00000020
[    7.205535] twl4030reg_is_enabled VMMC2 state 0x00000020
[    7.212951] twl4030reg_is_enabled VMMC2 state 0x00000020

Quoted 6 lines

Quoted 2 lines

I don't see twl4030ldo_set_voltage_sel() for VMMC2(V28_A) regulator, though there are calls for VMMC1 and VAUX3.

I guess that's because the voltage is only configured if at least one regulator consumer requests anything specific.

But then the board DTS is simply ignored. Doesn't look good :)

Quoted 6 lines

Quoted 5 lines

So, it seems to me that V28_A is not enabled or correctly set-up and all devices connected to it does not function. And it looks like even after power-on VMMC2 is not correctly set-up - it is supposed to have voltage of 2.85V (10) but kernel leaves it to 2.60V (8). However my twl-fu ends here so any help is appreciated.

Quoted 3 lines

So in case of reboot from stock kernel voltage is already configured to 2.8V, but it does not work, because of the sleep mode.

Yeah, that sleep is pretty clear, I was rather asking - "any idea how to fix that?". Or it is someone else expected to fix it?

Quoted 30 lines · Ivaylo Dimitrov

Quoted 22 lines · Sebastian Reichel

Quoted 12 lines · Ivaylo Dimitrov

Quoted 5 lines · Sebastian Reichel

Quoted 4 lines · Ivaylo Dimitrov

On 18.03.2016 15:36, Sebastian Reichel wrote:
Regulator is V28_A, which is always-on, so it is enabled no matter what probe does. Anyway, I added a various delays after regulator_enable(), to no success.

I guess we're getting closer - I put some printks in various functions in the twl-regulator.c, here is the result:

on power-up:

Quoted 8 lines

[    2.378601] twl4030ldo_get_voltage_sel VMMC2 vsel 0x00000008
[    2.384948] twl4030reg_enable VMMC2 grp 0x00000020
[    2.408416] twl4030ldo_get_voltage_sel VMMC2 vsel 0x00000008
[    7.196685] twl4030reg_is_enabled VMMC2 state 0x0000002e
[    7.202819] twl4030reg_is_enabled VMMC2 state 0x0000002e
[    7.209777] twl4030reg_is_enabled VMMC2 state 0x0000002e
[    7.215728] twl4030reg_is_enabled VMMC2 state 0x0000002e
[    7.223205] twl4030reg_is_enabled VMMC2 state 0x0000002e

Quoted 6 lines

Ok, so normal power up results in running VMMC2 (always-on works), but voltage is not configured correctly. 2.6V is default according to the TRM. I think this is a "bug" in the regulator framework. It should setup the minimum allowed voltage before enabling the always-on regulator.

Quoted 5 lines

/sys/kernel/debug/regulator/regulator_summary shows 2850mV for V28_A, so I would remove the quotes. Also, always-on is because if V28_A regulator is turned off, there is a leakage through tlv320aic34 VIO. BTW one of the things I did while trying to find the problem, was to remove that always-on property from the DTS - it didn't help.

Right thinking about it, the voltage must also be configured for the non always-on cases. So it's not a problem with the regulator framework, but with twl-regulator's probe function, that should take care of this.

Quoted 10 lines

Quoted 7 lines

In case of the tpa6130a2/tpa6140a2 driver it may also be nice to add something like this to the driver (Vdd may be between 2.5V and 5.5V according to both datasheets):

if (regulator_can_change_voltage(data->supply))

    regulator_set_voltage(data->supply, 2500000, 5500000);

and add DT property for that voltage range, as max output power and harmonics depend on the supply voltage.

I guess that's 2nd step.

Quoted 5 lines

Quoted 5 lines

Quoted 4 lines

after restart from stock kernel:

[    2.388610] twl4030ldo_get_voltage_sel VMMC2 vsel 0x0000000a
[    2.394958] twl4030reg_enable VMMC2 grp 0x00000028

Quoted 8 lines

Quoted 4 lines

I had a quick glance at this. I think stock kernel put VMMC2 into sleep mode. Mainline kernel does not expect sleep mode being set and does not disable it.

Well, one would think that kernel should not have expectations on what would be the state of the hardware by the time it takes control over it, but setup everything needed instead.

I thought it's obvious, that this is not the desired behaviour :)

Quoted 26 lines

Quoted 7 lines

Quoted 7 lines

[    2.418426] twl4030ldo_get_voltage_sel VMMC2 vsel 0x0000000a
[    7.186645] twl4030reg_is_enabled VMMC2 state 0x00000020
[    7.192718] twl4030reg_is_enabled VMMC2 state 0x00000020
[    7.199615] twl4030reg_is_enabled VMMC2 state 0x00000020
[    7.205535] twl4030reg_is_enabled VMMC2 state 0x00000020
[    7.212951] twl4030reg_is_enabled VMMC2 state 0x00000020

Quoted 6 lines

Quoted 2 lines

I don't see twl4030ldo_set_voltage_sel() for VMMC2(V28_A) regulator, though there are calls for VMMC1 and VAUX3.

I guess that's because the voltage is only configured if at least one regulator consumer requests anything specific.

But then the board DTS is simply ignored. Doesn't look good :)

Quoted 9 lines

Quoted 5 lines

So, it seems to me that V28_A is not enabled or correctly set-up and all devices connected to it does not function. And it looks like even after power-on VMMC2 is not correctly set-up - it is supposed to have voltage of 2.85V (10) but kernel leaves it to 2.60V (8). However my twl-fu ends here so any help is appreciated.

So in case of reboot from stock kernel voltage is already configured to 2.8V, but it does not work, because of the sleep mode.

Quoted 2 lines

Yeah, that sleep is pretty clear, I was rather asking - "any idea how to fix that?". Or it is someone else expected to fix it?

You may have noticed, that I included Mark and Liam. I hope they can give some feedback. I think there are two bugs:

1. twl_probe() should setup a default voltage based on DT information.

2. if regulator is in sleep mode, regulator enable should disable sleep mode.

Right thinking about it, the voltage must also be configured for the non always-on cases. So it's not a problem with the regulator framework, but with twl-regulator's probe function, that should take care of this.

Quoted 2 lines · Ivaylo Dimitrov

[    7.215728] twl4030reg_is_enabled VMMC2 state 0x0000002e
[    7.223205] twl4030reg_is_enabled VMMC2 state 0x0000002e

Ok, so normal power up results in running VMMC2 (always-on works), but voltage is not configured correctly. 2.6V is default according to the TRM. I think this is a "bug" in the regulator framework. It should setup the minimum allowed voltage before enabling the always-on regulator.

In case of the tpa6130a2/tpa6140a2 driver it may also be nice to add something like this to the driver (Vdd may be between 2.5V and 5.5V according to both datasheets):

if (regulator_can_change_voltage(data->supply))

    regulator_set_voltage(data->supply, 2500000, 5500000);

No, if the voltage is variable we can't tell what the current constraints are without something telling us so we just don't vary the voltage until we're told to do this. If we immediately lower the voltage to the minimum supported voltage that's going to break things.

Quoted 4 lines · Mark Brown

No, if the voltage is variable we can't tell what the current constraints are without something telling us so we just don't vary the voltage until we're told to do this. If we immediately lower the voltage to the minimum supported voltage that's going to break things.

There are constraints set by the board DTS. Isn't it reasonable the framework to set the voltage to minimum voltage from the dts if the current set one is bellow it?

Quoted 6 lines · Ivaylo Dimitrov

Quoted 4 lines · Mark Brown

No, if the voltage is variable we can't tell what the current constraints are without something telling us so we just don't vary the voltage until we're told to do this. If we immediately lower the voltage to the minimum supported voltage that's going to break things.

Quoted 3 lines

There are constraints set by the board DTS. Isn't it reasonable the framework to set the voltage to minimum voltage from the dts if the current set one is bellow it?

Yes, if it's out of bounds for the constraints we should bring it up/down to the minimum/maximum (when copying people into a thread it's a good idea to explain what the problem you are trying to solve is, especially if you're throwing around bodges).

Hi Mark,

Quoted 8 lines · Mark Brown

Quoted 6 lines · Ivaylo Dimitrov

Quoted 4 lines · Mark Brown

No, if the voltage is variable we can't tell what the current constraints are without something telling us so we just don't vary the voltage until we're told to do this. If we immediately lower the voltage to the minimum supported voltage that's going to break things.

Quoted 8 lines

Quoted 3 lines

There are constraints set by the board DTS. Isn't it reasonable the framework to set the voltage to minimum voltage from the dts if the current set one is bellow it?

Yes, if it's out of bounds for the constraints we should bring it up/down to the minimum/maximum (when copying people into a thread it's a good idea to explain what the problem you are trying to solve is, especially if you're throwing around bodges).

We have this regulator definition in omap3-n900.dts:

&vmmc2 {
	regulator-name = "V28_A";
	regulator-min-microvolt = <2800000>;
	regulator-max-microvolt = <3000000>;
	regulator-always-on; /* due VIO leak to AIC34 VDDs */
};

The regulator is enabled during probe, but the voltage is not configured. The default reset voltage of the regulator is 2.6V. So basically when the regulator is enabled, it uses a voltage, which is out of the DT specified range.

We also have a second problem: If the system has been rebooted from Nokia's stock kernel the regulator is left in STANDBY mode. Since the mode is not configured during probe, it results in different problems. According to my understanding it can be fixed trivially by adding

&vmmc2 {
     regulator-initial-mode = <2>;
};

Quoted 41 lines · Sebastian Reichel

Hi Mark,

Quoted 13 lines · Mark Brown

Quoted 11 lines · Ivaylo Dimitrov

Quoted 4 lines · Mark Brown

No, if the voltage is variable we can't tell what the current constraints are without something telling us so we just don't vary the voltage until we're told to do this. If we immediately lower the voltage to the minimum supported voltage that's going to break things.

There are constraints set by the board DTS. Isn't it reasonable the framework to set the voltage to minimum voltage from the dts if the current
set one is bellow it?

Quoted 4 lines

Yes, if it's out of bounds for the constraints we should bring it up/down to the minimum/maximum (when copying people into a thread it's a good idea to explain what the problem you are trying to solve is, especially if you're throwing around bodges).

We have this regulator definition in omap3-n900.dts:

&vmmc2 {
    regulator-name = "V28_A";
    regulator-min-microvolt = <2800000>;
    regulator-max-microvolt = <3000000>;
    regulator-always-on; /* due VIO leak to AIC34 VDDs */
};

The regulator is enabled during probe, but the voltage is not configured. The default reset voltage of the regulator is 2.6V. So basically when the regulator is enabled, it uses a voltage, which is out of the DT specified range.

We also have a second problem: If the system has been rebooted from Nokia's stock kernel the regulator is left in STANDBY mode. Since the mode is not configured during probe, it results in different problems. According to my understanding it can be fixed trivially by adding

Quoted 4 lines

&vmmc2 {
     regulator-initial-mode = <2>;
};

doesn't work:

"regulator-vmmc2: mapping for mode 2 not defined"

twl-regulator is missing .of_map_mode function.

Also, if we go that route, we should set the initial modes for all the regulators, not only vmmc2 (and not only for N900), as we don't really know what is the status of regulators at startup. I think a better approach is if regulator framework sets all always-on regulators to enabled, unless stated otherwise (which it already does iiuc).

I think there is a bug in twl-regulator twl4030reg_enable() and/or twl4030reg_is_enabled() - the latter only checks if DEV_GRP is P1, but not for the actual state of the regulator (bits 3:0). Also, what looks suspicious to me is that all the regulators are put in P1 device group. Legacy board code spreads the regulators all over the groups, so maybe this is simply a regression compared to legacy boot.

I just had another look at the driver and I think there is a race condition for tpa6130a2_add_controls() and tpa6130a2_stereo_enable().

As far as I can see both functions check for "tpa6130a2_client != NULL". tpa6130a2_client is set before the probe function has finished, though. Simplified probe:

...
tpa6130a2_client = client;
set_default_regs();
acquire_power_gpio();
acquire_regulator();
tpa6130a2_power(1); // needs tpa6130a2_client
check_device();
tpa6130a2_power(0); // needs tpa6130a2_client
...

If tpa6130a2_add_controls() or tpa6130a2_stereo_enable() is called after tpa6130a2 probe has started, but before probe has completed, tpa6130a2_client is set, but not yet initialized. The race condition can be fixed easily by moving the tpa6130a2_client assignment directly after the regulator acquisition.

-- Sebastian

Maybe some power management problem? Something is not always initialized correctly?

I remember that there is some problem (maybe in NoLo - Nokia bootloader) that sometimes chainloaded U-Boot (booted via NoLo) is not able to initialize mmc chip (all read operation fails). In U-Boot I added some code to enable some parts in twl4030 regulator and after that mmc is working always...

Hi!

Quoted 8 lines

Maybe some power management problem? Something is not always initialized correctly?

I remember that there is some problem (maybe in NoLo - Nokia bootloader) that sometimes chainloaded U-Boot (booted via NoLo) is not able to initialize mmc chip (all read operation fails). In U-Boot I added some code to enable some parts in twl4030 regulator and after that mmc is working always...

Could I get some details? Thay code should be moved to kernel, I'd really like mmc to work, no matter how machine was booted.

Best regards,

Pavel