Hi,
This patchset adds external GPIO control to s2mps11 regulator driver
for some of the S2MPS14 regulators.
Additionally (patch 4/4) it changes the bindings in s5m8767 regulator driver
for external control to match these used here:
"samsung,ext-control-gpios"
Best regards,
Krzysztof
Krzysztof Kozlowski (4):
regulator: s2mps11: Move DTS parsing code to separate function
regulator: s2mps11: Add external GPIO control for S2MPS14
Documentation: regulator: s2mps11: Document external GPIO control
regulator: s5m8767: Use same binding for external control as in
s2mps11
Documentation/devicetree/bindings/mfd/s2mps11.txt | 14 ++++
.../bindings/regulator/s5m8767-regulator.txt | 4 +-
drivers/regulator/s2mps11.c | 98 +++++++++++++++++++---
drivers/regulator/s5m8767.c | 2 +-
include/linux/mfd/samsung/s2mps14.h | 2 +
5 files changed, 105 insertions(+), 15 deletions(-)
--
1.8.3.2
Change the binding for regulators external control to the same used in
s2mps11 driver to be consistent.
Signed-off-by: Krzysztof Kozlowski <[email protected]>
Cc: Tomasz Figa <[email protected]>
Cc: [email protected]
Cc: Rob Herring <[email protected]>
Cc: Pawel Moll <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Ian Campbell <[email protected]>
Cc: Kumar Gala <[email protected]>
---
Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt | 4 ++--
drivers/regulator/s5m8767.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt b/Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt
index d290988ed975..558c80345ef0 100644
--- a/Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt
@@ -76,7 +76,7 @@ except these properties:
1 - on in normal mode
2 - low power mode
3 - suspend mode
- - s5m8767,pmic-ext-control-gpios: (optional) GPIO specifier for one
+ - samsung,ext-control-gpios: (optional) GPIO specifier for one
GPIO controlling this regulator (enable/disable); This is
valid only for buck9.
@@ -157,7 +157,7 @@ Example:
regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <2800000>;
op_mode = <3>; /* Standby Mode */
- s5m8767,pmic-ext-control-gpios = <&gpk0 2 0>;
+ samsung,ext-control-gpios = <&gpk0 2 0>;
};
};
};
diff --git a/drivers/regulator/s5m8767.c b/drivers/regulator/s5m8767.c
index 92f19a005dc3..dbfd98b04a77 100644
--- a/drivers/regulator/s5m8767.c
+++ b/drivers/regulator/s5m8767.c
@@ -534,7 +534,7 @@ static void s5m8767_pmic_dt_parse_ext_control_gpio(struct sec_pmic_dev *iodev,
struct device_node *reg_np)
{
rdata->ext_control_gpio = of_get_named_gpio(reg_np,
- "s5m8767,pmic-ext-control-gpios", 0);
+ "samsung,ext-control-gpios", 0);
if (!gpio_is_valid(rdata->ext_control_gpio))
rdata->ext_control_gpio = 0;
}
--
1.8.3.2
Add support for external control over GPIO for LDO10, LDO11 and LDO12
S2MPS14 regulators. External control can be turned on by writing 0x0 to
control register which in case of other regulators is used for disabling
them. These LDO10-LDO12 regulators can be disabled only by I2C GPIO or
PWREN pin so the patch actually allows proper way of disabling them.
Additionally the GPIO control has two benefits:
- It is faster than toggling it over I2C bus.
- It allows disabling the regulator during suspend to RAM; The AP will
enable it during resume.
Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
drivers/regulator/s2mps11.c | 67 +++++++++++++++++++++++++++++++++++--
include/linux/mfd/samsung/s2mps14.h | 2 ++
2 files changed, 67 insertions(+), 2 deletions(-)
diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
index 3aba0331fb5d..6dad0aa74a47 100644
--- a/drivers/regulator/s2mps11.c
+++ b/drivers/regulator/s2mps11.c
@@ -27,6 +27,7 @@
#include <linux/regulator/driver.h>
#include <linux/regulator/machine.h>
#include <linux/regulator/of_regulator.h>
+#include <linux/of_gpio.h>
#include <linux/mfd/samsung/core.h>
#include <linux/mfd/samsung/s2mps11.h>
#include <linux/mfd/samsung/s2mps14.h>
@@ -44,6 +45,8 @@ struct s2mps11_info {
* was enabled.
*/
unsigned int s2mps14_suspend_state:30;
+ /* Array of size rdev_num with GPIO-s for external sleep control */
+ int *ext_control_gpio;
};
static int get_ramp_delay(int ramp_delay)
@@ -409,6 +412,8 @@ static int s2mps14_regulator_enable(struct regulator_dev *rdev)
if (s2mps11->s2mps14_suspend_state & (1 << rdev_get_id(rdev)))
val = S2MPS14_ENABLE_SUSPEND;
+ else if (s2mps11->ext_control_gpio[rdev_get_id(rdev)])
+ val = S2MPS14_ENABLE_EXT_CONTROL;
else
val = rdev->desc->enable_mask;
@@ -565,9 +570,41 @@ static const struct regulator_desc s2mps14_regulators[] = {
regulator_desc_s2mps14_buck1235(5),
};
-static int s2mps11_pmic_dt_parse(struct platform_device *pdev,
+static int s2mps14_pmic_enable_ext_control(struct s2mps11_info *s2mps11,
+ struct regulator_dev *rdev)
+{
+ return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
+ rdev->desc->enable_mask, S2MPS14_ENABLE_EXT_CONTROL);
+}
+
+static void s2mps14_pmic_dt_parse_ext_control_gpio(struct platform_device *pdev,
struct of_regulator_match *rdata, struct s2mps11_info *s2mps11)
{
+ int *gpio = s2mps11->ext_control_gpio;
+ unsigned int i;
+ unsigned int valid_regulators[3] = { S2MPS14_LDO10, S2MPS14_LDO11,
+ S2MPS14_LDO12 };
+
+ for (i = 0; i < ARRAY_SIZE(valid_regulators); i++) {
+ unsigned int reg = valid_regulators[i];
+
+ if (!rdata[reg].init_data || !rdata[reg].of_node)
+ continue;
+
+ gpio[reg] = of_get_named_gpio(rdata[reg].of_node,
+ "samsung,ext-control-gpios", 0);
+ if (!gpio_is_valid(gpio[reg]))
+ gpio[reg] = 0;
+ else
+ dev_dbg(&pdev->dev, "Using GPIO %d for ext-control over %d/%s\n",
+ gpio[reg], reg, rdata[reg].name);
+ }
+}
+
+static int s2mps11_pmic_dt_parse(struct platform_device *pdev,
+ struct of_regulator_match *rdata, struct s2mps11_info *s2mps11,
+ enum sec_device_type dev_type)
+{
struct device_node *reg_np;
reg_np = of_get_child_by_name(pdev->dev.parent->of_node, "regulators");
@@ -577,6 +614,9 @@ static int s2mps11_pmic_dt_parse(struct platform_device *pdev,
}
of_regulator_match(&pdev->dev, reg_np, rdata, s2mps11->rdev_num);
+ if (dev_type == S2MPS14X)
+ s2mps14_pmic_dt_parse_ext_control_gpio(pdev, rdata, s2mps11);
+
of_node_put(reg_np);
return 0;
@@ -613,6 +653,12 @@ static int s2mps11_pmic_probe(struct platform_device *pdev)
return -EINVAL;
};
+ s2mps11->ext_control_gpio = devm_kzalloc(&pdev->dev,
+ sizeof(*s2mps11->ext_control_gpio) * s2mps11->rdev_num,
+ GFP_KERNEL);
+ if (!s2mps11->ext_control_gpio)
+ return -ENOMEM;
+
if (!iodev->dev->of_node) {
if (iodev->pdata) {
pdata = iodev->pdata;
@@ -631,7 +677,7 @@ static int s2mps11_pmic_probe(struct platform_device *pdev)
for (i = 0; i < s2mps11->rdev_num; i++)
rdata[i].name = regulators[i].name;
- ret = s2mps11_pmic_dt_parse(pdev, rdata, s2mps11);
+ ret = s2mps11_pmic_dt_parse(pdev, rdata, s2mps11, dev_type);
if (ret)
goto out;
@@ -652,6 +698,12 @@ common_reg:
config.of_node = rdata[i].of_node;
}
+ if (s2mps11->ext_control_gpio[i]) {
+ config.ena_gpio = s2mps11->ext_control_gpio[i];
+ config.ena_gpio_flags = GPIOF_OUT_INIT_HIGH;
+ } else
+ config.ena_gpio = config.ena_gpio_flags = 0;
+
regulator = devm_regulator_register(&pdev->dev,
®ulators[i], &config);
if (IS_ERR(regulator)) {
@@ -660,6 +712,17 @@ common_reg:
i);
goto out;
}
+
+ if (s2mps11->ext_control_gpio[i]) {
+ ret = s2mps14_pmic_enable_ext_control(s2mps11,
+ regulator);
+ if (ret < 0) {
+ dev_err(&pdev->dev,
+ "failed to enable GPIO control over %s: %d\n",
+ regulator->desc->name, ret);
+ goto out;
+ }
+ }
}
out:
diff --git a/include/linux/mfd/samsung/s2mps14.h b/include/linux/mfd/samsung/s2mps14.h
index 4b449b8ac548..900cd7a04314 100644
--- a/include/linux/mfd/samsung/s2mps14.h
+++ b/include/linux/mfd/samsung/s2mps14.h
@@ -148,6 +148,8 @@ enum s2mps14_regulators {
#define S2MPS14_ENABLE_SHIFT 6
/* On/Off controlled by PWREN */
#define S2MPS14_ENABLE_SUSPEND (0x01 << S2MPS14_ENABLE_SHIFT)
+/* On/Off controlled by LDO10EN or EMMCEN */
+#define S2MPS14_ENABLE_EXT_CONTROL (0x00 << S2MPS14_ENABLE_SHIFT)
#define S2MPS14_LDO_N_VOLTAGES (S2MPS14_LDO_VSEL_MASK + 1)
#define S2MPS14_BUCK_N_VOLTAGES (S2MPS14_BUCK_VSEL_MASK + 1)
--
1.8.3.2
Add documentation for new property for controlling (enable/disable) some
of the S2MPS14 regulators by GPIO.
Signed-off-by: Krzysztof Kozlowski <[email protected]>
Cc: Tomasz Figa <[email protected]>
Cc: [email protected]
Cc: Rob Herring <[email protected]>
Cc: Pawel Moll <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Ian Campbell <[email protected]>
Cc: Kumar Gala <[email protected]>
---
Documentation/devicetree/bindings/mfd/s2mps11.txt | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/Documentation/devicetree/bindings/mfd/s2mps11.txt b/Documentation/devicetree/bindings/mfd/s2mps11.txt
index 802e839b0829..d81ba30c0d8b 100644
--- a/Documentation/devicetree/bindings/mfd/s2mps11.txt
+++ b/Documentation/devicetree/bindings/mfd/s2mps11.txt
@@ -56,6 +56,20 @@ for a particular group of BUCKs. So provide same regulator-ramp-delay<value>.
Grouping of BUCKs sharing ramp rate setting is as follow : BUCK[1, 6],
BUCK[3, 4], and BUCK[7, 8, 10]
+On S2MPS14 the LDO10, LDO11 and LDO12 can be configured to external control
+over GPIO. To turn this feature on this property must be added to the regulator
+sub-node:
+ - samsung,ext-control-gpios: GPIO specifier for one GPIO
+ controlling this regulator (enable/disable);
+Example:
+ LDO12 {
+ regulator-name = "V_EMMC_2.8V";
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+ samsung,ext-control-gpios = <&gpk0 2 0>;
+ };
+
+
The regulator constraints inside the regulator nodes use the standard regulator
bindings which are documented elsewhere.
--
1.8.3.2
Refactor code for parsing DTS to increase a little code readability. The
behaviour should not change.
Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
drivers/regulator/s2mps11.c | 35 +++++++++++++++++++++++------------
1 file changed, 23 insertions(+), 12 deletions(-)
diff --git a/drivers/regulator/s2mps11.c b/drivers/regulator/s2mps11.c
index e713c162fbd4..3aba0331fb5d 100644
--- a/drivers/regulator/s2mps11.c
+++ b/drivers/regulator/s2mps11.c
@@ -565,12 +565,28 @@ static const struct regulator_desc s2mps14_regulators[] = {
regulator_desc_s2mps14_buck1235(5),
};
+static int s2mps11_pmic_dt_parse(struct platform_device *pdev,
+ struct of_regulator_match *rdata, struct s2mps11_info *s2mps11)
+{
+ struct device_node *reg_np;
+
+ reg_np = of_get_child_by_name(pdev->dev.parent->of_node, "regulators");
+ if (!reg_np) {
+ dev_err(&pdev->dev, "could not find regulators sub-node\n");
+ return -EINVAL;
+ }
+
+ of_regulator_match(&pdev->dev, reg_np, rdata, s2mps11->rdev_num);
+ of_node_put(reg_np);
+
+ return 0;
+}
+
static int s2mps11_pmic_probe(struct platform_device *pdev)
{
struct sec_pmic_dev *iodev = dev_get_drvdata(pdev->dev.parent);
- struct sec_platform_data *pdata = iodev->pdata;
+ struct sec_platform_data *pdata = NULL;
struct of_regulator_match *rdata = NULL;
- struct device_node *reg_np = NULL;
struct regulator_config config = { };
struct s2mps11_info *s2mps11;
int i, ret = 0;
@@ -598,7 +614,8 @@ static int s2mps11_pmic_probe(struct platform_device *pdev)
};
if (!iodev->dev->of_node) {
- if (pdata) {
+ if (iodev->pdata) {
+ pdata = iodev->pdata;
goto common_reg;
} else {
dev_err(pdev->dev.parent,
@@ -614,15 +631,9 @@ static int s2mps11_pmic_probe(struct platform_device *pdev)
for (i = 0; i < s2mps11->rdev_num; i++)
rdata[i].name = regulators[i].name;
- reg_np = of_get_child_by_name(iodev->dev->of_node, "regulators");
- if (!reg_np) {
- dev_err(&pdev->dev, "could not find regulators sub-node\n");
- ret = -EINVAL;
+ ret = s2mps11_pmic_dt_parse(pdev, rdata, s2mps11);
+ if (ret)
goto out;
- }
-
- of_regulator_match(&pdev->dev, reg_np, rdata, s2mps11->rdev_num);
- of_node_put(reg_np);
common_reg:
platform_set_drvdata(pdev, s2mps11);
@@ -633,7 +644,7 @@ common_reg:
for (i = 0; i < s2mps11->rdev_num; i++) {
struct regulator_dev *regulator;
- if (!reg_np) {
+ if (pdata) {
config.init_data = pdata->regulators[i].initdata;
config.of_node = pdata->regulators[i].reg_node;
} else {
--
1.8.3.2
On Mon, Apr 14, 2014 at 10:09:09AM +0200, Krzysztof Kozlowski wrote:
> - - s5m8767,pmic-ext-control-gpios: (optional) GPIO specifier for one
> + - samsung,ext-control-gpios: (optional) GPIO specifier for one
> GPIO controlling this regulator (enable/disable); This is
> valid only for buck9.
This is an incompatible change. It's OK to deprecate the old property
but it's bad form to just remove it.
On Mon, Apr 14, 2014 at 10:09:06AM +0200, Krzysztof Kozlowski wrote:
> Refactor code for parsing DTS to increase a little code readability. The
> behaviour should not change.
Applied, thanks.
On Mon, Apr 14, 2014 at 10:09:07AM +0200, Krzysztof Kozlowski wrote:
> Add support for external control over GPIO for LDO10, LDO11 and LDO12
> S2MPS14 regulators. External control can be turned on by writing 0x0 to
> control register which in case of other regulators is used for disabling
> them. These LDO10-LDO12 regulators can be disabled only by I2C GPIO or
> PWREN pin so the patch actually allows proper way of disabling them.
Applied, thanks.
On Mon, Apr 14, 2014 at 10:09:08AM +0200, Krzysztof Kozlowski wrote:
> Add documentation for new property for controlling (enable/disable) some
> of the S2MPS14 regulators by GPIO.
Applied, thanks.
On 15 April 2014 02:41, Mark Brown <[email protected]> wrote:
> On Mon, Apr 14, 2014 at 10:09:09AM +0200, Krzysztof Kozlowski wrote:
>
>> - - s5m8767,pmic-ext-control-gpios: (optional) GPIO specifier for one
>> + - samsung,ext-control-gpios: (optional) GPIO specifier for one
>> GPIO controlling this regulator (enable/disable); This is
>> valid only for buck9.
>
> This is an incompatible change. It's OK to deprecate the old property
> but it's bad form to just remove it.
I agree with Mark. Also, there is no need to make it generic.
--
With warm regards,
Sachin
On wto, 2014-04-15 at 13:26 +0530, Sachin Kamat wrote:
> On 15 April 2014 02:41, Mark Brown <[email protected]> wrote:
> > On Mon, Apr 14, 2014 at 10:09:09AM +0200, Krzysztof Kozlowski wrote:
> >
> >> - - s5m8767,pmic-ext-control-gpios: (optional) GPIO specifier for one
> >> + - samsung,ext-control-gpios: (optional) GPIO specifier for one
> >> GPIO controlling this regulator (enable/disable); This is
> >> valid only for buck9.
> >
> > This is an incompatible change. It's OK to deprecate the old property
> > but it's bad form to just remove it.
>
> I agree with Mark. Also, there is no need to make it generic.
I thought it would be good to make it consistent and to reduce the
number of bindings with same meaning on similar drivers. However I don't
mind skipping this patch.
Thanks for applying rest of patches.
Best regards,
Krzysztof
On 15 April 2014 13:42, Krzysztof Kozlowski <[email protected]> wrote:
> On wto, 2014-04-15 at 13:26 +0530, Sachin Kamat wrote:
>> On 15 April 2014 02:41, Mark Brown <[email protected]> wrote:
>> > On Mon, Apr 14, 2014 at 10:09:09AM +0200, Krzysztof Kozlowski wrote:
>> >
>> >> - - s5m8767,pmic-ext-control-gpios: (optional) GPIO specifier for one
>> >> + - samsung,ext-control-gpios: (optional) GPIO specifier for one
>> >> GPIO controlling this regulator (enable/disable); This is
>> >> valid only for buck9.
>> >
>> > This is an incompatible change. It's OK to deprecate the old property
>> > but it's bad form to just remove it.
>>
>> I agree with Mark. Also, there is no need to make it generic.
>
> I thought it would be good to make it consistent and to reduce the
> number of bindings with same meaning on similar drivers.
How about making the other one use "s5m8767,pmic-ext-control-gpios"
compatible instead of introducing a new one?
--
With warm regards,
Sachin
On wto, 2014-04-15 at 14:02 +0530, Sachin Kamat wrote:
> On 15 April 2014 13:42, Krzysztof Kozlowski <[email protected]> wrote:
> > On wto, 2014-04-15 at 13:26 +0530, Sachin Kamat wrote:
> >> On 15 April 2014 02:41, Mark Brown <[email protected]> wrote:
> >> > On Mon, Apr 14, 2014 at 10:09:09AM +0200, Krzysztof Kozlowski wrote:
> >> >
> >> >> - - s5m8767,pmic-ext-control-gpios: (optional) GPIO specifier for one
> >> >> + - samsung,ext-control-gpios: (optional) GPIO specifier for one
> >> >> GPIO controlling this regulator (enable/disable); This is
> >> >> valid only for buck9.
> >> >
> >> > This is an incompatible change. It's OK to deprecate the old property
> >> > but it's bad form to just remove it.
> >>
> >> I agree with Mark. Also, there is no need to make it generic.
> >
> > I thought it would be good to make it consistent and to reduce the
> > number of bindings with same meaning on similar drivers.
>
> How about making the other one use "s5m8767,pmic-ext-control-gpios"
> compatible instead of introducing a new one?
But then we would introduce semi-generic binding with a driver-specific
name.
Anyway more drivers seem to use this kind of binding (tps65090, max8952,
da9055, arizona) so maybe there is a point in making this generic?
Best regards,
Krzysztof
On 15 April 2014 14:25, Krzysztof Kozlowski <[email protected]> wrote:
> On wto, 2014-04-15 at 14:02 +0530, Sachin Kamat wrote:
>> On 15 April 2014 13:42, Krzysztof Kozlowski <[email protected]> wrote:
>> > On wto, 2014-04-15 at 13:26 +0530, Sachin Kamat wrote:
>> >> On 15 April 2014 02:41, Mark Brown <[email protected]> wrote:
>> >> > On Mon, Apr 14, 2014 at 10:09:09AM +0200, Krzysztof Kozlowski wrote:
>> >> >
>> >> >> - - s5m8767,pmic-ext-control-gpios: (optional) GPIO specifier for one
>> >> >> + - samsung,ext-control-gpios: (optional) GPIO specifier for one
>> >> >> GPIO controlling this regulator (enable/disable); This is
>> >> >> valid only for buck9.
>> >> >
>> >> > This is an incompatible change. It's OK to deprecate the old property
>> >> > but it's bad form to just remove it.
>> >>
>> >> I agree with Mark. Also, there is no need to make it generic.
>> >
>> > I thought it would be good to make it consistent and to reduce the
>> > number of bindings with same meaning on similar drivers.
>>
>> How about making the other one use "s5m8767,pmic-ext-control-gpios"
>> compatible instead of introducing a new one?
>
> But then we would introduce semi-generic binding with a driver-specific
> name.
We can have a IP specific name (first IP to have this property) common
across family of IPs.
>
> Anyway more drivers seem to use this kind of binding (tps65090, max8952,
> da9055, arizona) so maybe there is a point in making this generic?
In that case we could but probably not with samsung prefix.
--
With warm regards,
Sachin
On Tue, Apr 15, 2014 at 10:55:45AM +0200, Krzysztof Kozlowski wrote:
> Anyway more drivers seem to use this kind of binding (tps65090, max8952,
> da9055, arizona) so maybe there is a point in making this generic?
Yes.