All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael Walle" <mwalle@kernel.org>
To: "Cheng Ming Lin" <linchengming884@gmail.com>
Cc: "Miquel Raynal" <miquel.raynal@bootlin.com>,
	"Pratyush Yadav" <pratyush@kernel.org>,
	"Takahiro Kuwano" <takahiro.kuwano@infineon.com>,
	"Richard Weinberger" <richard@nod.at>,
	"Vignesh Raghavendra" <vigneshr@ti.com>,
	<linux-mtd@lists.infradead.org>, <linux-kernel@vger.kernel.org>,
	<alvinzhou@mxic.com.tw>,
	"Cheng Ming Lin" <chengminglin@mxic.com.tw>
Subject: Re: [PATCH] mtd: spi-nor: Add support for MX25L12833F and MX25L12845G
Date: Mon, 01 Jun 2026 14:57:00 +0200	[thread overview]
Message-ID: <DIXQA3JV2N5M.2UHTAAXQEWB86@kernel.org> (raw)
In-Reply-To: <CAAyq3SYX9UPwhC_Ume_S2yxhQwimRvB=Y6O_+FFqokhNmw7jQg@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 4028 bytes --]

On Mon Jun 1, 2026 at 2:56 AM CEST, Cheng Ming Lin wrote:
> Hi Michael,
>
> Michael Walle <mwalle@kernel.org> 於 2026年5月28日週四 下午3:52寫道:
>>
>> Hi,
>>
>> On Thu May 28, 2026 at 9:42 AM CEST, Miquel Raynal wrote:
>> > Hi Cheng Ming,
>> >
>> >>> It looks like you're getting bitten by the ID reuse. You can't just
>> >>> unconditionally add the quad PP because as far as I can see the
>> >>> MX25L12805D [1] is just a standard single bit i/o flash and doesn't
>> >>> support the 4PP.
>> >>
>> >> You are absolutely right. Thanks for catching this.
>> >>
>> >> The MX25L12805D is indeed a much older product (released around 2009).
>> >> Since the initial JESD216 SFDP standard wasn't published until 2011,
>> >> I double-checked with our internal PM and confirmed that the MX25L12805D
>> >> does not support SFDP at all.
>> >>
>> >> Since the newer flashes (MX25L12833F and MX25L12845G) do support SFDP,
>> >> we could use this as a differentiator to distinguish them from the legacy
>> >> MX25L12805D.
>> >>
>> >> What if we try to read the SFDP signature (RDSFDP) in the fixup hook?
>> >> If a valid SFDP signature is detected, we can safely identify it as the
>> >> newer flash and apply the SNOR_HWCAPS_PP_1_4_4 capability. If there is
>> >> no SFDP signature, we leave it as is for the legacy MX25L12805D.
>> >>
>> >> Do you think this approach is feasible and acceptable? If so, I will
>> >> implement this logic and submit a v2 patch.
>> >
>> > The ->post_sfdp() fixup hook is documented as "not called for SPI NORs
>> > that do not support SFDP". Alternatively, I believe an earlier hook,
>> > like ->post_bfpt() could also work since it does not seem to run on
>> > non SFDP compatible flashes.
>>
>> Moreover:
>>
>>  *    spi_nor_post_sfdp_fixups() is called after the SFDP tables are parsed.
>>  *    It is used to tweak various flash parameters when information provided
>>  *    by the SFDP tables are wrong.
>>
>> And it looks like that is the case for these flashes. Also, please
>> provide a comment on the fixup describing that the flash misses that
>> property. Basically, what you've written in the commit message.
>
> I am currently working on the flash part 0xc22018 and I am planning to
> add its size back to the flash info table. However, I encountered an
> issue regarding the fixup hooks.
>
> Since this specific flash does not support dual, quad, or octal reads,
> the core logic in spi_nor_init_params_deprecated() determines that SFDP
> parsing is not needed. Because the SFDP parsing is skipped, the post_bfpt
> or post_sfdp hooks are never triggered, which prevents us from applying
> the necessary fixups as we expected.
>
> I initially considered asking about the possibility of removing this
> specific check in spi_nor_init_params_deprecated(), which would allow
> all SPI-NOR flashes to go through SFDP parsing by default (unless
> SPI_NOR_SKIP_SFDP is explicitly set).

