| Message ID | 20241104085056.652294-1-stanislaw.gruszka@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/3] usb: misc: ljca: move usb_autopm_put_interface() after wait for response | expand |
Hi, On 4-Nov-24 9:50 AM, Stanislaw Gruszka wrote: > Do not mark interface as ready to suspend when we are still waiting > for response messages from the device. > > Fixes: acd6199f195d ("usb: Add support for Intel LJCA device") > Cc: stable@vger.kernel.org > Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com> > --- > drivers/usb/misc/usb-ljca.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/misc/usb-ljca.c b/drivers/usb/misc/usb-ljca.c > index 01ceafc4ab78..dcb3c5d248ac 100644 > --- a/drivers/usb/misc/usb-ljca.c > +++ b/drivers/usb/misc/usb-ljca.c > @@ -332,9 +332,6 @@ static int ljca_send(struct ljca_adapter *adap, u8 type, u8 cmd, > > ret = usb_bulk_msg(adap->usb_dev, adap->tx_pipe, header, > msg_len, &transferred, LJCA_WRITE_TIMEOUT_MS); > - > - usb_autopm_put_interface(adap->intf); > - > if (ret < 0) > goto out; > if (transferred != msg_len) { > @@ -353,6 +350,8 @@ static int ljca_send(struct ljca_adapter *adap, u8 type, u8 cmd, > ret = adap->actual_length; > > out: > + usb_autopm_put_interface(adap->intf); > + I'm afraid that this now does a double pm_runtime_put() on usb_autopm_get_interface() failures. usb_autopm_get_interface() uses pm_runtime_resume_and_get() which already does a pm_runtime_put() on failure. So you need to add a second out label which skips the new location of the usb_autopm_put_interface(adap->intf); call and jump to this second label on usb_autopm_get_interface() failures. With that fixed this is: Reviewed-by: Hans de Goede <hdegoede@redhat.com> I have also given this a test run on a "ThinkPad X1 Yoga Gen 8" and everything there works at least as well as before: Tested-by: Hans de Goede <hdegoede@redhat.com> # ThinkPad X1 Yoga Gen 8, ov2740 Regards, Hans > spin_lock_irqsave(&adap->lock, flags); > adap->ex_buf = NULL; > adap->ex_buf_len = 0;
On Mon, Nov 04, 2024 at 02:39:57PM +0100, Hans de Goede wrote: > Hi, > > On 4-Nov-24 9:50 AM, Stanislaw Gruszka wrote: > > Do not mark interface as ready to suspend when we are still waiting > > for response messages from the device. > > > > Fixes: acd6199f195d ("usb: Add support for Intel LJCA device") > > Cc: stable@vger.kernel.org > > Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com> > > --- > > drivers/usb/misc/usb-ljca.c | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/usb/misc/usb-ljca.c b/drivers/usb/misc/usb-ljca.c > > index 01ceafc4ab78..dcb3c5d248ac 100644 > > --- a/drivers/usb/misc/usb-ljca.c > > +++ b/drivers/usb/misc/usb-ljca.c > > @@ -332,9 +332,6 @@ static int ljca_send(struct ljca_adapter *adap, u8 type, u8 cmd, > > > > ret = usb_bulk_msg(adap->usb_dev, adap->tx_pipe, header, > > msg_len, &transferred, LJCA_WRITE_TIMEOUT_MS); > > - > > - usb_autopm_put_interface(adap->intf); > > - > > if (ret < 0) > > goto out; > > if (transferred != msg_len) { > > @@ -353,6 +350,8 @@ static int ljca_send(struct ljca_adapter *adap, u8 type, u8 cmd, > > ret = adap->actual_length; > > > > out: > > + usb_autopm_put_interface(adap->intf); > > + > > I'm afraid that this now does a double pm_runtime_put() on > usb_autopm_get_interface() failures. usb_autopm_get_interface() uses > pm_runtime_resume_and_get() which already does a pm_runtime_put() > on failure. > > So you need to add a second out label which skips the new > location of the usb_autopm_put_interface(adap->intf); call > and jump to this second label on usb_autopm_get_interface() > failures. Good point, thanks for catching this, I will fix and post v2. Regards Stanislaw
diff --git a/drivers/usb/misc/usb-ljca.c b/drivers/usb/misc/usb-ljca.c index 01ceafc4ab78..dcb3c5d248ac 100644 --- a/drivers/usb/misc/usb-ljca.c +++ b/drivers/usb/misc/usb-ljca.c @@ -332,9 +332,6 @@ static int ljca_send(struct ljca_adapter *adap, u8 type, u8 cmd, ret = usb_bulk_msg(adap->usb_dev, adap->tx_pipe, header, msg_len, &transferred, LJCA_WRITE_TIMEOUT_MS); - - usb_autopm_put_interface(adap->intf); - if (ret < 0) goto out; if (transferred != msg_len) { @@ -353,6 +350,8 @@ static int ljca_send(struct ljca_adapter *adap, u8 type, u8 cmd, ret = adap->actual_length; out: + usb_autopm_put_interface(adap->intf); + spin_lock_irqsave(&adap->lock, flags); adap->ex_buf = NULL; adap->ex_buf_len = 0;
Do not mark interface as ready to suspend when we are still waiting for response messages from the device. Fixes: acd6199f195d ("usb: Add support for Intel LJCA device") Cc: stable@vger.kernel.org Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com> --- drivers/usb/misc/usb-ljca.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)