mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v4 0/7] can: rcar_canfd: Add support for Renesas RZ/G3S
@ 2026-09-02 14:15 Claudiu Beznea
  2026-09-02 14:15 ` [PATCH v4 1/7] dt-bindings: can: renesas,rcar-canfd: Document RZ/G3S SoC Claudiu Beznea
                   ` (7 more replies)
  0 siblings, 8 replies; 20+ messages in thread
From: Claudiu Beznea @ 2026-09-02 14:15 UTC (permalink / raw)
  To: mkl, mailhol, robh, krzk+dt, conor+dt, geert+renesas,
	magnus.damm, biju.das.jz, fabrizio.castro.jz
  Cc: claudiu.beznea, linux-can, devicetree, linux-renesas-soc,
	linux-kernel, Claudiu Beznea

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Hi,

Series adds CAN support for the Renesas RZ/G3S SoC. Along with it a typo
fix patch was added on the CAN driver.

Merge strategy:
- patches 1-5/7 should go though the CAN tree
- patches 6-7/7 should go though Renesas tree

Thank you,
Claudiu

Changes in v4:
- dropped patch "clk: r9a08g045-cpg: Add clocks and resets for CAN-FD"
  since it was applied
- collected tags
- s/mode_select_na/fixed_canfd in patch 4/7
- used TCAN1046 compatible in patch 7/7 and adjusted the standby-gpios
  property

Changes in v3:
- addressed sashiko review comments

Changes in v2:
- collected tags
- addressed sashiko review comments

Claudiu Beznea (7):
  dt-bindings: can: renesas,rcar-canfd: Document RZ/G3S SoC
  can: rcar_canfd: Fix typos in macro names
  can: rcar_canfd: Allow the CAN FD clock to be sourced from fck
  can: rcar_canfd: Do not set registers selecting the CAN mode
  can: rcar_canfd: Add support for Renesas RZ/G3S
  arm64: dts: renesas: r9a08g045: Add CAN-FD node
  arm64: dts: renesas: rzg3s-smarc: Enable CAN-FD

 .../bindings/net/can/renesas,rcar-canfd.yaml  | 20 +++++-
 arch/arm64/boot/dts/renesas/r9a08g045.dtsi    | 39 +++++++++++
 .../boot/dts/renesas/rzg3s-smarc-switches.h   | 12 ++++
 arch/arm64/boot/dts/renesas/rzg3s-smarc.dtsi  | 45 +++++++++++++
 drivers/net/can/rcar/rcar_canfd.c             | 65 +++++++++++++++----
 5 files changed, 168 insertions(+), 13 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v4 1/7] dt-bindings: can: renesas,rcar-canfd: Document RZ/G3S SoC
  2026-09-02 14:15 [PATCH v4 0/7] can: rcar_canfd: Add support for Renesas RZ/G3S Claudiu Beznea
@ 2026-09-02 14:15 ` Claudiu Beznea
  2026-10-16 13:34   ` Marc Kleine-Budde
  2026-09-02 14:15 ` [PATCH v4 2/7] can: rcar_canfd: Fix typos in macro names Claudiu Beznea
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Claudiu Beznea @ 2026-09-02 14:15 UTC (permalink / raw)
  To: mkl, mailhol, robh, krzk+dt, conor+dt, geert+renesas,
	magnus.damm, biju.das.jz, fabrizio.castro.jz
  Cc: claudiu.beznea, linux-can, devicetree, linux-renesas-soc,
	linux-kernel, Claudiu Beznea

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

The CAN FD controller found on the Renesas RZ/G3S SoC is largely compatible
with the variant present on the RZ/G3E SoC. The main differences are:
- the RZ/G3S provides only two CAN FD channels
- the RZ/G3S supports only CAN FD operation; the Channel n CAN FD
  Configuration Register does not implement the bits used to select
  classical CAN-only mode (bit 30) or CAN FD-only mode (bit 28);
  consequently, bit 31 (CAN FD Frame Distinction Enable) of the same
  register is also not implemented
- some bits in several registers (mainly reserved or status bits) are
  read-write on the RZ/G3S but read-only on the RZ/G3E; their behavior is
  otherwise identical: the bits read back as 0 on both SoCs and software
  is allowed to write only 0 to them on the RZ/G3S
- the RZ/G3S provides 256 acceptance filters, compared to 768 on the
  RZ/G3E
- the RZ/G3S can use PCLK clock as the CAN FD clock source through an
  internal clock divider, while also supporting an external CAN FD clock
  source

Since:
- the SoC clock generator provides to the CAN IP only the peripheral and
  the RAM clocks
- when sourced from the peripheral clock, the CAN-FD clock is obtained
  inside the IP itself by dividing the peripheral clock
- the assigned-clocks and assigned-clock-rates properties are specific to
  the CAN-FD clock
the assigned-clocks and assigned-clock-rates properties were dropped from
the required properties list of the Renesas RZ/G3S SoC.

Add documentation for the Renesas RZ/G3S SoC.

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v4:
- collected tags

Changes in v3:
- updated the patch description to reflect RZ/G3S has 256 acceptance
  filters and RZ/G3E has 768 acceptance filters
- kept compatibles alphanumerically sorted

Changes in v2:
- forbid renesas,no-can-fd and renesas,fd-only since the IP supports
  only CAN-FD; for this didn't collect Biju's tag

 .../bindings/net/can/renesas,rcar-canfd.yaml  | 20 +++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
index b9d9dd7a7967..bbaaef8f4282 100644
--- a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
+++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
@@ -13,6 +13,7 @@ properties:
   compatible:
     oneOf:
       - enum:
+          - renesas,r9a08g045-canfd        # RZ/G3S
           - renesas,r9a09g047-canfd        # RZ/G3E
           - renesas,r9a09g077-canfd        # RZ/T2H
 
@@ -185,8 +186,6 @@ required:
   - clocks
   - clock-names
   - power-domains
-  - assigned-clocks
-  - assigned-clock-rates
   - channel0
   - channel1
 
@@ -198,6 +197,7 @@ allOf:
         compatible:
           contains:
             enum:
+              - renesas,r9a08g045-canfd
               - renesas,rzg2l-canfd
     then:
       properties:
@@ -266,6 +266,7 @@ allOf:
         compatible:
           contains:
             enum:
+              - renesas,r9a08g045-canfd
               - renesas,r9a09g077-canfd
               - renesas,rcar-gen3-canfd
               - renesas,rzg2l-canfd
@@ -330,6 +331,7 @@ allOf:
         compatible:
           contains:
             enum:
+              - renesas,r9a08g045-canfd
               - renesas,r9a09g047-canfd
               - renesas,rzg2l-canfd
     then:
@@ -350,6 +352,20 @@ allOf:
       properties:
         reset-names: false
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: renesas,r9a08g045-canfd
+    then:
+      properties:
+        renesas,no-can-fd: false
+        renesas,fd-only: false
+    else:
+      required:
+        - assigned-clocks
+        - assigned-clock-rates
+
 unevaluatedProperties: false
 
 examples:
-- 
2.43.0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v4 2/7] can: rcar_canfd: Fix typos in macro names
  2026-09-02 14:15 [PATCH v4 0/7] can: rcar_canfd: Add support for Renesas RZ/G3S Claudiu Beznea
  2026-09-02 14:15 ` [PATCH v4 1/7] dt-bindings: can: renesas,rcar-canfd: Document RZ/G3S SoC Claudiu Beznea
@ 2026-09-02 14:15 ` Claudiu Beznea
  2026-09-02 14:15 ` [PATCH v4 3/7] can: rcar_canfd: Allow the CAN FD clock to be sourced from fck Claudiu Beznea
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Claudiu Beznea @ 2026-09-02 14:15 UTC (permalink / raw)
  To: mkl, mailhol, robh, krzk+dt, conor+dt, geert+renesas,
	magnus.damm, biju.das.jz, fabrizio.castro.jz
  Cc: claudiu.beznea, linux-can, devicetree, linux-renesas-soc,
	linux-kernel, Claudiu Beznea

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

The bits 1..0 of the Channel n Control Register are named CHMDC (Channel
Mode select). Fix typos in macro names by replacing DMC with MDC.

Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v4:
- collected tags

Changes in v3:
- none

Changes in v2:
- collected tags

 drivers/net/can/rcar/rcar_canfd.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index fcc37b73ed43..b7c4fe3477ac 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -132,9 +132,9 @@
 #define RCANFD_CCTR_BEIE		BIT(8)
 #define RCANFD_CCTR_CSLPR		BIT(2)
 #define RCANFD_CCTR_CHMDC_MASK		(0x3)
-#define RCANFD_CCTR_CHDMC_COPM		(0x0)
-#define RCANFD_CCTR_CHDMC_CRESET	(0x1)
-#define RCANFD_CCTR_CHDMC_CHLT		(0x2)
+#define RCANFD_CCTR_CHMDC_COPM		(0x0)
+#define RCANFD_CCTR_CHMDC_CRESET	(0x1)
+#define RCANFD_CCTR_CHMDC_CHLT		(0x2)
 
 /* RSCFDnCFDCmSTS / RSCFDnCmSTS */
 #define RCANFD_CSTS_COMSTS		BIT(7)
@@ -828,7 +828,7 @@ static int rcar_canfd_reset_controller(struct rcar_canfd_global *gpriv)
 
 		rcar_canfd_update_bit(gpriv->base, RCANFD_CCTR(ch),
 				      RCANFD_CCTR_CHMDC_MASK,
-				      RCANFD_CCTR_CHDMC_CRESET);
+				      RCANFD_CCTR_CHMDC_CRESET);
 
 		/* Ensure Channel reset mode */
 		err = readl_poll_timeout((gpriv->base + RCANFD_CSTS(ch)), sts,
@@ -1504,7 +1504,7 @@ static int rcar_canfd_start(struct net_device *ndev)
 
 	/* Set channel to Operational mode */
 	rcar_canfd_update_bit(priv->base, RCANFD_CCTR(ch),
-			      RCANFD_CCTR_CHMDC_MASK, RCANFD_CCTR_CHDMC_COPM);
+			      RCANFD_CCTR_CHMDC_MASK, RCANFD_CCTR_CHMDC_COPM);
 
 	/* Verify channel mode change */
 	err = readl_poll_timeout((priv->base + RCANFD_CSTS(ch)), sts,
@@ -1578,7 +1578,7 @@ static void rcar_canfd_stop(struct net_device *ndev)
 
 	/* Transition to channel reset mode  */
 	rcar_canfd_update_bit(priv->base, RCANFD_CCTR(ch),
-			      RCANFD_CCTR_CHMDC_MASK, RCANFD_CCTR_CHDMC_CRESET);
+			      RCANFD_CCTR_CHMDC_MASK, RCANFD_CCTR_CHMDC_CRESET);
 
 	/* Check Channel reset mode */
 	err = readl_poll_timeout((priv->base + RCANFD_CSTS(ch)), sts,
-- 
2.43.0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v4 3/7] can: rcar_canfd: Allow the CAN FD clock to be sourced from fck
  2026-09-02 14:15 [PATCH v4 0/7] can: rcar_canfd: Add support for Renesas RZ/G3S Claudiu Beznea
  2026-09-02 14:15 ` [PATCH v4 1/7] dt-bindings: can: renesas,rcar-canfd: Document RZ/G3S SoC Claudiu Beznea
  2026-09-02 14:15 ` [PATCH v4 2/7] can: rcar_canfd: Fix typos in macro names Claudiu Beznea
@ 2026-09-02 14:15 ` Claudiu Beznea
  2026-10-16 13:39   ` Marc Kleine-Budde
  2026-09-02 14:15 ` [PATCH v4 4/7] can: rcar_canfd: Do not set registers selecting the CAN mode Claudiu Beznea
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Claudiu Beznea @ 2026-09-02 14:15 UTC (permalink / raw)
  To: mkl, mailhol, robh, krzk+dt, conor+dt, geert+renesas,
	magnus.damm, biju.das.jz, fabrizio.castro.jz
  Cc: claudiu.beznea, linux-can, devicetree, linux-renesas-soc,
	linux-kernel, Claudiu Beznea

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

On the Renesas RZ/G3S, the CAN FD clock can be sourced either from fck
(the peripheral clock) or from an external clock (can_clk). When fck is
used, it is divided internally by the CAN FD controller.

Adjust the existing canfd clock handling code to support the RZ/G3S CAN.

The existing struct rcar_canfd_hw_info instances were updated to address
the request in the Link discussion.

Link: https://lore.kernel.org/all/1d9719e3-10ff-4cd8-b729-55fea93c37ce@wanadoo.fr
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v4:
- collected tags

Changes in v3:
- set gpriv->can_clk = NULL if info->fcan_pclk

Changes in v2:
- changed the logic to request canfd only if info->fcan_pclk and contrary
  get the fcan clock from pclk; for this didn't collect the Rb tag

 drivers/net/can/rcar/rcar_canfd.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index b7c4fe3477ac..f9173774387f 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -444,6 +444,7 @@ struct rcar_canfd_hw_info {
 	unsigned ch_interface_mode:1;	/* Has channel interface mode */
 	unsigned shared_can_regs:1;	/* Has shared classical can registers */
 	unsigned external_clk:1;	/* Has external clock */
+	unsigned fcan_pclk:1;		/* Has fcan sourced from pclk. */
 };
 
 /* Channel priv data */
@@ -617,6 +618,7 @@ static const struct rcar_canfd_hw_info rcar_gen3_hw_info = {
 	.ch_interface_mode = 0,
 	.shared_can_regs = 0,
 	.external_clk = 1,
+	.fcan_pclk = 0,
 };
 
 static const struct rcar_canfd_hw_info rcar_gen4_hw_info = {
@@ -634,6 +636,7 @@ static const struct rcar_canfd_hw_info rcar_gen4_hw_info = {
 	.ch_interface_mode = 1,
 	.shared_can_regs = 1,
 	.external_clk = 1,
+	.fcan_pclk = 0,
 };
 
 static const struct rcar_canfd_hw_info rzg2l_hw_info = {
@@ -651,6 +654,7 @@ static const struct rcar_canfd_hw_info rzg2l_hw_info = {
 	.ch_interface_mode = 0,
 	.shared_can_regs = 0,
 	.external_clk = 1,
+	.fcan_pclk = 0,
 };
 
 static const struct rcar_canfd_hw_info r9a09g047_hw_info = {
@@ -668,6 +672,7 @@ static const struct rcar_canfd_hw_info r9a09g047_hw_info = {
 	.ch_interface_mode = 1,
 	.shared_can_regs = 1,
 	.external_clk = 0,
+	.fcan_pclk = 0,
 };
 
 static const struct rcar_canfd_hw_info r9a09g077_hw_info = {
@@ -685,6 +690,7 @@ static const struct rcar_canfd_hw_info r9a09g077_hw_info = {
 	.ch_interface_mode = 1,
 	.shared_can_regs = 1,
 	.external_clk = 1,
+	.fcan_pclk = 0,
 };
 
 /* Helper functions */
@@ -2190,13 +2196,20 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 	 */
 	gpriv->can_clk = devm_clk_get(dev, "can_clk");
 	if (IS_ERR(gpriv->can_clk) || (clk_get_rate(gpriv->can_clk) == 0)) {
-		gpriv->can_clk = devm_clk_get(dev, "canfd");
-		if (IS_ERR(gpriv->can_clk))
-			return dev_err_probe(dev, PTR_ERR(gpriv->can_clk),
-					     "cannot get canfd clock\n");
+		if (info->fcan_pclk) {
+			fcan_freq = clk_get_rate(gpriv->clkp);
+			gpriv->can_clk = NULL;
+		} else {
+			gpriv->can_clk = devm_clk_get(dev, "canfd");
+			if (IS_ERR(gpriv->can_clk))
+				return dev_err_probe(dev, PTR_ERR(gpriv->can_clk),
+						     "cannot get canfd clock\n");
+
+			fcan_freq = clk_get_rate(gpriv->can_clk);
+		}
 
 		/* CANFD clock may be further divided within the IP */
-		fcan_freq = clk_get_rate(gpriv->can_clk) / info->postdiv;
+		fcan_freq /= info->postdiv;
 	} else {
 		fcan_freq = clk_get_rate(gpriv->can_clk);
 		gpriv->extclk = gpriv->info->external_clk;
-- 
2.43.0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v4 4/7] can: rcar_canfd: Do not set registers selecting the CAN mode
  2026-09-02 14:15 [PATCH v4 0/7] can: rcar_canfd: Add support for Renesas RZ/G3S Claudiu Beznea
                   ` (2 preceding siblings ...)
  2026-09-02 14:15 ` [PATCH v4 3/7] can: rcar_canfd: Allow the CAN FD clock to be sourced from fck Claudiu Beznea
@ 2026-09-02 14:15 ` Claudiu Beznea
  2026-09-02 14:15 ` [PATCH v4 5/7] can: rcar_canfd: Add support for Renesas RZ/G3S Claudiu Beznea
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Claudiu Beznea @ 2026-09-02 14:15 UTC (permalink / raw)
  To: mkl, mailhol, robh, krzk+dt, conor+dt, geert+renesas,
	magnus.damm, biju.das.jz, fabrizio.castro.jz
  Cc: claudiu.beznea, linux-can, devicetree, linux-renesas-soc,
	linux-kernel, Claudiu Beznea

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

The bits 30 (classical CAN-only mode) and 28 (FD-only enable) of the
Channel n CAN-FD Configuration Register of the Renesas RZ/G3S SoC are not
available. The IP supports only CAN-FD mode. RZ/G3S HW manual (revision
1.30) specify the bits are read as zero and the write value should always
be zero.

Add the mode_select_na flag in struct rcar_canfd_hw_info to cover RZ/G3S
and avoid writing to unavailable bits.

The existing struct rcar_canfd_hw_info instances were updated to address
the request in the Link discussion.

Commit prepares for the addition of the Renesas RZ/G3S SoC.

Link: https://lore.kernel.org/all/1d9719e3-10ff-4cd8-b729-55fea93c37ce@wanadoo.fr
Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v4:
- collected tags 
- s/mode_select_na/fixed_canfd/g

Changes in v3:
- none

Changes in v2:
- collected the tags
- updated patch description to reflect this is a preparatory commit

 drivers/net/can/rcar/rcar_canfd.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index f9173774387f..ac5d5959d8e4 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -445,6 +445,7 @@ struct rcar_canfd_hw_info {
 	unsigned shared_can_regs:1;	/* Has shared classical can registers */
 	unsigned external_clk:1;	/* Has external clock */
 	unsigned fcan_pclk:1;		/* Has fcan sourced from pclk. */
+	unsigned fixed_canfd:1;		/* Has mode fixed to canfd. */
 };
 
 /* Channel priv data */
@@ -619,6 +620,7 @@ static const struct rcar_canfd_hw_info rcar_gen3_hw_info = {
 	.shared_can_regs = 0,
 	.external_clk = 1,
 	.fcan_pclk = 0,
+	.fixed_canfd = 0,
 };
 
 static const struct rcar_canfd_hw_info rcar_gen4_hw_info = {
@@ -637,6 +639,7 @@ static const struct rcar_canfd_hw_info rcar_gen4_hw_info = {
 	.shared_can_regs = 1,
 	.external_clk = 1,
 	.fcan_pclk = 0,
+	.fixed_canfd = 0,
 };
 
 static const struct rcar_canfd_hw_info rzg2l_hw_info = {
@@ -655,6 +658,7 @@ static const struct rcar_canfd_hw_info rzg2l_hw_info = {
 	.shared_can_regs = 0,
 	.external_clk = 1,
 	.fcan_pclk = 0,
+	.fixed_canfd = 0,
 };
 
 static const struct rcar_canfd_hw_info r9a09g047_hw_info = {
@@ -673,6 +677,7 @@ static const struct rcar_canfd_hw_info r9a09g047_hw_info = {
 	.shared_can_regs = 1,
 	.external_clk = 0,
 	.fcan_pclk = 0,
+	.fixed_canfd = 0,
 };
 
 static const struct rcar_canfd_hw_info r9a09g077_hw_info = {
@@ -691,6 +696,7 @@ static const struct rcar_canfd_hw_info r9a09g077_hw_info = {
 	.shared_can_regs = 1,
 	.external_clk = 1,
 	.fcan_pclk = 0,
+	.fixed_canfd = 0,
 };
 
 /* Helper functions */
@@ -846,6 +852,9 @@ static int rcar_canfd_reset_controller(struct rcar_canfd_global *gpriv)
 		}
 
 		/* Set the controller into appropriate mode */
+		if (gpriv->info->fixed_canfd)
+			continue;
+
 		if (gpriv->info->ch_interface_mode) {
 			/* Do not set CLOE and FDOE simultaneously */
 			if (!gpriv->fdmode) {
-- 
2.43.0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v4 5/7] can: rcar_canfd: Add support for Renesas RZ/G3S
  2026-09-02 14:15 [PATCH v4 0/7] can: rcar_canfd: Add support for Renesas RZ/G3S Claudiu Beznea
                   ` (3 preceding siblings ...)
  2026-09-02 14:15 ` [PATCH v4 4/7] can: rcar_canfd: Do not set registers selecting the CAN mode Claudiu Beznea
@ 2026-09-02 14:15 ` Claudiu Beznea
  2026-09-02 14:15 ` [PATCH v4 6/7] arm64: dts: renesas: r9a08g045: Add CAN-FD node Claudiu Beznea
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Claudiu Beznea @ 2026-09-02 14:15 UTC (permalink / raw)
  To: mkl, mailhol, robh, krzk+dt, conor+dt, geert+renesas,
	magnus.damm, biju.das.jz, fabrizio.castro.jz
  Cc: claudiu.beznea, linux-can, devicetree, linux-renesas-soc,
	linux-kernel, Claudiu Beznea

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Add support for Renesas RZ/G3S

The Renesas RZ/G3S CAN-FD controller is largely compatible with the
variant found on the Renesas RZ/G3E. The main differences are:
- the RZ/G3S provides 16 AFL pages
- the RZ/G3S supports only two channels
- the RZ/G3S supports only CAN-FD operation and does not implement the
  bits used to select between classical CAN-only and CAN FD-only modes.
- the RZ/G3S includes an internal divider that allows the peripheral
  clock to be used as the CAN FD clock source.

Add support for the Renesas RZ/G3S.

Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v4:
- collected tags

Changes in v3:
- updated max_aflpn as it refers to the number of available pages
  that could be used to access the acceptance filters; updated the
  patch description for this change

Changes in v2:
- initialized shared_global_irqs = 0 for RZ/G3S, to follow the request
  from https://lore.kernel.org/all/1d9719e3-10ff-4cd8-b729-55fea93c37ce@wanadoo.fr
- collected tags

 drivers/net/can/rcar/rcar_canfd.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index ac5d5959d8e4..74c3c3f20991 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -661,6 +661,26 @@ static const struct rcar_canfd_hw_info rzg2l_hw_info = {
 	.fixed_canfd = 0,
 };
 
+static const struct rcar_canfd_hw_info r9a08g045_hw_info = {
+	.nom_bittiming = &rcar_canfd_gen4_nom_bittiming_const,
+	.data_bittiming = &rcar_canfd_gen4_data_bittiming_const,
+	.tdc_const = &rcar_canfd_gen4_tdc_const,
+	.regs = &rcar_gen4_regs,
+	.sh = &rcar_gen4_shift_data,
+	.rnc_field_width = 16,
+	.max_aflpn = 15,
+	.max_cftml = 31,
+	.max_channels = 2,
+	.postdiv = 2,
+	.shared_global_irqs = 0,
+	.multi_channel_irqs = 1,
+	.ch_interface_mode = 1,
+	.shared_can_regs = 1,
+	.external_clk = 1,
+	.fcan_pclk = 1,
+	.fixed_canfd = 1,
+};
+
 static const struct rcar_canfd_hw_info r9a09g047_hw_info = {
 	.nom_bittiming = &rcar_canfd_gen4_nom_bittiming_const,
 	.data_bittiming = &rcar_canfd_gen4_data_bittiming_const,
@@ -2383,6 +2403,7 @@ static DEFINE_SIMPLE_DEV_PM_OPS(rcar_canfd_pm_ops, rcar_canfd_suspend,
 
 static const __maybe_unused struct of_device_id rcar_canfd_of_table[] = {
 	{ .compatible = "renesas,r8a779a0-canfd", .data = &rcar_gen4_hw_info },
+	{ .compatible = "renesas,r9a08g045-canfd", .data = &r9a08g045_hw_info },
 	{ .compatible = "renesas,r9a09g047-canfd", .data = &r9a09g047_hw_info },
 	{ .compatible = "renesas,r9a09g077-canfd", .data = &r9a09g077_hw_info },
 	{ .compatible = "renesas,rcar-gen3-canfd", .data = &rcar_gen3_hw_info },
-- 
2.43.0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v4 6/7] arm64: dts: renesas: r9a08g045: Add CAN-FD node
  2026-09-02 14:15 [PATCH v4 0/7] can: rcar_canfd: Add support for Renesas RZ/G3S Claudiu Beznea
                   ` (4 preceding siblings ...)
  2026-09-02 14:15 ` [PATCH v4 5/7] can: rcar_canfd: Add support for Renesas RZ/G3S Claudiu Beznea
@ 2026-09-02 14:15 ` Claudiu Beznea
  2026-09-02 14:15 ` [PATCH v4 7/7] arm64: dts: renesas: rzg3s-smarc: Enable CAN-FD Claudiu Beznea
  2026-09-29 14:19 ` [PATCH v4 0/7] can: rcar_canfd: Add support for Renesas RZ/G3S Marc Kleine-Budde
  7 siblings, 0 replies; 20+ messages in thread
