Re: [PATCH RFT 0/2/2] mtd: hyperbus: add Renesas RPC-IF driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Sergei,

On 07.02.20 20:09, Sergei Shtylyov wrote:
Hello!

On 02/07/2020 03:59 PM, Behme Dirk (CM/ESO2) wrote:

Add the HyperFLash driver for the Renesas RPC-IF.  It's the "front end"
driver using the "back end" APIs in the main driver to talk to the real
hardware.

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@xxxxxxxxxxxxxxxxxx>
[...]
Index: linux/drivers/mtd/hyperbus/rpc-if.c
===================================================================
--- /dev/null
+++ linux/drivers/mtd/hyperbus/rpc-if.c
@@ -0,0 +1,162 @@
[...]
+static u16 rpcif_hb_read16(struct hyperbus_device *hbdev, unsigned long addr)
+{
+    struct rpcif_hyperbus *hyperbus =
+        container_of(hbdev, struct rpcif_hyperbus, hbdev);
+    struct rpcif_op op = rpcif_op_tmpl;
+    map_word data;
+
+    op.cmd.opcode = 0xC0;
+    op.addr.val = addr >> 1;
+    op.dummy.buswidth = 1;
+    op.dummy.ncycles = 15;
+    op.data.dir = RPCIF_DATA_IN;
+    op.data.nbytes = 2;
+    op.data.buf.in = &data;
+    rpcif_prepare(&hyperbus->rpc, &op, NULL, NULL); // ?
+    rpcif_io_xfer(&hyperbus->rpc);
+
+    return be16_to_cpu(data.x[0]);
+}
+
+static void rpcif_hb_write16(struct hyperbus_device *hbdev, unsigned long addr,
+                 u16 data)
+{
+    struct rpcif_hyperbus *hyperbus =
+        container_of(hbdev, struct rpcif_hyperbus, hbdev);
+    struct rpcif_op op = rpcif_op_tmpl;
+
+    op.cmd.opcode = 0x40;
+    op.addr.val = addr >> 1;
+    op.data.dir = RPCIF_DATA_OUT;
+    op.data.nbytes = 2;
+    op.data.buf.out = &data;
+    cpu_to_be16s(&data);



Testing this, I found that writing data to the Hyperflash results in swapped _data_ in Hyperflash due to this cpu_to_be16s() conversion:

02 01 04 03 06 05 08 07 ...

Breaking the usage of the data written for other users, i.e. the boot loaders.

On the other hand, dropping this cpu_to_be16s() (and be16_to_cpu() in the read16 above) makes the probing to fail completely.

The topic seems to be that rpcif_hb_write16() handles command _and_ data, and the commands seem to need the conversion.

    The HyperBus spec says the register space is always big-endian but the again
HypoerFlash doesn't have the register space...

As mentioned, the first idea, dropping the conversion and adding some debug output in the driver [1] results in failed probe [2]. Successful probing of the unmodified driver  results in [3], then.

Seems I need some advice: Why is this conversion for successful probe required?
Why is the first 'QRY' returned by the device not detected by cfi_qry_mode_on()?

    "QRY" is in the MSBs?


Well, even if we have swapping enabled and with this it's in the LSBs, it's not detected in the first run. See the first 5 traces in [3] below.


Is the any possibility to drop the conversion _and_ make the driver probe successful? Or do we need to split the path the commands and the data are routed? If so, how?

    I've found some interesting options under the CFI advanced config options,
e.g. "Flash cmd/query data swapping" having MTD_CFI_BE_BYTE_SWAP value in this
item. With this variant chosen, I don't need any byte swapping in the driver
any more... and the QRY signature is read correctly on the very 1st try.


Yes, but ;)

I tried MTD_CFI_BE_BYTE_SWAP config option, too. Enabling that and dropping cpu_to_be16s()/be16_to_cpu() in the driver result in a successful probe. And /dev/mtdx afterwards. That's the good news.

But, the bad news:

Trying a write (dd to /dev/mtdx) hanged and never returned. In contrast to the solution with the cpu_to_be16s()/be16_to_cpu() in the driver, which wrote nicely to the Hyperflash, but swapped.

Best regards

Dirk


Many questions ;)

    Hopefully an answer was found.

Best regards

Dirk


[1] Dropping be16_to_cpu() & cpu_to_be16s() and adding some debug output:

diff --git a/drivers/mtd/chips/cfi_util.c b/drivers/mtd/chips/cfi_util.c
index 6f16552cd59f..e5dd8dd3b594 100644
--- a/drivers/mtd/chips/cfi_util.c
+++ b/drivers/mtd/chips/cfi_util.c
@@ -239,9 +239,13 @@ int __xipram cfi_qry_present(struct map_info *map, __u32 base,
  }
  EXPORT_SYMBOL_GPL(cfi_qry_present);

+static unsigned int count = 1;
+
  int __xipram cfi_qry_mode_on(uint32_t base, struct map_info *map,
                              struct cfi_private *cfi)
  {
+       pr_err("cfi_qry_mode_on() called #%i\n", count++);
+
         cfi_send_gen_cmd(0xF0, 0, base, map, cfi, cfi->device_type, NULL);
         cfi_send_gen_cmd(0x98, 0x55, base, map, cfi, cfi->device_type, NULL);
         if (cfi_qry_present(map, base, cfi))
@@ -273,6 +277,9 @@ int __xipram cfi_qry_mode_on(uint32_t base, struct map_info *map,
         if (cfi_qry_present(map, base, cfi))
                 return 1;
         /* QRY not found */
+
+       pr_err("cfi_qry_mode_on() failed\n");
+
         return 0;
  }
  EXPORT_SYMBOL_GPL(cfi_qry_mode_on);
diff --git a/drivers/mtd/hyperbus/rpc-if.c b/drivers/mtd/hyperbus/rpc-if.c
index a66a5080b482..bb83a8f3f3bc 100644
--- a/drivers/mtd/hyperbus/rpc-if.c
+++ b/drivers/mtd/hyperbus/rpc-if.c
@@ -60,7 +60,11 @@ static u16 rpcif_hb_read16(struct hyperbus_device *hbdev, unsigned long addr)
         rpcif_prepare(&hyperbus->rpc, &op, NULL, NULL); // ?
         rpcif_io_xfer(&hyperbus->rpc);

-       return be16_to_cpu(data.x[0]);
+       pr_err("read:  a: 0x%08lX  d: 0x%04X %c %c\n", addr, (unsigned short)data.x[0],
+              (unsigned char)((data.x[0] >> 8) & 0xFF),
+              (unsigned char)data.x[0]);
+
+       return data.x[0];
  }

  static void rpcif_hb_write16(struct hyperbus_device *hbdev, unsigned long addr,
@@ -75,7 +79,7 @@ static void rpcif_hb_write16(struct hyperbus_device *hbdev, unsigned long addr,
         op.data.dir = RPCIF_DATA_OUT;
         op.data.nbytes = 2;
         op.data.buf.out = &data;
-       cpu_to_be16s(&data);
+       pr_err("write: a: 0x%08lX  d: 0x%04X\n", addr, data);
         rpcif_prepare(&hyperbus->rpc, &op, NULL, NULL); // ?
         rpcif_io_xfer(&hyperbus->rpc);
  }


[2] Probe fails without be16_to_cpu/cpu_to_be16s:

cfi_qry_mode_on() called #1
write: a: 0x00000000  d: 0xF0F0
write: a: 0x000000AA  d: 0x9898
read:  a: 0x00000020  d: 0x5100 Q
read:  a: 0x00000022  d: 0x5200 R
read:  a: 0x00000024  d: 0x5900 Y
write: a: 0x00000000  d: 0xF0F0
write: a: 0x00000000  d: 0xFFFF
write: a: 0x000000AA  d: 0x9898
read:  a: 0x00000020  d: 0x5100 Q
read:  a: 0x00000022  d: 0x5200 R
read:  a: 0x00000024  d: 0x5900 Y
write: a: 0x00000000  d: 0xF0F0
write: a: 0x00000AAA  d: 0x9898
read:  a: 0x00000020  d: 0x5100 Q
read:  a: 0x00000022  d: 0x5200 R
read:  a: 0x00000024  d: 0x5900 Y
write: a: 0x00000000  d: 0xF0F0
write: a: 0x0000AAAA  d: 0xAAAA
write: a: 0x00005554  d: 0x5555
write: a: 0x0000AAAA  d: 0x9898
read:  a: 0x00000020  d: 0x0000
read:  a: 0x00000022  d: 0x0000
read:  a: 0x00000024  d: 0x0000
write: a: 0x00000000  d: 0xF0F0
write: a: 0x00000AAA  d: 0xAAAA
write: a: 0x00000554  d: 0x5555
write: a: 0x00000AAA  d: 0x9898
read:  a: 0x00000020  d: 0x0000
read:  a: 0x00000022  d: 0x0000
read:  a: 0x00000024  d: 0x0000
cfi_qry_mode_on() failed
cfi_qry_mode_on() called #2
write: a: 0x00000000  d: 0xF0F0
write: a: 0x00000154  d: 0x9898
read:  a: 0x00000040  d: 0x0000
read:  a: 0x00000044  d: 0x0000
read:  a: 0x00000048  d: 0x0000
write: a: 0x00000000  d: 0xF0F0
write: a: 0x00000000  d: 0xFFFF
write: a: 0x00000154  d: 0x9898
read:  a: 0x00000040  d: 0x0000
read:  a: 0x00000044  d: 0x0000
read:  a: 0x00000048  d: 0x0000
write: a: 0x00000000  d: 0xF0F0
write: a: 0x00001554  d: 0x9898
read:  a: 0x00000040  d: 0x0000
read:  a: 0x00000044  d: 0x0000
read:  a: 0x00000048  d: 0x0000
write: a: 0x00000000  d: 0xF0F0
write: a: 0x00015554  d: 0xAAAA
write: a: 0x0000AAAA  d: 0x5555
write: a: 0x00015554  d: 0x9898
read:  a: 0x00000040  d: 0x0000
read:  a: 0x00000044  d: 0x0000
read:  a: 0x00000048  d: 0x0000
write: a: 0x00000000  d: 0xF0F0
write: a: 0x00001554  d: 0xAAAA
write: a: 0x00000AAA  d: 0x5555
write: a: 0x00001554  d: 0x9898
read:  a: 0x00000040  d: 0x0000
read:  a: 0x00000044  d: 0x0000
read:  a: 0x00000048  d: 0x0000
cfi_qry_mode_on() failed
cfi_qry_mode_on() called #3
write: a: 0x00000000  d: 0xF0F0
write: a: 0x000002A8  d: 0x9898
read:  a: 0x00000080  d: 0xF358 ¦ X
read:  a: 0x00000088  d: 0x0057  W
read:  a: 0x00000090  d: 0x0000
write: a: 0x00000000  d: 0xF0F0
write: a: 0x00000000  d: 0xFFFF
write: a: 0x000002A8  d: 0x9898
read:  a: 0x00000080  d: 0xF358 ¦ X
read:  a: 0x00000088  d: 0x0057  W
read:  a: 0x00000090  d: 0x0000
write: a: 0x00000000  d: 0xF0F0
write: a: 0x00002AA8  d: 0x9898
read:  a: 0x00000080  d: 0xF358 ¦ X
read:  a: 0x00000088  d: 0x0057  W
read:  a: 0x00000090  d: 0x0000
write: a: 0x00000000  d: 0xF0F0
write: a: 0x0002AAA8  d: 0xAAAA
write: a: 0x00015554  d: 0x5555
write: a: 0x0002AAA8  d: 0x9898
read:  a: 0x00000080  d: 0xF358 ¦ X
read:  a: 0x00000088  d: 0x0057  W
read:  a: 0x00000090  d: 0x0000
write: a: 0x00000000  d: 0xF0F0
write: a: 0x00002AA8  d: 0xAAAA
write: a: 0x00001554  d: 0x5555
write: a: 0x00002AA8  d: 0x9898
read:  a: 0x00000080  d: 0xF358 ¦ X
read:  a: 0x00000088  d: 0x0057  W
read:  a: 0x00000090  d: 0x0000
cfi_qry_mode_on() failed
cfi_qry_mode_on() called #4
write: a: 0x00000000  d: 0x00F0
write: a: 0x000000AA  d: 0x0098
read:  a: 0x00000020  d: 0x0000
read:  a: 0x00000022  d: 0x0000
read:  a: 0x00000024  d: 0x0000
write: a: 0x00000000  d: 0x00F0
write: a: 0x00000000  d: 0x00FF
write: a: 0x000000AA  d: 0x0098
read:  a: 0x00000020  d: 0x0000
read:  a: 0x00000022  d: 0x0000
read:  a: 0x00000024  d: 0x0000
write: a: 0x00000000  d: 0x00F0
write: a: 0x00000AAA  d: 0x0098
read:  a: 0x00000020  d: 0x0000
read:  a: 0x00000022  d: 0x0000
read:  a: 0x00000024  d: 0x0000
write: a: 0x00000000  d: 0x00F0
write: a: 0x0000AAAA  d: 0x00AA
write: a: 0x00005554  d: 0x0055
write: a: 0x0000AAAA  d: 0x0098
read:  a: 0x00000020  d: 0x0000
read:  a: 0x00000022  d: 0x0000
read:  a: 0x00000024  d: 0x0000
write: a: 0x00000000  d: 0x00F0
write: a: 0x00000AAA  d: 0x00AA
write: a: 0x00000554  d: 0x0055
write: a: 0x00000AAA  d: 0x0098
read:  a: 0x00000020  d: 0x0000
read:  a: 0x00000022  d: 0x0000
read:  a: 0x00000024  d: 0x0000
cfi_qry_mode_on() failed
cfi_qry_mode_on() called #5
write: a: 0x00000000  d: 0x00F0
write: a: 0x00000154  d: 0x0098
read:  a: 0x00000040  d: 0x0000
read:  a: 0x00000044  d: 0x0000
read:  a: 0x00000048  d: 0x0000
write: a: 0x00000000  d: 0x00F0
write: a: 0x00000000  d: 0x00FF
write: a: 0x00000154  d: 0x0098
read:  a: 0x00000040  d: 0x0000
read:  a: 0x00000044  d: 0x0000
read:  a: 0x00000048  d: 0x0000
write: a: 0x00000000  d: 0x00F0
write: a: 0x00001554  d: 0x0098
read:  a: 0x00000040  d: 0x0000
read:  a: 0x00000044  d: 0x0000
read:  a: 0x00000048  d: 0x0000
write: a: 0x00000000  d: 0x00F0
write: a: 0x00015554  d: 0x00AA
write: a: 0x0000AAAA  d: 0x0055
write: a: 0x00015554  d: 0x0098
read:  a: 0x00000040  d: 0x0000
read:  a: 0x00000044  d: 0x0000
read:  a: 0x00000048  d: 0x0000
write: a: 0x00000000  d: 0x00F0
write: a: 0x00001554  d: 0x00AA
write: a: 0x00000AAA  d: 0x0055
write: a: 0x00001554  d: 0x0098
read:  a: 0x00000040  d: 0x0000
read:  a: 0x00000044  d: 0x0000
read:  a: 0x00000048  d: 0x0000
cfi_qry_mode_on() failed
rpc-if-hyperflash rpc-if-hyperflash: probing of hyperbus device failed
rpc-if-hyperflash rpc-if-hyperflash: failed to register device



[3] Probe success WITH be16_to_cpu/cpu_to_be16s:

cfi_qry_mode_on() called #1
write: a: 0x00000000  d: 0xF0F0
write: a: 0x000000AA  d: 0x9898
read:  a: 0x00000020  d: 0x0051  Q
read:  a: 0x00000022  d: 0x0052  R
read:  a: 0x00000024  d: 0x0059  Y
write: a: 0x00000000  d: 0xF0F0
write: a: 0x00000000  d: 0xFFFF
write: a: 0x000000AA  d: 0x9898
read:  a: 0x00000020  d: 0x0051  Q
read:  a: 0x00000022  d: 0x0052  R
read:  a: 0x00000024  d: 0x0059  Y
write: a: 0x00000000  d: 0xF0F0
write: a: 0x00000AAA  d: 0x9898
read:  a: 0x00000020  d: 0x0051  Q
read:  a: 0x00000022  d: 0x0052  R
read:  a: 0x00000024  d: 0x0059  Y
write: a: 0x00000000  d: 0xF0F0
write: a: 0x0000AAAA  d: 0xAAAA
write: a: 0x00005554  d: 0x5555
write: a: 0x0000AAAA  d: 0x9898
read:  a: 0x00000020  d: 0x0000
read:  a: 0x00000022  d: 0x0000
read:  a: 0x00000024  d: 0x0000
write: a: 0x00000000  d: 0xF0F0
write: a: 0x00000AAA  d: 0xAAAA
write: a: 0x00000554  d: 0x5555
write: a: 0x00000AAA  d: 0x9898
read:  a: 0x00000020  d: 0x0000
read:  a: 0x00000022  d: 0x0000
read:  a: 0x00000024  d: 0x0000
cfi_qry_mode_on() failed
cfi_qry_mode_on() called #2
write: a: 0x00000000  d: 0xF0F0
write: a: 0x00000154  d: 0x9898
read:  a: 0x00000040  d: 0x0000
read:  a: 0x00000044  d: 0x0000
read:  a: 0x00000048  d: 0x0000
write: a: 0x00000000  d: 0xF0F0
write: a: 0x00000000  d: 0xFFFF
write: a: 0x00000154  d: 0x9898
read:  a: 0x00000040  d: 0x0000
read:  a: 0x00000044  d: 0x0000
read:  a: 0x00000048  d: 0x0000
write: a: 0x00000000  d: 0xF0F0
write: a: 0x00001554  d: 0x9898
read:  a: 0x00000040  d: 0x0000
read:  a: 0x00000044  d: 0x0000
read:  a: 0x00000048  d: 0x0000
write: a: 0x00000000  d: 0xF0F0
write: a: 0x00015554  d: 0xAAAA
write: a: 0x0000AAAA  d: 0x5555
write: a: 0x00015554  d: 0x9898
read:  a: 0x00000040  d: 0x0000
read:  a: 0x00000044  d: 0x0000
read:  a: 0x00000048  d: 0x0000
write: a: 0x00000000  d: 0xF0F0
write: a: 0x00001554  d: 0xAAAA
write: a: 0x00000AAA  d: 0x5555
write: a: 0x00001554  d: 0x9898
read:  a: 0x00000040  d: 0x0000
read:  a: 0x00000044  d: 0x0000
read:  a: 0x00000048  d: 0x0000
cfi_qry_mode_on() failed
cfi_qry_mode_on() called #3
write: a: 0x00000000  d: 0xF0F0
write: a: 0x000002A8  d: 0x9898
read:  a: 0x00000080  d: 0x58F3 X ¦
read:  a: 0x00000088  d: 0x5700 W
read:  a: 0x00000090  d: 0x0000
write: a: 0x00000000  d: 0xF0F0
write: a: 0x00000000  d: 0xFFFF
write: a: 0x000002A8  d: 0x9898
read:  a: 0x00000080  d: 0x58F3 X ¦
read:  a: 0x00000088  d: 0x5700 W
read:  a: 0x00000090  d: 0x0000
write: a: 0x00000000  d: 0xF0F0
write: a: 0x00002AA8  d: 0x9898
read:  a: 0x00000080  d: 0x58F3 X ¦
read:  a: 0x00000088  d: 0x5700 W
read:  a: 0x00000090  d: 0x0000
write: a: 0x00000000  d: 0xF0F0
write: a: 0x0002AAA8  d: 0xAAAA
write: a: 0x00015554  d: 0x5555
write: a: 0x0002AAA8  d: 0x9898
read:  a: 0x00000080  d: 0x58F3 X ¦
read:  a: 0x00000088  d: 0x5700 W
read:  a: 0x00000090  d: 0x0000
write: a: 0x00000000  d: 0xF0F0
write: a: 0x00002AA8  d: 0xAAAA
write: a: 0x00001554  d: 0x5555
write: a: 0x00002AA8  d: 0x9898
read:  a: 0x00000080  d: 0x58F3 X ¦
read:  a: 0x00000088  d: 0x5700 W
read:  a: 0x00000090  d: 0x0000
cfi_qry_mode_on() failed
cfi_qry_mode_on() called #4
write: a: 0x00000000  d: 0xF000
write: a: 0x000000AA  d: 0x9800
read:  a: 0x00000020  d: 0x0051  Q
read:  a: 0x00000022  d: 0x0052  R
read:  a: 0x00000024  d: 0x0059  Y
read:  a: 0x00000058  d: 0x0001
read:  a: 0x00000020  d: 0x0051  Q
read:  a: 0x00000022  d: 0x0052  R
read:  a: 0x00000024  d: 0x0059  Y
read:  a: 0x00000026  d: 0x0002
read:  a: 0x00000028  d: 0x0000
read:  a: 0x0000002A  d: 0x0040  @
read:  a: 0x0000002C  d: 0x0000
read:  a: 0x0000002E  d: 0x0000
read:  a: 0x00000030  d: 0x0000
read:  a: 0x00000032  d: 0x0000
read:  a: 0x00000034  d: 0x0000
read:  a: 0x00000036  d: 0x0017
read:  a: 0x00000038  d: 0x0019
read:  a: 0x0000003A  d: 0x0000
read:  a: 0x0000003C  d: 0x0000
read:  a: 0x0000003E  d: 0x0009
read:  a: 0x00000040  d: 0x0009
read:  a: 0x00000042  d: 0x000A

read:  a: 0x00000044  d: 0x0012
read:  a: 0x00000046  d: 0x0002
read:  a: 0x00000048  d: 0x0002
read:  a: 0x0000004A  d: 0x0002
read:  a: 0x0000004C  d: 0x0002
read:  a: 0x0000004E  d: 0x001A
read:  a: 0x00000050  d: 0x0000
read:  a: 0x00000052  d: 0x0000
read:  a: 0x00000054  d: 0x0009
read:  a: 0x00000056  d: 0x0000
read:  a: 0x00000058  d: 0x0001
read:  a: 0x0000005A  d: 0x00FF  ¦
read:  a: 0x0000005C  d: 0x0000
read:  a: 0x0000005E  d: 0x0000
read:  a: 0x00000060  d: 0x0004
write: a: 0x00000000  d: 0xF000
write: a: 0x00000AAA  d: 0xAA00
write: a: 0x00000554  d: 0x5500
write: a: 0x00000AAA  d: 0x9000
read:  a: 0x00000000  d: 0x0001
read:  a: 0x00000002  d: 0x007E  ~
read:  a: 0x0000001C  d: 0x0070  p
read:  a: 0x0000001E  d: 0x0000
write: a: 0x00000000  d: 0xF000
write: a: 0x00000000  d: 0xFF00
cfi_qry_mode_on() called #5
write: a: 0x00000000  d: 0xF000
write: a: 0x000000AA  d: 0x9800
read:  a: 0x00000020  d: 0x0051  Q
read:  a: 0x00000022  d: 0x0052  R
read:  a: 0x00000024  d: 0x0059  Y
read:  a: 0x00000080  d: 0x0050  P
read:  a: 0x00000082  d: 0x0052  R
read:  a: 0x00000084  d: 0x0049  I
read:  a: 0x00000086  d: 0x0031  1
read:  a: 0x00000088  d: 0x0035  5
read:  a: 0x0000008A  d: 0x001C
read:  a: 0x0000008C  d: 0x0002
read:  a: 0x0000008E  d: 0x0001
read:  a: 0x00000090  d: 0x0000
read:  a: 0x00000092  d: 0x0008
read:  a: 0x00000094  d: 0x0000
read:  a: 0x00000096  d: 0x0001
read:  a: 0x00000098  d: 0x0000
read:  a: 0x0000009A  d: 0x0000
read:  a: 0x0000009C  d: 0x0000
read:  a: 0x0000009E  d: 0x0000
write: a: 0x00000000  d: 0xF000
write: a: 0x00000000  d: 0xFF00
=> success




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



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




[Index of Archives]     [LARTC]     [Bugtraq]     [Yosemite Forum]     [Photo]

  Powered by Linux