cxacru: Create sysfs attributes in atm_start instead of bind
Subsystems (3)
PATCH 1/2 cxacru: Create sysfs attributes in atm_start instead of bind
Since usbatm doesn't set the usb_interface driver data until after calling bind and heavy_init, it would be NULL when the sysfs attributes are read. Reading the MAC address from atm_dev before atm_dev exists would have been be possible too.
Calling create_device_file in atm_start will avoid this problem, and the data is useless until the first status poll runs. However, it must be ready before a status poll does a printk on line status change otherwise userspace could react before the files exist.
For completeness I've moved remove_device_file to atm_stop so it's not called in unbind when it's not needed.
---
This is a resend of my email 28/04/07 with part of the description removed due to patch 2/2.
Quoted 1 line
#undef
CXACRU_DEVICE_REVOVE_FILE
Oops.
drivers/usb/atm/cxacru.c | 45 +++++++++++++++++++++++++--------------------
1 files changed, 25 insertions(+), 20 deletions(-)
drivers/usb/atm/cxacru.c +0 -0
@@ -618,10 +618,22 @@ static int cxacru_card_status(struct cxacru_data *instance)
return 0; }
+static void cxacru_remove_device_files(struct usbatm_data *usbatm_instance,
+ struct atm_dev *atm_dev)
+{
+ struct usb_interface *intf = usbatm_instance->usb_intf;
+
+ #define CXACRU_DEVICE_REMOVE_FILE(_name) \
+ device_remove_file(&intf->dev, &dev_attr_##_name);
+ CXACRU_ALL_FILES(REMOVE);
+ #undef CXACRU_DEVICE_REMOVE_FILE
+}
+
static int cxacru_atm_start(struct usbatm_data *usbatm_instance,
struct atm_dev *atm_dev)
{
struct cxacru_data *instance = usbatm_instance->driver_data;
+ struct usb_interface *intf = usbatm_instance->usb_intf;
/*
struct atm_dev *atm_dev = usbatm_instance->atm_dev;
*/
@@ -638,6 +650,13 @@ static int cxacru_atm_start(struct usbatm_data *usbatm_instance,
return ret;
}+ #define CXACRU_DEVICE_CREATE_FILE(_name) \
+ ret = device_create_file(&intf->dev, &dev_attr_##_name); \
+ if (unlikely(ret)) \
+ goto fail_sysfs;
+ CXACRU_ALL_FILES(CREATE);
+ #undef CXACRU_DEVICE_CREATE_FILE
+
/* start ADSL */ mutex_lock(&instance->adsl_state_serialize); ret = cxacru_cm(instance, CM_REQUEST_CHIP_ADSL_LINE_START, NULL, 0, NULL, 0); @@ -669,6 +688,11 @@ static int cxacru_atm_start(struct usbatm_data *usbatm_instance, if (start_polling) cxacru_poll_status(&instance->poll_work.work); return 0;
+
+fail_sysfs:
+ usb_err(usbatm_instance, "cxacru_atm_start: device_create_file failed (%d)\n", ret);
+ cxacru_remove_device_files(usbatm_instance, atm_dev);
+ return ret;
}
static void cxacru_poll_status(struct work_struct *work)
@@ -1054,13 +1078,6 @@ static int cxacru_bind(struct usbatm_data *usbatm_instance,
goto fail; }
- #define CXACRU_DEVICE_CREATE_FILE(_name) \
- ret = device_create_file(&intf->dev, &dev_attr_##_name); \
- if (unlikely(ret)) \
- goto fail_sysfs;
- CXACRU_ALL_FILES(CREATE);
- #undef CXACRU_DEVICE_CREATE_FILE
-
usb_fill_int_urb(instance->rcv_urb, usb_dev, usb_rcvintpipe(usb_dev, CXACRU_EP_CMD), instance->rcv_buf, PAGE_SIZE,
@@ -1081,14 +1098,6 @@ static int cxacru_bind(struct usbatm_data *usbatm_instance,
return 0;
- fail_sysfs:
- dbg("cxacru_bind: device_create_file failed (%d)\n", ret);
-
- #define CXACRU_DEVICE_REMOVE_FILE(_name) \
- device_remove_file(&intf->dev, &dev_attr_##_name);
- CXACRU_ALL_FILES(REMOVE);
- #undef CXACRU_DEVICE_REVOVE_FILE
-
fail:
free_page((unsigned long) instance->snd_buf); free_page((unsigned long) instance->rcv_buf); @@ -1135,11 +1144,6 @@ static void cxacru_unbind(struct usbatm_data *usbatm_instance, free_page((unsigned long) instance->snd_buf); free_page((unsigned long) instance->rcv_buf);
- #define CXACRU_DEVICE_REMOVE_FILE(_name) \
- device_remove_file(&intf->dev, &dev_attr_##_name);
- CXACRU_ALL_FILES(REMOVE);
- #undef CXACRU_DEVICE_REVOVE_FILE
-
kfree(instance); usbatm_instance->driver_data = NULL;
@@ -1220,6 +1224,7 @@ static struct usbatm_driver cxacru_driver = {
.heavy_init = cxacru_heavy_init,
.unbind = cxacru_unbind,
.atm_start = cxacru_atm_start,
+ .atm_stop = cxacru_remove_device_files,
.bulk_in = CXACRU_EP_DATA, .bulk_out = CXACRU_EP_DATA, .rx_padding = 3,
Signature · 4 lines
1.5.0.1 -- Simon Arlott
Quoted 4 lines · On Fri, 04 May 2007 18:03:22 +0100 Simon Arlott <[email protected]> wrote:
Since usbatm doesn't set the
usb_interfacedriver data until after calling bind andheavy_init, it would be NULL when the sysfs attributes are read. Reading the MAC address fromatm_devbeforeatm_devexists would have been be possible too.
Quoted 8 lines
Calling
create_device_fileinatm_startwill avoid this problem, and the data is useless until the first status poll runs. However, it must be ready before a status poll does a printk on line status change otherwise userspace could react before the files exist.For completeness I've moved
remove_device_filetoatm_stopso it's not called in unbind when it's not needed.
This patch is mysteriously mangled.
Quoted 16 lines
drivers/usb/atm/cxacru.c| 45 +++++++++++++++++++++++++--------------------1 files changed, 25 insertions(+), 20 deletions(-)drivers/usb/atm/cxacru.c +4
-0@@ -618,10 +618,22 @@ static int cxacru_card_status(struct cxacru_data *instance)return 0 ; } +static void cxacru_remove_device_files (struct usbatm_data *usbatm_instance, +struct atm_dev *atm_dev) +{ +struct usb_interface *intf = usbatm_instance->usb_intf;
drivers/usb/atm/cxacru.c static int cxacru_card_status(struct cxacru_data *instance) · line 634
Quoted 10 lines
drivers/usb/atm/cxacru.c +7
-0+ +#define CXACRU_DEVICE_REMOVE_FILE (_name) \ + device_remove_file(&intf->dev, &dev_attr_##_name); +CXACRU_ALL_FILES (REMOVE ); +#undef CXACRU_DEVICE_REMOVE_FILE +} +static int cxacru_atm_start (struct usbatm_data *usbatm_instance,struct atm_dev *atm_dev) {
See, that "^{" should have been "^ {". Something has gobbled the leading space.
Anyway, after some futzing around I was able to determine that this patch is identical to the one I already have. Ho hum.