From a0340df7eca4f28e18afd3ecd6e464db5f2994c0 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Tue, 21 Mar 2023 17:15:00 +0100 Subject: [PATCH 01/11] can: rcar_canfd: Add transceiver support Add support for CAN transceivers described as PHYs. While simple CAN transceivers can do without, this is needed for CAN transceivers like NXP TJR1443 that need a configuration step (like pulling standby or enable lines), and/or impose a bitrate limit. Signed-off-by: Geert Uytterhoeven Reviewed-by: Simon Horman Reviewed-by: Vincent Mailhol Link: https://lore.kernel.org/all/1ce907572ac1d4e1733fa6ea7712250f2229cfcb.1679414936.git.geert+renesas@glider.be [mkl: squash error message update from patch 2] Signed-off-by: Marc Kleine-Budde --- drivers/net/can/rcar/rcar_canfd.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c index ef4e1b9a9e1e..d8fbc3fca475 100644 --- a/drivers/net/can/rcar/rcar_canfd.c +++ b/drivers/net/can/rcar/rcar_canfd.c @@ -35,6 +35,7 @@ #include #include #include +#include #include #include #include @@ -530,6 +531,7 @@ struct rcar_canfd_channel { struct net_device *ndev; struct rcar_canfd_global *gpriv; /* Controller reference */ void __iomem *base; /* Register base address */ + struct phy *transceiver; /* Optional transceiver */ struct napi_struct napi; u32 tx_head; /* Incremented on xmit */ u32 tx_tail; /* Incremented on xmit done */ @@ -1413,11 +1415,17 @@ static int rcar_canfd_open(struct net_device *ndev) struct rcar_canfd_global *gpriv = priv->gpriv; int err; + err = phy_power_on(priv->transceiver); + if (err) { + netdev_err(ndev, "failed to power on PHY: %pe\n", ERR_PTR(err)); + return err; + } + /* Peripheral clock is already enabled in probe */ err = clk_prepare_enable(gpriv->can_clk); if (err) { netdev_err(ndev, "failed to enable CAN clock, error %d\n", err); - goto out_clock; + goto out_phy; } err = open_candev(ndev); @@ -1437,7 +1445,8 @@ out_close: close_candev(ndev); out_can_clock: clk_disable_unprepare(gpriv->can_clk); -out_clock: +out_phy: + phy_power_off(priv->transceiver); return err; } @@ -1480,6 +1489,7 @@ static int rcar_canfd_close(struct net_device *ndev) napi_disable(&priv->napi); clk_disable_unprepare(gpriv->can_clk); close_candev(ndev); + phy_power_off(priv->transceiver); return 0; } @@ -1711,7 +1721,7 @@ static const struct ethtool_ops rcar_canfd_ethtool_ops = { }; static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch, - u32 fcan_freq) + u32 fcan_freq, struct phy *transceiver) { const struct rcar_canfd_hw_info *info = gpriv->info; struct platform_device *pdev = gpriv->pdev; @@ -1732,8 +1742,11 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch, ndev->flags |= IFF_ECHO; priv->ndev = ndev; priv->base = gpriv->base; + priv->transceiver = transceiver; priv->channel = ch; priv->gpriv = gpriv; + if (transceiver) + priv->can.bitrate_max = transceiver->attrs.max_link_rate; priv->can.clock.freq = fcan_freq; dev_info(dev, "can_clk rate is %u\n", priv->can.clock.freq); @@ -1836,6 +1849,7 @@ static void rcar_canfd_channel_remove(struct rcar_canfd_global *gpriv, u32 ch) static int rcar_canfd_probe(struct platform_device *pdev) { + struct phy *transceivers[RCANFD_NUM_CHANNELS] = { 0, }; const struct rcar_canfd_hw_info *info; struct device *dev = &pdev->dev; void __iomem *addr; @@ -1857,9 +1871,14 @@ static int rcar_canfd_probe(struct platform_device *pdev) for (i = 0; i < info->max_channels; ++i) { name[7] = '0' + i; of_child = of_get_child_by_name(dev->of_node, name); - if (of_child && of_device_is_available(of_child)) + if (of_child && of_device_is_available(of_child)) { channels_mask |= BIT(i); + transceivers[i] = devm_of_phy_optional_get(dev, + of_child, NULL); + } of_node_put(of_child); + if (IS_ERR(transceivers[i])) + return PTR_ERR(transceivers[i]); } if (info->shared_global_irqs) { @@ -2035,7 +2054,8 @@ static int rcar_canfd_probe(struct platform_device *pdev) } for_each_set_bit(ch, &gpriv->channels_mask, info->max_channels) { - err = rcar_canfd_channel_probe(gpriv, ch, fcan_freq); + err = rcar_canfd_channel_probe(gpriv, ch, fcan_freq, + transceivers[ch]); if (err) goto fail_channel; } From 33eced402b184b5f2127f45531099c265d34e269 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Tue, 21 Mar 2023 17:15:01 +0100 Subject: [PATCH 02/11] can: rcar_canfd: Improve error messages Improve printed error messages: - Replace numerical error codes by mnemotechnic error codes, to improve the user experience in case of errors, - Drop parentheses around printed numbers, cfr. Documentation/process/coding-style.rst, - Drop printing of an error message in case of out-of-memory, as the core memory allocation code already takes care of this. Suggested-by: Vincent Mailhol Signed-off-by: Geert Uytterhoeven Reviewed-by: Simon Horman Reviewed-by: Vincent Mailhol Link: https://lore.kernel.org/all/4162cc46f72257ec191007675933985b6df394b9.1679414936.git.geert+renesas@glider.be [mkl: use colon instead of comma] Signed-off-by: Marc Kleine-Budde --- drivers/net/can/rcar/rcar_canfd.c | 41 +++++++++++++++---------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c index d8fbc3fca475..701311dabab3 100644 --- a/drivers/net/can/rcar/rcar_canfd.c +++ b/drivers/net/can/rcar/rcar_canfd.c @@ -1424,13 +1424,13 @@ static int rcar_canfd_open(struct net_device *ndev) /* Peripheral clock is already enabled in probe */ err = clk_prepare_enable(gpriv->can_clk); if (err) { - netdev_err(ndev, "failed to enable CAN clock, error %d\n", err); + netdev_err(ndev, "failed to enable CAN clock: %pe\n", ERR_PTR(err)); goto out_phy; } err = open_candev(ndev); if (err) { - netdev_err(ndev, "open_candev() failed, error %d\n", err); + netdev_err(ndev, "open_candev() failed: %pe\n", ERR_PTR(err)); goto out_can_clock; } @@ -1731,10 +1731,9 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch, int err = -ENODEV; ndev = alloc_candev(sizeof(*priv), RCANFD_FIFO_DEPTH); - if (!ndev) { - dev_err(dev, "alloc_candev() failed\n"); + if (!ndev) return -ENOMEM; - } + priv = netdev_priv(ndev); ndev->netdev_ops = &rcar_canfd_netdev_ops; @@ -1777,8 +1776,8 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch, rcar_canfd_channel_err_interrupt, 0, irq_name, priv); if (err) { - dev_err(dev, "devm_request_irq CH Err(%d) failed, error %d\n", - err_irq, err); + dev_err(dev, "devm_request_irq CH Err %d failed: %pe\n", + err_irq, ERR_PTR(err)); goto fail; } irq_name = devm_kasprintf(dev, GFP_KERNEL, "canfd.ch%d_trx", @@ -1791,8 +1790,8 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch, rcar_canfd_channel_tx_interrupt, 0, irq_name, priv); if (err) { - dev_err(dev, "devm_request_irq Tx (%d) failed, error %d\n", - tx_irq, err); + dev_err(dev, "devm_request_irq Tx %d failed: %pe\n", + tx_irq, ERR_PTR(err)); goto fail; } } @@ -1823,7 +1822,7 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch, gpriv->ch[priv->channel] = priv; err = register_candev(ndev); if (err) { - dev_err(dev, "register_candev() failed, error %d\n", err); + dev_err(dev, "register_candev() failed: %pe\n", ERR_PTR(err)); goto fail_candev; } dev_info(dev, "device registered (channel %u)\n", priv->channel); @@ -1967,16 +1966,16 @@ static int rcar_canfd_probe(struct platform_device *pdev) rcar_canfd_channel_interrupt, 0, "canfd.ch_int", gpriv); if (err) { - dev_err(dev, "devm_request_irq(%d) failed, error %d\n", - ch_irq, err); + dev_err(dev, "devm_request_irq %d failed: %pe\n", + ch_irq, ERR_PTR(err)); goto fail_dev; } err = devm_request_irq(dev, g_irq, rcar_canfd_global_interrupt, 0, "canfd.g_int", gpriv); if (err) { - dev_err(dev, "devm_request_irq(%d) failed, error %d\n", - g_irq, err); + dev_err(dev, "devm_request_irq %d failed: %pe\n", + g_irq, ERR_PTR(err)); goto fail_dev; } } else { @@ -1985,8 +1984,8 @@ static int rcar_canfd_probe(struct platform_device *pdev) "canfd.g_recc", gpriv); if (err) { - dev_err(dev, "devm_request_irq(%d) failed, error %d\n", - g_recc_irq, err); + dev_err(dev, "devm_request_irq %d failed: %pe\n", + g_recc_irq, ERR_PTR(err)); goto fail_dev; } @@ -1994,8 +1993,8 @@ static int rcar_canfd_probe(struct platform_device *pdev) rcar_canfd_global_err_interrupt, 0, "canfd.g_err", gpriv); if (err) { - dev_err(dev, "devm_request_irq(%d) failed, error %d\n", - g_err_irq, err); + dev_err(dev, "devm_request_irq %d failed: %pe\n", + g_err_irq, ERR_PTR(err)); goto fail_dev; } } @@ -2012,14 +2011,14 @@ static int rcar_canfd_probe(struct platform_device *pdev) /* Enable peripheral clock for register access */ err = clk_prepare_enable(gpriv->clkp); if (err) { - dev_err(dev, "failed to enable peripheral clock, error %d\n", - err); + dev_err(dev, "failed to enable peripheral clock: %pe\n", + ERR_PTR(err)); goto fail_reset; } err = rcar_canfd_reset_controller(gpriv); if (err) { - dev_err(dev, "reset controller failed\n"); + dev_err(dev, "reset controller failed: %pe\n", ERR_PTR(err)); goto fail_clk; } From 594503341de74f69dbccaedf4dbbb3aa02ec0534 Mon Sep 17 00:00:00 2001 From: Cai Huoqing Date: Thu, 23 Mar 2023 19:33:15 +0800 Subject: [PATCH 03/11] can: c_can: Remove redundant pci_clear_master Remove pci_clear_master to simplify the code, the bus-mastering is also cleared in do_pci_disable_device, like this: ./drivers/pci/pci.c:2197 static void do_pci_disable_device(struct pci_dev *dev) { u16 pci_command; pci_read_config_word(dev, PCI_COMMAND, &pci_command); if (pci_command & PCI_COMMAND_MASTER) { pci_command &= ~PCI_COMMAND_MASTER; pci_write_config_word(dev, PCI_COMMAND, pci_command); } pcibios_disable_device(dev); }. And dev->is_busmaster is set to 0 in pci_disable_device. Signed-off-by: Cai Huoqing Link: https://lore.kernel.org/all/20230323113318.9473-1-cai.huoqing@linux.dev Signed-off-by: Marc Kleine-Budde --- drivers/net/can/c_can/c_can_pci.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/net/can/c_can/c_can_pci.c b/drivers/net/can/c_can/c_can_pci.c index bf2f8c3da1c1..093bea597f4e 100644 --- a/drivers/net/can/c_can/c_can_pci.c +++ b/drivers/net/can/c_can/c_can_pci.c @@ -227,7 +227,6 @@ out_iounmap: pci_iounmap(pdev, addr); out_release_regions: pci_disable_msi(pdev); - pci_clear_master(pdev); pci_release_regions(pdev); out_disable_device: pci_disable_device(pdev); @@ -247,7 +246,6 @@ static void c_can_pci_remove(struct pci_dev *pdev) pci_iounmap(pdev, addr); pci_disable_msi(pdev); - pci_clear_master(pdev); pci_release_regions(pdev); pci_disable_device(pdev); } From c9d23f9657cabfd2836a096bf6eddf8df2cf1434 Mon Sep 17 00:00:00 2001 From: Cai Huoqing Date: Thu, 23 Mar 2023 19:33:16 +0800 Subject: [PATCH 04/11] can: ctucanfd: Remove redundant pci_clear_master Remove pci_clear_master to simplify the code, the bus-mastering is also cleared in do_pci_disable_device, like this: ./drivers/pci/pci.c:2197 static void do_pci_disable_device(struct pci_dev *dev) { u16 pci_command; pci_read_config_word(dev, PCI_COMMAND, &pci_command); if (pci_command & PCI_COMMAND_MASTER) { pci_command &= ~PCI_COMMAND_MASTER; pci_write_config_word(dev, PCI_COMMAND, pci_command); } pcibios_disable_device(dev); }. And dev->is_busmaster is set to 0 in pci_disable_device. Signed-off-by: Cai Huoqing Link: https://lore.kernel.org/all/20230323113318.9473-2-cai.huoqing@linux.dev Signed-off-by: Marc Kleine-Budde --- drivers/net/can/ctucanfd/ctucanfd_pci.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/net/can/ctucanfd/ctucanfd_pci.c b/drivers/net/can/ctucanfd/ctucanfd_pci.c index 8f2956a8ae43..9da09e7dd63a 100644 --- a/drivers/net/can/ctucanfd/ctucanfd_pci.c +++ b/drivers/net/can/ctucanfd/ctucanfd_pci.c @@ -206,10 +206,8 @@ err_pci_iounmap_bar0: err_pci_iounmap_bar1: pci_iounmap(pdev, addr); err_release_regions: - if (msi_ok) { + if (msi_ok) pci_disable_msi(pdev); - pci_clear_master(pdev); - } pci_release_regions(pdev); err_disable_device: pci_disable_device(pdev); @@ -257,10 +255,8 @@ static void ctucan_pci_remove(struct pci_dev *pdev) pci_iounmap(pdev, bdata->bar1_base); - if (bdata->use_msi) { + if (bdata->use_msi) pci_disable_msi(pdev); - pci_clear_master(pdev); - } pci_release_regions(pdev); pci_disable_device(pdev); From 8db931835fad6a2229166fc0719f55c5ba070516 Mon Sep 17 00:00:00 2001 From: Cai Huoqing Date: Thu, 23 Mar 2023 19:33:17 +0800 Subject: [PATCH 05/11] can: kvaser_pciefd: Remove redundant pci_clear_master Remove pci_clear_master to simplify the code, the bus-mastering is also cleared in do_pci_disable_device, like this: ./drivers/pci/pci.c:2197 static void do_pci_disable_device(struct pci_dev *dev) { u16 pci_command; pci_read_config_word(dev, PCI_COMMAND, &pci_command); if (pci_command & PCI_COMMAND_MASTER) { pci_command &= ~PCI_COMMAND_MASTER; pci_write_config_word(dev, PCI_COMMAND, pci_command); } pcibios_disable_device(dev); }. And dev->is_busmaster is set to 0 in pci_disable_device. Signed-off-by: Cai Huoqing Link: https://lore.kernel.org/all/20230323113318.9473-3-cai.huoqing@linux.dev Signed-off-by: Marc Kleine-Budde --- drivers/net/can/kvaser_pciefd.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c index bcad11709bc9..53e8a914c88b 100644 --- a/drivers/net/can/kvaser_pciefd.c +++ b/drivers/net/can/kvaser_pciefd.c @@ -1907,7 +1907,6 @@ static void kvaser_pciefd_remove(struct pci_dev *pdev) free_irq(pcie->pci->irq, pcie); - pci_clear_master(pdev); pci_iounmap(pdev, pcie->reg_base); pci_release_regions(pdev); pci_disable_device(pdev); From a57915aee31596d2380a875964c78f743162383c Mon Sep 17 00:00:00 2001 From: Frank Jungclaus Date: Wed, 22 Feb 2023 17:37:54 +0100 Subject: [PATCH 06/11] 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 Signed-off-by: Frank Jungclaus Reviewed-by: Vincent Mailhol Link: https://lore.kernel.org/all/20230222163754.3711766-1-frank.jungclaus@esd.eu Signed-off-by: Marc Kleine-Budde --- 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 @@ 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, From 73042934e4a30d9f45ff9cb0b3b029a01dbe7130 Mon Sep 17 00:00:00 2001 From: Markus Schneider-Pargmann Date: Wed, 15 Mar 2023 12:05:31 +0100 Subject: [PATCH 07/11] can: m_can: Remove repeated check for is_peripheral Merge both if-blocks to fix this. Signed-off-by: Markus Schneider-Pargmann Reviewed-by: Simon Horman Link: https://lore.kernel.org/all/20230315110546.2518305-2-msp@baylibre.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/m_can/m_can.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c index 8e83d6963d85..563625a701fc 100644 --- a/drivers/net/can/m_can/m_can.c +++ b/drivers/net/can/m_can/m_can.c @@ -1592,10 +1592,8 @@ static int m_can_close(struct net_device *dev) cdev->tx_skb = NULL; destroy_workqueue(cdev->tx_wq); cdev->tx_wq = NULL; - } - - if (cdev->is_peripheral) can_rx_offload_disable(&cdev->offload); + } close_candev(dev); From 4ab639480900be41d7c7c0b89a6db7fa5fe951e4 Mon Sep 17 00:00:00 2001 From: Markus Schneider-Pargmann Date: Wed, 15 Mar 2023 12:05:32 +0100 Subject: [PATCH 08/11] can: m_can: Always acknowledge all interrupts The code already exits the function on !ir before this condition. No need to check again if anything is set as IR_ALL_INT is 0xffffffff. Signed-off-by: Markus Schneider-Pargmann Reviewed-by: Simon Horman Link: https://lore.kernel.org/all/20230315110546.2518305-3-msp@baylibre.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/m_can/m_can.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c index 563625a701fc..8eb327ae3bdf 100644 --- a/drivers/net/can/m_can/m_can.c +++ b/drivers/net/can/m_can/m_can.c @@ -1083,8 +1083,7 @@ static irqreturn_t m_can_isr(int irq, void *dev_id) return IRQ_NONE; /* ACK all irqs */ - if (ir & IR_ALL_INT) - m_can_write(cdev, M_CAN_IR, ir); + m_can_write(cdev, M_CAN_IR, ir); if (cdev->ops->clear_interrupts) cdev->ops->clear_interrupts(cdev); From 71725bfdbbf263ec4897ce310087d3f6bd331c19 Mon Sep 17 00:00:00 2001 From: Markus Schneider-Pargmann Date: Wed, 15 Mar 2023 12:05:33 +0100 Subject: [PATCH 09/11] can: m_can: Remove double interrupt enable Interrupts are enabled a few lines further down as well. Remove this second call to enable all interrupts. Signed-off-by: Markus Schneider-Pargmann Reviewed-by: Simon Horman Link: https://lore.kernel.org/all/20230315110546.2518305-4-msp@baylibre.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/m_can/m_can.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c index 8eb327ae3bdf..5274d9642566 100644 --- a/drivers/net/can/m_can/m_can.c +++ b/drivers/net/can/m_can/m_can.c @@ -1364,7 +1364,6 @@ static int m_can_chip_config(struct net_device *dev) m_can_write(cdev, M_CAN_TEST, test); /* Enable interrupts */ - m_can_write(cdev, M_CAN_IR, IR_ALL_INT); if (!(cdev->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING)) if (cdev->version == 30) m_can_write(cdev, M_CAN_IE, IR_ALL_INT & From 897e663218e2d05607a6f5b258134cb026a2fa85 Mon Sep 17 00:00:00 2001 From: Markus Schneider-Pargmann Date: Wed, 15 Mar 2023 12:05:34 +0100 Subject: [PATCH 10/11] can: m_can: Disable unused interrupts There are a number of interrupts that are not used by the driver at the moment. Disable all of these. Signed-off-by: Markus Schneider-Pargmann Reviewed-by: Simon Horman Link: https://lore.kernel.org/all/20230315110546.2518305-5-msp@baylibre.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/m_can/m_can.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c index 5274d9642566..e7aceeba3759 100644 --- a/drivers/net/can/m_can/m_can.c +++ b/drivers/net/can/m_can/m_can.c @@ -1261,6 +1261,7 @@ static int m_can_set_bittiming(struct net_device *dev) static int m_can_chip_config(struct net_device *dev) { struct m_can_classdev *cdev = netdev_priv(dev); + u32 interrupts = IR_ALL_INT; u32 cccr, test; int err; @@ -1270,6 +1271,11 @@ static int m_can_chip_config(struct net_device *dev) return err; } + /* Disable unused interrupts */ + interrupts &= ~(IR_ARA | IR_ELO | IR_DRX | IR_TEFF | IR_TEFW | IR_TFE | + IR_TCF | IR_HPM | IR_RF1F | IR_RF1W | IR_RF1N | + IR_RF0F | IR_RF0W); + m_can_config_endisable(cdev, true); /* RX Buffer/FIFO Element Size 64 bytes data field */ @@ -1364,15 +1370,13 @@ static int m_can_chip_config(struct net_device *dev) m_can_write(cdev, M_CAN_TEST, test); /* Enable interrupts */ - if (!(cdev->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING)) + if (!(cdev->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING)) { if (cdev->version == 30) - m_can_write(cdev, M_CAN_IE, IR_ALL_INT & - ~(IR_ERR_LEC_30X)); + interrupts &= ~(IR_ERR_LEC_30X); else - m_can_write(cdev, M_CAN_IE, IR_ALL_INT & - ~(IR_ERR_LEC_31X)); - else - m_can_write(cdev, M_CAN_IE, IR_ALL_INT); + interrupts &= ~(IR_ERR_LEC_31X); + } + m_can_write(cdev, M_CAN_IE, interrupts); /* route all interrupts to INT0 */ m_can_write(cdev, M_CAN_ILS, ILS_ALL_INT0); From 9083e0b09df33376abef2bb28e6a4b0c88f4d00b Mon Sep 17 00:00:00 2001 From: Markus Schneider-Pargmann Date: Wed, 15 Mar 2023 12:05:35 +0100 Subject: [PATCH 11/11] can: m_can: Keep interrupts enabled during peripheral read Interrupts currently get disabled if the interrupt status shows new received data. Non-peripheral chips handle receiving in a worker thread, but peripheral chips are handling the receive process in the threaded interrupt routine itself without scheduling it for a different worker. So there is no need to disable interrupts for peripheral chips. Signed-off-by: Markus Schneider-Pargmann Reviewed-by: Simon Horman Link: https://lore.kernel.org/all/20230315110546.2518305-6-msp@baylibre.com Signed-off-by: Marc Kleine-Budde --- drivers/net/can/m_can/m_can.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c index e7aceeba3759..a5003435802b 100644 --- a/drivers/net/can/m_can/m_can.c +++ b/drivers/net/can/m_can/m_can.c @@ -972,8 +972,8 @@ static int m_can_rx_peripheral(struct net_device *dev, u32 irqstatus) /* Don't re-enable interrupts if the driver had a fatal error * (e.g., FIFO read failure). */ - if (work_done >= 0) - m_can_enable_all_interrupts(cdev); + if (work_done < 0) + m_can_disable_all_interrupts(cdev); return work_done; } @@ -1095,11 +1095,12 @@ static irqreturn_t m_can_isr(int irq, void *dev_id) */ if ((ir & IR_RF0N) || (ir & IR_ERR_ALL_30X)) { cdev->irqstatus = ir; - m_can_disable_all_interrupts(cdev); - if (!cdev->is_peripheral) + if (!cdev->is_peripheral) { + m_can_disable_all_interrupts(cdev); napi_schedule(&cdev->napi); - else if (m_can_rx_peripheral(dev, ir) < 0) + } else if (m_can_rx_peripheral(dev, ir) < 0) { goto out_fail; + } } if (cdev->version == 30) {