From ced52c6ed257315c922dc870aafbc64dc8bd746d Mon Sep 17 00:00:00 2001 From: Duy Nguyen Date: Thu, 1 Aug 2024 16:03:17 +0200 Subject: [PATCH 1/6] dt-bindings: can: renesas,rcar-canfd: Document R-Car V4M support Document support for the CAN-FD Interface on the Renesas R-Car V4M (R8A779H0) SoC, which supports up to four channels. The CAN-FD module on R-Car V4M is very similar to the one on R-Car V4H, but differs in some hardware parameters, as reflected by the Parameter Status Information part of the Global IP Version Register. However, none of this parameterization should have any impact on the driver, as the driver does not access any register that is impacted by the parameterization (except for the number of channels). Signed-off-by: Duy Nguyen [geert: Clarify R-Car V4M differences] Signed-off-by: Geert Uytterhoeven Reviewed-by: Simon Horman Acked-by: Rob Herring (Arm) Link: https://patch.msgid.link/68b5f910bef89508e3455c768844ebe859d6ff1d.1722520779.git.geert+renesas@glider.be Signed-off-by: Marc Kleine-Budde --- .../bindings/net/can/renesas,rcar-canfd.yaml | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml index d3f45d29fa0a5..7c5ac5d2e880b 100644 --- a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml +++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml @@ -32,6 +32,7 @@ properties: - enum: - renesas,r8a779a0-canfd # R-Car V3U - renesas,r8a779g0-canfd # R-Car V4H + - renesas,r8a779h0-canfd # R-Car V4M - const: renesas,rcar-gen4-canfd # R-Car Gen4 - items: @@ -163,14 +164,23 @@ allOf: maxItems: 1 - if: - not: - properties: - compatible: - contains: - const: renesas,rcar-gen4-canfd + properties: + compatible: + contains: + const: renesas,r8a779h0-canfd then: patternProperties: - "^channel[2-7]$": false + "^channel[5-7]$": false + else: + if: + not: + properties: + compatible: + contains: + const: renesas,rcar-gen4-canfd + then: + patternProperties: + "^channel[2-7]$": false unevaluatedProperties: false From 09328600c2f930e8814cb9deff630a56788cc8e4 Mon Sep 17 00:00:00 2001 From: Frank Li Date: Wed, 14 Aug 2024 12:44:06 -0400 Subject: [PATCH 2/6] dt-bindings: can: convert microchip,mcp251x.txt to yaml Convert binding doc microchip,mcp251x.txt to yaml. Additional change: - add ref to spi-peripheral-props.yaml Fix below warning: arch/arm64/boot/dts/freescale/imx8dx-colibri-eval-v3.dtb: /bus@5a000000/spi@5a020000/can@0: failed to match any schema with compatible: ['microchip,mcp2515'] Signed-off-by: Frank Li Reviewed-by: Conor Dooley Link: https://patch.msgid.link/20240814164407.4022211-1-Frank.Li@nxp.com Signed-off-by: Marc Kleine-Budde --- .../bindings/net/can/microchip,mcp2510.yaml | 70 +++++++++++++++++++ .../bindings/net/can/microchip,mcp251x.txt | 30 -------- 2 files changed, 70 insertions(+), 30 deletions(-) create mode 100644 Documentation/devicetree/bindings/net/can/microchip,mcp2510.yaml delete mode 100644 Documentation/devicetree/bindings/net/can/microchip,mcp251x.txt diff --git a/Documentation/devicetree/bindings/net/can/microchip,mcp2510.yaml b/Documentation/devicetree/bindings/net/can/microchip,mcp2510.yaml new file mode 100644 index 0000000000000..db446dde68420 --- /dev/null +++ b/Documentation/devicetree/bindings/net/can/microchip,mcp2510.yaml @@ -0,0 +1,70 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/can/microchip,mcp2510.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Microchip MCP251X stand-alone CAN controller + +maintainers: + - Marc Kleine-Budde + +properties: + compatible: + enum: + - microchip,mcp2510 + - microchip,mcp2515 + - microchip,mcp25625 + + reg: + maxItems: 1 + + clocks: + maxItems: 1 + + interrupts: + maxItems: 1 + + vdd-supply: + description: Regulator that powers the CAN controller. + + xceiver-supply: + description: Regulator that powers the CAN transceiver. + + gpio-controller: true + + "#gpio-cells": + const: 2 + +required: + - compatible + - reg + - clocks + - interrupts + +allOf: + - $ref: /schemas/spi/spi-peripheral-props.yaml# + +unevaluatedProperties: false + +examples: + - | + #include + + spi { + #address-cells = <1>; + #size-cells = <0>; + + can@1 { + compatible = "microchip,mcp2515"; + reg = <1>; + clocks = <&clk24m>; + interrupt-parent = <&gpio4>; + interrupts = <13 IRQ_TYPE_LEVEL_LOW>; + vdd-supply = <®5v0>; + xceiver-supply = <®5v0>; + gpio-controller; + #gpio-cells = <2>; + }; + }; + diff --git a/Documentation/devicetree/bindings/net/can/microchip,mcp251x.txt b/Documentation/devicetree/bindings/net/can/microchip,mcp251x.txt deleted file mode 100644 index 381f8fb3e865a..0000000000000 --- a/Documentation/devicetree/bindings/net/can/microchip,mcp251x.txt +++ /dev/null @@ -1,30 +0,0 @@ -* Microchip MCP251X stand-alone CAN controller device tree bindings - -Required properties: - - compatible: Should be one of the following: - - "microchip,mcp2510" for MCP2510. - - "microchip,mcp2515" for MCP2515. - - "microchip,mcp25625" for MCP25625. - - reg: SPI chip select. - - clocks: The clock feeding the CAN controller. - - interrupts: Should contain IRQ line for the CAN controller. - -Optional properties: - - vdd-supply: Regulator that powers the CAN controller. - - xceiver-supply: Regulator that powers the CAN transceiver. - - gpio-controller: Indicates this device is a GPIO controller. - - #gpio-cells: Should be two. The first cell is the pin number and - the second cell is used to specify the gpio polarity. - -Example: - can0: can@1 { - compatible = "microchip,mcp2515"; - reg = <1>; - clocks = <&clk24m>; - interrupt-parent = <&gpio4>; - interrupts = <13 IRQ_TYPE_LEVEL_LOW>; - vdd-supply = <®5v0>; - xceiver-supply = <®5v0>; - gpio-controller; - #gpio-cells = <2>; - }; From dc2ddcd136fe9b6196a7dd01f75f824beb02d43f Mon Sep 17 00:00:00 2001 From: Zhang Changzhong Date: Thu, 29 Aug 2024 20:48:23 +0800 Subject: [PATCH 3/6] can: j1939: use correct function name in comment The function j1939_cancel_all_active_sessions() was renamed to j1939_cancel_active_session() but name in comment wasn't updated. Signed-off-by: Zhang Changzhong Acked-by: Oleksij Rempel Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol") Link: https://patch.msgid.link/1724935703-44621-1-git-send-email-zhangchangzhong@huawei.com Signed-off-by: Marc Kleine-Budde --- net/can/j1939/transport.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c index 4be73de5033cb..319f47df33300 100644 --- a/net/can/j1939/transport.c +++ b/net/can/j1939/transport.c @@ -1179,10 +1179,10 @@ static enum hrtimer_restart j1939_tp_txtimer(struct hrtimer *hrtimer) break; case -ENETDOWN: /* In this case we should get a netdev_event(), all active - * sessions will be cleared by - * j1939_cancel_all_active_sessions(). So handle this as an - * error, but let j1939_cancel_all_active_sessions() do the - * cleanup including propagation of the error to user space. + * sessions will be cleared by j1939_cancel_active_session(). + * So handle this as an error, but let + * j1939_cancel_active_session() do the cleanup including + * propagation of the error to user space. */ break; case -EOVERFLOW: From 2423cc20087ae9a7b7af575aa62304ef67cad7b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20M=C3=A4tje?= Date: Thu, 8 Aug 2024 18:42:24 +0200 Subject: [PATCH 4/6] can: netlink: avoid call to do_set_data_bittiming callback with stale can_priv::ctrlmode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch moves the evaluation of data[IFLA_CAN_CTRLMODE] in function can_changelink in front of the evaluation of data[IFLA_CAN_BITTIMING]. This avoids a call to do_set_data_bittiming providing a stale can_priv::ctrlmode with a CAN_CTRLMODE_FD flag not matching the requested state when switching between a CAN Classic and CAN-FD bitrate. In the same manner the evaluation of data[IFLA_CAN_CTRLMODE] in function can_validate is also moved in front of the evaluation of data[IFLA_CAN_BITTIMING]. This is a preparation for patches where the nominal and data bittiming may have interdependencies on the driver side depending on the CAN_CTRLMODE_FD flag state. Signed-off-by: Stefan Mätje Link: https://patch.msgid.link/20240808164224.213522-1-stefan.maetje@esd.eu Signed-off-by: Marc Kleine-Budde --- drivers/net/can/dev/netlink.c | 102 +++++++++++++++++----------------- 1 file changed, 51 insertions(+), 51 deletions(-) diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c index dfdc039d92a6c..01aacdcda2606 100644 --- a/drivers/net/can/dev/netlink.c +++ b/drivers/net/can/dev/netlink.c @@ -65,15 +65,6 @@ static int can_validate(struct nlattr *tb[], struct nlattr *data[], if (!data) return 0; - if (data[IFLA_CAN_BITTIMING]) { - struct can_bittiming bt; - - memcpy(&bt, nla_data(data[IFLA_CAN_BITTIMING]), sizeof(bt)); - err = can_validate_bittiming(&bt, extack); - if (err) - return err; - } - if (data[IFLA_CAN_CTRLMODE]) { struct can_ctrlmode *cm = nla_data(data[IFLA_CAN_CTRLMODE]); u32 tdc_flags = cm->flags & CAN_CTRLMODE_TDC_MASK; @@ -114,6 +105,15 @@ static int can_validate(struct nlattr *tb[], struct nlattr *data[], } } + if (data[IFLA_CAN_BITTIMING]) { + struct can_bittiming bt; + + memcpy(&bt, nla_data(data[IFLA_CAN_BITTIMING]), sizeof(bt)); + err = can_validate_bittiming(&bt, extack); + if (err) + return err; + } + if (is_can_fd) { if (!data[IFLA_CAN_BITTIMING] || !data[IFLA_CAN_DATA_BITTIMING]) return -EOPNOTSUPP; @@ -195,48 +195,6 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[], /* We need synchronization with dev->stop() */ ASSERT_RTNL(); - if (data[IFLA_CAN_BITTIMING]) { - struct can_bittiming bt; - - /* Do not allow changing bittiming while running */ - if (dev->flags & IFF_UP) - return -EBUSY; - - /* Calculate bittiming parameters based on - * bittiming_const if set, otherwise pass bitrate - * directly via do_set_bitrate(). Bail out if neither - * is given. - */ - if (!priv->bittiming_const && !priv->do_set_bittiming && - !priv->bitrate_const) - return -EOPNOTSUPP; - - memcpy(&bt, nla_data(data[IFLA_CAN_BITTIMING]), sizeof(bt)); - err = can_get_bittiming(dev, &bt, - priv->bittiming_const, - priv->bitrate_const, - priv->bitrate_const_cnt, - extack); - if (err) - return err; - - if (priv->bitrate_max && bt.bitrate > priv->bitrate_max) { - NL_SET_ERR_MSG_FMT(extack, - "arbitration bitrate %u bps surpasses transceiver capabilities of %u bps", - bt.bitrate, priv->bitrate_max); - return -EINVAL; - } - - memcpy(&priv->bittiming, &bt, sizeof(bt)); - - if (priv->do_set_bittiming) { - /* Finally, set the bit-timing registers */ - err = priv->do_set_bittiming(dev); - if (err) - return err; - } - } - if (data[IFLA_CAN_CTRLMODE]) { struct can_ctrlmode *cm; u32 ctrlstatic; @@ -284,6 +242,48 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[], priv->ctrlmode &= cm->flags | ~CAN_CTRLMODE_TDC_MASK; } + if (data[IFLA_CAN_BITTIMING]) { + struct can_bittiming bt; + + /* Do not allow changing bittiming while running */ + if (dev->flags & IFF_UP) + return -EBUSY; + + /* Calculate bittiming parameters based on + * bittiming_const if set, otherwise pass bitrate + * directly via do_set_bitrate(). Bail out if neither + * is given. + */ + if (!priv->bittiming_const && !priv->do_set_bittiming && + !priv->bitrate_const) + return -EOPNOTSUPP; + + memcpy(&bt, nla_data(data[IFLA_CAN_BITTIMING]), sizeof(bt)); + err = can_get_bittiming(dev, &bt, + priv->bittiming_const, + priv->bitrate_const, + priv->bitrate_const_cnt, + extack); + if (err) + return err; + + if (priv->bitrate_max && bt.bitrate > priv->bitrate_max) { + NL_SET_ERR_MSG_FMT(extack, + "arbitration bitrate %u bps surpasses transceiver capabilities of %u bps", + bt.bitrate, priv->bitrate_max); + return -EINVAL; + } + + memcpy(&priv->bittiming, &bt, sizeof(bt)); + + if (priv->do_set_bittiming) { + /* Finally, set the bit-timing registers */ + err = priv->do_set_bittiming(dev); + if (err) + return err; + } + } + if (data[IFLA_CAN_RESTART_MS]) { /* Do not allow changing restart delay while running */ if (dev->flags & IFF_UP) From a9c0fb33fd454fda5283281e47d556c81ee9fa47 Mon Sep 17 00:00:00 2001 From: Martin Jocic Date: Thu, 22 Aug 2024 18:38:59 +0200 Subject: [PATCH 5/6] can: kvaser_pciefd: Use IS_ENABLED() instead of #ifdef Use the IS_ENABLED() macro to check kernel config defines instead of ifdef. Use upper_32_bits() to avoid warnings about "right shift count >= width of type" on systems with CONFIG_ARCH_DMA_ADDR_T_64BIT not set. In kvaser_pciefd_write_dma_map_altera() use lower_32_bits() for symmetry. Signed-off-by: Martin Jocic Link: https://patch.msgid.link/20240830141038.1402217-1-mkl@pengutronix.de Signed-off-by: Marc Kleine-Budde --- drivers/net/can/kvaser_pciefd.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c index a60d9efd5f8d1..dc7e5ea1e3ac6 100644 --- a/drivers/net/can/kvaser_pciefd.c +++ b/drivers/net/can/kvaser_pciefd.c @@ -1053,13 +1053,13 @@ static void kvaser_pciefd_write_dma_map_altera(struct kvaser_pciefd *pcie, void __iomem *serdes_base; u32 word1, word2; -#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT - word1 = addr | KVASER_PCIEFD_ALTERA_DMA_64BIT; - word2 = addr >> 32; -#else - word1 = addr; - word2 = 0; -#endif + if (IS_ENABLED(CONFIG_ARCH_DMA_ADDR_T_64BIT)) { + word1 = lower_32_bits(addr) | KVASER_PCIEFD_ALTERA_DMA_64BIT; + word2 = upper_32_bits(addr); + } else { + word1 = addr; + word2 = 0; + } serdes_base = KVASER_PCIEFD_SERDES_ADDR(pcie) + 0x8 * index; iowrite32(word1, serdes_base); iowrite32(word2, serdes_base + 0x4); @@ -1072,9 +1072,9 @@ static void kvaser_pciefd_write_dma_map_sf2(struct kvaser_pciefd *pcie, u32 lsb = addr & KVASER_PCIEFD_SF2_DMA_LSB_MASK; u32 msb = 0x0; -#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT - msb = addr >> 32; -#endif + if (IS_ENABLED(CONFIG_ARCH_DMA_ADDR_T_64BIT)) + msb = upper_32_bits(addr); + serdes_base = KVASER_PCIEFD_SERDES_ADDR(pcie) + 0x10 * index; iowrite32(lsb, serdes_base); iowrite32(msb, serdes_base + 0x4); @@ -1087,9 +1087,9 @@ static void kvaser_pciefd_write_dma_map_xilinx(struct kvaser_pciefd *pcie, u32 lsb = addr & KVASER_PCIEFD_XILINX_DMA_LSB_MASK; u32 msb = 0x0; -#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT - msb = addr >> 32; -#endif + if (IS_ENABLED(CONFIG_ARCH_DMA_ADDR_T_64BIT)) + msb = upper_32_bits(addr); + serdes_base = KVASER_PCIEFD_SERDES_ADDR(pcie) + 0x8 * index; iowrite32(msb, serdes_base); iowrite32(lsb, serdes_base + 0x4); From 0315c0b5ed253853a7b07dc97487522345110548 Mon Sep 17 00:00:00 2001 From: Yan Zhen Date: Fri, 30 Aug 2024 19:06:51 +0800 Subject: [PATCH 6/6] can: kvaser_usb: Simplify with dev_err_probe() dev_err_probe() is used to log an error message during the probe process of a device. It can simplify the error path and unify a message template. Using this helper is totally fine even if err is known to never be -EPROBE_DEFER. The benefit compared to a normal dev_err() is the standardized format of the error code, it being emitted symbolically and the fact that the error code is returned which allows more compact error paths. Signed-off-by: Yan Zhen Link: https://patch.msgid.link/20240830110651.519119-1-yanzhen@vivo.com mkl: fix indention Signed-off-by: Marc Kleine-Budde --- .../net/can/usb/kvaser_usb/kvaser_usb_core.c | 42 +++++++------------ 1 file changed, 16 insertions(+), 26 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 35b4132b0639c..7d12776ab63e6 100644 --- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c +++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c @@ -898,10 +898,8 @@ static int kvaser_usb_probe(struct usb_interface *intf, ops = driver_info->ops; err = ops->dev_setup_endpoints(dev); - if (err) { - dev_err(&intf->dev, "Cannot get usb endpoint(s)"); - return err; - } + if (err) + return dev_err_probe(&intf->dev, err, "Cannot get usb endpoint(s)"); dev->udev = interface_to_usbdev(intf); @@ -912,26 +910,20 @@ static int kvaser_usb_probe(struct usb_interface *intf, dev->card_data.ctrlmode_supported = 0; dev->card_data.capabilities = 0; err = ops->dev_init_card(dev); - if (err) { - dev_err(&intf->dev, - "Failed to initialize card, error %d\n", err); - return err; - } + if (err) + return dev_err_probe(&intf->dev, err, + "Failed to initialize card\n"); err = ops->dev_get_software_info(dev); - if (err) { - dev_err(&intf->dev, - "Cannot get software info, error %d\n", err); - return err; - } + if (err) + return dev_err_probe(&intf->dev, err, + "Cannot get software info\n"); if (ops->dev_get_software_details) { err = ops->dev_get_software_details(dev); - if (err) { - dev_err(&intf->dev, - "Cannot get software details, error %d\n", err); - return err; - } + if (err) + return dev_err_probe(&intf->dev, err, + "Cannot get software details\n"); } if (WARN_ON(!dev->cfg)) @@ -945,18 +937,16 @@ static int kvaser_usb_probe(struct usb_interface *intf, dev_dbg(&intf->dev, "Max outstanding tx = %d URBs\n", dev->max_tx_urbs); err = ops->dev_get_card_info(dev); - if (err) { - dev_err(&intf->dev, "Cannot get card info, error %d\n", err); - return err; - } + if (err) + return dev_err_probe(&intf->dev, err, + "Cannot get card info\n"); if (ops->dev_get_capabilities) { err = ops->dev_get_capabilities(dev); if (err) { - dev_err(&intf->dev, - "Cannot get capabilities, error %d\n", err); kvaser_usb_remove_interfaces(dev); - return err; + return dev_err_probe(&intf->dev, err, + "Cannot get capabilities\n"); } }