Re: [PATCH v6 03/13] can: peak_usb: sort ctrlmode_supported flags

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message says the patch orders the CAN_CTRLMODE_* flags
  in ctrlmode_supported by numerical value.

--

can: peak_usb: sort ctrlmode_supported flags

This commit reorders the CAN_CTRLMODE_* flags in the static
ctrlmode_supported initializers of the peak_usb adapters so that they
follow their numerical values. It is described as a cosmetic change with
no behavior change.

> diff --git a/drivers/net/can/usb/peak_usb/pcan_usb.c b/drivers/net/can/usb/peak_usb/pcan_usb.c
> index 785224e00c4e9..169c00d93463c 100644
> --- a/drivers/net/can/usb/peak_usb/pcan_usb.c
> +++ b/drivers/net/can/usb/peak_usb/pcan_usb.c
> @@ -1016,7 +1016,8 @@ const struct peak_usb_adapter pcan_usb = {
>  	.name = "PCAN-USB",
>  	.device_id = PCAN_USB_PRODUCT_ID,
>  	.ctrl_count = 1,
> -	.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES | CAN_CTRLMODE_LISTENONLY |
> +	.ctrlmode_supported = CAN_CTRLMODE_LISTENONLY |
> +			      CAN_CTRLMODE_3_SAMPLES |
>  			      CAN_CTRLMODE_CC_LEN8_DLC,

[Severity: Low]
This isn't a bug, but the runtime ctrlmode_supported update in
pcan_usb_init() in the same file was not reordered:

drivers/net/can/usb/peak_usb/pcan_usb.c:pcan_usb_init() {
    ...
	if (dev->device_rev >= 41) {
		struct can_priv *priv = netdev_priv(dev->netdev);

		priv->ctrlmode_supported |= CAN_CTRLMODE_ONE_SHOT |
					    CAN_CTRLMODE_LOOPBACK;
    ...
}

CAN_CTRLMODE_LOOPBACK is 0x01 and CAN_CTRLMODE_ONE_SHOT is 0x08, so by
the ordering described in the commit message LOOPBACK would come first.

The commit message says:

  "Order the CAN_CTRLMODE_* flags in ctrlmode_supported according to
  their numerical values."

It doesn't limit this to the static initializers. Should this line be
reordered too, for consistency? No later patch in the series appears to
change it.

As the commit message says, the resulting bitmask is identical either
way.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261022-canfd_check_channel_idx-v6-0-197704231551%40peak-system.fr