Thread (20 messages) 20 messages, 4 authors, 2021-07-20

Re: [PATCH v2 2/5] can: rcar_canfd: Add support for RZ/G2L family

From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
Date: 2021-07-20 15:12:48
Also in: linux-can, linux-clk, linux-devicetree, linux-renesas-soc, lkml

Hi Philipp,

Thank you for the review.

On Tue, Jul 20, 2021 at 11:23 AM Philipp Zabel [off-list ref] wrote:
On Mon, 2021-07-19 at 15:38 +0100, Lad Prabhakar wrote:
quoted
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 and the IP doesn't divide (1/2)
CANFD clock within the IP.

This patch adds compatible string for RZ/G2L family and registers
the irq handlers required for CANFD operation. IRQ numbers are now
fetched based on names instead of indices. For backward compatibility
on non RZ/G2L SoC's we fallback reading based on indices.

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 | 178 ++++++++++++++++++++++++------
 1 file changed, 147 insertions(+), 31 deletions(-)
diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index 311e6ca3bdc4..d4affc002fb3 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -37,9 +37,15 @@
[...]
quoted
+     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");
Please consider requesting the reset controls by name instead of by
index. See also my reply to the binding patch.
Will do.
quoted
+                     return PTR_ERR(gpriv->rstc1);
+             }
+
+             err = reset_control_reset(gpriv->rstc1);
+             if (err)
+                     return err;
I suggest to wait until after all resource requests have succeeded
before triggering the resets, i.e. first get all reset controls and
clocks, etc., and only then trigger resets, enable clocks, and so on.

That way there will be no spurious resets in case of probe deferrals.
Agreed, will update the code.

Cheers,
Prabhakar
regards
Philipp
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help