2008-02-29 09:26:01

by Eibach, Dirk

[permalink] [raw]
Subject: [PATCH] usb: add sysfs configuration interface for CP2101

From: Dirk Eibach <[email protected]>

The usb configuration data for the Silabs CP2101 usb to uart bridge
controller can be customized:
- Vendor ID
- Product ID
- Power Descriptor
- Release Number
- Serial Number
- Product Description String

Silabs provides a windows-only tool to do that.
Since we use linux-only machines in our production-environment, we have no
proper way to customize the chips for our purpose.
So I added sysfs configuration support to the linux driver.

Signed-off-by: Dirk Eibach <[email protected]>
---
diff -purN linux-2.6.24/drivers/usb/serial/cp2101.c linux-2.6.24-diff/drivers/usb/serial/cp2101.c
--- linux-2.6.24/drivers/usb/serial/cp2101.c 2008-01-24 23:58:37.000000000 +0100
+++ linux-2.6.24-diff/drivers/usb/serial/cp2101.c 2008-02-29 09:38:30.847817025 +0100
@@ -18,6 +18,8 @@
*/

#include <linux/kernel.h>
+#include <linux/fs.h>
+#include <linux/platform_device.h>
#include <linux/errno.h>
#include <linux/slab.h>
#include <linux/tty.h>
@@ -52,6 +54,8 @@ static void cp2101_shutdown(struct usb_s

static int debug;

+bool enable_config = false;
+
static struct usb_device_id id_table [] = {
{ USB_DEVICE(0x08e6, 0x5501) }, /* Gemalto Prox-PU/CU contactless smartcard reader */
{ USB_DEVICE(0x0FCF, 0x1003) }, /* Dynastream ANT development board */
@@ -125,6 +129,7 @@ static struct usb_serial_driver cp2101_d
#define CP2101_CONTROL 0x07 /* Flow control line states */
#define CP2101_MODEMCTL 0x13 /* Modem controls */
#define CP2101_CONFIG_6 0x19 /* 6 bytes of config data ??? */
+#define CP2101_EEPROM 0xFF /* write configuration eeprom */

/* CP2101_UART */
#define UART_ENABLE 0x0001
@@ -167,6 +172,17 @@ static struct usb_serial_driver cp2101_d
#define CONTROL_WRITE_DTR 0x0100
#define CONTROL_WRITE_RTS 0x0200

+/* CP2101 CONFIGURATION EEPROM */
+#define EEPROM_RELOAD 0x0008
+#define EEPROM_VID 0x3701
+#define EEPROM_PID 0x3702
+#define EEPROM_PRODUCTSTRING 0x3703
+#define EEPROM_SERIALNUMBER 0x3704
+#define EEPROM_POWER_ATTRIB 0x3705
+#define EEPROM_MAX_POWER 0x3706
+#define EEPROM_RELEASE_VERSION 0x3707
+#define EEPROM_LOCK 0x370a
+
/*
* cp2101_get_config
* Reads from the CP2101 configuration registers
@@ -704,11 +720,262 @@ static void cp2101_break_ctl (struct usb
cp2101_set_config(port, CP2101_BREAK, &state, 2);
}

+static ssize_t write_reload(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+
+ /* writing "0" does not trigger */
+ if (!simple_strtoul(buf, NULL, 0))
+ return count;
+
+ usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_RELOAD,
+ 0, NULL, 0, 300);
+
+ return count;
+}
+
+static DEVICE_ATTR(reload, S_IWUGO, NULL, write_reload);
+
+static ssize_t write_vid(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+
+ unsigned long vid = simple_strtoul(buf, NULL, 0);
+
+ if (!vid || vid > 0xffff)
+ return -EINVAL;
+
+ usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_VID,
+ vid, NULL, 0, 300);
+
+ return count;
+}
+
+static DEVICE_ATTR(vid, S_IWUGO, NULL, write_vid);
+
+static ssize_t write_pid(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+
+ unsigned long pid = simple_strtoul(buf, NULL, 0);
+
+ if (!pid || pid > 0xffff)
+ return -EINVAL;
+
+ usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_PID,
+ pid, NULL, 0, 300);
+
+ return count;
+}
+
+static DEVICE_ATTR(pid, S_IWUGO, NULL, write_pid);
+
+static ssize_t write_serialnumber(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ int result;
+ unsigned int k;
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+ u8 serial_stringsize = 2 + 2*count;
+ char serial[serial_stringsize];
+
+ if (count > 63)
+ return -EINVAL;
+
+ serial[0] = serial_stringsize;
+ serial[1] = USB_DT_STRING;
+
+ /* convert to utf-16 */
+ for (k = 0; k < count; ++k) {
+ serial[2+2*k] = buf[k];
+ serial[2+2*k+1] = 0;
+ }
+
+ result = usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_SERIALNUMBER,
+ 0, serial, sizeof(serial), 300);
+
+ if (result != serial_stringsize)
+ return -EIO;
+
+ return count;
+}
+
+static DEVICE_ATTR(serialnumber, S_IWUGO, NULL, write_serialnumber);
+
+
+static ssize_t write_productstring(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ int result;
+ unsigned int k;
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+ u8 product_stringsize = 2 + 2*count;
+ char product[product_stringsize];
+
+ if (count > 126)
+ return -EINVAL;
+
+ product[0] = product_stringsize;
+ product[1] = USB_DT_STRING;
+
+ /* convert to utf-16 */
+ for (k = 0; k < count; ++k) {
+ product[2+2*k] = buf[k];
+ product[2+2*k+1] = 0;
+ }
+
+ result = usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE,
+ EEPROM_PRODUCTSTRING, 0,
+ product, sizeof(product), 300);
+
+ if (result != product_stringsize)
+ return -EIO;
+
+ return count;
+}
+
+static DEVICE_ATTR(productstring, S_IWUGO, NULL, write_productstring);
+
+static ssize_t write_self_powered(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+
+ unsigned long self_powered = simple_strtoul(buf, NULL, 0);
+
+ usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_POWER_ATTRIB,
+ self_powered ? 0x00c0 : 0x0080, NULL, 0, 300);
+
+ return count;
+}
+
+static DEVICE_ATTR(self_powered, S_IWUGO, NULL, write_self_powered);
+
+static ssize_t write_max_power(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+
+ unsigned long max_power = simple_strtoul(buf, NULL, 0);
+
+ if (!max_power || max_power > 0xff)
+ return -EINVAL;
+
+ usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_MAX_POWER,
+ max_power, NULL, 0, 300);
+
+ return count;
+}
+
+static DEVICE_ATTR(max_power, S_IWUGO, NULL, write_max_power);
+
+static ssize_t write_release_version(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+
+ unsigned long release_version = simple_strtoul(buf, NULL, 0);
+
+ if (release_version > 0xffff)
+ return -EINVAL;
+
+ usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_RELEASE_VERSION,
+ release_version, NULL, 0, 300);
+
+ return count;
+}
+
+static DEVICE_ATTR(release_version, S_IWUGO, NULL, write_release_version);
+
+static ssize_t write_lock_forever(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+
+ unsigned long lock = simple_strtoul(buf, NULL, 0);
+
+ /* be really, really sure to know what you are doing here */
+ if (lock != 0xabadbabe)
+ return -EINVAL;
+
+ usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_LOCK,
+ 0x00f0, NULL, 0, 300);
+
+ return count;
+}
+
+static DEVICE_ATTR(lock_forever, S_IWUGO, NULL, write_lock_forever);
+
static int cp2101_startup (struct usb_serial *serial)
{
+ int err;
+
/* CP2101 buffers behave strangely unless device is reset */
usb_reset_device(serial->dev);
+
+ if (!enable_config)
+ return 0;
+
+ err = device_create_file(&serial->dev->dev, &dev_attr_reload);
+ if (err) goto out;
+ err = device_create_file(&serial->dev->dev, &dev_attr_vid);
+ if (err) goto out_reload;
+ err = device_create_file(&serial->dev->dev, &dev_attr_pid);
+ if (err) goto out_vid;
+ err = device_create_file(&serial->dev->dev, &dev_attr_productstring);
+ if (err) goto out_pid;
+ err = device_create_file(&serial->dev->dev, &dev_attr_serialnumber);
+ if (err) goto out_productstring;
+ err = device_create_file(&serial->dev->dev, &dev_attr_self_powered);
+ if (err) goto out_serialnumber;
+ err = device_create_file(&serial->dev->dev, &dev_attr_max_power);
+ if (err) goto out_self_powered;
+ err = device_create_file(&serial->dev->dev, &dev_attr_release_version);
+ if (err) goto out_max_power;
+ err = device_create_file(&serial->dev->dev, &dev_attr_lock_forever);
+ if (err) goto out_release_version;
+
return 0;
+
+out_release_version:
+ device_remove_file(&serial->dev->dev, &dev_attr_release_version);
+out_max_power:
+ device_remove_file(&serial->dev->dev, &dev_attr_max_power);
+out_self_powered:
+ device_remove_file(&serial->dev->dev, &dev_attr_self_powered);
+out_serialnumber:
+ device_remove_file(&serial->dev->dev, &dev_attr_serialnumber);
+out_productstring:
+ device_remove_file(&serial->dev->dev, &dev_attr_productstring);
+out_pid:
+ device_remove_file(&serial->dev->dev, &dev_attr_pid);
+out_vid:
+ device_remove_file(&serial->dev->dev, &dev_attr_vid);
+out_reload:
+ device_remove_file(&serial->dev->dev, &dev_attr_reload);
+out:
+ return err;
}

static void cp2101_shutdown (struct usb_serial *serial)
@@ -721,6 +988,19 @@ static void cp2101_shutdown (struct usb_
for (i=0; i < serial->num_ports; ++i) {
cp2101_cleanup(serial->port[i]);
}
+
+ if (!enable_config)
+ return;
+
+ device_remove_file(&serial->dev->dev, &dev_attr_reload);
+ device_remove_file(&serial->dev->dev, &dev_attr_vid);
+ device_remove_file(&serial->dev->dev, &dev_attr_pid);
+ device_remove_file(&serial->dev->dev, &dev_attr_productstring);
+ device_remove_file(&serial->dev->dev, &dev_attr_serialnumber);
+ device_remove_file(&serial->dev->dev, &dev_attr_self_powered);
+ device_remove_file(&serial->dev->dev, &dev_attr_max_power);
+ device_remove_file(&serial->dev->dev, &dev_attr_release_version);
+ device_remove_file(&serial->dev->dev, &dev_attr_lock_forever);
}

static int __init cp2101_init (void)
@@ -758,3 +1038,6 @@ MODULE_LICENSE("GPL");

