From: "Jörn Engel" <joern@logfs.org>
To: Ben Hutchings <bhutchings@solarflare.com>
Cc: "Jörn Engel" <joern@logfs.org>,
linux-mtd@lists.infradead.org, linux-net-drivers@solarflare.com
Subject: Re: [PATCH] [MTD] [RFC] New Solarflare NIC EEPROM/Flash driver
Date: Fri, 11 Jan 2008 14:24:45 +0100 [thread overview]
Message-ID: <20080111132444.GA1399@lazybastard.org> (raw)
In-Reply-To: <20080111125000.GL3544@solarflare.com>
On Fri, 11 January 2008 12:50:01 +0000, Ben Hutchings wrote:
> >
> > The prefix "efx_mtd_" causes me personally some trouble. By the
> > time I have reached the relevant part of the name, my brain has
> > already fallen half asleep. But maybe I'm just overreacting after
> > working with in-house code for five years.
> >
> > I have no idea why you need eraseregions, if there is just one.
> > Kill them?
>
> Earlier versions of this driver supported our first controller and
> NICs, EF1/EF1002, which did have multiple regions of flash. I
> simplified the code after this support was removed, but I wasn't aware
> that we could avoid specifying eraseregions at all.
Even when eraseregions exist, most code simply ignores them and treats
the complete flash a having a single uniform erasesize.
*greps the source*
The only two users are UBI and mtdchar. UBI is bogus and mtdchar
exposes it to userspace, where anything could theoretically exist.
Anything else is confined to the CFI code, which you don't use.
> > 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.
> > 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.
Jörn
--
All models are wrong. Some models are useful.
-- George Box
next prev parent reply other threads:[~2008-01-11 13:30 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-01-10 18:51 [PATCH] [MTD] [RFC] New Solarflare NIC EEPROM/Flash driver Robert Stonehouse
2008-01-10 20:13 ` Jörn Engel
2008-01-10 23:16 ` Jörn Engel
2008-01-11 12:50 ` Ben Hutchings
2008-01-11 13:24 ` Jörn Engel [this message]
2008-01-11 18:55 ` Ben Hutchings
2008-01-11 19:57 ` Jörn Engel
2008-01-13 17:19 ` David Riddoch
2008-01-14 17:04 ` [PATCH] [MTD] [RFC] New Solarflare NIC EEPROM/Flash driver (2nd try) Ben Hutchings
2008-01-15 16:46 ` Jörn Engel
2008-01-15 17:35 ` Ben Hutchings
2008-01-15 17:55 ` Jörn Engel
2008-01-15 17:57 ` Ben Hutchings
2008-01-15 18:28 ` Jörn Engel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20080111132444.GA1399@lazybastard.org \
--to=joern@logfs.org \
--cc=bhutchings@solarflare.com \
--cc=linux-mtd@lists.infradead.org \
--cc=linux-net-drivers@solarflare.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox