Hi,
On 12/28/2013 02:10 AM, Dmitry Torokhov wrote:
Hi Hans,
On Fri, Dec 27, 2013 at 03:59:56PM +0100, Hans de Goede wrote:
+
+ 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:
Can we please not introduce gotos where they are not needed? If you
concerned about extra indent just split touchscreen handling into a
separate function and then do
if (ts->input)
sun4i_ts_handle_touchscreen_data(...);
Fixed.
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);
Hmm, it would be nice we touchscreen methods only affected touchscreen
functionality. Can you read current state and adjust it as needed
instead of clobbering it?
I could do a read-modify-write of TP_INT_FIFOC here, but I already
considered that and I decided not to do it because ...
Then you could do away with remove() method altogether.
No it will still be needed to clear the TEMP_IRQ_EN bit in
TP_INT_FIFOC. And it will still need to either explicitly deregister
the input_dev, or take the input_dev mutex to avoid the modification of
TP_INT_FIFOC in _remove racing with the read-modify-write in _close.
So I decided to keep this kiss, and simply force _close to run
before _remove clears the TEMP_IRQ_EN bit by doing an explicit
unregister before this.
}
+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;
+ 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);
+ }
+
+ 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);
Given that we now can't avoid freeing irq before freeing ts memory or
input device I think you can do away with synchronize_irq() here and
elsewhere as freeing irq is guaranteed to wait for current interrupt to
complete.
Ah yes, removed all calls to synchronize_irq.
I'll run some tests to ensure I did not accidentally break anything and
then I'll send a v3.
+ 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
Thanks.
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
