Re: [PATCH v3 2/3] mtd: spi-nor: add 4bit block protection support

From: Michael Walle <michael@walle.cc>
To: Tudor.Ambarus@microchip.com
Cc: linux-mtd@lists.infradead.org, john.garry@huawei.com,
	js07.lee@gmail.com, vigneshr@ti.com, js07.lee@samsung.com
Subject: Re: [PATCH v3 2/3] mtd: spi-nor: add 4bit block protection support
Date: Wed, 22 Jan 2020 18:14:34 +0100	[thread overview]
Message-ID: <87d1ed5d35cc5cb7addfb60d5dd028c9@walle.cc> (raw)
In-Reply-To: <1780435.Q0f6Nmcn8h@localhost.localdomain>

Am 2020-01-22 15:31, schrieb Tudor.Ambarus@microchip.com:
> Hi, Jungseung,
> 
> On Wednesday, January 22, 2020 1:42:00 PM EET Jungseung Lee wrote:
> 
> cut
> 
>> > > > +#define SPI_NOR_BP3_SR_BIT6    BIT(18) /*
>> > > > +                                        * BP3 is bit 6 of status
>> > > > register.
>> > > > +                                        * Must be used with
>> > >
>> > > Are we safe to replace SPI_NOR_TB_SR_BIT6 and SPI_NOR_BP3_SR_BIT6
>> > > with a
>> > > SPI_NOR_SR_TB_BIT6_BP3_BIT5? Or maybe with a
>> > > SPI_NOR_SR_BP3_BIT6_TB_BIT5, how
>> > > is more convenient?
>> >
>> > Let's think about some flash in which BP0-3 exists in the status
>> > register but TB exists in another register.
>> > for example, mx25u12835f.

And like the mx25u3232f, but this bit is only OTP programmable! For now,
I'd only add support for reading the TB bit to for this kind of flashes,
to prevent accidentially program this bit. It would be up to the board
manufacturer how to actually set the bit.

Like having a TB_READ_ONLY flag.

Its also some kind of asymmetrical because you can only set it one way,
eg. you can OTP the flash to be TB=1. Then you can be sure that the 
flash
will always be TB=1. But OTOH you cannot be sure that a TB=0 flash will
always be a TB=0 flash, because you cannot lock that bit.

Any thoughts?

-michael

>> > I haven't tested yet, but according to the datasheet, I think this
>> > patch can support 4bit block protection for the flash.
>> >
>> > In order to embrace the case, how about letting them as It is.
>> > Is there any suggestion?
> 
> Ok, this info should go in the commit message, together with details 
> about
> which flashes were tested.
> 
> I didn't know that the TB bit can be defined in the Configuration 
> register.
> This means that your suggestion with dedicated macros for BP3 and TB is 
> fine.
> 
> I looked a bit over your patches, they are in a pretty good shape. I 
> saw
> something that can be improved on patch 2/3, but I didn't manage to 
> finish the
> review. Your patches are the first on my TODO list, but now I'm a bit 
> busy. I
> hope that I'll come with a complete review by the end of the next week. 
> I'm
> going to do tests on few flashes too, to make sure that BP0-2 was not
> affected.
> 
> In the meantime, maybe Michael or John can review/test your patches, 
> they
> showed interest in BP0-3 support.
> 
> Cheers,
> ta

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/