Oki, believe Ive adressed all your suggestions.
diff --git a/drivers/input/touchscreen/jornada720_ts.c
b/drivers/input/touchscreen/jornada720_ts.c
new file mode 100644
index 0000000..124da74
--- /dev/null
+++ b/drivers/input/touchscreen/jornada720_ts.c
@@ -0,0 +1,172 @@
+/*
+ * drivers/input/touchscreen/jornada720_ts.c
+ *
+ * Copyright (C) 2007 Kristoffer Ericson <[EMAIL PROTECTED]>
+ *
+ * Copyright (C) 2006 Filip Zyzniewski <[EMAIL PROTECTED]>
+ * based on HP Jornada 56x touchscreen driver by Alex Lange <[EMAIL
PROTECTED]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * HP Jornada 710/720/729 Touchscreen Driver
+ */
+
+#include <linux/platform_device.h>
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+
+#include <asm/hardware.h>
+#include <asm/arch/jornada720.h>
+
+MODULE_AUTHOR("Kristoffer Ericson <[EMAIL PROTECTED]>");
+MODULE_DESCRIPTION("HP Jornada 710/720/728 touchscreen driver");
+MODULE_LICENSE("GPLv2");
+
+struct jornada_ts {
+ struct input_dev *dev;
+ int X[3]; /* X sample values */
+ int Y[3]; /* Y sample values */
+ int high_x, high_y;
+ int x, y; /* calculated value */
+};
+
+static void jornada720_ts_calculate(struct jornada_ts *jornada_ts)
+{
+ jornada_ts->X[0] |= (jornada_ts->high_x & 3) << 8;
+ jornada_ts->X[1] |= (jornada_ts->high_x & 0xc) << 6;
+ jornada_ts->X[2] |= (jornada_ts->high_x & 0x30) << 4;
+
+ jornada_ts->Y[0] |= (jornada_ts->high_y & 3) << 8;
+ jornada_ts->Y[1] |= (jornada_ts->high_y & 0xc) << 6;
+ jornada_ts->Y[2] |= (jornada_ts->high_y & 0x30) << 4;
+
+ jornada_ts->x = ((jornada_ts->X[0] + jornada_ts->X[1] + jornada_ts->X[2])
/ 3);
+ jornada_ts->y = ((jornada_ts->Y[0] + jornada_ts->Y[1] + jornada_ts->X[2])
/ 3);
+};
+
+static void jornada720_ts_collectdata(struct jornada_ts *jornada_ts)
+{
+ int i;
+
+ /* 3 low word X samples */
+ for (i = 0; i < 3; i++)
+ jornada_ts->X[i] = jornada_ssp_byte(TXDUMMY);
+ /* 3 low word Y samples */
+ for (i = 0; i < 3; i++)
+ jornada_ts->Y[i] = jornada_ssp_byte(TXDUMMY);
+
+ /* combined x samples bits */
+ jornada_ts->high_x = jornada_ssp_byte(TXDUMMY);
+ /* combined y samples bits */
+ jornada_ts->high_y = jornada_ssp_byte(TXDUMMY);
+}
+
+static irqreturn_t jornada720_ts_interrupt(int irq, void *dev_id)
+{
+ struct jornada_ts *jornada_ts = dev_id;
+
+ /* If GPIO_GPIO9 is set to high then report pen up */
+ if(GPLR & GPIO_GPIO(9)) {
+ input_report_key(jornada_ts->dev, BTN_TOUCH, 0);
+ input_sync(jornada_ts->dev);
+ return IRQ_HANDLED;
+ }
+
+ jornada_ssp_start();
+
+ /* proper reply to request is always TXDUMMY */
+ if (jornada_ssp_inout(GETTOUCHSAMPLES == TXDUMMY)) {
+ jornada720_ts_collectdata(jornada_ts);
+ jornada720_ts_calculate(jornada_ts);
+
+ input_report_key(jornada_ts->dev, BTN_TOUCH, 1);
+ input_report_abs(jornada_ts->dev, ABS_X, jornada_ts->x);
+ input_report_abs(jornada_ts->dev, ABS_Y, jornada_ts->y);
+ }
+
+ jornada_ssp_end();
+ return IRQ_HANDLED;
+}
+
+static int __devinit jornada720_ts_probe(struct platform_device *pdev)
+{
+ struct jornada_ts *jornada_ts;
+ struct input_dev *input_dev;
+ int ret;
+
+ jornada_ts = kzalloc(sizeof(struct jornada_ts), GFP_KERNEL);
+ if (!jornada_ts)
+ return -ENOMEM;
+
+ input_dev = input_allocate_device();
+ if (!input_dev)
+ goto failed1;
+
+ jornada_ts->dev = input_dev;
+
+ input_dev->evbit[0] = BIT(EV_KEY) | BIT(EV_ABS);
+ input_dev->absbit[0] = BIT(ABS_X) | BIT(ABS_Y) | BIT(ABS_PRESSURE);
+ input_dev->keybit[LONG(BTN_TOUCH)] = BIT(BTN_TOUCH);
+
+ input_set_abs_params(input_dev, ABS_X, 270, 3900, 0, 0);
+ input_set_abs_params(input_dev, ABS_Y, 180, 3700, 0, 0);
+
+ input_dev->name = "HP Jornada 710/720/728 Touchscreen driver";
+
+ ret = request_irq(IRQ_GPIO9,
+ jornada720_ts_interrupt,
+ IRQF_DISABLED | IRQF_TRIGGER_RISING,
+ "HP7XX Touchscreen driver", jornada_ts);
+ if (ret) {
+ printk(KERN_INFO "HP7XX TS : Unable to aquire irq!\n");
+ goto failed2;
+ }
+
+ if (input_register_device(input_dev))
+ goto failed2;
+
+ platform_set_drvdata(pdev, jornada_ts);
+
+ return 0;
+
+failed2:
+ input_free_device(input_dev);
+failed1:
+ kfree(jornada_ts);
+ return -ENODEV;
+}
+
+static int __devexit jornada720_ts_remove(struct platform_device *pdev)
+{
+ struct jornada_ts *jornada_ts = platform_get_drvdata(pdev);
+
+ free_irq(IRQ_GPIO9, pdev);
+ input_unregister_device(jornada_ts->dev);
+ kfree(jornada_ts);
+ return 0;
+}
+
+static struct platform_driver jornada720_ts_driver = {
+ .probe = jornada720_ts_probe,
+ .remove = __devexit_p(jornada720_ts_remove),
+ .driver = {
+ .name = "jornada_ts",
+ },
+};
+
+static int __init jornada720_ts_init(void)
+{
+ return platform_driver_register(&jornada720_ts_driver);
+}
+
+static void __exit jornada720_ts_exit(void)
+{
+ platform_driver_unregister(&jornada720_ts_driver);
+}
+
+module_init(jornada720_ts_init);
+module_exit(jornada720_ts_exit);
On Wed, 19 Sep 2007 15:19:36 -0400
"Dmitry Torokhov" <[EMAIL PROTECTED]> wrote:
> Hi Kristoffer,
>
> On 9/19/07, Kristoffer Ericson <[EMAIL PROTECTED]> wrote:
> > Greetings,
> >
> > Dmitry, I had some issues with the last patch I sent you. Something with
> > the init sequence of devices, it basicly made it kernel panic. I've
> > re-coded the driver to instead work as a platform driver, that way I have
> > more control of which gets started first.
>
> Yes, that's better. I guess the issue with the other driver was that
> you did not setup driver->bus and the kernel blew up in
> driver_register().
>
> > +
> > + /* start ssp and do a spinlock */
> > + jornada_ssp_start();
> > +
> > + /* request x & y data */
> > + if(jornada_ssp_inout(GETTOUCHSAMPLES) != TXDUMMY) {
> > + jornada_ssp_end();
> > + return IRQ_HANDLED;
> > + }
> > +
> ...
> > + /* end ssp communication and release spinlock */
> > + jornada_ssp_end();
>
> I really prefer acquire/release type of functions to be in pairs. I.e.
> I'd prefer something like:
>
> + /* start ssp and do a spinlock */
> + jornada_ssp_start();
> +
> + /* request x & y data */
> + if (jornada_ssp_inout(GETTOUCHSAMPLES) != TXDUMMY) {
> + jornada_parse_data(...);
> + jornada_post_data(...)
> + }
> + jornada_ssp_end();
> + return IRQ_HANDLED;
>
> > + /* report pen down */
> > + input_report_key(jornada_ts->dev, BTN_TOUCH, 1);
> > + input_report_abs(jornada_ts->dev, ABS_X, jornada_ts->x);
> > + input_report_abs(jornada_ts->dev, ABS_Y, jornada_ts->y);
> > + input_report_abs(jornada_ts->dev, ABS_PRESSURE, 1);
>
> The device does not really report pressure value, so I'd not report
> ABS_PRESSURE. BTN_TOUCH should be enogh.
>
> > +
> > +static int __init jornada720_ts_probe(struct platform_device *pdev)
>
> __devinit.
>
> > +{
> > + struct jornada_ts *jornada_ts;
> > + struct input_dev *input_dev;
> > + int ret;
> > +
> > + input_dev = input_allocate_device();
> > + if (!input_dev)
> > + return -ENODEV;
> > +
> > + jornada_ts = kzalloc(sizeof(struct jornada_ts), GFP_KERNEL);
> > + if (!jornada_ts)
> > + goto failed1;
> > +
> > + platform_set_drvdata(pdev, jornada_ts);
> > +
> > + jornada_ts->dev = input_dev;
> > +
> > + input_dev->evbit[0] = BIT(EV_KEY) | BIT(EV_ABS);
> > + input_dev->absbit[0] = BIT(ABS_X) | BIT(ABS_Y) | BIT(ABS_PRESSURE);
> > + input_dev->keybit[LONG(BTN_TOUCH)] = BIT(BTN_TOUCH);
> > +
> > + input_dev->absmin[ABS_X] = 270;
> > + input_dev->absmin[ABS_Y] = 180;
> > + input_dev->absmax[ABS_X] = 3900;
> > + input_dev->absmax[ABS_Y] = 3700;
>
> I like using input_set_abs_params(input_dev, ABS_X, 270, 3900, 0, 0); ...
>
> > +
> > + input_dev->name = "HP Jornada 710/720/728 Touchscreen driver";
> > +
> > + ret = request_irq(IRQ_GPIO9,
> > + jornada720_ts_interrupt,
> > + IRQF_DISABLED | IRQF_TRIGGER_RISING,
> > + "HP7XX Touchscreen driver", jornada_ts);
> > + if (ret) {
> > + printk(KERN_INFO "HP7XX TS : Unable to aquire irq!\n");
> > + goto failed2;
> > + }
> > +
> > + input_register_device(input_dev);
> > + return 0;
>
> Error handling please.
>
> > +
> > +failed1:
> > + input_free_device(input_dev);
> > + return -ENOMEM;
> > +failed2:
> > + kfree(jornada_ts);
> > + input_free_device(input_dev);
>
> This leaves invalid pointer in platform data structure. I'd recommend
> calling platform_set_drvdata(pdev, jornada_ts); ony after making sure
> that input_register_device() was successfull.
>
> > + return -ENODEV;
>
> If you use out-of-line error handling path please make it have single
> return point. Btw, if you allocate all memity that is needed first and
> then check for success you can fold the erro handling path somewhat.
>
> > +}
> > +
> > +static int jornada720_ts_remove(struct platform_device *pdev)
>
> __devexit.
>
> > +{
> > + struct jornada_ts *jornada_ts = platform_get_drvdata(pdev);
> > +
> > + free_irq(IRQ_GPIO9, pdev);
> > + input_unregister_device(jornada_ts->dev);
> > + kfree(jornada_ts);
> > + return 0;
> > +}
> > +
> > +static struct platform_driver jornada720_ts_driver = {
> > + .probe = jornada720_ts_probe,
> > + .remove = jornada720_ts_remove,
>
> __devexit_p(jornada720_ts_remove)
>
> > + .driver = {
> > + .name = "jornada_ts_driver",
>
> I don't think platform code will match on this name. I'd expect simply
> "jornada_ts" but it really depends on how you create the platform
> device.
>
> > + },
> > +};
> > +
> > +static int __devinit jornada720_ts_init(void)
>
> This one should be simply __init.
>
> > +{
> > + return platform_driver_register(&jornada720_ts_driver);
> > +}
> > +
> > +static void __exit jornada720_ts_exit(void)
> > +{
> > + platform_driver_unregister(&jornada720_ts_driver);
> > +}
> > +
> > +module_init(jornada720_ts_init);
> > +module_exit(jornada720_ts_exit);
> >
> >
>
>
> --
> Dmitry
hp7xx-touchscreen-support.patch
Description: Binary data
