* [PATCH 1/3] net: can: kvaser_usb: fix reception on "USBcan Pro" and "USBcan R" type hardware.
2013-06-03 12:15 pull-request: can 2013-06-03 Marc Kleine-Budde
@ 2013-06-03 12:15 ` Marc Kleine-Budde
2013-06-03 12:15 ` [PATCH 2/3] net: can: esd_usb2: Do not do dma on the stack Marc Kleine-Budde
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Marc Kleine-Budde @ 2013-06-03 12:15 UTC (permalink / raw)
To: netdev
Cc: linux-can, davem, Jonas Peterson, stable, Olivier Sobrie,
Marc Kleine-Budde
From: Jonas Peterson <jonas.peterson@gmail.com>
Unlike Kvaser Leaf light devices, some other Kvaser devices (like USBcan
Pro, USBcan R) receive CAN messages in CMD_LOG_MESSAGE frames. This
patch adds support for it.
Cc: linux-stable <stable@vger.kernel.org> # >= v3.8
Signed-off-by: Jonas Peterson <jonas.peterson@gmail.com>
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/usb/kvaser_usb.c | 64 +++++++++++++++++++++++++++-------------
1 file changed, 43 insertions(+), 21 deletions(-)
diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index 45cb9f3..3b95465 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -136,6 +136,9 @@
#define KVASER_CTRL_MODE_SELFRECEPTION 3
#define KVASER_CTRL_MODE_OFF 4
+/* log message */
+#define KVASER_EXTENDED_FRAME BIT(31)
+
struct kvaser_msg_simple {
u8 tid;
u8 channel;
@@ -817,8 +820,13 @@ static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
priv = dev->nets[channel];
stats = &priv->netdev->stats;
- if (msg->u.rx_can.flag & (MSG_FLAG_ERROR_FRAME | MSG_FLAG_NERR |
- MSG_FLAG_OVERRUN)) {
+ if ((msg->u.rx_can.flag & MSG_FLAG_ERROR_FRAME) &&
+ (msg->id == CMD_LOG_MESSAGE)) {
+ kvaser_usb_rx_error(dev, msg);
+ return;
+ } else if (msg->u.rx_can.flag & (MSG_FLAG_ERROR_FRAME |
+ MSG_FLAG_NERR |
+ MSG_FLAG_OVERRUN)) {
kvaser_usb_rx_can_err(priv, msg);
return;
} else if (msg->u.rx_can.flag & ~MSG_FLAG_REMOTE_FRAME) {
@@ -834,22 +842,40 @@ static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
return;
}
- cf->can_id = ((msg->u.rx_can.msg[0] & 0x1f) << 6) |
- (msg->u.rx_can.msg[1] & 0x3f);
- cf->can_dlc = get_can_dlc(msg->u.rx_can.msg[5]);
+ if (msg->id == CMD_LOG_MESSAGE) {
+ cf->can_id = le32_to_cpu(msg->u.log_message.id);
+ if (cf->can_id & KVASER_EXTENDED_FRAME)
+ cf->can_id &= CAN_EFF_MASK | CAN_EFF_FLAG;
+ else
+ cf->can_id &= CAN_SFF_MASK;
- if (msg->id == CMD_RX_EXT_MESSAGE) {
- cf->can_id <<= 18;
- cf->can_id |= ((msg->u.rx_can.msg[2] & 0x0f) << 14) |
- ((msg->u.rx_can.msg[3] & 0xff) << 6) |
- (msg->u.rx_can.msg[4] & 0x3f);
- cf->can_id |= CAN_EFF_FLAG;
- }
+ cf->can_dlc = get_can_dlc(msg->u.log_message.dlc);
- if (msg->u.rx_can.flag & MSG_FLAG_REMOTE_FRAME)
- cf->can_id |= CAN_RTR_FLAG;
- else
- memcpy(cf->data, &msg->u.rx_can.msg[6], cf->can_dlc);
+ if (msg->u.log_message.flags & MSG_FLAG_REMOTE_FRAME)
+ cf->can_id |= CAN_RTR_FLAG;
+ else
+ memcpy(cf->data, &msg->u.log_message.data,
+ cf->can_dlc);
+ } else {
+ cf->can_id = ((msg->u.rx_can.msg[0] & 0x1f) << 6) |
+ (msg->u.rx_can.msg[1] & 0x3f);
+
+ if (msg->id == CMD_RX_EXT_MESSAGE) {
+ cf->can_id <<= 18;
+ cf->can_id |= ((msg->u.rx_can.msg[2] & 0x0f) << 14) |
+ ((msg->u.rx_can.msg[3] & 0xff) << 6) |
+ (msg->u.rx_can.msg[4] & 0x3f);
+ cf->can_id |= CAN_EFF_FLAG;
+ }
+
+ cf->can_dlc = get_can_dlc(msg->u.rx_can.msg[5]);
+
+ if (msg->u.rx_can.flag & MSG_FLAG_REMOTE_FRAME)
+ cf->can_id |= CAN_RTR_FLAG;
+ else
+ memcpy(cf->data, &msg->u.rx_can.msg[6],
+ cf->can_dlc);
+ }
netif_rx(skb);
@@ -911,6 +937,7 @@ static void kvaser_usb_handle_message(const struct kvaser_usb *dev,
case CMD_RX_STD_MESSAGE:
case CMD_RX_EXT_MESSAGE:
+ case CMD_LOG_MESSAGE:
kvaser_usb_rx_can_msg(dev, msg);
break;
@@ -919,11 +946,6 @@ static void kvaser_usb_handle_message(const struct kvaser_usb *dev,
kvaser_usb_rx_error(dev, msg);
break;
- case CMD_LOG_MESSAGE:
- if (msg->u.log_message.flags & MSG_FLAG_ERROR_FRAME)
- kvaser_usb_rx_error(dev, msg);
- break;
-
case CMD_TX_ACKNOWLEDGE:
kvaser_usb_tx_acknowledge(dev, msg);
break;
--
1.8.2.rc2
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/3] net: can: esd_usb2: Do not do dma on the stack
2013-06-03 12:15 pull-request: can 2013-06-03 Marc Kleine-Budde
2013-06-03 12:15 ` [PATCH 1/3] net: can: kvaser_usb: fix reception on "USBcan Pro" and "USBcan R" type hardware Marc Kleine-Budde
@ 2013-06-03 12:15 ` Marc Kleine-Budde
2013-06-03 12:15 ` [PATCH 3/3] net: can: peak_usb: " Marc Kleine-Budde
2013-06-04 21:31 ` pull-request: can 2013-06-03 David Miller
3 siblings, 0 replies; 7+ messages in thread
From: Marc Kleine-Budde @ 2013-06-03 12:15 UTC (permalink / raw)
To: netdev; +Cc: linux-can, davem, Olivier Sobrie, Matthias Fuchs,
Marc Kleine-Budde
From: Olivier Sobrie <olivier@sobrie.be>
smatch reports the following warnings:
drivers/net/can/usb/esd_usb2.c:640 esd_usb2_start() error: doing dma on the stack (&msg)
drivers/net/can/usb/esd_usb2.c:846 esd_usb2_close() error: doing dma on the stack (&msg)
drivers/net/can/usb/esd_usb2.c:855 esd_usb2_close() error: doing dma on the stack (&msg)
drivers/net/can/usb/esd_usb2.c:923 esd_usb2_set_bittiming() error: doing dma on the stack (&msg)
drivers/net/can/usb/esd_usb2.c:1047 esd_usb2_probe() error: doing dma on the stack (&msg)
drivers/net/can/usb/esd_usb2.c:1053 esd_usb2_probe() error: doing dma on the stack (&msg)
See "Documentation/DMA-API-HOWTO.txt" section "What memory is DMA'able?"
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
Cc: Matthias Fuchs <matthias.fuchs@esd.eu>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/usb/esd_usb2.c | 127 ++++++++++++++++++++++++-----------------
1 file changed, 76 insertions(+), 51 deletions(-)
diff --git a/drivers/net/can/usb/esd_usb2.c b/drivers/net/can/usb/esd_usb2.c
index 9b74d1e..6aa7b32 100644
--- a/drivers/net/can/usb/esd_usb2.c
+++ b/drivers/net/can/usb/esd_usb2.c
@@ -612,9 +612,15 @@ static int esd_usb2_start(struct esd_usb2_net_priv *priv)
{
struct esd_usb2 *dev = priv->usb2;
struct net_device *netdev = priv->netdev;
- struct esd_usb2_msg msg;
+ struct esd_usb2_msg *msg;
int err, i;
+ msg = kmalloc(sizeof(*msg), GFP_KERNEL);
+ if (!msg) {
+ err = -ENOMEM;
+ goto out;
+ }
+
/*
* Enable all IDs
* The IDADD message takes up to 64 32 bit bitmasks (2048 bits).
@@ -628,33 +634,32 @@ static int esd_usb2_start(struct esd_usb2_net_priv *priv)
* the number of the starting bitmask (0..64) to the filter.option
* field followed by only some bitmasks.
*/
- msg.msg.hdr.cmd = CMD_IDADD;
- msg.msg.hdr.len = 2 + ESD_MAX_ID_SEGMENT;
- msg.msg.filter.net = priv->index;
- msg.msg.filter.option = ESD_ID_ENABLE; /* start with segment 0 */
+ msg->msg.hdr.cmd = CMD_IDADD;
+ msg->msg.hdr.len = 2 + ESD_MAX_ID_SEGMENT;
+ msg->msg.filter.net = priv->index;
+ msg->msg.filter.option = ESD_ID_ENABLE; /* start with segment 0 */
for (i = 0; i < ESD_MAX_ID_SEGMENT; i++)
- msg.msg.filter.mask[i] = cpu_to_le32(0xffffffff);
+ msg->msg.filter.mask[i] = cpu_to_le32(0xffffffff);
/* enable 29bit extended IDs */
- msg.msg.filter.mask[ESD_MAX_ID_SEGMENT] = cpu_to_le32(0x00000001);
+ msg->msg.filter.mask[ESD_MAX_ID_SEGMENT] = cpu_to_le32(0x00000001);
- err = esd_usb2_send_msg(dev, &msg);
+ err = esd_usb2_send_msg(dev, msg);
if (err)
- goto failed;
+ goto out;
err = esd_usb2_setup_rx_urbs(dev);
if (err)
- goto failed;
+ goto out;
priv->can.state = CAN_STATE_ERROR_ACTIVE;
- return 0;
-
-failed:
+out:
if (err == -ENODEV)
netif_device_detach(netdev);
+ if (err)
+ netdev_err(netdev, "couldn't start device: %d\n", err);
- netdev_err(netdev, "couldn't start device: %d\n", err);
-
+ kfree(msg);
return err;
}
@@ -833,26 +838,30 @@ nourbmem:
static int esd_usb2_close(struct net_device *netdev)
{
struct esd_usb2_net_priv *priv = netdev_priv(netdev);
- struct esd_usb2_msg msg;
+ struct esd_usb2_msg *msg;
int i;
+ msg = kmalloc(sizeof(*msg), GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
/* Disable all IDs (see esd_usb2_start()) */
- msg.msg.hdr.cmd = CMD_IDADD;
- msg.msg.hdr.len = 2 + ESD_MAX_ID_SEGMENT;
- msg.msg.filter.net = priv->index;
- msg.msg.filter.option = ESD_ID_ENABLE; /* start with segment 0 */
+ msg->msg.hdr.cmd = CMD_IDADD;
+ msg->msg.hdr.len = 2 + ESD_MAX_ID_SEGMENT;
+ msg->msg.filter.net = priv->index;
+ msg->msg.filter.option = ESD_ID_ENABLE; /* start with segment 0 */
for (i = 0; i <= ESD_MAX_ID_SEGMENT; i++)
- msg.msg.filter.mask[i] = 0;
- if (esd_usb2_send_msg(priv->usb2, &msg) < 0)
+ msg->msg.filter.mask[i] = 0;
+ if (esd_usb2_send_msg(priv->usb2, msg) < 0)
netdev_err(netdev, "sending idadd message failed\n");
/* set CAN controller to reset mode */
- msg.msg.hdr.len = 2;
- msg.msg.hdr.cmd = CMD_SETBAUD;
- msg.msg.setbaud.net = priv->index;
- msg.msg.setbaud.rsvd = 0;
- msg.msg.setbaud.baud = cpu_to_le32(ESD_USB2_NO_BAUDRATE);
- if (esd_usb2_send_msg(priv->usb2, &msg) < 0)
+ msg->msg.hdr.len = 2;
+ msg->msg.hdr.cmd = CMD_SETBAUD;
+ msg->msg.setbaud.net = priv->index;
+ msg->msg.setbaud.rsvd = 0;
+ msg->msg.setbaud.baud = cpu_to_le32(ESD_USB2_NO_BAUDRATE);
+ if (esd_usb2_send_msg(priv->usb2, msg) < 0)
netdev_err(netdev, "sending setbaud message failed\n");
priv->can.state = CAN_STATE_STOPPED;
@@ -861,6 +870,8 @@ static int esd_usb2_close(struct net_device *netdev)
close_candev(netdev);
+ kfree(msg);
+
return 0;
}
@@ -886,7 +897,8 @@ static int esd_usb2_set_bittiming(struct net_device *netdev)
{
struct esd_usb2_net_priv *priv = netdev_priv(netdev);
struct can_bittiming *bt = &priv->can.bittiming;
- struct esd_usb2_msg msg;
+ struct esd_usb2_msg *msg;
+ int err;
u32 canbtr;
int sjw_shift;
@@ -912,15 +924,22 @@ static int esd_usb2_set_bittiming(struct net_device *netdev)
if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
canbtr |= ESD_USB2_3_SAMPLES;
- msg.msg.hdr.len = 2;
- msg.msg.hdr.cmd = CMD_SETBAUD;
- msg.msg.setbaud.net = priv->index;
- msg.msg.setbaud.rsvd = 0;
- msg.msg.setbaud.baud = cpu_to_le32(canbtr);
+ msg = kmalloc(sizeof(*msg), GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ msg->msg.hdr.len = 2;
+ msg->msg.hdr.cmd = CMD_SETBAUD;
+ msg->msg.setbaud.net = priv->index;
+ msg->msg.setbaud.rsvd = 0;
+ msg->msg.setbaud.baud = cpu_to_le32(canbtr);
netdev_info(netdev, "setting BTR=%#x\n", canbtr);
- return esd_usb2_send_msg(priv->usb2, &msg);
+ err = esd_usb2_send_msg(priv->usb2, msg);
+
+ kfree(msg);
+ return err;
}
static int esd_usb2_get_berr_counter(const struct net_device *netdev,
@@ -1022,7 +1041,7 @@ static int esd_usb2_probe(struct usb_interface *intf,
const struct usb_device_id *id)
{
struct esd_usb2 *dev;
- struct esd_usb2_msg msg;
+ struct esd_usb2_msg *msg;
int i, err;
dev = kzalloc(sizeof(*dev), GFP_KERNEL);
@@ -1037,27 +1056,33 @@ static int esd_usb2_probe(struct usb_interface *intf,
usb_set_intfdata(intf, dev);
+ msg = kmalloc(sizeof(*msg), GFP_KERNEL);
+ if (!msg) {
+ err = -ENOMEM;
+ goto free_msg;
+ }
+
/* query number of CAN interfaces (nets) */
- msg.msg.hdr.cmd = CMD_VERSION;
- msg.msg.hdr.len = 2;
- msg.msg.version.rsvd = 0;
- msg.msg.version.flags = 0;
- msg.msg.version.drv_version = 0;
+ msg->msg.hdr.cmd = CMD_VERSION;
+ msg->msg.hdr.len = 2;
+ msg->msg.version.rsvd = 0;
+ msg->msg.version.flags = 0;
+ msg->msg.version.drv_version = 0;
- err = esd_usb2_send_msg(dev, &msg);
+ err = esd_usb2_send_msg(dev, msg);
if (err < 0) {
dev_err(&intf->dev, "sending version message failed\n");
- goto free_dev;
+ goto free_msg;
}
- err = esd_usb2_wait_msg(dev, &msg);
+ err = esd_usb2_wait_msg(dev, msg);
if (err < 0) {
dev_err(&intf->dev, "no version message answer\n");
- goto free_dev;
+ goto free_msg;
}
- dev->net_count = (int)msg.msg.version_reply.nets;
- dev->version = le32_to_cpu(msg.msg.version_reply.version);
+ dev->net_count = (int)msg->msg.version_reply.nets;
+ dev->version = le32_to_cpu(msg->msg.version_reply.version);
if (device_create_file(&intf->dev, &dev_attr_firmware))
dev_err(&intf->dev,
@@ -1075,10 +1100,10 @@ static int esd_usb2_probe(struct usb_interface *intf,
for (i = 0; i < dev->net_count; i++)
esd_usb2_probe_one_net(intf, i);
- return 0;
-
-free_dev:
- kfree(dev);
+free_msg:
+ kfree(msg);
+ if (err)
+ kfree(dev);
done:
return err;
}
--
1.8.2.rc2
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 3/3] net: can: peak_usb: Do not do dma on the stack
2013-06-03 12:15 pull-request: can 2013-06-03 Marc Kleine-Budde
2013-06-03 12:15 ` [PATCH 1/3] net: can: kvaser_usb: fix reception on "USBcan Pro" and "USBcan R" type hardware Marc Kleine-Budde
2013-06-03 12:15 ` [PATCH 2/3] net: can: esd_usb2: Do not do dma on the stack Marc Kleine-Budde
@ 2013-06-03 12:15 ` Marc Kleine-Budde
2013-12-12 14:44 ` Stephane Grosjean
2013-06-04 21:31 ` pull-request: can 2013-06-03 David Miller
3 siblings, 1 reply; 7+ messages in thread
From: Marc Kleine-Budde @ 2013-06-03 12:15 UTC (permalink / raw)
To: netdev; +Cc: linux-can, davem, Marc Kleine-Budde, Stephane Grosjean
smatch reports the following warnings:
drivers/net/can/usb/peak_usb/pcan_usb_pro.c:514 pcan_usb_pro_drv_loaded() error: doing dma on the stack (buffer)
drivers/net/can/usb/peak_usb/pcan_usb_pro.c:878 pcan_usb_pro_init() error: doing dma on the stack (&fi)
drivers/net/can/usb/peak_usb/pcan_usb_pro.c:889 pcan_usb_pro_init() error: doing dma on the stack (&bi)
See "Documentation/DMA-API-HOWTO.txt" section "What memory is DMA'able?"
Cc: Stephane Grosjean <s.grosjean@peak-system.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/usb/peak_usb/pcan_usb_pro.c | 61 +++++++++++++++++++----------
drivers/net/can/usb/peak_usb/pcan_usb_pro.h | 1 +
2 files changed, 41 insertions(+), 21 deletions(-)
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
index 30d79bf..8ee9d15 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
@@ -504,15 +504,24 @@ static int pcan_usb_pro_restart_async(struct peak_usb_device *dev,
return usb_submit_urb(urb, GFP_ATOMIC);
}
-static void pcan_usb_pro_drv_loaded(struct peak_usb_device *dev, int loaded)
+static int pcan_usb_pro_drv_loaded(struct peak_usb_device *dev, int loaded)
{
- u8 buffer[16];
+ u8 *buffer;
+ int err;
+
+ buffer = kmalloc(PCAN_USBPRO_FCT_DRVLD_REQ_LEN, GFP_KERNEL);
+ if (!buffer)
+ return -ENOMEM;
buffer[0] = 0;
buffer[1] = !!loaded;
- pcan_usb_pro_send_req(dev, PCAN_USBPRO_REQ_FCT,
- PCAN_USBPRO_FCT_DRVLD, buffer, sizeof(buffer));
+ err = pcan_usb_pro_send_req(dev, PCAN_USBPRO_REQ_FCT,
+ PCAN_USBPRO_FCT_DRVLD, buffer,
+ PCAN_USBPRO_FCT_DRVLD_REQ_LEN);
+ kfree(buffer);
+
+ return err;
}
static inline
@@ -851,21 +860,24 @@ static int pcan_usb_pro_stop(struct peak_usb_device *dev)
*/
static int pcan_usb_pro_init(struct peak_usb_device *dev)
{
- struct pcan_usb_pro_interface *usb_if;
struct pcan_usb_pro_device *pdev =
container_of(dev, struct pcan_usb_pro_device, dev);
+ struct pcan_usb_pro_interface *usb_if = NULL;
+ struct pcan_usb_pro_fwinfo *fi = NULL;
+ struct pcan_usb_pro_blinfo *bi = NULL;
+ int err;
/* do this for 1st channel only */
if (!dev->prev_siblings) {
- struct pcan_usb_pro_fwinfo fi;
- struct pcan_usb_pro_blinfo bi;
- int err;
-
/* allocate netdevices common structure attached to first one */
usb_if = kzalloc(sizeof(struct pcan_usb_pro_interface),
GFP_KERNEL);
- if (!usb_if)
- return -ENOMEM;
+ fi = kmalloc(sizeof(struct pcan_usb_pro_fwinfo), GFP_KERNEL);
+ bi = kmalloc(sizeof(struct pcan_usb_pro_blinfo), GFP_KERNEL);
+ if (!usb_if || !fi || !bi) {
+ err = -ENOMEM;
+ goto err_out;
+ }
/* number of ts msgs to ignore before taking one into account */
usb_if->cm_ignore_count = 5;
@@ -877,34 +889,34 @@ static int pcan_usb_pro_init(struct peak_usb_device *dev)
*/
err = pcan_usb_pro_send_req(dev, PCAN_USBPRO_REQ_INFO,
PCAN_USBPRO_INFO_FW,
- &fi, sizeof(fi));
+ fi, sizeof(*fi));
if (err) {
- kfree(usb_if);
dev_err(dev->netdev->dev.parent,
"unable to read %s firmware info (err %d)\n",
pcan_usb_pro.name, err);
- return err;
+ goto err_out;
}
err = pcan_usb_pro_send_req(dev, PCAN_USBPRO_REQ_INFO,
PCAN_USBPRO_INFO_BL,
- &bi, sizeof(bi));
+ bi, sizeof(*bi));
if (err) {
- kfree(usb_if);
dev_err(dev->netdev->dev.parent,
"unable to read %s bootloader info (err %d)\n",
pcan_usb_pro.name, err);
- return err;
+ goto err_out;
}
+ /* tell the device the can driver is running */
+ err = pcan_usb_pro_drv_loaded(dev, 1);
+ if (err)
+ goto err_out;
+
dev_info(dev->netdev->dev.parent,
"PEAK-System %s hwrev %u serial %08X.%08X (%u channels)\n",
pcan_usb_pro.name,
- bi.hw_rev, bi.serial_num_hi, bi.serial_num_lo,
+ bi->hw_rev, bi->serial_num_hi, bi->serial_num_lo,
pcan_usb_pro.ctrl_count);
-
- /* tell the device the can driver is running */
- pcan_usb_pro_drv_loaded(dev, 1);
} else {
usb_if = pcan_usb_pro_dev_if(dev->prev_siblings);
}
@@ -916,6 +928,13 @@ static int pcan_usb_pro_init(struct peak_usb_device *dev)
pcan_usb_pro_set_led(dev, 0, 1);
return 0;
+
+ err_out:
+ kfree(bi);
+ kfree(fi);
+ kfree(usb_if);
+
+ return err;
}
static void pcan_usb_pro_exit(struct peak_usb_device *dev)
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_pro.h b/drivers/net/can/usb/peak_usb/pcan_usb_pro.h
index a869918..32275af 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_pro.h
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_pro.h
@@ -29,6 +29,7 @@
/* Vendor Request value for XXX_FCT */
#define PCAN_USBPRO_FCT_DRVLD 5 /* tell device driver is loaded */
+#define PCAN_USBPRO_FCT_DRVLD_REQ_LEN 16
/* PCAN_USBPRO_INFO_BL vendor request record type */
struct __packed pcan_usb_pro_blinfo {
--
1.8.2.rc2
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 3/3] net: can: peak_usb: Do not do dma on the stack
2013-06-03 12:15 ` [PATCH 3/3] net: can: peak_usb: " Marc Kleine-Budde
@ 2013-12-12 14:44 ` Stephane Grosjean
2013-12-14 13:37 ` Marc Kleine-Budde
0 siblings, 1 reply; 7+ messages in thread
From: Stephane Grosjean @ 2013-12-12 14:44 UTC (permalink / raw)
To: Marc Kleine-Budde, netdev; +Cc: linux-can
Hello Marc,
Going back into linux-can peak_usb, and I'm currently having a quick
look to what has changed during these several past months.
I agree with the below patch that fixes the "DMAusage with USB core"
issue, but - maybe I'm wrong - isn't there now amemory leak issue in
"pcan_usb_pro_init()" function below?
I mean, you now allocate "fi" and "bi" memory objects but, AFAIK, they
aren't freed anywhere in the function, don't they?
Regards,
Stéphane
Le 03/06/2013 14:15, Marc Kleine-Budde a écrit :
> smatch reports the following warnings:
> drivers/net/can/usb/peak_usb/pcan_usb_pro.c:514 pcan_usb_pro_drv_loaded() error: doing dma on the stack (buffer)
> drivers/net/can/usb/peak_usb/pcan_usb_pro.c:878 pcan_usb_pro_init() error: doing dma on the stack (&fi)
> drivers/net/can/usb/peak_usb/pcan_usb_pro.c:889 pcan_usb_pro_init() error: doing dma on the stack (&bi)
>
> See "Documentation/DMA-API-HOWTO.txt" section "What memory is DMA'able?"
>
> Cc: Stephane Grosjean <s.grosjean@peak-system.com>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
> drivers/net/can/usb/peak_usb/pcan_usb_pro.c | 61 +++++++++++++++++++----------
> drivers/net/can/usb/peak_usb/pcan_usb_pro.h | 1 +
> 2 files changed, 41 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
> index 30d79bf..8ee9d15 100644
> --- a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
> +++ b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
> @@ -504,15 +504,24 @@ static int pcan_usb_pro_restart_async(struct peak_usb_device *dev,
> return usb_submit_urb(urb, GFP_ATOMIC);
> }
>
> -static void pcan_usb_pro_drv_loaded(struct peak_usb_device *dev, int loaded)
> +static int pcan_usb_pro_drv_loaded(struct peak_usb_device *dev, int loaded)
> {
> - u8 buffer[16];
> + u8 *buffer;
> + int err;
> +
> + buffer = kmalloc(PCAN_USBPRO_FCT_DRVLD_REQ_LEN, GFP_KERNEL);
> + if (!buffer)
> + return -ENOMEM;
>
> buffer[0] = 0;
> buffer[1] = !!loaded;
>
> - pcan_usb_pro_send_req(dev, PCAN_USBPRO_REQ_FCT,
> - PCAN_USBPRO_FCT_DRVLD, buffer, sizeof(buffer));
> + err = pcan_usb_pro_send_req(dev, PCAN_USBPRO_REQ_FCT,
> + PCAN_USBPRO_FCT_DRVLD, buffer,
> + PCAN_USBPRO_FCT_DRVLD_REQ_LEN);
> + kfree(buffer);
> +
> + return err;
> }
>
> static inline
> @@ -851,21 +860,24 @@ static int pcan_usb_pro_stop(struct peak_usb_device *dev)
> */
> static int pcan_usb_pro_init(struct peak_usb_device *dev)
> {
> - struct pcan_usb_pro_interface *usb_if;
> struct pcan_usb_pro_device *pdev =
> container_of(dev, struct pcan_usb_pro_device, dev);
> + struct pcan_usb_pro_interface *usb_if = NULL;
> + struct pcan_usb_pro_fwinfo *fi = NULL;
> + struct pcan_usb_pro_blinfo *bi = NULL;
> + int err;
>
> /* do this for 1st channel only */
> if (!dev->prev_siblings) {
> - struct pcan_usb_pro_fwinfo fi;
> - struct pcan_usb_pro_blinfo bi;
> - int err;
> -
> /* allocate netdevices common structure attached to first one */
> usb_if = kzalloc(sizeof(struct pcan_usb_pro_interface),
> GFP_KERNEL);
> - if (!usb_if)
> - return -ENOMEM;
> + fi = kmalloc(sizeof(struct pcan_usb_pro_fwinfo), GFP_KERNEL);
> + bi = kmalloc(sizeof(struct pcan_usb_pro_blinfo), GFP_KERNEL);
> + if (!usb_if || !fi || !bi) {
> + err = -ENOMEM;
> + goto err_out;
> + }
>
> /* number of ts msgs to ignore before taking one into account */
> usb_if->cm_ignore_count = 5;
> @@ -877,34 +889,34 @@ static int pcan_usb_pro_init(struct peak_usb_device *dev)
> */
> err = pcan_usb_pro_send_req(dev, PCAN_USBPRO_REQ_INFO,
> PCAN_USBPRO_INFO_FW,
> - &fi, sizeof(fi));
> + fi, sizeof(*fi));
> if (err) {
> - kfree(usb_if);
> dev_err(dev->netdev->dev.parent,
> "unable to read %s firmware info (err %d)\n",
> pcan_usb_pro.name, err);
> - return err;
> + goto err_out;
> }
>
> err = pcan_usb_pro_send_req(dev, PCAN_USBPRO_REQ_INFO,
> PCAN_USBPRO_INFO_BL,
> - &bi, sizeof(bi));
> + bi, sizeof(*bi));
> if (err) {
> - kfree(usb_if);
> dev_err(dev->netdev->dev.parent,
> "unable to read %s bootloader info (err %d)\n",
> pcan_usb_pro.name, err);
> - return err;
> + goto err_out;
> }
>
> + /* tell the device the can driver is running */
> + err = pcan_usb_pro_drv_loaded(dev, 1);
> + if (err)
> + goto err_out;
> +
> dev_info(dev->netdev->dev.parent,
> "PEAK-System %s hwrev %u serial %08X.%08X (%u channels)\n",
> pcan_usb_pro.name,
> - bi.hw_rev, bi.serial_num_hi, bi.serial_num_lo,
> + bi->hw_rev, bi->serial_num_hi, bi->serial_num_lo,
> pcan_usb_pro.ctrl_count);
> -
> - /* tell the device the can driver is running */
> - pcan_usb_pro_drv_loaded(dev, 1);
> } else {
> usb_if = pcan_usb_pro_dev_if(dev->prev_siblings);
> }
> @@ -916,6 +928,13 @@ static int pcan_usb_pro_init(struct peak_usb_device *dev)
> pcan_usb_pro_set_led(dev, 0, 1);
>
> return 0;
> +
> + err_out:
> + kfree(bi);
> + kfree(fi);
> + kfree(usb_if);
> +
> + return err;
> }
>
> static void pcan_usb_pro_exit(struct peak_usb_device *dev)
> diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_pro.h b/drivers/net/can/usb/peak_usb/pcan_usb_pro.h
> index a869918..32275af 100644
> --- a/drivers/net/can/usb/peak_usb/pcan_usb_pro.h
> +++ b/drivers/net/can/usb/peak_usb/pcan_usb_pro.h
> @@ -29,6 +29,7 @@
>
> /* Vendor Request value for XXX_FCT */
> #define PCAN_USBPRO_FCT_DRVLD 5 /* tell device driver is loaded */
> +#define PCAN_USBPRO_FCT_DRVLD_REQ_LEN 16
>
> /* PCAN_USBPRO_INFO_BL vendor request record type */
> struct __packed pcan_usb_pro_blinfo {
--
PEAK-System Technik GmbH, Otto-Roehm-Strasse 69, D-64293 Darmstadt
Geschaeftsleitung: A.Gach/U.Wilhelm,St.Nr.:007/241/13586 FA Darmstadt
HRB-9183 Darmstadt, Ust.IdNr.:DE 202220078, WEE-Reg.-Nr.: DE39305391
Tel.+49 (0)6151-817320 / Fax:+49 (0)6151-817329, info@peak-system.com
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 3/3] net: can: peak_usb: Do not do dma on the stack
2013-12-12 14:44 ` Stephane Grosjean
@ 2013-12-14 13:37 ` Marc Kleine-Budde
0 siblings, 0 replies; 7+ messages in thread
From: Marc Kleine-Budde @ 2013-12-14 13:37 UTC (permalink / raw)
To: Stephane Grosjean; +Cc: netdev, linux-can
[-- Attachment #1: Type: text/plain, Size: 4596 bytes --]
On 12/12/2013 03:44 PM, Stephane Grosjean wrote:
> Going back into linux-can peak_usb, and I'm currently having a quick
> look to what has changed during these several past months.
> I agree with the below patch that fixes the "DMAusage with USB core"
> issue, but - maybe I'm wrong - isn't there now amemory leak issue in
> "pcan_usb_pro_init()" function below?
>
> I mean, you now allocate "fi" and "bi" memory objects but, AFAIK, they
> aren't freed anywhere in the function, don't they?
Thanks for pointing out.
[...]
>> b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
>> index 30d79bf..8ee9d15 100644
>> --- a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
>> +++ b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
[...]
>> @@ -851,21 +860,24 @@ static int pcan_usb_pro_stop(struct
>> peak_usb_device *dev)
>> */
>> static int pcan_usb_pro_init(struct peak_usb_device *dev)
>> {
>> - struct pcan_usb_pro_interface *usb_if;
>> struct pcan_usb_pro_device *pdev =
>> container_of(dev, struct pcan_usb_pro_device, dev);
>> + struct pcan_usb_pro_interface *usb_if = NULL;
>> + struct pcan_usb_pro_fwinfo *fi = NULL;
>> + struct pcan_usb_pro_blinfo *bi = NULL;
>> + int err;
>> /* do this for 1st channel only */
>> if (!dev->prev_siblings) {
>> - struct pcan_usb_pro_fwinfo fi;
>> - struct pcan_usb_pro_blinfo bi;
>> - int err;
>> -
>> /* allocate netdevices common structure attached to first
>> one */
>> usb_if = kzalloc(sizeof(struct pcan_usb_pro_interface),
>> GFP_KERNEL);
>> - if (!usb_if)
>> - return -ENOMEM;
>> + fi = kmalloc(sizeof(struct pcan_usb_pro_fwinfo), GFP_KERNEL);
>> + bi = kmalloc(sizeof(struct pcan_usb_pro_blinfo), GFP_KERNEL);
>> + if (!usb_if || !fi || !bi) {
>> + err = -ENOMEM;
>> + goto err_out;
>> + }
>> /* number of ts msgs to ignore before taking one into
>> account */
>> usb_if->cm_ignore_count = 5;
>> @@ -877,34 +889,34 @@ static int pcan_usb_pro_init(struct
>> peak_usb_device *dev)
>> */
>> err = pcan_usb_pro_send_req(dev, PCAN_USBPRO_REQ_INFO,
>> PCAN_USBPRO_INFO_FW,
>> - &fi, sizeof(fi));
>> + fi, sizeof(*fi));
>> if (err) {
>> - kfree(usb_if);
>> dev_err(dev->netdev->dev.parent,
>> "unable to read %s firmware info (err %d)\n",
>> pcan_usb_pro.name, err);
>> - return err;
>> + goto err_out;
>> }
>> err = pcan_usb_pro_send_req(dev, PCAN_USBPRO_REQ_INFO,
>> PCAN_USBPRO_INFO_BL,
>> - &bi, sizeof(bi));
>> + bi, sizeof(*bi));
>> if (err) {
>> - kfree(usb_if);
>> dev_err(dev->netdev->dev.parent,
>> "unable to read %s bootloader info (err %d)\n",
>> pcan_usb_pro.name, err);
>> - return err;
>> + goto err_out;
>> }
>> + /* tell the device the can driver is running */
>> + err = pcan_usb_pro_drv_loaded(dev, 1);
>> + if (err)
>> + goto err_out;
>> +
>> dev_info(dev->netdev->dev.parent,
>> "PEAK-System %s hwrev %u serial %08X.%08X (%u
>> channels)\n",
>> pcan_usb_pro.name,
>> - bi.hw_rev, bi.serial_num_hi, bi.serial_num_lo,
>> + bi->hw_rev, bi->serial_num_hi, bi->serial_num_lo,
>> pcan_usb_pro.ctrl_count);
>> -
>> - /* tell the device the can driver is running */
>> - pcan_usb_pro_drv_loaded(dev, 1);
>> } else {
>> usb_if = pcan_usb_pro_dev_if(dev->prev_siblings);
>> }
>> @@ -916,6 +928,13 @@ static int pcan_usb_pro_init(struct
>> peak_usb_device *dev)
>> pcan_usb_pro_set_led(dev, 0, 1);
>> return 0;
>> +
>> + err_out:
>> + kfree(bi);
>> + kfree(fi);
>> + kfree(usb_if);
>> +
>> + return err;
>> }
>> static void pcan_usb_pro_exit(struct peak_usb_device *dev)
I'll send a fix.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: pull-request: can 2013-06-03
2013-06-03 12:15 pull-request: can 2013-06-03 Marc Kleine-Budde
` (2 preceding siblings ...)
2013-06-03 12:15 ` [PATCH 3/3] net: can: peak_usb: " Marc Kleine-Budde
@ 2013-06-04 21:31 ` David Miller
3 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2013-06-04 21:31 UTC (permalink / raw)
To: mkl; +Cc: netdev, linux-can
From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Mon, 3 Jun 2013 14:15:30 +0200
> here are there fixes for the v3.10 release cycle:
>
> The first patch by Jonas Peterson and Olivier Sobrie fixes the reception of CAN
> frames on Kvaser's "USBcan Pro" and "USBcan R" type hardware.
>
> The last two patches by Olivier Sobrie (for esd_usb2) and me (for peak_usb)
> change the memory handling for the USB messages from stack to kmalloc(), as
> memory used for DMA should not be allocated on stack.
...
> git://gitorious.org/linux-can/linux-can.git fixes-for-3.10
Pulled, thanks Marc.
^ permalink raw reply [flat|nested] 7+ messages in thread