lkml.org 
[lkml]   [2021]   [Dec]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH] driver core: platform: Fix wrong comment
Date
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
\
 
 \ /
  Last update: 2021-12-23 03:55    [W:0.052 / U:0.020 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site