[Gta04-owner] Sporadic USB charging issue identified

NeilBrown neilb at suse.de
Tue Feb 19 00:29:38 CET 2013


On Mon, 18 Feb 2013 22:44:53 +0100 "Dr. H. Nikolaus Schaller"
<hns at goldelico.com> wrote:

> Hi,
> 
> Am 18.02.2013 um 21:38 schrieb NeilBrown:
> 
> > On Mon, 18 Feb 2013 20:45:13 +0100 Dr.H.Nikolaus Schaller <hns at goldelico.com>
> > wrote:
> > 
> >> 
> >> Am 18.02.2013 um 19:45 schrieb Dr. H. Nikolaus Schaller:
> >> 
> >>> 
> >>> Am 17.02.2013 um 22:22 schrieb NeilBrown:
> >>> 
> >>>> On Sun, 17 Feb 2013 20:44:14 +0100 "Dr. H. Nikolaus Schaller"
> >>>> <hns at goldelico.com> wrote:
> >>>> 
> >>>>> I have found a sporadic charging issue on at least one GTA04 which may be
> >>>>> kernel/driver/PMU-initialisazion related.
> >>>>> 
> >>>>> The symptom is that the device starts to charge, suddenly stops and does
> >>>>> not continue to charge - although still connected to USB and VBUS being 5V.
> >>>>> 
> >>>>> After a while the battery is getting empty, although the device is still connected
> >>>>> to USB and switches off. The battery is even deeply discharged to 2.6V (because
> >>>>> VBUS is available and apparently overrides power off at 3.2V).
> >>>>> 
> >>>>>> From my tests it appears that the USB charger driver monitors VBUS and
> >>>>> resigns to charge if VBUS drops below ca. 4.3V - even for a short moment.
> >>>>> 
> >>>>> It only restarts if you plug off the USB cable, i.e. lowering VBUS to 0V and
> >>>>> rising it again.
> >>>>> 
> >>>>> Momentary VUSB variations may happen mechanically by a loose or weak
> >>>>> contact of the USB plug or cable. Or if the GTA04 suddenly draws a little more
> >>>>> current (e.g. switching on backlight and WLAN) leading to a small voltage drop.
> >>>>> 
> >>>>> If this drop is relevant may depend on component variations, and even the
> >>>>> length of the USB cable. So it is only recognized on some GTA04 units.
> >>>>> 
> >>>>> I think there is no simple threshold like "charge if VBUS > 4.2V",
> >>>>> but a hysteresis: stop charging for VBUS < 4.2V and start charging
> >>>>> of VBUS rises from below 1V to >1V or similar.
> >>>>> 
> >>>>> In other words: VBUS must be interrupted for at least 0.5 seconds or so.
> >>>>> Shorter interruptions can disturb the charger function.
> >>>>> 
> >>>>> That the charger is resigning to charge if VBUS is not above some minimum
> >>>>> is basically ok. But that it never restarts charging is not robust enough.
> >>>>> 
> >>>>> I think (but I have not tested completely) that this effect occurs on all
> >>>>> kernels we have (2.6.32 and all 3.x).
> >>>>> 
> >>>>> Part of the symptom is that the BTEMP ADC of the twl4030 hardware
> >>>>> monitor reports bad values (somehow proportional to the VUSB instead
> >>>>> of being constant) while reading VBUS, the battery voltage or current is ok.
> >>>>> 
> >>>>> But this might be simply a side-effect of a disabled charger.
> >>>>> 
> >>>>> My question now:
> >>>>> 
> >>>>> does anyone know how and where this hysteresis is implemented?
> >>>>> is it in the TPS hardware?
> >>>>> or programmable through registers?
> >>>>> or is it part of the twl4030 USB charger driver?
> >>>> 
> >>>> My guess would be that it is a subtle interplay between the TPS hardware and
> >>>> the twl4030 charger driver.
> >>>> 
> >>>> When the USB voltage drops below 4.4V, the TPS hardware turns off charging.
> >>>> When a cable is plugged in, an interrupt triggers some sort of notification
> >>>> from the USB driver to the CHARGER driver which turns on charging.
> >>>> 
> >>>> In your case, the hardware notices the voltage drop, but there is no "cable
> >>>> plugged in" event.
> >>>> 
> >>>> To delve further I would enable the
> >>>> 	dev_dbg(bci->dev, "BCI irq %02x %02x\n", irqs2, irqs1);
> >>>> line in twl4030_bci_interrupt() in twl4030_charger.c
> >>>> If you have dynamic debugging turned on you can do this with something like
> >>>> 
> >>>> echo file twl4030_charger.c function twl4030_bci_interrupt +p \
> >>>>> /sys/kernel/debug/dynamic_debugging/control
> >>>> 
> >>>> (I probably have the syntax a bit wrong - there is a file in Documentation/)
> >>>> 
> >>>> Then see if any BCI interrupts happen at the key time.
> >>>> 
> >>>> You might need to look for usb interrupts as well which would be 
> >>>> twl4030_usb_irq() in drivers/usb/otg/twl4030-usb.c.  This currently calls do_notify()
> >>>> which calls through a notifier chain to the charger module.
> >>>> 
> >>>> You possibly need to get twl4030_charger_enable_usb(bci, true) to run after
> >>>> the voltage has stablised again.  Alternately it could be that the USB PHY is
> >>>> being powered down (twl4030_phy_suspend) and not powered up again
> >>>> (twl4030_phy_resume).
> >>>> 
> >>>> I think it is very likely that there is a race somewhere in the software
> >>>> responding to hardware changes.  Once you can identify exactly the sequence
> >>>> of operations seen by the software it shouldn't be too hard to find a way to
> >>>> close the race.
> >>> 
> >>> I have not yet looked for interrupts or alike, but have studied the TPS65950
> >>> charger control registers and looked into the driver code.
> >>> 
> >>> To me it appears that the driver is simply doing the wrong thing based on false
> >>> assumptions that USB unplugging is always detected in the same way by OTG
> >>> and by the charger. But in fact there are two separate detectors.
> >>> 
> >>> If an OTG plug event is detected, twl4030_charger_enable_usb() enables the
> >>> charger by setting TWL4030_USBFASTMCHG=1.
> >>> 
> >>> Now the charger autonomously detects an USB-unplug event because
> >>> VBUSUNPLGEN=1 (default after reset!) and then sets
> >>> TWL4030_USBFASTMCHG=0 which stops charging (but does not generate
> >>> interrupts).
> >>> 
> >>> This is called "hardware method".
> >>> 
> >>> So it is exactly as I did conjecture that the charger is finding VBUS < 4.4V and
> >>> interprets this autonomously as an unplug event. While the OTG interface doesn't.
> >>> 
> >>> And no part of the system takes care of re-enabling the charger because
> >>> neither the hardware nor the OTG driver feels responsible to do so...
> >>> 
> >>> So it is not a race of events, it is simply that there are no events at all.
> >>> 
> >>> IMHO,  the better implementation would be the "software method". This would mean
> >>> that we actively set VBUSUNPLGEN=0. And extend twl4030_charger_enable_usb to
> >>> actively reset USBFASTMCHG on OTG unplug detection.
> >>> 
> >>> But I fear that we don't ever get this into mainline, because this part has
> >>> been written by TI and others.
> >>> 
> >>> And, another reason may be that using the "hardware method" is more
> >>> robust to kernel problems.
> >>> 
> >>> But do you see a chance to get it to mainline if we make this a configure option?
> >>> I.e. Hardware vs. Software unplug detection.
> >>> 
> >>> If yes, I will try to write a patch.
> >> 
> >> Well, it could be as simple as (not tested!):
> >> 
> >> diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
> >> index 02d67cb..62b4f35 100644
> >> --- a/drivers/power/twl4030_charger.c
> >> +++ b/drivers/power/twl4030_charger.c
> >> @@ -43,6 +43,7 @@
> >> #define TWL4030_BCIAUTOAC      BIT(0)
> >> #define TWL4030_CGAIN          BIT(5)
> >> #define TWL4030_USBFASTMCHG    BIT(2)
> >> +#define TWL4030_VBUSUNPLGEN    BIT(5)
> >> #define TWL4030_STS_VBUS       BIT(7)
> >> #define TWL4030_STS_USB_ID     BIT(2)
> >> #define TWL4030_BBCHEN         BIT(4)
> >> @@ -263,7 +264,7 @@ static int twl4030_charger_enable_usb(struct twl4030_bci *bci, bool enable)
> >>                        return ret;
> >> 
> >>                /* forcing USBFASTMCHG(BCIMFSTS4[2]) to 1 */
> >> -               ret = twl4030_clear_set(TWL4030_MODULE_MAIN_CHARGE, 0,
> >> +               ret = twl4030_clear_set(TWL4030_MODULE_MAIN_CHARGE, TWL4030_VBUSUNPLGEN,
> >>                        TWL4030_USBFASTMCHG, TWL4030_BCIMFSTS4);
> >>        } else {
> >>                ret = twl4030_clear_set_boot_bci(TWL4030_BCIAUTOUSB, 0);
> >> 
> >> I don't know if unplug detection by USB calls twl4030_bci_usb_ncb() with USB_EVENT_NONE.
> >> 
> >> BR,
> >> Nikolaus
> > 
> > 
> > Why off list?
> 
> was an accident. I have made the reply go back to the list.
> 
> > 
> > Why would you want to clear VBUSUNPLGEN?  When the docs explicitly say it
> > must be set....
> 
> Where?

Same section that you were  reading:

   In the hardware method, the VBUS comparator must be kept enabled by setting
   the BCIMFSTS4[5] VBUSUNPLGEN bit to 1. 
> 
> Section 7.4.1.2.1:
> 
> "In software mode, the VBUS comparator is disabled by setting the VBUSUNPLGEN bit to 0. The software detects that the USB device is unplugged and stops charging. The USB charge is stopped by forcing the USBFASTMCHG and USBSLOWMCHG bits to 0."
> 
> The patch above works a little different, by disabling the charger through setting TWL4030_BCIAUTOUSB to 0.
>

The hardware is designed to either have hardware-driven monitoring or
software driven monitoring.  You seem to want to leave it up to the hardware,
but to disable one little decision that it makes.  This doesn't seem safe to
me.
It isn't necessarily clear what all the consequences of clearing that bit
without doing any monitoring are.  If the charger is left enabled while the
USB voltage is too low, could there be some current backflow that might cause
problems?

I barely trust hardware where  you program it exactly according to the
documentation.  If you try something that the documentation seems to say is
not allowed, I stop trusting it altogether :-)

NeilBrown
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 828 bytes
Desc: not available
URL: <http://lists.goldelico.com/pipermail/gta04-owner/attachments/20130219/eab2c556/attachment-0001.bin>


More information about the Gta04-owner mailing list