On Fri, Dec 27, 2013 at 03:59:56PM +0100, Hans de Goede wrote:
> The sun4i resisitive touchscreen controller also comes with a built-in
> temperature sensor. This commit adds support for it.
>
> This commit also introduces a new "ts-attached" device-tree property,
> when this is not set, the input part of the driver won't register. This way
> the internal temperature sensor can be used to measure the SoC temperature
> independent of there actually being a touchscreen attached to the controller.
>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
Hi Hans,
Almost good, except for a couple of nitpicks and one moe question.
Assuming you fix the nitpicks and the question is not a concern,
feel free to add
Reviewed-by: Guenter Roeck <linux@xxxxxxxxxxxx>
Guenter
> ---
> .../bindings/input/touchscreen/sun4i.txt | 6 +
> drivers/input/touchscreen/sun4i-ts.c | 129 ++++++++++++++++-----
> 2 files changed, 109 insertions(+), 26 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt b/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt
> index e45927e..1fffd11 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt
> @@ -6,10 +6,16 @@ Required properties:
> - reg: mmio address range of the chip
> - interrupts: interrupt to which the chip is connected
>
> +Optional properties:
> + - ts-attached: boolean set this to tell the driver that an actual touchscreen
> + is attached and that it should register an input device,
> + without this it only registers the builtin temperate sensor
> +
> Example:
>
> rtp: rtp@01c25000 {
> compatible = "allwinner,sun4i-ts";
> reg = <0x01c25000 0x100>;
> interrupts = <29>;
> + ts-attached;
> };
> diff --git a/drivers/input/touchscreen/sun4i-ts.c b/drivers/input/touchscreen/sun4i-ts.c
> index 10839d2..2d437f6 100644
> --- a/drivers/input/touchscreen/sun4i-ts.c
> +++ b/drivers/input/touchscreen/sun4i-ts.c
> @@ -3,6 +3,9 @@
> *
> * Copyright (C) 2013 - 2014 Hans de Goede <hdegoede@xxxxxxxxxx>
> *
> + * The hwmon parts are based on work by Corentin LABBE which is:
> + * Copyright (C) 2013 Corentin LABBE <clabbe.montjoie@xxxxxxxxx>
> + *
> * 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
> @@ -30,6 +33,7 @@
> */
>
> #include <linux/err.h>
> +#include <linux/hwmon.h>
> #include <linux/init.h>
> #include <linux/input.h>
> #include <linux/interrupt.h>
> @@ -106,6 +110,7 @@ struct sun4i_ts_data {
> void __iomem *base;
> unsigned int irq;
> bool ignore_fifo_data;
> + int temp_data;
> };
>
> static irqreturn_t sun4i_ts_irq(int irq, void *dev_id)
> @@ -115,6 +120,12 @@ static irqreturn_t sun4i_ts_irq(int irq, void *dev_id)
>
> reg_val = readl(ts->base + TP_INT_FIFOS);
>
> + if (reg_val & TEMP_DATA_PENDING)
> + ts->temp_data = readl(ts->base + TEMP_DATA);
Is this read guaranteed to return 0 in the upper bit, or in other words is it
guaranteed to never return 0xffffffff ? Otherwise there might be a problem with
the implicit conversion to a negative integer.
> +
> + if (!ts->input)
> + goto leave;
> +
> if (reg_val & FIFO_DATA_PENDING) {
> x = readl(ts->base + TP_DATA);
> y = readl(ts->base + TP_DATA);
> @@ -140,6 +151,7 @@ static irqreturn_t sun4i_ts_irq(int irq, void *dev_id)
> input_sync(ts->input);
> }
>
> +leave:
> writel(reg_val, ts->base + TP_INT_FIFOS);
>
> return IRQ_HANDLED;
> @@ -149,9 +161,9 @@ static int sun4i_ts_open(struct input_dev *dev)
> {
> struct sun4i_ts_data *ts = input_get_drvdata(dev);
>
> - /* Flush, set trig level to 1, enable data and up irqs */
> - writel(DATA_IRQ_EN(1) | FIFO_TRIG(1) |FIFO_FLUSH(1) | TP_UP_IRQ_EN(1),
> - ts->base + TP_INT_FIFOC);
> + /* Flush, set trig level to 1, enable temp, data and up irqs */
> + writel(TEMP_IRQ_EN(1) | DATA_IRQ_EN(1) | FIFO_TRIG(1) | FIFO_FLUSH(1) |
> + TP_UP_IRQ_EN(1), ts->base + TP_INT_FIFOC);
>
> return 0;
> }
> @@ -160,40 +172,76 @@ static void sun4i_ts_close(struct input_dev *dev)
> {
> struct sun4i_ts_data *ts = input_get_drvdata(dev);
>
> - /* Deactivate all IRQs */
> - writel(0, ts->base + TP_INT_FIFOC);
> + /* Deactivate all input IRQs */
> + writel(TEMP_IRQ_EN(1), ts->base + TP_INT_FIFOC);
> synchronize_irq(ts->irq);
> }
>
> +static ssize_t show_temp(struct device *dev, struct device_attribute *devattr,
> + char *buf)
> +{
> + struct sun4i_ts_data *ts = dev_get_drvdata(dev);
> +
> + /* No temp_data until the first irq */
> + if (ts->temp_data == -1)
> + return -EAGAIN;
> +
> + return sprintf(buf, "%d\n", (ts->temp_data - 1447) * 100);
> +}
> +
> +static ssize_t show_temp_label(struct device *dev,
> + struct device_attribute *devattr, char *buf)
> +{
> + return sprintf(buf, "SoC temperature\n");
> +}
> +
> +static DEVICE_ATTR(temp1_input, S_IRUGO, show_temp, NULL);
> +static DEVICE_ATTR(temp1_label, S_IRUGO, show_temp_label, NULL);
> +
> +static struct attribute *sun4i_ts_attrs[] = {
> + &dev_attr_temp1_input.attr,
> + &dev_attr_temp1_label.attr,
> + NULL
> +};
> +ATTRIBUTE_GROUPS(sun4i_ts);
> +
> static int sun4i_ts_probe(struct platform_device *pdev)
> {
> struct sun4i_ts_data *ts;
> struct device *dev = &pdev->dev;
> + struct device_node *np =dev->of_node;
Missing space after '=' (checkpatch error).
> + struct device *hwmon;
> int ret;
> + bool ts_attached;
> +
> + ts_attached = of_property_read_bool(np, "ts-attached");
>
> ts = devm_kzalloc(dev, sizeof(struct sun4i_ts_data), GFP_KERNEL);
> if (!ts)
> return -ENOMEM;
>
> ts->dev = dev;
> -
> - ts->input = devm_input_allocate_device(dev);
> - if (!ts->input)
> - return -ENOMEM;
> -
> - ts->input->name = pdev->name;
> - ts->input->phys = "sun4i_ts/input0";
> - ts->input->open = sun4i_ts_open;
> - ts->input->close = sun4i_ts_close;
> - ts->input->id.bustype = BUS_HOST;
> - ts->input->id.vendor = 0x0001;
> - ts->input->id.product = 0x0001;
> - ts->input->id.version = 0x0100;
> - ts->input->evbit[0] = BIT(EV_SYN) | BIT(EV_KEY) | BIT(EV_ABS);
> - set_bit(BTN_TOUCH, ts->input->keybit);
> - input_set_abs_params(ts->input, ABS_X, 0, 4095, 0, 0);
> - input_set_abs_params(ts->input, ABS_Y, 0, 4095, 0, 0);
> - input_set_drvdata(ts->input, ts);
> + ts->temp_data = -1;
> +
> + if (ts_attached) {
> + ts->input = devm_input_allocate_device(dev);
> + if (!ts->input)
> + return -ENOMEM;
> +
> + ts->input->name = pdev->name;
> + ts->input->phys = "sun4i_ts/input0";
> + ts->input->open = sun4i_ts_open;
> + ts->input->close = sun4i_ts_close;
> + ts->input->id.bustype = BUS_HOST;
> + ts->input->id.vendor = 0x0001;
> + ts->input->id.product = 0x0001;
> + ts->input->id.version = 0x0100;
> + ts->input->evbit[0] = BIT(EV_SYN) | BIT(EV_KEY) | BIT(EV_ABS);
> + set_bit(BTN_TOUCH, ts->input->keybit);
> + input_set_abs_params(ts->input, ABS_X, 0, 4095, 0, 0);
> + input_set_abs_params(ts->input, ABS_Y, 0, 4095, 0, 0);
> + input_set_drvdata(ts->input, ts);
> + }
>
> ts->base = devm_ioremap_resource(dev,
> platform_get_resource(pdev, IORESOURCE_MEM, 0));
> @@ -232,14 +280,42 @@ static int sun4i_ts_probe(struct platform_device *pdev)
> writel(STYLUS_UP_DEBOUN(5) | STYLUS_UP_DEBOUN_EN(1) | TP_MODE_EN(1),
> ts->base + TP_CTRL1);
>
> - ret = input_register_device(ts->input);
> - if (ret)
> - return ret;
> + hwmon = devm_hwmon_device_register_with_groups(ts->dev, "sun4i_ts",
> + ts, sun4i_ts_groups);
> + if (IS_ERR(hwmon)) {
> + return PTR_ERR(hwmon);
> + }
{ } are not needed here (checkpatch warning).
> +
> + writel(TEMP_IRQ_EN(1), ts->base + TP_INT_FIFOC);
> +
> + if (ts_attached) {
> + ret = input_register_device(ts->input);
> + if (ret) {
> + writel(0, ts->base + TP_INT_FIFOC);
> + synchronize_irq(ts->irq);
> + return ret;
> + }
> + }
>
> platform_set_drvdata(pdev, ts);
> return 0;
> }
>
> +static int sun4i_ts_remove(struct platform_device *pdev)
> +{
> + struct sun4i_ts_data *ts = platform_get_drvdata(pdev);
> +
> + /* Explicit unregister to avoid open/close changing the imask later */
> + if (ts->input)
> + input_unregister_device(ts->input);
> +
> + /* Deactivate all IRQs */
> + writel(0, ts->base + TP_INT_FIFOC);
> + synchronize_irq(ts->irq);
> +
> + return 0;
> +}
> +
> static const struct of_device_id sun4i_ts_of_match[] = {
> { .compatible = "allwinner,sun4i-ts", },
> { /* sentinel */ }
> @@ -253,6 +329,7 @@ static struct platform_driver sun4i_ts_driver = {
> .of_match_table = of_match_ptr(sun4i_ts_of_match),
> },
> .probe = sun4i_ts_probe,
> + .remove = sun4i_ts_remove,
> };
>
> module_platform_driver(sun4i_ts_driver);
> --
> 1.8.4.2
>
>
> _______________________________________________
> lm-sensors mailing list
> lm-sensors@xxxxxxxxxxxxxx
> http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
