Re: [PATCH 1/1]linux-usb: optimize to match rules in USB storage for Huawei dongles.

2012-12-14 Thread Felipe Balbi 
On Fri, Dec 14, 2012 at 10:25:25AM +0800, fangxiaozhi 00110321 wrote:
 From: fangxiaozhi huana...@huawei.com
 
 1. Add a new macro define for USB storage match rule.
 2. Optimize the match rules with new macro for Huawei USB storage devices, 
to avoid to load USB storage driver for the modem interface 
with Huawei devices.
 3. Add to support new switch command for new Huawei USB dongles.

still doing too much in a single patch. Should be broken down. Most of
my comments weren't taken into account. Why bother sending a new
version ?

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH 1/1]linux-usb: optimize to match rules in USB storage for Huawei dongles.

2012-12-14 Thread Alan Stern 
On Fri, 14 Dec 2012, fangxiaozhi 00110321 wrote:

 From: fangxiaozhi huana...@huawei.com
 
 1. Add a new macro define for USB storage match rule.
 2. Optimize the match rules with new macro for Huawei USB storage devices, 
to avoid to load USB storage driver for the modem interface 
with Huawei devices.
 3. Add to support new switch command for new Huawei USB dongles.
 
 Signed-off-by: fangxiaozhi huana...@huawei.com
 
 
 diff -uprN linux-3.7_bak/drivers/usb/storage/initializers.c 
 linux-3.7/drivers/usb/storage/initializers.c
 --- linux-3.7_bak/drivers/usb/storage/initializers.c  2012-12-11 
 09:56:11.0 +0800
 +++ linux-3.7/drivers/usb/storage/initializers.c  2012-12-13 
 20:53:40.0 +0800

 @@ -104,3 +104,55 @@ int usb_stor_huawei_e220_init(struct us_
   US_DEBUGP(Huawei mode set result is %d\n, result);
   return 0;
  }
 +
 +/* Find the supported Huawei USB dongles */
 +static int usb_stor_huawei_dongles_pid(struct us_data *us)
 +{
 + struct usb_interface_descriptor *idesc;
 + int idProduct;

There should be a blank line here.

 + idesc = us-pusb_intf-cur_altsetting-desc;
 + idProduct = us-pusb_dev-descriptor.idProduct;
 + if (idesc  idesc-bInterfaceNumber == 0) {
 + if ((idProduct == 0x1001)
 + || (idProduct == 0x1003)
 + || (idProduct == 0x1004)
 + || (idProduct = 0x1401  idProduct  0x1501)
 + || (idProduct  0x1504  idProduct = 0x1600)
 + || (idProduct = 0x1c02  idProduct = 0x2202)) {
 + return 1;
 + }
 + }
 + return 0;
 +}
 +
 +static int usb_stor_huawei_scsi_init(struct us_data *us)
 +{
 + int result = 0;
 + int act_len = 0;
 + char rewind_cmd[] = {0x11, 0x06, 0x20, 0x00, 0x00, 0x01, 0x01, 0x00,
 + 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
 + struct bulk_cb_wrap bcbw = {0};

This is wrong.  You cannot use a buffer that is allocated on the
stack.  You should use us-iobuf for this purpose, like
usb_stor_Bulk_transport() does.

There should be a blank line here.

 + bcbw.Signature = cpu_to_le32(US_BULK_CB_SIGN);
 + bcbw.Tag = 0;
 + bcbw.DataTransferLength = cpu_to_le32(0);

What is the purpose of rearranging the byte order of a 0 value?  All
the bytes are equal to 0; the order doesn't matter.

 + bcbw.Flags = bcbw.Lun = 0;
 + bcbw.Length = sizeof(rewind_cmd);
 + memcpy(bcbw.CDB, rewind_cmd, sizeof(rewind_cmd));
 +
 + result = usb_stor_bulk_transfer_buf(us, us-send_bulk_pipe, bcbw,
 + US_BULK_CS_WRAP_LEN, act_len);
 + US_DEBUGP(transfer actual length=%d, result=%d\n, act_len, result);
 + return result;
 +}

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/1]linux-usb: optimize to match rules in USB storage for Huawei dongles.

2012-12-13 Thread fangxiaozhi 00110321 
From: fangxiaozhi huana...@huawei.com

1. Add a new macro define for USB storage match rule.
2. Optimize the match rules with new macro for Huawei USB storage devices, 
   to avoid to load USB storage driver for the modem interface 
   with Huawei devices.
3. Add to support new switch command for new Huawei USB dongles.

Signed-off-by: fangxiaozhi huana...@huawei.com


diff -uprN linux-3.7_bak/drivers/usb/storage/initializers.c 
linux-3.7/drivers/usb/storage/initializers.c
--- linux-3.7_bak/drivers/usb/storage/initializers.c2012-12-11 
09:56:11.0 +0800
+++ linux-3.7/drivers/usb/storage/initializers.c2012-12-13 
20:53:40.0 +0800
@@ -92,8 +92,8 @@ int usb_stor_ucr61s2b_init(struct us_dat
return 0;
 }
 
-/* This places the HUAWEI E220 devices in multi-port mode */
-int usb_stor_huawei_e220_init(struct us_data *us)
+/* This places the HUAWEI usb dongles in multi-port mode */
+static int usb_stor_huawei_feature_init(struct us_data *us)
 {
int result;
 
@@ -104,3 +104,55 @@ int usb_stor_huawei_e220_init(struct us_
US_DEBUGP(Huawei mode set result is %d\n, result);
return 0;
 }
+
+/* Find the supported Huawei USB dongles */
+static int usb_stor_huawei_dongles_pid(struct us_data *us)
+{
+   struct usb_interface_descriptor *idesc;
+   int idProduct;
+   idesc = us-pusb_intf-cur_altsetting-desc;
+   idProduct = us-pusb_dev-descriptor.idProduct;
+   if (idesc  idesc-bInterfaceNumber == 0) {
+   if ((idProduct == 0x1001)
+   || (idProduct == 0x1003)
+   || (idProduct == 0x1004)
+   || (idProduct = 0x1401  idProduct  0x1501)
+   || (idProduct  0x1504  idProduct = 0x1600)
+   || (idProduct = 0x1c02  idProduct = 0x2202)) {
+   return 1;
+   }
+   }
+   return 0;
+}
+
+static int usb_stor_huawei_scsi_init(struct us_data *us)
+{
+   int result = 0;
+   int act_len = 0;
+   char rewind_cmd[] = {0x11, 0x06, 0x20, 0x00, 0x00, 0x01, 0x01, 0x00,
+   0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+   struct bulk_cb_wrap bcbw = {0};
+   bcbw.Signature = cpu_to_le32(US_BULK_CB_SIGN);
+   bcbw.Tag = 0;
+   bcbw.DataTransferLength = cpu_to_le32(0);
+   bcbw.Flags = bcbw.Lun = 0;
+   bcbw.Length = sizeof(rewind_cmd);
+   memcpy(bcbw.CDB, rewind_cmd, sizeof(rewind_cmd));
+
+   result = usb_stor_bulk_transfer_buf(us, us-send_bulk_pipe, bcbw,
+   US_BULK_CS_WRAP_LEN, act_len);
+   US_DEBUGP(transfer actual length=%d, result=%d\n, act_len, result);
+   return result;
+}
+
+int usb_stor_huawei_init(struct us_data *us)
+{
+   int result = 0;
+   if (usb_stor_huawei_dongles_pid(us)) {
+   if (us-pusb_dev-descriptor.idProduct = 0x1446)
+   result = usb_stor_huawei_scsi_init(us);
+   else
+   result = usb_stor_huawei_feature_init(us);
+   }
+   return result;
+}
diff -uprN linux-3.7_bak/drivers/usb/storage/initializers.h 
linux-3.7/drivers/usb/storage/initializers.h
--- linux-3.7_bak/drivers/usb/storage/initializers.h2012-12-11 
09:56:11.0 +0800
+++ linux-3.7/drivers/usb/storage/initializers.h2012-12-12 
11:43:58.0 +0800
@@ -46,5 +46,5 @@ int usb_stor_euscsi_init(struct us_data 
  * flash reader */
 int usb_stor_ucr61s2b_init(struct us_data *us);
 
-/* This places the HUAWEI E220 devices in multi-port mode */
-int usb_stor_huawei_e220_init(struct us_data *us);
+/* This places the HUAWEI usb dongles in multi-port mode */
+int usb_stor_huawei_init(struct us_data *us);
diff -uprN linux-3.7_bak/drivers/usb/storage/unusual_devs.h 
linux-3.7/drivers/usb/storage/unusual_devs.h
--- linux-3.7_bak/drivers/usb/storage/unusual_devs.h2012-12-11 
09:56:11.0 +0800
+++ linux-3.7/drivers/usb/storage/unusual_devs.h2012-12-13 
20:51:23.0 +0800
@@ -1527,335 +1527,10 @@ UNUSUAL_DEV(  0x1210, 0x0003, 0x0100, 0x
 /* Reported by fangxiaozhi huana...@huawei.com
  * This brings the HUAWEI data card devices into multi-port mode
  */
-UNUSUAL_DEV(  0x12d1, 0x1001, 0x, 0x,
+UNUSUAL_VENDOR_INTF(0x12d1, 0x08, 0x06, 0x50,
HUAWEI MOBILE,
Mass Storage,
-   USB_SC_DEVICE, USB_PR_DEVICE, usb_stor_huawei_e220_init,
-   0),
-UNUSUAL_DEV(  0x12d1, 0x1003, 0x, 0x,
-   HUAWEI MOBILE,
-   Mass Storage,
-   USB_SC_DEVICE, USB_PR_DEVICE, usb_stor_huawei_e220_init,
-   0),
-UNUSUAL_DEV(  0x12d1, 0x1004, 0x, 0x,
-   HUAWEI MOBILE,
-   Mass Storage,
-   USB_SC_DEVICE, USB_PR_DEVICE, usb_stor_huawei_e220_init,
-   0),
-UNUSUAL_DEV(  0x12d1, 0x1401, 0x, 0x,
-