| Message ID | 20220721155257.631793-1-sdlyyxy@bupt.edu.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] USB: serial: usb_wwan: replace DTR/RTS magic numbers with macros | expand |
On Thu, Jul 21, 2022 at 11:52:57PM +0800, Yan Xinyu wrote: > 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 <sdlyyxy@bupt.edu.cn> > --- > v1->v2: > * Fix Signed-off-by name. > --- > drivers/usb/serial/usb_wwan.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c > index dab38b63eaf7..a6bd6144702d 100644 > --- a/drivers/usb/serial/usb_wwan.c > +++ b/drivers/usb/serial/usb_wwan.c > @@ -29,10 +29,14 @@ > #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" > > +#define ACM_CTRL_DTR 0x01 > +#define ACM_CTRL_RTS 0x02 Why are these not in the cdc.h file already? thanks, greg k-h
> On Jul 22, 2022, at 02:18, Greg KH <gregkh@linuxfoundation.org> wrote: > > On Thu, Jul 21, 2022 at 11:52:57PM +0800, Yan Xinyu wrote: >> 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 <sdlyyxy@bupt.edu.cn> >> --- >> v1->v2: >> * Fix Signed-off-by name. >> --- >> drivers/usb/serial/usb_wwan.c | 13 +++++++++---- >> 1 file changed, 9 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c >> index dab38b63eaf7..a6bd6144702d 100644 >> --- a/drivers/usb/serial/usb_wwan.c >> +++ b/drivers/usb/serial/usb_wwan.c >> @@ -29,10 +29,14 @@ >> #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" >> >> +#define ACM_CTRL_DTR 0x01 >> +#define ACM_CTRL_RTS 0x02 > > Why are these not in the cdc.h file already? These are defined in the drivers/usb/class/cdc-acm.h file. Is it safe to include it? Thanks, sdlyyxy
On Fri, Jul 22, 2022 at 10:40:51AM +0800, Yan Xinyu wrote: > > On Jul 22, 2022, at 02:18, Greg KH <gregkh@linuxfoundation.org> wrote: > > > > On Thu, Jul 21, 2022 at 11:52:57PM +0800, Yan Xinyu wrote: > >> 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 <sdlyyxy@bupt.edu.cn> > >> --- > >> v1->v2: > >> * Fix Signed-off-by name. > >> --- > >> drivers/usb/serial/usb_wwan.c | 13 +++++++++---- > >> 1 file changed, 9 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c > >> index dab38b63eaf7..a6bd6144702d 100644 > >> --- a/drivers/usb/serial/usb_wwan.c > >> +++ b/drivers/usb/serial/usb_wwan.c > >> @@ -29,10 +29,14 @@ > >> #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" > >> > >> +#define ACM_CTRL_DTR 0x01 > >> +#define ACM_CTRL_RTS 0x02 > > > > Why are these not in the cdc.h file already? > > These are defined in the drivers/usb/class/cdc-acm.h file. Is it safe > to include it? Yes, of course!
diff --git a/drivers/usb/serial/usb_wwan.c b/drivers/usb/serial/usb_wwan.c index dab38b63eaf7..a6bd6144702d 100644 --- a/drivers/usb/serial/usb_wwan.c +++ b/drivers/usb/serial/usb_wwan.c @@ -29,10 +29,14 @@ #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" +#define ACM_CTRL_DTR 0x01 +#define ACM_CTRL_RTS 0x02 + /* * Generate DTR/RTS signals on the port using the SET_CONTROL_LINE_STATE request * in CDC ACM. @@ -48,9 +52,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 |= ACM_CTRL_DTR; if (portdata->rts_state) - val |= 0x02; + val |= ACM_CTRL_RTS; ifnum = serial->interface->cur_altsetting->desc.bInterfaceNumber; @@ -59,8 +63,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);
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 <sdlyyxy@bupt.edu.cn> --- v1->v2: * Fix Signed-off-by name. --- drivers/usb/serial/usb_wwan.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)