From: Claudiu Beznea @ 2026-09-02 14:15 UTC (permalink / raw)
  To: mkl, mailhol, robh, krzk+dt, conor+dt, geert+renesas,
	magnus.damm, biju.das.jz, fabrizio.castro.jz
  Cc: claudiu.beznea, linux-can, devicetree, linux-renesas-soc,
	linux-kernel, Claudiu Beznea

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

The Renesas RZ/G3S SoC includes a CAN-FD controller with two channels.
Add the corresponding device tree node.

Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v4:
- collected tags

Changes in v3:
- none

Changes in v2:
- collected tags

 arch/arm64/boot/dts/renesas/r9a08g045.dtsi | 39 ++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
index b98ad375a3e9..eaebf5b40fee 100644
--- a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
@@ -29,6 +29,13 @@ audio_clk2: audio2-clk {
 		clock-frequency = <0>;
 	};
 
+	can_clk: can-clk {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		/* This value must be overridden by boards that provide it. */
+		clock-frequency = <0>;
+	};
+
 	cluster0_opp: opp-table-0 {
 		compatible = "operating-points-v2";
 		opp-shared;
@@ -498,6 +505,38 @@ ssi3: ssi@100a8c00 {
 			status = "disabled";
 		};
 
+		canfd: can@100c0000 {
+			compatible = "renesas,r9a08g045-canfd";
+			reg = <0 0x100c0000 0 0x20000>;
+			interrupts = <GIC_SPI 373 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 374 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 377 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 375 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 379 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 378 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 376 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 380 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "g_err", "g_recc",
+					  "ch0_err", "ch0_rec", "ch0_trx",
+					  "ch1_err", "ch1_rec", "ch1_trx";
+			clocks = <&cpg CPG_MOD R9A08G045_CANFD_PCLK>,
+				 <&cpg CPG_MOD R9A08G045_CANFD_CLK_RAM>,
+				 <&can_clk>;
+			clock-names = "fck", "ram_clk", "can_clk";
+			resets = <&cpg R9A08G045_CANFD_RSTP_N>,
+				 <&cpg R9A08G045_CANFD_RSTC_N>;
+			reset-names = "rstp_n", "rstc_n";
+			power-domains = <&cpg>;
+			status = "disabled";
+
+			channel0 {
+				status = "disabled";
+			};
+			channel1 {
+				status = "disabled";
+			};
+		};
+
 		cpg: clock-controller@11010000 {
 			compatible = "renesas,r9a08g045-cpg";
 			reg = <0 0x11010000 0 0x10000>;
-- 
2.43.0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v4 7/7] arm64: dts: renesas: rzg3s-smarc: Enable CAN-FD
  2026-09-02 14:15 [PATCH v4 0/7] can: rcar_canfd: Add support for Renesas RZ/G3S Claudiu Beznea
                   ` (5 preceding siblings ...)
  2026-09-02 14:15 ` [PATCH v4 6/7] arm64: dts: renesas: r9a08g045: Add CAN-FD node Claudiu Beznea
@ 2026-09-02 14:15 ` Claudiu Beznea
  2026-09-29 14:19 ` [PATCH v4 0/7] can: rcar_canfd: Add support for Renesas RZ/G3S Marc Kleine-Budde
  7 siblings, 0 replies; 20+ messages in thread
From: Claudiu Beznea @ 2026-09-02 14:15 UTC (permalink / raw)
  To: mkl, mailhol, robh, krzk+dt, conor+dt, geert+renesas,
	magnus.damm, biju.das.jz, fabrizio.castro.jz
  Cc: claudiu.beznea, linux-can, devicetree, linux-renesas-soc,
	linux-kernel, Claudiu Beznea

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

The two CAN FD channels available on the Renesas RZ/G3S SoC are routed
through the Renesas SMARC Carrier II board when used with the Renesas
RZ/G3S SMARC Module.

The CAN transceiver on the Carrier II board has its standby pins connected
to GPIOs that are selected through the SW_GPIO_CAN_PMOD switches. By
default, these switches route the GPIOs to the PMOD interfaces.

Enable the CAN FD controller and its two available channels.

Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v4:
- collected tags
- used ti,tcan1046 compatible
- adjusted the standby-gpio property along with CAN PHY node name and label
- adjusted the CAN channels phy phandles

Changes in v3:
- none

Changes in v2:
- collected tags

 .../boot/dts/renesas/rzg3s-smarc-switches.h   | 12 +++++
 arch/arm64/boot/dts/renesas/rzg3s-smarc.dtsi  | 45 +++++++++++++++++++
 2 files changed, 57 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/rzg3s-smarc-switches.h b/arch/arm64/boot/dts/renesas/rzg3s-smarc-switches.h
index 9cccc87da057..d3fd18626773 100644
--- a/arch/arm64/boot/dts/renesas/rzg3s-smarc-switches.h
+++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc-switches.h
@@ -41,4 +41,16 @@
  */
 #define SW_OPT_MUX4	SW_ON
 
+/*
+ * SW_GPIO_CAN_PMOD[x] switches' states:
+ * @SW_GPIO_CAN_PMOD1:
+ *	SW_OFF - GPIO8 connected to CAN0_STB (position 1-2)
+ *	SW_ON  - GPIO8 connected to PMOD1 (position 2-3)
+ * @SW_GPIO_CAN_PMOD2:
+ *	SW_OFF - GPIO9 connected to CAN1_STB (position 4-5)
+ *	SW_ON  - GPIO9 connected to PMOD1 (position 5-6)
+ */
+#define SW_GPIO_CAN_PMOD1	SW_ON
+#define SW_GPIO_CAN_PMOD2	SW_ON
+
 #endif /* __RZG3S_SMARC_SWITCHES_H__ */
diff --git a/arch/arm64/boot/dts/renesas/rzg3s-smarc.dtsi b/arch/arm64/boot/dts/renesas/rzg3s-smarc.dtsi
index e3821d8c01e3..ef24b6cbd690 100644
--- a/arch/arm64/boot/dts/renesas/rzg3s-smarc.dtsi
+++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc.dtsi
@@ -68,6 +68,23 @@ codec_dai: simple-audio-card,codec {
 		};
 	};
 
