[PATCH 0/3] remoteproc: uclass cleanup

[PATCH 0/3] remoteproc: uclass cleanup

[Samuel Holland]

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

-- 
2.39.2

[PATCH 1/3] remoteproc: Move rproc_cfg_arr out of the uclass header

[Samuel Holland]

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

Re: [PATCH 1/3] remoteproc: Move rproc_cfg_arr out of the uclass header

[Simon Glass]

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>

[PATCH 2/3] remoteproc: Remove unused mem_type platform data

[Samuel Holland]

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;
 };
 
-- 
2.39.2

Re: [PATCH 2/3] remoteproc: Remove unused mem_type platform data

[Simon Glass]

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

Re: [PATCH 2/3] remoteproc: Remove unused mem_type platform data

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 5176 bytes --]

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",
> -		.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;
>  };
>  

Adding Nishanth, since he knows the TI remoteproc stuff.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH 3/3] remoteproc: Remove legacy probing method

[Samuel Holland]

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;
 };
 
 /**
-- 
2.39.2

Re: [PATCH 3/3] remoteproc: Remove legacy probing method

[Simon Glass]

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>

Re: [PATCH 3/3] remoteproc: Remove legacy probing method

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 868 bytes --]

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!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]