From: b.brezillon@overkiz.com (boris brezillon)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/3] ARM: at91: move at91 aic driver to drivers/irqchip
Date: Thu, 23 May 2013 14:44:03 +0200 [thread overview]
Message-ID: <519E0F13.4080208@overkiz.com> (raw)
In-Reply-To: <20130523141344.7a01ce88@skate>
On 23/05/2013 14:13, Thomas Petazzoni wrote:
> Dear Russell King - ARM Linux,
>
> On Thu, 23 May 2013 11:18:25 +0100, Russell King - ARM Linux wrote:
>
>> I notice arch/arm/mach-at91/pm.c makes use of some of the register
>> definitions:
>>
>> at91_irq_suspend();
>>
>> pr_debug("AT91: PM - wake mask %08x, pm state %d\n",
>> /* remember all the always-wake irqs */
>> (at91_pmc_read(AT91_PMC_PCSR)
>> | (1 << AT91_ID_FIQ)
>> | (1 << AT91_ID_SYS)
>> | (at91_extern_irq))
>> & at91_aic_read(AT91_AIC_IMR),
>> state);
>>
>> at91_irq_suspend() is in arch/arm/mach-at91/irq.c already, so there's no
>> reason that fragment can't be moved there.
> The problem is that the goal of the patch set is to move
> arch/arm/mach-at91/irq.c into drivers/irqchip/.
>
> However, if you move that chunk of code to drivers/irqchip/irq-at91.c,
> you are using at91_pmc_read() which is defined in
> arch/arm/mach-at91/include/mach/at91_pmc.h. So,
> drivers/irqchip/irq-at91.c would have to include such an header file,
> which is something we want to avoid since drivers/ code should not
> include something in <mach/...>, as it breaks multiplatform kernels.
>
> So, I'm afraid, simply moving this chunk of code in at91_irq_suspend()
> doesn't make the thing any better.
What about keeping the former at91_aic.h in arch/arm/match-at91 and copy
the definitions we need in drivers/irqchip/irq-at91.c ?
This way we get an aic irqchip driver independant of any specific
machine headers and keep the non dt boards and pm drivers without any
change.
>
> Best regards,
>
> Thomas
WARNING: multiple messages have this Message-ID (diff)
From: boris brezillon <b.brezillon@overkiz.com>
To: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Russell King - ARM Linux <linux@arm.linux.org.uk>,
Nicolas Ferre <nicolas.ferre@atmel.com>,
linux-kernel@vger.kernel.org,
Thomas Gleixner <tglx@linutronix.de>,
Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>,
Andrew Victor <linux@maxim.org.za>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/3] ARM: at91: move at91 aic driver to drivers/irqchip
Date: Thu, 23 May 2013 14:44:03 +0200 [thread overview]
Message-ID: <519E0F13.4080208@overkiz.com> (raw)
In-Reply-To: <20130523141344.7a01ce88@skate>
On 23/05/2013 14:13, Thomas Petazzoni wrote:
> Dear Russell King - ARM Linux,
>
> On Thu, 23 May 2013 11:18:25 +0100, Russell King - ARM Linux wrote:
>
>> I notice arch/arm/mach-at91/pm.c makes use of some of the register
>> definitions:
>>
>> at91_irq_suspend();
>>
>> pr_debug("AT91: PM - wake mask %08x, pm state %d\n",
>> /* remember all the always-wake irqs */
>> (at91_pmc_read(AT91_PMC_PCSR)
>> | (1 << AT91_ID_FIQ)
>> | (1 << AT91_ID_SYS)
>> | (at91_extern_irq))
>> & at91_aic_read(AT91_AIC_IMR),
>> state);
>>
>> at91_irq_suspend() is in arch/arm/mach-at91/irq.c already, so there's no
>> reason that fragment can't be moved there.
> The problem is that the goal of the patch set is to move
> arch/arm/mach-at91/irq.c into drivers/irqchip/.
>
> However, if you move that chunk of code to drivers/irqchip/irq-at91.c,
> you are using at91_pmc_read() which is defined in
> arch/arm/mach-at91/include/mach/at91_pmc.h. So,
> drivers/irqchip/irq-at91.c would have to include such an header file,
> which is something we want to avoid since drivers/ code should not
> include something in <mach/...>, as it breaks multiplatform kernels.
>
> So, I'm afraid, simply moving this chunk of code in at91_irq_suspend()
> doesn't make the thing any better.
What about keeping the former at91_aic.h in arch/arm/match-at91 and copy
the definitions we need in drivers/irqchip/irq-at91.c ?
This way we get an aic irqchip driver independant of any specific
machine headers and keep the non dt boards and pm drivers without any
change.
>
> Best regards,
>
> Thomas
next prev parent reply other threads:[~2013-05-23 12:44 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-05-23 9:05 [RESEND PATCH 0/3] ARM: at91: move aic driver to drivers/irqchip Boris BREZILLON
2013-05-23 9:05 ` Boris BREZILLON
2013-05-23 9:05 ` [PATCH 1/3] ARM: at91: move at91 " Boris BREZILLON
2013-05-23 9:05 ` Boris BREZILLON
2013-05-23 9:06 ` Jean-Christophe PLAGNIOL-VILLARD
2013-05-23 9:06 ` Jean-Christophe PLAGNIOL-VILLARD
2013-05-23 9:51 ` boris brezillon
2013-05-23 9:51 ` boris brezillon
2013-05-23 10:09 ` Thomas Petazzoni
2013-05-23 10:09 ` Thomas Petazzoni
2013-05-23 10:09 ` Jean-Christophe PLAGNIOL-VILLARD
2013-05-23 10:09 ` Jean-Christophe PLAGNIOL-VILLARD
2013-05-23 10:20 ` Thomas Petazzoni
2013-05-23 10:20 ` Thomas Petazzoni
2013-05-23 10:26 ` Jean-Christophe PLAGNIOL-VILLARD
2013-05-23 10:26 ` Jean-Christophe PLAGNIOL-VILLARD
2013-05-23 11:22 ` Thomas Petazzoni
2013-05-23 11:22 ` Thomas Petazzoni
2013-05-23 11:58 ` Jean-Christophe PLAGNIOL-VILLARD
2013-05-23 11:58 ` Jean-Christophe PLAGNIOL-VILLARD
2013-05-23 12:09 ` Thomas Petazzoni
2013-05-23 12:09 ` Thomas Petazzoni
2013-05-23 13:00 ` Nicolas Ferre
2013-05-23 13:00 ` Nicolas Ferre
2013-05-23 10:18 ` Russell King - ARM Linux
2013-05-23 10:18 ` Russell King - ARM Linux
2013-05-23 11:59 ` Jean-Christophe PLAGNIOL-VILLARD
2013-05-23 11:59 ` Jean-Christophe PLAGNIOL-VILLARD
2013-05-23 12:13 ` Thomas Petazzoni
2013-05-23 12:13 ` Thomas Petazzoni
2013-05-23 12:42 ` Boris BREZILLON
2013-05-23 12:44 ` boris brezillon [this message]
2013-05-23 12:44 ` boris brezillon
2013-05-23 9:05 ` [PATCH 2/3] ARM: at91: use IRQCHIP_DECLARE instead of machine specific init_irq Boris BREZILLON
2013-05-23 9:05 ` Boris BREZILLON
2013-05-23 9:05 ` [PATCH 3/3] ARM: at91: use set_handle_irq instead of machine specific handle_irq Boris BREZILLON
2013-05-23 9:05 ` Boris BREZILLON
-- strict thread matches above, loose matches on Subject: below --
2013-05-23 8:49 [PATCH 0/3] ARM: at91: move aic driver to drivers/irqchip Boris BREZILLON
2013-05-23 8:49 ` [PATCH 1/3] ARM: at91: move at91 " Boris BREZILLON
2013-05-23 8:49 ` Boris BREZILLON
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=519E0F13.4080208@overkiz.com \
--to=b.brezillon@overkiz.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.