On 03/11/2015 06:39 PM, Ahmed S. Darwish wrote: > From: Ahmed S. Darwish <[email protected]> > > The driver currently limits the number of outstanding, not yet > ACKed, transfers to 16 URBs. Meanwhile, the Kvaser firmware > provides its actual max supported number of outstanding > transmissions in its reply to the CMD_GET_SOFTWARE_INFO message. > > One example is the UsbCan-II HS/LS device which reports support > of up to 48 tx URBs instead of just 16, increasing the driver > throughput by two-fold and reducing the possibility of -ENOBUFs. > > Dynamically set the max tx URBs value according to firmware > replies. > > Signed-off-by: Ahmed S. Darwish <[email protected]> > --- > drivers/net/can/usb/kvaser_usb.c | 55 > +++++++++++++++++++++++++++------------- > 1 file changed, 37 insertions(+), 18 deletions(-) > > changelog-v3: No changes > > diff --git a/drivers/net/can/usb/kvaser_usb.c > b/drivers/net/can/usb/kvaser_usb.c > index e97a08c..30b4d47 100644 > --- a/drivers/net/can/usb/kvaser_usb.c > +++ b/drivers/net/can/usb/kvaser_usb.c > @@ -25,7 +25,6 @@ > #include <linux/can/dev.h> > #include <linux/can/error.h> > > -#define MAX_TX_URBS 16 > #define MAX_RX_URBS 4 > #define START_TIMEOUT 1000 /* msecs */ > #define STOP_TIMEOUT 1000 /* msecs */ > @@ -456,8 +455,13 @@ struct kvaser_usb { > struct usb_endpoint_descriptor *bulk_in, *bulk_out; > struct usb_anchor rx_submitted; > > + /* @max_tx_urbs: Firmware-reported maximum number of possible > + * outstanding transmissions on this specific Kvaser hardware. The > + * value is also used as a sentinel for marking free URB contexts. > + */ > u32 fw_version; > unsigned int nchannels; > + unsigned int max_tx_urbs; > enum kvaser_usb_family family; > > bool rxinitdone; > @@ -470,7 +474,7 @@ struct kvaser_usb_net_priv { > > spinlock_t tx_contexts_lock; > int active_tx_contexts; > - struct kvaser_usb_tx_urb_context tx_contexts[MAX_TX_URBS]; > + struct kvaser_usb_tx_urb_context *tx_contexts; > > struct usb_anchor tx_submitted; > struct completion start_comp, stop_comp; > @@ -657,9 +661,13 @@ static int kvaser_usb_get_software_info(struct > kvaser_usb *dev) > switch (dev->family) { > case KVASER_LEAF: > dev->fw_version = le32_to_cpu(msg.u.leaf.softinfo.fw_version); > + dev->max_tx_urbs = > + le16_to_cpu(msg.u.leaf.softinfo.max_outstanding_tx); > break; > case KVASER_USBCAN: > dev->fw_version = le32_to_cpu(msg.u.usbcan.softinfo.fw_version); > + dev->max_tx_urbs = > + le16_to_cpu(msg.u.usbcan.softinfo.max_outstanding_tx); > break; > } > > @@ -715,7 +723,7 @@ static void kvaser_usb_tx_acknowledge(const struct > kvaser_usb *dev, > > stats = &priv->netdev->stats; > > - context = &priv->tx_contexts[tid % MAX_TX_URBS]; > + context = &priv->tx_contexts[tid % dev->max_tx_urbs]; > > /* Sometimes the state change doesn't come after a bus-off event */ > if (priv->can.restart_ms && > @@ -744,7 +752,7 @@ static void kvaser_usb_tx_acknowledge(const struct > kvaser_usb *dev, > spin_lock_irqsave(&priv->tx_contexts_lock, flags); > > can_get_echo_skb(priv->netdev, context->echo_index); > - context->echo_index = MAX_TX_URBS; > + context->echo_index = dev->max_tx_urbs; > --priv->active_tx_contexts; > netif_wake_queue(priv->netdev); > > @@ -1512,11 +1520,13 @@ error: > > static void kvaser_usb_reset_tx_urb_contexts(struct kvaser_usb_net_priv > *priv) > { > - int i; > + int i, max_tx_urbs; > + > + max_tx_urbs = priv->dev->max_tx_urbs; > > priv->active_tx_contexts = 0; > - for (i = 0; i < MAX_TX_URBS; i++) > - priv->tx_contexts[i].echo_index = MAX_TX_URBS; > + for (i = 0; i < max_tx_urbs; i++) > + priv->tx_contexts[i].echo_index = max_tx_urbs; > } > > /* This method might sleep. Do not call it in the atomic context > @@ -1702,14 +1712,14 @@ static netdev_tx_t kvaser_usb_start_xmit(struct > sk_buff *skb, > *msg_tx_can_flags |= MSG_FLAG_REMOTE_FRAME; > > spin_lock_irqsave(&priv->tx_contexts_lock, flags); > - for (i = 0; i < ARRAY_SIZE(priv->tx_contexts); i++) { > - if (priv->tx_contexts[i].echo_index == MAX_TX_URBS) { > + for (i = 0; i < dev->max_tx_urbs; i++) { > + if (priv->tx_contexts[i].echo_index == dev->max_tx_urbs) { > context = &priv->tx_contexts[i]; > > context->echo_index = i; > can_put_echo_skb(skb, netdev, context->echo_index); > ++priv->active_tx_contexts; > - if (priv->active_tx_contexts >= MAX_TX_URBS) > + if (priv->active_tx_contexts >= dev->max_tx_urbs) > netif_stop_queue(netdev); > > break; > @@ -1743,7 +1753,7 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff > *skb, > spin_lock_irqsave(&priv->tx_contexts_lock, flags); > > can_free_echo_skb(netdev, context->echo_index); > - context->echo_index = MAX_TX_URBS; > + context->echo_index = dev->max_tx_urbs; > --priv->active_tx_contexts; > netif_wake_queue(netdev); > > @@ -1881,7 +1891,7 @@ static int kvaser_usb_init_one(struct usb_interface > *intf, > if (err) > return err; > > - netdev = alloc_candev(sizeof(*priv), MAX_TX_URBS); > + netdev = alloc_candev(sizeof(*priv), dev->max_tx_urbs); > if (!netdev) { > dev_err(&intf->dev, "Cannot alloc candev\n"); > return -ENOMEM; > @@ -1889,6 +1899,13 @@ static int kvaser_usb_init_one(struct usb_interface > *intf, > > priv = netdev_priv(netdev); > > + priv->tx_contexts = kzalloc(dev->max_tx_urbs * > + sizeof(*priv->tx_contexts), GFP_KERNEL); > + if (!priv->tx_contexts) { > + free_candev(netdev); > + return -ENOMEM; > + }
I'm missing a free for the priv->tx_contexts. I see two options:
1) use devm_kzalloc(), or
2) move struct kvaser_usb_tx_urb_context tx_contexts[]; to the end of
struct kvaser_usb_net_priv, see [1] for an example.
Without further testing, I think the correct alloc for that case
would be:
alloc_candev(sizeof(*priv + dev->max_tx_urbs *
sizeof(struct kvaser_usb_tx_urb_context))
Marc
[1] http://stackoverflow.com/questions/2060974/dynamic-array-in-struct-c
--
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 |
signature.asc
Description: OpenPGP digital signature
