[U-Boot] [PATCH v2 07/15] qemu-malta: rename to just "malta"

[Paul Burton]

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.burton at 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