Supporting flash that is locked by default

[Nicolas Pitre]

On Tue, 28 Sep 2004, Todd Poynor wrote:

> On Sat, Sep 25, 2004 at 11:33:36AM +0100, David Woodhouse wrote:
> > This locking scheme is daft. My original reaction was to hide it in the
> > mapping driver for such idiotic flashes, but they're more common now and
> > it really is a function of the flash not of the board. I suspect we
> > ought to add a quirk entry for these flashes, and automatically unlock
> > them on probe.
> 
> Here's a try at automatically unlocking all blocks of Intel CFI flash at
> probe time and at system resume time.  The new logic is in the chip
> driver, but perhaps it would be better to move it generic, adding a new
> chip driver callback for "is unlock needed?"; can rework things that way
> if desired (was originally hoping to hide this "feature" from generic
> code).
> 
> I did not notice a query feature bit that identifies whether the flash
> has the "all blocks locked at power on" property in the datasheets, so
> the patch checks whether block 0 is locked and assumes its one of those
> flashes if so.

This is a rather bad idea since block 0 is often the bootloader and most 
likely to be locked on purpose.

I'm a bit unconfortable with a feature like this as well.  What if I 
want some partitions to remain locked at all times at the hw level?

I think the decision of unlocking the chip and what to unlock should 
remain in the map driver since this is where the knowledge of what the 
flash is meant to be used for is the most likely to be found.

> Since all blocks may be unlocked at resume time, it would be up to a
> userspace action to relock any previously explicitly locked blocks soon
> afterwards.

This sounds really really fishy to me.

If you can query which blocks are or are not locked before suspending, 
then why don't you just create a bitmap with the state of the entire 
flash and restore it upon resume?

> The chip driver (or generic layer) *could* keep track of
> locked blocks and only unlock those not previously locked, can send a
> patch for that if there's interest.

I think this is the only sane approach to this problem IMHO.


Nicolas