[PATCH v4 1/1] power: supply: cpcap-charger: Add usleep to cpcap charger to avoid usb plug bounce

[PATCH v4 1/1] power: supply: cpcap-charger: Add usleep to cpcap charger to avoid usb plug bounce

[Carl Philipp Klemm]

Adds 80000 us sleep when the usb cable is plugged in to hopfully avoid bounceing
contacts.

Upon pluging in the usb cable vbus will bounce for some time, causing cpcap to
dissconnect charging due to detecting an undervoltage condition. This is a
scope of vbus on xt894 while quickly inserting the usb cable with firm force,
probed at the far side of the usb soccet and vbus loaded with approx 1k:
http://uvos.xyz/maserati/usbplug.jpg.

As can clearly be seen, vbus is all over the place for the first 15 ms or so 
with a small blip at ~40 ms this causes the cpcap to trip up and disable 
charging again.

The delay helps cpcap_usb_detect avoid the worst of this. It is, however, still
not ideal as strong vibrations can cause the issue to reapear at any time during
charging. I have however not been able to cause the device to stop charging due
to this in practice as it is hard to vibrate the device sutch that the vbus pins
start bouncing again but cpcap_usb_detect is not called again due to a detected
dissconnect/reconnect event.

Changes in v4:
Increase timeout after some experiamentation suggested its not sufficant on all 
devices.

Signed-off-by: Carl Philipp Klemm <philipp@uvos.xyz>
---
 drivers/power/supply/cpcap-charger.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/power/supply/cpcap-charger.c b/drivers/power/supply/cpcap-charger.c
index c0d452e3dc8b..e6b230dd6de4 100644
--- a/drivers/power/supply/cpcap-charger.c
+++ b/drivers/power/supply/cpcap-charger.c
@@ -613,6 +613,9 @@ static void cpcap_usb_detect(struct work_struct *work)
        if (error)
                return;
 
+    /* Delay for 80ms to avoid vbus bouncing when usb cable is plugged in */
+       usleep_range(80000, 120000);
+
        /* Just init the state if a charger is connected with no chrg_det set */
        if (!s.chrg_det && s.chrgcurr1 && s.vbusvld) {
                cpcap_charger_update_state(ddata, CPCAP_CHARGER_DETECTING);
-- 
2.29.2

Re: [PATCH v4 1/1] power: supply: cpcap-charger: Add usleep to cpcap charger to avoid usb plug bounce

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 3025 bytes --]

Hi,

On Tue, Jan 05, 2021 at 08:48:03PM +0100, Ladislav Michl wrote:
> Dear Carl Philipp,
> 
> a few comments...
> 
> On Tue, Jan 05, 2021 at 07:52:31PM +0100, Carl Philipp Klemm wrote:
> > Adds 80000 us sleep when the usb cable is plugged in to hopfully avoid bounceing
> 							hopefully	bouncing
> > contacts.
> > 
> > Upon pluging in the usb cable vbus will bounce for some time, causing cpcap to
> > dissconnect charging due to detecting an undervoltage condition. This is a
> disconnect
> > scope of vbus on xt894 while quickly inserting the usb cable with firm force,
> > probed at the far side of the usb soccet and vbus loaded with approx 1k:
> 				socket
> > http://uvos.xyz/maserati/usbplug.jpg.
> > 
> > As can clearly be seen, vbus is all over the place for the first 15 ms or so 
> > with a small blip at ~40 ms this causes the cpcap to trip up and disable 
> > charging again.
> > 
> > The delay helps cpcap_usb_detect avoid the worst of this. It is, however, still
> > not ideal as strong vibrations can cause the issue to reapear at any time during
> > charging. I have however not been able to cause the device to stop charging due
> > to this in practice as it is hard to vibrate the device sutch that the vbus pins
> 							such
> > start bouncing again but cpcap_usb_detect is not called again due to a detected
> > dissconnect/reconnect event.
> disconnect
> > 
> > Changes in v4:
> > Increase timeout after some experiamentation suggested its not sufficant on all 
> > devices.
> 
> Changelog here would become part of a commit message, which is undesirable. Please
> move it bellow ---
> 
> > Signed-off-by: Carl Philipp Klemm <philipp@uvos.xyz>
> > ---
> >  drivers/power/supply/cpcap-charger.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/power/supply/cpcap-charger.c b/drivers/power/supply/cpcap-charger.c
> > index c0d452e3dc8b..e6b230dd6de4 100644
> > --- a/drivers/power/supply/cpcap-charger.c
> > +++ b/drivers/power/supply/cpcap-charger.c
> > @@ -613,6 +613,9 @@ static void cpcap_usb_detect(struct work_struct *work)
> >         if (error)
> >                 return;
> >  
> > +    /* Delay for 80ms to avoid vbus bouncing when usb cable is plugged in */
> > +       usleep_range(80000, 120000);
> > +
> 
> indent with tabs please, verify your mailer did not corrupt patch.
> 
> >         /* Just init the state if a charger is connected with no chrg_det set */
> >         if (!s.chrg_det && s.chrgcurr1 && s.vbusvld) {
> >                 cpcap_charger_update_state(ddata, CPCAP_CHARGER_DETECTING);

Patch itself looks sensible to me. After fixing the issues mentioned
by Ladislav, please check your mailer carefully. The patch you sent
uses spaces instead of tabs also for the surrounding code suggesting
the mailer broke this. Last but not least I applied a big series
from Tony to cpcap, so don't forget to rebase against current
power-supply for-next branch.

Thanks,

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]