[U-Boot] [PATCH v7 01/10] nds32: add header files support for nds32

[Chih-Min Chao]

On Sat, Apr 30, 2011 at 7:09 AM, Wolfgang Denk <wd at denx.de> wrote:
> Dear Macpaul Lin,
>
> In message <BANLkTi=haZVW17eNWYCmYf3McrGmbm-4Xw at mail.gmail.com> you wrote:
>>
>> I think we still have to discuss about the typedef's.
>> What does the "new" typedef means?
>
> It means adding any new code to U-Boot which includes "typedef"s.
>
>> According to the checkpatch result, "typedef" warning exists in 4 files.
>> arch/nds32/include/asm/posix_types.h
>> arch/nds32/include/asm/types.h
>> arch/nds32/include/asm/global_data.h
>> arch/nds32/include/asm/u-boot.h.
>>
>> File arch/nds32/include/asm/posix_types.h  and arch/nds32/include/asm/types.h
>> come from the Linux kernel. Which is usually used for posix
>> compatibility for Linux Kernel.
>
> So these files should be fixed in Linux, too, because it is the Linux
> checkpatch tool which throws this warning.
>
> I don't think this affects POSIX compatibility. Which typedef's are
> required by POSIX?
>
>> Which should be "old" features for posix and compatibility.
>> However, you cannot say for a new architecture to support posix and
>> other compatibility as
>> "new" typedef.
>> I've checked the latest kernel (2.6.38.1), arm, mips, avr32, powerpc
>> consist these posix_types.h and types.h with "typedef".
>
> Maybe this is old code that was added before checkpatch existed?
>
In Linux  "CodingStyle"

" Chapter 5: Typedefs

Please don't use things like "vps_t".

It's a _mistake_ to use typedef for structures and pointers."
[skip]
"Lots of people think that typedefs "help readability". Not so. They are
useful only for:"
[skip]
" (b) Clear integer types, where the abstraction _helps_ avoid confusion
     whether it is "int" or "long".

     u8/u16/u32 are perfectly fine typedefs, although they fit into
     category (d) better than here.

     NOTE! Again - there needs to be a _reason_ for this. If something is
     "unsigned long", then there's no reason to do

        typedef unsigned long myflags_t;

     but if there is a clear reason for why it under certain circumstances
     might be an "unsigned int" and under other configurations might be
     "unsigned long", then by all means go ahead and use a typedef."

I think posix_types may be an example of this case. nds32 have some
types different from
what has been defined in <asm-generic/posix_types.h>

As for commit time, the latest update for checkpatch.pl is in 2009-01-06
653d4876 (Andy Whitcroft     2007-06-23 17:16:34 -0700 1891)
 if ($line =~ /\btypedef\s/ &&
8054576d (Andy Whitcroft     2009-01-06 14:41:26 -0800 1892)
     $line !~ /\btypedef\s+$Type\s*\(\s*\*?$Ident\s*\)\s*\(/ &&
c45dcabd (Andy Whitcroft     2008-06-05 22:46:01 -0700 1893)
     $line !~ /\btypedef\s+$Type\s+$Ident\s*\(/ &&
8ed22cad (Andy Whitcroft     2008-10-15 22:02:32 -0700 1894)
     $line !~ /\b$typeTypedefs\b/ &&
653d4876 (Andy Whitcroft     2007-06-23 17:16:34 -0700 1895)
     $line !~ /\b__bitwise(?:__|)\b/) {
de7d4f0e (Andy Whitcroft     2007-07-15 23:37:22 -0700 1896)
         WARN("do not add new typedefs\n" . $herecurr);
0a920b5b (Andy Whitcroft     2007-06-01 00:46:48 -0700 1897)            }

But Michal Simek's commit, which added posix_types.h is adapted in 2009-03-27.
You can reference it by 6d9c3f208580 in Linux repo



>> It looks the kernel is not going to fix the "old" typedef for
>> posix_types.h and types.h
>
> Eventually they would not add these files as is any more today.
>
>> I think they say "please do not add any new typedef" might mean to
>> those typedef
>> used in drivers or protocols.
>
> I mean all of them.
>
>> In the other 2 files arch/nds32/include/asm/global_data.h and
>> arch/nds32/include/asm/u-boot.h,
>> typedef was used for
>> #449: FILE: arch/nds32/include/asm/global_data.h:46:
>> +typedef        struct global_data {
>> +} gd_t;
>> #1505: FILE: arch/nds32/include/asm/u-boot.h:41:
>> +typedef struct bd_info {
>> +} bd_t;
>
> Ouch. You got me there... :-(
>
>> I don't know if you have any idea of fixing it in u-boot.
>
> Well, the fix is basicly straightforward - replace all ocurrences of
> gd_t and bd_t in the U-Boot code.  But this is a bigger issue and more
> work than I dare to push on you.  I'm grinding my teeth, but I will
> accept these.
>
> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> If ignorance is bliss, why aren't there more happy people?
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>