On Wed, Oct 09, 2013 at 12:57:45PM +0200, Johan Hovold wrote:
> On Wed, Sep 25, 2013 at 11:53:00AM +0200, Andrew Lunn wrote:
[...]
> > +#define MX_INT_RS232 0
> > +#define MX_INT_2W_RS485 1
> > +#define MX_INT_RS422 2
> > +#define MX_INT_4W_RS485 3
> > +static ssize_t show_four_wire(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct usb_serial_port *port = to_usb_serial_port(dev);
> > + struct mxuport_port *mxport = usb_get_serial_port_data(port);
> > +
> > + return sprintf(buf, "%i\n", mxport->four_wire_rs485);
>
> Use scnprintf (and PAGE_SIZE) even if it's a bool.
>
> > +}
> > +
> > +static ssize_t store_four_wire(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf, size_t size)
> > +{
> > + struct usb_serial_port *port = to_usb_serial_port(dev);
> > + struct mxuport_port *mxport = usb_get_serial_port_data(port);
> > +
> > + if (strtobool(buf, &mxport->four_wire_rs485) < 0)
> > + return -EINVAL;
> > +
> > + return size;
> > +}
> > +
> > +static DEVICE_ATTR(4_wire_rs485, S_IWUSR | S_IRUGO,
> > + show_four_wire, store_four_wire);
>
> Use DEVICE_ATTR_RW.
>
> You dropped RS422-support from v3. Was that intentional?
>
> Hmmm. I've been thinking a bit how best to handle this, and I think we
> should consider adding a SER_RS485_4_WIRE flag to serial_rs485 instead
> of having a custom attribute. That still leaves RS422, which could
> possibly be enabled using the RS485-ioctl as well (or we use a
> rs422-attribute for now).
>
> I'll get back to you on this.
After giving this some more thought I don't really think the
TIOCSRS485-ioctl interface is appropriate after all. The
RQ_VENDOR_SET_INTERFACE-request doesn't just enable 2-wire-rs485-style
signalling, but also changes which pins on the DB9-connector that are
used.
This should probably remain a device-specific parameter. We already have
the MOXA mxser-driver with similar configuration options. This one uses
a custom ioctl to set the interface, but I think we should use a sysfs
parameter for this.
What do you say about a simple "interface"-parameter representing the
four MX_INT-values above? A string parameter would perhaps be even
better if you combine it with an "interface_available"-parameter, for
example:
# cat /sys/bus/usb-serial/devices/ttyUSB0/interface_available
rs232 rs422 rs485-2w rs485-4w
for devices which can use all four modes.
Sorry about the confusion.
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
