Hi. I am not exactly qualified in the area of spiflash internals, but I have used it on several occasions.
> On 08 Sep 2015, at 08:12, David Holland <dholland-t...@netbsd.org> wrote: > > 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. A few years ago, when I worked on bringing Armada XP into our tree, I had (still have somewhere in my closet) a Marvell dev board which featured m25p. The spiflash worked just fine, for what it is. So, yes, I never used it in the block device sense, but it was OK as a character device used to access the firmware contents. > 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. Current SPI variants can be pretty fast. Some modern M25P derivatives like MT25Q can transfer 4 bits at 133MHz (around 66MB/s). > 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? Yep. Probably it's easier for me to attach a standalone M25P16 to RPI than to search for that dev board. > - how much do we care about this driver? Not very much but it's nice to have it :P. Perhaps it should be converted to use flash(9)... > I'm willing to fix the memory leak without testing, but no more than > that. I can test it. Best regards, Radoslaw