> -----Original Message----- > From: David Daney [mailto:ddaney.cavm@xxxxxxxxx] > Sent: 06 December 2013 16:48 > To: Qais Yousef > Cc: linux-mips@xxxxxxxxxxxxxx > Subject: Re: [PATCH] mips/include/asm/mipsregs.h: s/u16/unsigned short/ > > On 12/06/2013 08:35 AM, Qais Yousef wrote: > >> -----Original Message----- > >> From: David Daney [mailto:ddaney.cavm@xxxxxxxxx] > >> Sent: 06 December 2013 16:32 > >> To: Qais Yousef > >> Cc: linux-mips@xxxxxxxxxxxxxx > >> Subject: Re: [PATCH] mips/include/asm/mipsregs.h: s/u16/unsigned > >> short/ > >> > >> On 12/06/2013 01:20 AM, Qais Yousef wrote: > >>> I was getting this error when including this header in my driver: > >>> > >>> arch/mips/include/asm/mipsregs.h:644:33: error: unknown type name > ‘u16’ > >>> > >>> since the use of u16 is not really necessary, convert it to unsigned short. > >>> > >>> Signed-off-by: Qais Yousef <qais.yousef@xxxxxxxxxx> > >>> Reviewed-by: Steven J. Hill <Steven.Hill@xxxxxxxxxx> > >> > >> NAK. > >> > >> Just #include <linux/types.h> at the top of asm/mipsregs.h instead. > > > > Funnily that was my first solution before I changed it to this :) > > > > I'll resend but can you please give some explanation why changing u16 to > unsigned short is bad? > > This is the linux kernel. People expect to see fixed width integer type definitions > using the conventional u8, u16, u32, etc. > > If you are doing something tricky enough that you need to explicitly use a type of > a given width, don't hide the fact, bring it to our attention by using the kernel > standard type. > > If you don't need exactly a u16, just make it an unsigned int and be done with it. > > It would appear that micro-MIPS instructions are 16 bit, so use u16 everywhere > for them. OK thanks for the explanation. u16 is more safe and future proof for sure. > > Also it looks like this function really should be declared as returning type bool, not > int. For the same reason: It cannot return any integer, only truth values. Don't > hide this fact. Bring it to our attention by using the proper types. I share this view about Booleans to be honest http://article.gmane.org/gmane.linux.kernel/1554183/match=bool v2 is on the way. Thanks, Qais > > > David Daney > > > > > > Thanks, > > Qais > > > >> > >> David Daney > >> > >> > >>> --- > >>> arch/mips/include/asm/mipsregs.h | 4 ++-- > >>> 1 files changed, 2 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/arch/mips/include/asm/mipsregs.h > >> b/arch/mips/include/asm/mipsregs.h > >>> index e033141..0a2d6ef 100644 > >>> --- a/arch/mips/include/asm/mipsregs.h > >>> +++ b/arch/mips/include/asm/mipsregs.h > >>> @@ -641,9 +641,9 @@ > >>> * microMIPS instructions can be 16-bit or 32-bit in length. This > >>> * returns a 1 if the instruction is 16-bit and a 0 if 32-bit. > >>> */ > >>> -static inline int mm_insn_16bit(u16 insn) > >>> +static inline int mm_insn_16bit(unsigned short insn) > >>> { > >>> - u16 opcode = (insn >> 10) & 0x7; > >>> + unsigned short opcode = (insn >> 10) & 0x7; > >>> > >>> return (opcode >= 1 && opcode <= 3) ? 1 : 0; > >>> } > >>> > >
