Commit a57915ae authored by Frank Jungclaus's avatar Frank Jungclaus Committed by Marc Kleine-Budde
Browse files

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: default avatarVincent MAILHOL <mailhol.vincent@wanadoo.fr>
Signed-off-by: default avatarFrank Jungclaus <frank.jungclaus@esd.eu>
Reviewed-by: default avatarVincent Mailhol <mailhol.vincent@wanadoo.fr>
Link: https://lore.kernel.org/all/20230222163754.3711766-1-frank.jungclaus@esd.eu


Signed-off-by: default avatarMarc Kleine-Budde <mkl@pengutronix.de>
parent af54c459
Loading
Loading
Loading
Loading
+82 −84
Original line number Diff line number Diff line
@@ -174,8 +174,7 @@ struct set_baudrate_msg {
};

/* Main message type used between library and application */
struct __packed esd_usb_msg {
	union {
union __packed esd_usb_msg {
	struct header_msg hdr;
	struct version_msg version;
	struct version_reply_msg version_reply;
@@ -184,7 +183,6 @@ struct __packed esd_usb_msg {
	struct tx_done_msg txdone;
	struct set_baudrate_msg setbaud;
	struct id_filter_msg filter;
	} msg;
};

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,