hpet: convert from ACPI to PNP driver
cover hpet: convert from ACPI to PNP driver
The hpet driver parses ACPI _CRS data manually, but PNPACPI already does that, and it does a more complete job, so we can simplify the hpet driver quite a bit by taking advantage of that.
---
Bjorn Helgaas (3):
hpet: convert to PNP driver
hpet: pass physical address, not entire hpet_data, to hpet_is_known()
MAINTAINERS: remove obsolete HPET ACPI entry
MAINTAINERS | 5 -- drivers/char/hpet.c | 110 ++++++++++++++------------------------------------- 2 files changed, 30 insertions(+), 85 deletions(-)
PATCH 1/3 hpet: convert to PNP driver
PNPACPI already parses _CRS, and does a more complete job than we did here, so let's just take advantage of that.
---
drivers/char/hpet.c | 103 +++++++++++++-------------------------------------- 1 files changed, 27 insertions(+), 76 deletions(-)
drivers/char/hpet.c +27 -76
@@ -31,6 +31,7 @@#include <linux/seq_file.h> #include <linux/bitops.h> #include <linux/clocksource.h> +#include <linux/pnp.h> #include <asm/current.h> #include <asm/uaccess.h>
@@ -40,7 +41,6 @@#include <asm/div64.h> #include <linux/acpi.h> -#include <acpi/acpi_bus.h> #include <linux/hpet.h> /*
@@ -899,100 +899,53 @@ int hpet_alloc(struct hpet_data *hdp)return 0 ; } -static acpi_status hpet_resources (struct acpi_resource *res,void *data) +static int hpet_pnp_add (struct pnp_dev *dev,const struct pnp_device_id *dev_id) { -struct hpet_data *hdp; -acpi_status status; -struct acpi_resource_address64 addr; - - hdp = data; - - status =acpi_resource_to_address64 (res, &addr); - -if (ACPI_SUCCESS (status)) { - hdp->hd_phys_address = addr.minimum; - hdp->hd_address =ioremap (addr.minimum, addr.address_length); - -if (hpet_is_known (hdp)) { -iounmap (hdp->hd_address); -return AE_ALREADY_EXISTS ; - } - }else if (res->type ==ACPI_RESOURCE_TYPE_FIXED_MEMORY32 ) { -struct acpi_resource_fixed_memory32 *fixmem32; - - fixmem32 = &res->data.fixed_memory32; -if (!fixmem32) -return AE_NO_MEMORY ; +struct hpet_data data; +struct resource *mem, *irq; +int i; - hdp->hd_phys_address = fixmem32->address; - hdp->hd_address =ioremap (fixmem32->address, -HPET_RANGE_SIZE ); +memset (&data,0 ,sizeof (data)); -if (hpet_is_known (hdp)) { -iounmap (hdp->hd_address); -return AE_ALREADY_EXISTS ; - } - }else if (res->type ==ACPI_RESOURCE_TYPE_EXTENDED_IRQ ) { -struct acpi_resource_extended_irq *irqp; -int i, irq; + mem =pnp_get_resource (dev,IORESOURCE_MEM ,0 ); +if (!mem) +return -ENODEV ; - irqp = &res->data.extended_irq; + data.hd_phys_address = mem->start; -for (i =0 ; i < irqp->interrupt_count; i++) { - irq =acpi_register_gsi (NULL , irqp->interrupts[i], - irqp->triggering, irqp->polarity); -if (irq <0 ) -return AE_ERROR ; +if (hpet_is_known (&data)) +return -EBUSY ; - hdp->hd_irq[hdp->hd_nirqs] = irq; - hdp->hd_nirqs++; - } + i =0 ; +while (data.hd_nirqs <ARRAY_SIZE (data.hd_irq)) { + irq =pnp_get_resource (dev,IORESOURCE_IRQ , i++); +if (pnp_resource_valid (irq) &&pnp_resource_enabled (irq)) + data.hd_irq[data.hd_nirqs++] = irq->start; } - -return AE_OK ; -} - -static int hpet_acpi_add (struct acpi_device *device) -{ -acpi_status result; -struct hpet_data data; - -memset (&data,0 ,sizeof (data)); - - result = -acpi_walk_resources (device->handle,METHOD_NAME__CRS , - hpet_resources, &data); - -if (ACPI_FAILURE (result)) +if (!data.hd_nirqs)return -ENODEV ; -if (!data.hd_address || !data.hd_nirqs) { -printk ("%s: no address or irqs in _CRS\n" , __func__); -return -ENODEV ; - } + data.hd_address =ioremap (mem->start,resource_size (mem));return hpet_alloc (&data); } -static int hpet_acpi_remove (struct acpi_device *device,int type) +static void hpet_pnp_remove (struct pnp_dev *dev) {/* XXX need to unregister clocksource, dealloc mem, etc */ -return -EINVAL ; } -static const struct acpi_device_id hpet_device_ids[] = { +static const struct pnp_device_id hpet_device_ids[] = { {"PNP0103" ,0 }, {"" ,0 }, };MODULE_DEVICE_TABLE (acpi, hpet_device_ids); -static struct acpi_driver hpet_acpi_driver = { +static struct pnp_driver hpet_pnp_driver = { .name ="hpet" , - .ids = hpet_device_ids, - .ops = { - .add = hpet_acpi_add, - .remove = hpet_acpi_remove, - }, + .id_table = hpet_device_ids, + .probe = hpet_pnp_add, + .remove = hpet_pnp_remove, };static struct miscdevice hpet_misc = {HPET_MINOR ,"hpet" , &hpet_fops };
@@ -1007,7 +960,7 @@ static int __init hpet_init(void) sysctl_header =register_sysctl_table (dev_root); - result =acpi_bus_register_driver (&hpet_acpi_driver); + result =pnp_register_driver (&hpet_pnp_driver);if (result <0 ) {if (sysctl_header)unregister_sysctl_table (sysctl_header);
@@ -1020,13 +973,11 @@ static int __init hpet_init(void) static void __exit hpet_exit(void) { -acpi_bus_unregister_driver (&hpet_acpi_driver); +pnp_unregister_driver (&hpet_pnp_driver);if (sysctl_header)unregister_sysctl_table (sysctl_header);misc_deregister (&hpet_misc); - -return ; }module_init (hpet_init);
Re: [PATCH 1/3] hpet: convert to PNP driver
Quoted 4 lines · Bjorn Helgaas
PNPACPI already parses
_CRS, and does a more complete job than we did here, so let's just take advantage of that.
Patch looks good.
One comment is that we now have a dependency on CONFIG_PNP. That probably needs to be added in drivers/char/Kconfig. But, that also means we may fail for users who do not have PNP in their config today. May be just select PNP when HPET is selected?
Quoted 8 lines
Signed-off-by Bjorn Helgaas <[email protected]>---
drivers/char/hpet.c | 103 +++++++++++++--------------------------------------
1 files changed, 27 insertions(+), 76 deletions(-)
Quoted 8 lines
- if (!data.hd_address || !data.hd_nirqs) {
-printk("%s: no address or irqs in _CRS\n", __func__);
- return-ENODEV;
- }
+ data.hd_address =ioremap(mem->start, resource_size(mem));return
hpet_alloc(&data);
}
Not a problem with this patch. But, don't we have a problem of rogue ioremap when hpet_alloc() fails here?
Thanks,
Venki
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] hpet: convert to PNP driver
Quoted 10 lines · Venkatesh Pallipadi
Quoted 3 lines · Bjorn Helgaas
PNPACPI already parses
_CRS, and does a more complete job than we did here, so let's just take advantage of that.Patch looks good.
One comment is that we now have a dependency onCONFIG_PNP. That probably needs to be added indrivers/char/Kconfig. But, that also means we may fail for users who do not have PNP in their config today. May be just select PNP when HPET is selected?
Good point; I missed that. I could just add #ifdef CONFIG_PNP around the relevant code in hpet.c. That's the typical way we handle this.
drivers/char/hpet.c already depends on ACPI for another reason (acpi_irq_model), and we do select PNP whenever CONFIG_ACPI=y, so I don't think the CONFIG_PNP=n case will arise in practice. But it'd probably be better to have the #ifdef wrapper than to have the implicit dependency on the "select PNP" in drivers/acpi/Kconfig.
Quoted 14 lines
Quoted 14 lines
Signed-off-by Bjorn Helgaas <[email protected]>---
drivers/char/hpet.c | 103 +++++++++++++--------------------------------------
1 files changed, 27 insertions(+), 76 deletions(-)- if (!data.hd_address || !data.hd_nirqs) {
-printk("%s: no address or irqs in _CRS\n", __func__);
- return-ENODEV;
- }
+ data.hd_address =ioremap(mem->start, resource_size(mem));
Quoted 5 lines
Quoted 2 lines
return
hpet_alloc(&data);
}Not a problem with this patch. But, don't we have a problem of rogue ioremap when
hpet_alloc()fails here?
You're right. I'll add a patch to fix that while I'm at it.
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 2/3 hpet: pass physical address, not entire hpet_data, to hpet_is_known()
In reply to Bjorn HelgaasNo functional change; hpet_is_known() only needs the physical address, so supplying that instead of the whole struct hpet_data makes the callers a little simpler.
---
drivers/char/hpet.c | 11 +++++------ 1 files changed, 5 insertions(+), 6 deletions(-)
drivers/char/hpet.c +5 -6
@@ -662,12 +662,12 @@ static const struct file_operations hpet_fops = { .mmap = hpet_mmap, }; -static int hpet_is_known (struct hpet_data *hdp) +static int hpet_is_known (unsigned long phys_address) {struct hpets *hpetp;for (hpetp = hpets; hpetp; hpetp = hpetp->hp_next) -if (hpetp->hp_hpet_phys == hdp->hd_phys_address) +if (hpetp->hp_hpet_phys == phys_address)return 1 ;return 0 ;
@@ -788,7 +788,7 @@ int hpet_alloc(struct hpet_data *hdp)* If platform dependent code has allocated the hpet that * ACPI has also reported, then we catch it here. */ -if (hpet_is_known (hdp)) { +if (hpet_is_known (hdp->hd_phys_address)) {printk (KERN_DEBUG "%s: duplicate HPET ignored\n" , __func__);return 0 ;
@@ -911,9 +911,7 @@ static int hpet_pnp_add(struct pnp_dev *dev, const struct pnp_device_id *dev_id)if (!mem)return -ENODEV ; - data.hd_phys_address = mem->start; - -if (hpet_is_known (&data)) +if (hpet_is_known (mem->start))return -EBUSY ; i =0 ;
@@ -925,6 +923,7 @@ static int hpet_pnp_add(struct pnp_dev *dev, const struct pnp_device_id *dev_id)if (!data.hd_nirqs)return -ENODEV ; + data.hd_phys_address = mem->start; data.hd_address =ioremap (mem->start,resource_size (mem));return hpet_alloc (&data);
Re: [PATCH 2/3] hpet: pass physical address, not entire hpet_data, to hpet_is_known()
Quoted 5 lines · Bjorn Helgaas
No functional change;
hpet_is_known()only needs the physical address, so supplying that instead of the whole structhpet_datamakes the callers a little simpler.Signed-off-by Bjorn Helgaas <[email protected]>
PATCH 3/3 MAINTAINERS: remove obsolete HPET ACPI entry
In reply to Bjorn HelgaasBob Picco is no longer at HP.
---
MAINTAINERS | 5 ----- 1 files changed, 0 insertions(+), 5 deletions(-)
MAINTAINERS +0 -5
@@ -2664,11 +2664,6 @@ HPET: x86_64 M: Vojtech Pavlik <[email protected]> S: Maintained -HPET: ACPI -M: Bob Picco <[email protected]> -S: Maintained -F: drivers/char/hpet.c - HPFS FILESYSTEM M: Mikulas Patocka <[email protected]> W: http://artax.karlin.mff.cuni.cz/~mikulas/vyplody/hpfs/index-e.cgi