All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
To: Grant Likely <grant.likely@secretlab.ca>
Cc: linux-kernel@vger.kernel.org,
	alexander.stein@systec-electronic.com, qi.wang@intel.com,
	yong.y.wang@intel.com, joel.clark@intel.com,
	kok.howg.ewe@intel.com, toshiharu-linux@dsn.okisemi.com,
	tglx@linutronix.de
Subject: Re: [PATCH v4] pch_gpio: Support interrupt function
Date: Tue, 05 Jul 2011 12:52:29 +0900	[thread overview]
Message-ID: <4E128A7D.9020508@dsn.okisemi.com> (raw)
In-Reply-To: <20110704163137.GA28042@ponder.secretlab.ca>

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.


>> @@ -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 ?

-- 
tomoya
OKI SEMICONDUCTOR CO., LTD.

  reply	other threads:[~2011-07-05  3:52 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-01  2:16 [PATCH v4] pch_gpio: Support interrupt function Tomoya MORINAGA
2011-07-04 16:31 ` Grant Likely
2011-07-05  3:52   ` Tomoya MORINAGA [this message]
2011-07-05  3:59     ` Grant Likely
2011-07-05  5:52       ` Tomoya MORINAGA
2011-07-11  8:27         ` Tomoya MORINAGA
2011-07-15 20:14           ` Grant Likely
2011-07-19  8:02             ` Tomoya MORINAGA

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4E128A7D.9020508@dsn.okisemi.com \
    --to=tomoya-linux@dsn.okisemi.com \
    --cc=alexander.stein@systec-electronic.com \
    --cc=grant.likely@secretlab.ca \
    --cc=joel.clark@intel.com \
    --cc=kok.howg.ewe@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=qi.wang@intel.com \
    --cc=tglx@linutronix.de \
    --cc=toshiharu-linux@dsn.okisemi.com \
    --cc=yong.y.wang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.