SMPS10 supports different modes such as BOOST mode, BYPASS mode and
SWITCH. Inorder to configure SMPS10 in these modes,
added palmas_set_mode_smps10() and palmas_get_mode_smps10() for the
consumers of SMPS10 to configure it accordingly.
Signed-off-by: Kishon Vijay Abraham I <[email protected]>
---
Only compile tested. Just sent a patch to get some comments
/ideas on how to handle such one off regulators.
to handle
drivers/regulator/palmas-regulator.c | 46 ++++++++++++++++++++++++++++++++++
include/linux/mfd/palmas.h | 1 +
2 files changed, 47 insertions(+)
diff --git a/drivers/regulator/palmas-regulator.c b/drivers/regulator/palmas-regulator.c
index 87d4f13..d8d37f2 100644
--- a/drivers/regulator/palmas-regulator.c
+++ b/drivers/regulator/palmas-regulator.c
@@ -355,6 +355,50 @@ static unsigned int palmas_get_mode_smps(struct regulator_dev *dev)
return 0;
}
+static int palmas_set_mode_smps10(struct regulator_dev *dev, unsigned int mode)
+{
+ struct palmas_pmic *pmic = rdev_get_drvdata(dev);
+ int id = rdev_get_id(dev);
+ unsigned int reg;
+
+ palmas_smps_read(pmic->palmas, palmas_regs_info[id].ctrl_addr, ®);
+ reg &= ~PALMAS_SMPS10_CTRL_MODE_ACTIVE_MODE_MASK;
+
+ if (mode == REGULATOR_MODE_NORMAL)
+ reg |= SMPS10_BOOST_EN;
+
+ if (mode == REGULATOR_MODE_FAST)
+ reg |= SMPS10_SWITCH_EN;
+
+ if (mode == REGULATOR_MODE_IDLE)
+ reg |= SMPS10_BYPASS_EN;
+
+ palmas_smps_write(pmic->palmas, palmas_regs_info[id].ctrl_addr, reg);
+ return 0;
+}
+
+static unsigned int palmas_get_mode_smps10(struct regulator_dev *dev)
+{
+ struct palmas_pmic *pmic = rdev_get_drvdata(dev);
+ int id = rdev_get_id(dev);
+ unsigned int reg;
+ int ret = 0;
+
+ reg = pmic->current_reg_mode[id] &
+ PALMAS_SMPS10_CTRL_MODE_ACTIVE_MODE_MASK;
+
+ if (reg == SMPS10_BOOST_EN)
+ ret |= REGULATOR_MODE_NORMAL;
+
+ if (reg == SMPS10_SWITCH_EN)
+ ret |= REGULATOR_MODE_FAST;
+
+ if (reg == SMPS10_BYPASS_EN)
+ ret |= REGULATOR_MODE_IDLE;
+
+ return ret;
+}
+
static int palmas_list_voltage_smps(struct regulator_dev *dev,
unsigned selector)
{
@@ -483,6 +527,8 @@ static struct regulator_ops palmas_ops_smps10 = {
.is_enabled = regulator_is_enabled_regmap,
.enable = regulator_enable_regmap,
.disable = regulator_disable_regmap,
+ .set_mode = palmas_set_mode_smps10,
+ .get_mode = palmas_get_mode_smps10,
.get_voltage_sel = regulator_get_voltage_sel_regmap,
.set_voltage_sel = regulator_set_voltage_sel_regmap,
.list_voltage = regulator_list_voltage_linear,
diff --git a/include/linux/mfd/palmas.h b/include/linux/mfd/palmas.h
index 8f21daf..4ad2b65 100644
--- a/include/linux/mfd/palmas.h
+++ b/include/linux/mfd/palmas.h
@@ -846,6 +846,7 @@ enum usb_irq_events {
#define PALMAS_SMPS10_CTRL_MODE_SLEEP_MASK 0xf0
#define PALMAS_SMPS10_CTRL_MODE_SLEEP_SHIFT 4
#define PALMAS_SMPS10_CTRL_MODE_ACTIVE_MASK 0x0f
+#define PALMAS_SMPS10_CTRL_MODE_ACTIVE_MODE_MASK 0x07
#define PALMAS_SMPS10_CTRL_MODE_ACTIVE_SHIFT 0
/* Bit definitions for SMPS10_STATUS */
--
1.7.10.4
On Thu, May 30, 2013 at 04:26:33PM +0530, Kishon Vijay Abraham I wrote:
> Only compile tested. Just sent a patch to get some comments
> /ideas on how to handle such one off regulators.
> to handle
What's unclear or confusing? This all looks really basic...
> + palmas_smps_read(pmic->palmas, palmas_regs_info[id].ctrl_addr, ®);
> + reg &= ~PALMAS_SMPS10_CTRL_MODE_ACTIVE_MODE_MASK;
> +
> + if (mode == REGULATOR_MODE_NORMAL)
> + reg |= SMPS10_BOOST_EN;
> +
> + if (mode == REGULATOR_MODE_FAST)
> + reg |= SMPS10_SWITCH_EN;
> +
> + if (mode == REGULATOR_MODE_IDLE)
> + reg |= SMPS10_BYPASS_EN;
> +
> + palmas_smps_write(pmic->palmas, palmas_regs_info[id].ctrl_addr, reg);
> + return 0;
This looks like a switch statement and isn't there an update bits
operation?
Hi,
On Thursday 30 May 2013 05:02 PM, Mark Brown wrote:
> On Thu, May 30, 2013 at 04:26:33PM +0530, Kishon Vijay Abraham I wrote:
>
>> Only compile tested. Just sent a patch to get some comments
>> /ideas on how to handle such one off regulators.
>> to handle
>
> What's unclear or confusing? This all looks really basic...
For instance mapping of regulator modes to smps10 modes is unclear.
>
>> + palmas_smps_read(pmic->palmas, palmas_regs_info[id].ctrl_addr, ®);
>> + reg &= ~PALMAS_SMPS10_CTRL_MODE_ACTIVE_MODE_MASK;
>> +
>> + if (mode == REGULATOR_MODE_NORMAL)
>> + reg |= SMPS10_BOOST_EN;
>> +
>> + if (mode == REGULATOR_MODE_FAST)
>> + reg |= SMPS10_SWITCH_EN;
>> +
>> + if (mode == REGULATOR_MODE_IDLE)
>> + reg |= SMPS10_BYPASS_EN;
>> +
>> + palmas_smps_write(pmic->palmas, palmas_regs_info[id].ctrl_addr, reg);
>> + return 0;
>
> This looks like a switch statement and isn't there an update bits
> operation?
There can be multiple modes set at the same time. Having switch
statement means we would need to call the same API multiple times to set
the mode. There isn't a palmas wrapper to regmap_update_bits. I can send
a patch to add a palmas wrapper.
Thanks
Kishon
HI Mark,
On Thursday 30 May 2013 05:02 PM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Thu, May 30, 2013 at 04:26:33PM +0530, Kishon Vijay Abraham I wrote:
>
>> Only compile tested. Just sent a patch to get some comments
>> /ideas on how to handle such one off regulators.
>> to handle
> What's unclear or confusing? This all looks really basic...
Palma have SMPS10 regulator which can generate two voltage level 3.75
and 5V.
This SMPS10 has the two outputs OUT1 and OUT2 and having one input IN1.
SMPS10-OUT2 is always connected to SMPS10-IN1 via following logic:
- Through parasitic diode (no sw control)
- In bypass mode (bit configuration is there to enable/disable Bypass)
- In Boost mode (bit configuration is there to enable/disable Boost mode)
SMPS10-OUT1 is connected to the SMPS10-OUT2 pin through Switch (SW
control for enabling/disabling this switch).
So I think:
regualtor enable/disable, we should toggle the bit for SWITCH.
In STANDBY mode, it should be BYPASS disable and Boost disable.
In Idle/Normal mode, BYPASS enable and Boost disable.
In Fast mode, it should be bypass disable and Boost enable.
Kishon,
Do you agree here?
On Thu, May 30, 2013 at 06:24:37PM +0530, Kishon Vijay Abraham I wrote:
> On Thursday 30 May 2013 05:02 PM, Mark Brown wrote:
> >On Thu, May 30, 2013 at 04:26:33PM +0530, Kishon Vijay Abraham I wrote:
> >>Only compile tested. Just sent a patch to get some comments
> >>/ideas on how to handle such one off regulators.
> >>to handle
> >What's unclear or confusing? This all looks really basic...
> For instance mapping of regulator modes to smps10 modes is unclear.
If you need clarification on that you need to ask a question about it.
There is no question in your code.
> >>+ palmas_smps_read(pmic->palmas, palmas_regs_info[id].ctrl_addr, ®);
> >>+ reg &= ~PALMAS_SMPS10_CTRL_MODE_ACTIVE_MODE_MASK;
> >>+
> >>+ if (mode == REGULATOR_MODE_NORMAL)
> >>+ reg |= SMPS10_BOOST_EN;
> >This looks like a switch statement and isn't there an update bits
> >operation?
> There can be multiple modes set at the same time. Having switch
No they can't and if you check your code you'll see that it's correctly
doing that - you're checking if the supplied mode is equal to a given
value...
On Thu, May 30, 2013 at 06:30:32PM +0530, Laxman Dewangan wrote:
> Palma have SMPS10 regulator which can generate two voltage level
> 3.75 and 5V.
> This SMPS10 has the two outputs OUT1 and OUT2 and having one input IN1.
> SMPS10-OUT2 is always connected to SMPS10-IN1 via following logic:
> - Through parasitic diode (no sw control)
> - In bypass mode (bit configuration is there to enable/disable Bypass)
> - In Boost mode (bit configuration is there to enable/disable Boost mode)
> SMPS10-OUT1 is connected to the SMPS10-OUT2 pin through Switch (SW
> control for enabling/disabling this switch).
> So I think:
> regualtor enable/disable, we should toggle the bit for SWITCH.
> In STANDBY mode, it should be BYPASS disable and Boost disable.
> In Idle/Normal mode, BYPASS enable and Boost disable.
> In Fast mode, it should be bypass disable and Boost enable.
No, that makes no sense at all to me. Why do you think this maps onto
the set mode API? Modes are all about accuracy of regulation.
On Sunday 02 June 2013 12:15 AM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Thu, May 30, 2013 at 06:30:32PM +0530, Laxman Dewangan wrote:
>
>> Palma have SMPS10 regulator which can generate two voltage level
>> 3.75 and 5V.
>> This SMPS10 has the two outputs OUT1 and OUT2 and having one input IN1.
>> SMPS10-OUT2 is always connected to SMPS10-IN1 via following logic:
>> - Through parasitic diode (no sw control)
>> - In bypass mode (bit configuration is there to enable/disable Bypass)
>> - In Boost mode (bit configuration is there to enable/disable Boost mode)
>> SMPS10-OUT1 is connected to the SMPS10-OUT2 pin through Switch (SW
>> control for enabling/disabling this switch).
>> So I think:
>> regualtor enable/disable, we should toggle the bit for SWITCH.
>> In STANDBY mode, it should be BYPASS disable and Boost disable.
>> In Idle/Normal mode, BYPASS enable and Boost disable.
>> In Fast mode, it should be bypass disable and Boost enable.
> No, that makes no sense at all to me. Why do you think this maps onto
> the set mode API? Modes are all about accuracy of regulation.
I mapped this to the regulation under different load, Fast mode is in
heavy load and so boost enable, normal/idle mode for normal load and so
bypass.
On Sun, Jun 02, 2013 at 12:33:10AM +0530, Laxman Dewangan wrote:
> On Sunday 02 June 2013 12:15 AM, Mark Brown wrote:
> >No, that makes no sense at all to me. Why do you think this maps onto
> >the set mode API? Modes are all about accuracy of regulation.
> I mapped this to the regulation under different load, Fast mode is
> in heavy load and so boost enable, normal/idle mode for normal load
> and so bypass.
This is still not making any sense. The quality of regulation and
output voltage are essentially orthogonal, and obviously there's a
specific API for bypass which is something different again to both
mode and output voltage selection.
Hi,
On Sunday 02 June 2013 02:35 AM, Mark Brown wrote:
> On Sun, Jun 02, 2013 at 12:33:10AM +0530, Laxman Dewangan wrote:
>> On Sunday 02 June 2013 12:15 AM, Mark Brown wrote:
>
>>> No, that makes no sense at all to me. Why do you think this maps onto
>>> the set mode API? Modes are all about accuracy of regulation.
>
>> I mapped this to the regulation under different load, Fast mode is
>> in heavy load and so boost enable, normal/idle mode for normal load
>> and so bypass.
>
> This is still not making any sense. The quality of regulation and
> output voltage are essentially orthogonal, and obviously there's a
> specific API for bypass which is something different again to both
> mode and output voltage selection.
Do you recommend adding API's (similar to bypass) for BOOST and SWITCH?
Thanks
Kishon
On Tue, Jun 04, 2013 at 02:38:08PM +0530, Kishon Vijay Abraham I wrote:
> On Sunday 02 June 2013 02:35 AM, Mark Brown wrote:
> >This is still not making any sense. The quality of regulation and
> >output voltage are essentially orthogonal, and obviously there's a
> >specific API for bypass which is something different again to both
> >mode and output voltage selection.
> Do you recommend adding API's (similar to bypass) for BOOST and SWITCH?
No. A boost regulator is still a voltage regulator, it just happens to
raise rather than lower the voltage but it's otherwise a normal
regulator. I'm not sure what you mean by "SWITCH" - from the name it'd
just be a bypass?
Hi,
On Tuesday 04 June 2013 03:08 PM, Mark Brown wrote:
> On Tue, Jun 04, 2013 at 02:38:08PM +0530, Kishon Vijay Abraham I wrote:
>> On Sunday 02 June 2013 02:35 AM, Mark Brown wrote:
>
>>> This is still not making any sense. The quality of regulation and
>>> output voltage are essentially orthogonal, and obviously there's a
>>> specific API for bypass which is something different again to both
>>> mode and output voltage selection.
>
>> Do you recommend adding API's (similar to bypass) for BOOST and SWITCH?
>
> No. A boost regulator is still a voltage regulator, it just happens to
> raise rather than lower the voltage but it's otherwise a normal
> regulator. I'm not sure what you mean by "SWITCH" - from the name it'd
> just be a bypass?
Copy-pasting Laxman's explanation
<...>
Palma have SMPS10 regulator which can generate two voltage level 3.75
and 5V.
This SMPS10 has the two outputs OUT1 and OUT2 and having one input IN1.
SMPS10-OUT2 is always connected to SMPS10-IN1 via following logic:
- Through parasitic diode (no sw control)
- In bypass mode (bit configuration is there to enable/disable Bypass)
- In Boost mode (bit configuration is there to enable/disable Boost mode)
SMPS10-OUT1 is connected to the SMPS10-OUT2 pin through Switch (SW
control for enabling/disabling this switch).
<...>
Thanks
Kishon
On Tue, Jun 04, 2013 at 03:13:27PM +0530, Kishon Vijay Abraham I wrote:
> On Tuesday 04 June 2013 03:08 PM, Mark Brown wrote:
> >No. A boost regulator is still a voltage regulator, it just happens to
> >raise rather than lower the voltage but it's otherwise a normal
> >regulator. I'm not sure what you mean by "SWITCH" - from the name it'd
> >just be a bypass?
> Copy-pasting Laxman's explanation
That doesn't clarify anything at all.
Hi,
On Tuesday 04 June 2013 03:35 PM, Mark Brown wrote:
> On Tue, Jun 04, 2013 at 03:13:27PM +0530, Kishon Vijay Abraham I wrote:
>> On Tuesday 04 June 2013 03:08 PM, Mark Brown wrote:
>
>>> No. A boost regulator is still a voltage regulator, it just happens to
>>> raise rather than lower the voltage but it's otherwise a normal
>>> regulator. I'm not sure what you mean by "SWITCH" - from the name it'd
>>> just be a bypass?
SMPS10 has two outputs OUT2 and OUT1. In order to connect OUT2 to OUT1
we have to set "SWITCH_EN" bit in SMPS10_CTRL register.
Thanks
Kishon
On Mon, Jun 10, 2013 at 03:20:06PM +0530, Kishon Vijay Abraham I wrote:
> On Tuesday 04 June 2013 03:35 PM, Mark Brown wrote:
> >On Tue, Jun 04, 2013 at 03:13:27PM +0530, Kishon Vijay Abraham I wrote:
> >>On Tuesday 04 June 2013 03:08 PM, Mark Brown wrote:
> >>>No. A boost regulator is still a voltage regulator, it just happens to
> >>>raise rather than lower the voltage but it's otherwise a normal
> >>>regulator. I'm not sure what you mean by "SWITCH" - from the name it'd
> >>>just be a bypass?
> SMPS10 has two outputs OUT2 and OUT1. In order to connect OUT2 to
> OUT1 we have to set "SWITCH_EN" bit in SMPS10_CTRL register.
Is that not just a normal enable?
Hi,
On Monday 10 June 2013 03:33 PM, Mark Brown wrote:
> On Mon, Jun 10, 2013 at 03:20:06PM +0530, Kishon Vijay Abraham I wrote:
>> On Tuesday 04 June 2013 03:35 PM, Mark Brown wrote:
>>> On Tue, Jun 04, 2013 at 03:13:27PM +0530, Kishon Vijay Abraham I wrote:
>>>> On Tuesday 04 June 2013 03:08 PM, Mark Brown wrote:
>
>>>>> No. A boost regulator is still a voltage regulator, it just happens to
>>>>> raise rather than lower the voltage but it's otherwise a normal
>>>>> regulator. I'm not sure what you mean by "SWITCH" - from the name it'd
>>>>> just be a bypass?
>
>> SMPS10 has two outputs OUT2 and OUT1. In order to connect OUT2 to
>> OUT1 we have to set "SWITCH_EN" bit in SMPS10_CTRL register.
>
> Is that not just a normal enable?
IIUC, normal enable should be when output is driven through OUT2.
Thanks
Kishon
On Mon, Jun 10, 2013 at 04:33:04PM +0530, Kishon Vijay Abraham I wrote:
> On Monday 10 June 2013 03:33 PM, Mark Brown wrote:
> >On Mon, Jun 10, 2013 at 03:20:06PM +0530, Kishon Vijay Abraham I wrote:
> >>SMPS10 has two outputs OUT2 and OUT1. In order to connect OUT2 to
> >>OUT1 we have to set "SWITCH_EN" bit in SMPS10_CTRL register.
> >Is that not just a normal enable?
> IIUC, normal enable should be when output is driven through OUT2.
I'm sorry but you're really not making much sense at all here. You say
there are two different outputs here...
Please think about what the hardware is doing and try to map it onto the
APIs in a coherent fashion - pasting in large sections of dataseet isn't
usually helpful, the goal is to map it onto abstractions.
On Monday 10 June 2013 06:19 PM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Mon, Jun 10, 2013 at 04:33:04PM +0530, Kishon Vijay Abraham I wrote:
>> IIUC, normal enable should be when output is driven through OUT2.
> I'm sorry but you're really not making much sense at all here. You say
> there are two different outputs here...
>
> Please think about what the hardware is doing and try to map it onto the
> APIs in a coherent fashion - pasting in large sections of dataseet isn't
> usually helpful, the goal is to map it onto abstractions.
>
I think ff we really want to abstract this stuff then we should have the
two different regulator SMPS10-OUT1 and SMPS-OUT2 as there is physically
two pins in the device for output and controlled different way.
On Wed, Jun 12, 2013 at 02:12:07PM +0530, Laxman Dewangan wrote:
> I think ff we really want to abstract this stuff then we should have
> the two different regulator SMPS10-OUT1 and SMPS-OUT2 as there is
> physically two pins in the device for output and controlled
> different way.
Yes, that seems like the most obvious thing here as far as I can tell
from the frankly fragmented and obscure information but given all the
confusion that seems to exist among the people working on the driver
it's really not clear if there's something else going on.
On 2013-06-12 15:14, Mark Brown wrote:
> On Wed, Jun 12, 2013 at 02:12:07PM +0530, Laxman Dewangan wrote:
>
>> I think ff we really want to abstract this stuff then we should have
>> the two different regulator SMPS10-OUT1 and SMPS-OUT2 as there is
>> physically two pins in the device for output and controlled
>> different way.
>
> Yes, that seems like the most obvious thing here as far as I can tell
> from the frankly fragmented and obscure information but given all the
> confusion that seems to exist among the people working on the driver
> it's really not clear if there's something else going on.
What about the case where they are inputs? For bus power?
Graeme
On Wed, Jun 12, 2013 at 03:31:09PM +0100, [email protected] wrote:
> On 2013-06-12 15:14, Mark Brown wrote:
> >Yes, that seems like the most obvious thing here as far as I can tell
> >from the frankly fragmented and obscure information but given all the
> >confusion that seems to exist among the people working on the driver
> >it's really not clear if there's something else going on.
> What about the case where they are inputs? For bus power?
So the current flow can change direction here?
On 2013-06-12 15:51, Mark Brown wrote:
> On Wed, Jun 12, 2013 at 03:31:09PM +0100, [email protected] wrote:
>> On 2013-06-12 15:14, Mark Brown wrote:
>
>> >Yes, that seems like the most obvious thing here as far as I can tell
>> >from the frankly fragmented and obscure information but given all the
>> >confusion that seems to exist among the people working on the driver
>> >it's really not clear if there's something else going on.
>
>> What about the case where they are inputs? For bus power?
>
> So the current flow can change direction here?
Just rechecked the data sheet and it looks like that use-case has
actually been
removed from more recent data sheets.
So ignore me, yes now it can be modelled by two regulators!
Graeme
On Wednesday 12 June 2013 08:53 PM, [email protected] wrote:
> On 2013-06-12 15:51, Mark Brown wrote:
>> On Wed, Jun 12, 2013 at 03:31:09PM +0100, [email protected] wrote:
>>> On 2013-06-12 15:14, Mark Brown wrote:
>>>> Yes, that seems like the most obvious thing here as far as I can tell
>>> >from the frankly fragmented and obscure information but given all the
>>>> confusion that seems to exist among the people working on the driver
>>>> it's really not clear if there's something else going on.
>>> What about the case where they are inputs? For bus power?
>> So the current flow can change direction here?
> Just rechecked the data sheet and it looks like that use-case has
> actually been
> removed from more recent data sheets.
>
> So ignore me, yes now it can be modelled by two regulators!
Thanks for concluding on this.
Kishon,
Are you going to send new patch for splitting this regulator or should I
send for this?
This will help on populating dts file what Keerthi is sending and also
to me for the tegra platform.
Hi,
On Friday 14 June 2013 06:04 PM, Laxman Dewangan wrote:
> On Wednesday 12 June 2013 08:53 PM, [email protected] wrote:
>> On 2013-06-12 15:51, Mark Brown wrote:
>>> On Wed, Jun 12, 2013 at 03:31:09PM +0100, [email protected] wrote:
>>>> On 2013-06-12 15:14, Mark Brown wrote:
>>>>> Yes, that seems like the most obvious thing here as far as I can tell
>>>> >from the frankly fragmented and obscure information but given all the
>>>>> confusion that seems to exist among the people working on the driver
>>>>> it's really not clear if there's something else going on.
>>>> What about the case where they are inputs? For bus power?
>>> So the current flow can change direction here?
>> Just rechecked the data sheet and it looks like that use-case has
>> actually been
>> removed from more recent data sheets.
>>
>> So ignore me, yes now it can be modelled by two regulators!
>
> Thanks for concluding on this.
>
> Kishon,
> Are you going to send new patch for splitting this regulator or should I
> send for this?
I'll send by today or tomorrow.
Thanks
Kishon