LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Ondřej Jirman" <firstname.lastname@example.org>
To: Frank Lee <email@example.com>
Cc: Icenowy Zheng <firstname.lastname@example.org>,
Mark Rutland <email@example.com>,
Maxime Ripard <firstname.lastname@example.org>,
Linux Kernel Mailing List <email@example.com>,
firstname.lastname@example.org, Chen-Yu Tsai <email@example.com>,
Linux ARM <firstname.lastname@example.org>,
Lee Jones <email@example.com>,
Jonathan Cameron <firstname.lastname@example.org>
Subject: Re: [PATCH 1/7] iio: adc: sun4i-gpadc: rework for support multiple thermal sensor
Date: Mon, 6 May 2019 19:55:25 +0200 [thread overview]
Message-ID: <email@example.com> (raw)
On Tue, May 07, 2019 at 01:08:39AM +0800, Frank Lee wrote:
> On Tue, May 7, 2019 at 12:52 AM Icenowy Zheng <firstname.lastname@example.org> wrote:
> > 在 2019-05-06一的 14:28 +0200，Maxime Ripard写道：
> > > 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 <email@example.com> 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).
> a hwmon driver or a thermal driver？
> > I think a thermal driver is better.
> This is what I hope to see a few months ago.
> > Other SoCs' thermal sensor drivers are all thermal drivers.
> > >
> > > 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?
> > I think the developer abandoned to upstream it because of the previous
> > problem ;-)
> > Maybe it can be taken and add A33&H6 support.
> If OK, I am going to start some thermal driver work this weekend. : )
There are plenty of thermal drivers flying around, with varying levels
of support for various SoCs:
- H3/H5: https://megous.com/git/linux/commit/?h=ths-5.1&id=b8e20c5da7a00b3a3fa1b274fc8d5bea95872b0a
- A83T: https://megous.com/git/linux/commit/?h=ths-5.1&id=796dff9a946fd475cc1e4bb948a723ea841c640c
- H6: https://megous.com/git/linux/commit/?h=opi3-5.1&id=aeab762c19b4aa228a295258c9d6b2e1f143bf86
For H3/H5 Icenowy also tried to upstream some variant of my THS driver, with
better SID/calibration data reading support.
I'd suggest starting with the H6 driver above (as that implements the
calibration data readout correctly), and make it so that it can support multiple
> > >
> > > Sorry for wasting everybody's time on this.
> > >
> > > Maxime
> > >
> > > --
> > > Maxime Ripard, Bootlin
> > > Embedded Linux and Kernel engineering
> > > https://bootlin.com
> > > _______________________________________________
> > > linux-arm-kernel mailing list
> > > firstname.lastname@example.org
> > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> linux-arm-kernel mailing list
next prev parent reply other threads:[~2019-05-06 17:55 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-03 7:28 [PATCH 0/7] Add support for H6 " Yangtao Li
2019-05-03 7:28 ` [PATCH 1/7] iio: adc: sun4i-gpadc: rework for support multiple " Yangtao Li
2019-05-05 15:22 ` Jonathan Cameron
2019-05-06 12:28 ` Maxime Ripard
2019-05-06 16:52 ` Icenowy Zheng
2019-05-06 17:08 ` Frank Lee
2019-05-06 17:55 ` Ondřej Jirman [this message]
2019-05-07 13:59 ` Maxime Ripard
2019-05-03 7:28 ` [PATCH 2/7] iio: adc: sun4i-gpadc: introduce temp_data in gpadc_data Yangtao Li
2019-05-03 7:28 ` [PATCH 3/7] iio: adc: sun4i-gpadc: introduce gpadc_enable and gpadc_disable " Yangtao Li
2019-05-03 7:28 ` [PATCH 4/7] iio: adc: sun4i-gpadc-iio: support clocks and reset Yangtao Li
2019-05-03 7:28 ` [PATCH 5/7] dt-bindings: mfd: Add H6 GPADC binding Yangtao Li
2019-05-08 11:44 ` Lee Jones
2019-05-03 7:28 ` [PATCH 6/7] iio: adc: sun4i-gpadc-iio: add support for H6 thermal sensor Yangtao Li
2019-05-05 15:25 ` Jonathan Cameron
2019-05-08 11:45 ` Lee Jones
2019-05-03 7:28 ` [PATCH 7/7] iio: adc: sun4i-gpadc-iio convert to SPDX license tags Yangtao Li
2019-05-03 9:18 ` Maxime Ripard
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--subject='Re: [PATCH 1/7] iio: adc: sun4i-gpadc: rework for support multiple thermal sensor' \
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).