[PATCH 0/2] Add input voltage configuration for TPS65218

[PATCH 0/2] Add input voltage configuration for TPS65218

[Christian Hohnstaedt]

This patch allows to configure input-voltage settings
of the TPS65218 regulator via device tree.

Christian Hohnstaedt (2):
  dt-bindings: regulator: extend tps65218 bindings
  mfd: tps65218.c: Add input voltage options

 .../devicetree/bindings/regulator/tps65218.txt     | 10 +++++
 drivers/mfd/tps65218.c                             | 46 ++++++++++++++++++++++
 2 files changed, 56 insertions(+)

-- 
2.7.4

[PATCH 1/2] dt-bindings: regulator: extend tps65218 bindings

[Christian Hohnstaedt]

Add input voltage configuration options

Signed-off-by: Christian Hohnstaedt <Christian.Hohnstaedt@wago.com>
---
 Documentation/devicetree/bindings/regulator/tps65218.txt | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/tps65218.txt b/Documentation/devicetree/bindings/regulator/tps65218.txt
index 02f0e9b..a9ee377 100644
--- a/Documentation/devicetree/bindings/regulator/tps65218.txt
+++ b/Documentation/devicetree/bindings/regulator/tps65218.txt
@@ -16,12 +16,22 @@ Required properties:
   regulator-dcdc5, regulator-dcdc6, regulator-ldo1, regulator-ls3.
   Each regulator is defined using the standard binding for regulators.
 
