Messages in this thread |  | | From | Guenter Roeck <> | Subject | Re: [PATCH v16 3/7] soc: mediatek: SVS: introduce MTK SVS engine | Date | Thu, 13 May 2021 20:33:14 -0700 |
| |
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: >> On Wed, Apr 28, 2021 at 02:54:36PM +0800, Roger Lu wrote: >>> 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 <roger.lu@mediatek.com> >>> --- >>> 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 >>> >> >> [ ... ] >> >>> + >>> + 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
|  |