module_param(debug, bool, S_IRUGO | S_IWUSR);
MODULE_PARM_DESC(debug, "Enable verbose debugging messages");
+
+module_param(enable_config, bool, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(enable_config, "Enable sysfs access to configuration eeprom.");
diff -purN linux-2.6.24/drivers/usb/serial/cp2101.c.orig linux-2.6.24-diff/drivers/usb/serial/cp2101.c.orig
--- linux-2.6.24/drivers/usb/serial/cp2101.c.orig 1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.24-diff/drivers/usb/serial/cp2101.c.orig 2008-01-24 23:58:37.000000000 +0100
@@ -0,0 +1,760 @@
+/*
+ * Silicon Laboratories CP2101/CP2102 USB to RS232 serial adaptor driver
+ *
+ * Copyright (C) 2005 Craig Shelley ([email protected])
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version
+ * 2 as published by the Free Software Foundation.
+ *
+ * Support to set flow control line levels using TIOCMGET and TIOCMSET
+ * thanks to Karl Hiramoto [email protected]. RTSCTS hardware flow
+ * control thanks to Munir Nassar [email protected]
+ *
+ * Outstanding Issues:
+ * Buffers are not flushed when the port is opened.
+ * Multiple calls to write() may fail with "Resource temporarily unavailable"
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/slab.h>
+#include <linux/tty.h>
+#include <linux/tty_flip.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/usb.h>
+#include <asm/uaccess.h>
+#include <linux/usb/serial.h>
+
+/*
+ * Version Information
+ */
+#define DRIVER_VERSION "v0.07"
+#define DRIVER_DESC "Silicon Labs CP2101/CP2102 RS232 serial adaptor driver"
+
+/*
+ * Function Prototypes
+ */
+static int cp2101_open(struct usb_serial_port*, struct file*);
+static void cp2101_cleanup(struct usb_serial_port*);
+static void cp2101_close(struct usb_serial_port*, struct file*);
+static void cp2101_get_termios(struct usb_serial_port*);
+static void cp2101_set_termios(struct usb_serial_port*, struct ktermios*);
+static int cp2101_tiocmget (struct usb_serial_port *, struct file *);
+static int cp2101_tiocmset (struct usb_serial_port *, struct file *,
+ unsigned int, unsigned int);
+static void cp2101_break_ctl(struct usb_serial_port*, int);
+static int cp2101_startup (struct usb_serial *);
+static void cp2101_shutdown(struct usb_serial*);
+
+
+static int debug;
+
+static struct usb_device_id id_table [] = {
+ { USB_DEVICE(0x08e6, 0x5501) }, /* Gemalto Prox-PU/CU contactless smartcard reader */
+ { USB_DEVICE(0x0FCF, 0x1003) }, /* Dynastream ANT development board */
+ { USB_DEVICE(0x0FCF, 0x1004) }, /* Dynastream ANT2USB */
+ { USB_DEVICE(0x10A6, 0xAA26) }, /* Knock-off DCU-11 cable */
+ { USB_DEVICE(0x10AB, 0x10C5) }, /* Siemens MC60 Cable */
+ { USB_DEVICE(0x10B5, 0xAC70) }, /* Nokia CA-42 USB */
+ { USB_DEVICE(0x10C4, 0x803B) }, /* Pololu USB-serial converter */
+ { USB_DEVICE(0x10C4, 0x8053) }, /* Enfora EDG1228 */
+ { USB_DEVICE(0x10C4, 0x8066) }, /* Argussoft In-System Programmer */
+ { USB_DEVICE(0x10C4, 0x807A) }, /* Crumb128 board */
+ { USB_DEVICE(0x10C4, 0x80CA) }, /* Degree Controls Inc */
+ { USB_DEVICE(0x10C4, 0x80DD) }, /* Tracient RFID */
+ { USB_DEVICE(0x10C4, 0x80F6) }, /* Suunto sports instrument */
+ { USB_DEVICE(0x10C4, 0x813D) }, /* Burnside Telecom Deskmobile */
+ { USB_DEVICE(0x10C4, 0x814A) }, /* West Mountain Radio RIGblaster P&P */
+ { USB_DEVICE(0x10C4, 0x814B) }, /* West Mountain Radio RIGtalk */
+ { USB_DEVICE(0x10C4, 0x815E) }, /* Helicomm IP-Link 1220-DVM */
+ { USB_DEVICE(0x10C4, 0x81C8) }, /* Lipowsky Industrie Elektronik GmbH, Baby-JTAG */
+ { USB_DEVICE(0x10C4, 0x81E2) }, /* Lipowsky Industrie Elektronik GmbH, Baby-LIN */
+ { USB_DEVICE(0x10C4, 0x81E7) }, /* Aerocomm Radio */
+ { USB_DEVICE(0x10C4, 0x8218) }, /* Lipowsky Industrie Elektronik GmbH, HARP-1 */
+ { USB_DEVICE(0x10C4, 0xEA60) }, /* Silicon Labs factory default */
+ { USB_DEVICE(0x10C4, 0xEA61) }, /* Silicon Labs factory default */
+ { USB_DEVICE(0x10C5, 0xEA61) }, /* Silicon Labs MobiData GPRS USB Modem */
+ { USB_DEVICE(0x13AD, 0x9999) }, /* Baltech card reader */
+ { USB_DEVICE(0x16D6, 0x0001) }, /* Jablotron serial interface */
+ { } /* Terminating Entry */
+};
+
+MODULE_DEVICE_TABLE (usb, id_table);
+
+static struct usb_driver cp2101_driver = {
+ .name = "cp2101",
+ .probe = usb_serial_probe,
+ .disconnect = usb_serial_disconnect,
+ .id_table = id_table,
+ .no_dynamic_id = 1,
+};
+
+static struct usb_serial_driver cp2101_device = {
+ .driver = {
+ .owner = THIS_MODULE,
+ .name = "cp2101",
+ },
+ .usb_driver = &cp2101_driver,
+ .id_table = id_table,
+ .num_interrupt_in = 0,
+ .num_bulk_in = NUM_DONT_CARE,
+ .num_bulk_out = NUM_DONT_CARE,
+ .num_ports = 1,
+ .open = cp2101_open,
+ .close = cp2101_close,
+ .break_ctl = cp2101_break_ctl,
+ .set_termios = cp2101_set_termios,
+ .tiocmget = cp2101_tiocmget,
+ .tiocmset = cp2101_tiocmset,
+ .attach = cp2101_startup,
+ .shutdown = cp2101_shutdown,
+};
+
+/* Config request types */
+#define REQTYPE_HOST_TO_DEVICE 0x41
+#define REQTYPE_DEVICE_TO_HOST 0xc1
+
+/* Config SET requests. To GET, add 1 to the request number */
+#define CP2101_UART 0x00 /* Enable / Disable */
+#define CP2101_BAUDRATE 0x01 /* (BAUD_RATE_GEN_FREQ / baudrate) */
+#define CP2101_BITS 0x03 /* 0x(0)(databits)(parity)(stopbits) */
+#define CP2101_BREAK 0x05 /* On / Off */
+#define CP2101_CONTROL 0x07 /* Flow control line states */
+#define CP2101_MODEMCTL 0x13 /* Modem controls */
+#define CP2101_CONFIG_6 0x19 /* 6 bytes of config data ??? */
+
+/* CP2101_UART */
+#define UART_ENABLE 0x0001
+#define UART_DISABLE 0x0000
+
+/* CP2101_BAUDRATE */
+#define BAUD_RATE_GEN_FREQ 0x384000
+
+/* CP2101_BITS */
+#define BITS_DATA_MASK 0X0f00
+#define BITS_DATA_5 0X0500
+#define BITS_DATA_6 0X0600
+#define BITS_DATA_7 0X0700
+#define BITS_DATA_8 0X0800
+#define BITS_DATA_9 0X0900
+
+#define BITS_PARITY_MASK 0x00f0
+#define BITS_PARITY_NONE 0x0000
+#define BITS_PARITY_ODD 0x0010
+#define BITS_PARITY_EVEN 0x0020
+#define BITS_PARITY_MARK 0x0030
+#define BITS_PARITY_SPACE 0x0040
+
+#define BITS_STOP_MASK 0x000f
+#define BITS_STOP_1 0x0000
+#define BITS_STOP_1_5 0x0001
+#define BITS_STOP_2 0x0002
+
+/* CP2101_BREAK */
+#define BREAK_ON 0x0000
+#define BREAK_OFF 0x0001
+
+/* CP2101_CONTROL */
+#define CONTROL_DTR 0x0001
+#define CONTROL_RTS 0x0002
+#define CONTROL_CTS 0x0010
+#define CONTROL_DSR 0x0020
+#define CONTROL_RING 0x0040
+#define CONTROL_DCD 0x0080
+#define CONTROL_WRITE_DTR 0x0100
+#define CONTROL_WRITE_RTS 0x0200
+
+/*
+ * cp2101_get_config
+ * Reads from the CP2101 configuration registers
+ * 'size' is specified in bytes.
+ * 'data' is a pointer to a pre-allocated array of integers large
+ * enough to hold 'size' bytes (with 4 bytes to each integer)
+ */
+static int cp2101_get_config(struct usb_serial_port* port, u8 request,
+ unsigned int *data, int size)
+{
+ struct usb_serial *serial = port->serial;
+ __le32 *buf;
+ int result, i, length;
+
+ /* Number of integers required to contain the array */
+ length = (((size - 1) | 3) + 1)/4;
+
+ buf = kcalloc(length, sizeof(__le32), GFP_KERNEL);
+ if (!buf) {
+ dev_err(&port->dev, "%s - out of memory.\n", __FUNCTION__);
+ return -ENOMEM;
+ }
+
+ /* For get requests, the request number must be incremented */
+ request++;
+
+ /* Issue the request, attempting to read 'size' bytes */
+ result = usb_control_msg (serial->dev,usb_rcvctrlpipe (serial->dev, 0),
+ request, REQTYPE_DEVICE_TO_HOST, 0x0000,
+ 0, buf, size, 300);
+
+ /* Convert data into an array of integers */
+ for (i=0; i<length; i++)
+ data[i] = le32_to_cpu(buf[i]);
+
+ kfree(buf);
+
+ if (result != size) {
+ dev_err(&port->dev, "%s - Unable to send config request, "
+ "request=0x%x size=%d result=%d\n",
+ __FUNCTION__, request, size, result);
+ return -EPROTO;
+ }
+
+ return 0;
+}
+
+/*
+ * cp2101_set_config
+ * Writes to the CP2101 configuration registers
+ * Values less than 16 bits wide are sent directly
+ * 'size' is specified in bytes.
+ */
+static int cp2101_set_config(struct usb_serial_port* port, u8 request,
+ unsigned int *data, int size)
+{
+ struct usb_serial *serial = port->serial;
+ __le32 *buf;
+ int result, i, length;
+
+ /* Number of integers required to contain the array */
+ length = (((size - 1) | 3) + 1)/4;
+
+ buf = kmalloc(length * sizeof(__le32), GFP_KERNEL);
+ if (!buf) {
+ dev_err(&port->dev, "%s - out of memory.\n",
+ __FUNCTION__);
+ return -ENOMEM;
+ }
+
+ /* Array of integers into bytes */
+ for (i = 0; i < length; i++)
+ buf[i] = cpu_to_le32(data[i]);
+
+ if (size > 2) {
+ result = usb_control_msg (serial->dev,
+ usb_sndctrlpipe(serial->dev, 0),
+ request, REQTYPE_HOST_TO_DEVICE, 0x0000,
+ 0, buf, size, 300);
+ } else {
+ result = usb_control_msg (serial->dev,
+ usb_sndctrlpipe(serial->dev, 0),
+ request, REQTYPE_HOST_TO_DEVICE, data[0],
+ 0, NULL, 0, 300);
+ }
+
+ kfree(buf);
+
+ if ((size > 2 && result != size) || result < 0) {
+ dev_err(&port->dev, "%s - Unable to send request, "
+ "request=0x%x size=%d result=%d\n",
+ __FUNCTION__, request, size, result);
+ return -EPROTO;
+ }
+
+ /* Single data value */
+ result = usb_control_msg (serial->dev,
+ usb_sndctrlpipe(serial->dev, 0),
+ request, REQTYPE_HOST_TO_DEVICE, data[0],
+ 0, NULL, 0, 300);
+ return 0;
+}
+
+/*
+ * cp2101_set_config_single
+ * Convenience function for calling cp2101_set_config on single data values
+ * without requiring an integer pointer
+ */
+static inline int cp2101_set_config_single(struct usb_serial_port* port,
+ u8 request, unsigned int data)
+{
+ return cp2101_set_config(port, request, &data, 2);
+}
+
+static int cp2101_open (struct usb_serial_port *port, struct file *filp)
+{
+ struct usb_serial *serial = port->serial;
+ int result;
+
+ dbg("%s - port %d", __FUNCTION__, port->number);
+
+ if (cp2101_set_config_single(port, CP2101_UART, UART_ENABLE)) {
+ dev_err(&port->dev, "%s - Unable to enable UART\n",
+ __FUNCTION__);
+ return -EPROTO;
+ }
+
+ /* Start reading from the device */
+ usb_fill_bulk_urb (port->read_urb, serial->dev,
+ usb_rcvbulkpipe(serial->dev,
+ port->bulk_in_endpointAddress),
+ port->read_urb->transfer_buffer,
+ port->read_urb->transfer_buffer_length,
+ serial->type->read_bulk_callback,
+ port);
+ result = usb_submit_urb(port->read_urb, GFP_KERNEL);
+ if (result) {
+ dev_err(&port->dev, "%s - failed resubmitting read urb, "
+ "error %d\n", __FUNCTION__, result);
+ return result;
+ }
+
+ /* Configure the termios structure */
+ cp2101_get_termios(port);
+
+ /* Set the DTR and RTS pins low */
+ cp2101_tiocmset(port, NULL, TIOCM_DTR | TIOCM_RTS, 0);
+
+ return 0;
+}
+
+static void cp2101_cleanup (struct usb_serial_port *port)
+{
+ struct usb_serial *serial = port->serial;
+
+ dbg("%s - port %d", __FUNCTION__, port->number);
+
+ if (serial->dev) {
+ /* shutdown any bulk reads that might be going on */
+ if (serial->num_bulk_out)
+ usb_kill_urb(port->write_urb);
+ if (serial->num_bulk_in)
+ usb_kill_urb(port->read_urb);
+ }
+}
+
+static void cp2101_close (struct usb_serial_port *port, struct file * filp)
+{
+ dbg("%s - port %d", __FUNCTION__, port->number);
+
+ /* shutdown our urbs */
+ dbg("%s - shutting down urbs", __FUNCTION__);
+ usb_kill_urb(port->write_urb);
+ usb_kill_urb(port->read_urb);
+
+ cp2101_set_config_single(port, CP2101_UART, UART_DISABLE);
+}
+
+/*
+ * cp2101_get_termios
+ * Reads the baud rate, data bits, parity, stop bits and flow control mode
+ * from the device, corrects any unsupported values, and configures the
+ * termios structure to reflect the state of the device
+ */
+static void cp2101_get_termios (struct usb_serial_port *port)
+{
+ unsigned int cflag, modem_ctl[4];
+ int baud;
+ int bits;
+
+ dbg("%s - port %d", __FUNCTION__, port->number);
+
+ if (!port->tty || !port->tty->termios) {
+ dbg("%s - no tty structures", __FUNCTION__);
+ return;
+ }
+
+ cp2101_get_config(port, CP2101_BAUDRATE, &baud, 2);
+ /* Convert to baudrate */
+ if (baud)
+ baud = BAUD_RATE_GEN_FREQ / baud;
+
+ dbg("%s - baud rate = %d", __FUNCTION__, baud);
+
+ tty_encode_baud_rate(port->tty, baud, baud);
+ cflag = port->tty->termios->c_cflag;
+
+ cp2101_get_config(port, CP2101_BITS, &bits, 2);
+ cflag &= ~CSIZE;
+ switch(bits & BITS_DATA_MASK) {
+ case BITS_DATA_5:
+ dbg("%s - data bits = 5", __FUNCTION__);
+ cflag |= CS5;
+ break;
+ case BITS_DATA_6:
+ dbg("%s - data bits = 6", __FUNCTION__);
+ cflag |= CS6;
+ break;
+ case BITS_DATA_7:
+ dbg("%s - data bits = 7", __FUNCTION__);
+ cflag |= CS7;
+ break;
+ case BITS_DATA_8:
+ dbg("%s - data bits = 8", __FUNCTION__);
+ cflag |= CS8;
+ break;
+ case BITS_DATA_9:
+ dbg("%s - data bits = 9 (not supported, "
+ "using 8 data bits)", __FUNCTION__);
+ cflag |= CS8;
+ bits &= ~BITS_DATA_MASK;
+ bits |= BITS_DATA_8;
+ cp2101_set_config(port, CP2101_BITS, &bits, 2);
+ break;
+ default:
+ dbg("%s - Unknown number of data bits, "
+ "using 8", __FUNCTION__);
+ cflag |= CS8;
+ bits &= ~BITS_DATA_MASK;
+ bits |= BITS_DATA_8;
+ cp2101_set_config(port, CP2101_BITS, &bits, 2);
+ break;
+ }
+
+ switch(bits & BITS_PARITY_MASK) {
+ case BITS_PARITY_NONE:
+ dbg("%s - parity = NONE", __FUNCTION__);
+ cflag &= ~PARENB;
+ break;
+ case BITS_PARITY_ODD:
+ dbg("%s - parity = ODD", __FUNCTION__);
+ cflag |= (PARENB|PARODD);
+ break;
+ case BITS_PARITY_EVEN:
+ dbg("%s - parity = EVEN", __FUNCTION__);
+ cflag &= ~PARODD;
+ cflag |= PARENB;
+ break;
+ case BITS_PARITY_MARK:
+ dbg("%s - parity = MARK (not supported, "
+ "disabling parity)", __FUNCTION__);
+ cflag &= ~PARENB;
+ bits &= ~BITS_PARITY_MASK;
+ cp2101_set_config(port, CP2101_BITS, &bits, 2);
+ break;
+ case BITS_PARITY_SPACE:
+ dbg("%s - parity = SPACE (not supported, "
+ "disabling parity)", __FUNCTION__);
+ cflag &= ~PARENB;
+ bits &= ~BITS_PARITY_MASK;
+ cp2101_set_config(port, CP2101_BITS, &bits, 2);
+ break;
+ default:
+ dbg("%s - Unknown parity mode, "
+ "disabling parity", __FUNCTION__);
+ cflag &= ~PARENB;
+ bits &= ~BITS_PARITY_MASK;
+ cp2101_set_config(port, CP2101_BITS, &bits, 2);
+ break;
+ }
+
+ cflag &= ~CSTOPB;
+ switch(bits & BITS_STOP_MASK) {
+ case BITS_STOP_1:
+ dbg("%s - stop bits = 1", __FUNCTION__);
+ break;
+ case BITS_STOP_1_5:
+ dbg("%s - stop bits = 1.5 (not supported, "
+ "using 1 stop bit)", __FUNCTION__);
+ bits &= ~BITS_STOP_MASK;
+ cp2101_set_config(port, CP2101_BITS, &bits, 2);
+ break;
+ case BITS_STOP_2:
+ dbg("%s - stop bits = 2", __FUNCTION__);
+ cflag |= CSTOPB;
+ break;
+ default:
+ dbg("%s - Unknown number of stop bits, "
+ "using 1 stop bit", __FUNCTION__);
+ bits &= ~BITS_STOP_MASK;
+ cp2101_set_config(port, CP2101_BITS, &bits, 2);
+ break;
+ }
+
+ cp2101_get_config(port, CP2101_MODEMCTL, modem_ctl, 16);
+ if (modem_ctl[0] & 0x0008) {
+ dbg("%s - flow control = CRTSCTS", __FUNCTION__);
+ cflag |= CRTSCTS;
+ } else {
+ dbg("%s - flow control = NONE", __FUNCTION__);
+ cflag &= ~CRTSCTS;
+ }
+
+ port->tty->termios->c_cflag = cflag;
+}
+
+static void cp2101_set_termios (struct usb_serial_port *port,
+ struct ktermios *old_termios)
+{
+ unsigned int cflag, old_cflag;
+ int baud=0, bits;
+ unsigned int modem_ctl[4];
+
+ dbg("%s - port %d", __FUNCTION__, port->number);
+
+ if (!port->tty || !port->tty->termios) {
+ dbg("%s - no tty structures", __FUNCTION__);
+ return;
+ }
+ port->tty->termios->c_cflag &= ~CMSPAR;
+
+ cflag = port->tty->termios->c_cflag;
+ old_cflag = old_termios->c_cflag;
+ baud = tty_get_baud_rate(port->tty);
+
+ /* If the baud rate is to be updated*/
+ if (baud != tty_termios_baud_rate(old_termios)) {
+ switch (baud) {
+ case 0:
+ case 600:
+ case 1200:
+ case 1800:
+ case 2400:
+ case 4800:
+ case 7200:
+ case 9600:
+ case 14400:
+ case 19200:
+ case 28800:
+ case 38400:
+ case 55854:
+ case 57600:
+ case 115200:
+ case 127117:
+ case 230400:
+ case 460800:
+ case 921600:
+ case 3686400:
+ break;
+ default:
+ baud = 9600;
+ break;
+ }
+
+ if (baud) {
+ dbg("%s - Setting baud rate to %d baud", __FUNCTION__,
+ baud);
+ if (cp2101_set_config_single(port, CP2101_BAUDRATE,
+ (BAUD_RATE_GEN_FREQ / baud))) {
+ dev_err(&port->dev, "Baud rate requested not "
+ "supported by device\n");
+ baud = tty_termios_baud_rate(old_termios);
+ }
+ }
+ }
+ /* Report back the resulting baud rate */
+ tty_encode_baud_rate(port->tty, baud, baud);
+
+ /* If the number of data bits is to be updated */
+ if ((cflag & CSIZE) != (old_cflag & CSIZE)) {
+ cp2101_get_config(port, CP2101_BITS, &bits, 2);
+ bits &= ~BITS_DATA_MASK;
+ switch (cflag & CSIZE) {
+ case CS5:
+ bits |= BITS_DATA_5;
+ dbg("%s - data bits = 5", __FUNCTION__);
+ break;
+ case CS6:
+ bits |= BITS_DATA_6;
+ dbg("%s - data bits = 6", __FUNCTION__);
+ break;
+ case CS7:
+ bits |= BITS_DATA_7;
+ dbg("%s - data bits = 7", __FUNCTION__);
+ break;
+ case CS8:
+ bits |= BITS_DATA_8;
+ dbg("%s - data bits = 8", __FUNCTION__);
+ break;
+ /*case CS9:
+ bits |= BITS_DATA_9;
+ dbg("%s - data bits = 9", __FUNCTION__);
+ break;*/
+ default:
+ dev_err(&port->dev, "cp2101 driver does not "
+ "support the number of bits requested,"
+ " using 8 bit mode\n");
+ bits |= BITS_DATA_8;
+ break;
+ }
+ if (cp2101_set_config(port, CP2101_BITS, &bits, 2))
+ dev_err(&port->dev, "Number of data bits requested "
+ "not supported by device\n");
+ }
+
+ if ((cflag & (PARENB|PARODD)) != (old_cflag & (PARENB|PARODD))) {
+ cp2101_get_config(port, CP2101_BITS, &bits, 2);
+ bits &= ~BITS_PARITY_MASK;
+ if (cflag & PARENB) {
+ if (cflag & PARODD) {
+ bits |= BITS_PARITY_ODD;
+ dbg("%s - parity = ODD", __FUNCTION__);
+ } else {
+ bits |= BITS_PARITY_EVEN;
+ dbg("%s - parity = EVEN", __FUNCTION__);
+ }
+ }
+ if (cp2101_set_config(port, CP2101_BITS, &bits, 2))
+ dev_err(&port->dev, "Parity mode not supported "
+ "by device\n");
+ }
+
+ if ((cflag & CSTOPB) != (old_cflag & CSTOPB)) {
+ cp2101_get_config(port, CP2101_BITS, &bits, 2);
+ bits &= ~BITS_STOP_MASK;
+ if (cflag & CSTOPB) {
+ bits |= BITS_STOP_2;
+ dbg("%s - stop bits = 2", __FUNCTION__);
+ } else {
+ bits |= BITS_STOP_1;
+ dbg("%s - stop bits = 1", __FUNCTION__);
+ }
+ if (cp2101_set_config(port, CP2101_BITS, &bits, 2))
+ dev_err(&port->dev, "Number of stop bits requested "
+ "not supported by device\n");
+ }
+
+ if ((cflag & CRTSCTS) != (old_cflag & CRTSCTS)) {
+ cp2101_get_config(port, CP2101_MODEMCTL, modem_ctl, 16);
+ dbg("%s - read modem controls = 0x%.4x 0x%.4x 0x%.4x 0x%.4x",
+ __FUNCTION__, modem_ctl[0], modem_ctl[1],
+ modem_ctl[2], modem_ctl[3]);
+
+ if (cflag & CRTSCTS) {
+ modem_ctl[0] &= ~0x7B;
+ modem_ctl[0] |= 0x09;
+ modem_ctl[1] = 0x80;
+ dbg("%s - flow control = CRTSCTS", __FUNCTION__);
+ } else {
+ modem_ctl[0] &= ~0x7B;
+ modem_ctl[0] |= 0x01;
+ modem_ctl[1] |= 0x40;
+ dbg("%s - flow control = NONE", __FUNCTION__);
+ }
+
+ dbg("%s - write modem controls = 0x%.4x 0x%.4x 0x%.4x 0x%.4x",
+ __FUNCTION__, modem_ctl[0], modem_ctl[1],
+ modem_ctl[2], modem_ctl[3]);
+ cp2101_set_config(port, CP2101_MODEMCTL, modem_ctl, 16);
+ }
+
+}
+
+static int cp2101_tiocmset (struct usb_serial_port *port, struct file *file,
+ unsigned int set, unsigned int clear)
+{
+ int control = 0;
+
+ dbg("%s - port %d", __FUNCTION__, port->number);
+
+ if (set & TIOCM_RTS) {
+ control |= CONTROL_RTS;
+ control |= CONTROL_WRITE_RTS;
+ }
+ if (set & TIOCM_DTR) {
+ control |= CONTROL_DTR;
+ control |= CONTROL_WRITE_DTR;
+ }
+ if (clear & TIOCM_RTS) {
+ control &= ~CONTROL_RTS;
+ control |= CONTROL_WRITE_RTS;
+ }
+ if (clear & TIOCM_DTR) {
+ control &= ~CONTROL_DTR;
+ control |= CONTROL_WRITE_DTR;
+ }
+
+ dbg("%s - control = 0x%.4x", __FUNCTION__, control);
+
+ return cp2101_set_config(port, CP2101_CONTROL, &control, 2);
+
+}
+
+static int cp2101_tiocmget (struct usb_serial_port *port, struct file *file)
+{
+ int control, result;
+
+ dbg("%s - port %d", __FUNCTION__, port->number);
+
+ cp2101_get_config(port, CP2101_CONTROL, &control, 1);
+
+ result = ((control & CONTROL_DTR) ? TIOCM_DTR : 0)
+ |((control & CONTROL_RTS) ? TIOCM_RTS : 0)
+ |((control & CONTROL_CTS) ? TIOCM_CTS : 0)
+ |((control & CONTROL_DSR) ? TIOCM_DSR : 0)
+ |((control & CONTROL_RING)? TIOCM_RI : 0)
+ |((control & CONTROL_DCD) ? TIOCM_CD : 0);
+
+ dbg("%s - control = 0x%.2x", __FUNCTION__, control);
+
+ return result;
+}
+
+static void cp2101_break_ctl (struct usb_serial_port *port, int break_state)
+{
+ int state;
+
+ dbg("%s - port %d", __FUNCTION__, port->number);
+ if (break_state == 0)
+ state = BREAK_OFF;
+ else
+ state = BREAK_ON;
+ dbg("%s - turning break %s", __FUNCTION__,
+ state==BREAK_OFF ? "off" : "on");
+ cp2101_set_config(port, CP2101_BREAK, &state, 2);
+}
+
+static int cp2101_startup (struct usb_serial *serial)
+{
+ /* CP2101 buffers behave strangely unless device is reset */
+ usb_reset_device(serial->dev);
+ return 0;
+}
+
+static void cp2101_shutdown (struct usb_serial *serial)
+{
+ int i;
+
+ dbg("%s", __FUNCTION__);
+
+ /* Stop reads and writes on all ports */
+ for (i=0; i < serial->num_ports; ++i) {
+ cp2101_cleanup(serial->port[i]);
+ }
+}
+
+static int __init cp2101_init (void)
+{
+ int retval;
+
+ retval = usb_serial_register(&cp2101_device);
+ if (retval)
+ return retval; /* Failed to register */
+
+ retval = usb_register(&cp2101_driver);
+ if (retval) {
+ /* Failed to register */
+ usb_serial_deregister(&cp2101_device);
+ return retval;
+ }
+
+ /* Success */
+ info(DRIVER_DESC " " DRIVER_VERSION);
+ return 0;
+}
+
+static void __exit cp2101_exit (void)
+{
+ usb_deregister (&cp2101_driver);
+ usb_serial_deregister (&cp2101_device);
+}
+
+module_init(cp2101_init);
+module_exit(cp2101_exit);
+
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_VERSION(DRIVER_VERSION);
+MODULE_LICENSE("GPL");
+
+module_param(debug, bool, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(debug, "Enable verbose debugging messages");



2008-02-29 09:40:44

by Eibach, Dirk

[permalink] [raw]
Subject: Re: [PATCH] usb: add sysfs configuration interface for CP2101

[email protected] wrote:
> From: Dirk Eibach <[email protected]>
>
> The usb configuration data for the Silabs CP2101 usb to uart bridge
> controller can be customized:
> - Vendor ID
> - Product ID
> - Power Descriptor
> - Release Number
> - Serial Number
> - Product Description String
>
> Silabs provides a windows-only tool to do that.
> Since we use linux-only machines in our production-environment, we have no
> proper way to customize the chips for our purpose.
> So I added sysfs configuration support to the linux driver.
>
> Signed-off-by: Dirk Eibach <[email protected]>
> ---
Sorry, I have to resubmit the patch, because a backup copy crept in...

diff -purN linux-2.6.24/drivers/usb/serial/cp2101.c linux-2.6.24-diff/drivers/usb/serial/cp2101.c
--- linux-2.6.24/drivers/usb/serial/cp2101.c 2008-01-24 23:58:37.000000000 +0100
+++ linux-2.6.24-diff/drivers/usb/serial/cp2101.c 2008-02-29 10:35:13.300208929 +0100
@@ -18,6 +18,8 @@
*/

#include <linux/kernel.h>
+#include <linux/fs.h>
+#include <linux/platform_device.h>
#include <linux/errno.h>
#include <linux/slab.h>
#include <linux/tty.h>
@@ -52,6 +54,8 @@ static void cp2101_shutdown(struct usb_s

static int debug;

+bool enable_config = false;
+
static struct usb_device_id id_table [] = {
{ USB_DEVICE(0x08e6, 0x5501) }, /* Gemalto Prox-PU/CU contactless smartcard reader */
{ USB_DEVICE(0x0FCF, 0x1003) }, /* Dynastream ANT development board */
@@ -125,6 +129,7 @@ static struct usb_serial_driver cp2101_d
#define CP2101_CONTROL 0x07 /* Flow control line states */
#define CP2101_MODEMCTL 0x13 /* Modem controls */
#define CP2101_CONFIG_6 0x19 /* 6 bytes of config data ??? */
+#define CP2101_EEPROM 0xFF /* write configuration eeprom */

/* CP2101_UART */
#define UART_ENABLE 0x0001
@@ -167,6 +172,17 @@ static struct usb_serial_driver cp2101_d
#define CONTROL_WRITE_DTR 0x0100
#define CONTROL_WRITE_RTS 0x0200

+/* CP2101 CONFIGURATION EEPROM */
+#define EEPROM_RELOAD 0x0008
+#define EEPROM_VID 0x3701
+#define EEPROM_PID 0x3702
+#define EEPROM_PRODUCTSTRING 0x3703
+#define EEPROM_SERIALNUMBER 0x3704
+#define EEPROM_POWER_ATTRIB 0x3705
+#define EEPROM_MAX_POWER 0x3706
+#define EEPROM_RELEASE_VERSION 0x3707
+#define EEPROM_LOCK 0x370a
+
/*
* cp2101_get_config
* Reads from the CP2101 configuration registers
@@ -704,11 +720,262 @@ static void cp2101_break_ctl (struct usb
cp2101_set_config(port, CP2101_BREAK, &state, 2);
}

+static ssize_t write_reload(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+
+ /* writing "0" does not trigger */
+ if (!simple_strtoul(buf, NULL, 0))
+ return count;
+
+ usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_RELOAD,
+ 0, NULL, 0, 300);
+
+ return count;
+}
+
+static DEVICE_ATTR(reload, S_IWUGO, NULL, write_reload);
+
+static ssize_t write_vid(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+
+ unsigned long vid = simple_strtoul(buf, NULL, 0);
+
+ if (!vid || vid > 0xffff)
+ return -EINVAL;
+
+ usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_VID,
+ vid, NULL, 0, 300);
+
+ return count;
+}
+
+static DEVICE_ATTR(vid, S_IWUGO, NULL, write_vid);
+
+static ssize_t write_pid(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+
+ unsigned long pid = simple_strtoul(buf, NULL, 0);
+
+ if (!pid || pid > 0xffff)
+ return -EINVAL;
+
+ usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_PID,
+ pid, NULL, 0, 300);
+
+ return count;
+}
+
+static DEVICE_ATTR(pid, S_IWUGO, NULL, write_pid);
+
+static ssize_t write_serialnumber(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ int result;
+ unsigned int k;
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+ u8 serial_stringsize = 2 + 2*count;
+ char serial[serial_stringsize];
+
+ if (count > 63)
+ return -EINVAL;
+
+ serial[0] = serial_stringsize;
+ serial[1] = USB_DT_STRING;
+
+ /* convert to utf-16 */
+ for (k = 0; k < count; ++k) {
+ serial[2+2*k] = buf[k];
+ serial[2+2*k+1] = 0;
+ }
+
+ result = usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_SERIALNUMBER,
+ 0, serial, sizeof(serial), 300);
+
+ if (result != serial_stringsize)
+ return -EIO;
+
+ return count;
+}
+
+static DEVICE_ATTR(serialnumber, S_IWUGO, NULL, write_serialnumber);
+
+
+static ssize_t write_productstring(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ int result;
+ unsigned int k;
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+ u8 product_stringsize = 2 + 2*count;
+ char product[product_stringsize];
+
+ if (count > 126)
+ return -EINVAL;
+
+ product[0] = product_stringsize;
+ product[1] = USB_DT_STRING;
+
+ /* convert to utf-16 */
+ for (k = 0; k < count; ++k) {
+ product[2+2*k] = buf[k];
+ product[2+2*k+1] = 0;
+ }
+
+ result = usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE,
+ EEPROM_PRODUCTSTRING, 0,
+ product, sizeof(product), 300);
+
+ if (result != product_stringsize)
+ return -EIO;
+
+ return count;
+}
+
+static DEVICE_ATTR(productstring, S_IWUGO, NULL, write_productstring);
+
+static ssize_t write_self_powered(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+
+ unsigned long self_powered = simple_strtoul(buf, NULL, 0);
+
+ usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_POWER_ATTRIB,
+ self_powered ? 0x00c0 : 0x0080, NULL, 0, 300);
+
+ return count;
+}
+
+static DEVICE_ATTR(self_powered, S_IWUGO, NULL, write_self_powered);
+
+static ssize_t write_max_power(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+
+ unsigned long max_power = simple_strtoul(buf, NULL, 0);
+
+ if (!max_power || max_power > 0xff)
+ return -EINVAL;
+
+ usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_MAX_POWER,
+ max_power, NULL, 0, 300);
+
+ return count;
+}
+
+static DEVICE_ATTR(max_power, S_IWUGO, NULL, write_max_power);
+
+static ssize_t write_release_version(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+
+ unsigned long release_version = simple_strtoul(buf, NULL, 0);
+
+ if (release_version > 0xffff)
+ return -EINVAL;
+
+ usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_RELEASE_VERSION,
+ release_version, NULL, 0, 300);
+
+ return count;
+}
+
+static DEVICE_ATTR(release_version, S_IWUGO, NULL, write_release_version);
+
+static ssize_t write_lock_forever(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+
+ unsigned long lock = simple_strtoul(buf, NULL, 0);
+
+ /* be really, really sure to know what you are doing here */
+ if (lock != 0xabadbabe)
+ return -EINVAL;
+
+ usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_LOCK,
+ 0x00f0, NULL, 0, 300);
+
+ return count;
+}
+
+static DEVICE_ATTR(lock_forever, S_IWUGO, NULL, write_lock_forever);
+
static int cp2101_startup (struct usb_serial *serial)
{
+ int err;
+
/* CP2101 buffers behave strangely unless device is reset */
usb_reset_device(serial->dev);
+
+ if (!enable_config)
+ return 0;
+
+ err = device_create_file(&serial->dev->dev, &dev_attr_reload);
+ if (err) goto out;
+ err = device_create_file(&serial->dev->dev, &dev_attr_vid);
+ if (err) goto out_reload;
+ err = device_create_file(&serial->dev->dev, &dev_attr_pid);
+ if (err) goto out_vid;
+ err = device_create_file(&serial->dev->dev, &dev_attr_productstring);
+ if (err) goto out_pid;
+ err = device_create_file(&serial->dev->dev, &dev_attr_serialnumber);
+ if (err) goto out_productstring;
+ err = device_create_file(&serial->dev->dev, &dev_attr_self_powered);
+ if (err) goto out_serialnumber;
+ err = device_create_file(&serial->dev->dev, &dev_attr_max_power);
+ if (err) goto out_self_powered;
+ err = device_create_file(&serial->dev->dev, &dev_attr_release_version);
+ if (err) goto out_max_power;
+ err = device_create_file(&serial->dev->dev, &dev_attr_lock_forever);
+ if (err) goto out_release_version;
+
return 0;
+
+out_release_version:
+ device_remove_file(&serial->dev->dev, &dev_attr_release_version);
+out_max_power:
+ device_remove_file(&serial->dev->dev, &dev_attr_max_power);
+out_self_powered:
+ device_remove_file(&serial->dev->dev, &dev_attr_self_powered);
+out_serialnumber:
+ device_remove_file(&serial->dev->dev, &dev_attr_serialnumber);
+out_productstring:
+ device_remove_file(&serial->dev->dev, &dev_attr_productstring);
+out_pid:
+ device_remove_file(&serial->dev->dev, &dev_attr_pid);
+out_vid:
+ device_remove_file(&serial->dev->dev, &dev_attr_vid);
+out_reload:
+ device_remove_file(&serial->dev->dev, &dev_attr_reload);
+out:
+ return err;
}

