[RFT,0/2/2] mtd: hyperbus: add Renesas RPC-IF driver
diff mbox series

Message ID eba43289-3cb2-406b-cc5f-1209778621bf@cogentembedded.com
State New
Delegated to: Vignesh R
Headers show
Series
  • Add RPC-IF HyperFlash driver
Related show

Commit Message

Sergei Shtylyov Jan. 29, 2020, 8:39 p.m. UTC
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@cogentembedded.com>

---
 drivers/mtd/hyperbus/Kconfig  |    6 +
 drivers/mtd/hyperbus/Makefile |    1 
 drivers/mtd/hyperbus/rpc-if.c |  162 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 169 insertions(+)

Comments

Vignesh Raghavendra Feb. 3, 2020, 4:59 a.m. UTC | #1
Hi Sergei,

On 30/01/20 2:09 am, Sergei Shtylyov 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@cogentembedded.com>
> 
> ---

Is the backend merged? Or are the patches still in RFC stage?

Regards
Vignesh

>  drivers/mtd/hyperbus/Kconfig  |    6 +
>  drivers/mtd/hyperbus/Makefile |    1 
>  drivers/mtd/hyperbus/rpc-if.c |  162 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 169 insertions(+)
> 
> Index: linux/drivers/mtd/hyperbus/Kconfig
> ===================================================================
> --- linux.orig/drivers/mtd/hyperbus/Kconfig
> +++ linux/drivers/mtd/hyperbus/Kconfig
> @@ -22,4 +22,10 @@ config HBMC_AM654
>  	 This is the driver for HyperBus controller on TI's AM65x and
>  	 other SoCs
>  
> +config RPCIF_HYPERBUS
> +	tristate "Renesas RPC-IF HyperBus driver"
> +	depends on RENESAS_RPCIF
> +	help
> +	  This option includes Renesas RPC-IF HyperFlash support.
> +
>  endif # MTD_HYPERBUS
> Index: linux/drivers/mtd/hyperbus/Makefile
> ===================================================================
> --- linux.orig/drivers/mtd/hyperbus/Makefile
> +++ linux/drivers/mtd/hyperbus/Makefile
> @@ -2,3 +2,4 @@
>  
>  obj-$(CONFIG_MTD_HYPERBUS)	+= hyperbus-core.o
>  obj-$(CONFIG_HBMC_AM654)	+= hbmc-am654.o
> +obj-$(CONFIG_RPCIF_HYPERBUS)	+= rpc-if.o
> Index: linux/drivers/mtd/hyperbus/rpc-if.c
> ===================================================================
> --- /dev/null
> +++ linux/drivers/mtd/hyperbus/rpc-if.c
> @@ -0,0 +1,162 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Linux driver for RPC-IF HyperFlash
> + *
> + * Copyright (C) 2019 Cogent Embedded, Inc.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mtd/hyperbus.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/mux/consumer.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/types.h>
> +
> +#include <memory/renesas-rpc-if.h>
> +
> +struct	rpcif_hyperbus {
> +	struct rpcif rpc;
> +	struct hyperbus_ctlr ctlr;
> +	struct hyperbus_device hbdev;
> +};
> +
> +static const struct rpcif_op rpcif_op_tmpl = {
> +	.cmd = {
> +		.buswidth = 8,
> +		.ddr = true,
> +	},
> +	.ocmd = {
> +		.buswidth = 8,
> +		.ddr = true,
> +	},
> +	.addr = {
> +		.nbytes = 1,
> +		.buswidth = 8,
> +		.ddr = true,
> +	},
> +	.data = {
> +		.buswidth = 8,
> +		.ddr = true,
> +	},
> +};
> +
> +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);
> +	rpcif_prepare(&hyperbus->rpc, &op, NULL, NULL); // ?
> +	rpcif_io_xfer(&hyperbus->rpc);
> +}
> +
> +static void rpcif_hb_copy_from(struct hyperbus_device *hbdev, void *to,
> +			       unsigned long from, ssize_t len)
> +{
> +	struct rpcif_hyperbus *hyperbus =
> +		container_of(hbdev, struct rpcif_hyperbus, hbdev);
> +	struct rpcif_op op = rpcif_op_tmpl;
> +
> +	op.cmd.opcode = 0xA0;
> +	op.addr.val = from;
> +	op.dummy.buswidth = 1;
> +	op.dummy.ncycles = 15;
> +	op.data.dir = RPCIF_DATA_IN;
> +	op.data.nbytes = len;
> +	op.data.buf.in = to;
> +	rpcif_prepare(&hyperbus->rpc, &op, NULL, NULL); // ?
> +	rpcif_dirmap_read(&hyperbus->rpc, from, len, to);
> +}
> +
> +static const struct hyperbus_ops rpcif_hb_ops = {
> +	.read16 = rpcif_hb_read16,
> +	.write16 = rpcif_hb_write16,
> +	.copy_from = rpcif_hb_copy_from,
> +};
> +
> +static int rpcif_hb_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct rpcif_hyperbus *hyperbus;
> +	int status;
> +
> +	hyperbus = devm_kzalloc(dev, sizeof(*hyperbus), GFP_KERNEL);
> +	if (!hyperbus)
> +		return -ENOMEM;
> +
> +	rpcif_sw_init(&hyperbus->rpc, pdev->dev.parent);
> +
> +	platform_set_drvdata(pdev, hyperbus);
> +
> +	rpcif_enable_rpm(&hyperbus->rpc);
> +
> +	rpcif_hw_init(&hyperbus->rpc, true);
> +
> +	hyperbus->hbdev.map.size = hyperbus->rpc.size;
> +	hyperbus->hbdev.map.virt = hyperbus->rpc.dirmap;
> +
> +	hyperbus->ctlr.dev = dev;
> +	hyperbus->ctlr.ops = &rpcif_hb_ops;
> +	hyperbus->hbdev.ctlr = &hyperbus->ctlr;
> +	hyperbus->hbdev.np = of_get_next_child(pdev->dev.parent->of_node, NULL);
> +	status = hyperbus_register_device(&hyperbus->hbdev);
> +	if (status) {
> +		dev_err(dev, "failed to register device\n");
> +		rpcif_disable_rpm(&hyperbus->rpc);
> +	}
> +
> +	return status;
> +}
> +
> +static int rpcif_hb_remove(struct platform_device *pdev)
> +{
> +	struct rpcif_hyperbus *hyperbus = platform_get_drvdata(pdev);
> +	int error = hyperbus_unregister_device(&hyperbus->hbdev);
> +	struct rpcif *rpc = dev_get_drvdata(pdev->dev.parent);
> +
> +	rpcif_disable_rpm(rpc);
> +	return error;
> +}
> +
> +static struct platform_driver rpcif_platform_driver = {
> +	.probe	= rpcif_hb_probe,
> +	.remove	= rpcif_hb_remove,
> +	.driver	= {
> +		.name	= "rpc-if-hyperflash",
> +	},
> +};
> +
> +module_platform_driver(rpcif_platform_driver);
> +
> +MODULE_DESCRIPTION("Renesas RPC-IF HyperFlash driver");
> +MODULE_LICENSE("GPL v2");
>
Sergei Shtylyov Feb. 3, 2020, 11:46 a.m. UTC | #2
On 02/03/2020 07:59 AM, Vignesh Raghavendra wrote:
> Hi Sergei,
> 
> On 30/01/20 2:09 am, Sergei Shtylyov 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@cogentembedded.com>
>>
>> ---
> 
> Is the backend merged? Or are the patches still in RFC stage?

   No, it's still the same RFC patch, and I'm not sure who'd merge it --
drivers/memory/ is unmaintained. Perhaps the best way would be to merge it
along with the SPI front end?

MBR, Sergei
Behme Dirk (CM/ESO2) Feb. 7, 2020, 12:59 p.m. UTC | #3
Hi,

On 29.01.2020 21:39, Sergei Shtylyov 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@cogentembedded.com>
> 
> ---
>   drivers/mtd/hyperbus/Kconfig  |    6 +
>   drivers/mtd/hyperbus/Makefile |    1
>   drivers/mtd/hyperbus/rpc-if.c |  162 ++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 169 insertions(+)
> 
> Index: linux/drivers/mtd/hyperbus/Kconfig
> ===================================================================
> --- linux.orig/drivers/mtd/hyperbus/Kconfig
> +++ linux/drivers/mtd/hyperbus/Kconfig
> @@ -22,4 +22,10 @@ config HBMC_AM654
>   	 This is the driver for HyperBus controller on TI's AM65x and
>   	 other SoCs
>   
> +config RPCIF_HYPERBUS
> +	tristate "Renesas RPC-IF HyperBus driver"
> +	depends on RENESAS_RPCIF
> +	help
> +	  This option includes Renesas RPC-IF HyperFlash support.
> +
>   endif # MTD_HYPERBUS
> Index: linux/drivers/mtd/hyperbus/Makefile
> ===================================================================
> --- linux.orig/drivers/mtd/hyperbus/Makefile
> +++ linux/drivers/mtd/hyperbus/Makefile
> @@ -2,3 +2,4 @@
>   
>   obj-$(CONFIG_MTD_HYPERBUS)	+= hyperbus-core.o
>   obj-$(CONFIG_HBMC_AM654)	+= hbmc-am654.o
> +obj-$(CONFIG_RPCIF_HYPERBUS)	+= rpc-if.o
> Index: linux/drivers/mtd/hyperbus/rpc-if.c
> ===================================================================
> --- /dev/null
> +++ linux/drivers/mtd/hyperbus/rpc-if.c
> @@ -0,0 +1,162 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Linux driver for RPC-IF HyperFlash
> + *
> + * Copyright (C) 2019 Cogent Embedded, Inc.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mtd/hyperbus.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/mux/consumer.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/types.h>
> +
> +#include <memory/renesas-rpc-if.h>
> +
> +struct	rpcif_hyperbus {
> +	struct rpcif rpc;
> +	struct hyperbus_ctlr ctlr;
> +	struct hyperbus_device hbdev;
> +};
> +
> +static const struct rpcif_op rpcif_op_tmpl = {
> +	.cmd = {
> +		.buswidth = 8,
> +		.ddr = true,
> +	},
> +	.ocmd = {
> +		.buswidth = 8,
> +		.ddr = true,
> +	},
> +	.addr = {
> +		.nbytes = 1,
> +		.buswidth = 8,
> +		.ddr = true,
> +	},
> +	.data = {
> +		.buswidth = 8,
> +		.ddr = true,
> +	},
> +};
> +
> +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.

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

