On 7/2/19 12:42 PM, Greg KH wrote: > On Tue, Jul 02, 2019 at 10:36:59AM +0800, JC Kuo wrote: >> On 7/1/19 4:52 PM, Greg KH wrote: >>> On Mon, Jul 01, 2019 at 04:48:48PM +0800, JC Kuo wrote: >>>> When usb-storage driver detects a UAS capable device, it ignores the >>>> device if CONFIG_USB_UAS is enabled. usb-storage driver assumes uas >>>> driver certainly will be loaded. However, it's possible that uas >>>> driver will not be loaded, for example, uas kernel module is not >>>> installed properly or it is in modprobe blacklist. >>>> >>>> In case of uas driver not being loaded, the UAS capable device will >>>> not fallback to work at Bulk-only-transfer mode. The device just >>>> disappears without any notification to user/userspace. >>>> >>>> This commit changes usb-storage driver to skip UAS capable device >>>> only when uas driver is already loaded to make sure the device will >>>> at least work with Bulk protocol. >>> >>> But what happens if the driver is loaded afterward, because 'modprobe' >>> was called by the driver core (or it should have been, because this is a >>> device that supports that protocol)? >> If uas driver is loaded after usb-storage driver probed the device, >> the device will still work with Bulk-only protocol, though it can't >> make uses of streams. > > Which is not a good thing, and is what the original code was there to > prevent happening. > >>> I think you just broke working systems :( >>> >>> Why wouldn't the UAS driver get loaded automatically if it is configured >>> in the system as it is today? >> An user might want to completely disable uas for some reason so he/she >> adds "blacklist uas" to modprobe conf file. I think in case of this, >> usb-storage driver has to enable this device with the legacy Bulk-only >> protocol instead of ignoring the device. > > Why would they want to do that? Where are people doing this in ways > that breaks their systems? I think that could be because user sees issues with UAS but he/she is not aware of the quirks parameter that usb-storage module offers to disable UAS for any particular device. IMHO, blacklisting uas driver should not break the system because the UAS devices are supposed to be working fine with legacy Bulk-only protocol if system software doesn't have UAS support. > >> As an alternative to this patch, I thought I could get uas driver >> loaded before usb-storage driver so I tried moving the functions in >> drivers/usb/storage/uas-detect.h into uas.c and letting usb-storage >> links uas_use_uas_driver() of uas.ko. However, that didn't work >> because uas driver actually depends on usb-storage driver for >> usb_stor_adjust_quirks(). There will be a recursive dependency. >> >> Please let me know if there is better approach to avoid the issue. > > If users blacklist the uas driver, that's their choice and they should > rebuild their kernel :) > > Or better yet, talk to us to get the issue fixed for why they would want > to blacklist such a driver. Agree. :) > > As it is, this patch is not acceptable. Understood. Thanks for the comments. I will drop this patch. Thanks, JC > > thanks, > > greg k-h > ----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. -----------------------------------------------------------------------------------