Oops, I've forgot to add you on CC. Sorry. I've just send a series
to get rid of that legacy handling. Please give it a try on your
flashes.

[1] https://lore.kernel.org/linux-mtd/20260601125438.3481722-1-mwalle@kernel.org/

-michael

> However, I am aware that forcing SFDP parsing globally on all legacy
> 1-1-1 flashes might be risky. Flashes predating the JESD216 standard
> might return garbage data when the SFDP opcode (0x5A) is issued,
> potentially causing regressions if the garbage data accidentally matches
> the SFDP signature.
>
> If dropping this global check is indeed considered too risky for older
> hardware, would it be acceptable to use the late_init hook to handle
> this flash individually? The idea would be to manually check and parse
> the SFDP table exclusively for 0xc22018 within its own fixup, ensuring
> that no other legacy 1-1-1 flashes are affected by this change.
>
> Any thoughts or guidance on whether this late_init approach is the right
> way to go would be greatly appreciated.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: "Michael Walle" <mwalle@kernel.org>
To: "Cheng Ming Lin" <linchengming884@gmail.com>
Cc: "Miquel Raynal" <miquel.raynal@bootlin.com>,
	"Pratyush Yadav" <pratyush@kernel.org>,
	"Takahiro Kuwano" <takahiro.kuwano@infineon.com>,
	"Richard Weinberger" <richard@nod.at>,
	"Vignesh Raghavendra" <vigneshr@ti.com>,
	<linux-mtd@lists.infradead.org>, <linux-kernel@vger.kernel.org>,
	<alvinzhou@mxic.com.tw>,
	"Cheng Ming Lin" <chengminglin@mxic.com.tw>
Subject: Re: [PATCH] mtd: spi-nor: Add support for MX25L12833F and MX25L12845G
Date: Mon, 01 Jun 2026 14:57:00 +0200	[thread overview]
Message-ID: <DIXQA3JV2N5M.2UHTAAXQEWB86@kernel.org> (raw)
In-Reply-To: <CAAyq3SYX9UPwhC_Ume_S2yxhQwimRvB=Y6O_+FFqokhNmw7jQg@mail.gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 4028 bytes --]

On Mon Jun 1, 2026 at 2:56 AM CEST, Cheng Ming Lin wrote:
> Hi Michael,
>
> Michael Walle <mwalle@kernel.org> 於 2026年5月28日週四 下午3:52寫道:
>>
>> Hi,
>>
>> On Thu May 28, 2026 at 9:42 AM CEST, Miquel Raynal wrote:
>> > Hi Cheng Ming,
>> >
>> >>> It looks like you're getting bitten by the ID reuse. You can't just
>> >>> unconditionally add the quad PP because as far as I can see the
>> >>> MX25L12805D [1] is just a standard single bit i/o flash and doesn't
>> >>> support the 4PP.
>> >>
>> >> You are absolutely right. Thanks for catching this.
>> >>
>> >> The MX25L12805D is indeed a much older product (released around 2009).
>> >> Since the initial JESD216 SFDP standard wasn't published until 2011,
>> >> I double-checked with our internal PM and confirmed that the MX25L12805D
>> >> does not support SFDP at all.
>> >>
>> >> Since the newer flashes (MX25L12833F and MX25L12845G) do support SFDP,
>> >> we could use this as a differentiator to distinguish them from the legacy
>> >> MX25L12805D.
>> >>
>> >> What if we try to read the SFDP signature (RDSFDP) in the fixup hook?
>> >> If a valid SFDP signature is detected, we can safely identify it as the
>> >> newer flash and apply the SNOR_HWCAPS_PP_1_4_4 capability. If there is
>> >> no SFDP signature, we leave it as is for the legacy MX25L12805D.
>> >>
>> >> Do you think this approach is feasible and acceptable? If so, I will
>> >> implement this logic and submit a v2 patch.
>> >
>> > The ->post_sfdp() fixup hook is documented as "not called for SPI NORs
>> > that do not support SFDP". Alternatively, I believe an earlier hook,
>> > like ->post_bfpt() could also work since it does not seem to run on
>> > non SFDP compatible flashes.
>>
>> Moreover:
>>
>>  *    spi_nor_post_sfdp_fixups() is called after the SFDP tables are parsed.
>>  *    It is used to tweak various flash parameters when information provided
>>  *    by the SFDP tables are wrong.
>>
>> And it looks like that is the case for these flashes. Also, please
>> provide a comment on the fixup describing that the flash misses that
>> property. Basically, what you've written in the commit message.
>
> I am currently working on the flash part 0xc22018 and I am planning to
> add its size back to the flash info table. However, I encountered an
> issue regarding the fixup hooks.
>
> Since this specific flash does not support dual, quad, or octal reads,
> the core logic in spi_nor_init_params_deprecated() determines that SFDP
> parsing is not needed. Because the SFDP parsing is skipped, the post_bfpt
> or post_sfdp hooks are never triggered, which prevents us from applying
> the necessary fixups as we expected.
>
> I initially considered asking about the possibility of removing this
> specific check in spi_nor_init_params_deprecated(), which would allow
> all SPI-NOR flashes to go through SFDP parsing by default (unless
> SPI_NOR_SKIP_SFDP is explicitly set).

Oops, I've forgot to add you on CC. Sorry. I've just send a series
to get rid of that legacy handling. Please give it a try on your
flashes.

[1] https://lore.kernel.org/linux-mtd/20260601125438.3481722-1-mwalle@kernel.org/

-michael

> However, I am aware that forcing SFDP parsing globally on all legacy
> 1-1-1 flashes might be risky. Flashes predating the JESD216 standard
> might return garbage data when the SFDP opcode (0x5A) is issued,
> potentially causing regressions if the garbage data accidentally matches
> the SFDP signature.
>
> If dropping this global check is indeed considered too risky for older
> hardware, would it be acceptable to use the late_init hook to handle
> this flash individually? The idea would be to manually check and parse
> the SFDP table exclusively for 0xc22018 within its own fixup, ensuring
> that no other legacy 1-1-1 flashes are affected by this change.
>
> Any thoughts or guidance on whether this late_init approach is the right
> way to go would be greatly appreciated.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

  reply	other threads:[~2026-06-01 12:57 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-28  5:17 [PATCH] mtd: spi-nor: Add support for MX25L12833F and MX25L12845G Cheng Ming Lin
2026-05-28  5:17 ` Cheng Ming Lin
2026-05-28  6:36 ` Michael Walle
2026-05-28  6:36   ` Michael Walle
2026-05-28  7:19   ` Cheng Ming Lin
2026-05-28  7:19     ` Cheng Ming Lin
2026-05-28  7:42     ` Miquel Raynal
2026-05-28  7:42       ` Miquel Raynal
2026-05-28  7:52       ` Michael Walle
2026-05-28  7:52         ` Michael Walle
2026-05-28  9:00         ` Cheng Ming Lin
2026-05-28  9:00           ` Cheng Ming Lin
2026-06-01  0:56         ` Cheng Ming Lin
2026-06-01  0:56           ` Cheng Ming Lin
2026-06-01 12:57           ` Michael Walle [this message]
2026-06-01 12:57             ` Michael Walle
2026-06-02  6:15             ` Cheng Ming Lin
2026-06-02  6:15               ` Cheng Ming Lin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DIXQA3JV2N5M.2UHTAAXQEWB86@kernel.org \
    --to=mwalle@kernel.org \
    --cc=alvinzhou@mxic.com.tw \
    --cc=chengminglin@mxic.com.tw \
    --cc=linchengming884@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=miquel.raynal@bootlin.com \
    --cc=pratyush@kernel.org \
    --cc=richard@nod.at \
    --cc=takahiro.kuwano@infineon.com \
    --cc=vigneshr@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.