diff mbox series

rtc: Fix missing IRQF_ONESHOT as only threaded handler

Message ID 20210416021949.1569-1-zhuguangqing83@gmail.com
State Rejected
Headers show
Series rtc: Fix missing IRQF_ONESHOT as only threaded handler | expand

Commit Message

zhuguangqing83@gmail.com April 16, 2021, 2:19 a.m. UTC
From: Guangqing Zhu <zhuguangqing83@gmail.com>

Coccinelle noticed:
1. drivers/rtc/rtc-s5m.c:810:7-32: ERROR: Threaded IRQ with no primary
   handler requested without IRQF_ONESHOT
2. drivers/rtc/rtc-rk808.c:441:7-32: ERROR: Threaded IRQ with no primary
   handler requested without IRQF_ONESHOT
3. drivers/rtc/rtc-max77686.c:779:7-27: ERROR: Threaded IRQ with no primary
   handler requested without IRQF_ONESHOT
4. drivers/rtc/rtc-tps65910.c:415:7-32: ERROR: Threaded IRQ with no primary
   handler requested without IRQF_ONESHOT
5. drivers/rtc/rtc-lp8788.c:277:8-33: ERROR: Threaded IRQ with no primary
   handler requested without IRQF_ONESHOT
6. drivers/rtc/rtc-max8998.c:283:7-32: ERROR: Threaded IRQ with no primary
   handler requested without IRQF_ONESHOT
7. drivers/rtc/rtc-rc5t583.c:241:7-32: ERROR: Threaded IRQ with no primary
   handler requested without IRQF_ONESHOT
8. drivers/rtc/rtc-max8997.c:495:7-32: ERROR: Threaded IRQ with no primary
   handler requested without IRQF_ONESHOT

Signed-off-by: Guangqing Zhu <zhuguangqing83@gmail.com>
---
 drivers/rtc/rtc-lp8788.c   | 2 +-
 drivers/rtc/rtc-max77686.c | 4 ++--
 drivers/rtc/rtc-max8997.c  | 2 +-
 drivers/rtc/rtc-max8998.c  | 3 ++-
 drivers/rtc/rtc-rc5t583.c  | 2 +-
 drivers/rtc/rtc-rk808.c    | 2 +-
 drivers/rtc/rtc-s5m.c      | 4 ++--
 drivers/rtc/rtc-tps65910.c | 2 +-
 8 files changed, 11 insertions(+), 10 deletions(-)

Comments

Krzysztof Kozlowski April 16, 2021, 8:44 a.m. UTC | #1
On 16/04/2021 04:19, zhuguangqing83@gmail.com wrote:
> From: Guangqing Zhu <zhuguangqing83@gmail.com>
> 
> Coccinelle noticed:
> 1. drivers/rtc/rtc-s5m.c:810:7-32: ERROR: Threaded IRQ with no primary
>    handler requested without IRQF_ONESHOT
> 2. drivers/rtc/rtc-rk808.c:441:7-32: ERROR: Threaded IRQ with no primary
>    handler requested without IRQF_ONESHOT
> 3. drivers/rtc/rtc-max77686.c:779:7-27: ERROR: Threaded IRQ with no primary
>    handler requested without IRQF_ONESHOT
> 4. drivers/rtc/rtc-tps65910.c:415:7-32: ERROR: Threaded IRQ with no primary
>    handler requested without IRQF_ONESHOT
> 5. drivers/rtc/rtc-lp8788.c:277:8-33: ERROR: Threaded IRQ with no primary
>    handler requested without IRQF_ONESHOT
> 6. drivers/rtc/rtc-max8998.c:283:7-32: ERROR: Threaded IRQ with no primary
>    handler requested without IRQF_ONESHOT
> 7. drivers/rtc/rtc-rc5t583.c:241:7-32: ERROR: Threaded IRQ with no primary
>    handler requested without IRQF_ONESHOT
> 8. drivers/rtc/rtc-max8997.c:495:7-32: ERROR: Threaded IRQ with no primary
>    handler requested without IRQF_ONESHOT
> 
> Signed-off-by: Guangqing Zhu <zhuguangqing83@gmail.com>
> ---
>  drivers/rtc/rtc-lp8788.c   | 2 +-
>  drivers/rtc/rtc-max77686.c | 4 ++--
>  drivers/rtc/rtc-max8997.c  | 2 +-
>  drivers/rtc/rtc-max8998.c  | 3 ++-
>  drivers/rtc/rtc-rc5t583.c  | 2 +-
>  drivers/rtc/rtc-rk808.c    | 2 +-
>  drivers/rtc/rtc-s5m.c      | 4 ++--
>  drivers/rtc/rtc-tps65910.c | 2 +-
>  8 files changed, 11 insertions(+), 10 deletions(-)
>

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 might not be needed in all of these
places, because at least some of them do not use default primary handler).
3. If you think otherwise, please explain.

