[PATCH 0/4] regulator: s2mps11: Add external GPIO control for S2MPS14

[PATCH 0/4] regulator: s2mps11: Add external GPIO control for S2MPS14

[Krzysztof Kozlowski]

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

[PATCH 1/4] regulator: s2mps11: Move DTS parsing code to separate function

[Krzysztof Kozlowski]

Refactor code for parsing DTS to increase a little code readability. The
behaviour should not change.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 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

[PATCH 2/4] regulator: s2mps11: Add external GPIO control for S2MPS14

[Krzysztof Kozlowski]

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 <k.kozlowski@samsung.com>
---
 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,
 						&regulators[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

[PATCH 3/4] Documentation: regulator: s2mps11: Document external GPIO control

[Krzysztof Kozlowski]

Add documentation for new property for controlling (enable/disable) some
of the S2MPS14 regulators by GPIO.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Tomasz Figa <t.figa@samsung.com>
Cc: devicetree@vger.kernel.org
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
---
 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

[PATCH 4/4] regulator: s5m8767: Use same binding for external control as in s2mps11

[Krzysztof Kozlowski]

Change the binding for regulators external control to the same used in
s2mps11 driver to be consistent.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Tomasz Figa <t.figa@samsung.com>
Cc: devicetree@vger.kernel.org
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
---
 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

Re: [PATCH 4/4] regulator: s5m8767: Use same binding for external control as in s2mps11

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 408 bytes --]

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.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/4] regulator: s2mps11: Move DTS parsing code to separate function

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 193 bytes --]

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.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/4] regulator: s2mps11: Add external GPIO control for S2MPS14

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 452 bytes --]

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.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/4] Documentation: regulator: s2mps11: Document external GPIO control

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 199 bytes --]

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.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 4/4] regulator: s5m8767: Use same binding for external control as in s2mps11

[Sachin Kamat]

On 15 April 2014 02:41, Mark Brown <broonie@kernel.org> 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

Re: [PATCH 4/4] regulator: s5m8767: Use same binding for external control as in s2mps11

[Krzysztof Kozlowski]

On wto, 2014-04-15 at 13:26 +0530, Sachin Kamat wrote:
> On 15 April 2014 02:41, Mark Brown <broonie@kernel.org> 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

Re: [PATCH 4/4] regulator: s5m8767: Use same binding for external control as in s2mps11

[Sachin Kamat]

On 15 April 2014 13:42, Krzysztof Kozlowski <k.kozlowski@samsung.com> wrote:
> On wto, 2014-04-15 at 13:26 +0530, Sachin Kamat wrote:
>> On 15 April 2014 02:41, Mark Brown <broonie@kernel.org> 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

Re: [PATCH 4/4] regulator: s5m8767: Use same binding for external control as in s2mps11

[Krzysztof Kozlowski]

On wto, 2014-04-15 at 14:02 +0530, Sachin Kamat wrote:
> On 15 April 2014 13:42, Krzysztof Kozlowski <k.kozlowski@samsung.com> wrote:
> > On wto, 2014-04-15 at 13:26 +0530, Sachin Kamat wrote:
> >> On 15 April 2014 02:41, Mark Brown <broonie@kernel.org> 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

Re: [PATCH 4/4] regulator: s5m8767: Use same binding for external control as in s2mps11

[Sachin Kamat]

On 15 April 2014 14:25, Krzysztof Kozlowski <k.kozlowski@samsung.com> wrote:
> On wto, 2014-04-15 at 14:02 +0530, Sachin Kamat wrote:
>> On 15 April 2014 13:42, Krzysztof Kozlowski <k.kozlowski@samsung.com> wrote:
>> > On wto, 2014-04-15 at 13:26 +0530, Sachin Kamat wrote:
>> >> On 15 April 2014 02:41, Mark Brown <broonie@kernel.org> 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

Re: [PATCH 4/4] regulator: s5m8767: Use same binding for external control as in s2mps11

[Sachin Kamat]

On 15 April 2014 14:25, Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> On wto, 2014-04-15 at 14:02 +0530, Sachin Kamat wrote:
>> On 15 April 2014 13:42, Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
>> > On wto, 2014-04-15 at 13:26 +0530, Sachin Kamat wrote:
>> >> On 15 April 2014 02:41, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] regulator: s5m8767: Use same binding for external control as in s2mps11

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 220 bytes --]

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.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]