Messages in this thread Patch in this message | | | From | Jiasheng Jiang <> | Subject | [PATCH] driver core: platform: Fix wrong comment | Date | Thu, 23 Dec 2021 10:53:46 +0800 |
| |
I notice that there is a 'WARN(ret == 0, "0 is an invalid IRQ number\n");' before 'return ret;', which means that it is possible to return 0 if fails. Therefore, it might be better to correct the wrong comment. And also there is reply sent by Damien Le Moal because I submitted a patch to remove the non-zero check of the platform_get_irq() previously. Damien Le Moal said that the comment for platform_get_irq() is wrond because it can actually return 0. Moreover, platform_get_irq() returns platform_get_irq_optional(). As a conclusion, the comments of the platform_get_irq() and platform_get_irq_optional() should be fixed. Not only that, the comments of platform_get_irq_byname() and platform_get_irq_byname_optional() have the same error. This time I only submit one as an example. If the patch is right, I will submit another version to fix all. But, I also notice that the 'return 0' is removed intentionally in the fixed tag. I am not sure which one is right. Anyway, the success IRQ number should be 'postive' other than 'non-zero'. So the comment should be corrected. Here is the mail from Damien Le Moal.
Link: https://lore.kernel.org/lkml/dd6e6054-3d7e-b43a-0386-71323c49ab27@opensource.wdc.com/ On Thu, Dec 23, 2021 at 08:41:54AM +0800, Damien Le Moal wrote: >> It can be found that platform_get_irq() returns nagative code but not >> null when fails. >> The comment of the platform_get_irq clearly shows that. >> Therefore it should be better to remove the useless check. > > Nope, platform_get_irq() can actually return 0 (the comment for that > function is wrong). So testing for !irq is valid and must be kept. > >> >> Fixes: fd990556f0fa ("ata: move library code from ahci_platform.c to libahci_platform.c") >> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn> >> --- >> drivers/ata/libahci_platform.c | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c >> index b2f552088291..5ec68f138c28 100644 >> --- a/drivers/ata/libahci_platform.c >> +++ b/drivers/ata/libahci_platform.c >> @@ -587,8 +587,6 @@ int ahci_platform_init_host(struct platform_device *pdev, >> dev_err(dev, "no irq\n"); >> return irq; >> } >> - if (!irq) >> - return -EINVAL; >> >> hpriv->irq = irq; >>
Fixes: c2f3f755f5c7 ("Revert "driver core: platform: Make platform_get_irq_optional() optional"") Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn> --- drivers/base/platform.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 9cd34def2237..fcba2559cc90 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -181,10 +181,10 @@ EXPORT_SYMBOL_GPL(devm_platform_ioremap_resource_byname); * For example:: * * int irq = platform_get_irq_optional(pdev, 0); - * if (irq < 0) + * if (irq <= 0) * return irq; * - * Return: non-zero IRQ number on success, negative error number on failure. + * Return: positive IRQ number on success, negative error number and zero on failure. */ int platform_get_irq_optional(struct platform_device *dev, unsigned int num) { -- 2.25.1
| |