2009-10-25 17:53:51

by Bart Hartgers

[permalink] [raw]
Subject: [PATCH 3/7] ark3116: (3rd try) Replace cmget

Signed-off-by: Bart Hartgers <[email protected]>
---
Index: linux-2.6.32-rc4/drivers/usb/serial/ark3116.c
===================================================================
--- linux-2.6.32-rc4.orig/drivers/usb/serial/ark3116.c 2009-10-18 16:20:02.000000000 +0200
+++ linux-2.6.32-rc4/drivers/usb/serial/ark3116.c 2009-10-18 16:20:13.000000000 +0200
@@ -526,32 +526,21 @@ static int ark3116_ioctl(struct tty_stru
static int ark3116_tiocmget(struct tty_struct *tty, struct file *file)
{
struct usb_serial_port *port = tty->driver_data;
- struct usb_serial *serial = port->serial;
- char *buf;
- char temp;
-
- /* seems like serial port status info (RTS, CTS, ...) is stored
- * in reg(?) 0x0006
- * pcb connection point 11 = GND -> sets bit4 of response
- * pcb connection point 7 = GND -> sets bit6 of response
- */
-
- buf = kmalloc(1, GFP_KERNEL);
- if (!buf) {
- dbg("error kmalloc");
- return -ENOMEM;
- }
-
- /* read register */
- ARK3116_RCV_QUIET(serial, 0xFE, 0xC0, 0x0000, 0x0006, buf);
- temp = buf[0];
- kfree(buf);
-
- /* i do not really know if bit4=CTS and bit6=DSR... just a
- * quick guess!
- */
- return (temp & (1<<4) ? TIOCM_CTS : 0)
- | (temp & (1<<6) ? TIOCM_DSR : 0);
+ struct ark3116_private *priv = usb_get_serial_port_data(port);
+
+ /* read modem status */
+ unsigned status = atomic_read(&priv->msr);
+ /* modem control is output */
+ unsigned ctrl = atomic_read(&priv->mcr);
+
+ return (status & UART_MSR_DSR ? TIOCM_DSR : 0) |
+ (status & UART_MSR_CTS ? TIOCM_CTS : 0) |
+ (status & UART_MSR_RI ? TIOCM_RI : 0) |
+ (status & UART_MSR_DCD ? TIOCM_CD : 0) |
+ (ctrl & UART_MCR_DTR ? TIOCM_DTR : 0) |
+ (ctrl & UART_MCR_RTS ? TIOCM_RTS : 0) |
+ (ctrl & UART_MCR_OUT1 ? TIOCM_OUT1 : 0) |
+ (ctrl & UART_MCR_OUT2 ? TIOCM_OUT2 : 0);
}

static struct usb_driver ark3116_driver = {

--


2009-10-25 19:50:04

by Oliver Neukum

[permalink] [raw]
Subject: Re: [PATCH 3/7] ark3116: (3rd try) Replace cmget

Am Sonntag, 25. Oktober 2009 18:51:00 schrieb [email protected]:
> +???????/* read modem status */
> +???????unsigned status = atomic_read(&priv->msr);
> +???????/* modem control is output */
> +???????unsigned ctrl = atomic_read(&priv->mcr);

What is the sense of having two atomic variables?
You can get races where one is changed but the other is not.

Regards
Oliver

2009-10-25 19:56:35

by Bart Hartgers

[permalink] [raw]
Subject: Re: [PATCH 3/7] ark3116: (3rd try) Replace cmget

2009/10/25 Oliver Neukum <[email protected]>:
> Am Sonntag, 25. Oktober 2009 18:51:00 schrieb [email protected]:
>> +       /* read modem status */
>> +       unsigned status = atomic_read(&priv->msr);
>> +       /* modem control is output */
>> +       unsigned ctrl = atomic_read(&priv->mcr);
>
> What is the sense of having two atomic variables?
> You can get races where one is changed but the other is not.
>
>        Regards
>                Oliver
>
>

Hi Oliver,

I don't think so. The two values are not really related.

These are two separate things: mcr (modem control reg) is ultimately
changed by whatever has the serial port open, msr (modem status
register) represents the status of the handshaking lines, and is
changed (via the interrupt urb callback) by hardware.

Groeten,
Bart

--
Bart Hartgers - New e-mail: [email protected]

2009-10-25 22:05:55

by Oliver Neukum

[permalink] [raw]
Subject: Re: [PATCH 3/7] ark3116: (3rd try) Replace cmget

Am Sonntag, 25. Oktober 2009 20:56:36 schrieb Bart Hartgers:
> I don't think so. The two values are not really related.
>
> These are two separate things: mcr (modem control reg) is ultimately
> changed by whatever has the serial port open, msr (modem status
> register) represents the status of the handshaking lines, and is
> changed (via the interrupt urb callback) by hardware.

If so, why is mcr of the type atomic_t ?

Regards
Oliver