[PATCH-v3 0/2] regulator: 88pm800: Add dual phase mode support on BUCK1

[PATCH-v3 0/2] regulator: 88pm800: Add dual phase mode support on BUCK1

[Vaibhav Hiremath]

88PM860 device supports dual phase mode on BUCK1 output.
In normal usecase, BUCK1A and BUCK1B operates independently with 3A
capacity. And they both can work as a dual phase providing 6A capacity.

This patch series is subset of earlier patch-series
Link to earlier series - https://lkml.org/lkml/2015/7/16/722

Except PATCH[5/5], all other patches in the series are accepted and
queued up for next merge window.
And based on discussion on the list, creating DT property to enable
dual-phase mode on BUCK1.

Testing:
 - Tested on 88PM860 based platform
 - Boot tested 
 - Tested with & without DT property being set
 - Read register value before and after probe to make sure that
   value has been set.

V2 => V3:
========
 - Based on discussion on earlier patch-series,
   (comments from Krzysztof Kozlowski)
   Dynamically controlled current capacity and registration of BUCK1B,
   in case of BUCK1 dual phase mode enabled.
   Now, if BUCK1 dual phase is enabled, current capacity is set to 6A,
   and, BUCK1B will not be registered to regulator framework.

V1 => V2:
========
 - This is new patch-series, where, all accepted patches dropped.
   Upgraded Patch version, to ease review.
 - Based on Mark Brown's comment, we should use DT property of its own.
   using set_current_limit() is not right way here.
   So, created DT property for Dual phase mode enable.
 - Updated binding for new DT property


Vaibhav Hiremath (2):
  mfd: devicetree: bindings: 88pm800: Add DT property for dual phase
    enable
  regulator: 88pm800: Add support for configuration of dual phase on
    BUCK1

 Documentation/devicetree/bindings/mfd/88pm800.txt |  6 ++++
 drivers/regulator/88pm800.c                       | 40 +++++++++++++++++++++++
 include/linux/mfd/88pm80x.h                       |  3 ++
 3 files changed, 49 insertions(+)

-- 
1.9.1

[PATCH-v3 1/2] mfd: devicetree: bindings: 88pm800: Add DT property for dual phase enable

[Vaibhav Hiremath]

88PM860 family of device supports dual phase mode on BUCK1 supply
providing total 6A capacity.
Note that by default they operate independently with 3A capacity.

This patch updates the devicetree binding with DT property
to enable dual-phase mode on BUCK1.

Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
---
 Documentation/devicetree/bindings/mfd/88pm800.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/88pm800.txt b/Documentation/devicetree/bindings/mfd/88pm800.txt
index dec842f..2c82fcb 100644
--- a/Documentation/devicetree/bindings/mfd/88pm800.txt
+++ b/Documentation/devicetree/bindings/mfd/88pm800.txt
@@ -9,6 +9,12 @@ Required parent device properties:
 - #interrupt-cells 	: should be 1.
 			  The cell is the 88pm80x local IRQ number
 
+Optional properties :
+- marvell,88pm860-buck1-dualphase-en  : If set, enable dual phase on BUCK1,
+  providing 6A capacity.
+  Without this both BUCK1A and BUCK1B operates independently with 3A capacity.
+  (This property is only applicable to 88PM860)
+
 88pm80x family of devices consists of varied group of sub-devices:
 
 Device		 	Supply Names	 Description
-- 
1.9.1

[PATCH-v3 1/2] mfd: devicetree: bindings: 88pm800: Add DT property for dual phase enable

[Lee Jones]

On Mon, 24 Aug 2015, Vaibhav Hiremath wrote:

> 88PM860 family of device supports dual phase mode on BUCK1 supply
> providing total 6A capacity.
> Note that by default they operate independently with 3A capacity.
> 
> This patch updates the devicetree binding with DT property
> to enable dual-phase mode on BUCK1.
> 
> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
> ---
>  Documentation/devicetree/bindings/mfd/88pm800.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/88pm800.txt b/Documentation/devicetree/bindings/mfd/88pm800.txt
> index dec842f..2c82fcb 100644
> --- a/Documentation/devicetree/bindings/mfd/88pm800.txt
> +++ b/Documentation/devicetree/bindings/mfd/88pm800.txt
> @@ -9,6 +9,12 @@ Required parent device properties:
>  - #interrupt-cells 	: should be 1.
>  			  The cell is the 88pm80x local IRQ number
>  
> +Optional properties :
> +- marvell,88pm860-buck1-dualphase-en  : If set, enable dual phase on BUCK1,
> +  providing 6A capacity.
> +  Without this both BUCK1A and BUCK1B operates independently with 3A capacity.
> +  (This property is only applicable to 88PM860)

This will require a Regulator Ack.

My suggestion would be to remove the 'buck' number, as the same
property could be used on any Buck, and remove the '-en' part, as
this is implied.

>  88pm80x family of devices consists of varied group of sub-devices:
>  
>  Device		 	Supply Names	 Description

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH-v3 1/2] mfd: devicetree: bindings: 88pm800: Add DT property for dual phase enable

[Vaibhav Hiremath]

On Monday 24 August 2015 06:32 PM, Lee Jones wrote:
> On Mon, 24 Aug 2015, Vaibhav Hiremath wrote:
>
>> 88PM860 family of device supports dual phase mode on BUCK1 supply
>> providing total 6A capacity.
>> Note that by default they operate independently with 3A capacity.
>>
>> This patch updates the devicetree binding with DT property
>> to enable dual-phase mode on BUCK1.
>>
>> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
>> ---
>>   Documentation/devicetree/bindings/mfd/88pm800.txt | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/88pm800.txt b/Documentation/devicetree/bindings/mfd/88pm800.txt
>> index dec842f..2c82fcb 100644
>> --- a/Documentation/devicetree/bindings/mfd/88pm800.txt
>> +++ b/Documentation/devicetree/bindings/mfd/88pm800.txt
>> @@ -9,6 +9,12 @@ Required parent device properties:
>>   - #interrupt-cells 	: should be 1.
>>   			  The cell is the 88pm80x local IRQ number
>>
>> +Optional properties :
>> +- marvell,88pm860-buck1-dualphase-en  : If set, enable dual phase on BUCK1,
>> +  providing 6A capacity.
>> +  Without this both BUCK1A and BUCK1B operates independently with 3A capacity.
>> +  (This property is only applicable to 88PM860)
>
> This will require a Regulator Ack.
>
> My suggestion would be to remove the 'buck' number, as the same
> property could be used on any Buck, and remove the '-en' part, as
> this is implied.
>

Ok, Will do it in next version.

Mark,

Any comments here before I spin V4.

Thanks,
Vaibhav

[PATCH-v3 1/2] mfd: devicetree: bindings: 88pm800: Add DT property for dual phase enable

[Mark Brown]

On Mon, Aug 24, 2015 at 08:24:10PM +0530, Vaibhav Hiremath wrote:

> Mark,

> Any comments here before I spin V4.

Please just resubmit things, seeing that a change was requested is
usually a good indication that a new version is incoming.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150827/5eb1b350/attachment.sig>

[PATCH-v3 2/2] regulator: 88pm800: Add support for configuration of dual phase on BUCK1

[Vaibhav Hiremath]

88PM860 device supports dual phase mode on BUCK1 output.
In normal usecase, BUCK1A and BUCK1B operates independently with 3A
capacity. And they both can work as a dual phase providing 6A capacity.

This patch updates the regulator driver to read the respective
DT property and enable dual-phase mode on BUCK1.
Note that if dual phase mode is enabled, then BUCK1B will not be
registered to the regulator framework and the current capacity of
BUCK1(A) would be set to 6A [3A of BUCK1A + 3A of BUCK1B].

Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
---
 drivers/regulator/88pm800.c | 40 ++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/88pm80x.h |  3 +++
 2 files changed, 43 insertions(+)

diff --git a/drivers/regulator/88pm800.c b/drivers/regulator/88pm800.c
index 365a154..7aca6d17 100644
--- a/drivers/regulator/88pm800.c
+++ b/drivers/regulator/88pm800.c
@@ -267,6 +267,37 @@ static struct pm800_regulator_info pm860_regulator_info[] = {
 	PM800_LDO(ldo20, LDO20, LDO_ENA1_3, 3, 10000, ldo_volt_table2),
 };
 
+static int pm800_regulator_init(struct platform_device *pdev,
+		struct pm800_regulator_info *info)
+{
+	struct pm800_regulators *pm800_data = platform_get_drvdata(pdev);
+	struct pm80x_chip *chip = pm800_data->chip;
+	int ret = 0;
+
+	/* Currently only supported on 88pm860 device */
+	if (chip->type != CHIP_PM860)
+		return 0;
+
+	if (of_property_read_bool(pdev->dev.of_node,
+				"marvell,88pm860-buck1-dualphase-en")) {
+		/* Update the constraint  to [3A (BUCK1A) + 3A (BUCK1B)] */
+		info[PM800_ID_BUCK1].max_ua = 6000000;
+		/* Remove BUCK1B from the list, as we are in dual phase mode */
+		memset(&info[PM800_ID_BUCK1B], 0, sizeof(*info));
+		/* Enable dual phase mode */
+		ret = regmap_update_bits(chip->subchip->regmap_power,
+					PM860_BUCK1_MISC,
+					BUCK1_DUAL_PHASE_SEL,
+					BUCK1_DUAL_PHASE_SEL);
+		if (ret) {
+			dev_err(chip->dev, "failed to set dual-pase mode %d\n", ret);
+			return ret;
+		}
+	}
+
+	return ret;
+}
+
 static int pm800_regulator_probe(struct platform_device *pdev)
 {
 	struct pm80x_chip *chip = dev_get_drvdata(pdev->dev.parent);
@@ -311,11 +342,20 @@ static int pm800_regulator_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	ret = pm800_regulator_init(pdev, info);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to init 88pm800 regulator device\n");
+		return ret;
+	}
+
 	config.dev = chip->dev;
 	config.regmap = pm800_data->map;
 	for (i = 0; i < PM800_ID_RG_MAX; i++) {
 		struct regulator_dev *regulator;
 
+		if (!info[i].desc.name)
+			continue;
+
 		if (pdata && pdata->num_regulators) {
 			init_data = pdata->regulators[i];
 			if (!init_data)
diff --git a/include/linux/mfd/88pm80x.h b/include/linux/mfd/88pm80x.h
index a92d173..122cfd2 100644
--- a/include/linux/mfd/88pm80x.h
+++ b/include/linux/mfd/88pm80x.h
@@ -295,6 +295,9 @@ enum {
 #define PM860_BUCK4_MISC2		(0x82)
 #define PM860_BUCK4_FULL_DRV		BIT(2)
 
+#define PM860_BUCK1_MISC		0x8E
+#define BUCK1_DUAL_PHASE_SEL		BIT(2)
+
 struct pm80x_rtc_pdata {
 	int		vrtc;
 	int		rtc_wakeup;
-- 
1.9.1

[PATCH-v3 2/2] regulator: 88pm800: Add support for configuration of dual phase on BUCK1

[Krzysztof Kozlowski]

On 24.08.2015 21:40, Vaibhav Hiremath wrote:
> 88PM860 device supports dual phase mode on BUCK1 output.
> In normal usecase, BUCK1A and BUCK1B operates independently with 3A
> capacity. And they both can work as a dual phase providing 6A capacity.
> 
> This patch updates the regulator driver to read the respective
> DT property and enable dual-phase mode on BUCK1.
> Note that if dual phase mode is enabled, then BUCK1B will not be
> registered to the regulator framework and the current capacity of
> BUCK1(A) would be set to 6A [3A of BUCK1A + 3A of BUCK1B].
> 
> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
> ---
>  drivers/regulator/88pm800.c | 40 ++++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/88pm80x.h |  3 +++
>  2 files changed, 43 insertions(+)
> 
> diff --git a/drivers/regulator/88pm800.c b/drivers/regulator/88pm800.c
> index 365a154..7aca6d17 100644
> --- a/drivers/regulator/88pm800.c
> +++ b/drivers/regulator/88pm800.c
> @@ -267,6 +267,37 @@ static struct pm800_regulator_info pm860_regulator_info[] = {
>  	PM800_LDO(ldo20, LDO20, LDO_ENA1_3, 3, 10000, ldo_volt_table2),
>  };
>  
> +static int pm800_regulator_init(struct platform_device *pdev,
> +		struct pm800_regulator_info *info)
> +{
> +	struct pm800_regulators *pm800_data = platform_get_drvdata(pdev);
> +	struct pm80x_chip *chip = pm800_data->chip;
> +	int ret = 0;
> +
> +	/* Currently only supported on 88pm860 device */
> +	if (chip->type != CHIP_PM860)
> +		return 0;
> +
> +	if (of_property_read_bool(pdev->dev.of_node,
> +				"marvell,88pm860-buck1-dualphase-en")) {
> +		/* Update the constraint  to [3A (BUCK1A) + 3A (BUCK1B)] */
> +		info[PM800_ID_BUCK1].max_ua = 6000000;
> +		/* Remove BUCK1B from the list, as we are in dual phase mode */
> +		memset(&info[PM800_ID_BUCK1B], 0, sizeof(*info));
> +		/* Enable dual phase mode */
> +		ret = regmap_update_bits(chip->subchip->regmap_power,
> +					PM860_BUCK1_MISC,
> +					BUCK1_DUAL_PHASE_SEL,
> +					BUCK1_DUAL_PHASE_SEL);
> +		if (ret) {
> +			dev_err(chip->dev, "failed to set dual-pase mode %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
>  static int pm800_regulator_probe(struct platform_device *pdev)
>  {
>  	struct pm80x_chip *chip = dev_get_drvdata(pdev->dev.parent);
> @@ -311,11 +342,20 @@ static int pm800_regulator_probe(struct platform_device *pdev)
>  		return -ENODEV;
>  	}
>  
> +	ret = pm800_regulator_init(pdev, info);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to init 88pm800 regulator device\n");
> +		return ret;
> +	}
> +
>  	config.dev = chip->dev;
>  	config.regmap = pm800_data->map;
>  	for (i = 0; i < PM800_ID_RG_MAX; i++) {
>  		struct regulator_dev *regulator;
>  
> +		if (!info[i].desc.name)
> +			continue;

How the driver was working without this on 88PM800? There is a gap in
pm800_regulator_info - three regulators are not set.

The patch itself looks good
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

[PATCH-v3 2/2] regulator: 88pm800: Add support for configuration of dual phase on BUCK1

[Vaibhav Hiremath]

On Tuesday 25 August 2015 10:18 AM, Krzysztof Kozlowski wrote:
> On 24.08.2015 21:40, Vaibhav Hiremath wrote:
>> 88PM860 device supports dual phase mode on BUCK1 output.
>> In normal usecase, BUCK1A and BUCK1B operates independently with 3A
>> capacity. And they both can work as a dual phase providing 6A capacity.
>>
>> This patch updates the regulator driver to read the respective
>> DT property and enable dual-phase mode on BUCK1.
>> Note that if dual phase mode is enabled, then BUCK1B will not be
>> registered to the regulator framework and the current capacity of
>> BUCK1(A) would be set to 6A [3A of BUCK1A + 3A of BUCK1B].
>>
>> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
>> ---
>>   drivers/regulator/88pm800.c | 40 ++++++++++++++++++++++++++++++++++++++++
>>   include/linux/mfd/88pm80x.h |  3 +++
>>   2 files changed, 43 insertions(+)
>>
>> diff --git a/drivers/regulator/88pm800.c b/drivers/regulator/88pm800.c
>> index 365a154..7aca6d17 100644
>> --- a/drivers/regulator/88pm800.c
>> +++ b/drivers/regulator/88pm800.c
>> @@ -267,6 +267,37 @@ static struct pm800_regulator_info pm860_regulator_info[] = {
>>   	PM800_LDO(ldo20, LDO20, LDO_ENA1_3, 3, 10000, ldo_volt_table2),
>>   };
>>
>> +static int pm800_regulator_init(struct platform_device *pdev,
>> +		struct pm800_regulator_info *info)
>> +{
>> +	struct pm800_regulators *pm800_data = platform_get_drvdata(pdev);
>> +	struct pm80x_chip *chip = pm800_data->chip;
>> +	int ret = 0;
>> +
>> +	/* Currently only supported on 88pm860 device */
>> +	if (chip->type != CHIP_PM860)
>> +		return 0;
>> +
>> +	if (of_property_read_bool(pdev->dev.of_node,
>> +				"marvell,88pm860-buck1-dualphase-en")) {
>> +		/* Update the constraint  to [3A (BUCK1A) + 3A (BUCK1B)] */
>> +		info[PM800_ID_BUCK1].max_ua = 6000000;
>> +		/* Remove BUCK1B from the list, as we are in dual phase mode */
>> +		memset(&info[PM800_ID_BUCK1B], 0, sizeof(*info));
>> +		/* Enable dual phase mode */
>> +		ret = regmap_update_bits(chip->subchip->regmap_power,
>> +					PM860_BUCK1_MISC,
>> +					BUCK1_DUAL_PHASE_SEL,
>> +					BUCK1_DUAL_PHASE_SEL);
>> +		if (ret) {
>> +			dev_err(chip->dev, "failed to set dual-pase mode %d\n", ret);
>> +			return ret;
>> +		}
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>>   static int pm800_regulator_probe(struct platform_device *pdev)
>>   {
>>   	struct pm80x_chip *chip = dev_get_drvdata(pdev->dev.parent);
>> @@ -311,11 +342,20 @@ static int pm800_regulator_probe(struct platform_device *pdev)
>>   		return -ENODEV;
>>   	}
>>
>> +	ret = pm800_regulator_init(pdev, info);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "failed to init 88pm800 regulator device\n");
>> +		return ret;
>> +	}
>> +
>>   	config.dev = chip->dev;
>>   	config.regmap = pm800_data->map;
>>   	for (i = 0; i < PM800_ID_RG_MAX; i++) {
>>   		struct regulator_dev *regulator;
>>
>> +		if (!info[i].desc.name)
>> +			continue;
>
> How the driver was working without this on 88PM800? There is a gap in
> pm800_regulator_info - three regulators are not set.
>

I have 88PM860 based board, where the whole array will be filled up.
So it was missed I guess :)

But when I introduced logic for skipping BUCK1B, I hit this.
And here is the fix.


> The patch itself looks good
> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>

Thanks for your review.

Thanks,
Vaibhav