All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC]: can-next gs-usb
@ 2022-03-09 12:41 Marc Kleine-Budde
  2022-03-09 12:41 ` [can-next-rfc 01/21] can: gs_usb: use consistent one space indention Marc Kleine-Budde
                   ` (20 more replies)
  0 siblings, 21 replies; 26+ messages in thread
From: Marc Kleine-Budde @ 2022-03-09 12:41 UTC (permalink / raw)
  To: linux-can; +Cc: kernel

Hello,

this patch series cleans up the gs-usb driver, documents some bits of
the USB ABI used by the widely used open source firmware candleLight,
adds support for up to 3 CAN interfaces per USB device, adds CAN-FD
support, adds quirks for some hardware and software workarounds and
finally adds support for 2 new devices.

regards,
Marc




^ permalink raw reply	[flat|nested] 26+ messages in thread

* [can-next-rfc 01/21] can: gs_usb: use consistent one space indention
  2022-03-09 12:41 [RFC]: can-next gs-usb Marc Kleine-Budde
@ 2022-03-09 12:41 ` Marc Kleine-Budde
  2022-03-09 12:41 ` [can-next-rfc 02/21] can: gs_usb: fix checkpatch warning Marc Kleine-Budde
                   ` (19 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Marc Kleine-Budde @ 2022-03-09 12:41 UTC (permalink / raw)
  To: linux-can; +Cc: kernel, Marc Kleine-Budde

With this patch a consistent one space indention throughout the whole
driver is used.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/gs_usb.c | 38 ++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index d35749fad1ef..a63650b17931 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -21,14 +21,14 @@
 #include <linux/can/error.h>
 
 /* Device specific constants */
-#define USB_GSUSB_1_VENDOR_ID      0x1d50
-#define USB_GSUSB_1_PRODUCT_ID     0x606f
+#define USB_GSUSB_1_VENDOR_ID 0x1d50
+#define USB_GSUSB_1_PRODUCT_ID 0x606f
 
-#define USB_CANDLELIGHT_VENDOR_ID  0x1209
+#define USB_CANDLELIGHT_VENDOR_ID 0x1209
 #define USB_CANDLELIGHT_PRODUCT_ID 0x2323
 
-#define GSUSB_ENDPOINT_IN          1
-#define GSUSB_ENDPOINT_OUT         2
+#define GSUSB_ENDPOINT_IN 1
+#define GSUSB_ENDPOINT_OUT 2
 
 /* Device specific constants */
 enum gs_usb_breq {
@@ -87,11 +87,11 @@ struct gs_device_config {
 	__le32 hw_version;
 } __packed;
 
-#define GS_CAN_MODE_NORMAL               0
-#define GS_CAN_MODE_LISTEN_ONLY          BIT(0)
-#define GS_CAN_MODE_LOOP_BACK            BIT(1)
-#define GS_CAN_MODE_TRIPLE_SAMPLE        BIT(2)
-#define GS_CAN_MODE_ONE_SHOT             BIT(3)
+#define GS_CAN_MODE_NORMAL 0
+#define GS_CAN_MODE_LISTEN_ONLY BIT(0)
+#define GS_CAN_MODE_LOOP_BACK BIT(1)
+#define GS_CAN_MODE_TRIPLE_SAMPLE BIT(2)
+#define GS_CAN_MODE_ONE_SHOT BIT(3)
 
 struct gs_device_mode {
 	__le32 mode;
@@ -116,12 +116,12 @@ struct gs_identify_mode {
 	__le32 mode;
 } __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)
-#define GS_CAN_FEATURE_ONE_SHOT         BIT(3)
-#define GS_CAN_FEATURE_HW_TIMESTAMP     BIT(4)
-#define GS_CAN_FEATURE_IDENTIFY         BIT(5)
+#define GS_CAN_FEATURE_LISTEN_ONLY BIT(0)
+#define GS_CAN_FEATURE_LOOP_BACK BIT(1)
+#define GS_CAN_FEATURE_TRIPLE_SAMPLE BIT(2)
+#define GS_CAN_FEATURE_ONE_SHOT BIT(3)
+#define GS_CAN_FEATURE_HW_TIMESTAMP BIT(4)
+#define GS_CAN_FEATURE_IDENTIFY BIT(5)
 
 struct gs_device_bt_const {
 	__le32 feature;
@@ -1043,10 +1043,10 @@ static const struct usb_device_id gs_usb_table[] = {
 MODULE_DEVICE_TABLE(usb, gs_usb_table);
 
 static struct usb_driver gs_usb_driver = {
-	.name       = "gs_usb",
-	.probe      = gs_usb_probe,
+	.name = "gs_usb",
+	.probe = gs_usb_probe,
 	.disconnect = gs_usb_disconnect,
-	.id_table   = gs_usb_table,
+	.id_table = gs_usb_table,
 };
 
 module_usb_driver(gs_usb_driver);

base-commit: d82a6c5ef9dc0aab296936e1aa4ad28fd5162a55
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [can-next-rfc 02/21] can: gs_usb: fix checkpatch warning
  2022-03-09 12:41 [RFC]: can-next gs-usb Marc Kleine-Budde
  2022-03-09 12:41 ` [can-next-rfc 01/21] can: gs_usb: use consistent one space indention Marc Kleine-Budde
