Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1302440
| From | Milo Kim <milo.kim@ti.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH 00/19] irqchip: atmel-aic: make unified AIC driver |
| Date | 2016-01-06 08:50 +0100 |
| Message-ID | <qNR10-6zh-3@gated-at.bofh.it> (permalink) |
| References | <qN4Wl-6EU-3@gated-at.bofh.it> <qN9jk-1sI-7@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
Hi Boris, Thanks a lot for your comments. On 01/04/2016 06:02 PM, Boris Brezillon wrote: > Hi Milo, > > 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. > >> >> How to handle two IRQ chips in one driver >> ----------------------------------------- >> Structure aic_reg_offset is used for device configuration. >> AIC5 IRQ chip uses SSR (Source Select Register) to select IRQ number. >> On the other hand, AIC IRQ chip has simple register access. >> To support both IRQ chips, aic_is_ssr_used() helper is used. >> >> Patches >> ------- >> 1 ~ 5: fix IRQ priority issue, clean up RTC/RTT fixup code and etc. > > As explained in my review, those irq fixup are essential, and cannot > remove them. My apologies for this regression. I should check your log carefully. Thanks for pointing this out. I'm just thinking about boot code modification for this. How about supporting RTC/RTT fixup code in at91bootstrap? > >> 6 ~ 19: create unified IRQ chip operation with aic_reg_offset data. > > I started to review those patches, but honestly I don't see the point of > this rework, since you're trying to merge drivers for 2 IPs that are > completely different from a functional POV (except for a few tiny things > like priority or irq type definition). > > 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." If AIC and AIC5 must be separate, then I'll try to rework irq-atmel-common part. > >> >> Target boards >> ------------- >> Tested with two boards. >> * Arietta G25 (SoC: AT91SAM9G25) >> * Xplained board (SoC: SAMA5D3) >> >> Number of driver files >> ---------------------- >> AIC: 3 (irq-atmel-aic.c, irq-atmel-aic-common.c and h) >> AIC5: 3 (irq-atmel-aic5.c, irq-atmel-aic-common.c and h) >> Consolidated AIC: 1 (irq-aic.c) >> >> Code size >> --------- >> AIC (irq-atmel-aic.o and irq-atmel-aic-common.o) >> text data bss dec hex filename >> 5137 196 4 5337 14d9 drivers/irqchip/built-in.o >> >> AIC5 (irq-atmel-aic5.o and irq-atmel-aic-common.o) >> text data bss dec hex filename >> 5548 196 4 5748 1674 drivers/irqchip/built-in.o >> >> Consolidated AIC (irq-aic.o) >> text data bss dec hex filename >> 4841 196 8 5045 13b5 drivers/irqchip/built-in.o >> >> Lines of code >> ------------- >> AIC: 597 >> AIC5: 688 >> Consolidated AIC: 609 > > > Please, redo the same thing, but after keeping the IRQ fixup stuff, and > I'm pretty sure the text section of the AIC/AIC5 and the consolidated > version will be much closer. Let me check it later if I could have a chance to create the 2nd patch ;) Best regards, Milo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH 00/19] irqchip: atmel-aic: make unified AIC driver Milo Kim <milo.kim@ti.com> - 2016-01-04 05:30 +0100
[PATCH 07/19] irqchip: atmel-aic: make common IRQ domain translate function Milo Kim <milo.kim@ti.com> - 2016-01-04 05:30 +0100
[PATCH 09/19] irqchip: atmel-aic: add common retrigger function Milo Kim <milo.kim@ti.com> - 2016-01-04 05:30 +0100
[PATCH 13/19] irqchip: atmel-aic: clean up irq_chip_generic Milo Kim <milo.kim@ti.com> - 2016-01-04 05:30 +0100
[PATCH 01/19] irqchip: atmel-aic: fix wrong bit operation for IRQ priority Milo Kim <milo.kim@ti.com> - 2016-01-04 05:30 +0100
Re: [PATCH 01/19] irqchip: atmel-aic: fix wrong bit operation for IRQ priority Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-01-04 09:20 +0100
[PATCH 17/19] irqchip: atmel-aic: use unified IRQ chip initialization function Milo Kim <milo.kim@ti.com> - 2016-01-04 05:30 +0100
[PATCH 06/19] irqchip: atmel-aic: introduce register data structure Milo Kim <milo.kim@ti.com> - 2016-01-04 05:30 +0100
Re: [PATCH 06/19] irqchip: atmel-aic: introduce register data structure Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-01-04 10:00 +0100
Re: [PATCH 06/19] irqchip: atmel-aic: introduce register data structure Milo Kim <milo.kim@ti.com> - 2016-01-06 09:40 +0100
[PATCH 04/19] irqchip: atmel-aic: replace magic numbers with named constant Milo Kim <milo.kim@ti.com> - 2016-01-04 05:30 +0100
Re: [PATCH 04/19] irqchip: atmel-aic: replace magic numbers with named constant Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-01-04 09:30 +0100
[PATCH 19/19] irqchip: atmel-aic: rename AIC driver and fix Kconfig Milo Kim <milo.kim@ti.com> - 2016-01-04 05:30 +0100
[PATCH 11/19] irqchip: atmel-aic: add common PM IRQ chip operation Milo Kim <milo.kim@ti.com> - 2016-01-04 05:30 +0100
[PATCH 12/19] irqchip: atmel-aic: use EOI register data in aic_reg_data Milo Kim <milo.kim@ti.com> - 2016-01-04 05:40 +0100
[PATCH 14/19] irqchip: atmel-aic: add common HW init function Milo Kim <milo.kim@ti.com> - 2016-01-04 05:40 +0100
[PATCH 10/19] irqchip: atmel-aic: add common set_type function Milo Kim <milo.kim@ti.com> - 2016-01-04 05:40 +0100
[PATCH 08/19] irqchip: atmel-aic: add common mask and unmask functions Milo Kim <milo.kim@ti.com> - 2016-01-04 05:40 +0100
Re: [PATCH 08/19] irqchip: atmel-aic: add common mask and unmask functions Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-01-04 09:50 +0100
[PATCH 02/19] irqchip: atmel-aic: clean up RTC interrupt code Milo Kim <milo.kim@ti.com> - 2016-01-04 05:40 +0100
Re: [PATCH 02/19] irqchip: atmel-aic: clean up RTC interrupt code Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-01-04 09:20 +0100
[PATCH 05/19] irqchip: atmel-aic: use simple constant to get number of interrupts per chip Milo Kim <milo.kim@ti.com> - 2016-01-04 05:40 +0100
Re: [PATCH 05/19] irqchip: atmel-aic: use simple constant to get number of interrupts per chip Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-01-04 09:40 +0100
[PATCH 03/19] irqchip: atmel-aic: clean up RTT interrupt code Milo Kim <milo.kim@ti.com> - 2016-01-04 05:40 +0100
Re: [PATCH 03/19] irqchip: atmel-aic: clean up RTT interrupt code Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-01-04 09:20 +0100
[PATCH 15/19] irqchip: atmel-aic: add common interrupt handler Milo Kim <milo.kim@ti.com> - 2016-01-04 05:40 +0100
Re: [PATCH 00/19] irqchip: atmel-aic: make unified AIC driver Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-01-04 10:10 +0100
Re: [PATCH 00/19] irqchip: atmel-aic: make unified AIC driver Nicolas Ferre <nicolas.ferre@atmel.com> - 2016-01-04 10:40 +0100
Re: [PATCH 00/19] irqchip: atmel-aic: make unified AIC driver Milo Kim <milo.kim@ti.com> - 2016-01-06 09:00 +0100
Re: [PATCH 00/19] irqchip: atmel-aic: make unified AIC driver Milo Kim <milo.kim@ti.com> - 2016-01-06 08:50 +0100
Re: [PATCH 00/19] irqchip: atmel-aic: make unified AIC driver Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-01-06 10:10 +0100
Re: [PATCH 00/19] irqchip: atmel-aic: make unified AIC driver Jason Cooper <jason@lakedaemon.net> - 2016-01-06 15:50 +0100
Re: [PATCH 00/19] irqchip: atmel-aic: make unified AIC driver Milo Kim <milo.kim@ti.com> - 2016-01-07 08:50 +0100
csiph-web