Hello Marc,
I was made aware by Stéphane that there has previously been discussion about
registering a sysfs file for reading and writing the device ID [1]. IMHO, I
believe the sysfs file approach would have one important advantage over using
ethtool: udev rule matching.
I often work with setups that feature several CAN interfaces. In such a setup, I
want to assign persistent interface names via udev in case device probing is not
deterministic or the devices are plugged in to different ports. At the moment,
the PEAK device driver and the underlying USB interface do not export any
practically usable identifier for discriminating between different interfaces of
the same type. The device ID solves this problem since it can be configured per
CAN interface to uniquely identify the interface. If the device ID is exported
as sysfs file, it is directly available for matching in udev rules via the
ATTR{...} key. This would solve the CAN interface identification problem and
allow easy read and write access to the device ID without Windows userspace tools.
Regards,
Lukas
[1]
https://lore.kernel.org/linux-can/20211117150132.37056-1-s.grosjean@xxxxxxxxxxxxxxx/
On 15.09.22 11:54, Lukas Magel wrote:
> Peak USB devices support a configurable u8 / u32 device ID. In
> multi-device setups, this device ID can be configured and used to
> identify individual CAN interfaces independent of the order in which
> they are plugged into the system. At the current time, the device ID
> is already queried from the device and stored in the peak_usb_device
> struct.
>
> This patch exports the device ID (called device_number in the struct)
> as a sysfs attribute. The attribute name was chosen to be device_id
> instead of device_number because the latter has been deprecated by Peak
> in their API.
>
> Signed-off-by: Lukas Magel <lukas.magel@xxxxxxxxxx>
> Reviewed-by: Vincent Mailhol <mailhol.vincent@xxxxxxxxxx>
> CC: Stephane Grosjean <s.grosjean@xxxxxxxxxxxxxxx>
> ---
> V2: Update netdev_warn to output the mnemonic of the error value
>
> Resubmission of the patch. Also added Stéphane as maintainer in CC.
>
> drivers/net/can/usb/peak_usb/pcan_usb_core.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> 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 27b0a72fd885..7af3dd0a1b35 100644
> --- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c
> +++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
> @@ -15,6 +15,8 @@
> #include <linux/netdevice.h>
> #include <linux/usb.h>
> #include <linux/ethtool.h>
> +#include <linux/device.h>
> +#include <linux/sysfs.h>
>
> #include <linux/can.h>
> #include <linux/can/dev.h>
> @@ -53,6 +55,15 @@ static const struct usb_device_id peak_usb_table[] = {
>
> MODULE_DEVICE_TABLE(usb, peak_usb_table);
>
> +static ssize_t device_id_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->device_number);
> +}
> +static DEVICE_ATTR_RO(device_id);
> +
> /*
> * dump memory
> */
> @@ -887,6 +898,11 @@ static int peak_usb_create_dev(const struct peak_usb_adapter *peak_usb_adapter,
> netdev_info(netdev, "attached to %s channel %u (device %u)\n",
> peak_usb_adapter->name, ctrl_idx, dev->device_number);
>
> + err = device_create_file(&netdev->dev, &dev_attr_device_id);
> + /* Do not error out since device was configured successfully */
> + if (err)
> + netdev_warn(netdev, "unable to expose device_id via sysfs: %pe\n", ERR_PTR(err));
> +
> return 0;
>
> adap_dev_free:
> @@ -923,6 +939,8 @@ static void peak_usb_disconnect(struct usb_interface *intf)
> dev->state &= ~PCAN_USB_STATE_CONNECTED;
> strlcpy(name, netdev->name, IFNAMSIZ);
>
> + device_remove_file(&netdev->dev, &dev_attr_device_id);
> +
> unregister_netdev(netdev);
>
> kfree(dev->cmd_buf);
