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: 6zIUMXGwlYaeutrirFQzE0FXp+J/w410tWtepZACcYX7hr/dmmkcAv7srNjMNkqE69ihu+8t+X
 9W5MFN4NlxvOtVvMBxwSUvsrNxAj6Jdu/OReb6MXPLXRHlYJFbBFILQ9lw0aYS09FrHyf2t6FZ
 iiTxOaKQL6+UPMiJ+YXh8TS8cZc1LQRqcXUEzJK2DS3baoGOxK4osn348THbe/U1PalHtCaIBk
 U/nnrGyAMAqNmACLsN447+GndKktj6HaSTla+q2W79RsneyZ3bNDQa6weW/xVnojTtN+j8ZL8I
 ZkE=
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:   Fri, 2 Jul 2021 13:34:41 +0000
Message-ID: <964153d0-87cd-7735-3c74-f89bc901065e@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>
 <d1473b12-daea-e5e8-b8d5-64084bfbe11c@microchip.com>
 <63c61200-2387-6f92-32a0-38baf7317cdf@rasmusvillemoes.dk>
In-Reply-To: <63c61200-2387-6f92-32a0-38baf7317cdf@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?jx+vRWcTdo8xy2vheBYRB3Xt/8CPT2YTG7d40yizeEBooK2P6smKS6mF?=
 =?Windows-1252?Q?rRFNcZp5O7ZKkmjvOUHUQEvuCKgXl+5RRCKuhqYZm/8GIq65pMFIzwGc?=
 =?Windows-1252?Q?LvHil0CMrpp/ZKcYtQyVeP0GWCny6uXih/8KkmCv3QH1bo/YF6qdfUE/?=
 =?Windows-1252?Q?huEtnTMs4UKXeLc2UDIR1p94MpgBEicjQLh35vjAr85+9QkfCsEpYoMF?=
 =?Windows-1252?Q?d+Scn42KQsYvJ/LrWeGmDZ7bj7bwQCyL5EF0r2T8jaMDOo1Ltd9HnoWW?=
 =?Windows-1252?Q?E7wwIbLN3rkEZO8pWQyPGVcYyGmuDzn8SKF2aiY902M8ZVago6PjuJ4F?=
 =?Windows-1252?Q?Llef3ZAVT+PbZ/RibGA2NQmf0LIQ0svwdflHyou7bqb1C1ykss7sucWI?=
 =?Windows-1252?Q?Iu6Di8xrl+jdVnITGwk0wwbqdb9LrUXkhrFwjX/zREHQstCdgPvx6x09?=
 =?Windows-1252?Q?A3CSZ5NNoz7oqykybOEkk0XUOccDJbBGwdrgXNqEyPPqzOwUwM+/VNGn?=
 =?Windows-1252?Q?yBOCq/1sFx9kFuyZKymYaGfdPsM8FfB9U4Zgvp18tH90FZDn+gf597C6?=
 =?Windows-1252?Q?egQByk5Pet8cXoGd/7hXF3ag7hHnAdV2skoXMGRSMyUca5HueZ08/cZV?=
 =?Windows-1252?Q?2Pb/U1or9+hEEy5OqUaq/usM50ceeyx5X14N+J4pYVSm892UNsdUqC/5?=
 =?Windows-1252?Q?p/2eWKg5blYOfY1ksBXFt6gex5IpD4y9Y6R3lZ5R01SzPYKuB4zQgt6d?=
 =?Windows-1252?Q?2kDzOdutpl7whQJWtEN3Oi7pH36DbjcnddZsYoPEDYT05t+2DtOak8ZW?=
 =?Windows-1252?Q?4JMJ/j6kbJbhrZIQjeeldEeDSH7Mev76IersuCorjbPXZpfQgVtV2gvA?=
 =?Windows-1252?Q?J8O4U1GSATQfxXvGAvsW2toappF9F/0544Ri2OsKYECRRw6KF6ao7eoC?=
 =?Windows-1252?Q?cWKyxEmjfKBWQzBnhCqQgUUUYMxFcikQmXIfm/+wBM1E+CnDNy/xrO4N?=
 =?Windows-1252?Q?XRv0OBHc0Cnu1+4mbxz+Xb+vsz5r5vUnjzDeoSgXOliIlpiQRbiVj4d5?=
 =?Windows-1252?Q?jjyS2SjuZ69Erv7WpTIS54GuVEd2kMC/YVyHE2s31q2jdBifDMhuhMgz?=
 =?Windows-1252?Q?mrwwOR8knWrjYzSMS0ksOQknZZTKx7x1ui1fIHsevACaaWD9oG+BU5Ww?=
 =?Windows-1252?Q?RUPoDuJo7qW/9Ht2yIi0Xah8IK+fVLgeCKDpnbJqLgT1XczGpX5G9wPs?=
 =?Windows-1252?Q?qPtXTtoKDKnuEoX+7W7jmNnQlzcZIxb7wy2P5HEB/PKRuUmqGp9rPhVh?=
 =?Windows-1252?Q?cHRl/Z6Yh9Ns+ja3nZAgM1K7rNcT7VnxNvXN3Ny1XfyhT6pREj7lhKUG?=
 =?Windows-1252?Q?T0iHQjiKX+YjkwWcX7GsT/JsbyWQVzBW8go=3D?=
x-ms-exchange-transport-forked: True
Content-Type: text/plain; charset="Windows-1252"
Content-ID: <8F3A11E52E5DE24D83099B58AA3B4593@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: 1cbffc79-2403-46b2-10f7-08d93d5e2560
X-MS-Exchange-CrossTenant-originalarrivaltime: 02 Jul 2021 13:34:41.2172
 (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: BQgH3VhgFo4esRtmjiFgr4eAr0AaKhi4a3YMtxrUOsPheGZFzQrFuILcq0umd8yMEezUHZT68UDDWAWGugOF2h9h0SSM+FgTFWW9NQ455GA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB5141
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 7/2/21 4:17 PM, Rasmus Villemoes wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know th=
e content is safe
>=20
> On 23/06/2021 08.46, Tudor.Ambarus@microchip.com wrote:
>> Hi,
>>
>> On 6/22/21 11:58 PM, Rasmus Villemoes wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know =
the content is safe
>>>
>>> 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 issu=
e
>>>>> any commands not listed ("It is not recommended to adopt any other
>>>>> code not in the command definition table, which will potentially ente=
r
>>>>> 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?
>>>
>>> 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.
>>>
>>> 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.
>>>
>>> Another response was
>>>
>>> "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."
>>>
>>> and that attached patch is a 173KB .patch file that made me taste my
>>> breakfast again.
>>>
>>> And they keep repeating the argument that when a chip is EOL, it's OK t=
o
>>> 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..=
.).
>>>
>>>>> 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 nam=
e
>>>>> 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.
>>>
>>> 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.
>>
>>>
>>>  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 o=
ther way
>> to differentiate at run-time.
>=20
> It seems that we have established that by now, right?
>=20

It is unlikely that RDSFDP will cause any problems for the old MX25L3205D,
I would like to differentiate between the two flashes by parsing SFDP.

I'm preparing a patch set to address the all the ID collision thingy.
https://github.com/ambarus/linux-0day/commit/b760260efecb4f3678de3b78250f99=
338ecbad1b