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

[Daniel Schwierzeck]

2013/10/24 Paul Burton <paul.burton at 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.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
>

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