LinuxLists.cc - [RFC PATCH] memory: renesas-rpc-if: Correct QSPI data transfer in Manual mode

[permalink] [raw]

Subject: Re: [RFC PATCH] memory: renesas-rpc-if: Correct QSPI data transfer in Manual mode

On 22/09/2021 11:10, Wolfram Sang wrote:
> This patch fixes 2 problems:
> [1] The output warning logs and data loss when performing
> mount/umount then remount the device with jffs2 format.
> [2] The access width of SMWDR[0:1]/SMRDR[0:1] register is wrong.
>
> This is the sample warning logs when performing mount/umount then
> remount the device with jffs2 format:
> jffs2: jffs2_scan_inode_node(): CRC failed on node at 0x031c51d4:
> Read 0x00034e00, calculated 0xadb272a7
>
> The reason for issue [1] is that the writing data seems to
> get messed up.
> Data is only completed when the number of bytes is divisible by 4.
> If you only have 3 bytes of data left to write, 1 garbage byte
> is inserted after the end of the write stream.
> If you only have 2 bytes of data left to write, 2 bytes of '00'
> are added into the write stream.
> If you only have 1 byte of data left to write, 2 bytes of '00'
> are added into the write stream. 1 garbage byte is inserted after
> the end of the write stream.
>
> To solve problem [1], data must be written continuously in serial
> and the write stream ends when data is out.
>
> Following HW manual 62.2.15, access to SMWDR0 register should be
> in the same size as the transfer size specified in the SPIDE[3:0]
> bits in the manual mode enable setting register (SMENR).
> Be sure to access from address 0.
>
> So, in 16-bit transfer (SPIDE[3:0]=b'1100), SMWDR0 should be
> accessed by 16-bit width.
> Similar to SMWDR1, SMDDR0/1 registers.
> In current code, SMWDR0 register is accessed by regmap_write()
> that only set up to do 32-bit width.

Is this part something worth splitting to its own patch?

>
> To solve problem [2], data must be written 16-bit or 8-bit when
> transferring 1-byte or 2-byte.
>
> Signed-off-by: Duc Nguyen <[email protected]>
> [wsa: refactored to use regmap only via reg_read/reg_write]
> Signed-off-by: Wolfram Sang <[email protected]>
> ---
>
> Hi,
>
> I could reproduce the issue by a simple:
>
> $ echo "Hello" > /dev/mtd10
>
> The original BSP patch fixed the issue but mixed regmap-acces with
> ioread/iowrite accesses. So, I refactored it to use custom regmap
> accessors. This keeps the code more readable IMO. With this patch, my
> custom test cases work as well as the JFFS2 remount mentioned in the
> commit message. Tested on a Renesas Condor board (R-Car V3M) and a
> Falcon board (R-Car V3U). I send this as RFC because this is my first
> patch for the RPC code and hope for feedback. The BSP team has been
> contacted as well for comments and testing. Nonetheless, this addresses
> a serious issue which has caused broken boards because of writing to
> unintended locations. So, I'd like to see this discussed and applied
> soon if possible.
>
> Thanks everyone,
>
> Wolfram
>
>
> drivers/memory/renesas-rpc-if.c | 113 ++++++++++++++++++++++----------
> include/memory/renesas-rpc-if.h | 1 +
> 2 files changed, 79 insertions(+), 35 deletions(-)

You sent the patch just slightly after this one:
https://lore.kernel.org/lkml/[email protected]/

Are you solving similar problem?

Best regards,
Krzysztof

2021-09-24 16:33:16

[permalink] [raw]

Subject: Re: [RFC PATCH] memory: renesas-rpc-if: Correct QSPI data transfer in Manual mode

Hi Krzysztof,

> > So, in 16-bit transfer (SPIDE[3:0]=b'1100), SMWDR0 should be
> > accessed by 16-bit width.
> > Similar to SMWDR1, SMDDR0/1 registers.
> > In current code, SMWDR0 register is accessed by regmap_write()
> > that only set up to do 32-bit width.
>
> Is this part something worth splitting to its own patch?

I don't think so because it is related. The patch ensures that a) only
8, 4, 2, or 1 byte blocks are used and b) whatever is used, the access
width to the data registers is proper. Only the combination of both
fixes the data corruption. Also, for backporting, it would be good to
not introduce dependencies, I think.

> You sent the patch just slightly after this one:
> https://lore.kernel.org/lkml/[email protected]/
>
> Are you solving similar problem?

Unlikely. I do not have HyperFlash on my board, so I can't test. But as
I understand the docs, HyperFlash in deed doubles the granularity from 8
to 16 bits when using memcpy. But I am dealing with MMIO register
accesses in manual mode here.

Thanks for the review,

Wolfram

Attachments:

(No filename) (1.10 kB)
signature.asc (849.00 B)
Download all attachments

2021-09-27 08:53:15

by Lad, Prabhakar

[permalink] [raw]

Subject: Re: [RFC PATCH] memory: renesas-rpc-if: Correct QSPI data transfer in Manual mode

Hi Wolfram,

Thank you for the patch.

On Wed, Sep 22, 2021 at 10:10 AM Wolfram Sang
<[email protected]> wrote:
>
> This patch fixes 2 problems:
> [1] The output warning logs and data loss when performing
> mount/umount then remount the device with jffs2 format.
> [2] The access width of SMWDR[0:1]/SMRDR[0:1] register is wrong.
>
> This is the sample warning logs when performing mount/umount then
> remount the device with jffs2 format:
> jffs2: jffs2_scan_inode_node(): CRC failed on node at 0x031c51d4:
> Read 0x00034e00, calculated 0xadb272a7
>
> The reason for issue [1] is that the writing data seems to
> get messed up.
> Data is only completed when the number of bytes is divisible by 4.
> If you only have 3 bytes of data left to write, 1 garbage byte
> is inserted after the end of the write stream.
> If you only have 2 bytes of data left to write, 2 bytes of '00'
> are added into the write stream.
> If you only have 1 byte of data left to write, 2 bytes of '00'
> are added into the write stream. 1 garbage byte is inserted after
> the end of the write stream.
>
> To solve problem [1], data must be written continuously in serial
> and the write stream ends when data is out.
>
> Following HW manual 62.2.15, access to SMWDR0 register should be
> in the same size as the transfer size specified in the SPIDE[3:0]
> bits in the manual mode enable setting register (SMENR).
> Be sure to access from address 0.
>
> So, in 16-bit transfer (SPIDE[3:0]=b'1100), SMWDR0 should be
> accessed by 16-bit width.
> Similar to SMWDR1, SMDDR0/1 registers.
> In current code, SMWDR0 register is accessed by regmap_write()
> that only set up to do 32-bit width.
>
> To solve problem [2], data must be written 16-bit or 8-bit when
> transferring 1-byte or 2-byte.
>
> Signed-off-by: Duc Nguyen <[email protected]>
> [wsa: refactored to use regmap only via reg_read/reg_write]
> Signed-off-by: Wolfram Sang <[email protected]>
> ---
>
> Hi,
>
> I could reproduce the issue by a simple:
>
> $ echo "Hello" > /dev/mtd10
>
> The original BSP patch fixed the issue but mixed regmap-acces with
> ioread/iowrite accesses. So, I refactored it to use custom regmap
> accessors. This keeps the code more readable IMO. With this patch, my
> custom test cases work as well as the JFFS2 remount mentioned in the
> commit message. Tested on a Renesas Condor board (R-Car V3M) and a
> Falcon board (R-Car V3U). I send this as RFC because this is my first
> patch for the RPC code and hope for feedback. The BSP team has been
> contacted as well for comments and testing. Nonetheless, this addresses
> a serious issue which has caused broken boards because of writing to
> unintended locations. So, I'd like to see this discussed and applied
> soon if possible.
>
I hit the exact same issue on RZ/G2L where erase/write operation
screwed some random sectors and made the board un-bootable. With the
patch applied, read/write/erase worked as expected. Below are the logs
on RZ/G2L SMARC EVK.

root@smarc-rzg2l:~# sh -x ./flash.sh
+ cat /proc/mtd
dev: size erasesize name
mtd0: 02000000 00001000 "boot"
mtd1: 02000000 00001000 "user"
+ flashcp -v sample.bin /dev/mtd1
Erasing blocks: 1024/1024 (100%)
Writing data: 4096k/4096k (100%)
Verifying data: 4096k/4096k (100%)
+ dd if=/dev/urandom of=/tmp/sample.bin bs=1024 count=4096
4096+0 records in
4096+0 records out
4194304 bytes (4.2 MB) copied, 0.0786743 s, 53.3 MB/s
+ flash_erase -j -q /dev/mtd1 0 0
+ mount -t jffs2 /dev/mtdblock1 /mnt
+ cp /tmp/sample.bin /mnt
+ ls -ltr /mnt
total 4096
-rw-r--r-- 1 root root 4194304 Sep 20 10:54 sample.bin
+ echo 'test write'
+ umount /mnt
+ mount -t jffs2 /dev/mtdblock1 /mnt
+ ls -ltr /mnt
total 4097
-rw-r--r-- 1 root root 11 Sep 20 10:54 write.txt
-rw-r--r-- 1 root root 4194304 Sep 20 10:54 sample.bin
+ cat /mnt/write.txt
test write
+ umount /mnt

Tested-by: Lad Prabhakar <[email protected]>

Cheers,
Prabhakar

> Thanks everyone,
>
> Wolfram
>
>
> drivers/memory/renesas-rpc-if.c | 113 ++++++++++++++++++++++----------
> include/memory/renesas-rpc-if.h | 1 +
> 2 files changed, 79 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
> index 45eed659b0c6..77a011d5ff8c 100644
> --- a/drivers/memory/renesas-rpc-if.c
> +++ b/drivers/memory/renesas-rpc-if.c
> @@ -160,10 +160,62 @@ static const struct regmap_access_table rpcif_volatile_table = {
> .n_yes_ranges = ARRAY_SIZE(rpcif_volatile_ranges),
> };
>
> +
> +/*
> + * Custom accessor functions to ensure SMRDR0 and SMWDR0 are always accessed
> + * with proper width. Requires SMENR_SPIDE to be correctly set before!
> + */
> +static int rpcif_reg_read(void *context, unsigned int reg, unsigned int *val)
> +{
> + struct rpcif *rpc = context;
> +
> + if (reg == RPCIF_SMRDR0 || reg == RPCIF_SMWDR0) {
> + u32 spide = readl(rpc->base + RPCIF_SMENR) & RPCIF_SMENR_SPIDE(0xF);
> +
> + if (spide == 0x8) {
> + *val = readb(rpc->base + reg);
> + return 0;
> + } else if (spide == 0xC) {
> + *val = readw(rpc->base + reg);
> + return 0;
> + } else if (spide != 0xF) {
> + return -EILSEQ;
> + }
> + }
> +
> + *val = readl(rpc->base + reg);
> + return 0;
> +
> +}
> +
> +static int rpcif_reg_write(void *context, unsigned int reg, unsigned int val)
> +{
> + struct rpcif *rpc = context;
> +
> + if (reg == RPCIF_SMRDR0 || reg == RPCIF_SMWDR0) {
> + u32 spide = readl(rpc->base + RPCIF_SMENR) & RPCIF_SMENR_SPIDE(0xF);
> +
> + if (spide == 0x8) {
> + writeb(val, rpc->base + reg);
> + return 0;
> + } else if (spide == 0xC) {
> + writew(val, rpc->base + reg);
> + return 0;
> + } else if (spide != 0xF) {
> + return -EILSEQ;
> + }
> + }
> +
> + writel(val, rpc->base + reg);
> + return 0;
> +}
> +
> static const struct regmap_config rpcif_regmap_config = {
> .reg_bits = 32,
> .val_bits = 32,
> .reg_stride = 4,
> + .reg_read = rpcif_reg_read,
> + .reg_write = rpcif_reg_write,
> .fast_io = true,
> .max_register = RPCIF_PHYINT,
> .volatile_table = &rpcif_volatile_table,
> @@ -173,17 +225,15 @@ int rpcif_sw_init(struct rpcif *rpc, struct device *dev)
> {
> struct platform_device *pdev = to_platform_device(dev);
> struct resource *res;
> - void __iomem *base;
>
> rpc->dev = dev;
>
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
> - base = devm_ioremap_resource(&pdev->dev, res);
> - if (IS_ERR(base))
> - return PTR_ERR(base);
> + rpc->base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(rpc->base))
> + return PTR_ERR(rpc->base);
>
> - rpc->regmap = devm_regmap_init_mmio(&pdev->dev, base,
> - &rpcif_regmap_config);
> + rpc->regmap = devm_regmap_init(&pdev->dev, NULL, rpc, &rpcif_regmap_config);
> if (IS_ERR(rpc->regmap)) {
> dev_err(&pdev->dev,
> "failed to init regmap for rpcif, error %ld\n",
> @@ -354,20 +404,16 @@ void rpcif_prepare(struct rpcif *rpc, const struct rpcif_op *op, u64 *offs,
> nbytes = op->data.nbytes;
> rpc->xferlen = nbytes;
>
> - rpc->enable |= RPCIF_SMENR_SPIDE(rpcif_bits_set(rpc, nbytes)) |
> - RPCIF_SMENR_SPIDB(rpcif_bit_size(op->data.buswidth));
> + rpc->enable |= RPCIF_SMENR_SPIDB(rpcif_bit_size(op->data.buswidth));
> }
> }
> EXPORT_SYMBOL(rpcif_prepare);
>
> int rpcif_manual_xfer(struct rpcif *rpc)
> {
> - u32 smenr, smcr, pos = 0, max = 4;
> + u32 smenr, smcr, pos = 0, max = rpc->bus_size == 2 ? 8 : 4;
> int ret = 0;
>
> - if (rpc->bus_size == 2)
> - max = 8;
> -
> pm_runtime_get_sync(rpc->dev);
>
> regmap_update_bits(rpc->regmap, RPCIF_PHYCNT,
> @@ -378,37 +424,36 @@ int rpcif_manual_xfer(struct rpcif *rpc)
> regmap_write(rpc->regmap, RPCIF_SMOPR, rpc->option);
> regmap_write(rpc->regmap, RPCIF_SMDMCR, rpc->dummy);
> regmap_write(rpc->regmap, RPCIF_SMDRENR, rpc->ddr);
> + regmap_write(rpc->regmap, RPCIF_SMADR, rpc->smadr);
> smenr = rpc->enable;
>
> switch (rpc->dir) {
> case RPCIF_DATA_OUT:
> while (pos < rpc->xferlen) {
> - u32 nbytes = rpc->xferlen - pos;
> - u32 data[2];
> + u32 bytes_left = rpc->xferlen - pos;
> + u32 nbytes, data[2];
>
> smcr = rpc->smcr | RPCIF_SMCR_SPIE;
> - if (nbytes > max) {
> - nbytes = max;
> +
> + /* nbytes may only be 1, 2, 4, or 8 */
> + nbytes = bytes_left >= max ? max : (1 << ilog2(bytes_left));
> + if (bytes_left > nbytes)
> smcr |= RPCIF_SMCR_SSLKP;
> - }
> +
> + smenr |= RPCIF_SMENR_SPIDE(rpcif_bits_set(rpc, nbytes));
> + regmap_write(rpc->regmap, RPCIF_SMENR, smenr);
>
> memcpy(data, rpc->buffer + pos, nbytes);
> - if (nbytes > 4) {
> + if (nbytes == 8) {
> regmap_write(rpc->regmap, RPCIF_SMWDR1,
> data[0]);
> regmap_write(rpc->regmap, RPCIF_SMWDR0,
> data[1]);
> - } else if (nbytes > 2) {
> + } else {
> regmap_write(rpc->regmap, RPCIF_SMWDR0,
> data[0]);
> - } else {
> - regmap_write(rpc->regmap, RPCIF_SMWDR0,
> - data[0] << 16);
> }
>
> - regmap_write(rpc->regmap, RPCIF_SMADR,
> - rpc->smadr + pos);
> - regmap_write(rpc->regmap, RPCIF_SMENR, smenr);
> regmap_write(rpc->regmap, RPCIF_SMCR, smcr);
> ret = wait_msg_xfer_end(rpc);
> if (ret)
> @@ -448,14 +493,16 @@ int rpcif_manual_xfer(struct rpcif *rpc)
> break;
> }
> while (pos < rpc->xferlen) {
> - u32 nbytes = rpc->xferlen - pos;
> - u32 data[2];
> + u32 bytes_left = rpc->xferlen - pos;
> + u32 nbytes, data[2];
>
> - if (nbytes > max)
> - nbytes = max;
> + /* nbytes may only be 1, 2, 4, or 8 */
> + nbytes = bytes_left >= max ? max : (1 << ilog2(bytes_left));
>
> regmap_write(rpc->regmap, RPCIF_SMADR,
> rpc->smadr + pos);
> + smenr &= ~RPCIF_SMENR_SPIDE(0xF);
> + smenr |= RPCIF_SMENR_SPIDE(rpcif_bits_set(rpc, nbytes));
> regmap_write(rpc->regmap, RPCIF_SMENR, smenr);
> regmap_write(rpc->regmap, RPCIF_SMCR,
> rpc->smcr | RPCIF_SMCR_SPIE);
> @@ -463,18 +510,14 @@ int rpcif_manual_xfer(struct rpcif *rpc)
> if (ret)
> goto err_out;
>
> - if (nbytes > 4) {
> + if (nbytes == 8) {
> regmap_read(rpc->regmap, RPCIF_SMRDR1,
> &data[0]);
> regmap_read(rpc->regmap, RPCIF_SMRDR0,
> &data[1]);
> - } else if (nbytes > 2) {
> - regmap_read(rpc->regmap, RPCIF_SMRDR0,
> - &data[0]);
> - } else {
> + } else {
> regmap_read(rpc->regmap, RPCIF_SMRDR0,
> &data[0]);
> - data[0] >>= 16;
> }
> memcpy(rpc->buffer + pos, data, nbytes);
>
> diff --git a/include/memory/renesas-rpc-if.h b/include/memory/renesas-rpc-if.h
> index e3e770f76f34..77c694a19149 100644
> --- a/include/memory/renesas-rpc-if.h
> +++ b/include/memory/renesas-rpc-if.h
> @@ -59,6 +59,7 @@ struct rpcif_op {
>
> struct rpcif {
> struct device *dev;
> + void __iomem *base;
> void __iomem *dirmap;
> struct regmap *regmap;
> struct reset_control *rstc;
> --
> 2.30.2
>

2021-09-27 09:46:59

[permalink] [raw]

Subject: Re: [RFC PATCH] memory: renesas-rpc-if: Correct QSPI data transfer in Manual mode

Hi Prabhakar,

On Mon, Sep 27, 2021 at 10:52 AM Lad, Prabhakar
<[email protected]> wrote:
> On Wed, Sep 22, 2021 at 10:10 AM Wolfram Sang
> <[email protected]> wrote:
> > This patch fixes 2 problems:
> > [1] The output warning logs and data loss when performing
> > mount/umount then remount the device with jffs2 format.
> > [2] The access width of SMWDR[0:1]/SMRDR[0:1] register is wrong.
> >
> > This is the sample warning logs when performing mount/umount then
> > remount the device with jffs2 format:
> > jffs2: jffs2_scan_inode_node(): CRC failed on node at 0x031c51d4:
> > Read 0x00034e00, calculated 0xadb272a7
> >
> > The reason for issue [1] is that the writing data seems to
> > get messed up.
> > Data is only completed when the number of bytes is divisible by 4.
> > If you only have 3 bytes of data left to write, 1 garbage byte
> > is inserted after the end of the write stream.
> > If you only have 2 bytes of data left to write, 2 bytes of '00'
> > are added into the write stream.
> > If you only have 1 byte of data left to write, 2 bytes of '00'
> > are added into the write stream. 1 garbage byte is inserted after
> > the end of the write stream.
> >
> > To solve problem [1], data must be written continuously in serial
> > and the write stream ends when data is out.
> >
> > Following HW manual 62.2.15, access to SMWDR0 register should be
> > in the same size as the transfer size specified in the SPIDE[3:0]
> > bits in the manual mode enable setting register (SMENR).
> > Be sure to access from address 0.
> >
> > So, in 16-bit transfer (SPIDE[3:0]=b'1100), SMWDR0 should be
> > accessed by 16-bit width.
> > Similar to SMWDR1, SMDDR0/1 registers.
> > In current code, SMWDR0 register is accessed by regmap_write()
> > that only set up to do 32-bit width.
> >
> > To solve problem [2], data must be written 16-bit or 8-bit when
> > transferring 1-byte or 2-byte.
> >
> > Signed-off-by: Duc Nguyen <[email protected]>
> > [wsa: refactored to use regmap only via reg_read/reg_write]
> > Signed-off-by: Wolfram Sang <[email protected]>
> > ---
> >
> > Hi,
> >
> > I could reproduce the issue by a simple:
> >
> > $ echo "Hello" > /dev/mtd10
> >
> > The original BSP patch fixed the issue but mixed regmap-acces with
> > ioread/iowrite accesses. So, I refactored it to use custom regmap
> > accessors. This keeps the code more readable IMO. With this patch, my
> > custom test cases work as well as the JFFS2 remount mentioned in the
> > commit message. Tested on a Renesas Condor board (R-Car V3M) and a
> > Falcon board (R-Car V3U). I send this as RFC because this is my first
> > patch for the RPC code and hope for feedback. The BSP team has been
> > contacted as well for comments and testing. Nonetheless, this addresses
> > a serious issue which has caused broken boards because of writing to
> > unintended locations. So, I'd like to see this discussed and applied
> > soon if possible.
> >
> I hit the exact same issue on RZ/G2L where erase/write operation
> screwed some random sectors and made the board un-bootable. With the
> patch applied, read/write/erase worked as expected. Below are the logs
> on RZ/G2L SMARC EVK.
>
> root@smarc-rzg2l:~# sh -x ./flash.sh
> + cat /proc/mtd
> dev: size erasesize name
> mtd0: 02000000 00001000 "boot"
> mtd1: 02000000 00001000 "user"
> + flashcp -v sample.bin /dev/mtd1
> Erasing blocks: 1024/1024 (100%)
> Writing data: 4096k/4096k (100%)
> Verifying data: 4096k/4096k (100%)
> + dd if=/dev/urandom of=/tmp/sample.bin bs=1024 count=4096
> 4096+0 records in
> 4096+0 records out
> 4194304 bytes (4.2 MB) copied, 0.0786743 s, 53.3 MB/s
> + flash_erase -j -q /dev/mtd1 0 0
> + mount -t jffs2 /dev/mtdblock1 /mnt
> + cp /tmp/sample.bin /mnt
> + ls -ltr /mnt
> total 4096
> -rw-r--r-- 1 root root 4194304 Sep 20 10:54 sample.bin
> + echo 'test write'
> + umount /mnt
> + mount -t jffs2 /dev/mtdblock1 /mnt
> + ls -ltr /mnt
> total 4097
> -rw-r--r-- 1 root root 11 Sep 20 10:54 write.txt
> -rw-r--r-- 1 root root 4194304 Sep 20 10:54 sample.bin
> + cat /mnt/write.txt
> test write
> + umount /mnt
>
> Tested-by: Lad Prabhakar <[email protected]>

Might be a good idea to update the erase test to make a copy first,
and verify that only the wanted blocks have been affected by the erase.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2021-09-27 19:52:02

by Lad, Prabhakar

[permalink] [raw]

Subject: Re: [RFC PATCH] memory: renesas-rpc-if: Correct QSPI data transfer in Manual mode

Hi Geert,

On Mon, Sep 27, 2021 at 10:46 AM Geert Uytterhoeven
<[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Mon, Sep 27, 2021 at 10:52 AM Lad, Prabhakar
> <[email protected]> wrote:
> > On Wed, Sep 22, 2021 at 10:10 AM Wolfram Sang
> > <[email protected]> wrote:
> > > This patch fixes 2 problems:
> > > [1] The output warning logs and data loss when performing
> > > mount/umount then remount the device with jffs2 format.
> > > [2] The access width of SMWDR[0:1]/SMRDR[0:1] register is wrong.
> > >
> > > This is the sample warning logs when performing mount/umount then
> > > remount the device with jffs2 format:
> > > jffs2: jffs2_scan_inode_node(): CRC failed on node at 0x031c51d4:
> > > Read 0x00034e00, calculated 0xadb272a7
> > >
> > > The reason for issue [1] is that the writing data seems to
> > > get messed up.
> > > Data is only completed when the number of bytes is divisible by 4.
> > > If you only have 3 bytes of data left to write, 1 garbage byte
> > > is inserted after the end of the write stream.
> > > If you only have 2 bytes of data left to write, 2 bytes of '00'
> > > are added into the write stream.
> > > If you only have 1 byte of data left to write, 2 bytes of '00'
> > > are added into the write stream. 1 garbage byte is inserted after
> > > the end of the write stream.
> > >
> > > To solve problem [1], data must be written continuously in serial
> > > and the write stream ends when data is out.
> > >
> > > Following HW manual 62.2.15, access to SMWDR0 register should be
> > > in the same size as the transfer size specified in the SPIDE[3:0]
> > > bits in the manual mode enable setting register (SMENR).
> > > Be sure to access from address 0.
> > >
> > > So, in 16-bit transfer (SPIDE[3:0]=b'1100), SMWDR0 should be
> > > accessed by 16-bit width.
> > > Similar to SMWDR1, SMDDR0/1 registers.
> > > In current code, SMWDR0 register is accessed by regmap_write()
> > > that only set up to do 32-bit width.
> > >
> > > To solve problem [2], data must be written 16-bit or 8-bit when
> > > transferring 1-byte or 2-byte.
> > >
> > > Signed-off-by: Duc Nguyen <[email protected]>
> > > [wsa: refactored to use regmap only via reg_read/reg_write]
> > > Signed-off-by: Wolfram Sang <[email protected]>
> > > ---
> > >
> > > Hi,
> > >
> > > I could reproduce the issue by a simple:
> > >
> > > $ echo "Hello" > /dev/mtd10
> > >
> > > The original BSP patch fixed the issue but mixed regmap-acces with
> > > ioread/iowrite accesses. So, I refactored it to use custom regmap
> > > accessors. This keeps the code more readable IMO. With this patch, my
> > > custom test cases work as well as the JFFS2 remount mentioned in the
> > > commit message. Tested on a Renesas Condor board (R-Car V3M) and a
> > > Falcon board (R-Car V3U). I send this as RFC because this is my first
> > > patch for the RPC code and hope for feedback. The BSP team has been
> > > contacted as well for comments and testing. Nonetheless, this addresses
> > > a serious issue which has caused broken boards because of writing to
> > > unintended locations. So, I'd like to see this discussed and applied
> > > soon if possible.
> > >
> > I hit the exact same issue on RZ/G2L where erase/write operation
> > screwed some random sectors and made the board un-bootable. With the
> > patch applied, read/write/erase worked as expected. Below are the logs
> > on RZ/G2L SMARC EVK.
> >
> > root@smarc-rzg2l:~# sh -x ./flash.sh
> > + cat /proc/mtd
> > dev: size erasesize name
> > mtd0: 02000000 00001000 "boot"
> > mtd1: 02000000 00001000 "user"
> > + flashcp -v sample.bin /dev/mtd1
> > Erasing blocks: 1024/1024 (100%)
> > Writing data: 4096k/4096k (100%)
> > Verifying data: 4096k/4096k (100%)
> > + dd if=/dev/urandom of=/tmp/sample.bin bs=1024 count=4096
> > 4096+0 records in
> > 4096+0 records out
> > 4194304 bytes (4.2 MB) copied, 0.0786743 s, 53.3 MB/s
> > + flash_erase -j -q /dev/mtd1 0 0
> > + mount -t jffs2 /dev/mtdblock1 /mnt
> > + cp /tmp/sample.bin /mnt
> > + ls -ltr /mnt
> > total 4096
> > -rw-r--r-- 1 root root 4194304 Sep 20 10:54 sample.bin
> > + echo 'test write'
> > + umount /mnt
> > + mount -t jffs2 /dev/mtdblock1 /mnt
> > + ls -ltr /mnt
> > total 4097
> > -rw-r--r-- 1 root root 11 Sep 20 10:54 write.txt
> > -rw-r--r-- 1 root root 4194304 Sep 20 10:54 sample.bin
> > + cat /mnt/write.txt
> > test write
> > + umount /mnt
> >
> > Tested-by: Lad Prabhakar <[email protected]>
>
> Might be a good idea to update the erase test to make a copy first,
> and verify that only the wanted blocks have been affected by the erase.
>
Right, I have updated my erase test. Below are logs for my erase test
(from a give offset),

root@smarc-rzg2l:~# flashcp -v sample.bin /dev/mtd1
Erasing blocks: 1024/1024 (100%)
Writing data: 4096k/4096k (100%)
Verifying data: 4096k/4096k (100%)
root@smarc-rzg2l:~# flash_erase /dev/mtd1 4096 1
Erasing 4 Kibyte @ 1000 -- 100 % complete
root@smarc-rzg2l:~# hexdump -C /dev/mtd1 | less
00000fd0 c0 2b 03 33 96 ff 87 5b f7 96 b5 a9 de 57 eb 2f |.+.3...[.....W./|
00000fe0 11 70 11 f1 71 53 48 94 67 c8 0e 53 34 76 f4 f6 |.p..qSH.g..S4v..|
00000ff0 a0 ec ed 8d 62 f3 f2 5a d0 0a 66 74 95 a7 91 7b |....b..Z..ft...{|
00001000 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
*
00002000 ae d0 f4 f5 4b 66 b7 6f 2c 09 04 f1 10 58 40 b0 |....Kf.o,....X@.|
00002010 e5 27 6a d7 1f 8e af 8f ff 57 34 75 6d b7 7d 99 |.'j......W4um.}.|
00002020 d8 e1 88 20 1a 83 37 e3 54 df 72 76 0a ec 1e 76 |... ..7.T.rv...v|
00002030 84 5c 05 f2 88 61 72 cd 34 4a 71 62 68 b7 b0 f6 |.\...ar.4Jqbh...|
00002040 2e eb a5 d6 79 d5 4d 1a 44 26 e9 77 0d 72 fb f3 |....y.M.D&.w.r..|
00002050 36 64 5a a0 44 1a 35 14 79 69 94 78 78 34 f2 04 |6dZ.D.5.yi.xx4..|
00002060 13 91 3a 5c 07 28 61 c8 a7 82 bc f6 7f 87 d4 da |..:\.(a.........|
00002070 b4 ec 27 b6 f2 7c 07 c8 b3 d3 8b 8e 1f 5e 75 97 |..'..|.......^u.|
00002080 14 e7 ac b0 bd 3a 20 ce ed 6a be 53 21 a3 7e 64 |.....: ..j.S!.~d|
00002090 99 0b 61 f0 dd 4c f6 90 c0 aa f4 52 8c 67 05 d0 |..a..L.....R.g..|
000020a0 b8 eb 0e 1e b8 40 09 52 ac 23 57 7f bd 94 3b 7a |[email protected].#W...;z|
000020b0 8e 8b 10 7a db bc 9f f8 15 dd 41 ac 92 cc b6 3f |...z......A....?|
000020c0 67 57 dd d0 fc f1 6e 1e 27 d8 4f 62 98 71 74 ea |gW....n.'.Ob.qt.|
000020d0 8c 62 82 50 8d ed 5b f1 a6 f1 99 7c e9 f1 8e 08 |.b.P..[....|....|
000020e0 48 c7 2d 73 83 03 96 78 f4 64 57 94 95 64 23 c2 |H.-s...x.dW..d#.|
000020f0 6f 53 32 e7 43 1b 5e 25 a8 b0 34 17 1f 33 4d f4 |oS2.C.^%..4..3M.|
00002100 30 95 91 4d f1 06 37 09 71 f3 ce 5d be f8 62 96 |0..M..7.q..]..b.|
00002110 0f d4 26 cb eb 50 a3 4c 81 6f 1c 8a e6 a2 c6 d3 |..&..P.L.o......|

Cheers,
Prabhakar

> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds

2021-09-27 20:12:58

[permalink] [raw]

Subject: Re: [RFC PATCH] memory: renesas-rpc-if: Correct QSPI data transfer in Manual mode

Hi Prabhakar,

> I hit the exact same issue on RZ/G2L where erase/write operation
> screwed some random sectors and made the board un-bootable. With the
> patch applied, read/write/erase worked as expected. Below are the logs
> on RZ/G2L SMARC EVK.

Thanks for testing. Glad it works for you as well!

All the best,

Wolfram

Attachments:

(No filename) (342.00 B)
signature.asc (849.00 B)
Download all attachments

2021-09-28 09:49:07

[permalink] [raw]

Subject: Re: [RFC PATCH] memory: renesas-rpc-if: Correct QSPI data transfer in Manual mode

On 22/09/2021 11:10, Wolfram Sang wrote:
> This patch fixes 2 problems:
> [1] The output warning logs and data loss when performing
> mount/umount then remount the device with jffs2 format.
> [2] The access width of SMWDR[0:1]/SMRDR[0:1] register is wrong.
>
> This is the sample warning logs when performing mount/umount then
> remount the device with jffs2 format:
> jffs2: jffs2_scan_inode_node(): CRC failed on node at 0x031c51d4:
> Read 0x00034e00, calculated 0xadb272a7
>
> The reason for issue [1] is that the writing data seems to
> get messed up.
> Data is only completed when the number of bytes is divisible by 4.
> If you only have 3 bytes of data left to write, 1 garbage byte
> is inserted after the end of the write stream.
> If you only have 2 bytes of data left to write, 2 bytes of '00'
> are added into the write stream.
> If you only have 1 byte of data left to write, 2 bytes of '00'
> are added into the write stream. 1 garbage byte is inserted after
> the end of the write stream.
>
> To solve problem [1], data must be written continuously in serial
> and the write stream ends when data is out.
>
> Following HW manual 62.2.15, access to SMWDR0 register should be
> in the same size as the transfer size specified in the SPIDE[3:0]
> bits in the manual mode enable setting register (SMENR).
> Be sure to access from address 0.
>
> So, in 16-bit transfer (SPIDE[3:0]=b'1100), SMWDR0 should be
> accessed by 16-bit width.
> Similar to SMWDR1, SMDDR0/1 registers.
> In current code, SMWDR0 register is accessed by regmap_write()
> that only set up to do 32-bit width.
>
> To solve problem [2], data must be written 16-bit or 8-bit when
> transferring 1-byte or 2-byte.
>
> Signed-off-by: Duc Nguyen <[email protected]>
> [wsa: refactored to use regmap only via reg_read/reg_write]
> Signed-off-by: Wolfram Sang <[email protected]>
> ---
>
> Hi,
>
> I could reproduce the issue by a simple:
>
> $ echo "Hello" > /dev/mtd10
>
> The original BSP patch fixed the issue but mixed regmap-acces with
> ioread/iowrite accesses. So, I refactored it to use custom regmap
> accessors. This keeps the code more readable IMO. With this patch, my
> custom test cases work as well as the JFFS2 remount mentioned in the
> commit message. Tested on a Renesas Condor board (R-Car V3M) and a
> Falcon board (R-Car V3U). I send this as RFC because this is my first
> patch for the RPC code and hope for feedback. The BSP team has been
> contacted as well for comments and testing. Nonetheless, this addresses
> a serious issue which has caused broken boards because of writing to
> unintended locations. So, I'd like to see this discussed and applied
> soon if possible.
>
> Thanks everyone,
>
> Wolfram
>

Thanks for testing and comments. Any Fixes tag for it?

Best regards,
Krzysztof

2021-09-28 10:33:56

[permalink] [raw]

Subject: Re: [RFC PATCH] memory: renesas-rpc-if: Correct QSPI data transfer in Manual mode

> Thanks for testing and comments. Any Fixes tag for it?

Sure:

Fixes: ca7d8b980b67 ("memory: add Renesas RPC-IF driver")

Thanks!

Attachments:

(No filename) (143.00 B)
signature.asc (849.00 B)
Download all attachments

2021-09-28 10:37:14

[permalink] [raw]

Subject: Re: [RFC PATCH] memory: renesas-rpc-if: Correct QSPI data transfer in Manual mode

On Wed, 22 Sep 2021 11:10:06 +0200, Wolfram Sang wrote:
> This patch fixes 2 problems:
> [1] The output warning logs and data loss when performing
> mount/umount then remount the device with jffs2 format.
> [2] The access width of SMWDR[0:1]/SMRDR[0:1] register is wrong.
>
> This is the sample warning logs when performing mount/umount then
> remount the device with jffs2 format:
> jffs2: jffs2_scan_inode_node(): CRC failed on node at 0x031c51d4:
> Read 0x00034e00, calculated 0xadb272a7
>
> [...]

Applied, thanks!

[1/1] memory: renesas-rpc-if: Correct QSPI data transfer in Manual mode
commit: fff53a551db50f5edecaa0b29a64056ab8d2bbca

Best regards,
--
Krzysztof Kozlowski <[email protected]>

2022-03-07 21:27:39

[permalink] [raw]

Subject: Re: [RFC PATCH] memory: renesas-rpc-if: Correct QSPI data transfer in Manual mode

On 07/03/2022 17:44, Geert Uytterhoeven wrote:
> Hi Krzysztof and Wolfram,
>
> CC Sergey, Prabhakar, Andrew,
>
> On Tue, Sep 28, 2021 at 12:36 PM Krzysztof Kozlowski
> <[email protected]> wrote:
>> On Wed, 22 Sep 2021 11:10:06 +0200, Wolfram Sang wrote:
>>> This patch fixes 2 problems:
>>> [1] The output warning logs and data loss when performing
>>> mount/umount then remount the device with jffs2 format.
>>> [2] The access width of SMWDR[0:1]/SMRDR[0:1] register is wrong.
>>>
>>> This is the sample warning logs when performing mount/umount then
>>> remount the device with jffs2 format:
>>> jffs2: jffs2_scan_inode_node(): CRC failed on node at 0x031c51d4:
>>> Read 0x00034e00, calculated 0xadb272a7
>>>
>>> [...]
>>
>> Applied, thanks!
>>
>> [1/1] memory: renesas-rpc-if: Correct QSPI data transfer in Manual mode
>> commit: fff53a551db50f5edecaa0b29a64056ab8d2bbca
>
> While trying to enable support for RPC on Salvator-X(S)[*], I
> discovered HyperFLASH detection is broken:
>
> rpc-if-hyperflash rpc-if-hyperflash: probing of hyperbus device failed
>
> Reverting all commits to the RPC driver since Sergey's original
> commit 5de15b610f785f0e ("mtd: hyperbus: add Renesas RPC-IF driver")
> fixed probing:
>
> 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
> 10 fixed-partitions partitions found on MTD device rpc-if-hyperflash
> Creating 10 MTD partitions on "rpc-if-hyperflash":
> 0x000000000000-0x000000040000 : "bootparam"
> [...]
>
> However, there's still something wrong, as all HyperFLASH contents read
> back as zeros, while the FLASH contains the full boot loader stack.
> Bisecting the reverts pointed to this patch, and just reverting this
> patch (small whitespace conflict) fixes probing, too. Still, all zeros.
>
> Summary: needs more investigation ;-)
>
> Wolfram: which platform did you use for QSPI testing, so I don't
> break that again?
>
> [*] Firmware compiled with RCAR_RPC_HYPERFLASH_LOCKED=0 of course.
> Without that (e.g. old H3 R-Car ES1.0), it crashes with an
> Asynchronous SError Interrupt during driver initialization.

Thanks for letting me know. This patch is already in mainline, so the
solution is either to revert it or fix it. I will wait for it from you
(as in plural "you" :) ).

Best regards,
Krzysztof

2022-03-08 09:32:36

[permalink] [raw]

Subject: Re: [RFC PATCH] memory: renesas-rpc-if: Correct QSPI data transfer in Manual mode

Hi Krzysztof and Wolfram,

CC Sergey, Prabhakar, Andrew,

On Tue, Sep 28, 2021 at 12:36 PM Krzysztof Kozlowski
<[email protected]> wrote:
> On Wed, 22 Sep 2021 11:10:06 +0200, Wolfram Sang wrote:
> > This patch fixes 2 problems:
> > [1] The output warning logs and data loss when performing
> > mount/umount then remount the device with jffs2 format.
> > [2] The access width of SMWDR[0:1]/SMRDR[0:1] register is wrong.
> >
> > This is the sample warning logs when performing mount/umount then
> > remount the device with jffs2 format:
> > jffs2: jffs2_scan_inode_node(): CRC failed on node at 0x031c51d4:
> > Read 0x00034e00, calculated 0xadb272a7
> >
> > [...]
>
> Applied, thanks!
>
> [1/1] memory: renesas-rpc-if: Correct QSPI data transfer in Manual mode
> commit: fff53a551db50f5edecaa0b29a64056ab8d2bbca

While trying to enable support for RPC on Salvator-X(S)[*], I
discovered HyperFLASH detection is broken:

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

Reverting all commits to the RPC driver since Sergey's original
commit 5de15b610f785f0e ("mtd: hyperbus: add Renesas RPC-IF driver")
fixed probing:

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
10 fixed-partitions partitions found on MTD device rpc-if-hyperflash
Creating 10 MTD partitions on "rpc-if-hyperflash":
0x000000000000-0x000000040000 : "bootparam"
[...]

However, there's still something wrong, as all HyperFLASH contents read
back as zeros, while the FLASH contains the full boot loader stack.
Bisecting the reverts pointed to this patch, and just reverting this
patch (small whitespace conflict) fixes probing, too. Still, all zeros.

Summary: needs more investigation ;-)

Wolfram: which platform did you use for QSPI testing, so I don't
break that again?

[*] Firmware compiled with RCAR_RPC_HYPERFLASH_LOCKED=0 of course.
Without that (e.g. old H3 R-Car ES1.0), it crashes with an
Asynchronous SError Interrupt during driver initialization.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2022-03-08 11:50:08

[permalink] [raw]

Subject: Re: [RFC PATCH] memory: renesas-rpc-if: Correct QSPI data transfer in Manual mode

Hi Wolfram,

On Tue, Mar 8, 2022 at 9:46 AM Wolfram Sang
<[email protected]> wrote:
> > This is not QSPI, but HF.
>
> Ah, okay.
>
> > Building a new firmware for R-Car H3 ES1.0 with HF unlocked will be
> > complicated, as it is not supported by upstream TF-A.
>
> You mean QSPI here?

No, HF. Salvator-X(S) boots from HF.

> > Note that HF also fails to probe on R-Car M3-W and M3-N ES1.0.
>
> Do you have this patch form Andrew in your tree:
>
> [PATCH] memory: renesas-rpc-if: Avoid unaligned bus access for HyperFlash

Sure I have it. It's in v5.16 ;-)

> Even if so, I don't think that reverting patches is the solution. As you

Sure, a plain revert is definitely not the right solution.

> could see from Andrew's patch, HyperFlash was also broken before and it
> just may need more fixes for Gen3 perhaps? IIRC my patches didn't break
> Andrew's tests but maybe we should ask him again. Maybe Andrew has also
> some more ideas, I only did QSPI.

My understanding from reading the threads is that Andrew's patch and
yours arrived in parallel, and are believed to fix two different and
non-intersecting things (QSPI vs. HF). I also found no explicit mention
that Andrew tried your patch.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2022-03-08 15:31:47

[permalink] [raw]

Subject: Re: [RFC PATCH] memory: renesas-rpc-if: Correct QSPI data transfer in Manual mode

> This is not QSPI, but HF.

Ah, okay.

> Building a new firmware for R-Car H3 ES1.0 with HF unlocked will be
> complicated, as it is not supported by upstream TF-A.

You mean QSPI here?

> Note that HF also fails to probe on R-Car M3-W and M3-N ES1.0.

Do you have this patch form Andrew in your tree:

[PATCH] memory: renesas-rpc-if: Avoid unaligned bus access for HyperFlash

Even if so, I don't think that reverting patches is the solution. As you
could see from Andrew's patch, HyperFlash was also broken before and it
just may need more fixes for Gen3 perhaps? IIRC my patches didn't break
Andrew's tests but maybe we should ask him again. Maybe Andrew has also
some more ideas, I only did QSPI.

Attachments:

(No filename) (726.00 B)
signature.asc (849.00 B)
Download all attachments

2022-03-08 23:01:43

[permalink] [raw]

Subject: Re: [RFC PATCH] memory: renesas-rpc-if: Correct QSPI data transfer in Manual mode

Hi Krzysztof,

On Mon, Mar 7, 2022 at 6:47 PM Krzysztof Kozlowski
<[email protected]> wrote:
> On 07/03/2022 17:44, Geert Uytterhoeven wrote:
> > On Tue, Sep 28, 2021 at 12:36 PM Krzysztof Kozlowski
> > <[email protected]> wrote:
> >> On Wed, 22 Sep 2021 11:10:06 +0200, Wolfram Sang wrote:
> >>> This patch fixes 2 problems:
> >>> [1] The output warning logs and data loss when performing
> >>> mount/umount then remount the device with jffs2 format.
> >>> [2] The access width of SMWDR[0:1]/SMRDR[0:1] register is wrong.
> >>>
> >>> This is the sample warning logs when performing mount/umount then
> >>> remount the device with jffs2 format:
> >>> jffs2: jffs2_scan_inode_node(): CRC failed on node at 0x031c51d4:
> >>> Read 0x00034e00, calculated 0xadb272a7
> >>>
> >>> [...]
> >>
> >> Applied, thanks!
> >>
> >> [1/1] memory: renesas-rpc-if: Correct QSPI data transfer in Manual mode
> >> commit: fff53a551db50f5edecaa0b29a64056ab8d2bbca
> >
> > While trying to enable support for RPC on Salvator-X(S)[*], I
> > discovered HyperFLASH detection is broken:
> >
> > rpc-if-hyperflash rpc-if-hyperflash: probing of hyperbus device failed

[...]

> Thanks for letting me know. This patch is already in mainline, so the
> solution is either to revert it or fix it. I will wait for it from you
> (as in plural "you" :) ).

Thanks, that's exactly what I had expected ;-)

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2022-03-08 23:47:58

[permalink] [raw]

Subject: Re: [RFC PATCH] memory: renesas-rpc-if: Correct QSPI data transfer in Manual mode

> Wolfram: which platform did you use for QSPI testing, so I don't
> break that again?

I did my refactoring using an Eagle board and once this worked, I
enabled QSPI on Falcon. All remotely. Condor was another candidate but
it was broken in the lab at that time.

> Without that (e.g. old H3 R-Car ES1.0), it crashes with an

Frankly, I wouldn't trust ES1.0 as a reliable source for QSPI. Could you
start with the newest Gen3 board you have and then go to previous ones?

Attachments:

(No filename) (491.00 B)
signature.asc (849.00 B)
Download all attachments

2022-03-09 00:33:41

[permalink] [raw]

Subject: Re: [RFC PATCH] memory: renesas-rpc-if: Correct QSPI data transfer in Manual mode

Hi Wolfram,

On Tue, Mar 8, 2022 at 9:21 AM Wolfram Sang
<[email protected]> wrote:
> > Wolfram: which platform did you use for QSPI testing, so I don't
> > break that again?
>
> I did my refactoring using an Eagle board and once this worked, I
> enabled QSPI on Falcon. All remotely. Condor was another candidate but
> it was broken in the lab at that time.

OK, thanks!

> > Without that (e.g. old H3 R-Car ES1.0), it crashes with an
>
> Frankly, I wouldn't trust ES1.0 as a reliable source for QSPI. Could you
> start with the newest Gen3 board you have and then go to previous ones?

This is not QSPI, but HF.

Building a new firmware for R-Car H3 ES1.0 with HF unlocked will be
complicated, as it is not supported by upstream TF-A.

Note that HF also fails to probe on R-Car M3-W and M3-N ES1.0.
Haven't tried it on R-Car E3 yet. All those have a (not so new) TF-A,
but built with RCAR_RPC_HYPERFLASH_LOCKED=0.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2022-03-11 22:25:49

[permalink] [raw]

Subject: Re: [RFC PATCH] memory: renesas-rpc-if: Correct QSPI data transfer in Manual mode

On Mon, Mar 7, 2022 at 5:44 PM Geert Uytterhoeven <[email protected]> wrote:
> On Tue, Sep 28, 2021 at 12:36 PM Krzysztof Kozlowski
> <[email protected]> wrote:
> > On Wed, 22 Sep 2021 11:10:06 +0200, Wolfram Sang wrote:
> > > This patch fixes 2 problems:
> > > [1] The output warning logs and data loss when performing
> > > mount/umount then remount the device with jffs2 format.
> > > [2] The access width of SMWDR[0:1]/SMRDR[0:1] register is wrong.
> > >
> > > This is the sample warning logs when performing mount/umount then
> > > remount the device with jffs2 format:
> > > jffs2: jffs2_scan_inode_node(): CRC failed on node at 0x031c51d4:
> > > Read 0x00034e00, calculated 0xadb272a7
> > >
> > > [...]
> >
> > Applied, thanks!
> >
> > [1/1] memory: renesas-rpc-if: Correct QSPI data transfer in Manual mode
> > commit: fff53a551db50f5edecaa0b29a64056ab8d2bbca
>
> While trying to enable support for RPC on Salvator-X(S)[*], I
> discovered HyperFLASH detection is broken:
>
> rpc-if-hyperflash rpc-if-hyperflash: probing of hyperbus device failed

I've sent an RFC fix, see
https://lore.kernel.org/all/27107f2d578b198078df841ee2e4d7b71b183898.1647017136.git.geert+renesas@glider.be

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2022-04-27 11:40:50