* [PATCH v3 0/6] mfd: axp20x: add AXP221 PMIC support
@ 2014-05-27 9:38 ` Boris BREZILLON
0 siblings, 0 replies; 30+ messages in thread
From: Boris BREZILLON @ 2014-05-27 9:38 UTC (permalink / raw)
To: linux-arm-kernel
Hello,
This patch series adds basic support for the X-Powers' AXP221 PMIC.
At the moment the MFD device only exposes the AXP221 regulators but other
subdevices might be added later.
I'm still waiting for inputs regarding the ipsout regulator representation:
* The ipsout regulator is currently represented as a fixed regulator
providing a 5v output, and this is not exactly the case.
AFAIU (here's is the datasheet traduction if you want to check [1]), the
ipsout output is a multiplexer that choose among vbus (5V), acin (12V
which is then converted to 5V) and the battery power supply (3,5 -> 4,2 V).
This means the output voltage of ipsout vary between 3,5V and 5V.
How can we express this kind of muxer in the regulator framework (is there
already something available ?) ?
Note that the power supply selection is automatic, though we could force
one power supply, but then we loose the ability to unplug one power supply
without impacting the system.
Best Regards,
Boris
[1] http://linux-sunxi.org/AXP221#Regulators
Changes since v2:
- add helper functions for regulator set registration
- fix a copy/paste error in the axp20x mfd driver
Changes since v1:
- merge code with the axp20x driver
Boris BREZILLON (6):
mfd: axp20x: add AXP221 PMIC support
regulator: axp20x: prepare support for multiple AXP chip families
regulator: axp20x: add support for AXP221 regulators
regulator: axp20x: reset probe data before each probe
regulator: add support for regulator set registration
regulator: axp20x: make use of devm_regulator_set_register
drivers/mfd/axp20x.c | 58 +++++++++-
drivers/regulator/axp20x-regulator.c | 202 ++++++++++++++++++++++++-----------
drivers/regulator/core.c | 106 ++++++++++++++++++
drivers/regulator/devres.c | 68 ++++++++++++
include/linux/mfd/axp20x.h | 56 ++++++++++
include/linux/regulator/driver.h | 51 +++++++++
6 files changed, 478 insertions(+), 63 deletions(-)
--
1.8.3.2
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v3 1/6] mfd: axp20x: add AXP221 PMIC support
2014-05-27 9:38 ` Boris BREZILLON
@ 2014-05-27 9:38 ` Boris BREZILLON
-1 siblings, 0 replies; 30+ messages in thread
From: Boris BREZILLON @ 2014-05-27 9:38 UTC (permalink / raw)
To: Samuel Ortiz, Lee Jones, Liam Girdwood, Mark Brown
Cc: Maxime Ripard, Carlo Caione, Shuge, kevin, linux-kernel,
linux-arm-kernel, dev, Boris BREZILLON
Add support for the AXP221 PMIC device to the existing AXP20x driver.
The AXP221 defines a new set of registers, power supplies and regulators,
but most of the API is similar to the AXP20x ones.
The AXP20x irq chip definition is reused, though some interrupts are not
available in the AXP221.
Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
---
drivers/mfd/axp20x.c | 58 +++++++++++++++++++++++++++++++++++++++++++---
include/linux/mfd/axp20x.h | 56 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 111 insertions(+), 3 deletions(-)
diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index dee6539..119a7ed 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -33,6 +33,11 @@ static const struct regmap_range axp20x_writeable_ranges[] = {
regmap_reg_range(AXP20X_DCDC_MODE, AXP20X_FG_RES),
};
+static const struct regmap_range axp22x_writeable_ranges[] = {
+ regmap_reg_range(AXP20X_DATACACHE(0), AXP20X_IRQ5_STATE),
+ regmap_reg_range(AXP20X_DCDC_MODE, AXP22X_BATLOW_THRES1),
+};
+
static const struct regmap_range axp20x_volatile_ranges[] = {
regmap_reg_range(AXP20X_IRQ1_EN, AXP20X_IRQ5_STATE),
};
@@ -42,6 +47,11 @@ static const struct regmap_access_table axp20x_writeable_table = {
.n_yes_ranges = ARRAY_SIZE(axp20x_writeable_ranges),
};
+static const struct regmap_access_table axp22x_writeable_table = {
+ .yes_ranges = axp22x_writeable_ranges,
+ .n_yes_ranges = ARRAY_SIZE(axp22x_writeable_ranges),
+};
+
static const struct regmap_access_table axp20x_volatile_table = {
.yes_ranges = axp20x_volatile_ranges,
.n_yes_ranges = ARRAY_SIZE(axp20x_volatile_ranges),
@@ -70,6 +80,15 @@ static const struct regmap_config axp20x_regmap_config = {
.cache_type = REGCACHE_RBTREE,
};
+static const struct regmap_config axp22x_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .wr_table = &axp22x_writeable_table,
+ .volatile_table = &axp20x_volatile_table,
+ .max_register = AXP22X_BATLOW_THRES1,
+ .cache_type = REGCACHE_RBTREE,
+};
+
#define AXP20X_IRQ(_irq, _off, _mask) \
[AXP20X_IRQ_##_irq] = { .reg_offset = (_off), .mask = BIT(_mask) }
@@ -116,6 +135,7 @@ static const struct regmap_irq axp20x_regmap_irqs[] = {
static const struct of_device_id axp20x_of_match[] = {
{ .compatible = "x-powers,axp202", .data = (void *) AXP202_ID },
{ .compatible = "x-powers,axp209", .data = (void *) AXP209_ID },
+ { .compatible = "x-powers,axp221", .data = (void *) AXP221_ID },
{ },
};
MODULE_DEVICE_TABLE(of, axp20x_of_match);
@@ -149,6 +169,21 @@ static const char * const axp20x_supplies[] = {
"ldo5in",
};
+static const char *axp22x_supplies[] = {
+ "vbus",
+ "acin",
+ "vin1",
+ "vin2",
+ "vin3",
+ "vin4",
+ "vin5",
+ "aldoin",
+ "dldoin",
+ "eldoin",
+ "ldoioin",
+ "rtcldoin",
+};
+
static struct mfd_cell axp20x_cells[] = {
{
.name = "axp20x-pek",
@@ -161,6 +196,14 @@ static struct mfd_cell axp20x_cells[] = {
},
};
+static struct mfd_cell axp22x_cells[] = {
+ {
+ .name = "axp20x-regulator",
+ .parent_supplies = axp22x_supplies,
+ .num_parent_supplies = ARRAY_SIZE(axp22x_supplies),
+ },
+};
+
static struct axp20x_dev *axp20x_pm_power_off;
static void axp20x_power_off(void)
{
@@ -190,7 +233,12 @@ static int axp20x_i2c_probe(struct i2c_client *i2c,
axp20x->dev = &i2c->dev;
dev_set_drvdata(axp20x->dev, axp20x);
- axp20x->regmap = devm_regmap_init_i2c(i2c, &axp20x_regmap_config);
+ if (axp20x->variant == AXP221_ID)
+ axp20x->regmap = devm_regmap_init_i2c(i2c,
+ &axp22x_regmap_config);
+ else
+ axp20x->regmap = devm_regmap_init_i2c(i2c,
+ &axp20x_regmap_config);
if (IS_ERR(axp20x->regmap)) {
ret = PTR_ERR(axp20x->regmap);
dev_err(&i2c->dev, "regmap init failed: %d\n", ret);
@@ -206,8 +254,12 @@ static int axp20x_i2c_probe(struct i2c_client *i2c,
return ret;
}
- ret = mfd_add_devices(axp20x->dev, -1, axp20x_cells,
- ARRAY_SIZE(axp20x_cells), NULL, 0, NULL);
+ if (axp20x->variant == AXP221_ID)
+ ret = mfd_add_devices(axp20x->dev, -1, axp22x_cells,
+ ARRAY_SIZE(axp22x_cells), NULL, 0, NULL);
+ else
+ ret = mfd_add_devices(axp20x->dev, -1, axp20x_cells,
+ ARRAY_SIZE(axp20x_cells), NULL, 0, NULL);
if (ret) {
dev_err(&i2c->dev, "failed to add MFD devices: %d\n", ret);
diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
index d0e31a2..8a8ce02 100644
--- a/include/linux/mfd/axp20x.h
+++ b/include/linux/mfd/axp20x.h
@@ -14,6 +14,7 @@
enum {
AXP202_ID = 0,
AXP209_ID,
+ AXP221_ID,
};
#define AXP20X_DATACACHE(m) (0x04 + (m))
@@ -43,6 +44,28 @@ enum {
#define AXP20X_V_LTF_DISCHRG 0x3c
#define AXP20X_V_HTF_DISCHRG 0x3d
+#define AXP22X_PWR_OUT_CTRL1 0x10
+#define AXP22X_PWR_OUT_CTRL2 0x12
+#define AXP22X_PWR_OUT_CTRL3 0x13
+#define AXP22X_DLDO1_V_OUT 0x15
+#define AXP22X_DLDO2_V_OUT 0x16
+#define AXP22X_DLDO3_V_OUT 0x17
+#define AXP22X_DLDO4_V_OUT 0x18
+#define AXP22X_ELDO1_V_OUT 0x19
+#define AXP22X_ELDO2_V_OUT 0x1a
+#define AXP22X_ELDO3_V_OUT 0x1b
+#define AXP22X_DC5LDO_V_OUT 0x1c
+#define AXP22X_DCDC1_V_OUT 0x21
+#define AXP22X_DCDC2_V_OUT 0x22
+#define AXP22X_DCDC3_V_OUT 0x23
+#define AXP22X_DCDC4_V_OUT 0x24
+#define AXP22X_DCDC5_V_OUT 0x25
+#define AXP22X_DCDC23_V_RAMP_CTRL 0x27
+#define AXP22X_ALDO1_V_OUT 0x28
+#define AXP22X_ALDO2_V_OUT 0x29
+#define AXP22X_ALDO3_V_OUT 0x2a
+#define AXP22X_CHRG_CTRL3 0x35
+
/* Interrupt */
#define AXP20X_IRQ1_EN 0x40
#define AXP20X_IRQ2_EN 0x41
@@ -96,6 +119,9 @@ enum {
#define AXP20X_VBUS_MON 0x8b
#define AXP20X_OVER_TMP 0x8f
+#define AXP22X_PWREN_CTRL1 0x8c
+#define AXP22X_PWREN_CTRL2 0x8d
+
/* GPIO */
#define AXP20X_GPIO0_CTRL 0x90
#define AXP20X_LDO5_V_OUT 0x91
@@ -104,6 +130,11 @@ enum {
#define AXP20X_GPIO20_SS 0x94
#define AXP20X_GPIO3_CTRL 0x95
+#define AXP22X_LDO_IO0_V_OUT 0x91
+#define AXP22X_LDO_IO1_V_OUT 0x93
+#define AXP22X_GPIO_STATE 0x94
+#define AXP22X_GPIO_PULL_DOWN 0x95
+
/* Battery */
#define AXP20X_CHRG_CC_31_24 0xb0
#define AXP20X_CHRG_CC_23_16 0xb1
@@ -116,6 +147,8 @@ enum {
#define AXP20X_CC_CTRL 0xb8
#define AXP20X_FG_RES 0xb9
+#define AXP22X_BATLOW_THRES1 0xe6
+
/* Regulators IDs */
enum {
AXP20X_LDO1 = 0,
@@ -128,6 +161,29 @@ enum {
AXP20X_REG_ID_MAX,
};
+enum {
+ AXP22X_DCDC1 = 0,
+ AXP22X_DCDC2,
+ AXP22X_DCDC3,
+ AXP22X_DCDC4,
+ AXP22X_DCDC5,
+ AXP22X_DC5LDO,
+ AXP22X_ALDO1,
+ AXP22X_ALDO2,
+ AXP22X_ALDO3,
+ AXP22X_ELDO1,
+ AXP22X_ELDO2,
+ AXP22X_ELDO3,
+ AXP22X_DLDO1,
+ AXP22X_DLDO2,
+ AXP22X_DLDO3,
+ AXP22X_DLDO4,
+ AXP22X_RTC_LDO,
+ AXP22X_LDO_IO0,
+ AXP22X_LDO_IO1,
+ AXP22X_REG_ID_MAX,
+};
+
/* IRQs */
enum {
AXP20X_IRQ_ACIN_OVER_V = 1,
--
1.8.3.2
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v3 1/6] mfd: axp20x: add AXP221 PMIC support
@ 2014-05-27 9:38 ` Boris BREZILLON
0 siblings, 0 replies; 30+ messages in thread
From: Boris BREZILLON @ 2014-05-27 9:38 UTC (permalink / raw)
To: linux-arm-kernel
Add support for the AXP221 PMIC device to the existing AXP20x driver.
The AXP221 defines a new set of registers, power supplies and regulators,
but most of the API is similar to the AXP20x ones.
The AXP20x irq chip definition is reused, though some interrupts are not
available in the AXP221.
Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
---
drivers/mfd/axp20x.c | 58 +++++++++++++++++++++++++++++++++++++++++++---
include/linux/mfd/axp20x.h | 56 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 111 insertions(+), 3 deletions(-)
diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index dee6539..119a7ed 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -33,6 +33,11 @@ static const struct regmap_range axp20x_writeable_ranges[] = {
regmap_reg_range(AXP20X_DCDC_MODE, AXP20X_FG_RES),
};
+static const struct regmap_range axp22x_writeable_ranges[] = {
+ regmap_reg_range(AXP20X_DATACACHE(0), AXP20X_IRQ5_STATE),
+ regmap_reg_range(AXP20X_DCDC_MODE, AXP22X_BATLOW_THRES1),
+};
+
static const struct regmap_range axp20x_volatile_ranges[] = {
regmap_reg_range(AXP20X_IRQ1_EN, AXP20X_IRQ5_STATE),
};
@@ -42,6 +47,11 @@ static const struct regmap_access_table axp20x_writeable_table = {
.n_yes_ranges = ARRAY_SIZE(axp20x_writeable_ranges),
};
+static const struct regmap_access_table axp22x_writeable_table = {
+ .yes_ranges = axp22x_writeable_ranges,
+ .n_yes_ranges = ARRAY_SIZE(axp22x_writeable_ranges),
+};
+
static const struct regmap_access_table axp20x_volatile_table = {
.yes_ranges = axp20x_volatile_ranges,
.n_yes_ranges = ARRAY_SIZE(axp20x_volatile_ranges),
@@ -70,6 +80,15 @@ static const struct regmap_config axp20x_regmap_config = {
.cache_type = REGCACHE_RBTREE,
};
+static const struct regmap_config axp22x_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .wr_table = &axp22x_writeable_table,
+ .volatile_table = &axp20x_volatile_table,
+ .max_register = AXP22X_BATLOW_THRES1,
+ .cache_type = REGCACHE_RBTREE,
+};
+
#define AXP20X_IRQ(_irq, _off, _mask) \
[AXP20X_IRQ_##_irq] = { .reg_offset = (_off), .mask = BIT(_mask) }
@@ -116,6 +135,7 @@ static const struct regmap_irq axp20x_regmap_irqs[] = {
static const struct of_device_id axp20x_of_match[] = {
{ .compatible = "x-powers,axp202", .data = (void *) AXP202_ID },
{ .compatible = "x-powers,axp209", .data = (void *) AXP209_ID },
+ { .compatible = "x-powers,axp221", .data = (void *) AXP221_ID },
{ },
};
MODULE_DEVICE_TABLE(of, axp20x_of_match);
@@ -149,6 +169,21 @@ static const char * const axp20x_supplies[] = {
"ldo5in",
};
+static const char *axp22x_supplies[] = {
+ "vbus",
+ "acin",
+ "vin1",
+ "vin2",
+ "vin3",
+ "vin4",
+ "vin5",
+ "aldoin",
+ "dldoin",
+ "eldoin",
+ "ldoioin",
+ "rtcldoin",
+};
+
static struct mfd_cell axp20x_cells[] = {
{
.name = "axp20x-pek",
@@ -161,6 +196,14 @@ static struct mfd_cell axp20x_cells[] = {
},
};
+static struct mfd_cell axp22x_cells[] = {
+ {
+ .name = "axp20x-regulator",
+ .parent_supplies = axp22x_supplies,
+ .num_parent_supplies = ARRAY_SIZE(axp22x_supplies),
+ },
+};
+
static struct axp20x_dev *axp20x_pm_power_off;
static void axp20x_power_off(void)
{
@@ -190,7 +233,12 @@ static int axp20x_i2c_probe(struct i2c_client *i2c,
axp20x->dev = &i2c->dev;
dev_set_drvdata(axp20x->dev, axp20x);
- axp20x->regmap = devm_regmap_init_i2c(i2c, &axp20x_regmap_config);
+ if (axp20x->variant == AXP221_ID)
+ axp20x->regmap = devm_regmap_init_i2c(i2c,
+ &axp22x_regmap_config);
+ else
+ axp20x->regmap = devm_regmap_init_i2c(i2c,
+ &axp20x_regmap_config);
if (IS_ERR(axp20x->regmap)) {
ret = PTR_ERR(axp20x->regmap);
dev_err(&i2c->dev, "regmap init failed: %d\n", ret);
@@ -206,8 +254,12 @@ static int axp20x_i2c_probe(struct i2c_client *i2c,
return ret;
}
- ret = mfd_add_devices(axp20x->dev, -1, axp20x_cells,
- ARRAY_SIZE(axp20x_cells), NULL, 0, NULL);
+ if (axp20x->variant == AXP221_ID)
+ ret = mfd_add_devices(axp20x->dev, -1, axp22x_cells,
+ ARRAY_SIZE(axp22x_cells), NULL, 0, NULL);
+ else
+ ret = mfd_add_devices(axp20x->dev, -1, axp20x_cells,
+ ARRAY_SIZE(axp20x_cells), NULL, 0, NULL);
if (ret) {
dev_err(&i2c->dev, "failed to add MFD devices: %d\n", ret);
diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
index d0e31a2..8a8ce02 100644
--- a/include/linux/mfd/axp20x.h
+++ b/include/linux/mfd/axp20x.h
@@ -14,6 +14,7 @@
enum {
AXP202_ID = 0,
AXP209_ID,
+ AXP221_ID,
};
#define AXP20X_DATACACHE(m) (0x04 + (m))
@@ -43,6 +44,28 @@ enum {
#define AXP20X_V_LTF_DISCHRG 0x3c
#define AXP20X_V_HTF_DISCHRG 0x3d
+#define AXP22X_PWR_OUT_CTRL1 0x10
+#define AXP22X_PWR_OUT_CTRL2 0x12
+#define AXP22X_PWR_OUT_CTRL3 0x13
+#define AXP22X_DLDO1_V_OUT 0x15
+#define AXP22X_DLDO2_V_OUT 0x16
+#define AXP22X_DLDO3_V_OUT 0x17
+#define AXP22X_DLDO4_V_OUT 0x18
+#define AXP22X_ELDO1_V_OUT 0x19
+#define AXP22X_ELDO2_V_OUT 0x1a
+#define AXP22X_ELDO3_V_OUT 0x1b
+#define AXP22X_DC5LDO_V_OUT 0x1c
+#define AXP22X_DCDC1_V_OUT 0x21
+#define AXP22X_DCDC2_V_OUT 0x22
+#define AXP22X_DCDC3_V_OUT 0x23
+#define AXP22X_DCDC4_V_OUT 0x24
+#define AXP22X_DCDC5_V_OUT 0x25
+#define AXP22X_DCDC23_V_RAMP_CTRL 0x27
+#define AXP22X_ALDO1_V_OUT 0x28
+#define AXP22X_ALDO2_V_OUT 0x29
+#define AXP22X_ALDO3_V_OUT 0x2a
+#define AXP22X_CHRG_CTRL3 0x35
+
/* Interrupt */
#define AXP20X_IRQ1_EN 0x40
#define AXP20X_IRQ2_EN 0x41
@@ -96,6 +119,9 @@ enum {
#define AXP20X_VBUS_MON 0x8b
#define AXP20X_OVER_TMP 0x8f
+#define AXP22X_PWREN_CTRL1 0x8c
+#define AXP22X_PWREN_CTRL2 0x8d
+
/* GPIO */
#define AXP20X_GPIO0_CTRL 0x90
#define AXP20X_LDO5_V_OUT 0x91
@@ -104,6 +130,11 @@ enum {
#define AXP20X_GPIO20_SS 0x94
#define AXP20X_GPIO3_CTRL 0x95
+#define AXP22X_LDO_IO0_V_OUT 0x91
+#define AXP22X_LDO_IO1_V_OUT 0x93
+#define AXP22X_GPIO_STATE 0x94
+#define AXP22X_GPIO_PULL_DOWN 0x95
+
/* Battery */
#define AXP20X_CHRG_CC_31_24 0xb0
#define AXP20X_CHRG_CC_23_16 0xb1
@@ -116,6 +147,8 @@ enum {
#define AXP20X_CC_CTRL 0xb8
#define AXP20X_FG_RES 0xb9
+#define AXP22X_BATLOW_THRES1 0xe6
+
/* Regulators IDs */
enum {
AXP20X_LDO1 = 0,
@@ -128,6 +161,29 @@ enum {
AXP20X_REG_ID_MAX,
};
+enum {
+ AXP22X_DCDC1 = 0,
+ AXP22X_DCDC2,
+ AXP22X_DCDC3,
+ AXP22X_DCDC4,
+ AXP22X_DCDC5,
+ AXP22X_DC5LDO,
+ AXP22X_ALDO1,
+ AXP22X_ALDO2,
+ AXP22X_ALDO3,
+ AXP22X_ELDO1,
+ AXP22X_ELDO2,
+ AXP22X_ELDO3,
+ AXP22X_DLDO1,
+ AXP22X_DLDO2,
+ AXP22X_DLDO3,
+ AXP22X_DLDO4,
+ AXP22X_RTC_LDO,
+ AXP22X_LDO_IO0,
+ AXP22X_LDO_IO1,
+ AXP22X_REG_ID_MAX,
+};
+
/* IRQs */
enum {
AXP20X_IRQ_ACIN_OVER_V = 1,
--
1.8.3.2
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v3 1/6] mfd: axp20x: add AXP221 PMIC support
2014-05-27 9:38 ` Boris BREZILLON
@ 2014-05-27 10:05 ` Lee Jones
-1 siblings, 0 replies; 30+ messages in thread
From: Lee Jones @ 2014-05-27 10:05 UTC (permalink / raw)
To: Boris BREZILLON
Cc: Samuel Ortiz, Liam Girdwood, Mark Brown, Maxime Ripard,
Carlo Caione, Shuge, kevin, linux-kernel, linux-arm-kernel, dev
> Add support for the AXP221 PMIC device to the existing AXP20x driver.
>
> The AXP221 defines a new set of registers, power supplies and regulators,
> but most of the API is similar to the AXP20x ones.
> The AXP20x irq chip definition is reused, though some interrupts are not
> available in the AXP221.
>
> Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> ---
> drivers/mfd/axp20x.c | 58 +++++++++++++++++++++++++++++++++++++++++++---
Quite a difference from 237 lines, eh?
> include/linux/mfd/axp20x.h | 56 ++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 111 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index dee6539..119a7ed 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
[...]
> @@ -190,7 +233,12 @@ static int axp20x_i2c_probe(struct i2c_client *i2c,
> axp20x->dev = &i2c->dev;
> dev_set_drvdata(axp20x->dev, axp20x);
>
> - axp20x->regmap = devm_regmap_init_i2c(i2c, &axp20x_regmap_config);
> + if (axp20x->variant == AXP221_ID)
> + axp20x->regmap = devm_regmap_init_i2c(i2c,
> + &axp22x_regmap_config);
> + else
> + axp20x->regmap = devm_regmap_init_i2c(i2c,
> + &axp20x_regmap_config);
Better to put these into a variable and have one instance of
devm_regmap_init_i2c() where you pass said variable as the second
parameter.
> if (IS_ERR(axp20x->regmap)) {
> ret = PTR_ERR(axp20x->regmap);
> dev_err(&i2c->dev, "regmap init failed: %d\n", ret);
> @@ -206,8 +254,12 @@ static int axp20x_i2c_probe(struct i2c_client *i2c,
> return ret;
> }
>
> - ret = mfd_add_devices(axp20x->dev, -1, axp20x_cells,
> - ARRAY_SIZE(axp20x_cells), NULL, 0, NULL);
> + if (axp20x->variant == AXP221_ID)
> + ret = mfd_add_devices(axp20x->dev, -1, axp22x_cells,
> + ARRAY_SIZE(axp22x_cells), NULL, 0, NULL);
> + else
> + ret = mfd_add_devices(axp20x->dev, -1, axp20x_cells,
> + ARRAY_SIZE(axp20x_cells), NULL, 0, NULL);
Same here. New variables to carry the cell structure and its size.
As the if statement is the same at the one above, you can move this
all into the first one.
[...]
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCH v3 1/6] mfd: axp20x: add AXP221 PMIC support
@ 2014-05-27 10:05 ` Lee Jones
0 siblings, 0 replies; 30+ messages in thread
From: Lee Jones @ 2014-05-27 10:05 UTC (permalink / raw)
To: linux-arm-kernel
> Add support for the AXP221 PMIC device to the existing AXP20x driver.
>
> The AXP221 defines a new set of registers, power supplies and regulators,
> but most of the API is similar to the AXP20x ones.
> The AXP20x irq chip definition is reused, though some interrupts are not
> available in the AXP221.
>
> Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> ---
> drivers/mfd/axp20x.c | 58 +++++++++++++++++++++++++++++++++++++++++++---
Quite a difference from 237 lines, eh?
> include/linux/mfd/axp20x.h | 56 ++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 111 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index dee6539..119a7ed 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
[...]
> @@ -190,7 +233,12 @@ static int axp20x_i2c_probe(struct i2c_client *i2c,
> axp20x->dev = &i2c->dev;
> dev_set_drvdata(axp20x->dev, axp20x);
>
> - axp20x->regmap = devm_regmap_init_i2c(i2c, &axp20x_regmap_config);
> + if (axp20x->variant == AXP221_ID)
> + axp20x->regmap = devm_regmap_init_i2c(i2c,
> + &axp22x_regmap_config);
> + else
> + axp20x->regmap = devm_regmap_init_i2c(i2c,
> + &axp20x_regmap_config);
Better to put these into a variable and have one instance of
devm_regmap_init_i2c() where you pass said variable as the second
parameter.
> if (IS_ERR(axp20x->regmap)) {
> ret = PTR_ERR(axp20x->regmap);
> dev_err(&i2c->dev, "regmap init failed: %d\n", ret);
> @@ -206,8 +254,12 @@ static int axp20x_i2c_probe(struct i2c_client *i2c,
> return ret;
> }
>
> - ret = mfd_add_devices(axp20x->dev, -1, axp20x_cells,
> - ARRAY_SIZE(axp20x_cells), NULL, 0, NULL);
> + if (axp20x->variant == AXP221_ID)
> + ret = mfd_add_devices(axp20x->dev, -1, axp22x_cells,
> + ARRAY_SIZE(axp22x_cells), NULL, 0, NULL);
> + else
> + ret = mfd_add_devices(axp20x->dev, -1, axp20x_cells,
> + ARRAY_SIZE(axp20x_cells), NULL, 0, NULL);
Same here. New variables to carry the cell structure and its size.
As the if statement is the same at the one above, you can move this
all into the first one.
[...]
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v3 1/6] mfd: axp20x: add AXP221 PMIC support
2014-05-27 10:05 ` Lee Jones
@ 2014-05-27 10:14 ` Boris BREZILLON
-1 siblings, 0 replies; 30+ messages in thread
From: Boris BREZILLON @ 2014-05-27 10:14 UTC (permalink / raw)
To: Lee Jones
Cc: Samuel Ortiz, Liam Girdwood, Mark Brown, Maxime Ripard,
Carlo Caione, Shuge, kevin, linux-kernel, linux-arm-kernel, dev
On 27/05/2014 12:05, Lee Jones wrote:
>> Add support for the AXP221 PMIC device to the existing AXP20x driver.
>>
>> The AXP221 defines a new set of registers, power supplies and regulators,
>> but most of the API is similar to the AXP20x ones.
>> The AXP20x irq chip definition is reused, though some interrupts are not
>> available in the AXP221.
>>
>> Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
>> ---
>> drivers/mfd/axp20x.c | 58 +++++++++++++++++++++++++++++++++++++++++++---
> Quite a difference from 237 lines, eh?
Yep.
I never said my RFC was using the best approach, I actually asked how
code sharing should be done in my cover letter ;-).
>> include/linux/mfd/axp20x.h | 56 ++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 111 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
>> index dee6539..119a7ed 100644
>> --- a/drivers/mfd/axp20x.c
>> +++ b/drivers/mfd/axp20x.c
> [...]
>
>> @@ -190,7 +233,12 @@ static int axp20x_i2c_probe(struct i2c_client *i2c,
>> axp20x->dev = &i2c->dev;
>> dev_set_drvdata(axp20x->dev, axp20x);
>>
>> - axp20x->regmap = devm_regmap_init_i2c(i2c, &axp20x_regmap_config);
>> + if (axp20x->variant == AXP221_ID)
>> + axp20x->regmap = devm_regmap_init_i2c(i2c,
>> + &axp22x_regmap_config);
>> + else
>> + axp20x->regmap = devm_regmap_init_i2c(i2c,
>> + &axp20x_regmap_config);
> Better to put these into a variable and have one instance of
> devm_regmap_init_i2c() where you pass said variable as the second
> parameter.
Sure, I'll change that.
Thanks for your review.
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v3 1/6] mfd: axp20x: add AXP221 PMIC support
@ 2014-05-27 10:14 ` Boris BREZILLON
0 siblings, 0 replies; 30+ messages in thread
From: Boris BREZILLON @ 2014-05-27 10:14 UTC (permalink / raw)
To: linux-arm-kernel
On 27/05/2014 12:05, Lee Jones wrote:
>> Add support for the AXP221 PMIC device to the existing AXP20x driver.
>>
>> The AXP221 defines a new set of registers, power supplies and regulators,
>> but most of the API is similar to the AXP20x ones.
>> The AXP20x irq chip definition is reused, though some interrupts are not
>> available in the AXP221.
>>
>> Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
>> ---
>> drivers/mfd/axp20x.c | 58 +++++++++++++++++++++++++++++++++++++++++++---
> Quite a difference from 237 lines, eh?
Yep.
I never said my RFC was using the best approach, I actually asked how
code sharing should be done in my cover letter ;-).
>> include/linux/mfd/axp20x.h | 56 ++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 111 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
>> index dee6539..119a7ed 100644
>> --- a/drivers/mfd/axp20x.c
>> +++ b/drivers/mfd/axp20x.c
> [...]
>
>> @@ -190,7 +233,12 @@ static int axp20x_i2c_probe(struct i2c_client *i2c,
>> axp20x->dev = &i2c->dev;
>> dev_set_drvdata(axp20x->dev, axp20x);
>>
>> - axp20x->regmap = devm_regmap_init_i2c(i2c, &axp20x_regmap_config);
>> + if (axp20x->variant == AXP221_ID)
>> + axp20x->regmap = devm_regmap_init_i2c(i2c,
>> + &axp22x_regmap_config);
>> + else
>> + axp20x->regmap = devm_regmap_init_i2c(i2c,
>> + &axp20x_regmap_config);
> Better to put these into a variable and have one instance of
> devm_regmap_init_i2c() where you pass said variable as the second
> parameter.
Sure, I'll change that.
Thanks for your review.
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v3 2/6] regulator: axp20x: prepare support for multiple AXP chip families
2014-05-27 9:38 ` Boris BREZILLON
@ 2014-05-27 9:38 ` Boris BREZILLON
-1 siblings, 0 replies; 30+ messages in thread
From: Boris BREZILLON @ 2014-05-27 9:38 UTC (permalink / raw)
To: Samuel Ortiz, Lee Jones, Liam Girdwood, Mark Brown
Cc: Maxime Ripard, Carlo Caione, Shuge, kevin, linux-kernel,
linux-arm-kernel, dev, Boris BREZILLON
Rework the AXP20X_ macros to support the several chip families, so that
each family can define it's own set of regulators, and regulator matches.
Pass a match table to the axp20x_regulator_parse_dt function instead of
statically using the axp20x match table.
Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
---
drivers/regulator/axp20x-regulator.c | 74 +++++++++++++++++++-----------------
1 file changed, 39 insertions(+), 35 deletions(-)
diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index 61ae4d4..5a59982 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -32,13 +32,13 @@
#define AXP20X_FREQ_DCDC_MASK 0x0f
-#define AXP20X_DESC_IO(_id, _supply, _min, _max, _step, _vreg, _vmask, _ereg, \
- _emask, _enable_val, _disable_val) \
- [AXP20X_##_id] = { \
+#define AXP_DESC_IO(_family, _id, _supply, _min, _max, _step, _vreg, _vmask, \
+ _ereg, _emask, _enable_val, _disable_val) \
+ [_family##_##_id] = { \
.name = #_id, \
.supply_name = (_supply), \
.type = REGULATOR_VOLTAGE, \
- .id = AXP20X_##_id, \
+ .id = _family##_##_id, \
.n_voltages = (((_max) - (_min)) / (_step) + 1), \
.owner = THIS_MODULE, \
.min_uV = (_min) * 1000, \
@@ -52,13 +52,13 @@
.ops = &axp20x_ops, \
}
-#define AXP20X_DESC(_id, _supply, _min, _max, _step, _vreg, _vmask, _ereg, \
- _emask) \
- [AXP20X_##_id] = { \
+#define AXP_DESC(_family, _id, _supply, _min, _max, _step, _vreg, _vmask, \
+ _ereg, _emask) \
+ [_family##_##_id] = { \
.name = #_id, \
.supply_name = (_supply), \
.type = REGULATOR_VOLTAGE, \
- .id = AXP20X_##_id, \
+ .id = _family##_##_id, \
.n_voltages = (((_max) - (_min)) / (_step) + 1), \
.owner = THIS_MODULE, \
.min_uV = (_min) * 1000, \
@@ -70,24 +70,25 @@
.ops = &axp20x_ops, \
}
-#define AXP20X_DESC_FIXED(_id, _supply, _volt) \
- [AXP20X_##_id] = { \
+#define AXP_DESC_FIXED(_family, _id, _supply, _volt) \
+ [_family##_##_id] = { \
.name = #_id, \
.supply_name = (_supply), \
.type = REGULATOR_VOLTAGE, \
- .id = AXP20X_##_id, \
+ .id = _family##_##_id, \
.n_voltages = 1, \
.owner = THIS_MODULE, \
.min_uV = (_volt) * 1000, \
.ops = &axp20x_ops_fixed \
}
-#define AXP20X_DESC_TABLE(_id, _supply, _table, _vreg, _vmask, _ereg, _emask) \
- [AXP20X_##_id] = { \
+#define AXP_DESC_TABLE(_family, _id, _supply, _table, _vreg, _vmask, _ereg, \
+ _emask) \
+ [_family##_##_id] = { \
.name = #_id, \
.supply_name = (_supply), \
.type = REGULATOR_VOLTAGE, \
- .id = AXP20X_##_id, \
+ .id = _family##_##_id, \
.n_voltages = ARRAY_SIZE(_table), \
.owner = THIS_MODULE, \
.vsel_reg = (_vreg), \
@@ -126,36 +127,36 @@ static struct regulator_ops axp20x_ops = {
};
static const struct regulator_desc axp20x_regulators[] = {
- AXP20X_DESC(DCDC2, "vin2", 700, 2275, 25, AXP20X_DCDC2_V_OUT, 0x3f,
+ AXP_DESC(AXP20X, DCDC2, "vin2", 700, 2275, 25, AXP20X_DCDC2_V_OUT, 0x3f,
AXP20X_PWR_OUT_CTRL, 0x10),
- AXP20X_DESC(DCDC3, "vin3", 700, 3500, 25, AXP20X_DCDC3_V_OUT, 0x7f,
+ AXP_DESC(AXP20X, DCDC3, "vin3", 700, 3500, 25, AXP20X_DCDC3_V_OUT, 0x7f,
AXP20X_PWR_OUT_CTRL, 0x02),
- AXP20X_DESC_FIXED(LDO1, "acin", 1300),
- AXP20X_DESC(LDO2, "ldo24in", 1800, 3300, 100, AXP20X_LDO24_V_OUT, 0xf0,
+ AXP_DESC_FIXED(AXP20X, LDO1, "acin", 1300),
+ AXP_DESC(AXP20X, LDO2, "ldo24in", 1800, 3300, 100, AXP20X_LDO24_V_OUT, 0xf0,
AXP20X_PWR_OUT_CTRL, 0x04),
- AXP20X_DESC(LDO3, "ldo3in", 700, 3500, 25, AXP20X_LDO3_V_OUT, 0x7f,
+ AXP_DESC(AXP20X, LDO3, "ldo3in", 700, 3500, 25, AXP20X_LDO3_V_OUT, 0x7f,
AXP20X_PWR_OUT_CTRL, 0x40),
- AXP20X_DESC_TABLE(LDO4, "ldo24in", axp20x_ldo4_data, AXP20X_LDO24_V_OUT, 0x0f,
+ AXP_DESC_TABLE(AXP20X, LDO4, "ldo24in", axp20x_ldo4_data, AXP20X_LDO24_V_OUT, 0x0f,
AXP20X_PWR_OUT_CTRL, 0x08),
- AXP20X_DESC_IO(LDO5, "ldo5in", 1800, 3300, 100, AXP20X_LDO5_V_OUT, 0xf0,
+ AXP_DESC_IO(AXP20X, LDO5, "ldo5in", 1800, 3300, 100, AXP20X_LDO5_V_OUT, 0xf0,
AXP20X_GPIO0_CTRL, 0x07, AXP20X_IO_ENABLED,
AXP20X_IO_DISABLED),
};
-#define AXP_MATCH(_name, _id) \
- [AXP20X_##_id] = { \
+#define AXP_MATCH(_family, _name, _id) \
+ [_family##_##_id] = { \
.name = #_name, \
- .driver_data = (void *) &axp20x_regulators[AXP20X_##_id], \
+ .driver_data = (void *) &axp20x_regulators[_family##_##_id], \
}
static struct of_regulator_match axp20x_matches[] = {
- AXP_MATCH(dcdc2, DCDC2),
- AXP_MATCH(dcdc3, DCDC3),
- AXP_MATCH(ldo1, LDO1),
- AXP_MATCH(ldo2, LDO2),
- AXP_MATCH(ldo3, LDO3),
- AXP_MATCH(ldo4, LDO4),
- AXP_MATCH(ldo5, LDO5),
+ AXP_MATCH(AXP20X, dcdc2, DCDC2),
+ AXP_MATCH(AXP20X, dcdc3, DCDC3),
+ AXP_MATCH(AXP20X, ldo1, LDO1),
+ AXP_MATCH(AXP20X, ldo2, LDO2),
+ AXP_MATCH(AXP20X, ldo3, LDO3),
+ AXP_MATCH(AXP20X, ldo4, LDO4),
+ AXP_MATCH(AXP20X, ldo5, LDO5),
};
static int axp20x_set_dcdc_freq(struct platform_device *pdev, u32 dcdcfreq)
@@ -178,7 +179,9 @@ static int axp20x_set_dcdc_freq(struct platform_device *pdev, u32 dcdcfreq)
AXP20X_FREQ_DCDC_MASK, dcdcfreq);
}
-static int axp20x_regulator_parse_dt(struct platform_device *pdev)
+static int axp20x_regulator_parse_dt(struct platform_device *pdev,
+ struct of_regulator_match *matches,
+ int nmatches)
{
struct device_node *np, *regulators;
int ret;
@@ -192,8 +195,8 @@ static int axp20x_regulator_parse_dt(struct platform_device *pdev)
if (!regulators) {
dev_warn(&pdev->dev, "regulators node not found\n");
} else {
- ret = of_regulator_match(&pdev->dev, regulators, axp20x_matches,
- ARRAY_SIZE(axp20x_matches));
+ ret = of_regulator_match(&pdev->dev, regulators, matches,
+ nmatches);
if (ret < 0) {
dev_err(&pdev->dev, "Error parsing regulator init data: %d\n", ret);
return ret;
@@ -237,7 +240,8 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
int ret, i;
u32 workmode;
- ret = axp20x_regulator_parse_dt(pdev);
+ ret = axp20x_regulator_parse_dt(pdev, axp20x_matches,
+ ARRAY_SIZE(axp20x_matches));
if (ret)
return ret;
--
1.8.3.2
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v3 2/6] regulator: axp20x: prepare support for multiple AXP chip families
@ 2014-05-27 9:38 ` Boris BREZILLON
0 siblings, 0 replies; 30+ messages in thread
From: Boris BREZILLON @ 2014-05-27 9:38 UTC (permalink / raw)
To: linux-arm-kernel
Rework the AXP20X_ macros to support the several chip families, so that
each family can define it's own set of regulators, and regulator matches.
Pass a match table to the axp20x_regulator_parse_dt function instead of
statically using the axp20x match table.
Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
---
drivers/regulator/axp20x-regulator.c | 74 +++++++++++++++++++-----------------
1 file changed, 39 insertions(+), 35 deletions(-)
diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index 61ae4d4..5a59982 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -32,13 +32,13 @@
#define AXP20X_FREQ_DCDC_MASK 0x0f
-#define AXP20X_DESC_IO(_id, _supply, _min, _max, _step, _vreg, _vmask, _ereg, \
- _emask, _enable_val, _disable_val) \
- [AXP20X_##_id] = { \
+#define AXP_DESC_IO(_family, _id, _supply, _min, _max, _step, _vreg, _vmask, \
+ _ereg, _emask, _enable_val, _disable_val) \
+ [_family##_##_id] = { \
.name = #_id, \
.supply_name = (_supply), \
.type = REGULATOR_VOLTAGE, \
- .id = AXP20X_##_id, \
+ .id = _family##_##_id, \
.n_voltages = (((_max) - (_min)) / (_step) + 1), \
.owner = THIS_MODULE, \
.min_uV = (_min) * 1000, \
@@ -52,13 +52,13 @@
.ops = &axp20x_ops, \
}
-#define AXP20X_DESC(_id, _supply, _min, _max, _step, _vreg, _vmask, _ereg, \
- _emask) \
- [AXP20X_##_id] = { \
+#define AXP_DESC(_family, _id, _supply, _min, _max, _step, _vreg, _vmask, \
+ _ereg, _emask) \
+ [_family##_##_id] = { \
.name = #_id, \
.supply_name = (_supply), \
.type = REGULATOR_VOLTAGE, \
- .id = AXP20X_##_id, \
+ .id = _family##_##_id, \
.n_voltages = (((_max) - (_min)) / (_step) + 1), \
.owner = THIS_MODULE, \
.min_uV = (_min) * 1000, \
@@ -70,24 +70,25 @@
.ops = &axp20x_ops, \
}
-#define AXP20X_DESC_FIXED(_id, _supply, _volt) \
- [AXP20X_##_id] = { \
+#define AXP_DESC_FIXED(_family, _id, _supply, _volt) \
+ [_family##_##_id] = { \
.name = #_id, \
.supply_name = (_supply), \
.type = REGULATOR_VOLTAGE, \
- .id = AXP20X_##_id, \
+ .id = _family##_##_id, \
.n_voltages = 1, \
.owner = THIS_MODULE, \
.min_uV = (_volt) * 1000, \
.ops = &axp20x_ops_fixed \
}
-#define AXP20X_DESC_TABLE(_id, _supply, _table, _vreg, _vmask, _ereg, _emask) \
- [AXP20X_##_id] = { \
+#define AXP_DESC_TABLE(_family, _id, _supply, _table, _vreg, _vmask, _ereg, \
+ _emask) \
+ [_family##_##_id] = { \
.name = #_id, \
.supply_name = (_supply), \
.type = REGULATOR_VOLTAGE, \
- .id = AXP20X_##_id, \
+ .id = _family##_##_id, \
.n_voltages = ARRAY_SIZE(_table), \
.owner = THIS_MODULE, \
.vsel_reg = (_vreg), \
@@ -126,36 +127,36 @@ static struct regulator_ops axp20x_ops = {
};
static const struct regulator_desc axp20x_regulators[] = {
- AXP20X_DESC(DCDC2, "vin2", 700, 2275, 25, AXP20X_DCDC2_V_OUT, 0x3f,
+ AXP_DESC(AXP20X, DCDC2, "vin2", 700, 2275, 25, AXP20X_DCDC2_V_OUT, 0x3f,
AXP20X_PWR_OUT_CTRL, 0x10),
- AXP20X_DESC(DCDC3, "vin3", 700, 3500, 25, AXP20X_DCDC3_V_OUT, 0x7f,
+ AXP_DESC(AXP20X, DCDC3, "vin3", 700, 3500, 25, AXP20X_DCDC3_V_OUT, 0x7f,
AXP20X_PWR_OUT_CTRL, 0x02),
- AXP20X_DESC_FIXED(LDO1, "acin", 1300),
- AXP20X_DESC(LDO2, "ldo24in", 1800, 3300, 100, AXP20X_LDO24_V_OUT, 0xf0,
+ AXP_DESC_FIXED(AXP20X, LDO1, "acin", 1300),
+ AXP_DESC(AXP20X, LDO2, "ldo24in", 1800, 3300, 100, AXP20X_LDO24_V_OUT, 0xf0,
AXP20X_PWR_OUT_CTRL, 0x04),
- AXP20X_DESC(LDO3, "ldo3in", 700, 3500, 25, AXP20X_LDO3_V_OUT, 0x7f,
+ AXP_DESC(AXP20X, LDO3, "ldo3in", 700, 3500, 25, AXP20X_LDO3_V_OUT, 0x7f,
AXP20X_PWR_OUT_CTRL, 0x40),
- AXP20X_DESC_TABLE(LDO4, "ldo24in", axp20x_ldo4_data, AXP20X_LDO24_V_OUT, 0x0f,
+ AXP_DESC_TABLE(AXP20X, LDO4, "ldo24in", axp20x_ldo4_data, AXP20X_LDO24_V_OUT, 0x0f,
AXP20X_PWR_OUT_CTRL, 0x08),
- AXP20X_DESC_IO(LDO5, "ldo5in", 1800, 3300, 100, AXP20X_LDO5_V_OUT, 0xf0,
+ AXP_DESC_IO(AXP20X, LDO5, "ldo5in", 1800, 3300, 100, AXP20X_LDO5_V_OUT, 0xf0,
AXP20X_GPIO0_CTRL, 0x07, AXP20X_IO_ENABLED,
AXP20X_IO_DISABLED),
};
-#define AXP_MATCH(_name, _id) \
- [AXP20X_##_id] = { \
+#define AXP_MATCH(_family, _name, _id) \
+ [_family##_##_id] = { \
.name = #_name, \
- .driver_data = (void *) &axp20x_regulators[AXP20X_##_id], \
+ .driver_data = (void *) &axp20x_regulators[_family##_##_id], \
}
static struct of_regulator_match axp20x_matches[] = {
- AXP_MATCH(dcdc2, DCDC2),
- AXP_MATCH(dcdc3, DCDC3),
- AXP_MATCH(ldo1, LDO1),
- AXP_MATCH(ldo2, LDO2),
- AXP_MATCH(ldo3, LDO3),
- AXP_MATCH(ldo4, LDO4),
- AXP_MATCH(ldo5, LDO5),
+ AXP_MATCH(AXP20X, dcdc2, DCDC2),
+ AXP_MATCH(AXP20X, dcdc3, DCDC3),
+ AXP_MATCH(AXP20X, ldo1, LDO1),
+ AXP_MATCH(AXP20X, ldo2, LDO2),
+ AXP_MATCH(AXP20X, ldo3, LDO3),
+ AXP_MATCH(AXP20X, ldo4, LDO4),
+ AXP_MATCH(AXP20X, ldo5, LDO5),
};
static int axp20x_set_dcdc_freq(struct platform_device *pdev, u32 dcdcfreq)
@@ -178,7 +179,9 @@ static int axp20x_set_dcdc_freq(struct platform_device *pdev, u32 dcdcfreq)
AXP20X_FREQ_DCDC_MASK, dcdcfreq);
}
-static int axp20x_regulator_parse_dt(struct platform_device *pdev)
+static int axp20x_regulator_parse_dt(struct platform_device *pdev,
+ struct of_regulator_match *matches,
+ int nmatches)
{
struct device_node *np, *regulators;
int ret;
@@ -192,8 +195,8 @@ static int axp20x_regulator_parse_dt(struct platform_device *pdev)
if (!regulators) {
dev_warn(&pdev->dev, "regulators node not found\n");
} else {
- ret = of_regulator_match(&pdev->dev, regulators, axp20x_matches,
- ARRAY_SIZE(axp20x_matches));
+ ret = of_regulator_match(&pdev->dev, regulators, matches,
+ nmatches);
if (ret < 0) {
dev_err(&pdev->dev, "Error parsing regulator init data: %d\n", ret);
return ret;
@@ -237,7 +240,8 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
int ret, i;
u32 workmode;
- ret = axp20x_regulator_parse_dt(pdev);
+ ret = axp20x_regulator_parse_dt(pdev, axp20x_matches,
+ ARRAY_SIZE(axp20x_matches));
if (ret)
return ret;
--
1.8.3.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v3 3/6] regulator: axp20x: add support for AXP221 regulators
2014-05-27 9:38 ` Boris BREZILLON
@ 2014-05-27 9:38 ` Boris BREZILLON
-1 siblings, 0 replies; 30+ messages in thread
From: Boris BREZILLON @ 2014-05-27 9:38 UTC (permalink / raw)
To: Samuel Ortiz, Lee Jones, Liam Girdwood, Mark Brown
Cc: Maxime Ripard, Carlo Caione, Shuge, kevin, linux-kernel,
linux-arm-kernel, dev, Boris BREZILLON
Add AXP221 regulator definitions and choose the appropriate definitions
according to the variant id passed by the MFD device.
Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
---
drivers/regulator/axp20x-regulator.c | 98 +++++++++++++++++++++++++++++++++---
1 file changed, 90 insertions(+), 8 deletions(-)
diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index 5a59982..feaa0b5 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -27,6 +27,9 @@
#define AXP20X_IO_ENABLED 0x03
#define AXP20X_IO_DISABLED 0x07
+#define AXP22X_IO_ENABLED 0x04
+#define AXP22X_IO_DISABLED 0x03
+
#define AXP20X_WORKMODE_DCDC2_MASK BIT(2)
#define AXP20X_WORKMODE_DCDC3_MASK BIT(1)
@@ -143,6 +146,48 @@ static const struct regulator_desc axp20x_regulators[] = {
AXP20X_IO_DISABLED),
};
+static const struct regulator_desc axp22x_regulators[] = {
+ AXP_DESC(AXP22X, DCDC1, "vin1", 1600, 3400, 100, AXP22X_DCDC1_V_OUT, 0x1f,
+ AXP22X_PWR_OUT_CTRL1, BIT(1)),
+ AXP_DESC(AXP22X, DCDC2, "vin2", 600, 1540, 20, AXP22X_DCDC2_V_OUT, 0x3f,
+ AXP22X_PWR_OUT_CTRL1, BIT(2)),
+ AXP_DESC(AXP22X, DCDC3, "vin3", 600, 1860, 20, AXP22X_DCDC3_V_OUT, 0x3f,
+ AXP22X_PWR_OUT_CTRL1, BIT(3)),
+ AXP_DESC(AXP22X, DCDC4, "vin4", 600, 1540, 20, AXP22X_DCDC4_V_OUT, 0x3f,
+ AXP22X_PWR_OUT_CTRL1, BIT(3)),
+ AXP_DESC(AXP22X, DCDC5, "vin5", 1000, 2550, 50, AXP22X_DCDC5_V_OUT, 0x1f,
+ AXP22X_PWR_OUT_CTRL1, BIT(4)),
+ AXP_DESC(AXP22X, DC5LDO, "vin5", 700, 1400, 100, AXP22X_DC5LDO_V_OUT, 0x7,
+ AXP22X_PWR_OUT_CTRL1, BIT(0)),
+ AXP_DESC(AXP22X, ALDO1, "aldoin", 700, 3300, 100, AXP22X_ALDO1_V_OUT, 0x1f,
+ AXP22X_PWR_OUT_CTRL1, BIT(6)),
+ AXP_DESC(AXP22X, ALDO2, "aldoin", 700, 3300, 100, AXP22X_ALDO2_V_OUT, 0x1f,
+ AXP22X_PWR_OUT_CTRL1, BIT(7)),
+ AXP_DESC(AXP22X, ALDO3, "aldoin", 700, 3300, 100, AXP22X_ALDO3_V_OUT, 0x1f,
+ AXP22X_PWR_OUT_CTRL3, BIT(7)),
+ AXP_DESC(AXP22X, DLDO1, "dldoin", 700, 3300, 100, AXP22X_DLDO1_V_OUT, 0x1f,
+ AXP22X_PWR_OUT_CTRL2, BIT(3)),
+ AXP_DESC(AXP22X, DLDO2, "dldoin", 700, 3300, 100, AXP22X_DLDO2_V_OUT, 0x1f,
+ AXP22X_PWR_OUT_CTRL2, BIT(4)),
+ AXP_DESC(AXP22X, DLDO3, "dldoin", 700, 3300, 100, AXP22X_DLDO3_V_OUT, 0x1f,
+ AXP22X_PWR_OUT_CTRL2, BIT(5)),
+ AXP_DESC(AXP22X, DLDO4, "dldoin", 700, 3300, 100, AXP22X_DLDO4_V_OUT, 0x1f,
+ AXP22X_PWR_OUT_CTRL2, BIT(6)),
+ AXP_DESC(AXP22X, ELDO1, "eldoin", 700, 3300, 100, AXP22X_ELDO1_V_OUT, 0x1f,
+ AXP22X_PWR_OUT_CTRL2, BIT(0)),
+ AXP_DESC(AXP22X, ELDO2, "eldoin", 700, 3300, 100, AXP22X_ELDO2_V_OUT, 0x1f,
+ AXP22X_PWR_OUT_CTRL2, BIT(1)),
+ AXP_DESC(AXP22X, ELDO3, "eldoin", 700, 3300, 100, AXP22X_ELDO3_V_OUT, 0x1f,
+ AXP22X_PWR_OUT_CTRL2, BIT(2)),
+ AXP_DESC_IO(AXP22X, LDO_IO0, "ldoioin", 1800, 3300, 100, AXP22X_LDO_IO0_V_OUT,
+ 0x1f, AXP20X_GPIO0_CTRL, 0x07, AXP22X_IO_ENABLED,
+ AXP22X_IO_DISABLED),
+ AXP_DESC_IO(AXP22X, LDO_IO1, "ldoioin", 1800, 3300, 100, AXP22X_LDO_IO1_V_OUT,
+ 0x1f, AXP20X_GPIO1_CTRL, 0x07, AXP22X_IO_ENABLED,
+ AXP22X_IO_DISABLED),
+ AXP_DESC_FIXED(AXP22X, RTC_LDO, "rtcldoin", 3000),
+};
+
#define AXP_MATCH(_family, _name, _id) \
[_family##_##_id] = { \
.name = #_name, \
@@ -159,6 +204,28 @@ static struct of_regulator_match axp20x_matches[] = {
AXP_MATCH(AXP20X, ldo5, LDO5),
};
+static struct of_regulator_match axp22x_matches[] = {
+ AXP_MATCH(AXP22X, dcdc1, DCDC1),
+ AXP_MATCH(AXP22X, dcdc2, DCDC2),
+ AXP_MATCH(AXP22X, dcdc3, DCDC3),
+ AXP_MATCH(AXP22X, dcdc4, DCDC4),
+ AXP_MATCH(AXP22X, dcdc5, DCDC5),
+ AXP_MATCH(AXP22X, dc5ldo, DC5LDO),
+ AXP_MATCH(AXP22X, aldo1, ALDO1),
+ AXP_MATCH(AXP22X, aldo2, ALDO2),
+ AXP_MATCH(AXP22X, aldo3, ALDO3),
+ AXP_MATCH(AXP22X, dldo1, DLDO1),
+ AXP_MATCH(AXP22X, dldo2, DLDO2),
+ AXP_MATCH(AXP22X, dldo3, DLDO3),
+ AXP_MATCH(AXP22X, dldo4, DLDO4),
+ AXP_MATCH(AXP22X, eldo1, ELDO1),
+ AXP_MATCH(AXP22X, eldo2, ELDO2),
+ AXP_MATCH(AXP22X, eldo3, ELDO3),
+ AXP_MATCH(AXP22X, ldo_io0, LDO_IO0),
+ AXP_MATCH(AXP22X, ldo_io1, LDO_IO1),
+ AXP_MATCH(AXP22X, rtc_ldo, RTC_LDO),
+};
+
static int axp20x_set_dcdc_freq(struct platform_device *pdev, u32 dcdcfreq)
{
struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
@@ -237,37 +304,52 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
struct regulator_config config = { };
struct regulator_init_data *init_data;
+ struct of_regulator_match *matches;
+ int nmatches;
+ const struct regulator_desc *regulators;
+ int nregulators;
int ret, i;
u32 workmode;
- ret = axp20x_regulator_parse_dt(pdev, axp20x_matches,
- ARRAY_SIZE(axp20x_matches));
+ if (axp20x->variant == AXP221_ID) {
+ matches = axp22x_matches;
+ nmatches = ARRAY_SIZE(axp22x_matches);
+ regulators = axp22x_regulators;
+ nregulators = AXP22X_REG_ID_MAX;
+ } else {
+ matches = axp20x_matches;
+ nmatches = ARRAY_SIZE(axp20x_matches);
+ regulators = axp20x_regulators;
+ nregulators = AXP20X_REG_ID_MAX;
+ }
+
+ ret = axp20x_regulator_parse_dt(pdev, matches, nmatches);
if (ret)
return ret;
for (i = 0; i < AXP20X_REG_ID_MAX; i++) {
- init_data = axp20x_matches[i].init_data;
+ init_data = matches[i].init_data;
config.dev = &pdev->dev;
config.init_data = init_data;
config.regmap = axp20x->regmap;
- config.of_node = axp20x_matches[i].of_node;
+ config.of_node = matches[i].of_node;
- rdev = devm_regulator_register(&pdev->dev, &axp20x_regulators[i],
+ rdev = devm_regulator_register(&pdev->dev, ®ulators[i],
&config);
if (IS_ERR(rdev)) {
dev_err(&pdev->dev, "Failed to register %s\n",
- axp20x_regulators[i].name);
+ regulators[i].name);
return PTR_ERR(rdev);
}
- ret = of_property_read_u32(axp20x_matches[i].of_node, "x-powers,dcdc-workmode",
+ ret = of_property_read_u32(matches[i].of_node, "x-powers,dcdc-workmode",
&workmode);
if (!ret) {
if (axp20x_set_dcdc_workmode(rdev, i, workmode))
dev_err(&pdev->dev, "Failed to set workmode on %s\n",
- axp20x_regulators[i].name);
+ regulators[i].name);
}
}
--
1.8.3.2
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v3 3/6] regulator: axp20x: add support for AXP221 regulators
@ 2014-05-27 9:38 ` Boris BREZILLON
0 siblings, 0 replies; 30+ messages in thread
From: Boris BREZILLON @ 2014-05-27 9:38 UTC (permalink / raw)
To: linux-arm-kernel
Add AXP221 regulator definitions and choose the appropriate definitions
according to the variant id passed by the MFD device.
Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
---
drivers/regulator/axp20x-regulator.c | 98 +++++++++++++++++++++++++++++++++---
1 file changed, 90 insertions(+), 8 deletions(-)
diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index 5a59982..feaa0b5 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -27,6 +27,9 @@
#define AXP20X_IO_ENABLED 0x03
#define AXP20X_IO_DISABLED 0x07
+#define AXP22X_IO_ENABLED 0x04
+#define AXP22X_IO_DISABLED 0x03
+
#define AXP20X_WORKMODE_DCDC2_MASK BIT(2)
#define AXP20X_WORKMODE_DCDC3_MASK BIT(1)
@@ -143,6 +146,48 @@ static const struct regulator_desc axp20x_regulators[] = {
AXP20X_IO_DISABLED),
};
+static const struct regulator_desc axp22x_regulators[] = {
+ AXP_DESC(AXP22X, DCDC1, "vin1", 1600, 3400, 100, AXP22X_DCDC1_V_OUT, 0x1f,
+ AXP22X_PWR_OUT_CTRL1, BIT(1)),
+ AXP_DESC(AXP22X, DCDC2, "vin2", 600, 1540, 20, AXP22X_DCDC2_V_OUT, 0x3f,
+ AXP22X_PWR_OUT_CTRL1, BIT(2)),
+ AXP_DESC(AXP22X, DCDC3, "vin3", 600, 1860, 20, AXP22X_DCDC3_V_OUT, 0x3f,
+ AXP22X_PWR_OUT_CTRL1, BIT(3)),
+ AXP_DESC(AXP22X, DCDC4, "vin4", 600, 1540, 20, AXP22X_DCDC4_V_OUT, 0x3f,
+ AXP22X_PWR_OUT_CTRL1, BIT(3)),
+ AXP_DESC(AXP22X, DCDC5, "vin5", 1000, 2550, 50, AXP22X_DCDC5_V_OUT, 0x1f,
+ AXP22X_PWR_OUT_CTRL1, BIT(4)),
+ AXP_DESC(AXP22X, DC5LDO, "vin5", 700, 1400, 100, AXP22X_DC5LDO_V_OUT, 0x7,
+ AXP22X_PWR_OUT_CTRL1, BIT(0)),
+ AXP_DESC(AXP22X, ALDO1, "aldoin", 700, 3300, 100, AXP22X_ALDO1_V_OUT, 0x1f,
+ AXP22X_PWR_OUT_CTRL1, BIT(6)),
+ AXP_DESC(AXP22X, ALDO2, "aldoin", 700, 3300, 100, AXP22X_ALDO2_V_OUT, 0x1f,
+ AXP22X_PWR_OUT_CTRL1, BIT(7)),
+ AXP_DESC(AXP22X, ALDO3, "aldoin", 700, 3300, 100, AXP22X_ALDO3_V_OUT, 0x1f,
+ AXP22X_PWR_OUT_CTRL3, BIT(7)),
+ AXP_DESC(AXP22X, DLDO1, "dldoin", 700, 3300, 100, AXP22X_DLDO1_V_OUT, 0x1f,
+ AXP22X_PWR_OUT_CTRL2, BIT(3)),
+ AXP_DESC(AXP22X, DLDO2, "dldoin", 700, 3300, 100, AXP22X_DLDO2_V_OUT, 0x1f,
+ AXP22X_PWR_OUT_CTRL2, BIT(4)),
+ AXP_DESC(AXP22X, DLDO3, "dldoin", 700, 3300, 100, AXP22X_DLDO3_V_OUT, 0x1f,
+ AXP22X_PWR_OUT_CTRL2, BIT(5)),
+ AXP_DESC(AXP22X, DLDO4, "dldoin", 700, 3300, 100, AXP22X_DLDO4_V_OUT, 0x1f,
+ AXP22X_PWR_OUT_CTRL2, BIT(6)),
+ AXP_DESC(AXP22X, ELDO1, "eldoin", 700, 3300, 100, AXP22X_ELDO1_V_OUT, 0x1f,
+ AXP22X_PWR_OUT_CTRL2, BIT(0)),
+ AXP_DESC(AXP22X, ELDO2, "eldoin", 700, 3300, 100, AXP22X_ELDO2_V_OUT, 0x1f,
+ AXP22X_PWR_OUT_CTRL2, BIT(1)),
+ AXP_DESC(AXP22X, ELDO3, "eldoin", 700, 3300, 100, AXP22X_ELDO3_V_OUT, 0x1f,
+ AXP22X_PWR_OUT_CTRL2, BIT(2)),
+ AXP_DESC_IO(AXP22X, LDO_IO0, "ldoioin", 1800, 3300, 100, AXP22X_LDO_IO0_V_OUT,
+ 0x1f, AXP20X_GPIO0_CTRL, 0x07, AXP22X_IO_ENABLED,
+ AXP22X_IO_DISABLED),
+ AXP_DESC_IO(AXP22X, LDO_IO1, "ldoioin", 1800, 3300, 100, AXP22X_LDO_IO1_V_OUT,
+ 0x1f, AXP20X_GPIO1_CTRL, 0x07, AXP22X_IO_ENABLED,
+ AXP22X_IO_DISABLED),
+ AXP_DESC_FIXED(AXP22X, RTC_LDO, "rtcldoin", 3000),
+};
+
#define AXP_MATCH(_family, _name, _id) \
[_family##_##_id] = { \
.name = #_name, \
@@ -159,6 +204,28 @@ static struct of_regulator_match axp20x_matches[] = {
AXP_MATCH(AXP20X, ldo5, LDO5),
};
+static struct of_regulator_match axp22x_matches[] = {
+ AXP_MATCH(AXP22X, dcdc1, DCDC1),
+ AXP_MATCH(AXP22X, dcdc2, DCDC2),
+ AXP_MATCH(AXP22X, dcdc3, DCDC3),
+ AXP_MATCH(AXP22X, dcdc4, DCDC4),
+ AXP_MATCH(AXP22X, dcdc5, DCDC5),
+ AXP_MATCH(AXP22X, dc5ldo, DC5LDO),
+ AXP_MATCH(AXP22X, aldo1, ALDO1),
+ AXP_MATCH(AXP22X, aldo2, ALDO2),
+ AXP_MATCH(AXP22X, aldo3, ALDO3),
+ AXP_MATCH(AXP22X, dldo1, DLDO1),
+ AXP_MATCH(AXP22X, dldo2, DLDO2),
+ AXP_MATCH(AXP22X, dldo3, DLDO3),
+ AXP_MATCH(AXP22X, dldo4, DLDO4),
+ AXP_MATCH(AXP22X, eldo1, ELDO1),
+ AXP_MATCH(AXP22X, eldo2, ELDO2),
+ AXP_MATCH(AXP22X, eldo3, ELDO3),
+ AXP_MATCH(AXP22X, ldo_io0, LDO_IO0),
+ AXP_MATCH(AXP22X, ldo_io1, LDO_IO1),
+ AXP_MATCH(AXP22X, rtc_ldo, RTC_LDO),
+};
+
static int axp20x_set_dcdc_freq(struct platform_device *pdev, u32 dcdcfreq)
{
struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
@@ -237,37 +304,52 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
struct regulator_config config = { };
struct regulator_init_data *init_data;
+ struct of_regulator_match *matches;
+ int nmatches;
+ const struct regulator_desc *regulators;
+ int nregulators;
int ret, i;
u32 workmode;
- ret = axp20x_regulator_parse_dt(pdev, axp20x_matches,
- ARRAY_SIZE(axp20x_matches));
+ if (axp20x->variant == AXP221_ID) {
+ matches = axp22x_matches;
+ nmatches = ARRAY_SIZE(axp22x_matches);
+ regulators = axp22x_regulators;
+ nregulators = AXP22X_REG_ID_MAX;
+ } else {
+ matches = axp20x_matches;
+ nmatches = ARRAY_SIZE(axp20x_matches);
+ regulators = axp20x_regulators;
+ nregulators = AXP20X_REG_ID_MAX;
+ }
+
+ ret = axp20x_regulator_parse_dt(pdev, matches, nmatches);
if (ret)
return ret;
for (i = 0; i < AXP20X_REG_ID_MAX; i++) {
- init_data = axp20x_matches[i].init_data;
+ init_data = matches[i].init_data;
config.dev = &pdev->dev;
config.init_data = init_data;
config.regmap = axp20x->regmap;
- config.of_node = axp20x_matches[i].of_node;
+ config.of_node = matches[i].of_node;
- rdev = devm_regulator_register(&pdev->dev, &axp20x_regulators[i],
+ rdev = devm_regulator_register(&pdev->dev, ®ulators[i],
&config);
if (IS_ERR(rdev)) {
dev_err(&pdev->dev, "Failed to register %s\n",
- axp20x_regulators[i].name);
+ regulators[i].name);
return PTR_ERR(rdev);
}
- ret = of_property_read_u32(axp20x_matches[i].of_node, "x-powers,dcdc-workmode",
+ ret = of_property_read_u32(matches[i].of_node, "x-powers,dcdc-workmode",
&workmode);
if (!ret) {
if (axp20x_set_dcdc_workmode(rdev, i, workmode))
dev_err(&pdev->dev, "Failed to set workmode on %s\n",
- axp20x_regulators[i].name);
+ regulators[i].name);
}
}
--
1.8.3.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v3 4/6] regulator: axp20x: reset probe data before each probe
2014-05-27 9:38 ` Boris BREZILLON
@ 2014-05-27 9:38 ` Boris BREZILLON
-1 siblings, 0 replies; 30+ messages in thread
From: Boris BREZILLON @ 2014-05-27 9:38 UTC (permalink / raw)
To: Samuel Ortiz, Lee Jones, Liam Girdwood, Mark Brown
Cc: Maxime Ripard, Carlo Caione, Shuge, kevin, linux-kernel,
linux-arm-kernel, dev, Boris BREZILLON
The init_data and of_node fields of the axp2xx_matches tables are filled
at each device probe by the axp20x_regulator_parse_dt function (which then
calls the of_regulator_match function).
This means we can probe a new device and consider data initialized during
the probe of another device as valid.
Reset init_data and of_node field to NULL before each probe in order to
avoid this kind of issue.
Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
---
drivers/regulator/axp20x-regulator.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index feaa0b5..4486517 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -323,6 +323,15 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
nregulators = AXP20X_REG_ID_MAX;
}
+ /*
+ * Reset matches table (this table might have been modified by a
+ * previous AXP2xx device probe).
+ */
+ for (i = 0; i < nmatches; i++) {
+ matches[i].init_data = NULL;
+ matches[i].of_node = NULL;
+ }
+
ret = axp20x_regulator_parse_dt(pdev, matches, nmatches);
if (ret)
return ret;
--
1.8.3.2
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v3 4/6] regulator: axp20x: reset probe data before each probe
@ 2014-05-27 9:38 ` Boris BREZILLON
0 siblings, 0 replies; 30+ messages in thread
From: Boris BREZILLON @ 2014-05-27 9:38 UTC (permalink / raw)
To: linux-arm-kernel
The init_data and of_node fields of the axp2xx_matches tables are filled
at each device probe by the axp20x_regulator_parse_dt function (which then
calls the of_regulator_match function).
This means we can probe a new device and consider data initialized during
the probe of another device as valid.
Reset init_data and of_node field to NULL before each probe in order to
avoid this kind of issue.
Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
---
drivers/regulator/axp20x-regulator.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index feaa0b5..4486517 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -323,6 +323,15 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
nregulators = AXP20X_REG_ID_MAX;
}
+ /*
+ * Reset matches table (this table might have been modified by a
+ * previous AXP2xx device probe).
+ */
+ for (i = 0; i < nmatches; i++) {
+ matches[i].init_data = NULL;
+ matches[i].of_node = NULL;
+ }
+
ret = axp20x_regulator_parse_dt(pdev, matches, nmatches);
if (ret)
return ret;
--
1.8.3.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v3 5/6] regulator: add support for regulator set registration
2014-05-27 9:38 ` Boris BREZILLON
@ 2014-05-27 9:38 ` Boris BREZILLON
-1 siblings, 0 replies; 30+ messages in thread
From: Boris BREZILLON @ 2014-05-27 9:38 UTC (permalink / raw)
To: Samuel Ortiz, Lee Jones, Liam Girdwood, Mark Brown
Cc: Maxime Ripard, Carlo Caione, Shuge, kevin, linux-kernel,
linux-arm-kernel, dev, Boris BREZILLON
PMIC devices often provide several regulators, and these regulators are
all using the same regmap and are all attached to the same device (the
PMIC device).
Add helper functions (both simple and resource managed versions) to
register and unregister such kind of regulator set.
This implementation handles self dependency issues where one regulator
provided the PMIC depends on another one provided by the same PMIC.
Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
---
drivers/regulator/core.c | 106 +++++++++++++++++++++++++++++++++++++++
drivers/regulator/devres.c | 68 +++++++++++++++++++++++++
include/linux/regulator/driver.h | 51 +++++++++++++++++++
3 files changed, 225 insertions(+)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 9a09f3c..7703853 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3595,6 +3595,112 @@ void regulator_unregister(struct regulator_dev *rdev)
EXPORT_SYMBOL_GPL(regulator_unregister);
/**
+ * regulator_set_register - register a set of regulators
+ * @config: runtime configuration for regulator set
+ *
+ * Called by regulator drivers to register a set of regulators.
+ * Returns a valid pointer to struct regulator_set on success
+ * or an ERR_PTR() on error.
+ */
+struct regulator_set *
+regulator_set_register(const struct regulator_set_config *config)
+{
+ struct regulator_set *rset;
+ int prev_nregistered = -1;
+ int nregistered = 0;
+ int ret;
+ int i;
+
+ if (!config->descs || !config->nregulators)
+ return ERR_PTR(-EINVAL);
+
+ rset = kzalloc(sizeof(*rset) +
+ (config->nregulators * sizeof(struct regulator_dev *)),
+ GFP_KERNEL);
+ if (!rset)
+ return ERR_PTR(-ENOMEM);
+
+ rset->nregulators = config->nregulators;
+
+ while (nregistered < config->nregulators &&
+ prev_nregistered < nregistered) {
+
+ prev_nregistered = nregistered;
+
+ for (i = 0; i < config->nregulators; i++) {
+ struct regulator_config conf;
+ struct regulator_dev *regulator;
+
+ if (rset->regulators[i])
+ continue;
+
+ memset(&conf, 0, sizeof(conf));
+
+ conf.dev = config->dev;
+ conf.regmap = config->regmap;
+ conf.driver_data = config->driver_data;
+ if (config->matches && config->matches[i].init_data)
+ conf.init_data = config->matches[i].init_data;
+ else if (config->init_data)
+ conf.init_data = config->init_data[i];
+
+ if (config->matches)
+ conf.of_node = config->matches[i].of_node;
+
+ regulator = regulator_register(&config->descs[i],
+ &conf);
+ if (IS_ERR(regulator)) {
+ ret = PTR_ERR(regulator);
+ if (ret == -EPROBE_DEFER)
+ continue;
+
+ goto err;
+ }
+
+ rset->regulators[i] = regulator;
+ nregistered++;
+ }
+ }
+
+ if (nregistered < config->nregulators) {
+ ret = -EPROBE_DEFER;
+ goto err;
+ }
+
+ return rset;
+
+err:
+ for (i = 0; i < config->nregulators; i++) {
+ if (rset->regulators[i])
+ regulator_unregister(rset->regulators[i]);
+ }
+
+ kfree(rset);
+
+ return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(regulator_set_register);
+
+/**
+ * regulator_set_unregister - unregister regulator set
+ * @rset: regulator set to unregister
+ *
+ * Called by regulator drivers to unregister a regulator set.
+ */
+void regulator_set_unregister(struct regulator_set *rset)
+{
+ int i;
+
+ for (i = 0; i < rset->nregulators; i++) {
+ if (rset->regulators[i])
+ regulator_unregister(rset->regulators[i]);
+ }
+
+ kfree(rset);
+}
+EXPORT_SYMBOL_GPL(regulator_set_unregister);
+
+/**
* regulator_suspend_prepare - prepare regulators for system wide suspend
* @state: system suspend state
*
diff --git a/drivers/regulator/devres.c b/drivers/regulator/devres.c
index f44818b..a34f34c 100644
--- a/drivers/regulator/devres.c
+++ b/drivers/regulator/devres.c
@@ -251,6 +251,74 @@ void devm_regulator_unregister(struct device *dev, struct regulator_dev *rdev)
}
EXPORT_SYMBOL_GPL(devm_regulator_unregister);
+static void devm_rset_release(struct device *dev, void *res)
+{
+ regulator_set_unregister(*(struct regulator_set **)res);
+}
+
+/**
+ * devm_regulator_set_register - Resource managed regulator_set_register()
+ * @dev: device registering the regulator set
+ * @config: runtime configuration for regulator set
+ *
+ * Called by regulator drivers to register a set of regulators. Returns a
+ * valid pointer to struct regulator_set on success or an ERR_PTR() on
+ * error. The regulator will automatically be released when the device
+ * is unbound.
+ */
+struct regulator_set *
+devm_regulator_set_register(struct device *dev,
+ const struct regulator_set_config *config)
+{
+ struct regulator_set **ptr, *rset;
+
+ ptr = devres_alloc(devm_rset_release, sizeof(*ptr),
+ GFP_KERNEL);
+ if (!ptr)
+ return ERR_PTR(-ENOMEM);
+
+ rset = regulator_set_register(config);
+ if (!IS_ERR(rset)) {
+ *ptr = rset;
+ devres_add(dev, ptr);
+ } else {
+ devres_free(ptr);
+ }
+
+ return rset;
+}
+EXPORT_SYMBOL_GPL(devm_regulator_set_register);
+
+static int devm_rset_match(struct device *dev, void *res, void *data)
+{
+ struct regulator_set **r = res;
+ if (!r || !*r) {
+ WARN_ON(!r || !*r);
+ return 0;
+ }
+ return *r == data;
+}
+
+/**
+ * devm_regulator_set_unregister - Resource managed regulator_set_unregister()
+ * @dev: device unregistering the regulator set
+ * @rset: regulator set to release
+ *
+ * Unregister a regulator set registered with devm_regulator_set_register().
+ * Normally this function will not need to be called and the resource
+ * management code will ensure that the resource is freed.
+ */
+void devm_regulator_set_unregister(struct device *dev,
+ struct regulator_set *rset)
+{
+ int rc;
+
+ rc = devres_release(dev, devm_rdev_release, devm_rset_match, rset);
+ if (rc != 0)
+ WARN_ON(rc);
+}
+EXPORT_SYMBOL_GPL(devm_regulator_unregister);
+
struct regulator_supply_alias_match {
struct device *dev;
const char *id;
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index bbe03a1..b683f68 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -350,6 +350,48 @@ struct regulator_dev {
unsigned int ena_gpio_state:1;
};
+/**
+ * struct regulator_set_config - Dynamic regulator set descriptor
+ *
+ * This structure describe a regulator and each regulator in this set
+ * will be registered using informations passed through regulator_desc
+ * and of_regulator_match tables.
+ *
+ * @dev: struct device providing the regulator set.
+ * @driver_data: private driver data for this regulator set.
+ * @regmap: regmap to use for core regmap helpers if dev_get_regulator() is
+ * insufficient.
+ * @descs: table of regulator descriptors.
+ * @matches: table of DT regulator descriptors. This table should have
+ * been filled by the of_regulator_match function.
+ * @init_data: a table of init_data in case matches is NULL or the init_data
+ * of the match entry is NULL.
+ * @nregulators: number of regulators in the regulator set.
+ */
+struct regulator_set_config {
+ struct device *dev;
+ void *driver_data;
+ struct regmap *regmap;
+ const struct regulator_desc *descs;
+ const struct of_regulator_match *matches;
+ const struct regulator_init_data **init_data;
+ int nregulators;
+};
+
+/*
+ * struct regulator_set - Regulator set
+ *
+ * This structure stores a set of regulator devices provided by a single
+ * device (e.g. a PMIC device).
+ *
+ * @nregulators: number of regulators in the set
+ * @regulators: regulators table
+ */
+struct regulator_set {
+ int nregulators;
+ struct regulator_dev *regulators[0];
+};
+
struct regulator_dev *
regulator_register(const struct regulator_desc *regulator_desc,
const struct regulator_config *config);
@@ -360,6 +402,15 @@ devm_regulator_register(struct device *dev,
void regulator_unregister(struct regulator_dev *rdev);
void devm_regulator_unregister(struct device *dev, struct regulator_dev *rdev);
+struct regulator_set *
+regulator_set_register(const struct regulator_set_config *config);
+struct regulator_set *
+devm_regulator_set_register(struct device *dev,
+ const struct regulator_set_config *config);
+void regulator_set_unregister(struct regulator_set *rset);
+void devm_regulator_set_unregister(struct device *dev,
+ struct regulator_set *rset);
+
int regulator_notifier_call_chain(struct regulator_dev *rdev,
unsigned long event, void *data);
--
1.8.3.2
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v3 5/6] regulator: add support for regulator set registration
@ 2014-05-27 9:38 ` Boris BREZILLON
0 siblings, 0 replies; 30+ messages in thread
From: Boris BREZILLON @ 2014-05-27 9:38 UTC (permalink / raw)
To: linux-arm-kernel
PMIC devices often provide several regulators, and these regulators are
all using the same regmap and are all attached to the same device (the
PMIC device).
Add helper functions (both simple and resource managed versions) to
register and unregister such kind of regulator set.
This implementation handles self dependency issues where one regulator
provided the PMIC depends on another one provided by the same PMIC.
Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
---
drivers/regulator/core.c | 106 +++++++++++++++++++++++++++++++++++++++
drivers/regulator/devres.c | 68 +++++++++++++++++++++++++
include/linux/regulator/driver.h | 51 +++++++++++++++++++
3 files changed, 225 insertions(+)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 9a09f3c..7703853 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3595,6 +3595,112 @@ void regulator_unregister(struct regulator_dev *rdev)
EXPORT_SYMBOL_GPL(regulator_unregister);
/**
+ * regulator_set_register - register a set of regulators
+ * @config: runtime configuration for regulator set
+ *
+ * Called by regulator drivers to register a set of regulators.
+ * Returns a valid pointer to struct regulator_set on success
+ * or an ERR_PTR() on error.
+ */
+struct regulator_set *
+regulator_set_register(const struct regulator_set_config *config)
+{
+ struct regulator_set *rset;
+ int prev_nregistered = -1;
+ int nregistered = 0;
+ int ret;
+ int i;
+
+ if (!config->descs || !config->nregulators)
+ return ERR_PTR(-EINVAL);
+
+ rset = kzalloc(sizeof(*rset) +
+ (config->nregulators * sizeof(struct regulator_dev *)),
+ GFP_KERNEL);
+ if (!rset)
+ return ERR_PTR(-ENOMEM);
+
+ rset->nregulators = config->nregulators;
+
+ while (nregistered < config->nregulators &&
+ prev_nregistered < nregistered) {
+
+ prev_nregistered = nregistered;
+
+ for (i = 0; i < config->nregulators; i++) {
+ struct regulator_config conf;
+ struct regulator_dev *regulator;
+
+ if (rset->regulators[i])
+ continue;
+
+ memset(&conf, 0, sizeof(conf));
+
+ conf.dev = config->dev;
+ conf.regmap = config->regmap;
+ conf.driver_data = config->driver_data;
+ if (config->matches && config->matches[i].init_data)
+ conf.init_data = config->matches[i].init_data;
+ else if (config->init_data)
+ conf.init_data = config->init_data[i];
+
+ if (config->matches)
+ conf.of_node = config->matches[i].of_node;
+
+ regulator = regulator_register(&config->descs[i],
+ &conf);
+ if (IS_ERR(regulator)) {
+ ret = PTR_ERR(regulator);
+ if (ret == -EPROBE_DEFER)
+ continue;
+
+ goto err;
+ }
+
+ rset->regulators[i] = regulator;
+ nregistered++;
+ }
+ }
+
+ if (nregistered < config->nregulators) {
+ ret = -EPROBE_DEFER;
+ goto err;
+ }
+
+ return rset;
+
+err:
+ for (i = 0; i < config->nregulators; i++) {
+ if (rset->regulators[i])
+ regulator_unregister(rset->regulators[i]);
+ }
+
+ kfree(rset);
+
+ return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(regulator_set_register);
+
+/**
+ * regulator_set_unregister - unregister regulator set
+ * @rset: regulator set to unregister
+ *
+ * Called by regulator drivers to unregister a regulator set.
+ */
+void regulator_set_unregister(struct regulator_set *rset)
+{
+ int i;
+
+ for (i = 0; i < rset->nregulators; i++) {
+ if (rset->regulators[i])
+ regulator_unregister(rset->regulators[i]);
+ }
+
+ kfree(rset);
+}
+EXPORT_SYMBOL_GPL(regulator_set_unregister);
+
+/**
* regulator_suspend_prepare - prepare regulators for system wide suspend
* @state: system suspend state
*
diff --git a/drivers/regulator/devres.c b/drivers/regulator/devres.c
index f44818b..a34f34c 100644
--- a/drivers/regulator/devres.c
+++ b/drivers/regulator/devres.c
@@ -251,6 +251,74 @@ void devm_regulator_unregister(struct device *dev, struct regulator_dev *rdev)
}
EXPORT_SYMBOL_GPL(devm_regulator_unregister);
+static void devm_rset_release(struct device *dev, void *res)
+{
+ regulator_set_unregister(*(struct regulator_set **)res);
+}
+
+/**
+ * devm_regulator_set_register - Resource managed regulator_set_register()
+ * @dev: device registering the regulator set
+ * @config: runtime configuration for regulator set
+ *
+ * Called by regulator drivers to register a set of regulators. Returns a
+ * valid pointer to struct regulator_set on success or an ERR_PTR() on
+ * error. The regulator will automatically be released when the device
+ * is unbound.
+ */
+struct regulator_set *
+devm_regulator_set_register(struct device *dev,
+ const struct regulator_set_config *config)
+{
+ struct regulator_set **ptr, *rset;
+
+ ptr = devres_alloc(devm_rset_release, sizeof(*ptr),
+ GFP_KERNEL);
+ if (!ptr)
+ return ERR_PTR(-ENOMEM);
+
+ rset = regulator_set_register(config);
+ if (!IS_ERR(rset)) {
+ *ptr = rset;
+ devres_add(dev, ptr);
+ } else {
+ devres_free(ptr);
+ }
+
+ return rset;
+}
+EXPORT_SYMBOL_GPL(devm_regulator_set_register);
+
+static int devm_rset_match(struct device *dev, void *res, void *data)
+{
+ struct regulator_set **r = res;
+ if (!r || !*r) {
+ WARN_ON(!r || !*r);
+ return 0;
+ }
+ return *r == data;
+}
+
+/**
+ * devm_regulator_set_unregister - Resource managed regulator_set_unregister()
+ * @dev: device unregistering the regulator set
+ * @rset: regulator set to release
+ *
+ * Unregister a regulator set registered with devm_regulator_set_register().
+ * Normally this function will not need to be called and the resource
+ * management code will ensure that the resource is freed.
+ */
+void devm_regulator_set_unregister(struct device *dev,
+ struct regulator_set *rset)
+{
+ int rc;
+
+ rc = devres_release(dev, devm_rdev_release, devm_rset_match, rset);
+ if (rc != 0)
+ WARN_ON(rc);
+}
+EXPORT_SYMBOL_GPL(devm_regulator_unregister);
+
struct regulator_supply_alias_match {
struct device *dev;
const char *id;
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index bbe03a1..b683f68 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -350,6 +350,48 @@ struct regulator_dev {
unsigned int ena_gpio_state:1;
};
+/**
+ * struct regulator_set_config - Dynamic regulator set descriptor
+ *
+ * This structure describe a regulator and each regulator in this set
+ * will be registered using informations passed through regulator_desc
+ * and of_regulator_match tables.
+ *
+ * @dev: struct device providing the regulator set.
+ * @driver_data: private driver data for this regulator set.
+ * @regmap: regmap to use for core regmap helpers if dev_get_regulator() is
+ * insufficient.
+ * @descs: table of regulator descriptors.
+ * @matches: table of DT regulator descriptors. This table should have
+ * been filled by the of_regulator_match function.
+ * @init_data: a table of init_data in case matches is NULL or the init_data
+ * of the match entry is NULL.
+ * @nregulators: number of regulators in the regulator set.
+ */
+struct regulator_set_config {
+ struct device *dev;
+ void *driver_data;
+ struct regmap *regmap;
+ const struct regulator_desc *descs;
+ const struct of_regulator_match *matches;
+ const struct regulator_init_data **init_data;
+ int nregulators;
+};
+
+/*
+ * struct regulator_set - Regulator set
+ *
+ * This structure stores a set of regulator devices provided by a single
+ * device (e.g. a PMIC device).
+ *
+ * @nregulators: number of regulators in the set
+ * @regulators: regulators table
+ */
+struct regulator_set {
+ int nregulators;
+ struct regulator_dev *regulators[0];
+};
+
struct regulator_dev *
regulator_register(const struct regulator_desc *regulator_desc,
const struct regulator_config *config);
@@ -360,6 +402,15 @@ devm_regulator_register(struct device *dev,
void regulator_unregister(struct regulator_dev *rdev);
void devm_regulator_unregister(struct device *dev, struct regulator_dev *rdev);
+struct regulator_set *
+regulator_set_register(const struct regulator_set_config *config);
+struct regulator_set *
+devm_regulator_set_register(struct device *dev,
+ const struct regulator_set_config *config);
+void regulator_set_unregister(struct regulator_set *rset);
+void devm_regulator_set_unregister(struct device *dev,
+ struct regulator_set *rset);
+
int regulator_notifier_call_chain(struct regulator_dev *rdev,
unsigned long event, void *data);
--
1.8.3.2
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH v3 5/6] regulator: add support for regulator set registration
2014-05-27 9:38 ` Boris BREZILLON
@ 2014-06-10 13:40 ` Boris BREZILLON
-1 siblings, 0 replies; 30+ messages in thread
From: Boris BREZILLON @ 2014-06-10 13:40 UTC (permalink / raw)
To: Samuel Ortiz, Lee Jones, Liam Girdwood, Mark Brown
Cc: Maxime Ripard, Carlo Caione, Shuge, kevin, linux-kernel,
linux-arm-kernel, dev
Hello Mark,
Did you have time to take a look at this patch ?
I'd like to post a new version of this series addressing Lee's comments,
but it would be great to have your feedback on this patch before posting
a new version.
Best Regards,
Boris
On 27/05/2014 11:38, Boris BREZILLON wrote:
> PMIC devices often provide several regulators, and these regulators are
> all using the same regmap and are all attached to the same device (the
> PMIC device).
>
> Add helper functions (both simple and resource managed versions) to
> register and unregister such kind of regulator set.
>
> This implementation handles self dependency issues where one regulator
> provided the PMIC depends on another one provided by the same PMIC.
>
> Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> ---
> drivers/regulator/core.c | 106 +++++++++++++++++++++++++++++++++++++++
> drivers/regulator/devres.c | 68 +++++++++++++++++++++++++
> include/linux/regulator/driver.h | 51 +++++++++++++++++++
> 3 files changed, 225 insertions(+)
>
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index 9a09f3c..7703853 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -3595,6 +3595,112 @@ void regulator_unregister(struct regulator_dev *rdev)
> EXPORT_SYMBOL_GPL(regulator_unregister);
>
> /**
> + * regulator_set_register - register a set of regulators
> + * @config: runtime configuration for regulator set
> + *
> + * Called by regulator drivers to register a set of regulators.
> + * Returns a valid pointer to struct regulator_set on success
> + * or an ERR_PTR() on error.
> + */
> +struct regulator_set *
> +regulator_set_register(const struct regulator_set_config *config)
> +{
> + struct regulator_set *rset;
> + int prev_nregistered = -1;
> + int nregistered = 0;
> + int ret;
> + int i;
> +
> + if (!config->descs || !config->nregulators)
> + return ERR_PTR(-EINVAL);
> +
> + rset = kzalloc(sizeof(*rset) +
> + (config->nregulators * sizeof(struct regulator_dev *)),
> + GFP_KERNEL);
> + if (!rset)
> + return ERR_PTR(-ENOMEM);
> +
> + rset->nregulators = config->nregulators;
> +
> + while (nregistered < config->nregulators &&
> + prev_nregistered < nregistered) {
> +
> + prev_nregistered = nregistered;
> +
> + for (i = 0; i < config->nregulators; i++) {
> + struct regulator_config conf;
> + struct regulator_dev *regulator;
> +
> + if (rset->regulators[i])
> + continue;
> +
> + memset(&conf, 0, sizeof(conf));
> +
> + conf.dev = config->dev;
> + conf.regmap = config->regmap;
> + conf.driver_data = config->driver_data;
> + if (config->matches && config->matches[i].init_data)
> + conf.init_data = config->matches[i].init_data;
> + else if (config->init_data)
> + conf.init_data = config->init_data[i];
> +
> + if (config->matches)
> + conf.of_node = config->matches[i].of_node;
> +
> + regulator = regulator_register(&config->descs[i],
> + &conf);
> + if (IS_ERR(regulator)) {
> + ret = PTR_ERR(regulator);
> + if (ret == -EPROBE_DEFER)
> + continue;
> +
> + goto err;
> + }
> +
> + rset->regulators[i] = regulator;
> + nregistered++;
> + }
> + }
> +
> + if (nregistered < config->nregulators) {
> + ret = -EPROBE_DEFER;
> + goto err;
> + }
> +
> + return rset;
> +
> +err:
> + for (i = 0; i < config->nregulators; i++) {
> + if (rset->regulators[i])
> + regulator_unregister(rset->regulators[i]);
> + }
> +
> + kfree(rset);
> +
> + return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_GPL(regulator_set_register);
> +
> +/**
> + * regulator_set_unregister - unregister regulator set
> + * @rset: regulator set to unregister
> + *
> + * Called by regulator drivers to unregister a regulator set.
> + */
> +void regulator_set_unregister(struct regulator_set *rset)
> +{
> + int i;
> +
> + for (i = 0; i < rset->nregulators; i++) {
> + if (rset->regulators[i])
> + regulator_unregister(rset->regulators[i]);
> + }
> +
> + kfree(rset);
> +}
> +EXPORT_SYMBOL_GPL(regulator_set_unregister);
> +
> +/**
> * regulator_suspend_prepare - prepare regulators for system wide suspend
> * @state: system suspend state
> *
> diff --git a/drivers/regulator/devres.c b/drivers/regulator/devres.c
> index f44818b..a34f34c 100644
> --- a/drivers/regulator/devres.c
> +++ b/drivers/regulator/devres.c
> @@ -251,6 +251,74 @@ void devm_regulator_unregister(struct device *dev, struct regulator_dev *rdev)
> }
> EXPORT_SYMBOL_GPL(devm_regulator_unregister);
>
> +static void devm_rset_release(struct device *dev, void *res)
> +{
> + regulator_set_unregister(*(struct regulator_set **)res);
> +}
> +
> +/**
> + * devm_regulator_set_register - Resource managed regulator_set_register()
> + * @dev: device registering the regulator set
> + * @config: runtime configuration for regulator set
> + *
> + * Called by regulator drivers to register a set of regulators. Returns a
> + * valid pointer to struct regulator_set on success or an ERR_PTR() on
> + * error. The regulator will automatically be released when the device
> + * is unbound.
> + */
> +struct regulator_set *
> +devm_regulator_set_register(struct device *dev,
> + const struct regulator_set_config *config)
> +{
> + struct regulator_set **ptr, *rset;
> +
> + ptr = devres_alloc(devm_rset_release, sizeof(*ptr),
> + GFP_KERNEL);
> + if (!ptr)
> + return ERR_PTR(-ENOMEM);
> +
> + rset = regulator_set_register(config);
> + if (!IS_ERR(rset)) {
> + *ptr = rset;
> + devres_add(dev, ptr);
> + } else {
> + devres_free(ptr);
> + }
> +
> + return rset;
> +}
> +EXPORT_SYMBOL_GPL(devm_regulator_set_register);
> +
> +static int devm_rset_match(struct device *dev, void *res, void *data)
> +{
> + struct regulator_set **r = res;
> + if (!r || !*r) {
> + WARN_ON(!r || !*r);
> + return 0;
> + }
> + return *r == data;
> +}
> +
> +/**
> + * devm_regulator_set_unregister - Resource managed regulator_set_unregister()
> + * @dev: device unregistering the regulator set
> + * @rset: regulator set to release
> + *
> + * Unregister a regulator set registered with devm_regulator_set_register().
> + * Normally this function will not need to be called and the resource
> + * management code will ensure that the resource is freed.
> + */
> +void devm_regulator_set_unregister(struct device *dev,
> + struct regulator_set *rset)
> +{
> + int rc;
> +
> + rc = devres_release(dev, devm_rdev_release, devm_rset_match, rset);
> + if (rc != 0)
> + WARN_ON(rc);
> +}
> +EXPORT_SYMBOL_GPL(devm_regulator_unregister);
> +
> struct regulator_supply_alias_match {
> struct device *dev;
> const char *id;
> diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
> index bbe03a1..b683f68 100644
> --- a/include/linux/regulator/driver.h
> +++ b/include/linux/regulator/driver.h
> @@ -350,6 +350,48 @@ struct regulator_dev {
> unsigned int ena_gpio_state:1;
> };
>
> +/**
> + * struct regulator_set_config - Dynamic regulator set descriptor
> + *
> + * This structure describe a regulator and each regulator in this set
> + * will be registered using informations passed through regulator_desc
> + * and of_regulator_match tables.
> + *
> + * @dev: struct device providing the regulator set.
> + * @driver_data: private driver data for this regulator set.
> + * @regmap: regmap to use for core regmap helpers if dev_get_regulator() is
> + * insufficient.
> + * @descs: table of regulator descriptors.
> + * @matches: table of DT regulator descriptors. This table should have
> + * been filled by the of_regulator_match function.
> + * @init_data: a table of init_data in case matches is NULL or the init_data
> + * of the match entry is NULL.
> + * @nregulators: number of regulators in the regulator set.
> + */
> +struct regulator_set_config {
> + struct device *dev;
> + void *driver_data;
> + struct regmap *regmap;
> + const struct regulator_desc *descs;
> + const struct of_regulator_match *matches;
> + const struct regulator_init_data **init_data;
> + int nregulators;
> +};
> +
> +/*
> + * struct regulator_set - Regulator set
> + *
> + * This structure stores a set of regulator devices provided by a single
> + * device (e.g. a PMIC device).
> + *
> + * @nregulators: number of regulators in the set
> + * @regulators: regulators table
> + */
> +struct regulator_set {
> + int nregulators;
> + struct regulator_dev *regulators[0];
> +};
> +
> struct regulator_dev *
> regulator_register(const struct regulator_desc *regulator_desc,
> const struct regulator_config *config);
> @@ -360,6 +402,15 @@ devm_regulator_register(struct device *dev,
> void regulator_unregister(struct regulator_dev *rdev);
> void devm_regulator_unregister(struct device *dev, struct regulator_dev *rdev);
>
> +struct regulator_set *
> +regulator_set_register(const struct regulator_set_config *config);
> +struct regulator_set *
> +devm_regulator_set_register(struct device *dev,
> + const struct regulator_set_config *config);
> +void regulator_set_unregister(struct regulator_set *rset);
> +void devm_regulator_set_unregister(struct device *dev,
> + struct regulator_set *rset);
> +
> int regulator_notifier_call_chain(struct regulator_dev *rdev,
> unsigned long event, void *data);
>
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCH v3 5/6] regulator: add support for regulator set registration
@ 2014-06-10 13:40 ` Boris BREZILLON
0 siblings, 0 replies; 30+ messages in thread
From: Boris BREZILLON @ 2014-06-10 13:40 UTC (permalink / raw)
To: linux-arm-kernel
Hello Mark,
Did you have time to take a look at this patch ?
I'd like to post a new version of this series addressing Lee's comments,
but it would be great to have your feedback on this patch before posting
a new version.
Best Regards,
Boris
On 27/05/2014 11:38, Boris BREZILLON wrote:
> PMIC devices often provide several regulators, and these regulators are
> all using the same regmap and are all attached to the same device (the
> PMIC device).
>
> Add helper functions (both simple and resource managed versions) to
> register and unregister such kind of regulator set.
>
> This implementation handles self dependency issues where one regulator
> provided the PMIC depends on another one provided by the same PMIC.
>
> Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> ---
> drivers/regulator/core.c | 106 +++++++++++++++++++++++++++++++++++++++
> drivers/regulator/devres.c | 68 +++++++++++++++++++++++++
> include/linux/regulator/driver.h | 51 +++++++++++++++++++
> 3 files changed, 225 insertions(+)
>
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index 9a09f3c..7703853 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -3595,6 +3595,112 @@ void regulator_unregister(struct regulator_dev *rdev)
> EXPORT_SYMBOL_GPL(regulator_unregister);
>
> /**
> + * regulator_set_register - register a set of regulators
> + * @config: runtime configuration for regulator set
> + *
> + * Called by regulator drivers to register a set of regulators.
> + * Returns a valid pointer to struct regulator_set on success
> + * or an ERR_PTR() on error.
> + */
> +struct regulator_set *
> +regulator_set_register(const struct regulator_set_config *config)
> +{
> + struct regulator_set *rset;
> + int prev_nregistered = -1;
> + int nregistered = 0;
> + int ret;
> + int i;
> +
> + if (!config->descs || !config->nregulators)
> + return ERR_PTR(-EINVAL);
> +
> + rset = kzalloc(sizeof(*rset) +
> + (config->nregulators * sizeof(struct regulator_dev *)),
> + GFP_KERNEL);
> + if (!rset)
> + return ERR_PTR(-ENOMEM);
> +
> + rset->nregulators = config->nregulators;
> +
> + while (nregistered < config->nregulators &&
> + prev_nregistered < nregistered) {
> +
> + prev_nregistered = nregistered;
> +
> + for (i = 0; i < config->nregulators; i++) {
> + struct regulator_config conf;
> + struct regulator_dev *regulator;
> +
> + if (rset->regulators[i])
> + continue;
> +
> + memset(&conf, 0, sizeof(conf));
> +
> + conf.dev = config->dev;
> + conf.regmap = config->regmap;
> + conf.driver_data = config->driver_data;
> + if (config->matches && config->matches[i].init_data)
> + conf.init_data = config->matches[i].init_data;
> + else if (config->init_data)
> + conf.init_data = config->init_data[i];
> +
> + if (config->matches)
> + conf.of_node = config->matches[i].of_node;
> +
> + regulator = regulator_register(&config->descs[i],
> + &conf);
> + if (IS_ERR(regulator)) {
> + ret = PTR_ERR(regulator);
> + if (ret == -EPROBE_DEFER)
> + continue;
> +
> + goto err;
> + }
> +
> + rset->regulators[i] = regulator;
> + nregistered++;
> + }
> + }
> +
> + if (nregistered < config->nregulators) {
> + ret = -EPROBE_DEFER;
> + goto err;
> + }
> +
> + return rset;
> +
> +err:
> + for (i = 0; i < config->nregulators; i++) {
> + if (rset->regulators[i])
> + regulator_unregister(rset->regulators[i]);
> + }
> +
> + kfree(rset);
> +
> + return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_GPL(regulator_set_register);
> +
> +/**
> + * regulator_set_unregister - unregister regulator set
> + * @rset: regulator set to unregister
> + *
> + * Called by regulator drivers to unregister a regulator set.
> + */
> +void regulator_set_unregister(struct regulator_set *rset)
> +{
> + int i;
> +
> + for (i = 0; i < rset->nregulators; i++) {
> + if (rset->regulators[i])
> + regulator_unregister(rset->regulators[i]);
> + }
> +
> + kfree(rset);
> +}
> +EXPORT_SYMBOL_GPL(regulator_set_unregister);
> +
> +/**
> * regulator_suspend_prepare - prepare regulators for system wide suspend
> * @state: system suspend state
> *
> diff --git a/drivers/regulator/devres.c b/drivers/regulator/devres.c
> index f44818b..a34f34c 100644
> --- a/drivers/regulator/devres.c
> +++ b/drivers/regulator/devres.c
> @@ -251,6 +251,74 @@ void devm_regulator_unregister(struct device *dev, struct regulator_dev *rdev)
> }
> EXPORT_SYMBOL_GPL(devm_regulator_unregister);
>
> +static void devm_rset_release(struct device *dev, void *res)
> +{
> + regulator_set_unregister(*(struct regulator_set **)res);
> +}
> +
> +/**
> + * devm_regulator_set_register - Resource managed regulator_set_register()
> + * @dev: device registering the regulator set
> + * @config: runtime configuration for regulator set
> + *
> + * Called by regulator drivers to register a set of regulators. Returns a
> + * valid pointer to struct regulator_set on success or an ERR_PTR() on
> + * error. The regulator will automatically be released when the device
> + * is unbound.
> + */
> +struct regulator_set *
> +devm_regulator_set_register(struct device *dev,
> + const struct regulator_set_config *config)
> +{
> + struct regulator_set **ptr, *rset;
> +
> + ptr = devres_alloc(devm_rset_release, sizeof(*ptr),
> + GFP_KERNEL);
> + if (!ptr)
> + return ERR_PTR(-ENOMEM);
> +
> + rset = regulator_set_register(config);
> + if (!IS_ERR(rset)) {
> + *ptr = rset;
> + devres_add(dev, ptr);
> + } else {
> + devres_free(ptr);
> + }
> +
> + return rset;
> +}
> +EXPORT_SYMBOL_GPL(devm_regulator_set_register);
> +
> +static int devm_rset_match(struct device *dev, void *res, void *data)
> +{
> + struct regulator_set **r = res;
> + if (!r || !*r) {
> + WARN_ON(!r || !*r);
> + return 0;
> + }
> + return *r == data;
> +}
> +
> +/**
> + * devm_regulator_set_unregister - Resource managed regulator_set_unregister()
> + * @dev: device unregistering the regulator set
> + * @rset: regulator set to release
> + *
> + * Unregister a regulator set registered with devm_regulator_set_register().
> + * Normally this function will not need to be called and the resource
> + * management code will ensure that the resource is freed.
> + */
> +void devm_regulator_set_unregister(struct device *dev,
> + struct regulator_set *rset)
> +{
> + int rc;
> +
> + rc = devres_release(dev, devm_rdev_release, devm_rset_match, rset);
> + if (rc != 0)
> + WARN_ON(rc);
> +}
> +EXPORT_SYMBOL_GPL(devm_regulator_unregister);
> +
> struct regulator_supply_alias_match {
> struct device *dev;
> const char *id;
> diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
> index bbe03a1..b683f68 100644
> --- a/include/linux/regulator/driver.h
> +++ b/include/linux/regulator/driver.h
> @@ -350,6 +350,48 @@ struct regulator_dev {
> unsigned int ena_gpio_state:1;
> };
>
> +/**
> + * struct regulator_set_config - Dynamic regulator set descriptor
> + *
> + * This structure describe a regulator and each regulator in this set
> + * will be registered using informations passed through regulator_desc
> + * and of_regulator_match tables.
> + *
> + * @dev: struct device providing the regulator set.
> + * @driver_data: private driver data for this regulator set.
> + * @regmap: regmap to use for core regmap helpers if dev_get_regulator() is
> + * insufficient.
> + * @descs: table of regulator descriptors.
> + * @matches: table of DT regulator descriptors. This table should have
> + * been filled by the of_regulator_match function.
> + * @init_data: a table of init_data in case matches is NULL or the init_data
> + * of the match entry is NULL.
> + * @nregulators: number of regulators in the regulator set.
> + */
> +struct regulator_set_config {
> + struct device *dev;
> + void *driver_data;
> + struct regmap *regmap;
> + const struct regulator_desc *descs;
> + const struct of_regulator_match *matches;
> + const struct regulator_init_data **init_data;
> + int nregulators;
> +};
> +
> +/*
> + * struct regulator_set - Regulator set
> + *
> + * This structure stores a set of regulator devices provided by a single
> + * device (e.g. a PMIC device).
> + *
> + * @nregulators: number of regulators in the set
> + * @regulators: regulators table
> + */
> +struct regulator_set {
> + int nregulators;
> + struct regulator_dev *regulators[0];
> +};
> +
> struct regulator_dev *
> regulator_register(const struct regulator_desc *regulator_desc,
> const struct regulator_config *config);
> @@ -360,6 +402,15 @@ devm_regulator_register(struct device *dev,
> void regulator_unregister(struct regulator_dev *rdev);
> void devm_regulator_unregister(struct device *dev, struct regulator_dev *rdev);
>
> +struct regulator_set *
> +regulator_set_register(const struct regulator_set_config *config);
> +struct regulator_set *
> +devm_regulator_set_register(struct device *dev,
> + const struct regulator_set_config *config);
> +void regulator_set_unregister(struct regulator_set *rset);
> +void devm_regulator_set_unregister(struct device *dev,
> + struct regulator_set *rset);
> +
> int regulator_notifier_call_chain(struct regulator_dev *rdev,
> unsigned long event, void *data);
>
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v3 5/6] regulator: add support for regulator set registration
2014-06-10 13:40 ` Boris BREZILLON
@ 2014-06-16 8:08 ` Lee Jones
-1 siblings, 0 replies; 30+ messages in thread
From: Lee Jones @ 2014-06-16 8:08 UTC (permalink / raw)
To: Boris BREZILLON
Cc: Samuel Ortiz, Liam Girdwood, Mark Brown, Maxime Ripard,
Carlo Caione, Shuge, kevin, linux-kernel, linux-arm-kernel, dev
> Hello Mark,
>
> Did you have time to take a look at this patch ?
>
> I'd like to post a new version of this series addressing Lee's comments,
> but it would be great to have your feedback on this patch before posting
> a new version.
I wouldn't do that, as you're likely to upset him.
Just fix what you know and resend.
Mark will subsequently review I'm sure.
> Best Regards,
>
> Boris
>
> On 27/05/2014 11:38, Boris BREZILLON wrote:
> > PMIC devices often provide several regulators, and these regulators are
> > all using the same regmap and are all attached to the same device (the
> > PMIC device).
> >
> > Add helper functions (both simple and resource managed versions) to
> > register and unregister such kind of regulator set.
> >
> > This implementation handles self dependency issues where one regulator
> > provided the PMIC depends on another one provided by the same PMIC.
> >
> > Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> > ---
> > drivers/regulator/core.c | 106 +++++++++++++++++++++++++++++++++++++++
> > drivers/regulator/devres.c | 68 +++++++++++++++++++++++++
> > include/linux/regulator/driver.h | 51 +++++++++++++++++++
> > 3 files changed, 225 insertions(+)
> >
> > diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> > index 9a09f3c..7703853 100644
> > --- a/drivers/regulator/core.c
> > +++ b/drivers/regulator/core.c
> > @@ -3595,6 +3595,112 @@ void regulator_unregister(struct regulator_dev *rdev)
> > EXPORT_SYMBOL_GPL(regulator_unregister);
> >
> > /**
> > + * regulator_set_register - register a set of regulators
> > + * @config: runtime configuration for regulator set
> > + *
> > + * Called by regulator drivers to register a set of regulators.
> > + * Returns a valid pointer to struct regulator_set on success
> > + * or an ERR_PTR() on error.
> > + */
> > +struct regulator_set *
> > +regulator_set_register(const struct regulator_set_config *config)
> > +{
> > + struct regulator_set *rset;
> > + int prev_nregistered = -1;
> > + int nregistered = 0;
> > + int ret;
> > + int i;
> > +
> > + if (!config->descs || !config->nregulators)
> > + return ERR_PTR(-EINVAL);
> > +
> > + rset = kzalloc(sizeof(*rset) +
> > + (config->nregulators * sizeof(struct regulator_dev *)),
> > + GFP_KERNEL);
> > + if (!rset)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + rset->nregulators = config->nregulators;
> > +
> > + while (nregistered < config->nregulators &&
> > + prev_nregistered < nregistered) {
> > +
> > + prev_nregistered = nregistered;
> > +
> > + for (i = 0; i < config->nregulators; i++) {
> > + struct regulator_config conf;
> > + struct regulator_dev *regulator;
> > +
> > + if (rset->regulators[i])
> > + continue;
> > +
> > + memset(&conf, 0, sizeof(conf));
> > +
> > + conf.dev = config->dev;
> > + conf.regmap = config->regmap;
> > + conf.driver_data = config->driver_data;
> > + if (config->matches && config->matches[i].init_data)
> > + conf.init_data = config->matches[i].init_data;
> > + else if (config->init_data)
> > + conf.init_data = config->init_data[i];
> > +
> > + if (config->matches)
> > + conf.of_node = config->matches[i].of_node;
> > +
> > + regulator = regulator_register(&config->descs[i],
> > + &conf);
> > + if (IS_ERR(regulator)) {
> > + ret = PTR_ERR(regulator);
> > + if (ret == -EPROBE_DEFER)
> > + continue;
> > +
> > + goto err;
> > + }
> > +
> > + rset->regulators[i] = regulator;
> > + nregistered++;
> > + }
> > + }
> > +
> > + if (nregistered < config->nregulators) {
> > + ret = -EPROBE_DEFER;
> > + goto err;
> > + }
> > +
> > + return rset;
> > +
> > +err:
> > + for (i = 0; i < config->nregulators; i++) {
> > + if (rset->regulators[i])
> > + regulator_unregister(rset->regulators[i]);
> > + }
> > +
> > + kfree(rset);
> > +
> > + return ERR_PTR(ret);
> > +}
> > +EXPORT_SYMBOL_GPL(regulator_set_register);
> > +
> > +/**
> > + * regulator_set_unregister - unregister regulator set
> > + * @rset: regulator set to unregister
> > + *
> > + * Called by regulator drivers to unregister a regulator set.
> > + */
> > +void regulator_set_unregister(struct regulator_set *rset)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < rset->nregulators; i++) {
> > + if (rset->regulators[i])
> > + regulator_unregister(rset->regulators[i]);
> > + }
> > +
> > + kfree(rset);
> > +}
> > +EXPORT_SYMBOL_GPL(regulator_set_unregister);
> > +
> > +/**
> > * regulator_suspend_prepare - prepare regulators for system wide suspend
> > * @state: system suspend state
> > *
> > diff --git a/drivers/regulator/devres.c b/drivers/regulator/devres.c
> > index f44818b..a34f34c 100644
> > --- a/drivers/regulator/devres.c
> > +++ b/drivers/regulator/devres.c
> > @@ -251,6 +251,74 @@ void devm_regulator_unregister(struct device *dev, struct regulator_dev *rdev)
> > }
> > EXPORT_SYMBOL_GPL(devm_regulator_unregister);
> >
> > +static void devm_rset_release(struct device *dev, void *res)
> > +{
> > + regulator_set_unregister(*(struct regulator_set **)res);
> > +}
> > +
> > +/**
> > + * devm_regulator_set_register - Resource managed regulator_set_register()
> > + * @dev: device registering the regulator set
> > + * @config: runtime configuration for regulator set
> > + *
> > + * Called by regulator drivers to register a set of regulators. Returns a
> > + * valid pointer to struct regulator_set on success or an ERR_PTR() on
> > + * error. The regulator will automatically be released when the device
> > + * is unbound.
> > + */
> > +struct regulator_set *
> > +devm_regulator_set_register(struct device *dev,
> > + const struct regulator_set_config *config)
> > +{
> > + struct regulator_set **ptr, *rset;
> > +
> > + ptr = devres_alloc(devm_rset_release, sizeof(*ptr),
> > + GFP_KERNEL);
> > + if (!ptr)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + rset = regulator_set_register(config);
> > + if (!IS_ERR(rset)) {
> > + *ptr = rset;
> > + devres_add(dev, ptr);
> > + } else {
> > + devres_free(ptr);
> > + }
> > +
> > + return rset;
> > +}
> > +EXPORT_SYMBOL_GPL(devm_regulator_set_register);
> > +
> > +static int devm_rset_match(struct device *dev, void *res, void *data)
> > +{
> > + struct regulator_set **r = res;
> > + if (!r || !*r) {
> > + WARN_ON(!r || !*r);
> > + return 0;
> > + }
> > + return *r == data;
> > +}
> > +
> > +/**
> > + * devm_regulator_set_unregister - Resource managed regulator_set_unregister()
> > + * @dev: device unregistering the regulator set
> > + * @rset: regulator set to release
> > + *
> > + * Unregister a regulator set registered with devm_regulator_set_register().
> > + * Normally this function will not need to be called and the resource
> > + * management code will ensure that the resource is freed.
> > + */
> > +void devm_regulator_set_unregister(struct device *dev,
> > + struct regulator_set *rset)
> > +{
> > + int rc;
> > +
> > + rc = devres_release(dev, devm_rdev_release, devm_rset_match, rset);
> > + if (rc != 0)
> > + WARN_ON(rc);
> > +}
> > +EXPORT_SYMBOL_GPL(devm_regulator_unregister);
> > +
> > struct regulator_supply_alias_match {
> > struct device *dev;
> > const char *id;
> > diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
> > index bbe03a1..b683f68 100644
> > --- a/include/linux/regulator/driver.h
> > +++ b/include/linux/regulator/driver.h
> > @@ -350,6 +350,48 @@ struct regulator_dev {
> > unsigned int ena_gpio_state:1;
> > };
> >
> > +/**
> > + * struct regulator_set_config - Dynamic regulator set descriptor
> > + *
> > + * This structure describe a regulator and each regulator in this set
> > + * will be registered using informations passed through regulator_desc
> > + * and of_regulator_match tables.
> > + *
> > + * @dev: struct device providing the regulator set.
> > + * @driver_data: private driver data for this regulator set.
> > + * @regmap: regmap to use for core regmap helpers if dev_get_regulator() is
> > + * insufficient.
> > + * @descs: table of regulator descriptors.
> > + * @matches: table of DT regulator descriptors. This table should have
> > + * been filled by the of_regulator_match function.
> > + * @init_data: a table of init_data in case matches is NULL or the init_data
> > + * of the match entry is NULL.
> > + * @nregulators: number of regulators in the regulator set.
> > + */
> > +struct regulator_set_config {
> > + struct device *dev;
> > + void *driver_data;
> > + struct regmap *regmap;
> > + const struct regulator_desc *descs;
> > + const struct of_regulator_match *matches;
> > + const struct regulator_init_data **init_data;
> > + int nregulators;
> > +};
> > +
> > +/*
> > + * struct regulator_set - Regulator set
> > + *
> > + * This structure stores a set of regulator devices provided by a single
> > + * device (e.g. a PMIC device).
> > + *
> > + * @nregulators: number of regulators in the set
> > + * @regulators: regulators table
> > + */
> > +struct regulator_set {
> > + int nregulators;
> > + struct regulator_dev *regulators[0];
> > +};
> > +
> > struct regulator_dev *
> > regulator_register(const struct regulator_desc *regulator_desc,
> > const struct regulator_config *config);
> > @@ -360,6 +402,15 @@ devm_regulator_register(struct device *dev,
> > void regulator_unregister(struct regulator_dev *rdev);
> > void devm_regulator_unregister(struct device *dev, struct regulator_dev *rdev);
> >
> > +struct regulator_set *
> > +regulator_set_register(const struct regulator_set_config *config);
> > +struct regulator_set *
> > +devm_regulator_set_register(struct device *dev,
> > + const struct regulator_set_config *config);
> > +void regulator_set_unregister(struct regulator_set *rset);
> > +void devm_regulator_set_unregister(struct device *dev,
> > + struct regulator_set *rset);
> > +
> > int regulator_notifier_call_chain(struct regulator_dev *rdev,
> > unsigned long event, void *data);
> >
>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCH v3 5/6] regulator: add support for regulator set registration
@ 2014-06-16 8:08 ` Lee Jones
0 siblings, 0 replies; 30+ messages in thread
From: Lee Jones @ 2014-06-16 8:08 UTC (permalink / raw)
To: linux-arm-kernel
> Hello Mark,
>
> Did you have time to take a look at this patch ?
>
> I'd like to post a new version of this series addressing Lee's comments,
> but it would be great to have your feedback on this patch before posting
> a new version.
I wouldn't do that, as you're likely to upset him.
Just fix what you know and resend.
Mark will subsequently review I'm sure.
> Best Regards,
>
> Boris
>
> On 27/05/2014 11:38, Boris BREZILLON wrote:
> > PMIC devices often provide several regulators, and these regulators are
> > all using the same regmap and are all attached to the same device (the
> > PMIC device).
> >
> > Add helper functions (both simple and resource managed versions) to
> > register and unregister such kind of regulator set.
> >
> > This implementation handles self dependency issues where one regulator
> > provided the PMIC depends on another one provided by the same PMIC.
> >
> > Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> > ---
> > drivers/regulator/core.c | 106 +++++++++++++++++++++++++++++++++++++++
> > drivers/regulator/devres.c | 68 +++++++++++++++++++++++++
> > include/linux/regulator/driver.h | 51 +++++++++++++++++++
> > 3 files changed, 225 insertions(+)
> >
> > diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> > index 9a09f3c..7703853 100644
> > --- a/drivers/regulator/core.c
> > +++ b/drivers/regulator/core.c
> > @@ -3595,6 +3595,112 @@ void regulator_unregister(struct regulator_dev *rdev)
> > EXPORT_SYMBOL_GPL(regulator_unregister);
> >
> > /**
> > + * regulator_set_register - register a set of regulators
> > + * @config: runtime configuration for regulator set
> > + *
> > + * Called by regulator drivers to register a set of regulators.
> > + * Returns a valid pointer to struct regulator_set on success
> > + * or an ERR_PTR() on error.
> > + */
> > +struct regulator_set *
> > +regulator_set_register(const struct regulator_set_config *config)
> > +{
> > + struct regulator_set *rset;
> > + int prev_nregistered = -1;
> > + int nregistered = 0;
> > + int ret;
> > + int i;
> > +
> > + if (!config->descs || !config->nregulators)
> > + return ERR_PTR(-EINVAL);
> > +
> > + rset = kzalloc(sizeof(*rset) +
> > + (config->nregulators * sizeof(struct regulator_dev *)),
> > + GFP_KERNEL);
> > + if (!rset)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + rset->nregulators = config->nregulators;
> > +
> > + while (nregistered < config->nregulators &&
> > + prev_nregistered < nregistered) {
> > +
> > + prev_nregistered = nregistered;
> > +
> > + for (i = 0; i < config->nregulators; i++) {
> > + struct regulator_config conf;
> > + struct regulator_dev *regulator;
> > +
> > + if (rset->regulators[i])
> > + continue;
> > +
> > + memset(&conf, 0, sizeof(conf));
> > +
> > + conf.dev = config->dev;
> > + conf.regmap = config->regmap;
> > + conf.driver_data = config->driver_data;
> > + if (config->matches && config->matches[i].init_data)
> > + conf.init_data = config->matches[i].init_data;
> > + else if (config->init_data)
> > + conf.init_data = config->init_data[i];
> > +
> > + if (config->matches)
> > + conf.of_node = config->matches[i].of_node;
> > +
> > + regulator = regulator_register(&config->descs[i],
> > + &conf);
> > + if (IS_ERR(regulator)) {
> > + ret = PTR_ERR(regulator);
> > + if (ret == -EPROBE_DEFER)
> > + continue;
> > +
> > + goto err;
> > + }
> > +
> > + rset->regulators[i] = regulator;
> > + nregistered++;
> > + }
> > + }
> > +
> > + if (nregistered < config->nregulators) {
> > + ret = -EPROBE_DEFER;
> > + goto err;
> > + }
> > +
> > + return rset;
> > +
> > +err:
> > + for (i = 0; i < config->nregulators; i++) {
> > + if (rset->regulators[i])
> > + regulator_unregister(rset->regulators[i]);
> > + }
> > +
> > + kfree(rset);
> > +
> > + return ERR_PTR(ret);
> > +}
> > +EXPORT_SYMBOL_GPL(regulator_set_register);
> > +
> > +/**
> > + * regulator_set_unregister - unregister regulator set
> > + * @rset: regulator set to unregister
> > + *
> > + * Called by regulator drivers to unregister a regulator set.
> > + */
> > +void regulator_set_unregister(struct regulator_set *rset)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < rset->nregulators; i++) {
> > + if (rset->regulators[i])
> > + regulator_unregister(rset->regulators[i]);
> > + }
> > +
> > + kfree(rset);
> > +}
> > +EXPORT_SYMBOL_GPL(regulator_set_unregister);
> > +
> > +/**
> > * regulator_suspend_prepare - prepare regulators for system wide suspend
> > * @state: system suspend state
> > *
> > diff --git a/drivers/regulator/devres.c b/drivers/regulator/devres.c
> > index f44818b..a34f34c 100644
> > --- a/drivers/regulator/devres.c
> > +++ b/drivers/regulator/devres.c
> > @@ -251,6 +251,74 @@ void devm_regulator_unregister(struct device *dev, struct regulator_dev *rdev)
> > }
> > EXPORT_SYMBOL_GPL(devm_regulator_unregister);
> >
> > +static void devm_rset_release(struct device *dev, void *res)
> > +{
> > + regulator_set_unregister(*(struct regulator_set **)res);
> > +}
> > +
> > +/**
> > + * devm_regulator_set_register - Resource managed regulator_set_register()
> > + * @dev: device registering the regulator set
> > + * @config: runtime configuration for regulator set
> > + *
> > + * Called by regulator drivers to register a set of regulators. Returns a
> > + * valid pointer to struct regulator_set on success or an ERR_PTR() on
> > + * error. The regulator will automatically be released when the device
> > + * is unbound.
> > + */
> > +struct regulator_set *
> > +devm_regulator_set_register(struct device *dev,
> > + const struct regulator_set_config *config)
> > +{
> > + struct regulator_set **ptr, *rset;
> > +
> > + ptr = devres_alloc(devm_rset_release, sizeof(*ptr),
> > + GFP_KERNEL);
> > + if (!ptr)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + rset = regulator_set_register(config);
> > + if (!IS_ERR(rset)) {
> > + *ptr = rset;
> > + devres_add(dev, ptr);
> > + } else {
> > + devres_free(ptr);
> > + }
> > +
> > + return rset;
> > +}
> > +EXPORT_SYMBOL_GPL(devm_regulator_set_register);
> > +
> > +static int devm_rset_match(struct device *dev, void *res, void *data)
> > +{
> > + struct regulator_set **r = res;
> > + if (!r || !*r) {
> > + WARN_ON(!r || !*r);
> > + return 0;
> > + }
> > + return *r == data;
> > +}
> > +
> > +/**
> > + * devm_regulator_set_unregister - Resource managed regulator_set_unregister()
> > + * @dev: device unregistering the regulator set
> > + * @rset: regulator set to release
> > + *
> > + * Unregister a regulator set registered with devm_regulator_set_register().
> > + * Normally this function will not need to be called and the resource
> > + * management code will ensure that the resource is freed.
> > + */
> > +void devm_regulator_set_unregister(struct device *dev,
> > + struct regulator_set *rset)
> > +{
> > + int rc;
> > +
> > + rc = devres_release(dev, devm_rdev_release, devm_rset_match, rset);
> > + if (rc != 0)
> > + WARN_ON(rc);
> > +}
> > +EXPORT_SYMBOL_GPL(devm_regulator_unregister);
> > +
> > struct regulator_supply_alias_match {
> > struct device *dev;
> > const char *id;
> > diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
> > index bbe03a1..b683f68 100644
> > --- a/include/linux/regulator/driver.h
> > +++ b/include/linux/regulator/driver.h
> > @@ -350,6 +350,48 @@ struct regulator_dev {
> > unsigned int ena_gpio_state:1;
> > };
> >
> > +/**
> > + * struct regulator_set_config - Dynamic regulator set descriptor
> > + *
> > + * This structure describe a regulator and each regulator in this set
> > + * will be registered using informations passed through regulator_desc
> > + * and of_regulator_match tables.
> > + *
> > + * @dev: struct device providing the regulator set.
> > + * @driver_data: private driver data for this regulator set.
> > + * @regmap: regmap to use for core regmap helpers if dev_get_regulator() is
> > + * insufficient.
> > + * @descs: table of regulator descriptors.
> > + * @matches: table of DT regulator descriptors. This table should have
> > + * been filled by the of_regulator_match function.
> > + * @init_data: a table of init_data in case matches is NULL or the init_data
> > + * of the match entry is NULL.
> > + * @nregulators: number of regulators in the regulator set.
> > + */
> > +struct regulator_set_config {
> > + struct device *dev;
> > + void *driver_data;
> > + struct regmap *regmap;
> > + const struct regulator_desc *descs;
> > + const struct of_regulator_match *matches;
> > + const struct regulator_init_data **init_data;
> > + int nregulators;
> > +};
> > +
> > +/*
> > + * struct regulator_set - Regulator set
> > + *
> > + * This structure stores a set of regulator devices provided by a single
> > + * device (e.g. a PMIC device).
> > + *
> > + * @nregulators: number of regulators in the set
> > + * @regulators: regulators table
> > + */
> > +struct regulator_set {
> > + int nregulators;
> > + struct regulator_dev *regulators[0];
> > +};
> > +
> > struct regulator_dev *
> > regulator_register(const struct regulator_desc *regulator_desc,
> > const struct regulator_config *config);
> > @@ -360,6 +402,15 @@ devm_regulator_register(struct device *dev,
> > void regulator_unregister(struct regulator_dev *rdev);
> > void devm_regulator_unregister(struct device *dev, struct regulator_dev *rdev);
> >
> > +struct regulator_set *
> > +regulator_set_register(const struct regulator_set_config *config);
> > +struct regulator_set *
> > +devm_regulator_set_register(struct device *dev,
> > + const struct regulator_set_config *config);
> > +void regulator_set_unregister(struct regulator_set *rset);
> > +void devm_regulator_set_unregister(struct device *dev,
> > + struct regulator_set *rset);
> > +
> > int regulator_notifier_call_chain(struct regulator_dev *rdev,
> > unsigned long event, void *data);
> >
>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH v3 5/6] regulator: add support for regulator set registration
2014-06-16 8:08 ` Lee Jones
@ 2014-06-16 13:24 ` Boris BREZILLON
-1 siblings, 0 replies; 30+ messages in thread
From: Boris BREZILLON @ 2014-06-16 13:24 UTC (permalink / raw)
To: Lee Jones
Cc: Samuel Ortiz, Liam Girdwood, Mark Brown, Maxime Ripard,
Carlo Caione, Shuge, kevin, linux-kernel, linux-arm-kernel, dev
On 16/06/2014 10:08, Lee Jones wrote:
>> Hello Mark,
>>
>> Did you have time to take a look at this patch ?
>>
>> I'd like to post a new version of this series addressing Lee's comments,
>> but it would be great to have your feedback on this patch before posting
>> a new version.
> I wouldn't do that, as you're likely to upset him.
>
> Just fix what you know and resend.
>
> Mark will subsequently review I'm sure.
Okay, I'll send a new version soon.
>
>> Best Regards,
>>
>> Boris
>>
>> On 27/05/2014 11:38, Boris BREZILLON wrote:
>>> PMIC devices often provide several regulators, and these regulators are
>>> all using the same regmap and are all attached to the same device (the
>>> PMIC device).
>>>
>>> Add helper functions (both simple and resource managed versions) to
>>> register and unregister such kind of regulator set.
>>>
>>> This implementation handles self dependency issues where one regulator
>>> provided the PMIC depends on another one provided by the same PMIC.
>>>
>>> Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
>>> ---
>>> drivers/regulator/core.c | 106 +++++++++++++++++++++++++++++++++++++++
>>> drivers/regulator/devres.c | 68 +++++++++++++++++++++++++
>>> include/linux/regulator/driver.h | 51 +++++++++++++++++++
>>> 3 files changed, 225 insertions(+)
>>>
>>> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
>>> index 9a09f3c..7703853 100644
>>> --- a/drivers/regulator/core.c
>>> +++ b/drivers/regulator/core.c
>>> @@ -3595,6 +3595,112 @@ void regulator_unregister(struct regulator_dev *rdev)
>>> EXPORT_SYMBOL_GPL(regulator_unregister);
>>>
>>> /**
>>> + * regulator_set_register - register a set of regulators
>>> + * @config: runtime configuration for regulator set
>>> + *
>>> + * Called by regulator drivers to register a set of regulators.
>>> + * Returns a valid pointer to struct regulator_set on success
>>> + * or an ERR_PTR() on error.
>>> + */
>>> +struct regulator_set *
>>> +regulator_set_register(const struct regulator_set_config *config)
>>> +{
>>> + struct regulator_set *rset;
>>> + int prev_nregistered = -1;
>>> + int nregistered = 0;
>>> + int ret;
>>> + int i;
>>> +
>>> + if (!config->descs || !config->nregulators)
>>> + return ERR_PTR(-EINVAL);
>>> +
>>> + rset = kzalloc(sizeof(*rset) +
>>> + (config->nregulators * sizeof(struct regulator_dev *)),
>>> + GFP_KERNEL);
>>> + if (!rset)
>>> + return ERR_PTR(-ENOMEM);
>>> +
>>> + rset->nregulators = config->nregulators;
>>> +
>>> + while (nregistered < config->nregulators &&
>>> + prev_nregistered < nregistered) {
>>> +
>>> + prev_nregistered = nregistered;
>>> +
>>> + for (i = 0; i < config->nregulators; i++) {
>>> + struct regulator_config conf;
>>> + struct regulator_dev *regulator;
>>> +
>>> + if (rset->regulators[i])
>>> + continue;
>>> +
>>> + memset(&conf, 0, sizeof(conf));
>>> +
>>> + conf.dev = config->dev;
>>> + conf.regmap = config->regmap;
>>> + conf.driver_data = config->driver_data;
>>> + if (config->matches && config->matches[i].init_data)
>>> + conf.init_data = config->matches[i].init_data;
>>> + else if (config->init_data)
>>> + conf.init_data = config->init_data[i];
>>> +
>>> + if (config->matches)
>>> + conf.of_node = config->matches[i].of_node;
>>> +
>>> + regulator = regulator_register(&config->descs[i],
>>> + &conf);
>>> + if (IS_ERR(regulator)) {
>>> + ret = PTR_ERR(regulator);
>>> + if (ret == -EPROBE_DEFER)
>>> + continue;
>>> +
>>> + goto err;
>>> + }
>>> +
>>> + rset->regulators[i] = regulator;
>>> + nregistered++;
>>> + }
>>> + }
>>> +
>>> + if (nregistered < config->nregulators) {
>>> + ret = -EPROBE_DEFER;
>>> + goto err;
>>> + }
>>> +
>>> + return rset;
>>> +
>>> +err:
>>> + for (i = 0; i < config->nregulators; i++) {
>>> + if (rset->regulators[i])
>>> + regulator_unregister(rset->regulators[i]);
>>> + }
>>> +
>>> + kfree(rset);
>>> +
>>> + return ERR_PTR(ret);
>>> +}
>>> +EXPORT_SYMBOL_GPL(regulator_set_register);
>>> +
>>> +/**
>>> + * regulator_set_unregister - unregister regulator set
>>> + * @rset: regulator set to unregister
>>> + *
>>> + * Called by regulator drivers to unregister a regulator set.
>>> + */
>>> +void regulator_set_unregister(struct regulator_set *rset)
>>> +{
>>> + int i;
>>> +
>>> + for (i = 0; i < rset->nregulators; i++) {
>>> + if (rset->regulators[i])
>>> + regulator_unregister(rset->regulators[i]);
>>> + }
>>> +
>>> + kfree(rset);
>>> +}
>>> +EXPORT_SYMBOL_GPL(regulator_set_unregister);
>>> +
>>> +/**
>>> * regulator_suspend_prepare - prepare regulators for system wide suspend
>>> * @state: system suspend state
>>> *
>>> diff --git a/drivers/regulator/devres.c b/drivers/regulator/devres.c
>>> index f44818b..a34f34c 100644
>>> --- a/drivers/regulator/devres.c
>>> +++ b/drivers/regulator/devres.c
>>> @@ -251,6 +251,74 @@ void devm_regulator_unregister(struct device *dev, struct regulator_dev *rdev)
>>> }
>>> EXPORT_SYMBOL_GPL(devm_regulator_unregister);
>>>
>>> +static void devm_rset_release(struct device *dev, void *res)
>>> +{
>>> + regulator_set_unregister(*(struct regulator_set **)res);
>>> +}
>>> +
>>> +/**
>>> + * devm_regulator_set_register - Resource managed regulator_set_register()
>>> + * @dev: device registering the regulator set
>>> + * @config: runtime configuration for regulator set
>>> + *
>>> + * Called by regulator drivers to register a set of regulators. Returns a
>>> + * valid pointer to struct regulator_set on success or an ERR_PTR() on
>>> + * error. The regulator will automatically be released when the device
>>> + * is unbound.
>>> + */
>>> +struct regulator_set *
>>> +devm_regulator_set_register(struct device *dev,
>>> + const struct regulator_set_config *config)
>>> +{
>>> + struct regulator_set **ptr, *rset;
>>> +
>>> + ptr = devres_alloc(devm_rset_release, sizeof(*ptr),
>>> + GFP_KERNEL);
>>> + if (!ptr)
>>> + return ERR_PTR(-ENOMEM);
>>> +
>>> + rset = regulator_set_register(config);
>>> + if (!IS_ERR(rset)) {
>>> + *ptr = rset;
>>> + devres_add(dev, ptr);
>>> + } else {
>>> + devres_free(ptr);
>>> + }
>>> +
>>> + return rset;
>>> +}
>>> +EXPORT_SYMBOL_GPL(devm_regulator_set_register);
>>> +
>>> +static int devm_rset_match(struct device *dev, void *res, void *data)
>>> +{
>>> + struct regulator_set **r = res;
>>> + if (!r || !*r) {
>>> + WARN_ON(!r || !*r);
>>> + return 0;
>>> + }
>>> + return *r == data;
>>> +}
>>> +
>>> +/**
>>> + * devm_regulator_set_unregister - Resource managed regulator_set_unregister()
>>> + * @dev: device unregistering the regulator set
>>> + * @rset: regulator set to release
>>> + *
>>> + * Unregister a regulator set registered with devm_regulator_set_register().
>>> + * Normally this function will not need to be called and the resource
>>> + * management code will ensure that the resource is freed.
>>> + */
>>> +void devm_regulator_set_unregister(struct device *dev,
>>> + struct regulator_set *rset)
>>> +{
>>> + int rc;
>>> +
>>> + rc = devres_release(dev, devm_rdev_release, devm_rset_match, rset);
>>> + if (rc != 0)
>>> + WARN_ON(rc);
>>> +}
>>> +EXPORT_SYMBOL_GPL(devm_regulator_unregister);
>>> +
>>> struct regulator_supply_alias_match {
>>> struct device *dev;
>>> const char *id;
>>> diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
>>> index bbe03a1..b683f68 100644
>>> --- a/include/linux/regulator/driver.h
>>> +++ b/include/linux/regulator/driver.h
>>> @@ -350,6 +350,48 @@ struct regulator_dev {
>>> unsigned int ena_gpio_state:1;
>>> };
>>>
>>> +/**
>>> + * struct regulator_set_config - Dynamic regulator set descriptor
>>> + *
>>> + * This structure describe a regulator and each regulator in this set
>>> + * will be registered using informations passed through regulator_desc
>>> + * and of_regulator_match tables.
>>> + *
>>> + * @dev: struct device providing the regulator set.
>>> + * @driver_data: private driver data for this regulator set.
>>> + * @regmap: regmap to use for core regmap helpers if dev_get_regulator() is
>>> + * insufficient.
>>> + * @descs: table of regulator descriptors.
>>> + * @matches: table of DT regulator descriptors. This table should have
>>> + * been filled by the of_regulator_match function.
>>> + * @init_data: a table of init_data in case matches is NULL or the init_data
>>> + * of the match entry is NULL.
>>> + * @nregulators: number of regulators in the regulator set.
>>> + */
>>> +struct regulator_set_config {
>>> + struct device *dev;
>>> + void *driver_data;
>>> + struct regmap *regmap;
>>> + const struct regulator_desc *descs;
>>> + const struct of_regulator_match *matches;
>>> + const struct regulator_init_data **init_data;
>>> + int nregulators;
>>> +};
>>> +
>>> +/*
>>> + * struct regulator_set - Regulator set
>>> + *
>>> + * This structure stores a set of regulator devices provided by a single
>>> + * device (e.g. a PMIC device).
>>> + *
>>> + * @nregulators: number of regulators in the set
>>> + * @regulators: regulators table
>>> + */
>>> +struct regulator_set {
>>> + int nregulators;
>>> + struct regulator_dev *regulators[0];
>>> +};
>>> +
>>> struct regulator_dev *
>>> regulator_register(const struct regulator_desc *regulator_desc,
>>> const struct regulator_config *config);
>>> @@ -360,6 +402,15 @@ devm_regulator_register(struct device *dev,
>>> void regulator_unregister(struct regulator_dev *rdev);
>>> void devm_regulator_unregister(struct device *dev, struct regulator_dev *rdev);
>>>
>>> +struct regulator_set *
>>> +regulator_set_register(const struct regulator_set_config *config);
>>> +struct regulator_set *
>>> +devm_regulator_set_register(struct device *dev,
>>> + const struct regulator_set_config *config);
>>> +void regulator_set_unregister(struct regulator_set *rset);
>>> +void devm_regulator_set_unregister(struct device *dev,
>>> + struct regulator_set *rset);
>>> +
>>> int regulator_notifier_call_chain(struct regulator_dev *rdev,
>>> unsigned long event, void *data);
>>>
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCH v3 5/6] regulator: add support for regulator set registration
@ 2014-06-16 13:24 ` Boris BREZILLON
0 siblings, 0 replies; 30+ messages in thread
From: Boris BREZILLON @ 2014-06-16 13:24 UTC (permalink / raw)
To: linux-arm-kernel
On 16/06/2014 10:08, Lee Jones wrote:
>> Hello Mark,
>>
>> Did you have time to take a look at this patch ?
>>
>> I'd like to post a new version of this series addressing Lee's comments,
>> but it would be great to have your feedback on this patch before posting
>> a new version.
> I wouldn't do that, as you're likely to upset him.
>
> Just fix what you know and resend.
>
> Mark will subsequently review I'm sure.
Okay, I'll send a new version soon.
>
>> Best Regards,
>>
>> Boris
>>
>> On 27/05/2014 11:38, Boris BREZILLON wrote:
>>> PMIC devices often provide several regulators, and these regulators are
>>> all using the same regmap and are all attached to the same device (the
>>> PMIC device).
>>>
>>> Add helper functions (both simple and resource managed versions) to
>>> register and unregister such kind of regulator set.
>>>
>>> This implementation handles self dependency issues where one regulator
>>> provided the PMIC depends on another one provided by the same PMIC.
>>>
>>> Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
>>> ---
>>> drivers/regulator/core.c | 106 +++++++++++++++++++++++++++++++++++++++
>>> drivers/regulator/devres.c | 68 +++++++++++++++++++++++++
>>> include/linux/regulator/driver.h | 51 +++++++++++++++++++
>>> 3 files changed, 225 insertions(+)
>>>
>>> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
>>> index 9a09f3c..7703853 100644
>>> --- a/drivers/regulator/core.c
>>> +++ b/drivers/regulator/core.c
>>> @@ -3595,6 +3595,112 @@ void regulator_unregister(struct regulator_dev *rdev)
>>> EXPORT_SYMBOL_GPL(regulator_unregister);
>>>
>>> /**
>>> + * regulator_set_register - register a set of regulators
>>> + * @config: runtime configuration for regulator set
>>> + *
>>> + * Called by regulator drivers to register a set of regulators.
>>> + * Returns a valid pointer to struct regulator_set on success
>>> + * or an ERR_PTR() on error.
>>> + */
>>> +struct regulator_set *
>>> +regulator_set_register(const struct regulator_set_config *config)
>>> +{
>>> + struct regulator_set *rset;
>>> + int prev_nregistered = -1;
>>> + int nregistered = 0;
>>> + int ret;
>>> + int i;
>>> +
>>> + if (!config->descs || !config->nregulators)
>>> + return ERR_PTR(-EINVAL);
>>> +
>>> + rset = kzalloc(sizeof(*rset) +
>>> + (config->nregulators * sizeof(struct regulator_dev *)),
>>> + GFP_KERNEL);
>>> + if (!rset)
>>> + return ERR_PTR(-ENOMEM);
>>> +
>>> + rset->nregulators = config->nregulators;
>>> +
>>> + while (nregistered < config->nregulators &&
>>> + prev_nregistered < nregistered) {
>>> +
>>> + prev_nregistered = nregistered;
>>> +
>>> + for (i = 0; i < config->nregulators; i++) {
>>> + struct regulator_config conf;
>>> + struct regulator_dev *regulator;
>>> +
>>> + if (rset->regulators[i])
>>> + continue;
>>> +
>>> + memset(&conf, 0, sizeof(conf));
>>> +
>>> + conf.dev = config->dev;
>>> + conf.regmap = config->regmap;
>>> + conf.driver_data = config->driver_data;
>>> + if (config->matches && config->matches[i].init_data)
>>> + conf.init_data = config->matches[i].init_data;
>>> + else if (config->init_data)
>>> + conf.init_data = config->init_data[i];
>>> +
>>> + if (config->matches)
>>> + conf.of_node = config->matches[i].of_node;
>>> +
>>> + regulator = regulator_register(&config->descs[i],
>>> + &conf);
>>> + if (IS_ERR(regulator)) {
>>> + ret = PTR_ERR(regulator);
>>> + if (ret == -EPROBE_DEFER)
>>> + continue;
>>> +
>>> + goto err;
>>> + }
>>> +
>>> + rset->regulators[i] = regulator;
>>> + nregistered++;
>>> + }
>>> + }
>>> +
>>> + if (nregistered < config->nregulators) {
>>> + ret = -EPROBE_DEFER;
>>> + goto err;
>>> + }
>>> +
>>> + return rset;
>>> +
>>> +err:
>>> + for (i = 0; i < config->nregulators; i++) {
>>> + if (rset->regulators[i])
>>> + regulator_unregister(rset->regulators[i]);
>>> + }
>>> +
>>> + kfree(rset);
>>> +
>>> + return ERR_PTR(ret);
>>> +}
>>> +EXPORT_SYMBOL_GPL(regulator_set_register);
>>> +
>>> +/**
>>> + * regulator_set_unregister - unregister regulator set
>>> + * @rset: regulator set to unregister
>>> + *
>>> + * Called by regulator drivers to unregister a regulator set.
>>> + */
>>> +void regulator_set_unregister(struct regulator_set *rset)
>>> +{
>>> + int i;
>>> +
>>> + for (i = 0; i < rset->nregulators; i++) {
>>> + if (rset->regulators[i])
>>> + regulator_unregister(rset->regulators[i]);
>>> + }
>>> +
>>> + kfree(rset);
>>> +}
>>> +EXPORT_SYMBOL_GPL(regulator_set_unregister);
>>> +
>>> +/**
>>> * regulator_suspend_prepare - prepare regulators for system wide suspend
>>> * @state: system suspend state
>>> *
>>> diff --git a/drivers/regulator/devres.c b/drivers/regulator/devres.c
>>> index f44818b..a34f34c 100644
>>> --- a/drivers/regulator/devres.c
>>> +++ b/drivers/regulator/devres.c
>>> @@ -251,6 +251,74 @@ void devm_regulator_unregister(struct device *dev, struct regulator_dev *rdev)
>>> }
>>> EXPORT_SYMBOL_GPL(devm_regulator_unregister);
>>>
>>> +static void devm_rset_release(struct device *dev, void *res)
>>> +{
>>> + regulator_set_unregister(*(struct regulator_set **)res);
>>> +}
>>> +
>>> +/**
>>> + * devm_regulator_set_register - Resource managed regulator_set_register()
>>> + * @dev: device registering the regulator set
>>> + * @config: runtime configuration for regulator set
>>> + *
>>> + * Called by regulator drivers to register a set of regulators. Returns a
>>> + * valid pointer to struct regulator_set on success or an ERR_PTR() on
>>> + * error. The regulator will automatically be released when the device
>>> + * is unbound.
>>> + */
>>> +struct regulator_set *
>>> +devm_regulator_set_register(struct device *dev,
>>> + const struct regulator_set_config *config)
>>> +{
>>> + struct regulator_set **ptr, *rset;
>>> +
>>> + ptr = devres_alloc(devm_rset_release, sizeof(*ptr),
>>> + GFP_KERNEL);
>>> + if (!ptr)
>>> + return ERR_PTR(-ENOMEM);
>>> +
>>> + rset = regulator_set_register(config);
>>> + if (!IS_ERR(rset)) {
>>> + *ptr = rset;
>>> + devres_add(dev, ptr);
>>> + } else {
>>> + devres_free(ptr);
>>> + }
>>> +
>>> + return rset;
>>> +}
>>> +EXPORT_SYMBOL_GPL(devm_regulator_set_register);
>>> +
>>> +static int devm_rset_match(struct device *dev, void *res, void *data)
>>> +{
>>> + struct regulator_set **r = res;
>>> + if (!r || !*r) {
>>> + WARN_ON(!r || !*r);
>>> + return 0;
>>> + }
>>> + return *r == data;
>>> +}
>>> +
>>> +/**
>>> + * devm_regulator_set_unregister - Resource managed regulator_set_unregister()
>>> + * @dev: device unregistering the regulator set
>>> + * @rset: regulator set to release
>>> + *
>>> + * Unregister a regulator set registered with devm_regulator_set_register().
>>> + * Normally this function will not need to be called and the resource
>>> + * management code will ensure that the resource is freed.
>>> + */
>>> +void devm_regulator_set_unregister(struct device *dev,
>>> + struct regulator_set *rset)
>>> +{
>>> + int rc;
>>> +
>>> + rc = devres_release(dev, devm_rdev_release, devm_rset_match, rset);
>>> + if (rc != 0)
>>> + WARN_ON(rc);
>>> +}
>>> +EXPORT_SYMBOL_GPL(devm_regulator_unregister);
>>> +
>>> struct regulator_supply_alias_match {
>>> struct device *dev;
>>> const char *id;
>>> diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
>>> index bbe03a1..b683f68 100644
>>> --- a/include/linux/regulator/driver.h
>>> +++ b/include/linux/regulator/driver.h
>>> @@ -350,6 +350,48 @@ struct regulator_dev {
>>> unsigned int ena_gpio_state:1;
>>> };
>>>
>>> +/**
>>> + * struct regulator_set_config - Dynamic regulator set descriptor
>>> + *
>>> + * This structure describe a regulator and each regulator in this set
>>> + * will be registered using informations passed through regulator_desc
>>> + * and of_regulator_match tables.
>>> + *
>>> + * @dev: struct device providing the regulator set.
>>> + * @driver_data: private driver data for this regulator set.
>>> + * @regmap: regmap to use for core regmap helpers if dev_get_regulator() is
>>> + * insufficient.
>>> + * @descs: table of regulator descriptors.
>>> + * @matches: table of DT regulator descriptors. This table should have
>>> + * been filled by the of_regulator_match function.
>>> + * @init_data: a table of init_data in case matches is NULL or the init_data
>>> + * of the match entry is NULL.
>>> + * @nregulators: number of regulators in the regulator set.
>>> + */
>>> +struct regulator_set_config {
>>> + struct device *dev;
>>> + void *driver_data;
>>> + struct regmap *regmap;
>>> + const struct regulator_desc *descs;
>>> + const struct of_regulator_match *matches;
>>> + const struct regulator_init_data **init_data;
>>> + int nregulators;
>>> +};
>>> +
>>> +/*
>>> + * struct regulator_set - Regulator set
>>> + *
>>> + * This structure stores a set of regulator devices provided by a single
>>> + * device (e.g. a PMIC device).
>>> + *
>>> + * @nregulators: number of regulators in the set
>>> + * @regulators: regulators table
>>> + */
>>> +struct regulator_set {
>>> + int nregulators;
>>> + struct regulator_dev *regulators[0];
>>> +};
>>> +
>>> struct regulator_dev *
>>> regulator_register(const struct regulator_desc *regulator_desc,
>>> const struct regulator_config *config);
>>> @@ -360,6 +402,15 @@ devm_regulator_register(struct device *dev,
>>> void regulator_unregister(struct regulator_dev *rdev);
>>> void devm_regulator_unregister(struct device *dev, struct regulator_dev *rdev);
>>>
>>> +struct regulator_set *
>>> +regulator_set_register(const struct regulator_set_config *config);
>>> +struct regulator_set *
>>> +devm_regulator_set_register(struct device *dev,
>>> + const struct regulator_set_config *config);
>>> +void regulator_set_unregister(struct regulator_set *rset);
>>> +void devm_regulator_set_unregister(struct device *dev,
>>> + struct regulator_set *rset);
>>> +
>>> int regulator_notifier_call_chain(struct regulator_dev *rdev,
>>> unsigned long event, void *data);
>>>
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v3 6/6] regulator: axp20x: make use of devm_regulator_set_register
2014-05-27 9:38 ` Boris BREZILLON
@ 2014-05-27 9:38 ` Boris BREZILLON
-1 siblings, 0 replies; 30+ messages in thread
From: Boris BREZILLON @ 2014-05-27 9:38 UTC (permalink / raw)
To: Samuel Ortiz, Lee Jones, Liam Girdwood, Mark Brown
Cc: Maxime Ripard, Carlo Caione, Shuge, kevin, linux-kernel,
linux-arm-kernel, dev, Boris BREZILLON
Make use of the devm_regulator_set_register instead of registering each
regulator provided by the PMIC.
This also solves a self dependency issue where one regulator of the PMIC
is used as a supply for anoher regulator provided by the same PMIC.
Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
---
drivers/regulator/axp20x-regulator.c | 59 ++++++++++++++----------------------
1 file changed, 23 insertions(+), 36 deletions(-)
diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index 4486517..c799827 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -300,66 +300,53 @@ static int axp20x_set_dcdc_workmode(struct regulator_dev *rdev, int id, u32 work
static int axp20x_regulator_probe(struct platform_device *pdev)
{
- struct regulator_dev *rdev;
+ struct regulator_set *rset;
struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
- struct regulator_config config = { };
- struct regulator_init_data *init_data;
+ struct regulator_set_config config;
struct of_regulator_match *matches;
- int nmatches;
- const struct regulator_desc *regulators;
- int nregulators;
int ret, i;
u32 workmode;
+ memset(&config, 0, sizeof(config));
+ config.dev = &pdev->dev;
+ config.regmap = axp20x->regmap;
if (axp20x->variant == AXP221_ID) {
matches = axp22x_matches;
- nmatches = ARRAY_SIZE(axp22x_matches);
- regulators = axp22x_regulators;
- nregulators = AXP22X_REG_ID_MAX;
+ config.descs = axp22x_regulators;
+ config.nregulators = AXP22X_REG_ID_MAX;
} else {
matches = axp20x_matches;
- nmatches = ARRAY_SIZE(axp20x_matches);
- regulators = axp20x_regulators;
- nregulators = AXP20X_REG_ID_MAX;
+ config.descs = axp20x_regulators;
+ config.nregulators = AXP20X_REG_ID_MAX;
}
+ config.matches = matches;
/*
* Reset matches table (this table might have been modified by a
* previous AXP2xx device probe).
*/
- for (i = 0; i < nmatches; i++) {
+ for (i = 0; i < config.nregulators; i++) {
matches[i].init_data = NULL;
matches[i].of_node = NULL;
}
- ret = axp20x_regulator_parse_dt(pdev, matches, nmatches);
+ ret = axp20x_regulator_parse_dt(pdev, matches,
+ config.nregulators);
if (ret)
return ret;
- for (i = 0; i < AXP20X_REG_ID_MAX; i++) {
- init_data = matches[i].init_data;
+ rset = devm_regulator_set_register(&pdev->dev, &config);
+ if (IS_ERR(rset))
+ return PTR_ERR(rset);
- config.dev = &pdev->dev;
- config.init_data = init_data;
- config.regmap = axp20x->regmap;
- config.of_node = matches[i].of_node;
-
- rdev = devm_regulator_register(&pdev->dev, ®ulators[i],
- &config);
- if (IS_ERR(rdev)) {
- dev_err(&pdev->dev, "Failed to register %s\n",
- regulators[i].name);
-
- return PTR_ERR(rdev);
- }
-
- ret = of_property_read_u32(matches[i].of_node, "x-powers,dcdc-workmode",
+ for (i = 0; i < rset->nregulators; i++) {
+ ret = of_property_read_u32(config.matches[i].of_node,
+ "x-powers,dcdc-workmode",
&workmode);
- if (!ret) {
- if (axp20x_set_dcdc_workmode(rdev, i, workmode))
- dev_err(&pdev->dev, "Failed to set workmode on %s\n",
- regulators[i].name);
- }
+ if (!ret &&
+ axp20x_set_dcdc_workmode(rset->regulators[i], i, workmode))
+ dev_err(&pdev->dev, "Failed to set workmode on %s\n",
+ config.descs[i].name);
}
return 0;
--
1.8.3.2
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH v3 6/6] regulator: axp20x: make use of devm_regulator_set_register
@ 2014-05-27 9:38 ` Boris BREZILLON
0 siblings, 0 replies; 30+ messages in thread
From: Boris BREZILLON @ 2014-05-27 9:38 UTC (permalink / raw)
To: linux-arm-kernel
Make use of the devm_regulator_set_register instead of registering each
regulator provided by the PMIC.
This also solves a self dependency issue where one regulator of the PMIC
is used as a supply for anoher regulator provided by the same PMIC.
Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
---
drivers/regulator/axp20x-regulator.c | 59 ++++++++++++++----------------------
1 file changed, 23 insertions(+), 36 deletions(-)
diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index 4486517..c799827 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -300,66 +300,53 @@ static int axp20x_set_dcdc_workmode(struct regulator_dev *rdev, int id, u32 work
static int axp20x_regulator_probe(struct platform_device *pdev)
{
- struct regulator_dev *rdev;
+ struct regulator_set *rset;
struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
- struct regulator_config config = { };
- struct regulator_init_data *init_data;
+ struct regulator_set_config config;
struct of_regulator_match *matches;
- int nmatches;
- const struct regulator_desc *regulators;
- int nregulators;
int ret, i;
u32 workmode;
+ memset(&config, 0, sizeof(config));
+ config.dev = &pdev->dev;
+ config.regmap = axp20x->regmap;
if (axp20x->variant == AXP221_ID) {
matches = axp22x_matches;
- nmatches = ARRAY_SIZE(axp22x_matches);
- regulators = axp22x_regulators;
- nregulators = AXP22X_REG_ID_MAX;
+ config.descs = axp22x_regulators;
+ config.nregulators = AXP22X_REG_ID_MAX;
} else {
matches = axp20x_matches;
- nmatches = ARRAY_SIZE(axp20x_matches);
- regulators = axp20x_regulators;
- nregulators = AXP20X_REG_ID_MAX;
+ config.descs = axp20x_regulators;
+ config.nregulators = AXP20X_REG_ID_MAX;
}
+ config.matches = matches;
/*
* Reset matches table (this table might have been modified by a
* previous AXP2xx device probe).
*/
- for (i = 0; i < nmatches; i++) {
+ for (i = 0; i < config.nregulators; i++) {
matches[i].init_data = NULL;
matches[i].of_node = NULL;
}
- ret = axp20x_regulator_parse_dt(pdev, matches, nmatches);
+ ret = axp20x_regulator_parse_dt(pdev, matches,
+ config.nregulators);
if (ret)
return ret;
- for (i = 0; i < AXP20X_REG_ID_MAX; i++) {
- init_data = matches[i].init_data;
+ rset = devm_regulator_set_register(&pdev->dev, &config);
+ if (IS_ERR(rset))
+ return PTR_ERR(rset);
- config.dev = &pdev->dev;
- config.init_data = init_data;
- config.regmap = axp20x->regmap;
- config.of_node = matches[i].of_node;
-
- rdev = devm_regulator_register(&pdev->dev, ®ulators[i],
- &config);
- if (IS_ERR(rdev)) {
- dev_err(&pdev->dev, "Failed to register %s\n",
- regulators[i].name);
-
- return PTR_ERR(rdev);
- }
-
- ret = of_property_read_u32(matches[i].of_node, "x-powers,dcdc-workmode",
+ for (i = 0; i < rset->nregulators; i++) {
+ ret = of_property_read_u32(config.matches[i].of_node,
+ "x-powers,dcdc-workmode",
&workmode);
- if (!ret) {
- if (axp20x_set_dcdc_workmode(rdev, i, workmode))
- dev_err(&pdev->dev, "Failed to set workmode on %s\n",
- regulators[i].name);
- }
+ if (!ret &&
+ axp20x_set_dcdc_workmode(rset->regulators[i], i, workmode))
+ dev_err(&pdev->dev, "Failed to set workmode on %s\n",
+ config.descs[i].name);
}
return 0;
--
1.8.3.2
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v3 0/6] mfd: axp20x: add AXP221 PMIC support
2014-05-27 9:38 ` Boris BREZILLON
@ 2014-05-27 19:49 ` Mark Brown
-1 siblings, 0 replies; 30+ messages in thread
From: Mark Brown @ 2014-05-27 19:49 UTC (permalink / raw)
To: Boris BREZILLON
Cc: Samuel Ortiz, Lee Jones, Liam Girdwood, Maxime Ripard,
Carlo Caione, Shuge, kevin, linux-kernel, linux-arm-kernel, dev
[-- Attachment #1: Type: text/plain, Size: 1391 bytes --]
On Tue, May 27, 2014 at 11:38:49AM +0200, Boris BREZILLON wrote:
> I'm still waiting for inputs regarding the ipsout regulator representation:
If you've got questions you need an answer to urgently it's possibly
best to highlight them outside of a series like this doing that helps
make it more obvious.
> * The ipsout regulator is currently represented as a fixed regulator
> providing a 5v output, and this is not exactly the case.
> AFAIU (here's is the datasheet traduction if you want to check [1]), the
> ipsout output is a multiplexer that choose among vbus (5V), acin (12V
> which is then converted to 5V) and the battery power supply (3,5 -> 4,2 V).
> This means the output voltage of ipsout vary between 3,5V and 5V.
> How can we express this kind of muxer in the regulator framework (is there
> already something available ?) ?
> Note that the power supply selection is automatic, though we could force
> one power supply, but then we loose the ability to unplug one power supply
> without impacting the system.
This sounds like it may as well just be represented as an unregulated
supply - it's just the system root supply really. Nothing can rely on
the voltage anyway as it's going to vary randomly depending on what the
user does power wise and typically it'd only be used as a supply for
things that don't care about the specific voltage.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v3 0/6] mfd: axp20x: add AXP221 PMIC support
@ 2014-05-27 19:49 ` Mark Brown
0 siblings, 0 replies; 30+ messages in thread
From: Mark Brown @ 2014-05-27 19:49 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, May 27, 2014 at 11:38:49AM +0200, Boris BREZILLON wrote:
> I'm still waiting for inputs regarding the ipsout regulator representation:
If you've got questions you need an answer to urgently it's possibly
best to highlight them outside of a series like this doing that helps
make it more obvious.
> * The ipsout regulator is currently represented as a fixed regulator
> providing a 5v output, and this is not exactly the case.
> AFAIU (here's is the datasheet traduction if you want to check [1]), the
> ipsout output is a multiplexer that choose among vbus (5V), acin (12V
> which is then converted to 5V) and the battery power supply (3,5 -> 4,2 V).
> This means the output voltage of ipsout vary between 3,5V and 5V.
> How can we express this kind of muxer in the regulator framework (is there
> already something available ?) ?
> Note that the power supply selection is automatic, though we could force
> one power supply, but then we loose the ability to unplug one power supply
> without impacting the system.
This sounds like it may as well just be represented as an unregulated
supply - it's just the system root supply really. Nothing can rely on
the voltage anyway as it's going to vary randomly depending on what the
user does power wise and typically it'd only be used as a supply for
things that don't care about the specific voltage.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140527/0a89a4cc/attachment.sig>
^ permalink raw reply [flat|nested] 30+ messages in thread
[parent not found: <9ed19670-7a2e-4324-a201-6d3c8514bdb3@googlegroups.com>]
* Re: [PATCH v3 0/6] mfd: axp20x: add AXP221 PMIC support
[not found] ` <9ed19670-7a2e-4324-a201-6d3c8514bdb3@googlegroups.com>
@ 2014-05-28 17:49 ` Mark Brown
0 siblings, 0 replies; 30+ messages in thread
From: Mark Brown @ 2014-05-28 17:49 UTC (permalink / raw)
To: Dimitar Penev
Cc: linux-sunxi, Boris BREZILLON, Samuel Ortiz, Lee Jones,
Liam Girdwood, Maxime Ripard, Carlo Caione, Shuge, kevin,
linux-kernel, linux-arm-kernel, dev
[-- Attachment #1: Type: text/plain, Size: 488 bytes --]
On Wed, May 28, 2014 at 09:00:17AM -0700, Dimitar Penev wrote:
> Hi Mark,
Don't top post and if you are addressing someone please put them in the
To: rather than Ccs - I very nearly just deleted this since it's a MFD
mail not directly addressed to me.
> I plan to use AXP221 on A20 based system as I require more switchers than
> the recommended AXP209 has.
> Do you see any obstacles which I may be missing now?
I'm afraid I don't really understand your question, sorry.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v3 0/6] mfd: axp20x: add AXP221 PMIC support
2014-05-27 19:49 ` Mark Brown
@ 2014-05-28 19:46 ` Boris BREZILLON
-1 siblings, 0 replies; 30+ messages in thread
From: Boris BREZILLON @ 2014-05-28 19:46 UTC (permalink / raw)
To: Mark Brown
Cc: Samuel Ortiz, Lee Jones, Liam Girdwood, Maxime Ripard,
Carlo Caione, Shuge, kevin, linux-kernel, linux-arm-kernel, dev
Hello Mark,
On 27/05/2014 21:49, Mark Brown wrote:
> On Tue, May 27, 2014 at 11:38:49AM +0200, Boris BREZILLON wrote:
>
>> I'm still waiting for inputs regarding the ipsout regulator representation:
> If you've got questions you need an answer to urgently it's possibly
> best to highlight them outside of a series like this doing that helps
> make it more obvious.
Noted. I'll take care to send a separate mail next time.
BTW, I don't if you've noticed, but patch 5 of this series implements
helper functions to register several regulators from a regulator_desc
and an of_regulator_match table (as you suggested).
>> * The ipsout regulator is currently represented as a fixed regulator
>> providing a 5v output, and this is not exactly the case.
>> AFAIU (here's is the datasheet traduction if you want to check [1]), the
>> ipsout output is a multiplexer that choose among vbus (5V), acin (12V
>> which is then converted to 5V) and the battery power supply (3,5 -> 4,2 V).
>> This means the output voltage of ipsout vary between 3,5V and 5V.
>> How can we express this kind of muxer in the regulator framework (is there
>> already something available ?) ?
>> Note that the power supply selection is automatic, though we could force
>> one power supply, but then we loose the ability to unplug one power supply
>> without impacting the system.
> This sounds like it may as well just be represented as an unregulated
> supply - it's just the system root supply really. Nothing can rely on
> the voltage anyway as it's going to vary randomly depending on what the
> user does power wise and typically it'd only be used as a supply for
> things that don't care about the specific voltage.
Okay, I'm fine with that.
Thanks.
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v3 0/6] mfd: axp20x: add AXP221 PMIC support
@ 2014-05-28 19:46 ` Boris BREZILLON
0 siblings, 0 replies; 30+ messages in thread
From: Boris BREZILLON @ 2014-05-28 19:46 UTC (permalink / raw)
To: linux-arm-kernel
Hello Mark,
On 27/05/2014 21:49, Mark Brown wrote:
> On Tue, May 27, 2014 at 11:38:49AM +0200, Boris BREZILLON wrote:
>
>> I'm still waiting for inputs regarding the ipsout regulator representation:
> If you've got questions you need an answer to urgently it's possibly
> best to highlight them outside of a series like this doing that helps
> make it more obvious.
Noted. I'll take care to send a separate mail next time.
BTW, I don't if you've noticed, but patch 5 of this series implements
helper functions to register several regulators from a regulator_desc
and an of_regulator_match table (as you suggested).
>> * The ipsout regulator is currently represented as a fixed regulator
>> providing a 5v output, and this is not exactly the case.
>> AFAIU (here's is the datasheet traduction if you want to check [1]), the
>> ipsout output is a multiplexer that choose among vbus (5V), acin (12V
>> which is then converted to 5V) and the battery power supply (3,5 -> 4,2 V).
>> This means the output voltage of ipsout vary between 3,5V and 5V.
>> How can we express this kind of muxer in the regulator framework (is there
>> already something available ?) ?
>> Note that the power supply selection is automatic, though we could force
>> one power supply, but then we loose the ability to unplug one power supply
>> without impacting the system.
> This sounds like it may as well just be represented as an unregulated
> supply - it's just the system root supply really. Nothing can rely on
> the voltage anyway as it's going to vary randomly depending on what the
> user does power wise and typically it'd only be used as a supply for
> things that don't care about the specific voltage.
Okay, I'm fine with that.
Thanks.
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 30+ messages in thread