Re: [patch 15/18] PNP: support optional IRQ resources

[Bjorn Helgaas <bjorn.helgaas@hp.com>]

On Saturday 14 June 2008 02:29:01 am Rene Herman wrote:
> On 05-06-08 00:09, Bjorn Helgaas wrote:
> 
> > This patch adds an IORESOURCE_IRQ_OPTIONAL flag for use when
> > assigning resources to a device.  If the flag is set and we are
> > unable to assign an IRQ to the device, we can leave the IRQ
> > disabled but allow the overall resource allocation to succeed.
> > 
> > Some devices request an IRQ, but can run without an IRQ
> > (possibly with degraded performance).  This flag lets us run
> > the device without the IRQ instead of just leaving the
> > device disabled.
> > 
> > This is a reimplementation of this previous change by Rene
> > Herman <rene.herman@gmail.com>:
> >     http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=3b73a223661ed137c5d3d2635f954382e94f5a43
> > 
> > I reimplemented this for two reasons:
> >     - to prepare for converting all resource options into a single linked
> >       list, as opposed to the per-resource-type lists we have now, and
> >     - to preserve the order and number of resource options.
> > 
> > In PNPBIOS and ACPI, we configure a device by giving firmware a
> > list of resource assignments.  It is important that this list
> > has exactly the same number of resources, in the same order,
> > as the "template" list we got from the firmware in the first
> > place.
> > 
> > The problem of a sound card MPU401 being left disabled for want of
> > an IRQ was reported by Uwe Bugla <uwe.bugla@gmx.de>.
> > 
> > Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> 
> You rework this again in the later patch doing the final switch over 
> after which things end up right but also already at this point:
> 
> > @@ -176,33 +184,10 @@ static void quirk_ad1815_mpu_resources(s
> >  	if (!irq || irq->next)
> >  		return;
> >  
> > -	res = dev->dependent;
> > -	if (!res)
> > -		return;
> > -
> > -	while (1) {
> > -		struct pnp_irq *copy;
> > -
> > -		copy = pnp_alloc(sizeof *copy);
> > -		if (!copy)
> > -			break;
> > -
> > -		bitmap_copy(copy->map.bits, irq->map.bits, PNP_IRQ_NR);
> > -		copy->flags = irq->flags;
> > -
> > -		copy->next = res->irq; /* Yes, this is NULL */
> > -		res->irq = copy;
> > -
> > -		if (!res->next)
> > -			break;
> > -		res = res->next;
> > -	}
> > -	kfree(irq);
> > +	irq->flags |= IORESOURCE_IRQ_OPTIONAL;
> > +	dev_info(&dev->dev, "made independent IRQ optional\n");
> >  
> >  	res->next = quirk_isapnp_mpu_options(dev);
> 
> ... this line should just go.

You mean I should delete the "res->next = quirk_isapnp_mpu_options(dev)"
line?

> Previously "res" ended up as the last dependent set through the while 
> loop but now it's the independendent set which in this previous setup of 
> things shouldn't even have a ->next. Just deleting this line makes this 
> patch fine and
> 
> Acked-by: Rene Herman <rene.herman@gmail.com>

OK.  This will mean replacing patches 15 and 18 in this series.  If
I repost those two, is that easy for you to deal with, Andrew?

If I replace these, I'll also update patch 18 to fix the
pnp_independent_option() comment Rene made.

Bjorn