Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1351140 > unrolled thread

Re: Nokia N900 - audio TPA6130A2 problems

Started bySebastian Reichel <sre@kernel.org>
First post2016-03-06 16:30 +0100
Last post2016-04-01 12:50 +0200
Articles 20 on this page of 41 — 8 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: Nokia N900 - audio TPA6130A2 problems Sebastian Reichel <sre@kernel.org> - 2016-03-06 16:30 +0100
    Re: Nokia N900 - audio TPA6130A2 problems Pali Rohár <pali.rohar@gmail.com> - 2016-03-07 13:00 +0100
      Re: Nokia N900 - audio TPA6130A2 problems Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> - 2016-03-08 07:50 +0100
        Re: Nokia N900 - audio TPA6130A2 problems Pali Rohár <pali.rohar@gmail.com> - 2016-03-12 13:50 +0100
    Re: Nokia N900 - audio TPA6130A2 problems Pali Rohár <pali.rohar@gmail.com> - 2016-03-12 13:50 +0100
      Re: Nokia N900 - audio TPA6130A2 problems Peter Ujfalusi <peter.ujfalusi@ti.com> - 2016-03-14 11:10 +0100
        Re: Nokia N900 - audio TPA6130A2 problems Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> - 2016-03-14 18:10 +0100
        Re: Nokia N900 - audio TPA6130A2 problems Pali Rohár <pali.rohar@gmail.com> - 2016-03-16 14:40 +0100
          Re: Nokia N900 - audio TPA6130A2 problems Sebastian Reichel <sre@kernel.org> - 2016-03-16 15:50 +0100
            Re: Nokia N900 - audio TPA6130A2 problems Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> - 2016-03-16 19:30 +0100
              Re: Nokia N900 - audio TPA6130A2 problems Grygorii Strashko <grygorii.strashko@ti.com> - 2016-03-16 19:40 +0100
                Re: Nokia N900 - audio TPA6130A2 problems Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> - 2016-03-16 21:00 +0100
                  Re: Nokia N900 - audio TPA6130A2 problems Sebastian Reichel <sre@kernel.org> - 2016-03-17 01:50 +0100
                    Re: Nokia N900 - audio TPA6130A2 problems Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> - 2016-03-17 09:00 +0100
                      Re: Nokia N900 - audio TPA6130A2 problems Pali Rohár <pali.rohar@gmail.com> - 2016-03-17 14:10 +0100
                        Re: Nokia N900 - audio TPA6130A2 problems Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> - 2016-03-17 14:20 +0100
                          Re: Nokia N900 - audio TPA6130A2 problems Tony Lindgren <tony@atomide.com> - 2016-03-17 14:40 +0100
                            Re: Nokia N900 - audio TPA6130A2 problems Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> - 2016-03-17 15:00 +0100
                              Re: Nokia N900 - audio TPA6130A2 problems Tony Lindgren <tony@atomide.com> - 2016-03-17 15:40 +0100
                                Re: Nokia N900 - audio TPA6130A2 problems Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> - 2016-03-17 16:00 +0100
                  Re: Nokia N900 - audio TPA6130A2 problems Peter Ujfalusi <peter.ujfalusi@ti.com> - 2016-03-17 09:00 +0100
                    Re: Nokia N900 - audio TPA6130A2 problems Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> - 2016-03-17 18:30 +0100
                      Re: Nokia N900 - audio TPA6130A2 problems Peter Ujfalusi <peter.ujfalusi@ti.com> - 2016-03-18 11:40 +0100
                        Re: Nokia N900 - audio TPA6130A2 problems Ивайло Димитров   <ivo.g.dimitrov.75@gmail.com> - 2016-03-18 14:20 +0100
                          Re: Nokia N900 - audio TPA6130A2 problems Sebastian Reichel <sre@kernel.org> - 2016-03-18 14:40 +0100
                            Re: Nokia N900 - audio TPA6130A2 problems Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> - 2016-03-18 14:50 +0100
                              Re: Nokia N900 - audio TPA6130A2 problems Sebastian Reichel <sre@kernel.org> - 2016-03-18 16:10 +0100
                                Re: Nokia N900 - audio TPA6130A2 problems Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> - 2016-03-18 17:00 +0100
                                Re: Nokia N900 - audio TPA6130A2 problems Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> - 2016-03-19 10:00 +0100
                                  Re: Nokia N900 - audio TPA6130A2 problems Sebastian Reichel <sre@kernel.org> - 2016-03-20 06:20 +0100
                                    Re: Nokia N900 - audio TPA6130A2 problems Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> - 2016-03-20 20:50 +0100
                                      Re: Nokia N900 - audio TPA6130A2 problems Sebastian Reichel <sre@kernel.org> - 2016-03-21 01:10 +0100
                                        Re: Nokia N900 - audio TPA6130A2 problems Sebastian Reichel <sre@kernel.org> - 2016-03-21 02:50 +0100
                                        Re: Nokia N900 - audio TPA6130A2 problems Mark Brown <broonie@kernel.org> - 2016-03-21 13:10 +0100
                                    Re: Nokia N900 - audio TPA6130A2 problems Mark Brown <broonie@kernel.org> - 2016-03-21 12:50 +0100
                                      Re: Nokia N900 - audio TPA6130A2 problems Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> - 2016-03-21 14:40 +0100
                                        Re: Nokia N900 - audio TPA6130A2 problems Mark Brown <broonie@kernel.org> - 2016-03-21 14:50 +0100
                                          Re: Nokia N900 - audio TPA6130A2 problems Sebastian Reichel <sre@kernel.org> - 2016-03-21 16:00 +0100
                                            Re: Nokia N900 - audio TPA6130A2 problems Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> - 2016-03-21 20:40 +0100
                                              Re: Nokia N900 - audio TPA6130A2 problems Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> - 2016-03-22 09:10 +0100
            Race condition in TPA6130A2 (Was: Re: Nokia N900 - audio TPA6130A2  problems) Pali Rohár <pali.rohar@gmail.com> - 2016-04-01 12:50 +0200

