2013/10/24 Paul Burton <paul.bur...@imgtec.com>: > > > On 24/10/13 11:43, Gabor Juhos wrote: >> >> 2013.10.24. 12:10 keltezéssel, Paul Burton írta: >>> >>> On 24/10/13 11:04, Gabor Juhos wrote: >>>> >>>> 2013.10.24. 11:31 keltezéssel, Paul Burton írta: >>>>> >>>>> This is in preparation for adapting this board to function correctly on >>>>> a physical MIPS Malta board. The board is moved into an "imgtec" vendor >>>>> directory at the same time in order to ready us for any other boards >>>>> supported by Imagination in the future. >>>>> >>>>> Signed-off-by: Paul Burton <paul.bur...@imgtec.com> >>>>> --- >>>>> Changes in v2: >>>>> - move to an imgtec vendor directory to reduce churn should other >>>>> Imagination boards be added later >>>>> - enable rename detection for the patch >>>>> --- >>>>> arch/mips/lib/bootm.c | 12 >>>>> ++++++------ >>>>> board/{qemu-malta => imgtec/malta}/Makefile | 0 >>>>> board/{qemu-malta => imgtec/malta}/lowlevel_init.S | 0 >>>>> board/{qemu-malta/qemu-malta.c => imgtec/malta/malta.c} | 0 >>>>> boards.cfg | 4 ++-- >>>>> include/configs/{qemu-malta.h => malta.h} | 2 +- >>>>> 6 files changed, 9 insertions(+), 9 deletions(-) >>>>> rename board/{qemu-malta => imgtec/malta}/Makefile (100%) >>>>> rename board/{qemu-malta => imgtec/malta}/lowlevel_init.S (100%) >>>>> rename board/{qemu-malta/qemu-malta.c => imgtec/malta/malta.c} >>>>> (100%) >>>>> rename include/configs/{qemu-malta.h => malta.h} (98%) >>>>> >>>>> diff --git a/arch/mips/lib/bootm.c b/arch/mips/lib/bootm.c >>>>> index 66340ea..1febf29 100644 >>>>> --- a/arch/mips/lib/bootm.c >>>>> +++ b/arch/mips/lib/bootm.c >>>>> @@ -17,10 +17,10 @@ DECLARE_GLOBAL_DATA_PTR; >>>>> #define LINUX_MAX_ENVS 256 >>>>> #define LINUX_MAX_ARGS 256 >>>>> >>>>> -#if defined(CONFIG_QEMU_MALTA) >>>>> -#define mips_boot_qemu_malta 1 >>>>> +#if defined(CONFIG_MALTA) >>>>> +#define mips_boot_malta 1 >>>>> #else >>>>> -#define mips_boot_qemu_malta 0 >>>>> +#define mips_boot_malta 0 >>>>> #endif >>>>> >>>>> static int linux_argc; >>>>> @@ -139,7 +139,7 @@ static void linux_env_set(const char *env_name, >>>>> const >>>>> char *env_val) >>>>> strcpy(linux_env_p, env_name); >>>>> linux_env_p += strlen(env_name); >>>>> >>>>> - if (mips_boot_qemu_malta) { >>>>> + if (mips_boot_malta) { >>>>> linux_env_p++; >>>>> linux_env[++linux_env_idx] = linux_env_p; >>>>> } else { >>>>> @@ -196,7 +196,7 @@ static void boot_prep_linux(bootm_headers_t >>>>> *images) >>>>> if (cp) >>>>> linux_env_set("eth1addr", cp); >>>>> >>>>> - if (mips_boot_qemu_malta) >>>>> + if (mips_boot_malta) >>>>> linux_env_set("modetty0", "38400n8r"); >>>>> } >>>>> >>>>> @@ -210,7 +210,7 @@ static void boot_jump_linux(bootm_headers_t >>>>> *images) >>>>> >>>>> bootstage_mark(BOOTSTAGE_ID_RUN_OS); >>>>> >>>>> - if (mips_boot_qemu_malta) >>>>> + if (mips_boot_malta) >>>>> linux_extra = gd->ram_size; >>>>> >>>>> /* we assume that the kernel is in place */ >>>> >>>> >>>> The changes in bootm.c are not strictly related to the rename. It would >>>> be >>>> better to do these in a separate patch. >>> >>> >>> I disagree. The board was renamed from qemu-malta to malta, so it makes >>> perfect >>> sense to change CONFIG_QEMU_MALTA to CONFIG_MALTA and >>> mips_boot_qemu_malta to >>> mips_boot_malta at the same time. These changes are purely a part of the >>> rename, >>> made for consistency. >> >> >> I agree that it is related to the rename. However renaming the >> mips_boot_qemu_malta constant is a simple cleanup which can be done in a >> preceding patch. The same is true for the CONFIG_QEMU_MALTA->CONFIG_MALTA >> conversion. >> >> In general, renaming a file and altering that within the same patch makes >> the >> review harder. >> >> -Gabor >> >> > > And for more complex changes I might agree, but only a single file is both > renamed and modified by this patch: include/configs/qemu-malta.h => > include/configs/malta.h. Only a single line is modified within that file, > changing CONFIG_QEMU_MALTA to CONFIG_MALTA. I don't think it's very > difficult to understand and I think splitting it would be overkill. So if > Daniel tells me (v3 of) this patch isn't getting in as-is then I'll split > it, but short of that I'm not changing it. > > Thanks, > Paul >
Actually I don't care. It is one logical change which is also trivial and bisectable. If you want, you could move the renaming of CONFIG_QEMU_MALTA and mips_boot_qemu_malta to a separate patch. -- Best regards, Daniel _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot