[PATCH 00/10] can: grcan: Enhance driver with CANFD Support and Improvements

[PATCH 00/10] 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 (3):
  dt-bindings: net: can: grcan: Convert GRCAN CAN controllers binding
    from txt to YAML
  MAINTAINERS: Add maintainers for GRCAN CAN network driver
  can: grcan: Add CANFD support alongside legacy CAN

Daniel Hellstrom (6):
  can: grcan: Add clock handling
  can: grcan: add FD capability detection and nominal bit-timing
  can: grcan: optimize DMA by 32-bit accesses
  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

 .../bindings/net/can/gaisler,grcan.yaml       |  85 ++++
 .../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                       | 470 ++++++++++++++----
 6 files changed, 473 insertions(+), 126 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/can/gaisler,grcan.yaml
 delete mode 100644 Documentation/devicetree/bindings/net/can/grcan.txt


base-commit: 4001bda0cc911fcdd3dde36963a17f4eac173d7d
--
2.51.0

[PATCH 01/10] 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>
---
 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 424aa7b911a7..4e1b4eeff9ff 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -594,6 +594,8 @@ patternProperties:
     description: Fujitsu Ltd.
   "^fxtec,.*":
     description: FX Technology Ltd.
+  "^gaisler,.*":
+    description: Frontgrade Gaisler AB
   "^galaxycore,.*":
     description: GalaxyCore Inc.
   "^gameforce,.*":
--
2.51.0

[PATCH 02/10] dt-bindings: net: can: grcan: Convert GRCAN CAN controllers binding from txt to YAML

[Arun Muthusamy]

Migrate device tree bindings for Gaisler GRCAN, GRHCAN
and GRCANFD CAN controllers from a text format to YAML format.
    - Add properties such as `compatible`, `reg`, `interrupts`
    and `clocks` for the CAN controllers.
    - Removal of the old `grcan.txt` file as its contents have
    been fully migrated to the YAML file.
    - YAML file includes examples of device tree bindings for
    the CAN controllers

Signed-off-by: Arun Muthusamy <arun.muthusamy@gaisler.com>
---
 .../bindings/net/can/gaisler,grcan.yaml       | 85 +++++++++++++++++++
 .../devicetree/bindings/net/can/grcan.txt     | 28 ------
 2 files changed, 85 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..521bdd89f130
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/gaisler,grcan.yaml
@@ -0,0 +1,85 @@
+# 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
+  name:
+    description: |
+      Fallback on node name matching for systems that don't provide compatible.
+    enum:
+      - GAISLER_GRCAN
+      - 01_03d
+      - GAISLER_GRHCAN
+      - "01_034"
+
+  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 that utilize the common clock
+      framework is not supported.
+
+unevaluatedProperties: false
+
+required:
+  - reg
+  - interrupts
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    can@ff400000 {
+        compatible = "gaisler,grcanfd";
+        clocks = <&sysclock>;
+        reg = <0xff400000 0x400>;
+        interrupt-parent = <&plic0>;
+        interrupts = <6>;
+    };
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    can@ff400000 {
+        compatible = "gaisler,grcan";
+        clocks = <&sysclock>;
+        reg = <0xff400000 0x400>;
+        interrupt-parent = <&plic0>;
+        interrupts = <6>;
+    };
+  - |
+    GAISLER_GRCAN@ff840000 {
+        reg = <0xff840000 0x400>;
+        freq = <50000000>;
+        interrupts = <16>;
+    };
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 03/10] MAINTAINERS: Add entry for GRCAN CAN network driver

[Arun Muthusamy]

Add Arun Muthusamy and Andreas Larsson as maintainers for
the GRCAN CAN network driver.

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

diff --git a/MAINTAINERS b/MAINTAINERS
index 0dc4aa37d903..14ddd48e063f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10706,6 +10706,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 04/10] 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: Arun Muthusamy <arun.muthusamy@gaisler.com>
Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
---
 drivers/net/can/grcan.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
index 3b1b09943436..538a9b4f82ab 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"
@@ -1644,6 +1644,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;
@@ -1663,8 +1664,20 @@ 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;
+		}
+		if (clk) {
+			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 05/10] can: grcan: add FD capability detection and nominal bit-timing

[Arun Muthusamy]

From: Daniel Hellstrom <daniel@gaisler.com>

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 and 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: Arun Muthusamy <arun.muthusamy@gaisler.com>
Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
---
 drivers/net/can/Kconfig |   6 +-
 drivers/net/can/grcan.c | 189 ++++++++++++++++++++++++++++++++++++----
 2 files changed, 176 insertions(+), 19 deletions(-)

diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
index d43d56694667..816b6e71a990 100644
--- a/drivers/net/can/Kconfig
+++ b/drivers/net/can/Kconfig
@@ -133,10 +133,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, GRCANFD 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 538a9b4f82ab..b9b0dd7d53f6 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>
@@ -50,7 +51,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 */
@@ -212,6 +217,67 @@ struct grcan_registers {
 #error "Invalid default buffer size"
 #endif

+#define GRCANFD_NOMCONF_SJW_MIN	1
+#define GRCANFD_NOMCONF_SJW_MAX	16
+#define GRCANFD_NOMCONF_PS1_MIN	2
+#define GRCANFD_NOMCONF_PS1_MAX	63
+#define GRCANFD_NOMCONF_PS2_MIN	2
+#define GRCANFD_NOMCONF_PS2_MAX	16
+#define GRCANFD_NOMCONF_SCALER_MIN	0
+#define GRCANFD_NOMCONF_SCALER_MAX	255
+#define GRCANFD_NOMCONF_SCALER_INC	1
+
+#define GRCANFD_NBTR_SCALER 0x00ff0000
+#define GRCANFD_NBTR_PS1 0x0000fc00
+#define GRCANFD_NBTR_PS2 0x000003e0
+#define GRCANFD_NBTR_SJW 0x0000001f
+#define GRCANFD_NBTR_TIMING						\
+	(GRCANFD_NBTR_SCALER | GRCANFD_NBTR_PS1 | GRCANFD_NBTR_PS2 |	\
+	 GRCANFD_NBTR_SJW)
+
+#define GRCANFD_NBTR_SCALER_BIT 16
+#define GRCANFD_NBTR_PS1_BIT 10
+#define GRCANFD_NBTR_PS2_BIT 5
+#define GRCANFD_NBTR_SJW_BIT 0
+
+#define GRCANFD_FDCONF_SJW_MIN	1
+#define GRCANFD_FDCONF_SJW_MAX	8
+#define GRCANFD_FDCONF_PS1_MIN	1
+#define GRCANFD_FDCONF_PS1_MAX	15
+#define GRCANFD_FDCONF_PS2_MIN	2
+#define GRCANFD_FDCONF_PS2_MAX	8
+#define GRCANFD_FDCONF_SCALER_MIN	0
+#define GRCANFD_FDCONF_SCALER_MAX	255
+#define GRCANFD_FDCONF_SCALER_INC	1
+
+#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.
+	 */
+	bool fd;
+	bool txbug_possible;
+	const struct can_bittiming_const *bt_const;
+	int (*set_bittiming)(struct net_device *dev);
+};
+
+static const struct grcan_hwcap grcan_hwcap;
+static const struct of_device_id grcan_match[];
+
 struct grcan_dma_buffer {
 	size_t size;
 	void *buf;
@@ -304,6 +370,8 @@ struct grcan_priv {
 	 */
 	bool resetting;
 	bool closing;
+
+	const struct grcan_hwcap *hwcap;
 };

 /* Wait time for a short wait for ongoing to clear */
@@ -402,6 +470,19 @@ static const struct can_bittiming_const grcan_bittiming_const = {
 	.brp_inc	= GRCAN_CONF_SCALER_INC,
 };

+/* GRCANFD nominal boundaries for baud-rate parameters */
+static const struct can_bittiming_const grcanfd_bittiming_const = {
+	.name		= DRV_NAME,
+	.tseg1_min	= GRCANFD_NOMCONF_PS1_MIN,
+	.tseg1_max	= GRCANFD_NOMCONF_PS1_MAX,
+	.tseg2_min	= GRCANFD_NOMCONF_PS2_MIN,
+	.tseg2_max	= GRCANFD_NOMCONF_PS2_MAX,
+	.sjw_max	= GRCANFD_NOMCONF_SJW_MAX,
+	.brp_min	= GRCANFD_NOMCONF_SCALER_MIN + 1,
+	.brp_max	= GRCANFD_NOMCONF_SCALER_MAX + 1,
+	.brp_inc	= GRCANFD_NOMCONF_SCALER_INC,
+};
+
 static int grcan_set_bittiming(struct net_device *dev)
 {
 	struct grcan_priv *priv = netdev_priv(dev);
@@ -439,12 +520,55 @@ static int grcan_set_bittiming(struct net_device *dev)
 	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);
+	netdev_dbg(dev, "setting timing=0x%x\n", timing);
 	grcan_write_bits(&regs->conf, timing, GRCAN_CONF_TIMING);

 	return 0;
 }

+static int grcanfd_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;
+	const char *msg;
+	u32 timing = 0;
+	int sjw, ps1, ps2, scaler;
+
+	/* Should never happen - function will not be called when
+	 * device is up
+	 */
+	if (grcan_read_bits(&regs->ctrl, GRCAN_CTRL_ENABLE))
+		return -EBUSY;
+
+	sjw = bt->sjw;
+	ps1 = (bt->prop_seg + bt->phase_seg1); /* tseg1 */
+	ps2 = bt->phase_seg2;
+	scaler = (bt->brp - 1);
+	netdev_dbg(dev, "Request for SJW=%d, PS1=%d, PS2=%d, SCALER=%d",
+		   sjw, ps1, ps2, scaler);
+	if (sjw > min(ps1, ps2)) {
+		msg = "SJW <= MIN(PS1,PS2) must hold: PS1=%d, PS2=%d, SJW=%d\n";
+		netdev_err(dev, msg, ps1, ps2, sjw);
+		return -EINVAL;
+	}
+	if (ps2 < sjw) {
+		netdev_err(dev, "PS2 >= SJW must hold: PS2=%d, SJW=%d\n",
+			   ps2, sjw);
+		return -EINVAL;
+	}
+
+	timing |= (sjw << GRCANFD_NBTR_SJW_BIT) & GRCANFD_NBTR_SJW;
+	timing |= (ps1 << GRCANFD_NBTR_PS1_BIT) & GRCANFD_NBTR_PS1;
+	timing |= (ps2 << GRCANFD_NBTR_PS2_BIT) & GRCANFD_NBTR_PS2;
+	timing |= (scaler << GRCANFD_NBTR_SCALER_BIT) &
+		  GRCANFD_NBTR_SCALER;
+	netdev_info(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)
 {
@@ -1569,7 +1693,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;
@@ -1592,14 +1717,15 @@ 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_bittiming = hwcap->set_bittiming;
 	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;
@@ -1644,22 +1770,32 @@ static int grcan_probe(struct platform_device *ofdev)
 {
 	struct device_node *np = ofdev->dev.of_node;
 	struct device_node *sysid_parent;
+	const struct of_device_id *of_id;
+	const struct grcan_hwcap *hwcap = &grcan_hwcap;
 	struct clk *clk;
 	u32 sysid, ambafreq;
 	int irq, err;
 	void __iomem *base;
 	bool txbug = true;

+	of_id = of_match_device(grcan_match, &ofdev->dev);
+	if (of_id && of_id->data)
+		hwcap = (struct grcan_hwcap *)of_id->data;
+
 	/* 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);
@@ -1695,7 +1831,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;

@@ -1722,11 +1858,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 06/10] can: grcan: optimize DMA by 32-bit accesses

[Arun Muthusamy]

From: Daniel Hellstrom <daniel@gaisler.com>

Optimizes DMA transfers in the GRCAN driver by reorganizing
data handling to use 32-bit accesses instead of individual
byte accesses.

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

diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
index b9b0dd7d53f6..e367581faa57 100644
--- a/drivers/net/can/grcan.c
+++ b/drivers/net/can/grcan.c
@@ -1287,7 +1287,7 @@ static int grcan_receive(struct net_device *dev, int budget)
 	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);
@@ -1323,10 +1323,10 @@ static int grcan_receive(struct net_device *dev, int budget)
 		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->can_dlc > 0) {
+				*(u32 *)(cf->data) = slot[2];
+				if (cf->can_dlc > 4)
+					*(u32 *)(cf->data + 4) = slot[3];
 			}

 			stats->rx_bytes += cf->len;
@@ -1466,8 +1466,7 @@ static netdev_tx_t grcan_start_xmit(struct sk_buff *skb,
 	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;

@@ -1520,10 +1519,10 @@ static netdev_tx_t grcan_start_xmit(struct sk_buff *skb,
 	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;
+	if (dlc > 0) {
+		slot[2] = *(u32 *)(cf->data); /* data aligned 64-bit */
+		if (dlc > 4)
+			slot[3] = *(u32 *)(cf->data + 4);
 	}

 	/* Checking that channel has not been disabled. These cases
--
2.51.0

[PATCH 07/10] 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: Arun Muthusamy <arun.muthusamy@gaisler.com>
Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
---
 drivers/net/can/grcan.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
index e367581faa57..51a10fae2faf 100644
--- a/drivers/net/can/grcan.c
+++ b/drivers/net/can/grcan.c
@@ -1074,6 +1074,12 @@ static int grcan_allocate_dma_buffers(struct net_device *dev,

 	/* Extra GRCAN_BUFFER_ALIGNMENT to allow for alignment */
 	dma->base_size = lsize + ssize + GRCAN_BUFFER_ALIGNMENT;
+
+	/* On 64-bit systems.. GRCAN and GRCANFD can only address 32-bit */
+	if (dma_set_mask_and_coherent(priv->ofdev_dev, DMA_BIT_MASK(32))) {
+		netdev_warn(dev, "No suitable DMA available\n");
+		return -ENOMEM;
+	}
 	dma->base_buf = dma_alloc_coherent(priv->ofdev_dev,
 					   dma->base_size,
 					   &dma->base_handle,
--
2.51.0

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

diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
index 51a10fae2faf..b0e2367fb163 100644
--- a/drivers/net/can/grcan.c
+++ b/drivers/net/can/grcan.c
@@ -589,9 +589,19 @@ static void grcan_reset(struct net_device *dev)
 	struct grcan_priv *priv = netdev_priv(dev);
 	struct grcan_registers __iomem *regs = priv->regs;
 	u32 config = grcan_read_reg(&regs->conf);
+	u32 nbtr, fdbtr;
+
+	if (priv->hwcap->fd) {
+		nbtr = grcan_read_reg(&regs->nbtr);
+		fdbtr = grcan_read_reg(&regs->fdbtr);
+	}

 	grcan_set_bits(&regs->ctrl, GRCAN_CTRL_RESET);
 	grcan_write_reg(&regs->conf, config);
+	if (priv->hwcap->fd) {
+		grcan_write_reg(&regs->nbtr, nbtr);
+		grcan_write_reg(&regs->fdbtr, fdbtr);
+	}

 	priv->eskbp = grcan_read_reg(&regs->txrd);
 	priv->can.state = CAN_STATE_STOPPED;
--
2.51.0

[PATCH 09/10] 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: Arun Muthusamy <arun.muthusamy@gaisler.com>
Signed-off-by: Daniel Hellstrom <daniel@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 b0e2367fb163..8753bff4f917 100644
--- a/drivers/net/can/grcan.c
+++ b/drivers/net/can/grcan.c
@@ -48,7 +48,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 10/10] can: grcan: Add CANFD support alongside legacy CAN

[Arun Muthusamy]

Include CANFD support with the legacy CAN support, enabling
support for extended data payloads up to 64 bytes, higher bit rates,
handle canecho frames.

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

diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
index 8753bff4f917..ff7ab979d2c9 100644
--- a/drivers/net/can/grcan.c
+++ b/drivers/net/can/grcan.c
@@ -44,6 +44,8 @@

 #define GRCAN_RESERVE_SIZE(slot1, slot2) (((slot2) - (slot1)) / 4 - 1)

+#define CHECK_SLOT_FDF(slot) ((slot) & GRCAN_RX_FDF)
+
 struct grcan_registers {
 	u32 conf;	/* 0x00 */
 	u32 stat;	/* 0x04 */
@@ -181,8 +183,11 @@ struct grcan_registers {
 			  | GRCAN_IRQ_TXAHBERR | GRCAN_IRQ_RXAHBERR	\
 			  | GRCAN_IRQ_TXLOSS)
 #define GRCAN_IRQ_DEFAULT (GRCAN_IRQ_RX | GRCAN_IRQ_TX | GRCAN_IRQ_ERRORS)
+#define GRCAN_CTRL_MODES (CAN_CTRLMODE_LISTENONLY | CAN_CTRLMODE_ONE_SHOT)
+#define GRCAN_CTRL_MODES_FD (CAN_CTRLMODE_LISTENONLY | CAN_CTRLMODE_ONE_SHOT | CAN_CTRLMODE_FD)

 #define GRCAN_MSG_SIZE		16
+#define GRCAN_CLASSIC_DATA_SIZE 8

 #define GRCAN_MSG_IDE		0x80000000
 #define GRCAN_MSG_RTR		0x40000000
@@ -264,6 +269,12 @@ 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)
+
+#define GRCAN_RX_BRS  BIT(25)
+#define GRCAN_RX_FDF  BIT(26)
+
 /* Hardware capabilities */
 struct grcan_hwcap {
 	/* CAN-FD capable, indicates GRCANFD IP.
@@ -326,6 +337,13 @@ 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.
+	 * It's important for ensuring that we correctly manage the echo skb.
+	 */
+	u32 echo_skb_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.
@@ -637,7 +655,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
@@ -648,19 +666,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;
 }
@@ -1174,6 +1195,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->echo_skb_idx = 0;
 		spin_unlock_irqrestore(&priv->lock, flags);
 		return err;
 	}
