[spi-nor] Macronix MX25L requires CR read opcode 0x15 (not 0x35)

[Pratyush Yadav]

On Tue, Sep 02 2025, Maarten Zanders wrote:

> Hi,
>
> On Mon, Sep 1, 2025 at 4:46 PM Pratyush Yadav <pratyush at kernel.org> wrote:
>> > On the Macronix MX25L12833F (ID 0xC22018) & others of this family/mfg,
>> > the CR (condition register) must be read with opcode 0x15. The driver
>> > currently uses 0x35, which the chip does not recognize.
>>
>> Is it one of those devices for which Macronix has re-used the flash IDs?
>> I vaguely recall reading that somewhere. If so, then we also need to be
>> careful of breaking the older variants of the flash.
>
> Yes, there is quite some flash ID reuse. This is indeed one of them.
> I just went through a lot of datasheets from Macronix and for those
> supporting RDCR, they are always using opcode 0x15.
> Also, when inspecting patches (for older kernel, uboot,..) which add
> MX spi-nor support from the Macronix website, they do something like
> this:
>
> #define SPINOR_OP_RDCR_MX 0x15 /* Read configuration register for Macronix*/
> ...
> opcode = (nor->jedec_id == CFI_MFR_MACRONIX) ? SPINOR_OP_RDCR_MX :
> SPINOR_OP_RDCR;

How do any of the flashes even work on mainline then? Won't they all
have the same problem? Does it only end up touching the OTP bits, and
those don't get noticed?

>
> A cleaner/modern approach would be to add a parameter in struct
> spi_nor_flash_parameter which gets defaulted to SPINOR_OP_RDCR (0x35)
> and fixed up using manufacturer late_init() for Macronix?

Yeah, or maybe a callback like for ready?

>
>> Is it at all possible to discover the opcode from the flash SFDP table
>> (SCCR map perhaps)? If so, I think that would be the best way forward
>> since it would be generic.
>
> Unfortunately, no. I received the SFDP data for this part and there's
> no SCCR map or other means to distinguish opcodes. I believe only the
> more recent parts include this (SCCR was defined in 2019).
>
>> There is the SNOR_F_NO_READ_CR flag that seems to do what you want. In
>> spi_nor_write_16bit_sr_and_check(), if the flag is set it does not issue
>> the read CR command and either sets the second byte to 0 or to
>> SR2_QUAD_EN_BIT1. The flag can be discovered via BFPT (see
>> spi_nor_parse_bfpt()). Is the BFPT table on the flash incorrect? In that
>> case, perhaps we should add a post-BFPT fixup?
>
> That flag will have similar results as reading garbage today. It will
> ignore the actual contents of CR when we're actually trying to do a
> read-don't-modify-write. In the case of this part, it will clear some
> bits which were 1 by default, changing output driver strength.

Hmm, makes sense. I was hoping the bits would also default to 0 so there
won't be a problem.

-- 
Regards,
Pratyush Yadav