From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751175AbcGNLKo (ORCPT ); Thu, 14 Jul 2016 07:10:44 -0400 Received: from mout.kundenserver.de ([212.227.17.10]:52817 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750916AbcGNLKl convert rfc822-to-8bit (ORCPT ); Thu, 14 Jul 2016 07:10:41 -0400 From: Arnd Bergmann To: Wan Zongshun Cc: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, jason@lakedaemon.net, Wan Zongshun , Daniel Lezcano , linux-kernel@vger.kernel.org, Russell King , p.zabel@pengutronix.de, Thomas Gleixner , linux-clk@vger.kernel.org Subject: Re: [PATCH v2 02/10] irqchip: add irqchip driver for nuc900 Date: Thu, 14 Jul 2016 13:09:54 +0200 Message-ID: <4372420.eqVKsk1ckW@wuerfel> User-Agent: KMail/5.1.3 (Linux/4.4.0-28-generic; KDE/5.18.0; x86_64; ; ) In-Reply-To: <578752CD.3010408@iommu.org> References: <1468135649-19980-1-git-send-email-vw@iommu.org> <5434334.4rKzx80y6W@wuerfel> <578752CD.3010408@iommu.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8BIT Content-Type: text/plain; charset="UTF-8" X-Provags-ID: V03:K0:bUNUxje50uPZnnAFyCDEqR4zg6TFzOBHDicSUPs/+o7fMRhqauA QcC8J+p/4jRGWSA91DPYDcjMw0FJ+TUZtfq4a1PUZVmXkbwsN0NHv8gA8/OW4xwh1GAzaV8 52LEwuN5aHD11M2sUfJDkpApR+EERxwz96xm1qgsmnPjt3WfUI4zPXF4b2OAI9iaCHvLe7B vJlNJImk/aphx/rLzTZnw== X-UI-Out-Filterresults: notjunk:1;V01:K0:dfmXbnyT1+M=:9MBxmtiPeJg2sm2H9Ixyw/ 4MP+VyGYUPNGagTq7woWUzzEFVYBRpx4JL8RsjYoyGu+FTLamn/VP3Dqq1zecr9lKXz2GU7UI lACrIQmdI0IYg/MpI9cShXhhXjutZKdo+P6jjL9P84knG2lTrlmp/A5LH5leZEsP64QtgD8gI RIVqSd3BZ8Woq2UpCIt0UUTB5Q0nmwKMH6rivYo/1PJRIqHlU/wHE8Sc2Ifh+iBPa1YbnGp+k bQ9uWLdjnRcZsaJuVC+NrRxpIQ/kYoqoISfnjsncx8LaDaGT37p4UVxVMUDKvkwxTCmz49WW3 9KZPiOgtjt/rNpU06i8McBU5DPkAsJ3STBPjR6q4WkiW6yOo5d7pB0zNRDEKAzes+/LIJ9W60 +lLGVCQYSb0xyZ3n29rsYdfP+khXhPCUxvDurCeCDxM2hzb+3Kn1kCbBDT/n57JgWgHHXb8vU ICxBlFlbdAp+MnZlTzJ1pXygocWkOBHTIfrTy36ftxx6c6A+uF5TUR91zdskM58oUmIujNGL9 fg9hUDZPRUURT8rAeKXsfSSs/fY5+C/0uA1fdpxZhRiytfib/PDpMCkqQ/bQ59J4gBLm6UhLM q20/korIpboWbI8AKIYi+/PfHNLs6swykS5wJgya7crVz6LgmOqIMdx9ouy7t+weLmIjUVmRY P638/lhRlKUxNLhS/1bazXEoeKAP4AOvqpVot9KKD+JlTdkeoPNrtf/sYAOD/mJKNZVUQS+KI c/KPZ5W/XKtrTYfC Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thursday, July 14, 2016 4:52:29 PM CEST Wan Zongshun wrote: > > On 2016年07月12日 16:26, Arnd Bergmann wrote: > > On Tuesday, July 12, 2016 3:04:42 PM CEST Wan Zongshun wrote: > >>> > >>> Ideally, this should just go away once we use SPARSE_IRQ. > >> > >> This platform also can use SPARSE_IRQ? this just a simple irq map and no > >> more irq number in this Soc. > >> > > > > SPARSE_IRQ is implied by ARCH_MULTIPLATFORM, so we will have to > > use it once that gets enabled. > > > > Your new irqchip driver already handles IRQ domains, so it will > > work out of the box with SPARSE_IRQ, but you have to change the > > reference to "NR_IRQS" into something else. > > > > I've prototyped a patch series to enable ARCH_MULTIPLATFORM, > > I hope you can start working from what I have and get it to run. > > I go through the ARCH_MULTIPLATFORM and SPARSE_IRQ related codes, but I > find I also have to define the NUC900_NR_IRQS firstly like below, so > that I can init the .nr_irq. > > +#if !defined(CONFIG_SOC_NUC970) > #define NUC900_NR_IRQS (IRQ_ADC+1) > +#else > +#define NUC900_NR_IRQS 62 > +#endif > > DT_MACHINE_START(nuc900_dt, "Nuvoton NUC900 (Device Tree Support)") > .dt_compat = nuc900_dt_compat, > + .nr_irqs = NUC900_NR_IRQS, > MACHINE_END You don't need to set this for the DT based machines, this number is just for the set of IRQ that have a hardcoded mapping. With DT, they get dynamically allocated as required. For the board files, you can hardcode the original definition of 32 IRQs, but I think you don't need that if you register a legacy IRQ domain in mach-w90x900/irq.c. > and then in my irqchip driver, I will use the NUC900_NR_IRQS: > > +aic_domain = irq_domain_add_linear(node, NUC900_NR_IRQS, > + &aic_irq_domain_ops, NULL); > > > Is that a right usage? This does not look right when NUC900_NR_IRQS can have configuration dependent values. I can see two ways of handling it: a) register the maximum number of IRQs that the irqchip can handle. There is no real cost for having a large number here, as SPARSE_IRQ ensures we only need to allocate the descriptors that are actually used. b) make the number of interrupts dependent on the compatible string for the irqchip, and handle NUC970 differently from the others in the driver. In the meantime, I also have a series to enable multiplatform support for all of mach-w90x900 based on your patches, but lacking a proper clk driver. I'll send that to you so you can include it in your series (after verifying that it works, or fixing it where necessary). Arnd From mboxrd@z Thu Jan 1 00:00:00 1970 From: arnd@arndb.de (Arnd Bergmann) Date: Thu, 14 Jul 2016 13:09:54 +0200 Subject: [PATCH v2 02/10] irqchip: add irqchip driver for nuc900 In-Reply-To: <578752CD.3010408@iommu.org> References: <1468135649-19980-1-git-send-email-vw@iommu.org> <5434334.4rKzx80y6W@wuerfel> <578752CD.3010408@iommu.org> Message-ID: <4372420.eqVKsk1ckW@wuerfel> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Thursday, July 14, 2016 4:52:29 PM CEST Wan Zongshun wrote: > > On 2016?07?12? 16:26, Arnd Bergmann wrote: > > On Tuesday, July 12, 2016 3:04:42 PM CEST Wan Zongshun wrote: > >>> > >>> Ideally, this should just go away once we use SPARSE_IRQ. > >> > >> This platform also can use SPARSE_IRQ? this just a simple irq map and no > >> more irq number in this Soc. > >> > > > > SPARSE_IRQ is implied by ARCH_MULTIPLATFORM, so we will have to > > use it once that gets enabled. > > > > Your new irqchip driver already handles IRQ domains, so it will > > work out of the box with SPARSE_IRQ, but you have to change the > > reference to "NR_IRQS" into something else. > > > > I've prototyped a patch series to enable ARCH_MULTIPLATFORM, > > I hope you can start working from what I have and get it to run. > > I go through the ARCH_MULTIPLATFORM and SPARSE_IRQ related codes, but I > find I also have to define the NUC900_NR_IRQS firstly like below, so > that I can init the .nr_irq. > > +#if !defined(CONFIG_SOC_NUC970) > #define NUC900_NR_IRQS (IRQ_ADC+1) > +#else > +#define NUC900_NR_IRQS 62 > +#endif > > DT_MACHINE_START(nuc900_dt, "Nuvoton NUC900 (Device Tree Support)") > .dt_compat = nuc900_dt_compat, > + .nr_irqs = NUC900_NR_IRQS, > MACHINE_END You don't need to set this for the DT based machines, this number is just for the set of IRQ that have a hardcoded mapping. With DT, they get dynamically allocated as required. For the board files, you can hardcode the original definition of 32 IRQs, but I think you don't need that if you register a legacy IRQ domain in mach-w90x900/irq.c. > and then in my irqchip driver, I will use the NUC900_NR_IRQS: > > +aic_domain = irq_domain_add_linear(node, NUC900_NR_IRQS, > + &aic_irq_domain_ops, NULL); > > > Is that a right usage? This does not look right when NUC900_NR_IRQS can have configuration dependent values. I can see two ways of handling it: a) register the maximum number of IRQs that the irqchip can handle. There is no real cost for having a large number here, as SPARSE_IRQ ensures we only need to allocate the descriptors that are actually used. b) make the number of interrupts dependent on the compatible string for the irqchip, and handle NUC970 differently from the others in the driver. In the meantime, I also have a series to enable multiplatform support for all of mach-w90x900 based on your patches, but lacking a proper clk driver. I'll send that to you so you can include it in your series (after verifying that it works, or fixing it where necessary). Arnd