| Message ID | 1371470954-9124-2-git-send-email-j-keerthy@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jun 17, 2013 at 05:39:11PM +0530, J Keerthy wrote: > Palmas PMICs have an INT line. This line is one single > Interrupt line to the application processor. The interrupt > feature enables to selectively request irq for only those > specific chips which have INT line connected to a valid > IRQ line of the application processor. Does the support for the interrupt line need to be explicitly flagged like this or can the driver not simply support an interrupt line not being configured? That would also support cases where the hardware has an interrupt line but the system integrator has opeted not to connect it for some reason which seems generally more flexible than doing things on a chip ID basis. > +/** > + * DOC: Palmas PMIC feature types > + * Is "DOC: " normal kerneldoc?
Hi Mark, Thanks for the review. > -----Original Message----- > From: Mark Brown [mailto:broonie@kernel.org] > Sent: Monday, June 17, 2013 9:46 PM > To: J, KEERTHY > Cc: linux-omap@vger.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 1/4] MFD: Palmas: Add Interrupt feature > > On Mon, Jun 17, 2013 at 05:39:11PM +0530, J Keerthy wrote: > > > Palmas PMICs have an INT line. This line is one single Interrupt line > > to the application processor. The interrupt feature enables to > > selectively request irq for only those specific chips which have INT > > line connected to a valid IRQ line of the application processor. > > Does the support for the interrupt line need to be explicitly flagged > like this or can the driver not simply support an interrupt line not > being configured? That would also support cases where the hardware has > an interrupt line but the system integrator has opeted not to connect > it for some reason which seems generally more flexible than doing > things on a chip ID basis. > I understand your point. The IRQ is passed from device tree node. Say if the chip for some reason is not connected to any valid IRQ line the driver might end up requesting for a wrong IRQ line. So should I be validating the irq entry populated from device tree? Explicitly checking on chip ID helps to avoid wrongly populated Device tree data. > > +/** > > + * DOC: Palmas PMIC feature types > > + * > > Is "DOC: " normal kerneldoc? Normal kerneldoc I shall remove "DOC:" Regards, Keerthy -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jun 18, 2013 at 05:15:03AM +0000, J, KEERTHY wrote: > I understand your point. The IRQ is passed from device tree node. > Say if the chip for some reason is not connected to any valid > IRQ line the driver might end up requesting for a wrong IRQ line. > So should I be validating the irq entry populated from device tree? Yes, you should be checking that there's actually an interrupt there - the number will be zero if there isn't. > Explicitly checking on chip ID helps to avoid wrongly populated > Device tree data. Right, but on the other hand it ought to be possible to handle chips that could but aren't configured to do interrupts and if the driver can do that then this becomes redundant.
Hi Mark, > -----Original Message----- > From: Mark Brown [mailto:broonie@kernel.org] > Sent: Tuesday, June 18, 2013 2:28 PM > To: J, KEERTHY > Cc: linux-omap@vger.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 1/4] MFD: Palmas: Add Interrupt feature > > On Tue, Jun 18, 2013 at 05:15:03AM +0000, J, KEERTHY wrote: > > > I understand your point. The IRQ is passed from device tree node. > > Say if the chip for some reason is not connected to any valid IRQ > line > > the driver might end up requesting for a wrong IRQ line. > > > So should I be validating the irq entry populated from device tree? > > Yes, you should be checking that there's actually an interrupt there - > the number will be zero if there isn't. > > > Explicitly checking on chip ID helps to avoid wrongly populated > Device > > tree data. > > Right, but on the other hand it ought to be possible to handle chips > that could but aren't configured to do interrupts and if the driver can > do that then this becomes redundant. I understood. I am now implementing this in the driver making use Of the of_parse_phandle and check if the interrupts property is Populated and only then request for irq else skip that. Hope this Approach is fine. I will send v2 in a while. Regards, Keerthy -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c index 62fa728..d06ce62 100644 --- a/drivers/mfd/palmas.c +++ b/drivers/mfd/palmas.c @@ -231,6 +231,16 @@ static void palmas_dt_to_pdata(struct i2c_client *i2c, palmas_set_pdata_irq_flag(i2c, pdata); } +static unsigned int palmas_features = PALMAS_PMIC_FEATURE_INTERRUPT; + +static const struct of_device_id of_palmas_match_tbl[] = { + { + .compatible = "ti,palmas", + .data = &palmas_features, + }, + { }, +}; + static int palmas_i2c_probe(struct i2c_client *i2c, const struct i2c_device_id *id) { @@ -238,8 +248,9 @@ static int palmas_i2c_probe(struct i2c_client *i2c, struct palmas_platform_data *pdata; struct device_node *node = i2c->dev.of_node; int ret = 0, i; - unsigned int reg, addr; + unsigned int reg, addr, *features; int slave; + const struct of_device_id *match; pdata = dev_get_platdata(&i2c->dev); @@ -261,9 +272,17 @@ 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; + match = of_match_device(of_match_ptr(of_palmas_match_tbl), &i2c->dev); + + if (match) { + features = (unsigned int *)match->data; + palmas->features = *features; + } else { + return -ENODATA; + } + for (i = 0; i < PALMAS_NUM_CLIENTS; i++) { if (i == 0) palmas->i2c_clients[i] = i2c; @@ -290,6 +309,9 @@ static int palmas_i2c_probe(struct i2c_client *i2c, } } + if (!PALMAS_PMIC_HAS(palmas, INTERRUPT)) + goto no_int; + /* Change interrupt line output polarity */ if (pdata->irq_flags & IRQ_TYPE_LEVEL_HIGH) reg = PALMAS_POLARITY_CTRL_INT_POLARITY; @@ -316,6 +338,7 @@ static int palmas_i2c_probe(struct i2c_client *i2c, if (ret < 0) goto err; +no_int: slave = PALMAS_BASE_TO_SLAVE(PALMAS_PU_PD_OD_BASE); addr = PALMAS_BASE_TO_REG(PALMAS_PU_PD_OD_BASE, PALMAS_PRIMARY_SECONDARY_PAD1); @@ -427,11 +450,6 @@ static const struct i2c_device_id palmas_i2c_id[] = { }; MODULE_DEVICE_TABLE(i2c, palmas_i2c_id); -static struct of_device_id of_palmas_match_tbl[] = { - { .compatible = "ti,palmas", }, - { /* end */ } -}; - static struct i2c_driver palmas_i2c_driver = { .driver = { .name = "palmas", diff --git a/include/linux/mfd/palmas.h b/include/linux/mfd/palmas.h index 8f21daf..1b5b5f3 100644 --- a/include/linux/mfd/palmas.h +++ b/include/linux/mfd/palmas.h @@ -32,6 +32,19 @@ ((a) == PALMAS_CHIP_ID)) #define is_palmas_charger(a) ((a) == PALMAS_CHIP_CHARGER_ID) +/** + * DOC: Palmas PMIC feature types + * + * PALMAS_PMIC_FEATURE_INTERRUPT - used when the PMIC has Interrupt line going + * to an application processor. + * + * PALMAS_PMIC_HAS(b, f) - macro to check if a bandgap device is capable of a + * specific feature (above) or not. Return non-zero, if yes. + */ +#define PALMAS_PMIC_FEATURE_INTERRUPT BIT(0) +#define PALMAS_PMIC_HAS(b, f) \ + ((b)->features & PALMAS_PMIC_FEATURE_ ## f) + struct palmas_pmic; struct palmas_gpadc; struct palmas_resource; @@ -46,6 +59,7 @@ struct palmas { /* Stored chip id */ int id; + unsigned int features; /* IRQ Data */ int irq; u32 irq_mask;
Palmas PMICs have an INT line. This line is one single Interrupt line to the application processor. The interrupt feature enables to selectively request irq for only those specific chips which have INT line connected to a valid IRQ line of the application processor. Signed-off-by: J Keerthy <j-keerthy@ti.com> --- drivers/mfd/palmas.c | 32 +++++++++++++++++++++++++------- include/linux/mfd/palmas.h | 14 ++++++++++++++ 2 files changed, 39 insertions(+), 7 deletions(-)