Re: [RFC][PATCH]add Honeywell hmc5843 3-Axis Magnetometer (digital compass) driver.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/01/10 07:40, Datta, Shubhrajyoti wrote:
> 
> nitpick: move comment up to above the function.
>> +       /* Return the measurement value from the  specified channel */
>> +       struct i2c_client *client = to_i2c_client(dev);
>> +       s16 coordinate_val;
>> +       struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
>> +       s32 result;
> nitpick: New line after defs and before code.
>> +       result = i2c_smbus_read_byte_data(client, HMC5843_STATUS_REG);
>> +       while (!(result & DATA_READY))
>> +               result = i2c_smbus_read_byte_data(client, HMC5843_STATUS_REG);
>> +
>> +       result = i2c_smbus_read_word_data(client, this_attr->address);
>> +       if (result > 0) {
>  as coordinate_val is an s16 which not type cast to that?
>> +               coordinate_val  = (int)swab16((u16)result);
> curious line break below...
>> +               return sprintf(buf, "%d\n",
>> +                               coordinate_val);
> So, this is a raw value? (e.g. it will need scaling in userspace)
> 
> 
> Would you prefer to convert in SI units and then report?
That depends on whether it is a linear conversion.  If it is
linear the you can provide additional attributes to allow
userspace to rescale it.  Either option is fine here.
Call it _input if in SI units or _raw if it is a value that needs
conversion.  

Jonathan
> 
>> +       }
>> +       return result;
>> +}
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux