Generic firmware loader

PATCH New tien.fong.chee at intel.com 5 participants 13 messages Mar 5, 2018 – Mar 15, 2018

lore

Share & tools

lore

tien.fong.chee at intel.com

author

2018-03-05 09:43 UTC #0 lore raw

v10 cover Generic firmware loader

From: Tien Fong Chee <[email protected]>

This patchset contains generic firmware loader which is very close to Linux firmware loader but for U-Boot framework. Generic firmware loader can be used load whatever into target location, and then consumer driver would use it to program whatever, ie. the FPGA. This version mainly resolved comments from Lothar WaÃŸmann in [v9].

This series is working on top of u-boot.git -
http://git.denx.de/u-boot.git .

[v9]: https://www.mail-archive.com/u-boot at lists.denx.de/msg279152.html

v9 -> v10 changes:
-----------------
- Resolved memory leaking and cleaning up messy codes.

Patchset history
----------------
[v1]: https://www.mail-archive.com/u-boot at lists.denx.de/msg271905.html [v2]: https://www.mail-archive.com/u-boot at lists.denx.de/msg271979.html [v3]: https://www.mail-archive.com/u-boot at lists.denx.de/msg272039.html [v4]: https://www.mail-archive.com/u-boot at lists.denx.de/msg272432.html [v5]: https://www.mail-archive.com/u-boot at lists.denx.de/msg272771.html [v6]: https://www.mail-archive.com/u-boot at lists.denx.de/msg273961.html [v7]: https://www.mail-archive.com/u-boot at lists.denx.de/msg276775.html [v8]: https://www.mail-archive.com/u-boot at lists.denx.de/msg278735.html

Tien Fong Chee (4):
spl: Remove static declaration on spl_mmc_find_device function
cmd: ubifs: Move ubifs_initialized checking into cmd_ubifs_umount()
cmd: ubifs: Factor out some checking codes into cmd_ubifs_mount()
common: Generic firmware loader for file system

 cmd/ubifs.c                |  40 ++---
 common/Kconfig             |  10 ++
 common/Makefile            |   1 +
 common/fs_loader.c         | 353 +++++++++++++++++++++++++++++++++++++++++++++
 common/spl/spl_mmc.c       |   2 +-
 doc/README.firmware_loader |  86 +++++++++++
 include/fs_loader.h        |  28 ++++
 include/spl.h              |   2 +
 include/ubi_uboot.h        |   2 +
 9 files changed, 506 insertions(+), 18 deletions(-)
 create mode 100644 common/fs_loader.c
 create mode 100644 doc/README.firmware_loader
 create mode 100644 include/fs_loader.h

Signature · 1 line

2.2.0

tien.fong.chee at intel.com

author

2018-03-05 09:43 UTC #1 lore raw

PATCH v10 1/4 spl: Remove static declaration on spl_mmc_find_device function

From: Tien Fong Chee <[email protected]>

This patch removes the static declation on spl_mmc_find_device_function so this function is accessible by the caller from other file. This patch is required for later patch.

Signed-off-by Tien Fong Chee <[email protected]>

Reviewed-by Tom Rini <[email protected]>

Reviewed-by Marek Vasut <[email protected]>

---
 common/spl/spl_mmc.c | 2 +-
 include/spl.h        | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

common/spl/spl_mmc.c +1 -1

index 351f4ed..f1c4168 100644
@@ -114,7 +114,7 @@ static int spl_mmc_get_device_index(u32 boot_device)
 	return -ENODEV;
 }
 
-static int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device)
+int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device)
 {
 #if CONFIG_IS_ENABLED(DM_MMC)
 	struct udevice *dev;

include/spl.h +2 -0

index c14448b..60424d7 100644
@@ -12,6 +12,7 @@
 /* Platform-specific defines */
 #include <linux/compiler.h>
 #include <asm/spl.h>
+#include <mmc.h>
 
 /* Value in r0 indicates we booted from U-Boot */
 #define UBOOT_NOT_LOADED_FROM_SPL	0x13578642

@@ -83,6 +84,7 @@ void preloader_console_init(void);
 u32 spl_boot_device(void);
 u32 spl_boot_mode(const u32 boot_device);
 void spl_set_bd(void);
+int spl_mmc_find_device(struct mmc **mmcp, u32 boot_device);
 
 /**
  * spl_set_header_raw_uboot() - Set up a standard SPL image structure

Signature · 1 line

2.2.0

tien.fong.chee at intel.com

author

2018-03-05 09:43 UTC #2 lore raw

PATCH v10 2/4 cmd: ubifs: Move ubifs_initialized checking into cmd_ubifs_umount()

In reply to tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

cmd_ubifs_umount() function would be called directly instead of involving whole command machinery in generic firmware loader, so checking on ubifs_initialized status need to be done in cmd_ubifs_umount() without breaking original functionality design.

Signed-off-by Tien Fong Chee <[email protected]>

Reviewed-by Marek Vasut <[email protected]>

---
 cmd/ubifs.c         | 18 +++++++++---------
 include/ubi_uboot.h |  1 +
 2 files changed, 10 insertions(+), 9 deletions(-)

cmd/ubifs.c +9 -9

index 5e9d357..0d059ca 100644
@@ -51,11 +51,18 @@ int ubifs_is_mounted(void)
 	return ubifs_mounted;
 }
 
-void cmd_ubifs_umount(void)
+int cmd_ubifs_umount(void)
 {
+	if (ubifs_initialized == 0) {
+		printf("No UBIFS volume mounted!\n");
+		return -1;
+	}
+
 	uboot_ubifs_umount();
 	ubifs_mounted = 0;
 	ubifs_initialized = 0;
+
+	return 0;
 }
 
 static int do_ubifs_umount(cmd_tbl_t *cmdtp, int flag, int argc,

@@ -64,14 +71,7 @@ static int do_ubifs_umount(cmd_tbl_t *cmdtp, int flag, int argc,
 	if (argc != 1)
 		return CMD_RET_USAGE;
 
-	if (ubifs_initialized == 0) {
-		printf("No UBIFS volume mounted!\n");
-		return -1;
-	}
-
-	cmd_ubifs_umount();
-
-	return 0;
+	return cmd_ubifs_umount();
 }
 
 static int do_ubifs_ls(cmd_tbl_t *cmdtp, int flag, int argc,

include/ubi_uboot.h +1 -0

index 80acbcb..827dbfc 100644
@@ -75,5 +75,6 @@ extern int ubi_volume_write(char *volume, void *buf, size_t size);
 extern int ubi_volume_read(char *volume, char *buf, size_t size);
 
 extern struct ubi_device *ubi_devices[];
+int cmd_ubifs_umount(void);
 
 #endif

Signature · 1 line

2.2.0

Tom Rini

2018-03-05 14:34 UTC #3 lore raw

PATCH v10 2/4 cmd: ubifs: Move ubifs_initialized checking into cmd_ubifs_umount()

Quoted 9 lines · tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>
cmd_ubifs_umount() function would be called directly instead of involving whole command machinery in generic firmware loader, so checking on ubifs_initialized status need to be done in cmd_ubifs_umount() without breaking original functionality design.
Signed-off-by Tien Fong Chee <[email protected]>
Reviewed-by Marek Vasut <[email protected]>

Reviewed-by Tom Rini <[email protected]>

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180305/b42e130e/attachment.sig>

tien.fong.chee at intel.com

author

2018-03-05 09:43 UTC #4 lore raw

PATCH v10 3/4 cmd: ubifs: Factor out some checking codes into cmd_ubifs_mount()

In reply to tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

cmd_ubifs_mount() function would be called directly instead of involving whole command machinery for mounting ubifs in generic firmware loader, so some checking codes need to be factored out into cmd_ubifs_mount() without breaking original functionality design.

Signed-off-by Tien Fong Chee <[email protected]>

Reviewed-by Marek Vasut <[email protected]>

---
 cmd/ubifs.c         | 22 ++++++++++++++--------
 include/ubi_uboot.h |  1 +
 2 files changed, 15 insertions(+), 8 deletions(-)

cmd/ubifs.c +14 -8

index 0d059ca..8339bed 100644
@@ -20,16 +20,10 @@
 static int ubifs_initialized;
 static int ubifs_mounted;
 
-static int do_ubifs_mount(cmd_tbl_t *cmdtp, int flag, int argc,
-				char * const argv[])
+int cmd_ubifs_mount(char *vol_name)
 {
-	char *vol_name;
 	int ret;
 
-	if (argc != 2)
-		return CMD_RET_USAGE;
-
-	vol_name = argv[1];
 	debug("Using volume %s\n", vol_name);
 
 	if (ubifs_initialized == 0) {

@@ -43,7 +37,19 @@ static int do_ubifs_mount(cmd_tbl_t *cmdtp, int flag, int argc,
 
 	ubifs_mounted = 1;
 
-	return 0;
+	return ret;
+}
+static int do_ubifs_mount(cmd_tbl_t *cmdtp, int flag, int argc,
+				char * const argv[])
+{
+	char *vol_name;
+
+	if (argc != 2)
+		return CMD_RET_USAGE;
+
+	vol_name = argv[1];
+
+	return cmd_ubifs_mount(vol_name);
 }
 
 int ubifs_is_mounted(void)

include/ubi_uboot.h +1 -0

index 827dbfc..0770228 100644
@@ -75,6 +75,7 @@ extern int ubi_volume_write(char *volume, void *buf, size_t size);
 extern int ubi_volume_read(char *volume, char *buf, size_t size);
 
 extern struct ubi_device *ubi_devices[];
+int cmd_ubifs_mount(char *vol_name);
 int cmd_ubifs_umount(void);
 
 #endif

Signature · 1 line

2.2.0

Tom Rini

2018-03-05 14:34 UTC #5 lore raw

PATCH v10 3/4 cmd: ubifs: Factor out some checking codes into cmd_ubifs_mount()

Quoted 9 lines · tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>
cmd_ubifs_mount() function would be called directly instead of involving whole command machinery for mounting ubifs in generic firmware loader, so some checking codes need to be factored out into cmd_ubifs_mount() without breaking original functionality design.
Signed-off-by Tien Fong Chee <[email protected]>
Reviewed-by Marek Vasut <[email protected]>

Reviewed-by Tom Rini <[email protected]>

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180305/c21b2e72/attachment.sig>

tien.fong.chee at intel.com

author

2018-03-05 09:43 UTC #6 lore raw

PATCH v10 4/4 common: Generic firmware loader for file system

In reply to tien.fong.chee at intel.com

From: Tien Fong Chee <[email protected]>

This is file system generic loader which can be used to load the file image from the storage into target such as memory. The consumer driver would then use this loader to program whatever, ie. the FPGA device.

Signed-off-by Tien Fong Chee <[email protected]>

Reviewed-by Lothar WaÃŸmann <[email protected]>

---
 common/Kconfig             |  10 ++
 common/Makefile            |   1 +
 common/fs_loader.c         | 353 +++++++++++++++++++++++++++++++++++++++++++++
 doc/README.firmware_loader |  86 +++++++++++
 include/fs_loader.h        |  28 ++++
 5 files changed, 478 insertions(+)
 create mode 100644 common/fs_loader.c
 create mode 100644 doc/README.firmware_loader
 create mode 100644 include/fs_loader.h

common/Kconfig +10 -0

index d5bc37e..ff88c6e 100644
@@ -560,6 +560,16 @@ config DISPLAY_BOARDINFO
 	  when U-Boot starts up. The board function checkboard() is called
 	  to do this.
 
+config GENERIC_FIRMWARE_LOADER
+	bool "Enable generic firmware loader driver for file system"
+	depends on SPL
+	help
+	  This is file system generic loader which can be used to load
+	  the file image from the storage into target such as memory.
+
+	  The consumer driver would then use this loader to program whatever,
+	  ie. the FPGA device.
+
 menu "Start-up hooks"
 
 config ARCH_EARLY_INIT_R

common/Makefile +1 -0

index c7bde23..92d8fb3 100644
@@ -134,3 +134,4 @@ obj-$(CONFIG_$(SPL_)LOG) += log.o
 obj-$(CONFIG_$(SPL_)LOG_CONSOLE) += log_console.o
 obj-y += s_record.o
 obj-y += xyzModem.o
+obj-$(CONFIG_GENERIC_FIRMWARE_LOADER) += fs_loader.o

common/fs_loader.c new file +353 -0

353 lines not on this page · raw message

doc/README.firmware_loader new file +86 -0

86 lines not on this page · raw message

include/fs_loader.h new file +28 -0

new file mode 100644
index 0000000..60a6ba2
@@ -0,0 +1,28 @@
+/*
+ * Copyright (C) 2018 Intel Corporation <www.intel.com>
+ *
+ * SPDX-License-Identifier:    GPL-2.0
+ */
+#ifndef _FS_LOADER_H_
+#define _FS_LOADER_H_
+
+#include <linux/types.h>
+
+struct firmware {
+	size_t size;		/* Size of a file */
+	const u8 *data;		/* Buffer for file */
+	void *priv;		/* Firmware loader private fields */
+};
+
+struct device_location {
+	char *name;	/* Such as mmc, usb,and sata. */
+	char *devpart;  /* Use the load command dev:part conventions */
+	char *mtdpart;	/* MTD partition for ubi part */
+	char *ubivol;	/* UBI volume-name for ubifsmount */
+};
+
+int request_firmware_into_buf(struct firmware **firmware_p,
+			      const char *name,
+			      struct device_location *location,
+			      void *buf, size_t size, u32 offset);
+#endif

Signature · 1 line

2.2.0

Simon Glass

2018-03-06 17:51 UTC #7 lore raw

PATCH v10 4/4 common: Generic firmware loader for file system

Hi,

Quoted 9 lines · On 5 March 2018 at 02:43, <[email protected]> wrote:

From: Tien Fong Chee <[email protected]>
This is file system generic loader which can be used to load the file image from the storage into target such as memory. The consumer driver would then use this loader to program whatever, ie. the FPGA device.
Signed-off-by Tien Fong Chee <[email protected]>
Reviewed-by Lothar WaÃŸmann <[email protected]>

Quoted 10 lines

---
 common/Kconfig             |  10 ++
 common/Makefile            |   1 +
 common/fs_loader.c         | 353 +++++++++++++++++++++++++++++++++++++++++++++
 doc/README.firmware_loader |  86 +++++++++++
 include/fs_loader.h        |  28 ++++
 5 files changed, 478 insertions(+)
 create mode 100644 common/fs_loader.c
 create mode 100644 doc/README.firmware_loader
 create mode 100644 include/fs_loader.h

This looks fine as a concept but I am not keen on the implementation.

1. It should use driver model (only) in U-Boot proper. If there is some SPL problem then add a specific function or feature for SPL.
2. It should not be necessary ti manually init subsystems - driver model does this for you
3. You can use the uclass name to find things

Please let me know if you need more info.

Regards,
Simon

Chee

2018-03-08 05:03 UTC #8 lore raw

PATCH v10 4/4 common: Generic firmware loader for file system

Quoted 24 lines · Simon Glass

Hi,
Quoted 17 lines · On 5 March 2018 at 02:43,  <[email protected]> wrote:
From: Tien Fong Chee <[email protected]>
This is file system generic loader which can be used to load the file image from the storage into target such as memory. The consumer driver would then use this loader to program whatever, ie. the FPGA device.
Signed-off-by Tien Fong Chee <[email protected]>
Reviewed-by Lothar Waßmann <[email protected]>
---
common/Kconfig             |  10 ++
common/Makefile            |   1 +
common/fs_loader.c         | 353
+++++++++++++++++++++++++++++++++++++++++++++
doc/README.firmware_loader |  86 +++++++++++
include/fs_loader.h        |  28 ++++
Quoted 4 lines
 5 files changed, 478 insertions(+)
create mode 100644 common/fs_loader.c
create mode 100644 doc/README.firmware_loader
create mode 100644 include/fs_loader.h

Quoted 2 lines

This looks fine as a concept but I am not keen on the implementation.

This patchset has been going through many rounds and a lot of time spending in review, and it is already working and being tested. I still have a lot subsequent patches pending on this patchset. I would suggest to accept this patchset, then we can enhance it to driver model in later.

Quoted 2 lines

1. It should use driver model (only) in U-Boot proper. If there is some SPL problem then add a specific function or feature for SPL.

We can doing this in later since it is require sometime to figure out and testing.

Quoted 2 lines

2. It should not be necessary ti manually init subsystems - driver model does this for you

This is for initializing storage driver in very early SPL, where loading from storage to configure some critical HW need to done first.

Quoted 1 line

3. You can use the uclass name to find things

Yeah, once it is converted to driver model, we can use the uclass for searching HW info in DTS.

Quoted 5 lines

Please let me know if you need more info.
Regards,
Simon

Simon Glass

2018-03-08 21:04 UTC #9 lore raw

PATCH v10 4/4 common: Generic firmware loader for file system

Hi,

Quoted 38 lines · Tien Fong

Quoted 24 lines · Simon Glass
Hi,
Quoted 10 lines · On 5 March 2018 at 02:43, <[email protected]> wrote:
From: Tien Fong Chee <[email protected]>
This is file system generic loader which can be used to load the file image from the storage into target such as memory. The consumer driver would then use this loader to program whatever, ie. the FPGA device.
Signed-off-by Tien Fong Chee <[email protected]>
Reviewed-by Lothar WaÃŸmann <[email protected]>
Quoted 11 lines
---
 common/Kconfig             |  10 ++
 common/Makefile            |   1 +
 common/fs_loader.c         | 353
+++++++++++++++++++++++++++++++++++++++++++++
 doc/README.firmware_loader |  86 +++++++++++
 include/fs_loader.h        |  28 ++++
 5 files changed, 478 insertions(+)
 create mode 100644 common/fs_loader.c
 create mode 100644 doc/README.firmware_loader
 create mode 100644 include/fs_loader.h
Quoted 2 lines
This looks fine as a concept but I am not keen on the implementation.
This patchset has been going through many rounds and a lot of time spending in review, and it is already working and being tested. I still have a lot subsequent patches pending on this patchset. I would suggest to accept this patchset, then we can enhance it to driver model in later.
Quoted 2 lines
1. It should use driver model (only) in U-Boot proper. If there is some SPL problem then add a specific function or feature for SPL.
We can doing this in later since it is require sometime to figure out and testing.

Quoted 10 lines

Quoted 2 lines
2. It should not be necessary ti manually init subsystems - driver model does this for you
This is for initializing storage driver in very early SPL, where loading from storage to configure some critical HW need to done first.
Quoted 1 line
3. You can use the uclass name to find things
Yeah, once it is converted to driver model, we can use the uclass for searching HW info in DTS.
Quoted 2 lines
Please let me know if you need more info.

Well I'm still not keen on this. Are you planning to do the conversion? It seems like a lot of work for someone else to do.

Regards,
Simon

Chee

2018-03-15 07:18 UTC #10 lore raw

PATCH v10 4/4 common: Generic firmware loader for file system

Quoted 62 lines · Simon Glass

Hi,
Quoted 58 lines · Tien Fong
Quoted 27 lines · Simon Glass
Hi,
Quoted 19 lines · On 5 March 2018 at 02:43,  <[email protected]> wrote:
From: Tien Fong Chee <[email protected]>
This is file system generic loader which can be used to load the file image from the storage into target such as memory. The consumer driver would then use this loader to program whatever,
ie. the FPGA device.
Signed-off-by Tien Fong Chee <[email protected]>
Reviewed-by Lothar Waßmann <[email protected]>
---
common/Kconfig             |  10 ++
common/Makefile            |   1 +
common/fs_loader.c         | 353
+++++++++++++++++++++++++++++++++++++++++++++
doc/README.firmware_loader |  86 +++++++++++
include/fs_loader.h        |  28 ++++
Quoted 4 lines
 5 files changed, 478 insertions(+)
create mode 100644 common/fs_loader.c
create mode 100644 doc/README.firmware_loader
create mode 100644 include/fs_loader.h
Quoted 3 lines
This looks fine as a concept but I am not keen on the implementation.
This patchset has been going through many rounds and a lot of time spending in review, and it is already working and being tested. I still
have a lot subsequent patches pending on this patchset. I would suggest
to accept this patchset, then we can enhance it to driver model in later.
Quoted 4 lines
1. It should use driver model (only) in U-Boot proper. If there is
some SPL problem then add a specific function or feature for SPL.
We can doing this in later since it is require sometime to figure out
and testing.
Quoted 4 lines
2. It should not be necessary ti manually init subsystems - driver
model does this for you
This is for initializing storage driver in very early SPL, where loading from storage to configure some critical HW need to done first.
Quoted 2 lines
3. You can use the uclass name to find things

Quoted 9 lines

Quoted 6 lines
Yeah, once it is converted to driver model, we can use the uclass for
searching HW info in DTS.
Quoted 3 lines
Please let me know if you need more info.
Well I'm still not keen on this. Are you planning to do the conversion? It seems like a lot of work for someone else to do.

Okay, i will try to convert it into driver model.

Quoted 2 lines

Regards,
Simon

Tom Rini

2018-03-15 12:21 UTC #11 lore raw

PATCH v10 4/4 common: Generic firmware loader for file system

Quoted 68 lines · Tien Fong

Quoted 67 lines · Simon Glass
Hi,
Quoted 63 lines · Tien Fong
Quoted 27 lines · Simon Glass
Hi,
Quoted 19 lines · On 5 March 2018 at 02:43,  <[email protected]> wrote:
From: Tien Fong Chee <[email protected]>
This is file system generic loader which can be used to load the file image from the storage into target such as memory. The consumer driver would then use this loader to program whatever,
ie. the FPGA device.
Signed-off-by Tien Fong Chee <[email protected]>
Reviewed-by Lothar Waßmann <[email protected]>
---
common/Kconfig             |  10 ++
common/Makefile            |   1 +
common/fs_loader.c         | 353
+++++++++++++++++++++++++++++++++++++++++++++
doc/README.firmware_loader |  86 +++++++++++
include/fs_loader.h        |  28 ++++
Quoted 4 lines
 5 files changed, 478 insertions(+)
create mode 100644 common/fs_loader.c
create mode 100644 doc/README.firmware_loader
create mode 100644 include/fs_loader.h
Quoted 3 lines
This looks fine as a concept but I am not keen on the implementation.
This patchset has been going through many rounds and a lot of time spending in review, and it is already working and being tested. I still
have a lot subsequent patches pending on this patchset. I would suggest
to accept this patchset, then we can enhance it to driver model in later.
Quoted 4 lines
1. It should use driver model (only) in U-Boot proper. If there is
some SPL problem then add a specific function or feature for SPL.
We can doing this in later since it is require sometime to figure out
and testing.
Quoted 4 lines
2. It should not be necessary ti manually init subsystems - driver
model does this for you
This is for initializing storage driver in very early SPL, where loading from storage to configure some critical HW need to done first.
Quoted 2 lines
3. You can use the uclass name to find things
Yeah, once it is converted to driver model, we can use the uclass for
searching HW info in DTS.
Quoted 2 lines

Quoted 5 lines

Quoted 4 lines
Quoted 1 line
Quoted 1 line
Please let me know if you need more info.
Well I'm still not keen on this. Are you planning to do the conversion? It seems like a lot of work for someone else to do.
Okay, i will try to convert it into driver model.

Thanks!

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180315/5dcf391a/attachment.sig>

Wolfgang Denk

2018-03-08 23:02 UTC #12 lore raw

PATCH v10 4/4 common: Generic firmware loader for file system

In reply to Chee

Dear Tien Fong,

Quoted 5 lines · In message <[email protected]> you wrote:

Quoted 3 lines
This looks fine as a concept but I am not keen on the implementation.
This patchset has been going through many rounds and a lot of time spending in review, and it is already working and being tested. I still

That's life....

Quoted 3 lines

have a lot subsequent patches pending on this patchset. I would suggest to accept this patchset, then we can enhance it to driver model in later.

No. The usual approach is to _first_ clean up the code, _before_ it gets added. there have been just too many cases that interest in cleaning up disappeared quickly once the unclean version was pulled into mainline.

Quoted 4 lines

Quoted 2 lines
1. It should use driver model (only) in U-Boot proper. If there is some SPL problem then add a specific function or feature for SPL.
We can doing this in later since it is require sometime to figure out and testing.

We can also do it now, and that is better.

Quoted 4 lines

Quoted 2 lines
2. It should not be necessary ti manually init subsystems - driver model does this for you
This is for initializing storage driver in very early SPL, where loading from storage to configure some critical HW need to done first.

The key problem is that your approach does not scale. We had the same discussion elsewhere.

Quoted 3 lines

Quoted 1 line
3. You can use the uclass name to find things
Yeah, once it is converted to driver model, we can use the uclass for searching HW info in DTS.

Then please convert it.

Thanks.

Best regards,

Wolfgang Denk

Signature · 4 lines

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I read part of it all the way through.