[PATCH RFC 0/4] Add set_iofv() callback

[Biju Das]

Hi Michael Walle,

> Subject: Re: [PATCH RFC 0/4] Add set_iofv() callback
> 
> Hi Biju,
> 
> >> >> > As per section 8.14 on the AT25QL128A hardware manual[1],
> >> >> > IO0..IO3 must be set to Hi-Z state for this flash for fast read
> >> >> > quad IO.
> >> >> > Snippet from HW manual section 8.14:
> >> >> > The upper nibble of the Mode(M7-4) controls the length of the
> >> >> > next FAST Read Quad IO instruction through the inclusion or
> >> >> > exclusion of the first byte instruction code. The lower nibble
> >> >> > bits of the
> >> >> > Mode(M3-0) are don't care. However, the IO pins must be
> >> >> > high-impedance before the falling edge of the first data out
> clock.
> >> >>
> >> >> I'm still not sure what you are trying to fix here. For any quad
> >> >> I/O mode, the pins of the controller must be in hiZ during the
> >> >> data phase on a read operation. Otherwise the flash couldn't send
> >> >> any data, there would be two drivers for one signal. So being in
> >> >> hiZ state should be the default and shouldn't depend on any
> connected flash.
> >> >
> >> > OK, I will make hiZ state as the default.
> >>
> >> I still think this iofv setting is the wrong approach, though. Do you
> >> have a link to the spi controller datasheet where I can look up what
> >> the controller is doing.
> >
> > Please find the below link.
> >
> > https://www.renesas.com/eu/en/products/microcontrollers-microprocessor
> > s/rz-mpus/rzg2l-general-purpose-microprocessors-dual-core-arm-cortex-a
> > 55-12-ghz-cpus-and-single-core-arm-cortex-m33#overview
> 
> Thanks.
> 
> >> This seem to be a general problem with what we are sending during the
> >> command phase and I'm curious why there wasn't more reports on non
> >> working micron flashes for now.
> >
> > 1-bit mode, we don't have any issue. Once we switch to 4-bit mode we
> > have this issue with micron MT25QU512A flash and we need to set the
> > correct IO fixed values.
> >
> > Maybe others are testing with 1-bit mode and not testing the full
> > capability of the flash.
> >
> >>
> >> >> You've mentioned the micron flash which needs a '1' on its
> >> >> hold/reset pin.
> >> >> I would have expected a fixup for this flash, not for the flash
> >> >> which behaves normal.
> >> >
> >> > I will drop fixup for Renesas AT25QL128A  and will add fixup for
> >> > micron flash.
> >>
> >> btw, what will happen if you always use the {3,3,3,1} setting? I
> >> guess the atmel flash will also work? because HiZ should mean "don't
> >> care"
> >> from
> >> the point of view of the flash.
> >
> > With atmel flash if use {3,3,3,1} setting, I get below error.
> >
> > root at smarc-rzg2ul:/cip-test-scripts# ./rpcif_t_001.sh [  144.078854]
> > spi-nor spi1.0: spi-nor-generic (16384 Kbytes) [  144.120468] 2
> > fixed-partitions partitions found on MTD device spi1.0 [  144.126982]
> > Creating 2 MTD partitions on "spi1.0":
> > [  144.133004] 0x000000000000-0x000000200000 : "boot"
> > [  144.141879] 0x000000200000-0x000001000000 : "user"
> > [  358.476963] jffs2: notice: (230) read_dnode: node CRC failed on
> > dnode at 0xdfe084: read 0x336ebbbc, calculated 0x961503c7 [
> > 358.488509] jffs2: notice: (230) read_dnode: node CRC failed on dnode
> > at 0xdfd118: read 0xff6a5df6, calculated 0x786a5df6 [  358.502963]
> > jffs2: notice: (230) read_dnode: node CRC failed on dnode at 0xdfa2d4:
> > read 0x1fc99948, calculated 0xbab22133 [  358.515357] jffs2: notice:
> > (230) read_dnode: node CRC failed on dnode at 0xdf9368: read
> > 0xffd184a7, calculated 0x3d184a7 [  358.528175] jffs2: notice: (230)
> > read_dnode: node CRC failed on dnode at 0xdf6524: read 0x5deb2462,
> > calculated 0xf8909c19
> 
> Strange. Can't make any sense of this at the moment.
> 
> >> > With iofv settings {3,3,3,3} (all pins on Hi-Z state) with Micron
> >> > flash
> >> > -------------------------------------------------------------------
> >> > ---
> >> > -
> >> >
> >> > ./rpcif_t_001.sh
> >> > [   37.950986] spi-nor spi1.0: unrecognized JEDEC id bytes: ff ff ff
> ff
> >> > ff ff
> >>
> >> As mentioned earlier, I suspect that HiZ on IO3 means low and the
> >> flash will be in reset. Could you perhaps verify that by probing IO3?
> >> I know that other flashes will *either* support RESET#/HOLD# or quad
> >> mode.


> >> Thus I was saying, that we probably wont support that and the easiest
> >> fix should be to disable this behavior for the atmel flash (there was
> >> nv setting).
> >
> > The fix up is invoked only for quad mode, I believe it is safe to add
> > fixup for micron flash As it is the one deviating from normal
> > according to you, rather than adding fixup for generic flash like
> > ATMEL flash(Now Renesas flash)
> 
> Could you please try setting bit 4 in the Nonvolatile Configuration
> Register (Table 7) and see if the problem goes away?

You mean, if it works, we need to disable reset for all the boards, maybe at bootloader level??

OK, I will check that. Currently I have read that register and it is showing a value
Of 0xffbb. I need to do write operation. Before that how do we recover flash, if
something goes wrong during writing for NV register?


> 
> Also could you have a look at the schematics, does the IO3/RESET# have a
> pull-up? If not, who is in control of driving the correct value here? If
> it has a pull-up, I'm puzzled why you need any other setting than HiZ.

Unfortunately, there is no pullup on IO3 line and also there is no SoC pullup.

> 
> The correct fix would be to the information about the missing IO state in
> the "struct spi_mem_op". That is, what should be the default values of all
> the IO lines which are unused. For example if we have a 1s1s4s
> transaction, what should be the state of IO0,
> IO2 and IO3 during the command and address phase. If we have a 1s2s2s,
> what should be the state of IO0 during the command phase etc.
> 
> That can then be used within your driver to set the corresponding IOFV
> values (for each spi-mem op).
> 
> But I'm not sure if other SPI controllers will support that, though.

Currently driving SoC IOFV register, it fixes the issue and it is just one time
operation during post sfdp. I take this as a SoC feature which other controllers
don't have.

Cheers,
Biju