[U-Boot-Users] PCI stopped working on MPC8343

[U-Boot-Users] PCI stopped working on MPC8343

[Andre Schwarz]

Last week my PCI bus has been running fine showing all devices.
Right now no devices are shown on the bus.

I'm using CONFIG_83XX_GENERIC_PCI with common setup code for PCI.

Nothing changed from my side during the last 2 weeks.


Did I miss any changes in u-boot ?


regards,
Andre Schwarz
Matrix Vision



MATRIX VISION GmbH, Talstra?e 16, DE-71570 Oppenweiler  - Registergericht: Amtsgericht Stuttgart, HRB 271090
Gesch?ftsf?hrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner

[U-Boot-Users] PCI stopped working on MPC8343

[Larry Johnson]

Andre Schwarz wrote:
> Last week 
> my PCI bus has been running fine showing all devices.
> Right now no devices are shown on the bus.
> 
> I'm using CONFIG_83XX_GENERIC_PCI with common setup code for PCI.
> 
> Nothing changed from my side during the last 2 weeks.
> 
> 
> Did I miss any changes in u-boot ?
> 
> 
> regards,
> Andre Schwarz
> Matrix Vision

Hi Andre and everyone,

I was just about to post on I problem I belatedly found with PCI on our
Korat PPC400EPx board.  (I believe the same problem exists on the
Sequoia board, but have not proved it.)  What appears to have broken it
was the following patch (I got to use "git bisect" for the first time):

<patch>
commit 55774b512fdf63c0516d441cc5da7c54bbffb7f2
Author: Nobuhiro Iwamatsu <iwamatsu@nigauri.org>
Date:   Fri Mar 7 16:04:25 2008 +0900

    pci: Add CONFIG_PCI_SKIP_HOST_BRIDGE config option
    
    In current source code, when the device number of PCI is 0, process PCI
    bridge without fail. However, when the device number is 0, it is not PCI
    always bridge. There are times when device of PCI allocates.
    
    When CONFIG_PCI_SKIP_HOST_BRIDGE is enable, this problem is solved when
    use this patch.
    
    Signed-off-by: Nobuhiro Iwamatsu <iwamatsu@nigauri.org>
    Acked-by: Stefan Roese <sr@denx.de>

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 50ca6b0..7944b66 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -425,6 +425,9 @@ int pci_hose_scan_bus(struct pci_controller *hose, int bus)
 	     dev <  PCI_BDF(bus,PCI_MAX_PCI_DEVICES-1,PCI_MAX_PCI_FUNCTIONS-1);
 	     dev += PCI_BDF(0,0,1))
 	{
+
+	/* Bus 0 is not necessarily PCI bridge. */
+#if defined(CONFIG_PCI_SKIP_HOST_BRIDGE)
 		/* Skip our host bridge */
 		if ( dev == PCI_BDF(hose->first_busno,0,0) ) {
 #if defined(CONFIG_PCI_CONFIG_HOST_BRIDGE)              /* don't skip host bridge */
@@ -434,10 +437,11 @@ int pci_hose_scan_bus(struct pci_controller *hose, int bus)
 			if (getenv("pciconfighost") == NULL) {
 				continue; /* Skip our host bridge */
 			}
-#else
+#else /* CONFIG_PCI_CONFIG_HOST_BRIDGE */
 			continue; /* Skip our host bridge */
-#endif
+#endif /* CONFIG_PCI_CONFIG_HOST_BRIDGE */
 		}
+#endif /* CONFIG_PCI_SKIP_HOST_BRIDGE */
 
 		if (PCI_FUNC(dev) && !found_multi)
 			continue;
@@ -473,8 +477,11 @@ int pci_hose_scan_bus(struct pci_controller *hose, int bus)
 				hose->fixup_irq(hose, dev);
 
 #ifdef CONFIG_PCI_SCAN_SHOW
+#if defined(CONFIG_PCI_SKIP_HOST_BRIDGE)
 			/* Skip our host bridge */
-			if ( dev != PCI_BDF(hose->first_busno,0,0) ) {
+			if ( dev != PCI_BDF(hose->first_busno,0,0) )
+#endif
+			{
 			    unsigned char int_line;
 
 			    pci_hose_read_config_byte(hose, dev, PCI_INTERRUPT_LINE,

</patch>

The patch changes the behavior of the code _unless_
CONFIG_PCI_SKIP_HOST_BRIDGE is defined.  Defining that switch in the
Korat configuration fixes the problem.  The MPC8323ERDB configuration
has also been patched to add the switch, so maybe this is is the cause
of your problem as well.

I was going to submit the patch for Korat, but then decided I'd first
like to ask Nobuhiro and the group whether it might not be better to
change the occurrences of

#if defined(CONFIG_PCI_SKIP_HOST_BRIDGE)

in "drivers/pci/pci.c" to

#if !defined(CONFIG_PCI_NO_SKIP_HOST_BRIDGE)

That would allow those boards that need to patch to specify it
explicity, while leaving the default functionality unchanged.

Comments?

Best regards,
Larry

[U-Boot-Users] PCI stopped working on MPC8343

[André Schwarz]

Larry,

you're right - but it doesn't fix my problem.


Thanks,
Andr?


Larry Johnson schrieb:
> Andre Schwarz wrote:
>> Last week my PCI bus has been running fine showing all devices.
>> Right now no devices are shown on the bus.
>>
>> I'm using CONFIG_83XX_GENERIC_PCI with common setup code for PCI.
>>
>> Nothing changed from my side during the last 2 weeks.
>>
>>
>> Did I miss any changes in u-boot ?
>>
>>
>> regards,
>> Andre Schwarz
>> Matrix Vision
> 
> Hi Andre and everyone,
> 
> I was just about to post on I problem I belatedly found with PCI on our
> Korat PPC400EPx board.  (I believe the same problem exists on the
> Sequoia board, but have not proved it.)  What appears to have broken it
> was the following patch (I got to use "git bisect" for the first time):
> 
> <patch>
> commit 55774b512fdf63c0516d441cc5da7c54bbffb7f2
> Author: Nobuhiro Iwamatsu <iwamatsu@nigauri.org>
> Date:   Fri Mar 7 16:04:25 2008 +0900
> 
>    pci: Add CONFIG_PCI_SKIP_HOST_BRIDGE config option
>       In current source code, when the device number of PCI is 0, 
> process PCI
>    bridge without fail. However, when the device number is 0, it is not PCI
>    always bridge. There are times when device of PCI allocates.
>       When CONFIG_PCI_SKIP_HOST_BRIDGE is enable, this problem is solved 
> when
>    use this patch.
>       Signed-off-by: Nobuhiro Iwamatsu <iwamatsu@nigauri.org>
>    Acked-by: Stefan Roese <sr@denx.de>
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 50ca6b0..7944b66 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -425,6 +425,9 @@ int pci_hose_scan_bus(struct pci_controller *hose, 
> int bus)
>          dev <  PCI_BDF(bus,PCI_MAX_PCI_DEVICES-1,PCI_MAX_PCI_FUNCTIONS-1);
>          dev += PCI_BDF(0,0,1))
>     {
> +
> +    /* Bus 0 is not necessarily PCI bridge. */
> +#if defined(CONFIG_PCI_SKIP_HOST_BRIDGE)
>         /* Skip our host bridge */
>         if ( dev == PCI_BDF(hose->first_busno,0,0) ) {
> #if defined(CONFIG_PCI_CONFIG_HOST_BRIDGE)              /* don't skip 
> host bridge */
> @@ -434,10 +437,11 @@ int pci_hose_scan_bus(struct pci_controller *hose, 
> int bus)
>             if (getenv("pciconfighost") == NULL) {
>                 continue; /* Skip our host bridge */
>             }
> -#else
> +#else /* CONFIG_PCI_CONFIG_HOST_BRIDGE */
>             continue; /* Skip our host bridge */
> -#endif
> +#endif /* CONFIG_PCI_CONFIG_HOST_BRIDGE */
>         }
> +#endif /* CONFIG_PCI_SKIP_HOST_BRIDGE */
> 
>         if (PCI_FUNC(dev) && !found_multi)
>             continue;
> @@ -473,8 +477,11 @@ int pci_hose_scan_bus(struct pci_controller *hose, 
> int bus)
>                 hose->fixup_irq(hose, dev);
> 
> #ifdef CONFIG_PCI_SCAN_SHOW
> +#if defined(CONFIG_PCI_SKIP_HOST_BRIDGE)
>             /* Skip our host bridge */
> -            if ( dev != PCI_BDF(hose->first_busno,0,0) ) {
> +            if ( dev != PCI_BDF(hose->first_busno,0,0) )
> +#endif
> +            {
>                 unsigned char int_line;
> 
>                 pci_hose_read_config_byte(hose, dev, PCI_INTERRUPT_LINE,
> 
> </patch>
> 
> The patch changes the behavior of the code _unless_
> CONFIG_PCI_SKIP_HOST_BRIDGE is defined.  Defining that switch in the
> Korat configuration fixes the problem.  The MPC8323ERDB configuration
> has also been patched to add the switch, so maybe this is is the cause
> of your problem as well.
> 
> I was going to submit the patch for Korat, but then decided I'd first
> like to ask Nobuhiro and the group whether it might not be better to
> change the occurrences of
> 
> #if defined(CONFIG_PCI_SKIP_HOST_BRIDGE)
> 
> in "drivers/pci/pci.c" to
> 
> #if !defined(CONFIG_PCI_NO_SKIP_HOST_BRIDGE)
> 
> That would allow those boards that need to patch to specify it
> explicity, while leaving the default functionality unchanged.
> 
> Comments?
> 
> Best regards,
> Larry
> 


MATRIX VISION GmbH, Talstra?e 16, DE-71570 Oppenweiler  - Registergericht: Amtsgericht Stuttgart, HRB 271090
Gesch?ftsf?hrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner

[U-Boot-Users] PCI stopped working on MPC8343

[Stefan Roese]

On Friday 18 April 2008, Larry Johnson wrote:
> I was just about to post on I problem I belatedly found with PCI on our
> Korat PPC400EPx board.  (I believe the same problem exists on the
> Sequoia board, but have not proved it.)  What appears to have broken it
> was the following patch (I got to use "git bisect" for the first time):

> <patch>
> commit 55774b512fdf63c0516d441cc5da7c54bbffb7f2
> Author: Nobuhiro Iwamatsu <iwamatsu@nigauri.org>
> Date:   Fri Mar 7 16:04:25 2008 +0900
>
>     pci: Add CONFIG_PCI_SKIP_HOST_BRIDGE config option

<snip>

> The patch changes the behavior of the code _unless_
> CONFIG_PCI_SKIP_HOST_BRIDGE is defined.  Defining that switch in the
> Korat configuration fixes the problem.  The MPC8323ERDB configuration
> has also been patched to add the switch, so maybe this is is the cause
> of your problem as well.
>
> I was going to submit the patch for Korat, but then decided I'd first
> like to ask Nobuhiro and the group whether it might not be better to
> change the occurrences of
>
> #if defined(CONFIG_PCI_SKIP_HOST_BRIDGE)
>
> in "drivers/pci/pci.c" to
>
> #if !defined(CONFIG_PCI_NO_SKIP_HOST_BRIDGE)
>
> That would allow those boards that need to patch to specify it
> explicity, while leaving the default functionality unchanged.
>
> Comments?

Yes, I totally agree. Patches should *not* change the default behavior. We 
should change it as you suggested. Sorry, I must have missed this.

Nobuhiro, could you please send a new patch to fix this problem?

Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot-Users] PCI stopped working on MPC8343

[Larry Johnson]

Andr? Schwarz wrote:
> Larry,
> 
> you're right - but it doesn't fix my problem.
> 
> 
> Thanks,
> Andr?
> 
> 
> Larry Johnson schrieb:
>> Andre Schwarz wrote:
>>> Last week my PCI bus has been running fine showing all devices.
>>> Right now no devices are shown on the bus.
>>>
>>> I'm using CONFIG_83XX_GENERIC_PCI with common setup code for PCI.
>>>
>>> Nothing changed from my side during the last 2 weeks.
>>>
>>>
>>> Did I miss any changes in u-boot ?
>>>
>>>
>>> regards,
>>> Andre Schwarz
>>> Matrix Vision
>>
>> Hi Andre and everyone,
>>
>> I was just about to post on I problem I belatedly found with PCI on our
>> Korat PPC400EPx board.  (I believe the same problem exists on the
>> Sequoia board, but have not proved it.)  What appears to have broken it
>> was the following patch (I got to use "git bisect" for the first time):
>>
>> <snip>

I'm sorry to hear that. (And it was such a nice theory!)

Best regards,
Larry

[U-Boot-Users] [PATCH][RFC] pci: Remove CONFIG_PCI_SKIP_HOST_BRIDGE and Add check PCI class of host bridge (Re: PCI stopped working on MPC8343)

[Nobuhiro Iwamatsu]

Hi, all.

Sorry for the delay.

On Fri, 18 Apr 2008 16:57:15 +0200
Stefan Roese <sr@denx.de> wrote:

> > The patch changes the behavior of the code _unless_
> > CONFIG_PCI_SKIP_HOST_BRIDGE is defined.  Defining that switch in the
> > Korat configuration fixes the problem.  The MPC8323ERDB configuration
> > has also been patched to add the switch, so maybe this is is the cause
> > of your problem as well.
> >
> > I was going to submit the patch for Korat, but then decided I'd first
> > like to ask Nobuhiro and the group whether it might not be better to
> > change the occurrences of
> >
> > #if defined(CONFIG_PCI_SKIP_HOST_BRIDGE)
> >
> > in "drivers/pci/pci.c" to
> >
> > #if !defined(CONFIG_PCI_NO_SKIP_HOST_BRIDGE)
> >
> > That would allow those boards that need to patch to specify it
> > explicity, while leaving the default functionality unchanged.
> >
> > Comments?
> 
> Yes, I totally agree. Patches should *not* change the default behavior. We 
> should change it as you suggested. Sorry, I must have missed this.
> 
> Nobuhiro, could you please send a new patch to fix this problem?

Thank you for your check.
It has troubled you with my patch. 

I rewirte new patch . Could you please check this patch and confirming the work?

Best regards,
 Nobuhiro 
-- 
Nobuhiro Iwamatsu

---
Remove CONFIG_PCI_SKIP_HOST_BRIDGE from drivers/pci/pci.c.
Add check PCI class of host bridge(CPU to PCI bridge).

Signed-off-by: Nobuhiro Iwamatsu <iwamatsu@nigauri.org>
---
 drivers/pci/pci.c |   20 ++++++++------------
 1 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 7944b66..129a6c9 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -425,14 +425,15 @@ int pci_hose_scan_bus(struct pci_controller *hose, int bus)
 	     dev <  PCI_BDF(bus,PCI_MAX_PCI_DEVICES-1,PCI_MAX_PCI_FUNCTIONS-1);
 	     dev += PCI_BDF(0,0,1))
 	{
-
-	/* Bus 0 is not necessarily PCI bridge. */
-#if defined(CONFIG_PCI_SKIP_HOST_BRIDGE)
+		/* Read class register */
+		pci_hose_read_config_word(hose, dev, PCI_CLASS_DEVICE, &class);
 		/* Skip our host bridge */
-		if ( dev == PCI_BDF(hose->first_busno,0,0) ) {
-#if defined(CONFIG_PCI_CONFIG_HOST_BRIDGE)              /* don't skip host bridge */
+		if ( (dev == PCI_BDF(hose->first_busno,0,0)) &&
+			(class == 0x0600)) { /* CPU to PCI bridge */
+#if defined(CONFIG_PCI_CONFIG_HOST_BRIDGE) /* don't skip host bridge */
 			/*
-			 * Only skip hostbridge configuration if "pciconfighost" is not set
+			 * Only skip hostbridge configuration if 
+			 * "pciconfighost" is not set
 			 */
 			if (getenv("pciconfighost") == NULL) {
 				continue; /* Skip our host bridge */
@@ -441,8 +442,6 @@ int pci_hose_scan_bus(struct pci_controller *hose, int bus)
 			continue; /* Skip our host bridge */
 #endif /* CONFIG_PCI_CONFIG_HOST_BRIDGE */
 		}
-#endif /* CONFIG_PCI_SKIP_HOST_BRIDGE */
-
 		if (PCI_FUNC(dev) && !found_multi)
 			continue;
 
@@ -477,11 +476,8 @@ int pci_hose_scan_bus(struct pci_controller *hose, int bus)
 				hose->fixup_irq(hose, dev);
 
 #ifdef CONFIG_PCI_SCAN_SHOW
-#if defined(CONFIG_PCI_SKIP_HOST_BRIDGE)
 			/* Skip our host bridge */
-			if ( dev != PCI_BDF(hose->first_busno,0,0) )
-#endif
-			{
+			if( class != 0x0600 ) { /* CPU to PCI bridge */
 			    unsigned char int_line;
 
 			    pci_hose_read_config_byte(hose, dev, PCI_INTERRUPT_LINE,
-- 
1.5.5

[U-Boot-Users] [PATCH][RFC] pci: Remove CONFIG_PCI_SKIP_HOST_BRIDGE and Add check PCI class of host bridge

[Larry Johnson]

Nobuhiro Iwamatsu wrote:
> Hi, all.
> 
> Sorry for the delay.
> 
> On Fri, 18 Apr 2008 16:57:15 +0200
> Stefan Roese <sr@denx.de> wrote:
> 
>>> The patch changes the behavior of the code _unless_
>>> CONFIG_PCI_SKIP_HOST_BRIDGE is defined.  Defining that switch in the
>>> Korat configuration fixes the problem.  The MPC8323ERDB configuration
>>> has also been patched to add the switch, so maybe this is is the cause
>>> of your problem as well.
>>>
>>> I was going to submit the patch for Korat, but then decided I'd first
>>> like to ask Nobuhiro and the group whether it might not be better to
>>> change the occurrences of
>>>
>>> #if defined(CONFIG_PCI_SKIP_HOST_BRIDGE)
>>>
>>> in "drivers/pci/pci.c" to
>>>
>>> #if !defined(CONFIG_PCI_NO_SKIP_HOST_BRIDGE)
>>>
>>> That would allow those boards that need to patch to specify it
>>> explicity, while leaving the default functionality unchanged.
>>>
>>> Comments?
>> Yes, I totally agree. Patches should *not* change the default behavior. We 
>> should change it as you suggested. Sorry, I must have missed this.
>>
>> Nobuhiro, could you please send a new patch to fix this problem?
> 
> Thank you for your check.
> It has troubled you with my patch. 
> 
> I rewirte new patch . Could you please check this patch and confirming the work?
> 
> Best regards,
>  Nobuhiro 

Hi Nobuhiro, Stefan, and everyone,

Sorry for the delay in getting back to you.  My mail reader thinks your 
patch is part of your signature, so I missed it the first time around.

I did a quick check using the Sequoia board, and it looks like the patch 
isn't working there.  U-Boot is reporting the class of the bridge as 
"0680", which I assume is why the patch doesn't skip it.

Nobuhiro and Stefan, do you think we can get a working fix for this into 
1.3.3 (even if it is not the final fix)?  If not, we should patch 
Sequoia and Korat to add CONFIG_PCI_SKIP_HOST_BRIDGE so they won't be 
broken in that release.

Best regards,
Larry

[U-Boot-Users] [PATCH][RFC] pci: Remove CONFIG_PCI_SKIP_HOST_BRIDGE and Add check PCI class of host bridge

[Stefan Roese]

On Monday 05 May 2008, Larry Johnson wrote:
> >> Yes, I totally agree. Patches should *not* change the default behavior.
> >> We should change it as you suggested. Sorry, I must have missed this.
> >>
> >> Nobuhiro, could you please send a new patch to fix this problem?
> >
> > Thank you for your check.
> > It has troubled you with my patch.
> >
> > I rewirte new patch . Could you please check this patch and confirming
> > the work?
> >
> > Best regards,
> >  Nobuhiro
>
> Hi Nobuhiro, Stefan, and everyone,
>
> Sorry for the delay in getting back to you.  My mail reader thinks your
> patch is part of your signature, so I missed it the first time around.
>
> I did a quick check using the Sequoia board, and it looks like the patch
> isn't working there.  U-Boot is reporting the class of the bridge as
> "0680", which I assume is why the patch doesn't skip it.

Yes, I just gave the new patch a try and noticed (still) a problem on Sequoia. 
The PCI host bridge (PPC) is listed upon bootup which was not the case with 
the U-Boot version prior to the first patch.

> Nobuhiro and Stefan, do you think we can get a working fix for this into
> 1.3.3 (even if it is not the final fix)?

I definitely hope so. Even though I will probably not find the time to submit 
a patch to really fix this problem till the scheduled release date. But I 
will gladly test a patch. :)

> If not, we should patch 
> Sequoia and Korat to add CONFIG_PCI_SKIP_HOST_BRIDGE so they won't be
> broken in that release.

Or perhaps we should revert the first patch and give it another try in the 
next merge-window when/if a new patch is available.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot-Users] [PATCH][RFC] pci: Remove CONFIG_PCI_SKIP_HOST_BRIDGE and Add check PCI class of host bridge

[Anton Vorontsov]

On Mon, May 05, 2008 at 08:41:59PM +0200, Stefan Roese wrote:
> On Monday 05 May 2008, Larry Johnson wrote:
> > >> Yes, I totally agree. Patches should *not* change the default behavior.
> > >> We should change it as you suggested. Sorry, I must have missed this.
> > >>
> > >> Nobuhiro, could you please send a new patch to fix this problem?
> > >
> > > Thank you for your check.
> > > It has troubled you with my patch.
> > >
> > > I rewirte new patch . Could you please check this patch and confirming
> > > the work?
> > >
> > > Best regards,
> > >  Nobuhiro
> >
> > Hi Nobuhiro, Stefan, and everyone,
> >
> > Sorry for the delay in getting back to you.  My mail reader thinks your
> > patch is part of your signature, so I missed it the first time around.
> >
> > I did a quick check using the Sequoia board, and it looks like the patch
> > isn't working there.  U-Boot is reporting the class of the bridge as
> > "0680", which I assume is why the patch doesn't skip it.
> 
> Yes, I just gave the new patch a try and noticed (still) a problem on Sequoia. 
> The PCI host bridge (PPC) is listed upon bootup which was not the case with 
> the U-Boot version prior to the first patch.
> 
> > Nobuhiro and Stefan, do you think we can get a working fix for this into
> > 1.3.3 (even if it is not the final fix)?
> 
> I definitely hope so. Even though I will probably not find the time to submit 
> a patch to really fix this problem till the scheduled release date. But I 
> will gladly test a patch. :)
> 
> > If not, we should patch 
> > Sequoia and Korat to add CONFIG_PCI_SKIP_HOST_BRIDGE so they won't be
> > broken in that release.
> 
> Or perhaps we should revert the first patch and give it another try in the 
> next merge-window when/if a new patch is available.

Yes, please. That patch also broke probably all Freescale PowerPC boards.
Particularly, without #define CONFIG_PCI_SKIP_HOST_BRIDGE I'm getting
these errors from the Linux using PCI 3com driver:

eth4: Host error, FIFO diagnostic register 0000.
eth4: PCI bus error, bus status 80000020
eth4:  setting full-duplex.
[infinite times]

This is MPC8315E-RDB.

-- 
Anton Vorontsov
email: cbouatmailru at gmail.com
irc://irc.freenode.net/bd2

[U-Boot-Users] [PATCH][RFC] pci: Remove CONFIG_PCI_SKIP_HOST_BRIDGE and Add check PCI class of host bridge

[Larry Johnson]

Anton Vorontsov wrote:
> On Mon, May 05, 2008 at 08:41:59PM +0200, Stefan Roese wrote:
>> On Monday 05 May 2008, Larry Johnson wrote:
>>>>> Yes, I totally agree. Patches should *not* change the default behavior.
>>>>> We should change it as you suggested. Sorry, I must have missed this.
>>>>>
>>>>> Nobuhiro, could you please send a new patch to fix this problem?
>>>> Thank you for your check.
>>>> It has troubled you with my patch.
>>>>
>>>> I rewirte new patch . Could you please check this patch and confirming
>>>> the work?
>>>>
>>>> Best regards,
>>>>  Nobuhiro
>>> Hi Nobuhiro, Stefan, and everyone,
>>>
>>> Sorry for the delay in getting back to you.  My mail reader thinks your
>>> patch is part of your signature, so I missed it the first time around.
>>>
>>> I did a quick check using the Sequoia board, and it looks like the patch
>>> isn't working there.  U-Boot is reporting the class of the bridge as
>>> "0680", which I assume is why the patch doesn't skip it.
>> Yes, I just gave the new patch a try and noticed (still) a problem on Sequoia. 
>> The PCI host bridge (PPC) is listed upon bootup which was not the case with 
>> the U-Boot version prior to the first patch.
>>
>>> Nobuhiro and Stefan, do you think we can get a working fix for this into
>>> 1.3.3 (even if it is not the final fix)?
>> I definitely hope so. Even though I will probably not find the time to submit 
>> a patch to really fix this problem till the scheduled release date. But I 
>> will gladly test a patch. :)
>>
>>> If not, we should patch 
>>> Sequoia and Korat to add CONFIG_PCI_SKIP_HOST_BRIDGE so they won't be
>>> broken in that release.
>> Or perhaps we should revert the first patch and give it another try in the 
>> next merge-window when/if a new patch is available.
> 
> Yes, please. That patch also broke probably all Freescale PowerPC boards.
> Particularly, without #define CONFIG_PCI_SKIP_HOST_BRIDGE I'm getting
> these errors from the Linux using PCI 3com driver:
> 
> eth4: Host error, FIFO diagnostic register 0000.
> eth4: PCI bus error, bus status 80000020
> eth4:  setting full-duplex.
> [infinite times]
> 
> This is MPC8315E-RDB.

At this point, I'm in favor of reverting the patch for this release and 
letting Nobuhiro put in a temporary board-specific patch for those 
boards that need the modified functionality.

The if we do a non-board-specific patch for the next window, we won't be 
rushed to make sure it doesn't break any boards.

Best regards,
Larry

[U-Boot-Users] [PATCH][RFC] pci: Remove CONFIG_PCI_SKIP_HOST_BRIDGE and Add check PCI class of host bridge

[Wolfgang Denk]

In message <20080507163121.GA15435@polina.dev.rtsoft.ru>
Anton Vorontsov wrote:
>
> On Mon, May 05, 2008 at 08:41:59PM +0200, Stefan Roese wrote:
...
> > Or perhaps we should revert the first patch and give it another try in the 
> > next merge-window when/if a new patch is available.
> 
> Yes, please. That patch also broke probably all Freescale PowerPC boards.
> Particularly, without #define CONFIG_PCI_SKIP_HOST_BRIDGE I'm getting
> these errors from the Linux using PCI 3com driver:

I now reverted that patch.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Brain fried - Core dumped

[U-Boot-Users] [PATCH][RFC] pci: Remove CONFIG_PCI_SKIP_HOST_BRIDGE and Add check PCI class of host bridge

[Nobuhiro Iwamatsu]

On Mon, 05 May 2008 11:34:20 -0400
Larry Johnson <lrj@acm.org> wrote:

> 
> Hi Nobuhiro, Stefan, and everyone,
> 
> Sorry for the delay in getting back to you.  My mail reader thinks your 
> patch is part of your signature, so I missed it the first time around.
> 
> I did a quick check using the Sequoia board, and it looks like the patch 
> isn't working there.  U-Boot is reporting the class of the bridge as 
> "0680", which I assume is why the patch doesn't skip it.
OK. I understood.

> Nobuhiro and Stefan, do you think we can get a working fix for this into 
> 1.3.3 (even if it is not the final fix)?  If not, we should patch 
> Sequoia and Korat to add CONFIG_PCI_SKIP_HOST_BRIDGE so they won't be 
> broken in that release.

Yes, I do not want to release the one that I broke, too. 

I made the patch that applied your comment. 
This patch skips PCI bridge ( Class code : 0x06XX). 

Could you check this patch?

Best regards,
 Nobuhiro

-- 
Nobuhiro Iwamatsu
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-pci-Remove-CONFIG_PCI_SKIP_HOST_BRIDGE-and-Add-chec.patch
Type: text/x-diff
Size: 2356 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080512/af59bf59/attachment.patch 

[U-Boot-Users] [PATCH][RFC] pci: Remove CONFIG_PCI_SKIP_HOST_BRIDGE and Add check PCI class of host bridge

[Wolfgang Denk]

In message <20080512224352.a3823c7a.iwamatsu@nigauri.org> you wrote:
> 
> I made the patch that applied your comment. 
> This patch skips PCI bridge ( Class code : 0x06XX). 
> 
> Could you check this patch?

Thanks - but note that this will not make it in the v1.3.3 release in
any case.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
There is is no reason for any individual to have a computer in  their
home.      -- Ken Olsen (President of Digital Equipment Corporation),
              Convention of the World Future Society, in Boston, 1977

[U-Boot-Users] [PATCH][RFC] pci: Remove CONFIG_PCI_SKIP_HOST_BRIDGE and Add check PCI class of host bridge

[Nobuhiro Iwamatsu]

Hi, all.

2008/5/13 Wolfgang Denk <wd@denx.de>:
> In message <20080512224352.a3823c7a.iwamatsu@nigauri.org> you wrote:
>>
>> I made the patch that applied your comment.
>> This patch skips PCI bridge ( Class code : 0x06XX).
>>
>> Could you check this patch?
>
> Thanks - but note that this will not make it in the v1.3.3 release in
> any case.

Does the developer and Board maintainer get a problem with this patch?
Would you test this patch?

Best regards,
 Nobuhiro
-- 
----------------------------------
Nobuhiro Iwamatsu

[U-Boot-Users] [PATCH][RFC] pci: Add check PCI bridge class (Re: [PATCH][RFC] pci: Remove CONFIG_PCI_SKIP_HOST_BRIDGE and Add check PCI class of host bridge)

[Nobuhiro Iwamatsu]

Nobuhiro Iwamatsu wrote:
> Hi, all.
> 
> 2008/5/13 Wolfgang Denk <wd@denx.de>:
>> In message <20080512224352.a3823c7a.iwamatsu@nigauri.org> you wrote:
>>> I made the patch that applied your comment.
>>> This patch skips PCI bridge ( Class code : 0x06XX).
>>>
>>> Could you check this patch?
>> Thanks - but note that this will not make it in the v1.3.3 release in
>> any case.
> 
> Does the developer and Board maintainer get a problem with this patch?
> Would you test this patch?

Sorry , I forgot attached patch.

Best regards,
  Nobuhiro
---
In current source code, when the device number of PCI is 0, process PCI
bridge without fail. However, when the device number is 0, it is not PCI
always bridge. There are times when device of PCI allocates.

This patch add check PCI bridge class.

  - 0x0604 PCI to AGP bridge / PCI to PCI bridge
  - 0x0605 PCI to PCMCIA bridge
  - 0x0606 Nu Bus bridge
  - 0x0607 PCMCIA CardBus controller
  - 0x0608 RACEWay bridge

Signed-off-by: Nobuhiro Iwamatsu <iwamatsu.nobuhiro@renesas.com>
---
  drivers/pci/pci.c |   23 +++++++++++++++--------
  1 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b3ae3b1..39ca130 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -425,14 +425,21 @@ int pci_hose_scan_bus(struct pci_controller *hose, int bus)
  	     dev <  PCI_BDF(bus,PCI_MAX_PCI_DEVICES-1,PCI_MAX_PCI_FUNCTIONS-1);
  	     dev += PCI_BDF(0,0,1))
  	{
+		/* Read class register */
+		pci_hose_read_config_word(hose, dev, PCI_CLASS_DEVICE, &class);
  		/* Skip our host bridge */
-		if ( dev == PCI_BDF(hose->first_busno,0,0) ) {
-#if defined(CONFIG_PCI_CONFIG_HOST_BRIDGE)              /* don't skip host bridge */
-			/*
-			 * Only skip hostbridge configuration if "pciconfighost" is not set
-			 */
-			if (getenv("pciconfighost") == NULL) {
-				continue; /* Skip our host bridge */
+		if (dev == PCI_BDF(hose->first_busno,0,0)) {
+			/* CPU to PCI bridge check */
+			if (((class & 0xFF00) == 0x0600) && (class != 0x0680))
+#if defined(CONFIG_PCI_CONFIG_HOST_BRIDGE) /* don't skip host bridge */
+			{
+				/*
+				 * Only skip hostbridge configuration if
+				 * "pciconfighost" is not set
+				 */
+				if (getenv("pciconfighost") == NULL) {
+					continue; /* Skip our host bridge */
+				}
  			}
  #else
  			continue; /* Skip our host bridge */
@@ -474,7 +481,7 @@ int pci_hose_scan_bus(struct pci_controller *hose, int bus)

  #ifdef CONFIG_PCI_SCAN_SHOW
  			/* Skip our host bridge */
-			if ( dev != PCI_BDF(hose->first_busno,0,0) ) {
+			if (((class & 0xFF00) != 0x0600) || (class == 0x0680)) {
  			    unsigned char int_line;

  			    pci_hose_read_config_byte(hose, dev, PCI_INTERRUPT_LINE,
-- 1.5.5.1

[U-Boot-Users] [PATCH][RFC] pci: Remove CONFIG_PCI_SKIP_HOST_BRIDGE and Add check PCI class of host bridge

[Wolfgang Denk]

Hi Stefan,

in message <bc6f41e00806162159hc396149sf11ac0e2497a6b13@mail.gmail.com>
Nobuhiro Iwamatsu wrote:
> 
> 2008/5/13 Wolfgang Denk <wd@denx.de>:
> > In message <20080512224352.a3823c7a.iwamatsu@nigauri.org> you wrote:
> >>
> >> I made the patch that applied your comment.
> >> This patch skips PCI bridge ( Class code : 0x06XX).
> >>
> >> Could you check this patch?
> >
> > Thanks - but note that this will not make it in the v1.3.3 release in
> > any case.
> 
> Does the developer and Board maintainer get a problem with this patch?
> Would you test this patch?

Do you have any comments on this? Does it now work on Sequoia et al?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I believe you find life such a problem because you  think  there  are
the  good  people  and the bad people. You're wrong, of course. There
are, always and only, the bad people, but some of them are  on  oppo-
site sides.                      - Terry Pratchett, _Guards! Guards!_

[U-Boot-Users] [PATCH][RFC] pci: Add check PCI bridge class (Re: [PATCH][RFC] pci: Remove CONFIG_PCI_SKIP_HOST_BRIDGE and Add check PCI class of host bridge)

[Stefan Roese]

Hi Nobuhiro,

sorry about the late reply.

On Tuesday 17 June 2008, Nobuhiro Iwamatsu wrote:
> In current source code, when the device number of PCI is 0, process PCI
> bridge without fail. However, when the device number is 0, it is not PCI
> always bridge. There are times when device of PCI allocates.
>
> This patch add check PCI bridge class.
>
>   - 0x0604 PCI to AGP bridge / PCI to PCI bridge
>   - 0x0605 PCI to PCMCIA bridge
>   - 0x0606 Nu Bus bridge
>   - 0x0607 PCMCIA CardBus controller
>   - 0x0608 RACEWay bridge

I now finally took the time to take a closer look at this issue. Your approach 
doesn't look "clean" to me. To be honsest, the original code isn't "clean" 
either. But with your addition it really get confusing. At least for me.

So let me summarize what the current code (without your patch) does and 
perhaps you could write again what exactly you want to fix/change. And please 
correct me if I am wrong.

Currently, the configuration of the PCI device at BDF == 0,0,0 will be skipped 
if:

a) CONFIG_PCI_CONFIG_HOST_BRIDGE is not defined

or

b) CONFIG_PCI_CONFIG_HOST_BRIDGE is defined but the env variable
   "pciconfighost" is not defined

So this PCI device will get configured if:

c) CONFIG_PCI_CONFIG_HOST_BRIDGE is defined and the env variable
   "pciconfighost" is defined too

If I understand you correctly, then you have some PCI devices at BDF == 0,0,0 
that need to be configured (and not skipped). Correct? If this is correct, 
wouldn't it be enough for you to use "solution" c) from above?

If this is not the case, then please explain again exactly what you need to 
achieve.

Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot-Users] [PATCH][RFC] pci: Add check PCI bridge class (Re: [PATCH][RFC] pci: Remove CONFIG_PCI_SKIP_HOST_BRIDGE and Add check PCI class of host bridge)

[Nobuhiro Iwamatsu]

Hi, Stefan.

2008/7/7 Stefan Roese <sr@denx.de>:
- ???????????? -
> Hi Nobuhiro,
>
> sorry about the late reply.
>
> On Tuesday 17 June 2008, Nobuhiro Iwamatsu wrote:
>> In current source code, when the device number of PCI is 0, process PCI
>> bridge without fail. However, when the device number is 0, it is not PCI
>> always bridge. There are times when device of PCI allocates.
>>
>> This patch add check PCI bridge class.
>>
>>   - 0x0604 PCI to AGP bridge / PCI to PCI bridge
>>   - 0x0605 PCI to PCMCIA bridge
>>   - 0x0606 Nu Bus bridge
>>   - 0x0607 PCMCIA CardBus controller
>>   - 0x0608 RACEWay bridge
>
> I now finally took the time to take a closer look at this issue. Your approach
> doesn't look "clean" to me. To be honsest, the original code isn't "clean"
> either. But with your addition it really get confusing. At least for me.
>
> So let me summarize what the current code (without your patch) does and
> perhaps you could write again what exactly you want to fix/change. And please
> correct me if I am wrong.
>
> Currently, the configuration of the PCI device at BDF == 0,0,0 will be skipped
> if:
>
> a) CONFIG_PCI_CONFIG_HOST_BRIDGE is not defined
>
> or
>
> b) CONFIG_PCI_CONFIG_HOST_BRIDGE is defined but the env variable
>   "pciconfighost" is not defined
>
> So this PCI device will get configured if:
>
> c) CONFIG_PCI_CONFIG_HOST_BRIDGE is defined and the env variable
>   "pciconfighost" is defined too
>
> If I understand you correctly, then you have some PCI devices at BDF == 0,0,0
> that need to be configured (and not skipped). Correct? If this is correct,
> wouldn't it be enough for you to use "solution" c) from above?
>
Yes, Your indication is right. And there was the part which I did not
understand.
But, I think that there is still a problem only by these measures.

For example, BDF == 0,0,0 are not displayed when I validated
CONFIG_PCI_SCAN_SHOW.

475 #ifdef CONFIG_PCI_SCAN_SHOW
476             /* Skip our host bridge */
477             if ( dev != PCI_BDF(hose->first_busno,0,0) ) {
478                 unsigned char int_line;
479
480                 pci_hose_read_config_byte(hose, dev, PCI_INTERRUPT_LINE,
481                               &int_line);
482                 printf("        %02x  %02x  %04x  %04x  %04x  %02x\n",
483                    PCI_BUS(dev), PCI_DEV(dev), vendor, device, class,
484                    int_line);
485             }
486 #endif

I think that it is a problem to handle only  "BDF == 0,0,0".
I think that I had better check it in Class ID.
?Of cource, I understand that my implementation is mean.?

> If this is not the case, then please explain again exactly what you need to
> achieve.

Best regards,
 Nobuhiro

[U-Boot-Users] [PATCH][RFC] pci: Add check PCI bridge class (Re: [PATCH][RFC] pci: Remove CONFIG_PCI_SKIP_HOST_BRIDGE and Add check PCI class of host bridge)

[Stefan Roese]

Hi Nobuhiro,

On Tuesday 08 July 2008, Nobuhiro Iwamatsu wrote:
> > c) CONFIG_PCI_CONFIG_HOST_BRIDGE is defined and the env variable
> >   "pciconfighost" is defined too
> >
> > If I understand you correctly, then you have some PCI devices at BDF ==
> > 0,0,0 that need to be configured (and not skipped). Correct? If this is
> > correct, wouldn't it be enough for you to use "solution" c) from above?
>
> Yes, Your indication is right. And there was the part which I did not
> understand.
> But, I think that there is still a problem only by these measures.
>
> For example, BDF == 0,0,0 are not displayed when I validated
> CONFIG_PCI_SCAN_SHOW.

Right, this is a problem.

> 475 #ifdef CONFIG_PCI_SCAN_SHOW
> 476             /* Skip our host bridge */
> 477             if ( dev != PCI_BDF(hose->first_busno,0,0) ) {
> 478                 unsigned char int_line;
> 479
> 480                 pci_hose_read_config_byte(hose, dev,
> PCI_INTERRUPT_LINE, 481                               &int_line);
> 482                 printf("        %02x  %02x  %04x  %04x  %04x  %02x\n",
> 483                    PCI_BUS(dev), PCI_DEV(dev), vendor, device, class,
> 484                    int_line);
> 485             }
> 486 #endif
>
> I think that it is a problem to handle only  "BDF == 0,0,0".
> I think that I had better check it in Class ID.
> ?Of cource, I understand that my implementation is mean.?

Yes, switching to this class-ID check would be probably "cleaner". The main 
problem I see with this is, that we could break backward compatibility. I 
really don't know if all boards currently using this "Host-Bridge-Skipping" 
implementation have the class-ID set to 0x0680 (Other bridge type).

So I suggest the following solution: I'll prepare a patch to move 
this "Skip-device-check" into a separate weak function with the current 
behavior. You (and others) can then define a custom "Skip-device-check" 
function in your platform code that overrides this default function.

I'll send this patch to the list in a short while. Please try it out and let 
me know if this works for you.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================