In article <87fzvv329t.fsf@power.cnet.aladdin.de>
cpg@aladdin.de wrote:

> >- The device name should be "mbe" that is defined in sys/conf/files,
> >  rather than "lana", and lan_adapter.c should be renamed as
> >  if_mbe_g2.c and so. (see sys/dev/pcmcia/if_mbe_pcmcia.c)
> 
> Ok, I've changed this. Now I recognize the system. But isa/if_fmv.c
> also doesn't follow it.

There are two variants of mb8696x based ISA boards, ate and fmv.
fmv seems to have a special ASIC which handles some hardware
configuration, so I think it's OK to have a differnt device name.
But if_ate.c seems to have only generic MB86965, which has additinal
bus interface for ISA, maybe it should be renamed to if_mbe_isa.c.

Anyway, I guess these mb8696x drivers on ISA are ported in 1995,
so they need more cleanup. It's better to refer more modern drivers :-)

> So I left it as it is for now.
> What shall I do here? Add a stride variable to the g2 bus space handle? 

This is implementation issue of g2bus devices, as discussed with Marcus.
For now, I think it's acceptable to override function pointers in
bus space tag as your patch does.

> What is "FE_D7_IDENT_NICE"? Is it that nice? According to my data
> sheet,
> 
> #define FE_D7_IDENT_86960       0x00
> #define FE_D7_IDENT_86964       0x40
> #define FE_D7_IDENT_86967       0x80
> #define FE_D7_IDENT_86965       0xC0
> 
> in dev/ic/mb86960reg.h would make more sense.

Then please also fix mb86960reg.h and add it to your patch ;-)

> >- lan_adapter.c has "Initialize ASIC" code (seems pulled from if_fmv.c),
> >  but is it really required?
> 
> Cut + Paste error. I've tried it and it still works without. Removed.

There is still fmv ASIC related code:

>>	/* Is this really needs to be done here? XXX */
>>	/* Turn the "master interrupt control" flag of ASIC on. */
>>	bus_space_write_1(memt, memh, FE_FMV3, FE_FMV3_ENABLE_FLAG);

I guess this is not needed and we don't have to include
dev/isa/if_fereg.h via dev/ic/ate_subr.c.
(ate_subr.c and if_fereg.h should be fixed anyway)

> >- It also includes dev/ic/ate_subr.c, but is it needed?
> >  (Maybe we need to cleanup register configuration of mbe vs fmv.)
> 
> I need it for the ate_read_eeprom() function. There is the mac address
> stored in the eeprom.

Hmm, if the EEPROM routine is common for all MB86965 chips,
we should move it into sys/dev/ic/mb86960.c and nuke ate_subr.c.

As noted above ate and fmv drivers should be reorganized,
but for now the easiest way is to copy the EEPROM routine into
if_mbe_g2.c until all other MB8696x drivers are cleaned up.
If you still want to use current dev/ic/ate_subr.c,
you should declare it in dreamcast/conf/files.dreamcast like:
>> file dev/ic/ate_subr.c mbe_g2bus
rather than including it directly from if_mbe_g2.c.
---
Izumi Tsutsui
tsutsui@ceres.dti.ne.jp