On 11/6/20 6:02 PM, Gerhard Uttenthaler wrote:
> Signed-off-by: Gerhard Uttenthaler <uttenthaler@xxxxxxxxxxxxxxxx>
> ---
> drivers/net/can/usb/ems_usb.c | 31 +++++++++++++++++++++++--------
> 1 file changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/can/usb/ems_usb.c b/drivers/net/can/usb/ems_usb.c
> index b51a5eb65946..c464d644c833 100644
> --- a/drivers/net/can/usb/ems_usb.c
> +++ b/drivers/net/can/usb/ems_usb.c
> @@ -902,25 +902,37 @@ static netdev_tx_t ems_usb_start_xmit(struct sk_buff *skb, struct net_device *ne
> struct can_frame *cf = (struct can_frame *)skb->data;
> struct ems_cpc_msg *msg;
> struct urb *urb;
> - u8 *buf;
> int i, err;
> - size_t size = CPC_HEADER_SIZE + CPC_MSG_HEADER_LEN
> - + sizeof(struct cpc_can_msg);
> +
> + u8 *buf;
> + size_t buf_size;
> + size_t buf_len = CPC_HEADER_SIZE + CPC_MSG_HEADER_LEN;
>
> if (can_dropped_invalid_skb(netdev, skb))
> return NETDEV_TX_OK;
>
> - /* create a URB, and a buffer for it, and copy the data to the URB */
> + buf_size = CPC_HEADER_SIZE +
> + CPC_MSG_HEADER_LEN +
> + sizeof(struct cpc_can_msg);
does it make sense to only allocate the length of the buffer needed to hold the
data?
> +
> + /* Create an URB, and a buffer for it
> + * and copy the data to the URB
> + */
> urb = usb_alloc_urb(0, GFP_ATOMIC);
> if (!urb)
> goto nomem;
>
> - buf = usb_alloc_coherent(dev->udev, size, GFP_ATOMIC, &urb->transfer_dma);
> + buf = usb_alloc_coherent(dev->udev,
> + buf_size,
> + GFP_ATOMIC,
> + &urb->transfer_dma);
> if (!buf) {
> netdev_err(netdev, "No memory left for USB buffer\n");
> usb_free_urb(urb);
> goto nomem;
> }
> + // Clear first 4 bytes
no C++ comments
> + *(u32 *)buf = 0;
>
> msg = (struct ems_cpc_msg *)&buf[CPC_HEADER_SIZE];
>
> @@ -942,6 +954,9 @@ static netdev_tx_t ems_usb_start_xmit(struct sk_buff *skb, struct net_device *ne
> msg->length = CPC_CAN_MSG_MIN_SIZE + cf->can_dlc;
> }
>
> + // Send only significant bytes of buffer
> + buf_len += msg->length;
> +
> for (i = 0; i < MAX_TX_URBS; i++) {
> if (dev->tx_contexts[i].echo_index == MAX_TX_URBS) {
> context = &dev->tx_contexts[i];
> @@ -953,7 +968,7 @@ static netdev_tx_t ems_usb_start_xmit(struct sk_buff *skb, struct net_device *ne
> * allowed (MAX_TX_URBS).
> */
> if (!context) {
> - usb_free_coherent(dev->udev, size, buf, urb->transfer_dma);
> + usb_free_coherent(dev->udev, buf_size, buf, urb->transfer_dma);
> usb_free_urb(urb);
>
> netdev_warn(netdev, "couldn't find free context\n");
> @@ -966,7 +981,7 @@ static netdev_tx_t ems_usb_start_xmit(struct sk_buff *skb, struct net_device *ne
> context->dlc = cf->can_dlc;
>
> usb_fill_bulk_urb(urb, dev->udev, usb_sndbulkpipe(dev->udev, 2), buf,
> - size, ems_usb_write_bulk_callback, context);
> + buf_len, ems_usb_write_bulk_callback, context);
> urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> usb_anchor_urb(urb, &dev->tx_submitted);
>
> @@ -979,7 +994,7 @@ static netdev_tx_t ems_usb_start_xmit(struct sk_buff *skb, struct net_device *ne
> can_free_echo_skb(netdev, context->echo_index);
>
> usb_unanchor_urb(urb);
> - usb_free_coherent(dev->udev, size, buf, urb->transfer_dma);
> + usb_free_coherent(dev->udev, buf_size, buf, urb->transfer_dma);
> dev_kfree_skb(skb);
>
> atomic_dec(&dev->active_tx_urbs);
>
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Attachment:
signature.asc
Description: OpenPGP digital signature
