[U-Boot-Users] [PATCH 00/07 v2]: Add mpc7448hpc2 (Taiga) board support

[Zang Roy-r61911 <tie-fei.zang@freescale.com>]

Dear Wolfgang

On Fri, 2006-12-01 at 22:31, Wolfgang Denk wrote:
> In message <1164960567.6742.25.camel@localhost.localdomain> you wrote:
> > 
> > index d92f142..8a4d141 100644
> > --- a/lib_ppc/extable.c
> > +++ b/lib_ppc/extable.c
> > @@ -50,15 +50,29 @@ search_one_table(const struct exception_
> >                const struct exception_table_entry *last,
> >                unsigned long value)
> >  {
> > +     DECLARE_GLOBAL_DATA_PTR;
> > +
> >       while (first <= last) {
> >               const struct exception_table_entry *mid;
> >               long diff;
> >  
> >               mid = (last - first) / 2 + first;
> > -             diff = mid->insn - value;
> > -             if (diff == 0)
> > -                     return mid->fixup;
> > -             else if (diff < 0)
> > +             if (mid > CFG_MONITOR_BASE) { 
> > +             /* exception occurs in FLASH, before u-boot
> relocation.
> > +              * No relocation offset is needed. 
> > +              */
> > +                     diff = mid->insn - value;
> > +                     if (diff == 0)
> > +                             return mid->fixup;
> > +             } else {
> > +             /* exception occurs in RAM, after u-boot relocation. 
> > +              * A relocation offset should be added.
> > +              */
> > +                     diff = (mid->insn + gd->reloc_off) - value;
> > +                     if (diff == 0)
> > +                             return (mid->fixup + gd->reloc_off);
> > +             }
> > +             if (diff < 0)
> >                       first = mid+1;
> >               else
> >                       last = mid-1;
> 
> The problem I see  with  this  code  is  that  it  is  based  on  the
> assumption  that  CFG_MONITOR_BASE  is  greater than any RAM address.
> While this is always true so far, I would rather not rely on this.
I would not rely on this either, if there is a better way :-). Anyway,
it's better than a define. In future, there might be a board whose
exception code will occur in both RAM and Flash. A fixed macro define
can not deal with this condition, although, I do not see this kind of
ppc board in u-boot tree until now.
> 
> And I still don't understand why this change is necessary, and/or  if
> this  is  the right fix. If a fix is needed, then probably the values
> of "value" is wrong in the first place, so the fix should be  in  the
> calling routine.
> 
"The "value" is the address of exception occurring. For mpc7448hpc2
board, a tsi108/9 bridge is used. There is a hardware chip errata, when
the tsi108 pci controller has a config read operation. This operation
will induce a processor exception.
The following code does a pci config dword read.

+unsigned int __get_pci_config_dword (u32 addr)
+{
+	unsigned int retval;
+
+	__asm__ __volatile__ ("       lwbrx %0,0,%1\n"
+			     "1:     eieio\n"
+			     "2:\n"
+			     ".section .fixup,\"ax\"\n"
+			     "3:     li %0,-1\n"
+			     "       b 2b\n"
+			     ".section __ex_table,\"a\"\n"
+			     "       .align 2\n"
+			     "       .long 1b,3b\n"
+			     ".text":"=r"(retval):"r"(addr));
+
+	return (retval);
+} 

A exception will occur at address "1" ("value"), when the code executes
in RAM (after relocation). While the address 1b and 3b are assigned when
u-boot compiling. 1b and 3b are located in Flash. If the exception
occurs in Flash, everything will be OK. While, for pci config read
occurs in RAM, if I do not consider the reloc_off, I can not find the
__ex_table.

Do you think it is reasonable for the code to jump back to flash to
execute? This might ensure the exception occurring address locates in
FlASH. 

I can see the mechanism to deal with exception in u-boot is just similar
to  kernel, while kernel does not has such relocation. The same code is
OK in kernel. For u-boot, we should consider this issue, although there
is no other ppc board encounter this.

thanks!
Roy