Message-ID: <1d27b3b5-0ece-4abe-acd7-3a3c86490dae@oracle.com>
Date: Sun, 13 Apr 2025 20:45:45 +0530
From: ALOK TIWARI <alok.a.tiwari@...cle.com>
To: Svyatoslav Ryhel <clamor95@...il.com>, Sebastian Reichel
 <sre@...nel.org>,
        Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley
 <conor+dt@...nel.org>,
        Thierry Reding <thierry.reding@...il.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Neil Armstrong <neil.armstrong@...aro.org>,
        Jonathan Cameron <Jonathan.Cameron@...wei.com>,
        Heiko Stuebner <heiko@...ech.de>,
        Rafał Miłecki
 <rafal@...ecki.pl>,
        Aradhya Bhatia <a-bhatia1@...com>
Cc: linux-pm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-tegra@...r.kernel.org
Subject: Re: [PATCH v2 3/4] power/supply: Add driver for Pegatron Chagall
 battery



subject  -> "power: supply:"

On 13-04-2025 16:40, Svyatoslav Ryhel wrote:
> The Pegatron Chagall is an Android tablet utilizing a customized Cypress
> CG7153AM microcontroller (MCU) as its battery fuel gauge. It supports a
> single-cell battery and features a dual-color charging LED.
> 
> Signed-off-by: Svyatoslav Ryhel <clamor95@...il.com>
> ---
>   drivers/power/supply/Kconfig           |  12 +
>   drivers/power/supply/Makefile          |   1 +
>   drivers/power/supply/chagall-battery.c | 308 +++++++++++++++++++++++++
>   3 files changed, 321 insertions(+)
>   create mode 100644 drivers/power/supply/chagall-battery.c
> 
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index 9f2eef6787f7..d0fc9db524bd 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -107,6 +107,18 @@ config BATTERY_ACT8945A
>   	  Say Y here to enable support for power supply provided by
>   	  Active-semi ActivePath ACT8945A charger.
>   
> +config BATTERY_CHAGALL
> +	tristate "Pegatron Chagall battery driver"
> +	depends on I2C
> +	depends on LEDS_CLASS
> +	help
> +	  Say Y to include support for Cypress CG7153AM IC based battery
> +	  fuel gauge with custom firmware found in Pegatron Chagall based
> +	  tablet line.
> +
> +	  This driver can also be built as a module. If so, the module will be
> +	  called chagall-battery.
> +
>   config BATTERY_CPCAP
>   	tristate "Motorola CPCAP PMIC battery driver"
>   	depends on MFD_CPCAP && IIO
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index 59c4a9f40d28..4ecf48a33fdd 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -23,6 +23,7 @@ obj-$(CONFIG_CHARGER_ADP5061)	+= adp5061.o
>   obj-$(CONFIG_BATTERY_ACT8945A)	+= act8945a_charger.o
>   obj-$(CONFIG_BATTERY_AXP20X)	+= axp20x_battery.o
>   obj-$(CONFIG_CHARGER_AXP20X)	+= axp20x_ac_power.o
> +obj-$(CONFIG_BATTERY_CHAGALL)	+= chagall-battery.o
>   obj-$(CONFIG_BATTERY_CPCAP)	+= cpcap-battery.o
>   obj-$(CONFIG_BATTERY_CW2015)	+= cw2015_battery.o
>   obj-$(CONFIG_BATTERY_DS2760)	+= ds2760_battery.o
> diff --git a/drivers/power/supply/chagall-battery.c b/drivers/power/supply/chagall-battery.c
> new file mode 100644
> index 000000000000..1a278331efe7
> --- /dev/null
> +++ b/drivers/power/supply/chagall-battery.c
> @@ -0,0 +1,308 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later

Blank line require

> +#include <linux/array_size.h>
> +#include <linux/delay.h>
> +#include <linux/devm-helpers.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/leds.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/power_supply.h>
> +#include <linux/regmap.h>
> +
> +#define CHAGALL_REG_LED_AMBER				0x60
> +#define CHAGALL_REG_LED_WHITE				0x70
> +#define CHAGALL_REG_BATTERY_TEMPERATURE			0xa2
> +#define CHAGALL_REG_BATTERY_VOLTAGE			0xa4
> +#define CHAGALL_REG_BATTERY_CURRENT			0xa6
> +#define CHAGALL_REG_BATTERY_CAPACITY			0xa8
> +#define CHAGALL_REG_BATTERY_CHARGING_CURRENT		0xaa
> +#define CHAGALL_REG_BATTERY_CHARGING_VOLTAGE		0xac
> +#define CHAGALL_REG_BATTERY_STATUS			0xae
> +#define   BATTERY_DISCHARGING				BIT(6)
> +#define   BATTERY_FULL_CHARGED				BIT(5)
> +#define   BATTERY_FULL_DISCHARGED			BIT(4)

' ' after "#define", remove extra ' '

> +#define CHAGALL_REG_BATTERY_REMAIN_CAPACITY		0xb0
> +#define CHAGALL_REG_BATTERY_FULL_CAPACITY		0xb2
> +#define CHAGALL_REG_MAX_COUNT				0xb4
> +
> +#define CHAGALL_BATTERY_DATA_REFRESH			5000
> +#define TEMP_CELSIUS_OFFSET				2731
> +
> +static const struct regmap_config chagall_battery_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = CHAGALL_REG_MAX_COUNT,
> +	.reg_format_endian = REGMAP_ENDIAN_LITTLE,
> +	.val_format_endian = REGMAP_ENDIAN_LITTLE,
> +};
> +
> +struct chagall_battery_data {
> +	struct regmap *regmap;
> +	struct led_classdev amber_led;
> +	struct led_classdev white_led;
> +	struct power_supply *battery;
> +	struct delayed_work poll_work;
> +	u16 last_state;
> +};
> +
> +static void chagall_led_set_brightness_amber(struct led_classdev *led,
> +					     enum led_brightness brightness)
> +{
> +	struct chagall_battery_data *cg =
> +		container_of(led, struct chagall_battery_data, amber_led);
> +
> +	regmap_write(cg->regmap, CHAGALL_REG_LED_AMBER, brightness);
> +}
> +
> +static void chagall_led_set_brightness_white(struct led_classdev *led,
> +					     enum led_brightness brightness)
> +{
> +	struct chagall_battery_data *cg =
> +		container_of(led, struct chagall_battery_data, white_led);
> +
> +	regmap_write(cg->regmap, CHAGALL_REG_LED_WHITE, brightness);
> +}
> +
> +static void chagall_leds_status_update(struct chagall_battery_data *cg, int state)
> +{
> +	switch (state) {
> +	case POWER_SUPPLY_STATUS_FULL:
> +		led_set_brightness(&cg->amber_led, LED_OFF);
> +		led_set_brightness(&cg->white_led,  LED_ON);
> +		break;
> +

no Blank line.

> +	case POWER_SUPPLY_STATUS_CHARGING:
> +		led_set_brightness(&cg->white_led, LED_OFF);
> +		led_set_brightness(&cg->amber_led,  LED_ON);
> +		break;
> +

no Blank line.

> +	default:
> +		led_set_brightness(&cg->amber_led, LED_OFF);
> +		led_set_brightness(&cg->white_led, LED_OFF);
> +		break;
> +	}
> +}
> +
> +static const enum power_supply_property chagall_battery_properties[] = {
> +	POWER_SUPPLY_PROP_STATUS,
> +	POWER_SUPPLY_PROP_PRESENT,
> +	POWER_SUPPLY_PROP_VOLTAGE_NOW,
> +	POWER_SUPPLY_PROP_VOLTAGE_MAX,
> +	POWER_SUPPLY_PROP_CURRENT_NOW,
> +	POWER_SUPPLY_PROP_CURRENT_MAX,
> +	POWER_SUPPLY_PROP_CAPACITY,
> +	POWER_SUPPLY_PROP_TEMP,
> +	POWER_SUPPLY_PROP_CHARGE_FULL,
> +	POWER_SUPPLY_PROP_CHARGE_NOW,
> +};
> +
> +static const unsigned int chagall_battery_prop_offs[] = {
> +	[POWER_SUPPLY_PROP_TEMP] = CHAGALL_REG_BATTERY_TEMPERATURE,
> +	[POWER_SUPPLY_PROP_VOLTAGE_NOW] = CHAGALL_REG_BATTERY_VOLTAGE,
> +	[POWER_SUPPLY_PROP_CURRENT_NOW] = CHAGALL_REG_BATTERY_CURRENT,
> +	[POWER_SUPPLY_PROP_CAPACITY] = CHAGALL_REG_BATTERY_CAPACITY,
> +	[POWER_SUPPLY_PROP_CURRENT_MAX] = CHAGALL_REG_BATTERY_CHARGING_CURRENT,
> +	[POWER_SUPPLY_PROP_VOLTAGE_MAX] = CHAGALL_REG_BATTERY_CHARGING_VOLTAGE,
> +	[POWER_SUPPLY_PROP_STATUS] = CHAGALL_REG_BATTERY_STATUS,
> +	[POWER_SUPPLY_PROP_CHARGE_NOW] = CHAGALL_REG_BATTERY_REMAIN_CAPACITY,
> +	[POWER_SUPPLY_PROP_CHARGE_FULL] = CHAGALL_REG_BATTERY_FULL_CAPACITY,
> +};
> +
> +static int chagall_battery_get_value(struct chagall_battery_data *cg,
> +				     enum power_supply_property psp, u32 *val)
> +{
> +	if (psp >= ARRAY_SIZE(chagall_battery_prop_offs))
> +		return -EINVAL;
> +	if (!chagall_battery_prop_offs[psp])
> +		return -EINVAL;
> +
> +	/* Battery data is stored in 2 consecutive registers with little-endian */
> +	return regmap_bulk_read(cg->regmap, chagall_battery_prop_offs[psp], val, 2);
> +}
> +
> +static int chagall_battery_get_property(struct power_supply *psy,
> +					enum power_supply_property psp,
> +					union power_supply_propval *val)
> +{
> +	struct chagall_battery_data *cg = power_supply_get_drvdata(psy);
> +	int ret;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_PRESENT:
> +		val->intval = 1;
> +		break;
> +
> +	default:
> +		ret = chagall_battery_get_value(cg, psp, &val->intval);
> +		if (ret)
> +			return ret;
> +
> +		switch (psp) {
> +		case POWER_SUPPLY_PROP_TEMP:
> +			val->intval -= TEMP_CELSIUS_OFFSET;
> +			break;
> +
> +		case POWER_SUPPLY_PROP_VOLTAGE_MAX:
> +		case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> +		case POWER_SUPPLY_PROP_CURRENT_MAX:
> +		case POWER_SUPPLY_PROP_CURRENT_NOW:
> +		case POWER_SUPPLY_PROP_CHARGE_FULL:
> +		case POWER_SUPPLY_PROP_CHARGE_NOW:
> +			val->intval *= 1000;
> +			break;
> +
> +		case POWER_SUPPLY_PROP_STATUS:
> +			if (val->intval & BATTERY_FULL_CHARGED)
> +				val->intval = POWER_SUPPLY_STATUS_FULL;
> +			else if (val->intval & BATTERY_FULL_DISCHARGED)
> +				val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
> +			else if (val->intval & BATTERY_DISCHARGING)
> +				val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
> +			else
> +				val->intval = POWER_SUPPLY_STATUS_CHARGING;
> +			break;
> +

no blank line is required between a case label.

> +		default:
> +			break;
> +		}
> +
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +static void chagall_battery_poll_work(struct work_struct *work)
> +{
> +	struct chagall_battery_data *cg =
> +		container_of(work, struct chagall_battery_data, poll_work.work);
> +	u32 state;
> +	int ret;
> +
> +	ret = chagall_battery_get_value(cg, POWER_SUPPLY_PROP_STATUS, &state);
> +	if (ret)
> +		return;
> +
> +	if (state & BATTERY_FULL_CHARGED)
> +		state = POWER_SUPPLY_STATUS_FULL;
> +	else if (state & BATTERY_DISCHARGING)
> +		state = POWER_SUPPLY_STATUS_DISCHARGING;
> +	else
> +		state = POWER_SUPPLY_STATUS_CHARGING;
> +
> +	if (cg->last_state != state) {
> +		cg->last_state = state;
> +		power_supply_changed(cg->battery);
> +	}
> +
> +	chagall_leds_status_update(cg, state);
> +
> +	/* continuously send uevent notification */
> +	schedule_delayed_work(&cg->poll_work,
> +			      msecs_to_jiffies(CHAGALL_BATTERY_DATA_REFRESH));
> +}
> +
> +static const struct power_supply_desc chagall_battery_desc = {
> +	.name = "chagall-battery",
> +	.type = POWER_SUPPLY_TYPE_BATTERY,
> +	.properties = chagall_battery_properties,
> +	.num_properties = ARRAY_SIZE(chagall_battery_properties),
> +	.get_property = chagall_battery_get_property,
> +	.external_power_changed = power_supply_changed,
> +};
> +
> +static int chagall_battery_probe(struct i2c_client *client)
> +{
> +	struct chagall_battery_data *cg;
> +	struct device *dev = &client->dev;
> +	struct power_supply_config cfg = { };
> +	int ret;
> +
> +	cg = devm_kzalloc(dev, sizeof(*cg), GFP_KERNEL);
> +	if (!cg)
> +		return -ENOMEM;
> +
> +	cfg.drv_data = cg;
> +	cfg.fwnode = dev_fwnode(dev);
> +
> +	i2c_set_clientdata(client, cg);
> +
> +	cg->regmap = devm_regmap_init_i2c(client, &chagall_battery_regmap_config);
> +	if (IS_ERR(cg->regmap))
> +		return dev_err_probe(dev, PTR_ERR(cg->regmap), "cannot allocate regmap\n");
> +
> +	cg->last_state = POWER_SUPPLY_STATUS_UNKNOWN;
> +	cg->battery = devm_power_supply_register(dev, &chagall_battery_desc, &cfg);
> +	if (IS_ERR(cg->battery))
> +		return dev_err_probe(dev, PTR_ERR(cg->battery), "failed to register power supply\n");
> +
> +	cg->amber_led.name = "power::amber";
> +	cg->amber_led.max_brightness = 1;
> +	cg->amber_led.flags = LED_CORE_SUSPENDRESUME;
> +	cg->amber_led.brightness_set = chagall_led_set_brightness_amber;
> +
> +	ret = devm_led_classdev_register(dev, &cg->amber_led);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to register amber LED\n");
> +
> +	cg->white_led.name = "power::white";
> +	cg->white_led.max_brightness = 1;
> +	cg->white_led.flags = LED_CORE_SUSPENDRESUME;
> +	cg->white_led.brightness_set = chagall_led_set_brightness_white;
> +
> +	ret = devm_led_classdev_register(dev, &cg->white_led);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to register white LED\n");
> +
> +	led_set_brightness(&cg->amber_led, LED_OFF);
> +	led_set_brightness(&cg->white_led, LED_OFF);
> +
> +	ret = devm_delayed_work_autocancel(dev, &cg->poll_work, chagall_battery_poll_work);
> +	if (ret)
> +		return ret;
> +
> +	schedule_delayed_work(&cg->poll_work, msecs_to_jiffies(CHAGALL_BATTERY_DATA_REFRESH));

a '\n' before return is customary

> +	return 0;
> +}
> +
> +static int __maybe_unused chagall_battery_suspend(struct device *dev)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct chagall_battery_data *cg = i2c_get_clientdata(client);
> +
> +	cancel_delayed_work_sync(&cg->poll_work);

a '\n' before return

> +	return 0;
> +}
> +
> +static int __maybe_unused chagall_battery_resume(struct device *dev)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct chagall_battery_data *cg = i2c_get_clientdata(client);
> +
> +	schedule_delayed_work(&cg->poll_work, msecs_to_jiffies(CHAGALL_BATTERY_DATA_REFRESH));

a '\n' before return is customary

> +	return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(chagall_battery_pm_ops,
> +			 chagall_battery_suspend, chagall_battery_resume);
> +
> +static const struct of_device_id chagall_of_match[] = {
> +	{ .compatible = "pegatron,chagall-ec" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, chagall_of_match);
> +
> +static struct i2c_driver chagall_battery_driver = {
> +	.driver = {
> +		.name = "chagall-battery",
> +		.pm = &chagall_battery_pm_ops,
> +		.of_match_table = chagall_of_match,
> +	},
> +	.probe = chagall_battery_probe,
> +};
> +module_i2c_driver(chagall_battery_driver);
> +
> +MODULE_AUTHOR("Svyatoslav Ryhel <clamor95@...il.com>");
> +MODULE_DESCRIPTION("Pegatron Chagall fuel gauge driver");
> +MODULE_LICENSE("GPL");



Thanks,
Alok

Powered by blists - more mailing lists