Hi Matthias,
On Friday 19 December 2008 00:48:34 Matthias Fuchs wrote:
> Hi,
>
> please see my patch for enabling the RS485 half-duplex control below.
> Please note that I am using this on a PowerPC platform. So I needed to
> add the ioctl to the PowerPC header. As Wolfgang stated, it is
> already in the x86 header. I am not sure if I have to post the modification
> on the powerpc header to the PowerPC list or if it will be accepted here
> as well. .... But first I will have to see what you think of this patch
> :-)
You will have to submit ioctls.h modifications as a separate patch to the
powerpc-dev mailing list.
The patch looks good, a few comments below.
> Matthias
>
>
> Some Exar UARTs support a auto rs485 mode. In this mode
> the UART's RTS# pin is activated during transmitting and
> can be used to enable a rs485 line driver. This has nothing
> to do with attempts to do this by manually by asserting/
> deasserting handshake lines.
>
> Signed-off-by: Matthias Fuchs <mfuchs@xxxxxxxx>
> ---
> arch/powerpc/include/asm/ioctls.h | 2 +
> drivers/serial/8250.c | 59
> +++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 0
> deletions(-)
>
> diff --git a/arch/powerpc/include/asm/ioctls.h
> b/arch/powerpc/include/asm/ioctls.h index 279a622..1842186 100644
> --- a/arch/powerpc/include/asm/ioctls.h
> +++ b/arch/powerpc/include/asm/ioctls.h
> @@ -89,6 +89,8 @@
> #define TIOCSBRK 0x5427 /* BSD compatibility */
> #define TIOCCBRK 0x5428 /* BSD compatibility */
> #define TIOCGSID 0x5429 /* Return the session ID of FD */
> +#define TIOCGRS485 0x542e
> +#define TIOCSRS485 0x542f
> #define TIOCGPTN _IOR('T',0x30, unsigned int) /* Get Pty Number (of
> pty-mux device) */ #define TIOCSPTLCK _IOW('T',0x31, int) /* Lock/unlock
> Pty */
>
> diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
> index 303272a..e6df50f 100644
> --- a/drivers/serial/8250.c
> +++ b/drivers/serial/8250.c
> @@ -2511,6 +2511,64 @@ serial8250_type(struct uart_port *port)
> return uart_config[type].name;
> }
>
> +static int
> +serial8250_ioctl_port(struct uart_port *port, unsigned int cmd, unsigned
> long arg) +{
> + struct uart_8250_port *up = (struct uart_8250_port *)port;
> +
> + switch (cmd) {
> + case TIOCSRS485:
> + {
> + struct serial_rs485 rs485ctrl;
> + unsigned char fctr;
> +
> + if (port->type != PORT_16850)
> + return -ENODEV;
According to the ioctl manpage, -ENOTTY might be better:
"ENOTTY The specified request does not apply to the kind of object that the
descriptor d references."
> + if (copy_from_user(&rs485ctrl,
> + (struct serial_rs485 *)arg,
> + sizeof(rs485ctrl)))
> + return -EFAULT;
> +
> + serial_outp(up, UART_LCR, 0xbf);
> + fctr = serial_inp(up, UART_FCTR);
> + if (rs485ctrl.flags & SER_RS485_ENABLED)
> + fctr |= 0x08;
> + else
> + fctr &= ~0x08;
> + serial_outp(up, UART_FCTR, fctr);
> + serial_outp(up, UART_LCR, 0);
Doesn't this require locking ? serial8250_break_ctl and serial8250_set_termios
both take the port.lock spinlock to prevent races from messing up with
UART_LCR.
> + return 0;
> + }
> +
> + case TIOCGRS485:
> + {
> + struct serial_rs485 rs485ctrl;
> +
> + if (port->type != PORT_16850)
> + return -ENODEV;
What about setting rs485ctrl.flags to 0 and returning with success when RS485
auto direction control is not supported by the hardware ? I'm not sure if the
preferred semantic for get requests on unsupported features is to return an
error or return a 'feature disabled' value.
> + serial_outp(up, UART_LCR, 0xbf);
> + if (serial_inp(up, UART_FCTR) & 0x08)
> + rs485ctrl.flags = SER_RS485_ENABLED;
> + else
> + rs485ctrl.flags = 0;
> + serial_outp(up, UART_LCR, 0);
Ditto, doesn't this require locking ?
> +
> + if (copy_to_user((struct serial_rs485 *)arg,
> + &rs485ctrl,
> + sizeof(rs485ctrl)))
> + return -EFAULT;
> + return 0;
> + }
> +
> + default:
> + return -ENOIOCTLCMD;
> + }
> +
> + return 0;
> +}
> +
> static struct uart_ops serial8250_pops = {
> .tx_empty = serial8250_tx_empty,
> .set_mctrl = serial8250_set_mctrl,
> @@ -2529,6 +2587,7 @@ static struct uart_ops serial8250_pops = {
> .request_port = serial8250_request_port,
> .config_port = serial8250_config_port,
> .verify_port = serial8250_verify_port,
> + .ioctl = serial8250_ioctl_port,
> #ifdef CONFIG_CONSOLE_POLL
> .poll_get_char = serial8250_get_poll_char,
> .poll_put_char = serial8250_put_poll_char,
--
Laurent Pinchart
CSE Semaphore Belgium
Chaussee de Bruxelles, 732A
B-1410 Waterloo
Belgium
T +32 (2) 387 42 59
F +32 (2) 387 42 75
--
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
