| Message ID | 20230529162056.3786301-1-uwu@icenowy.me (mailing list archive) |
|---|---|
| Headers | show |
| Series | thermal/drivers/mediatek: fix a regression affecting other subsystems | expand |
在 2023-06-13星期二的 10:44 +0200，Daniel Lezcano写道： > On 29/05/2023 18:20, Icenowy Zheng wrote: > > In the commit I reverted as the first commit of this patchset, the > > of_iomap function call, which allows multiple mapping of the same > > physical memory space, is replaced to calling devm_of_iomap, which > > registers exclusivity, and on my system (mt8173-elm), preventing > > display > > from working. > > > > So I reverted it, and to really solve the problem that the original > > commit wants to solve, I read the source of auxadc-thermal and > > realized > > that the address of these two memory blocks are not saved after > > probe, > > and they're only used when initializing the thermal sensors. This > > leads > > to my final fix, which is the second commit here, that adds > > of_iounmap > > just to the probe function. > > > > Icenowy Zheng (2): > > Revert "thermal/drivers/mediatek: Use devm_of_iomap to avoid > > resource > > leak in mtk_thermal_probe" > > thermal/drivers/mediatek: unmap foreign MMIO after probing > > > > drivers/thermal/mediatek/auxadc_thermal.c | 46 ++++++++++++------ > > ----- > > 1 file changed, 24 insertions(+), 22 deletions(-) > > I'll apply only the revert and let you revisit the patch 2 which > could > be improved. Sorry, is the first patch applied? I didn't see it in any kernel trees... I have no current idea about how to fix in patch 2. >
On 22/07/2023 14:13, Icenowy Zheng wrote: > 在 2023-06-13星期二的 10:44 +0200，Daniel Lezcano写道： >> On 29/05/2023 18:20, Icenowy Zheng wrote: >>> In the commit I reverted as the first commit of this patchset, the >>> of_iomap function call, which allows multiple mapping of the same >>> physical memory space, is replaced to calling devm_of_iomap, which >>> registers exclusivity, and on my system (mt8173-elm), preventing >>> display >>> from working. >>> >>> So I reverted it, and to really solve the problem that the original >>> commit wants to solve, I read the source of auxadc-thermal and >>> realized >>> that the address of these two memory blocks are not saved after >>> probe, >>> and they're only used when initializing the thermal sensors. This >>> leads >>> to my final fix, which is the second commit here, that adds >>> of_iounmap >>> just to the probe function. >>> >>> Icenowy Zheng (2): >>> Revert "thermal/drivers/mediatek: Use devm_of_iomap to avoid >>> resource >>> leak in mtk_thermal_probe" >>> thermal/drivers/mediatek: unmap foreign MMIO after probing >>> >>> drivers/thermal/mediatek/auxadc_thermal.c | 46 ++++++++++++------ >>> ----- >>> 1 file changed, 24 insertions(+), 22 deletions(-) >> >> I'll apply only the revert and let you revisit the patch 2 which >> could >> be improved. > > Sorry, is the first patch applied? I didn't see it in any kernel > trees... Yes, sorry, I got another patch meanwhile which provided the same revert with more tags https://lore.kernel.org/r/20230525121811.3360268-1-ricardo.canuelo@collabora.com
On Tue, Jun 13, 2023 at 10:44:51AM +0200, Daniel Lezcano wrote: > On 29/05/2023 18:20, Icenowy Zheng wrote: > > In the commit I reverted as the first commit of this patchset, the > > of_iomap function call, which allows multiple mapping of the same > > physical memory space, is replaced to calling devm_of_iomap, which > > registers exclusivity, and on my system (mt8173-elm), preventing display > > from working. > > > > So I reverted it, and to really solve the problem that the original > > commit wants to solve, I read the source of auxadc-thermal and realized > > that the address of these two memory blocks are not saved after probe, > > and they're only used when initializing the thermal sensors. This leads > > to my final fix, which is the second commit here, that adds of_iounmap > > just to the probe function. > > > > Icenowy Zheng (2): > > Revert "thermal/drivers/mediatek: Use devm_of_iomap to avoid resource > > leak in mtk_thermal_probe" > > thermal/drivers/mediatek: unmap foreign MMIO after probing > > > > drivers/thermal/mediatek/auxadc_thermal.c | 46 ++++++++++++----------- > > 1 file changed, 24 insertions(+), 22 deletions(-) > > I'll apply only the revert and let you revisit the patch 2 which could be > improved. What's the issue with patch 2/2? It looks okay to me. regards, dan carpenter
In the commit I reverted as the first commit of this patchset, the of_iomap function call, which allows multiple mapping of the same physical memory space, is replaced to calling devm_of_iomap, which registers exclusivity, and on my system (mt8173-elm), preventing display from working. So I reverted it, and to really solve the problem that the original commit wants to solve, I read the source of auxadc-thermal and realized that the address of these two memory blocks are not saved after probe, and they're only used when initializing the thermal sensors. This leads to my final fix, which is the second commit here, that adds of_iounmap just to the probe function. Icenowy Zheng (2): Revert "thermal/drivers/mediatek: Use devm_of_iomap to avoid resource leak in mtk_thermal_probe" thermal/drivers/mediatek: unmap foreign MMIO after probing drivers/thermal/mediatek/auxadc_thermal.c | 46 ++++++++++++----------- 1 file changed, 24 insertions(+), 22 deletions(-)