[U-Boot] Commit 294b91a5817147d4b7f47be2ac69bac2a1f26491 broke mpc85xx

[U-Boot] Commit 294b91a5817147d4b7f47be2ac69bac2a1f26491 broke mpc85xx

[York Sun]

Simon,

I didn't notice until today the commit
294b91a5817147d4b7f47be2ac69bac2a1f26491 broke at least T4240QDS. I have
narrowed down to these two lines in common/board_r.c

initr_barrier,
        initr_malloc,

If I move these two lines below this part


#if defined(CONFIG_SYS_INIT_RAM_LOCK) && defined(CONFIG_E500)
        initr_unlock_ram_in_cache,
#endif


U-boot boots OK on T4240QDS (I can see the prompt). But if I move them
anywhere above this initr_unlock_ram_in_cache, it hangs the core when
initializing PCI. It may break other mpc85xx platforms but I didn't have
time to check more today. I haven't figured out why you have to move these
two lines up. Please take a close look.

Thanks,

York

[U-Boot] Commit 294b91a5817147d4b7f47be2ac69bac2a1f26491 broke mpc85xx

[Simon Glass]

Hi York,


On 30 September 2014 22:06, York Sun <yorksun@freescale.com> wrote:
> Simon,
>
> I didn't notice until today the commit
> 294b91a5817147d4b7f47be2ac69bac2a1f26491 broke at least T4240QDS. I have
> narrowed down to these two lines in common/board_r.c
>
> initr_barrier,
>         initr_malloc,
>
> If I move these two lines below this part
>
>
> #if defined(CONFIG_SYS_INIT_RAM_LOCK) && defined(CONFIG_E500)
>         initr_unlock_ram_in_cache,
> #endif
>
>
> U-boot boots OK on T4240QDS (I can see the prompt). But if I move them
> anywhere above this initr_unlock_ram_in_cache, it hangs the core when
> initializing PCI. It may break other mpc85xx platforms but I didn't have
> time to check more today. I haven't figured out why you have to move these
> two lines up. Please take a close look.

I could adjust this so that the ordering changes only when driver model is used.

This would be a case of putting '#ifdef CONFIG_DM' around the first
section, then repeating it later with '#ifndef CONFIG_DM'. You can see
that I did this for stdio out of an abundance of caution.

However, in the interests of supporting driver model on these
platforms I wonder if it might be possible to move the cache logic
earlier. I suspect that the unlock/invalidate should happen before
post-relocation RAM is used.

Please take a look and let me know if that might be possible.
Otherwise we'll have to go with the fallback.

BTW I don't have any of these boards. If there is one that is readily
and cheaply available, or if you have an old one you can send me, I'd
be happy to test on this and take a closer look at driver model
support.

Regards,
Simon

[U-Boot] Commit 294b91a5817147d4b7f47be2ac69bac2a1f26491 broke mpc85xx

[York Sun]

On 10/01/2014 08:11 AM, Simon Glass wrote:
> Hi York,
> 
> 
> On 30 September 2014 22:06, York Sun <yorksun@freescale.com> wrote:
>> Simon,
>>
>> I didn't notice until today the commit
>> 294b91a5817147d4b7f47be2ac69bac2a1f26491 broke at least T4240QDS. I have
>> narrowed down to these two lines in common/board_r.c
>>
>> initr_barrier,
>>         initr_malloc,
>>
>> If I move these two lines below this part
>>
>>
>> #if defined(CONFIG_SYS_INIT_RAM_LOCK) && defined(CONFIG_E500)
>>         initr_unlock_ram_in_cache,
>> #endif
>>
>>
>> U-boot boots OK on T4240QDS (I can see the prompt). But if I move them
>> anywhere above this initr_unlock_ram_in_cache, it hangs the core when
>> initializing PCI. It may break other mpc85xx platforms but I didn't have
>> time to check more today. I haven't figured out why you have to move these
>> two lines up. Please take a close look.
> 
> I could adjust this so that the ordering changes only when driver model is used.
> 
> This would be a case of putting '#ifdef CONFIG_DM' around the first
> section, then repeating it later with '#ifndef CONFIG_DM'. You can see
> that I did this for stdio out of an abundance of caution.
> 
> However, in the interests of supporting driver model on these
> platforms I wonder if it might be possible to move the cache logic
> earlier. I suspect that the unlock/invalidate should happen before
> post-relocation RAM is used.
> 
> Please take a look and let me know if that might be possible.
> Otherwise we'll have to go with the fallback.

I can change init sequence as far as I put initr_unlock_ram_in_cache before
initr_malloc, T4240QDS still boots.

I examine the code but don't understand why I have to do so. P4080DS doesn't
suffer this issue. I am going to seek some help from Scott and other in-house
experts.

> 
> BTW I don't have any of these boards. If there is one that is readily
> and cheaply available, or if you have an old one you can send me, I'd
> be happy to test on this and take a closer look at driver model
> support.

I checked but we don't have surplus. This board is relatively new and it sells
for $4000. I will keep an eye on it.

York

[U-Boot] Commit 294b91a5817147d4b7f47be2ac69bac2a1f26491 broke mpc85xx

[Simon Glass]

Hi York,

On 1 October 2014 10:27, York Sun <yorksun@freescale.com> wrote:

> On 10/01/2014 08:11 AM, Simon Glass wrote:
> > Hi York,
> >
> >
> > On 30 September 2014 22:06, York Sun <yorksun@freescale.com> wrote:
> >> Simon,
> >>
> >> I didn't notice until today the commit
> >> 294b91a5817147d4b7f47be2ac69bac2a1f26491 broke at least T4240QDS. I have
> >> narrowed down to these two lines in common/board_r.c
> >>
> >> initr_barrier,
> >>         initr_malloc,
> >>
> >> If I move these two lines below this part
> >>
> >>
> >> #if defined(CONFIG_SYS_INIT_RAM_LOCK) && defined(CONFIG_E500)
> >>         initr_unlock_ram_in_cache,
> >> #endif
> >>
> >>
> >> U-boot boots OK on T4240QDS (I can see the prompt). But if I move them
> >> anywhere above this initr_unlock_ram_in_cache, it hangs the core when
> >> initializing PCI. It may break other mpc85xx platforms but I didn't have
> >> time to check more today. I haven't figured out why you have to move
> these
> >> two lines up. Please take a close look.
> >
> > I could adjust this so that the ordering changes only when driver model
> is used.
> >
> > This would be a case of putting '#ifdef CONFIG_DM' around the first
> > section, then repeating it later with '#ifndef CONFIG_DM'. You can see
> > that I did this for stdio out of an abundance of caution.
> >
> > However, in the interests of supporting driver model on these
> > platforms I wonder if it might be possible to move the cache logic
> > earlier. I suspect that the unlock/invalidate should happen before
> > post-relocation RAM is used.
> >
> > Please take a look and let me know if that might be possible.
> > Otherwise we'll have to go with the fallback.
>
> I can change init sequence as far as I put initr_unlock_ram_in_cache before
> initr_malloc, T4240QDS still boots.
>
> I examine the code but don't understand why I have to do so. P4080DS
> doesn't
> suffer this issue. I am going to seek some help from Scott and other
> in-house
> experts.
>
> >
> > BTW I don't have any of these boards. If there is one that is readily
> > and cheaply available, or if you have an old one you can send me, I'd
> > be happy to test on this and take a closer look at driver model
> > support.
>
> I checked but we don't have surplus. This board is relatively new and it
> sells
> for $4000. I will keep an eye on it.
>

Gosh. Is there something else relatively common in the PPC world that sells
for $100-200?

Regards,
Simon

[U-Boot] Commit 294b91a5817147d4b7f47be2ac69bac2a1f26491 broke mpc85xx

[York Sun]

On 10/01/2014 09:37 AM, Simon Glass wrote:
> 
> Gosh. Is there something else relatively common in the PPC world that sells for
> $100-200?
> 

Unlikely. This is high end SoC.

York

[U-Boot] Commit 294b91a5817147d4b7f47be2ac69bac2a1f26491 broke mpc85xx

[Simon Glass]

Hi York,

On 1 October 2014 10:52, York Sun <yorksun@freescale.com> wrote:
> On 10/01/2014 09:37 AM, Simon Glass wrote:
>>
>> Gosh. Is there something else relatively common in the PPC world that sells for
>> $100-200?
>>
>
> Unlikely. This is high end SoC.

Well is there is some PPC board that is supported by U-Boot,
relatively recent? Not high-end?

