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

[Vignesh Raghavendra <vigneshr@xxxxxx>]

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@xxxxxxxxxxxxxxxxxx>
> 
> ---
>  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");
> 

-- 
Regards
Vignesh

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