On Tue, Nov 13, 2018 at 03:10:14PM +0100, Marek Vasut 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.
I don't think that's worth it. We have exactly one user of this fixup and weak functions do grow the binaries.
It seems you don't want this series to be queued for the v2018.11.
Please stop guessing what I do or do not want. What I do not want is having patches with obvious and known bugs being queued, I think that is a reasonable requirement ?
It's true that none of us can read minds. But at this point in this patch series cycle, no, you're not talking about "known bugs". Yes, externs should always be in header files. But a quick and simple git grep shows 171 .c files with extern in them. So I hardly think this is a critical must change thing right now. Deserving of a spot on the TODO list? Yes.