@@ -1223,7 +1245,6 @@ static int grcan_open(struct net_device *dev)
 		netif_start_queue(dev);
 	priv->resetting = false;
 	priv->closing = false;
-
 	spin_unlock_irqrestore(&priv->lock, flags);

 	return 0;
@@ -1294,20 +1315,29 @@ 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 + ((len - GRCAN_CLASSIC_DATA_SIZE + GRCAN_MSG_SIZE) / GRCAN_MSG_SIZE);
+}
+
 static int grcan_receive(struct net_device *dev, int budget)
 {
 	struct grcan_priv *priv = netdev_priv(dev);
 	struct grcan_registers __iomem *regs = priv->regs;
 	struct grcan_dma *dma = &priv->dma;
 	struct net_device_stats *stats = &dev->stats;
-	struct can_frame *cf;
+	struct canfd_frame *cf;
 	struct sk_buff *skb;
-	u32 wr, rd, startrd;
+	u32 wr, rd, dlc, startrd;
 	u32 *slot;
 	u32 rtr, eff;
-	int work_done = 0;
+	u8 *data;
+	int i, bds, payload_offset, copy_len, work_done = 0;

 	rd = grcan_read_reg(&regs->rxrd);
+
 	startrd = rd;
 	for (work_done = 0; work_done < budget; work_done++) {
 		/* Check for packet to receive */
@@ -1315,44 +1345,70 @@ 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) {
+		slot = dma->rx.buf + rd;
+
+		if (CHECK_SLOT_FDF(slot[1]))
+			skb = alloc_canfd_skb(dev, &cf);
+		else
+			skb = alloc_can_skb(priv->dev, (struct can_frame **)&cf);
+
+		if (unlikely(!skb)) {
 			netdev_err(dev,
 				   "dropping frame: skb allocation failed\n");
 			stats->rx_dropped++;
 			continue;
 		}

-		slot = dma->rx.buf + rd;
-		eff = slot[0] & GRCAN_MSG_IDE;
-		rtr = slot[0] & GRCAN_MSG_RTR;
-		if (eff) {
-			cf->can_id = ((slot[0] & GRCAN_MSG_EID)
-				      >> GRCAN_MSG_EID_BIT);
-			cf->can_id |= CAN_EFF_FLAG;
-		} else {
-			cf->can_id = ((slot[0] & GRCAN_MSG_BID)
-				      >> GRCAN_MSG_BID_BIT);
-		}
-		cf->len = can_cc_dlc2len((slot[1] & GRCAN_MSG_DLC)
-					  >> GRCAN_MSG_DLC_BIT);
-		if (rtr) {
-			cf->can_id |= CAN_RTR_FLAG;
-		} else {
-			if (cf->can_dlc > 0) {
-				*(u32 *)(cf->data) = slot[2];
-				if (cf->can_dlc > 4)
-					*(u32 *)(cf->data + 4) = slot[3];
+		dlc = (slot[1] & GRCAN_MSG_DLC) >> GRCAN_MSG_DLC_BIT;
+		if (CHECK_SLOT_FDF(slot[1]))
+			cf->len = can_fd_dlc2len(dlc);
+		else
+			cf->len = can_cc_dlc2len(dlc);
+
+		bds = grcan_numbds(cf->len);
+		payload_offset = 0;
+		data = cf->data;
+
+		for (i = 0; i < bds; i++) {
+			slot = dma->rx.buf + rd;
+
+			if (i == 0) {
+				eff = slot[0] & GRCAN_MSG_IDE;
+				rtr = slot[0] & GRCAN_MSG_RTR;
+				if (eff) {
+					cf->can_id = ((slot[0] & GRCAN_MSG_EID)
+						      >> GRCAN_MSG_EID_BIT);
+					cf->can_id |= CAN_EFF_FLAG;
+				} else {
+					cf->can_id = ((slot[0] & GRCAN_MSG_BID)
+						      >> GRCAN_MSG_BID_BIT);
+				}
+				if (rtr)
+					cf->can_id |= CAN_RTR_FLAG;
+
+				dlc = (slot[1] & GRCAN_MSG_DLC) >> GRCAN_MSG_DLC_BIT;
+				if (CHECK_SLOT_FDF(slot[1]))
+					cf->len = can_fd_dlc2len(dlc);
+				else
+					cf->len = can_cc_dlc2len(dlc);
+
+				copy_len = min(cf->len, GRCAN_CLASSIC_DATA_SIZE);
+				memcpy(data, &slot[2], copy_len);
+				payload_offset += copy_len;
+			} else {
+				copy_len =  min(cf->len - payload_offset, GRCAN_MSG_SIZE);
+				memcpy(data + payload_offset, slot, copy_len);
+				payload_offset += copy_len;
 			}
-
-			stats->rx_bytes += cf->len;
+			rd += GRCAN_MSG_SIZE;
+			if (rd >= dma->tx.size)
+				rd -= dma->tx.size;
 		}
-		stats->rx_packets++;

+		/* Update statistics and read pointer */
+		stats->rx_packets++;
+		stats->rx_bytes += cf->len;
 		netif_receive_skb(skb);
-
-		rd = grcan_ring_add(rd, GRCAN_MSG_SIZE, dma->rx.size);
 	}

 	/* Make sure everything is read before allowing hardware to
@@ -1479,12 +1535,15 @@ static netdev_tx_t grcan_start_xmit(struct sk_buff *skb,
 	struct grcan_priv *priv = netdev_priv(dev);
 	struct grcan_registers __iomem *regs = priv->regs;
 	struct grcan_dma *dma = &priv->dma;
-	struct can_frame *cf = (struct can_frame *)skb->data;
-	u32 id, txwr, txrd, space, txctrl;
-	int slotindex;
-	u32 *slot;
-	u32 rtr, eff, dlc, tmp, err;
+	struct can_frame *cf;
+	struct canfd_frame *cfd;
+	int i, bds, copy_len, payload_offset;
 	unsigned long flags;
+	u8 *payload;
+	u8 len;
+	u32 *slot;
+	u32 eff, rtr, dlc, tmp, err, can_id;
+	u32 id, txwr, txrd, space, txctrl;
 	u32 oneshotmode = priv->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT;

 	if (can_dev_dropped_skb(dev, skb))
@@ -1496,6 +1555,20 @@ static netdev_tx_t grcan_start_xmit(struct sk_buff *skb,
 	if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
 		return NETDEV_TX_BUSY;

+	if (can_is_canfd_skb(skb)) {
+		cfd = (struct canfd_frame *)skb->data;
+		len = cfd->len;
+		dlc = can_fd_len2dlc(cfd->len);
+		can_id = cfd->can_id;
+		payload = cfd->data;
+	} else {
+		cf = (struct can_frame *)skb->data;
+		len = cf->len;
+		dlc = can_get_cc_dlc(cf, priv->can.ctrlmode);
+		can_id = cf->can_id;
+		payload = cf->data;
+	}
+
 	/* 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.
@@ -1504,9 +1577,7 @@ 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;
-	slot = dma->tx.buf + txwr;
+	bds = grcan_numbds(len);

 	if (unlikely(space == 1))
 		netif_stop_queue(dev);
@@ -1522,24 +1593,38 @@ static netdev_tx_t grcan_start_xmit(struct sk_buff *skb,
 		return NETDEV_TX_BUSY;
 	}

-	/* 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;
-	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;
-	if (dlc > 0) {
-		slot[2] = *(u32 *)(cf->data); /* data aligned 64-bit */
-		if (dlc > 4)
-			slot[3] = *(u32 *)(cf->data + 4);
+	payload_offset = 0;
+	for (i = 0; i < bds; i++) {
+		slot = dma->tx.buf + txwr;
+		memset(slot, 0, GRCAN_MSG_SIZE);
+
+		if (i == 0) {
+			eff = can_id & CAN_EFF_FLAG;
+			rtr = can_id & CAN_RTR_FLAG;
+			id = can_id & (eff ? CAN_EFF_MASK : CAN_SFF_MASK);
+			if (eff)
+				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);
+			if (can_is_canfd_skb(skb)) {
+				slot[1] |= GRCAN_TX_FDF;
+				if (cfd->flags & CANFD_BRS)
+					slot[1] |= GRCAN_TX_BRS;
+			}
+
+			copy_len = min(len, 8);
+			memcpy(&slot[2], payload, copy_len);
+			payload_offset += copy_len;
+		} else {
+			copy_len =  min(len - payload_offset, GRCAN_MSG_SIZE);
+			memcpy(slot, payload + payload_offset, copy_len);
+			payload_offset += copy_len;
+		}
+		txwr += GRCAN_MSG_SIZE;
+		if (txwr >= dma->tx.size)
+			txwr -= dma->tx.size;
 	}

 	/* Checking that channel has not been disabled. These cases
@@ -1574,7 +1659,14 @@ 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);
+
+	can_put_echo_skb(skb, dev, priv->echo_skb_idx, 0);
+
+	/* Move to the next index in the echo skb buffer */
+	priv->echo_skb_idx = (priv->echo_skb_idx + 1) % priv->can.echo_skb_max;
+
+	if (priv->can.echo_skb[priv->echo_skb_idx])
+		netif_stop_queue(dev);

 	/* Make sure everything is written before allowing hardware to
 	 * read from the memory
@@ -1582,8 +1674,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;
 }
@@ -1734,12 +1825,15 @@ 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_bittiming = hwcap->set_bittiming;
 	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 = GRCAN_CTRL_MODES_FD;
+	else
+		priv->can.ctrlmode_supported = GRCAN_CTRL_MODES;
 	priv->need_txbug_workaround = txbug;
 	priv->hwcap = hwcap;

--
2.51.0

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

[Krzysztof Kozlowski]

On 18/11/2025 10:21, Arun Muthusamy wrote:
> 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.
> 

So here is the rest... but I already responded to this email. Don't send
duplicates. Other feedback applies.

Best regards,
Krzysztof

Re: [PATCH 02/10] dt-bindings: net: can: grcan: Convert GRCAN CAN controllers binding from txt to YAML

[Krzysztof Kozlowski]

On 18/11/2025 10:21, Arun Muthusamy wrote:
> Migrate device tree bindings for Gaisler GRCAN, GRHCAN
> and GRCANFD CAN controllers from a text format to YAML format.
>     - Add properties such as `compatible`, `reg`, `interrupts`

Odd indentation. Please write readable commit msgs.

Also:
1. Why? You need to explain why you are changing binding during conversion.
2. Reg was already there, so I don't understand why you need to add it.


>     and `clocks` for the CAN controllers.
>     - Removal of the old `grcan.txt` file as its contents have
>     been fully migrated to the YAML file.

Drop, that's not relevant.

>     - YAML file includes examples of device tree bindings for
>     the CAN controllers

Drop, not relevant. Please look at git history how commits are written.

> 
> Signed-off-by: Arun Muthusamy <arun.muthusamy@gaisler.com>
> ---
>  .../bindings/net/can/gaisler,grcan.yaml       | 85 +++++++++++++++++++
>  .../devicetree/bindings/net/can/grcan.txt     | 28 ------
>  2 files changed, 85 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..521bdd89f130
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/can/gaisler,grcan.yaml
> @@ -0,0 +1,85 @@
> +# 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

Blank line

> +  name:
> +    description: |

Do not need '|' unless you need to preserve formatting.

> +      Fallback on node name matching for systems that don't provide compatible.
> +    enum:
> +      - GAISLER_GRCAN
> +      - 01_03d
> +      - GAISLER_GRHCAN
> +      - "01_034"

This does not really work. Are you really defining here "name" property?

> +
> +  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 that utilize the common clock
> +      framework is not supported.

Missing systemid. Your commit msg must explain any changes done to the
binding during conversion.

> +
> +unevaluatedProperties: false

This goes after required block.

> +
> +required:

compatible as well

> +  - reg
> +  - interrupts

Where is freq? It was required in the old binding. Again, you need to
explain the changes.

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

One example is enough

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

Especially no such examples. Please read DTS coding style.

> +        reg = <0xff840000 0x400>;
> +        freq = <50000000>;
> +        interrupts = <16>;
> +    };
> 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 @@
Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On 18/11/2025 10:21, Arun Muthusamy wrote:
> From: Daniel Hellstrom <daniel@gaisler.com>
> 
> Add clock handling and add error messages for missing 'freq' DT property.
> 
> Signed-off-by: Arun Muthusamy <arun.muthusamy@gaisler.com>
> Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
> ---
>  drivers/net/can/grcan.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
> index 3b1b09943436..538a9b4f82ab 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"
> @@ -1644,6 +1644,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;
> @@ -1663,8 +1664,20 @@ 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);

Nope, your binding said there is no clock... you cannot add undocumented
ABI.


Best regards,
Krzysztof

Re: [PATCH 05/10] can: grcan: add FD capability detection and nominal bit-timing

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 11316 bytes --]

On 18.11.2025 10:21:10, Arun Muthusamy wrote:
> From: Daniel Hellstrom <daniel@gaisler.com>
>
> 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 and 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: Arun Muthusamy <arun.muthusamy@gaisler.com>
> Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>

Your S-o-b must come last.

> ---
>  drivers/net/can/Kconfig |   6 +-
>  drivers/net/can/grcan.c | 189 ++++++++++++++++++++++++++++++++++++----
>  2 files changed, 176 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
> index d43d56694667..816b6e71a990 100644
> --- a/drivers/net/can/Kconfig
> +++ b/drivers/net/can/Kconfig
> @@ -133,10 +133,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, GRCANFD 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 538a9b4f82ab..b9b0dd7d53f6 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>
> @@ -50,7 +51,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 */
> @@ -212,6 +217,67 @@ struct grcan_registers {
>  #error "Invalid default buffer size"
>  #endif
>
> +#define GRCANFD_NOMCONF_SJW_MIN	1
> +#define GRCANFD_NOMCONF_SJW_MAX	16
> +#define GRCANFD_NOMCONF_PS1_MIN	2
> +#define GRCANFD_NOMCONF_PS1_MAX	63
> +#define GRCANFD_NOMCONF_PS2_MIN	2
> +#define GRCANFD_NOMCONF_PS2_MAX	16
> +#define GRCANFD_NOMCONF_SCALER_MIN	0
> +#define GRCANFD_NOMCONF_SCALER_MAX	255
> +#define GRCANFD_NOMCONF_SCALER_INC	1

Please directly add the values to the struct can_bittiming_const.

> +
> +#define GRCANFD_NBTR_SCALER 0x00ff0000
> +#define GRCANFD_NBTR_PS1 0x0000fc00
> +#define GRCANFD_NBTR_PS2 0x000003e0
> +#define GRCANFD_NBTR_SJW 0x0000001f

Please use GEN_MASK() for these

> +#define GRCANFD_NBTR_TIMING						\
> +	(GRCANFD_NBTR_SCALER | GRCANFD_NBTR_PS1 | GRCANFD_NBTR_PS2 |	\
> +	 GRCANFD_NBTR_SJW)
> +
> +#define GRCANFD_NBTR_SCALER_BIT 16
> +#define GRCANFD_NBTR_PS1_BIT 10
> +#define GRCANFD_NBTR_PS2_BIT 5
> +#define GRCANFD_NBTR_SJW_BIT 0

These can be removed, if you use FIELD_PREP()...see below

Same comments apply to the FD defines...

> +
> +#define GRCANFD_FDCONF_SJW_MIN	1
> +#define GRCANFD_FDCONF_SJW_MAX	8
> +#define GRCANFD_FDCONF_PS1_MIN	1
> +#define GRCANFD_FDCONF_PS1_MAX	15
> +#define GRCANFD_FDCONF_PS2_MIN	2
> +#define GRCANFD_FDCONF_PS2_MAX	8
> +#define GRCANFD_FDCONF_SCALER_MIN	0
> +#define GRCANFD_FDCONF_SCALER_MAX	255
> +#define GRCANFD_FDCONF_SCALER_INC	1
> +
> +#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.
> +	 */
> +	bool fd;
> +	bool txbug_possible;

Please move the bools the end of the struct for better packing.

> +	const struct can_bittiming_const *bt_const;
> +	int (*set_bittiming)(struct net_device *dev);
> +};
> +
> +static const struct grcan_hwcap grcan_hwcap;
> +static const struct of_device_id grcan_match[];

can you avoid forward declaration by re-arranging you code?

> +
>  struct grcan_dma_buffer {
>  	size_t size;
>  	void *buf;
> @@ -304,6 +370,8 @@ struct grcan_priv {
>  	 */
>  	bool resetting;
>  	bool closing;
> +
> +	const struct grcan_hwcap *hwcap;

Same here, move the pointer in front of the bools. You can check the
struct packing with "pahole -a /path/to/driver.o"

>  };
>
>  /* Wait time for a short wait for ongoing to clear */
> @@ -402,6 +470,19 @@ static const struct can_bittiming_const grcan_bittiming_const = {
>  	.brp_inc	= GRCAN_CONF_SCALER_INC,
>  };
>
> +/* GRCANFD nominal boundaries for baud-rate parameters */
> +static const struct can_bittiming_const grcanfd_bittiming_const = {
> +	.name		= DRV_NAME,
> +	.tseg1_min	= GRCANFD_NOMCONF_PS1_MIN,
> +	.tseg1_max	= GRCANFD_NOMCONF_PS1_MAX,
> +	.tseg2_min	= GRCANFD_NOMCONF_PS2_MIN,
> +	.tseg2_max	= GRCANFD_NOMCONF_PS2_MAX,
> +	.sjw_max	= GRCANFD_NOMCONF_SJW_MAX,
> +	.brp_min	= GRCANFD_NOMCONF_SCALER_MIN + 1,
> +	.brp_max	= GRCANFD_NOMCONF_SCALER_MAX + 1,
> +	.brp_inc	= GRCANFD_NOMCONF_SCALER_INC,
> +};
> +
>  static int grcan_set_bittiming(struct net_device *dev)
>  {
>  	struct grcan_priv *priv = netdev_priv(dev);
> @@ -439,12 +520,55 @@ static int grcan_set_bittiming(struct net_device *dev)
>  	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);
> +	netdev_dbg(dev, "setting timing=0x%x\n", timing);

nice!

>  	grcan_write_bits(&regs->conf, timing, GRCAN_CONF_TIMING);
>
>  	return 0;
>  }
>
> +static int grcanfd_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;
> +	const char *msg;
> +	u32 timing = 0;
> +	int sjw, ps1, ps2, scaler;
> +
> +	/* Should never happen - function will not be called when
> +	 * device is up
> +	 */
> +	if (grcan_read_bits(&regs->ctrl, GRCAN_CTRL_ENABLE))
> +		return -EBUSY;

The framework will take care of this

> +
> +	sjw = bt->sjw;
> +	ps1 = (bt->prop_seg + bt->phase_seg1); /* tseg1 */

nitpick:
I think the comment can be removed, it's obvious.

> +	ps2 = bt->phase_seg2;
> +	scaler = (bt->brp - 1);

please remove the ( )

> +	netdev_dbg(dev, "Request for SJW=%d, PS1=%d, PS2=%d, SCALER=%d",
> +		   sjw, ps1, ps2, scaler);

I think the debug can be removed

> +	if (sjw > min(ps1, ps2)) {
> +		msg = "SJW <= MIN(PS1,PS2) must hold: PS1=%d, PS2=%d, SJW=%d\n";
> +		netdev_err(dev, msg, ps1, ps2, sjw);
> +		return -EINVAL;
> +	}
> +	if (ps2 < sjw) {
> +		netdev_err(dev, "PS2 >= SJW must hold: PS2=%d, SJW=%d\n",
> +			   ps2, sjw);
> +		return -EINVAL;
> +	}

The framework already does these checks for you:

https://elixir.bootlin.com/linux/v6.17.8/source/drivers/net/can/dev/bittiming.c#L18

> +
> +	timing |= (sjw << GRCANFD_NBTR_SJW_BIT) & GRCANFD_NBTR_SJW;
> +	timing |= (ps1 << GRCANFD_NBTR_PS1_BIT) & GRCANFD_NBTR_PS1;
> +	timing |= (ps2 << GRCANFD_NBTR_PS2_BIT) & GRCANFD_NBTR_PS2;
> +	timing |= (scaler << GRCANFD_NBTR_SCALER_BIT) &
> +		  GRCANFD_NBTR_SCALER;

Please use FIELD_PREP() instead of doing the shifting yourself.

> +	netdev_info(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)
>  {
> @@ -1569,7 +1693,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;
> @@ -1592,14 +1717,15 @@ 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_bittiming = hwcap->set_bittiming;
>  	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;
> @@ -1644,22 +1770,32 @@ static int grcan_probe(struct platform_device *ofdev)
>  {
>  	struct device_node *np = ofdev->dev.of_node;
>  	struct device_node *sysid_parent;
> +	const struct of_device_id *of_id;
> +	const struct grcan_hwcap *hwcap = &grcan_hwcap;
                                          ^^^^^^^^^^^^^

Why do you initialize this variable?

The networking subsystem thinks that the variables should be placed in
reverse-christmas-tree style. Here it should be possible to arrange it
like this:

>  {
> +	const struct grcan_hwcap *hwcap = &grcan_hwcap;
>  	struct device_node *np = ofdev->dev.of_node;
>  	struct device_node *sysid_parent;
> +	const struct of_device_id *of_id;


>  	struct clk *clk;
>  	u32 sysid, ambafreq;
>  	int irq, err;
>  	void __iomem *base;
>  	bool txbug = true;
>
> +	of_id = of_match_device(grcan_match, &ofdev->dev);
> +	if (of_id && of_id->data)
> +		hwcap = (struct grcan_hwcap *)of_id->data;
> +

please use device_get_match_data(&ofdev->dev);

regards,
Marc

--
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 06/10] can: grcan: optimize DMA by 32-bit accesses

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 2610 bytes --]

On 18.11.2025 10:21:11, Arun Muthusamy wrote:
> From: Daniel Hellstrom <daniel@gaisler.com>
>
> Optimizes DMA transfers in the GRCAN driver by reorganizing
> data handling to use 32-bit accesses instead of individual
> byte accesses.
>
> Signed-off-by: Arun Muthusamy <arun.muthusamy@gaisler.com>
> Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
> ---
>  drivers/net/can/grcan.c | 21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
> index b9b0dd7d53f6..e367581faa57 100644
> --- a/drivers/net/can/grcan.c
> +++ b/drivers/net/can/grcan.c
> @@ -1287,7 +1287,7 @@ static int grcan_receive(struct net_device *dev, int budget)
>  	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);
> @@ -1323,10 +1323,10 @@ static int grcan_receive(struct net_device *dev, int budget)
>  		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->can_dlc > 0) {
> +				*(u32 *)(cf->data) = slot[2];
> +				if (cf->can_dlc > 4)
> +					*(u32 *)(cf->data + 4) = slot[3];

Can you can use memcpy() for this?

>  			}

>
>  			stats->rx_bytes += cf->len;
> @@ -1466,8 +1466,7 @@ static netdev_tx_t grcan_start_xmit(struct sk_buff *skb,
>  	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;
>
> @@ -1520,10 +1519,10 @@ static netdev_tx_t grcan_start_xmit(struct sk_buff *skb,
>  	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;
> +	if (dlc > 0) {
> +		slot[2] = *(u32 *)(cf->data); /* data aligned 64-bit */
> +		if (dlc > 4)
> +			slot[3] = *(u32 *)(cf->data + 4);

Can you can use memcpy() for this?

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 07/10] can: grcan: set DMA mask for GRCAN and GRCANFD to 32-bit

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1487 bytes --]

On 18.11.2025 10:21:12, Arun Muthusamy wrote:
> 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: Arun Muthusamy <arun.muthusamy@gaisler.com>
> Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
> ---
>  drivers/net/can/grcan.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
> index e367581faa57..51a10fae2faf 100644
> --- a/drivers/net/can/grcan.c
> +++ b/drivers/net/can/grcan.c
> @@ -1074,6 +1074,12 @@ static int grcan_allocate_dma_buffers(struct net_device *dev,
>
>  	/* Extra GRCAN_BUFFER_ALIGNMENT to allow for alignment */
>  	dma->base_size = lsize + ssize + GRCAN_BUFFER_ALIGNMENT;
> +
> +	/* On 64-bit systems.. GRCAN and GRCANFD can only address 32-bit */
> +	if (dma_set_mask_and_coherent(priv->ofdev_dev, DMA_BIT_MASK(32))) {
> +		netdev_warn(dev, "No suitable DMA available\n");
> +		return -ENOMEM;

Please move this to probe(), return the error code, and use
dev_err_probe() for the error message.

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 08/10] can: grcan: Add saving and restoring of CAN FD baud-rate registers

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 713 bytes --]

On 18.11.2025 10:21:13, Arun Muthusamy wrote:
> 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.

What about removing the do_set_bittiming callback

	priv->can.do_set_bittiming = grcan_set_bittiming;

and calling grcan_set_bittiming() explicitly after the reset?

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 10/10] can: grcan: Add CANFD support alongside legacy CAN

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 9657 bytes --]

On 18.11.2025 10:21:15, Arun Muthusamy wrote:
> Include CANFD support with the legacy CAN support, enabling
> support for extended data payloads up to 64 bytes, higher bit rates,
> handle canecho frames.
>
> Signed-off-by: Arun Muthusamy <arun.muthusamy@gaisler.com>
> ---
>  drivers/net/can/grcan.c | 240 ++++++++++++++++++++++++++++------------
>  1 file changed, 167 insertions(+), 73 deletions(-)
>
> diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
> index 8753bff4f917..ff7ab979d2c9 100644
> --- a/drivers/net/can/grcan.c
> +++ b/drivers/net/can/grcan.c
> @@ -44,6 +44,8 @@
>
>  #define GRCAN_RESERVE_SIZE(slot1, slot2) (((slot2) - (slot1)) / 4 - 1)
>
> +#define CHECK_SLOT_FDF(slot) ((slot) & GRCAN_RX_FDF)
> +
>  struct grcan_registers {
>  	u32 conf;	/* 0x00 */
>  	u32 stat;	/* 0x04 */
> @@ -181,8 +183,11 @@ struct grcan_registers {
>  			  | GRCAN_IRQ_TXAHBERR | GRCAN_IRQ_RXAHBERR	\
>  			  | GRCAN_IRQ_TXLOSS)
>  #define GRCAN_IRQ_DEFAULT (GRCAN_IRQ_RX | GRCAN_IRQ_TX | GRCAN_IRQ_ERRORS)
> +#define GRCAN_CTRL_MODES (CAN_CTRLMODE_LISTENONLY | CAN_CTRLMODE_ONE_SHOT)
> +#define GRCAN_CTRL_MODES_FD (CAN_CTRLMODE_LISTENONLY | CAN_CTRLMODE_ONE_SHOT | CAN_CTRLMODE_FD)

IMHO no need for these defines, just list the CAN_CTRLMODE

>
>  #define GRCAN_MSG_SIZE		16
> +#define GRCAN_CLASSIC_DATA_SIZE 8
>
>  #define GRCAN_MSG_IDE		0x80000000
>  #define GRCAN_MSG_RTR		0x40000000
> @@ -264,6 +269,12 @@ 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)
> +
> +#define GRCAN_RX_BRS  BIT(25)
> +#define GRCAN_RX_FDF  BIT(26)
> +
>  /* Hardware capabilities */
>  struct grcan_hwcap {
>  	/* CAN-FD capable, indicates GRCANFD IP.
> @@ -326,6 +337,13 @@ 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.
> +	 * It's important for ensuring that we correctly manage the echo skb.
> +	 */
> +	u32 echo_skb_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.
> @@ -637,7 +655,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
> @@ -648,19 +666,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;
>  }
> @@ -1174,6 +1195,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->echo_skb_idx = 0;
>  		spin_unlock_irqrestore(&priv->lock, flags);
>  		return err;
>  	}
> @@ -1223,7 +1245,6 @@ static int grcan_open(struct net_device *dev)
>  		netif_start_queue(dev);
>  	priv->resetting = false;
>  	priv->closing = false;
> -
>  	spin_unlock_irqrestore(&priv->lock, flags);
>
>  	return 0;
> @@ -1294,20 +1315,29 @@ 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 + ((len - GRCAN_CLASSIC_DATA_SIZE + GRCAN_MSG_SIZE) / GRCAN_MSG_SIZE);
> +}
> +
>  static int grcan_receive(struct net_device *dev, int budget)
>  {
>  	struct grcan_priv *priv = netdev_priv(dev);
>  	struct grcan_registers __iomem *regs = priv->regs;
>  	struct grcan_dma *dma = &priv->dma;
>  	struct net_device_stats *stats = &dev->stats;
> -	struct can_frame *cf;
> +	struct canfd_frame *cf;
>  	struct sk_buff *skb;
> -	u32 wr, rd, startrd;
> +	u32 wr, rd, dlc, startrd;
>  	u32 *slot;
>  	u32 rtr, eff;
> -	int work_done = 0;
> +	u8 *data;
> +	int i, bds, payload_offset, copy_len, work_done = 0;

Please take care of the infamous reverse-xmas-tree.

>
>  	rd = grcan_read_reg(&regs->rxrd);
> +
>  	startrd = rd;
>  	for (work_done = 0; work_done < budget; work_done++) {
>  		/* Check for packet to receive */
> @@ -1315,44 +1345,70 @@ 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) {
> +		slot = dma->rx.buf + rd;
> +
> +		if (CHECK_SLOT_FDF(slot[1]))

IMHO "slot[1] & GRCAN_RX_FDF" is more readable

> +			skb = alloc_canfd_skb(dev, &cf);
> +		else
> +			skb = alloc_can_skb(priv->dev, (struct can_frame **)&cf);
> +
> +		if (unlikely(!skb)) {
>  			netdev_err(dev,
>  				   "dropping frame: skb allocation failed\n");
>  			stats->rx_dropped++;
>  			continue;
>  		}
>
> -		slot = dma->rx.buf + rd;
> -		eff = slot[0] & GRCAN_MSG_IDE;
> -		rtr = slot[0] & GRCAN_MSG_RTR;
> -		if (eff) {
> -			cf->can_id = ((slot[0] & GRCAN_MSG_EID)
> -				      >> GRCAN_MSG_EID_BIT);
> -			cf->can_id |= CAN_EFF_FLAG;
> -		} else {
> -			cf->can_id = ((slot[0] & GRCAN_MSG_BID)
> -				      >> GRCAN_MSG_BID_BIT);
> -		}
> -		cf->len = can_cc_dlc2len((slot[1] & GRCAN_MSG_DLC)
> -					  >> GRCAN_MSG_DLC_BIT);
> -		if (rtr) {
> -			cf->can_id |= CAN_RTR_FLAG;
> -		} else {
> -			if (cf->can_dlc > 0) {
> -				*(u32 *)(cf->data) = slot[2];
> -				if (cf->can_dlc > 4)
> -					*(u32 *)(cf->data + 4) = slot[3];
> +		dlc = (slot[1] & GRCAN_MSG_DLC) >> GRCAN_MSG_DLC_BIT;
> +		if (CHECK_SLOT_FDF(slot[1]))
> +			cf->len = can_fd_dlc2len(dlc);
> +		else
> +			cf->len = can_cc_dlc2len(dlc);
> +
> +		bds = grcan_numbds(cf->len);
> +		payload_offset = 0;
> +		data = cf->data;
> +
> +		for (i = 0; i < bds; i++) {
> +			slot = dma->rx.buf + rd;
> +
> +			if (i == 0) {
> +				eff = slot[0] & GRCAN_MSG_IDE;
> +				rtr = slot[0] & GRCAN_MSG_RTR;
> +				if (eff) {
> +					cf->can_id = ((slot[0] & GRCAN_MSG_EID)
> +						      >> GRCAN_MSG_EID_BIT);

you can use up to 100 chars now, but please move the ">>" to the end of
the line

> +					cf->can_id |= CAN_EFF_FLAG;
> +				} else {
> +					cf->can_id = ((slot[0] & GRCAN_MSG_BID)
> +						      >> GRCAN_MSG_BID_BIT);
> +				}
> +				if (rtr)
> +					cf->can_id |= CAN_RTR_FLAG;
> +
> +				dlc = (slot[1] & GRCAN_MSG_DLC) >> GRCAN_MSG_DLC_BIT;
> +				if (CHECK_SLOT_FDF(slot[1]))
> +					cf->len = can_fd_dlc2len(dlc);
> +				else
> +					cf->len = can_cc_dlc2len(dlc);
> +
> +				copy_len = min(cf->len, GRCAN_CLASSIC_DATA_SIZE);
> +				memcpy(data, &slot[2], copy_len);
> +				payload_offset += copy_len;
> +			} else {
> +				copy_len =  min(cf->len - payload_offset, GRCAN_MSG_SIZE);
> +				memcpy(data + payload_offset, slot, copy_len);
> +				payload_offset += copy_len;
>  			}
> -
> -			stats->rx_bytes += cf->len;
> +			rd += GRCAN_MSG_SIZE;
> +			if (rd >= dma->tx.size)
> +				rd -= dma->tx.size;
>  		}
> -		stats->rx_packets++;
>
> +		/* Update statistics and read pointer */
> +		stats->rx_packets++;
> +		stats->rx_bytes += cf->len;
>  		netif_receive_skb(skb);
> -
> -		rd = grcan_ring_add(rd, GRCAN_MSG_SIZE, dma->rx.size);
>  	}
>
>  	/* Make sure everything is read before allowing hardware to
> @@ -1479,12 +1535,15 @@ static netdev_tx_t grcan_start_xmit(struct sk_buff *skb,
>  	struct grcan_priv *priv = netdev_priv(dev);
>  	struct grcan_registers __iomem *regs = priv->regs;
>  	struct grcan_dma *dma = &priv->dma;
> -	struct can_frame *cf = (struct can_frame *)skb->data;
> -	u32 id, txwr, txrd, space, txctrl;
> -	int slotindex;
> -	u32 *slot;
> -	u32 rtr, eff, dlc, tmp, err;
> +	struct can_frame *cf;
> +	struct canfd_frame *cfd;
> +	int i, bds, copy_len, payload_offset;
>  	unsigned long flags;
> +	u8 *payload;
> +	u8 len;
> +	u32 *slot;
> +	u32 eff, rtr, dlc, tmp, err, can_id;
> +	u32 id, txwr, txrd, space, txctrl;

reverse-xmas-tree for all vars you touch please

regards,
Marc

--
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 02/10] dt-bindings: net: can: grcan: Convert GRCAN CAN controllers binding from txt to YAML

[Arun Muthusamy]

Hi Krzysztof,

Thank you for your thorough review and insightful questions. I’d like to 
clarify a few points regarding the DT binding and get your guidance.

Node name vs. compatible matching:
SPARC systems do not use DTS files; the device tree is generated by the 
PROM. On LEON (SPARC32), AMBA Plug & Play information creates the DT 
properties, and drivers historically match devices based on node names.
For DTS-based systems such as NOEL, this patch series adds 
compatible-string matching. To reflect this, I updated the $nodename 
pattern to support LEON-style node names:
properties:
   $nodename:
     pattern: "^(GAISLER_GRCAN|01_03d|GAISLER_GRHCAN|01_034)$"
I’d appreciate any suggestions on the preferred way to describe this 
dual matching approach: node name for PROM-based LEON, compatible string 
for DTS-based NOEL.


Freq and Clocks:
The driver needs to support both LEON and NOEL platforms:
LEON: relies on the freq property
NOEL: uses a standard clocks binding
Because of this dual approach, the freq property is no longer required 
in the DTS binding itself.
It is only relevant for LEON/PROM-based systems and is handled 
internally by the driver


Systemid:
The driver now reads systemid directly from /ambapp0, so the property no 
longer needs to be defined in the DTS. The previous documentation was 
outdated and should have been updated after commit:
1e93ed26acf0 ("can: grcan: grcan_probe(): fix broken system id check for 
errata workaround needs")


Thanks,

-- 
BR,

Arun Muthusamy
Software Engineer
Frontgrade Gaisler
T : +46 (0) 700 558 528
arun.muthusamy@gaisler.com

Frontgrade Gaisler AB, Kungsgatan 12, SE-411 19 GÖTEBORG, Sweden.
+46 (0) 31 775 8650, www.gaisler.com

On 18.11.2025 12:01, Krzysztof Kozlowski wrote:
> On 18/11/2025 10:21, Arun Muthusamy wrote:
>> Migrate device tree bindings for Gaisler GRCAN, GRHCAN
>> and GRCANFD CAN controllers from a text format to YAML format.
>>     - Add properties such as `compatible`, `reg`, `interrupts`
> 
> Odd indentation. Please write readable commit msgs.
> 
> Also:
> 1. Why? You need to explain why you are changing binding during 
> conversion.
> 2. Reg was already there, so I don't understand why you need to add it.
> 
> 
>>     and `clocks` for the CAN controllers.
>>     - Removal of the old `grcan.txt` file as its contents have
>>     been fully migrated to the YAML file.
> 
> Drop, that's not relevant.
> 
>>     - YAML file includes examples of device tree bindings for
>>     the CAN controllers
> 
> Drop, not relevant. Please look at git history how commits are written.
> 
>> 
>> Signed-off-by: Arun Muthusamy <arun.muthusamy@gaisler.com>
>> ---
>>  .../bindings/net/can/gaisler,grcan.yaml       | 85 
>> +++++++++++++++++++
>>  .../devicetree/bindings/net/can/grcan.txt     | 28 ------
>>  2 files changed, 85 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..521bdd89f130
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/can/gaisler,grcan.yaml
>> @@ -0,0 +1,85 @@
>> +# 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
> 
> Blank line
> 
>> +  name:
>> +    description: |
> 
> Do not need '|' unless you need to preserve formatting.
> 
>> +      Fallback on node name matching for systems that don't provide 
>> compatible.
>> +    enum:
>> +      - GAISLER_GRCAN
>> +      - 01_03d
>> +      - GAISLER_GRHCAN
>> +      - "01_034"
> 
> This does not really work. Are you really defining here "name" 
> property?
> 
>> +
>> +  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 that utilize the common 
>> clock
>> +      framework is not supported.
> 
> Missing systemid. Your commit msg must explain any changes done to the
> binding during conversion.
> 
>> +
>> +unevaluatedProperties: false
> 
> This goes after required block.
> 
>> +
>> +required:
> 
> compatible as well
> 
>> +  - reg
>> +  - interrupts
> 
> Where is freq? It was required in the old binding. Again, you need to
> explain the changes.
> 
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +    can@ff400000 {
>> +        compatible = "gaisler,grcanfd";
>> +        clocks = <&sysclock>;
>> +        reg = <0xff400000 0x400>;
>> +        interrupt-parent = <&plic0>;
>> +        interrupts = <6>;
>> +    };
> 
> One example is enough
> 
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +    can@ff400000 {
>> +        compatible = "gaisler,grcan";
>> +        clocks = <&sysclock>;
>> +        reg = <0xff400000 0x400>;
>> +        interrupt-parent = <&plic0>;
>> +        interrupts = <6>;
>> +    };
>> +  - |
>> +    GAISLER_GRCAN@ff840000 {
> 
> Especially no such examples. Please read DTS coding style.
> 
>> +        reg = <0xff840000 0x400>;
>> +        freq = <50000000>;
>> +        interrupts = <16>;
>> +    };
>> 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 @@
> Best regards,
> Krzysztof

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

[Arun Muthusamy]

Hi Krzysztof,

Thank you for your thorough review. I wanted to get your guidance 
regarding the clock property in the DT binding.

In the binding, I included the clocks property with maxItems: 1 to 
indicate that a clock should be described. The driver calls:

clk = devm_clk_get(dev, NULL);

Since we pass NULL, the driver always requests the first (and only) 
clock from the clocks property.

I want to ensure the binding is fully compliant with the Linux DT ABI. 
Could you advise the preferred way to document the clocks and 
clock-names properties in this scenario? Specifically:

Do we still need a clock-names entry even if the driver never uses it by 
name?
For LEON systems, the driver relies on the "freq" property, while NOEL 
systems use a standard "clocks" binding. Given this dual approach, 
should the clocks property be marked as optional or required in the 
binding?

Thank you for your time and help.

-- 
BR,

Arun Muthusamy
Software Engineer
Frontgrade Gaisler
T : +46 (0) 700 558 528
arun.muthusamy@gaisler.com

Frontgrade Gaisler AB, Kungsgatan 12, SE-411 19 GÖTEBORG, Sweden.
+46 (0) 31 775 8650, www.gaisler.com

On 18.11.2025 12:01, Krzysztof Kozlowski wrote:
> On 18/11/2025 10:21, Arun Muthusamy wrote:
>> From: Daniel Hellstrom <daniel@gaisler.com>
>> 
>> Add clock handling and add error messages for missing 'freq' DT 
>> property.
>> 
>> Signed-off-by: Arun Muthusamy <arun.muthusamy@gaisler.com>
>> Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
>> ---
>>  drivers/net/can/grcan.c | 19 ++++++++++++++++---
>>  1 file changed, 16 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/net/can/grcan.c b/drivers/net/can/grcan.c
>> index 3b1b09943436..538a9b4f82ab 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"
>> @@ -1644,6 +1644,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;
>> @@ -1663,8 +1664,20 @@ 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);
> 
> Nope, your binding said there is no clock... you cannot add 
> undocumented
> ABI.
> 
> 
> Best regards,
> Krzysztof

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

[Krzysztof Kozlowski]

On 24/11/2025 10:46, Arun Muthusamy wrote:
> Hi Krzysztof,
> 
> Thank you for your thorough review. I wanted to get your guidance 
> regarding the clock property in the DT binding.
> 
> In the binding, I included the clocks property with maxItems: 1 to 
> indicate that a clock should be described. The driver calls:

Ah, you added clocks, I completely missed it.

> 
> clk = devm_clk_get(dev, NULL);
> 
> Since we pass NULL, the driver always requests the first (and only) 
> clock from the clocks property

Yes this is correct.

.
> 
> I want to ensure the binding is fully compliant with the Linux DT ABI. 

It is fine, please ignore my comment. It was a mistake.


Best regards,
Krzysztof

Re: [PATCH 02/10] dt-bindings: net: can: grcan: Convert GRCAN CAN controllers binding from txt to YAML

[Krzysztof Kozlowski]

On 24/11/2025 10:37, Arun Muthusamy wrote:
> Hi Krzysztof,
> 
> Thank you for your thorough review and insightful questions. I’d like to 
> clarify a few points regarding the DT binding and get your guidance.
> 
> Node name vs. compatible matching:

I don't understand what you are referring to. You cut everything and
pasted something, which I have no clue what is that. I read many patches
per day, so you are not helping here to understand the context.

Please do not top-post.

Best regards,
Krzysztof

Re: [PATCH 08/10] can: grcan: Add saving and restoring of CAN FD baud-rate registers

[Arun Muthusamy]

Thank you for your suggestion.

 From the design point of view, I prefer to retain the "do_set_bittiming" 
callback to maintain flexibility in adjusting baud rates by the 
framework. Since CAN and CANFD configurations differ as they use 
different registers for timing configuration and Specifically, the 
timing configuration is closely tied to the reset logic only in 
scenarios where the baud rate for CANFD is stored in a register. This 
differentiation is not applicable to CAN timing configuration, as CAN 
and CANFD are handled differently.

-- 
BR,

Arun Muthusamy
Software Engineer
Frontgrade Gaisler
T : +46 (0) 700 558 528
arun.muthusamy@gaisler.com

Frontgrade Gaisler AB, Kungsgatan 12, SE-411 19 GÖTEBORG, Sweden.
+46 (0) 31 775 8650, www.gaisler.com

On 21.11.2025 13:50, Marc Kleine-Budde wrote:
> On 18.11.2025 10:21:13, Arun Muthusamy wrote:
>> 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.
> 
> What about removing the do_set_bittiming callback
> 
> 	priv->can.do_set_bittiming = grcan_set_bittiming;
> 
> and calling grcan_set_bittiming() explicitly after the reset?
> 
> regards,
> Marc

Re: [PATCH 02/10] dt-bindings: net: can: grcan: Convert GRCAN CAN controllers binding from txt to YAML

[Arun Muthusamy]

Hi Krzysztof,

Thank you for your thorough review. I’d like to clarify a few points 
regarding the DT binding and get your guidance.

On 11/18/25 12:01, Krzysztof Kozlowski wrote:
> On 18/11/2025 10:21, Arun Muthusamy wrote:
>> +      Fallback on node name matching for systems that don't provide compatible.
>> +    enum:
>> +      - GAISLER_GRCAN
>> +      - 01_03d
>> +      - GAISLER_GRHCAN
>> +      - "01_034"
> This does not really work. Are you really defining here "name" property?

The driver supports two of the platforms which are LEON and NOEL 
platforms. PROM-based *LEON* systems identify uses the "node name" 
property, while DTS based *NOEL* systems use proper "|compatible"|strings. On LEON (SPARC32), AMBA Plug & Play information creates the DT 
properties, and drivers historically match devices based on node names.
To reflect this, I updated the $nodename pattern to support LEON-style 
node names: properties:
   $nodename:
     pattern: "^(GAISLER_GRCAN|01_03d|GAISLER_GRHCAN|01_034)$" I’d 
appreciate any suggestions on the preferred way to describe this node 
name for PROM-based LEON.

>> +
>> +  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 that utilize the common clock
>> +      framework is not supported.
> Missing systemid. Your commit msg must explain any changes done to the
> binding during conversion.

The driver now reads systemid directly from /ambapp0, so the property no 
longer needs to be defined in the DTS. The previous documentation was 
outdated and should have been updated after commit:
1e93ed26acf0 ("can: grcan: grcan_probe(): fix broken system id check for 
errata workaround needs").

>> +  - reg
>> +  - interrupts
> Where is freq? It was required in the old binding. Again, you need to
> explain the changes.

LEON: relies on the freq property
NOEL: uses a standard clocks binding
Because of this dual approach, the freq property is no longer required 
in the DTS binding itself as theAMBA Plug & Play creates the DT properties.


Thanks,

-- 
BR,

Arun Muthusamy
Software Engineer
Frontgrade Gaisler
T : +46 (0) 700 558 528
arun.muthusamy@gaisler.com

Frontgrade Gaisler AB, Kungsgatan 12, SE-411 19 GÖTEBORG, Sweden.
+46 (0) 31 775 8650,www.gaisler.com <http://www.gaisler.com/>

Re: [PATCH 08/10] can: grcan: Add saving and restoring of CAN FD baud-rate registers

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 2051 bytes --]

On 11.12.2025 10:13:15, Arun Muthusamy wrote:
> From the design point of view, I prefer to retain the "do_set_bittiming"
> callback to maintain flexibility in adjusting baud rates by the framework.

If you don't implement the do_set_bittiming callback you don't loose any
flexibility.

> Since CAN and CANFD configurations differ as they use different registers
> for timing configuration and Specifically, the timing configuration is
> closely tied to the reset logic only in scenarios where the baud rate for
> CANFD is stored in a register. This differentiation is not applicable to CAN
> timing configuration, as CAN and CANFD are handled differently.

From my point of view not implementing the do_set_bittiming makes it
easier from the driver's perspective.

Now
---

If the interface is down do_set_bittiming may be called at any time.

Consider a scenario where the device and driver support deep sleep,
power down clocks/voltages etc. .... In the do_set_bittiming callback,
you must switch on the device, write the bit timing information and
switch the device off again. Some devices lose their configuration
when they are switched off. It therefore makes no sense to implement
this callback on these devices.


What I propose
--------------

Do not implement do_set_bittiming.

If the interface is down the user can configure the bit timing. The
information is stored as usual in priv->can.bittiming,
priv->can.data_bittiming.

If the user brings up the interface the open callback is executed. In
this callback you power on the device, do a reset and then call
grcan_set_bittiming() explicitly. You don't have to take care to
preserve the timing register information around the reset.


Does this make sense?

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]