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

Am 2021-10-22 14:51, schrieb Tudor.Ambarus@microchip.com:

On 10/22/21 2:57 PM, Michael Walle wrote:

quoted

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:

quoted

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.

quoted

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.

-michael

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel