* [PATCH] can: rcar_canfd: Print mnemotechnic error codes
@ 2023-03-09 8:10 Geert Uytterhoeven
2023-03-09 15:19 ` Simon Horman
2023-03-09 16:14 ` Vincent MAILHOL
0 siblings, 2 replies; 3+ messages in thread
From: Geert Uytterhoeven @ 2023-03-09 8:10 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde, Vincent Mailhol
Cc: linux-can, netdev, linux-renesas-soc, Geert Uytterhoeven,
Vincent Mailhol
Replace printed numerical error codes by mnemotechnic error codes, to
improve the user experience in case of errors.
While at it, drop printing of an error message in case of out-of-memory,
as the core memory allocation code already takes care of this.
Suggested-by: Vincent Mailhol <vincent.mailhol@gmail.com>
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
This depends on "[PATCH v2] can: rcar_canfd: Add transceiver support"
https://lore.kernel.org/r/e825b50a843ffe40e33f34e4d858c07c1b2ff259.1678280913.git.geert+renesas@glider.be
---
drivers/net/can/rcar/rcar_canfd.c | 43 +++++++++++++++----------------
1 file changed, 21 insertions(+), 22 deletions(-)
diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
index 6df9a259e5e4f92c..bc75da349676d867 100644
--- a/drivers/net/can/rcar/rcar_canfd.c
+++ b/drivers/net/can/rcar/rcar_canfd.c
@@ -1417,20 +1417,20 @@ static int rcar_canfd_open(struct net_device *ndev)
err = phy_power_on(priv->transceiver);
if (err) {
- netdev_err(ndev, "failed to power on PHY, error %d\n", err);
+ netdev_err(ndev, "failed to power on PHY, %pe\n", ERR_PTR(err));
return err;
}
/* Peripheral clock is already enabled in probe */
err = clk_prepare_enable(gpriv->can_clk);
if (err) {
- netdev_err(ndev, "failed to enable CAN clock, error %d\n", err);
+ netdev_err(ndev, "failed to enable CAN clock, %pe\n", ERR_PTR(err));
goto out_phy;
}
err = open_candev(ndev);
if (err) {
- netdev_err(ndev, "open_candev() failed, error %d\n", err);
+ netdev_err(ndev, "open_candev() failed, %pe\n", ERR_PTR(err));
goto out_can_clock;
}
@@ -1731,10 +1731,9 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch,
int err = -ENODEV;
ndev = alloc_candev(sizeof(*priv), RCANFD_FIFO_DEPTH);
- if (!ndev) {
- dev_err(dev, "alloc_candev() failed\n");
+ if (!ndev)
return -ENOMEM;
- }
+
priv = netdev_priv(ndev);
ndev->netdev_ops = &rcar_canfd_netdev_ops;
@@ -1777,8 +1776,8 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch,
rcar_canfd_channel_err_interrupt, 0,
irq_name, priv);
if (err) {
- dev_err(dev, "devm_request_irq CH Err(%d) failed, error %d\n",
- err_irq, err);
+ dev_err(dev, "devm_request_irq CH Err(%d) failed, %pe\n",
+ err_irq, ERR_PTR(err));
goto fail;
}
irq_name = devm_kasprintf(dev, GFP_KERNEL, "canfd.ch%d_trx",
@@ -1791,8 +1790,8 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch,
rcar_canfd_channel_tx_interrupt, 0,
irq_name, priv);
if (err) {
- dev_err(dev, "devm_request_irq Tx (%d) failed, error %d\n",
- tx_irq, err);
+ dev_err(dev, "devm_request_irq Tx (%d) failed, %pe\n",
+ tx_irq, ERR_PTR(err));
goto fail;
}
}
@@ -1823,7 +1822,7 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch,
gpriv->ch[priv->channel] = priv;
err = register_candev(ndev);
if (err) {
- dev_err(dev, "register_candev() failed, error %d\n", err);
+ dev_err(dev, "register_candev() failed, %pe\n", ERR_PTR(err));
goto fail_candev;
}
dev_info(dev, "device registered (channel %u)\n", priv->channel);
@@ -1967,16 +1966,16 @@ static int rcar_canfd_probe(struct platform_device *pdev)
rcar_canfd_channel_interrupt, 0,
"canfd.ch_int", gpriv);
if (err) {
- dev_err(dev, "devm_request_irq(%d) failed, error %d\n",
- ch_irq, err);
+ dev_err(dev, "devm_request_irq(%d) failed, %pe\n",
+ ch_irq, ERR_PTR(err));
goto fail_dev;
}
err = devm_request_irq(dev, g_irq, rcar_canfd_global_interrupt,
0, "canfd.g_int", gpriv);
if (err) {
- dev_err(dev, "devm_request_irq(%d) failed, error %d\n",
- g_irq, err);
+ dev_err(dev, "devm_request_irq(%d) failed, %pe\n",
+ g_irq, ERR_PTR(err));
goto fail_dev;
}
} else {
@@ -1985,8 +1984,8 @@ static int rcar_canfd_probe(struct platform_device *pdev)
"canfd.g_recc", gpriv);
if (err) {
- dev_err(dev, "devm_request_irq(%d) failed, error %d\n",
- g_recc_irq, err);
+ dev_err(dev, "devm_request_irq(%d) failed, %pe\n",
+ g_recc_irq, ERR_PTR(err));
goto fail_dev;
}
@@ -1994,8 +1993,8 @@ static int rcar_canfd_probe(struct platform_device *pdev)
rcar_canfd_global_err_interrupt, 0,
"canfd.g_err", gpriv);
if (err) {
- dev_err(dev, "devm_request_irq(%d) failed, error %d\n",
- g_err_irq, err);
+ dev_err(dev, "devm_request_irq(%d) failed, %pe\n",
+ g_err_irq, ERR_PTR(err));
goto fail_dev;
}
}
@@ -2012,14 +2011,14 @@ static int rcar_canfd_probe(struct platform_device *pdev)
/* Enable peripheral clock for register access */
err = clk_prepare_enable(gpriv->clkp);
if (err) {
- dev_err(dev, "failed to enable peripheral clock, error %d\n",
- err);
+ dev_err(dev, "failed to enable peripheral clock, %pe\n",
+ ERR_PTR(err));
goto fail_reset;
}
err = rcar_canfd_reset_controller(gpriv);
if (err) {
- dev_err(dev, "reset controller failed\n");
+ dev_err(dev, "reset controller failed, %pe\n", ERR_PTR(err));
goto fail_clk;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] can: rcar_canfd: Print mnemotechnic error codes
2023-03-09 8:10 [PATCH] can: rcar_canfd: Print mnemotechnic error codes Geert Uytterhoeven
@ 2023-03-09 15:19 ` Simon Horman
2023-03-09 16:14 ` Vincent MAILHOL
1 sibling, 0 replies; 3+ messages in thread
From: Simon Horman @ 2023-03-09 15:19 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Wolfgang Grandegger, Marc Kleine-Budde, Vincent Mailhol,
linux-can, netdev, linux-renesas-soc, Vincent Mailhol
On Thu, Mar 09, 2023 at 09:10:22AM +0100, Geert Uytterhoeven wrote:
> Replace printed numerical error codes by mnemotechnic error codes, to
> improve the user experience in case of errors.
>
> While at it, drop printing of an error message in case of out-of-memory,
> as the core memory allocation code already takes care of this.
>
> Suggested-by: Vincent Mailhol <vincent.mailhol@gmail.com>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] can: rcar_canfd: Print mnemotechnic error codes
2023-03-09 8:10 [PATCH] can: rcar_canfd: Print mnemotechnic error codes Geert Uytterhoeven
2023-03-09 15:19 ` Simon Horman
@ 2023-03-09 16:14 ` Vincent MAILHOL
1 sibling, 0 replies; 3+ messages in thread
From: Vincent MAILHOL @ 2023-03-09 16:14 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Wolfgang Grandegger, Marc Kleine-Budde, linux-can, netdev,
linux-renesas-soc
On Thu. 9 Mar. 2023 at 17:10, Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
> Replace printed numerical error codes by mnemotechnic error codes, to
> improve the user experience in case of errors.
>
> While at it, drop printing of an error message in case of out-of-memory,
> as the core memory allocation code already takes care of this.
>
> Suggested-by: Vincent Mailhol <vincent.mailhol@gmail.com>
^^^^^^^^^^^^^^^^^^^^^^^^^
Can you use my other address?
Suggested-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Thanks for the patch. Same as before, I have one nitpick on the
already existing code (c.f. below). You can add my review tag in next
version:
Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> This depends on "[PATCH v2] can: rcar_canfd: Add transceiver support"
> https://lore.kernel.org/r/e825b50a843ffe40e33f34e4d858c07c1b2ff259.1678280913.git.geert+renesas@glider.be
> ---
> drivers/net/can/rcar/rcar_canfd.c | 43 +++++++++++++++----------------
> 1 file changed, 21 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/can/rcar/rcar_canfd.c b/drivers/net/can/rcar/rcar_canfd.c
> index 6df9a259e5e4f92c..bc75da349676d867 100644
> --- a/drivers/net/can/rcar/rcar_canfd.c
> +++ b/drivers/net/can/rcar/rcar_canfd.c
> @@ -1417,20 +1417,20 @@ static int rcar_canfd_open(struct net_device *ndev)
>
> err = phy_power_on(priv->transceiver);
> if (err) {
> - netdev_err(ndev, "failed to power on PHY, error %d\n", err);
> + netdev_err(ndev, "failed to power on PHY, %pe\n", ERR_PTR(err));
> return err;
> }
>
> /* Peripheral clock is already enabled in probe */
> err = clk_prepare_enable(gpriv->can_clk);
> if (err) {
> - netdev_err(ndev, "failed to enable CAN clock, error %d\n", err);
> + netdev_err(ndev, "failed to enable CAN clock, %pe\n", ERR_PTR(err));
> goto out_phy;
> }
>
> err = open_candev(ndev);
> if (err) {
> - netdev_err(ndev, "open_candev() failed, error %d\n", err);
> + netdev_err(ndev, "open_candev() failed, %pe\n", ERR_PTR(err));
> goto out_can_clock;
> }
>
> @@ -1731,10 +1731,9 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch,
> int err = -ENODEV;
>
> ndev = alloc_candev(sizeof(*priv), RCANFD_FIFO_DEPTH);
> - if (!ndev) {
> - dev_err(dev, "alloc_candev() failed\n");
> + if (!ndev)
> return -ENOMEM;
> - }
> +
> priv = netdev_priv(ndev);
>
> ndev->netdev_ops = &rcar_canfd_netdev_ops;
> @@ -1777,8 +1776,8 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch,
> rcar_canfd_channel_err_interrupt, 0,
> irq_name, priv);
> if (err) {
> - dev_err(dev, "devm_request_irq CH Err(%d) failed, error %d\n",
> - err_irq, err);
> + dev_err(dev, "devm_request_irq CH Err(%d) failed, %pe\n",
^^^^
From the Linux coding style:
Printing numbers in parentheses (%d) adds no value and should be avoided.
Ref: https://www.kernel.org/doc/html/v4.10/process/coding-style.html#printing-kernel-messages
One more time, this is already existing code, so bonus points if you
fix this as well, but I will not blame you if you feel lazy and keep
the patch as is.
> + err_irq, ERR_PTR(err));
> goto fail;
> }
> irq_name = devm_kasprintf(dev, GFP_KERNEL, "canfd.ch%d_trx",
> @@ -1791,8 +1790,8 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch,
> rcar_canfd_channel_tx_interrupt, 0,
> irq_name, priv);
> if (err) {
> - dev_err(dev, "devm_request_irq Tx (%d) failed, error %d\n",
> - tx_irq, err);
> + dev_err(dev, "devm_request_irq Tx (%d) failed, %pe\n",
> + tx_irq, ERR_PTR(err));
> goto fail;
> }
> }
> @@ -1823,7 +1822,7 @@ static int rcar_canfd_channel_probe(struct rcar_canfd_global *gpriv, u32 ch,
> gpriv->ch[priv->channel] = priv;
> err = register_candev(ndev);
> if (err) {
> - dev_err(dev, "register_candev() failed, error %d\n", err);
> + dev_err(dev, "register_candev() failed, %pe\n", ERR_PTR(err));
> goto fail_candev;
> }
> dev_info(dev, "device registered (channel %u)\n", priv->channel);
> @@ -1967,16 +1966,16 @@ static int rcar_canfd_probe(struct platform_device *pdev)
> rcar_canfd_channel_interrupt, 0,
> "canfd.ch_int", gpriv);
> if (err) {
> - dev_err(dev, "devm_request_irq(%d) failed, error %d\n",
> - ch_irq, err);
> + dev_err(dev, "devm_request_irq(%d) failed, %pe\n",
Same as above.
> + ch_irq, ERR_PTR(err));
> goto fail_dev;
> }
>
> err = devm_request_irq(dev, g_irq, rcar_canfd_global_interrupt,
> 0, "canfd.g_int", gpriv);
> if (err) {
> - dev_err(dev, "devm_request_irq(%d) failed, error %d\n",
> - g_irq, err);
> + dev_err(dev, "devm_request_irq(%d) failed, %pe\n",
Same as above.
> + g_irq, ERR_PTR(err));
> goto fail_dev;
> }
> } else {
> @@ -1985,8 +1984,8 @@ static int rcar_canfd_probe(struct platform_device *pdev)
> "canfd.g_recc", gpriv);
>
> if (err) {
> - dev_err(dev, "devm_request_irq(%d) failed, error %d\n",
> - g_recc_irq, err);
> + dev_err(dev, "devm_request_irq(%d) failed, %pe\n",
> + g_recc_irq, ERR_PTR(err));
> goto fail_dev;
> }
>
> @@ -1994,8 +1993,8 @@ static int rcar_canfd_probe(struct platform_device *pdev)
> rcar_canfd_global_err_interrupt, 0,
> "canfd.g_err", gpriv);
> if (err) {
> - dev_err(dev, "devm_request_irq(%d) failed, error %d\n",
> - g_err_irq, err);
> + dev_err(dev, "devm_request_irq(%d) failed, %pe\n",
Same as above.
> + g_err_irq, ERR_PTR(err));
> goto fail_dev;
> }
> }
> @@ -2012,14 +2011,14 @@ static int rcar_canfd_probe(struct platform_device *pdev)
> /* Enable peripheral clock for register access */
> err = clk_prepare_enable(gpriv->clkp);
> if (err) {
> - dev_err(dev, "failed to enable peripheral clock, error %d\n",
> - err);
> + dev_err(dev, "failed to enable peripheral clock, %pe\n",
> + ERR_PTR(err));
> goto fail_reset;
> }
>
> err = rcar_canfd_reset_controller(gpriv);
> if (err) {
> - dev_err(dev, "reset controller failed\n");
> + dev_err(dev, "reset controller failed, %pe\n", ERR_PTR(err));
> goto fail_clk;
> }
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-03-09 16:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-09 8:10 [PATCH] can: rcar_canfd: Print mnemotechnic error codes Geert Uytterhoeven
2023-03-09 15:19 ` Simon Horman
2023-03-09 16:14 ` Vincent MAILHOL
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).