| Message ID | 20240402032729.2736685-1-nico@fluxnic.net |
|---|---|
| Headers | show |
| Series | Mediatek thermal sensor driver support for MT8186 and MT8188 | expand |
On Mon, 1 Apr 2024, Nicolas Pitre wrote: > This is a bunch of patches to support the MT8186 and MT8188 thermal > sensor configurations. They were tagged "v2" despite being "v3". Please follow the thread. Nicolas
On Mon, 01 Apr 2024 23:25:46 -0400, Nicolas Pitre wrote: > From: Nicolas Pitre <npitre@baylibre.com> > > Add LVTS thermal controller definition for MT8188. > > Signed-off-by: Nicolas Pitre <npitre@baylibre.com> > --- > .../bindings/thermal/mediatek,lvts-thermal.yaml | 4 ++++ > .../dt-bindings/thermal/mediatek,lvts-thermal.h | 16 ++++++++++++++++ > 2 files changed, 20 insertions(+) > Reviewed-by: Rob Herring <robh@kernel.org>
Hi Nico, applied the series, except the DT changes Thanks -- Daniel On 02/04/2024 05:25, Nicolas Pitre wrote: > This is a bunch of patches to support the MT8186 and MT8188 thermal > sensor configurations. Several changes are needed to cope with oddities > these SOCs implement. > > All values (calibration data offsets, etc.) were lifted and adapted from > the vendor driver source code. > > Changes from v2: > > - use meaningful name for binding index definitions > - reuse LVTS_COEFF_*_MT7988 on MT8186 per reviewer request > - do similarly for MT8188 that now reuses LVTS_COEFF_*_MT8195 > - use thermal zone names the svs driver wants > - adjust some DT node names and iospace length > - remove variable .hw_tshut_temp as it is constant across all SOCs > > Version 2 can be found here: > > https://lore.kernel.org/all/20240318212428.3843952-1-nico@fluxnic.net/ > > Changes from v1: > > - renamed CPU cluster thermal zones in DT > - fixed logic to cope with empty controller slots at the beginning > - isolated bindings to their own patches > - added MT8188 default thermal zones > > Version 1 can be found here: > > https://lore.kernel.org/all/20240111223020.3593558-1-nico@fluxnic.net/T/ > > diffstat: > > .../thermal/mediatek,lvts-thermal.yaml | 6 + > arch/arm64/boot/dts/mediatek/mt8186.dtsi | 256 +++++++++++ > arch/arm64/boot/dts/mediatek/mt8188.dtsi | 383 ++++++++++++++++ > drivers/thermal/mediatek/lvts_thermal.c | 434 +++++++++++++----- > .../thermal/mediatek,lvts-thermal.h | 26 ++ > 5 files changed, 987 insertions(+), 118 deletions(-)
Il 02/04/24 05:25, Nicolas Pitre ha scritto: > This is a bunch of patches to support the MT8186 and MT8188 thermal > sensor configurations. Several changes are needed to cope with oddities > these SOCs implement. > > All values (calibration data offsets, etc.) were lifted and adapted from > the vendor driver source code. > I picked patches 7 and 12 (and also fixed them) introducing the nodes for the LVTS controllers, but will not pick 9 and 15, as they're either missing thermal zones and/or using the wrong names; let's wait for the next cycle for those, as I will also be able to add the SVS on top (needs a bit of time for testing), getting both SoCs complete on the LVTS side, without rushing. Cheers, Angelo > Changes from v2: > > - use meaningful name for binding index definitions > - reuse LVTS_COEFF_*_MT7988 on MT8186 per reviewer request > - do similarly for MT8188 that now reuses LVTS_COEFF_*_MT8195 > - use thermal zone names the svs driver wants > - adjust some DT node names and iospace length > - remove variable .hw_tshut_temp as it is constant across all SOCs > > Version 2 can be found here: > > https://lore.kernel.org/all/20240318212428.3843952-1-nico@fluxnic.net/ > > Changes from v1: > > - renamed CPU cluster thermal zones in DT > - fixed logic to cope with empty controller slots at the beginning > - isolated bindings to their own patches > - added MT8188 default thermal zones > > Version 1 can be found here: > > https://lore.kernel.org/all/20240111223020.3593558-1-nico@fluxnic.net/T/ > > diffstat: > > .../thermal/mediatek,lvts-thermal.yaml | 6 + > arch/arm64/boot/dts/mediatek/mt8186.dtsi | 256 +++++++++++ > arch/arm64/boot/dts/mediatek/mt8188.dtsi | 383 ++++++++++++++++ > drivers/thermal/mediatek/lvts_thermal.c | 434 +++++++++++++----- > .../thermal/mediatek,lvts-thermal.h | 26 ++ > 5 files changed, 987 insertions(+), 118 deletions(-)
On 4/23/24 11:22, AngeloGioacchino Del Regno wrote: > Il 02/04/24 05:25, Nicolas Pitre ha scritto: >> This is a bunch of patches to support the MT8186 and MT8188 thermal >> sensor configurations. Several changes are needed to cope with oddities >> these SOCs implement. >> >> All values (calibration data offsets, etc.) were lifted and adapted from >> the vendor driver source code. >> > > I picked patches 7 and 12 (and also fixed them) introducing the nodes for the > LVTS controllers, but will not pick 9 and 15, as they're either missing thermal > zones and/or using the wrong names; let's wait for the next cycle for those, as > I will also be able to add the SVS on top (needs a bit of time for testing), > getting both SoCs complete on the LVTS side, without rushing. > > Cheers, > Angelo > Hello Angelo. I took over Nico's work, so I might have missed a few things, but I'm a little bit confused with patches 7 and 13 (you wrote '12' but meant '13' I guess, didn't you ?). It seems to me that patches 7 and 13 were applied in next-20240503 (f5bcf8ab0950 and d3dbc472ac66). But I don't find them any more in next-20240520. It's likely that I don't understand well the process, but I prefer being sure...Should I resend them in next series ? Just a comment about d3dbc472ac66. There's a typo error, I think: nvmem-cell-names = "lvts-calib-data1"; ...should be replaced with nvmem-cell-names = "lvts-calib-data-1"; ...according to the related yaml. Julien
On 4/23/24 11:22, AngeloGioacchino Del Regno wrote: > Il 02/04/24 05:25, Nicolas Pitre ha scritto: >> This is a bunch of patches to support the MT8186 and MT8188 thermal >> sensor configurations. Several changes are needed to cope with oddities >> these SOCs implement. >> >> All values (calibration data offsets, etc.) were lifted and adapted from >> the vendor driver source code. >> > > I picked patches 7 and 12 (and also fixed them) introducing the nodes for the > LVTS controllers, but will not pick 9 and 15, as they're either missing thermal > zones and/or using the wrong names; let's wait for the next cycle for those, as > I will also be able to add the SVS on top (needs a bit of time for testing), > getting both SoCs complete on the LVTS side, without rushing. > > Cheers, > Angelo > What do you mean by "missing" thermal zones ? (is there some reference code somewhere listing them, or whatever ? how can I know which ones are missing ?) It seems to me that Nico took into account your comment about 'tzone_name' and fixed these names in v3 ('cpu-little0-thermal', ...). Are they still wrong ? Julien
Il 20/05/24 14:53, Julien Panis ha scritto: > On 4/23/24 11:22, AngeloGioacchino Del Regno wrote: >> Il 02/04/24 05:25, Nicolas Pitre ha scritto: >>> This is a bunch of patches to support the MT8186 and MT8188 thermal >>> sensor configurations. Several changes are needed to cope with oddities >>> these SOCs implement. >>> >>> All values (calibration data offsets, etc.) were lifted and adapted from >>> the vendor driver source code. >>> >> >> I picked patches 7 and 12 (and also fixed them) introducing the nodes for the >> LVTS controllers, but will not pick 9 and 15, as they're either missing thermal >> zones and/or using the wrong names; let's wait for the next cycle for those, as >> I will also be able to add the SVS on top (needs a bit of time for testing), >> getting both SoCs complete on the LVTS side, without rushing. >> >> Cheers, >> Angelo >> > > Hello Angelo. > > I took over Nico's work, so I might have missed a few things, but I'm > a little bit confused with patches 7 and 13 (you wrote '12' but meant > '13' I guess, didn't you ?). > > It seems to me that patches 7 and 13 were applied in next-20240503 > (f5bcf8ab0950 and d3dbc472ac66). But I don't find them any more in > next-20240520. It's likely that I don't understand well the process, but > I prefer being sure...Should I resend them in next series ? Yes, please. > > Just a comment about d3dbc472ac66. There's a typo error, I think: > nvmem-cell-names = "lvts-calib-data1"; > ...should be replaced with > nvmem-cell-names = "lvts-calib-data-1"; > ...according to the related yaml. Yes, that was a typo :-) Cheers, Angelo
Il 20/05/24 15:18, Julien Panis ha scritto: > On 4/23/24 11:22, AngeloGioacchino Del Regno wrote: >> Il 02/04/24 05:25, Nicolas Pitre ha scritto: >>> This is a bunch of patches to support the MT8186 and MT8188 thermal >>> sensor configurations. Several changes are needed to cope with oddities >>> these SOCs implement. >>> >>> All values (calibration data offsets, etc.) were lifted and adapted from >>> the vendor driver source code. >>> >> >> I picked patches 7 and 12 (and also fixed them) introducing the nodes for the >> LVTS controllers, but will not pick 9 and 15, as they're either missing thermal >> zones and/or using the wrong names; let's wait for the next cycle for those, as >> I will also be able to add the SVS on top (needs a bit of time for testing), >> getting both SoCs complete on the LVTS side, without rushing. >> >> Cheers, >> Angelo >> > > What do you mean by "missing" thermal zones ? (is there some reference > code somewhere listing them, or whatever ? how can I know which ones are > missing ?) > > It seems to me that Nico took into account your comment about 'tzone_name' > and fixed these names in v3 ('cpu-little0-thermal', ...). Are they still wrong ? > Check mt8195.dtsi ;-) Cheers