Subject: Re: Alchemy Au15XX PCI diffs
To: Matt Thomas <matt@3am-software.com>
From: Garrett D'Amore <garrett_damore@tadpole.com>
List: port-mips
Date: 01/24/2006 16:01:47
Matt Thomas wrote:
> Garrett D'Amore wrote:
>   
>> Everyone,
>>
>> I've posted my latest diffs for Alchemy PCI support (and probably a few
>> other Alchemy fixes as well -- I didn't want to split them all out --
>> kind of painful to do that) into a diff file at
>>
>>     http://garrett.damore.org/software/netbsd/aupci.diff
>>
>> Constructive criticism is welcomed.  Flames to /dev/null.
>>     
>
> Use const more.  For instance, obiodev_t arrays should be const.
> In machdep.c, cpus[] should be const and make name [8] instead of
> a pointer.
>   
Good suggestions.

Out of curiosity, is there some benefit to declaring name[8] other than
the slight storage improvement -- perhaps 4 pointers in this case, but
I'd also think this would be offset slightly by the fact that the
strings have to be aligned and same sized whereas with a pointer and
strings stored off in text they can be packed.   (In this case it is a
win to use name[8], but if you had strings of very different sizes, the
other way with packed strings could be more space efficient.)

> be consistent.  either use obiodev_t everywhere or struct obiodev
> but don't mix them.
>   
Point taken.  I probably switched to obiodev_t midstream, but also some
of this could be legacy coming from pb1000_obiodev.  (I'd have to check
to find out.)  I'll check to see if there are any other instances of
this kind of inconsistency.
> Add file-system TMPFS to the config files.
>   
Is TMPFS robust enough that it should be included by default now?  I
wasn't totally clear on that bit yet.  Or do you just want it listed
commented out so that it can be easily be added later?  (Btw, TMPFS is
*not* in the original Alchemy PB1000 config file.  I doubt it is in the
other evbmips port -- Malta -- config file either.)
> Are the interrupts evcnt'ed?
>   
At the top-level, yes.  But the PCI should probably also evcnt them. 
There is a comment in the PCI driver (aupb.c) about that.  (Not sure how
useful evcnt at the PCI level is anyway... they are already reported at
the levels above and below, where they are likely to be a lot more
useful for debug, etc.)  In fact, corrected evcnt reporting at the
interrupt controller is one of the major "side" improvements that is in
this patch.

By the way, in no way do I intend this code to be "perfect".  So
comments like these are great.   I  hope though  that I can go ahead and
commit the code at some point even if I have not implemented every
possible feature (e.g. the evcnt stuff).  I can always flesh out and
improve the stuff later, especially as it gets more use by a wider
audience.  Right now these changes represent major missing functionality
for the Alchemy port, and I'd rather get them committed (and hence
exercised) sooner rather than later.  Now that we're starting a new
cycle on 4.x it seems like an ideal time to do this.

-- 
Garrett D'Amore                          http://www.tadpolecomputer.com/
Sr. Staff Engineer          Extending the Power of 64-bit UNIX Computing
Tadpole Computer, Inc.                             Phone: (951) 325-2134