[PATCH] extcon: maxim: Fix missing IRQF_ONESHOT as only threaded handler

[PATCH] extcon: maxim: Fix missing IRQF_ONESHOT as only threaded handler

[zhuguangqing83]

From: Guangqing Zhu <zhuguangqing83@gmail.com>

Coccinelle noticed:
  1. drivers/extcon/extcon-max14577.c:699:8-33: ERROR: Threaded IRQ with
no primary handler requested without IRQF_ONESHOT
  2. drivers/extcon/extcon-max77693.c:1143:8-33: ERROR: Threaded IRQ with
no primary handler requested without IRQF_ONESHOT
  3. drivers/extcon/extcon-max77843.c:907:8-33: ERROR: Threaded IRQ with
no primary handler requested without IRQF_ONESHOT
  4. drivers/extcon/extcon-max8997.c:665:8-28: ERROR: Threaded IRQ with
no primary handler requested without IRQF_ONESHOT

Signed-off-by: Guangqing Zhu <zhuguangqing83@gmail.com>
---
 drivers/extcon/extcon-max14577.c | 2 +-
 drivers/extcon/extcon-max77693.c | 2 +-
 drivers/extcon/extcon-max77843.c | 3 ++-
 drivers/extcon/extcon-max8997.c  | 2 +-
 4 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/extcon/extcon-max14577.c b/drivers/extcon/extcon-max14577.c
