| Message ID | 20170630052210.GA19186@embeddedgus |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
Hello! On 6/30/2017 8:22 AM, Gustavo A. R. Silva wrote: > Print error message and propagate the return value of > platform_get_irq on failure. You should have probably mentioned that this function no longer returns 0 on error. > Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com> Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jun 30, 2017 at 12:42:38PM +0300, Sergei Shtylyov wrote: > Hello! > > On 6/30/2017 8:22 AM, Gustavo A. R. Silva wrote: > > > Print error message and propagate the return value of > > platform_get_irq on failure. > > You should have probably mentioned that this function no longer returns 0 > on error. Yeah, the patches looks good to me but I'd really appreciate more context in the changelogs. Gustavo, can you please respin the patches? Thanks.
On 06/30/2017 12:42 PM, Sergei Shtylyov wrote: >> Print error message and propagate the return value of >> platform_get_irq on failure. > > You should have probably mentioned that this function no longer returns 0 > on error. It's prolly also worth mentioning that enforcing the error # on return from probe defeats the deferred probing. MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Tejun, Sergei, Quoting Tejun Heo <tj@kernel.org>: > On Fri, Jun 30, 2017 at 12:42:38PM +0300, Sergei Shtylyov wrote: >> Hello! >> >> On 6/30/2017 8:22 AM, Gustavo A. R. Silva wrote: >> >> > Print error message and propagate the return value of >> > platform_get_irq on failure. >> >> You should have probably mentioned that this function no longer returns 0 >> on error. > > Yeah, the patches looks good to me but I'd really appreciate more > context in the changelogs. Gustavo, can you please respin the > patches? > Absolutely. What do you think about the following changelog: platform_get_irq() returns an error code, but the sata_rcar driver ignores it and always returns -EINVAL. This is not correct, and prevents -EPROBE_DEFER from being propagated properly. Print error message and propagate the return value of platform_get_irq on failure. Also, with this change function sata_rcar_probe() no longer returns 0 on error. Thank you! -- Gustavo A. R. Silva -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/30/2017 10:36 PM, Gustavo A. R. Silva wrote: >>> > Print error message and propagate the return value of >>> > platform_get_irq on failure. >>> >>> You should have probably mentioned that this function no longer returns 0 >>> on error. >> >> Yeah, the patches looks good to me but I'd really appreciate more >> context in the changelogs. Gustavo, can you please respin the >> patches? >> > > Absolutely. > > What do you think about the following changelog: > > platform_get_irq() returns an error code, but the sata_rcar driver > ignores it and always returns -EINVAL. This is not correct, and This *was* correct, because it prevented returning 0 on error. > prevents -EPROBE_DEFER from being propagated properly. Yes, this is a real problem. > Print error message and propagate the return value of platform_get_irq > on failure. Also, with this change function sata_rcar_probe() no longer > returns 0 on error. It never did -- I was talking about platform_get_irq() which might return 0 on error until I fixed it: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e330b9a6bb35dc7097a4f02cb1ae7b6f96df92af > Thank you! MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoting "Gustavo A. R. Silva" <garsilva@embeddedor.com>: > Hi Tejun, Sergei, > > Quoting Tejun Heo <tj@kernel.org>: > >> On Fri, Jun 30, 2017 at 12:42:38PM +0300, Sergei Shtylyov wrote: >>> Hello! >>> >>> On 6/30/2017 8:22 AM, Gustavo A. R. Silva wrote: >>> >>>> Print error message and propagate the return value of >>>> platform_get_irq on failure. >>> >>> You should have probably mentioned that this function no longer returns 0 >>> on error. >> >> Yeah, the patches looks good to me but I'd really appreciate more >> context in the changelogs. Gustavo, can you please respin the >> patches? >> > > Absolutely. > > What do you think about the following changelog: > > platform_get_irq() returns an error code, but the sata_rcar driver > ignores it and always returns -EINVAL. This is not correct, and > prevents -EPROBE_DEFER from being propagated properly. > > Print error message and propagate the return value of platform_get_irq > on failure. Also, with this change function sata_rcar_probe() no longer > returns 0 on error. > Errata, this would be final the chagelog: platform_get_irq() returns an error code, but the sata_rcar driver ignores it and always returns -EINVAL. This is not correct, and prevents -EPROBE_DEFER from being propagated properly. Also, notice that platform_get_irq() no longer returns 0 on error. Print error message and propagate the return value of platform_get_irq on failure. Thanks -- Gustavo A. R. Silva -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Sergei, Quoting Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>: > On 06/30/2017 10:36 PM, Gustavo A. R. Silva wrote: > >>>>> Print error message and propagate the return value of >>>>> platform_get_irq on failure. >>>> >>>> You should have probably mentioned that this function no longer >>>> returns 0 >>>> on error. >>> >>> Yeah, the patches looks good to me but I'd really appreciate more >>> context in the changelogs. Gustavo, can you please respin the >>> patches? >>> >> >> Absolutely. >> >> What do you think about the following changelog: >> >> platform_get_irq() returns an error code, but the sata_rcar driver >> ignores it and always returns -EINVAL. This is not correct, and > > This *was* correct, because it prevented returning 0 on error. > Yeah, I got it. >> prevents -EPROBE_DEFER from being propagated properly. > > Yes, this is a real problem. > >> Print error message and propagate the return value of platform_get_irq >> on failure. Also, with this change function sata_rcar_probe() no longer >> returns 0 on error. > > It never did -- I was talking about platform_get_irq() which > might return 0 on error until I fixed it: > Yep, I sent a new email immediately after I realized this was incorrect. Please, check it out. > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e330b9a6bb35dc7097a4f02cb1ae7b6f96df92af > Great work! Thank you -- Gustavo A. R. Silva -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/30/2017 10:46 PM, Gustavo A. R. Silva wrote: >>>>> Print error message and propagate the return value of >>>>> platform_get_irq on failure. >>>> >>>> You should have probably mentioned that this function no longer returns 0 >>>> on error. >>> >>> Yeah, the patches looks good to me but I'd really appreciate more >>> context in the changelogs. Gustavo, can you please respin the >>> patches? >>> >> >> Absolutely. >> >> What do you think about the following changelog: >> >> platform_get_irq() returns an error code, but the sata_rcar driver >> ignores it and always returns -EINVAL. This is not correct, and >> prevents -EPROBE_DEFER from being propagated properly. >> >> Print error message and propagate the return value of platform_get_irq >> on failure. Also, with this change function sata_rcar_probe() no longer >> returns 0 on error. >> > > Errata, this would be final the chagelog: > > platform_get_irq() returns an error code, but the sata_rcar driver > ignores it and always returns -EINVAL. This is not correct, and > prevents -EPROBE_DEFER from being propagated properly. Also, > notice that platform_get_irq() no longer returns 0 on error. > > Print error message and propagate the return value of platform_get_irq > on failure. Now I'm OK with that. > Thanks MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoting Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>: > On 06/30/2017 10:46 PM, Gustavo A. R. Silva wrote: > >>>>>> Print error message and propagate the return value of >>>>>> platform_get_irq on failure. >>>>> >>>>> You should have probably mentioned that this function no longer >>>>> returns 0 >>>>> on error. >>>> >>>> Yeah, the patches looks good to me but I'd really appreciate more >>>> context in the changelogs. Gustavo, can you please respin the >>>> patches? >>>> >>> >>> Absolutely. >>> >>> What do you think about the following changelog: >>> >>> platform_get_irq() returns an error code, but the sata_rcar driver >>> ignores it and always returns -EINVAL. This is not correct, and >>> prevents -EPROBE_DEFER from being propagated properly. >>> >>> Print error message and propagate the return value of platform_get_irq >>> on failure. Also, with this change function sata_rcar_probe() no longer >>> returns 0 on error. >>> >> >> Errata, this would be final the chagelog: >> >> platform_get_irq() returns an error code, but the sata_rcar driver >> ignores it and always returns -EINVAL. This is not correct, and >> prevents -EPROBE_DEFER from being propagated properly. Also, >> notice that platform_get_irq() no longer returns 0 on error. >> >> Print error message and propagate the return value of platform_get_irq >> on failure. > > Now I'm OK with that. > Great :) Thank you -- Gustavo A. R. Silva -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c index ee98447..c936b2a 100644 --- a/drivers/ata/sata_rcar.c +++ b/drivers/ata/sata_rcar.c @@ -872,8 +872,10 @@ static int sata_rcar_probe(struct platform_device *pdev) int ret = 0; irq = platform_get_irq(pdev, 0); - if (irq <= 0) - return -EINVAL; + if (irq < 0) { + dev_err(&pdev->dev, "failed to get IRQ\n"); + return irq; + } priv = devm_kzalloc(&pdev->dev, sizeof(struct sata_rcar_priv), GFP_KERNEL);
Print error message and propagate the return value of platform_get_irq on failure. Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com> --- drivers/ata/sata_rcar.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)