On 5/13/21 8:10 PM, Roger Lu wrote:

Hi Guenter,

Sorry for the late reply and thanks for the notice.

On Wed, 2021-05-05 at 21:51 -0700, Guenter Roeck wrote:

quoted

On Wed, Apr 28, 2021 at 02:54:36PM +0800, Roger Lu wrote:

quoted

The Smart Voltage Scaling(SVS) engine is a piece of hardware
which calculates suitable SVS bank voltages to OPP voltage table.
Then, DVFS driver could apply those SVS bank voltages to PMIC/Buck
when receiving OPP_EVENT_ADJUST_VOLTAGE.

Signed-off-by: Roger Lu <redacted>
---
  drivers/soc/mediatek/Kconfig   |   10 +
  drivers/soc/mediatek/Makefile  |    1 +
  drivers/soc/mediatek/mtk-svs.c | 1723
++++++++++++++++++++++++++++++++
  3 files changed, 1734 insertions(+)
  create mode 100644 drivers/soc/mediatek/mtk-svs.c

[ ... ]

quoted

+
+	svsp_irq = irq_of_parse_and_map(svsp->dev->of_node, 0);
+	ret = devm_request_threaded_irq(svsp->dev, svsp_irq, NULL,
svs_isr,
+					svsp->irqflags, svsp->name,
svsp);

0-day reports:

drivers/soc/mediatek/mtk-svs.c:1663:7-32: ERROR:
	Threaded IRQ with no primary handler requested without
IRQF_ONESHOT

I would be a bit concerned about this. There is no primary (hard)
interrupt handler, meaning the hard interrupt may be re-enabled after
the default hard interrupt handler runs. This might result in endless
interrupts.

Oh, we add IRQF_ONESHOT in "svs_get_svs_mt8183_platform_data()" for
threaded irq. So, please kindly let us know if we need to set more
flags or any other potential risks we should be aware. Thanks in
advance.

After reviewing the code, I think this was actually a false alarm,
at least if svsp->irqflags always includes IRQF_ONESHOT.
The code is kind of unusual, though. Unless I am missing something,
svsp->irqflags is only set in one place and it is always set
to IRQF_TRIGGER_LOW | IRQF_ONESHOT. If there is a remote chance
that the flag is ever different, it would have been better (and less
confusing) to specify IRQF_ONESHOT directly when requesting the
interrupt (because it is always needed, no matter which SOC).
If the flags are always the same, there is no reason for having
the svsp->irqflags variable in the first place.

Thanks,
Guenter