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

[Miquel Raynal]

Hi Tudor,

tudor.ambarus at 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 at 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 at 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.
> 
> 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).

The flag is set, no need for hardcoding it.

Excellent, I'll prepare the patch.

While I was testing I figured out: the bank size calculation happens
too early, so if we discover the size with the SFDP table, that won't
work. I'll fix it, feel free to squash the patch if you prefer.

> 
> So I'd try:
> { "mx25uw51245g", INFOB(0xc2813a, 0, 0, 0, 4)
> 		PARSE_SFDP
> 		FLAGS(SPI_NOR_RWW) },
> 
> 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?
> 
> Cheers,
> ta


Thanks,
Miquèl