Re: [PATCH] kvaser_usb: fix "dma on the stack" errors

[Olivier Sobrie <olivier@sobrie.be>]

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
> 

-- 
Olivier