Hi,
On 2/21/21 9:51 AM, Hui Wang wrote:
> Once pre_reset() or post_reset() returns non-zero, the disconnect()
> and probe() of the usb_driver will be called. In the disconnect(),
> the scsi_host will be removed and be freed after scsi_host_put(), in
> the probe(), the new scsi_host and uas_dev_info will be created.
>
> If the usb_reset_device() is triggered by eh_device_reset_handler(),
> and pre_reset()/post_reset() returns non-zero, the disconnect() and
> probe() will be called, then returns to the eh_device_reset_handler(),
> it still accesses old scsi related variables and uas_dev_info, and so
> do its caller functions.
>
> Here change the pre_reset() and post_reset() to let them only return
> 0, after this change, the usb_reset_device() will only reset this
> usb devcie from its hub port, will not execute unbind and rebind
> usb_driver during reset.
We only return non 0 from the pre/post reset handles if we failed
to ensure the device is in a known state.
E.g. in uas_post_reset() we only fail if we failed to switch the
device from the good old usb-storage protocol back to the UAS mode
which it was in before.
Continuing with the UAS driver bound, as if everything is fine,
while the device is actually in longer in UAS mode is a bad idea.
Summarizing this patch is wrong: NACK.
###
I assume that you wrote this patch because of some bug report ?
In such a case please always include a link to the bug (or forum
discussion) in the commit message.
UAS problems typically are caused by people shoving a 2.5 inch
or m2 SSD in an USB enclosure which is powered from the USB bus
only. SSD-s can cause pretty hefty power-consumption peaks when
high queue depts are used; and these bus powered devices typically
cannot handle these peaks. Either because the port they are
plugged in does not deliver enough current; and/or because they
don't have enough buffering (capacitors) on the enclosure's PCB
to deal with short but quite large consumption peaks.
Regards,
Hans
>
> Signed-off-by: Hui Wang <hui.wang@xxxxxxxxxxxxx>
> ---
> drivers/usb/storage/uas.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
> index bef89c6bd1d7..c66287448e34 100644
> --- a/drivers/usb/storage/uas.c
> +++ b/drivers/usb/storage/uas.c
> @@ -1121,7 +1121,6 @@ static int uas_pre_reset(struct usb_interface *intf)
> if (uas_wait_for_pending_cmnds(devinfo) != 0) {
> shost_printk(KERN_ERR, shost, "%s: timed out\n", __func__);
> scsi_unblock_requests(shost);
> - return 1;
> }
>
> uas_free_streams(devinfo);
> @@ -1152,7 +1151,7 @@ static int uas_post_reset(struct usb_interface *intf)
>
> scsi_unblock_requests(shost);
>
> - return err ? 1 : 0;
> + return 0;
> }
>
> static int uas_suspend(struct usb_interface *intf, pm_message_t message)
>
