Re: [PATCH 2/3] mtd: spi-nor: use rdid-dummy-ncycles DT property

From: Takahiro Kuwano <tkuw584924@gmail.com>
To: Tudor Ambarus <tudor.ambarus@linaro.org>,
	Michael Walle <mwalle@kernel.org>, Rob Herring <robh@kernel.org>
Cc: Pratyush Yadav <pratyush@kernel.org>,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	linux-mtd@lists.infradead.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Bacem Daassi <Bacem.Daassi@infineon.com>,
	Takahiro Kuwano <Takahiro.Kuwano@infineon.com>,
	Mark Brown <broonie@kernel.org>
Subject: Re: [PATCH 2/3] mtd: spi-nor: use rdid-dummy-ncycles DT property
Date: Thu, 27 Mar 2025 19:43:57 +0900	[thread overview]
Message-ID: <e1baa67a-d62a-477f-9ccd-83cc2dae501a@gmail.com> (raw)
In-Reply-To: <be8e5758-8f85-4476-b6c0-4400c8151cbe@linaro.org>

On 3/26/2025 11:44 PM, Tudor Ambarus wrote:
> Hi, Michael,
> 
> Sorry, I somehow missed your replies.
> 
> On 3/21/25 8:00 AM, Michael Walle wrote:
> 
> cut
> 
>>>>> The
>>>>> problem that I see with that is that we no longer bind against the
>>>>> generic jedec,spi-nor compatible, so people need to update their DT in
>>>>> case they use/plug-in a different flash on their board.
>>>>
>>>> This chip is clearly *not* compatible with a generic chip.
>>>
>>> I think it is compatible. The chip defines the SFDP (serial flash
>>> discoverable parameters) tables. At probe time we parse those tables and
>>> initialize the flash based on them.
>>
>> I disagree. It's not compatible with "jedec,spi-nor", which is
>> defined as
>>
> 
> cut
> 
>>
>> See my first reply, on how to possibly fix this mess (new
>> compatible if accepted, just use RDSFDP sequence which is backed by
>> the standard and do some fingerprinting).
>>
> 
> this won't work unless there's a unique parameter or ID in the sfdp or
> vendors tables, which I doubt. Takahiro to confirm.
> 
No, cyrs17b doesn't have it.

>> FWIW, a new (or rather different) compatible is needed because we
>> cannot distinguish between random data returned during the dummy
>> cycles and a proper manufacturer id. So there is no way we could fix
>> this in the core itself.
> 
> Yes, I agree, new compatible it is then.
> 
> cut
> 
>>> I think the property vs compatible decision resumes at whether we
>>> consider that the dummy cycles requirement for Read ID is/will be
>>> generic or not.
>>
>> It is not generic. Because it will break autodetection. And that is
>> the whole purpose of this. Adding that property means, we can just
>> autodetect flashes within this 'group'. And personally, I think this
>> is a bad precedent.
>>
> 
> yes, I agree.
> 
>>> I noticed that with higher frequencies or protocol modes (e.g, octal
>>> DTR), flashes tend to require more dummy cycles. I think with time,
>>> we'll have more flashes with such requirement. Takahiro can jump in and
>>> tell if it's already the case with IFX.
>>
>> But hopefully not with RDID. Again this doesn't play nice with other
>> flashes (or all flashes for now). Instead of adding random delay
>> cycles one should rather define a max clock speed for this opcode.
> 
> This could work, yes. But not for this flash. Or maybe encourage vendors
> to either contribute and enlarge the SFDP database or define their own
> vendor tables for all the flash properties that are not covered yet.
> It's strange how Block Protection is not yet covered by SFDP after all
> these years.
> 
> Thanks,
> ta


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