Re: [flashrom] [PATCH] Add support for GigaDevice GD25VQ41B

[Stefan Tauner]

On Fri, 12 Feb 2016 12:39:51 +0200
Urja Rannikko  wrote:
> > >         .feature_bits   = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_QPI,  
>
> These arent "truly/100%" OTP, but one-time-lockable "security
> registers" - the effect is most likely
> the same (=if they're used they're most likely locked and that makes
> them effectively ROM), so
> I'm ok with the FEATURE_OTP, but maybe change the comment, my suggestion:
> /* Lockable Security Registers: 3 * 512B pages, read 0x48, write 0x42,
> erase 0x44 */

Hm... That is most likely my fault. I think there are quite some other
chips where I deliberately added the OTP comment (and flag) in the same
form although they are one-time-lock-only.
Rationale: I wanted to make any chip with any permanently lockable bits
easily recognizable before starting to work on adding support for it.

Therefore I think the original comment is the way to go for the time
being (no need to resend the patch though, Hatim).

[PATCH] Add support for GigaDevice GD25VQ41B

[Urja Rannikko]

On Fri, Feb 12, 2016 at 11:42 PM, Stefan Tauner
 wrote:
> On Fri, 12 Feb 2016 12:39:51 +0200
> Urja Rannikko  wrote:
> > > >         .feature_bits   = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_QPI,
> >
> > These arent "truly/100%" OTP, but one-time-lockable "security
> > registers" - the effect is most likely
> > the same (=if they're used they're most likely locked and that makes
> > them effectively ROM), so
> > I'm ok with the FEATURE_OTP, but maybe change the comment, my suggestion:
> > /* Lockable Security Registers: 3 * 512B pages, read 0x48, write 0x42,
> > erase 0x44 */
>
> Hm... That is most likely my fault. I think there are quite some other
> chips where I deliberately added the OTP comment (and flag) in the same
> form although they are one-time-lock-only.
> Rationale: I wanted to make any chip with any permanently lockable bits
> easily recognizable before starting to work on adding support for it.

Ok, then i'll "revert" my comment, if that is purposefully like that :)

I had always assumed OTP means it has memory that does 1->0 but not 0->1,
which this technically isnt (it's erasable&writable until locked with
the high status register bit(s),
and then not writable at all).
> Therefore I think the original comment is the way to go for the time
> being (no need to resend the patch though, Hatim).