All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] Renesas RZ/G2L CANFD support
@ 2021-07-15 18:21 Lad Prabhakar
  2021-07-15 18:21 ` [PATCH 1/6] dt-bindings: net: can: renesas,rcar-canfd: Document RZ/G2L SoC Lad Prabhakar
                   ` (5 more replies)
  0 siblings, 6 replies; 19+ messages in thread
From: Lad Prabhakar @ 2021-07-15 18:21 UTC (permalink / raw)
  To: Geert Uytterhoeven, Rob Herring, Wolfgang Grandegger,
	Marc Kleine-Budde, David S. Miller, Jakub Kicinski,
	Michael Turquette, Stephen Boyd, Philipp Zabel
  Cc: linux-can, netdev, devicetree, linux-clk, linux-kernel,
	linux-renesas-soc, Prabhakar, Biju Das, Lad Prabhakar

Hi All,

This patch series adds CANFD support to Renesas RZ/G2L family.

CANFD block on RZ/G2L SoC is almost identical to one found on
R-Car Gen3 SoC's. On RZ/G2L SoC interrupt sources for each channel
are split into individual sources.

Patches are based on top of [1] (master branch) + patch [2].

Cheers,
Prabhakar

[1] https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/log/
[2] https://patchwork.kernel.org/project/linux-renesas-soc/patch/
20210712194422.12405-4-prabhakar.mahadev-lad.rj@bp.renesas.com/

Lad Prabhakar (6):
  dt-bindings: net: can: renesas,rcar-canfd: Document RZ/G2L SoC
  can: rcar_canfd: Add support for RZ/G2L family
  dt-bindings: clk: r9a07g044-cpg: Add entry for P0_DIV2 core clock
  clk: renesas: r9a07g044-cpg: Add entry for fixed clock P0_DIV2
  clk: renesas: r9a07g044-cpg: Add clock and reset entries for CANFD
  arm64: dts: renesas: r9a07g044: Add CANFD node

 .../bindings/net/can/renesas,rcar-canfd.yaml  |  45 ++-
 arch/arm64/boot/dts/renesas/r9a07g044.dtsi    |  37 +++
 drivers/clk/renesas/r9a07g044-cpg.c           |   7 +-
 drivers/net/can/rcar/rcar_canfd.c             | 275 ++++++++++++++++--
 include/dt-bindings/clock/r9a07g044-cpg.h     |   2 +
 5 files changed, 328 insertions(+), 38 deletions(-)


base-commit: b37235d5fdf50e5f1c23f868ab70bbe640081b21
prerequisite-patch-id: 7436c0d801737268ef470fcb50e620428286e085
-- 
2.17.1


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

* [PATCH 1/6] dt-bindings: net: can: renesas,rcar-canfd: Document RZ/G2L SoC
  2021-07-15 18:21 [PATCH 0/6] Renesas RZ/G2L CANFD support Lad Prabhakar
@ 2021-07-15 18:21 ` Lad Prabhakar
  2021-07-16  7:38   ` Geert Uytterhoeven
  2021-07-15 18:21 ` [PATCH 2/6] can: rcar_canfd: Add support for RZ/G2L family Lad Prabhakar
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 19+ messages in thread
From: Lad Prabhakar @ 2021-07-15 18:21 UTC (permalink / raw)
  To: Geert Uytterhoeven, Rob Herring, Wolfgang Grandegger,
	Marc Kleine-Budde, David S. Miller, Jakub Kicinski,
	Michael Turquette, Stephen Boyd, Philipp Zabel
  Cc: linux-can, netdev, devicetree, linux-clk, linux-kernel,
	linux-renesas-soc, Prabhakar, Biju Das, Lad Prabhakar

Add CANFD binding documentation for Renesas RZ/G2L SoC.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 .../bindings/net/can/renesas,rcar-canfd.yaml  | 45 ++++++++++++++++---
 1 file changed, 39 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
index 0b33ba9ccb47..38243c261622 100644
--- a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
+++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
@@ -30,13 +30,15 @@ properties:
               - renesas,r8a77995-canfd     # R-Car D3
           - const: renesas,rcar-gen3-canfd # R-Car Gen3 and RZ/G2
 
+      - items:
+          - enum:
+              - renesas,r9a07g044-canfd    # RZ/G2{L,LC}
+          - const: renesas,rzg2l-canfd     # RZ/G2L family
+
   reg:
     maxItems: 1
 
-  interrupts:
-    items:
-      - description: Channel interrupt
-      - description: Global interrupt
+  interrupts: true
 
   clocks:
     maxItems: 3
@@ -50,8 +52,7 @@ properties:
   power-domains:
     maxItems: 1
 
-  resets:
-    maxItems: 1
+  resets: true
 
   renesas,no-can-fd:
     $ref: /schemas/types.yaml#/definitions/flag
@@ -78,6 +79,38 @@ patternProperties:
       node.  Each child node supports the "status" property only, which
       is used to enable/disable the respective channel.
 
+if:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - renesas,rzg2l-canfd
+then:
+  properties:
+    interrupts:
+      items:
+        - description: CAN global error interrupt
+        - description: CAN receive FIFO interrupt
+        - description: CAN0 error interrupt
+        - description: CAN0 transmit interrupt
+        - description: CAN0 transmit/receive FIFO receive completion interrupt
+        - description: CAN1 error interrupt
+        - description: CAN1 transmit interrupt
+        - description: CAN1 transmit/receive FIFO receive completion interrupt
+
+    resets:
+      maxItems: 2
+
+else:
+  properties:
+    interrupts:
+      items:
+        - description: Channel interrupt
+        - description: Global interrupt
+
+    resets:
+      maxItems: 1
+
 required:
   - compatible
   - reg
-- 
2.17.1


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

* [PATCH 2/6] can: rcar_canfd: Add support for RZ/G2L family
  2021-07-15 18:21 [PATCH 0/6] Renesas RZ/G2L CANFD support Lad Prabhakar
  2021-07-15 18:21 ` [PATCH 1/6] dt-bindings: net: can: renesas,rcar-canfd: Document RZ/G2L SoC Lad Prabhakar
@ 2021-07-15 18:21 ` Lad Prabhakar
  2021-07-16  7:47   ` Geert Uytterhoeven
  2021-07-16 10:10   ` Marc Kleine-Budde
  2021-07-15 18:21 ` [PATCH 3/6] dt-bindings: clk: r9a07g044-cpg: Add entry for P0_DIV2 core clock Lad Prabhakar
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 19+ messages in thread
From: Lad Prabhakar @ 2021-07-15 18:21 UTC (permalink / raw)
  To: Geert Uytterhoeven, Rob Herring, Wolfgang Grandegger,
	Marc Kleine-Budde, David S. Miller, Jakub Kicinski,
	Michael Turquette, Stephen Boyd, Philipp Zabel
  Cc: linux-can, netdev, devicetree, linux-clk, linux-kernel,
	linux-renesas-soc, Prabhakar, Biju Das, Lad Prabhakar

CANFD block on RZ/G2L SoC is almost identical to one found on
R-Car Gen3 SoC's.

On RZ/G2L SoC interrupt sources for each channel are split into
different sources, irq handlers for the same are added.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/net/can/rcar/rcar_canfd.c | 275 ++++++++++++++++++++++++++----
 1 file changed, 244 insertions(+), 31 deletions(-)

diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index 311e6ca3bdc4..5dfbc5fa2d81 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -37,9 +37,13 @@
 #include <linux/bitmap.h>
 #include <linux/bitops.h>
 #include <linux/iopoll.h>
+#include <linux/reset.h>
 
 #define RCANFD_DRV_NAME			"rcar_canfd"
 
+#define RENESAS_RCAR_GEN3	0
+#define RENESAS_RZG2L		1
+
 /* Global register bits */
 
 /* RSCFDnCFDGRMCFG */
@@ -513,6 +517,9 @@ struct rcar_canfd_global {
 	enum rcar_canfd_fcanclk fcan;	/* CANFD or Ext clock */
 	unsigned long channels_mask;	/* Enabled channels mask */
 	bool fdmode;			/* CAN FD or Classical CAN only mode */
+	struct reset_control *rstc1;     /* Pointer to reset source1 */
+	struct reset_control *rstc2;     /* Pointer to reset source2 */
+	unsigned int chip_id;
 };
 
 /* CAN FD mode nominal rate constants */
@@ -1070,6 +1077,56 @@ static void rcar_canfd_tx_done(struct net_device *ndev)
 	can_led_event(ndev, CAN_LED_EVENT_TX);
 }
 