Many questions ;)

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
Sergei Shtylyov Feb. 7, 2020, 7:09 p.m. UTC | #4
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@cogentembedded.com>
[...]
>> 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?

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

> 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
> 
>
Dirk Behme Feb. 7, 2020, 7:31 p.m. UTC | #5
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@cogentembedded.com>
> [...]
>>> 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/
>
Sergei Shtylyov Feb. 7, 2020, 8:17 p.m. UTC | #6
On 02/07/2020 10:31 PM, Dirk Behme 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@cogentembedded.com>
>> [...]
>>>> 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
                                                                          ^^^ then

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

   Not for me:

root@192.168.2.11:~# dd if=jffs2.img of=/dev/mtd11                              
random: crng init done                                                          
2666+1 records in                                                               
2666+1 records out                                                              
1365320 bytes (1.4 MB) copied, 33.0917 seconds, 41.3 kB/s                       

> solution with the cpu_to_be16s()/be16_to_cpu() in the driver, which wrote nicely to the Hyperflash, but swapped.

   Something's wrong at your end...

> Best regards
> 
> Dirk

MBR, Sergei
Behme Dirk (CM/ESO2) Feb. 10, 2020, 9:18 a.m. UTC | #7
On 07.02.2020 21:17, Sergei Shtylyov wrote:
> On 02/07/2020 10:31 PM, Dirk Behme 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@cogentembedded.com>
>>> [...]
>>>>> 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
>                                                                            ^^^ then
> 
>>> 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
> 
>     Not for me:
> 
> root@192.168.2.11:~# dd if=jffs2.img of=/dev/mtd11
> random: crng init done
> 2666+1 records in
> 2666+1 records out
> 1365320 bytes (1.4 MB) copied, 33.0917 seconds, 41.3 kB/s
> 
>> solution with the cpu_to_be16s()/be16_to_cpu() in the driver, which wrote nicely to the Hyperflash, but swapped.
> 
>     Something's wrong at your end...


Yes, fixed that, working now :)

For reference, I did [1].

Best regards

Dirk

[1]

 From af977d8e53cca6f2e20fb737b4c8655d83e2d7c4 Mon Sep 17 00:00:00 2001
From: Dirk Behme <dirk.behme@de.bosch.com>
Date: Mon, 10 Feb 2020 09:11:40 +0100
Subject: [PATCH] mtd: hyperbus: rpc-if: Use built in endian conversion

Instead of 'manually' doing the endian conversion in the driver,
use the MTD built in one.

FIXME: How to autoselect MTD_CFI_BE_BYTE_SWAP? 'select MTD_CFI_BE_BYTE_SWAP'
        in Kconfig doesn't seem to work?

Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
---
  arch/arm64/configs/rcar3_defconfig | 1 +
  drivers/mtd/hyperbus/Kconfig       | 2 ++
  drivers/mtd/hyperbus/rpc-if.c      | 8 ++++++--
  3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/configs/rcar3_defconfig 
b/arch/arm64/configs/rcar3_defconfig
index d04d5bd83580..cf5636b333b9 100644
--- a/arch/arm64/configs/rcar3_defconfig
+++ b/arch/arm64/configs/rcar3_defconfig
@@ -172,6 +172,7 @@ CONFIG_DEVTMPFS_MOUNT=y
  CONFIG_DMA_CMA=y
  CONFIG_CONNECTOR=m
  CONFIG_MTD=y
+CONFIG_MTD_CFI_BE_BYTE_SWAP=y
  CONFIG_MTD_PHYSMAP_OF=y
  CONFIG_MTD_M25P80=m
  CONFIG_MTD_SPI_NOR=m
diff --git a/drivers/mtd/hyperbus/Kconfig b/drivers/mtd/hyperbus/Kconfig
index d80489d9989c..353be8c8f339 100644
--- a/drivers/mtd/hyperbus/Kconfig
+++ b/drivers/mtd/hyperbus/Kconfig
@@ -25,6 +25,8 @@ config HBMC_AM654
  config RPCIF_HYPERBUS
  	tristate "Renesas RPC-IF HyperBus driver"
  	depends on RENESAS_RPCIF
+	select MTD_CFI_ADV_OPTIONS
+	select MTD_CFI_BE_BYTE_SWAP
  	help
  	  This option includes Renesas RPC-IF HyperFlash support.

diff --git a/drivers/mtd/hyperbus/rpc-if.c b/drivers/mtd/hyperbus/rpc-if.c
index a66a5080b482..6e0c45b5ef95 100644
--- a/drivers/mtd/hyperbus/rpc-if.c
+++ b/drivers/mtd/hyperbus/rpc-if.c
@@ -17,6 +17,11 @@

  #include <memory/renesas-rpc-if.h>

+/* FIXME: How to drop this? */
+#if  !defined(CONFIG_MTD_CFI_BE_BYTE_SWAP)
+#error Enable config "Flash cmd/query data swapping (BIG_ENDIAN_BYTE)"
+#endif
+
  struct	rpcif_hyperbus {
  	struct rpcif rpc;
  	struct hyperbus_ctlr ctlr;
@@ -60,7 +65,7 @@ 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]);
+	return data.x[0];
  }

  static void rpcif_hb_write16(struct hyperbus_device *hbdev, unsigned 
long addr,
@@ -75,7 +80,6 @@ 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);
  	rpcif_prepare(&hyperbus->rpc, &op, NULL, NULL); // ?
  	rpcif_io_xfer(&hyperbus->rpc);
  }
Vignesh Raghavendra Feb. 18, 2020, 4 a.m. UTC | #8
Hi Sergei

On 30/01/20 2:09 am, Sergei Shtylyov 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@cogentembedded.com>
> 
> ---
>  drivers/mtd/hyperbus/Kconfig  |    6 +
>  drivers/mtd/hyperbus/Makefile |    1 
>  drivers/mtd/hyperbus/rpc-if.c |  162 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 169 insertions(+)
> 
> Index: linux/drivers/mtd/hyperbus/Kconfig
> ===================================================================
> --- linux.orig/drivers/mtd/hyperbus/Kconfig
> +++ linux/drivers/mtd/hyperbus/Kconfig
> @@ -22,4 +22,10 @@ config HBMC_AM654
>  	 This is the driver for HyperBus controller on TI's AM65x and
>  	 other SoCs
>  
> +config RPCIF_HYPERBUS
> +	tristate "Renesas RPC-IF HyperBus driver"
> +	depends on RENESAS_RPCIF
> +	help
> +	  This option includes Renesas RPC-IF HyperFlash support.
> +
>  endif # MTD_HYPERBUS
> Index: linux/drivers/mtd/hyperbus/Makefile
> ===================================================================
> --- linux.orig/drivers/mtd/hyperbus/Makefile
> +++ linux/drivers/mtd/hyperbus/Makefile
> @@ -2,3 +2,4 @@
>  
>  obj-$(CONFIG_MTD_HYPERBUS)	+= hyperbus-core.o
>  obj-$(CONFIG_HBMC_AM654)	+= hbmc-am654.o
> +obj-$(CONFIG_RPCIF_HYPERBUS)	+= rpc-if.o
> Index: linux/drivers/mtd/hyperbus/rpc-if.c
> ===================================================================
> --- /dev/null
> +++ linux/drivers/mtd/hyperbus/rpc-if.c
> @@ -0,0 +1,162 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Linux driver for RPC-IF HyperFlash
> + *
> + * Copyright (C) 2019 Cogent Embedded, Inc.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mtd/hyperbus.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/mux/consumer.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/types.h>
> +
> +#include <memory/renesas-rpc-if.h>
> +
> +struct	rpcif_hyperbus {
> +	struct rpcif rpc;
> +	struct hyperbus_ctlr ctlr;
> +	struct hyperbus_device hbdev;
> +};
> +
> +static const struct rpcif_op rpcif_op_tmpl = {
> +	.cmd = {
> +		.buswidth = 8,
> +		.ddr = true,
> +	},
> +	.ocmd = {
> +		.buswidth = 8,
> +		.ddr = true,
> +	},
> +	.addr = {
> +		.nbytes = 1,
> +		.buswidth = 8,
> +		.ddr = true,
> +	},
> +	.data = {
> +		.buswidth = 8,
> +		.ddr = true,
> +	},
> +};
> +

Looking around, there seems to be more than one SPI controllers, apart
from Renesas, which also support SPI NOR and HyperFlash protocol within
a single IP block. E.g.: Cadence xSPI controller [1]. Therefore, we need
a generic framework to support these kind of controllers.

One way would be to extend spi_mem_op to support above template along
with a new field to distinguish SPI NOR vs HyperFlash protocol. HyperBus
core can then register a spi_device and use spi-mem ops to talk to
controller driver.
So, I suggest making Renesas RPC-IF backend a full fledged spi-mem
driver (instead of driver/memory) and use extended spi_mem_op to support
HyperFlash.


[1]
https://ip.cadence.com/uploads/1244/cdn-dsd-mem-fla-host-controller-ip-for-xspi-pdf


Regards
Vignesh

