[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190812084915.lasb4dh25bfeoigr@flea>
Date: Mon, 12 Aug 2019 10:49:15 +0200
From: Maxime Ripard <maxime.ripard@...tlin.com>
To: Yangtao Li <tiny.windzz@...il.com>
Cc: rui.zhang@...el.com, edubezval@...il.com,
daniel.lezcano@...aro.org, robh+dt@...nel.org,
mark.rutland@....com, wens@...e.org, mchehab+samsung@...nel.org,
davem@...emloft.net, gregkh@...uxfoundation.org,
Jonathan.Cameron@...wei.com, nicolas.ferre@...rochip.com,
linux-pm@...r.kernel.org, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
Icenowy Zheng <icenowy@...c.io>
Subject: Re: [PATCH v5 15/18] thermal: sun8i: allow to use custom temperature
calculation function
Hi,
On Sat, Aug 10, 2019 at 05:28:26AM +0000, Yangtao Li wrote:
> From: Icenowy Zheng <icenowy@...c.io>
>
> The H5 temperature calculation function is strange. Firstly, it's
> segmented. Secondly, the formula of two sensors are different in the
> second segment.
>
> Allow to use a custom temperature calculation function, in case of
> the function is complex.
>
> Signed-off-by: Icenowy Zheng <icenowy@...c.io>
When you send a patch on someone else's behalf, you need to put your
Signed-off-by as well.
> ---
> drivers/thermal/sun8i_thermal.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c
> index 3259081da841..a761e2afda08 100644
> --- a/drivers/thermal/sun8i_thermal.c
> +++ b/drivers/thermal/sun8i_thermal.c
> @@ -76,6 +76,7 @@ struct ths_thermal_chip {
> u16 *caldata, int callen);
> int (*init)(struct ths_device *tmdev);
> int (*irq_ack)(struct ths_device *tmdev);
> + int (*calc_temp)(int id, int reg);
> };
>
> struct ths_device {
> @@ -90,9 +91,12 @@ struct ths_device {
>
> /* Temp Unit: millidegree Celsius */
> static int sun8i_ths_reg2temp(struct ths_device *tmdev,
> - int reg)
> + int id, int reg)
> {
> - return (reg + tmdev->chip->offset) * tmdev->chip->scale;
> + if (tmdev->chip->calc_temp)
> + return tmdev->chip->calc_temp(id, reg);
> + else
> + return (reg + tmdev->chip->offset) * tmdev->chip->scale;
You're not consistent here compared to the other callbacks you have
introduced: calibrate, init and irq_ack all need to be set and will
fail (hard) if you don't set them, yet this one will have a different
behaviour (that behaviour being to use the H6 formula, which is the
latest SoC, which is a bit odd in itself).
I guess we should either make it mandatory as the rest of the
callbacks, or document which callbacks are mandatory and which are
optional (and the behaviour when it's optional).
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists