From: Henk - Apart from the trivial rename from usb_probe to yealink_probe I tried to fixup on Dmitrys comments. - I have just one assumption: sysfs_remove_group will kill any active sessions to the corresponding sysfs files. Is that ok? To: Andrew Morton To: Dmitry Torokhov Cc: Greg KH Cc: Vojtech Pavlik Signed-off-by: Henk Signed-off-by: Andrew Morton --- drivers/usb/input/map_to_7segment.h | 2 drivers/usb/input/yealink.c | 212 +++++++++++++++++++----------------- drivers/usb/input/yealink.h | 43 ------- 3 files changed, 116 insertions(+), 141 deletions(-) diff -puN drivers/usb/input/map_to_7segment.h~yealink-updates drivers/usb/input/map_to_7segment.h --- devel/drivers/usb/input/map_to_7segment.h~yealink-updates 2005-06-30 22:28:58.000000000 -0700 +++ devel-akpm/drivers/usb/input/map_to_7segment.h 2005-06-30 22:28:58.000000000 -0700 @@ -1,5 +1,5 @@ /* - * include/map/map_to_7segment.h + * drivers/usb/input/map_to_7segment.h * * Copyright (c) 2005 Henk Vergonet * diff -puN drivers/usb/input/yealink.c~yealink-updates drivers/usb/input/yealink.c --- devel/drivers/usb/input/yealink.c~yealink-updates 2005-06-30 22:28:58.000000000 -0700 +++ devel-akpm/drivers/usb/input/yealink.c 2005-06-30 22:28:58.000000000 -0700 @@ -43,6 +43,7 @@ * will pop-up on the ../input/eventX bus. * 20050531 henk Added led, LCD, dialtone and sysfs interface. * 20050610 henk Cleanups, make it ready for public consumption. + * 20050630 henk Cleanups, fixes in response to comments. */ #include @@ -54,15 +55,14 @@ #include #include -// #include /* proposed location */ #include "map_to_7segment.h" -#define DRIVER_VERSION "yld-20050610" +#define DRIVER_VERSION "yld-20050630" #define DRIVER_AUTHOR "Henk Vergonet" #define DRIVER_DESC "Yealink phone driver" #define USB_PKT_LEN 16 -#define MAX_REPEAT_COUNT 16 +#define MAX_REPEAT_COUNT 8 struct yld_status { u8 lcd[24]; @@ -70,11 +70,21 @@ struct yld_status { u8 tone; } __attribute__ ((packed)); -struct lcd_segment_map { - char type, value; - /* actually the "value" should be part of an yld structure as it's the - * only part that is not a constant. - */ +/* + * Register the LCD segment and icon map + */ +#define _LOC(k,l) { .a = (k), .m = (l) } +#define _SEG(t, v, a, am, b, bm, c, cm, d, dm, e, em, f, fm, g, gm) \ + { .type = (t), \ + .u = { .s = { _LOC(a, am), _LOC(b, bm), _LOC(c, cm), \ + _LOC(d, dm), _LOC(e, em), _LOC(g, gm), \ + _LOC(f, fm) } } } +#define _PIC(t, v, h, hm, n) \ + { .type = (t), \ + .u = { .p = { .name = (n), .a = (h), .m = (hm) } } } + +static const struct lcd_segment_map { + char type; union { struct pictogram_map { u8 a,m; @@ -84,13 +94,32 @@ struct lcd_segment_map { u8 a,m; } s[7]; } u; +} lcdMap[] = { +#include "yealink.h" }; -/* get the lcd_segment_map */ -#define _REGISTER_TABLES +/* + * Register the command table + */ + +/* Register command strings */ +#define _TABLE(lbl,a,b,c,d,e,f,g,h,i,j,k,l,m,n,o) \ + static const u8 cmd_##lbl [USB_PKT_LEN] = { \ + a,b,c,d,e,f,g,h,i,j,k,l,m,n,o, \ + (-a-b-c-d-e-f-g-h-i-j-k-l-m-n-o) & 0xff \ + }; #include "yealink.h" -/* Register a 7 segment character set */ +/* Register host notations of CMD opcodes */ +#define _TABLE(lbl,a,b,c,d,e,f,g,h,i,j,k,l,m,n,o) \ + OP16_##lbl = __constant_ntohs((a) << 8 | (b)), +enum u16 { +#include "yealink.h" +}; + +/* + * Register a default 7 segment character set + */ static SEG7_DEFAULT_MAP(map_seg7); struct yealink_dev { @@ -109,9 +138,9 @@ struct yealink_dev { dma_addr_t ctl_req_dma; struct urb *urb_ctl; - int open_count; /* reference count */ char phys[64]; /* physical device path */ + u8 lcdMap[ARRAY_SIZE(lcdMap)]; /* state of LCD, LED ... */ int key_number; int key_code; @@ -136,13 +165,13 @@ static int setChar(struct yealink_dev *y { int i, a, m, val; - if (el >= sizeof(lcdMap)/sizeof(lcdMap[0])) + if (el >= ARRAY_SIZE(lcdMap)) return -EINVAL; if (chr == '\t' || chr == '\n') return 0; - lcdMap[el].value = chr; + yld->lcdMap[el] = chr; if (lcdMap[el].type == '.') { a = lcdMap[el].u.p.a; @@ -268,33 +297,33 @@ static int yealink_do_idle_tasks(struct for (i = 0; i < sizeof(yld->master); i++) { ix = (yld->stat_ix + i) % sizeof(yld->master); - if (yld->master.b[ix] != yld->copy.b[ix]) - goto update; + if (yld->master.b[ix] != yld->copy.b[ix]) { + /* something needs an update */ + yld->copy.b[ix] = val = yld->master.b[ix]; + + if (ix == offsetof(struct yld_status, tone)) { + memcpy(buf, cmd_TONE, USB_PKT_LEN); + buf[4] = val; + buf[15] -= val; + } else if (ix == offsetof(struct yld_status, led)) { + memcpy(buf, cmd_LED_USB, USB_PKT_LEN); + if (val) { + buf[3] = 0x0; + buf[4] = 0xff; + } + } else { + memcpy(buf, cmd_LCD, USB_PKT_LEN); + buf[3] = ix; + buf[4] = val; + buf[15] -= (ix + val); + } + yld->stat_ix = (ix + 1) % sizeof(yld->master); + return 1; + } } yld->idle_repeat = 0; yld->stat_ix = (yld->stat_ix + 1) % sizeof(yld->master); return 0; -update: - yld->copy.b[ix] = val = yld->master.b[ix]; - - if (ix == offsetof(struct yld_status, tone)) { - memcpy(buf, cmd_TONE, USB_PKT_LEN); - buf[4] = val; - buf[15] -= val; - } else if (ix == offsetof(struct yld_status, led)) { - memcpy(buf, cmd_LED_USB, USB_PKT_LEN); - if (val) { - buf[3] = 0x0; - buf[4] = 0xff; - } - } else { - memcpy(buf, cmd_LCD, USB_PKT_LEN); - buf[3] = ix; - buf[4] = val; - buf[15] -= (ix + val); - } - yld->stat_ix = (ix + 1) % sizeof(yld->master); - return 1; } /* Decide on how to handle responses @@ -396,7 +425,7 @@ static void urb_irq_callback(struct urb /* figure out what todo next */ if ((ret = yealink_state_machine(yld, regs)) != 0) - err ("%s - yealink_state_machine result %d", __FUNCTION__, ret); + err("%s - yealink_state_machine result %d", __FUNCTION__, ret); } static void urb_ctl_callback(struct urb *urb, struct pt_regs *regs) @@ -417,11 +446,10 @@ static void urb_ctl_callback(struct urb * input event interface ******************************************************************************/ -// TODO should we issue a ringtone on a SND_BELL event? +/* TODO should we issue a ringtone on a SND_BELL event? */ static int input_ev(struct input_dev *dev, unsigned int type, unsigned int code, int value) { - // struct yealink_dev *yld = dev->private; if (type != EV_SND) return -EINVAL; @@ -444,9 +472,6 @@ static int input_open(struct input_dev * dbg("%s", __FUNCTION__); - if (yld->open_count++) - return 0; - /* force updates to device */ for (i = 0; imaster); i++) yld->copy.b[i] = ~yld->master.b[i]; @@ -460,7 +485,6 @@ static int input_open(struct input_dev * if ((ret = usb_submit_urb(yld->urb_ctl, GFP_KERNEL)) != 0) { dbg("%s - usb_submit_urb failed with result %d", __FUNCTION__, ret); - yld->open_count--; return ret; } return 0; @@ -471,8 +495,7 @@ static void input_close(struct input_dev struct yealink_dev *yld = dev->private; dbg("%s", __FUNCTION__); - if (!--yld->open_count) - usb_kill_urb(yld->urb_irq); + usb_kill_urb(yld->urb_irq); } /******************************************************************************* @@ -505,14 +528,16 @@ static ssize_t store_map(struct device * * 888888888888 * Linux Rocks! */ -static ssize_t show_line(char *buf, int a, int b) +static ssize_t show_line(struct device *dev, char *buf, int a, int b) { + struct yealink_dev *yld = dev_get_drvdata(dev); int i = 0; + for (i = a; i < b; i++) *buf++ = lcdMap[i].type; *buf++ = '\n'; for (i = a; i < b; i++) - *buf++ = lcdMap[i].value; + *buf++ = yld->lcdMap[i]; *buf++ = '\n'; *buf = 0; return 3 + ((b - a) << 1); @@ -521,19 +546,19 @@ static ssize_t show_line(char *buf, int static ssize_t show_line1(struct device *dev, struct device_attribute *attr, char *buf) { - return show_line(buf, LCD_LINE1_OFFSET, LCD_LINE2_OFFSET); + return show_line(dev, buf, LCD_LINE1_OFFSET, LCD_LINE2_OFFSET); } static ssize_t show_line2(struct device *dev, struct device_attribute *attr, char *buf) { - return show_line(buf, LCD_LINE2_OFFSET, LCD_LINE3_OFFSET); + return show_line(dev, buf, LCD_LINE2_OFFSET, LCD_LINE3_OFFSET); } static ssize_t show_line3(struct device *dev, struct device_attribute *attr, char *buf) { - return show_line(buf, LCD_LINE3_OFFSET, LCD_LINE4_OFFSET); + return show_line(dev, buf, LCD_LINE3_OFFSET, LCD_LINE4_OFFSET); } /* Writing to /sys/../lineX will set the coresponding LCD line. @@ -549,9 +574,6 @@ static ssize_t store_line(struct device struct yealink_dev *yld = dev_get_drvdata(dev); int i; - if (yld == NULL) - return 0; - if (len > count) len = count; for (i = 0; i < len; i++) @@ -585,12 +607,13 @@ static ssize_t store_line3(struct device static ssize_t get_icons(struct device *dev, struct device_attribute *attr, char *buf) { + struct yealink_dev *yld = dev_get_drvdata(dev); int i, ret = 1; for (i = 0; i < ARRAY_SIZE(lcdMap); i++) { if (lcdMap[i].type != '.') continue; ret += sprintf(&buf[ret], "%s %s\n", - lcdMap[i].value == ' ' ? " " : "on", + yld->lcdMap[i] == ' ' ? " " : "on", lcdMap[i].u.p.name); } return ret; @@ -652,38 +675,20 @@ static DEVICE_ATTR(get_icons, S_IRUGO, g static DEVICE_ATTR(show_icon, S_IWUSR|S_IWGRP, NULL, show_icon); static DEVICE_ATTR(hide_icon, S_IWUSR|S_IWGRP, NULL, hide_icon); -static void remove_sysfs_files(struct device *dev) -{ - device_remove_file(dev, &dev_attr_line1); - device_remove_file(dev, &dev_attr_line2); - device_remove_file(dev, &dev_attr_line3); - device_remove_file(dev, &dev_attr_get_icons); - device_remove_file(dev, &dev_attr_show_icon); - device_remove_file(dev, &dev_attr_hide_icon); - device_remove_file(dev, &dev_attr_map_seg7); -} - -static int create_sysfs_files(struct device *dev) -{ - struct yealink_dev *yld = dev_get_drvdata(dev); - int i, ret; +static struct attribute *yld_attributes[] = { + &dev_attr_line1.attr, + &dev_attr_line2.attr, + &dev_attr_line3.attr, + &dev_attr_get_icons.attr, + &dev_attr_show_icon.attr, + &dev_attr_hide_icon.attr, + &dev_attr_map_seg7.attr, + NULL +}; - for (i = 0; i < ARRAY_SIZE(lcdMap); i++) - setChar(yld, i, lcdMap[i].value); - store_line3(dev, NULL, DRIVER_VERSION, sizeof(DRIVER_VERSION)); - if ( (ret = device_create_file(dev, &dev_attr_line1)) || - (ret = device_create_file(dev, &dev_attr_line2)) || - (ret = device_create_file(dev, &dev_attr_line3)) || - (ret = device_create_file(dev, &dev_attr_get_icons)) || - (ret = device_create_file(dev, &dev_attr_show_icon)) || - (ret = device_create_file(dev, &dev_attr_hide_icon)) || - (ret = device_create_file(dev, &dev_attr_map_seg7)) ) - { - err("killing own sysfs device files"); - remove_sysfs_files(dev); - } - return ret; -} +static struct attribute_group yld_attr_group = { + .attrs = yld_attributes, +}; /******************************************************************************* * Linux interface and usb initialisation @@ -732,11 +737,22 @@ static void usb_disconnect(struct usb_in { struct yealink_dev *yld = usb_get_intfdata(intf); + sysfs_remove_group(&intf->dev.kobj, &yld_attr_group); usb_set_intfdata(intf, NULL); - remove_sysfs_files(&intf->dev); usb_cleanup(yld, 0); } +static int usb_match(struct usb_device *udev) +{ + int i; + for (i = 0; i < ARRAY_SIZE(yld_device); i++) { + if ((udev->descriptor.idVendor == yld_device[i].idVendor) && + (udev->descriptor.idProduct == yld_device[i].idProduct)) + return i; + } + return -ENODEV; +} + static int usb_probe(struct usb_interface *intf, const struct usb_device_id *id) { struct usb_device *udev = interface_to_usbdev (intf); @@ -746,15 +762,9 @@ static int usb_probe(struct usb_interfac char path[64]; int ret, pipe, i; - /* test for vendor */ - for (i = 0; i < ARRAY_SIZE(yld_device); i++) { - if ((udev->descriptor.idVendor == yld_device[i].idVendor) && - (udev->descriptor.idProduct == yld_device[i].idProduct)) - goto device_found; - } - return -ENODEV; - -device_found: + i = usb_match(udev); + if (i < 0) + return -ENODEV; interface = intf->cur_altsetting; endpoint = &interface->endpoint[0].desc; @@ -860,7 +870,15 @@ device_found: usb_set_intfdata(intf, yld); - create_sysfs_files(&intf->dev); + /* clear visible elements */ + for (i=0; idev, NULL, DRIVER_VERSION, sizeof(DRIVER_VERSION)); + + /* Register sysfs hooks (don't care about failure) */ + sysfs_create_group(&intf->dev.kobj, &yld_attr_group); return 0; } diff -puN drivers/usb/input/yealink.h~yealink-updates drivers/usb/input/yealink.h --- devel/drivers/usb/input/yealink.h~yealink-updates 2005-06-30 22:28:58.000000000 -0700 +++ devel-akpm/drivers/usb/input/yealink.h 2005-06-30 22:28:58.000000000 -0700 @@ -180,46 +180,3 @@ _TABLE(SPKR, 0x03,0x01,0x00,0x00,0x00,0 #undef _SEG #undef _PIC #endif /* _SEG && _PIC */ - - - -#ifdef _REGISTER_TABLES -#undef _REGISTER_TABLES - -/* - * Command table and defines. - */ - -/* Register command strings */ -#define _TABLE(lbl,a,b,c,d,e,f,g,h,i,j,k,l,m,n,o) \ - static const u8 cmd_##lbl [USB_PKT_LEN] = { \ - a,b,c,d,e,f,g,h,i,j,k,l,m,n,o, \ - (-a-b-c-d-e-f-g-h-i-j-k-l-m-n-o) & 0xff \ - }; -#include "yealink.h" - -/* Register host notations of CMD opcodes */ -#define _TABLE(lbl,a,b,c,d,e,f,g,h,i,j,k,l,m,n,o) \ - OP16_##lbl = __constant_ntohs((a) << 8 | (b)), -enum u16 { -#include "yealink.h" -}; - -/* - * LCD segment and icon map - */ -#define _LOC(k,l) { .a = (k), .m = (l) } -#define _SEG(t, v, a, am, b, bm, c, cm, d, dm, e, em, f, fm, g, gm) \ - { .type = (t), .value = (v), \ - .u = { .s = { _LOC(a, am), _LOC(b, bm), _LOC(c, cm), \ - _LOC(d, dm), _LOC(e, em), _LOC(g, gm), \ - _LOC(f, fm) } } } -#define _PIC(t, v, h, hm, n) \ - { .type = (t), .value = (v), \ - .u = { .p = { .name = (n), .a = (h), .m = (hm) } } } - -static struct lcd_segment_map lcdMap[] = { -#include "yealink.h" -}; - -#endif /* _REGISTER_TABLES */ _