Subject: Re: LAN Adapter driver
To: None <firstname.lastname@example.org>
From: Izumi Tsutsui <email@example.com>
Date: 09/29/2002 00:54:14
In article <firstname.lastname@example.org>
> >- 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
> #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.