@ 2022-03-09 12:41 ` Marc Kleine-Budde
  2022-03-09 12:41 ` [can-next-rfc 03/21] can: gs_usb: sort include files alphabetically Marc Kleine-Budde
                   ` (18 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Marc Kleine-Budde @ 2022-03-09 12:41 UTC (permalink / raw)
  To: linux-can; +Cc: kernel, Marc Kleine-Budde

This patch fixes a checkpatch warning by converting a "unsigned" into
an "unsigned int".

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/gs_usb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index a63650b17931..894cfc1f7a43 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -1015,8 +1015,9 @@ static int gs_usb_probe(struct usb_interface *intf,
 
 static void gs_usb_disconnect(struct usb_interface *intf)
 {
-	unsigned i;
 	struct gs_usb *dev = usb_get_intfdata(intf);
+	unsigned int i;
+
 	usb_set_intfdata(intf, NULL);
 
 	if (!dev) {
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [can-next-rfc 03/21] can: gs_usb: sort include files alphabetically
  2022-03-09 12:41 [RFC]: can-next gs-usb Marc Kleine-Budde
  2022-03-09 12:41 ` [can-next-rfc 01/21] can: gs_usb: use consistent one space indention Marc Kleine-Budde
  2022-03-09 12:41 ` [can-next-rfc 02/21] can: gs_usb: fix checkpatch warning Marc Kleine-Budde
@ 2022-03-09 12:41 ` Marc Kleine-Budde
  2022-03-09 12:41 ` [can-next-rfc 04/21] can: gs_usb: GS_CAN_FLAG_OVERFLOW: make use of BIT() Marc Kleine-Budde
                   ` (17 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Marc Kleine-Budde @ 2022-03-09 12:41 UTC (permalink / raw)
  To: linux-can; +Cc: kernel, Marc Kleine-Budde

This patch sorts the include files alphabetically.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/gs_usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index 894cfc1f7a43..6231c34b29e9 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -11,9 +11,9 @@
 
 #include <linux/ethtool.h>
 #include <linux/init.h>
-#include <linux/signal.h>
 #include <linux/module.h>
 #include <linux/netdevice.h>
+#include <linux/signal.h>
 #include <linux/usb.h>
 
 #include <linux/can.h>
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [can-next-rfc 04/21] can: gs_usb: GS_CAN_FLAG_OVERFLOW: make use of BIT()
  2022-03-09 12:41 [RFC]: can-next gs-usb Marc Kleine-Budde
                   ` (2 preceding siblings ...)
  2022-03-09 12:41 ` [can-next-rfc 03/21] can: gs_usb: sort include files alphabetically Marc Kleine-Budde
@ 2022-03-09 12:41 ` Marc Kleine-Budde
  2022-03-09 12:41 ` [can-next-rfc 05/21] can: gs_usb: rewrap error messages Marc Kleine-Budde
                   ` (16 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Marc Kleine-Budde @ 2022-03-09 12:41 UTC (permalink / raw)
  To: linux-can; +Cc: kernel, Marc Kleine-Budde

This patch converts the driver to use BIT() to define flags.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/gs_usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index 6231c34b29e9..23dba5c162c6 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -136,7 +136,7 @@ struct gs_device_bt_const {
 	__le32 brp_inc;
 } __packed;
 
-#define GS_CAN_FLAG_OVERFLOW 1
+#define GS_CAN_FLAG_OVERFLOW BIT(0)
 
 struct gs_host_frame {
 	u32 echo_id;
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [can-next-rfc 05/21] can: gs_usb: rewrap error messages
  2022-03-09 12:41 [RFC]: can-next gs-usb Marc Kleine-Budde
                   ` (3 preceding siblings ...)
  2022-03-09 12:41 ` [can-next-rfc 04/21] can: gs_usb: GS_CAN_FLAG_OVERFLOW: make use of BIT() Marc Kleine-Budde
@ 2022-03-09 12:41 ` Marc Kleine-Budde
  2022-03-09 12:41 ` [can-next-rfc 06/21] can: gs_usb: rewrap usb_control_msg() and usb_fill_bulk_urb() Marc Kleine-Budde
                   ` (15 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Marc Kleine-Budde @ 2022-03-09 12:41 UTC (permalink / raw)
  To: linux-can; +Cc: kernel, Marc Kleine-Budde

This patch rewraps the arguments of netdev_err() to make full use of
the standard line length of 80 characters.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/gs_usb.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index 23dba5c162c6..b99f526fdfcd 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -630,8 +630,7 @@ static int gs_can_open(struct net_device *netdev)
 					netif_device_detach(dev->netdev);
 
 				netdev_err(netdev,
-					   "usb_submit failed (err=%d)\n",
-					   rc);
+					   "usb_submit failed (err=%d)\n", rc);
 
 				usb_unanchor_urb(urb);
 				usb_free_urb(urb);
@@ -941,8 +940,7 @@ static int gs_usb_probe(struct usb_interface *intf,
 	kfree(hconf);
 
 	if (rc < 0) {
-		dev_err(&intf->dev, "Couldn't send data format (err=%d)\n",
-			rc);
+		dev_err(&intf->dev, "Couldn't send data format (err=%d)\n", rc);
 		return rc;
 	}
 
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [can-next-rfc 06/21] can: gs_usb: rewrap usb_control_msg() and usb_fill_bulk_urb()
  2022-03-09 12:41 [RFC]: can-next gs-usb Marc Kleine-Budde
                   ` (4 preceding siblings ...)
  2022-03-09 12:41 ` [can-next-rfc 05/21] can: gs_usb: rewrap error messages Marc Kleine-Budde
@ 2022-03-09 12:41 ` Marc Kleine-Budde
  2022-03-09 12:41 ` [can-next-rfc 07/21] can: gs_usb: gs_make_candev(): call SET_NETDEV_DEV() after handling all bt_const->feature Marc Kleine-Budde
                   ` (14 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Marc Kleine-Budde @ 2022-03-09 12:41 UTC (permalink / raw)
  To: linux-can; +Cc: kernel, Marc Kleine-Budde

This patch rewraps the arguments of usb_control_msg() and
usb_fill_bulk_urb() to make full use of the standard line length of 80
characters.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/gs_usb.c | 87 +++++++++---------------------------
 1 file changed, 22 insertions(+), 65 deletions(-)

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index b99f526fdfcd..7ba492150cdb 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -258,11 +258,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);
+			     gsdev->channel, 0, dm, sizeof(*dm), 1000);
 
 	kfree(dm);
 
@@ -392,14 +388,10 @@ static void gs_usb_receive_bulk_callback(struct urb *urb)
 	}
 
  resubmit_urb:
-	usb_fill_bulk_urb(urb,
-			  usbcan->udev,
+	usb_fill_bulk_urb(urb, usbcan->udev,
 			  usb_rcvbulkpipe(usbcan->udev, GSUSB_ENDPOINT_IN),
-			  hf,
-			  sizeof(struct gs_host_frame),
-			  gs_usb_receive_bulk_callback,
-			  usbcan
-			  );
+			  hf, sizeof(struct gs_host_frame),
+			  gs_usb_receive_bulk_callback, usbcan);
 
 	rc = usb_submit_urb(urb, GFP_ATOMIC);
 
@@ -436,11 +428,7 @@ static int gs_usb_set_bittiming(struct net_device *netdev)
 			     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);
+			     dev->channel, 0, dbt, sizeof(*dbt), 1000);
 
 	kfree(dbt);
 
@@ -460,10 +448,8 @@ 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);
+	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,
@@ -519,10 +505,8 @@ static netdev_tx_t gs_can_start_xmit(struct sk_buff *skb,
 
 	usb_fill_bulk_urb(urb, dev->udev,
 			  usb_sndbulkpipe(dev->udev, GSUSB_ENDPOINT_OUT),
-			  hf,
-			  sizeof(*hf),
-			  gs_usb_xmit_callback,
-			  txc);
+			  hf, sizeof(*hf),
+			  gs_usb_xmit_callback, txc);
 
 	urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
 	usb_anchor_urb(urb, &dev->tx_submitted);
@@ -540,9 +524,7 @@ static netdev_tx_t gs_can_start_xmit(struct sk_buff *skb,
 
 		usb_unanchor_urb(urb);
 		usb_free_coherent(dev->udev,
-				  sizeof(*hf),
-				  hf,
-				  urb->transfer_dma);
+				  sizeof(*hf), hf, urb->transfer_dma);
 
 		if (rc == -ENODEV) {
 			netif_device_detach(netdev);
@@ -562,10 +544,7 @@ static netdev_tx_t gs_can_start_xmit(struct sk_buff *skb,
 	return NETDEV_TX_OK;
 
  badidx:
-	usb_free_coherent(dev->udev,
-			  sizeof(*hf),
-			  hf,
-			  urb->transfer_dma);
+	usb_free_coherent(dev->udev, sizeof(*hf), hf, urb->transfer_dma);
  nomem_hf:
 	usb_free_urb(urb);
 
@@ -618,8 +597,7 @@ static int gs_can_open(struct net_device *netdev)
 							  GSUSB_ENDPOINT_IN),
 					  buf,
 					  sizeof(struct gs_host_frame),
-					  gs_usb_receive_bulk_callback,
-					  parent);
+					  gs_usb_receive_bulk_callback, parent);
 			urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
 
 			usb_anchor_urb(urb, &parent->rx_submitted);
@@ -671,13 +649,8 @@ static int gs_can_open(struct net_device *netdev)
 	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);
+			     USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_INTERFACE,
+			     dev->channel, 0, dm, sizeof(*dm), 1000);
 
 	if (rc < 0) {
 		netdev_err(netdev, "Couldn't start device (err=%d)\n", rc);
@@ -754,16 +727,10 @@ static int gs_usb_set_identify(struct net_device *netdev, bool do_identify)
 		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),
+			     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);
+			     USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_INTERFACE,
+			     dev->channel, 0, imode, sizeof(*imode), 100);
 
 	kfree(imode);
 
@@ -813,11 +780,7 @@ static struct gs_can *gs_make_candev(unsigned int channel,
 			     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);
+			     channel, 0, bt_const, sizeof(*bt_const), 1000);
 
 	if (rc < 0) {
 		dev_err(&intf->dev,
@@ -931,11 +894,8 @@ static int gs_usb_probe(struct usb_interface *intf,
 			     usb_sndctrlpipe(interface_to_usbdev(intf), 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);
+			     1, intf->cur_altsetting->desc.bInterfaceNumber,
+			     hconf, sizeof(*hconf), 1000);
 
 	kfree(hconf);
 
@@ -953,11 +913,8 @@ static int gs_usb_probe(struct usb_interface *intf,
 			     usb_rcvctrlpipe(interface_to_usbdev(intf), 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);
+			     1, intf->cur_altsetting->desc.bInterfaceNumber,
+			     dconf, sizeof(*dconf), 1000);
 	if (rc < 0) {
 		dev_err(&intf->dev, "Couldn't get device config: (err=%d)\n",
 			rc);
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [can-next-rfc 07/21] can: gs_usb: gs_make_candev(): call SET_NETDEV_DEV() after handling all bt_const->feature
  2022-03-09 12:41 [RFC]: can-next gs-usb Marc Kleine-Budde
                   ` (5 preceding siblings ...)
  2022-03-09 12:41 ` [can-next-rfc 06/21] can: gs_usb: rewrap usb_control_msg() and usb_fill_bulk_urb() Marc Kleine-Budde
@ 2022-03-09 12:41 ` Marc Kleine-Budde
  2022-03-09 12:41 ` [can-next-rfc 08/21] can: gs_usb: add HW timestamp mode bit Marc Kleine-Budde
                   ` (13 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Marc Kleine-Budde @ 2022-03-09 12:41 UTC (permalink / raw)
  To: linux-can; +Cc: kernel, Marc Kleine-Budde

This patch moves the call to SET_NETDEV_DEV() after all handling
(including cleanup) of the bt_const->feature is done. This looks more
consistent.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/gs_usb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index 7ba492150cdb..fa370549bd9e 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -849,14 +849,14 @@ static struct gs_can *gs_make_candev(unsigned int channel,
 	if (feature & GS_CAN_FEATURE_ONE_SHOT)
 		dev->can.ctrlmode_supported |= CAN_CTRLMODE_ONE_SHOT;
 
-	SET_NETDEV_DEV(netdev, &intf->dev);
-
 	if (le32_to_cpu(dconf->sw_version) > 1)
 		if (feature & GS_CAN_FEATURE_IDENTIFY)
 			netdev->ethtool_ops = &gs_usb_ethtool_ops;
 
 	kfree(bt_const);
 
+	SET_NETDEV_DEV(netdev, &intf->dev);
+
 	rc = register_candev(dev->netdev);
 	if (rc) {
 		free_candev(dev->netdev);
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [can-next-rfc 08/21] can: gs_usb: add HW timestamp mode bit
  2022-03-09 12:41 [RFC]: can-next gs-usb Marc Kleine-Budde
                   ` (6 preceding siblings ...)
  2022-03-09 12:41 ` [can-next-rfc 07/21] can: gs_usb: gs_make_candev(): call SET_NETDEV_DEV() after handling all bt_const->feature Marc Kleine-Budde
@ 2022-03-09 12:41 ` Marc Kleine-Budde
  2022-03-09 12:41 ` [can-next-rfc 09/21] can: gs_usb: update GS_CAN_FEATURE_IDENTIFY documentation Marc Kleine-Budde
                   ` (12 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Marc Kleine-Budde @ 2022-03-09 12:41 UTC (permalink / raw)
  To: linux-can; +Cc: kernel, Marc Kleine-Budde

Newer versions of the widely used open source firmware candleLight
support hardware timestamps. The support is activated by setting the
GS_CAN_MODE_HW_TIMESTAMP in the GS_USB_BREQ_MODE request.

Although timestamp support is not yet supported by this driver, add
the missing bit for documentation purpose.

Link: https://github.com/candle-usb/candleLight_fw/commit/44431f4a4354a878fbd15b273bf04fce1dcdff7e
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/gs_usb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index fa370549bd9e..62076c5e8e4e 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -92,6 +92,7 @@ struct gs_device_config {
 #define GS_CAN_MODE_LOOP_BACK BIT(1)
 #define GS_CAN_MODE_TRIPLE_SAMPLE BIT(2)
 #define GS_CAN_MODE_ONE_SHOT BIT(3)
+#define GS_CAN_MODE_HW_TIMESTAMP BIT(4)
 
 struct gs_device_mode {
 	__le32 mode;
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [can-next-rfc 09/21] can: gs_usb: update GS_CAN_FEATURE_IDENTIFY documentation
  2022-03-09 12:41 [RFC]: can-next gs-usb Marc Kleine-Budde
                   ` (7 preceding siblings ...)
  2022-03-09 12:41 ` [can-next-rfc 08/21] can: gs_usb: add HW timestamp mode bit Marc Kleine-Budde
@ 2022-03-09 12:41 ` Marc Kleine-Budde
  2022-03-09 12:41 ` [can-next-rfc 10/21] can: gs_usb: document the USER_ID feature Marc Kleine-Budde
                   ` (11 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Marc Kleine-Budde @ 2022-03-09 12:41 UTC (permalink / raw)
  To: linux-can; +Cc: kernel, Marc Kleine-Budde

In the binary interface a feature bit might have a corresponding mode
bit (of the same value).

The GS_CAN_FEATURE_IDENTIFY feature doesn't come with a mode. Document
this, to avoid gaps when adding more features/modes later.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/gs_usb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index 62076c5e8e4e..ecb57e94993e 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -93,6 +93,7 @@ struct gs_device_config {
 #define GS_CAN_MODE_TRIPLE_SAMPLE BIT(2)
 #define GS_CAN_MODE_ONE_SHOT BIT(3)
 #define GS_CAN_MODE_HW_TIMESTAMP BIT(4)
+/* GS_CAN_FEATURE_IDENTIFY BIT(5) */
 
 struct gs_device_mode {
 	__le32 mode;
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [can-next-rfc 10/21] can: gs_usb: document the USER_ID feature
  2022-03-09 12:41 [RFC]: can-next gs-usb Marc Kleine-Budde
                   ` (8 preceding siblings ...)
  2022-03-09 12:41 ` [can-next-rfc 09/21] can: gs_usb: update GS_CAN_FEATURE_IDENTIFY documentation Marc Kleine-Budde
@ 2022-03-09 12:41 ` Marc Kleine-Budde
  2022-03-09 12:41 ` [can-next-rfc 11/21] can: gs_usb: document the PAD_PKTS_TO_MAX_PKT_SIZE feature Marc Kleine-Budde
                   ` (10 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Marc Kleine-Budde @ 2022-03-09 12:41 UTC (permalink / raw)
  To: linux-can; +Cc: kernel, Marc Kleine-Budde

The widely used open source firmware candleLight has optional support
for reading/writing of an user defined value into the device's flash.

This is indicated by the GS_CAN_FEATURE_USER_ID feature. The
corresponding request are GS_USB_BREQ_GET_USER_ID and
GS_USB_BREQ_SET_USER_ID.

This patch documents these values.

Link: https://github.com/candle-usb/candleLight_fw/commit/1453d70dc9a9d98ac254893ba5114b8e826e0e39
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/gs_usb.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index ecb57e94993e..e739980589d7 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -40,6 +40,8 @@ enum gs_usb_breq {
 	GS_USB_BREQ_DEVICE_CONFIG,
 	GS_USB_BREQ_TIMESTAMP,
 	GS_USB_BREQ_IDENTIFY,
+	GS_USB_BREQ_GET_USER_ID,
+	GS_USB_BREQ_SET_USER_ID,
 };
 
 enum gs_can_mode {
@@ -94,6 +96,7 @@ struct gs_device_config {
 #define GS_CAN_MODE_ONE_SHOT BIT(3)
 #define GS_CAN_MODE_HW_TIMESTAMP BIT(4)
 /* GS_CAN_FEATURE_IDENTIFY BIT(5) */
+/* GS_CAN_FEATURE_USER_ID BIT(6) */
 
 struct gs_device_mode {
 	__le32 mode;
@@ -124,6 +127,7 @@ struct gs_identify_mode {
 #define GS_CAN_FEATURE_ONE_SHOT BIT(3)
 #define GS_CAN_FEATURE_HW_TIMESTAMP BIT(4)
 #define GS_CAN_FEATURE_IDENTIFY BIT(5)
+#define GS_CAN_FEATURE_USER_ID BIT(6)
 
 struct gs_device_bt_const {
 	__le32 feature;
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [can-next-rfc 11/21] can: gs_usb: document the PAD_PKTS_TO_MAX_PKT_SIZE feature
  2022-03-09 12:41 [RFC]: can-next gs-usb Marc Kleine-Budde
                   ` (9 preceding siblings ...)
  2022-03-09 12:41 ` [can-next-rfc 10/21] can: gs_usb: document the USER_ID feature Marc Kleine-Budde
@ 2022-03-09 12:41 ` Marc Kleine-Budde
  2022-03-09 12:41 ` [can-next-rfc 12/21] can: gs_usb: gs_usb_probe(): introduce udev and make use of it Marc Kleine-Budde
                   ` (9 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Marc Kleine-Budde @ 2022-03-09 12:41 UTC (permalink / raw)
  To: linux-can; +Cc: kernel, Marc Kleine-Budde

The widely used open source firmware candleLight supports padding the
USB packets to max packet size to improve performance on Windows
systems.

This patch documents the bit.

Link: https://github.com/candle-usb/candleLight_fw/pull/7
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/gs_usb.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index e739980589d7..52c84792361e 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -97,6 +97,7 @@ struct gs_device_config {
 #define GS_CAN_MODE_HW_TIMESTAMP BIT(4)
 /* GS_CAN_FEATURE_IDENTIFY BIT(5) */
 /* GS_CAN_FEATURE_USER_ID BIT(6) */
+#define GS_CAN_MODE_PAD_PKTS_TO_MAX_PKT_SIZE BIT(7)
 
 struct gs_device_mode {
 	__le32 mode;
@@ -128,6 +129,7 @@ struct gs_identify_mode {
 #define GS_CAN_FEATURE_HW_TIMESTAMP BIT(4)
 #define GS_CAN_FEATURE_IDENTIFY BIT(5)
 #define GS_CAN_FEATURE_USER_ID BIT(6)
+#define GS_CAN_FEATURE_PAD_PKTS_TO_MAX_PKT_SIZE BIT(7)
 
 struct gs_device_bt_const {
 	__le32 feature;
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [can-next-rfc 12/21] can: gs_usb: gs_usb_probe(): introduce udev and make use of it
  2022-03-09 12:41 [RFC]: can-next gs-usb Marc Kleine-Budde
                   ` (10 preceding siblings ...)
  2022-03-09 12:41 ` [can-next-rfc 11/21] can: gs_usb: document the PAD_PKTS_TO_MAX_PKT_SIZE feature Marc Kleine-Budde
@ 2022-03-09 12:41 ` Marc Kleine-Budde
  2022-03-09 12:41 ` [can-next-rfc 13/21] can: gs_usb: support up to 3 channels per device Marc Kleine-Budde
                   ` (8 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Marc Kleine-Budde @ 2022-03-09 12:41 UTC (permalink / raw)
  To: linux-can; +Cc: kernel, Marc Kleine-Budde

Introduce the variable udev in the gs_usb_probe() function to hold a
pointer to the struct usb_device. This avoids recalculating the value
several times in this function.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/gs_usb.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index 52c84792361e..f56bfbeae3be 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -885,6 +885,7 @@ static void gs_destroy_candev(struct gs_can *dev)
 static int gs_usb_probe(struct usb_interface *intf,
 			const struct usb_device_id *id)
 {
+	struct usb_device *udev = interface_to_usbdev(intf);
 	struct gs_usb *dev;
 	int rc = -ENOMEM;
 	unsigned int icount, i;
@@ -898,8 +899,7 @@ static int gs_usb_probe(struct usb_interface *intf,
 	hconf->byte_order = cpu_to_le32(0x0000beef);
 
 	/* send host config */
-	rc = usb_control_msg(interface_to_usbdev(intf),
-			     usb_sndctrlpipe(interface_to_usbdev(intf), 0),
+	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,
@@ -917,8 +917,7 @@ static int gs_usb_probe(struct usb_interface *intf,
 		return -ENOMEM;
 
 	/* read device config */
-	rc = usb_control_msg(interface_to_usbdev(intf),
-			     usb_rcvctrlpipe(interface_to_usbdev(intf), 0),
+	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,
@@ -950,7 +949,7 @@ static int gs_usb_probe(struct usb_interface *intf,
 	init_usb_anchor(&dev->rx_submitted);
 
 	usb_set_intfdata(intf, dev);
-	dev->udev = interface_to_usbdev(intf);
+	dev->udev = udev;
 
 	for (i = 0; i < icount; i++) {
 		dev->canch[i] = gs_make_candev(i, intf, dconf);
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [can-next-rfc 13/21] can: gs_usb: support up to 3 channels per device
  2022-03-09 12:41 [RFC]: can-next gs-usb Marc Kleine-Budde
                   ` (11 preceding siblings ...)
  2022-03-09 12:41 ` [can-next-rfc 12/21] can: gs_usb: gs_usb_probe(): introduce udev and make use of it Marc Kleine-Budde
@ 2022-03-09 12:41 ` Marc Kleine-Budde
  2022-03-09 12:41 ` [can-next-rfc 14/21] can: gs_usb: use union and FLEX_ARRAY for data in struct gs_host_frame Marc Kleine-Budde
                   ` (7 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Marc Kleine-Budde @ 2022-03-09 12:41 UTC (permalink / raw)
  To: linux-can; +Cc: kernel, Marc Kleine-Budde, Ryan Edwards

Some STM32G3 chips support up to 3 CAN-FD channels, increase number of
supported channels in this driver to 3 accordingly.

Suggested-by: Ryan Edwards <ryan.edwards@gmail.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/gs_usb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index f56bfbeae3be..4bc10264005b 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -166,9 +166,9 @@ struct gs_host_frame {
 /* Only launch a max of GS_MAX_RX_URBS usb requests at a time. */
 #define GS_MAX_RX_URBS 30
 /* Maximum number of interfaces the driver supports per device.
- * Current hardware only supports 2 interfaces. The future may vary.
+ * Current hardware only supports 3 interfaces. The future may vary.
  */
-#define GS_MAX_INTF 2
+#define GS_MAX_INTF 3
 
 struct gs_tx_context {
 	struct gs_can *dev;
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [can-next-rfc 14/21] can: gs_usb: use union and FLEX_ARRAY for data in struct gs_host_frame
  2022-03-09 12:41 [RFC]: can-next gs-usb Marc Kleine-Budde
                   ` (12 preceding siblings ...)
  2022-03-09 12:41 ` [can-next-rfc 13/21] can: gs_usb: support up to 3 channels per device Marc Kleine-Budde
@ 2022-03-09 12:41 ` Marc Kleine-Budde
  2022-03-09 14:05   ` Vincent Mailhol
  2022-03-09 12:41 ` [can-next-rfc 15/21] can: gs_usb: add CAN-FD support Marc Kleine-Budde
                   ` (6 subsequent siblings)
  20 siblings, 1 reply; 26+ messages in thread
From: Marc Kleine-Budde @ 2022-03-09 12:41 UTC (permalink / raw)
  To: linux-can; +Cc: kernel, Peter Fink, Marc Kleine-Budde

From: Peter Fink <pfink@christ-es.de>

Modify struct gs_host_frame to make use of a union and
DECLARE_FLEX_ARRAY to be able to store different data (lengths), which
will be added in later commits.

Store the gs_host_frame length in TX direction (host -> device) in
struct gs_can::hf_size_tx and RX direction (device -> host) in struct
gs_usb::hf_size_rx so it must be calculated only once.

Signed-off-by: Peter Fink <pfink@christ-es.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/gs_usb.c | 37 +++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index 4bc10264005b..1fe9d9f08c17 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -146,6 +146,10 @@ struct gs_device_bt_const {
 
 #define GS_CAN_FLAG_OVERFLOW BIT(0)
 
+struct classic_can {
+	u8 data[8];
+} __packed;
+
 struct gs_host_frame {
 	u32 echo_id;
 	__le32 can_id;
@@ -155,7 +159,9 @@ struct gs_host_frame {
 	u8 flags;
 	u8 reserved;
 
-	u8 data[8];
+	union {
+		DECLARE_FLEX_ARRAY(struct classic_can, classic_can);
+	};
 } __packed;
 /* The GS USB devices make use of the same flags and masks as in
  * linux/can.h and linux/can/error.h, and no additional mapping is necessary.
@@ -187,6 +193,8 @@ struct gs_can {
 	struct can_bittiming_const bt_const;
 	unsigned int channel;	/* channel number */
 
+	unsigned int hf_size_tx;
+
 	/* This lock prevents a race condition between xmit and receive. */
 	spinlock_t tx_ctx_lock;
 	struct gs_tx_context tx_context[GS_MAX_TX_URBS];
@@ -200,6 +208,7 @@ struct gs_usb {
 	struct gs_can *canch[GS_MAX_INTF];
 	struct usb_anchor rx_submitted;
 	struct usb_device *udev;
+	unsigned int hf_size_rx;
 	u8 active_channels;
 };
 
@@ -343,7 +352,7 @@ static void gs_usb_receive_bulk_callback(struct urb *urb)
 		cf->can_id = le32_to_cpu(hf->can_id);
 
 		can_frame_set_cc_len(cf, hf->can_dlc, dev->can.ctrlmode);
-		memcpy(cf->data, hf->data, 8);
+		memcpy(cf->data, hf->classic_can->data, 8);
 
 		/* ERROR frames tell us information about the controller */
 		if (le32_to_cpu(hf->can_id) & CAN_ERR_FLAG)
@@ -398,7 +407,7 @@ static void gs_usb_receive_bulk_callback(struct urb *urb)
  resubmit_urb:
 	usb_fill_bulk_urb(urb, usbcan->udev,
 			  usb_rcvbulkpipe(usbcan->udev, GSUSB_ENDPOINT_IN),
-			  hf, sizeof(struct gs_host_frame),
+			  hf, dev->parent->hf_size_rx,
 			  gs_usb_receive_bulk_callback, usbcan);
 
 	rc = usb_submit_urb(urb, GFP_ATOMIC);
@@ -485,7 +494,7 @@ static netdev_tx_t gs_can_start_xmit(struct sk_buff *skb,
 	if (!urb)
 		goto nomem_urb;
 
-	hf = usb_alloc_coherent(dev->udev, sizeof(*hf), GFP_ATOMIC,
+	hf = usb_alloc_coherent(dev->udev, dev->hf_size_tx, GFP_ATOMIC,
 				&urb->transfer_dma);
 	if (!hf) {
 		netdev_err(netdev, "No memory left for USB buffer\n");
@@ -509,11 +518,11 @@ static netdev_tx_t gs_can_start_xmit(struct sk_buff *skb,
 	hf->can_id = cpu_to_le32(cf->can_id);
 	hf->can_dlc = can_get_cc_dlc(cf, dev->can.ctrlmode);
 
-	memcpy(hf->data, cf->data, cf->len);
+	memcpy(hf->classic_can->data, cf->data, cf->len);
 
 	usb_fill_bulk_urb(urb, dev->udev,
 			  usb_sndbulkpipe(dev->udev, GSUSB_ENDPOINT_OUT),
-			  hf, sizeof(*hf),
+			  hf, dev->hf_size_tx,
 			  gs_usb_xmit_callback, txc);
 
 	urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
@@ -531,8 +540,8 @@ 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,
-				  sizeof(*hf), hf, urb->transfer_dma);
+		usb_free_coherent(dev->udev, urb->transfer_buffer_length,
+				  urb->transfer_buffer, urb->transfer_dma);
 
 		if (rc == -ENODEV) {
 			netif_device_detach(netdev);
@@ -552,7 +561,8 @@ static netdev_tx_t gs_can_start_xmit(struct sk_buff *skb,
 	return NETDEV_TX_OK;
 
  badidx:
-	usb_free_coherent(dev->udev, sizeof(*hf), hf, urb->transfer_dma);
+	usb_free_coherent(dev->udev, urb->transfer_buffer_length,
+			  urb->transfer_buffer, urb->transfer_dma);
  nomem_hf:
 	usb_free_urb(urb);
 
@@ -569,6 +579,7 @@ static int gs_can_open(struct net_device *netdev)
 	struct gs_usb *parent = dev->parent;
 	int rc, i;
 	struct gs_device_mode *dm;
+	struct gs_host_frame *hf;
 	u32 ctrlmode;
 	u32 flags = 0;
 
@@ -576,6 +587,8 @@ static int gs_can_open(struct net_device *netdev)
 	if (rc)
 		return rc;
 
+	dev->hf_size_tx = struct_size(hf, classic_can, 1);
+
 	if (!parent->active_channels) {
 		for (i = 0; i < GS_MAX_RX_URBS; i++) {
 			struct urb *urb;
@@ -588,7 +601,7 @@ static int gs_can_open(struct net_device *netdev)
 
 			/* alloc rx buffer */
 			buf = usb_alloc_coherent(dev->udev,
-						 sizeof(struct gs_host_frame),
+						 dev->parent->hf_size_rx,
 						 GFP_KERNEL,
 						 &urb->transfer_dma);
 			if (!buf) {
@@ -604,7 +617,7 @@ static int gs_can_open(struct net_device *netdev)
 					  usb_rcvbulkpipe(dev->udev,
 							  GSUSB_ENDPOINT_IN),
 					  buf,
-					  sizeof(struct gs_host_frame),
+					  dev->parent->hf_size_rx,
 					  gs_usb_receive_bulk_callback, parent);
 			urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
 
@@ -886,6 +899,7 @@ static int gs_usb_probe(struct usb_interface *intf,
 			const struct usb_device_id *id)
 {
 	struct usb_device *udev = interface_to_usbdev(intf);
+	struct gs_host_frame *hf;
 	struct gs_usb *dev;
 	int rc = -ENOMEM;
 	unsigned int icount, i;
@@ -947,6 +961,7 @@ static int gs_usb_probe(struct usb_interface *intf,
 	}
 
 	init_usb_anchor(&dev->rx_submitted);
+	dev->hf_size_rx = struct_size(hf, classic_can, 1);
 
 	usb_set_intfdata(intf, dev);
 	dev->udev = udev;
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [can-next-rfc 15/21] can: gs_usb: add CAN-FD support
  2022-03-09 12:41 [RFC]: can-next gs-usb Marc Kleine-Budde
                   ` (13 preceding siblings ...)
  2022-03-09 12:41 ` [can-next-rfc 14/21] can: gs_usb: use union and FLEX_ARRAY for data in struct gs_host_frame Marc Kleine-Budde
@ 2022-03-09 12:41 ` Marc Kleine-Budde
  2022-03-09 12:41 ` [can-next-rfc 16/21] can: gs_usb: add usb quirk for NXP LPC546xx controllers Marc Kleine-Budde
                   ` (5 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Marc Kleine-Budde @ 2022-03-09 12:41 UTC (permalink / raw)
  To: linux-can; +Cc: kernel, Peter Fink, Eric Evenchick, Marc Kleine-Budde

From: Peter Fink <pfink@christ-es.de>

CANtact Pro from Linklayer is the first gs_usb compatible device
supporting CAN-FD with a different HW and re-written candlelight FW.

Support for CAN-FD is indicated by the device setting the
GS_CAN_FEATURE_FD flag. CAN-FD support is requested by the driver with
the GS_CAN_MODE_FD flag. The CAN-FD specific data bit timing
parameters are set with the GS_USB_BREQ_DATA_BITTIMING control
message.

This patch is based on the Eric Evenchick's gs_usb_fd driver (which
itself is a fork of gs_usb). The gs_usb_fd code base was reintegrated
into the gs_usb driver, and reworked to not break the existing
classical-CAN only hardware.

Link: https://github.com/linklayer/gs_usb_fd/issues/2
Co-developed-by: Eric Evenchick <eric@evenchick.com>
Signed-off-by: Eric Evenchick <eric@evenchick.com>
Signed-off-by: Peter Fink <pfink@christ-es.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/gs_usb.c | 124 ++++++++++++++++++++++++++++++-----
 1 file changed, 108 insertions(+), 16 deletions(-)

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index 1fe9d9f08c17..29389de99326 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -42,6 +42,7 @@ enum gs_usb_breq {
 	GS_USB_BREQ_IDENTIFY,
 	GS_USB_BREQ_GET_USER_ID,
 	GS_USB_BREQ_SET_USER_ID,
+	GS_USB_BREQ_DATA_BITTIMING,
 };
 
 enum gs_can_mode {
@@ -98,6 +99,7 @@ struct gs_device_config {
 /* GS_CAN_FEATURE_IDENTIFY BIT(5) */
 /* GS_CAN_FEATURE_USER_ID BIT(6) */
 #define GS_CAN_MODE_PAD_PKTS_TO_MAX_PKT_SIZE BIT(7)
+#define GS_CAN_MODE_FD BIT(8)
 
 struct gs_device_mode {
 	__le32 mode;
@@ -130,6 +132,7 @@ struct gs_identify_mode {
 #define GS_CAN_FEATURE_IDENTIFY BIT(5)
 #define GS_CAN_FEATURE_USER_ID BIT(6)
 #define GS_CAN_FEATURE_PAD_PKTS_TO_MAX_PKT_SIZE BIT(7)
+#define GS_CAN_FEATURE_FD BIT(8)
 
 struct gs_device_bt_const {
 	__le32 feature;
@@ -145,11 +148,18 @@ struct gs_device_bt_const {
 } __packed;
 
 #define GS_CAN_FLAG_OVERFLOW BIT(0)
+#define GS_CAN_FLAG_FD BIT(1)
+#define GS_CAN_FLAG_BRS BIT(2)
+#define GS_CAN_FLAG_ESI BIT(3)
 
 struct classic_can {
 	u8 data[8];
 } __packed;
 
+struct canfd {
+	u8 data[64];
+} __packed;
+
 struct gs_host_frame {
 	u32 echo_id;
 	__le32 can_id;
@@ -161,6 +171,7 @@ struct gs_host_frame {
 
 	union {
 		DECLARE_FLEX_ARRAY(struct classic_can, classic_can);
+		DECLARE_FLEX_ARRAY(struct canfd, canfd);
 	};
 } __packed;
 /* The GS USB devices make use of the same flags and masks as in
@@ -317,6 +328,7 @@ static void gs_usb_receive_bulk_callback(struct urb *urb)
 	struct gs_host_frame *hf = urb->transfer_buffer;
 	struct gs_tx_context *txc;
 	struct can_frame *cf;
+	struct canfd_frame *cfd;
 	struct sk_buff *skb;
 
 	BUG_ON(!usbcan);
@@ -345,18 +357,33 @@ static void gs_usb_receive_bulk_callback(struct urb *urb)
 		return;
 
 	if (hf->echo_id == -1) { /* normal rx */
-		skb = alloc_can_skb(dev->netdev, &cf);
-		if (!skb)
-			return;
+		if (hf->flags & GS_CAN_FLAG_FD) {
+			skb = alloc_canfd_skb(dev->netdev, &cfd);
+			if (!skb)
+				return;
+
+			cfd->can_id = le32_to_cpu(hf->can_id);
+			cfd->len = can_fd_dlc2len(hf->can_dlc);
+			if (hf->flags & GS_CAN_FLAG_BRS)
+				cfd->flags |= CANFD_BRS;
+			if (hf->flags & GS_CAN_FLAG_ESI)
+				cfd->flags |= CANFD_ESI;
+
+			memcpy(cfd->data, hf->canfd->data, cfd->len);
+		} else {
+			skb = alloc_can_skb(dev->netdev, &cf);
+			if (!skb)
+				return;
 
-		cf->can_id = le32_to_cpu(hf->can_id);
+			cf->can_id = le32_to_cpu(hf->can_id);
+			can_frame_set_cc_len(cf, hf->can_dlc, dev->can.ctrlmode);
 
-		can_frame_set_cc_len(cf, hf->can_dlc, dev->can.ctrlmode);
-		memcpy(cf->data, hf->classic_can->data, 8);
+			memcpy(cf->data, hf->classic_can->data, 8);
 
-		/* ERROR frames tell us information about the controller */
-		if (le32_to_cpu(hf->can_id) & CAN_ERR_FLAG)
-			gs_update_state(dev, cf);
+			/* ERROR frames tell us information about the controller */
+			if (le32_to_cpu(hf->can_id) & CAN_ERR_FLAG)
+				gs_update_state(dev, cf);
+		}
 
 		netdev->stats.rx_packets++;
 		netdev->stats.rx_bytes += hf->can_dlc;
@@ -456,6 +483,40 @@ static int gs_usb_set_bittiming(struct net_device *netdev)
 	return (rc > 0) ? 0 : rc;
 }
 
+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;
+	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);
+
+	/* request bit timings */
+	rc = usb_control_msg(interface_to_usbdev(intf),
+			     usb_sndctrlpipe(interface_to_usbdev(intf), 0),
+			     GS_USB_BREQ_DATA_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 data bittimings (err=%d)", rc);
+
+	return (rc > 0) ? 0 : rc;
+}
+
 static void gs_usb_xmit_callback(struct urb *urb)
 {
 	struct gs_tx_context *txc = urb->context;
@@ -477,6 +538,7 @@ static netdev_tx_t gs_can_start_xmit(struct sk_buff *skb,
 	struct urb *urb;
 	struct gs_host_frame *hf;
 	struct can_frame *cf;
+	struct canfd_frame *cfd;
 	int rc;
 	unsigned int idx;
 	struct gs_tx_context *txc;
@@ -513,12 +575,26 @@ static netdev_tx_t gs_can_start_xmit(struct sk_buff *skb,
 	hf->flags = 0;
 	hf->reserved = 0;
 
-	cf = (struct can_frame *)skb->data;
+	if (can_is_canfd_skb(skb)) {
+		cfd = (struct canfd_frame *)skb->data;
+
+		hf->can_id = cpu_to_le32(cfd->can_id);
+		hf->can_dlc = can_fd_len2dlc(cfd->len);
+		hf->flags |= GS_CAN_FLAG_FD;
+		if (cfd->flags & CANFD_BRS)
+			hf->flags |= GS_CAN_FLAG_BRS;
+		if (cfd->flags & CANFD_ESI)
+			hf->flags |= GS_CAN_FLAG_ESI;
 
-	hf->can_id = cpu_to_le32(cf->can_id);
-	hf->can_dlc = can_get_cc_dlc(cf, dev->can.ctrlmode);
+		memcpy(hf->canfd->data, cfd->data, cfd->len);
+	} else {
+		cf = (struct can_frame *)skb->data;
 
-	memcpy(hf->classic_can->data, cf->data, cf->len);
+		hf->can_id = cpu_to_le32(cf->can_id);
+		hf->can_dlc = can_get_cc_dlc(cf, dev->can.ctrlmode);
+
+		memcpy(hf->classic_can->data, cf->data, cf->len);
+	}
 
 	usb_fill_bulk_urb(urb, dev->udev,
 			  usb_sndbulkpipe(dev->udev, GSUSB_ENDPOINT_OUT),
@@ -587,7 +663,13 @@ static int gs_can_open(struct net_device *netdev)
 	if (rc)
 		return rc;
 
-	dev->hf_size_tx = struct_size(hf, classic_can, 1);
+	ctrlmode = dev->can.ctrlmode;
+	if (ctrlmode & CAN_CTRLMODE_FD) {
+		flags |= GS_CAN_MODE_FD;
+		dev->hf_size_tx = struct_size(hf, canfd, 1);
+	} else {
+		dev->hf_size_tx = struct_size(hf, classic_can, 1);
+	}
 
 	if (!parent->active_channels) {
 		for (i = 0; i < GS_MAX_RX_URBS; i++) {
@@ -648,8 +730,6 @@ static int gs_can_open(struct net_device *netdev)
 		return -ENOMEM;
 
 	/* flags */
-	ctrlmode = dev->can.ctrlmode;
-
 	if (ctrlmode & CAN_CTRLMODE_LOOPBACK)
 		flags |= GS_CAN_MODE_LOOP_BACK;
 	else if (ctrlmode & CAN_CTRLMODE_LISTENONLY)
@@ -870,6 +950,12 @@ static struct gs_can *gs_make_candev(unsigned int channel,
 	if (feature & GS_CAN_FEATURE_ONE_SHOT)
 		dev->can.ctrlmode_supported |= CAN_CTRLMODE_ONE_SHOT;
 
+	if (feature & GS_CAN_FEATURE_FD) {
+		dev->can.ctrlmode_supported |= CAN_CTRLMODE_FD;
+		dev->can.data_bittiming_const = &dev->bt_const;
+		dev->can.do_set_data_bittiming = gs_usb_set_data_bittiming;
+	}
+
 	if (le32_to_cpu(dconf->sw_version) > 1)
 		if (feature & GS_CAN_FEATURE_IDENTIFY)
 			netdev->ethtool_ops = &gs_usb_ethtool_ops;
@@ -961,6 +1047,9 @@ static int gs_usb_probe(struct usb_interface *intf,
 	}
 
 	init_usb_anchor(&dev->rx_submitted);
+	/* default to classic CAN, switch to CAN-FD if at least one of
+	 * our channels support CAN-FD.
+	 */
 	dev->hf_size_rx = struct_size(hf, classic_can, 1);
 
 	usb_set_intfdata(intf, dev);
@@ -983,6 +1072,9 @@ static int gs_usb_probe(struct usb_interface *intf,
 			return rc;
 		}
 		dev->canch[i]->parent = dev;
+
+		if (dev->canch[i]->can.ctrlmode_supported & CAN_CTRLMODE_FD)
+			dev->hf_size_rx = struct_size(hf, canfd, 1);
 	}
 
 	kfree(dconf);
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [can-next-rfc 16/21] can: gs_usb: add usb quirk for NXP LPC546xx controllers
  2022-03-09 12:41 [RFC]: can-next gs-usb Marc Kleine-Budde
                   ` (14 preceding siblings ...)
  2022-03-09 12:41 ` [can-next-rfc 15/21] can: gs_usb: add CAN-FD support Marc Kleine-Budde
@ 2022-03-09 12:41 ` Marc Kleine-Budde
  2022-03-09 12:41 ` [can-next-rfc 17/21] can: gs_usb: add quirk for CANtact Pro overlapping GS_USB_BREQ value Marc Kleine-Budde
                   ` (4 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Marc Kleine-Budde @ 2022-03-09 12:41 UTC (permalink / raw)
  To: linux-can
  Cc: kernel, Peter Fink, Christoph Möhring, Alexander Schartner,
	Marc Kleine-Budde

From: Peter Fink <pfink@christ-es.de>

Introduce a workaround for a NXP chip errata on LPC546xx
controllers (Errata sheet LPC546xx / USB.15).

According to the document corruption can occur when the following
conditions are met:
* A TX (IN) transfer happens after a RX (OUT) transfer.
* The RX (OUT) transfer length is 4 + N * 16 (N >= 0) bytes.

Even though the struct gs_host_frame has a size of 76 bytes for a FD
frame, which does not apply to the above rule, corruption could be
seen.

Adding a dummy byte to break the second condition also on transfer
lengths with 4 + N * 8 bytes reliably circumvents USB transfer data
corruption.

The firmware can now request this quirk by setting
GS_CAN_FEATURE_REQ_USB_QUIRK_LPC546XX.

Signed-off-by: Peter Fink <pfink@christ-es.de>
Signed-off-by: Christoph Möhring <cmoehring@christ-es.de>
Signed-off-by: Alexander Schartner <aschartner@christ-es.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/gs_usb.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index 29389de99326..b0651789ccd3 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -9,6 +9,7 @@
  * Many thanks to all socketcan devs!
  */
 
+#include <linux/bitfield.h>
 #include <linux/ethtool.h>
 #include <linux/init.h>
 #include <linux/module.h>
@@ -100,6 +101,7 @@ struct gs_device_config {
 /* GS_CAN_FEATURE_USER_ID BIT(6) */
 #define GS_CAN_MODE_PAD_PKTS_TO_MAX_PKT_SIZE BIT(7)
 #define GS_CAN_MODE_FD BIT(8)
+/* GS_CAN_FEATURE_REQ_USB_QUIRK_LPC546XX BIT(9) */
 
 struct gs_device_mode {
 	__le32 mode;
@@ -133,6 +135,8 @@ struct gs_identify_mode {
 #define GS_CAN_FEATURE_USER_ID BIT(6)
 #define GS_CAN_FEATURE_PAD_PKTS_TO_MAX_PKT_SIZE BIT(7)
 #define GS_CAN_FEATURE_FD BIT(8)
+#define GS_CAN_FEATURE_REQ_USB_QUIRK_LPC546XX BIT(9)
+#define GS_CAN_FEATURE_MASK GENMASK(9, 0)
 
 struct gs_device_bt_const {
 	__le32 feature;
@@ -156,10 +160,20 @@ struct classic_can {
 	u8 data[8];
 } __packed;
 
+struct classic_can_quirk {
+	u8 data[8];
+	u8 quirk;
+} __packed;
+
 struct canfd {
 	u8 data[64];
 } __packed;
 
+struct canfd_quirk {
+	u8 data[64];
+	u8 quirk;
+} __packed;
+
 struct gs_host_frame {
 	u32 echo_id;
 	__le32 can_id;
@@ -171,7 +185,9 @@ struct gs_host_frame {
 
 	union {
 		DECLARE_FLEX_ARRAY(struct classic_can, classic_can);
+		DECLARE_FLEX_ARRAY(struct classic_can_quirk, classic_can_quirk);
 		DECLARE_FLEX_ARRAY(struct canfd, canfd);
+		DECLARE_FLEX_ARRAY(struct canfd_quirk, canfd_quirk);
 	};
 } __packed;
 /* The GS USB devices make use of the same flags and masks as in
@@ -204,6 +220,7 @@ struct gs_can {
 	struct can_bittiming_const bt_const;
 	unsigned int channel;	/* channel number */
 
+	u32 feature;
 	unsigned int hf_size_tx;
 
 	/* This lock prevents a race condition between xmit and receive. */
@@ -666,9 +683,16 @@ static int gs_can_open(struct net_device *netdev)
 	ctrlmode = dev->can.ctrlmode;
 	if (ctrlmode & CAN_CTRLMODE_FD) {
 		flags |= GS_CAN_MODE_FD;
-		dev->hf_size_tx = struct_size(hf, canfd, 1);
+
+		if (dev->feature & GS_CAN_FEATURE_REQ_USB_QUIRK_LPC546XX)
+			dev->hf_size_tx = struct_size(hf, canfd_quirk, 1);
+		else
+			dev->hf_size_tx = struct_size(hf, canfd, 1);
 	} else {
-		dev->hf_size_tx = struct_size(hf, classic_can, 1);
+		if (dev->feature & GS_CAN_FEATURE_REQ_USB_QUIRK_LPC546XX)
+			dev->hf_size_tx = struct_size(hf, classic_can_quirk, 1);
+		else
+			dev->hf_size_tx = struct_size(hf, classic_can, 1);
 	}
 
 	if (!parent->active_channels) {
@@ -938,6 +962,7 @@ static struct gs_can *gs_make_candev(unsigned int channel,
 	dev->can.ctrlmode_supported = CAN_CTRLMODE_CC_LEN8_DLC;
 
 	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;
 
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [can-next-rfc 17/21] can: gs_usb: add quirk for CANtact Pro overlapping GS_USB_BREQ value
  2022-03-09 12:41 [RFC]: can-next gs-usb Marc Kleine-Budde
                   ` (15 preceding siblings ...)
  2022-03-09 12:41 ` [can-next-rfc 16/21] can: gs_usb: add usb quirk for NXP LPC546xx controllers Marc Kleine-Budde
@ 2022-03-09 12:41 ` Marc Kleine-Budde
  2022-03-09 12:41 ` [can-next-rfc 18/21] can: gs_usb: activate quirks for CANtact Pro unconditionally Marc Kleine-Budde
                   ` (3 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Marc Kleine-Budde @ 2022-03-09 12:41 UTC (permalink / raw)
  To: linux-can; +Cc: kernel, Marc Kleine-Budde

For the GS_USB_BREQ_DATA_BITTIMING USB control message the CANtact Pro
firmware uses a request value, which is already used by the
candleLight firmware for a different
purpose (GS_USB_BREQ_GET_USER_ID).

This patch adds a quirk to use the CANtact Pro's value for the
GS_USB_BREQ_DATA_BITTIMING USB control message instead of the official
one.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/gs_usb.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index b0651789ccd3..1661b91de364 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -42,6 +42,7 @@ enum gs_usb_breq {
 	GS_USB_BREQ_TIMESTAMP,
 	GS_USB_BREQ_IDENTIFY,
 	GS_USB_BREQ_GET_USER_ID,
+	GS_USB_BREQ_QUIRK_CANTACT_PRO_DATA_BITTIMING = GS_USB_BREQ_GET_USER_ID,
 	GS_USB_BREQ_SET_USER_ID,
 	GS_USB_BREQ_DATA_BITTIMING,
 };
@@ -138,6 +139,13 @@ struct gs_identify_mode {
 #define GS_CAN_FEATURE_REQ_USB_QUIRK_LPC546XX BIT(9)
 #define GS_CAN_FEATURE_MASK GENMASK(9, 0)
 
+/* internal quirks - keep in GS_CAN_FEATURE space for now */
+
+/* CANtact Pro original firmware:
+ * BREQ DATA_BITTIMING overlaps with GET_USER_ID
+ */
+#define GS_CAN_FEATURE_QUIRK_BREQ_CANTACT_PRO BIT(31)
+
 struct gs_device_bt_const {
 	__le32 feature;
 	__le32 fclk_can;
@@ -506,6 +514,7 @@ static int gs_usb_set_data_bittiming(struct net_device *netdev)
 	struct can_bittiming *bt = &dev->can.data_bittiming;
 	struct usb_interface *intf = dev->iface;
 	struct gs_device_bittiming *dbt;
+	u8 request = GS_USB_BREQ_DATA_BITTIMING;
 	int rc;
 
 	dbt = kmalloc(sizeof(*dbt), GFP_KERNEL);
@@ -518,10 +527,13 @@ static int gs_usb_set_data_bittiming(struct net_device *netdev)
 	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),
-			     GS_USB_BREQ_DATA_BITTIMING,
+			     request,
 			     USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_INTERFACE,
 			     dev->channel, 0, dbt, sizeof(*dbt), 1000);
 
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [can-next-rfc 18/21] can: gs_usb: activate quirks for CANtact Pro unconditionally
  2022-03-09 12:41 [RFC]: can-next gs-usb Marc Kleine-Budde
                   ` (16 preceding siblings ...)
  2022-03-09 12:41 ` [can-next-rfc 17/21] can: gs_usb: add quirk for CANtact Pro overlapping GS_USB_BREQ value Marc Kleine-Budde
@ 2022-03-09 12:41 ` Marc Kleine-Budde
  2022-03-09 12:41 ` [can-next-rfc 19/21] can: gs_usb: add extended bt_const feature Marc Kleine-Budde
                   ` (2 subsequent siblings)
  20 siblings, 0 replies; 26+ messages in thread
From: Marc Kleine-Budde @ 2022-03-09 12:41 UTC (permalink / raw)
  To: linux-can
  Cc: kernel, Peter Fink, Ryan Edwards, Christoph Möhring,
	Marc Kleine-Budde

From: Peter Fink <pfink@christ-es.de>

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 set the
GS_CAN_FEATURE_REQ_USB_QUIRK_LPC546XX bit.

This patch sets the feature GS_CAN_FEATURE_REQ_USB_QUIRK_LPC546XX to
workaround this issue.

For the GS_USB_BREQ_DATA_BITTIMING USB control message the CANtact Pro
firmware uses a request value, which is already used by the
candleLight firmware for a different purpose
(GS_USB_BREQ_GET_USER_ID).

This patch set the feature GS_CAN_FEATURE_QUIRK_BREQ_CANTACT_PRO to
workaround this issue.

Cc: Ryan Edwards <ryan.edwards@gmail.com>
Signed-off-by: Peter Fink <pfink@christ-es.de>
Signed-off-by: Christoph Möhring <cmoehring@christ-es.de>
[mkl: improve check for CANtact Pro and add GS_CAN_FEATURE_QUIRK_BREQ_OVERLAP quirk]
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/gs_usb.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index 1661b91de364..915c5dd8199b 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -993,6 +993,29 @@ static struct gs_can *gs_make_candev(unsigned int channel,
 		dev->can.do_set_data_bittiming = gs_usb_set_data_bittiming;
 	}
 
+	/* 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
+	 * set the GS_CAN_FEATURE_REQ_USB_QUIRK_LPC546XX bit. Set the
+	 * feature GS_CAN_FEATURE_REQ_USB_QUIRK_LPC546XX to workaround
+	 * this issue.
+	 *
+	 * For the GS_USB_BREQ_DATA_BITTIMING USB control message the
+	 * CANtact Pro firmware uses a request value, which is already
+	 * used by the candleLight firmware for a different purpose
+	 * (GS_USB_BREQ_GET_USER_ID). Set the feature
+	 * GS_CAN_FEATURE_QUIRK_BREQ_CANTACT_PRO to workaround this
+	 * issue.
+	 */
+	if (dev->udev->descriptor.idVendor == cpu_to_le16(USB_GSUSB_1_VENDOR_ID) &&
+	    dev->udev->descriptor.idProduct == cpu_to_le16(USB_GSUSB_1_PRODUCT_ID) &&
+	    dev->udev->manufacturer && dev->udev->product &&
+	    !strcmp(dev->udev->manufacturer, "LinkLayer Labs") &&
+	    !strcmp(dev->udev->product, "CANtact Pro") &&
+	    (le32_to_cpu(dconf->sw_version) <= 2))
+		dev->feature |= GS_CAN_FEATURE_REQ_USB_QUIRK_LPC546XX |
+			GS_CAN_FEATURE_QUIRK_BREQ_CANTACT_PRO;
+
 	if (le32_to_cpu(dconf->sw_version) > 1)
 		if (feature & GS_CAN_FEATURE_IDENTIFY)
 			netdev->ethtool_ops = &gs_usb_ethtool_ops;
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [can-next-rfc 19/21] can: gs_usb: add extended bt_const feature
  2022-03-09 12:41 [RFC]: can-next gs-usb Marc Kleine-Budde
                   ` (17 preceding siblings ...)
  2022-03-09 12:41 ` [can-next-rfc 18/21] can: gs_usb: activate quirks for CANtact Pro unconditionally Marc Kleine-Budde
@ 2022-03-09 12:41 ` Marc Kleine-Budde
  2022-03-09 12:41 ` [can-next-rfc 20/21] can: gs_usb: add VID/PID for CES CANext FD devices Marc Kleine-Budde
  2022-03-09 12:41 ` [can-next-rfc 21/21] can: gs_usb: add VID/PID for ABE CAN Debugger devices Marc Kleine-Budde
  20 siblings, 0 replies; 26+ messages in thread
From: Marc Kleine-Budde @ 2022-03-09 12:41 UTC (permalink / raw)
  To: linux-can; +Cc: kernel, Peter Fink, Christoph Möhring, Marc Kleine-Budde

From: Peter Fink <pfink@christ-es.de>

For example CANext FD needs to distinguish between bittiming constants
valid for arbitration phase and data phase to reach maximum
performance at higher speeds.

Signed-off-by: Peter Fink <pfink@christ-es.de>
Signed-off-by: Christoph Möhring <cmoehring@christ-es.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/gs_usb.c | 70 ++++++++++++++++++++++++++++++++++--
 1 file changed, 68 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index 915c5dd8199b..8bc219823ccf 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -45,6 +45,7 @@ enum gs_usb_breq {
 	GS_USB_BREQ_QUIRK_CANTACT_PRO_DATA_BITTIMING = GS_USB_BREQ_GET_USER_ID,
 	GS_USB_BREQ_SET_USER_ID,
 	GS_USB_BREQ_DATA_BITTIMING,
+	GS_USB_BREQ_BT_CONST_EXT,
 };
 
 enum gs_can_mode {
@@ -103,6 +104,7 @@ struct gs_device_config {
 #define GS_CAN_MODE_PAD_PKTS_TO_MAX_PKT_SIZE BIT(7)
 #define GS_CAN_MODE_FD BIT(8)
 /* GS_CAN_FEATURE_REQ_USB_QUIRK_LPC546XX BIT(9) */
+/* GS_CAN_FEATURE_BT_CONST_EXT BIT(10) */
 
 struct gs_device_mode {
 	__le32 mode;
@@ -137,7 +139,8 @@ struct gs_identify_mode {
 #define GS_CAN_FEATURE_PAD_PKTS_TO_MAX_PKT_SIZE BIT(7)
 #define GS_CAN_FEATURE_FD BIT(8)
 #define GS_CAN_FEATURE_REQ_USB_QUIRK_LPC546XX BIT(9)
-#define GS_CAN_FEATURE_MASK GENMASK(9, 0)
+#define GS_CAN_FEATURE_BT_CONST_EXT BIT(10)
+#define GS_CAN_FEATURE_MASK GENMASK(10, 0)
 
 /* internal quirks - keep in GS_CAN_FEATURE space for now */
 
@@ -159,6 +162,28 @@ struct gs_device_bt_const {
 	__le32 brp_inc;
 } __packed;
 
+struct gs_device_bt_const_extended {
+	__le32 feature;
+	__le32 fclk_can;
+	__le32 tseg1_min;
+	__le32 tseg1_max;
+	__le32 tseg2_min;
+	__le32 tseg2_max;
+	__le32 sjw_max;
+	__le32 brp_min;
+	__le32 brp_max;
+	__le32 brp_inc;
+
+	__le32 dtseg1_min;
+	__le32 dtseg1_max;
+	__le32 dtseg2_min;
+	__le32 dtseg2_max;
+	__le32 dsjw_max;
+	__le32 dbrp_min;
+	__le32 dbrp_max;
+	__le32 dbrp_inc;
+} __packed;
+
 #define GS_CAN_FLAG_OVERFLOW BIT(0)
 #define GS_CAN_FLAG_FD BIT(1)
 #define GS_CAN_FLAG_BRS BIT(2)
@@ -225,7 +250,7 @@ struct gs_can {
 	struct usb_device *udev;
 	struct usb_interface *iface;
 
-	struct can_bittiming_const bt_const;
+	struct can_bittiming_const bt_const, data_bt_const;
 	unsigned int channel;	/* channel number */
 
 	u32 feature;
@@ -906,6 +931,7 @@ static struct gs_can *gs_make_candev(unsigned int channel,
 	struct net_device *netdev;
 	int rc;
 	struct gs_device_bt_const *bt_const;
+	struct gs_device_bt_const_extended *bt_const_extended;
 	u32 feature;
 
 	bt_const = kmalloc(sizeof(*bt_const), GFP_KERNEL);
@@ -989,6 +1015,9 @@ static struct gs_can *gs_make_candev(unsigned int channel,
 
 	if (feature & GS_CAN_FEATURE_FD) {
 		dev->can.ctrlmode_supported |= CAN_CTRLMODE_FD;
+		/* The data bit timing will be overwritten, if
+		 * GS_CAN_FEATURE_BT_CONST_EXT is set.
+		 */
 		dev->can.data_bittiming_const = &dev->bt_const;
 		dev->can.do_set_data_bittiming = gs_usb_set_data_bittiming;
 	}
@@ -1022,6 +1051,43 @@ static struct gs_can *gs_make_candev(unsigned int channel,
 
 	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) {
+			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, "gs_usb");
+		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;
+	}
+
 	SET_NETDEV_DEV(netdev, &intf->dev);
 
 	rc = register_candev(dev->netdev);
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [can-next-rfc 20/21] can: gs_usb: add VID/PID for CES CANext FD devices
  2022-03-09 12:41 [RFC]: can-next gs-usb Marc Kleine-Budde
                   ` (18 preceding siblings ...)
  2022-03-09 12:41 ` [can-next-rfc 19/21] can: gs_usb: add extended bt_const feature Marc Kleine-Budde
@ 2022-03-09 12:41 ` Marc Kleine-Budde
  2022-03-09 12:41 ` [can-next-rfc 21/21] can: gs_usb: add VID/PID for ABE CAN Debugger devices Marc Kleine-Budde
  20 siblings, 0 replies; 26+ messages in thread
From: Marc Kleine-Budde @ 2022-03-09 12:41 UTC (permalink / raw)
  To: linux-can; +Cc: kernel, Peter Fink, Marc Kleine-Budde

From: Peter Fink <pfink@christ-es.de>

Add support for Christ Electronic Systems GmbH
CANext FD devices using the gs_usb driver.

Signed-off-by: Peter Fink <pfink@christ-es.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/gs_usb.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index 8bc219823ccf..816b4d8b1182 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -28,6 +28,9 @@
 #define USB_CANDLELIGHT_VENDOR_ID 0x1209
 #define USB_CANDLELIGHT_PRODUCT_ID 0x2323
 
+#define USB_CES_CANEXT_FD_VENDOR_ID 0x1cd2
+#define USB_CES_CANEXT_FD_PRODUCT_ID 0x606f
+
 #define GSUSB_ENDPOINT_IN 1
 #define GSUSB_ENDPOINT_OUT 2
 
@@ -1233,6 +1236,8 @@ static const struct usb_device_id gs_usb_table[] = {
 				      USB_GSUSB_1_PRODUCT_ID, 0) },
 	{ USB_DEVICE_INTERFACE_NUMBER(USB_CANDLELIGHT_VENDOR_ID,
 				      USB_CANDLELIGHT_PRODUCT_ID, 0) },
+	{ USB_DEVICE_INTERFACE_NUMBER(USB_CES_CANEXT_FD_VENDOR_ID,
+				      USB_CES_CANEXT_FD_PRODUCT_ID, 0) },
 	{} /* Terminating entry */
 };
 
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [can-next-rfc 21/21] can: gs_usb: add VID/PID for ABE CAN Debugger devices
  2022-03-09 12:41 [RFC]: can-next gs-usb Marc Kleine-Budde
                   ` (19 preceding siblings ...)
  2022-03-09 12:41 ` [can-next-rfc 20/21] can: gs_usb: add VID/PID for CES CANext FD devices Marc Kleine-Budde
@ 2022-03-09 12:41 ` Marc Kleine-Budde
  20 siblings, 0 replies; 26+ messages in thread
From: Marc Kleine-Budde @ 2022-03-09 12:41 UTC (permalink / raw)
  To: linux-can; +Cc: kernel, Ben Evans, Peter Fink, Marc Kleine-Budde

From: Ben Evans <benny.j.evans92@gmail.com>

Add support for ABE CAN Debugger devices using the gs_usb driver.

Signed-off-by: Ben Evans <benny.j.evans92@gmail.com>
Signed-off-by: Peter Fink <pfink@christ-es.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/gs_usb.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index 816b4d8b1182..67408e316062 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -31,6 +31,9 @@
 #define USB_CES_CANEXT_FD_VENDOR_ID 0x1cd2
 #define USB_CES_CANEXT_FD_PRODUCT_ID 0x606f
 
+#define USB_ABE_CANDEBUGGER_FD_VENDOR_ID 0x16d0
+#define USB_ABE_CANDEBUGGER_FD_PRODUCT_ID 0x10b8
+
 #define GSUSB_ENDPOINT_IN 1
 #define GSUSB_ENDPOINT_OUT 2
 
@@ -1238,6 +1241,8 @@ static const struct usb_device_id gs_usb_table[] = {
 				      USB_CANDLELIGHT_PRODUCT_ID, 0) },
 	{ USB_DEVICE_INTERFACE_NUMBER(USB_CES_CANEXT_FD_VENDOR_ID,
 				      USB_CES_CANEXT_FD_PRODUCT_ID, 0) },
+	{ USB_DEVICE_INTERFACE_NUMBER(USB_ABE_CANDEBUGGER_FD_VENDOR_ID,
+				      USB_ABE_CANDEBUGGER_FD_PRODUCT_ID, 0) },
 	{} /* Terminating entry */
 };
 
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [can-next-rfc 14/21] can: gs_usb: use union and FLEX_ARRAY for data in struct gs_host_frame
  2022-03-09 12:41 ` [can-next-rfc 14/21] can: gs_usb: use union and FLEX_ARRAY for data in struct gs_host_frame Marc Kleine-Budde
@ 2022-03-09 14:05   ` Vincent Mailhol
  2022-03-09 14:16     ` Vincent Mailhol
  2022-03-09 14:18     ` Marc Kleine-Budde
  0 siblings, 2 replies; 26+ messages in thread
From: Vincent Mailhol @ 2022-03-09 14:05 UTC (permalink / raw)
  To: Marc Kleine-Budde; +Cc: linux-can, kernel, Peter Fink

On Wed. 9 Mar 2022 à 22:39, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> From: Peter Fink <pfink@christ-es.de>
>
> Modify struct gs_host_frame to make use of a union and
> DECLARE_FLEX_ARRAY to be able to store different data (lengths), which
> will be added in later commits.
>
> Store the gs_host_frame length in TX direction (host -> device) in
> struct gs_can::hf_size_tx and RX direction (device -> host) in struct
> gs_usb::hf_size_rx so it must be calculated only once.
>
> Signed-off-by: Peter Fink <pfink@christ-es.de>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
>  drivers/net/can/usb/gs_usb.c | 37 +++++++++++++++++++++++++-----------
>  1 file changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
> index 4bc10264005b..1fe9d9f08c17 100644
> --- a/drivers/net/can/usb/gs_usb.c
> +++ b/drivers/net/can/usb/gs_usb.c
> @@ -146,6 +146,10 @@ struct gs_device_bt_const {
>
>  #define GS_CAN_FLAG_OVERFLOW BIT(0)
>
> +struct classic_can {
> +       u8 data[8];
> +} __packed;
> +
>  struct gs_host_frame {
>         u32 echo_id;
>         __le32 can_id;
> @@ -155,7 +159,9 @@ struct gs_host_frame {
>         u8 flags;
>         u8 reserved;
>
> -       u8 data[8];
> +       union {
> +               DECLARE_FLEX_ARRAY(struct classic_can, classic_can);
> +       };
>  } __packed;
>  /* The GS USB devices make use of the same flags and masks as in
>   * linux/can.h and linux/can/error.h, and no additional mapping is necessary.
> @@ -187,6 +193,8 @@ struct gs_can {
>         struct can_bittiming_const bt_const;
>         unsigned int channel;   /* channel number */
>
> +       unsigned int hf_size_tx;
> +
>         /* This lock prevents a race condition between xmit and receive. */
>         spinlock_t tx_ctx_lock;
>         struct gs_tx_context tx_context[GS_MAX_TX_URBS];
> @@ -200,6 +208,7 @@ struct gs_usb {
>         struct gs_can *canch[GS_MAX_INTF];
>         struct usb_anchor rx_submitted;
>         struct usb_device *udev;
> +       unsigned int hf_size_rx;
>         u8 active_channels;
>  };
>
> @@ -343,7 +352,7 @@ static void gs_usb_receive_bulk_callback(struct urb *urb)
>                 cf->can_id = le32_to_cpu(hf->can_id);
>
>                 can_frame_set_cc_len(cf, hf->can_dlc, dev->can.ctrlmode);
> -               memcpy(cf->data, hf->data, 8);
> +               memcpy(cf->data, hf->classic_can->data, 8);
>
>                 /* ERROR frames tell us information about the controller */
>                 if (le32_to_cpu(hf->can_id) & CAN_ERR_FLAG)
> @@ -398,7 +407,7 @@ static void gs_usb_receive_bulk_callback(struct urb *urb)
>   resubmit_urb:
>         usb_fill_bulk_urb(urb, usbcan->udev,
>                           usb_rcvbulkpipe(usbcan->udev, GSUSB_ENDPOINT_IN),
> -                         hf, sizeof(struct gs_host_frame),
> +                         hf, dev->parent->hf_size_rx,
>                           gs_usb_receive_bulk_callback, usbcan);
>
>         rc = usb_submit_urb(urb, GFP_ATOMIC);
> @@ -485,7 +494,7 @@ static netdev_tx_t gs_can_start_xmit(struct sk_buff *skb,
>         if (!urb)
>                 goto nomem_urb;
>
> -       hf = usb_alloc_coherent(dev->udev, sizeof(*hf), GFP_ATOMIC,
> +       hf = usb_alloc_coherent(dev->udev, dev->hf_size_tx, GFP_ATOMIC,
>                                 &urb->transfer_dma);
>         if (!hf) {
>                 netdev_err(netdev, "No memory left for USB buffer\n");
> @@ -509,11 +518,11 @@ static netdev_tx_t gs_can_start_xmit(struct sk_buff *skb,
>         hf->can_id = cpu_to_le32(cf->can_id);
>         hf->can_dlc = can_get_cc_dlc(cf, dev->can.ctrlmode);
>
> -       memcpy(hf->data, cf->data, cf->len);
> +       memcpy(hf->classic_can->data, cf->data, cf->len);
>
>         usb_fill_bulk_urb(urb, dev->udev,
>                           usb_sndbulkpipe(dev->udev, GSUSB_ENDPOINT_OUT),
> -                         hf, sizeof(*hf),
> +                         hf, dev->hf_size_tx,
>                           gs_usb_xmit_callback, txc);
>
>         urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> @@ -531,8 +540,8 @@ 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,
> -                                 sizeof(*hf), hf, urb->transfer_dma);
> +               usb_free_coherent(dev->udev, urb->transfer_buffer_length,
> +                                 urb->transfer_buffer, urb->transfer_dma);
>
>                 if (rc == -ENODEV) {
>                         netif_device_detach(netdev);
> @@ -552,7 +561,8 @@ static netdev_tx_t gs_can_start_xmit(struct sk_buff *skb,
>         return NETDEV_TX_OK;
>
>   badidx:
> -       usb_free_coherent(dev->udev, sizeof(*hf), hf, urb->transfer_dma);
> +       usb_free_coherent(dev->udev, urb->transfer_buffer_length,
> +                         urb->transfer_buffer, urb->transfer_dma);
>   nomem_hf:
>         usb_free_urb(urb);
>
> @@ -569,6 +579,7 @@ static int gs_can_open(struct net_device *netdev)
>         struct gs_usb *parent = dev->parent;
>         int rc, i;
>         struct gs_device_mode *dm;
> +       struct gs_host_frame *hf;
>         u32 ctrlmode;
>         u32 flags = 0;
>
> @@ -576,6 +587,8 @@ static int gs_can_open(struct net_device *netdev)
>         if (rc)
>                 return rc;
>
> +       dev->hf_size_tx = struct_size(hf, classic_can, 1);

struct_size(hf, classic_can, 1) is a constant value, isn't it?
Why not make this a macro?
Also, what is the purpose of the FLEX_ARRAY if it contains only one element?

I do not understand the logic. Sorry if I am wrong.

> +
>         if (!parent->active_channels) {
>                 for (i = 0; i < GS_MAX_RX_URBS; i++) {
>                         struct urb *urb;
> @@ -588,7 +601,7 @@ static int gs_can_open(struct net_device *netdev)
>
>                         /* alloc rx buffer */
>                         buf = usb_alloc_coherent(dev->udev,
> -                                                sizeof(struct gs_host_frame),
> +                                                dev->parent->hf_size_rx,
>                                                  GFP_KERNEL,
>                                                  &urb->transfer_dma);
>                         if (!buf) {
> @@ -604,7 +617,7 @@ static int gs_can_open(struct net_device *netdev)
>                                           usb_rcvbulkpipe(dev->udev,
>                                                           GSUSB_ENDPOINT_IN),
>                                           buf,
> -                                         sizeof(struct gs_host_frame),
> +                                         dev->parent->hf_size_rx,
>                                           gs_usb_receive_bulk_callback, parent);
>                         urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
>
> @@ -886,6 +899,7 @@ static int gs_usb_probe(struct usb_interface *intf,
>                         const struct usb_device_id *id)
>  {
>         struct usb_device *udev = interface_to_usbdev(intf);
> +       struct gs_host_frame *hf;
>         struct gs_usb *dev;
>         int rc = -ENOMEM;
>         unsigned int icount, i;
> @@ -947,6 +961,7 @@ static int gs_usb_probe(struct usb_interface *intf,
>         }
>
>         init_usb_anchor(&dev->rx_submitted);
> +       dev->hf_size_rx = struct_size(hf, classic_can, 1);

Same comment as hf_size_tx.

>
>         usb_set_intfdata(intf, dev);
>         dev->udev = udev;
> --
> 2.34.1
>
>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [can-next-rfc 14/21] can: gs_usb: use union and FLEX_ARRAY for data in struct gs_host_frame
  2022-03-09 14:05   ` Vincent Mailhol
@ 2022-03-09 14:16     ` Vincent Mailhol
  2022-03-09 14:20       ` Marc Kleine-Budde
  2022-03-09 14:18     ` Marc Kleine-Budde
  1 sibling, 1 reply; 26+ messages in thread
From: Vincent Mailhol @ 2022-03-09 14:16 UTC (permalink / raw)
  To: Marc Kleine-Budde; +Cc: linux-can, kernel, Peter Fink

On Wed. 9 Mar. 2022 at 23:05, Vincent Mailhol <vincent.mailhol@gmail.com> wrote:
>
> On Wed. 9 Mar 2022 à 22:39, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> > From: Peter Fink <pfink@christ-es.de>
> >
> > Modify struct gs_host_frame to make use of a union and
> > DECLARE_FLEX_ARRAY to be able to store different data (lengths), which
> > will be added in later commits.

I missed that part. You can ignore my previous message.


Yours sincerely,
Vinent Mailhol

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [can-next-rfc 14/21] can: gs_usb: use union and FLEX_ARRAY for data in struct gs_host_frame
  2022-03-09 14:05   ` Vincent Mailhol
  2022-03-09 14:16     ` Vincent Mailhol
@ 2022-03-09 14:18     ` Marc Kleine-Budde
  1 sibling, 0 replies; 26+ messages in thread
From: Marc Kleine-Budde @ 2022-03-09 14:18 UTC (permalink / raw)
  To: Vincent Mailhol; +Cc: linux-can, kernel, Peter Fink

[-- Attachment #1: Type: text/plain, Size: 9238 bytes --]

On 09.03.2022 23:05:57, Vincent Mailhol wrote:
> On Wed. 9 Mar 2022 à 22:39, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> > From: Peter Fink <pfink@christ-es.de>
> >
> > Modify struct gs_host_frame to make use of a union and
> > DECLARE_FLEX_ARRAY to be able to store different data (lengths), which
> > will be added in later commits.
> >
> > Store the gs_host_frame length in TX direction (host -> device) in
> > struct gs_can::hf_size_tx and RX direction (device -> host) in struct
> > gs_usb::hf_size_rx so it must be calculated only once.
> >
> > Signed-off-by: Peter Fink <pfink@christ-es.de>
> > Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> > ---
> >  drivers/net/can/usb/gs_usb.c | 37 +++++++++++++++++++++++++-----------
> >  1 file changed, 26 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
> > index 4bc10264005b..1fe9d9f08c17 100644
> > --- a/drivers/net/can/usb/gs_usb.c
> > +++ b/drivers/net/can/usb/gs_usb.c
> > @@ -146,6 +146,10 @@ struct gs_device_bt_const {
> >
> >  #define GS_CAN_FLAG_OVERFLOW BIT(0)
> >
> > +struct classic_can {
> > +       u8 data[8];
> > +} __packed;
> > +
> >  struct gs_host_frame {
> >         u32 echo_id;
> >         __le32 can_id;
> > @@ -155,7 +159,9 @@ struct gs_host_frame {
> >         u8 flags;
> >         u8 reserved;
> >
> > -       u8 data[8];
> > +       union {
> > +               DECLARE_FLEX_ARRAY(struct classic_can, classic_can);
> > +       };
> >  } __packed;
> >  /* The GS USB devices make use of the same flags and masks as in
> >   * linux/can.h and linux/can/error.h, and no additional mapping is necessary.
> > @@ -187,6 +193,8 @@ struct gs_can {
> >         struct can_bittiming_const bt_const;
> >         unsigned int channel;   /* channel number */
> >
> > +       unsigned int hf_size_tx;
> > +
> >         /* This lock prevents a race condition between xmit and receive. */
> >         spinlock_t tx_ctx_lock;
> >         struct gs_tx_context tx_context[GS_MAX_TX_URBS];
> > @@ -200,6 +208,7 @@ struct gs_usb {
> >         struct gs_can *canch[GS_MAX_INTF];
> >         struct usb_anchor rx_submitted;
> >         struct usb_device *udev;
> > +       unsigned int hf_size_rx;
> >         u8 active_channels;
> >  };
> >
> > @@ -343,7 +352,7 @@ static void gs_usb_receive_bulk_callback(struct urb *urb)
> >                 cf->can_id = le32_to_cpu(hf->can_id);
> >
> >                 can_frame_set_cc_len(cf, hf->can_dlc, dev->can.ctrlmode);
> > -               memcpy(cf->data, hf->data, 8);
> > +               memcpy(cf->data, hf->classic_can->data, 8);
> >
> >                 /* ERROR frames tell us information about the controller */
> >                 if (le32_to_cpu(hf->can_id) & CAN_ERR_FLAG)
> > @@ -398,7 +407,7 @@ static void gs_usb_receive_bulk_callback(struct urb *urb)
> >   resubmit_urb:
> >         usb_fill_bulk_urb(urb, usbcan->udev,
> >                           usb_rcvbulkpipe(usbcan->udev, GSUSB_ENDPOINT_IN),
> > -                         hf, sizeof(struct gs_host_frame),
> > +                         hf, dev->parent->hf_size_rx,
> >                           gs_usb_receive_bulk_callback, usbcan);
> >
> >         rc = usb_submit_urb(urb, GFP_ATOMIC);
> > @@ -485,7 +494,7 @@ static netdev_tx_t gs_can_start_xmit(struct sk_buff *skb,
> >         if (!urb)
> >                 goto nomem_urb;
> >
> > -       hf = usb_alloc_coherent(dev->udev, sizeof(*hf), GFP_ATOMIC,
> > +       hf = usb_alloc_coherent(dev->udev, dev->hf_size_tx, GFP_ATOMIC,
> >                                 &urb->transfer_dma);
> >         if (!hf) {
> >                 netdev_err(netdev, "No memory left for USB buffer\n");
> > @@ -509,11 +518,11 @@ static netdev_tx_t gs_can_start_xmit(struct sk_buff *skb,
> >         hf->can_id = cpu_to_le32(cf->can_id);
> >         hf->can_dlc = can_get_cc_dlc(cf, dev->can.ctrlmode);
> >
> > -       memcpy(hf->data, cf->data, cf->len);
> > +       memcpy(hf->classic_can->data, cf->data, cf->len);
> >
> >         usb_fill_bulk_urb(urb, dev->udev,
> >                           usb_sndbulkpipe(dev->udev, GSUSB_ENDPOINT_OUT),
> > -                         hf, sizeof(*hf),
> > +                         hf, dev->hf_size_tx,
> >                           gs_usb_xmit_callback, txc);
> >
> >         urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> > @@ -531,8 +540,8 @@ 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,
> > -                                 sizeof(*hf), hf, urb->transfer_dma);
> > +               usb_free_coherent(dev->udev, urb->transfer_buffer_length,
> > +                                 urb->transfer_buffer, urb->transfer_dma);
> >
> >                 if (rc == -ENODEV) {
> >                         netif_device_detach(netdev);
> > @@ -552,7 +561,8 @@ static netdev_tx_t gs_can_start_xmit(struct sk_buff *skb,
> >         return NETDEV_TX_OK;
> >
> >   badidx:
> > -       usb_free_coherent(dev->udev, sizeof(*hf), hf, urb->transfer_dma);
> > +       usb_free_coherent(dev->udev, urb->transfer_buffer_length,
> > +                         urb->transfer_buffer, urb->transfer_dma);
> >   nomem_hf:
> >         usb_free_urb(urb);
> >
> > @@ -569,6 +579,7 @@ static int gs_can_open(struct net_device *netdev)
> >         struct gs_usb *parent = dev->parent;
> >         int rc, i;
> >         struct gs_device_mode *dm;
> > +       struct gs_host_frame *hf;
> >         u32 ctrlmode;
> >         u32 flags = 0;
> >
> > @@ -576,6 +587,8 @@ static int gs_can_open(struct net_device *netdev)
> >         if (rc)
> >                 return rc;
> >
> > +       dev->hf_size_tx = struct_size(hf, classic_can, 1);
> 
> struct_size(hf, classic_can, 1) is a constant value, isn't it?

ACK

> Why not make this a macro?

It will not be constant with the addition of CAN-FD and the quirks
needed for some CAN devices.

> Also, what is the purpose of the FLEX_ARRAY if it contains only one element?

More will be added.

> I do not understand the logic. Sorry if I am wrong.

Granted, this patch without context looks a bit strange. Can we update
the patch description to make this easier to understand?

> > Modify struct gs_host_frame to make use of a union and
> > DECLARE_FLEX_ARRAY to be able to store different data (lengths), which
> > will be added in later commits.

> > +
> >         if (!parent->active_channels) {
> >                 for (i = 0; i < GS_MAX_RX_URBS; i++) {
> >                         struct urb *urb;
> > @@ -588,7 +601,7 @@ static int gs_can_open(struct net_device *netdev)
> >
> >                         /* alloc rx buffer */
> >                         buf = usb_alloc_coherent(dev->udev,
> > -                                                sizeof(struct gs_host_frame),
> > +                                                dev->parent->hf_size_rx,
> >                                                  GFP_KERNEL,
> >                                                  &urb->transfer_dma);
> >                         if (!buf) {
> > @@ -604,7 +617,7 @@ static int gs_can_open(struct net_device *netdev)
> >                                           usb_rcvbulkpipe(dev->udev,
> >                                                           GSUSB_ENDPOINT_IN),
> >                                           buf,
> > -                                         sizeof(struct gs_host_frame),
> > +                                         dev->parent->hf_size_rx,
> >                                           gs_usb_receive_bulk_callback, parent);
> >                         urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> >
> > @@ -886,6 +899,7 @@ static int gs_usb_probe(struct usb_interface *intf,
> >                         const struct usb_device_id *id)
> >  {
> >         struct usb_device *udev = interface_to_usbdev(intf);
> > +       struct gs_host_frame *hf;
> >         struct gs_usb *dev;
> >         int rc = -ENOMEM;
> >         unsigned int icount, i;
> > @@ -947,6 +961,7 @@ static int gs_usb_probe(struct usb_interface *intf,
> >         }
> >
> >         init_usb_anchor(&dev->rx_submitted);
> > +       dev->hf_size_rx = struct_size(hf, classic_can, 1);
> 
> Same comment as hf_size_tx.

ACK, in a later patch they will be different. For TX we use the size
corresponding to the selected CAN mode (CAN-2.0 or CAN-FD) of the CAN
interface. For the RX direction we have to use CAN-FD if a at least one
CAN interface of the USB device uses CAN-FD.

The TX size is per CAN interface, the RX size if per USB device (which
can consist of several CAN interfaces).

Many thanks for the deep review, otherwise you wouldn't have found this!

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [can-next-rfc 14/21] can: gs_usb: use union and FLEX_ARRAY for data in struct gs_host_frame
  2022-03-09 14:16     ` Vincent Mailhol
@ 2022-03-09 14:20       ` Marc Kleine-Budde
  0 siblings, 0 replies; 26+ messages in thread
From: Marc Kleine-Budde @ 2022-03-09 14:20 UTC (permalink / raw)
  To: Vincent Mailhol; +Cc: linux-can, kernel, Peter Fink

[-- Attachment #1: Type: text/plain, Size: 809 bytes --]

On 09.03.2022 23:16:14, Vincent Mailhol wrote:
> On Wed. 9 Mar. 2022 at 23:05, Vincent Mailhol <vincent.mailhol@gmail.com> wrote:
> >
> > On Wed. 9 Mar 2022 à 22:39, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> > > From: Peter Fink <pfink@christ-es.de>
> > >
> > > Modify struct gs_host_frame to make use of a union and
> > > DECLARE_FLEX_ARRAY to be able to store different data (lengths), which
> > > will be added in later commits.
> 
> I missed that part. You can ignore my previous message.

np :)

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2022-03-09 14:20 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-09 12:41 [RFC]: can-next gs-usb Marc Kleine-Budde
2022-03-09 12:41 ` [can-next-rfc 01/21] can: gs_usb: use consistent one space indention Marc Kleine-Budde
2022-03-09 12:41 ` [can-next-rfc 02/21] can: gs_usb: fix checkpatch warning Marc Kleine-Budde
2022-03-09 12:41 ` [can-next-rfc 03/21] can: gs_usb: sort include files alphabetically Marc Kleine-Budde
2022-03-09 12:41 ` [can-next-rfc 04/21] can: gs_usb: GS_CAN_FLAG_OVERFLOW: make use of BIT() Marc Kleine-Budde
2022-03-09 12:41 ` [can-next-rfc 05/21] can: gs_usb: rewrap error messages Marc Kleine-Budde
2022-03-09 12:41 ` [can-next-rfc 06/21] can: gs_usb: rewrap usb_control_msg() and usb_fill_bulk_urb() Marc Kleine-Budde
2022-03-09 12:41 ` [can-next-rfc 07/21] can: gs_usb: gs_make_candev(): call SET_NETDEV_DEV() after handling all bt_const->feature Marc Kleine-Budde
2022-03-09 12:41 ` [can-next-rfc 08/21] can: gs_usb: add HW timestamp mode bit Marc Kleine-Budde
2022-03-09 12:41 ` [can-next-rfc 09/21] can: gs_usb: update GS_CAN_FEATURE_IDENTIFY documentation Marc Kleine-Budde
2022-03-09 12:41 ` [can-next-rfc 10/21] can: gs_usb: document the USER_ID feature Marc Kleine-Budde
2022-03-09 12:41 ` [can-next-rfc 11/21] can: gs_usb: document the PAD_PKTS_TO_MAX_PKT_SIZE feature Marc Kleine-Budde
2022-03-09 12:41 ` [can-next-rfc 12/21] can: gs_usb: gs_usb_probe(): introduce udev and make use of it Marc Kleine-Budde
2022-03-09 12:41 ` [can-next-rfc 13/21] can: gs_usb: support up to 3 channels per device Marc Kleine-Budde
2022-03-09 12:41 ` [can-next-rfc 14/21] can: gs_usb: use union and FLEX_ARRAY for data in struct gs_host_frame Marc Kleine-Budde
2022-03-09 14:05   ` Vincent Mailhol
2022-03-09 14:16     ` Vincent Mailhol
2022-03-09 14:20       ` Marc Kleine-Budde
2022-03-09 14:18     ` Marc Kleine-Budde
2022-03-09 12:41 ` [can-next-rfc 15/21] can: gs_usb: add CAN-FD support Marc Kleine-Budde
2022-03-09 12:41 ` [can-next-rfc 16/21] can: gs_usb: add usb quirk for NXP LPC546xx controllers Marc Kleine-Budde
2022-03-09 12:41 ` [can-next-rfc 17/21] can: gs_usb: add quirk for CANtact Pro overlapping GS_USB_BREQ value Marc Kleine-Budde
2022-03-09 12:41 ` [can-next-rfc 18/21] can: gs_usb: activate quirks for CANtact Pro unconditionally Marc Kleine-Budde
2022-03-09 12:41 ` [can-next-rfc 19/21] can: gs_usb: add extended bt_const feature Marc Kleine-Budde
2022-03-09 12:41 ` [can-next-rfc 20/21] can: gs_usb: add VID/PID for CES CANext FD devices Marc Kleine-Budde
2022-03-09 12:41 ` [can-next-rfc 21/21] can: gs_usb: add VID/PID for ABE CAN Debugger devices Marc Kleine-Budde

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.