RE: [PATCH 6/6] can: peak_usb: add ethtool interface to user defined flashed device number

* RE: [PATCH 6/6] can: peak_usb: add ethtool interface to user defined flashed device number
@ 2022-01-31 14:25 Stéphane Grosjean
  2022-01-31 14:31 ` Marc Kleine-Budde
  0 siblings, 1 reply; 10+ messages in thread
From: Stéphane Grosjean @ 2022-01-31 14:25 UTC (permalink / raw)
  To: Marc Kleine-Budde; +Cc: linux-can Mailing List

Hi,

>> +     u32 devid;
>>
>> +             memcpy(data, (u8 *)&devid + eeprom->offset, eeprom->len);
>
> cast not needed.
>

We need to cast the u32 * into a u8 * because eeprom->offset is a count of bytes, isn't it?

Regards,

— Stéphane

            De: Marc Kleine-Budde
Envoyé: Samedi 29 janvier 2022 14:58
À: Stéphane Grosjean
Cc: linux-can Mailing List
Objet: Re: [PATCH 6/6] can: peak_usb: add ethtool interface to user defined flashed device number

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 |

--
PEAK-System Technik GmbH
Sitz der Gesellschaft Darmstadt - HRB 9183
Geschaeftsfuehrung: Alexander Gach / Uwe Wilhelm
Unsere Datenschutzerklaerung mit wichtigen Hinweisen
zur Behandlung personenbezogener Daten finden Sie unter
www.peak-system.com/Datenschutz.483.0.html

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