| Message ID | 1397462949-22379-5-git-send-email-k.kozlowski@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 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.
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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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?
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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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.
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; }
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(-)