On Tue, Jan 02, 2024 at 03:59:49PM +0530, Bhavya Kapoor wrote:
> When multiple CAN's are present, then names that are getting assigned
> changes after every boot even after providing alias in the device tree.
> Thus, Add support for implementing CAN aliasing so that names or
> alias for CAN will now be provided from device tree.
>
> Signed-off-by: Bhavya Kapoor <b-kapoor@xxxxxx>
Hi Bhavya,
some minor feedback from my side.
...
> diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
> index 3a3be5cdfc1f..ed483c23ec79 100644
> --- a/drivers/net/can/dev/dev.c
> +++ b/drivers/net/can/dev/dev.c
> @@ -247,12 +247,14 @@ void can_setup(struct net_device *dev)
>
> /* Allocate and setup space for the CAN network device */
> struct net_device *alloc_candev_mqs(int sizeof_priv, unsigned int echo_skb_max,
> - unsigned int txqs, unsigned int rxqs)
> + unsigned int txqs, unsigned int rxqs,
> + struct device *candev)
> {
> struct can_ml_priv *can_ml;
> struct net_device *dev;
> struct can_priv *priv;
> - int size;
> + int size, aliasid;
> + char devname[6] = "can%d";
nit: Please consider arranging local variables in Networking code
in reverse xmas tree order - longest line to shortest.
>
> /* We put the driver's priv, the CAN mid layer priv and the
> * echo skb into the netdevice's priv. The memory layout for
> @@ -273,7 +275,14 @@ struct net_device *alloc_candev_mqs(int sizeof_priv, unsigned int echo_skb_max,
> size = ALIGN(size, sizeof(struct sk_buff *)) +
> echo_skb_max * sizeof(struct sk_buff *);
>
> - dev = alloc_netdev_mqs(size, "can%d", NET_NAME_UNKNOWN, can_setup,
> + if (candev) {
> + aliasid = of_alias_get_id(candev->of_node, "can");
> + if (aliasid >= 0)
> + snprintf(devname, sizeof(devname), "%s%d", "can", aliasid);
The size of devname is 6 bytes (can%d\0).
This means that snprintf() will truncate devname if alias is greater than 99.
Is this a concern?
If so, perhaps devname could be declared to be IFNAMSIZ bytes long?
Flagged by gcc-13 -Wformat-truncation
> + }
> + dev_dbg(candev, "Name of CAN assigned is : %s\n", devname);
> +
> + dev = alloc_netdev_mqs(size, devname, NET_NAME_UNKNOWN, can_setup,
> txqs, rxqs);
> if (!dev)
> return NULL;
...