+static irqreturn_t rcar_canfd_global_err_interrupt(int irq, void *dev_id)
+{
+	struct rcar_canfd_global *gpriv = dev_id;
+	struct net_device *ndev;
+	struct rcar_canfd_channel *priv;
+	u32 gerfl;
+	u32 ch, ridx;
+
+	/* Global error interrupts still indicate a condition specific to a channel. */
+	for_each_set_bit(ch, &gpriv->channels_mask, RCANFD_NUM_CHANNELS) {
+		priv = gpriv->ch[ch];
+		ndev = priv->ndev;
+		ridx = ch + RCANFD_RFFIFO_IDX;
+
+		/* Global error interrupts */
+		gerfl = rcar_canfd_read(priv->base, RCANFD_GERFL);
+		if (unlikely(RCANFD_GERFL_ERR(gpriv, gerfl)))
+			rcar_canfd_global_error(ndev);
+	}
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t rcar_canfd_global_recieve_fifo_interrupt(int irq, void *dev_id)
+{
+	struct rcar_canfd_global *gpriv = dev_id;
+	struct net_device *ndev;
+	struct rcar_canfd_channel *priv;
+	u32 sts;
+	u32 ch, ridx;
+
+	for_each_set_bit(ch, &gpriv->channels_mask, RCANFD_NUM_CHANNELS) {
+		priv = gpriv->ch[ch];
+		ndev = priv->ndev;
+		ridx = ch + RCANFD_RFFIFO_IDX;
+
+		/* Handle Rx interrupts */
+		sts = rcar_canfd_read(priv->base, RCANFD_RFSTS(ridx));
+		if (likely(sts & RCANFD_RFSTS_RFIF)) {
+			if (napi_schedule_prep(&priv->napi)) {
+				/* Disable Rx FIFO interrupts */
+				rcar_canfd_clear_bit(priv->base,
+						     RCANFD_RFCC(ridx),
+						     RCANFD_RFCC_RFIE);
+				__napi_schedule(&priv->napi);
+			}
+		}
+	}
+	return IRQ_HANDLED;
+}
+
 static irqreturn_t rcar_canfd_global_interrupt(int irq, void *dev_id)
 {
 	struct rcar_canfd_global *gpriv = dev_id;
@@ -1139,6 +1196,56 @@ static void rcar_canfd_state_change(struct net_device *ndev,
 	}
 }
 
+static irqreturn_t rcar_canfd_channel_tx_interrupt(int irq, void *dev_id)
+{
+	struct rcar_canfd_global *gpriv = dev_id;
+	struct net_device *ndev;
+	struct rcar_canfd_channel *priv;
+	u32 sts, ch;
+
+	/* Common FIFO is a per channel resource */
+	for_each_set_bit(ch, &gpriv->channels_mask, RCANFD_NUM_CHANNELS) {
+		priv = gpriv->ch[ch];
+		ndev = priv->ndev;
+
+		/* Handle Tx interrupts */
+		sts = rcar_canfd_read(priv->base,
+				      RCANFD_CFSTS(ch, RCANFD_CFFIFO_IDX));
+		if (likely(sts & RCANFD_CFSTS_CFTXIF))
+			rcar_canfd_tx_done(ndev);
+	}
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t rcar_canfd_channel_err_interrupt(int irq, void *dev_id)
+{
+	struct rcar_canfd_global *gpriv = dev_id;
+	struct net_device *ndev;
+	struct rcar_canfd_channel *priv;
+	u32 sts, ch, cerfl;
+	u16 txerr, rxerr;
+
+	/* Common FIFO is a per channel resource */
+	for_each_set_bit(ch, &gpriv->channels_mask, RCANFD_NUM_CHANNELS) {
+		priv = gpriv->ch[ch];
+		ndev = priv->ndev;
+
+		/* Channel error interrupts */
+		cerfl = rcar_canfd_read(priv->base, RCANFD_CERFL(ch));
+		sts = rcar_canfd_read(priv->base, RCANFD_CSTS(ch));
+		txerr = RCANFD_CSTS_TECCNT(sts);
+		rxerr = RCANFD_CSTS_RECCNT(sts);
+		if (unlikely(RCANFD_CERFL_ERR(cerfl)))
+			rcar_canfd_error(ndev, cerfl, txerr, rxerr);
+
+		/* Handle state change to lower states */
+		if (unlikely(priv->can.state != CAN_STATE_ERROR_ACTIVE &&
+			     priv->can.state != CAN_STATE_BUS_OFF))
+			rcar_canfd_state_change(ndev, txerr, rxerr);
+	}
+	return IRQ_HANDLED;
+}
+
 static irqreturn_t rcar_canfd_channel_interrupt(int irq, void *dev_id)
 {
 	struct rcar_canfd_global *gpriv = dev_id;
@@ -1560,6 +1667,10 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch,
 	struct rcar_canfd_channel *priv;
 	struct net_device *ndev;
 	int err = -ENODEV;
+	char *irq_name;
+	int err_irq;
+	int tx_irq;
+	int i;
 
 	ndev = alloc_candev(sizeof(*priv), RCANFD_FIFO_DEPTH);
 	if (!ndev) {
@@ -1577,6 +1688,44 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch,
 	priv->can.clock.freq = fcan_freq;
 	dev_info(&pdev->dev, "can_clk rate is %u\n", priv->can.clock.freq);
 
+	if (gpriv->chip_id == RENESAS_RZG2L) {
+		if (!ch)
+			i = 2;
+		else
+			i = 5;
+		err_irq = platform_get_irq(pdev, i++);
+		if (err_irq < 0) {
+			err = err_irq;
+			goto fail;
+		}
+		tx_irq = platform_get_irq(pdev, i);
+		if (tx_irq < 0) {
+			err = tx_irq;
+			goto fail;
+		}
+
+		irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
+					  "canfd.chnerr%d", ch);
+		err = devm_request_irq(&pdev->dev, err_irq,
+				       rcar_canfd_channel_err_interrupt, 0,
+				       irq_name, gpriv);
+		if (err) {
+			dev_err(&pdev->dev, "devm_request_irq CH Err(%d) failed, error %d\n",
+				err_irq, err);
+			goto fail;
+		}
+		irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
+					  "canfd.chntx%d", ch);
+		err = devm_request_irq(&pdev->dev, tx_irq,
+				       rcar_canfd_channel_tx_interrupt, 0,
+				       irq_name, gpriv);
+		if (err) {
+			dev_err(&pdev->dev, "devm_request_irq Tx (%d) failed, error %d\n",
+				tx_irq, err);
+			goto fail;
+		}
+	}
+
 	if (gpriv->fdmode) {
 		priv->can.bittiming_const = &rcar_canfd_nom_bittiming_const;
 		priv->can.data_bittiming_const =
@@ -1635,8 +1784,11 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 	struct rcar_canfd_global *gpriv;
 	struct device_node *of_child;
 	unsigned long channels_mask = 0;
-	int err, ch_irq, g_irq;
+	int err, ch_irq, g_irq, g_rx_irq;
 	bool fdmode = true;			/* CAN FD only mode - default */
+	unsigned int chip_id;
+
+	chip_id = (uintptr_t)of_device_get_match_data(&pdev->dev);
 
 	if (of_property_read_bool(pdev->dev.of_node, "renesas,no-can-fd"))
 		fdmode = false;			/* Classical CAN only mode */
@@ -1649,27 +1801,56 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 	if (of_child && of_device_is_available(of_child))
 		channels_mask |= BIT(1);	/* Channel 1 */
 
-	ch_irq = platform_get_irq(pdev, 0);
-	if (ch_irq < 0) {
-		err = ch_irq;
-		goto fail_dev;
-	}
+	if (chip_id == RENESAS_RCAR_GEN3) {
+		ch_irq = platform_get_irq(pdev, 0);
+		if (ch_irq < 0)
+			return ch_irq;
 
-	g_irq = platform_get_irq(pdev, 1);
-	if (g_irq < 0) {
-		err = g_irq;
-		goto fail_dev;
+		g_irq = platform_get_irq(pdev, 1);
+		if (g_irq < 0)
+			return g_irq;
+	} else {
+		g_irq = platform_get_irq(pdev, 0);
+		if (g_irq < 0)
+			return g_irq;
+
+		g_rx_irq = platform_get_irq(pdev, 1);
+		if (g_rx_irq < 0)
+			return g_rx_irq;
 	}
 
 	/* Global controller context */
 	gpriv = devm_kzalloc(&pdev->dev, sizeof(*gpriv), GFP_KERNEL);
-	if (!gpriv) {
-		err = -ENOMEM;
-		goto fail_dev;
-	}
+	if (!gpriv)
+		return -ENOMEM;
+
 	gpriv->pdev = pdev;
 	gpriv->channels_mask = channels_mask;
 	gpriv->fdmode = fdmode;
+	gpriv->chip_id = chip_id;
+
+	if (gpriv->chip_id == RENESAS_RZG2L) {
+		gpriv->rstc1 = devm_reset_control_get_exclusive_by_index(&pdev->dev, 0);
+		if (IS_ERR(gpriv->rstc1)) {
+			dev_err(&pdev->dev, "failed to get reset index 0\n");
+			return PTR_ERR(gpriv->rstc1);
+		}
+
+		err = reset_control_reset(gpriv->rstc1);
+		if (err)
+			return err;
+
+		gpriv->rstc2 = devm_reset_control_get_exclusive_by_index(&pdev->dev, 1);
+		if (IS_ERR(gpriv->rstc2)) {
+			dev_err(&pdev->dev, "failed to get reset index 1\n");
+			return PTR_ERR(gpriv->rstc2);
+		}
+		err = reset_control_reset(gpriv->rstc2);
+		if (err) {
+			reset_control_assert(gpriv->rstc1);
+			return err;
+		}
+	}
 
 	/* Peripheral clock */
 	gpriv->clkp = devm_clk_get(&pdev->dev, "fck");
@@ -1699,7 +1880,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 	}
 	fcan_freq = clk_get_rate(gpriv->can_clk);
 
-	if (gpriv->fcan == RCANFD_CANFDCLK)
+	if (gpriv->fcan == RCANFD_CANFDCLK && gpriv->chip_id == RENESAS_RCAR_GEN3)
 		/* CANFD clock is further divided by (1/2) within the IP */
 		fcan_freq /= 2;
 
@@ -1711,21 +1892,43 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 	gpriv->base = addr;
 
 	/* Request IRQ that's common for both channels */
-	err = devm_request_irq(&pdev->dev, ch_irq,
-			       rcar_canfd_channel_interrupt, 0,
-			       "canfd.chn", gpriv);
-	if (err) {
-		dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n",
-			ch_irq, err);
-		goto fail_dev;
-	}
-	err = devm_request_irq(&pdev->dev, g_irq,
-			       rcar_canfd_global_interrupt, 0,
-			       "canfd.gbl", gpriv);
-	if (err) {
-		dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n",
-			g_irq, err);
-		goto fail_dev;
+	if (gpriv->chip_id == RENESAS_RCAR_GEN3) {
+		err = devm_request_irq(&pdev->dev, ch_irq,
+				       rcar_canfd_channel_interrupt, 0,
+				       "canfd.chn", gpriv);
+		if (err) {
+			dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n",
+				ch_irq, err);
+			goto fail_dev;
+		}
+
+		err = devm_request_irq(&pdev->dev, g_irq,
+				       rcar_canfd_global_interrupt, 0,
+				       "canfd.gbl", gpriv);
+		if (err) {
+			dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n",
+				g_irq, err);
+			goto fail_dev;
+		}
+	} else {
+		err = devm_request_irq(&pdev->dev, g_rx_irq,
+				       rcar_canfd_global_recieve_fifo_interrupt, 0,
+				       "canfd.gblrx", gpriv);
+
+		if (err) {
+			dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n",
+				g_rx_irq, err);
+			goto fail_dev;
+		}
+
+		err = devm_request_irq(&pdev->dev, g_irq,
+				       rcar_canfd_global_err_interrupt, 0,
+				       "canfd.gblerr", gpriv);
+		if (err) {
+			dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n",
+				g_irq, err);
+			goto fail_dev;
+		}
 	}
 
 	/* Enable peripheral clock for register access */
