Re: [PATCH] [MTD] [RFC] New Solarflare NIC EEPROM/Flash driver

Ben Hutchings
2008-01-11T18:56:03Z

Jörn Engel wrote:
> > > efx_mtd->dead is fun. Does this still happen with production
> > > hardware?
> >
> > It shouldn't happen with anything that passed manufacturing tests.
> > This is playing safe.
>
> Fair enough. Then I would prefer the dead_device_operations approach.

It turns out that struct efx_mtd_operations is a redundant indirection
layer, but I can introduce a dummy struct efx_spi_device which does
the same thing.

> > > Even if it does, instead of setting the flag and checking it in
> > > every function, you could replace the operations with
> > > dead_device_operations that simply return -EIO for every call.
> > >
> > > struct semaphore access_lock; should become a mutex.
> >
> > Right. We've tended to be quite conservative in using newer kernel
> > features, since we also need to support old kernels, but we have a
> > good backward-compatibility layer now (unifdef'd out of the submitted
> > code) so this shouldn't be a problem.
>
> Ok. A straight conversion to a mutex will likely cause trouble with
> your reset routine. Not sure what to do here.

We can use a mutex if the net driver is guaranteed to call
reset_suspend() and reset_resume() in the same context. This is
currently true (they are always called in a pair by efx_reset() in
efx.c) but I'm not sure we want to guarantee that. Unfortunately it
*is* sometimes necessary to reset the controller after it's been
exposed through driverlink, so this is not something we can ignore.

Ben.

--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.