From: 悬溺 <2063309626@qq.com>
To: "Marc Kleine-Budde" <mkl@pengutronix.de>
Cc: "Vincent Mailhol" <mailhol@kernel.org>,
"Jakub Kicinski" <kuba@kernel.org>,
"Bartosz Golaszewski" <brgl@kernel.org>,
"Maud Spierings" <maudspierings@gocontroll.com>,
"Marco Crivellari" <marco.crivellari@suse.com>,
"Alban Bedel" <alban.bedel@lht.dlh.de>,
gszhai <gszhai@bjtu.edu.cn>, 25125332 <25125332@bjtu.edu.cn>,
25125283 <25125283@bjtu.edu.cn>, 23120469 <23120469@bjtu.edu.cn>,
linux-can <linux-can@vger.kernel.org>,
linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH V2] can: mcp251x: add error handling for power enable inopen and resume
Date: Thu, 12 Mar 2026 20:48:43 +0800 [thread overview]
Message-ID: <tencent_9D40B736A2D06AF745F473AAC55B45561106@qq.com> (raw)
In-Reply-To: <20260312-peridot-galago-of-painting-7d6e1c-mkl@pengutronix.de>
[-- Attachment #1.1: Type: text/plain, Size: 9849 bytes --]
Add missing error handling for mcp251x_power_enable() calls in both
mcp251x_open() and mcp251x_can_resume() functions.
In mcp251x_open(), if power enable fails, jump to error path to close
candev without attempting to disable power again.
In mcp251x_can_resume(), properly check return values of power enable
calls for both power and transceiver regulators. If any fails, return
the error code to the PM framework and log the failure.
This ensures the driver properly handles power control failures and
maintains correct device state.
Signed-off-by: Wenyuan Li <2063309626@qq.com>
---
drivers/net/can/spi/mcp251x.c | 30 ++++++++++++++++++++++++------
1 file changed, 24 insertions(+), 6 deletions(-)
diff --git a/drivers/net/can/spi/mcp251x.c b/drivers/net/can/spi/mcp251x.c
index bb7782582f40..0bc208814758 100644
--- a/drivers/net/can/spi/mcp251x.c
+++ b/drivers/net/can/spi/mcp251x.c
@@ -1225,7 +1225,11 @@ static int mcp251x_open(struct net_device *net)
}
mutex_lock(&priv->mcp_lock);
- mcp251x_power_enable(priv->transceiver, 1);
+ ret = mcp251x_power_enable(priv->transceiver, 1);
+ if (ret) {
+ dev_err(&spi->dev, "failed to enable power: %d\n", ret);
+ goto out_close_candev;
+ }
priv->force_quit = 0;
priv->tx_skb = NULL;
@@ -1272,6 +1276,7 @@ static int mcp251x_open(struct net_device *net)
mcp251x_hw_sleep(spi);
out_close:
mcp251x_power_enable(priv->transceiver, 0);
+out_close_candev:
close_candev(net);
mutex_unlock(&priv->mcp_lock);
if (release_irq)
@@ -1516,11 +1521,23 @@ static int __maybe_unused mcp251x_can_resume(struct device *dev)
{
struct spi_device *spi = to_spi_device(dev);
struct mcp251x_priv *priv = spi_get_drvdata(spi);
+ int ret = 0;
- if (priv->after_suspend & AFTER_SUSPEND_POWER)
- mcp251x_power_enable(priv->power, 1);
- if (priv->after_suspend & AFTER_SUSPEND_UP)
- mcp251x_power_enable(priv->transceiver, 1);
+ if (priv->after_suspend & AFTER_SUSPEND_POWER) {
+ ret = mcp251x_power_enable(priv->power, 1);
+ if (ret) {
+ dev_err(&spi->dev, "failed to enable power: %d\n", ret);
+ goto out;
+ }
+ }
+
+ if (priv->after_suspend & AFTER_SUSPEND_UP) {
+ ret = mcp251x_power_enable(priv->transceiver, 1);
+ if (ret) {
+ dev_err(&spi->dev, "failed to enable transceiver: %d\n", ret);
+ goto out;
+ }
+ }
if (priv->after_suspend & (AFTER_SUSPEND_POWER | AFTER_SUSPEND_UP))
queue_work(priv->wq, &priv->restart_work);
@@ -1529,7 +1546,8 @@ static int __maybe_unused mcp251x_can_resume(struct device *dev)
priv->force_quit = 0;
enable_irq(spi->irq);
- return 0;
+out:
+ return ret;
}
static SIMPLE_DEV_PM_OPS(mcp251x_can_pm_ops, mcp251x_can_suspend,
--
2.43.0
发自我的iPhone
------------------ Original ------------------
From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Thu,Mar 12,2026 3:46 PM
To: Wenyuan Li <2063309626@qq.com>
Cc: Vincent Mailhol <mailhol@kernel.org>, Jakub Kicinski <kuba@kernel.org>, Bartosz Golaszewski <brgl@kernel.org>, Maud Spierings <maudspierings@gocontroll.com>, Marco Crivellari <marco.crivellari@suse.com>, Alban Bedel <alban.bedel@lht.dlh.de>, gszhai <gszhai@bjtu.edu.cn>, 25125332 <25125332@bjtu.edu.cn>, 25125283 <25125283@bjtu.edu.cn>, 23120469 <23120469@bjtu.edu.cn>, linux-can <linux-can@vger.kernel.org>, linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] can: mcp251x: add error handling for power enable inopen and resume
On 11.03.2026 23:50:53, Wenyuan Li wrote:
> Add missing error handling for mcp251x_power_enable() calls in both
> mcp251x_open() and mcp251x_can_resume() functions.
>
> In mcp251x_open(), if power enable fails, the driver should not continue
> with device initialization. Add proper error checking and jump to
> existing out_close label.
>
> In mcp251x_can_resume(), if power enable fails during system resume,
> propagate the error to PM framework and log the error with dev_err()
> for debugging.
>
> This ensures the driver properly handles power control failures and
> maintains correct device state.
> Signed-off-by: Wenyuan Li <2063309626@qq.com>
> ---
> drivers/net/can/spi/mcp251x.c | 28 +++++++++++++++++++++++-----
> 1 file changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/can/spi/mcp251x.c b/drivers/net/can/spi/mcp251x.c
> index bb7782582f40..807293a7857f 100644
> --- a/drivers/net/can/spi/mcp251x.c
> +++ b/drivers/net/can/spi/mcp251x.c
> @@ -1225,7 +1225,11 @@ static int mcp251x_open(struct net_device *net)
> }
>
> mutex_lock(&priv->mcp_lock);
> - mcp251x_power_enable(priv->transceiver, 1);
> + ret = mcp251x_power_enable(priv->transceiver, 1);
> + if (ret) {
> + dev_err(&spi->dev, "failed to enable power: %d\n", ret);
> + goto out_close_candev;
> + }
>
> priv->force_quit = 0;
> priv->tx_skb = NULL;
> @@ -1272,6 +1276,7 @@ static int mcp251x_open(struct net_device *net)
> mcp251x_hw_sleep(spi);
> out_close:
> mcp251x_power_enable(priv->transceiver, 0);
> +out_close_candev:
> close_candev(net);
> mutex_unlock(&priv->mcp_lock);
> if (release_irq)
> @@ -1516,11 +1521,23 @@ static int __maybe_unused mcp251x_can_resume(struct device *dev)
> {
> struct spi_device *spi = to_spi_device(dev);
> struct mcp251x_priv *priv = spi_get_drvdata(spi);
> + int ret = 0;
>
> - if (priv->after_suspend & AFTER_SUSPEND_POWER)
> - mcp251x_power_enable(priv->power, 1);
> - if (priv->after_suspend & AFTER_SUSPEND_UP)
> - mcp251x_power_enable(priv->transceiver, 1);
^^^^^^^^^^^
The original code enables the "transceiver"
> + if (priv->after_suspend & AFTER_SUSPEND_POWER) {
> + ret = mcp251x_power_enable(priv->power, 1);
> + if (ret) {
> + dev_err(dev, "Failed to restore power: %d\n", ret);
> + goto out;
> + }
> + }
> +
> + if (priv->after_suspend & AFTER_SUSPEND_UP) {
> + ret = mcp251x_power_enable(priv->power, 1);
^^^^^
You enable the "power".
> + if (ret) {
> + dev_err(dev, "Failed to restore power: %d\n", ret);
> + goto out;
> + }
> + }
>
> if (priv->after_suspend & (AFTER_SUSPEND_POWER | AFTER_SUSPEND_UP))
> queue_work(priv->wq, &priv->restart_work);
> @@ -1529,6 +1546,7 @@ static int __maybe_unused mcp251x_can_resume(struct device *dev)
>
> priv->force_quit = 0;
> enable_irq(spi->irq);
> +out:
> return 0;
Do you want to return "ret" instead of "0"?
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung Nürnberg | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
[-- Attachment #1.2: Type: text/html, Size: 17884 bytes --]
[-- Attachment #2: v2-0001-mcp251x-add-error-handling-for-power-enable-durin.patch --]
[-- Type: application/octet-stream, Size: 2898 bytes --]
From 65d484eff16453737f87b0c605599f438d4cddb1 Mon Sep 17 00:00:00 2001
From: Wenyuan Li <2063309626@qq.com>
Date: Thu, 12 Mar 2026 19:43:03 +0800
Subject: [PATCH v2] mcp251x: add error handling for power enable during open
and resume
Add missing error handling for mcp251x_power_enable() calls in both
mcp251x_open() and mcp251x_can_resume() functions.
In mcp251x_open(), if power enable fails, jump to error path to close
candev without attempting to disable power again.
In mcp251x_can_resume(), properly check return values of power enable
calls for both power and transceiver regulators. If any fails, return
the error code to the PM framework and log the failure.
This ensures the driver properly handles power control failures and
maintains correct device state.
Signed-off-by: Wenyuan Li <2063309626@qq.com>
---
drivers/net/can/spi/mcp251x.c | 30 ++++++++++++++++++++++++------
1 file changed, 24 insertions(+), 6 deletions(-)
diff --git a/drivers/net/can/spi/mcp251x.c b/drivers/net/can/spi/mcp251x.c
index bb7782582f40..0bc208814758 100644
--- a/drivers/net/can/spi/mcp251x.c
+++ b/drivers/net/can/spi/mcp251x.c
@@ -1225,7 +1225,11 @@ static int mcp251x_open(struct net_device *net)
}
mutex_lock(&priv->mcp_lock);
- mcp251x_power_enable(priv->transceiver, 1);
+ ret = mcp251x_power_enable(priv->transceiver, 1);
+ if (ret) {
+ dev_err(&spi->dev, "failed to enable power: %d\n", ret);
+ goto out_close_candev;
+ }
priv->force_quit = 0;
priv->tx_skb = NULL;
@@ -1272,6 +1276,7 @@ static int mcp251x_open(struct net_device *net)
mcp251x_hw_sleep(spi);
out_close:
mcp251x_power_enable(priv->transceiver, 0);
+out_close_candev:
close_candev(net);
mutex_unlock(&priv->mcp_lock);
if (release_irq)
@@ -1516,11 +1521,23 @@ static int __maybe_unused mcp251x_can_resume(struct device *dev)
{
struct spi_device *spi = to_spi_device(dev);
struct mcp251x_priv *priv = spi_get_drvdata(spi);
+ int ret = 0;
- if (priv->after_suspend & AFTER_SUSPEND_POWER)
- mcp251x_power_enable(priv->power, 1);
- if (priv->after_suspend & AFTER_SUSPEND_UP)
- mcp251x_power_enable(priv->transceiver, 1);
+ if (priv->after_suspend & AFTER_SUSPEND_POWER) {
+ ret = mcp251x_power_enable(priv->power, 1);
+ if (ret) {
+ dev_err(&spi->dev, "failed to enable power: %d\n", ret);
+ goto out;
+ }
+ }
+
+ if (priv->after_suspend & AFTER_SUSPEND_UP) {
+ ret = mcp251x_power_enable(priv->transceiver, 1);
+ if (ret) {
+ dev_err(&spi->dev, "failed to enable transceiver: %d\n", ret);
+ goto out;
+ }
+ }
if (priv->after_suspend & (AFTER_SUSPEND_POWER | AFTER_SUSPEND_UP))
queue_work(priv->wq, &priv->restart_work);
@@ -1529,7 +1546,8 @@ static int __maybe_unused mcp251x_can_resume(struct device *dev)
priv->force_quit = 0;
enable_irq(spi->irq);
- return 0;
+out:
+ return ret;
}
static SIMPLE_DEV_PM_OPS(mcp251x_can_pm_ops, mcp251x_can_suspend,
--
2.43.0
next prev parent reply other threads:[~2026-03-12 12:48 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-11 15:50 [PATCH] can: mcp251x: add error handling for power enable in open " Wenyuan Li
2026-03-11 18:05 ` Markus Elfring
2026-03-12 7:46 ` Marc Kleine-Budde
2026-03-12 12:48 ` 悬溺 [this message]
2026-03-12 13:01 ` [PATCH V2] can: mcp251x: add error handling for power enable inopen " Marc Kleine-Budde
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=tencent_9D40B736A2D06AF745F473AAC55B45561106@qq.com \
--to=2063309626@qq.com \
--cc=23120469@bjtu.edu.cn \
--cc=25125283@bjtu.edu.cn \
--cc=25125332@bjtu.edu.cn \
--cc=alban.bedel@lht.dlh.de \
--cc=brgl@kernel.org \
--cc=gszhai@bjtu.edu.cn \
--cc=kuba@kernel.org \
--cc=linux-can@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mailhol@kernel.org \
--cc=marco.crivellari@suse.com \
--cc=maudspierings@gocontroll.com \
--cc=mkl@pengutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®