From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <denis.ciocca@st.com>
From: Denis CIOCCA <denis.ciocca@st.com>
To: Jonathan Cameron <jic23@kernel.org>
Cc: Denis Ciocca <denis.ciocca@gmail.com>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Jonathan Cameron <jic23@jic23.retrosnub.co.uk>,
	Pavel Machek <pavel@denx.de>,
	"linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>,
	"burman.yan@gmail.com" <burman.yan@gmail.com>
Date: Mon, 12 Nov 2012 18:10:36 +0100
Subject: Re: STMicroelectronics accelerometers driver.
Message-ID: <50A12D8C.6040207@st.com>
References: <5072F3B9.4050309@st.com> <5073260C.3010506@metafoo.de>
 <20121008195007.GA32042@elf.ucw.cz> <507338B5.60307@metafoo.de>
 <665718a4-de6b-4d09-ba80-35705fcf2595@email.android.com>
 <CAEE_umojW5OyB3yGgE54wH7_k__dBu9=Yew_0tCCf5--=C2g5g@mail.gmail.com>
 <507D9EA3.4070009@metafoo.de> <5085128C.7020507@st.com>
 <50858B44.2080006@kernel.org> <5087E2A8.4070304@st.com>
 <508A7DA6.7050505@metafoo.de> <508E4492.8080000@st.com>
 <508E48D1.3000703@metafoo.de> <508E5978.5000907@st.com>
 <508E5AD5.1040907@metafoo.de> <50914366.4090000@st.com>
 <5091546D.9000601@metafoo.de>
 <CAEE_umoKvv9ciBfUSahDqsmcbHi9W=93o5YSpE-vxUduCmqWSQ@mail.gmail.com>
 <50982F71.3060606@kernel.org> <5098F065.1090009@st.com>
In-Reply-To: <5098F065.1090009@st.com>
Content-Type: text/plain; charset="iso-8859-1"
MIME-Version: 1.0
List-ID: <linux-iio@vger.kernel.org>

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
>
>
>
>
>
>
>
>
>
>