2005-10-12 19:02:25

by Randy Dunlap

[permalink] [raw]
Subject: [PATCH 1/3] hpet: allow fixed_mem32 ACPI resource type

From: Randy Dunlap <[email protected]>

Allow the ACPI HPET description table to use a resource type
of FIXED_MEM32 for the HPET reource. Use the fixed resoure
size of 1 KB for the HPET resource as per the HPET spec.

Signed-off-by: Randy Dunlap <[email protected]>
Acked-by: Bob Picco <[email protected]>
---

drivers/char/hpet.c | 17 +++++++++++++++++
1 files changed, 17 insertions(+)

diff -Naurp linux-2614-rc4/drivers/char/hpet.c~hpet_fixmem32 linux-2614-rc4/drivers/char/hpet.c
--- linux-2614-rc4/drivers/char/hpet.c~hpet_fixmem32 2005-10-12 09:40:24.000000000 -0700
+++ linux-2614-rc4/drivers/char/hpet.c 2005-10-12 09:47:20.000000000 -0700
@@ -49,6 +49,8 @@
#define HPET_USER_FREQ (64)
#define HPET_DRIFT (500)

+#define HPET_RANGE_SIZE 1024 /* from HPET spec */
+
static u32 hpet_ntimer, hpet_nhpet, hpet_max_freq = HPET_USER_FREQ;

/* A lock for concurrent access by app and isr hpet activity. */
@@ -896,6 +898,21 @@ static acpi_status hpet_resources(struct
for (hpetp = hpets; hpetp; hpetp = hpetp->hp_next)
if (hpetp->hp_hpet == hdp->hd_address)
return -EBUSY;
+ } else if (res->id == ACPI_RSTYPE_FIXED_MEM32) {
+ struct acpi_resource_fixed_mem32 *fixmem32;
+
+ fixmem32 = &res->data.fixed_memory32;
+ if (!fixmem32)
+ return -EINVAL;
+
+ hdp->hd_phys_address = fixmem32->range_base_address;
+ hdp->hd_address = ioremap(fixmem32->range_base_address,
+ HPET_RANGE_SIZE);
+
+ for (hpetp = hpets; hpetp; hpetp = hpetp->hp_next)
+ if (hpetp->hp_hpet == hdp->hd_address) {
+ return -EBUSY;
+ }
} else if (res->id == ACPI_RSTYPE_EXT_IRQ) {
struct acpi_resource_ext_irq *irqp;
int i;


---


2005-10-12 20:08:43

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH 1/3] hpet: allow fixed_mem32 ACPI resource type

On Wednesday 12 October 2005 12:58 pm, Randy Dunlap wrote:
> Allow the ACPI HPET description table to use a resource type
> of FIXED_MEM32 for the HPET reource. Use the fixed resoure
> size of 1 KB for the HPET resource as per the HPET spec.

I have a patch in my tree to convert HPET from an ACPI
driver to a PNP driver, using PNPACPI. That should take
care of issues like this.

But my patch is waiting on some PNP work by Adam to allow
PNPACPI devices to have more than 2 IRQs.

In the meantime, I think your patch is fine.

> +#define HPET_RANGE_SIZE 1024 /* from HPET spec */

Out of curiosity, why do you need this? ACPI_RSTYPE_FIXED_MEM32
contains a length field, and my patch uses it. Did you run
into some firmware that supplies incorrect information about the
size of the HPET MMIO area?

Another minor HPET nit I fixed is that it currently doesn't
use request_mem_region(). I did it in PNP terms, so it's
waiting on Adam's work, but maybe it'd be worth an interim
patch until that's ready.

2005-10-12 21:55:54

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH 1/3] hpet: allow fixed_mem32 ACPI resource type

(Sorry, replying from different email address; I've waited > 1 hour
for your email to arrive at linux.intel.com ... :(
Maybe it was a bad choice.)


On Wed, 12 Oct 2005, Bjorn Helgaas wrote:

> On Wednesday 12 October 2005 12:58 pm, Randy Dunlap wrote:
> > Allow the ACPI HPET description table to use a resource type
> > of FIXED_MEM32 for the HPET reource. Use the fixed resoure
> > size of 1 KB for the HPET resource as per the HPET spec.
>
> I have a patch in my tree to convert HPET from an ACPI
> driver to a PNP driver, using PNPACPI. That should take
> care of issues like this.

Thanks, that's good news.

> But my patch is waiting on some PNP work by Adam to allow
> PNPACPI devices to have more than 2 IRQs.
>
> In the meantime, I think your patch is fine.
>
> > +#define HPET_RANGE_SIZE 1024 /* from HPET spec */
>
> Out of curiosity, why do you need this? ACPI_RSTYPE_FIXED_MEM32
> contains a length field, and my patch uses it. Did you run
> into some firmware that supplies incorrect information about the
> size of the HPET MMIO area?

a. The HPET spec says that the HPET block size is 1 KB
in section 3.2.1.

b. Table 3 (HPET description table) requires an HPET base
address (offset 40), but seems not to require a length.
However, the description text again states a fixed block size
of 1 KB.

c. Yes, I've seen descriptor.length field value of 0.
Whether it's incorrect is a discussion for an specster IMO.
At least it's debatable (by someone).

d. Yes, I would prefer to use (valid) length from the descriptor.
Maybe default to 1 KB if desc.length is 0 (?).


> Another minor HPET nit I fixed is that it currently doesn't
> use request_mem_region(). I did it in PNP terms, so it's
> waiting on Adam's work, but maybe it'd be worth an interim
> patch until that's ready.

OK, maybe you can get Bob Picco or me to add that to our
todo lists.

Thanks,
--
~Randy