Oliver Neukum wrote on Tue 26/05/20 13:13:
>
> Hi,
>
> may I ask whether you did the test with removing the battery twice with
> an older kernel? Could you please go back to
> f6cc6093a729ede1ff5658b493237c42b82ba107
> and repeat the test of a second battery removal with that state?
> I just cannot find anything pointing to a change that could cause
> this issue within that time.
Hi,
I tested again with 5.7-rc6 and the following applied to drivers/usb/class/cdc-acm.c :
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -579,8 +579,8 @@ static void acm_softint(struct work_struct *work)
usb_kill_urb(acm->read_urbs[i]);
usb_clear_halt(acm->dev, acm->in);
acm_submit_read_urbs(acm, GFP_KERNEL);
- clear_bit(EVENT_RX_STALL, &acm->flags);
}
+ clear_bit(EVENT_RX_STALL, &acm->flags);
}
if (test_and_clear_bit(ACM_ERROR_DELAY, &acm->flags)) {
This in addtion to your earlier patch :
> > > diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
> > > index 7678ae4afd53..be4543569822 100644
> > > --- a/drivers/usb/class/cdc-acm.c
> > > +++ b/drivers/usb/class/cdc-acm.c
> > > @@ -585,7 +585,7 @@ static void acm_softint(struct work_struct *work)
> > > }
> > >
> > > if (test_and_clear_bit(ACM_ERROR_DELAY, &acm->flags)) {
> > > - for (i = 0; i < ACM_NR; i++)
> > > + for (i = 0; i < acm->rx_buflimit; i++)
> > > if (test_and_clear_bit(i, &acm->urbs_in_error_delay))
> > > acm_submit_read_urb(acm, i, GFP_NOIO);
And with these patches the behaviour seems to me like with
f6cc6093a729ede1ff5658b493237c42b82ba10. I'm not sure if this is correct
but to me it looked odd that before 0afccd7601514c4b83d8cc58c740089cc447051d the function
clear_bit(EVENT_RX_STALL, &acm->flags);
was always called when the block of
if (test_bit(EVENT_RX_STALL, &acm->flags)) {
was entered.
But since 0afccd7601514c4b83d8cc58c740089cc447051d
only when condition
if (!acm->susp_count) {
was also fulfilled:
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 84d6f7df09a4..4ef68e6671aa 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -557,14 +557,14 @@ static void acm_softint(struct work_struct *work)
struct acm *acm = container_of(work, struct acm, work);
if (test_bit(EVENT_RX_STALL, &acm->flags)) {
- if (!(usb_autopm_get_interface(acm->data))) {
+ smp_mb(); /* against acm_suspend() */
+ if (!acm->susp_count) {
for (i = 0; i < acm->rx_buflimit; i++)
usb_kill_urb(acm->read_urbs[i]);
usb_clear_halt(acm->dev, acm->in);
acm_submit_read_urbs(acm, GFP_KERNEL);
- usb_autopm_put_interface(acm->data);
+ clear_bit(EVENT_RX_STALL, &acm->flags);
}
- clear_bit(EVENT_RX_STALL, &acm->flags);
}
if (test_and_clear_bit(EVENT_TTY_WAKEUP, &acm->flags))
Regards,
Jean Rene
