[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <925723a2-dc2d-9949-5b8f-4351b49d2f4b@microchip.com>
Date: Tue, 26 Sep 2023 14:59:08 +0200
From: Nicolas Ferre <nicolas.ferre@...rochip.com>
To: Tudor Ambarus <tudor.ambarus@...aro.org>,
<miquel.raynal@...tlin.com>, <linux-mtd@...ts.infradead.org>
CC: <linux-arm-kernel@...ts.infradead.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mtd: spi-nor: atmel: add at25ff321a entry
On 21/09/2023 at 09:14, Tudor Ambarus wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On 9/20/23 17:16, Nicolas Ferre wrote:
>> On 19/09/2023 at 18:12, Tudor Ambarus wrote:
>>> On 08.09.2023 18:14, nicolas.ferre@...rochip.com wrote:
>>>> From: Nicolas Ferre <nicolas.ferre@...rochip.com>
>>>>
>>>> Add the at25ff321a 4MB SPI flash which is able to provide
>>>> SFDP informations.
>>>> Datasheet:
>>>> https://www.renesas.com/us/en/document/dst/at25ff321a-datasheet
>>>>
>>>> Signed-off-by: Nicolas Ferre <nicolas.ferre@...rochip.com>
>>>> ---
>>
>> [..]
>>
>>>> drivers/mtd/spi-nor/atmel.c | 4 ++++
>>>> 1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/drivers/mtd/spi-nor/atmel.c b/drivers/mtd/spi-nor/atmel.c
>>>> index 58968c1e7d2f..c94d52951481 100644
>>>> --- a/drivers/mtd/spi-nor/atmel.c
>>>> +++ b/drivers/mtd/spi-nor/atmel.c
>>>> @@ -184,6 +184,10 @@ static const struct flash_info atmel_nor_parts[]
>>>> = {
>>>> FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE)
>>>> NO_SFDP_FLAGS(SECT_4K)
>>>> .fixups = &atmel_nor_global_protection_fixups },
>>>> + { "at25ff321a", INFO(0x1f4708, 0, 64 * 1024, 64)
>>>> + FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE)
>>>> + PARSE_SFDP
>>>> + .fixups = &atmel_nor_global_protection_fixups },
>>>
>>> We have recently changed how the flash entries are defined. Would you
>>> please try the following changes instead?
>>>
>>> diff --git a/drivers/mtd/spi-nor/atmel.c b/drivers/mtd/spi-nor/atmel.c
>>> index 95f0e139284e..44218716d81e 100644
>>> --- a/drivers/mtd/spi-nor/atmel.c
>>> +++ b/drivers/mtd/spi-nor/atmel.c
>>> @@ -213,6 +213,12 @@ static const struct flash_info atmel_nor_parts[] = {
>>> .flags = SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE,
>>> .no_sfdp_flags = SECT_4K,
>>> .fixups = &atmel_nor_global_protection_fixups
>>> + }, {
>>> + .id = SNOR_ID(0x1f, 0x47, 0x08),
>>> + .name = "at25ff321a",
>>> + .flags = SPI_NOR_HAS_LOCK | SPI_NOR_SWP_IS_VOLATILE,
>>
>> Here, I added:
>>
>> .no_sfdp_flags = SECT_4K,
>>
>> to match the other devices of the family... I checked on the datasheet,
>> 4K sectors are okay, but I don't know exactly if this is eligible to the
>> "no_sfdb_flags" property... forgive me, I didn't check further knowing
>> that you might have better view on this than me ;-)
>
> yeah, no worries. You should not specify 4k here, as it should already
> be set when parsing SFDP. You can use mtdinfo /dev/mtdx to check what
> erase size gets set. In the core we set the maximum supported erase size
> for speed considerations and making ubifs happy, so you'll probably see
> 64k when checking with mtdinfo. But you can try 4k erases as well by
> setting MTD_SPI_NOR_USE_4K_SECTORS.
Absolutely, here is what I'm experiencing:
# no_sfdp_flags not set / linux-next / sama5d27 wlsom1 ek
# CONFIG_MTD_SPI_NOR_USE_4K_SECTORS=y
root@...a5d27-wlsom1-ek-sd:~# mtdinfo /dev/mtd0
mtd0
Name: spi0.1
Type: nor
Eraseblock size: 4096 bytes, 4.0 KiB
Amount of eraseblocks: 1024 (4194304 bytes, 4.0 MiB)
Minimum input/output unit size: 1 byte
Sub-page size: 1 byte
Character device major/minor: 90:0
Bad blocks are allowed: false
Device is writable: true
# no_sfdp_flags not set / linux-next / sama5d27 wlsom1 ek
# # CONFIG_MTD_SPI_NOR_USE_4K_SECTORS is not set
root@...a5d27-wlsom1-ek-sd:~# mtdinfo /dev/mtd0
mtd0
Name: spi0.1
Type: nor
Eraseblock size: 65536 bytes, 64.0 KiB
Amount of eraseblocks: 64 (4194304 bytes, 4.0 MiB)
Minimum input/output unit size: 1 byte
Sub-page size: 1 byte
Character device major/minor: 90:0
Bad blocks are allowed: false
Device is writable: true
>>> + .fixups = &atmel_nor_global_protection_fixups
>>> + }, {
>>> }, {
>>> .id = SNOR_ID(0x1f, 0x48, 0x00),
>>> .name = "at25df641",
>>
>> That works perfectly well (I can re-post the test results as my previous
>> patch if needed): do you want me to send the updated patch with your
>> Suggested-by tag or you can send yours, tell me what you prefer.
>
> Send a new patch please, it was just a suggestion to speed the things up.
Sure, I'm on it ;-)
>> Thanks for the heads-up on this update that I hadn't noticed. Best regards,
>> Nicolas
>>
> you're welcome! Cheers!
Best regards,
Nicolas
Powered by blists - more mailing lists