> +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);
> +	rpcif_prepare(&hyperbus->rpc, &op, NULL, NULL); // ?
> +	rpcif_io_xfer(&hyperbus->rpc);
> +}
> +
> +static void rpcif_hb_copy_from(struct hyperbus_device *hbdev, void *to,
> +			       unsigned long from, ssize_t len)
> +{
> +	struct rpcif_hyperbus *hyperbus =
> +		container_of(hbdev, struct rpcif_hyperbus, hbdev);
> +	struct rpcif_op op = rpcif_op_tmpl;
> +
> +	op.cmd.opcode = 0xA0;
> +	op.addr.val = from;
> +	op.dummy.buswidth = 1;
> +	op.dummy.ncycles = 15;
> +	op.data.dir = RPCIF_DATA_IN;
> +	op.data.nbytes = len;
> +	op.data.buf.in = to;
> +	rpcif_prepare(&hyperbus->rpc, &op, NULL, NULL); // ?
> +	rpcif_dirmap_read(&hyperbus->rpc, from, len, to);
> +}
> +
> +static const struct hyperbus_ops rpcif_hb_ops = {
> +	.read16 = rpcif_hb_read16,
> +	.write16 = rpcif_hb_write16,
> +	.copy_from = rpcif_hb_copy_from,
> +};
> +
> +static int rpcif_hb_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct rpcif_hyperbus *hyperbus;
> +	int status;
> +
> +	hyperbus = devm_kzalloc(dev, sizeof(*hyperbus), GFP_KERNEL);
> +	if (!hyperbus)
> +		return -ENOMEM;
> +
> +	rpcif_sw_init(&hyperbus->rpc, pdev->dev.parent);
> +
> +	platform_set_drvdata(pdev, hyperbus);
> +
> +	rpcif_enable_rpm(&hyperbus->rpc);
> +
> +	rpcif_hw_init(&hyperbus->rpc, true);
> +
> +	hyperbus->hbdev.map.size = hyperbus->rpc.size;
> +	hyperbus->hbdev.map.virt = hyperbus->rpc.dirmap;
> +
> +	hyperbus->ctlr.dev = dev;
> +	hyperbus->ctlr.ops = &rpcif_hb_ops;
> +	hyperbus->hbdev.ctlr = &hyperbus->ctlr;
> +	hyperbus->hbdev.np = of_get_next_child(pdev->dev.parent->of_node, NULL);
> +	status = hyperbus_register_device(&hyperbus->hbdev);
> +	if (status) {
> +		dev_err(dev, "failed to register device\n");
> +		rpcif_disable_rpm(&hyperbus->rpc);
> +	}
> +
> +	return status;
> +}
> +
> +static int rpcif_hb_remove(struct platform_device *pdev)
> +{
> +	struct rpcif_hyperbus *hyperbus = platform_get_drvdata(pdev);
> +	int error = hyperbus_unregister_device(&hyperbus->hbdev);
> +	struct rpcif *rpc = dev_get_drvdata(pdev->dev.parent);
> +
> +	rpcif_disable_rpm(rpc);
> +	return error;
> +}
> +
> +static struct platform_driver rpcif_platform_driver = {
> +	.probe	= rpcif_hb_probe,
> +	.remove	= rpcif_hb_remove,
> +	.driver	= {
> +		.name	= "rpc-if-hyperflash",
> +	},
> +};
> +
> +module_platform_driver(rpcif_platform_driver);
> +
> +MODULE_DESCRIPTION("Renesas RPC-IF HyperFlash driver");
> +MODULE_LICENSE("GPL v2");
>
Behme Dirk (CM/ESO2) Feb. 18, 2020, 7:12 a.m. UTC | #9
Hi Vignesh,

