RE: Re: ACPI PCI bus mismatch ?

RE: Re: ACPI PCI bus mismatch ?

[Grover, Andrew]

> From: Alvaro Lopes [mailto:alvieboy-JkNoWtNHh5AAvxtiuMwx3w@public.gmane.org] 
> OK, I tracked the problem down to acpi_os_derive_pci_id(). If 
> I remove 
> the call to acpi_os_derive_pci_id() in 
> acpi_ev_pci_config_region_setup() 
> it works fine.
> 
> Will do some tests on 2.5.x now and let you know.

Grrreat.

We stuck that code in because some IA64 systems needed it to work.

Sounds like we need to re-think all this. A hack for right now might be
to only call derive on IA64 boxes.

Regards -- Andy



-------------------------------------------------------
This SF.NET email is sponsored by:
SourceForge Enterprise Edition + IBM + LinuxWorld = Something 2 See!
http://www.vasoftware.com

Re: Re: ACPI PCI bus mismatch ?

[Alvaro Lopes]

>
>
>>Will do some tests on 2.5.x now and let you know.
>>    
>>
Forgot to say: it works fine in 2.5.59 too. :)

-- 

Álvaro Lopes 
---------------------
A .sig is just a .sig




-------------------------------------------------------
This SF.NET email is sponsored by:
SourceForge Enterprise Edition + IBM + LinuxWorld = Something 2 See!
http://www.vasoftware.com

Re: Re: ACPI PCI bus mismatch ?

[Ducrot Bruno]

Hi!

On Thu, Feb 06, 2003 at 12:39:43PM -0800, Grover, Andrew wrote:
> > From: Alvaro Lopes [mailto:alvieboy-JkNoWtNHh5AAvxtiuMwx3w@public.gmane.org] 
> > OK, I tracked the problem down to acpi_os_derive_pci_id(). If 
> > I remove 
> > the call to acpi_os_derive_pci_id() in 
> > acpi_ev_pci_config_region_setup() 
> > it works fine.
> > 
> > Will do some tests on 2.5.x now and let you know.
> 
> Grrreat.
> 
> We stuck that code in because some IA64 systems needed it to work.
> 
> Sounds like we need to re-think all this. A hack for right now might be
> to only call derive on IA64 boxes.


It is KO for me.

I sticked:
printk(KERN_DEBUG "BEFORE: 0x%x 0x%x 0x%x 0x%x\n", pci_id->segment, pci_id->bus,
                                                   pci_id->device, pci_id->function);
acpi_os_derive_pci_id(...);
printk(KERN_DEBUG "AFTER: 0x%x 0x%x 0x%x 0x%x\n", pci_id->segment, pci_id->bus,
                                                   pci_id->device, pci_id->function);

in drivers/acpi/events/evrgnini.c::ev_pci_config_region_setup()

and it show me that pci_id->bus is *always* 0 at BEFORE, and will be
correct in AFTER (tested by creating 2 OPs, in fact).

Cheers,

-- 
Ducrot Bruno

--  Which is worse:  ignorance or apathy?
--  Don't know.  Don't care.


-------------------------------------------------------
This SF.NET email is sponsored by:
SourceForge Enterprise Edition + IBM + LinuxWorld = Something 2 See!
http://www.vasoftware.com

Re: Re: ACPI PCI bus mismatch ?

[Alvaro Lopes]

Ducrot Bruno wrote:

>Hi!
>
>On Thu, Feb 06, 2003 at 12:39:43PM -0800, Grover, Andrew wrote:
>  
>
>>>From: Alvaro Lopes [mailto:alvieboy-JkNoWtNHh5AAvxtiuMwx3w@public.gmane.org] 
>>>OK, I tracked the problem down to acpi_os_derive_pci_id(). If 
>>>I remove 
>>>the call to acpi_os_derive_pci_id() in 
>>>acpi_ev_pci_config_region_setup() 
>>>it works fine.
>>>
>>>Will do some tests on 2.5.x now and let you know.
>>>      
>>>
>>Grrreat.
>>
>>We stuck that code in because some IA64 systems needed it to work.
>>
>>Sounds like we need to re-think all this. A hack for right now might be
>>to only call derive on IA64 boxes.
>>    
>>
>
>
>It is KO for me.
>
>I sticked:
>printk(KERN_DEBUG "BEFORE: 0x%x 0x%x 0x%x 0x%x\n", pci_id->segment, pci_id->bus,
>                                                   pci_id->device, pci_id->function);
>acpi_os_derive_pci_id(...);
>printk(KERN_DEBUG "AFTER: 0x%x 0x%x 0x%x 0x%x\n", pci_id->segment, pci_id->bus,
>                                                   pci_id->device, pci_id->function);
>
>in drivers/acpi/events/evrgnini.c::ev_pci_config_region_setup()
>
>and it show me that pci_id->bus is *always* 0 at BEFORE, and will be
>correct in AFTER (tested by creating 2 OPs, in fact).
>  
>
Perhaps a stupid question, but:

If we are reading the busID for the brigde itself, shouldn't we use the 
"Primary bus number" @ 0x18, instead of "Secondary bus number" @ 0x19 ?

Álvaro
-- 

Álvaro Lopes 
---------------------
A .sig is just a .sig




-------------------------------------------------------
This SF.NET email is sponsored by:
SourceForge Enterprise Edition + IBM + LinuxWorld = Something 2 See!
http://www.vasoftware.com

Re: Re: ACPI PCI bus mismatch ?

[Ducrot Bruno]

[-- Attachment #1: Type: text/plain, Size: 2000 bytes --]

Hi Alvaro,

On Mon, Feb 10, 2003 at 06:13:03PM +0000, Alvaro Lopes wrote:
> Ducrot Bruno wrote:
> 
> >Hi!
> >
> >On Thu, Feb 06, 2003 at 12:39:43PM -0800, Grover, Andrew wrote:
> > 
> >
> >>>From: Alvaro Lopes [mailto:alvieboy-JkNoWtNHh5AAvxtiuMwx3w@public.gmane.org] 
> >>>OK, I tracked the problem down to acpi_os_derive_pci_id(). If 
> >>>I remove 
> >>>the call to acpi_os_derive_pci_id() in 
> >>>acpi_ev_pci_config_region_setup() 
> >>>it works fine.
> >>>
> >>>Will do some tests on 2.5.x now and let you know.
> >>>     
> >>>
> >>Grrreat.
> >>
> >>We stuck that code in because some IA64 systems needed it to work.
> >>
> >>Sounds like we need to re-think all this. A hack for right now might be
> >>to only call derive on IA64 boxes.
> >>   
> >>
> >
> >
> >It is KO for me.
> >
> >I sticked:
> >printk(KERN_DEBUG "BEFORE: 0x%x 0x%x 0x%x 0x%x\n", pci_id->segment, 
> >pci_id->bus,
> >                                                  pci_id->device, 
> >                                                  pci_id->function);
> >acpi_os_derive_pci_id(...);
> >printk(KERN_DEBUG "AFTER: 0x%x 0x%x 0x%x 0x%x\n", pci_id->segment, 
> >pci_id->bus,
> >                                                  pci_id->device, 
> >                                                  pci_id->function);
> >
> >in drivers/acpi/events/evrgnini.c::ev_pci_config_region_setup()
> >
> >and it show me that pci_id->bus is *always* 0 at BEFORE, and will be
> >correct in AFTER (tested by creating 2 OPs, in fact).
> > 
> >
> Perhaps a stupid question, but:
> 
> If we are reading the busID for the brigde itself, shouldn't we use the 
> "Primary bus number" @ 0x18, instead of "Secondary bus number" @ 0x19 ?
> 

Could you please try this patch (for 2.5 series) ?

The first chunk is rejected for 2.4 though, but it is trivial
to do it by hand.

(hem, it is ugly, I know, there is for now no simple solution to
do it)

Cheers,

-- 
Ducrot Bruno

--  Which is worse:  ignorance or apathy?
--  Don't know.  Don't care.

[-- Attachment #2: acpi_derive_pci_id_fix-2.5.diff --]
[-- Type: text/plain, Size: 2201 bytes --]

--- linux-2.5/drivers/acpi/osl.c	2003/02/10 23:43:24	1.1
+++ linux-2.5/drivers/acpi/osl.c	2003/02/11 17:46:39
@@ -502,11 +504,17 @@
 }
 
 /* TODO: Change code to take advantage of driver model more */
-void
-acpi_os_derive_pci_id (
+/*
+ * Some DSDT try to access OP really early (_STA).
+ * What if PCI subsystem is not initialized by firmware at this stage?
+ */
+static void
+acpi_os_derive_pci_id_2 (
 	acpi_handle		rhandle,        /* upper bound  */
 	acpi_handle		chandle,        /* current node */
-	struct acpi_pci_id	**id)
+	struct acpi_pci_id	**id,
+	int			*is_bridge,
+	u8			*bus_number)
 {
 	acpi_handle		handle;
 	struct acpi_pci_id	*pci_id = *id;
@@ -517,7 +525,7 @@
 
 	acpi_get_parent(chandle, &handle);
 	if (handle != rhandle) {
-		acpi_os_derive_pci_id(rhandle, handle, &pci_id);
+		acpi_os_derive_pci_id_2(rhandle, handle, &pci_id, is_bridge, bus_number);
 
 		status = acpi_get_type(handle, &type);
 		if ( (ACPI_FAILURE(status)) || (type != ACPI_TYPE_DEVICE) )
@@ -528,15 +536,41 @@
 			pci_id->device  = ACPI_HIWORD (ACPI_LODWORD (temp));
 			pci_id->function = ACPI_LOWORD (ACPI_LODWORD (temp));
 
+			if (*is_bridge)
+				pci_id->bus = *bus_number;
+
 			/* any nicer way to get bus number of bridge ? */
 			status = acpi_os_read_pci_configuration(pci_id, 0x0e, &tu8, 8);
-			if (ACPI_SUCCESS(status) && (tu8 & 0x7f) == 1) {
+			if (ACPI_SUCCESS(status) &&
+			    ((tu8 & 0x7f) == 1 || (tu8 & 0x7f) == 2)) {
+				status = acpi_os_read_pci_configuration(pci_id, 0x18, &tu8, 8);
+				if (!ACPI_SUCCESS(status)) {
+					return;
+				}
+				*is_bridge = 1;
+				pci_id->bus = tu8;
 				status = acpi_os_read_pci_configuration(pci_id, 0x19, &tu8, 8);
-				if (ACPI_SUCCESS(status))
-					pci_id->bus = tu8;
+				if (ACPI_SUCCESS(status)) {
+					*bus_number = tu8;
+				}
+			} else {
+				if (!ACPI_SUCCESS(status))
+				*is_bridge = 0;
 			}
 		}
 	}
+}
+
+void
+acpi_os_derive_pci_id (
+	acpi_handle		rhandle,        /* upper bound  */
+	acpi_handle		chandle,        /* current node */
+	struct acpi_pci_id	**id)
+{
+	int is_bridge = 1;
+	u8 bus_number = (*id)->bus;
+
+	acpi_os_derive_pci_id_2(rhandle, chandle, id, &is_bridge, &bus_number);
 }
 
 #else /*!CONFIG_ACPI_PCI*/

Re: Re: ACPI PCI bus mismatch ?

[Ducrot Bruno]

On Tue, Feb 11, 2003 at 06:55:12PM +0100, Ducrot Bruno wrote:
> Hi Alvaro,
> 
> On Mon, Feb 10, 2003 at 06:13:03PM +0000, Alvaro Lopes wrote:
> > Ducrot Bruno wrote:
> > 
> > >Hi!
> > >
> > >On Thu, Feb 06, 2003 at 12:39:43PM -0800, Grover, Andrew wrote:
> > > 
> > >
> > >>>From: Alvaro Lopes [mailto:alvieboy-JkNoWtNHh5AAvxtiuMwx3w@public.gmane.org] 
> > >>>OK, I tracked the problem down to acpi_os_derive_pci_id(). If 
> > >>>I remove 
> > >>>the call to acpi_os_derive_pci_id() in 
> > >>>acpi_ev_pci_config_region_setup() 
> > >>>it works fine.
> > >>>
> > >>>Will do some tests on 2.5.x now and let you know.
> > >>>     
> > >>>
> > >>Grrreat.
> > >>
> > >>We stuck that code in because some IA64 systems needed it to work.
> > >>
> > >>Sounds like we need to re-think all this. A hack for right now might be
> > >>to only call derive on IA64 boxes.
> > >>   
> > >>
> > >
> > >
> > >It is KO for me.
> > >
> > >I sticked:
> > >printk(KERN_DEBUG "BEFORE: 0x%x 0x%x 0x%x 0x%x\n", pci_id->segment, 
> > >pci_id->bus,
> > >                                                  pci_id->device, 
> > >                                                  pci_id->function);
> > >acpi_os_derive_pci_id(...);
> > >printk(KERN_DEBUG "AFTER: 0x%x 0x%x 0x%x 0x%x\n", pci_id->segment, 
> > >pci_id->bus,
> > >                                                  pci_id->device, 
> > >                                                  pci_id->function);
> > >
> > >in drivers/acpi/events/evrgnini.c::ev_pci_config_region_setup()
> > >
> > >and it show me that pci_id->bus is *always* 0 at BEFORE, and will be
> > >correct in AFTER (tested by creating 2 OPs, in fact).
> > > 
> > >
> > Perhaps a stupid question, but:
> > 
> > If we are reading the busID for the brigde itself, shouldn't we use the 
> > "Primary bus number" @ 0x18, instead of "Secondary bus number" @ 0x19 ?
> > 
> 
> Could you please try this patch (for 2.5 series) ?
> 
> The first chunk is rejected for 2.4 though, but it is trivial
> to do it by hand.
> 
> (hem, it is ugly, I know, there is for now no simple solution to
> do it)
> 
> Cheers,
> 
> -- 
> Ducrot Bruno
> 
> --  Which is worse:  ignorance or apathy?
> --  Don't know.  Don't care.

> --- linux-2.5/drivers/acpi/osl.c	2003/02/10 23:43:24	1.1
> +++ linux-2.5/drivers/acpi/osl.c	2003/02/11 17:46:39
> @@ -502,11 +504,17 @@
>  }
>  
>  /* TODO: Change code to take advantage of driver model more */
> -void
> -acpi_os_derive_pci_id (
> +/*
> + * Some DSDT try to access OP really early (_STA).
> + * What if PCI subsystem is not initialized by firmware at this stage?
> + */
> +static void
> +acpi_os_derive_pci_id_2 (
>  	acpi_handle		rhandle,        /* upper bound  */
>  	acpi_handle		chandle,        /* current node */
> -	struct acpi_pci_id	**id)
> +	struct acpi_pci_id	**id,
> +	int			*is_bridge,
> +	u8			*bus_number)
>  {
>  	acpi_handle		handle;
>  	struct acpi_pci_id	*pci_id = *id;
> @@ -517,7 +525,7 @@
>  
>  	acpi_get_parent(chandle, &handle);
>  	if (handle != rhandle) {
> -		acpi_os_derive_pci_id(rhandle, handle, &pci_id);
> +		acpi_os_derive_pci_id_2(rhandle, handle, &pci_id, is_bridge, bus_number);
>  
>  		status = acpi_get_type(handle, &type);
>  		if ( (ACPI_FAILURE(status)) || (type != ACPI_TYPE_DEVICE) )
> @@ -528,15 +536,41 @@
>  			pci_id->device  = ACPI_HIWORD (ACPI_LODWORD (temp));
>  			pci_id->function = ACPI_LOWORD (ACPI_LODWORD (temp));
>  
> +			if (*is_bridge)
> +				pci_id->bus = *bus_number;
> +
>  			/* any nicer way to get bus number of bridge ? */
>  			status = acpi_os_read_pci_configuration(pci_id, 0x0e, &tu8, 8);
> -			if (ACPI_SUCCESS(status) && (tu8 & 0x7f) == 1) {
> +			if (ACPI_SUCCESS(status) &&
> +			    ((tu8 & 0x7f) == 1 || (tu8 & 0x7f) == 2)) {
> +				status = acpi_os_read_pci_configuration(pci_id, 0x18, &tu8, 8);
> +				if (!ACPI_SUCCESS(status)) {
> +					return;
> +				}
> +				*is_bridge = 1;
> +				pci_id->bus = tu8;
>  				status = acpi_os_read_pci_configuration(pci_id, 0x19, &tu8, 8);
> -				if (ACPI_SUCCESS(status))
> -					pci_id->bus = tu8;
> +				if (ACPI_SUCCESS(status)) {
> +					*bus_number = tu8;
> +				}
> +			} else {
> +				if (!ACPI_SUCCESS(status))
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^
			     Grumpf.  kill this line.  I have removed
			     some printk debug statements, and this line has survived..


> +				*is_bridge = 0;
>  			}
>  		}
>  	}
> +}
> +
> +void
> +acpi_os_derive_pci_id (
> +	acpi_handle		rhandle,        /* upper bound  */
> +	acpi_handle		chandle,        /* current node */
> +	struct acpi_pci_id	**id)
> +{
> +	int is_bridge = 1;
> +	u8 bus_number = (*id)->bus;
> +
> +	acpi_os_derive_pci_id_2(rhandle, chandle, id, &is_bridge, &bus_number);
>  }
>  
>  #else /*!CONFIG_ACPI_PCI*/


-- 
Ducrot Bruno

--  Which is worse:  ignorance or apathy?
--  Don't know.  Don't care.


-------------------------------------------------------
This SF.NET email is sponsored by:
SourceForge Enterprise Edition + IBM + LinuxWorld = Something 2 See!
http://www.vasoftware.com