linux-mips
[Top] [All Lists]

Re: [loongson-PATCH-v3 00/25] loongson-based machines support

To: wuzhangjin@gmail.com
Subject: Re: [loongson-PATCH-v3 00/25] loongson-based machines support
From: Zhang Le <r0bertz@gentoo.org>
Date: Tue, 9 Jun 2009 00:38:22 +0800
Cc: linux-mips@linux-mips.org, ralf@linux-mips.org, Wu Zhangjin <wuzj@lemote.com>, Yan Hua <yanh@lemote.com>, Philippe Vachon <philippe@cowpig.ca>, Zhang Fuxin <zhangfx@lemote.com>, loongson-dev <loongson-dev@googlegroups.com>, Liu Junliang <liujl@lemote.com>, Erwan Lerale <erwan@thiscow.com>
In-reply-to: <cover.1244119295.git.wuzj@lemote.com>
Mail-followup-to: wuzhangjin@gmail.com, linux-mips@linux-mips.org, ralf@linux-mips.org, Wu Zhangjin <wuzj@lemote.com>, Yan Hua <yanh@lemote.com>, Philippe Vachon <philippe@cowpig.ca>, Zhang Fuxin <zhangfx@lemote.com>, loongson-dev <loongson-dev@googlegroups.com>, Liu Junliang <liujl@lemote.com>, Erwan Lerale <erwan@thiscow.com>
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <cover.1244119295.git.wuzj@lemote.com>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Mutt/1.5.19 (2009-01-05)
First of all, sorry for late comment and thanks to Zhangjin for the great work.

However, I have some suggestions.

On 20:58 Thu 04 Jun     , wuzhangjin@gmail.com wrote:
> Wu Zhangjin (25):
>   add vmlinux.32 in .gitignore
>   fix-warning: incompatible argument type of pci_fixup_irqs
>   fix-warning: incompatible argument type of virt_to_phys

I think these 3 patch could be submitted separately, since they are not quite
related to Loongson.

>   change the naming methods

In this patch, I found function get_system_type() still returns wrong name,
"lemote-fulong". In later patches, I found this string was changed to a macro,
MACH_NAME. Then, the function becomes more complicated and/or sophisticated,
because of the addition of machname array.

I don't know if this is an established or widely accepted policy, but
intuitively, at the very least IMHO, a series of patches should only provide
one correct implementation of a particular function, not provide one wrong
function then override it with a correct one.

If I were you, I would do a 'git reset' first.
Then 'git add' and/or 'git rm' some files which contain similar changes.
Then 'git commit'.
Repeat the last two steps, until all the changes have been committed.

-- 
Zhang, Le
Gentoo/Loongson Developer
http://zhangle.is-a-geek.org
0260 C902 B8F8 6506 6586 2B90 BC51 C808 1E4E 2973

Attachment: pgpod62XXB2q7.pgp
Description: PGP signature

<Prev in Thread] Current Thread [Next in Thread>