In article <200601281113.12131.garrett_damore@tadpole.com>
garrett_damore@tadpole.com wrote:

> I figured it best to minimize arbitrary changes.

Ok.

BTW, it's better to add config files to src/etc/etc.evbmips/Makefile.inc
so that autobuild could find any MD fallout.

> > > +options	_MIPS_PADDR_T_64BIT
> >
> > This should be in <machine/types.h> or each config file?
 :
> As far as I can tell, userland binaries are not impacted.  At least I'm able 
> to use most of the usual tools that grovel kmem -- ps, top, netstat, vmstat, 
> etc.

Unless it's defined in <machine/types.h>, I guess LKM would have
compatibility problem.

I think most modern embedded mips CPUs have R4K style MMU,
it's OK to add #define _MIPS_PADDR_T_64BIT into
evbmips/include/types.h like arc port.

> Even if I switch to a case table, I still need to have #ifdef's around each 
> case, as the necessary processor support routines may not be included in the 
> config file.

Hmm, IMHO, *_match() function should not do any initialization.
Could it be a simple compromise to rename *_match() functions
to *_init() or something?

> Anyway, I'll make the conversion if there is sufficient belief that I should 
> do so, but right now my first preference is to leave it as is.

Yeah, you are right :-)

> > all __volatile has been changed to volatile recently?
> 
> I noticed that it had.  I guessed this is to make us more strictly C99 
> conformant?

perry changed most of them recently.

> > > +#define	AU_WIRED_EXTENT_SZ	EXTENT_FIXED_STORAGE_SIZE(10)
 :
> > This value could be smaller. The static strage is only required
> > by devices mapped before VM is initialized, i.e. only some
> > console devices do. During cpu_configure(9) (after MD cpu_startup())
> > we could pass EX_MALLOCOK to extent(9).
> 
> Frankly, this is the result of my not being entirely confident in a few 
> things:
 :

arch/arc/arc/bus_space.c:arc_bus_space_malloc_safe() might help?
I think we can use malloc(9) after uvm_init() in kern/init_main.c:main(),
so you can change flags for extent(9) in cpu_startup().

> Anyway, for now I'd prefer to leave it alone, and then possibly change it 
> later after a commit if we are confident that it is safe to do so.

No problem :-)

> > > +#if defined(__MIPSEB__)
> >
> > Isn't it better to use "#if _BYTE_ORDER == BIG_ENDIAN"?
> 
> Possibly.  The code will only ever be used in mips, so it probably doesn't 
> matter, other than to suit taste.

Most (all?) other sources use _BYTE_ORDER, so maybe it's better.

> > "_chipdep" is needed for these files?
> > Just au1xx0.c and au1[015][05]0.c look fine for me.
> 
> I prefered the longer name for two (admittedly weak) reasons:
> 
> 	1) it reflects that these files really are specializations of chipdep
> 	2) it avoids possible filename conflict in the build system later.

There is no strict rules, but alpha and pmax use dec_3100.c etc.,
arc uses c_nec_pci.c (chipset?) and p_nec_jc94.c (platform) etc.,
and prep uses ibm_6050.c etc. But if any namespace conficts are
expected in the future, it's no problem to leave them.

> OK, so I want to get a little more consensus, if possible, unless you feel 
> confident in saying it is safe for me to commit.

Well, you could ask:
"If there is no objection I'll commit the diff in next week"
or something like this ;-)

Anyway, all changes in your patch only affect MD evbmips and alchemy
files (BTW any other ports which use MI mips bus_space?) and
there is no API change exported to MI part, so it's safe to commit.
Any style issue could be fixed later, though (bad) API can't
be changed easily because of possible compatibility problem.
---
Izumi Tsutsui