+Optional properties:
+  If any of these properties is absent, the setting will not be touched.
+- strict-supply-voltage: <1|0> Set/Reset STRICT flag in CONFIG1
+- under-voltage-limit: Select 2.75, 2.95, 3.25 or 3.35V as UVLO in CONFIG1
+- under-voltage-hysteresis: Select 200mV or 400mV UVLOHYS in CONFIG2
+
 Example:
 tps65218: tps65218@24 {
 	reg = <0x24>;
 	compatible = "ti,tps65218";
 	interrupts = <GIC_SPI 7 IRQ_TYPE_NONE>; /* NMIn */
 	interrupt-controller;
+	strict-supply-voltage = <1>;
+	under-voltage-hysteresis = <400000>;
+	under-voltage-limit = <3350000>;
+
 	#interrupt-cells = <2>;
 
 	dcdc1: regulator-dcdc1 {
-- 
2.7.4

[PATCH 2/2] mfd: tps65218.c: Add input voltage options

[Christian Hohnstaedt]

These options apply to all regulators in this chip.

strict-supply-voltage:
  Set STRICT flag in CONFIG1
under-voltage-limit:
  Select 2.75, 2.95, 3.25 or 3.35 V UVLO in CONFIG1
under-voltage-hysteresis:
  Select 200mV or 400mV UVLOHYS in CONFIG2

Signed-off-by: Christian Hohnstaedt <Christian.Hohnstaedt@wago.com>
---
 drivers/mfd/tps65218.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/drivers/mfd/tps65218.c b/drivers/mfd/tps65218.c
index 8bcdecf..f5e559b 100644
--- a/drivers/mfd/tps65218.c
+++ b/drivers/mfd/tps65218.c
@@ -211,6 +211,50 @@ static const struct of_device_id of_tps65218_match_table[] = {
 };
 MODULE_DEVICE_TABLE(of, of_tps65218_match_table);
 
+static void tps65218_options(struct tps65218 *tps)
+{
+	struct device *dev = tps->dev;
+	struct device_node *np = dev->of_node;
+	u32 pval;
+
+	if (!of_property_read_u32(np, "strict-supply-voltage", &pval)) {
+		tps65218_update_bits(tps, TPS65218_REG_CONFIG1,
+			TPS65218_CONFIG1_STRICT,
+			pval ? TPS65218_CONFIG1_STRICT : 0,
+			TPS65218_PROTECT_L1);
+		dev_dbg(dev, "tps65218 strict-supply-voltage: %d\n", pval);
+	}
+	if (!of_property_read_u32(np, "under-voltage-hysteresis", &pval)) {
+		if (pval != 400000 && pval != 200000) {
+			dev_err(dev,
+				 "under-voltage-hysteresis must be %d or %d\n",
+				 200000, 400000);
+		} else {
+			tps65218_update_bits(tps, TPS65218_REG_CONFIG2,
+				TPS65218_CONFIG2_UVLOHYS,
+				pval == 400000 ? TPS65218_CONFIG2_UVLOHYS : 0,
+				TPS65218_PROTECT_L1);
+		}
+		dev_dbg(dev, "tps65218 under-voltage-hysteresis: %d\n", pval);
+	}
+	if (!of_property_read_u32(np, "under-voltage-limit", &pval)) {
+		int i, vals[] = { 275, 295, 325, 335 };
+
+		for (i = 0; i < ARRAY_SIZE(vals); i++) {
+			if (pval == vals[i] * 10000)
+				break;
+		}
+		if (i < ARRAY_SIZE(vals)) {
+			tps65218_update_bits(tps, TPS65218_REG_CONFIG1,
+				TPS65218_CONFIG1_UVLO_MASK, i,
+				TPS65218_PROTECT_L1);
+		} else {
+			dev_err(dev, "Invalid under-voltage-limit: %d\n", pval);
+		}
+		dev_dbg(dev, "tps65218 under-voltage-limit: %d=%d\n", pval, i);
+	}
+}
+
 static int tps65218_probe(struct i2c_client *client,
 				const struct i2c_device_id *ids)
 {
@@ -249,6 +293,8 @@ static int tps65218_probe(struct i2c_client *client,
 
 	tps->rev = chipid & TPS65218_CHIPID_REV_MASK;
 
+	tps65218_options(tps);
+
 	ret = mfd_add_devices(tps->dev, PLATFORM_DEVID_AUTO, tps65218_cells,
 			      ARRAY_SIZE(tps65218_cells), NULL, 0,
 			      regmap_irq_get_domain(tps->irq_data));
-- 
2.7.4

Re: [PATCH 2/2] mfd: tps65218.c: Add input voltage options

[Lee Jones]

On Tue, 18 Dec 2018, Christian Hohnstaedt wrote:

> These options apply to all regulators in this chip.
> 
> strict-supply-voltage:
>   Set STRICT flag in CONFIG1
> under-voltage-limit:
>   Select 2.75, 2.95, 3.25 or 3.35 V UVLO in CONFIG1
> under-voltage-hysteresis:
>   Select 200mV or 400mV UVLOHYS in CONFIG2
> 
> Signed-off-by: Christian Hohnstaedt <Christian.Hohnstaedt@wago.com>
> ---
>  drivers/mfd/tps65218.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)

This needs a close review by Tony and any of the other OMAP guys.

At the very least, please put '\n's between the if() statements.  You
also need to return after an error print, else I suggest it's not an
error.

It would also look tidier if you changed the if()s to one liners to
assign to different variables, then dealt with them separately later
on.  The way it's done here looks messy to say the least.

> diff --git a/drivers/mfd/tps65218.c b/drivers/mfd/tps65218.c
> index 8bcdecf..f5e559b 100644
> --- a/drivers/mfd/tps65218.c
> +++ b/drivers/mfd/tps65218.c
> @@ -211,6 +211,50 @@ static const struct of_device_id of_tps65218_match_table[] = {
>  };
>  MODULE_DEVICE_TABLE(of, of_tps65218_match_table);
>  
> +static void tps65218_options(struct tps65218 *tps)
> +{
> +	struct device *dev = tps->dev;
> +	struct device_node *np = dev->of_node;
> +	u32 pval;

What does pval mean?  I suggest just val is more common.

> +	if (!of_property_read_u32(np, "strict-supply-voltage", &pval)) {
> +		tps65218_update_bits(tps, TPS65218_REG_CONFIG1,
> +			TPS65218_CONFIG1_STRICT,
> +			pval ? TPS65218_CONFIG1_STRICT : 0,
> +			TPS65218_PROTECT_L1);
> +		dev_dbg(dev, "tps65218 strict-supply-voltage: %d\n", pval);
> +	}
> +	if (!of_property_read_u32(np, "under-voltage-hysteresis", &pval)) {
> +		if (pval != 400000 && pval != 200000) {
> +			dev_err(dev,
> +				 "under-voltage-hysteresis must be %d or %d\n",
> +				 200000, 400000);
> +		} else {
> +			tps65218_update_bits(tps, TPS65218_REG_CONFIG2,
> +				TPS65218_CONFIG2_UVLOHYS,
> +				pval == 400000 ? TPS65218_CONFIG2_UVLOHYS : 0,
> +				TPS65218_PROTECT_L1);
> +		}
> +		dev_dbg(dev, "tps65218 under-voltage-hysteresis: %d\n", pval);
> +	}
> +	if (!of_property_read_u32(np, "under-voltage-limit", &pval)) {
> +		int i, vals[] = { 275, 295, 325, 335 };
> +
> +		for (i = 0; i < ARRAY_SIZE(vals); i++) {
> +			if (pval == vals[i] * 10000)

Just use the full value in 'vals'.

> +				break;
> +		}

It took me a few seconds to realise what you're doing here.

I think a switch() statement would be cleaner.

You should also #define the values.

TPS65218_UNDER_VOLT_LIM_2750000 0

Etc.

> +		if (i < ARRAY_SIZE(vals)) {
> +			tps65218_update_bits(tps, TPS65218_REG_CONFIG1,
> +				TPS65218_CONFIG1_UVLO_MASK, i,
> +				TPS65218_PROTECT_L1);
> +		} else {
> +			dev_err(dev, "Invalid under-voltage-limit: %d\n", pval);

This could go in the default: section.

> +		}
> +		dev_dbg(dev, "tps65218 under-voltage-limit: %d=%d\n", pval, i);

I suggest considering removing these.

> +	}
> +}
> +
>  static int tps65218_probe(struct i2c_client *client,
>  				const struct i2c_device_id *ids)
>  {
> @@ -249,6 +293,8 @@ static int tps65218_probe(struct i2c_client *client,
>  
>  	tps->rev = chipid & TPS65218_CHIPID_REV_MASK;
>  
> +	tps65218_options(tps);

Options is not good nomenclature as it doesn't really tell us
anything.  Looks like all the values are voltage related to me?

>  	ret = mfd_add_devices(tps->dev, PLATFORM_DEVID_AUTO, tps65218_cells,
>  			      ARRAY_SIZE(tps65218_cells), NULL, 0,
>  			      regmap_irq_get_domain(tps->irq_data));

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 2/2] mfd: tps65218.c: Add input voltage options

[Tony Lindgren]

* Lee Jones <lee.jones@linaro.org> [181221 11:01]:
> On Tue, 18 Dec 2018, Christian Hohnstaedt wrote:
> 
> > These options apply to all regulators in this chip.
> > 
> > strict-supply-voltage:
> >   Set STRICT flag in CONFIG1
> > under-voltage-limit:
> >   Select 2.75, 2.95, 3.25 or 3.35 V UVLO in CONFIG1
> > under-voltage-hysteresis:
> >   Select 200mV or 400mV UVLOHYS in CONFIG2
> > 
> > Signed-off-by: Christian Hohnstaedt <Christian.Hohnstaedt@wago.com>
> > ---
> >  drivers/mfd/tps65218.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 46 insertions(+)
> 
> This needs a close review by Tony and any of the other OMAP guys.

Adding Keerthy to Cc, he's probably the best person to review these
changes.

Regards,

Tony

Re: [PATCH 2/2] mfd: tps65218.c: Add input voltage options

[Lee Jones]

On Sun, 23 Dec 2018, Tony Lindgren wrote:

> * Lee Jones <lee.jones@linaro.org> [181221 11:01]:
> > On Tue, 18 Dec 2018, Christian Hohnstaedt wrote:
> > 
> > > These options apply to all regulators in this chip.
> > > 
> > > strict-supply-voltage:
> > >   Set STRICT flag in CONFIG1
> > > under-voltage-limit:
> > >   Select 2.75, 2.95, 3.25 or 3.35 V UVLO in CONFIG1
> > > under-voltage-hysteresis:
> > >   Select 200mV or 400mV UVLOHYS in CONFIG2
> > > 
> > > Signed-off-by: Christian Hohnstaedt <Christian.Hohnstaedt@wago.com>
> > > ---
> > >  drivers/mfd/tps65218.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 46 insertions(+)
> > 
> > This needs a close review by Tony and any of the other OMAP guys.
> 
> Adding Keerthy to Cc, he's probably the best person to review these
> changes.

But you've dropped the patch, so all he has it a commit log to review?

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 2/2] mfd: tps65218.c: Add input voltage options

[J, KEERTHY]

On 12/24/2018 12:02 PM, Lee Jones wrote:
> On Sun, 23 Dec 2018, Tony Lindgren wrote:
> 
>> * Lee Jones <lee.jones@linaro.org> [181221 11:01]:
>>> On Tue, 18 Dec 2018, Christian Hohnstaedt wrote:
>>>
>>>> These options apply to all regulators in this chip.
>>>>
>>>> strict-supply-voltage:
>>>>    Set STRICT flag in CONFIG1
>>>> under-voltage-limit:
>>>>    Select 2.75, 2.95, 3.25 or 3.35 V UVLO in CONFIG1
>>>> under-voltage-hysteresis:
>>>>    Select 200mV or 400mV UVLOHYS in CONFIG2
>>>>
>>>> Signed-off-by: Christian Hohnstaedt <Christian.Hohnstaedt@wago.com>
>>>> ---
>>>>   drivers/mfd/tps65218.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>>>>   1 file changed, 46 insertions(+)
>>>
>>> This needs a close review by Tony and any of the other OMAP guys.
>>
>> Adding Keerthy to Cc, he's probably the best person to review these
>> changes.

Thanks Tony.

> 
> But you've dropped the patch, so all he has it a commit log to review?
> 
Lee Jones,

I am subscribed to linux-omap so i have the patch. I will review them.

Thanks,
Keerthy

Re: [PATCH 1/2] dt-bindings: regulator: extend tps65218 bindings

[J, KEERTHY]

On 12/18/2018 1:35 PM, Christian Hohnstaedt wrote:
> Add input voltage configuration options
> 
> Signed-off-by: Christian Hohnstaedt <Christian.Hohnstaedt@wago.com>
> ---
>   Documentation/devicetree/bindings/regulator/tps65218.txt | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/regulator/tps65218.txt b/Documentation/devicetree/bindings/regulator/tps65218.txt
> index 02f0e9b..a9ee377 100644
> --- a/Documentation/devicetree/bindings/regulator/tps65218.txt
> +++ b/Documentation/devicetree/bindings/regulator/tps65218.txt
> @@ -16,12 +16,22 @@ Required properties:
>     regulator-dcdc5, regulator-dcdc6, regulator-ldo1, regulator-ls3.
>     Each regulator is defined using the standard binding for regulators.
>   
> +Optional properties:
> +  If any of these properties is absent, the setting will not be touched.

I would reword this to:

"If any of these properties is absent then the corresponding setting 
will be untouched"

> +- strict-supply-voltage: <1|0> Set/Reset STRICT flag in CONFIG1

This is too less info on the strict bit:

You can perhaps add some more description from data sheet.

The Supply Voltage supervisor has two modes of operation, controlled by 
the STRICT bit. With the STRICT bit set to 0, all
enabled rails of the five regulators are monitored for undervoltage only 
with relaxed thresholds and
deglitch times. With the STRCT bit set to 1, all enabled rails of the 
five regulators are monitored for
undervoltage and overvoltage with tight limits and short deglitch times.

> +- under-voltage-limit: Select 2.75, 2.95, 3.25 or 3.35V as UVLO in CONFIG1

Again some more description on uvlo please.

> +- under-voltage-hysteresis: Select 200mV or 400mV UVLOHYS in CONFIG2

Even here for hysteresis.

> +
>   Example:
>   tps65218: tps65218@24 {
>   	reg = <0x24>;
>   	compatible = "ti,tps65218";
>   	interrupts = <GIC_SPI 7 IRQ_TYPE_NONE>; /* NMIn */
>   	interrupt-controller;
> +	strict-supply-voltage = <1>;
> +	under-voltage-hysteresis = <400000>;
> +	under-voltage-limit = <3350000>;
> +
>   	#interrupt-cells = <2>;
>   
>   	dcdc1: regulator-dcdc1 {
> 

Re: [PATCH 2/2] mfd: tps65218.c: Add input voltage options

[J, KEERTHY]

On 12/21/2018 4:31 PM, Lee Jones wrote:
> On Tue, 18 Dec 2018, Christian Hohnstaedt wrote:
> 
>> These options apply to all regulators in this chip.
>>
>> strict-supply-voltage:
>>    Set STRICT flag in CONFIG1
>> under-voltage-limit:
>>    Select 2.75, 2.95, 3.25 or 3.35 V UVLO in CONFIG1
>> under-voltage-hysteresis:
>>    Select 200mV or 400mV UVLOHYS in CONFIG2
>>
>> Signed-off-by: Christian Hohnstaedt <Christian.Hohnstaedt@wago.com>
>> ---
>>   drivers/mfd/tps65218.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 46 insertions(+)
> 
> This needs a close review by Tony and any of the other OMAP guys.
> 
> At the very least, please put '\n's between the if() statements.  You
> also need to return after an error print, else I suggest it's not an
> error.
> 
> It would also look tidier if you changed the if()s to one liners to
> assign to different variables, then dealt with them separately later
> on.  The way it's done here looks messy to say the least.
> 
>> diff --git a/drivers/mfd/tps65218.c b/drivers/mfd/tps65218.c
>> index 8bcdecf..f5e559b 100644
>> --- a/drivers/mfd/tps65218.c
>> +++ b/drivers/mfd/tps65218.c
>> @@ -211,6 +211,50 @@ static const struct of_device_id of_tps65218_match_table[] = {
>>   };
>>   MODULE_DEVICE_TABLE(of, of_tps65218_match_table);
>>   
>> +static void tps65218_options(struct tps65218 *tps)
>> +{
>> +	struct device *dev = tps->dev;
>> +	struct device_node *np = dev->of_node;
>> +	u32 pval;
> 
> What does pval mean?  I suggest just val is more common.
> 
>> +	if (!of_property_read_u32(np, "strict-supply-voltage", &pval)) {
>> +		tps65218_update_bits(tps, TPS65218_REG_CONFIG1,
>> +			TPS65218_CONFIG1_STRICT,
>> +			pval ? TPS65218_CONFIG1_STRICT : 0,
>> +			TPS65218_PROTECT_L1);
>> +		dev_dbg(dev, "tps65218 strict-supply-voltage: %d\n", pval);
>> +	}
>> +	if (!of_property_read_u32(np, "under-voltage-hysteresis", &pval)) {
>> +		if (pval != 400000 && pval != 200000) {
>> +			dev_err(dev,
>> +				 "under-voltage-hysteresis must be %d or %d\n",
>> +				 200000, 400000);
>> +		} else {
>> +			tps65218_update_bits(tps, TPS65218_REG_CONFIG2,
>> +				TPS65218_CONFIG2_UVLOHYS,
>> +				pval == 400000 ? TPS65218_CONFIG2_UVLOHYS : 0,
>> +				TPS65218_PROTECT_L1);
>> +		}
>> +		dev_dbg(dev, "tps65218 under-voltage-hysteresis: %d\n", pval);
>> +	}
>> +	if (!of_property_read_u32(np, "under-voltage-limit", &pval)) {
>> +		int i, vals[] = { 275, 295, 325, 335 };
>> +
>> +		for (i = 0; i < ARRAY_SIZE(vals); i++) {
>> +			if (pval == vals[i] * 10000)
> 
> Just use the full value in 'vals'.
> 
>> +				break;
>> +		}
> 
> It took me a few seconds to realise what you're doing here.
> 
> I think a switch() statement would be cleaner.
> 
> You should also #define the values.
> 
> TPS65218_UNDER_VOLT_LIM_2750000 0
> 
> Etc.
> 
>> +		if (i < ARRAY_SIZE(vals)) {
>> +			tps65218_update_bits(tps, TPS65218_REG_CONFIG1,
>> +				TPS65218_CONFIG1_UVLO_MASK, i,
>> +				TPS65218_PROTECT_L1);
>> +		} else {
>> +			dev_err(dev, "Invalid under-voltage-limit: %d\n", pval);
> 
> This could go in the default: section.
> 
>> +		}
>> +		dev_dbg(dev, "tps65218 under-voltage-limit: %d=%d\n", pval, i);
> 
> I suggest considering removing these.
> 
>> +	}
>> +}
>> +
>>   static int tps65218_probe(struct i2c_client *client,
>>   				const struct i2c_device_id *ids)
>>   {
>> @@ -249,6 +293,8 @@ static int tps65218_probe(struct i2c_client *client,
>>   
>>   	tps->rev = chipid & TPS65218_CHIPID_REV_MASK;
>>   
>> +	tps65218_options(tps);
> 
> Options is not good nomenclature as it doesn't really tell us
> anything.  Looks like all the values are voltage related to me?

Can we simply call them tps65218_voltage_set_strict, 
tps65218_voltage_set_uvlo, tps65218_voltage_set_uv_hyst?

or if you want them under one function then i would suggest 
tps65218_set_voltage_quirks or something like that.

- Keerthy
> 
>>   	ret = mfd_add_devices(tps->dev, PLATFORM_DEVID_AUTO, tps65218_cells,
>>   			      ARRAY_SIZE(tps65218_cells), NULL, 0,
>>   			      regmap_irq_get_domain(tps->irq_data));
> 

Re: [PATCH 1/2] dt-bindings: regulator: extend tps65218 bindings

[Rob Herring]

On Tue, Dec 18, 2018 at 09:05:55AM +0100, Christian Hohnstaedt wrote:
> Add input voltage configuration options
> 
> Signed-off-by: Christian Hohnstaedt <Christian.Hohnstaedt@wago.com>
> ---
>  Documentation/devicetree/bindings/regulator/tps65218.txt | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/regulator/tps65218.txt b/Documentation/devicetree/bindings/regulator/tps65218.txt
> index 02f0e9b..a9ee377 100644
> --- a/Documentation/devicetree/bindings/regulator/tps65218.txt
> +++ b/Documentation/devicetree/bindings/regulator/tps65218.txt
> @@ -16,12 +16,22 @@ Required properties:
>    regulator-dcdc5, regulator-dcdc6, regulator-ldo1, regulator-ls3.
>    Each regulator is defined using the standard binding for regulators.
>  
> +Optional properties:
> +  If any of these properties is absent, the setting will not be touched.
> +- strict-supply-voltage: <1|0> Set/Reset STRICT flag in CONFIG1
> +- under-voltage-limit: Select 2.75, 2.95, 3.25 or 3.35V as UVLO in CONFIG1
> +- under-voltage-hysteresis: Select 200mV or 400mV UVLOHYS in CONFIG2

These need vendor prefixes and unit suffixes (as defined in 
property-units.txt).

> +
>  Example:
>  tps65218: tps65218@24 {
>  	reg = <0x24>;
>  	compatible = "ti,tps65218";
>  	interrupts = <GIC_SPI 7 IRQ_TYPE_NONE>; /* NMIn */
>  	interrupt-controller;
> +	strict-supply-voltage = <1>;
> +	under-voltage-hysteresis = <400000>;
> +	under-voltage-limit = <3350000>;
> +
>  	#interrupt-cells = <2>;
>  
>  	dcdc1: regulator-dcdc1 {
> -- 
> 2.7.4
> 

[PATCH v2 0/2] Add input voltage configuration for TPS65218

[Christian Hohnstaedt]

This patch allows to configure input-voltage settings
of the TPS65218 regulator via device tree.

Changes for v2:
 - handle the different options in separate functions
 - add "ti," prefix and "-microvolt" postfix
 - remove debug prints
 - abort with an error in case of invalid values.
 - extend documentation of the options (taken from the datasheet)

Christian Hohnstaedt (2):
  dt-bindings: regulator: extend tps65218 bindings
  mfd: tps65218.c: Add input voltage options

 .../devicetree/bindings/regulator/tps65218.txt     | 22 ++++++
 drivers/mfd/tps65218.c                             | 87 ++++++++++++++++++++++
 include/linux/mfd/tps65218.h                       |  4 +
 3 files changed, 113 insertions(+)

-- 
2.7.4

[PATCH v2 1/2] dt-bindings: regulator: extend tps65218 bindings

[Christian Hohnstaedt]

Add input voltage configuration options

Signed-off-by: Christian Hohnstaedt <Christian.Hohnstaedt@wago.com>
---
 .../devicetree/bindings/regulator/tps65218.txt     | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/tps65218.txt b/Documentation/devicetree/bindings/regulator/tps65218.txt
index 02f0e9b..982597d 100644
--- a/Documentation/devicetree/bindings/regulator/tps65218.txt
+++ b/Documentation/devicetree/bindings/regulator/tps65218.txt
@@ -16,12 +16,34 @@ Required properties:
   regulator-dcdc5, regulator-dcdc6, regulator-ldo1, regulator-ls3.
   Each regulator is defined using the standard binding for regulators.
 
+Optional properties:
+  If any of these properties is absent then the corresponding setting will be
+  untouched.
+- ti,strict-supply-voltage-supervision: Set/Reset STRICT flag in CONFIG1.
+  The supervisor has two modes of operation, controlled by the STRICT bit.
+  With the STRICT bit set to 0, all five rails are monitored for under-voltage
+  only with relaxed thresholds and deglitch times.
+  With the STRCT bit set to 1, all five rails are monitored for under-voltage
+  and over-voltage with tight limits and short deglitch times.
+- ti,under-voltage-limit-microvolt: Configures UVLO in CONFIG1.
+  Valid values are: 2750000, 2950000, 3250000, 3350000
+- ti,under-voltage-hyst-microvolt: Select 200mV or 400mV UVLOHYS in CONFIG2
+  Power rails are only enabled if the input voltage measured at the IN_BIAS pin
+  is greater than the under-voltage lockout threshold plus hysteresis
+  (UVLO + UVLOHYS). Once the input voltage rises above this level,
+  the input voltage may drop to the UVLO level before the PMIC shuts down.
+  UVLO is deglitched by 5 ms on rising and falling edge.
+
 Example:
 tps65218: tps65218@24 {
 	reg = <0x24>;
 	compatible = "ti,tps65218";
 	interrupts = <GIC_SPI 7 IRQ_TYPE_NONE>; /* NMIn */
 	interrupt-controller;
+	ti,strict-supply-voltage-supervision = <1>;
+	ti,under-voltage-hyst-microvolt = <400000>;
+	ti,under-voltage-limit-microvolt = <3350000>;
+
 	#interrupt-cells = <2>;
 
 	dcdc1: regulator-dcdc1 {
-- 
2.7.4

[PATCH v2 2/2] mfd: tps65218.c: Add input voltage options

[Christian Hohnstaedt]

These options apply to all regulators in this chip.

ti,strict-supply-voltage-supervision:
  Set STRICT flag in CONFIG1
ti,under-voltage-limit-microvolt:
  Select 2.75, 2.95, 3.25 or 3.35 V UVLO in CONFIG1
ti,under-voltage-hyst-microvolt:
  Select 200mV or 400mV UVLOHYS in CONFIG2

Signed-off-by: Christian Hohnstaedt <Christian.Hohnstaedt@wago.com>
---
 drivers/mfd/tps65218.c       | 87 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/tps65218.h |  4 ++
 2 files changed, 91 insertions(+)

diff --git a/drivers/mfd/tps65218.c b/drivers/mfd/tps65218.c
index 8bcdecf..b5e0f3a 100644
--- a/drivers/mfd/tps65218.c
+++ b/drivers/mfd/tps65218.c
@@ -211,6 +211,81 @@ static const struct of_device_id of_tps65218_match_table[] = {
 };
 MODULE_DEVICE_TABLE(of, of_tps65218_match_table);
 
+static int tps65218_voltage_set_strict(struct tps65218 *tps)
+{
+	u32 strict;
+
+	if (of_property_read_u32(tps->dev->of_node,
+			"ti,strict-supply-voltage-supervision", &strict))
+		return 0;
+
+	if (strict != 0 && strict != 1) {
+		dev_err(tps->dev,
+			"Invalid ti,strict-supply-voltage-supervision value\n");
+		return -EINVAL;
+	}
+
+	tps65218_update_bits(tps, TPS65218_REG_CONFIG1,
+		TPS65218_CONFIG1_STRICT,
+		strict ? TPS65218_CONFIG1_STRICT : 0, TPS65218_PROTECT_L1);
+	return 0;
+}
+
+static int tps65218_voltage_set_uv_hyst(struct tps65218 *tps)
+{
+	u32 hyst;
+
+	if (of_property_read_u32(tps->dev->of_node,
+			"ti,under-voltage-hyst-microvolt", &hyst))
+		return 0;
+
+	if (hyst != 400000 && hyst != 200000) {
+		dev_err(tps->dev,
+			"Invalid ti,under-voltage-hyst-microvolt value\n");
+		return -EINVAL;
+	}
+
+	tps65218_update_bits(tps, TPS65218_REG_CONFIG2,
+		TPS65218_CONFIG2_UVLOHYS,
+		hyst == 400000 ? TPS65218_CONFIG2_UVLOHYS : 0,
+		TPS65218_PROTECT_L1);
+	return 0;
+}
+
+static int tps65218_voltage_set_uvlo(struct tps65218 *tps)
+{
+	u32 uvlo;
+	int uvloval;
+
+	if (of_property_read_u32(tps->dev->of_node,
+			 "ti,under-voltage-limit-microvolt", &uvlo))
+		return 0;
+
+	switch (uvlo) {
+	case 2750000:
+		uvloval = TPS65218_CONFIG1_UVLO_2750000;
+		break;
+	case 2950000:
+		uvloval = TPS65218_CONFIG1_UVLO_2950000;
+		break;
+	case 3250000:
+		uvloval = TPS65218_CONFIG1_UVLO_3250000;
+		break;
+	case 3350000:
+		uvloval = TPS65218_CONFIG1_UVLO_3350000;
+		break;
+	default:
+		dev_err(tps->dev,
+			"Invalid ti,under-voltage-limit-microvolt value\n");
+		return -EINVAL;
+	}
+
+	tps65218_update_bits(tps, TPS65218_REG_CONFIG1,
+		TPS65218_CONFIG1_UVLO_MASK, uvloval,
+		TPS65218_PROTECT_L1);
+	return 0;
+}
+
 static int tps65218_probe(struct i2c_client *client,
 				const struct i2c_device_id *ids)
 {
@@ -249,6 +324,18 @@ static int tps65218_probe(struct i2c_client *client,
 
 	tps->rev = chipid & TPS65218_CHIPID_REV_MASK;
 
+	ret = tps65218_voltage_set_strict(tps);
+	if (ret)
+		return ret;
+
+	ret = tps65218_voltage_set_uvlo(tps);
+	if (ret)
+		return ret;
+
+	ret = tps65218_voltage_set_uv_hyst(tps);
+	if (ret)
+		return ret;
+
 	ret = mfd_add_devices(tps->dev, PLATFORM_DEVID_AUTO, tps65218_cells,
 			      ARRAY_SIZE(tps65218_cells), NULL, 0,
 			      regmap_irq_get_domain(tps->irq_data));
diff --git a/include/linux/mfd/tps65218.h b/include/linux/mfd/tps65218.h
index c204d9a..3cbe103 100644
--- a/include/linux/mfd/tps65218.h
+++ b/include/linux/mfd/tps65218.h
@@ -137,6 +137,10 @@
 #define TPS65218_CONFIG1_PGDLY_MASK	0x18
 #define TPS65218_CONFIG1_STRICT		BIT(2)
 #define TPS65218_CONFIG1_UVLO_MASK	0x3
+#define TPS65218_CONFIG1_UVLO_2750000	0x0
+#define TPS65218_CONFIG1_UVLO_2950000	0x1
+#define TPS65218_CONFIG1_UVLO_3250000	0x2
+#define TPS65218_CONFIG1_UVLO_3350000	0x3
 
 #define TPS65218_CONFIG2_DC12_RST	BIT(7)
 #define TPS65218_CONFIG2_UVLOHYS	BIT(6)
-- 
2.7.4

Re: [PATCH v2 1/2] dt-bindings: regulator: extend tps65218 bindings

[Rob Herring]

On Thu, 3 Jan 2019 14:47:05 +0100, Christian Hohnstaedt wrote:
> Add input voltage configuration options
> 
> Signed-off-by: Christian Hohnstaedt <Christian.Hohnstaedt@wago.com>
> ---
>  .../devicetree/bindings/regulator/tps65218.txt     | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v2 2/2] mfd: tps65218.c: Add input voltage options

[Keerthy]

On Thursday 03 January 2019 07:17 PM, Christian Hohnstaedt wrote:
> These options apply to all regulators in this chip.
> 
> ti,strict-supply-voltage-supervision:
>   Set STRICT flag in CONFIG1
> ti,under-voltage-limit-microvolt:
>   Select 2.75, 2.95, 3.25 or 3.35 V UVLO in CONFIG1
> ti,under-voltage-hyst-microvolt:
>   Select 200mV or 400mV UVLOHYS in CONFIG2

CHECK: Alignment should match open parenthesis
#146: FILE: drivers/mfd/tps65218.c:219:
+	if (of_property_read_u32(tps->dev->of_node,
+			"ti,strict-supply-voltage-supervision", &strict))

CHECK: Alignment should match open parenthesis
#156: FILE: drivers/mfd/tps65218.c:229:
+	tps65218_update_bits(tps, TPS65218_REG_CONFIG1,
+		TPS65218_CONFIG1_STRICT,

CHECK: Alignment should match open parenthesis
#166: FILE: drivers/mfd/tps65218.c:239:
+	if (of_property_read_u32(tps->dev->of_node,
+			"ti,under-voltage-hyst-microvolt", &hyst))

CHECK: Alignment should match open parenthesis
#176: FILE: drivers/mfd/tps65218.c:249:
+	tps65218_update_bits(tps, TPS65218_REG_CONFIG2,
+		TPS65218_CONFIG2_UVLOHYS,

CHECK: Alignment should match open parenthesis
#188: FILE: drivers/mfd/tps65218.c:261:
+	if (of_property_read_u32(tps->dev->of_node,
+			 "ti,under-voltage-limit-microvolt", &uvlo))

CHECK: Alignment should match open parenthesis
#211: FILE: drivers/mfd/tps65218.c:284:
+	tps65218_update_bits(tps, TPS65218_REG_CONFIG1,
+		TPS65218_CONFIG1_UVLO_MASK, uvloval,

Some checkpatch warnings to be fixed.

Other than that i tested your patch on am437x-gp-evm for boot and suspend/resume.

After fixing them you can add my:

Tested-by: Keerthy <j-keerthy@ti.com>
Reviewed-by: Keerthy <j-keerthy@ti.com>

> 
> Signed-off-by: Christian Hohnstaedt <Christian.Hohnstaedt@wago.com>
> ---
>  drivers/mfd/tps65218.c       | 87 ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/tps65218.h |  4 ++
>  2 files changed, 91 insertions(+)
> 
> diff --git a/drivers/mfd/tps65218.c b/drivers/mfd/tps65218.c
> index 8bcdecf..b5e0f3a 100644
> --- a/drivers/mfd/tps65218.c
> +++ b/drivers/mfd/tps65218.c
> @@ -211,6 +211,81 @@ static const struct of_device_id of_tps65218_match_table[] = {
>  };
>  MODULE_DEVICE_TABLE(of, of_tps65218_match_table);
>  
> +static int tps65218_voltage_set_strict(struct tps65218 *tps)
> +{
> +	u32 strict;
> +
> +	if (of_property_read_u32(tps->dev->of_node,
> +			"ti,strict-supply-voltage-supervision", &strict))
> +		return 0;
> +
> +	if (strict != 0 && strict != 1) {
> +		dev_err(tps->dev,
> +			"Invalid ti,strict-supply-voltage-supervision value\n");
> +		return -EINVAL;
> +	}
> +
> +	tps65218_update_bits(tps, TPS65218_REG_CONFIG1,
> +		TPS65218_CONFIG1_STRICT,
> +		strict ? TPS65218_CONFIG1_STRICT : 0, TPS65218_PROTECT_L1);
> +	return 0;
> +}
> +
> +static int tps65218_voltage_set_uv_hyst(struct tps65218 *tps)
> +{
> +	u32 hyst;
> +
> +	if (of_property_read_u32(tps->dev->of_node,
> +			"ti,under-voltage-hyst-microvolt", &hyst))
> +		return 0;
> +
> +	if (hyst != 400000 && hyst != 200000) {
> +		dev_err(tps->dev,
> +			"Invalid ti,under-voltage-hyst-microvolt value\n");
> +		return -EINVAL;
> +	}
> +
> +	tps65218_update_bits(tps, TPS65218_REG_CONFIG2,
> +		TPS65218_CONFIG2_UVLOHYS,
> +		hyst == 400000 ? TPS65218_CONFIG2_UVLOHYS : 0,
> +		TPS65218_PROTECT_L1);
> +	return 0;
> +}
> +
> +static int tps65218_voltage_set_uvlo(struct tps65218 *tps)
> +{
> +	u32 uvlo;
> +	int uvloval;
> +
> +	if (of_property_read_u32(tps->dev->of_node,
> +			 "ti,under-voltage-limit-microvolt", &uvlo))
> +		return 0;
> +
> +	switch (uvlo) {
> +	case 2750000:
> +		uvloval = TPS65218_CONFIG1_UVLO_2750000;
> +		break;
> +	case 2950000:
> +		uvloval = TPS65218_CONFIG1_UVLO_2950000;
> +		break;
> +	case 3250000:
> +		uvloval = TPS65218_CONFIG1_UVLO_3250000;
> +		break;
> +	case 3350000:
> +		uvloval = TPS65218_CONFIG1_UVLO_3350000;
> +		break;
> +	default:
> +		dev_err(tps->dev,
> +			"Invalid ti,under-voltage-limit-microvolt value\n");
> +		return -EINVAL;
> +	}
> +
> +	tps65218_update_bits(tps, TPS65218_REG_CONFIG1,
> +		TPS65218_CONFIG1_UVLO_MASK, uvloval,
> +		TPS65218_PROTECT_L1);
> +	return 0;
> +}
> +
>  static int tps65218_probe(struct i2c_client *client,
>  				const struct i2c_device_id *ids)
>  {
> @@ -249,6 +324,18 @@ static int tps65218_probe(struct i2c_client *client,
>  
>  	tps->rev = chipid & TPS65218_CHIPID_REV_MASK;
>  
> +	ret = tps65218_voltage_set_strict(tps);
> +	if (ret)
> +		return ret;
> +
> +	ret = tps65218_voltage_set_uvlo(tps);
> +	if (ret)
> +		return ret;
> +
> +	ret = tps65218_voltage_set_uv_hyst(tps);
> +	if (ret)
> +		return ret;
> +
>  	ret = mfd_add_devices(tps->dev, PLATFORM_DEVID_AUTO, tps65218_cells,
>  			      ARRAY_SIZE(tps65218_cells), NULL, 0,
>  			      regmap_irq_get_domain(tps->irq_data));
> diff --git a/include/linux/mfd/tps65218.h b/include/linux/mfd/tps65218.h
> index c204d9a..3cbe103 100644
> --- a/include/linux/mfd/tps65218.h
> +++ b/include/linux/mfd/tps65218.h
> @@ -137,6 +137,10 @@
>  #define TPS65218_CONFIG1_PGDLY_MASK	0x18
>  #define TPS65218_CONFIG1_STRICT		BIT(2)
>  #define TPS65218_CONFIG1_UVLO_MASK	0x3
> +#define TPS65218_CONFIG1_UVLO_2750000	0x0
> +#define TPS65218_CONFIG1_UVLO_2950000	0x1
> +#define TPS65218_CONFIG1_UVLO_3250000	0x2
> +#define TPS65218_CONFIG1_UVLO_3350000	0x3
>  
>  #define TPS65218_CONFIG2_DC12_RST	BIT(7)
>  #define TPS65218_CONFIG2_UVLOHYS	BIT(6)
> 

[PATCH v3 0/2] Add input voltage configuration for TPS65218

[Christian Hohnstaedt]

This patch allows to configure input-voltage settings
of the TPS65218 regulator via device tree.

Changes for v3 (whitespace changes only):
 - fix indentation issues found by "checkpatch.pl --strict" in tps65218.c

Changes for v2:
 - handle the different options in separate functions
 - add "ti," prefix and "-microvolt" postfix
 - remove debug prints
 - abort with an error in case of invalid values.
 - extend documentation of the options (taken from the datasheet)

Christian Hohnstaedt (2):
  dt-bindings: regulator: extend tps65218 bindings
  mfd: tps65218.c: Add input voltage options

 .../devicetree/bindings/regulator/tps65218.txt     | 22 ++++++
 drivers/mfd/tps65218.c                             | 89 ++++++++++++++++++++++
 include/linux/mfd/tps65218.h                       |  4 +
 3 files changed, 115 insertions(+)

--
2.7.4

[PATCH v3 1/2] dt-bindings: regulator: extend tps65218 bindings

[Christian Hohnstaedt]

Add input voltage configuration options

Signed-off-by: Christian Hohnstaedt <Christian.Hohnstaedt@wago.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/regulator/tps65218.txt     | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/tps65218.txt b/Documentation/devicetree/bindings/regulator/tps65218.txt
index 02f0e9b..982597d 100644
--- a/Documentation/devicetree/bindings/regulator/tps65218.txt
+++ b/Documentation/devicetree/bindings/regulator/tps65218.txt
@@ -16,12 +16,34 @@ Required properties:
   regulator-dcdc5, regulator-dcdc6, regulator-ldo1, regulator-ls3.
   Each regulator is defined using the standard binding for regulators.
 
+Optional properties:
+  If any of these properties is absent then the corresponding setting will be
+  untouched.
+- ti,strict-supply-voltage-supervision: Set/Reset STRICT flag in CONFIG1.
+  The supervisor has two modes of operation, controlled by the STRICT bit.
+  With the STRICT bit set to 0, all five rails are monitored for under-voltage
+  only with relaxed thresholds and deglitch times.
+  With the STRCT bit set to 1, all five rails are monitored for under-voltage
+  and over-voltage with tight limits and short deglitch times.
+- ti,under-voltage-limit-microvolt: Configures UVLO in CONFIG1.
+  Valid values are: 2750000, 2950000, 3250000, 3350000
+- ti,under-voltage-hyst-microvolt: Select 200mV or 400mV UVLOHYS in CONFIG2
+  Power rails are only enabled if the input voltage measured at the IN_BIAS pin
+  is greater than the under-voltage lockout threshold plus hysteresis
+  (UVLO + UVLOHYS). Once the input voltage rises above this level,
+  the input voltage may drop to the UVLO level before the PMIC shuts down.
+  UVLO is deglitched by 5 ms on rising and falling edge.
+
 Example:
 tps65218: tps65218@24 {
 	reg = <0x24>;
 	compatible = "ti,tps65218";
 	interrupts = <GIC_SPI 7 IRQ_TYPE_NONE>; /* NMIn */
 	interrupt-controller;
+	ti,strict-supply-voltage-supervision = <1>;
+	ti,under-voltage-hyst-microvolt = <400000>;
+	ti,under-voltage-limit-microvolt = <3350000>;
+
 	#interrupt-cells = <2>;
 
 	dcdc1: regulator-dcdc1 {
-- 
2.7.4

[PATCH v3 2/2] mfd: tps65218.c: Add input voltage options

[Christian Hohnstaedt]

These options apply to all regulators in this chip.

ti,strict-supply-voltage-supervision:
  Set STRICT flag in CONFIG1
ti,under-voltage-limit-microvolt:
  Select 2.75, 2.95, 3.25 or 3.35 V UVLO in CONFIG1
ti,under-voltage-hyst-microvolt:
  Select 200mV or 400mV UVLOHYS in CONFIG2

Signed-off-by: Christian Hohnstaedt <Christian.Hohnstaedt@wago.com>
Tested-by: Keerthy <j-keerthy@ti.com>
Reviewed-by: Keerthy <j-keerthy@ti.com>
---
 drivers/mfd/tps65218.c       | 89 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/tps65218.h |  4 ++
 2 files changed, 93 insertions(+)

diff --git a/drivers/mfd/tps65218.c b/drivers/mfd/tps65218.c
index 8bcdecf..a62ea4c 100644
--- a/drivers/mfd/tps65218.c
+++ b/drivers/mfd/tps65218.c
@@ -211,6 +211,83 @@ static const struct of_device_id of_tps65218_match_table[] = {
 };
 MODULE_DEVICE_TABLE(of, of_tps65218_match_table);
 
+static int tps65218_voltage_set_strict(struct tps65218 *tps)
+{
+	u32 strict;
+
+	if (of_property_read_u32(tps->dev->of_node,
+				 "ti,strict-supply-voltage-supervision",
+				 &strict))
+		return 0;
+
+	if (strict != 0 && strict != 1) {
+		dev_err(tps->dev,
+			"Invalid ti,strict-supply-voltage-supervision value\n");
+		return -EINVAL;
+	}
+
+	tps65218_update_bits(tps, TPS65218_REG_CONFIG1,
+			     TPS65218_CONFIG1_STRICT,
+			     strict ? TPS65218_CONFIG1_STRICT : 0,
+			     TPS65218_PROTECT_L1);
+	return 0;
+}
+
+static int tps65218_voltage_set_uv_hyst(struct tps65218 *tps)
+{
+	u32 hyst;
+
+	if (of_property_read_u32(tps->dev->of_node,
+				 "ti,under-voltage-hyst-microvolt", &hyst))
+		return 0;
+
+	if (hyst != 400000 && hyst != 200000) {
+		dev_err(tps->dev,
+			"Invalid ti,under-voltage-hyst-microvolt value\n");
+		return -EINVAL;
+	}
+
+	tps65218_update_bits(tps, TPS65218_REG_CONFIG2,
+			     TPS65218_CONFIG2_UVLOHYS,
+			     hyst == 400000 ? TPS65218_CONFIG2_UVLOHYS : 0,
+			     TPS65218_PROTECT_L1);
+	return 0;
+}
+
+static int tps65218_voltage_set_uvlo(struct tps65218 *tps)
+{
+	u32 uvlo;
+	int uvloval;
+
+	if (of_property_read_u32(tps->dev->of_node,
+				 "ti,under-voltage-limit-microvolt", &uvlo))
+		return 0;
+
+	switch (uvlo) {
+	case 2750000:
+		uvloval = TPS65218_CONFIG1_UVLO_2750000;
+		break;
+	case 2950000:
+		uvloval = TPS65218_CONFIG1_UVLO_2950000;
+		break;
+	case 3250000:
+		uvloval = TPS65218_CONFIG1_UVLO_3250000;
+		break;
+	case 3350000:
+		uvloval = TPS65218_CONFIG1_UVLO_3350000;
+		break;
+	default:
+		dev_err(tps->dev,
+			"Invalid ti,under-voltage-limit-microvolt value\n");
+		return -EINVAL;
+	}
+
+	tps65218_update_bits(tps, TPS65218_REG_CONFIG1,
+			     TPS65218_CONFIG1_UVLO_MASK, uvloval,
+			     TPS65218_PROTECT_L1);
+	return 0;
+}
+
 static int tps65218_probe(struct i2c_client *client,
 				const struct i2c_device_id *ids)
 {
@@ -249,6 +326,18 @@ static int tps65218_probe(struct i2c_client *client,
 
 	tps->rev = chipid & TPS65218_CHIPID_REV_MASK;
 
+	ret = tps65218_voltage_set_strict(tps);
+	if (ret)
+		return ret;
+
+	ret = tps65218_voltage_set_uvlo(tps);
+	if (ret)
+		return ret;
+
+	ret = tps65218_voltage_set_uv_hyst(tps);
+	if (ret)
+		return ret;
+
 	ret = mfd_add_devices(tps->dev, PLATFORM_DEVID_AUTO, tps65218_cells,
 			      ARRAY_SIZE(tps65218_cells), NULL, 0,
 			      regmap_irq_get_domain(tps->irq_data));
diff --git a/include/linux/mfd/tps65218.h b/include/linux/mfd/tps65218.h
index c204d9a..3cbe103 100644
--- a/include/linux/mfd/tps65218.h
+++ b/include/linux/mfd/tps65218.h
@@ -137,6 +137,10 @@
 #define TPS65218_CONFIG1_PGDLY_MASK	0x18
 #define TPS65218_CONFIG1_STRICT		BIT(2)
 #define TPS65218_CONFIG1_UVLO_MASK	0x3
+#define TPS65218_CONFIG1_UVLO_2750000	0x0
+#define TPS65218_CONFIG1_UVLO_2950000	0x1
+#define TPS65218_CONFIG1_UVLO_3250000	0x2
+#define TPS65218_CONFIG1_UVLO_3350000	0x3
 
 #define TPS65218_CONFIG2_DC12_RST	BIT(7)
 #define TPS65218_CONFIG2_UVLOHYS	BIT(6)
-- 
2.7.4

Re: [PATCH v3 2/2] mfd: tps65218.c: Add input voltage options

[Lee Jones]

On Mon, 14 Jan 2019, Christian Hohnstaedt wrote:

> These options apply to all regulators in this chip.
> 
> ti,strict-supply-voltage-supervision:
>   Set STRICT flag in CONFIG1
> ti,under-voltage-limit-microvolt:
>   Select 2.75, 2.95, 3.25 or 3.35 V UVLO in CONFIG1
> ti,under-voltage-hyst-microvolt:
>   Select 200mV or 400mV UVLOHYS in CONFIG2
> 
> Signed-off-by: Christian Hohnstaedt <Christian.Hohnstaedt@wago.com>
> Tested-by: Keerthy <j-keerthy@ti.com>
> Reviewed-by: Keerthy <j-keerthy@ti.com>
> ---
>  drivers/mfd/tps65218.c       | 89 ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/tps65218.h |  4 ++
>  2 files changed, 93 insertions(+)

This looks like regulator code.

Why aren't you placing it into the regulator driver?

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 2/2] mfd: tps65218.c: Add input voltage options

[Christian Hohnstaedt]

On Wed, Jan 16, 2019 at 02:13:36PM +0000, Lee Jones wrote:
> On Mon, 14 Jan 2019, Christian Hohnstaedt wrote:
> 
> > These options apply to all regulators in this chip.
> > 
> > ti,strict-supply-voltage-supervision:
> >   Set STRICT flag in CONFIG1
> > ti,under-voltage-limit-microvolt:
> >   Select 2.75, 2.95, 3.25 or 3.35 V UVLO in CONFIG1
> > ti,under-voltage-hyst-microvolt:
> >   Select 200mV or 400mV UVLOHYS in CONFIG2
> > 
> > Signed-off-by: Christian Hohnstaedt <Christian.Hohnstaedt@wago.com>
> > Tested-by: Keerthy <j-keerthy@ti.com>
> > Reviewed-by: Keerthy <j-keerthy@ti.com>
> > ---
> >  drivers/mfd/tps65218.c       | 89 ++++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/mfd/tps65218.h |  4 ++
> >  2 files changed, 93 insertions(+)
> 
> This looks like regulator code.
> 
> Why aren't you placing it into the regulator driver?

This code manages properties of the chip, affecting all regulators.

The regulator-driver has no hook to be called once for the chip.
Even if I put this code into the regulator driver file,
I will have to call it from here.

This would introduce a dependency from the mfd to the regulator code.


Christian

Re: [PATCH v3 2/2] mfd: tps65218.c: Add input voltage options

[Lee Jones]

On Thu, 17 Jan 2019, Christian Hohnstaedt wrote:

> On Wed, Jan 16, 2019 at 02:13:36PM +0000, Lee Jones wrote:
> > On Mon, 14 Jan 2019, Christian Hohnstaedt wrote:
> > 
> > > These options apply to all regulators in this chip.
> > > 
> > > ti,strict-supply-voltage-supervision:
> > >   Set STRICT flag in CONFIG1
> > > ti,under-voltage-limit-microvolt:
> > >   Select 2.75, 2.95, 3.25 or 3.35 V UVLO in CONFIG1
> > > ti,under-voltage-hyst-microvolt:
> > >   Select 200mV or 400mV UVLOHYS in CONFIG2
> > > 
> > > Signed-off-by: Christian Hohnstaedt <Christian.Hohnstaedt@wago.com>
> > > Tested-by: Keerthy <j-keerthy@ti.com>
> > > Reviewed-by: Keerthy <j-keerthy@ti.com>
> > > ---
> > >  drivers/mfd/tps65218.c       | 89 ++++++++++++++++++++++++++++++++++++++++++++
> > >  include/linux/mfd/tps65218.h |  4 ++
> > >  2 files changed, 93 insertions(+)
> > 
> > This looks like regulator code.
> > 
> > Why aren't you placing it into the regulator driver?
> 
> This code manages properties of the chip, affecting all regulators.
> 
> The regulator-driver has no hook to be called once for the chip.
> Even if I put this code into the regulator driver file,
> I will have to call it from here.
> 
> This would introduce a dependency from the mfd to the regulator code.

Sounds reasonable.  Thanks for the explanation.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 2/2] mfd: tps65218.c: Add input voltage options

[Lee Jones]

On Mon, 14 Jan 2019, Christian Hohnstaedt wrote:

> These options apply to all regulators in this chip.
> 
> ti,strict-supply-voltage-supervision:
>   Set STRICT flag in CONFIG1
> ti,under-voltage-limit-microvolt:
>   Select 2.75, 2.95, 3.25 or 3.35 V UVLO in CONFIG1
> ti,under-voltage-hyst-microvolt:
>   Select 200mV or 400mV UVLOHYS in CONFIG2
> 
> Signed-off-by: Christian Hohnstaedt <Christian.Hohnstaedt@wago.com>
> Tested-by: Keerthy <j-keerthy@ti.com>
> Reviewed-by: Keerthy <j-keerthy@ti.com>
> ---
>  drivers/mfd/tps65218.c       | 89 ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/tps65218.h |  4 ++
>  2 files changed, 93 insertions(+)

Applied, thanks.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog