[PATCH 0/6] mfd: 88pm800: Add 88pm860 device support

[PATCH 0/6] mfd: 88pm800: Add 88pm860 device support

[Vaibhav Hiremath]

88PM860 falls under 88pm800 family of devices, with some feature
additions, for example, support for dual phase on BUCK1.

This patch series, enabled chip ID support for 88pm860 in the driver
and adds,
  - Init time configuration support based on chip ID
  - DT property for 32KHz low jitter clock enable
  - DT property for dual phase enable

TODO:
 - Some of init time configurations are common to both 88pm800 and 88pm860
   devices, but since I can not validate it, decided to move it to
   88pm860 block only.
   But if someone is willing to help me in validation, we can move it to
   common code later. No issues there.
 - Sleep/low-power-mode related configuration is also part of init time,
   but since we are too far from sleep mode support and without testing
   I do not want add anything, decided to take it later when we actually
   start looking at sleep support.
 - Init time configuration also includes pinmux setting for the device.
   I am working on using pinctrl-single driver to have standard and generic
   interface, hopefully it will get handled through pinctrl subsystem.
   Link to RFC - https://patches.linaro.org/50604/

Vaibhav Hiremath (6):
  mfd: 88pm80x: Add 88pm860 chip type support
  mfd: 88pm800: Add init time initial configuration support
  mfd: devicetree: bindings: 88pm800: Add DT property for 32KHz output
    enable
  mfd: 88pm800: Enable 32KHZ XO low jitter clock out
  mfd: devicetree: bindings: 88pm800: Add DT property for dual phase
    enable
  mfd: 88pm800: Add support for configuration of dual phase on BUCK1

 Documentation/devicetree/bindings/mfd/88pm800.txt | 12 ++++
 drivers/mfd/88pm800.c                             | 88 +++++++++++++++++++++++
 drivers/mfd/88pm80x.c                             |  2 +
 include/linux/mfd/88pm80x.h                       | 18 +++++
 4 files changed, 120 insertions(+)

-- 
1.9.1

[PATCH 1/6] mfd: 88pm80x: Add 88pm860 chip type support

[Vaibhav Hiremath]

Add chip identification support for 88PM860 device
to the pm80x_chip_mapping table.

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