static void cp2101_shutdown (struct usb_serial *serial)
@@ -721,6 +988,19 @@ static void cp2101_shutdown (struct usb_
for (i=0; i < serial->num_ports; ++i) {
cp2101_cleanup(serial->port[i]);
}
+
+ if (!enable_config)
+ return;
+
+ device_remove_file(&serial->dev->dev, &dev_attr_reload);
+ device_remove_file(&serial->dev->dev, &dev_attr_vid);
+ device_remove_file(&serial->dev->dev, &dev_attr_pid);
+ device_remove_file(&serial->dev->dev, &dev_attr_productstring);
+ device_remove_file(&serial->dev->dev, &dev_attr_serialnumber);
+ device_remove_file(&serial->dev->dev, &dev_attr_self_powered);
+ device_remove_file(&serial->dev->dev, &dev_attr_max_power);
+ device_remove_file(&serial->dev->dev, &dev_attr_release_version);
+ device_remove_file(&serial->dev->dev, &dev_attr_lock_forever);
}

static int __init cp2101_init (void)
@@ -758,3 +1038,6 @@ MODULE_LICENSE("GPL");

module_param(debug, bool, S_IRUGO | S_IWUSR);
MODULE_PARM_DESC(debug, "Enable verbose debugging messages");
+
+module_param(enable_config, bool, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(enable_config, "Enable sysfs access to configuration eeprom.");


2008-02-29 10:03:26

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] usb: add sysfs configuration interface for CP2101

On Fri, 29 Feb 2008 10:02:18 +0100 Dirk Eibach <[email protected]> wrote:

> From: Dirk Eibach <[email protected]>
>
> The usb configuration data for the Silabs CP2101 usb to uart bridge
> controller can be customized:
> - Vendor ID
> - Product ID
> - Power Descriptor
> - Release Number
> - Serial Number
> - Product Description String
>
> Silabs provides a windows-only tool to do that.
> Since we use linux-only machines in our production-environment, we have no
> proper way to customize the chips for our purpose.
> So I added sysfs configuration support to the linux driver.
>

Thanks.


>From where do our users get the information which they are to write into
these sysfs files?


Please pass your diff through scripts/checkpatch.pl and address the (valid)
errors which it reports.

> +static ssize_t write_vid(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
> +
> + unsigned long vid = simple_strtoul(buf, NULL, 0);

If a user writes "12bucklemyshoe" into one of your sysfs files the kernel
will treat this as 12, which is poor form.

We have a new strict_strtoul() (and related functions) which will perform
proper checking for a valid number. Please use that interface.

Andy, this is going to happen so much that a "should you have used
strict_strtoul?" warning in checkpatch would reduce my email output.


> + if (!vid || vid > 0xffff)
> + return -EINVAL;
> +
> + usb_control_msg(usbdev,
> + usb_sndctrlpipe(usbdev, 0),
> + CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_VID,
> + vid, NULL, 0, 300);

usb_control_msg() can return an error, but this driver doesn't check for
that in several places.

> + return count;
> +}
> +
> +static DEVICE_ATTR(vid, S_IWUGO, NULL, write_vid);
> +
> +static ssize_t write_pid(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
> +
> + unsigned long pid = simple_strtoul(buf, NULL, 0);
> +
> + if (!pid || pid > 0xffff)
> + return -EINVAL;
> +
> + usb_control_msg(usbdev,
> + usb_sndctrlpipe(usbdev, 0),
> + CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_PID,
> + pid, NULL, 0, 300);
> +
> + return count;
> +}

<gets worried>

Oh. Here, "pid" means product ID, not process ID. A little comment over
these functions would be nice ;)

