Skip to contentSkip to conversation

regulators: max8660: use i2c_id->driver_data rather than ->name

PATCH Applied Daniel Mack 2 participants 6 messages Aug 2, 2013

Subsystems (2)
Device tree bindings Voltage and current regulator framework
lore
Share & tools
lore

Daniel Mack

author
#0 lore raw

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(-)
drivers/regulator/max8660.c +10 -3
index d428ef9..ba3b8af 100644
@@ -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);
Signature · 1 line
1.8.3.1
author
#1 lore raw

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(-)
drivers/regulator/max8660.c +7 -6
index ba3b8af..a5caaa0 100644
@@ -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;
 		}
Signature · 1 line
1.8.3.1
maintainer Applied
#2 lore raw

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

Quoted 1 line · Daniel Mack

No functional change, just makes the code shorter.

Applied, thanks.

author
#3 lore raw

PATCH 3/3 regulators: max8660: add DT bindings

In reply to Daniel Mack

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
Documentation/devicetree/bindings/regulator/max8660.txt new file +47 -0
new file mode 100644
index 0000000..03dd839
@@ -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>;
+				};
+			};
+		};
+	};
drivers/regulator/max8660.c +78 -1

109 lines not on this page · raw message

include/linux/regulator/max8660.h +2 -1
index 9936763..58e1296 100644
@@ -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;
 };
 
 /**
Signature · 1 line
1.8.3.1
maintainer
#4 lore raw

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

Quoted 2 lines · Daniel Mack

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

This...

drivers/regulator/max8660.c enum { · line 319

Quoted 4 lines
drivers/regulator/max8660.c +4 -0
+#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.

include/linux/regulator/max8660.h enum { · line 45

Quoted 7 lines

struct max8660_subdev_data {

include/linux/regulator/max8660.h +2 -1
 	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.

maintainer Applied
#5 lore raw
In reply to Daniel Mack
Quoted 2 lines · Daniel Mack

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

Applied, thanks.