On Mon, Apr 11, 2016 at 02:27:20PM -0700, Aaron Marburg wrote:
>
> Add callbacks to handle TIOCGRS485 and TIOCSRS485 ioctl
> calls in mos7840.c, allowing configuration of the chip's
> "scratchpad" register to strobe the DTR line while transmitting.
>
> This functionality is required for RS485 mode on the B&B Electronics
> USOPTL4-4P and USOPTL4-2P USB-to-RS485/422 hubs based on this chip.
>
> Signed-off-by: Aaron Marburg <amarburg@xxxxxxxxxxxxxxxxxx>
> ---
> Changes relative to v2:
> -- As suggested, implemented handlers for TIOC?RS485 ioctl callbacks
> rather than command-line options to enable RS485 configurations.
Thanks for the update, and sorry for not getting back to you on this
one.
Always remember to run checkpatch.pl on patches before submitting, it
would have let know about a few minor issues.
> drivers/usb/serial/mos7840.c | 78 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 78 insertions(+)
>
> diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
> index ed378fb..8a78143 100644
> --- a/drivers/usb/serial/mos7840.c
> +++ b/drivers/usb/serial/mos7840.c
> @@ -1977,6 +1977,74 @@ static int mos7840_get_serial_info(struct moschip_port *mos7840_port,
> }
>
> /*****************************************************************************
> + * mos7840_get_rs485_info
> + * function to handle RS485 "get" ioctl
> + *****************************************************************************/
> +
No need to add such function headers, just drop them.
> +static int mos7840_get_rs485_info(struct usb_serial_port *port,
> + struct serial_rs485 __user *retinfo)
> +{
> + struct serial_rs485 tmp;
> + int status;
> + __u16 scratchpad;
Use u16.
> +
> + if (port == NULL)
> + return -EFAULT;
> +
> + if (!retinfo)
> + return -EFAULT;
Neither NULL check is needed.
> +
> + memset(&tmp, 0, sizeof(tmp));
> +
> + status = mos7840_get_uart_reg(port, SCRATCH_PAD_REGISTER, &scratchpad);
> + if (status < 0) {
> + dev_dbg(&port->dev, "Reading ScratchPad (RS485 config) register failed status=0x%x\n", status);
This should be dev_err, and please use a more compact error such as
"failed to read scratch-pad register: %d\n"
> + return -EFAULT;
Return usb_translate_errors(status) here.
> + }
> +
> + tmp.flags |= (scratchpad & 0x80) ? SER_RS485_ENABLED : 0x00;
> + tmp.flags |= (scratchpad & 0x40) ? SER_RS485_RTS_ON_SEND : 0x00;
> +
> + if (copy_to_user(retinfo, &tmp, sizeof(*retinfo)))
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> +/*****************************************************************************
> + * mos7840_set_rs485_info
> + * function to handle RS485 "set" ioctl
> + *****************************************************************************/
> +
> +static int mos7840_set_rs485_info(struct usb_serial_port *port,
> + struct serial_rs485 __user *retinfo)
> +{
> + struct serial_rs485 tmp;
> + int status;
> + __u16 scratchpad = 0;
> +
> + if (port == NULL)
> + return -EFAULT;
> +
> + if (!retinfo)
> + return -EFAULT;
Same comments as above.
> + if (copy_from_user(&tmp, retinfo, sizeof(*retinfo)))
> + return -EFAULT;
> +
> + if (tmp.flags & SER_RS485_ENABLED)
> + scratchpad = 0x80 | ((tmp.flags & SER_RS485_RTS_ON_SEND) ? 0x40 : 0x00);
Please us an inner conditional for rts-on-send.
> +
> + status = mos7840_set_uart_reg(port, SCRATCH_PAD_REGISTER, scratchpad);
> + if (status < 0) {
> + dev_dbg(&port->dev, "Writing ScratchPad (RS485 config) register failed status=0x%x\n", status);
> + return -EFAULT;
Same comments as above apply.
> + }
You also need to clear any unsupported fields in retinfo, and copy the
accepted flags back before returning.
> +
> + return 0;
> +}
> +
> +/*****************************************************************************
> * SerialIoctl
> * this function handles any ioctl calls to the driver
> *****************************************************************************/
> @@ -2010,6 +2078,16 @@ static int mos7840_ioctl(struct tty_struct *tty,
> case TIOCSSERIAL:
> dev_dbg(&port->dev, "%s TIOCSSERIAL\n", __func__);
> break;
> +
> + case TIOCGRS485:
> + dev_dbg(&port->dev, "%s TIOCGRS485\n", __func__);
> + return mos7840_get_rs485_info(port, argp);
> +
> + case TIOCSRS485:
> + dev_dbg(&port->dev, "%s TIOCSRS485\n", __func__);
> + return mos7840_set_rs485_info(port, argp);
> +
> +
Drop the dev_dbgs along whit the empty lines.
> default:
> break;
> }
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
