Re: [PATCH v2 7/7] can: gs_usb: add support for reading error counters

[Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>]

On 19.10.2022 23:30:35, Marc Kleine-Budde wrote:
> From: Jeroen Hofstee <jhofstee@xxxxxxxxxxxxxxxxx>
>
> Add support for reading the device state and CAN error counters, using
> the GS_USB_BREQ_GET_STATE control message, if supported by the device,
> indicated by the GS_CAN_FEATURE_GET_STATE feature flag.
>
> Signed-off-by: Jeroen Hofstee <jhofstee@xxxxxxxxxxxxxxxxx>
> Signed-off-by: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
> ---
>  drivers/net/can/usb/gs_usb.c | 39 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>
> diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
> index 3f00e8d79b75..b279cf54cca1 100644
> --- a/drivers/net/can/usb/gs_usb.c
> +++ b/drivers/net/can/usb/gs_usb.c
> @@ -961,6 +961,42 @@ static int gs_can_open(struct net_device *netdev)
>  	return 0;
>  }
>
> +static int gs_usb_get_state(const struct net_device *netdev,
> +			    struct can_berr_counter *bec,
> +			    enum can_state *state)
> +{
> +	struct gs_can *dev = netdev_priv(netdev);
> +	struct gs_device_state ds;
> +	int rc;
> +
> +	rc = usb_control_msg_recv(interface_to_usbdev(dev->iface), 0,
> +				  GS_USB_BREQ_GET_STATE,
> +				  USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_INTERFACE,
> +				  dev->channel, 0,
> +				  &ds, sizeof(ds),
> +				  USB_CTRL_GET_TIMEOUT,
> +				  GFP_KERNEL);
> +	if (rc)
> +		return rc;
> +
> +	if (ds.state >= CAN_STATE_MAX)
> +		return -EOPNOTSUPP;
> +
> +	*state = ds.state;
> +	bec->txerr = ds.txerr;
> +	bec->rxerr = ds.rxerr;

This is not endianness safe:

| drivers/net/can/usb/gs_usb.c:982:15: warning: restricted __le32 degrades to integer
| drivers/net/can/usb/gs_usb.c:985:16: warning: incorrect type in assignment (different base types)
| drivers/net/can/usb/gs_usb.c:985:16:    expected unsigned int enum can_state
| drivers/net/can/usb/gs_usb.c:985:16:    got restricted __le32 [addressable] [usertype] state
| drivers/net/can/usb/gs_usb.c:986:20: warning: incorrect type in assignment (different base types)
| drivers/net/can/usb/gs_usb.c:986:20:    expected unsigned short [usertype] txerr
| drivers/net/can/usb/gs_usb.c:986:20:    got restricted __le32 [addressable] [usertype] txerr
| drivers/net/can/usb/gs_usb.c:987:20: warning: incorrect type in assignment (different base types)
| drivers/net/can/usb/gs_usb.c:987:20:    expected unsigned short [usertype] rxerr
| drivers/net/can/usb/gs_usb.c:987:20:    got restricted __le32 [addressable] [usertype] rxerr

Fixes with:

diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index b279cf54cca1..ccb1a29835a2 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -979,12 +979,12 @@ static int gs_usb_get_state(const struct net_device *netdev,
        if (rc)
                return rc;
 
-       if (ds.state >= CAN_STATE_MAX)
+       if (le32_to_cpu(ds.state) >= CAN_STATE_MAX)
                return -EOPNOTSUPP;
 
-       *state = ds.state;
-       bec->txerr = ds.txerr;
-       bec->rxerr = ds.rxerr;
+       *state = le32_to_cpu(ds.state);
+       bec->txerr = le32_to_cpu(ds.txerr);
+       bec->rxerr = le32_to_cpu(ds.rxerr);
 
        return 0;
 }

regards,
Marc

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

Attachment: signature.asc
Description: PGP signature