On 18.02.2020 05:00, Vignesh Raghavendra wrote:
> Hi Sergei
> 
> On 30/01/20 2:09 am, Sergei Shtylyov 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@cogentembedded.com>
>>
>> ---
>>   drivers/mtd/hyperbus/Kconfig  |    6 +
>>   drivers/mtd/hyperbus/Makefile |    1
>>   drivers/mtd/hyperbus/rpc-if.c |  162 ++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 169 insertions(+)
>>
>> Index: linux/drivers/mtd/hyperbus/Kconfig
>> ===================================================================
>> --- linux.orig/drivers/mtd/hyperbus/Kconfig
>> +++ linux/drivers/mtd/hyperbus/Kconfig
>> @@ -22,4 +22,10 @@ config HBMC_AM654
>>   	 This is the driver for HyperBus controller on TI's AM65x and
>>   	 other SoCs
>>   
>> +config RPCIF_HYPERBUS
>> +	tristate "Renesas RPC-IF HyperBus driver"
>> +	depends on RENESAS_RPCIF
>> +	help
>> +	  This option includes Renesas RPC-IF HyperFlash support.
>> +
>>   endif # MTD_HYPERBUS
>> Index: linux/drivers/mtd/hyperbus/Makefile
>> ===================================================================
>> --- linux.orig/drivers/mtd/hyperbus/Makefile
>> +++ linux/drivers/mtd/hyperbus/Makefile
>> @@ -2,3 +2,4 @@
>>   
>>   obj-$(CONFIG_MTD_HYPERBUS)	+= hyperbus-core.o
>>   obj-$(CONFIG_HBMC_AM654)	+= hbmc-am654.o
>> +obj-$(CONFIG_RPCIF_HYPERBUS)	+= rpc-if.o
>> Index: linux/drivers/mtd/hyperbus/rpc-if.c
>> ===================================================================
>> --- /dev/null
>> +++ linux/drivers/mtd/hyperbus/rpc-if.c
>> @@ -0,0 +1,162 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Linux driver for RPC-IF HyperFlash
>> + *
>> + * Copyright (C) 2019 Cogent Embedded, Inc.
>> + */
>> +
>> +#include <linux/err.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/mtd/hyperbus.h>
>> +#include <linux/mtd/mtd.h>
>> +#include <linux/mux/consumer.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/types.h>
>> +
>> +#include <memory/renesas-rpc-if.h>
>> +
>> +struct	rpcif_hyperbus {
>> +	struct rpcif rpc;
>> +	struct hyperbus_ctlr ctlr;
>> +	struct hyperbus_device hbdev;
>> +};
>> +
>> +static const struct rpcif_op rpcif_op_tmpl = {
>> +	.cmd = {
>> +		.buswidth = 8,
>> +		.ddr = true,
>> +	},
>> +	.ocmd = {
>> +		.buswidth = 8,
>> +		.ddr = true,
>> +	},
>> +	.addr = {
>> +		.nbytes = 1,
>> +		.buswidth = 8,
>> +		.ddr = true,
>> +	},
>> +	.data = {
>> +		.buswidth = 8,
>> +		.ddr = true,
>> +	},
>> +};
>> +
> 
> Looking around, there seems to be more than one SPI controllers, apart
> from Renesas, which also support SPI NOR and HyperFlash protocol within
> a single IP block. E.g.: Cadence xSPI controller [1]. Therefore, we need
> a generic framework to support these kind of controllers.
> 
> One way would be to extend spi_mem_op to support above template along
> with a new field to distinguish SPI NOR vs HyperFlash protocol. HyperBus
> core can then register a spi_device and use spi-mem ops to talk to
> controller driver.
> So, I suggest making Renesas RPC-IF backend a full fledged spi-mem
> driver (instead of driver/memory) and use extended spi_mem_op to support
> HyperFlash.


 From Renesas Hyperflash user point of view, I wonder if a two step 
approach would be possible and acceptable, here?

Being a user of the Renesas Hyperflash, I want a driver for that. And, 
of course, I want it "now" ;)

So I wonder if it would be a valid option to have a functioning Renesas 
Hypeflash driver, first. And in a second step abstract that in a more 
generic way to support additional controllers. While in parallel having 
a functional driver for the Renesas people, already.

Is the support for [1] a more or less theoretical one, at the moment? Or 
are there users of that which need support "now", too?

Best regards

Dirk