> +static DEVICE_ATTR(pid, S_IWUGO, NULL, write_pid);
> +
> +static ssize_t write_serialnumber(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t count)
> +{
> + int result;
> + unsigned int k;
> + struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
> + u8 serial_stringsize = 2 + 2*count;
> + char serial[serial_stringsize];
> +
> + if (count > 63)
> + return -EINVAL;

uh-oh. serial[] is dynamically allocated on the stack _before_ we've
checked `count'. Methinks we've just give people a way to allocate 8GB of
kernel stack, which won't work terribly well.

I suggest a switch to kmalloc().

> + serial[0] = serial_stringsize;
> + serial[1] = USB_DT_STRING;
> +
> + /* convert to utf-16 */
> + for (k = 0; k < count; ++k) {
> + serial[2+2*k] = buf[k];
> + serial[2+2*k+1] = 0;
> + }

Are you sure this doesn't run off the end of serial[]?

> + result = usb_control_msg(usbdev,
> + usb_sndctrlpipe(usbdev, 0),
> + CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_SERIALNUMBER,
> + 0, serial, sizeof(serial), 300);
> +
> + if (result != serial_stringsize)
> + return -EIO;
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR(serialnumber, S_IWUGO, NULL, write_serialnumber);
> +
> +
> +static ssize_t write_productstring(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t count)
> +{
> + int result;
> + unsigned int k;
> + struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
> + u8 product_stringsize = 2 + 2*count;
> + char product[product_stringsize];
> +
> + if (count > 126)
> + return -EINVAL;
> +
> + product[0] = product_stringsize;
> + product[1] = USB_DT_STRING;
> +
> + /* convert to utf-16 */
> + for (k = 0; k < count; ++k) {
> + product[2+2*k] = buf[k];
> + product[2+2*k+1] = 0;
> + }
> +
> + result = usb_control_msg(usbdev,
> + usb_sndctrlpipe(usbdev, 0),
> + CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE,
> + EEPROM_PRODUCTSTRING, 0,
> + product, sizeof(product), 300);
> +
> + if (result != product_stringsize)
> + return -EIO;
> +
> + return count;
> +}

dittoes.

> +static DEVICE_ATTR(productstring, S_IWUGO, NULL, write_productstring);
> +
> +static ssize_t write_self_powered(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t count)
> +{
> + struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
> +
> + unsigned long self_powered = simple_strtoul(buf, NULL, 0);
> +
> + usb_control_msg(usbdev,
> + usb_sndctrlpipe(usbdev, 0),
> + CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_POWER_ATTRIB,
> + self_powered ? 0x00c0 : 0x0080, NULL, 0, 300);
> +
> + return count;
> +}
>
> ...
>
> +static ssize_t write_lock_forever(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t count)
> +{
> + struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
> +
> + unsigned long lock = simple_strtoul(buf, NULL, 0);
> +
> + /* be really, really sure to know what you are doing here */
> + if (lock != 0xabadbabe)
> + return -EINVAL;
> +
> + usb_control_msg(usbdev,
> + usb_sndctrlpipe(usbdev, 0),
> + CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_LOCK,
> + 0x00f0, NULL, 0, 300);
> +
> + return count;
> +}

hm, interesting. A bit of interface documetation would be nice.

> +static DEVICE_ATTR(lock_forever, S_IWUGO, NULL, write_lock_forever);
> +
> static int cp2101_startup (struct usb_serial *serial)
> {
> + int err;
> +
> /* CP2101 buffers behave strangely unless device is reset */
> usb_reset_device(serial->dev);
> +
> + if (!enable_config)
> + return 0;
> +
> + err = device_create_file(&serial->dev->dev, &dev_attr_reload);
> + if (err) goto out;
> + err = device_create_file(&serial->dev->dev, &dev_attr_vid);
> + if (err) goto out_reload;
> + err = device_create_file(&serial->dev->dev, &dev_attr_pid);
> + if (err) goto out_vid;
> + err = device_create_file(&serial->dev->dev, &dev_attr_productstring);
> + if (err) goto out_pid;
> + err = device_create_file(&serial->dev->dev, &dev_attr_serialnumber);
> + if (err) goto out_productstring;
> + err = device_create_file(&serial->dev->dev, &dev_attr_self_powered);
> + if (err) goto out_serialnumber;
> + err = device_create_file(&serial->dev->dev, &dev_attr_max_power);
> + if (err) goto out_self_powered;
> + err = device_create_file(&serial->dev->dev, &dev_attr_release_version);
> + if (err) goto out_max_power;
> + err = device_create_file(&serial->dev->dev, &dev_attr_lock_forever);
> + if (err) goto out_release_version;
> +
> return 0;
> +
> +out_release_version:
> + device_remove_file(&serial->dev->dev, &dev_attr_release_version);
> +out_max_power:
> + device_remove_file(&serial->dev->dev, &dev_attr_max_power);
> +out_self_powered:
> + device_remove_file(&serial->dev->dev, &dev_attr_self_powered);
> +out_serialnumber:
> + device_remove_file(&serial->dev->dev, &dev_attr_serialnumber);
> +out_productstring:
> + device_remove_file(&serial->dev->dev, &dev_attr_productstring);
> +out_pid:
> + device_remove_file(&serial->dev->dev, &dev_attr_pid);
> +out_vid:
> + device_remove_file(&serial->dev->dev, &dev_attr_vid);
> +out_reload:
> + device_remove_file(&serial->dev->dev, &dev_attr_reload);
> +out:
> + return err;
> }

I think the attribute-group interface will permit you to do this more
elegantly.

> static void cp2101_shutdown (struct usb_serial *serial)
> @@ -721,6 +988,19 @@ static void cp2101_shutdown (struct usb_
> for (i=0; i < serial->num_ports; ++i) {
> cp2101_cleanup(serial->port[i]);
> }
> +
> + if (!enable_config)
> + return;
> +
> + device_remove_file(&serial->dev->dev, &dev_attr_reload);
> + device_remove_file(&serial->dev->dev, &dev_attr_vid);
> + device_remove_file(&serial->dev->dev, &dev_attr_pid);
> + device_remove_file(&serial->dev->dev, &dev_attr_productstring);
> + device_remove_file(&serial->dev->dev, &dev_attr_serialnumber);
> + device_remove_file(&serial->dev->dev, &dev_attr_self_powered);
> + device_remove_file(&serial->dev->dev, &dev_attr_max_power);
> + device_remove_file(&serial->dev->dev, &dev_attr_release_version);
> + device_remove_file(&serial->dev->dev, &dev_attr_lock_forever);
> }
>
> ...
>
> +/*
> + * Silicon Laboratories CP2101/CP2102 USB to RS232 serial adaptor driver
> + *
> + * Copyright (C) 2005 Craig Shelley ([email protected])
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License version
> + * 2 as published by the Free Software Foundation.
> + *
> + * Support to set flow control line levels using TIOCMGET and TIOCMSET
> + * thanks to Karl Hiramoto [email protected]. RTSCTS hardware flow
> + * control thanks to Munir Nassar [email protected]
> + *
> + * Outstanding Issues:
> + * Buffers are not flushed when the port is opened.
> + * Multiple calls to write() may fail with "Resource temporarily unavailable"
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/errno.h>
> +#include <linux/slab.h>
> +#include <linux/tty.h>
> +#include <linux/tty_flip.h>
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/usb.h>
> +#include <asm/uaccess.h>

Please include linux/foo.h in preference to asm/foo.h when linux/foo.h
exists.

checkpatch used to detect this but it got broken.

> +#include <linux/usb/serial.h>
> +
> +/*
> + * Version Information
> + */
> +#define DRIVER_VERSION "v0.07"
> +#define DRIVER_DESC "Silicon Labs CP2101/CP2102 RS232 serial adaptor driver"
> +
> +/*
> + * Function Prototypes
> + */
> +static int cp2101_open(struct usb_serial_port*, struct file*);
> +static void cp2101_cleanup(struct usb_serial_port*);
> +static void cp2101_close(struct usb_serial_port*, struct file*);
> +static void cp2101_get_termios(struct usb_serial_port*);
> +static void cp2101_set_termios(struct usb_serial_port*, struct ktermios*);
> +static int cp2101_tiocmget (struct usb_serial_port *, struct file *);
> +static int cp2101_tiocmset (struct usb_serial_port *, struct file *,
> + unsigned int, unsigned int);
> +static void cp2101_break_ctl(struct usb_serial_port*, int);
> +static int cp2101_startup (struct usb_serial *);
> +static void cp2101_shutdown(struct usb_serial*);
> +
> +
> +static int debug;
> +
> +static struct usb_device_id id_table [] = {

This should have been laid out as "id_table[]". checkpatch misses this.

> + { USB_DEVICE(0x08e6, 0x5501) }, /* Gemalto Prox-PU/CU contactless smartcard reader */
> + { USB_DEVICE(0x0FCF, 0x1003) }, /* Dynastream ANT development board */
> + { USB_DEVICE(0x0FCF, 0x1004) }, /* Dynastream ANT2USB */
> + { USB_DEVICE(0x10A6, 0xAA26) }, /* Knock-off DCU-11 cable */
> + { USB_DEVICE(0x10AB, 0x10C5) }, /* Siemens MC60 Cable */
> + { USB_DEVICE(0x10B5, 0xAC70) }, /* Nokia CA-42 USB */
> + { USB_DEVICE(0x10C4, 0x803B) }, /* Pololu USB-serial converter */
> + { USB_DEVICE(0x10C4, 0x8053) }, /* Enfora EDG1228 */
> + { USB_DEVICE(0x10C4, 0x8066) }, /* Argussoft In-System Programmer */
> + { USB_DEVICE(0x10C4, 0x807A) }, /* Crumb128 board */
> + { USB_DEVICE(0x10C4, 0x80CA) }, /* Degree Controls Inc */
> + { USB_DEVICE(0x10C4, 0x80DD) }, /* Tracient RFID */
> + { USB_DEVICE(0x10C4, 0x80F6) }, /* Suunto sports instrument */
> + { USB_DEVICE(0x10C4, 0x813D) }, /* Burnside Telecom Deskmobile */
> + { USB_DEVICE(0x10C4, 0x814A) }, /* West Mountain Radio RIGblaster P&P */
> + { USB_DEVICE(0x10C4, 0x814B) }, /* West Mountain Radio RIGtalk */
> + { USB_DEVICE(0x10C4, 0x815E) }, /* Helicomm IP-Link 1220-DVM */
> + { USB_DEVICE(0x10C4, 0x81C8) }, /* Lipowsky Industrie Elektronik GmbH, Baby-JTAG */
> + { USB_DEVICE(0x10C4, 0x81E2) }, /* Lipowsky Industrie Elektronik GmbH, Baby-LIN */
> + { USB_DEVICE(0x10C4, 0x81E7) }, /* Aerocomm Radio */
> + { USB_DEVICE(0x10C4, 0x8218) }, /* Lipowsky Industrie Elektronik GmbH, HARP-1 */
> + { USB_DEVICE(0x10C4, 0xEA60) }, /* Silicon Labs factory default */
> + { USB_DEVICE(0x10C4, 0xEA61) }, /* Silicon Labs factory default */
> + { USB_DEVICE(0x10C5, 0xEA61) }, /* Silicon Labs MobiData GPRS USB Modem */
> + { USB_DEVICE(0x13AD, 0x9999) }, /* Baltech card reader */
> + { USB_DEVICE(0x16D6, 0x0001) }, /* Jablotron serial interface */
> + { } /* Terminating Entry */
> +};
> +
>
> ...
>
> +/*
> + * cp2101_get_config
> + * Reads from the CP2101 configuration registers
> + * 'size' is specified in bytes.
> + * 'data' is a pointer to a pre-allocated array of integers large
> + * enough to hold 'size' bytes (with 4 bytes to each integer)
> + */
> +static int cp2101_get_config(struct usb_serial_port* port, u8 request,
> + unsigned int *data, int size)
> +{
> + struct usb_serial *serial = port->serial;
> + __le32 *buf;
> + int result, i, length;
> +
> + /* Number of integers required to contain the array */
> + length = (((size - 1) | 3) + 1)/4;

That looks tricky. Could we just do ROUND_UP(size, 4)?

> + buf = kcalloc(length, sizeof(__le32), GFP_KERNEL);
> + if (!buf) {
> + dev_err(&port->dev, "%s - out of memory.\n", __FUNCTION__);
> + return -ENOMEM;
> + }
> +
> + /* For get requests, the request number must be incremented */
> + request++;
> +
> + /* Issue the request, attempting to read 'size' bytes */
> + result = usb_control_msg (serial->dev,usb_rcvctrlpipe (serial->dev, 0),
> + request, REQTYPE_DEVICE_TO_HOST, 0x0000,
> + 0, buf, size, 300);

Please don't pu a space before the function call operator "(".

checkpatch misses this too - I suspect it got confused and bailed out.


> + /* Convert data into an array of integers */
> + for (i=0; i<length; i++)

spaces around the "=" and the "<" here.

> + data[i] = le32_to_cpu(buf[i]);
> +
> + kfree(buf);
> +
> + if (result != size) {
> + dev_err(&port->dev, "%s - Unable to send config request, "
> + "request=0x%x size=%d result=%d\n",
> + __FUNCTION__, request, size, result);
> + return -EPROTO;
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * cp2101_set_config
> + * Writes to the CP2101 configuration registers
> + * Values less than 16 bits wide are sent directly
> + * 'size' is specified in bytes.
> + */
> +static int cp2101_set_config(struct usb_serial_port* port, u8 request,
> + unsigned int *data, int size)
> +{
> + struct usb_serial *serial = port->serial;
> + __le32 *buf;
> + int result, i, length;
> +
> + /* Number of integers required to contain the array */
> + length = (((size - 1) | 3) + 1)/4;

ROUND_UP?

> + buf = kmalloc(length * sizeof(__le32), GFP_KERNEL);
> + if (!buf) {
> + dev_err(&port->dev, "%s - out of memory.\n",
> + __FUNCTION__);
> + return -ENOMEM;
> + }
> +
> + /* Array of integers into bytes */
> + for (i = 0; i < length; i++)

yes, like that.

> + buf[i] = cpu_to_le32(data[i]);
> +
> + if (size > 2) {
> + result = usb_control_msg (serial->dev,
> + usb_sndctrlpipe(serial->dev, 0),
> + request, REQTYPE_HOST_TO_DEVICE, 0x0000,
> + 0, buf, size, 300);
> + } else {
> + result = usb_control_msg (serial->dev,
> + usb_sndctrlpipe(serial->dev, 0),
> + request, REQTYPE_HOST_TO_DEVICE, data[0],
> + 0, NULL, 0, 300);
> + }
> +
> + kfree(buf);
> +
> + if ((size > 2 && result != size) || result < 0) {
> + dev_err(&port->dev, "%s - Unable to send request, "
> + "request=0x%x size=%d result=%d\n",
> + __FUNCTION__, request, size, result);
> + return -EPROTO;
> + }
> +
> + /* Single data value */
> + result = usb_control_msg (serial->dev,
> + usb_sndctrlpipe(serial->dev, 0),
> + request, REQTYPE_HOST_TO_DEVICE, data[0],
> + 0, NULL, 0, 300);
> + return 0;
> +}
>
> ...
>
> +static int cp2101_open (struct usb_serial_port *port, struct file *filp)

yes, checkpatch has failed us here.

> +{
> + struct usb_serial *serial = port->serial;
> + int result;
> +
> + dbg("%s - port %d", __FUNCTION__, port->number);
> +
> + if (cp2101_set_config_single(port, CP2101_UART, UART_ENABLE)) {
> + dev_err(&port->dev, "%s - Unable to enable UART\n",
> + __FUNCTION__);
> + return -EPROTO;
> + }
> +
> + /* Start reading from the device */
> + usb_fill_bulk_urb (port->read_urb, serial->dev,
> + usb_rcvbulkpipe(serial->dev,
> + port->bulk_in_endpointAddress),
> + port->read_urb->transfer_buffer,
> + port->read_urb->transfer_buffer_length,
> + serial->type->read_bulk_callback,
> + port);
> + result = usb_submit_urb(port->read_urb, GFP_KERNEL);
> + if (result) {
> + dev_err(&port->dev, "%s - failed resubmitting read urb, "
> + "error %d\n", __FUNCTION__, result);
> + return result;
> + }
> +
> + /* Configure the termios structure */
> + cp2101_get_termios(port);
> +
> + /* Set the DTR and RTS pins low */
> + cp2101_tiocmset(port, NULL, TIOCM_DTR | TIOCM_RTS, 0);
> +
> + return 0;
> +}
> +
> +/*
> + * cp2101_get_termios
> + * Reads the baud rate, data bits, parity, stop bits and flow control mode
> + * from the device, corrects any unsupported values, and configures the
> + * termios structure to reflect the state of the device
> + */
> +static void cp2101_get_termios (struct usb_serial_port *port)

Please cc Alan Cox <[email protected]> on the next version - he
might review the termios stuff for us.

> +{
> + unsigned int cflag, modem_ctl[4];
> + int baud;
> + int bits;
> +
> + dbg("%s - port %d", __FUNCTION__, port->number);
> +
> + if (!port->tty || !port->tty->termios) {

can this happen?

> + dbg("%s - no tty structures", __FUNCTION__);
> + return;
> + }
> +
> + cp2101_get_config(port, CP2101_BAUDRATE, &baud, 2);
> + /* Convert to baudrate */
> + if (baud)
> + baud = BAUD_RATE_GEN_FREQ / baud;
> +
> + dbg("%s - baud rate = %d", __FUNCTION__, baud);
> +
> + tty_encode_baud_rate(port->tty, baud, baud);
> + cflag = port->tty->termios->c_cflag;
> +
> + cp2101_get_config(port, CP2101_BITS, &bits, 2);
> + cflag &= ~CSIZE;
> + switch(bits & BITS_DATA_MASK) {
> + case BITS_DATA_5:
> + dbg("%s - data bits = 5", __FUNCTION__);
> + cflag |= CS5;
> + break;
> + case BITS_DATA_6:
> + dbg("%s - data bits = 6", __FUNCTION__);
> + cflag |= CS6;
> + break;
> + case BITS_DATA_7:
> + dbg("%s - data bits = 7", __FUNCTION__);
> + cflag |= CS7;
> + break;
> + case BITS_DATA_8:
> + dbg("%s - data bits = 8", __FUNCTION__);
> + cflag |= CS8;
> + break;
> + case BITS_DATA_9:
> + dbg("%s - data bits = 9 (not supported, "
> + "using 8 data bits)", __FUNCTION__);
> + cflag |= CS8;
> + bits &= ~BITS_DATA_MASK;
> + bits |= BITS_DATA_8;
> + cp2101_set_config(port, CP2101_BITS, &bits, 2);
> + break;
> + default:
> + dbg("%s - Unknown number of data bits, "
> + "using 8", __FUNCTION__);
> + cflag |= CS8;
> + bits &= ~BITS_DATA_MASK;
> + bits |= BITS_DATA_8;
> + cp2101_set_config(port, CP2101_BITS, &bits, 2);
> + break;
> + }

We normally indent the switch body one tabsyop less than this.

I'll stop emulating checkpatch now - let's work out why it failed, get that
fixed and then please run it.

2008-02-29 12:31:34

by Eibach, Dirk

[permalink] [raw]
Subject: Re: [PATCH] usb: add sysfs configuration interface for CP2101

So here is a new try.

---
diff -purN linux-2.6.24/drivers/usb/serial/cp2101.c linux-2.6.24-diff/drivers/usb/serial/cp2101.c
--- linux-2.6.24/drivers/usb/serial/cp2101.c 2008-01-24 23:58:37.000000000 +0100
+++ linux-2.6.24-diff/drivers/usb/serial/cp2101.c 2008-02-29 13:15:58.168233146 +0100
@@ -18,6 +18,8 @@
*/

#include <linux/kernel.h>
+#include <linux/fs.h>
+#include <linux/platform_device.h>
#include <linux/errno.h>
#include <linux/slab.h>
#include <linux/tty.h>
@@ -52,6 +54,8 @@ static void cp2101_shutdown(struct usb_s

static int debug;

+static int enable_config = false;
+
static struct usb_device_id id_table [] = {
{ USB_DEVICE(0x08e6, 0x5501) }, /* Gemalto Prox-PU/CU contactless smartcard reader */
{ USB_DEVICE(0x0FCF, 0x1003) }, /* Dynastream ANT development board */
@@ -125,6 +129,7 @@ static struct usb_serial_driver cp2101_d
#define CP2101_CONTROL 0x07 /* Flow control line states */
#define CP2101_MODEMCTL 0x13 /* Modem controls */
#define CP2101_CONFIG_6 0x19 /* 6 bytes of config data ??? */
+#define CP2101_EEPROM 0xFF /* write configuration eeprom */

/* CP2101_UART */
#define UART_ENABLE 0x0001
@@ -167,6 +172,20 @@ static struct usb_serial_driver cp2101_d
#define CONTROL_WRITE_DTR 0x0100
#define CONTROL_WRITE_RTS 0x0200

+/* CP2101 CONFIGURATION EEPROM */
+#define EEPROM_RELOAD 0x0008
+#define EEPROM_VENDOR_ID 0x3701
+#define EEPROM_PRODUCT_ID 0x3702
+#define EEPROM_PRODUCTSTRING 0x3703
+#define EEPROM_SERIALNUMBER 0x3704
+#define EEPROM_POWER_ATTRIB 0x3705
+#define EEPROM_MAX_POWER 0x3706
+#define EEPROM_RELEASE_VERSION 0x3707
+#define EEPROM_LOCK 0x370a
+
+#define SERIALNUMBER_MAX_CHARS 63
+#define PRODUCTSTRING_MAX_CHARS 126
+
/*
* cp2101_get_config
* Reads from the CP2101 configuration registers
@@ -704,11 +723,286 @@ static void cp2101_break_ctl (struct usb
cp2101_set_config(port, CP2101_BREAK, &state, 2);
}

+/*
+ * When loaded with module parameter "enable_config=1" the driver offers the
+ * following sysfs attributes to customize USB configuration data:
+ * reload: write "1" to reboot CP210x and activate the new configuration
+ * vendor_id: write 16 bit value to set
+ * product_id: write 16 bit value to set
+ * productstring: write up to 126 characters to set
+ * serialnumber: write up to 63 characters to set
+ * self_powered: write "1" to set, "0" to unset
+ * max_power: write 8 bit value (unit 2 mA) to set
+ * release_version: write 16 bit BCD-value to set (0x1234 => version 12.34)
+ * lock_forever: write 0xabadbabe to lock the settings permanently
+ */
+
+static ssize_t write_reload(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int result;
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+
+ /* writing "0" does not trigger */
+ if (!strict_strtoul(buf, NULL, 0))
+ return count;
+
+ result = usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_RELOAD,
+ 0, NULL, 0, 300);
+
+ /* this request is expected to fail because cp210x reboots */
+
+ return count;
+}
+
+static DEVICE_ATTR(reload, S_IWUGO, NULL, write_reload);
+
+static ssize_t write_vendor_id(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int result;
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+
+ unsigned long vendor_id = strict_strtoul(buf, NULL, 0);
+
+ if (!vendor_id || vendor_id > 0xffff)
+ return -EINVAL;
+
+ result = usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_VENDOR_ID,
+ vendor_id, NULL, 0, 300);
+
+ if (result)
+ return -EIO;
+
+ return count;
+}
+
+static DEVICE_ATTR(vendor_id, S_IWUGO, NULL, write_vendor_id);
+
+static ssize_t write_product_id(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int result;
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+
+ unsigned long product_id = strict_strtoul(buf, NULL, 0);
+
+ if (!product_id || product_id > 0xffff)
+ return -EINVAL;
+
+ result = usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_PRODUCT_ID,
+ product_id, NULL, 0, 300);
+
+ if (result)
+ return -EIO;
+
+ return count;
+}
+
+static DEVICE_ATTR(product_id, S_IWUGO, NULL, write_product_id);
+
+static ssize_t write_serialnumber(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ int result;
+ unsigned int k;
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+ u8 serial_stringsize = 2 + 2*count;
+ char serial[2 + 2*SERIALNUMBER_MAX_CHARS];
+
+ if (count > SERIALNUMBER_MAX_CHARS)
+ return -EINVAL;
+
+ serial[0] = serial_stringsize;
+ serial[1] = USB_DT_STRING;
+
+ /* convert to utf-16 */
+ printk("#Max index: %d\n", serial_stringsize - 1);
+ for (k = 0; k < count; ++k) {
+ printk("Address index %d and %d\n", 2+2*k, 2+2*k+1);
+ serial[2+2*k] = buf[k];
+ serial[2+2*k+1] = 0;
+ }
+
+ result = usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_SERIALNUMBER,
+ 0, serial, serial_stringsize, 300);
+
+ if (result != serial_stringsize)
+ return -EIO;
+
+ return count;
+}
+
+static DEVICE_ATTR(serialnumber, S_IWUGO, NULL, write_serialnumber);
+
+
+static ssize_t write_productstring(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ int result;
+ unsigned int k;
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+ u8 product_stringsize = 2 + 2*count;
+ char product[2 + 2*PRODUCTSTRING_MAX_CHARS];
+
+ if (count > PRODUCTSTRING_MAX_CHARS)
+ return -EINVAL;
+
+ product[0] = product_stringsize;
+ product[1] = USB_DT_STRING;
+
+ /* convert to utf-16 */
+ for (k = 0; k < count; ++k) {
+ product[2+2*k] = buf[k];
+ product[2+2*k+1] = 0;
+ }
+
+ result = usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE,
+ EEPROM_PRODUCTSTRING, 0,
+ product, product_stringsize, 300);
+
+ if (result != product_stringsize)
+ return -EIO;
+
+ return count;
+}
+
+static DEVICE_ATTR(productstring, S_IWUGO, NULL, write_productstring);
+
+static ssize_t write_self_powered(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ int result;
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+
+ unsigned long self_powered = strict_strtoul(buf, NULL, 0);
+
+ result = usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_POWER_ATTRIB,
+ self_powered ? 0x00c0 : 0x0080, NULL, 0, 300);
+
+ if (result)
+ return -EIO;
+
+ return count;
+}
+
+static DEVICE_ATTR(self_powered, S_IWUGO, NULL, write_self_powered);
+
+static ssize_t write_max_power(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ int result;
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+
+ unsigned long max_power = strict_strtoul(buf, NULL, 0);
+
+ if (!max_power || max_power > 0xff)
+ return -EINVAL;
+
+ result = usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_MAX_POWER,
+ max_power, NULL, 0, 300);
+
+ if (result)
+ return -EIO;
+
+ return count;
+}
+
+static DEVICE_ATTR(max_power, S_IWUGO, NULL, write_max_power);
+
+static ssize_t write_release_version(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ int result;
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+
+ unsigned long release_version = strict_strtoul(buf, NULL, 0);
+
+ if (release_version > 0xffff)
+ return -EINVAL;
+
+ result = usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_RELEASE_VERSION,
+ release_version, NULL, 0, 300);
+
+ if (result)
+ return -EIO;
+
+ return count;
+}
+
+static DEVICE_ATTR(release_version, S_IWUGO, NULL, write_release_version);
+
+static ssize_t write_lock_forever(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ int result;
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+
+ unsigned long lock = strict_strtoul(buf, NULL, 0);
+
+ /* be really, really sure to know what you are doing here */
+ if (lock != 0xabadbabe)
+ return -EINVAL;
+
+ result = usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_LOCK,
+ 0x00f0, NULL, 0, 300);
+
+ if (result)
+ return -EIO;
+
+ return count;
+}
+
+static DEVICE_ATTR(lock_forever, S_IWUGO, NULL, write_lock_forever);
+
+static struct attribute *cp2101_attributes[] = {
+ &dev_attr_reload.attr,
+ &dev_attr_vendor_id.attr,
+ &dev_attr_product_id.attr,
+ &dev_attr_productstring.attr,
+ &dev_attr_serialnumber.attr,
+ &dev_attr_self_powered.attr,
+ &dev_attr_max_power.attr,
+ &dev_attr_release_version.attr,
+ &dev_attr_lock_forever.attr,
+ NULL
+};
+
+static const struct attribute_group cp2101_group = {
+ .attrs = cp2101_attributes,
+};
+
static int cp2101_startup (struct usb_serial *serial)
{
+ int err;
+
/* CP2101 buffers behave strangely unless device is reset */
usb_reset_device(serial->dev);
- return 0;
+
+ if (!enable_config)
+ return 0;
+
+ err = sysfs_create_group(&serial->dev->dev.kobj, &cp2101_group);
+
+ return err;
}

static void cp2101_shutdown (struct usb_serial *serial)
@@ -721,6 +1015,11 @@ static void cp2101_shutdown (struct usb_
for (i=0; i < serial->num_ports; ++i) {
cp2101_cleanup(serial->port[i]);
}
+
+ if (!enable_config)
+ return;
+
+ sysfs_remove_group(&serial->dev->dev.kobj, &cp2101_group);
}

static int __init cp2101_init (void)
@@ -758,3 +1057,6 @@ MODULE_LICENSE("GPL");

module_param(debug, bool, S_IRUGO | S_IWUSR);
MODULE_PARM_DESC(debug, "Enable verbose debugging messages");
+
+module_param(enable_config, bool, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(enable_config, "Enable sysfs access to configuration eeprom.");


2008-02-29 14:43:37

by Andy Whitcroft

[permalink] [raw]
Subject: Re: [PATCH] usb: add sysfs configuration interface for CP2101

On Fri, Feb 29, 2008 at 02:02:45AM -0800, Andrew Morton wrote:
> On Fri, 29 Feb 2008 10:02:18 +0100 Dirk Eibach <[email protected]> wrote:

> We have a new strict_strtoul() (and related functions) which will perform
> proper checking for a valid number. Please use that interface.
>
> Andy, this is going to happen so much that a "should you have used
> strict_strtoul?" warning in checkpatch would reduce my email output.

Sure, will add something. I wonder if this new interface is documented.

I note that this interface is new in -mm at this time. So that brings
up an interesting question as to how one would integrate this check with
checkpatch. As checking patches for mainline, this would be an
incorrect check until that patch merges. I guess the right thing to do
is provide a separate patch for checkpatch which adds this check which
should sit with the patch in your tree which adds the functionality.

Make sense?

-apw

2008-02-29 20:36:06

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] usb: add sysfs configuration interface for CP2101

On Fri, 29 Feb 2008 13:31:11 +0100
Dirk Eibach <[email protected]> wrote:

> So here is a new try.
>

Please reissue the possibly-updated changelog and signed-off-by: with each
revision of a patch.

Please cc linux-usb on usb patches.

> diff -purN linux-2.6.24/drivers/usb/serial/cp2101.c linux-2.6.24-diff/drivers/usb/serial/cp2101.c
> --- linux-2.6.24/drivers/usb/serial/cp2101.c 2008-01-24 23:58:37.000000000 +0100
> +++ linux-2.6.24-diff/drivers/usb/serial/cp2101.c 2008-02-29 13:15:58.168233146 +0100
> @@ -18,6 +18,8 @@
> */
>
> #include <linux/kernel.h>
> +#include <linux/fs.h>
> +#include <linux/platform_device.h>
> #include <linux/errno.h>
> #include <linux/slab.h>
> #include <linux/tty.h>
> @@ -52,6 +54,8 @@ static void cp2101_shutdown(struct usb_s
>
> static int debug;
>
> +static int enable_config = false;

Should be either

static int enable_config;

or, if you're being very formal,

static bool enable_config = false;

or, if you're being less formal and want to avoid bloating the kernel image,

static bool enable_config;

> +static ssize_t write_reload(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + int result;
> + struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
> +
> + /* writing "0" does not trigger */
> + if (!strict_strtoul(buf, NULL, 0))
> + return count;
> +
> + result = usb_control_msg(usbdev,
> + usb_sndctrlpipe(usbdev, 0),
> + CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_RELOAD,
> + 0, NULL, 0, 300);
> +
> + /* this request is expected to fail because cp210x reboots */
> +
> + return count;
> +}

The comment will suffice - there's no need to add the do-nothing `result'.

> +static DEVICE_ATTR(reload, S_IWUGO, NULL, write_reload);
> +
> +static ssize_t write_vendor_id(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + int result;
> + struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
> +
> + unsigned long vendor_id = strict_strtoul(buf, NULL, 0);
> +
> + if (!vendor_id || vendor_id > 0xffff)
> + return -EINVAL;
> +
> + result = usb_control_msg(usbdev,
> + usb_sndctrlpipe(usbdev, 0),
> + CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_VENDOR_ID,
> + vendor_id, NULL, 0, 300);
> +
> + if (result)
> + return -EIO;
> +
> + return count;
> +}

usb_control_msg() already returns an errno - we should just propagate that
back to the caller rather than overwriting, say, -ENOMEM with -EIO.

> +static DEVICE_ATTR(vendor_id, S_IWUGO, NULL, write_vendor_id);
> +
> +static ssize_t write_product_id(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + int result;
> + struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
> +
> + unsigned long product_id = strict_strtoul(buf, NULL, 0);
> +
> + if (!product_id || product_id > 0xffff)
> + return -EINVAL;
> +
> + result = usb_control_msg(usbdev,
> + usb_sndctrlpipe(usbdev, 0),
> + CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_PRODUCT_ID,
> + product_id, NULL, 0, 300);
> +
> + if (result)
> + return -EIO;
> +
> + return count;
> +}

ditto

> +static DEVICE_ATTR(product_id, S_IWUGO, NULL, write_product_id);
> +
> +static ssize_t write_serialnumber(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t count)
> +{
> + int result;
> + unsigned int k;
> + struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
> + u8 serial_stringsize = 2 + 2*count;
> + char serial[2 + 2*SERIALNUMBER_MAX_CHARS];

128 bytes of stack is a little more than we'd like (the kernel is really
squeezy on stack space sometimes). kmalloc would be better...

> + if (count > SERIALNUMBER_MAX_CHARS)
> + return -EINVAL;
> +
> + serial[0] = serial_stringsize;
> + serial[1] = USB_DT_STRING;
> +
> + /* convert to utf-16 */
> + printk("#Max index: %d\n", serial_stringsize - 1);

checkpatch...

> + for (k = 0; k < count; ++k) {
> + printk("Address index %d and %d\n", 2+2*k, 2+2*k+1);
> + serial[2+2*k] = buf[k];
> + serial[2+2*k+1] = 0;
> + }
> +
> + result = usb_control_msg(usbdev,
> + usb_sndctrlpipe(usbdev, 0),
> + CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_SERIALNUMBER,
> + 0, serial, serial_stringsize, 300);
> +
> + if (result != serial_stringsize)
> + return -EIO;
> +
> + return count;
> +}
>
> ...
>
> +static ssize_t write_lock_forever(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t count)
> +{
> + int result;
> + struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
> +
> + unsigned long lock = strict_strtoul(buf, NULL, 0);
> +
> + /* be really, really sure to know what you are doing here */
> + if (lock != 0xabadbabe)
> + return -EINVAL;

this is incomprehehsible without docs

> + result = usb_control_msg(usbdev,
> + usb_sndctrlpipe(usbdev, 0),
> + CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_LOCK,
> + 0x00f0, NULL, 0, 300);
> +
> + if (result)
> + return -EIO;
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR(lock_forever, S_IWUGO, NULL, write_lock_forever);
> +
> +static struct attribute *cp2101_attributes[] = {
> + &dev_attr_reload.attr,
> + &dev_attr_vendor_id.attr,
> + &dev_attr_product_id.attr,
> + &dev_attr_productstring.attr,
> + &dev_attr_serialnumber.attr,
> + &dev_attr_self_powered.attr,
> + &dev_attr_max_power.attr,
> + &dev_attr_release_version.attr,
> + &dev_attr_lock_forever.attr,
> + NULL
> +};
> +
> +static const struct attribute_group cp2101_group = {
> + .attrs = cp2101_attributes,
> +};
> +
> static int cp2101_startup (struct usb_serial *serial)
> {
> + int err;
> +
> /* CP2101 buffers behave strangely unless device is reset */
> usb_reset_device(serial->dev);
> - return 0;
> +
> + if (!enable_config)
> + return 0;
> +
> + err = sysfs_create_group(&serial->dev->dev.kobj, &cp2101_group);
> +
> + return err;
> }
>
> static void cp2101_shutdown (struct usb_serial *serial)
> @@ -721,6 +1015,11 @@ static void cp2101_shutdown (struct usb_
> for (i=0; i < serial->num_ports; ++i) {
> cp2101_cleanup(serial->port[i]);
> }
> +
> + if (!enable_config)
> + return;
> +
> + sysfs_remove_group(&serial->dev->dev.kobj, &cp2101_group);
> }
>
> static int __init cp2101_init (void)
> @@ -758,3 +1057,6 @@ MODULE_LICENSE("GPL");
>
> module_param(debug, bool, S_IRUGO | S_IWUSR);
> MODULE_PARM_DESC(debug, "Enable verbose debugging messages");
> +
> +module_param(enable_config, bool, S_IRUGO | S_IWUSR);
> +MODULE_PARM_DESC(enable_config, "Enable sysfs access to configuration eeprom.");
>

2008-02-29 20:46:55

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] usb: add sysfs configuration interface for CP2101

On Fri, 29 Feb 2008 14:44:37 +0000
Andy Whitcroft <[email protected]> wrote:

> On Fri, Feb 29, 2008 at 02:02:45AM -0800, Andrew Morton wrote:
> > On Fri, 29 Feb 2008 10:02:18 +0100 Dirk Eibach <[email protected]> wrote:
>
> > We have a new strict_strtoul() (and related functions) which will perform
> > proper checking for a valid number. Please use that interface.
> >
> > Andy, this is going to happen so much that a "should you have used
> > strict_strtoul?" warning in checkpatch would reduce my email output.
>
> Sure, will add something. I wonder if this new interface is documented.

It has kerneldoc.

> I note that this interface is new in -mm at this time.

It is in mainline.

> So that brings
> up an interesting question as to how one would integrate this check with
> checkpatch. As checking patches for mainline, this would be an
> incorrect check until that patch merges. I guess the right thing to do
> is provide a separate patch for checkpatch which adds this check which
> should sit with the patch in your tree which adds the functionality.
>

It would be easier to just update your kernel tree ;)

2008-03-01 01:19:57

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] usb: add sysfs configuration interface for CP2101

On Fri, Feb 29, 2008 at 01:31:11PM +0100, Dirk Eibach wrote:
> So here is a new try.

