| Message ID | trinity-f3e7d8e0-2e93-4e84-a489-3993c819d2c3-1693488871086@3c-app-gmx-bs12 (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Aw: regression with 33140e668b10 thermal/drivers/mediatek: Control buffer enablement tweaks | expand |
Hi Frank, thanks for reporting and investigating the issue. On 31/08/2023 15:34, Frank Wunderlich wrote: > Hi, > > looked a bit deeper into it and it looks like i only need to add the fields in the mtk_thermal_data struct > > --- a/drivers/thermal/mediatek/auxadc_thermal.c > +++ b/drivers/thermal/mediatek/auxadc_thermal.c > @@ -690,6 +690,9 @@ static const struct mtk_thermal_data mt7986_thermal_data = { > .adcpnp = mt7986_adcpnp, > .sensor_mux_values = mt7986_mux_values, > .version = MTK_THERMAL_V3, > + .apmixed_buffer_ctl_reg = APMIXED_SYS_TS_CON1, > + .apmixed_buffer_ctl_mask = GENMASK(31, 6) | BIT(3), > + .apmixed_buffer_ctl_set = BIT(0), > }; > > in my quick test the temprature can be read again and i'm near room temperature...i though it was a bit higher before, but as far as i understand the code, the values were fixed before and only made it configurable. > > if someone can confirm that i'm right, i can send official patch. At the first glance, it seems the proposed change is correct and could be proposed as a hot fix. However, the conditions with the version and the apmixed_buffer_ctl_reg looks a bit fuzzy. Markus, can you revisit this part of code and consolidate the configurable approach ?
Hi Frank and Daniel, On Tue, Sep 12, 2023 at 02:57:45PM +0200, Daniel Lezcano wrote: > > Hi Frank, > > thanks for reporting and investigating the issue. > > > On 31/08/2023 15:34, Frank Wunderlich wrote: > > Hi, > > > > looked a bit deeper into it and it looks like i only need to add the fields in the mtk_thermal_data struct > > > > --- a/drivers/thermal/mediatek/auxadc_thermal.c > > +++ b/drivers/thermal/mediatek/auxadc_thermal.c > > @@ -690,6 +690,9 @@ static const struct mtk_thermal_data mt7986_thermal_data = { > > .adcpnp = mt7986_adcpnp, > > .sensor_mux_values = mt7986_mux_values, > > .version = MTK_THERMAL_V3, > > + .apmixed_buffer_ctl_reg = APMIXED_SYS_TS_CON1, > > + .apmixed_buffer_ctl_mask = GENMASK(31, 6) | BIT(3), > > + .apmixed_buffer_ctl_set = BIT(0), > > }; > > > > in my quick test the temprature can be read again and i'm near room temperature...i though it was a bit higher before, but as far as i understand the code, the values were fixed before and only made it configurable. > > > > if someone can confirm that i'm right, i can send official patch. Sorry for the delay, this was in my inbox but somehow not on my todo list. The patch looks correct. I think I simply missed adding the fields for MTK_THERMAL_V3 chips. Sorry! > > At the first glance, it seems the proposed change is correct and could be > proposed as a hot fix. > > However, the conditions with the version and the apmixed_buffer_ctl_reg > looks a bit fuzzy. > > Markus, can you revisit this part of code and consolidate the configurable > approach ? Yes, it indeed looks like I broke another condition there. I will give it another close look and send a fix. Best, Markus
Am 12. September 2023 15:46:17 MESZ schrieb Markus Schneider-Pargmann <msp@baylibre.com>: >Hi Frank and Daniel, > >On Tue, Sep 12, 2023 at 02:57:45PM +0200, Daniel Lezcano wrote: >> >> Hi Frank, >> >> thanks for reporting and investigating the issue. >> >> >> On 31/08/2023 15:34, Frank Wunderlich wrote: >> > Hi, >> > >> > looked a bit deeper into it and it looks like i only need to add the fields in the mtk_thermal_data struct >> > >> > --- a/drivers/thermal/mediatek/auxadc_thermal.c >> > +++ b/drivers/thermal/mediatek/auxadc_thermal.c >> > @@ -690,6 +690,9 @@ static const struct mtk_thermal_data mt7986_thermal_data = { >> > .adcpnp = mt7986_adcpnp, >> > .sensor_mux_values = mt7986_mux_values, >> > .version = MTK_THERMAL_V3, >> > + .apmixed_buffer_ctl_reg = APMIXED_SYS_TS_CON1, >> > + .apmixed_buffer_ctl_mask = GENMASK(31, 6) | BIT(3), >> > + .apmixed_buffer_ctl_set = BIT(0), >> > }; >> > >> > in my quick test the temprature can be read again and i'm near room temperature...i though it was a bit higher before, but as far as i understand the code, the values were fixed before and only made it configurable. >> > >> > if someone can confirm that i'm right, i can send official patch. > >Sorry for the delay, this was in my inbox but somehow not on my todo >list. > >The patch looks correct. I think I simply missed adding the fields for >MTK_THERMAL_V3 chips. Sorry! > >> >> At the first glance, it seems the proposed change is correct and could be >> proposed as a hot fix. >> >> However, the conditions with the version and the apmixed_buffer_ctl_reg >> looks a bit fuzzy. >> >> Markus, can you revisit this part of code and consolidate the configurable >> approach ? > >Yes, it indeed looks like I broke another condition there. I will give >it another close look and send a fix. > >Best, >Markus Hi I already sent the Patch below https://patchwork.kernel.org/project/linux-mediatek/patch/20230907112018.52811-1-linux@fw-web.de/ If it is correct (handled the same way as mt7622 before) you have not to send a patch. But you should maybe check condition !=V2 is correct when loocking on v3. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/thermal/mediatek/auxadc_thermal.c?id=33140e668b10200b775779f302b143b32e6ae7ca Regards Frank
--- a/drivers/thermal/mediatek/auxadc_thermal.c +++ b/drivers/thermal/mediatek/auxadc_thermal.c @@ -690,6 +690,9 @@ static const struct mtk_thermal_data mt7986_thermal_data = { .adcpnp = mt7986_adcpnp, .sensor_mux_values = mt7986_mux_values, .version = MTK_THERMAL_V3, + .apmixed_buffer_ctl_reg = APMIXED_SYS_TS_CON1, + .apmixed_buffer_ctl_mask = GENMASK(31, 6) | BIT(3), + .apmixed_buffer_ctl_set = BIT(0), }; in my quick test the temprature can be read again and i'm near room temperature...i though it was a bit higher before, but as far as i understand the code, the values were fixed before and only made it configurable.