| Message ID | 9db9fb39e655b514c21952e0a5005572510ad3ec.1713154967.git.Takahiro.Kuwano@infineon.com |
|---|---|
| State | Superseded |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | mtd: Make sure UBIFS does not do multi-pass page programming on flashes that don't support it | expand |
On 4/15/24 05:33, tkuw584924@gmail.com wrote: > From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com> > > default_init() fixup hook should be used to initialize flash parameters > when its information is not provided in SFDP. To support that case, it > needs to take flash_parameter structure like as other hooks. > > Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com> > --- I'd like to get rid of the default_init hook, let's not extend it if possible. Can you use the late_init hook instead? Cheers, ta
Hi Tudor, On 4/15/2024 3:47 PM, Tudor Ambarus wrote: > > > On 4/15/24 05:33, tkuw584924@gmail.com wrote: >> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com> >> >> default_init() fixup hook should be used to initialize flash parameters >> when its information is not provided in SFDP. To support that case, it >> needs to take flash_parameter structure like as other hooks. >> >> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com> >> --- > > I'd like to get rid of the default_init hook, let's not extend it if > possible. Can you use the late_init hook instead? > It looks easy to migrate from default_init to late_init so I will do it. Could you provide the links to related discussion in Linux MTD side so that I can summarize it in commit message? Thanks, Takahiro
On 4/15/24 08:09, Takahiro Kuwano wrote: > Hi Tudor, Hi! > > On 4/15/2024 3:47 PM, Tudor Ambarus wrote: >> >> >> On 4/15/24 05:33, tkuw584924@gmail.com wrote: >>> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com> >>> >>> default_init() fixup hook should be used to initialize flash parameters >>> when its information is not provided in SFDP. To support that case, it >>> needs to take flash_parameter structure like as other hooks. >>> >>> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com> >>> --- >> >> I'd like to get rid of the default_init hook, let's not extend it if >> possible. Can you use the late_init hook instead? >> > It looks easy to migrate from default_init to late_init so I will do it. > Could you provide the links to related discussion in Linux MTD side so that > I can summarize it in commit message? > I can't, I don't remember if I brought this up or when, but I can explain why. default_init() is wrong, it contributes to the maze of initializing flash parameters. We'd like to get rid of it because the flash parameters that it initializes are not really used at SFDP parsing time, thus they can be initialized later. Ideally we want SFDP to initialize all the flash parameters. If (when) SFDP tables are wrong, we fix them with the post_sfdp/bfpt hooks, to emphasize that SFDP is indeed wrong. When there are parameters that are not covered by SFDP, we initialize them in late_init() - these parameters have nothing to do with SFDP and they are not needed earlier. With this we'll have a clearer view of who initializes what. Feel free to use this in the commit message if you think it helps. Cheers, ta
On 4/15/2024 4:27 PM, Tudor Ambarus wrote: > > > On 4/15/24 08:09, Takahiro Kuwano wrote: >> Hi Tudor, > > Hi! > >> >> On 4/15/2024 3:47 PM, Tudor Ambarus wrote: >>> >>> >>> On 4/15/24 05:33, tkuw584924@gmail.com wrote: >>>> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com> >>>> >>>> default_init() fixup hook should be used to initialize flash parameters >>>> when its information is not provided in SFDP. To support that case, it >>>> needs to take flash_parameter structure like as other hooks. >>>> >>>> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com> >>>> --- >>> >>> I'd like to get rid of the default_init hook, let's not extend it if >>> possible. Can you use the late_init hook instead? >>> >> It looks easy to migrate from default_init to late_init so I will do it. >> Could you provide the links to related discussion in Linux MTD side so that >> I can summarize it in commit message? >> > > I can't, I don't remember if I brought this up or when, but I can > explain why. > > default_init() is wrong, it contributes to the maze of initializing > flash parameters. We'd like to get rid of it because the flash > parameters that it initializes are not really used at SFDP parsing time, > thus they can be initialized later. > > Ideally we want SFDP to initialize all the flash parameters. If (when) > SFDP tables are wrong, we fix them with the post_sfdp/bfpt hooks, to > emphasize that SFDP is indeed wrong. When there are parameters that are > not covered by SFDP, we initialize them in late_init() - these > parameters have nothing to do with SFDP and they are not needed earlier. > With this we'll have a clearer view of who initializes what. > > Feel free to use this in the commit message if you think it helps. > Cheers, > ta Of course it helps!
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 1bfef6797f..8f371a5213 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -203,7 +203,8 @@ struct sfdp_bfpt { * table is broken or not available. */ struct spi_nor_fixups { - void (*default_init)(struct spi_nor *nor); + void (*default_init)(struct spi_nor *nor, + struct spi_nor_flash_parameter *params); int (*post_bfpt)(struct spi_nor *nor, const struct sfdp_parameter_header *bfpt_header, const struct sfdp_bfpt *bfpt, @@ -2775,10 +2776,11 @@ static void spi_nor_post_sfdp_fixups(struct spi_nor *nor, nor->fixups->post_sfdp(nor, params); } -static void spi_nor_default_init_fixups(struct spi_nor *nor) +static void spi_nor_default_init_fixups(struct spi_nor *nor, + struct spi_nor_flash_parameter *params) { if (nor->fixups && nor->fixups->default_init) - nor->fixups->default_init(nor); + nor->fixups->default_init(nor, params); } static int spi_nor_init_params(struct spi_nor *nor, @@ -2885,7 +2887,7 @@ static int spi_nor_init_params(struct spi_nor *nor, } } - spi_nor_default_init_fixups(nor); + spi_nor_default_init_fixups(nor, params); /* Override the parameters with data read from SFDP tables. */ nor->addr_width = 0; @@ -3328,7 +3330,8 @@ static int s25fs_s_setup(struct spi_nor *nor, const struct flash_info *info, return spi_nor_default_setup(nor, info, params); } -static void s25fs_s_default_init(struct spi_nor *nor) +static void s25fs_s_default_init(struct spi_nor *nor, + struct spi_nor_flash_parameter *params) { nor->setup = s25fs_s_setup; } @@ -3452,7 +3455,8 @@ static int s25_s28_setup(struct spi_nor *nor, const struct flash_info *info, return spi_nor_default_setup(nor, info, params); } -static void s25_default_init(struct spi_nor *nor) +static void s25_default_init(struct spi_nor *nor, + struct spi_nor_flash_parameter *params) { nor->setup = s25_s28_setup; } @@ -3544,7 +3548,8 @@ static int s25fl256l_setup(struct spi_nor *nor, const struct flash_info *info, return -ENOTSUPP; /* Bank Address Register is not supported */ } -static void s25fl256l_default_init(struct spi_nor *nor) +static void s25fl256l_default_init(struct spi_nor *nor, + struct spi_nor_flash_parameter *params) { nor->setup = s25fl256l_setup; } @@ -3613,7 +3618,8 @@ static int spi_nor_cypress_octal_dtr_enable(struct spi_nor *nor) return 0; } -static void s28hx_t_default_init(struct spi_nor *nor) +static void s28hx_t_default_init(struct spi_nor *nor, + struct spi_nor_flash_parameter *params) { nor->octal_dtr_enable = spi_nor_cypress_octal_dtr_enable; nor->setup = s25_s28_setup; @@ -3705,7 +3711,8 @@ static int spi_nor_micron_octal_dtr_enable(struct spi_nor *nor) return 0; } -static void mt35xu512aba_default_init(struct spi_nor *nor) +static void mt35xu512aba_default_init(struct spi_nor *nor, + struct spi_nor_flash_parameter *params) { nor->octal_dtr_enable = spi_nor_micron_octal_dtr_enable; } @@ -3795,7 +3802,8 @@ static int spi_nor_macronix_octal_dtr_enable(struct spi_nor *nor) return 0; } -static void macronix_octal_default_init(struct spi_nor *nor) +static void macronix_octal_default_init(struct spi_nor *nor, + struct spi_nor_flash_parameter *params) { nor->octal_dtr_enable = spi_nor_macronix_octal_dtr_enable; }