Please provide a full changelog entry, and signed-off-by: if you wish to
have it applied to the tree.

As you are adding new sysfs entries, I need some documentation to be
added in Documentation/ABI/ describing these new files and how they are
to be used. See the instructions there for the format needed.


>
> ---
> diff -purN linux-2.6.24/drivers/usb/serial/cp2101.c linux-2.6.24-diff/drivers/usb/serial/cp2101.c
> --- linux-2.6.24/drivers/usb/serial/cp2101.c 2008-01-24 23:58:37.000000000 +0100
> +++ linux-2.6.24-diff/drivers/usb/serial/cp2101.c 2008-02-29 13:15:58.168233146 +0100
> @@ -18,6 +18,8 @@
> */
>
> #include <linux/kernel.h>
> +#include <linux/fs.h>
> +#include <linux/platform_device.h>
> #include <linux/errno.h>
> #include <linux/slab.h>
> #include <linux/tty.h>
> @@ -52,6 +54,8 @@ static void cp2101_shutdown(struct usb_s
>
> static int debug;
>
> +static int enable_config = false;
> +
> static struct usb_device_id id_table [] = {
> { USB_DEVICE(0x08e6, 0x5501) }, /* Gemalto Prox-PU/CU contactless smartcard reader */
> { USB_DEVICE(0x0FCF, 0x1003) }, /* Dynastream ANT development board */
> @@ -125,6 +129,7 @@ static struct usb_serial_driver cp2101_d
> #define CP2101_CONTROL 0x07 /* Flow control line states */
> #define CP2101_MODEMCTL 0x13 /* Modem controls */
> #define CP2101_CONFIG_6 0x19 /* 6 bytes of config data ??? */
> +#define CP2101_EEPROM 0xFF /* write configuration eeprom */
>
> /* CP2101_UART */
> #define UART_ENABLE 0x0001
> @@ -167,6 +172,20 @@ static struct usb_serial_driver cp2101_d
> #define CONTROL_WRITE_DTR 0x0100
> #define CONTROL_WRITE_RTS 0x0200
>
> +/* CP2101 CONFIGURATION EEPROM */
> +#define EEPROM_RELOAD 0x0008
> +#define EEPROM_VENDOR_ID 0x3701
> +#define EEPROM_PRODUCT_ID 0x3702
> +#define EEPROM_PRODUCTSTRING 0x3703
> +#define EEPROM_SERIALNUMBER 0x3704
> +#define EEPROM_POWER_ATTRIB 0x3705
> +#define EEPROM_MAX_POWER 0x3706
> +#define EEPROM_RELEASE_VERSION 0x3707
> +#define EEPROM_LOCK 0x370a
> +
> +#define SERIALNUMBER_MAX_CHARS 63
> +#define PRODUCTSTRING_MAX_CHARS 126
> +
> /*
> * cp2101_get_config
> * Reads from the CP2101 configuration registers
> @@ -704,11 +723,286 @@ static void cp2101_break_ctl (struct usb
> cp2101_set_config(port, CP2101_BREAK, &state, 2);
> }
>
> +/*
> + * When loaded with module parameter "enable_config=1" the driver offers the
> + * following sysfs attributes to customize USB configuration data:
> + * reload: write "1" to reboot CP210x and activate the new configuration
> + * vendor_id: write 16 bit value to set
> + * product_id: write 16 bit value to set


This seems odd, you know we already allow any usb vendor and product id
to be automatically added to a driver after it has been loaded through
sysfs, right? Wouldn't that take care of the majority of the issues you
have here?

Oh, wait, this is setting the device itself, nevermind.

You should use configfs for this instead, that's what it is for moreso
than sysfs, right?

thanks,

greg k-h

2008-03-03 09:27:32

by Eibach, Dirk

[permalink] [raw]
Subject: Re: [PATCH] usb: add sysfs configuration interface for CP2101

From: Dirk Eibach <[email protected]>

The usb configuration data for the Silabs CP2101 usb to uart bridge
controller can be customized:
- Vendor ID
- Product ID
- Power Descriptor
- Release Number
- Serial Number
- Product Description String

Silabs provides a windows-only tool to do that.
Since we use linux-only machines in our production-environment, we have no
proper way to customize the chips for our purpose.
So I added sysfs configuration support to the linux driver.

Signed-off-by: Dirk Eibach <[email protected]>
---
diff -purN linux-2.6.24/Documentation/ABI/testing/sysfs-bus-usb-devices-cp2101 linux-2.6.24-diff/Documentation/ABI/testing/sysfs-bus-usb-devices-cp2101
--- linux-2.6.24/Documentation/ABI/testing/sysfs-bus-usb-devices-cp2101 1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.24-diff/Documentation/ABI/testing/sysfs-bus-usb-devices-cp2101 2008-03-03 09:29:21.883042194 +0100
@@ -0,0 +1,44 @@
+What: cp2101 sysfs device customization-interface
+Date: 03.03.2008
+KernelVersion: 2.6.26
+Contact: Dirk Eibach <[email protected]>
+Description:
+
+The usb configuration data for the Silabs CP2101 usb to uart bridge
+controller are stored in an on-chip eeprom and can be customized:
+- Vendor ID
+- Product ID
+- Power Descriptor
+- Release Number
+- Serial Number
+- Product Description String
+
+When loaded with module parameter "enable_config=1" the cp2101 driver offers
+the following sysfs attributes to customize USB configuration data:
+
+reload
+ write "1" to reboot CP210x and activate the new configuration
+
+vendor_id
+ write 16 bit value to set
+
+product_id
+ write 16 bit value to set
+
+productstring
+ write up to 126 characters to set
+
+serialnumber
+ write up to 63 characters to set
+
+self_powered
+ write "1" to set, "0" to unset
+
+max_power
+ write 8 bit value (unit 2 mA) to set
+
+release_version
+ write 16 bit BCD-value to set (0x1234 => version 12.34)
+
+lock_forever
+ write "1" to lock the settings permanently (*no way back*)
diff -purN linux-2.6.24/drivers/usb/serial/cp2101.c linux-2.6.24-diff/drivers/usb/serial/cp2101.c
--- linux-2.6.24/drivers/usb/serial/cp2101.c 2008-01-24 23:58:37.000000000 +0100
+++ linux-2.6.24-diff/drivers/usb/serial/cp2101.c 2008-03-03 10:24:16.905807660 +0100
@@ -18,6 +18,8 @@
*/

#include <linux/kernel.h>
+#include <linux/fs.h>
+#include <linux/platform_device.h>
#include <linux/errno.h>
#include <linux/slab.h>
#include <linux/tty.h>
@@ -52,6 +54,8 @@ static void cp2101_shutdown(struct usb_s

static int debug;

+static int enable_config;
+
static struct usb_device_id id_table [] = {
{ USB_DEVICE(0x08e6, 0x5501) }, /* Gemalto Prox-PU/CU contactless smartcard reader */
{ USB_DEVICE(0x0FCF, 0x1003) }, /* Dynastream ANT development board */
@@ -125,6 +129,7 @@ static struct usb_serial_driver cp2101_d
#define CP2101_CONTROL 0x07 /* Flow control line states */
#define CP2101_MODEMCTL 0x13 /* Modem controls */
#define CP2101_CONFIG_6 0x19 /* 6 bytes of config data ??? */
+#define CP2101_EEPROM 0xFF /* write configuration eeprom */

/* CP2101_UART */
#define UART_ENABLE 0x0001
@@ -167,6 +172,20 @@ static struct usb_serial_driver cp2101_d
#define CONTROL_WRITE_DTR 0x0100
#define CONTROL_WRITE_RTS 0x0200

+/* CP2101 CONFIGURATION EEPROM */
+#define EEPROM_RELOAD 0x0008
+#define EEPROM_VENDOR_ID 0x3701
+#define EEPROM_PRODUCT_ID 0x3702
+#define EEPROM_PRODUCTSTRING 0x3703
+#define EEPROM_SERIALNUMBER 0x3704
+#define EEPROM_POWER_ATTRIB 0x3705
+#define EEPROM_MAX_POWER 0x3706
+#define EEPROM_RELEASE_VERSION 0x3707
+#define EEPROM_LOCK 0x370a
+
+#define SERIALNUMBER_MAX_CHARS 63
+#define PRODUCTSTRING_MAX_CHARS 126
+
/*
* cp2101_get_config
* Reads from the CP2101 configuration registers
@@ -704,11 +723,309 @@ static void cp2101_break_ctl (struct usb
cp2101_set_config(port, CP2101_BREAK, &state, 2);
}

+/*
+ * When loaded with module parameter "enable_config=1" the driver offers
+ * sysfs attributes to customize USB configuration data as described in
+ * Documentation/ABI/testing/sysfs-bus-usb-devices-cp2101.
+ */
+
+static ssize_t write_reload(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+
+ unsigned long reload;
+
+ if (strict_strtoul(buf, 0, &reload))
+ return -EINVAL;
+
+ /* writing "0" does not trigger */
+ if (!reload)
+ return count;
+
+ usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_RELOAD,
+ 0, NULL, 0, 300);
+
+ /* this request is expected to fail because cp210x reboots */
+
+ return count;
+}
+
+static DEVICE_ATTR(reload, S_IWUGO, NULL, write_reload);
+
+static ssize_t write_vendor_id(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ int result;
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+
+ unsigned long vendor_id;
+
+ if (strict_strtoul(buf, 0, &vendor_id))
+ return -EINVAL;
+
+ if (!vendor_id || vendor_id > 0xffff)
+ return -EINVAL;
+
+ result = usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_VENDOR_ID,
+ vendor_id, NULL, 0, 300);
+
+ if (result)
+ return result;
+
+ return count;
+}
+
+static DEVICE_ATTR(vendor_id, S_IWUGO, NULL, write_vendor_id);
+
+static ssize_t write_product_id(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ int result;
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+
+ unsigned long product_id;
+
+ if (strict_strtoul(buf, 0, &product_id))
+ return -EINVAL;
+
+ if (!product_id || product_id > 0xffff)
+ return -EINVAL;
+
+ result = usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_PRODUCT_ID,
+ product_id, NULL, 0, 300);
+
+ if (result)
+ return result;
+
+ return count;
+}
+
+static DEVICE_ATTR(product_id, S_IWUGO, NULL, write_product_id);
+
+static ssize_t write_serialnumber(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ int result;
+ unsigned int k;
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+ u8 serial_stringsize = 2 + 2*count;
+ char *serial;
+
+ if (count > SERIALNUMBER_MAX_CHARS)
+ return -EINVAL;
+
+ serial = kmalloc(serial_stringsize, GFP_KERNEL);
+ if (!serial)
+ return -ENOMEM;
+
+ serial[0] = serial_stringsize;
+ serial[1] = USB_DT_STRING;
+
+ /* convert to utf-16 */
+ for (k = 0; k < count; ++k) {
+ serial[2+2*k] = buf[k];
+ serial[2+2*k+1] = 0;
+ }
+
+ result = usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_SERIALNUMBER,
+ 0, serial, serial_stringsize, 300);
+
+ kfree(serial);
+
+ if (result != serial_stringsize)
+ return result;
+
+ return count;
+}
+
+static DEVICE_ATTR(serialnumber, S_IWUGO, NULL, write_serialnumber);
+
+
+static ssize_t write_productstring(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ int result;
+ unsigned int k;
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+ u8 product_stringsize = 2 + 2*count;
+ char *product;
+
+ if (count > PRODUCTSTRING_MAX_CHARS)
+ return -EINVAL;
+
+ product = kmalloc(product_stringsize, GFP_KERNEL);
+ if (!product)
+ return -ENOMEM;
+
+ product[0] = product_stringsize;
+ product[1] = USB_DT_STRING;
+
+ /* convert to utf-16 */
+ for (k = 0; k < count; ++k) {
+ product[2+2*k] = buf[k];
+ product[2+2*k+1] = 0;
+ }
+
+ result = usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE,
+ EEPROM_PRODUCTSTRING, 0,
+ product, product_stringsize, 300);
+
+ kfree(product);
+
+ if (result != product_stringsize)
+ return result;
+
+ return count;
+}
+
+static DEVICE_ATTR(productstring, S_IWUGO, NULL, write_productstring);
+
+static ssize_t write_self_powered(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ int result;
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+
+ unsigned long self_powered;
+
+ if (strict_strtoul(buf, 0, &self_powered))
+ return -EINVAL;
+
+ result = usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_POWER_ATTRIB,
+ self_powered ? 0x00c0 : 0x0080, NULL, 0, 300);
+
+ if (result)
+ return result;
+
+ return count;
+}
+
+static DEVICE_ATTR(self_powered, S_IWUGO, NULL, write_self_powered);
+
+static ssize_t write_max_power(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ int result;
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+
+ unsigned long max_power;
+
+ if (strict_strtoul(buf, 0, &max_power))
+ return -EINVAL;
+
+ if (!max_power || max_power > 0xff)
+ return -EINVAL;
+
+ result = usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_MAX_POWER,
+ max_power, NULL, 0, 300);
+
+ if (result)
+ return result;
+
+ return count;
+}
+
+static DEVICE_ATTR(max_power, S_IWUGO, NULL, write_max_power);
+
+static ssize_t write_release_version(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ int result;
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+
+ unsigned long release_version;
+
+ if (strict_strtoul(buf, 0, &release_version))
+ return -EINVAL;
+
+ if (release_version > 0xffff)
+ return -EINVAL;
+
+ result = usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_RELEASE_VERSION,
+ release_version, NULL, 0, 300);
+
+ if (result)
+ return result;
+
+ return count;
+}
+
+static DEVICE_ATTR(release_version, S_IWUGO, NULL, write_release_version);
+
+static ssize_t write_lock_forever(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t count)
+{
+ int result;
+ struct usb_device *usbdev = container_of(dev, struct usb_device, dev);
+
+ unsigned long lock;
+
+ if (strict_strtoul(buf, 0, &lock))
+ return -EINVAL;
+
+ if (lock != 1)
+ return count;
+
+ result = usb_control_msg(usbdev,
+ usb_sndctrlpipe(usbdev, 0),
+ CP2101_EEPROM, REQTYPE_HOST_TO_DEVICE, EEPROM_LOCK,
+ 0x00f0, NULL, 0, 300);
+
+ if (result)
+ return result;
+
+ return count;
+}
+
+static DEVICE_ATTR(lock_forever, S_IWUGO, NULL, write_lock_forever);
+
+static struct attribute *cp2101_attributes[] = {
+ &dev_attr_reload.attr,
+ &dev_attr_vendor_id.attr,
+ &dev_attr_product_id.attr,
+ &dev_attr_productstring.attr,
+ &dev_attr_serialnumber.attr,
+ &dev_attr_self_powered.attr,
+ &dev_attr_max_power.attr,
+ &dev_attr_release_version.attr,
+ &dev_attr_lock_forever.attr,
+ NULL
+};
+
+static const struct attribute_group cp2101_group = {
+ .attrs = cp2101_attributes,
+};
+
static int cp2101_startup (struct usb_serial *serial)
{
+ int err;
+
/* CP2101 buffers behave strangely unless device is reset */
usb_reset_device(serial->dev);
- return 0;
+
+ if (!enable_config)
+ return 0;
+
+ err = sysfs_create_group(&serial->dev->dev.kobj, &cp2101_group);
+
+ return err;
}

