login
Search
Hi,
x86: get irq for hpet timer
HPET timer's IRQ is 0 by default, so we have to select which irq
will be used for these timers. We wait to set the timer's irq until
we really turn on interrupt in order to reduce the chance of
conflicting with some legacy device.Signed-off-by: Kevin Hao <kexin.hao@windriver.com>
---
drivers/char/hpet.c | 62 +++++++++++++++++++++++++++++++++++++------------
include/linux/hpet.h | 3 +-
2 files changed, 49 insertions(+), 16 deletions(-)diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
index e7fb0bc..67714c2 100644
--- a/drivers/char/hpet.c
+++ b/drivers/char/hpet.c
@@ -383,6 +383,51 @@ hpet_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
return hpet_ioctl_common(devp, cmd, arg, 0);
}+static int hpet_timer_get_irq(struct hpet_dev *devp)
+{
+ struct hpet_timer __iomem *timer;
+ struct hpet __iomem *hpet;
+ struct hpets *hpetp;
+ unsigned long cap, irq_flags;
+ int irq;
+
+ timer = devp->hd_timer;
+ hpet = devp->hd_hpet;
+ hpetp = devp->hd_hpets;
+
+ irq = devp->hd_hdwirq;
+
+ sprintf(devp->hd_name, "hpet%d", (int)(devp - hpetp->hp_dev));
+ irq_flags = devp->hd_flags & HPET_SHARED_IRQ
+ ? IRQF_SHARED : IRQF_DISABLED;
+ if (irq) {
+ if (request_irq(irq, hpet_interrupt, irq_flags,
+ devp->hd_name, (void *)devp)) {
+ printk(KERN_ERR "hpet: IRQ %d is not free\n", irq);
+ irq = 0;
+ }
+ return irq;
+ }
+
+ cap = (readq(&timer->hpet_config) & Tn_INT_ROUTE_CAP_MASK)
+ >> Tn_INT_ROUTE_CAP_SHIFT;
+
+ for (irq = find_first_bit(&cap, HPET_MAX_IRQ); irq < HPET_MAX_IRQ;
+ irq = find_next_bit(&cap, HPET_MAX_IRQ, 1 + irq)) {
+ if (request_irq(irq, hpet_interrupt, irq_flags,
+ devp->hd_name, (void *)devp)) {
+ printk(KERN_WARNING "hpet: IRQ %d is not free\n", irq);
+ continue;
+ }
+ break;
+ }
+
+ if (irq >= HPET_MAX_IRQ)
+ irq = 0;
+
+ return irq;
+}
+
static int hpet_ioctl_ieon(str...
This warning message will be output for every interrupt that is in use
by another device. I think it would be better to postpone complaining
until after the loop, if no interrupt has been found at all.Regards,
Clemens
--
Ok, I agree with you.
This is the revised version.Best Regards,
Kevin
---
drivers/char/hpet.c | 63 ++++++++++++++++++++++++++++++++++++++------------
include/linux/hpet.h | 3 +-
2 files changed, 50 insertions(+), 16 deletions(-)diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
index e7fb0bc..0fdc627 100644
--- a/drivers/char/hpet.c
+++ b/drivers/char/hpet.c
@@ -383,6 +383,52 @@ hpet_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
return hpet_ioctl_common(devp, cmd, arg, 0);
}+static int hpet_timer_get_irq(struct hpet_dev *devp)
+{
+ struct hpet_timer __iomem *timer;
+ struct hpet __iomem *hpet;
+ struct hpets *hpetp;
+ unsigned long cap, irq_flags;
+ int irq;
+
+ timer = devp->hd_timer;
+ hpet = devp->hd_hpet;
+ hpetp = devp->hd_hpets;
+
+ irq = devp->hd_hdwirq;
+
+ sprintf(devp->hd_name, "hpet%d", (int)(devp - hpetp->hp_dev));
+ irq_flags = devp->hd_flags & HPET_SHARED_IRQ
+ ? IRQF_SHARED : IRQF_DISABLED;
+ if (irq) {
+ if (request_irq(irq, hpet_interrupt, irq_flags,
+ devp->hd_name, (void *)devp)) {
+ printk(KERN_ERR "hpet: IRQ %d is not free\n", irq);
+ irq = 0;
+ }
+ return irq;
+ }
+
+ cap = (readq(&timer->hpet_config) & Tn_INT_ROUTE_CAP_MASK)
+ >> Tn_INT_ROUTE_CAP_SHIFT;
+
+ for (irq = find_first_bit(&cap, HPET_MAX_IRQ); irq < HPET_MAX_IRQ;
+ irq = find_next_bit(&cap, HPET_MAX_IRQ, 1 + irq)) {
+ if (request_irq(irq, hpet_interrupt, irq_flags,
+ devp->hd_name, (void *)devp))
+ continue;
+ break;
+ }
+
+ if (irq >= HPET_MAX_IRQ) {
+ printk(KERN_WARNING "hpet: No available IRQ for %s timer\n",
+ devp->hd_name);
+ irq = 0;
+ }
+
+ return irq;
+}
+
static int hpet_ioctl_ieon(struct hpet_dev *devp)
{
struct hpet_timer __iomem *timer;
@@ -412,21 +458,7 @@ static int hpet_ioctl_ieon(struct hpet_dev *devp)
devp->hd_flags |= HPET_SHARED_IRQ;
spin_unlock_irq(&hpet_lock);- irq = devp->hd_hdwi...
This spams my log with interrupt sharing violations. As long as we do
not know that the interrupt slot is empty, we need IRQF_PROBE_SHARED
here.Another problem: the interrupt controller doesn't get correctly
initialized for some interrupt line that didn't already have some
routing:
| $ cat /proc/interrupts
| CPU0
| 0: 63 IO-APIC-edge timer
| 1: 96 IO-APIC-edge i8042
| 2: 0 XT-PIC-XT hpet2
| 6: 3 IO-APIC-edge floppy
| 7: 0 IO-APIC-edge parport0
| 8: 3 IO-APIC-edge rtc
| 9: 0 IO-APIC-fasteoi acpi
| ...Additionally, I vaguely remember that on X86, there is some funny stuff
going on with interrupt lines 0, 2 and 8 which means that the interrupt
number passed to request_irq() is not necessarily identical to the
hardware interrupt line.I don't know which of these problems is responsible, or if I'm totally
wrong, but on my machine, interrupts from hpet2 do not arrive.Regards,
Clemens
--
We can simply skip these special IRQ. :-)
Does anyone has a better solution?----
diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
index 0fdc627..b04a15d 100644
--- a/drivers/char/hpet.c
+++ b/drivers/char/hpet.c
@@ -390,6 +390,11 @@ static int hpet_timer_get_irq(struct hpet_dev *devp)
struct hpets *hpetp;
unsigned long cap, irq_flags;
int irq;
+ /*
+ * skip IRQ0, IRQ2, IRQ8 because which is always used by some
+ * legacy device
+ */
+ unsigned long skip_irq = (1 << 0) | (1 << 2) | (1 << 8);timer = devp->hd_timer;
hpet = devp->hd_hpet;
@@ -411,6 +416,9 @@ static int hpet_timer_get_irq(struct hpet_dev *devp)cap = (readq(&timer->hpet_config) & Tn_INT_ROUTE_CAP_MASK)
>> Tn_INT_ROUTE_CAP_SHIFT;
+ cap &= ~skip_irq;
+
+ irq_flags |= IRQF_PROBE_SHARED;for (irq = find_first_bit(&cap, HPET_MAX_IRQ); irq < HPET_MAX_IRQ;
irq = find_next_bit(&cap, HPET_MAX_IRQ, 1 + irq)) {---
Best Regards,
--
Hmm, you probably want to skip all lines that are edge-triggered.
Otherwise you may have problems with sharing. Drivers for devices used
with these edge-triggered lines may have special provisions to permit
sharing in a crafted way in special arrangements (cf. the 8250 serial or
the IDE driver), but that may not work in a generic way with a different
driver in the scenario. And the polarity may be wrong too --
edge-triggered lines are often active-high, because it's fine with them.This driver is quite platform-specific -- how about instead of blindly
probing for interrupt lines, you actually allocate one somehow in platform
code? For example the x86 platform could select an otherwise unused
interrupt line for you -- having routed all the legacy and PCI devices,
these systems quite frequently are left with a couple of otherwise unused
I/O APIC inputs. If not, sharing with a PCI interrupt line would be a
good choice. AFAIK, except from 8254 and RTC emulation the HPET cannot be
used with the legacy 8259A interrupt controllers, so that is a non issue
(but platform will know to disable your driver then).Maciej
--
HPET interrupts can be either edge or level triggered. Probably we
should modify it according to the type of the interrupt line we'reAn edge-triggered HPET interrupt line is not shared (we set IRQF_SHARED
I don't have much of a clue about that "somehow", but this sound like a
good idea. ;-)I think hpet_reserve_platform_timers() might be the place for this.
It gets called from hpet_late_init(), which is a fs_initcall, so I think
we should be careful not to grab some unsharable interrupt that might be
needed by some ISA device whose driver is initialized later.
(This was a bug in 2.6.25-rc5: <http://bugzilla.kernel.org/show_bug.cgi?id=10382>)Regards,
Clemens
--
Edge-triggered lines are generally associated with legacy devices. It
I have had a look at the relevant areas of ACPI and HPET specs and it
looks pretty straightforward, although not all the information is recorded
in system tables. Essentially you are free to choose an arbitrary
interrupt supported by the HPET -- which you can find in the timer's
routing capability (as the proposed patch is doing).However you are right you really want to select one which does not
conflict with a legacy device, so it's probably best to avoid the legacy
range altogether (worth noting as for example the system I have handy has
the capability of its timer #2 set to 0x00f00800 -- here IRQ11 may not be
safe to use) and then you have to check how many inputs beyond the legacy
range are supported by the I/O APICs in the system -- you can have a look
at mp_find_ioapic() to see how obtain that information and then you can
call mp_register_gsi() on the interrupt line selected like this to set up
routing in the I/O APIC as necessary. Level-triggered mode has to be used
as the resulting interrupt entry may happen to be shared with a PCI
interrupt.Though I have just noticed there is something wrong with the spec -- it
says that "The interrupts are all active high." which precludes sharing,
hmm... -- broken spec? If hardware designers actually followed it in this
respect (I wouldn't be surprised as for some of them software is abstract
enough a concept not to be bothered with, and then it is a spec after
all), then I am afraid we need to have a way to get an exclusive
reservation of an I/O APIC line. It could be tough with a system using
fixed routing and reusing a legacy IRQ might be the only choice -- if
supported by the HPET router, that is.Of course if the HPET supports MSI delivery and the kernel configuration
has it enabled, then you can avoid all the hassle with finding an
available IRQ line altogether.Maciej
--
But if we avoid all the legacy range, the hpet timer will have no chance
Yes, we should setup routing in the I/O APIC and use level-triggered
mode. I think it's better to use acpi_register_gsi than mp_register_gsi.
Is that right?Best Regards,
--
It's not meant to be wired to the legacy PIC anyway (except from the
timers #0 and #1 in some configurations, meant to emulate timer interrupts
of the 8254 and the RTC), though I can imagine such a non-standardIt sounds right to me.
Maciej
--
Sorry for the delays.
I have revised this patch according to Maciej & Clemens suggestions.
The modifications include:1. Assign a irq to the timer when we open the timer device. I think it's
better than do it in platform code. So we will have little impact on
other device.2. Set IRQ routing entry for the irq we select.
3. If we in PIC mode, we skip all the well known legacy IRQ. And in io
apic we skip all the legacy IRQ.4. Use level triggered mode by default.
BTW, Maciej. I am not familiar with ACPI subsystem. But I think the
acpi_register_gsi should return -1 if mp_find_ioapic return error.
Is that right? If you also think so, I will make a patch for that.---
drivers/char/hpet.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/hpet.h | 3 +-
2 files changed, 63 insertions(+), 1 deletion(-)
---
commit fa8e14bc3abb93832fbc9b7f86da0d2cf5b3b7a7
Author: Kevin Hao <kexin.hao@windriver.com>
Date: Wed May 28 13:28:20 2008 +0800HPET timer's IRQ is 0 by default. So we have to select which irq
will be used by these timers. We wait to set the timer's irq until
we really open it in order to reduce the chance of conflicting with
other device.Signed-off-by: Kevin Hao <kexin.hao@windriver.com>
diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
index e7fb0bc..94dbfdc 100644
--- a/drivers/char/hpet.c
+++ b/drivers/char/hpet.c
@@ -184,6 +184,65 @@ static irqreturn_t hpet_interrupt(int irq, void *data)
return IRQ_HANDLED;
}+static void hpet_timer_set_irq(struct hpet_dev *devp)
+{
+ unsigned long v;
+ int irq, gsi;
+ struct hpet_timer __iomem *timer;
+
+ spin_lock_irq(&hpet_lock);
+ if (devp->hd_hdwirq) {
+ spin_unlock_irq(&hpet_lock);
+ return;
+ }
+
+ timer = devp->hd_timer;
+
+ /* we prefer level triggered mode */
+ v = readl(&timer->hpet_config);
+ if (!(v & Tn_INT_TYPE_CNF_MASK)) {
+ v |= Tn_INT_TYPE_CNF_MASK;
+ writel(v, &timer->hpet_...
Well, the interface is documented in the sources -- I do not feel like
reading through all the execution paths to see whether the implementation
matches though. ;)A few comments below -- I may not have time available to throw your piece
Please note that when routing through the 8259A you need to match the
trigger mode set for the respective input. It is normally set in the ELCR
register in the hardware and should be recorded in the ACPI interrupt
source tables too -- the default for IRQ0-15 is edge-triggered, activeQuite reasonable assumption IMO, but again -- I'd expect the information
about legacy devices present on these lines to be recorded in ACPI tables.
Note that IRQ9 is used for the SCI -- I am not sure if that's a goodI am assuming you have verified ACPI_ACTIVE_LOW works here, contrary to
Tab here for consistency with the others?
Otherwise no obvious problems.
Maciej
--
The IRQ trigger mode is set to level by acpi_register_gsi in PIC mode.
And I have tested it on a host using legacy PIC. It works well.Yes, we should record these information in ACPI interrupt source table.
And I have noticed that a patch has been made to update mptable.
http://lkml.org/lkml/2008/5/25/255We can use mp_config_acpi_gsi to setup route table. But now can we add
somethings like "FIXME: setup interrupt source table" and wait for thatOk, fix it.
Thanks for your comments.
---
HPET timer's IRQ is 0 by default. So we have to select which irq
will be used by these timers. We wait to set the timer's irq until
we really open it in order to reduce the chance of conflicting with
other device.Signed-off-by: Kevin Hao <kexin.hao@windriver.com>
---
diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
index e7fb0bc..c9bf5d4 100644
--- a/drivers/char/hpet.c
+++ b/drivers/char/hpet.c
@@ -184,6 +184,67 @@ static irqreturn_t hpet_interrupt(int irq, void *data)
return IRQ_HANDLED;
}+static void hpet_timer_set_irq(struct hpet_dev *devp)
+{
+ unsigned long v;
+ int irq, gsi;
+ struct hpet_timer __iomem *timer;
+
+ spin_lock_irq(&hpet_lock);
+ if (devp->hd_hdwirq) {
+ spin_unlock_irq(&hpet_lock);
+ return;
+ }
+
+ timer = devp->hd_timer;
+
+ /* we prefer level triggered mode */
+ v = readl(&timer->hpet_config);
+ if (!(v & Tn_INT_TYPE_CNF_MASK)) {
+ v |= Tn_INT_TYPE_CNF_MASK;
+ writel(v, &timer->hpet_config);
+ }
+ spin_unlock_irq(&hpet_lock);
+
+ v = (readq(&timer->hpet_config) & Tn_INT_ROUTE_CAP_MASK) >>
+ Tn_INT_ROUTE_CAP_SHIFT;
+
+ /*
+ * In PIC mode, skip IRQ0-4, IRQ6-9, IRQ12-15 which is always used by
+ * legacy device. In IO APIC mode, we skip all the legacy IRQS.
+ */
+ if (acpi_irq_model == ACPI_IRQ_MODEL_PIC)
+ v &= ~0xf3df;
+ else
+ v &= ~0xffff;
+
+ for (irq = find_first_bit(&v, HPET_MAX_IRQ); irq < HPET_MAX...
It may work by chance. All what the code in question guarantees is:
int acpi_gsi_to_irq(u32 gsi, unsigned int *irq)
{
*irq = gsi;
return 0;
}int acpi_register_gsi(u32 gsi, int triggering, int polarity)
{
unsigned int irq;
unsigned int plat_gsi = gsi;acpi_gsi_to_irq(plat_gsi, &irq);
return irq;
}CONFIG_PCI is optional for the x86 platform. You could argue this is a
bug in acpi_register_gsi() and I tend to agree -- I think the existence of
the ELCR is implied for ACPI-compliant systems, though I am happy to be
corrected. The ACPI spec does not mention the register, but quotes:
"Existing industry standard register interfaces to: CMOS, PIC, PITs, ..."Anyway, blindly attaching to an IRQ line is not a terribly good idea as
there may be an edge-triggered line from a device wired there after all.
For example many LPC SuperIO chips have configurable IRQ resources -- the
parallel port can use either IRQ5 or IRQ7 (but not both at a time). OTOH
IRQ6 may be free after all as floppy drives become rare. Only IRQ10 and
IRQ11 are probably safe to be assumed free in systems with no ISA slots,
but then again, I may have missed something. Hopefully there are no true
PCI-ISA bridges with the HPET implemented -- I have checked a couple of
datasheets for ACPI-compliant PCI-ISA bridges (such as the PIIX4) that IGood to know. Thanks for verifying this.
Maciej
--
Oh, I overlook the existence of CONFIG_PCI.
After look into the code, I think we should add a acpi_pic_set_trigger
Yes, I agree with you. But at present we have not a better way to
resolve this problem.Thanks,
Kevin--
i've applied your patch for more testing to tip/timers/hpet. Thanks,
Ingo
--
IRQ2 cannot be wired to the legacy 8259A chips, because this line is
taken for the cascade from the slave to the master. If you know for sure
that the HPET2 can be routed to the an otherwise unused input of the I/OYou are correct as far as input lines INT0 and INT2 of the I/O APIC are
concerned. INT8 is not generally a problem except from some systems where
the polarity used to be recorded incorrectly in the BIOS tables (hardware
can be configured for either). With INT0 and INT2 the following is
usually the case.INT0 is wired to the output of the master 8259A legacy controller. It is
either configured as an ExtINTA interrupt or disabled by default
altogether (and the LINT0 input of the local APIC of the bootstrap
processor used instead). Even if some additional logic is present in the
system so that another source can be routed to this line and the 8259A is
quiesced, sharing is probably impossible, because the output of the 8259A
is traditionally active high.INT2 is wired to the output of the 8254 timer (and in some more recent
systems shared with the HPET0). This is the line that is referred to as
IRQ0 above. This is because in an ACPI-based system the IRQ numbers refer
to lines as defined in ACPI tables, which may not necessarily identity-map
to I/O APIC input lines.Then the reasons for routing INT0 and INT2 like this are generally
historical and date back to the old Multi-Processor Specification. There
may be systems that differ, but care must be taken and the correct
configuration table consulted.Maciej
--
Yes, this is alright. But i've seen some machines where we have a
valid IRQ in 'devp->hd_hdwirq' which you should not ignore. So, first
check if it is zero and only then do a hpet_timer_get_irq.--
warm regardsBalaji Rao
NITK Surathkal
--
Yes, I have checked that case in hpet_timer_get_irq function. See the
following code:+ irq = devp->hd_hdwirq;
+ if (irq) {
+ if (request_irq(irq, hpet_interrupt, irq_flags,
+ devp->hd_name, (void *)devp)) {
+ printk(KERN_ERR "hpet: IRQ %d is not free\n",
irq);
+ irq = 0;
+ }
+ return irq;
+ }Is that right?
Thanks for your comments.
Best Regards,
--
Yes. You are right. Sorry for not going through the patch fully before making
noise.--
Warm Regards,Balaji Rao
Dept. of Mechanical Engineering
NITK
--
| Benjamin Herrenschmidt | Re: [linux-pm] [PATCH] Remove process freezer from suspend to RAM pathway |
| Ulrich Drepper | Re: [patch 7/8] fdmap v2 - implement sys_socket2 |
| Washington Odhiambo | Weird Problem with NAT - more details |
| Greg Kroah-Hartman | [PATCH 001/196] Chinese: Add the known_regression URI to the HOWTO |
git: | |
| Gerrit Renker | [PATCH 27/37] dccp: Integration of dynamic feature activation - part 2 (server side) |
| Frans Pop | svc: failed to register lockdv1 RPC service (errno 97). |
| Jarek Poplawski | [PATCH take 2] pkt_sched: Protect gen estimators under est_lock. |
| David Miller | Re: [GIT]: Networking |
