Jonathan Cameron writes:
>On 10/25/13 12:47, Harald Geyer wrote:
>> This driver handles DHT11 and DHT22 sensors.
>>
>> Signed-off-by: Harald Geyer <[email protected]>
>
> 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.
Well, was going to do so in a few days ... :)
> 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?
At least there are 'leds-gpio' and 'w1-gpio' already.
I was under the impression that drivers providing gpios
start with 'gpio' like 'gpio-mxs' ...
> Just go with dht11. This applies to the bindings as well.
I'm happy to grab 'dht11' but am reluctant to do so because
somebody might write a DHT11 driver using some other IO hardware
then gpio. How would this be called then?
> 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.
> 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.
I will change this according to the result of the naming
discussion.
>> + 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? */
Ok.
>> +
>> +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.
I will do so, if you think it's more apropriate. I figured that
commenting it out is like removing but retaining a useful comment.
Maybe prefix the whole comment with asterix to make it more obvious
this actually is a comment?
>> +/*
>> + * 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.
Yes, I'll send a v3 with this after the other issues are sorted
out.
Thanks,
Harald
>> +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 <[email protected]>");
>> +MODULE_DESCRIPTION("DHT11 humidity/temperature sensor driver");
>> +MODULE_LICENSE("GPL v2");
>> +
>>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>the body of a message to [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html
--
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
