[PATCH v2 16/35] mtd: spi-nor: core: Mark default_init() as deprecated

[Pratyush Yadav]

On 01/10/21 02:18PM, Tudor.Ambarus at microchip.com wrote:
> On 8/16/21 10:36 PM, Pratyush Yadav wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > On 27/07/21 07:52AM, Tudor Ambarus wrote:
> >> The goal is to remove the spagetti init of params. The flash should
> >> be initialized by the SFDP data, and when SFDP tables are not defined,
> >> by the flash_info flags. SFDP data can be corrected by the
> >> post_{bfpt, sfdp} when wrong, and in case of flash_info flags init,
> >> we'll use the late_init() hook, where checking for the
> >> SPI_NOR_SKIP_SFDP flag.
> > 
> > Why depreciate it? It is not like we have external callers that we need
> > to notify. We know and control all the users of this function. Just move
> > all users to late_init() and delete this. You have already done a large
> > part of that work in the previous patches. Why not convert all other
> > callers as well? Is there some complicated piece of code that stops you
> > from touching it for now?
> 
> Nothing complicated, but I don't have flashes to test. Let's take them one by one:
> 
> drivers/mtd/spi-nor/gigadevice.c:static void gd25q256_default_init(struct spi_nor *nor)
> sets nor->params->quad_enable method. Quad enable is SFDP discoverable, if I replace
> the default_init() with late_init() I'll risk to overwrite the method discovered by SFDP.
> 
> drivers/mtd/spi-nor/issi.c:     .default_init = issi_default_init,
> same for ISSI. Here it is a manufacturer default init, which is worse. All flashes
> have to be checked. This is another reason why I strongly prefer late_init() calls
> to set flags per flash and not at manufacturer level.
> 
> drivers/mtd/spi-nor/micron-st.c:static void mt35xu512aba_default_init(struct spi_nor *nor)
> you have this flash. If the octal_dtr_enable method is not SFDP discoverable, I can
> move if to late_init(). I'm waiting for your call. I'll need a tested-by after I'll
> make the patch.
> 
> drivers/mtd/spi-nor/micron-st.c:        .default_init = micron_st_default_init,
> This one is hard to get rid of. I won't be surprised if these fields are overwritten
> when parsing SFDP. Moving them in late_init() will nullify SFDP parsing.
> 
> drivers/mtd/spi-nor/spansion.c:static void s28hs512t_default_init(struct spi_nor *nor)
> you have this flash. I'll move it to late init if octal_dtr_enable method is not SFDP
> discoverable. I'll need a tested-by.
> 
> drivers/mtd/spi-nor/winbond.c:static void winbond_default_init(struct spi_nor *nor)
> 4byte addr mode is SFDP discoverable, if I move it in late_init() I'll overwrite what
> I get from SFDP.
> 
> So just mark the hook as deprecated should be enough for now (at least for this release).
> We'll probably never have all these flashes at hand, so maybe we'll agree for a time limit
> after which we'll switch to late_init() or post_bfpt/sfpd() solely based on datasheet info.
> If the datasheets will be wrong, we'll have bugs and people will scream, but nothing that
> we can't handle/fix.

Ok, makes sense to me.

> 
> Cheers,
> ta
> 
> > 
> >>
> >> Signed-off-by: Tudor Ambarus <tudor.ambarus at microchip.com>
> >> ---
> >>  drivers/mtd/spi-nor/core.h | 5 ++---
> >>  1 file changed, 2 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> >> index 13d5c5edfd27..625f4eed75f1 100644
> >> --- a/drivers/mtd/spi-nor/core.h
> >> +++ b/drivers/mtd/spi-nor/core.h
> >> @@ -289,9 +289,8 @@ struct spi_nor_flash_parameter {
> >>
> >>  /**
> >>   * struct spi_nor_fixups - SPI NOR fixup hooks
> >> - * @default_init: called after default flash parameters init. Used to tweak
> >> - *                flash parameters when information provided by the flash_info
> >> - *                table is incomplete or wrong.
> >> + * @default_init: Deprecated. Use the post_{bfpt, sfdp}, or the late_init()
> >> + *                hooks instead.
> >>   * @post_bfpt: called after the BFPT table has been parsed
> >>   * @post_sfdp: called after SFDP has been parsed (is also called for SPI NORs
> >>   *             that do not support RDSFDP). Typically used to tweak various
> >> --
> >> 2.25.1
> >>
> > 
> > --
> > Regards,
> > Pratyush Yadav
> > Texas Instruments Inc.
> > 
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.