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

[Macpaul Lin]

Hi Wolfgang,

2011/4/30 Wolfgang Denk <wd at denx.de>:
> Dear Macpaul Lin,
>
> 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?

1. posix_types.h
I cannot say I'm sure that the posix_types.h is not necessary for u-boot.
If this posix_types.h won't be used any more, should we keep it in u-boot?

I listed the required typedef for POSIX compatibility in Linux kernel
as follows which
are also support by other architectures.
typedef unsigned short         __kernel_dev_t;
typedef unsigned long          __kernel_ino_t;
typedef unsigned short         __kernel_mode_t;
typedef unsigned short         __kernel_nlink_t;
typedef long                   __kernel_off_t;
typedef int                    __kernel_pid_t;
typedef unsigned short         __kernel_ipc_pid_t;
typedef unsigned short         __kernel_uid_t;
typedef unsigned short         __kernel_gid_t;
typedef unsigned int           __kernel_size_t;
typedef int                    __kernel_ssize_t;
typedef int                    __kernel_ptrdiff_t;
typedef long                   __kernel_time_t;
typedef long                   __kernel_suseconds_t;
typedef long                   __kernel_clock_t;
typedef int                    __kernel_daddr_t;
typedef char                   *__kernel_caddr_t;
typedef unsigned short         __kernel_uid16_t;
typedef unsigned short         __kernel_gid16_t;
typedef unsigned int           __kernel_uid32_t;
typedef unsigned int           __kernel_gid32_t;
typedef unsigned short         __kernel_old_uid_t;
typedef unsigned short         __kernel_old_gid_t;
typedef long long              __kernel_loff_t;
typedef struct {
#if defined(__KERNEL__) || defined(__USE_ALL)
        int     val[2];
#else /* !defined(__KERNEL__) && !defined(__USE_ALL) */
        int     __val[2];
#endif /* !defined(__KERNEL__) && !defined(__USE_ALL) */
} __kernel_fsid_t;

> Maybe this is old code that was added before checkpatch existed?
>
>> 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.
>

Even though kernel would not add these files with "typedef" for
implementing POSIX,
however, any new architecture want to be commit to Linux kernel must
implementing
the old "typedef" code for POSIX compatibility support unless they
want to rewrite all
POSIX interfaces.

2. types.h
typedef unsigned short umode_t;
typedef unsigned long phys_addr_t;
typedef unsigned long phys_size_t;
Take phys_addr_t as example, the following code in u-boot will use it.
 include/common.h:223:phys_size_t initdram (int) will use such typedef.
../include/lmb.h:40:extern phys_addr_t lmb_alloc(struct lmb *lmb,
phys_size_t size, ulong align);
../include/addr_map.h:27:                               phys_size_t
size, int idx);
../include/image.h:455:phys_size_t getenv_bootm_size(void);

This file will use umode_t
../fs/ubifs/ubifs.h:115:        umode_t                 i_mode;

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

Okay, well then, according to the list above I think the problem of typedefs in
global_data.h and u-boot.h is similar to types.h. I don't know if you
agree with it?

If posix_types.h won't be used in u-boot, I'll suggest u-boot remove it.
However, I've found that if I have removed posix_types.h in
arch/nds32/includes/asm
will lead the following compilation fail.

/home/macpaul/u-boot/u-boot-nds32/include/linux/posix_types.h:46:29:
asm/posix_types.h: No such file or directory
In file included from /home/macpaul/u-boot/u-boot-nds32/include/linux/types.h:8,
                 from /home/macpaul/u-boot/u-boot-nds32/include/common.h:40,
                 from lib/asm-offsets.c:18:
/home/macpaul/u-boot/u-boot-nds32/include/linux/posix_types.h:46:29:
asm/posix_types.h: No such file or directory
/home/macpaul/u-boot/u-boot-nds32/include/linux/types.h:14: error:
parse error before "dev_t"
/home/macpaul/u-boot/u-boot-nds32/include/linux/types.h:14: warning:
type defaults to `int' in declaration of `dev_t'
/home/macpaul/u-boot/u-boot-nds32/include/linux/types.h:14: warning:
data definition has no type or storage class
/home/macpaul/u-boot/u-boot-nds32/include/linux/types.h:15: error:
parse error before "ino_t"
/home/macpaul/u-boot/u-boot-nds32/include/linux/types.h:15: warning:
type defaults to `int' in declaration of `ino_t'
/home/macpaul/u-boot/u-boot-nds32/include/linux/types.h:15: warning:
data definition has no type or storage class
/home/macpaul/u-boot/u-boot-nds32/include/linux/types.h:16: error:
parse error before "mode_t"
... and more.

These are indeed related to typedef in posix_types.h

According to Chao's informations, I think the files of these typedef
we've send is comfort to Linux's coding styles.

Thanks.

-- 
Best regards,
Macpaul Lin