>-----Original Message-----
>From: Rich Felker [mailto:dalias@aerifal.cx] On Behalf Of Rich Felker
>(dalias@libc.org)
>Sent: 02 March 2016 AM 10:12
>To: Jaydeep Patil
>Cc: Mahesh Bodapati; musl@lists.openwall.com; nsz@port70.net
>Subject: Re: [musl] MUSL MIPS64 N64 port
>
>On Tue, Mar 01, 2016 at 07:52:33AM +0000, Jaydeep Patil wrote:
>> Also got:
>>
>> src/thread/pthread_create.c:212:11: warning: cast to pointer from integer
>of different size [-Wint-to-pointer-cast]
>>    stack = (void *)(attr._a_stackaddr & -16);
>>
>> This is because your pthread types are wrong; you're using the 32-bit arch
>ones:
>
>This does not seem to be fixed; see below in your new patch:
>
>> diff --git a/arch/mips64/bits/alltypes.h.in b/arch/mips64/bits/alltypes.h.in
>> new file mode 100644
>> index 0000000..64385c9
>> --- /dev/null
>> +++ b/arch/mips64/bits/alltypes.h.in
>> [...]
>> +TYPEDEF struct { union { int __i[9]; volatile int __vi[9]; unsigned long __s[9];
>} __u; } pthread_attr_t;
>> +TYPEDEF struct { union { int __i[6]; volatile int __vi[6]; volatile void *volatile
>__p[6]; } __u; } pthread_mutex_t;
>> +TYPEDEF struct { union { int __i[6]; volatile int __vi[6]; volatile void *volatile
>__p[6]; } __u; } mtx_t;
>> +TYPEDEF struct { union { int __i[12]; volatile int __vi[12]; void *__p[12]; }
>__u; } pthread_cond_t;
>> +TYPEDEF struct { union { int __i[12]; volatile int __vi[12]; void *__p[12]; }
>__u; } cnd_t;
>> +TYPEDEF struct { union { int __i[8]; volatile int __vi[8]; void *__p[8]; } __u; }
>pthread_rwlock_t;
>> +TYPEDEF struct { union { int __i[5]; volatile int __vi[5]; void *__p[5]; } __u; }
>pthread_barrier_t;
>
>These are still the 32-bit definitions not the 64-bit ones.
>

Ok. 

>> diff --git a/arch/mips64/bits/stat.h b/arch/mips64/bits/stat.h
>> new file mode 100644
>> index 0000000..dfc80b3
>> --- /dev/null
>> +++ b/arch/mips64/bits/stat.h
>> @@ -0,0 +1,23 @@
>> +#include <string.h>
>> +#include <bits/alltypes.h>
>> +
>> +struct stat {
>> +	dev_t st_dev;
>> +	int pad1[3];
>> +	ino_t st_ino;
>> +	mode_t st_mode;
>> +	nlink_t st_nlink;
>> +	uid_t st_uid;
>> +	gid_t st_gid;
>> +	dev_t st_rdev;
>> +	unsigned int pad2[2];
>> +	off_t st_size;
>> +	int pad3;
>> +	struct timespec st_atim;
>> +	struct timespec st_mtim;
>> +	struct timespec st_ctim;
>> +	blksize_t st_blksize;
>> +	unsigned int pad4;
>> +	blkcnt_t st_blocks;
>> +	int pad5[14];
>> +};
>
>pad1, pad2 etc. are in the application-reserved namespace. These need
>to be __pad1, __pad2, etc. or similar. -- something reserved.
>

Ok.

>> +static void __stat_fix(struct kernel_stat *kst, struct stat *st)
>> +{
>> +	st->st_dev = kst->st_dev;
>> +	st->pad1[0] = 0;
>> +	st->pad1[1] = 0;
>> +	st->pad1[2] = 0;
>> +	st->st_ino = kst->st_ino;
>> +	st->st_mode = kst->st_mode;
>> +	st->st_nlink = kst->st_nlink;
>> +	st->st_uid = kst->st_uid;
>> +	st->st_gid = kst->st_gid;
>> +	st->st_rdev = kst->st_rdev;
>> +	st->pad2[0] = 0;
>> +	st->pad2[1] = 0;
>> +	st->st_size = kst->st_size;
>> +	st->pad3 = 0;
>> +	st->st_atim.tv_sec = kst->st_atime_sec;
>> +	st->st_atim.tv_nsec = kst->st_atime_nsec;
>> +	st->st_mtim.tv_sec = kst->st_mtime_sec;
>> +	st->st_mtim.tv_nsec = kst->st_mtime_nsec;
>> +	st->st_ctim.tv_sec = kst->st_ctime_sec;
>> +	st->st_ctim.tv_nsec = kst->st_ctime_nsec;
>> +	st->st_blksize = kst->st_blksize;
>> +	st->st_blocks = kst->st_blocks;
>> +	return;
>> +}
>
>When I said the memset can be removed I didn't mean to replace it with
>assignments. Although they're not as bad, I don't see any reason
>they're needed at all. The padding does not need to contain meaningful
>content. Just leaving it uninitialized is fine.
>

Ok.

>Also, the return statement is not needed.
>
>> +static inline long __syscall2(long n, long a, long b)
>> +{
>> +	struct kernel_stat kst = {0,};
>
>I didn't catch this before, but is there a reason you're filling kst
>with zeros before passing it to the kernel? I don't think it's useful,
>and it generates extra code the compiler cannot optimize out (because
>it cannot know the kernel won't read the memory). Same applies for all
>the oher __syscallN's.
>

Ok.
Please refer to https://github.com/JaydeepIMG/musl-1 for MIPS64 N64 port. 
I have created mips64port_v3 branch (patch attached) to address the review comments.

>The rest of your changes all look okay.
>
>Since it looks like it's about ready to commit, I have a couple
>general questions about merging. How should the port be credited? To
>imgtec.com? Should someone on your team be the commit author in the
>commit, or should I just commit it in my name with your team credited
>in the commit message and COPYRIGHT file?
>

You can credit it to Imagination Technologies.
Could you please commit it with "Mahesh Bodapati and Jaydeep Patil" in the message?

>Thanks for your work on this and your patience with review. :)
>
>Rich

Thanks for your comments  :)

Regards,
Jaydeep