Re: [PATCH v3 2/2] thermal: thermal-generic-adc: add temperature sensor channel

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 3/5/25 10:06, Svyatoslav Ryhel wrote:
ср, 5 бер. 2025 р. о 11:52 Lukasz Luba <lukasz.luba@xxxxxxx> пише:



On 3/3/25 12:21, Svyatoslav Ryhel wrote:
To avoid duplicating sensor functionality and conversion tables, this design
allows converting an ADC IIO channel's output directly into a temperature IIO
channel. This is particularly useful for devices where hwmon isn't suitable
or where temperature data must be accessible through IIO.

One such device is, for example, the MAX17040 fuel gauge.

Signed-off-by: Svyatoslav Ryhel <clamor95@xxxxxxxxx>
---
   drivers/thermal/thermal-generic-adc.c | 54 ++++++++++++++++++++++++++-
   1 file changed, 53 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/thermal-generic-adc.c b/drivers/thermal/thermal-generic-adc.c
...

+static const struct iio_chan_spec gadc_thermal_iio_channel[] = {
+     {
+             .type = IIO_TEMP,
+             .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),

I would add the IIO_CHAN_INFO_SCALE and say it's in milli-degrees.


I have hit this issue already with als sensor. This should definitely
be a IIO_CHAN_INFO_PROCESSED since there is no raw temp data we have,
it gets processed into temp data via conversion table. I will add
Jonathan Cameron to list if you don't mind, he might give some good
advice.

I'm not talking about 'PROCESSED' vs 'RAW'...
I'm asking if you can add the 'SCALE' case to handle and report
that this device will report 'processed' temp value in milli-degrees
of Celsius.


+     }
+};
+
+static int gadc_thermal_read_raw(struct iio_dev *indio_dev,
+                              struct iio_chan_spec const *chan,
+                              int *temp, int *val2, long mask)
+{
+     struct gadc_thermal_info *gtinfo = iio_priv(indio_dev);
+     int ret;
+
+     if (mask != IIO_CHAN_INFO_PROCESSED)
+             return -EINVAL;

Therefore, here it would need to handle such case as well, when
a client is asking about scale.

+
+     ret = gadc_thermal_get_temp(gtinfo->tz_dev, temp);
+     if (ret < 0)
+             return ret;
+
+     *temp /= 1000;

IMO we shouldn't cut the precision if it's provided.
The user of this would know what to do with the value (when
the proper information about scale is also available).


The it will not fit existing IIO framework and thermal readings will
be 1000 off. I have had to adjust this since my battery suddenly got
temperature reading of 23200C which obviously was not true. With
adjustment temperature will be in 10th of C (yes, odd, I know but it
is what it is).

Your battery driver should get and check the 'SCALE' info first, then
it will know that the value is in higher resolution than it needs.
Therefore, it can divide the value inside its code.
Your proposed division here is creating a limitation.

You shouldn't force all other drivers to ignore and drop the
available information about milli-degC (which is done in this patch).




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux