On 3/21/2025 12:45 AM, Tudor Ambarus wrote: > > > On 3/20/25 2:06 PM, Rob Herring wrote: >> On Thu, Mar 20, 2025 at 2:44 AM Tudor Ambarus <tudor.ambarus@xxxxxxxxxx> wrote: >>> >>> Hi, Rob, >>> >>> On 3/19/25 11:30 PM, Rob Herring wrote: >>>> On Wed, Mar 19, 2025 at 06:47:44PM +0900, Takahiro Kuwano wrote: >>>>> There are infineon flashes [1] that require 8 dummy cycles for the >>>>> 1-1-1 Read ID command. Since the command is not covered by JESD216 >>>>> or any other standard, get the number of dummy cycles from DT and use >>>>> them to correctly identify the flash. >>>> >>>> If Read ID fails, then couldn't you just retry with dummy cycles? Or >>> >>> I think Read ID won't fail when the op requires 8 dummy cycles, it >>> probably just reads garbage on the first 8 cycles, so we risk to wrongly >>> match other flash IDs. >>> >>>> would unconditionally adding dummy cycles adversely affect other chips? >>> >>> Adding 8 dummy cycles to chips that don't need it, would mean ignoring >>> the first byte of the flash ID, thus we again risk to wrongly match >>> against other flash IDs. >>> >>>> >>>> Otherwise, add a specific compatible to imply this requirement. Adding >>>> quirk properties doesn't scale. >>> >>> Do you mean a flash name compatible, like "cyrs17b512,spi-nor"? >> >> Yes, but that's not the format of compatible strings. >> >>> 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. > > We don't even care about the chip ID, if all the flash parameters can be > discovered via SFDP. Unfortunately these tables do not describe all the > flash capabilities (block protection being one). Or worse, manufacturers > mangle these tables. > > So vendors need to identify chips to either fix those tables via some > quirks after the parsing is done, or to specify support that's not > covered by those tables. > > For basic ops, flashes that get the SFDP tables right, don't even need a > flash entry defined, we don't care about their ID, we just initialize > the flash solely based on SFDP. > > In this particular case, this flash needs identification to fix some > wrong SFDP field, it corrects just the mode cycles for the FAST READ > command. All the other commands seem fine according to patch 3/3. > >> >> You have the same problem with a property. Users have to add or remove > > True. It's the same problem. Even if we specify the dummy cycles via a > property, the next plugged-in flash will use those. We can of course > fallback to the SFDP only init if the ID doesn't match any flash entry, > but the problem is the same. > >> the property if the flash changes. Anyone thinking they can use this >> chip as a compatible 2nd source is SOL. >> > > 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. > > 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. > Infineon SEMPER flash families (S25Hx-T/S28Hx-T) requires dummy cycles in Read ID, depending on Configuration Register setting. That is to support higher frequencies. By factory default dummy is 0 in 1S-1S-1S and it works up to 133MHz. Users can change the setting to support higher frequencies. > Thus instead of having lots of new compatibles for this, I lean towards > having this property. I'm still open for the compatible idea, I just > wanted to explain better where we are. > > Thanks, > ta >
