Re: [Etherboot-developers] main.c:cleanup()

[Marty C. <md...@th...>]

On 6/22/2001 11:43 PM ke...@us... 
ke...@us... wrote:
>I've found that it's not safe to call eth_reset after eth_disable in
>cleanup(). On my 3c509 it hangs the program. So I have removed it from
>5.0.2. This probably means 5.1.0 wasn't much tested.

I agree with this.  Depending on the card, this may work or not.

>Part of the problem is that the semantics of eth_probe, eth_reset and
>eth_disable have never been rigorously defined. Informally I took it to
>be:
>eth_probe:	Finds the card

In eth_probe, most cards assume the card has been found by the core 
Etherboot routines, and do some initializations and checking, including:

- Putting the MAC address in *nic_node_addr [I think this should be in 
reset()]
- setting the addresses of the other driver routines
- calling eth_reset to prepare the card to send and receive packets

I'm not really sure if the etherboot core calls eth_reset before actually 
trying to send and receive packets;  If it does, we could probably not 
call it in eth_reset since it is probably being called redundantly in 
some cases.

>eth_reset:	Resets the card to a known state

Most drivers in this routine:

- do a hard/soft reset of the card
- setup TX and RX rings
- tell the card where the TX and RX rings are
- select proper media interface
- check duplex

Here's the sis900 eth-reset routine:

sis900_init(struct nic *nic)
{
    /* Soft reset the chip. */
    sis900_reset(nic);

    sis900_init_rxfilter(nic);

    sis900_init_txd(nic);
    sis900_init_rxd(nic);

    sis900_set_rx_mode(nic);

    sis900_check_mode(nic);

    outl(RxENA, ioaddr + cr);
}

in eepro100.c the eth_reset routine is:

static void eepro100_reset(struct nic *nic)
{
  outl(0, ioaddr + SCBPort);
}

This is probably inadequate for a lot of cases where one would call 
eth_reset, and a lot of what is in the eepro100 eth_probe() routine 
should be moved to the reset routine.  We are depending on a lot of 
things not changing by using such a simple reset routine.

I've taken to creating a routine called eth_init() in drivers because the 
word "reset" is a little overloaded.  Most cards have specific semantics 
about simply being reset, whereas Etherboot expects a lot of stateful 
initializations to be done.

One of those rainy-day projects I have is to go through all the drivers 
and make them more semantically consistent in this area.  Of course, 
great care must be taken in such endeavors, and I normally don't alter 
drivers unless there is a problem.

>		can be called in last part of eth_probe

I'd like to make this "must be called".

This would make it the driver's responsibility to get the card in the 
proper state.
I tend to think "probe" routines should do as little as possible, since 
they will likely only get called once by the core.

>Normal operation:
>		eth_reset can be called as often as wished

This seems fine.  Putting the card in a state where it can send and 
receive packets may require resetting some global driver state (RX 
pointers, counters, etc.) as well as some card state (resetting registers 
to point to driver globals).

>eth_disable:	Disables the card
>		Only eth_probe can be called thereafter

Hmmm, I think some drivers assume that disable just disables TX and RX.  
I think of the sematics as being "put the card in such a state that 
another driver can safely start and manipulate the card".  I tend to stop 
RX and TX, disable interrupts and DMA, and issue a hard reset to the 
card.  Since DMA could have been happening, it's important to make sure 
the card engine is not writing to memory after Etherboot stops because it 
could corrupt memory in unexpected and hard to debug ways.

>I feel that if eth_disable is felt to be inadequate for cleanup, then it
>should be enhanced, rather than calling eth_reset from cleanup.

I agree.

>Another possibility is for cleanup() to call eth_reset first, then
>eth_disable. I would be willing to introduce this in 5.1.1 but I feel
>5.0.2 should not risk breaking things for the majority of users.
>Comments?

This would probably work in the majority of cases.  This will motivate me 
to go through all the drivers and attempt to have consistent semantics 
for the five basic routines (probe, reset, transmit, poll, disable).

Partially this depends on what the Etherboot core expects.  Is reset() 
always called after probe() ?   If disable() is called, are both probe() 
and reset() called before any other driver routines?  If only probe() is 
called, then I'd say we have to insist that people call reset() at the 
end of probe().  

So I guess I'm in favor of very simple probe() routines, since reset() is 
more likely to get called to reinitialize the card and driver state.  
probe() seems like it should only do PCI bus related stuff, set some 
globals (including the addresses of the other routines), and call 
reset().  Anything that the card or driver could change during normal 
operation should be done in reset(). 

Anyway, that's my initial thinking on this.  I welcome other opinions.

Marty

---
    Try: http://rom-o-matic.net/ to make Etherboot images instantly.

   Name: Marty Connor
US Mail: Entity Cyber, Inc.; P.O. Box 391827; Cambridge, MA 02139; USA
  Voice: (617) 491-6935, Fax: (617) 491-7046 
  Email: md...@th...
    Web: http://www.thinguin.org/