Received: by 10.213.65.68 with SMTP id h4csp537766imn; Sat, 17 Mar 2018 13:41:35 -0700 (PDT) X-Google-Smtp-Source: AG47ELttIz/qbmqL5HgVj04NhtirCZCH5jmAWFKf3xLZfmQiCwYj7g22J0B01Lemz+n0B1e/UAft X-Received: by 2002:a17:902:3183:: with SMTP id x3-v6mr6827799plb.383.1521319295000; Sat, 17 Mar 2018 13:41:35 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1521319294; cv=none; d=google.com; s=arc-20160816; b=D0TwszuMPm8soYQQLX+xJ76eZXFJYZEXJEesiU1pJs65KGigvKJE9U6N32hcTqngZp zedwsv/Zcee2vIGxZdeaLNNpP37WepsLm19KeztKXgCcShGuDDaTl1FJ1kjFRXGwOQD5 TIObTSTqFq31YuUrBvwlbKKIIwB8NCBWYNWUHrVJYEYwA9gimzR+8qwEkNsnv+A3og7E r03zMcFRKMZotMlF/c8a5CK/Or4H8pVyfkthjwEdOmhL9XTpoxHfdFpUqdE+D10dSIBs MY/F97KmOcdC3/91URDQKW1d58TFA3NbwXXXyePIccMQO9O61ujwKqp2ZK6CrQWiuzn1 Fj0g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :references:in-reply-to:message-id:subject:cc:to:from:date :dmarc-filter:arc-authentication-results; bh=Lf/JPrHX2AH4Ig4C+ayy3nzfKWu5WrssAr4jBLuBtRg=; b=IPuByDwc0EgFDqD5PHYDL8vkPOurl+OZEJIy5eNaDsaVxdpQiB715MhN1/jvlfzTa0 h5qx+wBldKQvDpthox6sZTAWzFcjlk5UkbDSJALklOJblX4rBhVyVnrg7Y6b73/jgm/t UBcIiJafSof69AMBXM4eE3mhYfwvyXEekKEG/e1oxICFEKTq/XTLCIT9ffJmsZdZqUhp lUtd1t6VMbUKj0x0k97tJcI7Su4yWVuTsw916fCCq5H+8o9TuyKBgNbhw5tuf5ouTn6M plRdGLUzVkeCn9CwQt9NM58n++/GTUoUm2MsB5Ifo5wbIKd2JrYypPXAMBndqIfaCbLH AwFA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id q10si7156509pgr.126.2018.03.17.13.41.21; Sat, 17 Mar 2018 13:41:34 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752755AbeCQUjB convert rfc822-to-8bit (ORCPT + 99 others); Sat, 17 Mar 2018 16:39:01 -0400 Received: from mail.kernel.org ([198.145.29.99]:55206 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752591AbeCQUi4 (ORCPT ); Sat, 17 Mar 2018 16:38:56 -0400 Received: from archlinux (cpc91196-cmbg18-2-0-cust659.5-4.cable.virginm.net [81.96.234.148]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id F344421720; Sat, 17 Mar 2018 20:38:53 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org F344421720 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=jic23@kernel.org Date: Sat, 17 Mar 2018 20:38:50 +0000 From: Jonathan Cameron To: Shreeya Patel Cc: lars@metafoo.de, Michael.Hennerich@analog.com, knaack.h@gmx.de, pmeerw@pmeerw.net, gregkh@linuxfoundation.org, linux-iio@vger.kernel.org, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org, daniel.baluta@gmail.com Subject: Re: [PATCH v2] Staging: iio: adis16209: Move adis16209 driver out of staging Message-ID: <20180317203850.3da4cd87@archlinux> In-Reply-To: References: <1520677223-11505-1-git-send-email-shreeya.patel23498@gmail.com> <20180310155731.7b7f584d@archlinux> <1521140513.7627.10.camel@gmail.com> X-Mailer: Claws Mail 3.16.0 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 16 Mar 2018 02:33:49 +0530 Shreeya Patel wrote: > On 16 March 2018 00:31:53 GMT+05:30, Shreeya Patel wrote: > >On Sat, 2018-03-10 at 15:57 +0000, Jonathan Cameron wrote: > > > >Hi Jonathan, > > > >> On Sat, 10 Mar 2018 15:50:23 +0530 > >> Shreeya Patel wrote: > >> > >> > > >> > Move the adis16209 driver out of staging directory and merge to the > >> > mainline IIO subsystem. > >> > > >> > Signed-off-by: Shreeya Patel > >> As this has a clear dependency on the previous patch, please put them > >> in the same series for the next version.  That way I won't miss one! > >> > >> This also doesn't actually seem to have all the patches in place. > >> The sign extend one is definitely missing for some reason. > >> > >> One question on the ABI choice of X for the rotation axis. > >> I think the logical choice is actually Z but would like to know what > >> you and others think. > >> > >> All existing users of IIO_ROT (outside staging) have been > >> magnetometer > >> where we don't have an axis, but rather a magnetic reference frame or > >> a quaternion output which includes all the axes. > >> > >> Jonathan > >> > >> > > >> > --- > >> > > >> > Changes in v2 > >> >   -Re-send the patch after having some cleanups in the > >> > file included in this patch. > >> > > >> >  drivers/iio/accel/Kconfig             |  12 ++ > >> >  drivers/iio/accel/Makefile            |   1 + > >> >  drivers/iio/accel/adis16209.c         | 329 > >> > ++++++++++++++++++++++++++++++++++ > >> >  drivers/staging/iio/accel/Kconfig     |  12 -- > >> >  drivers/staging/iio/accel/Makefile    |   1 - > >> >  drivers/staging/iio/accel/adis16209.c | 329 ---------------------- > >> > ------------ > >> >  6 files changed, 342 insertions(+), 342 deletions(-) > >> >  create mode 100644 drivers/iio/accel/adis16209.c > >> >  delete mode 100644 drivers/staging/iio/accel/adis16209.c > >> > > >> > diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig > >> > index c6d9517..f95f43c 100644 > >> > --- a/drivers/iio/accel/Kconfig > >> > +++ b/drivers/iio/accel/Kconfig > >> > @@ -5,6 +5,18 @@ > >> >   > >> >  menu "Accelerometers" > >> >   > >> > +config ADIS16209 > >> > +        tristate "Analog Devices ADIS16209 Dual-Axis Digital > >> > Inclinometer and Accelerometer" > >> > +        depends on SPI > >> > +        select IIO_ADIS_LIB > >> > +        select IIO_ADIS_LIB_BUFFER if IIO_BUFFER > >> > +        help > >> > +          Say Y here to build support for Analog Devices adis16209 > >> > dual-axis digital inclinometer > >> > +          and accelerometer. > >> > + > >> > +          To compile this driver as a module, say M here: the > >> > module will be > >> > +          called adis16209. > >> > + > >> >  config ADXL345 > >> >   tristate > >> >   > >> > diff --git a/drivers/iio/accel/Makefile > >> > b/drivers/iio/accel/Makefile > >> > index 368aedb..40861b9 100644 > >> > --- a/drivers/iio/accel/Makefile > >> > +++ b/drivers/iio/accel/Makefile > >> > @@ -4,6 +4,7 @@ > >> >  # > >> >   > >> >  # When adding new entries keep the list in alphabetical order > >> > +obj-$(CONFIG_ADIS16209) += adis16209.o > >> >  obj-$(CONFIG_ADXL345) += adxl345_core.o > >> >  obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o > >> >  obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o > >> > diff --git a/drivers/iio/accel/adis16209.c > >> > b/drivers/iio/accel/adis16209.c > >> > new file mode 100644 > >> > index 0000000..ed2e89f > >> > --- /dev/null > >> > +++ b/drivers/iio/accel/adis16209.c > >> > @@ -0,0 +1,329 @@ > >> > +/* > >> > + * ADIS16209 Dual-Axis Digital Inclinometer and Accelerometer > >> > + * > >> > + * Copyright 2010 Analog Devices Inc. > >> > + * > >> > + * Licensed under the GPL-2 or later. > >> > + */ > >> > + > >> > +#include > >> > +#include > >> > +#include > >> > +#include > >> > +#include > >> > +#include > >> > +#include > >> > +#include > >> > + > >> > +#include > >> > +#include > >> > +#include > >> > +#include > >> > + > >> > +#define ADIS16209_STARTUP_DELAY_MS 220 > >> > +#define ADIS16209_FLASH_CNT_REG 0x00 > >> > + > >> > +/* Data Output Register Definitions */ > >> > +#define ADIS16209_SUPPLY_OUT_REG 0x02 > >> > +#define ADIS16209_XACCL_OUT_REG 0x04 > >> > +#define ADIS16209_YACCL_OUT_REG 0x06 > >> > +/* Output, auxiliary ADC input */ > >> > +#define ADIS16209_AUX_ADC_REG 0x08 > >> > +/* Output, temperature */ > >> > +#define ADIS16209_TEMP_OUT_REG 0x0A > >> > +/* Output, +/- 90 degrees X-axis inclination */ > >> > +#define ADIS16209_XINCL_OUT_REG 0x0C > >> > +#define ADIS16209_YINCL_OUT_REG 0x0E > >> > +/* Output, +/-180 vertical rotational position */ > >> > +#define ADIS16209_ROT_OUT_REG 0x10 > >> > + > >> > +/* > >> > + * Calibration Register Definitions. > >> > + * Acceleration, inclination or rotation offset null. > >> > + */ > >> > +#define ADIS16209_XACCL_NULL_REG 0x12 > >> > +#define ADIS16209_YACCL_NULL_REG 0x14 > >> > +#define ADIS16209_XINCL_NULL_REG 0x16 > >> > +#define ADIS16209_YINCL_NULL_REG 0x18 > >> > +#define ADIS16209_ROT_NULL_REG 0x1A > >> > + > >> > +/* Alarm Register Definitions */ > >> > +#define ADIS16209_ALM_MAG1_REG 0x20 > >> > +#define ADIS16209_ALM_MAG2_REG 0x22 > >> > +#define ADIS16209_ALM_SMPL1_REG 0x24 > >> > +#define ADIS16209_ALM_SMPL2_REG 0x26 > >> > +#define ADIS16209_ALM_CTRL_REG 0x28 > >> > + > >> > +#define ADIS16209_AUX_DAC_REG 0x30 > >> > +#define ADIS16209_GPIO_CTRL_REG 0x32 > >> > +#define ADIS16209_SMPL_PRD_REG 0x36 > >> > +#define ADIS16209_AVG_CNT_REG 0x38 > >> > +#define ADIS16209_SLP_CNT_REG 0x3A > >> > + > >> > +#define ADIS16209_MSC_CTRL_REG 0x34 > >> > +#define  ADIS16209_MSC_CTRL_PWRUP_SELF_TEST BIT(10) > >> > +#define  ADIS16209_MSC_CTRL_SELF_TEST_EN BIT(8) > >> > +#define  ADIS16209_MSC_CTRL_DATA_RDY_EN BIT(2) > >> > +/* Data-ready polarity: 1 = active high, 0 = active low */ > >> > +#define  ADIS16209_MSC_CTRL_ACTIVE_HIGH BIT(1) > >> > +#define  ADIS16209_MSC_CTRL_DATA_RDY_DIO2 BIT(0) > >> > + > >> > +#define ADIS16209_STAT_REG 0x3C > >> > +#define  ADIS16209_STAT_ALARM2 BIT(9) > >> > +#define  ADIS16209_STAT_ALARM1 BIT(8) > >> > +#define  ADIS16209_STAT_SELFTEST_FAIL_BIT 5 > >> > +#define  ADIS16209_STAT_SPI_FAIL_BIT 3 > >> > +#define  ADIS16209_STAT_FLASH_UPT_FAIL_BIT 2 > >> > +/* Power supply above 3.625 V */ > >> > +#define  ADIS16209_STAT_POWER_HIGH_BIT 1 > >> > +/* Power supply below 3.15 V */ > >> > +#define  ADIS16209_STAT_POWER_LOW_BIT 0 > >> > + > >> > +#define ADIS16209_CMD_REG 0x3E > >> > +#define  ADIS16209_CMD_SW_RESET BIT(7) > >> > +#define  ADIS16209_CMD_CLEAR_STAT BIT(4) > >> > +#define  ADIS16209_CMD_FACTORY_CAL BIT(1) > >> > + > >> > +#define ADIS16209_ERROR_ACTIVE BIT(14) > >> > + > >> > +enum adis16209_scan { > >> > + ADIS16209_SCAN_SUPPLY, > >> > + ADIS16209_SCAN_ACC_X, > >> > + ADIS16209_SCAN_ACC_Y, > >> > + ADIS16209_SCAN_AUX_ADC, > >> > + ADIS16209_SCAN_TEMP, > >> > + ADIS16209_SCAN_INCLI_X, > >> > + ADIS16209_SCAN_INCLI_Y, > >> > + ADIS16209_SCAN_ROT, > >> > +}; > >> > + > >> > +static const u8 adis16209_addresses[8][1] = { > >> > + [ADIS16209_SCAN_SUPPLY] = { }, > >> > + [ADIS16209_SCAN_AUX_ADC] = { }, > >> > + [ADIS16209_SCAN_ACC_X] = { ADIS16209_XACCL_NULL_REG }, > >> > + [ADIS16209_SCAN_ACC_Y] = { ADIS16209_YACCL_NULL_REG }, > >> > + [ADIS16209_SCAN_INCLI_X] = { ADIS16209_XINCL_NULL_REG }, > >> > + [ADIS16209_SCAN_INCLI_Y] = { ADIS16209_YINCL_NULL_REG }, > >> > + [ADIS16209_SCAN_ROT] = { }, > >> > + [ADIS16209_SCAN_TEMP] = { }, > >> > +}; > >> > + > >> > +static int adis16209_write_raw(struct iio_dev *indio_dev, > >> > +        struct iio_chan_spec const *chan, > >> > +        int val, > >> > +        int val2, > >> > +        long mask) > >> > +{ > >> > + struct adis *st = iio_priv(indio_dev); > >> > + int bits; > >> > + s16 val16; > >> > + u8 addr; > >> > + > >> > + switch (mask) { > >> > + case IIO_CHAN_INFO_CALIBBIAS: > >> > + switch (chan->type) { > >> > + case IIO_ACCEL: > >> > + case IIO_INCLI: > >> > + bits = 14; > >> > + break; > >> > + default: > >> > + return -EINVAL; > >> > + } > >> > + val16 = val & ((1 << bits) - 1); > >> > + addr = adis16209_addresses[chan->scan_index][0]; > >> > + return adis_write_reg_16(st, addr, val16); > >> > + } > >> > + return -EINVAL; > >> > +} > >> > + > >> > +static int adis16209_read_raw(struct iio_dev *indio_dev, > >> > +       struct iio_chan_spec const *chan, > >> > +       int *val, int *val2, > >> > +       long mask) > >> > +{ > >> > + struct adis *st = iio_priv(indio_dev); > >> > + int ret; > >> > + int bits; > >> > + u8 addr; > >> > + s16 val16; > >> > + > >> > + switch (mask) { > >> > + case IIO_CHAN_INFO_RAW: > >> > + return adis_single_conversion(indio_dev, chan, > >> > + ADIS16209_ERROR_ACTIVE, val); > >> > + case IIO_CHAN_INFO_SCALE: > >> > + switch (chan->type) { > >> > + case IIO_VOLTAGE: > >> > + *val = 0; > >> > + if (chan->channel == 0) > >> > + *val2 = 305180; /* 0.30518 mV */ > >> > + else > >> > + *val2 = 610500; /* 0.6105 mV */ > >> > + return IIO_VAL_INT_PLUS_MICRO; > >> > + case IIO_TEMP: > >> > + *val = -470; > >> > + *val2 = 0; > >> > + return IIO_VAL_INT_PLUS_MICRO; > >> > + case IIO_ACCEL: > >> > + /* > >> > +  * IIO base unit for sensitivity of > >> > accelerometer > >> > +  * is milli g. > >> > +  * 1 LSB represents 0.244 mg. > >> > +  */ > >> > + *val = 0; > >> > + *val2 = IIO_G_TO_M_S_2(244140); > >> > + return IIO_VAL_INT_PLUS_NANO; > >> > + case IIO_INCLI: > >> > + case IIO_ROT: > >> > + /* > >> > +  * IIO base units for rotation are > >> > degrees. > >> > +  * 1 LSB represents 0.025 milli degrees. > >> > +  */ > >> > + *val = 0; > >> > + *val2 = 25000; > >> > + return IIO_VAL_INT_PLUS_MICRO; > >> > + default: > >> > + return -EINVAL; > >> > + } > >> > + break; > >> > + case IIO_CHAN_INFO_OFFSET: > >> > + /* > >> > +  * The raw ADC value is 0x4FE when the temperature > >> > +  * is 45 degrees and the scale factor per milli > >> > +  * degree celcius is -470. > >> > +  */ > >> > + *val = 25000 / -470 - 0x4FE; > >> > + return IIO_VAL_INT; > >> > + case IIO_CHAN_INFO_CALIBBIAS: > >> > + switch (chan->type) { > >> > + case IIO_ACCEL: > >> > + bits = 14; > >> > + break; > >> > + default: > >> > + return -EINVAL; > >> > + } > >> > + addr = adis16209_addresses[chan->scan_index][0]; > >> > + ret = adis_read_reg_16(st, addr, &val16); > >> > + if (ret) > >> > + return ret; > >> > + val16 &= (1 << bits) - 1; > >> > + val16 = (s16)(val16 << (16 - bits)) >> (16 - > >> > bits); > >> > + *val = val16; > >> Something odd here as this isn't the current code - doesn't have the > >> sign extend patch in place. > >> > >> > > >> > + return IIO_VAL_INT; > >> > + } > >> > + return -EINVAL; > >> > +} > >> > + > >> > +static const struct iio_chan_spec adis16209_channels[] = { > >> > + ADIS_SUPPLY_CHAN(ADIS16209_SUPPLY_OUT_REG, > >> > ADIS16209_SCAN_SUPPLY, > >> > +  0, 14), > >> > + ADIS_TEMP_CHAN(ADIS16209_TEMP_OUT_REG, > >> > ADIS16209_SCAN_TEMP, 0, 12), > >> > + ADIS_ACCEL_CHAN(X, ADIS16209_XACCL_OUT_REG, > >> > ADIS16209_SCAN_ACC_X, > >> > + BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14), > >> > + ADIS_ACCEL_CHAN(Y, ADIS16209_YACCL_OUT_REG, > >> > ADIS16209_SCAN_ACC_Y, > >> > + BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14), > >> > + ADIS_AUX_ADC_CHAN(ADIS16209_AUX_ADC_REG, > >> > ADIS16209_SCAN_AUX_ADC, 0, 12), > >> > + ADIS_INCLI_CHAN(X, ADIS16209_XINCL_OUT_REG, > >> > ADIS16209_SCAN_INCLI_X, > >> > + 0, 0, 14), > >> > + ADIS_INCLI_CHAN(Y, ADIS16209_YINCL_OUT_REG, > >> > ADIS16209_SCAN_INCLI_Y, > >> > + 0, 0, 14), > >> > + ADIS_ROT_CHAN(X, ADIS16209_ROT_OUT_REG, > >> > ADIS16209_SCAN_ROT, 0, 0, 14), > >> This raises an interesting question.  How do we define rotation axes? > >> I would assume it was rotation about the axis, as such I think the > >> correct > >> axis for this is Z. > >> > >> However, then we consider the two inclination axis. Again fiddly.  I > >> suppose > >> it is inclination 'from' an axis, but in what plane?  I guess x and y > >> is about as good as we can do on those ones with the assumption they > >> are aligned to perpendicular to the vertical. > > > >I went through some datasheets to find out more about the single axis > >tilt inclination and double axis tilt inclination. > > > I meant *dual* axis here and even below. > > > >Rotational angle is provided by double axis tilt theory. > >As the name tells us that it is dual axis i.e. we are considering X and > >Y axis here so maybe this is why we are not taking Z axis here. > >Angle of rotation is taken from the X axis. > > > >I am attaching some screenshots about more information on this. > >Maybe they can be helpful. > > > >Also If I have understood something wrong then please do correct > >me :) Sure, I had seen the datasheet, just wasn't sure how to define it in IIO terms :) Jonathan > > > >Thanks > > > > > >> > >> > > >> > + IIO_CHAN_SOFT_TIMESTAMP(8) > >> > +}; > >> > + > >> > +static const struct iio_info adis16209_info = { > >> > + .read_raw = adis16209_read_raw, > >> > + .write_raw = adis16209_write_raw, > >> > + .update_scan_mode = adis_update_scan_mode, > >> > +}; > >> > + > >> > +static const char * const adis16209_status_error_msgs[] = { > >> > + [ADIS16209_STAT_SELFTEST_FAIL_BIT] = "Self test failure", > >> > + [ADIS16209_STAT_SPI_FAIL_BIT] = "SPI failure", > >> > + [ADIS16209_STAT_FLASH_UPT_FAIL_BIT] = "Flash update > >> > failed", > >> > + [ADIS16209_STAT_POWER_HIGH_BIT] = "Power supply above > >> > 3.625V", > >> > + [ADIS16209_STAT_POWER_LOW_BIT] = "Power supply below > >> > 3.15V", > >> > +}; > >> > + > >> > +static const struct adis_data adis16209_data = { > >> > + .read_delay = 30, > >> > + .msc_ctrl_reg = ADIS16209_MSC_CTRL_REG, > >> > + .glob_cmd_reg = ADIS16209_CMD_REG, > >> > + .diag_stat_reg = ADIS16209_STAT_REG, > >> > + > >> > + .self_test_mask = ADIS16209_MSC_CTRL_SELF_TEST_EN, > >> > + .self_test_no_autoclear = true, > >> > + .startup_delay = ADIS16209_STARTUP_DELAY_MS, > >> > + > >> > + .status_error_msgs = adis16209_status_error_msgs, > >> > + .status_error_mask = BIT(ADIS16209_STAT_SELFTEST_FAIL_BIT) > >> > | > >> > + BIT(ADIS16209_STAT_SPI_FAIL_BIT) | > >> > + BIT(ADIS16209_STAT_FLASH_UPT_FAIL_BIT) | > >> > + BIT(ADIS16209_STAT_POWER_HIGH_BIT) | > >> > + BIT(ADIS16209_STAT_POWER_LOW_BIT), > >> > +}; > >> > + > >> > +static int adis16209_probe(struct spi_device *spi) > >> > +{ > >> > + int ret; > >> > + struct adis *st; > >> > + struct iio_dev *indio_dev; > >> It's not one I personally feel strongly about but when there is no > >> reason not to do it, reverse Christmas tree ordering is preferred for > >> variable declarations. > >> > >> > > >> > + > >> > + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st)); > >> > + if (!indio_dev) > >> > + return -ENOMEM; > >> Blank line here. > >> > >> > > >> > + st = iio_priv(indio_dev); > >> > + spi_set_drvdata(spi, indio_dev); > >> > + > >> > + indio_dev->name = spi->dev.driver->name; > >> > + indio_dev->dev.parent = &spi->dev; > >> > + indio_dev->info = &adis16209_info; > >> > + indio_dev->channels = adis16209_channels; > >> > + indio_dev->num_channels = ARRAY_SIZE(adis16209_channels); > >> > + indio_dev->modes = INDIO_DIRECT_MODE; > >> > + > >> > + ret = adis_init(st, indio_dev, spi, &adis16209_data); > >> > + if (ret) > >> > + return ret; > >> Blank line here. > >> > >> > > >> > + ret = adis_setup_buffer_and_trigger(st, indio_dev, NULL); > >> > + if (ret) > >> > + return ret; > >> > + > >> > + ret = adis_initial_startup(st); > >> > + if (ret) > >> > + goto error_cleanup_buffer_trigger; > >> > + ret = iio_device_register(indio_dev); > >> > + if (ret) > >> > + goto error_cleanup_buffer_trigger; > >> > + > >> > + return 0; > >> > + > >> > +error_cleanup_buffer_trigger: > >> > + adis_cleanup_buffer_and_trigger(st, indio_dev); > >> > + return ret; > >> > +} > >> > + > >> > +static int adis16209_remove(struct spi_device *spi) > >> > +{ > >> > + struct iio_dev *indio_dev = spi_get_drvdata(spi); > >> > + struct adis *st = iio_priv(indio_dev); > >> > + > >> > + iio_device_unregister(indio_dev); > >> > + adis_cleanup_buffer_and_trigger(st, indio_dev); > >> > + > >> > + return 0; > >> > +} > >> > + > >> > +static struct spi_driver adis16209_driver = { > >> > + .driver = { > >> > + .name = "adis16209", > >> > + }, > >> > + .probe = adis16209_probe, > >> > + .remove = adis16209_remove, > >> > +}; > >> > +module_spi_driver(adis16209_driver); > >> > + > >> > +MODULE_AUTHOR("Barry Song <21cnbao@gmail.com>"); > >> > +MODULE_DESCRIPTION("Analog Devices ADIS16209 Dual-Axis Digital > >> > Inclinometer and Accelerometer"); > >> > +MODULE_LICENSE("GPL v2"); > >> > +MODULE_ALIAS("spi:adis16209");] > >> >