Re: [2/3][PATCH][upstream] TDM Framework

[Timur Tabi <timur@freescale.com>]

Singh Sandeep-B37400 wrote:

>> +int tdm_adap_send(struct tdm_adapter *adap, void **buf, int count) {
>> +       int res;
>> +
>> +       if (adap->algo->tdm_write)
>> +               res = adap->algo->tdm_write(adap, buf, count);
> 
> Why does tdm_write() return a u32?  And shouldn't 'res' also be a u32, to make tdm_write()?
> [Sandeep] tdm_write() returns number of bytes written. You are right, 'res' should be declared as u32

Then it should return an unsigned int.  You should used a sized integer
type only when the size really matters (e.g. hardware registers or packed
fields in a structure).

>> +/* tdm_adapter_mode is to define in mode of the device */ enum 
>> +tdm_adapter_mode {
>> +       TDM_ADAPTER_MODE_NONE = 0x00,
>> +       TDM_ADAPTER_MODE_T1 = 0x01,
>> +       TDM_ADAPTER_MODE_E1 = 0x02,
>> +       TDM_ADAPTER_MODE_T1_RAW = 0x10,
>> +       TDM_ADAPTER_MODE_E1_RAW = 0x20,
> 
> Where did these numbers come from?
> [Sandeep] This is not related to any bit definition, just enum values.

Yes, but why these particular numbers?  Also, do they really need to be an
enum?  Since you're defining hard values for each enum, you're not really
using them as an enum.  Make these into macros.


-- 
Timur Tabi
Linux kernel developer at Freescale