[Letux-kernel] [PATCH 0/6] bq2429x: Clean up driver for future fixes
nicholaselsmore at gmail.com
Thu Aug 13 17:51:25 CEST 2020
> * Interrupts/poll events are often missed, possibly due to hangups in the
> work queue.
> I haven't observed this in my tests. Do you have a specific example? Or a
> theory when and why it can happen?
I instrumented the driver to print before var_lock was acquired (I left
loglevel below debug, because currently there are lots of debug prints
which flood the output and make it difficult to filter out whats
important). I noticed that I would often not see bq2429x_usb_detect
executing until a number of seconds or minutes after I plugged or unplugged
the USB OTG. If I plugged or unplugged multiple times, I would eventually
get all of the calls occurring at once after the delay. This leads me to
believe that the workqueue is hanging somewhere.
Interestingly, I would sometimes see no new interrupts for this device
triggered in /proc/interrupts. This contradicts the theory that the issue
is a hangup in the workqueue, but it could be a combination of both issues
in the workqueue and IRQ handler.
> > * Plug/unplug events are not properly detected, due to either work queue
> hangups as described above or faulty logic determining plug status
> Yes, plug/unplug detection is quite some guesswork. A better strategy
> would be to connect to the USB-PHY...
I do not see why we would need to connect to the USB-PHY. Interrupts from
the bq2429x should be sufficient to determine whether or not the bq2429x is
connected to VBUS and whether or not the bq2429x has determined this to be
a good power source and is using it. I have patches which implement the
correct logic for handling this in my queue.
> > * Watchdog is left on (if not disabled by bootloader) and not serviced.
> It should be disabled by bootloader and kept in disabled state by the
> Contrary to what we all would suspect it is not a safety watchdog which
> would e.g. cut processor power or charging after a kernel panic or stall.
> It just a mechanism to auto-reset the bq2429x registers to default values
> which could also be done through writing all registers.
> So I do not see how to use it for practical purposes.
Yes. We should not assume, however, that the bootloader has disabled the
watchdog. We should make sure its disabled.
> > This is not properly handled.
> > * The following properties are missing:
> > * POWER_SUPPLY_PROP_HEALTH
> I am not sure if the charger can report a reasonable value for health.
We'll see. I have patches in my queue that determine reasonable values for
health, but we will see how they fare in review.
> Generally I find the patches good from a cursory glance, but I have to do
> a life check since it is not easy to see from the git diffs, what is
> changed (any why) and if all functions are transformed correctly. This
> takes at least until next week.
Yep, while these patches don't (shouldn't) modify any of the logic, they
are rather large and introduce a good amount of restructuring. I have
submitted these patches first before integrating any logic modifications as
its possible they have introduced regression. From my tests, the behavior
seems the same with and without the patches.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the Letux-kernel