@@ -1791,6 +1994,10 @@ static int rcar_canfd_probe(struct platform_device *pdev)
 fail_clk:
 	clk_disable_unprepare(gpriv->clkp);
 fail_dev:
+	if (gpriv->chip_id == RENESAS_RZG2L) {
+		reset_control_assert(gpriv->rstc1);
+		reset_control_assert(gpriv->rstc2);
+	}
 	return err;
 }
 
@@ -1810,6 +2017,11 @@ static int rcar_canfd_remove(struct platform_device *pdev)
 	/* Enter global sleep mode */
 	rcar_canfd_set_bit(gpriv->base, RCANFD_GCTR, RCANFD_GCTR_GSLPR);
 	clk_disable_unprepare(gpriv->clkp);
+	if (gpriv->chip_id == RENESAS_RZG2L) {
+		reset_control_assert(gpriv->rstc1);
+		reset_control_assert(gpriv->rstc2);
+	}
+
 	return 0;
 }
 
@@ -1827,7 +2039,8 @@ static SIMPLE_DEV_PM_OPS(rcar_canfd_pm_ops, rcar_canfd_suspend,
 			 rcar_canfd_resume);
 
 static const struct of_device_id rcar_canfd_of_table[] = {
-	{ .compatible = "renesas,rcar-gen3-canfd" },
+	{ .compatible = "renesas,rcar-gen3-canfd", .data = (void *)RENESAS_RCAR_GEN3 },
+	{ .compatible = "renesas,rzg2l-canfd", .data = (void *)RENESAS_RZG2L },
 	{ }
 };
 
-- 
2.17.1


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

* [PATCH 3/6] dt-bindings: clk: r9a07g044-cpg: Add entry for P0_DIV2 core clock
  2021-07-15 18:21 [PATCH 0/6] Renesas RZ/G2L CANFD support Lad Prabhakar
  2021-07-15 18:21 ` [PATCH 1/6] dt-bindings: net: can: renesas,rcar-canfd: Document RZ/G2L SoC Lad Prabhakar
  2021-07-15 18:21 ` [PATCH 2/6] can: rcar_canfd: Add support for RZ/G2L family Lad Prabhakar
@ 2021-07-15 18:21 ` Lad Prabhakar
  2021-07-16  8:07   ` Geert Uytterhoeven
  2021-07-15 18:21 ` [PATCH 4/6] clk: renesas: r9a07g044-cpg: Add entry for fixed clock P0_DIV2 Lad Prabhakar
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 19+ messages in thread
From: Lad Prabhakar @ 2021-07-15 18:21 UTC (permalink / raw)
  To: Geert Uytterhoeven, Rob Herring, Wolfgang Grandegger,
	Marc Kleine-Budde, David S. Miller, Jakub Kicinski,
	Michael Turquette, Stephen Boyd, Philipp Zabel
  Cc: linux-can, netdev, devicetree, linux-clk, linux-kernel,
	linux-renesas-soc, Prabhakar, Biju Das, Lad Prabhakar

Add P0_DIV2 core clock required for CANFD module. CANFD core clock is
sourced from P0_DIV2 referenced from HW manual Rev.0.50.

Also add R9A07G044_LAST_CORE_CLK entry to avoid changes in
r9a07g044-cpg.c file.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 include/dt-bindings/clock/r9a07g044-cpg.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/dt-bindings/clock/r9a07g044-cpg.h b/include/dt-bindings/clock/r9a07g044-cpg.h
index 0728ad07ff7a..2fd20db0b2f4 100644
--- a/include/dt-bindings/clock/r9a07g044-cpg.h
+++ b/include/dt-bindings/clock/r9a07g044-cpg.h
@@ -30,6 +30,8 @@
 #define R9A07G044_CLK_P2		19
 #define R9A07G044_CLK_AT		20
 #define R9A07G044_OSCCLK		21
+#define R9A07G044_CLK_P0_DIV2		22
+#define R9A07G044_LAST_CORE_CLK		23
 
 /* R9A07G044 Module Clocks */
 #define R9A07G044_CA55_SCLK		0
-- 
2.17.1


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

* [PATCH 4/6] clk: renesas: r9a07g044-cpg: Add entry for fixed clock P0_DIV2
  2021-07-15 18:21 [PATCH 0/6] Renesas RZ/G2L CANFD support Lad Prabhakar
                   ` (2 preceding siblings ...)
  2021-07-15 18:21 ` [PATCH 3/6] dt-bindings: clk: r9a07g044-cpg: Add entry for P0_DIV2 core clock Lad Prabhakar
@ 2021-07-15 18:21 ` Lad Prabhakar
  2021-07-16  8:09   ` Geert Uytterhoeven
  2021-07-15 18:21 ` [PATCH 5/6] clk: renesas: r9a07g044-cpg: Add clock and reset entries for CANFD Lad Prabhakar
  2021-07-15 18:21 ` [PATCH 6/6] arm64: dts: renesas: r9a07g044: Add CANFD node Lad Prabhakar
  5 siblings, 1 reply; 19+ messages in thread
From: Lad Prabhakar @ 2021-07-15 18:21 UTC (permalink / raw)
  To: Geert Uytterhoeven, Rob Herring, Wolfgang Grandegger,
	Marc Kleine-Budde, David S. Miller, Jakub Kicinski,
	Michael Turquette, Stephen Boyd, Philipp Zabel
  Cc: linux-can, netdev, devicetree, linux-clk, linux-kernel,
	linux-renesas-soc, Prabhakar, Biju Das, Lad Prabhakar

Add entry for fixed core clock P0_DIV2 and assign LAST_DT_CORE_CLK
to R9A07G044_LAST_CORE_CLK.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/clk/renesas/r9a07g044-cpg.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/renesas/r9a07g044-cpg.c b/drivers/clk/renesas/r9a07g044-cpg.c
index 78fae93cf249..0876df9c286d 100644
--- a/drivers/clk/renesas/r9a07g044-cpg.c
+++ b/drivers/clk/renesas/r9a07g044-cpg.c
@@ -16,7 +16,7 @@
 
 enum clk_ids {
 	/* Core Clock Outputs exported to DT */
-	LAST_DT_CORE_CLK = R9A07G044_OSCCLK,
+	LAST_DT_CORE_CLK = R9A07G044_LAST_CORE_CLK,
 
 	/* External Input Clocks */
 	CLK_EXTAL,
@@ -77,6 +77,7 @@ static const struct cpg_core_clk r9a07g044_core_clks[] __initconst = {
 	DEF_FIXED("I", R9A07G044_CLK_I, CLK_PLL1, 1, 1),
 	DEF_DIV("P0", R9A07G044_CLK_P0, CLK_PLL2_DIV16, DIVPL2A,
 		dtable_1_32, CLK_DIVIDER_HIWORD_MASK),
+	DEF_FIXED("P0_DIV2", R9A07G044_CLK_P0_DIV2, R9A07G044_CLK_P0, 1, 2),
 	DEF_FIXED("TSU", R9A07G044_CLK_TSU, CLK_PLL2_DIV20, 1, 1),
 	DEF_DIV("P1", R9A07G044_CLK_P1, CLK_PLL3_DIV2_4,
 		DIVPL3B, dtable_1_32, CLK_DIVIDER_HIWORD_MASK),
-- 
2.17.1


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

* [PATCH 5/6] clk: renesas: r9a07g044-cpg: Add clock and reset entries for CANFD
  2021-07-15 18:21 [PATCH 0/6] Renesas RZ/G2L CANFD support Lad Prabhakar
                   ` (3 preceding siblings ...)
  2021-07-15 18:21 ` [PATCH 4/6] clk: renesas: r9a07g044-cpg: Add entry for fixed clock P0_DIV2 Lad Prabhakar
@ 2021-07-15 18:21 ` Lad Prabhakar
  2021-07-16  7:55   ` Geert Uytterhoeven
  2021-07-15 18:21 ` [PATCH 6/6] arm64: dts: renesas: r9a07g044: Add CANFD node Lad Prabhakar
  5 siblings, 1 reply; 19+ messages in thread
