linux-kernel - Re: [PATCH v2 2/2] thermal: thermal-generic-adc: add temperature sensor channel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <93ddb2d9-ce3d-4e6d-bf5f-0b30b2d04d84@arm.com>
Date: Mon, 3 Mar 2025 10:53:23 +0000
From: Lukasz Luba <lukasz.luba@....com>
To: Svyatoslav Ryhel <clamor95@...il.com>
Cc: linux-pm@...r.kernel.org, Conor Dooley <conor+dt@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Rob Herring <robh@...nel.org>,
 devicetree@...r.kernel.org, Zhang Rui <rui.zhang@...el.com>,
 Daniel Lezcano <daniel.lezcano@...aro.org>,
 "Rafael J. Wysocki" <rafael@...nel.org>,
 Laxman Dewangan <ldewangan@...dia.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] thermal: thermal-generic-adc: add temperature
 sensor channel



On 2/28/25 13:22, Svyatoslav Ryhel wrote:
> пт, 28 лют. 2025 р. о 15:11 Lukasz Luba <lukasz.luba@....com> пише:
>>
>> Hi Svyatoslav,
>>
>> On 2/19/25 08:28, Svyatoslav Ryhel wrote:
>>> Add IIO sensor channel along with existing thermal sensor cell. This
>>> would benefit devices that use adc sensors to detect temperature and
>>> need a custom conversion table.
>>>
>>> Signed-off-by: Svyatoslav Ryhel <clamor95@...il.com>
>>> ---
>>>    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
>>> index ee3d0aa31406..a8f3b965b39b 100644
>>> --- a/drivers/thermal/thermal-generic-adc.c
>>> +++ b/drivers/thermal/thermal-generic-adc.c
>>> @@ -7,6 +7,7 @@
>>>     * Author: Laxman Dewangan <ldewangan@...dia.com>
>>>     */
>>>    #include <linux/iio/consumer.h>
>>> +#include <linux/iio/iio.h>
>>>    #include <linux/kernel.h>
>>>    #include <linux/module.h>
>>>    #include <linux/platform_device.h>
>>> @@ -73,6 +74,57 @@ static const struct thermal_zone_device_ops gadc_thermal_ops = {
>>>        .get_temp = gadc_thermal_get_temp,
>>>    };
>>>
>>> +static const struct iio_chan_spec gadc_thermal_iio_channel[] = {
>>> +     {
>>> +             .type = IIO_TEMP,
>>> +             .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
>>> +     }
>>> +};
>>> +
>>> +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;
>>> +
>>> +     ret = gadc_thermal_get_temp(gtinfo->tz_dev, temp);
>>> +     if (ret < 0)
>>> +             return ret;
>>> +
>>> +     *temp /= 1000;
>>> +
>>> +     return IIO_VAL_INT;
>>> +}
>>> +
>>> +static const struct iio_info gadc_thermal_iio_info = {
>>> +     .read_raw = gadc_thermal_read_raw,
>>> +};
>>> +
>>> +static int gadc_iio_register(struct device *dev, struct gadc_thermal_info *gti)
>>> +{
>>> +     struct gadc_thermal_info *gtinfo;
>>> +     struct iio_dev *indio_dev;
>>> +
>>> +     indio_dev = devm_iio_device_alloc(dev, sizeof(struct gadc_thermal_info));
>>> +     if (!indio_dev)
>>> +             return -ENOMEM;
>>> +
>>> +     gtinfo = iio_priv(indio_dev);
>>> +     memcpy(gtinfo, gti, sizeof(struct gadc_thermal_info));
>>> +
>>> +     indio_dev->name = dev_name(dev);
>>> +     indio_dev->info = &gadc_thermal_iio_info;
>>> +     indio_dev->modes = INDIO_DIRECT_MODE;
>>> +     indio_dev->channels = gadc_thermal_iio_channel;
>>> +     indio_dev->num_channels = ARRAY_SIZE(gadc_thermal_iio_channel);
>>> +
>>> +     return devm_iio_device_register(dev, indio_dev);
>>
>> I don't get the idea why we need iio device, while we already have the
>> hwmon.
>>
> 
> Idea behind this is to be able to convert adc iio channel into temp
> iio channel without introducing a new sensor which will duplicate
> behavior of existing one (by this I mean conversion table use). Not
> all devices can or have to use hwmon and some may require iio channel
> hooked up.
> 
> Real life example. I own a device (LG P985) which has a fuel gauge
> that does not support battery thermal readings. Vendor provided a
> dedicated adc sensor and one of its channels is used as thermal sensor
> with device specific conversion table. Fuel gauge on the other hand
> supports linking in a dedicated temp iio channel to get thermal
> readings.

Thanks. IMO you can add these two sentences into the patch header.
It's telling more about the need of this change.

BTW, I would like to see later how you use it in your battery driver
(please add me on CC, because I'm curious).

The code looks good, so please resend with better patch header
and I'll review the whole patch