spiflash.c process_write()

[David Holland <dholland-tech%netbsd.org@localhost>]

As noted in passing elsewhere, it seems that process_write() in
spiflash.c allocates a scratch buffer on every call... and leaks it on
every call too. This clearly isn't a good thing.

Meanwhile the size of buffer it tries to allocate doesn't have any
obvious bound; I suspect it's limited to MAXPHYS, and if so that's
probably ok, but if not there could be problems even if the leak is
fixed.

Meanwhile, spiflash.c has only one client (m25p) which, in its man
page, says:

BUGS
     Some important bits that are vital for use as a mass storage device are
     still missing.  Specifically, there is no API to erase the device, and
     there is no support for device partitioning.

So, presumably this code is only lightly tested.

ISTM that the code probably ought to be rewritten to handle one erase
block at a time; and, unless there's a reason to worry about
concurrent throughput (how fast is SPI anyway?), it ought to allocate
one write buffer up front instead of churning through them on every
write request.

And the driver isn't MPSAFE, which seems fairly readily fixed if one's
going to hack it anyhow. But it isn't clear to me how important this
is. Among other things, NOR flash is in general not the future.

There has been some discussion of these points since the issue
appeared (in maxv's Brainy posting a couple weeks ago) but no
conclusions. I guess the questions at this point are:
 - does anyone have an m25p they can test driver changes with?
 - how much do we care about this driver?

I'm willing to fix the memory leak without testing, but no more than
that.

-- 
David A. Holland
dholland%netbsd.org@localhost