Hi Geert,
> -----Original Message-----
> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Sent: 12 September 2025 09:27
> Subject: Re: [PATCH/RFC 6/6] can: rcar_canfd: Add suspend/resume support
>
> Hi Biju,
>
> On Fri, 12 Sept 2025 at 09:54, Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > > -----Original Message-----
> > > From: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> > > Sent: 22 August 2025 10:51
> > > Subject: [PATCH/RFC 6/6] can: rcar_canfd: Add suspend/resume support
> > >
> > > On R-Car Gen3 using PSCI, s2ram powers down the SoC. After resume,
> > > the CAN-FD interface no longer works. Trying to bring it up again fails:
> > >
> > > # ip link set can0 up
> > > RTNETLINK answers: Connection timed out
> > >
> > > # dmesg
> > > ...
> > > channel 0 communication state failed
> > >
> > > Fix this by populating the (currently empty) suspend and resume
> > > callbacks, to stop/start the individual CAN-FD channels, and (de)initialize the CAN-FD controller.
> > >
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> >
> > Tested-by: Reviewed-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> >
> > With adaption to RZ/G3E for ram_clk [1]
>
> Thanks!
>
> > > --- a/drivers/net/can/rcar/rcar_canfd.c
> > > +++ b/drivers/net/can/rcar/rcar_canfd.c
>
> > >
> > > static int rcar_canfd_resume(struct device *dev) {
> > > + struct rcar_canfd_global *gpriv = dev_get_drvdata(dev);
> > > + int err;
> > > + u32 ch;
> > > +
> > > + err = rcar_canfd_global_init(gpriv);
> > > + if (err) {
> > > + dev_err(dev, "rcar_canfd_open() failed %pe\n",
> > > + ERR_PTR(err));
> >
> > Typo: rcar_canfd_global_init
>
> Oops...
>
> > [1]
> >
> > biju@biju:~/linux-work/linux$ git diff diff --git
> > a/drivers/net/can/rcar/rcar_canfd.c
> > b/drivers/net/can/rcar/rcar_canfd.c
> > index 57ac90e57f11..cb358a4e49d0 100644
> > --- a/drivers/net/can/rcar/rcar_canfd.c
> > +++ b/drivers/net/can/rcar/rcar_canfd.c
> > @@ -468,6 +468,7 @@ struct rcar_canfd_global {
> > struct platform_device *pdev; /* Respective platform device */
> > struct clk *clkp; /* Peripheral clock */
> > struct clk *can_clk; /* fCAN clock */
> > + struct clk *clk_ram; /* Clock RAM */
> > unsigned long channels_mask; /* Enabled channels mask */
> > bool extclk; /* CANFD or Ext clock */
> > bool fdmode; /* CAN FD or Classical CAN only mode */
> > @@ -1983,10 +1984,18 @@ static int rcar_canfd_global_init(struct rcar_canfd_global *gpriv)
> > goto fail_reset2;
> > }
> >
> > + /* Enable RAM clock */
> > + err = clk_prepare_enable(gpriv->clk_ram);
> > + if (err) {
> > + dev_err(dev,
> > + "failed to enable ram clock, error %d\n", err);
> > + goto fail_clk;
> > + }
> > +
> > err = rcar_canfd_reset_controller(gpriv);
> > if (err) {
> > dev_err(dev, "reset controller failed: %pe\n", ERR_PTR(err));
> > - goto fail_clk;
> > + goto fail_ram_clk;
> > }
> >
> > /* Controller in Global reset & Channel reset mode */ @@
> > -2026,6 +2035,8 @@ static int rcar_canfd_global_init(struct rcar_canfd_global *gpriv)
> > rcar_canfd_disable_global_interrupts(gpriv);
> > fail_clk:
> > clk_disable_unprepare(gpriv->clkp);
> > +fail_ram_clk:
> > + clk_disable_unprepare(gpriv->clk_ram);
> > fail_reset2:
> > reset_control_assert(gpriv->rstc2);
> > fail_reset1:
> > @@ -2045,6 +2056,7 @@ static void rcar_canfd_global_deinit(struct rcar_canfd_global *gpriv, bool
> full)
> > }
> >
> > clk_disable_unprepare(gpriv->clkp);
> > + clk_disable_unprepare(gpriv->clk_ram);
> > reset_control_assert(gpriv->rstc2);
> > reset_control_assert(gpriv->rstc1);
> > }
> > @@ -2153,10 +2165,10 @@ static int rcar_canfd_probe(struct platform_device *pdev)
> > gpriv->extclk = gpriv->info->external_clk;
> > }
> >
> > - clk_ram = devm_clk_get_optional_enabled(dev, "ram_clk");
> > + gpriv->clk_ram = devm_clk_get_optional(dev, "ram_clk");
> > if (IS_ERR(clk_ram))
> > return dev_err_probe(dev, PTR_ERR(clk_ram),
> > - "cannot get enabled ram clock\n");
> > + "cannot get ram clock\n");
>
> I guess this should be folded into "[PATCH 3/6] can: rcar_canfd:
> Extract rcar_canfd_global_{,de}init()", or even better, inserted as a new patch before that?
I agree this has to be a new patch before this. I will send next series based on this.
Cheers,
Biju
