2013-08-02 08:19:35

by Daniel Mack

[permalink] [raw]
Subject: [PATCH 1/3] regulators: max8660: use i2c_id->driver_data rather than ->name

Introduce an enum and denote the device type via struct i2c_id's
driver_data field rather than comparing strings.

Signed-off-by: Daniel Mack <[email protected]>
---
drivers/regulator/max8660.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/max8660.c b/drivers/regulator/max8660.c
index d428ef9..ba3b8af 100644
--- a/drivers/regulator/max8660.c
+++ b/drivers/regulator/max8660.c
@@ -305,6 +305,11 @@ static const struct regulator_desc max8660_reg[] = {
},
};

+enum {
+ MAX8660 = 0,
+ MAX8661 = 1,
+};
+
static int max8660_probe(struct i2c_client *client,
const struct i2c_device_id *i2c_id)
{
@@ -313,6 +318,7 @@ static int max8660_probe(struct i2c_client *client,
struct regulator_config config = { };
struct max8660 *max8660;
int boot_on, i, id, ret = -EINVAL;
+ unsigned int type;

if (pdata->num_subdevs > MAX8660_V_END) {
dev_err(&client->dev, "Too many regulators found!\n");
@@ -327,6 +333,7 @@ static int max8660_probe(struct i2c_client *client,

max8660->client = client;
rdev = max8660->rdev;
+ type = i2c_id->driver_data;

if (pdata->en34_is_high) {
/* Simulate always on */
@@ -376,7 +383,7 @@ static int max8660_probe(struct i2c_client *client,
break;

case MAX8660_V7:
- if (!strcmp(i2c_id->name, "max8661")) {
+ if (type == MAX8661) {
dev_err(&client->dev, "Regulator not on this chip!\n");
goto err_out;
}
@@ -431,8 +438,8 @@ static int max8660_remove(struct i2c_client *client)
}

static const struct i2c_device_id max8660_id[] = {
- { "max8660", 0 },
- { "max8661", 0 },
+ { .name = "max8660", .driver_data = MAX8660 },
+ { .name = "max8661", .driver_data = MAX8661 },
{ }
};
MODULE_DEVICE_TABLE(i2c, max8660_id);
--
1.8.3.1


2013-08-02 08:19:33

by Daniel Mack

[permalink] [raw]
Subject: [PATCH 2/3] regulators: max8660: add a shorthand to &client->dev

No functional change, just makes the code shorter.

Signed-off-by: Daniel Mack <[email protected]>
---
drivers/regulator/max8660.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/regulator/max8660.c b/drivers/regulator/max8660.c
index ba3b8af..a5caaa0 100644
--- a/drivers/regulator/max8660.c
+++ b/drivers/regulator/max8660.c
@@ -314,6 +314,7 @@ static int max8660_probe(struct i2c_client *client,
const struct i2c_device_id *i2c_id)
{
struct regulator_dev **rdev;
+ struct device *dev = &client->dev;
struct max8660_platform_data *pdata = client->dev.platform_data;
struct regulator_config config = { };
struct max8660 *max8660;
@@ -321,11 +322,11 @@ static int max8660_probe(struct i2c_client *client,
unsigned int type;

if (pdata->num_subdevs > MAX8660_V_END) {
- dev_err(&client->dev, "Too many regulators found!\n");
+ dev_err(dev, "Too many regulators found!\n");
return -EINVAL;
}

- max8660 = devm_kzalloc(&client->dev, sizeof(struct max8660) +
+ max8660 = devm_kzalloc(dev, sizeof(struct max8660) +
sizeof(struct regulator_dev *) * MAX8660_V_END,
GFP_KERNEL);
if (!max8660)
@@ -384,7 +385,7 @@ static int max8660_probe(struct i2c_client *client,

case MAX8660_V7:
if (type == MAX8661) {
- dev_err(&client->dev, "Regulator not on this chip!\n");
+ dev_err(dev, "Regulator not on this chip!\n");
goto err_out;
}

@@ -393,7 +394,7 @@ static int max8660_probe(struct i2c_client *client,
break;

default:
- dev_err(&client->dev, "invalid regulator %s\n",
+ dev_err(dev, "invalid regulator %s\n",
pdata->subdevs[i].name);
goto err_out;
}
@@ -404,14 +405,14 @@ static int max8660_probe(struct i2c_client *client,

id = pdata->subdevs[i].id;

- config.dev = &client->dev;
+ config.dev = dev;
config.init_data = pdata->subdevs[i].platform_data;
config.driver_data = max8660;

rdev[i] = regulator_register(&max8660_reg[id], &config);
if (IS_ERR(rdev[i])) {
ret = PTR_ERR(rdev[i]);
- dev_err(&client->dev, "failed to register %s\n",
+ dev_err(dev, "failed to register %s\n",
max8660_reg[id].name);
goto err_unregister;
}
--
1.8.3.1

2013-08-02 08:20:10

by Daniel Mack

[permalink] [raw]
Subject: [PATCH 3/3] regulators: max8660: add DT bindings

This patch adds devicetree bindings for max8660, along with some
documentation.

Signed-off-by: Daniel Mack <[email protected]>
---
.../devicetree/bindings/regulator/max8660.txt | 47 +++++++++++++
drivers/regulator/max8660.c | 79 +++++++++++++++++++++-
include/linux/regulator/max8660.h | 3 +-
3 files changed, 127 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/regulator/max8660.txt

diff --git a/Documentation/devicetree/bindings/regulator/max8660.txt b/Documentation/devicetree/bindings/regulator/max8660.txt
new file mode 100644
index 0000000..03dd839
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/max8660.txt
@@ -0,0 +1,47 @@
+Maxim MAX8660 voltage regulator
+
+Required properties:
+- compatible: must be equal to "maxim,max8660"
+- reg: I2C slave address, usually 0x34
+- any required generic properties defined in regulator.txt
+
+Example:
+
+ i2c_master {
+ max8660@34 {
+ compatible = "maxim,max8660";
+ reg = <0x34>;
+
+ regulators {
+ regulator@0 {
+ regulator-compatible= "V3(DCDC)";
+ regulator-min-microvolt = <725000>;
+ regulator-max-microvolt = <1800000>;
+ };
+
+ regulator@1 {
+ regulator-compatible= "V4(DCDC)";
+ regulator-min-microvolt = <725000>;
+ regulator-max-microvolt = <1800000>;
+ };
+
+ regulator@2 {
+ regulator-compatible= "V5(LDO)";
+ regulator-min-microvolt = <1700000>;
+ regulator-max-microvolt = <2000000>;
+ };
+
+ regulator@3 {
+ regulator-compatible= "V6(LDO)";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <3300000>;
+ };
+
+ regulator@4 {
+ regulator-compatible= "V7(LDO)";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <3300000>;
+ };
+ };
+ };
+ };
diff --git a/drivers/regulator/max8660.c b/drivers/regulator/max8660.c
index a5caaa0..597d311 100644
--- a/drivers/regulator/max8660.c
+++ b/drivers/regulator/max8660.c
@@ -44,6 +44,9 @@
#include <linux/regulator/driver.h>
#include <linux/slab.h>
#include <linux/regulator/max8660.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/regulator/of_regulator.h>

#define MAX8660_DCDC_MIN_UV 725000
#define MAX8660_DCDC_MAX_UV 1800000
@@ -310,6 +313,62 @@ enum {
MAX8661 = 1,
};

+#ifdef CONFIG_OF
+static const struct of_device_id max8660_dt_ids[] = {
+ { .compatible = "maxim,max8660", .data = (void *) MAX8660 },
+ { .compatible = "maxim,max8661", .data = (void *) MAX8661 },
+ { }
+};
+MODULE_DEVICE_TABLE(of, max8660_dt_ids);
+
+static int max8660_pdata_from_dt(struct device *dev,
+ struct max8660_platform_data *pdata)
+{
+ int matched, i;
+ struct device_node *np;
+ struct max8660_subdev_data *sub;
+ struct of_regulator_match rmatch[ARRAY_SIZE(max8660_reg)];
+
+ np = of_find_node_by_name(dev->of_node, "regulators");
+ if (!np) {
+ dev_err(dev, "missing 'regulators' subnode in DT\n");
+ return -EINVAL;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(rmatch); i++)
+ rmatch[i].name = max8660_reg[i].name;
+
+ matched = of_regulator_match(dev, np, rmatch, ARRAY_SIZE(rmatch));
+ if (matched <= 0)
+ return matched;
+
+ pdata->subdevs = devm_kzalloc(dev, sizeof(struct max8660_subdev_data) *
+ matched, GFP_KERNEL);
+ if (!pdata->subdevs)
+ return -ENOMEM;
+
+ pdata->num_subdevs = matched;
+
+ sub = pdata->subdevs;
+
+ for (i = 0; i < matched; i++) {
+ sub->id = i;
+ sub->name = rmatch[i].name;
+ sub->platform_data = rmatch[i].init_data;
+ sub->of_node = rmatch[i].of_node;
+ sub++;
+ }
+
+ return 0;
+}
+#else
+static inline int max8660_pdata_from_dt(struct device *dev,
+ struct max8660_platform_data **pdata)
+{
+ return 0;
+}
+#endif
+
static int max8660_probe(struct i2c_client *client,
const struct i2c_device_id *i2c_id)
{
@@ -321,6 +380,24 @@ static int max8660_probe(struct i2c_client *client,
int boot_on, i, id, ret = -EINVAL;
unsigned int type;

+ if (dev->of_node && !pdata) {
+ const struct of_device_id *id;
+ struct max8660_platform_data pdata_of;
+
+ id = of_match_device(max8660_dt_ids, dev);
+ if (!id)
+ return -ENODEV;
+
+ ret = max8660_pdata_from_dt(dev, &pdata_of);
+ if (ret < 0)
+ return ret;
+
+ pdata = &pdata_of;
+ type = (unsigned int) id->data;
+ } else {
+ type = i2c_id->driver_data;
+ }
+
if (pdata->num_subdevs > MAX8660_V_END) {
dev_err(dev, "Too many regulators found!\n");
return -EINVAL;
@@ -334,7 +411,6 @@ static int max8660_probe(struct i2c_client *client,

max8660->client = client;
rdev = max8660->rdev;
- type = i2c_id->driver_data;

if (pdata->en34_is_high) {
/* Simulate always on */
@@ -407,6 +483,7 @@ static int max8660_probe(struct i2c_client *client,

config.dev = dev;
config.init_data = pdata->subdevs[i].platform_data;
+ config.of_node = pdata->subdevs[i].of_node;
config.driver_data = max8660;

rdev[i] = regulator_register(&max8660_reg[id], &config);
diff --git a/include/linux/regulator/max8660.h b/include/linux/regulator/max8660.h
index 9936763..58e1296 100644
--- a/include/linux/regulator/max8660.h
+++ b/include/linux/regulator/max8660.h
@@ -39,8 +39,9 @@ enum {
*/
struct max8660_subdev_data {
int id;
- char *name;
+ const char *name;
struct regulator_init_data *platform_data;
+ struct device_node *of_node;
};

/**
--
1.8.3.1

2013-08-02 10:16:28

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 2/3] regulators: max8660: add a shorthand to &client->dev

On Fri, Aug 02, 2013 at 10:19:16AM +0200, Daniel Mack wrote:
> No functional change, just makes the code shorter.

Applied, thanks.


Attachments:
(No filename) (132.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2013-08-02 10:16:42

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 1/3] regulators: max8660: use i2c_id->driver_data rather than ->name

On Fri, Aug 02, 2013 at 10:19:15AM +0200, Daniel Mack wrote:
> Introduce an enum and denote the device type via struct i2c_id's
> driver_data field rather than comparing strings.

Applied, thanks.


Attachments:
(No filename) (197.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2013-08-02 10:18:54

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 3/3] regulators: max8660: add DT bindings

On Fri, Aug 02, 2013 at 10:19:17AM +0200, Daniel Mack wrote:

> +Required properties:
> +- compatible: must be equal to "maxim,max8660"

This...

> +#ifdef CONFIG_OF
> +static const struct of_device_id max8660_dt_ids[] = {
> + { .compatible = "maxim,max8660", .data = (void *) MAX8660 },
> + { .compatible = "maxim,max8661", .data = (void *) MAX8661 },

...and this don't match up.

> struct max8660_subdev_data {
> int id;
> - char *name;
> + const char *name;
> struct regulator_init_data *platform_data;
> + struct device_node *of_node;
> };

Can't you just pass the of_node around while you need it? Not that it
makes a big difference.

Otherwise looks good.


Attachments:
(No filename) (679.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments