* [U-Boot] [PATCH 0/3] drivers/sandbox: Introduce a simplified remoteproc framework
@ 2015-08-24 17:28 Nishanth Menon
2015-08-24 17:28 ` [U-Boot] [PATCH 1/3] drivers: " Nishanth Menon
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Nishanth Menon @ 2015-08-24 17:28 UTC (permalink / raw)
To: u-boot
Hi,
Many System on Chip(SoC) solutions are complex with multiple
processors on the same die dedicated to either general purpose of
specialized functions. Many examples do exist in today's SoCs from
various vendors. Typical examples are micro controllers such as an ARM
M3/M0 doing a offload of specific function such as event integration
or power management or controlling camera etc.
Traditionally, the responsibility of loading up such a processor with
a firmware and communication has been with a High Level Operating
System(HLOS) such as Linux. However, there exists classes of products
where Linux would need to expect services from such a processor or
the delay of Linux and operating system being able to load up such a
firmware is unacceptable.
The intent here is to introduce a simplified remoteproc framework
which can then be used to provide basic services to these remote
processors.
Nishanth Menon (3):
drivers: Introduce a simplified remoteproc framework
remoteproc: Introduce a sandbox dummy driver
sandbox: Introduce dummy remoteproc nodes
arch/sandbox/dts/test.dts | 13 +
common/Kconfig | 5 +
common/Makefile | 1 +
common/cmd_remoteproc.c | 224 ++++++++++++
configs/sandbox_defconfig | 2 +
doc/device-tree-bindings/remoteproc/remoteproc.txt | 14 +
doc/driver-model/remoteproc-framework.txt | 163 +++++++++
drivers/Kconfig | 2 +
drivers/Makefile | 1 +
drivers/remoteproc/Kconfig | 24 ++
drivers/remoteproc/Makefile | 10 +
drivers/remoteproc/rproc-uclass.c | 406 +++++++++++++++++++++
drivers/remoteproc/sandbox_testproc.c | 243 ++++++++++++
include/dm/uclass-id.h | 1 +
include/remoteproc.h | 81 ++++
15 files changed, 1190 insertions(+)
create mode 100644 common/cmd_remoteproc.c
create mode 100644 doc/device-tree-bindings/remoteproc/remoteproc.txt
create mode 100644 doc/driver-model/remoteproc-framework.txt
create mode 100644 drivers/remoteproc/Kconfig
create mode 100644 drivers/remoteproc/Makefile
create mode 100644 drivers/remoteproc/rproc-uclass.c
create mode 100644 drivers/remoteproc/sandbox_testproc.c
create mode 100644 include/remoteproc.h
Simple test with test.dtb:
u-boot$ ./u-boot -d ./arch/sandbox/dts/test.dtb
U-Boot 2015.10-rc2-00067-g05997426fef2 (Aug 24 2015 - 17:23:58 +0000)
DRAM: 128 MiB
Using default environment
In: serial
Out: lcd
Err: lcd
Net: Net Initialization Skipped
eth0: eth at 10002000, eth5: eth at 10003000, eth1: eth at 10004000
=> help rproc
rproc - Control operation of remote processors in an SoC
Usage:
rproc [init|list|load|start|stop|reset|ping]
Where:
[addr] is a memory address
<id> is a numerical identifier for the remote processor
provided by 'list' command.
Note: Remote processors must be initalized prior to usage
Note: Services are dependent on the driver capability
'list' command shows the capability of each device
Subcommands:
init - Enumerate and initalize the remote processors
list - list available remote processors
load <id> [addr] [size]- Load the remote processor with binary
image stored at address [addr] in memory
start <id> - Start the remote processor(must be loaded)
stop <id> - Stop the remote processor
reset <id> - Reset the remote processor
is_running <id> - Reports if the remote processor is running
ping <id> - Ping the remote processor for communication
=> rproc init
=> rproc list
0 - Name:'proc_3_legacy' type:'internal memory mapped' supports: load start stop reset is_running ping
1 - Name:'remoteproc-test-dev1' type:'internal memory mapped' supports: load start stop reset is_running ping
2 - Name:'remoteproc-test-dev2' type:'internal memory mapped' supports: load start stop reset is_running ping
=> rproc load 0 1 2
Loading to 'proc_3_legacy' from address 0x00000001 size of 2 bytes
=> rproc ping 0
Pinging proc_3_legacy...
proc_3_legacy: No response.(Not started?)
=> rproc start 0
Starting proc_3_legacy...
=> rproc is_running 0
Checking if running: proc_3_legacy...
Remote processor is Running
=> rproc ping 0
Pinging proc_3_legacy...
Remote processor responds 'Pong'
=> rproc ping 1
Pinging remoteproc-test-dev1...
remoteproc-test-dev1: No response.(Not started?)
=> rproc is_running 1
Checking if running: remoteproc-test-dev1...
Remote processor is NOT Running
=>
--
2.1.4
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 1/3] drivers: Introduce a simplified remoteproc framework
2015-08-24 17:28 [U-Boot] [PATCH 0/3] drivers/sandbox: Introduce a simplified remoteproc framework Nishanth Menon
@ 2015-08-24 17:28 ` Nishanth Menon
2015-08-25 5:04 ` Simon Glass
2015-08-24 17:28 ` [U-Boot] [PATCH 2/3] remoteproc: Introduce a sandbox dummy driver Nishanth Menon
` (2 subsequent siblings)
3 siblings, 1 reply; 15+ messages in thread
From: Nishanth Menon @ 2015-08-24 17:28 UTC (permalink / raw)
To: u-boot
Many System on Chip(SoC) solutions are complex with multiple processors
on the same die dedicated to either general purpose of specialized
functions. Many examples do exist in today's SoCs from various vendors.
Typical examples are micro controllers such as an ARM M3/M0 doing a
offload of specific function such as event integration or power
management or controlling camera etc.
Traditionally, the responsibility of loading up such a processor with a
firmware and communication has been with a High Level Operating
System(HLOS) such as Linux. However, there exists classes of products
where Linux would need to expect services from such a processor or the
delay of Linux and operating system being able to load up such a
firmware is unacceptable.
To address these needs, we need some minimal capability to load such a
system and ensure it is started prior to an Operating System(Linux or
any other) is started up.
NOTE: This is NOT meant to be a solve-all solution, instead, it tries to
address certain class of SoCs and products that need such a solution.
A very simple model is introduced here as part of the initial support
that supports microcontrollers with internal memory (no MMU, no
execution from external memory, or specific image format needs). This
basic framework can then (hopefully) be extensible to other complex SoC
processor support as need be.
Signed-off-by: Nishanth Menon <nm@ti.com>
---
common/Kconfig | 5 +
common/Makefile | 1 +
common/cmd_remoteproc.c | 224 ++++++++++++
doc/device-tree-bindings/remoteproc/remoteproc.txt | 14 +
doc/driver-model/remoteproc-framework.txt | 163 +++++++++
drivers/Kconfig | 2 +
drivers/Makefile | 1 +
drivers/remoteproc/Kconfig | 15 +
drivers/remoteproc/Makefile | 7 +
drivers/remoteproc/rproc-uclass.c | 406 +++++++++++++++++++++
include/dm/uclass-id.h | 1 +
include/remoteproc.h | 81 ++++
12 files changed, 920 insertions(+)
create mode 100644 common/cmd_remoteproc.c
create mode 100644 doc/device-tree-bindings/remoteproc/remoteproc.txt
create mode 100644 doc/driver-model/remoteproc-framework.txt
create mode 100644 drivers/remoteproc/Kconfig
create mode 100644 drivers/remoteproc/Makefile
create mode 100644 drivers/remoteproc/rproc-uclass.c
create mode 100644 include/remoteproc.h
diff --git a/common/Kconfig b/common/Kconfig
index 88dc0160796e..16420c880919 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -344,6 +344,11 @@ config CMD_FPGA
help
FPGA support.
+config CMD_REMOTEPROC
+ bool "remoteproc"
+ depends on DM_REMOTEPROC
+ help
+ Support for Remote Processor control
endmenu
diff --git a/common/Makefile b/common/Makefile
index dc82433e90de..c3a62a2e1379 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -154,6 +154,7 @@ obj-$(CONFIG_CMD_PXE) += cmd_pxe.o
obj-$(CONFIG_CMD_READ) += cmd_read.o
obj-$(CONFIG_CMD_REGINFO) += cmd_reginfo.o
obj-$(CONFIG_CMD_REISER) += cmd_reiser.o
+obj-$(CONFIG_CMD_REMOTEPROC) += cmd_remoteproc.o
obj-$(CONFIG_SANDBOX) += cmd_host.o
obj-$(CONFIG_CMD_SATA) += cmd_sata.o
obj-$(CONFIG_CMD_SF) += cmd_sf.o
diff --git a/common/cmd_remoteproc.c b/common/cmd_remoteproc.c
new file mode 100644
index 000000000000..e8fdb124e251
--- /dev/null
+++ b/common/cmd_remoteproc.c
@@ -0,0 +1,224 @@
+/*
+ * (C) Copyright 2015
+ * Texas Instruments Incorporated - http://www.ti.com/
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+#include <common.h>
+#include <command.h>
+#include <remoteproc.h>
+#include <dm.h>
+#include <errno.h>
+#include <malloc.h>
+
+static int print_remoteproc_list(void)
+{
+ struct udevice *dev;
+ struct uclass *uc;
+ int ret;
+ char *type;
+
+ ret = uclass_get(UCLASS_RPROC, &uc);
+ if (ret) {
+ printf("Cannot find Remote processor class\n");
+ return ret;
+ }
+
+ uclass_foreach_dev(dev, uc) {
+ struct dm_rproc_uclass_pdata *uc_pdata;
+ const struct dm_rproc_ops *ops = device_get_ops(dev);
+
+ uc_pdata = dev_get_uclass_platdata(dev);
+ if (!uc_pdata) {
+ debug("%s: no uclass_platdata?\n", dev->name);
+ return -ENXIO;
+ }
+
+ switch (uc_pdata->mem_type) {
+ case RPROC_INTERNAL_MEMORY_MAPPED:
+ type = "internal memory mapped";
+ break;
+ default:
+ type = "unknown";
+ break;
+ }
+ printf("%d - Name:'%s' type:'%s' supports: %s%s%s%s%s%s\n",
+ dev->seq,
+ uc_pdata->name,
+ type,
+ ops->load ? "load " : "",
+ ops->start ? "start " : "",
+ ops->stop ? "stop " : "",
+ ops->reset ? "reset " : "",
+ ops->is_running ? "is_running " : "",
+ ops->ping ? "ping " : "");
+ }
+ return 0;
+}
+
+static int do_rproc_init(cmd_tbl_t *cmdtp, int flag, int argc,
+ char *const argv[])
+{
+ if (rproc_is_initialized()) {
+ printf("\tRemote Processors are already initialized\n");
+ } else {
+ if (!rproc_init())
+ return 0;
+ printf("Few Remote Processors failed to be initalized\n");
+ }
+
+ return CMD_RET_FAILURE;
+}
+
+static int do_remoteproc_list(cmd_tbl_t *cmdtp, int flag, int argc,
+ char *const argv[])
+{
+ if (!rproc_is_initialized()) {
+ printf("\t Remote Processors is not initialized\n");
+ return CMD_RET_USAGE;
+ }
+
+ if (print_remoteproc_list())
+ return CMD_RET_FAILURE;
+
+ return 0;
+}
+
+static int do_remoteproc_load(cmd_tbl_t *cmdtp, int flag, int argc,
+ char *const argv[])
+{
+ ulong addr, size;
+ int id, ret;
+
+ if (argc != 4)
+ return CMD_RET_USAGE;
+
+ id = (int)simple_strtoul(argv[1], NULL, 3);
+ addr = simple_strtoul(argv[2], NULL, 16);
+
+ size = simple_strtoul(argv[3], NULL, 16);
+
+ if (!size) {
+ printf("\t Expect some size??\n");
+ return CMD_RET_USAGE;
+ }
+
+ if (!rproc_is_initialized()) {
+ printf("\tRemote Processors are not initialized\n");
+ return CMD_RET_USAGE;
+ }
+
+ ret = rproc_load(id, addr, size);
+
+ return ret ? CMD_RET_FAILURE : 0;
+}
+
+static int do_remoteproc_wrapper(cmd_tbl_t *cmdtp, int flag, int argc,
+ char *const argv[])
+{
+ int id, ret = CMD_RET_USAGE;
+
+ if (argc != 2)
+ return CMD_RET_USAGE;
+
+ id = (int)simple_strtoul(argv[1], NULL, 3);
+
+ if (!rproc_is_initialized()) {
+ printf("\tRemote Processors are not initialized\n");
+ return CMD_RET_USAGE;
+ }
+
+ if (!strcmp(argv[0], "start")) {
+ ret = rproc_start(id);
+ } else if (!strcmp(argv[0], "stop")) {
+ ret = rproc_stop(id);
+ } else if (!strcmp(argv[0], "reset")) {
+ ret = rproc_reset(id);
+ } else if (!strcmp(argv[0], "is_running")) {
+ ret = rproc_is_running(id);
+ if (!ret) {
+ printf("Remote processor is Running\n");
+ } else if (ret == 1) {
+ printf("Remote processor is NOT Running\n");
+ ret = 0;
+ }
+ /* Else error.. */
+ } else if (!strcmp(argv[0], "ping")) {
+ ret = rproc_ping(id);
+ if (!ret) {
+ printf("Remote processor responds 'Pong'\n");
+ } else if (ret == 1) {
+ printf("No response from Remote processor\n");
+ ret = 0;
+ }
+ /* Else error.. */
+ }
+
+ return ret ? CMD_RET_FAILURE : 0;
+}
+
+static cmd_tbl_t cmd_remoteproc_sub[] = {
+ U_BOOT_CMD_MKENT(init, 0, 1, do_rproc_init,
+ "Enumerate and initialize all processors", ""),
+ U_BOOT_CMD_MKENT(list, 0, 1, do_remoteproc_list,
+ "list remote processors", ""),
+ U_BOOT_CMD_MKENT(load, 5, 1, do_remoteproc_load,
+ "Load remote processor with provided image",
+ "<id> [addr] [size]\n"
+ "- id: ID of the remote processor(see 'list' cmd)\n"
+ "- addr: Address in memory of the image to loadup\n"
+ "- size: Size of the image to loadup\n"),
+ U_BOOT_CMD_MKENT(start, 1, 1, do_remoteproc_wrapper,
+ "Start remote processor",
+ "id - ID of the remote processor (see 'list' cmd)\n"),
+ U_BOOT_CMD_MKENT(stop, 1, 1, do_remoteproc_wrapper,
+ "Stop remote processor",
+ "id - ID of the remote processor (see 'list' cmd)\n"),
+ U_BOOT_CMD_MKENT(reset, 1, 1, do_remoteproc_wrapper,
+ "Reset remote processor",
+ "id - ID of the remote processor (see 'list' cmd)\n"),
+ U_BOOT_CMD_MKENT(is_running, 1, 1, do_remoteproc_wrapper,
+ "Check to see if remote processor is running\n",
+ "id - ID of the remote processor (see 'list' cmd)\n"),
+ U_BOOT_CMD_MKENT(ping, 1, 1, do_remoteproc_wrapper,
+ "Ping to communicate with remote processor\n",
+ "id - ID of the remote processor (see 'list' cmd)\n"),
+};
+
+static int do_remoteproc(cmd_tbl_t *cmdtp, int flag, int argc,
+ char *const argv[])
+{
+ cmd_tbl_t *c = NULL;
+
+ /* Strip off leading 'rproc' command argument */
+ argc--;
+ argv++;
+
+ if (argc)
+ c = find_cmd_tbl(argv[0], cmd_remoteproc_sub,
+ ARRAY_SIZE(cmd_remoteproc_sub));
+ if (c)
+ return c->cmd(cmdtp, flag, argc, argv);
+
+ return CMD_RET_USAGE;
+}
+
+U_BOOT_CMD(rproc, 5, 1, do_remoteproc,
+ "Control operation of remote processors in an SoC",
+ " [init|list|load|start|stop|reset|ping]\n"
+ "\t\t Where:\n"
+ "\t\t[addr] is a memory address\n"
+ "\t\t<id> is a numerical identifier for the remote processor\n"
+ "\t\t provided by 'list' command.\n"
+ "\t\tNote: Remote processors must be initalized prior to usage\n"
+ "\t\tNote: Services are dependent on the driver capability\n"
+ "\t\t 'list' command shows the capability of each device\n"
+ "\n\tSubcommands:\n"
+ "\tinit - Enumerate and initalize the remote processors\n"
+ "\tlist - list available remote processors\n"
+ "\tload <id> [addr] [size]- Load the remote processor with binary\n"
+ "\t image stored at address [addr] in memory\n"
+ "\tstart <id> - Start the remote processor(must be loaded)\n"
+ "\tstop <id> - Stop the remote processor\n"
+ "\treset <id> - Reset the remote processor\n"
+ "\tis_running <id> - Reports if the remote processor is running\n"
+ "\tping <id> - Ping the remote processor for communication\n");
diff --git a/doc/device-tree-bindings/remoteproc/remoteproc.txt b/doc/device-tree-bindings/remoteproc/remoteproc.txt
new file mode 100644
index 000000000000..1a98a2e3a03c
--- /dev/null
+++ b/doc/device-tree-bindings/remoteproc/remoteproc.txt
@@ -0,0 +1,14 @@
+Remote Processor uClass
+
+Binding:
+
+Remoteproc devices shall have compatible corresponding to thier
+drivers. However the following generic properties will be supported
+
+Optional Properties:
+- remoteproc-name: a string, used if provided to describe the processor.
+ This must be unique in an operational system.
+- remoteproc-internal-memory-mapped: a bool, indicates that the remote
+ processor has internal memory that it uses to execute code and store
+ data. Such a device is not expected to have a MMU. If no type property
+ is provided, the device is assumed to map to such a model.
diff --git a/doc/driver-model/remoteproc-framework.txt b/doc/driver-model/remoteproc-framework.txt
new file mode 100644
index 000000000000..32cb40242e62
--- /dev/null
+++ b/doc/driver-model/remoteproc-framework.txt
@@ -0,0 +1,163 @@
+#
+# (C) Copyright 2015
+# Texas Instruments Incorporated - http://www.ti.com/
+# SPDX-License-Identifier: GPL-2.0+
+#
+
+Remote Processor Framework
+==========================
+TOC:
+1. Introduction
+2. How does it work - The driver
+3. Describing the device using platform data
+4. Describing the device using device tree
+
+1. Introduction
+===============
+
+This is an introduction to driver-model for Remote Processors found
+on various System on Chip(SoCs). The term remote processor is used to
+indicate that this is not the processor on which u-boot is operating
+on, instead is yet another processing entity that may be controlled by
+the processor on which we are functional.
+
+The simplified model depends on a single UCLASS - UCLASS_RPROC
+
+UCLASS_RPROC:
+- drivers/remoteproc/rproc-uclass.c
+- include/remoteproc.h
+
+Commands:
+- common/cmd_remoteproc.c
+
+Configuration:
+CONFIG_DM_REMOTEPROC is selected by drivers as needed
+CONFIG_CMD_REMOTEPROC for the commands if required.
+
+2. How does it work - The driver
+=================================
+
+Overall, the driver statemachine transitions are typically as follows:
+ (entry)
+ +-------+
+ +---+ init |
+ | | | <---------------------+
+ | +-------+ |
+ | |
+ | |
+ | +--------+ |
+Load| | reset | |
+ | | | <----------+ |
+ | +--------+ | |
+ | |Load | |
+ | | | |
+ | +----v----+ reset | |
+ +-> | | (opt) | |
+ | Loaded +-----------+ |
+ | | |
+ +----+----+ |
+ | Start |
+ +---v-----+ (opt) |
+ +->| Running | Stop |
+Ping +- | +--------------------+
+(opt) +---------+
+
+(is_running does not change state)
+opt: Optional state transition implemented by driver.
+
+NOTE: It depends on the remote processor as to the exact behavior
+of the statemachine, remoteproc core does not intent to implement
+statemachine logic. Certain processors may allow start/stop without
+reloading the image in the middle, certain other processors may only
+allow us to start the processor(image from a EEPROM/OTP) etc.
+
+It is hence the responsibility of the driver to handle the requisite
+state transitions of the device as necessary.
+
+Basic design assumptions:
+
+Remote processor can operate on a certain firmware that maybe loaded
+and released from reset.
+
+The driver follows a standard UCLASS DM.
+
+in the bare minimum form:
+
+static const struct dm_rproc_ops sandbox_testproc_ops = {
+ .load = sandbox_testproc_load,
+ .start = sandbox_testproc_start,
+};
+
+static const struct udevice_id sandbox_ids[] = {
+ {.compatible = "sandbox,test-processor"},
+ {}
+};
+
+U_BOOT_DRIVER(sandbox_testproc) = {
+ .name = "sandbox_test_proc",
+ .of_match = sandbox_ids,
+ .id = UCLASS_RPROC,
+ .ops = &sandbox_testproc_ops,
+ .probe = sandbox_testproc_probe,
+};
+
+This allows for the device to be probed as part of the "init" command
+or invocation of 'rproc_init()' function as the system dependencies define.
+
+The driver is expected to maintain it's own statemachine which is
+appropriate for the device it maintains. It must, at the very least
+provide a load and start function. We assume here that the device
+needs to be loaded and started, else, there is no real purpose of
+using the remoteproc framework.
+
+3. Describing the device using platform data
+============================================
+
+Considering that many platforms are yet to move to device-tree model,
+a simplified definition of a device is as follows:
+
+struct dm_rproc_uclass_pdata proc_3_test = {
+ .name = "proc_3_legacy",
+ .mem_type = RPROC_INTERNAL_MEMORY_MAPPED,
+ .driver_plat_data = &mydriver_data;
+};
+
+U_BOOT_DEVICE(proc_3_demo) = {
+ .name = "sandbox_test_proc",
+ .platdata = &proc_3_test,
+};
+
+There can be additional data that may be desired depending on the
+remoteproc driver specific needs (for example: SoC integration
+details such as clock handle or something similar). See appropriate
+documentation for specific remoteproc driver for further details.
+These are passed via driver_plat_data.
+
+3. Describing the device using device tree
+==========================================
+/ {
+ ...
+ aliases {
+ ...
+ remoteproc0 = &rproc_1;
+ remoteproc1 = &rproc_2;
+
+ };
+ ...
+
+ rproc_1: rproc at 1 {
+ compatible = "sandbox,test-processor";
+ remoteproc-name = "remoteproc-test-dev1";
+ };
+
+ rproc_2: rproc at 2 {
+ compatible = "sandbox,test-processor";
+ internal-memory-mapped;
+ remoteproc-name = "remoteproc-test-dev2";
+ };
+ ...
+};
+
+aliases usage is optional, but it is usually recommended to ensure the
+users have a consistent usage model for a platform.
+the compatible string used here is specific to the remoteproc driver involved.
diff --git a/drivers/Kconfig b/drivers/Kconfig
index 092bc02b304e..24bd51269602 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -60,6 +60,8 @@ source "drivers/crypto/Kconfig"
source "drivers/thermal/Kconfig"
+source "drivers/remoteproc/Kconfig"
+
endmenu
config PHYS_TO_BUS
diff --git a/drivers/Makefile b/drivers/Makefile
index a721ec86dfef..7c18113ce133 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -59,5 +59,6 @@ obj-y += input/
# SOC specific infrastructure drivers.
obj-y += soc/
obj-y += thermal/
+obj-$(CONFIG_DM_REMOTEPROC) += remoteproc/
endif
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
new file mode 100644
index 000000000000..700f52caf1dc
--- /dev/null
+++ b/drivers/remoteproc/Kconfig
@@ -0,0 +1,15 @@
+#
+# (C) Copyright 2015
+# Texas Instruments Incorporated - http://www.ti.com/
+# SPDX-License-Identifier: GPL-2.0+
+#
+
+menu "Remote Processor drivers"
+
+# DM_REMOTEPROC gets selected by drivers as needed
+# All users should depend on DM
+config DM_REMOTEPROC
+ bool
+ depends on DM
+
+endmenu
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
new file mode 100644
index 000000000000..af90ace95899
--- /dev/null
+++ b/drivers/remoteproc/Makefile
@@ -0,0 +1,7 @@
+#
+# (C) Copyright 2015
+# Texas Instruments Incorporated - http://www.ti.com/
+# SPDX-License-Identifier: GPL-2.0+
+#
+
+obj-$(CONFIG_DM_REMOTEPROC) += rproc-uclass.o
diff --git a/drivers/remoteproc/rproc-uclass.c b/drivers/remoteproc/rproc-uclass.c
new file mode 100644
index 000000000000..cafc293f78f3
--- /dev/null
+++ b/drivers/remoteproc/rproc-uclass.c
@@ -0,0 +1,406 @@
+/*
+ * (C) Copyright 2015
+ * Texas Instruments Incorporated - http://www.ti.com/
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+#define pr_fmt(fmt) "%s: " fmt, __func__
+#include <common.h>
+#include <dm.h>
+#include <dm/uclass.h>
+#include <dm/uclass-internal.h>
+#include <dm/device-internal.h>
+#include <errno.h>
+#include <fdtdec.h>
+#include <malloc.h>
+#include <asm/io.h>
+#include <remoteproc.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static int for_each_remoteproc_device(int (*fn) (struct udevice *dev,
+ struct dm_rproc_uclass_pdata *uc_pdata,
+ const void *data),
+ struct udevice *skip_dev,
+ const void *data)
+{
+ struct udevice *dev;
+ struct dm_rproc_uclass_pdata *uc_pdata;
+ int ret;
+
+ for (ret = uclass_find_first_device(UCLASS_RPROC, &dev); dev;
+ ret = uclass_find_next_device(&dev)) {
+ if (ret || dev == skip_dev)
+ continue;
+ uc_pdata = dev_get_uclass_platdata(dev);
+ ret = fn(dev, uc_pdata, data);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int _rproc_name_is_unique(struct udevice *dev,
+ struct dm_rproc_uclass_pdata *uc_pdata,
+ const void *data)
+{
+ const char *check_name = data;
+
+ /* devices not yet populated with data - so skip them */
+ if (!uc_pdata->name && check_name)
+ return 0;
+
+ /* Return 0 to search further if we dont match */
+ if (strlen(uc_pdata->name) != strlen(check_name))
+ return 0;
+
+ if (!strcmp(uc_pdata->name, check_name))
+ return -EINVAL;
+
+ return 0;
+}
+
+static bool rproc_name_is_unique(struct udevice *check_dev,
+ const char *check_name)
+{
+ int ret;
+
+ ret = for_each_remoteproc_device(_rproc_name_is_unique,
+ check_dev, check_name);
+ return ret ? false : true;
+}
+
+static struct udevice *rproc_find_dev_for_id(int id)
+{
+ struct udevice *dev;
+ int ret;
+
+ for (ret = uclass_find_first_device(UCLASS_RPROC, &dev); dev;
+ ret = uclass_find_next_device(&dev)) {
+ if (ret)
+ continue;
+ if (dev->seq == id)
+ return dev;
+ }
+
+ return NULL;
+}
+
+static int rproc_pre_probe(struct udevice *dev)
+{
+ struct dm_rproc_uclass_pdata *uc_pdata;
+ const struct dm_rproc_ops *ops;
+
+ uc_pdata = dev_get_uclass_platdata(dev);
+ if (!uc_pdata) {
+ debug("%s: no uclass_platdata?\n", dev->name);
+ return -ENXIO;
+ }
+
+ /* See if we need to populate via fdt */
+
+ if (!dev->platdata) {
+#ifdef CONFIG_OF_CONTROL
+ int node = dev->of_offset;
+ const void *blob = gd->fdt_blob;
+ bool tmp;
+ if (!blob) {
+ debug("'%s' no dt?\n", dev->name);
+ return -EINVAL;
+ }
+ debug("'%s': using fdt\n", dev->name);
+ uc_pdata->name = fdt_getprop(blob, node,
+ "remoteproc-name", NULL);
+
+ /* Default is internal memory mapped */
+ uc_pdata->mem_type = RPROC_INTERNAL_MEMORY_MAPPED;
+ tmp = fdtdec_get_bool(blob, node,
+ "remoteproc-internal-memory-mapped");
+ if (tmp)
+ uc_pdata->mem_type = RPROC_INTERNAL_MEMORY_MAPPED;
+#else
+ /* Nothing much we can do about this, can we? */
+ return -EINVAL;
+#endif
+
+ } else {
+ struct dm_rproc_uclass_pdata *pdata = dev->platdata;
+
+ debug("'%s': using legacy data\n", dev->name);
+ if (pdata->name)
+ uc_pdata->name = pdata->name;
+ uc_pdata->mem_type = pdata->mem_type;
+ uc_pdata->driver_plat_data = pdata->driver_plat_data;
+ }
+
+ /* Else try using device Name */
+ if (!uc_pdata->name)
+ uc_pdata->name = dev->name;
+ if (!uc_pdata->name) {
+ debug("Unnamed device!");
+ return -EINVAL;
+ }
+
+ if (!rproc_name_is_unique(dev, uc_pdata->name)) {
+ debug("%s duplicate name '%s'\n", dev->name, uc_pdata->name);
+ return -EINVAL;
+ }
+
+ ops = device_get_ops(dev);
+ if (!ops) {
+ debug("%s driver has no ops?\n", dev->name);
+ return -EINVAL;
+ }
+
+ if (!ops->load || !ops->start) {
+ debug("%s driver has missing mandatory ops?\n", dev->name);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int rproc_post_probe(struct udevice *dev)
+{
+ struct dm_rproc_uclass_pdata *uc_pdata;
+ const struct dm_rproc_ops *ops;
+
+ uc_pdata = dev_get_uclass_platdata(dev);
+ if (!uc_pdata) {
+ debug("%s: no uclass_platdata?\n", dev->name);
+ return -ENXIO;
+ }
+
+ ops = device_get_ops(dev);
+ if (!ops) {
+ debug("%s driver has no ops?\n", dev->name);
+ return -EINVAL;
+ }
+
+ if (ops->init)
+ return ops->init(dev);
+
+ return 0;
+}
+
+UCLASS_DRIVER(rproc) = {
+ /* *INDENT-OFF* */
+ .id = UCLASS_RPROC,
+ .name = "remoteproc",
+ .flags = DM_UC_FLAG_SEQ_ALIAS,
+ .pre_probe = rproc_pre_probe,
+ .post_probe = rproc_post_probe,
+ .per_device_platdata_auto_alloc_size =
+ sizeof(struct dm_rproc_uclass_pdata),
+ /* *INDENT-ON* */
+};
+
+/* exported functions */
+static int _rproc_probe_dev(struct udevice *dev,
+ struct dm_rproc_uclass_pdata *uc_pdata,
+ const void *data)
+{
+ int ret;
+
+ ret = device_probe(dev);
+
+ if (ret)
+ printf("%s: Failed to initialize - %d\n", dev->name, ret);
+ return ret;
+}
+
+static bool _rproc_init_called;
+
+/**
+ * rproc_init() - Initialize all bound remote proc devices
+ */
+int rproc_init(void)
+{
+ int ret;
+
+ if (_rproc_init_called) {
+ debug("Already initialized\n");
+ return -EINVAL;
+ }
+
+ ret = for_each_remoteproc_device(_rproc_probe_dev, NULL, NULL);
+ if (!ret)
+ _rproc_init_called = true;
+ return ret;
+}
+
+/**
+ * rproc_is_initialized() - check to see if remoteproc devices are initialized
+ */
+int rproc_is_initialized(void)
+{
+ return _rproc_init_called;
+}
+
+/**
+ * rproc_load() - load binary to a remote processor
+ * @id: id of the remote processor
+ * @addr: address in memory where the binary image is located
+ * @size: size of the binary image
+ */
+int rproc_load(int id, ulong addr, ulong size)
+{
+ struct udevice *dev = rproc_find_dev_for_id(id);
+ struct dm_rproc_uclass_pdata *uc_pdata;
+ const struct dm_rproc_ops *ops;
+
+ if (!dev) {
+ printf("Unknown remote processor id '%d' requested\n", id);
+ return -EINVAL;
+ }
+
+ uc_pdata = dev_get_uclass_platdata(dev);
+ if (!uc_pdata) {
+ debug("%s: no uclass_platdata?\n", dev->name);
+ return -ENXIO;
+ }
+
+ ops = device_get_ops(dev);
+ if (!ops) {
+ debug("%s driver has no ops?\n", dev->name);
+ return -EINVAL;
+ }
+
+ printf("Loading to '%s' from address 0x%08lX size of %lu bytes\n",
+ uc_pdata->name, addr, size);
+ if (ops->load)
+ return ops->load(dev, addr, size);
+
+ debug("%s: data corruption?? mandatory function is missing!\n",
+ dev->name);
+
+ return -EINVAL;
+};
+
+/*
+ * Completely internal helper enums..
+ * Keeping this isolated helps this code evolve independent of other
+ * parts..
+ */
+enum rproc_ops {
+ RPROC_START,
+ RPROC_STOP,
+ RPROC_RESET,
+ RPROC_PING,
+ RPROC_RUNNING,
+};
+
+static int _rproc_wrapper(int id, enum rproc_ops op)
+{
+ struct udevice *dev = rproc_find_dev_for_id(id);
+ struct dm_rproc_uclass_pdata *uc_pdata;
+ const struct dm_rproc_ops *ops;
+ int (*fn)(struct udevice *dev);
+ bool mandatory = false;
+ char *op_str;
+
+ if (!dev) {
+ printf("Unknown remote processor id '%d' requested\n", id);
+ return -EINVAL;
+ }
+
+ uc_pdata = dev_get_uclass_platdata(dev);
+ if (!uc_pdata) {
+ debug("%s: no uclass_platdata?\n", dev->name);
+ return -ENXIO;
+ }
+
+ ops = device_get_ops(dev);
+ if (!ops) {
+ debug("%s driver has no ops?\n", dev->name);
+ return -EINVAL;
+ }
+ switch (op) {
+ case RPROC_START:
+ fn = ops->start;
+ mandatory = true;
+ op_str = "Starting";
+ break;
+ case RPROC_STOP:
+ fn = ops->stop;
+ op_str = "Stopping";
+ break;
+ case RPROC_RESET:
+ fn = ops->reset;
+ op_str = "Resetting";
+ break;
+ case RPROC_RUNNING:
+ fn = ops->is_running;
+ op_str = "Checking if running:";
+ break;
+ case RPROC_PING:
+ fn = ops->ping;
+ op_str = "Pinging";
+ break;
+ default:
+ debug("what is '%d' operation??\n", op);
+ return -EINVAL;
+ }
+
+ printf("%s %s...\n", op_str, uc_pdata->name);
+ if (fn)
+ return fn(dev);
+
+ if (mandatory)
+ debug("%s: data corruption?? mandatory function is missing!\n",
+ dev->name);
+
+ return -EINVAL;
+}
+
+/**
+ * rproc_start() - Start a remote processor
+ * @id: id of the remote processor
+ */
+int rproc_start(int id)
+{
+ return _rproc_wrapper(id, RPROC_START);
+};
+
+/**
+ * rproc_stop() - Stop a remote processor
+ * @id: id of the remote processor
+ */
+int rproc_stop(int id)
+{
+ return _rproc_wrapper(id, RPROC_STOP);
+};
+
+/**
+ * rproc_reset() - reset a remote processor
+ * @id: id of the remote processor
+ */
+int rproc_reset(int id)
+{
+ return _rproc_wrapper(id, RPROC_RESET);
+};
+
+/**
+ * rproc_ping() - ping a remote processor to check if it can communicate
+ * @id: id of the remote processor
+ *
+ * NOTE: this might need communication path available, which is not implemented
+ * as part of remoteproc framework - hook on to appropriate bus architecture to
+ * do the same
+ */
+int rproc_ping(int id)
+{
+ return _rproc_wrapper(id, RPROC_PING);
+};
+
+/**
+ * rproc_is_running() - check to see if remote processor is running
+ * @id: id of the remote processor
+ *
+ * NOTE: this may not involve actual communication capability of the remote
+ * processor, but just ensures that it is out of reset and executing code.
+ */
+int rproc_is_running(int id)
+{
+ return _rproc_wrapper(id, RPROC_RUNNING);
+};
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index c744044bb8aa..a2958c234db4 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -47,6 +47,7 @@ enum uclass_id {
UCLASS_PMIC, /* PMIC I/O device */
UCLASS_REGULATOR, /* Regulator device */
UCLASS_RESET, /* Reset device */
+ UCLASS_RPROC, /* Remote Processor device */
UCLASS_RTC, /* Real time clock device */
UCLASS_SERIAL, /* Serial UART */
UCLASS_SPI, /* SPI bus */
diff --git a/include/remoteproc.h b/include/remoteproc.h
new file mode 100644
index 000000000000..b92d40e0ca2e
--- /dev/null
+++ b/include/remoteproc.h
@@ -0,0 +1,81 @@
+/*
+ * (C) Copyright 2015
+ * Texas Instruments Incorporated - http://www.ti.com/
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#ifndef _RPROC_H_
+#define _RPROC_H_
+
+#include <dm/platdata.h> /* For platform data support - non dt world */
+
+/**
+ * enum rproc_mem_type - What type of memory model does the rproc use
+ * @RPROC_INTERNAL: Remote processor uses own memory and is memory mapped
+ * to the host processor over an address range.
+ *
+ * Please note that this is an enumeration of memory model of different types
+ * of remote processors. Few of the remote processors do have own internal
+ * memories, while others use external memory for instruction and data.
+ */
+enum rproc_mem_type {
+ RPROC_INTERNAL_MEMORY_MAPPED = 0,
+};
+
+/**
+ * struct dm_rproc_uclass_pdata - platform data for a CPU
+ *
+ * This can be accessed with dev_get_platdata() for any UCLASS_RPROC
+ * device.
+ *
+ * @name: Platform-specific way of naming the Remote proc
+ * @mem_type: one of 'enum rproc_mem_type'
+ * @driver_data: driver specific platform data that may be needed.
+ */
+struct dm_rproc_uclass_pdata {
+ const char *name;
+ enum rproc_mem_type mem_type;
+ void *driver_plat_data;
+};
+
+/**
+ * struct dm_rproc_ops - Operations that are provided by remote proc driver
+ * @init: Initialize the remoteproc device invoked after probe (optional)
+ * @load: Load the remoteproc device using data provided(mandatory)
+ * @start: Start the remoteproc device (mandatory)
+ * @stop: Stop the remoteproc device (optional)
+ * @reset: Reset the remote proc device (optional)
+ * @is_running: Check if the remote processor is running(optional)
+ * @ping: Ping the remote device for basic communication check(optional)
+ */
+struct dm_rproc_ops {
+ int (*init)(struct udevice *dev);
+ int (*load)(struct udevice *dev, ulong addr, ulong size);
+ int (*start)(struct udevice *dev);
+ int (*stop)(struct udevice *dev);
+ int (*reset)(struct udevice *dev);
+ int (*is_running)(struct udevice *dev);
+ int (*ping)(struct udevice *dev);
+};
+
+#ifdef CONFIG_DM_REMOTEPROC
+int rproc_init(void);
+int rproc_is_initialized(void);
+int rproc_load(int id, ulong addr, ulong size);
+int rproc_start(int id);
+int rproc_stop(int id);
+int rproc_reset(int id);
+int rproc_ping(int id);
+int rproc_is_running(int id);
+#else
+static inline int rproc_init(void) { return -EINVAL; }
+static inline int rproc_is_initialized(void) { return false; }
+static inline int rproc_load(int id, ulong addr, ulong size) { return -EINVAL; }
+static inline int rproc_start(int id) { return -EINVAL; }
+static inline int rproc_stop(int id) { return -EINVAL; }
+static inline int rproc_reset(int id) { return -EINVAL; }
+static inline int rproc_ping(int id) { return -EINVAL; }
+static inline int rproc_is_running(int id) { return -EINVAL; }
+#endif
+
+#endif /* _RPROC_H_ */
--
2.1.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/3] remoteproc: Introduce a sandbox dummy driver
2015-08-24 17:28 [U-Boot] [PATCH 0/3] drivers/sandbox: Introduce a simplified remoteproc framework Nishanth Menon
2015-08-24 17:28 ` [U-Boot] [PATCH 1/3] drivers: " Nishanth Menon
@ 2015-08-24 17:28 ` Nishanth Menon
2015-08-25 5:04 ` Simon Glass
2015-08-24 17:28 ` [U-Boot] [PATCH 3/3] sandbox: Introduce dummy remoteproc nodes Nishanth Menon
2015-08-25 5:04 ` [U-Boot] [PATCH 0/3] drivers/sandbox: Introduce a simplified remoteproc framework Simon Glass
3 siblings, 1 reply; 15+ messages in thread
From: Nishanth Menon @ 2015-08-24 17:28 UTC (permalink / raw)
To: u-boot
Introduce a dummy driver for sandbox that allows us to verify basic
functionality. This is not meant to do anything functional - but is
more or less meant as a framework plumbing debug helper.
The sandbox remoteproc driver maintains absolutey no states and is a
simple driver which just is filled with empty hooks. Idea being to give
an approximate idea to implement own remoteproc driver using this as a
template.
Signed-off-by: Nishanth Menon <nm@ti.com>
---
drivers/remoteproc/Kconfig | 9 ++
drivers/remoteproc/Makefile | 3 +
drivers/remoteproc/sandbox_testproc.c | 243 ++++++++++++++++++++++++++++++++++
3 files changed, 255 insertions(+)
create mode 100644 drivers/remoteproc/sandbox_testproc.c
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 700f52caf1dc..61cc506d6baa 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -12,4 +12,13 @@ config DM_REMOTEPROC
bool
depends on DM
+# Please keep the configuration alphabetically sorted.
+config DM_TESTPROC_SANDBOX
+ bool "Support for Test processor for Sandbox"
+ select DM_REMOTEPROC
+ depends on DM
+ depends on SANDBOX
+ help
+ Say 'y' here to add support for test processor which does dummy
+ operations for sandbox platform.
endmenu
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index af90ace95899..57c04d0e024a 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -5,3 +5,6 @@
#
obj-$(CONFIG_DM_REMOTEPROC) += rproc-uclass.o
+
+# Remote proc drivers - Please keep this list alphabetically sorted.
+obj-$(CONFIG_DM_TESTPROC_SANDBOX) += sandbox_testproc.o
diff --git a/drivers/remoteproc/sandbox_testproc.c b/drivers/remoteproc/sandbox_testproc.c
new file mode 100644
index 000000000000..e942f66fc2dc
--- /dev/null
+++ b/drivers/remoteproc/sandbox_testproc.c
@@ -0,0 +1,243 @@
+/*
+ * (C) Copyright 2015
+ * Texas Instruments Incorporated - http://www.ti.com/
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+#define pr_fmt(fmt) "%s: " fmt, __func__
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <remoteproc.h>
+
+enum sandbox_state {
+ sb_entry,
+ sb_init,
+ sb_reset,
+ sb_loaded,
+ sb_running
+};
+
+struct sandbox_test_devdata {
+ enum sandbox_state current_state;
+};
+
+static int sandbox_dev_move_to_state(struct udevice *dev,
+ enum sandbox_state next_state)
+{
+ struct sandbox_test_devdata *ddata = dev_get_priv(dev);
+
+ /* No state transition is OK */
+ if (ddata->current_state == next_state)
+ return 0;
+
+ debug("current_state=%d, next_state=%d\n", ddata->current_state, next_state);
+ switch (ddata->current_state) {
+ case sb_entry:
+ if (next_state == sb_init)
+ goto ok_state;
+ break;
+
+ case sb_init:
+ if (next_state == sb_reset || next_state == sb_loaded)
+ goto ok_state;
+ break;
+
+ case sb_reset:
+ if (next_state == sb_loaded || next_state == sb_init)
+ goto ok_state;
+ break;
+
+ case sb_loaded:
+ if (next_state == sb_reset || next_state == sb_init ||
+ next_state == sb_running)
+ goto ok_state;
+ break;
+
+ case sb_running:
+ if (next_state == sb_reset || next_state == sb_init)
+ goto ok_state;
+ break;
+ };
+ return -EINVAL;
+
+ok_state:
+ ddata->current_state = next_state;
+ return 0;
+}
+
+static int sandbox_testproc_probe(struct udevice *dev)
+{
+ struct dm_rproc_uclass_pdata *uc_pdata;
+ struct sandbox_test_devdata *ddata;
+ int ret;
+
+ ddata = dev_get_priv(dev);
+ uc_pdata = dev_get_uclass_platdata(dev);
+ if (!uc_pdata || !ddata) {
+ printf("%s: platform data %p OR driver data %p missing\n",
+ dev->name, uc_pdata, ddata);
+ return -EINVAL;
+ }
+ ret = sandbox_dev_move_to_state(dev, sb_entry);
+
+ debug("%s: called(%d)\n", uc_pdata->name, ret);
+
+ return ret;
+}
+
+static int sandbox_testproc_init(struct udevice *dev)
+{
+ struct dm_rproc_uclass_pdata *uc_pdata;
+ int ret;
+
+ uc_pdata = dev_get_uclass_platdata(dev);
+
+ ret = sandbox_dev_move_to_state(dev, sb_init);
+
+ debug("%s: called(%d)\n", uc_pdata->name, ret);
+ if (ret)
+ printf("%s init failed\n", uc_pdata->name);
+
+ return ret;
+}
+
+static int sandbox_testproc_reset(struct udevice *dev)
+{
+ struct dm_rproc_uclass_pdata *uc_pdata;
+ int ret;
+
+ uc_pdata = dev_get_uclass_platdata(dev);
+
+ ret = sandbox_dev_move_to_state(dev, sb_reset);
+
+ debug("%s: called(%d)\n", uc_pdata->name, ret);
+
+ if (ret)
+ printf("%s reset failed\n", uc_pdata->name);
+ return ret;
+}
+
+static int sandbox_testproc_load(struct udevice *dev, ulong addr, ulong size)
+{
+ struct dm_rproc_uclass_pdata *uc_pdata;
+ int ret;
+
+ uc_pdata = dev_get_uclass_platdata(dev);
+
+ ret = sandbox_dev_move_to_state(dev, sb_loaded);
+
+ debug("%s: called(%d) Loading to %08lX %lu size\n",
+ uc_pdata->name, ret, addr, size);
+
+ if (ret)
+ printf("%s load failed\n", uc_pdata->name);
+ return ret;
+}
+
+static int sandbox_testproc_start(struct udevice *dev)
+{
+ struct dm_rproc_uclass_pdata *uc_pdata;
+ int ret;
+
+ uc_pdata = dev_get_uclass_platdata(dev);
+
+ ret = sandbox_dev_move_to_state(dev, sb_running);
+
+ debug("%s: called(%d)\n", uc_pdata->name, ret);
+
+ if (ret)
+ printf("%s start failed\n", uc_pdata->name);
+ return ret;
+}
+
+static int sandbox_testproc_stop(struct udevice *dev)
+{
+ struct dm_rproc_uclass_pdata *uc_pdata;
+ int ret;
+
+ uc_pdata = dev_get_uclass_platdata(dev);
+
+ ret = sandbox_dev_move_to_state(dev, sb_init);
+
+ debug("%s: called(%d)\n", uc_pdata->name, ret);
+
+ if (ret)
+ printf("%s stop failed\n", uc_pdata->name);
+ return ret;
+}
+
+static int sandbox_testproc_is_running(struct udevice *dev)
+{
+ struct dm_rproc_uclass_pdata *uc_pdata;
+ struct sandbox_test_devdata *ddata;
+ int ret = 1;
+
+ uc_pdata = dev_get_uclass_platdata(dev);
+ ddata = dev_get_priv(dev);
+
+ if (ddata->current_state == sb_running)
+ ret = 0;
+ debug("%s: called(%d)\n", uc_pdata->name, ret);
+
+ return ret;
+}
+
+static int sandbox_testproc_ping(struct udevice *dev)
+{
+ struct dm_rproc_uclass_pdata *uc_pdata;
+ struct sandbox_test_devdata *ddata;
+ int ret = 1;
+
+ uc_pdata = dev_get_uclass_platdata(dev);
+ ddata = dev_get_priv(dev);
+
+ if (ddata->current_state == sb_running)
+ ret = 0;
+ else
+ ret = -EINVAL;
+
+ debug("%s: called(%d)\n", uc_pdata->name, ret);
+ if (ret) {
+ printf("%s: No response.(Not started?)\n", uc_pdata->name);
+ }
+
+ return ret;
+}
+
+static const struct dm_rproc_ops sandbox_testproc_ops = {
+ .init = sandbox_testproc_init,
+ .reset = sandbox_testproc_reset,
+ .load = sandbox_testproc_load,
+ .start = sandbox_testproc_start,
+ .stop = sandbox_testproc_stop,
+ .is_running = sandbox_testproc_is_running,
+ .ping = sandbox_testproc_ping,
+};
+
+static const struct udevice_id sandbox_ids[] = {
+ {.compatible = "sandbox,test-processor"},
+ {}
+};
+
+U_BOOT_DRIVER(sandbox_testproc) = {
+ /* *INDENT-OFF* */
+ .name = "sandbox_test_proc",
+ .of_match = sandbox_ids,
+ .id = UCLASS_RPROC,
+ .ops = &sandbox_testproc_ops,
+ .probe = sandbox_testproc_probe,
+ .priv_auto_alloc_size = sizeof(struct sandbox_test_devdata),
+ /* *INDENT-ON* */
+};
+
+static struct dm_rproc_uclass_pdata proc_3_test = {
+ .name = "proc_3_legacy",
+ .mem_type = RPROC_INTERNAL_MEMORY_MAPPED,
+};
+
+U_BOOT_DEVICE(proc_3_demo) = {
+ /* *INDENT-OFF* */
+ .name = "sandbox_test_proc",
+ .platdata = &proc_3_test,
+ /* *INDENT-ON* */
+};
--
2.1.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 3/3] sandbox: Introduce dummy remoteproc nodes
2015-08-24 17:28 [U-Boot] [PATCH 0/3] drivers/sandbox: Introduce a simplified remoteproc framework Nishanth Menon
2015-08-24 17:28 ` [U-Boot] [PATCH 1/3] drivers: " Nishanth Menon
2015-08-24 17:28 ` [U-Boot] [PATCH 2/3] remoteproc: Introduce a sandbox dummy driver Nishanth Menon
@ 2015-08-24 17:28 ` Nishanth Menon
2015-08-25 5:04 ` Simon Glass
2015-08-25 5:04 ` [U-Boot] [PATCH 0/3] drivers/sandbox: Introduce a simplified remoteproc framework Simon Glass
3 siblings, 1 reply; 15+ messages in thread
From: Nishanth Menon @ 2015-08-24 17:28 UTC (permalink / raw)
To: u-boot
Introduce dummy devices for sandbox remoteproc device and enable it by
default
Signed-off-by: Nishanth Menon <nm@ti.com>
---
arch/sandbox/dts/test.dts | 13 +++++++++++++
configs/sandbox_defconfig | 2 ++
2 files changed, 15 insertions(+)
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index c948df8c864b..df9b71310dbe 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -25,6 +25,8 @@
usb0 = &usb_0;
usb1 = &usb_1;
usb2 = &usb_2;
+ remoteproc1 = &rproc_1;
+ remoteproc2 = &rproc_2;
};
a-test {
@@ -296,6 +298,17 @@
status = "disabled";
};
+
+ rproc_1: rproc at 1 {
+ compatible = "sandbox,test-processor";
+ remoteproc-name = "remoteproc-test-dev1";
+ };
+
+ rproc_2: rproc at 2 {
+ compatible = "sandbox,test-processor";
+ internal-memory-mapped;
+ remoteproc-name = "remoteproc-test-dev2";
+ };
};
#include "sandbox_pmic.dtsi"
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 874a26b572aa..4a2750c154bc 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -54,3 +54,5 @@ CONFIG_LED_GPIO=y
CONFIG_SYSCON=y
CONFIG_REGMAP=y
CONFIG_DEVRES=y
+CONFIG_DM_TESTPROC_SANDBOX=y
+CONFIG_CMD_REMOTEPROC=y
--
2.1.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 0/3] drivers/sandbox: Introduce a simplified remoteproc framework
2015-08-24 17:28 [U-Boot] [PATCH 0/3] drivers/sandbox: Introduce a simplified remoteproc framework Nishanth Menon
` (2 preceding siblings ...)
2015-08-24 17:28 ` [U-Boot] [PATCH 3/3] sandbox: Introduce dummy remoteproc nodes Nishanth Menon
@ 2015-08-25 5:04 ` Simon Glass
2015-08-25 15:48 ` Nishanth Menon
3 siblings, 1 reply; 15+ messages in thread
From: Simon Glass @ 2015-08-25 5:04 UTC (permalink / raw)
To: u-boot
Hi Nishanth,
On 24 August 2015 at 11:28, Nishanth Menon <nm@ti.com> wrote:
> Hi,
>
> Many System on Chip(SoC) solutions are complex with multiple
> processors on the same die dedicated to either general purpose of
> specialized functions. Many examples do exist in today's SoCs from
> various vendors. Typical examples are micro controllers such as an ARM
> M3/M0 doing a offload of specific function such as event integration
> or power management or controlling camera etc.
>
> Traditionally, the responsibility of loading up such a processor with
> a firmware and communication has been with a High Level Operating
> System(HLOS) such as Linux. However, there exists classes of products
> where Linux would need to expect services from such a processor or
> the delay of Linux and operating system being able to load up such a
> firmware is unacceptable.
>
> The intent here is to introduce a simplified remoteproc framework
> which can then be used to provide basic services to these remote
> processors.
>
> Nishanth Menon (3):
> drivers: Introduce a simplified remoteproc framework
> remoteproc: Introduce a sandbox dummy driver
> sandbox: Introduce dummy remoteproc nodes
>
> arch/sandbox/dts/test.dts | 13 +
> common/Kconfig | 5 +
> common/Makefile | 1 +
> common/cmd_remoteproc.c | 224 ++++++++++++
> configs/sandbox_defconfig | 2 +
> doc/device-tree-bindings/remoteproc/remoteproc.txt | 14 +
> doc/driver-model/remoteproc-framework.txt | 163 +++++++++
> drivers/Kconfig | 2 +
> drivers/Makefile | 1 +
> drivers/remoteproc/Kconfig | 24 ++
> drivers/remoteproc/Makefile | 10 +
> drivers/remoteproc/rproc-uclass.c | 406 +++++++++++++++++++++
> drivers/remoteproc/sandbox_testproc.c | 243 ++++++++++++
> include/dm/uclass-id.h | 1 +
> include/remoteproc.h | 81 ++++
> 15 files changed, 1190 insertions(+)
> create mode 100644 common/cmd_remoteproc.c
> create mode 100644 doc/device-tree-bindings/remoteproc/remoteproc.txt
> create mode 100644 doc/driver-model/remoteproc-framework.txt
> create mode 100644 drivers/remoteproc/Kconfig
> create mode 100644 drivers/remoteproc/Makefile
> create mode 100644 drivers/remoteproc/rproc-uclass.c
> create mode 100644 drivers/remoteproc/sandbox_testproc.c
> create mode 100644 include/remoteproc.h
>
> Simple test with test.dtb:
> u-boot$ ./u-boot -d ./arch/sandbox/dts/test.dtb
Can you please also add a test to test/dm/remoteproc.c? It should try
a few operations as a sanity check. Ideally it shouldn't output
anything on the console.
Regards,
Simon
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 1/3] drivers: Introduce a simplified remoteproc framework
2015-08-24 17:28 ` [U-Boot] [PATCH 1/3] drivers: " Nishanth Menon
@ 2015-08-25 5:04 ` Simon Glass
2015-08-25 15:44 ` Nishanth Menon
0 siblings, 1 reply; 15+ messages in thread
From: Simon Glass @ 2015-08-25 5:04 UTC (permalink / raw)
To: u-boot
Hi Nishanth,
On 24 August 2015 at 11:28, Nishanth Menon <nm@ti.com> wrote:
> Many System on Chip(SoC) solutions are complex with multiple processors
> on the same die dedicated to either general purpose of specialized
> functions. Many examples do exist in today's SoCs from various vendors.
> Typical examples are micro controllers such as an ARM M3/M0 doing a
> offload of specific function such as event integration or power
> management or controlling camera etc.
>
> Traditionally, the responsibility of loading up such a processor with a
> firmware and communication has been with a High Level Operating
> System(HLOS) such as Linux. However, there exists classes of products
> where Linux would need to expect services from such a processor or the
> delay of Linux and operating system being able to load up such a
> firmware is unacceptable.
>
> To address these needs, we need some minimal capability to load such a
> system and ensure it is started prior to an Operating System(Linux or
> any other) is started up.
>
> NOTE: This is NOT meant to be a solve-all solution, instead, it tries to
> address certain class of SoCs and products that need such a solution.
>
> A very simple model is introduced here as part of the initial support
> that supports microcontrollers with internal memory (no MMU, no
> execution from external memory, or specific image format needs). This
> basic framework can then (hopefully) be extensible to other complex SoC
> processor support as need be.
>
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
> common/Kconfig | 5 +
> common/Makefile | 1 +
> common/cmd_remoteproc.c | 224 ++++++++++++
> doc/device-tree-bindings/remoteproc/remoteproc.txt | 14 +
> doc/driver-model/remoteproc-framework.txt | 163 +++++++++
> drivers/Kconfig | 2 +
> drivers/Makefile | 1 +
> drivers/remoteproc/Kconfig | 15 +
> drivers/remoteproc/Makefile | 7 +
> drivers/remoteproc/rproc-uclass.c | 406 +++++++++++++++++++++
> include/dm/uclass-id.h | 1 +
> include/remoteproc.h | 81 ++++
> 12 files changed, 920 insertions(+)
> create mode 100644 common/cmd_remoteproc.c
> create mode 100644 doc/device-tree-bindings/remoteproc/remoteproc.txt
> create mode 100644 doc/driver-model/remoteproc-framework.txt
> create mode 100644 drivers/remoteproc/Kconfig
> create mode 100644 drivers/remoteproc/Makefile
> create mode 100644 drivers/remoteproc/rproc-uclass.c
> create mode 100644 include/remoteproc.h
>
> diff --git a/common/Kconfig b/common/Kconfig
> index 88dc0160796e..16420c880919 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -344,6 +344,11 @@ config CMD_FPGA
> help
> FPGA support.
>
> +config CMD_REMOTEPROC
> + bool "remoteproc"
> + depends on DM_REMOTEPROC
> + help
> + Support for Remote Processor control
> endmenu
>
>
> diff --git a/common/Makefile b/common/Makefile
> index dc82433e90de..c3a62a2e1379 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -154,6 +154,7 @@ obj-$(CONFIG_CMD_PXE) += cmd_pxe.o
> obj-$(CONFIG_CMD_READ) += cmd_read.o
> obj-$(CONFIG_CMD_REGINFO) += cmd_reginfo.o
> obj-$(CONFIG_CMD_REISER) += cmd_reiser.o
> +obj-$(CONFIG_CMD_REMOTEPROC) += cmd_remoteproc.o
> obj-$(CONFIG_SANDBOX) += cmd_host.o
> obj-$(CONFIG_CMD_SATA) += cmd_sata.o
> obj-$(CONFIG_CMD_SF) += cmd_sf.o
> diff --git a/common/cmd_remoteproc.c b/common/cmd_remoteproc.c
> new file mode 100644
> index 000000000000..e8fdb124e251
> --- /dev/null
> +++ b/common/cmd_remoteproc.c
> @@ -0,0 +1,224 @@
> +/*
> + * (C) Copyright 2015
> + * Texas Instruments Incorporated - http://www.ti.com/
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +#include <common.h>
> +#include <command.h>
> +#include <remoteproc.h>
> +#include <dm.h>
> +#include <errno.h>
> +#include <malloc.h>
> +
nit: can you please sort those includes?
> +static int print_remoteproc_list(void)
> +{
> + struct udevice *dev;
> + struct uclass *uc;
> + int ret;
> + char *type;
> +
> + ret = uclass_get(UCLASS_RPROC, &uc);
> + if (ret) {
> + printf("Cannot find Remote processor class\n");
> + return ret;
> + }
> +
> + uclass_foreach_dev(dev, uc) {
> + struct dm_rproc_uclass_pdata *uc_pdata;
> + const struct dm_rproc_ops *ops = device_get_ops(dev);
Can you create a rproc_get_ops() static inline as is done with other
uclasses, and use that?
> +
> + uc_pdata = dev_get_uclass_platdata(dev);
> + if (!uc_pdata) {
> + debug("%s: no uclass_platdata?\n", dev->name);
> + return -ENXIO;
> + }
That can never happen so you can remove this check.
> +
> + switch (uc_pdata->mem_type) {
> + case RPROC_INTERNAL_MEMORY_MAPPED:
> + type = "internal memory mapped";
> + break;
> + default:
> + type = "unknown";
> + break;
> + }
> + printf("%d - Name:'%s' type:'%s' supports: %s%s%s%s%s%s\n",
> + dev->seq,
> + uc_pdata->name,
> + type,
> + ops->load ? "load " : "",
> + ops->start ? "start " : "",
> + ops->stop ? "stop " : "",
> + ops->reset ? "reset " : "",
> + ops->is_running ? "is_running " : "",
> + ops->ping ? "ping " : "");
> + }
> + return 0;
> +}
> +
> +static int do_rproc_init(cmd_tbl_t *cmdtp, int flag, int argc,
> + char *const argv[])
> +{
> + if (rproc_is_initialized()) {
> + printf("\tRemote Processors are already initialized\n");
> + } else {
> + if (!rproc_init())
> + return 0;
> + printf("Few Remote Processors failed to be initalized\n");
> + }
> +
> + return CMD_RET_FAILURE;
> +}
> +
> +static int do_remoteproc_list(cmd_tbl_t *cmdtp, int flag, int argc,
> + char *const argv[])
> +{
> + if (!rproc_is_initialized()) {
> + printf("\t Remote Processors is not initialized\n");
> + return CMD_RET_USAGE;
> + }
> +
> + if (print_remoteproc_list())
> + return CMD_RET_FAILURE;
> +
> + return 0;
> +}
> +
> +static int do_remoteproc_load(cmd_tbl_t *cmdtp, int flag, int argc,
> + char *const argv[])
> +{
> + ulong addr, size;
> + int id, ret;
> +
> + if (argc != 4)
> + return CMD_RET_USAGE;
> +
> + id = (int)simple_strtoul(argv[1], NULL, 3);
> + addr = simple_strtoul(argv[2], NULL, 16);
> +
> + size = simple_strtoul(argv[3], NULL, 16);
> +
> + if (!size) {
> + printf("\t Expect some size??\n");
> + return CMD_RET_USAGE;
> + }
> +
> + if (!rproc_is_initialized()) {
> + printf("\tRemote Processors are not initialized\n");
> + return CMD_RET_USAGE;
> + }
> +
> + ret = rproc_load(id, addr, size);
> +
> + return ret ? CMD_RET_FAILURE : 0;
> +}
> +
> +static int do_remoteproc_wrapper(cmd_tbl_t *cmdtp, int flag, int argc,
> + char *const argv[])
> +{
> + int id, ret = CMD_RET_USAGE;
> +
> + if (argc != 2)
> + return CMD_RET_USAGE;
> +
> + id = (int)simple_strtoul(argv[1], NULL, 3);
> +
> + if (!rproc_is_initialized()) {
> + printf("\tRemote Processors are not initialized\n");
> + return CMD_RET_USAGE;
> + }
> +
> + if (!strcmp(argv[0], "start")) {
> + ret = rproc_start(id);
> + } else if (!strcmp(argv[0], "stop")) {
> + ret = rproc_stop(id);
> + } else if (!strcmp(argv[0], "reset")) {
> + ret = rproc_reset(id);
> + } else if (!strcmp(argv[0], "is_running")) {
> + ret = rproc_is_running(id);
> + if (!ret) {
> + printf("Remote processor is Running\n");
> + } else if (ret == 1) {
> + printf("Remote processor is NOT Running\n");
> + ret = 0;
> + }
> + /* Else error.. */
> + } else if (!strcmp(argv[0], "ping")) {
> + ret = rproc_ping(id);
> + if (!ret) {
> + printf("Remote processor responds 'Pong'\n");
> + } else if (ret == 1) {
> + printf("No response from Remote processor\n");
> + ret = 0;
> + }
> + /* Else error.. */
> + }
> +
> + return ret ? CMD_RET_FAILURE : 0;
> +}
> +
> +static cmd_tbl_t cmd_remoteproc_sub[] = {
> + U_BOOT_CMD_MKENT(init, 0, 1, do_rproc_init,
> + "Enumerate and initialize all processors", ""),
> + U_BOOT_CMD_MKENT(list, 0, 1, do_remoteproc_list,
> + "list remote processors", ""),
> + U_BOOT_CMD_MKENT(load, 5, 1, do_remoteproc_load,
> + "Load remote processor with provided image",
> + "<id> [addr] [size]\n"
> + "- id: ID of the remote processor(see 'list' cmd)\n"
> + "- addr: Address in memory of the image to loadup\n"
> + "- size: Size of the image to loadup\n"),
> + U_BOOT_CMD_MKENT(start, 1, 1, do_remoteproc_wrapper,
> + "Start remote processor",
> + "id - ID of the remote processor (see 'list' cmd)\n"),
> + U_BOOT_CMD_MKENT(stop, 1, 1, do_remoteproc_wrapper,
> + "Stop remote processor",
> + "id - ID of the remote processor (see 'list' cmd)\n"),
> + U_BOOT_CMD_MKENT(reset, 1, 1, do_remoteproc_wrapper,
> + "Reset remote processor",
> + "id - ID of the remote processor (see 'list' cmd)\n"),
> + U_BOOT_CMD_MKENT(is_running, 1, 1, do_remoteproc_wrapper,
> + "Check to see if remote processor is running\n",
> + "id - ID of the remote processor (see 'list' cmd)\n"),
> + U_BOOT_CMD_MKENT(ping, 1, 1, do_remoteproc_wrapper,
> + "Ping to communicate with remote processor\n",
> + "id - ID of the remote processor (see 'list' cmd)\n"),
> +};
> +
> +static int do_remoteproc(cmd_tbl_t *cmdtp, int flag, int argc,
> + char *const argv[])
> +{
> + cmd_tbl_t *c = NULL;
> +
> + /* Strip off leading 'rproc' command argument */
> + argc--;
> + argv++;
> +
> + if (argc)
> + c = find_cmd_tbl(argv[0], cmd_remoteproc_sub,
> + ARRAY_SIZE(cmd_remoteproc_sub));
> + if (c)
> + return c->cmd(cmdtp, flag, argc, argv);
> +
> + return CMD_RET_USAGE;
> +}
> +
> +U_BOOT_CMD(rproc, 5, 1, do_remoteproc,
> + "Control operation of remote processors in an SoC",
> + " [init|list|load|start|stop|reset|ping]\n"
> + "\t\t Where:\n"
> + "\t\t[addr] is a memory address\n"
> + "\t\t<id> is a numerical identifier for the remote processor\n"
> + "\t\t provided by 'list' command.\n"
> + "\t\tNote: Remote processors must be initalized prior to usage\n"
> + "\t\tNote: Services are dependent on the driver capability\n"
> + "\t\t 'list' command shows the capability of each device\n"
> + "\n\tSubcommands:\n"
> + "\tinit - Enumerate and initalize the remote processors\n"
> + "\tlist - list available remote processors\n"
> + "\tload <id> [addr] [size]- Load the remote processor with binary\n"
> + "\t image stored at address [addr] in memory\n"
> + "\tstart <id> - Start the remote processor(must be loaded)\n"
> + "\tstop <id> - Stop the remote processor\n"
> + "\treset <id> - Reset the remote processor\n"
> + "\tis_running <id> - Reports if the remote processor is running\n"
> + "\tping <id> - Ping the remote processor for communication\n");
> diff --git a/doc/device-tree-bindings/remoteproc/remoteproc.txt b/doc/device-tree-bindings/remoteproc/remoteproc.txt
> new file mode 100644
> index 000000000000..1a98a2e3a03c
> --- /dev/null
> +++ b/doc/device-tree-bindings/remoteproc/remoteproc.txt
> @@ -0,0 +1,14 @@
> +Remote Processor uClass
uclass
> +
> +Binding:
> +
> +Remoteproc devices shall have compatible corresponding to thier
> +drivers. However the following generic properties will be supported
> +
> +Optional Properties:
> +- remoteproc-name: a string, used if provided to describe the processor.
> + This must be unique in an operational system.
> +- remoteproc-internal-memory-mapped: a bool, indicates that the remote
> + processor has internal memory that it uses to execute code and store
> + data. Such a device is not expected to have a MMU. If no type property
> + is provided, the device is assumed to map to such a model.
Perhaps you could also specific a fallback compatible string so that
it is possible to have both that and the specific string. Then it is
easy to see what type this device is.
Also does this correspond to any existing device tree binding in (e.g.) Linux?
> diff --git a/doc/driver-model/remoteproc-framework.txt b/doc/driver-model/remoteproc-framework.txt
> new file mode 100644
> index 000000000000..32cb40242e62
> --- /dev/null
> +++ b/doc/driver-model/remoteproc-framework.txt
> @@ -0,0 +1,163 @@
> +#
> +# (C) Copyright 2015
> +# Texas Instruments Incorporated - http://www.ti.com/
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +
> +Remote Processor Framework
> +==========================
> +TOC:
> +1. Introduction
> +2. How does it work - The driver
> +3. Describing the device using platform data
> +4. Describing the device using device tree
> +
> +1. Introduction
> +===============
> +
> +This is an introduction to driver-model for Remote Processors found
> +on various System on Chip(SoCs). The term remote processor is used to
> +indicate that this is not the processor on which u-boot is operating
U-Boot
> +on, instead is yet another processing entity that may be controlled by
> +the processor on which we are functional.
> +
> +The simplified model depends on a single UCLASS - UCLASS_RPROC
> +
> +UCLASS_RPROC:
> +- drivers/remoteproc/rproc-uclass.c
> +- include/remoteproc.h
> +
> +Commands:
> +- common/cmd_remoteproc.c
> +
> +Configuration:
> +CONFIG_DM_REMOTEPROC is selected by drivers as needed
> +CONFIG_CMD_REMOTEPROC for the commands if required.
> +
> +2. How does it work - The driver
> +=================================
> +
> +Overall, the driver statemachine transitions are typically as follows:
> + (entry)
> + +-------+
> + +---+ init |
> + | | | <---------------------+
> + | +-------+ |
> + | |
> + | |
> + | +--------+ |
> +Load| | reset | |
> + | | | <----------+ |
> + | +--------+ | |
> + | |Load | |
> + | | | |
> + | +----v----+ reset | |
> + +-> | | (opt) | |
> + | Loaded +-----------+ |
> + | | |
> + +----+----+ |
> + | Start |
> + +---v-----+ (opt) |
> + +->| Running | Stop |
> +Ping +- | +--------------------+
> +(opt) +---------+
> +
> +(is_running does not change state)
> +opt: Optional state transition implemented by driver.
> +
> +NOTE: It depends on the remote processor as to the exact behavior
> +of the statemachine, remoteproc core does not intent to implement
> +statemachine logic. Certain processors may allow start/stop without
> +reloading the image in the middle, certain other processors may only
> +allow us to start the processor(image from a EEPROM/OTP) etc.
> +
> +It is hence the responsibility of the driver to handle the requisite
> +state transitions of the device as necessary.
> +
> +Basic design assumptions:
> +
> +Remote processor can operate on a certain firmware that maybe loaded
> +and released from reset.
> +
> +The driver follows a standard UCLASS DM.
> +
> +in the bare minimum form:
> +
> +static const struct dm_rproc_ops sandbox_testproc_ops = {
> + .load = sandbox_testproc_load,
> + .start = sandbox_testproc_start,
> +};
> +
> +static const struct udevice_id sandbox_ids[] = {
> + {.compatible = "sandbox,test-processor"},
> + {}
> +};
> +
> +U_BOOT_DRIVER(sandbox_testproc) = {
> + .name = "sandbox_test_proc",
> + .of_match = sandbox_ids,
> + .id = UCLASS_RPROC,
> + .ops = &sandbox_testproc_ops,
> + .probe = sandbox_testproc_probe,
> +};
> +
> +This allows for the device to be probed as part of the "init" command
> +or invocation of 'rproc_init()' function as the system dependencies define.
> +
> +The driver is expected to maintain it's own statemachine which is
> +appropriate for the device it maintains. It must, at the very least
> +provide a load and start function. We assume here that the device
> +needs to be loaded and started, else, there is no real purpose of
> +using the remoteproc framework.
> +
> +3. Describing the device using platform data
> +============================================
> +
> +Considering that many platforms are yet to move to device-tree model,
> +a simplified definition of a device is as follows:
> +
> +struct dm_rproc_uclass_pdata proc_3_test = {
> + .name = "proc_3_legacy",
> + .mem_type = RPROC_INTERNAL_MEMORY_MAPPED,
> + .driver_plat_data = &mydriver_data;
> +};
> +
> +U_BOOT_DEVICE(proc_3_demo) = {
> + .name = "sandbox_test_proc",
> + .platdata = &proc_3_test,
> +};
> +
> +There can be additional data that may be desired depending on the
> +remoteproc driver specific needs (for example: SoC integration
> +details such as clock handle or something similar). See appropriate
> +documentation for specific remoteproc driver for further details.
> +These are passed via driver_plat_data.
> +
> +3. Describing the device using device tree
> +==========================================
> +/ {
> + ...
> + aliases {
> + ...
> + remoteproc0 = &rproc_1;
> + remoteproc1 = &rproc_2;
> +
> + };
> + ...
> +
> + rproc_1: rproc at 1 {
> + compatible = "sandbox,test-processor";
> + remoteproc-name = "remoteproc-test-dev1";
> + };
> +
> + rproc_2: rproc at 2 {
> + compatible = "sandbox,test-processor";
> + internal-memory-mapped;
> + remoteproc-name = "remoteproc-test-dev2";
> + };
> + ...
> +};
> +
> +aliases usage is optional, but it is usually recommended to ensure the
> +users have a consistent usage model for a platform.
> +the compatible string used here is specific to the remoteproc driver involved.
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 092bc02b304e..24bd51269602 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -60,6 +60,8 @@ source "drivers/crypto/Kconfig"
>
> source "drivers/thermal/Kconfig"
>
> +source "drivers/remoteproc/Kconfig"
Please sort these, so remoteproc should go up above thermal.
> +
> endmenu
>
> config PHYS_TO_BUS
> diff --git a/drivers/Makefile b/drivers/Makefile
> index a721ec86dfef..7c18113ce133 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -59,5 +59,6 @@ obj-y += input/
> # SOC specific infrastructure drivers.
> obj-y += soc/
> obj-y += thermal/
> +obj-$(CONFIG_DM_REMOTEPROC) += remoteproc/
>
> endif
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> new file mode 100644
> index 000000000000..700f52caf1dc
> --- /dev/null
> +++ b/drivers/remoteproc/Kconfig
> @@ -0,0 +1,15 @@
> +#
> +# (C) Copyright 2015
> +# Texas Instruments Incorporated - http://www.ti.com/
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +
> +menu "Remote Processor drivers"
> +
> +# DM_REMOTEPROC gets selected by drivers as needed
> +# All users should depend on DM
> +config DM_REMOTEPROC
Can we use USE REMOTEPROC? The DM_ prefix indicates that driver model
is an optional feature for that subsystem, and when everything is
converted we intend to remove all the DM_<subsystem> options.
> + bool
> + depends on DM
> +
> +endmenu
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> new file mode 100644
> index 000000000000..af90ace95899
> --- /dev/null
> +++ b/drivers/remoteproc/Makefile
> @@ -0,0 +1,7 @@
> +#
> +# (C) Copyright 2015
> +# Texas Instruments Incorporated - http://www.ti.com/
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +
> +obj-$(CONFIG_DM_REMOTEPROC) += rproc-uclass.o
> diff --git a/drivers/remoteproc/rproc-uclass.c b/drivers/remoteproc/rproc-uclass.c
> new file mode 100644
> index 000000000000..cafc293f78f3
> --- /dev/null
> +++ b/drivers/remoteproc/rproc-uclass.c
> @@ -0,0 +1,406 @@
> +/*
> + * (C) Copyright 2015
> + * Texas Instruments Incorporated - http://www.ti.com/
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +#define pr_fmt(fmt) "%s: " fmt, __func__
> +#include <common.h>
> +#include <dm.h>
> +#include <dm/uclass.h>
> +#include <dm/uclass-internal.h>
> +#include <dm/device-internal.h>
> +#include <errno.h>
> +#include <fdtdec.h>
> +#include <malloc.h>
> +#include <asm/io.h>
> +#include <remoteproc.h>
This is the ordering I'm keen on:
> +#include <common.h>
> +#include <dm.h>
> +#include <errno.h>
> +#include <fdtdec.h>
> +#include <malloc.h>
> +#include <remoteproc.h>
> +#include <asm/io.h>
> +#include <dm/uclass.h>
> +#include <dm/uclass-internal.h>
> +#include <dm/device-internal.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
function comment please
> +static int for_each_remoteproc_device(int (*fn) (struct udevice *dev,
> + struct dm_rproc_uclass_pdata *uc_pdata,
> + const void *data),
> + struct udevice *skip_dev,
> + const void *data)
> +{
> + struct udevice *dev;
> + struct dm_rproc_uclass_pdata *uc_pdata;
> + int ret;
> +
> + for (ret = uclass_find_first_device(UCLASS_RPROC, &dev); dev;
> + ret = uclass_find_next_device(&dev)) {
> + if (ret || dev == skip_dev)
> + continue;
> + uc_pdata = dev_get_uclass_platdata(dev);
> + ret = fn(dev, uc_pdata, data);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
function comment
> +static int _rproc_name_is_unique(struct udevice *dev,
> + struct dm_rproc_uclass_pdata *uc_pdata,
> + const void *data)
> +{
> + const char *check_name = data;
> +
> + /* devices not yet populated with data - so skip them */
> + if (!uc_pdata->name && check_name)
> + return 0;
> +
> + /* Return 0 to search further if we dont match */
> + if (strlen(uc_pdata->name) != strlen(check_name))
> + return 0;
> +
> + if (!strcmp(uc_pdata->name, check_name))
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static bool rproc_name_is_unique(struct udevice *check_dev,
> + const char *check_name)
> +{
> + int ret;
> +
> + ret = for_each_remoteproc_device(_rproc_name_is_unique,
> + check_dev, check_name);
> + return ret ? false : true;
> +}
> +
> +static struct udevice *rproc_find_dev_for_id(int id)
> +{
> + struct udevice *dev;
> + int ret;
> +
> + for (ret = uclass_find_first_device(UCLASS_RPROC, &dev); dev;
> + ret = uclass_find_next_device(&dev)) {
> + if (ret)
> + continue;
> + if (dev->seq == id)
> + return dev;
> + }
Can you not use uclass_get_device_by_seq()?
> +
> + return NULL;
> +}
> +
> +static int rproc_pre_probe(struct udevice *dev)
> +{
> + struct dm_rproc_uclass_pdata *uc_pdata;
> + const struct dm_rproc_ops *ops;
> +
> + uc_pdata = dev_get_uclass_platdata(dev);
> + if (!uc_pdata) {
> + debug("%s: no uclass_platdata?\n", dev->name);
> + return -ENXIO;
> + }
No need - it cannot happen.
> +
> + /* See if we need to populate via fdt */
> +
> + if (!dev->platdata) {
> +#ifdef CONFIG_OF_CONTROL
> + int node = dev->of_offset;
> + const void *blob = gd->fdt_blob;
> + bool tmp;
> + if (!blob) {
> + debug("'%s' no dt?\n", dev->name);
> + return -EINVAL;
> + }
> + debug("'%s': using fdt\n", dev->name);
> + uc_pdata->name = fdt_getprop(blob, node,
> + "remoteproc-name", NULL);
> +
> + /* Default is internal memory mapped */
> + uc_pdata->mem_type = RPROC_INTERNAL_MEMORY_MAPPED;
> + tmp = fdtdec_get_bool(blob, node,
> + "remoteproc-internal-memory-mapped");
> + if (tmp)
> + uc_pdata->mem_type = RPROC_INTERNAL_MEMORY_MAPPED;
> +#else
> + /* Nothing much we can do about this, can we? */
> + return -EINVAL;
> +#endif
> +
> + } else {
> + struct dm_rproc_uclass_pdata *pdata = dev->platdata;
> +
> + debug("'%s': using legacy data\n", dev->name);
> + if (pdata->name)
> + uc_pdata->name = pdata->name;
> + uc_pdata->mem_type = pdata->mem_type;
> + uc_pdata->driver_plat_data = pdata->driver_plat_data;
> + }
> +
> + /* Else try using device Name */
> + if (!uc_pdata->name)
> + uc_pdata->name = dev->name;
> + if (!uc_pdata->name) {
> + debug("Unnamed device!");
> + return -EINVAL;
> + }
> +
> + if (!rproc_name_is_unique(dev, uc_pdata->name)) {
> + debug("%s duplicate name '%s'\n", dev->name, uc_pdata->name);
> + return -EINVAL;
> + }
> +
> + ops = device_get_ops(dev);
> + if (!ops) {
> + debug("%s driver has no ops?\n", dev->name);
> + return -EINVAL;
> + }
> +
> + if (!ops->load || !ops->start) {
> + debug("%s driver has missing mandatory ops?\n", dev->name);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int rproc_post_probe(struct udevice *dev)
> +{
> + struct dm_rproc_uclass_pdata *uc_pdata;
> + const struct dm_rproc_ops *ops;
> +
> + uc_pdata = dev_get_uclass_platdata(dev);
> + if (!uc_pdata) {
> + debug("%s: no uclass_platdata?\n", dev->name);
> + return -ENXIO;
> + }
Cannot happen.
> +
> + ops = device_get_ops(dev);
> + if (!ops) {
> + debug("%s driver has no ops?\n", dev->name);
> + return -EINVAL;
> + }
> +
> + if (ops->init)
> + return ops->init(dev);
> +
> + return 0;
> +}
> +
> +UCLASS_DRIVER(rproc) = {
> + /* *INDENT-OFF* */
What is that? ^^
> + .id = UCLASS_RPROC,
> + .name = "remoteproc",
> + .flags = DM_UC_FLAG_SEQ_ALIAS,
> + .pre_probe = rproc_pre_probe,
> + .post_probe = rproc_post_probe,
> + .per_device_platdata_auto_alloc_size =
> + sizeof(struct dm_rproc_uclass_pdata),
> + /* *INDENT-ON* */
> +};
> +
> +/* exported functions */
But this is not exported is it?
> +static int _rproc_probe_dev(struct udevice *dev,
> + struct dm_rproc_uclass_pdata *uc_pdata,
> + const void *data)
> +{
> + int ret;
> +
> + ret = device_probe(dev);
> +
> + if (ret)
> + printf("%s: Failed to initialize - %d\n", dev->name, ret);
> + return ret;
> +}
> +
> +static bool _rproc_init_called;
Can you instead check each device individually and drop this flag? In
general I would like drivers to avoid declaring any static data.
> +
> +/**
> + * rproc_init() - Initialize all bound remote proc devices
> + */
> +int rproc_init(void)
> +{
> + int ret;
> +
> + if (_rproc_init_called) {
> + debug("Already initialized\n");
> + return -EINVAL;
> + }
> +
> + ret = for_each_remoteproc_device(_rproc_probe_dev, NULL, NULL);
> + if (!ret)
> + _rproc_init_called = true;
> + return ret;
> +}
> +
> +/**
> + * rproc_is_initialized() - check to see if remoteproc devices are initialized
> + */
> +int rproc_is_initialized(void)
> +{
> + return _rproc_init_called;
> +}
> +
> +/**
> + * rproc_load() - load binary to a remote processor
> + * @id: id of the remote processor
> + * @addr: address in memory where the binary image is located
> + * @size: size of the binary image
> + */
> +int rproc_load(int id, ulong addr, ulong size)
> +{
> + struct udevice *dev = rproc_find_dev_for_id(id);
> + struct dm_rproc_uclass_pdata *uc_pdata;
> + const struct dm_rproc_ops *ops;
> +
> + if (!dev) {
> + printf("Unknown remote processor id '%d' requested\n", id);
debug()? We should not print out messages in drivers
> + return -EINVAL;
> + }
> +
> + uc_pdata = dev_get_uclass_platdata(dev);
> + if (!uc_pdata) {
> + debug("%s: no uclass_platdata?\n", dev->name);
> + return -ENXIO;
> + }
Can't happen.
> +
> + ops = device_get_ops(dev);
> + if (!ops) {
> + debug("%s driver has no ops?\n", dev->name);
> + return -EINVAL;
> + }
> +
> + printf("Loading to '%s' from address 0x%08lX size of %lu bytes\n",
> + uc_pdata->name, addr, size);
> + if (ops->load)
> + return ops->load(dev, addr, size);
> +
> + debug("%s: data corruption?? mandatory function is missing!\n",
> + dev->name);
> +
> + return -EINVAL;
-ENOSYS, which means that a method is missing.
> +};
> +
> +/*
> + * Completely internal helper enums..
> + * Keeping this isolated helps this code evolve independent of other
> + * parts..
> + */
> +enum rproc_ops {
> + RPROC_START,
> + RPROC_STOP,
> + RPROC_RESET,
> + RPROC_PING,
> + RPROC_RUNNING,
> +};
> +
> +static int _rproc_wrapper(int id, enum rproc_ops op)
> +{
> + struct udevice *dev = rproc_find_dev_for_id(id);
> + struct dm_rproc_uclass_pdata *uc_pdata;
> + const struct dm_rproc_ops *ops;
> + int (*fn)(struct udevice *dev);
> + bool mandatory = false;
> + char *op_str;
> +
> + if (!dev) {
> + printf("Unknown remote processor id '%d' requested\n", id);
> + return -EINVAL;
> + }
> +
> + uc_pdata = dev_get_uclass_platdata(dev);
> + if (!uc_pdata) {
> + debug("%s: no uclass_platdata?\n", dev->name);
> + return -ENXIO;
> + }
Can't happen
> +
> + ops = device_get_ops(dev);
> + if (!ops) {
> + debug("%s driver has no ops?\n", dev->name);
> + return -EINVAL;
> + }
> + switch (op) {
> + case RPROC_START:
> + fn = ops->start;
> + mandatory = true;
> + op_str = "Starting";
> + break;
> + case RPROC_STOP:
> + fn = ops->stop;
> + op_str = "Stopping";
> + break;
> + case RPROC_RESET:
> + fn = ops->reset;
> + op_str = "Resetting";
> + break;
> + case RPROC_RUNNING:
> + fn = ops->is_running;
> + op_str = "Checking if running:";
> + break;
> + case RPROC_PING:
> + fn = ops->ping;
> + op_str = "Pinging";
> + break;
> + default:
> + debug("what is '%d' operation??\n", op);
> + return -EINVAL;
> + }
> +
> + printf("%s %s...\n", op_str, uc_pdata->name);
debug()? This is a driver.
> + if (fn)
> + return fn(dev);
> +
> + if (mandatory)
> + debug("%s: data corruption?? mandatory function is missing!\n",
> + dev->name);
> +
> + return -EINVAL;
> +}
> +
> +/**
> + * rproc_start() - Start a remote processor
> + * @id: id of the remote processor
Please document the return value in these functions
> + */
> +int rproc_start(int id)
> +{
> + return _rproc_wrapper(id, RPROC_START);
> +};
> +
> +/**
> + * rproc_stop() - Stop a remote processor
> + * @id: id of the remote processor
> + */
> +int rproc_stop(int id)
> +{
> + return _rproc_wrapper(id, RPROC_STOP);
> +};
> +
> +/**
> + * rproc_reset() - reset a remote processor
> + * @id: id of the remote processor
> + */
> +int rproc_reset(int id)
> +{
> + return _rproc_wrapper(id, RPROC_RESET);
> +};
> +
> +/**
> + * rproc_ping() - ping a remote processor to check if it can communicate
> + * @id: id of the remote processor
> + *
> + * NOTE: this might need communication path available, which is not implemented
> + * as part of remoteproc framework - hook on to appropriate bus architecture to
> + * do the same
> + */
> +int rproc_ping(int id)
> +{
> + return _rproc_wrapper(id, RPROC_PING);
> +};
> +
> +/**
> + * rproc_is_running() - check to see if remote processor is running
> + * @id: id of the remote processor
> + *
> + * NOTE: this may not involve actual communication capability of the remote
> + * processor, but just ensures that it is out of reset and executing code.
> + */
> +int rproc_is_running(int id)
> +{
> + return _rproc_wrapper(id, RPROC_RUNNING);
> +};
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index c744044bb8aa..a2958c234db4 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -47,6 +47,7 @@ enum uclass_id {
> UCLASS_PMIC, /* PMIC I/O device */
> UCLASS_REGULATOR, /* Regulator device */
> UCLASS_RESET, /* Reset device */
> + UCLASS_RPROC, /* Remote Processor device */
Maybe UCLASS_REMOTEPROC would be better to be consistent and more descriptive?
> UCLASS_RTC, /* Real time clock device */
> UCLASS_SERIAL, /* Serial UART */
> UCLASS_SPI, /* SPI bus */
> diff --git a/include/remoteproc.h b/include/remoteproc.h
> new file mode 100644
> index 000000000000..b92d40e0ca2e
> --- /dev/null
> +++ b/include/remoteproc.h
> @@ -0,0 +1,81 @@
> +/*
> + * (C) Copyright 2015
> + * Texas Instruments Incorporated - http://www.ti.com/
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#ifndef _RPROC_H_
> +#define _RPROC_H_
> +
> +#include <dm/platdata.h> /* For platform data support - non dt world */
Does this need to be supported for a new uclass?
> +
> +/**
> + * enum rproc_mem_type - What type of memory model does the rproc use
> + * @RPROC_INTERNAL: Remote processor uses own memory and is memory mapped
> + * to the host processor over an address range.
> + *
> + * Please note that this is an enumeration of memory model of different types
> + * of remote processors. Few of the remote processors do have own internal
> + * memories, while others use external memory for instruction and data.
> + */
> +enum rproc_mem_type {
> + RPROC_INTERNAL_MEMORY_MAPPED = 0,
> +};
> +
> +/**
> + * struct dm_rproc_uclass_pdata - platform data for a CPU
> + *
> + * This can be accessed with dev_get_platdata() for any UCLASS_RPROC
> + * device.
> + *
> + * @name: Platform-specific way of naming the Remote proc
> + * @mem_type: one of 'enum rproc_mem_type'
> + * @driver_data: driver specific platform data that may be needed.
The comment names do not match the struct.
> + */
> +struct dm_rproc_uclass_pdata {
> + const char *name;
> + enum rproc_mem_type mem_type;
> + void *driver_plat_data;
> +};
> +
> +/**
> + * struct dm_rproc_ops - Operations that are provided by remote proc driver
> + * @init: Initialize the remoteproc device invoked after probe (optional)
> + * @load: Load the remoteproc device using data provided(mandatory)
> + * @start: Start the remoteproc device (mandatory)
> + * @stop: Stop the remoteproc device (optional)
> + * @reset: Reset the remote proc device (optional)
> + * @is_running: Check if the remote processor is running(optional)
> + * @ping: Ping the remote device for basic communication check(optional)
You should document the return value (0 for success, -ve on error?).
For is_running(), what return value means what?
> + */
> +struct dm_rproc_ops {
> + int (*init)(struct udevice *dev);
> + int (*load)(struct udevice *dev, ulong addr, ulong size);
document args
> + int (*start)(struct udevice *dev);
> + int (*stop)(struct udevice *dev);
> + int (*reset)(struct udevice *dev);
> + int (*is_running)(struct udevice *dev);
> + int (*ping)(struct udevice *dev);
> +};
> +
> +#ifdef CONFIG_DM_REMOTEPROC
> +int rproc_init(void);
> +int rproc_is_initialized(void);
> +int rproc_load(int id, ulong addr, ulong size);
> +int rproc_start(int id);
> +int rproc_stop(int id);
> +int rproc_reset(int id);
> +int rproc_ping(int id);
> +int rproc_is_running(int id);
> +#else
> +static inline int rproc_init(void) { return -EINVAL; }
-ENOSYS
> +static inline int rproc_is_initialized(void) { return false; }
> +static inline int rproc_load(int id, ulong addr, ulong size) { return -EINVAL; }
> +static inline int rproc_start(int id) { return -EINVAL; }
> +static inline int rproc_stop(int id) { return -EINVAL; }
> +static inline int rproc_reset(int id) { return -EINVAL; }
> +static inline int rproc_ping(int id) { return -EINVAL; }
> +static inline int rproc_is_running(int id) { return -EINVAL; }
> +#endif
> +
> +#endif /* _RPROC_H_ */
> --
> 2.1.4
>
Regards,
Simon
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/3] remoteproc: Introduce a sandbox dummy driver
2015-08-24 17:28 ` [U-Boot] [PATCH 2/3] remoteproc: Introduce a sandbox dummy driver Nishanth Menon
@ 2015-08-25 5:04 ` Simon Glass
2015-08-25 15:47 ` Nishanth Menon
0 siblings, 1 reply; 15+ messages in thread
From: Simon Glass @ 2015-08-25 5:04 UTC (permalink / raw)
To: u-boot
Hi Nishanth,
On 24 August 2015 at 11:28, Nishanth Menon <nm@ti.com> wrote:
> Introduce a dummy driver for sandbox that allows us to verify basic
> functionality. This is not meant to do anything functional - but is
> more or less meant as a framework plumbing debug helper.
>
> The sandbox remoteproc driver maintains absolutey no states and is a
> simple driver which just is filled with empty hooks. Idea being to give
> an approximate idea to implement own remoteproc driver using this as a
> template.
>
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
> drivers/remoteproc/Kconfig | 9 ++
> drivers/remoteproc/Makefile | 3 +
> drivers/remoteproc/sandbox_testproc.c | 243 ++++++++++++++++++++++++++++++++++
> 3 files changed, 255 insertions(+)
> create mode 100644 drivers/remoteproc/sandbox_testproc.c
>
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 700f52caf1dc..61cc506d6baa 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -12,4 +12,13 @@ config DM_REMOTEPROC
> bool
> depends on DM
>
> +# Please keep the configuration alphabetically sorted.
> +config DM_TESTPROC_SANDBOX
Should this be REMOTEPROC_SANDBOX?
> + bool "Support for Test processor for Sandbox"
> + select DM_REMOTEPROC
> + depends on DM
> + depends on SANDBOX
> + help
> + Say 'y' here to add support for test processor which does dummy
> + operations for sandbox platform.
> endmenu
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index af90ace95899..57c04d0e024a 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -5,3 +5,6 @@
> #
>
> obj-$(CONFIG_DM_REMOTEPROC) += rproc-uclass.o
> +
> +# Remote proc drivers - Please keep this list alphabetically sorted.
> +obj-$(CONFIG_DM_TESTPROC_SANDBOX) += sandbox_testproc.o
> diff --git a/drivers/remoteproc/sandbox_testproc.c b/drivers/remoteproc/sandbox_testproc.c
> new file mode 100644
> index 000000000000..e942f66fc2dc
> --- /dev/null
> +++ b/drivers/remoteproc/sandbox_testproc.c
> @@ -0,0 +1,243 @@
> +/*
> + * (C) Copyright 2015
> + * Texas Instruments Incorporated - http://www.ti.com/
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +#define pr_fmt(fmt) "%s: " fmt, __func__
> +#include <common.h>
> +#include <dm.h>
> +#include <errno.h>
> +#include <remoteproc.h>
> +
> +enum sandbox_state {
> + sb_entry,
> + sb_init,
> + sb_reset,
> + sb_loaded,
> + sb_running
> +};
> +
> +struct sandbox_test_devdata {
> + enum sandbox_state current_state;
> +};
> +
> +static int sandbox_dev_move_to_state(struct udevice *dev,
> + enum sandbox_state next_state)
> +{
> + struct sandbox_test_devdata *ddata = dev_get_priv(dev);
> +
> + /* No state transition is OK */
> + if (ddata->current_state == next_state)
> + return 0;
> +
> + debug("current_state=%d, next_state=%d\n", ddata->current_state, next_state);
> + switch (ddata->current_state) {
> + case sb_entry:
> + if (next_state == sb_init)
> + goto ok_state;
> + break;
> +
> + case sb_init:
> + if (next_state == sb_reset || next_state == sb_loaded)
> + goto ok_state;
> + break;
> +
> + case sb_reset:
> + if (next_state == sb_loaded || next_state == sb_init)
> + goto ok_state;
> + break;
> +
> + case sb_loaded:
> + if (next_state == sb_reset || next_state == sb_init ||
> + next_state == sb_running)
> + goto ok_state;
> + break;
> +
> + case sb_running:
> + if (next_state == sb_reset || next_state == sb_init)
> + goto ok_state;
> + break;
> + };
> + return -EINVAL;
> +
> +ok_state:
> + ddata->current_state = next_state;
> + return 0;
> +}
> +
> +static int sandbox_testproc_probe(struct udevice *dev)
> +{
> + struct dm_rproc_uclass_pdata *uc_pdata;
> + struct sandbox_test_devdata *ddata;
> + int ret;
> +
> + ddata = dev_get_priv(dev);
> + uc_pdata = dev_get_uclass_platdata(dev);
> + if (!uc_pdata || !ddata) {
> + printf("%s: platform data %p OR driver data %p missing\n",
> + dev->name, uc_pdata, ddata);
> + return -EINVAL;
> + }
> + ret = sandbox_dev_move_to_state(dev, sb_entry);
> +
> + debug("%s: called(%d)\n", uc_pdata->name, ret);
> +
> + return ret;
> +}
> +
> +static int sandbox_testproc_init(struct udevice *dev)
> +{
> + struct dm_rproc_uclass_pdata *uc_pdata;
> + int ret;
> +
> + uc_pdata = dev_get_uclass_platdata(dev);
> +
> + ret = sandbox_dev_move_to_state(dev, sb_init);
> +
> + debug("%s: called(%d)\n", uc_pdata->name, ret);
> + if (ret)
> + printf("%s init failed\n", uc_pdata->name);
> +
> + return ret;
> +}
> +
> +static int sandbox_testproc_reset(struct udevice *dev)
> +{
> + struct dm_rproc_uclass_pdata *uc_pdata;
> + int ret;
> +
> + uc_pdata = dev_get_uclass_platdata(dev);
> +
> + ret = sandbox_dev_move_to_state(dev, sb_reset);
> +
> + debug("%s: called(%d)\n", uc_pdata->name, ret);
> +
> + if (ret)
> + printf("%s reset failed\n", uc_pdata->name);
> + return ret;
> +}
> +
> +static int sandbox_testproc_load(struct udevice *dev, ulong addr, ulong size)
> +{
> + struct dm_rproc_uclass_pdata *uc_pdata;
> + int ret;
> +
> + uc_pdata = dev_get_uclass_platdata(dev);
> +
> + ret = sandbox_dev_move_to_state(dev, sb_loaded);
> +
> + debug("%s: called(%d) Loading to %08lX %lu size\n",
> + uc_pdata->name, ret, addr, size);
> +
> + if (ret)
> + printf("%s load failed\n", uc_pdata->name);
> + return ret;
> +}
> +
> +static int sandbox_testproc_start(struct udevice *dev)
> +{
> + struct dm_rproc_uclass_pdata *uc_pdata;
> + int ret;
> +
> + uc_pdata = dev_get_uclass_platdata(dev);
> +
> + ret = sandbox_dev_move_to_state(dev, sb_running);
> +
> + debug("%s: called(%d)\n", uc_pdata->name, ret);
> +
> + if (ret)
> + printf("%s start failed\n", uc_pdata->name);
> + return ret;
> +}
> +
> +static int sandbox_testproc_stop(struct udevice *dev)
> +{
> + struct dm_rproc_uclass_pdata *uc_pdata;
> + int ret;
> +
> + uc_pdata = dev_get_uclass_platdata(dev);
> +
> + ret = sandbox_dev_move_to_state(dev, sb_init);
> +
> + debug("%s: called(%d)\n", uc_pdata->name, ret);
> +
> + if (ret)
> + printf("%s stop failed\n", uc_pdata->name);
> + return ret;
> +}
> +
> +static int sandbox_testproc_is_running(struct udevice *dev)
> +{
> + struct dm_rproc_uclass_pdata *uc_pdata;
> + struct sandbox_test_devdata *ddata;
> + int ret = 1;
> +
> + uc_pdata = dev_get_uclass_platdata(dev);
> + ddata = dev_get_priv(dev);
> +
> + if (ddata->current_state == sb_running)
> + ret = 0;
> + debug("%s: called(%d)\n", uc_pdata->name, ret);
should that say "is_running called" / do you need __func__?
> +
> + return ret;
> +}
> +
> +static int sandbox_testproc_ping(struct udevice *dev)
> +{
> + struct dm_rproc_uclass_pdata *uc_pdata;
> + struct sandbox_test_devdata *ddata;
> + int ret = 1;
> +
> + uc_pdata = dev_get_uclass_platdata(dev);
> + ddata = dev_get_priv(dev);
> +
> + if (ddata->current_state == sb_running)
> + ret = 0;
> + else
> + ret = -EINVAL;
> +
> + debug("%s: called(%d)\n", uc_pdata->name, ret);
> + if (ret) {
> + printf("%s: No response.(Not started?)\n", uc_pdata->name);
> + }
> +
> + return ret;
> +}
> +
> +static const struct dm_rproc_ops sandbox_testproc_ops = {
> + .init = sandbox_testproc_init,
> + .reset = sandbox_testproc_reset,
> + .load = sandbox_testproc_load,
> + .start = sandbox_testproc_start,
> + .stop = sandbox_testproc_stop,
> + .is_running = sandbox_testproc_is_running,
> + .ping = sandbox_testproc_ping,
> +};
> +
> +static const struct udevice_id sandbox_ids[] = {
> + {.compatible = "sandbox,test-processor"},
> + {}
> +};
> +
> +U_BOOT_DRIVER(sandbox_testproc) = {
> + /* *INDENT-OFF* */
What is that for? ^^
> + .name = "sandbox_test_proc",
> + .of_match = sandbox_ids,
> + .id = UCLASS_RPROC,
> + .ops = &sandbox_testproc_ops,
> + .probe = sandbox_testproc_probe,
> + .priv_auto_alloc_size = sizeof(struct sandbox_test_devdata),
> + /* *INDENT-ON* */
> +};
> +
> +static struct dm_rproc_uclass_pdata proc_3_test = {
> + .name = "proc_3_legacy",
> + .mem_type = RPROC_INTERNAL_MEMORY_MAPPED,
> +};
> +
> +U_BOOT_DEVICE(proc_3_demo) = {
> + /* *INDENT-OFF* */
> + .name = "sandbox_test_proc",
> + .platdata = &proc_3_test,
> + /* *INDENT-ON* */
> +};
> --
> 2.1.4
>
Regards,
Simon
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 3/3] sandbox: Introduce dummy remoteproc nodes
2015-08-24 17:28 ` [U-Boot] [PATCH 3/3] sandbox: Introduce dummy remoteproc nodes Nishanth Menon
@ 2015-08-25 5:04 ` Simon Glass
2015-08-25 15:47 ` Nishanth Menon
0 siblings, 1 reply; 15+ messages in thread
From: Simon Glass @ 2015-08-25 5:04 UTC (permalink / raw)
To: u-boot
Hi Nishanth,
On 24 August 2015 at 11:28, Nishanth Menon <nm@ti.com> wrote:
> Introduce dummy devices for sandbox remoteproc device and enable it by
> default
>
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
> arch/sandbox/dts/test.dts | 13 +++++++++++++
> configs/sandbox_defconfig | 2 ++
> 2 files changed, 15 insertions(+)
>
> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> index c948df8c864b..df9b71310dbe 100644
> --- a/arch/sandbox/dts/test.dts
> +++ b/arch/sandbox/dts/test.dts
> @@ -25,6 +25,8 @@
> usb0 = &usb_0;
> usb1 = &usb_1;
> usb2 = &usb_2;
> + remoteproc1 = &rproc_1;
> + remoteproc2 = &rproc_2;
Can you please put these in alpha order?
> };
>
> a-test {
> @@ -296,6 +298,17 @@
> status = "disabled";
> };
>
> +
> + rproc_1: rproc at 1 {
> + compatible = "sandbox,test-processor";
> + remoteproc-name = "remoteproc-test-dev1";
> + };
> +
> + rproc_2: rproc at 2 {
> + compatible = "sandbox,test-processor";
> + internal-memory-mapped;
> + remoteproc-name = "remoteproc-test-dev2";
> + };
And these should go after reset {} I think.
> };
>
> #include "sandbox_pmic.dtsi"
> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> index 874a26b572aa..4a2750c154bc 100644
> --- a/configs/sandbox_defconfig
> +++ b/configs/sandbox_defconfig
> @@ -54,3 +54,5 @@ CONFIG_LED_GPIO=y
> CONFIG_SYSCON=y
> CONFIG_REGMAP=y
> CONFIG_DEVRES=y
> +CONFIG_DM_TESTPROC_SANDBOX=y
CONFIG_TESTPROC_SANDBOX
> +CONFIG_CMD_REMOTEPROC=y
> --
> 2.1.4
>
Regards,
Simon
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 1/3] drivers: Introduce a simplified remoteproc framework
2015-08-25 5:04 ` Simon Glass
@ 2015-08-25 15:44 ` Nishanth Menon
2015-08-26 2:26 ` Simon Glass
0 siblings, 1 reply; 15+ messages in thread
From: Nishanth Menon @ 2015-08-25 15:44 UTC (permalink / raw)
To: u-boot
On 08/25/2015 12:04 AM, Simon Glass wrote:
[...]
>> index 000000000000..e8fdb124e251
>> --- /dev/null
>> +++ b/common/cmd_remoteproc.c
>> @@ -0,0 +1,224 @@
>> +/*
>> + * (C) Copyright 2015
>> + * Texas Instruments Incorporated - http://www.ti.com/
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +#include <common.h>
>> +#include <command.h>
>> +#include <remoteproc.h>
>> +#include <dm.h>
>> +#include <errno.h>
>> +#include <malloc.h>
>> +
>
> nit: can you please sort those includes?
Yes - Sorry abotu that , I missed.
[...]
>> +static int print_remoteproc_list(void)
>> +{
>> + struct udevice *dev;
>> + struct uclass *uc;
>> + int ret;
>> + char *type;
>> +
>> + ret = uclass_get(UCLASS_RPROC, &uc);
>> + if (ret) {
>> + printf("Cannot find Remote processor class\n");
>> + return ret;
>> + }
>> +
>> + uclass_foreach_dev(dev, uc) {
>> + struct dm_rproc_uclass_pdata *uc_pdata;
>> + const struct dm_rproc_ops *ops = device_get_ops(dev);
>
> Can you create a rproc_get_ops() static inline as is done with other
> uclasses, and use that?
Will do. thanks on the hint.
>> +
>> + uc_pdata = dev_get_uclass_platdata(dev);
>> + if (!uc_pdata) {
>> + debug("%s: no uclass_platdata?\n", dev->name);
>> + return -ENXIO;
>> + }
>
> That can never happen so you can remove this check.
OK. will remove elsewhere as well.
>> diff --git a/doc/device-tree-bindings/remoteproc/remoteproc.txt b/doc/device-tree-bindings/remoteproc/remoteproc.txt
>> new file mode 100644
>> index 000000000000..1a98a2e3a03c
>> --- /dev/null
>> +++ b/doc/device-tree-bindings/remoteproc/remoteproc.txt
>> @@ -0,0 +1,14 @@
>> +Remote Processor uClass
>
> uclass
Thanks. will do.
>
>> +
>> +Binding:
>> +
>> +Remoteproc devices shall have compatible corresponding to thier
>> +drivers. However the following generic properties will be supported
>> +
>> +Optional Properties:
>> +- remoteproc-name: a string, used if provided to describe the processor.
>> + This must be unique in an operational system.
>> +- remoteproc-internal-memory-mapped: a bool, indicates that the remote
>> + processor has internal memory that it uses to execute code and store
>> + data. Such a device is not expected to have a MMU. If no type property
>> + is provided, the device is assumed to map to such a model.
>
> Perhaps you could also specific a fallback compatible string so that
> it is possible to have both that and the specific string. Then it is
> easy to see what type this device is.
That assumes a standard compatible is available for all devices -> but
with remoteproc devices, we cannot really do that, correct?.
>
> Also does this correspond to any existing device tree binding in (e.g.) Linux?
As of v4.2-rc8, only binding we have in upstream kernel is
Documentation/devicetree/bindings/remoteproc/wkup_m3_rproc.txt
which is not really helpful here.
>> diff --git a/doc/driver-model/remoteproc-framework.txt b/doc/driver-model/remoteproc-framework.txt
>> new file mode 100644
>> index 000000000000..32cb40242e62
>> --- /dev/null
>> +++ b/doc/driver-model/remoteproc-framework.txt
>> @@ -0,0 +1,163 @@
>> +#
>> +# (C) Copyright 2015
>> +# Texas Instruments Incorporated - http://www.ti.com/
>> +# SPDX-License-Identifier: GPL-2.0+
>> +#
>> +
>> +Remote Processor Framework
>> +==========================
>> +TOC:
>> +1. Introduction
>> +2. How does it work - The driver
>> +3. Describing the device using platform data
>> +4. Describing the device using device tree
>> +
>> +1. Introduction
>> +===============
>> +
>> +This is an introduction to driver-model for Remote Processors found
>> +on various System on Chip(SoCs). The term remote processor is used to
>> +indicate that this is not the processor on which u-boot is operating
>
> U-Boot
thanks.
[...]
>> index 092bc02b304e..24bd51269602 100644
>> --- a/drivers/Kconfig
>> +++ b/drivers/Kconfig
>> @@ -60,6 +60,8 @@ source "drivers/crypto/Kconfig"
>>
>> source "drivers/thermal/Kconfig"
>>
>> +source "drivers/remoteproc/Kconfig"
>
> Please sort these, so remoteproc should go up above thermal.
will do, thanks.
[..]
>> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
>> new file mode 100644
>> index 000000000000..700f52caf1dc
>> --- /dev/null
>> +++ b/drivers/remoteproc/Kconfig
>> @@ -0,0 +1,15 @@
>> +#
>> +# (C) Copyright 2015
>> +# Texas Instruments Incorporated - http://www.ti.com/
>> +# SPDX-License-Identifier: GPL-2.0+
>> +#
>> +
>> +menu "Remote Processor drivers"
>> +
>> +# DM_REMOTEPROC gets selected by drivers as needed
>> +# All users should depend on DM
>> +config DM_REMOTEPROC
>
> Can we use USE REMOTEPROC? The DM_ prefix indicates that driver model
> is an optional feature for that subsystem, and when everything is
> converted we intend to remove all the DM_<subsystem> options.
Aaaah... thanks for clarifying.. I had gotten confused on the naming.. i
had used without the "DM" prefix originally, will go back.
[...]
>> diff --git a/drivers/remoteproc/rproc-uclass.c b/drivers/remoteproc/rproc-uclass.c
>> new file mode 100644
>> index 000000000000..cafc293f78f3
>> --- /dev/null
>> +++ b/drivers/remoteproc/rproc-uclass.c
>> @@ -0,0 +1,406 @@
>> +/*
>> + * (C) Copyright 2015
>> + * Texas Instruments Incorporated - http://www.ti.com/
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +#define pr_fmt(fmt) "%s: " fmt, __func__
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <dm/uclass.h>
>> +#include <dm/uclass-internal.h>
>> +#include <dm/device-internal.h>
>> +#include <errno.h>
>> +#include <fdtdec.h>
>> +#include <malloc.h>
>> +#include <asm/io.h>
>> +#include <remoteproc.h>
>
> This is the ordering I'm keen on:
>
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <errno.h>
>> +#include <fdtdec.h>
>> +#include <malloc.h>
>> +#include <remoteproc.h>
>> +#include <asm/io.h>
>> +#include <dm/uclass.h>
>> +#include <dm/uclass-internal.h>
>> +#include <dm/device-internal.h>
alright. will do.
>
>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>
> function comment please
ok. will update all. I had stuck to providing doc for public functions
alone.. but it is better to document all of them.
[...]
>> +static struct udevice *rproc_find_dev_for_id(int id)
>> +{
>> + struct udevice *dev;
>> + int ret;
>> +
>> + for (ret = uclass_find_first_device(UCLASS_RPROC, &dev); dev;
>> + ret = uclass_find_next_device(&dev)) {
>> + if (ret)
>> + continue;
>> + if (dev->seq == id)
>> + return dev;
>> + }
>
> Can you not use uclass_get_device_by_seq()?
Arrghh.. ofcourse! Thanks.
>> +static int rproc_pre_probe(struct udevice *dev)
>> +{
>> + struct dm_rproc_uclass_pdata *uc_pdata;
>> + const struct dm_rproc_ops *ops;
>> +
>> + uc_pdata = dev_get_uclass_platdata(dev);
>> + if (!uc_pdata) {
>> + debug("%s: no uclass_platdata?\n", dev->name);
>> + return -ENXIO;
>> + }
>
> No need - it cannot happen.
Will drop it. Here and elsewhere.
>> +UCLASS_DRIVER(rproc) = {
>> + /* *INDENT-OFF* */
>
> What is that? ^^
Sorry - I should have removed it -> indent messes up this section of
code and the comment keeps it out of my hair..
>
>> + .id = UCLASS_RPROC,
>> + .name = "remoteproc",
>> + .flags = DM_UC_FLAG_SEQ_ALIAS,
>> + .pre_probe = rproc_pre_probe,
>> + .post_probe = rproc_post_probe,
>> + .per_device_platdata_auto_alloc_size =
>> + sizeof(struct dm_rproc_uclass_pdata),
>> + /* *INDENT-ON* */
>> +};
>> +
>> +/* exported functions */
>
> But this is not exported is it?
Agreed. will rephrase. how about "remoteproc subsystem access functions" ?
[...]
> Can you instead check each device individually and drop this flag? In
> general I would like drivers to avoid declaring any static data.
ok. will try and do that.
>> +/**
>> + * rproc_load() - load binary to a remote processor
>> + * @id: id of the remote processor
>> + * @addr: address in memory where the binary image is located
>> + * @size: size of the binary image
>> + */
>> +int rproc_load(int id, ulong addr, ulong size)
>> +{
>> + struct udevice *dev = rproc_find_dev_for_id(id);
>> + struct dm_rproc_uclass_pdata *uc_pdata;
>> + const struct dm_rproc_ops *ops;
>> +
>> + if (!dev) {
>> + printf("Unknown remote processor id '%d' requested\n", id);
>
> debug()? We should not print out messages in drivers
OK.
>> +
>> + debug("%s: data corruption?? mandatory function is missing!\n",
>> + dev->name);
>> +
>> + return -EINVAL;
>
> -ENOSYS, which means that a method is missing.
yep. thanks.
[...]
>> +
>> + printf("%s %s...\n", op_str, uc_pdata->name);
>
> debug()? This is a driver.
Here and elsewhere -> will fix.
[...]
>> +
>> +/**
>> + * rproc_start() - Start a remote processor
>> + * @id: id of the remote processor
>
> Please document the return value in these functions
OK. will do.
[...]
>> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
>> index c744044bb8aa..a2958c234db4 100644
>> --- a/include/dm/uclass-id.h
>> +++ b/include/dm/uclass-id.h
>> @@ -47,6 +47,7 @@ enum uclass_id {
>> UCLASS_PMIC, /* PMIC I/O device */
>> UCLASS_REGULATOR, /* Regulator device */
>> UCLASS_RESET, /* Reset device */
>> + UCLASS_RPROC, /* Remote Processor device */
>
> Maybe UCLASS_REMOTEPROC would be better to be consistent and more descriptive?
ok.
>
>> UCLASS_RTC, /* Real time clock device */
>> UCLASS_SERIAL, /* Serial UART */
>> UCLASS_SPI, /* SPI bus */
>> diff --git a/include/remoteproc.h b/include/remoteproc.h
>> new file mode 100644
>> index 000000000000..b92d40e0ca2e
>> --- /dev/null
>> +++ b/include/remoteproc.h
>> @@ -0,0 +1,81 @@
>> +/*
>> + * (C) Copyright 2015
>> + * Texas Instruments Incorporated - http://www.ti.com/
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +
>> +#ifndef _RPROC_H_
>> +#define _RPROC_H_
>> +
>> +#include <dm/platdata.h> /* For platform data support - non dt world */
>
> Does this need to be supported for a new uclass?
we do have platforms that are not using DT yet... they will need to pass
platform data.
>
>> +
>> +/**
>> + * enum rproc_mem_type - What type of memory model does the rproc use
>> + * @RPROC_INTERNAL: Remote processor uses own memory and is memory mapped
>> + * to the host processor over an address range.
>> + *
>> + * Please note that this is an enumeration of memory model of different types
>> + * of remote processors. Few of the remote processors do have own internal
>> + * memories, while others use external memory for instruction and data.
>> + */
>> +enum rproc_mem_type {
>> + RPROC_INTERNAL_MEMORY_MAPPED = 0,
>> +};
>> +
>> +/**
>> + * struct dm_rproc_uclass_pdata - platform data for a CPU
>> + *
>> + * This can be accessed with dev_get_platdata() for any UCLASS_RPROC
>> + * device.
>> + *
>> + * @name: Platform-specific way of naming the Remote proc
>> + * @mem_type: one of 'enum rproc_mem_type'
>> + * @driver_data: driver specific platform data that may be needed.
>
> The comment names do not match the struct.
uggh.. sorry about that. will fix.
>
>> + */
>> +struct dm_rproc_uclass_pdata {
>> + const char *name;
>> + enum rproc_mem_type mem_type;
>> + void *driver_plat_data;
>> +};
>> +
>> +/**
>> + * struct dm_rproc_ops - Operations that are provided by remote proc driver
>> + * @init: Initialize the remoteproc device invoked after probe (optional)
>> + * @load: Load the remoteproc device using data provided(mandatory)
>> + * @start: Start the remoteproc device (mandatory)
>> + * @stop: Stop the remoteproc device (optional)
>> + * @reset: Reset the remote proc device (optional)
>> + * @is_running: Check if the remote processor is running(optional)
>> + * @ping: Ping the remote device for basic communication check(optional)
>
> You should document the return value (0 for success, -ve on error?).
> For is_running(), what return value means what?
agreed - will try to do better in the next rev.
>
>> + */
>> +struct dm_rproc_ops {
>> + int (*init)(struct udevice *dev);
>> + int (*load)(struct udevice *dev, ulong addr, ulong size);
>
> document args
ok. will try to do that.
[...]
>> +static inline int rproc_init(void) { return -EINVAL; }
>
> -ENOSYS
here and else where - will update.
>
>> +static inline int rproc_is_initialized(void) { return false; }
>> +static inline int rproc_load(int id, ulong addr, ulong size) { return -EINVAL; }
>> +static inline int rproc_start(int id) { return -EINVAL; }
>> +static inline int rproc_stop(int id) { return -EINVAL; }
>> +static inline int rproc_reset(int id) { return -EINVAL; }
>> +static inline int rproc_ping(int id) { return -EINVAL; }
>> +static inline int rproc_is_running(int id) { return -EINVAL; }
>> +#endif
Thanks for the detailed review. will drop in an updated rev soon.
--
Regards,
Nishanth Menon
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/3] remoteproc: Introduce a sandbox dummy driver
2015-08-25 5:04 ` Simon Glass
@ 2015-08-25 15:47 ` Nishanth Menon
0 siblings, 0 replies; 15+ messages in thread
From: Nishanth Menon @ 2015-08-25 15:47 UTC (permalink / raw)
To: u-boot
On 08/25/2015 12:04 AM, Simon Glass wrote:
[...]
>> +# Please keep the configuration alphabetically sorted.
>> +config DM_TESTPROC_SANDBOX
>
> Should this be REMOTEPROC_SANDBOX?
Yep - will do.
>> diff --git a/drivers/remoteproc/sandbox_testproc.c b/drivers/remoteproc/sandbox_testproc.c
>> new file mode 100644
>> index 000000000000..e942f66fc2dc
>> --- /dev/null
>> +++ b/drivers/remoteproc/sandbox_testproc.c
>> @@ -0,0 +1,243 @@
>> +/*
>> + * (C) Copyright 2015
>> + * Texas Instruments Incorporated - http://www.ti.com/
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +#define pr_fmt(fmt) "%s: " fmt, __func__
^^
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <errno.h>
>> +#include <remoteproc.h>
[...]
>> +
>> +static int sandbox_testproc_is_running(struct udevice *dev)
>> +{
>> + struct dm_rproc_uclass_pdata *uc_pdata;
>> + struct sandbox_test_devdata *ddata;
>> + int ret = 1;
>> +
>> + uc_pdata = dev_get_uclass_platdata(dev);
>> + ddata = dev_get_priv(dev);
>> +
>> + if (ddata->current_state == sb_running)
>> + ret = 0;
>> + debug("%s: called(%d)\n", uc_pdata->name, ret);
>
> should that say "is_running called" / do you need __func__?
pr_fmt (at the start of the file) should take care of it.
[...]
>> +U_BOOT_DRIVER(sandbox_testproc) = {
>> + /* *INDENT-OFF* */
>
> What is that for? ^^
trying to keep indent sane - will drop.
Thanks once again for the review.
--
Regards,
Nishanth Menon
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 3/3] sandbox: Introduce dummy remoteproc nodes
2015-08-25 5:04 ` Simon Glass
@ 2015-08-25 15:47 ` Nishanth Menon
0 siblings, 0 replies; 15+ messages in thread
From: Nishanth Menon @ 2015-08-25 15:47 UTC (permalink / raw)
To: u-boot
On 08/25/2015 12:04 AM, Simon Glass wrote:
> Hi Nishanth,
>
> On 24 August 2015 at 11:28, Nishanth Menon <nm@ti.com> wrote:
>> Introduce dummy devices for sandbox remoteproc device and enable it by
>> default
>>
>> Signed-off-by: Nishanth Menon <nm@ti.com>
>> ---
>> arch/sandbox/dts/test.dts | 13 +++++++++++++
>> configs/sandbox_defconfig | 2 ++
>> 2 files changed, 15 insertions(+)
>>
>> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
>> index c948df8c864b..df9b71310dbe 100644
>> --- a/arch/sandbox/dts/test.dts
>> +++ b/arch/sandbox/dts/test.dts
>> @@ -25,6 +25,8 @@
>> usb0 = &usb_0;
>> usb1 = &usb_1;
>> usb2 = &usb_2;
>> + remoteproc1 = &rproc_1;
>> + remoteproc2 = &rproc_2;
>
> Can you please put these in alpha order?
OK. will do.
>
>> };
>>
>> a-test {
>> @@ -296,6 +298,17 @@
>> status = "disabled";
>> };
>>
>> +
>> + rproc_1: rproc at 1 {
>> + compatible = "sandbox,test-processor";
>> + remoteproc-name = "remoteproc-test-dev1";
>> + };
>> +
>> + rproc_2: rproc at 2 {
>> + compatible = "sandbox,test-processor";
>> + internal-memory-mapped;
>> + remoteproc-name = "remoteproc-test-dev2";
>> + };
>
> And these should go after reset {} I think.
Ack.
>
>> };
>>
>> #include "sandbox_pmic.dtsi"
>> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
>> index 874a26b572aa..4a2750c154bc 100644
>> --- a/configs/sandbox_defconfig
>> +++ b/configs/sandbox_defconfig
>> @@ -54,3 +54,5 @@ CONFIG_LED_GPIO=y
>> CONFIG_SYSCON=y
>> CONFIG_REGMAP=y
>> CONFIG_DEVRES=y
>> +CONFIG_DM_TESTPROC_SANDBOX=y
>
> CONFIG_TESTPROC_SANDBOX
Ack.
[...]
--
Regards,
Nishanth Menon
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 0/3] drivers/sandbox: Introduce a simplified remoteproc framework
2015-08-25 5:04 ` [U-Boot] [PATCH 0/3] drivers/sandbox: Introduce a simplified remoteproc framework Simon Glass
@ 2015-08-25 15:48 ` Nishanth Menon
0 siblings, 0 replies; 15+ messages in thread
From: Nishanth Menon @ 2015-08-25 15:48 UTC (permalink / raw)
To: u-boot
On 08/25/2015 12:04 AM, Simon Glass wrote:
[...]
> Can you please also add a test to test/dm/remoteproc.c? It should try
> a few operations as a sanity check. Ideally it shouldn't output
> anything on the console.
OK. Will do. Thanks a ton for the detailed review. I will try and get
everything done in day or so, hopefully. Thanks once again for all the work.
--
Regards,
Nishanth Menon
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 1/3] drivers: Introduce a simplified remoteproc framework
2015-08-25 15:44 ` Nishanth Menon
@ 2015-08-26 2:26 ` Simon Glass
[not found] ` <CAGo_u6pb_BSzT0fXZ7uxoqEM1RzwKKL3dXq0wyrphAB80Yf-Nw@mail.gmail.com>
0 siblings, 1 reply; 15+ messages in thread
From: Simon Glass @ 2015-08-26 2:26 UTC (permalink / raw)
To: u-boot
Hi Nishanth,
On 25 August 2015 at 09:44, Nishanth Menon <nm@ti.com> wrote:
> On 08/25/2015 12:04 AM, Simon Glass wrote:
> [...]
>>> index 000000000000..e8fdb124e251
>>> --- /dev/null
>>> +++ b/common/cmd_remoteproc.c
[snip]
>>
>>> + .id = UCLASS_RPROC,
>>> + .name = "remoteproc",
>>> + .flags = DM_UC_FLAG_SEQ_ALIAS,
>>> + .pre_probe = rproc_pre_probe,
>>> + .post_probe = rproc_post_probe,
>>> + .per_device_platdata_auto_alloc_size =
>>> + sizeof(struct dm_rproc_uclass_pdata),
>>> + /* *INDENT-ON* */
>>> +};
>>> +
>>> +/* exported functions */
>>
>> But this is not exported is it?
>
> Agreed. will rephrase. how about "remoteproc subsystem access functions" ?
Sure
>>> +#include <dm/platdata.h> /* For platform data support - non dt world */
>>
>> Does this need to be supported for a new uclass?
>
> we do have platforms that are not using DT yet... they will need to pass
> platform data.
Is this a good opportunity to convert them?
>
> Thanks for the detailed review. will drop in an updated rev soon.
My comments are minor. It's a very tidy patch set already.
Regards,
Simon
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 1/3] drivers: Introduce a simplified remoteproc framework
[not found] ` <CAGo_u6pb_BSzT0fXZ7uxoqEM1RzwKKL3dXq0wyrphAB80Yf-Nw@mail.gmail.com>
@ 2015-08-27 15:49 ` Simon Glass
2015-08-27 15:54 ` Nishanth Menon
0 siblings, 1 reply; 15+ messages in thread
From: Simon Glass @ 2015-08-27 15:49 UTC (permalink / raw)
To: u-boot
+U-Boot
On 26 August 2015 at 16:51, Nishanth Menon <nm@ti.com> wrote:
> On Tue, Aug 25, 2015 at 9:26 PM, Simon Glass <sjg@chromium.org> wrote:
>>
>>>>> +#include <dm/platdata.h> /* For platform data support - non dt world */
>>>>
>>>> Does this need to be supported for a new uclass?
>>>
>>> we do have platforms that are not using DT yet... they will need to pass
>>> platform data.
>>
>> Is this a good opportunity to convert them?
>
> It might be better not to hold the platforms hostage yet -> at least
> the TI ones(which depend on this series) are already in the pipe for
> conversion - the journey will take some time to complete though.
That seems OK, but please add a comment that new platforms should not
use the platform data.
Regards,
Simon
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 1/3] drivers: Introduce a simplified remoteproc framework
2015-08-27 15:49 ` Simon Glass
@ 2015-08-27 15:54 ` Nishanth Menon
0 siblings, 0 replies; 15+ messages in thread
From: Nishanth Menon @ 2015-08-27 15:54 UTC (permalink / raw)
To: u-boot
On Thu, Aug 27, 2015 at 10:49 AM, Simon Glass <sjg@chromium.org> wrote:
> +U-Boot
Aaah.. i missed a reply-all, did'nt I?.. Sorry about that.
>
> On 26 August 2015 at 16:51, Nishanth Menon <nm@ti.com> wrote:
>> On Tue, Aug 25, 2015 at 9:26 PM, Simon Glass <sjg@chromium.org> wrote:
>>>
>>>>>> +#include <dm/platdata.h> /* For platform data support - non dt world */
>>>>>
>>>>> Does this need to be supported for a new uclass?
>>>>
>>>> we do have platforms that are not using DT yet... they will need to pass
>>>> platform data.
>>>
>>> Is this a good opportunity to convert them?
>>
>> It might be better not to hold the platforms hostage yet -> at least
>> the TI ones(which depend on this series) are already in the pipe for
>> conversion - the journey will take some time to complete though.
>
> That seems OK, but please add a comment that new platforms should not
> use the platform data.
>
That is fair enough. will definitely do that. thanks for the suggestion.
--
---
Regards,
Nishanth Menon
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2015-08-27 15:54 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-24 17:28 [U-Boot] [PATCH 0/3] drivers/sandbox: Introduce a simplified remoteproc framework Nishanth Menon
2015-08-24 17:28 ` [U-Boot] [PATCH 1/3] drivers: " Nishanth Menon
2015-08-25 5:04 ` Simon Glass
2015-08-25 15:44 ` Nishanth Menon
2015-08-26 2:26 ` Simon Glass
[not found] ` <CAGo_u6pb_BSzT0fXZ7uxoqEM1RzwKKL3dXq0wyrphAB80Yf-Nw@mail.gmail.com>
2015-08-27 15:49 ` Simon Glass
2015-08-27 15:54 ` Nishanth Menon
2015-08-24 17:28 ` [U-Boot] [PATCH 2/3] remoteproc: Introduce a sandbox dummy driver Nishanth Menon
2015-08-25 5:04 ` Simon Glass
2015-08-25 15:47 ` Nishanth Menon
2015-08-24 17:28 ` [U-Boot] [PATCH 3/3] sandbox: Introduce dummy remoteproc nodes Nishanth Menon
2015-08-25 5:04 ` Simon Glass
2015-08-25 15:47 ` Nishanth Menon
2015-08-25 5:04 ` [U-Boot] [PATCH 0/3] drivers/sandbox: Introduce a simplified remoteproc framework Simon Glass
2015-08-25 15:48 ` Nishanth Menon
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.