On Tue, Nov 13, 2018 at 03:37:26PM +0100, Boris Brezillon wrote:
On Tue, 13 Nov 2018 15:10:14 +0100 Marek Vasut marek.vasut@gmail.com wrote:
On 11/13/2018 01:39 PM, Boris Brezillon wrote:
Hi Marek,
On Tue, 13 Nov 2018 13:19:52 +0100 Marek Vasut marek.vasut@gmail.com wrote:
On 11/13/2018 12:43 PM, Boris Brezillon wrote:
U-boot provides a mean to define default values for mtdids and mtdparts when they're not defined in the environment. Patch mtd_probe_devices() to use those default values when env_get("mtdparts") or env_get("mtdids") return NULL.
This implementation is based on the logic found in cmd/mtdparts.c.
Fixes: 5db66b3aee6f ("cmd: mtd: add 'mtd' command") Reported-by: Stefan Roese sr@denx.de Signed-off-by: Boris Brezillon boris.brezillon@bootlin.com Tested-by: Stefan Roese sr@denx.de Reviewed-by: Lukasz Majewski lukma@denx.de
Changes in v3:
- Fix env_get_f() call
- Add Lukasz R-b
Changes in v2:
- none
drivers/mtd/mtd_uboot.c | 62 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 60 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c index 7d7a11c990d6..5ca560c96879 100644 --- a/drivers/mtd/mtd_uboot.c +++ b/drivers/mtd/mtd_uboot.c @@ -92,12 +92,70 @@ static void mtd_probe_uclass_mtd_devs(void) { } #endif
#if defined(CONFIG_MTD_PARTITIONS) +extern void board_mtdparts_default(const char **mtdids,
const char **mtdparts);Why is there this extern ?
extern is not needed indeed.
This should use a prototype of function defined in a header file. Once someone changes the prototype and forgets to update this location, it'll be a disaster.
When I provide a fix I try to avoid dependencies on changes that are not absolutely required hence the decision to keep this function prototype locally defined.
I am also expecting a patch which cleans this up then. I think __weak function would be appropriate.
Well, I think the best would be to get rid of this function. It's only implemented by board/isee/igep00x0/igep00x0.c, and we might be able to use CONFIG_{MTDPARTS,MTDIDS}_DEFAULT to replace it:
CONFIG_MTDPARTS_DEFAULT "omap2-nand:Xk(SPL),-(UBI);omap2-onenand:Yk(SPL),-(UBI)" CONFIG_MTDIDS_DEFAULT "nand0=omap2-nand,onenand0=omap2-onenand"
This assumes we know the eraseblock size of the NAND and OneNAND devices of course (X and Y in the above def).
I'm not sure, I think it's doing run-time checks to see if we're on the NAND or OneNAND variant.