| 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: <54C57FE2.3070908@samsung.com>
Date: Mon, 26 Jan 2015 08:44:34 +0900
From: Beomho Seo <beomho.seo@...sung.com>
To: Sebastian Reichel <sre@...nel.org>,
Jaewon Kim <jaewon02.kim@...sung.com>
Cc: linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
linux-pm@...r.kernel.org, Inki Dae <inki.dae@...sung.com>,
Rob Herring <robh+dt@...nel.org>,
Pawel Moll <pawel.moll@....com>,
Mark Rutland <mark.rutland@....com>,
Ian Campbell <ijc+devicetree@...lion.org.uk>,
Kumar Gala <galak@...eaurora.org>,
Lee Jones <lee.jones@...aro.org>,
Chanwoo Choi <cw00.choi@...sung.com>,
Mark Brown <broonie@...nel.org>
Subject: Re: [PATCH 4/6] power: max77843_battery: Add Max77843 fuel gauge
device driver
Hi,
Thanks for your review.
I'll fix patch as your advice.
Thanks,
Beomho Seo
On 01/25/2015 10:53 PM, Sebastian Reichel wrote:
> Hi,
>
> On Fri, Jan 23, 2015 at 02:02:45PM +0900, Jaewon Kim wrote:
>> From: Beomho Seo <beomho.seo@...sung.com>
>>
>> This patch adds device driver of max77843 fuel gauge.
>> The driver support for battery fuel gauge in Maxim Max77843.
>> It is fuel-gauge systems for lithuum-ion batteries in handled and
>> portable devices.
>>
>> Cc: Sebastian Reichel <sre@...nel.org>
>> Signed-off-by: Beomho Seo <beomho.seo@...sung.com>
>> ---
>> drivers/power/Kconfig | 9 ++
>> drivers/power/Makefile | 1 +
>> drivers/power/max77843_battery.c | 283 ++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 293 insertions(+)
>> create mode 100644 drivers/power/max77843_battery.c
>>
>> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
>> index a054a28..0039bbb 100644
>> --- a/drivers/power/Kconfig
>> +++ b/drivers/power/Kconfig
>> @@ -212,6 +212,15 @@ config BATTERY_MAX17042
>> with MAX17042. This driver also supports max17047/50 chips which are
>> improved version of max17042.
>>
>> +config BATTERY_MAX77843
>> + tristate "Maxim MAX77843 Fuel Gauge"
>> + depends on MFD_MAX77843
>> + help
>> + This add support for battery fuel gauge in Maxim MAX77843.
>
> add -> adds
>
>> + It is fuel-gauge systems for lithuum-ion (Li+) batteries in handled and
>> + portable devices. The MAX17040 is configured to operate with a single
>> + lithium cell.
>
> It is a fuel-gauge for a lithium-ion batteries with a single
> cell and can be found in portable devices.
>
>> +
>> config BATTERY_Z2
>> tristate "Z2 battery driver"
>> depends on I2C && MACH_ZIPIT2
>> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
>> index 212c6a2..ae0d795 100644
>> --- a/drivers/power/Makefile
>> +++ b/drivers/power/Makefile
>> @@ -33,6 +33,7 @@ obj-$(CONFIG_BATTERY_DA9030) += da9030_battery.o
>> obj-$(CONFIG_BATTERY_DA9052) += da9052-battery.o
>> obj-$(CONFIG_BATTERY_MAX17040) += max17040_battery.o
>> obj-$(CONFIG_BATTERY_MAX17042) += max17042_battery.o
>> +obj-$(CONFIG_BATTERY_MAX77843) += max77843_battery.o
>> obj-$(CONFIG_BATTERY_Z2) += z2_battery.o
>> obj-$(CONFIG_BATTERY_S3C_ADC) += s3c_adc_battery.o
>> obj-$(CONFIG_BATTERY_TWL4030_MADC) += twl4030_madc_battery.o
>> diff --git a/drivers/power/max77843_battery.c b/drivers/power/max77843_battery.c
>> new file mode 100644
>> index 0000000..a08dd0c
>> --- /dev/null
>> +++ b/drivers/power/max77843_battery.c
>> @@ -0,0 +1,283 @@
>> +/*
>> + * Fuel gauge driver for Maxim MAX77843
>> + *
>> + * Copyright (C) 2014 Samsung Electronics, Co., Ltd.
>> + * Author: Beomho Seo <beomho.seo@...sung.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published bythe Free Software Foundation.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/power_supply.h>
>> +#include <linux/mfd/max77843-private.h>
>> +
>> +struct max77843_battery {
>> + struct device *dev;
>> + struct max77843 *max77843;
>> + struct i2c_client *client;
>> + struct regmap *regmap;
>> + struct power_supply psy;
>> +};
>> +
>> +static int max77843_battery_get_capacity(struct max77843_battery *battery)
>> +{
>> + struct regmap *regmap = battery->regmap;
>> + int ret, val;
>> + u8 reg_data[2];
>> +
>> + ret = regmap_bulk_read(regmap, MAX77843_FG_REG_SOCREP, reg_data, 2);
>> + if (ret) {
>> + dev_err(battery->dev, "Failed to read fuelgauge register\n");
>> + return ret;
>> + }
>> +
>> + val = ((reg_data[1] * 100) + (reg_data[0] * 100 / 256)) / 100;
>
> so
>
> val = reg_data[1]?
>
>> + return val;
>> +}
>> +
>> +static int max77843_battery_get_energy_prop(struct max77843_battery *battery,
>> + enum power_supply_property psp)
>> +{
>> + struct regmap *regmap = battery->regmap;
>> + unsigned int reg;
>> + int ret, val;
>> + u8 reg_data[2];
>> +
>> + switch (psp) {
>> + case POWER_SUPPLY_PROP_ENERGY_FULL:
>> + reg = MAX77843_FG_REG_FULLCAP;
>> + break;
>> + case POWER_SUPPLY_PROP_ENERGY_NOW:
>> + reg = MAX77843_FG_REG_REMCAP_REP;
>> + break;
>> + case POWER_SUPPLY_PROP_ENERGY_AVG:
>> + reg = MAX77843_FG_REG_REMCAP_AV;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + ret = regmap_bulk_read(regmap, reg, reg_data, 2);
>> + if (ret) {
>> + dev_err(battery->dev, "Failed to read fuelgauge register\n");
>> + return ret;
>> + }
>> +
>> + val = (reg_data[1] << 8) | reg_data[0];
>> +
>> + return val;
>> +}
>> +
>> +static int max77843_battery_get_current_prop(struct max77843_battery *battery,
>> + enum power_supply_property psp)
>> +{
>> + struct regmap *regmap = battery->regmap;
>> + unsigned int reg;
>> + int ret, val;
>> + u8 reg_data[2];
>> +
>> + switch (psp) {
>> + case POWER_SUPPLY_PROP_CURRENT_NOW:
>> + reg = MAX77843_FG_REG_CURRENT;
>> + break;
>> + case POWER_SUPPLY_PROP_CURRENT_AVG:
>> + reg = MAX77843_FG_REG_AVG_CURRENT;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + ret = regmap_bulk_read(regmap, reg, reg_data, 2);
>> + if (ret) {
>> + dev_err(battery->dev, "Failed to read fuelgauge register\n");
>> + return ret;
>> + }
>> +
>> + val = (reg_data[1] << 8) | reg_data[0];
>> + if (val & 0x8000) {
>> + /* Negative */
>> + val = ~val & 0xffff;
>> + val++;
>> + val *= -1;
>> + }
>> + /* Unit of current is mA */
>> + val = val * 15625 / 100000;
>> +
>> + return val;
>> +}
>> +
>> +static int max77843_battery_get_voltage_prop(struct max77843_battery *battery,
>> + enum power_supply_property psp)
>> +{
>> + struct regmap *regmap = battery->regmap;
>> + unsigned int reg;
>> + int ret, val;
>> + u8 reg_data[2];
>> +
>> + switch (psp) {
>> + case POWER_SUPPLY_PROP_VOLTAGE_NOW:
>> + reg = MAX77843_FG_REG_VCELL;
>> + break;
>> + case POWER_SUPPLY_PROP_VOLTAGE_AVG:
>> + reg = MAX77843_FG_REG_AVG_VCELL;
>> + break;
>> + case POWER_SUPPLY_PROP_VOLTAGE_OCV:
>> + reg = MAX77843_FG_REG_OCV;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + ret = regmap_bulk_read(regmap, reg, reg_data, 2);
>> + if (ret) {
>> + dev_err(battery->dev, "Failed to read fuelgauge register\n");
>> + return ret;
>> + }
>> +
>> + val = ((reg_data[1] << 4) + (reg_data[0] >> 4)) * 125;
>> + val /= 100;
>> +
>> + return val;
>> +}
>> +
>> +static const char *model_name = "MAX77843";
>> +static const char *manufacturer = "Maxim Integrated";
>> +
>> +static int max77843_battery_get_property(struct power_supply *psy,
>> + enum power_supply_property psp,
>> + union power_supply_propval *val)
>> +{
>> + struct max77843_battery *battery = container_of(psy,
>> + struct max77843_battery, psy);
>> + switch (psp) {
>> + case POWER_SUPPLY_PROP_VOLTAGE_NOW:
>> + case POWER_SUPPLY_PROP_VOLTAGE_AVG:
>> + case POWER_SUPPLY_PROP_VOLTAGE_OCV:
>> + val->intval = max77843_battery_get_voltage_prop(battery, psp);
>> + break;
>> + case POWER_SUPPLY_PROP_CURRENT_NOW:
>> + case POWER_SUPPLY_PROP_CURRENT_AVG:
>> + val->intval = max77843_battery_get_current_prop(battery, psp);
>> + break;
>> + case POWER_SUPPLY_PROP_ENERGY_FULL:
>> + case POWER_SUPPLY_PROP_ENERGY_NOW:
>> + case POWER_SUPPLY_PROP_ENERGY_AVG:
>> + val->intval = max77843_battery_get_energy_prop(battery, psp);
>> + break;
>> + case POWER_SUPPLY_PROP_CAPACITY:
>> + val->intval = max77843_battery_get_capacity(battery);
>> + break;
>> + case POWER_SUPPLY_PROP_MODEL_NAME:
>> + val->strval = model_name;
>> + break;
>> + case POWER_SUPPLY_PROP_MANUFACTURER:
>> + val->strval = manufacturer;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static enum power_supply_property max77843_battery_props[] = {
>> + POWER_SUPPLY_PROP_VOLTAGE_NOW,
>> + POWER_SUPPLY_PROP_VOLTAGE_AVG,
>> + POWER_SUPPLY_PROP_VOLTAGE_OCV,
>> + POWER_SUPPLY_PROP_CURRENT_NOW,
>> + POWER_SUPPLY_PROP_CURRENT_AVG,
>> + POWER_SUPPLY_PROP_ENERGY_FULL,
>> + POWER_SUPPLY_PROP_ENERGY_NOW,
>> + POWER_SUPPLY_PROP_ENERGY_AVG,
>> + POWER_SUPPLY_PROP_CAPACITY,
>> + POWER_SUPPLY_PROP_MODEL_NAME,
>> + POWER_SUPPLY_PROP_MANUFACTURER,
>> +};
>> +
>> +static const struct regmap_config max77843_fuel_regmap_config = {
>> + .reg_bits = 8,
>> + .val_bits = 8,
>> + .max_register = MAX77843_FG_END,
>> +};
>
> You always read 2 bytes, so it seems the device uses 16 bit sized
> values?
>
>> +static int max77843_battery_probe(struct platform_device *pdev)
>> +{
>> + struct max77843 *max77843 = dev_get_drvdata(pdev->dev.parent);
>> + struct max77843_battery *battery;
>> + int ret;
>> +
>> + battery = devm_kzalloc(&pdev->dev, sizeof(*battery), GFP_KERNEL);
>> + if (!battery)
>> + return -ENOMEM;
>> +
>> + battery->dev = &pdev->dev;
>> + battery->max77843 = max77843;
>> +
>> + battery->client = i2c_new_dummy(max77843->i2c->adapter, I2C_ADDR_FG);
>> + if (!battery->client) {
>> + dev_err(&pdev->dev, "Failed to get dummy i2c client.\n");
>> + return PTR_ERR(battery->client);
>> + }
>> + i2c_set_clientdata(battery->client, max77843);
>> +
>> + battery->regmap = devm_regmap_init_i2c(battery->client,
>> + &max77843_fuel_regmap_config);
>> + if (IS_ERR(battery->regmap)) {
>> + ret = PTR_ERR(battery->regmap);
>> + goto err_i2c;
>> + }
>> +
>> + platform_set_drvdata(pdev, battery);
>> +
>> + battery->psy.name = "max77843-fuelgauge";
>> + battery->psy.type = POWER_SUPPLY_TYPE_BATTERY;
>> + battery->psy.get_property = max77843_battery_get_property;
>> + battery->psy.properties = max77843_battery_props;
>> + battery->psy.num_properties = ARRAY_SIZE(max77843_battery_props);
>> +
>> + ret = power_supply_register(&pdev->dev, &battery->psy);
>> + if (ret) {
>> + dev_err(&pdev->dev, "Failed to register power supply\n");
>> + goto err_i2c;
>> + }
>> +
>> + return 0;
>> +
>> +err_i2c:
>> + i2c_unregister_device(battery->client);
>
> This is missing in max77843_battery_remove()
>
>> +
>> + return ret;
>> +}
>> +
>> +static int max77843_battery_remove(struct platform_device *pdev)
>> +{
>> + struct max77843_battery *battery = platform_get_drvdata(pdev);
>> +
>> + power_supply_unregister(&battery->psy);
>> +
>> + return 0;
>> +}
>
>> [...]
>
> -- Sebastian
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists