2022-03-11 22:46:24

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH RFC] memory: renesas-rpc-if: Fix HF/OSPI data transfer in Manual mode

HyperFlash devices fail to probe:

rpc-if-hyperflash rpc-if-hyperflash: probing of hyperbus device failed

In HyperFLASH or Octal-SPI Flash mode, the Transfer Data Enable bits
(SPIDE) in the Manual Mode Enable Setting Register (SMENR) are derived
from half of the transfer size, cfr. the rpcif_bits_set() helper
function.

Hence when converting back from Transfer Data Enable bits to transfer
size, the bus width must be taken into account, and all Manual Mode Data
Register access sizes must be doubled when communicating with a
HyperFLASH or Octal-SPI Flash device.

Fixes: fff53a551db50f5e ("memory: renesas-rpc-if: Correct QSPI data transfer in Manual mode")
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
Marked RFC, a (1) we should avoid the back-and-forth conversion between
transfer size and Transfer Data Enable bits, and (2) actual HyperFlash
data reads (which follows a completely different code path) still return
all zeros.

On Salvator-XS with unlocked HyperFlash, the HyperFlash is now detected
again, cfr. (with DEBUG_CFI enabled):

Number of erase regions: 1
Primary Vendor Command Set: 0002 (AMD/Fujitsu Standard)
Primary Algorithm Table at 0040
Alternative Vendor Command Set: 0000 (None)
No Alternate Algorithm Table
Vcc Minimum: 1.7 V
Vcc Maximum: 1.9 V
No Vpp line
Typical byte/word write timeout: 512 \xc2\xb5s
Maximum byte/word write timeout: 2048 \xc2\xb5s
Typical full buffer write timeout: 512 \xc2\xb5s
Maximum full buffer write timeout: 2048 \xc2\xb5s
Typical block erase timeout: 1024 ms
Maximum block erase timeout: 4096 ms
Typical chip erase timeout: 262144 ms
Maximum chip erase timeout: 1048576 ms
Device size: 0x4000000 bytes (64 MiB)
Flash Device Interface description: 0x0000
- x8-only asynchronous interface
Max. bytes in buffer write: 0x200
Number of Erase Block Regions: 1
Erase Region #0: BlockSize 0x40000 bytes, 256 blocks
rpc-if-hyperflash: Found 1 x16 devices at 0x0 in 16-bit bank. Manufacturer ID 0x000001 Chip ID 0x007000
Amd/Fujitsu Extended Query Table at 0x0040
Amd/Fujitsu Extended Query version 1.5.
rpc-if-hyperflash: CFI contains unrecognised boot bank location (0). Assuming bottom.
number of CFI chips: 1

At first, the failure looked like an endianness-issue, as reading two
bytes resulted in e.g. 0x51 0x00 instead of 0x00 0x51. But it turned
out to be reading a single 8-bit value of 0x51 instead of a 16-bit value
of 0x5100.

Note that commit 0d37f69cacb33435 ("memory: renesas-rpc-if: Correct QSPI
data transfer in Manual mode") in the BSP does not suffer from this bug,
as it bases its decision on the real number of bytes to transfer, not on
the SMENR.SPIDE register bits.
---
drivers/memory/renesas-rpc-if.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
index 3fbdd2bb8bfdb6ec..9bf880f843437dc2 100644
--- a/drivers/memory/renesas-rpc-if.c
+++ b/drivers/memory/renesas-rpc-if.c
@@ -175,10 +175,16 @@ static int rpcif_reg_read(void *context, unsigned int reg, unsigned int *val)
u32 spide = readl(rpc->base + RPCIF_SMENR) & RPCIF_SMENR_SPIDE(0xF);

if (spide == 0x8) {
- *val = readb(rpc->base + reg);
+ if (rpc->bus_size == 2)
+ *val = readw(rpc->base + reg);
+ else
+ *val = readb(rpc->base + reg);
return 0;
} else if (spide == 0xC) {
- *val = readw(rpc->base + reg);
+ if (rpc->bus_size == 2)
+ *val = readl(rpc->base + reg);
+ else
+ *val = readw(rpc->base + reg);
return 0;
} else if (spide != 0xF) {
return -EILSEQ;
@@ -198,10 +204,16 @@ static int rpcif_reg_write(void *context, unsigned int reg, unsigned int val)
u32 spide = readl(rpc->base + RPCIF_SMENR) & RPCIF_SMENR_SPIDE(0xF);

if (spide == 0x8) {
- writeb(val, rpc->base + reg);
+ if (rpc->bus_size == 2)
+ writew(val, rpc->base + reg);
+ else
+ writeb(val, rpc->base + reg);
return 0;
} else if (spide == 0xC) {
- writew(val, rpc->base + reg);
+ if (rpc->bus_size == 2)
+ writel(val, rpc->base + reg);
+ else
+ writew(val, rpc->base + reg);
return 0;
} else if (spide != 0xF) {
return -EILSEQ;
--
2.25.1


2022-03-17 09:58:22

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH RFC] memory: renesas-rpc-if: Fix HF/OSPI data transfer in Manual mode

On 11/03/2022 17:53, Geert Uytterhoeven wrote:
> HyperFlash devices fail to probe:
>
> rpc-if-hyperflash rpc-if-hyperflash: probing of hyperbus device failed
>
> In HyperFLASH or Octal-SPI Flash mode, the Transfer Data Enable bits
> (SPIDE) in the Manual Mode Enable Setting Register (SMENR) are derived
> from half of the transfer size, cfr. the rpcif_bits_set() helper
> function.
>
> Hence when converting back from Transfer Data Enable bits to transfer
> size, the bus width must be taken into account, and all Manual Mode Data
> Register access sizes must be doubled when communicating with a
> HyperFLASH or Octal-SPI Flash device.
>
> Fixes: fff53a551db50f5e ("memory: renesas-rpc-if: Correct QSPI data transfer in Manual mode")
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> Marked RFC, a (1) we should avoid the back-and-forth conversion between
> transfer size and Transfer Data Enable bits, and (2) actual HyperFlash
> data reads (which follows a completely different code path) still return
> all zeros.
>
> On Salvator-XS with unlocked HyperFlash, the HyperFlash is now detected
> again, cfr. (with DEBUG_CFI enabled):
>
> Number of erase regions: 1
> Primary Vendor Command Set: 0002 (AMD/Fujitsu Standard)
> Primary Algorithm Table at 0040
> Alternative Vendor Command Set: 0000 (None)
> No Alternate Algorithm Table
> Vcc Minimum: 1.7 V
> Vcc Maximum: 1.9 V
> No Vpp line
> Typical byte/word write timeout: 512 \xc2\xb5s
> Maximum byte/word write timeout: 2048 \xc2\xb5s
> Typical full buffer write timeout: 512 \xc2\xb5s
> Maximum full buffer write timeout: 2048 \xc2\xb5s
> Typical block erase timeout: 1024 ms
> Maximum block erase timeout: 4096 ms
> Typical chip erase timeout: 262144 ms
> Maximum chip erase timeout: 1048576 ms
> Device size: 0x4000000 bytes (64 MiB)
> Flash Device Interface description: 0x0000
> - x8-only asynchronous interface
> Max. bytes in buffer write: 0x200
> Number of Erase Block Regions: 1
> Erase Region #0: BlockSize 0x40000 bytes, 256 blocks
> rpc-if-hyperflash: Found 1 x16 devices at 0x0 in 16-bit bank. Manufacturer ID 0x000001 Chip ID 0x007000
> Amd/Fujitsu Extended Query Table at 0x0040
> Amd/Fujitsu Extended Query version 1.5.
> rpc-if-hyperflash: CFI contains unrecognised boot bank location (0). Assuming bottom.
> number of CFI chips: 1
>
> At first, the failure looked like an endianness-issue, as reading two
> bytes resulted in e.g. 0x51 0x00 instead of 0x00 0x51. But it turned
> out to be reading a single 8-bit value of 0x51 instead of a 16-bit value
> of 0x5100.
>
> Note that commit 0d37f69cacb33435 ("memory: renesas-rpc-if: Correct QSPI
> data transfer in Manual mode") in the BSP does not suffer from this bug,
> as it bases its decision on the real number of bytes to transfer, not on
> the SMENR.SPIDE register bits.
> ---
> drivers/memory/renesas-rpc-if.c | 20 ++++++++++++++++----
> 1 file changed, 16 insertions(+), 4 deletions(-)
>

Some reviews and tests will be useful. Anyway it's too late upcoming
cycle, so I will pick it up after merge window.


Best regards,
Krzysztof

2022-03-17 20:30:56

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH RFC] memory: renesas-rpc-if: Fix HF/OSPI data transfer in Manual mode


> Some reviews and tests will be useful. Anyway it's too late upcoming
> cycle, so I will pick it up after merge window.

We just discussed on IRC a better solution. So expect a new patch for
this issue. Thanks!


Attachments:
(No filename) (221.00 B)
signature.asc (849.00 B)
Download all attachments