static void cp2101_shutdown (struct usb_serial *serial)
@@ -721,6 +1038,11 @@ static void cp2101_shutdown (struct usb_
for (i=0; i < serial->num_ports; ++i) {
cp2101_cleanup(serial->port[i]);
}
+
+ if (!enable_config)
+ return;
+
+ sysfs_remove_group(&serial->dev->dev.kobj, &cp2101_group);
}

static int __init cp2101_init (void)
@@ -758,3 +1080,6 @@ MODULE_LICENSE("GPL");

module_param(debug, bool, S_IRUGO | S_IWUSR);
MODULE_PARM_DESC(debug, "Enable verbose debugging messages");
+
+module_param(enable_config, bool, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(enable_config, "Enable sysfs access to configuration eeprom.");


2008-03-03 15:07:52

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH] usb: add sysfs configuration interface for CP2101

On Mon, 3 Mar 2008, Dirk Eibach wrote:

> From: Dirk Eibach <[email protected]>
>
> The usb configuration data for the Silabs CP2101 usb to uart bridge
> controller can be customized:
> - Vendor ID
> - Product ID
> - Power Descriptor
> - Release Number
> - Serial Number
> - Product Description String
>
> Silabs provides a windows-only tool to do that.
> Since we use linux-only machines in our production-environment, we have no
> proper way to customize the chips for our purpose.
> So I added sysfs configuration support to the linux driver.

Surely this sort of thing is better done in userspace, with libusb.

Alan Stern

2008-03-03 16:30:24

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] usb: add sysfs configuration interface for CP2101

On Mon, Mar 03, 2008 at 10:07:41AM -0500, Alan Stern wrote:
> On Mon, 3 Mar 2008, Dirk Eibach wrote:
>
> > From: Dirk Eibach <[email protected]>
> >
> > The usb configuration data for the Silabs CP2101 usb to uart bridge
> > controller can be customized:
> > - Vendor ID
> > - Product ID
> > - Power Descriptor
> > - Release Number
> > - Serial Number
> > - Product Description String
> >
> > Silabs provides a windows-only tool to do that.
> > Since we use linux-only machines in our production-environment, we have no
> > proper way to customize the chips for our purpose.
> > So I added sysfs configuration support to the linux driver.
>
> Surely this sort of thing is better done in userspace, with libusb.

I agree, any reason why you have not tried that? Adding files like
"vendor_id" to an interface directory in sysfs could cause a lot of
people to get very confused, very quickly :)

thanks,

greg k-h

2008-03-04 07:03:35

by Eibach, Dirk

[permalink] [raw]
Subject: Re: [PATCH] usb: add sysfs configuration interface for CP2101

[email protected] wrote:
> On Mon, Mar 03, 2008 at 10:07:41AM -0500, Alan Stern wrote:
>> On Mon, 3 Mar 2008, Dirk Eibach wrote:
>>
>>> From: Dirk Eibach <[email protected]>
>>>
>>> The usb configuration data for the Silabs CP2101 usb to uart bridge
>>> controller can be customized:
>>> - Vendor ID
>>> - Product ID
>>> - Power Descriptor
>>> - Release Number
>>> - Serial Number
>>> - Product Description String
>>>
>>> Silabs provides a windows-only tool to do that.
>>> Since we use linux-only machines in our production-environment, we have no
>>> proper way to customize the chips for our purpose.
>>> So I added sysfs configuration support to the linux driver.
>> Surely this sort of thing is better done in userspace, with libusb.
>
> I agree, any reason why you have not tried that? Adding files like
> "vendor_id" to an interface directory in sysfs could cause a lot of
> people to get very confused, very quickly :)

Greg, any reason why you didn't suggest using libusb in your first mail ;) ?

I have not used libusb yet, but looking at the api it seems to be just what I need. I had heard about libusb before, but never really had a look at it. So thanks for your advice, Alan. I will implement this stuff in userspace.

On the other hand it would a little bit sad, if Silabs windows tool would stay the only way to setup for all other people. I cannot see how I could spread the userspace tool the way it would be spread if it was in the kernel.
Maybe someone has an idea on that.

So, sorry for the noise guys.

Cheers
Dirk


2008-03-04 17:52:21

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH] usb: add sysfs configuration interface for CP2101

On Tue, 4 Mar 2008, Dirk Eibach wrote:

> On the other hand it would a little bit sad, if Silabs windows tool
> would stay the only way to setup for all other people. I cannot see
> how I could spread the userspace tool the way it would be spread if
> it was in the kernel. Maybe someone has an idea on that.

Maybe you shouldn't try to spread your userspace tool as widely as the
kernel. After all, most kernel users don't have a CP2101.

If you put the tool on a web site, people who need it will be able to
find it using Google.

Alan Stern

2008-03-04 19:55:41

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] usb: add sysfs configuration interface for CP2101

On Tue, Mar 04, 2008 at 08:03:11AM +0100, Dirk Eibach wrote:
> [email protected] wrote:
> > On Mon, Mar 03, 2008 at 10:07:41AM -0500, Alan Stern wrote:
> >> On Mon, 3 Mar 2008, Dirk Eibach wrote:
> >>
> >>> From: Dirk Eibach <[email protected]>
> >>>
> >>> The usb configuration data for the Silabs CP2101 usb to uart bridge
> >>> controller can be customized:
> >>> - Vendor ID
> >>> - Product ID
> >>> - Power Descriptor
> >>> - Release Number
> >>> - Serial Number
> >>> - Product Description String
> >>>
> >>> Silabs provides a windows-only tool to do that.
> >>> Since we use linux-only machines in our production-environment, we have no
> >>> proper way to customize the chips for our purpose.
> >>> So I added sysfs configuration support to the linux driver.
> >> Surely this sort of thing is better done in userspace, with libusb.
> >
> > I agree, any reason why you have not tried that? Adding files like
> > "vendor_id" to an interface directory in sysfs could cause a lot of
> > people to get very confused, very quickly :)
>
> Greg, any reason why you didn't suggest using libusb in your first mail ;) ?

My appologies, I totally forgot about it at the moment. It's usually my
first question when confronted with a new driver, not a patch to an
existing one.

> I have not used libusb yet, but looking at the api it seems to be just
> what I need. I had heard about libusb before, but never really had a
> look at it. So thanks for your advice, Alan. I will implement this
> stuff in userspace.
>
> On the other hand it would a little bit sad, if Silabs windows tool
> would stay the only way to setup for all other people. I cannot see
> how I could spread the userspace tool the way it would be spread if it
> was in the kernel. Maybe someone has an idea on that.

Just get it added to all of the distros, that shouldn't be that hard, I
think almost all of them let you request a package to be added.

thanks,

greg k-h