Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: W6bwCSeVRt+pbWhumKAW9IjFmY59PBeYyX6VCa9bQxvZ/rBbCIqEyBS51Ge1rlqgCZb36SckBo
 bjJ+uj+s/fUCcb8hkabc4OM/0FX2ISOa58BC1axVeJw3AoDwLtfa3rR46UKGpWzNf438G7SCGb
 5Ni54Be7hS9BJkZ64YqltQQ+uCZlg1l9r5dNo8tnC0fW1zefMe2I7uZ9b4ekT6c4dK8fWfgxsA
 z/JE2kFE7FqIa5IrjWvXFxs4Z2wk5jkZil9ByHnF7epqNGrVYLYI2XcM+ySCylfJ5nhc5veDUF
 1Rs=
From:   <Tudor.Ambarus@microchip.com>
To:     <linux@rasmusvillemoes.dk>, <michael@walle.cc>
CC:     <linux-mtd@lists.infradead.org>, <frieder.schrempf@kontron.de>,
        <bbrezillon@kernel.org>, <p.yadav@ti.com>,
        <linux-kernel@vger.kernel.org>, <esben@geanix.com>,
        <zhengxunli@mxic.com.tw>, <jaimeliao@mxic.com.tw>,
        <masonccyang@mxic.com.tw>, <ycllin@mxic.com.tw>
Subject: Re: [RFC 2/3] mtd: spi-nor: core: compare JEDEC bytes to already
 found flash_info
Thread-Topic: [RFC 2/3] mtd: spi-nor: core: compare JEDEC bytes to already
 found flash_info
Thread-Index: AQHXZ/txAD1daNN5FEGbvnbgvaM45A==
Date:   Wed, 23 Jun 2021 06:46:08 +0000
Message-ID: <d1473b12-daea-e5e8-b8d5-64084bfbe11c@microchip.com>
References: <20210621152320.3811194-1-linux@rasmusvillemoes.dk>
 <20210621152320.3811194-3-linux@rasmusvillemoes.dk>
 <223232ecd0865e819477f17cd464ab08@walle.cc>
 <d43f021d-8f95-7857-b70b-91e22b0a8e2a@rasmusvillemoes.dk>
