linux-kernel - Re: [PATCH v4] pch_gpio: Support interrupt function

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110705035902.GB13713@ponder.secretlab.ca>
Date:	Mon, 4 Jul 2011 21:59:02 -0600
From:	Grant Likely <grant.likely@...retlab.ca>
To:	Tomoya MORINAGA <tomoya-linux@....okisemi.com>
Cc:	linux-kernel@...r.kernel.org,
	alexander.stein@...tec-electronic.com, qi.wang@...el.com,
	yong.y.wang@...el.com, joel.clark@...el.com,
	kok.howg.ewe@...el.com, toshiharu-linux@....okisemi.com,
	tglx@...utronix.de
Subject: Re: [PATCH v4] pch_gpio: Support interrupt function

On Tue, Jul 05, 2011 at 12:52:29PM +0900, Tomoya MORINAGA wrote:
> Hi Grant,
> 
> (2011/07/05 1:31), Grant Likely wrote:
> 
> >>+#define PCH_IRQ_BASE		23
> >
> >Yikes!  IRQ ranges should really be dynamically assigned.  Don't hard
> >code an irq base.
> 
> As we discussed before,
> without specifying PCH_IRQ_BASE,
> it seemed IRQ number collision issue occurred.
> Thus, I added PCH_IRQ_BASE.

Then fix the irq collision.  If the core code hands out an irq range
that is in-use by some other driver, then the other driver isn't
reserving it's irqs correctly and it needs to be fixed.  Hacking
around the problem doesn't help anything in the long run.

> >>@@ -202,8 +345,36 @@ static int __devinit pch_gpio_probe(struct pci_dev *pdev,
> >>  		goto err_gpiochip_add;
> >>  	}
> >>
> >>+	irq_base = irq_alloc_descs(-1, PCH_IRQ_BASE, GPIO_NUM_PINS, GFP_KERNEL);
> >>+	if (irq_base<  0) {
> >>+		dev_err(&pdev->dev, "PCH gpio: Failed to get IRQ base num\n");
> >>+		goto err_irq_alloc_descs;
> >>+	}
> >>+	chip->irq_base = irq_base;
> >
> >This looks like it will cause the driver probe to completely fail,
> >even if the GPIO portion of the chip was setup correctly.  I would
> >think that if GPIO works the driver should at least enable that bit
> >even if IRQs are broken.
> 
> Do you mean in case of failing irq_alloc_descs,
> probe() shouldn't return ERROR but SUCCESS. Right ?

That's what I'm suggesting.  I'll leave it up to you whether or not it
is worthwhile though.

g.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/