[PATCH v2] firmware: arm_scmi: Use dev_err_probe() simplify the code

[PATCH v2] firmware: arm_scmi: Use dev_err_probe() simplify the code

[long.yunjian]

From: Yumeng Fang <fang.yumeng@zte.com.cn>

In the probe path, dev_err() can be replaced with dev_err_probe()
which will check if error code is -EPROBE_DEFER and prints the
error name. It also sets the defer probe reason which can be
checked later through debugfs.

Signed-off-by: Yumeng Fang <fang.yumeng@zte.com.cn>
---
v1 -> v2
(1) Order the includes alphabetically.
(2) Delete "ret = PTR_ERR(*)", and then replace ret in dev_err_probe with "PTR_ERR(*)".

 .../firmware/arm_scmi/transports/mailbox.c    | 20 +++++++------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/firmware/arm_scmi/transports/mailbox.c b/drivers/firmware/arm_scmi/transports/mailbox.c
index bd041c99b92b..764cbeac2492 100644
--- a/drivers/firmware/arm_scmi/transports/mailbox.c
+++ b/drivers/firmware/arm_scmi/transports/mailbox.c
@@ -8,6 +8,7 @@

 #include <linux/err.h>
 #include <linux/device.h>
+#include <linux/dev_printk.h>
 #include <linux/mailbox_client.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
@@ -214,31 +215,24 @@ static int mailbox_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,

 	smbox->chan = mbox_request_channel(cl, tx ? 0 : p2a_chan);
 	if (IS_ERR(smbox->chan)) {
-		ret = PTR_ERR(smbox->chan);
-		if (ret != -EPROBE_DEFER)
-			dev_err(cdev,
-				"failed to request SCMI %s mailbox\n", desc);
-		return ret;
+		return dev_err_probe(cdev, PTR_ERR(smbox->chan),
+				     "failed to request SCMI %s mailbox\n", desc);
 	}

 	/* Additional unidirectional channel for TX if needed */
 	if (tx && a2p_rx_chan) {
 		smbox->chan_receiver = mbox_request_channel(cl, a2p_rx_chan);
 		if (IS_ERR(smbox->chan_receiver)) {
-			ret = PTR_ERR(smbox->chan_receiver);
-			if (ret != -EPROBE_DEFER)
-				dev_err(cdev, "failed to request SCMI Tx Receiver mailbox\n");
-			return ret;
+			return dev_err_probe(cdev, PTR_ERR(smbox->chan_receiver),
+					     "failed to request SCMI Tx Receiver mailbox\n");
 		}
 	}

 	if (!tx && p2a_rx_chan) {
 		smbox->chan_platform_receiver = mbox_request_channel(cl, p2a_rx_chan);
 		if (IS_ERR(smbox->chan_platform_receiver)) {
-			ret = PTR_ERR(smbox->chan_platform_receiver);
-			if (ret != -EPROBE_DEFER)
-				dev_err(cdev, "failed to request SCMI P2A Receiver mailbox\n");
-			return ret;
+			return dev_err_probe(cdev, PTR_ERR(smbox->chan_platform_receiver),
+					     "failed to request SCMI P2A Receiver mailbox\n");
 		}
 	}

-- 
2.25.1

Re: [PATCH v2] firmware: arm_scmi: Use dev_err_probe() simplify the code

[Krzysztof Kozlowski]

On 21/05/2025 10:14, long.yunjian@zte.com.cn wrote:
> From: Yumeng Fang <fang.yumeng@zte.com.cn>
> 
> In the probe path, dev_err() can be replaced with dev_err_probe()

That's mailbox channel setup, not probe path.

Either this patch is wrong or commit msg is just not relevant.


> which will check if error code is -EPROBE_DEFER and prints the
> error name. It also sets the defer probe reason which can be
> checked later through debugfs.

You explain the basic stuff, we all know it, but you miss to explain
things which we do not know. Rewrite your commit msgs to explain the
non-obvious.

We all know how dev_err_probe works. What we do not know is ALWAYS that
chan setup is the probe path (so prove that it is ALWAYS probe path).



Best regards,
Krzysztof

Re: [PATCH v2] firmware: arm_scmi: Use dev_err_probe() simplify the code

[Dan Carpenter]

On Wed, May 21, 2025 at 04:14:49PM +0800, long.yunjian@zte.com.cn wrote:
> From: Yumeng Fang <fang.yumeng@zte.com.cn>
> 
> In the probe path, dev_err() can be replaced with dev_err_probe()
> which will check if error code is -EPROBE_DEFER and prints the
> error name. It also sets the defer probe reason which can be
> checked later through debugfs.
> 
> Signed-off-by: Yumeng Fang <fang.yumeng@zte.com.cn>
> ---
> v1 -> v2
> (1) Order the includes alphabetically.
> (2) Delete "ret = PTR_ERR(*)", and then replace ret in dev_err_probe with "PTR_ERR(*)".
> 
>  .../firmware/arm_scmi/transports/mailbox.c    | 20 +++++++------------
>  1 file changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/firmware/arm_scmi/transports/mailbox.c b/drivers/firmware/arm_scmi/transports/mailbox.c
> index bd041c99b92b..764cbeac2492 100644
> --- a/drivers/firmware/arm_scmi/transports/mailbox.c
> +++ b/drivers/firmware/arm_scmi/transports/mailbox.c
> @@ -8,6 +8,7 @@
> 
>  #include <linux/err.h>
>  #include <linux/device.h>
> +#include <linux/dev_printk.h>
>  #include <linux/mailbox_client.h>
>  #include <linux/of.h>
>  #include <linux/of_address.h>
> @@ -214,31 +215,24 @@ static int mailbox_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
> 
>  	smbox->chan = mbox_request_channel(cl, tx ? 0 : p2a_chan);
>  	if (IS_ERR(smbox->chan)) {
> -		ret = PTR_ERR(smbox->chan);
> -		if (ret != -EPROBE_DEFER)
> -			dev_err(cdev,
> -				"failed to request SCMI %s mailbox\n", desc);
> -		return ret;
> +		return dev_err_probe(cdev, PTR_ERR(smbox->chan),
> +				     "failed to request SCMI %s mailbox\n", desc);
>  	}

Remove the { } braces as well.  They will cause a checkpatch problem if
you re-run checkpatch.pl --strict on the resulting file.  Same for the
other two as well.

regards,
dan carpenter