Garrett D'Amore | 25 Jan 2006 00:04

Alchemy Au15XX PCI diffs

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

The diffs are about 7K lines, and are done against -current updated last
night.  They appear to work properly (at least I'm able to use a RealTek
gigE card without problem).

There appears to be at least two debugging lines still lurking in the
source, and I'll remove those before final commit to CVS.

I would love to have someone review and (for those with hardware) test
these changes.

These changes introduce three kernel configs in evbmips:

    * ALCHEMY - generic Au1550 support (no PCI support) -- same as the
old PB1000 file basically.

    * ZINFANDEL - Au1500 development board support

    * CABERNET - Au1550 development board support

I have removed the old PB1000 file.

Configs for other boards/hardware can be created if someone wants to
(Continue reading)

Matt Thomas | 25 Jan 2006 00:33

Re: Alchemy Au15XX PCI diffs

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.

be consistent.  either use obiodev_t everywhere or struct obiodev
but don't mix them.

Add file-system TMPFS to the config files.

Are the interrupts evcnt'ed?
--

-- 
Matt Thomas                     email: matt <at> 3am-software.com
3am Software Foundry              www: http://3am-software.com/bio/matt/
Cupertino, CA              disclaimer: I avow all knowledge of this message.

Garrett D'Amore | 25 Jan 2006 01:01

Re: Alchemy Au15XX PCI diffs

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.
(Continue reading)

Garrett D'Amore | 27 Jan 2006 02:42

Re: Alchemy Au15XX PCI diffs

On Tuesday 24 January 2006 3:33 pm, 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.

I've made this change.

>
> be consistent.  either use obiodev_t everywhere or struct obiodev
> but don't mix them.

Looks like I started to convert to obiodev_t, but didn't bother to change all 
of the orginal code from which this was derived.  I've completed the change 
now.

>
> Add file-system TMPFS to the config files.

Hmmm... upon inspection, a lot of ports do not include this by default (e.g. 
(Continue reading)

Izumi Tsutsui | 28 Jan 2006 17:57
Picon
Gravatar

Re: Alchemy Au15XX PCI diffs

In article <43D6B26E.1070409 <at> tadpole.com>
garrett_damore <at> tadpole.com wrote:

>     http://garrett.damore.org/software/netbsd/aupci.diff
 :
> Constructive criticism is welcomed.  Flames to /dev/null.

Mostly it looks fine for me, but I'd like some more cosmetics:

> +extern void board_init(void);

I think function declarations don't need "extern".

> +	return (cabernet_obio);

No parentheses are needed around the return value, as share/misc/style.
(though many other sources have them)

> +evbmips_iointr(u_int32_t status, u_int32_t cause, u_int32_t pc,
> +    u_int32_t ipending)

uintXX_t is better rather than u_intXX_t.

> +# CPU support
> +options		ALCHEMY_AU1000

Use "options<space><tab>".

> +options 	DIAGNOSTIC	# extra kernel sanity checking

(Continue reading)

Garrett D'Amore | 28 Jan 2006 20:13

Re: Alchemy Au15XX PCI diffs

Thank you for the review comments.  My responses below.

On Saturday 28 January 2006 8:57 am, Izumi Tsutsui wrote:
> In article <43D6B26E.1070409 <at> tadpole.com>
>
> garrett_damore <at> tadpole.com wrote:
> >     http://garrett.damore.org/software/netbsd/aupci.diff
> >
> > Constructive criticism is welcomed.  Flames to /dev/null.
>
> Mostly it looks fine for me, but I'd like some more cosmetics:
> > +extern void board_init(void);
>
> I think function declarations don't need "extern".

True.  Not sure what the style guide says here, but I'm happy enough to remove 
them.

>
> > +	return (cabernet_obio);
>
> No parentheses are needed around the return value, as share/misc/style.
> (though many other sources have them)

True.  Old habits die hard I guess.  (The Solaris coding style requires them.  
It is one of the very few ways in which the Sun KNF style differs from 
BSD's.)  I guess I missed this one.

>
> > +evbmips_iointr(u_int32_t status, u_int32_t cause, u_int32_t pc,
(Continue reading)

Izumi Tsutsui | 30 Jan 2006 11:52
Picon
Gravatar

Re: Alchemy Au15XX PCI diffs

In article <200601281113.12131.garrett_damore <at> tadpole.com>
garrett_damore <at> 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.
(Continue reading)

Garrett D'Amore | 30 Jan 2006 19:07

Re: Alchemy Au15XX PCI diffs

On Monday 30 January 2006 2:52 am, you wrote:
> In article <200601281113.12131.garrett_damore <at> tadpole.com>
>
> garrett_damore <at> 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.

I had asked this question to SimonB earlier, and at the time he suggested that 
I not do this.  I of course would prefer to have it as you suggest, because 
it avoids having paddr_t be different for different compiles within evbmips.

(Continue reading)

Garrett D'Amore | 30 Jan 2006 21:00

64-bit paddr_t (again, arrgh....)


As Izumi-san recently pointed out, there is a problem with my current approach 
of conditionalizing the size of paddr_t on 64-bit platforms.  The problem is 
that LKMs are likely to be impacted.  (Most of the userland tools work fine, 
which was my original concern.)

Therefore, I'd like to follow one of two approaches, and I would like feedback 
on which to follow *sooner* rather than later.

Option one: make paddr_t 64-bit on all evbmips platforms (basically modifyin 
<machine/types.h>).  The current thinking is that this will not adversely 
impact any "current" hardware that has R4K style MMUs.  What I'm not sure is 
whether or not there is any desire to include future additional MIPS parts in 
the evbmips port that this would cause a problem for.  Apart from 
compatibility, there is the concern of increasing the size of the kernel, but 
I consider this concern rather negligible.

Option two: separate out the Alchemy parts into a separate port (aumips).  The 
main objection to this, I think, is additional complexity that it would 
create.    A few kernel bits could be simplified a bit, since at that point 
you don't have to consider e.g. 64-bit procs and compatibility with malta.

I would like to reach a decision on this as soon as possible.  My preference 
is to just change the paddr_t size across the entire evbmips port.
--

-- 
Garrett D'Amore, Principal Software Engineer
Tadpole Computer / Computing Technologies Division,
General Dynamics C4 Systems
http://www.tadpolecomputer.com/
Phone: 951 325-2134  Fax: 951 325-2191
(Continue reading)

Matt Thomas | 30 Jan 2006 22:04

Re: 64-bit paddr_t (again, arrgh....)

Garrett D'Amore wrote:
> As Izumi-san recently pointed out, there is a problem with my current approach 
> of conditionalizing the size of paddr_t on 64-bit platforms.  The problem is 
> that LKMs are likely to be impacted.  (Most of the userland tools work fine, 
> which was my original concern.)
> 
> I would like to reach a decision on this as soon as possible.  My preference 
> is to just change the paddr_t size across the entire evbmips port.

does evbmips encompass any non-R4k processors?
--

-- 
Matt Thomas                     email: matt <at> 3am-software.com
3am Software Foundry              www: http://3am-software.com/bio/matt/
Cupertino, CA              disclaimer: I avow all knowledge of this message.


Gmane