Re: STMicroelectronics accelerometers driver.

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

Hi Jonathan,

can you please provide me one direction to modify the part in question=20
about the style of struct, in such a way I can do some modifications.

Thanks,

Denis Ciocca



On 11/06/2012 12:11 PM, Denis Ciocca wrote:
> Hi Jonathan,
>
> Thanks for your support!
> I have applied your comments, only two point I would check better.
>
>
>
>>> +} st_accel_sensors[] =3D {
>>> +     {
>>> +             .wai =3D ST_ACCEL_1_WAI_EXP,
>>> +             .ch =3D (struct iio_chan_spec *)st_accel_12bit_channels,
>>> +             .odr =3D {
>>> +                     .addr =3D ST_ACCEL_1_ODR_ADDR,
>>> +                     .mask =3D ST_ACCEL_1_ODR_MASK,
>>> +                     .num_bit =3D ST_ACCEL_1_ODR_N_BIT,
>>> +                     .odr_avl =3D {
>>    While sometimes c99 assignment helps with clarity - sometimes it
>> just bloats the code.
>>    Personally I'd do these as
>>    .odr_avl =3D {{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 =3D {{1, 0x01},
>>                   {10, 0x02},
>> etc.
>> Or maybe if it fits on the line
>>     .odr_avl =3D {{ .hz =3D 1, .value =3D 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 =3D {
>  >        [0] =3D {
>  >                ...
>  >        },
>  >        [1] =3D {
>  >                ...
>  >
>  >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 row=
s.
>
>
>
>>> +/**
>>> + * 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
>
>
>
>
>
>
>
>
>
>