[v3,1/2] power: supply: Add STC3117 fuel gauge unit driver
Commit Message
Adding minimal support for stc3117 fuel gauge driver
to read battery voltage level
Signed-off-by: Bhavin Sharma <bhavin.sharma@siliconsignals.io>
---
Changelogs :
v2 -> v3
- Resolved kernel test robot build warnings
- Aligned included header files in alphabetical order
- Removed unused header files
- Removed unnecessary blank lines
- Aligned all the macros in alphabetical order
- Changed macro LSB_VALUE to VOLTAGE_LSB_VALUE
- Dropped function prototypes and arranged the code accordingly
- Used macros instead of static numbers for array declaration
- Removed redundant code
- Replaced 'power_supply_register' with 'devm_power_supply_register' and 'pr_err' with 'dev_err'
- Removed global variables
v1 -> v2
- No change
---
drivers/power/supply/Kconfig | 7 ++
drivers/power/supply/Makefile | 1 +
drivers/power/supply/stc3117_fuel_gauge.c | 107 ++++++++++++++++++++++
3 files changed, 115 insertions(+)
create mode 100644 drivers/power/supply/stc3117_fuel_gauge.c
Comments
Hi,
I have received comments for documentation patch of this driver and I am working on it.
Any comments or suggestions here?
Regards,
Bhavin Sharma
Hi,
On Mon, Feb 05, 2024 at 10:43:17AM +0530, Bhavin Sharma wrote:
> Adding minimal support for stc3117 fuel gauge driver
> to read battery voltage level
Why only voltage?
It should be easy to support more data exposed by the chip.
> Signed-off-by: Bhavin Sharma <bhavin.sharma@siliconsignals.io>
> ---
> Changelogs :
>
> v2 -> v3
> - Resolved kernel test robot build warnings
> - Aligned included header files in alphabetical order
> - Removed unused header files
> - Removed unnecessary blank lines
> - Aligned all the macros in alphabetical order
> - Changed macro LSB_VALUE to VOLTAGE_LSB_VALUE
> - Dropped function prototypes and arranged the code accordingly
> - Used macros instead of static numbers for array declaration
> - Removed redundant code
> - Replaced 'power_supply_register' with 'devm_power_supply_register' and 'pr_err' with 'dev_err'
> - Removed global variables
>
> v1 -> v2
> - No change
> ---
> drivers/power/supply/Kconfig | 7 ++
> drivers/power/supply/Makefile | 1 +
> drivers/power/supply/stc3117_fuel_gauge.c | 107 ++++++++++++++++++++++
> 3 files changed, 115 insertions(+)
> create mode 100644 drivers/power/supply/stc3117_fuel_gauge.c
>
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index f21cb05815ec..e2e3af4bcd5f 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -875,6 +875,13 @@ config FUEL_GAUGE_SC27XX
> Say Y here to enable support for fuel gauge with SC27XX
> PMIC chips.
>
> +config FUEL_GAUGE_STC3117
> + tristate "STMicroelectronics STC3117 fuel gauge driver"
> + depends on I2C
> + help
> + Say Y here to enable support for fuel gauge with STC3117
> + PMIC chips.
> +
> config CHARGER_UCS1002
> tristate "Microchip UCS1002 USB Port Power Controller"
> depends on I2C
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index 58b567278034..be8961661bd1 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -104,6 +104,7 @@ obj-$(CONFIG_CHARGER_CROS_USBPD) += cros_usbpd-charger.o
> obj-$(CONFIG_CHARGER_CROS_PCHG) += cros_peripheral_charger.o
> obj-$(CONFIG_CHARGER_SC2731) += sc2731_charger.o
> obj-$(CONFIG_FUEL_GAUGE_SC27XX) += sc27xx_fuel_gauge.o
> +obj-$(CONFIG_FUEL_GAUGE_STC3117) += stc3117_fuel_gauge.o
> obj-$(CONFIG_CHARGER_UCS1002) += ucs1002_power.o
> obj-$(CONFIG_CHARGER_BD99954) += bd99954-charger.o
> obj-$(CONFIG_CHARGER_WILCO) += wilco-charger.o
> diff --git a/drivers/power/supply/stc3117_fuel_gauge.c b/drivers/power/supply/stc3117_fuel_gauge.c
> new file mode 100644
> index 000000000000..29eb00b44e21
> --- /dev/null
> +++ b/drivers/power/supply/stc3117_fuel_gauge.c
> @@ -0,0 +1,107 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * stc3117_fuel_gauge.c - STMicroelectronics STC3117 Fuel Gauge Driver
> + *
> + * Copyright (c) 2024 Silicon Signals Pvt Ltd.
> + * Author: Bhavin Sharma <bhavin.sharma@siliconsignals.io>
> + * Hardevsinh Palaniya <hardevsinh.palaniya@siliconsignals.com>
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/power_supply.h>
> +
> +#define VOLTAGE_DATA_SIZE 2 // in bytes
> +#define VOLTAGE_LSB_VALUE 2200 // in micro-volts
> +#define VOLTAGE_REG_ADDR 0x08
> +#define VOLTAGE_REG_ADDR_SIZE 1 // in bytes
> +
> +static int stc3117_get_batt_volt(const struct i2c_client *stc_client)
> +{
> + int ret, volt = 0;
> + char i2c_tx = VOLTAGE_REG_ADDR, i2c_rx[VOLTAGE_DATA_SIZE] = {0};
> +
> + ret = i2c_master_send(stc_client, &i2c_tx, VOLTAGE_REG_ADDR_SIZE);
> + if (ret < 0)
> + return ret;
> +
> + ret = i2c_master_recv(stc_client, i2c_rx, VOLTAGE_DATA_SIZE);
> + if (ret < 0)
> + return ret;
> +
> + volt = (i2c_rx[1] << 8) + i2c_rx[0];
> + volt *= VOLTAGE_LSB_VALUE;
> +
> + return volt;
> +}
Please use regmap.
> +static int stc3117_get_property(struct power_supply *psy,
> + enum power_supply_property psp, union power_supply_propval *val)
> +{
> + struct i2c_client *client = to_i2c_client(psy->dev.parent);
> +
> + switch (psp) {
> + case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> + val->intval = stc3117_get_batt_volt(client);
> + break;
> + default:
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +static enum power_supply_property stc3117_battery_props[] = {
> + POWER_SUPPLY_PROP_VOLTAGE_NOW,
> +};
> +
> +static const struct power_supply_desc stc3117_battery_desc = {
> + .name = "stc3117-battery",
> + .type = POWER_SUPPLY_TYPE_BATTERY,
> + .get_property = stc3117_get_property,
> + .properties = stc3117_battery_props,
> + .num_properties = ARRAY_SIZE(stc3117_battery_props),
> +};
> +
> +static int stc3117_probe(struct i2c_client *client)
> +{
> + struct power_supply_config psy_cfg = {};
> + struct device *dev;
> + struct power_supply *stc_sply;
> +
> + dev = &client->dev;
Just initialize this at declaration time.
> + psy_cfg.of_node = dev->of_node;
> +
> + stc_sply = devm_power_supply_register(dev, &stc3117_battery_desc, &psy_cfg);
> + if (IS_ERR(stc_sply))
> + dev_err(dev, "failed to register battery\n");
return dev_err_probe(dev, PTR_ERR(stc_sply), "failed to register battery\n");
> +
> + return 0;
> +}
> +
> +static const struct i2c_device_id stc3117_id[] = {
> + {"stc3117", 0},
> + {},
> +};
> +MODULE_DEVICE_TABLE(i2c, stc3117_id);
> +
> +static const struct of_device_id stc3117_of_match[] = {
> + { .compatible = "st,stc3117" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, stc3117_of_match);
> +
> +static struct i2c_driver stc3117_i2c_driver = {
> + .driver = {
> + .name = "stc3117_i2c_driver",
> + .of_match_table = stc3117_of_match,
> + },
> + .probe = stc3117_probe,
> + .id_table = stc3117_id,
> +};
> +
> +module_i2c_driver(stc3117_i2c_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Bhavin Sharma <bhavin.sharma@siliconsignals.io>");
> +MODULE_AUTHOR("Hardevsinh Palaniya <hardevsinh.palaniya@siliconsignals.io>");
> +MODULE_DESCRIPTION("STC3117 Fuel Gauge Driver");
Greetings,
-- Sebastian
Hi Sebastian
I apologize if I'm mistaken, but I noticed other minimal drivers in the codebase. My understanding is that using a minimal driver shouldn't cause any issues. Additionally, we can easily update this driver at any time, as we're actively updating all other drivers.
Regards,
Bhavin Sharma
Hi,
On Fri, Feb 16, 2024 at 01:34:11PM +0000, Bhavin Sharma wrote:
> I apologize if I'm mistaken, but I noticed other minimal drivers
> in the codebase.
Please point me to a driver, which just exposes the voltage.
> My understanding is that using a minimal driver shouldn't cause
> any issues. Additionally, we can easily update this driver at any
> time, as we're actively updating all other drivers.
So why are you not doing it now? Adding current, state of charge,
temperature and OCV is trivial. I'm not talking about supporting
every feature of the chip, but just the bits that are a simple
mapping between standard power supply properties and a chip
register.
-- Sebastian
@@ -875,6 +875,13 @@ config FUEL_GAUGE_SC27XX
Say Y here to enable support for fuel gauge with SC27XX
PMIC chips.
+config FUEL_GAUGE_STC3117
+ tristate "STMicroelectronics STC3117 fuel gauge driver"
+ depends on I2C
+ help
+ Say Y here to enable support for fuel gauge with STC3117
+ PMIC chips.
+
config CHARGER_UCS1002
tristate "Microchip UCS1002 USB Port Power Controller"
depends on I2C
@@ -104,6 +104,7 @@ obj-$(CONFIG_CHARGER_CROS_USBPD) += cros_usbpd-charger.o
obj-$(CONFIG_CHARGER_CROS_PCHG) += cros_peripheral_charger.o
obj-$(CONFIG_CHARGER_SC2731) += sc2731_charger.o
obj-$(CONFIG_FUEL_GAUGE_SC27XX) += sc27xx_fuel_gauge.o
+obj-$(CONFIG_FUEL_GAUGE_STC3117) += stc3117_fuel_gauge.o
obj-$(CONFIG_CHARGER_UCS1002) += ucs1002_power.o
obj-$(CONFIG_CHARGER_BD99954) += bd99954-charger.o
obj-$(CONFIG_CHARGER_WILCO) += wilco-charger.o
new file mode 100644
@@ -0,0 +1,107 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * stc3117_fuel_gauge.c - STMicroelectronics STC3117 Fuel Gauge Driver
+ *
+ * Copyright (c) 2024 Silicon Signals Pvt Ltd.
+ * Author: Bhavin Sharma <bhavin.sharma@siliconsignals.io>
+ * Hardevsinh Palaniya <hardevsinh.palaniya@siliconsignals.com>
+ */
+
+#include <linux/i2c.h>
+#include <linux/power_supply.h>
+
+#define VOLTAGE_DATA_SIZE 2 // in bytes
+#define VOLTAGE_LSB_VALUE 2200 // in micro-volts
+#define VOLTAGE_REG_ADDR 0x08
+#define VOLTAGE_REG_ADDR_SIZE 1 // in bytes
+
+static int stc3117_get_batt_volt(const struct i2c_client *stc_client)
+{
+ int ret, volt = 0;
+ char i2c_tx = VOLTAGE_REG_ADDR, i2c_rx[VOLTAGE_DATA_SIZE] = {0};
+
+ ret = i2c_master_send(stc_client, &i2c_tx, VOLTAGE_REG_ADDR_SIZE);
+ if (ret < 0)
+ return ret;
+
+ ret = i2c_master_recv(stc_client, i2c_rx, VOLTAGE_DATA_SIZE);
+ if (ret < 0)
+ return ret;
+
+ volt = (i2c_rx[1] << 8) + i2c_rx[0];
+ volt *= VOLTAGE_LSB_VALUE;
+
+ return volt;
+}
+
+static int stc3117_get_property(struct power_supply *psy,
+ enum power_supply_property psp, union power_supply_propval *val)
+{
+ struct i2c_client *client = to_i2c_client(psy->dev.parent);
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+ val->intval = stc3117_get_batt_volt(client);
+ break;
+ default:
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static enum power_supply_property stc3117_battery_props[] = {
+ POWER_SUPPLY_PROP_VOLTAGE_NOW,
+};
+
+static const struct power_supply_desc stc3117_battery_desc = {
+ .name = "stc3117-battery",
+ .type = POWER_SUPPLY_TYPE_BATTERY,
+ .get_property = stc3117_get_property,
+ .properties = stc3117_battery_props,
+ .num_properties = ARRAY_SIZE(stc3117_battery_props),
+};
+
+static int stc3117_probe(struct i2c_client *client)
+{
+ struct power_supply_config psy_cfg = {};
+ struct device *dev;
+ struct power_supply *stc_sply;
+
+ dev = &client->dev;
+
+ psy_cfg.of_node = dev->of_node;
+
+ stc_sply = devm_power_supply_register(dev, &stc3117_battery_desc, &psy_cfg);
+ if (IS_ERR(stc_sply))
+ dev_err(dev, "failed to register battery\n");
+
+ return 0;
+}
+
+static const struct i2c_device_id stc3117_id[] = {
+ {"stc3117", 0},
+ {},
+};
+MODULE_DEVICE_TABLE(i2c, stc3117_id);
+
+static const struct of_device_id stc3117_of_match[] = {
+ { .compatible = "st,stc3117" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, stc3117_of_match);
+
+static struct i2c_driver stc3117_i2c_driver = {
+ .driver = {
+ .name = "stc3117_i2c_driver",
+ .of_match_table = stc3117_of_match,
+ },
+ .probe = stc3117_probe,
+ .id_table = stc3117_id,
+};
+
+module_i2c_driver(stc3117_i2c_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Bhavin Sharma <bhavin.sharma@siliconsignals.io>");
+MODULE_AUTHOR("Hardevsinh Palaniya <hardevsinh.palaniya@siliconsignals.io>");
+MODULE_DESCRIPTION("STC3117 Fuel Gauge Driver");