Hi Vignesh,
Am 2019-12-19 06:33, schrieb Vignesh Raghavendra:
Hi Michael,
[...]
+- no-unlock : By default, linux unlocks the whole flash because
there
+ are legacy flash devices which are locked by default
+ after reset. Set this flag if you don't want linux to
+ unlock the whole flash automatically. In this case you
+ can control the non-volatile bits by the
+ flash_lock/flash_unlock tools.
Current SPI NOR framework unconditionally unlocks entire flash which
I agree is not the best thing to do, but I don't think we need
new DT property here. MTD cmdline partitions and DT partitions
already
provide a way to specify that a partition should remain locked[1][2]
I know that the MTD layer has the same kind of unlocking. But that
unlocking is done on a per mtd partition basis. Eg. consider something
like the following
mtd1 bootloader (locked)
mtd2 firmware (locked)
mtd3 kernel
mtd4 environment
Further assume, that the end of mtd2 aligns with one of the possible
locking areas which are supported by the flash chip. Eg. the first
quarter.
The mtd layer would do two (or four, if "lock" property is set)
unlock()
calls, one for mtd1 and one for mtd2.
My point here is, that the mtd partitions doesn't always map to the
locking regions of the SPI flash (at least if the are not merged
together).
You are right! This will be an issue if existing partitions are not
aligned to locking regions.
I take my comments back... But I am not sure if a new DT property is
the
needed. This does not describe HW and is specific to Linux SPI NOR
stack. How about a module parameter instead?
Module parameter won't provide per flash granularity in controlling
unlocking behavior. But I don't think that matters.
I don't argue against having a kernel parameter, but just wanting to
point
out another alternative (which might be controversial):
- What is the purpose of this unlock_all() at all. Apparently there are
some flashes which have the protection bits set. Either at startup
(and then they are non-volatile) or they come in that state out of
the
factory. The latter makes little sense IMHO.
- Actually, all newer flashes we've used have these bits non-volatile
and
are unlocked by default.
So can't we have a whitelist (ie. a new flag in the spi_nor_ids) which
supresses the unlock if they haven't any block protections bit set
according to the manual? Because in this case the unlocking makes never
sense IMHO.
-michael
Tudor,
You had a patch doing something similar. Does module param sound good
to
you?
