From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755400AbcGEPxS (ORCPT ); Tue, 5 Jul 2016 11:53:18 -0400 Received: from outbound1a.ore.mailhop.org ([54.213.22.21]:33853 "EHLO outbound1a.ore.mailhop.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754573AbcGEPxK (ORCPT ); Tue, 5 Jul 2016 11:53:10 -0400 X-MHO-User: a8e3bab2-42c8-11e6-a0ff-e511cd071b9b X-Report-Abuse-To: https://support.duocircle.com/support/solutions/articles/5000540958-duocircle-standard-smtp-abuse-information X-Originating-IP: 74.99.78.160 X-Mail-Handler: DuoCircle Outbound SMTP X-DKIM: OpenDKIM Filter v2.6.8 io C1DBF8005C Date: Tue, 5 Jul 2016 15:53:06 +0000 From: Jason Cooper To: Sebastian Frias Cc: Mason , LKML , Thomas Gleixner , Marc Zyngier Subject: Re: [RFC PATCH v1] irqchip: add support for SMP irq router Message-ID: <20160705155306.GG3348@io.lakedaemon.net> References: <577542D1.4070307@laposte.net> <577A5260.3070001@free.fr> <577BA854.6090503@laposte.net> <20160705144151.GE3348@io.lakedaemon.net> <577BCFD2.8060203@laposte.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <577BCFD2.8060203@laposte.net> 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 Sebastian, On Tue, Jul 05, 2016 at 05:18:42PM +0200, Sebastian Frias wrote: > On 07/05/2016 04:41 PM, Jason Cooper wrote: > > On Tue, Jul 05, 2016 at 02:30:12PM +0200, Sebastian Frias wrote: > >> On 07/04/2016 02:11 PM, Mason wrote: > > ... > >>>> .../sigma,smp87xx-irqrouter.txt | 69 +++ > >>> > >>> In the *actual* submission, we can't use a wildcard like smp87xx > >>> we'll have to use an actual part number. > >> > >> Are you sure? > >> That would hinder genericity. > >> Actually I wanted to call it "sigma,smp-irqrouter.txt" (or "sigma,smp,irqrouter.txt"). > > > > sigma,smp-irqrouter.txt should be fine. The devicetree maintainers > > should yelp if they want something different. > > > >> To me there's no need to link the compatible string of a given HW > >> module with that of the chip name the module it is embedded into. For > >> example, the generic USB3 driver is "generic-xhci". While this module > >> is not generic to be embedded in chips from different manufacturers, > >> it is supposed to be generic within Sigma, and multiple Sigma chips > >> (with potentially different denominations) can use it. > >> > >>> > >>>> drivers/irqchip/Makefile | 1 + > >>>> drivers/irqchip/irq-tango_v2.c | 594 +++++++++++++++++++++ > >>> > >>> Likewise, I don't like the "_v2" suffix, it's too generic. > >>> Actual submission should use something more specific. > >> > >> Well, the other driver is irq-tango.c that is generic as well. > >> I prefer versioning, as it is unrelated with the actual chip name. > > > > Is there a name, similar to 'tango', for this version of the IP? > > Something that would spark recognition for someone looking for "the damn > > driver for this XYZ irqchip I have". If not, irq-tango_v2.c is fine. > > > > Thanks for your comments. > So, aside from some naming issues, do you think the driver is ok? Well, it's going to be few days before I can really dig in to this. Until then, what I can say I see is that it looks like you're using devicetree to tell Linux how to lay out the irq domains. That's not right :( The devicetree should *only* describe the hardware. Would *BSD be able to use the description in the dtb effectively? iiuc, I think irq-crossbar.c may be a similar enough in task to give you an idea or two. Beyond that, as you mention, there are a bunch of style issues, unneeded macros, etc. thx, Jason.