2012-02-22 18:06:18

by Karol Lewandowski

[permalink] [raw]
Subject: [PATCH 0/3] power: max17042_battery: Add DT bindings

Hi,

This series of patches adds bindings to max17042_battery driver making
it possible to instantiate it from device tree description.

First, convience patch, simply replaces kzalloc with devm_kzalloc
where possible.

Second, most intrusive patch adds variables orginally found only in
platform data to max17042_chip structure. I decided it to do it that
way rather than allocating new platform data and then populating it
from device tree description.

Last patch adds DT bindings itself.

I would be grateful for any comments and I will be more than happy to
rework this patchset if needed.


Karol Lewandowski (3):
max17042_battery: Use devm_kzalloc() where applicable
max17042_battery: Preserve properties outside of platform data
max17042_battery: Make it possible to instantiate driver from DT

.../bindings/power_supply/max17042_battery.txt | 18 +++++
drivers/power/max17042_battery.c | 77 ++++++++++++++++----
2 files changed, 79 insertions(+), 16 deletions(-)
create mode 100644 Documentation/devicetree/bindings/power_supply/max17042_battery.txt

--
1.7.8.3


2012-02-22 18:06:23

by Karol Lewandowski

[permalink] [raw]
Subject: [PATCH 1/3] max17042_battery: Use devm_kzalloc() where applicable

This allows us to simplify probe and exit function.

Signed-off-by: Karol Lewandowski <[email protected]>
Signed-off-by: Kyungmin Park <[email protected]>
---
drivers/power/max17042_battery.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/power/max17042_battery.c b/drivers/power/max17042_battery.c
index 86acee2..21a3650 100644
--- a/drivers/power/max17042_battery.c
+++ b/drivers/power/max17042_battery.c
@@ -220,7 +220,7 @@ static int __devinit max17042_probe(struct i2c_client *client,
if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WORD_DATA))
return -EIO;

- chip = kzalloc(sizeof(*chip), GFP_KERNEL);
+ chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
if (!chip)
return -ENOMEM;

@@ -246,7 +246,6 @@ static int __devinit max17042_probe(struct i2c_client *client,
ret = power_supply_register(&client->dev, &chip->battery);
if (ret) {
dev_err(&client->dev, "failed: power supply register\n");
- kfree(chip);
return ret;
}

@@ -269,7 +268,6 @@ static int __devexit max17042_remove(struct i2c_client *client)
struct max17042_chip *chip = i2c_get_clientdata(client);

power_supply_unregister(&chip->battery);
- kfree(chip);
return 0;
}

--
1.7.8.3

2012-02-22 18:06:26

by Karol Lewandowski

[permalink] [raw]
Subject: [PATCH 2/3] max17042_battery: Preserve properties outside of platform data

Add fields originally found in platform data back to max17042_chip,
as the former data structure might be not available on device
tree-based systems.

This commit makes it possible to safely declare platform data with
__initdata tag.

Signed-off-by: Karol Lewandowski <[email protected]>
Signed-off-by: Kyungmin Park <[email protected]>
---
drivers/power/max17042_battery.c | 36 +++++++++++++++++++++++-------------
1 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/drivers/power/max17042_battery.c b/drivers/power/max17042_battery.c
index 21a3650..49c1377 100644
--- a/drivers/power/max17042_battery.c
+++ b/drivers/power/max17042_battery.c
@@ -33,7 +33,8 @@
struct max17042_chip {
struct i2c_client *client;
struct power_supply battery;
- struct max17042_platform_data *pdata;
+ bool enable_current_sense;
+ u32 r_sns;
};

