Re: [PATCH 2/2] staging: iio: accel: adis16240: move out of staging
On Thu, 12 Sep 2019 16:59:10 +0300 Alexandru Ardelean wrote: > On Wed, Sep 11, 2019 at 7:21 PM Rodrigo Carvalho > wrote: > > > > Hi, > > > > Em seg, 9 de set de 2019 às 02:53, Ardelean, Alexandru > > escreveu: > > > > > > On Sun, 2019-09-08 at 12:09 +0100, Jonathan Cameron wrote: > > > > On Mon, 2 Sep 2019 13:26:02 + > > > > "Ardelean, Alexandru" wrote: > > > > > > > > > On Sun, 2019-09-01 at 21:59 -0300, Rodrigo Carvalho wrote: > > > > > > Move ADIS16240 driver from staging to mainline. > > > > > > > > > > > > The ADIS16240 is a fully integrated digital shock detection > > > > > > and recorder system. > > > > > > > > > > Hey, > > > > > > > > > > Comments inline. > > > > > > > > > > I'll probably take a look in the next days again. > > > > > There seem to be some ABI/sysfs attributes that need to be resolved > > > > > before moving this out of staging. > > > > > > > > Absolutely. It is a 'new' type of device so there are definitely some > > > > corners that need discussing before we move out of staging and commit > > > > to maintaining the ABI moving forwards. > > > > > > > > That is the real reason this driver was still in staging! No one > > > > had been through the process of proposing the ABI and responding to > > > > questions etc. > > > > > > > > The issue with impact sensors has always been that they don't really fit > > > > our normal model for buffers or triggers. > > > > > > > > So normally a trigger (if exposed in IIO) is used as one trigger > > > > causes 1 set of samples (so like a frame trigger for a camera). > > > > > > > > These devices tend to work in a mode where one trigger causes data > > > > to be captured for a period of time. In this part that's the event > > > > recorder function > > > > > > > > No one is realistically going to buy an impact sensor to just use it > > > > as an accelerometer which is what this driver is currently doing. > > > > I suppose we could just leave support in that form for now, but > > > > I'm no sure how much use it is to anyone. > > > > > > > > Analog Devices people, worth working out how to support the event > > > > recorder? For that someone needs to have hardware as it is complex > > > > to say the least! > > > > > > Worth it: yes. > > > But we don't have any resources to allocate for this [at this point in > > > time]. > > > > > > > > > > > We could move it out but might be worth adding a comment somewhere > > > > saying this only really supports direct access to channels, and > > > > not the event recorder functionality. > > > > > > > > > > I guess, I would vote for leaving it in staging. > > > It's also a way to mark it as a > > > work-in-progress/not-done/still-needs-something kind of thing. > > > If we move it now, it gets the status of "everything-resolved" which is > > > not yet the case. > > > > > > Thanks for the insight/background info. > > > Much of it was discussed before my time. > > > > How about adis16203? Is it more simple to move this driver out of staging? > > Replying from my personal email, as I seem to have my periodical > issues with the work-email. > > At first glance I can't see any obvious blockers for this. > > I think Jonathan may need to provide some feedback here. There is a 'fixme' in the channels which I suspect is the remaining blocker on this one. The current y axis is actually just a different representation of the value on the x axis. I 'think' the only reason to support both of these is the interaction with the threshold detectors. Those are rather simple, so if you have -10 on the second channel which is 350 on the first, it would detected as < 10 on the second but not the first. It is ugly, but we may be better just treating these as two separate channels on the same axis - similar to the way we would deal with the case of two accelerometers in a package, with different ranges. Also a rather odd bit of abstraction around _addresses given there is only one specified. Might be nice to clean out some of the less useful comments for registers where the register name makes it clear what they are. It's a mixture of some useful ones and some pointless ones at the moment. So a few bits to tidy up and may more come up in a thorough review. Thanks, Jonathan > > Thanks > Alex > > > > > > > > > > > Jonathan > > > > > > Alex > > > > > > > > > > > > > > > > > Signed-off-by: Rodrigo Ribeiro Carvalho > > > > > > --- > > > > > > drivers/iio/accel/Kconfig | 12 + > > > > > > drivers/iio/accel/Makefile| 1 + > > > > > > drivers/iio/accel/adis16240.c | 454 > > > > > > ++ > > > > > > drivers/staging/iio/accel/Kconfig | 12 - > > > > > > drivers/staging/iio/accel/Makefile| 1 - > > > > > > drivers/staging/iio/accel/adis16240.c | 454 > > > > > > -- > > > > > > 6 files changed, 467 insertions(+), 467 deletions(-) > > > > > > create mode 100644 drivers/iio/accel/adis16240.c > >
Re: [PATCH 2/2] staging: iio: accel: adis16240: move out of staging
On Wed, Sep 11, 2019 at 7:21 PM Rodrigo Carvalho wrote: > > Hi, > > Em seg, 9 de set de 2019 às 02:53, Ardelean, Alexandru > escreveu: > > > > On Sun, 2019-09-08 at 12:09 +0100, Jonathan Cameron wrote: > > > On Mon, 2 Sep 2019 13:26:02 + > > > "Ardelean, Alexandru" wrote: > > > > > > > On Sun, 2019-09-01 at 21:59 -0300, Rodrigo Carvalho wrote: > > > > > Move ADIS16240 driver from staging to mainline. > > > > > > > > > > The ADIS16240 is a fully integrated digital shock detection > > > > > and recorder system. > > > > > > > > Hey, > > > > > > > > Comments inline. > > > > > > > > I'll probably take a look in the next days again. > > > > There seem to be some ABI/sysfs attributes that need to be resolved > > > > before moving this out of staging. > > > > > > Absolutely. It is a 'new' type of device so there are definitely some > > > corners that need discussing before we move out of staging and commit > > > to maintaining the ABI moving forwards. > > > > > > That is the real reason this driver was still in staging! No one > > > had been through the process of proposing the ABI and responding to > > > questions etc. > > > > > > The issue with impact sensors has always been that they don't really fit > > > our normal model for buffers or triggers. > > > > > > So normally a trigger (if exposed in IIO) is used as one trigger > > > causes 1 set of samples (so like a frame trigger for a camera). > > > > > > These devices tend to work in a mode where one trigger causes data > > > to be captured for a period of time. In this part that's the event > > > recorder function > > > > > > No one is realistically going to buy an impact sensor to just use it > > > as an accelerometer which is what this driver is currently doing. > > > I suppose we could just leave support in that form for now, but > > > I'm no sure how much use it is to anyone. > > > > > > Analog Devices people, worth working out how to support the event > > > recorder? For that someone needs to have hardware as it is complex > > > to say the least! > > > > Worth it: yes. > > But we don't have any resources to allocate for this [at this point in > > time]. > > > > > > > > We could move it out but might be worth adding a comment somewhere > > > saying this only really supports direct access to channels, and > > > not the event recorder functionality. > > > > > > > I guess, I would vote for leaving it in staging. > > It's also a way to mark it as a > > work-in-progress/not-done/still-needs-something kind of thing. > > If we move it now, it gets the status of "everything-resolved" which is not > > yet the case. > > > > Thanks for the insight/background info. > > Much of it was discussed before my time. > > How about adis16203? Is it more simple to move this driver out of staging? Replying from my personal email, as I seem to have my periodical issues with the work-email. At first glance I can't see any obvious blockers for this. I think Jonathan may need to provide some feedback here. Thanks Alex > > > > > > > Jonathan > > > > Alex > > > > > > > > > > > > > Signed-off-by: Rodrigo Ribeiro Carvalho > > > > > --- > > > > > drivers/iio/accel/Kconfig | 12 + > > > > > drivers/iio/accel/Makefile| 1 + > > > > > drivers/iio/accel/adis16240.c | 454 > > > > > ++ > > > > > drivers/staging/iio/accel/Kconfig | 12 - > > > > > drivers/staging/iio/accel/Makefile| 1 - > > > > > drivers/staging/iio/accel/adis16240.c | 454 > > > > > -- > > > > > 6 files changed, 467 insertions(+), 467 deletions(-) > > > > > create mode 100644 drivers/iio/accel/adis16240.c > > > > > delete mode 100644 drivers/staging/iio/accel/adis16240.c > > > > > > > > Looks like MAINTAINERS file also needs to be updated, also with the DT > > > > bindings file. > > > > I think checkpatch usually complains about these. > > > > > > > > > diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig > > > > > index d4ef35aeb579..91fd8741c95f 100644 > > > > > --- a/drivers/iio/accel/Kconfig > > > > > +++ b/drivers/iio/accel/Kconfig > > > > > @@ -30,6 +30,18 @@ config ADIS16209 > > > > > To compile this driver as a module, say M here: the module will be > > > > > called adis16209. > > > > > > > > > > +config ADIS16240 > > > > > + tristate "Analog Devices ADIS16240 Programmable Impact Sensor and > > > > > Recorder" > > > > > + 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 adis16240 > > > > > programmable > > > > > + impact Sensor and recorder. > > > > > + > > > > > + To compile this driver as a module, say M here: the module will be > > > > > + called adis16240. > > > > > + > > > > > config ADXL345 > > > > > tristate > > > > > > > > > > diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile > > > > > index 56bd0215e0d4..f7e025a86
Re: [PATCH 2/2] staging: iio: accel: adis16240: move out of staging
Hi, Em seg, 9 de set de 2019 às 02:53, Ardelean, Alexandru escreveu: > > On Sun, 2019-09-08 at 12:09 +0100, Jonathan Cameron wrote: > > On Mon, 2 Sep 2019 13:26:02 + > > "Ardelean, Alexandru" wrote: > > > > > On Sun, 2019-09-01 at 21:59 -0300, Rodrigo Carvalho wrote: > > > > Move ADIS16240 driver from staging to mainline. > > > > > > > > The ADIS16240 is a fully integrated digital shock detection > > > > and recorder system. > > > > > > Hey, > > > > > > Comments inline. > > > > > > I'll probably take a look in the next days again. > > > There seem to be some ABI/sysfs attributes that need to be resolved > > > before moving this out of staging. > > > > Absolutely. It is a 'new' type of device so there are definitely some > > corners that need discussing before we move out of staging and commit > > to maintaining the ABI moving forwards. > > > > That is the real reason this driver was still in staging! No one > > had been through the process of proposing the ABI and responding to > > questions etc. > > > > The issue with impact sensors has always been that they don't really fit > > our normal model for buffers or triggers. > > > > So normally a trigger (if exposed in IIO) is used as one trigger > > causes 1 set of samples (so like a frame trigger for a camera). > > > > These devices tend to work in a mode where one trigger causes data > > to be captured for a period of time. In this part that's the event > > recorder function > > > > No one is realistically going to buy an impact sensor to just use it > > as an accelerometer which is what this driver is currently doing. > > I suppose we could just leave support in that form for now, but > > I'm no sure how much use it is to anyone. > > > > Analog Devices people, worth working out how to support the event > > recorder? For that someone needs to have hardware as it is complex > > to say the least! > > Worth it: yes. > But we don't have any resources to allocate for this [at this point in time]. > > > > > We could move it out but might be worth adding a comment somewhere > > saying this only really supports direct access to channels, and > > not the event recorder functionality. > > > > I guess, I would vote for leaving it in staging. > It's also a way to mark it as a > work-in-progress/not-done/still-needs-something kind of thing. > If we move it now, it gets the status of "everything-resolved" which is not > yet the case. > > Thanks for the insight/background info. > Much of it was discussed before my time. How about adis16203? Is it more simple to move this driver out of staging? > > > > Jonathan > > Alex > > > > > > > > > Signed-off-by: Rodrigo Ribeiro Carvalho > > > > --- > > > > drivers/iio/accel/Kconfig | 12 + > > > > drivers/iio/accel/Makefile| 1 + > > > > drivers/iio/accel/adis16240.c | 454 ++ > > > > drivers/staging/iio/accel/Kconfig | 12 - > > > > drivers/staging/iio/accel/Makefile| 1 - > > > > drivers/staging/iio/accel/adis16240.c | 454 -- > > > > 6 files changed, 467 insertions(+), 467 deletions(-) > > > > create mode 100644 drivers/iio/accel/adis16240.c > > > > delete mode 100644 drivers/staging/iio/accel/adis16240.c > > > > > > Looks like MAINTAINERS file also needs to be updated, also with the DT > > > bindings file. > > > I think checkpatch usually complains about these. > > > > > > > diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig > > > > index d4ef35aeb579..91fd8741c95f 100644 > > > > --- a/drivers/iio/accel/Kconfig > > > > +++ b/drivers/iio/accel/Kconfig > > > > @@ -30,6 +30,18 @@ config ADIS16209 > > > > To compile this driver as a module, say M here: the module will be > > > > called adis16209. > > > > > > > > +config ADIS16240 > > > > + tristate "Analog Devices ADIS16240 Programmable Impact Sensor and > > > > Recorder" > > > > + 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 adis16240 > > > > programmable > > > > + impact Sensor and recorder. > > > > + > > > > + To compile this driver as a module, say M here: the module will be > > > > + called adis16240. > > > > + > > > > config ADXL345 > > > > tristate > > > > > > > > diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile > > > > index 56bd0215e0d4..f7e025a86dd9 100644 > > > > --- a/drivers/iio/accel/Makefile > > > > +++ b/drivers/iio/accel/Makefile > > > > @@ -6,6 +6,7 @@ > > > > # When adding new entries keep the list in alphabetical order > > > > obj-$(CONFIG_ADIS16201) += adis16201.o > > > > obj-$(CONFIG_ADIS16209) += adis16209.o > > > > +obj-$(CONFIG_ADIS16240) += adis16240.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/adis16240.c > > > > b/dr
Re: [PATCH 2/2] staging: iio: accel: adis16240: move out of staging
On Sun, 2019-09-08 at 12:09 +0100, Jonathan Cameron wrote: > On Mon, 2 Sep 2019 13:26:02 + > "Ardelean, Alexandru" wrote: > > > On Sun, 2019-09-01 at 21:59 -0300, Rodrigo Carvalho wrote: > > > Move ADIS16240 driver from staging to mainline. > > > > > > The ADIS16240 is a fully integrated digital shock detection > > > and recorder system. > > > > Hey, > > > > Comments inline. > > > > I'll probably take a look in the next days again. > > There seem to be some ABI/sysfs attributes that need to be resolved before > > moving this out of staging. > > Absolutely. It is a 'new' type of device so there are definitely some > corners that need discussing before we move out of staging and commit > to maintaining the ABI moving forwards. > > That is the real reason this driver was still in staging! No one > had been through the process of proposing the ABI and responding to > questions etc. > > The issue with impact sensors has always been that they don't really fit > our normal model for buffers or triggers. > > So normally a trigger (if exposed in IIO) is used as one trigger > causes 1 set of samples (so like a frame trigger for a camera). > > These devices tend to work in a mode where one trigger causes data > to be captured for a period of time. In this part that's the event > recorder function > > No one is realistically going to buy an impact sensor to just use it > as an accelerometer which is what this driver is currently doing. > I suppose we could just leave support in that form for now, but > I'm no sure how much use it is to anyone. > > Analog Devices people, worth working out how to support the event > recorder? For that someone needs to have hardware as it is complex > to say the least! Worth it: yes. But we don't have any resources to allocate for this [at this point in time]. > > We could move it out but might be worth adding a comment somewhere > saying this only really supports direct access to channels, and > not the event recorder functionality. > I guess, I would vote for leaving it in staging. It's also a way to mark it as a work-in-progress/not-done/still-needs-something kind of thing. If we move it now, it gets the status of "everything-resolved" which is not yet the case. Thanks for the insight/background info. Much of it was discussed before my time. > > Jonathan Alex > > > > > Signed-off-by: Rodrigo Ribeiro Carvalho > > > --- > > > drivers/iio/accel/Kconfig | 12 + > > > drivers/iio/accel/Makefile| 1 + > > > drivers/iio/accel/adis16240.c | 454 ++ > > > drivers/staging/iio/accel/Kconfig | 12 - > > > drivers/staging/iio/accel/Makefile| 1 - > > > drivers/staging/iio/accel/adis16240.c | 454 -- > > > 6 files changed, 467 insertions(+), 467 deletions(-) > > > create mode 100644 drivers/iio/accel/adis16240.c > > > delete mode 100644 drivers/staging/iio/accel/adis16240.c > > > > Looks like MAINTAINERS file also needs to be updated, also with the DT > > bindings file. > > I think checkpatch usually complains about these. > > > > > diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig > > > index d4ef35aeb579..91fd8741c95f 100644 > > > --- a/drivers/iio/accel/Kconfig > > > +++ b/drivers/iio/accel/Kconfig > > > @@ -30,6 +30,18 @@ config ADIS16209 > > > To compile this driver as a module, say M here: the module will be > > > called adis16209. > > > > > > +config ADIS16240 > > > + tristate "Analog Devices ADIS16240 Programmable Impact Sensor and > > > Recorder" > > > + 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 adis16240 programmable > > > + impact Sensor and recorder. > > > + > > > + To compile this driver as a module, say M here: the module will be > > > + called adis16240. > > > + > > > config ADXL345 > > > tristate > > > > > > diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile > > > index 56bd0215e0d4..f7e025a86dd9 100644 > > > --- a/drivers/iio/accel/Makefile > > > +++ b/drivers/iio/accel/Makefile > > > @@ -6,6 +6,7 @@ > > > # When adding new entries keep the list in alphabetical order > > > obj-$(CONFIG_ADIS16201) += adis16201.o > > > obj-$(CONFIG_ADIS16209) += adis16209.o > > > +obj-$(CONFIG_ADIS16240) += adis16240.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/adis16240.c b/drivers/iio/accel/adis16240.c > > > new file mode 100644 > > > index ..82099db4bf0c > > > --- /dev/null > > > +++ b/drivers/iio/accel/adis16240.c > > > @@ -0,0 +1,454 @@ > > > +// SPDX-License-Identifier: GPL-2.0+ > > > +/* > > > + * ADIS16240 Programmable Impact Sensor and Recorder driver > > > + * > > > + * Copyright 2010 Analog Devices Inc. > > > + */ > > > +
Re: [PATCH 2/2] staging: iio: accel: adis16240: move out of staging
On Mon, 2 Sep 2019 13:26:02 + "Ardelean, Alexandru" wrote: > On Sun, 2019-09-01 at 21:59 -0300, Rodrigo Carvalho wrote: > > Move ADIS16240 driver from staging to mainline. > > > > The ADIS16240 is a fully integrated digital shock detection > > and recorder system. > > > Hey, > > Comments inline. > > I'll probably take a look in the next days again. > There seem to be some ABI/sysfs attributes that need to be resolved before > moving this out of staging. Absolutely. It is a 'new' type of device so there are definitely some corners that need discussing before we move out of staging and commit to maintaining the ABI moving forwards. That is the real reason this driver was still in staging! No one had been through the process of proposing the ABI and responding to questions etc. The issue with impact sensors has always been that they don't really fit our normal model for buffers or triggers. So normally a trigger (if exposed in IIO) is used as one trigger causes 1 set of samples (so like a frame trigger for a camera). These devices tend to work in a mode where one trigger causes data to be captured for a period of time. In this part that's the event recorder function No one is realistically going to buy an impact sensor to just use it as an accelerometer which is what this driver is currently doing. I suppose we could just leave support in that form for now, but I'm no sure how much use it is to anyone. Analog Devices people, worth working out how to support the event recorder? For that someone needs to have hardware as it is complex to say the least! We could move it out but might be worth adding a comment somewhere saying this only really supports direct access to channels, and not the event recorder functionality. Jonathan > > > > > Signed-off-by: Rodrigo Ribeiro Carvalho > > --- > > drivers/iio/accel/Kconfig | 12 + > > drivers/iio/accel/Makefile| 1 + > > drivers/iio/accel/adis16240.c | 454 ++ > > drivers/staging/iio/accel/Kconfig | 12 - > > drivers/staging/iio/accel/Makefile| 1 - > > drivers/staging/iio/accel/adis16240.c | 454 -- > > 6 files changed, 467 insertions(+), 467 deletions(-) > > create mode 100644 drivers/iio/accel/adis16240.c > > delete mode 100644 drivers/staging/iio/accel/adis16240.c > > Looks like MAINTAINERS file also needs to be updated, also with the DT > bindings file. > I think checkpatch usually complains about these. > > > > > diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig > > index d4ef35aeb579..91fd8741c95f 100644 > > --- a/drivers/iio/accel/Kconfig > > +++ b/drivers/iio/accel/Kconfig > > @@ -30,6 +30,18 @@ config ADIS16209 > > To compile this driver as a module, say M here: the module will be > > called adis16209. > > > > +config ADIS16240 > > + tristate "Analog Devices ADIS16240 Programmable Impact Sensor and > > Recorder" > > + 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 adis16240 programmable > > + impact Sensor and recorder. > > + > > + To compile this driver as a module, say M here: the module will be > > + called adis16240. > > + > > config ADXL345 > > tristate > > > > diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile > > index 56bd0215e0d4..f7e025a86dd9 100644 > > --- a/drivers/iio/accel/Makefile > > +++ b/drivers/iio/accel/Makefile > > @@ -6,6 +6,7 @@ > > # When adding new entries keep the list in alphabetical order > > obj-$(CONFIG_ADIS16201) += adis16201.o > > obj-$(CONFIG_ADIS16209) += adis16209.o > > +obj-$(CONFIG_ADIS16240) += adis16240.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/adis16240.c b/drivers/iio/accel/adis16240.c > > new file mode 100644 > > index ..82099db4bf0c > > --- /dev/null > > +++ b/drivers/iio/accel/adis16240.c > > @@ -0,0 +1,454 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * ADIS16240 Programmable Impact Sensor and Recorder driver > > + * > > + * Copyright 2010 Analog Devices Inc. > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include > > +#include > > +#include > > +#include > > + > > +#define ADIS16240_STARTUP_DELAY220 /* ms */ > > + > > +/* Flash memory write count */ > > +#define ADIS16240_FLASH_CNT 0x00 > > + > > +/* Output, power supply */ > > +#define ADIS16240_SUPPLY_OUT 0x02 > > + > > +/* Output, x-axis accelerometer */ > > +#define ADIS16240_XACCL_OUT 0x04 > > + > > +/* Output, y-axis accelerometer */ > > +#define ADIS16240_YACCL_OUT 0x06 > > + > > +/* Output, z-axis accelerome
Re: [PATCH 2/2] staging: iio: accel: adis16240: move out of staging
On Sun, 2019-09-01 at 21:59 -0300, Rodrigo Carvalho wrote: > Move ADIS16240 driver from staging to mainline. > > The ADIS16240 is a fully integrated digital shock detection > and recorder system. Hey, Comments inline. I'll probably take a look in the next days again. There seem to be some ABI/sysfs attributes that need to be resolved before moving this out of staging. > > Signed-off-by: Rodrigo Ribeiro Carvalho > --- > drivers/iio/accel/Kconfig | 12 + > drivers/iio/accel/Makefile| 1 + > drivers/iio/accel/adis16240.c | 454 ++ > drivers/staging/iio/accel/Kconfig | 12 - > drivers/staging/iio/accel/Makefile| 1 - > drivers/staging/iio/accel/adis16240.c | 454 -- > 6 files changed, 467 insertions(+), 467 deletions(-) > create mode 100644 drivers/iio/accel/adis16240.c > delete mode 100644 drivers/staging/iio/accel/adis16240.c Looks like MAINTAINERS file also needs to be updated, also with the DT bindings file. I think checkpatch usually complains about these. > > diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig > index d4ef35aeb579..91fd8741c95f 100644 > --- a/drivers/iio/accel/Kconfig > +++ b/drivers/iio/accel/Kconfig > @@ -30,6 +30,18 @@ config ADIS16209 > To compile this driver as a module, say M here: the module will be > called adis16209. > > +config ADIS16240 > + tristate "Analog Devices ADIS16240 Programmable Impact Sensor and > Recorder" > + 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 adis16240 programmable > + impact Sensor and recorder. > + > + To compile this driver as a module, say M here: the module will be > + called adis16240. > + > config ADXL345 > tristate > > diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile > index 56bd0215e0d4..f7e025a86dd9 100644 > --- a/drivers/iio/accel/Makefile > +++ b/drivers/iio/accel/Makefile > @@ -6,6 +6,7 @@ > # When adding new entries keep the list in alphabetical order > obj-$(CONFIG_ADIS16201) += adis16201.o > obj-$(CONFIG_ADIS16209) += adis16209.o > +obj-$(CONFIG_ADIS16240) += adis16240.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/adis16240.c b/drivers/iio/accel/adis16240.c > new file mode 100644 > index ..82099db4bf0c > --- /dev/null > +++ b/drivers/iio/accel/adis16240.c > @@ -0,0 +1,454 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * ADIS16240 Programmable Impact Sensor and Recorder driver > + * > + * Copyright 2010 Analog Devices Inc. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > + > +#define ADIS16240_STARTUP_DELAY 220 /* ms */ > + > +/* Flash memory write count */ > +#define ADIS16240_FLASH_CNT 0x00 > + > +/* Output, power supply */ > +#define ADIS16240_SUPPLY_OUT 0x02 > + > +/* Output, x-axis accelerometer */ > +#define ADIS16240_XACCL_OUT 0x04 > + > +/* Output, y-axis accelerometer */ > +#define ADIS16240_YACCL_OUT 0x06 > + > +/* Output, z-axis accelerometer */ > +#define ADIS16240_ZACCL_OUT 0x08 > + > +/* Output, auxiliary ADC input */ > +#define ADIS16240_AUX_ADC0x0A > + > +/* Output, temperature */ > +#define ADIS16240_TEMP_OUT 0x0C > + > +/* Output, x-axis acceleration peak */ > +#define ADIS16240_XPEAK_OUT 0x0E > + > +/* Output, y-axis acceleration peak */ > +#define ADIS16240_YPEAK_OUT 0x10 > + > +/* Output, z-axis acceleration peak */ > +#define ADIS16240_ZPEAK_OUT 0x12 > + > +/* Output, sum-of-squares acceleration peak */ > +#define ADIS16240_XYZPEAK_OUT0x14 > + > +/* Output, Capture Buffer 1, X and Y acceleration */ > +#define ADIS16240_CAPT_BUF1 0x16 > + > +/* Output, Capture Buffer 2, Z acceleration */ > +#define ADIS16240_CAPT_BUF2 0x18 > + > +/* Diagnostic, error flags */ > +#define ADIS16240_DIAG_STAT 0x1A > +This looks like it could be converted to IIO_CHAN_INFO_SAMP_FREQ attribute. > +/* Diagnostic, event counter */ > +#define ADIS16240_EVNT_CNTR 0x1C > + > +/* Diagnostic, check sum value from firmware test */ > +#define ADIS16240_CHK_SUM0x1E > + > +/* Calibration, x-axis acceleration offset adjustment */ > +#define ADIS16240_XACCL_OFF 0x20 > + > +/* Calibration, y-axis acceleration offset adjustment */ > +#define ADIS16240_YACCL_OFF 0x22 > + > +/* Calibration, z-axis acceleration offset adjustment */ > +#define ADIS16240_ZACCL_OFF 0x24 > +This looks like it could be converted to IIO_CHAN_INFO_SAMP_FREQ attribute. > +/* Clock, hour and minute */ > +#define ADIS16240_CLK_TIME 0x2E > + > +/* Clock, mont