From: Lad Prabhakar @ 2021-07-15 18:21 UTC (permalink / raw)
  To: Geert Uytterhoeven, Rob Herring, Wolfgang Grandegger,
	Marc Kleine-Budde, David S. Miller, Jakub Kicinski,
	Michael Turquette, Stephen Boyd, Philipp Zabel
  Cc: linux-can, netdev, devicetree, linux-clk, linux-kernel,
	linux-renesas-soc, Prabhakar, Biju Das, Lad Prabhakar

Add clock and reset entries for CANFD in CPG driver.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/clk/renesas/r9a07g044-cpg.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/clk/renesas/r9a07g044-cpg.c b/drivers/clk/renesas/r9a07g044-cpg.c
index 0876df9c286d..78f0efb19af8 100644
--- a/drivers/clk/renesas/r9a07g044-cpg.c
+++ b/drivers/clk/renesas/r9a07g044-cpg.c
@@ -141,6 +141,8 @@ static struct rzg2l_mod_clk r9a07g044_mod_clks[] = {
 				0x584, 4),
 	DEF_MOD("sci0",		R9A07G044_SCI0_CLKP, R9A07G044_CLK_P0,
 				0x588, 0),
+	DEF_MOD("canfd",	R9A07G044_CANFD_PCLK, R9A07G044_CLK_P0,
+				0x594, 0),
 	DEF_MOD("gpio",		R9A07G044_GPIO_HCLK, R9A07G044_OSCCLK,
 				0x598, 0),
 };
@@ -169,6 +171,8 @@ static struct rzg2l_reset r9a07g044_resets[] = {
 	DEF_RST(R9A07G044_SCIF3_RST_SYSTEM_N, 0x884, 3),
 	DEF_RST(R9A07G044_SCIF4_RST_SYSTEM_N, 0x884, 4),
 	DEF_RST(R9A07G044_SCI0_RST, 0x888, 0),
+	DEF_RST(R9A07G044_CANFD_RSTP_N, 0x894, 0),
+	DEF_RST(R9A07G044_CANFD_RSTC_N, 0x894, 1),
 	DEF_RST(R9A07G044_GPIO_RSTN, 0x898, 0),
 	DEF_RST(R9A07G044_GPIO_PORT_RESETN, 0x898, 1),
 	DEF_RST(R9A07G044_GPIO_SPARE_RESETN, 0x898, 2),
-- 
2.17.1


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

* [PATCH 6/6] arm64: dts: renesas: r9a07g044: Add CANFD node
  2021-07-15 18:21 [PATCH 0/6] Renesas RZ/G2L CANFD support Lad Prabhakar
                   ` (4 preceding siblings ...)
  2021-07-15 18:21 ` [PATCH 5/6] clk: renesas: r9a07g044-cpg: Add clock and reset entries for CANFD Lad Prabhakar
@ 2021-07-15 18:21 ` Lad Prabhakar
  5 siblings, 0 replies; 19+ messages in thread
From: Lad Prabhakar @ 2021-07-15 18:21 UTC (permalink / raw)
  To: Geert Uytterhoeven, Rob Herring, Wolfgang Grandegger,
	Marc Kleine-Budde, David S. Miller, Jakub Kicinski,
	Michael Turquette, Stephen Boyd, Philipp Zabel
  Cc: linux-can, netdev, devicetree, linux-clk, linux-kernel,
	linux-renesas-soc, Prabhakar, Biju Das, Lad Prabhakar

Add CANFD node to R9A07G044 (RZ/G2L) SoC DTSI.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
index 9a7489dc70d1..fdb990b90f72 100644
--- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
@@ -13,6 +13,13 @@
 	#address-cells = <2>;
 	#size-cells = <2>;
 
+	/* External CAN clock - to be overridden by boards that provide it */
+	can_clk: can {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <0>;
+	};
+
 	/* clock can be either from exclk or crystal oscillator (XIN/XOUT) */
 	extal_clk: extal {
 		compatible = "fixed-clock";
@@ -89,6 +96,36 @@
 			status = "disabled";
 		};
 
+		canfd: can@10050000 {
+			compatible = "renesas,r9a07g044-canfd", "renesas,rzg2l-canfd";
+			reg = <0 0x10050000 0 0x8000>;
+			interrupts = <GIC_SPI 426 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 427 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 422 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 428 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 424 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 423 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 429 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 425 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD R9A07G044_CANFD_PCLK>,
+				 <&cpg CPG_CORE R9A07G044_CLK_P0_DIV2>,
+				 <&can_clk>;
+			clock-names = "fck", "canfd", "can_clk";
+			assigned-clocks = <&cpg CPG_CORE R9A07G044_CLK_P0_DIV2>;
+			assigned-clock-rates = <50000000>;
+			resets = <&cpg R9A07G044_CANFD_RSTP_N>,
+				 <&cpg R9A07G044_CANFD_RSTC_N>;
+			power-domains = <&cpg>;
+			status = "disabled";
+
+			channel0 {
+				status = "disabled";
+			};
+			channel1 {
+				status = "disabled";
+			};
+		};
+
 		i2c0: i2c@10058000 {
 			#address-cells = <1>;
 			#size-cells = <0>;
-- 
2.17.1


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

* Re: [PATCH 1/6] dt-bindings: net: can: renesas,rcar-canfd: Document RZ/G2L SoC
  2021-07-15 18:21 ` [PATCH 1/6] dt-bindings: net: can: renesas,rcar-canfd: Document RZ/G2L SoC Lad Prabhakar
@ 2021-07-16  7:38   ` Geert Uytterhoeven
  2021-07-16  8:30     ` Lad, Prabhakar
  0 siblings, 1 reply; 19+ messages in thread
From: Geert Uytterhoeven @ 2021-07-16  7:38 UTC (permalink / raw)
  To: Lad Prabhakar
  Cc: Rob Herring, Wolfgang Grandegger, Marc Kleine-Budde,
	David S. Miller, Jakub Kicinski, Michael Turquette, Stephen Boyd,
	Philipp Zabel, linux-can, netdev,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-clk, Linux Kernel Mailing List, Linux-Renesas, Prabhakar,
	Biju Das

Hi Prabhakar,

On Thu, Jul 15, 2021 at 8:21 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Add CANFD binding documentation for Renesas RZ/G2L SoC.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

> --- a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> +++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml

> @@ -78,6 +79,38 @@ patternProperties:
>        node.  Each child node supports the "status" property only, which
>        is used to enable/disable the respective channel.
>
> +if:
> +  properties:
> +    compatible:
> +      contains:
> +        enum:
> +          - renesas,rzg2l-canfd
> +then:
> +  properties:
> +    interrupts:
> +      items:
> +        - description: CAN global error interrupt
> +        - description: CAN receive FIFO interrupt
> +        - description: CAN0 error interrupt
> +        - description: CAN0 transmit interrupt
> +        - description: CAN0 transmit/receive FIFO receive completion interrupt
> +        - description: CAN1 error interrupt
> +        - description: CAN1 transmit interrupt
> +        - description: CAN1 transmit/receive FIFO receive completion interrupt

Does it make sense to add interrupt-names?

> +
> +    resets:
> +      maxItems: 2

Same here, for reset-names?
Or a list of descriptions, so we know which reset serves what purpose.

> +
> +else:
> +  properties:
> +    interrupts:
> +      items:
> +        - description: Channel interrupt
> +        - description: Global interrupt
> +
> +    resets:
> +      maxItems: 1
> +
>  required:
>    - compatible
>    - reg

The rest looks good to me.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH 2/6] can: rcar_canfd: Add support for RZ/G2L family
  2021-07-15 18:21 ` [PATCH 2/6] can: rcar_canfd: Add support for RZ/G2L family Lad Prabhakar
@ 2021-07-16  7:47   ` Geert Uytterhoeven
  2021-07-16  8:32     ` Lad, Prabhakar
  2021-07-16 10:10   ` Marc Kleine-Budde
  1 sibling, 1 reply; 19+ messages in thread
From: Geert Uytterhoeven @ 2021-07-16  7:47 UTC (permalink / raw)
  To: Lad Prabhakar
  Cc: Rob Herring, Wolfgang Grandegger, Marc Kleine-Budde,
	David S. Miller, Jakub Kicinski, Michael Turquette, Stephen Boyd,
	Philipp Zabel, linux-can, netdev,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-clk, Linux Kernel Mailing List, Linux-Renesas, Prabhakar,
	Biju Das

Hi Prabhakar,

On Thu, Jul 15, 2021 at 8:21 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> CANFD block on RZ/G2L SoC is almost identical to one found on
> R-Car Gen3 SoC's.
>
> On RZ/G2L SoC interrupt sources for each channel are split into
> different sources, irq handlers for the same are added.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -1070,6 +1077,56 @@ static void rcar_canfd_tx_done(struct net_device *ndev)
>         can_led_event(ndev, CAN_LED_EVENT_TX);
>  }
>
> +static irqreturn_t rcar_canfd_global_err_interrupt(int irq, void *dev_id)
> +{

> +static irqreturn_t rcar_canfd_global_recieve_fifo_interrupt(int irq, void *dev_id)
> +{

>  static irqreturn_t rcar_canfd_global_interrupt(int irq, void *dev_id)
>  {
>         struct rcar_canfd_global *gpriv = dev_id;
> @@ -1139,6 +1196,56 @@ static void rcar_canfd_state_change(struct net_device *ndev,
>         }
>  }
>
> +static irqreturn_t rcar_canfd_channel_tx_interrupt(int irq, void *dev_id)
> +{

> +static irqreturn_t rcar_canfd_channel_err_interrupt(int irq, void *dev_id)
> +{

It looks like the new split interrupt handlers duplicate code from
the existing unified interrupt handlers.  Perhaps the latter can be
made to call the former instead?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH 5/6] clk: renesas: r9a07g044-cpg: Add clock and reset entries for CANFD
  2021-07-15 18:21 ` [PATCH 5/6] clk: renesas: r9a07g044-cpg: Add clock and reset entries for CANFD Lad Prabhakar
@ 2021-07-16  7:55   ` Geert Uytterhoeven
  0 siblings, 0 replies; 19+ messages in thread
From: Geert Uytterhoeven @ 2021-07-16  7:55 UTC (permalink / raw)
  To: Lad Prabhakar
  Cc: Rob Herring, Wolfgang Grandegger, Marc Kleine-Budde,
	David S. Miller, Jakub Kicinski, Michael Turquette, Stephen Boyd,
	Philipp Zabel, linux-can, netdev,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-clk, Linux Kernel Mailing List, Linux-Renesas, Prabhakar,
	Biju Das

On Thu, Jul 15, 2021 at 8:21 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Add clock and reset entries for CANFD in CPG driver.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-clk-for-v5.15.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH 3/6] dt-bindings: clk: r9a07g044-cpg: Add entry for P0_DIV2 core clock
  2021-07-15 18:21 ` [PATCH 3/6] dt-bindings: clk: r9a07g044-cpg: Add entry for P0_DIV2 core clock Lad Prabhakar
@ 2021-07-16  8:07   ` Geert Uytterhoeven
  2021-07-16  8:45     ` Lad, Prabhakar
  0 siblings, 1 reply; 19+ messages in thread
From: Geert Uytterhoeven @ 2021-07-16  8:07 UTC (permalink / raw)
  To: Lad Prabhakar
  Cc: Rob Herring, Wolfgang Grandegger, Marc Kleine-Budde,
	David S. Miller, Jakub Kicinski, Michael Turquette, Stephen Boyd,
	Philipp Zabel, linux-can, netdev,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-clk, Linux Kernel Mailing List, Linux-Renesas, Prabhakar,
	Biju Das

Hi Prabhakar,

Thanks for your patch!

On Thu, Jul 15, 2021 at 8:21 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Add P0_DIV2 core clock required for CANFD module. CANFD core clock is
> sourced from P0_DIV2 referenced from HW manual Rev.0.50.

OK.

> Also add R9A07G044_LAST_CORE_CLK entry to avoid changes in
> r9a07g044-cpg.c file.

I'm not so fond of adding this.  Unlike the other definitions, it is
not really part of the bindings, but merely a convenience definition
for the driver.  Furthermore it has to change when a new definition
is ever added.

> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  include/dt-bindings/clock/r9a07g044-cpg.h | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/include/dt-bindings/clock/r9a07g044-cpg.h b/include/dt-bindings/clock/r9a07g044-cpg.h
> index 0728ad07ff7a..2fd20db0b2f4 100644
> --- a/include/dt-bindings/clock/r9a07g044-cpg.h
> +++ b/include/dt-bindings/clock/r9a07g044-cpg.h
> @@ -30,6 +30,8 @@
>  #define R9A07G044_CLK_P2               19
>  #define R9A07G044_CLK_AT               20
>  #define R9A07G044_OSCCLK               21
> +#define R9A07G044_CLK_P0_DIV2          22
> +#define R9A07G044_LAST_CORE_CLK                23

Third issue: off-by-one error, it should be 22 ;-)

>
>  /* R9A07G044 Module Clocks */
>  #define R9A07G044_CA55_SCLK            0

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH 4/6] clk: renesas: r9a07g044-cpg: Add entry for fixed clock P0_DIV2
  2021-07-15 18:21 ` [PATCH 4/6] clk: renesas: r9a07g044-cpg: Add entry for fixed clock P0_DIV2 Lad Prabhakar
@ 2021-07-16  8:09   ` Geert Uytterhoeven
  2021-07-16  8:46     ` Lad, Prabhakar
  0 siblings, 1 reply; 19+ messages in thread
From: Geert Uytterhoeven @ 2021-07-16  8:09 UTC (permalink / raw)
  To: Lad Prabhakar
  Cc: Rob Herring, Wolfgang Grandegger, Marc Kleine-Budde,
	David S. Miller, Jakub Kicinski, Michael Turquette, Stephen Boyd,
	Philipp Zabel, linux-can, netdev,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-clk, Linux Kernel Mailing List, Linux-Renesas, Prabhakar,
	Biju Das

Hi Prabhakar,

On Thu, Jul 15, 2021 at 8:21 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> Add entry for fixed core clock P0_DIV2 and assign LAST_DT_CORE_CLK
> to R9A07G044_LAST_CORE_CLK.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/clk/renesas/r9a07g044-cpg.c
> +++ b/drivers/clk/renesas/r9a07g044-cpg.c
> @@ -16,7 +16,7 @@
>
>  enum clk_ids {
>         /* Core Clock Outputs exported to DT */
> -       LAST_DT_CORE_CLK = R9A07G044_OSCCLK,
> +       LAST_DT_CORE_CLK = R9A07G044_LAST_CORE_CLK,

Please use R9A07G044_CLK_P0_DIV2 instead.

>
>         /* External Input Clocks */
>         CLK_EXTAL,
> @@ -77,6 +77,7 @@ static const struct cpg_core_clk r9a07g044_core_clks[] __initconst = {
>         DEF_FIXED("I", R9A07G044_CLK_I, CLK_PLL1, 1, 1),
>         DEF_DIV("P0", R9A07G044_CLK_P0, CLK_PLL2_DIV16, DIVPL2A,
>                 dtable_1_32, CLK_DIVIDER_HIWORD_MASK),
> +       DEF_FIXED("P0_DIV2", R9A07G044_CLK_P0_DIV2, R9A07G044_CLK_P0, 1, 2),
>         DEF_FIXED("TSU", R9A07G044_CLK_TSU, CLK_PLL2_DIV20, 1, 1),
>         DEF_DIV("P1", R9A07G044_CLK_P1, CLK_PLL3_DIV2_4,
>                 DIVPL3B, dtable_1_32, CLK_DIVIDER_HIWORD_MASK),

The rest looks good to me.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH 1/6] dt-bindings: net: can: renesas,rcar-canfd: Document RZ/G2L SoC
  2021-07-16  7:38   ` Geert Uytterhoeven
@ 2021-07-16  8:30     ` Lad, Prabhakar
  0 siblings, 0 replies; 19+ messages in thread
From: Lad, Prabhakar @ 2021-07-16  8:30 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Lad Prabhakar, Rob Herring, Wolfgang Grandegger,
	Marc Kleine-Budde, David S. Miller, Jakub Kicinski,
	Michael Turquette, Stephen Boyd, Philipp Zabel, linux-can,
	netdev,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-clk, Linux Kernel Mailing List, Linux-Renesas, Biju Das

Hi Geert,

Thank you for the review.

On Fri, Jul 16, 2021 at 8:38 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Thu, Jul 15, 2021 at 8:21 PM Lad Prabhakar
> <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > Add CANFD binding documentation for Renesas RZ/G2L SoC.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
>
> Thanks for your patch!
>
> > --- a/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
> > +++ b/Documentation/devicetree/bindings/net/can/renesas,rcar-canfd.yaml
>
> > @@ -78,6 +79,38 @@ patternProperties:
> >        node.  Each child node supports the "status" property only, which
> >        is used to enable/disable the respective channel.
> >
> > +if:
> > +  properties:
> > +    compatible:
> > +      contains:
> > +        enum:
> > +          - renesas,rzg2l-canfd
> > +then:
> > +  properties:
> > +    interrupts:
> > +      items:
> > +        - description: CAN global error interrupt
> > +        - description: CAN receive FIFO interrupt
> > +        - description: CAN0 error interrupt
> > +        - description: CAN0 transmit interrupt
> > +        - description: CAN0 transmit/receive FIFO receive completion interrupt
> > +        - description: CAN1 error interrupt
> > +        - description: CAN1 transmit interrupt
> > +        - description: CAN1 transmit/receive FIFO receive completion interrupt
>
> Does it make sense to add interrupt-names?
>
Agreed will drop this and add interrupt-names instead. Also I will
update the driver to pick up the interrupts based on names.

> > +
> > +    resets:
> > +      maxItems: 2
>
> Same here, for reset-names?
> Or a list of descriptions, so we know which reset serves what purpose.
>
OK I'll  add the reset-names.

Cheers,
Prabhakar

> > +
> > +else:
> > +  properties:
> > +    interrupts:
> > +      items:
> > +        - description: Channel interrupt
> > +        - description: Global interrupt
> > +
> > +    resets:
> > +      maxItems: 1
> > +
> >  required:
> >    - compatible
> >    - reg
>
> The rest looks good to me.
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

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

* Re: [PATCH 2/6] can: rcar_canfd: Add support for RZ/G2L family
  2021-07-16  7:47   ` Geert Uytterhoeven
@ 2021-07-16  8:32     ` Lad, Prabhakar
  0 siblings, 0 replies; 19+ messages in thread
From: Lad, Prabhakar @ 2021-07-16  8:32 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Lad Prabhakar, Rob Herring, Wolfgang Grandegger,
	Marc Kleine-Budde, David S. Miller, Jakub Kicinski,
	Michael Turquette, Stephen Boyd, Philipp Zabel, linux-can,
	netdev,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-clk, Linux Kernel Mailing List, Linux-Renesas, Biju Das

Hi Geert,

On Fri, Jul 16, 2021 at 8:47 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Thu, Jul 15, 2021 at 8:21 PM Lad Prabhakar
> <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > CANFD block on RZ/G2L SoC is almost identical to one found on
> > R-Car Gen3 SoC's.
> >
> > On RZ/G2L SoC interrupt sources for each channel are split into
> > different sources, irq handlers for the same are added.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
>
> Thanks for your patch!
>
> > --- a/drivers/net/can/rcar/rcar_canfd.c
> > +++ b/drivers/net/can/rcar/rcar_canfd.c
> > @@ -1070,6 +1077,56 @@ static void rcar_canfd_tx_done(struct net_device *ndev)
> >         can_led_event(ndev, CAN_LED_EVENT_TX);
> >  }
> >
> > +static irqreturn_t rcar_canfd_global_err_interrupt(int irq, void *dev_id)
> > +{
>
> > +static irqreturn_t rcar_canfd_global_recieve_fifo_interrupt(int irq, void *dev_id)
> > +{
>
> >  static irqreturn_t rcar_canfd_global_interrupt(int irq, void *dev_id)
> >  {
> >         struct rcar_canfd_global *gpriv = dev_id;
> > @@ -1139,6 +1196,56 @@ static void rcar_canfd_state_change(struct net_device *ndev,
> >         }
> >  }
> >
> > +static irqreturn_t rcar_canfd_channel_tx_interrupt(int irq, void *dev_id)
> > +{
>
> > +static irqreturn_t rcar_canfd_channel_err_interrupt(int irq, void *dev_id)
> > +{
>
> It looks like the new split interrupt handlers duplicate code from
> the existing unified interrupt handlers.  Perhaps the latter can be
> made to call the former instead?
>
Agreed.

Cheers,
Prabhakar

> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

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

* Re: [PATCH 3/6] dt-bindings: clk: r9a07g044-cpg: Add entry for P0_DIV2 core clock
  2021-07-16  8:07   ` Geert Uytterhoeven
@ 2021-07-16  8:45     ` Lad, Prabhakar
  2021-07-16  8:56       ` Geert Uytterhoeven
  0 siblings, 1 reply; 19+ messages in thread
From: Lad, Prabhakar @ 2021-07-16  8:45 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Lad Prabhakar, Rob Herring, Wolfgang Grandegger,
	Marc Kleine-Budde, David S. Miller, Jakub Kicinski,
	Michael Turquette, Stephen Boyd, Philipp Zabel, linux-can,
	netdev,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-clk, Linux Kernel Mailing List, Linux-Renesas, Biju Das

Hi Geert,

Thank you for the review.

On Fri, Jul 16, 2021 at 9:08 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> Thanks for your patch!
>
> On Thu, Jul 15, 2021 at 8:21 PM Lad Prabhakar
> <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > Add P0_DIV2 core clock required for CANFD module. CANFD core clock is
> > sourced from P0_DIV2 referenced from HW manual Rev.0.50.
>
> OK.
>
> > Also add R9A07G044_LAST_CORE_CLK entry to avoid changes in
> > r9a07g044-cpg.c file.
>
> I'm not so fond of adding this.  Unlike the other definitions, it is
> not really part of the bindings, but merely a convenience definition
> for the driver.  Furthermore it has to change when a new definition
> is ever added.
>
Agreed will drop this.

> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> >  include/dt-bindings/clock/r9a07g044-cpg.h | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/include/dt-bindings/clock/r9a07g044-cpg.h b/include/dt-bindings/clock/r9a07g044-cpg.h
> > index 0728ad07ff7a..2fd20db0b2f4 100644
> > --- a/include/dt-bindings/clock/r9a07g044-cpg.h
> > +++ b/include/dt-bindings/clock/r9a07g044-cpg.h
> > @@ -30,6 +30,8 @@
> >  #define R9A07G044_CLK_P2               19
> >  #define R9A07G044_CLK_AT               20
> >  #define R9A07G044_OSCCLK               21
> > +#define R9A07G044_CLK_P0_DIV2          22
> > +#define R9A07G044_LAST_CORE_CLK                23
>
> Third issue: off-by-one error, it should be 22 ;-)
>
23 was intentionally as these numbers aren't used for core clock count
we use r9a07g044_core_clks[] instead. Said that I'll drop this.

Cheers,
Prabhakar
> >
> >  /* R9A07G044 Module Clocks */
> >  #define R9A07G044_CA55_SCLK            0
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

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

* Re: [PATCH 4/6] clk: renesas: r9a07g044-cpg: Add entry for fixed clock P0_DIV2
  2021-07-16  8:09   ` Geert Uytterhoeven
@ 2021-07-16  8:46     ` Lad, Prabhakar
  0 siblings, 0 replies; 19+ messages in thread
From: Lad, Prabhakar @ 2021-07-16  8:46 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Lad Prabhakar, Rob Herring, Wolfgang Grandegger,
	Marc Kleine-Budde, David S. Miller, Jakub Kicinski,
	Michael Turquette, Stephen Boyd, Philipp Zabel, linux-can,
	netdev,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-clk, Linux Kernel Mailing List, Linux-Renesas, Biju Das

Hi Geert,

Thank you for the review.

On Fri, Jul 16, 2021 at 9:09 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Thu, Jul 15, 2021 at 8:21 PM Lad Prabhakar
> <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > Add entry for fixed core clock P0_DIV2 and assign LAST_DT_CORE_CLK
> > to R9A07G044_LAST_CORE_CLK.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
>
> Thanks for your patch!
>
> > --- a/drivers/clk/renesas/r9a07g044-cpg.c
> > +++ b/drivers/clk/renesas/r9a07g044-cpg.c
> > @@ -16,7 +16,7 @@
> >
> >  enum clk_ids {
> >         /* Core Clock Outputs exported to DT */
> > -       LAST_DT_CORE_CLK = R9A07G044_OSCCLK,
> > +       LAST_DT_CORE_CLK = R9A07G044_LAST_CORE_CLK,
>
> Please use R9A07G044_CLK_P0_DIV2 instead.
>
Ok, I will update it.

Cheers,
Prabhakar

> >
> >         /* External Input Clocks */
> >         CLK_EXTAL,
> > @@ -77,6 +77,7 @@ static const struct cpg_core_clk r9a07g044_core_clks[] __initconst = {
> >         DEF_FIXED("I", R9A07G044_CLK_I, CLK_PLL1, 1, 1),
> >         DEF_DIV("P0", R9A07G044_CLK_P0, CLK_PLL2_DIV16, DIVPL2A,
> >                 dtable_1_32, CLK_DIVIDER_HIWORD_MASK),
> > +       DEF_FIXED("P0_DIV2", R9A07G044_CLK_P0_DIV2, R9A07G044_CLK_P0, 1, 2),
> >         DEF_FIXED("TSU", R9A07G044_CLK_TSU, CLK_PLL2_DIV20, 1, 1),
> >         DEF_DIV("P1", R9A07G044_CLK_P1, CLK_PLL3_DIV2_4,
> >                 DIVPL3B, dtable_1_32, CLK_DIVIDER_HIWORD_MASK),
>
> The rest looks good to me.
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

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

* Re: [PATCH 3/6] dt-bindings: clk: r9a07g044-cpg: Add entry for P0_DIV2 core clock
  2021-07-16  8:45     ` Lad, Prabhakar
@ 2021-07-16  8:56       ` Geert Uytterhoeven
  2021-07-16  9:02         ` Lad, Prabhakar
  0 siblings, 1 reply; 19+ messages in thread
From: Geert Uytterhoeven @ 2021-07-16  8:56 UTC (permalink / raw)
  To: Lad, Prabhakar
  Cc: Lad Prabhakar, Rob Herring, Wolfgang Grandegger,
	Marc Kleine-Budde, David S. Miller, Jakub Kicinski,
	Michael Turquette, Stephen Boyd, Philipp Zabel, linux-can,
	netdev,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-clk, Linux Kernel Mailing List, Linux-Renesas, Biju Das

Hi Prabhakar,

On Fri, Jul 16, 2021 at 10:45 AM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
> On Fri, Jul 16, 2021 at 9:08 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Thu, Jul 15, 2021 at 8:21 PM Lad Prabhakar
> > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > > Add P0_DIV2 core clock required for CANFD module. CANFD core clock is
> > > sourced from P0_DIV2 referenced from HW manual Rev.0.50.
> >
> > OK.
> >
> > > Also add R9A07G044_LAST_CORE_CLK entry to avoid changes in
> > > r9a07g044-cpg.c file.
> >
> > I'm not so fond of adding this.  Unlike the other definitions, it is
> > not really part of the bindings, but merely a convenience definition
> > for the driver.  Furthermore it has to change when a new definition
> > is ever added.
> >
> Agreed will drop this.
>
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > ---
> > >  include/dt-bindings/clock/r9a07g044-cpg.h | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/include/dt-bindings/clock/r9a07g044-cpg.h b/include/dt-bindings/clock/r9a07g044-cpg.h
> > > index 0728ad07ff7a..2fd20db0b2f4 100644
> > > --- a/include/dt-bindings/clock/r9a07g044-cpg.h
> > > +++ b/include/dt-bindings/clock/r9a07g044-cpg.h
> > > @@ -30,6 +30,8 @@
> > >  #define R9A07G044_CLK_P2               19
> > >  #define R9A07G044_CLK_AT               20
> > >  #define R9A07G044_OSCCLK               21
> > > +#define R9A07G044_CLK_P0_DIV2          22
> > > +#define R9A07G044_LAST_CORE_CLK                23
> >
> > Third issue: off-by-one error, it should be 22 ;-)
> >
> 23 was intentionally as these numbers aren't used for core clock count
> we use r9a07g044_core_clks[] instead.

It ends up as an off-by-one bug in the range check in
rzg2l_cpg_clk_src_twocell_get().

> Said that I'll drop this.

OK.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH 3/6] dt-bindings: clk: r9a07g044-cpg: Add entry for P0_DIV2 core clock
  2021-07-16  8:56       ` Geert Uytterhoeven
@ 2021-07-16  9:02         ` Lad, Prabhakar
  0 siblings, 0 replies; 19+ messages in thread
From: Lad, Prabhakar @ 2021-07-16  9:02 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Lad Prabhakar, Rob Herring, Wolfgang Grandegger,
	Marc Kleine-Budde, David S. Miller, Jakub Kicinski,
	Michael Turquette, Stephen Boyd, Philipp Zabel, linux-can,
	netdev,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-clk, Linux Kernel Mailing List, Linux-Renesas, Biju Das

Hi Geert,

On Fri, Jul 16, 2021 at 9:56 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Fri, Jul 16, 2021 at 10:45 AM Lad, Prabhakar
> <prabhakar.csengg@gmail.com> wrote:
> > On Fri, Jul 16, 2021 at 9:08 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Thu, Jul 15, 2021 at 8:21 PM Lad Prabhakar
> > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> > > > Add P0_DIV2 core clock required for CANFD module. CANFD core clock is
> > > > sourced from P0_DIV2 referenced from HW manual Rev.0.50.
> > >
> > > OK.
> > >
> > > > Also add R9A07G044_LAST_CORE_CLK entry to avoid changes in
> > > > r9a07g044-cpg.c file.
> > >
> > > I'm not so fond of adding this.  Unlike the other definitions, it is
> > > not really part of the bindings, but merely a convenience definition
> > > for the driver.  Furthermore it has to change when a new definition
> > > is ever added.
> > >
> > Agreed will drop this.
> >
> > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > ---
> > > >  include/dt-bindings/clock/r9a07g044-cpg.h | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/include/dt-bindings/clock/r9a07g044-cpg.h b/include/dt-bindings/clock/r9a07g044-cpg.h
> > > > index 0728ad07ff7a..2fd20db0b2f4 100644
> > > > --- a/include/dt-bindings/clock/r9a07g044-cpg.h
> > > > +++ b/include/dt-bindings/clock/r9a07g044-cpg.h
> > > > @@ -30,6 +30,8 @@
> > > >  #define R9A07G044_CLK_P2               19
> > > >  #define R9A07G044_CLK_AT               20
> > > >  #define R9A07G044_OSCCLK               21
> > > > +#define R9A07G044_CLK_P0_DIV2          22
> > > > +#define R9A07G044_LAST_CORE_CLK                23
> > >
> > > Third issue: off-by-one error, it should be 22 ;-)
> > >
> > 23 was intentionally as these numbers aren't used for core clock count
> > we use r9a07g044_core_clks[] instead.
>
> It ends up as an off-by-one bug in the range check in
> rzg2l_cpg_clk_src_twocell_get().
>
Ooops missed that!

Cheers,
Prabhakar

> > Said that I'll drop this.
>
> OK.
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

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

* Re: [PATCH 2/6] can: rcar_canfd: Add support for RZ/G2L family
  2021-07-15 18:21 ` [PATCH 2/6] can: rcar_canfd: Add support for RZ/G2L family Lad Prabhakar
  2021-07-16  7:47   ` Geert Uytterhoeven
@ 2021-07-16 10:10   ` Marc Kleine-Budde
  1 sibling, 0 replies; 19+ messages in thread
From: Marc Kleine-Budde @ 2021-07-16 10:10 UTC (permalink / raw)
  To: Lad Prabhakar
  Cc: Geert Uytterhoeven, Rob Herring, Wolfgang Grandegger,
	David S. Miller, Jakub Kicinski, Michael Turquette, Stephen Boyd,
	Philipp Zabel, linux-can, netdev, devicetree, linux-clk,
	linux-kernel, linux-renesas-soc, Prabhakar, Biju Das

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

On 15.07.2021 19:21:19, Lad Prabhakar wrote:
> CANFD block on RZ/G2L SoC is almost identical to one found on
> R-Car Gen3 SoC's.
> 
> On RZ/G2L SoC interrupt sources for each channel are split into
> different sources, irq handlers for the same are added.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for the patch! Some nitpicks inline, Geert already commented to
use the same IRQ handler for all interrutps.

> ---
>  drivers/net/can/rcar/rcar_canfd.c | 275 ++++++++++++++++++++++++++----
>  1 file changed, 244 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
> index 311e6ca3bdc4..5dfbc5fa2d81 100644
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -37,9 +37,13 @@
>  #include <linux/bitmap.h>
>  #include <linux/bitops.h>
>  #include <linux/iopoll.h>
> +#include <linux/reset.h>
>  
>  #define RCANFD_DRV_NAME			"rcar_canfd"
>  
> +#define RENESAS_RCAR_GEN3	0
> +#define RENESAS_RZG2L		1
> +

Please make this an enum.

>  /* Global register bits */
>  
>  /* RSCFDnCFDGRMCFG */
> @@ -513,6 +517,9 @@ struct rcar_canfd_global {
>  	enum rcar_canfd_fcanclk fcan;	/* CANFD or Ext clock */
>  	unsigned long channels_mask;	/* Enabled channels mask */
>  	bool fdmode;			/* CAN FD or Classical CAN only mode */
> +	struct reset_control *rstc1;     /* Pointer to reset source1 */
> +	struct reset_control *rstc2;     /* Pointer to reset source2 */
> +	unsigned int chip_id;

enum here, too

>  };
>  
>  /* CAN FD mode nominal rate constants */
> @@ -1070,6 +1077,56 @@ static void rcar_canfd_tx_done(struct net_device *ndev)
>  	can_led_event(ndev, CAN_LED_EVENT_TX);
>  }
>  
[...]

> @@ -1635,8 +1784,11 @@ static int rcar_canfd_probe(struct platform_device *pdev)
>  	struct rcar_canfd_global *gpriv;
>  	struct device_node *of_child;
>  	unsigned long channels_mask = 0;
> -	int err, ch_irq, g_irq;
> +	int err, ch_irq, g_irq, g_rx_irq;
>  	bool fdmode = true;			/* CAN FD only mode - default */
> +	unsigned int chip_id;
> +
> +	chip_id = (uintptr_t)of_device_get_match_data(&pdev->dev);

The cast looks wrong.

>  
>  	if (of_property_read_bool(pdev->dev.of_node, "renesas,no-can-fd"))
>  		fdmode = false;			/* Classical CAN only mode */
> @@ -1649,27 +1801,56 @@ static int rcar_canfd_probe(struct platform_device *pdev)
>  	if (of_child && of_device_is_available(of_child))
>  		channels_mask |= BIT(1);	/* Channel 1 */
>  
> -	ch_irq = platform_get_irq(pdev, 0);
> -	if (ch_irq < 0) {
> -		err = ch_irq;
> -		goto fail_dev;
> -	}
> +	if (chip_id == RENESAS_RCAR_GEN3) {
> +		ch_irq = platform_get_irq(pdev, 0);
> +		if (ch_irq < 0)
> +			return ch_irq;
>  
> -	g_irq = platform_get_irq(pdev, 1);
> -	if (g_irq < 0) {
> -		err = g_irq;
> -		goto fail_dev;
> +		g_irq = platform_get_irq(pdev, 1);
> +		if (g_irq < 0)
> +			return g_irq;
> +	} else {
> +		g_irq = platform_get_irq(pdev, 0);
> +		if (g_irq < 0)
> +			return g_irq;
> +
> +		g_rx_irq = platform_get_irq(pdev, 1);
> +		if (g_rx_irq < 0)
> +			return g_rx_irq;
>  	}
>  
>  	/* Global controller context */
>  	gpriv = devm_kzalloc(&pdev->dev, sizeof(*gpriv), GFP_KERNEL);
> -	if (!gpriv) {
> -		err = -ENOMEM;
> -		goto fail_dev;
> -	}
> +	if (!gpriv)
> +		return -ENOMEM;
> +
>  	gpriv->pdev = pdev;
>  	gpriv->channels_mask = channels_mask;
>  	gpriv->fdmode = fdmode;
> +	gpriv->chip_id = chip_id;
> +
> +	if (gpriv->chip_id == RENESAS_RZG2L) {
> +		gpriv->rstc1 = devm_reset_control_get_exclusive_by_index(&pdev->dev, 0);
> +		if (IS_ERR(gpriv->rstc1)) {
> +			dev_err(&pdev->dev, "failed to get reset index 0\n");
> +			return PTR_ERR(gpriv->rstc1);
> +		}
> +
> +		err = reset_control_reset(gpriv->rstc1);
> +		if (err)
> +			return err;
> +
> +		gpriv->rstc2 = devm_reset_control_get_exclusive_by_index(&pdev->dev, 1);
> +		if (IS_ERR(gpriv->rstc2)) {
> +			dev_err(&pdev->dev, "failed to get reset index 1\n");
> +			return PTR_ERR(gpriv->rstc2);
> +		}
> +		err = reset_control_reset(gpriv->rstc2);
> +		if (err) {
> +			reset_control_assert(gpriv->rstc1);
> +			return err;
> +		}
> +	}
>  
>  	/* Peripheral clock */
>  	gpriv->clkp = devm_clk_get(&pdev->dev, "fck");
> @@ -1699,7 +1880,7 @@ static int rcar_canfd_probe(struct platform_device *pdev)
>  	}
>  	fcan_freq = clk_get_rate(gpriv->can_clk);
>  
> -	if (gpriv->fcan == RCANFD_CANFDCLK)
> +	if (gpriv->fcan == RCANFD_CANFDCLK && gpriv->chip_id == RENESAS_RCAR_GEN3)
>  		/* CANFD clock is further divided by (1/2) within the IP */
>  		fcan_freq /= 2;
>  
> @@ -1711,21 +1892,43 @@ static int rcar_canfd_probe(struct platform_device *pdev)
>  	gpriv->base = addr;
>  
>  	/* Request IRQ that's common for both channels */
> -	err = devm_request_irq(&pdev->dev, ch_irq,
> -			       rcar_canfd_channel_interrupt, 0,
> -			       "canfd.chn", gpriv);
> -	if (err) {
> -		dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n",
> -			ch_irq, err);
> -		goto fail_dev;
> -	}
> -	err = devm_request_irq(&pdev->dev, g_irq,
> -			       rcar_canfd_global_interrupt, 0,
> -			       "canfd.gbl", gpriv);
> -	if (err) {
> -		dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n",
> -			g_irq, err);
> -		goto fail_dev;
> +	if (gpriv->chip_id == RENESAS_RCAR_GEN3) {
> +		err = devm_request_irq(&pdev->dev, ch_irq,
> +				       rcar_canfd_channel_interrupt, 0,
> +				       "canfd.chn", gpriv);
> +		if (err) {
> +			dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n",
> +				ch_irq, err);
> +			goto fail_dev;
> +		}
> +
> +		err = devm_request_irq(&pdev->dev, g_irq,
> +				       rcar_canfd_global_interrupt, 0,
> +				       "canfd.gbl", gpriv);
> +		if (err) {
> +			dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n",
> +				g_irq, err);
> +			goto fail_dev;
> +		}
> +	} else {
> +		err = devm_request_irq(&pdev->dev, g_rx_irq,
> +				       rcar_canfd_global_recieve_fifo_interrupt, 0,
> +				       "canfd.gblrx", gpriv);
> +
> +		if (err) {
> +			dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n",
> +				g_rx_irq, err);
> +			goto fail_dev;
> +		}
> +
> +		err = devm_request_irq(&pdev->dev, g_irq,
> +				       rcar_canfd_global_err_interrupt, 0,
> +				       "canfd.gblerr", gpriv);
> +		if (err) {
> +			dev_err(&pdev->dev, "devm_request_irq(%d) failed, error %d\n",
> +				g_irq, err);
> +			goto fail_dev;
> +		}
>  	}
>  
>  	/* Enable peripheral clock for register access */
> @@ -1791,6 +1994,10 @@ static int rcar_canfd_probe(struct platform_device *pdev)
>  fail_clk:
>  	clk_disable_unprepare(gpriv->clkp);
>  fail_dev:
> +	if (gpriv->chip_id == RENESAS_RZG2L) {
> +		reset_control_assert(gpriv->rstc1);
> +		reset_control_assert(gpriv->rstc2);

reset_control_assert() can handle NULL pointers

> +	}
>  	return err;
>  }
>  
> @@ -1810,6 +2017,11 @@ static int rcar_canfd_remove(struct platform_device *pdev)
>  	/* Enter global sleep mode */
>  	rcar_canfd_set_bit(gpriv->base, RCANFD_GCTR, RCANFD_GCTR_GSLPR);
>  	clk_disable_unprepare(gpriv->clkp);
> +	if (gpriv->chip_id == RENESAS_RZG2L) {
> +		reset_control_assert(gpriv->rstc1);
> +		reset_control_assert(gpriv->rstc2);
> +	}

same here

> +
>  	return 0;
>  }
>  
> @@ -1827,7 +2039,8 @@ static SIMPLE_DEV_PM_OPS(rcar_canfd_pm_ops, rcar_canfd_suspend,
>  			 rcar_canfd_resume);
>  
>  static const struct of_device_id rcar_canfd_of_table[] = {
> -	{ .compatible = "renesas,rcar-gen3-canfd" },
> +	{ .compatible = "renesas,rcar-gen3-canfd", .data = (void *)RENESAS_RCAR_GEN3 },
> +	{ .compatible = "renesas,rzg2l-canfd", .data = (void *)RENESAS_RZG2L },
>  	{ }
>  };

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

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

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

end of thread, other threads:[~2021-07-16 10:10 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-15 18:21 [PATCH 0/6] Renesas RZ/G2L CANFD support Lad Prabhakar
2021-07-15 18:21 ` [PATCH 1/6] dt-bindings: net: can: renesas,rcar-canfd: Document RZ/G2L SoC Lad Prabhakar
2021-07-16  7:38   ` Geert Uytterhoeven
2021-07-16  8:30     ` Lad, Prabhakar
2021-07-15 18:21 ` [PATCH 2/6] can: rcar_canfd: Add support for RZ/G2L family Lad Prabhakar
2021-07-16  7:47   ` Geert Uytterhoeven
2021-07-16  8:32     ` Lad, Prabhakar
2021-07-16 10:10   ` Marc Kleine-Budde
2021-07-15 18:21 ` [PATCH 3/6] dt-bindings: clk: r9a07g044-cpg: Add entry for P0_DIV2 core clock Lad Prabhakar
2021-07-16  8:07   ` Geert Uytterhoeven
2021-07-16  8:45     ` Lad, Prabhakar
2021-07-16  8:56       ` Geert Uytterhoeven
2021-07-16  9:02         ` Lad, Prabhakar
2021-07-15 18:21 ` [PATCH 4/6] clk: renesas: r9a07g044-cpg: Add entry for fixed clock P0_DIV2 Lad Prabhakar
2021-07-16  8:09   ` Geert Uytterhoeven
2021-07-16  8:46     ` Lad, Prabhakar
2021-07-15 18:21 ` [PATCH 5/6] clk: renesas: r9a07g044-cpg: Add clock and reset entries for CANFD Lad Prabhakar
2021-07-16  7:55   ` Geert Uytterhoeven
2021-07-15 18:21 ` [PATCH 6/6] arm64: dts: renesas: r9a07g044: Add CANFD node Lad Prabhakar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.