In-Reply-To: <d43f021d-8f95-7857-b70b-91e22b0a8e2a@rasmusvillemoes.dk>
Accept-Language: en-US
Content-Language: en-US
user-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.11.0
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?Windows-1252?Q?bO2griUo1cZOfCnIftUFJcu8j2/XxoqIde46gJ3dS+HTX/NI2vZoPRkm?=
 =?Windows-1252?Q?I36FW1wR+T6O1+25BjEURNI+5268jZRGMFqCtjT+wDRHEJ36aU6eOgrc?=
 =?Windows-1252?Q?z4HZymRqPQMphaOmN/fM1ptwAOstHd7049crPpSR+7g9LyHIIipeZMuW?=
 =?Windows-1252?Q?+BRkP+JeBdZigaVyESqLkuStFBDfzAYVceobuFgi8RHcdYeAe3Cwlhgk?=
 =?Windows-1252?Q?x2ZqKkq9qaoIZJn7Zo2aqubqwMC5Ae9XT2riCLBBFBSmn+G7NhEpq8vn?=
 =?Windows-1252?Q?y5u58R+Woa/q3MFEmP2vrk2wOXiAOe5cm20t/A8di3hPbiEyXD4R69BF?=
 =?Windows-1252?Q?8k6JUgpBj2AQeR/tfVrlQUjdS/m4VU06y+LDCEW2rBAmxNooq/W54PSY?=
 =?Windows-1252?Q?OPZAN089C0NW7AK/W4UglUIRVidLSRYT8Al7hHjkxyMO+GhQP2C1PNFt?=
 =?Windows-1252?Q?dDNtBx5rMyBpW2n4fu3YVR+ztLd36jXlGIXLFQ1xmIJL7qUU0fAsZ5Bz?=
 =?Windows-1252?Q?cA489ZwqnOzvO/Kp544NDWGJUdrUjjS0In95kziwCTFTnaqn75LXDwhE?=
 =?Windows-1252?Q?Ytknh+9Q+p55N6JxN39W+cS7m3kMD+5IG7k5rhMCyHuieJ8StLlnaDCr?=
 =?Windows-1252?Q?PcKpJyir3hs4co0C73w++t8rRy0Lajg9VYg84uQUjOF+aG9/nOub/cw+?=
 =?Windows-1252?Q?fHxVRa9TDLx2lyVTzkDf4DTdRjLCphgRopBHkqVhBvpNbQ/ZAFffZlNO?=
 =?Windows-1252?Q?SqrhxQX1PCrOGPMqmOSRgrNHsugtdFD2S/uPMx8Lo2aoEGIHxIB4SYMH?=
 =?Windows-1252?Q?uoGn0opJTvgAeQNGbTnC2V2CKRWmT3lC6XbCrA7r2BDS244MPy6bAZud?=
 =?Windows-1252?Q?JnTm/EJoNrlzq11f0Ad+0Mk5EuwKFABH9Ayok3O9J80Y3R/IYHLmvDnc?=
 =?Windows-1252?Q?2XRMOU2Ws88ZU35qgPpGd/9bjsyOWesYTM/SHQK4fxu5Q+gsio7oDLFj?=
 =?Windows-1252?Q?iIjUxDcHCJFDqPgaxiH1JtVKs5lur5Axsw2zIw08lzJG/EwHLyhNVyBz?=
 =?Windows-1252?Q?Vc0vt04QNaoA5o7JLmWiOlO9tgpsLwwbFHgjqdaY+BcqrkSa1C0qL8cu?=
 =?Windows-1252?Q?iNiWzysR1vsI/Nwyr8k2WNeE0kyKyvE0CZKLRRcCOrRk8v7rNKixO37Q?=
 =?Windows-1252?Q?XWha2V82bldFwg01Bkw07n1ThmTsRYCjgGC2Z0eKlBY3H1kx+Ef9SLiV?=
 =?Windows-1252?Q?y2FSKKD4F9SpD1RI8j5sXRO6j0VONszejNdFZxNEaUdphd7fagUP/EA1?=
 =?Windows-1252?Q?wQCHaQBFE0OQ44UzF0VUfMYjTbjrJHndcAYnA15UjcfEZcBFHwNLkK/4?=
 =?Windows-1252?Q?SOB+026PP6PW9W+xtomGazw4rQx/No1DL2M=3D?=
x-ms-exchange-transport-forked: True
Content-Type: text/plain; charset="Windows-1252"
Content-ID: <A42A73CAC17D0841B9405202F3E939F5@namprd11.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB4869.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1d5d46c7-840c-4b28-369d-08d9361294f0
X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Jun 2021 06:46:08.4283
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 3f4057f3-b418-4d4e-ba84-d55b4e897d88
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 1+ROkRDKeUXj1EXHM/Xcnx/mncctRGNeeJJhTxH0VLfcKnK5KTY5wqj7+id+J/fS6gCBKHByssFyKtzL7314hOo7XPbX6x0+ghqK5cz5VJk=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB4997
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi,

