[PATCH v2 22/35] mtd: spi-nor: core: Use common naming scheme for setting mtd_info fields

[Tudor.Ambarus at microchip.com]

On 10/22/21 4:08 PM, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Am 2021-10-22 14:51, schrieb Tudor.Ambarus at microchip.com:
>> On 10/22/21 2:57 PM, Michael Walle wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>>> the content is safe
>>>
>>> Am 2021-07-27 06:52, schrieb Tudor Ambarus:
>>>> The functions names are self explanatory, get rid of the comment
>>>> for the OTP function.
>>>
>>> Mhh. I see, this partly addresses my comments to the previous patch.
>>> Maybe it would have been better to have this squashed into one
>>> commit :p
>>
>> one thing per patch
> 
> well it is really just one thing, you're moving the function. but then
> the function name doesn't make sense anymore. So the patch on its own
> is wrong or at least it makes things worse. Also, I'm reviewing that
> just to see this function was renamed (correctly) in the next patch.

I'll squash it then.

> 
>>> But my main concern remains: what if we need more that just
>>> the mtd callbacks assigments. Basically we loose the otp_init()
>>> call.
>>
>>         if (WARN_ON(!is_power_of_2(spi_nor_otp_region_len(nor))))
>>                 return;
>>
>> is there just to avoid setting the mtd function pointers when OTP
>> region length is not power of 2. This is not an init, just a check,
>> so for the moment I see it ok to have it in spi_nor_set_mtd_otp_ops().
> 
> It is a sanity check which actually has nothing to do with the mtd ops
> registration. Just to see if there was a mistake in the otp info flags.
> 
> Of course, if that is not the case, we don't set the ops. But that
> doesn't
> mean it is part of the mtd op assigment.
> 
> I said stricly speaking, I'm ok with having that in the (now renamed)
> "otp set mtd ops" function.

ok, I'll keep it. If condition is true, you don't set the mtd ops. I find it
part of the mtd op assignment.