From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jamie Iles Subject: Re: [PATCH 2/2] gpio/tegra: Dynamically allocate IRQ base, and support DT Date: Thu, 1 Dec 2011 16:55:18 +0000 Message-ID: <20111201165518.GH4556@totoro> References: <1322700336-26866-1-git-send-email-swarren@nvidia.com> <1322700336-26866-2-git-send-email-swarren@nvidia.com> <4ED78461.40006@gmail.com> <20111201141112.GG4556@totoro> <74CDBE0F657A3D45AFBB94109FB122FF174FDB01DD@HQMAIL01.nvidia.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <74CDBE0F657A3D45AFBB94109FB122FF174FDB01DD-C7FfzLzN0UxDw2glCA4ptUEOCMrvLtNR@public.gmane.org> Sender: linux-tegra-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Stephen Warren Cc: Jamie Iles , Rob Herring , Peter De Schrijver , "linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Colin Cross , "devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org" , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" List-Id: devicetree@vger.kernel.org On Thu, Dec 01, 2011 at 08:52:49AM -0800, Stephen Warren wrote: > Jamie Iles wrote at Thursday, December 01, 2011 7:11 AM: > > On Thu, Dec 01, 2011 at 07:42:57AM -0600, Rob Herring wrote: > > > On 11/30/2011 06:45 PM, Stephen Warren wrote: > > > > Enhance the driver to dynamically allocate the base IRQ number, and > > > > create an IRQ domain for itself. The use of an IRQ domain ensures that > > > > any device tree node interrupts properties are correctly parsed. > > > > > > > > Fix the DT binding documentation to describe interrupt-related properties, > > > > and the contents of "child" node interrupts property. > > > > > > > > Update tegra20.dtsi to specify the required interrupt-related properties. > > > > > > > > Finally, remove the definition of TEGRA_GPIO_TO_IRQ; this macro no longer > > > > gives correct results since the IRQ numbers for GPIOs are dynamically > > > > allocated. > > > > > > > > Signed-off-by: Stephen Warren > > [...] > > > > static int tegra_gpio_irq_set_type(struct irq_data *d, unsigned int type) > > > > { > > > > - int gpio = d->irq - INT_GPIO_BASE; > > > > + int gpio = d->hwirq; > > > > struct tegra_gpio_bank *bank = irq_data_get_irq_chip_data(d); > > > > int port = GPIO_PORT(gpio); > > > > int lvl_type; > > > > @@ -343,6 +345,22 @@ static int __devinit tegra_gpio_probe(struct platform_device *pdev) > > > > int i; > > > > int j; > > > > > > > > + irq_domain.irq_base = irq_alloc_descs(-1, 0, TEGRA_NR_GPIOS, 0); > > > > + if (irq_domain.irq_base < 0) { > > > > + dev_err(&pdev->dev, "Couldn't allocate IRQ numbers\n"); > > > > + return -ENODEV; > > > > + } > > > > + irq_domain.nr_irq = TEGRA_NR_GPIOS; > > > > + irq_domain.ops = &irq_domain_ops; > > > > > > Why don't you just use irq_domain_simple_ops? > > > > This would need the patch I posted earlier > > (https://lkml.org/lkml/2011/12/1/109) so they can work for the > > !CONFIG_OF case ;-) > > Part of my reasoning was that simple_ops appeared to be intended for > DT-based controllers; is it safe to use those ops for a controller that > wasn't instantiated from DT? I know that right now, the only op in that > structure is dt_translate, and that won't ever be called for the non-DT > case, but is there a guarantee that more functions won't be added to > the simple ops, and that they won't assume DT is in use, and fail if > not? > > If these are safe to use in the non-DT case, then yet I could build off > Jamie's patch, although managing the dependencies might be awkward. Yes, it's absolutely fine to use it just that irq_simple_ops isn't currently exported unless you have CONFIG_OF_IRQ set so you'd get an undefined reference for !CONFIG_OF at the moment. Jamie