Hi Jonathan,
Thanks for your support!
I have applied your comments, only two point I would check better.
>> +} st_accel_sensors[] = {
>> + {
>> + .wai = ST_ACCEL_1_WAI_EXP,
>> + .ch = (struct iio_chan_spec *)st_accel_12bit_channels,
>> + .odr = {
>> + .addr = ST_ACCEL_1_ODR_ADDR,
>> + .mask = ST_ACCEL_1_ODR_MASK,
>> + .num_bit = ST_ACCEL_1_ODR_N_BIT,
>> + .odr_avl = {
> While sometimes c99 assignment helps with clarity - sometimes it just bloats the code.
> Personally I'd do these as
> .odr_avl = {{ST_ACCEL_ODR_AVL_1HZ, ST_ACCEL_1_ODR_AVL_1HZ_VAL},
> {...}
> etc
>
> or for that matter take the view that the defines are largely pointless as they
> are only used here and
> just do
> .odr_avl = {{1, 0x01},
> {10, 0x02},
> etc.
> Or maybe if it fits on the line
> .odr_avl = {{ .hz = 1, .value = 0x01 } might be the clearest... Not sure.
>
In my first version I have used about this compact approach but
Lars-Peter tell to me to use this style code:
>I'd use
> .odr_avl = {
> [0] = {
> ...
> },
> [1] = {
> ...
>
>Makes things a bit more readable in my opinion.
Ok for me it is not a problem but you must tell me one definitive style
please. I think this style is more cleanly, though are necessary more rows.
>> +/**
>> + * struct st_accel_platform_data - ST accel device platform data
>> + * @fullscale: Value of fullscale used for the sensor.
>> + * @sampling_frequency: Value of sampling frequency used for the sensor.
>> + */
>> +
>> +struct st_accel_platform_data {
>> + int fullscale;
>> + int sampling_frequency;
>> +};
>
> What is the purpose of having these as platform data?
>
For now the purpose is to set only the initial values for fullscale and
sampling_frequency but I think it's good thing include the mapping of
axis. You think it's necessary to remove this for now?
Thanks,
Denis
--
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
