From edd1a7e42f1d2d09c5f79ecef05ae19dc669bf34 Mon Sep 17 00:00:00 2001 From: Ziyang Xuan Date: Thu, 15 Sep 2022 09:55:55 +0800 Subject: [PATCH 01/11] can: bcm: registration process optimization in bcm_module_init() Now, register_netdevice_notifier() and register_pernet_subsys() are both after can_proto_register(). It can create CAN_BCM socket and process socket once can_proto_register() successfully, so it is possible missing notifier event or proc node creation because notifier or bcm proc directory is not registered or created yet. Although this is a low probability scenario, it is not impossible. Move register_pernet_subsys() and register_netdevice_notifier() to the front of can_proto_register(). In addition, register_pernet_subsys() and register_netdevice_notifier() may fail, check their results are necessary. Signed-off-by: Ziyang Xuan Link: https://lore.kernel.org/all/823cff0ebec33fa9389eeaf8b8ded3217c32cb38.1663206163.git.william.xuanziyang@huawei.com Acked-by: Oliver Hartkopp Signed-off-by: Marc Kleine-Budde --- net/can/bcm.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/net/can/bcm.c b/net/can/bcm.c index e5d179ba6f7dc..0a2adb844280b 100644 --- a/net/can/bcm.c +++ b/net/can/bcm.c @@ -1749,15 +1749,27 @@ static int __init bcm_module_init(void) pr_info("can: broadcast manager protocol\n"); + err = register_pernet_subsys(&canbcm_pernet_ops); + if (err) + return err; + + err = register_netdevice_notifier(&canbcm_notifier); + if (err) + goto register_notifier_failed; + err = can_proto_register(&bcm_can_proto); if (err < 0) { printk(KERN_ERR "can: registration of bcm protocol failed\n"); - return err; + goto register_proto_failed; } - register_pernet_subsys(&canbcm_pernet_ops); - register_netdevice_notifier(&canbcm_notifier); return 0; + +register_proto_failed: + unregister_netdevice_notifier(&canbcm_notifier); +register_notifier_failed: + unregister_pernet_subsys(&canbcm_pernet_ops); + return err; } static void __exit bcm_module_exit(void) From 3fd7bfd28cfd68ae80a2fe92ea1615722cc2ee6e Mon Sep 17 00:00:00 2001 From: Ziyang Xuan Date: Thu, 15 Sep 2022 09:55:56 +0800 Subject: [PATCH 02/11] can: bcm: check the result of can_send() in bcm_can_tx() If can_send() fail, it should not update frames_abs counter in bcm_can_tx(). Add the result check for can_send() in bcm_can_tx(). Suggested-by: Marc Kleine-Budde Suggested-by: Oliver Hartkopp Signed-off-by: Ziyang Xuan Link: https://lore.kernel.org/all/9851878e74d6d37aee2f1ee76d68361a46f89458.1663206163.git.william.xuanziyang@huawei.com Acked-by: Oliver Hartkopp Signed-off-by: Marc Kleine-Budde --- net/can/bcm.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/net/can/bcm.c b/net/can/bcm.c index 0a2adb844280b..27706f6ace34a 100644 --- a/net/can/bcm.c +++ b/net/can/bcm.c @@ -274,6 +274,7 @@ static void bcm_can_tx(struct bcm_op *op) struct sk_buff *skb; struct net_device *dev; struct canfd_frame *cf = op->frames + op->cfsiz * op->currframe; + int err; /* no target device? => exit */ if (!op->ifindex) @@ -298,11 +299,11 @@ static void bcm_can_tx(struct bcm_op *op) /* send with loopback */ skb->dev = dev; can_skb_set_owner(skb, op->sk); - can_send(skb, 1); + err = can_send(skb, 1); + if (!err) + op->frames_abs++; - /* update statistics */ op->currframe++; - op->frames_abs++; /* reached last frame? */ if (op->currframe >= op->nframes) From 593b5e2f5a4abff998644effe19033f440651fd6 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Tue, 20 Sep 2022 11:56:57 +0200 Subject: [PATCH 03/11] can: gs_usb: gs_usb_get_timestamp(): fix endpoint parameter for usb_control_msg_recv() The 2nd argument of usb_control_msg_recv() is the "endpoint", usb_control_msg_recv() will internally convert the endpoint into a pipe with usb_rcvctrlpipe(). In gs_usb_get_timestamp() not the endpoint "0" is passed, but the pipe. This worked by accident as endpoint is a __u8 and the lowest 8 bits of the pipe are 0. Fix this copy/paste error by using the correct endpoint of "0". Fixes: 45dfa45f52e6 ("can: gs_usb: add RX and TX hardware timestamp support") Link: https://lore.kernel.org/all/20220920100416.959226-2-mkl@pengutronix.de Cc: John Whittington Tested-by: John Whittington Signed-off-by: Marc Kleine-Budde --- drivers/net/can/usb/gs_usb.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c index 5e0d280b0cd3a..1430368ca3277 100644 --- a/drivers/net/can/usb/gs_usb.c +++ b/drivers/net/can/usb/gs_usb.c @@ -386,8 +386,7 @@ static inline int gs_usb_get_timestamp(const struct gs_can *dev, __le32 timestamp; int rc; - rc = usb_control_msg_recv(interface_to_usbdev(dev->iface), - usb_sndctrlpipe(interface_to_usbdev(dev->iface), 0), + rc = usb_control_msg_recv(interface_to_usbdev(dev->iface), 0, GS_USB_BREQ_TIMESTAMP, USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_INTERFACE, dev->channel, 0, From 29a8c9ec9090b335ece3bd58d779af7f569b5a65 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Mon, 19 Sep 2022 09:53:45 +0200 Subject: [PATCH 04/11] can: gs_usb: add missing lock to protect struct timecounter::cycle_last The struct timecounter::cycle_last is a 64 bit variable, read by timecounter_cyc2time(), and written by timecounter_read(). On 32 bit architectures this is not atomic. Add a spinlock to protect access to struct timecounter::cycle_last. In the gs_usb_timestamp_read() callback the lock is dropped to execute a sleeping synchronous USB transfer. This is safe, as the variable we want to protect is accessed during this call. Fixes: 45dfa45f52e6 ("can: gs_usb: add RX and TX hardware timestamp support") Link: https://lore.kernel.org/all/20220920100416.959226-3-mkl@pengutronix.de Cc: John Whittington Tested-by: John Whittington Signed-off-by: Marc Kleine-Budde --- drivers/net/can/usb/gs_usb.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c index 1430368ca3277..9a8a7f1b20021 100644 --- a/drivers/net/can/usb/gs_usb.c +++ b/drivers/net/can/usb/gs_usb.c @@ -286,6 +286,7 @@ struct gs_can { /* time counter for hardware timestamps */ struct cyclecounter cc; struct timecounter tc; + spinlock_t tc_lock; /* spinlock to guard access tc->cycle_last */ struct delayed_work timestamp; u32 feature; @@ -401,14 +402,18 @@ static inline int gs_usb_get_timestamp(const struct gs_can *dev, return 0; } -static u64 gs_usb_timestamp_read(const struct cyclecounter *cc) +static u64 gs_usb_timestamp_read(const struct cyclecounter *cc) __must_hold(&dev->tc_lock) { - const struct gs_can *dev; + struct gs_can *dev = container_of(cc, struct gs_can, cc); u32 timestamp = 0; int err; - dev = container_of(cc, struct gs_can, cc); + lockdep_assert_held(&dev->tc_lock); + + /* drop lock for synchronous USB transfer */ + spin_unlock_bh(&dev->tc_lock); err = gs_usb_get_timestamp(dev, ×tamp); + spin_lock_bh(&dev->tc_lock); if (err) netdev_err(dev->netdev, "Error %d while reading timestamp. HW timestamps may be inaccurate.", @@ -423,19 +428,24 @@ static void gs_usb_timestamp_work(struct work_struct *work) struct gs_can *dev; dev = container_of(delayed_work, struct gs_can, timestamp); + spin_lock_bh(&dev->tc_lock); timecounter_read(&dev->tc); + spin_unlock_bh(&dev->tc_lock); schedule_delayed_work(&dev->timestamp, GS_USB_TIMESTAMP_WORK_DELAY_SEC * HZ); } -static void gs_usb_skb_set_timestamp(const struct gs_can *dev, +static void gs_usb_skb_set_timestamp(struct gs_can *dev, struct sk_buff *skb, u32 timestamp) { struct skb_shared_hwtstamps *hwtstamps = skb_hwtstamps(skb); u64 ns; + spin_lock_bh(&dev->tc_lock); ns = timecounter_cyc2time(&dev->tc, timestamp); + spin_unlock_bh(&dev->tc_lock); + hwtstamps->hwtstamp = ns_to_ktime(ns); } @@ -448,7 +458,10 @@ static void gs_usb_timestamp_init(struct gs_can *dev) cc->shift = 32 - bits_per(NSEC_PER_SEC / GS_USB_TIMESTAMP_TIMER_HZ); cc->mult = clocksource_hz2mult(GS_USB_TIMESTAMP_TIMER_HZ, cc->shift); + spin_lock_init(&dev->tc_lock); + spin_lock_bh(&dev->tc_lock); timecounter_init(&dev->tc, &dev->cc, ktime_get_real_ns()); + spin_unlock_bh(&dev->tc_lock); INIT_DELAYED_WORK(&dev->timestamp, gs_usb_timestamp_work); schedule_delayed_work(&dev->timestamp, @@ -485,7 +498,7 @@ static void gs_update_state(struct gs_can *dev, struct can_frame *cf) } } -static void gs_usb_set_timestamp(const struct gs_can *dev, struct sk_buff *skb, +static void gs_usb_set_timestamp(struct gs_can *dev, struct sk_buff *skb, const struct gs_host_frame *hf) { u32 timestamp; From 103108cb9673814a1f73522dacc79ad28cfc0271 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Tue, 20 Sep 2022 11:46:12 +0200 Subject: [PATCH 05/11] can: gs_usb: gs_can_open(): initialize time counter before starting device On busy networks the CAN controller might receive CAN frames directly after starting it but before the timecounter is setup. This will lead to NULL pointer deref while converting the converting the CAN frame's timestamp with the timecounter. Close the race window by setting up the timecounter before starting the CAN controller. Fixes: 45dfa45f52e6 ("can: gs_usb: add RX and TX hardware timestamp support") Link: https://lore.kernel.org/all/20220921081329.385509-1-mkl@pengutronix.de Cc: John Whittington Signed-off-by: Marc Kleine-Budde --- drivers/net/can/usb/gs_usb.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c index 9a8a7f1b20021..aa619dfc3ff2a 100644 --- a/drivers/net/can/usb/gs_usb.c +++ b/drivers/net/can/usb/gs_usb.c @@ -972,6 +972,10 @@ static int gs_can_open(struct net_device *netdev) if (dev->feature & GS_CAN_FEATURE_HW_TIMESTAMP) flags |= GS_CAN_MODE_HW_TIMESTAMP; + /* start polling timestamp */ + if (dev->feature & GS_CAN_FEATURE_HW_TIMESTAMP) + gs_usb_timestamp_init(dev); + /* finally start device */ dev->can.state = CAN_STATE_ERROR_ACTIVE; dm->mode = cpu_to_le32(GS_CAN_MODE_START); @@ -985,16 +989,14 @@ static int gs_can_open(struct net_device *netdev) if (rc < 0) { netdev_err(netdev, "Couldn't start device (err=%d)\n", rc); kfree(dm); + if (dev->feature & GS_CAN_FEATURE_HW_TIMESTAMP) + gs_usb_timestamp_stop(dev); dev->can.state = CAN_STATE_STOPPED; return rc; } kfree(dm); - /* start polling timestamp */ - if (dev->feature & GS_CAN_FEATURE_HW_TIMESTAMP) - gs_usb_timestamp_init(dev); - parent->active_channels++; if (!(dev->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)) netif_start_queue(netdev); From 00246751868888cad3f506eac2b32df916145de7 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Tue, 20 Sep 2022 23:21:42 +0200 Subject: [PATCH 06/11] can: gs_usb: gs_cmd_reset(): rename variable holding struct gs_can pointer to dev Most of the driver uses the variable "dev" to point to the struct gs_can. Use the same name in gs_cmd_reset(), too. Rename gsdev to dev. Fixes: d08e973a77d1 ("can: gs_usb: Added support for the GS_USB CAN devices") Link: https://lore.kernel.org/all/20220921193902.575416-2-mkl@pengutronix.de Signed-off-by: Marc Kleine-Budde --- drivers/net/can/usb/gs_usb.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c index aa619dfc3ff2a..629ab664ca3f6 100644 --- a/drivers/net/can/usb/gs_usb.c +++ b/drivers/net/can/usb/gs_usb.c @@ -358,10 +358,10 @@ static struct gs_tx_context *gs_get_tx_context(struct gs_can *dev, return NULL; } -static int gs_cmd_reset(struct gs_can *gsdev) +static int gs_cmd_reset(struct gs_can *dev) { struct gs_device_mode *dm; - struct usb_interface *intf = gsdev->iface; + struct usb_interface *intf = dev->iface; int rc; dm = kzalloc(sizeof(*dm), GFP_KERNEL); @@ -374,7 +374,7 @@ static int gs_cmd_reset(struct gs_can *gsdev) usb_sndctrlpipe(interface_to_usbdev(intf), 0), GS_USB_BREQ_MODE, USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_INTERFACE, - gsdev->channel, 0, dm, sizeof(*dm), 1000); + dev->channel, 0, dm, sizeof(*dm), 1000); kfree(dm); From 3814ed27548a1eb9c935c56321e26b383ed8f1d7 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Sun, 18 Sep 2022 22:42:59 +0200 Subject: [PATCH 07/11] can: gs_usb: convert from usb_control_msg() to usb_control_msg_{send,recv}() Convert the driver to use usb_control_msg_{send,recv}() instead of usb_control_msg(). These functions allow the data to be placed on the stack. This makes the driver a lot easier as we don't have to deal with dynamically allocated memory. Link: https://lore.kernel.org/all/20220921193902.575416-3-mkl@pengutronix.de Signed-off-by: Marc Kleine-Budde --- drivers/net/can/usb/gs_usb.c | 301 +++++++++++++---------------------- 1 file changed, 107 insertions(+), 194 deletions(-) diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c index 629ab664ca3f6..28a645409df90 100644 --- a/drivers/net/can/usb/gs_usb.c +++ b/drivers/net/can/usb/gs_usb.c @@ -360,25 +360,15 @@ static struct gs_tx_context *gs_get_tx_context(struct gs_can *dev, static int gs_cmd_reset(struct gs_can *dev) { - struct gs_device_mode *dm; - struct usb_interface *intf = dev->iface; - int rc; - - dm = kzalloc(sizeof(*dm), GFP_KERNEL); - if (!dm) - return -ENOMEM; - - dm->mode = GS_CAN_MODE_RESET; - - rc = usb_control_msg(interface_to_usbdev(intf), - usb_sndctrlpipe(interface_to_usbdev(intf), 0), - GS_USB_BREQ_MODE, - USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_INTERFACE, - dev->channel, 0, dm, sizeof(*dm), 1000); - - kfree(dm); + struct gs_device_mode dm = { + .mode = GS_CAN_MODE_RESET, + }; - return rc; + return usb_control_msg_send(interface_to_usbdev(dev->iface), 0, + GS_USB_BREQ_MODE, + USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_INTERFACE, + dev->channel, 0, &dm, sizeof(dm), 1000, + GFP_KERNEL); } static inline int gs_usb_get_timestamp(const struct gs_can *dev, @@ -656,72 +646,44 @@ static int gs_usb_set_bittiming(struct net_device *netdev) { struct gs_can *dev = netdev_priv(netdev); struct can_bittiming *bt = &dev->can.bittiming; - struct usb_interface *intf = dev->iface; - int rc; - struct gs_device_bittiming *dbt; - - dbt = kmalloc(sizeof(*dbt), GFP_KERNEL); - if (!dbt) - return -ENOMEM; - - dbt->prop_seg = cpu_to_le32(bt->prop_seg); - dbt->phase_seg1 = cpu_to_le32(bt->phase_seg1); - dbt->phase_seg2 = cpu_to_le32(bt->phase_seg2); - dbt->sjw = cpu_to_le32(bt->sjw); - dbt->brp = cpu_to_le32(bt->brp); + struct gs_device_bittiming dbt = { + .prop_seg = cpu_to_le32(bt->prop_seg), + .phase_seg1 = cpu_to_le32(bt->phase_seg1), + .phase_seg2 = cpu_to_le32(bt->phase_seg2), + .sjw = cpu_to_le32(bt->sjw), + .brp = cpu_to_le32(bt->brp), + }; /* request bit timings */ - rc = usb_control_msg(interface_to_usbdev(intf), - usb_sndctrlpipe(interface_to_usbdev(intf), 0), - GS_USB_BREQ_BITTIMING, - USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_INTERFACE, - dev->channel, 0, dbt, sizeof(*dbt), 1000); - - kfree(dbt); - - if (rc < 0) - dev_err(netdev->dev.parent, "Couldn't set bittimings (err=%d)", - rc); - - return (rc > 0) ? 0 : rc; + return usb_control_msg_send(interface_to_usbdev(dev->iface), 0, + GS_USB_BREQ_BITTIMING, + USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_INTERFACE, + dev->channel, 0, &dbt, sizeof(dbt), 1000, + GFP_KERNEL); } static int gs_usb_set_data_bittiming(struct net_device *netdev) { struct gs_can *dev = netdev_priv(netdev); struct can_bittiming *bt = &dev->can.data_bittiming; - struct usb_interface *intf = dev->iface; - struct gs_device_bittiming *dbt; + struct gs_device_bittiming dbt = { + .prop_seg = cpu_to_le32(bt->prop_seg), + .phase_seg1 = cpu_to_le32(bt->phase_seg1), + .phase_seg2 = cpu_to_le32(bt->phase_seg2), + .sjw = cpu_to_le32(bt->sjw), + .brp = cpu_to_le32(bt->brp), + }; u8 request = GS_USB_BREQ_DATA_BITTIMING; - int rc; - - dbt = kmalloc(sizeof(*dbt), GFP_KERNEL); - if (!dbt) - return -ENOMEM; - - dbt->prop_seg = cpu_to_le32(bt->prop_seg); - dbt->phase_seg1 = cpu_to_le32(bt->phase_seg1); - dbt->phase_seg2 = cpu_to_le32(bt->phase_seg2); - dbt->sjw = cpu_to_le32(bt->sjw); - dbt->brp = cpu_to_le32(bt->brp); if (dev->feature & GS_CAN_FEATURE_QUIRK_BREQ_CANTACT_PRO) request = GS_USB_BREQ_QUIRK_CANTACT_PRO_DATA_BITTIMING; - /* request bit timings */ - rc = usb_control_msg(interface_to_usbdev(intf), - usb_sndctrlpipe(interface_to_usbdev(intf), 0), - request, - USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_INTERFACE, - dev->channel, 0, dbt, sizeof(*dbt), 1000); - - kfree(dbt); - - if (rc < 0) - dev_err(netdev->dev.parent, - "Couldn't set data bittimings (err=%d)", rc); - - return (rc > 0) ? 0 : rc; + /* request data bit timings */ + return usb_control_msg_send(interface_to_usbdev(dev->iface), 0, + request, + USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_INTERFACE, + dev->channel, 0, &dbt, sizeof(dbt), 1000, + GFP_KERNEL); } static void gs_usb_xmit_callback(struct urb *urb) @@ -860,11 +822,13 @@ static int gs_can_open(struct net_device *netdev) { struct gs_can *dev = netdev_priv(netdev); struct gs_usb *parent = dev->parent; - int rc, i; - struct gs_device_mode *dm; + struct gs_device_mode dm = { + .mode = cpu_to_le32(GS_CAN_MODE_START), + }; struct gs_host_frame *hf; u32 ctrlmode; u32 flags = 0; + int rc, i; rc = open_candev(netdev); if (rc) @@ -949,10 +913,6 @@ static int gs_can_open(struct net_device *netdev) } } - dm = kmalloc(sizeof(*dm), GFP_KERNEL); - if (!dm) - return -ENOMEM; - /* flags */ if (ctrlmode & CAN_CTRLMODE_LOOPBACK) flags |= GS_CAN_MODE_LOOP_BACK; @@ -978,25 +938,20 @@ static int gs_can_open(struct net_device *netdev) /* finally start device */ dev->can.state = CAN_STATE_ERROR_ACTIVE; - dm->mode = cpu_to_le32(GS_CAN_MODE_START); - dm->flags = cpu_to_le32(flags); - rc = usb_control_msg(interface_to_usbdev(dev->iface), - usb_sndctrlpipe(interface_to_usbdev(dev->iface), 0), - GS_USB_BREQ_MODE, - USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_INTERFACE, - dev->channel, 0, dm, sizeof(*dm), 1000); - - if (rc < 0) { + dm.flags = cpu_to_le32(flags); + rc = usb_control_msg_send(interface_to_usbdev(dev->iface), 0, + GS_USB_BREQ_MODE, + USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_INTERFACE, + dev->channel, 0, &dm, sizeof(dm), 1000, + GFP_KERNEL); + if (rc) { netdev_err(netdev, "Couldn't start device (err=%d)\n", rc); - kfree(dm); if (dev->feature & GS_CAN_FEATURE_HW_TIMESTAMP) gs_usb_timestamp_stop(dev); dev->can.state = CAN_STATE_STOPPED; return rc; } - kfree(dm); - parent->active_channels++; if (!(dev->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)) netif_start_queue(netdev); @@ -1070,28 +1025,18 @@ static const struct net_device_ops gs_usb_netdev_ops = { static int gs_usb_set_identify(struct net_device *netdev, bool do_identify) { struct gs_can *dev = netdev_priv(netdev); - struct gs_identify_mode *imode; - int rc; - - imode = kmalloc(sizeof(*imode), GFP_KERNEL); - - if (!imode) - return -ENOMEM; + struct gs_identify_mode imode; if (do_identify) - imode->mode = cpu_to_le32(GS_CAN_IDENTIFY_ON); + imode.mode = cpu_to_le32(GS_CAN_IDENTIFY_ON); else - imode->mode = cpu_to_le32(GS_CAN_IDENTIFY_OFF); - - rc = usb_control_msg(interface_to_usbdev(dev->iface), - usb_sndctrlpipe(interface_to_usbdev(dev->iface), 0), - GS_USB_BREQ_IDENTIFY, - USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_INTERFACE, - dev->channel, 0, imode, sizeof(*imode), 100); - - kfree(imode); + imode.mode = cpu_to_le32(GS_CAN_IDENTIFY_OFF); - return (rc > 0) ? 0 : rc; + return usb_control_msg_send(interface_to_usbdev(dev->iface), 0, + GS_USB_BREQ_IDENTIFY, + USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_INTERFACE, + dev->channel, 0, &imode, sizeof(imode), 100, + GFP_KERNEL); } /* blink LED's for finding the this interface */ @@ -1142,26 +1087,21 @@ static struct gs_can *gs_make_candev(unsigned int channel, struct gs_can *dev; struct net_device *netdev; int rc; - struct gs_device_bt_const *bt_const; - struct gs_device_bt_const_extended *bt_const_extended; + struct gs_device_bt_const_extended bt_const_extended; + struct gs_device_bt_const bt_const; u32 feature; - bt_const = kmalloc(sizeof(*bt_const), GFP_KERNEL); - if (!bt_const) - return ERR_PTR(-ENOMEM); - /* fetch bit timing constants */ - rc = usb_control_msg(interface_to_usbdev(intf), - usb_rcvctrlpipe(interface_to_usbdev(intf), 0), - GS_USB_BREQ_BT_CONST, - USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_INTERFACE, - channel, 0, bt_const, sizeof(*bt_const), 1000); + rc = usb_control_msg_recv(interface_to_usbdev(intf), 0, + GS_USB_BREQ_BT_CONST, + USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_INTERFACE, + channel, 0, &bt_const, sizeof(bt_const), 1000, + GFP_KERNEL); - if (rc < 0) { + if (rc) { dev_err(&intf->dev, "Couldn't get bit timing const for channel (err=%d)\n", rc); - kfree(bt_const); return ERR_PTR(rc); } @@ -1169,7 +1109,6 @@ static struct gs_can *gs_make_candev(unsigned int channel, netdev = alloc_candev(sizeof(struct gs_can), GS_MAX_TX_URBS); if (!netdev) { dev_err(&intf->dev, "Couldn't allocate candev\n"); - kfree(bt_const); return ERR_PTR(-ENOMEM); } @@ -1182,14 +1121,14 @@ static struct gs_can *gs_make_candev(unsigned int channel, /* dev setup */ strcpy(dev->bt_const.name, KBUILD_MODNAME); - dev->bt_const.tseg1_min = le32_to_cpu(bt_const->tseg1_min); - dev->bt_const.tseg1_max = le32_to_cpu(bt_const->tseg1_max); - dev->bt_const.tseg2_min = le32_to_cpu(bt_const->tseg2_min); - dev->bt_const.tseg2_max = le32_to_cpu(bt_const->tseg2_max); - dev->bt_const.sjw_max = le32_to_cpu(bt_const->sjw_max); - dev->bt_const.brp_min = le32_to_cpu(bt_const->brp_min); - dev->bt_const.brp_max = le32_to_cpu(bt_const->brp_max); - dev->bt_const.brp_inc = le32_to_cpu(bt_const->brp_inc); + dev->bt_const.tseg1_min = le32_to_cpu(bt_const.tseg1_min); + dev->bt_const.tseg1_max = le32_to_cpu(bt_const.tseg1_max); + dev->bt_const.tseg2_min = le32_to_cpu(bt_const.tseg2_min); + dev->bt_const.tseg2_max = le32_to_cpu(bt_const.tseg2_max); + dev->bt_const.sjw_max = le32_to_cpu(bt_const.sjw_max); + dev->bt_const.brp_min = le32_to_cpu(bt_const.brp_min); + dev->bt_const.brp_max = le32_to_cpu(bt_const.brp_max); + dev->bt_const.brp_inc = le32_to_cpu(bt_const.brp_inc); dev->udev = interface_to_usbdev(intf); dev->iface = intf; @@ -1206,13 +1145,13 @@ static struct gs_can *gs_make_candev(unsigned int channel, /* can setup */ dev->can.state = CAN_STATE_STOPPED; - dev->can.clock.freq = le32_to_cpu(bt_const->fclk_can); + dev->can.clock.freq = le32_to_cpu(bt_const.fclk_can); dev->can.bittiming_const = &dev->bt_const; dev->can.do_set_bittiming = gs_usb_set_bittiming; dev->can.ctrlmode_supported = CAN_CTRLMODE_CC_LEN8_DLC; - feature = le32_to_cpu(bt_const->feature); + feature = le32_to_cpu(bt_const.feature); dev->feature = FIELD_GET(GS_CAN_FEATURE_MASK, feature); if (feature & GS_CAN_FEATURE_LISTEN_ONLY) dev->can.ctrlmode_supported |= CAN_CTRLMODE_LISTENONLY; @@ -1263,45 +1202,35 @@ static struct gs_can *gs_make_candev(unsigned int channel, feature & GS_CAN_FEATURE_IDENTIFY)) dev->feature &= ~GS_CAN_FEATURE_IDENTIFY; - kfree(bt_const); - /* fetch extended bit timing constants if device has feature * GS_CAN_FEATURE_FD and GS_CAN_FEATURE_BT_CONST_EXT */ if (feature & GS_CAN_FEATURE_FD && feature & GS_CAN_FEATURE_BT_CONST_EXT) { - bt_const_extended = kmalloc(sizeof(*bt_const_extended), GFP_KERNEL); - if (!bt_const_extended) - return ERR_PTR(-ENOMEM); - - rc = usb_control_msg(interface_to_usbdev(intf), - usb_rcvctrlpipe(interface_to_usbdev(intf), 0), - GS_USB_BREQ_BT_CONST_EXT, - USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_INTERFACE, - channel, 0, bt_const_extended, - sizeof(*bt_const_extended), - 1000); - if (rc < 0) { + rc = usb_control_msg_recv(interface_to_usbdev(intf), 0, + GS_USB_BREQ_BT_CONST_EXT, + USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_INTERFACE, + channel, 0, &bt_const_extended, + sizeof(bt_const_extended), + 1000, GFP_KERNEL); + if (rc) { dev_err(&intf->dev, "Couldn't get extended bit timing const for channel (err=%d)\n", rc); - kfree(bt_const_extended); return ERR_PTR(rc); } strcpy(dev->data_bt_const.name, KBUILD_MODNAME); - dev->data_bt_const.tseg1_min = le32_to_cpu(bt_const_extended->dtseg1_min); - dev->data_bt_const.tseg1_max = le32_to_cpu(bt_const_extended->dtseg1_max); - dev->data_bt_const.tseg2_min = le32_to_cpu(bt_const_extended->dtseg2_min); - dev->data_bt_const.tseg2_max = le32_to_cpu(bt_const_extended->dtseg2_max); - dev->data_bt_const.sjw_max = le32_to_cpu(bt_const_extended->dsjw_max); - dev->data_bt_const.brp_min = le32_to_cpu(bt_const_extended->dbrp_min); - dev->data_bt_const.brp_max = le32_to_cpu(bt_const_extended->dbrp_max); - dev->data_bt_const.brp_inc = le32_to_cpu(bt_const_extended->dbrp_inc); + dev->data_bt_const.tseg1_min = le32_to_cpu(bt_const_extended.dtseg1_min); + dev->data_bt_const.tseg1_max = le32_to_cpu(bt_const_extended.dtseg1_max); + dev->data_bt_const.tseg2_min = le32_to_cpu(bt_const_extended.dtseg2_min); + dev->data_bt_const.tseg2_max = le32_to_cpu(bt_const_extended.dtseg2_max); + dev->data_bt_const.sjw_max = le32_to_cpu(bt_const_extended.dsjw_max); + dev->data_bt_const.brp_min = le32_to_cpu(bt_const_extended.dbrp_min); + dev->data_bt_const.brp_max = le32_to_cpu(bt_const_extended.dbrp_max); + dev->data_bt_const.brp_inc = le32_to_cpu(bt_const_extended.dbrp_inc); dev->can.data_bittiming_const = &dev->data_bt_const; - - kfree(bt_const_extended); } SET_NETDEV_DEV(netdev, &intf->dev); @@ -1329,64 +1258,51 @@ static int gs_usb_probe(struct usb_interface *intf, struct usb_device *udev = interface_to_usbdev(intf); struct gs_host_frame *hf; struct gs_usb *dev; - int rc = -ENOMEM; + struct gs_host_config hconf = { + .byte_order = cpu_to_le32(0x0000beef), + }; + struct gs_device_config dconf; unsigned int icount, i; - struct gs_host_config *hconf; - struct gs_device_config *dconf; - - hconf = kmalloc(sizeof(*hconf), GFP_KERNEL); - if (!hconf) - return -ENOMEM; - - hconf->byte_order = cpu_to_le32(0x0000beef); + int rc; /* send host config */ - rc = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), - GS_USB_BREQ_HOST_FORMAT, - USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_INTERFACE, - 1, intf->cur_altsetting->desc.bInterfaceNumber, - hconf, sizeof(*hconf), 1000); - - kfree(hconf); - - if (rc < 0) { + rc = usb_control_msg_send(udev, 0, + GS_USB_BREQ_HOST_FORMAT, + USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_INTERFACE, + 1, intf->cur_altsetting->desc.bInterfaceNumber, + &hconf, sizeof(hconf), 1000, + GFP_KERNEL); + if (rc) { dev_err(&intf->dev, "Couldn't send data format (err=%d)\n", rc); return rc; } - dconf = kmalloc(sizeof(*dconf), GFP_KERNEL); - if (!dconf) - return -ENOMEM; - /* read device config */ - rc = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0), - GS_USB_BREQ_DEVICE_CONFIG, - USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_INTERFACE, - 1, intf->cur_altsetting->desc.bInterfaceNumber, - dconf, sizeof(*dconf), 1000); - if (rc < 0) { + rc = usb_control_msg_recv(udev, 0, + GS_USB_BREQ_DEVICE_CONFIG, + USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_INTERFACE, + 1, intf->cur_altsetting->desc.bInterfaceNumber, + &dconf, sizeof(dconf), 1000, + GFP_KERNEL); + if (rc) { dev_err(&intf->dev, "Couldn't get device config: (err=%d)\n", rc); - kfree(dconf); return rc; } - icount = dconf->icount + 1; + icount = dconf.icount + 1; dev_info(&intf->dev, "Configuring for %u interfaces\n", icount); if (icount > GS_MAX_INTF) { dev_err(&intf->dev, "Driver cannot handle more that %u CAN interfaces\n", GS_MAX_INTF); - kfree(dconf); return -EINVAL; } dev = kzalloc(sizeof(*dev), GFP_KERNEL); - if (!dev) { - kfree(dconf); + if (!dev) return -ENOMEM; - } init_usb_anchor(&dev->rx_submitted); @@ -1396,7 +1312,7 @@ static int gs_usb_probe(struct usb_interface *intf, for (i = 0; i < icount; i++) { unsigned int hf_size_rx = 0; - dev->canch[i] = gs_make_candev(i, intf, dconf); + dev->canch[i] = gs_make_candev(i, intf, &dconf); if (IS_ERR_OR_NULL(dev->canch[i])) { /* save error code to return later */ rc = PTR_ERR(dev->canch[i]); @@ -1407,7 +1323,6 @@ static int gs_usb_probe(struct usb_interface *intf, gs_destroy_candev(dev->canch[i]); usb_kill_anchored_urbs(&dev->rx_submitted); - kfree(dconf); kfree(dev); return rc; } @@ -1430,8 +1345,6 @@ static int gs_usb_probe(struct usb_interface *intf, dev->hf_size_rx = max(dev->hf_size_rx, hf_size_rx); } - kfree(dconf); - return 0; } From 68822f4e74f35168134b0d3ad7e536a15f42ba04 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Sun, 18 Sep 2022 22:59:27 +0200 Subject: [PATCH 08/11] can: gs_usb: gs_make_candev(): clean up error handling Introduce a label to free the allocated candev in case of an error and make use of if. Fix a memory leak if the extended bit timing cannot be read. Extend the error messages to print the number of the failing channel and the symbolic error name. Link: https://lore.kernel.org/all/20220921193902.575416-4-mkl@pengutronix.de Signed-off-by: Marc Kleine-Budde --- drivers/net/can/usb/gs_usb.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c index 28a645409df90..e9b07e5f988fb 100644 --- a/drivers/net/can/usb/gs_usb.c +++ b/drivers/net/can/usb/gs_usb.c @@ -1100,8 +1100,8 @@ static struct gs_can *gs_make_candev(unsigned int channel, if (rc) { dev_err(&intf->dev, - "Couldn't get bit timing const for channel (err=%d)\n", - rc); + "Couldn't get bit timing const for channel %d (%pe)\n", + channel, ERR_PTR(rc)); return ERR_PTR(rc); } @@ -1215,9 +1215,9 @@ static struct gs_can *gs_make_candev(unsigned int channel, 1000, GFP_KERNEL); if (rc) { dev_err(&intf->dev, - "Couldn't get extended bit timing const for channel (err=%d)\n", - rc); - return ERR_PTR(rc); + "Couldn't get extended bit timing const for channel %d (%pe)\n", + channel, ERR_PTR(rc)); + goto out_free_candev; } strcpy(dev->data_bt_const.name, KBUILD_MODNAME); @@ -1237,12 +1237,17 @@ static struct gs_can *gs_make_candev(unsigned int channel, rc = register_candev(dev->netdev); if (rc) { - free_candev(dev->netdev); - dev_err(&intf->dev, "Couldn't register candev (err=%d)\n", rc); - return ERR_PTR(rc); + dev_err(&intf->dev, + "Couldn't register candev for channel %d (%pe)\n", + channel, ERR_PTR(rc)); + goto out_free_candev; } return dev; + + out_free_candev: + free_candev(dev->netdev); + return ERR_PTR(rc); } static void gs_destroy_candev(struct gs_can *dev) From 906e0e6886afcad6f9cd86660d4b0bdf63f4f200 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Sun, 18 Sep 2022 16:41:38 +0200 Subject: [PATCH 09/11] can: gs_usb: add switchable termination support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The candleLight community is working on switchable termination support for the candleLight firmware. As the the Linux CAN framework supports switchable termination add this feature to the gs_usb driver. Devices supporting the feature should set the GS_CAN_FEATURE_TERMINATION and implement the GS_USB_BREQ_SET_TERMINATION and GS_USB_BREQ_GET_TERMINATION control messages. For now the driver assumes for activated termination the standard termination value of 120Ω. Link: https://lore.kernel.org/all/20220923074114.662045-1-mkl@pengutronix.de Link: https://github.com/candle-usb/candleLight_fw/issues/92 Link: https://github.com/candle-usb/candleLight_fw/pull/109 Link: https://github.com/candle-usb/candleLight_fw/pull/108 Cc: Daniel Trevitz Cc: Ryan Edwards Signed-off-by: Marc Kleine-Budde --- drivers/net/can/usb/gs_usb.c | 79 +++++++++++++++++++++++++++++++++++- 1 file changed, 78 insertions(+), 1 deletion(-) diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c index e9b07e5f988fb..fbe9db46a41a2 100644 --- a/drivers/net/can/usb/gs_usb.c +++ b/drivers/net/can/usb/gs_usb.c @@ -64,6 +64,8 @@ enum gs_usb_breq { GS_USB_BREQ_SET_USER_ID, GS_USB_BREQ_DATA_BITTIMING, GS_USB_BREQ_BT_CONST_EXT, + GS_USB_BREQ_SET_TERMINATION, + GS_USB_BREQ_GET_TERMINATION, }; enum gs_can_mode { @@ -87,6 +89,14 @@ enum gs_can_identify_mode { GS_CAN_IDENTIFY_ON }; +enum gs_can_termination_state { + GS_CAN_TERMINATION_STATE_OFF = 0, + GS_CAN_TERMINATION_STATE_ON +}; + +#define GS_USB_TERMINATION_DISABLED CAN_TERMINATION_DISABLED +#define GS_USB_TERMINATION_ENABLED 120 + /* data types passed between host and device */ /* The firmware on the original USB2CAN by Geschwister Schneider @@ -123,6 +133,7 @@ struct gs_device_config { #define GS_CAN_MODE_FD BIT(8) /* GS_CAN_FEATURE_REQ_USB_QUIRK_LPC546XX BIT(9) */ /* GS_CAN_FEATURE_BT_CONST_EXT BIT(10) */ +/* GS_CAN_FEATURE_TERMINATION BIT(11) */ struct gs_device_mode { __le32 mode; @@ -147,6 +158,10 @@ struct gs_identify_mode { __le32 mode; } __packed; +struct gs_device_termination_state { + __le32 state; +} __packed; + #define GS_CAN_FEATURE_LISTEN_ONLY BIT(0) #define GS_CAN_FEATURE_LOOP_BACK BIT(1) #define GS_CAN_FEATURE_TRIPLE_SAMPLE BIT(2) @@ -158,7 +173,8 @@ struct gs_identify_mode { #define GS_CAN_FEATURE_FD BIT(8) #define GS_CAN_FEATURE_REQ_USB_QUIRK_LPC546XX BIT(9) #define GS_CAN_FEATURE_BT_CONST_EXT BIT(10) -#define GS_CAN_FEATURE_MASK GENMASK(10, 0) +#define GS_CAN_FEATURE_TERMINATION BIT(11) +#define GS_CAN_FEATURE_MASK GENMASK(11, 0) /* internal quirks - keep in GS_CAN_FEATURE space for now */ @@ -1080,6 +1096,52 @@ static const struct ethtool_ops gs_usb_ethtool_ops = { .get_ts_info = gs_usb_get_ts_info, }; +static int gs_usb_get_termination(struct net_device *netdev, u16 *term) +{ + struct gs_can *dev = netdev_priv(netdev); + struct gs_device_termination_state term_state; + int rc; + + rc = usb_control_msg_recv(interface_to_usbdev(dev->iface), 0, + GS_USB_BREQ_GET_TERMINATION, + USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_INTERFACE, + dev->channel, 0, + &term_state, sizeof(term_state), 1000, + GFP_KERNEL); + if (rc) + return rc; + + if (term_state.state == cpu_to_le32(GS_CAN_TERMINATION_STATE_ON)) + *term = GS_USB_TERMINATION_ENABLED; + else + *term = GS_USB_TERMINATION_DISABLED; + + return 0; +} + +static int gs_usb_set_termination(struct net_device *netdev, u16 term) +{ + struct gs_can *dev = netdev_priv(netdev); + struct gs_device_termination_state term_state; + + if (term == GS_USB_TERMINATION_ENABLED) + term_state.state = cpu_to_le32(GS_CAN_TERMINATION_STATE_ON); + else + term_state.state = cpu_to_le32(GS_CAN_TERMINATION_STATE_OFF); + + return usb_control_msg_send(interface_to_usbdev(dev->iface), 0, + GS_USB_BREQ_SET_TERMINATION, + USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_INTERFACE, + dev->channel, 0, + &term_state, sizeof(term_state), 1000, + GFP_KERNEL); +} + +static const u16 gs_usb_termination_const[] = { + GS_USB_TERMINATION_DISABLED, + GS_USB_TERMINATION_ENABLED +}; + static struct gs_can *gs_make_candev(unsigned int channel, struct usb_interface *intf, struct gs_device_config *dconf) @@ -1174,6 +1236,21 @@ static struct gs_can *gs_make_candev(unsigned int channel, dev->can.do_set_data_bittiming = gs_usb_set_data_bittiming; } + if (feature & GS_CAN_FEATURE_TERMINATION) { + rc = gs_usb_get_termination(netdev, &dev->can.termination); + if (rc) { + dev->feature &= ~GS_CAN_FEATURE_TERMINATION; + + dev_info(&intf->dev, + "Disabling termination support for channel %d (%pe)\n", + channel, ERR_PTR(rc)); + } else { + dev->can.termination_const = gs_usb_termination_const; + dev->can.termination_const_cnt = ARRAY_SIZE(gs_usb_termination_const); + dev->can.do_set_termination = gs_usb_set_termination; + } + } + /* The CANtact Pro from LinkLayer Labs is based on the * LPC54616 µC, which is affected by the NXP LPC USB transfer * erratum. However, the current firmware (version 2) doesn't From 62f102c0d1563ff6a31082f5d83b886ad2ff7ca0 Mon Sep 17 00:00:00 2001 From: Vasanth Sadhasivan Date: Tue, 20 Sep 2022 11:47:24 -0400 Subject: [PATCH 10/11] can: gs_usb: remove dma allocations DMA allocated buffers are a precious resource. If there is no need for DMA allocations, then it might be worth to use non-dma allocated buffers. After testing the gs_usb driver with and without DMA allocation, there does not seem to be a significant change in latency or CPU utilization either way. Therefore, DMA allocation is not necessary and removed. Internal buffers used within urbs were managed and freed manually. These buffers are no longer needed to be managed by the driver. The URB_FREE_BUFFER flag, allows for the buffers in question to be automatically freed. Co-developed-by: Rhett Aultman Signed-off-by: Rhett Aultman Signed-off-by: Vasanth Sadhasivan Link: https://lore.kernel.org/all/20220920154724.861093-2-rhett.aultman@samsara.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/usb/gs_usb.c | 39 ++++++------------------------------ 1 file changed, 6 insertions(+), 33 deletions(-) diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c index fbe9db46a41a2..f0065d40eb241 100644 --- a/drivers/net/can/usb/gs_usb.c +++ b/drivers/net/can/usb/gs_usb.c @@ -314,8 +314,6 @@ struct gs_can { struct usb_anchor tx_submitted; atomic_t active_tx_urbs; - void *rxbuf[GS_MAX_RX_URBS]; - dma_addr_t rxbuf_dma[GS_MAX_RX_URBS]; }; /* usb interface struct */ @@ -710,9 +708,6 @@ static void gs_usb_xmit_callback(struct urb *urb) if (urb->status) netdev_info(netdev, "usb xmit fail %u\n", txc->echo_id); - - usb_free_coherent(urb->dev, urb->transfer_buffer_length, - urb->transfer_buffer, urb->transfer_dma); } static netdev_tx_t gs_can_start_xmit(struct sk_buff *skb, @@ -741,8 +736,7 @@ static netdev_tx_t gs_can_start_xmit(struct sk_buff *skb, if (!urb) goto nomem_urb; - hf = usb_alloc_coherent(dev->udev, dev->hf_size_tx, GFP_ATOMIC, - &urb->transfer_dma); + hf = kmalloc(dev->hf_size_tx, GFP_ATOMIC); if (!hf) { netdev_err(netdev, "No memory left for USB buffer\n"); goto nomem_hf; @@ -786,7 +780,7 @@ static netdev_tx_t gs_can_start_xmit(struct sk_buff *skb, hf, dev->hf_size_tx, gs_usb_xmit_callback, txc); - urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP; + urb->transfer_flags |= URB_FREE_BUFFER; usb_anchor_urb(urb, &dev->tx_submitted); can_put_echo_skb(skb, netdev, idx, 0); @@ -801,8 +795,6 @@ static netdev_tx_t gs_can_start_xmit(struct sk_buff *skb, gs_free_tx_context(txc); usb_unanchor_urb(urb); - usb_free_coherent(dev->udev, urb->transfer_buffer_length, - urb->transfer_buffer, urb->transfer_dma); if (rc == -ENODEV) { netif_device_detach(netdev); @@ -822,8 +814,7 @@ static netdev_tx_t gs_can_start_xmit(struct sk_buff *skb, return NETDEV_TX_OK; badidx: - usb_free_coherent(dev->udev, urb->transfer_buffer_length, - urb->transfer_buffer, urb->transfer_dma); + kfree(hf); nomem_hf: usb_free_urb(urb); @@ -869,7 +860,6 @@ static int gs_can_open(struct net_device *netdev) for (i = 0; i < GS_MAX_RX_URBS; i++) { struct urb *urb; u8 *buf; - dma_addr_t buf_dma; /* alloc rx urb */ urb = usb_alloc_urb(0, GFP_KERNEL); @@ -877,10 +867,8 @@ static int gs_can_open(struct net_device *netdev) return -ENOMEM; /* alloc rx buffer */ - buf = usb_alloc_coherent(dev->udev, - dev->parent->hf_size_rx, - GFP_KERNEL, - &buf_dma); + buf = kmalloc(dev->parent->hf_size_rx, + GFP_KERNEL); if (!buf) { netdev_err(netdev, "No memory left for USB buffer\n"); @@ -888,8 +876,6 @@ static int gs_can_open(struct net_device *netdev) return -ENOMEM; } - urb->transfer_dma = buf_dma; - /* fill, anchor, and submit rx urb */ usb_fill_bulk_urb(urb, dev->udev, @@ -898,7 +884,7 @@ static int gs_can_open(struct net_device *netdev) buf, dev->parent->hf_size_rx, gs_usb_receive_bulk_callback, parent); - urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP; + urb->transfer_flags |= URB_FREE_BUFFER; usb_anchor_urb(urb, &parent->rx_submitted); @@ -911,17 +897,10 @@ static int gs_can_open(struct net_device *netdev) "usb_submit failed (err=%d)\n", rc); usb_unanchor_urb(urb); - usb_free_coherent(dev->udev, - sizeof(struct gs_host_frame), - buf, - buf_dma); usb_free_urb(urb); break; } - dev->rxbuf[i] = buf; - dev->rxbuf_dma[i] = buf_dma; - /* Drop reference, * USB core will take care of freeing it */ @@ -980,7 +959,6 @@ static int gs_can_close(struct net_device *netdev) int rc; struct gs_can *dev = netdev_priv(netdev); struct gs_usb *parent = dev->parent; - unsigned int i; netif_stop_queue(netdev); @@ -992,11 +970,6 @@ static int gs_can_close(struct net_device *netdev) parent->active_channels--; if (!parent->active_channels) { usb_kill_anchored_urbs(&parent->rx_submitted); - for (i = 0; i < GS_MAX_RX_URBS; i++) - usb_free_coherent(dev->udev, - sizeof(struct gs_host_frame), - dev->rxbuf[i], - dev->rxbuf_dma[i]); } /* Stop sending URBs */ From 6eed756408c69687613a83fd221431c8790cf0bb Mon Sep 17 00:00:00 2001 From: Shang XiaoJing Date: Fri, 23 Sep 2022 17:58:35 +0800 Subject: [PATCH 11/11] can: ctucanfd: Remove redundant dev_err call devm_ioremap_resource() prints error message in itself. Remove the dev_err call to avoid redundant error message. Signed-off-by: Shang XiaoJing Link: https://lore.kernel.org/all/20220923095835.14647-1-shangxiaojing@huawei.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/ctucanfd/ctucanfd_platform.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/net/can/ctucanfd/ctucanfd_platform.c b/drivers/net/can/ctucanfd/ctucanfd_platform.c index 89d54c2151e19..f83684f006ea1 100644 --- a/drivers/net/can/ctucanfd/ctucanfd_platform.c +++ b/drivers/net/can/ctucanfd/ctucanfd_platform.c @@ -58,7 +58,6 @@ static int ctucan_platform_probe(struct platform_device *pdev) res = platform_get_resource(pdev, IORESOURCE_MEM, 0); addr = devm_ioremap_resource(dev, res); if (IS_ERR(addr)) { - dev_err(dev, "Cannot remap address.\n"); ret = PTR_ERR(addr); goto err; }