[PATCH 0/3] remoteproc: uclass cleanup

This series cleans up some misplaced and dead code that I ran in to while writing a new remoteproc driver for the Bouffalo Lab BL808.
Samuel Holland (3): remoteproc: Move rproc_cfg_arr out of the uclass header remoteproc: Remove unused mem_type platform data remoteproc: Remove legacy probing method
cmd/remoteproc.c | 12 +------- .../driver-model/remoteproc-framework.rst | 30 ------------------- drivers/remoteproc/ipu_rproc.c | 4 ++- drivers/remoteproc/rproc-uclass.c | 25 +--------------- drivers/remoteproc/sandbox_testproc.c | 11 ------- include/remoteproc.h | 18 ----------- 6 files changed, 5 insertions(+), 95 deletions(-)

This array is private to the IPU driver, so it should be declared there.
Signed-off-by: Samuel Holland samuel@sholland.org ---
drivers/remoteproc/ipu_rproc.c | 4 +++- include/remoteproc.h | 1 - 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/remoteproc/ipu_rproc.c b/drivers/remoteproc/ipu_rproc.c index b4a06bc955a..2783628b23a 100644 --- a/drivers/remoteproc/ipu_rproc.c +++ b/drivers/remoteproc/ipu_rproc.c @@ -145,6 +145,8 @@ unsigned long mem_count; unsigned int pgtable_l2_map[MAX_NUM_L2_PAGE_TABLES]; unsigned int pgtable_l2_cnt;
+static struct rproc *rproc_cfg_arr[2]; + void *ipu_alloc_mem(struct udevice *dev, unsigned long len, unsigned long align) { unsigned long mask; @@ -597,7 +599,7 @@ struct rproc ipu2_config = { .intmem_to_l3_mapping = &ipu2_intmem_to_l3_mapping };
-struct rproc *rproc_cfg_arr[2] = { +static struct rproc *rproc_cfg_arr[2] = { [IPU2] = &ipu2_config, [IPU1] = &ipu1_config, }; diff --git a/include/remoteproc.h b/include/remoteproc.h index f48054de6ba..d8cde73748b 100644 --- a/include/remoteproc.h +++ b/include/remoteproc.h @@ -383,7 +383,6 @@ struct rproc { u32 trace_len; };
-extern struct rproc *rproc_cfg_arr[2]; /** * enum rproc_mem_type - What type of memory model does the rproc use * @RPROC_INTERNAL_MEMORY_MAPPED: Remote processor uses own memory and is memory

On Sun, 19 Feb 2023 at 23:13, Samuel Holland samuel@sholland.org wrote:
This array is private to the IPU driver, so it should be declared there.
Signed-off-by: Samuel Holland samuel@sholland.org
drivers/remoteproc/ipu_rproc.c | 4 +++- include/remoteproc.h | 1 - 2 files changed, 3 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

There is only one possible value for this field, it is unused except for debugging, and the devicetree property is not documented.
Signed-off-by: Samuel Holland samuel@sholland.org ---
cmd/remoteproc.c | 12 +----------- doc/develop/driver-model/remoteproc-framework.rst | 1 - drivers/remoteproc/rproc-uclass.c | 7 ------- drivers/remoteproc/sandbox_testproc.c | 1 - include/remoteproc.h | 15 --------------- 5 files changed, 1 insertion(+), 35 deletions(-)
diff --git a/cmd/remoteproc.c b/cmd/remoteproc.c index ca3b436242a..2b2e52e7d3e 100644 --- a/cmd/remoteproc.c +++ b/cmd/remoteproc.c @@ -20,7 +20,6 @@ static int print_remoteproc_list(void) struct udevice *dev; struct uclass *uc; int ret; - char *type;
ret = uclass_get(UCLASS_REMOTEPROC, &uc); if (ret) { @@ -38,18 +37,9 @@ static int print_remoteproc_list(void) if (!(dev_get_flags(dev) & DM_FLAG_ACTIVATED)) continue;
- 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", + printf("%d - Name:'%s' supports: %s%s%s%s%s%s\n", dev_seq(dev), uc_pdata->name, - type, ops->load ? "load " : "", ops->start ? "start " : "", ops->stop ? "stop " : "", diff --git a/doc/develop/driver-model/remoteproc-framework.rst b/doc/develop/driver-model/remoteproc-framework.rst index 566495a21c4..bdbbb8ab7be 100644 --- a/doc/develop/driver-model/remoteproc-framework.rst +++ b/doc/develop/driver-model/remoteproc-framework.rst @@ -121,7 +121,6 @@ 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; };
diff --git a/drivers/remoteproc/rproc-uclass.c b/drivers/remoteproc/rproc-uclass.c index 50bcc9030e9..3eacd4a8d9b 100644 --- a/drivers/remoteproc/rproc-uclass.c +++ b/drivers/remoteproc/rproc-uclass.c @@ -136,12 +136,6 @@ static int rproc_pre_probe(struct udevice *dev) bool tmp; debug("'%s': using fdt\n", dev->name); uc_pdata->name = dev_read_string(dev, "remoteproc-name"); - - /* Default is internal memory mapped */ - uc_pdata->mem_type = RPROC_INTERNAL_MEMORY_MAPPED; - tmp = dev_read_bool(dev, "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; @@ -153,7 +147,6 @@ static int rproc_pre_probe(struct udevice *dev) 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; }
diff --git a/drivers/remoteproc/sandbox_testproc.c b/drivers/remoteproc/sandbox_testproc.c index 78b108184bb..4cb784ce32e 100644 --- a/drivers/remoteproc/sandbox_testproc.c +++ b/drivers/remoteproc/sandbox_testproc.c @@ -349,7 +349,6 @@ U_BOOT_DRIVER(sandbox_testproc) = { /* TODO(nm@ti.com): Remove this along with non-DT support */ static struct dm_rproc_uclass_pdata proc_3_test = { .name = "proc_3_legacy", - .mem_type = RPROC_INTERNAL_MEMORY_MAPPED, };
U_BOOT_DRVINFO(proc_3_demo) = { diff --git a/include/remoteproc.h b/include/remoteproc.h index d8cde73748b..0c4d64706d9 100644 --- a/include/remoteproc.h +++ b/include/remoteproc.h @@ -383,23 +383,9 @@ struct rproc { u32 trace_len; };
-/** - * enum rproc_mem_type - What type of memory model does the rproc use - * @RPROC_INTERNAL_MEMORY_MAPPED: 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 * @name: Platform-specific way of naming the Remote proc - * @mem_type: one of 'enum rproc_mem_type' * @driver_plat_data: driver specific platform data that may be needed. * * This can be accessed with dev_get_uclass_plat() for any UCLASS_REMOTEPROC @@ -408,7 +394,6 @@ enum rproc_mem_type { */ struct dm_rproc_uclass_pdata { const char *name; - enum rproc_mem_type mem_type; void *driver_plat_data; };

On Sun, 19 Feb 2023 at 23:13, Samuel Holland samuel@sholland.org wrote:
There is only one possible value for this field, it is unused except for debugging, and the devicetree property is not documented.
Signed-off-by: Samuel Holland samuel@sholland.org
cmd/remoteproc.c | 12 +----------- doc/develop/driver-model/remoteproc-framework.rst | 1 - drivers/remoteproc/rproc-uclass.c | 7 ------- drivers/remoteproc/sandbox_testproc.c | 1 - include/remoteproc.h | 15 --------------- 5 files changed, 1 insertion(+), 35 deletions(-)
+Tom Rini for TI

On Mon, Feb 20, 2023 at 12:13:02AM -0600, Samuel Holland wrote:
There is only one possible value for this field, it is unused except for debugging, and the devicetree property is not documented.
Signed-off-by: Samuel Holland samuel@sholland.org
cmd/remoteproc.c | 12 +----------- doc/develop/driver-model/remoteproc-framework.rst | 1 - drivers/remoteproc/rproc-uclass.c | 7 ------- drivers/remoteproc/sandbox_testproc.c | 1 - include/remoteproc.h | 15 --------------- 5 files changed, 1 insertion(+), 35 deletions(-)
diff --git a/cmd/remoteproc.c b/cmd/remoteproc.c index ca3b436242a..2b2e52e7d3e 100644 --- a/cmd/remoteproc.c +++ b/cmd/remoteproc.c @@ -20,7 +20,6 @@ static int print_remoteproc_list(void) struct udevice *dev; struct uclass *uc; int ret;
char *type;
ret = uclass_get(UCLASS_REMOTEPROC, &uc); if (ret) {
@@ -38,18 +37,9 @@ static int print_remoteproc_list(void) if (!(dev_get_flags(dev) & DM_FLAG_ACTIVATED)) continue;
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",
printf("%d - Name:'%s' supports: %s%s%s%s%s%s\n", dev_seq(dev), uc_pdata->name,
type, ops->load ? "load " : "", ops->start ? "start " : "", ops->stop ? "stop " : "",
diff --git a/doc/develop/driver-model/remoteproc-framework.rst b/doc/develop/driver-model/remoteproc-framework.rst index 566495a21c4..bdbbb8ab7be 100644 --- a/doc/develop/driver-model/remoteproc-framework.rst +++ b/doc/develop/driver-model/remoteproc-framework.rst @@ -121,7 +121,6 @@ a simplified definition of a device is as follows:
struct dm_rproc_uclass_pdata proc_3_test = { .name = "proc_3_legacy",
.driver_plat_data = &mydriver_data; };.mem_type = RPROC_INTERNAL_MEMORY_MAPPED,
diff --git a/drivers/remoteproc/rproc-uclass.c b/drivers/remoteproc/rproc-uclass.c index 50bcc9030e9..3eacd4a8d9b 100644 --- a/drivers/remoteproc/rproc-uclass.c +++ b/drivers/remoteproc/rproc-uclass.c @@ -136,12 +136,6 @@ static int rproc_pre_probe(struct udevice *dev) bool tmp; debug("'%s': using fdt\n", dev->name); uc_pdata->name = dev_read_string(dev, "remoteproc-name");
/* Default is internal memory mapped */
uc_pdata->mem_type = RPROC_INTERNAL_MEMORY_MAPPED;
tmp = dev_read_bool(dev, "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; @@ -153,7 +147,6 @@ static int rproc_pre_probe(struct udevice *dev) debug("'%s': using legacy data\n", dev->name); if (pdata->name) uc_pdata->name = pdata->name;
uc_pdata->driver_plat_data = pdata->driver_plat_data; }uc_pdata->mem_type = pdata->mem_type;
diff --git a/drivers/remoteproc/sandbox_testproc.c b/drivers/remoteproc/sandbox_testproc.c index 78b108184bb..4cb784ce32e 100644 --- a/drivers/remoteproc/sandbox_testproc.c +++ b/drivers/remoteproc/sandbox_testproc.c @@ -349,7 +349,6 @@ U_BOOT_DRIVER(sandbox_testproc) = { /* TODO(nm@ti.com): Remove this along with non-DT support */ static struct dm_rproc_uclass_pdata proc_3_test = { .name = "proc_3_legacy",
- .mem_type = RPROC_INTERNAL_MEMORY_MAPPED,
};
U_BOOT_DRVINFO(proc_3_demo) = { diff --git a/include/remoteproc.h b/include/remoteproc.h index d8cde73748b..0c4d64706d9 100644 --- a/include/remoteproc.h +++ b/include/remoteproc.h @@ -383,23 +383,9 @@ struct rproc { u32 trace_len; };
-/**
- enum rproc_mem_type - What type of memory model does the rproc use
- @RPROC_INTERNAL_MEMORY_MAPPED: 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
- @name: Platform-specific way of naming the Remote proc
- @mem_type: one of 'enum rproc_mem_type'
- @driver_plat_data: driver specific platform data that may be needed.
- This can be accessed with dev_get_uclass_plat() for any UCLASS_REMOTEPROC
@@ -408,7 +394,6 @@ enum rproc_mem_type { */ struct dm_rproc_uclass_pdata { const char *name;
- enum rproc_mem_type mem_type; void *driver_plat_data;
};
Adding Nishanth, since he knows the TI remoteproc stuff.

This removes code that abused the device's platform data, interpreting the driver platform data as if it was the uclass platform data.
Signed-off-by: Samuel Holland samuel@sholland.org ---
.../driver-model/remoteproc-framework.rst | 29 ------------------- drivers/remoteproc/rproc-uclass.c | 18 +----------- drivers/remoteproc/sandbox_testproc.c | 10 ------- include/remoteproc.h | 2 -- 4 files changed, 1 insertion(+), 58 deletions(-)
diff --git a/doc/develop/driver-model/remoteproc-framework.rst b/doc/develop/driver-model/remoteproc-framework.rst index bdbbb8ab7be..ce76e5ea495 100644 --- a/doc/develop/driver-model/remoteproc-framework.rst +++ b/doc/develop/driver-model/remoteproc-framework.rst @@ -106,35 +106,6 @@ 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.
-Describing the device using platform data ------------------------------------------ - -*IMPORTANT* NOTE: THIS SUPPORT IS NOT MEANT FOR USE WITH NEWER PLATFORM -SUPPORT. THIS IS ONLY FOR LEGACY DEVICES. THIS MODE OF INITIALIZATION -*WILL* BE EVENTUALLY REMOVED ONCE ALL NECESSARY PLATFORMS HAVE MOVED -TO DM/FDT. - -Considering that many platforms are yet to move to device-tree model, -a simplified definition of a device is as follows: - -.. code-block:: c - - struct dm_rproc_uclass_pdata proc_3_test = { - .name = "proc_3_legacy", - .driver_plat_data = &mydriver_data; - }; - - U_BOOT_DRVINFO(proc_3_demo) = { - .name = "sandbox_test_proc", - .plat = &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. - Describing the device using device tree ---------------------------------------
diff --git a/drivers/remoteproc/rproc-uclass.c b/drivers/remoteproc/rproc-uclass.c index 3eacd4a8d9b..def43a8cf32 100644 --- a/drivers/remoteproc/rproc-uclass.c +++ b/drivers/remoteproc/rproc-uclass.c @@ -131,24 +131,8 @@ static int rproc_pre_probe(struct udevice *dev)
/* See if we need to populate via fdt */
- if (!dev_get_plat(dev)) { -#if CONFIG_IS_ENABLED(OF_CONTROL) - bool tmp; - debug("'%s': using fdt\n", dev->name); + if (dev_has_ofnode(dev)) uc_pdata->name = dev_read_string(dev, "remoteproc-name"); -#else - /* Nothing much we can do about this, can we? */ - return -EINVAL; -#endif - - } else { - struct dm_rproc_uclass_pdata *pdata = dev_get_plat(dev); - - debug("'%s': using legacy data\n", dev->name); - if (pdata->name) - uc_pdata->name = pdata->name; - uc_pdata->driver_plat_data = pdata->driver_plat_data; - }
/* Else try using device Name */ if (!uc_pdata->name) diff --git a/drivers/remoteproc/sandbox_testproc.c b/drivers/remoteproc/sandbox_testproc.c index 4cb784ce32e..b19477daa7f 100644 --- a/drivers/remoteproc/sandbox_testproc.c +++ b/drivers/remoteproc/sandbox_testproc.c @@ -345,13 +345,3 @@ U_BOOT_DRIVER(sandbox_testproc) = { .probe = sandbox_testproc_probe, .priv_auto = sizeof(struct sandbox_test_devdata), }; - -/* TODO(nm@ti.com): Remove this along with non-DT support */ -static struct dm_rproc_uclass_pdata proc_3_test = { - .name = "proc_3_legacy", -}; - -U_BOOT_DRVINFO(proc_3_demo) = { - .name = "sandbox_test_proc", - .plat = &proc_3_test, -}; diff --git a/include/remoteproc.h b/include/remoteproc.h index 0c4d64706d9..e0fccb0177f 100644 --- a/include/remoteproc.h +++ b/include/remoteproc.h @@ -386,7 +386,6 @@ struct rproc { /** * struct dm_rproc_uclass_pdata - platform data for a CPU * @name: Platform-specific way of naming the Remote proc - * @driver_plat_data: driver specific platform data that may be needed. * * This can be accessed with dev_get_uclass_plat() for any UCLASS_REMOTEPROC * device. @@ -394,7 +393,6 @@ struct rproc { */ struct dm_rproc_uclass_pdata { const char *name; - void *driver_plat_data; };
/**

On Sun, 19 Feb 2023 at 23:13, Samuel Holland samuel@sholland.org wrote:
This removes code that abused the device's platform data, interpreting the driver platform data as if it was the uclass platform data.
Signed-off-by: Samuel Holland samuel@sholland.org
.../driver-model/remoteproc-framework.rst | 29 ------------------- drivers/remoteproc/rproc-uclass.c | 18 +----------- drivers/remoteproc/sandbox_testproc.c | 10 ------- include/remoteproc.h | 2 -- 4 files changed, 1 insertion(+), 58 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

On Mon, Feb 20, 2023 at 12:13:03AM -0600, Samuel Holland wrote:
This removes code that abused the device's platform data, interpreting the driver platform data as if it was the uclass platform data.
Signed-off-by: Samuel Holland samuel@sholland.org Reviewed-by: Simon Glass sjg@chromium.org
.../driver-model/remoteproc-framework.rst | 29 ------------------- drivers/remoteproc/rproc-uclass.c | 18 +----------- drivers/remoteproc/sandbox_testproc.c | 10 ------- include/remoteproc.h | 2 -- 4 files changed, 1 insertion(+), 58 deletions(-)
This breaks the existing tests, please see https://u-boot.readthedocs.io/en/latest/develop/ci_testing.html and/or https://u-boot.readthedocs.io/en/latest/develop/py_testing.html for how to run them. Thanks!
participants (3)
-
Samuel Holland
-
Simon Glass
-
Tom Rini