> static inline void __stop_tx(struct uart_8250_port *p)
> {
> + if (p->rs485.flags & SER_RS485_ENABLED) {
> + int ret;
> +
> + ret = (p->rs485.flags & SER_RS485_RTS_AFTER_SEND) ? 1 : 0;
> + if (gpio_get_value(p->rts_gpio) != ret) {
> + if (p->rs485.delay_rts_after_send > 0)
> + mdelay(p->rs485.delay_rts_after_send);
> + gpio_set_value(p->rts_gpio, ret);
> + }
RTS is not normally a GPIO. We should be controlling the UART RTS here,
and if the UART has a magic special case RTS wired to a GPIO then really
the hardware specific part should handle that gunge. I don't care whether
the drive does it via serial_out magic or a more explicit hook but it
doesn't belong here in core code.
Likewise the mdelay probably should be in the device specific bits or
controlled by a flag as not all hardware is so braindead.
> @@ -1330,6 +1356,20 @@ static void serial8250_start_tx(struct uart_port *port)
> if (up->dma && !serial8250_tx_dma(up)) {
Ditto
> +int serial8250_probe_rs485(struct uart_8250_port *up,
> + struct device *dev)
> +{
> + struct serial_rs485 *rs485conf = &up->rs485;
> + struct device_node *np = dev->of_node;
> + u32 rs485_delay[2];
> + enum of_gpio_flags flags;
> + int ret;
> +
> + rs485conf->flags = 0;
> + if (!np)
> + return 0;
> +
> + /* check for tx enable gpio */
> + up->rts_gpio = of_get_named_gpio_flags(np, "rts-gpio", 0, &flags);
No of_ dependencies in core 8250.c either please. That looks a perfectly
good implementation of serial8250_of_probe_rs485 however, just belongs in
the right place.
> +static int serial8250_ioctl(struct uart_port *port, unsigned int cmd,
> + unsigned long arg)
> +{
> + struct serial_rs485 rs485conf;
> + struct uart_8250_port *up;
> +
> + up = container_of(port, struct uart_8250_port, port);
> + switch (cmd) {
> + case TIOCSRS485:
> + if (!gpio_is_valid(up->rts_gpio))
> + return -ENODEV;
GPIO assumption again needs to go
> diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
> index 0ec21ec..056a73f 100644
> --- a/include/linux/serial_8250.h
> +++ b/include/linux/serial_8250.h
> @@ -78,6 +78,7 @@ struct uart_8250_port {
> unsigned char acr;
> unsigned char ier;
> unsigned char lcr;
> + unsigned char fcr;
> unsigned char mcr;
> unsigned char mcr_mask; /* mask of user bits */
> unsigned char mcr_force; /* mask of forced bits */
> @@ -94,6 +95,9 @@ struct uart_8250_port {
> unsigned char msr_saved_flags;
>
> struct uart_8250_dma *dma;
> + struct serial_rs485 rs485;
> + int rts_gpio;
> + bool rts_gpio_valid;
Keeping the gpio here doesn't look unreasonable if one is in use.
Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
