On Fri, Jan 6, 2012 at 3:47 AM, Greg KH <greg@xxxxxxxxx> wrote:
> On Wed, Jan 04, 2012 at 10:54:22PM -0500, Alan Stern wrote:
>> On Wed, 4 Jan 2012, Greg KH wrote:
>>
>> > On Wed, Jan 04, 2012 at 07:25:33PM +0800, Huajun Li wrote:
>> > > Echo vendor and product number of a non usb-storage device to
>> > > usb-storage driver's new_id, then plug in the device to host and you
>> > > will find following oops msg, the root cause is usb_stor_probe1()
>> > > refers invalid id entry if giving a dynamic id, so just disable the
>> > > feature.
>> >
>> > How about fixing this so it works properly?
>> >
>> > I'll take this patch now, but fixing the feature really would be nice to
>> > have, right?
>>
>> This should be easy enough, by adding an extra check to
>> storage_probe(). If id doesn't lie between the start and end of
>> usb_storage_usb_ids then it must come from a dynamic ID. In that case,
>> pass usb_stor_probe1() a fixed usb_device_id entry that describes a
>> device using the Bulk-Only transport with the Transparent SCSI
>> protocol.
>>
>> The various subdrivers, like alauda, datafab, and so on, probably
>> should have their no_dynamic_id flags set. They could be fixed up in
>> the same way, but it would be a lot of work with very little gain.
>
> Ok, that sounds good, anyone want to send a patch? I'm waist deep in
> stable kernel releases and the merge window at the moment, not to
> mention my "real job" to be able to do this for a few weeks.
>
> thanks,
>
> greg k-h
Hi Greg,
I worked out a patch based on the idea. But put 'index' validation
codes to usb_stor_probe1() rather than individual subdrivers' probing
routines, and add a global variable 'for_dynamic_ids' , which defines
Bulk-Only transport with the Transparent SCSI protocol. When storage
drivers receive dynamic id request, they can use the transport and
protocol specified in the variable.
Moreover, just as Alan said, subdriver need lots of work but little
gain, and I find most of them need pre-initialization, this may not
suitable for using dynamic id, so disable the feature for them.
Following proposal is against usb-next, please check it, thanks.
-----------------------------------------------------------------
diff --git a/drivers/usb/storage/alauda.c b/drivers/usb/storage/alauda.c
index 9ce3bba..af18e4b 100644
--- a/drivers/usb/storage/alauda.c
+++ b/drivers/usb/storage/alauda.c
@@ -1252,7 +1252,7 @@ static int alauda_probe(struct usb_interface *intf,
int result;
result = usb_stor_probe1(&us, intf, id,
- (id - alauda_usb_ids) + alauda_unusual_dev_list);
+ id - alauda_usb_ids, alauda_unusual_dev_list);
if (result)
return result;
@@ -1276,6 +1276,7 @@ static struct usb_driver alauda_driver = {
.post_reset = usb_stor_post_reset,
.id_table = alauda_usb_ids,
.soft_unbind = 1,
+ .no_dynamic_id = 1,
};
static int __init alauda_init(void)
diff --git a/drivers/usb/storage/cypress_atacb.c
b/drivers/usb/storage/cypress_atacb.c
index 740bfe6..ad4ab65 100644
--- a/drivers/usb/storage/cypress_atacb.c
+++ b/drivers/usb/storage/cypress_atacb.c
@@ -250,7 +250,7 @@ static int cypress_probe(struct usb_interface *intf,
int result;
result = usb_stor_probe1(&us, intf, id,
- (id - cypress_usb_ids) + cypress_unusual_dev_list);
+ id - cypress_usb_ids, cypress_unusual_dev_list);
if (result)
return result;
@@ -272,6 +272,7 @@ static struct usb_driver cypress_driver = {
.post_reset = usb_stor_post_reset,
.id_table = cypress_usb_ids,
.soft_unbind = 1,
+ .no_dynamic_id = 1,
};
static int __init cypress_init(void)
diff --git a/drivers/usb/storage/datafab.c b/drivers/usb/storage/datafab.c
index 0d8d97c..fc14bc9 100644
--- a/drivers/usb/storage/datafab.c
+++ b/drivers/usb/storage/datafab.c
@@ -727,7 +727,7 @@ static int datafab_probe(struct usb_interface *intf,
int result;
result = usb_stor_probe1(&us, intf, id,
- (id - datafab_usb_ids) + datafab_unusual_dev_list);
+ id - datafab_usb_ids, datafab_unusual_dev_list);
if (result)
return result;
@@ -751,6 +751,7 @@ static struct usb_driver datafab_driver = {
.post_reset = usb_stor_post_reset,
.id_table = datafab_usb_ids,
.soft_unbind = 1,
+ .no_dynamic_id = 1,
};
static int __init datafab_init(void)
diff --git a/drivers/usb/storage/ene_ub6250.c b/drivers/usb/storage/ene_ub6250.c
index b990726..6aca93c 100644
--- a/drivers/usb/storage/ene_ub6250.c
+++ b/drivers/usb/storage/ene_ub6250.c
@@ -2308,7 +2308,7 @@ static int ene_ub6250_probe(struct usb_interface *intf,
struct us_data *us;
result = usb_stor_probe1(&us, intf, id,
- (id - ene_ub6250_usb_ids) + ene_ub6250_unusual_dev_list);
+ id - ene_ub6250_usb_ids, ene_ub6250_unusual_dev_list);
if (result)
return result;
@@ -2407,6 +2407,7 @@ static struct usb_driver ene_ub6250_driver = {
.post_reset = usb_stor_post_reset,
.id_table = ene_ub6250_usb_ids,
.soft_unbind = 1,
+ .no_dynamic_id = 1,
};
static int __init ene_ub6250_init(void)
diff --git a/drivers/usb/storage/freecom.c b/drivers/usb/storage/freecom.c
index 8cf16f8..b08b23c 100644
--- a/drivers/usb/storage/freecom.c
+++ b/drivers/usb/storage/freecom.c
@@ -529,7 +529,7 @@ static int freecom_probe(struct usb_interface *intf,
int result;
result = usb_stor_probe1(&us, intf, id,
- (id - freecom_usb_ids) + freecom_unusual_dev_list);
+ id - freecom_usb_ids, freecom_unusual_dev_list);
if (result)
return result;
@@ -553,6 +553,7 @@ static struct usb_driver freecom_driver = {
.post_reset = usb_stor_post_reset,
.id_table = freecom_usb_ids,
.soft_unbind = 1,
+ .no_dynamic_id = 1,
};
static int __init freecom_init(void)
diff --git a/drivers/usb/storage/isd200.c b/drivers/usb/storage/isd200.c
index 7b81303..af9ca20 100644
--- a/drivers/usb/storage/isd200.c
+++ b/drivers/usb/storage/isd200.c
@@ -1544,7 +1544,7 @@ static int isd200_probe(struct usb_interface *intf,
int result;
result = usb_stor_probe1(&us, intf, id,
- (id - isd200_usb_ids) + isd200_unusual_dev_list);
+ id - isd200_usb_ids, isd200_unusual_dev_list);
if (result)
return result;
@@ -1566,6 +1566,7 @@ static struct usb_driver isd200_driver = {
.post_reset = usb_stor_post_reset,
.id_table = isd200_usb_ids,
.soft_unbind = 1,
+ .no_dynamic_id = 1,
};
static int __init isd200_init(void)
diff --git a/drivers/usb/storage/jumpshot.c b/drivers/usb/storage/jumpshot.c
index 5ef55c7..3229823 100644
--- a/drivers/usb/storage/jumpshot.c
+++ b/drivers/usb/storage/jumpshot.c
@@ -653,7 +653,7 @@ static int jumpshot_probe(struct usb_interface *intf,
int result;
result = usb_stor_probe1(&us, intf, id,
- (id - jumpshot_usb_ids) + jumpshot_unusual_dev_list);
+ id - jumpshot_usb_ids, jumpshot_unusual_dev_list);
if (result)
return result;
@@ -677,6 +677,7 @@ static struct usb_driver jumpshot_driver = {
.post_reset = usb_stor_post_reset,
.id_table = jumpshot_usb_ids,
.soft_unbind = 1,
+ .no_dynamic_id = 1,
};
static int __init jumpshot_init(void)
diff --git a/drivers/usb/storage/karma.c b/drivers/usb/storage/karma.c
index fb5bfb0..d977bef 100644
--- a/drivers/usb/storage/karma.c
+++ b/drivers/usb/storage/karma.c
@@ -207,7 +207,7 @@ static int karma_probe(struct usb_interface *intf,
int result;
result = usb_stor_probe1(&us, intf, id,
- (id - karma_usb_ids) + karma_unusual_dev_list);
+ id - karma_usb_ids, karma_unusual_dev_list);
if (result)
return result;
@@ -230,6 +230,7 @@ static struct usb_driver karma_driver = {
.post_reset = usb_stor_post_reset,
.id_table = karma_usb_ids,
.soft_unbind = 1,
+ .no_dynamic_id = 1,
};
static int __init karma_init(void)
diff --git a/drivers/usb/storage/onetouch.c b/drivers/usb/storage/onetouch.c
index d29be3e..1243543 100644
--- a/drivers/usb/storage/onetouch.c
+++ b/drivers/usb/storage/onetouch.c
@@ -291,7 +291,7 @@ static int onetouch_probe(struct usb_interface *intf,
int result;
result = usb_stor_probe1(&us, intf, id,
- (id - onetouch_usb_ids) + onetouch_unusual_dev_list);
+ id - onetouch_usb_ids, onetouch_unusual_dev_list);
if (result)
return result;
@@ -312,6 +312,7 @@ static struct usb_driver onetouch_driver = {
.post_reset = usb_stor_post_reset,
.id_table = onetouch_usb_ids,
.soft_unbind = 1,
+ .no_dynamic_id = 1,
};
static int __init onetouch_init(void)
diff --git a/drivers/usb/storage/realtek_cr.c b/drivers/usb/storage/realtek_cr.c
index 7114767..d9b25e3 100644
--- a/drivers/usb/storage/realtek_cr.c
+++ b/drivers/usb/storage/realtek_cr.c
@@ -1075,8 +1075,7 @@ static int realtek_cr_probe(struct usb_interface *intf,
US_DEBUGP("Probe Realtek Card Reader!\n");
- result = usb_stor_probe1(&us, intf, id,
- (id - realtek_cr_ids) +
+ result = usb_stor_probe1(&us, intf, id, id - realtek_cr_ids,
realtek_cr_unusual_dev_list);
if (result)
return result;
@@ -1100,6 +1099,7 @@ static struct usb_driver realtek_cr_driver = {
.id_table = realtek_cr_ids,
.soft_unbind = 1,
.supports_autosuspend = 1,
+ .no_dynamic_id = 1,
};
static int __init realtek_cr_init(void)
diff --git a/drivers/usb/storage/sddr09.c b/drivers/usb/storage/sddr09.c
index 6ecbf44..966b100 100644
--- a/drivers/usb/storage/sddr09.c
+++ b/drivers/usb/storage/sddr09.c
@@ -1756,7 +1756,7 @@ static int sddr09_probe(struct usb_interface *intf,
int result;
result = usb_stor_probe1(&us, intf, id,
- (id - sddr09_usb_ids) + sddr09_unusual_dev_list);
+ id - sddr09_usb_ids, sddr09_unusual_dev_list);
if (result)
return result;
@@ -1787,6 +1787,7 @@ static struct usb_driver sddr09_driver = {
.post_reset = usb_stor_post_reset,
.id_table = sddr09_usb_ids,
.soft_unbind = 1,
+ .no_dynamic_id = 1,
};
static int __init sddr09_init(void)
diff --git a/drivers/usb/storage/sddr55.c b/drivers/usb/storage/sddr55.c
index f293044..7ed025c 100644
--- a/drivers/usb/storage/sddr55.c
+++ b/drivers/usb/storage/sddr55.c
@@ -982,7 +982,7 @@ static int sddr55_probe(struct usb_interface *intf,
int result;
result = usb_stor_probe1(&us, intf, id,
- (id - sddr55_usb_ids) + sddr55_unusual_dev_list);
+ id - sddr55_usb_ids, sddr55_unusual_dev_list);
if (result)
return result;
@@ -1006,6 +1006,7 @@ static struct usb_driver sddr55_driver = {
.post_reset = usb_stor_post_reset,
.id_table = sddr55_usb_ids,
.soft_unbind = 1,
+ .no_dynamic_id = 1,
};
static int __init sddr55_init(void)
diff --git a/drivers/usb/storage/shuttle_usbat.c
b/drivers/usb/storage/shuttle_usbat.c
index 7d642c8..71d128c 100644
--- a/drivers/usb/storage/shuttle_usbat.c
+++ b/drivers/usb/storage/shuttle_usbat.c
@@ -1836,7 +1836,7 @@ static int usbat_probe(struct usb_interface *intf,
int result;
result = usb_stor_probe1(&us, intf, id,
- (id - usbat_usb_ids) + usbat_unusual_dev_list);
+ id - usbat_usb_ids, usbat_unusual_dev_list);
if (result)
return result;
@@ -1863,6 +1863,7 @@ static struct usb_driver usbat_driver = {
.post_reset = usb_stor_post_reset,
.id_table = usbat_usb_ids,
.soft_unbind = 1,
+ .no_dynamic_id = 1,
};
static int __init usbat_init(void)
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 9e069ef..8ae5878 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -125,6 +125,10 @@ static struct us_unusual_dev us_unusual_dev_list[] = {
{ } /* Terminating entry */
};
+static struct us_unusual_dev for_dynamic_ids = USUAL_DEV(USB_SC_SCSI,
+ USB_PR_BULK, 0);
+
+
#undef UNUSUAL_DEV
#undef COMPLIANT_DEV
#undef USUAL_DEV
@@ -521,6 +525,10 @@ static int get_device_info(struct us_data *us,
const struct usb_device_id *id,
&us->pusb_intf->cur_altsetting->desc;
struct device *pdev = &us->pusb_intf->dev;
+ /* fill_inquiry_response() may need the 2 info, initialize them here */
+ unusual_dev->vendorName = dev->manufacturer;
+ unusual_dev->productName = dev->product;
+
/* Store the entries */
us->unusual_dev = unusual_dev;
us->subclass = (unusual_dev->useProtocol == USB_SC_DEVICE) ?
@@ -885,10 +893,12 @@ static unsigned int usb_stor_sg_tablesize(struct
usb_interface *intf)
int usb_stor_probe1(struct us_data **pus,
struct usb_interface *intf,
const struct usb_device_id *id,
+ const unsigned long index,
struct us_unusual_dev *unusual_dev)
{
struct Scsi_Host *host;
struct us_data *us;
+ unsigned int size;
int result;
US_DEBUGP("USB Mass Storage device detected\n");
@@ -922,7 +932,25 @@ int usb_stor_probe1(struct us_data **pus,
if (result)
goto BadDevice;
+ /* FIXME: any easy way to get unusual_dev's size? */
+ for (size = 0; ; size++)
+ if (unusual_dev[size].vendorName == NULL &&
+ unusual_dev[size].productName == NULL &&
+ unusual_dev[size].useProtocol == 0 &&
+ unusual_dev[size].useTransport == 0 &&
+ unusual_dev[size].initFunction == NULL)
+ break;
+
/* Get the unusual_devs entries and the descriptors */
+ if (index < 0 || index >= size) {
+ unusual_dev = &for_dynamic_ids;
+
+ US_DEBUGP("%s %s 0x%04x 0x%04x\n", "Use Bulk-Only transport",
+ "with the Transparent SCSI protocol for dynamic id:",
+ id->idVendor, id->idProduct);
+ } else
+ unusual_dev += index;
+
result = get_device_info(us, id, unusual_dev);
if (result)
goto BadDevice;
@@ -1047,7 +1075,7 @@ static int storage_probe(struct usb_interface *intf,
* corresponding unusual_devs entry.
*/
result = usb_stor_probe1(&us, intf, id,
- (id - usb_storage_usb_ids) + us_unusual_dev_list);
+ id - usb_storage_usb_ids, us_unusual_dev_list);
if (result)
return result;
@@ -1073,7 +1101,6 @@ static struct usb_driver usb_storage_driver = {
.id_table = usb_storage_usb_ids,
.supports_autosuspend = 1,
.soft_unbind = 1,
- .no_dynamic_id = 1,
};
static int __init usb_stor_init(void)
diff --git a/drivers/usb/storage/usb.h b/drivers/usb/storage/usb.h
index 7b0f211..9f1a45f 100644
--- a/drivers/usb/storage/usb.h
+++ b/drivers/usb/storage/usb.h
@@ -196,6 +196,7 @@ extern int usb_stor_post_reset(struct usb_interface *iface);
extern int usb_stor_probe1(struct us_data **pus,
struct usb_interface *intf,
const struct usb_device_id *id,
+ const unsigned long index,
struct us_unusual_dev *unusual_dev);
extern int usb_stor_probe2(struct us_data *us);
extern void usb_stor_disconnect(struct usb_interface *intf);
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
