Re: [PATCH 00/15] usb: serial: avoid using usb_control_msg() directly
On Fri, Dec 4, 2020 at 2:38 PM Johan Hovold wrote: > > Hi Himadri, > > and sorry about the late feedback on this one. I'm still trying to dig > myself out of some backlog. > > On Wed, Nov 04, 2020 at 12:16:48PM +0530, Himadri Pandya wrote: > > There are many usages of usb_control_msg() that can use the new wrapper > > functions usb_contro_msg_send() & usb_control_msg_recv() for better > > error checks on short reads and writes. Hence use them whenever possible > > and avoid using usb_control_msg() directly. > > Replacing working code with shiny new helpers unfortunately often ends > up introducing new bugs and I'm afraid there are a few examples of that > in this series as well. > > I'll comment on the patches individually, but my impression is that we > should primarily use these helpers to replace code which allocates a > temporary buffer for each transfer as otherwise there's no clear gain > from using them. > > Some of your patches contains unrelated changes which needs to go in > separate patches if they're to be included at all. > > Please also try to write dedicated commit messages rater than reusing > more or less the same stock message since not everything in these > messages apply to each patch. You never mention that these helpers > nicely hides the allocation of temporary transfer buffers in some cases > for examples. In other places they instead introduce additional > allocations which at least should have been highlighted. > > > Himadri Pandya (15): > > usb: serial: ark3116: use usb_control_msg_recv() and > > usb_control_msg_send() > > Nit: please also use an uppercase "USB" prefix. Hi Johan, Thanks for reviewing this series and sorry for the late reply. I'll soon send a v2 according to your comments. Best regards, Himadri > > > usb: serial: belkin_sa: use usb_control_msg_send() > > usb: serial: ch314: use usb_control_msg_recv() and > > usb_control_msg_send() > > usb: serial: cp210x: use usb_control_msg_recv() and > > usb_control_msg_send() > > usb: serial: cypress_m8: use usb_control_msg_recv() and > > usb_control_msg_send() > > usb: serial: f81232: use usb_control_msg_recv() and > > usb_control_msg_send() > > usb: serial: f81534: use usb_control_msg_recv() and > > usb_control_msg_send() > > usb: serial: ftdi_sio: use usb_control_msg_recv() and > > usb_control_msg_send() > > usb: serial: io_edgeport: use usb_control_msg_recv() and > > usb_control_msg_send() > > usb: serial: io_ti: use usb_control_msg_recv() and > > usb_control_msg_send() > > usb: serial: ipaq: use usb_control_msg_send() > > usb: serial: ipw: use usb_control_msg_send() > > usb: serial: iuu_phoenix: use usb_control_msg_send() > > usb: serial: keyspan_pda: use usb_control_msg_recv() and > > usb_control_msg_send() > > usb: serial: kl5kusb105: use usb_control_msg_recv() and > > usb_control_msg_send() > > > > drivers/usb/serial/ark3116.c | 29 + > > drivers/usb/serial/belkin_sa.c | 35 +++--- > > drivers/usb/serial/ch341.c | 45 +++- > > drivers/usb/serial/cp210x.c | 148 +++-- > > drivers/usb/serial/cypress_m8.c | 38 --- > > drivers/usb/serial/f81232.c | 88 +++ > > drivers/usb/serial/f81534.c | 63 +++ > > drivers/usb/serial/ftdi_sio.c| 182 +-- > > drivers/usb/serial/io_edgeport.c | 73 + > > drivers/usb/serial/io_ti.c | 28 ++--- > > drivers/usb/serial/ipaq.c| 9 +- > > drivers/usb/serial/ipw.c | 107 ++ > > drivers/usb/serial/iuu_phoenix.c | 5 +- > > drivers/usb/serial/keyspan_pda.c | 172 - > > drivers/usb/serial/kl5kusb105.c | 94 > > 15 files changed, 406 insertions(+), 710 deletions(-) > > Johan
[PATCH 11/15] usb: serial: ipaq: use usb_control_msg_send()
The new usb_control_msg_send() nicely wraps usb_control_msg() with proper
error check. Hence use the wrapper instead of calling usb_control_msg()
directly.
Signed-off-by: Himadri Pandya
---
drivers/usb/serial/ipaq.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/serial/ipaq.c b/drivers/usb/serial/ipaq.c
index f81746c3c26c..99505a76035d 100644
--- a/drivers/usb/serial/ipaq.c
+++ b/drivers/usb/serial/ipaq.c
@@ -530,15 +530,14 @@ static int ipaq_open(struct tty_struct *tty,
*/
while (retries) {
retries--;
- result = usb_control_msg(serial->dev,
- usb_sndctrlpipe(serial->dev, 0), 0x22, 0x21,
- 0x1, 0, NULL, 0, 100);
- if (!result)
+ result = usb_control_msg_send(serial->dev, 0, 0x22, 0x21, 0x1,
+ 0, NULL, 0, 100, GFP_KERNEL);
+ if (result == 0)
break;
msleep(1000);
}
- if (!retries && result) {
+ if (result) {
dev_err(>dev, "%s - failed doing control urb, error %d\n",
__func__, result);
return result;
--
2.17.1
[PATCH 07/15] usb: serial: f81534: use usb_control_msg_recv() and usb_control_msg_send()
The new usb_control_msg_recv() and usb_control_msg_send() nicely wraps
usb_control_msg() with proper error check. Hence use the wrappers
instead of calling usb_control_msg() directly.
Signed-off-by: Himadri Pandya
---
drivers/usb/serial/f81534.c | 63 +++--
1 file changed, 18 insertions(+), 45 deletions(-)
diff --git a/drivers/usb/serial/f81534.c b/drivers/usb/serial/f81534.c
index 5661fd03e545..23eb17a2c052 100644
--- a/drivers/usb/serial/f81534.c
+++ b/drivers/usb/serial/f81534.c
@@ -217,38 +217,26 @@ static int f81534_set_register(struct usb_serial *serial,
u16 reg, u8 data)
struct usb_device *dev = serial->dev;
size_t count = F81534_USB_MAX_RETRY;
int status;
- u8 *tmp;
-
- tmp = kmalloc(sizeof(u8), GFP_KERNEL);
- if (!tmp)
- return -ENOMEM;
-
- *tmp = data;
/*
* Our device maybe not reply when heavily loading, We'll retry for
* F81534_USB_MAX_RETRY times.
*/
while (count--) {
- status = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
-F81534_SET_GET_REGISTER,
-USB_TYPE_VENDOR | USB_DIR_OUT,
-reg, 0, tmp, sizeof(u8),
-F81534_USB_TIMEOUT);
- if (status > 0) {
- status = 0;
- break;
- } else if (status == 0) {
- status = -EIO;
+ status = usb_control_msg_send(dev, 0, F81534_SET_GET_REGISTER,
+ USB_TYPE_VENDOR | USB_DIR_OUT,
+ reg, 0, , sizeof(u8),
+ F81534_USB_TIMEOUT, GFP_KERNEL);
+ if (status) {
+ /* Try again */
+ continue;
}
}
- if (status < 0) {
+ if (status)
dev_err(>dev, "%s: reg: %x data: %x failed: %d\n",
- __func__, reg, data, status);
- }
+ __func__, reg, data, status);
- kfree(tmp);
return status;
}
@@ -258,40 +246,25 @@ static int f81534_get_register(struct usb_serial *serial,
u16 reg, u8 *data)
struct usb_device *dev = serial->dev;
size_t count = F81534_USB_MAX_RETRY;
int status;
- u8 *tmp;
-
- tmp = kmalloc(sizeof(u8), GFP_KERNEL);
- if (!tmp)
- return -ENOMEM;
/*
* Our device maybe not reply when heavily loading, We'll retry for
* F81534_USB_MAX_RETRY times.
*/
while (count--) {
- status = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
-F81534_SET_GET_REGISTER,
-USB_TYPE_VENDOR | USB_DIR_IN,
-reg, 0, tmp, sizeof(u8),
-F81534_USB_TIMEOUT);
- if (status > 0) {
- status = 0;
- break;
- } else if (status == 0) {
- status = -EIO;
+ status = usb_control_msg_recv(dev, 0, F81534_SET_GET_REGISTER,
+ USB_TYPE_VENDOR | USB_DIR_IN, reg,
+ 0, data, sizeof(u8),
+ F81534_USB_TIMEOUT, GFP_KERNEL);
+ if (status) {
+ /* Try again */
+ continue;
}
}
- if (status < 0) {
+ if (status)
dev_err(>dev, "%s: reg: %x failed: %d\n", __func__,
- reg, status);
- goto end;
- }
-
- *data = *tmp;
-
-end:
- kfree(tmp);
+ reg, status);
return status;
}
--
2.17.1
[PATCH 08/15] usb: serial: ftdi_sio: use usb_control_msg_recv() and usb_control_msg_send()
The new usb_control_msg_recv() and usb_control_msg_send() nicely wraps
usb_control_msg() with proper error check. Hence use the wrappers
instead of calling usb_control_msg() directly.
Signed-off-by: Himadri Pandya
---
drivers/usb/serial/ftdi_sio.c | 182 +++---
1 file changed, 78 insertions(+), 104 deletions(-)
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index e0f4c3d9649c..37e9e75b90d0 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -1245,13 +1245,12 @@ static int update_mctrl(struct usb_serial_port *port,
unsigned int set,
value |= FTDI_SIO_SET_DTR_HIGH;
if (set & TIOCM_RTS)
value |= FTDI_SIO_SET_RTS_HIGH;
- rv = usb_control_msg(port->serial->dev,
- usb_sndctrlpipe(port->serial->dev, 0),
- FTDI_SIO_SET_MODEM_CTRL_REQUEST,
- FTDI_SIO_SET_MODEM_CTRL_REQUEST_TYPE,
- value, priv->interface,
- NULL, 0, WDR_TIMEOUT);
- if (rv < 0) {
+ rv = usb_control_msg_send(port->serial->dev, 0,
+ FTDI_SIO_SET_MODEM_CTRL_REQUEST,
+ FTDI_SIO_SET_MODEM_CTRL_REQUEST_TYPE,
+ value, priv->interface,
+ NULL, 0, WDR_TIMEOUT, GFP_KERNEL);
+ if (rv) {
dev_dbg(dev, "%s Error from MODEM_CTRL urb: DTR %s, RTS %s\n",
__func__,
(set & TIOCM_DTR) ? "HIGH" : (clear & TIOCM_DTR) ?
"LOW" : "unchanged",
@@ -1393,12 +1392,11 @@ static int change_speed(struct tty_struct *tty, struct
usb_serial_port *port)
index = (u16)((index << 8) | priv->interface);
}
- rv = usb_control_msg(port->serial->dev,
- usb_sndctrlpipe(port->serial->dev, 0),
- FTDI_SIO_SET_BAUDRATE_REQUEST,
- FTDI_SIO_SET_BAUDRATE_REQUEST_TYPE,
- value, index,
- NULL, 0, WDR_SHORT_TIMEOUT);
+ rv = usb_control_msg_send(port->serial->dev, 0,
+ FTDI_SIO_SET_BAUDRATE_REQUEST,
+ FTDI_SIO_SET_BAUDRATE_REQUEST_TYPE,
+ value, index,
+ NULL, 0, WDR_SHORT_TIMEOUT, GFP_KERNEL);
return rv;
}
@@ -1417,13 +1415,12 @@ static int write_latency_timer(struct usb_serial_port
*port)
dev_dbg(>dev, "%s: setting latency timer = %i\n", __func__, l);
- rv = usb_control_msg(udev,
-usb_sndctrlpipe(udev, 0),
-FTDI_SIO_SET_LATENCY_TIMER_REQUEST,
-FTDI_SIO_SET_LATENCY_TIMER_REQUEST_TYPE,
-l, priv->interface,
-NULL, 0, WDR_TIMEOUT);
- if (rv < 0)
+ rv = usb_control_msg_send(udev, 0,
+ FTDI_SIO_SET_LATENCY_TIMER_REQUEST,
+ FTDI_SIO_SET_LATENCY_TIMER_REQUEST_TYPE,
+ l, priv->interface,
+ NULL, 0, WDR_TIMEOUT, GFP_KERNEL);
+ if (rv)
dev_err(>dev, "Unable to write latency timer: %i\n", rv);
return rv;
}
@@ -1432,27 +1429,16 @@ static int _read_latency_timer(struct usb_serial_port
*port)
{
struct ftdi_private *priv = usb_get_serial_port_data(port);
struct usb_device *udev = port->serial->dev;
- unsigned char *buf;
+ u8 buf;
int rv;
- buf = kmalloc(1, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
-
- rv = usb_control_msg(udev,
-usb_rcvctrlpipe(udev, 0),
-FTDI_SIO_GET_LATENCY_TIMER_REQUEST,
-FTDI_SIO_GET_LATENCY_TIMER_REQUEST_TYPE,
-0, priv->interface,
-buf, 1, WDR_TIMEOUT);
- if (rv < 1) {
- if (rv >= 0)
- rv = -EIO;
- } else {
- rv = buf[0];
- }
-
- kfree(buf);
+ rv = usb_control_msg_recv(udev, 0,
+ FTDI_SIO_GET_LATENCY_TIMER_REQUEST,
+ FTDI_SIO_GET_LATENCY_TIMER_REQUEST_TYPE,
+ 0, priv->interface,
+ , 1, WDR_TIMEOUT, GFP_KERNEL);
+ if (rv == 0)
+ rv = buf;
return rv;
}
@@ -1731,13 +1717,12 @@ static ssize_t event_char_store(struct device *dev,
[PATCH 06/15] usb: serial: f81232: use usb_control_msg_recv() and usb_control_msg_send()
The new usb_control_msg_recv() and usb_control_msg_send() nicely wraps
usb_control_msg() with proper error check. Hence use the wrappers
instead of calling usb_control_msg() directly.
Signed-off-by: Himadri Pandya
---
drivers/usb/serial/f81232.c | 88 -
1 file changed, 18 insertions(+), 70 deletions(-)
diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 0c7eacc630e0..78107feef8a8 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -139,71 +139,36 @@ static int calc_baud_divisor(speed_t baudrate, speed_t
clockrate)
static int f81232_get_register(struct usb_serial_port *port, u16 reg, u8 *val)
{
int status;
- u8 *tmp;
struct usb_device *dev = port->serial->dev;
- tmp = kmalloc(sizeof(*val), GFP_KERNEL);
- if (!tmp)
- return -ENOMEM;
-
- status = usb_control_msg(dev,
- usb_rcvctrlpipe(dev, 0),
- F81232_REGISTER_REQUEST,
- F81232_GET_REGISTER,
- reg,
- 0,
- tmp,
- sizeof(*val),
- USB_CTRL_GET_TIMEOUT);
- if (status != sizeof(*val)) {
+ status = usb_control_msg_recv(dev, 0, F81232_REGISTER_REQUEST,
+ F81232_GET_REGISTER, reg, 0, val,
+ sizeof(*val), USB_CTRL_GET_TIMEOUT,
+ GFP_KERNEL);
+ if (status) {
dev_err(>dev, "%s failed status: %d\n", __func__, status);
- if (status < 0)
- status = usb_translate_errors(status);
- else
- status = -EIO;
- } else {
- status = 0;
- *val = *tmp;
+ status = usb_translate_errors(status);
}
- kfree(tmp);
return status;
}
static int f81232_set_register(struct usb_serial_port *port, u16 reg, u8 val)
{
int status;
- u8 *tmp;
struct usb_device *dev = port->serial->dev;
- tmp = kmalloc(sizeof(val), GFP_KERNEL);
- if (!tmp)
- return -ENOMEM;
-
- *tmp = val;
-
- status = usb_control_msg(dev,
- usb_sndctrlpipe(dev, 0),
- F81232_REGISTER_REQUEST,
- F81232_SET_REGISTER,
- reg,
- 0,
- tmp,
- sizeof(val),
- USB_CTRL_SET_TIMEOUT);
- if (status != sizeof(val)) {
+ status = usb_control_msg_send(dev, 0,
+ F81232_REGISTER_REQUEST,
+ F81232_SET_REGISTER, reg, 0,
+ , sizeof(val), USB_CTRL_SET_TIMEOUT,
+ GFP_KERNEL);
+ if (status) {
dev_err(>dev, "%s failed status: %d\n", __func__, status);
- if (status < 0)
- status = usb_translate_errors(status);
- else
- status = -EIO;
- } else {
- status = 0;
+ status = usb_translate_errors(status);
}
-
- kfree(tmp);
return status;
}
@@ -866,32 +831,16 @@ static int f81534a_ctrl_set_register(struct usb_interface
*intf, u16 reg,
struct usb_device *dev = interface_to_usbdev(intf);
int retry = F81534A_ACCESS_REG_RETRY;
int status;
- u8 *tmp;
-
- tmp = kmemdup(val, size, GFP_KERNEL);
- if (!tmp)
- return -ENOMEM;
while (retry--) {
- status = usb_control_msg(dev,
- usb_sndctrlpipe(dev, 0),
- F81232_REGISTER_REQUEST,
- F81232_SET_REGISTER,
- reg,
- 0,
- tmp,
- size,
- USB_CTRL_SET_TIMEOUT);
- if (status < 0) {
+ status = usb_control_msg_send(dev, 0, F81232_REGISTER_REQUEST,
+ F81232_SET_REGISTER, reg, 0, val,
+ size, USB_CTRL_SET_TIMEOUT,
GFP_KERNEL);
+ if (status) {
+ /* Retry on error or short transfers */
status = usb_translate_errors(status);
if (status == -EIO)
continue;
- } else if (status != size) {
-
[PATCH 13/15] usb: serial: iuu_phoenix: use usb_control_msg_send()
The new usb_control_msg_send() nicely wraps usb_control_msg() with proper
error check. Hence use the wrapper instead of calling usb_control_msg()
directly.
Signed-off-by: Himadri Pandya
---
drivers/usb/serial/iuu_phoenix.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/serial/iuu_phoenix.c b/drivers/usb/serial/iuu_phoenix.c
index b4ba79123d9d..dfbcdcec94e7 100644
--- a/drivers/usb/serial/iuu_phoenix.c
+++ b/drivers/usb/serial/iuu_phoenix.c
@@ -968,9 +968,8 @@ static int iuu_open(struct tty_struct *tty, struct
usb_serial_port *port)
priv->poll = 0;
#define SOUP(a, b, c, d) do { \
- result = usb_control_msg(port->serial->dev, \
- usb_sndctrlpipe(port->serial->dev, 0), \
- b, a, c, d, NULL, 0, 1000); \
+ result = usb_control_msg_send(port->serial->dev, 0, b, a, c, d, NULL,\
+ 0, 1000, GFP_KERNEL); \
dev_dbg(dev, "0x%x:0x%x:0x%x:0x%x %d\n", a, b, c, d, result); } while
(0)
/* This is not UART related but IUU USB driver related or something */
--
2.17.1
[PATCH 15/15] usb: serial: kl5kusb105: use usb_control_msg_recv() and usb_control_msg_send()
The new usb_control_msg_recv() and usb_control_msg_send() nicely wraps
usb_control_msg() with proper error check. Hence use the wrappers
instead of calling usb_control_msg() directly
Signed-off-by: Himadri Pandya
---
drivers/usb/serial/kl5kusb105.c | 94 +++--
1 file changed, 44 insertions(+), 50 deletions(-)
diff --git a/drivers/usb/serial/kl5kusb105.c b/drivers/usb/serial/kl5kusb105.c
index 5ee48b0650c4..75cfd1c907f3 100644
--- a/drivers/usb/serial/kl5kusb105.c
+++ b/drivers/usb/serial/kl5kusb105.c
@@ -124,16 +124,17 @@ static int klsi_105_chg_port_settings(struct
usb_serial_port *port,
{
int rc;
- rc = usb_control_msg(port->serial->dev,
- usb_sndctrlpipe(port->serial->dev, 0),
- KL5KUSB105A_SIO_SET_DATA,
- USB_TYPE_VENDOR | USB_DIR_OUT | USB_RECIP_INTERFACE,
- 0, /* value */
- 0, /* index */
- settings,
- sizeof(struct klsi_105_port_settings),
- KLSI_TIMEOUT);
- if (rc < 0)
+ rc = usb_control_msg_send(port->serial->dev, 0,
+ KL5KUSB105A_SIO_SET_DATA,
+ USB_TYPE_VENDOR | USB_DIR_OUT |
+ USB_RECIP_INTERFACE,
+ 0, /* value */
+ 0, /* index */
+ settings,
+ sizeof(struct klsi_105_port_settings),
+ KLSI_TIMEOUT,
+ GFP_KERNEL);
+ if (rc)
dev_err(>dev,
"Change port settings failed (error = %d)\n", rc);
@@ -167,28 +168,21 @@ static int klsi_105_get_line_state(struct usb_serial_port
*port,
unsigned long *line_state_p)
{
int rc;
- u8 *status_buf;
+ u8 status_buf[2];
__u16 status;
- status_buf = kmalloc(KLSI_STATUSBUF_LEN, GFP_KERNEL);
- if (!status_buf)
- return -ENOMEM;
-
status_buf[0] = 0xff;
status_buf[1] = 0xff;
- rc = usb_control_msg(port->serial->dev,
-usb_rcvctrlpipe(port->serial->dev, 0),
-KL5KUSB105A_SIO_POLL,
-USB_TYPE_VENDOR | USB_DIR_IN,
-0, /* value */
-0, /* index */
-status_buf, KLSI_STATUSBUF_LEN,
-1
-);
- if (rc != KLSI_STATUSBUF_LEN) {
+ rc = usb_control_msg_recv(port->serial->dev, 0,
+ KL5KUSB105A_SIO_POLL,
+ USB_TYPE_VENDOR | USB_DIR_IN,
+ 0, /* value */
+ 0, /* index */
+ _buf, KLSI_STATUSBUF_LEN,
+ 1,
+ GFP_KERNEL);
+ if (rc) {
dev_err(>dev, "reading line status failed: %d\n", rc);
- if (rc >= 0)
- rc = -EIO;
} else {
status = get_unaligned_le16(status_buf);
@@ -198,7 +192,6 @@ static int klsi_105_get_line_state(struct usb_serial_port
*port,
*line_state_p = klsi_105_status2linestate(status);
}
- kfree(status_buf);
return rc;
}
@@ -283,16 +276,17 @@ static int klsi_105_open(struct tty_struct *tty, struct
usb_serial_port *port)
goto err_free_cfg;
}
- rc = usb_control_msg(port->serial->dev,
-usb_sndctrlpipe(port->serial->dev, 0),
-KL5KUSB105A_SIO_CONFIGURE,
-USB_TYPE_VENDOR|USB_DIR_OUT|USB_RECIP_INTERFACE,
-KL5KUSB105A_SIO_CONFIGURE_READ_ON,
-0, /* index */
-NULL,
-0,
-KLSI_TIMEOUT);
- if (rc < 0) {
+ rc = usb_control_msg_send(port->serial->dev, 0,
+ KL5KUSB105A_SIO_CONFIGURE,
+ USB_TYPE_VENDOR | USB_DIR_OUT |
+ USB_RECIP_INTERFACE,
+ KL5KUSB105A_SIO_CONFIGURE_READ_ON,
+ 0, /* index */
+ NULL,
+ 0,
+ KLSI_TIMEOUT,
+ GFP_KERNEL);
+ if (rc) {
dev_err(>dev, "Enabling read failed (error = %d)\n", rc);
retval = rc;
[PATCH 10/15] usb: serial: io_ti: use usb_control_msg_recv() and usb_control_msg_send()
The new usb_control_msg_recv() and usb_control_msg_send() nicely wraps
usb_control_msg() with proper error check. Hence use the wrappers
instead of calling usb_control_msg() directly
Signed-off-by: Himadri Pandya
---
drivers/usb/serial/io_ti.c | 28 ++--
1 file changed, 10 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/serial/io_ti.c b/drivers/usb/serial/io_ti.c
index c327d4cf7928..ab2370b80b3c 100644
--- a/drivers/usb/serial/io_ti.c
+++ b/drivers/usb/serial/io_ti.c
@@ -260,16 +260,12 @@ static int ti_vread_sync(struct usb_device *dev, __u8
request,
{
int status;
- status = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0), request,
- (USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN),
- value, index, data, size, 1000);
- if (status < 0)
+ status = usb_control_msg_recv(dev, 0, request, USB_TYPE_VENDOR |
+ USB_RECIP_DEVICE | USB_DIR_IN, value,
+ index, data, size, 1000, GFP_KERNEL);
+ if (status)
return status;
- if (status != size) {
- dev_dbg(>dev, "%s - wanted to write %d, but only wrote
%d\n",
- __func__, size, status);
- return -ECOMM;
- }
+
return 0;
}
@@ -278,16 +274,12 @@ static int ti_vsend_sync(struct usb_device *dev, u8
request, u16 value,
{
int status;
- status = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), request,
- (USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT),
- value, index, data, size, timeout);
- if (status < 0)
+ status = usb_control_msg_send(dev, 0, request, USB_TYPE_VENDOR |
+ USB_RECIP_DEVICE | USB_DIR_OUT, value,
+ index, data, size, timeout, GFP_KERNEL);
+ if (status)
return status;
- if (status != size) {
- dev_dbg(>dev, "%s - wanted to write %d, but only wrote
%d\n",
- __func__, size, status);
- return -ECOMM;
- }
+
return 0;
}
--
2.17.1
[PATCH 12/15] usb: serial: ipw: use usb_control_msg_send()
The new usb_control_msg_send() nicely wraps usb_control_msg() with proper error check. Hence use the wrapper instead of calling usb_control_msg() directly. Signed-off-by: Himadri Pandya --- drivers/usb/serial/ipw.c | 107 +-- 1 file changed, 36 insertions(+), 71 deletions(-) diff --git a/drivers/usb/serial/ipw.c b/drivers/usb/serial/ipw.c index d04c7cc5c1c2..3c3895b4dff8 100644 --- a/drivers/usb/serial/ipw.c +++ b/drivers/usb/serial/ipw.c @@ -134,26 +134,16 @@ static int ipw_open(struct tty_struct *tty, struct usb_serial_port *port) struct usb_device *udev = port->serial->dev; struct device *dev = >dev; u8 buf_flow_static[16] = IPW_BYTES_FLOWINIT; - u8 *buf_flow_init; int result; - buf_flow_init = kmemdup(buf_flow_static, 16, GFP_KERNEL); - if (!buf_flow_init) - return -ENOMEM; - /* --1: Tell the modem to initialize (we think) From sniffs this is * always the first thing that gets sent to the modem during * opening of the device */ dev_dbg(dev, "%s: Sending SIO_INIT (we guess)\n", __func__); - result = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), -IPW_SIO_INIT, -USB_TYPE_VENDOR | USB_RECIP_INTERFACE | USB_DIR_OUT, -0, -0, /* index */ -NULL, -0, -10); - if (result < 0) + result = usb_control_msg_send(udev, 0, IPW_SIO_INIT, USB_TYPE_VENDOR | + USB_RECIP_INTERFACE | USB_DIR_OUT, 0, + 0, NULL, 0, 10, GFP_KERNEL); + if (result) dev_err(dev, "Init of modem failed (error = %d)\n", result); /* reset the bulk pipes */ @@ -166,31 +156,22 @@ static int ipw_open(struct tty_struct *tty, struct usb_serial_port *port) /*--3: Tell the modem to open the floodgates on the rx bulk channel */ dev_dbg(dev, "%s:asking modem for RxRead (RXBULK_ON)\n", __func__); - result = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), -IPW_SIO_RXCTL, -USB_TYPE_VENDOR | USB_RECIP_INTERFACE | USB_DIR_OUT, -IPW_RXBULK_ON, -0, /* index */ -NULL, -0, -10); - if (result < 0) + result = usb_control_msg_send(udev, 0, IPW_SIO_RXCTL, USB_TYPE_VENDOR | + USB_RECIP_INTERFACE | USB_DIR_OUT, + IPW_RXBULK_ON, 0, NULL, 0, 10, + GFP_KERNEL); + if (result) dev_err(dev, "Enabling bulk RxRead failed (error = %d)\n", result); /*--4: setup the initial flowcontrol */ - dev_dbg(dev, "%s:setting init flowcontrol (%s)\n", __func__, buf_flow_init); - result = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), -IPW_SIO_HANDFLOW, -USB_TYPE_VENDOR | USB_RECIP_INTERFACE | USB_DIR_OUT, -0, -0, -buf_flow_init, -0x10, -20); - if (result < 0) + dev_dbg(dev, "%s:setting init flowcontrol (%s)\n", __func__, buf_flow_static); + result = usb_control_msg_send(udev, 0, IPW_SIO_HANDFLOW, + USB_TYPE_VENDOR | USB_RECIP_INTERFACE | + USB_DIR_OUT, 0, 0, _flow_static, 0x10, + 20, GFP_KERNEL); + if (result) dev_err(dev, "initial flowcontrol failed (error = %d)\n", result); - kfree(buf_flow_init); return 0; } @@ -223,26 +204,20 @@ static void ipw_dtr_rts(struct usb_serial_port *port, int on) dev_dbg(dev, "%s: on = %d\n", __func__, on); - result = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), -IPW_SIO_SET_PIN, -USB_TYPE_VENDOR | USB_RECIP_INTERFACE | USB_DIR_OUT, -on ? IPW_PIN_SETDTR : IPW_PIN_CLRDTR, -0, -NULL, -0, -20); - if (result < 0) + result = usb_control_msg_send(udev, 0, IPW_SIO_SET_PIN, + USB_TYPE_VENDOR | USB_RECIP_INTERFACE | + USB_DIR_OUT, + on ? IPW_PIN_SETDTR : IPW_PIN_CLRDTR, 0, + NULL, 0, 20, GFP_KERNEL); + if (result) dev_err(dev, "set
[PATCH 14/15] usb: serial: keyspan_pda: use usb_control_msg_recv() and usb_control_msg_send()
The new usb_control_msg_recv() and usb_control_msg_send() nicely wraps
usb_control_msg() with proper error check. Hence use the wrappers
instead of calling usb_control_msg() directly.
Signed-off-by: Himadri Pandya
---
drivers/usb/serial/keyspan_pda.c | 172 +--
1 file changed, 72 insertions(+), 100 deletions(-)
diff --git a/drivers/usb/serial/keyspan_pda.c b/drivers/usb/serial/keyspan_pda.c
index c1333919716b..44e1c4490fa9 100644
--- a/drivers/usb/serial/keyspan_pda.c
+++ b/drivers/usb/serial/keyspan_pda.c
@@ -115,17 +115,17 @@ static void keyspan_pda_request_unthrottle(struct
work_struct *work)
/* ask the device to tell us when the tx buffer becomes
sufficiently empty */
- result = usb_control_msg(serial->dev,
-usb_sndctrlpipe(serial->dev, 0),
-7, /* request_unthrottle */
-USB_TYPE_VENDOR | USB_RECIP_INTERFACE
-| USB_DIR_OUT,
-16, /* value: threshold */
-0, /* index */
-NULL,
-0,
-2000);
- if (result < 0)
+ result = usb_control_msg_send(serial->dev, 0,
+ 7, /* request_unthrottle */
+ USB_TYPE_VENDOR | USB_RECIP_INTERFACE
+ | USB_DIR_OUT,
+ 16, /* value: threshold */
+ 0, /* index */
+ NULL,
+ 0,
+ 2000,
+ GFP_KERNEL);
+ if (result)
dev_dbg(>dev->dev, "%s - error %d from
usb_control_msg\n",
__func__, result);
}
@@ -269,17 +269,18 @@ static speed_t keyspan_pda_setbaud(struct usb_serial
*serial, speed_t baud)
/* rather than figure out how to sleep while waiting for this
to complete, I just use the "legacy" API. */
- rc = usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
-0, /* set baud */
-USB_TYPE_VENDOR
-| USB_RECIP_INTERFACE
-| USB_DIR_OUT, /* type */
-bindex, /* value */
-0, /* index */
-NULL, /* */
-0, /* size */
-2000); /* timeout */
- if (rc < 0)
+ rc = usb_control_msg_send(serial->dev, 0,
+ 0, /* set baud */
+ USB_TYPE_VENDOR
+ | USB_RECIP_INTERFACE
+ | USB_DIR_OUT, /* type */
+ bindex, /* value */
+ 0, /* index */
+ NULL, /* */
+ 0, /* size */
+ 2000, /* timeout */
+ GFP_KERNEL);
+ if (rc)
return 0;
return baud;
}
@@ -296,11 +297,12 @@ static void keyspan_pda_break_ctl(struct tty_struct *tty,
int break_state)
value = 1; /* start break */
else
value = 0; /* clear break */
- result = usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
- 4, /* set break */
- USB_TYPE_VENDOR | USB_RECIP_INTERFACE | USB_DIR_OUT,
- value, 0, NULL, 0, 2000);
- if (result < 0)
+ result = usb_control_msg_send(serial->dev, 0,
+ 4, /* set break */
+ USB_TYPE_VENDOR | USB_RECIP_INTERFACE |
+ USB_DIR_OUT,
+ value, 0, NULL, 0, 2000, GFP_KERNEL);
+ if (result)
dev_dbg(>dev, "%s - error %d from usb_control_msg\n",
__func__, result);
/* there is something funky about this.. the TCSBRK that 'cu' performs
@@ -359,22 +361,11 @@ static int keyspan_pda_get_modem_info(struct usb_serial
*serial,
unsigned char *value)
{
int rc;
- u8 *data;
-
- data = kmalloc(1, GFP_KERNEL);
- if (!data)
- return -ENOMEM;
-
- rc = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
-3, /* get pins */
-USB_TYPE_VENDOR|USB_RECIP_INTERFACE|USB_DIR_IN,
-0, 0, data, 1, 2000);
- if (rc == 1)
- *va
[PATCH 09/15] usb: serial: io_edgeport: use usb_control_msg_recv() and usb_control_msg_send()
The new usb_control_msg_recv() and usb_control_msg_send() nicely wraps
usb_control_msg() with proper error check. Hence use the wrappers
instead of calling usb_control_msg() directly.
Signed-off-by: Himadri Pandya
---
drivers/usb/serial/io_edgeport.c | 73
1 file changed, 27 insertions(+), 46 deletions(-)
diff --git a/drivers/usb/serial/io_edgeport.c b/drivers/usb/serial/io_edgeport.c
index ba5d8df69518..8479d5684af7 100644
--- a/drivers/usb/serial/io_edgeport.c
+++ b/drivers/usb/serial/io_edgeport.c
@@ -568,34 +568,29 @@ static int get_epic_descriptor(struct edgeport_serial *ep)
int result;
struct usb_serial *serial = ep->serial;
struct edgeport_product_info *product_info = >product_info;
- struct edge_compatibility_descriptor *epic;
+ struct edge_compatibility_descriptor epic;
struct edge_compatibility_bits *bits;
struct device *dev = >dev->dev;
ep->is_epic = 0;
- epic = kmalloc(sizeof(*epic), GFP_KERNEL);
- if (!epic)
- return -ENOMEM;
-
- result = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
-USB_REQUEST_ION_GET_EPIC_DESC,
-0xC0, 0x00, 0x00,
-epic, sizeof(*epic),
-300);
- if (result == sizeof(*epic)) {
+ result = usb_control_msg_recv(serial->dev, 0,
+ USB_REQUEST_ION_GET_EPIC_DESC, 0xC0,
+ 0x00, 0x00, , sizeof(epic), 300,
+ GFP_KERNEL);
+ if (result) {
ep->is_epic = 1;
- memcpy(>epic_descriptor, epic, sizeof(*epic));
+ memcpy(>epic_descriptor, , sizeof(epic));
memset(product_info, 0, sizeof(struct edgeport_product_info));
- product_info->NumPorts = epic->NumPorts;
+ product_info->NumPorts = epic.NumPorts;
product_info->ProdInfoVer = 0;
- product_info->FirmwareMajorVersion = epic->MajorVersion;
- product_info->FirmwareMinorVersion = epic->MinorVersion;
- product_info->FirmwareBuildNumber = epic->BuildNumber;
- product_info->iDownloadFile = epic->iDownloadFile;
- product_info->EpicVer = epic->EpicVer;
- product_info->Epic = epic->Supports;
+ product_info->FirmwareMajorVersion = epic.MajorVersion;
+ product_info->FirmwareMinorVersion = epic.MinorVersion;
+ product_info->FirmwareBuildNumber = epic.BuildNumber;
+ product_info->iDownloadFile = epic.iDownloadFile;
+ product_info->EpicVer = epic.EpicVer;
+ product_info->Epic = epic.Supports;
product_info->ProductId = ION_DEVICE_ID_EDGEPORT_COMPATIBLE;
dump_product_info(ep, product_info);
@@ -614,16 +609,12 @@ static int get_epic_descriptor(struct edgeport_serial *ep)
dev_dbg(dev, " IOSPWriteLCR : %s\n", bits->IOSPWriteLCR
? "TRUE": "FALSE");
dev_dbg(dev, " IOSPSetBaudRate : %s\n", bits->IOSPSetBaudRate
? "TRUE": "FALSE");
dev_dbg(dev, " TrueEdgeport : %s\n", bits->TrueEdgeport
? "TRUE": "FALSE");
-
- result = 0;
- } else if (result >= 0) {
+ } else {
dev_warn(>interface->dev, "short epic descriptor
received: %d\n",
result);
result = -EIO;
}
- kfree(epic);
-
return result;
}
@@ -2168,11 +2159,6 @@ static int rom_read(struct usb_serial *serial, __u16
extAddr,
{
int result;
__u16 current_length;
- unsigned char *transfer_buffer;
-
- transfer_buffer = kmalloc(64, GFP_KERNEL);
- if (!transfer_buffer)
- return -ENOMEM;
/* need to split these reads up into 64 byte chunks */
result = 0;
@@ -2181,25 +2167,18 @@ static int rom_read(struct usb_serial *serial, __u16
extAddr,
current_length = 64;
else
current_length = length;
- result = usb_control_msg(serial->dev,
- usb_rcvctrlpipe(serial->dev, 0),
- USB_REQUEST_ION_READ_ROM,
- 0xC0, addr, extAddr, transfer_buffer,
- current_length, 300);
- if (result < current_length) {
- if (result >= 0)
- result = -EIO;
+ result = usb_control_msg_recv(serial
[PATCH 02/15] usb: serial: belkin_sa: use usb_control_msg_send()
The new usb_control_msg_send() nicely wraps usb_control_msg() with
proper error check. Hence use the wrapper instead of calling
usb_control_msg() directly.
Signed-off-by: Himadri Pandya
---
drivers/usb/serial/belkin_sa.c | 35 +-
1 file changed, 17 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/serial/belkin_sa.c b/drivers/usb/serial/belkin_sa.c
index 9bb123ab9bc9..a5ff55f48303 100644
--- a/drivers/usb/serial/belkin_sa.c
+++ b/drivers/usb/serial/belkin_sa.c
@@ -105,9 +105,10 @@ struct belkin_sa_private {
#define WDR_TIMEOUT 5000 /* default urb timeout */
/* assumes that struct usb_serial *serial is available */
-#define BSA_USB_CMD(c, v) usb_control_msg(serial->dev,
usb_sndctrlpipe(serial->dev, 0), \
- (c), BELKIN_SA_SET_REQUEST_TYPE, \
- (v), 0, NULL, 0, WDR_TIMEOUT)
+#define BSA_USB_CMD(c, v) usb_control_msg_send(serial->dev, 0, (c), \
+ BELKIN_SA_SET_REQUEST_TYPE, \
+ (v), 0, NULL, 0, WDR_TIMEOUT, \
+ GFP_KERNEL)
static int belkin_sa_port_probe(struct usb_serial_port *port)
{
@@ -309,12 +310,11 @@ static void belkin_sa_set_termios(struct tty_struct *tty,
/* reassert DTR and (maybe) RTS on transition from B0 */
if ((old_cflag & CBAUD) == B0) {
control_state |= (TIOCM_DTR|TIOCM_RTS);
- if (BSA_USB_CMD(BELKIN_SA_SET_DTR_REQUEST, 1) < 0)
+ if (BSA_USB_CMD(BELKIN_SA_SET_DTR_REQUEST, 1))
dev_err(>dev, "Set DTR error\n");
/* don't set RTS if using hardware flow control */
if (!(old_cflag & CRTSCTS))
- if (BSA_USB_CMD(BELKIN_SA_SET_RTS_REQUEST
- , 1) < 0)
+ if (BSA_USB_CMD(BELKIN_SA_SET_RTS_REQUEST, 1))
dev_err(>dev, "Set RTS error\n");
}
}
@@ -330,18 +330,18 @@ static void belkin_sa_set_termios(struct tty_struct *tty,
/* Report the actual baud rate back to the caller */
tty_encode_baud_rate(tty, baud, baud);
- if (BSA_USB_CMD(BELKIN_SA_SET_BAUDRATE_REQUEST, urb_value) < 0)
+ if (BSA_USB_CMD(BELKIN_SA_SET_BAUDRATE_REQUEST, urb_value))
dev_err(>dev, "Set baudrate error\n");
} else {
/* Disable flow control */
if (BSA_USB_CMD(BELKIN_SA_SET_FLOW_CTRL_REQUEST,
- BELKIN_SA_FLOW_NONE) < 0)
+ BELKIN_SA_FLOW_NONE))
dev_err(>dev, "Disable flowcontrol error\n");
/* Drop RTS and DTR */
control_state &= ~(TIOCM_DTR | TIOCM_RTS);
- if (BSA_USB_CMD(BELKIN_SA_SET_DTR_REQUEST, 0) < 0)
+ if (BSA_USB_CMD(BELKIN_SA_SET_DTR_REQUEST, 0))
dev_err(>dev, "DTR LOW error\n");
- if (BSA_USB_CMD(BELKIN_SA_SET_RTS_REQUEST, 0) < 0)
+ if (BSA_USB_CMD(BELKIN_SA_SET_RTS_REQUEST, 0))
dev_err(>dev, "RTS LOW error\n");
}
@@ -352,7 +352,7 @@ static void belkin_sa_set_termios(struct tty_struct *tty,
: BELKIN_SA_PARITY_EVEN;
else
urb_value = BELKIN_SA_PARITY_NONE;
- if (BSA_USB_CMD(BELKIN_SA_SET_PARITY_REQUEST, urb_value) < 0)
+ if (BSA_USB_CMD(BELKIN_SA_SET_PARITY_REQUEST, urb_value))
dev_err(>dev, "Set parity error\n");
}
@@ -377,7 +377,7 @@ static void belkin_sa_set_termios(struct tty_struct *tty,
urb_value = BELKIN_SA_DATA_BITS(8);
break;
}
- if (BSA_USB_CMD(BELKIN_SA_SET_DATA_BITS_REQUEST, urb_value) < 0)
+ if (BSA_USB_CMD(BELKIN_SA_SET_DATA_BITS_REQUEST, urb_value))
dev_err(>dev, "Set data bits error\n");
}
@@ -385,8 +385,7 @@ static void belkin_sa_set_termios(struct tty_struct *tty,
if ((cflag & CSTOPB) != (old_cflag & CSTOPB)) {
urb_value = (cflag & CSTOPB) ? BELKIN_SA_STOP_BITS(2)
: BELKIN_SA_STOP_BITS(1);
- if (BSA_USB_CMD(BELKIN_SA_SET_STOP_BITS_REQUEST,
- urb_value) < 0)
+ if (BSA_USB_CMD(BELKIN_SA_SET_STOP_BITS_REQUEST, urb_value))
de
[PATCH 05/15] usb: serial: cypress_m8: use usb_control_msg_recv() and usb_control_msg_send()
The new usb_control_msg_recv() and usb_control_msg_send() nicely wraps
usb_control_msg() with proper error check. Hence use the wrappers
instead of calling usb_control_msg() directly.
Signed-off-by: Himadri Pandya
---
drivers/usb/serial/cypress_m8.c | 38 +
1 file changed, 20 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/serial/cypress_m8.c b/drivers/usb/serial/cypress_m8.c
index cc028601c388..4d66cab8eece 100644
--- a/drivers/usb/serial/cypress_m8.c
+++ b/drivers/usb/serial/cypress_m8.c
@@ -341,20 +341,21 @@ static int cypress_serial_control(struct tty_struct *tty,
feature_buffer[4]);
do {
- retval = usb_control_msg(port->serial->dev,
- usb_sndctrlpipe(port->serial->dev, 0),
- HID_REQ_SET_REPORT,
- USB_DIR_OUT | USB_RECIP_INTERFACE |
USB_TYPE_CLASS,
- 0x0300, 0, feature_buffer,
- feature_len, 500);
+ retval = usb_control_msg_send(port->serial->dev, 0,
+ HID_REQ_SET_REPORT,
+ USB_DIR_OUT |
+ USB_RECIP_INTERFACE |
+ USB_TYPE_CLASS, 0x0300,
+ 0, feature_buffer,
+ feature_len, 500,
+ GFP_KERNEL);
if (tries++ >= 3)
break;
- } while (retval != feature_len &&
-retval != -ENODEV);
+ } while (retval != -ENODEV);
- if (retval != feature_len) {
+ if (retval) {
dev_err(dev, "%s - failed sending serial line settings
- %d\n",
__func__, retval);
cypress_set_dead(port);
@@ -379,19 +380,20 @@ static int cypress_serial_control(struct tty_struct *tty,
}
dev_dbg(dev, "%s - retrieving serial line settings\n",
__func__);
do {
- retval = usb_control_msg(port->serial->dev,
- usb_rcvctrlpipe(port->serial->dev, 0),
- HID_REQ_GET_REPORT,
- USB_DIR_IN | USB_RECIP_INTERFACE |
USB_TYPE_CLASS,
- 0x0300, 0, feature_buffer,
- feature_len, 500);
+ retval = usb_control_msg_recv(port->serial->dev, 0,
+ HID_REQ_GET_REPORT,
+ USB_DIR_IN |
+ USB_RECIP_INTERFACE |
+ USB_TYPE_CLASS, 0x0300,
+ 0, feature_buffer,
+ feature_len, 500,
+ GFP_KERNEL);
if (tries++ >= 3)
break;
- } while (retval != feature_len
- && retval != -ENODEV);
+ } while (retval != -ENODEV);
- if (retval != feature_len) {
+ if (retval) {
dev_err(dev, "%s - failed to retrieve serial line
settings - %d\n",
__func__, retval);
cypress_set_dead(port);
--
2.17.1
[PATCH 04/15] usb: serial: cp210x: use usb_control_msg_recv() and usb_control_msg_send()
The new usb_control_msg_recv() and usb_control_msg_send() nicely wraps
usb_control_msg() with proper error check. Hence use the wrappers
instead of calling usb_control_msg() directly.
Signed-off-by: Himadri Pandya
---
drivers/usb/serial/cp210x.c | 148 ++--
1 file changed, 42 insertions(+), 106 deletions(-)
diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index d0c05aa8a0d6..29436ab392e8 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -555,31 +555,15 @@ static int cp210x_read_reg_block(struct usb_serial_port
*port, u8 req,
{
struct usb_serial *serial = port->serial;
struct cp210x_port_private *port_priv = usb_get_serial_port_data(port);
- void *dmabuf;
int result;
- dmabuf = kmalloc(bufsize, GFP_KERNEL);
- if (!dmabuf) {
- /*
-* FIXME Some callers don't bother to check for error,
-* at least give them consistent junk until they are fixed
-*/
- memset(buf, 0, bufsize);
- return -ENOMEM;
- }
-
- result = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
- req, REQTYPE_INTERFACE_TO_HOST, 0,
- port_priv->bInterfaceNumber, dmabuf, bufsize,
- USB_CTRL_SET_TIMEOUT);
- if (result == bufsize) {
- memcpy(buf, dmabuf, bufsize);
- result = 0;
- } else {
+ result = usb_control_msg_recv(serial->dev, 0, req,
+ REQTYPE_INTERFACE_TO_HOST, 0,
+ port_priv->bInterfaceNumber, buf, bufsize,
+ USB_CTRL_SET_TIMEOUT, GFP_KERNEL);
+ if (result) {
dev_err(>dev, "failed get req 0x%x size %d status: %d\n",
- req, bufsize, result);
- if (result >= 0)
- result = -EIO;
+ req, bufsize, result);
/*
* FIXME Some callers don't bother to check for error,
@@ -588,8 +572,6 @@ static int cp210x_read_reg_block(struct usb_serial_port
*port, u8 req,
memset(buf, 0, bufsize);
}
- kfree(dmabuf);
-
return result;
}
@@ -648,29 +630,16 @@ static int cp210x_read_u8_reg(struct usb_serial_port
*port, u8 req, u8 *val)
static int cp210x_read_vendor_block(struct usb_serial *serial, u8 type, u16
val,
void *buf, int bufsize)
{
- void *dmabuf;
int result;
- dmabuf = kmalloc(bufsize, GFP_KERNEL);
- if (!dmabuf)
- return -ENOMEM;
-
- result = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
-CP210X_VENDOR_SPECIFIC, type, val,
-cp210x_interface_num(serial), dmabuf, bufsize,
-USB_CTRL_GET_TIMEOUT);
- if (result == bufsize) {
- memcpy(buf, dmabuf, bufsize);
- result = 0;
- } else {
+ result = usb_control_msg_recv(serial->dev, 0, CP210X_VENDOR_SPECIFIC,
+ type, val, cp210x_interface_num(serial),
+ buf, bufsize, USB_CTRL_GET_TIMEOUT,
+ GFP_KERNEL);
+ if (result)
dev_err(>interface->dev,
"failed to get vendor val 0x%04x size %d: %d\n", val,
bufsize, result);
- if (result >= 0)
- result = -EIO;
- }
-
- kfree(dmabuf);
return result;
}
@@ -685,14 +654,13 @@ static int cp210x_write_u16_reg(struct usb_serial_port
*port, u8 req, u16 val)
struct cp210x_port_private *port_priv = usb_get_serial_port_data(port);
int result;
- result = usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
- req, REQTYPE_HOST_TO_INTERFACE, val,
- port_priv->bInterfaceNumber, NULL, 0,
- USB_CTRL_SET_TIMEOUT);
- if (result < 0) {
+ result = usb_control_msg_send(serial->dev, 0, req,
+ REQTYPE_HOST_TO_INTERFACE, val,
+ port_priv->bInterfaceNumber, NULL, 0,
+ USB_CTRL_SET_TIMEOUT, GFP_KERNEL);
+ if (result)
dev_err(>dev, "failed set request 0x%x status: %d\n",
req, result);
- }
return result;
}
@@ -706,28 +674,17 @@ static int cp210x_write_reg_block(struct usb_serial_port
*port, u8 req,
{
struct usb_serial *serial = port->serial;
struct cp210x_port_private *port_priv = usb_get_serial_po
[PATCH 01/15] usb: serial: ark3116: use usb_control_msg_recv() and usb_control_msg_send()
The new usb_control_msg_recv() and usb_control_msg_send() nicely wraps
usb_control_msg() with proper error check. Hence use the wrappers
instead of calling usb_control_msg() directly.
Signed-off-by: Himadri Pandya
---
drivers/usb/serial/ark3116.c | 29 -
1 file changed, 4 insertions(+), 25 deletions(-)
diff --git a/drivers/usb/serial/ark3116.c b/drivers/usb/serial/ark3116.c
index 71a9206ea1e2..51302892c779 100644
--- a/drivers/usb/serial/ark3116.c
+++ b/drivers/usb/serial/ark3116.c
@@ -77,38 +77,17 @@ struct ark3116_private {
static int ark3116_write_reg(struct usb_serial *serial,
unsigned reg, __u8 val)
{
- int result;
/* 0xfe 0x40 are magic values taken from original driver */
- result = usb_control_msg(serial->dev,
-usb_sndctrlpipe(serial->dev, 0),
-0xfe, 0x40, val, reg,
-NULL, 0, ARK_TIMEOUT);
- if (result)
- return result;
-
- return 0;
+ return usb_control_msg_send(serial->dev, 0, 0xfe, 0x40, val, reg, NULL,
0,
+ ARK_TIMEOUT, GFP_KERNEL);
}
static int ark3116_read_reg(struct usb_serial *serial,
unsigned reg, unsigned char *buf)
{
- int result;
/* 0xfe 0xc0 are magic values taken from original driver */
- result = usb_control_msg(serial->dev,
-usb_rcvctrlpipe(serial->dev, 0),
-0xfe, 0xc0, 0, reg,
-buf, 1, ARK_TIMEOUT);
- if (result < 1) {
- dev_err(>interface->dev,
- "failed to read register %u: %d\n",
- reg, result);
- if (result >= 0)
- result = -EIO;
-
- return result;
- }
-
- return 0;
+ return usb_control_msg_recv(serial->dev, 0, 0xfe, 0xc0, 0, reg, buf, 1,
+ ARK_TIMEOUT, GFP_KERNEL);
}
static inline int calc_divisor(int bps)
--
2.17.1
[PATCH 00/15] usb: serial: avoid using usb_control_msg() directly
There are many usages of usb_control_msg() that can use the new wrapper functions usb_contro_msg_send() & usb_control_msg_recv() for better error checks on short reads and writes. Hence use them whenever possible and avoid using usb_control_msg() directly. Himadri Pandya (15): usb: serial: ark3116: use usb_control_msg_recv() and usb_control_msg_send() usb: serial: belkin_sa: use usb_control_msg_send() usb: serial: ch314: use usb_control_msg_recv() and usb_control_msg_send() usb: serial: cp210x: use usb_control_msg_recv() and usb_control_msg_send() usb: serial: cypress_m8: use usb_control_msg_recv() and usb_control_msg_send() usb: serial: f81232: use usb_control_msg_recv() and usb_control_msg_send() usb: serial: f81534: use usb_control_msg_recv() and usb_control_msg_send() usb: serial: ftdi_sio: use usb_control_msg_recv() and usb_control_msg_send() usb: serial: io_edgeport: use usb_control_msg_recv() and usb_control_msg_send() usb: serial: io_ti: use usb_control_msg_recv() and usb_control_msg_send() usb: serial: ipaq: use usb_control_msg_send() usb: serial: ipw: use usb_control_msg_send() usb: serial: iuu_phoenix: use usb_control_msg_send() usb: serial: keyspan_pda: use usb_control_msg_recv() and usb_control_msg_send() usb: serial: kl5kusb105: use usb_control_msg_recv() and usb_control_msg_send() drivers/usb/serial/ark3116.c | 29 + drivers/usb/serial/belkin_sa.c | 35 +++--- drivers/usb/serial/ch341.c | 45 +++- drivers/usb/serial/cp210x.c | 148 +++-- drivers/usb/serial/cypress_m8.c | 38 --- drivers/usb/serial/f81232.c | 88 +++ drivers/usb/serial/f81534.c | 63 +++ drivers/usb/serial/ftdi_sio.c| 182 +-- drivers/usb/serial/io_edgeport.c | 73 + drivers/usb/serial/io_ti.c | 28 ++--- drivers/usb/serial/ipaq.c| 9 +- drivers/usb/serial/ipw.c | 107 ++ drivers/usb/serial/iuu_phoenix.c | 5 +- drivers/usb/serial/keyspan_pda.c | 172 - drivers/usb/serial/kl5kusb105.c | 94 15 files changed, 406 insertions(+), 710 deletions(-) -- 2.17.1
[PATCH 03/15] usb: serial: ch314: use usb_control_msg_recv() and usb_control_msg_send()
The new usb_control_msg_recv() and usb_control_msg_send() nicely wraps
usb_control_msg() with proper error check. Hence use the wrappers
instead of calling usb_control_msg() directly.
Signed-off-by: Himadri Pandya
---
drivers/usb/serial/ch341.c | 45 --
1 file changed, 14 insertions(+), 31 deletions(-)
diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index a2e2f56c88cd..58c5d3ce4f75 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -111,10 +111,10 @@ static int ch341_control_out(struct usb_device *dev, u8
request,
dev_dbg(>dev, "%s - (%02x,%04x,%04x)\n", __func__,
request, value, index);
- r = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), request,
- USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
- value, index, NULL, 0, DEFAULT_TIMEOUT);
- if (r < 0)
+ r = usb_control_msg_send(dev, 0, request, USB_TYPE_VENDOR |
+USB_RECIP_DEVICE | USB_DIR_OUT, value, index,
+NULL, 0, DEFAULT_TIMEOUT, GFP_KERNEL);
+ if (r)
dev_err(>dev, "failed to send control message: %d\n", r);
return r;
@@ -129,23 +129,14 @@ static int ch341_control_in(struct usb_device *dev,
dev_dbg(>dev, "%s - (%02x,%04x,%04x,%u)\n", __func__,
request, value, index, bufsize);
- r = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0), request,
- USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
- value, index, buf, bufsize, DEFAULT_TIMEOUT);
- if (r < (int)bufsize) {
- if (r >= 0) {
- dev_err(>dev,
- "short control message received (%d < %u)\n",
- r, bufsize);
- r = -EIO;
- }
-
+ r = usb_control_msg_recv(dev, 0, request, USB_TYPE_VENDOR |
+USB_RECIP_DEVICE | USB_DIR_IN, value, index,
+buf, bufsize, DEFAULT_TIMEOUT, GFP_KERNEL);
+ if (r)
dev_err(>dev, "failed to receive control message: %d\n",
r);
- return r;
- }
- return 0;
+ return r;
}
#define CH341_CLKRATE 4800
@@ -343,22 +334,18 @@ static int ch341_detect_quirks(struct usb_serial_port
*port)
struct usb_device *udev = port->serial->dev;
const unsigned int size = 2;
unsigned long quirks = 0;
- char *buffer;
+ u8 buffer[2];
int r;
- buffer = kmalloc(size, GFP_KERNEL);
- if (!buffer)
- return -ENOMEM;
-
/*
* A subset of CH34x devices does not support all features. The
* prescaler is limited and there is no support for sending a RS232
* break condition. A read failure when trying to set up the latter is
* used to detect these devices.
*/
- r = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0), CH341_REQ_READ_REG,
- USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
- CH341_REG_BREAK, 0, buffer, size, DEFAULT_TIMEOUT);
+ r = usb_control_msg_recv(udev, 0, CH341_REQ_READ_REG, USB_TYPE_VENDOR |
+USB_RECIP_DEVICE | USB_DIR_IN, CH341_REG_BREAK,
+0, , size, DEFAULT_TIMEOUT, GFP_KERNEL);
if (r == -EPIPE) {
dev_info(>dev, "break control not supported, using
simulated break\n");
quirks = CH341_QUIRK_LIMITED_PRESCALER |
CH341_QUIRK_SIMULATE_BREAK;
@@ -366,17 +353,13 @@ static int ch341_detect_quirks(struct usb_serial_port
*port)
goto out;
}
- if (r != size) {
- if (r >= 0)
- r = -EIO;
+ if (r) {
dev_err(>dev, "failed to read break control: %d\n", r);
goto out;
}
r = 0;
out:
- kfree(buffer);
-
if (quirks) {
dev_dbg(>dev, "enabling quirk flags: 0x%02lx\n", quirks);
priv->quirks |= quirks;
--
2.17.1
Re: [PATCH 3/4] net: usb: rtl8150: use usb_control_msg_recv() and usb_control_msg_send()
On Thu, Sep 24, 2020 at 5:06 PM Oliver Neukum wrote: > > Am Mittwoch, den 23.09.2020, 20:02 +0530 schrieb Himadri Pandya: > > > I meant that it was stupid to change it without properly understanding > > the significance of GFP_NOIO in this context. > > > > So now, do we re-write the wrapper functions with flag passed as a > > parameter? > > Hi, > > I hope I set you in CC for a patch set doing exactly that. > Yes. > Do not let me or other maintainers discourage you from writing patches. > Look at it this way. Had you not written this patch, I would not have > looked into the matter. Patches are supposed to be reviewed. > If you want additional information, just ask. We do not want > people discouraged from writing substantial patches. > Understood :). I'll send v2 after the update in API is merged. Thanks, Himadri > Regards > Oliver > >
Re: [PATCH 3/4] net: usb: rtl8150: use usb_control_msg_recv() and usb_control_msg_send()
On Wed, Sep 23, 2020 at 7:51 PM Oliver Neukum wrote: > > Am Mittwoch, den 23.09.2020, 19:36 +0530 schrieb Himadri Pandya: > > On Wed, Sep 23, 2020 at 3:52 PM Oliver Neukum wrote: > > > > > > Am Mittwoch, den 23.09.2020, 14:35 +0530 schrieb Himadri Pandya: > > > > GFP_NOIO is used here for a reason. You need to use this helper > > > while in contexts of error recovery and runtime PM. > > > > > > > Understood. Apologies for proposing such a stupid change. > > Hi, > > sorry if you concluded that the patch was stupid. That was not my > intent. It was the best the API allowed for. If an API makes it > easy to make a mistake, the problem is with the API, not the developer. > > Regards > Oliver > I meant that it was stupid to change it without properly understanding the significance of GFP_NOIO in this context. So now, do we re-write the wrapper functions with flag passed as a parameter? Regards, Himadri
Re: [PATCH 4/4] net: rndis_host: use usb_control_msg_recv() and usb_control_msg_send()
On Wed, Sep 23, 2020 at 3:52 PM Greg KH wrote:
>
> On Wed, Sep 23, 2020 at 02:35:19PM +0530, Himadri Pandya wrote:
> > The new usb_control_msg_recv() and usb_control_msg_send() nicely wraps
> > usb_control_msg() with proper error check. Hence use the wrappers
> > instead of calling usb_control_msg() directly.
> >
> > Signed-off-by: Himadri Pandya
> > ---
> > drivers/net/usb/rndis_host.c | 44 ++--
> > 1 file changed, 17 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c
> > index 6fa7a009a24a..30fc4a7183d3 100644
> > --- a/drivers/net/usb/rndis_host.c
> > +++ b/drivers/net/usb/rndis_host.c
> > @@ -113,14 +113,13 @@ int rndis_command(struct usbnet *dev, struct
> > rndis_msg_hdr *buf, int buflen)
> > buf->request_id = (__force __le32) xid;
> > }
> > master_ifnum = info->control->cur_altsetting->desc.bInterfaceNumber;
> > - retval = usb_control_msg(dev->udev,
> > - usb_sndctrlpipe(dev->udev, 0),
> > - USB_CDC_SEND_ENCAPSULATED_COMMAND,
> > - USB_TYPE_CLASS | USB_RECIP_INTERFACE,
> > - 0, master_ifnum,
> > - buf, le32_to_cpu(buf->msg_len),
> > - RNDIS_CONTROL_TIMEOUT_MS);
> > - if (unlikely(retval < 0 || xid == 0))
> > + retval = usb_control_msg_send(dev->udev, 0,
> > + USB_CDC_SEND_ENCAPSULATED_COMMAND,
> > + USB_TYPE_CLASS | USB_RECIP_INTERFACE,
> > + 0, master_ifnum, buf,
> > + le32_to_cpu(buf->msg_len),
> > + RNDIS_CONTROL_TIMEOUT_MS);
> > + if (unlikely(xid == 0))
> > return retval;
> >
> > /* Some devices don't respond on the control channel until
> > @@ -139,14 +138,11 @@ int rndis_command(struct usbnet *dev, struct
> > rndis_msg_hdr *buf, int buflen)
> > /* Poll the control channel; the request probably completed
> > immediately */
> > rsp = le32_to_cpu(buf->msg_type) | RNDIS_MSG_COMPLETION;
> > for (count = 0; count < 10; count++) {
> > - memset(buf, 0, CONTROL_BUFFER_SIZE);
> > - retval = usb_control_msg(dev->udev,
> > - usb_rcvctrlpipe(dev->udev, 0),
> > - USB_CDC_GET_ENCAPSULATED_RESPONSE,
> > - USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
> > - 0, master_ifnum,
> > - buf, buflen,
> > - RNDIS_CONTROL_TIMEOUT_MS);
> > + retval = usb_control_msg_recv(dev->udev, 0,
> > +
> > USB_CDC_GET_ENCAPSULATED_RESPONSE,
> > + USB_DIR_IN | USB_TYPE_CLASS |
> > USB_RECIP_INTERFACE,
> > + 0, master_ifnum, buf, buflen,
> > + RNDIS_CONTROL_TIMEOUT_MS);
> > if (likely(retval >= 8)) {
>
> retval here is never going to be positive, right? So I don't think this
> patch is correct :(
>
Yes :(.
> > msg_type = le32_to_cpu(buf->msg_type);
> > msg_len = le32_to_cpu(buf->msg_len);
> > @@ -178,17 +174,11 @@ int rndis_command(struct usbnet *dev, struct
> > rndis_msg_hdr *buf, int buflen)
> > msg->msg_type =
> > cpu_to_le32(RNDIS_MSG_KEEPALIVE_C);
> > msg->msg_len = cpu_to_le32(sizeof *msg);
> > msg->status =
> > cpu_to_le32(RNDIS_STATUS_SUCCESS);
> > - retval = usb_control_msg(dev->udev,
> > - usb_sndctrlpipe(dev->udev, 0),
> > - USB_CDC_SEND_ENCAPSULATED_COMMAND,
> > - USB_TYPE_CLASS | USB_RECIP_INTERFACE,
> > - 0, master_ifnum,
> > - msg, sizeof *msg,
> > - RNDIS_CONTROL_TIMEOUT_MS);
> > - if (unlikely(retval < 0))
> > - dev_dbg(>control->dev,
> > - "rndis keepalive err %d\n",
> > -
Re: [PATCH 1/4] net: usbnet: use usb_control_msg_recv() and usb_control_msg_send()
On Wed, Sep 23, 2020 at 3:54 PM Greg KH wrote:
>
> On Wed, Sep 23, 2020 at 02:35:16PM +0530, Himadri Pandya wrote:
> > Potential incorrect use of usb_control_msg() has resulted in new wrapper
> > functions to enforce its correct usage with proper error check. Hence
> > use these new wrapper functions instead of calling usb_control_msg()
> > directly.
> >
> > Signed-off-by: Himadri Pandya
> > ---
> > drivers/net/usb/usbnet.c | 46
> > 1 file changed, 4 insertions(+), 42 deletions(-)
> >
> > diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> > index 2b2a841cd938..a38a85bef46a 100644
> > --- a/drivers/net/usb/usbnet.c
> > +++ b/drivers/net/usb/usbnet.c
> > @@ -1982,64 +1982,26 @@ EXPORT_SYMBOL(usbnet_link_change);
> > static int __usbnet_read_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
> >u16 value, u16 index, void *data, u16 size)
> > {
> > - void *buf = NULL;
> > - int err = -ENOMEM;
> > -
> > netdev_dbg(dev->net, "usbnet_read_cmd cmd=0x%02x reqtype=%02x"
> > " value=0x%04x index=0x%04x size=%d\n",
> > cmd, reqtype, value, index, size);
> >
> > - if (size) {
> > - buf = kmalloc(size, GFP_KERNEL);
> > - if (!buf)
> > - goto out;
> > - }
> > -
> > - err = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
> > - cmd, reqtype, value, index, buf, size,
> > + return usb_control_msg_recv(dev->udev, 0,
> > + cmd, reqtype, value, index, data, size,
> > USB_CTRL_GET_TIMEOUT);
> > - if (err > 0 && err <= size) {
> > -if (data)
> > -memcpy(data, buf, err);
> > -else
> > -netdev_dbg(dev->net,
> > -"Huh? Data requested but thrown away.\n");
> > -}
> > - kfree(buf);
> > -out:
> > - return err;
> > }
>
> Now there is no real need for these wrapper functions at all, except for
> the debugging which I doubt anyone needs anymore.
>
> So how about just deleting these and calling the real function instead?
>
Yes, that would be a better thing to do.
Thanks,
Himadri
> thanks,
>
> greg k-h
Re: [PATCH 3/4] net: usb: rtl8150: use usb_control_msg_recv() and usb_control_msg_send()
On Wed, Sep 23, 2020 at 3:52 PM Oliver Neukum wrote:
>
> Am Mittwoch, den 23.09.2020, 14:35 +0530 schrieb Himadri Pandya:
>
> Hi,
>
> > Many usage of usb_control_msg() do not have proper error check on return
> > value leaving scope for bugs on short reads. New usb_control_msg_recv()
> > and usb_control_msg_send() nicely wraps usb_control_msg() with proper
> > error check. Hence use the wrappers instead of calling usb_control_msg()
> > directly.
> >
> > Signed-off-by: Himadri Pandya
> Nacked-by: Oliver Neukum
>
> > ---
> > drivers/net/usb/rtl8150.c | 32 ++--
> > 1 file changed, 6 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
> > index 733f120c852b..e3002b675921 100644
> > --- a/drivers/net/usb/rtl8150.c
> > +++ b/drivers/net/usb/rtl8150.c
> > @@ -152,36 +152,16 @@ static const char driver_name [] = "rtl8150";
> > */
> > static int get_registers(rtl8150_t * dev, u16 indx, u16 size, void *data)
> > {
> > - void *buf;
> > - int ret;
> > -
> > - buf = kmalloc(size, GFP_NOIO);
>
> GFP_NOIO is used here for a reason. You need to use this helper
> while in contexts of error recovery and runtime PM.
>
Understood. Apologies for proposing such a stupid change.
> > - if (!buf)
> > - return -ENOMEM;
> > -
> > - ret = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
> > - RTL8150_REQ_GET_REGS, RTL8150_REQT_READ,
> > - indx, 0, buf, size, 500);
> > - if (ret > 0 && ret <= size)
> > - memcpy(data, buf, ret);
> > - kfree(buf);
> > - return ret;
> > + return usb_control_msg_recv(dev->udev, 0, RTL8150_REQ_GET_REGS,
> > + RTL8150_REQT_READ, indx, 0, data,
> > + size, 500);
>
> This internally uses kmemdup() with GFP_KERNEL.
> You cannot make this change. The API does not support it.
> I am afraid we will have to change the API first, before more
> such changes are done.
>
> I would suggest dropping the whole series for now.
Okay. Thanks for the review.
Regards,
Himadri
>
> Regards
> Oliver
>
[PATCH 4/4] net: rndis_host: use usb_control_msg_recv() and usb_control_msg_send()
The new usb_control_msg_recv() and usb_control_msg_send() nicely wraps
usb_control_msg() with proper error check. Hence use the wrappers
instead of calling usb_control_msg() directly.
Signed-off-by: Himadri Pandya
---
drivers/net/usb/rndis_host.c | 44 ++--
1 file changed, 17 insertions(+), 27 deletions(-)
diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c
index 6fa7a009a24a..30fc4a7183d3 100644
--- a/drivers/net/usb/rndis_host.c
+++ b/drivers/net/usb/rndis_host.c
@@ -113,14 +113,13 @@ int rndis_command(struct usbnet *dev, struct
rndis_msg_hdr *buf, int buflen)
buf->request_id = (__force __le32) xid;
}
master_ifnum = info->control->cur_altsetting->desc.bInterfaceNumber;
- retval = usb_control_msg(dev->udev,
- usb_sndctrlpipe(dev->udev, 0),
- USB_CDC_SEND_ENCAPSULATED_COMMAND,
- USB_TYPE_CLASS | USB_RECIP_INTERFACE,
- 0, master_ifnum,
- buf, le32_to_cpu(buf->msg_len),
- RNDIS_CONTROL_TIMEOUT_MS);
- if (unlikely(retval < 0 || xid == 0))
+ retval = usb_control_msg_send(dev->udev, 0,
+ USB_CDC_SEND_ENCAPSULATED_COMMAND,
+ USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+ 0, master_ifnum, buf,
+ le32_to_cpu(buf->msg_len),
+ RNDIS_CONTROL_TIMEOUT_MS);
+ if (unlikely(xid == 0))
return retval;
/* Some devices don't respond on the control channel until
@@ -139,14 +138,11 @@ int rndis_command(struct usbnet *dev, struct
rndis_msg_hdr *buf, int buflen)
/* Poll the control channel; the request probably completed immediately
*/
rsp = le32_to_cpu(buf->msg_type) | RNDIS_MSG_COMPLETION;
for (count = 0; count < 10; count++) {
- memset(buf, 0, CONTROL_BUFFER_SIZE);
- retval = usb_control_msg(dev->udev,
- usb_rcvctrlpipe(dev->udev, 0),
- USB_CDC_GET_ENCAPSULATED_RESPONSE,
- USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
- 0, master_ifnum,
- buf, buflen,
- RNDIS_CONTROL_TIMEOUT_MS);
+ retval = usb_control_msg_recv(dev->udev, 0,
+ USB_CDC_GET_ENCAPSULATED_RESPONSE,
+ USB_DIR_IN | USB_TYPE_CLASS |
USB_RECIP_INTERFACE,
+ 0, master_ifnum, buf, buflen,
+ RNDIS_CONTROL_TIMEOUT_MS);
if (likely(retval >= 8)) {
msg_type = le32_to_cpu(buf->msg_type);
msg_len = le32_to_cpu(buf->msg_len);
@@ -178,17 +174,11 @@ int rndis_command(struct usbnet *dev, struct
rndis_msg_hdr *buf, int buflen)
msg->msg_type =
cpu_to_le32(RNDIS_MSG_KEEPALIVE_C);
msg->msg_len = cpu_to_le32(sizeof *msg);
msg->status = cpu_to_le32(RNDIS_STATUS_SUCCESS);
- retval = usb_control_msg(dev->udev,
- usb_sndctrlpipe(dev->udev, 0),
- USB_CDC_SEND_ENCAPSULATED_COMMAND,
- USB_TYPE_CLASS | USB_RECIP_INTERFACE,
- 0, master_ifnum,
- msg, sizeof *msg,
- RNDIS_CONTROL_TIMEOUT_MS);
- if (unlikely(retval < 0))
- dev_dbg(>control->dev,
- "rndis keepalive err %d\n",
- retval);
+ retval = usb_control_msg_send(dev->udev, 0,
+
USB_CDC_SEND_ENCAPSULATED_COMMAND,
+ USB_TYPE_CLASS |
USB_RECIP_INTERFACE,
+ 0, master_ifnum,
msg, sizeof(*msg),
+
RNDIS_CONTROL_TIMEOUT_MS);
}
break;
default:
--
2.17.1
[PATCH 1/4] net: usbnet: use usb_control_msg_recv() and usb_control_msg_send()
Potential incorrect use of usb_control_msg() has resulted in new wrapper
functions to enforce its correct usage with proper error check. Hence
use these new wrapper functions instead of calling usb_control_msg()
directly.
Signed-off-by: Himadri Pandya
---
drivers/net/usb/usbnet.c | 46
1 file changed, 4 insertions(+), 42 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 2b2a841cd938..a38a85bef46a 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1982,64 +1982,26 @@ EXPORT_SYMBOL(usbnet_link_change);
static int __usbnet_read_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
u16 value, u16 index, void *data, u16 size)
{
- void *buf = NULL;
- int err = -ENOMEM;
-
netdev_dbg(dev->net, "usbnet_read_cmd cmd=0x%02x reqtype=%02x"
" value=0x%04x index=0x%04x size=%d\n",
cmd, reqtype, value, index, size);
- if (size) {
- buf = kmalloc(size, GFP_KERNEL);
- if (!buf)
- goto out;
- }
-
- err = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
- cmd, reqtype, value, index, buf, size,
+ return usb_control_msg_recv(dev->udev, 0,
+ cmd, reqtype, value, index, data, size,
USB_CTRL_GET_TIMEOUT);
- if (err > 0 && err <= size) {
-if (data)
-memcpy(data, buf, err);
-else
-netdev_dbg(dev->net,
-"Huh? Data requested but thrown away.\n");
-}
- kfree(buf);
-out:
- return err;
}
static int __usbnet_write_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
u16 value, u16 index, const void *data,
u16 size)
{
- void *buf = NULL;
- int err = -ENOMEM;
-
netdev_dbg(dev->net, "usbnet_write_cmd cmd=0x%02x reqtype=%02x"
" value=0x%04x index=0x%04x size=%d\n",
cmd, reqtype, value, index, size);
- if (data) {
- buf = kmemdup(data, size, GFP_KERNEL);
- if (!buf)
- goto out;
- } else {
-if (size) {
-WARN_ON_ONCE(1);
-err = -EINVAL;
-goto out;
-}
-}
-
- err = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
- cmd, reqtype, value, index, buf, size,
+ return usb_control_msg_send(dev->udev, 0,
+ cmd, reqtype, value, index, data, size,
USB_CTRL_SET_TIMEOUT);
- kfree(buf);
-
-out:
- return err;
}
/*
--
2.17.1
[PATCH 0/4] net: usb: avoid using usb_control_msg() directly
A recent bug-fix shaded light on possible incorrect use of usb_control_msg() without proper error check. This resulted in introducing new usb core api wrapper functions usb_control_msg_send() and usb_control_msg_recv() by Greg KH. This patch series continue the clean-up using the new functions. Himadri Pandya (4): net: usbnet: use usb_control_msg_recv() and usb_control_msg_send() net: sierra_net: use usb_control_msg_recv() net: usb: rtl8150: use usb_control_msg_recv() and usb_control_msg_send() net: rndis_host: use usb_control_msg_recv() and usb_control_msg_send() drivers/net/usb/rndis_host.c | 44 +- drivers/net/usb/rtl8150.c| 32 + drivers/net/usb/sierra_net.c | 17 ++--- drivers/net/usb/usbnet.c | 46 4 files changed, 34 insertions(+), 105 deletions(-) -- 2.17.1
[PATCH 3/4] net: usb: rtl8150: use usb_control_msg_recv() and usb_control_msg_send()
Many usage of usb_control_msg() do not have proper error check on return
value leaving scope for bugs on short reads. New usb_control_msg_recv()
and usb_control_msg_send() nicely wraps usb_control_msg() with proper
error check. Hence use the wrappers instead of calling usb_control_msg()
directly.
Signed-off-by: Himadri Pandya
---
drivers/net/usb/rtl8150.c | 32 ++--
1 file changed, 6 insertions(+), 26 deletions(-)
diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
index 733f120c852b..e3002b675921 100644
--- a/drivers/net/usb/rtl8150.c
+++ b/drivers/net/usb/rtl8150.c
@@ -152,36 +152,16 @@ static const char driver_name [] = "rtl8150";
*/
static int get_registers(rtl8150_t * dev, u16 indx, u16 size, void *data)
{
- void *buf;
- int ret;
-
- buf = kmalloc(size, GFP_NOIO);
- if (!buf)
- return -ENOMEM;
-
- ret = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
- RTL8150_REQ_GET_REGS, RTL8150_REQT_READ,
- indx, 0, buf, size, 500);
- if (ret > 0 && ret <= size)
- memcpy(data, buf, ret);
- kfree(buf);
- return ret;
+ return usb_control_msg_recv(dev->udev, 0, RTL8150_REQ_GET_REGS,
+ RTL8150_REQT_READ, indx, 0, data,
+ size, 500);
}
static int set_registers(rtl8150_t * dev, u16 indx, u16 size, const void *data)
{
- void *buf;
- int ret;
-
- buf = kmemdup(data, size, GFP_NOIO);
- if (!buf)
- return -ENOMEM;
-
- ret = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
- RTL8150_REQ_SET_REGS, RTL8150_REQT_WRITE,
- indx, 0, buf, size, 500);
- kfree(buf);
- return ret;
+ return usb_control_msg_send(dev->udev, 0, RTL8150_REQ_SET_REGS,
+ RTL8150_REQT_WRITE, indx, 0, data,
+ size, 500);
}
static void async_set_reg_cb(struct urb *urb)
--
2.17.1
[PATCH 2/4] net: sierra_net: use usb_control_msg_recv()
The new usb api function usb_control_msg_recv() nicely wrapps
usb_control_msg() with proper error check. Hence use it instead of
directly calling usb_control_msg().
Signed-off-by: Himadri Pandya
---
drivers/net/usb/sierra_net.c | 17 +++--
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/net/usb/sierra_net.c b/drivers/net/usb/sierra_net.c
index 0abd257b634c..f3a5f83cb256 100644
--- a/drivers/net/usb/sierra_net.c
+++ b/drivers/net/usb/sierra_net.c
@@ -478,16 +478,13 @@ static void sierra_net_kevent(struct work_struct *work)
return;
ifnum = priv->ifnum;
- len = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
- USB_CDC_GET_ENCAPSULATED_RESPONSE,
- USB_DIR_IN|USB_TYPE_CLASS|USB_RECIP_INTERFACE,
- 0, ifnum, buf, SIERRA_NET_USBCTL_BUF_LEN,
- USB_CTRL_SET_TIMEOUT);
-
- if (len < 0) {
- netdev_err(dev->net,
- "usb_control_msg failed, status %d\n", len);
- } else {
+ len = usb_control_msg_recv(dev->udev,
usb_rcvctrlpipe(dev->udev, 0),
+ USB_CDC_GET_ENCAPSULATED_RESPONSE,
+ USB_DIR_IN | USB_TYPE_CLASS |
USB_RECIP_INTERFACE,
+ 0, ifnum, buf,
SIERRA_NET_USBCTL_BUF_LEN,
+ USB_CTRL_SET_TIMEOUT);
+
+ if (len) {
struct hip_hdr hh;
dev_dbg(>udev->dev, "%s: Received status message,"
--
2.17.1
Re: [PATCH] net: usb: Fix uninit-was-stored issue in asix_read_phy_addr()
On Thu, Aug 27, 2020 at 1:28 PM Sergei Shtylyov
wrote:
>
> Hello!
>
> On 27.08.2020 9:53, Himadri Pandya wrote:
>
> > The buffer size is 2 Bytes and we expect to receive the same amount of
> > data. But sometimes we receive less data and run into uninit-was-stored
> > issue upon read. Hence modify the error check on the return value to match
> > with the buffer size as a prevention.
> >
> > Reported-and-tested by:
> > syzbot+a7e220df5a81d1ab4...@syzkaller.appspotmail.com
> > Signed-off-by: Himadri Pandya
> > ---
> > drivers/net/usb/asix_common.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
> > index e39f41efda3e..7bc6e8f856fe 100644
> > --- a/drivers/net/usb/asix_common.c
> > +++ b/drivers/net/usb/asix_common.c
> > @@ -296,7 +296,7 @@ int asix_read_phy_addr(struct usbnet *dev, int internal)
> >
> > netdev_dbg(dev->net, "asix_get_phy_addr()\n");
> >
> > - if (ret < 0) {
> > + if (ret < 2) {
> > netdev_err(dev->net, "Error reading PHYID register: %02x\n",
> > ret);
>
> Hm... printing possibly negative values as hex?
>
Yeah. That's odd! Fixing it.
Thanks,
Himadri
> [...]
>
> MBR, Sergei
[PATCH] net: usb: Fix uninit-was-stored issue in asix_read_phy_addr()
The buffer size is 2 Bytes and we expect to receive the same amount of
data. But sometimes we receive less data and run into uninit-was-stored
issue upon read. Hence modify the error check on the return value to match
with the buffer size as a prevention.
Reported-and-tested by: syzbot+a7e220df5a81d1ab4...@syzkaller.appspotmail.com
Signed-off-by: Himadri Pandya
---
drivers/net/usb/asix_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
index e39f41efda3e..7bc6e8f856fe 100644
--- a/drivers/net/usb/asix_common.c
+++ b/drivers/net/usb/asix_common.c
@@ -296,7 +296,7 @@ int asix_read_phy_addr(struct usbnet *dev, int internal)
netdev_dbg(dev->net, "asix_get_phy_addr()\n");
- if (ret < 0) {
+ if (ret < 2) {
netdev_err(dev->net, "Error reading PHYID register: %02x\n",
ret);
goto out;
}
--
2.17.1
Re: [PATCH] net: usb: Fix uninit-was-stored issue in asix_read_cmd()
On Mon, Aug 24, 2020 at 11:16:55AM -0700, Jakub Kicinski wrote: > On Sun, 23 Aug 2020 13:50:42 +0530 Himadri Pandya wrote: > > Initialize the buffer before passing it to usb_read_cmd() function(s) to > > fix the uninit-was-stored issue in asix_read_cmd(). > > > > Fixes: KMSAN: kernel-infoleak in raw_ioctl > > Regardless of the ongoing discussion - could you please make this line > a correct Fixes tag? > > Right now integration scripts are complaining that it doesn't contain a > valid git hash. > > > Reported by: syzbot+a7e220df5a81d1ab4...@syzkaller.appspotmail.com > > > > This empty line is unnecessary. > > > Signed-off-by: Himadri Pandya Thank you. I'll fix it. Himadri
[PATCH] net: usb: Fix uninit-was-stored issue in asix_read_cmd()
Initialize the buffer before passing it to usb_read_cmd() function(s) to fix the uninit-was-stored issue in asix_read_cmd(). Fixes: KMSAN: kernel-infoleak in raw_ioctl Reported by: syzbot+a7e220df5a81d1ab4...@syzkaller.appspotmail.com Signed-off-by: Himadri Pandya --- drivers/net/usb/asix_common.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c index e39f41efda3e..a67ea1971b78 100644 --- a/drivers/net/usb/asix_common.c +++ b/drivers/net/usb/asix_common.c @@ -17,6 +17,8 @@ int asix_read_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index, BUG_ON(!dev); + memset(data, 0, size); + if (!in_pm) fn = usbnet_read_cmd; else -- 2.17.1
[PATCH] Drivers: hv: balloon: Remove dependencies on guest page size
Hyper-V assumes page size to be 4K. This might not be the case for
ARM64 architecture. Hence use hyper-v specific page size and page
shift definitions to avoid conflicts between different host and guest
page sizes on ARM64.
Also, remove some old and incorrect comments and redefine ballooning
granularities to handle larger page sizes correctly.
Signed-off-by: Himadri Pandya
---
drivers/hv/hv_balloon.c | 25 -
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index 34bd73526afd..935904830d42 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -23,6 +23,7 @@
#include
#include
+#include
#define CREATE_TRACE_POINTS
#include "hv_trace_balloon.h"
@@ -341,8 +342,6 @@ struct dm_unballoon_response {
*
* mem_range: Memory range to hot add.
*
- * On Linux we currently don't support this since we cannot hot add
- * arbitrary granularity of memory.
*/
struct dm_hot_add {
@@ -477,7 +476,7 @@ module_param(pressure_report_delay, uint, (S_IRUGO |
S_IWUSR));
MODULE_PARM_DESC(pressure_report_delay, "Delay in secs in reporting pressure");
static atomic_t trans_id = ATOMIC_INIT(0);
-static int dm_ring_size = (5 * PAGE_SIZE);
+static int dm_ring_size = 20 * 1024;
/*
* Driver specific state.
@@ -493,10 +492,10 @@ enum hv_dm_state {
};
-static __u8 recv_buffer[PAGE_SIZE];
-static __u8 balloon_up_send_buffer[PAGE_SIZE];
-#define PAGES_IN_2M512
-#define HA_CHUNK (32 * 1024)
+static __u8 recv_buffer[HV_HYP_PAGE_SIZE];
+static __u8 balloon_up_send_buffer[HV_HYP_PAGE_SIZE];
+#define PAGES_IN_2M (2 * 1024 * 1024 / PAGE_SIZE)
+#define HA_CHUNK (128 * 1024 * 1024 / PAGE_SIZE)
struct hv_dynmem_device {
struct hv_device *dev;
@@ -1076,7 +1075,7 @@ static void process_info(struct hv_dynmem_device *dm,
struct dm_info_msg *msg)
__u64 *max_page_count = (__u64 *)_hdr[1];
pr_info("Max. dynamic memory size: %llu MB\n",
- (*max_page_count) >> (20 - PAGE_SHIFT));
+ (*max_page_count) >> (20 - HV_HYP_PAGE_SHIFT));
}
break;
@@ -1218,7 +1217,7 @@ static unsigned int alloc_balloon_pages(struct
hv_dynmem_device *dm,
for (i = 0; (i * alloc_unit) < num_pages; i++) {
if (bl_resp->hdr.size + sizeof(union dm_mem_page_range) >
- PAGE_SIZE)
+ HV_HYP_PAGE_SIZE)
return i * alloc_unit;
/*
@@ -1274,9 +1273,9 @@ static void balloon_up(struct work_struct *dummy)
/*
* We will attempt 2M allocations. However, if we fail to
-* allocate 2M chunks, we will go back to 4k allocations.
+* allocate 2M chunks, we will go back to PAGE_SIZE allocations.
*/
- alloc_unit = 512;
+ alloc_unit = PAGES_IN_2M;
avail_pages = si_mem_available();
floor = compute_balloon_floor();
@@ -1292,7 +1291,7 @@ static void balloon_up(struct work_struct *dummy)
}
while (!done) {
- memset(balloon_up_send_buffer, 0, PAGE_SIZE);
+ memset(balloon_up_send_buffer, 0, HV_HYP_PAGE_SIZE);
bl_resp = (struct dm_balloon_response *)balloon_up_send_buffer;
bl_resp->hdr.type = DM_BALLOON_RESPONSE;
bl_resp->hdr.size = sizeof(struct dm_balloon_response);
@@ -1491,7 +1490,7 @@ static void balloon_onchannelcallback(void *context)
memset(recv_buffer, 0, sizeof(recv_buffer));
vmbus_recvpacket(dev->channel, recv_buffer,
-PAGE_SIZE, , );
+HV_HYP_PAGE_SIZE, , );
if (recvlen > 0) {
dm_msg = (struct dm_message *)recv_buffer;
--
2.17.1
[PATCH 0/2] Drivers: hv: Remove dependencies on guest page size
Hyper-V assumes page size to be 4KB. This might not be the case on ARM64 architecture. The first patch in this patchset introduces a hyer-v specific function for allocating a zeroed page which can have a different implementation on ARM64 to address the issue of different guest and host page sizes. The second patch removes dependencies on guest page size in vmbus by using hyper-v specific page symbol and functions. Himadri Pandya (2): x86: hv: Add function to allocate zeroed page for Hyper-V Drivers: hv: vmbus: Remove dependencies on guest page size arch/x86/hyperv/hv_init.c | 8 arch/x86/include/asm/mshyperv.h | 1 + drivers/hv/connection.c | 14 +++--- drivers/hv/vmbus_drv.c | 6 +++--- 4 files changed, 19 insertions(+), 10 deletions(-) -- 2.17.1
[PATCH 1/2] x86: hv: Add function to allocate zeroed page for Hyper-V
Hyper-V assumes page size to be 4K. While this assumption holds true on
x86 architecture, it might not be true for ARM64 architecture. Hence
define hyper-v specific function to allocate a zeroed page which can
have a different implementation on ARM64 architecture to handle the
conflict between hyper-v's assumed page size and actual guest page size.
Signed-off-by: Himadri Pandya
---
arch/x86/hyperv/hv_init.c | 8
arch/x86/include/asm/mshyperv.h | 1 +
2 files changed, 9 insertions(+)
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index d314cf1e15fd..2d0b9b2bddf7 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -45,6 +45,14 @@ void *hv_alloc_hyperv_page(void)
}
EXPORT_SYMBOL_GPL(hv_alloc_hyperv_page);
+void *hv_alloc_hyperv_zeroed_page(void)
+{
+BUILD_BUG_ON(PAGE_SIZE != HV_HYP_PAGE_SIZE);
+
+return (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
+}
+EXPORT_SYMBOL_GPL(hv_alloc_hyperv_zeroed_page);
+
void hv_free_hyperv_page(unsigned long addr)
{
free_page(addr);
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index f4138aeb4280..6b79515abb82 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -219,6 +219,7 @@ static inline struct hv_vp_assist_page
*hv_get_vp_assist_page(unsigned int cpu)
void __init hyperv_init(void);
void hyperv_setup_mmu_ops(void);
void *hv_alloc_hyperv_page(void);
+void *hv_alloc_hyperv_zeroed_page(void);
void hv_free_hyperv_page(unsigned long addr);
void hyperv_reenlightenment_intr(struct pt_regs *regs);
void set_hv_tscchange_cb(void (*cb)(void));
--
2.17.1
[PATCH 2/2] Drivers: hv: vmbus: Remove dependencies on guest page size
Hyper-V assumes page size to be 4K. This might not be the case for ARM64
architecture. Hence use hyper-v page size and page allocation function
to avoid conflicts between different host and guest page size on ARM64.
Signed-off-by: Himadri Pandya
---
drivers/hv/connection.c | 14 +++---
drivers/hv/vmbus_drv.c | 6 +++---
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 09829e15d4a0..dcb8f6a8c08c 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -202,7 +202,7 @@ int vmbus_connect(void)
* abstraction stuff
*/
vmbus_connection.int_page =
- (void *)__get_free_pages(GFP_KERNEL|__GFP_ZERO, 0);
+ (void *)hv_alloc_hyperv_zeroed_page();
if (vmbus_connection.int_page == NULL) {
ret = -ENOMEM;
goto cleanup;
@@ -211,14 +211,14 @@ int vmbus_connect(void)
vmbus_connection.recv_int_page = vmbus_connection.int_page;
vmbus_connection.send_int_page =
(void *)((unsigned long)vmbus_connection.int_page +
- (PAGE_SIZE >> 1));
+ (HV_HYP_PAGE_SIZE >> 1));
/*
* Setup the monitor notification facility. The 1st page for
* parent->child and the 2nd page for child->parent
*/
- vmbus_connection.monitor_pages[0] = (void
*)__get_free_pages((GFP_KERNEL|__GFP_ZERO), 0);
- vmbus_connection.monitor_pages[1] = (void
*)__get_free_pages((GFP_KERNEL|__GFP_ZERO), 0);
+ vmbus_connection.monitor_pages[0] = (void
*)hv_alloc_hyperv_zeroed_page();
+ vmbus_connection.monitor_pages[1] = (void
*)hv_alloc_hyperv_zeroed_page();
if ((vmbus_connection.monitor_pages[0] == NULL) ||
(vmbus_connection.monitor_pages[1] == NULL)) {
ret = -ENOMEM;
@@ -291,12 +291,12 @@ void vmbus_disconnect(void)
destroy_workqueue(vmbus_connection.work_queue);
if (vmbus_connection.int_page) {
- free_pages((unsigned long)vmbus_connection.int_page, 0);
+ hv_free_hyperv_page((unsigned long)vmbus_connection.int_page);
vmbus_connection.int_page = NULL;
}
- free_pages((unsigned long)vmbus_connection.monitor_pages[0], 0);
- free_pages((unsigned long)vmbus_connection.monitor_pages[1], 0);
+ hv_free_hyperv_page((unsigned long)vmbus_connection.monitor_pages[0]);
+ hv_free_hyperv_page((unsigned long)vmbus_connection.monitor_pages[1]);
vmbus_connection.monitor_pages[0] = NULL;
vmbus_connection.monitor_pages[1] = NULL;
}
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index ebd35fc35290..2ee388a23c8f 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1186,7 +1186,7 @@ static void hv_kmsg_dump(struct kmsg_dumper *dumper,
* Write dump contents to the page. No need to synchronize; panic should
* be single-threaded.
*/
- kmsg_dump_get_buffer(dumper, true, hv_panic_page, PAGE_SIZE,
+ kmsg_dump_get_buffer(dumper, true, hv_panic_page, HV_HYP_PAGE_SIZE,
_written);
if (bytes_written)
hyperv_report_panic_msg(panic_pa, bytes_written);
@@ -1290,7 +1290,7 @@ static int vmbus_bus_init(void)
*/
hv_get_crash_ctl(hyperv_crash_ctl);
if (hyperv_crash_ctl & HV_CRASH_CTL_CRASH_NOTIFY_MSG) {
- hv_panic_page = (void *)get_zeroed_page(GFP_KERNEL);
+ hv_panic_page = (void *)hv_alloc_hyperv_zeroed_page();
if (hv_panic_page) {
ret = kmsg_dump_register(_kmsg_dumper);
if (ret)
@@ -1319,7 +1319,7 @@ static int vmbus_bus_init(void)
hv_remove_vmbus_irq();
bus_unregister(_bus);
- free_page((unsigned long)hv_panic_page);
+ hv_free_hyperv_page((unsigned long)hv_panic_page);
unregister_sysctl_table(hv_ctl_table_hdr);
hv_ctl_table_hdr = NULL;
return ret;
--
2.17.1
Re: [PATCH] hv_sock: use HV_HYP_PAGE_SIZE instead of PAGE_SIZE_4K
On 7/27/2019 10:50 AM, kbuild test robot wrote: Hi Himadri, Thank you for the patch! Yet something to improve: [auto build test ERROR on linus/master] [cannot apply to v5.3-rc1 next-20190726] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] This patch should be applied to linux-next git tree. Thank you. - Himadri url: https://github.com/0day-ci/linux/commits/Himadri-Pandya/hv_sock-use-HV_HYP_PAGE_SIZE-instead-of-PAGE_SIZE_4K/20190726-085229 config: x86_64-allyesconfig (attached as .config) compiler: gcc-7 (Debian 7.4.0-10) 7.4.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 If you fix the issue, kindly add following tag Reported-by: kbuild test robot All error/warnings (new ones prefixed by >>): net/vmw_vsock/hyperv_transport.c:58:28: error: 'HV_HYP_PAGE_SIZE' undeclared here (not in a function); did you mean 'HV_MESSAGE_SIZE'? #define HVS_SEND_BUF_SIZE (HV_HYP_PAGE_SIZE - sizeof(struct vmpipe_proto_header)) ^ net/vmw_vsock/hyperv_transport.c:65:10: note: in expansion of macro 'HVS_SEND_BUF_SIZE' u8 data[HVS_SEND_BUF_SIZE]; ^ In file included from include/linux/list.h:9:0, from include/linux/module.h:9, from net/vmw_vsock/hyperv_transport.c:11: net/vmw_vsock/hyperv_transport.c: In function 'hvs_open_connection': include/linux/kernel.h:845:2: error: first argument to '__builtin_choose_expr' not a constant __builtin_choose_expr(__safe_cmp(x, y), \ ^ include/linux/kernel.h:921:27: note: in expansion of macro '__careful_cmp' #define max_t(type, x, y) __careful_cmp((type)(x), (type)(y), >) ^ net/vmw_vsock/hyperv_transport.c:390:12: note: in expansion of macro 'max_t' sndbuf = max_t(int, sk->sk_sndbuf, RINGBUFFER_HVS_SND_SIZE); ^ include/linux/kernel.h:845:2: error: first argument to '__builtin_choose_expr' not a constant __builtin_choose_expr(__safe_cmp(x, y), \ ^ include/linux/kernel.h:913:27: note: in expansion of macro '__careful_cmp' #define min_t(type, x, y) __careful_cmp((type)(x), (type)(y), <) ^ net/vmw_vsock/hyperv_transport.c:391:12: note: in expansion of macro 'min_t' sndbuf = min_t(int, sndbuf, RINGBUFFER_HVS_MAX_SIZE); ^ include/linux/kernel.h:845:2: error: first argument to '__builtin_choose_expr' not a constant __builtin_choose_expr(__safe_cmp(x, y), \ ^ include/linux/kernel.h:921:27: note: in expansion of macro '__careful_cmp' #define max_t(type, x, y) __careful_cmp((type)(x), (type)(y), >) ^ net/vmw_vsock/hyperv_transport.c:393:12: note: in expansion of macro 'max_t' rcvbuf = max_t(int, sk->sk_rcvbuf, RINGBUFFER_HVS_RCV_SIZE); ^ include/linux/kernel.h:845:2: error: first argument to '__builtin_choose_expr' not a constant __builtin_choose_expr(__safe_cmp(x, y), \ ^ include/linux/kernel.h:913:27: note: in expansion of macro '__careful_cmp' #define min_t(type, x, y) __careful_cmp((type)(x), (type)(y), <) ^ net/vmw_vsock/hyperv_transport.c:394:12: note: in expansion of macro 'min_t' rcvbuf = min_t(int, rcvbuf, RINGBUFFER_HVS_MAX_SIZE); ^ net/vmw_vsock/hyperv_transport.c: In function 'hvs_stream_enqueue': include/linux/kernel.h:845:2: error: first argument to '__builtin_choose_expr' not a constant __builtin_choose_expr(__safe_cmp(x, y), \ ^ include/linux/kernel.h:913:27: note: in expansion of macro '__careful_cmp' #define min_t(type, x, y) __careful_cmp((type)(x), (type)(y), <) ^ net/vmw_vsock/hyperv_transport.c:681:14: note: in expansion of macro 'min_t' to_write = min_t(ssize_t, to_write, HVS_SEND_BUF_SIZE); ^ vim +58 net/vmw_vsock/hyperv_transport.c --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
[PATCH] hv_sock: use HV_HYP_PAGE_SIZE instead of PAGE_SIZE_4K
Older windows hosts require the hv_sock ring buffer to be defined
using 4K pages. This was achieved by using the symbol PAGE_SIZE_4K
defined specifically for this purpose. But now we have a new symbol
HV_HYP_PAGE_SIZE defined in hyperv-tlfs which can be used for this.
This patch removes the definition of symbol PAGE_SIZE_4K and replaces
its usage with the symbol HV_HYP_PAGE_SIZE. This patch also aligns
sndbuf and rcvbuf to hyper-v specific page size using HV_HYP_PAGE_SIZE
instead of the guest page size(PAGE_SIZE) as hyper-v expects the page
size to be 4K and it might not be the case on ARM64 architecture.
Signed-off-by: Himadri Pandya
---
net/vmw_vsock/hyperv_transport.c | 21 +++--
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
index f2084e3f7aa4..ecb5d72d8010 100644
--- a/net/vmw_vsock/hyperv_transport.c
+++ b/net/vmw_vsock/hyperv_transport.c
@@ -13,15 +13,16 @@
#include
#include
#include
+#include
/* Older (VMBUS version 'VERSION_WIN10' or before) Windows hosts have some
- * stricter requirements on the hv_sock ring buffer size of six 4K pages. Newer
- * hosts don't have this limitation; but, keep the defaults the same for
compat.
+ * stricter requirements on the hv_sock ring buffer size of six 4K pages.
+ * hyperv-tlfs defines HV_HYP_PAGE_SIZE as 4K. Newer hosts don't have this
+ * limitation; but, keep the defaults the same for compat.
*/
-#define PAGE_SIZE_4K 4096
-#define RINGBUFFER_HVS_RCV_SIZE (PAGE_SIZE_4K * 6)
-#define RINGBUFFER_HVS_SND_SIZE (PAGE_SIZE_4K * 6)
-#define RINGBUFFER_HVS_MAX_SIZE (PAGE_SIZE_4K * 64)
+#define RINGBUFFER_HVS_RCV_SIZE (HV_HYP_PAGE_SIZE * 6)
+#define RINGBUFFER_HVS_SND_SIZE (HV_HYP_PAGE_SIZE * 6)
+#define RINGBUFFER_HVS_MAX_SIZE (HV_HYP_PAGE_SIZE * 64)
/* The MTU is 16KB per the host side's design */
#define HVS_MTU_SIZE (1024 * 16)
@@ -54,7 +55,7 @@ struct hvs_recv_buf {
* ringbuffer APIs that allow us to directly copy data from userspace buffer
* to VMBus ringbuffer.
*/
-#define HVS_SEND_BUF_SIZE (PAGE_SIZE_4K - sizeof(struct vmpipe_proto_header))
+#define HVS_SEND_BUF_SIZE (HV_HYP_PAGE_SIZE - sizeof(struct
vmpipe_proto_header))
struct hvs_send_buf {
/* The header before the payload data */
@@ -388,10 +389,10 @@ static void hvs_open_connection(struct vmbus_channel
*chan)
} else {
sndbuf = max_t(int, sk->sk_sndbuf, RINGBUFFER_HVS_SND_SIZE);
sndbuf = min_t(int, sndbuf, RINGBUFFER_HVS_MAX_SIZE);
- sndbuf = ALIGN(sndbuf, PAGE_SIZE);
+ sndbuf = ALIGN(sndbuf, HV_HYP_PAGE_SIZE);
rcvbuf = max_t(int, sk->sk_rcvbuf, RINGBUFFER_HVS_RCV_SIZE);
rcvbuf = min_t(int, rcvbuf, RINGBUFFER_HVS_MAX_SIZE);
- rcvbuf = ALIGN(rcvbuf, PAGE_SIZE);
+ rcvbuf = ALIGN(rcvbuf, HV_HYP_PAGE_SIZE);
}
ret = vmbus_open(chan, sndbuf, rcvbuf, NULL, 0, hvs_channel_cb,
@@ -662,7 +663,7 @@ static ssize_t hvs_stream_enqueue(struct vsock_sock *vsk,
struct msghdr *msg,
ssize_t ret = 0;
ssize_t bytes_written = 0;
- BUILD_BUG_ON(sizeof(*send_buf) != PAGE_SIZE_4K);
+ BUILD_BUG_ON(sizeof(*send_buf) != HV_HYP_PAGE_SIZE);
send_buf = kmalloc(sizeof(*send_buf), GFP_KERNEL);
if (!send_buf)
--
2.17.1
[PATCH 1/2] Drivers: hv: Specify receive buffer size using Hyper-V page size
The recv_buffer is used to retrieve data from the VMbus ring buffer.
VMbus ring buffers are sized based on the guest page size which
Hyper-V assumes to be 4KB. But it may be different on some
architectures. So use the Hyper-V page size to allocate the
recv_buffer and set the maximum size to receive.
Signed-off-by: Himadri Pandya
---
drivers/hv/hv_fcopy.c| 3 ++-
drivers/hv/hv_kvp.c | 3 ++-
drivers/hv/hv_snapshot.c | 3 ++-
drivers/hv/hv_util.c | 8
4 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c
index 7e30ae0635cc..08fa4a5de644 100644
--- a/drivers/hv/hv_fcopy.c
+++ b/drivers/hv/hv_fcopy.c
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include "hyperv_vmbus.h"
#include "hv_utils_transport.h"
@@ -234,7 +235,7 @@ void hv_fcopy_onchannelcallback(void *context)
if (fcopy_transaction.state > HVUTIL_READY)
return;
- vmbus_recvpacket(channel, recv_buffer, PAGE_SIZE * 2, ,
+ vmbus_recvpacket(channel, recv_buffer, HV_HYP_PAGE_SIZE * 2, ,
);
if (recvlen <= 0)
return;
diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
index 5054d1105236..ae7c028dc5a8 100644
--- a/drivers/hv/hv_kvp.c
+++ b/drivers/hv/hv_kvp.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include "hyperv_vmbus.h"
#include "hv_utils_transport.h"
@@ -661,7 +662,7 @@ void hv_kvp_onchannelcallback(void *context)
if (kvp_transaction.state > HVUTIL_READY)
return;
- vmbus_recvpacket(channel, recv_buffer, PAGE_SIZE * 4, ,
+ vmbus_recvpacket(channel, recv_buffer, HV_HYP_PAGE_SIZE * 4, ,
);
if (recvlen > 0) {
diff --git a/drivers/hv/hv_snapshot.c b/drivers/hv/hv_snapshot.c
index 20ba95b75a94..03b6454268b3 100644
--- a/drivers/hv/hv_snapshot.c
+++ b/drivers/hv/hv_snapshot.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include "hyperv_vmbus.h"
#include "hv_utils_transport.h"
@@ -297,7 +298,7 @@ void hv_vss_onchannelcallback(void *context)
if (vss_transaction.state > HVUTIL_READY)
return;
- vmbus_recvpacket(channel, recv_buffer, PAGE_SIZE * 2, ,
+ vmbus_recvpacket(channel, recv_buffer, HV_HYP_PAGE_SIZE * 2, ,
);
if (recvlen > 0) {
diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index e32681ee7b9f..c2c08f26bd5f 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -136,7 +136,7 @@ static void shutdown_onchannelcallback(void *context)
struct icmsg_hdr *icmsghdrp;
vmbus_recvpacket(channel, shut_txf_buf,
-PAGE_SIZE, , );
+HV_HYP_PAGE_SIZE, , );
if (recvlen > 0) {
icmsghdrp = (struct icmsg_hdr *)_txf_buf[
@@ -284,7 +284,7 @@ static void timesync_onchannelcallback(void *context)
u8 *time_txf_buf = util_timesynch.recv_buffer;
vmbus_recvpacket(channel, time_txf_buf,
-PAGE_SIZE, , );
+HV_HYP_PAGE_SIZE, , );
if (recvlen > 0) {
icmsghdrp = (struct icmsg_hdr *)_txf_buf[
@@ -346,7 +346,7 @@ static void heartbeat_onchannelcallback(void *context)
while (1) {
vmbus_recvpacket(channel, hbeat_txf_buf,
-PAGE_SIZE, , );
+HV_HYP_PAGE_SIZE, , );
if (!recvlen)
break;
@@ -390,7 +390,7 @@ static int util_probe(struct hv_device *dev,
(struct hv_util_service *)dev_id->driver_data;
int ret;
- srv->recv_buffer = kmalloc(PAGE_SIZE * 4, GFP_KERNEL);
+ srv->recv_buffer = kmalloc(HV_HYP_PAGE_SIZE * 4, GFP_KERNEL);
if (!srv->recv_buffer)
return -ENOMEM;
srv->channel = dev->channel;
--
2.17.1
[PATCH 2/2] Drivers: hv: util: Specify ring buffer size using Hyper-V page size
VMbus ring buffers are sized based on the 4K page size used by Hyper-V. The Linux guest page size may not be 4K on all architectures so use the Hyper-V page size to specify the ring buffer size. Signed-off-by: Himadri Pandya --- drivers/hv/hv_util.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c index c2c08f26bd5f..766bd8457346 100644 --- a/drivers/hv/hv_util.c +++ b/drivers/hv/hv_util.c @@ -413,8 +413,9 @@ static int util_probe(struct hv_device *dev, hv_set_drvdata(dev, srv); - ret = vmbus_open(dev->channel, 4 * PAGE_SIZE, 4 * PAGE_SIZE, NULL, 0, - srv->util_cb, dev->channel); + ret = vmbus_open(dev->channel, 4 * HV_HYP_PAGE_SIZE, +4 * HV_HYP_PAGE_SIZE, NULL, 0, srv->util_cb, +dev->channel); if (ret) goto error; -- 2.17.1
[PATCH 0/2] Drivers: hv: Specify buffer size using Hyper-V page size
recv_buffer and VMbus ring buffers are sized based on guest page size which Hyper-V assumes to be 4KB. It might not be the case for some architectures. Hence instead use the Hyper-V page size. Himadri Pandya (2): Drivers: hv: Specify receive buffer size using Hyper-V page size Drivers: hv: util: Specify ring buffer size using Hyper-V page size drivers/hv/hv_fcopy.c| 3 ++- drivers/hv/hv_kvp.c | 3 ++- drivers/hv/hv_snapshot.c | 3 ++- drivers/hv/hv_util.c | 13 +++-- 4 files changed, 13 insertions(+), 9 deletions(-) -- 2.17.1
[PATCH] staging: wlan-ng: Fix improper SPDX comment style
The SPDX license identifier should have the form // SPDX-License-Identifier: for a .c source file. File hfa384x_usb.c has instead the form /* SPDX-License-Identifier: */ which is the form for C header files. Hence this patch corrects it. Issue identified by checkpatch. Signed-off-by: Himadri Pandya --- drivers/staging/wlan-ng/hfa384x_usb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/wlan-ng/hfa384x_usb.c b/drivers/staging/wlan-ng/hfa384x_usb.c index 81a6b0324641..6fde75d4f064 100644 --- a/drivers/staging/wlan-ng/hfa384x_usb.c +++ b/drivers/staging/wlan-ng/hfa384x_usb.c @@ -1,4 +1,4 @@ -/* SPDX-License-Identifier: (GPL-2.0 OR MPL-1.1) */ +// SPDX-License-Identifier: (GPL-2.0 OR MPL-1.1) /* src/prism2/driver/hfa384x_usb.c * * Functions that talk to the USB variant of the Intersil hfa384x MAC -- 2.17.1
Re: [PATCH] staging: fbtft: fix alignment should match open parenthesis
On 10/04/19 4:12 PM, Dan Carpenter wrote: On Wed, Apr 10, 2019 at 03:30:22PM +0530, Himadri Pandya wrote: Fix checkpatch warning "Alignment should match open parenthesis". Signed-off-by: Himadri Pandya --- drivers/staging/fbtft/fb_tinylcd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/fbtft/fb_tinylcd.c b/drivers/staging/fbtft/fb_tinylcd.c index 9469248f2c50..60cda57bcb33 100644 --- a/drivers/staging/fbtft/fb_tinylcd.c +++ b/drivers/staging/fbtft/fb_tinylcd.c @@ -38,7 +38,7 @@ static int init_display(struct fbtft_par *par) write_reg(par, 0xE5, 0x00); write_reg(par, 0xF0, 0x36, 0xA5, 0x53); write_reg(par, 0xE0, 0x00, 0x35, 0x33, 0x00, 0x00, 0x00, - 0x00, 0x35, 0x33, 0x00, 0x00, 0x00); + 0x00, 0x35, 0x33, 0x00, 0x00, 0x00); The original code was written that way deliberately. I don't know why it's broken up like that but it probably means something to people who have read the hardware spec. I would leave it as-is. regards, dan carpenter Okay. Thank you for reviewing it. - Himadri
[PATCH] staging: gasket: replace symbolic permissions with octal permissions
Resolve checkpatch warning for using symbolic permissions by replacing
them with octal permissions.
Signed-off-by: Himadri Pandya
---
drivers/staging/gasket/gasket_sysfs.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/gasket/gasket_sysfs.h
b/drivers/staging/gasket/gasket_sysfs.h
index 1d0eed66a7f4..28dc422d388c 100644
--- a/drivers/staging/gasket/gasket_sysfs.h
+++ b/drivers/staging/gasket/gasket_sysfs.h
@@ -75,7 +75,7 @@ struct gasket_sysfs_attribute {
#define GASKET_SYSFS_RO(_name, _show_function, _attr_type)
\
{ \
- .attr = __ATTR(_name, S_IRUGO, _show_function, NULL), \
+ .attr = __ATTR(_name, 0444, _show_function, NULL), \
.data.attr_type = _attr_type \
}
--
2.17.1
[PATCH] staging: fbtft: fix alignment should match open parenthesis
Fix checkpatch warning "Alignment should match open parenthesis". Signed-off-by: Himadri Pandya --- drivers/staging/fbtft/fb_tinylcd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/fbtft/fb_tinylcd.c b/drivers/staging/fbtft/fb_tinylcd.c index 9469248f2c50..60cda57bcb33 100644 --- a/drivers/staging/fbtft/fb_tinylcd.c +++ b/drivers/staging/fbtft/fb_tinylcd.c @@ -38,7 +38,7 @@ static int init_display(struct fbtft_par *par) write_reg(par, 0xE5, 0x00); write_reg(par, 0xF0, 0x36, 0xA5, 0x53); write_reg(par, 0xE0, 0x00, 0x35, 0x33, 0x00, 0x00, 0x00, - 0x00, 0x35, 0x33, 0x00, 0x00, 0x00); + 0x00, 0x35, 0x33, 0x00, 0x00, 0x00); write_reg(par, MIPI_DCS_SET_PIXEL_FORMAT, 0x55); write_reg(par, MIPI_DCS_EXIT_SLEEP_MODE); udelay(250); -- 2.17.1
Re: [PATCH net-next] net: dsa: add missing of_node_put
On 22/02/19 8:06 PM, Andrew Lunn wrote: On Fri, Feb 22, 2019 at 04:48:18PM +0530, Himadri Pandya wrote: Decrement the reference count on port while returning out of the loop. Issue identified by Coccinelle. You and Wen Yang are both fixing the same issue. Maybe you can coordinate? Sure. - Himadri Andrew
[PATCH net-next] net: dsa: add missing of_node_put
Decrement the reference count on port while returning out of the
loop. Issue identified by Coccinelle.
Signed-off-by: Himadri Pandya
---
net/dsa/dsa2.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index a1917025e155..396e7433dd8f 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -624,19 +624,25 @@ static int dsa_switch_parse_ports_of(struct dsa_switch
*ds,
for_each_available_child_of_node(ports, port) {
err = of_property_read_u32(port, "reg", );
if (err)
- return err;
+ goto put_port;
- if (reg >= ds->num_ports)
- return -EINVAL;
+ if (reg >= ds->num_ports) {
+ err = -EINVAL;
+ goto put_port;
+ }
dp = >ports[reg];
err = dsa_port_parse_of(dp, port);
if (err)
- return err;
+ goto put_port;
}
return 0;
+
+put_port:
+ of_node_put(port);
+ return err;
}
static int dsa_switch_parse_member_of(struct dsa_switch *ds,
--
2.17.1
Re: [Outreachy kernel] [PATCH] net: dsa: add missing of_node_put
On 20/02/19 9:23 AM, Vaishali Thakkar wrote:
On Wed, Feb 20, 2019 at 8:54 AM Himadri Pandya wrote:
Hi Himadri,
Thanks for the patch!
For the scope of Outreachy, we prefer that you send patches in staging
directory as Greg makes sure to pick them during the application
period. Of course, you're very much encouraged to contribute to other
subsystems as well but there patches are mainly picked up based on
maintainer's cycle which may or may not be picked up for linux-next when
selection of interns happens.
I hope that makes sense.
Understood. Thank you for letting me know that.
Decrement the reference count on port while returning out of the loop.
How did you find out about this issue?
I believe that Julia Lawall has been working on this for a while. After
doing some cleanup patches, I'm trying to continue the work with her help.
I think it would be good to
give credit to tool in commit log if the issue is identified or produced
by tool. [In this case, I assume it's Coccinelle]
Yes, it was identified by Coccinelle and I should include it in the
commit message. Thank you for the remark. I'll revise the patch accordingly.
- Himadri
Signed-off-by: Himadri Pandya
---
net/dsa/dsa2.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index a1917025e155..396e7433dd8f 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -624,19 +624,25 @@ static int dsa_switch_parse_ports_of(struct dsa_switch
*ds,
for_each_available_child_of_node(ports, port) {
err = of_property_read_u32(port, "reg", );
if (err)
- return err;
+ goto put_port;
- if (reg >= ds->num_ports)
- return -EINVAL;
+ if (reg >= ds->num_ports) {
+ err = -EINVAL;
+ goto put_port;
+ }
dp = >ports[reg];
err = dsa_port_parse_of(dp, port);
if (err)
- return err;
+ goto put_port;
}
return 0;
+
+put_port:
+ of_node_put(port);
+ return err;
}
static int dsa_switch_parse_member_of(struct dsa_switch *ds,
--
2.17.1
--
You received this message because you are subscribed to the Google Groups
"outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to outreachy-kernel+unsubscr...@googlegroups.com.
To post to this group, send email to outreachy-ker...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/outreachy-kernel/20190220032432.2878-1-himadri18.07%40gmail.com.
For more options, visit https://groups.google.com/d/optout.
[PATCH] net: dsa: add missing of_node_put
Decrement the reference count on port while returning out of the loop.
Signed-off-by: Himadri Pandya
---
net/dsa/dsa2.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index a1917025e155..396e7433dd8f 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -624,19 +624,25 @@ static int dsa_switch_parse_ports_of(struct dsa_switch
*ds,
for_each_available_child_of_node(ports, port) {
err = of_property_read_u32(port, "reg", );
if (err)
- return err;
+ goto put_port;
- if (reg >= ds->num_ports)
- return -EINVAL;
+ if (reg >= ds->num_ports) {
+ err = -EINVAL;
+ goto put_port;
+ }
dp = >ports[reg];
err = dsa_port_parse_of(dp, port);
if (err)
- return err;
+ goto put_port;
}
return 0;
+
+put_port:
+ of_node_put(port);
+ return err;
}
static int dsa_switch_parse_member_of(struct dsa_switch *ds,
--
2.17.1
