[PATCH v3 0/4] Add optional properties to MAX17040

[PATCH v3 0/4] Add optional properties to MAX17040

[Svyatoslav Ryhel]

Extend properties supported by max17040 fuel gauge if it is accompanied
by different devices.

If max17040 is coupled with a charger, pass charger status since it should
match and max17040 has no dedicated status detection ability.

max17040_get_online can be reused for PRESENT property since if it is
online it must be present.

Finally, max17040 may be coupled with a dedicated thermal sensor which
monitors battery temperature so lets add support for iio channel to match
hw setup. With that said, the driver got dependency on CONFIG_IIO which
was added to Kconfig. All defconfigs apart s5pv210_defconfig have IIO
already enabled so only s5pv210_defconfig needed adjustment.

---
Changes from v2:
- documentation: fixed typo i2c0 > i2c
- added dependency on CONFIG_IIO
- enabled CONFIG_IIO for s5pv210_defconfig to avoid regressions (all other
  defconfigs which include max17040 already have IIO enabled)

Changes from v1:
- documentation: dropped monitored-battery and power-supplies (inherited
  from inclusion)
- dropped passing charger health as battery health
- dropped patch for simple battery cell support
- switched iio_read_channel_raw to iio_read_channel_processed_scale
- switched iio_channel_get to devm_iio_channel_get
- re-organized implementation of temp channel (implemented in way 
  *_get_optional functions usually act)
---

Svyatoslav Ryhel (4):
  dt-bindings: power: supply: maxim,max17040: update properties
  power: max17040: pass status property from supplier
  power: max17040: get thermal data from adc if available
  ARM: configs: s5pv210_defconfig: enable IIO required by MAX17040

 .../bindings/power/supply/maxim,max17040.yaml | 31 +++++++++++++++++++
 arch/arm/configs/s5pv210_defconfig            |  1 +
 drivers/power/supply/Kconfig                  |  2 +-
 drivers/power/supply/max17040_battery.c       | 27 ++++++++++++++++
 4 files changed, 60 insertions(+), 1 deletion(-)

-- 
2.39.2

[PATCH v3 1/4] dt-bindings: power: supply: maxim,max17040: update properties

[Svyatoslav Ryhel]

Add status and temperature properties.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../bindings/power/supply/maxim,max17040.yaml | 31 +++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/supply/maxim,max17040.yaml b/Documentation/devicetree/bindings/power/supply/maxim,max17040.yaml
index 2627cd3eed83..377cbb2c2c1f 100644
--- a/Documentation/devicetree/bindings/power/supply/maxim,max17040.yaml
+++ b/Documentation/devicetree/bindings/power/supply/maxim,max17040.yaml
@@ -55,6 +55,14 @@ properties:
   interrupts:
     maxItems: 1
 
+  io-channels:
+    items:
+      - description: battery temperature
+
+  io-channel-names:
+    items:
+      - const: temp
+
   wakeup-source:
     type: boolean
     description: |
@@ -95,3 +103,26 @@ examples:
         wakeup-source;
       };
     };
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      fuel-gauge@36 {
+        compatible = "maxim,max17043";
+        reg = <0x36>;
+
+        interrupt-parent = <&gpio>;
+        interrupts = <144 IRQ_TYPE_EDGE_FALLING>;
+
+        monitored-battery = <&battery>;
+        power-supplies = <&charger>;
+
+        io-channels = <&adc 8>;
+        io-channel-names = "temp";
+
+        maxim,alert-low-soc-level = <10>;
+        wakeup-source;
+      };
+    };
-- 
2.39.2

[PATCH v3 2/4] power: max17040: pass status property from supplier

[Svyatoslav Ryhel]

Optionally pass status property from supplier if has support
for it. If cell is online assume it is present as well.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/power/supply/max17040_battery.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/power/supply/max17040_battery.c b/drivers/power/supply/max17040_battery.c
index d1075959dd46..3301e8a4b16c 100644
--- a/drivers/power/supply/max17040_battery.c
+++ b/drivers/power/supply/max17040_battery.c
@@ -389,6 +389,7 @@ static int max17040_get_property(struct power_supply *psy,
 
 	switch (psp) {
 	case POWER_SUPPLY_PROP_ONLINE:
+	case POWER_SUPPLY_PROP_PRESENT:
 		val->intval = max17040_get_online(chip);
 		break;
 	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
@@ -400,6 +401,9 @@ static int max17040_get_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_CAPACITY_ALERT_MIN:
 		val->intval = chip->low_soc_alert;
 		break;
+	case POWER_SUPPLY_PROP_STATUS:
+		power_supply_get_property_from_supplier(psy, psp, val);
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -415,9 +419,11 @@ static const struct regmap_config max17040_regmap = {
 
 static enum power_supply_property max17040_battery_props[] = {
 	POWER_SUPPLY_PROP_ONLINE,
+	POWER_SUPPLY_PROP_PRESENT,
 	POWER_SUPPLY_PROP_VOLTAGE_NOW,
 	POWER_SUPPLY_PROP_CAPACITY,
 	POWER_SUPPLY_PROP_CAPACITY_ALERT_MIN,
+	POWER_SUPPLY_PROP_STATUS,
 };
 
 static const struct power_supply_desc max17040_battery_desc = {
-- 
2.39.2

[PATCH v3 3/4] power: max17040: get thermal data from adc if available

[Svyatoslav Ryhel]

Since fuel gauge does not support thermal monitoring,
some vendors may couple this fuel gauge with thermal/adc
sensor to monitor battery cell exact temperature.

Add this feature by adding optional iio thermal channel.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/power/supply/Kconfig            |  2 +-
 drivers/power/supply/max17040_battery.c | 21 +++++++++++++++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index c78be9f322e6..32600609865b 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -374,7 +374,7 @@ config AXP288_FUEL_GAUGE
 
 config BATTERY_MAX17040
 	tristate "Maxim MAX17040/17041/17043 family Fuel Gauge"
-	depends on I2C
+	depends on I2C && IIO
 	select REGMAP_I2C
 	help
 	  Driver supports Maxim fuel-gauge systems for lithium-ion (Li+)
diff --git a/drivers/power/supply/max17040_battery.c b/drivers/power/supply/max17040_battery.c
index 3301e8a4b16c..54db20637c87 100644
--- a/drivers/power/supply/max17040_battery.c
+++ b/drivers/power/supply/max17040_battery.c
@@ -18,6 +18,7 @@
 #include <linux/of_device.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
+#include <linux/iio/consumer.h>
 
 #define MAX17040_VCELL	0x02
 #define MAX17040_SOC	0x04
@@ -142,6 +143,7 @@ struct max17040_chip {
 	struct delayed_work		work;
 	struct power_supply		*battery;
 	struct chip_data		data;
+	struct iio_channel		*channel_temp;
 
 	/* battery capacity */
 	int soc;
@@ -404,6 +406,13 @@ static int max17040_get_property(struct power_supply *psy,
 	case POWER_SUPPLY_PROP_STATUS:
 		power_supply_get_property_from_supplier(psy, psp, val);
 		break;
+	case POWER_SUPPLY_PROP_TEMP:
+		if (!chip->channel_temp)
+			return -ENODATA;
+
+		iio_read_channel_processed_scale(chip->channel_temp,
+						 &val->intval, 10);
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -424,6 +433,7 @@ static enum power_supply_property max17040_battery_props[] = {
 	POWER_SUPPLY_PROP_CAPACITY,
 	POWER_SUPPLY_PROP_CAPACITY_ALERT_MIN,
 	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_TEMP,
 };
 
 static const struct power_supply_desc max17040_battery_desc = {
@@ -469,6 +479,17 @@ static int max17040_probe(struct i2c_client *client)
 	i2c_set_clientdata(client, chip);
 	psy_cfg.drv_data = chip;
 
+	/* Switch to devm_iio_channel_get_optional when available  */
+	chip->channel_temp = devm_iio_channel_get(&client->dev, "temp");
+	if (IS_ERR(chip->channel_temp)) {
+		ret = PTR_ERR(chip->channel_temp);
+		if (ret != -ENODEV)
+			return dev_err_probe(&client->dev, PTR_ERR(chip->channel_temp),
+					     "failed to get temp\n");
+		else
+			chip->channel_temp = NULL;
+	}
+
 	chip->battery = devm_power_supply_register(&client->dev,
 				&max17040_battery_desc, &psy_cfg);
 	if (IS_ERR(chip->battery)) {
-- 
2.39.2

[PATCH v3 4/4] ARM: configs: s5pv210_defconfig: enable IIO required by MAX17040

[Svyatoslav Ryhel]

After adding support for passing temperature data from thermal sensor
to MAX17040 it got dependency on CONFIG_IIO. From all defconfigs
using MAX17040 only s5pv210_defconfig did not have IIO already enabled
so let's enable it to avoid regression.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 arch/arm/configs/s5pv210_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/s5pv210_defconfig b/arch/arm/configs/s5pv210_defconfig
index 4c1e480b5bbd..24070ee3d43e 100644
--- a/arch/arm/configs/s5pv210_defconfig
+++ b/arch/arm/configs/s5pv210_defconfig
@@ -97,6 +97,7 @@ CONFIG_MMC_SDHCI_S3C_DMA=y
 CONFIG_RTC_CLASS=y
 CONFIG_RTC_DRV_MAX8998=m
 CONFIG_DMADEVICES=y
+CONFIG_IIO=y
 CONFIG_PWM=y
 CONFIG_PWM_SAMSUNG=y
 CONFIG_PHY_SAMSUNG_USB2=m
-- 
2.39.2

Re: [PATCH v3 4/4] ARM: configs: s5pv210_defconfig: enable IIO required by MAX17040

[Andi Shyti]

Hi Svyatoslav,

On Mon, Jul 31, 2023 at 10:36:13AM +0300, Svyatoslav Ryhel wrote:
> After adding support for passing temperature data from thermal sensor
> to MAX17040 it got dependency on CONFIG_IIO. From all defconfigs
> using MAX17040 only s5pv210_defconfig did not have IIO already enabled
> so let's enable it to avoid regression.
> 
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  arch/arm/configs/s5pv210_defconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/configs/s5pv210_defconfig b/arch/arm/configs/s5pv210_defconfig
> index 4c1e480b5bbd..24070ee3d43e 100644
> --- a/arch/arm/configs/s5pv210_defconfig
> +++ b/arch/arm/configs/s5pv210_defconfig
> @@ -97,6 +97,7 @@ CONFIG_MMC_SDHCI_S3C_DMA=y
>  CONFIG_RTC_CLASS=y
>  CONFIG_RTC_DRV_MAX8998=m
>  CONFIG_DMADEVICES=y
> +CONFIG_IIO=y
>  CONFIG_PWM=y
>  CONFIG_PWM_SAMSUNG=y
>  CONFIG_PHY_SAMSUNG_USB2=m

Should this patch be squashed to the previous patch? I think you
break bisectability for this board if you enable iio only here.

Andi

Re: [PATCH v3 4/4] ARM: configs: s5pv210_defconfig: enable IIO required by MAX17040

[Krzysztof Kozlowski]

On 06/08/2023 00:42, Andi Shyti wrote:
> Hi Svyatoslav,
> 
> On Mon, Jul 31, 2023 at 10:36:13AM +0300, Svyatoslav Ryhel wrote:
>> After adding support for passing temperature data from thermal sensor
>> to MAX17040 it got dependency on CONFIG_IIO. From all defconfigs
>> using MAX17040 only s5pv210_defconfig did not have IIO already enabled
>> so let's enable it to avoid regression.
>>
>> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
>> ---
>>  arch/arm/configs/s5pv210_defconfig | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/configs/s5pv210_defconfig b/arch/arm/configs/s5pv210_defconfig
>> index 4c1e480b5bbd..24070ee3d43e 100644
>> --- a/arch/arm/configs/s5pv210_defconfig
>> +++ b/arch/arm/configs/s5pv210_defconfig
>> @@ -97,6 +97,7 @@ CONFIG_MMC_SDHCI_S3C_DMA=y
>>  CONFIG_RTC_CLASS=y
>>  CONFIG_RTC_DRV_MAX8998=m
>>  CONFIG_DMADEVICES=y
>> +CONFIG_IIO=y
>>  CONFIG_PWM=y
>>  CONFIG_PWM_SAMSUNG=y
>>  CONFIG_PHY_SAMSUNG_USB2=m
> 
> Should this patch be squashed to the previous patch? I think you
> break bisectability for this board if you enable iio only here.

The defconfig change matters less - distros don't use them - so this
points to the fact that patchset affected the users. All existing users
of max17040 drivers, who do not enable IIO, will have their setups broken.

Best regards,
Krzysztof

Re: [PATCH v3 4/4] ARM: configs: s5pv210_defconfig: enable IIO required by MAX17040

[Andi Shyti]

Hi,

On Sun, Aug 06, 2023 at 10:29:04AM +0200, Krzysztof Kozlowski wrote:
> On 06/08/2023 00:42, Andi Shyti wrote:
> > Hi Svyatoslav,
> > 
> > On Mon, Jul 31, 2023 at 10:36:13AM +0300, Svyatoslav Ryhel wrote:
> >> After adding support for passing temperature data from thermal sensor
> >> to MAX17040 it got dependency on CONFIG_IIO. From all defconfigs
> >> using MAX17040 only s5pv210_defconfig did not have IIO already enabled
> >> so let's enable it to avoid regression.
> >>
> >> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> >> ---
> >>  arch/arm/configs/s5pv210_defconfig | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/arch/arm/configs/s5pv210_defconfig b/arch/arm/configs/s5pv210_defconfig
> >> index 4c1e480b5bbd..24070ee3d43e 100644
> >> --- a/arch/arm/configs/s5pv210_defconfig
> >> +++ b/arch/arm/configs/s5pv210_defconfig
> >> @@ -97,6 +97,7 @@ CONFIG_MMC_SDHCI_S3C_DMA=y
> >>  CONFIG_RTC_CLASS=y
> >>  CONFIG_RTC_DRV_MAX8998=m
> >>  CONFIG_DMADEVICES=y
> >> +CONFIG_IIO=y
> >>  CONFIG_PWM=y
> >>  CONFIG_PWM_SAMSUNG=y
> >>  CONFIG_PHY_SAMSUNG_USB2=m
> > 
> > Should this patch be squashed to the previous patch? I think you
> > break bisectability for this board if you enable iio only here.
> 
> The defconfig change matters less - distros don't use them - so this
> points to the fact that patchset affected the users. All existing users
> of max17040 drivers, who do not enable IIO, will have their setups broken.

That's why I'm suggesting to squash this patch with the previous.

Anyway, up to you... except of this note everything looks fine in
the series.

Andi

Re: [PATCH v3 4/4] ARM: configs: s5pv210_defconfig: enable IIO required by MAX17040

[Krzysztof Kozlowski]

On 07/08/2023 10:55, Andi Shyti wrote:
> Hi,
> 
> On Sun, Aug 06, 2023 at 10:29:04AM +0200, Krzysztof Kozlowski wrote:
>> On 06/08/2023 00:42, Andi Shyti wrote:
>>> Hi Svyatoslav,
>>>
>>> On Mon, Jul 31, 2023 at 10:36:13AM +0300, Svyatoslav Ryhel wrote:
>>>> After adding support for passing temperature data from thermal sensor
>>>> to MAX17040 it got dependency on CONFIG_IIO. From all defconfigs
>>>> using MAX17040 only s5pv210_defconfig did not have IIO already enabled
>>>> so let's enable it to avoid regression.
>>>>
>>>> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
>>>> ---
>>>>  arch/arm/configs/s5pv210_defconfig | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/arch/arm/configs/s5pv210_defconfig b/arch/arm/configs/s5pv210_defconfig
>>>> index 4c1e480b5bbd..24070ee3d43e 100644
>>>> --- a/arch/arm/configs/s5pv210_defconfig
>>>> +++ b/arch/arm/configs/s5pv210_defconfig
>>>> @@ -97,6 +97,7 @@ CONFIG_MMC_SDHCI_S3C_DMA=y
>>>>  CONFIG_RTC_CLASS=y
>>>>  CONFIG_RTC_DRV_MAX8998=m
>>>>  CONFIG_DMADEVICES=y
>>>> +CONFIG_IIO=y
>>>>  CONFIG_PWM=y
>>>>  CONFIG_PWM_SAMSUNG=y
>>>>  CONFIG_PHY_SAMSUNG_USB2=m
>>>
>>> Should this patch be squashed to the previous patch? I think you
>>> break bisectability for this board if you enable iio only here.
>>
>> The defconfig change matters less - distros don't use them - so this
>> points to the fact that patchset affected the users. All existing users
>> of max17040 drivers, who do not enable IIO, will have their setups broken.
> 
> That's why I'm suggesting to squash this patch with the previous.

It would not solve much. All existing users will be still broken.

> 
> Anyway, up to you... except of this note everything looks fine in
> the series.

I would actually prefer not to depend on IIO, but this would require
stubs for missing IIO functions.

Best regards,
Krzysztof

Re: (subset) [PATCH v3 4/4] ARM: configs: s5pv210_defconfig: enable IIO required by MAX17040

[Krzysztof Kozlowski]

On Mon, 31 Jul 2023 10:36:13 +0300, Svyatoslav Ryhel wrote:
> After adding support for passing temperature data from thermal sensor
> to MAX17040 it got dependency on CONFIG_IIO. From all defconfigs
> using MAX17040 only s5pv210_defconfig did not have IIO already enabled
> so let's enable it to avoid regression.
> 
> 

Applied, thanks!

[4/4] ARM: configs: s5pv210_defconfig: enable IIO required by MAX17040
      https://git.kernel.org/krzk/linux/c/dc836afd2be7618d8c849fd93bd3e15513289b70

Best regards,
-- 
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Re: [PATCH v3 0/4] Add optional properties to MAX17040

[Iskren Chernev]

On 7/31/23 10:36, Svyatoslav Ryhel wrote:
> Extend properties supported by max17040 fuel gauge if it is accompanied
> by different devices.
> 
> If max17040 is coupled with a charger, pass charger status since it should
> match and max17040 has no dedicated status detection ability.
> 
> max17040_get_online can be reused for PRESENT property since if it is
> online it must be present.
> 
> Finally, max17040 may be coupled with a dedicated thermal sensor which
> monitors battery temperature so lets add support for iio channel to match
> hw setup. With that said, the driver got dependency on CONFIG_IIO which
> was added to Kconfig. All defconfigs apart s5pv210_defconfig have IIO
> already enabled so only s5pv210_defconfig needed adjustment.

[whole series]
Reviewed-by: Iskren Chernev <me@iskren.info>

> ---
> Changes from v2:
> - documentation: fixed typo i2c0 > i2c
> - added dependency on CONFIG_IIO
> - enabled CONFIG_IIO for s5pv210_defconfig to avoid regressions (all other
>   defconfigs which include max17040 already have IIO enabled)
> 
> Changes from v1:
> - documentation: dropped monitored-battery and power-supplies (inherited
>   from inclusion)
> - dropped passing charger health as battery health
> - dropped patch for simple battery cell support
> - switched iio_read_channel_raw to iio_read_channel_processed_scale
> - switched iio_channel_get to devm_iio_channel_get
> - re-organized implementation of temp channel (implemented in way 
>   *_get_optional functions usually act)
> ---
> 
> Svyatoslav Ryhel (4):
>   dt-bindings: power: supply: maxim,max17040: update properties
>   power: max17040: pass status property from supplier
>   power: max17040: get thermal data from adc if available
>   ARM: configs: s5pv210_defconfig: enable IIO required by MAX17040
> 
>  .../bindings/power/supply/maxim,max17040.yaml | 31 +++++++++++++++++++
>  arch/arm/configs/s5pv210_defconfig            |  1 +
>  drivers/power/supply/Kconfig                  |  2 +-
>  drivers/power/supply/max17040_battery.c       | 27 ++++++++++++++++
>  4 files changed, 60 insertions(+), 1 deletion(-)
> 

Re: (subset) [PATCH v3 0/4] Add optional properties to MAX17040

[Sebastian Reichel]

On Mon, 31 Jul 2023 10:36:09 +0300, Svyatoslav Ryhel wrote:
> Extend properties supported by max17040 fuel gauge if it is accompanied
> by different devices.
> 
> If max17040 is coupled with a charger, pass charger status since it should
> match and max17040 has no dedicated status detection ability.
> 
> max17040_get_online can be reused for PRESENT property since if it is
> online it must be present.
> 
> [...]

Applied, thanks!

[3/4] power: max17040: get thermal data from adc if available
      commit: 814755c48f8b2c3e83b3c11535c48ab416128978

Best regards,
-- 
Sebastian Reichel <sebastian.reichel@collabora.com>