| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAPVz0n3+gbAmxqevwXnepJUpr+rEtM5znMf82gvTFRzANbB4ig@mail.gmail.com>
Date: Sun, 13 Apr 2025 18:20:52 +0300
From: Svyatoslav Ryhel <clamor95@...il.com>
To: ALOK TIWARI <alok.a.tiwari@...cle.com>
Cc: 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>, 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
нд, 13 квіт. 2025 р. о 18:16 ALOK TIWARI <alok.a.tiwari@...cle.com> пише:
>
>
>
> 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
Acknowledged. Thank you.
Powered by blists - more mailing lists