Linux-RTC Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] rtc: Fix missing IRQF_ONESHOT as only threaded handler
@ 2021-04-16  2:19 zhuguangqing83
  2021-04-16  8:44 ` Krzysztof Kozlowski
  2021-04-16  9:13 ` Krzysztof Kozlowski
  0 siblings, 2 replies; 3+ messages in thread
From: zhuguangqing83 @ 2021-04-16  2:19 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni, Chanwoo Choi,
	Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz
  Cc: linux-rtc, linux-kernel, linux-samsung-soc, Guangqing Zhu

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(-)

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;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] rtc: Fix missing IRQF_ONESHOT as only threaded handler
  2021-04-16  2:19 [PATCH] rtc: Fix missing IRQF_ONESHOT as only threaded handler zhuguangqing83
@ 2021-04-16  8:44 ` Krzysztof Kozlowski
  2021-04-16  9:13 ` Krzysztof Kozlowski
  1 sibling, 0 replies; 3+ messages in thread
From: Krzysztof Kozlowski @ 2021-04-16  8:44 UTC (permalink / raw)
  To: zhuguangqing83, Alessandro Zummo, Alexandre Belloni,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz
  Cc: linux-rtc, linux-kernel, linux-samsung-soc

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] rtc: Fix missing IRQF_ONESHOT as only threaded handler
  2021-04-16  2:19 [PATCH] rtc: Fix missing IRQF_ONESHOT as only threaded handler zhuguangqing83
  2021-04-16  8:44 ` Krzysztof Kozlowski
@ 2021-04-16  9:13 ` Krzysztof Kozlowski
  1 sibling, 0 replies; 3+ messages in thread
From: Krzysztof Kozlowski @ 2021-04-16  9:13 UTC (permalink / raw)
  To: zhuguangqing83, Alessandro Zummo, Alexandre Belloni,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz
  Cc: linux-rtc, linux-kernel, linux-samsung-soc

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-16  2:19 [PATCH] rtc: Fix missing IRQF_ONESHOT as only threaded handler zhuguangqing83
2021-04-16  8:44 ` Krzysztof Kozlowski
2021-04-16  9:13 ` Krzysztof Kozlowski

Linux-RTC Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-rtc/0 linux-rtc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-rtc linux-rtc/ https://lore.kernel.org/linux-rtc \
		linux-rtc@vger.kernel.org
	public-inbox-index linux-rtc

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-rtc


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git