> [1]
> https://ip.cadence.com/uploads/1244/cdn-dsd-mem-fla-host-controller-ip-for-xspi-pdf
> 
> 
> Regards
> Vignesh
> 
>> +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);
>> +	rpcif_prepare(&hyperbus->rpc, &op, NULL, NULL); // ?
>> +	rpcif_io_xfer(&hyperbus->rpc);
>> +}
>> +
>> +static void rpcif_hb_copy_from(struct hyperbus_device *hbdev, void *to,
>> +			       unsigned long from, ssize_t len)
>> +{
>> +	struct rpcif_hyperbus *hyperbus =
>> +		container_of(hbdev, struct rpcif_hyperbus, hbdev);
>> +	struct rpcif_op op = rpcif_op_tmpl;
>> +
>> +	op.cmd.opcode = 0xA0;
>> +	op.addr.val = from;
>> +	op.dummy.buswidth = 1;
>> +	op.dummy.ncycles = 15;
>> +	op.data.dir = RPCIF_DATA_IN;
>> +	op.data.nbytes = len;
>> +	op.data.buf.in = to;
>> +	rpcif_prepare(&hyperbus->rpc, &op, NULL, NULL); // ?
>> +	rpcif_dirmap_read(&hyperbus->rpc, from, len, to);
>> +}
>> +
>> +static const struct hyperbus_ops rpcif_hb_ops = {
>> +	.read16 = rpcif_hb_read16,
>> +	.write16 = rpcif_hb_write16,
>> +	.copy_from = rpcif_hb_copy_from,
>> +};
>> +
>> +static int rpcif_hb_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct rpcif_hyperbus *hyperbus;
>> +	int status;
>> +
>> +	hyperbus = devm_kzalloc(dev, sizeof(*hyperbus), GFP_KERNEL);
>> +	if (!hyperbus)
>> +		return -ENOMEM;
>> +
>> +	rpcif_sw_init(&hyperbus->rpc, pdev->dev.parent);
>> +
>> +	platform_set_drvdata(pdev, hyperbus);
>> +
>> +	rpcif_enable_rpm(&hyperbus->rpc);
>> +
>> +	rpcif_hw_init(&hyperbus->rpc, true);
>> +
>> +	hyperbus->hbdev.map.size = hyperbus->rpc.size;
>> +	hyperbus->hbdev.map.virt = hyperbus->rpc.dirmap;
>> +
>> +	hyperbus->ctlr.dev = dev;
>> +	hyperbus->ctlr.ops = &rpcif_hb_ops;
>> +	hyperbus->hbdev.ctlr = &hyperbus->ctlr;
>> +	hyperbus->hbdev.np = of_get_next_child(pdev->dev.parent->of_node, NULL);
>> +	status = hyperbus_register_device(&hyperbus->hbdev);
>> +	if (status) {
>> +		dev_err(dev, "failed to register device\n");
>> +		rpcif_disable_rpm(&hyperbus->rpc);
>> +	}
>> +
>> +	return status;
>> +}
>> +
>> +static int rpcif_hb_remove(struct platform_device *pdev)
>> +{
>> +	struct rpcif_hyperbus *hyperbus = platform_get_drvdata(pdev);
>> +	int error = hyperbus_unregister_device(&hyperbus->hbdev);
>> +	struct rpcif *rpc = dev_get_drvdata(pdev->dev.parent);
>> +
>> +	rpcif_disable_rpm(rpc);
>> +	return error;
>> +}
>> +
>> +static struct platform_driver rpcif_platform_driver = {
>> +	.probe	= rpcif_hb_probe,
>> +	.remove	= rpcif_hb_remove,
>> +	.driver	= {
>> +		.name	= "rpc-if-hyperflash",
>> +	},
>> +};
>> +
>> +module_platform_driver(rpcif_platform_driver);
>> +
>> +MODULE_DESCRIPTION("Renesas RPC-IF HyperFlash driver");
>> +MODULE_LICENSE("GPL v2");
Vignesh Raghavendra Feb. 18, 2020, 11:11 a.m. UTC | #10
Hi,

On 18/02/20 12:42 pm, Behme Dirk (CM/ESO2) wrote:
> Hi Vignesh,
> 
> On 18.02.2020 05:00, Vignesh Raghavendra wrote:
>> Hi Sergei
>>
[...]
>>
>> Looking around, there seems to be more than one SPI controllers, apart
>> from Renesas, which also support SPI NOR and HyperFlash protocol within
>> a single IP block. E.g.: Cadence xSPI controller [1]. Therefore, we need
>> a generic framework to support these kind of controllers.
>>
>> One way would be to extend spi_mem_op to support above template along
>> with a new field to distinguish SPI NOR vs HyperFlash protocol. HyperBus
>> core can then register a spi_device and use spi-mem ops to talk to
>> controller driver.
>> So, I suggest making Renesas RPC-IF backend a full fledged spi-mem
>> driver (instead of driver/memory) and use extended spi_mem_op to support
>> HyperFlash.
> 
> 
> From Renesas Hyperflash user point of view, I wonder if a two step
> approach would be possible and acceptable, here?
> 
> Being a user of the Renesas Hyperflash, I want a driver for that. And,
> of course, I want it "now" ;)
> 
> So I wonder if it would be a valid option to have a functioning Renesas
> Hypeflash driver, first. And in a second step abstract that in a more
> generic way to support additional controllers. While in parallel having
> a functional driver for the Renesas people, already.
> 

AFAICS, the backend driver is not merged and is still in RFC phase.
Therefore I don't see any benefit of two step approach here. Besides
you'll have to throw away this new driver (hyperbus/rpc-if.c) entirely
later on.

How difficult is it to rewrite backend to be spi-mem driver? There is
already has a spi_mem_ops frontend implementation, so I don't see much
of an issue.
Extending hyperbus core to use spi-mem should also straight forward
Would involve moving this patch into core file.

> Is the support for [1] a more or less theoretical one, at the moment? Or
> are there users of that which need support "now", too?
> 

Its not theoretical, I do see patches for xSPI controller here:
https://patchwork.kernel.org/cover/11354193/

So, its best to sort this out now so as to avoid possible backward
compatibility issues (especially with DT bindings)

Regards
Vignesh

Patch
diff mbox series

Index: linux/drivers/mtd/hyperbus/Kconfig
===================================================================
--- linux.orig/drivers/mtd/hyperbus/Kconfig
+++ linux/drivers/mtd/hyperbus/Kconfig
@@ -22,4 +22,10 @@  config HBMC_AM654
 	 This is the driver for HyperBus controller on TI's AM65x and
 	 other SoCs
 
+config RPCIF_HYPERBUS
+	tristate "Renesas RPC-IF HyperBus driver"
+	depends on RENESAS_RPCIF
+	help
+	  This option includes Renesas RPC-IF HyperFlash support.
+
 endif # MTD_HYPERBUS
