* [PATCH] mfd: da9052-core: Fix request_threaded_irq() parameter
@ 2012-09-27 3:55 Fabio Estevam
2012-09-27 13:21 ` Marek Vasut
2012-09-27 14:08 ` Mark Brown
0 siblings, 2 replies; 5+ messages in thread
From: Fabio Estevam @ 2012-09-27 3:55 UTC (permalink / raw)
To: sameo; +Cc: broonie, marex, ashish.jangam, dchen, linux-kernel, Fabio Estevam
From: Fabio Estevam <fabio.estevam@freescale.com>
On a mx53qsb dt-kernel the da9052-core driver fails to probe:
da9052 1-0048: DA9052 ADC IRQ failed ret=-22
In request_threaded_irq() the first parameter is missing the da9052->irq_base.
Fix it and avoid the error.
Also define 'DA9052_IRQF' for improving readability.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/mfd/da9052-core.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/mfd/da9052-core.c b/drivers/mfd/da9052-core.c
index a0a62b2..a3450d2 100644
--- a/drivers/mfd/da9052-core.c
+++ b/drivers/mfd/da9052-core.c
@@ -33,6 +33,7 @@
#define DA9052_IRQ_MASK_POS_6 0x20
#define DA9052_IRQ_MASK_POS_7 0x40
#define DA9052_IRQ_MASK_POS_8 0x80
+#define DA9052_IRQF (IRQF_TRIGGER_LOW | IRQF_ONESHOT)
static bool da9052_reg_readable(struct device *dev, unsigned int reg)
{
@@ -788,16 +789,15 @@ int __devinit da9052_device_init(struct da9052 *da9052, u8 chip_id)
da9052->irq_base = pdata->irq_base;
ret = regmap_add_irq_chip(da9052->regmap, da9052->chip_irq,
- IRQF_TRIGGER_LOW | IRQF_ONESHOT,
- da9052->irq_base, &da9052_regmap_irq_chip,
- &da9052->irq_data);
+ DA9052_IRQF, da9052->irq_base,
+ &da9052_regmap_irq_chip, &da9052->irq_data);
if (ret < 0)
goto regmap_err;
da9052->irq_base = regmap_irq_chip_get_base(da9052->irq_data);
- ret = request_threaded_irq(DA9052_IRQ_ADC_EOM, NULL, da9052_auxadc_irq,
- IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+ ret = request_threaded_irq(da9052->irq_base + DA9052_IRQ_ADC_EOM, NULL,
+ da9052_auxadc_irq, DA9052_IRQF,
"adc irq", da9052);
if (ret != 0)
dev_err(da9052->dev, "DA9052 ADC IRQ failed ret=%d\n", ret);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] mfd: da9052-core: Fix request_threaded_irq() parameter
2012-09-27 3:55 [PATCH] mfd: da9052-core: Fix request_threaded_irq() parameter Fabio Estevam
@ 2012-09-27 13:21 ` Marek Vasut
2012-09-27 14:08 ` Mark Brown
1 sibling, 0 replies; 5+ messages in thread
From: Marek Vasut @ 2012-09-27 13:21 UTC (permalink / raw)
To: Fabio Estevam
Cc: sameo, broonie, ashish.jangam, dchen, linux-kernel, Fabio Estevam
Dear Fabio Estevam,
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> On a mx53qsb dt-kernel the da9052-core driver fails to probe:
>
> da9052 1-0048: DA9052 ADC IRQ failed ret=-22
>
> In request_threaded_irq() the first parameter is missing the
> da9052->irq_base.
>
> Fix it and avoid the error.
>
> Also define 'DA9052_IRQF' for improving readability.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
Reviewed-by: Marek Vasut <marex@denx.de>
> ---
> drivers/mfd/da9052-core.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mfd/da9052-core.c b/drivers/mfd/da9052-core.c
> index a0a62b2..a3450d2 100644
> --- a/drivers/mfd/da9052-core.c
> +++ b/drivers/mfd/da9052-core.c
> @@ -33,6 +33,7 @@
> #define DA9052_IRQ_MASK_POS_6 0x20
> #define DA9052_IRQ_MASK_POS_7 0x40
> #define DA9052_IRQ_MASK_POS_8 0x80
> +#define DA9052_IRQF (IRQF_TRIGGER_LOW | IRQF_ONESHOT)
>
> static bool da9052_reg_readable(struct device *dev, unsigned int reg)
> {
> @@ -788,16 +789,15 @@ int __devinit da9052_device_init(struct da9052
> *da9052, u8 chip_id) da9052->irq_base = pdata->irq_base;
>
> ret = regmap_add_irq_chip(da9052->regmap, da9052->chip_irq,
> - IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> - da9052->irq_base, &da9052_regmap_irq_chip,
> - &da9052->irq_data);
> + DA9052_IRQF, da9052->irq_base,
> + &da9052_regmap_irq_chip, &da9052->irq_data);
> if (ret < 0)
> goto regmap_err;
>
> da9052->irq_base = regmap_irq_chip_get_base(da9052->irq_data);
>
> - ret = request_threaded_irq(DA9052_IRQ_ADC_EOM, NULL, da9052_auxadc_irq,
> - IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> + ret = request_threaded_irq(da9052->irq_base + DA9052_IRQ_ADC_EOM, NULL,
> + da9052_auxadc_irq, DA9052_IRQF,
> "adc irq", da9052);
> if (ret != 0)
> dev_err(da9052->dev, "DA9052 ADC IRQ failed ret=%d\n", ret);
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] mfd: da9052-core: Fix request_threaded_irq() parameter
2012-09-27 3:55 [PATCH] mfd: da9052-core: Fix request_threaded_irq() parameter Fabio Estevam
2012-09-27 13:21 ` Marek Vasut
@ 2012-09-27 14:08 ` Mark Brown
2012-09-27 14:34 ` Arnd Bergmann
1 sibling, 1 reply; 5+ messages in thread
From: Mark Brown @ 2012-09-27 14:08 UTC (permalink / raw)
To: Fabio Estevam
Cc: sameo, marex, ashish.jangam, dchen, linux-kernel, Fabio Estevam
On Thu, Sep 27, 2012 at 12:55:35AM -0300, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> On a mx53qsb dt-kernel the da9052-core driver fails to probe:
>
> da9052 1-0048: DA9052 ADC IRQ failed ret=-22
>
> In request_threaded_irq() the first parameter is missing the da9052->irq_base.
>
> Fix it and avoid the error.
The driver shouldn't be relying on irq_base at all, it should use
regmap_get_virq() to look up the interrupt number. If it relies on
irq_base then the user is forced to assign one (or rely on automatic
assignment, which is a bit erratic. Otherwise it can use a linear
domain which doesn't rely on being able to allocate a big block of
interrupt numbers.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mfd: da9052-core: Fix request_threaded_irq() parameter
2012-09-27 14:08 ` Mark Brown
@ 2012-09-27 14:34 ` Arnd Bergmann
2012-09-27 16:13 ` Mark Brown
0 siblings, 1 reply; 5+ messages in thread
From: Arnd Bergmann @ 2012-09-27 14:34 UTC (permalink / raw)
To: Mark Brown
Cc: Fabio Estevam, sameo, marex, ashish.jangam, dchen, linux-kernel,
Fabio Estevam
On Thursday 27 September 2012, Mark Brown wrote:
> On Thu, Sep 27, 2012 at 12:55:35AM -0300, Fabio Estevam wrote:
> > From: Fabio Estevam <fabio.estevam@freescale.com>
> >
> > On a mx53qsb dt-kernel the da9052-core driver fails to probe:
> >
> > da9052 1-0048: DA9052 ADC IRQ failed ret=-22
> >
> > In request_threaded_irq() the first parameter is missing the da9052->irq_base.
> >
> > Fix it and avoid the error.
>
> The driver shouldn't be relying on irq_base at all, it should use
> regmap_get_virq() to look up the interrupt number. If it relies on
> irq_base then the user is forced to assign one (or rely on automatic
> assignment, which is a bit erratic. Otherwise it can use a linear
> domain which doesn't rely on being able to allocate a big block of
> interrupt numbers.
For all I can tell, the driver implements the automatic assignment
correctly, but I was also going to ask for removing the da9052->irq_base
variable and using the linear domain instead.
We don't have any platforms actually setting the irq_base in the
mainline kernel (I assume some of out tree platforms do this), but there
are a few drivers that need to be adapted to use regmap_irq_get_virq
or irq_to_desc(irq)->hw_irq, respectively:
drivers/gpio/gpio-da9052.c: return da9052->irq_base + DA9052_IRQ_GPI0 + offset;
drivers/input/touchscreen/da9052_tsi.c: tsi->irq_pendwn = da9052->irq_base + irq_pendwn;
drivers/input/touchscreen/da9052_tsi.c: tsi->irq_datardy = da9052->irq_base + irq_datardy;
drivers/power/da9052-battery.c: irq -= bat->da9052->irq_base;
drivers/power/da9052-battery.c: ret = request_threaded_irq(bat->da9052->irq_base + irq,
drivers/power/da9052-battery.c: free_irq(bat->da9052->irq_base + irq, bat);
drivers/power/da9052-battery.c: free_irq(bat->da9052->irq_base + irq, bat);
Arnd
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mfd: da9052-core: Fix request_threaded_irq() parameter
2012-09-27 14:34 ` Arnd Bergmann
@ 2012-09-27 16:13 ` Mark Brown
0 siblings, 0 replies; 5+ messages in thread
From: Mark Brown @ 2012-09-27 16:13 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Fabio Estevam, sameo, marex, ashish.jangam, dchen, linux-kernel,
Fabio Estevam
On Thu, Sep 27, 2012 at 02:34:45PM +0000, Arnd Bergmann wrote:
> On Thursday 27 September 2012, Mark Brown wrote:
> > The driver shouldn't be relying on irq_base at all, it should use
> > regmap_get_virq() to look up the interrupt number. If it relies on
> > irq_base then the user is forced to assign one (or rely on automatic
> > assignment, which is a bit erratic. Otherwise it can use a linear
> > domain which doesn't rely on being able to allocate a big block of
> > interrupt numbers.
> For all I can tell, the driver implements the automatic assignment
> correctly, but I was also going to ask for removing the da9052->irq_base
> variable and using the linear domain instead.
It's possible, IIRC last time I looked at it for API updates I decided
it was so clearly never going to work due to requesting without using
irq_base that I just ignored it for the purposes of compliation.
> We don't have any platforms actually setting the irq_base in the
> mainline kernel (I assume some of out tree platforms do this), but there
It's relatively rare to use the GPIOs as IRQs which is the only reason
you'd need to do this. That said I'm frankly unconvinced that the
driver has ever been tested given the general pain with the original
submission process.
> are a few drivers that need to be adapted to use regmap_irq_get_virq
> or irq_to_desc(irq)->hw_irq, respectively:
> drivers/gpio/gpio-da9052.c: return da9052->irq_base + DA9052_IRQ_GPI0 + offset;
Yeah, they all ought to be converted.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-09-27 16:14 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-27 3:55 [PATCH] mfd: da9052-core: Fix request_threaded_irq() parameter Fabio Estevam
2012-09-27 13:21 ` Marek Vasut
2012-09-27 14:08 ` Mark Brown
2012-09-27 14:34 ` Arnd Bergmann
2012-09-27 16:13 ` Mark Brown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox