Re: [PATCH v2 11/35] mtd: spi-nor: winbond: Use manufacturer late_init() for OTP ops

Pratyush Yadav
2021-08-16T19:18:59Z

On 27/07/21 07:51AM, Tudor Ambarus wrote:
> OTP info is not yet discoverable via SFDP, use late_init() to init
> the OTP ops.

What do you mean by the "yet"? Does it mean that OTP info is planned to
be added to the next SFDP version? Or does it mean that it is possible
to discover it via SFDP but we just don't support it yet?

If it is neither and it just means "SFDP does not mention OTP at all",
like it is for locking, then you should just drop the "yet". I know this
is very nitpicky but it just caught my eye.

>
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
> drivers/mtd/spi-nor/winbond.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
> index 96573f61caf5..6be45d2291c6 100644
> --- a/drivers/mtd/spi-nor/winbond.c
> +++ b/drivers/mtd/spi-nor/winbond.c
> @@ -147,17 +147,22 @@ static const struct spi_nor_otp_ops winbond_otp_ops = {
> static void winbond_default_init(struct spi_nor *nor)
> {
> nor->params->set_4byte_addr_mode = winbond_set_4byte_addr_mode;

Why not move this to late_init() as well?

> - if (nor->params->otp.org->n_regions)
> - nor->params->otp.ops = &winbond_otp_ops;
> }
>
> static const struct spi_nor_fixups winbond_fixups = {
> .default_init = winbond_default_init,
> };
>
> +static void winbond_late_init(struct spi_nor *nor)
> +{
> + if (nor->params->otp.org->n_regions)
> + nor->params->otp.ops = &winbond_otp_ops;
> +}
> +
> const struct spi_nor_manufacturer spi_nor_winbond = {
> .name = "winbond",
> .parts = winbond_parts,
> .nparts = ARRAY_SIZE(winbond_parts),
> .fixups = &winbond_fixups,
> + .late_init = winbond_late_init,
> };
> --
> 2.25.1
>

--
Regards,
Pratyush Yadav
Texas Instruments Inc.