Re: STMicroelectronics accelerometers driver.

[Denis CIOCCA <denis.ciocca@st.com>]

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