From: Jonathan Cameron <jic23@kernel.org>
To: Manuel Stahl <manuel.stahl@iis.fraunhofer.de>
Cc: Lars-Peter Clausen <lars@metafoo.de>,
Jonathan Cameron <jic23@jic23.retrosnub.co.uk>,
Jonathan Cameron <jic23@cam.ac.uk>,
linux-iio@vger.kernel.org
Subject: Re: [PATCH 10/15] staging:iio:adis16400: Code style cleanup
Date: Wed, 16 Jan 2013 18:34:09 +0000 [thread overview]
Message-ID: <50F6F2A1.6030703@kernel.org> (raw)
In-Reply-To: <201301161509.33730.manuel.stahl@iis.fraunhofer.de>
On 01/16/2013 02:09 PM, Manuel Stahl wrote:
> I must admit that the only thing I found was this one:
> http://yarchive.net/comp/linux/kernel_headers.html
Style guide says you are welcome to use either in new code but
should match whatever is in use in existing code.
Honestly I find myself not caring so unless Manuel minds,
I'll take this patch purely to save Lars-Peter the effort
of regenerating the series with those removed ;)
>
> Am Mittwoch, 16. Januar 2013, 15:07:59 schrieb Lars-Peter Clausen:
>> On 01/16/2013 02:57 PM, Manuel Stahl wrote:
>>> Hi everyone,
>>>
>>> just cought that one up.
>>> I think there was a strong policy to use u8 etc in the kernel. Linus wrote something about it some time ago.
>>>
>>
>> That's what the advocates of u8, etc like to tell ;)
>>
>>
>>> Am Mittwoch, 16. Januar 2013, 14:11:24 schrieb Lars-Peter Clausen:
>>>> On 01/16/2013 01:58 PM, Jonathan Cameron wrote:
>>>>> On 16/01/13 12:48, Lars-Peter Clausen wrote:
>>>>>> Do a set of minor miscellaneous code style cleanups for the adis16400 before
>>>>>> moving it out of staging. Delete outdated comments, removed excess
>>>>>> whitespace,
>>>>>> add missing whitespace, replace u{8,16} with uint{8,16}_t.
>>>>>
>>>>> Is there a move that I've missed to move away from u8 and friends?
>>>>
>>>> Unfortunately not. I think the policy is more or less use whichever you
>>>> like. But I definitely prefer uint16_t and friends since it's the official C
>>>> type for that type. Also most of the other adis drivers including the
>>>> library use uint{8,16,32}_t so it's only consistent.
>>>>
>>>> I wouldn't have done this if it had been the only style issue, but since the
>>>> file needed a cleanup anyway I thought I'd include this as well.
>>>>
>>>> - Lars
>>>>
>>>>>>
>>>>>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>>>>>> ---
>>>>>> drivers/staging/iio/imu/adis16400_core.c | 81
>>>>>> +++++++++++++++-----------------
>>>>>> 1 file changed, 37 insertions(+), 44 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/staging/iio/imu/adis16400_core.c
>>>>>> b/drivers/staging/iio/imu/adis16400_core.c
>>>>>> index c08490b..1bbe5ee 100644
>>>>>> --- a/drivers/staging/iio/imu/adis16400_core.c
>>>>>> +++ b/drivers/staging/iio/imu/adis16400_core.c
>>>>>> @@ -45,7 +45,7 @@ enum adis16400_chip_variant {
>>>>>> static int adis16334_get_freq(struct adis16400_state *st)
>>>>>> {
>>>>>> int ret;
>>>>>> - u16 t;
>>>>>> + uint16_t t;
>>>>>>
>>>>>> ret = adis_read_reg_16(&st->adis, ADIS16400_SMPL_PRD, &t);
>>>>>> if (ret < 0)
>>>>>> @@ -78,12 +78,13 @@ static int adis16334_set_freq(struct adis16400_state
>>>>>> *st, unsigned int freq)
>>>>>> static int adis16400_get_freq(struct adis16400_state *st)
>>>>>> {
>>>>>> int sps, ret;
>>>>>> - u16 t;
>>>>>> + uint16_t t;
>>>>>>
>>>>>> ret = adis_read_reg_16(&st->adis, ADIS16400_SMPL_PRD, &t);
>>>>>> if (ret < 0)
>>>>>> return ret;
>>>>>> - sps = (t & ADIS16400_SMPL_PRD_TIME_BASE) ? 53 : 1638;
>>>>>> +
>>>>>> + sps = (t & ADIS16400_SMPL_PRD_TIME_BASE) ? 53 : 1638;
>>>>>> sps /= (t & ADIS16400_SMPL_PRD_DIV_MASK) + 1;
>>>>>>
>>>>>> return sps;
>>>>>> @@ -97,6 +98,7 @@ static int adis16400_set_freq(struct adis16400_state
>>>>>> *st, unsigned int freq)
>>>>>> if (t > 0)
>>>>>> t--;
>>>>>> t &= ADIS16400_SMPL_PRD_DIV_MASK;
>>>>>> +
>>>>>> if ((t & ADIS16400_SMPL_PRD_DIV_MASK) >= 0x0A)
>>>>>> st->adis.spi->max_speed_hz = ADIS16400_SPI_SLOW;
>>>>>> else
>>>>>> @@ -111,13 +113,13 @@ static ssize_t adis16400_read_frequency(struct
>>>>>> device *dev,
>>>>>> {
>>>>>> struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>>>>> struct adis16400_state *st = iio_priv(indio_dev);
>>>>>> - int ret, len = 0;
>>>>>> + int ret;
>>>>>>
>>>>>> ret = st->variant->get_freq(st);
>>>>>> if (ret < 0)
>>>>>> return ret;
>>>>>> - len = sprintf(buf, "%d\n", ret);
>>>>>> - return len;
>>>>>> +
>>>>>> + return sprintf(buf, "%d\n", ret);
>>>>>> }
>>>>>>
>>>>>> static const unsigned adis16400_3db_divisors[] = {
>>>>>> @@ -134,8 +136,8 @@ static const unsigned adis16400_3db_divisors[] = {
>>>>>> static int adis16400_set_filter(struct iio_dev *indio_dev, int sps, int
>>>>>> val)
>>>>>> {
>>>>>> struct adis16400_state *st = iio_priv(indio_dev);
>>>>>> + uint16_t val16;
>>>>>> int i, ret;
>>>>>> - u16 val16;
>>>>>>
>>>>>> for (i = ARRAY_SIZE(adis16400_3db_divisors) - 1; i >= 1; i--) {
>>>>>> if (sps / adis16400_3db_divisors[i] >= val)
>>>>>> @@ -152,26 +154,22 @@ static int adis16400_set_filter(struct iio_dev
>>>>>> *indio_dev, int sps, int val)
>>>>>> }
>>>>>>
>>>>>> static ssize_t adis16400_write_frequency(struct device *dev,
>>>>>> - struct device_attribute *attr,
>>>>>> - const char *buf,
>>>>>> - size_t len)
>>>>>> + struct device_attribute *attr, const char *buf, size_t len)
>>>>>> {
>>>>>> struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>>>>> struct adis16400_state *st = iio_priv(indio_dev);
>>>>>> long val;
>>>>>> int ret;
>>>>>>
>>>>>> - ret = strict_strtol(buf, 10, &val);
>>>>>> + ret = kstrtol(buf, 10, &val);
>>>>>> if (ret)
>>>>>> return ret;
>>>>>> +
>>>>>> if (val == 0)
>>>>>> return -EINVAL;
>>>>>>
>>>>>> mutex_lock(&indio_dev->mlock);
>>>>>> -
>>>>>> st->variant->set_freq(st, val);
>>>>>> -
>>>>>> - /* Also update the filter */
>>>>>> mutex_unlock(&indio_dev->mlock);
>>>>>>
>>>>>> return ret ? ret : len;
>>>>>> @@ -182,9 +180,9 @@ static int adis16400_stop_device(struct iio_dev
>>>>>> *indio_dev)
>>>>>> {
>>>>>> struct adis16400_state *st = iio_priv(indio_dev);
>>>>>> int ret;
>>>>>> - u16 val = ADIS16400_SLP_CNT_POWER_OFF;
>>>>>>
>>>>>> - ret = adis_write_reg_16(&st->adis, ADIS16400_SLP_CNT, val);
>>>>>> + ret = adis_write_reg_16(&st->adis, ADIS16400_SLP_CNT,
>>>>>> + ADIS16400_SLP_CNT_POWER_OFF);
>>>>>> if (ret)
>>>>>> dev_err(&indio_dev->dev,
>>>>>> "problem with turning device off: SLP_CNT");
>>>>>> @@ -194,10 +192,10 @@ static int adis16400_stop_device(struct iio_dev
>>>>>> *indio_dev)
>>>>>>
>>>>>> static int adis16400_initial_setup(struct iio_dev *indio_dev)
>>>>>> {
>>>>>> - int ret;
>>>>>> - u16 prod_id, smp_prd;
>>>>>> - unsigned int device_id;
>>>>>> struct adis16400_state *st = iio_priv(indio_dev);
>>>>>> + uint16_t prod_id, smp_prd;
>>>>>> + unsigned int device_id;
>>>>>> + int ret;
>>>>>>
>>>>>> /* use low spi speed for init if the device has a slow mode */
>>>>>> if (st->variant->flags & ADIS16400_HAS_SLOW_MODE)
>>>>>> @@ -224,8 +222,8 @@ static int adis16400_initial_setup(struct iio_dev
>>>>>> *indio_dev)
>>>>>> device_id, prod_id);
>>>>>>
>>>>>> dev_info(&indio_dev->dev, "%s: prod_id 0x%04x at CS%d (irq %d)\n",
>>>>>> - indio_dev->name, prod_id,
>>>>>> - st->adis.spi->chip_select, st->adis.spi->irq);
>>>>>> + indio_dev->name, prod_id,
>>>>>> + st->adis.spi->chip_select, st->adis.spi->irq);
>>>>>> }
>>>>>> /* use high spi speed if possible */
>>>>>> if (st->variant->flags & ADIS16400_HAS_SLOW_MODE) {
>>>>>> @@ -247,7 +245,7 @@ static IIO_DEV_ATTR_SAMP_FREQ(S_IWUSR | S_IRUGO,
>>>>>> adis16400_read_frequency,
>>>>>> adis16400_write_frequency);
>>>>>>
>>>>>> -static const u8 adis16400_addresses[] = {
>>>>>> +static const uint8_t adis16400_addresses[] = {
>>>>>> [ADIS16400_SCAN_GYRO_X] = ADIS16400_XGYRO_OFF,
>>>>>> [ADIS16400_SCAN_GYRO_Y] = ADIS16400_YGYRO_OFF,
>>>>>> [ADIS16400_SCAN_GYRO_Z] = ADIS16400_ZGYRO_OFF,
>>>>>> @@ -257,15 +255,12 @@ static const u8 adis16400_addresses[] = {
>>>>>> };
>>>>>>
>>>>>> static int adis16400_write_raw(struct iio_dev *indio_dev,
>>>>>> - struct iio_chan_spec const *chan,
>>>>>> - int val,
>>>>>> - int val2,
>>>>>> - long mask)
>>>>>> + struct iio_chan_spec const *chan, int val, int val2, long info)
>>>>>> {
>>>>>> struct adis16400_state *st = iio_priv(indio_dev);
>>>>>> int ret, sps;
>>>>>>
>>>>>> - switch (mask) {
>>>>>> + switch (info) {
>>>>>> case IIO_CHAN_INFO_CALIBBIAS:
>>>>>> mutex_lock(&indio_dev->mlock);
>>>>>> ret = adis_write_reg_16(&st->adis,
>>>>>> @@ -273,8 +268,10 @@ static int adis16400_write_raw(struct iio_dev
>>>>>> *indio_dev,
>>>>>> mutex_unlock(&indio_dev->mlock);
>>>>>> return ret;
>>>>>> case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
>>>>>> - /* Need to cache values so we can update if the frequency
>>>>>> - changes */
>>>>>> + /*
>>>>>> + * Need to cache values so we can update if the frequency
>>>>>> + * changes.
>>>>>> + */
>>>>>> mutex_lock(&indio_dev->mlock);
>>>>>> st->filt_int = val;
>>>>>> /* Work out update to current value */
>>>>>> @@ -293,16 +290,13 @@ static int adis16400_write_raw(struct iio_dev
>>>>>> *indio_dev,
>>>>>> }
>>>>>>
>>>>>> static int adis16400_read_raw(struct iio_dev *indio_dev,
>>>>>> - struct iio_chan_spec const *chan,
>>>>>> - int *val,
>>>>>> - int *val2,
>>>>>> - long mask)
>>>>>> + struct iio_chan_spec const *chan, int *val, int *val2, long info)
>>>>>> {
>>>>>> struct adis16400_state *st = iio_priv(indio_dev);
>>>>>> + int16_t val16;
>>>>>> int ret;
>>>>>> - s16 val16;
>>>>>>
>>>>>> - switch (mask) {
>>>>>> + switch (info) {
>>>>>> case IIO_CHAN_INFO_RAW:
>>>>>> return adis_single_conversion(indio_dev, chan, 0, val);
>>>>>> case IIO_CHAN_INFO_SCALE:
>>>>>> @@ -721,13 +715,14 @@ static const struct adis_data adis16400_data = {
>>>>>>
>>>>>> static int adis16400_probe(struct spi_device *spi)
>>>>>> {
>>>>>> - int ret;
>>>>>> struct adis16400_state *st;
>>>>>> - struct iio_dev *indio_dev = iio_device_alloc(sizeof(*st));
>>>>>> - if (indio_dev == NULL) {
>>>>>> - ret = -ENOMEM;
>>>>>> - goto error_ret;
>>>>>> - }
>>>>>> + struct iio_dev *indio_dev;
>>>>>> + int ret;
>>>>>> +
>>>>>> + indio_dev = iio_device_alloc(sizeof(*st));
>>>>>> + if (indio_dev == NULL)
>>>>>> + return -ENOMEM;
>>>>>> +
>>>>>> st = iio_priv(indio_dev);
>>>>>> /* this is only used for removal purposes */
>>>>>> spi_set_drvdata(spi, indio_dev);
>>>>>> @@ -767,14 +762,12 @@ error_cleanup_buffer:
>>>>>> adis_cleanup_buffer_and_trigger(&st->adis, indio_dev);
>>>>>> error_free_dev:
>>>>>> iio_device_free(indio_dev);
>>>>>> -error_ret:
>>>>>> return ret;
>>>>>> }
>>>>>>
>>>>>> -/* fixme, confirm ordering in this function */
>>>>>> static int adis16400_remove(struct spi_device *spi)
>>>>>> {
>>>>>> - struct iio_dev *indio_dev = spi_get_drvdata(spi);
>>>>>> + struct iio_dev *indio_dev = spi_get_drvdata(spi);
>>>>>> struct adis16400_state *st = iio_priv(indio_dev);
>>>>>>
>>>>>> iio_device_unregister(indio_dev);
>>>>>>
>>>>>
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>>
>>>
>>>
>>
>>
>
>
next prev parent reply other threads:[~2013-01-16 18:34 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-16 12:48 [PATCH 01/15] staging:iio:adis16400: Don't pass 0 to ilog2 Lars-Peter Clausen
2013-01-16 12:48 ` [PATCH 02/15] staging:iio:adis16400: Fix and cleanup 3db filter setting Lars-Peter Clausen
2013-01-16 12:48 ` [PATCH 03/15] staging:iio:adis16400: Remove unused default_scan_mask Lars-Peter Clausen
2013-01-16 12:48 ` [PATCH 04/15] staging:iio:adis16400: Use adis library Lars-Peter Clausen
2013-01-16 12:48 ` [PATCH 05/15] staging:iio:adis16400: Use triggered buffer setup helper function Lars-Peter Clausen
2013-01-16 12:48 ` [PATCH 06/15] staging:iio:adis16400: Add helper macros for channel declaration Lars-Peter Clausen
2013-01-16 12:48 ` [PATCH 07/15] staging:iio:adis16400: Preallocate transfer message Lars-Peter Clausen
2013-01-16 12:48 ` [PATCH 08/15] staging:iio:adis16400: Remove unit suffix from samplerate attribute Lars-Peter Clausen
2013-01-16 12:48 ` [PATCH 09/15] staging:iio:adis16400: Remove samplerate_available attribute Lars-Peter Clausen
2013-01-16 12:48 ` [PATCH 10/15] staging:iio:adis16400: Code style cleanup Lars-Peter Clausen
2013-01-16 12:58 ` Jonathan Cameron
2013-01-16 13:11 ` Lars-Peter Clausen
2013-01-16 13:57 ` Manuel Stahl
2013-01-16 14:07 ` Lars-Peter Clausen
2013-01-16 14:09 ` Manuel Stahl
2013-01-16 18:34 ` Jonathan Cameron [this message]
2013-01-17 9:18 ` Manuel Stahl
2013-01-16 12:48 ` [PATCH 11/15] staging:iio: Move adis16400 out of staging Lars-Peter Clausen
2013-01-16 12:48 ` [PATCH 12/15] iio:adis16400: Increase samplerate precession Lars-Peter Clausen
2013-01-16 12:48 ` [PATCH 13/15] iio:adis16400: Add support for the 52.85 Hz base sampling rate Lars-Peter Clausen
2013-01-16 12:48 ` [PATCH 14/15] iio:adis16400: Expose some debug information in debugfs Lars-Peter Clausen
2013-01-16 12:48 ` [PATCH 15/15] iio:adis16400: Add support for the adis16448 Lars-Peter Clausen
2013-01-19 13:50 ` [PATCH 01/15] staging:iio:adis16400: Don't pass 0 to ilog2 Jonathan Cameron
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=50F6F2A1.6030703@kernel.org \
--to=jic23@kernel.org \
--cc=jic23@cam.ac.uk \
--cc=jic23@jic23.retrosnub.co.uk \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
--cc=manuel.stahl@iis.fraunhofer.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.