From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751144AbeAVLqq (ORCPT ); Mon, 22 Jan 2018 06:46:46 -0500 Received: from mail-wm0-f68.google.com ([74.125.82.68]:42934 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751053AbeAVLqn (ORCPT ); Mon, 22 Jan 2018 06:46:43 -0500 X-Google-Smtp-Source: AH8x226Z+cm5D9a6V0DCfW7EcEqbDuOCcoFSWW5/pKupxL5CTuileKLMyFlfLKOJRdbOfOIFz5ObQQ== Date: Mon, 22 Jan 2018 11:46:39 +0000 From: Lee Jones To: Marc Zyngier Cc: Paul Cercueil , Michael Turquette , Stephen Boyd , Rob Herring , Mark Rutland , Thomas Gleixner , Jason Cooper , Daniel Lezcano , linux-clk@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 6/9] irqchip: Add the ingenic-tcu-intc driver Message-ID: <20180122114639.yjfdbvizs6fcpfdq@dell> References: <20180101143344.2099-1-paul@crapouillou.net> <20180110224838.16711-1-paul@crapouillou.net> <20180110224838.16711-6-paul@crapouillou.net> <58349291-93cd-f803-52a9-f88120ace4dc@arm.com> <1515687945.2170.1@smtp.crapouillou.net> <20180120130649.7f326f75@why.wild-wind.fr.eu.org> <20180122092617.36ggqqike4gwwgrd@dell> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 22 Jan 2018, Marc Zyngier wrote: > On 22/01/18 09:26, Lee Jones wrote: > > On Sat, 20 Jan 2018, Marc Zyngier wrote: > > > >> On Thu, 11 Jan 2018 17:25:45 +0100 > >> Paul Cercueil wrote: > >> > >>> Hi Marc, > >>> > >>>>> +static int __init ingenic_tcu_intc_of_init(struct device_node > >>>>> *node, > >>>>> + struct device_node *parent) > >>>>> +{ > >>>>> + struct irq_domain *domain; > >>>>> + struct irq_chip_generic *gc; > >>>>> + struct irq_chip_type *ct; > >>>>> + int err, i, num_parent_irqs; > >>>>> + unsigned int parent_irqs[3]; > >>>> > >>>> 3 parent interrupts? Really? How do you pick one? Also, given the > >>>> useage > >>>> model below, "int" is the wrong type. Probably should be u32. > >>> > >>> See below. > >>> > >>>>> + struct regmap *map; > >>>>> + > >>>>> + num_parent_irqs = of_property_count_elems_of_size( > >>>>> + node, "interrupts", 4); > >>>> > >>>> Nit: on a single line, as here is nothing that hurts my eyes more than > >>>> reading something like( > >>>> this). Also, 4 is better expressed as sizeof(u32). > >>> > >>> That will make checkpatch.pl unhappy :( > >> > >> And I don't care about checkpatch. I maintain the irqchip stuff, while > >> checkpatch doesn't. Hence, I win. > > > > num_parent_irqs = > > of_property_count_elems_of_size(node, "interrupts", 4); > > > > Everybody wins! > > > > As I said before, I've stopped using a physical DEC VT100 around 1990, > and gained the ability to extend my terminal to a bit more that 80 > columns. And even the VT100 could be coerced into using a 132 column mode... > > Right, Greg has spoken about this before. > Adhering to a convention can be good, but common sense must apply first. > Splitting an assignment is visually annoying and in that case, it > doesn't make much sense. I'll happily take a line that goes beyond 80 > cols I'm not adverse to the idea, but we should agree to do this centrally, rather than a few of us going rouge. This way we can go ahead and change all of the documentation and tooling (inc. Checkpatch) too, which will save on countless inevitable conversations/patches attempting to 'fix' non-conforming lines/files. > and if you really wanted to stay within boundaries, how about > turning "num_parent_irqs" something shorter? -- Lee Jones Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog