From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752149AbcAFOtw (ORCPT ); Wed, 6 Jan 2016 09:49:52 -0500 Received: from outbound1a.ore.mailhop.org ([54.213.22.21]:15785 "EHLO outbound1a.ore.mailhop.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751643AbcAFOtv (ORCPT ); Wed, 6 Jan 2016 09:49:51 -0500 X-DKIM: OpenDKIM Filter v2.6.8 io B297480025 Date: Wed, 6 Jan 2016 14:49:47 +0000 From: Jason Cooper To: Boris Brezillon Cc: Milo Kim , 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 Message-ID: <20160106144947.GU18249@io.lakedaemon.net> References: <1451881723-2478-1-git-send-email-milo.kim@ti.com> <20160104100238.2b40f736@bbrezillon> <568CC6C7.3040809@ti.com> <20160106100755.2b6a4983@bbrezillon> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160106100755.2b6a4983@bbrezillon> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 wrote: > > On 01/04/2016 06:02 PM, Boris Brezillon wrote: > > > On Mon, 4 Jan 2016 13:28:24 +0900 Milo Kim 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.