[PATCH v2 0/4] MFD: Palmas: Add TPS659038 PMIC support on Palmas

[PATCH v2 0/4] MFD: Palmas: Add TPS659038 PMIC support on Palmas

[J Keerthy]

The Patch series adds TPS659038 PMIC support in the palmas MFD and Regulator
drivers. The TPS659038 has almost the same registers as of the earlier
supported variants of PALMAS family such as the TWL6035.

The critical differences between TPS659038 and TWL6035 being:

1) TPS659038 has nothing related to battery charging and back up battery stuff.
2) TPS659038 does not have does not have SMPS10(Boost) step up convertor.
3) TPS659038 does not have Battery detection and anything related to battery.
4) SD card detection, Battery presence detection, Vibrator, USB OTG are missing
   when compared to TWL6035.

The patch series is based on the patch:
	http://www.mail-archive.com/linux-omap@vger.kernel.org/msg90598.html

V2:

Implemented Interrupts checking via DT instead of creating flags
and checking based on chip ID.
 
J Keerthy (4):
  MFD: Palmas: Check if interrupts property exists and then only
    request irq
  MFD: Palmas: Add SMPS10_BOOST feature
  mfd: Palmas: Add TPS659038 PMIC support
  regulator: Palmas: Add TPS659038 support

 Documentation/devicetree/bindings/mfd/palmas.txt   |    2 +
 .../devicetree/bindings/regulator/palmas-pmic.txt  |    1 +
 drivers/mfd/palmas.c                               |   45 ++++++++++++++++----
 drivers/regulator/palmas-regulator.c               |    4 ++
 include/linux/mfd/palmas.h                         |   14 ++++++
 5 files changed, 58 insertions(+), 8 deletions(-)

-- 
1.7.5.4

[PATCH v2 1/4] MFD: Palmas: Check if interrupts property exists and then only request irq

[J Keerthy]

Check if interrupts property exists and then only request irq.
On some boards INT line might not be connected to a valid
irq line on the application processor. Hence keeping a check
before requesting irq.

Boot tested on OMAP5-UEVM board.

Signed-off-by: J Keerthy <j-keerthy@ti.com>
---
 drivers/mfd/palmas.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c
