On Thu, Dec 08, 2016 at 10:13:37PM +0300, Maksim Salau wrote: > The adaptor can be found on development boards for 78k, RL78 and V850 > microcontrollers produced by Renesas Electronics Corporation. > > This is not a full-featured USB to serial converter, however it allows > basic communication and simple control which is enough for programming of > on-board flash and debugging through a debug monitor. > > uPD78F0730 is a USB-enabled microcontroller with USB-to-UART conversion > implemented in firmware. > > This chip is also present in some debugging adaptors which use it for > USB-to-SPI conversion as well. The present driver doesn't cover SPI, > only USB-to-UART conversion is supported. > > Signed-off-by: Maksim Salau <maksim.salau@xxxxxxxxx> > --- > > PATCH v2 can be found here: > http://thread.gmane.org/gmane.linux.usb.general/137999 > > Changes from v2: > * Addressed all comments > * Dropped inversion of DTR and RTS signals Did you figure out whether this was needed or not, for example by comparing to the hardware flow control levels? > * Removed support of software flow control and > added hardware flow control (Both are untested) > > The device I have have only one output signal that is toggled by > tiocmset(fd, TIOCM_DTR, 0) and tiocmset(fd, TIOCM_RTS, 0). What do you mean by the above? Does your device only have one of DTR and RTS wired? I'd still expect you to only use one of the TIOCM defines to set and clear it. > drivers/usb/serial/Kconfig | 9 + > drivers/usb/serial/Makefile | 1 + > drivers/usb/serial/upd78f0730.c | 445 ++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 455 insertions(+) > create mode 100644 drivers/usb/serial/upd78f0730.c > > diff --git a/drivers/usb/serial/Kconfig b/drivers/usb/serial/Kconfig > index 56ecb8b..311d1cf 100644 > --- a/drivers/usb/serial/Kconfig > +++ b/drivers/usb/serial/Kconfig > @@ -703,6 +703,15 @@ config USB_SERIAL_QT2 > To compile this driver as a module, choose M here: the > module will be called quatech-serial. > > +config USB_SERIAL_UPD78F0730 > + tristate "USB Renesas uPD78F0730 Single Port Serial Driver" > + help > + Say Y here if you want to use the Renesas uPD78F0730 > + serial driver. > + > + To compile this driver as a module, choose M here: the > + module will be called upd78f0730. > + > config USB_SERIAL_DEBUG > tristate "USB Debugging Device" > help > diff --git a/drivers/usb/serial/Makefile b/drivers/usb/serial/Makefile > index 349d9df..93fc707 100644 > --- a/drivers/usb/serial/Makefile > +++ b/drivers/usb/serial/Makefile > @@ -60,3 +60,4 @@ obj-$(CONFIG_USB_SERIAL_WISHBONE) += wishbone-serial.o > obj-$(CONFIG_USB_SERIAL_WHITEHEAT) += whiteheat.o > obj-$(CONFIG_USB_SERIAL_XIRCOM) += keyspan_pda.o > obj-$(CONFIG_USB_SERIAL_XSENS_MT) += xsens_mt.o > +obj-$(CONFIG_USB_SERIAL_UPD78F0730) += upd78f0730.o Please keep the entries sorted by config-symbol name (i.e. put it after CONFIG_USB_SERIAL_TI). > diff --git a/drivers/usb/serial/upd78f0730.c b/drivers/usb/serial/upd78f0730.c > new file mode 100644 > index 0000000..e2bed13 > --- /dev/null > +++ b/drivers/usb/serial/upd78f0730.c > @@ -0,0 +1,445 @@ > +/* > + * Renesas Electronics uPD78F0730 USB to serial converter driver > + * > + * Copyright (C) 2014,2016 Maksim Salau <maksim.salau@xxxxxxxxx> > + * > + * 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. As I mentioned in my comments to v2, this should match the MODULE_LICENSE below which is currently "GPL" (GNU Public License v2 or later). Either use "GPL v2" as MODULE_LICENSE or update the text here. > + * > + * Protocol of the adaptor is described in the application note U19660EJ1V0AN00 > + * μPD78F0730 8-bit Single-Chip Microcontroller > + * USB-to-Serial Conversion Software > + * <https://www.renesas.com/en-eu/doc/DocumentServer/026/U19660EJ1V0AN00.pdf> > + * > + * The adaptor functionality is limited to the following: > + * - data bits: 7 or 8 > + * - stop bits: 1 or 2 > + * - parity: even, odd or none > + * - flow control: hardware or none > + * - baud rates: 2400, 4800, 9600, 19200, 38400, 57600, 115200 > + * - signals: DTS, RTS and BREAK DTR? > + * - there is an option to enable parity error character substitution, > + * but is not supported by the driver > + * - XON/XOFF flow control is described in the document, > + * but is not supported by the driver > + */ > + > +#include <linux/kernel.h> > +#include <linux/string.h> > +#include <linux/module.h> > +#include <linux/slab.h> > +#include <linux/spinlock.h> > +#include <linux/tty.h> > +#include <linux/usb.h> > +#include <linux/usb/serial.h> > + > +#define DRIVER_DESC "Renesas uPD78F0730 USB to serial converter driver" > + > +#define DRIVER_AUTHOR "Maksim Salau <maksim.salau@xxxxxxxxx>" > + > +static const struct usb_device_id id_table[] = { > + { USB_DEVICE(0x045B, 0x0212) }, /* YRPBRL78G13, YRPBRL78G14 */ > + { USB_DEVICE(0x0409, 0x0063) }, /* V850ESJX3-STICK */ > + {} > +}; > + > +MODULE_DEVICE_TABLE(usb, id_table); > + > +/* > + * Private data structure type declaration. I'd drop this sentence, the struct name is self describing. In fact, rename it upd78f0730_port_private, since you now associate it with the port (rather than struct usb_serial). > + * Each adaptor is associated with a private structure, that holds the current > + * state of control signals (DTR, RTS and BREAK). > + */ > +struct upd78f0730_serial_private { > + struct mutex lock; /* mutex to protect line_signals */ > + u8 line_signals; > +}; > + > +/* Op-codes of control commands */ > +#define UPD78F0730_CMD_LINE_CONTROL 0x00 > +#define UPD78F0730_CMD_SET_DTR_RTS 0x01 > +#define UPD78F0730_CMD_SET_XON_XOFF_CHR 0x02 > +#define UPD78F0730_CMD_OPEN_CLOSE 0x03 > +#define UPD78F0730_CMD_SET_ERR_CHR 0x04 > + > +/* Data sizes in UPD78F0730_CMD_LINE_CONTROL command */ > +#define UPD78F0730_DATA_SIZE_7_BITS 0x00 > +#define UPD78F0730_DATA_SIZE_8_BITS 0x01 > +#define UPD78F0730_DATA_SIZE_MASK 0x01 > + > +/* Stop-bit modes in UPD78F0730_CMD_LINE_CONTROL command */ > +#define UPD78F0730_STOP_BIT_1_BIT 0x00 > +#define UPD78F0730_STOP_BIT_2_BIT 0x02 > +#define UPD78F0730_STOP_BIT_MASK 0x02 > + > +/* Parity modes in UPD78F0730_CMD_LINE_CONTROL command */ > +#define UPD78F0730_PARITY_NONE 0x00 > +#define UPD78F0730_PARITY_EVEN 0x04 > +#define UPD78F0730_PARITY_ODD 0x08 > +#define UPD78F0730_PARITY_MASK 0x0C > + > +/* Flow control modes in UPD78F0730_CMD_LINE_CONTROL command */ > +#define UPD78F0730_FLOW_CONTROL_NONE 0x00 > +#define UPD78F0730_FLOW_CONTROL_HW 0x10 > +#define UPD78F0730_FLOW_CONTROL_SW 0x20 > +#define UPD78F0730_FLOW_CONTROL_MASK 0x30 > + > +/* Control signal bits in UPD78F0730_CMD_SET_DTR_RTS command */ > +#define UPD78F0730_RTS 0x01 > +#define UPD78F0730_DTR 0x02 > +#define UPD78F0730_BREAK 0x04 > + > +/* Port modes in UPD78F0730_CMD_OPEN_CLOSE command */ > +#define UPD78F0730_PORT_CLOSE 0x00 > +#define UPD78F0730_PORT_OPEN 0x01 > + > +/* Error character substitution modes in UPD78F0730_CMD_SET_ERR_CHR command */ > +#define UPD78F0730_ERR_CHR_DISABLED 0x00 > +#define UPD78F0730_ERR_CHR_ENABLED 0x01 > + > +/* > + * Declaration of command structures > + */ > + > +/* UPD78F0730_CMD_LINE_CONTROL command */ > +struct line_control { Please use a common prefix (e.g. upd_) also for the command structs. > + u8 opcode; > + __le32 baud_rate; > + u8 params; > +} __packed; > + > +/* UPD78F0730_CMD_SET_DTR_RTS command */ > +struct set_dtr_rts { > + u8 opcode; > + u8 params; > +}; > + > +/* UPD78F0730_CMD_SET_XON_OFF_CHR command */ > +struct set_xon_xoff_chr { > + u8 opcode; > + u8 xon; > + u8 xoff; > +}; > + > +/* UPD78F0730_CMD_OPEN_CLOSE command */ > +struct open_close { > + u8 opcode; > + u8 state; > +}; > + > +/* UPD78F0730_CMD_SET_ERR_CHR command */ > +struct set_err_chr { > + u8 opcode; > + u8 state; > + u8 err_char; > +}; > + > +static int upd78f0730_send_ctl(struct usb_serial_port *port, > + void *data, int size) > +{ > + int res; > + struct device *dev = &port->dev; > + struct usb_device *usbdev = port->serial->dev; > + void *buf; Please reorder these declarations and keep the more complex types first (and res last). > + > + if (!size) > + return 0; -EINVAL? > + if (!data) { > + dev_err(dev, "invalid arguments\n"); Drop the dev_err. > + return -EINVAL; > + } But as I mentioned before, you can just drop both these tests. Your driver always passes a buffer (and even if it didn't, things would still work). > + > + buf = kmemdup(data, size, GFP_KERNEL); > + No empty line. > + if (!buf) > + return -ENOMEM; > + > + res = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0), 0x00, > + USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT, > + 0x0000, 0x0000, buf, size, USB_CTRL_SET_TIMEOUT); > + > + kfree(buf); > + > + if (res < 0 || res != size) { > + dev_err(dev, "send failed: opcode=%02x, size=%d, res=%d\n", > + *(u8 *)data, size, res); > + /* The maximum expected length of a transfer is 6 bytes */ > + return -EIO; Just return the error from USB core in case res < 0, otherwise -EIO (as also mentioned before). > + } > + > + return 0; > +} > + > +static int upd78f0730_port_probe(struct usb_serial_port *port) > +{ > + struct upd78f0730_serial_private *private; > + > + private = kzalloc(sizeof(*private), GFP_KERNEL); > + if (!private) > + return -ENOMEM; Newline. > + mutex_init(&private->lock); > + private->line_signals = 0; You just kzalloced above. > + usb_set_serial_port_data(port, private); Newline. > + return 0; > +} > + > +static int upd78f0730_port_remove(struct usb_serial_port *port) > +{ > + struct upd78f0730_serial_private *private; > + > + private = usb_get_serial_port_data(port); > + mutex_destroy(&private->lock); > + kfree(private); Newline. > + return 0; > +} > + > +static int upd78f0730_tiocmget(struct tty_struct *tty) > +{ > + int res = 0; > + int signals; > + struct device *dev = tty->dev; > + struct upd78f0730_serial_private *private; > + struct usb_serial_port *port = tty->driver_data; Reorder as mentioned above, and same below. > + > + private = usb_get_serial_port_data(port); > + > + mutex_lock(&private->lock); > + signals = private->line_signals; > + mutex_unlock(&private->lock); > + > + res = ((signals & UPD78F0730_DTR) ? TIOCM_DTR : 0) | > + ((signals & UPD78F0730_RTS) ? TIOCM_RTS : 0); > + > + dev_dbg(dev, "res = %x\n", res); Add "%s - " __func__ prefix to make message self-contained. > + > + return res; > +} > + > +static int upd78f0730_tiocmset(struct tty_struct *tty, > + unsigned int set, unsigned int clear) > +{ > + int res; > + struct device *dev = tty->dev; > + struct upd78f0730_serial_private *private; > + struct usb_serial_port *port = tty->driver_data; > + struct set_dtr_rts request = { .opcode = UPD78F0730_CMD_SET_DTR_RTS }; > + > + private = usb_get_serial_port_data(port); > + > + mutex_lock(&private->lock); > + if (set & TIOCM_DTR) { > + private->line_signals |= UPD78F0730_DTR; > + dev_dbg(dev, "set DTR\n"); > + } > + if (set & TIOCM_RTS) { > + private->line_signals |= UPD78F0730_RTS; > + dev_dbg(dev, "set RTS\n"); > + } > + if (clear & TIOCM_DTR) { > + private->line_signals &= ~UPD78F0730_DTR; > + dev_dbg(dev, "reset DTR\n"); s/reset/clear/ > + } > + if (clear & TIOCM_RTS) { > + private->line_signals &= ~UPD78F0730_RTS; > + dev_dbg(dev, "reset RTS\n"); > + } > + request.params = private->line_signals; > + > + res = upd78f0730_send_ctl(port, &request, sizeof(request)); > + mutex_unlock(&private->lock); > + > + return res; > +} > + > +static void upd78f0730_break_ctl(struct tty_struct *tty, int break_state) > +{ > + struct device *dev = tty->dev; > + struct upd78f0730_serial_private *private; > + struct usb_serial_port *port = tty->driver_data; > + struct set_dtr_rts request = { .opcode = UPD78F0730_CMD_SET_DTR_RTS }; > + > + private = usb_get_serial_port_data(port); > + > + mutex_lock(&private->lock); > + if (break_state) { > + private->line_signals |= UPD78F0730_BREAK; > + dev_dbg(dev, "set BREAK\n"); > + } else { > + private->line_signals &= ~UPD78F0730_BREAK; > + dev_dbg(dev, "reset BREAK\n"); s/reset/clear/ > + } > + request.params = private->line_signals; > + > + upd78f0730_send_ctl(port, &request, sizeof(request)); > + mutex_unlock(&private->lock); > +} > + > +static void upd78f0730_dtr_rts(struct usb_serial_port *port, int on) > +{ > + struct tty_struct *tty = port->port.tty; > + unsigned int set = 0; > + unsigned int clear = 0; > + > + if (on) > + set = TIOCM_DTR | TIOCM_RTS; > + else > + clear = TIOCM_DTR | TIOCM_RTS; > + > + upd78f0730_tiocmset(tty, set, clear); > +} > + > +static speed_t upd78f0730_get_baud_rate(struct tty_struct *tty) > +{ > + int i; > + const tcflag_t baud_rate = C_BAUD(tty); Use tty_get_baud_rate() to retrieve the requested rate (need to support BOTHER) and then look it that rate up in your table. > + const tcflag_t supported[] = {B2400, B4800, B9600, > + B19200, B38400, B57600, B115200}; > + > + for (i = ARRAY_SIZE(supported) - 1; i >= 0; i--) { > + if (baud_rate == supported[i]) > + return tty_get_baud_rate(tty); > + } > + > + /* If the baud rate is not supported, switch to the default baud rate */ > + tty->termios.c_cflag &= ~CBAUD; > + tty->termios.c_cflag |= B9600; Don't set these directly, use tty_encode_baud_rate(). > + > + return tty_get_baud_rate(tty); > +} > + > +static void upd78f0730_set_termios(struct tty_struct *tty, > + struct usb_serial_port *port, > + struct ktermios *old_termios) > +{ > + struct device *dev = &port->dev; > + struct line_control request_control; > + speed_t baud_rate; > + > + if (old_termios && !tty_termios_hw_change(&tty->termios, old_termios)) > + return; > + > + if (C_BAUD(tty) == B0) > + upd78f0730_dtr_rts(port, 0); Also raise DTR/RTS if changing from B0 (i.e. when you have old_termios). > + > + baud_rate = upd78f0730_get_baud_rate(tty); > + request_control.opcode = UPD78F0730_CMD_LINE_CONTROL; > + request_control.baud_rate = cpu_to_le32(baud_rate); > + request_control.params = 0; > + dev_dbg(dev, "baud rate = %d\n", baud_rate); > + > + switch (C_CSIZE(tty)) { > + case CS7: > + request_control.params |= UPD78F0730_DATA_SIZE_7_BITS; > + dev_dbg(dev, "7 data bits\n"); > + break; > + default: > + tty->termios.c_cflag &= ~CSIZE; > + tty->termios.c_cflag |= CS8; > + dev_warn(dev, "data size is not supported, using 8 bits\n"); > + /* fall through */ > + case CS8: > + request_control.params |= UPD78F0730_DATA_SIZE_8_BITS; > + dev_dbg(dev, "8 data bits\n"); > + break; > + } > + > + if (C_PARENB(tty)) { > + if (C_PARODD(tty)) { > + request_control.params |= UPD78F0730_PARITY_ODD; > + dev_dbg(dev, "odd parity\n"); > + } else { > + request_control.params |= UPD78F0730_PARITY_EVEN; > + dev_dbg(dev, "even parity\n"); > + } > + > + if (C_CMSPAR(tty)) { > + tty->termios.c_cflag &= ~CMSPAR; > + dev_warn(dev, "MARK/SPACE parity is not supported\n"); > + } > + } else { > + request_control.params |= UPD78F0730_PARITY_NONE; > + dev_dbg(dev, "no parity\n"); > + } > + > + if (C_CSTOPB(tty)) { > + request_control.params |= UPD78F0730_STOP_BIT_2_BIT; > + dev_dbg(dev, "2 stop bits\n"); > + } else { > + request_control.params |= UPD78F0730_STOP_BIT_1_BIT; > + dev_dbg(dev, "1 stop bit\n"); > + } > + > + if (C_CRTSCTS(tty)) { > + request_control.params |= UPD78F0730_FLOW_CONTROL_HW; > + dev_dbg(dev, "hardware flow control\n"); > + } else { > + request_control.params |= UPD78F0730_FLOW_CONTROL_NONE; > + dev_dbg(dev, "no flow control\n"); > + } This should be coordinated with B0 handling (e.g. disable hardware flow control if B0 is requested). > + > + if (I_IXOFF(tty) || I_IXON(tty)) { > + tty->termios.c_iflag &= ~(IXOFF | IXON); > + dev_warn(dev, "software flow control is not supported\n"); > + } > + > + upd78f0730_send_ctl(port, &request_control, sizeof(request_control)); > +} > + > +static int upd78f0730_open(struct tty_struct *tty, struct usb_serial_port *port) > +{ > + int res; > + struct open_close request_open = { > + .opcode = UPD78F0730_CMD_OPEN_CLOSE, > + .state = UPD78F0730_PORT_OPEN > + }; > + > + res = upd78f0730_send_ctl(port, &request_open, sizeof(request_open)); > + if (res) > + return res; > + > + if (tty) > + upd78f0730_set_termios(tty, port, NULL); > + > + return usb_serial_generic_open(tty, port); > +} > + > +static void upd78f0730_close(struct usb_serial_port *port) > +{ > + struct open_close request_close = { > + .opcode = UPD78F0730_CMD_OPEN_CLOSE, > + .state = UPD78F0730_PORT_CLOSE > + }; > + > + usb_serial_generic_close(port); > + upd78f0730_send_ctl(port, &request_close, sizeof(request_close)); > +} > + > +static struct usb_serial_driver upd78f0730_device = { > + .driver = { > + .owner = THIS_MODULE, > + .name = "upd78f0730", > + }, > + .id_table = id_table, > + .num_ports = 1, > + .port_probe = upd78f0730_port_probe, > + .port_remove = upd78f0730_port_remove, > + .open = upd78f0730_open, > + .close = upd78f0730_close, > + .set_termios = upd78f0730_set_termios, > + .tiocmget = upd78f0730_tiocmget, > + .tiocmset = upd78f0730_tiocmset, > + .dtr_rts = upd78f0730_dtr_rts, > + .break_ctl = upd78f0730_break_ctl, > +}; > + > +static struct usb_serial_driver * const serial_drivers[] = { > + &upd78f0730_device, > + NULL > +}; > + > +module_usb_serial_driver(serial_drivers, id_table); > + > +MODULE_DESCRIPTION(DRIVER_DESC); > +MODULE_AUTHOR(DRIVER_AUTHOR); > +MODULE_LICENSE("GPL"); Thanks, 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