Best regards,
Krzysztof
Krzysztof Kozlowski April 16, 2021, 9:13 a.m. UTC | #2
On 16/04/2021 04:19, zhuguangqing83@gmail.com wrote:
> From: Guangqing Zhu <zhuguangqing83@gmail.com>
> 
> Coccinelle noticed:
> 1. drivers/rtc/rtc-s5m.c:810:7-32: ERROR: Threaded IRQ with no primary
>    handler requested without IRQF_ONESHOT
> 2. drivers/rtc/rtc-rk808.c:441:7-32: ERROR: Threaded IRQ with no primary
>    handler requested without IRQF_ONESHOT
> 3. drivers/rtc/rtc-max77686.c:779:7-27: ERROR: Threaded IRQ with no primary
>    handler requested without IRQF_ONESHOT
> 4. drivers/rtc/rtc-tps65910.c:415:7-32: ERROR: Threaded IRQ with no primary
>    handler requested without IRQF_ONESHOT
> 5. drivers/rtc/rtc-lp8788.c:277:8-33: ERROR: Threaded IRQ with no primary
>    handler requested without IRQF_ONESHOT
> 6. drivers/rtc/rtc-max8998.c:283:7-32: ERROR: Threaded IRQ with no primary
>    handler requested without IRQF_ONESHOT
> 7. drivers/rtc/rtc-rc5t583.c:241:7-32: ERROR: Threaded IRQ with no primary
>    handler requested without IRQF_ONESHOT
> 8. drivers/rtc/rtc-max8997.c:495:7-32: ERROR: Threaded IRQ with no primary
>    handler requested without IRQF_ONESHOT
> 
> Signed-off-by: Guangqing Zhu <zhuguangqing83@gmail.com>
> ---
>  drivers/rtc/rtc-lp8788.c   | 2 +-
>  drivers/rtc/rtc-max77686.c | 4 ++--
>  drivers/rtc/rtc-max8997.c  | 2 +-
>  drivers/rtc/rtc-max8998.c  | 3 ++-
>  drivers/rtc/rtc-rc5t583.c  | 2 +-
>  drivers/rtc/rtc-rk808.c    | 2 +-
>  drivers/rtc/rtc-s5m.c      | 4 ++--

The commit msg suggests in misleading way that there is an issue here to
solve but at least for max* and s5m it is not true. These are nested
interrupts.

I tested *only* the S5M:
Tested-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>

but still I wonder - why this change is needed, except satisfying blind
Coccinelle runs? Does it really bring benefit for the nested interrupts?


Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/drivers/rtc/rtc-lp8788.c b/drivers/rtc/rtc-lp8788.c
index c0b8fbce1082..ebb717ae6c8b 100644
--- a/drivers/rtc/rtc-lp8788.c
+++ b/drivers/rtc/rtc-lp8788.c
@@ -276,7 +276,7 @@  static int lp8788_alarm_irq_register(struct platform_device *pdev,
 
 	return devm_request_threaded_irq(&pdev->dev, rtc->irq, NULL,
 				lp8788_alarm_irq_handler,
-				0, LP8788_ALM_IRQ, rtc);
+				IRQF_ONESHOT, LP8788_ALM_IRQ, rtc);
 }
 
 static int lp8788_rtc_probe(struct platform_device *pdev)
diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index d51cc12114cb..a23825ccf62a 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -776,8 +776,8 @@  static int max77686_rtc_probe(struct platform_device *pdev)
 		goto err_rtc;
 	}
 
-	ret = request_threaded_irq(info->virq, NULL, max77686_rtc_alarm_irq, 0,
-				   "rtc-alarm1", info);
+	ret = request_threaded_irq(info->virq, NULL, max77686_rtc_alarm_irq,
+				   IRQF_ONESHOT, "rtc-alarm1", info);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Failed to request alarm IRQ: %d: %d\n",
 			info->virq, ret);
diff --git a/drivers/rtc/rtc-max8997.c b/drivers/rtc/rtc-max8997.c
index 20e50d9fdf88..15843ed12e36 100644
--- a/drivers/rtc/rtc-max8997.c
+++ b/drivers/rtc/rtc-max8997.c
@@ -493,7 +493,7 @@  static int max8997_rtc_probe(struct platform_device *pdev)
 	info->virq = virq;
 
 	ret = devm_request_threaded_irq(&pdev->dev, virq, NULL,
-				max8997_rtc_alarm_irq, 0,
+				max8997_rtc_alarm_irq, IRQF_ONESHOT,
 				"rtc-alarm0", info);
 	if (ret < 0)
 		dev_err(&pdev->dev, "Failed to request alarm IRQ: %d: %d\n",
diff --git a/drivers/rtc/rtc-max8998.c b/drivers/rtc/rtc-max8998.c
index c873b4509b3c..28c5b367f633 100644
--- a/drivers/rtc/rtc-max8998.c
+++ b/drivers/rtc/rtc-max8998.c
@@ -281,7 +281,8 @@  static int max8998_rtc_probe(struct platform_device *pdev)
 	}
 
 	ret = devm_request_threaded_irq(&pdev->dev, info->irq, NULL,
-				max8998_rtc_alarm_irq, 0, "rtc-alarm0", info);
+				max8998_rtc_alarm_irq, IRQF_ONESHOT,
+				"rtc-alarm0", info);
 
 	if (ret < 0)
 		dev_err(&pdev->dev, "Failed to request alarm IRQ: %d: %d\n",
diff --git a/drivers/rtc/rtc-rc5t583.c b/drivers/rtc/rtc-rc5t583.c
index 18684a7026c4..d9f4e0d4d943 100644
--- a/drivers/rtc/rtc-rc5t583.c
+++ b/drivers/rtc/rtc-rc5t583.c
@@ -239,7 +239,7 @@  static int rc5t583_rtc_probe(struct platform_device *pdev)
 
 	irq += RC5T583_IRQ_YALE;
 	ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
-		rc5t583_rtc_interrupt, IRQF_TRIGGER_LOW,
+		rc5t583_rtc_interrupt, IRQF_TRIGGER_LOW | IRQF_ONESHOT,
 		"rtc-rc5t583", &pdev->dev);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "IRQ is not free.\n");
diff --git a/drivers/rtc/rtc-rk808.c b/drivers/rtc/rtc-rk808.c
index e920da8c08da..753583cff5d4 100644
--- a/drivers/rtc/rtc-rk808.c
+++ b/drivers/rtc/rtc-rk808.c
@@ -439,7 +439,7 @@  static int rk808_rtc_probe(struct platform_device *pdev)
 
 	/* request alarm irq of rk808 */
 	ret = devm_request_threaded_irq(&pdev->dev, rk808_rtc->irq, NULL,
-					rk808_alarm_irq, 0,
+					rk808_alarm_irq, IRQF_ONESHOT,
 					"RTC alarm", rk808_rtc);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to request alarm IRQ %d: %d\n",
diff --git a/drivers/rtc/rtc-s5m.c b/drivers/rtc/rtc-s5m.c
index 80b66f16db89..56e82987b4a2 100644
--- a/drivers/rtc/rtc-s5m.c
+++ b/drivers/rtc/rtc-s5m.c
@@ -808,8 +808,8 @@  static int s5m_rtc_probe(struct platform_device *pdev)
 	}
 
 	ret = devm_request_threaded_irq(&pdev->dev, info->irq, NULL,
-					s5m_rtc_alarm_irq, 0, "rtc-alarm0",
-					info);
+					s5m_rtc_alarm_irq, IRQF_ONESHOT,
+					"rtc-alarm0", info);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Failed to request alarm IRQ: %d: %d\n",
 			info->irq, ret);
diff --git a/drivers/rtc/rtc-tps65910.c b/drivers/rtc/rtc-tps65910.c
index bc89c62ccb9b..6ab67820d77c 100644
--- a/drivers/rtc/rtc-tps65910.c
+++ b/drivers/rtc/rtc-tps65910.c
@@ -413,7 +413,7 @@  static int tps65910_rtc_probe(struct platform_device *pdev)
 	}
 
 	ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
-		tps65910_rtc_interrupt, IRQF_TRIGGER_LOW,
+		tps65910_rtc_interrupt, IRQF_TRIGGER_LOW | IRQF_ONESHOT,
 		dev_name(&pdev->dev), &pdev->dev);
 	if (ret < 0)
 		irq = -1;