iio: adc: hx711: Add IIO driver for AVIA HX711
Subsystems (2)
Andreas Klinger
maintainerPATCH v3 2/2 iio: adc: hx711: Add IIO driver for AVIA HX711
This is the IIO driver for AVIA HX711 ADC which ist mostly used in weighting cells.
The protocol is quite simple and using GPIOs:
One GPIO is used as clock (SCK) while another GPIO is read (DOUT)
The raw value read from the chip is delivered.
To get a weight one needs to subtract the zero offset and scale it.
--- drivers/iio/adc/Kconfig | 18 +++ drivers/iio/adc/Makefile | 1 + drivers/iio/adc/hx711.c | 292 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 311 insertions(+) create mode 100644 drivers/iio/adc/hx711.c
drivers/iio/adc/Kconfig +18 -0
@@ -205,6 +205,24 @@ config HI8435 This driver can also be built as a module. If so, the module will be called hi8435. +config HX711 + tristate "AVIA HX711 ADC for weight cells" + depends on GPIOLIB + help + If you say yes here you get support for AVIA HX711 ADC which is used + for weight cells + + This driver uses two GPIOs, one for setting the clock and the other + one for getting the data + + Currently the raw value is read from the chip and delivered. + For getting an actual weight one needs to subtract the + zero offset and multiply by a scale factor. + This should be done in userspace. + + This driver can also be built as a module. If so, the module will be + called hx711. + config INA2XX_ADC tristate "Texas Instruments INA2xx Power Monitors IIO driver" depends on I2C && !SENSORS_INA2XX
drivers/iio/adc/Makefile +1 -0
@@ -21,6 +21,7 @@ obj-$(CONFIG_CC10001_ADC) += cc10001_adc.o obj-$(CONFIG_DA9150_GPADC) += da9150-gpadc.o obj-$(CONFIG_EXYNOS_ADC) += exynos_adc.o obj-$(CONFIG_HI8435) += hi8435.o +obj-$(CONFIG_HX711) += hx711.o obj-$(CONFIG_IMX7D_ADC) += imx7d_adc.o obj-$(CONFIG_INA2XX_ADC) += ina2xx-adc.o obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
drivers/iio/adc/hx711.c new file +292 -0
292 lines not on this page · raw message
Signature · 6 lines
2.1.4 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/14/2016 05:17 PM, Andreas Klinger wrote:
[...]
drivers/iio/adc/hx711.c line 18
Quoted 2 lines
drivers/iio/adc/hx711.c new file +2
-0+#include <linux/err.h> +#include <linux/gpio.h>
Since you used the consumer API linux/gpio.h is not needed.
Quoted 22 lines
drivers/iio/adc/hx711.c new file +22
-0+#include <linux/gpio/consumer.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/property.h> +#include <linux/slab.h> +#include <linux/sched.h> +#include <linux/delay.h> +#include <linux/iio/iio.h> +#include <linux/iio/sysfs.h> + +#define HX711_GAIN_32 2/* gain = 32 for channel B */ +#define HX711_GAIN_64 3/* gain = 64 for channel A */ +#define HX711_GAIN_128 1/* gain = 128 for channel A */ + +struct hx711_data { +struct device *dev; +struct gpio_desc *gpiod_sck; +struct gpio_desc *gpiod_dout; +int gain_pulse; +struct mutex lock;
drivers/iio/adc/hx711.c line 89
Quoted 10 lines
drivers/iio/adc/hx711.c new file +10
-0+}; + +static int hx711_read (struct hx711_data *hx711_data) +{ +int i, ret; +int value =0 ; + +mutex_lock (&hx711_data->lock); + +if (hx711_reset (hx711_data)) {
If you reset the device before each conversion wont this clear the channel and gain selection? Wouldn't the driver always read from channel A at 128 gain regardless of what has been selected?
drivers/iio/adc/hx711.c line 92
Quoted 3 lines
drivers/iio/adc/hx711.c new file +3
-0+dev_err (hx711_data->dev,"reset failed!" ); +mutex_unlock (&hx711_data->lock); +return -1 ;
If there is an error it should be propagated to the higher layers. At the moment you only return a bogus conversion value.
Quoted 38 lines
drivers/iio/adc/hx711.c new file +38
-0+ } + +for (i =0 ; i <24 ; i++) { + value <<=1 ; + ret =hx711_cycle (hx711_data); +if (ret) + value++; + } + + value ^=0x800000 ; + +for (i =0 ; i < hx711_data->gain_pulse; i++) + ret =hx711_cycle (hx711_data); + +mutex_unlock (&hx711_data->lock); + +return value; +} + +static int hx711_read_raw (struct iio_dev *iio_dev, +const struct iio_chan_spec *chan, +int *val,int *val2,long mask) +{ +struct hx711_data *hx711_data =iio_priv (iio_dev); + +switch (mask) { +case IIO_CHAN_INFO_RAW : +switch (chan->type) { +case IIO_VOLTAGE : + *val =hx711_read (hx711_data); +return IIO_VAL_INT ; +default : +return -EINVAL ; + } +default : +return -EINVAL ; + } +}
[...]
drivers/iio/adc/hx711.c line 194
Quoted 2 lines
drivers/iio/adc/hx711.c new file +2
-0+static struct attribute *hx711_attributes[] = { + &iio_dev_attr_gain.dev_attr.attr,
For IIO devices the gain is typically expressed through the scale attribute. Which is kind of the inverse of gain. It would be good if this driver follows this standard notation. The scale is the value of 1LSB in mV. So this includes the resolution of the ADC, the reference voltage and any gain that is applied to the input signal.
The possible values can be listed in the scale_available attribute.
Quoted 7 lines
drivers/iio/adc/hx711.c new file +7
-0+ NULL, +}; + +static struct attribute_group hx711_attribute_group = { + .attrs = hx711_attributes, +}; +
drivers/iio/adc/hx711.c line 211
Quoted 10 lines
drivers/iio/adc/hx711.c new file +10
-0+static const struct iio_info hx711_iio_info = { + .driver_module =THIS_MODULE , + .read_raw = hx711_read_raw, + .attrs = &hx711_attribute_group, +}; + +static const struct iio_chan_spec hx711_chan_spec[] = { + { .type =IIO_VOLTAGE , + .info_mask_separate = +BIT (IIO_CHAN_INFO_RAW ),
Given that there are two separate physical input channels this should be expressed here and there should be two IIO channels for the device.
drivers/iio/adc/hx711.c line 218
Quoted 7 lines
drivers/iio/adc/hx711.c new file +7
-0+ }, +}; + +static int hx711_probe (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct hx711_data *hx711_data =NULL ;
The = NULL is not needed as it is overwritten a few lines below.
drivers/iio/adc/hx711.c line 220
Quoted 2 lines
drivers/iio/adc/hx711.c new file +2
-0+struct iio_dev *iio; +int ret =0 ;
Again = 0 no needed.
Quoted 17 lines
drivers/iio/adc/hx711.c new file +17
-0+ + iio =devm_iio_device_alloc (dev,sizeof (struct hx711_data )); +if (!iio) { +dev_err (dev,"failed to allocate IIO device\n" ); +return -ENOMEM ; + } + + hx711_data =iio_priv (iio); + hx711_data->dev = dev; + +mutex_init (&hx711_data->lock); + + hx711_data->gpiod_sck =devm_gpiod_get (dev,"sck" ,GPIOD_OUT_HIGH ); +if (IS_ERR (hx711_data->gpiod_sck)) { +dev_err (dev,"failed to get sck-gpiod: err=%ld\n" , +PTR_ERR (hx711_data->gpiod_sck)); +return PTR_ERR (hx711_data->gpiod_sck);
drivers/iio/adc/hx711.c line 247
Quoted 10 lines
drivers/iio/adc/hx711.c new file +10
-0+ } + + hx711_data->gpiod_dout =devm_gpiod_get (dev,"dout" ,GPIOD_OUT_HIGH ); +if (IS_ERR (hx711_data->gpiod_dout)) { +dev_err (dev,"failed to get dout-gpiod: err=%ld\n" , +PTR_ERR (hx711_data->gpiod_dout)); +return PTR_ERR (hx711_data->gpiod_dout); + } + + ret =gpiod_direction_input (hx711_data->gpiod_dout);
If dout is used as a input GPIO you should request it with GPIOD_IN. In that case you can remove the gpiod_direction_input() call.
drivers/iio/adc/hx711.c line 253
Quoted 6 lines
drivers/iio/adc/hx711.c new file +6
-0+if (ret <0 ) { +dev_err (hx711_data->dev,"gpiod_direction_input: %d\n" , ret); +return ret; + } + + ret =gpiod_direction_output (hx711_data->gpiod_sck,0 );
Similar to above. If you want this to be a output GPIO with the default value of 0 request it with GPIOD_OUT_LOW.
drivers/iio/adc/hx711.c line 259
Quoted 6 lines
drivers/iio/adc/hx711.c new file +6
-0+if (ret <0 ) { +dev_err (hx711_data->dev,"gpiod_direction_output: %d\n" , ret); +return ret; + } + +platform_set_drvdata (pdev, iio);
There is no matching platform_get_drvdata() so this can probably be removed.
drivers/iio/adc/hx711.c line 261
Quoted 2 lines
drivers/iio/adc/hx711.c new file +2
-0+ + iio->name = pdev->name;
This should be the part name. E.g. "hx711" in this case.
drivers/iio/adc/hx711.c line 269
Quoted 8 lines
drivers/iio/adc/hx711.c new file +8
-0+ iio->dev.parent = &pdev->dev; + iio->info = &hx711_iio_info; + iio->modes =INDIO_DIRECT_MODE ; + iio->channels = hx711_chan_spec; + iio->num_channels =ARRAY_SIZE (hx711_chan_spec); + +return devm_iio_device_register (dev, iio); +}
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Andreas Klinger
maintainerHello Lars,
thank you for the thorough review.
I have some questions. See below.
Thanks,
Andreas
Lars-Peter Clausen <[email protected]> schrieb am Mon, 19. Dec 17:28:
Quoted 35 lines · Andreas Klinger
[...]
drivers/iio/adc/hx711.c line 18
Quoted 2 lines
drivers/iio/adc/hx711.c new file +2
-0+#include <linux/err.h> +#include <linux/gpio.h> Since you used the consumer API
linux/gpio.his not needed.Quoted 28 lines
drivers/iio/adc/hx711.c new file +28
-0+#include <linux/gpio/consumer.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/property.h> +#include <linux/slab.h> +#include <linux/sched.h> +#include <linux/delay.h> +#include <linux/iio/iio.h> +#include <linux/iio/sysfs.h> + +#define HX711_GAIN_32 2/* gain = 32 for channel B */ +#define HX711_GAIN_64 3/* gain = 64 for channel A */ +#define HX711_GAIN_128 1/* gain = 128 for channel A */ + +struct hx711_data { +struct device *dev; +struct gpio_desc *gpiod_sck; +struct gpio_desc *gpiod_dout; +int gain_pulse; +struct mutex lock; +}; + +static int hx711_read (struct hx711_data *hx711_data) +{ +int i, ret; +int value =0 ;
Quoted 9 lines
drivers/iio/adc/hx711.c line 89
Quoted 4 lines
drivers/iio/adc/hx711.c new file +4
-0+ +mutex_lock (&hx711_data->lock); + +if (hx711_reset (hx711_data)) {If you reset the device before each conversion wont this clear the channel and gain selection? Wouldn't the driver always read from channel A at 128 gain regardless of what has been selected?
This is a bug, i need to fix. Thank you.
Quoted 55 lines
drivers/iio/adc/hx711.c line 92
Quoted 3 lines
drivers/iio/adc/hx711.c new file +3
-0+dev_err (hx711_data->dev,"reset failed!" ); +mutex_unlock (&hx711_data->lock); +return -1 ;If there is an error it should be propagated to the higher layers. At the moment you only return a bogus conversion value.
Quoted 38 lines
drivers/iio/adc/hx711.c new file +38
-0+ } + +for (i =0 ; i <24 ; i++) { + value <<=1 ; + ret =hx711_cycle (hx711_data); +if (ret) + value++; + } + + value ^=0x800000 ; + +for (i =0 ; i < hx711_data->gain_pulse; i++) + ret =hx711_cycle (hx711_data); + +mutex_unlock (&hx711_data->lock); + +return value; +} + +static int hx711_read_raw (struct iio_dev *iio_dev, +const struct iio_chan_spec *chan, +int *val,int *val2,long mask) +{ +struct hx711_data *hx711_data =iio_priv (iio_dev); + +switch (mask) { +case IIO_CHAN_INFO_RAW : +switch (chan->type) { +case IIO_VOLTAGE : + *val =hx711_read (hx711_data); +return IIO_VAL_INT ; +default : +return -EINVAL ; + } +default : +return -EINVAL ; + } +}[...]
drivers/iio/adc/hx711.c line 194
Quoted 2 lines
drivers/iio/adc/hx711.c new file +2
-0+static struct attribute *hx711_attributes[] = { + &iio_dev_attr_gain.dev_attr.attr,For IIO devices the gain is typically expressed through the scale attribute. Which is kind of the inverse of gain. It would be good if this driver follows this standard notation. The scale is the value of 1LSB in mV. So this includes the resolution of the ADC, the reference voltage and any gain that is applied to the input signal.
Quoted 2 lines
The possible values can be listed in the
scale_availableattribute.
The reference voltage is in the hardware.
Should i use a DT entry for the reference voltage? Or is it better to use a buildin scale and make it changeable?
Quoted 12 lines
Quoted 12 lines
drivers/iio/adc/hx711.c new file +12
-0+ NULL, +}; + +static struct attribute_group hx711_attribute_group = { + .attrs = hx711_attributes, +}; + +static const struct iio_info hx711_iio_info = { + .driver_module =THIS_MODULE , + .read_raw = hx711_read_raw, + .attrs = &hx711_attribute_group, +};
Quoted 9 lines
drivers/iio/adc/hx711.c line 211
Quoted 5 lines
drivers/iio/adc/hx711.c new file +5
-0+ +static const struct iio_chan_spec hx711_chan_spec[] = { + { .type =IIO_VOLTAGE , + .info_mask_separate = +BIT (IIO_CHAN_INFO_RAW ),Given that there are two separate physical input channels this should be expressed here and there should be two IIO channels for the device.
One who is toggling between channel A and B will cause a dummy read additional to every normal read.
Should i offer a "toggling mode" which means that after reading channel A the channel B is selected for the next read and after reading channel B channel A is selected? Simply expecting the channel is toggled on every read. If it's not toggled there need to be a dummy read, of course. This should be an attribute, right?
Quoted 79 lines
drivers/iio/adc/hx711.c line 218
Quoted 7 lines
drivers/iio/adc/hx711.c new file +7
-0+ }, +}; + +static int hx711_probe (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct hx711_data *hx711_data =NULL ;The = NULL is not needed as it is overwritten a few lines below.
drivers/iio/adc/hx711.c line 220
Quoted 2 lines
drivers/iio/adc/hx711.c new file +2
-0+struct iio_dev *iio; +int ret =0 ;Again = 0 no needed.
Quoted 17 lines
drivers/iio/adc/hx711.c new file +17
-0+ + iio =devm_iio_device_alloc (dev,sizeof (struct hx711_data )); +if (!iio) { +dev_err (dev,"failed to allocate IIO device\n" ); +return -ENOMEM ; + } + + hx711_data =iio_priv (iio); + hx711_data->dev = dev; + +mutex_init (&hx711_data->lock); + + hx711_data->gpiod_sck =devm_gpiod_get (dev,"sck" ,GPIOD_OUT_HIGH ); +if (IS_ERR (hx711_data->gpiod_sck)) { +dev_err (dev,"failed to get sck-gpiod: err=%ld\n" , +PTR_ERR (hx711_data->gpiod_sck)); +return PTR_ERR (hx711_data->gpiod_sck);drivers/iio/adc/hx711.c line 247
Quoted 10 lines
drivers/iio/adc/hx711.c new file +10
-0+ } + + hx711_data->gpiod_dout =devm_gpiod_get (dev,"dout" ,GPIOD_OUT_HIGH ); +if (IS_ERR (hx711_data->gpiod_dout)) { +dev_err (dev,"failed to get dout-gpiod: err=%ld\n" , +PTR_ERR (hx711_data->gpiod_dout)); +return PTR_ERR (hx711_data->gpiod_dout); + } + + ret =gpiod_direction_input (hx711_data->gpiod_dout);If dout is used as a input GPIO you should request it with
GPIOD_IN. In that case you can remove thegpiod_direction_input()call.drivers/iio/adc/hx711.c line 253
Quoted 6 lines
drivers/iio/adc/hx711.c new file +6
-0+if (ret <0 ) { +dev_err (hx711_data->dev,"gpiod_direction_input: %d\n" , ret); +return ret; + } + + ret =gpiod_direction_output (hx711_data->gpiod_sck,0 );Similar to above. If you want this to be a output GPIO with the default value of 0 request it with
GPIOD_OUT_LOW.drivers/iio/adc/hx711.c line 259
Quoted 6 lines
drivers/iio/adc/hx711.c new file +6
-0+if (ret <0 ) { +dev_err (hx711_data->dev,"gpiod_direction_output: %d\n" , ret); +return ret; + } + +platform_set_drvdata (pdev, iio);There is no matching
platform_get_drvdata()so this can probably be removed.drivers/iio/adc/hx711.c line 261
Quoted 2 lines
drivers/iio/adc/hx711.c new file +2
-0+ + iio->name = pdev->name;This should be the part name. E.g. "hx711" in this case.
Quoted 8 lines
drivers/iio/adc/hx711.c new file +8
-0+ iio->dev.parent = &pdev->dev; + iio->info = &hx711_iio_info; + iio->modes =INDIO_DIRECT_MODE ; + iio->channels = hx711_chan_spec; + iio->num_channels =ARRAY_SIZE (hx711_chan_spec); + +return devm_iio_device_register (dev, iio); +}
Quoted 9 lines · Andreas Klinger
Quoted 9 lines
drivers/iio/adc/hx711.c line 194
Quoted 2 lines
drivers/iio/adc/hx711.c new file +2
-0+static struct attribute *hx711_attributes[] = { + &iio_dev_attr_gain.dev_attr.attr,For IIO devices the gain is typically expressed through the scale attribute. Which is kind of the inverse of gain. It would be good if this driver follows this standard notation. The scale is the value of 1LSB in mV. So this includes the resolution of the ADC, the reference voltage and any gain that is applied to the input signal.
Quoted 6 lines
Quoted 2 lines
The possible values can be listed in the
scale_availableattribute.The reference voltage is in the hardware.
Should i use a DT entry for the reference voltage? Or is it better to use a buildin scale and make it changeable?
Typically the reference voltage is specified through the devicetree using a regulator. Have a look at e.g. ad7291.c.
Quoted 24 lines
Quoted 14 lines
Quoted 14 lines
drivers/iio/adc/hx711.c new file +12
-0+ NULL, +}; + +static struct attribute_group hx711_attribute_group = { + .attrs = hx711_attributes, +}; + +static const struct iio_info hx711_iio_info = { + .driver_module =THIS_MODULE , + .read_raw = hx711_read_raw, + .attrs = &hx711_attribute_group, +};Quoted 9 lines
drivers/iio/adc/hx711.c line 211
Quoted 5 lines
drivers/iio/adc/hx711.c new file +5
-0+ +static const struct iio_chan_spec hx711_chan_spec[] = { + { .type =IIO_VOLTAGE , + .info_mask_separate = +BIT (IIO_CHAN_INFO_RAW ),Given that there are two separate physical input channels this should be expressed here and there should be two IIO channels for the device.
Quoted 8 lines
One who is toggling between channel A and B will cause a dummy read additional to every normal read.
Should i offer a "toggling mode" which means that after reading channel A the channel B is selected for the next read and after reading channel B channel A is selected? Simply expecting the channel is toggled on every read. If it's not toggled there need to be a dummy read, of course. This should be an attribute, right?
I don't think that is necessary. On one hand when doing these kind of single shot conversions there is often not a predictable pattern on the other hand such custom device specific attributes are difficult to handle in a generic framework. We try to establish standard semantics so a generic application is able to talk to a wide range of devices. When a device implements custom attributes that becomes more difficult.
And there is also buffered mode. In buffered mode the application can select the channels which the converter should cycle through and connect a trigger like a timer to schedule the conversions. If switching between A and B in a circular way is required I'd recommend to implement this by using buffered mode.
Jonathan Cameron
maintainerQuoted 5 lines · Andreas Klinger
This is the IIO driver for AVIA HX711 ADC which ist mostly used in weighting cells.
The protocol is quite simple and using GPIOs:
One GPIO is used as clock (SCK) while another GPIO is read (DOUT)
Youch. Controlling the next conversion via the number of clocks is hideous! Oh well, guess it's one solution that limits the number of wires needed.
Still not as hideous as some ;) (sht15 I'm looking at you :)
Few comments inline.
Jonathan
Quoted 17 lines
The raw value read from the chip is delivered.
To get a weight one needs to subtract the zero offset and scale it.Signed-off-by Andreas Klinger <[email protected]>--- drivers/iio/adc/Kconfig | 18 +++ drivers/iio/adc/Makefile | 1 + drivers/iio/adc/hx711.c | 292 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 311 insertions(+) create mode 100644 drivers/iio/adc/hx711.cdrivers/iio/adc/Kconfig +0
-0
drivers/iio/adc/Kconfig config HI8435 · line 213
Quoted 9 lines
drivers/iio/adc/Kconfig +6
-0@@ -205,6 +205,24 @@ config HI8435 This driver can also be built as a module. If so, the module will be called hi8435. +config HX711 + tristate "AVIA HX711 ADC for weight cells" + depends on GPIOLIB + help + If you say yes here you get support for AVIA HX711 ADC which is used + for weight cells
Typically just called weigh cells rather than weight cells. One of those ugly bits of English.
drivers/iio/adc/Kconfig config HI8435 · line 216
Quoted 3 lines
drivers/iio/adc/Kconfig +3
-0+ + This driver uses two GPIOs, one for setting the clock and the other + one for getting the data
This driver uses two GPIOs, one acts as the clock and controls the channel selection and gain, the other is used for the measurement data (or something like that).
drivers/iio/adc/Kconfig config HI8435 · line 219
Quoted 3 lines
drivers/iio/adc/Kconfig +3
-0+ + Currently the raw value is read from the chip and delivered. + For getting an actual weight one needs to subtract the
To get an actual weight...
Quoted 32 lines
drivers/iio/adc/Kconfig +6
-0+ zero offset and multiply by a scale factor. + This should be done in userspace. + + This driver can also be built as a module. If so, the module will be + called hx711. + config INA2XX_ADC tristate "Texas Instruments INA2xx Power Monitors IIO driver" depends on I2C && !SENSORS_INA2XXdrivers/iio/adc/Makefile +1
-0@@ -21,6 +21,7 @@ obj-$(CONFIG_CC10001_ADC) += cc10001_adc.o obj-$(CONFIG_DA9150_GPADC) += da9150-gpadc.o obj-$(CONFIG_EXYNOS_ADC) += exynos_adc.o obj-$(CONFIG_HI8435) += hi8435.o +obj-$(CONFIG_HX711) += hx711.o obj-$(CONFIG_IMX7D_ADC) += imx7d_adc.o obj-$(CONFIG_INA2XX_ADC) += ina2xx-adc.o obj-$(CONFIG_LP8788_ADC) += lp8788_adc.odrivers/iio/adc/hx711.c new file +5
-0@@ -0,0 +1,292 @@ +/* +* HX711: analog to digital converter for weight sensor module +* +* Copyright (c) 2016 Andreas Klinger <[email protected]> +*
drivers/iio/adc/hx711.c line 15
Quoted 10 lines
drivers/iio/adc/hx711.c new file +10
-0+* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +*
No need for this blank line.
Quoted 25 lines
drivers/iio/adc/hx711.c new file +25
-0+*/ +#include <linux/err.h> +#include <linux/gpio.h> +#include <linux/gpio/consumer.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/property.h> +#include <linux/slab.h> +#include <linux/sched.h> +#include <linux/delay.h> +#include <linux/iio/iio.h> +#include <linux/iio/sysfs.h> + +#define HX711_GAIN_32 2/* gain = 32 for channel B */ +#define HX711_GAIN_64 3/* gain = 64 for channel A */ +#define HX711_GAIN_128 1/* gain = 128 for channel A */ + +struct hx711_data { +struct device *dev; +struct gpio_desc *gpiod_sck; +struct gpio_desc *gpiod_dout; +int gain_pulse; +struct mutex lock;
drivers/iio/adc/hx711.c line 50
Quoted 10 lines
drivers/iio/adc/hx711.c new file +10
-0+}; + +static int hx711_reset (struct hx711_data *hx711_data) +{ +int i, val; + + val =gpiod_get_value (hx711_data->gpiod_dout); +if (val) { +gpiod_set_value (hx711_data->gpiod_sck,1 ); +udelay (80 );
a comment here on why 80 would be good (it's bigger than 60?)
Quoted 8 lines
drivers/iio/adc/hx711.c new file +8
-0+gpiod_set_value (hx711_data->gpiod_sck,0 ); + +for (i =0 ; i <1000 ; i++) { + val =gpiod_get_value (hx711_data->gpiod_dout); +if (!val) +break ; +/* sleep at least 1 ms */ +msleep (1 );
drivers/iio/adc/hx711.c line 68
Quoted 10 lines
drivers/iio/adc/hx711.c new file +10
-0+ } + } + +return val; +} + +static int hx711_cycle (struct hx711_data *hx711_data) +{ +int val; +
/*
* if pre...
drivers/iio/adc/hx711.c line 74
Quoted 6 lines
drivers/iio/adc/hx711.c new file +6
-0+/* if preempted for more then 60us while SCK is high: +* hx711 is going in reset +* ==> measuring is false +*/ +preempt_disable (); +gpiod_set_value (hx711_data->gpiod_sck,1 );
I'm reading the datasheet as suggesting you need to wait at least 0.1 microsecs here...
Quoted 113 lines
drivers/iio/adc/hx711.c new file +113
-0+ val =gpiod_get_value (hx711_data->gpiod_dout); +gpiod_set_value (hx711_data->gpiod_sck,0 ); +preempt_enable (); + +return val; +} + +static int hx711_read (struct hx711_data *hx711_data) +{ +int i, ret; +int value =0 ; + +mutex_lock (&hx711_data->lock); + +if (hx711_reset (hx711_data)) { +dev_err (hx711_data->dev,"reset failed!" ); +mutex_unlock (&hx711_data->lock); +return -1 ; + } + +for (i =0 ; i <24 ; i++) { + value <<=1 ; + ret =hx711_cycle (hx711_data); +if (ret) + value++; + } + + value ^=0x800000 ; + +for (i =0 ; i < hx711_data->gain_pulse; i++) + ret =hx711_cycle (hx711_data); + +mutex_unlock (&hx711_data->lock); + +return value; +} + +static int hx711_read_raw (struct iio_dev *iio_dev, +const struct iio_chan_spec *chan, +int *val,int *val2,long mask) +{ +struct hx711_data *hx711_data =iio_priv (iio_dev); + +switch (mask) { +case IIO_CHAN_INFO_RAW : +switch (chan->type) { +case IIO_VOLTAGE : + *val =hx711_read (hx711_data); +return IIO_VAL_INT ; +default : +return -EINVAL ; + } +default : +return -EINVAL ; + } +} + +static ssize_t hx711_gain_show (struct device *dev, +struct device_attribute *attr, +char *buf) +{ +struct hx711_data *hx711_data =iio_priv (dev_to_iio_dev (dev)); +int val; + +switch (hx711_data->gain_pulse) { +case HX711_GAIN_32 : + val =32 ; +break ; +case HX711_GAIN_64 : + val =64 ; +break ; +default : + val =128 ; + } + +return sprintf (buf,"%d\n" , val); +} + +static ssize_t hx711_gain_store (struct device *dev, +struct device_attribute *attr, +const char *buf,size_t len) +{ +struct hx711_data *hx711_data =iio_priv (dev_to_iio_dev (dev)); +int ret, val; +int gain_save = hx711_data->gain_pulse; + + ret =kstrtoint ((const char *) buf,10 , &val); +if (ret) +return -EINVAL ; + +switch (val) { +case 32 : + hx711_data->gain_pulse =HX711_GAIN_32 ; +break ; +case 64 : + hx711_data->gain_pulse =HX711_GAIN_64 ; +break ; +case 128 : + hx711_data->gain_pulse =HX711_GAIN_128 ; +break ; +default : +return -EINVAL ; + } + +dev_dbg (hx711_data->dev,"gain_pulse: %d\n" , hx711_data->gain_pulse); + +/* if gain has changed do a fake read for the new gain to be set +* for the next read +*/ +if (gain_save != hx711_data->gain_pulse) +hx711_read (hx711_data); + +return len;
drivers/iio/adc/hx711.c line 197
Quoted 10 lines
drivers/iio/adc/hx711.c new file +10
-0+} + +static IIO_DEVICE_ATTR(gain, S_IRUGO | S_IWUSR, + hx711_gain_show, hx711_gain_store, 0); + +static struct attribute *hx711_attributes[] = { + &iio_dev_attr_gain.dev_attr.attr, +NULL , +}; +
As Lars suggested, please use standard ABI (easier if you use the info_mask elements and do it through read raw...
drivers/iio/adc/hx711.c line 216
Quoted 12 lines
drivers/iio/adc/hx711.c new file +12
-0+static struct attribute_group hx711_attribute_group = { + .attrs = hx711_attributes, +}; + +static const struct iio_info hx711_iio_info = { + .driver_module =THIS_MODULE , + .read_raw = hx711_read_raw, + .attrs = &hx711_attribute_group, +}; + +static const struct iio_chan_spec hx711_chan_spec[] = { + {
new line here is slightly nicer to read.
Quoted 22 lines
.type =
IIO_VOLTAGE,drivers/iio/adc/hx711.c new file +21
-0+ .info_mask_separate = + BIT(IIO_CHAN_INFO_RAW), + }, +}; + +static int hx711_probe (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct hx711_data *hx711_data =NULL ; +struct iio_dev *iio; +int ret =0 ; + + iio =devm_iio_device_alloc (dev,sizeof (struct hx711_data )); +if (!iio) { +dev_err (dev,"failed to allocate IIO device\n" ); +return -ENOMEM ; + } + + hx711_data =iio_priv (iio); + hx711_data->dev = dev; +
drivers/iio/adc/hx711.c line 240
Quoted 10 lines
drivers/iio/adc/hx711.c new file +10
-0+mutex_init (&hx711_data->lock); + + hx711_data->gpiod_sck =devm_gpiod_get (dev,"sck" ,GPIOD_OUT_HIGH ); +if (IS_ERR (hx711_data->gpiod_sck)) { +dev_err (dev,"failed to get sck-gpiod: err=%ld\n" , +PTR_ERR (hx711_data->gpiod_sck)); +return PTR_ERR (hx711_data->gpiod_sck); + } + + hx711_data->gpiod_dout =devm_gpiod_get (dev,"dout" ,GPIOD_OUT_HIGH );
If there is a reason for getting an input as an output then it wants a comment!
drivers/iio/adc/hx711.c line 253
Quoted 13 lines
drivers/iio/adc/hx711.c new file +13
-0+if (IS_ERR (hx711_data->gpiod_dout)) { +dev_err (dev,"failed to get dout-gpiod: err=%ld\n" , +PTR_ERR (hx711_data->gpiod_dout)); +return PTR_ERR (hx711_data->gpiod_dout); + } + + ret =gpiod_direction_input (hx711_data->gpiod_dout); +if (ret <0 ) { +dev_err (hx711_data->dev,"gpiod_direction_input: %d\n" , ret); +return ret; + } + + ret =gpiod_direction_output (hx711_data->gpiod_sck,0 );
Doesn't the flag above already mean we are in output mode for this pin?
Quoted 40 lines
drivers/iio/adc/hx711.c new file +39
-0+if (ret <0 ) { +dev_err (hx711_data->dev,"gpiod_direction_output: %d\n" , ret); +return ret; + } + +platform_set_drvdata (pdev, iio); + + iio->name = pdev->name; + iio->dev.parent = &pdev->dev; + iio->info = &hx711_iio_info; + iio->modes =INDIO_DIRECT_MODE ; + iio->channels = hx711_chan_spec; + iio->num_channels =ARRAY_SIZE (hx711_chan_spec); + +return devm_iio_device_register (dev, iio); +} + +static const struct of_device_id of_hx711_match[] = { + { .compatible ="avia,hx711" , }, + {}, +}; + +MODULE_DEVICE_TABLE (of, of_hx711_match); + +static struct platform_driver hx711_driver = { + .probe = hx711_probe, + .driver = { + .name ="hx711-gpio" , + .of_match_table = of_hx711_match, + }, +}; + +module_platform_driver (hx711_driver); + +MODULE_AUTHOR ("Andreas Klinger <[email protected]>" ); +MODULE_DESCRIPTION ("HX711 bitbanging driver - ADC for weight cells" ); +MODULE_LICENSE ("GPL" ); +MODULE_ALIAS ("platform:hx711-gpio" ); +