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