Several drivers use the control-line and serial-state bitmap values from
the CDC spec, but there were no matching defines in the global CDC
(UAPI) header.
This series adds the missing defines and converts cdc-acm and f_acm to
use them.
One staging driver also had an unused set of CDC defines which are
removed.
The final patch by Yan Xinyu, which triggered this work, converts the
usb_wwan driver to use CDC defines instead of hardcoded values in its
line-control handling.
Greg, are you ok with me taking these through USB serial (where there
are a few more drivers that could potentially use them) or do you want
to take the lot through your tree directly?
Johan
Johan Hovold (6):
USB: cdc: add control-signal defines
USB: cdc: add serial-state defines
USB: cdc-acm: use CDC control-line defines
USB: cdc-acm: use CDC serial-state defines
staging: gdm724x: drop unused CDC defines
USB: gadget: f_acm: use CDC defines
Yan Xinyu (1):
USB: serial: usb_wwan: replace DTR/RTS magic numbers with macros
drivers/staging/gdm724x/gdm_tty.c | 6 -----
drivers/usb/class/cdc-acm.c | 42 ++++++++++++++---------------
drivers/usb/class/cdc-acm.h | 20 --------------
drivers/usb/gadget/function/f_acm.c | 20 ++++----------
drivers/usb/serial/usb_wwan.c | 10 ++++---
include/uapi/linux/usb/cdc.h | 13 +++++++++
6 files changed, 45 insertions(+), 66 deletions(-)
--
2.35.1
Use the new CDC control-line defines.
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/class/cdc-acm.c | 18 +++++++++---------
drivers/usb/class/cdc-acm.h | 7 -------
2 files changed, 9 insertions(+), 16 deletions(-)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 9b9aea24d58c..e2d80b99e074 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -658,7 +658,7 @@ static void acm_port_dtr_rts(struct tty_port *port, int raise)
int res;
if (raise)
- val = ACM_CTRL_DTR | ACM_CTRL_RTS;
+ val = USB_CDC_CTRL_DTR | USB_CDC_CTRL_RTS;
else
val = 0;
@@ -903,8 +903,8 @@ static int acm_tty_tiocmget(struct tty_struct *tty)
{
struct acm *acm = tty->driver_data;
- return (acm->ctrlout & ACM_CTRL_DTR ? TIOCM_DTR : 0) |
- (acm->ctrlout & ACM_CTRL_RTS ? TIOCM_RTS : 0) |
+ return (acm->ctrlout & USB_CDC_CTRL_DTR ? TIOCM_DTR : 0) |
+ (acm->ctrlout & USB_CDC_CTRL_RTS ? TIOCM_RTS : 0) |
(acm->ctrlin & ACM_CTRL_DSR ? TIOCM_DSR : 0) |
(acm->ctrlin & ACM_CTRL_RI ? TIOCM_RI : 0) |
(acm->ctrlin & ACM_CTRL_DCD ? TIOCM_CD : 0) |
@@ -918,10 +918,10 @@ static int acm_tty_tiocmset(struct tty_struct *tty,
unsigned int newctrl;
newctrl = acm->ctrlout;
- set = (set & TIOCM_DTR ? ACM_CTRL_DTR : 0) |
- (set & TIOCM_RTS ? ACM_CTRL_RTS : 0);
- clear = (clear & TIOCM_DTR ? ACM_CTRL_DTR : 0) |
- (clear & TIOCM_RTS ? ACM_CTRL_RTS : 0);
+ set = (set & TIOCM_DTR ? USB_CDC_CTRL_DTR : 0) |
+ (set & TIOCM_RTS ? USB_CDC_CTRL_RTS : 0);
+ clear = (clear & TIOCM_DTR ? USB_CDC_CTRL_DTR : 0) |
+ (clear & TIOCM_RTS ? USB_CDC_CTRL_RTS : 0);
newctrl = (newctrl & ~clear) | set;
@@ -1068,9 +1068,9 @@ static void acm_tty_set_termios(struct tty_struct *tty,
if (C_BAUD(tty) == B0) {
newline.dwDTERate = acm->line.dwDTERate;
- newctrl &= ~ACM_CTRL_DTR;
+ newctrl &= ~USB_CDC_CTRL_DTR;
} else if (termios_old && (termios_old->c_cflag & CBAUD) == B0) {
- newctrl |= ACM_CTRL_DTR;
+ newctrl |= USB_CDC_CTRL_DTR;
}
if (newctrl != acm->ctrlout)
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index d26ecd15be60..da7e8b8aaf28 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -22,13 +22,6 @@
#define USB_RT_ACM (USB_TYPE_CLASS | USB_RECIP_INTERFACE)
-/*
- * Output control lines.
- */
-
-#define ACM_CTRL_DTR 0x01
-#define ACM_CTRL_RTS 0x02
-
/*
* Input control lines and line errors.
*/
--
2.35.1
Add defines for the serial-state bitmap values from section 6.3.5
SerialState of the CDC specification version 1.1.
Note that the bTxCarrier and bRxCarrier bits have been named after their
RS-232 signal equivalents DSR and DCD.
Signed-off-by: Johan Hovold <[email protected]>
---
include/uapi/linux/usb/cdc.h | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/include/uapi/linux/usb/cdc.h b/include/uapi/linux/usb/cdc.h
index 372c81425cae..78caa9bdc4ae 100644
--- a/include/uapi/linux/usb/cdc.h
+++ b/include/uapi/linux/usb/cdc.h
@@ -306,6 +306,15 @@ struct usb_cdc_notification {
__le16 wLength;
} __attribute__ ((packed));
+/* UART State Bitmap Values from 6.3.5 SerialState */
+#define USB_CDC_SERIAL_STATE_DCD (1 << 0)
+#define USB_CDC_SERIAL_STATE_DSR (1 << 1)
+#define USB_CDC_SERIAL_STATE_BREAK (1 << 2)
+#define USB_CDC_SERIAL_STATE_RING_SIGNAL (1 << 3)
+#define USB_CDC_SERIAL_STATE_FRAMING (1 << 4)
+#define USB_CDC_SERIAL_STATE_PARITY (1 << 5)
+#define USB_CDC_SERIAL_STATE_OVERRUN (1 << 6)
+
struct usb_cdc_speed_change {
__le32 DLBitRRate; /* contains the downlink bit rate (IN pipe) */
__le32 ULBitRate; /* contains the uplink bit rate (OUT pipe) */
--
2.35.1
Use the new CDC serial-state defines.
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/class/cdc-acm.c | 24 ++++++++++++------------
drivers/usb/class/cdc-acm.h | 13 -------------
2 files changed, 12 insertions(+), 25 deletions(-)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index e2d80b99e074..352df48469b2 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -311,7 +311,7 @@ static void acm_process_notification(struct acm *acm, unsigned char *buf)
dev_dbg(&acm->control->dev,
"%s - serial state: 0x%x\n", __func__, newctrl);
- if (!acm->clocal && (acm->ctrlin & ~newctrl & ACM_CTRL_DCD)) {
+ if (!acm->clocal && (acm->ctrlin & ~newctrl & USB_CDC_SERIAL_STATE_DCD)) {
dev_dbg(&acm->control->dev,
"%s - calling hangup\n", __func__);
tty_port_tty_hangup(&acm->port, false);
@@ -322,25 +322,25 @@ static void acm_process_notification(struct acm *acm, unsigned char *buf)
acm->ctrlin = newctrl;
acm->oldcount = acm->iocount;
- if (difference & ACM_CTRL_DSR)
+ if (difference & USB_CDC_SERIAL_STATE_DSR)
acm->iocount.dsr++;
- if (difference & ACM_CTRL_DCD)
+ if (difference & USB_CDC_SERIAL_STATE_DCD)
acm->iocount.dcd++;
- if (newctrl & ACM_CTRL_BRK) {
+ if (newctrl & USB_CDC_SERIAL_STATE_BREAK) {
acm->iocount.brk++;
tty_insert_flip_char(&acm->port, 0, TTY_BREAK);
}
- if (newctrl & ACM_CTRL_RI)
+ if (newctrl & USB_CDC_SERIAL_STATE_RING_SIGNAL)
acm->iocount.rng++;
- if (newctrl & ACM_CTRL_FRAMING)
+ if (newctrl & USB_CDC_SERIAL_STATE_FRAMING)
acm->iocount.frame++;
- if (newctrl & ACM_CTRL_PARITY)
+ if (newctrl & USB_CDC_SERIAL_STATE_PARITY)
acm->iocount.parity++;
- if (newctrl & ACM_CTRL_OVERRUN)
+ if (newctrl & USB_CDC_SERIAL_STATE_OVERRUN)
acm->iocount.overrun++;
spin_unlock_irqrestore(&acm->read_lock, flags);
- if (newctrl & ACM_CTRL_BRK)
+ if (newctrl & USB_CDC_SERIAL_STATE_BREAK)
tty_flip_buffer_push(&acm->port);
if (difference)
@@ -905,9 +905,9 @@ static int acm_tty_tiocmget(struct tty_struct *tty)
return (acm->ctrlout & USB_CDC_CTRL_DTR ? TIOCM_DTR : 0) |
(acm->ctrlout & USB_CDC_CTRL_RTS ? TIOCM_RTS : 0) |
- (acm->ctrlin & ACM_CTRL_DSR ? TIOCM_DSR : 0) |
- (acm->ctrlin & ACM_CTRL_RI ? TIOCM_RI : 0) |
- (acm->ctrlin & ACM_CTRL_DCD ? TIOCM_CD : 0) |
+ (acm->ctrlin & USB_CDC_SERIAL_STATE_DSR ? TIOCM_DSR : 0) |
+ (acm->ctrlin & USB_CDC_SERIAL_STATE_RING_SIGNAL ? TIOCM_RI : 0) |
+ (acm->ctrlin & USB_CDC_SERIAL_STATE_DCD ? TIOCM_CD : 0) |
TIOCM_CTS;
}
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index da7e8b8aaf28..759ac15631d3 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -22,19 +22,6 @@
#define USB_RT_ACM (USB_TYPE_CLASS | USB_RECIP_INTERFACE)
-/*
- * Input control lines and line errors.
- */
-
-#define ACM_CTRL_DCD 0x01
-#define ACM_CTRL_DSR 0x02
-#define ACM_CTRL_BRK 0x04
-#define ACM_CTRL_RI 0x08
-
-#define ACM_CTRL_FRAMING 0x10
-#define ACM_CTRL_PARITY 0x20
-#define ACM_CTRL_OVERRUN 0x40
-
/*
* Internal driver structures.
*/
--
2.35.1
This driver has a copy of some of the CDC defines but which are
currently unused.
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/staging/gdm724x/gdm_tty.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/staging/gdm724x/gdm_tty.c b/drivers/staging/gdm724x/gdm_tty.c
index 04df6f9f5403..cc6d80554c98 100644
--- a/drivers/staging/gdm724x/gdm_tty.c
+++ b/drivers/staging/gdm724x/gdm_tty.c
@@ -17,12 +17,6 @@
#define GDM_TTY_MAJOR 0
#define GDM_TTY_MINOR 32
-#define ACM_CTRL_DTR 0x01
-#define ACM_CTRL_RTS 0x02
-#define ACM_CTRL_DSR 0x02
-#define ACM_CTRL_RI 0x08
-#define ACM_CTRL_DCD 0x01
-
#define WRITE_SIZE 2048
#define MUX_TX_MAX_SIZE 2048
--
2.35.1
Add defines for the Control Signal Bitmap Values from section 6.2.14
SetControlLineState of the CDC specification version 1.1.
Signed-off-by: Johan Hovold <[email protected]>
---
include/uapi/linux/usb/cdc.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/include/uapi/linux/usb/cdc.h b/include/uapi/linux/usb/cdc.h
index 6d61550959ef..372c81425cae 100644
--- a/include/uapi/linux/usb/cdc.h
+++ b/include/uapi/linux/usb/cdc.h
@@ -271,6 +271,10 @@ struct usb_cdc_line_coding {
__u8 bDataBits;
} __attribute__ ((packed));
+/* Control Signal Bitmap Values from 6.2.14 SetControlLineState */
+#define USB_CDC_CTRL_DTR (1 << 0)
+#define USB_CDC_CTRL_RTS (1 << 1)
+
/* table 62; bits in multicast filter */
#define USB_CDC_PACKET_TYPE_PROMISCUOUS (1 << 0)
#define USB_CDC_PACKET_TYPE_ALL_MULTICAST (1 << 1) /* no filter */
--
2.35.1
From: Yan Xinyu <[email protected]>
The usb_wwan_send_setup function generates DTR/RTS signals in compliance
with CDC ACM standard. This patch changes magic numbers in this function
to equivalent macros.
Signed-off-by: Yan Xinyu <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
[ johan: use the new CDC control-line defines ]
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/usb_wwan.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c
index dab38b63eaf7..6129a6e26f2c 100644
--- a/drivers/usb/serial/usb_wwan.c
+++ b/drivers/usb/serial/usb_wwan.c
@@ -29,6 +29,7 @@
#include <linux/bitops.h>
#include <linux/uaccess.h>
#include <linux/usb.h>
+#include <linux/usb/cdc.h>
#include <linux/usb/serial.h>
#include <linux/serial.h>
#include "usb-wwan.h"
@@ -48,9 +49,9 @@ static int usb_wwan_send_setup(struct usb_serial_port *port)
portdata = usb_get_serial_port_data(port);
if (portdata->dtr_state)
- val |= 0x01;
+ val |= USB_CDC_CTRL_DTR;
if (portdata->rts_state)
- val |= 0x02;
+ val |= USB_CDC_CTRL_RTS;
ifnum = serial->interface->cur_altsetting->desc.bInterfaceNumber;
@@ -59,8 +60,9 @@ static int usb_wwan_send_setup(struct usb_serial_port *port)
return res;
res = usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
- 0x22, 0x21, val, ifnum, NULL, 0,
- USB_CTRL_SET_TIMEOUT);
+ USB_CDC_REQ_SET_CONTROL_LINE_STATE,
+ USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+ val, ifnum, NULL, 0, USB_CTRL_SET_TIMEOUT);
usb_autopm_put_interface(port->serial->interface);
--
2.35.1
Use the new CDC control-line and serial-state defines.
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/gadget/function/f_acm.c | 20 +++++---------------
1 file changed, 5 insertions(+), 15 deletions(-)
diff --git a/drivers/usb/gadget/function/f_acm.c b/drivers/usb/gadget/function/f_acm.c
index 411eb489e0ff..cb523f118f04 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -57,18 +57,8 @@ struct f_acm {
/* SetControlLineState request -- CDC 1.1 section 6.2.14 (INPUT) */
u16 port_handshake_bits;
-#define ACM_CTRL_RTS (1 << 1) /* unused with full duplex */
-#define ACM_CTRL_DTR (1 << 0) /* host is ready for data r/w */
-
/* SerialState notification -- CDC 1.1 section 6.3.5 (OUTPUT) */
u16 serial_state;
-#define ACM_CTRL_OVERRUN (1 << 6)
-#define ACM_CTRL_PARITY (1 << 5)
-#define ACM_CTRL_FRAMING (1 << 4)
-#define ACM_CTRL_RI (1 << 3)
-#define ACM_CTRL_BRK (1 << 2)
-#define ACM_CTRL_DSR (1 << 1)
-#define ACM_CTRL_DCD (1 << 0)
};
static inline struct f_acm *func_to_acm(struct usb_function *f)
@@ -387,7 +377,7 @@ static int acm_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
value = 0;
/* FIXME we should not allow data to flow until the
- * host sets the ACM_CTRL_DTR bit; and when it clears
+ * host sets the USB_CDC_CTRL_DTR bit; and when it clears
* that bit, we should return to that no-flow state.
*/
acm->port_handshake_bits = w_value;
@@ -585,7 +575,7 @@ static void acm_connect(struct gserial *port)
{
struct f_acm *acm = port_to_acm(port);
- acm->serial_state |= ACM_CTRL_DSR | ACM_CTRL_DCD;
+ acm->serial_state |= USB_CDC_SERIAL_STATE_DSR | USB_CDC_SERIAL_STATE_DCD;
acm_notify_serial_state(acm);
}
@@ -593,7 +583,7 @@ static void acm_disconnect(struct gserial *port)
{
struct f_acm *acm = port_to_acm(port);
- acm->serial_state &= ~(ACM_CTRL_DSR | ACM_CTRL_DCD);
+ acm->serial_state &= ~(USB_CDC_SERIAL_STATE_DSR | USB_CDC_SERIAL_STATE_DCD);
acm_notify_serial_state(acm);
}
@@ -603,9 +593,9 @@ static int acm_send_break(struct gserial *port, int duration)
u16 state;
state = acm->serial_state;
- state &= ~ACM_CTRL_BRK;
+ state &= ~USB_CDC_SERIAL_STATE_BREAK;
if (duration)
- state |= ACM_CTRL_BRK;
+ state |= USB_CDC_SERIAL_STATE_BREAK;
acm->serial_state = state;
return acm_notify_serial_state(acm);
--
2.35.1
On Mon, Jul 25, 2022 at 10:52:17AM +0200, Greg Kroah-Hartman wrote:
> On Mon, Jul 25, 2022 at 09:58:34AM +0200, Johan Hovold wrote:
> > Several drivers use the control-line and serial-state bitmap values from
> > the CDC spec, but there were no matching defines in the global CDC
> > (UAPI) header.
> >
> > This series adds the missing defines and converts cdc-acm and f_acm to
> > use them.
> >
> > One staging driver also had an unused set of CDC defines which are
> > removed.
> >
> > The final patch by Yan Xinyu, which triggered this work, converts the
> > usb_wwan driver to use CDC defines instead of hardcoded values in its
> > line-control handling.
> >
> > Greg, are you ok with me taking these through USB serial (where there
> > are a few more drivers that could potentially use them) or do you want
> > to take the lot through your tree directly?
>
> Many thanks for this, I'll just take this now and you can send future
> patches for usb-serial that can rely on these being present.
Sounds good. That probably won't happen before the merge window anyway.
Thanks.
Johan
On Mon, Jul 25, 2022 at 09:58:34AM +0200, Johan Hovold wrote:
> Several drivers use the control-line and serial-state bitmap values from
> the CDC spec, but there were no matching defines in the global CDC
> (UAPI) header.
>
> This series adds the missing defines and converts cdc-acm and f_acm to
> use them.
>
> One staging driver also had an unused set of CDC defines which are
> removed.
>
> The final patch by Yan Xinyu, which triggered this work, converts the
> usb_wwan driver to use CDC defines instead of hardcoded values in its
> line-control handling.
>
> Greg, are you ok with me taking these through USB serial (where there
> are a few more drivers that could potentially use them) or do you want
> to take the lot through your tree directly?
Many thanks for this, I'll just take this now and you can send future
patches for usb-serial that can rely on these being present.
greg k-h
On 25.07.22 09:58, Johan Hovold wrote:
> Use the new CDC control-line defines.
>
> Signed-off-by: Johan Hovold <[email protected]>
Acked-by: Oliver Neukum <[email protected]>
On 25.07.22 09:58, Johan Hovold wrote:
> Add defines for the Control Signal Bitmap Values from section 6.2.14
> SetControlLineState of the CDC specification version 1.1.
>
> Signed-off-by: Johan Hovold <[email protected]>
Acked-by: Oliver Neukum <[email protected]>
On 25.07.22 09:58, Johan Hovold wrote:
> Add defines for the serial-state bitmap values from section 6.3.5
> SerialState of the CDC specification version 1.1.
>
> Note that the bTxCarrier and bRxCarrier bits have been named after their
> RS-232 signal equivalents DSR and DCD.
>
> Signed-off-by: Johan Hovold <[email protected]>
Acked-by: Oliver Neukum <[email protected]>
On 25.07.22 09:58, Johan Hovold wrote:
> Use the new CDC serial-state defines.
>
> Signed-off-by: Johan Hovold <[email protected]>
Acked-by: Oliver Neukum <[email protected]>
> On Jul 25, 2022, at 15:58, Johan Hovold <[email protected]> wrote:
>
> Several drivers use the control-line and serial-state bitmap values from
> the CDC spec, but there were no matching defines in the global CDC
> (UAPI) header.
>
> This series adds the missing defines and converts cdc-acm and f_acm to
> use them.
>
> One staging driver also had an unused set of CDC defines which are
> removed.
>
> The final patch by Yan Xinyu, which triggered this work, converts the
> usb_wwan driver to use CDC defines instead of hardcoded values in its
> line-control handling.
>
> Greg, are you ok with me taking these through USB serial (where there
> are a few more drivers that could potentially use them) or do you want
> to take the lot through your tree directly?
>
> Johan
>
>
> Johan Hovold (6):
> USB: cdc: add control-signal defines
> USB: cdc: add serial-state defines
> USB: cdc-acm: use CDC control-line defines
> USB: cdc-acm: use CDC serial-state defines
> staging: gdm724x: drop unused CDC defines
> USB: gadget: f_acm: use CDC defines
>
> Yan Xinyu (1):
> USB: serial: usb_wwan: replace DTR/RTS magic numbers with macros
>
> drivers/staging/gdm724x/gdm_tty.c | 6 -----
> drivers/usb/class/cdc-acm.c | 42 ++++++++++++++---------------
> drivers/usb/class/cdc-acm.h | 20 --------------
> drivers/usb/gadget/function/f_acm.c | 20 ++++----------
> drivers/usb/serial/usb_wwan.c | 10 ++++---
> include/uapi/linux/usb/cdc.h | 13 +++++++++
> 6 files changed, 45 insertions(+), 66 deletions(-)
>
> --
> 2.35.1
>
Thank you so much for spending your time to write this patch series!
And thank you and Greg for your reviews, they are very informative
and educational!
sdlyyxy