[PATCH net-next 0/11] pull-request: can-next 2022-09-23

[PATCH net-next 0/11] pull-request: can-next 2022-09-23

[Marc Kleine-Budde]

Hello Jakub, hello David,

this is a pull request of 11 patches for net-next/main.

The first 2 patches are by Ziyang Xuan and optimize registration and
the sending in the CAN BCM protocol a bit.

The next 8 patches target the gs_usb driver. 7 are by me and first fix
the time hardware stamping support (added during this net-next cycle),
rename a variable, convert the usb_control_msg + manual
kmalloc()/kfree() to usb_control_msg_{send,rev}(), clean up the error
handling and add switchable termination support. The patch by Rhett
Aultman and Vasanth Sadhasivan convert the driver from
usb_alloc_coherent()/usb_free_coherent() to kmalloc()/URB_FREE_BUFFER.

The last patch is by Shang XiaoJing and removes an unneeded call to
dev_err() from the ctucanfd driver.

regards,
Marc

---

The following changes since commit d05d9eb79d0cd0f7a978621b4a56a1f2db444f86:

  Merge branch 'net-dsa-remove-unnecessary-set_drvdata' (2022-09-22 19:31:39 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git tags/linux-can-next-for-6.1-20220923

for you to fetch changes up to 6eed756408c69687613a83fd221431c8790cf0bb:

  can: ctucanfd: Remove redundant dev_err call (2022-09-23 13:55:01 +0200)

----------------------------------------------------------------
linux-can-next-for-6.1-20220923

----------------------------------------------------------------
Marc Kleine-Budde (8):
      Merge patch series "can: bcm: can: bcm: random optimizations"
      can: gs_usb: gs_usb_get_timestamp(): fix endpoint parameter for usb_control_msg_recv()
      can: gs_usb: add missing lock to protect struct timecounter::cycle_last
      can: gs_usb: gs_can_open(): initialize time counter before starting device
      can: gs_usb: gs_cmd_reset(): rename variable holding struct gs_can pointer to dev
      can: gs_usb: convert from usb_control_msg() to usb_control_msg_{send,recv}()
      can: gs_usb: gs_make_candev(): clean up error handling
      can: gs_usb: add switchable termination support

Shang XiaoJing (1):
      can: ctucanfd: Remove redundant dev_err call

Vasanth Sadhasivan (1):
      can: gs_usb: remove dma allocations

Ziyang Xuan (2):
      can: bcm: registration process optimization in bcm_module_init()
      can: bcm: check the result of can_send() in bcm_can_tx()

 drivers/net/can/ctucanfd/ctucanfd_platform.c |   1 -
 drivers/net/can/usb/gs_usb.c                 | 478 +++++++++++++--------------
 net/can/bcm.c                                |  25 +-
 3 files changed, 249 insertions(+), 255 deletions(-)

[PATCH net-next 01/11] can: bcm: registration process optimization in bcm_module_init()

[Marc Kleine-Budde]

From: Ziyang Xuan <william.xuanziyang@huawei.com>

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 <william.xuanziyang@huawei.com>
Link: https://lore.kernel.org/all/823cff0ebec33fa9389eeaf8b8ded3217c32cb38.1663206163.git.william.xuanziyang@huawei.com
Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 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 e5d179ba6f7d..0a2adb844280 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)

base-commit: d05d9eb79d0cd0f7a978621b4a56a1f2db444f86
-- 
2.35.1

[PATCH net-next 02/11] can: bcm: check the result of can_send() in bcm_can_tx()

[Marc Kleine-Budde]

From: Ziyang Xuan <william.xuanziyang@huawei.com>

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 <mkl@pengutronix.de>
Suggested-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com>
Link: https://lore.kernel.org/all/9851878e74d6d37aee2f1ee76d68361a46f89458.1663206163.git.william.xuanziyang@huawei.com
Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 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 0a2adb844280..27706f6ace34 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)
-- 
2.35.1

[PATCH net-next 03/11] can: gs_usb: gs_usb_get_timestamp(): fix endpoint parameter for usb_control_msg_recv()

[Marc Kleine-Budde]

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 <git@jbrengineering.co.uk>
Tested-by: John Whittington <git@jbrengineering.co.uk>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 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 5e0d280b0cd3..1430368ca327 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,
-- 
2.35.1

[PATCH net-next 04/11] can: gs_usb: add missing lock to protect struct timecounter::cycle_last

[Marc Kleine-Budde]

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 <git@jbrengineering.co.uk>
Tested-by: John Whittington <git@jbrengineering.co.uk>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 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 1430368ca327..9a8a7f1b2002 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, &timestamp);
+	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;
-- 
2.35.1

[PATCH net-next 05/11] can: gs_usb: gs_can_open(): initialize time counter before starting device

[Marc Kleine-Budde]

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 <git@jbrengineering.co.uk
Tested-by: John Whittington <git@jbrengineering.co.uk>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 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 9a8a7f1b2002..aa619dfc3ff2 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);
-- 
2.35.1

[PATCH net-next 06/11] can: gs_usb: gs_cmd_reset(): rename variable holding struct gs_can pointer to dev

[Marc Kleine-Budde]

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 <mkl@pengutronix.de>
---
 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 aa619dfc3ff2..629ab664ca3f 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);
 
-- 
2.35.1

[PATCH net-next 07/11] can: gs_usb: convert from usb_control_msg() to usb_control_msg_{send,recv}()

[Marc Kleine-Budde]

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 <mkl@pengutronix.de>
---
 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 629ab664ca3f..28a645409df9 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;
 }
 
-- 
2.35.1

[PATCH net-next 08/11] can: gs_usb: gs_make_candev(): clean up error handling

[Marc Kleine-Budde]

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 <mkl@pengutronix.de>
---
 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 28a645409df9..e9b07e5f988f 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)
-- 
2.35.1

[PATCH net-next 09/11] can: gs_usb: add switchable termination support

[Marc Kleine-Budde]

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 <daniel.trevitz@wika.com>
Cc: Ryan Edwards <ryan.edwards@gmail.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 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 e9b07e5f988f..fbe9db46a41a 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
-- 
2.35.1

[PATCH net-next 10/11] can: gs_usb: remove dma allocations

[Marc Kleine-Budde]

From: Vasanth Sadhasivan <vasanth.sadhasivan@samsara.com>

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 <rhett.aultman@samsara.com>
Signed-off-by: Rhett Aultman <rhett.aultman@samsara.com>
Signed-off-by: Vasanth Sadhasivan <vasanth.sadhasivan@samsara.com>
Link: https://lore.kernel.org/all/20220920154724.861093-2-rhett.aultman@samsara.com
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 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 fbe9db46a41a..f0065d40eb24 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 */
-- 
2.35.1

[PATCH net-next 11/11] can: ctucanfd: Remove redundant dev_err call

[Marc Kleine-Budde]

From: Shang XiaoJing <shangxiaojing@huawei.com>

devm_ioremap_resource() prints error message in itself. Remove the
dev_err call to avoid redundant error message.

Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
Link: https://lore.kernel.org/all/20220923095835.14647-1-shangxiaojing@huawei.com
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 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 89d54c2151e1..f83684f006ea 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;
 	}
-- 
2.35.1

Re: [PATCH net-next 01/11] can: bcm: registration process optimization in bcm_module_init()

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by Marc Kleine-Budde <mkl@pengutronix.de>:

On Fri, 23 Sep 2022 14:08:49 +0200 you wrote:
> From: Ziyang Xuan <william.xuanziyang@huawei.com>
> 
> 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.
> 
> [...]

Here is the summary with links:
  - [net-next,01/11] can: bcm: registration process optimization in bcm_module_init()
    https://git.kernel.org/netdev/net-next/c/edd1a7e42f1d
  - [net-next,02/11] can: bcm: check the result of can_send() in bcm_can_tx()
    https://git.kernel.org/netdev/net-next/c/3fd7bfd28cfd
  - [net-next,03/11] can: gs_usb: gs_usb_get_timestamp(): fix endpoint parameter for usb_control_msg_recv()
    https://git.kernel.org/netdev/net-next/c/593b5e2f5a4a
  - [net-next,04/11] can: gs_usb: add missing lock to protect struct timecounter::cycle_last
    https://git.kernel.org/netdev/net-next/c/29a8c9ec9090
  - [net-next,05/11] can: gs_usb: gs_can_open(): initialize time counter before starting device
    https://git.kernel.org/netdev/net-next/c/103108cb9673
  - [net-next,06/11] can: gs_usb: gs_cmd_reset(): rename variable holding struct gs_can pointer to dev
    https://git.kernel.org/netdev/net-next/c/002467518688
  - [net-next,07/11] can: gs_usb: convert from usb_control_msg() to usb_control_msg_{send,recv}()
    https://git.kernel.org/netdev/net-next/c/3814ed27548a
  - [net-next,08/11] can: gs_usb: gs_make_candev(): clean up error handling
    https://git.kernel.org/netdev/net-next/c/68822f4e74f3
  - [net-next,09/11] can: gs_usb: add switchable termination support
    https://git.kernel.org/netdev/net-next/c/906e0e6886af
  - [net-next,10/11] can: gs_usb: remove dma allocations
    https://git.kernel.org/netdev/net-next/c/62f102c0d156
  - [net-next,11/11] can: ctucanfd: Remove redundant dev_err call
    https://git.kernel.org/netdev/net-next/c/6eed756408c6

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html