gmane.linux.kernel.pci

Chen, Justin | 10 Sep 01:22

[PATCH] acpiphp: Match the variable types for ia64

The variable "sun" defined in acpiphp_glue.c as "unsigned long".  But the struct acpiphp_slot defines the
sun as u32 in acpiphp.h.

On the ia64, since the "unsigned long" is u64, the above mismatch may cause the problem if the _SUN returns
the value in 64 bits.

The following patch change the definition in acpiphp.h to match the type of sun with other places.

Singed-off-by: Justin Chen <justin.chen <at> hp.com>

=========================================================================
--- a/drivers/pci/hotplug/acpiphp.h     2008-01-26 15:31:18.000000000 -0800
+++ b/drivers/pci/hotplug/acpiphp.h     2008-09-03 16:05:10.610000000 -0700
@@ -112,7 +112,7 @@ struct acpiphp_slot {

        u8              device;         /* pci device# */

-       u32             sun;            /* ACPI _SUN (slot unique number) */
+       unsigned long   sun;            /* ACPI _SUN (slot unique number) */
        u32             slotno;         /* slot number relative to bridge */
        u32             flags;          /* see below */
 };

Matthew Wilcox | 10 Sep 02:20

Re: [PATCH] acpiphp: Match the variable types for ia64

On Tue, Sep 09, 2008 at 11:22:45PM +0000, Chen, Justin wrote:
> The variable "sun" defined in acpiphp_glue.c as "unsigned long".  But the struct acpiphp_slot defines
the sun as u32 in acpiphp.h.
> 
> On the ia64, since the "unsigned long" is u64, the above mismatch may cause the problem if the _SUN returns
the value in 64 bits.
> 
> The following patch change the definition in acpiphp.h to match the type of sun with other places.

Does HP intend to have slot numbers larger than 4 billion?  The ACPI 3.0
spec is mute on how large the 'integer' returned from _SUN is --
presumably it can be 64 bit.  But if we're going to permit _SUN to
return 64-bit, we should make it u64 everywhere, not just 64-bit for
64-bit machines.

--

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Chen, Justin | 10 Sep 02:49

RE: [PATCH] acpiphp: Match the variable types for ia64

>> On the ia64, since the "unsigned long" is u64, the above
>mismatch may cause the problem if the _SUN returns the value
>in 64 bits.
>>
>> The following patch change the definition in acpiphp.h to
>match the type of sun with other places.
>
>Does HP intend to have slot numbers larger than 4 billion?
>The ACPI 3.0 spec is mute on how large the 'integer' returned
>from _SUN is -- presumably it can be 64 bit.  But if we're
>going to permit _SUN to return 64-bit, we should make it u64
>everywhere, not just 64-bit for 64-bit machines.
>

HP has a new developing system does report the _SUN in 64 bits.  That's why the mismatch shows up.  I can try to
persuade the FW not to use the 64 bits.  But isn't this patch making more sense to have the variables in same type?

Justin

Alex Chiang | 10 Sep 09:49

Re: [PATCH] acpiphp: Match the variable types for ia64

* Matthew Wilcox <matthew <at> wil.cx>:
> On Tue, Sep 09, 2008 at 11:22:45PM +0000, Chen, Justin wrote:
> > The variable "sun" defined in acpiphp_glue.c as "unsigned long".  But the struct acpiphp_slot defines
the sun as u32 in acpiphp.h.
> > 
> > On the ia64, since the "unsigned long" is u64, the above mismatch may cause the problem if the _SUN returns
the value in 64 bits.
> > 
> > The following patch change the definition in acpiphp.h to match the type of sun with other places.
> 
> Does HP intend to have slot numbers larger than 4 billion?  

Yes.

We've got some machines where this:

	50001010206ff85

Is actually a sensible slot number. It makes sense to our
manageability software. ;)

> The ACPI 3.0 spec is mute on how large the 'integer' returned
> from _SUN is -- presumably it can be 64 bit.  But if we're
> going to permit _SUN to return 64-bit, we should make it u64
> everywhere, not just 64-bit for 64-bit machines.

Yeah, that's a better idea, let's just turn the type into a u64.

* Chen, Justin <justin.chen <at> hp.com>:
>

(Continue reading)

Jesse Barnes | 10 Sep 20:03

Re: [PATCH] acpiphp: Match the variable types for ia64

On Wednesday, September 10, 2008 12:49 am Alex Chiang wrote:
> * Matthew Wilcox <matthew <at> wil.cx>:
> > On Tue, Sep 09, 2008 at 11:22:45PM +0000, Chen, Justin wrote:
> > > The variable "sun" defined in acpiphp_glue.c as "unsigned long".  But
> > > the struct acpiphp_slot defines the sun as u32 in acpiphp.h.
> > >
> > > On the ia64, since the "unsigned long" is u64, the above mismatch may
> > > cause the problem if the _SUN returns the value in 64 bits.
> > >
> > > The following patch change the definition in acpiphp.h to match the
> > > type of sun with other places.
> >
> > Does HP intend to have slot numbers larger than 4 billion?
>
> Yes.
>
> We've got some machines where this:
>
> 	50001010206ff85
>
> Is actually a sensible slot number. It makes sense to our
> manageability software. ;)
>
> > The ACPI 3.0 spec is mute on how large the 'integer' returned
> > from _SUN is -- presumably it can be 64 bit.  But if we're
> > going to permit _SUN to return 64-bit, we should make it u64
> > everywhere, not just 64-bit for 64-bit machines.
>
> Yeah, that's a better idea, let's just turn the type into a u64.

(Continue reading)

Chen, Justin | 11 Sep 05:24

RE: [PATCH] acpiphp: Match the variable types for ia64

>-----Original Message-----
>> > The ACPI 3.0 spec is mute on how large the 'integer' returned from
>> > _SUN is -- presumably it can be 64 bit.  But if we're going to
>> > permit _SUN to return 64-bit, we should make it u64
>everywhere, not
>> > just 64-bit for 64-bit machines.
>>
>> Yeah, that's a better idea, let's just turn the type into a u64.
>
>Sounds good.  Justin can you generate a patch for this?
>

Here it goes.  The following patch changes (against 2.6.26) the type of sun into a u64.

Singed-off-by: Justin Chen <justin.chen <at> hp.com>

=========================================================================
diff -Nru a/drivers/pci/hotplug/acpiphp_core.c b/drivers/pci/hotplug/acpiphp_core.c
--- a/drivers/pci/hotplug/acpiphp_core.c        2008-07-13 14:51:29.000000000 -0700
+++ b/drivers/pci/hotplug/acpiphp_core.c        2008-09-10 19:25:19.118880324 -0700
@@ -355,7 +355,8 @@
        slot->hotplug_slot->info->cur_bus_speed = PCI_SPEED_UNKNOWN;

        acpiphp_slot->slot = slot;