Page 1 of 3  [1] 2 3  Next page →


#1351140 — Re: Nokia N900 - audio TPA6130A2 problems

FromSebastian Reichel <sre@kernel.org>
Date2016-03-06 16:30 +0100
SubjectRe: Nokia N900 - audio TPA6130A2 problems
Message-ID<r9IN3-6MU-5@gated-at.bofh.it>

[Multipart message — attachments visible in raw view] — view raw

Hi Pali,

On Tue, Jan 05, 2016 at 12:34:12AM +0100, Pali Rohár wrote:
> On Tuesday 04 August 2015 09:02:39 Peter Ujfalusi wrote:
> > On 08/03/2015 09:48 PM, Jarkko Nikula wrote:
> > > 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.
> > 
> > >> 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.
> > 
> > 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

[toc] | [next] | [standalone]


#1351561

FromPali Rohár <pali.rohar@gmail.com>
Date2016-03-07 13:00 +0100
Message-ID<ra1Zn-2mv-1@gated-at.bofh.it>
In reply to#1351140
On Sunday 06 March 2016 16:23:39 Sebastian Reichel wrote:
> Hi Pali,
> 
> On Tue, Jan 05, 2016 at 12:34:12AM +0100, Pali Rohár wrote:
> > On Tuesday 04 August 2015 09:02:39 Peter Ujfalusi wrote:
> > > On 08/03/2015 09:48 PM, Jarkko Nikula wrote:
> > > > 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.
> > > 
> > > >> 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.
> > > 
> > > 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! Thank you for info. That error occurs randomly, not
always. When I see it next time, I will try to comment that function if
it happens...

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]


#1352671

FromIvaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Date2016-03-08 07:50 +0100
Message-ID<rajCV-5Gr-1@gated-at.bofh.it>
In reply to#1351561
Hi,

On  7.03.2016 13:59, Pali Rohár wrote:
>
> ... 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)

Ivo

[toc] | [prev] | [next] | [standalone]


#1356416

FromPali Rohár <pali.rohar@gmail.com>
Date2016-03-12 13:50 +0100
Message-ID<rbR9v-65c-7@gated-at.bofh.it>
In reply to#1352671

[Multipart message — attachments visible in raw view] — view raw

On Tuesday 08 March 2016 07:45:32 Ivaylo Dimitrov wrote:
> Hi,
> 
> On  7.03.2016 13:59, Pali Rohár wrote:
> > ... 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)

You are right, I'm getting it maybe always when I reboot from Nokia 
stock kernel to upstream...

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]


#1356418

FromPali Rohár <pali.rohar@gmail.com>
Date2016-03-12 13:50 +0100
Message-ID<rbR9w-65c-17@gated-at.bofh.it>
In reply to#1351140

[Multipart message — attachments visible in raw view] — view raw

On Sunday 06 March 2016 16:23:39 Sebastian Reichel wrote:
> Hi Pali,
> 
> On Tue, Jan 05, 2016 at 12:34:12AM +0100, Pali Rohár wrote:
> > On Tuesday 04 August 2015 09:02:39 Peter Ujfalusi wrote:
> > > On 08/03/2015 09:48 PM, Jarkko Nikula wrote:
> > > > 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.
> > > 
> > > >> 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.
> > > 
> > > 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.

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]


#1357114

FromPeter Ujfalusi <peter.ujfalusi@ti.com>
Date2016-03-14 11:10 +0100
Message-ID<rcxBM-2ly-9@gated-at.bofh.it>
In reply to#1356418
On 2016-03-12 14:42, Pali Rohár wrote:
> 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.

I'm sorry, but I can not debug my n900 as I rely on it as primary phone
time-to-time.

I would try to disable one by one the drivers for devices on the i2c_2 bus
in the stock kernel and see if this will point to something.
It might worth looking at the driver init codes for the devices we have on
i2c_2 also. Since rebooting to stock kernel does not have issue, it might be
the chip init for at least one of the device might cause the i2c bus lock.
It is also possible that the driver load order is different and we might
need to load one of the drivers before the others?
In the board file (board-rx51-peripherals.c) the tpa is the last entry in
the i2c_board_info, so it is most likely the last one to load among the
drivers for i2c_2 devices. In the dts si4713 and bq24150a is after the
tpa... Try to move the tpa as last one in the dts?

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

-- 
Péter

[toc] | [prev] | [next] | [standalone]


#1357423

FromIvaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Date2016-03-14 18:10 +0100
Message-ID<rcEae-6JJ-3@gated-at.bofh.it>
In reply to#1357114
Hi,

On 14.03.2016 11:59, Peter Ujfalusi wrote:
>
> Does the i2c communication breaks with DT _and_ non DT boot?
>

IIRC, there was the same problem with legacy boot as well, but because 
there were tons of other problems, we did not investigate it :)

Regards,
Ivo

[toc] | [prev] | [next] | [standalone]


#1358996

FromPali Rohár <pali.rohar@gmail.com>
Date2016-03-16 14:40 +0100
Message-ID<rdjQ5-18n-7@gated-at.bofh.it>
In reply to#1357114
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?

On Monday 14 March 2016 11:59:13 Peter Ujfalusi wrote:
> Does the i2c communication breaks with DT _and_ non DT boot?

Yes, for both DT and non DT boot. And this problem is there for a long
time...

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]


#1359061

FromSebastian Reichel <sre@kernel.org>
Date2016-03-16 15:50 +0100
Message-ID<rdkVQ-1NZ-25@gated-at.bofh.it>
In reply to#1358996

[Multipart message — attachments visible in raw view] — view raw

Hi,

On Wed, Mar 16, 2016 at 02:33:19PM +0100, Pali Rohár wrote:
> 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?

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

Just dump the stack during the tpa6130a2 initialization and you can
see it in the kernel log:

dump_stack();

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

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

[toc] | [prev] | [next] | [standalone]


#1359221

FromIvaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Date2016-03-16 19:30 +0100
Message-ID<rdomK-4bz-19@gated-at.bofh.it>
In reply to#1359061
Hi,

On 16.03.2016 16:47, Sebastian Reichel wrote:
> Hi,
>
> On Wed, Mar 16, 2016 at 02:33:19PM +0100, Pali Rohár wrote:
>> 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.

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

I'll try that, thanks.

Regards,
Ivo

[toc] | [prev] | [next] | [standalone]


#1359234

FromGrygorii Strashko <grygorii.strashko@ti.com>
Date2016-03-16 19:40 +0100
Message-ID<rdowq-4gp-23@gated-at.bofh.it>
In reply to#1359221
On 03/16/2016 08:21 PM, Ivaylo Dimitrov wrote:
> Hi,
> 
> On 16.03.2016 16:47, Sebastian Reichel wrote:
>> Hi,
>>
>> On Wed, Mar 16, 2016 at 02:33:19PM +0100, Pali Rohár wrote:
>>> 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.

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

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



-- 
regards,
-grygorii

[toc] | [prev] | [next] | [standalone]


#1359263

FromIvaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Date2016-03-16 21:00 +0100
Message-ID<rdpLP-4Ym-3@gated-at.bofh.it>
In reply to#1359234
Hi,

On 16.03.2016 20:32, Grygorii Strashko wrote:
>
> 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
>

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.

Ivo

[toc] | [prev] | [next] | [standalone]


#1359496

FromSebastian Reichel <sre@kernel.org>
Date2016-03-17 01:50 +0100
Message-ID<rduit-8ly-7@gated-at.bofh.it>
In reply to#1359263

[Multipart message — attachments visible in raw view] — view raw

Hi,

On Wed, Mar 16, 2016 at 09:50:40PM +0200, Ivaylo Dimitrov wrote:
> 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.

-- Sebastian

[toc] | [prev] | [next] | [standalone]


#1359615

FromIvaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Date2016-03-17 09:00 +0100
Message-ID<rdB0C-4pn-15@gated-at.bofh.it>
In reply to#1359496
Hi,

On 17.03.2016 02:49, Sebastian Reichel wrote:
>
> 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.


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

Regards,
Ivo

[toc] | [prev] | [next] | [standalone]


#1359839

FromPali Rohár <pali.rohar@gmail.com>
Date2016-03-17 14:10 +0100
Message-ID<rdFQD-7QE-31@gated-at.bofh.it>
In reply to#1359615
On Thursday 17 March 2016 09:56:22 Ivaylo Dimitrov wrote:
> Hi,
> 
> On 17.03.2016 02:49, Sebastian Reichel wrote:
> >
> >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.
> 
> 
> 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 :)

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

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

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]


#1359844

FromIvaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Date2016-03-17 14:20 +0100
Message-ID<rdG0i-7TR-9@gated-at.bofh.it>
In reply to#1359839
Hi,

On 17.03.2016 15:01, Pali Rohár wrote:
> 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.

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

Regards,
Ivo

[toc] | [prev] | [next] | [standalone]


#1359855

FromTony Lindgren <tony@atomide.com>
Date2016-03-17 14:40 +0100
Message-ID<rdGjE-80w-19@gated-at.bofh.it>
In reply to#1359844
* Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> [160317 06:11]:
> Hi,
> 
> On 17.03.2016 15:01, Pali Rohár wrote:
> >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.
> 
> >>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.

Regards,

Tony

[toc] | [prev] | [next] | [standalone]


#1359867

FromIvaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Date2016-03-17 15:00 +0100
Message-ID<rdGD0-87J-1@gated-at.bofh.it>
In reply to#1359855
Hi,

On 17.03.2016 15:33, Tony Lindgren wrote:
>
> 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?

Regards,
Ivo

[toc] | [prev] | [next] | [standalone]


#1359913

FromTony Lindgren <tony@atomide.com>
Date2016-03-17 15:40 +0100
Message-ID<rdHfI-9X-7@gated-at.bofh.it>
In reply to#1359867
* Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> [160317 06:51]:
> Hi,
> 
> On 17.03.2016 15:33, Tony Lindgren wrote:
> >
> >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?

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

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

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.

Regards,

Tony

[toc] | [prev] | [next] | [standalone]


#1359926

FromIvaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Date2016-03-17 16:00 +0100
Message-ID<rdHz4-gE-13@gated-at.bofh.it>
In reply to#1359913
Hi,

On 17.03.2016 16:32, Tony Lindgren wrote:
>
> Seems like a bug to me. My bets are on the deferred probe related
> Peter posted.
>

I will test Peter's patch as well, however I really doubt internal 
pull-ups enabled by Nokia to be a bug - keep in mind there are (or at 
least were) enough devices on the field for such a bug to not remain 
unnoticed, esp if it directly affects the signal quality over the i2c bus.

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

Sure :)

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

And it seems stock kernel can read/write without problems with those 
pull-ups enabled.

However, lets continue the discussion after I have tested both the 
Peter's patch and internal pull-ups enabled.

Thanks,
Ivo

[toc] | [prev] | [next] | [standalone]


Page 1 of 3  [1] 2 3  Next page →

Back to top | Article view | linux.kernel


csiph-web