From: thomas.petazzoni@free-electrons.com (Thomas Petazzoni)
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 12:20:00 +0200 [thread overview]
Message-ID: <20130523122000.22a9a4d0@skate> (raw)
In-Reply-To: <20130523100951.GH9937@game.jcrosoft.org>
Dear Jean-Christophe PLAGNIOL-VILLARD,
On Thu, 23 May 2013 12:09:51 +0200, Jean-Christophe PLAGNIOL-VILLARD
wrote:
> > You can put it in <linux/irqchip/at91-aic.h>, there are already a few
> > headers there. But I think it's not the right solution.
> >
> > *However*, ideally, this header should disappear completely. All the
> > register defines should go directly into the driver C file. The only
> > usage of the AIC defines outside of the IRQ driver are for debug prints
> > in the pm.c code, which I think you could get rid of.
> >
> > Ditto for the at91_aic_{read,write} macros.
> >
> > The NR_AIC5_IRQS can also move to the driver itself.
> >
> > The only remaining one would be NR_AIC_IRQS, you can't get rid of it,
> > because it's used for the IRQ priority arrays. But I believe keeping
> > this one in <linux/irqchip/at91-aic.h> is reasonable.
>
> do want either as we expose the register acces for pm & co
Did you read what I write? In arch/arm/mach-at91/pm.c, the only
accesses made to the AIC registers are for pr_debug() calls, i.e only
to print some debugging messages. I believe those ones can simply be
removed to decouple the pm.c code from the irq.c code.
> until this is cleanup NACK
Is this really the way we want to welcome contributions in the kernel
community?
Boris, what you'd have to do is just do a few preparations patches
in your patch set. Something like:
* PATCH 1: remove usage of AIC registers for debug messages in pm.c
* PATCH 2: move all of at91_aic.h in irq.c, except NR_AIC_IRQS
* PATCH 3: move irq.c to drivers/irqchip/, and at91_aic.h to
include/linux/irqchip/, and adjust what's necessary
* and then your other patches.
Best regards,
Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
WARNING: multiple messages have this Message-ID (diff)
From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
To: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Cc: boris brezillon <b.brezillon@overkiz.com>,
Thomas Gleixner <tglx@linutronix.de>,
Nicolas Ferre <nicolas.ferre@atmel.com>,
Andrew Victor <linux@maxim.org.za>,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] ARM: at91: move at91 aic driver to drivers/irqchip
Date: Thu, 23 May 2013 12:20:00 +0200 [thread overview]
Message-ID: <20130523122000.22a9a4d0@skate> (raw)
In-Reply-To: <20130523100951.GH9937@game.jcrosoft.org>
Dear Jean-Christophe PLAGNIOL-VILLARD,
On Thu, 23 May 2013 12:09:51 +0200, Jean-Christophe PLAGNIOL-VILLARD
wrote:
> > You can put it in <linux/irqchip/at91-aic.h>, there are already a few
> > headers there. But I think it's not the right solution.
> >
> > *However*, ideally, this header should disappear completely. All the
> > register defines should go directly into the driver C file. The only
> > usage of the AIC defines outside of the IRQ driver are for debug prints
> > in the pm.c code, which I think you could get rid of.
> >
> > Ditto for the at91_aic_{read,write} macros.
> >
> > The NR_AIC5_IRQS can also move to the driver itself.
> >
> > The only remaining one would be NR_AIC_IRQS, you can't get rid of it,
> > because it's used for the IRQ priority arrays. But I believe keeping
> > this one in <linux/irqchip/at91-aic.h> is reasonable.
>
> do want either as we expose the register acces for pm & co
Did you read what I write? In arch/arm/mach-at91/pm.c, the only
accesses made to the AIC registers are for pr_debug() calls, i.e only
to print some debugging messages. I believe those ones can simply be
removed to decouple the pm.c code from the irq.c code.
> until this is cleanup NACK
Is this really the way we want to welcome contributions in the kernel
community?
Boris, what you'd have to do is just do a few preparations patches
in your patch set. Something like:
* PATCH 1: remove usage of AIC registers for debug messages in pm.c
* PATCH 2: move all of at91_aic.h in irq.c, except NR_AIC_IRQS
* PATCH 3: move irq.c to drivers/irqchip/, and at91_aic.h to
include/linux/irqchip/, and adjust what's necessary
* and then your other patches.
Best regards,
Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
next prev parent reply other threads:[~2013-05-23 10:20 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 [this message]
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
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=20130523122000.22a9a4d0@skate \
--to=thomas.petazzoni@free-electrons.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.