[PATCH] can: esd_usb: Improve code readability by means of replacing struct esd_usb_msg with a union

From: Frank Jungclaus
Date: Wed Feb 22 2023 - 11:38:19 EST

Next message: Sean Christopherson: "Re: [PATCH 08/12] KVM: nSVM: Use KVM-governed feature framework to track "vVM{SAVE,LOAD} enabled""
Previous message: Jochen Henneberg: "Re: Issue: stmmac reset by netdev watchdog due to tx queue timeout"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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@xxxxxxxxxxxxxx/
Suggested-by: Vincent MAILHOL <mailhol.vincent@xxxxxxxxxx>
Signed-off-by: Frank Jungclaus <frank.jungclaus@xxxxxx>
---
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

Next message: Sean Christopherson: "Re: [PATCH 08/12] KVM: nSVM: Use KVM-governed feature framework to track "vVM{SAVE,LOAD} enabled""
Previous message: Jochen Henneberg: "Re: Issue: stmmac reset by netdev watchdog due to tx queue timeout"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]