Hi Jonathan,
can you please provide me one direction to modify the part in question
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[] = {
>>> + {
>>> + .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
