Re: [PATCH 2/2] gpio/tegra: Dynamically allocate IRQ base, and support DT

[Jamie Iles <jamie-wmLquQDDieKakBO8gow8eQ@public.gmane.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 <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > [...]
> > > >  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