-       snprintf(slot->name, sizeof(slot->name), "%u", slot->acpi_slot->sun);
+       snprintf(slot->name, sizeof(slot->name), "%lu",
+               (unsigned long) slot->acpi_slot->sun);

        retval = pci_hp_register(slot->hotplug_slot);
        if (retval) {

(Continue reading)

Matthew Wilcox | 11 Sep 12:44

Re: [PATCH] acpiphp: Match the variable types for ia64

On Thu, Sep 11, 2008 at 03:24:22AM +0000, Chen, Justin wrote:
>         acpiphp_slot->slot = slot;
> -       snprintf(slot->name, sizeof(slot->name), "%u", slot->acpi_slot->sun);
> +       snprintf(slot->name, sizeof(slot->name), "%lu",
> +               (unsigned long) slot->acpi_slot->sun);
> 

For printfs, cast to (unsigned long long) and use %llu.

>                 slot->device = device;
> -               slot->sun = sun;
> +               slot->sun = (u64) sun;

No ... the right thing to do here is make the local variable 'sun' a
u64.  adr ought to be a u32, not an unsigned long.

--

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Chen, Justin | 12 Sep 19:03

RE: [PATCH] acpiphp: Match the variable types for ia64

>-----Original Message-----
>
>>                 slot->device = device;
>> -               slot->sun = sun;
>> +               slot->sun = (u64) sun;
>
>No ... the right thing to do here is make the local variable
>'sun' a u64.  adr ought to be a u32, not an unsigned long.
>
>--

If I understand your concern right, you want to change the local variable 'sun' in the routine
register_slot() in the file acpiphp_glue.c from unsigned long to u64.  If that's the case, it will
introduce the other issue that since the 'sun' is passed as the argument 4 of the routine
acpi_evaluate_integer() and it is taking the unsigned long, what's the point to define 'sun' in u64?

Justin

Matthew Wilcox | 12 Sep 20:32

Re: [PATCH] acpiphp: Match the variable types for ia64

On Fri, Sep 12, 2008 at 05:03:50PM +0000, Chen, Justin wrote:
> >>                 slot->device = device;
> >> -               slot->sun = sun;
> >> +               slot->sun = (u64) sun;
> >
> >No ... the right thing to do here is make the local variable
> >'sun' a u64.  adr ought to be a u32, not an unsigned long.
> 
> If I understand your concern right, you want to change the local
> variable 'sun' in the routine register_slot() in the file acpiphp_glue.c
> from unsigned long to u64.  If that's the case, it will introduce the
> other issue that since the 'sun' is passed as the argument 4 of the
> routine acpi_evaluate_integer() and it is taking the unsigned long,
> what's the point to define 'sun' in u64?

Ooh, I think you've found a bug in the ACPI-CA.

Consider this:

/*
 * Acpi integer width. In ACPI version 1, integers are 32 bits.  In ACPI
 * version 2, integers are 64 bits. Note that this pertains to the ACPI integer
 * type only, not other integers used in the implementation of the ACPI CA
 * subsystem.
 */
typedef unsigned long long acpi_integer;

and compare and contrast it with this:

acpi_status

(Continue reading)

Moore, Robert | 12 Sep 21:12

RE: [PATCH] acpiphp: Match the variable types for ia64

>acpi_status
>acpi_evaluate_integer(acpi_handle handle,
>                      acpi_string pathname,
>                      struct acpi_object_list *arguments, unsigned long

This is not an ACPICA interface, it is local to Linux (drivers/acpi/utils.c)

It should be a pointer to a 64-bit number always. acpi_integer * works well.

Bob

>-----Original Message-----
>From: linux-acpi-owner <at> vger.kernel.org [mailto:linux-acpi-
>owner <at> vger.kernel.org] On Behalf Of Matthew Wilcox
>Sent: Friday, September 12, 2008 11:32 AM
>To: Chen, Justin
>Cc: Jesse Barnes; Chiang, Alexander; Accardi, Kristen C; linux-
>pci <at> vger.kernel.org; linux-acpi <at> vger.kernel.org; Brown, Len
>Subject: Re: [PATCH] acpiphp: Match the variable types for ia64
>
>On Fri, Sep 12, 2008 at 05:03:50PM +0000, Chen, Justin wrote:
>> >>                 slot->device = device;
>> >> -               slot->sun = sun;
>> >> +               slot->sun = (u64) sun;
>> >
>> >No ... the right thing to do here is make the local variable
>> >'sun' a u64.  adr ought to be a u32, not an unsigned long.
>>
>> If I understand your concern right, you want to change the local
>> variable 'sun' in the routine register_slot() in the file acpiphp_glue.c

(Continue reading)

Matthew Wilcox | 15 Sep 20:45

Re: [PATCH] acpiphp: Match the variable types for ia64

On Fri, Sep 12, 2008 at 12:12:42PM -0700, Moore, Robert wrote:
> This is not an ACPICA interface, it is local to Linux (drivers/acpi/utils.c)
> 
> It should be a pointer to a 64-bit number always. acpi_integer * works well.

Thanks, Bob.

Len, could you drop this patch into your testing?  It compiles without
warnings for me, but I'll admit to not having test-booted.

----

[PATCH] Change acpi_evaluate_integer to support 64-bit on 32-bit kernels

As of version 2.0, ACPI can return 64-bit integers.  The current
acpi_evaluate_integer only supports 64-bit integers on 64-bit platforms.
Change the argument to take a pointer to an acpi_integer so we support
64-bit integers on all platforms.

Signed-off-by: Matthew Wilcox <willy <at> linux.intel.com>

diff --git a/arch/ia64/sn/kernel/io_acpi_init.c b/arch/ia64/sn/kernel/io_acpi_init.c
index 6568942..1a8440c 100644
--- a/arch/ia64/sn/kernel/io_acpi_init.c
+++ b/arch/ia64/sn/kernel/io_acpi_init.c
@@ -232,7 +232,7 @@ exit:
 static unsigned int
 get_host_devfn(acpi_handle device_handle, acpi_handle rootbus_handle)
 {
-	unsigned long adr;

(Continue reading)

Matthew Wilcox | 24 Sep 20:55

Re: [PATCH] acpiphp: Match the variable types for ia64


Ping.  I appreciate I sent this during kernel summit, but we're all home
again now, right?

On Mon, Sep 15, 2008 at 12:45:09PM -0600, Matthew Wilcox wrote:
> On Fri, Sep 12, 2008 at 12:12:42PM -0700, Moore, Robert wrote:
> > This is not an ACPICA interface, it is local to Linux (drivers/acpi/utils.c)
> > 
> > It should be a pointer to a 64-bit number always. acpi_integer * works well.
> 
> Thanks, Bob.
> 
> Len, could you drop this patch into your testing?  It compiles without
> warnings for me, but I'll admit to not having test-booted.
> 
> ----
> 
> [PATCH] Change acpi_evaluate_integer to support 64-bit on 32-bit kernels
> 
> As of version 2.0, ACPI can return 64-bit integers.  The current
> acpi_evaluate_integer only supports 64-bit integers on 64-bit platforms.
> Change the argument to take a pointer to an acpi_integer so we support
> 64-bit integers on all platforms.
> 
> Signed-off-by: Matthew Wilcox <willy <at> linux.intel.com>
> 
> diff --git a/arch/ia64/sn/kernel/io_acpi_init.c b/arch/ia64/sn/kernel/io_acpi_init.c
> index 6568942..1a8440c 100644
> --- a/arch/ia64/sn/kernel/io_acpi_init.c
> +++ b/arch/ia64/sn/kernel/io_acpi_init.c

(Continue reading)

Len Brown | 10 Oct 08:20

Re: [PATCH] acpiphp: Match the variable types for ia64


> [PATCH] Change acpi_evaluate_integer to support 64-bit on 32-bit kernels

Matt,
I applied this patch, but first i global replaced
acpi_integer with "unsigned long long" because we
actually don't actually want that ACPICA type spreading
any more out of the ACPICA code than necessary.

Yes, it boots
It has a couple of conflicts with other pathces
in my queue, and when i sift through those
(friday), it will appear in my public test tree.

thanks,
-Len

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Return

Return to gmane.linux.kernel.pci.

Advertisement

Search Archive

Language

Change language

Options

Current view: Threads only / Showing only 20 lines / Not hiding cited text.
Change to All messages, whole messages, or hide cited text.

Post a message
NNTP Newsgroup
Classic Gmane web interface
XML

XML

RSS Feed
List Information

About Gmane