RE: [PATCH] mtd: spi-nor: spansion: Add support for CY15V104QSN

[LECOINTRE Philippe <philippe.lecointre@thalesgroup.com>]

Classified as: {OPEN}

Hello,


{OPEN}

> -----Original Message-----
> From: Tudor Ambarus <tudor.ambarus@linaro.org>
> Sent: Thursday, March 20, 2025 9:47 AM
> To: LECOINTRE Philippe <philippe.lecointre@thalesgroup.com>; Pratyush
> Yadav <pratyush@kernel.org>; Michael Walle <mwalle@kernel.org>; linux-
> mtd@lists.infradead.org; Takahiro Kuwano
> <Takahiro.Kuwano@infineon.com>
> Cc: LEJEUNE Sebastien <sebastien.lejeune@thalesgroup.com>; LENAIN
> Simon <simon.lenain@thalesgroup.com>; RENAULT Xavier
> <xavier.renault@thalesgroup.com>
> Subject: Re: [PATCH] mtd: spi-nor: spansion: Add support for CY15V104QSN
> 
> 
> 
> On 3/19/25 3:02 PM, LECOINTRE Philippe wrote:
> > Classified as: {OPEN}
> >
> > Hello,
> 
> Hi,
> 
> Don't top post please, it's hard to follow the emails exchanged.
> >
> > Thank you for your feedback.
> 
> you're welcome!
> 
> >
> > I encounter some issue attempting to use the at25 driver.
> > I am not able to read the flash ID with at25 driver because the chipselect
> appear to be deselected between the RDID command and the read data. The
> RDID command is not taken into account and the chip only reply with 0xff.
> > With at25 driver, it use a spi_message containing two spi_transfer, one
> spi_transfer for the command and one spi_transfer for the read data. For
> some reason, this end up in .transfer_one() in the spi_controller driver of my
> SoC.
> 
> What spi controller driver, is it upstreamed?

The SoC use the upstream "snps,dw-apb-ssi" spi controller driver.

> 
> > With spi-nor driver, it use spi_mem_exec_op() which work well in my case.
> > It appear at25 driver can currently be used with Cypress FM25 chip and
> from some of the datasheet of this family it seems that the chipselect need
> to be driven low during the entire RDID sequence.
> > Am I missing something here ?
> 
> Splitting opcode, address, dummy and data per spi_transfers shall work,
> spi_mem_exec_op() does it too when the controller does not define
> mem_ops.
> 
> It probably just a bug somewhere, raising CS up between the transfers shall
> be fine.

Using an oscilloscope with spi protocol decode, it is the big difference I can see.

> >
> > Currently, the at25 driver assume a lot of property from the ID data
> (manufacturer and size) which don't seems to be relevant with the
> CY15V104QSN model.
> > This chip is similar to cy15b104q and cy15v104q currently supported by spi-
> nor driver.
> > https://web.git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/c
> > ommit/drivers/mtd/spi-
> nor/spansion.c?id=8a2644d5f3608822925c9204a3d19a
> > 8e3025fd4a
> 
> having those flashes in SPI NOR was a mistake. We can't just drop them as
> we'll break backward compatibility.
> 
> > Also, I don't have access to at25 spi eeprom to validate any deep
> modifications on this driver if we need to do so according to the thread you
> are pointing to.
> >
> > How can we handle this ?
> 
> You need to figure out why your flash doesn't work as expected and fix it. Is
> your controller spimem capable? If yes, try dropping the ctrl mem_ops and
> see if it behaves sane using SPI NOR and the controller using spi transfers. If
> it doesn't behave sane, then there's probably a problem into the controller
> driver.

Good advice !
I have commented out the dw_spi_init_mem_ops() function call in dw_spi_add_host() and I end up with the same result as at25 driver.
spi-nor spi3.0: unrecognized JEDEC id bytes: ff ff ff ff ff ff

I need to investigate further on this issue.
The drawback is that our project is currently based on Linux 6.1 CIP kernel and I need some efforts to rebase on the current master.
Until now, it wasn't an issue regarding the complexity of spi-nor modifications of the initial contribution.
From what I see, there is not much changes on the spi controller drivers, probably more in the spi core.
If you have any other advice, I will be glad to have them.

Regards,
Philippe

> 
> Cheers,
> ta
> >
> > Regards,
> > Philippe
> >
> >
> > {OPEN}
> >
> >> -----Original Message-----
> >> From: Tudor Ambarus <tudor.ambarus@linaro.org>
> >> Sent: Thursday, March 6, 2025 11:54 AM
> >> To: LECOINTRE Philippe <philippe.lecointre@thalesgroup.com>; Pratyush
> >> Yadav <pratyush@kernel.org>; Michael Walle <mwalle@kernel.org>;
> >> linux- mtd@lists.infradead.org
> >> Cc: LEJEUNE Sebastien <sebastien.lejeune@thalesgroup.com>; LENAIN
> >> Simon <simon.lenain@thalesgroup.com>; RENAULT Xavier
> >> <xavier.renault@thalesgroup.com>
> >> Subject: Re: [PATCH] mtd: spi-nor: spansion: Add support for
> >> CY15V104QSN
> >>
> >>
> >>
> >> On 2/28/25 2:01 PM, LECOINTRE Philippe wrote:
> >>> Infineon CY15V104QSN is 4Mbit serial SPI F-RAM device.
> >>
> >> Please consider moving this flash to the at25 EEPROM driver:
> >> https://lore.kernel.org/linux-mtd/20240604074231.1874972-1-
> >> mwalle@kernel.org/
> >>
> >> Cheers,
> >> ta
> >>>
> >>> Signed-off-by: Philippe Lecointre
> >>> <philippe.lecointre@thalesgroup.com>
> >>> Acked-by: Simon Lenain <simon.lenain@thalesgroup.com>
> >>> ---
> >>>  drivers/mtd/spi-nor/spansion.c | 6 ++++++
> >>>  1 file changed, 6 insertions(+)
> >>>
> >>> diff --git a/drivers/mtd/spi-nor/spansion.c
> >>> b/drivers/mtd/spi-nor/spansion.c index bf08dbf5e742..32f8892d7afe
> >>> 100644
> >>> --- a/drivers/mtd/spi-nor/spansion.c
> >>> +++ b/drivers/mtd/spi-nor/spansion.c
> >>> @@ -922,6 +922,12 @@ static const struct flash_info
> >>> spansion_nor_parts[]
> >> = {
> >>>  		.size = SZ_512K,
> >>>  		.sector_size = SZ_512K,
> >>>  		.flags = SPI_NOR_NO_ERASE,
> >>> +	}, {
> >>> +		.id = SNOR_ID(0x50, 0x51, 0x80, 0x06, 0x00, 0x00),
> >>> +		.name = "cy15v104qsn",
> >>> +		.size = SZ_512K,
> >>> +		.sector_size = SZ_512K,
> >>> +		.flags = SPI_NOR_NO_ERASE,
> >>>  	}, {
> >>>  		.id = SNOR_ID(0x34, 0x2a, 0x1a, 0x0f, 0x03, 0x90),
> >>>  		.name = "s25hl512t",
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/