index 62fa728..69c4e62 100644
--- a/drivers/mfd/palmas.c
+++ b/drivers/mfd/palmas.c
@@ -236,6 +236,7 @@ static int palmas_i2c_probe(struct i2c_client *i2c,
 {
 	struct palmas *palmas;
 	struct palmas_platform_data *pdata;
+	struct device_node *regnode = NULL;
 	struct device_node *node = i2c->dev.of_node;
 	int ret = 0, i;
 	unsigned int reg, addr;
@@ -262,7 +263,6 @@ static int palmas_i2c_probe(struct i2c_client *i2c,
 	i2c_set_clientdata(i2c, palmas);
 	palmas->dev = &i2c->dev;
 	palmas->id = id->driver_data;
-	palmas->irq = i2c->irq;
 
 	for (i = 0; i < PALMAS_NUM_CLIENTS; i++) {
 		if (i == 0)
@@ -290,6 +290,15 @@ static int palmas_i2c_probe(struct i2c_client *i2c,
 		}
 	}
 
+	regnode = of_parse_phandle(node, "interrupts", 0);
+
+	if (!regnode) {
+		dev_warn(palmas->dev, "IRQ missing: skipping irq request\n");
+		goto no_irq;
+	} else {
+		palmas->irq = i2c->irq;
+	}
+
 	/* Change interrupt line output polarity */
 	if (pdata->irq_flags & IRQ_TYPE_LEVEL_HIGH)
 		reg = PALMAS_POLARITY_CTRL_INT_POLARITY;
@@ -316,6 +325,7 @@ static int palmas_i2c_probe(struct i2c_client *i2c,
 	if (ret < 0)
 		goto err;
 
+no_irq:
 	slave = PALMAS_BASE_TO_SLAVE(PALMAS_PU_PD_OD_BASE);
 	addr = PALMAS_BASE_TO_REG(PALMAS_PU_PD_OD_BASE,
 			PALMAS_PRIMARY_SECONDARY_PAD1);
-- 
1.7.5.4

Re: [PATCH v2 1/4] MFD: Palmas: Check if interrupts property exists and then only request irq

[Stephen Warren]

On 06/18/2013 04:01 AM, J Keerthy wrote:
> Check if interrupts property exists and then only request irq.
> On some boards INT line might not be connected to a valid
> irq line on the application processor. Hence keeping a check
> before requesting irq.

When there is no interrupts property, surely i2c->irq == 0, which is an
invalid IRQ, and hence there's no need to check this before copying the
value?

In other words, I think this whole patch could just be:

+ 	palmas->irq = i2c->irq;

right?

RE: [PATCH v2 1/4] MFD: Palmas: Check if interrupts property exists and then only request irq

[J, KEERTHY]

Hi Stephen,

> -----Original Message-----
> From: Stephen Warren [mailto:swarren@wwwdotorg.org]
> Sent: Tuesday, June 18, 2013 9:22 PM
> To: J, KEERTHY
> Cc: linux-omap@vger.kernel.org; broonie@kernel.org;
> ldewangan@nvidia.com; sameo@linux.intel.com; grant.likely@secretlab.ca;
> swarren@nvidia.com; linux-kernel@vger.kernel.org; linux-
> doc@vger.kernel.org; gg@slimlogic.co.uk
> Subject: Re: [PATCH v2 1/4] MFD: Palmas: Check if interrupts property
> exists and then only request irq
> 
> On 06/18/2013 04:01 AM, J Keerthy wrote:
> > Check if interrupts property exists and then only request irq.
> > On some boards INT line might not be connected to a valid irq line on
> > the application processor. Hence keeping a check before requesting
> > irq.
> 
> When there is no interrupts property, surely i2c->irq == 0, which is an
> invalid IRQ, and hence there's no need to check this before copying the
> value?

The intent here is NOT to request irq with 0 or Invalid IRQ. The board
File will not populate the interrupts entry if the INT line is not
Connected. Hence the patch checks for the 'interrupts' property.

This is essential since I do not want the probe to return error
Just because the i2c->irq == 0. The explicit check for the property
Ensures that the board does not have the INT line connected to
A valid interrupt line on the other side.

> 
> In other words, I think this whole patch could just be:
> 
> + 	palmas->irq = i2c->irq;
> 
> right?

Just this will cause a probe failure. That is not what is needed.
Instead the probe should continue skipping irq part.

Regards,
Keerthy

Re: [PATCH v2 1/4] MFD: Palmas: Check if interrupts property exists and then only request irq

[Stephen Warren]

On 06/18/2013 10:54 AM, J, KEERTHY wrote:
> Hi Stephen,
> 
>> -----Original Message-----
>> From: Stephen Warren [mailto:swarren@wwwdotorg.org]
>> Sent: Tuesday, June 18, 2013 9:22 PM
>> To: J, KEERTHY
>> Cc: linux-omap@vger.kernel.org; broonie@kernel.org;
>> ldewangan@nvidia.com; sameo@linux.intel.com; grant.likely@secretlab.ca;
>> swarren@nvidia.com; linux-kernel@vger.kernel.org; linux-
>> doc@vger.kernel.org; gg@slimlogic.co.uk
>> Subject: Re: [PATCH v2 1/4] MFD: Palmas: Check if interrupts property
>> exists and then only request irq
>>
>> On 06/18/2013 04:01 AM, J Keerthy wrote:
>>> Check if interrupts property exists and then only request irq.
>>> On some boards INT line might not be connected to a valid irq line on
>>> the application processor. Hence keeping a check before requesting
>>> irq.
>>
>> When there is no interrupts property, surely i2c->irq == 0, which is an
>> invalid IRQ, and hence there's no need to check this before copying the
>> value?
> 
> The intent here is NOT to request irq with 0 or Invalid IRQ.

Sure.

> The board File will not populate the interrupts entry if the INT line is not
> Connected.

Do you mean the interrupts DT property won't be present if there is no
interrupt. If so, sure.

> Hence the patch checks for the 'interrupts' property.

That shouldn't be necessary; IIRC, the I2C core has already parsed the
interrupts property if there was one, and if there wasn't, it has set
i2c->irq to some invalid value already.

So, you simply need to check the value in i2c->irq, and don't need to
look at the DT at all.

RE: [PATCH v2 1/4] MFD: Palmas: Check if interrupts property exists and then only request irq

[J, KEERTHY]

> -----Original Message-----
> From: Stephen Warren [mailto:swarren@wwwdotorg.org]
> Sent: Tuesday, June 18, 2013 10:38 PM
> To: J, KEERTHY
> Cc: linux-omap@vger.kernel.org; broonie@kernel.org;
> ldewangan@nvidia.com; sameo@linux.intel.com; grant.likely@secretlab.ca;
> swarren@nvidia.com; linux-kernel@vger.kernel.org; linux-
> doc@vger.kernel.org; gg@slimlogic.co.uk
> Subject: Re: [PATCH v2 1/4] MFD: Palmas: Check if interrupts property
> exists and then only request irq
> 
> On 06/18/2013 10:54 AM, J, KEERTHY wrote:
> > Hi Stephen,
> >
> >> -----Original Message-----
> >> From: Stephen Warren [mailto:swarren@wwwdotorg.org]
> >> Sent: Tuesday, June 18, 2013 9:22 PM
> >> To: J, KEERTHY
> >> Cc: linux-omap@vger.kernel.org; broonie@kernel.org;
> >> ldewangan@nvidia.com; sameo@linux.intel.com;
> >> grant.likely@secretlab.ca; swarren@nvidia.com;
> >> linux-kernel@vger.kernel.org; linux- doc@vger.kernel.org;
> >> gg@slimlogic.co.uk
> >> Subject: Re: [PATCH v2 1/4] MFD: Palmas: Check if interrupts
> property
> >> exists and then only request irq
> >>
> >> On 06/18/2013 04:01 AM, J Keerthy wrote:
> >>> Check if interrupts property exists and then only request irq.
> >>> On some boards INT line might not be connected to a valid irq line
> >>> on the application processor. Hence keeping a check before
> >>> requesting irq.
> >>
> >> When there is no interrupts property, surely i2c->irq == 0, which is
> >> an invalid IRQ, and hence there's no need to check this before
> >> copying the value?
> >
> > The intent here is NOT to request irq with 0 or Invalid IRQ.
> 
> Sure.
> 
> > The board File will not populate the interrupts entry if the INT line
> > is not Connected.
> 
> Do you mean the interrupts DT property won't be present if there is no
> interrupt. If so, sure.

Yes.

> 
> > Hence the patch checks for the 'interrupts' property.
> 
> That shouldn't be necessary; IIRC, the I2C core has already parsed the
> interrupts property if there was one, and if there wasn't, it has set
> i2c->irq to some invalid value already.
> 
> So, you simply need to check the value in i2c->irq, and don't need to
> look at the DT at all.

Instead of checking the Invalid irq value which most likely can be 0.
I am not sure.
I am explicitly checking if the interrupts property exists or not.

If not present then It throws out a warning. Either there is no
Valid INT line connection or the DeviceTree was not populated fully.

This additional piece of information is good to have in the driver
IMHO. Let me know if this is rational enough to have in the driver.

Regards,
Keerthy

Re: [PATCH v2 1/4] MFD: Palmas: Check if interrupts property exists and then only request irq

[Stephen Warren]

On 06/18/2013 11:19 AM, J, KEERTHY wrote:
> 
> 
>> -----Original Message-----
>> From: Stephen Warren [mailto:swarren@wwwdotorg.org]
>> Sent: Tuesday, June 18, 2013 10:38 PM
>> To: J, KEERTHY
>> Cc: linux-omap@vger.kernel.org; broonie@kernel.org;
>> ldewangan@nvidia.com; sameo@linux.intel.com; grant.likely@secretlab.ca;
>> swarren@nvidia.com; linux-kernel@vger.kernel.org; linux-
>> doc@vger.kernel.org; gg@slimlogic.co.uk
>> Subject: Re: [PATCH v2 1/4] MFD: Palmas: Check if interrupts property
>> exists and then only request irq
>>
>> On 06/18/2013 10:54 AM, J, KEERTHY wrote:
>>> Hi Stephen,
>>>
>>>> -----Original Message-----
>>>> From: Stephen Warren [mailto:swarren@wwwdotorg.org]
>>>> Sent: Tuesday, June 18, 2013 9:22 PM
>>>> To: J, KEERTHY
>>>> Cc: linux-omap@vger.kernel.org; broonie@kernel.org;
>>>> ldewangan@nvidia.com; sameo@linux.intel.com;
>>>> grant.likely@secretlab.ca; swarren@nvidia.com;
>>>> linux-kernel@vger.kernel.org; linux- doc@vger.kernel.org;
>>>> gg@slimlogic.co.uk
>>>> Subject: Re: [PATCH v2 1/4] MFD: Palmas: Check if interrupts
>> property
>>>> exists and then only request irq
>>>>
>>>> On 06/18/2013 04:01 AM, J Keerthy wrote:
>>>>> Check if interrupts property exists and then only request irq.
>>>>> On some boards INT line might not be connected to a valid irq line
>>>>> on the application processor. Hence keeping a check before
>>>>> requesting irq.
>>>>
>>>> When there is no interrupts property, surely i2c->irq == 0, which is
>>>> an invalid IRQ, and hence there's no need to check this before
>>>> copying the value?
>>>
>>> The intent here is NOT to request irq with 0 or Invalid IRQ.
>>
>> Sure.
>>
>>> The board File will not populate the interrupts entry if the INT line
>>> is not Connected.
>>
>> Do you mean the interrupts DT property won't be present if there is no
>> interrupt. If so, sure.
> 
> Yes.
> 
>>
>>> Hence the patch checks for the 'interrupts' property.
>>
>> That shouldn't be necessary; IIRC, the I2C core has already parsed the
>> interrupts property if there was one, and if there wasn't, it has set
>> i2c->irq to some invalid value already.
>>
>> So, you simply need to check the value in i2c->irq, and don't need to
>> look at the DT at all.
> 
> Instead of checking the Invalid irq value which most likely can be 0.
> I am not sure.
> I am explicitly checking if the interrupts property exists or not.
> 
> If not present then It throws out a warning. Either there is no
> Valid INT line connection or the DeviceTree was not populated fully.
> 
> This additional piece of information is good to have in the driver
> IMHO. Let me know if this is rational enough to have in the driver.

No, you should just check the IRQ number.

Consider this:

If the device was instantiated from a board file *or* a device tree,
i2c->irq is correctly set. Hence, checking that value works in both cases.

If you check the interrupts DT property, that will only work if the
device was instantiated from device tree, and not if it was instantiated
from a board file; the property will never exist in the board file case,
and hence you'll never be able to have a board file provide an interrupt.

RE: [PATCH v2 1/4] MFD: Palmas: Check if interrupts property exists and then only request irq

[J, KEERTHY]

> -----Original Message-----
> From: Stephen Warren [mailto:swarren@wwwdotorg.org]
> Sent: Tuesday, June 18, 2013 10:53 PM
> To: J, KEERTHY
> Cc: linux-omap@vger.kernel.org; broonie@kernel.org;
> ldewangan@nvidia.com; sameo@linux.intel.com; grant.likely@secretlab.ca;
> swarren@nvidia.com; linux-kernel@vger.kernel.org; linux-
> doc@vger.kernel.org; gg@slimlogic.co.uk
> Subject: Re: [PATCH v2 1/4] MFD: Palmas: Check if interrupts property
> exists and then only request irq
> 
> On 06/18/2013 11:19 AM, J, KEERTHY wrote:
> >
> >
> >> -----Original Message-----
> >> From: Stephen Warren [mailto:swarren@wwwdotorg.org]
> >> Sent: Tuesday, June 18, 2013 10:38 PM
> >> To: J, KEERTHY
> >> Cc: linux-omap@vger.kernel.org; broonie@kernel.org;
> >> ldewangan@nvidia.com; sameo@linux.intel.com;
> >> grant.likely@secretlab.ca; swarren@nvidia.com;
> >> linux-kernel@vger.kernel.org; linux- doc@vger.kernel.org;
> >> gg@slimlogic.co.uk
> >> Subject: Re: [PATCH v2 1/4] MFD: Palmas: Check if interrupts
> property
> >> exists and then only request irq
> >>
> >> On 06/18/2013 10:54 AM, J, KEERTHY wrote:
> >>> Hi Stephen,
> >>>
> >>>> -----Original Message-----
> >>>> From: Stephen Warren [mailto:swarren@wwwdotorg.org]
> >>>> Sent: Tuesday, June 18, 2013 9:22 PM
> >>>> To: J, KEERTHY
> >>>> Cc: linux-omap@vger.kernel.org; broonie@kernel.org;
> >>>> ldewangan@nvidia.com; sameo@linux.intel.com;
> >>>> grant.likely@secretlab.ca; swarren@nvidia.com;
> >>>> linux-kernel@vger.kernel.org; linux- doc@vger.kernel.org;
> >>>> gg@slimlogic.co.uk
> >>>> Subject: Re: [PATCH v2 1/4] MFD: Palmas: Check if interrupts
> >> property
> >>>> exists and then only request irq
> >>>>
> >>>> On 06/18/2013 04:01 AM, J Keerthy wrote:
> >>>>> Check if interrupts property exists and then only request irq.
> >>>>> On some boards INT line might not be connected to a valid irq
> line
> >>>>> on the application processor. Hence keeping a check before
> >>>>> requesting irq.
> >>>>
> >>>> When there is no interrupts property, surely i2c->irq == 0, which
> >>>> is an invalid IRQ, and hence there's no need to check this before
> >>>> copying the value?
> >>>
> >>> The intent here is NOT to request irq with 0 or Invalid IRQ.
> >>
> >> Sure.
> >>
> >>> The board File will not populate the interrupts entry if the INT
> >>> line is not Connected.
> >>
> >> Do you mean the interrupts DT property won't be present if there is
> >> no interrupt. If so, sure.
> >
> > Yes.
> >
> >>
> >>> Hence the patch checks for the 'interrupts' property.
> >>
> >> That shouldn't be necessary; IIRC, the I2C core has already parsed
> >> the interrupts property if there was one, and if there wasn't, it
> has
> >> set
> >> i2c->irq to some invalid value already.
> >>
> >> So, you simply need to check the value in i2c->irq, and don't need
> to
> >> look at the DT at all.
> >
> > Instead of checking the Invalid irq value which most likely can be 0.
> > I am not sure.
> > I am explicitly checking if the interrupts property exists or not.
> >
> > If not present then It throws out a warning. Either there is no Valid
> > INT line connection or the DeviceTree was not populated fully.
> >
> > This additional piece of information is good to have in the driver
> > IMHO. Let me know if this is rational enough to have in the driver.
> 
> No, you should just check the IRQ number.

Hmmm...so something like (!i2c->irq)

> 
> Consider this:
> 
> If the device was instantiated from a board file *or* a device tree,
> i2c->irq is correctly set. Hence, checking that value works in both
> cases.
> 
> If you check the interrupts DT property, that will only work if the
> device was instantiated from device tree, and not if it was
> instantiated from a board file; the property will never exist in the
> board file case, and hence you'll never be able to have a board file
> provide an interrupt.

The board file approach is getting deprecated for this. I
Myself removed board file related pdata stuff in one of the patches.

http://www.mail-archive.com/linux-omap@vger.kernel.org/msg90598.html

So going the DeviceTree way.

Regards,
Keerthy

Re: [PATCH v2 1/4] MFD: Palmas: Check if interrupts property exists and then only request irq

[Stephen Warren]

On 06/18/2013 11:33 AM, J, KEERTHY wrote:
> Stephen Warren wrote at Tuesday, June 18, 2013 10:53 PM:
...>> No, you should just check the IRQ number.
> 
> Hmmm...so something like (!i2c->irq)

Yes.

>> Consider this:
>>
>> If the device was instantiated from a board file *or* a device tree,
>> i2c->irq is correctly set. Hence, checking that value works in both
>> cases.
>>
>> If you check the interrupts DT property, that will only work if the
>> device was instantiated from device tree, and not if it was
>> instantiated from a board file; the property will never exist in the
>> board file case, and hence you'll never be able to have a board file
>> provide an interrupt.
> 
> The board file approach is getting deprecated for this. I
> Myself removed board file related pdata stuff in one of the patches.
> 
> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg90598.html
> 
> So going the DeviceTree way.

Even if you're 100% sure this driver will only ever work with DT (which
seems like a bad assumption to make no matter what the circumstance),
it'd still be best to detect whether an IRQ was specified in a generic
way. That way, nobody will read this driver, assume the code is generic,
and just copy/paste it without thinking.

RE: [PATCH v2 1/4] MFD: Palmas: Check if interrupts property exists and then only request irq

[J, KEERTHY]

Hi Stephen,

> -----Original Message-----
> From: Stephen Warren [mailto:swarren@wwwdotorg.org]
> Sent: Wednesday, June 19, 2013 12:42 AM
> To: J, KEERTHY
> Cc: linux-omap@vger.kernel.org; broonie@kernel.org;
> ldewangan@nvidia.com; sameo@linux.intel.com; grant.likely@secretlab.ca;
> swarren@nvidia.com; linux-kernel@vger.kernel.org; linux-
> doc@vger.kernel.org; gg@slimlogic.co.uk
> Subject: Re: [PATCH v2 1/4] MFD: Palmas: Check if interrupts property
> exists and then only request irq
> 
> On 06/18/2013 11:33 AM, J, KEERTHY wrote:
> > Stephen Warren wrote at Tuesday, June 18, 2013 10:53 PM:
> ...>> No, you should just check the IRQ number.
> >
> > Hmmm...so something like (!i2c->irq)
> 
> Yes.

Ok.

> 
> >> Consider this:
> >>
> >> If the device was instantiated from a board file *or* a device tree,
> >> i2c->irq is correctly set. Hence, checking that value works in both
> >> cases.
> >>
> >> If you check the interrupts DT property, that will only work if the
> >> device was instantiated from device tree, and not if it was
> >> instantiated from a board file; the property will never exist in the
> >> board file case, and hence you'll never be able to have a board file
> >> provide an interrupt.
> >
> > The board file approach is getting deprecated for this. I Myself
> > removed board file related pdata stuff in one of the patches.
> >
> > http://www.mail-archive.com/linux-omap@vger.kernel.org/msg90598.html
> >
> > So going the DeviceTree way.
> 
> Even if you're 100% sure this driver will only ever work with DT (which
> seems like a bad assumption to make no matter what the circumstance),
> it'd still be best to detect whether an IRQ was specified in a generic
> way. That way, nobody will read this driver, assume the code is
> generic, and just copy/paste it without thinking.

Ok. I understand. I will incorporate this.

Thanks,
Keerthy

Re: [PATCH v2 1/4] MFD: Palmas: Check if interrupts property exists and then only request irq

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 753 bytes --]

On Tue, Jun 18, 2013 at 11:22:41AM -0600, Stephen Warren wrote:

> If the device was instantiated from a board file *or* a device tree,
> i2c->irq is correctly set. Hence, checking that value works in both cases.

The same thing will happen with any other firmware interface that gets
introduced in the future - one of the goals with factoring all this out
into the bus code is that it means the driver doesn't need to have any
special handling.

> If you check the interrupts DT property, that will only work if the
> device was instantiated from device tree, and not if it was instantiated
> from a board file; the property will never exist in the board file case,
> and hence you'll never be able to have a board file provide an interrupt.

Exactly.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]