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

From: Biju Das <biju.das.jz@bp.renesas.com>
To: Michael Walle <michael@walle.cc>
Cc: Mark Brown <broonie@kernel.org>,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
	"linux-spi@vger.kernel.org" <linux-spi@vger.kernel.org>,
	"linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>,
	biju.das.au <biju.das.au@gmail.com>,
	"linux-renesas-soc@vger.kernel.org"
	<linux-renesas-soc@vger.kernel.org>
Subject: RE: [PATCH RFC 0/4] Add set_iofv() callback
Date: Thu, 9 Nov 2023 11:48:25 +0000	[thread overview]
Message-ID: <TYVPR01MB11279575676708170F3B3270D86AFA@TYVPR01MB11279.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <b9831be88008b9f9960d1d79cd0e5a3a@walle.cc>

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-microprocessors/rz-mpus/rzg2l-general-purpose-microprocessors-dual-core-arm-cortex-a55-12-ghz-cpus-and-single-core-arm-cortex-m33#overview

> 
> 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@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

> 
> >
> > 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)

Cheers,
Biju

> 
> I guess, the correct fix would be to somehow add support to control
> IO1-IO3 during the (single bit) command phase.

> 
> 
> >
> > EXIT|FAIL|rpcif_t_001.sh|[00:00:01] Failed to detect mt25qu512a
> > flash!||
> >
> >
> > With iofv settings {3,3,3,1} with Micron falsh
> > ---------------------------------------------
> > root@smarc-rzg2l:/cip-test-scripts# ./rpcif_t_001.sh
> > [   26.500035] spi-nor spi1.0: mt25qu512a (65536 Kbytes)
> > [   26.533995] 2 fixed-partitions partitions found on MTD device spi1.0
> > [   26.540410] Creating 2 MTD partitions on "spi1.0":
> > [   26.545239] 0x000000000000-0x000002000000 : "boot"
> > [   26.554381] 0x000002000000-0x000004000000 : "user"
> >
> > EXIT|PASS|rpcif_t_001.sh|[00:03:01] ||
> >
> > Cheers,
> > Biju

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