| Submitter | Olivier Sobrie |
|---|---|
| Date | Nov. 23, 2012, 1:30 p.m. |
| Message ID | <1353677428-15805-1-git-send-email-olivier@sobrie.be> |
| Download | mbox | patch |
| Permalink | /patch/201319/ |
| State | Superseded |
| Delegated to: | David Miller |
| Headers | show |
Comments
On Fri, Nov 23, 2012 at 02:30:28PM +0100, Olivier Sobrie wrote: > The dma buffer given to usb_bulk_msg() must be allocated and not on > the stack. > See Documentation/DMA-API-HOWTO.txt section "What memory is DMA'able?" > > Signed-off-by: Olivier Sobrie <olivier@sobrie.be> > --- > Here is the incremental patch. > Thank you Greg ! > > Olivier > > drivers/net/can/usb/kvaser_usb.c | 110 ++++++++++++++++++++++++-------------- > 1 file changed, 69 insertions(+), 41 deletions(-) > > diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c > index 8807bf8..7ac6e82 100644 > --- a/drivers/net/can/usb/kvaser_usb.c > +++ b/drivers/net/can/usb/kvaser_usb.c > @@ -421,14 +421,21 @@ end: > static int kvaser_usb_send_simple_msg(const struct kvaser_usb *dev, > u8 msg_id, int channel) > { > - struct kvaser_msg msg = { > - .len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_simple), > - .id = msg_id, > - .u.simple.channel = channel, > - .u.simple.tid = 0xff, > - }; > - > - return kvaser_usb_send_msg(dev, &msg); > + struct kvaser_msg *msg; > + int rc; > + > + msg = kmalloc(sizeof(*msg), GFP_KERNEL); > + if (!msg) > + return -ENOMEM; > + Doh! I removed by mistake the line "msg->id = msg_id"... grr > + msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_simple); > + msg->u.simple.channel = channel; > + msg->u.simple.tid = 0xff; > + > + rc = kvaser_usb_send_msg(dev, msg); > + > + kfree(msg); > + return rc; > } > > static int kvaser_usb_get_software_info(struct kvaser_usb *dev) > @@ -1057,20 +1064,27 @@ static int kvaser_usb_setup_rx_urbs(struct kvaser_usb *dev) > > static int kvaser_usb_set_opt_mode(const struct kvaser_usb_net_priv *priv) > { > - struct kvaser_msg msg = { > - .id = CMD_SET_CTRL_MODE, > - .len = MSG_HEADER_LEN + > - sizeof(struct kvaser_msg_ctrl_mode), > - .u.ctrl_mode.tid = 0xff, > - .u.ctrl_mode.channel = priv->channel, > - }; > + struct kvaser_msg *msg; > + int rc; > + > + msg = kmalloc(sizeof(*msg), GFP_KERNEL); > + if (!msg) > + return -ENOMEM; > + > + msg->id = CMD_SET_CTRL_MODE; > + msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_ctrl_mode); > + msg->u.ctrl_mode.tid = 0xff; > + msg->u.ctrl_mode.channel = priv->channel; > > if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY) > - msg.u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_SILENT; > + msg->u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_SILENT; > else > - msg.u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_NORMAL; > + msg->u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_NORMAL; > + > + rc = kvaser_usb_send_msg(priv->dev, msg); > > - return kvaser_usb_send_msg(priv->dev, &msg); > + kfree(msg); > + return rc; > } > > static int kvaser_usb_start_chip(struct kvaser_usb_net_priv *priv) > @@ -1163,15 +1177,22 @@ static int kvaser_usb_stop_chip(struct kvaser_usb_net_priv *priv) > > static int kvaser_usb_flush_queue(struct kvaser_usb_net_priv *priv) > { > - struct kvaser_msg msg = { > - .id = CMD_FLUSH_QUEUE, > - .len = MSG_HEADER_LEN + > - sizeof(struct kvaser_msg_flush_queue), > - .u.flush_queue.channel = priv->channel, > - .u.flush_queue.flags = 0x00, > - }; > - > - return kvaser_usb_send_msg(priv->dev, &msg); > + struct kvaser_msg *msg; > + int rc; > + > + msg = kmalloc(sizeof(*msg), GFP_KERNEL); > + if (!msg) > + return -ENOMEM; > + > + msg->id = CMD_FLUSH_QUEUE; > + msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_flush_queue); > + msg->u.flush_queue.channel = priv->channel; > + msg->u.flush_queue.flags = 0x00; > + > + rc = kvaser_usb_send_msg(priv->dev, msg); > + > + kfree(msg); > + return rc; > } > > static int kvaser_usb_close(struct net_device *netdev) > @@ -1364,24 +1385,31 @@ static int kvaser_usb_set_bittiming(struct net_device *netdev) > struct kvaser_usb_net_priv *priv = netdev_priv(netdev); > struct can_bittiming *bt = &priv->can.bittiming; > struct kvaser_usb *dev = priv->dev; > - struct kvaser_msg msg = { > - .id = CMD_SET_BUS_PARAMS, > - .len = MSG_HEADER_LEN + > - sizeof(struct kvaser_msg_busparams), > - .u.busparams.channel = priv->channel, > - .u.busparams.tid = 0xff, > - .u.busparams.bitrate = cpu_to_le32(bt->bitrate), > - .u.busparams.sjw = bt->sjw, > - .u.busparams.tseg1 = bt->prop_seg + bt->phase_seg1, > - .u.busparams.tseg2 = bt->phase_seg2, > - }; > + struct kvaser_msg *msg; > + int rc; > + > + msg = kmalloc(sizeof(*msg), GFP_KERNEL); > + if (!msg) > + return -ENOMEM; > + > + msg->id = CMD_SET_BUS_PARAMS; > + msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_busparams); > + msg->u.busparams.channel = priv->channel; > + msg->u.busparams.tid = 0xff; > + msg->u.busparams.bitrate = cpu_to_le32(bt->bitrate); > + msg->u.busparams.sjw = bt->sjw; > + msg->u.busparams.tseg1 = bt->prop_seg + bt->phase_seg1; > + msg->u.busparams.tseg2 = bt->phase_seg2; > > if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES) > - msg.u.busparams.no_samp = 3; > + msg->u.busparams.no_samp = 3; > else > - msg.u.busparams.no_samp = 1; > + msg->u.busparams.no_samp = 1; > + > + rc = kvaser_usb_send_msg(dev, msg); > > - return kvaser_usb_send_msg(dev, &msg); > + kfree(msg); > + return rc; > } > > static int kvaser_usb_set_mode(struct net_device *netdev, > -- > 1.7.9.5 >
On 11/23/2012 02:40 PM, Olivier Sobrie wrote: > On Fri, Nov 23, 2012 at 02:30:28PM +0100, Olivier Sobrie wrote: >> The dma buffer given to usb_bulk_msg() must be allocated and not on >> the stack. >> See Documentation/DMA-API-HOWTO.txt section "What memory is DMA'able?" >> >> Signed-off-by: Olivier Sobrie <olivier@sobrie.be> >> --- >> Here is the incremental patch. >> Thank you Greg ! >> >> Olivier >> >> drivers/net/can/usb/kvaser_usb.c | 110 ++++++++++++++++++++++++-------------- >> 1 file changed, 69 insertions(+), 41 deletions(-) >> >> diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c >> index 8807bf8..7ac6e82 100644 >> --- a/drivers/net/can/usb/kvaser_usb.c >> +++ b/drivers/net/can/usb/kvaser_usb.c >> @@ -421,14 +421,21 @@ end: >> static int kvaser_usb_send_simple_msg(const struct kvaser_usb *dev, >> u8 msg_id, int channel) >> { >> - struct kvaser_msg msg = { >> - .len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_simple), >> - .id = msg_id, >> - .u.simple.channel = channel, >> - .u.simple.tid = 0xff, >> - }; >> - >> - return kvaser_usb_send_msg(dev, &msg); >> + struct kvaser_msg *msg; >> + int rc; >> + >> + msg = kmalloc(sizeof(*msg), GFP_KERNEL); >> + if (!msg) >> + return -ENOMEM; >> + > > Doh! I removed by mistake the line "msg->id = msg_id"... grr Please send a v2 version of this patch with this problem fixed. MarcMarc
Patch
diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c index 8807bf8..7ac6e82 100644 --- a/drivers/net/can/usb/kvaser_usb.c +++ b/drivers/net/can/usb/kvaser_usb.c @@ -421,14 +421,21 @@ end: static int kvaser_usb_send_simple_msg(const struct kvaser_usb *dev, u8 msg_id, int channel) { - struct kvaser_msg msg = { - .len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_simple), - .id = msg_id, - .u.simple.channel = channel, - .u.simple.tid = 0xff, - }; - - return kvaser_usb_send_msg(dev, &msg); + struct kvaser_msg *msg; + int rc; + + msg = kmalloc(sizeof(*msg), GFP_KERNEL); + if (!msg) + return -ENOMEM; + + msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_simple); + msg->u.simple.channel = channel; + msg->u.simple.tid = 0xff; + + rc = kvaser_usb_send_msg(dev, msg); + + kfree(msg); + return rc; } static int kvaser_usb_get_software_info(struct kvaser_usb *dev) @@ -1057,20 +1064,27 @@ static int kvaser_usb_setup_rx_urbs(struct kvaser_usb *dev) static int kvaser_usb_set_opt_mode(const struct kvaser_usb_net_priv *priv) { - struct kvaser_msg msg = { - .id = CMD_SET_CTRL_MODE, - .len = MSG_HEADER_LEN + - sizeof(struct kvaser_msg_ctrl_mode), - .u.ctrl_mode.tid = 0xff, - .u.ctrl_mode.channel = priv->channel, - }; + struct kvaser_msg *msg; + int rc; + + msg = kmalloc(sizeof(*msg), GFP_KERNEL); + if (!msg) + return -ENOMEM; + + msg->id = CMD_SET_CTRL_MODE; + msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_ctrl_mode); + msg->u.ctrl_mode.tid = 0xff; + msg->u.ctrl_mode.channel = priv->channel; if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY) - msg.u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_SILENT; + msg->u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_SILENT; else - msg.u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_NORMAL; + msg->u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_NORMAL; + + rc = kvaser_usb_send_msg(priv->dev, msg); - return kvaser_usb_send_msg(priv->dev, &msg); + kfree(msg); + return rc; } static int kvaser_usb_start_chip(struct kvaser_usb_net_priv *priv) @@ -1163,15 +1177,22 @@ static int kvaser_usb_stop_chip(struct kvaser_usb_net_priv *priv) static int kvaser_usb_flush_queue(struct kvaser_usb_net_priv *priv) { - struct kvaser_msg msg = { - .id = CMD_FLUSH_QUEUE, - .len = MSG_HEADER_LEN + - sizeof(struct kvaser_msg_flush_queue), - .u.flush_queue.channel = priv->channel, - .u.flush_queue.flags = 0x00, - }; - - return kvaser_usb_send_msg(priv->dev, &msg); + struct kvaser_msg *msg; + int rc; + + msg = kmalloc(sizeof(*msg), GFP_KERNEL); + if (!msg) + return -ENOMEM; + + msg->id = CMD_FLUSH_QUEUE; + msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_flush_queue); + msg->u.flush_queue.channel = priv->channel; + msg->u.flush_queue.flags = 0x00; + + rc = kvaser_usb_send_msg(priv->dev, msg); + + kfree(msg); + return rc; } static int kvaser_usb_close(struct net_device *netdev) @@ -1364,24 +1385,31 @@ static int kvaser_usb_set_bittiming(struct net_device *netdev) struct kvaser_usb_net_priv *priv = netdev_priv(netdev); struct can_bittiming *bt = &priv->can.bittiming; struct kvaser_usb *dev = priv->dev; - struct kvaser_msg msg = { - .id = CMD_SET_BUS_PARAMS, - .len = MSG_HEADER_LEN + - sizeof(struct kvaser_msg_busparams), - .u.busparams.channel = priv->channel, - .u.busparams.tid = 0xff, - .u.busparams.bitrate = cpu_to_le32(bt->bitrate), - .u.busparams.sjw = bt->sjw, - .u.busparams.tseg1 = bt->prop_seg + bt->phase_seg1, - .u.busparams.tseg2 = bt->phase_seg2, - }; + struct kvaser_msg *msg; + int rc; + + msg = kmalloc(sizeof(*msg), GFP_KERNEL); + if (!msg) + return -ENOMEM; + + msg->id = CMD_SET_BUS_PARAMS; + msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_busparams); + msg->u.busparams.channel = priv->channel; + msg->u.busparams.tid = 0xff; + msg->u.busparams.bitrate = cpu_to_le32(bt->bitrate); + msg->u.busparams.sjw = bt->sjw; + msg->u.busparams.tseg1 = bt->prop_seg + bt->phase_seg1; + msg->u.busparams.tseg2 = bt->phase_seg2; if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES) - msg.u.busparams.no_samp = 3; + msg->u.busparams.no_samp = 3; else - msg.u.busparams.no_samp = 1; + msg->u.busparams.no_samp = 1; + + rc = kvaser_usb_send_msg(dev, msg); - return kvaser_usb_send_msg(dev, &msg); + kfree(msg); + return rc; } static int kvaser_usb_set_mode(struct net_device *netdev,
The dma buffer given to usb_bulk_msg() must be allocated and not on the stack. See Documentation/DMA-API-HOWTO.txt section "What memory is DMA'able?" Signed-off-by: Olivier Sobrie <olivier@sobrie.be> --- Here is the incremental patch. Thank you Greg ! Olivier drivers/net/can/usb/kvaser_usb.c | 110 ++++++++++++++++++++++++-------------- 1 file changed, 69 insertions(+), 41 deletions(-)