| Message ID | 1463120984-19813-10-git-send-email-yanmiaobest@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Bin Meng |
| Headers | show |
Hi Miao, On Fri, May 13, 2016 at 2:29 PM, Miao Yan <yanmiaobest@gmail.com> wrote: > The qfw command interface makes use of CONFIG_LOADADDR and > CONFIG_RAMDISKADDR to setup kernel. But not all boards have these macro, > which causes build problem on those platforms. > > This patch fixes this issue. > > Signed-off-by: Miao Yan <yanmiaobest@gmail.com> Looks good. Reviewed-by: Bin Meng <bmeng.cn@gmail.com> But the commit title "workaround" looks to be a workaround, which does not sound that good. > --- > cmd/cmd_qfw.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/cmd/cmd_qfw.c b/cmd/cmd_qfw.c > index aeb576b..13ad673 100644 > --- a/cmd/cmd_qfw.c > +++ b/cmd/cmd_qfw.c > @@ -126,12 +126,20 @@ static int qemu_fwcfg_do_load(cmd_tbl_t *cmdtp, int flag, > env = getenv("loadaddr"); > load_addr = env ? > (void *)simple_strtoul(env, NULL, 16) : > +#ifdef CONFIG_LOADADDR > (void *)CONFIG_LOADADDR; > +#else > + NULL; > +#endif > > env = getenv("ramdiskaddr"); > initrd_addr = env ? > (void *)simple_strtoul(env, NULL, 16) : > +#ifdef CONFIG_RAMDISK_ADDR > (void *)CONFIG_RAMDISK_ADDR; > +#else > + NULL; > +#endif > > if (argc == 2) { > load_addr = (void *)simple_strtoul(argv[0], NULL, 16); > @@ -140,6 +148,11 @@ static int qemu_fwcfg_do_load(cmd_tbl_t *cmdtp, int flag, > load_addr = (void *)simple_strtoul(argv[0], NULL, 16); > } > > + if (!load_addr || !initrd_addr) { > + printf("missing load or initrd address\n"); > + return CMD_RET_FAILURE; > + } > + > return qemu_fwcfg_setup_kernel(load_addr, initrd_addr); > } > > -- Regards, Bin
On Fri, May 13, 2016 at 10:00:37PM +0800, Bin Meng wrote: > Hi Miao, > > On Fri, May 13, 2016 at 2:29 PM, Miao Yan <yanmiaobest@gmail.com> wrote: > > The qfw command interface makes use of CONFIG_LOADADDR and > > CONFIG_RAMDISKADDR to setup kernel. But not all boards have these macro, > > which causes build problem on those platforms. > > > > This patch fixes this issue. > > > > Signed-off-by: Miao Yan <yanmiaobest@gmail.com> > > Looks good. > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com> > > But the commit title "workaround" looks to be a workaround, which does > not sound that good. Yes, maybe just say that it's no longer requiring default values?
2016-05-14 4:46 GMT+08:00 Tom Rini <trini@konsulko.com>: > On Fri, May 13, 2016 at 10:00:37PM +0800, Bin Meng wrote: >> Hi Miao, >> >> On Fri, May 13, 2016 at 2:29 PM, Miao Yan <yanmiaobest@gmail.com> wrote: >> > The qfw command interface makes use of CONFIG_LOADADDR and >> > CONFIG_RAMDISKADDR to setup kernel. But not all boards have these macro, >> > which causes build problem on those platforms. >> > >> > This patch fixes this issue. >> > >> > Signed-off-by: Miao Yan <yanmiaobest@gmail.com> >> >> Looks good. >> >> Reviewed-by: Bin Meng <bmeng.cn@gmail.com> >> >> But the commit title "workaround" looks to be a workaround, which does >> not sound that good. > > Yes, maybe just say that it's no longer requiring default values? OK. Will fix. Thanks, Miao > > -- > Tom
diff --git a/cmd/cmd_qfw.c b/cmd/cmd_qfw.c index aeb576b..13ad673 100644 --- a/cmd/cmd_qfw.c +++ b/cmd/cmd_qfw.c @@ -126,12 +126,20 @@ static int qemu_fwcfg_do_load(cmd_tbl_t *cmdtp, int flag, env = getenv("loadaddr"); load_addr = env ? (void *)simple_strtoul(env, NULL, 16) : +#ifdef CONFIG_LOADADDR (void *)CONFIG_LOADADDR; +#else + NULL; +#endif env = getenv("ramdiskaddr"); initrd_addr = env ? (void *)simple_strtoul(env, NULL, 16) : +#ifdef CONFIG_RAMDISK_ADDR (void *)CONFIG_RAMDISK_ADDR; +#else + NULL; +#endif if (argc == 2) { load_addr = (void *)simple_strtoul(argv[0], NULL, 16); @@ -140,6 +148,11 @@ static int qemu_fwcfg_do_load(cmd_tbl_t *cmdtp, int flag, load_addr = (void *)simple_strtoul(argv[0], NULL, 16); } + if (!load_addr || !initrd_addr) { + printf("missing load or initrd address\n"); + return CMD_RET_FAILURE; + } + return qemu_fwcfg_setup_kernel(load_addr, initrd_addr); }
The qfw command interface makes use of CONFIG_LOADADDR and CONFIG_RAMDISKADDR to setup kernel. But not all boards have these macro, which causes build problem on those platforms. This patch fixes this issue. Signed-off-by: Miao Yan <yanmiaobest@gmail.com> --- cmd/cmd_qfw.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)