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 [PATCH 00/19] irqchip: atmel-aic: make unified AIC driver 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 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.