+	can_transceiver: can-phy {
+		compatible = "ti,tcan1046", "nxp,tja1048";
+		#phy-cells = <1>;
+		max-bitrate = <8000000>;
+		standby-gpios =
+#if SW_GPIO_CAN_PMOD1 == SW_OFF
+				<&pinctrl RZG2L_GPIO(13, 0) GPIO_ACTIVE_HIGH>,
+#else
+				<0>,
+#endif
+#if SW_GPIO_CAN_PMOD2 == SW_OFF
+				<&pinctrl RZG2L_GPIO(13, 1) GPIO_ACTIVE_HIGH>;
+#else
+				<0>;
+#endif
+	};
+
 	vcc_sdhi1: regulator-vcc-sdhi1 {
 		compatible = "regulator-fixed";
 		regulator-name = "SDHI1 Vcc";
@@ -92,6 +109,22 @@ &audio_clk2 {
 	clock-frequency = <12288000>;
 };
 
+&canfd {
+	pinctrl-0 = <&canfd_pins>;
+	pinctrl-names = "default";
+	status = "okay";
+
+	channel0 {
+		phys = <&can_transceiver 0>;
+		status = "okay";
+	};
+
+	channel1 {
+		phys = <&can_transceiver 1>;
+		status = "okay";
+	};
+};
+
 &ehci0 {
 	dr_mode = "otg";
 	status = "okay";
@@ -171,6 +204,18 @@ audio_clock_pins: audio-clock {
 		input-enable;
 	};
 
+	canfd_pins: canfd {
+		can0_pins: can0 {
+			pinmux = <RZG2L_PORT_PINMUX(6, 1, 3)>, /* CAN0_TX */
+				 <RZG2L_PORT_PINMUX(6, 2, 3)>; /* CAN0_RX */
+		};
+
+		can1_pins: can1 {
+			pinmux = <RZG2L_PORT_PINMUX(17, 0, 3)>, /* CAN1_TX */
+				 <RZG2L_PORT_PINMUX(17, 1, 3)>; /* CAN1_RX */
+		};
+	};
+
 	key-1-gpio-hog {
 		gpio-hog;
 		gpios = <RZG2L_GPIO(18, 0) GPIO_ACTIVE_LOW>;
-- 
2.43.0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 0/7] can: rcar_canfd: Add support for Renesas RZ/G3S
  2026-09-02 14:15 [PATCH v4 0/7] can: rcar_canfd: Add support for Renesas RZ/G3S Claudiu Beznea
                   ` (6 preceding siblings ...)
  2026-09-02 14:15 ` [PATCH v4 7/7] arm64: dts: renesas: rzg3s-smarc: Enable CAN-FD Claudiu Beznea
@ 2026-09-29 14:19 ` Marc Kleine-Budde
  7 siblings, 0 replies; 20+ messages in thread
From: Marc Kleine-Budde @ 2026-09-29 14:19 UTC (permalink / raw)
  To: Claudiu Beznea
  Cc: mailhol, robh, krzk+dt, conor+dt, geert+renesas, magnus.damm,
	biju.das.jz, fabrizio.castro.jz, claudiu.beznea, linux-can,
	devicetree, linux-renesas-soc, linux-kernel, Claudiu Beznea

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

On 02.09.2026 17:15:37, Claudiu Beznea wrote:
> Series adds CAN support for the Renesas RZ/G3S SoC. Along with it a typo
> fix patch was added on the CAN driver.
>
> Merge strategy:
> - patches 1-5/7 should go though the CAN tree

Applied 1...5 to the linux-can-next tree.

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: 228 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 1/7] dt-bindings: can: renesas,rcar-canfd: Document RZ/G3S SoC
  2026-09-02 14:15 ` [PATCH v4 1/7] dt-bindings: can: renesas,rcar-canfd: Document RZ/G3S SoC Claudiu Beznea
@ 2026-10-16 13:34   ` Marc Kleine-Budde
  2026-10-19  7:24     ` Biju Das
  0 siblings, 1 reply; 20+ messages in thread
From: Marc Kleine-Budde @ 2026-10-16 13:34 UTC (permalink / raw)
  To: Claudiu Beznea
  Cc: mailhol, robh, krzk+dt, conor+dt, geert+renesas, magnus.damm,
	biju.das.jz, fabrizio.castro.jz, claudiu.beznea, linux-can,
	devicetree, linux-renesas-soc, linux-kernel, Claudiu Beznea

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

On 02.09.2026 17:15:38, Claudiu Beznea wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> The CAN FD controller found on the Renesas RZ/G3S SoC is largely compatible
> with the variant present on the RZ/G3E SoC. The main differences are:
> - the RZ/G3S provides only two CAN FD channels
> - the RZ/G3S supports only CAN FD operation; the Channel n CAN FD
>   Configuration Register does not implement the bits used to select
>   classical CAN-only mode (bit 30) or CAN FD-only mode (bit 28);
>   consequently, bit 31 (CAN FD Frame Distinction Enable) of the same
>   register is also not implemented
> - some bits in several registers (mainly reserved or status bits) are
>   read-write on the RZ/G3S but read-only on the RZ/G3E; their behavior is
>   otherwise identical: the bits read back as 0 on both SoCs and software
>   is allowed to write only 0 to them on the RZ/G3S
> - the RZ/G3S provides 256 acceptance filters, compared to 768 on the
>   RZ/G3E
> - the RZ/G3S can use PCLK clock as the CAN FD clock source through an
>   internal clock divider, while also supporting an external CAN FD clock
>   source
>
> Since:
> - the SoC clock generator provides to the CAN IP only the peripheral and
>   the RAM clocks
> - when sourced from the peripheral clock, the CAN-FD clock is obtained
>   inside the IP itself by dividing the peripheral clock
> - the assigned-clocks and assigned-clock-rates properties are specific to
>   the CAN-FD clock
> the assigned-clocks and assigned-clock-rates properties were dropped from
> the required properties list of the Renesas RZ/G3S SoC.
>
> Add documentation for the Renesas RZ/G3S SoC.
>
> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

the netdev review bot has found a nitpick, it's about the comment in
"rcar_canfd.c". Can you fix it, if you re-spin the series.

regards,
Marc

pw-bot: cr

--- Patch [11]: [PATCH net-next 11/37] dt-bindings: can: renesas,rcar-canfd: Document RZ/G3S SoC ---
commit 69b4e37d18e5229e1c800b45a5df061af8a98eb9
Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

dt-bindings: can: renesas,rcar-canfd: Document RZ/G3S SoC

This adds the renesas,r9a08g045-canfd compatible for the RZ/G3S CAN FD
controller. It adds the compatible to the existing conditional blocks. It
also forbids renesas,no-can-fd and renesas,fd-only on RZ/G3S, and makes
assigned-clocks and assigned-clock-rates required only for the other SoCs.

Link: https://patch.msgid.link/20260902141544.565763-2-claudiu.beznea+renesas@tuxon.dev

> diff --git a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> index b9d9dd7a7967..bbaaef8f4282 100644
> --- a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> +++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml

[ ... ]

> @@ -350,6 +352,20 @@ allOf:
>        properties:
>          reset-names: false
>
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: renesas,r9a08g045-canfd
> +    then:
> +      properties:
> +        renesas,no-can-fd: false

Discovered by: opus-5-5 · Missed by: fable-5-1, gpt-6-astra, sashiko-gemini · Confirmed by: fable-5-1
This isn't a bug, but should the renesas,no-can-fd description be updated
to match this new constraint?

The property description in the same binding still says that every
controller can run in Classical CAN mode:

  renesas,no-can-fd:
    $ref: /schemas/types.yaml#/definitions/flag
    description:
      The controller can operate in either CAN-FD mode (default) or FD-Only
      mode (RZ/{G2L,G3E} and R-Car Gen4) or Classical CAN mode. Specify this
      property to put the controller in Classical CAN mode.

With this patch the property is forbidden for renesas,r9a08g045-canfd,
because RZ/G3S does not have the mode select bits.

The header comment in drivers/net/can/rcar/rcar_canfd.c says the same
thing:

 * To switch the controller to Classical CAN (CAN 2.0) only mode, add
 * "renesas,no-can-fd" optional property to the device tree node. A h/w reset is
 * also required to switch modes.

Neither text is changed later in the series. Both now contradict the schema
for RZ/G3S, and for RZ/G3L once it is added the same way.

Could the description exclude the SoCs that only support CAN FD mode?

> +        renesas,fd-only: false
> +    else:
> +      required:
> +        - assigned-clocks
> +        - assigned-clock-rates
> +
>  unevaluatedProperties: false
>
>  examples:
-- 
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: 228 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 3/7] can: rcar_canfd: Allow the CAN FD clock to be sourced from fck
  2026-09-02 14:15 ` [PATCH v4 3/7] can: rcar_canfd: Allow the CAN FD clock to be sourced from fck Claudiu Beznea
@ 2026-10-16 13:39   ` Marc Kleine-Budde
  2026-10-16 18:07     ` Claudiu Beznea
  0 siblings, 1 reply; 20+ messages in thread
From: Marc Kleine-Budde @ 2026-10-16 13:39 UTC (permalink / raw)
  To: Claudiu Beznea
  Cc: mailhol, robh, krzk+dt, conor+dt, geert+renesas, magnus.damm,
	biju.das.jz, fabrizio.castro.jz, claudiu.beznea, linux-can,
	devicetree, linux-renesas-soc, linux-kernel, Claudiu Beznea

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

On 02.09.2026 17:15:40, Claudiu Beznea wrote:
> diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
> index b7c4fe3477ac..f9173774387f 100644
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c

[...]

>
>  /* Helper functions */
> @@ -2190,13 +2196,20 @@ static int rcar_canfd_probe(struct platform_device *pdev)
>  	 */
>  	gpriv->can_clk = devm_clk_get(dev, "can_clk");
>  	if (IS_ERR(gpriv->can_clk) || (clk_get_rate(gpriv->can_clk) == 0)) {
            ^^^^^^^^^^^^^^^^^^^^^

As sashiko.dev [1] pointed out here's no check for -EPROBE_DEFER.

[1] https://lore.kernel.org/all/sashiko-outbox-173569@kernel.org/

The netdev bot found more issues.
pw-bot: cr

regards,
Marc

--- Patch [15]: [PATCH net-next 15/37] can: rcar_canfd: Allow the CAN FD clock to be sourced from fck ---
commit 4ef0042ccf34457229cdde398a008bebaab54e69
Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

can: rcar_canfd: Allow the CAN FD clock to be sourced from fck

This commit adds an fcan_pclk hardware info flag. With it set, the fCAN
clock falls back to the peripheral clock (fck) when the external can_clk
is unavailable, the rate is divided by info->postdiv and can_clk is set
to NULL. All existing rcar_canfd_hw_info instances now set fcan_pclk = 0
explicitly.

Link: https://lore.kernel.org/all/1d9719e3-10ff-4cd8-b729-55fea93c37ce@wanadoo.fr
Link: https://patch.msgid.link/20260902141544.565763-4-claudiu.beznea+renesas@tuxon.dev

> diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
> index b7c4fe3477ac..f9173774387f 100644
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c

[ ... ]

> @@ -2190,13 +2196,20 @@ static int rcar_canfd_probe(struct platform_device *pdev)
>  	 */
>  	gpriv->can_clk = devm_clk_get(dev, "can_clk");
>  	if (IS_ERR(gpriv->can_clk) || (clk_get_rate(gpriv->can_clk) == 0)) {
> -		gpriv->can_clk = devm_clk_get(dev, "canfd");
> -		if (IS_ERR(gpriv->can_clk))
> -			return dev_err_probe(dev, PTR_ERR(gpriv->can_clk),
> -					     "cannot get canfd clock\n");
> +		if (info->fcan_pclk) {
> +			fcan_freq = clk_get_rate(gpriv->clkp);
> +			gpriv->can_clk = NULL;
> +		} else {

Discovered by: opus-5-5 · Missed by: gpt-6-astra, sashiko-gemini · Confirmed by: gpt-6-astra
Should the clock comments and the probe banner be updated for this new
fck source? Several places still describe only the external and canfd
clocks.

The comment just above this block in rcar_canfd_probe() still says:

	/* fCAN clock: Pick External clock. If not available fallback to
	 * CANFD clock
	 */

With fcan_pclk set, the fallback is fck (gpriv->clkp), not the canfd
clock.

In struct rcar_canfd_global, the field comments also don't cover this
case. Here can_clk is NULL and the fCAN source is clkp:

	struct clk *can_clk;		/* fCAN clock */
	...
	bool extclk;			/* CANFD or Ext clock */

The banner at the end of rcar_canfd_probe() wasn't changed either:

	dev_info(dev, "global operational state (%s clk, %s mode)\n",
		 gpriv->extclk ? "ext" : "canfd",

On this path extclk stays false. Won't the banner then say "canfd clk"
for a controller that has no canfd clock and runs from fck / postdiv?

The later "can: rcar_canfd: Add support for Renesas RZ/G3S" patch sets
.fcan_pclk = 1 in r9a08g045_hw_info. After that, an RZ/G3S board whose
can_clk is missing or has a rate of 0 would print this label.

Also, the new bitfield comment calls this clock pclk:

	unsigned fcan_pclk:1;		/* Has fcan sourced from pclk. */

The commit message and the code call it fck. Could the comment use the
same name?

[ ... ]

--
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: 228 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 3/7] can: rcar_canfd: Allow the CAN FD clock to be sourced from fck
  2026-10-16 13:39   ` Marc Kleine-Budde
@ 2026-10-16 18:07     ` Claudiu Beznea
  2026-10-17 20:01       ` Marc Kleine-Budde
  0 siblings, 1 reply; 20+ messages in thread
From: Claudiu Beznea @ 2026-10-16 18:07 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: mailhol, robh, krzk+dt, conor+dt, geert+renesas, magnus.damm,
	biju.das.jz, fabrizio.castro.jz, claudiu.beznea, linux-can,
	devicetree, linux-renesas-soc, linux-kernel, Claudiu Beznea



On 10/16/26 16:39, Marc Kleine-Budde wrote:
> On 02.09.2026 17:15:40, Claudiu Beznea wrote:
>> diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
>> index b7c4fe3477ac..f9173774387f 100644
>> --- a/drivers/net/can/rcar/rcar_canfd.c
>> +++ b/drivers/net/can/rcar/rcar_canfd.c
> 
> [...]
> 
>>
>>   /* Helper functions */
>> @@ -2190,13 +2196,20 @@ static int rcar_canfd_probe(struct platform_device *pdev)
>>   	 */
>>   	gpriv->can_clk = devm_clk_get(dev, "can_clk");
>>   	if (IS_ERR(gpriv->can_clk) || (clk_get_rate(gpriv->can_clk) == 0)) {
>              ^^^^^^^^^^^^^^^^^^^^^
> 
> As sashiko.dev [1] pointed out here's no check for -EPROBE_DEFER.

This looks to me a pre-existing issue.

The newly added support does:

fcan_freq = clk_get_rate(gpriv->clkp);

At the time this is executed the clkp was already checked against any errors 
(above) including -EPROBE_DEFER.

Please let me know what is the procedure now. Should I re-send the series and 
address the fixes? Should I send follow up patches?

> 
> [1] https://lore.kernel.org/all/sashiko-outbox-173569@kernel.org/
> 
> The netdev bot found more issues.
> pw-bot: cr
> 
> regards,
> Marc
> 
> --- Patch [15]: [PATCH net-next 15/37] can: rcar_canfd: Allow the CAN FD clock to be sourced from fck ---
> commit 4ef0042ccf34457229cdde398a008bebaab54e69
> Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> can: rcar_canfd: Allow the CAN FD clock to be sourced from fck
> 
> This commit adds an fcan_pclk hardware info flag. With it set, the fCAN
> clock falls back to the peripheral clock (fck) when the external can_clk
> is unavailable, the rate is divided by info->postdiv and can_clk is set
> to NULL. All existing rcar_canfd_hw_info instances now set fcan_pclk = 0
> explicitly.
> 
> Link: https://lore.kernel.org/all/1d9719e3-10ff-4cd8-b729-55fea93c37ce@wanadoo.fr
> Link: https://patch.msgid.link/20260902141544.565763-4-claudiu.beznea+renesas@tuxon.dev
> 
>> diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
>> index b7c4fe3477ac..f9173774387f 100644
>> --- a/drivers/net/can/rcar/rcar_canfd.c
>> +++ b/drivers/net/can/rcar/rcar_canfd.c
> 
> [ ... ]
> 
>> @@ -2190,13 +2196,20 @@ static int rcar_canfd_probe(struct platform_device *pdev)
>>   	 */
>>   	gpriv->can_clk = devm_clk_get(dev, "can_clk");
>>   	if (IS_ERR(gpriv->can_clk) || (clk_get_rate(gpriv->can_clk) == 0)) {
>> -		gpriv->can_clk = devm_clk_get(dev, "canfd");
>> -		if (IS_ERR(gpriv->can_clk))
>> -			return dev_err_probe(dev, PTR_ERR(gpriv->can_clk),
>> -					     "cannot get canfd clock\n");
>> +		if (info->fcan_pclk) {
>> +			fcan_freq = clk_get_rate(gpriv->clkp);
>> +			gpriv->can_clk = NULL;
>> +		} else {
> 
> Discovered by: opus-5-5 · Missed by: gpt-6-astra, sashiko-gemini · Confirmed by: gpt-6-astra
> Should the clock comments and the probe banner be updated for this new
> fck source? 
> Several places still describe only the external and canfd
> clocks.
> 
> The comment just above this block in rcar_canfd_probe() still says:
> 
> 	/* fCAN clock: Pick External clock. If not available fallback to
> 	 * CANFD clock
> 	 */
> 
> With fcan_pclk set, the fallback is fck (gpriv->clkp), not the canfd
> clock.

We can update the comment as follows to comply with the clock name in the driver:

/* fCAN clock: Pick External clock. If not available fallback to
  * CANFD clock or peripheral clock in case fcan_pclk = 1
  */

or, to comply with the clock name in the bindings:

/* fCAN clock: Pick External clock. If not available fallback to
  * CANFD clock or fck in case fcan_pclk = 1
  */

Please let me know what is prefered.


> 
> In struct rcar_canfd_global, the field comments also don't cover this
> case. Here can_clk is NULL and the fCAN source is clkp:
> 
> 	struct clk *can_clk;		/* fCAN clock */
> 	...
> 	bool extclk;			/* CANFD or Ext clock */

There is already a pointer covering the clkp:

         struct clk *clkp;		/* Peripheral clock */

> 
> The banner at the end of rcar_canfd_probe() wasn't changed either:
> 
> 	dev_info(dev, "global operational state (%s clk, %s mode)\n",
> 		 gpriv->extclk ? "ext" : "canfd",
> 
> On this path extclk stays false. Won't the banner then say "canfd clk"
> for a controller that has no canfd clock and runs from fck / postdiv?

That could be updated with "clkp" to comply with the driver name or "fck" to 
comply with the bindings or with "peripheral". Please let me know what is 
preferred.

> 
> The later "can: rcar_canfd: Add support for Renesas RZ/G3S" patch sets
> .fcan_pclk = 1 in r9a08g045_hw_info. After that, an RZ/G3S board whose
> can_clk is missing or has a rate of 0 would print this label.
> 
> Also, the new bitfield comment calls this clock pclk:
> 
> 	unsigned fcan_pclk:1;		/* Has fcan sourced from pclk. */
> 
> The commit message and the code call it fck. 

The commit message mentions: ...the CAN FD clock can be sourced either from fck
(the peripheral clock) or from an external clock (can_clk).

The driver uses clkp (peripheral clock) to refer to the fck.

fck is the clock name used in the bindings. In bindings I kept the same clock 
name and provided the proper phandle in the device tree. I don't consider we 
should change the driver and the bindings for this thing. Please let me know 
otherwise.

The clock diagram is something like:

                         +----+
pclk (fck) -> 1/2 div ->|    |
                         | mux|---> fcan
ext  (can_clk)--------->|    |
                         ++++++

b/w () are the clock names in bindings.

The full HW block with the clocks involved can be found at 
https://github.com/claudiubeznea/logs/blob/master/canfd-clk.png

Please let me know what is the procedure for this. Should I re-spin the series? 
Should I send follow up patches?

Thank you,
Claudiu


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 3/7] can: rcar_canfd: Allow the CAN FD clock to be sourced from fck
  2026-10-16 18:07     ` Claudiu Beznea
@ 2026-10-17 20:01       ` Marc Kleine-Budde
  2026-10-19  9:58         ` claudiu beznea
  0 siblings, 1 reply; 20+ messages in thread
From: Marc Kleine-Budde @ 2026-10-17 20:01 UTC (permalink / raw)
  To: Claudiu Beznea
  Cc: mailhol, robh, krzk+dt, conor+dt, geert+renesas, magnus.damm,
	biju.das.jz, fabrizio.castro.jz, linux-can, devicetree,
	linux-renesas-soc, linux-kernel, Claudiu Beznea

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

On 16.10.2026 21:07:07, Claudiu Beznea wrote:
>
>
> On 10/16/26 16:39, Marc Kleine-Budde wrote:
> > On 02.09.2026 17:15:40, Claudiu Beznea wrote:
> > > diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
> > > index b7c4fe3477ac..f9173774387f 100644
> > > --- a/drivers/net/can/rcar/rcar_canfd.c
> > > +++ b/drivers/net/can/rcar/rcar_canfd.c
> >
> > [...]
> >
> > >
> > >   /* Helper functions */
> > > @@ -2190,13 +2196,20 @@ static int rcar_canfd_probe(struct platform_device *pdev)
> > >   	 */
> > >   	gpriv->can_clk = devm_clk_get(dev, "can_clk");
> > >   	if (IS_ERR(gpriv->can_clk) || (clk_get_rate(gpriv->can_clk) == 0)) {
> >              ^^^^^^^^^^^^^^^^^^^^^
> >
> > As sashiko.dev [1] pointed out here's no check for -EPROBE_DEFER.
>
> This looks to me a pre-existing issue.
>
> The newly added support does:
>
> fcan_freq = clk_get_rate(gpriv->clkp);
>
> At the time this is executed the clkp was already checked against any errors
                                   ^^^^
> (above) including -EPROBE_DEFER.

It's not about "clkp".

What's missing is, that "gpriv->can_clk" is checked against IS_ERR():

> > >   	gpriv->can_clk = devm_clk_get(dev, "can_clk");
> > >   	if (IS_ERR(gpriv->can_clk) || (clk_get_rate(gpriv->can_clk) == 0)) {

but if it's a -EPROBE_DEFER, IS_ERR() returns true. I think in this case
you want to return with dev_err_probe().

There's also a difference between "can_clk" not found (e.g. not
configured in DT), and there was a problem with "can_clk". I don't know
if this is important for this driver, but if you want to distinguish
between those, you can use devm_clk_get_optional(). I will return NULL
if the clock is not found, -EPROBE_DEFER if not available yet, the clock
or, in case of an error -errno.

Thinking about it: if "can_clk" is not available, I would consider it
intentional, so the driver should use the other clock. If the clock has
an error, the driver should not fallback to a probably untested clock
configuration.

> Please let me know what is the procedure now. Should I re-send the series
> and address the fixes? Should I send follow up patches?

netdev's sashiko's classified this as low. So the patch should go in as
is, if it comes to the same conclusion I in my next PR. Please send a
follow up series.

> > --- Patch [15]: [PATCH net-next 15/37] can: rcar_canfd: Allow the CAN FD clock to be sourced from fck ---
> > commit 4ef0042ccf34457229cdde398a008bebaab54e69
> > Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >
> > can: rcar_canfd: Allow the CAN FD clock to be sourced from fck
> >
> > This commit adds an fcan_pclk hardware info flag. With it set, the fCAN
> > clock falls back to the peripheral clock (fck) when the external can_clk
> > is unavailable, the rate is divided by info->postdiv and can_clk is set
> > to NULL. All existing rcar_canfd_hw_info instances now set fcan_pclk = 0
> > explicitly.
> >
> > Link: https://lore.kernel.org/all/1d9719e3-10ff-4cd8-b729-55fea93c37ce@wanadoo.fr
> > Link: https://patch.msgid.link/20260902141544.565763-4-claudiu.beznea+renesas@tuxon.dev
> >
> > > diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
> > > index b7c4fe3477ac..f9173774387f 100644
> > > --- a/drivers/net/can/rcar/rcar_canfd.c
> > > +++ b/drivers/net/can/rcar/rcar_canfd.c
> >
> > [ ... ]
> >
> > > @@ -2190,13 +2196,20 @@ static int rcar_canfd_probe(struct platform_device *pdev)
> > >   	 */
> > >   	gpriv->can_clk = devm_clk_get(dev, "can_clk");
> > >   	if (IS_ERR(gpriv->can_clk) || (clk_get_rate(gpriv->can_clk) == 0)) {
> > > -		gpriv->can_clk = devm_clk_get(dev, "canfd");
> > > -		if (IS_ERR(gpriv->can_clk))
> > > -			return dev_err_probe(dev, PTR_ERR(gpriv->can_clk),
> > > -					     "cannot get canfd clock\n");
> > > +		if (info->fcan_pclk) {
> > > +			fcan_freq = clk_get_rate(gpriv->clkp);
> > > +			gpriv->can_clk = NULL;
> > > +		} else {
> >
> > Discovered by: opus-5-5 · Missed by: gpt-6-astra, sashiko-gemini · Confirmed by: gpt-6-astra
> > Should the clock comments and the probe banner be updated for this new
> > fck source? Several places still describe only the external and canfd
> > clocks.
> >
> > The comment just above this block in rcar_canfd_probe() still says:
> >
> > 	/* fCAN clock: Pick External clock. If not available fallback to
> > 	 * CANFD clock
> > 	 */
> >
> > With fcan_pclk set, the fallback is fck (gpriv->clkp), not the canfd
> > clock.
>
> We can update the comment as follows to comply with the clock name in the driver:
>
> /* fCAN clock: Pick External clock. If not available fallback to
>  * CANFD clock or peripheral clock in case fcan_pclk = 1
>  */
>
> or, to comply with the clock name in the bindings:
>
> /* fCAN clock: Pick External clock. If not available fallback to
>  * CANFD clock or fck in case fcan_pclk = 1
>  */
>
> Please let me know what is prefered.

Use the clock names from the binding. (Even in the linked clock tree the
clock have more than one name :))

> > In struct rcar_canfd_global, the field comments also don't cover this
> > case. Here can_clk is NULL and the fCAN source is clkp:
> >
> > 	struct clk *can_clk;		/* fCAN clock */
> > 	...
> > 	bool extclk;			/* CANFD or Ext clock */
>
> There is already a pointer covering the clkp:
>
>         struct clk *clkp;		/* Peripheral clock */
>
> >
> > The banner at the end of rcar_canfd_probe() wasn't changed either:
> >
> > 	dev_info(dev, "global operational state (%s clk, %s mode)\n",
> > 		 gpriv->extclk ? "ext" : "canfd",
> >
> > On this path extclk stays false. Won't the banner then say "canfd clk"
> > for a controller that has no canfd clock and runs from fck / postdiv?
>
> That could be updated with "clkp" to comply with the driver name or "fck" to
> comply with the bindings or with "peripheral". Please let me know what is
> preferred.

IMHO: Use the clock names from the binding. The binding name is ABI, the
driver's internal name can change.

> > The later "can: rcar_canfd: Add support for Renesas RZ/G3S" patch sets
> > .fcan_pclk = 1 in r9a08g045_hw_info. After that, an RZ/G3S board whose
> > can_clk is missing or has a rate of 0 would print this label.
> >
> > Also, the new bitfield comment calls this clock pclk:
> >
> > 	unsigned fcan_pclk:1;		/* Has fcan sourced from pclk. */
> >
> > The commit message and the code call it fck.
>
> The commit message mentions: ...the CAN FD clock can be sourced either from fck
> (the peripheral clock) or from an external clock (can_clk).
>
> The driver uses clkp (peripheral clock) to refer to the fck.
>
> fck is the clock name used in the bindings. In bindings I kept the same
> clock name and provided the proper phandle in the device tree. I don't
> consider we should change the driver and the bindings for this thing. Please
> let me know otherwise.

Fine :) The binding cannot be changed, it ABI.

> The clock diagram is something like:
>
>                         +----+
> pclk (fck) -> 1/2 div ->|    |
>                         | mux|---> fcan
> ext  (can_clk)--------->|    |
>                         ++++++
>
> b/w () are the clock names in bindings.
>
> The full HW block with the clocks involved can be found at
> https://github.com/claudiubeznea/logs/blob/master/canfd-clk.png
>
> Please let me know what is the procedure for this. Should I re-spin the
> series? Should I send follow up patches?

Follow up patches is fine.

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: 228 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH v4 1/7] dt-bindings: can: renesas,rcar-canfd: Document RZ/G3S SoC
  2026-10-16 13:34   ` Marc Kleine-Budde
@ 2026-10-19  7:24     ` Biju Das
  2026-10-19  8:01       ` Claudiu Beznea
  0 siblings, 1 reply; 20+ messages in thread
From: Biju Das @ 2026-10-19  7:24 UTC (permalink / raw)
  To: Marc Kleine-Budde, Claudiu Beznea
  Cc: mailhol, robh, krzk+dt, conor+dt, geert+renesas, magnus.damm,
	Fabrizio Castro, Claudiu.Beznea, linux-can, devicetree,
	linux-renesas-soc, linux-kernel, Claudiu Beznea

Hi Claudiu,

> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 16 October 2026 14:34
> Subject: Re: [PATCH v4 1/7] dt-bindings: can: renesas,rcar-canfd: Document RZ/G3S SoC
> 
> On 02.09.2026 17:15:38, Claudiu Beznea wrote:
> > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >
> > The CAN FD controller found on the Renesas RZ/G3S SoC is largely
> > compatible with the variant present on the RZ/G3E SoC. The main differences are:
> > - the RZ/G3S provides only two CAN FD channels
> > - the RZ/G3S supports only CAN FD operation; the Channel n CAN FD
> >   Configuration Register does not implement the bits used to select
> >   classical CAN-only mode (bit 30) or CAN FD-only mode (bit 28);
> >   consequently, bit 31 (CAN FD Frame Distinction Enable) of the same
> >   register is also not implemented
> > - some bits in several registers (mainly reserved or status bits) are
> >   read-write on the RZ/G3S but read-only on the RZ/G3E; their behavior is
> >   otherwise identical: the bits read back as 0 on both SoCs and software
> >   is allowed to write only 0 to them on the RZ/G3S
> > - the RZ/G3S provides 256 acceptance filters, compared to 768 on the
> >   RZ/G3E
> > - the RZ/G3S can use PCLK clock as the CAN FD clock source through an
> >   internal clock divider, while also supporting an external CAN FD clock
> >   source
> >
> > Since:
> > - the SoC clock generator provides to the CAN IP only the peripheral and
> >   the RAM clocks
> > - when sourced from the peripheral clock, the CAN-FD clock is obtained
> >   inside the IP itself by dividing the peripheral clock
> > - the assigned-clocks and assigned-clock-rates properties are specific to
> >   the CAN-FD clock
> > the assigned-clocks and assigned-clock-rates properties were dropped
> > from the required properties list of the Renesas RZ/G3S SoC.
> >
> > Add documentation for the Renesas RZ/G3S SoC.
> >
> > Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> the netdev review bot has found a nitpick, it's about the comment in "rcar_canfd.c". Can you fix it, if
> you re-spin the series.
> 
> regards,
> Marc
> 
> pw-bot: cr
> 
> --- Patch [11]: [PATCH net-next 11/37] dt-bindings: can: renesas,rcar-canfd: Document RZ/G3S SoC ---
> commit 69b4e37d18e5229e1c800b45a5df061af8a98eb9
> Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> dt-bindings: can: renesas,rcar-canfd: Document RZ/G3S SoC
> 
> This adds the renesas,r9a08g045-canfd compatible for the RZ/G3S CAN FD controller. It adds the compatible
> to the existing conditional blocks. It also forbids renesas,no-can-fd and renesas,fd-only on RZ/G3S, and
> makes assigned-clocks and assigned-clock-rates required only for the other SoCs.
> 
> Link: https://patch.msgid.link/20260902141544.565763-2-claudiu.beznea+renesas@tuxon.dev
> 
> > diff --git
> > a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> > b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> > index b9d9dd7a7967..bbaaef8f4282 100644
> > ---
> > a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> > +++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yam
> > +++ l
> 
> [ ... ]
> 
> > @@ -350,6 +352,20 @@ allOf:
> >        properties:
> >          reset-names: false
> >
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: renesas,r9a08g045-canfd
> > +    then:
> > +      properties:
> > +        renesas,no-can-fd: false
> 
> Discovered by: opus-5-5 · Missed by: fable-5-1, gpt-6-astra, sashiko-gemini · Confirmed by: fable-5-1
> This isn't a bug, but should the renesas,no-can-fd description be updated to match this new constraint?
> 
> The property description in the same binding still says that every controller can run in Classical CAN
> mode:
> 
>   renesas,no-can-fd:
>     $ref: /schemas/types.yaml#/definitions/flag
>     description:
>       The controller can operate in either CAN-FD mode (default) or FD-Only
>       mode (RZ/{G2L,G3E} and R-Car Gen4) or Classical CAN mode. Specify this
>       property to put the controller in Classical CAN mode.
> 
> With this patch the property is forbidden for renesas,r9a08g045-canfd, because RZ/G3S does not have the
> mode select bits.
> 
> The header comment in drivers/net/can/rcar/rcar_canfd.c says the same
> thing:
> 
>  * To switch the controller to Classical CAN (CAN 2.0) only mode, add
>  * "renesas,no-can-fd" optional property to the device tree node. A h/w reset is
>  * also required to switch modes.
> 
> Neither text is changed later in the series. Both now contradict the schema for RZ/G3S, and for RZ/G3L
> once it is added the same way.
> 
> Could the description exclude the SoCs that only support CAN FD mode?


Do you plan to update description of renesas,no-canfd in bindings/driver exempting Classical CAN mode
for RZ/G3S? Or do you want me to update?

Please let me know.

Cheers,
Biju

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 1/7] dt-bindings: can: renesas,rcar-canfd: Document RZ/G3S SoC
  2026-10-19  7:24     ` Biju Das
@ 2026-10-19  8:01       ` Claudiu Beznea
  2026-10-19  8:26         ` Biju Das
  0 siblings, 1 reply; 20+ messages in thread
From: Claudiu Beznea @ 2026-10-19  8:01 UTC (permalink / raw)
  To: Biju Das, Marc Kleine-Budde
  Cc: mailhol, robh, krzk+dt, conor+dt, geert+renesas, magnus.damm,
	Fabrizio Castro, Claudiu.Beznea, linux-can, devicetree,
	linux-renesas-soc, linux-kernel, Claudiu Beznea

Hi, Biju,

On 10/19/26 10:24, Biju Das wrote:
> Hi Claudiu,
> 
>> -----Original Message-----
>> From: Marc Kleine-Budde <mkl@pengutronix.de>
>> Sent: 16 October 2026 14:34
>> Subject: Re: [PATCH v4 1/7] dt-bindings: can: renesas,rcar-canfd: Document RZ/G3S SoC
>>
>> On 02.09.2026 17:15:38, Claudiu Beznea wrote:
>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>
>>> The CAN FD controller found on the Renesas RZ/G3S SoC is largely
>>> compatible with the variant present on the RZ/G3E SoC. The main differences are:
>>> - the RZ/G3S provides only two CAN FD channels
>>> - the RZ/G3S supports only CAN FD operation; the Channel n CAN FD
>>>    Configuration Register does not implement the bits used to select
>>>    classical CAN-only mode (bit 30) or CAN FD-only mode (bit 28);
>>>    consequently, bit 31 (CAN FD Frame Distinction Enable) of the same
>>>    register is also not implemented
>>> - some bits in several registers (mainly reserved or status bits) are
>>>    read-write on the RZ/G3S but read-only on the RZ/G3E; their behavior is
>>>    otherwise identical: the bits read back as 0 on both SoCs and software
>>>    is allowed to write only 0 to them on the RZ/G3S
>>> - the RZ/G3S provides 256 acceptance filters, compared to 768 on the
>>>    RZ/G3E
>>> - the RZ/G3S can use PCLK clock as the CAN FD clock source through an
>>>    internal clock divider, while also supporting an external CAN FD clock
>>>    source
>>>
>>> Since:
>>> - the SoC clock generator provides to the CAN IP only the peripheral and
>>>    the RAM clocks
>>> - when sourced from the peripheral clock, the CAN-FD clock is obtained
>>>    inside the IP itself by dividing the peripheral clock
>>> - the assigned-clocks and assigned-clock-rates properties are specific to
>>>    the CAN-FD clock
>>> the assigned-clocks and assigned-clock-rates properties were dropped
>>> from the required properties list of the Renesas RZ/G3S SoC.
>>>
>>> Add documentation for the Renesas RZ/G3S SoC.
>>>
>>> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
>>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> the netdev review bot has found a nitpick, it's about the comment in "rcar_canfd.c". Can you fix it, if
>> you re-spin the series.
>>
>> regards,
>> Marc
>>
>> pw-bot: cr
>>
>> --- Patch [11]: [PATCH net-next 11/37] dt-bindings: can: renesas,rcar-canfd: Document RZ/G3S SoC ---
>> commit 69b4e37d18e5229e1c800b45a5df061af8a98eb9
>> Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> dt-bindings: can: renesas,rcar-canfd: Document RZ/G3S SoC
>>
>> This adds the renesas,r9a08g045-canfd compatible for the RZ/G3S CAN FD controller. It adds the compatible
>> to the existing conditional blocks. It also forbids renesas,no-can-fd and renesas,fd-only on RZ/G3S, and
>> makes assigned-clocks and assigned-clock-rates required only for the other SoCs.
>>
>> Link: https://patch.msgid.link/20260902141544.565763-2-claudiu.beznea+renesas@tuxon.dev
>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
>>> b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
>>> index b9d9dd7a7967..bbaaef8f4282 100644
>>> ---
>>> a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
>>> +++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yam
>>> +++ l
>>
>> [ ... ]
>>
>>> @@ -350,6 +352,20 @@ allOf:
>>>         properties:
>>>           reset-names: false
>>>
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: renesas,r9a08g045-canfd
>>> +    then:
>>> +      properties:
>>> +        renesas,no-can-fd: false
>>
>> Discovered by: opus-5-5 · Missed by: fable-5-1, gpt-6-astra, sashiko-gemini · Confirmed by: fable-5-1
>> This isn't a bug, but should the renesas,no-can-fd description be updated to match this new constraint?
>>
>> The property description in the same binding still says that every controller can run in Classical CAN
>> mode:
>>
>>    renesas,no-can-fd:
>>      $ref: /schemas/types.yaml#/definitions/flag
>>      description:
>>        The controller can operate in either CAN-FD mode (default) or FD-Only
>>        mode (RZ/{G2L,G3E} and R-Car Gen4) or Classical CAN mode. Specify this
>>        property to put the controller in Classical CAN mode.
>>
>> With this patch the property is forbidden for renesas,r9a08g045-canfd, because RZ/G3S does not have the
>> mode select bits.
>>
>> The header comment in drivers/net/can/rcar/rcar_canfd.c says the same
>> thing:
>>
>>   * To switch the controller to Classical CAN (CAN 2.0) only mode, add
>>   * "renesas,no-can-fd" optional property to the device tree node. A h/w reset is
>>   * also required to switch modes.
>>
>> Neither text is changed later in the series. Both now contradict the schema for RZ/G3S, and for RZ/G3L
>> once it is added the same way.
>>
>> Could the description exclude the SoCs that only support CAN FD mode?
> 
> 
> Do you plan to update description of renesas,no-canfd in bindings/driver exempting Classical CAN mode
> for RZ/G3S? Or do you want me to update?

I intend to send follow up patches today. If you already have something prepared 
for this, please send it.

Thank you,
Claudiu


^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH v4 1/7] dt-bindings: can: renesas,rcar-canfd: Document RZ/G3S SoC
  2026-10-19  8:01       ` Claudiu Beznea
@ 2026-10-19  8:26         ` Biju Das
  0 siblings, 0 replies; 20+ messages in thread
From: Biju Das @ 2026-10-19  8:26 UTC (permalink / raw)
  To: Claudiu.Beznea, Marc Kleine-Budde
  Cc: mailhol, robh, krzk+dt, conor+dt, geert+renesas, magnus.damm,
	Fabrizio Castro, Claudiu.Beznea, linux-can, devicetree,
	linux-renesas-soc, linux-kernel, Claudiu Beznea

Hi Claudiu,

> -----Original Message-----
> From: Claudiu Beznea <claudiu.beznea@tuxon.dev>
> Sent: 19 October 2026 09:02
> Subject: Re: [PATCH v4 1/7] dt-bindings: can: renesas,rcar-canfd: Document RZ/G3S SoC
> 
> Hi, Biju,
> 
> On 10/19/26 10:24, Biju Das wrote:
> > Hi Claudiu,
> >
> >> -----Original Message-----
> >> From: Marc Kleine-Budde <mkl@pengutronix.de>
> >> Sent: 16 October 2026 14:34
> >> Subject: Re: [PATCH v4 1/7] dt-bindings: can: renesas,rcar-canfd:
> >> Document RZ/G3S SoC
> >>
> >> On 02.09.2026 17:15:38, Claudiu Beznea wrote:
> >>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >>>
> >>> The CAN FD controller found on the Renesas RZ/G3S SoC is largely
> >>> compatible with the variant present on the RZ/G3E SoC. The main differences are:
> >>> - the RZ/G3S provides only two CAN FD channels
> >>> - the RZ/G3S supports only CAN FD operation; the Channel n CAN FD
> >>>    Configuration Register does not implement the bits used to select
> >>>    classical CAN-only mode (bit 30) or CAN FD-only mode (bit 28);
> >>>    consequently, bit 31 (CAN FD Frame Distinction Enable) of the same
> >>>    register is also not implemented
> >>> - some bits in several registers (mainly reserved or status bits) are
> >>>    read-write on the RZ/G3S but read-only on the RZ/G3E; their behavior is
> >>>    otherwise identical: the bits read back as 0 on both SoCs and software
> >>>    is allowed to write only 0 to them on the RZ/G3S
> >>> - the RZ/G3S provides 256 acceptance filters, compared to 768 on the
> >>>    RZ/G3E
> >>> - the RZ/G3S can use PCLK clock as the CAN FD clock source through an
> >>>    internal clock divider, while also supporting an external CAN FD clock
> >>>    source
> >>>
> >>> Since:
> >>> - the SoC clock generator provides to the CAN IP only the peripheral and
> >>>    the RAM clocks
> >>> - when sourced from the peripheral clock, the CAN-FD clock is obtained
> >>>    inside the IP itself by dividing the peripheral clock
> >>> - the assigned-clocks and assigned-clock-rates properties are specific to
> >>>    the CAN-FD clock
> >>> the assigned-clocks and assigned-clock-rates properties were dropped
> >>> from the required properties list of the Renesas RZ/G3S SoC.
> >>>
> >>> Add documentation for the Renesas RZ/G3S SoC.
> >>>
> >>> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
> >>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >>
> >> the netdev review bot has found a nitpick, it's about the comment in
> >> "rcar_canfd.c". Can you fix it, if you re-spin the series.
> >>
> >> regards,
> >> Marc
> >>
> >> pw-bot: cr
> >>
> >> --- Patch [11]: [PATCH net-next 11/37] dt-bindings: can:
> >> renesas,rcar-canfd: Document RZ/G3S SoC --- commit
> >> 69b4e37d18e5229e1c800b45a5df061af8a98eb9
> >> Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >>
> >> dt-bindings: can: renesas,rcar-canfd: Document RZ/G3S SoC
> >>
> >> This adds the renesas,r9a08g045-canfd compatible for the RZ/G3S CAN
> >> FD controller. It adds the compatible to the existing conditional
> >> blocks. It also forbids renesas,no-can-fd and renesas,fd-only on RZ/G3S, and makes assigned-clocks and
> assigned-clock-rates required only for the other SoCs.
> >>
> >> Link:
> >> https://patch.msgid.link/20260902141544.565763-2-claudiu.beznea+renes
> >> as@tuxon.dev
> >>
> >>> diff --git
> >>> a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> >>> b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> >>> index b9d9dd7a7967..bbaaef8f4282 100644
> >>> ---
> >>> a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> >>> +++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.y
> >>> +++ am
> >>> +++ l
> >>
> >> [ ... ]
> >>
> >>> @@ -350,6 +352,20 @@ allOf:
> >>>         properties:
> >>>           reset-names: false
> >>>
> >>> +  - if:
> >>> +      properties:
> >>> +        compatible:
> >>> +          contains:
> >>> +            const: renesas,r9a08g045-canfd
> >>> +    then:
> >>> +      properties:
> >>> +        renesas,no-can-fd: false
> >>
> >> Discovered by: opus-5-5 · Missed by: fable-5-1, gpt-6-astra,
> >> sashiko-gemini · Confirmed by: fable-5-1 This isn't a bug, but should the renesas,no-can-fd
> description be updated to match this new constraint?
> >>
> >> The property description in the same binding still says that every
> >> controller can run in Classical CAN
> >> mode:
> >>
> >>    renesas,no-can-fd:
> >>      $ref: /schemas/types.yaml#/definitions/flag
> >>      description:
> >>        The controller can operate in either CAN-FD mode (default) or FD-Only
> >>        mode (RZ/{G2L,G3E} and R-Car Gen4) or Classical CAN mode. Specify this
> >>        property to put the controller in Classical CAN mode.
> >>
> >> With this patch the property is forbidden for
> >> renesas,r9a08g045-canfd, because RZ/G3S does not have the mode select bits.
> >>
> >> The header comment in drivers/net/can/rcar/rcar_canfd.c says the same
> >> thing:
> >>
> >>   * To switch the controller to Classical CAN (CAN 2.0) only mode, add
> >>   * "renesas,no-can-fd" optional property to the device tree node. A h/w reset is
> >>   * also required to switch modes.
> >>
> >> Neither text is changed later in the series. Both now contradict the
> >> schema for RZ/G3S, and for RZ/G3L once it is added the same way.
> >>
> >> Could the description exclude the SoCs that only support CAN FD mode?
> >
> >
> > Do you plan to update description of renesas,no-canfd in
> > bindings/driver exempting Classical CAN mode for RZ/G3S? Or do you want me to update?
> 
> I intend to send follow up patches today. If you already have something prepared for this, please send
> it.

OK I will rebase on top of the commit [1] and will send next version.

This has generic binding change from Krzysztof + RZ/G3S binding support.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git/commit/?id=2b4a5055d517b11bf4a5671cef32e4d77bc45626


Cheers,
Biju

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 3/7] can: rcar_canfd: Allow the CAN FD clock to be sourced from fck
  2026-10-17 20:01       ` Marc Kleine-Budde
@ 2026-10-19  9:58         ` claudiu beznea
  2026-10-19 10:21           ` Marc Kleine-Budde
  2026-10-19 10:36           ` claudiu beznea
  0 siblings, 2 replies; 20+ messages in thread
From: claudiu beznea @ 2026-10-19  9:58 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: mailhol, robh, krzk+dt, conor+dt, geert+renesas, magnus.damm,
	biju.das.jz, fabrizio.castro.jz, linux-can, devicetree,
	linux-renesas-soc, linux-kernel, Claudiu Beznea

Hi, Mark,

On 10/17/26 23:01, Marc Kleine-Budde wrote:
>>>>    /* Helper functions */
>>>> @@ -2190,13 +2196,20 @@ static int rcar_canfd_probe(struct platform_device *pdev)
>>>>    	 */
>>>>    	gpriv->can_clk = devm_clk_get(dev, "can_clk");
>>>>    	if (IS_ERR(gpriv->can_clk) || (clk_get_rate(gpriv->can_clk) == 0)) {
>>>               ^^^^^^^^^^^^^^^^^^^^^
>>>
>>> As sashiko.dev [1] pointed out here's no check for -EPROBE_DEFER.
>> This looks to me a pre-existing issue.
>>
>> The newly added support does:
>>
>> fcan_freq = clk_get_rate(gpriv->clkp);
>>
>> At the time this is executed the clkp was already checked against any errors
>                                     ^^^^
>> (above) including -EPROBE_DEFER.
> It's not about "clkp".
> 
> What's missing is, that "gpriv->can_clk" is checked against IS_ERR():
> 
>>>>    	gpriv->can_clk = devm_clk_get(dev, "can_clk");
>>>>    	if (IS_ERR(gpriv->can_clk) || (clk_get_rate(gpriv->can_clk) == 0)) {
> but if it's a -EPROBE_DEFER, IS_ERR() returns true. I think in this case
> you want to return with dev_err_probe().
> 
> There's also a difference between "can_clk" not found (e.g. not
> configured in DT), and there was a problem with "can_clk". I don't know
> if this is important for this driver, but if you want to distinguish
> between those, you can use devm_clk_get_optional(). I will return NULL
> if the clock is not found, -EPROBE_DEFER if not available yet, the clock
> or, in case of an error -errno.
> 
> Thinking about it: if "can_clk" is not available, I would consider it
> intentional, so the driver should use the other clock. If the clock has
> an error, the driver should not fallback to a probably untested clock
> configuration.

         /* fCAN clock: Pick External clock. If not available fallback to
          * CANFD clock
          */
         gpriv->can_clk = devm_clk_get(dev, "can_clk");
         if (IS_ERR(gpriv->can_clk) || (clk_get_rate(gpriv->can_clk) == 0)) {

Based on the comment on this code "If not available fallback to CANFD clock"
I think the intention with IS_ERR() is to allow probing go forward, falling back 
to CANFD clock, in case of any error of devm_clk_get().

The device trees that I've checked sets the can_clk to a fixed clock with rate = 
0 or have the clock provided directly by the SoC clock provider. I think the 
intention with IS_ERR() was not only for -ENOENT but for any other kind of 
errors. We can return in case of -EPROBE_DEFER, to ensure the expected clock is 
used, when it's available and sane.

I'll switch to something like:

  gpriv->can_clk = devm_clk_get(&pdev->dev, "can_clk");
+if (ERR_PTR(gpriv->can_clk) == -EPROBE_DEFER)
+	return dev_err_ptr(dev, ...);

The devm_clk_get_optional() would involve changing more lines:

-       gpriv->can_clk = devm_clk_get(dev, "can_clk");
-       if (IS_ERR(gpriv->can_clk) || (clk_get_rate(gpriv->can_clk) == 0)) {
+       gpriv->can_clk = devm_clk_get_optional(dev, "can_clk");
+       if (ERR_PTR(gpriv->can_clk) == -EPROBE_DEFER)
+               return dev_err_probe(dev, -EPROBE_DEFER, "cannot get can_clk");
+
+       if (IS_ERR_OR_NULL(gpriv->can_clk) || (clk_get_rate(gpriv->can_clk) == 0)) {
                 gpriv->can_clk = devm_clk_get(dev, "canfd");
                 if (IS_ERR(gpriv->can_clk))
                         return dev_err_probe(dev, PTR_ERR(gpriv->can_clk),

Please let me know otherwise.

Thank you,
Claudiu

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 3/7] can: rcar_canfd: Allow the CAN FD clock to be sourced from fck
  2026-10-19  9:58         ` claudiu beznea
@ 2026-10-19 10:21           ` Marc Kleine-Budde
  2026-10-19 10:36           ` claudiu beznea
  1 sibling, 0 replies; 20+ messages in thread
From: Marc Kleine-Budde @ 2026-10-19 10:21 UTC (permalink / raw)
  To: claudiu beznea
  Cc: mailhol, robh, krzk+dt, conor+dt, geert+renesas, magnus.damm,
	biju.das.jz, fabrizio.castro.jz, linux-can, devicetree,
	linux-renesas-soc, linux-kernel, Claudiu Beznea

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

On 19.10.2026 12:58:24, claudiu beznea wrote:
> Hi, Mark,
>
> On 10/17/26 23:01, Marc Kleine-Budde wrote:
> > > > >    /* Helper functions */
> > > > > @@ -2190,13 +2196,20 @@ static int rcar_canfd_probe(struct platform_device *pdev)
> > > > >    	 */
> > > > >    	gpriv->can_clk = devm_clk_get(dev, "can_clk");
> > > > >    	if (IS_ERR(gpriv->can_clk) || (clk_get_rate(gpriv->can_clk) == 0)) {
> > > >               ^^^^^^^^^^^^^^^^^^^^^
> > > >
> > > > As sashiko.dev [1] pointed out here's no check for -EPROBE_DEFER.
> > > This looks to me a pre-existing issue.
> > >
> > > The newly added support does:
> > >
> > > fcan_freq = clk_get_rate(gpriv->clkp);
> > >
> > > At the time this is executed the clkp was already checked against any errors
> >                                     ^^^^
> > > (above) including -EPROBE_DEFER.
> > It's not about "clkp".
> >
> > What's missing is, that "gpriv->can_clk" is checked against IS_ERR():
> >
> > > > >    	gpriv->can_clk = devm_clk_get(dev, "can_clk");
> > > > >    	if (IS_ERR(gpriv->can_clk) || (clk_get_rate(gpriv->can_clk) == 0)) {
> > but if it's a -EPROBE_DEFER, IS_ERR() returns true. I think in this case
> > you want to return with dev_err_probe().
> >
> > There's also a difference between "can_clk" not found (e.g. not
> > configured in DT), and there was a problem with "can_clk". I don't know
> > if this is important for this driver, but if you want to distinguish
> > between those, you can use devm_clk_get_optional(). I will return NULL
> > if the clock is not found, -EPROBE_DEFER if not available yet, the clock
> > or, in case of an error -errno.
> >
> > Thinking about it: if "can_clk" is not available, I would consider it
> > intentional, so the driver should use the other clock. If the clock has
> > an error, the driver should not fallback to a probably untested clock
> > configuration.
>
>         /* fCAN clock: Pick External clock. If not available fallback to
>          * CANFD clock
>          */
>         gpriv->can_clk = devm_clk_get(dev, "can_clk");
>         if (IS_ERR(gpriv->can_clk) || (clk_get_rate(gpriv->can_clk) == 0)) {
>
> Based on the comment on this code "If not available fallback to CANFD clock"
> I think the intention with IS_ERR() is to allow probing go forward, falling
> back to CANFD clock, in case of any error of devm_clk_get().

From my point of view it's not consistent to continue probing if the
"can_clk" has been configured in the DT, and it fails for some reason.

> The device trees that I've checked sets the can_clk to a fixed clock with
> rate = 0 or have the clock provided directly by the SoC clock
> provider.

Using a clock rate of 0 to signal something to the driver sounds like a
hack to me :)

> I
> think the intention with IS_ERR() was not only for -ENOENT but for any other
> kind of errors. We can return in case of -EPROBE_DEFER, to ensure the
> expected clock is used, when it's available and sane.
>
> I'll switch to something like:
>
>  gpriv->can_clk = devm_clk_get(&pdev->dev, "can_clk");
> +if (ERR_PTR(gpriv->can_clk) == -EPROBE_DEFER)
> +	return dev_err_ptr(dev, ...);
>
> The devm_clk_get_optional() would involve changing more lines:



> -       gpriv->can_clk = devm_clk_get(dev, "can_clk");
> -       if (IS_ERR(gpriv->can_clk) || (clk_get_rate(gpriv->can_clk) == 0)) {
> +       gpriv->can_clk = devm_clk_get_optional(dev, "can_clk");
> +       if (ERR_PTR(gpriv->can_clk) == -EPROBE_DEFER)
> +               return dev_err_probe(dev, -EPROBE_DEFER, "cannot get can_clk");
> +
> +       if (IS_ERR_OR_NULL(gpriv->can_clk) || (clk_get_rate(gpriv->can_clk) == 0)) {
>                 gpriv->can_clk = devm_clk_get(dev, "canfd");
>                 if (IS_ERR(gpriv->can_clk))
>                         return dev_err_probe(dev, PTR_ERR(gpriv->can_clk),
>
> Please let me know otherwise.

I was thinking of not ignoring errors:

       gpriv->can_clk = devm_clk_get_optional(dev, "can_clk");
       if (ERR_PTR(gpriv->can_clk))
               return dev_err_probe(dev, ERR_PTR(gpriv->can_clk), "cannot get can_clk");

       if (!gpriv->can_clk || (clk_get_rate(gpriv->can_clk) == 0)) {

If you plan to keep the current logic and always fall back to "canfd",
please:
- add -EPROBE_DEFER handling
- add a comment that all other errors result in fallback to "canfd" clock
- add a comment that a "can_clk" rate of "0" means don't use the clock

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: 228 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 3/7] can: rcar_canfd: Allow the CAN FD clock to be sourced from fck
  2026-10-19  9:58         ` claudiu beznea
  2026-10-19 10:21           ` Marc Kleine-Budde
@ 2026-10-19 10:36           ` claudiu beznea
  2026-10-19 10:42             ` Marc Kleine-Budde
  1 sibling, 1 reply; 20+ messages in thread
From: claudiu beznea @ 2026-10-19 10:36 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: mailhol, robh, krzk+dt, conor+dt, geert+renesas, magnus.damm,
	biju.das.jz, fabrizio.castro.jz, linux-can, devicetree,
	linux-renesas-soc, linux-kernel, Claudiu Beznea



On 10/19/26 12:58, claudiu beznea wrote:
> Hi, Mark,
> 
> On 10/17/26 23:01, Marc Kleine-Budde wrote:
>>>>>    /* Helper functions */
>>>>> @@ -2190,13 +2196,20 @@ static int rcar_canfd_probe(struct platform_device 
>>>>> *pdev)
>>>>>         */
>>>>>        gpriv->can_clk = devm_clk_get(dev, "can_clk");
>>>>>        if (IS_ERR(gpriv->can_clk) || (clk_get_rate(gpriv->can_clk) == 0)) {
>>>>               ^^^^^^^^^^^^^^^^^^^^^
>>>>
>>>> As sashiko.dev [1] pointed out here's no check for -EPROBE_DEFER.
>>> This looks to me a pre-existing issue.
>>>
>>> The newly added support does:
>>>
>>> fcan_freq = clk_get_rate(gpriv->clkp);
>>>
>>> At the time this is executed the clkp was already checked against any errors
>>                                     ^^^^
>>> (above) including -EPROBE_DEFER.
>> It's not about "clkp".
>>
>> What's missing is, that "gpriv->can_clk" is checked against IS_ERR():
>>
>>>>>        gpriv->can_clk = devm_clk_get(dev, "can_clk");
>>>>>        if (IS_ERR(gpriv->can_clk) || (clk_get_rate(gpriv->can_clk) == 0)) {
>> but if it's a -EPROBE_DEFER, IS_ERR() returns true. I think in this case
>> you want to return with dev_err_probe().
>>
>> There's also a difference between "can_clk" not found (e.g. not
>> configured in DT), and there was a problem with "can_clk". I don't know
>> if this is important for this driver, but if you want to distinguish
>> between those, you can use devm_clk_get_optional(). I will return NULL
>> if the clock is not found, -EPROBE_DEFER if not available yet, the clock
>> or, in case of an error -errno.
>>
>> Thinking about it: if "can_clk" is not available, I would consider it
>> intentional, so the driver should use the other clock. If the clock has
>> an error, the driver should not fallback to a probably untested clock
>> configuration.
> 
>          /* fCAN clock: Pick External clock. If not available fallback to
>           * CANFD clock
>           */
>          gpriv->can_clk = devm_clk_get(dev, "can_clk");
>          if (IS_ERR(gpriv->can_clk) || (clk_get_rate(gpriv->can_clk) == 0)) {
> 
> Based on the comment on this code "If not available fallback to CANFD clock"
> I think the intention with IS_ERR() is to allow probing go forward, falling back 
> to CANFD clock, in case of any error of devm_clk_get().
> 
> The device trees that I've checked sets the can_clk to a fixed clock with rate = 
> 0 or have the clock provided directly by the SoC clock provider. I think the 
> intention with IS_ERR() was not only for -ENOENT but for any other kind of 
> errors. We can return in case of -EPROBE_DEFER, to ensure the expected clock is 
> used, when it's available and sane.
> 
> I'll switch to something like:
> 
>   gpriv->can_clk = devm_clk_get(&pdev->dev, "can_clk");
> +if (ERR_PTR(gpriv->can_clk) == -EPROBE_DEFER)
> +    return dev_err_ptr(dev, ...);

On the other hand, returning -EPROBE_DEFER here means the driver may never probe 
if the can_clk provider fails to probe, even though the fallback clock is 
available. That would go against what I understand to be the intent of the 
IS_ERR() check: letting the probe continue when can_clk can't be obtained.

Please let me know how you would like me to proceed.

Thank you,
Claudiu



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v4 3/7] can: rcar_canfd: Allow the CAN FD clock to be sourced from fck
  2026-10-19 10:36           ` claudiu beznea
@ 2026-10-19 10:42             ` Marc Kleine-Budde
  0 siblings, 0 replies; 20+ messages in thread
From: Marc Kleine-Budde @ 2026-10-19 10:42 UTC (permalink / raw)
  To: claudiu beznea
  Cc: mailhol, robh, krzk+dt, conor+dt, geert+renesas, magnus.damm,
	biju.das.jz, fabrizio.castro.jz, linux-can, devicetree,
	linux-renesas-soc, linux-kernel, Claudiu Beznea

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

On 19.10.2026 13:36:49, claudiu beznea wrote:
> > Based on the comment on this code "If not available fallback to CANFD clock"
> > I think the intention with IS_ERR() is to allow probing go forward,
> > falling back to CANFD clock, in case of any error of devm_clk_get().
> >
> > The device trees that I've checked sets the can_clk to a fixed clock
> > with rate = 0 or have the clock provided directly by the SoC clock
> > provider. I think the intention with IS_ERR() was not only for -ENOENT
> > but for any other kind of errors. We can return in case of
> > -EPROBE_DEFER, to ensure the expected clock is used, when it's available
> > and sane.
> >
> > I'll switch to something like:
> >
> >   gpriv->can_clk = devm_clk_get(&pdev->dev, "can_clk");
> > +if (ERR_PTR(gpriv->can_clk) == -EPROBE_DEFER)
> > +    return dev_err_ptr(dev, ...);
>
> On the other hand, returning -EPROBE_DEFER here means the driver may never
> probe if the can_clk provider fails to probe, even though the fallback clock
> is available. That would go against what I understand to be the intent of
> the IS_ERR() check: letting the probe continue when can_clk can't be
> obtained.
>
> Please let me know how you would like me to proceed.

If the clock continuously fails with the error -EPROBE_DEFER, your
.config/DT/system is broken. I don't think it's a good idea to ignore
these errors.

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: 228 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2026-10-19 10:42 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-02 14:15 [PATCH v4 0/7] can: rcar_canfd: Add support for Renesas RZ/G3S Claudiu Beznea
2026-09-02 14:15 ` [PATCH v4 1/7] dt-bindings: can: renesas,rcar-canfd: Document RZ/G3S SoC Claudiu Beznea
2026-10-16 13:34   ` Marc Kleine-Budde
2026-10-19  7:24     ` Biju Das
2026-10-19  8:01       ` Claudiu Beznea
2026-10-19  8:26         ` Biju Das
2026-09-02 14:15 ` [PATCH v4 2/7] can: rcar_canfd: Fix typos in macro names Claudiu Beznea
2026-09-02 14:15 ` [PATCH v4 3/7] can: rcar_canfd: Allow the CAN FD clock to be sourced from fck Claudiu Beznea
2026-10-16 13:39   ` Marc Kleine-Budde
2026-10-16 18:07     ` Claudiu Beznea
2026-10-17 20:01       ` Marc Kleine-Budde
2026-10-19  9:58         ` claudiu beznea
2026-10-19 10:21           ` Marc Kleine-Budde
2026-10-19 10:36           ` claudiu beznea
2026-10-19 10:42             ` Marc Kleine-Budde
2026-09-02 14:15 ` [PATCH v4 4/7] can: rcar_canfd: Do not set registers selecting the CAN mode Claudiu Beznea
2026-09-02 14:15 ` [PATCH v4 5/7] can: rcar_canfd: Add support for Renesas RZ/G3S Claudiu Beznea
2026-09-02 14:15 ` [PATCH v4 6/7] arm64: dts: renesas: r9a08g045: Add CAN-FD node Claudiu Beznea
2026-09-02 14:15 ` [PATCH v4 7/7] arm64: dts: renesas: rzg3s-smarc: Enable CAN-FD Claudiu Beznea
2026-09-29 14:19 ` [PATCH v4 0/7] can: rcar_canfd: Add support for Renesas RZ/G3S Marc Kleine-Budde

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®