* [PATCH] remoteproc: k3-r5: Fix an error message
@ 2021-05-06 20:46 Christophe JAILLET
2021-05-06 23:51 ` Suman Anna
2021-05-07 5:26 ` Dan Carpenter
0 siblings, 2 replies; 5+ messages in thread
From: Christophe JAILLET @ 2021-05-06 20:46 UTC (permalink / raw)
To: ohad, bjorn.andersson, mathieu.poirier, s-anna
Cc: linux-remoteproc, linux-kernel, kernel-janitors, Christophe JAILLET
'ret' is known to be 0 here.
Reorder the code so that the expected error code is printed.
Fixes: 6dedbd1d5443 ("remoteproc: k3-r5: Add a remoteproc driver for R5F subsystem")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
drivers/remoteproc/ti_k3_r5_remoteproc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 5cf8d030a1f0..4104e4846dbf 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -1272,9 +1272,9 @@ static int k3_r5_core_of_init(struct platform_device *pdev)
core->tsp = k3_r5_core_of_get_tsp(dev, core->ti_sci);
if (IS_ERR(core->tsp)) {
+ ret = PTR_ERR(core->tsp);
dev_err(dev, "failed to construct ti-sci proc control, ret = %d\n",
ret);
- ret = PTR_ERR(core->tsp);
goto err;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] remoteproc: k3-r5: Fix an error message 2021-05-06 20:46 [PATCH] remoteproc: k3-r5: Fix an error message Christophe JAILLET @ 2021-05-06 23:51 ` Suman Anna 2021-05-07 5:26 ` Dan Carpenter 1 sibling, 0 replies; 5+ messages in thread From: Suman Anna @ 2021-05-06 23:51 UTC (permalink / raw) To: Christophe JAILLET, ohad, bjorn.andersson, mathieu.poirier Cc: linux-remoteproc, linux-kernel, kernel-janitors On 5/6/21 3:46 PM, Christophe JAILLET wrote: > 'ret' is known to be 0 here. > Reorder the code so that the expected error code is printed. > > Fixes: 6dedbd1d5443 ("remoteproc: k3-r5: Add a remoteproc driver for R5F subsystem") > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> Thanks for catching the issue. Acked-by: Suman Anna <s-anna@ti.com> regards Suman > --- > drivers/remoteproc/ti_k3_r5_remoteproc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c > index 5cf8d030a1f0..4104e4846dbf 100644 > --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c > +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c > @@ -1272,9 +1272,9 @@ static int k3_r5_core_of_init(struct platform_device *pdev) > > core->tsp = k3_r5_core_of_get_tsp(dev, core->ti_sci); > if (IS_ERR(core->tsp)) { > + ret = PTR_ERR(core->tsp); > dev_err(dev, "failed to construct ti-sci proc control, ret = %d\n", > ret); > - ret = PTR_ERR(core->tsp); > goto err; > } > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] remoteproc: k3-r5: Fix an error message 2021-05-06 20:46 [PATCH] remoteproc: k3-r5: Fix an error message Christophe JAILLET 2021-05-06 23:51 ` Suman Anna @ 2021-05-07 5:26 ` Dan Carpenter 2021-05-07 5:58 ` Christophe JAILLET 1 sibling, 1 reply; 5+ messages in thread From: Dan Carpenter @ 2021-05-07 5:26 UTC (permalink / raw) To: Christophe JAILLET Cc: ohad, bjorn.andersson, mathieu.poirier, s-anna, linux-remoteproc, linux-kernel, kernel-janitors On Thu, May 06, 2021 at 10:46:01PM +0200, Christophe JAILLET wrote: > 'ret' is known to be 0 here. > Reorder the code so that the expected error code is printed. > > Fixes: 6dedbd1d5443 ("remoteproc: k3-r5: Add a remoteproc driver for R5F subsystem") > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > --- > drivers/remoteproc/ti_k3_r5_remoteproc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c > index 5cf8d030a1f0..4104e4846dbf 100644 > --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c > +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c > @@ -1272,9 +1272,9 @@ static int k3_r5_core_of_init(struct platform_device *pdev) > > core->tsp = k3_r5_core_of_get_tsp(dev, core->ti_sci); > if (IS_ERR(core->tsp)) { > + ret = PTR_ERR(core->tsp); > dev_err(dev, "failed to construct ti-sci proc control, ret = %d\n", > ret); I recently learned about the %pe format specifier, which prints "-ENOMEM" instead of -12. dev_err(dev, "failed to construct ti-sci proc control, ret = %pe\n", core->tsp); regards, dan carpenter ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] remoteproc: k3-r5: Fix an error message 2021-05-07 5:26 ` Dan Carpenter @ 2021-05-07 5:58 ` Christophe JAILLET 2021-05-07 6:59 ` Dan Carpenter 0 siblings, 1 reply; 5+ messages in thread From: Christophe JAILLET @ 2021-05-07 5:58 UTC (permalink / raw) To: Dan Carpenter Cc: ohad, bjorn.andersson, mathieu.poirier, s-anna, linux-remoteproc, linux-kernel, kernel-janitors Le 07/05/2021 à 07:26, Dan Carpenter a écrit : > On Thu, May 06, 2021 at 10:46:01PM +0200, Christophe JAILLET wrote: >> 'ret' is known to be 0 here. >> Reorder the code so that the expected error code is printed. >> >> Fixes: 6dedbd1d5443 ("remoteproc: k3-r5: Add a remoteproc driver for R5F subsystem") >> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> >> --- >> drivers/remoteproc/ti_k3_r5_remoteproc.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c >> index 5cf8d030a1f0..4104e4846dbf 100644 >> --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c >> +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c >> @@ -1272,9 +1272,9 @@ static int k3_r5_core_of_init(struct platform_device *pdev) >> >> core->tsp = k3_r5_core_of_get_tsp(dev, core->ti_sci); >> if (IS_ERR(core->tsp)) { >> + ret = PTR_ERR(core->tsp); >> dev_err(dev, "failed to construct ti-sci proc control, ret = %d\n", >> ret); > > I recently learned about the %pe format specifier, which prints "-ENOMEM" > instead of -12. Hi Dan, I see that we are reading the same ML :) Well, I'm a bit puzzled by it. On one hand, it is more user-friendly. On the other hand it is not widely used up to now. So is it better to keep the legacy way of reporting error code? Do you know if there is preferred way? Using it after a IS_ERR is straightforward, but should we also do things like (kmalloc usually don't need error message, just given as an example): x = kmalloc(...); if (!x) dev_err(dev, "Memory allocation failure (%pe)\n", ERR_PTR(-ENOMEM)); When changing a message and make use of %pe, should all the messages in the neighborhood be changed as well to keep some kind of consistancy? CJ > > dev_err(dev, "failed to construct ti-sci proc control, ret = %pe\n", > core->tsp); > regards, > dan carpenter > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] remoteproc: k3-r5: Fix an error message 2021-05-07 5:58 ` Christophe JAILLET @ 2021-05-07 6:59 ` Dan Carpenter 0 siblings, 0 replies; 5+ messages in thread From: Dan Carpenter @ 2021-05-07 6:59 UTC (permalink / raw) To: Christophe JAILLET Cc: ohad, bjorn.andersson, mathieu.poirier, s-anna, linux-remoteproc, linux-kernel, kernel-janitors On Fri, May 07, 2021 at 07:58:39AM +0200, Christophe JAILLET wrote: > Le 07/05/2021 à 07:26, Dan Carpenter a écrit : > > On Thu, May 06, 2021 at 10:46:01PM +0200, Christophe JAILLET wrote: > > > 'ret' is known to be 0 here. > > > Reorder the code so that the expected error code is printed. > > > > > > Fixes: 6dedbd1d5443 ("remoteproc: k3-r5: Add a remoteproc driver for R5F subsystem") > > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > > > --- > > > drivers/remoteproc/ti_k3_r5_remoteproc.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c > > > index 5cf8d030a1f0..4104e4846dbf 100644 > > > --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c > > > +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c > > > @@ -1272,9 +1272,9 @@ static int k3_r5_core_of_init(struct platform_device *pdev) > > > core->tsp = k3_r5_core_of_get_tsp(dev, core->ti_sci); > > > if (IS_ERR(core->tsp)) { > > > + ret = PTR_ERR(core->tsp); > > > dev_err(dev, "failed to construct ti-sci proc control, ret = %d\n", > > > ret); > > > > I recently learned about the %pe format specifier, which prints "-ENOMEM" > > instead of -12. > > Hi Dan, > > I see that we are reading the same ML :) > > > Well, I'm a bit puzzled by it. > On one hand, it is more user-friendly. On the other hand it is not widely > used up to now. > > So is it better to keep the legacy way of reporting error code? It might make back porting things more complicated? I'm surprised this hasn't been backported further back to 5.4. > > Do you know if there is preferred way? It's new. Soon it will be the prefered way. You're right, of course, that needs to introduce a %e which takes an int. I have left this as an exercise for the reader. ;) Eventually someone will work up the energy required and do this work. regards, dan carpenter ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-05-07 6:59 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-06 20:46 [PATCH] remoteproc: k3-r5: Fix an error message Christophe JAILLET 2021-05-06 23:51 ` Suman Anna 2021-05-07 5:26 ` Dan Carpenter 2021-05-07 5:58 ` Christophe JAILLET 2021-05-07 6:59 ` Dan Carpenter
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).