* [PATCH v2 0/2] lvts_thermal: Make reset optional for MT8196 @ 2026-07-21 10:52 AngeloGioacchino Del Regno 2026-07-21 10:52 ` [PATCH v2 1/2] dt-bindings: thermal: mediatek: Make resets " AngeloGioacchino Del Regno 2026-07-21 10:52 ` [PATCH v2 2/2] thermal/drivers/mediatek/lvts_thermal: Make reset " AngeloGioacchino Del Regno 0 siblings, 2 replies; 8+ messages in thread From: AngeloGioacchino Del Regno @ 2026-07-21 10:52 UTC (permalink / raw) To: rafael Cc: daniel.lezcano, rui.zhang, lukasz.luba, robh, krzk+dt, conor+dt, p.zabel, matthias.bgg, angelogioacchino.delregno, laura.nao, fshao, wenst, jiapeng.chong, frank-w, bchihi, linux-pm, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek, kernel Changes in v2: - Added change in bindings to allow resets to be optional on MT8196 - Made the code change simpler and without paranoid nonsense checks AngeloGioacchino Del Regno (2): dt-bindings: thermal: mediatek: Make resets optional for MT8196 thermal/drivers/mediatek/lvts_thermal: Make reset optional for MT8196 .../bindings/thermal/mediatek,lvts-thermal.yaml | 13 ++++++++++++- drivers/thermal/mediatek/lvts_thermal.c | 15 ++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) -- 2.55.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/2] dt-bindings: thermal: mediatek: Make resets optional for MT8196 2026-07-21 10:52 [PATCH v2 0/2] lvts_thermal: Make reset optional for MT8196 AngeloGioacchino Del Regno @ 2026-07-21 10:52 ` AngeloGioacchino Del Regno 2026-07-21 11:01 ` sashiko-bot 2026-07-22 3:35 ` Chen-Yu Tsai 2026-07-21 10:52 ` [PATCH v2 2/2] thermal/drivers/mediatek/lvts_thermal: Make reset " AngeloGioacchino Del Regno 1 sibling, 2 replies; 8+ messages in thread From: AngeloGioacchino Del Regno @ 2026-07-21 10:52 UTC (permalink / raw) To: rafael Cc: daniel.lezcano, rui.zhang, lukasz.luba, robh, krzk+dt, conor+dt, p.zabel, matthias.bgg, angelogioacchino.delregno, laura.nao, fshao, wenst, jiapeng.chong, frank-w, bchihi, linux-pm, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek, kernel Both LVTS-AP and LVTS-MCU may be shared with SoC-internal MCUs running some sort of firmware that checks thermals in order to scale frequency, or to take action for critical SoC thermal protection - and this is seen on most MT8196 boards. Make resets optional, as doing a HW reset on such boards will result in either an immediate thermal protect shutdown or in a rather important and usually permanent system slowdown. Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> --- .../bindings/thermal/mediatek,lvts-thermal.yaml | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml b/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml index 975235130670..29f431fcdcd5 100644 --- a/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml +++ b/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml @@ -94,12 +94,23 @@ allOf: nvmem-cell-names: minItems: 2 + - if: + properties: + compatible: + not: + contains: + enum: + - mediatek,mt8196-lvts-ap + - mediatek,mt8196-lvts-mcu + then: + required: + - resets + required: - compatible - reg - interrupts - clocks - - resets - nvmem-cells - nvmem-cell-names -- 2.55.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: thermal: mediatek: Make resets optional for MT8196 2026-07-21 10:52 ` [PATCH v2 1/2] dt-bindings: thermal: mediatek: Make resets " AngeloGioacchino Del Regno @ 2026-07-21 11:01 ` sashiko-bot 2026-07-21 11:20 ` AngeloGioacchino Del Regno 2026-07-22 3:35 ` Chen-Yu Tsai 1 sibling, 1 reply; 8+ messages in thread From: sashiko-bot @ 2026-07-21 11:01 UTC (permalink / raw) To: AngeloGioacchino Del Regno; +Cc: devicetree, robh, conor+dt Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Driver-Binding Mismatch: The `lvts_thermal` driver unconditionally requires the `resets` property, which causes probe failure if omitted on MT8196. -- --- Patch [1]: [PATCH v2 1/2] dt-bindings: thermal: mediatek: Make resets optional for MT8196 --- commit 8c88985c870e0f500a0c1636d7b632cee772d3c6 Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> dt-bindings: thermal: mediatek: Make resets optional for MT8196 This commit updates the device tree binding for mediatek,lvts-thermal to make the resets property optional for the MT8196 AP and MCU platforms. This prevents hardware resets that could result in thermal protect shutdowns or permanent system slowdowns. > diff --git a/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml b/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml > index 9752351306703..29f431fcdcd5f 100644 > --- a/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml > +++ b/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml > @@ -94,12 +94,23 @@ allOf: > nvmem-cell-names: > minItems: 2 > > + - if: > + properties: > + compatible: > + not: > + contains: > + enum: > + - mediatek,mt8196-lvts-ap > + - mediatek,mt8196-lvts-mcu > + then: > + required: > + - resets > + > required: > - compatible > - reg > - interrupts > - clocks > - - resets [Severity: High] Will omitting the resets property cause a probe regression? Looking at lvts_probe() in drivers/thermal/mediatek/lvts_thermal.c, the driver unconditionally requires the reset control: lvts_td->reset = devm_reset_control_get_by_index(dev, 0); if (IS_ERR(lvts_td->reset)) return dev_err_probe(dev, PTR_ERR(lvts_td->reset), "Failed to get reset control\n"); If the property is omitted to conform to this new schema on MT8196 boards, devm_reset_control_get_by_index() returns ERR_PTR(-ENOENT) and the driver aborts probing. Should the driver be updated to use devm_reset_control_get_optional_exclusive() or similar to match this binding change? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260721105230.101906-1-angelogioacchino.delregno@collabora.com?part=1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: thermal: mediatek: Make resets optional for MT8196 2026-07-21 11:01 ` sashiko-bot @ 2026-07-21 11:20 ` AngeloGioacchino Del Regno 0 siblings, 0 replies; 8+ messages in thread From: AngeloGioacchino Del Regno @ 2026-07-21 11:20 UTC (permalink / raw) To: sashiko-reviews; +Cc: devicetree, robh, conor+dt On 7/21/26 13:01, sashiko-bot@kernel.org wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [High] Driver-Binding Mismatch: The `lvts_thermal` driver unconditionally requires the `resets` property, which causes probe failure if omitted on MT8196. No, that's not true. > -- > > --- Patch [1]: [PATCH v2 1/2] dt-bindings: thermal: mediatek: Make resets optional for MT8196 --- > commit 8c88985c870e0f500a0c1636d7b632cee772d3c6 > Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > > dt-bindings: thermal: mediatek: Make resets optional for MT8196 > > This commit updates the device tree binding for mediatek,lvts-thermal to > make the resets property optional for the MT8196 AP and MCU platforms. > This prevents hardware resets that could result in thermal protect > shutdowns or permanent system slowdowns. > >> diff --git a/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml b/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml >> index 9752351306703..29f431fcdcd5f 100644 >> --- a/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml >> +++ b/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml >> @@ -94,12 +94,23 @@ allOf: >> nvmem-cell-names: >> minItems: 2 >> >> + - if: >> + properties: >> + compatible: >> + not: >> + contains: >> + enum: >> + - mediatek,mt8196-lvts-ap >> + - mediatek,mt8196-lvts-mcu >> + then: >> + required: >> + - resets >> + >> required: >> - compatible >> - reg >> - interrupts >> - clocks >> - - resets > > [Severity: High] > Will omitting the resets property cause a probe regression? > > Looking at lvts_probe() in drivers/thermal/mediatek/lvts_thermal.c, the driver > unconditionally requires the reset control: > > lvts_td->reset = devm_reset_control_get_by_index(dev, 0); The driver uses devm_reset_control_get_optional_exclusive() in commit 2. This is a false positive. > if (IS_ERR(lvts_td->reset)) > return dev_err_probe(dev, PTR_ERR(lvts_td->reset), "Failed to get reset control\n"); > > If the property is omitted to conform to this new schema on MT8196 boards, > devm_reset_control_get_by_index() returns ERR_PTR(-ENOENT) and the driver > aborts probing. > > Should the driver be updated to use devm_reset_control_get_optional_exclusive() > or similar to match this binding change? > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: thermal: mediatek: Make resets optional for MT8196 2026-07-21 10:52 ` [PATCH v2 1/2] dt-bindings: thermal: mediatek: Make resets " AngeloGioacchino Del Regno 2026-07-21 11:01 ` sashiko-bot @ 2026-07-22 3:35 ` Chen-Yu Tsai 1 sibling, 0 replies; 8+ messages in thread From: Chen-Yu Tsai @ 2026-07-22 3:35 UTC (permalink / raw) To: AngeloGioacchino Del Regno Cc: rafael, daniel.lezcano, rui.zhang, lukasz.luba, robh, krzk+dt, conor+dt, p.zabel, matthias.bgg, laura.nao, fshao, jiapeng.chong, frank-w, bchihi, linux-pm, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek, kernel On Tue, Jul 21, 2026 at 6:52 PM AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote: > > Both LVTS-AP and LVTS-MCU may be shared with SoC-internal MCUs > running some sort of firmware that checks thermals in order to > scale frequency, or to take action for critical SoC thermal > protection - and this is seen on most MT8196 boards. Yes the number of MCUs on this SoC is a bit crazy, and the OS has almost no control over them. :D > Make resets optional, as doing a HW reset on such boards will > result in either an immediate thermal protect shutdown or in > a rather important and usually permanent system slowdown. > > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org> > --- > .../bindings/thermal/mediatek,lvts-thermal.yaml | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml b/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml > index 975235130670..29f431fcdcd5 100644 > --- a/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml > +++ b/Documentation/devicetree/bindings/thermal/mediatek,lvts-thermal.yaml > @@ -94,12 +94,23 @@ allOf: > nvmem-cell-names: > minItems: 2 > > + - if: > + properties: > + compatible: > + not: > + contains: > + enum: > + - mediatek,mt8196-lvts-ap > + - mediatek,mt8196-lvts-mcu > + then: > + required: > + - resets > + > required: > - compatible > - reg > - interrupts > - clocks > - - resets > - nvmem-cells > - nvmem-cell-names > > -- > 2.55.0 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] thermal/drivers/mediatek/lvts_thermal: Make reset optional for MT8196 2026-07-21 10:52 [PATCH v2 0/2] lvts_thermal: Make reset optional for MT8196 AngeloGioacchino Del Regno 2026-07-21 10:52 ` [PATCH v2 1/2] dt-bindings: thermal: mediatek: Make resets " AngeloGioacchino Del Regno @ 2026-07-21 10:52 ` AngeloGioacchino Del Regno 2026-07-22 3:37 ` Chen-Yu Tsai 2026-07-22 6:47 ` Philipp Zabel 1 sibling, 2 replies; 8+ messages in thread From: AngeloGioacchino Del Regno @ 2026-07-21 10:52 UTC (permalink / raw) To: rafael Cc: daniel.lezcano, rui.zhang, lukasz.luba, robh, krzk+dt, conor+dt, p.zabel, matthias.bgg, angelogioacchino.delregno, laura.nao, fshao, wenst, jiapeng.chong, frank-w, bchihi, linux-pm, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek, kernel Depending on the SoC+Firmware combination, the LVTS hardware may be may be actively used by one or even multiple concurrent MCUs! In this case, resetting it may produce either a severe slowdown of the entire system, or even a thermal protection AP reset, as some MCU(s) may be reading a very high or very low temperature while the LVTS is being reset. On those, don't fail if no reset is found as that may be omitted on purpose, but still check if there's one, because some board(s) may be running on a different bootchain with reduced firmwares or using firmwares with reduced functionality. So, use devm_reset_control_get_optional_exclusive() instead, as the LVTS controller always had only one reset and retrieving that by index, specifically, always made little sense anyway. Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> --- drivers/thermal/mediatek/lvts_thermal.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c index 92711896ce24..8b779fc5e970 100644 --- a/drivers/thermal/mediatek/lvts_thermal.c +++ b/drivers/thermal/mediatek/lvts_thermal.c @@ -1473,7 +1473,20 @@ static int lvts_probe(struct platform_device *pdev) if (IS_ERR(lvts_td->base)) return dev_err_probe(dev, PTR_ERR(lvts_td->base), "Failed to map io resource\n"); - lvts_td->reset = devm_reset_control_get_by_index(dev, 0); + /* + * Depending on the SoC+Firmware combination, the LVTS hardware may be + * may be actively used by one or even multiple concurrent MCUs! + * In this case, resetting it may produce either a severe slowdown of + * the entire system, or even a thermal protection AP reset, as some + * MCU(s) may be reading a very high or very low temperature while the + * LVTS is being reset. + * + * On those, don't fail if no reset is found as that may be omitted on + * purpose, but still check if there's one, because some board(s) may + * be running on a different bootchain with reduced firmwares or using + * firmwares with reduced functionality. + */ + lvts_td->reset = devm_reset_control_get_optional_exclusive(dev, NULL); if (IS_ERR(lvts_td->reset)) return dev_err_probe(dev, PTR_ERR(lvts_td->reset), "Failed to get reset control\n"); -- 2.55.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] thermal/drivers/mediatek/lvts_thermal: Make reset optional for MT8196 2026-07-21 10:52 ` [PATCH v2 2/2] thermal/drivers/mediatek/lvts_thermal: Make reset " AngeloGioacchino Del Regno @ 2026-07-22 3:37 ` Chen-Yu Tsai 2026-07-22 6:47 ` Philipp Zabel 1 sibling, 0 replies; 8+ messages in thread From: Chen-Yu Tsai @ 2026-07-22 3:37 UTC (permalink / raw) To: AngeloGioacchino Del Regno Cc: rafael, daniel.lezcano, rui.zhang, lukasz.luba, robh, krzk+dt, conor+dt, p.zabel, matthias.bgg, laura.nao, fshao, jiapeng.chong, frank-w, bchihi, linux-pm, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek, kernel On Tue, Jul 21, 2026 at 6:52 PM AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote: > > Depending on the SoC+Firmware combination, the LVTS hardware may be > may be actively used by one or even multiple concurrent MCUs! > In this case, resetting it may produce either a severe slowdown of > the entire system, or even a thermal protection AP reset, as some > MCU(s) may be reading a very high or very low temperature while the > LVTS is being reset. > > On those, don't fail if no reset is found as that may be omitted on > purpose, but still check if there's one, because some board(s) may > be running on a different bootchain with reduced firmwares or using > firmwares with reduced functionality. > > So, use devm_reset_control_get_optional_exclusive() instead, as the > LVTS controller always had only one reset and retrieving that by > index, specifically, always made little sense anyway. > > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] thermal/drivers/mediatek/lvts_thermal: Make reset optional for MT8196 2026-07-21 10:52 ` [PATCH v2 2/2] thermal/drivers/mediatek/lvts_thermal: Make reset " AngeloGioacchino Del Regno 2026-07-22 3:37 ` Chen-Yu Tsai @ 2026-07-22 6:47 ` Philipp Zabel 1 sibling, 0 replies; 8+ messages in thread From: Philipp Zabel @ 2026-07-22 6:47 UTC (permalink / raw) To: AngeloGioacchino Del Regno, rafael Cc: daniel.lezcano, rui.zhang, lukasz.luba, robh, krzk+dt, conor+dt, matthias.bgg, laura.nao, fshao, wenst, jiapeng.chong, frank-w, bchihi, linux-pm, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek, kernel On Di, 2026-07-21 at 12:52 +0200, AngeloGioacchino Del Regno wrote: > Depending on the SoC+Firmware combination, the LVTS hardware may be > may be actively used by one or even multiple concurrent MCUs! > In this case, resetting it may produce either a severe slowdown of > the entire system, or even a thermal protection AP reset, as some > MCU(s) may be reading a very high or very low temperature while the > LVTS is being reset. > > On those, don't fail if no reset is found as that may be omitted on > purpose, but still check if there's one, because some board(s) may > be running on a different bootchain with reduced firmwares or using > firmwares with reduced functionality. > > So, use devm_reset_control_get_optional_exclusive() instead, as the > LVTS controller always had only one reset and retrieving that by > index, specifically, always made little sense anyway. > > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > --- > drivers/thermal/mediatek/lvts_thermal.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c > index 92711896ce24..8b779fc5e970 100644 > --- a/drivers/thermal/mediatek/lvts_thermal.c > +++ b/drivers/thermal/mediatek/lvts_thermal.c > @@ -1473,7 +1473,20 @@ static int lvts_probe(struct platform_device *pdev) > if (IS_ERR(lvts_td->base)) > return dev_err_probe(dev, PTR_ERR(lvts_td->base), "Failed to map io resource\n"); > > - lvts_td->reset = devm_reset_control_get_by_index(dev, 0); > + /* > + * Depending on the SoC+Firmware combination, the LVTS hardware may be > + * may be actively used by one or even multiple concurrent MCUs! > + * In this case, resetting it may produce either a severe slowdown of > + * the entire system, or even a thermal protection AP reset, as some > + * MCU(s) may be reading a very high or very low temperature while the > + * LVTS is being reset. > + * > + * On those, don't fail if no reset is found as that may be omitted on > + * purpose, but still check if there's one, because some board(s) may > + * be running on a different bootchain with reduced firmwares or using > + * firmwares with reduced functionality. > + */ > + lvts_td->reset = devm_reset_control_get_optional_exclusive(dev, NULL); Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de> regards Philipp ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-07-22 6:47 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-21 10:52 [PATCH v2 0/2] lvts_thermal: Make reset optional for MT8196 AngeloGioacchino Del Regno 2026-07-21 10:52 ` [PATCH v2 1/2] dt-bindings: thermal: mediatek: Make resets " AngeloGioacchino Del Regno 2026-07-21 11:01 ` sashiko-bot 2026-07-21 11:20 ` AngeloGioacchino Del Regno 2026-07-22 3:35 ` Chen-Yu Tsai 2026-07-21 10:52 ` [PATCH v2 2/2] thermal/drivers/mediatek/lvts_thermal: Make reset " AngeloGioacchino Del Regno 2026-07-22 3:37 ` Chen-Yu Tsai 2026-07-22 6:47 ` Philipp Zabel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox