Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752658Ab3DVKCu (ORCPT ); Mon, 22 Apr 2013 06:02:50 -0400 Received: from metis.ext.pengutronix.de ([92.198.50.35]:54613 "EHLO metis.ext.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751203Ab3DVKCt (ORCPT ); Mon, 22 Apr 2013 06:02:49 -0400 Date: Mon, 22 Apr 2013 12:02:19 +0200 From: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= To: Thomas Gleixner Cc: kernel@pengutronix.de, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Jonathan Austin , Catalin Marinas , Arnd Bergmann , Benjamin Herrenschmidt , Grant Likely , Paul Mundt , linux-sh@vger.kernel.org Subject: Re: [PATCH v3] irqchip: Add support for ARMv7-M's NVIC Message-ID: <20130422100219.GH15233@pengutronix.de> References: <1366214540-31166-1-git-send-email-u.kleine-koenig@pengutronix.de> <20130419150927.GE15233@pengutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20130419150927.GE15233@pengutronix.de> User-Agent: Mutt/1.5.21 (2010-09-15) X-SA-Exim-Connect-IP: 2001:6f8:1178:2:21e:67ff:fe11:9c5c X-SA-Exim-Mail-From: ukl@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-kernel@vger.kernel.org Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3804 Lines: 110 Hello, (for the new readers of this thread: This is about using u32 mask = 1 << (d->hwirq % 32); instead of u32 mask = 1 << (d->irq - gc->irq_base); in the callbacks for the irq generic chip.) On Fri, Apr 19, 2013 at 05:09:27PM +0200, Uwe Kleine-K?nig wrote: > On Thu, Apr 18, 2013 at 11:35:22AM +0200, Thomas Gleixner wrote: > > On Wed, 17 Apr 2013, Uwe Kleine-K?nig wrote: > > How is that different from what the generic irq chip implementation > > does? The only difference is that mask is generated by d->hwirq and > > not by d->irq. And due to the fact, that you use a full linear mapping > > between hwirq and virq the generic code simply works. > I'm not sure what you mean when you say "full linear mapping". AFAICT > using irq_domain_add_linear doesn't imply that two consecutive hardware > irq numbers get consecutive Linux irq numbers, so using d->irq won't work. > > > Even if it would not work, it would be trivial to extend the generic > > chip with that functionality instead of hacking another slightly > > different copy of the same thing. > I will try that and report back. I wonder if using hwirq % 32 should work everywhere where now d->irq - gc->irqbase is used. Depending on d->hwirq and not d->irq has the upside of working with non-legacy irq domains, too. Looking at next-20130419 the affected functions (irq_gc_mask_disable_reg, irq_gc_mask_set_bit, irq_gc_mask_clr_bit, irq_gc_unmask_enable_reg, irq_gc_ack_set_bit, irq_gc_ack_clr_bit, irq_gc_mask_disable_reg_and_ack, irq_gc_eoi, irq_gc_set_wake) are used in: arch/arm/mach-davinci/irq.c arch/arm/mach-imx/avic.c arch/arm/mach-imx/tzic.c arch/arm/mach-omap2/irq.c arch/arm/mach-omap2/prm_common.c arch/arm/mach-s5p64x0/common.c -> uses irq_base=101 for irq_alloc_generic_chip arch/arm/plat-orion/gpio.c -> depends on how orion_gpio_of_init is called. No callers found. arch/arm/plat-orion/irq.c arch/arm/plat-samsung/irq-vic-timer.c -> used for a single irq that isn't a multiple of 32 arch/arm/plat-samsung/s5p-irq-gpioint.c -> would need % 8? arch/mips/jz4740/gpio.c -> JZ4740_IRQ_GPIO(0) != JZ4740_IRQ_GPIO0 ? -> uses 56 + i * 32 as irqbase arch/mips/jz4740/irq.c -> uses 8 as irqbase arch/sh/boards/mach-se/7343/irq.c -> uses irq_base = irq_linear_revmap(se7343_irq_domain, 0) where se7343_irq_domain is a linear domain. AFAICT this is a bug. (After adding the domain they map all irqs in increasing order which currently seems to guarantee that it works. But IMHO it should use a legacy domain.) arch/sh/boards/mach-se/7722/irq.c as above. drivers/gpio/gpio-mxc.c drivers/gpio/gpio-mxs.c drivers/gpio/gpio-omap.c drivers/gpio/gpio-sodaville.c drivers/irqchip/irq-sirfsoc.c drivers/mfd/jz4740-adc.c -> uses platform_get_irq(pdev, 1) as irq_base for 5 irqs. For the uncommented files using %32 instead of -gc->irq_base should work. So it seems I cannot just substitute how the mask is called. The options I see are: - introduce a new set of functions Do you have a nice naming scheme? irq_gc_unmask_enable_reg_hwirqmod32? Or should I rename the existing ones to irq_gc_unmask_enable_reg_irqbaseoffset? - use u32 mask = 1 << (d->hwirq - gc->irq_base) % 32; This is ugly but might work assuming irq_base == 0 for chips with irq domain support and hwirq == irq for the others. I'm not lucky with the options, so I'm looking forward to suggestions. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ | -- 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/