linux-usb:Define a new macro for USB storage match rules

PATCH In review fangxiaozhi 00110321 5 participants 8 messages Jan 25, 2013 – Jan 27, 2013

Subsystems (2)

USB USB mass storage driver

lore

Share & tools

lore

fangxiaozhi 00110321

author

2013-01-25 02:45 UTC #0 lore raw

PATCH 1/2 linux-usb:Define a new macro for USB storage match rules

From: fangxiaozhi <[email protected]>

1. Define a new macro for USB storage match rules:
matching with Vendor ID and interface descriptors.

Signed-off-by fangxiaozhi <[email protected]>

--------------------------------------------------------------------

diff -uprN linux-3.8-rc4_orig/drivers/usb/storage/usb.c linux-3.8-rc4/drivers/usb/storage/usb.c

drivers/usb/storage/usb.c 2013-01-22 14:16:01.398250305 +0800 renamed from drivers/usb/storage/usb.c 2013-01-22 14:12:42.595238727 +0800 +12 -0

@@ -120,6 +120,17 @@ MODULE_PARM_DESC(quirks, "supplemental l
  .useTransport = use_transport, \
 }
 
+#define UNUSUAL_VENDOR_INTF(idVendor, cl, sc, pr, \
+         vendor_name, product_name, use_protocol, use_transport, \
+         init_function, Flags) \
+{ \
+ .vendorName = vendor_name, \
+ .productName = product_name, \
+ .useProtocol = use_protocol, \
+ .useTransport = use_transport, \
+ .initFunction = init_function, \
+}
+
 static struct us_unusual_dev us_unusual_dev_list[] = {
 # include "unusual_devs.h"
  { }         /* Terminating entry */

@@ -131,6 +142,7 @@ static struct us_unusual_dev for_dynamic
 #undef UNUSUAL_DEV
 #undef COMPLIANT_DEV
 #undef USUAL_DEV
+#undef UNUSUAL_VENDOR_INTF
 
 #ifdef CONFIG_LOCKDEP

drivers/usb/storage/usual-tables.c 2013-01-22 14:16:01.426250199 +0800 renamed from drivers/usb/storage/usual-tables.c 2013-01-22 14:12:42.594238726 +0800 +14 -0

diff -uprN linux-3.8-rc4_orig/drivers/usb/storage/usual-tables.c linux-3.8-rc4/drivers/usb/storage/usual-tables.c
@@ -41,6 +41,19 @@
 #define USUAL_DEV(useProto, useTrans) \
 { USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, useProto, useTrans) }
 
+/* Define the device is matched with Vendor ID and interface descriptors */
+#define UNUSUAL_VENDOR_INTF(id_vendor, cl, sc, pr, \
+                 vendorName, productName, useProtocol, useTransport, \
+                 initFunction, flags) \
+{ \
+ .match_flags = USB_DEVICE_ID_MATCH_INT_INFO \
+                         | USB_DEVICE_ID_MATCH_VENDOR, \
+ .idVendor    = (id_vendor), \
+ .bInterfaceClass = (cl), \
+ .bInterfaceSubClass = (sc), \
+ .bInterfaceProtocol = (pr), \
+ .driver_info = (flags) }
+
 struct usb_device_id usb_storage_usb_ids[] = {
 # include "unusual_devs.h"
  { }         /* Terminating entry */

@@ -50,6 +63,7 @@ MODULE_DEVICE_TABLE(usb, usb_storage_usb
 #undef UNUSUAL_DEV
 #undef COMPLIANT_DEV
 #undef USUAL_DEV
+#undef UNUSUAL_VENDOR_INTF
 
 /*
  * The table of devices to ignore

Sergei Shtylyov

2013-01-25 12:18 UTC #1 lore raw

Hello.

Quoted 15 lines · On 25-01-2013 6:44, fangxiaozhi 00110321 wrote:

From: fangxiaozhi <[email protected]>
1. Define a new macro for USB storage match rules:
matching with Vendor ID and interface descriptors.
Signed-off-by fangxiaozhi <[email protected]>
--------------------------------------------------------------------
diff -uprN linux-3.8-rc4_orig/drivers/usb/storage/usb.c linux-3.8-rc4/drivers/usb/storage/usb.c
drivers/usb/storage/usb.c 2013-01-22 14:16:01.398250305 +0800 renamed from drivers/usb/storage/usb.c 2013-01-22 14:12:42.595238727 +0800 +0 -0
@@ -120,6 +120,17 @@ MODULE_PARM_DESC(quirks, "supplemental l
   .useTransport = use_transport, \
  }
 

drivers/usb/storage/usb.c 2013-01-22 14:16:01.398250305 +0800 MODULE_PARM_DESC(quirks, "supplemental l · line 132

Quoted 10 lines

drivers/usb/storage/usb.c 2013-01-22 14:16:01.398250305 +0800 renamed from drivers/usb/storage/usb.c 2013-01-22 14:12:42.595238727 +0800 +10 -0

+#define UNUSUAL_VENDOR_INTF(idVendor, cl, sc, pr, \
+         vendor_name, product_name, use_protocol, use_transport, \
+         init_function, Flags) \
+{ \
+ .vendorName = vendor_name, \
+ .productName = product_name, \
+ .useProtocol = use_protocol, \
+ .useTransport = use_transport, \
+ .initFunction = init_function, \
+}

Shouldn't the field initilaizers be indented with tab, not space?

Quoted 9 lines

diff -uprN linux-3.8-rc4_orig/drivers/usb/storage/usual-tables.c linux-3.8-rc4/drivers/usb/storage/usual-tables.c
drivers/usb/storage/usual-tables.c 2013-01-22 14:16:01.426250199 +0800 renamed from drivers/usb/storage/usual-tables.c 2013-01-22 14:12:42.594238726 +0800 +2 -0
@@ -41,6 +41,19 @@
  #define USUAL_DEV(useProto, useTrans) \
  { USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, useProto, useTrans) }
 
+/* Define the device is matched with Vendor ID and interface descriptors */
+#define UNUSUAL_VENDOR_INTF(id_vendor, cl, sc, pr, \

drivers/usb/storage/usual-tables.c 2013-01-22 14:16:01.426250199 +0800 line 55

Quoted 10 lines

drivers/usb/storage/usual-tables.c 2013-01-22 14:16:01.426250199 +0800 renamed from drivers/usb/storage/usual-tables.c 2013-01-22 14:12:42.594238726 +0800 +10 -0

+                 vendorName, productName, useProtocol, useTransport, \
+                 initFunction, flags) \
+{ \
+ .match_flags = USB_DEVICE_ID_MATCH_INT_INFO \
+                         | USB_DEVICE_ID_MATCH_VENDOR, \
+ .idVendor    = (id_vendor), \
+ .bInterfaceClass = (cl), \
+ .bInterfaceSubClass = (sc), \
+ .bInterfaceProtocol = (pr), \
+ .driver_info = (flags) }

Same question. And trailing '}' should be on a separate line.

WBR, Sergei

Greg KH

2013-01-25 17:44 UTC #2 lore raw

Quoted 19 lines · Sergei Shtylyov

Hello.
Quoted 15 lines · On 25-01-2013 6:44, fangxiaozhi 00110321 wrote:
From: fangxiaozhi <[email protected]>
1. Define a new macro for USB storage match rules:
matching with Vendor ID and interface descriptors.
Signed-off-by fangxiaozhi <[email protected]>
--------------------------------------------------------------------
diff -uprN linux-3.8-rc4_orig/drivers/usb/storage/usb.c linux-3.8-rc4/drivers/usb/storage/usb.c
drivers/usb/storage/usb.c 2013-01-22 14:16:01.398250305 +0800 renamed from drivers/usb/storage/usb.c 2013-01-22 14:12:42.595238727 +0800 +0 -0
@@ -120,6 +120,17 @@ MODULE_PARM_DESC(quirks, "supplemental l
  .useTransport = use_transport, \
 }
 

Quoted 12 lines

drivers/usb/storage/usb.c 2013-01-22 14:16:01.398250305 +0800 MODULE_PARM_DESC(quirks, "supplemental l · line 132
Quoted 10 lines
drivers/usb/storage/usb.c 2013-01-22 14:16:01.398250305 +0800 renamed from drivers/usb/storage/usb.c 2013-01-22 14:12:42.595238727 +0800 +10 -0
+#define UNUSUAL_VENDOR_INTF(idVendor, cl, sc, pr, \
+         vendor_name, product_name, use_protocol, use_transport, \
+         init_function, Flags) \
+{ \
+ .vendorName = vendor_name, \
+ .productName = product_name, \
+ .useProtocol = use_protocol, \
+ .useTransport = use_transport, \
+ .initFunction = init_function, \
+}
Shouldn't the field initilaizers be indented with tab, not space?

Yes it must. fangxiaozhi, please always run your patches through the scripts/checkpatch.pl tool before sending them out (note, you will have to ignore the CamelCase warnings your patch produces, but not the other ones.)

Please do that on both of these patches and resend them.

thanks,

greg k-h

Fangxiaozhi (Franko)

author

2013-01-26 01:40 UTC #3 lore raw

Quoted 49 lines

-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Saturday, January 26, 2013 1:45 AM
To: Fangxiaozhi (Franko)
Cc: Sergei Shtylyov; [email protected]; [email protected]; Xueguiying (Zihan); Linlei (Lei Lin); Yili (Neil); Wangyuhua (Roger, Credit); Huqiao (C); [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 1/2]linux-usb:Define a new macro for USB storage match rules
Quoted 22 lines · Sergei Shtylyov
Hello.
Quoted 18 lines · On 25-01-2013 6:44, fangxiaozhi 00110321 wrote:
From: fangxiaozhi <[email protected]>
1. Define a new macro for USB storage match rules:
matching with Vendor ID and interface descriptors.
Signed-off-by fangxiaozhi <[email protected]>
--------------------------------------------------------------------
diff -uprN linux-3.8-rc4_orig/drivers/usb/storage/usb.c
linux-3.8-rc4/drivers/usb/storage/usb.c
--- linux-3.8-rc4_orig/drivers/usb/storage/usb.c 2013-01-22 14:12:42.595238727 +0800
+++ linux-3.8-rc4/drivers/usb/storage/usb.c 2013-01-22 +++ 14:16:01.398250305 +0800
drivers/usb/storage/usb.c 2013-01-22 14:16:01.398250305 +0800 renamed from drivers/usb/storage/usb.c 2013-01-22 14:12:42.595238727 +0800 +0 -0
@@ -120,6 +120,17 @@ MODULE_PARM_DESC(quirks, "supplemental l
  .useTransport = use_transport, \
 }
 
Quoted 12 lines
drivers/usb/storage/usb.c 2013-01-22 14:16:01.398250305 +0800 MODULE_PARM_DESC(quirks, "supplemental l · line 132
Quoted 10 lines
drivers/usb/storage/usb.c 2013-01-22 14:16:01.398250305 +0800 renamed from drivers/usb/storage/usb.c 2013-01-22 14:12:42.595238727 +0800 +10 -0
+#define UNUSUAL_VENDOR_INTF(idVendor, cl, sc, pr, \
+         vendor_name, product_name, use_protocol, use_transport, \
+         init_function, Flags) \
+{ \
+ .vendorName = vendor_name, \
+ .productName = product_name, \
+ .useProtocol = use_protocol, \
+ .useTransport = use_transport, \
+ .initFunction = init_function, \
+}
Shouldn't the field initilaizers be indented with tab, not space?

Quoted 5 lines

Yes it must. fangxiaozhi, please always run your patches through the scripts/checkpatch.pl tool before sending them out (note, you will have to ignore the CamelCase warnings your patch produces, but not the other ones.)

-----What's wrong with it?
-----I have checked the patches with scripts/checkpatch.pl before sending. -----There is no other warning or error in my patches except CamelCase warnings. -----So what's wrong now?

Quoted 5 lines

Please do that on both of these patches and resend them.
thanks,
greg k-h

��{.n�+��+%��ݶ��w��{.n�+��{��G��{ay�ʇڙ�,j��f��h��z_�(�階�ݢj"��m��G��?��&��~��iO��z��v�^�m��?�I�

Greg KH

2013-01-26 02:04 UTC #4 lore raw

Quoted 54 lines · Fangxiaozhi (Franko)

Quoted 49 lines
-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Saturday, January 26, 2013 1:45 AM
To: Fangxiaozhi (Franko)
Cc: Sergei Shtylyov; [email protected]; [email protected]; Xueguiying (Zihan); Linlei (Lei Lin); Yili (Neil); Wangyuhua (Roger, Credit); Huqiao (C); [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 1/2]linux-usb:Define a new macro for USB storage match rules
Quoted 22 lines · Sergei Shtylyov
Hello.
Quoted 18 lines · On 25-01-2013 6:44, fangxiaozhi 00110321 wrote:
From: fangxiaozhi <[email protected]>
1. Define a new macro for USB storage match rules:
matching with Vendor ID and interface descriptors.
Signed-off-by fangxiaozhi <[email protected]>
--------------------------------------------------------------------
diff -uprN linux-3.8-rc4_orig/drivers/usb/storage/usb.c
linux-3.8-rc4/drivers/usb/storage/usb.c
--- linux-3.8-rc4_orig/drivers/usb/storage/usb.c 2013-01-22 14:12:42.595238727 +0800
+++ linux-3.8-rc4/drivers/usb/storage/usb.c 2013-01-22 +++ 14:16:01.398250305 +0800
drivers/usb/storage/usb.c 2013-01-22 14:16:01.398250305 +0800 renamed from drivers/usb/storage/usb.c 2013-01-22 14:12:42.595238727 +0800 +0 -0
@@ -120,6 +120,17 @@ MODULE_PARM_DESC(quirks, "supplemental l
  .useTransport = use_transport, \
 }
 
Quoted 12 lines
drivers/usb/storage/usb.c 2013-01-22 14:16:01.398250305 +0800 MODULE_PARM_DESC(quirks, "supplemental l · line 132
Quoted 10 lines
drivers/usb/storage/usb.c 2013-01-22 14:16:01.398250305 +0800 renamed from drivers/usb/storage/usb.c 2013-01-22 14:12:42.595238727 +0800 +10 -0
+#define UNUSUAL_VENDOR_INTF(idVendor, cl, sc, pr, \
+         vendor_name, product_name, use_protocol, use_transport, \
+         init_function, Flags) \
+{ \
+ .vendorName = vendor_name, \
+ .productName = product_name, \
+ .useProtocol = use_protocol, \
+ .useTransport = use_transport, \
+ .initFunction = init_function, \
+}
Shouldn't the field initilaizers be indented with tab, not space?
Quoted 5 lines
Yes it must. fangxiaozhi, please always run your patches through the scripts/checkpatch.pl tool before sending them out (note, you will have to ignore the CamelCase warnings your patch produces, but not the other ones.)

Quoted 4 lines

-----What's wrong with it?
-----I have checked the patches with scripts/checkpatch.pl before sending. -----There is no other warning or error in my patches except CamelCase warnings. -----So what's wrong now?

Then your email client messed up the patches and put spaces in the code instead of tabs. Try looking at the message on the mailing list and run that through checkpatch, it will show you the problems.

What I received isn't ok, sorry.

greg k-h

Matthew Dharm

2013-01-26 03:10 UTC #5 lore raw

Quoted 60 lines · Greg KH

Quoted 54 lines · Fangxiaozhi (Franko)
Quoted 49 lines
-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Saturday, January 26, 2013 1:45 AM
To: Fangxiaozhi (Franko)
Cc: Sergei Shtylyov; [email protected]; [email protected]; Xueguiying (Zihan); Linlei (Lei Lin); Yili (Neil); Wangyuhua (Roger, Credit); Huqiao (C); [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 1/2]linux-usb:Define a new macro for USB storage match rules
Quoted 22 lines · Sergei Shtylyov
Hello.
Quoted 18 lines · On 25-01-2013 6:44, fangxiaozhi 00110321 wrote:
From: fangxiaozhi <[email protected]>
1. Define a new macro for USB storage match rules:
matching with Vendor ID and interface descriptors.
Signed-off-by fangxiaozhi <[email protected]>
--------------------------------------------------------------------
diff -uprN linux-3.8-rc4_orig/drivers/usb/storage/usb.c
linux-3.8-rc4/drivers/usb/storage/usb.c
--- linux-3.8-rc4_orig/drivers/usb/storage/usb.c 2013-01-22 14:12:42.595238727 +0800
+++ linux-3.8-rc4/drivers/usb/storage/usb.c 2013-01-22 +++ 14:16:01.398250305 +0800
drivers/usb/storage/usb.c 2013-01-22 14:16:01.398250305 +0800 renamed from drivers/usb/storage/usb.c 2013-01-22 14:12:42.595238727 +0800 +0 -0
@@ -120,6 +120,17 @@ MODULE_PARM_DESC(quirks, "supplemental l
  .useTransport = use_transport, \
 }
 
Quoted 12 lines
drivers/usb/storage/usb.c 2013-01-22 14:16:01.398250305 +0800 MODULE_PARM_DESC(quirks, "supplemental l · line 132
Quoted 10 lines
drivers/usb/storage/usb.c 2013-01-22 14:16:01.398250305 +0800 renamed from drivers/usb/storage/usb.c 2013-01-22 14:12:42.595238727 +0800 +10 -0
+#define UNUSUAL_VENDOR_INTF(idVendor, cl, sc, pr, \
+         vendor_name, product_name, use_protocol, use_transport, \
+         init_function, Flags) \
+{ \
+ .vendorName = vendor_name, \
+ .productName = product_name, \
+ .useProtocol = use_protocol, \
+ .useTransport = use_transport, \
+ .initFunction = init_function, \
+}
Shouldn't the field initilaizers be indented with tab, not space?
Quoted 5 lines
Yes it must. fangxiaozhi, please always run your patches through the scripts/checkpatch.pl tool before sending them out (note, you will have to ignore the CamelCase warnings your patch produces, but not the other ones.)
Quoted 4 lines
-----What's wrong with it?
-----I have checked the patches with scripts/checkpatch.pl before sending. -----There is no other warning or error in my patches except CamelCase warnings. -----So what's wrong now?

Quoted 5 lines

Then your email client messed up the patches and put spaces in the code instead of tabs. Try looking at the message on the mailing list and run that through checkpatch, it will show you the problems.
What I received isn't ok, sorry.

Fangxiaozhi --

According to the headers of your E-mail, you are using MS Outlook to send your patches. Outlook commonly mangles patches, unfortunately. It is not a very good e-mail client.

I suggest one of two options:

1) Setup an alternative mail client. There are many to choose from which will not damage your patches. I personally like 'mutt' (which
you should be able to install on your linux machine). Others may be
able to recommend ones that work for them; in general, I think you will find that most e-mail clients that run on Linux will be suitable.

2) If you plan on contributing to the linux kernel in the future, it may be worth your time to setup a repo on github that Greg can then directly pull from. All you would need to do is send Greg a "pull request" indicating the URL of the branch in your repo that he should pull from. Greg can then pull directly from your repo, bypassing this issue entirely.

Matt

--
Matthew Dharm
Maintainer, USB Mass Storage driver for Linux

Greg KH

2013-01-26 03:37 UTC #6 lore raw

Quoted 7 lines · Matthew Dharm

I suggest one of two options:
1) Setup an alternative mail client. There are many to choose from which will not damage your patches. I personally like 'mutt' (which
you should be able to install on your linux machine). Others may be
able to recommend ones that work for them; in general, I think you will find that most e-mail clients that run on Linux will be suitable.

The file, Documentation/email_clients.txt will help out here.

Quoted 6 lines

2) If you plan on contributing to the linux kernel in the future, it may be worth your time to setup a repo on github that Greg can then directly pull from. All you would need to do is send Greg a "pull request" indicating the URL of the branch in your repo that he should pull from. Greg can then pull directly from your repo, bypassing this issue entirely.

No, sorry, I only pull trees from a _very_ few people, patches are what I prefer for almost all stuff. Only subsystem maintainers who I have been working with for many years will I pull trees from.

sorry,

greg k-h

Sebastian Andrzej Siewior

2013-01-27 18:42 UTC #7 lore raw

In reply to fangxiaozhi 00110321

Quoted 9 lines · On Fri, Jan 25, 2013 at 10:44:50AM +0800, fangxiaozhi 00110321 wrote:

diff -uprN linux-3.8-rc4_orig/drivers/usb/storage/usual-tables.c linux-3.8-rc4/drivers/usb/storage/usual-tables.c
drivers/usb/storage/usual-tables.c 2013-01-22 14:16:01.426250199 +0800 renamed from drivers/usb/storage/usual-tables.c 2013-01-22 14:12:42.594238726 +0800 +2 -0
@@ -41,6 +41,19 @@
 #define USUAL_DEV(useProto, useTrans) \
 { USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, useProto, useTrans) }
 
+/* Define the device is matched with Vendor ID and interface descriptors */
+#define UNUSUAL_VENDOR_INTF(id_vendor, cl, sc, pr, \

drivers/usb/storage/usual-tables.c 2013-01-22 14:16:01.426250199 +0800 line 55

Quoted 10 lines

drivers/usb/storage/usual-tables.c 2013-01-22 14:16:01.426250199 +0800 renamed from drivers/usb/storage/usual-tables.c 2013-01-22 14:12:42.594238726 +0800 +10 -0

+                 vendorName, productName, useProtocol, useTransport, \
+                 initFunction, flags) \
+{ \
+ .match_flags = USB_DEVICE_ID_MATCH_INT_INFO \
+                         | USB_DEVICE_ID_MATCH_VENDOR, \
+ .idVendor    = (id_vendor), \
+ .bInterfaceClass = (cl), \
+ .bInterfaceSubClass = (sc), \
+ .bInterfaceProtocol = (pr), \
+ .driver_info = (flags) }

The braces aren't required and you did not need in UNUSUAL_VENDOR_INTF.

Sebastian