Re: [PATCH v5 8/8] mtd: spi-nor: macronix: Add support for mx25uw51245g with RWW

From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Tudor Ambarus <tudor.ambarus@linaro.org>
Cc: liao jaime <jaimeliao.tw@gmail.com>,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Pratyush Yadav <pratyush@kernel.org>,
	Michael Walle <michael@walle.cc>,
	linux-mtd@lists.infradead.org, Julien Su <juliensu@mxic.com.tw>,
	Jaime Liao <jaimeliao@mxic.com.tw>,
	Alvin Zhou <alvinzhou@mxic.com.tw>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Michal Simek <monstr@monstr.eu>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v5 8/8] mtd: spi-nor: macronix: Add support for mx25uw51245g with RWW
Date: Wed, 29 Mar 2023 13:54:42 +0200	[thread overview]
Message-ID: <20230329135442.20669531@xps-13> (raw)
In-Reply-To: <f243010c-72e3-6b59-6696-af7c7cdc6596@linaro.org>

Hi Tudor,

tudor.ambarus@linaro.org wrote on Wed, 29 Mar 2023 12:12:29 +0100:

> On 3/29/23 06:48, liao jaime wrote:
> > Hi Miquel
> >   
> >>
> >> Hi Tudor,
> >>
> >> tudor.ambarus@linaro.org wrote on Tue, 28 Mar 2023 17:31:46 +0100:
> >>  
> >>> On 3/28/23 16:41, Miquel Raynal wrote:  
> >>>> Describe this new part and provide the RWW flag for it.
> >>>>
> >>>> There is no public datasheet, but here are the sfdp tables plus base
> >>>> testing to show it works.
> >>>>
> >>>> mx25uw51245g
> >>>> c2813a
> >>>> macronix
> >>>> 53464450080104fd00070114400000ff8701011c900000ff0a0001080001
> >>>> 00ff05000105200100ff84000102340100ff0000000000000000ffffffff
> >>>> ffffffffe5208affffffff1f00ff00ff00ff00ffeeffffffffff00ffffff
> >>>> 00ff0c2010d800ff00ff87790100821200e27704674630b030b0f4bdd55c
> >>>> 000000ff101000200000000000007ca14800000000008888000000000000
> >>>> 00400fd1fff30fd1fff300050090000500b1002b0095002b0096727103b8
> >>>> 727103b80000000090a3188200c069960000000000000000727100987271
> >>>> 00b8727100990000000072710098727100f872710099727100f900000000
> >>>> 00000000011501d0727106d8000086500000060100000000020001030002
> >>>> 00000000060100000000000072060002000000eec0697272717100d8f7f6
> >>>> 000a00001445988043060f0021dcffff
> >>>> 047a884cf44d9ffc2a94d3ab37b48c63  /sys/bus/spi/devices/spi0.0/spi-nor/sfdp  
> >>>
> >>> which hash alg was used, was it sha1? No need to resubmnit just for this.  
> >>
> >> Actually git just ignored all the lines starting with '#'
> >> like if they were comments and I did not notice...
> >>
> >> I will amend my commit logs and resend.
> >>  
> >>>>
> >>>> 6+0 records in
> >>>> 6+0 records out
> >>>> Copied 6291456 bytes from qspi_test to address 0x00000000 in flash
> >>>> Erased 6291456 bytes from address 0x00000000 in flash
> >>>> Copied 6291456 bytes from address 0x00000000 in flash to qspi_read
> >>>> 0000000 ffff ffff ffff ffff ffff ffff ffff ffff
> >>>> *
> >>>> 0600000
> >>>> Copied 6291456 bytes from qspi_test to address 0x00000000 in flash
> >>>> Copied 6291456 bytes from address 0x00000000 in flash to qspi_read
> >>>> d24a9523db829a0df688f34b8dc76a1383b74024  qspi_test
> >>>> d24a9523db829a0df688f34b8dc76a1383b74024  qspi_read
> >>>>
> >>>> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> >>>> ---
> >>>>  drivers/mtd/spi-nor/macronix.c | 4 ++++
> >>>>  1 file changed, 4 insertions(+)
> >>>>
> >>>> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> >>>> index 6853ec9ae65d..b65d41e5f749 100644
> >>>> --- a/drivers/mtd/spi-nor/macronix.c
> >>>> +++ b/drivers/mtd/spi-nor/macronix.c
> >>>> @@ -82,6 +82,10 @@ static const struct flash_info macronix_nor_parts[] = {
> >>>>     { "mx25u51245g", INFO(0xc2253a, 0, 64 * 1024, 1024)
> >>>>             NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> >>>>             FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
> >>>> +   { "mx25uw51245g", INFOB(0xc2813a, 0, 16 * 1024, 1024, 4)  
> >>>
> >>> Sector size and nsectors shall be discovered at parse SFDP time.
> >>> Does INFOB(0xc2813a, 0, 0, 0, 4) work for you?  
> >>
> >> I'll check.
> >>  
> >>>  
> >>>> +           PARSE_SFDP  
> >>>
> >>> Yaay
> >>>  
> >>>> +           FLAGS(SPI_NOR_RWW)
> >>>> +           FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },  
> >>>
> >>> Is SPI_NOR_4B_OPCODES flag really needed?  
> >>
> >> I have no idea, I took that from older code.
> >>
> >> Jaime, what do you think?  
> > I think Tudor hopes that SFDP table could determine how many address bytes.  
> 
> No, no.
> 
> :) and hope and programming don't get along.
> 
> Does this flash define the 4bait table? Can you add a print in
> parse_4bait table and check if SNOR_F_4B_OPCODES is set? If yes, the
> fixup flag is not needed.

Alright, will check.

> 
> SNOR_F_4B_OPCODES is an indication that the flash supports dedicated
> opcodes for 4byte address, without the need to change the address mode
> to 4 byte. When 4B_OPCODES are supported, we prefer not changing the
> address mode, because when changing to 4 byte address mode, the state is
> kept at reset (or power loss) and flashes might not be able to boot at
> lower layers (u-boot for example).

Oh yeah I remember.

> 
> So I'd try:
> { "mx25uw51245g", INFOB(0xc2813a, 0, 0, 0, 4)
> 		PARSE_SFDP
> 		FLAGS(SPI_NOR_RWW) },

Clear!

> 
> In the future we'd like to add a sfdp-decoder in mtd-utils to decode the
> sfdp dump and ease the review and avoid such questions, but we couldn't
> allocate the time to implement it up to now. Meh, we're doing it at best
> effort, in our spare time. Volunteers?

I really like the idea! Would be excellent.

Thanks,
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel