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] [thread-next>] [day] [month] [year] [list]

Message-ID: <01b6fbe7-6e49-4ac2-af2f-a512ae865eae@wanadoo.fr>
Date: Tue, 4 Mar 2025 13:06:10 +0900
From: Vincent Mailhol <mailhol.vincent@...adoo.fr>
To: haibo.chen@....com, mkl@...gutronix.de
Cc: 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?

>  			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?

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


Yours sincerely,
Vincent Mailhol