* [PATCH] mfd: arizona: convert to regmap_add_irq_chips
@ 2012-07-27 19:02 Stephen Warren
2012-07-29 20:38 ` Mark Brown
0 siblings, 1 reply; 6+ messages in thread
From: Stephen Warren @ 2012-07-27 19:02 UTC (permalink / raw)
To: Samuel Ortiz; +Cc: Mark Brown, linux-kernel, Stephen Warren
From: Stephen Warren <swarren@nvidia.com>
The Arizona chip contains a single interrupt that represents the unified
output of multiple internal interrupt controllers. This pattern has been
factored out into regmap-irq, so convert the Arizona driver to use the
new regmap-irq code.
This introduces some hopefully minor functional changes:
1) regmap_add_irq_chips() calls regmap_add_irq_chip() with irq==0 rather
than -1, so in turn irq_domain_add_linear() is called rather than
irq_domain_add_legacy(). This change could be avoided by providing an
irq_bases array to regmap_add_irq_chips().
2) regmap_add_irq_chips() requests the top-level interrupt itself, so this
happens before the Arizona driver hooks the child BOOT_DONE and
CTRLIF_ERR interrupts. In the original, all the IRQ chips were created
first, and then the top-level IRQ was requested. This may cause a
functional difference if those interrupts are pending at probe() time.
3) The top-level interrupt domain set up by regmap_add_irq_chips() does
not have a .xlate op, whereas the removed domain in the Arizona driver
did. However, I don't think this is an issue, since the original code
simply used the default irq_domain_xlate_twocell, which didn't handle
mapping the IRQ numbers to the child interrupt controllers anyway, and
hence probably wasn't that useful. regmap-irq could easily be extended
to provide a useful xlate function if required.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
Note, this depends on the regmap series I just posted to compile.
---
drivers/mfd/arizona-irq.c | 132 ++++++--------------------------------
include/linux/mfd/arizona/core.h | 4 +-
2 files changed, 20 insertions(+), 116 deletions(-)
diff --git a/drivers/mfd/arizona-irq.c b/drivers/mfd/arizona-irq.c
index 98ac345..f693f85 100644
--- a/drivers/mfd/arizona-irq.c
+++ b/drivers/mfd/arizona-irq.c
@@ -28,11 +28,15 @@
static int arizona_map_irq(struct arizona *arizona, int irq)
{
+ struct regmap_irq_chip_data *irq_chip;
int ret;
- ret = regmap_irq_get_virq(arizona->aod_irq_chip, irq);
- if (ret < 0)
- ret = regmap_irq_get_virq(arizona->irq_chip, irq);
+ irq_chip = regmap_irq_chips_get_chip(arizona->irq_chips, 0);
+ ret = regmap_irq_get_virq(irq_chip, irq);
+ if (ret < 0) {
+ irq_chip = regmap_irq_chips_get_chip(arizona->irq_chips, 1);
+ ret = regmap_irq_get_virq(irq_chip, irq);
+ }
return ret;
}
@@ -91,83 +95,24 @@ static irqreturn_t arizona_ctrlif_err(int irq, void *data)
return IRQ_HANDLED;
}
-static irqreturn_t arizona_irq_thread(int irq, void *data)
-{
- struct arizona *arizona = data;
- int i, ret;
-
- ret = pm_runtime_get_sync(arizona->dev);
- if (ret < 0) {
- dev_err(arizona->dev, "Failed to resume device: %d\n", ret);
- return IRQ_NONE;
- }
-
- /* Check both domains */
- for (i = 0; i < 2; i++)
- handle_nested_irq(irq_find_mapping(arizona->virq, i));
-
- pm_runtime_mark_last_busy(arizona->dev);
- pm_runtime_put_autosuspend(arizona->dev);
-
- return IRQ_HANDLED;
-}
-
-static void arizona_irq_enable(struct irq_data *data)
-{
-}
-
-static void arizona_irq_disable(struct irq_data *data)
-{
-}
-
-static struct irq_chip arizona_irq_chip = {
- .name = "arizona",
- .irq_disable = arizona_irq_disable,
- .irq_enable = arizona_irq_enable,
-};
-
-static int arizona_irq_map(struct irq_domain *h, unsigned int virq,
- irq_hw_number_t hw)
-{
- struct regmap_irq_chip_data *data = h->host_data;
-
- irq_set_chip_data(virq, data);
- irq_set_chip_and_handler(virq, &arizona_irq_chip, handle_edge_irq);
- irq_set_nested_thread(virq, 1);
-
- /* ARM needs us to explicitly flag the IRQ as valid
- * and will set them noprobe when we do so. */
-#ifdef CONFIG_ARM
- set_irq_flags(virq, IRQF_VALID);
-#else
- irq_set_noprobe(virq);
-#endif
-
- return 0;
-}
-
-static struct irq_domain_ops arizona_domain_ops = {
- .map = arizona_irq_map,
- .xlate = irq_domain_xlate_twocell,
-};
-
int arizona_irq_init(struct arizona *arizona)
{
int flags = IRQF_ONESHOT;
int ret, i;
- const struct regmap_irq_chip *aod, *irq;
+ struct regmap *maps[2] = { arizona->regmap, arizona->regmap };
+ const struct regmap_irq_chip *chips[2];
switch (arizona->type) {
#ifdef CONFIG_MFD_WM5102
case WM5102:
- aod = &wm5102_aod;
- irq = &wm5102_irq;
+ chips[0] = &wm5102_aod;
+ chips[1] = &wm5102_irq;
break;
#endif
#ifdef CONFIG_MFD_WM5110
case WM5110:
- aod = &wm5110_aod;
- irq = &wm5110_irq;
+ chips[0] = &wm5110_aod;
+ chips[1] = &wm5110_irq;
break;
#endif
default:
@@ -189,30 +134,11 @@ int arizona_irq_init(struct arizona *arizona)
flags |= IRQF_TRIGGER_LOW;
}
- /* Allocate a virtual IRQ domain to distribute to the regmap domains */
- arizona->virq = irq_domain_add_linear(NULL, 2, &arizona_domain_ops,
- arizona);
- if (!arizona->virq) {
- ret = -EINVAL;
- goto err;
- }
-
- ret = regmap_add_irq_chip(arizona->regmap,
- irq_create_mapping(arizona->virq, 0),
- IRQF_ONESHOT, -1, aod,
- &arizona->aod_irq_chip);
+ ret = regmap_add_irq_chips(arizona->irq, flags, 2, maps, NULL, chips,
+ &arizona->irq_chips);
if (ret != 0) {
- dev_err(arizona->dev, "Failed to add AOD IRQs: %d\n", ret);
- goto err_domain;
- }
-
- ret = regmap_add_irq_chip(arizona->regmap,
- irq_create_mapping(arizona->virq, 1),
- IRQF_ONESHOT, -1, irq,
- &arizona->irq_chip);
- if (ret != 0) {
- dev_err(arizona->dev, "Failed to add AOD IRQs: %d\n", ret);
- goto err_aod;
+ dev_err(arizona->dev, "Failed to add IRQs: %d\n", ret);
+ goto err;
}
/* Make sure the boot done IRQ is unmasked for resumes */
@@ -235,28 +161,12 @@ int arizona_irq_init(struct arizona *arizona)
goto err_ctrlif;
}
- ret = request_threaded_irq(arizona->irq, NULL, arizona_irq_thread,
- flags, "arizona", arizona);
-
- if (ret != 0) {
- dev_err(arizona->dev, "Failed to request IRQ %d: %d\n",
- arizona->irq, ret);
- goto err_main_irq;
- }
-
return 0;
-err_main_irq:
- free_irq(arizona_map_irq(arizona, ARIZONA_IRQ_CTRLIF_ERR), arizona);
err_ctrlif:
free_irq(arizona_map_irq(arizona, ARIZONA_IRQ_BOOT_DONE), arizona);
err_boot_done:
- regmap_del_irq_chip(irq_create_mapping(arizona->virq, 1),
- arizona->irq_chip);
-err_aod:
- regmap_del_irq_chip(irq_create_mapping(arizona->virq, 0),
- arizona->aod_irq_chip);
-err_domain:
+ regmap_del_irq_chips(arizona->irq_chips);
err:
return ret;
}
@@ -265,11 +175,7 @@ int arizona_irq_exit(struct arizona *arizona)
{
free_irq(arizona_map_irq(arizona, ARIZONA_IRQ_CTRLIF_ERR), arizona);
free_irq(arizona_map_irq(arizona, ARIZONA_IRQ_BOOT_DONE), arizona);
- regmap_del_irq_chip(irq_create_mapping(arizona->virq, 1),
- arizona->irq_chip);
- regmap_del_irq_chip(irq_create_mapping(arizona->virq, 0),
- arizona->aod_irq_chip);
- free_irq(arizona->irq, arizona);
+ regmap_del_irq_chips(arizona->irq_chips);
return 0;
}
diff --git a/include/linux/mfd/arizona/core.h b/include/linux/mfd/arizona/core.h
index dd231ac..769b3a0 100644
--- a/include/linux/mfd/arizona/core.h
+++ b/include/linux/mfd/arizona/core.h
@@ -92,9 +92,7 @@ struct arizona {
struct arizona_pdata pdata;
int irq;
- struct irq_domain *virq;
- struct regmap_irq_chip_data *aod_irq_chip;
- struct regmap_irq_chip_data *irq_chip;
+ struct regmap_irq_chips_data *irq_chips;
struct mutex clk_lock;
int clk32k_ref;
--
1.7.0.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mfd: arizona: convert to regmap_add_irq_chips
2012-07-27 19:02 [PATCH] mfd: arizona: convert to regmap_add_irq_chips Stephen Warren
@ 2012-07-29 20:38 ` Mark Brown
2012-07-30 17:05 ` Stephen Warren
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2012-07-29 20:38 UTC (permalink / raw)
To: Stephen Warren; +Cc: Samuel Ortiz, linux-kernel, Stephen Warren
On Fri, Jul 27, 2012 at 01:02:56PM -0600, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
>
> The Arizona chip contains a single interrupt that represents the unified
> output of multiple internal interrupt controllers. This pattern has been
> factored out into regmap-irq, so convert the Arizona driver to use the
> new regmap-irq code.
So, I didn't like the patch this depends on but anyway..
> 1) regmap_add_irq_chips() calls regmap_add_irq_chip() with irq==0 rather
> than -1, so in turn irq_domain_add_linear() is called rather than
> irq_domain_add_legacy(). This change could be avoided by providing an
> irq_bases array to regmap_add_irq_chips().
This is a problem.
> 2) regmap_add_irq_chips() requests the top-level interrupt itself, so this
> happens before the Arizona driver hooks the child BOOT_DONE and
> CTRLIF_ERR interrupts. In the original, all the IRQ chips were created
> first, and then the top-level IRQ was requested. This may cause a
> functional difference if those interrupts are pending at probe() time.
Boot done is very likely to be asserted.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mfd: arizona: convert to regmap_add_irq_chips
2012-07-29 20:38 ` Mark Brown
@ 2012-07-30 17:05 ` Stephen Warren
2012-07-30 18:03 ` Mark Brown
0 siblings, 1 reply; 6+ messages in thread
From: Stephen Warren @ 2012-07-30 17:05 UTC (permalink / raw)
To: Mark Brown; +Cc: Samuel Ortiz, linux-kernel, Stephen Warren
On 07/29/2012 02:38 PM, Mark Brown wrote:
> On Fri, Jul 27, 2012 at 01:02:56PM -0600, Stephen Warren wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> The Arizona chip contains a single interrupt that represents the unified
>> output of multiple internal interrupt controllers. This pattern has been
>> factored out into regmap-irq, so convert the Arizona driver to use the
>> new regmap-irq code.
>
> So, I didn't like the patch this depends on but anyway..
>
>> 1) regmap_add_irq_chips() calls regmap_add_irq_chip() with irq==0 rather
>> than -1, so in turn irq_domain_add_linear() is called rather than
>> irq_domain_add_legacy(). This change could be avoided by providing an
>> irq_bases array to regmap_add_irq_chips().
>
> This is a problem.
OK, can you explain why? Is the problem the difference between the two
types of IRQ domain? I would have assumed this was an internal detail of
the driver hence not an issue. I assume there's no issue with
known/static IRQ numbers, since both 0 and -1 end up dynamically
allocating the IRQ bases IIRC.
>> 2) regmap_add_irq_chips() requests the top-level interrupt itself, so this
>> happens before the Arizona driver hooks the child BOOT_DONE and
>> CTRLIF_ERR interrupts. In the original, all the IRQ chips were created
>> first, and then the top-level IRQ was requested. This may cause a
>> functional difference if those interrupts are pending at probe() time.
>
> Boot done is very likely to be asserted.
Hmmm. Perhaps that means regmap_add_irq_chips() should be split into two
parts; one to create all the IRQ chips and hook everything together, and
the second to actually enable the interrupt.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mfd: arizona: convert to regmap_add_irq_chips
2012-07-30 17:05 ` Stephen Warren
@ 2012-07-30 18:03 ` Mark Brown
2012-07-31 17:20 ` Stephen Warren
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2012-07-30 18:03 UTC (permalink / raw)
To: Stephen Warren; +Cc: Samuel Ortiz, linux-kernel, Stephen Warren
On Mon, Jul 30, 2012 at 11:05:08AM -0600, Stephen Warren wrote:
> On 07/29/2012 02:38 PM, Mark Brown wrote:
> > On Fri, Jul 27, 2012 at 01:02:56PM -0600, Stephen Warren wrote:
> >> 1) regmap_add_irq_chips() calls regmap_add_irq_chip() with irq==0 rather
> >> than -1, so in turn irq_domain_add_linear() is called rather than
> >> irq_domain_add_legacy(). This change could be avoided by providing an
> >> irq_bases array to regmap_add_irq_chips().
> > This is a problem.
> OK, can you explain why? Is the problem the difference between the two
> types of IRQ domain? I would have assumed this was an internal detail of
> the driver hence not an issue. I assume there's no issue with
> known/static IRQ numbers, since both 0 and -1 end up dynamically
> allocating the IRQ bases IIRC.
We have GPIOs we might want to do interrupts on, if the API doesn't
support providing a base we've got an issue.
> >> 2) regmap_add_irq_chips() requests the top-level interrupt itself, so this
> >> happens before the Arizona driver hooks the child BOOT_DONE and
> >> CTRLIF_ERR interrupts. In the original, all the IRQ chips were created
> >> first, and then the top-level IRQ was requested. This may cause a
> >> functional difference if those interrupts are pending at probe() time.
> > Boot done is very likely to be asserted.
> Hmmm. Perhaps that means regmap_add_irq_chips() should be split into two
> parts; one to create all the IRQ chips and hook everything together, and
> the second to actually enable the interrupt.
That's what the existing arizona code does, it only requests the primary
IRQ line once it's completed setup.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mfd: arizona: convert to regmap_add_irq_chips
2012-07-30 18:03 ` Mark Brown
@ 2012-07-31 17:20 ` Stephen Warren
2012-07-31 17:22 ` Mark Brown
0 siblings, 1 reply; 6+ messages in thread
From: Stephen Warren @ 2012-07-31 17:20 UTC (permalink / raw)
To: Mark Brown; +Cc: Samuel Ortiz, linux-kernel, Stephen Warren
On 07/30/2012 12:03 PM, Mark Brown wrote:
> On Mon, Jul 30, 2012 at 11:05:08AM -0600, Stephen Warren wrote:
>> On 07/29/2012 02:38 PM, Mark Brown wrote:
>>> On Fri, Jul 27, 2012 at 01:02:56PM -0600, Stephen Warren wrote:
>
>>>> 1) regmap_add_irq_chips() calls regmap_add_irq_chip() with irq==0 rather
>>>> than -1, so in turn irq_domain_add_linear() is called rather than
>>>> irq_domain_add_legacy(). This change could be avoided by providing an
>>>> irq_bases array to regmap_add_irq_chips().
>
>>> This is a problem.
>
>> OK, can you explain why? Is the problem the difference between the two
>> types of IRQ domain? I would have assumed this was an internal detail of
>> the driver hence not an issue. I assume there's no issue with
>> known/static IRQ numbers, since both 0 and -1 end up dynamically
>> allocating the IRQ bases IIRC.
>
> We have GPIOs we might want to do interrupts on, if the API doesn't
> support providing a base we've got an issue.
I agree in general, but I don't see how this is a regression in this
change - the arizona pdata doesn't specify an IRQ base anywhere, and
hence the IRQ base is already dynamically allocated...
The (regmap-irq) API (in the patch I sent) does support optionally
specifying a base if you want, it's just that the arizona-irq.c patch I
sent didn't specify a base, since the original code didn't.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mfd: arizona: convert to regmap_add_irq_chips
2012-07-31 17:20 ` Stephen Warren
@ 2012-07-31 17:22 ` Mark Brown
0 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2012-07-31 17:22 UTC (permalink / raw)
To: Stephen Warren; +Cc: Samuel Ortiz, linux-kernel, Stephen Warren
On Tue, Jul 31, 2012 at 11:20:53AM -0600, Stephen Warren wrote:
> I agree in general, but I don't see how this is a regression in this
> change - the arizona pdata doesn't specify an IRQ base anywhere, and
> hence the IRQ base is already dynamically allocated...
> The (regmap-irq) API (in the patch I sent) does support optionally
> specifying a base if you want, it's just that the arizona-irq.c patch I
> sent didn't specify a base, since the original code didn't.
I see. It sounded like the regmap patch had the regression, not the
device specific patch.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-07-31 17:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-27 19:02 [PATCH] mfd: arizona: convert to regmap_add_irq_chips Stephen Warren
2012-07-29 20:38 ` Mark Brown
2012-07-30 17:05 ` Stephen Warren
2012-07-30 18:03 ` Mark Brown
2012-07-31 17:20 ` Stephen Warren
2012-07-31 17:22 ` Mark Brown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).