From 66224f6656d10c379da10db2208ebfae02854a08 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Wed, 11 Nov 2020 09:08:47 +0100 Subject: [PATCH 01/36] dt-binding: can: mcp251xfd: include common CAN controller bindings Since commit | 1f9234401ce0 ("dt-bindings: can: add can-controller.yaml") there is a common CAN controller binding. Add this to the mcp251xfd binding. Link: https://lore.kernel.org/all/20220124220653.3477172-2-mkl@pengutronix.de Cc: Manivannan Sadhasivam Cc: Thomas Kopp Acked-by: Rob Herring Signed-off-by: Marc Kleine-Budde --- .../devicetree/bindings/net/can/microchip,mcp251xfd.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/net/can/microchip,mcp251xfd.yaml b/Documentation/devicetree/bindings/net/can/microchip,mcp251xfd.yaml index 2a884c1fe0e0..b3826af6bd6e 100644 --- a/Documentation/devicetree/bindings/net/can/microchip,mcp251xfd.yaml +++ b/Documentation/devicetree/bindings/net/can/microchip,mcp251xfd.yaml @@ -11,6 +11,9 @@ title: maintainers: - Marc Kleine-Budde +allOf: + - $ref: can-controller.yaml# + properties: compatible: oneOf: From d931686dc21f9092c5367ba6b51d4b12772d4685 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Wed, 11 Nov 2020 09:08:47 +0100 Subject: [PATCH 02/36] dt-binding: can: sun4i_can: include common CAN controller bindings Since commit | 1f9234401ce0 ("dt-bindings: can: add can-controller.yaml") there is a common CAN controller binding. Add this to the sun4i_can binding. Link: https://lore.kernel.org/all/20220124220653.3477172-3-mkl@pengutronix.de Cc: Evgeny Boger Cc: Gerhard Bertelsmann Acked-by: Rob Herring Signed-off-by: Marc Kleine-Budde --- .../devicetree/bindings/net/can/allwinner,sun4i-a10-can.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/net/can/allwinner,sun4i-a10-can.yaml b/Documentation/devicetree/bindings/net/can/allwinner,sun4i-a10-can.yaml index c93fe9d3ea82..3c51b2d02957 100644 --- a/Documentation/devicetree/bindings/net/can/allwinner,sun4i-a10-can.yaml +++ b/Documentation/devicetree/bindings/net/can/allwinner,sun4i-a10-can.yaml @@ -10,6 +10,9 @@ maintainers: - Chen-Yu Tsai - Maxime Ripard +allOf: + - $ref: can-controller.yaml# + properties: compatible: oneOf: From edd056a109eedaa8445991089cd6c0f57a871061 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Thu, 17 Feb 2022 12:36:09 +0100 Subject: [PATCH 03/36] dt-binding: can: m_can: list Chandrasekar Ramakrishnan as maintainer Since Sriram Dash's email bounces, change the maintainer entry to Chandrasekar Ramakrishnan. Chandrasekar Ramakrishnan is already listed as a maintainer in the MAINTAINERS file. Link: https://lore.kernel.org/all/20220217113839.2311417-1-mkl@pengutronix.de Cc: Chandrasekar Ramakrishnan Acked-by: Krzysztof Kozlowski Signed-off-by: Marc Kleine-Budde --- Documentation/devicetree/bindings/net/can/bosch,m_can.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml b/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml index 401ab7cdb379..f8f504953e78 100644 --- a/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml +++ b/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml @@ -9,7 +9,7 @@ title: Bosch MCAN controller Bindings description: Bosch MCAN controller for CAN bus maintainers: - - Sriram Dash + - Chandrasekar Ramakrishnan properties: compatible: From bffd5217ca2e17b069604a2de3bc00296b951c6f Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Tue, 18 Jan 2022 15:18:50 +0100 Subject: [PATCH 04/36] dt-binding: can: m_can: fix indention of table in bosch,mram-cfg description This patch fixes the indention of the table in the description of the bosch,mram-cfg property. Link: https://lore.kernel.org/all/20220217101111.2291151-1-mkl@pengutronix.de Reviewed-by: Chandrasekar Ramakrishnan Signed-off-by: Marc Kleine-Budde --- Documentation/devicetree/bindings/net/can/bosch,m_can.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml b/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml index f8f504953e78..fd4d291fe4c1 100644 --- a/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml +++ b/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml @@ -66,8 +66,8 @@ properties: M_CAN includes the following elements according to user manual: 11-bit Filter 0-128 elements / 0-128 words 29-bit Filter 0-64 elements / 0-128 words - Rx FIFO 0 0-64 elements / 0-1152 words - Rx FIFO 1 0-64 elements / 0-1152 words + Rx FIFO 0 0-64 elements / 0-1152 words + Rx FIFO 1 0-64 elements / 0-1152 words Rx Buffers 0-64 elements / 0-1152 words Tx Event FIFO 0-32 elements / 0-64 words Tx Buffers 0-32 elements / 0-576 words From 58212e03e5ec315b368afea5a32a87866feb59b0 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Wed, 11 Nov 2020 09:08:47 +0100 Subject: [PATCH 05/36] dt-binding: can: m_can: include common CAN controller bindings Since commit | 1f9234401ce0 ("dt-bindings: can: add can-controller.yaml") there is a common CAN controller binding. Add this to the m_can binding. Link: https://lore.kernel.org/all/20220124220653.3477172-4-mkl@pengutronix.de Reviewed-by: Chandrasekar Ramakrishnan Acked-by: Rob Herring Signed-off-by: Marc Kleine-Budde --- Documentation/devicetree/bindings/net/can/bosch,m_can.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml b/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml index fd4d291fe4c1..b7f9803c1c6d 100644 --- a/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml +++ b/Documentation/devicetree/bindings/net/can/bosch,m_can.yaml @@ -11,6 +11,9 @@ description: Bosch MCAN controller for CAN bus maintainers: - Chandrasekar Ramakrishnan +allOf: + - $ref: can-controller.yaml# + properties: compatible: const: bosch,m_can From 181d4447905d551cc664f1e7e796b482c1eec992 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Mon, 7 Feb 2022 11:07:06 -0800 Subject: [PATCH 06/36] can: gw: use call_rcu() instead of costly synchronize_rcu() Commit fb8696ab14ad ("can: gw: synchronize rcu operations before removing gw job entry") added three synchronize_rcu() calls to make sure one rcu grace period was observed before freeing a "struct cgw_job" (which are tiny objects). This should be converted to call_rcu() to avoid adding delays in device / network dismantles. Use the rcu_head that was already in struct cgw_job, not yet used. Link: https://lore.kernel.org/all/20220207190706.1499190-1-eric.dumazet@gmail.com Signed-off-by: Eric Dumazet Cc: Oliver Hartkopp Tested-by: Oliver Hartkopp Signed-off-by: Marc Kleine-Budde --- net/can/gw.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/net/can/gw.c b/net/can/gw.c index 24221352e059..1ea4cc527db3 100644 --- a/net/can/gw.c +++ b/net/can/gw.c @@ -577,6 +577,13 @@ static inline void cgw_unregister_filter(struct net *net, struct cgw_job *gwj) gwj->ccgw.filter.can_mask, can_can_gw_rcv, gwj); } +static void cgw_job_free_rcu(struct rcu_head *rcu_head) +{ + struct cgw_job *gwj = container_of(rcu_head, struct cgw_job, rcu); + + kmem_cache_free(cgw_cache, gwj); +} + static int cgw_notifier(struct notifier_block *nb, unsigned long msg, void *ptr) { @@ -596,8 +603,7 @@ static int cgw_notifier(struct notifier_block *nb, if (gwj->src.dev == dev || gwj->dst.dev == dev) { hlist_del(&gwj->list); cgw_unregister_filter(net, gwj); - synchronize_rcu(); - kmem_cache_free(cgw_cache, gwj); + call_rcu(&gwj->rcu, cgw_job_free_rcu); } } } @@ -1155,8 +1161,7 @@ static void cgw_remove_all_jobs(struct net *net) hlist_for_each_entry_safe(gwj, nx, &net->can.cgw_list, list) { hlist_del(&gwj->list); cgw_unregister_filter(net, gwj); - synchronize_rcu(); - kmem_cache_free(cgw_cache, gwj); + call_rcu(&gwj->rcu, cgw_job_free_rcu); } } @@ -1224,8 +1229,7 @@ static int cgw_remove_job(struct sk_buff *skb, struct nlmsghdr *nlh, hlist_del(&gwj->list); cgw_unregister_filter(net, gwj); - synchronize_rcu(); - kmem_cache_free(cgw_cache, gwj); + call_rcu(&gwj->rcu, cgw_job_free_rcu); err = 0; break; } From 5b60d334e42a37febfd284bd388e526fcddce02a Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Thu, 19 Dec 2019 10:17:37 +0100 Subject: [PATCH 07/36] can: bittiming: can_validate_bitrate(): simplify bit rate checking This patch simplifies the validation of the fixed bit rates. If a supported bit rate is found, directly return 0. If no valid bit rate is found return -EINVAL; Link: https://lore.kernel.org/all/20220124215642.3474154-6-mkl@pengutronix.de Signed-off-by: Marc Kleine-Budde --- drivers/net/can/dev/bittiming.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/net/can/dev/bittiming.c b/drivers/net/can/dev/bittiming.c index d5fca3bfaf9a..1b1d1499e2f1 100644 --- a/drivers/net/can/dev/bittiming.c +++ b/drivers/net/can/dev/bittiming.c @@ -248,18 +248,14 @@ can_validate_bitrate(struct net_device *dev, struct can_bittiming *bt, const u32 *bitrate_const, const unsigned int bitrate_const_cnt) { - struct can_priv *priv = netdev_priv(dev); unsigned int i; for (i = 0; i < bitrate_const_cnt; i++) { if (bt->bitrate == bitrate_const[i]) - break; + return 0; } - if (i >= priv->bitrate_const_cnt) - return -EINVAL; - - return 0; + return -EINVAL; } int can_get_bittiming(struct net_device *dev, struct can_bittiming *bt, From 5597f082fcaf17e2d9adee7a1f6fa02f5872f9d5 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Thu, 13 Jan 2022 15:34:07 +0100 Subject: [PATCH 08/36] can: bittiming: mark function arguments and local variables as const This patch marks the arguments of some functions as well as some local variables as constant. Link: https://lore.kernel.org/all/20220124215642.3474154-7-mkl@pengutronix.de Signed-off-by: Marc Kleine-Budde --- drivers/net/can/dev/bittiming.c | 12 ++++++------ include/linux/can/bittiming.h | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/net/can/dev/bittiming.c b/drivers/net/can/dev/bittiming.c index 1b1d1499e2f1..2103bcca9012 100644 --- a/drivers/net/can/dev/bittiming.c +++ b/drivers/net/can/dev/bittiming.c @@ -24,7 +24,7 @@ */ static int can_update_sample_point(const struct can_bittiming_const *btc, - unsigned int sample_point_nominal, unsigned int tseg, + const unsigned int sample_point_nominal, const unsigned int tseg, unsigned int *tseg1_ptr, unsigned int *tseg2_ptr, unsigned int *sample_point_error_ptr) { @@ -63,7 +63,7 @@ can_update_sample_point(const struct can_bittiming_const *btc, return best_sample_point; } -int can_calc_bittiming(struct net_device *dev, struct can_bittiming *bt, +int can_calc_bittiming(const struct net_device *dev, struct can_bittiming *bt, const struct can_bittiming_const *btc) { struct can_priv *priv = netdev_priv(dev); @@ -208,10 +208,10 @@ void can_calc_tdco(struct can_tdc *tdc, const struct can_tdc_const *tdc_const, * prescaler value brp. You can find more information in the header * file linux/can/netlink.h. */ -static int can_fixup_bittiming(struct net_device *dev, struct can_bittiming *bt, +static int can_fixup_bittiming(const struct net_device *dev, struct can_bittiming *bt, const struct can_bittiming_const *btc) { - struct can_priv *priv = netdev_priv(dev); + const struct can_priv *priv = netdev_priv(dev); unsigned int tseg1, alltseg; u64 brp64; @@ -244,7 +244,7 @@ static int can_fixup_bittiming(struct net_device *dev, struct can_bittiming *bt, /* Checks the validity of predefined bitrate settings */ static int -can_validate_bitrate(struct net_device *dev, struct can_bittiming *bt, +can_validate_bitrate(const struct net_device *dev, const struct can_bittiming *bt, const u32 *bitrate_const, const unsigned int bitrate_const_cnt) { @@ -258,7 +258,7 @@ can_validate_bitrate(struct net_device *dev, struct can_bittiming *bt, return -EINVAL; } -int can_get_bittiming(struct net_device *dev, struct can_bittiming *bt, +int can_get_bittiming(const struct net_device *dev, struct can_bittiming *bt, const struct can_bittiming_const *btc, const u32 *bitrate_const, const unsigned int bitrate_const_cnt) diff --git a/include/linux/can/bittiming.h b/include/linux/can/bittiming.h index a81652d1c6f3..7ae21c0f7f23 100644 --- a/include/linux/can/bittiming.h +++ b/include/linux/can/bittiming.h @@ -113,7 +113,7 @@ struct can_tdc_const { }; #ifdef CONFIG_CAN_CALC_BITTIMING -int can_calc_bittiming(struct net_device *dev, struct can_bittiming *bt, +int can_calc_bittiming(const struct net_device *dev, struct can_bittiming *bt, const struct can_bittiming_const *btc); void can_calc_tdco(struct can_tdc *tdc, const struct can_tdc_const *tdc_const, @@ -121,7 +121,7 @@ void can_calc_tdco(struct can_tdc *tdc, const struct can_tdc_const *tdc_const, u32 *ctrlmode, u32 ctrlmode_supported); #else /* !CONFIG_CAN_CALC_BITTIMING */ static inline int -can_calc_bittiming(struct net_device *dev, struct can_bittiming *bt, +can_calc_bittiming(const struct net_device *dev, struct can_bittiming *bt, const struct can_bittiming_const *btc) { netdev_err(dev, "bit-timing calculation not available\n"); @@ -136,7 +136,7 @@ can_calc_tdco(struct can_tdc *tdc, const struct can_tdc_const *tdc_const, } #endif /* CONFIG_CAN_CALC_BITTIMING */ -int can_get_bittiming(struct net_device *dev, struct can_bittiming *bt, +int can_get_bittiming(const struct net_device *dev, struct can_bittiming *bt, const struct can_bittiming_const *btc, const u32 *bitrate_const, const unsigned int bitrate_const_cnt); From 1c256e3a2c7622d8a01a0bb9f0500330753adc70 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Sun, 7 Jun 2020 19:02:18 +0200 Subject: [PATCH 09/36] can: kvaser_usb: kvaser_usb_send_cmd(): remove redundant variable actual_len The function usb_bulk_msg() can be called with a NULL pointer as the "actual_length" parameter. This patch removes this variable. Link: https://lore.kernel.org/all/20220124215642.3474154-9-mkl@pengutronix.de Cc: Jimmy Assarsson Signed-off-by: Marc Kleine-Budde --- drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c index c4b4d3d0a387..e67658b53d02 100644 --- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c +++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c @@ -205,12 +205,10 @@ MODULE_DEVICE_TABLE(usb, kvaser_usb_table); int kvaser_usb_send_cmd(const struct kvaser_usb *dev, void *cmd, int len) { - int actual_len; /* Not used */ - return usb_bulk_msg(dev->udev, usb_sndbulkpipe(dev->udev, dev->bulk_out->bEndpointAddress), - cmd, len, &actual_len, KVASER_USB_TIMEOUT); + cmd, len, NULL, KVASER_USB_TIMEOUT); } int kvaser_usb_recv_cmd(const struct kvaser_usb *dev, void *cmd, int len, From 8d0a82e1f42f4c2531552a5d140ee119ecc253b6 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Wed, 12 Jan 2022 21:34:41 +0100 Subject: [PATCH 10/36] can: c_can: ethtool: use default drvinfo The ethtool core implements a default drvinfo. There's no need to replicate this in the driver, no additional information is added, so remove this and rely on the default. Link: https://lore.kernel.org/all/20220124215642.3474154-10-mkl@pengutronix.de Cc: Dario Binacchi Signed-off-by: Marc Kleine-Budde --- drivers/net/can/c_can/c_can_ethtool.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/drivers/net/can/c_can/c_can_ethtool.c b/drivers/net/can/c_can/c_can_ethtool.c index 6655146294fc..8a826a6813bd 100644 --- a/drivers/net/can/c_can/c_can_ethtool.c +++ b/drivers/net/can/c_can/c_can_ethtool.c @@ -11,14 +11,6 @@ #include "c_can.h" -static void c_can_get_drvinfo(struct net_device *netdev, - struct ethtool_drvinfo *info) -{ - struct c_can_priv *priv = netdev_priv(netdev); - strscpy(info->driver, "c_can", sizeof(info->driver)); - strscpy(info->bus_info, dev_name(priv->device), sizeof(info->bus_info)); -} - static void c_can_get_ringparam(struct net_device *netdev, struct ethtool_ringparam *ring, struct kernel_ethtool_ringparam *kernel_ring, @@ -33,7 +25,6 @@ static void c_can_get_ringparam(struct net_device *netdev, } static const struct ethtool_ops c_can_ethtool_ops = { - .get_drvinfo = c_can_get_drvinfo, .get_ringparam = c_can_get_ringparam, }; From 51ae468aa7e412315203d13ef387751f0c58c55c Mon Sep 17 00:00:00 2001 From: Minghao Chi Date: Wed, 12 Jan 2022 08:06:29 +0000 Subject: [PATCH 11/36] can: softing: softing_netdev_open(): remove redundant ret variable Return value from softing_startstop() directly instead of taking this in another redundant variable. Link: https://lore.kernel.org/all/20220112080629.667191-1-chi.minghao@zte.com.cn Reported-by: Zeal Robot Signed-off-by: Minghao Chi Signed-off-by: CGEL ZTE Signed-off-by: Marc Kleine-Budde --- drivers/net/can/softing/softing_main.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/net/can/softing/softing_main.c b/drivers/net/can/softing/softing_main.c index d74e895bddf7..8d27ac66ca7f 100644 --- a/drivers/net/can/softing/softing_main.c +++ b/drivers/net/can/softing/softing_main.c @@ -392,13 +392,10 @@ static int softing_netdev_open(struct net_device *ndev) static int softing_netdev_stop(struct net_device *ndev) { - int ret; - netif_stop_queue(ndev); /* softing cycle does close_candev() */ - ret = softing_startstop(ndev, 0); - return ret; + return softing_startstop(ndev, 0); } static int softing_candev_set_mode(struct net_device *ndev, enum can_mode mode) From 2206fcbc1090bdcf7058d8c9bbc1a147c7359cfa Mon Sep 17 00:00:00 2001 From: Srinivas Neeli Date: Tue, 8 Feb 2022 21:50:53 +0530 Subject: [PATCH 12/36] can: xilinx_can: Add check for NAPI Poll function Add check for NAPI poll function to avoid enabling interrupts with out completing the NAPI call. Link: https://lore.kernel.org/all/20220208162053.39896-1-srinivas.neeli@xilinx.com Signed-off-by: Srinivas Neeli Signed-off-by: Marc Kleine-Budde --- drivers/net/can/xilinx_can.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c index 1674b561c9a2..e562c5ab1149 100644 --- a/drivers/net/can/xilinx_can.c +++ b/drivers/net/can/xilinx_can.c @@ -1215,10 +1215,11 @@ static int xcan_rx_poll(struct napi_struct *napi, int quota) } if (work_done < quota) { - napi_complete_done(napi, work_done); - ier = priv->read_reg(priv, XCAN_IER_OFFSET); - ier |= xcan_rx_int_mask(priv); - priv->write_reg(priv, XCAN_IER_OFFSET, ier); + if (napi_complete_done(napi, work_done)) { + ier = priv->read_reg(priv, XCAN_IER_OFFSET); + ier |= xcan_rx_int_mask(priv); + priv->write_reg(priv, XCAN_IER_OFFSET, ier); + } } return work_done; } From 2ae9856d70b64c3e7eccdb865e7ae77380699bd1 Mon Sep 17 00:00:00 2001 From: Vincent Mailhol Date: Sat, 12 Feb 2022 22:07:37 +0900 Subject: [PATCH 13/36] can: etas_es58x: use BITS_PER_TYPE() instead of manual calculation The input to the GENMASK() macro was calculated by hand. Replaced it with a dedicated macro: BITS_PER_TYPE() which does the exact same job. Link: https://lore.kernel.org/all/20220212130737.3008-1-mailhol.vincent@wanadoo.fr Signed-off-by: Vincent Mailhol Signed-off-by: Marc Kleine-Budde --- drivers/net/can/usb/etas_es58x/es58x_fd.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/can/usb/etas_es58x/es58x_fd.c b/drivers/net/can/usb/etas_es58x/es58x_fd.c index ec87126e1a7d..88d2540abbbe 100644 --- a/drivers/net/can/usb/etas_es58x/es58x_fd.c +++ b/drivers/net/can/usb/etas_es58x/es58x_fd.c @@ -69,7 +69,8 @@ static int es58x_fd_echo_msg(struct net_device *netdev, int i, num_element; u32 rcv_packet_idx; - const u32 mask = GENMASK(31, sizeof(echo_msg->packet_idx) * 8); + const u32 mask = GENMASK(BITS_PER_TYPE(mask) - 1, + BITS_PER_TYPE(echo_msg->packet_idx)); num_element = es58x_msg_num_element(es58x_dev->dev, es58x_fd_urb_cmd->echo_msg, From 3f5c91b4ce8f85cb807cafab126c5cd4c173d745 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Wed, 21 Oct 2020 10:31:29 +0200 Subject: [PATCH 14/36] can: mcp251xfd: mcp251xfd_reg_invalid(): rename from mcp251xfd_osc_invalid() This patch renames mcp251xfd_osc_invalid() to mcp251xfd_reg_invalid(), as it will be used for other registers than the "osc" register in a later patch. This patch also moves this function to more towards the beginning of the file, to be available for other functions, too. Link: https://lore.kernel.org/all/20220207131047.282110-2-mkl@pengutronix.de Signed-off-by: Marc Kleine-Budde --- drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c index b5986df6eca0..7e0c4e662381 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c @@ -178,6 +178,11 @@ static int mcp251xfd_clks_and_vdd_disable(const struct mcp251xfd_priv *priv) return 0; } +static inline bool mcp251xfd_reg_invalid(u32 reg) +{ + return reg == 0x0 || reg == 0xffffffff; +} + static inline int mcp251xfd_chip_get_mode(const struct mcp251xfd_priv *priv, u8 *mode) { @@ -241,11 +246,6 @@ mcp251xfd_chip_set_mode_nowait(const struct mcp251xfd_priv *priv, return __mcp251xfd_chip_set_mode(priv, mode_req, true); } -static inline bool mcp251xfd_osc_invalid(u32 reg) -{ - return reg == 0x0 || reg == 0xffffffff; -} - static int mcp251xfd_chip_clock_enable(const struct mcp251xfd_priv *priv) { u32 osc, osc_reference, osc_mask; @@ -274,7 +274,7 @@ static int mcp251xfd_chip_clock_enable(const struct mcp251xfd_priv *priv) (osc & osc_mask) == osc_reference, MCP251XFD_OSC_STAB_SLEEP_US, MCP251XFD_OSC_STAB_TIMEOUT_US); - if (mcp251xfd_osc_invalid(osc)) { + if (mcp251xfd_reg_invalid(osc)) { netdev_err(priv->ndev, "Failed to detect %s (osc=0x%08x).\n", mcp251xfd_get_model_str(priv), osc); From 25386c9a010069af63df887379ed29dd963f76a1 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Thu, 29 Oct 2020 10:46:51 +0100 Subject: [PATCH 15/36] can: mcp251xfd: mcp251xfd_regmap_crc_read(): ignore CRC error only if solely OSC register is read MCP251XFD_REG_OSC is the first register the driver reads from. The chip may be in deep sleep and the SPI transfer (i.e. the assertion of the CS) will wake the chip up. This takes about 3ms. The CRC of this transfer is wrong, or there isn't any chip at all, in this case the CRC will be wrong, too. The driver ignores the CRC error and returns the read data to the caller. To avoid any confusion, this patch changes the mcp251xfd_regmap_crc_read() function to only ignore the CRC error if solely the OSC register is read. So when reading more than the OSC registers at once, CRC errors are not ignored. Link: https://lore.kernel.org/all/20220207131047.282110-3-mkl@pengutronix.de Signed-off-by: Marc Kleine-Budde --- drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c index 7b120c716228..8112f8df22a6 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c @@ -368,7 +368,7 @@ mcp251xfd_regmap_crc_read(void *context, * to the caller. It will take care of both cases. * */ - if (reg == MCP251XFD_REG_OSC) { + if (reg == MCP251XFD_REG_OSC && val_len == sizeof(__le32)) { err = 0; goto out; } From 72362dcdf654bcb0a7b39b4c372ed21e9316856b Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Fri, 23 Oct 2020 08:41:25 +0200 Subject: [PATCH 16/36] can: mcp251xfd: mcp251xfd_unregister(): simplify runtime PM handling The mcp251xfd driver supports runtime PM enabled kernels, but also works on !CONFIG_PM configurations. This patch simplifies the runtime PM handling in the mcp251xfd_unregister(). In the CONFIG_PM case, runtime PM has been enabled in the mcp251xfd_probe() function, so we can disable it here. For !CONFIG_PM builds call mcp251xfd_clks_and_vdd_disable() directly. Link: https://lore.kernel.org/all/20220207131047.282110-4-mkl@pengutronix.de Signed-off-by: Marc Kleine-Budde --- drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c index 7e0c4e662381..49ce5ff34903 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c @@ -1768,10 +1768,10 @@ static inline void mcp251xfd_unregister(struct mcp251xfd_priv *priv) unregister_candev(ndev); - pm_runtime_get_sync(ndev->dev.parent); - pm_runtime_put_noidle(ndev->dev.parent); - mcp251xfd_clks_and_vdd_disable(priv); - pm_runtime_disable(ndev->dev.parent); + if (pm_runtime_enabled(ndev->dev.parent)) + pm_runtime_disable(ndev->dev.parent); + else + mcp251xfd_clks_and_vdd_disable(priv); } static const struct of_device_id mcp251xfd_of_match[] = { From 1ba3690fa2c63bfa51351674d8f6032bb1ee9619 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Fri, 23 Oct 2020 08:41:25 +0200 Subject: [PATCH 17/36] can: mcp251xfd: mcp251xfd_chip_sleep(): introduce function to bring chip into sleep mode This patch adds a new function to bring the chip into sleep mode, and replaces several occurrences of open coded variants. Link: https://lore.kernel.org/all/20220207131047.282110-5-mkl@pengutronix.de Signed-off-by: Marc Kleine-Budde --- .../net/can/spi/mcp251xfd/mcp251xfd-core.c | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c index 49ce5ff34903..4c8c45f246d8 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c @@ -289,6 +289,11 @@ static int mcp251xfd_chip_clock_enable(const struct mcp251xfd_priv *priv) return err; } +static inline int mcp251xfd_chip_sleep(const struct mcp251xfd_priv *priv) +{ + return mcp251xfd_chip_set_mode(priv, MCP251XFD_REG_CON_MODE_SLEEP); +} + static int mcp251xfd_chip_softreset_do(const struct mcp251xfd_priv *priv) { const __be16 cmd = mcp251xfd_cmd_reset(); @@ -635,7 +640,7 @@ static int mcp251xfd_chip_stop(struct mcp251xfd_priv *priv, mcp251xfd_chip_interrupts_disable(priv); mcp251xfd_chip_rx_int_disable(priv); - return mcp251xfd_chip_set_mode(priv, MCP251XFD_REG_CON_MODE_SLEEP); + return mcp251xfd_chip_sleep(priv); } static int mcp251xfd_chip_start(struct mcp251xfd_priv *priv) @@ -1719,19 +1724,19 @@ static int mcp251xfd_register(struct mcp251xfd_priv *priv) if (err == -ENODEV) goto out_runtime_disable; if (err) - goto out_chip_set_mode_sleep; + goto out_chip_sleep; err = mcp251xfd_register_chip_detect(priv); if (err) - goto out_chip_set_mode_sleep; + goto out_chip_sleep; err = mcp251xfd_register_check_rx_int(priv); if (err) - goto out_chip_set_mode_sleep; + goto out_chip_sleep; err = register_candev(ndev); if (err) - goto out_chip_set_mode_sleep; + goto out_chip_sleep; err = mcp251xfd_register_done(priv); if (err) @@ -1741,7 +1746,7 @@ static int mcp251xfd_register(struct mcp251xfd_priv *priv) * disable the clocks and vdd. If CONFIG_PM is not enabled, * the clocks and vdd will stay powered. */ - err = mcp251xfd_chip_set_mode(priv, MCP251XFD_REG_CON_MODE_SLEEP); + err = mcp251xfd_chip_sleep(priv); if (err) goto out_unregister_candev; @@ -1751,8 +1756,8 @@ static int mcp251xfd_register(struct mcp251xfd_priv *priv) out_unregister_candev: unregister_candev(ndev); - out_chip_set_mode_sleep: - mcp251xfd_chip_set_mode(priv, MCP251XFD_REG_CON_MODE_SLEEP); + out_chip_sleep: + mcp251xfd_chip_sleep(priv); out_runtime_disable: pm_runtime_disable(ndev->dev.parent); out_runtime_put_noidle: From 13c54a1ee12fdd71e76dd9c446fa5b7a330857a9 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Fri, 23 Oct 2020 08:41:25 +0200 Subject: [PATCH 18/36] can: mcp251xfd: mcp251xfd_chip_stop(): convert to a void function The mcp251xfd_chip_stop() function tries the best to stop the chip and put it into sleep mode. It continues, even if some intermediate steps fail. As none of the callers use the return value, let this function return void. Link: https://lore.kernel.org/all/20220207131047.282110-6-mkl@pengutronix.de Signed-off-by: Marc Kleine-Budde --- drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c index 4c8c45f246d8..e8e736eeb69c 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c @@ -633,14 +633,14 @@ static int mcp251xfd_chip_interrupts_disable(const struct mcp251xfd_priv *priv) return regmap_write(priv->map_reg, MCP251XFD_REG_CRC, 0); } -static int mcp251xfd_chip_stop(struct mcp251xfd_priv *priv, - const enum can_state state) +static void mcp251xfd_chip_stop(struct mcp251xfd_priv *priv, + const enum can_state state) { priv->can.state = state; mcp251xfd_chip_interrupts_disable(priv); mcp251xfd_chip_rx_int_disable(priv); - return mcp251xfd_chip_sleep(priv); + mcp251xfd_chip_sleep(priv); } static int mcp251xfd_chip_start(struct mcp251xfd_priv *priv) From 0445e5ff55cce1a0a00922189648d09c8d7afe40 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Tue, 20 Oct 2020 22:23:09 +0200 Subject: [PATCH 19/36] can: mcp251xfd: mcp251xfd_chip_wait_for_osc_ready(): factor out into separate function This patch factors out mcp251xfd_chip_wait_for_osc_ready() into a separate function, it will be used in several places in the next patches. Link: https://lore.kernel.org/all/20220207131047.282110-7-mkl@pengutronix.de Signed-off-by: Marc Kleine-Budde --- .../net/can/spi/mcp251xfd/mcp251xfd-core.c | 45 +++++++++++-------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c index e8e736eeb69c..6222633ee7d9 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c @@ -246,6 +246,32 @@ mcp251xfd_chip_set_mode_nowait(const struct mcp251xfd_priv *priv, return __mcp251xfd_chip_set_mode(priv, mode_req, true); } +static int +mcp251xfd_chip_wait_for_osc_ready(const struct mcp251xfd_priv *priv, + u32 osc_reference, u32 osc_mask) +{ + u32 osc; + int err; + + err = regmap_read_poll_timeout(priv->map_reg, MCP251XFD_REG_OSC, osc, + (osc & osc_mask) == osc_reference, + MCP251XFD_OSC_STAB_SLEEP_US, + MCP251XFD_OSC_STAB_TIMEOUT_US); + if (mcp251xfd_reg_invalid(osc)) { + netdev_err(priv->ndev, + "Failed to detect %s (osc=0x%08x).\n", + mcp251xfd_get_model_str(priv), osc); + return -ENODEV; + } else if (err == -ETIMEDOUT) { + netdev_err(priv->ndev, + "Timeout waiting for Oscillator Ready (osc=0x%08x, osc_reference=0x%08x)\n", + osc, osc_reference); + return -ETIMEDOUT; + } + + return 0; +} + static int mcp251xfd_chip_clock_enable(const struct mcp251xfd_priv *priv) { u32 osc, osc_reference, osc_mask; @@ -269,24 +295,7 @@ static int mcp251xfd_chip_clock_enable(const struct mcp251xfd_priv *priv) if (err) return err; - /* Wait for "Oscillator Ready" bit */ - err = regmap_read_poll_timeout(priv->map_reg, MCP251XFD_REG_OSC, osc, - (osc & osc_mask) == osc_reference, - MCP251XFD_OSC_STAB_SLEEP_US, - MCP251XFD_OSC_STAB_TIMEOUT_US); - if (mcp251xfd_reg_invalid(osc)) { - netdev_err(priv->ndev, - "Failed to detect %s (osc=0x%08x).\n", - mcp251xfd_get_model_str(priv), osc); - return -ENODEV; - } else if (err == -ETIMEDOUT) { - netdev_err(priv->ndev, - "Timeout waiting for Oscillator Ready (osc=0x%08x, osc_reference=0x%08x)\n", - osc, osc_reference); - return -ETIMEDOUT; - } - - return err; + return mcp251xfd_chip_wait_for_osc_ready(priv, osc_reference, osc_mask); } static inline int mcp251xfd_chip_sleep(const struct mcp251xfd_priv *priv) From 197656de8d1e119f594dd1fe1b2d54fd2ae2add9 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Thu, 22 Oct 2020 23:13:42 +0200 Subject: [PATCH 20/36] can: mcp251xfd: mcp251xfd_chip_wait_for_osc_ready(): improve chip detection and error handling The function mcp251xfd_chip_wait_for_osc_ready() polls the Oscillator Control Register for the oscillator to get ready. This is the first register the driver reads from. Reading implausible values (all bits set or unset) can be caused by the chip starting up after power on, waking up after sleep, or by the chip not being preset at all. Add check for implausible register content mcp251xfd_reg_invalid() to the regmap_read_poll_timeout() loop. In case of a regmap_read_poll_timeout() returns a fatal error (and not a timeout), forward it to the caller. As mcp251xfd_chip_wait_for_osc_ready() will be called after the probe function has finished, (currently during ifup), move error message about failed chip detection from there into the probe function. Link: https://lore.kernel.org/all/20220207131047.282110-8-mkl@pengutronix.de Signed-off-by: Marc Kleine-Budde --- .../net/can/spi/mcp251xfd/mcp251xfd-core.c | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c index 6222633ee7d9..68518e8410dd 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c @@ -254,22 +254,25 @@ mcp251xfd_chip_wait_for_osc_ready(const struct mcp251xfd_priv *priv, int err; err = regmap_read_poll_timeout(priv->map_reg, MCP251XFD_REG_OSC, osc, + !mcp251xfd_reg_invalid(osc) && (osc & osc_mask) == osc_reference, MCP251XFD_OSC_STAB_SLEEP_US, MCP251XFD_OSC_STAB_TIMEOUT_US); + if (err != -ETIMEDOUT) + return err; + if (mcp251xfd_reg_invalid(osc)) { netdev_err(priv->ndev, - "Failed to detect %s (osc=0x%08x).\n", - mcp251xfd_get_model_str(priv), osc); + "Failed to read Oscillator Configuration Register (osc=0x%08x).\n", + osc); return -ENODEV; - } else if (err == -ETIMEDOUT) { - netdev_err(priv->ndev, - "Timeout waiting for Oscillator Ready (osc=0x%08x, osc_reference=0x%08x)\n", - osc, osc_reference); - return -ETIMEDOUT; } - return 0; + netdev_err(priv->ndev, + "Timeout waiting for Oscillator Ready (osc=0x%08x, osc_reference=0x%08x)\n", + osc, osc_reference); + + return -ETIMEDOUT; } static int mcp251xfd_chip_clock_enable(const struct mcp251xfd_priv *priv) @@ -1965,8 +1968,11 @@ static int mcp251xfd_probe(struct spi_device *spi) goto out_free_candev; err = mcp251xfd_register(priv); - if (err) + if (err) { + dev_err_probe(&spi->dev, err, "Failed to detect %s.\n", + mcp251xfd_get_model_str(priv)); goto out_can_rx_offload_del; + } return 0; From 06db5dbc8ebfd3c9750349875d236cb51164c414 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Thu, 22 Oct 2020 23:13:42 +0200 Subject: [PATCH 21/36] can: mcp251xfd: mcp251xfd_chip_wait_for_osc_ready(): prepare for PLL support The function mcp251xfd_chip_wait_for_osc_ready() polls the Oscillator Control Register for the oscillator to get ready. By passing the appropriate parameters (osc_reference and osc_mask) it can also poll for PLL ready. This patch adjusts the error message if the Oscillator and/or PLL fail to get ready. Link: https://lore.kernel.org/all/20220207131047.282110-9-mkl@pengutronix.de Signed-off-by: Marc Kleine-Budde --- .../net/can/spi/mcp251xfd/mcp251xfd-core.c | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c index 68518e8410dd..f2aac990d285 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c @@ -112,6 +112,22 @@ static const char *mcp251xfd_get_mode_str(const u8 mode) return ""; } +static const char * +mcp251xfd_get_osc_str(const u32 osc, const u32 osc_reference) +{ + switch (~osc & osc_reference & + (MCP251XFD_REG_OSC_OSCRDY | MCP251XFD_REG_OSC_PLLRDY)) { + case MCP251XFD_REG_OSC_PLLRDY: + return "PLL"; + case MCP251XFD_REG_OSC_OSCRDY: + return "Oscillator"; + case MCP251XFD_REG_OSC_PLLRDY | MCP251XFD_REG_OSC_OSCRDY: + return "Oscillator/PLL"; + } + + return ""; +} + static inline int mcp251xfd_vdd_enable(const struct mcp251xfd_priv *priv) { if (!priv->reg_vdd) @@ -269,8 +285,9 @@ mcp251xfd_chip_wait_for_osc_ready(const struct mcp251xfd_priv *priv, } netdev_err(priv->ndev, - "Timeout waiting for Oscillator Ready (osc=0x%08x, osc_reference=0x%08x)\n", - osc, osc_reference); + "Timeout waiting for %s ready (osc=0x%08x, osc_reference=0x%08x, osc_mask=0x%08x).\n", + mcp251xfd_get_osc_str(osc, osc_reference), + osc, osc_reference, osc_mask); return -ETIMEDOUT; } @@ -298,6 +315,10 @@ static int mcp251xfd_chip_clock_enable(const struct mcp251xfd_priv *priv) if (err) return err; + /* Sometimes the PLL is stuck enabled, the controller never + * sets the OSC Ready bit, and we get an -ETIMEDOUT. Our + * caller takes care of retry. + */ return mcp251xfd_chip_wait_for_osc_ready(priv, osc_reference, osc_mask); } From 01a80d688a411277e5135a010dc47728909bd69e Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Tue, 27 Oct 2020 16:05:19 +0100 Subject: [PATCH 22/36] can: mcp251xfd: mcp251xfd_chip_softreset_check(): wait for OSC ready before accessing chip This patch changes the order of reading the Mode and Oscillator Ready bits. Instead of reading the Mode of the chip directly after reset, first wait for the oscillator to get ready and the chip to fully start up. Read the Mode after this. Link: https://lore.kernel.org/all/20220207131047.282110-10-mkl@pengutronix.de Signed-off-by: Marc Kleine-Budde --- .../net/can/spi/mcp251xfd/mcp251xfd-core.c | 29 ++++++++----------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c index f2aac990d285..9b94272da8bc 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c @@ -349,10 +349,21 @@ static int mcp251xfd_chip_softreset_do(const struct mcp251xfd_priv *priv) static int mcp251xfd_chip_softreset_check(const struct mcp251xfd_priv *priv) { - u32 osc, osc_reference; + u32 osc_reference, osc_mask; u8 mode; int err; + /* Check for reset defaults of OSC reg. + * This will take care of stabilization period. + */ + osc_reference = MCP251XFD_REG_OSC_OSCRDY | + FIELD_PREP(MCP251XFD_REG_OSC_CLKODIV_MASK, + MCP251XFD_REG_OSC_CLKODIV_10); + osc_mask = osc_reference | MCP251XFD_REG_OSC_PLLRDY; + err = mcp251xfd_chip_wait_for_osc_ready(priv, osc_reference, osc_mask); + if (err) + return err; + err = mcp251xfd_chip_get_mode(priv, &mode); if (err) return err; @@ -364,22 +375,6 @@ static int mcp251xfd_chip_softreset_check(const struct mcp251xfd_priv *priv) return -ETIMEDOUT; } - osc_reference = MCP251XFD_REG_OSC_OSCRDY | - FIELD_PREP(MCP251XFD_REG_OSC_CLKODIV_MASK, - MCP251XFD_REG_OSC_CLKODIV_10); - - /* check reset defaults of OSC reg */ - err = regmap_read(priv->map_reg, MCP251XFD_REG_OSC, &osc); - if (err) - return err; - - if (osc != osc_reference) { - netdev_info(priv->ndev, - "Controller failed to reset. osc=0x%08x, reference value=0x%08x.\n", - osc, osc_reference); - return -ETIMEDOUT; - } - return 0; } From 14193ea2bfee7bcd1c0e3aa1cbc4e09e15887847 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Fri, 16 Oct 2020 18:02:15 +0200 Subject: [PATCH 23/36] can: mcp251xfd: mcp251xfd_chip_timestamp_init(): factor out into separate function This patch factors out the timestamp initialization from the clock initialization. This is a preparation patch for the PLL support, where clock and timestamp init must be done separately. Link: https://lore.kernel.org/all/20220207131047.282110-11-mkl@pengutronix.de Signed-off-by: Marc Kleine-Budde --- drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c index 9b94272da8bc..bc10ca1e7384 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c @@ -421,6 +421,11 @@ static int mcp251xfd_chip_clock_init(const struct mcp251xfd_priv *priv) if (err) return err; + return 0; +} + +static int mcp251xfd_chip_timestamp_init(const struct mcp251xfd_priv *priv) +{ /* Set Time Base Counter Prescaler to 1. * * This means an overflow of the 32 bit Time Base Counter @@ -683,6 +688,10 @@ static int mcp251xfd_chip_start(struct mcp251xfd_priv *priv) if (err) goto out_chip_stop; + err = mcp251xfd_chip_timestamp_init(priv); + if (err) + goto out_chip_stop; + err = mcp251xfd_set_bittiming(priv); if (err) goto out_chip_stop; From 1a4abba640113f7a224aeee076bba5dc8a823dd6 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Fri, 16 Oct 2020 20:51:13 +0200 Subject: [PATCH 24/36] can: mcp251xfd: mcp251xfd_chip_wake(): renamed from mcp251xfd_chip_clock_enable() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch renames mcp251xfd_chip_clock_enable() into mcp251xfd_chip_wake() as this function actually wakes the chip. Additionally the documentation is adopted. Link: https://lore.kernel.org/all/20220207131047.282110-12-mkl@pengutronix.de Co-developed-by: Magnus Aagaard Sørensen Signed-off-by: Magnus Aagaard Sørensen Signed-off-by: Marc Kleine-Budde --- .../net/can/spi/mcp251xfd/mcp251xfd-core.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c index bc10ca1e7384..154e6c376670 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c @@ -292,22 +292,23 @@ mcp251xfd_chip_wait_for_osc_ready(const struct mcp251xfd_priv *priv, return -ETIMEDOUT; } -static int mcp251xfd_chip_clock_enable(const struct mcp251xfd_priv *priv) +static int mcp251xfd_chip_wake(const struct mcp251xfd_priv *priv) { u32 osc, osc_reference, osc_mask; int err; - /* Set Power On Defaults for "Clock Output Divisor" and remove - * "Oscillator Disable" bit. + /* For normal sleep on MCP2517FD and MCP2518FD, clearing + * "Oscillator Disable" will wake the chip. For low power mode + * on MCP2518FD, asserting the chip select will wake the + * chip. Writing to the Oscillator register will wake it in + * both cases. */ osc = FIELD_PREP(MCP251XFD_REG_OSC_CLKODIV_MASK, MCP251XFD_REG_OSC_CLKODIV_10); osc_reference = MCP251XFD_REG_OSC_OSCRDY; osc_mask = MCP251XFD_REG_OSC_OSCRDY | MCP251XFD_REG_OSC_PLLRDY; - /* Note: - * - * If the controller is in Sleep Mode the following write only + /* If the controller is in Sleep Mode the following write only * removes the "Oscillator Disable" bit and powers it up. All * other bits are unaffected. */ @@ -332,10 +333,10 @@ static int mcp251xfd_chip_softreset_do(const struct mcp251xfd_priv *priv) const __be16 cmd = mcp251xfd_cmd_reset(); int err; - /* The Set Mode and SPI Reset command only seems to works if - * the controller is not in Sleep Mode. + /* The Set Mode and SPI Reset command only works if the + * controller is not in Sleep Mode. */ - err = mcp251xfd_chip_clock_enable(priv); + err = mcp251xfd_chip_wake(priv); if (err) return err; From a10fd91e42e8be9280927fa40b2b146837abc9c9 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Wed, 21 Oct 2020 10:33:58 +0200 Subject: [PATCH 25/36] can: mcp251xfd: __mcp251xfd_chip_set_mode(): prepare for PLL support: improve error handling and diagnostics This patch prepares the __mcp251xfd_chip_set_mode() function for PLL support by adding more error checks and diagnostics. Link: https://lore.kernel.org/all/20220207131047.282110-13-mkl@pengutronix.de Signed-off-by: Marc Kleine-Budde --- .../net/can/spi/mcp251xfd/mcp251xfd-core.c | 41 ++++++++++++++----- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c index 154e6c376670..d08e0481df35 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c @@ -218,34 +218,55 @@ static int __mcp251xfd_chip_set_mode(const struct mcp251xfd_priv *priv, const u8 mode_req, bool nowait) { - u32 con, con_reqop; + u32 con = 0, con_reqop, osc = 0; + u8 mode; int err; con_reqop = FIELD_PREP(MCP251XFD_REG_CON_REQOP_MASK, mode_req); err = regmap_update_bits(priv->map_reg, MCP251XFD_REG_CON, MCP251XFD_REG_CON_REQOP_MASK, con_reqop); - if (err) + if (err == -EBADMSG) { + netdev_err(priv->ndev, + "Failed to set Requested Operation Mode.\n"); + + return -ENODEV; + } else if (err) { return err; + } if (mode_req == MCP251XFD_REG_CON_MODE_SLEEP || nowait) return 0; err = regmap_read_poll_timeout(priv->map_reg, MCP251XFD_REG_CON, con, + !mcp251xfd_reg_invalid(con) && FIELD_GET(MCP251XFD_REG_CON_OPMOD_MASK, con) == mode_req, MCP251XFD_POLL_SLEEP_US, MCP251XFD_POLL_TIMEOUT_US); - if (err) { - u8 mode = FIELD_GET(MCP251XFD_REG_CON_OPMOD_MASK, con); - - netdev_err(priv->ndev, - "Controller failed to enter mode %s Mode (%u) and stays in %s Mode (%u).\n", - mcp251xfd_get_mode_str(mode_req), mode_req, - mcp251xfd_get_mode_str(mode), mode); + if (err != -ETIMEDOUT && err != -EBADMSG) return err; + + /* Ignore return value. + * Print below error messages, even if this fails. + */ + regmap_read(priv->map_reg, MCP251XFD_REG_OSC, &osc); + + if (mcp251xfd_reg_invalid(con)) { + netdev_err(priv->ndev, + "Failed to read CAN Control Register (con=0x%08x, osc=0x%08x).\n", + con, osc); + + return -ENODEV; } - return 0; + mode = FIELD_GET(MCP251XFD_REG_CON_OPMOD_MASK, con); + netdev_err(priv->ndev, + "Controller failed to enter mode %s Mode (%u) and stays in %s Mode (%u) (con=0x%08x, osc=0x%08x).\n", + mcp251xfd_get_mode_str(mode_req), mode_req, + mcp251xfd_get_mode_str(mode), mode, + con, osc); + + return -ETIMEDOUT; } static inline int From e39ea1360ca7d6b309b42cfe3d8f1fa7830eb894 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Mon, 19 Oct 2020 09:59:28 +0200 Subject: [PATCH 26/36] can: mcp251xfd: mcp251xfd_chip_clock_init(): prepare for PLL support, wait for OSC ready This patch prepares the mcp251xfd_chip_clock_init() function for PLL support. If the PLL is needed is must be switched on after chip reset. This should be done in the mcp251xfd_chip_clock_init() function. Prepare this function to wait for the OSC and PLL to be ready. Link: https://lore.kernel.org/all/20220207131047.282110-14-mkl@pengutronix.de Signed-off-by: Marc Kleine-Budde --- drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c index d08e0481df35..937424e5ac2b 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c @@ -429,7 +429,7 @@ static int mcp251xfd_chip_softreset(const struct mcp251xfd_priv *priv) static int mcp251xfd_chip_clock_init(const struct mcp251xfd_priv *priv) { - u32 osc; + u32 osc, osc_reference, osc_mask; int err; /* Activate Low Power Mode on Oscillator Disable. This only @@ -439,10 +439,17 @@ static int mcp251xfd_chip_clock_init(const struct mcp251xfd_priv *priv) osc = MCP251XFD_REG_OSC_LPMEN | FIELD_PREP(MCP251XFD_REG_OSC_CLKODIV_MASK, MCP251XFD_REG_OSC_CLKODIV_10); + osc_reference = MCP251XFD_REG_OSC_OSCRDY; + osc_mask = MCP251XFD_REG_OSC_OSCRDY | MCP251XFD_REG_OSC_PLLRDY; + err = regmap_write(priv->map_reg, MCP251XFD_REG_OSC, osc); if (err) return err; + err = mcp251xfd_chip_wait_for_osc_ready(priv, osc_reference, osc_mask); + if (err) + return err; + return 0; } From 445dd72a6d63b7ebdf35c8a7eeb746d88f61ec27 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Mon, 19 Oct 2020 09:59:28 +0200 Subject: [PATCH 27/36] can: mcp251xfd: mcp251xfd_register(): prepare to activate PLL after softreset If the PLL is needed it must be switched on after chip reset. This patch adds the required call to mcp251xfd_register(). Link: https://lore.kernel.org/all/20220207131047.282110-15-mkl@pengutronix.de Signed-off-by: Marc Kleine-Budde --- drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c index 937424e5ac2b..1086c8974f89 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c @@ -1792,6 +1792,12 @@ static int mcp251xfd_register(struct mcp251xfd_priv *priv) if (err) goto out_chip_sleep; + err = mcp251xfd_chip_clock_init(priv); + if (err == -ENODEV) + goto out_runtime_disable; + if (err) + goto out_chip_sleep; + err = mcp251xfd_register_chip_detect(priv); if (err) goto out_chip_sleep; From 2a68dd8663ea5d2f32e093e9c375d103d8434bff Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Fri, 16 Oct 2020 22:36:39 +0200 Subject: [PATCH 28/36] can: mcp251xfd: add support for internal PLL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PLL is enabled if the configured clock is less than or equal to 10 times the max clock frequency. The device will operate with two different SPI speeds. A slow speed determined by the clock without the PLL enabled, and a fast speed derived from the frequency with the PLL enabled. Link: https://lore.kernel.org/all/20220207131047.282110-16-mkl@pengutronix.de Link: https://lore.kernel.org/all/20201015124401.2766-3-mas@csselectronics.com Co-developed-by: Magnus Aagaard Sørensen Signed-off-by: Magnus Aagaard Sørensen Signed-off-by: Marc Kleine-Budde --- .../net/can/spi/mcp251xfd/mcp251xfd-core.c | 88 +++++++++++++++---- drivers/net/can/spi/mcp251xfd/mcp251xfd.h | 7 +- 2 files changed, 75 insertions(+), 20 deletions(-) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c index 1086c8974f89..e5641696cdc2 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c @@ -326,8 +326,13 @@ static int mcp251xfd_chip_wake(const struct mcp251xfd_priv *priv) */ osc = FIELD_PREP(MCP251XFD_REG_OSC_CLKODIV_MASK, MCP251XFD_REG_OSC_CLKODIV_10); + + /* We cannot check for the PLL ready bit (either set or + * unset), as the PLL might be enabled. This can happen if the + * system reboots, while the mcp251xfd stays powered. + */ osc_reference = MCP251XFD_REG_OSC_OSCRDY; - osc_mask = MCP251XFD_REG_OSC_OSCRDY | MCP251XFD_REG_OSC_PLLRDY; + osc_mask = MCP251XFD_REG_OSC_OSCRDY; /* If the controller is in Sleep Mode the following write only * removes the "Oscillator Disable" bit and powers it up. All @@ -346,6 +351,21 @@ static int mcp251xfd_chip_wake(const struct mcp251xfd_priv *priv) static inline int mcp251xfd_chip_sleep(const struct mcp251xfd_priv *priv) { + if (priv->pll_enable) { + u32 osc; + int err; + + /* Turn off PLL */ + osc = FIELD_PREP(MCP251XFD_REG_OSC_CLKODIV_MASK, + MCP251XFD_REG_OSC_CLKODIV_10); + err = regmap_write(priv->map_reg, MCP251XFD_REG_OSC, osc); + if (err) + netdev_err(priv->ndev, + "Failed to disable PLL.\n"); + + priv->spi->max_speed_hz = priv->spi_max_speed_hz_slow; + } + return mcp251xfd_chip_set_mode(priv, MCP251XFD_REG_CON_MODE_SLEEP); } @@ -442,6 +462,11 @@ static int mcp251xfd_chip_clock_init(const struct mcp251xfd_priv *priv) osc_reference = MCP251XFD_REG_OSC_OSCRDY; osc_mask = MCP251XFD_REG_OSC_OSCRDY | MCP251XFD_REG_OSC_PLLRDY; + if (priv->pll_enable) { + osc |= MCP251XFD_REG_OSC_PLLEN; + osc_reference |= MCP251XFD_REG_OSC_PLLRDY; + } + err = regmap_write(priv->map_reg, MCP251XFD_REG_OSC, osc); if (err) return err; @@ -450,6 +475,8 @@ static int mcp251xfd_chip_clock_init(const struct mcp251xfd_priv *priv) if (err) return err; + priv->spi->max_speed_hz = priv->spi_max_speed_hz_fast; + return 0; } @@ -1692,8 +1719,9 @@ static int mcp251xfd_register_check_rx_int(struct mcp251xfd_priv *priv) } static int -mcp251xfd_register_get_dev_id(const struct mcp251xfd_priv *priv, - u32 *dev_id, u32 *effective_speed_hz) +mcp251xfd_register_get_dev_id(const struct mcp251xfd_priv *priv, u32 *dev_id, + u32 *effective_speed_hz_slow, + u32 *effective_speed_hz_fast) { struct mcp251xfd_map_buf_nocrc *buf_rx; struct mcp251xfd_map_buf_nocrc *buf_tx; @@ -1712,16 +1740,20 @@ mcp251xfd_register_get_dev_id(const struct mcp251xfd_priv *priv, xfer[0].tx_buf = buf_tx; xfer[0].len = sizeof(buf_tx->cmd); + xfer[0].speed_hz = priv->spi_max_speed_hz_slow; xfer[1].rx_buf = buf_rx->data; xfer[1].len = sizeof(dev_id); + xfer[1].speed_hz = priv->spi_max_speed_hz_fast; mcp251xfd_spi_cmd_read_nocrc(&buf_tx->cmd, MCP251XFD_REG_DEVID); + err = spi_sync_transfer(priv->spi, xfer, ARRAY_SIZE(xfer)); if (err) goto out_kfree_buf_tx; *dev_id = be32_to_cpup((__be32 *)buf_rx->data); - *effective_speed_hz = xfer->effective_speed_hz; + *effective_speed_hz_slow = xfer[0].effective_speed_hz; + *effective_speed_hz_fast = xfer[1].effective_speed_hz; out_kfree_buf_tx: kfree(buf_tx); @@ -1737,34 +1769,45 @@ mcp251xfd_register_get_dev_id(const struct mcp251xfd_priv *priv, static int mcp251xfd_register_done(const struct mcp251xfd_priv *priv) { - u32 dev_id, effective_speed_hz; + u32 dev_id, effective_speed_hz_slow, effective_speed_hz_fast; + unsigned long clk_rate; int err; err = mcp251xfd_register_get_dev_id(priv, &dev_id, - &effective_speed_hz); + &effective_speed_hz_slow, + &effective_speed_hz_fast); if (err) return err; + clk_rate = clk_get_rate(priv->clk); + netdev_info(priv->ndev, - "%s rev%lu.%lu (%cRX_INT %cMAB_NO_WARN %cCRC_REG %cCRC_RX %cCRC_TX %cECC %cHD c:%u.%02uMHz m:%u.%02uMHz r:%u.%02uMHz e:%u.%02uMHz) successfully initialized.\n", + "%s rev%lu.%lu (%cRX_INT %cPLL %cMAB_NO_WARN %cCRC_REG %cCRC_RX %cCRC_TX %cECC %cHD o:%lu.%02luMHz c:%u.%02uMHz m:%u.%02uMHz rs:%u.%02uMHz es:%u.%02uMHz rf:%u.%02uMHz ef:%u.%02uMHz) successfully initialized.\n", mcp251xfd_get_model_str(priv), FIELD_GET(MCP251XFD_REG_DEVID_ID_MASK, dev_id), FIELD_GET(MCP251XFD_REG_DEVID_REV_MASK, dev_id), priv->rx_int ? '+' : '-', + priv->pll_enable ? '+' : '-', MCP251XFD_QUIRK_ACTIVE(MAB_NO_WARN), MCP251XFD_QUIRK_ACTIVE(CRC_REG), MCP251XFD_QUIRK_ACTIVE(CRC_RX), MCP251XFD_QUIRK_ACTIVE(CRC_TX), MCP251XFD_QUIRK_ACTIVE(ECC), MCP251XFD_QUIRK_ACTIVE(HALF_DUPLEX), + clk_rate / 1000000, + clk_rate % 1000000 / 1000 / 10, priv->can.clock.freq / 1000000, priv->can.clock.freq % 1000000 / 1000 / 10, priv->spi_max_speed_hz_orig / 1000000, priv->spi_max_speed_hz_orig % 1000000 / 1000 / 10, - priv->spi->max_speed_hz / 1000000, - priv->spi->max_speed_hz % 1000000 / 1000 / 10, - effective_speed_hz / 1000000, - effective_speed_hz % 1000000 / 1000 / 10); + priv->spi_max_speed_hz_slow / 1000000, + priv->spi_max_speed_hz_slow % 1000000 / 1000 / 10, + effective_speed_hz_slow / 1000000, + effective_speed_hz_slow % 1000000 / 1000 / 10, + priv->spi_max_speed_hz_fast / 1000000, + priv->spi_max_speed_hz_fast % 1000000 / 1000 / 10, + effective_speed_hz_fast / 1000000, + effective_speed_hz_fast % 1000000 / 1000 / 10); return 0; } @@ -1891,6 +1934,7 @@ static int mcp251xfd_probe(struct spi_device *spi) struct gpio_desc *rx_int; struct regulator *reg_vdd, *reg_xceiver; struct clk *clk; + bool pll_enable = false; u32 freq = 0; int err; @@ -1941,12 +1985,8 @@ static int mcp251xfd_probe(struct spi_device *spi) return -ERANGE; } - if (freq <= MCP251XFD_SYSCLOCK_HZ_MAX / MCP251XFD_OSC_PLL_MULTIPLIER) { - dev_err(&spi->dev, - "Oscillator frequency (%u Hz) is too low and PLL is not supported.\n", - freq); - return -ERANGE; - } + if (freq <= MCP251XFD_SYSCLOCK_HZ_MAX / MCP251XFD_OSC_PLL_MULTIPLIER) + pll_enable = true; ndev = alloc_candev(sizeof(struct mcp251xfd_priv), MCP251XFD_TX_OBJ_NUM_MAX); @@ -1962,6 +2002,8 @@ static int mcp251xfd_probe(struct spi_device *spi) priv = netdev_priv(ndev); spi_set_drvdata(spi, priv); priv->can.clock.freq = freq; + if (pll_enable) + priv->can.clock.freq *= MCP251XFD_OSC_PLL_MULTIPLIER; priv->can.do_set_mode = mcp251xfd_set_mode; priv->can.do_get_berr_counter = mcp251xfd_get_berr_counter; priv->can.bittiming_const = &mcp251xfd_bittiming_const; @@ -1974,6 +2016,7 @@ static int mcp251xfd_probe(struct spi_device *spi) priv->spi = spi; priv->rx_int = rx_int; priv->clk = clk; + priv->pll_enable = pll_enable; priv->reg_vdd = reg_vdd; priv->reg_xceiver = reg_xceiver; @@ -2011,7 +2054,16 @@ static int mcp251xfd_probe(struct spi_device *spi) * */ priv->spi_max_speed_hz_orig = spi->max_speed_hz; - spi->max_speed_hz = min(spi->max_speed_hz, freq / 2 / 1000 * 850); + priv->spi_max_speed_hz_slow = min(spi->max_speed_hz, + freq / 2 / 1000 * 850); + if (priv->pll_enable) + priv->spi_max_speed_hz_fast = min(spi->max_speed_hz, + freq * + MCP251XFD_OSC_PLL_MULTIPLIER / + 2 / 1000 * 850); + else + priv->spi_max_speed_hz_fast = priv->spi_max_speed_hz_slow; + spi->max_speed_hz = priv->spi_max_speed_hz_slow; spi->bits_per_word = 8; spi->rt = true; err = spi_setup(spi); diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h index f551c900803e..ded927b4873d 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h @@ -2,8 +2,8 @@ * * mcp251xfd - Microchip MCP251xFD Family CAN controller driver * - * Copyright (c) 2019 Pengutronix, - * Marc Kleine-Budde + * Copyright (c) 2019, 2020 Pengutronix, + * Marc Kleine-Budde * Copyright (c) 2019 Martin Sperl */ @@ -592,6 +592,8 @@ struct mcp251xfd_priv { struct spi_device *spi; u32 spi_max_speed_hz_orig; + u32 spi_max_speed_hz_fast; + u32 spi_max_speed_hz_slow; struct mcp251xfd_tef_ring tef[1]; struct mcp251xfd_tx_ring tx[1]; @@ -608,6 +610,7 @@ struct mcp251xfd_priv { struct gpio_desc *rx_int; struct clk *clk; + bool pll_enable; struct regulator *reg_vdd; struct regulator *reg_xceiver; From c912f19ee3820b1f93aa289c7bbe1bd132ccba9a Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Fri, 21 May 2021 19:51:32 +0200 Subject: [PATCH 29/36] can: mcp251xfd: introduce struct mcp251xfd_tx_ring::nr and ::fifo_nr and make use of it This patch removes the hard coded assumption that the TX ring uses hardware FIFO 1. This allows the hardware FIFO 1 to be used for RX and the next free FIFO for TX. This gives the opportunity to minimize the number of SPI transfers in the IRQ handler. The read of the IRQ status register and RX FIFO status registers can be combined into single SPI transfer. If the RX ring uses FIFO 1, the overall length of the transfer is smaller than in the original layout, where the RX FIFO comes after the TX FIFO. Link: https://lore.kernel.org/all/20220217103826.2299157-2-mkl@pengutronix.de Signed-off-by: Marc Kleine-Budde --- .../can/spi/mcp251xfd/mcp251xfd-chip-fifo.c | 4 ++-- .../net/can/spi/mcp251xfd/mcp251xfd-dump.c | 4 ++-- .../net/can/spi/mcp251xfd/mcp251xfd-regmap.c | 22 ++++++++++++++----- .../net/can/spi/mcp251xfd/mcp251xfd-ring.c | 4 +++- drivers/net/can/spi/mcp251xfd/mcp251xfd.h | 4 +++- 5 files changed, 26 insertions(+), 12 deletions(-) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-chip-fifo.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-chip-fifo.c index 2f9a623d381d..0d96097a2547 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-chip-fifo.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-chip-fifo.c @@ -78,7 +78,7 @@ int mcp251xfd_chip_fifo_init(const struct mcp251xfd_priv *priv) if (err) return err; - /* FIFO 1 - TX */ + /* TX FIFO */ val = FIELD_PREP(MCP251XFD_REG_FIFOCON_FSIZE_MASK, tx_ring->obj_num - 1) | MCP251XFD_REG_FIFOCON_TXEN | @@ -99,7 +99,7 @@ int mcp251xfd_chip_fifo_init(const struct mcp251xfd_priv *priv) MCP251XFD_REG_FIFOCON_TXAT_UNLIMITED); err = regmap_write(priv->map_reg, - MCP251XFD_REG_FIFOCON(MCP251XFD_TX_FIFO), + MCP251XFD_REG_FIFOCON(priv->tx->fifo_nr), val); if (err) return err; diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-dump.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-dump.c index ffae8fdd3af0..c991b30bc9f0 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-dump.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-dump.c @@ -207,10 +207,10 @@ static void mcp251xfd_dump_tx_ring(const struct mcp251xfd_priv *priv, .val = tx->base, }, { .key = MCP251XFD_DUMP_OBJECT_RING_KEY_NR, - .val = 0, + .val = tx->nr, }, { .key = MCP251XFD_DUMP_OBJECT_RING_KEY_FIFO_NR, - .val = MCP251XFD_TX_FIFO, + .val = tx->fifo_nr, }, { .key = MCP251XFD_DUMP_OBJECT_RING_KEY_OBJ_NUM, .val = tx->obj_num, diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c index 8112f8df22a6..217510c12af5 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-regmap.c @@ -2,8 +2,8 @@ // // mcp251xfd - Microchip MCP251xFD Family CAN controller driver // -// Copyright (c) 2019, 2020 Pengutronix, -// Marc Kleine-Budde +// Copyright (c) 2019, 2020, 2021 Pengutronix, +// Marc Kleine-Budde // #include "mcp251xfd.h" @@ -47,22 +47,32 @@ mcp251xfd_regmap_nocrc_gather_write(void *context, return spi_sync_transfer(spi, xfer, ARRAY_SIZE(xfer)); } -static inline bool mcp251xfd_update_bits_read_reg(unsigned int reg) +static inline bool +mcp251xfd_update_bits_read_reg(const struct mcp251xfd_priv *priv, + unsigned int reg) { + struct mcp251xfd_rx_ring *ring; + int n; + switch (reg) { case MCP251XFD_REG_INT: case MCP251XFD_REG_TEFCON: - case MCP251XFD_REG_FIFOCON(MCP251XFD_RX_FIFO(0)): case MCP251XFD_REG_FLTCON(0): case MCP251XFD_REG_ECCSTAT: case MCP251XFD_REG_CRC: return false; case MCP251XFD_REG_CON: - case MCP251XFD_REG_FIFOSTA(MCP251XFD_RX_FIFO(0)): case MCP251XFD_REG_OSC: case MCP251XFD_REG_ECCCON: return true; default: + mcp251xfd_for_each_rx_ring(priv, ring, n) { + if (reg == MCP251XFD_REG_FIFOCON(ring->fifo_nr)) + return false; + if (reg == MCP251XFD_REG_FIFOSTA(ring->fifo_nr)) + return true; + } + WARN(1, "Status of reg 0x%04x unknown.\n", reg); } @@ -92,7 +102,7 @@ mcp251xfd_regmap_nocrc_update_bits(void *context, unsigned int reg, last_byte = mcp251xfd_last_byte_set(mask); len = last_byte - first_byte + 1; - if (mcp251xfd_update_bits_read_reg(reg)) { + if (mcp251xfd_update_bits_read_reg(priv, reg)) { struct spi_transfer xfer[2] = { }; struct spi_message msg; diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c index 92f9e9b01289..6e394ac0bc5e 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c @@ -136,9 +136,11 @@ void mcp251xfd_ring_init(struct mcp251xfd_priv *priv) tx_ring->head = 0; tx_ring->tail = 0; tx_ring->base = mcp251xfd_get_tef_obj_addr(tx_ring->obj_num); + tx_ring->nr = 0; + tx_ring->fifo_nr = MCP251XFD_TX_FIFO; /* FIFO request to send */ - addr = MCP251XFD_REG_FIFOCON(MCP251XFD_TX_FIFO); + addr = MCP251XFD_REG_FIFOCON(tx_ring->fifo_nr); val = MCP251XFD_REG_FIFOCON_TXREQ | MCP251XFD_REG_FIFOCON_UINC; len = mcp251xfd_cmd_prepare_write_reg(priv, &tx_ring->rts_buf, addr, val, val); diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h index ded927b4873d..12d3f2b84c0a 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h @@ -521,6 +521,8 @@ struct mcp251xfd_tx_ring { unsigned int tail; u16 base; + u8 nr; + u8 fifo_nr; u8 obj_num; u8 obj_size; @@ -779,7 +781,7 @@ mcp251xfd_tx_tail_get_from_chip(const struct mcp251xfd_priv *priv, int err; err = regmap_read(priv->map_reg, - MCP251XFD_REG_FIFOSTA(MCP251XFD_TX_FIFO), + MCP251XFD_REG_FIFOSTA(priv->tx->fifo_nr), &fifo_sta); if (err) return err; From d2d5397fcae17d75f84ba6fab2a52795445a2388 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Tue, 3 Aug 2021 16:17:57 +0200 Subject: [PATCH 30/36] can: mcp251xfd: mcp251xfd_ring_init(): split ring_init into separate functions This patch splits the initialization of the TEF, TX and RX FIFO in the mcp251xfd_ring_init() function into separate functions. This is a preparation patch to move the RX FIFO in front of the TX FIFO. Link: https://lore.kernel.org/all/20220217103826.2299157-3-mkl@pengutronix.de Signed-off-by: Marc Kleine-Budde --- .../net/can/spi/mcp251xfd/mcp251xfd-ring.c | 100 +++++++++++------- 1 file changed, 62 insertions(+), 38 deletions(-) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c index 6e394ac0bc5e..b9300554543e 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c @@ -52,6 +52,45 @@ mcp251xfd_cmd_prepare_write_reg(const struct mcp251xfd_priv *priv, return len; } +static void mcp251xfd_ring_init_tef(struct mcp251xfd_priv *priv) +{ + struct mcp251xfd_tef_ring *tef_ring; + struct spi_transfer *xfer; + u32 val; + u16 addr; + u8 len; + int i; + + /* TEF */ + tef_ring = priv->tef; + tef_ring->head = 0; + tef_ring->tail = 0; + + /* FIFO increment TEF tail pointer */ + addr = MCP251XFD_REG_TEFCON; + val = MCP251XFD_REG_TEFCON_UINC; + len = mcp251xfd_cmd_prepare_write_reg(priv, &tef_ring->uinc_buf, + addr, val, val); + + for (i = 0; i < ARRAY_SIZE(tef_ring->uinc_xfer); i++) { + xfer = &tef_ring->uinc_xfer[i]; + xfer->tx_buf = &tef_ring->uinc_buf; + xfer->len = len; + xfer->cs_change = 1; + xfer->cs_change_delay.value = 0; + xfer->cs_change_delay.unit = SPI_DELAY_UNIT_NSECS; + } + + /* "cs_change == 1" on the last transfer results in an active + * chip select after the complete SPI message. This causes the + * controller to interpret the next register access as + * data. Set "cs_change" of the last transfer to "0" to + * properly deactivate the chip select at the end of the + * message. + */ + xfer->cs_change = 0; +} + static void mcp251xfd_tx_ring_init_tx_obj(const struct mcp251xfd_priv *priv, const struct mcp251xfd_tx_ring *ring, @@ -88,50 +127,15 @@ mcp251xfd_tx_ring_init_tx_obj(const struct mcp251xfd_priv *priv, ARRAY_SIZE(tx_obj->xfer)); } -void mcp251xfd_ring_init(struct mcp251xfd_priv *priv) +static void mcp251xfd_ring_init_tx(struct mcp251xfd_priv *priv) { - struct mcp251xfd_tef_ring *tef_ring; struct mcp251xfd_tx_ring *tx_ring; - struct mcp251xfd_rx_ring *rx_ring, *prev_rx_ring = NULL; struct mcp251xfd_tx_obj *tx_obj; - struct spi_transfer *xfer; u32 val; u16 addr; u8 len; - int i, j; + int i; - netdev_reset_queue(priv->ndev); - - /* TEF */ - tef_ring = priv->tef; - tef_ring->head = 0; - tef_ring->tail = 0; - - /* FIFO increment TEF tail pointer */ - addr = MCP251XFD_REG_TEFCON; - val = MCP251XFD_REG_TEFCON_UINC; - len = mcp251xfd_cmd_prepare_write_reg(priv, &tef_ring->uinc_buf, - addr, val, val); - - for (j = 0; j < ARRAY_SIZE(tef_ring->uinc_xfer); j++) { - xfer = &tef_ring->uinc_xfer[j]; - xfer->tx_buf = &tef_ring->uinc_buf; - xfer->len = len; - xfer->cs_change = 1; - xfer->cs_change_delay.value = 0; - xfer->cs_change_delay.unit = SPI_DELAY_UNIT_NSECS; - } - - /* "cs_change == 1" on the last transfer results in an active - * chip select after the complete SPI message. This causes the - * controller to interpret the next register access as - * data. Set "cs_change" of the last transfer to "0" to - * properly deactivate the chip select at the end of the - * message. - */ - xfer->cs_change = 0; - - /* TX */ tx_ring = priv->tx; tx_ring->head = 0; tx_ring->tail = 0; @@ -147,8 +151,19 @@ void mcp251xfd_ring_init(struct mcp251xfd_priv *priv) mcp251xfd_for_each_tx_obj(tx_ring, tx_obj, i) mcp251xfd_tx_ring_init_tx_obj(priv, tx_ring, tx_obj, len, i); +} - /* RX */ +static void mcp251xfd_ring_init_rx(struct mcp251xfd_priv *priv) +{ + struct mcp251xfd_rx_ring *rx_ring, *prev_rx_ring = NULL; + struct mcp251xfd_tx_ring *tx_ring; + struct spi_transfer *xfer; + u32 val; + u16 addr; + u8 len; + int i, j; + + tx_ring = priv->tx; mcp251xfd_for_each_rx_ring(priv, rx_ring, i) { rx_ring->head = 0; rx_ring->tail = 0; @@ -192,6 +207,15 @@ void mcp251xfd_ring_init(struct mcp251xfd_priv *priv) } } +void mcp251xfd_ring_init(struct mcp251xfd_priv *priv) +{ + netdev_reset_queue(priv->ndev); + + mcp251xfd_ring_init_tef(priv); + mcp251xfd_ring_init_tx(priv); + mcp251xfd_ring_init_rx(priv); +} + void mcp251xfd_ring_free(struct mcp251xfd_priv *priv) { int i; From 617283b9c4dbc1384b063f59c840e1d1ab612108 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Tue, 3 Aug 2021 16:32:01 +0200 Subject: [PATCH 31/36] can: mcp251xfd: ring: prepare to change order of TX and RX FIFOs This patch improves the initialization of the TX and RX rings. The initialization functions are now called with pointers to the next free address (in the on chip RAM) and next free hardware FIFO. The rings are initialized using these values and the pointers are modified to point to the next free elements. This means the order of the mcp251xfd_ring_init_*() functions specifies the order of the rings in the hardware FIFO. This makes it possible to change the order of the TX and RX FIFOs, which is done in the next patch. This gives the opportunity to minimize the number of SPI transfers in the IRQ handler. The read of the IRQ status register and RX FIFO status registers can be combined into single SPI transfer. If the RX ring uses FIFO 1, the overall length of the transfer is smaller than in the original layout, where the RX FIFO comes after the TX FIFO. Link: https://lore.kernel.org/all/20220217103826.2299157-4-mkl@pengutronix.de Signed-off-by: Marc Kleine-Budde --- .../net/can/spi/mcp251xfd/mcp251xfd-ring.c | 47 ++++++++++--------- drivers/net/can/spi/mcp251xfd/mcp251xfd.h | 2 - 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c index b9300554543e..39005725c665 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c @@ -52,7 +52,8 @@ mcp251xfd_cmd_prepare_write_reg(const struct mcp251xfd_priv *priv, return len; } -static void mcp251xfd_ring_init_tef(struct mcp251xfd_priv *priv) +static void +mcp251xfd_ring_init_tef(struct mcp251xfd_priv *priv, u16 *base) { struct mcp251xfd_tef_ring *tef_ring; struct spi_transfer *xfer; @@ -66,6 +67,9 @@ static void mcp251xfd_ring_init_tef(struct mcp251xfd_priv *priv) tef_ring->head = 0; tef_ring->tail = 0; + /* TEF- and TX-FIFO have same number of objects */ + *base = mcp251xfd_get_tef_obj_addr(priv->tx->obj_num); + /* FIFO increment TEF tail pointer */ addr = MCP251XFD_REG_TEFCON; val = MCP251XFD_REG_TEFCON_UINC; @@ -127,7 +131,8 @@ mcp251xfd_tx_ring_init_tx_obj(const struct mcp251xfd_priv *priv, ARRAY_SIZE(tx_obj->xfer)); } -static void mcp251xfd_ring_init_tx(struct mcp251xfd_priv *priv) +static void +mcp251xfd_ring_init_tx(struct mcp251xfd_priv *priv, u16 *base, u8 *fifo_nr) { struct mcp251xfd_tx_ring *tx_ring; struct mcp251xfd_tx_obj *tx_obj; @@ -139,9 +144,12 @@ static void mcp251xfd_ring_init_tx(struct mcp251xfd_priv *priv) tx_ring = priv->tx; tx_ring->head = 0; tx_ring->tail = 0; - tx_ring->base = mcp251xfd_get_tef_obj_addr(tx_ring->obj_num); + tx_ring->base = *base; tx_ring->nr = 0; - tx_ring->fifo_nr = MCP251XFD_TX_FIFO; + tx_ring->fifo_nr = *fifo_nr; + + *base = mcp251xfd_get_tx_obj_addr(tx_ring, tx_ring->obj_num); + *fifo_nr += 1; /* FIFO request to send */ addr = MCP251XFD_REG_FIFOCON(tx_ring->fifo_nr); @@ -153,33 +161,25 @@ static void mcp251xfd_ring_init_tx(struct mcp251xfd_priv *priv) mcp251xfd_tx_ring_init_tx_obj(priv, tx_ring, tx_obj, len, i); } -static void mcp251xfd_ring_init_rx(struct mcp251xfd_priv *priv) +static void +mcp251xfd_ring_init_rx(struct mcp251xfd_priv *priv, u16 *base, u8 *fifo_nr) { - struct mcp251xfd_rx_ring *rx_ring, *prev_rx_ring = NULL; - struct mcp251xfd_tx_ring *tx_ring; + struct mcp251xfd_rx_ring *rx_ring; struct spi_transfer *xfer; u32 val; u16 addr; u8 len; int i, j; - tx_ring = priv->tx; mcp251xfd_for_each_rx_ring(priv, rx_ring, i) { rx_ring->head = 0; rx_ring->tail = 0; + rx_ring->base = *base; rx_ring->nr = i; - rx_ring->fifo_nr = MCP251XFD_RX_FIFO(i); + rx_ring->fifo_nr = *fifo_nr; - if (!prev_rx_ring) - rx_ring->base = - mcp251xfd_get_tx_obj_addr(tx_ring, - tx_ring->obj_num); - else - rx_ring->base = prev_rx_ring->base + - prev_rx_ring->obj_size * - prev_rx_ring->obj_num; - - prev_rx_ring = rx_ring; + *base = mcp251xfd_get_rx_obj_addr(rx_ring, rx_ring->obj_num); + *fifo_nr += 1; /* FIFO increment RX tail pointer */ addr = MCP251XFD_REG_FIFOCON(rx_ring->fifo_nr); @@ -209,11 +209,14 @@ static void mcp251xfd_ring_init_rx(struct mcp251xfd_priv *priv) void mcp251xfd_ring_init(struct mcp251xfd_priv *priv) { + u16 base = 0; + u8 fifo_nr = 1; + netdev_reset_queue(priv->ndev); - mcp251xfd_ring_init_tef(priv); - mcp251xfd_ring_init_tx(priv); - mcp251xfd_ring_init_rx(priv); + mcp251xfd_ring_init_tef(priv, &base); + mcp251xfd_ring_init_tx(priv, &base, &fifo_nr); + mcp251xfd_ring_init_rx(priv, &base, &fifo_nr); } void mcp251xfd_ring_free(struct mcp251xfd_priv *priv) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h index 12d3f2b84c0a..5c3f7f25caf0 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h @@ -383,8 +383,6 @@ #endif #define MCP251XFD_NAPI_WEIGHT 32 -#define MCP251XFD_TX_FIFO 1 -#define MCP251XFD_RX_FIFO(x) (MCP251XFD_TX_FIFO + 1 + (x)) /* SPI commands */ #define MCP251XFD_SPI_INSTRUCTION_RESET 0x0000 From 62713f0d9a38f2e0b772e6448adfa9c405316237 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Tue, 3 Aug 2021 16:33:48 +0200 Subject: [PATCH 32/36] can: mcp251xfd: ring: change order of TX and RX FIFOs This patch actually changes the order of the TX and RX FIFOs. This gives the opportunity to minimize the number of SPI transfers in the IRQ handler. The read of the IRQ status register and RX FIFO status registers can be combined into single SPI transfer. If the RX ring uses FIFO 1, the overall length of the transfer is smaller than in the original layout, where the RX FIFO comes after the TX FIFO. Link: https://lore.kernel.org/all/20220217103826.2299157-5-mkl@pengutronix.de Signed-off-by: Marc Kleine-Budde --- drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c | 2 +- drivers/net/can/spi/mcp251xfd/mcp251xfd.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c index 39005725c665..9657dbf251b0 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c @@ -215,8 +215,8 @@ void mcp251xfd_ring_init(struct mcp251xfd_priv *priv) netdev_reset_queue(priv->ndev); mcp251xfd_ring_init_tef(priv, &base); - mcp251xfd_ring_init_tx(priv, &base, &fifo_nr); mcp251xfd_ring_init_rx(priv, &base, &fifo_nr); + mcp251xfd_ring_init_tx(priv, &base, &fifo_nr); } void mcp251xfd_ring_free(struct mcp251xfd_priv *priv) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h index 5c3f7f25caf0..8ee959890aea 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h @@ -596,8 +596,8 @@ struct mcp251xfd_priv { u32 spi_max_speed_hz_slow; struct mcp251xfd_tef_ring tef[1]; - struct mcp251xfd_tx_ring tx[1]; struct mcp251xfd_rx_ring *rx[1]; + struct mcp251xfd_tx_ring tx[1]; u8 rx_ring_num; From fa0b68df7c95043685f339cf27d2539c001a21fd Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Tue, 3 Aug 2021 16:32:01 +0200 Subject: [PATCH 33/36] can: mcp251xfd: ring: mcp251xfd_ring_init(): checked RAM usage of ring setup With this patch the usage of the on-chip RAM is checked. In the current driver the FIFO setup is fixed and always fits into the RAM. With an upcoming patch series the ring and FIFO setup will be more dynamic. Although using more RAM than available should not happen, but add this safety check, just in case. Link: https://lore.kernel.org/all/20220217103826.2299157-6-mkl@pengutronix.de Signed-off-by: Marc Kleine-Budde --- drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c | 4 +++- drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c | 14 ++++++++++++-- drivers/net/can/spi/mcp251xfd/mcp251xfd.h | 2 +- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c index e5641696cdc2..2d033d12cdbb 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c @@ -760,7 +760,9 @@ static int mcp251xfd_chip_start(struct mcp251xfd_priv *priv) if (err) goto out_chip_stop; - mcp251xfd_ring_init(priv); + err = mcp251xfd_ring_init(priv); + if (err) + goto out_chip_stop; err = mcp251xfd_chip_fifo_init(priv); if (err) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c index 9657dbf251b0..8126f88d57d7 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c @@ -207,9 +207,9 @@ mcp251xfd_ring_init_rx(struct mcp251xfd_priv *priv, u16 *base, u8 *fifo_nr) } } -void mcp251xfd_ring_init(struct mcp251xfd_priv *priv) +int mcp251xfd_ring_init(struct mcp251xfd_priv *priv) { - u16 base = 0; + u16 base = 0, ram_used; u8 fifo_nr = 1; netdev_reset_queue(priv->ndev); @@ -217,6 +217,16 @@ void mcp251xfd_ring_init(struct mcp251xfd_priv *priv) mcp251xfd_ring_init_tef(priv, &base); mcp251xfd_ring_init_rx(priv, &base, &fifo_nr); mcp251xfd_ring_init_tx(priv, &base, &fifo_nr); + + ram_used = base - MCP251XFD_RAM_START; + if (ram_used > MCP251XFD_RAM_SIZE) { + netdev_err(priv->ndev, + "Error during ring configuration, using more RAM (%u bytes) than available (%u bytes).\n", + ram_used, MCP251XFD_RAM_SIZE); + return -ENOMEM; + } + + return 0; } void mcp251xfd_ring_free(struct mcp251xfd_priv *priv) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h index 8ee959890aea..58e76064cf9e 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h @@ -882,7 +882,7 @@ u16 mcp251xfd_crc16_compute2(const void *cmd, size_t cmd_size, const void *data, size_t data_size); u16 mcp251xfd_crc16_compute(const void *data, size_t data_size); int mcp251xfd_regmap_init(struct mcp251xfd_priv *priv); -void mcp251xfd_ring_init(struct mcp251xfd_priv *priv); +int mcp251xfd_ring_init(struct mcp251xfd_priv *priv); void mcp251xfd_ring_free(struct mcp251xfd_priv *priv); int mcp251xfd_ring_alloc(struct mcp251xfd_priv *priv); int mcp251xfd_handle_rxif(struct mcp251xfd_priv *priv); From 83daa863f16bc8fcc8c277f80ffd94042b71e0cf Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Wed, 4 Aug 2021 13:26:34 +0200 Subject: [PATCH 34/36] can: mcp251xfd: ring: update FIFO setup debug info The recent change of the order of the TX and RX FIFOs is not reflected in the debug info of the FIFO setup. This patch adjust the order and additionally prints the base address of each FIFO. Since the mcp251xfd_ring_init() may fail due to wrongly configured FIFOs, printing of the FIFO setup is moved there. In case of an error it would not be printed in mcp251xfd_ring_init(). Link: https://lore.kernel.org/all/20220217103826.2299157-7-mkl@pengutronix.de Signed-off-by: Marc Kleine-Budde --- .../net/can/spi/mcp251xfd/mcp251xfd-ring.c | 44 ++++++++++++------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c index 8126f88d57d7..9610d262e966 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c @@ -209,8 +209,10 @@ mcp251xfd_ring_init_rx(struct mcp251xfd_priv *priv, u16 *base, u8 *fifo_nr) int mcp251xfd_ring_init(struct mcp251xfd_priv *priv) { + const struct mcp251xfd_rx_ring *rx_ring; u16 base = 0, ram_used; u8 fifo_nr = 1; + int i; netdev_reset_queue(priv->ndev); @@ -218,6 +220,32 @@ int mcp251xfd_ring_init(struct mcp251xfd_priv *priv) mcp251xfd_ring_init_rx(priv, &base, &fifo_nr); mcp251xfd_ring_init_tx(priv, &base, &fifo_nr); + netdev_dbg(priv->ndev, + "FIFO setup: TEF: 0x%03x: %2d*%zu bytes = %4zu bytes\n", + mcp251xfd_get_tef_obj_addr(0), + priv->tx->obj_num, sizeof(struct mcp251xfd_hw_tef_obj), + priv->tx->obj_num * sizeof(struct mcp251xfd_hw_tef_obj)); + + mcp251xfd_for_each_rx_ring(priv, rx_ring, i) { + netdev_dbg(priv->ndev, + "FIFO setup: RX-%u: FIFO %u/0x%03x: %2u*%u bytes = %4u bytes\n", + rx_ring->nr, rx_ring->fifo_nr, + mcp251xfd_get_rx_obj_addr(rx_ring, 0), + rx_ring->obj_num, rx_ring->obj_size, + rx_ring->obj_num * rx_ring->obj_size); + } + + netdev_dbg(priv->ndev, + "FIFO setup: TX: FIFO %u/0x%03x: %2u*%u bytes = %4u bytes\n", + priv->tx->fifo_nr, + mcp251xfd_get_tx_obj_addr(priv->tx, 0), + priv->tx->obj_num, priv->tx->obj_size, + priv->tx->obj_num * priv->tx->obj_size); + + netdev_dbg(priv->ndev, + "FIFO setup: free: %4u bytes\n", + MCP251XFD_RAM_SIZE - (base - MCP251XFD_RAM_START)); + ram_used = base - MCP251XFD_RAM_START; if (ram_used > MCP251XFD_RAM_SIZE) { netdev_err(priv->ndev, @@ -288,21 +316,5 @@ int mcp251xfd_ring_alloc(struct mcp251xfd_priv *priv) } priv->rx_ring_num = i; - netdev_dbg(priv->ndev, - "FIFO setup: TEF: %d*%d bytes = %d bytes, TX: %d*%d bytes = %d bytes\n", - tx_obj_num, tef_obj_size, tef_obj_size * tx_obj_num, - tx_obj_num, tx_obj_size, tx_obj_size * tx_obj_num); - - mcp251xfd_for_each_rx_ring(priv, rx_ring, i) { - netdev_dbg(priv->ndev, - "FIFO setup: RX-%d: %d*%d bytes = %d bytes\n", - i, rx_ring->obj_num, rx_ring->obj_size, - rx_ring->obj_size * rx_ring->obj_num); - } - - netdev_dbg(priv->ndev, - "FIFO setup: free: %d bytes\n", - ram_free); - return 0; } From 887e359d6cce1c143d0ac39208538492d5d42191 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Sun, 10 Oct 2021 21:39:59 +0200 Subject: [PATCH 35/36] can: mcp251xfd: prepare for multiple RX-FIFOs This patch prepares the driver to use more than one RX-FIFO. Having a bigger RX buffer is beneficial in high load situations, where the system temporarily cannot keep up reading CAN frames from the chip. Using a bigger RX buffer also allows to implement RX IRQ coalescing, which will be added in a later patch series. If using more than 1 RX-FIFO the driver has to figure out, which FIFOs have RX'ed CAN frames pending. This is indicated by a set bit in the RXIF register, which is positioned directly after the interrupt status register INT. If more than 1 RX-FIFO is used, the driver reads both registers in 1 transfer. The mcp251xfd_handle_rxif() function iterates over all RX rings and reads out the RX'ed CAN frames for for all pending FIFOs. To keep the logic for the 1 RX-FIFO only case in mcp251xfd_handle_rxif() simple, the driver marks that FIFO pending in mcp251xfd_ring_init(). The chip has a dedicated RX interrupt line to signal pending RX'ed frames. If connected to an input GPIO and the driver will skip the initial read of the interrupt status register (INT) and directly read the pending RX'ed frames if the line is active. The driver assumes the 1st RX-FIFO pending (a read of the RXIF register would re-introduce the skipped initial read of the INT register). Any other pending RX-FIFO will be served in the main interrupt handler. Link: https://lore.kernel.org/all/20220217103826.2299157-8-mkl@pengutronix.de Signed-off-by: Marc Kleine-Budde --- .../net/can/spi/mcp251xfd/mcp251xfd-core.c | 32 +++++++++++++++---- .../net/can/spi/mcp251xfd/mcp251xfd-ring.c | 18 +++++++++++ drivers/net/can/spi/mcp251xfd/mcp251xfd-rx.c | 12 +++++-- drivers/net/can/spi/mcp251xfd/mcp251xfd.h | 1 + 4 files changed, 54 insertions(+), 9 deletions(-) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c index 2d033d12cdbb..d9aaaa91109d 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c @@ -1384,6 +1384,20 @@ static int mcp251xfd_handle_spicrcif(struct mcp251xfd_priv *priv) return 0; } +static int mcp251xfd_read_regs_status(struct mcp251xfd_priv *priv) +{ + const int val_bytes = regmap_get_val_bytes(priv->map_reg); + size_t len; + + if (priv->rx_ring_num == 1) + len = sizeof(priv->regs_status.intf); + else + len = sizeof(priv->regs_status); + + return regmap_bulk_read(priv->map_reg, MCP251XFD_REG_INT, + &priv->regs_status, len / val_bytes); +} + #define mcp251xfd_handle(priv, irq, ...) \ ({ \ struct mcp251xfd_priv *_priv = (priv); \ @@ -1400,7 +1414,6 @@ static int mcp251xfd_handle_spicrcif(struct mcp251xfd_priv *priv) static irqreturn_t mcp251xfd_irq(int irq, void *dev_id) { struct mcp251xfd_priv *priv = dev_id; - const int val_bytes = regmap_get_val_bytes(priv->map_reg); irqreturn_t handled = IRQ_NONE; int err; @@ -1412,21 +1425,28 @@ static irqreturn_t mcp251xfd_irq(int irq, void *dev_id) if (!rx_pending) break; + /* Assume 1st RX-FIFO pending, if other FIFOs + * are pending the main IRQ handler will take + * care. + */ + priv->regs_status.rxif = BIT(priv->rx[0]->fifo_nr); err = mcp251xfd_handle(priv, rxif); if (err) goto out_fail; handled = IRQ_HANDLED; - } while (1); + + /* We don't know which RX-FIFO is pending, but only + * handle the 1st RX-FIFO. Leave loop here if we have + * more than 1 RX-FIFO to avoid starvation. + */ + } while (priv->rx_ring_num == 1); do { u32 intf_pending, intf_pending_clearable; bool set_normal_mode = false; - err = regmap_bulk_read(priv->map_reg, MCP251XFD_REG_INT, - &priv->regs_status, - sizeof(priv->regs_status) / - val_bytes); + err = mcp251xfd_read_regs_status(priv); if (err) goto out_fail; diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c index 9610d262e966..848b8b2ecb5f 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-ring.c @@ -220,6 +220,24 @@ int mcp251xfd_ring_init(struct mcp251xfd_priv *priv) mcp251xfd_ring_init_rx(priv, &base, &fifo_nr); mcp251xfd_ring_init_tx(priv, &base, &fifo_nr); + /* mcp251xfd_handle_rxif() will iterate over all RX rings. + * Rings with their corresponding bit set in + * priv->regs_status.rxif are read out. + * + * If the chip is configured for only 1 RX-FIFO, and if there + * is an RX interrupt pending (RXIF in INT register is set), + * it must be the 1st RX-FIFO. + * + * We mark the RXIF of the 1st FIFO as pending here, so that + * we can skip the read of the RXIF register in + * mcp251xfd_read_regs_status() for the 1 RX-FIFO only case. + * + * If we use more than 1 RX-FIFO, this value gets overwritten + * in mcp251xfd_read_regs_status(), so set it unconditionally + * here. + */ + priv->regs_status.rxif = BIT(priv->rx[0]->fifo_nr); + netdev_dbg(priv->ndev, "FIFO setup: TEF: 0x%03x: %2d*%zu bytes = %4zu bytes\n", mcp251xfd_get_tef_obj_addr(0), diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-rx.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-rx.c index 63f2526464b3..e6d39876065a 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-rx.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-rx.c @@ -19,7 +19,7 @@ static inline int mcp251xfd_rx_head_get_from_chip(const struct mcp251xfd_priv *priv, const struct mcp251xfd_rx_ring *ring, - u8 *rx_head) + u8 *rx_head, bool *fifo_empty) { u32 fifo_sta; int err; @@ -30,6 +30,7 @@ mcp251xfd_rx_head_get_from_chip(const struct mcp251xfd_priv *priv, return err; *rx_head = FIELD_GET(MCP251XFD_REG_FIFOSTA_FIFOCI_MASK, fifo_sta); + *fifo_empty = !(fifo_sta & MCP251XFD_REG_FIFOSTA_TFNRFNIF); return 0; } @@ -84,10 +85,12 @@ mcp251xfd_rx_ring_update(const struct mcp251xfd_priv *priv, { u32 new_head; u8 chip_rx_head; + bool fifo_empty; int err; - err = mcp251xfd_rx_head_get_from_chip(priv, ring, &chip_rx_head); - if (err) + err = mcp251xfd_rx_head_get_from_chip(priv, ring, &chip_rx_head, + &fifo_empty); + if (err || fifo_empty) return err; /* chip_rx_head, is the next RX-Object filled by the HW. @@ -251,6 +254,9 @@ int mcp251xfd_handle_rxif(struct mcp251xfd_priv *priv) int err, n; mcp251xfd_for_each_rx_ring(priv, ring, n) { + if (!(priv->regs_status.rxif & BIT(ring->fifo_nr))) + continue; + err = mcp251xfd_handle_rxif_ring(priv, ring); if (err) return err; diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h index 58e76064cf9e..f359dd0aa458 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h @@ -561,6 +561,7 @@ struct mcp251xfd_ecc { struct mcp251xfd_regs_status { u32 intf; + u32 rxif; }; enum mcp251xfd_model { From aada74220f0076a2f76392806224ca385fae536a Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Mon, 11 Oct 2021 22:37:34 +0200 Subject: [PATCH 36/36] can: mcp251xfd: mcp251xfd_priv: introduce macros specifying the number of supported TEF/RX/TX rings This patch introduces macros to define the number of supported TEF, RX and TX rings. As well as some assertions as sanity checks. Link: https://lore.kernel.org/all/20220217103826.2299157-9-mkl@pengutronix.de Signed-off-by: Marc Kleine-Budde --- drivers/net/can/spi/mcp251xfd/mcp251xfd.h | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h index f359dd0aa458..87cc13d455c1 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h @@ -2,7 +2,7 @@ * * mcp251xfd - Microchip MCP251xFD Family CAN controller driver * - * Copyright (c) 2019, 2020 Pengutronix, + * Copyright (c) 2019, 2020, 2021 Pengutronix, * Marc Kleine-Budde * Copyright (c) 2019 Martin Sperl */ @@ -410,6 +410,15 @@ static_assert(MCP251XFD_TIMESTAMP_WORK_DELAY_SEC < #define MCP251XFD_SANITIZE_SPI 1 #define MCP251XFD_SANITIZE_CAN 1 +/* FIFO and Ring */ +#define MCP251XFD_FIFO_TEF_NUM 1U +#define MCP251XFD_FIFO_RX_NUM_MAX 1U +#define MCP251XFD_FIFO_TX_NUM 1U + +static_assert(MCP251XFD_FIFO_TEF_NUM == 1U); +static_assert(MCP251XFD_FIFO_TEF_NUM == MCP251XFD_FIFO_TX_NUM); +static_assert(MCP251XFD_FIFO_RX_NUM_MAX <= 4U); + /* Silence TX MAB overflow warnings */ #define MCP251XFD_QUIRK_MAB_NO_WARN BIT(0) /* Use CRC to access registers */ @@ -596,9 +605,9 @@ struct mcp251xfd_priv { u32 spi_max_speed_hz_fast; u32 spi_max_speed_hz_slow; - struct mcp251xfd_tef_ring tef[1]; - struct mcp251xfd_rx_ring *rx[1]; - struct mcp251xfd_tx_ring tx[1]; + struct mcp251xfd_tef_ring tef[MCP251XFD_FIFO_TEF_NUM]; + struct mcp251xfd_rx_ring *rx[MCP251XFD_FIFO_RX_NUM_MAX]; + struct mcp251xfd_tx_ring tx[MCP251XFD_FIFO_TX_NUM]; u8 rx_ring_num;