[v5,0/7] add support for H616 thermal system

Hi,

this is v5 of this series originally by Martin, only some cosmetic
changes this time, for instance  mentioning experiments with the SRAM
controller registers to confirm that it's not an SRAM region which fixes
the temperature reporting issue.
See the Changelog below for more details.
==================

This series introduces support for the thermal sensors in the Allwinner
H616 SoCs, which includes its siblings H618 and T507. The actual
temperature reading turns out to be very similar to the H6 SoC, just
with support for two more sensors. One nasty complication is caused
by reports about temperatures above 200C, which are related to the
firmware being run (because the vendor U-Boot contains a hack avoiding
this problem). Some investigation and digging in BSP code later
we identified that bit 16 in register 0x3000000 (SYS_CFG) needs to be
cleared for the raw temperature register values to contain reasonable
values.
To achieve this, patch 1/7 exports this very register from the already
existing SRAM/syscon device. Patch 5/7 then adds code to the thermal
driver to find that device via a new DT property, and query its regmap
to clear bit 16 in there.
Patch 4/7 reworks the existing H6 calibration function to become
compatible with the H616, many thanks to Maksim for figuring this out.
This makes the actual enablement patch 6/7 very easy.

The rest of the patches are straightforward and build on Martin's
original work, with some simplifications, resulting in more code sharing.

Please have a look!

Cheers,
Andre

Changelog v4 .. v5:
- add tags
- extend commit message of SRAM controller patch (1/7)
- remove extra empty line (1/7)
- use global static constant for SRAM regmap field (5/7)

Changelog v3 .. v4:
- rebase on top of v6.8-rc2
- rework SYS_CFG bit poking patches to avoid syscon
- use sram lock for the SRAM driver regmap as well
- correctly advertise new allwinner,sram property in binding
- fix conditional definition of sram property
- new patch 4/7 to make the H6 and H616 calibrate functions compatible
- drop now obsolete definition of sun50i_h616_ths_calibrate()

Changelog v2 .. v3:
- rebase on top of v6.7-rc3
- add patches to clear bit 16 in SYS_CFG register 0x3000000
- add syscon to the binding documentation
- add patch explaining the unknown control register value

Changelog v1 .. v2:
- Fix typos
- Remove h616 calc and init functions
- Use TEMP_CALIB_MASK insteaf of 0xffff
- Adjust calibration function to new offset and scale
- Add proper comment to bindings patch
- Split delta calculations to 2 lines
- Add parentheses around caldata[2|3] >> 12
- Negate bindings if for clocks


Andre Przywara (3):
  soc: sunxi: sram: export register 0 for THS on H616
  thermal: sun8i: explain unknown H6 register value
  thermal: sun8i: add SRAM register access code

Maksim Kiselev (1):
  thermal: sun8i: extend H6 calibration to support 4 sensors

Martin Botka (3):
  dt-bindings: thermal: sun8i: Add H616 THS controller
  thermal: sun8i: add support for H616 THS controller
  arm64: dts: allwinner: h616: Add thermal sensor and zones

 .../thermal/allwinner,sun8i-a83t-ths.yaml     |  34 +++--
 .../arm64/boot/dts/allwinner/sun50i-h616.dtsi |  88 +++++++++++++
 drivers/soc/sunxi/sunxi_sram.c                |  22 ++++
 drivers/thermal/sun8i_thermal.c               | 123 +++++++++++++++---
 4 files changed, 235 insertions(+), 32 deletions(-)

On 19/02/2024 16:36, Andre Przywara wrote:
> Hi,
> 
> this is v5 of this series originally by Martin, only some cosmetic
> changes this time, for instance  mentioning experiments with the SRAM
> controller registers to confirm that it's not an SRAM region which fixes
> the temperature reporting issue.
> See the Changelog below for more details.
> ==================
> 
> This series introduces support for the thermal sensors in the Allwinner
> H616 SoCs, which includes its siblings H618 and T507. The actual
> temperature reading turns out to be very similar to the H6 SoC, just
> with support for two more sensors. One nasty complication is caused
> by reports about temperatures above 200C, which are related to the
> firmware being run (because the vendor U-Boot contains a hack avoiding
> this problem). Some investigation and digging in BSP code later
> we identified that bit 16 in register 0x3000000 (SYS_CFG) needs to be
> cleared for the raw temperature register values to contain reasonable
> values.
> To achieve this, patch 1/7 exports this very register from the already
> existing SRAM/syscon device. Patch 5/7 then adds code to the thermal
> driver to find that device via a new DT property, and query its regmap
> to clear bit 16 in there.
> Patch 4/7 reworks the existing H6 calibration function to become
> compatible with the H616, many thanks to Maksim for figuring this out.
> This makes the actual enablement patch 6/7 very easy.
> 
> The rest of the patches are straightforward and build on Martin's
> original work, with some simplifications, resulting in more code sharing.
> 
> Please have a look!

Thanks for the detailed explanation.

I'm willing to pick the patches 1-6 and let the last one to go through 
the allwinner tree.

However I need the blessing from the different designed maintainers for 
the thermal driver and from the sunxi_sram

Thanks

Dne četrtek, 22. februar 2024 ob 19:44:12 CET je Daniel Lezcano napisal(a):
> On 22/02/2024 19:26, Jernej Škrabec wrote:
> > Dne ponedeljek, 19. februar 2024 ob 16:36:33 CET je Andre Przywara napisal(a):
> >> The Allwinner H616 SoC contains a mysterious bit at register offset 0x0
> >> in the SRAM control block. If bit 16 is set (the reset value), the
> >> temperature readings of the THS are way off, leading to reports about
> >> 200C, at normal ambient temperatures. Clearing this bits brings the
> >> reported values down to the expected values.
> >> The BSP code clears this bit in firmware (U-Boot), and has an explicit
> >> comment about this, but offers no real explanation.
> >>
> >> Experiments in U-Boot show that register 0x0 has no effect on the SRAM C
> >> visibility: all tested bit settings still allow full read and write
> >> access by the CPU to the whole of SRAM C. Only bit 24 of the register at
> >> offset 0x4 makes all of SRAM C inaccessible by the CPU. So modelling
> >> the THS switch functionality as an SRAM region would not reflect reality.
> >>
> >> Since we should not rely on firmware settings, allow other code (the THS
> >> driver) to access this register, by exporting it through the already
> >> existing regmap. This mimics what we already do for the LDO control and
> >> the EMAC register.
> >>
> >> To avoid concurrent accesses to the same register at the same time, by
> >> the SRAM switch code and the regmap code, use the same lock to protect
> >> the access. The regmap subsystem allows to use an existing lock, so we
> >> just need to hook in there.
> >>
> >> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > 
> > Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> > 
> > I guess this one goes through sunxi tree, right?
> 
> I'll pick this patch along with the patch 2-6, so through the thermal 
> tree. The patch 7/7 will go indeed via the sunxi tree

Ok.

Best regards,
Jernej