| Message ID | 20210727045222.905056-10-tudor.ambarus@microchip.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mtd: spi-nor: Handle ID collisions and clean params init | expand |
On 27/07/21 07:51AM, Tudor Ambarus wrote: > Locking is not described in JESD216 SFDP standard, place the locking > init in late_init(). You are chaning the order of setting the locking ops here. Earlier, they were set before we parsed SFDP. Now they are set after we parse SFDP. Though I don't see it making much of a difference. > > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> > --- > drivers/mtd/spi-nor/atmel.c | 30 +++++++++++------------------- > 1 file changed, 11 insertions(+), 19 deletions(-) > > diff --git a/drivers/mtd/spi-nor/atmel.c b/drivers/mtd/spi-nor/atmel.c > index 1fea5cab492c..b937ef734e55 100644 > --- a/drivers/mtd/spi-nor/atmel.c > +++ b/drivers/mtd/spi-nor/atmel.c > @@ -48,15 +48,11 @@ static const struct spi_nor_locking_ops atmel_at25fs_locking_ops = { > .is_locked = atmel_at25fs_is_locked, > }; > > -static void atmel_at25fs_default_init(struct spi_nor *nor) > +static void atmel_at25fs_late_init(struct spi_nor *nor) > { > nor->params->locking_ops = &atmel_at25fs_locking_ops; > } > > -static const struct spi_nor_fixups atmel_at25fs_fixups = { > - .default_init = atmel_at25fs_default_init, > -}; > - > /** > * atmel_set_global_protection - Do a Global Protect or Unprotect command > * @nor: pointer to 'struct spi_nor' > @@ -146,34 +142,30 @@ static const struct spi_nor_locking_ops atmel_global_protection_ops = { > .is_locked = atmel_is_global_protected, > }; > > -static void atmel_global_protection_default_init(struct spi_nor *nor) > +static void atmel_global_protection_late_init(struct spi_nor *nor) > { > nor->params->locking_ops = &atmel_global_protection_ops; > } > > -static const struct spi_nor_fixups atmel_global_protection_fixups = { > - .default_init = atmel_global_protection_default_init, > -}; > - > static const struct flash_info atmel_parts[] = { > /* Atmel -- some are (confusingly) marketed as "DataFlash" */ > { "at25fs010", INFO(0x1f6601, 0, 32 * 1024, 4, SECT_4K | SPI_NOR_HAS_LOCK) > - .fixups = &atmel_at25fs_fixups }, > + .late_init = atmel_at25fs_late_init }, > { "at25fs040", INFO(0x1f6604, 0, 64 * 1024, 8, SECT_4K | SPI_NOR_HAS_LOCK) > - .fixups = &atmel_at25fs_fixups }, > + .late_init = atmel_at25fs_late_init }, > > { "at25df041a", INFO(0x1f4401, 0, 64 * 1024, 8, > SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) > - .fixups = &atmel_global_protection_fixups }, > + .late_init = atmel_global_protection_late_init }, Won't you be better off setting this in the manufacturer late_init()? It seems common for most atmel flashes. Of course, this would cause a problem for atmel flashes that don't have this at all, since we would set locking for those as well. But I think we can avoid that by checking for SNOR_F_HAS_LOCK in spi_nor_register_locking_ops(). > { "at25df321", INFO(0x1f4700, 0, 64 * 1024, 64, > SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) > - .fixups = &atmel_global_protection_fixups }, > + .late_init = atmel_global_protection_late_init }, > { "at25df321a", INFO(0x1f4701, 0, 64 * 1024, 64, > SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) > - .fixups = &atmel_global_protection_fixups }, > + .late_init = atmel_global_protection_late_init }, > { "at25df641", INFO(0x1f4800, 0, 64 * 1024, 128, > SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) > - .fixups = &atmel_global_protection_fixups }, > + .late_init = atmel_global_protection_late_init }, > > { "at25sl321", INFO(0x1f4216, 0, 64 * 1024, 64, > SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, > @@ -181,13 +173,13 @@ static const struct flash_info atmel_parts[] = { > { "at26f004", INFO(0x1f0400, 0, 64 * 1024, 8, SECT_4K) }, > { "at26df081a", INFO(0x1f4501, 0, 64 * 1024, 16, > SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) > - .fixups = &atmel_global_protection_fixups }, > + .late_init = atmel_global_protection_late_init }, > { "at26df161a", INFO(0x1f4601, 0, 64 * 1024, 32, > SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) > - .fixups = &atmel_global_protection_fixups }, > + .late_init = atmel_global_protection_late_init }, > { "at26df321", INFO(0x1f4700, 0, 64 * 1024, 64, > SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) > - .fixups = &atmel_global_protection_fixups }, > + .late_init = atmel_global_protection_late_init }, > > { "at45db081d", INFO(0x1f2500, 0, 64 * 1024, 16, SECT_4K) }, > }; > -- > 2.25.1 >
Am 2021-08-16 21:06, schrieb Pratyush Yadav: > On 27/07/21 07:51AM, Tudor Ambarus wrote: >> Locking is not described in JESD216 SFDP standard, place the locking >> init in late_init(). Btw, we should differentiate between the block protection bits and individual block locking. At least the latter is described in the SFDP (I've seen it in the XTX SFDP, haven't checked the standard yet). > You are chaning the order of setting the locking ops here. Earlier, > they > were set before we parsed SFDP. Now they are set after we parse SFDP. > Though I don't see it making much of a difference. > >> >> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> >> --- >> drivers/mtd/spi-nor/atmel.c | 30 +++++++++++------------------- >> 1 file changed, 11 insertions(+), 19 deletions(-) >> >> diff --git a/drivers/mtd/spi-nor/atmel.c b/drivers/mtd/spi-nor/atmel.c >> index 1fea5cab492c..b937ef734e55 100644 >> --- a/drivers/mtd/spi-nor/atmel.c >> +++ b/drivers/mtd/spi-nor/atmel.c >> @@ -48,15 +48,11 @@ static const struct spi_nor_locking_ops >> atmel_at25fs_locking_ops = { >> .is_locked = atmel_at25fs_is_locked, >> }; >> >> -static void atmel_at25fs_default_init(struct spi_nor *nor) >> +static void atmel_at25fs_late_init(struct spi_nor *nor) >> { >> nor->params->locking_ops = &atmel_at25fs_locking_ops; >> } >> >> -static const struct spi_nor_fixups atmel_at25fs_fixups = { >> - .default_init = atmel_at25fs_default_init, >> -}; >> - >> /** >> * atmel_set_global_protection - Do a Global Protect or Unprotect >> command >> * @nor: pointer to 'struct spi_nor' >> @@ -146,34 +142,30 @@ static const struct spi_nor_locking_ops >> atmel_global_protection_ops = { >> .is_locked = atmel_is_global_protected, >> }; >> >> -static void atmel_global_protection_default_init(struct spi_nor *nor) >> +static void atmel_global_protection_late_init(struct spi_nor *nor) >> { >> nor->params->locking_ops = &atmel_global_protection_ops; >> } >> >> -static const struct spi_nor_fixups atmel_global_protection_fixups = { >> - .default_init = atmel_global_protection_default_init, >> -}; >> - >> static const struct flash_info atmel_parts[] = { >> /* Atmel -- some are (confusingly) marketed as "DataFlash" */ >> { "at25fs010", INFO(0x1f6601, 0, 32 * 1024, 4, SECT_4K | >> SPI_NOR_HAS_LOCK) >> - .fixups = &atmel_at25fs_fixups }, >> + .late_init = atmel_at25fs_late_init }, >> { "at25fs040", INFO(0x1f6604, 0, 64 * 1024, 8, SECT_4K | >> SPI_NOR_HAS_LOCK) >> - .fixups = &atmel_at25fs_fixups }, >> + .late_init = atmel_at25fs_late_init }, >> >> { "at25df041a", INFO(0x1f4401, 0, 64 * 1024, 8, >> SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) >> - .fixups = &atmel_global_protection_fixups }, >> + .late_init = atmel_global_protection_late_init }, > > Won't you be better off setting this in the manufacturer late_init()? > It > seems common for most atmel flashes. > > Of course, this would cause a problem for atmel flashes that don't have > this at all, since we would set locking for those as well. But I think > we can avoid that by checking for SNOR_F_HAS_LOCK in > spi_nor_register_locking_ops(). +1 -michael
On 9/10/21 12:44 AM, Michael Walle wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Am 2021-08-16 21:06, schrieb Pratyush Yadav: >> On 27/07/21 07:51AM, Tudor Ambarus wrote: >>> Locking is not described in JESD216 SFDP standard, place the locking >>> init in late_init(). > > Btw, we should differentiate between the block protection > bits and individual block locking. At least the latter is described > in the SFDP (I've seen it in the XTX SFDP, haven't checked the > standard yet). that's probably a vendor specific table, not something standardized by SFDP. > >> You are chaning the order of setting the locking ops here. Earlier, >> they >> were set before we parsed SFDP. Now they are set after we parse SFDP. >> Though I don't see it making much of a difference. Right, as the locking is not covered by SFDP, we should place it after parsing SFDP. >> >>> >>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> >>> --- >>> drivers/mtd/spi-nor/atmel.c | 30 +++++++++++------------------- >>> 1 file changed, 11 insertions(+), 19 deletions(-) >>> >>> diff --git a/drivers/mtd/spi-nor/atmel.c b/drivers/mtd/spi-nor/atmel.c >>> index 1fea5cab492c..b937ef734e55 100644 >>> --- a/drivers/mtd/spi-nor/atmel.c >>> +++ b/drivers/mtd/spi-nor/atmel.c >>> @@ -48,15 +48,11 @@ static const struct spi_nor_locking_ops >>> atmel_at25fs_locking_ops = { >>> .is_locked = atmel_at25fs_is_locked, >>> }; >>> >>> -static void atmel_at25fs_default_init(struct spi_nor *nor) >>> +static void atmel_at25fs_late_init(struct spi_nor *nor) >>> { >>> nor->params->locking_ops = &atmel_at25fs_locking_ops; >>> } >>> >>> -static const struct spi_nor_fixups atmel_at25fs_fixups = { >>> - .default_init = atmel_at25fs_default_init, >>> -}; >>> - >>> /** >>> * atmel_set_global_protection - Do a Global Protect or Unprotect >>> command >>> * @nor: pointer to 'struct spi_nor' >>> @@ -146,34 +142,30 @@ static const struct spi_nor_locking_ops >>> atmel_global_protection_ops = { >>> .is_locked = atmel_is_global_protected, >>> }; >>> >>> -static void atmel_global_protection_default_init(struct spi_nor *nor) >>> +static void atmel_global_protection_late_init(struct spi_nor *nor) >>> { >>> nor->params->locking_ops = &atmel_global_protection_ops; >>> } >>> >>> -static const struct spi_nor_fixups atmel_global_protection_fixups = { >>> - .default_init = atmel_global_protection_default_init, >>> -}; >>> - >>> static const struct flash_info atmel_parts[] = { >>> /* Atmel -- some are (confusingly) marketed as "DataFlash" */ >>> { "at25fs010", INFO(0x1f6601, 0, 32 * 1024, 4, SECT_4K | >>> SPI_NOR_HAS_LOCK) >>> - .fixups = &atmel_at25fs_fixups }, >>> + .late_init = atmel_at25fs_late_init }, >>> { "at25fs040", INFO(0x1f6604, 0, 64 * 1024, 8, SECT_4K | >>> SPI_NOR_HAS_LOCK) >>> - .fixups = &atmel_at25fs_fixups }, >>> + .late_init = atmel_at25fs_late_init }, >>> >>> { "at25df041a", INFO(0x1f4401, 0, 64 * 1024, 8, >>> SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) >>> - .fixups = &atmel_global_protection_fixups }, >>> + .late_init = atmel_global_protection_late_init }, >> >> Won't you be better off setting this in the manufacturer late_init()? >> It >> seems common for most atmel flashes. >> >> Of course, this would cause a problem for atmel flashes that don't have >> this at all, since we would set locking for those as well. But I think >> we can avoid that by checking for SNOR_F_HAS_LOCK in >> spi_nor_register_locking_ops(). > > +1 > we also have the atmel_at25fs_late_init() method. setting it per manufacturer will result in setting the manufacturer locking ops for at25fs as well, which will be overwritten by the at25fs locking ops. For those that don't support locking at all, we should clear the locking ops as you said. This will make the code a little difficult to follow and we return a bit to spaghetti. defining late_init() takes only a line anyway. I would keep the code as it is if you don't mind. We can ask ourselves about scalability when we have lots of entries, we can reevaluate this in the future. Tell me if you have strong opinions on this. Cheers, ta
Am 2021-10-01 13:40, schrieb Tudor.Ambarus@microchip.com: > On 9/10/21 12:44 AM, Michael Walle wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know >> the content is safe >> >> Am 2021-08-16 21:06, schrieb Pratyush Yadav: >>> On 27/07/21 07:51AM, Tudor Ambarus wrote: >>>> Locking is not described in JESD216 SFDP standard, place the locking >>>> init in late_init(). >> >> Btw, we should differentiate between the block protection >> bits and individual block locking. At least the latter is described >> in the SFDP (I've seen it in the XTX SFDP, haven't checked the >> standard yet). > > that's probably a vendor specific table, not something standardized by > SFDP. correct. >>> You are chaning the order of setting the locking ops here. Earlier, >>> they >>> were set before we parsed SFDP. Now they are set after we parse SFDP. >>> Though I don't see it making much of a difference. > > Right, as the locking is not covered by SFDP, we should place it after > parsing SFDP. > >>> >>>> >>>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> >>>> --- >>>> drivers/mtd/spi-nor/atmel.c | 30 +++++++++++------------------- >>>> 1 file changed, 11 insertions(+), 19 deletions(-) >>>> >>>> diff --git a/drivers/mtd/spi-nor/atmel.c >>>> b/drivers/mtd/spi-nor/atmel.c >>>> index 1fea5cab492c..b937ef734e55 100644 >>>> --- a/drivers/mtd/spi-nor/atmel.c >>>> +++ b/drivers/mtd/spi-nor/atmel.c >>>> @@ -48,15 +48,11 @@ static const struct spi_nor_locking_ops >>>> atmel_at25fs_locking_ops = { >>>> .is_locked = atmel_at25fs_is_locked, >>>> }; >>>> >>>> -static void atmel_at25fs_default_init(struct spi_nor *nor) >>>> +static void atmel_at25fs_late_init(struct spi_nor *nor) >>>> { >>>> nor->params->locking_ops = &atmel_at25fs_locking_ops; >>>> } >>>> >>>> -static const struct spi_nor_fixups atmel_at25fs_fixups = { >>>> - .default_init = atmel_at25fs_default_init, >>>> -}; >>>> - >>>> /** >>>> * atmel_set_global_protection - Do a Global Protect or Unprotect >>>> command >>>> * @nor: pointer to 'struct spi_nor' >>>> @@ -146,34 +142,30 @@ static const struct spi_nor_locking_ops >>>> atmel_global_protection_ops = { >>>> .is_locked = atmel_is_global_protected, >>>> }; >>>> >>>> -static void atmel_global_protection_default_init(struct spi_nor >>>> *nor) >>>> +static void atmel_global_protection_late_init(struct spi_nor *nor) >>>> { >>>> nor->params->locking_ops = &atmel_global_protection_ops; >>>> } >>>> >>>> -static const struct spi_nor_fixups atmel_global_protection_fixups = >>>> { >>>> - .default_init = atmel_global_protection_default_init, >>>> -}; >>>> - >>>> static const struct flash_info atmel_parts[] = { >>>> /* Atmel -- some are (confusingly) marketed as "DataFlash" */ >>>> { "at25fs010", INFO(0x1f6601, 0, 32 * 1024, 4, SECT_4K | >>>> SPI_NOR_HAS_LOCK) >>>> - .fixups = &atmel_at25fs_fixups }, >>>> + .late_init = atmel_at25fs_late_init }, >>>> { "at25fs040", INFO(0x1f6604, 0, 64 * 1024, 8, SECT_4K | >>>> SPI_NOR_HAS_LOCK) >>>> - .fixups = &atmel_at25fs_fixups }, >>>> + .late_init = atmel_at25fs_late_init }, >>>> >>>> { "at25df041a", INFO(0x1f4401, 0, 64 * 1024, 8, >>>> SECT_4K | SPI_NOR_HAS_LOCK | >>>> SPI_NOR_SWP_IS_VOLATILE) >>>> - .fixups = &atmel_global_protection_fixups }, >>>> + .late_init = atmel_global_protection_late_init }, >>> >>> Won't you be better off setting this in the manufacturer late_init()? >>> It >>> seems common for most atmel flashes. >>> >>> Of course, this would cause a problem for atmel flashes that don't >>> have >>> this at all, since we would set locking for those as well. But I >>> think >>> we can avoid that by checking for SNOR_F_HAS_LOCK in >>> spi_nor_register_locking_ops(). >> >> +1 >> > > we also have the atmel_at25fs_late_init() method. setting it per > manufacturer will result > in setting the manufacturer locking ops for at25fs as well, which will > be overwritten by the > at25fs locking ops. For those that don't support locking at all, we > should clear the locking > ops as you said. This will make the code a little difficult to follow > and we return a bit > to spaghetti. defining late_init() takes only a line anyway. I would > keep the code as it is > if you don't mind. We can ask ourselves about scalability when we have > lots of entries, > we can reevaluate this in the future. Tell me if you have strong > opinions on this. What about the following: int atmel_late_init(struct spi_nor *nor) { if (nor->info->flags & SPI_NOR_HAS_LOCK) nor->params->locking_ops = &atmel_global_protection_ops; } Of course it depends on wether we expect these ops to be the ones used for most atmel flashes. The at25fs would then overwrite the ops in their .late_init. In any case, I don't have a strong opinion either way. -michael
On 01/10/21 11:40AM, Tudor.Ambarus@microchip.com wrote: > On 9/10/21 12:44 AM, Michael Walle wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > Am 2021-08-16 21:06, schrieb Pratyush Yadav: > >> On 27/07/21 07:51AM, Tudor Ambarus wrote: > >>> Locking is not described in JESD216 SFDP standard, place the locking > >>> init in late_init(). > > > > Btw, we should differentiate between the block protection > > bits and individual block locking. At least the latter is described > > in the SFDP (I've seen it in the XTX SFDP, haven't checked the > > standard yet). > > that's probably a vendor specific table, not something standardized by SFDP. > > > > >> You are chaning the order of setting the locking ops here. Earlier, > >> they > >> were set before we parsed SFDP. Now they are set after we parse SFDP. > >> Though I don't see it making much of a difference. > > Right, as the locking is not covered by SFDP, we should place it after > parsing SFDP. > > >> > >>> > >>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> > >>> --- > >>> drivers/mtd/spi-nor/atmel.c | 30 +++++++++++------------------- > >>> 1 file changed, 11 insertions(+), 19 deletions(-) > >>> > >>> diff --git a/drivers/mtd/spi-nor/atmel.c b/drivers/mtd/spi-nor/atmel.c > >>> index 1fea5cab492c..b937ef734e55 100644 > >>> --- a/drivers/mtd/spi-nor/atmel.c > >>> +++ b/drivers/mtd/spi-nor/atmel.c > >>> @@ -48,15 +48,11 @@ static const struct spi_nor_locking_ops > >>> atmel_at25fs_locking_ops = { > >>> .is_locked = atmel_at25fs_is_locked, > >>> }; > >>> > >>> -static void atmel_at25fs_default_init(struct spi_nor *nor) > >>> +static void atmel_at25fs_late_init(struct spi_nor *nor) > >>> { > >>> nor->params->locking_ops = &atmel_at25fs_locking_ops; > >>> } > >>> > >>> -static const struct spi_nor_fixups atmel_at25fs_fixups = { > >>> - .default_init = atmel_at25fs_default_init, > >>> -}; > >>> - > >>> /** > >>> * atmel_set_global_protection - Do a Global Protect or Unprotect > >>> command > >>> * @nor: pointer to 'struct spi_nor' > >>> @@ -146,34 +142,30 @@ static const struct spi_nor_locking_ops > >>> atmel_global_protection_ops = { > >>> .is_locked = atmel_is_global_protected, > >>> }; > >>> > >>> -static void atmel_global_protection_default_init(struct spi_nor *nor) > >>> +static void atmel_global_protection_late_init(struct spi_nor *nor) > >>> { > >>> nor->params->locking_ops = &atmel_global_protection_ops; > >>> } > >>> > >>> -static const struct spi_nor_fixups atmel_global_protection_fixups = { > >>> - .default_init = atmel_global_protection_default_init, > >>> -}; > >>> - > >>> static const struct flash_info atmel_parts[] = { > >>> /* Atmel -- some are (confusingly) marketed as "DataFlash" */ > >>> { "at25fs010", INFO(0x1f6601, 0, 32 * 1024, 4, SECT_4K | > >>> SPI_NOR_HAS_LOCK) > >>> - .fixups = &atmel_at25fs_fixups }, > >>> + .late_init = atmel_at25fs_late_init }, > >>> { "at25fs040", INFO(0x1f6604, 0, 64 * 1024, 8, SECT_4K | > >>> SPI_NOR_HAS_LOCK) > >>> - .fixups = &atmel_at25fs_fixups }, > >>> + .late_init = atmel_at25fs_late_init }, > >>> > >>> { "at25df041a", INFO(0x1f4401, 0, 64 * 1024, 8, > >>> SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) > >>> - .fixups = &atmel_global_protection_fixups }, > >>> + .late_init = atmel_global_protection_late_init }, > >> > >> Won't you be better off setting this in the manufacturer late_init()? > >> It > >> seems common for most atmel flashes. > >> > >> Of course, this would cause a problem for atmel flashes that don't have > >> this at all, since we would set locking for those as well. But I think > >> we can avoid that by checking for SNOR_F_HAS_LOCK in > >> spi_nor_register_locking_ops(). > > > > +1 > > > > we also have the atmel_at25fs_late_init() method. setting it per manufacturer will result > in setting the manufacturer locking ops for at25fs as well, which will be overwritten by the > at25fs locking ops. For those that don't support locking at all, we should clear the locking > ops as you said. This will make the code a little difficult to follow and we return a bit > to spaghetti. defining late_init() takes only a line anyway. I would keep the code as it is > if you don't mind. We can ask ourselves about scalability when we have lots of entries, > we can reevaluate this in the future. Tell me if you have strong opinions on this. Okay, this should be fine I think. Looking at the Micron output driver strength patch has make me rethink manufacturer-wide settings in general. If we add manufacturer-wide settings, and then the manufacturer later change their minds and start using different settings for their newer flashes, we could easily run into a situation where half the flashes are just overwriting the manufacturer-wide default. > > Cheers, > ta > >
diff --git a/drivers/mtd/spi-nor/atmel.c b/drivers/mtd/spi-nor/atmel.c index 1fea5cab492c..b937ef734e55 100644 --- a/drivers/mtd/spi-nor/atmel.c +++ b/drivers/mtd/spi-nor/atmel.c @@ -48,15 +48,11 @@ static const struct spi_nor_locking_ops atmel_at25fs_locking_ops = { .is_locked = atmel_at25fs_is_locked, }; -static void atmel_at25fs_default_init(struct spi_nor *nor) +static void atmel_at25fs_late_init(struct spi_nor *nor) { nor->params->locking_ops = &atmel_at25fs_locking_ops; } -static const struct spi_nor_fixups atmel_at25fs_fixups = { - .default_init = atmel_at25fs_default_init, -}; - /** * atmel_set_global_protection - Do a Global Protect or Unprotect command * @nor: pointer to 'struct spi_nor' @@ -146,34 +142,30 @@ static const struct spi_nor_locking_ops atmel_global_protection_ops = { .is_locked = atmel_is_global_protected, }; -static void atmel_global_protection_default_init(struct spi_nor *nor) +static void atmel_global_protection_late_init(struct spi_nor *nor) { nor->params->locking_ops = &atmel_global_protection_ops; } -static const struct spi_nor_fixups atmel_global_protection_fixups = { - .default_init = atmel_global_protection_default_init, -}; - static const struct flash_info atmel_parts[] = { /* Atmel -- some are (confusingly) marketed as "DataFlash" */ { "at25fs010", INFO(0x1f6601, 0, 32 * 1024, 4, SECT_4K | SPI_NOR_HAS_LOCK) - .fixups = &atmel_at25fs_fixups }, + .late_init = atmel_at25fs_late_init }, { "at25fs040", INFO(0x1f6604, 0, 64 * 1024, 8, SECT_4K | SPI_NOR_HAS_LOCK) - .fixups = &atmel_at25fs_fixups }, + .late_init = atmel_at25fs_late_init }, { "at25df041a", INFO(0x1f4401, 0, 64 * 1024, 8, SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) - .fixups = &atmel_global_protection_fixups }, + .late_init = atmel_global_protection_late_init }, { "at25df321", INFO(0x1f4700, 0, 64 * 1024, 64, SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) - .fixups = &atmel_global_protection_fixups }, + .late_init = atmel_global_protection_late_init }, { "at25df321a", INFO(0x1f4701, 0, 64 * 1024, 64, SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) - .fixups = &atmel_global_protection_fixups }, + .late_init = atmel_global_protection_late_init }, { "at25df641", INFO(0x1f4800, 0, 64 * 1024, 128, SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) - .fixups = &atmel_global_protection_fixups }, + .late_init = atmel_global_protection_late_init }, { "at25sl321", INFO(0x1f4216, 0, 64 * 1024, 64, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, @@ -181,13 +173,13 @@ static const struct flash_info atmel_parts[] = { { "at26f004", INFO(0x1f0400, 0, 64 * 1024, 8, SECT_4K) }, { "at26df081a", INFO(0x1f4501, 0, 64 * 1024, 16, SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) - .fixups = &atmel_global_protection_fixups }, + .late_init = atmel_global_protection_late_init }, { "at26df161a", INFO(0x1f4601, 0, 64 * 1024, 32, SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) - .fixups = &atmel_global_protection_fixups }, + .late_init = atmel_global_protection_late_init }, { "at26df321", INFO(0x1f4700, 0, 64 * 1024, 64, SECT_4K | SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE) - .fixups = &atmel_global_protection_fixups }, + .late_init = atmel_global_protection_late_init }, { "at45db081d", INFO(0x1f2500, 0, 64 * 1024, 16, SECT_4K) }, };
Locking is not described in JESD216 SFDP standard, place the locking init in late_init(). Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> --- drivers/mtd/spi-nor/atmel.c | 30 +++++++++++------------------- 1 file changed, 11 insertions(+), 19 deletions(-)