[PATCH v4 1/2] mtd: spi-nor: Added spi-nor init function
Kamal Dasu
kamal.dasu at broadcom.com
Sun Feb 19 14:38:20 PST 2017
Marek,
On Sun, Feb 19, 2017 at 5:37 AM, Marek Vasut <marex at denx.de> wrote:
> On 02/19/2017 10:47 AM, Kamal Dasu wrote:
>> On Sat, Feb 18, 2017 at 5:29 PM, Marek Vasut <marex at denx.de> wrote:
>>> On 02/14/2017 04:32 PM, Kamal Dasu wrote:
>>>> Refactored spi_nor_scan() code to add spi_nor_init() function that
>>>> programs the spi-nor flash to:
>>>> 1) remove flash protection if applicable
>>>> 2) set read mode to quad mode if configured such
>>>> 3) set the address width based on the flash size and vendor
>>>>
>>>> spi_nor_scan() now calls spi_nor_init() to setup nor flash.
>>>>
>>>> Signed-off-by: Kamal Dasu <kdasu.kdev at gmail.com>
>>>
>>> Changelog missing ?
>>
>> Yes will add it and resend.
>>
>>>
>>>> ---
>>>> drivers/mtd/spi-nor/spi-nor.c | 72 +++++++++++++++++++++++++++----------------
>>>> include/linux/mtd/spi-nor.h | 18 ++++++++++-
>>>> 2 files changed, 62 insertions(+), 28 deletions(-)
>>>>
>>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>>>> index 70e52ff..2bf7f4f 100644
>>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>>> @@ -1526,6 +1526,44 @@ static int s3an_nor_scan(const struct flash_info *info, struct spi_nor *nor)
>>>> return 0;
>>>> }
>>>>
>>>> +int spi_nor_init(struct spi_nor *nor)
>>>> +{
>>>> + int ret = 0;
>>>> + const struct flash_info *info = nor->info;
>>>> + struct device *dev = nor->dev;
>>>> +
>>>> + /*
>>>> + * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
>>>> + * with the software protection bits set
>>>> + */
>>>> +
>>>> + if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
>>>> + JEDEC_MFR(info) == SNOR_MFR_INTEL ||
>>>> + JEDEC_MFR(info) == SNOR_MFR_SST ||
>>>> + info->flags & SPI_NOR_HAS_LOCK) {
>>>> + write_enable(nor);
>>>> + write_sr(nor, 0);
>>>> + spi_nor_wait_till_ready(nor);
>>>> + }
>>>> +
>>>> + if (nor->flash_read == SPI_NOR_QUAD) {
>>>> + ret = set_quad_mode(nor, info);
>>>> + if (ret) {
>>>> + dev_err(dev, "quad mode not supported\n");
>>>> + return ret;
>>>> + }
>>>> + }
>>
>> quad mode is being set in the above block of code.
>>
>>>> +
>>>> + if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
>>>> + info->flags & SPI_NOR_4B_OPCODES)
>>>> + spi_nor_set_4byte_opcodes(nor, info);
>>>> + else
>>>> + set_4byte(nor, info, 1);
>>>> +
>>>> + return ret;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(spi_nor_init);
>>>> +
>>>> int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>>>> {
>>>> const struct flash_info *info = NULL;
>>>> @@ -1571,6 +1609,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>>>> }
>>>> }
>>>>
>>>> + nor->info = info;
>>>> mutex_init(&nor->lock);
>>>>
>>>> /*
>>>> @@ -1581,20 +1620,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>>>> if (info->flags & SPI_S3AN)
>>>> nor->flags |= SNOR_F_READY_XSR_RDY;
>>>>
>>>> - /*
>>>> - * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
>>>> - * with the software protection bits set
>>>> - */
>>>> -
>>>> - if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
>>>> - JEDEC_MFR(info) == SNOR_MFR_INTEL ||
>>>> - JEDEC_MFR(info) == SNOR_MFR_SST ||
>>>> - info->flags & SPI_NOR_HAS_LOCK) {
>>>> - write_enable(nor);
>>>> - write_sr(nor, 0);
>>>> - spi_nor_wait_till_ready(nor);
>>>> - }
>>>> -
>>>> if (!mtd->name)
>>>> mtd->name = dev_name(dev);
>>>> mtd->priv = nor;
>>>> @@ -1669,11 +1694,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>>>>
>>>> /* Quad/Dual-read mode takes precedence over fast/normal */
>>>> if (mode == SPI_NOR_QUAD && info->flags & SPI_NOR_QUAD_READ) {
>>>> - ret = set_quad_mode(nor, info);
>>>> - if (ret) {
>>>> - dev_err(dev, "quad mode not supported\n");
>>>> - return ret;
>>>> - }
>>>> nor->flash_read = SPI_NOR_QUAD;
>>>
>>> So from here on, any user will expect quad mode to be correctly set, but
>>> it really isn't. Is that OK ?
>>
>> It is being set in spi_nor_init() based on the nor->flash_read == SPI_NOR_QUAD;
>>
>
> But that's invoked later, so whoever adds code between this place and
> the invocation of spi_nor_init() will be misled into believing the SPI
> NOR is set up in quad-mode, while it would not be, right ?
>
I made this change based on previous comments from Cyrlle. All
spi-nor based settings that need commands to the flash happens in one
function now. The code before calling spi_nor_init is setting the
configuration in the nor structure. I don't see how its very different
from before. I did separate each setting in functions without changing
the flow in spi_nor_scan() before and was to told to change it this
way to be more optimal. I don't see how anyone will have any
confusion. So please let me know how exactly you want it.
> [...]
>
> --
> Best regards,
> Marek Vasut
More information about the linux-mtd
mailing list