Hi, On Sun, May 05, 2019 at 04:22:15PM +0100, Jonathan Cameron wrote: > On Fri, 3 May 2019 03:28:07 -0400 > Yangtao Li <tiny.windzz@xxxxxxxxx> wrote: > > > For some SOCs, there are more than one thermal sensor, and there are > > currently four sensors on the A80. So we need to do some work in order > > to support multiple thermal sensors: > > > > 1) add sensor_count in gpadc_data. > > 2) introduce sun4i_sensor_tzd in sun4i_gpadc_iio, to support multiple > > thermal_zone_device and distinguish between different sensors. > > 3) modify read temperature and initialization function. > > This comment doesn't mention the devm change. If it had it would have > raised immediate alarm bells. > > I'm also not keen on the web of pointers that this driver is steadily > evolving. I can't immediately see how to reduce that complexity however. So I might be responsible for that, and looking back, this has been a mistake. This driver was initally put together to support a controller found in older (A10 up to A31) Allwinner SoCs. This controller had an ADC driver that could be operated as a touchscreen controller, and was providing a CPU temperature sensor and a general purpose ADC. However, we already had a driver for that controller in drivers/input to report the CPU temperature, and the one in IIO was introduced to support the general purpose ADC (and the CPU temperature). The long term goal was to add the touchscreen feature as well eventually so that we could remove the one in drivers/input. That didn't happen. At the same time, the Allwinner hardware slowly evolved to remove the touchscreen and ADC features, and only keep the CPU temperature readout. It then evolved further on to support multiple temperatures (for different clusters, the GPU, and so on). So, today, we're in a situation where I was pushing everything into that IIO drivers since there was similiraties between all the generations, but the fact that we have to support so many odd cases (DT bindings compatibility, controllers with and without ADC, etc) that it becomes a real mess. And that mess isn't really used by anybody, since we want to have the touchscreen. There's only one SoC that is supported only by that driver, which is the A33 that only had a CPU temperature readout, and is still pretty similar to the latest SoC from Allwinner (that is supported by this series). I guess, for everyone's sanity and in order to not stall this further, it would just be better to create an hwmon driver for the A33 (and onwards, including the H6) for the SoC that just have the temperature readout feature. And for the older SoC, we just keep the older driver under input/. Once the A33 is supported, we'll remove the driver in IIO (and the related bits in drivers/mfd). Armbian already has a driver for that they never upstreamed iirc, so it might be a good starting point, and we would add the support for the H6. How does that sound? Sorry for wasting everybody's time on this. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Attachment:
signature.asc
Description: PGP signature
