Hi, On Tue, 2009-06-09 at 00:38 +0800, Zhang Le wrote: > 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@xxxxxxxxx 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. > Yeap, I will split all of the non-loongson-specific patches out in the coming -v4 patch series and also split the immature ones out too. > > 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. the MACH_NAME macro method is originally picked from lm2e-fixes branch of Philippe's git://git.linux-cisco.org/linux-mips.git, this method is used to share the get_system_type() function between different machines. and the machtype with machname array is only used to share the same kernel image file between yeeloong-7inch(menglong?) and yeeloong-8.9inch source code. I think it will be "bad" to add a new kernel option named MENGLONG or something else, and add a new yeeloong-7inch directory in arch/mips/loongson/ for it, because the difference between yeeloong-7inch and yeeloong-8.0 inch is very small(the shutdown logic and screen size). and also, simply add two new kernel options like YEELOONG-7INCH and YEELOONG-89INCH with #ifdef..#else...#endif is also not that good, is that? so, i just added a machtype kernel command line(perhaps we can use the systype or machtype variable in pmon instead as Arnaud Patard suggested). and perhaps this machtype can also be used to share the the kernel image file among the future machines. what about your suggestion here? is there another good solution? or just keep it simple: just define the get_system_type() function for every machine and add a new kernel option for yeeloong-7inch? > 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. > I will try to rebase and merge some of the similar patches, if not work, then did as you suggested, thanks a lot! thanks! Wu Zhangjin
