On 28.01.2022 16:01:57, Stephane Grosjean wrote:
> 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>
> ---
>  drivers/net/can/usb/peak_usb/pcan_usb.c      |  9 +++
>  drivers/net/can/usb/peak_usb/pcan_usb_core.c | 69 ++++++++++++++++++++
>  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, 90 insertions(+)
> 
> diff --git a/drivers/net/can/usb/peak_usb/pcan_usb.c b/drivers/net/can/usb/peak_usb/pcan_usb.c
> index b29daaab2e6e..60c9329701a5 100644
> --- a/drivers/net/can/usb/peak_usb/pcan_usb.c
> +++ b/drivers/net/can/usb/peak_usb/pcan_usb.c
> @@ -981,8 +981,17 @@ 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_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 aa8bcdcfa2fb..4e858d592e59 100644
> --- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c
> +++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
> @@ -782,6 +782,75 @@ 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;
> +	int err;
> +
> +	if (!eeprom->len)
> +		return -EINVAL;

There already is a check for len == 0.

> +	err = dev->adapter->dev_get_user_devid(dev, &devid);
> +	if (!err) {

Please return on error.

> +		memcpy(data, (u8 *)&devid + eeprom->offset, eeprom->len);

cast not needed.
> +
> +		/* update cached value */
> +		dev->device_number = 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;
> +	int err;
> +
> +	if (!eeprom->len)
> +		return -EINVAL;

There already is a check for len == 0.

> +
> +	/* 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 */
> +	memcpy((u8 *)&devid + eeprom->offset, data, eeprom->len);

cast not needed.

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 |