On Monday 23 October 2006 22:46, Pallipadi, Venkatesh wrote:
> >Section 8.4.4.1 (_PCT) of the 3.0 ACPI spec says:
> >
> >  OSPM performs processor performance transitions by writing
> >  the performance state-specific control value to a Performance
> >  Control Register (PERF_CTRL).
> >
> >According to one of our architecture guys, this means we really
> >ought to have an OpRegion driver that encapsulates the PAL_SET_PSTATE
> >call.
> 
> Actually it is slightly different from low_level_read and write. 
> Generic ACPI definition of ACPI PERF_CTRL and PERF_STATUS define 
> them as if they are registers. But, with FfixedHW, ACPI allows 
> architectures to implement this functionality in a native way.

I'd like to understand this better.  Something like the following
patch (not for inclusion, may not even compile) is what I'm
thinking.  This seems more in line with the spec intent.

Apart from details like "bit_width <= 8" vs. "bit_width == 8",
this should be functionally the same.  Are you saying it's different
because of those details, or is there a larger difference I don't
understand?

Nacked-by: Bjorn Helgaas

Index: work-1/arch/i386/kernel/cpu/cpufreq/acpi-cpufreq.c
===================================================================
--- work-1.orig/arch/i386/kernel/cpu/cpufreq/acpi-cpufreq.c     2006-10-25 
17:04:46.000000000 -0600
+++ work-1/arch/i386/kernel/cpu/cpufreq/acpi-cpufreq.c  2006-10-25 
17:07:34.000000000 -0600
@@ -61,55 +61,16 @@
 static unsigned int acpi_pstate_strict;
 
 static int
-acpi_processor_write_port(
-       u16     port,
-       u8      bit_width,
-       u32     value)
-{
-       if (bit_width <= 8) {
-               outb(value, port);
-       } else if (bit_width <= 16) {
-               outw(value, port);
-       } else if (bit_width <= 32) {
-               outl(value, port);
-       } else {
-               return -ENODEV;
-       }
-       return 0;
-}
-
-static int
-acpi_processor_read_port(
-       u16     port,
-       u8      bit_width,
-       u32     *ret)
-{
-       *ret = 0;
-       if (bit_width <= 8) {
-               *ret = inb(port);
-       } else if (bit_width <= 16) {
-               *ret = inw(port);
-       } else if (bit_width <= 32) {
-               *ret = inl(port);
-       } else {
-               return -ENODEV;
-       }
-       return 0;
-}
-
-static int
 acpi_processor_set_performance (
        struct cpufreq_acpi_io  *data,
        unsigned int            cpu,
        int                     state)
 {
-       u16                     port = 0;
-       u8                      bit_width = 0;
        int                     i = 0;
-       int                     ret = 0;
        u32                     value = 0;
        int                     retval;
        struct acpi_processor_performance       *perf;
+       acpi_status             status;
 
        dprintk("acpi_processor_set_performance\n");
 
@@ -132,16 +93,16 @@
         * control_register.
         */
 
-       port = perf->control_register.address;
-       bit_width = perf->control_register.bit_width;
        value = (u32) perf->states[state].control;
 
-       dprintk("Writing 0x%08x to port 0x%04x\n", value, port);
+       dprintk("Writing 0x%08x to PERF_CTRL\n", value);
 
-       ret = acpi_processor_write_port(port, bit_width, value);
-       if (ret) {
-               dprintk("Invalid port width 0x%04x\n", bit_width);
-               return (ret);
+       status = acpi_hw_low_level_write(perf->control_register.bit_width,
+               value, perf->control_register);
+
+       if (ACPI_FAILURE(status)) {
+               dprintk("Failure writing PERF_CTRL\n");
+               return -ENODEV;
        }
 
        /*
@@ -157,17 +118,15 @@
                 * before giving up.
                 */
 
-               port = perf->status_register.address;
-               bit_width = perf->status_register.bit_width;
-
-               dprintk("Looking for 0x%08x from port 0x%04x\n",
-                       (u32) perf->states[state].status, port);
+               dprintk("Looking for 0x%08x from PERF_CTRL\n",
+                       (u32) perf->states[state].status);
 
                for (i = 0; i < 100; i++) {
-                       ret = acpi_processor_read_port(port, bit_width, &value);
-                       if (ret) {      
-                               dprintk("Invalid port width 0x%04x\n", 
bit_width);
-                               return (ret);
+                       status = 
acpi_hw_low_level_read(perf->control_register.bit_width,
+                               &value, perf->control_register);
+                       if (ACPI_FAILURE(status)) {
+                               dprintk("Failure reading PERF_CTRL\n");
+                               return -ENODEV;
                        }
                        if (value == (u32) perf->states[state].status)
                                break;
@@ -473,15 +432,6 @@
                goto err_unreg;
        }
 
-       if ((perf->control_register.space_id != ACPI_ADR_SPACE_SYSTEM_IO) ||
-           (perf->status_register.space_id != ACPI_ADR_SPACE_SYSTEM_IO)) {
-               dprintk("Unsupported address space [%d, %d]\n",
-                       (u32) (perf->control_register.space_id),
-                       (u32) (perf->status_register.space_id));
-               result = -ENODEV;
-               goto err_unreg;
-       }
-
        /* alloc freq_table */
        data->freq_table = kmalloc(sizeof(struct cpufreq_frequency_table) * 
(perf->state_count + 1), GFP_KERNEL);
        if (!data->freq_table) {

-
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

Reply via email to