Index: linux/drivers/mtd/hyperbus/Makefile
===================================================================
--- linux.orig/drivers/mtd/hyperbus/Makefile
+++ linux/drivers/mtd/hyperbus/Makefile
@@ -2,3 +2,4 @@ 
 
 obj-$(CONFIG_MTD_HYPERBUS)	+= hyperbus-core.o
 obj-$(CONFIG_HBMC_AM654)	+= hbmc-am654.o
+obj-$(CONFIG_RPCIF_HYPERBUS)	+= rpc-if.o
Index: linux/drivers/mtd/hyperbus/rpc-if.c
===================================================================
--- /dev/null
+++ linux/drivers/mtd/hyperbus/rpc-if.c
@@ -0,0 +1,162 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Linux driver for RPC-IF HyperFlash
+ *
+ * Copyright (C) 2019 Cogent Embedded, Inc.
+ */
+
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mtd/hyperbus.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mux/consumer.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+
+#include <memory/renesas-rpc-if.h>
+
+struct	rpcif_hyperbus {
+	struct rpcif rpc;
+	struct hyperbus_ctlr ctlr;
+	struct hyperbus_device hbdev;
+};
+
+static const struct rpcif_op rpcif_op_tmpl = {
+	.cmd = {
+		.buswidth = 8,
+		.ddr = true,
+	},
+	.ocmd = {
+		.buswidth = 8,
+		.ddr = true,
+	},
+	.addr = {
+		.nbytes = 1,
+		.buswidth = 8,
+		.ddr = true,
+	},
+	.data = {
+		.buswidth = 8,
+		.ddr = true,
+	},
+};
+
+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);
+	rpcif_prepare(&hyperbus->rpc, &op, NULL, NULL); // ?
+	rpcif_io_xfer(&hyperbus->rpc);
+}
+
+static void rpcif_hb_copy_from(struct hyperbus_device *hbdev, void *to,
+			       unsigned long from, ssize_t len)
+{
+	struct rpcif_hyperbus *hyperbus =
+		container_of(hbdev, struct rpcif_hyperbus, hbdev);
+	struct rpcif_op op = rpcif_op_tmpl;
+
+	op.cmd.opcode = 0xA0;
+	op.addr.val = from;
+	op.dummy.buswidth = 1;
+	op.dummy.ncycles = 15;
+	op.data.dir = RPCIF_DATA_IN;
+	op.data.nbytes = len;
+	op.data.buf.in = to;
+	rpcif_prepare(&hyperbus->rpc, &op, NULL, NULL); // ?
+	rpcif_dirmap_read(&hyperbus->rpc, from, len, to);
+}
+
+static const struct hyperbus_ops rpcif_hb_ops = {
+	.read16 = rpcif_hb_read16,
+	.write16 = rpcif_hb_write16,
+	.copy_from = rpcif_hb_copy_from,
+};
+
+static int rpcif_hb_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rpcif_hyperbus *hyperbus;
+	int status;
+
+	hyperbus = devm_kzalloc(dev, sizeof(*hyperbus), GFP_KERNEL);
+	if (!hyperbus)
+		return -ENOMEM;
+
+	rpcif_sw_init(&hyperbus->rpc, pdev->dev.parent);
+
+	platform_set_drvdata(pdev, hyperbus);
+
+	rpcif_enable_rpm(&hyperbus->rpc);
+
+	rpcif_hw_init(&hyperbus->rpc, true);
+
+	hyperbus->hbdev.map.size = hyperbus->rpc.size;
+	hyperbus->hbdev.map.virt = hyperbus->rpc.dirmap;
+
+	hyperbus->ctlr.dev = dev;
+	hyperbus->ctlr.ops = &rpcif_hb_ops;
+	hyperbus->hbdev.ctlr = &hyperbus->ctlr;
+	hyperbus->hbdev.np = of_get_next_child(pdev->dev.parent->of_node, NULL);
+	status = hyperbus_register_device(&hyperbus->hbdev);
+	if (status) {
+		dev_err(dev, "failed to register device\n");
+		rpcif_disable_rpm(&hyperbus->rpc);
+	}
+
+	return status;
+}
+
+static int rpcif_hb_remove(struct platform_device *pdev)
+{
+	struct rpcif_hyperbus *hyperbus = platform_get_drvdata(pdev);
+	int error = hyperbus_unregister_device(&hyperbus->hbdev);
+	struct rpcif *rpc = dev_get_drvdata(pdev->dev.parent);
+
+	rpcif_disable_rpm(rpc);
+	return error;
+}
+
+static struct platform_driver rpcif_platform_driver = {
+	.probe	= rpcif_hb_probe,
+	.remove	= rpcif_hb_remove,
+	.driver	= {
+		.name	= "rpc-if-hyperflash",
+	},
+};
+
+module_platform_driver(rpcif_platform_driver);
+
+MODULE_DESCRIPTION("Renesas RPC-IF HyperFlash driver");
+MODULE_LICENSE("GPL v2");