Re: Commit 1.20 ofw_machdep.c breaks PowerMac G5 in NetBSD HEAD

[Phileas Fogg <phileas-fogg%mail.ru@localhost>]

On 04/04/2013 07:13 PM, Michael wrote:
> Hello,
>
> On Thu, 04 Apr 2013 16:58:11 +0200
> Phileas Fogg <phileas-fogg%mail.ru@localhost> wrote:
>
> > On 04/04/2013 03:31 AM, Michael wrote:
> >
> > > What's in your /memory properties? Especially "available" and "reg"?
> > > Also, what's in / "#size-cells" and "#address-cells" ?
>
> > #address-cells          00000002
> > #size-cells             00000001
> Ok, 64bit addresses, 32bit sizes - just like mine.
>
> > reg                     00000000 00000000  00000000
> >                           00000000 00000000  00000000
> >                           00000000 00000000  40000000
> >                           00000000 00000000  00000000
> >                           00000000 00000000  00000000
> >                           00000000 00000000  00000000
> >                           00000000 40000000  40000000
> >                           00000001 00000000  40000000
>
> Hmm, a few zero entries but they should be ignored.
>
> > available               pseudo=00003000 7f5ed000
> ... and this should not trip the code below...
>
> > > Finally, does it work if you remove this:
> > >           if (((regcnt >> 2) % (acells + scells)) != 0) {
> > >                   aprint_normal("messed up 'available' property detected\n");
> > >                   acells = 1;
> > >           }
> > > ... and just set acells to 1 there?
> >
> > It didn't help.
>
> Yeah, it would screw up if "available" had a size of a multiple of 3 which 
> yours doesn't ( and neither does mine ).
> There should be some debug output, like listing the memory regions found. Does 
> that look sane?
>
> Wait a minute.
>
> Does this help?
>
> Index: powerpc.h
> ===================================================================
> RCS file: /cvsroot/src/sys/arch/macppc/include/powerpc.h,v
> retrieving revision 1.7
> diff -u -w -r1.7 powerpc.h
> --- powerpc.h   14 Mar 2009 14:46:02 -0000      1.7
> +++ powerpc.h   4 Apr 2013 17:12:12 -0000
> @@ -34,7 +34,7 @@
>   #define        _MACHINE_POWERPC_H_
>
>   struct mem_region {
> -#if defined (PMAC_G5)
> +#if defined (PMAC_G5___)
>          paddr_t start_hi;
>   #endif
>          paddr_t start;
>
> start_hi isn't used anywhere as fas as I can tell, and shouldn't be
> used in a 32bit kernel anyway. And on LP64 paddr_t would be 64bit so no
> need for an additional field there.
>
> have fun
> Michael

I found the problem.
Any kind of printf in mem_regions causes this problem.
I removed all DEBUG printfs and aprints and it works now.
But no idea why, i'm using rascons as the boot cons.

One more question.

for (i=0, cnt=0; i <= numregs; i++) {

shouldn't it be

for (i=0, cnt=0; i < numregs; i++) {


Off-by-one ?

regards