On 6/22/21 11:58 PM, Rasmus Villemoes wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know th=
e content is safe
>=20
> On 22/06/2021 13.57, Michael Walle wrote:
>> [+ some people from MXIC as they are ones who posted to the ML
>> lately. Feel free to forward this mail to the corresponding people.]
>>
>> Am 2021-06-21 17:23, schrieb Rasmus Villemoes:
>>> Macronix engineers, in their infinite wisdom, have a habit of reusing
>>> JEDEC ids for different chips. There's already one
>>> workaround (MX25L25635F v MX25L25635E), but the same problem exists
>>> for MX25L3205D v MX25L3233F, the latter of which is not currently
>>> supported by linux.
>>>
>>> AFAICT, that case cannot really be handled with any of the ->fixup
>>> machinery: The correct entry for the MX25L3233F would read
>>>
>>>         { "mx25l3233f",  INFO(0xc22016, 0, 64 * 1024,  64, SECT_4K |
>>> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ ) },
>>>
>>> while the existing one is
>>>
>>>     { "mx25l3205d",  INFO(0xc22016, 0, 64 * 1024,  64, SECT_4K) },
>>>
>>> So in spi_nor_init_params(), we won't even try reading the sfdp
>>> info (i.e. call spi_nor_sfdp_init_params), and hence
>>> spi_nor_post_sfdp_fixups() has no way of distinguishing the
>>> chips.
>>>
>>> Replacing the existing entry with the mx25l3233f one to coerce the
>>> core into issuing the SPINOR_OP_RDSFDP is also not really an option,
>>> because the data sheet for the mx25l3205d explicitly says not to issue
>>> any commands not listed ("It is not recommended to adopt any other
>>> code not in the command definition table, which will potentially enter
>>> the hidden mode.", whatever that means).
>>
>> Maybe we should ask Macronix if it is safe to send the RDSFDP command.
>> Can anyone from MXIC comment this?
>=20
> Yeah, that would be useful to know, but I don't have any hopes
> whatsoever of Macronix engineers being able to help sort out the mess
> they've created by reusing IDs in the first place. They don't seem to
> understand how that can possibly be a problem.
>=20
> I, and my client, have contacted them on several occasions to ask how
> we're supposed to deal with that. At one point, the answer was
> "MX25L3233F support Serial Flash Discoverable Parameters (SFDP) mode,
> MX25L3205D does not support.", but when I asked the obvious follow-up
> ("but the MX25L3205D datasheet warns against doing RDSFDP or any other
> not explicitly allowed command"), I got no response.
>=20
> Another response was
>=20
> "I can only comment on Linux 4.4, as that is the only version that I
> have supporting material for. Basically we have a patch for MTD/SPI-NOR
> (see attached). This is to allow allow the MTD subsystem to cope with
> devices that have the same ID (see below first paragraph of application
> note attached). Please note that the MX25L3205D had an EOL notification
> on 14th May 2010."
>=20
> and that attached patch is a 173KB .patch file that made me taste my
> breakfast again.
>=20
> And they keep repeating the argument that when a chip is EOL, it's OK to
> reuse its ID (because obviously nobody have used that chip in a product
> that would receive OS updates, so any OS released later than that EOL
> date can just include support for the newer chip and drop the old one...)=
.
>=20
>>> In order to support such cases, extend the logic in spi_nor_read_id()
>>> a little so that if we already have a struct flash_info* from the name
>>> in device tree, check the JEDEC bytes against that, and if it is a
>>> match, accept that (device tree compatible + matching JEDEC bytes) is
>>> stronger than merely matching JEDEC bytes.
>>
>> This won't help much without a proper dt schema. No in-tree devicetree
>> could use is because the DT validation would complain.
>=20
> I can certainly extend the regexp in jedec,spi-nor.yaml to match this
> new one. DT is supposed to describe the hardware, so I can't see how
> that could possibly be controversial.

No, please don't go that path yet.

>=20
>  So if this will
>> go in (and the maintainers are rather hesitant to add it, I tried
>> it myself [1]), you'd also need to add it to jedec,spi-nor.yaml and
>> get an ack from Rob.

I'm not hesitant, I'm keeping my NACK until we're sure there isn't any othe=
r way
to differentiate at run-time. I've contacted a macronix representative, ask=
ing
if RDSFDP is harmful for MX25L3205D, let's wait for a few days. Maybe Zheng=
xun or
ycllin in cc can help too. Is there anyone here having a MX25L3205D?

Cheers,
ta

>=20
> Thanks for that link. So it seems this isn't the first time recycled IDs
> have come up, and not just for Macronix.
>=20
> Yes, vendors shouldn't recycle IDs. They do. They should be punished by
> people not using those chips in new designs. Doesn't work, hardware
> designers do use them. Auto-detection is preferred over using hard-coded
> values from DT. Sure, absolutely, and when the ID is known to be
> ambiguous, by all means throw in all the heuristics and chip-specific
> quirks one can think of to disambiguate. But at the end of the day,
> there are chips out there which cannot be distinguished without help
> from DT, and as DT is supposed to describe the hardware, why is that
> such a big problem?
>=20
> And I'm not suggesting any change whatsoever as to how a compatible
> string of merely "jedec,spi-nor" would be handled - it would just take
> the first chip with the read JEDEC id, then apply any appropriate quirks
> and fixups.
>=20
> Rasmus
>=20