[PATCH 0/7] Cleanup SBS power-supply drivers

[PATCH 0/7] Cleanup SBS power-supply drivers

[Sebastian Reichel]

Hi,

This is a collection of cleanups for the SBS battery/charger/manager.
The series does three things:

1. remove legacy gpio usage (only headers needed updates)
2. simple code cleanups
3. remove probe defer message logging

To provide some more data for the last point: The following messages
appeared on a SBS battery using system if the battery driver is probed
before the charger at default loglevel and will be degraded to debug
level:

[    0.348325] power_supply sbs-0-000b: Not all required supplies found, defer probe
[    0.348337] sbs-battery 0-000b: sbs_probe: Failed to register power supply
[    0.588072] power_supply sbs-0-000b: sbs-0-000b: Found supply : battery-charger

-- Sebastian

Sebastian Reichel (7):
  power: supply: sbs-battery: use dev_err_probe
  power: supply: sbs-charger: use dev_err_probe
  power: supply: sbs-charger: drop unused gpio includes
  power: supply: sbs-manager: use managed i2c_mux_adapter
  power: supply: sbs-manager: use dev_err_probe
  power: supply: sbs-manager: update gpio include
  power: supply: core: reduce loglevel for probe defer info

 drivers/power/supply/power_supply_core.c |  4 +-
 drivers/power/supply/sbs-battery.c       | 28 +++------
 drivers/power/supply/sbs-charger.c       | 24 +++-----
 drivers/power/supply/sbs-manager.c       | 78 +++++++++---------------
 4 files changed, 47 insertions(+), 87 deletions(-)

-- 
2.30.1

[PATCH 1/7] power: supply: sbs-battery: use dev_err_probe

[Sebastian Reichel]

Introduce usage of dev_err_probe in probe routine, which
makes the code slightly more readable and removes some
lines of code. It also removes PROBE_DEFER errors being
logged by default, which are common when the battery is
waiting for the charger driver to be registered.

This also cleans up a useless goto and instead returns
directly.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/power/supply/sbs-battery.c | 28 +++++++++-------------------
 1 file changed, 9 insertions(+), 19 deletions(-)

diff --git a/drivers/power/supply/sbs-battery.c b/drivers/power/supply/sbs-battery.c
index b6a538ebb378..4bf92831cb06 100644
--- a/drivers/power/supply/sbs-battery.c
+++ b/drivers/power/supply/sbs-battery.c
@@ -1123,11 +1123,9 @@ static int sbs_probe(struct i2c_client *client)
 
 	chip->gpio_detect = devm_gpiod_get_optional(&client->dev,
 			"sbs,battery-detect", GPIOD_IN);
-	if (IS_ERR(chip->gpio_detect)) {
-		dev_err(&client->dev, "Failed to get gpio: %ld\n",
-			PTR_ERR(chip->gpio_detect));
-		return PTR_ERR(chip->gpio_detect);
-	}
+	if (IS_ERR(chip->gpio_detect))
+		return dev_err_probe(&client->dev, PTR_ERR(chip->gpio_detect),
+				     "Failed to get gpio\n");
 
 	i2c_set_clientdata(client, chip);
 
@@ -1158,31 +1156,23 @@ static int sbs_probe(struct i2c_client *client)
 
 		rc = sbs_get_battery_presence_and_health(
 				client, POWER_SUPPLY_PROP_PRESENT, &val);
-		if (rc < 0 || !val.intval) {
-			dev_err(&client->dev, "Failed to get present status\n");
-			rc = -ENODEV;
-			goto exit_psupply;
-		}
+		if (rc < 0 || !val.intval)
+			return dev_err_probe(&client->dev, -ENODEV,
+					     "Failed to get present status\n");
 	}
 
 	INIT_DELAYED_WORK(&chip->work, sbs_delayed_work);
 
 	chip->power_supply = devm_power_supply_register(&client->dev, sbs_desc,
 						   &psy_cfg);
-	if (IS_ERR(chip->power_supply)) {
-		dev_err(&client->dev,
-			"%s: Failed to register power supply\n", __func__);
-		rc = PTR_ERR(chip->power_supply);
-		goto exit_psupply;
-	}
+	if (IS_ERR(chip->power_supply))
+		return dev_err_probe(&client->dev, PTR_ERR(chip->power_supply),
+				     "Failed to register power supply\n");
 
 	dev_info(&client->dev,
 		"%s: battery gas gauge device registered\n", client->name);
 
 	return 0;
-
-exit_psupply:
-	return rc;
 }
 
 static int sbs_remove(struct i2c_client *client)
-- 
2.30.1

[PATCH 2/7] power: supply: sbs-charger: use dev_err_probe

[Sebastian Reichel]

Introduce usage of dev_err_probe in probe routine, which
makes the code slightly more readable and removes some
lines of code. It also removes PROBE_DEFER errors being
logged by default.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/power/supply/sbs-charger.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/power/supply/sbs-charger.c b/drivers/power/supply/sbs-charger.c
index fbfb6a620961..36a048d687e0 100644
--- a/drivers/power/supply/sbs-charger.c
+++ b/drivers/power/supply/sbs-charger.c
@@ -189,18 +189,14 @@ static int sbs_probe(struct i2c_client *client,
 	 * to the battery.
 	 */
 	ret = regmap_read(chip->regmap, SBS_CHARGER_REG_STATUS, &val);
-	if (ret) {
-		dev_err(&client->dev, "Failed to get device status\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(&client->dev, ret, "Failed to get device status\n");
 	chip->last_state = val;
 
-	chip->power_supply = devm_power_supply_register(&client->dev, &sbs_desc,
-							&psy_cfg);
-	if (IS_ERR(chip->power_supply)) {
-		dev_err(&client->dev, "Failed to register power supply\n");
-		return PTR_ERR(chip->power_supply);
-	}
+	chip->power_supply = devm_power_supply_register(&client->dev, &sbs_desc, &psy_cfg);
+	if (IS_ERR(chip->power_supply))
+		return dev_err_probe(&client->dev, PTR_ERR(chip->power_supply),
+				     "Failed to register power supply\n");
 
 	/*
 	 * The sbs-charger spec doesn't impose the use of an interrupt. So in
@@ -212,10 +208,8 @@ static int sbs_probe(struct i2c_client *client,
 					NULL, sbs_irq_thread,
 					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
 					dev_name(&client->dev), chip);
-		if (ret) {
-			dev_err(&client->dev, "Failed to request irq, %d\n", ret);
-			return ret;
-		}
+		if (ret)
+			return dev_err_probe(&client->dev, ret, "Failed to request irq\n");
 	} else {
 		INIT_DELAYED_WORK(&chip->work, sbs_delayed_work);
 		schedule_delayed_work(&chip->work,
-- 
2.30.1

[PATCH 3/7] power: supply: sbs-charger: drop unused gpio includes

[Sebastian Reichel]

sbs-charger does not use any GPIOs, so no need to include
gpio.h and of_gpio.h.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/power/supply/sbs-charger.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/power/supply/sbs-charger.c b/drivers/power/supply/sbs-charger.c
index 36a048d687e0..6fa65d118ec1 100644
--- a/drivers/power/supply/sbs-charger.c
+++ b/drivers/power/supply/sbs-charger.c
@@ -16,9 +16,7 @@
 #include <linux/i2c.h>
 #include <linux/slab.h>
 #include <linux/interrupt.h>
-#include <linux/gpio.h>
 #include <linux/regmap.h>
-#include <linux/of_gpio.h>
 #include <linux/bitops.h>
 
 #define SBS_CHARGER_REG_SPEC_INFO		0x11
-- 
2.30.1

[PATCH 4/7] power: supply: sbs-manager: use managed i2c_mux_adapter

[Sebastian Reichel]

Simplify code by using devm_add_action_or_reset to unregister
the i2c_mux_adapter.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/power/supply/sbs-manager.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/power/supply/sbs-manager.c b/drivers/power/supply/sbs-manager.c
index 666243d9dd59..cd2bf0b247fe 100644
--- a/drivers/power/supply/sbs-manager.c
+++ b/drivers/power/supply/sbs-manager.c
@@ -311,6 +311,12 @@ static const struct power_supply_desc sbsm_default_psy_desc = {
 	.property_is_writeable = &sbsm_prop_is_writeable,
 };
 
+static void sbsm_del_mux_adapter(void *data)
+{
+	struct sbsm_data *sbsm = data;
+	i2c_mux_del_adapters(sbsm->muxc);
+}
+
 static int sbsm_probe(struct i2c_client *client,
 		      const struct i2c_device_id *id)
 {
@@ -350,6 +356,10 @@ static int sbsm_probe(struct i2c_client *client,
 	}
 	data->muxc->priv = data;
 
+	ret = devm_add_action_or_reset(dev, sbsm_del_mux_adapter, data);
+	if (ret)
+		return ret;
+
 	/* register muxed i2c channels. One for each supported battery */
 	for (i = 0; i < SBSM_MAX_BATS; ++i) {
 		if (data->supported_bats & BIT(i)) {
@@ -395,20 +405,10 @@ static int sbsm_probe(struct i2c_client *client,
 
 err_psy:
 err_mux_register:
-	i2c_mux_del_adapters(data->muxc);
-
 err_mux_alloc:
 	return ret;
 }
 
-static int sbsm_remove(struct i2c_client *client)
-{
-	struct sbsm_data *data = i2c_get_clientdata(client);
-
-	i2c_mux_del_adapters(data->muxc);
-	return 0;
-}
-
 static const struct i2c_device_id sbsm_ids[] = {
 	{ "sbs-manager", 0 },
 	{ "ltc1760",     0 },
@@ -431,7 +431,6 @@ static struct i2c_driver sbsm_driver = {
 		.of_match_table = of_match_ptr(sbsm_dt_ids),
 	},
 	.probe		= sbsm_probe,
-	.remove		= sbsm_remove,
 	.alert		= sbsm_alert,
 	.id_table	= sbsm_ids
 };
-- 
2.30.1

[PATCH 5/7] power: supply: sbs-manager: use dev_err_probe

[Sebastian Reichel]

Introduce usage of dev_err_probe in probe routine, which
makes the code slightly more readable and removes some
lines of code. It also removes PROBE_DEFER errors being
logged by default.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/power/supply/sbs-manager.c | 55 +++++++++---------------------
 1 file changed, 17 insertions(+), 38 deletions(-)

diff --git a/drivers/power/supply/sbs-manager.c b/drivers/power/supply/sbs-manager.c
index cd2bf0b247fe..bd2af3ef1021 100644
--- a/drivers/power/supply/sbs-manager.c
+++ b/drivers/power/supply/sbs-manager.c
@@ -294,10 +294,8 @@ static int sbsm_gpio_setup(struct sbsm_data *data)
 	gc->owner = THIS_MODULE;
 
 	ret = devm_gpiochip_add_data(dev, gc, data);
-	if (ret) {
-		dev_err(dev, "devm_gpiochip_add_data failed: %d\n", ret);
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "devm_gpiochip_add_data failed\n");
 
 	return ret;
 }
@@ -349,11 +347,8 @@ static int sbsm_probe(struct i2c_client *client,
 	data->supported_bats = ret & SBSM_MASK_BAT_SUPPORTED;
 	data->muxc = i2c_mux_alloc(adapter, dev, SBSM_MAX_BATS, 0,
 				   I2C_MUX_LOCKED, &sbsm_select, NULL);
-	if (!data->muxc) {
-		dev_err(dev, "failed to alloc i2c mux\n");
-		ret = -ENOMEM;
-		goto err_mux_alloc;
-	}
+	if (!data->muxc)
+		return dev_err_probe(dev, -ENOMEM, "failed to alloc i2c mux\n");
 	data->muxc->priv = data;
 
 	ret = devm_add_action_or_reset(dev, sbsm_del_mux_adapter, data);
@@ -368,45 +363,29 @@ static int sbsm_probe(struct i2c_client *client,
 				break;
 		}
 	}
-	if (ret) {
-		dev_err(dev, "failed to register i2c mux channel %d\n", i + 1);
-		goto err_mux_register;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to register i2c mux channel %d\n", i + 1);
 
-	psy_desc = devm_kmemdup(dev, &sbsm_default_psy_desc,
-				sizeof(struct power_supply_desc),
-				GFP_KERNEL);
-	if (!psy_desc) {
-		ret = -ENOMEM;
-		goto err_psy;
-	}
+	psy_desc = devm_kmemdup(dev, &sbsm_default_psy_desc, sizeof(*psy_desc), GFP_KERNEL);
+	if (!psy_desc)
+		return -ENOMEM;
+
+	psy_desc->name = devm_kasprintf(dev, GFP_KERNEL, "sbsm-%s", dev_name(&client->dev));
+	if (!psy_desc->name)
+		return -ENOMEM;
 
-	psy_desc->name = devm_kasprintf(dev, GFP_KERNEL, "sbsm-%s",
-					dev_name(&client->dev));
-	if (!psy_desc->name) {
-		ret = -ENOMEM;
-		goto err_psy;
-	}
 	ret = sbsm_gpio_setup(data);
 	if (ret < 0)
-		goto err_psy;
+		return ret;
 
 	psy_cfg.drv_data = data;
 	psy_cfg.of_node = dev->of_node;
 	data->psy = devm_power_supply_register(dev, psy_desc, &psy_cfg);
-	if (IS_ERR(data->psy)) {
-		ret = PTR_ERR(data->psy);
-		dev_err(dev, "failed to register power supply %s\n",
-			psy_desc->name);
-		goto err_psy;
-	}
+	if (IS_ERR(data->psy))
+		return dev_err_probe(dev, PTR_ERR(data->psy),
+				     "failed to register power supply %s\n", psy_desc->name);
 
 	return 0;
-
-err_psy:
-err_mux_register:
-err_mux_alloc:
-	return ret;
 }
 
 static const struct i2c_device_id sbsm_ids[] = {
-- 
2.30.1

[PATCH 6/7] power: supply: sbs-manager: update gpio include

[Sebastian Reichel]

sbs-manager implements a GPIO chip, so include the proper
gpio driver include instead of the legacy gpio.h.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/power/supply/sbs-manager.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/power/supply/sbs-manager.c b/drivers/power/supply/sbs-manager.c
index bd2af3ef1021..71ec8f74f835 100644
--- a/drivers/power/supply/sbs-manager.c
+++ b/drivers/power/supply/sbs-manager.c
@@ -13,7 +13,7 @@
  * Karl-Heinz Schneider <karl-heinz@schneider-inet.de>
  */
 
-#include <linux/gpio.h>
+#include <linux/gpio/driver.h>
 #include <linux/module.h>
 #include <linux/i2c.h>
 #include <linux/i2c-mux.h>
-- 
2.30.1

[PATCH 7/7] power: supply: core: reduce loglevel for probe defer info

[Sebastian Reichel]

Avoid logging probe defer information for default loglevel
configurations. This is only required for debugging probe
defer issues, which requires enabling debug messages for
other subsystems.

This dev_info() message predates having deferred devices
information available in /sys/kernel/debug/devices_deferred,
which is generally more useful.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/power/supply/power_supply_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index 38e3aa642131..d99e2f11c183 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -169,7 +169,7 @@ static int __power_supply_populate_supplied_from(struct device *dev,
 			break;
 
 		if (np == epsy->of_node) {
-			dev_info(&psy->dev, "%s: Found supply : %s\n",
+			dev_dbg(&psy->dev, "%s: Found supply : %s\n",
 				psy->desc->name, epsy->desc->name);
 			psy->supplied_from[i-1] = (char *)epsy->desc->name;
 			psy->num_supplies++;
@@ -1143,7 +1143,7 @@ __power_supply_register(struct device *parent,
 
 	rc = power_supply_check_supplies(psy);
 	if (rc) {
-		dev_info(dev, "Not all required supplies found, defer probe\n");
+		dev_dbg(dev, "Not all required supplies found, defer probe\n");
 		goto check_supplies_failed;
 	}
 
-- 
2.30.1