Re: [PATCH v2 2/2] iio: Add new driver dht11-gpio

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 10/25/13 12:47, Harald Geyer wrote:
> This driver handles DHT11 and DHT22 sensors.
>
> Signed-off-by: Harald Geyer <harald@xxxxxxxxx>

Hi Harald,

I'm afraid I completely missed your posting of this version 2.

Do feel free to pester me if I fail to say anything about a patch
for a couple of weeks.

Anyhow
> ---
> changes since v1 (tested on DHT11):
> Add dependency on GPIOLIB
> Prefix all local preprocessor macros with DHT11_
> Rename STARTUP to START_TRANSMISSION
> Remove leading zeros
> Simplify channel disambiguation
> Remove obvious comments
> Make whitespace more consistent
> Strip unnecessary output and simplify error handling
> Fix spelling errors
>
> The v1 patch has been tested with DHT11 and DHT22 hardware.
There are a few redundant bits below that want to be dropped.

The only real issue I have otherwise is with the naming.
I would expect *-gpio to be a driver providing gpios?
Just go with dht11.  This applies to the bindings as well.

Also note that policy is now to cc the device tree list for
all patches changing or introducing bindings.

I've cc'd them on this reply.

Thanks,

Jonathan
>
> Thanks,
> Harald
>
>  .../bindings/iio/humidity/dht11-gpio.txt           |   14 +
>  drivers/iio/Kconfig                                |    1 +
>  drivers/iio/Makefile                               |    1 +
>  drivers/iio/humidity/Kconfig                       |   15 +
>  drivers/iio/humidity/Makefile                      |    5 +
>  drivers/iio/humidity/dht11-gpio.c                  |  325 ++++++++++++++++++++
>  6 files changed, 361 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/iio/humidity/dht11-gpio.txt
>  create mode 100644 drivers/iio/humidity/Kconfig
>  create mode 100644 drivers/iio/humidity/Makefile
>  create mode 100644 drivers/iio/humidity/dht11-gpio.c
>
> diff --git a/Documentation/devicetree/bindings/iio/humidity/dht11-gpio.txt b/Documentation/devicetree/bindings/iio/humidity/dht11-gpio.txt
> new file mode 100644
> index 0000000..ee8fa04
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/humidity/dht11-gpio.txt
> @@ -0,0 +1,14 @@
> +* DHT11 humidity/temperature sensor (and compatibles like DHT22)
> +
> +Required properties:
> +  - compatible: Should be "dht11-gpio"
> +  - gpios: Should specify the GPIO connected to the sensor's data
> +    line, see "gpios property" in
> +    Documentation/devicetree/bindings/gpio/gpio.txt.
> +
> +Example:
> +
> +humidity_sensor {
> +	compatible = "dht11-gpio";
> +	gpios = <&gpio0 6 0>;
> +}
> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
> index 90cf0cd..a5ed882 100644
> --- a/drivers/iio/Kconfig
> +++ b/drivers/iio/Kconfig
> @@ -65,6 +65,7 @@ source "drivers/iio/common/Kconfig"
>  source "drivers/iio/dac/Kconfig"
>  source "drivers/iio/frequency/Kconfig"
>  source "drivers/iio/gyro/Kconfig"
> +source "drivers/iio/humidity/Kconfig"
>  source "drivers/iio/imu/Kconfig"
>  source "drivers/iio/light/Kconfig"
>  source "drivers/iio/magnetometer/Kconfig"
> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
> index bcf7e9e..b3b5096 100644
> --- a/drivers/iio/Makefile
> +++ b/drivers/iio/Makefile
> @@ -18,6 +18,7 @@ obj-y += common/
>  obj-y += dac/
>  obj-y += gyro/
>  obj-y += frequency/
> +obj-y += humidity/
>  obj-y += imu/
>  obj-y += light/
>  obj-y += magnetometer/
> diff --git a/drivers/iio/humidity/Kconfig b/drivers/iio/humidity/Kconfig
> new file mode 100644
> index 0000000..bbc44f2
> --- /dev/null
> +++ b/drivers/iio/humidity/Kconfig
> @@ -0,0 +1,15 @@
> +#
> +# humidity sensor drivers
> +#
> +menu "Humidity sensors"
> +
> +config DHT11_GPIO
The _GPIO to my mind would normally imply that this as a driver
providing GPIOs rather than using them.
> +	tristate "DHT11 (and compatible sensors) driver"
> +	depends on GPIOLIB
> +	help
> +	  This driver supports reading data via a single interrupt
> +	  generating GPIO line. Currently tested are DHT11 and DHT22.
> +	  Other sensors should work as well as long as they speak the
> +	  same protocol.
> +
> +endmenu
> diff --git a/drivers/iio/humidity/Makefile b/drivers/iio/humidity/Makefile
> new file mode 100644
> index 0000000..5e8226f
> --- /dev/null
> +++ b/drivers/iio/humidity/Makefile
> @@ -0,0 +1,5 @@
> +#
> +# Makefile for IIO humidity sensor drivers
> +#
> +
> +obj-$(CONFIG_DHT11_GPIO) += dht11-gpio.o
> diff --git a/drivers/iio/humidity/dht11-gpio.c b/drivers/iio/humidity/dht11-gpio.c
> new file mode 100644
> index 0000000..3bb4d81
> --- /dev/null
> +++ b/drivers/iio/humidity/dht11-gpio.c
> @@ -0,0 +1,325 @@
...
Don't bother with this comment.  If anyone wants it they can
add support :)
> +/* TODO?: Support systems without DT? */
> +
> +struct dht11_gpio {
> +	struct device			*dev;
> +
> +	int				gpio;
> +	int				irq;
> +
> +	struct completion		completion;
> +
> +	s64				timestamp;
> +	int				temperature;
> +	int				humidity;
> +
> +	/* num_edges: -1 means "no transmission in progress" */
> +	int				num_edges;
> +	struct {s64 ts; int value; }	edges[DHT11_EDGES_PER_READ];
> +};
> +
This isn't called anywhere and so should be removed from the driver.
> +/*
> + * dht11_edges_print: show the data as actually received by the
> + *                    driver.
> + * This is dead code, keeping it for now just in case somebody needs
> + * it for porting the driver to new sensor HW, etc.
> + *
> +static void dht11_edges_print(struct dht11_gpio *dht11)
> +{
> +	int i;
> +
> +	for (i = 1; i < dht11->num_edges; ++i) {
> +		pr_err("dht11-gpio: %d: %lld ns %s\n", i,
> +			dht11->edges[i].ts - dht11->edges[i-1].ts,
> +			dht11->edges[i-1].value ? "high" : "low");
> +	}
> +}
> +*/
...

> +static const struct of_device_id dht11_gpio_dt_ids[] = {
> +	{ .compatible = "dht11-gpio", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, dht11_gpio_dt_ids);
> +
> +static int dht11_gpio_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *node = dev->of_node;
> +	struct dht11_gpio *dht11;
> +	struct iio_dev *iio;
> +	int ret;
> +
> +	iio = devm_iio_device_alloc(dev, sizeof(*dht11));
> +	if (!iio) {
> +		dev_err(dev, "Failed to allocate IIO device\n");
> +		return -ENOMEM;
> +	}
> +
> +	dht11 = iio_priv(iio);
> +	dht11->dev = dev;
> +
> +	dht11->gpio = ret = of_get_gpio(node, 0);
> +	if (ret < 0)
> +		return ret;
> +	ret = devm_gpio_request_one(dev, dht11->gpio, GPIOF_IN, pdev->name);
> +	if (ret)
> +		return ret;
> +
> +	dht11->irq = gpio_to_irq(dht11->gpio);
> +	if (dht11->irq < 0) {
> +		dev_err(dev, "GPIO %d has no interrupt\n", dht11->gpio);
> +		return -EINVAL;
> +	}
> +	ret = devm_request_irq(dev, dht11->irq, dht11_gpio_handle_irq,
> +				IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> +				pdev->name, iio);
> +	if (ret)
> +		return ret;
> +
> +	dht11->timestamp = iio_get_time_ns() - DHT11_DATA_VALID_TIME - 1;
> +	dht11->num_edges = -1;
> +
> +	platform_set_drvdata(pdev, iio);
> +
> +	init_completion(&dht11->completion);
> +	iio->name = pdev->name;
> +	iio->dev.parent = &pdev->dev;
> +	iio->info = &dht11_gpio_iio_info;
> +	iio->modes = INDIO_DIRECT_MODE;
> +	iio->channels = dht11_gpio_chan_spec;
> +	iio->num_channels = ARRAY_SIZE(dht11_gpio_chan_spec);
> +
> +	return iio_device_register(iio);
> +}
> +

Could use the new devm_iio_device_register() call and drop
this boilerplate.

> +static int dht11_gpio_remove(struct platform_device *pdev)
> +{
> +	struct iio_dev *iio = platform_get_drvdata(pdev);
> +
> +	iio_device_unregister(iio);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver dht11_gpio_driver = {
> +	.driver = {
> +		.name	= DRIVER_NAME,
> +		.owner	= THIS_MODULE,
> +		.of_match_table = dht11_gpio_dt_ids,
> +	},
> +	.probe  = dht11_gpio_probe,
> +	.remove = dht11_gpio_remove,
> +};
> +
> +module_platform_driver(dht11_gpio_driver);
> +
> +MODULE_AUTHOR("Harald Geyer <harald@xxxxxxxxx>");
> +MODULE_DESCRIPTION("DHT11 humidity/temperature sensor driver");
> +MODULE_LICENSE("GPL v2");
> +
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux