Message-Id: <20230222163754.3711766-1-frank.jungclaus@esd.eu>
Date:   Wed, 22 Feb 2023 17:37:54 +0100
From:   Frank Jungclaus <frank.jungclaus@....eu>
To:     linux-can@...r.kernel.org, Marc Kleine-Budde <mkl@...gutronix.de>,
        Wolfgang Grandegger <wg@...ndegger.com>,
        Vincent Mailhol <mailhol.vincent@...adoo.fr>
Cc:     Stefan Mätje <stefan.maetje@....eu>,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Frank Jungclaus <frank.jungclaus@....eu>
Subject: [PATCH] can: esd_usb: Improve code readability by means of replacing struct esd_usb_msg with a union

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@...adoo.fr>
Signed-off-by: Frank Jungclaus <frank.jungclaus@....eu>
---
 drivers/net/can/usb/esd_usb.c | 166 +++++++++++++++++-----------------
 1 file changed, 82 insertions(+), 84 deletions(-)

diff --git a/drivers/net/can/usb/esd_usb.c b/drivers/net/can/usb/esd_usb.c
index 55b36973952d..e78bb468115a 100644
--- a/drivers/net/can/usb/esd_usb.c
+++ b/drivers/net/can/usb/esd_usb.c
@@ -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 @@ static netdev_tx_t esd_usb_start_xmit(struct sk_buff *skb,
 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,

base-commit: 6ad172748db49deef0da9038d29019aedf991a7e
-- 
2.25.1

Powered by blists - more mailing lists