All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 0/14] pull-request: can-next 2022-10-31
@ 2022-10-31 15:43 Marc Kleine-Budde
  2022-10-31 15:43 ` [PATCH net-next 01/14] can: peak_usb: rename device_id to a more explicit name Marc Kleine-Budde
                   ` (14 more replies)
  0 siblings, 15 replies; 21+ messages in thread
From: Marc Kleine-Budde @ 2022-10-31 15:43 UTC (permalink / raw)
  To: netdev; +Cc: davem, kuba, linux-can, kernel

Hello Jakub, hello David,

this is a pull request of 14 patches for net-next/master.

The first 7 patches are by Stephane Grosjean and Lukas Magel and
target the peak_usb driver. Support for flashing a user defined device
ID via the ethtool flash interface is added. A read only sysfs
attribute for that value is added to distinguish between devices via
udev.

The next 2 patches are by me an fix minor coding style issues in the
kvaser_usb driver.

The last 5 patches are by Biju Das, target the rcar_canfd driver and
clean up the support for different IP cores.

regards,
Marc

---

The following changes since commit 02a97e02c64fb3245b84835cbbed1c3a3222e2f1:

  Merge tag 'mlx5-updates-2022-10-24' of git://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux (2022-10-28 22:07:48 -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.2-20221031

for you to fetch changes up to 3755b56a9b8ff8f1a6a21a979cc215c220401278:

  Merge patch series "R-Car CAN FD driver enhancements" (2022-10-31 16:15:25 +0100)

----------------------------------------------------------------
linux-can-next-for-6.2-20221031

----------------------------------------------------------------
Biju Das (5):
      can: rcar_canfd: rcar_canfd_probe: Add struct rcar_canfd_hw_info to driver data
      can: rcar_canfd: Add max_channels to struct rcar_canfd_hw_info
      can: rcar_canfd: Add shared_global_irqs to struct rcar_canfd_hw_info
      can: rcar_canfd: Add postdiv to struct rcar_canfd_hw_info
      can: rcar_canfd: Add multi_channel_irqs to struct rcar_canfd_hw_info

Lukas Magel (2):
      can: peak_usb: export PCAN user device ID as sysfs device attribute
      can: peak_usb: align user device id format in log with sysfs attribute

Marc Kleine-Budde (4):
      Merge patch series "can: peak_usb: Introduce configurable user dev id"
      can: kvaser_usb: kvaser_usb_set_bittiming(): fix redundant initialization warning for err
      can: kvaser_usb: kvaser_usb_set_{,data}bittiming(): remove empty lines in variable declaration
      Merge patch series "R-Car CAN FD driver enhancements"

Stephane Grosjean (5):
      can: peak_usb: rename device_id to a more explicit name
      can: peak_usb: add callback to read user value of CANFD devices
      can: peak_usb: allow flashing of the user device id
      can: peak_usb: replace unregister_netdev() with unregister_candev()
      can: peak_usb: add ethtool interface to user defined flashed device number

 Documentation/ABI/testing/sysfs-class-net-peak_usb |  15 +++
 drivers/net/can/rcar/rcar_canfd.c                  |  85 +++++++++------
 drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c   |   4 +-
 drivers/net/can/usb/peak_usb/pcan_usb.c            |  43 ++++++--
 drivers/net/can/usb/peak_usb/pcan_usb_core.c       | 119 +++++++++++++++++++--
 drivers/net/can/usb/peak_usb/pcan_usb_core.h       |  11 +-
 drivers/net/can/usb/peak_usb/pcan_usb_fd.c         |  64 ++++++++++-
 drivers/net/can/usb/peak_usb/pcan_usb_pro.c        |  26 ++++-
 drivers/net/can/usb/peak_usb/pcan_usb_pro.h        |   1 +
 9 files changed, 306 insertions(+), 62 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-net-peak_usb



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

* [PATCH net-next 01/14] can: peak_usb: rename device_id to a more explicit name
  2022-10-31 15:43 [PATCH net-next 0/14] pull-request: can-next 2022-10-31 Marc Kleine-Budde
@ 2022-10-31 15:43 ` Marc Kleine-Budde
  2022-10-31 15:43 ` [PATCH net-next 02/14] can: peak_usb: add callback to read user value of CANFD devices Marc Kleine-Budde
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Marc Kleine-Budde @ 2022-10-31 15:43 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Stephane Grosjean, Lukas Magel,
	Marc Kleine-Budde

From: Stephane Grosjean <s.grosjean@peak-system.com>

The so-called "device id" is in fact a value defined by the user.
Therefore, in order not to confuse it with the device id used by the USB,
the functions and variables for reading this user-defined value are
renamed.

Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com>
Signed-off-by: Lukas Magel <lukas.magel@posteo.net>
Link: https://lore.kernel.org/all/20221030105939.87658-2-lukas.magel@posteo.net
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/peak_usb/pcan_usb.c      | 8 ++++----
 drivers/net/can/usb/peak_usb/pcan_usb_core.c | 6 +++---
 drivers/net/can/usb/peak_usb/pcan_usb_core.h | 4 ++--
 drivers/net/can/usb/peak_usb/pcan_usb_fd.c   | 2 +-
 drivers/net/can/usb/peak_usb/pcan_usb_pro.c  | 8 ++++----
 5 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/net/can/usb/peak_usb/pcan_usb.c b/drivers/net/can/usb/peak_usb/pcan_usb.c
index 687dd542f7f6..11fd033b57f3 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb.c
@@ -381,9 +381,9 @@ static int pcan_usb_get_serial(struct peak_usb_device *dev, u32 *serial_number)
 }
 
 /*
- * read device id from device
+ * read user device id from device
  */
-static int pcan_usb_get_device_id(struct peak_usb_device *dev, u32 *device_id)
+static int pcan_usb_get_user_devid(struct peak_usb_device *dev, u32 *user_devid)
 {
 	u8 args[PCAN_USB_CMD_ARGS_LEN];
 	int err;
@@ -393,7 +393,7 @@ static int pcan_usb_get_device_id(struct peak_usb_device *dev, u32 *device_id)
 		netdev_err(dev->netdev, "getting device id failure: %d\n", err);
 
 	else
-		*device_id = args[0];
+		*user_devid = args[0];
 
 	return err;
 }
@@ -1017,7 +1017,7 @@ const struct peak_usb_adapter pcan_usb = {
 	.dev_init = pcan_usb_init,
 	.dev_set_bus = pcan_usb_write_mode,
 	.dev_set_bittiming = pcan_usb_set_bittiming,
-	.dev_get_device_id = pcan_usb_get_device_id,
+	.dev_get_user_devid = pcan_usb_get_user_devid,
 	.dev_decode_buf = pcan_usb_decode_buf,
 	.dev_encode_msg = pcan_usb_encode_msg,
 	.dev_start = pcan_usb_start,
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
index 225697d70a9a..8e1b3c19f92f 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
@@ -922,11 +922,11 @@ static int peak_usb_create_dev(const struct peak_usb_adapter *peak_usb_adapter,
 	}
 
 	/* get device number early */
-	if (dev->adapter->dev_get_device_id)
-		dev->adapter->dev_get_device_id(dev, &dev->device_number);
+	if (dev->adapter->dev_get_user_devid)
+		dev->adapter->dev_get_user_devid(dev, &dev->user_devid);
 
 	netdev_info(netdev, "attached to %s channel %u (device %u)\n",
-			peak_usb_adapter->name, ctrl_idx, dev->device_number);
+			peak_usb_adapter->name, ctrl_idx, dev->user_devid);
 
 	return 0;
 
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.h b/drivers/net/can/usb/peak_usb/pcan_usb_core.h
index f6bdd8b3f290..6ff32673a256 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_core.h
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.h
@@ -60,7 +60,7 @@ struct peak_usb_adapter {
 	int (*dev_set_data_bittiming)(struct peak_usb_device *dev,
 				      struct can_bittiming *bt);
 	int (*dev_set_bus)(struct peak_usb_device *dev, u8 onoff);
-	int (*dev_get_device_id)(struct peak_usb_device *dev, u32 *device_id);
+	int (*dev_get_user_devid)(struct peak_usb_device *dev, u32 *user_devid);
 	int (*dev_decode_buf)(struct peak_usb_device *dev, struct urb *urb);
 	int (*dev_encode_msg)(struct peak_usb_device *dev, struct sk_buff *skb,
 					u8 *obuf, size_t *size);
@@ -122,7 +122,7 @@ struct peak_usb_device {
 	u8 *cmd_buf;
 	struct usb_anchor rx_submitted;
 
-	u32 device_number;
+	u32 user_devid;
 	u8 device_rev;
 
 	u8 ep_msg_in;
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
index 2ea1500df393..b8053e697261 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
@@ -972,7 +972,7 @@ static int pcan_usb_fd_init(struct peak_usb_device *dev)
 	}
 
 	pdev->usb_if->dev[dev->ctrl_idx] = dev;
-	dev->device_number =
+	dev->user_devid =
 		le32_to_cpu(pdev->usb_if->fw_info.dev_id[dev->ctrl_idx]);
 
 	/* if vendor rsp is of type 2, then it contains EP numbers to
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
index 5d8f6a40bb2c..15410d60cdf7 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
@@ -419,8 +419,8 @@ static int pcan_usb_pro_set_led(struct peak_usb_device *dev, u8 mode,
 	return pcan_usb_pro_send_cmd(dev, &um);
 }
 
-static int pcan_usb_pro_get_device_id(struct peak_usb_device *dev,
-				      u32 *device_id)
+static int pcan_usb_pro_get_user_devid(struct peak_usb_device *dev,
+				       u32 *user_devid)
 {
 	struct pcan_usb_pro_devid *pdn;
 	struct pcan_usb_pro_msg um;
@@ -439,7 +439,7 @@ static int pcan_usb_pro_get_device_id(struct peak_usb_device *dev,
 		return err;
 
 	pdn = (struct pcan_usb_pro_devid *)pc;
-	*device_id = le32_to_cpu(pdn->dev_num);
+	*user_devid = le32_to_cpu(pdn->dev_num);
 
 	return err;
 }
@@ -1076,7 +1076,7 @@ const struct peak_usb_adapter pcan_usb_pro = {
 	.dev_free = pcan_usb_pro_free,
 	.dev_set_bus = pcan_usb_pro_set_bus,
 	.dev_set_bittiming = pcan_usb_pro_set_bittiming,
-	.dev_get_device_id = pcan_usb_pro_get_device_id,
+	.dev_get_user_devid = pcan_usb_pro_get_user_devid,
 	.dev_decode_buf = pcan_usb_pro_decode_buf,
 	.dev_encode_msg = pcan_usb_pro_encode_msg,
 	.dev_start = pcan_usb_pro_start,

base-commit: 02a97e02c64fb3245b84835cbbed1c3a3222e2f1
-- 
2.35.1



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

* [PATCH net-next 02/14] can: peak_usb: add callback to read user value of CANFD devices
  2022-10-31 15:43 [PATCH net-next 0/14] pull-request: can-next 2022-10-31 Marc Kleine-Budde
  2022-10-31 15:43 ` [PATCH net-next 01/14] can: peak_usb: rename device_id to a more explicit name Marc Kleine-Budde
@ 2022-10-31 15:43 ` Marc Kleine-Budde
  2022-10-31 15:43 ` [PATCH net-next 03/14] can: peak_usb: allow flashing of the user device id Marc Kleine-Budde
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Marc Kleine-Budde @ 2022-10-31 15:43 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Stephane Grosjean, Lukas Magel,
	Marc Kleine-Budde

From: Stephane Grosjean <s.grosjean@peak-system.com>

This patch adds the specific function that allows to read a user defined
value from the non volatile memory of the USB CANFD interfaces of
PEAK-System.

Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com>
Signed-off-by: Lukas Magel <lukas.magel@posteo.net>
Link: https://lore.kernel.org/all/20221030105939.87658-3-lukas.magel@posteo.net
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/peak_usb/pcan_usb_core.c |  3 +-
 drivers/net/can/usb/peak_usb/pcan_usb_fd.c   | 33 +++++++++++++++++---
 2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
index 8e1b3c19f92f..ca282b0aa9a5 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
@@ -922,8 +922,7 @@ static int peak_usb_create_dev(const struct peak_usb_adapter *peak_usb_adapter,
 	}
 
 	/* get device number early */
-	if (dev->adapter->dev_get_user_devid)
-		dev->adapter->dev_get_user_devid(dev, &dev->user_devid);
+	dev->adapter->dev_get_user_devid(dev, &dev->user_devid);
 
 	netdev_info(netdev, "attached to %s channel %u (device %u)\n",
 			peak_usb_adapter->name, ctrl_idx, dev->user_devid);
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
index b8053e697261..2db358d0295e 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
@@ -234,6 +234,15 @@ static int pcan_usb_fd_send_cmd(struct peak_usb_device *dev, void *cmd_tail)
 	return err;
 }
 
+static int pcan_usb_fd_read_fwinfo(struct peak_usb_device *dev,
+				   struct pcan_ufd_fw_info *fw_info)
+{
+	return pcan_usb_pro_send_req(dev, PCAN_USBPRO_REQ_INFO,
+				     PCAN_USBPRO_INFO_FW,
+				     fw_info,
+				     sizeof(*fw_info));
+}
+
 /* build the commands list in the given buffer, to enter operational mode */
 static int pcan_usb_fd_build_restart_cmd(struct peak_usb_device *dev, u8 *buf)
 {
@@ -434,6 +443,21 @@ static int pcan_usb_fd_set_bittiming_fast(struct peak_usb_device *dev,
 	return pcan_usb_fd_send_cmd(dev, ++cmd);
 }
 
+/* read user device id from device */
+static int pcan_usb_fd_get_user_devid(struct peak_usb_device *dev,
+				      u32 *user_devid)
+{
+	int err;
+	struct pcan_usb_fd_if *usb_if = pcan_usb_fd_dev_if(dev);
+
+	err = pcan_usb_fd_read_fwinfo(dev, &usb_if->fw_info);
+	if (err)
+		return err;
+
+	*user_devid = le32_to_cpu(usb_if->fw_info.dev_id[dev->ctrl_idx]);
+	return err;
+}
+
 /* handle restart but in asynchronously way
  * (uses PCAN-USB Pro code to complete asynchronous request)
  */
@@ -907,10 +931,7 @@ static int pcan_usb_fd_init(struct peak_usb_device *dev)
 
 		fw_info = &pdev->usb_if->fw_info;
 
-		err = pcan_usb_pro_send_req(dev, PCAN_USBPRO_REQ_INFO,
-					    PCAN_USBPRO_INFO_FW,
-					    fw_info,
-					    sizeof(*fw_info));
+		err = pcan_usb_fd_read_fwinfo(dev, fw_info);
 		if (err) {
 			dev_err(dev->netdev->dev.parent,
 				"unable to read %s firmware info (err %d)\n",
@@ -1148,6 +1169,7 @@ const struct peak_usb_adapter pcan_usb_fd = {
 	.dev_set_bus = pcan_usb_fd_set_bus,
 	.dev_set_bittiming = pcan_usb_fd_set_bittiming_slow,
 	.dev_set_data_bittiming = pcan_usb_fd_set_bittiming_fast,
+	.dev_get_user_devid = pcan_usb_fd_get_user_devid,
 	.dev_decode_buf = pcan_usb_fd_decode_buf,
 	.dev_start = pcan_usb_fd_start,
 	.dev_stop = pcan_usb_fd_stop,
@@ -1222,6 +1244,7 @@ const struct peak_usb_adapter pcan_usb_chip = {
 	.dev_set_bus = pcan_usb_fd_set_bus,
 	.dev_set_bittiming = pcan_usb_fd_set_bittiming_slow,
 	.dev_set_data_bittiming = pcan_usb_fd_set_bittiming_fast,
+	.dev_get_user_devid = pcan_usb_fd_get_user_devid,
 	.dev_decode_buf = pcan_usb_fd_decode_buf,
 	.dev_start = pcan_usb_fd_start,
 	.dev_stop = pcan_usb_fd_stop,
@@ -1296,6 +1319,7 @@ const struct peak_usb_adapter pcan_usb_pro_fd = {
 	.dev_set_bus = pcan_usb_fd_set_bus,
 	.dev_set_bittiming = pcan_usb_fd_set_bittiming_slow,
 	.dev_set_data_bittiming = pcan_usb_fd_set_bittiming_fast,
+	.dev_get_user_devid = pcan_usb_fd_get_user_devid,
 	.dev_decode_buf = pcan_usb_fd_decode_buf,
 	.dev_start = pcan_usb_fd_start,
 	.dev_stop = pcan_usb_fd_stop,
@@ -1370,6 +1394,7 @@ const struct peak_usb_adapter pcan_usb_x6 = {
 	.dev_set_bus = pcan_usb_fd_set_bus,
 	.dev_set_bittiming = pcan_usb_fd_set_bittiming_slow,
 	.dev_set_data_bittiming = pcan_usb_fd_set_bittiming_fast,
+	.dev_get_user_devid = pcan_usb_fd_get_user_devid,
 	.dev_decode_buf = pcan_usb_fd_decode_buf,
 	.dev_start = pcan_usb_fd_start,
 	.dev_stop = pcan_usb_fd_stop,
-- 
2.35.1



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

* [PATCH net-next 03/14] can: peak_usb: allow flashing of the user device id
  2022-10-31 15:43 [PATCH net-next 0/14] pull-request: can-next 2022-10-31 Marc Kleine-Budde
  2022-10-31 15:43 ` [PATCH net-next 01/14] can: peak_usb: rename device_id to a more explicit name Marc Kleine-Budde
  2022-10-31 15:43 ` [PATCH net-next 02/14] can: peak_usb: add callback to read user value of CANFD devices Marc Kleine-Budde
@ 2022-10-31 15:43 ` Marc Kleine-Budde
  2022-10-31 15:43 ` [PATCH net-next 04/14] can: peak_usb: replace unregister_netdev() with unregister_candev() Marc Kleine-Budde
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Marc Kleine-Budde @ 2022-10-31 15:43 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Stephane Grosjean, Lukas Magel,
	Marc Kleine-Budde

From: Stephane Grosjean <s.grosjean@peak-system.com>

This series of patches adds a callback that allows the user to flash a
self-defined device id to all USB - CAN/CANFD interfaces of PEAK-System
managed by this driver, namely:
- PCAN-USB
- PCAN-USB FD
- PCAN-USB Pro FD
- PCAN-USB X6
- PCAN-Chip USB
- PCAN-USB Pro

Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com>
Signed-off-by: Lukas Magel <lukas.magel@posteo.net>
Link: https://lore.kernel.org/all/20221030105939.87658-4-lukas.magel@posteo.net
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/peak_usb/pcan_usb.c      | 26 ++++++++++++++++++--
 drivers/net/can/usb/peak_usb/pcan_usb_core.h |  1 +
 drivers/net/can/usb/peak_usb/pcan_usb_fd.c   | 26 ++++++++++++++++++++
 drivers/net/can/usb/peak_usb/pcan_usb_pro.c  | 15 +++++++++++
 drivers/net/can/usb/peak_usb/pcan_usb_pro.h  |  1 +
 5 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/usb/peak_usb/pcan_usb.c b/drivers/net/can/usb/peak_usb/pcan_usb.c
index 11fd033b57f3..6c01b2c6212b 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb.c
@@ -9,10 +9,12 @@
  * Many thanks to Klaus Hitschler <klaus.hitschler@gmx.de>
  */
 #include <asm/unaligned.h>
+
+#include <linux/ethtool.h>
+#include <linux/limits.h>
+#include <linux/module.h>
 #include <linux/netdevice.h>
 #include <linux/usb.h>
-#include <linux/module.h>
-#include <linux/ethtool.h>
 
 #include <linux/can.h>
 #include <linux/can/dev.h>
@@ -398,6 +400,25 @@ static int pcan_usb_get_user_devid(struct peak_usb_device *dev, u32 *user_devid)
 	return err;
 }
 
+/* set a new user device id in the flash memory of the device */
+static int pcan_usb_set_user_devid(struct peak_usb_device *dev, u32 user_devid)
+{
+	u8 args[PCAN_USB_CMD_ARGS_LEN];
+
+	/* this kind of device supports 8-bit values only */
+	if (user_devid > U8_MAX)
+		return -EINVAL;
+
+	/* during the flash process the device disconnects during ~1.25 s.:
+	 * prohibit access when interface is UP
+	 */
+	if (dev->netdev->flags & IFF_UP)
+		return -EBUSY;
+
+	args[0] = user_devid;
+	return pcan_usb_send_cmd(dev, PCAN_USB_CMD_DEVID, PCAN_USB_SET, args);
+}
+
 /*
  * update current time ref with received timestamp
  */
@@ -1018,6 +1039,7 @@ const struct peak_usb_adapter pcan_usb = {
 	.dev_set_bus = pcan_usb_write_mode,
 	.dev_set_bittiming = pcan_usb_set_bittiming,
 	.dev_get_user_devid = pcan_usb_get_user_devid,
+	.dev_set_user_devid = pcan_usb_set_user_devid,
 	.dev_decode_buf = pcan_usb_decode_buf,
 	.dev_encode_msg = pcan_usb_encode_msg,
 	.dev_start = pcan_usb_start,
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.h b/drivers/net/can/usb/peak_usb/pcan_usb_core.h
index 6ff32673a256..d9c9e333c47a 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_core.h
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.h
@@ -61,6 +61,7 @@ struct peak_usb_adapter {
 				      struct can_bittiming *bt);
 	int (*dev_set_bus)(struct peak_usb_device *dev, u8 onoff);
 	int (*dev_get_user_devid)(struct peak_usb_device *dev, u32 *user_devid);
+	int (*dev_set_user_devid)(struct peak_usb_device *dev, u32 user_devid);
 	int (*dev_decode_buf)(struct peak_usb_device *dev, struct urb *urb);
 	int (*dev_encode_msg)(struct peak_usb_device *dev, struct sk_buff *skb,
 					u8 *obuf, size_t *size);
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
index 2db358d0295e..1f5ad0dc2b1c 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
@@ -147,6 +147,15 @@ struct __packed pcan_ufd_ovr_msg {
 	u8	unused[3];
 };
 
+#define PCAN_UFD_CMD_DEVID_SET		0x81
+
+struct __packed pcan_ufd_device_id {
+	__le16	opcode_channel;
+
+	u16	unused;
+	__le32	device_id;
+};
+
 static inline int pufd_omsg_get_channel(struct pcan_ufd_ovr_msg *om)
 {
 	return om->channel & 0xf;
@@ -458,6 +467,19 @@ static int pcan_usb_fd_get_user_devid(struct peak_usb_device *dev,
 	return err;
 }
 
+/* set a new user device id in the flash memory of the device */
+static int pcan_usb_fd_set_user_devid(struct peak_usb_device *dev, u32 devid)
+{
+	struct pcan_ufd_device_id *cmd = pcan_usb_fd_cmd_buffer(dev);
+
+	cmd->opcode_channel = pucan_cmd_opcode_channel(dev->ctrl_idx,
+						       PCAN_UFD_CMD_DEVID_SET);
+	cmd->device_id = cpu_to_le32(devid);
+
+	/* send the command */
+	return pcan_usb_fd_send_cmd(dev, ++cmd);
+}
+
 /* handle restart but in asynchronously way
  * (uses PCAN-USB Pro code to complete asynchronous request)
  */
@@ -1170,6 +1192,7 @@ const struct peak_usb_adapter pcan_usb_fd = {
 	.dev_set_bittiming = pcan_usb_fd_set_bittiming_slow,
 	.dev_set_data_bittiming = pcan_usb_fd_set_bittiming_fast,
 	.dev_get_user_devid = pcan_usb_fd_get_user_devid,
+	.dev_set_user_devid = pcan_usb_fd_set_user_devid,
 	.dev_decode_buf = pcan_usb_fd_decode_buf,
 	.dev_start = pcan_usb_fd_start,
 	.dev_stop = pcan_usb_fd_stop,
@@ -1245,6 +1268,7 @@ const struct peak_usb_adapter pcan_usb_chip = {
 	.dev_set_bittiming = pcan_usb_fd_set_bittiming_slow,
 	.dev_set_data_bittiming = pcan_usb_fd_set_bittiming_fast,
 	.dev_get_user_devid = pcan_usb_fd_get_user_devid,
+	.dev_set_user_devid = pcan_usb_fd_set_user_devid,
 	.dev_decode_buf = pcan_usb_fd_decode_buf,
 	.dev_start = pcan_usb_fd_start,
 	.dev_stop = pcan_usb_fd_stop,
@@ -1320,6 +1344,7 @@ const struct peak_usb_adapter pcan_usb_pro_fd = {
 	.dev_set_bittiming = pcan_usb_fd_set_bittiming_slow,
 	.dev_set_data_bittiming = pcan_usb_fd_set_bittiming_fast,
 	.dev_get_user_devid = pcan_usb_fd_get_user_devid,
+	.dev_set_user_devid = pcan_usb_fd_set_user_devid,
 	.dev_decode_buf = pcan_usb_fd_decode_buf,
 	.dev_start = pcan_usb_fd_start,
 	.dev_stop = pcan_usb_fd_stop,
@@ -1395,6 +1420,7 @@ const struct peak_usb_adapter pcan_usb_x6 = {
 	.dev_set_bittiming = pcan_usb_fd_set_bittiming_slow,
 	.dev_set_data_bittiming = pcan_usb_fd_set_bittiming_fast,
 	.dev_get_user_devid = pcan_usb_fd_get_user_devid,
+	.dev_set_user_devid = pcan_usb_fd_set_user_devid,
 	.dev_decode_buf = pcan_usb_fd_decode_buf,
 	.dev_start = pcan_usb_fd_start,
 	.dev_stop = pcan_usb_fd_stop,
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
index 15410d60cdf7..d3731c5aa4ed 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
@@ -76,6 +76,7 @@ static u16 pcan_usb_pro_sizeof_rec[256] = {
 	[PCAN_USBPRO_SETFILTR] = sizeof(struct pcan_usb_pro_filter),
 	[PCAN_USBPRO_SETTS] = sizeof(struct pcan_usb_pro_setts),
 	[PCAN_USBPRO_GETDEVID] = sizeof(struct pcan_usb_pro_devid),
+	[PCAN_USBPRO_SETDEVID] = sizeof(struct pcan_usb_pro_devid),
 	[PCAN_USBPRO_SETLED] = sizeof(struct pcan_usb_pro_setled),
 	[PCAN_USBPRO_RXMSG8] = sizeof(struct pcan_usb_pro_rxmsg),
 	[PCAN_USBPRO_RXMSG4] = sizeof(struct pcan_usb_pro_rxmsg) - 4,
@@ -149,6 +150,7 @@ static int pcan_msg_add_rec(struct pcan_usb_pro_msg *pm, int id, ...)
 
 	case PCAN_USBPRO_SETBTR:
 	case PCAN_USBPRO_GETDEVID:
+	case PCAN_USBPRO_SETDEVID:
 		*pc++ = va_arg(ap, int);
 		pc += 2;
 		*(__le32 *)pc = cpu_to_le32(va_arg(ap, u32));
@@ -444,6 +446,18 @@ static int pcan_usb_pro_get_user_devid(struct peak_usb_device *dev,
 	return err;
 }
 
+static int pcan_usb_pro_set_user_devid(struct peak_usb_device *dev,
+				       u32 user_devid)
+{
+	struct pcan_usb_pro_msg um;
+
+	pcan_msg_init_empty(&um, dev->cmd_buf, PCAN_USB_MAX_CMD_LEN);
+	pcan_msg_add_rec(&um, PCAN_USBPRO_SETDEVID, dev->ctrl_idx,
+			 user_devid);
+
+	return pcan_usb_pro_send_cmd(dev, &um);
+}
+
 static int pcan_usb_pro_set_bittiming(struct peak_usb_device *dev,
 				      struct can_bittiming *bt)
 {
@@ -1077,6 +1091,7 @@ const struct peak_usb_adapter pcan_usb_pro = {
 	.dev_set_bus = pcan_usb_pro_set_bus,
 	.dev_set_bittiming = pcan_usb_pro_set_bittiming,
 	.dev_get_user_devid = pcan_usb_pro_get_user_devid,
+	.dev_set_user_devid = pcan_usb_pro_set_user_devid,
 	.dev_decode_buf = pcan_usb_pro_decode_buf,
 	.dev_encode_msg = pcan_usb_pro_encode_msg,
 	.dev_start = pcan_usb_pro_start,
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_pro.h b/drivers/net/can/usb/peak_usb/pcan_usb_pro.h
index a34e0fc021c9..28e740af905d 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_pro.h
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_pro.h
@@ -62,6 +62,7 @@ struct __packed pcan_usb_pro_fwinfo {
 #define PCAN_USBPRO_SETBTR	0x02
 #define PCAN_USBPRO_SETBUSACT	0x04
 #define PCAN_USBPRO_SETSILENT	0x05
+#define PCAN_USBPRO_SETDEVID	0x06
 #define PCAN_USBPRO_SETFILTR	0x0a
 #define PCAN_USBPRO_SETTS	0x10
 #define PCAN_USBPRO_GETDEVID	0x12
-- 
2.35.1



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

* [PATCH net-next 04/14] can: peak_usb: replace unregister_netdev() with unregister_candev()
  2022-10-31 15:43 [PATCH net-next 0/14] pull-request: can-next 2022-10-31 Marc Kleine-Budde
                   ` (2 preceding siblings ...)
  2022-10-31 15:43 ` [PATCH net-next 03/14] can: peak_usb: allow flashing of the user device id Marc Kleine-Budde
@ 2022-10-31 15:43 ` Marc Kleine-Budde
  2022-10-31 15:43 ` [PATCH net-next 05/14] can: peak_usb: add ethtool interface to user defined flashed device number Marc Kleine-Budde
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Marc Kleine-Budde @ 2022-10-31 15:43 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Stephane Grosjean, Lukas Magel,
	Marc Kleine-Budde

From: Stephane Grosjean <s.grosjean@peak-system.com>

This patch changes call to unregister_netdev() with unregister_candev().

Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com>
Signed-off-by: Lukas Magel <lukas.magel@posteo.net>
Link: https://lore.kernel.org/all/20221030105939.87658-5-lukas.magel@posteo.net
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/peak_usb/pcan_usb_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
index ca282b0aa9a5..b010b546f6d1 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
@@ -963,7 +963,7 @@ static void peak_usb_disconnect(struct usb_interface *intf)
 		dev->state &= ~PCAN_USB_STATE_CONNECTED;
 		strscpy(name, netdev->name, IFNAMSIZ);
 
-		unregister_netdev(netdev);
+		unregister_candev(netdev);
 
 		kfree(dev->cmd_buf);
 		dev->next_siblings = NULL;
-- 
2.35.1



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

* [PATCH net-next 05/14] can: peak_usb: add ethtool interface to user defined flashed device number
  2022-10-31 15:43 [PATCH net-next 0/14] pull-request: can-next 2022-10-31 Marc Kleine-Budde
                   ` (3 preceding siblings ...)
  2022-10-31 15:43 ` [PATCH net-next 04/14] can: peak_usb: replace unregister_netdev() with unregister_candev() Marc Kleine-Budde
@ 2022-10-31 15:43 ` Marc Kleine-Budde
  2022-10-31 15:43 ` [PATCH net-next 06/14] can: peak_usb: export PCAN user device ID as sysfs device attribute Marc Kleine-Budde
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Marc Kleine-Budde @ 2022-10-31 15:43 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Stephane Grosjean, Lukas Magel,
	Marc Kleine-Budde

From: Stephane Grosjean <s.grosjean@peak-system.com>

This patch introduces 3 new functions implementing support for eeprom
access of USB - CAN network interfaces managed by the driver, through the
ethtool interface. All of them (except the PCAN-USB interface) interpret
the 4 data bytes as a 32-bit value to be read/write in the non-volatile
memory of the device. The PCAN-USB only manages a single byte value.

Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com>
Signed-off-by: Lukas Magel <lukas.magel@posteo.net>
Link: https://lore.kernel.org/all/20221030105939.87658-6-lukas.magel@posteo.net
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/peak_usb/pcan_usb.c      |  9 +++
 drivers/net/can/usb/peak_usb/pcan_usb_core.c | 80 ++++++++++++++++++++
 drivers/net/can/usb/peak_usb/pcan_usb_core.h |  6 ++
 drivers/net/can/usb/peak_usb/pcan_usb_fd.c   |  3 +
 drivers/net/can/usb/peak_usb/pcan_usb_pro.c  |  3 +
 5 files changed, 101 insertions(+)

diff --git a/drivers/net/can/usb/peak_usb/pcan_usb.c b/drivers/net/can/usb/peak_usb/pcan_usb.c
index 6c01b2c6212b..d205da827016 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb.c
@@ -984,9 +984,18 @@ static int pcan_usb_set_phys_id(struct net_device *netdev,
 	return err;
 }
 
+/* This device only handles 8-bit user device id. */
+static int pcan_usb_get_eeprom_len(struct net_device *netdev)
+{
+	return sizeof(u8);
+}
+
 static const struct ethtool_ops pcan_usb_ethtool_ops = {
 	.set_phys_id = pcan_usb_set_phys_id,
 	.get_ts_info = pcan_get_ts_info,
+	.get_eeprom_len	= pcan_usb_get_eeprom_len,
+	.get_eeprom = peak_usb_get_eeprom,
+	.set_eeprom = peak_usb_set_eeprom,
 };
 
 /*
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
index b010b546f6d1..d8af448058fa 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
@@ -808,6 +808,86 @@ static const struct net_device_ops peak_usb_netdev_ops = {
 	.ndo_change_mtu = can_change_mtu,
 };
 
+/* CAN-USB devices generally handle 32-bit user device id.
+ * In case one doesn't, then it have to overload this function.
+ */
+int peak_usb_get_eeprom_len(struct net_device *netdev)
+{
+	return sizeof(u32);
+}
+
+/* Every CAN-USB device exports the dev_get_user_devid() operation. It is used
+ * here to fill the data buffer with the user defined device number.
+ */
+int peak_usb_get_eeprom(struct net_device *netdev,
+			struct ethtool_eeprom *eeprom, u8 *data)
+{
+	struct peak_usb_device *dev = netdev_priv(netdev);
+	u32 devid;
+	__le32 devid_le;
+	int err;
+
+	err = dev->adapter->dev_get_user_devid(dev, &devid);
+	if (err)
+		return err;
+
+	/* ethtool operates on individual bytes. The byte order of the user
+	 * device id in memory depends on the kernel architecture. We
+	 * convert the user device id back to the native byte order of the PEAK
+	 * device itself to ensure that the order is consistent for all
+	 * host architectures.
+	 */
+	devid_le = cpu_to_le32(devid);
+	memcpy(data, (u8 *)&devid_le + eeprom->offset, eeprom->len);
+
+	/* update cached value */
+	dev->user_devid = devid;
+	return err;
+}
+
+/* Every CAN-USB device exports the dev_get_user_devid()/dev_set_user_devid()
+ * operations. They are used here to set the new user defined device number.
+ */
+int peak_usb_set_eeprom(struct net_device *netdev,
+			struct ethtool_eeprom *eeprom, u8 *data)
+{
+	struct peak_usb_device *dev = netdev_priv(netdev);
+	u32 devid;
+	__le32 devid_le;
+	int err;
+
+	/* first, read the current user defined device value number */
+	err = dev->adapter->dev_get_user_devid(dev, &devid);
+	if (err) {
+		netdev_err(netdev, "Failed to init device id (err %d)\n", err);
+		return err;
+	}
+
+	/* do update the value with user given bytes.
+	 * ethtool operates on individual bytes. The byte order of the user
+	 * device id in memory depends on the kernel architecture. We
+	 * convert the user device id back to the native byte order of the PEAK
+	 * device itself to ensure that the order is consistent for all
+	 * host architectures.
+	 */
+	devid_le = cpu_to_le32(devid);
+	memcpy((u8 *)&devid_le + eeprom->offset, data, eeprom->len);
+	devid = le32_to_cpu(devid_le);
+
+	/* flash the new value now */
+	err = dev->adapter->dev_set_user_devid(dev, devid);
+	if (err) {
+		netdev_err(netdev, "Failed to write new device id (err %d)\n",
+			   err);
+		return err;
+	}
+
+	/* update cached value with the new one */
+	dev->user_devid = devid;
+
+	return 0;
+}
+
 int pcan_get_ts_info(struct net_device *dev, struct ethtool_ts_info *info)
 {
 	info->so_timestamping =
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.h b/drivers/net/can/usb/peak_usb/pcan_usb_core.h
index d9c9e333c47a..a4a43edc0456 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_core.h
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.h
@@ -148,4 +148,10 @@ void peak_usb_async_complete(struct urb *urb);
 void peak_usb_restart_complete(struct peak_usb_device *dev);
 int pcan_get_ts_info(struct net_device *dev, struct ethtool_ts_info *info);
 
+/* common 32-bit devid ethtool management */
+int peak_usb_get_eeprom_len(struct net_device *netdev);
+int peak_usb_get_eeprom(struct net_device *netdev,
+			struct ethtool_eeprom *eeprom, u8 *data);
+int peak_usb_set_eeprom(struct net_device *netdev,
+			struct ethtool_eeprom *eeprom, u8 *data);
 #endif
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
index 1f5ad0dc2b1c..5188915c4299 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
@@ -1124,6 +1124,9 @@ static int pcan_usb_fd_set_phys_id(struct net_device *netdev,
 static const struct ethtool_ops pcan_usb_fd_ethtool_ops = {
 	.set_phys_id = pcan_usb_fd_set_phys_id,
 	.get_ts_info = pcan_get_ts_info,
+	.get_eeprom_len	= peak_usb_get_eeprom_len,
+	.get_eeprom = peak_usb_get_eeprom,
+	.set_eeprom = peak_usb_set_eeprom,
 };
 
 /* describes the PCAN-USB FD adapter */
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
index d3731c5aa4ed..d45e6562a4bb 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
@@ -1037,6 +1037,9 @@ static int pcan_usb_pro_set_phys_id(struct net_device *netdev,
 static const struct ethtool_ops pcan_usb_pro_ethtool_ops = {
 	.set_phys_id = pcan_usb_pro_set_phys_id,
 	.get_ts_info = pcan_get_ts_info,
+	.get_eeprom_len	= peak_usb_get_eeprom_len,
+	.get_eeprom = peak_usb_get_eeprom,
+	.set_eeprom = peak_usb_set_eeprom,
 };
 
 /*
-- 
2.35.1



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

* [PATCH net-next 06/14] can: peak_usb: export PCAN user device ID as sysfs device attribute
  2022-10-31 15:43 [PATCH net-next 0/14] pull-request: can-next 2022-10-31 Marc Kleine-Budde
                   ` (4 preceding siblings ...)
  2022-10-31 15:43 ` [PATCH net-next 05/14] can: peak_usb: add ethtool interface to user defined flashed device number Marc Kleine-Budde
@ 2022-10-31 15:43 ` Marc Kleine-Budde
  2022-10-31 15:43 ` [PATCH net-next 07/14] can: peak_usb: align user device id format in log with sysfs attribute Marc Kleine-Budde
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Marc Kleine-Budde @ 2022-10-31 15:43 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Lukas Magel, Stephane Grosjean,
	Marc Kleine-Budde

From: Lukas Magel <lukas.magel@posteo.net>

This patch exports the user device ID as a sysfs attribute. This allows
users to easily read the ID and to write udev rules that can match against
the ID.

Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com>
Signed-off-by: Lukas Magel <lukas.magel@posteo.net>
Link: https://lore.kernel.org/all/20221030105939.87658-7-lukas.magel@posteo.net
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 .../ABI/testing/sysfs-class-net-peak_usb      | 15 ++++++++++
 drivers/net/can/usb/peak_usb/pcan_usb_core.c  | 30 +++++++++++++++++--
 2 files changed, 42 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-net-peak_usb

diff --git a/Documentation/ABI/testing/sysfs-class-net-peak_usb b/Documentation/ABI/testing/sysfs-class-net-peak_usb
new file mode 100644
index 000000000000..f7f23f9bdfde
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-net-peak_usb
@@ -0,0 +1,15 @@
+
+What:		/sys/class/net/<iface>/peak_usb/user_devid
+Date:		October 2022
+KernelVersion:	6.1
+Contact:	Stephane Grosjean <s.grosjean@peak-system.com>
+Description:
+		PEAK PCAN-USB devices support a user-configurable device
+		identifier. This attribute provides read-only access to the
+		currently configured value of the device identifier. Depending
+		on the device type, the identifier has a length of 8 or 32 bit.
+		The value read from this attribute is always an 8 digit 32 bit
+		hexadecimal value in big endian format. If the device only
+		supports an 8 bit identifier, the upper 24 bit of the value are
+		set to zero.
+
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
index d8af448058fa..e558746a0252 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
@@ -8,13 +8,15 @@
  *
  * Many thanks to Klaus Hitschler <klaus.hitschler@gmx.de>
  */
+#include <linux/device.h>
+#include <linux/ethtool.h>
 #include <linux/init.h>
-#include <linux/signal.h>
-#include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/netdevice.h>
+#include <linux/signal.h>
+#include <linux/slab.h>
+#include <linux/sysfs.h>
 #include <linux/usb.h>
-#include <linux/ethtool.h>
 
 #include <linux/can.h>
 #include <linux/can/dev.h>
@@ -53,6 +55,25 @@ static const struct usb_device_id peak_usb_table[] = {
 
 MODULE_DEVICE_TABLE(usb, peak_usb_table);
 
+static ssize_t user_devid_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct net_device *netdev = to_net_dev(dev);
+	struct peak_usb_device *peak_dev = netdev_priv(netdev);
+
+	return sysfs_emit(buf, "%08X\n", peak_dev->user_devid);
+}
+static DEVICE_ATTR_RO(user_devid);
+
+static const struct attribute *peak_usb_sysfs_attrs[] = {
+	&dev_attr_user_devid.attr,
+	NULL,
+};
+
+static const struct attribute_group peak_usb_sysfs_group = {
+	.name	= "peak_usb",
+	.attrs	= (struct attribute **)peak_usb_sysfs_attrs,
+};
+
 /*
  * dump memory
  */
@@ -961,6 +982,9 @@ static int peak_usb_create_dev(const struct peak_usb_adapter *peak_usb_adapter,
 	/* add ethtool support */
 	netdev->ethtool_ops = peak_usb_adapter->ethtool_ops;
 
+	/* register peak_usb sysfs files */
+	netdev->sysfs_groups[0] = &peak_usb_sysfs_group;
+
 	init_usb_anchor(&dev->rx_submitted);
 
 	init_usb_anchor(&dev->tx_submitted);
-- 
2.35.1



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

* [PATCH net-next 07/14] can: peak_usb: align user device id format in log with sysfs attribute
  2022-10-31 15:43 [PATCH net-next 0/14] pull-request: can-next 2022-10-31 Marc Kleine-Budde
                   ` (5 preceding siblings ...)
  2022-10-31 15:43 ` [PATCH net-next 06/14] can: peak_usb: export PCAN user device ID as sysfs device attribute Marc Kleine-Budde
@ 2022-10-31 15:43 ` Marc Kleine-Budde
  2022-10-31 15:44 ` [PATCH net-next 08/14] can: kvaser_usb: kvaser_usb_set_bittiming(): fix redundant initialization warning for err Marc Kleine-Budde
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Marc Kleine-Budde @ 2022-10-31 15:43 UTC (permalink / raw)
  To: netdev; +Cc: davem, kuba, linux-can, kernel, Lukas Magel, Marc Kleine-Budde

From: Lukas Magel <lukas.magel@posteo.net>

Previously, the user device id was printed to the kernel log in decimal
upon connecting a new PEAK device. This behavior is inconsistent with
the hexadecimal format of the user device id sysfs attribute. This patch
updates the log message to output the id in hexadecimal.

Signed-off-by: Lukas Magel <lukas.magel@posteo.net>
Link: https://lore.kernel.org/all/20221030105939.87658-8-lukas.magel@posteo.net
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/peak_usb/pcan_usb_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
index e558746a0252..731bd6553cfc 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
@@ -1028,8 +1028,8 @@ static int peak_usb_create_dev(const struct peak_usb_adapter *peak_usb_adapter,
 	/* get device number early */
 	dev->adapter->dev_get_user_devid(dev, &dev->user_devid);
 
-	netdev_info(netdev, "attached to %s channel %u (device %u)\n",
-			peak_usb_adapter->name, ctrl_idx, dev->user_devid);
+	netdev_info(netdev, "attached to %s channel %u (device %08Xh)\n",
+		    peak_usb_adapter->name, ctrl_idx, dev->user_devid);
 
 	return 0;
 
-- 
2.35.1



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

* [PATCH net-next 08/14] can: kvaser_usb: kvaser_usb_set_bittiming(): fix redundant initialization warning for err
  2022-10-31 15:43 [PATCH net-next 0/14] pull-request: can-next 2022-10-31 Marc Kleine-Budde
                   ` (6 preceding siblings ...)
  2022-10-31 15:43 ` [PATCH net-next 07/14] can: peak_usb: align user device id format in log with sysfs attribute Marc Kleine-Budde
@ 2022-10-31 15:44 ` Marc Kleine-Budde
  2022-10-31 15:44 ` [PATCH net-next 09/14] can: kvaser_usb: kvaser_usb_set_{,data}bittiming(): remove empty lines in variable declaration Marc Kleine-Budde
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Marc Kleine-Budde @ 2022-10-31 15:44 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Marc Kleine-Budde,
	Jimmy Assarsson, Anssi Hannula

The variable err is initialized, but the initialized value is
Overwritten before it is read. Fix the warning by not initializing the
variable err at all.

Fixes: 39d3df6b0ea8 ("can: kvaser_usb: Compare requested bittiming parameters with actual parameters in do_set_{,data}_bittiming")
Cc: Jimmy Assarsson <extja@kvaser.com>
Cc: Anssi Hannula <anssi.hannula@bitwise.fi>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
index 989e75351062..810dfe4053ca 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
@@ -541,7 +541,7 @@ static int kvaser_usb_set_bittiming(struct net_device *netdev)
 	int tseg1 = bt->prop_seg + bt->phase_seg1;
 	int tseg2 = bt->phase_seg2;
 	int sjw = bt->sjw;
-	int err = -EOPNOTSUPP;
+	int err;
 
 	busparams.bitrate = cpu_to_le32(bt->bitrate);
 	busparams.sjw = (u8)sjw;
-- 
2.35.1



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

* [PATCH net-next 09/14] can: kvaser_usb: kvaser_usb_set_{,data}bittiming(): remove empty lines in variable declaration
  2022-10-31 15:43 [PATCH net-next 0/14] pull-request: can-next 2022-10-31 Marc Kleine-Budde
                   ` (7 preceding siblings ...)
  2022-10-31 15:44 ` [PATCH net-next 08/14] can: kvaser_usb: kvaser_usb_set_bittiming(): fix redundant initialization warning for err Marc Kleine-Budde
@ 2022-10-31 15:44 ` Marc Kleine-Budde
  2022-10-31 15:44 ` [PATCH net-next 10/14] can: rcar_canfd: rcar_canfd_probe: Add struct rcar_canfd_hw_info to driver data Marc Kleine-Budde
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Marc Kleine-Budde @ 2022-10-31 15:44 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Marc Kleine-Budde,
	Jimmy Assarsson, Anssi Hannula

Fix coding style by removing empty lines in variable declaration.

Fixes: 39d3df6b0ea8 ("can: kvaser_usb: Compare requested bittiming parameters with actual parameters in do_set_{,data}_bittiming")
Cc: Jimmy Assarsson <extja@kvaser.com>
Cc: Anssi Hannula <anssi.hannula@bitwise.fi>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
index 810dfe4053ca..fe3a591dcc05 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
@@ -536,7 +536,6 @@ static int kvaser_usb_set_bittiming(struct net_device *netdev)
 	struct kvaser_usb *dev = priv->dev;
 	const struct kvaser_usb_dev_ops *ops = dev->driver_info->ops;
 	struct can_bittiming *bt = &priv->can.bittiming;
-
 	struct kvaser_usb_busparams busparams;
 	int tseg1 = bt->prop_seg + bt->phase_seg1;
 	int tseg2 = bt->phase_seg2;
@@ -581,7 +580,6 @@ static int kvaser_usb_set_data_bittiming(struct net_device *netdev)
 	struct kvaser_usb *dev = priv->dev;
 	const struct kvaser_usb_dev_ops *ops = dev->driver_info->ops;
 	struct can_bittiming *dbt = &priv->can.data_bittiming;
-
 	struct kvaser_usb_busparams busparams;
 	int tseg1 = dbt->prop_seg + dbt->phase_seg1;
 	int tseg2 = dbt->phase_seg2;
-- 
2.35.1



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

* [PATCH net-next 10/14] can: rcar_canfd: rcar_canfd_probe: Add struct rcar_canfd_hw_info to driver data
  2022-10-31 15:43 [PATCH net-next 0/14] pull-request: can-next 2022-10-31 Marc Kleine-Budde
                   ` (8 preceding siblings ...)
  2022-10-31 15:44 ` [PATCH net-next 09/14] can: kvaser_usb: kvaser_usb_set_{,data}bittiming(): remove empty lines in variable declaration Marc Kleine-Budde
@ 2022-10-31 15:44 ` Marc Kleine-Budde
  2022-10-31 15:44 ` [PATCH net-next 11/14] can: rcar_canfd: Add max_channels to struct rcar_canfd_hw_info Marc Kleine-Budde
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Marc Kleine-Budde @ 2022-10-31 15:44 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Biju Das, Geert Uytterhoeven,
	Marc Kleine-Budde

From: Biju Das <biju.das.jz@bp.renesas.com>

The CAN FD IP found on RZ/G2L SoC has some HW features different to that
of R-Car. For example, it has multiple resets and multiple IRQs for global
and channel interrupts. Also, it does not have ECC error flag registers
and clk post divider present on R-Car. Similarly, R-Car V3U has 8 channels
whereas other SoCs has only 2 channels.

This patch adds the struct rcar_canfd_hw_info to take care of the
HW feature differences and driver data present on both IPs. It also
replaces the driver data chip type with struct rcar_canfd_hw_info by
moving chip type to it.

Whilst started using driver data instead of chip_id for detecting
R-Car V3U SoCs.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Link: https://lore.kernel.org/all/20221027082158.95895-2-biju.das.jz@bp.renesas.com
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/rcar/rcar_canfd.c | 43 +++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index a0dd6044830b..5660bf0cd755 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -523,6 +523,10 @@ enum rcar_canfd_fcanclk {
 
 struct rcar_canfd_global;
 
+struct rcar_canfd_hw_info {
+	enum rcanfd_chip_id chip_id;
+};
+
 /* Channel priv data */
 struct rcar_canfd_channel {
 	struct can_priv can;			/* Must be the first member */
@@ -548,7 +552,7 @@ struct rcar_canfd_global {
 	bool fdmode;			/* CAN FD or Classical CAN only mode */
 	struct reset_control *rstc1;
 	struct reset_control *rstc2;
-	enum rcanfd_chip_id chip_id;
+	const struct rcar_canfd_hw_info *info;
 	u32 max_channels;
 };
 
@@ -591,10 +595,22 @@ static const struct can_bittiming_const rcar_canfd_bittiming_const = {
 	.brp_inc = 1,
 };
 
+static const struct rcar_canfd_hw_info rcar_gen3_hw_info = {
+	.chip_id = RENESAS_RCAR_GEN3,
+};
+
+static const struct rcar_canfd_hw_info rzg2l_hw_info = {
+	.chip_id = RENESAS_RZG2L,
+};
+
+static const struct rcar_canfd_hw_info r8a779a0_hw_info = {
+	.chip_id = RENESAS_R8A779A0,
+};
+
 /* Helper functions */
 static inline bool is_v3u(struct rcar_canfd_global *gpriv)
 {
-	return gpriv->chip_id == RENESAS_R8A779A0;
+	return gpriv->info == &r8a779a0_hw_info;
 }
 
 static inline u32 reg_v3u(struct rcar_canfd_global *gpriv,
@@ -1701,6 +1717,7 @@ static const struct ethtool_ops rcar_canfd_ethtool_ops = {
 static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch,
 				    u32 fcan_freq)
 {
+	const struct rcar_canfd_hw_info *info = gpriv->info;
 	struct platform_device *pdev = gpriv->pdev;
 	struct rcar_canfd_channel *priv;
 	struct net_device *ndev;
@@ -1723,7 +1740,7 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch,
 	priv->can.clock.freq = fcan_freq;
 	dev_info(&pdev->dev, "can_clk rate is %u\n", priv->can.clock.freq);
 
-	if (gpriv->chip_id == RENESAS_RZG2L) {
+	if (info->chip_id == RENESAS_RZG2L) {
 		char *irq_name;
 		int err_irq;
 		int tx_irq;
@@ -1823,6 +1840,7 @@ static void rcar_canfd_channel_remove(struct rcar_canfd_global *gpriv, u32 ch)
 
 static int rcar_canfd_probe(struct platform_device *pdev)
 {
+	const struct rcar_canfd_hw_info *info;
 	void __iomem *addr;
 	u32 sts, ch, fcan_freq;
 	struct rcar_canfd_global *gpriv;
@@ -1831,13 +1849,12 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 	int err, ch_irq, g_irq;
 	int g_err_irq, g_recc_irq;
 	bool fdmode = true;			/* CAN FD only mode - default */
-	enum rcanfd_chip_id chip_id;
 	int max_channels;
 	char name[9] = "channelX";
 	int i;
 
-	chip_id = (uintptr_t)of_device_get_match_data(&pdev->dev);
-	max_channels = chip_id == RENESAS_R8A779A0 ? 8 : 2;
+	info = of_device_get_match_data(&pdev->dev);
+	max_channels = info->chip_id == RENESAS_R8A779A0 ? 8 : 2;
 
 	if (of_property_read_bool(pdev->dev.of_node, "renesas,no-can-fd"))
 		fdmode = false;			/* Classical CAN only mode */
@@ -1850,7 +1867,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 		of_node_put(of_child);
 	}
 
-	if (chip_id != RENESAS_RZG2L) {
+	if (info->chip_id != RENESAS_RZG2L) {
 		ch_irq = platform_get_irq_byname_optional(pdev, "ch_int");
 		if (ch_irq < 0) {
 			/* For backward compatibility get irq by index */
@@ -1884,7 +1901,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 	gpriv->pdev = pdev;
 	gpriv->channels_mask = channels_mask;
 	gpriv->fdmode = fdmode;
-	gpriv->chip_id = chip_id;
+	gpriv->info = info;
 	gpriv->max_channels = max_channels;
 
 	gpriv->rstc1 = devm_reset_control_get_optional_exclusive(&pdev->dev,
@@ -1922,7 +1939,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 	}
 	fcan_freq = clk_get_rate(gpriv->can_clk);
 
-	if (gpriv->fcan == RCANFD_CANFDCLK && gpriv->chip_id != RENESAS_RZG2L)
+	if (gpriv->fcan == RCANFD_CANFDCLK && info->chip_id != RENESAS_RZG2L)
 		/* CANFD clock is further divided by (1/2) within the IP */
 		fcan_freq /= 2;
 
@@ -1934,7 +1951,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 	gpriv->base = addr;
 
 	/* Request IRQ that's common for both channels */
-	if (gpriv->chip_id != RENESAS_RZG2L) {
+	if (info->chip_id != RENESAS_RZG2L) {
 		err = devm_request_irq(&pdev->dev, ch_irq,
 				       rcar_canfd_channel_interrupt, 0,
 				       "canfd.ch_int", gpriv);
@@ -2087,9 +2104,9 @@ static SIMPLE_DEV_PM_OPS(rcar_canfd_pm_ops, rcar_canfd_suspend,
 			 rcar_canfd_resume);
 
 static const __maybe_unused struct of_device_id rcar_canfd_of_table[] = {
-	{ .compatible = "renesas,rcar-gen3-canfd", .data = (void *)RENESAS_RCAR_GEN3 },
-	{ .compatible = "renesas,rzg2l-canfd", .data = (void *)RENESAS_RZG2L },
-	{ .compatible = "renesas,r8a779a0-canfd", .data = (void *)RENESAS_R8A779A0 },
+	{ .compatible = "renesas,rcar-gen3-canfd", .data = &rcar_gen3_hw_info },
+	{ .compatible = "renesas,rzg2l-canfd", .data = &rzg2l_hw_info },
+	{ .compatible = "renesas,r8a779a0-canfd", .data = &r8a779a0_hw_info },
 	{ }
 };
 
-- 
2.35.1



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

* [PATCH net-next 11/14] can: rcar_canfd: Add max_channels to struct rcar_canfd_hw_info
  2022-10-31 15:43 [PATCH net-next 0/14] pull-request: can-next 2022-10-31 Marc Kleine-Budde
                   ` (9 preceding siblings ...)
  2022-10-31 15:44 ` [PATCH net-next 10/14] can: rcar_canfd: rcar_canfd_probe: Add struct rcar_canfd_hw_info to driver data Marc Kleine-Budde
@ 2022-10-31 15:44 ` Marc Kleine-Budde
  2022-10-31 15:44 ` [PATCH net-next 12/14] can: rcar_canfd: Add shared_global_irqs " Marc Kleine-Budde
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Marc Kleine-Budde @ 2022-10-31 15:44 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Biju Das, Geert Uytterhoeven,
	Marc Kleine-Budde

From: Biju Das <biju.das.jz@bp.renesas.com>

R-Car V3U supports a maximum of 8 channels whereas rest of the SoCs
support 2 channels.

Add max_channels variable to struct rcar_canfd_hw_info to handle this
difference.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Link: https://lore.kernel.org/all/20221027082158.95895-3-biju.das.jz@bp.renesas.com
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/rcar/rcar_canfd.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index 5660bf0cd755..255cb0825f13 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -525,6 +525,7 @@ struct rcar_canfd_global;
 
 struct rcar_canfd_hw_info {
 	enum rcanfd_chip_id chip_id;
+	u8 max_channels;
 };
 
 /* Channel priv data */
@@ -553,7 +554,6 @@ struct rcar_canfd_global {
 	struct reset_control *rstc1;
 	struct reset_control *rstc2;
 	const struct rcar_canfd_hw_info *info;
-	u32 max_channels;
 };
 
 /* CAN FD mode nominal rate constants */
@@ -597,14 +597,17 @@ static const struct can_bittiming_const rcar_canfd_bittiming_const = {
 
 static const struct rcar_canfd_hw_info rcar_gen3_hw_info = {
 	.chip_id = RENESAS_RCAR_GEN3,
+	.max_channels = 2,
 };
 
 static const struct rcar_canfd_hw_info rzg2l_hw_info = {
 	.chip_id = RENESAS_RZG2L,
+	.max_channels = 2,
 };
 
 static const struct rcar_canfd_hw_info r8a779a0_hw_info = {
 	.chip_id = RENESAS_R8A779A0,
+	.max_channels = 8,
 };
 
 /* Helper functions */
@@ -738,7 +741,7 @@ static int rcar_canfd_reset_controller(struct rcar_canfd_global *gpriv)
 	rcar_canfd_set_mode(gpriv);
 
 	/* Transition all Channels to reset mode */
-	for_each_set_bit(ch, &gpriv->channels_mask, gpriv->max_channels) {
+	for_each_set_bit(ch, &gpriv->channels_mask, gpriv->info->max_channels) {
 		rcar_canfd_clear_bit(gpriv->base,
 				     RCANFD_CCTR(ch), RCANFD_CCTR_CSLPR);
 
@@ -779,7 +782,7 @@ static void rcar_canfd_configure_controller(struct rcar_canfd_global *gpriv)
 	rcar_canfd_set_bit(gpriv->base, RCANFD_GCFG, cfg);
 
 	/* Channel configuration settings */
-	for_each_set_bit(ch, &gpriv->channels_mask, gpriv->max_channels) {
+	for_each_set_bit(ch, &gpriv->channels_mask, gpriv->info->max_channels) {
 		rcar_canfd_set_bit(gpriv->base, RCANFD_CCTR(ch),
 				   RCANFD_CCTR_ERRD);
 		rcar_canfd_update_bit(gpriv->base, RCANFD_CCTR(ch),
@@ -1163,7 +1166,7 @@ static irqreturn_t rcar_canfd_global_err_interrupt(int irq, void *dev_id)
 	struct rcar_canfd_global *gpriv = dev_id;
 	u32 ch;
 
-	for_each_set_bit(ch, &gpriv->channels_mask, gpriv->max_channels)
+	for_each_set_bit(ch, &gpriv->channels_mask, gpriv->info->max_channels)
 		rcar_canfd_handle_global_err(gpriv, ch);
 
 	return IRQ_HANDLED;
@@ -1195,7 +1198,7 @@ static irqreturn_t rcar_canfd_global_receive_fifo_interrupt(int irq, void *dev_i
 	struct rcar_canfd_global *gpriv = dev_id;
 	u32 ch;
 
-	for_each_set_bit(ch, &gpriv->channels_mask, gpriv->max_channels)
+	for_each_set_bit(ch, &gpriv->channels_mask, gpriv->info->max_channels)
 		rcar_canfd_handle_global_receive(gpriv, ch);
 
 	return IRQ_HANDLED;
@@ -1209,7 +1212,7 @@ static irqreturn_t rcar_canfd_global_interrupt(int irq, void *dev_id)
 	/* Global error interrupts still indicate a condition specific
 	 * to a channel. RxFIFO interrupt is a global interrupt.
 	 */
-	for_each_set_bit(ch, &gpriv->channels_mask, gpriv->max_channels) {
+	for_each_set_bit(ch, &gpriv->channels_mask, gpriv->info->max_channels) {
 		rcar_canfd_handle_global_err(gpriv, ch);
 		rcar_canfd_handle_global_receive(gpriv, ch);
 	}
@@ -1305,7 +1308,7 @@ static irqreturn_t rcar_canfd_channel_interrupt(int irq, void *dev_id)
 	u32 ch;
 
 	/* Common FIFO is a per channel resource */
-	for_each_set_bit(ch, &gpriv->channels_mask, gpriv->max_channels) {
+	for_each_set_bit(ch, &gpriv->channels_mask, gpriv->info->max_channels) {
 		rcar_canfd_handle_channel_err(gpriv, ch);
 		rcar_canfd_handle_channel_tx(gpriv, ch);
 	}
@@ -1849,17 +1852,15 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 	int err, ch_irq, g_irq;
 	int g_err_irq, g_recc_irq;
 	bool fdmode = true;			/* CAN FD only mode - default */
-	int max_channels;
 	char name[9] = "channelX";
 	int i;
 
 	info = of_device_get_match_data(&pdev->dev);
-	max_channels = info->chip_id == RENESAS_R8A779A0 ? 8 : 2;
 
 	if (of_property_read_bool(pdev->dev.of_node, "renesas,no-can-fd"))
 		fdmode = false;			/* Classical CAN only mode */
 
-	for (i = 0; i < max_channels; ++i) {
+	for (i = 0; i < info->max_channels; ++i) {
 		name[7] = '0' + i;
 		of_child = of_get_child_by_name(pdev->dev.of_node, name);
 		if (of_child && of_device_is_available(of_child))
@@ -1902,7 +1903,6 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 	gpriv->channels_mask = channels_mask;
 	gpriv->fdmode = fdmode;
 	gpriv->info = info;
-	gpriv->max_channels = max_channels;
 
 	gpriv->rstc1 = devm_reset_control_get_optional_exclusive(&pdev->dev,
 								 "rstp_n");
@@ -2017,7 +2017,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 	rcar_canfd_configure_controller(gpriv);
 
 	/* Configure per channel attributes */
-	for_each_set_bit(ch, &gpriv->channels_mask, max_channels) {
+	for_each_set_bit(ch, &gpriv->channels_mask, info->max_channels) {
 		/* Configure Channel's Rx fifo */
 		rcar_canfd_configure_rx(gpriv, ch);
 
@@ -2043,7 +2043,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 		goto fail_mode;
 	}
 
-	for_each_set_bit(ch, &gpriv->channels_mask, max_channels) {
+	for_each_set_bit(ch, &gpriv->channels_mask, info->max_channels) {
 		err = rcar_canfd_channel_probe(gpriv, ch, fcan_freq);
 		if (err)
 			goto fail_channel;
@@ -2055,7 +2055,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 	return 0;
 
 fail_channel:
-	for_each_set_bit(ch, &gpriv->channels_mask, max_channels)
+	for_each_set_bit(ch, &gpriv->channels_mask, info->max_channels)
 		rcar_canfd_channel_remove(gpriv, ch);
 fail_mode:
 	rcar_canfd_disable_global_interrupts(gpriv);
@@ -2076,7 +2076,7 @@ static int rcar_canfd_remove(struct platform_device *pdev)
 	rcar_canfd_reset_controller(gpriv);
 	rcar_canfd_disable_global_interrupts(gpriv);
 
-	for_each_set_bit(ch, &gpriv->channels_mask, gpriv->max_channels) {
+	for_each_set_bit(ch, &gpriv->channels_mask, gpriv->info->max_channels) {
 		rcar_canfd_disable_channel_interrupts(gpriv->ch[ch]);
 		rcar_canfd_channel_remove(gpriv, ch);
 	}
-- 
2.35.1



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

* [PATCH net-next 12/14] can: rcar_canfd: Add shared_global_irqs to struct rcar_canfd_hw_info
  2022-10-31 15:43 [PATCH net-next 0/14] pull-request: can-next 2022-10-31 Marc Kleine-Budde
                   ` (10 preceding siblings ...)
  2022-10-31 15:44 ` [PATCH net-next 11/14] can: rcar_canfd: Add max_channels to struct rcar_canfd_hw_info Marc Kleine-Budde
@ 2022-10-31 15:44 ` Marc Kleine-Budde
  2022-10-31 15:44 ` [PATCH net-next 13/14] can: rcar_canfd: Add postdiv " Marc Kleine-Budde
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 21+ messages in thread
From: Marc Kleine-Budde @ 2022-10-31 15:44 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Biju Das, Geert Uytterhoeven,
	Marc Kleine-Budde

From: Biju Das <biju.das.jz@bp.renesas.com>

RZ/G2L has separate IRQ lines for receive FIFO and global error interrupt
whereas R-Car has shared IRQ line.

Add shared_global_irqs to struct rcar_canfd_hw_info to select the driver to
choose between shared and separate irq registration for global
interrupts.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Link: https://lore.kernel.org/all/20221027082158.95895-4-biju.das.jz@bp.renesas.com
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/rcar/rcar_canfd.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index 255cb0825f13..c8d44dc36515 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -526,6 +526,8 @@ struct rcar_canfd_global;
 struct rcar_canfd_hw_info {
 	enum rcanfd_chip_id chip_id;
 	u8 max_channels;
+	/* hardware features */
+	unsigned shared_global_irqs:1;	/* Has shared global irqs */
 };
 
 /* Channel priv data */
@@ -598,6 +600,7 @@ static const struct can_bittiming_const rcar_canfd_bittiming_const = {
 static const struct rcar_canfd_hw_info rcar_gen3_hw_info = {
 	.chip_id = RENESAS_RCAR_GEN3,
 	.max_channels = 2,
+	.shared_global_irqs = 1,
 };
 
 static const struct rcar_canfd_hw_info rzg2l_hw_info = {
@@ -608,6 +611,7 @@ static const struct rcar_canfd_hw_info rzg2l_hw_info = {
 static const struct rcar_canfd_hw_info r8a779a0_hw_info = {
 	.chip_id = RENESAS_R8A779A0,
 	.max_channels = 8,
+	.shared_global_irqs = 1,
 };
 
 /* Helper functions */
@@ -1868,7 +1872,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 		of_node_put(of_child);
 	}
 
-	if (info->chip_id != RENESAS_RZG2L) {
+	if (info->shared_global_irqs) {
 		ch_irq = platform_get_irq_byname_optional(pdev, "ch_int");
 		if (ch_irq < 0) {
 			/* For backward compatibility get irq by index */
@@ -1951,7 +1955,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 	gpriv->base = addr;
 
 	/* Request IRQ that's common for both channels */
-	if (info->chip_id != RENESAS_RZG2L) {
+	if (info->shared_global_irqs) {
 		err = devm_request_irq(&pdev->dev, ch_irq,
 				       rcar_canfd_channel_interrupt, 0,
 				       "canfd.ch_int", gpriv);
-- 
2.35.1



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

* [PATCH net-next 13/14] can: rcar_canfd: Add postdiv to struct rcar_canfd_hw_info
  2022-10-31 15:43 [PATCH net-next 0/14] pull-request: can-next 2022-10-31 Marc Kleine-Budde
                   ` (11 preceding siblings ...)
  2022-10-31 15:44 ` [PATCH net-next 12/14] can: rcar_canfd: Add shared_global_irqs " Marc Kleine-Budde
@ 2022-10-31 15:44 ` Marc Kleine-Budde
  2022-10-31 15:44 ` [PATCH net-next 14/14] can: rcar_canfd: Add multi_channel_irqs " Marc Kleine-Budde
  2022-11-01  3:27 ` [PATCH net-next 0/14] pull-request: can-next 2022-10-31 Jakub Kicinski
  14 siblings, 0 replies; 21+ messages in thread
From: Marc Kleine-Budde @ 2022-10-31 15:44 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Biju Das, Geert Uytterhoeven,
	Marc Kleine-Budde

From: Biju Das <biju.das.jz@bp.renesas.com>

R-Car has a clock divider for CAN FD clock within the IP, whereas
it is not available on RZ/G2L.

Add postdiv variable to struct rcar_canfd_hw_info to take care of this
difference.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Link: https://lore.kernel.org/all/20221027082158.95895-5-biju.das.jz@bp.renesas.com
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/rcar/rcar_canfd.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index c8d44dc36515..bc5df7a39f91 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -526,6 +526,7 @@ struct rcar_canfd_global;
 struct rcar_canfd_hw_info {
 	enum rcanfd_chip_id chip_id;
 	u8 max_channels;
+	u8 postdiv;
 	/* hardware features */
 	unsigned shared_global_irqs:1;	/* Has shared global irqs */
 };
@@ -600,17 +601,20 @@ static const struct can_bittiming_const rcar_canfd_bittiming_const = {
 static const struct rcar_canfd_hw_info rcar_gen3_hw_info = {
 	.chip_id = RENESAS_RCAR_GEN3,
 	.max_channels = 2,
+	.postdiv = 2,
 	.shared_global_irqs = 1,
 };
 
 static const struct rcar_canfd_hw_info rzg2l_hw_info = {
 	.chip_id = RENESAS_RZG2L,
+	.postdiv = 1,
 	.max_channels = 2,
 };
 
 static const struct rcar_canfd_hw_info r8a779a0_hw_info = {
 	.chip_id = RENESAS_R8A779A0,
 	.max_channels = 8,
+	.postdiv = 2,
 	.shared_global_irqs = 1,
 };
 
@@ -1943,9 +1947,9 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 	}
 	fcan_freq = clk_get_rate(gpriv->can_clk);
 
-	if (gpriv->fcan == RCANFD_CANFDCLK && info->chip_id != RENESAS_RZG2L)
+	if (gpriv->fcan == RCANFD_CANFDCLK)
 		/* CANFD clock is further divided by (1/2) within the IP */
-		fcan_freq /= 2;
+		fcan_freq /= info->postdiv;
 
 	addr = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(addr)) {
-- 
2.35.1



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

* [PATCH net-next 14/14] can: rcar_canfd: Add multi_channel_irqs to struct rcar_canfd_hw_info
  2022-10-31 15:43 [PATCH net-next 0/14] pull-request: can-next 2022-10-31 Marc Kleine-Budde
                   ` (12 preceding siblings ...)
  2022-10-31 15:44 ` [PATCH net-next 13/14] can: rcar_canfd: Add postdiv " Marc Kleine-Budde
@ 2022-10-31 15:44 ` Marc Kleine-Budde
  2022-11-01  3:27 ` [PATCH net-next 0/14] pull-request: can-next 2022-10-31 Jakub Kicinski
  14 siblings, 0 replies; 21+ messages in thread
From: Marc Kleine-Budde @ 2022-10-31 15:44 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, linux-can, kernel, Biju Das, Geert Uytterhoeven,
	Marc Kleine-Budde

From: Biju Das <biju.das.jz@bp.renesas.com>

RZ/G2L has separate IRQ lines for tx and error interrupt for each
channel whereas R-Car has a combined IRQ line for all the channel
specific tx and error interrupts.

Add multi_channel_irqs to struct rcar_canfd_hw_info to select the
driver to choose between combined and separate irq registration for
channel interrupts. This patch also removes enum rcanfd_chip_id and
chip_id from both struct rcar_canfd_hw_info, as it is unused.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Link: https://lore.kernel.org/all/20221027082158.95895-6-biju.das.jz@bp.renesas.com
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/rcar/rcar_canfd.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index bc5df7a39f91..f8eafb132b39 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -41,12 +41,6 @@
 
 #define RCANFD_DRV_NAME			"rcar_canfd"
 
-enum rcanfd_chip_id {
-	RENESAS_RCAR_GEN3 = 0,
-	RENESAS_RZG2L,
-	RENESAS_R8A779A0,
-};
-
 /* Global register bits */
 
 /* RSCFDnCFDGRMCFG */
@@ -524,11 +518,11 @@ enum rcar_canfd_fcanclk {
 struct rcar_canfd_global;
 
 struct rcar_canfd_hw_info {
-	enum rcanfd_chip_id chip_id;
 	u8 max_channels;
 	u8 postdiv;
 	/* hardware features */
 	unsigned shared_global_irqs:1;	/* Has shared global irqs */
+	unsigned multi_channel_irqs:1;	/* Has multiple channel irqs */
 };
 
 /* Channel priv data */
@@ -599,20 +593,18 @@ static const struct can_bittiming_const rcar_canfd_bittiming_const = {
 };
 
 static const struct rcar_canfd_hw_info rcar_gen3_hw_info = {
-	.chip_id = RENESAS_RCAR_GEN3,
 	.max_channels = 2,
 	.postdiv = 2,
 	.shared_global_irqs = 1,
 };
 
 static const struct rcar_canfd_hw_info rzg2l_hw_info = {
-	.chip_id = RENESAS_RZG2L,
-	.postdiv = 1,
 	.max_channels = 2,
+	.postdiv = 1,
+	.multi_channel_irqs = 1,
 };
 
 static const struct rcar_canfd_hw_info r8a779a0_hw_info = {
-	.chip_id = RENESAS_R8A779A0,
 	.max_channels = 8,
 	.postdiv = 2,
 	.shared_global_irqs = 1,
@@ -1751,7 +1743,7 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch,
 	priv->can.clock.freq = fcan_freq;
 	dev_info(&pdev->dev, "can_clk rate is %u\n", priv->can.clock.freq);
 
-	if (info->chip_id == RENESAS_RZG2L) {
+	if (info->multi_channel_irqs) {
 		char *irq_name;
 		int err_irq;
 		int tx_irq;
-- 
2.35.1



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

* Re: [PATCH net-next 0/14] pull-request: can-next 2022-10-31
  2022-10-31 15:43 [PATCH net-next 0/14] pull-request: can-next 2022-10-31 Marc Kleine-Budde
                   ` (13 preceding siblings ...)
  2022-10-31 15:44 ` [PATCH net-next 14/14] can: rcar_canfd: Add multi_channel_irqs " Marc Kleine-Budde
@ 2022-11-01  3:27 ` Jakub Kicinski
  2022-11-01  5:06   ` Greg Kroah-Hartman
  2022-11-01  8:26   ` Marc Kleine-Budde
  14 siblings, 2 replies; 21+ messages in thread
From: Jakub Kicinski @ 2022-11-01  3:27 UTC (permalink / raw)
  To: Marc Kleine-Budde, Greg Kroah-Hartman; +Cc: netdev, davem, linux-can, kernel

On Mon, 31 Oct 2022 16:43:52 +0100 Marc Kleine-Budde wrote:
> The first 7 patches are by Stephane Grosjean and Lukas Magel and
> target the peak_usb driver. Support for flashing a user defined device
> ID via the ethtool flash interface is added. A read only sysfs

nit: ethtool eeprom set != ethtool flash

> attribute for that value is added to distinguish between devices via
> udev.

So the user can write an arbitrary u32 value into flash which then
persistently pops up in sysfs across reboots (as a custom attribute
called "user_devid")?

I don't know.. the whole thing strikes me as odd. Greg do you have any
feelings about such.. solutions?

patches 5 and 6 here:
https://lore.kernel.org/all/20221031154406.259857-1-mkl@pengutronix.de/

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

* Re: [PATCH net-next 0/14] pull-request: can-next 2022-10-31
  2022-11-01  3:27 ` [PATCH net-next 0/14] pull-request: can-next 2022-10-31 Jakub Kicinski
@ 2022-11-01  5:06   ` Greg Kroah-Hartman
  2022-11-04 13:08     ` Marc Kleine-Budde
  2022-11-08 19:07     ` Lukas Magel
  2022-11-01  8:26   ` Marc Kleine-Budde
  1 sibling, 2 replies; 21+ messages in thread
From: Greg Kroah-Hartman @ 2022-11-01  5:06 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: Marc Kleine-Budde, netdev, davem, linux-can, kernel

On Mon, Oct 31, 2022 at 08:27:14PM -0700, Jakub Kicinski wrote:
> On Mon, 31 Oct 2022 16:43:52 +0100 Marc Kleine-Budde wrote:
> > The first 7 patches are by Stephane Grosjean and Lukas Magel and
> > target the peak_usb driver. Support for flashing a user defined device
> > ID via the ethtool flash interface is added. A read only sysfs
> 
> nit: ethtool eeprom set != ethtool flash
> 
> > attribute for that value is added to distinguish between devices via
> > udev.
> 
> So the user can write an arbitrary u32 value into flash which then
> persistently pops up in sysfs across reboots (as a custom attribute
> called "user_devid")?
> 
> I don't know.. the whole thing strikes me as odd. Greg do you have any
> feelings about such.. solutions?
> 
> patches 5 and 6 here:
> https://lore.kernel.org/all/20221031154406.259857-1-mkl@pengutronix.de/

Device-specific attributes should be in the device-specific directory,
not burried in a class directory somewhere that is generic like this one
is.

Why isn't this an attribute of the usb device instead?

And there's no need to reorder the .h file includes in patch 06 while
you are adding a sysfs entry, that should be a separate commit, right?

Also, the line:

+	.attrs	= (struct attribute **)peak_usb_sysfs_attrs,

Is odd, there should never be a need to cast anything like this if you
are doing things properly.

So this still needs work, sorry.

thanks,

greg k-h


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

* Re: [PATCH net-next 0/14] pull-request: can-next 2022-10-31
  2022-11-01  3:27 ` [PATCH net-next 0/14] pull-request: can-next 2022-10-31 Jakub Kicinski
  2022-11-01  5:06   ` Greg Kroah-Hartman
@ 2022-11-01  8:26   ` Marc Kleine-Budde
  1 sibling, 0 replies; 21+ messages in thread
From: Marc Kleine-Budde @ 2022-11-01  8:26 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: Greg Kroah-Hartman, netdev, davem, linux-can, kernel

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

On 31.10.2022 20:27:14, Jakub Kicinski wrote:
> On Mon, 31 Oct 2022 16:43:52 +0100 Marc Kleine-Budde wrote:
> > The first 7 patches are by Stephane Grosjean and Lukas Magel and
> > target the peak_usb driver. Support for flashing a user defined device
> > ID via the ethtool flash interface is added. A read only sysfs
> 
> nit: ethtool eeprom set != ethtool flash

Right, the driver uses the eeprom callbacks of ethtool.

> > attribute for that value is added to distinguish between devices via
> > udev.
> 
> So the user can write an arbitrary u32 value into flash which then
> persistently pops up in sysfs across reboots (as a custom attribute
> called "user_devid")?

ACK - Some devices support a u32, others only a u8.

> I don't know.. the whole thing strikes me as odd. Greg do you have any
> feelings about such.. solutions?
> 
> patches 5 and 6 here:
> https://lore.kernel.org/all/20221031154406.259857-1-mkl@pengutronix.de/

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] 21+ messages in thread

* Re: [PATCH net-next 0/14] pull-request: can-next 2022-10-31
  2022-11-01  5:06   ` Greg Kroah-Hartman
@ 2022-11-04 13:08     ` Marc Kleine-Budde
  2022-11-04 14:53       ` Greg Kroah-Hartman
  2022-11-08 19:07     ` Lukas Magel
  1 sibling, 1 reply; 21+ messages in thread
From: Marc Kleine-Budde @ 2022-11-04 13:08 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jakub Kicinski, netdev, davem, linux-can, kernel

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

On 01.11.2022 06:06:13, Greg Kroah-Hartman wrote:
> On Mon, Oct 31, 2022 at 08:27:14PM -0700, Jakub Kicinski wrote:
> > On Mon, 31 Oct 2022 16:43:52 +0100 Marc Kleine-Budde wrote:
> > > The first 7 patches are by Stephane Grosjean and Lukas Magel and
> > > target the peak_usb driver. Support for flashing a user defined device
> > > ID via the ethtool flash interface is added. A read only sysfs
> > 
> > nit: ethtool eeprom set != ethtool flash
> > 
> > > attribute for that value is added to distinguish between devices via
> > > udev.
> > 
> > So the user can write an arbitrary u32 value into flash which then
> > persistently pops up in sysfs across reboots (as a custom attribute
> > called "user_devid")?
> > 
> > I don't know.. the whole thing strikes me as odd. Greg do you have any
> > feelings about such.. solutions?
> > 
> > patches 5 and 6 here:
> > https://lore.kernel.org/all/20221031154406.259857-1-mkl@pengutronix.de/
> 
> Device-specific attributes should be in the device-specific directory,
> not burried in a class directory somewhere that is generic like this one
> is.
>
> Why isn't this an attribute of the usb device instead?

What about:

| /sys/devices/pci0000:00/0000:00:13.0/usb1/1-1/1-1:1.0/device_id

> And there's no need to reorder the .h file includes in patch 06 while
> you are adding a sysfs entry, that should be a separate commit, right?

ACK

> Also, the line:
> 
> +	.attrs	= (struct attribute **)peak_usb_sysfs_attrs,
> 
> Is odd, there should never be a need to cast anything like this if you
> are doing things properly.

After marking the struct attribute not as const, we can remove the cast:

| --- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c
| +++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
| @@ -64,14 +64,14 @@ static ssize_t user_devid_show(struct device *dev, struct device_attribute *attr
|  }
|  static DEVICE_ATTR_RO(user_devid);
|  
| -static const struct attribute *peak_usb_sysfs_attrs[] = {
| +static struct attribute *peak_usb_sysfs_attrs[] = {
|         &dev_attr_user_devid.attr,
|         NULL,
|  };
|  
|  static const struct attribute_group peak_usb_sysfs_group = {
|         .name   = "peak_usb",
| -       .attrs  = (struct attribute **)peak_usb_sysfs_attrs,
| +       .attrs  = peak_usb_sysfs_attrs,
|  };
|  
|  /*

But this code is obsolete, if we move the sysfs entry into the USB
device.

> So this still needs work, sorry.

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] 21+ messages in thread

* Re: [PATCH net-next 0/14] pull-request: can-next 2022-10-31
  2022-11-04 13:08     ` Marc Kleine-Budde
@ 2022-11-04 14:53       ` Greg Kroah-Hartman
  0 siblings, 0 replies; 21+ messages in thread
From: Greg Kroah-Hartman @ 2022-11-04 14:53 UTC (permalink / raw)
  To: Marc Kleine-Budde; +Cc: Jakub Kicinski, netdev, davem, linux-can, kernel

On Fri, Nov 04, 2022 at 02:08:57PM +0100, Marc Kleine-Budde wrote:
> On 01.11.2022 06:06:13, Greg Kroah-Hartman wrote:
> > Also, the line:
> > 
> > +	.attrs	= (struct attribute **)peak_usb_sysfs_attrs,
> > 
> > Is odd, there should never be a need to cast anything like this if you
> > are doing things properly.
> 
> After marking the struct attribute not as const, we can remove the cast:
> 
> | --- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c
> | +++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
> | @@ -64,14 +64,14 @@ static ssize_t user_devid_show(struct device *dev, struct device_attribute *attr
> |  }
> |  static DEVICE_ATTR_RO(user_devid);
> |  
> | -static const struct attribute *peak_usb_sysfs_attrs[] = {
> | +static struct attribute *peak_usb_sysfs_attrs[] = {

Ah.  Yeah, I would love to make this a const pointer, but people do some
pretty crazy dynamic stuff with attribute groups at times, so that it
will not always work.

I have a long series of patches I'm working on that add more const
markings to the kobject and then the driver core apis, I'll add this
type of thing to the idea list as what to work on next.

thanks,

greg k-h

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

* Re: [PATCH net-next 0/14] pull-request: can-next 2022-10-31
  2022-11-01  5:06   ` Greg Kroah-Hartman
  2022-11-04 13:08     ` Marc Kleine-Budde
@ 2022-11-08 19:07     ` Lukas Magel
  1 sibling, 0 replies; 21+ messages in thread
From: Lukas Magel @ 2022-11-08 19:07 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jakub Kicinski
  Cc: Marc Kleine-Budde, netdev, davem, linux-can, kernel

On 01.11.22 06:06, Greg Kroah-Hartman wrote:
> On Mon, Oct 31, 2022 at 08:27:14PM -0700, Jakub Kicinski wrote:
>> On Mon, 31 Oct 2022 16:43:52 +0100 Marc Kleine-Budde wrote:
>>> The first 7 patches are by Stephane Grosjean and Lukas Magel and
>>> target the peak_usb driver. Support for flashing a user defined device
>>> ID via the ethtool flash interface is added. A read only sysfs
>> nit: ethtool eeprom set != ethtool flash
>>
>>> attribute for that value is added to distinguish between devices via
>>> udev.
>> So the user can write an arbitrary u32 value into flash which then
>> persistently pops up in sysfs across reboots (as a custom attribute
>> called "user_devid")?
>>
>> I don't know.. the whole thing strikes me as odd. Greg do you have any
>> feelings about such.. solutions?
>>
>> patches 5 and 6 here:
>> https://lore.kernel.org/all/20221031154406.259857-1-mkl@pengutronix.de/
I now realize that I didn't do a sufficient job at describing the purpose of the
device ID in the patches (which I am working on improving at the moment). In
contrast to other devices (such as the ones from ETAS), at least the PCAN USB-FD
devices do not export an iSerial attribute at USB level, which makes it hard to
distinguish them if you are using multiple with the same product ID. The user
device ID is a freely configurable identifier (basically an arbitrary u8 / u32
value like you said) that can be set individually for each CAN channel of any
PEAK device and can serve as a replacement for the missing serial number. This
use case is also explicitly stated in the Windows API manual for the PEAK
devices (see page 11 in [1]). The patch series implements write support for the
value via ethtool and exports it readonly as a sysfs attribute for udev matching.
> Device-specific attributes should be in the device-specific directory,
> not burried in a class directory somewhere that is generic like this one
> is.
>
> Why isn't this an attribute of the usb device instead?

Each CAN channel of a PEAK device can have its own device ID, meaning that there
is a potential one-to-n mapping between USB device and device IDs. I can see how
the name might appear confusing in that regard, we chose it to be consistent
with the API description put out by PEAK (also see [1]).

> And there's no need to reorder the .h file includes in patch 06 while
> you are adding a sysfs entry, that should be a separate commit, right?
I have split this into a separate commit.
> Also, the line:
>
> +	.attrs	= (struct attribute **)peak_usb_sysfs_attrs,
>
> Is odd, there should never be a need to cast anything like this if you
> are doing things properly.
I have removed the const modifier from the struct as well as the cast.
> So this still needs work, sorry.
>
> thanks,
>
> greg k-h
>
Please let me know if you require further changes to the patch series or want
the attribute to be renamed.

Regards,


Lukas

[1] See PCAN-Parameter_Documentation.pdf in
https://www.peak-system.com/fileadmin/media/files/pcan-basic.zip

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

end of thread, other threads:[~2022-11-08 19:08 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-31 15:43 [PATCH net-next 0/14] pull-request: can-next 2022-10-31 Marc Kleine-Budde
2022-10-31 15:43 ` [PATCH net-next 01/14] can: peak_usb: rename device_id to a more explicit name Marc Kleine-Budde
2022-10-31 15:43 ` [PATCH net-next 02/14] can: peak_usb: add callback to read user value of CANFD devices Marc Kleine-Budde
2022-10-31 15:43 ` [PATCH net-next 03/14] can: peak_usb: allow flashing of the user device id Marc Kleine-Budde
2022-10-31 15:43 ` [PATCH net-next 04/14] can: peak_usb: replace unregister_netdev() with unregister_candev() Marc Kleine-Budde
2022-10-31 15:43 ` [PATCH net-next 05/14] can: peak_usb: add ethtool interface to user defined flashed device number Marc Kleine-Budde
2022-10-31 15:43 ` [PATCH net-next 06/14] can: peak_usb: export PCAN user device ID as sysfs device attribute Marc Kleine-Budde
2022-10-31 15:43 ` [PATCH net-next 07/14] can: peak_usb: align user device id format in log with sysfs attribute Marc Kleine-Budde
2022-10-31 15:44 ` [PATCH net-next 08/14] can: kvaser_usb: kvaser_usb_set_bittiming(): fix redundant initialization warning for err Marc Kleine-Budde
2022-10-31 15:44 ` [PATCH net-next 09/14] can: kvaser_usb: kvaser_usb_set_{,data}bittiming(): remove empty lines in variable declaration Marc Kleine-Budde
2022-10-31 15:44 ` [PATCH net-next 10/14] can: rcar_canfd: rcar_canfd_probe: Add struct rcar_canfd_hw_info to driver data Marc Kleine-Budde
2022-10-31 15:44 ` [PATCH net-next 11/14] can: rcar_canfd: Add max_channels to struct rcar_canfd_hw_info Marc Kleine-Budde
2022-10-31 15:44 ` [PATCH net-next 12/14] can: rcar_canfd: Add shared_global_irqs " Marc Kleine-Budde
2022-10-31 15:44 ` [PATCH net-next 13/14] can: rcar_canfd: Add postdiv " Marc Kleine-Budde
2022-10-31 15:44 ` [PATCH net-next 14/14] can: rcar_canfd: Add multi_channel_irqs " Marc Kleine-Budde
2022-11-01  3:27 ` [PATCH net-next 0/14] pull-request: can-next 2022-10-31 Jakub Kicinski
2022-11-01  5:06   ` Greg Kroah-Hartman
2022-11-04 13:08     ` Marc Kleine-Budde
2022-11-04 14:53       ` Greg Kroah-Hartman
2022-11-08 19:07     ` Lukas Magel
2022-11-01  8:26   ` 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.