> -----Original Message----- > From: Andy Shevchenko [mailto:andy.shevchenko@xxxxxxxxx] > Sent: Thursday, February 18, 2010 10:18 PM > To: Hebbar, Shivananda > Cc: Menon, Nishanth; linux-omap > Subject: Re: Patch[0/2]:DSPBRIDGE: Excessive u32 Cleanup > > On Thu, Feb 18, 2010 at 10:10 PM, Hebbar, Shivananda <x0hebbar@xxxxxx> > wrote: > >> What is the motivation of this change? > > Sometime back ,there was comment on excessive u32ypes used in bridge. > > http://marc.info/?l=linux-omap&m=123998339213416&w=2 > I respect Artem for his work, but his personal hate of u* types not a > reason to bring some useless patches. > > Look: > [andy@localhost linux-2.6]$ git grep -n -w u32 | wc > 76327 388062 5253203 > > I guess you don't need further exlanations here. > I partly agree. u32 usage change to int needs to be on a need basis. This series is IMHO half hearted if you are attempting to do that. e.g snippets from the patch: from 1/2: u32 uWordSize; /* Size in bytes of DSP word */ hmm.. lets see here.. DSP word uses 4294967295 byes?? - u32 cChannels; /* Total number of channels */ - u32 cOpenChannels; /* Total number of open channels */ + short int cChannels; /* Total number of channels */ + short int cOpenChannels;/* Total number of open channels */ Well.. 4294967295 open channels and same number of open channels mebbe a little too ambitious I can understand.. is 32767 enough either? The patch commit did not tell me about this. struct CHNL_OBJECT **apChannel; /* Array of channels */ - u32 dwType; /* Type of channel class library */ + short int dwType; /* Type of channel class library */ 32767? Why the signedness change? >From 2/2: - u32 uChipType; /* DSP chip type. */ + int uChipType; /* DSP chip type. */ Well.. I am seriously lost here why this change. In short, Artem's comment in: > - u32 temp; > + u32 temp; I'm not 100% sure about this particular case, but overall, the code excessively uses these u32 types. This makse few sense. Unless you work with registers, packets, or something which needs some strict types, use int or long. Use natural C types. Do not try to be too smart, do not try to limit CPU and compiler by u32, if it is not necessary. Makes sense for temp variable, which probably should have been an int or even a char based on max value it might take, but a NAK to this series - would prefer any specific replacements to be available with rationale why it is done.. Regards, Nishanth Menon ��.n��������+%������w��{.n�����{�������ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
