From: Jason Cooper <jason@lakedaemon.net>
To: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: Milo Kim <milo.kim@ti.com>,
tglx@linutronix.de, marc.zyngier@arm.com,
alexandre.belloni@free-electrons.com,
ludovic.desroches@atmel.com, nicolas.ferre@atmel.com,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 00/19] irqchip: atmel-aic: make unified AIC driver
Date: Wed, 6 Jan 2016 14:49:47 +0000 [thread overview]
Message-ID: <20160106144947.GU18249@io.lakedaemon.net> (raw)
In-Reply-To: <20160106100755.2b6a4983@bbrezillon>
Hey Milo,
On Wed, Jan 06, 2016 at 10:07:55AM +0100, Boris Brezillon wrote:
> On Wed, 6 Jan 2016 16:48:23 +0900 Milo Kim <milo.kim@ti.com> wrote:
> > On 01/04/2016 06:02 PM, Boris Brezillon wrote:
> > > On Mon, 4 Jan 2016 13:28:24 +0900 Milo Kim <milo.kim@ti.com> wrote:
> > >
> > >> This patch-set provides unified Atmel AIC (Advanced Interrupt Controller)
> > >> driver. Currently, there are two AIC drivers, AIC and AIC5.
> > >> Each driver consists of chip specific part (irq-atmel-aic.o or
> > >> irq-atmel-aic5.o) and shared code (irq-atmel-aic-common.o).
> > >> But consolidated AIC driver is just one file driver which supports both
> > >> IRQ chip systems.
> > >
> > > Sorry, but what's the real motivation behind this rework?
> >
> > During my driver development on Atmel boards, I just found major
> > difference between two IRQ chips is how to select HW IRQ number. Other
> > parts could be merged into single driver like OMAP.
>
> Except that this major difference is a central aspect, and if you look
> at your changes, you'll see that you're introducing
> 'if (aic_is_ssr_used())' statements in pretty much all irqchip
> callbacks.
>
> As I said, I'm not against code factorization, but it's not really
> one to me, because you're adding extra conditional path all over the
> code to differentiate the two chips, which means those are not so
> similar
...
> > > Before reviewing the remaining patches, I'd like to know more about your
> > > real motivations for pushing those changes?
> >
> > Yeap, thanks for your time. My idea is simple.
> >
> > "Different IRQ chip operation can be consolidated if simple data
> > structure is used."
>
> As pointed, I don't think that's a good idea, but let's see what others
> say.
> Thomas, Jason, any comments?
I'm with Nicolas on this one. I appreciate the effort, but it's best to
discuss the proposal with at91/irqchip maintainers prior to investing so
much effort.
sorry,
Jason.
next prev parent reply other threads:[~2016-01-06 14:49 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-04 4:28 Milo Kim
2016-01-04 4:28 ` [PATCH 01/19] irqchip: atmel-aic: fix wrong bit operation for IRQ priority Milo Kim
2016-01-04 8:11 ` Boris Brezillon
2016-01-04 4:28 ` [PATCH 02/19] irqchip: atmel-aic: clean up RTC interrupt code Milo Kim
2016-01-04 8:16 ` Boris Brezillon
2016-01-04 4:28 ` [PATCH 03/19] irqchip: atmel-aic: clean up RTT " Milo Kim
2016-01-04 8:17 ` Boris Brezillon
2016-01-04 4:28 ` [PATCH 04/19] irqchip: atmel-aic: replace magic numbers with named constant Milo Kim
2016-01-04 8:29 ` Boris Brezillon
2016-01-04 4:28 ` [PATCH 05/19] irqchip: atmel-aic: use simple constant to get number of interrupts per chip Milo Kim
2016-01-04 8:33 ` Boris Brezillon
2016-01-04 4:28 ` [PATCH 06/19] irqchip: atmel-aic: introduce register data structure Milo Kim
2016-01-04 8:53 ` Boris Brezillon
2016-01-06 8:30 ` Milo Kim
2016-01-04 4:28 ` [PATCH 07/19] irqchip: atmel-aic: make common IRQ domain translate function Milo Kim
2016-01-04 4:28 ` [PATCH 08/19] irqchip: atmel-aic: add common mask and unmask functions Milo Kim
2016-01-04 8:48 ` Boris Brezillon
2016-01-04 4:28 ` [PATCH 09/19] irqchip: atmel-aic: add common retrigger function Milo Kim
2016-01-04 4:28 ` [PATCH 10/19] irqchip: atmel-aic: add common set_type function Milo Kim
2016-01-04 4:28 ` [PATCH 11/19] irqchip: atmel-aic: add common PM IRQ chip operation Milo Kim
2016-01-04 4:28 ` [PATCH 12/19] irqchip: atmel-aic: use EOI register data in aic_reg_data Milo Kim
2016-01-04 4:28 ` [PATCH 13/19] irqchip: atmel-aic: clean up irq_chip_generic Milo Kim
2016-01-04 4:28 ` [PATCH 14/19] irqchip: atmel-aic: add common HW init function Milo Kim
2016-01-04 4:28 ` [PATCH 15/19] irqchip: atmel-aic: add common interrupt handler Milo Kim
2016-01-04 4:28 ` [PATCH 16/19] irqchip: atmel-aic: get total number of IRQs from device node Milo Kim
2016-01-04 4:28 ` [PATCH 17/19] irqchip: atmel-aic: use unified IRQ chip initialization function Milo Kim
2016-01-04 4:28 ` [PATCH 18/19] irqchip: atmel-aic: use unified AIC driver Milo Kim
2016-01-04 4:28 ` [PATCH 19/19] irqchip: atmel-aic: rename AIC driver and fix Kconfig Milo Kim
2016-01-04 9:02 ` [PATCH 00/19] irqchip: atmel-aic: make unified AIC driver Boris Brezillon
2016-01-04 9:37 ` Nicolas Ferre
2016-01-06 7:55 ` Milo Kim
2016-01-06 7:48 ` Milo Kim
2016-01-06 9:07 ` Boris Brezillon
2016-01-06 14:49 ` Jason Cooper [this message]
2016-01-07 7:48 ` Milo Kim
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=20160106144947.GU18249@io.lakedaemon.net \
--to=jason@lakedaemon.net \
--cc=alexandre.belloni@free-electrons.com \
--cc=boris.brezillon@free-electrons.com \
--cc=linux-kernel@vger.kernel.org \
--cc=ludovic.desroches@atmel.com \
--cc=marc.zyngier@arm.com \
--cc=milo.kim@ti.com \
--cc=nicolas.ferre@atmel.com \
--cc=tglx@linutronix.de \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome