* [PATCH v2 0/2] thermal: thermal-generic-adc: add temp sensor function
@ 2025-02-19 8:28 Svyatoslav Ryhel
2025-02-19 8:28 ` [PATCH v2 1/2] dt-bindings: thermal: generic-adc: Add optional io-channel-cells property Svyatoslav Ryhel
2025-02-19 8:28 ` [PATCH v2 2/2] thermal: thermal-generic-adc: add temperature sensor channel Svyatoslav Ryhel
0 siblings, 2 replies; 9+ messages in thread
From: Svyatoslav Ryhel @ 2025-02-19 8:28 UTC (permalink / raw)
To: Rafael J. Wysocki, Daniel Lezcano, Zhang Rui, Lukasz Luba,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Laxman Dewangan
Cc: linux-pm, devicetree, linux-kernel
Add IIO sensor cell to thermal-generic-adc, which would benefit
devices that use adc sensors to detect temperature and need a
custom conversion table.
---
Changes on switching from v1 to v2:
- documented #iio-channel-cells property
- switched to IIO_CHAN_INFO_PROCESSED
---
Svyatoslav Ryhel (2):
dt-bindings: thermal: generic-adc: Add optional io-channel-cells
property
thermal: thermal-generic-adc: add temperature sensor channel
.../bindings/thermal/generic-adc-thermal.yaml | 4 ++
drivers/thermal/thermal-generic-adc.c | 54 ++++++++++++++++++-
2 files changed, 57 insertions(+), 1 deletion(-)
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/2] dt-bindings: thermal: generic-adc: Add optional io-channel-cells property
2025-02-19 8:28 [PATCH v2 0/2] thermal: thermal-generic-adc: add temp sensor function Svyatoslav Ryhel
@ 2025-02-19 8:28 ` Svyatoslav Ryhel
2025-02-21 21:13 ` Rob Herring
2025-02-19 8:28 ` [PATCH v2 2/2] thermal: thermal-generic-adc: add temperature sensor channel Svyatoslav Ryhel
1 sibling, 1 reply; 9+ messages in thread
From: Svyatoslav Ryhel @ 2025-02-19 8:28 UTC (permalink / raw)
To: Rafael J. Wysocki, Daniel Lezcano, Zhang Rui, Lukasz Luba,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Laxman Dewangan
Cc: linux-pm, devicetree, linux-kernel
Since device is a thermal sensor it needs '#io-channel-cells' to allow
exposure of thermal data via IIO means.
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
.../devicetree/bindings/thermal/generic-adc-thermal.yaml | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/thermal/generic-adc-thermal.yaml b/Documentation/devicetree/bindings/thermal/generic-adc-thermal.yaml
index 12e6418dc24d..4bc2cff0593c 100644
--- a/Documentation/devicetree/bindings/thermal/generic-adc-thermal.yaml
+++ b/Documentation/devicetree/bindings/thermal/generic-adc-thermal.yaml
@@ -30,6 +30,9 @@ properties:
io-channel-names:
const: sensor-channel
+ '#io-channel-cells':
+ const: 1
+
temperature-lookup-table:
description: |
Lookup table to map the relation between ADC value and temperature.
@@ -60,6 +63,7 @@ examples:
#thermal-sensor-cells = <0>;
io-channels = <&ads1015 1>;
io-channel-names = "sensor-channel";
+ #io-channel-cells = <1>;
temperature-lookup-table = <
(-40000) 2578
(-39000) 2577
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] thermal: thermal-generic-adc: add temperature sensor channel
2025-02-19 8:28 [PATCH v2 0/2] thermal: thermal-generic-adc: add temp sensor function Svyatoslav Ryhel
2025-02-19 8:28 ` [PATCH v2 1/2] dt-bindings: thermal: generic-adc: Add optional io-channel-cells property Svyatoslav Ryhel
@ 2025-02-19 8:28 ` Svyatoslav Ryhel
2025-02-28 13:11 ` Lukasz Luba
1 sibling, 1 reply; 9+ messages in thread
From: Svyatoslav Ryhel @ 2025-02-19 8:28 UTC (permalink / raw)
To: Rafael J. Wysocki, Daniel Lezcano, Zhang Rui, Lukasz Luba,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Laxman Dewangan
Cc: linux-pm, devicetree, linux-kernel
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@gmail.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@nvidia.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);
+}
+
static int gadc_thermal_read_linear_lookup_table(struct device *dev,
struct gadc_thermal_info *gti)
{
@@ -153,7 +205,7 @@ static int gadc_thermal_probe(struct platform_device *pdev)
devm_thermal_add_hwmon_sysfs(dev, gti->tz_dev);
- return 0;
+ return gadc_iio_register(&pdev->dev, gti);
}
static const struct of_device_id of_adc_thermal_match[] = {
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: thermal: generic-adc: Add optional io-channel-cells property
2025-02-19 8:28 ` [PATCH v2 1/2] dt-bindings: thermal: generic-adc: Add optional io-channel-cells property Svyatoslav Ryhel
@ 2025-02-21 21:13 ` Rob Herring
2025-02-22 7:12 ` Svyatoslav Ryhel
0 siblings, 1 reply; 9+ messages in thread
From: Rob Herring @ 2025-02-21 21:13 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Rafael J. Wysocki, Daniel Lezcano, Zhang Rui, Lukasz Luba,
Krzysztof Kozlowski, Conor Dooley, Laxman Dewangan, linux-pm,
devicetree, linux-kernel
On Wed, Feb 19, 2025 at 10:28:16AM +0200, Svyatoslav Ryhel wrote:
> Since device is a thermal sensor it needs '#io-channel-cells' to allow
> exposure of thermal data via IIO means.
This looks odd. The consumer is also a producer? What in DT would be the
2nd consumer. If you don't have a consumer in the DT, then you don't
need '#io-channel-cells'.
I would like to see Jonathan's buy in on this.
>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
> .../devicetree/bindings/thermal/generic-adc-thermal.yaml | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/thermal/generic-adc-thermal.yaml b/Documentation/devicetree/bindings/thermal/generic-adc-thermal.yaml
> index 12e6418dc24d..4bc2cff0593c 100644
> --- a/Documentation/devicetree/bindings/thermal/generic-adc-thermal.yaml
> +++ b/Documentation/devicetree/bindings/thermal/generic-adc-thermal.yaml
> @@ -30,6 +30,9 @@ properties:
> io-channel-names:
> const: sensor-channel
>
> + '#io-channel-cells':
> + const: 1
You have to document what is in the cells.
> +
> temperature-lookup-table:
> description: |
> Lookup table to map the relation between ADC value and temperature.
> @@ -60,6 +63,7 @@ examples:
> #thermal-sensor-cells = <0>;
> io-channels = <&ads1015 1>;
> io-channel-names = "sensor-channel";
> + #io-channel-cells = <1>;
> temperature-lookup-table = <
> (-40000) 2578
> (-39000) 2577
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: thermal: generic-adc: Add optional io-channel-cells property
2025-02-21 21:13 ` Rob Herring
@ 2025-02-22 7:12 ` Svyatoslav Ryhel
0 siblings, 0 replies; 9+ messages in thread
From: Svyatoslav Ryhel @ 2025-02-22 7:12 UTC (permalink / raw)
To: Rob Herring
Cc: Rafael J. Wysocki, Daniel Lezcano, Zhang Rui, Lukasz Luba,
Krzysztof Kozlowski, Conor Dooley, Laxman Dewangan, linux-pm,
devicetree, linux-kernel
пт, 21 лют. 2025 р. о 23:13 Rob Herring <robh@kernel.org> пише:
>
> On Wed, Feb 19, 2025 at 10:28:16AM +0200, Svyatoslav Ryhel wrote:
> > Since device is a thermal sensor it needs '#io-channel-cells' to allow
> > exposure of thermal data via IIO means.
>
> This looks odd. The consumer is also a producer? What in DT would be the
> 2nd consumer. If you don't have a consumer in the DT, then you don't
> need '#io-channel-cells'.
>
No, this binding in general describes converting of iio adc data into
thermal data, using device specific conversion table. Resulting
thermal data is exposed as thermal sensor cell for thermal zone
binding. What I did is that I just exposed thermal data as temp iio
channel (processed). Real use case: I own an embedded device which has
a dedicated sensor for battery temperature control, but to indicate
the temperature is used adc channel of that sensor and adc data is
converted using a device specific table into temperature. Basically
this fits existing generic-adc-thermal setup to create a thermal zone,
but along with all that, the fuel gauge which monitors battery can
accept iio temp channel to expose battery temperature. This is what I
am adding here.
adc sensor > generic-adc-thermal > temp channel > battery gauge
> I would like to see Jonathan's buy in on this.
>
> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > ---
> > .../devicetree/bindings/thermal/generic-adc-thermal.yaml | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/thermal/generic-adc-thermal.yaml b/Documentation/devicetree/bindings/thermal/generic-adc-thermal.yaml
> > index 12e6418dc24d..4bc2cff0593c 100644
> > --- a/Documentation/devicetree/bindings/thermal/generic-adc-thermal.yaml
> > +++ b/Documentation/devicetree/bindings/thermal/generic-adc-thermal.yaml
> > @@ -30,6 +30,9 @@ properties:
> > io-channel-names:
> > const: sensor-channel
> >
> > + '#io-channel-cells':
> > + const: 1
>
> You have to document what is in the cells.
>
> > +
> > temperature-lookup-table:
> > description: |
> > Lookup table to map the relation between ADC value and temperature.
> > @@ -60,6 +63,7 @@ examples:
> > #thermal-sensor-cells = <0>;
> > io-channels = <&ads1015 1>;
> > io-channel-names = "sensor-channel";
> > + #io-channel-cells = <1>;
> > temperature-lookup-table = <
> > (-40000) 2578
> > (-39000) 2577
> > --
> > 2.43.0
> >
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] thermal: thermal-generic-adc: add temperature sensor channel
2025-02-19 8:28 ` [PATCH v2 2/2] thermal: thermal-generic-adc: add temperature sensor channel Svyatoslav Ryhel
@ 2025-02-28 13:11 ` Lukasz Luba
2025-02-28 13:22 ` Svyatoslav Ryhel
0 siblings, 1 reply; 9+ messages in thread
From: Lukasz Luba @ 2025-02-28 13:11 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: linux-pm, Conor Dooley, Krzysztof Kozlowski, Rob Herring,
devicetree, Zhang Rui, Daniel Lezcano, Rafael J. Wysocki,
Laxman Dewangan, linux-kernel
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@gmail.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@nvidia.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.
Could you explain this a bit more, the cover letter also misses
such justification and details.
Regards,
Lukasz
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] thermal: thermal-generic-adc: add temperature sensor channel
2025-02-28 13:11 ` Lukasz Luba
@ 2025-02-28 13:22 ` Svyatoslav Ryhel
2025-03-03 10:53 ` Lukasz Luba
0 siblings, 1 reply; 9+ messages in thread
From: Svyatoslav Ryhel @ 2025-02-28 13:22 UTC (permalink / raw)
To: Lukasz Luba
Cc: linux-pm, Conor Dooley, Krzysztof Kozlowski, Rob Herring,
devicetree, Zhang Rui, Daniel Lezcano, Rafael J. Wysocki,
Laxman Dewangan, linux-kernel
пт, 28 лют. 2025 р. о 15:11 Lukasz Luba <lukasz.luba@arm.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@gmail.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@nvidia.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.
> Could you explain this a bit more, the cover letter also misses
> such justification and details.
>
> Regards,
> Lukasz
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] thermal: thermal-generic-adc: add temperature sensor channel
2025-02-28 13:22 ` Svyatoslav Ryhel
@ 2025-03-03 10:53 ` Lukasz Luba
2025-03-03 11:08 ` Svyatoslav Ryhel
0 siblings, 1 reply; 9+ messages in thread
From: Lukasz Luba @ 2025-03-03 10:53 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: linux-pm, Conor Dooley, Krzysztof Kozlowski, Rob Herring,
devicetree, Zhang Rui, Daniel Lezcano, Rafael J. Wysocki,
Laxman Dewangan, linux-kernel
On 2/28/25 13:22, Svyatoslav Ryhel wrote:
> пт, 28 лют. 2025 р. о 15:11 Lukasz Luba <lukasz.luba@arm.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@gmail.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@nvidia.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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] thermal: thermal-generic-adc: add temperature sensor channel
2025-03-03 10:53 ` Lukasz Luba
@ 2025-03-03 11:08 ` Svyatoslav Ryhel
0 siblings, 0 replies; 9+ messages in thread
From: Svyatoslav Ryhel @ 2025-03-03 11:08 UTC (permalink / raw)
To: Lukasz Luba
Cc: linux-pm, Conor Dooley, Krzysztof Kozlowski, Rob Herring,
devicetree, Zhang Rui, Daniel Lezcano, Rafael J. Wysocki,
Laxman Dewangan, linux-kernel
пн, 3 бер. 2025 р. о 12:53 Lukasz Luba <lukasz.luba@arm.com> пише:
>
>
>
> On 2/28/25 13:22, Svyatoslav Ryhel wrote:
> > пт, 28 лют. 2025 р. о 15:11 Lukasz Luba <lukasz.luba@arm.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@gmail.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@nvidia.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.
>
Sure, will do
> BTW, I would like to see later how you use it in your battery driver
> (please add me on CC, because I'm curious).
>
It is already present in the kernel. Here is the schema
https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/power/supply/maxim,max17040.yaml?h=v6.14-rc5
> The code looks good, so please resend with better patch header
> and I'll review the whole patch
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-03-03 11:08 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-19 8:28 [PATCH v2 0/2] thermal: thermal-generic-adc: add temp sensor function Svyatoslav Ryhel
2025-02-19 8:28 ` [PATCH v2 1/2] dt-bindings: thermal: generic-adc: Add optional io-channel-cells property Svyatoslav Ryhel
2025-02-21 21:13 ` Rob Herring
2025-02-22 7:12 ` Svyatoslav Ryhel
2025-02-19 8:28 ` [PATCH v2 2/2] thermal: thermal-generic-adc: add temperature sensor channel Svyatoslav Ryhel
2025-02-28 13:11 ` Lukasz Luba
2025-02-28 13:22 ` Svyatoslav Ryhel
2025-03-03 10:53 ` Lukasz Luba
2025-03-03 11:08 ` Svyatoslav Ryhel
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).