Message-ID: <CANeKEMMRuFXiqQ6uHuo47qai7w5E_1KdTht_rNczPdOHsEcpfg@mail.gmail.com>
Date: Fri, 16 Aug 2024 13:29:24 +0200
From: Erez <erezgeva2@...il.com>
To: Esben Haabendal <esben@...nix.com>
Cc: Michael Walle <mwalle@...nel.org>, Tudor Ambarus <tudor.ambarus@...aro.org>, 
	Jaime Liao <jaimeliao@...c.com.tw>, leoyu@...c.com.tw, 
	Alvin Zhou <alvinzhou@...c.com.tw>, Julien Su <juliensu@...c.com.tw>, 
	Erez Geva <erezgeva@...ime.org>, linux-mtd@...ts.infradead.org, 
	Pratyush Yadav <pratyush@...nel.org>, linux-kernel@...r.kernel.org, 
	Miquel Raynal <miquel.raynal@...tlin.com>, Richard Weinberger <richard@....at>, 
	Vignesh Raghavendra <vigneshr@...com>, devicetree@...r.kernel.org, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>
Subject: Re: [PATCH v2 3/4] dt-bindings: mtd: macronix,mx25l12833f: add
 SPI-NOR chip

Hi,

I now try to test the OTP code using a Mactronix MX25L3233F with
Beagle bone black.
I manage to connect using a SOP DIP and connect it to an spi port using SPIDEV.
And I use the flashprog tool from https://flashprog.org.

That is just the background of my environment.
Now to the point, it seems flashprog.org does follow Mactronix.
The same JEDEC "C2 20 16" used for 4 different chips:

# ./flashprog -p linux_spi:dev=/dev/spidev0.0,spispeed=512 --flash-name

flashprog v1.1-dirty on Linux 6.6.32-ti-arm32-r5 (armv7l)
flashprog is free software, get the source code at https://flashprog.org

Using clock_gettime for delay loops (clk_id: 1, resolution: 1ns).
Found Macronix flash chip "MX25L3205(A)" (4096 kB, SPI) on linux_spi.
Found Macronix flash chip "MX25L3205D/MX25L3208D" (4096 kB, SPI) on linux_spi.
Found Macronix flash chip "MX25L3206E/MX25L3208E" (4096 kB, SPI) on linux_spi.
Found Macronix flash chip "MX25L3233F/MX25L3273E" (4096 kB, SPI) on linux_spi.
Multiple flash chip definitions match the detected chip(s):
"MX25L3205(A)", "MX25L3205D/MX25L3208D", "MX25L3206E/MX25L3208E",
"MX25L3233F/MX25L3273E"
Please specify which chip definition to use with the -c <chipname> option.

Now for OTP, looking at Mactronix data sheets:
MX25L3205(A) does not have SFDP and no OTP
MX25L3205D does not have SFDP, uses 2 asymmetric OTP banks of 128-bit
and 384-bit.
MX25L3206E supports SFDP, uses 2 asymmetric OTP banks of 128-bit and 384-bit.
MX25L3233F supports SFDP, uses 1 bank of 4096-bit.

New Mactronix chips also use symmetric OTP banks:
For example, a chip I tested before with OTP:
MX25L12833F with a JEDEC "C2 20 18" supports SFDP, uses 2 symmetric
OTP banks of 4096-bit.


My conclusions are:
1. The best way to support Mactronix chips is to read the SFDP.
    The table with IDs is relevant only to the old chips that do not
support SFDP.
2. The OTP parameters need to be provided via device tree or sysfs.
     We can not find them on SFDP nor can we deduce them from JEDEC ID
with conjunction of having SFDP or not.
     Sadly, we need to enter them on a hardware basis.
     As OTP support in driver is for symmetric only.
     We are talking about 2 parameters. bank size and number of banks
as Mactronix offset is always equal to bank size.

Erez




On Fri, 12 Jul 2024 at 22:16, Erez <erezgeva2@...il.com> wrote:
>
>
>
> On Fri, 12 Jul 2024 at 10:21, Esben Haabendal <esben@...nix.com> wrote:
>>
>> Erez <erezgeva2@...il.com> writes:
>>
>> > On Thu, 11 Jul 2024 at 21:57, Michael Walle <mwalle@...nel.org> wrote:
>> >> On Thu Jul 11, 2024 at 8:57 PM CEST, Erez wrote:
>> >> > Yes, I think we should.
>> >> >
>> >> > Reading the specification provided publicly by Macronix.
>> >> > For all the JEDEC IDs with the no SFDP flag in drivers/mtd/spi-nor/macronix.c
>> >> > All of them have a new version or a new chip with the same JEDEC ID
>> >> > that supports SFDP.
>> >> > There are 2 chips that Macronix does not provide spec. in public.
>> >> > I can ask Macronix technical support on these 2 chips.
>> >>
>> >> We don't add flashes we cannot test.
>> >
>> > I did not suggest adding anything new.
>> > I refer to the list of chips we already have in drivers/mtd/spi-nor/macronix.c
>> > I presume someone tested them before adding them to the list in the past.
>> > And probably the old chip did not have the SFDP table back then.
>> >
>> > What I checked with the chip specifications is that all Macronix chips
>> > since 2010 have SFDP.
>> >
>> > The situation today is that all Macronix chips that are NOT in the
>> > Macronix table work based on the SFDP table.
>> > But new chips that use a JEDEC found in the Macronix table, skip the
>> > SFDP table and use the setting of the old chip.
>>
>> Not entirely true.
>>
>> Those that entries in the Macronix table that has SPI_NOR_DUAL_READ
>> and/or SPI_NOR_QUAD_READ in no_sfdp_flags is caught by
>> the magic flags matching in spi_nor_init_params_deprecated() and will
>> have spi_nor_parse_sfdp() called from
>> spi_nor_sfdp_init_params_deprecated(). So flashes reusing ID for these
>> will have the SFDP tables parsed.
>>
>> The rest of the entries in the Macronix table is not so lucky. When a
>> replacement chip (with the same ID) is used, it will not be configured
>> with the values found in SFDP table.
>>
>> > So I suggest we read the SFDP table for all Macronix chips.
>>
>> Based on their strategy of re-using flash ID, I think this is a sane
>> approach.
>>
>> > Old Macronix chips that do not have SFDP will use the setting from the
>> > Macronix table. i.e backward compatible.
>> > While new chips which do have an SFDP table will work with the new
>> > setting we find in the table.
>>
>> Yes, if we apply the new SPI_NOR_TRY_SFDP flag to the matching table
>> entries.
>>
>> > Of course, we might have issues in parsing the SFDP table itself.
>> > So we fix them as developers report and send chip ID and part number
>> > with the SFDP table content.
>> > I do not see the point of "hiding" with the old setting.
>> > Anyhow, as we do not like the IDs table and keep it for backward-compatible,
>> > so it only makes sense we should use the SFDP table as much as possible.
>> >
>> > My check was to ensure Tudor that all Macronix chips have SFDP whether
>> > they are in the IDs table or not and we are not wasting a no-op on a
>> > chip which can not have an SFDP table.
>> >
>> > All I suggest is we add the new 'SPI_NOR_TRY_SFDP' flag, to all
>> > Macronix chips.. Which will try to read the SFDP to any Macronix chip.
>>
>> Makes sense. But obviously comes with a risk as we won't be able to
>> test all chips for regression when doing that.
>
>
> Finding an old chip for testing might be a challenge.
> But I think we can simulate the behaviour with a new chip that does support RDSFDP.
>
> We can change the RDSFDP opcode of 0x5A to another value of an unrecognized opcode of the chip we use for testing.
> I assume the behaviour of receiving unrecognized opcodes remains.
>
> Erez
>
>
>
>
>>
>>
>> /Esben

Powered by blists - more mailing lists