[PATCH v7 RESEND 00/15] can: grcan: Enhance driver with CANFD Support and Improvements

[PATCH v7 RESEND 00/15] can: grcan: Enhance driver with CANFD Support and Improvements

[Arun Muthusamy]

This patch series updates the GRCAN driver to support the GRCANFD core
from the GRLIB IP core library.

In addition to GRCANFD support, the updates include enhancements for
compatibility with NOEL-V (RISC-V) systems, such as matching drivers
using the 'compatible' identifier and adding support for reading clock
frequency via the common clock framework where available. The series
also includes improvements like functions for configuring
nominal bit-timing and optimizations for DMA operations.

This series also updates the driver documentation and bindings.
The old text binding is converted to YAML, a new vendor prefix
is added to reflect the updated ownership and an entry for the
driver is added to the MAINTAINERS file.

Arun Muthusamy (10):
  dt-bindings: net: can: gaisler,grcan: Convert to DT schema
  MAINTAINERS: Add maintainers for GRCAN CAN network driver
  can: grcan: Replace bit timing macros with literal values
  can: grcan: Simplify timing configuration
  can: grcan: add FD capability detection and nominal bit-timing
  can: grcan: Refactor GRCAN DMA buffer to use structured memory layout
  can: grcan: Add CANFD TX support alongside legacy CAN
  can: grcan: Add CANFD RX support alongside legacy CAN
  can: grcan: Update echo skb handling to match variable length CANFD
    frame
  can: grcan: Advertise CANFD capability

Daniel Hellstrom (4):
  can: grcan: Add clock handling
  can: grcan: set DMA mask for GRCAN and GRCANFD to 32-bit
  can: grcan: Add saving and restoring of CAN FD baud-rate registers
  can: grcan: Reserve space between cap and next register to align with
    address layout

Ludwig Rydberg (1):
  dt-bindings: Add vendor prefix for Frontgrade Gaisler AB

Changes in v7:
- No functional changes in v7 compared to v6 except resolving a conflict
  with the treewide change for kcalloc to kzalloc_objs in
  patch 0014-can-grcan-Update-echo-skb-handling-to-match-variable.patch.
- Link to v6: https://lore.kernel.org/all/20260306162934.22955-1-arun.muthusamy@gaisler.com/

Changes in v6:
- can: Implemented anonymous unions for cleaner buffer access.
- can: Removed unnecessary patch.
- can: Replaced GRCAN_CLASSIC_DATA_SIZE with CAN_MAX_DLEN.
- Link to v5: https://lore.kernel.org/all/20260216135344.23246-6-arun.muthusamy@gaisler.com/

Changes in v5:
- can: Removed pointer arithmetic from opaque buffers.
- can: Refactor GRCAN DMA buffer to use structured memory layout.
- Link to v4: https://lore.kernel.org/all/20260128144921.5458-13-arun.muthusamy@gaisler.com/

Changes in v4:
- can: Removed unnecessary saving and restoring of registers.
- can: Corrected spacing inconsistencies.
- can: Revised how timing information is logged.
- can: Eliminated redundant checks for platform data,
- can: Recalculated the number of descriptors needed for both CAN and
    CANFD.
- can: Eliminated multiple independent queue stop conditions.
- can: Aligned the order of patches.
- Link to v3: https://lore.kernel.org/all/20260122121038.7910-1-arun.muthusamy@gaisler.com/

Changes in v3:
- can: Replaced custom bit shifting with FIELD_PREP and GENMASK.
- can: Factorized common elements of struct canfd_frame and
    struct can_frame, excluding DLC assignment.
- can: Split the patch to separate code refactoring from the
    introduction of the new feature.
- can: Remove Unused macros
- Link to v2: https://lore.kernel.org/all/20251223105604.12675-1-arun.muthusamy@gaisler.com/

Changes in v2:
- bindings: Updated commit message to explain the removal of freq
  and systemid in the new binding.
- can: S-o-b is placed last in commit messages.
- can: Add values are directly added to the struct can_bittiming_const.
- can: Replaced custom bit shifting with FIELD_PREP and GENMASK for clarity.
- can: Drop do_set_bittiming() callback
- can: Remove forward declarations, unnecessary parentheses, redundant comments
- can: Adjust line breaks according to new character limits
- can: Eliminating unnecessary defines
- can: Enhance code efficiency with memcpy, use standard API to fetch device specific data and
  error handling.
- Link to v1: https://lore.kernel.org/all/20251118092115.3455-1-arun.muthusamy@gaisler.com/

 .../bindings/net/can/gaisler,grcan.yaml       |  62 +++
 .../devicetree/bindings/net/can/grcan.txt     |  28 -
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 MAINTAINERS                                   |   8 +
 drivers/net/can/Kconfig                       |   6 +-
 drivers/net/can/grcan.c                       | 516 +++++++++++++-----
 6 files changed, 445 insertions(+), 177 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/can/gaisler,grcan.yaml
 delete mode 100644 Documentation/devicetree/bindings/net/can/grcan.txt


base-commit: f1359c240191e686614847905fc861cbda480b47
--
2.51.0

[PATCH v7 01/15] dt-bindings: Add vendor prefix for Frontgrade Gaisler AB

[Arun Muthusamy]

From: Ludwig Rydberg <ludwig.rydberg@gaisler.com>

Frontgrade Gaisler AB provides IP cores and supporting development tools
for embedded processors based on the SPARC and RISC-V architectures.
Some essential products are the LEON and NOEL synthesizable processor
models together with a complete development environment and a library of
IP cores (GRLIB).

The company specializes in digital hardware design (ASIC/FPGA) for both
commercial and aerospace applications.

Web site: https://www.gaisler.com/

Signed-off-by: Ludwig Rydberg <ludwig.rydberg@gaisler.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Arun Muthusamy <arun.muthusamy@gaisler.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index ee7fd3cfe203..d70ec6226f0c 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -610,6 +610,8 @@ patternProperties:
     description: Fujitsu Ltd.
   "^fxtec,.*":
     description: FX Technology Ltd.
+  "^gaisler,.*":
+    description: Frontgrade Gaisler AB
   "^galaxycore,.*":
     description: GalaxyCore Inc.
   "^gameforce,.*":
-- 
2.51.0

[PATCH v7 02/15] dt-bindings: net: can: gaisler,grcan: Convert to DT schema

[Arun Muthusamy]

Migrate device tree bindings for Gaisler GRCAN, GRHCAN
and GRCANFD CAN controllers from a text format to YAML format.

