Hi Jan,
thanks a lot for the super-quick review!
On Tue, Jun 05, 2018 at 07:36:42AM +0200, Jan Kiszka wrote:
> On 2018-06-04 23:33, Daniel Golle wrote:
> > Move the non-board-specific part of the RS485 initialization from
> > iot2040_rs485_config function to a new generic function used also for
> > other boards.
> > This allows using TIOCGRS485 and TIOCSRS485 on boards (such as mPCIe
> > serial IO modules) which are hard-wired to RS485 or have jumpers for
> > their configurations.
>
> If the configuration is hardwired on that board, is it a good idea to
> ask userspace to enable the mode also manually? The alternative would be
> deriving it from the IDs of those modules - ok, provided they are known...
Well, what I got here is a mPCIe modules based on Exar XR17V352,
PCI ID 13a8:0352 like all other Exar devices and no EEPROM
what-so-ever. The board got jumpers to configure each port to
be either half-duplex (RS485) or full-duplex (RS422) -- and there is
no way to know in software which jumper setting is currently chosen.
Yet the device requires UART_FCTR_EXAR_485 to be set in addition to
the corresponding jumper position in order for half-duplex mode to
work...
>
> >
> > Signed-off-by: Daniel Golle <daniel@xxxxxxxxxxxxxx>
> > ---
> > drivers/tty/serial/8250/8250_exar.c | 38 +++++++++++++++++++----------
> > 1 file changed, 25 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
> > index e0aa5f03004c..55b4d6d581f0 100644
> > --- a/drivers/tty/serial/8250/8250_exar.c
> > +++ b/drivers/tty/serial/8250/8250_exar.c
> > @@ -275,8 +275,32 @@ static int xr17v35x_register_gpio(struct pci_dev *pcidev,
> > return 0;
> > }
> >
> > +static int generic_rs485_config(struct uart_port *port,
> > + struct serial_rs485 *rs485)
> > +{
> > + bool is_rs485 = !!(rs485->flags & SER_RS485_ENABLED);
> > + u8 __iomem *p = port->membase;
> > + u8 value;
> > +
> > + value = readb(p + UART_EXAR_FCTR);
> > + if (is_rs485)
> > + value |= UART_FCTR_EXAR_485;
> > + else
> > + value &= ~UART_FCTR_EXAR_485;
> > +
> > + writeb(value, p + UART_EXAR_FCTR);
> > +
> > + if (is_rs485)
> > + writeb(UART_EXAR_RS485_DLY(4), p + UART_MSR);
> > +
> > + port->rs485 = *rs485;
> > +
> > + return 0;
> > +}
> > +
> > static const struct exar8250_platform exar8250_default_platform = {
> > .register_gpio = xr17v35x_register_gpio,
> > + .rs485_config = generic_rs485_config,
> > };
> >
> > static int iot2040_rs485_config(struct uart_port *port,
> > @@ -309,19 +333,7 @@ static int iot2040_rs485_config(struct uart_port *port,
> > value |= mode;
> > writeb(value, p + UART_EXAR_MPIOLVL_7_0);
> >
> > - value = readb(p + UART_EXAR_FCTR);
> > - if (is_rs485)
> > - value |= UART_FCTR_EXAR_485;
> > - else
> > - value &= ~UART_FCTR_EXAR_485;
> > - writeb(value, p + UART_EXAR_FCTR);
> > -
> > - if (is_rs485)
> > - writeb(UART_EXAR_RS485_DLY(4), p + UART_MSR);
> > -
> > - port->rs485 = *rs485;
> > -
> > - return 0;
> > + return generic_rs485_config(port, rs485);
> > }
> >
> > static const struct property_entry iot2040_gpio_properties[] = {
> >
>
> From the code perspective, this is fine:
>
> Reviewed-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
Cheers!
Daniel
>
> Jan
>
> --
> Siemens AG, Corporate Technology, CT RDA IOT SES-DE
> Corporate Competence Center Embedded Linux
--
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