Regards,
Simon

[U-Boot] Commit 294b91a5817147d4b7f47be2ac69bac2a1f26491 broke mpc85xx

[York Sun]

On 10/01/2014 11:01 AM, Simon Glass wrote:
> Hi York,
> 
> On 1 October 2014 10:52, York Sun <yorksun@freescale.com> wrote:
>> On 10/01/2014 09:37 AM, Simon Glass wrote:
>>>
>>> Gosh. Is there something else relatively common in the PPC world that sells for
>>> $100-200?
>>>
>>
>> Unlikely. This is high end SoC.
> 
> Well is there is some PPC board that is supported by U-Boot,
> relatively recent? Not high-end?
>

The most recent T1040RDB costs about $1250 according to searching online.
$100-$200 is not the range for recent mpc85xx boards.

York

[U-Boot] Commit 294b91a5817147d4b7f47be2ac69bac2a1f26491 broke mpc85xx

[Scott Wood]

On Wed, 2014-10-01 at 09:27 -0700, York Sun wrote:
> On 10/01/2014 08:11 AM, Simon Glass wrote:
> > Hi York,
> > 
> > 
> > On 30 September 2014 22:06, York Sun <yorksun@freescale.com> wrote:
> >> Simon,
> >>
> >> I didn't notice until today the commit
> >> 294b91a5817147d4b7f47be2ac69bac2a1f26491 broke at least T4240QDS. I have
> >> narrowed down to these two lines in common/board_r.c
> >>
> >> initr_barrier,
> >>         initr_malloc,
> >>
> >> If I move these two lines below this part
> >>
> >>
> >> #if defined(CONFIG_SYS_INIT_RAM_LOCK) && defined(CONFIG_E500)
> >>         initr_unlock_ram_in_cache,
> >> #endif
> >>
> >>
> >> U-boot boots OK on T4240QDS (I can see the prompt). But if I move them
> >> anywhere above this initr_unlock_ram_in_cache, it hangs the core when
> >> initializing PCI. It may break other mpc85xx platforms but I didn't have
> >> time to check more today. I haven't figured out why you have to move these
> >> two lines up. Please take a close look.
> > 
> > I could adjust this so that the ordering changes only when driver model is used.
> > 
> > This would be a case of putting '#ifdef CONFIG_DM' around the first
> > section, then repeating it later with '#ifndef CONFIG_DM'. You can see
> > that I did this for stdio out of an abundance of caution.
> > 
> > However, in the interests of supporting driver model on these
> > platforms I wonder if it might be possible to move the cache logic
> > earlier. I suspect that the unlock/invalidate should happen before
> > post-relocation RAM is used.
> > 
> > Please take a look and let me know if that might be possible.
> > Otherwise we'll have to go with the fallback.
> 
> I can change init sequence as far as I put initr_unlock_ram_in_cache before
> initr_malloc, T4240QDS still boots.

Yes, we shouldn't have any reason to keep the cache locked that long.

> I examine the code but don't understand why I have to do so. P4080DS doesn't
> suffer this issue. I am going to seek some help from Scott and other in-house
> experts.

My guess is that it has to do with the way caches work on e6500.  We try
to lock lines in L1, but L1 can't hold dirty lines, so perhaps the hang
comes when the DDR activity evicts the dirty lines from L2 (where they
weren't locked).

-Scott

[U-Boot] Commit 294b91a5817147d4b7f47be2ac69bac2a1f26491 broke mpc85xx

[York Sun]

On 10/01/2014 07:15 PM, Scott Wood wrote:
>>
>> I can change init sequence as far as I put initr_unlock_ram_in_cache before
>> initr_malloc, T4240QDS still boots.
> 
> Yes, we shouldn't have any reason to keep the cache locked that long.
> 
>> I examine the code but don't understand why I have to do so. P4080DS doesn't
>> suffer this issue. I am going to seek some help from Scott and other in-house
>> experts.
> 
> My guess is that it has to do with the way caches work on e6500.  We try
> to lock lines in L1, but L1 can't hold dirty lines, so perhaps the hang
> comes when the DDR activity evicts the dirty lines from L2 (where they
> weren't locked).
> 

Scott,

Thanks for looking into this. I can propose a fix, but first I will have to do a
thorough test on all varieties of 85xx board we have.

York