Additional changes:
  - Remove stale systemid property
    removed in commit 1e93ed26acf0 ("can: grcan: grcan_probe():
    fix broken system id check for errata workaround needs")
  - Make freq optional
  - Add clocks
  - Add compatible
  - Add example

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Arun Muthusamy <arun.muthusamy@gaisler.com>
---
 .../bindings/net/can/gaisler,grcan.yaml       | 62 +++++++++++++++++++
 .../devicetree/bindings/net/can/grcan.txt     | 28 ---------
 2 files changed, 62 insertions(+), 28 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/can/gaisler,grcan.yaml
 delete mode 100644 Documentation/devicetree/bindings/net/can/grcan.txt

diff --git a/Documentation/devicetree/bindings/net/can/gaisler,grcan.yaml b/Documentation/devicetree/bindings/net/can/gaisler,grcan.yaml
new file mode 100644
index 000000000000..8fea97c7319a
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/gaisler,grcan.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/can/gaisler,grcan.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Aeroflex Gaisler GRCAN, GRHCAN and GRCANFD CAN controllers.
+
+description: |
+  GRCAN, GRCANFD, GRHCAN controllers are available in the GRLIB VHDL
+  IP core library.
+  For further information look in the documentation for the GRLIB IP library:
+  https://download.gaisler.com/products/GRLIB/doc/grip.pdf
+
+maintainers:
+  - Arun Muthusamy <arun.muthusamy@gaisler.com>
+  - Andreas Larsson <andreas@gaisler.com>
+
+allOf:
+  - $ref: can-controller.yaml#
+
+properties:
+  compatible:
+    enum:
+      - gaisler,grcan
+      - gaisler,grcanfd
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  freq:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Frequency of the external oscillator clock in Hz (the frequency of the
+      AMBA bus in the ordinary case).
+      This property should be used by systems where the common clock
+      framework is not supported.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    can@ff400000 {
+        compatible = "gaisler,grcanfd";
+        clocks = <&sysclock>;
+        reg = <0xff400000 0x400>;
+        interrupt-parent = <&plic0>;
+        interrupts = <6>;
+    };
diff --git a/Documentation/devicetree/bindings/net/can/grcan.txt b/Documentation/devicetree/bindings/net/can/grcan.txt
deleted file mode 100644
index 34ef3498f887..000000000000
--- a/Documentation/devicetree/bindings/net/can/grcan.txt
+++ /dev/null
@@ -1,28 +0,0 @@
-Aeroflex Gaisler GRCAN and GRHCAN CAN controllers.
-
-The GRCAN and CRHCAN CAN controllers are available in the GRLIB VHDL IP core
-library.
-
-Note: These properties are built from the AMBA plug&play in a Leon SPARC system
-(the ordinary environment for GRCAN and GRHCAN). There are no dts files for
-sparc.
-
-Required properties:
-
-- name : Should be "GAISLER_GRCAN", "01_03d", "GAISLER_GRHCAN" or "01_034"
-
-- reg : Address and length of the register set for the device
-
-- freq : Frequency of the external oscillator clock in Hz (the frequency of
-	the amba bus in the ordinary case)
-
-- interrupts : Interrupt number for this device
-
-Optional properties:
-
-- systemid : If not present or if the value of the least significant 16 bits
-	of this 32-bit property is smaller than GRCAN_TXBUG_SAFE_GRLIB_VERSION
-	a bug workaround is activated.
-
-For further information look in the documentation for the GLIB IP core library:
-http://www.gaisler.com/products/grlib/grip.pdf
-- 
2.51.0

[PATCH v7 03/15] MAINTAINERS: Add maintainers for GRCAN CAN network driver

[Arun Muthusamy]

Add Arun Muthusamy and Andreas Larsson as maintainers for
the GRCAN CAN network driver, This entry ensures clear
communication channels for maintaining the Documentation
and driver code associated with GRCAN.

Signed-off-by: Arun Muthusamy <arun.muthusamy@gaisler.com>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7a2ffd9d37d5..60b5d6212207 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10972,6 +10972,14 @@ F:	drivers/gpio/gpiolib-cdev.c
 F:	include/uapi/linux/gpio.h
 F:	tools/gpio/
 
+GRCAN CAN NETWORK DRIVER
+M:	Andreas Larsson <andreas@gaisler.com>
+M:	Arun Muthusamy <arun.muthusamy@gaisler.com>
+L:	linux-can@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/net/can/gaisler,grcan.yaml
+F:	drivers/net/can/grcan.c
+
 GRETH 10/100/1G Ethernet MAC device driver
 M:	Andreas Larsson <andreas@gaisler.com>
 L:	netdev@vger.kernel.org
-- 
2.51.0

[PATCH v7 04/15] can: grcan: Add clock handling

[Arun Muthusamy]

From: Daniel Hellstrom <daniel@gaisler.com>

Add clock handling and add error messages for missing 'freq' DT property.

Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
Signed-off-by: Arun Muthusamy <arun.muthusamy@gaisler.com>
---
 drivers/net/can/grcan.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
index ce12fa6df56d..a5f750d7e2d8 100644
--- a/drivers/net/can/grcan.c
+++ b/drivers/net/can/grcan.c
@@ -34,7 +34,7 @@
 #include <linux/spinlock.h>
 #include <linux/of.h>
 #include <linux/of_irq.h>
-
+#include <linux/clk.h>
 #include <linux/dma-mapping.h>
 
 #define DRV_NAME	"grcan"
@@ -1643,6 +1643,7 @@ static int grcan_probe(struct platform_device *ofdev)
 {
 	struct device_node *np = ofdev->dev.of_node;
 	struct device_node *sysid_parent;
+	struct clk *clk;
 	u32 sysid, ambafreq;
 	int irq, err;
 	void __iomem *base;
@@ -1662,8 +1663,19 @@ static int grcan_probe(struct platform_device *ofdev)
 
 	err = of_property_read_u32(np, "freq", &ambafreq);
 	if (err) {
-		dev_err(&ofdev->dev, "unable to fetch \"freq\" property\n");
-		goto exit_error;
+		clk = devm_clk_get(&ofdev->dev, NULL);
+		if (IS_ERR(clk)) {
+			dev_err_probe(&ofdev->dev, PTR_ERR(clk),
+				      "Failed both to get \"freq\" property and clock for fallback\n");
+			err = PTR_ERR(clk);
+			goto exit_error;
+		}
+
+		ambafreq = clk_get_rate(clk);
+		if (!ambafreq) {
+			dev_err(&ofdev->dev, "Invalid or Uninitialized clock\n");
+			return -EINVAL;
+		}
 	}
 
 	base = devm_platform_ioremap_resource(ofdev, 0);
-- 
2.51.0

[PATCH v7 05/15] can: grcan: Replace bit timing macros with literal values

[Arun Muthusamy]

Refactor the bit timing constants in grcan_bittiming_const by replacing
macros with literal values.

Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
Signed-off-by: Arun Muthusamy <arun.muthusamy@gaisler.com>
---
 drivers/net/can/grcan.c | 27 +++++++++------------------
 1 file changed, 9 insertions(+), 18 deletions(-)

diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
index a5f750d7e2d8..8d7bde1c006f 100644
--- a/drivers/net/can/grcan.c
+++ b/drivers/net/can/grcan.c
@@ -36,6 +36,7 @@
 #include <linux/of_irq.h>
 #include <linux/clk.h>
 #include <linux/dma-mapping.h>
+#include <linux/bitfield.h>
 
 #define DRV_NAME	"grcan"
 
@@ -93,16 +94,6 @@ struct grcan_registers {
 	(GRCAN_CONF_BPR | GRCAN_CONF_RSJ | GRCAN_CONF_PS1		\
 	 | GRCAN_CONF_PS2 | GRCAN_CONF_SCALER)
 
-#define GRCAN_CONF_RSJ_MIN	1
-#define GRCAN_CONF_RSJ_MAX	4
-#define GRCAN_CONF_PS1_MIN	1
-#define GRCAN_CONF_PS1_MAX	15
-#define GRCAN_CONF_PS2_MIN	2
-#define GRCAN_CONF_PS2_MAX	8
-#define GRCAN_CONF_SCALER_MIN	0
-#define GRCAN_CONF_SCALER_MAX	255
-#define GRCAN_CONF_SCALER_INC	1
-
 #define GRCAN_CONF_BPR_BIT	8
 #define GRCAN_CONF_RSJ_BIT	12
 #define GRCAN_CONF_PS1_BIT	20
@@ -392,14 +383,14 @@ static struct grcan_device_config grcan_module_config =
 
 static const struct can_bittiming_const grcan_bittiming_const = {
 	.name		= DRV_NAME,
-	.tseg1_min	= GRCAN_CONF_PS1_MIN + 1,
-	.tseg1_max	= GRCAN_CONF_PS1_MAX + 1,
-	.tseg2_min	= GRCAN_CONF_PS2_MIN,
-	.tseg2_max	= GRCAN_CONF_PS2_MAX,
-	.sjw_max	= GRCAN_CONF_RSJ_MAX,
-	.brp_min	= GRCAN_CONF_SCALER_MIN + 1,
-	.brp_max	= GRCAN_CONF_SCALER_MAX + 1,
-	.brp_inc	= GRCAN_CONF_SCALER_INC,
+	.tseg1_min	= 2,
+	.tseg1_max	= 16,
+	.tseg2_min	= 2,
+	.tseg2_max	= 8,
+	.sjw_max	= 4,
+	.brp_min	= 1,
+	.brp_max	= 256,
+	.brp_inc	= 1,
 };
 
 static int grcan_set_bittiming(struct net_device *dev)
-- 
2.51.0

[PATCH v7 06/15] can: grcan: Simplify timing configuration

[Arun Muthusamy]

Remove redundant error checks and use FIELD_PREP for bit timing
assignments to simplify the timing configuration

Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
Signed-off-by: Arun Muthusamy <arun.muthusamy@gaisler.com>
---
 drivers/net/can/grcan.c | 41 +++++++++++++----------------------------
 1 file changed, 13 insertions(+), 28 deletions(-)

diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
index 8d7bde1c006f..8ba21d94e87d 100644
--- a/drivers/net/can/grcan.c
+++ b/drivers/net/can/grcan.c
@@ -396,41 +396,26 @@ static const struct can_bittiming_const grcan_bittiming_const = {
 static int grcan_set_bittiming(struct net_device *dev)
 {
 	struct grcan_priv *priv = netdev_priv(dev);
-	struct grcan_registers __iomem *regs = priv->regs;
-	struct can_bittiming *bt = &priv->can.bittiming;
-	u32 timing = 0;
+	struct grcan_registers __iomem *regs;
 	int bpr, rsj, ps1, ps2, scaler;
+	struct can_bittiming *bt;
+	u32 timing = 0;
 
-	/* Should never happen - function will not be called when
-	 * device is up
-	 */
-	if (grcan_read_bits(&regs->ctrl, GRCAN_CTRL_ENABLE))
-		return -EBUSY;
+	regs = priv->regs;
+	bt = &priv->can.bittiming;
 
 	bpr = 0; /* Note bpr and brp are different concepts */
 	rsj = bt->sjw;
 	ps1 = (bt->prop_seg + bt->phase_seg1) - 1; /* tseg1 - 1 */
 	ps2 = bt->phase_seg2;
-	scaler = (bt->brp - 1);
-	netdev_dbg(dev, "Request for BPR=%d, RSJ=%d, PS1=%d, PS2=%d, SCALER=%d",
-		   bpr, rsj, ps1, ps2, scaler);
-	if (!(ps1 > ps2)) {
-		netdev_err(dev, "PS1 > PS2 must hold: PS1=%d, PS2=%d\n",
-			   ps1, ps2);
-		return -EINVAL;
-	}
-	if (!(ps2 >= rsj)) {
-		netdev_err(dev, "PS2 >= RSJ must hold: PS2=%d, RSJ=%d\n",
-			   ps2, rsj);
-		return -EINVAL;
-	}
-
-	timing |= (bpr << GRCAN_CONF_BPR_BIT) & GRCAN_CONF_BPR;
-	timing |= (rsj << GRCAN_CONF_RSJ_BIT) & GRCAN_CONF_RSJ;
-	timing |= (ps1 << GRCAN_CONF_PS1_BIT) & GRCAN_CONF_PS1;
-	timing |= (ps2 << GRCAN_CONF_PS2_BIT) & GRCAN_CONF_PS2;
-	timing |= (scaler << GRCAN_CONF_SCALER_BIT) & GRCAN_CONF_SCALER;
-	netdev_info(dev, "setting timing=0x%x\n", timing);
+	scaler = bt->brp - 1;
+
+	timing |= FIELD_PREP(GRCAN_CONF_BPR, bpr);
+	timing |= FIELD_PREP(GRCAN_CONF_RSJ, rsj);
+	timing |= FIELD_PREP(GRCAN_CONF_PS1, ps1);
+	timing |= FIELD_PREP(GRCAN_CONF_PS2, ps2);
+	timing |= FIELD_PREP(GRCAN_CONF_SCALER, scaler);
+	netdev_dbg(dev, "setting timing=0x%x\n", timing);
 	grcan_write_bits(&regs->conf, timing, GRCAN_CONF_TIMING);
 
 	return 0;
-- 
2.51.0

[PATCH v7 07/15] can: grcan: add FD capability detection and nominal bit-timing

[Arun Muthusamy]

Add capability for the driver to detect CAN FD support
and adjust accordingly. Introduce structures and functions
for setting nominal bit-timing for standard CAN FD.
The `grcan_hwcap` structure defines hardware capabilities like
CAN FD support and baud-rate options. Additionally, improved
device tree compatibility by updating the `of_device_id` table
for better matching of GRCAN and GRCANFD devices. Also update
Kconfig to mention GRCANFD support.

Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
Signed-off-by: Arun Muthusamy <arun.muthusamy@gaisler.com>
---
 drivers/net/can/Kconfig |   6 +-
 drivers/net/can/grcan.c | 137 +++++++++++++++++++++++++++++++++++-----
 2 files changed, 125 insertions(+), 18 deletions(-)

diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
index e15e320db476..83b7dc8c925c 100644
--- a/drivers/net/can/Kconfig
+++ b/drivers/net/can/Kconfig
@@ -150,10 +150,12 @@ config CAN_FLEXCAN
 	  Say Y here if you want to support for Freescale FlexCAN.
 
 config CAN_GRCAN
-	tristate "Aeroflex Gaisler GRCAN and GRHCAN CAN devices"
+	tristate "Aeroflex Gaisler GRCAN(FD) and GRHCAN CAN devices"
 	depends on OF && HAS_DMA && HAS_IOMEM
 	help
-	  Say Y here if you want to use Aeroflex Gaisler GRCAN or GRHCAN.
+	  Say Y here if you want to use Aeroflex Gaisler GRCAN or GRCANFD
+	  or GRHCAN.
+
 	  Note that the driver supports little endian, even though little
 	  endian syntheses of the cores would need some modifications on
 	  the hardware level to work.
diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
index 8ba21d94e87d..c9175a6a013f 100644
--- a/drivers/net/can/grcan.c
+++ b/drivers/net/can/grcan.c
@@ -33,6 +33,7 @@
 #include <linux/platform_device.h>
 #include <linux/spinlock.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/of_irq.h>
 #include <linux/clk.h>
 #include <linux/dma-mapping.h>
@@ -51,7 +52,11 @@ struct grcan_registers {
 	u32 __reserved1[GRCAN_RESERVE_SIZE(0x08, 0x18)];
 	u32 smask;	/* 0x18 - CanMASK */
 	u32 scode;	/* 0x1c - CanCODE */
-	u32 __reserved2[GRCAN_RESERVE_SIZE(0x1c, 0x100)];
+	u32 __reserved2[GRCAN_RESERVE_SIZE(0x1c, 0x40)];
+	u32 nbtr;	/* 0x40 */
+	u32 fdbtr;	/* 0x44 */
+	u32 tdelay;	/* 0x48 */
+	u32 __reserved2_[GRCAN_RESERVE_SIZE(0x48, 0x100)];
 	u32 pimsr;	/* 0x100 */
 	u32 pimr;	/* 0x104 */
 	u32 pisr;	/* 0x108 */
@@ -203,6 +208,39 @@ struct grcan_registers {
 #error "Invalid default buffer size"
 #endif
 
+#define GRCANFD_NBTR_SCALER     GENMASK(23, 16)
+#define GRCANFD_NBTR_PS1        GENMASK(17, 10)
+#define GRCANFD_NBTR_PS2        GENMASK(9, 5)
+#define GRCANFD_NBTR_SJW        GENMASK(4, 0)
+#define GRCANFD_NBTR_TIMING						\
+	(GRCANFD_NBTR_SCALER | GRCANFD_NBTR_PS1 | GRCANFD_NBTR_PS2 |	\
+	 GRCANFD_NBTR_SJW)
+
+#define GRCANFD_FDBTR_SCALER 0x00ff0000
+#define GRCANFD_FDBTR_PS1 0x00003c00
+#define GRCANFD_FDBTR_PS2 0x000001e0
+#define GRCANFD_FDBTR_SJW 0x0000000f
+#define GRCANFD_FDBTR_TIMING						\
+	(GRCANFD_FDBTR_SCALER | GRCANFD_FDBTR_PS1 | GRCANFD_FDBTR_PS2 |	\
+	 GRCANFD_FDBTR_SJW)
+
+#define GRCANFD_FDBTR_SCALER_BIT 16
+#define GRCANFD_FDBTR_PS1_BIT 10
+#define GRCANFD_FDBTR_PS2_BIT 5
+#define GRCANFD_FDBTR_SJW_BIT 0
+
+/* Hardware capabilities */
+struct grcan_hwcap {
+	/* CAN-FD capable, indicates GRCANFD IP.
+	 * The GRCANFD has different baud-rate registers and extended DMA
+	 * format to also describe FD-frames.
+	 */
+	const struct can_bittiming_const *bt_const;
+	int (*set_bittiming)(struct net_device *dev);
+	bool txbug_possible;
+	bool fd;
+};
+
 struct grcan_dma_buffer {
 	size_t size;
 	void *buf;
@@ -245,6 +283,7 @@ struct grcan_priv {
 	struct napi_struct napi;
 
 	struct grcan_registers __iomem *regs;	/* ioremap'ed registers */
+	const struct grcan_hwcap *hwcap;
 	struct grcan_device_config config;
 	struct grcan_dma dma;
 
@@ -393,6 +432,19 @@ static const struct can_bittiming_const grcan_bittiming_const = {
 	.brp_inc	= 1,
 };
 
+/* GRCANFD nominal boundaries for baud-rate parameters */
+static const struct can_bittiming_const grcanfd_bittiming_const = {
+	.name		= DRV_NAME,
+	.tseg1_min	= 2,
+	.tseg1_max	= 63,
+	.tseg2_min	= 2,
+	.tseg2_max	= 16,
+	.sjw_max	= 16,
+	.brp_min	= 1,
+	.brp_max	= 256,
+	.brp_inc	= 1,
+};
+
 static int grcan_set_bittiming(struct net_device *dev)
 {
 	struct grcan_priv *priv = netdev_priv(dev);
@@ -421,6 +473,32 @@ static int grcan_set_bittiming(struct net_device *dev)
 	return 0;
 }
 
+static int grcanfd_set_bittiming(struct net_device *dev)
+{
+	struct grcan_priv *priv = netdev_priv(dev);
+	struct grcan_registers __iomem *regs;
+	int sjw, ps1, ps2, scaler;
+	struct can_bittiming *bt;
+	u32 timing = 0;
+
+	regs = priv->regs;
+	bt = &priv->can.bittiming;
+
+	sjw = bt->sjw;
+	ps1 = (bt->prop_seg + bt->phase_seg1);
+	ps2 = bt->phase_seg2;
+	scaler = bt->brp - 1;
+
+	timing |= FIELD_PREP(GRCANFD_NBTR_SJW, sjw);
+	timing |= FIELD_PREP(GRCANFD_NBTR_PS1, ps1);
+	timing |= FIELD_PREP(GRCANFD_NBTR_PS2, ps2);
+	timing |= FIELD_PREP(GRCANFD_NBTR_SCALER, scaler);
+	netdev_dbg(dev, "setting timing=0x%x\n", timing);
+	grcan_write_bits(&regs->nbtr, timing, GRCANFD_NBTR_TIMING);
+
+	return 0;
+}
+
 static int grcan_get_berr_counter(const struct net_device *dev,
 				  struct can_berr_counter *bec)
 {
@@ -1544,7 +1622,8 @@ static const struct ethtool_ops grcan_ethtool_ops = {
 
 static int grcan_setup_netdev(struct platform_device *ofdev,
 			      void __iomem *base,
-			      int irq, u32 ambafreq, bool txbug)
+			      int irq, u32 ambafreq, bool txbug,
+			      const struct grcan_hwcap *hwcap)
 {
 	struct net_device *dev;
 	struct grcan_priv *priv;
@@ -1567,14 +1646,14 @@ static int grcan_setup_netdev(struct platform_device *ofdev,
 	priv->dev = dev;
 	priv->ofdev_dev = &ofdev->dev;
 	priv->regs = base;
-	priv->can.bittiming_const = &grcan_bittiming_const;
-	priv->can.do_set_bittiming = grcan_set_bittiming;
+	priv->can.bittiming_const = hwcap->bt_const;
 	priv->can.do_set_mode = grcan_set_mode;
 	priv->can.do_get_berr_counter = grcan_get_berr_counter;
 	priv->can.clock.freq = ambafreq;
 	priv->can.ctrlmode_supported =
 		CAN_CTRLMODE_LISTENONLY | CAN_CTRLMODE_ONE_SHOT;
 	priv->need_txbug_workaround = txbug;
+	priv->hwcap = hwcap;
 
 	/* Discover if triple sampling is supported by hardware */
 	regs = priv->regs;
@@ -1619,22 +1698,29 @@ static int grcan_probe(struct platform_device *ofdev)
 {
 	struct device_node *np = ofdev->dev.of_node;
 	struct device_node *sysid_parent;
+	const struct grcan_hwcap *hwcap;
 	struct clk *clk;
 	u32 sysid, ambafreq;
 	int irq, err;
 	void __iomem *base;
 	bool txbug = true;
 
+	hwcap = device_get_match_data(&ofdev->dev);
+
 	/* Compare GRLIB version number with the first that does not
 	 * have the tx bug (see start_xmit)
 	 */
-	sysid_parent = of_find_node_by_path("/ambapp0");
-	if (sysid_parent) {
-		err = of_property_read_u32(sysid_parent, "systemid", &sysid);
-		if (!err && ((sysid & GRLIB_VERSION_MASK) >=
-			     GRCAN_TXBUG_SAFE_GRLIB_VERSION))
-			txbug = false;
-		of_node_put(sysid_parent);
+	if (!hwcap->txbug_possible) {
+		txbug = false;
+	} else {
+		sysid_parent = of_find_node_by_path("/ambapp0");
+		if (sysid_parent) {
+			err = of_property_read_u32(sysid_parent, "systemid", &sysid);
+			if (!err && ((sysid & GRLIB_VERSION_MASK) >=
+				     GRCAN_TXBUG_SAFE_GRLIB_VERSION))
+				txbug = false;
+			of_node_put(sysid_parent);
+		}
 	}
 
 	err = of_property_read_u32(np, "freq", &ambafreq);
@@ -1669,7 +1755,7 @@ static int grcan_probe(struct platform_device *ofdev)
 
 	grcan_sanitize_module_config(ofdev);
 
-	err = grcan_setup_netdev(ofdev, base, irq, ambafreq, txbug);
+	err = grcan_setup_netdev(ofdev, base, irq, ambafreq, txbug, hwcap);
 	if (err)
 		goto exit_dispose_irq;
 
@@ -1696,11 +1782,30 @@ static void grcan_remove(struct platform_device *ofdev)
 	free_candev(dev);
 }
 
+static const struct grcan_hwcap grcan_hwcap = {
+	.fd = false,
+	.txbug_possible = true,
+	.bt_const = &grcan_bittiming_const,
+	.set_bittiming = grcan_set_bittiming,
+};
+
+static const struct grcan_hwcap grcanfd_hwcap = {
+	.fd = true,
+	.txbug_possible = false,
+	.bt_const = &grcanfd_bittiming_const,
+	.set_bittiming = grcanfd_set_bittiming,
+};
+
 static const struct of_device_id grcan_match[] = {
-	{.name = "GAISLER_GRCAN"},
-	{.name = "01_03d"},
-	{.name = "GAISLER_GRHCAN"},
-	{.name = "01_034"},
+	{.name = "GAISLER_GRCAN", .data = &grcan_hwcap},
+	{.name = "01_03d", .data = &grcan_hwcap},
+	{.name = "GAISLER_GRHCAN", .data = &grcan_hwcap},
+	{.name = "01_034", .data = &grcan_hwcap},
+	{.compatible = "gaisler,grcan", .data = &grcan_hwcap},
+	/* GRCANFD */
+	{.compatible = "gaisler,grcanfd", .data = &grcanfd_hwcap},
+	{.name = "GAISLER_GRCANFD", .data = &grcanfd_hwcap},
+	{.name = "01_0b5", .data = &grcanfd_hwcap},
 	{},
 };
 
-- 
2.51.0

[PATCH v7 08/15] can: grcan: set DMA mask for GRCAN and GRCANFD to 32-bit

[Arun Muthusamy]

From: Daniel Hellstrom <daniel@gaisler.com>

Sets the DMA mask for GRCAN and GRCANFD devices to 32-bit.
Setting the DMA mask and coherent DMA mask to 32-bit ensures proper
memory addressing during DMA operations

Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
Signed-off-by: Arun Muthusamy <arun.muthusamy@gaisler.com>
---
 drivers/net/can/grcan.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
index c9175a6a013f..6e0a393785e8 100644
--- a/drivers/net/can/grcan.c
+++ b/drivers/net/can/grcan.c
@@ -1746,6 +1746,13 @@ static int grcan_probe(struct platform_device *ofdev)
 		goto exit_error;
 	}
 
+	/* On 64-bit systems.. GRCAN and GRCANFD can only address 32-bit */
+	err = dma_set_mask_and_coherent(&ofdev->dev, DMA_BIT_MASK(32));
+	if (err) {
+		dev_err_probe(&ofdev->dev, err, "No usable DMA configuration.\n");
+		goto exit_error;
+	}
+
 	irq = irq_of_parse_and_map(np, GRCAN_IRQIX_IRQ);
 	if (!irq) {
 		dev_err(&ofdev->dev, "no irq found\n");
-- 
2.51.0

[PATCH v7 09/15] can: grcan: Add saving and restoring of CAN FD baud-rate registers

[Arun Muthusamy]

From: Daniel Hellstrom <daniel@gaisler.com>

While reset the GRCAN baud-rates are preserved, since GRCANFD has the
baud-rate in different registers we need to add saving of those
registers too.

Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
Signed-off-by: Arun Muthusamy <arun.muthusamy@gaisler.com>
---
 drivers/net/can/grcan.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
index 6e0a393785e8..ccf4032ed642 100644
--- a/drivers/net/can/grcan.c
+++ b/drivers/net/can/grcan.c
@@ -528,6 +528,8 @@ static void grcan_reset(struct net_device *dev)
 
 	/* Turn off hardware filtering - regs->rxcode set to 0 by reset */
 	grcan_write_reg(&regs->rxmask, 0);
+
+	priv->hwcap->set_bittiming(dev);
 }
 
 /* stop device without changing any configurations */
-- 
2.51.0

[PATCH v7 10/15] can: grcan: Reserve space between cap and next register to align with address layout

[Arun Muthusamy]

From: Daniel Hellstrom <daniel@gaisler.com>

Reserves space between the capability register and the next register
within the GRCAN driver to align with the hardware address layout.

Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
Signed-off-by: Arun Muthusamy <arun.muthusamy@gaisler.com>
---
 drivers/net/can/grcan.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
index ccf4032ed642..b5527fd9e02e 100644
--- a/drivers/net/can/grcan.c
+++ b/drivers/net/can/grcan.c
@@ -49,7 +49,8 @@ struct grcan_registers {
 	u32 conf;	/* 0x00 */
 	u32 stat;	/* 0x04 */
 	u32 ctrl;	/* 0x08 */
-	u32 __reserved1[GRCAN_RESERVE_SIZE(0x08, 0x18)];
+	u32 cap;	/* 0x0c */
+	u32 __reserved1[GRCAN_RESERVE_SIZE(0x0c, 0x18)];
 	u32 smask;	/* 0x18 - CanMASK */
 	u32 scode;	/* 0x1c - CanCODE */
 	u32 __reserved2[GRCAN_RESERVE_SIZE(0x1c, 0x40)];
-- 
2.51.0

[PATCH v7 11/15] can: grcan: Refactor GRCAN DMA buffer to use structured memory layout

[Arun Muthusamy]

- Introduce a structured layout to represent hardware memory using
  struct grcan_msg_header and struct grcan_msg_fragment.
- Updated the GRCAN receive and transmit functions to utilize this
  new structured approach.
- Implemented helper functions grcan_msg_header_at for accessing
  header structures within DMA buffers.
- Replaced array and bitwise operations with memcpy for copying
  between CAN frames and header structures.

Signed-off-by: Arun Muthusamy <arun.muthusamy@gaisler.com>
---
 drivers/net/can/grcan.c | 80 +++++++++++++++++++++++++----------------
 1 file changed, 49 insertions(+), 31 deletions(-)

diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
index b5527fd9e02e..4921da349a0e 100644
--- a/drivers/net/can/grcan.c
+++ b/drivers/net/can/grcan.c
@@ -195,9 +195,6 @@ struct grcan_registers {
 #define GRCAN_MSG_OFF		0x00000002
 #define GRCAN_MSG_PASS		0x00000001
 
-#define GRCAN_MSG_DATA_SLOT_INDEX(i) (2 + (i) / 4)
-#define GRCAN_MSG_DATA_SHIFT(i) ((3 - (i) % 4) * 8)
-
 #define GRCAN_BUFFER_ALIGNMENT		1024
 #define GRCAN_DEFAULT_BUFFER_SIZE	1024
 #define GRCAN_VALID_TR_SIZE_MASK	0x001fffc0
@@ -242,9 +239,23 @@ struct grcan_hwcap {
 	bool fd;
 };
 
+struct grcan_msg_header {
+	u32 id;
+	u32 ctrl;
+	u8 data[CAN_MAX_DLEN];
+} __packed;
+
+struct grcan_msg_fragment {
+	u8 data[GRCAN_MSG_SIZE];
+} __packed;
+
 struct grcan_dma_buffer {
 	size_t size;
 	void *buf;
+	union {
+		struct grcan_msg_header *header;
+		struct grcan_msg_fragment *frag;
+	};
 	dma_addr_t handle;
 };
 
@@ -417,6 +428,12 @@ static inline u32 grcan_txspace(size_t txsize, u32 txwr, u32 eskbp)
 	return slots - used;
 }
 
+static inline struct grcan_msg_header *
+grcan_msg_header_at(struct grcan_dma_buffer *dbuf, u32 offset)
+{
+	return (struct grcan_msg_header *)((u8 *)dbuf->buf + offset);
+}
+
 /* Configuration parameters that can be set via module parameters */
 static struct grcan_device_config grcan_module_config =
 	GRCAN_DEFAULT_DEVICE_CONFIG;
@@ -1215,11 +1232,11 @@ static int grcan_receive(struct net_device *dev, int budget)
 	struct grcan_registers __iomem *regs = priv->regs;
 	struct grcan_dma *dma = &priv->dma;
 	struct net_device_stats *stats = &dev->stats;
+	struct grcan_msg_header *hdr;
 	struct can_frame *cf;
 	struct sk_buff *skb;
 	u32 wr, rd, startrd;
-	u32 *slot;
-	u32 i, rtr, eff, j, shift;
+	u32 rtr, eff;
 	int work_done = 0;
 
 	rd = grcan_read_reg(&regs->rxrd);
@@ -1239,30 +1256,33 @@ static int grcan_receive(struct net_device *dev, int budget)
 			continue;
 		}
 
-		slot = dma->rx.buf + rd;
-		eff = slot[0] & GRCAN_MSG_IDE;
-		rtr = slot[0] & GRCAN_MSG_RTR;
+		hdr = grcan_msg_header_at(&dma->rx, rd);
+
+		eff = hdr->id & GRCAN_MSG_IDE;
+		rtr = hdr->id & GRCAN_MSG_RTR;
+
 		if (eff) {
-			cf->can_id = ((slot[0] & GRCAN_MSG_EID)
+			cf->can_id = ((hdr->id & GRCAN_MSG_EID)
 				      >> GRCAN_MSG_EID_BIT);
 			cf->can_id |= CAN_EFF_FLAG;
 		} else {
-			cf->can_id = ((slot[0] & GRCAN_MSG_BID)
+			cf->can_id = ((hdr->id & GRCAN_MSG_BID)
 				      >> GRCAN_MSG_BID_BIT);
 		}
-		cf->len = can_cc_dlc2len((slot[1] & GRCAN_MSG_DLC)
-					  >> GRCAN_MSG_DLC_BIT);
+
+		cf->len = can_cc_dlc2len((hdr->ctrl & GRCAN_MSG_DLC)
+					 >> GRCAN_MSG_DLC_BIT);
+
 		if (rtr) {
 			cf->can_id |= CAN_RTR_FLAG;
 		} else {
-			for (i = 0; i < cf->len; i++) {
-				j = GRCAN_MSG_DATA_SLOT_INDEX(i);
-				shift = GRCAN_MSG_DATA_SHIFT(i);
-				cf->data[i] = (u8)(slot[j] >> shift);
-			}
+			if (cf->len > 0)
+				memcpy(cf->data, hdr->data,
+				       min_t(u32, cf->len, CAN_MAX_DLEN));
 
 			stats->rx_bytes += cf->len;
 		}
+
 		stats->rx_packets++;
 
 		netif_receive_skb(skb);
@@ -1395,11 +1415,10 @@ static netdev_tx_t grcan_start_xmit(struct sk_buff *skb,
 	struct grcan_registers __iomem *regs = priv->regs;
 	struct grcan_dma *dma = &priv->dma;
 	struct can_frame *cf = (struct can_frame *)skb->data;
+	struct grcan_msg_header *hdr;
 	u32 id, txwr, txrd, space, txctrl;
 	int slotindex;
-	u32 *slot;
-	u32 i, rtr, eff, dlc, tmp, err;
-	int j, shift;
+	u32 rtr, eff, dlc, tmp, err;
 	unsigned long flags;
 	u32 oneshotmode = priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT;
 
@@ -1422,7 +1441,6 @@ static netdev_tx_t grcan_start_xmit(struct sk_buff *skb,
 	space = grcan_txspace(dma->tx.size, txwr, priv->eskbp);
 
 	slotindex = txwr / GRCAN_MSG_SIZE;
-	slot = dma->tx.buf + txwr;
 
 	if (unlikely(space == 1))
 		netif_stop_queue(dev);
@@ -1438,6 +1456,9 @@ static netdev_tx_t grcan_start_xmit(struct sk_buff *skb,
 		return NETDEV_TX_BUSY;
 	}
 
+	hdr = grcan_msg_header_at(&dma->tx, txwr);
+	memset(hdr, 0, sizeof(*hdr));
+
 	/* Convert and write CAN message to DMA buffer */
 	eff = cf->can_id & CAN_EFF_FLAG;
 	rtr = cf->can_id & CAN_RTR_FLAG;
@@ -1447,16 +1468,13 @@ static netdev_tx_t grcan_start_xmit(struct sk_buff *skb,
 		tmp = (id << GRCAN_MSG_EID_BIT) & GRCAN_MSG_EID;
 	else
 		tmp = (id << GRCAN_MSG_BID_BIT) & GRCAN_MSG_BID;
-	slot[0] = (eff ? GRCAN_MSG_IDE : 0) | (rtr ? GRCAN_MSG_RTR : 0) | tmp;
-
-	slot[1] = ((dlc << GRCAN_MSG_DLC_BIT) & GRCAN_MSG_DLC);
-	slot[2] = 0;
-	slot[3] = 0;
-	for (i = 0; i < dlc; i++) {
-		j = GRCAN_MSG_DATA_SLOT_INDEX(i);
-		shift = GRCAN_MSG_DATA_SHIFT(i);
-		slot[j] |= cf->data[i] << shift;
-	}
+
+	hdr->id = (eff ? GRCAN_MSG_IDE : 0) | (rtr ? GRCAN_MSG_RTR : 0) | tmp;
+
+	hdr->ctrl = ((dlc << GRCAN_MSG_DLC_BIT) & GRCAN_MSG_DLC);
+
+	if (dlc > 0)
+		memcpy(hdr->data, cf->data, min_t(u32, cf->len, CAN_MAX_DLEN));
 
 	/* Checking that channel has not been disabled. These cases
 	 * should never happen
-- 
2.51.0

[PATCH v7 12/15] can: grcan: Add CANFD TX support alongside legacy CAN

[Arun Muthusamy]

Include CANFD TX support with the legacy CAN support, enabling
support for extended data payloads to provide higher bit rates.

Signed-off-by: Arun Muthusamy <arun.muthusamy@gaisler.com>
---
 drivers/net/can/grcan.c | 117 +++++++++++++++++++++++++++++-----------
 1 file changed, 87 insertions(+), 30 deletions(-)

diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
index 4921da349a0e..690e07c7b1ef 100644
--- a/drivers/net/can/grcan.c
+++ b/drivers/net/can/grcan.c
@@ -174,6 +174,7 @@ struct grcan_registers {
 #define GRCAN_IRQ_DEFAULT (GRCAN_IRQ_RX | GRCAN_IRQ_TX | GRCAN_IRQ_ERRORS)
 
 #define GRCAN_MSG_SIZE		16
+#define GRCAN_CLASSIC_DATA_SIZE 8
 
 #define GRCAN_MSG_IDE		0x80000000
 #define GRCAN_MSG_RTR		0x40000000
@@ -195,6 +196,10 @@ struct grcan_registers {
 #define GRCAN_MSG_OFF		0x00000002
 #define GRCAN_MSG_PASS		0x00000001
 
+#define GRCAN_MSG_EID_MASK      GENMASK(28, 0)
+#define GRCAN_MSG_BID_MASK      GENMASK(28, 18)
+#define GRCAN_MSG_DLC_MASK      GENMASK(31, 28)
+
 #define GRCAN_BUFFER_ALIGNMENT		1024
 #define GRCAN_DEFAULT_BUFFER_SIZE	1024
 #define GRCAN_VALID_TR_SIZE_MASK	0x001fffc0
@@ -227,6 +232,9 @@ struct grcan_registers {
 #define GRCANFD_FDBTR_PS2_BIT 5
 #define GRCANFD_FDBTR_SJW_BIT 0
 
+#define GRCAN_TX_BRS  BIT(25)
+#define GRCAN_TX_FDF  BIT(26)
+
 /* Hardware capabilities */
 struct grcan_hwcap {
 	/* CAN-FD capable, indicates GRCANFD IP.
@@ -434,6 +442,12 @@ grcan_msg_header_at(struct grcan_dma_buffer *dbuf, u32 offset)
 	return (struct grcan_msg_header *)((u8 *)dbuf->buf + offset);
 }
 
+static inline struct grcan_msg_fragment *
+grcan_msg_frag_at(struct grcan_dma_buffer *dbuf, u32 offset)
+{
+	return (struct grcan_msg_fragment *)((u8 *)dbuf->buf + offset);
+}
+
 /* Configuration parameters that can be set via module parameters */
 static struct grcan_device_config grcan_module_config =
 	GRCAN_DEFAULT_DEVICE_CONFIG;
@@ -1226,6 +1240,14 @@ static void grcan_transmit_catch_up(struct net_device *dev)
 	spin_unlock_irqrestore(&priv->lock, flags);
 }
 
+static int grcan_numbds(int len)
+{
+	if (len <= GRCAN_CLASSIC_DATA_SIZE)
+		return 1;
+
+	return 1 + DIV_ROUND_UP(len - GRCAN_CLASSIC_DATA_SIZE, GRCAN_MSG_SIZE);
+}
+
 static int grcan_receive(struct net_device *dev, int budget)
 {
 	struct grcan_priv *priv = netdev_priv(dev);
@@ -1412,15 +1434,24 @@ static netdev_tx_t grcan_start_xmit(struct sk_buff *skb,
 				    struct net_device *dev)
 {
 	struct grcan_priv *priv = netdev_priv(dev);
-	struct grcan_registers __iomem *regs = priv->regs;
+	struct grcan_registers __iomem *regs;
+	u32 eff, rtr, dlc, tmp, err, can_id;
 	struct grcan_dma *dma = &priv->dma;
-	struct can_frame *cf = (struct can_frame *)skb->data;
-	struct grcan_msg_header *hdr;
+	u32 bds, copy_len, payload_offset;
 	u32 id, txwr, txrd, space, txctrl;
-	int slotindex;
-	u32 rtr, eff, dlc, tmp, err;
+	struct grcan_msg_fragment *frag;
+	struct grcan_msg_header *hdr;
+	struct canfd_frame *cfd;
+	struct can_frame *cf;
 	unsigned long flags;
-	u32 oneshotmode = priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT;
+	u32 oneshotmode;
+	int slotindex;
+	u8 *payload;
+	u8 len;
+	int i;
+
+	regs = priv->regs;
+	oneshotmode = priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT;
 
 	if (can_dev_dropped_skb(dev, skb))
 		return NETDEV_TX_OK;
@@ -1431,6 +1462,18 @@ static netdev_tx_t grcan_start_xmit(struct sk_buff *skb,
 	if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
 		return NETDEV_TX_BUSY;
 
+	cfd = (struct canfd_frame *)skb->data;
+	len = cfd->len;
+	can_id  = cfd->can_id;
+	payload = cfd->data;
+
+	if (can_is_canfd_skb(skb)) {
+		dlc = can_fd_len2dlc(len);
+	} else {
+		cf = (struct can_frame *)skb->data;
+		dlc = can_get_cc_dlc(cf, priv->can.ctrlmode);
+	}
+
 	/* Reads of priv->eskbp and shut-downs of the queue needs to
 	 * be atomic towards the updates to priv->eskbp and wake-ups
 	 * of the queue in the interrupt handler.
@@ -1441,40 +1484,55 @@ static netdev_tx_t grcan_start_xmit(struct sk_buff *skb,
 	space = grcan_txspace(dma->tx.size, txwr, priv->eskbp);
 
 	slotindex = txwr / GRCAN_MSG_SIZE;
+	bds = grcan_numbds(len);
 
-	if (unlikely(space == 1))
+	if (unlikely(space < bds)) {
 		netif_stop_queue(dev);
+		spin_unlock_irqrestore(&priv->lock, flags);
+		return NETDEV_TX_BUSY;
+	}
 
 	spin_unlock_irqrestore(&priv->lock, flags);
 	/* End of critical section*/
 
-	/* This should never happen. If circular buffer is full, the
-	 * netif_stop_queue should have been stopped already.
-	 */
-	if (unlikely(!space)) {
-		netdev_err(dev, "No buffer space, but queue is non-stopped.\n");
-		return NETDEV_TX_BUSY;
-	}
-
 	hdr = grcan_msg_header_at(&dma->tx, txwr);
 	memset(hdr, 0, sizeof(*hdr));
 
-	/* Convert and write CAN message to DMA buffer */
-	eff = cf->can_id & CAN_EFF_FLAG;
-	rtr = cf->can_id & CAN_RTR_FLAG;
-	id = cf->can_id & (eff ? CAN_EFF_MASK : CAN_SFF_MASK);
-	dlc = cf->len;
-	if (eff)
-		tmp = (id << GRCAN_MSG_EID_BIT) & GRCAN_MSG_EID;
-	else
-		tmp = (id << GRCAN_MSG_BID_BIT) & GRCAN_MSG_BID;
+	eff = can_id & CAN_EFF_FLAG;
+	rtr = can_id & CAN_RTR_FLAG;
+	id  = can_id & (eff ? CAN_EFF_MASK : CAN_SFF_MASK);
+
+	tmp = eff ? FIELD_PREP(GRCAN_MSG_EID_MASK, id)
+		  : FIELD_PREP(GRCAN_MSG_BID_MASK, id);
+
+	hdr->id = (eff ? GRCAN_MSG_IDE : 0) |
+		  (rtr ? GRCAN_MSG_RTR : 0) |
+		  tmp;
 
-	hdr->id = (eff ? GRCAN_MSG_IDE : 0) | (rtr ? GRCAN_MSG_RTR : 0) | tmp;
+	hdr->ctrl = FIELD_PREP(GRCAN_MSG_DLC_MASK, dlc);
+
+	if (can_is_canfd_skb(skb)) {
+		hdr->ctrl |= GRCAN_TX_FDF;
+		if (cfd->flags & CANFD_BRS)
+			hdr->ctrl |= GRCAN_TX_BRS;
+	}
 
-	hdr->ctrl = ((dlc << GRCAN_MSG_DLC_BIT) & GRCAN_MSG_DLC);
+	copy_len = min_t(u32, len, CAN_MAX_DLEN);
+	memcpy(hdr->data, payload, copy_len);
+	payload_offset = copy_len;
 
-	if (dlc > 0)
-		memcpy(hdr->data, cf->data, min_t(u32, cf->len, CAN_MAX_DLEN));
+	txwr = grcan_ring_add(txwr, GRCAN_MSG_SIZE, dma->tx.size);
+
+	for (i = 1; i < bds; i++) {
+		frag = grcan_msg_frag_at(&dma->tx, txwr);
+
+		memset(frag, 0, sizeof(*frag));
+		copy_len = min_t(u32, (u32)len - payload_offset, (u32)GRCAN_MSG_SIZE);
+		memcpy(frag->data, payload + payload_offset, copy_len);
+		payload_offset += copy_len;
+
+		txwr = grcan_ring_add(txwr, GRCAN_MSG_SIZE, dma->tx.size);
+	}
 
 	/* Checking that channel has not been disabled. These cases
 	 * should never happen
@@ -1516,8 +1574,7 @@ static netdev_tx_t grcan_start_xmit(struct sk_buff *skb,
 	wmb();
 
 	/* Update write pointer to start transmission */
-	grcan_write_reg(&regs->txwr,
-			grcan_ring_add(txwr, GRCAN_MSG_SIZE, dma->tx.size));
+	grcan_write_reg(&regs->txwr, txwr);
 
 	return NETDEV_TX_OK;
 }
-- 
2.51.0

[PATCH v7 13/15] can: grcan: Add CANFD RX support alongside legacy CAN

[Arun Muthusamy]

Include CANFD RX support with the legacy CAN support, enabling
support for extended data payloads to provide higher bit rates.

Signed-off-by: Arun Muthusamy <arun.muthusamy@gaisler.com>
---
 drivers/net/can/grcan.c | 72 +++++++++++++++++++++++++++--------------
 1 file changed, 47 insertions(+), 25 deletions(-)

diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
index 690e07c7b1ef..eba52701c882 100644
--- a/drivers/net/can/grcan.c
+++ b/drivers/net/can/grcan.c
@@ -235,6 +235,9 @@ struct grcan_registers {
 #define GRCAN_TX_BRS  BIT(25)
 #define GRCAN_TX_FDF  BIT(26)
 
+#define GRCAN_RX_BRS  BIT(25)
+#define GRCAN_RX_FDF  BIT(26)
+
 /* Hardware capabilities */
 struct grcan_hwcap {
 	/* CAN-FD capable, indicates GRCANFD IP.
@@ -1250,17 +1253,21 @@ static int grcan_numbds(int len)
 
 static int grcan_receive(struct net_device *dev, int budget)
 {
+	struct net_device_stats *stats = &dev->stats;
 	struct grcan_priv *priv = netdev_priv(dev);
-	struct grcan_registers __iomem *regs = priv->regs;
+	struct grcan_registers __iomem *regs;
 	struct grcan_dma *dma = &priv->dma;
-	struct net_device_stats *stats = &dev->stats;
+	u32 bds, copy_len, payload_offset;
+	struct grcan_msg_fragment *frag;
 	struct grcan_msg_header *hdr;
-	struct can_frame *cf;
+	u32 wr, rd, dlc, startrd;
+	struct canfd_frame *cf;
+	int i, work_done = 0;
 	struct sk_buff *skb;
-	u32 wr, rd, startrd;
 	u32 rtr, eff;
-	int work_done = 0;
+	u8 *data;
 
+	regs = priv->regs;
 	rd = grcan_read_reg(&regs->rxrd);
 	startrd = rd;
 	for (work_done = 0; work_done < budget; work_done++) {
@@ -1269,47 +1276,62 @@ static int grcan_receive(struct net_device *dev, int budget)
 		if (rd == wr)
 			break;
 
-		/* Take care of packet */
-		skb = alloc_can_skb(dev, &cf);
-		if (skb == NULL) {
-			netdev_err(dev,
-				   "dropping frame: skb allocation failed\n");
+		hdr = grcan_msg_header_at(&dma->rx, rd);
+		if (hdr->ctrl & GRCAN_RX_FDF)
+			skb = alloc_canfd_skb(dev, &cf);
+		else
+			skb = alloc_can_skb(dev, (struct can_frame **)&cf);
+
+		if (unlikely(!skb)) {
+			netdev_err(dev, "dropping frame: skb allocation failed\n");
 			stats->rx_dropped++;
 			continue;
 		}
 
-		hdr = grcan_msg_header_at(&dma->rx, rd);
+		dlc = FIELD_GET(GRCAN_MSG_DLC_MASK, hdr->ctrl);
+		if (hdr->ctrl & GRCAN_RX_FDF)
+			cf->len = can_fd_dlc2len(dlc);
+		else
+			cf->len = can_cc_dlc2len(dlc);
+
+		bds = grcan_numbds(cf->len);
+		payload_offset = 0;
+		data = cf->data;
 
 		eff = hdr->id & GRCAN_MSG_IDE;
 		rtr = hdr->id & GRCAN_MSG_RTR;
 
 		if (eff) {
-			cf->can_id = ((hdr->id & GRCAN_MSG_EID)
-				      >> GRCAN_MSG_EID_BIT);
+			cf->can_id = FIELD_GET(GRCAN_MSG_EID_MASK, hdr->id);
 			cf->can_id |= CAN_EFF_FLAG;
 		} else {
-			cf->can_id = ((hdr->id & GRCAN_MSG_BID)
-				      >> GRCAN_MSG_BID_BIT);
+			cf->can_id = FIELD_GET(GRCAN_MSG_BID_MASK, hdr->id);
 		}
-
-		cf->len = can_cc_dlc2len((hdr->ctrl & GRCAN_MSG_DLC)
-					 >> GRCAN_MSG_DLC_BIT);
-
 		if (rtr) {
 			cf->can_id |= CAN_RTR_FLAG;
+			rd = grcan_ring_add(rd, GRCAN_MSG_SIZE, dma->rx.size);
 		} else {
-			if (cf->len > 0)
-				memcpy(cf->data, hdr->data,
-				       min_t(u32, cf->len, CAN_MAX_DLEN));
+			copy_len = min_t(u32, cf->len, CAN_MAX_DLEN);
+			memcpy(data, hdr->data, copy_len);
+			payload_offset += copy_len;
+
+			rd = grcan_ring_add(rd, GRCAN_MSG_SIZE, dma->rx.size);
 
+			for (i = 1; i < bds; i++) {
+				frag = grcan_msg_frag_at(&dma->rx, rd);
+
+				copy_len = min_t(u32, (u32)cf->len - payload_offset,
+						 (u32)GRCAN_MSG_SIZE);
+				memcpy(data + payload_offset, frag->data, copy_len);
+				payload_offset += copy_len;
+
+				rd = grcan_ring_add(rd, GRCAN_MSG_SIZE, dma->rx.size);
+			}
 			stats->rx_bytes += cf->len;
 		}
 
 		stats->rx_packets++;
-
 		netif_receive_skb(skb);
-
-		rd = grcan_ring_add(rd, GRCAN_MSG_SIZE, dma->rx.size);
 	}
 
 	/* Make sure everything is read before allowing hardware to
-- 
2.51.0

[PATCH v7 14/15] can: grcan: Update echo skb handling to match variable length CANFD frame

[Arun Muthusamy]

Refactor echo socket buffer management by introducing dedicated indices
for current and next echo slots.

- Introduce "echo_skb_idx" to keep track of the current packet index in
  the echo buffer, and "next_echo_idx" for the next available slot.
- Adjust memory allocation for echo skb to calculate the number of slots
  based on slot size.
- Enhance logic in catch_up_echo_skb() to correctly process and free echo
  skbs.
- Initialize "next_echo_idx" in grcan_set_mode() to ensure proper starting
  conditions when the device enters proper modes.
- Improve memory and index handling in grcan_start_xmit() and
  added a check to stop the network queue when necessary.

Signed-off-by: Arun Muthusamy <arun.muthusamy@gaisler.com>
---
 drivers/net/can/grcan.c | 53 ++++++++++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 17 deletions(-)

diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
index eba52701c882..06ef081970a2 100644
--- a/drivers/net/can/grcan.c
+++ b/drivers/net/can/grcan.c
@@ -312,6 +312,15 @@ struct grcan_priv {
 
 	struct sk_buff **echo_skb;	/* We allocate this on our own */
 
+	/*
+	 * Since the CAN FD frame has a variable length, this variable is used
+	 * to keep track of the index of the CAN echo skb (socket buffer) frame.
+	 */
+	u32 echo_skb_idx;
+
+	/* Next echo skb free slot index */
+	u32 next_echo_idx;
+
 	/* The echo skb pointer, pointing into echo_skb and indicating which
 	 * frames can be echoed back. See the "Notes on the tx cyclic buffer
 	 * handling"-comment for grcan_start_xmit for more details.
@@ -593,7 +602,7 @@ static int catch_up_echo_skb(struct net_device *dev, int budget, bool echo)
 	struct grcan_registers __iomem *regs = priv->regs;
 	struct grcan_dma *dma = &priv->dma;
 	struct net_device_stats *stats = &dev->stats;
-	int i, work_done;
+	int work_done;
 
 	/* Updates to priv->eskbp and wake-ups of the queue needs to
 	 * be atomic towards the reads of priv->eskbp and shut-downs
@@ -604,19 +613,22 @@ static int catch_up_echo_skb(struct net_device *dev, int budget, bool echo)
 	for (work_done = 0; work_done < budget || budget < 0; work_done++) {
 		if (priv->eskbp == txrd)
 			break;
-		i = priv->eskbp / GRCAN_MSG_SIZE;
-		if (echo) {
-			/* Normal echo of messages */
-			stats->tx_packets++;
-			stats->tx_bytes += can_get_echo_skb(dev, i, NULL);
-		} else {
-			/* For cleanup of untransmitted messages */
-			can_free_echo_skb(dev, i, NULL);
-		}
 
 		priv->eskbp = grcan_ring_add(priv->eskbp, GRCAN_MSG_SIZE,
 					     dma->tx.size);
 		txrd = grcan_read_reg(&regs->txrd);
+
+		/* Grab the packet once the packet is send or free untransmitted packet */
+		if (priv->eskbp == txrd) {
+			if (echo) {
+				/* Normal echo of messages */
+				stats->tx_packets++;
+				stats->tx_bytes += can_get_echo_skb(dev, priv->echo_skb_idx, NULL);
+			} else {
+				/* For cleanup of untransmitted messages */
+				can_free_echo_skb(dev, priv->echo_skb_idx, NULL);
+			}
+		}
 	}
 	return work_done;
 }
@@ -1124,6 +1136,7 @@ static int grcan_set_mode(struct net_device *dev, enum can_mode mode)
 			if (!(priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY))
 				netif_wake_queue(dev);
 		}
+		priv->next_echo_idx = 0;
 		spin_unlock_irqrestore(&priv->lock, flags);
 		return err;
 	}
@@ -1135,6 +1148,7 @@ static int grcan_open(struct net_device *dev)
 	struct grcan_priv *priv = netdev_priv(dev);
 	struct grcan_dma *dma = &priv->dma;
 	unsigned long flags;
+	u32 nr_echo_slots;
 	int err;
 
 	/* Allocate memory */
@@ -1145,12 +1159,14 @@ static int grcan_open(struct net_device *dev)
 		return err;
 	}
 
-	priv->echo_skb = kzalloc_objs(*priv->echo_skb, dma->tx.size);
+	nr_echo_slots = dma->tx.size / GRCAN_MSG_SIZE;
+
+	priv->echo_skb = kzalloc_objs(*priv->echo_skb, nr_echo_slots);
 	if (!priv->echo_skb) {
 		err = -ENOMEM;
 		goto exit_free_dma_buffers;
 	}
-	priv->can.echo_skb_max = dma->tx.size;
+	priv->can.echo_skb_max = nr_echo_slots;
 	priv->can.echo_skb = priv->echo_skb;
 
 	/* Get can device up */
@@ -1467,7 +1483,6 @@ static netdev_tx_t grcan_start_xmit(struct sk_buff *skb,
 	struct can_frame *cf;
 	unsigned long flags;
 	u32 oneshotmode;
-	int slotindex;
 	u8 *payload;
 	u8 len;
 	int i;
@@ -1504,11 +1519,9 @@ static netdev_tx_t grcan_start_xmit(struct sk_buff *skb,
 
 	txwr = grcan_read_reg(&regs->txwr);
 	space = grcan_txspace(dma->tx.size, txwr, priv->eskbp);
-
-	slotindex = txwr / GRCAN_MSG_SIZE;
 	bds = grcan_numbds(len);
 
-	if (unlikely(space < bds)) {
+	if (unlikely(space < bds || priv->can.echo_skb[priv->echo_skb_idx])) {
 		netif_stop_queue(dev);
 		spin_unlock_irqrestore(&priv->lock, flags);
 		return NETDEV_TX_BUSY;
@@ -1588,7 +1601,13 @@ static netdev_tx_t grcan_start_xmit(struct sk_buff *skb,
 	 * can_put_echo_skb would be an error unless other measures are
 	 * taken.
 	 */
-	can_put_echo_skb(skb, dev, slotindex, 0);
+
+	priv->echo_skb_idx = priv->next_echo_idx;
+
+	can_put_echo_skb(skb, dev, priv->next_echo_idx, 0);
+
+	/* Move to the next index in the echo skb buffer */
+	priv->next_echo_idx = (priv->next_echo_idx + 1) % priv->can.echo_skb_max;
 
 	/* Make sure everything is written before allowing hardware to
 	 * read from the memory
-- 
2.51.0

[PATCH v7 15/15] can: grcan: Advertise CANFD capability

[Arun Muthusamy]

Advertise CANFD capability for GRCANFD hardware by enabling
CAN_CTRLMODE_FD flag and providing CANFD timing operations.

Signed-off-by: Arun Muthusamy <arun.muthusamy@gaisler.com>
---
 drivers/net/can/grcan.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
index 06ef081970a2..060031df8192 100644
--- a/drivers/net/can/grcan.c
+++ b/drivers/net/can/grcan.c
@@ -1766,11 +1766,14 @@ static int grcan_setup_netdev(struct platform_device *ofdev,
 	priv->ofdev_dev = &ofdev->dev;
 	priv->regs = base;
 	priv->can.bittiming_const = hwcap->bt_const;
+	priv->can.fd.data_bittiming_const = hwcap->bt_const;
 	priv->can.do_set_mode = grcan_set_mode;
 	priv->can.do_get_berr_counter = grcan_get_berr_counter;
 	priv->can.clock.freq = ambafreq;
 	priv->can.ctrlmode_supported =
 		CAN_CTRLMODE_LISTENONLY | CAN_CTRLMODE_ONE_SHOT;
+	if (hwcap->fd)
+		priv->can.ctrlmode_supported |= CAN_CTRLMODE_FD;
 	priv->need_txbug_workaround = txbug;
 	priv->hwcap = hwcap;
 
-- 
2.51.0

Re: [PATCH v7 02/15] dt-bindings: net: can: gaisler,grcan: Convert to DT schema

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The `gaisler,grhcan` compatible string is missing from the schema.
- [Medium] Incomplete schema validation for required clock properties.
- [Low] Unused header inclusion in the device tree example.
--

commit e39872eda9f93cf639be07e78cd2690d6ce06614
Author: Arun Muthusamy <arun.muthusamy@gaisler.com>

dt-bindings: net: can: gaisler,grcan: Convert to DT schema

This commit converts the device tree bindings for Gaisler GRCAN, GRHCAN,
and GRCANFD CAN controllers from a text format to YAML format. It removes
the stale systemid property, makes freq optional, adds clocks, and provides
an example.

> diff --git a/Documentation/devicetree/bindings/net/can/gaisler,grcan.yaml b/Documentation/devicetree/bindings/net/can/gaisler,grcan.yaml
> new file mode 100644
> index 0000000000000..8fea97c7319ad
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/can/gaisler,grcan.yaml
> @@ -0,0 +1,62 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/can/gaisler,grcan.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#

[ ... ]

> +properties:
> +  compatible:
> +    enum:
> +      - gaisler,grcan
> +      - gaisler,grcanfd

The commit message, title, and description explicitly state that the binding
supports the GRHCAN CAN controller, and the original text binding also
documented support for it. 

Does this enum unintentionally omit "gaisler,grhcan"? Without it, device
trees specifying the GRHCAN hardware variant might fail schema validation.

> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  freq:
> +    $ref: /schemas/types.yaml#/definitions/uint32

[ ... ]

> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +

The original text binding required the freq property for the CAN controller
to operate. The new schema makes freq optional and introduces clocks, but
neither is required here. 

Should the schema enforce that at least one clock source is provided using an
anyOf constraint, to prevent device trees lacking clock definitions from
silently passing validation?

> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    can@ff400000 {
> +        compatible = "gaisler,grcanfd";
> +        clocks = <&sysclock>;
> +        reg = <0xff400000 0x400>;
> +        interrupt-parent = <&plic0>;
> +        interrupts = <6>;
> +    };

This isn't a bug, but the example includes irq.h while the interrupts
property is assigned a raw integer <6>. Is this include necessary here?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508070121.6918-1-arun.muthusamy@gaisler.com?part=2

Re: [PATCH v7 04/15] can: grcan: Add clock handling

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The fallback clock is acquired with `devm_clk_get()` but is never prepared or enabled.
- [Low] The deferred probe log suppression provided by `dev_err_probe()` is defeated by an unconditional `dev_err()` at the `exit_error` label.
--

commit e67893a17ab7e0f12006c5fe41c33d231ab03961
Author: Daniel Hellstrom <daniel@gaisler.com>

can: grcan: Add clock handling

This commit adds clock handling and error messages to provide a fallback
for when the freq device tree property is missing.

> diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
> --- a/drivers/net/can/grcan.c
> +++ b/drivers/net/can/grcan.c
[ ... ]
> @@ -1662,8 +1663,19 @@ static int grcan_probe(struct platform_device *ofdev)
>  
>  	err = of_property_read_u32(np, "freq", &ambafreq);
>  	if (err) {
> -		dev_err(&ofdev->dev, "unable to fetch \"freq\" property\n");
> -		goto exit_error;
> +		clk = devm_clk_get(&ofdev->dev, NULL);

Does this code need to prepare and enable the clock?

When acquired via the Common Clock Framework, merely reading the rate
with clk_get_rate() does not increment the enable count. If the clock is
gateable, the late initcall clk_disable_unused might disable it, which
can lead to a system crash or bus lockup when accessing the CAN controller
registers.

Would devm_clk_get_enabled() be more appropriate here?

> +		if (IS_ERR(clk)) {
> +			dev_err_probe(&ofdev->dev, PTR_ERR(clk),
> +				      "Failed both to get \"freq\" property and clock for fallback\n");
> +			err = PTR_ERR(clk);
> +			goto exit_error;

Does this goto defeat the deferred probe log suppression?

If devm_clk_get returns -EPROBE_DEFER, dev_err_probe() correctly suppresses
its own log message. However, jumping to the exit_error label unconditionally
calls dev_err(), which will log the probe deferral anyway:

exit_error:
	dev_err(&ofdev->dev,
		"%s socket CAN driver initialization failed with error %d\n",
		DRV_NAME, err);
	return err;

Since no manual resource cleanup is required at this stage of the probe
function, could the driver directly return the result of dev_err_probe()
instead?

> +		}
> +
> +		ambafreq = clk_get_rate(clk);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508070121.6918-1-arun.muthusamy@gaisler.com?part=4

Re: [PATCH v7 06/15] can: grcan: Simplify timing configuration

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch removes the hardware-specific bounds check `ps1 > ps2` under the false assumption that it is a redundant check handled by the CAN core.
--

commit 245e4bc80a0c4a622684c9f656acd758d80ee6a0
Author: Arun Muthusamy <arun.muthusamy@gaisler.com>

can: grcan: Simplify timing configuration

This commit simplifies the CAN bit timing configuration by removing
error checks deemed redundant and using FIELD_PREP for bit assignments.

> diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
> index 8d7bde1c006f2..8ba21d94e87df 100644
> --- a/drivers/net/can/grcan.c
> +++ b/drivers/net/can/grcan.c
[ ... ]
> @@ -396,41 +396,26 @@ static const struct can_bittiming_const grcan_bittiming_const = {
>  static int grcan_set_bittiming(struct net_device *dev)
>  {
[ ... ]
>  	ps1 = (bt->prop_seg + bt->phase_seg1) - 1; /* tseg1 - 1 */
>  	ps2 = bt->phase_seg2;
> -	scaler = (bt->brp - 1);
> -	netdev_dbg(dev, "Request for BPR=%d, RSJ=%d, PS1=%d, PS2=%d, SCALER=%d",
> -		   bpr, rsj, ps1, ps2, scaler);
> -	if (!(ps1 > ps2)) {
> -		netdev_err(dev, "PS1 > PS2 must hold: PS1=%d, PS2=%d\n",
> -			   ps1, ps2);
> -		return -EINVAL;
> -	}

The commit message claims the error checks are redundant. While the
ps2 >= rsj constraint is enforced by the CAN framework via can_sjw_check(),
does the CAN core actually enforce the ps1 > ps2 constraint?

A user with network admin privileges can configure CAN standard values via
netlink that result in ps1 <= ps2. For example, configuring an unusually low
sample point can lead can_calc_bittiming() to generate parameters where
ps1 <= ps2.

If the hardware strictly requires ps1 > ps2, as the original error message
suggests, could removing this safeguard allow unsupported configurations to
be programmed into the hardware?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508070121.6918-1-arun.muthusamy@gaisler.com?part=6

Re: [PATCH v7 07/15] can: grcan: add FD capability detection and nominal bit-timing

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing assignment of `priv->can.do_set_bittiming` breaks hardware bit-timing configuration.
- [Medium] Missing NULL check on `device_get_match_data()` leads to potential kernel panic.
--

commit 37c33b36466e8784c1911d95ae987b6dfff15154
Author: Arun Muthusamy <arun.muthusamy@gaisler.com>

can: grcan: add FD capability detection and nominal bit-timing

This commit adds CAN FD capability detection and structures for setting
nominal bit-timing for standard CAN FD. It also updates the device tree
match table and Kconfig for GRCANFD support.

> diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
> index 8ba21d94e87df..c9175a6a013fc 100644
> --- a/drivers/net/can/grcan.c
> +++ b/drivers/net/can/grcan.c

[ ... ]

> @@ -1567,14 +1646,14 @@ static int grcan_setup_netdev(struct platform_device *ofdev,
>  	priv->dev = dev;
>  	priv->ofdev_dev = &ofdev->dev;
>  	priv->regs = base;
> -	priv->can.bittiming_const = &grcan_bittiming_const;
> -	priv->can.do_set_bittiming = grcan_set_bittiming;
> +	priv->can.bittiming_const = hwcap->bt_const;

Will the hardware bit-timing still be configured correctly without this
callback?

It appears the assignment to priv->can.do_set_bittiming was removed but not
replaced with hwcap->set_bittiming. If this is left as NULL, the CAN
framework will skip programming the hardware's bit-timing registers and
return 0. This could cause the hardware to attempt transmissions using
uninitialized registers, potentially leading to continuous error frames on
the bus.

[ ... ]

> @@ -1619,22 +1698,29 @@ static int grcan_probe(struct platform_device *ofdev)
>  {
>  	struct device_node *np = ofdev->dev.of_node;
>  	struct device_node *sysid_parent;
> +	const struct grcan_hwcap *hwcap;
>  	struct clk *clk;
>  	u32 sysid, ambafreq;
>  	int irq, err;
>  	void __iomem *base;
>  	bool txbug = true;
>  
> +	hwcap = device_get_match_data(&ofdev->dev);
> +
>  	/* Compare GRLIB version number with the first that does not
>  	 * have the tx bug (see start_xmit)
>  	 */
> -	sysid_parent = of_find_node_by_path("/ambapp0");
> -	if (sysid_parent) {
> -		err = of_property_read_u32(sysid_parent, "systemid", &sysid);
> -		if (!err && ((sysid & GRLIB_VERSION_MASK) >=
> -			     GRCAN_TXBUG_SAFE_GRLIB_VERSION))
> -			txbug = false;
> -		of_node_put(sysid_parent);
> +	if (!hwcap->txbug_possible) {

Could this cause a regression if device_get_match_data() returns NULL?

If the driver is instantiated via a platform device match (matching the
driver string) rather than through Open Firmware, device_get_match_data()
will return NULL. Dereferencing hwcap immediately could result in a NULL
pointer dereference. Is a fallback or check needed here before accessing
hwcap->txbug_possible?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508070121.6918-1-arun.muthusamy@gaisler.com?part=7

Re: [PATCH v7 08/15] can: grcan: set DMA mask for GRCAN and GRCANFD to 32-bit

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Pre-existing NULL pointer dereference if driver is bound manually.
- [Low] Misuse of `dev_err_probe()` causing redundant error logging.
--

commit d15f249cce5269da5a1fa1e8f9d22ab136f42013
Author: Daniel Hellstrom <daniel@gaisler.com>

can: grcan: set DMA mask for GRCAN and GRCANFD to 32-bit

Sets the DMA mask for GRCAN and GRCANFD devices to 32-bit.
Setting the DMA mask and coherent DMA mask to 32-bit ensures proper
memory addressing during DMA operations.

> diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
> --- a/drivers/net/can/grcan.c
> +++ b/drivers/net/can/grcan.c
> @@ -1746,6 +1746,13 @@ static int grcan_probe(struct platform_device *ofdev)
>  		goto exit_error;
>  	}
>  
> +	/* On 64-bit systems.. GRCAN and GRCANFD can only address 32-bit */
> +	err = dma_set_mask_and_coherent(&ofdev->dev, DMA_BIT_MASK(32));
> +	if (err) {
> +		dev_err_probe(&ofdev->dev, err, "No usable DMA configuration.\n");
> +		goto exit_error;

Does this dev_err_probe() call result in redundant error logging?

If dma_set_mask_and_coherent() fails, dev_err_probe() logs the error, but
its return value is ignored and execution falls through to the exit_error
label.

The exit_error path unconditionally calls dev_err() to log the failure:

drivers/net/can/grcan.c:grcan_probe() {
    ...
exit_error:
    dev_err(&ofdev->dev,
        "%s socket CAN driver initialization failed with error %d\n",
        DRV_NAME, err);
    return err;
}

Could this cause the error to be logged twice for a single failure
condition?

> +	}
> +
>  	irq = irq_of_parse_and_map(np, GRCAN_IRQIX_IRQ);

This wasn't introduced by this patch, but is it possible for
device_get_match_data() to return a null pointer earlier in this function?

If the driver is bound manually via sysfs or instantiated as a platform
device without a matching device tree node, device_get_match_data() will
return NULL:

drivers/net/can/grcan.c:grcan_probe() {
    ...
    hwcap = device_get_match_data(&ofdev->dev);

    if (!hwcap->txbug_possible) {
        txbug = false;
    ...
}

Could dereferencing hwcap without a check cause a kernel panic in that
scenario?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508070121.6918-1-arun.muthusamy@gaisler.com?part=8

Re: [PATCH v7 09/15] can: grcan: Add saving and restoring of CAN FD baud-rate registers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The patch claims to save and restore CAN FD baud-rate registers upon reset but fails to restore the data bit-timing register (`regs->fdbtr`), causing it to be permanently wiped.
- [Medium] The `GENMASK` definitions for the GRCANFD `NBTR` register fields overlap, creating a latent bug in the hardware interface definition.
--

commit c685fd29f8a299a3b71b10448a638f1df08b5e0d
Author: Daniel Hellstrom <daniel@gaisler.com>

can: grcan: Add saving and restoring of CAN FD baud-rate registers

While reset the GRCAN baud-rates are preserved, since GRCANFD has the
baud-rate in different registers we need to add saving of those
registers too.

> diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
> index 6e0a393785e86..ccf4032ed642e 100644
> --- a/drivers/net/can/grcan.c
> +++ b/drivers/net/can/grcan.c
> @@ -528,6 +528,8 @@ static void grcan_reset(struct net_device *dev)
>  
>  	/* Turn off hardware filtering - regs->rxcode set to 0 by reset */
>  	grcan_write_reg(&regs->rxmask, 0);
> +
> +	priv->hwcap->set_bittiming(dev);

Does this fully restore the CAN FD baud-rate registers?

The commit message indicates that the GRCANFD baud-rate registers need
to be saved and restored, but it looks like set_bittiming() only sets the
nominal bit-timing register (regs->nbtr):

drivers/net/can/grcan.c:grcanfd_set_bittiming() {
    ...
	timing |= FIELD_PREP(GRCANFD_NBTR_SJW, sjw);
	timing |= FIELD_PREP(GRCANFD_NBTR_PS1, ps1);
	timing |= FIELD_PREP(GRCANFD_NBTR_PS2, ps2);
	timing |= FIELD_PREP(GRCANFD_NBTR_SCALER, scaler);
	netdev_dbg(dev, "setting timing=0x%x\n", timing);
	grcan_write_bits(&regs->nbtr, timing, GRCANFD_NBTR_TIMING);

	return 0;
}

Since the hardware reset clears the registers, and the fast data bit-timing
register (regs->fdbtr) is not saved before the reset or recalculated here,
will regs->fdbtr be permanently wiped?

This could cause the data baud-rate scaler to become zero and break CAN FD
data phase communication if Bit Rate Switch (BRS) is enabled.

Additionally, while looking at the register field definitions used by
set_bittiming(), do the GENMASK definitions for the GRCANFD NBTR overlap?

drivers/net/can/grcan.c:
#define GRCANFD_NBTR_SCALER     GENMASK(23, 16)
#define GRCANFD_NBTR_PS1        GENMASK(17, 10)

Bits 16 and 17 are included in both GRCANFD_NBTR_SCALER and GRCANFD_NBTR_PS1.

While it seems the current maximum phase segment limit prevents active
collisions at runtime, any future increase or use of FIELD_GET could lead
to silent data corruption because the scaler and ps1 bits bleed into
each other.

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508070121.6918-1-arun.muthusamy@gaisler.com?part=9

Re: [PATCH v7 11/15] can: grcan: Refactor GRCAN DMA buffer to use structured memory layout

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Unnecessary __packed attribute on struct grcan_msg_header causes inefficient and non-atomic DMA descriptor accesses.
- [High] Replacing word-wise bit shifts with memcpy() for CAN payload breaks endianness on little-endian architectures.
- [High] Pre-existing NAPI livelock when alloc_can_skb() fails under memory pressure.
--

commit be98321b7518fa6d89c56b12cd7c4501bba55c9b
Author: Arun Muthusamy <arun.muthusamy@gaisler.com>

can: grcan: Refactor GRCAN DMA buffer to use structured memory layout

This commit refactors the GRCAN DMA buffer handling to use structured memory
layouts instead of array indices. It introduces struct grcan_msg_header and
updates the transmit and receive functions to use memcpy for payload handling.

> diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
> index b5527fd9e02e7..4921da349a0ef 100644
> --- a/drivers/net/can/grcan.c
> +++ b/drivers/net/can/grcan.c
> @@ -242,9 +239,23 @@ struct grcan_hwcap {
>  	bool fd;
>  };
>  
> +struct grcan_msg_header {
> +	u32 id;
> +	u32 ctrl;
> +	u8 data[CAN_MAX_DLEN];
> +} __packed;

Since this structure is already naturally aligned to 16 bytes, is the __packed
attribute necessary? On strict-alignment architectures like SPARC, which are
typically used with Aeroflex Gaisler cores, this attribute might force the
compiler to synthesize non-atomic byte-by-byte memory accesses (ldub/stb) for
the 32-bit id and ctrl fields instead of atomic 32-bit instructions.

Could this disrupt the atomicity of descriptor reads and writes, leading the
GRCAN DMA hardware to observe partially updated descriptors?

[ ... ]

> @@ -1239,30 +1256,33 @@ static int grcan_receive(struct net_device *dev, int budget)
>  			continue;
>  		}

This is a pre-existing issue, but if alloc_can_skb() returns NULL due to memory
pressure, the code uses continue and skips the remainder of the loop.

Does this bypass the rd = grcan_ring_add(...) call at the end of the loop?
If the read pointer rd is never advanced, won't the loop process the same
packet repeatedly until budget is exhausted, potentially leading to a NAPI
livelock?

>  
> -		slot = dma->rx.buf + rd;
> -		eff = slot[0] & GRCAN_MSG_IDE;
> -		rtr = slot[0] & GRCAN_MSG_RTR;
> +		hdr = grcan_msg_header_at(&dma->rx, rd);
> +
> +		eff = hdr->id & GRCAN_MSG_IDE;
> +		rtr = hdr->id & GRCAN_MSG_RTR;
> +
>  		if (eff) {
> -			cf->can_id = ((slot[0] & GRCAN_MSG_EID)
> +			cf->can_id = ((hdr->id & GRCAN_MSG_EID)
>  				      >> GRCAN_MSG_EID_BIT);
>  			cf->can_id |= CAN_EFF_FLAG;
>  		} else {
> -			cf->can_id = ((slot[0] & GRCAN_MSG_BID)
> +			cf->can_id = ((hdr->id & GRCAN_MSG_BID)
>  				      >> GRCAN_MSG_BID_BIT);
>  		}
> -		cf->len = can_cc_dlc2len((slot[1] & GRCAN_MSG_DLC)
> -					  >> GRCAN_MSG_DLC_BIT);
> +
> +		cf->len = can_cc_dlc2len((hdr->ctrl & GRCAN_MSG_DLC)
> +					 >> GRCAN_MSG_DLC_BIT);
> +
>  		if (rtr) {
>  			cf->can_id |= CAN_RTR_FLAG;
>  		} else {
> -			for (i = 0; i < cf->len; i++) {
> -				j = GRCAN_MSG_DATA_SLOT_INDEX(i);
> -				shift = GRCAN_MSG_DATA_SHIFT(i);
> -				cf->data[i] = (u8)(slot[j] >> shift);
> -			}
> +			if (cf->len > 0)
> +				memcpy(cf->data, hdr->data,
> +				       min_t(u32, cf->len, CAN_MAX_DLEN));

The original code explicitly extracted payload bytes from 32-bit words using
bitwise shifts, which placed the bytes correctly regardless of the host
architecture's endianness.

Will replacing this with a byte-wise memcpy() break the payload memory layout
on little-endian systems, causing the CAN hardware to receive byte-swapped
corrupted payloads?

>  
>  			stats->rx_bytes += cf->len;
>  		}

[ ... ]

> @@ -1447,16 +1468,13 @@ static netdev_tx_t grcan_start_xmit(struct sk_buff *skb,
>  		tmp = (id << GRCAN_MSG_EID_BIT) & GRCAN_MSG_EID;
>  	else
>  		tmp = (id << GRCAN_MSG_BID_BIT) & GRCAN_MSG_BID;
> -	slot[0] = (eff ? GRCAN_MSG_IDE : 0) | (rtr ? GRCAN_MSG_RTR : 0) | tmp;
> -
> -	slot[1] = ((dlc << GRCAN_MSG_DLC_BIT) & GRCAN_MSG_DLC);
> -	slot[2] = 0;
> -	slot[3] = 0;
> -	for (i = 0; i < dlc; i++) {
> -		j = GRCAN_MSG_DATA_SLOT_INDEX(i);
> -		shift = GRCAN_MSG_DATA_SHIFT(i);
> -		slot[j] |= cf->data[i] << shift;
> -	}
> +
> +	hdr->id = (eff ? GRCAN_MSG_IDE : 0) | (rtr ? GRCAN_MSG_RTR : 0) | tmp;
> +
> +	hdr->ctrl = ((dlc << GRCAN_MSG_DLC_BIT) & GRCAN_MSG_DLC);
> +
> +	if (dlc > 0)
> +		memcpy(hdr->data, cf->data, min_t(u32, cf->len, CAN_MAX_DLEN));

Similarly, does changing this transmit packing from explicit bitwise shifts
to memcpy() risk transmitting byte-swapped payloads on little-endian
architectures?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508070121.6918-1-arun.muthusamy@gaisler.com?part=11

Re: [PATCH v7 12/15] can: grcan: Add CANFD TX support alongside legacy CAN

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch breaks the `grcan_txbug_workaround` mitigation for a known hardware hang, causing it to bypass the trigger condition or unconditionally time out.
--

commit bce48dc84fac934ecb22c0ada575ba8344b8ec53
Author: Arun Muthusamy <arun.muthusamy@gaisler.com>

can: grcan: Add CANFD TX support alongside legacy CAN

This commit includes CANFD TX support with the legacy CAN support, enabling
extended data payloads to provide higher bit rates.

> diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
> index 4921da349a0ef..690e07c7b1ef8 100644
> --- a/drivers/net/can/grcan.c
> +++ b/drivers/net/can/grcan.c

[ ... ]

> @@ -1441,40 +1484,55 @@ static netdev_tx_t grcan_start_xmit(struct sk_buff *skb,
>  	spin_lock_irqsave(&priv->lock, flags);
>  
>  	txwr = grcan_read_reg(&regs->txwr);
>  	space = grcan_txspace(dma->tx.size, txwr, priv->eskbp);
>  
>  	slotindex = txwr / GRCAN_MSG_SIZE;
> +	bds = grcan_numbds(len);

[ ... ]

> +	copy_len = min_t(u32, len, CAN_MAX_DLEN);
> +	memcpy(hdr->data, payload, copy_len);
> +	payload_offset = copy_len;
>  
> -	if (dlc > 0)
> -		memcpy(hdr->data, cf->data, min_t(u32, cf->len, CAN_MAX_DLEN));
> +	txwr = grcan_ring_add(txwr, GRCAN_MSG_SIZE, dma->tx.size);
> +
> +	for (i = 1; i < bds; i++) {
> +		frag = grcan_msg_frag_at(&dma->tx, txwr);
> +
> +		memset(frag, 0, sizeof(*frag));
> +		copy_len = min_t(u32, (u32)len - payload_offset, (u32)GRCAN_MSG_SIZE);
> +		memcpy(frag->data, payload + payload_offset, copy_len);
> +		payload_offset += copy_len;
> +
> +		txwr = grcan_ring_add(txwr, GRCAN_MSG_SIZE, dma->tx.size);
> +	}

[ ... ]

>  	/* Bug workaround for old version of grcan where updating txwr
>  	 * in the same clock cycle as the controller updates txrd to
>  	 * the current txwr could hang the can controller
>  	 */
>  	if (priv->need_txbug_workaround) {
>  		txrd = grcan_read_reg(&regs->txrd);
>  		if (unlikely(grcan_ring_sub(txwr, txrd, dma->tx.size) == 1)) {

Does this break the grcan_txbug_workaround mitigation?

In the original code, txwr represented the current hardware write pointer 
when it was used in this check. Now, txwr is updated in the loop above to 
calculate the end of the new message fragments before the workaround check 
is evaluated.

Because txwr is now the new advanced write pointer, won't the trigger
condition grcan_ring_sub(txwr, txrd, dma->tx.size) == 1 evaluate against 
the wrong offset and fail to prevent the controller hang?

Additionally, grcan_txbug_workaround() waits for the hardware read pointer
to catch up to the provided txwr. Since the advanced txwr hasn't been 
written to the hardware yet (it is updated in the hardware at the end of 
the function), will this guarantee a timeout, returning -EBUSY and 
permanently stalling the TX queue?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508070121.6918-1-arun.muthusamy@gaisler.com?part=12

Re: [PATCH v7 13/15] can: grcan: Add CANFD RX support alongside legacy CAN

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Failure to advance the DMA ring read pointer on SKB allocation failure causes a CPU livelock.
- [Medium] The driver silently ignores the Bit Rate Switch (BRS) flag for received CAN FD frames.
--

commit ce68eaadc0f30d6e6864ce5c3ae7544dc009b802
Author: Arun Muthusamy <arun.muthusamy@gaisler.com>

can: grcan: Add CANFD RX support alongside legacy CAN

This commit adds CAN FD reception capabilities to the grcan driver,
allowing it to handle extended data payloads and higher bit rates.

> diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
> index 690e07c7b1ef8..eba52701c882e 100644
> --- a/drivers/net/can/grcan.c
> +++ b/drivers/net/can/grcan.c
[ ... ]
> @@ -1269,47 +1276,62 @@ static int grcan_receive(struct net_device *dev, int budget)
>  		if (rd == wr)
>  			break;
>  
> -		/* Take care of packet */
> -		skb = alloc_can_skb(dev, &cf);
> -		if (skb == NULL) {
> -			netdev_err(dev,
> -				   "dropping frame: skb allocation failed\n");
> +		hdr = grcan_msg_header_at(&dma->rx, rd);
> +		if (hdr->ctrl & GRCAN_RX_FDF)
> +			skb = alloc_canfd_skb(dev, &cf);
> +		else
> +			skb = alloc_can_skb(dev, (struct can_frame **)&cf);
> +
> +		if (unlikely(!skb)) {
> +			netdev_err(dev, "dropping frame: skb allocation failed\n");
>  			stats->rx_dropped++;
>  			continue;
>  		}

This isn't a bug introduced by this patch, but if the skb allocation fails
here under memory pressure, the loop executes a continue without advancing the
hardware read pointer (rd).

Because rd is not advanced, the next iteration of the loop will attempt to read
the exact same packet, fail allocation again, and repeat this until the NAPI
budget is exhausted. Will this cause NAPI to continuously reschedule the poll
function, leading to a CPU softirq lockup?

Should the driver calculate the number of descriptors and advance rd
appropriately before continuing?

>  
> -		hdr = grcan_msg_header_at(&dma->rx, rd);
> +		dlc = FIELD_GET(GRCAN_MSG_DLC_MASK, hdr->ctrl);
> +		if (hdr->ctrl & GRCAN_RX_FDF)
> +			cf->len = can_fd_dlc2len(dlc);
> +		else
> +			cf->len = can_cc_dlc2len(dlc);
> +

The patch defines GRCAN_RX_BRS as BIT(25) to indicate that the hardware
received a CAN FD frame with Bit Rate Switch enabled, but the receive path
does not appear to check this bit.

If hdr->ctrl & GRCAN_RX_BRS is set, does the driver need to set the CANFD_BRS
bit in cf->flags before passing the frame to the network stack?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508070121.6918-1-arun.muthusamy@gaisler.com?part=13

Re: [PATCH v7 14/15] can: grcan: Update echo skb handling to match variable length CANFD frame

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Flawed queue space check artificially limits TX queue depth to 1 packet.
- [High] Decoupled echo SKB tracking corrupts TX completions and leaks SKBs when multiple frames are batched.
- [Medium] Data race on echo SKB indices between transmission and completion contexts.
--

commit 0d1b72a766dc2d38f5550398f1494cc94c0dd17e
Author: Arun Muthusamy <arun.muthusamy@gaisler.com>
Subject: can: grcan: Update echo skb handling to match variable length CANFD frame

The commit refactors echo socket buffer management in the grcan driver to
support variable length CANFD frames. It introduces echo_skb_idx and
next_echo_idx to decouple the echo skb index from the ring buffer slots, and
adjusts the echo skb allocation based on the number of slots.

[ ... ]

> diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
> index eba52701c882e..06ef081970a24 100644
> --- a/drivers/net/can/grcan.c
> +++ b/drivers/net/can/grcan.c

[ ... ]

> @@ -604,19 +613,22 @@ static int catch_up_echo_skb(struct net_device *dev, int budget, bool echo)
>  	for (work_done = 0; work_done < budget || budget < 0; work_done++) {
>  		if (priv->eskbp == txrd)
>  			break;
>  
>  		priv->eskbp = grcan_ring_add(priv->eskbp, GRCAN_MSG_SIZE,
>  					     dma->tx.size);
>  		txrd = grcan_read_reg(&regs->txrd);
> +
> +		/* Grab the packet once the packet is send or free untransmitted packet */
> +		if (priv->eskbp == txrd) {
> +			if (echo) {
> +				/* Normal echo of messages */
> +				stats->tx_packets++;
> +				stats->tx_bytes += can_get_echo_skb(dev, priv->echo_skb_idx, NULL);

If multiple frames complete at the same time, the hardware will advance txrd
by multiple slots. The condition if (priv->eskbp == txrd) will only be true
for the last frame, skipping any intermediate frames. Does this logic miss
tracking and leak the skbs for the intermediate frames?

Additionally, priv->echo_skb_idx is a single scalar value updated in
grcan_start_xmit() to point to the most recently queued packet. If multiple
packets are in flight, wouldn't this echo the newest packet repeatedly
instead of the oldest, breaking FIFO ordering and leaking the earlier ones?

Finally, priv->echo_skb_idx is read here in the completion context, but it
is modified in grcan_start_xmit() outside of any lock. Is a lock required
here to prevent reading a stale or partially updated index?

> +			} else {
> +				/* For cleanup of untransmitted messages */
> +				can_free_echo_skb(dev, priv->echo_skb_idx, NULL);
> +			}
> +		}
>  	}
>  	return work_done;
>  }

[ ... ]

> @@ -1504,11 +1519,9 @@ static netdev_tx_t grcan_start_xmit(struct sk_buff *skb,
>  
>  	txwr = grcan_read_reg(&regs->txwr);
>  	space = grcan_txspace(dma->tx.size, txwr, priv->eskbp);
> -
> -	slotindex = txwr / GRCAN_MSG_SIZE;
>  	bds = grcan_numbds(len);
>  
> -	if (unlikely(space < bds)) {
> +	if (unlikely(space < bds || priv->can.echo_skb[priv->echo_skb_idx])) {

At this point in the function, priv->echo_skb_idx still holds the index of
the previously queued packet (it is only updated to priv->next_echo_idx at
the end of the function).

If the previous packet is still in flight, its slot in priv->can.echo_skb
is occupied. Does this cause the condition to evaluate to true, returning
NETDEV_TX_BUSY and artificially limiting the transmission queue depth to a
single packet?

>  		netif_stop_queue(dev);
>  		spin_unlock_irqrestore(&priv->lock, flags);
>  		return NETDEV_TX_BUSY;

[ ... ]

> @@ -1588,7 +1601,13 @@ static netdev_tx_t grcan_start_xmit(struct sk_buff *skb,
>  	 * can_put_echo_skb would be an error unless other measures are
>  	 * taken.
>  	 */
> -	can_put_echo_skb(skb, dev, slotindex, 0);
> +
> +	priv->echo_skb_idx = priv->next_echo_idx;
> +
> +	can_put_echo_skb(skb, dev, priv->next_echo_idx, 0);
> +
> +	/* Move to the next index in the echo skb buffer */
> +	priv->next_echo_idx = (priv->next_echo_idx + 1) % priv->can.echo_skb_max;

These variables are updated here outside of the priv->lock critical section
that was unlocked earlier in the function. Since they are read concurrently
in catch_up_echo_skb() during softirq, does this introduce a data race?

>  	/* Make sure everything is written before allowing hardware to
>  	 * read from the memory
>  	 */
>  	wmb();

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508070121.6918-1-arun.muthusamy@gaisler.com?part=14

Re: [PATCH v7 15/15] can: grcan: Advertise CANFD capability

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Missing CAN FD Data Phase hardware configuration.
- [High] Advertised data bit timing constraints exceed hardware register limits.
- [High] Socket buffer allocation failures permanently stall the NAPI receive queue (infinite loop).
- [Medium] The CANFD_BRS flag is lost on received CAN FD frames.
- [Medium] Unconditional exposure of CAN FD data bit timing to userspace for non-FD hardware.
--

commit 73799692e455f9bd1a3d6243e60de78467c0a58b
Author: Arun Muthusamy <arun.muthusamy@gaisler.com>

can: grcan: Advertise CANFD capability

Advertise CANFD capability for GRCANFD hardware by enabling
CAN_CTRLMODE_FD flag and providing CANFD timing operations.

> diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
> index 06ef081970a24..060031df81924 100644
> --- a/drivers/net/can/grcan.c
> +++ b/drivers/net/can/grcan.c
> @@ -1766,11 +1766,14 @@ static int grcan_setup_netdev(struct platform_device *ofdev,
>  	priv->ofdev_dev = &ofdev->dev;
>  	priv->regs = base;
>  	priv->can.bittiming_const = hwcap->bt_const;
> +	priv->can.fd.data_bittiming_const = hwcap->bt_const;

Are the data bit timing constraints advertised here too large for the
hardware registers? It looks like hwcap->bt_const (which sets
tseg1_max = 63) is assigned, but the hardware macro for the Data Bit
Timing Register (GRCANFD_FDBTR_PS1) defines only a 4-bit field (max value
15). Could this result in bitwise truncation and corrupted baud rates if the
register is programmed?

Also, is it safe to assign data_bittiming_const unconditionally, even for
hardware that does not support CAN FD? Since hwcap->fd is not checked
before this assignment, this seems to unconditionally expose
IFLA_CAN_DATA_BITTIMING_CONST to userspace tools for classic CAN controllers.

>  	priv->can.do_set_mode = grcan_set_mode;
>  	priv->can.do_get_berr_counter = grcan_get_berr_counter;
>  	priv->can.clock.freq = ambafreq;
>  	priv->can.ctrlmode_supported =
>  		CAN_CTRLMODE_LISTENONLY | CAN_CTRLMODE_ONE_SHOT;
> +	if (hwcap->fd)
> +		priv->can.ctrlmode_supported |= CAN_CTRLMODE_FD;

With CAN FD being enabled here, there appear to be a few issues in other
parts of the code handling the FD functionality.

First, does the hardware timing configuration callback grcanfd_set_bittiming()
omit programming the CAN FD data phase? It seems to only read the nominal
bit timing (priv->can.bittiming) and write it to the Nominal Bit Timing
Register (regs->nbtr), completely ignoring priv->can.fd.data_bittiming
and never programming the Fast/Data Bit Timing Register (regs->fdbtr).
Will this cause immediate communication failures on the bus when transmitting
CAN FD frames with Bit Rate Switching (BRS)?

Second, is the CANFD_BRS flag lost on received CAN FD frames?
In grcan_receive(), when a CAN FD frame is received:

    hdr = grcan_msg_header_at(&dma->rx, rd);
    if (hdr->ctrl & GRCAN_RX_FDF)
        skb = alloc_canfd_skb(dev, &cf);

It doesn't look like the hardware's BRS bit (GRCAN_RX_BRS, BIT(25)) is
ever checked to set cf->flags |= CANFD_BRS. Does this mean userspace is
incorrectly informed that no received frames used bit-rate switching?

Finally, this code exists in grcan_receive():

    if (unlikely(!skb)) {
        netdev_err(dev, "dropping frame: skb allocation failed\n");
        stats->rx_dropped++;
        continue;
    }

If alloc_can_skb() or alloc_canfd_skb() fails, the code continues to the
next loop iteration. However, because rd is not advanced (via
grcan_ring_add()), won't the next iteration read the exact same message,
fail allocation again, and repeat until the NAPI budget is exhausted? Could
this permanently stall the receive ring and cause a NAPI livelock under
memory pressure?

>  	priv->need_txbug_workaround = txbug;
>  	priv->hwcap = hwcap;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508070121.6918-1-arun.muthusamy@gaisler.com?part=15