can: esd_usb: Improve code readability by means of replacing struct esd_usb_msg with...
authorFrank Jungclaus <frank.jungclaus@esd.eu>
Wed, 22 Feb 2023 16:37:54 +0000 (17:37 +0100)
committerMarc Kleine-Budde <mkl@pengutronix.de>
Fri, 24 Mar 2023 18:12:38 +0000 (19:12 +0100)
As suggested by Vincent Mailhol, declare struct esd_usb_msg as a union
instead of a struct. Then replace all msg->msg.something constructs,
that make use of esd_usb_msg, with simpler and prettier looking
msg->something variants.

Link: https://lore.kernel.org/all/CAMZ6RqKRzJwmMShVT9QKwiQ5LJaQupYqkPkKjhRBsP=12QYpfA@mail.gmail.com/
Suggested-by: Vincent MAILHOL <mailhol.vincent@wanadoo.fr>
Signed-off-by: Frank Jungclaus <frank.jungclaus@esd.eu>
Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Link: https://lore.kernel.org/all/20230222163754.3711766-1-frank.jungclaus@esd.eu
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
drivers/net/can/usb/esd_usb.c

index 55b36973952d9d6249152652409fceb7e283f596..e78bb468115a9a77dc4b987c549678274814af30 100644 (file)
@@ -174,17 +174,15 @@ struct set_baudrate_msg {
 };
 
 /* Main message type used between library and application */
-struct __packed esd_usb_msg {
-       union {
-               struct header_msg hdr;
-               struct version_msg version;
-               struct version_reply_msg version_reply;
-               struct rx_msg rx;
-               struct tx_msg tx;
-               struct tx_done_msg txdone;
-               struct set_baudrate_msg setbaud;
-               struct id_filter_msg filter;
-       } msg;
+union __packed esd_usb_msg {
+       struct header_msg hdr;
+       struct version_msg version;
+       struct version_reply_msg version_reply;
+       struct rx_msg rx;
+       struct tx_msg tx;
+       struct tx_done_msg txdone;
+       struct set_baudrate_msg setbaud;
+       struct id_filter_msg filter;
 };
 
 static struct usb_device_id esd_usb_table[] = {
@@ -229,22 +227,22 @@ struct esd_usb_net_priv {
 };
 
 static void esd_usb_rx_event(struct esd_usb_net_priv *priv,
-                            struct esd_usb_msg *msg)
+                            union esd_usb_msg *msg)
 {
        struct net_device_stats *stats = &priv->netdev->stats;
        struct can_frame *cf;
        struct sk_buff *skb;
-       u32 id = le32_to_cpu(msg->msg.rx.id) & ESD_IDMASK;
+       u32 id = le32_to_cpu(msg->rx.id) & ESD_IDMASK;
 
        if (id == ESD_EV_CAN_ERROR_EXT) {
-               u8 state = msg->msg.rx.ev_can_err_ext.status;
-               u8 ecc = msg->msg.rx.ev_can_err_ext.ecc;
-               u8 rxerr = msg->msg.rx.ev_can_err_ext.rec;
-               u8 txerr = msg->msg.rx.ev_can_err_ext.tec;
+               u8 state = msg->rx.ev_can_err_ext.status;
+               u8 ecc = msg->rx.ev_can_err_ext.ecc;
+               u8 rxerr = msg->rx.ev_can_err_ext.rec;
+               u8 txerr = msg->rx.ev_can_err_ext.tec;
 
                netdev_dbg(priv->netdev,
                           "CAN_ERR_EV_EXT: dlc=%#02x state=%02x ecc=%02x rec=%02x tec=%02x\n",
-                          msg->msg.rx.dlc, state, ecc, rxerr, txerr);
+                          msg->rx.dlc, state, ecc, rxerr, txerr);
 
                skb = alloc_can_err_skb(priv->netdev, &cf);
 
@@ -322,7 +320,7 @@ static void esd_usb_rx_event(struct esd_usb_net_priv *priv,
 }
 
 static void esd_usb_rx_can_msg(struct esd_usb_net_priv *priv,
-                              struct esd_usb_msg *msg)
+                              union esd_usb_msg *msg)
 {
        struct net_device_stats *stats = &priv->netdev->stats;
        struct can_frame *cf;
@@ -333,7 +331,7 @@ static void esd_usb_rx_can_msg(struct esd_usb_net_priv *priv,
        if (!netif_device_present(priv->netdev))
                return;
 
-       id = le32_to_cpu(msg->msg.rx.id);
+       id = le32_to_cpu(msg->rx.id);
 
        if (id & ESD_EVENT) {
                esd_usb_rx_event(priv, msg);
@@ -345,17 +343,17 @@ static void esd_usb_rx_can_msg(struct esd_usb_net_priv *priv,
                }
 
                cf->can_id = id & ESD_IDMASK;
-               can_frame_set_cc_len(cf, msg->msg.rx.dlc & ~ESD_RTR,
+               can_frame_set_cc_len(cf, msg->rx.dlc & ~ESD_RTR,
                                     priv->can.ctrlmode);
 
                if (id & ESD_EXTID)
                        cf->can_id |= CAN_EFF_FLAG;
 
-               if (msg->msg.rx.dlc & ESD_RTR) {
+               if (msg->rx.dlc & ESD_RTR) {
                        cf->can_id |= CAN_RTR_FLAG;
                } else {
                        for (i = 0; i < cf->len; i++)
-                               cf->data[i] = msg->msg.rx.data[i];
+                               cf->data[i] = msg->rx.data[i];
 
                        stats->rx_bytes += cf->len;
                }
@@ -366,7 +364,7 @@ static void esd_usb_rx_can_msg(struct esd_usb_net_priv *priv,
 }
 
 static void esd_usb_tx_done_msg(struct esd_usb_net_priv *priv,
-                               struct esd_usb_msg *msg)
+                               union esd_usb_msg *msg)
 {
        struct net_device_stats *stats = &priv->netdev->stats;
        struct net_device *netdev = priv->netdev;
@@ -375,9 +373,9 @@ static void esd_usb_tx_done_msg(struct esd_usb_net_priv *priv,
        if (!netif_device_present(netdev))
                return;
 
-       context = &priv->tx_contexts[msg->msg.txdone.hnd & (MAX_TX_URBS - 1)];
+       context = &priv->tx_contexts[msg->txdone.hnd & (MAX_TX_URBS - 1)];
 
-       if (!msg->msg.txdone.status) {
+       if (!msg->txdone.status) {
                stats->tx_packets++;
                stats->tx_bytes += can_get_echo_skb(netdev, context->echo_index,
                                                    NULL);
@@ -417,32 +415,32 @@ static void esd_usb_read_bulk_callback(struct urb *urb)
        }
 
        while (pos < urb->actual_length) {
-               struct esd_usb_msg *msg;
+               union esd_usb_msg *msg;
 
-               msg = (struct esd_usb_msg *)(urb->transfer_buffer + pos);
+               msg = (union esd_usb_msg *)(urb->transfer_buffer + pos);
 
-               switch (msg->msg.hdr.cmd) {
+               switch (msg->hdr.cmd) {
                case CMD_CAN_RX:
-                       if (msg->msg.rx.net >= dev->net_count) {
+                       if (msg->rx.net >= dev->net_count) {
                                dev_err(dev->udev->dev.parent, "format error\n");
                                break;
                        }
 
-                       esd_usb_rx_can_msg(dev->nets[msg->msg.rx.net], msg);
+                       esd_usb_rx_can_msg(dev->nets[msg->rx.net], msg);
                        break;
 
                case CMD_CAN_TX:
-                       if (msg->msg.txdone.net >= dev->net_count) {
+                       if (msg->txdone.net >= dev->net_count) {
                                dev_err(dev->udev->dev.parent, "format error\n");
                                break;
                        }
 
-                       esd_usb_tx_done_msg(dev->nets[msg->msg.txdone.net],
+                       esd_usb_tx_done_msg(dev->nets[msg->txdone.net],
                                            msg);
                        break;
                }
 
-               pos += msg->msg.hdr.len << 2;
+               pos += msg->hdr.len << 2;
 
                if (pos > urb->actual_length) {
                        dev_err(dev->udev->dev.parent, "format error\n");
@@ -473,7 +471,7 @@ static void esd_usb_write_bulk_callback(struct urb *urb)
        struct esd_tx_urb_context *context = urb->context;
        struct esd_usb_net_priv *priv;
        struct net_device *netdev;
-       size_t size = sizeof(struct esd_usb_msg);
+       size_t size = sizeof(union esd_usb_msg);
 
        WARN_ON(!context);
 
@@ -529,20 +527,20 @@ static ssize_t nets_show(struct device *d,
 }
 static DEVICE_ATTR_RO(nets);
 
-static int esd_usb_send_msg(struct esd_usb *dev, struct esd_usb_msg *msg)
+static int esd_usb_send_msg(struct esd_usb *dev, union esd_usb_msg *msg)
 {
        int actual_length;
 
        return usb_bulk_msg(dev->udev,
                            usb_sndbulkpipe(dev->udev, 2),
                            msg,
-                           msg->msg.hdr.len << 2,
+                           msg->hdr.len << 2,
                            &actual_length,
                            1000);
 }
 
 static int esd_usb_wait_msg(struct esd_usb *dev,
-                           struct esd_usb_msg *msg)
+                           union esd_usb_msg *msg)
 {
        int actual_length;
 
@@ -630,7 +628,7 @@ static int esd_usb_start(struct esd_usb_net_priv *priv)
 {
        struct esd_usb *dev = priv->usb;
        struct net_device *netdev = priv->netdev;
-       struct esd_usb_msg *msg;
+       union esd_usb_msg *msg;
        int err, i;
 
        msg = kmalloc(sizeof(*msg), GFP_KERNEL);
@@ -651,14 +649,14 @@ static int esd_usb_start(struct esd_usb_net_priv *priv)
         * the number of the starting bitmask (0..64) to the filter.option
         * field followed by only some bitmasks.
         */
-       msg->msg.hdr.cmd = CMD_IDADD;
-       msg->msg.hdr.len = 2 + ESD_MAX_ID_SEGMENT;
-       msg->msg.filter.net = priv->index;
-       msg->msg.filter.option = ESD_ID_ENABLE; /* start with segment 0 */
+       msg->hdr.cmd = CMD_IDADD;
+       msg->hdr.len = 2 + ESD_MAX_ID_SEGMENT;
+       msg->filter.net = priv->index;
+       msg->filter.option = ESD_ID_ENABLE; /* start with segment 0 */
        for (i = 0; i < ESD_MAX_ID_SEGMENT; i++)
-               msg->msg.filter.mask[i] = cpu_to_le32(0xffffffff);
+               msg->filter.mask[i] = cpu_to_le32(0xffffffff);
        /* enable 29bit extended IDs */
-       msg->msg.filter.mask[ESD_MAX_ID_SEGMENT] = cpu_to_le32(0x00000001);
+       msg->filter.mask[ESD_MAX_ID_SEGMENT] = cpu_to_le32(0x00000001);
 
        err = esd_usb_send_msg(dev, msg);
        if (err)
@@ -734,12 +732,12 @@ static netdev_tx_t esd_usb_start_xmit(struct sk_buff *skb,
        struct esd_tx_urb_context *context = NULL;
        struct net_device_stats *stats = &netdev->stats;
        struct can_frame *cf = (struct can_frame *)skb->data;
-       struct esd_usb_msg *msg;
+       union esd_usb_msg *msg;
        struct urb *urb;
        u8 *buf;
        int i, err;
        int ret = NETDEV_TX_OK;
-       size_t size = sizeof(struct esd_usb_msg);
+       size_t size = sizeof(union esd_usb_msg);
 
        if (can_dev_dropped_skb(netdev, skb))
                return NETDEV_TX_OK;
@@ -761,24 +759,24 @@ static netdev_tx_t esd_usb_start_xmit(struct sk_buff *skb,
                goto nobufmem;
        }
 
-       msg = (struct esd_usb_msg *)buf;
+       msg = (union esd_usb_msg *)buf;
 
-       msg->msg.hdr.len = 3; /* minimal length */
-       msg->msg.hdr.cmd = CMD_CAN_TX;
-       msg->msg.tx.net = priv->index;
-       msg->msg.tx.dlc = can_get_cc_dlc(cf, priv->can.ctrlmode);
-       msg->msg.tx.id = cpu_to_le32(cf->can_id & CAN_ERR_MASK);
+       msg->hdr.len = 3; /* minimal length */
+       msg->hdr.cmd = CMD_CAN_TX;
+       msg->tx.net = priv->index;
+       msg->tx.dlc = can_get_cc_dlc(cf, priv->can.ctrlmode);
+       msg->tx.id = cpu_to_le32(cf->can_id & CAN_ERR_MASK);
 
        if (cf->can_id & CAN_RTR_FLAG)
-               msg->msg.tx.dlc |= ESD_RTR;
+               msg->tx.dlc |= ESD_RTR;
 
        if (cf->can_id & CAN_EFF_FLAG)
-               msg->msg.tx.id |= cpu_to_le32(ESD_EXTID);
+               msg->tx.id |= cpu_to_le32(ESD_EXTID);
 
        for (i = 0; i < cf->len; i++)
-               msg->msg.tx.data[i] = cf->data[i];
+               msg->tx.data[i] = cf->data[i];
 
-       msg->msg.hdr.len += (cf->len + 3) >> 2;
+       msg->hdr.len += (cf->len + 3) >> 2;
 
        for (i = 0; i < MAX_TX_URBS; i++) {
                if (priv->tx_contexts[i].echo_index == MAX_TX_URBS) {
@@ -798,10 +796,10 @@ static netdev_tx_t esd_usb_start_xmit(struct sk_buff *skb,
        context->echo_index = i;
 
        /* hnd must not be 0 - MSB is stripped in txdone handling */
-       msg->msg.tx.hnd = 0x80000000 | i; /* returned in TX done message */
+       msg->tx.hnd = 0x80000000 | i; /* returned in TX done message */
 
        usb_fill_bulk_urb(urb, dev->udev, usb_sndbulkpipe(dev->udev, 2), buf,
-                         msg->msg.hdr.len << 2,
+                         msg->hdr.len << 2,
                          esd_usb_write_bulk_callback, context);
 
        urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
@@ -855,7 +853,7 @@ nourbmem:
 static int esd_usb_close(struct net_device *netdev)
 {
        struct esd_usb_net_priv *priv = netdev_priv(netdev);
-       struct esd_usb_msg *msg;
+       union esd_usb_msg *msg;
        int i;
 
        msg = kmalloc(sizeof(*msg), GFP_KERNEL);
@@ -863,21 +861,21 @@ static int esd_usb_close(struct net_device *netdev)
                return -ENOMEM;
 
        /* Disable all IDs (see esd_usb_start()) */
-       msg->msg.hdr.cmd = CMD_IDADD;
-       msg->msg.hdr.len = 2 + ESD_MAX_ID_SEGMENT;
-       msg->msg.filter.net = priv->index;
-       msg->msg.filter.option = ESD_ID_ENABLE; /* start with segment 0 */
+       msg->hdr.cmd = CMD_IDADD;
+       msg->hdr.len = 2 + ESD_MAX_ID_SEGMENT;
+       msg->filter.net = priv->index;
+       msg->filter.option = ESD_ID_ENABLE; /* start with segment 0 */
        for (i = 0; i <= ESD_MAX_ID_SEGMENT; i++)
-               msg->msg.filter.mask[i] = 0;
+               msg->filter.mask[i] = 0;
        if (esd_usb_send_msg(priv->usb, msg) < 0)
                netdev_err(netdev, "sending idadd message failed\n");
 
        /* set CAN controller to reset mode */
-       msg->msg.hdr.len = 2;
-       msg->msg.hdr.cmd = CMD_SETBAUD;
-       msg->msg.setbaud.net = priv->index;
-       msg->msg.setbaud.rsvd = 0;
-       msg->msg.setbaud.baud = cpu_to_le32(ESD_USB_NO_BAUDRATE);
+       msg->hdr.len = 2;
+       msg->hdr.cmd = CMD_SETBAUD;
+       msg->setbaud.net = priv->index;
+       msg->setbaud.rsvd = 0;
+       msg->setbaud.baud = cpu_to_le32(ESD_USB_NO_BAUDRATE);
        if (esd_usb_send_msg(priv->usb, msg) < 0)
                netdev_err(netdev, "sending setbaud message failed\n");
 
@@ -919,7 +917,7 @@ static int esd_usb2_set_bittiming(struct net_device *netdev)
 {
        struct esd_usb_net_priv *priv = netdev_priv(netdev);
        struct can_bittiming *bt = &priv->can.bittiming;
-       struct esd_usb_msg *msg;
+       union esd_usb_msg *msg;
        int err;
        u32 canbtr;
        int sjw_shift;
@@ -950,11 +948,11 @@ static int esd_usb2_set_bittiming(struct net_device *netdev)
        if (!msg)
                return -ENOMEM;
 
-       msg->msg.hdr.len = 2;
-       msg->msg.hdr.cmd = CMD_SETBAUD;
-       msg->msg.setbaud.net = priv->index;
-       msg->msg.setbaud.rsvd = 0;
-       msg->msg.setbaud.baud = cpu_to_le32(canbtr);
+       msg->hdr.len = 2;
+       msg->hdr.cmd = CMD_SETBAUD;
+       msg->setbaud.net = priv->index;
+       msg->setbaud.rsvd = 0;
+       msg->setbaud.baud = cpu_to_le32(canbtr);
 
        netdev_info(netdev, "setting BTR=%#x\n", canbtr);
 
@@ -1065,7 +1063,7 @@ static int esd_usb_probe(struct usb_interface *intf,
                         const struct usb_device_id *id)
 {
        struct esd_usb *dev;
-       struct esd_usb_msg *msg;
+       union esd_usb_msg *msg;
        int i, err;
 
        dev = kzalloc(sizeof(*dev), GFP_KERNEL);
@@ -1087,11 +1085,11 @@ static int esd_usb_probe(struct usb_interface *intf,
        }
 
        /* query number of CAN interfaces (nets) */
-       msg->msg.hdr.cmd = CMD_VERSION;
-       msg->msg.hdr.len = 2;
-       msg->msg.version.rsvd = 0;
-       msg->msg.version.flags = 0;
-       msg->msg.version.drv_version = 0;
+       msg->hdr.cmd = CMD_VERSION;
+       msg->hdr.len = 2;
+       msg->version.rsvd = 0;
+       msg->version.flags = 0;
+       msg->version.drv_version = 0;
 
        err = esd_usb_send_msg(dev, msg);
        if (err < 0) {
@@ -1105,8 +1103,8 @@ static int esd_usb_probe(struct usb_interface *intf,
                goto free_msg;
        }
 
-       dev->net_count = (int)msg->msg.version_reply.nets;
-       dev->version = le32_to_cpu(msg->msg.version_reply.version);
+       dev->net_count = (int)msg->version_reply.nets;
+       dev->version = le32_to_cpu(msg->version_reply.version);
 
        if (device_create_file(&intf->dev, &dev_attr_firmware))
                dev_err(&intf->dev,