diff --git a/drivers/mfd/88pm80x.c b/drivers/mfd/88pm80x.c
index 5e72f65..63445ea 100644
--- a/drivers/mfd/88pm80x.c
+++ b/drivers/mfd/88pm80x.c
@@ -33,6 +33,8 @@ static struct pm80x_chip_mapping chip_mapping[] = {
 	{0x3,	CHIP_PM800},
 	/* 88PM805 chip id number */
 	{0x0,	CHIP_PM805},
+	/* 88PM860 chip id number */
+	{0x4,	CHIP_PM860},
 };
 
 /*
diff --git a/include/linux/mfd/88pm80x.h b/include/linux/mfd/88pm80x.h
index 9c5773b..2e25fb1 100644
--- a/include/linux/mfd/88pm80x.h
+++ b/include/linux/mfd/88pm80x.h
@@ -21,6 +21,7 @@ enum {
 	CHIP_INVALID = 0,
 	CHIP_PM800,
 	CHIP_PM805,
+	CHIP_PM860,
 	CHIP_MAX,
 };
 
-- 
1.9.1

[PATCH 1/6] mfd: 88pm80x: Add 88pm860 chip type support

[Krzysztof Kozlowski]

W dniu 09.07.2015 o 20:47, Vaibhav Hiremath pisze:
> Add chip identification support for 88PM860 device
> to the pm80x_chip_mapping table.
> 
> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
> ---
>  drivers/mfd/88pm80x.c       | 2 ++
>  include/linux/mfd/88pm80x.h | 1 +
>  2 files changed, 3 insertions(+)

FWIW:
Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

[PATCH 2/6] mfd: 88pm800: Add init time initial configuration support

[Vaibhav Hiremath]

This patch adds init time configuration of 88PM800/805 and
88PM860. It includes,

  - Enable BUCK clock gating in low power mode
  - Full mode support for BUCK2 and 4
  - Enable voltage change (LPF, DVC) in PMIC

Note that both 88PM800 and 88PM860 do share common configurations,
but since I can not validate the configuration on 88PM800,
restricting myself only to 88PM860.
If anyone can validate on 88PM800, we can move common code accordingly.

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

diff --git a/drivers/mfd/88pm800.c b/drivers/mfd/88pm800.c
index 95c8ad4..80a1bc1 100644
--- a/drivers/mfd/88pm800.c
+++ b/drivers/mfd/88pm800.c
@@ -521,6 +521,63 @@ out:
 	return ret;
 }
 
+static int pm800_init_config(struct pm80x_chip *chip, struct device_node *np)
+{
+	int ret;
+	unsigned int val;
+
+	switch (chip->type) {
+	case CHIP_PM800:
+	case CHIP_PM805:
+		break;
+	case CHIP_PM860:
+		/* Enable LDO and BUCK clock gating in low power mode */
+		ret = regmap_update_bits(chip->regmap, PM800_LOW_POWER_CONFIG3,
+					PM800_LDOBK_FREEZE, PM800_LDOBK_FREEZE);
+		if (ret)
+			goto error;
+
+		/* Enable voltage change in pmic, POWER_HOLD = 1 */
+		ret = regmap_update_bits(chip->regmap, PM800_WAKEUP1,
+					PM800_PWR_HOLD_EN, PM800_PWR_HOLD_EN);
+		if (ret)
+			goto error;
+
+		/*
+		 * Set buck2 and buck4 driver selection to be full.
+		 * The default value is 0, for full drive support
+		 * it should be set to 1.
+		 * In A1 version it will be set to 1 by default.
+		 * To be on safer side, set it explicitly
+		 */
+		ret = regmap_update_bits(chip->subchip->regmap_power,
+					PM860_BUCK2_MISC2,
+					PM860_BUCK2_FULL_DRV,
+					PM860_BUCK2_FULL_DRV);
+		if (ret)
+			goto error;
+
+		ret = regmap_update_bits(chip->subchip->regmap_power,
+					PM860_BUCK4_MISC2,
+					PM860_BUCK4_FULL_DRV,
+					PM860_BUCK4_FULL_DRV);
+		if (ret)
+			goto error;
+
+
+		break;
+	default:
+		dev_err(chip->dev, "Unknown device type: %d\n", chip->type);
+		break;
+	}
+
+	return 0;
+
+error:
+	dev_err(chip->dev, "failed to access registers\n");
+	return ret;
+}
+
 static int pm800_probe(struct i2c_client *client,
 				 const struct i2c_device_id *id)
 {
@@ -585,6 +642,13 @@ static int pm800_probe(struct i2c_client *client,
 	if (pdata->plat_config)
 		pdata->plat_config(chip, pdata);
 
+	/* common register configurations , init time only */
+	ret = pm800_init_config(chip, np);
+	if (ret) {
+		dev_err(chip->dev, "Failed to configure 88pm800 devices\n");
+		goto err_device_init;
+	}
+
 	return 0;
 
 err_device_init:
diff --git a/include/linux/mfd/88pm80x.h b/include/linux/mfd/88pm80x.h
index 2e25fb1..2ef62af 100644
--- a/include/linux/mfd/88pm80x.h
+++ b/include/linux/mfd/88pm80x.h
@@ -74,6 +74,7 @@ enum {
 
 /* Wakeup Registers */
 #define PM800_WAKEUP1			(0x0D)
+#define PM800_PWR_HOLD_EN		BIT(7)
 
 #define PM800_WAKEUP2			(0x0E)
 #define PM800_WAKEUP2_INV_INT		BIT(0)
@@ -87,7 +88,10 @@ enum {
 /* Referance and low power registers */
 #define PM800_LOW_POWER1		(0x20)
 #define PM800_LOW_POWER2		(0x21)
+
 #define PM800_LOW_POWER_CONFIG3		(0x22)
+#define PM800_LDOBK_FREEZE		BIT(7)
+
 #define PM800_LOW_POWER_CONFIG4		(0x23)
 
 /* GPIO register */
@@ -279,6 +283,15 @@ enum {
 #define PM805_EARPHONE_SETTING		(0x29)
 #define PM805_AUTO_SEQ_SETTING		(0x2A)
 
+
+/* 88PM860 Registers */
+
+#define PM860_BUCK2_MISC2		(0x7C)
+#define PM860_BUCK2_FULL_DRV		BIT(2)
+
+#define PM860_BUCK4_MISC2		(0x82)
+#define PM860_BUCK4_FULL_DRV		BIT(2)
+
 struct pm80x_rtc_pdata {
 	int		vrtc;
 	int		rtc_wakeup;
-- 
1.9.1

[PATCH 2/6] mfd: 88pm800: Add init time initial configuration support

[Krzysztof Kozlowski]

W dniu 09.07.2015 o 20:47, Vaibhav Hiremath pisze:
> This patch adds init time configuration of 88PM800/805 and
> 88PM860. It includes,
> 
>   - Enable BUCK clock gating in low power mode
>   - Full mode support for BUCK2 and 4
>   - Enable voltage change (LPF, DVC) in PMIC
> 
> Note that both 88PM800 and 88PM860 do share common configurations,
> but since I can not validate the configuration on 88PM800,
> restricting myself only to 88PM860.
> If anyone can validate on 88PM800, we can move common code accordingly.
> 
> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>

Although I am not familiar with the device and driver, patch looks good
to me, except one idea below. Anyway feel free to add:

Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>


> ---
>  drivers/mfd/88pm800.c       | 64 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/88pm80x.h | 13 +++++++++
>  2 files changed, 77 insertions(+)
> 
> diff --git a/drivers/mfd/88pm800.c b/drivers/mfd/88pm800.c
> index 95c8ad4..80a1bc1 100644
> --- a/drivers/mfd/88pm800.c
> +++ b/drivers/mfd/88pm800.c
> @@ -521,6 +521,63 @@ out:
>  	return ret;
>  }
>  
> +static int pm800_init_config(struct pm80x_chip *chip, struct device_node *np)
> +{
> +	int ret;
> +	unsigned int val;
> +
> +	switch (chip->type) {
> +	case CHIP_PM800:
> +	case CHIP_PM805:
> +		break;

It may be useful for future generations to put short notice here why
there is no init for these devices? I saw the explanation in commit
message but still someone in the future may look at the code and wonder
why only 88PM860 is initialized and the others are not?

Best regards,
Krzysztof

> +	case CHIP_PM860:
> +		/* Enable LDO and BUCK clock gating in low power mode */
> +		ret = regmap_update_bits(chip->regmap, PM800_LOW_POWER_CONFIG3,
> +					PM800_LDOBK_FREEZE, PM800_LDOBK_FREEZE);
> +		if (ret)
> +			goto error;
> +
> +		/* Enable voltage change in pmic, POWER_HOLD = 1 */
> +		ret = regmap_update_bits(chip->regmap, PM800_WAKEUP1,
> +					PM800_PWR_HOLD_EN, PM800_PWR_HOLD_EN);
> +		if (ret)
> +			goto error;
> +
> +		/*
> +		 * Set buck2 and buck4 driver selection to be full.
> +		 * The default value is 0, for full drive support
> +		 * it should be set to 1.
> +		 * In A1 version it will be set to 1 by default.
> +		 * To be on safer side, set it explicitly
> +		 */
> +		ret = regmap_update_bits(chip->subchip->regmap_power,
> +					PM860_BUCK2_MISC2,
> +					PM860_BUCK2_FULL_DRV,
> +					PM860_BUCK2_FULL_DRV);
> +		if (ret)
> +			goto error;
> +
> +		ret = regmap_update_bits(chip->subchip->regmap_power,
> +					PM860_BUCK4_MISC2,
> +					PM860_BUCK4_FULL_DRV,
> +					PM860_BUCK4_FULL_DRV);
> +		if (ret)
> +			goto error;
> +
> +
> +		break;
> +	default:
> +		dev_err(chip->dev, "Unknown device type: %d\n", chip->type);
> +		break;
> +	}
> +
> +	return 0;
> +
> +error:
> +	dev_err(chip->dev, "failed to access registers\n");
> +	return ret;
> +}
> +
>  static int pm800_probe(struct i2c_client *client,
>  				 const struct i2c_device_id *id)
>  {
> @@ -585,6 +642,13 @@ static int pm800_probe(struct i2c_client *client,
>  	if (pdata->plat_config)
>  		pdata->plat_config(chip, pdata);
>  
> +	/* common register configurations , init time only */
> +	ret = pm800_init_config(chip, np);
> +	if (ret) {
> +		dev_err(chip->dev, "Failed to configure 88pm800 devices\n");
> +		goto err_device_init;
> +	}
> +
>  	return 0;
>  
>  err_device_init:
> diff --git a/include/linux/mfd/88pm80x.h b/include/linux/mfd/88pm80x.h
> index 2e25fb1..2ef62af 100644
> --- a/include/linux/mfd/88pm80x.h
> +++ b/include/linux/mfd/88pm80x.h
> @@ -74,6 +74,7 @@ enum {
>  
>  /* Wakeup Registers */
>  #define PM800_WAKEUP1			(0x0D)
> +#define PM800_PWR_HOLD_EN		BIT(7)
>  
>  #define PM800_WAKEUP2			(0x0E)
>  #define PM800_WAKEUP2_INV_INT		BIT(0)
> @@ -87,7 +88,10 @@ enum {
>  /* Referance and low power registers */
>  #define PM800_LOW_POWER1		(0x20)
>  #define PM800_LOW_POWER2		(0x21)
> +
>  #define PM800_LOW_POWER_CONFIG3		(0x22)
> +#define PM800_LDOBK_FREEZE		BIT(7)
> +
>  #define PM800_LOW_POWER_CONFIG4		(0x23)
>  
>  /* GPIO register */
> @@ -279,6 +283,15 @@ enum {
>  #define PM805_EARPHONE_SETTING		(0x29)
>  #define PM805_AUTO_SEQ_SETTING		(0x2A)
>  
> +
> +/* 88PM860 Registers */
> +
> +#define PM860_BUCK2_MISC2		(0x7C)
> +#define PM860_BUCK2_FULL_DRV		BIT(2)
> +
> +#define PM860_BUCK4_MISC2		(0x82)
> +#define PM860_BUCK4_FULL_DRV		BIT(2)
> +
>  struct pm80x_rtc_pdata {
>  	int		vrtc;
>  	int		rtc_wakeup;
> 

[PATCH 2/6] mfd: 88pm800: Add init time initial configuration support

[Vaibhav Hiremath]

On Saturday 11 July 2015 12:23 PM, Krzysztof Kozlowski wrote:
> W dniu 09.07.2015 o 20:47, Vaibhav Hiremath pisze:
>> This patch adds init time configuration of 88PM800/805 and
>> 88PM860. It includes,
>>
>>    - Enable BUCK clock gating in low power mode
>>    - Full mode support for BUCK2 and 4
>>    - Enable voltage change (LPF, DVC) in PMIC
>>
>> Note that both 88PM800 and 88PM860 do share common configurations,
>> but since I can not validate the configuration on 88PM800,
>> restricting myself only to 88PM860.
>> If anyone can validate on 88PM800, we can move common code accordingly.
>>
>> Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
>
> Although I am not familiar with the device and driver, patch looks good
> to me, except one idea below. Anyway feel free to add:
>
> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>

Thanks for your review.

>
>> ---
>>   drivers/mfd/88pm800.c       | 64 +++++++++++++++++++++++++++++++++++++++++++++
>>   include/linux/mfd/88pm80x.h | 13 +++++++++
>>   2 files changed, 77 insertions(+)
>>
>> diff --git a/drivers/mfd/88pm800.c b/drivers/mfd/88pm800.c
>> index 95c8ad4..80a1bc1 100644
>> --- a/drivers/mfd/88pm800.c
>> +++ b/drivers/mfd/88pm800.c
>> @@ -521,6 +521,63 @@ out:
>>   	return ret;
>>   }
>>
>> +static int pm800_init_config(struct pm80x_chip *chip, struct device_node *np)
>> +{
>> +	int ret;
>> +	unsigned int val;
>> +
>> +	switch (chip->type) {
>> +	case CHIP_PM800:
>> +	case CHIP_PM805:
>> +		break;
>
> It may be useful for future generations to put short notice here why
> there is no init for these devices? I saw the explanation in commit
> message but still someone in the future may look at the code and wonder
> why only 88PM860 is initialized and the others are not?
>

Yeup, Agreed.

I will incorporate it in V2.

Thanks,
Vaibhav

[PATCH 3/6] mfd: devicetree: bindings: 88pm800: Add DT property for 32KHz output enable

[Vaibhav Hiremath]

88PM800 family of device supports output of 32KHz clock (low jitter)
on CLK32K2/3 pin which can be supplied to other peripherals on the board.

This patch adds the devicetree binding to enable this feature.

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..ae1311c 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,88pm800-32khz-xolj-out-en	: If set, driver will enable low jitter
+   version of 32Khz clock output on
+      CLK32K3 - for 88pm800
+      CLK32K2 - for 88pm860
+
 88pm80x family of devices consists of varied group of sub-devices:
 
 Device		 	Supply Names	 Description
-- 
1.9.1

[PATCH 3/6] mfd: devicetree: bindings: 88pm800: Add DT property for 32KHz output enable

[Krzysztof Kozlowski]

W dniu 09.07.2015 o 20:47, Vaibhav Hiremath pisze:
> 88PM800 family of device supports output of 32KHz clock (low jitter)
> on CLK32K2/3 pin which can be supplied to other peripherals on the board.
> 
> This patch adds the devicetree binding to enable this feature.
> 
> 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..ae1311c 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,88pm800-32khz-xolj-out-en	: If set, driver will enable low jitter
> +   version of 32Khz clock output on

I am not sure if I understand it correctly. The hardware always has such
clocks and you only want to enable/disable it in DT? Any reasons why
these should not be enabled always?

Enabling it in DT does not look like a job for DT. Maybe you there
should be just a clock driver (clock provider)?

Best regards,
Krzysztof

> +      CLK32K3 - for 88pm800
> +      CLK32K2 - for 88pm860
> +
>  88pm80x family of devices consists of varied group of sub-devices:
>  
>  Device		 	Supply Names	 Description
> 

[PATCH 3/6] mfd: devicetree: bindings: 88pm800: Add DT property for 32KHz output enable

[Vaibhav Hiremath]

On Saturday 11 July 2015 12:41 PM, Krzysztof Kozlowski wrote:
> W dniu 09.07.2015 o 20:47, Vaibhav Hiremath pisze:
>> 88PM800 family of device supports output of 32KHz clock (low jitter)
>> on CLK32K2/3 pin which can be supplied to other peripherals on the board.
>>
>> This patch adds the devicetree binding to enable this feature.
>>
>> 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..ae1311c 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,88pm800-32khz-xolj-out-en	: If set, driver will enable low jitter
>> +   version of 32Khz clock output on
>
> I am not sure if I understand it correctly. The hardware always has such
> clocks and you only want to enable/disable it in DT? Any reasons why
> these should not be enabled always?
>

Small amount of Power savings...
Although currently I do not have power numbers to justify this.

As per spec, (it only talks about power consumption in power down state)

Power-down State => VSYS > 2.8   => 4.5 ?A
                     CLK32K2 = 0  => 18 ?W

> Enabling it in DT does not look like a job for DT. Maybe you there
> should be just a clock driver (clock provider)?
>

It's init time (and one time) settings, wouldn't clock-provider
be overkill for this?


Thanks,
Vaibhav

[PATCH 3/6] mfd: devicetree: bindings: 88pm800: Add DT property for 32KHz output enable

[Krzysztof Kozlowski]

On 13.07.2015 16:24, Vaibhav Hiremath wrote:
> 
> 
> On Saturday 11 July 2015 12:41 PM, Krzysztof Kozlowski wrote:
>> W dniu 09.07.2015 o 20:47, Vaibhav Hiremath pisze:
>>> 88PM800 family of device supports output of 32KHz clock (low jitter)
>>> on CLK32K2/3 pin which can be supplied to other peripherals on the
>>> board.
>>>
>>> This patch adds the devicetree binding to enable this feature.
>>>
>>> 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..ae1311c 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,88pm800-32khz-xolj-out-en    : If set, driver will enable
>>> low jitter
>>> +   version of 32Khz clock output on
>>
>> I am not sure if I understand it correctly. The hardware always has such
>> clocks and you only want to enable/disable it in DT? Any reasons why
>> these should not be enabled always?
>>
> 
> Small amount of Power savings...
> Although currently I do not have power numbers to justify this.
> 
> As per spec, (it only talks about power consumption in power down state)
> 
> Power-down State => VSYS > 2.8   => 4.5 ?A
>                     CLK32K2 = 0  => 18 ?W

This would be a power saving if it could be enabled/disabled runtime.
But with DT you will either:
1. enable it always so there won't be any power saving,
2. disable it always so there won't be such clock.

I can find a use case - when on some boards the clock output is not
wired to anything so it cannot be used. In other cases (there is some
potential user) this should be triggered per-use (runtime enabled/disabled).

Do you have such case? I mean boards where this is not connected at all
and boards where this is used?

>> Enabling it in DT does not look like a job for DT. Maybe you there
>> should be just a clock driver (clock provider)?
>>
> 
> It's init time (and one time) settings, wouldn't clock-provider
> be overkill for this?

The clock provider would be a proper way to do it and some PMICs I know
do this. Examples are max77686 and s5m8767/s2mps11. These are PMICs used
for Samsung SoCs. They have two or three 32kHz clocks.

Best regards,
Krzysztof

[PATCH 3/6] mfd: devicetree: bindings: 88pm800: Add DT property for 32KHz output enable

[Vaibhav Hiremath]

On Monday 13 July 2015 01:01 PM, Krzysztof Kozlowski wrote:
> On 13.07.2015 16:24, Vaibhav Hiremath wrote:
>>
>>
>> On Saturday 11 July 2015 12:41 PM, Krzysztof Kozlowski wrote:
>>> W dniu 09.07.2015 o 20:47, Vaibhav Hiremath pisze:
>>>> 88PM800 family of device supports output of 32KHz clock (low jitter)
>>>> on CLK32K2/3 pin which can be supplied to other peripherals on the
>>>> board.
>>>>
>>>> This patch adds the devicetree binding to enable this feature.
>>>>
>>>> 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..ae1311c 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,88pm800-32khz-xolj-out-en    : If set, driver will enable
>>>> low jitter
>>>> +   version of 32Khz clock output on
>>>
>>> I am not sure if I understand it correctly. The hardware always has such
>>> clocks and you only want to enable/disable it in DT? Any reasons why
>>> these should not be enabled always?
>>>
>>
>> Small amount of Power savings...
>> Although currently I do not have power numbers to justify this.
>>
>> As per spec, (it only talks about power consumption in power down state)
>>
>> Power-down State => VSYS > 2.8   => 4.5 ?A
>>                      CLK32K2 = 0  => 18 ?W
>
> This would be a power saving if it could be enabled/disabled runtime.
> But with DT you will either:
> 1. enable it always so there won't be any power saving,
> 2. disable it always so there won't be such clock.
>
> I can find a use case - when on some boards the clock output is not
> wired to anything so it cannot be used. In other cases (there is some
> potential user) this should be triggered per-use (runtime enabled/disabled).
>

Exactly,

> Do you have such case? I mean boards where this is not connected at all
> and boards where this is used?
>

The PXA1928 based board which I have,

CLK32K1 (G3 pad)  = Used by Wireless chip
CLK32K2 (G11 pad) = Not connected to anything, so we need to disable
                     this output

>>> Enabling it in DT does not look like a job for DT. Maybe you there
>>> should be just a clock driver (clock provider)?
>>>
>>
>> It's init time (and one time) settings, wouldn't clock-provider
>> be overkill for this?
>
> The clock provider would be a proper way to do it and some PMICs I know
> do this. Examples are max77686 and s5m8767/s2mps11. These are PMICs used
> for Samsung SoCs. They have two or three 32kHz clocks.
>

This is good to know.
Let me take a look at these drivers.

Thanks,
Vaibhav

[PATCH 3/6] mfd: devicetree: bindings: 88pm800: Add DT property for 32KHz output enable

[Krzysztof Kozlowski]

On 13.07.2015 16:38, Vaibhav Hiremath wrote:
> 
> 
> On Monday 13 July 2015 01:01 PM, Krzysztof Kozlowski wrote:
>> On 13.07.2015 16:24, Vaibhav Hiremath wrote:
>>>
>>>
>>> On Saturday 11 July 2015 12:41 PM, Krzysztof Kozlowski wrote:
>>>> W dniu 09.07.2015 o 20:47, Vaibhav Hiremath pisze:
>>>>> 88PM800 family of device supports output of 32KHz clock (low jitter)
>>>>> on CLK32K2/3 pin which can be supplied to other peripherals on the
>>>>> board.
>>>>>
>>>>> This patch adds the devicetree binding to enable this feature.
>>>>>
>>>>> 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..ae1311c 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,88pm800-32khz-xolj-out-en    : If set, driver will enable
>>>>> low jitter
>>>>> +   version of 32Khz clock output on
>>>>
>>>> I am not sure if I understand it correctly. The hardware always has
>>>> such
>>>> clocks and you only want to enable/disable it in DT? Any reasons why
>>>> these should not be enabled always?
>>>>
>>>
>>> Small amount of Power savings...
>>> Although currently I do not have power numbers to justify this.
>>>
>>> As per spec, (it only talks about power consumption in power down state)
>>>
>>> Power-down State => VSYS > 2.8   => 4.5 ?A
>>>                      CLK32K2 = 0  => 18 ?W
>>
>> This would be a power saving if it could be enabled/disabled runtime.
>> But with DT you will either:
>> 1. enable it always so there won't be any power saving,
>> 2. disable it always so there won't be such clock.
>>
>> I can find a use case - when on some boards the clock output is not
>> wired to anything so it cannot be used. In other cases (there is some
>> potential user) this should be triggered per-use (runtime
>> enabled/disabled).
>>
> 
> Exactly,
> 
>> Do you have such case? I mean boards where this is not connected at all
>> and boards where this is used?
>>
> 
> The PXA1928 based board which I have,
> 
> CLK32K1 (G3 pad)  = Used by Wireless chip
> CLK32K2 (G11 pad) = Not connected to anything, so we need to disable
>                     this output

Okay... however still such binding is uncommon. Such cases are modelled
by using (or not) the clock phandle. So the wireless driver would take
phandle to CLK32K1 clock thus enabling it after probed/started. The
CLK32K2 phandle would be unused and clock would remain disabled.

The binding for device should describe hardware and device always has
these clocks.

> 
>>>> Enabling it in DT does not look like a job for DT. Maybe you there
>>>> should be just a clock driver (clock provider)?
>>>>
>>>
>>> It's init time (and one time) settings, wouldn't clock-provider
>>> be overkill for this?
>>
>> The clock provider would be a proper way to do it and some PMICs I know
>> do this. Examples are max77686 and s5m8767/s2mps11. These are PMICs used
>> for Samsung SoCs. They have two or three 32kHz clocks.
>>
> 
> This is good to know.
> Let me take a look at these drivers.

drivers/mfd/sec-core.c - MFD for the family of PMICs
drivers/clk/clk-s2mps11.c for clocks
The drivers are actually a little more complicated than they should... :)

Best regards,
Krzysztof

[PATCH 4/6] mfd: 88pm800: Enable 32KHZ XO low jitter clock out

[Vaibhav Hiremath]

88PM800/860 device supports output of 32KHz low jitter XO clock
out on
 - CLK32K3 - for 88pm800
 - CLK32K2 - for 88pm860

Both devices share same register bit-field to configure this.

This patch adds support to enable this clock out, using DT property
"marvell,88pm800-32khz-xolj-out-en"

Since this configuration is controlled through DT property, it is
safe to put it as common code.

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

diff --git a/drivers/mfd/88pm800.c b/drivers/mfd/88pm800.c
index 80a1bc1..8930fd8 100644
--- a/drivers/mfd/88pm800.c
+++ b/drivers/mfd/88pm800.c
@@ -526,6 +526,19 @@ static int pm800_init_config(struct pm80x_chip *chip, struct device_node *np)
 	int ret;
 	unsigned int val;
 
+	/* Enable 32Khz-out-3 low jitter XO_LJ = 1 in pm800
+	 * Enable 32Khz-out-2 low jitter XO_LJ = 1 in pm860
+	 * they are the same bit
+	 */
+	if (of_property_read_bool(np, "marvell,88pm800-32khz-xolj-out-en")) {
+		ret = regmap_update_bits(chip->regmap,
+					PM800_LOW_POWER2,
+					PM800_XO_LJ_OUT_EN,
+					PM800_XO_LJ_OUT_EN);
+		if (ret)
+			goto error;
+	}
+
 	switch (chip->type) {
 	case CHIP_PM800:
 	case CHIP_PM805:
diff --git a/include/linux/mfd/88pm80x.h b/include/linux/mfd/88pm80x.h
index 2ef62af..fb916f1 100644
--- a/include/linux/mfd/88pm80x.h
+++ b/include/linux/mfd/88pm80x.h
@@ -88,6 +88,7 @@ enum {
 /* Referance and low power registers */
 #define PM800_LOW_POWER1		(0x20)
 #define PM800_LOW_POWER2		(0x21)
+#define PM800_XO_LJ_OUT_EN		BIT(5)
 
 #define PM800_LOW_POWER_CONFIG3		(0x22)
 #define PM800_LDOBK_FREEZE		BIT(7)
-- 
1.9.1

[PATCH 5/6] 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 adds the devicetree binding to enable this
feature.

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 ae1311c..c756b31 100644
--- a/Documentation/devicetree/bindings/mfd/88pm800.txt
+++ b/Documentation/devicetree/bindings/mfd/88pm800.txt
@@ -15,6 +15,12 @@ Optional properties :
       CLK32K3 - for 88pm800
       CLK32K2 - for 88pm860
 
+   (Applicable only to PXA910 family):
+
+ - 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.
+
 88pm80x family of devices consists of varied group of sub-devices:
 
 Device		 	Supply Names	 Description
-- 
1.9.1

[PATCH 5/6] mfd: devicetree: bindings: 88pm800: Add DT property for dual phase enable

[Krzysztof Kozlowski]

W dniu 09.07.2015 o 20:47, Vaibhav Hiremath pisze:
> 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 adds the devicetree binding to enable this
> feature.
> 
> 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 ae1311c..c756b31 100644
> --- a/Documentation/devicetree/bindings/mfd/88pm800.txt
> +++ b/Documentation/devicetree/bindings/mfd/88pm800.txt
> @@ -15,6 +15,12 @@ Optional properties :
>        CLK32K3 - for 88pm800
>        CLK32K2 - for 88pm860
>  
> +   (Applicable only to PXA910 family):
> +
> + - 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.
> +

1. How does this relates to regulator driver? The
drivers/regulator/88pm800.c defines constraints for regulator which may
be contradictory.

2. This looks like a job for regulator driver, not MFD. Then you could
use standard regulator bindings (setting maximum current to 6A would
change the regulator to different mode).

Best regards,
Krzysztof

[PATCH 5/6] mfd: devicetree: bindings: 88pm800: Add DT property for dual phase enable

[Vaibhav Hiremath]

On Saturday 11 July 2015 12:46 PM, Krzysztof Kozlowski wrote:
> W dniu 09.07.2015 o 20:47, Vaibhav Hiremath pisze:
>> 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 adds the devicetree binding to enable this
>> feature.
>>
>> 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 ae1311c..c756b31 100644
>> --- a/Documentation/devicetree/bindings/mfd/88pm800.txt
>> +++ b/Documentation/devicetree/bindings/mfd/88pm800.txt
>> @@ -15,6 +15,12 @@ Optional properties :
>>         CLK32K3 - for 88pm800
>>         CLK32K2 - for 88pm860
>>
>> +   (Applicable only to PXA910 family):
>> +
>> + - 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.
>> +
>
> 1. How does this relates to regulator driver? The
> drivers/regulator/88pm800.c defines constraints for regulator which may
> be contradictory.
>
> 2. This looks like a job for regulator driver, not MFD. Then you could
> use standard regulator bindings (setting maximum current to 6A would
> change the regulator to different mode).

Make sense. and even better.

I believe you are referring to,

regulator-min-microamp = <3000000>;
regulator-max-microamp = <6000000>;

And provide set_current_limit() callback, to set the dualphase.

Just to clarify more on this,

The DT property would look something like,

regulators {
	compatible = "marvell,88pm80x-regulator";

	buck1a: BUCK1A {
		regulator-compatible = "buck1";
		regulator-min-microvolt = <600000>;
		regulator-max-microvolt = <1800000>;

		regulator-min-microamp = <3000000>;
		regulator-max-microamp = <6000000>;

		regulator-boot-on;
		regulator-always-on;
	};

	buck1b: BUCK1B {
		regulator-compatible = "buck1b";
		regulator-min-microvolt = <600000>;
		regulator-max-microvolt = <1800000>;

		regulator-min-microamp = <3000000>;
		regulator-max-microamp = <6000000>;

		regulator-boot-on;
		regulator-always-on;
	};
};


And for the platforms, where dual phase is not required,
we can either choose not to set these properties or set it to


		regulator-min-microamp = <3000000>;
		regulator-max-microamp = <3000000>;


Thanks,
Vaibhav

[PATCH 5/6] mfd: devicetree: bindings: 88pm800: Add DT property for dual phase enable

[Krzysztof Kozlowski]

On 13.07.2015 16:50, Vaibhav Hiremath wrote:
> 
> 
> On Saturday 11 July 2015 12:46 PM, Krzysztof Kozlowski wrote:
>> W dniu 09.07.2015 o 20:47, Vaibhav Hiremath pisze:
>>> 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 adds the devicetree binding to enable this
>>> feature.
>>>
>>> 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 ae1311c..c756b31 100644
>>> --- a/Documentation/devicetree/bindings/mfd/88pm800.txt
>>> +++ b/Documentation/devicetree/bindings/mfd/88pm800.txt
>>> @@ -15,6 +15,12 @@ Optional properties :
>>>         CLK32K3 - for 88pm800
>>>         CLK32K2 - for 88pm860
>>>
>>> +   (Applicable only to PXA910 family):
>>> +
>>> + - 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.
>>> +
>>
>> 1. How does this relates to regulator driver? The
>> drivers/regulator/88pm800.c defines constraints for regulator which may
>> be contradictory.
>>
>> 2. This looks like a job for regulator driver, not MFD. Then you could
>> use standard regulator bindings (setting maximum current to 6A would
>> change the regulator to different mode).
> 
> Make sense. and even better.
> 
> I believe you are referring to,
> 
> regulator-min-microamp = <3000000>;
> regulator-max-microamp = <6000000>;
> 
> And provide set_current_limit() callback, to set the dualphase.
> 
> Just to clarify more on this,
> 
> The DT property would look something like,
> 
> regulators {
>     compatible = "marvell,88pm80x-regulator";
> 
>     buck1a: BUCK1A {
>         regulator-compatible = "buck1";
>         regulator-min-microvolt = <600000>;
>         regulator-max-microvolt = <1800000>;
> 
>         regulator-min-microamp = <3000000>;
>         regulator-max-microamp = <6000000>;
> 
>         regulator-boot-on;
>         regulator-always-on;
>     };
> 
>     buck1b: BUCK1B {
>         regulator-compatible = "buck1b";
>         regulator-min-microvolt = <600000>;
>         regulator-max-microvolt = <1800000>;
> 
>         regulator-min-microamp = <3000000>;
>         regulator-max-microamp = <6000000>;
> 
>         regulator-boot-on;
>         regulator-always-on;
>     };
> };
> 
> 
> And for the platforms, where dual phase is not required,
> we can either choose not to set these properties or set it to
> 
> 
>         regulator-min-microamp = <3000000>;
>         regulator-max-microamp = <3000000>;

Yes, exactly. I only wonder if regulator core would support properly
such bindings - setting current limit for a voltage regulator. I didn't
tried this but it should work.

Best regards,
Krzysztof

[PATCH 5/6] mfd: devicetree: bindings: 88pm800: Add DT property for dual phase enable

[Vaibhav Hiremath]

On Monday 13 July 2015 01:40 PM, Krzysztof Kozlowski wrote:
> On 13.07.2015 16:50, Vaibhav Hiremath wrote:
>>
>>
>> On Saturday 11 July 2015 12:46 PM, Krzysztof Kozlowski wrote:
>>> W dniu 09.07.2015 o 20:47, Vaibhav Hiremath pisze:
>>>> 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 adds the devicetree binding to enable this
>>>> feature.
>>>>
>>>> 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 ae1311c..c756b31 100644
>>>> --- a/Documentation/devicetree/bindings/mfd/88pm800.txt
>>>> +++ b/Documentation/devicetree/bindings/mfd/88pm800.txt
>>>> @@ -15,6 +15,12 @@ Optional properties :
>>>>          CLK32K3 - for 88pm800
>>>>          CLK32K2 - for 88pm860
>>>>
>>>> +   (Applicable only to PXA910 family):
>>>> +
>>>> + - 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.
>>>> +
>>>
>>> 1. How does this relates to regulator driver? The
>>> drivers/regulator/88pm800.c defines constraints for regulator which may
>>> be contradictory.
>>>
>>> 2. This looks like a job for regulator driver, not MFD. Then you could
>>> use standard regulator bindings (setting maximum current to 6A would
>>> change the regulator to different mode).
>>
>> Make sense. and even better.
>>
>> I believe you are referring to,
>>
>> regulator-min-microamp = <3000000>;
>> regulator-max-microamp = <6000000>;
>>
>> And provide set_current_limit() callback, to set the dualphase.
>>
>> Just to clarify more on this,
>>
>> The DT property would look something like,
>>
>> regulators {
>>      compatible = "marvell,88pm80x-regulator";
>>
>>      buck1a: BUCK1A {
>>          regulator-compatible = "buck1";
>>          regulator-min-microvolt = <600000>;
>>          regulator-max-microvolt = <1800000>;
>>
>>          regulator-min-microamp = <3000000>;
>>          regulator-max-microamp = <6000000>;
>>
>>          regulator-boot-on;
>>          regulator-always-on;
>>      };
>>
>>      buck1b: BUCK1B {
>>          regulator-compatible = "buck1b";
>>          regulator-min-microvolt = <600000>;
>>          regulator-max-microvolt = <1800000>;
>>
>>          regulator-min-microamp = <3000000>;
>>          regulator-max-microamp = <6000000>;
>>
>>          regulator-boot-on;
>>          regulator-always-on;
>>      };
>> };
>>
>>
>> And for the platforms, where dual phase is not required,
>> we can either choose not to set these properties or set it to
>>
>>
>>          regulator-min-microamp = <3000000>;
>>          regulator-max-microamp = <3000000>;
>
> Yes, exactly. I only wonder if regulator core would support properly
> such bindings - setting current limit for a voltage regulator. I didn't
> tried this but it should work.
>

Quickly I created a patch (pasted below) for review,


commit 22e846837d425ff6f98b087b8f93989dd0cfc666
Author: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>
Date:   Mon Jul 13 19:48:40 2015 +0530

     regulator: 88pm800: Add support for configuration of dual phase on 
BUCK1

     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 adds support for regulator_ops.set_current_limit() 
callback fn,
     in turn enabling support for current min and max supply constraint 
on BUCK1
     (and optionally on BUCK1B). Based on that driver enables dual-phase 
mode.

     (and optionally on BUCK1B). Based on that driver enables dual-phase 
mode.

     Note that, if max current supply constraint is > 3A on BUCK1(A) 
then driver
     enables the dual-phase mode, irrespective of BUCK1B constraint.

     Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@linaro.org>

diff --git a/drivers/regulator/88pm800.c b/drivers/regulator/88pm800.c
index ee4b370..7596210 100644
--- a/drivers/regulator/88pm800.c
+++ b/drivers/regulator/88pm800.c
@@ -196,6 +196,42 @@ static int pm800_get_current_limit(struct 
regulator_dev *rdev)
         return info->max_ua;
  }

+/*
+ * 88pm860 device supports dual-phase mode on BUCK1, where BUCK1A and 
BUCK1B can
+ * be used together to supply 6A current.  Note that, independently, 
they can
+ * source 3A each.
+ *
+ * So, this function checks for max_uA for BUCK1 (only), and if it is 
more than
+ * 3A, then enable dual-phase mode.
+ */
+static int pm800_set_current_limit(struct regulator_dev *rdev,
+                               int min_uA, int max_uA)
+{
+       struct pm800_regulators *pm800_data = 
dev_get_drvdata(rdev_get_dev(rdev)->parent);
+       struct pm80x_chip *chip = pm800_data->chip;
+       int ret;
+
+       /* Currently only supported on 88pm860 device */
+       if (chip->type != CHIP_PM860)
+               return 0;
+
+       if (rdev->desc->id == PM800_ID_BUCK1) {
+               /* If max_uA is greater that 3A, enable dual-phase on 
BUCK1 */
+               if (max_uA > 3000000) {
+                       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 access 
registers\n");
+                               return ret;
+                       }
+               }
+       }
+
+       return 0;
+}
+
  static struct regulator_ops pm800_volt_range_ops = {
         .list_voltage = regulator_list_voltage_linear_range,
         .map_voltage = regulator_map_voltage_linear_range,
@@ -205,6 +241,7 @@ static struct regulator_ops pm800_volt_range_ops = {
         .disable = regulator_disable_regmap,
         .is_enabled = regulator_is_enabled_regmap,
         .get_current_limit = pm800_get_current_limit,
+       .set_current_limit = pm800_set_current_limit,
  };

  static struct regulator_ops pm800_volt_table_ops = {

[PATCH 6/6] mfd: 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 adds support to enable dual phase mode, using DT property
"marvell,88pm860-buck1-dualphase-en"

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

diff --git a/drivers/mfd/88pm800.c b/drivers/mfd/88pm800.c
index 8930fd8..a2ef0c7 100644
--- a/drivers/mfd/88pm800.c
+++ b/drivers/mfd/88pm800.c
@@ -556,6 +556,17 @@ static int pm800_init_config(struct pm80x_chip *chip, struct device_node *np)
 		if (ret)
 			goto error;
 
+		/* enable buck1 dual phase mode*/
+		if (of_property_read_bool(np,
+					"marvell,88pm860-buck1-dualphase-en")) {
+			ret = regmap_update_bits(chip->subchip->regmap_power,
+						PM860_BUCK1_MISC,
+						BUCK1_DUAL_PHASE_SEL,
+						BUCK1_DUAL_PHASE_SEL);
+			if (ret)
+				goto error;
+		}
+
 		/*
 		 * Set buck2 and buck4 driver selection to be full.
 		 * The default value is 0, for full drive support
diff --git a/include/linux/mfd/88pm80x.h b/include/linux/mfd/88pm80x.h
index fb916f1..b40d15f 100644
--- a/include/linux/mfd/88pm80x.h
+++ b/include/linux/mfd/88pm80x.h
@@ -293,6 +293,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 0/6] mfd: 88pm800: Add 88pm860 device support

[Krzysztof Kozlowski]

2015-07-09 20:47 GMT+09:00 Vaibhav Hiremath <vaibhav.hiremath@linaro.org>:
> 88PM860 falls under 88pm800 family of devices, with some feature
> additions, for example, support for dual phase on BUCK1.

Hi,

There are already drivers for 88PM860: mfd, charger and battery. Isn't
you duplicating existing code?

Best regards,
Krzysztof

>
> This patch series, enabled chip ID support for 88pm860 in the driver
> and adds,
>   - Init time configuration support based on chip ID
>   - DT property for 32KHz low jitter clock enable
>   - DT property for dual phase enable
>
> TODO:
>  - Some of init time configurations are common to both 88pm800 and 88pm860
>    devices, but since I can not validate it, decided to move it to
>    88pm860 block only.
>    But if someone is willing to help me in validation, we can move it to
>    common code later. No issues there.
>  - Sleep/low-power-mode related configuration is also part of init time,
>    but since we are too far from sleep mode support and without testing
>    I do not want add anything, decided to take it later when we actually
>    start looking at sleep support.
>  - Init time configuration also includes pinmux setting for the device.
>    I am working on using pinctrl-single driver to have standard and generic
>    interface, hopefully it will get handled through pinctrl subsystem.
>    Link to RFC - https://patches.linaro.org/50604/
>
> Vaibhav Hiremath (6):
>   mfd: 88pm80x: Add 88pm860 chip type support
>   mfd: 88pm800: Add init time initial configuration support
>   mfd: devicetree: bindings: 88pm800: Add DT property for 32KHz output
>     enable
>   mfd: 88pm800: Enable 32KHZ XO low jitter clock out
>   mfd: devicetree: bindings: 88pm800: Add DT property for dual phase
>     enable
>   mfd: 88pm800: Add support for configuration of dual phase on BUCK1
>
>  Documentation/devicetree/bindings/mfd/88pm800.txt | 12 ++++
>  drivers/mfd/88pm800.c                             | 88 +++++++++++++++++++++++
>  drivers/mfd/88pm80x.c                             |  2 +
>  include/linux/mfd/88pm80x.h                       | 18 +++++
>  4 files changed, 120 insertions(+)
>
> --
> 1.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 0/6] mfd: 88pm800: Add 88pm860 device support

[Vaibhav Hiremath]

On Thursday 09 July 2015 05:34 PM, Krzysztof Kozlowski wrote:
> 2015-07-09 20:47 GMT+09:00 Vaibhav Hiremath <vaibhav.hiremath@linaro.org>:
>> 88PM860 falls under 88pm800 family of devices, with some feature
>> additions, for example, support for dual phase on BUCK1.
>
> Hi,
>
> There are already drivers for 88PM860: mfd, charger and battery. Isn't
> you duplicating existing code?
>

Can you please point me to MFD driver for 88PM860 (Not the charger and
battery, as I treat them subdevice) ???

I do not think 88PM860 MFD driver is supported already. Correct me if I
am wrong.

Thanks,
Vaibhav

[PATCH 0/6] mfd: 88pm800: Add 88pm860 device support

[Vaibhav Hiremath]

On Thursday 09 July 2015 06:14 PM, Vaibhav Hiremath wrote:
>
>
> On Thursday 09 July 2015 05:34 PM, Krzysztof Kozlowski wrote:
>> 2015-07-09 20:47 GMT+09:00 Vaibhav Hiremath
>> <vaibhav.hiremath@linaro.org>:
>>> 88PM860 falls under 88pm800 family of devices, with some feature
>>> additions, for example, support for dual phase on BUCK1.
>>
>> Hi,
>>
>> There are already drivers for 88PM860: mfd, charger and battery. Isn't
>> you duplicating existing code?
>>
>
> Can you please point me to MFD driver for 88PM860 (Not the charger and
> battery, as I treat them subdevice) ???
>
> I do not think 88PM860 MFD driver is supported already. Correct me if I
> am wrong.
>

If you are referring to 88pm860x driver, then the answer is NO,
as they both are totally different.

Thanks,
Vaibhav

[PATCH 0/6] mfd: 88pm800: Add 88pm860 device support

[Krzysztof Kozlowski]

2015-07-09 21:53 GMT+09:00 Vaibhav Hiremath <vaibhav.hiremath@linaro.org>:
>
>
> On Thursday 09 July 2015 06:14 PM, Vaibhav Hiremath wrote:
>>
>>
>>
>> On Thursday 09 July 2015 05:34 PM, Krzysztof Kozlowski wrote:
>>>
>>> 2015-07-09 20:47 GMT+09:00 Vaibhav Hiremath
>>> <vaibhav.hiremath@linaro.org>:
>>>>
>>>> 88PM860 falls under 88pm800 family of devices, with some feature
>>>> additions, for example, support for dual phase on BUCK1.
>>>
>>>
>>> Hi,
>>>
>>> There are already drivers for 88PM860: mfd, charger and battery. Isn't
>>> you duplicating existing code?
>>>
>>
>> Can you please point me to MFD driver for 88PM860 (Not the charger and
>> battery, as I treat them subdevice) ???
>>
>> I do not think 88PM860 MFD driver is supported already. Correct me if I
>> am wrong.
>>
>
> If you are referring to 88pm860x driver, then the answer is NO,
> as they both are totally different.

I am confused... so the driver drivers/mfd/88pm860x-core.c and its
child drivers do not support 88pm860 device? They support 88pm860x
device which is totally different than 88pm860 device?

Best regards,
Krzysztof

[PATCH 0/6] mfd: 88pm800: Add 88pm860 device support

[Vaibhav Hiremath]

On Thursday 09 July 2015 06:58 PM, Krzysztof Kozlowski wrote:
> 2015-07-09 21:53 GMT+09:00 Vaibhav Hiremath <vaibhav.hiremath@linaro.org>:
>>
>>
>> On Thursday 09 July 2015 06:14 PM, Vaibhav Hiremath wrote:
>>>
>>>
>>>
>>> On Thursday 09 July 2015 05:34 PM, Krzysztof Kozlowski wrote:
>>>>
>>>> 2015-07-09 20:47 GMT+09:00 Vaibhav Hiremath
>>>> <vaibhav.hiremath@linaro.org>:
>>>>>
>>>>> 88PM860 falls under 88pm800 family of devices, with some feature
>>>>> additions, for example, support for dual phase on BUCK1.
>>>>
>>>>
>>>> Hi,
>>>>
>>>> There are already drivers for 88PM860: mfd, charger and battery. Isn't
>>>> you duplicating existing code?
>>>>
>>>
>>> Can you please point me to MFD driver for 88PM860 (Not the charger and
>>> battery, as I treat them subdevice) ???
>>>
>>> I do not think 88PM860 MFD driver is supported already. Correct me if I
>>> am wrong.
>>>
>>
>> If you are referring to 88pm860x driver, then the answer is NO,
>> as they both are totally different.
>
> I am confused... so the driver drivers/mfd/88pm860x-core.c and its
> child drivers do not support 88pm860 device? They support 88pm860x
> device which is totally different than 88pm860 device?
>

Yes, that's correct.
That's my understanding as well.

Thanks,
Vaibhav

[PATCH 0/6] mfd: 88pm800: Add 88pm860 device support

[Krzysztof Kozlowski]

2015-07-10 21:03 GMT+09:00 Vaibhav Hiremath <vaibhav.hiremath@linaro.org>:
>
>
> On Thursday 09 July 2015 06:58 PM, Krzysztof Kozlowski wrote:
>>
>> 2015-07-09 21:53 GMT+09:00 Vaibhav Hiremath <vaibhav.hiremath@linaro.org>:
>>>
>>>
>>>
>>> On Thursday 09 July 2015 06:14 PM, Vaibhav Hiremath wrote:
>>>>
>>>>
>>>>
>>>>
>>>> On Thursday 09 July 2015 05:34 PM, Krzysztof Kozlowski wrote:
>>>>>
>>>>>
>>>>> 2015-07-09 20:47 GMT+09:00 Vaibhav Hiremath
>>>>> <vaibhav.hiremath@linaro.org>:
>>>>>>
>>>>>>
>>>>>> 88PM860 falls under 88pm800 family of devices, with some feature
>>>>>> additions, for example, support for dual phase on BUCK1.
>>>>>
>>>>>
>>>>>
>>>>> Hi,
>>>>>
>>>>> There are already drivers for 88PM860: mfd, charger and battery. Isn't
>>>>> you duplicating existing code?
>>>>>
>>>>
>>>> Can you please point me to MFD driver for 88PM860 (Not the charger and
>>>> battery, as I treat them subdevice) ???
>>>>
>>>> I do not think 88PM860 MFD driver is supported already. Correct me if I
>>>> am wrong.
>>>>
>>>
>>> If you are referring to 88pm860x driver, then the answer is NO,
>>> as they both are totally different.
>>
>>
>> I am confused... so the driver drivers/mfd/88pm860x-core.c and its
>> child drivers do not support 88pm860 device? They support 88pm860x
>> device which is totally different than 88pm860 device?
>>
>
> Yes, that's correct.
> That's my understanding as well.

Ok, thanks for patient clarification!

Best regards,
Krzysztof