index ace523924e58..af15a9e00ee9 100644
--- a/drivers/extcon/extcon-max14577.c
+++ b/drivers/extcon/extcon-max14577.c
@@ -698,7 +698,7 @@ static int max14577_muic_probe(struct platform_device *pdev)
 
 		ret = devm_request_threaded_irq(&pdev->dev, virq, NULL,
 				max14577_muic_irq_handler,
-				IRQF_NO_SUSPEND,
+				IRQF_NO_SUSPEND | IRQF_ONESHOT,
 				muic_irq->name, info);
 		if (ret) {
 			dev_err(&pdev->dev,
diff --git a/drivers/extcon/extcon-max77693.c b/drivers/extcon/extcon-max77693.c
index 92af97e00828..4494eefce31f 100644
--- a/drivers/extcon/extcon-max77693.c
+++ b/drivers/extcon/extcon-max77693.c
@@ -1142,7 +1142,7 @@ static int max77693_muic_probe(struct platform_device *pdev)
 
 		ret = devm_request_threaded_irq(&pdev->dev, virq, NULL,
 				max77693_muic_irq_handler,
-				IRQF_NO_SUSPEND,
+				IRQF_NO_SUSPEND | IRQF_ONESHOT,
 				muic_irq->name, info);
 		if (ret) {
 			dev_err(&pdev->dev,
diff --git a/drivers/extcon/extcon-max77843.c b/drivers/extcon/extcon-max77843.c
index 8e6e97ec65a8..9167f99d2979 100644
--- a/drivers/extcon/extcon-max77843.c
+++ b/drivers/extcon/extcon-max77843.c
@@ -905,7 +905,8 @@ static int max77843_muic_probe(struct platform_device *pdev)
 		muic_irq->virq = virq;
 
 		ret = devm_request_threaded_irq(&pdev->dev, virq, NULL,
-				max77843_muic_irq_handler, IRQF_NO_SUSPEND,
+				max77843_muic_irq_handler,
+				IRQF_NO_SUSPEND | IRQF_ONESHOT,
 				muic_irq->name, info);
 		if (ret) {
 			dev_err(&pdev->dev,
diff --git a/drivers/extcon/extcon-max8997.c b/drivers/extcon/extcon-max8997.c
index e1408075ef7d..8b5efbca15e3 100644
--- a/drivers/extcon/extcon-max8997.c
+++ b/drivers/extcon/extcon-max8997.c
@@ -664,7 +664,7 @@ static int max8997_muic_probe(struct platform_device *pdev)
 
 		ret = request_threaded_irq(virq, NULL,
 				max8997_muic_irq_handler,
-				IRQF_NO_SUSPEND,
+				IRQF_NO_SUSPEND | IRQF_ONESHOT,
 				muic_irq->name, info);
 		if (ret) {
 			dev_err(&pdev->dev,
-- 
2.17.1

Re: [PATCH] extcon: maxim: Fix missing IRQF_ONESHOT as only threaded handler

[Krzysztof Kozlowski]

On 15/04/2021 13:36, zhuguangqing83@gmail.com wrote:
> From: Guangqing Zhu <zhuguangqing83@gmail.com>
> 
> Coccinelle noticed:
>   1. drivers/extcon/extcon-max14577.c:699:8-33: ERROR: Threaded IRQ with
> no primary handler requested without IRQF_ONESHOT
>   2. drivers/extcon/extcon-max77693.c:1143:8-33: ERROR: Threaded IRQ with
> no primary handler requested without IRQF_ONESHOT
>   3. drivers/extcon/extcon-max77843.c:907:8-33: ERROR: Threaded IRQ with
> no primary handler requested without IRQF_ONESHOT
>   4. drivers/extcon/extcon-max8997.c:665:8-28: ERROR: Threaded IRQ with
> no primary handler requested without IRQF_ONESHOT
> 
> Signed-off-by: Guangqing Zhu <zhuguangqing83@gmail.com>
> ---
>  drivers/extcon/extcon-max14577.c | 2 +-
>  drivers/extcon/extcon-max77693.c | 2 +-
>  drivers/extcon/extcon-max77843.c | 3 ++-
>  drivers/extcon/extcon-max8997.c  | 2 +-
>  4 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-max14577.c b/drivers/extcon/extcon-max14577.c
> index ace523924e58..af15a9e00ee9 100644
> --- a/drivers/extcon/extcon-max14577.c
> +++ b/drivers/extcon/extcon-max14577.c
> @@ -698,7 +698,7 @@ static int max14577_muic_probe(struct platform_device *pdev)
>  
>  		ret = devm_request_threaded_irq(&pdev->dev, virq, NULL,
>  				max14577_muic_irq_handler,
> -				IRQF_NO_SUSPEND,
> +				IRQF_NO_SUSPEND | IRQF_ONESHOT,

The same with all other patches for IRQF_ONESHOT which are send recently:
1. On what board did you test it?
2. Is this just blind patch from Coccinelle without investigation
whether it is needed (hint: it's not needed here, it does not use
default primary handler).
3. If you think otherwise, please explain.

Best regards,
Krzysztof

Re: [PATCH] extcon: maxim: Fix missing IRQF_ONESHOT as only threaded handler

[Guangqing Zhu]

On 16/04/2021 16:43, Krzysztof Kozlowski wrote:
> On 15/04/2021 13:36, zhuguangqing83@gmail.com wrote:
>> From: Guangqing Zhu <zhuguangqing83@gmail.com>
>>
>> Coccinelle noticed:
>>    1. drivers/extcon/extcon-max14577.c:699:8-33: ERROR: Threaded IRQ with
>> no primary handler requested without IRQF_ONESHOT
>>    2. drivers/extcon/extcon-max77693.c:1143:8-33: ERROR: Threaded IRQ with
>> no primary handler requested without IRQF_ONESHOT
>>    3. drivers/extcon/extcon-max77843.c:907:8-33: ERROR: Threaded IRQ with
>> no primary handler requested without IRQF_ONESHOT
>>    4. drivers/extcon/extcon-max8997.c:665:8-28: ERROR: Threaded IRQ with
>> no primary handler requested without IRQF_ONESHOT
>>
>> Signed-off-by: Guangqing Zhu <zhuguangqing83@gmail.com>
>> ---
>>   drivers/extcon/extcon-max14577.c | 2 +-
>>   drivers/extcon/extcon-max77693.c | 2 +-
>>   drivers/extcon/extcon-max77843.c | 3 ++-
>>   drivers/extcon/extcon-max8997.c  | 2 +-
>>   4 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/extcon/extcon-max14577.c b/drivers/extcon/extcon-max14577.c
>> index ace523924e58..af15a9e00ee9 100644
>> --- a/drivers/extcon/extcon-max14577.c
>> +++ b/drivers/extcon/extcon-max14577.c
>> @@ -698,7 +698,7 @@ static int max14577_muic_probe(struct platform_device *pdev)
>>   
>>   		ret = devm_request_threaded_irq(&pdev->dev, virq, NULL,
>>   				max14577_muic_irq_handler,
>> -				IRQF_NO_SUSPEND,
>> +				IRQF_NO_SUSPEND | IRQF_ONESHOT,
> 
> The same with all other patches for IRQF_ONESHOT which are send recently:
> 1. On what board did you test it?

I didn't test it.

> 2. Is this just blind patch from Coccinelle without investigation
> whether it is needed (hint: it's not needed here, it does not use
> default primary handler).

I found the error notice from Coccinelle and I saw the code. Maybe
I'm mistaken, I think it's needed here. Because handler == NULL and
thread_fn != NULL, it use irq_default_primary_handler() in
request_threaded_irq().

> 3. If you think otherwise, please explain.
> 
> Best regards,
> Krzysztof
> 

Thanks for your review.

Best regards,
Guangqing Zhu

Re: [PATCH] extcon: maxim: Fix missing IRQF_ONESHOT as only threaded handler

[Krzysztof Kozlowski]

On 18/04/2021 16:41, Guangqing Zhu wrote:
> 
> 
> On 16/04/2021 16:43, Krzysztof Kozlowski wrote:
>> On 15/04/2021 13:36, zhuguangqing83@gmail.com wrote:
>>> From: Guangqing Zhu <zhuguangqing83@gmail.com>
>>>
>>> Coccinelle noticed:
>>>    1. drivers/extcon/extcon-max14577.c:699:8-33: ERROR: Threaded IRQ with
>>> no primary handler requested without IRQF_ONESHOT
>>>    2. drivers/extcon/extcon-max77693.c:1143:8-33: ERROR: Threaded IRQ with
>>> no primary handler requested without IRQF_ONESHOT
>>>    3. drivers/extcon/extcon-max77843.c:907:8-33: ERROR: Threaded IRQ with
>>> no primary handler requested without IRQF_ONESHOT
>>>    4. drivers/extcon/extcon-max8997.c:665:8-28: ERROR: Threaded IRQ with
>>> no primary handler requested without IRQF_ONESHOT
>>>
>>> Signed-off-by: Guangqing Zhu <zhuguangqing83@gmail.com>
>>> ---
>>>   drivers/extcon/extcon-max14577.c | 2 +-
>>>   drivers/extcon/extcon-max77693.c | 2 +-
>>>   drivers/extcon/extcon-max77843.c | 3 ++-
>>>   drivers/extcon/extcon-max8997.c  | 2 +-
>>>   4 files changed, 5 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/extcon/extcon-max14577.c b/drivers/extcon/extcon-max14577.c
>>> index ace523924e58..af15a9e00ee9 100644
>>> --- a/drivers/extcon/extcon-max14577.c
>>> +++ b/drivers/extcon/extcon-max14577.c
>>> @@ -698,7 +698,7 @@ static int max14577_muic_probe(struct platform_device *pdev)
>>>   
>>>   		ret = devm_request_threaded_irq(&pdev->dev, virq, NULL,
>>>   				max14577_muic_irq_handler,
>>> -				IRQF_NO_SUSPEND,
>>> +				IRQF_NO_SUSPEND | IRQF_ONESHOT,
>>
>> The same with all other patches for IRQF_ONESHOT which are send recently:
>> 1. On what board did you test it?
> 
> I didn't test it.
> 
>> 2. Is this just blind patch from Coccinelle without investigation
>> whether it is needed (hint: it's not needed here, it does not use
>> default primary handler).
> 
> I found the error notice from Coccinelle and I saw the code. Maybe
> I'm mistaken, I think it's needed here. Because handler == NULL and
> thread_fn != NULL, it use irq_default_primary_handler() in
> request_threaded_irq().

No, the primary handler is nested, not default one. Otherwise it would
have absolutely never worked. Therefore you are not fixing anything,
except Coccinelle report.


Best regards,
Krzysztof