On Tue, Nov 30, 2004 at 05:25:36PM -0500, Edwin Olson wrote:
> Woah, I did. Someone missed their afternoon nap... <grumble>
>
> Signed-off-by: Edwin Olson ([EMAIL PROTECTED])
Tiny nits, you're almost there :)
You forgot to put the description of the patch in the email body.
> + * (30/Nov/2004) Edwin Olson
> + * Exposed event_char and latency_timer registers via sysfs
> + *
Don't put changelog info in the file itself. It's not good form (yeah,
this driver has done that in the past, I'm trying to wean everyone off
of it as it's unwieldly over time.)
> +/* Sysfs-related declarations */
> +static void create_sysfs_attrs(struct usb_serial *serial);
> +static void remove_sysfs_attrs(struct usb_serial *serial);
You can't just move these functions above where they are called from to
get rid of these declarations?
> @@ -1290,6 +1297,9 @@ static int ftdi_FT232BM_startup (struct
> priv->chip_type = FT232BM;
> priv->baud_base = 48000000 / 2; /* Would be / 16, but FT232BM supports
> multiple of 0.125 divisor fractions! */
>
> + dbg("adding sysfs attributes");
> + create_sysfs_attrs(serial);
> +
Is the dbg() line really needed, as you do it again when you create the
files in the create_sysfs_attrs() function?
> +ssize_t show_latency_timer(struct device *dev, char *buf)
> +{
> + struct usb_serial_port *port = to_usb_serial_port(dev);
> + struct ftdi_private *priv = usb_get_serial_port_data(port);
> + struct usb_device *udev;
> +
> + unsigned short latency=0;
Delete the extra line here, and add spaces around the =
> +
> + int rv = 0;
Delete the line there too.
> +
> + udev = to_usb_device(dev);
> +
> + dbg("%s",__FUNCTION__);
> +
> + rv = usb_control_msg(udev,
> + usb_rcvctrlpipe(udev, 0),
> + FTDI_SIO_GET_LATENCY_TIMER_REQUEST,
> + FTDI_SIO_GET_LATENCY_TIMER_REQUEST_TYPE,
> + 0, priv->interface,
> + (char*) &latency, 1, WDR_TIMEOUT);
> +
> + if (rv < 0) {
> + dev_err(dev, "Unable to read latency timer: %i", rv);
> + return EIO;
return a negative error number please.
> + }
> + return sprintf(buf, "%i\n", latency);
> +}
> +
> +/* Write a new value of the latency timer, in units of milliseconds. */
> +ssize_t store_latency_timer(struct device *dev, const char *valbuf, size_t
> count)
> +{
> + struct usb_serial_port *port = to_usb_serial_port(dev);
> + struct ftdi_private *priv = usb_get_serial_port_data(port);
> + struct usb_device *udev;
> +
> + char buf[1];
> + int v = simple_strtoul(valbuf, NULL, 10);
> + int rv = 0;
Push the variables all together with no blank lines.
> +
> + udev = to_usb_device(dev);
> +
> + dbg("%s: setting latency timer = %i", __FUNCTION__, v);
> +
> + rv = usb_control_msg(udev,
> + usb_sndctrlpipe(udev, 0),
> + FTDI_SIO_SET_LATENCY_TIMER_REQUEST,
> + FTDI_SIO_SET_LATENCY_TIMER_REQUEST_TYPE,
> + v, priv->interface,
> + buf, 0, WDR_TIMEOUT);
> +
> + if (rv < 0) {
> + dev_err(dev, "Unable to write latency timer: %i", rv);
> + return EIO;
Again, return -EIO not EIO.
> + }
>
> + return count;
> +}
> +
> +/* Write an event character directly to the FTDI register. The ASCII
> + value is in the low 8 bits, with the enable bit in the 9th bit. */
> +ssize_t store_event_char(struct device *dev, const char *valbuf, size_t
> count)
> +{
> + struct usb_serial_port *port = to_usb_serial_port(dev);
> + struct ftdi_private *priv = usb_get_serial_port_data(port);
> + struct usb_device *udev;
> +
> + char buf[1];
> + int v = simple_strtoul(valbuf, NULL, 10);
> + int rv = 0;
Same with the extra line.
> +
> + udev = to_usb_device(dev);
> +
> + dbg("%s: setting event char = %i", __FUNCTION__, v);
> +
> + rv = usb_control_msg(udev,
> + usb_sndctrlpipe(udev, 0),
> + FTDI_SIO_SET_EVENT_CHAR_REQUEST,
> + FTDI_SIO_SET_EVENT_CHAR_REQUEST_TYPE,
> + v, priv->interface,
> + buf, 0, WDR_TIMEOUT);
> +
> + if (rv < 0) {
> + dbg("Unable to write event character: %i", rv);
> + return EIO;
Same with the -EIO.
> + }
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR(latency_timer, S_IWUGO | S_IRUGO, show_latency_timer,
> store_latency_timer);
> +static DEVICE_ATTR(event_char, S_IWUGO | S_IRUGO, NULL, store_event_char);
Why make the file readable if it isn't? :)
thanks,
greg k-h
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://productguide.itmanagersjournal.com/
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
