Re: [PATCH 0/1] arm: imx: fsl_otp: make fuses (OTP memory) read-only

[Alexander Holler <holler@ahsoftware.de>]

Am 07.11.2014 17:00, schrieb Otavio Salvador:
> Hello Alexander,
>
> On Fri, Nov 7, 2014 at 1:23 PM, Alexander Holler <holler@ahsoftware.de> wrote:
>> Am 07.11.2014 16:07, schrieb Otavio Salvador:
>>>
>>> On Fri, Nov 7, 2014 at 1:04 PM, Eric Bénard <eric@eukrea.com> wrote:
>>>>
>>>> Hi Jon,
>>>>
>>>> Le Fri, 7 Nov 2014 15:31:15 +0100,
>>>> Jon Nettleton <jon.nettleton@gmail.com> a écrit :
>>>>>
>>>>> Fair enough.  Can we all agree that it should default to disabled?
>>>>
>>>>
>>>> On your product sure, on the evaluation board that's more questionable.
>>>>
>>>> When an evaluation board's user reach this sysfs entries to do a write
>>>> to it I believe that's not a random choice but a user's choice.
>>>>
>>>> Maybe adding a otp_unlock entry to the sysfs before allowing write could
>>>> secure more the feature when running on the evaluation boards so that
>>>> the user can think twice before writing to the fuses.
>>>
>>>
>>> I like Eric's idea of the unlock sysfs entry for this purpose.
>>
>>
>> Maybe I should mention that this extremly dangerous driver already was
>> posted for submission into the (production) kernel and nobody had a problem
>> with it's write functionality, just with some other stuff.
>>
>> And even if it's ok for FSL to promote this feature with their BSP, I have
>> absolutely no love for the way it was done. It now is likely part of many
>> production kernels, something never should have happened.
>>
>> And I still see absolutely no need to have some one time functionality as
>> part of the kernel, regardless how usefull it might be for some developers.
>> It's a waste of time and resources to include that write functionality in
>> something meant for the greater public. Especially something that dangerous.
>
> We have two different things here.
>
>   * Linux support: the driver is perfectly capable of handling the
> reading and writing
>   * Driver settings: We should, indeed, have it to be explicitly
> enabled on the kernel config to allow writing
>
> This is done in other areas of kernel (one which comes to my mind is
> NTFS Writing support) and what I proposed was you to make:
>
> CONFIG_FSL_OTP -> read-only
> CONFIG_FSL_OTP_WRITE -> write support
>
> The write support needs to be enabled on mfgtools defconfig and
> disabled on regular kernels (this is what I think it should be but it
> is up to the board maintainer).

Sorry, but based on my experience and confrontations with Murphy I can't 
agree.

To conclude what already happened:

- Someone has written that driver in the way it is.
- Several devs likely have seen it before it made it's way into the BSP.
- It got enabled by default in the BSP (defconfigs).
- It made it's way into YOCTO.
- It was submitted for inclusion into the kernel.

So the driver already was seen by a lot of people (most likely all devs) 
and still nobody has seen a problem. And most of these people did know 
more about its functionality than most people which are just changing 
switches in kernel configs or do play with sysfs.

And because I don't like to play the bad boy, I'm not going to discuss 
this any further. I did my part as responsible developer and I think I 
made my standpoint clear. I hoped it would have happened more silently 
without that discussion, but ... ;)

But feel free to continue this discussion, I don't care if my patch will 
be included or not. I just will not modify it. ;)

Regards,

Alexander Holler