linux-kernel - RE: [PATCH] can: flexcan: disable transceiver during system PM

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID:
 <DU0PR04MB94963271989728FA82C5A25D90C82@DU0PR04MB9496.eurprd04.prod.outlook.com>
Date: Tue, 4 Mar 2025 04:24:57 +0000
From: Bough Chen <haibo.chen@....com>
To: Vincent Mailhol <mailhol.vincent@...adoo.fr>, "mkl@...gutronix.de"
	<mkl@...gutronix.de>
CC: "Ciprian Marian Costea (OSS)" <ciprianmarian.costea@....nxp.com>,
	"u.kleine-koenig@...libre.com" <u.kleine-koenig@...libre.com>,
	"linux-can@...r.kernel.org" <linux-can@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"imx@...ts.linux.dev" <imx@...ts.linux.dev>
Subject: RE: [PATCH] can: flexcan: disable transceiver during system PM

> -----Original Message-----
> From: Vincent Mailhol <mailhol.vincent@...adoo.fr>
> Sent: 2025年3月4日 12:06
> To: Bough Chen <haibo.chen@....com>; mkl@...gutronix.de
> Cc: Ciprian Marian Costea (OSS) <ciprianmarian.costea@....nxp.com>;
> u.kleine-koenig@...libre.com; linux-can@...r.kernel.org;
> linux-kernel@...r.kernel.org; imx@...ts.linux.dev
> Subject: Re: [PATCH] can: flexcan: disable transceiver during system PM
> 
> On 04/03/2025 at 12:37, haibo.chen@....com wrote:
> > From: Haibo Chen <haibo.chen@....com>
> >
> > During system PM, if no wakeup requirement, disable transceiver to
> > save power.
> >
> > Signed-off-by: Haibo Chen <haibo.chen@....com>> ---
> > drivers/net/can/flexcan/flexcan-core.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/net/can/flexcan/flexcan-core.c
> > b/drivers/net/can/flexcan/flexcan-core.c
> > index b347a1c93536..74f3137295fb 100644
> > --- a/drivers/net/can/flexcan/flexcan-core.c
> > +++ b/drivers/net/can/flexcan/flexcan-core.c
> > @@ -2292,6 +2292,7 @@ static int __maybe_unused flexcan_suspend(struct
> device *device)
> >  				return err;
> >
> >  			flexcan_chip_interrupts_disable(dev);
> > +			flexcan_transceiver_disable(priv);
> 
> flexcan_transceiver_disable() may return an error. Can you check its return
> value?

Yes, sure.

> 
> >  			err = pinctrl_pm_select_sleep_state(device);
> >  			if (err)
> > @@ -2325,6 +2326,10 @@ static int __maybe_unused flexcan_resume(struct
> device *device)
> >  			if (err)
> >  				return err;
> >
> > +			err = flexcan_transceiver_enable(priv);
> > +			if (err)
> > +				return err;
> > +
> >  			err = flexcan_chip_start(dev);
> >  			if (err)
> >  				return err;
> 
> If flexcan_chip_start() fails, shouldn't you have a clean-up branch that will
> flexcan_transceiver_disable()? Or do you consider that it is acceptable to keep
> the transceiver enabled if an error occurs?

Good suggestion, I will add a clean-up branch.

> 
> Speaking of which, if flexcan_resume() fails, priv->can.state would still be set to
> CAN_STATE_ERROR_ACTIVE.

Thanks for your quick response, I will take all these suggestions into consideration and prepare v2 patch.

Regards
Haibo Chen
> 
> 
> Yours sincerely,
> Vincent Mailhol