static int max17042_write_reg(struct i2c_client *client, u8 reg, u16 value)
@@ -168,7 +169,7 @@ static int max17042_get_property(struct power_supply *psy,
val->intval = val->intval * 10 / 256;
break;
case POWER_SUPPLY_PROP_CURRENT_NOW:
- if (chip->pdata->enable_current_sense) {
+ if (chip->enable_current_sense) {
ret = max17042_read_reg(chip->client, MAX17042_Current);
if (ret < 0)
return ret;
@@ -180,13 +181,13 @@ static int max17042_get_property(struct power_supply *psy,
val->intval++;
val->intval *= -1;
}
- val->intval *= 1562500 / chip->pdata->r_sns;
+ val->intval *= 1562500 / chip->r_sns;
} else {
return -EINVAL;
}
break;
case POWER_SUPPLY_PROP_CURRENT_AVG:
- if (chip->pdata->enable_current_sense) {
+ if (chip->enable_current_sense) {
ret = max17042_read_reg(chip->client,
MAX17042_AvgCurrent);
if (ret < 0)
@@ -199,7 +200,7 @@ static int max17042_get_property(struct power_supply *psy,
val->intval++;
val->intval *= -1;
}
- val->intval *= 1562500 / chip->pdata->r_sns;
+ val->intval *= 1562500 / chip->r_sns;
} else {
return -EINVAL;
}
@@ -215,6 +216,7 @@ static int __devinit max17042_probe(struct i2c_client *client,
{
struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
struct max17042_chip *chip;
+ struct max17042_platform_data *pdata;
int ret;

if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WORD_DATA))
@@ -225,7 +227,7 @@ static int __devinit max17042_probe(struct i2c_client *client,
return -ENOMEM;

chip->client = client;
- chip->pdata = client->dev.platform_data;
+ pdata = client->dev.platform_data;

i2c_set_clientdata(client, chip);

@@ -235,13 +237,21 @@ static int __devinit max17042_probe(struct i2c_client *client,
chip->battery.properties = max17042_battery_props;
chip->battery.num_properties = ARRAY_SIZE(max17042_battery_props);

+ if (pdata) {
+ chip->r_sns = pdata->r_sns;
+ chip->enable_current_sense = pdata->enable_current_sense;
+ } else {
+ dev_warn(&client->dev, "no driver data provided\n");
+ return -ENODEV;
+ }
+
/* When current is not measured,
* CURRENT_NOW and CURRENT_AVG properties should be invisible. */
- if (!chip->pdata->enable_current_sense)
+ if (!chip->enable_current_sense)
chip->battery.num_properties -= 2;

- if (chip->pdata->r_sns == 0)
- chip->pdata->r_sns = MAX17042_DEFAULT_SNS_RESISTOR;
+ if (chip->r_sns == 0)
+ chip->r_sns = MAX17042_DEFAULT_SNS_RESISTOR;

ret = power_supply_register(&client->dev, &chip->battery);
if (ret) {
@@ -250,11 +260,11 @@ static int __devinit max17042_probe(struct i2c_client *client,
}

/* Initialize registers according to values from the platform data */
- if (chip->pdata->init_data)
- max17042_set_reg(client, chip->pdata->init_data,
- chip->pdata->num_init_data);
+ if (pdata && pdata->init_data)
+ max17042_set_reg(client, pdata->init_data,
+ pdata->num_init_data);

- if (!chip->pdata->enable_current_sense) {
+ if (!chip->enable_current_sense) {
max17042_write_reg(client, MAX17042_CGAIN, 0x0000);
max17042_write_reg(client, MAX17042_MiscCFG, 0x0003);
max17042_write_reg(client, MAX17042_LearnCFG, 0x0007);
--
1.7.8.3

2012-02-22 18:06:39

by Karol Lewandowski

[permalink] [raw]
Subject: [PATCH 3/3] max17042_battery: Make it possible to instantiate driver from DT

Allow both device tree (preferred) and platform data-based driver
instantiation.

Signed-off-by: Karol Lewandowski <[email protected]>
Signed-off-by: Kyungmin Park <[email protected]>
---
.../bindings/power_supply/max17042_battery.txt | 18 ++++++++
drivers/power/max17042_battery.c | 45 ++++++++++++++++++--
2 files changed, 59 insertions(+), 4 deletions(-)
create mode 100644 Documentation/devicetree/bindings/power_supply/max17042_battery.txt

diff --git a/Documentation/devicetree/bindings/power_supply/max17042_battery.txt b/Documentation/devicetree/bindings/power_supply/max17042_battery.txt
new file mode 100644
index 0000000..5bc9b68
--- /dev/null
+++ b/Documentation/devicetree/bindings/power_supply/max17042_battery.txt
@@ -0,0 +1,18 @@
+max17042_battery
+~~~~~~~~~~~~~~~~
+
+Required properties :
+ - compatible : "maxim,max17042"
+
+Optional properties :
+ - maxim,rsns-microohm : Resistance of rsns resistor in micro Ohms
+ (datasheet-recommended value is 10000).
+ Defining this property enables current-sense functionality.
+
+Example:
+
+ battery-charger@36 {
+ compatible = "maxim,max17042";
+ reg = <0x36>;
+ maxim,rsns-microohm = <10000>;
+ };
diff --git a/drivers/power/max17042_battery.c b/drivers/power/max17042_battery.c
index 49c1377..aec883b 100644
--- a/drivers/power/max17042_battery.c
+++ b/drivers/power/max17042_battery.c
@@ -29,6 +29,7 @@
#include <linux/mod_devicetable.h>
#include <linux/power_supply.h>
#include <linux/power/max17042_battery.h>
+#include <linux/of.h>

struct max17042_chip {
struct i2c_client *client;
@@ -211,6 +212,31 @@ static int max17042_get_property(struct power_supply *psy,
return 0;
}

+#ifdef CONFIG_OF
+static int max17042_dt_parse(struct max17042_chip *chip, struct device_node *np)
+{
+ u32 prop;
+
+ if (!np)
+ return -EINVAL;
+
+ /* require current sense resistor value to be specified for
+ current-sense functionality to be enabled at all */
+ if (of_property_read_u32(np, "maxim,rsns-microohm", &prop) == 0) {
+ chip->r_sns = prop;
+ chip->enable_current_sense = true;
+ } else
+ chip->enable_current_sense = false;
+
+ return 0;
+}
+#else
+static int max17042_dt_parse(struct max17042_chip *chip, struct device_node *np)
+{
+ return -EINVAL;
+}
+#endif
+
static int __devinit max17042_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -237,12 +263,14 @@ static int __devinit max17042_probe(struct i2c_client *client,
chip->battery.properties = max17042_battery_props;
chip->battery.num_properties = ARRAY_SIZE(max17042_battery_props);

- if (pdata) {
+ ret = max17042_dt_parse(chip, client->dev.of_node);
+ if (ret < 0) {
+ if (!pdata) {
+ dev_warn(&client->dev, "no driver data provided\n");
+ return -ENODEV;
+ }
chip->r_sns = pdata->r_sns;
chip->enable_current_sense = pdata->enable_current_sense;
- } else {
- dev_warn(&client->dev, "no driver data provided\n");
- return -ENODEV;
}

/* When current is not measured,
@@ -281,6 +309,14 @@ static int __devexit max17042_remove(struct i2c_client *client)
return 0;
}

+#ifdef CONFIG_OF
+static const struct of_device_id max17042_dt_match[] = {
+ { .compatible = "maxim,max17042" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, max17042_dt_match);
+#endif
+
static const struct i2c_device_id max17042_id[] = {
{ "max17042", 0 },
{ }
@@ -290,6 +326,7 @@ MODULE_DEVICE_TABLE(i2c, max17042_id);
static struct i2c_driver max17042_i2c_driver = {
.driver = {
.name = "max17042",
+ .of_match_table = of_match_ptr(max17042_dt_match),
},
.probe = max17042_probe,
.remove = __devexit_p(max17042_remove),
--
1.7.8.3