2015-05-05 6:41 GMT+09:00 James Bottomley
<James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>:
> On Mon, 2015-05-04 at 16:09 -0400, Alan Stern wrote:
>> On Mon, 4 May 2015, James Bottomley wrote:
>>
>> > However, it does also strike me that these three drivers have problems
>> > because they're using the wrong initialisation pattern: the template is
>> > supposed to be in the bus connector for compound drivers not in the
>> > core.
>>
>> Why is it supposed to be done that way? Isn't that less efficient? It
>> means you have to have a separate copy of the template for each bus
>> connector driver, instead of letting them all share a common template
>> in the core.
>
> Well, no it doesn't. The way 53c700 implements it is that there is a
> common template in the core. The drivers just initialise their variant
> fields (for 53c700 it's name, proc_name and this_id) and the core fills
> in the rest. Admittedly wd33c93 doesn't quite get this right, that's
> why I cited 53c700.
I have converted usb-storage and ums-alauda in the attached patch.
Each ums-* driver needs to expand module_usb_driver() macro as we need
to initialize their scsi host template in module_init().
Alan, how do you feel the change like this?
From 4c6aa0b56c153afe331a98969a857f16d99f96b6 Mon Sep 17 00:00:00 2001
From: Akinobu Mita <akinobu.mita@xxxxxxxxx>
Date: Tue, 5 May 2015 22:30:04 +0900
Subject: [PATCH] usb: storage: fix module reference for scsi host
---
drivers/usb/storage/alauda.c | 21 +++++++++++++++++++--
drivers/usb/storage/scsiglue.c | 12 +++++++++++-
drivers/usb/storage/scsiglue.h | 3 ++-
drivers/usb/storage/usb.c | 25 +++++++++++++++++++++----
drivers/usb/storage/usb.h | 3 ++-
5 files changed, 55 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/storage/alauda.c b/drivers/usb/storage/alauda.c
index 4b55ab6..0baa484 100644
--- a/drivers/usb/storage/alauda.c
+++ b/drivers/usb/storage/alauda.c
@@ -42,6 +42,7 @@
#include "transport.h"
#include "protocol.h"
#include "debug.h"
+#include "scsiglue.h"
MODULE_DESCRIPTION("Driver for Alauda-based card readers");
MODULE_AUTHOR("Daniel Drake <dsd@xxxxxxxxxx>");
@@ -1232,6 +1233,8 @@ static int alauda_transport(struct scsi_cmnd *srb, struct us_data *us)
return USB_STOR_TRANSPORT_FAILED;
}
+static struct scsi_host_template alauda_host_template;
+
static int alauda_probe(struct usb_interface *intf,
const struct usb_device_id *id)
{
@@ -1239,7 +1242,8 @@ 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,
+ &alauda_host_template);
if (result)
return result;
@@ -1266,4 +1270,17 @@ static struct usb_driver alauda_driver = {
.no_dynamic_id = 1,
};
-module_usb_driver(alauda_driver);
+static int __init alauda_driver_init(void)
+{
+ usb_stor_host_template_init(&alauda_host_template, "ums-alauda",
+ THIS_MODULE);
+
+ return usb_register(&alauda_driver);
+}
+module_init(alauda_driver_init);
+
+static void __exit alauda_driver_exit(void)
+{
+ usb_deregister(&alauda_driver);
+}
+module_exit(alauda_driver_exit);
diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
index 0e400f3..05b8fe2 100644
--- a/drivers/usb/storage/scsiglue.c
+++ b/drivers/usb/storage/scsiglue.c
@@ -540,7 +540,7 @@ static struct device_attribute *sysfs_device_attr_list[] = {
* this defines our host template, with which we'll allocate hosts
*/
-struct scsi_host_template usb_stor_host_template = {
+static struct scsi_host_template usb_stor_host_template = {
/* basic userland interface stuff */
.name = "usb-storage",
.proc_name = "usb-storage",
@@ -592,6 +592,16 @@ struct scsi_host_template usb_stor_host_template = {
.module = THIS_MODULE
};
+void usb_stor_host_template_init(struct scsi_host_template *sht,
+ const char *name, struct module *owner)
+{
+ memcpy(sht, &usb_stor_host_template, sizeof(*sht));
+ sht->name = name;
+ sht->proc_name = name;
+ sht->module = owner;
+}
+EXPORT_SYMBOL_GPL(usb_stor_host_template_init);
+
/* To Report "Illegal Request: Invalid Field in CDB */
unsigned char usb_stor_sense_invalidCDB[18] = {
[0] = 0x70, /* current error */
diff --git a/drivers/usb/storage/scsiglue.h b/drivers/usb/storage/scsiglue.h
index ffa1cca..1909601 100644
--- a/drivers/usb/storage/scsiglue.h
+++ b/drivers/usb/storage/scsiglue.h
@@ -43,6 +43,7 @@ extern void usb_stor_report_device_reset(struct us_data *us);
extern void usb_stor_report_bus_reset(struct us_data *us);
extern unsigned char usb_stor_sense_invalidCDB[18];
-extern struct scsi_host_template usb_stor_host_template;
+extern void usb_stor_host_template_init(struct scsi_host_template *sht,
+ const char *name, struct module *owner);
#endif
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 6c10c88..0f76899 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -924,7 +924,8 @@ 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,
- struct us_unusual_dev *unusual_dev)
+ struct us_unusual_dev *unusual_dev,
+ struct scsi_host_template *sht)
{
struct Scsi_Host *host;
struct us_data *us;
@@ -936,7 +937,7 @@ int usb_stor_probe1(struct us_data **pus,
* Ask the SCSI layer to allocate a host structure, with extra
* space at the end for our private us_data structure.
*/
- host = scsi_host_alloc(&usb_stor_host_template, sizeof(*us));
+ host = scsi_host_alloc(sht, sizeof(*us));
if (!host) {
dev_warn(&intf->dev, "Unable to allocate the scsi host\n");
return -ENOMEM;
@@ -1073,6 +1074,8 @@ void usb_stor_disconnect(struct usb_interface *intf)
}
EXPORT_SYMBOL_GPL(usb_stor_disconnect);
+static struct scsi_host_template usb_stor_host_template;
+
/* The main probe routine for standard devices */
static int storage_probe(struct usb_interface *intf,
const struct usb_device_id *id)
@@ -1113,7 +1116,8 @@ static int storage_probe(struct usb_interface *intf,
id->idVendor, id->idProduct);
}
- result = usb_stor_probe1(&us, intf, id, unusual_dev);
+ result = usb_stor_probe1(&us, intf, id, unusual_dev,
+ &usb_stor_host_template);
if (result)
return result;
@@ -1137,4 +1141,17 @@ static struct usb_driver usb_storage_driver = {
.soft_unbind = 1,
};
-module_usb_driver(usb_storage_driver);
+static int __init usb_storage_driver_init(void)
+{
+ usb_stor_host_template_init(&usb_stor_host_template, "usb-storage",
+ THIS_MODULE);
+
+ return usb_register(&usb_storage_driver);
+}
+module_init(usb_storage_driver_init);
+
+static void __exit usb_storage_driver_exit(void)
+{
+ usb_deregister(&usb_storage_driver);
+}
+module_exit(usb_storage_driver_exit);
diff --git a/drivers/usb/storage/usb.h b/drivers/usb/storage/usb.h
index 307e339..a00ec4f 100644
--- a/drivers/usb/storage/usb.h
+++ b/drivers/usb/storage/usb.h
@@ -197,7 +197,8 @@ 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,
- struct us_unusual_dev *unusual_dev);
+ struct us_unusual_dev *unusual_dev,
+ struct scsi_host_template *sht);
extern int usb_stor_probe2(struct us_data *us);
extern void usb_stor_disconnect(struct usb_interface *intf);
--
1.9.1
