* Re: [PATCH] Add quirk HID_QUIRK_NO_INIT_REPORTS for RNDPLUS touchscreen
From: Benjamin Tissoires @ 2014-03-21 20:58 UTC (permalink / raw)
To: Yufeng Shen
Cc: linux-input, Jiri Kosina, linux-kernel@vger.kernel.org, linux-usb,
Andrew de los Reyes
In-Reply-To: <1395430798-25606-1-git-send-email-miletus@chromium.org>
On Fri, Mar 21, 2014 at 3:39 PM, Yufeng Shen <miletus@chromium.org> wrote:
> There is timeout error during initialization:
> kernel: [ 14.167086] hid-multitouch 0003:2512:5003.0001: usb_submit_urb(ctrl) failed: -1
> kernel: [ 14.167135] hid-multitouch 0003:2512:5003.0001: timeout initializing reports
> kernel: [ 14.167407] input: RNDPLUS Co., LTD PULSEIR TSR4601 as /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.3/2-1.3:1.0/input/input14
> kernel: [ 14.167975] cpufreq_interactive: monitoring input on RNDPLUS Co., LTD PULSEIR TSR4601
> kernel: [ 14.168266] hid-multitouch 0003:2512:5003.0001: input,hiddev0,hidraw1: USB HID v1.10 Mouse [RNDPLUS Co., LTD PULSEIR TSR4601] on usb-0000:00:1d.0-1.3/input0
>
> Adding quirk HID_QUIRK_NO_INIT_REPORTS can solve the problem.
I already asked you to test if HID_QUIRK_NO_INIT_INPUT_REPORTS was
working for the same same kind of timeout
(https://patchwork.kernel.org/patch/3544281/).
So I am asking again: please test HID_QUIRK_NO_INIT_INPUT_REPORTS and
if it works, use this quirk instead the one in this patch.
This *really* matters because the quirk HID_QUIRK_NO_INIT_REPORTS does
not initialize feature reports which contains important information
regarding hid-multitouch.
Cheers,
Benjamin
>
> Signed-off-by: Yufeng Shen <miletus@chromium.org>
> ---
> drivers/hid/hid-ids.h | 5 +++++
> drivers/hid/usbhid/hid-quirks.c | 3 +++
> 2 files changed, 8 insertions(+)
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index f9304cb..772f937 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -442,6 +442,11 @@
> #define USB_DEVICE_ID_IDEACOM_IDC6650 0x6650
> #define USB_DEVICE_ID_IDEACOM_IDC6651 0x6651
>
> +#define USB_VENDOR_ID_IDSPULSE 0x2512
> +#define USB_DEVICE_ID_IDSPULSE_EVIR1 0x5003
> +#define USB_DEVICE_ID_IDSPULSE_EVIR2 0x5004
> +#define USB_DEVICE_ID_IDSPULSE_EVIR3 0x5005
> +
> #define USB_VENDOR_ID_ILITEK 0x222a
> #define USB_DEVICE_ID_ILITEK_MULTITOUCH 0x0001
>
> diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
> index 0db9a67..e59ad07 100644
> --- a/drivers/hid/usbhid/hid-quirks.c
> +++ b/drivers/hid/usbhid/hid-quirks.c
> @@ -72,6 +72,9 @@ static const struct hid_blacklist {
> { USB_VENDOR_ID_ELO, USB_DEVICE_ID_ELO_TS2700, HID_QUIRK_NOGET },
> { USB_VENDOR_ID_FORMOSA, USB_DEVICE_ID_FORMOSA_IR_RECEIVER, HID_QUIRK_NO_INIT_REPORTS },
> { USB_VENDOR_ID_FREESCALE, USB_DEVICE_ID_FREESCALE_MX28, HID_QUIRK_NOGET },
> + { USB_VENDOR_ID_IDSPULSE, USB_DEVICE_ID_IDSPULSE_EVIR1, HID_QUIRK_NO_INIT_REPORTS },
> + { USB_VENDOR_ID_IDSPULSE, USB_DEVICE_ID_IDSPULSE_EVIR2, HID_QUIRK_NO_INIT_REPORTS },
> + { USB_VENDOR_ID_IDSPULSE, USB_DEVICE_ID_IDSPULSE_EVIR3, HID_QUIRK_NO_INIT_REPORTS },
> { USB_VENDOR_ID_MGE, USB_DEVICE_ID_MGE_UPS, HID_QUIRK_NOGET },
> { USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GX680R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS },
> { USB_VENDOR_ID_NOVATEK, USB_DEVICE_ID_NOVATEK_MOUSE, HID_QUIRK_NO_INIT_REPORTS },
> --
> 1.9.1.423.g4596e3a
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: [PATCH 1/3] Input: synaptics-rmi4 - add capabilities for touchpads
From: Christopher Heiny @ 2014-03-21 22:24 UTC (permalink / raw)
To: Benjamin Tissoires, Dmitry Torokhov
Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
Daniel Rosenberg, Linus Walleij, David Herrmann, Jiri Kosina
In-Reply-To: <5329A9CE.1080504@redhat.com>
On 03/19/2014 07:29 AM, Benjamin Tissoires wrote:
> Hi Chris,
>
> On 03/18/2014 09:03 PM, Christopher Heiny wrote:
>> When the device is a touchpad additional capabilities need to
>> be set and reported.
>>
>
> We have a problem here. While this patch would have been fine in the
> pre-v3.8 era, it is not true anymore.
> However, the current branch where synaptics-rmi4 is attached is v3.4.
>
> So if you use the right API (the current one), it will not compile :(
>
> Dmitry, would it be possible to update the branch to at least v3.8 to
> get the new input-mt API? (if the Synaptics guys are ok).
Hi Benjamin,
Yes, v3.8 migration is OK with us. Touchpad related development is
already there, and we're in the process of migrating to v3.8 for the
touchscreen development work.
>
>> Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
>> Acked-by: Christopher Heiny <cheiny@synaptics.com>
>> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>> Cc: Linux Walleij <linus.walleij@linaro.org>
>> Cc: David Herrmann <dh.herrmann@gmail.com>
>> Cc: Jiri Kosina <jkosina@suse.cz>
[snip]
Since Andrew is the most familiar with the code in this patchset, I've
asked him to reply to most of the direct code-related comments.
Cheers,
Chris
^ permalink raw reply
* Re: [PATCH 2/3] Input: synaptics-rmi4 - ability disable abs or rel reporting
From: Christopher Heiny @ 2014-03-21 22:32 UTC (permalink / raw)
To: Benjamin Tissoires, Dmitry Torokhov
Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
Linus Walleij, David Herrmann, Jiri Kosina
In-Reply-To: <5329B17C.8080202@redhat.com>
On 03/19/2014 08:02 AM, Benjamin Tissoires wrote:
>
>
> On 03/18/2014 09:03 PM, Christopher Heiny wrote:
>> Even if the RMI4 touchscreen/touchpad provides reporting both
>> relative and absolute coordinates, reporting both to userspace
>> could be confusing. Allow the platform data to disable either
>> absolute or relative coordinates.
>
> General comments on the patch:
> Is there really a need to export the rel axis when there is already an
> abs collection?
> I mean, with the RMI4 over HID over I2C found on the XPS Haswell series,
> RMI4 will be bound automatically, and the sensor may (will) pretend that
> it can do both abs and rel. However, we are not using a platform_data
> for them (I think we should not), and we will get the two collections.
>
> I would personally be in favor of having a priority mechanism: if abs is
> here, skip rel, otherwise use rel. But I have no clue if you will ship
> devices which will require both. So you make the call.
>
Hi Benjamin,
That's a great idea. I'm a tad embarrassed we didn't think of that
approach.
We do ship products with both abs and rel enabled. We'll implement the
abs-priority approach, but also keep the platform data (or device tree,
once that's implemented) top optionally disable abs reporting, because
it's very handy to have during new system bring up and certain
prototyping use cases.
As before, I'll let Andrew handle the code specific comments.
Cheers,
Chris
[snip]
^ permalink raw reply
* Re: [PATCH] Add quirk HID_QUIRK_NO_INIT_REPORTS for RNDPLUS touchscreen
From: Benjamin Tissoires @ 2014-03-21 23:46 UTC (permalink / raw)
To: Yufeng Shen
Cc: linux-input, Jiri Kosina, linux-kernel@vger.kernel.org, linux-usb,
Andrew de los Reyes
In-Reply-To: <CAPDwgkNuiMmVXSznGAT2-cZ1rApHbSPyBewxGzLvSWt1mUdnAA@mail.gmail.com>
On Fri, Mar 21, 2014 at 6:08 PM, Yufeng Shen <miletus@google.com> wrote:
>
>
> On Fri, Mar 21, 2014 at 4:58 PM, Benjamin Tissoires
> <benjamin.tissoires@gmail.com> wrote:
>>
>> On Fri, Mar 21, 2014 at 3:39 PM, Yufeng Shen <miletus@chromium.org> wrote:
>> > There is timeout error during initialization:
>> > kernel: [ 14.167086] hid-multitouch 0003:2512:5003.0001:
>> > usb_submit_urb(ctrl) failed: -1
>> > kernel: [ 14.167135] hid-multitouch 0003:2512:5003.0001: timeout
>> > initializing reports
>> > kernel: [ 14.167407] input: RNDPLUS Co., LTD PULSEIR TSR4601 as
>> > /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.3/2-1.3:1.0/input/input14
>> > kernel: [ 14.167975] cpufreq_interactive: monitoring input on RNDPLUS
>> > Co., LTD PULSEIR TSR4601
>> > kernel: [ 14.168266] hid-multitouch 0003:2512:5003.0001:
>> > input,hiddev0,hidraw1: USB HID v1.10 Mouse [RNDPLUS Co., LTD PULSEIR
>> > TSR4601] on usb-0000:00:1d.0-1.3/input0
>> >
>> > Adding quirk HID_QUIRK_NO_INIT_REPORTS can solve the problem.
>>
>> I already asked you to test if HID_QUIRK_NO_INIT_INPUT_REPORTS was
>> working for the same same kind of timeout
>> (https://patchwork.kernel.org/patch/3544281/).
>
>
> The reason I didn't test HID_QUIRK_NO_INIT_INPUT_REPORTS was that I am
> mainly
> working with kernel 3.8/3.10. And HID_QUIRK_NO_INIT_INPUT_REPORTS was not
> present
You should consider working with an upstream kernel when sending patches.
> in the kernel source. Do you have the patch set that I can port to 3.8/3.10
> kernel for testing ?
git blames gives 595e9276ce68791317484ec7f0f9f2e0457c3b34, which is
Linus' tree since 3.12.
Cheers,
Benjamin
>
>
>>
>>
>> So I am asking again: please test HID_QUIRK_NO_INIT_INPUT_REPORTS and
>> if it works, use this quirk instead the one in this patch.
>> This *really* matters because the quirk HID_QUIRK_NO_INIT_REPORTS does
>> not initialize feature reports which contains important information
>> regarding hid-multitouch.
>>
>> Cheers,
>> Benjamin
>>
>> >
^ permalink raw reply
* Elan touchscreen report (HP Pavilion 10-e010nr)
From: Robert Kraus @ 2014-03-22 3:53 UTC (permalink / raw)
To: linux-input
Hello,
I'm sending this to you as per the directions at
https://wiki.archlinux.org/index.php/Multitouch_Displays
Hardware: HP Pavilion 10-e010nr touchscreen laptop
OS: xubuntu 12.04 LTS, kernel 3.2.0-60-generic
Module: hid-multitouch, vendor class
MT_CLS_GENERALTOUCH_PWT_TENFINGERS 0x0109 (decimal 265)
lsusb output: (note, the device # kept incrementing due to mtp-probe
thinking it's an MTP device)
Bus 003 Device 119: ID 04f3:0268 Elan Microelectronics Corp.
It may already be known to you guys as the touchscreen worked without
configuration in Steam OS, in which case feel free to trash this
message.
Regards,
-Bob
^ permalink raw reply
* [PATCH] input: xpad - add queue for interrupt out transfers
From: Felix Rueegg @ 2014-03-22 9:15 UTC (permalink / raw)
To: dmitry.torokhov
Cc: linux-kernel, linux-input, magissia, jms.576, paul.gortmaker,
petr, thor27, Felix Rueegg
xpad_play_effect() directly calls usb_submit_urb() to send a force
feedback command to the device. This causes the urb to fail with a
warning (URB submitted while active) when xpad_play_effect() is called
mutliple times in close succession. The issue also happens when a led
command is sent at the same time as a force feedback command, since they
use the same urb.
This patch fixes the issue by adding a buffer to queue all interrupt out
transfers (similar to the implementation in the iforce driver).
Signed-off-by: Felix Rueegg <felix.rueegg@gmail.com>
---
drivers/input/joystick/xpad.c | 212 +++++++++++++++++++++++++++++++-----------
1 file changed, 158 insertions(+), 54 deletions(-)
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 603fe0d..6147ad7 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -77,6 +77,7 @@
#include <linux/slab.h>
#include <linux/stat.h>
#include <linux/module.h>
+#include <linux/circ_buf.h>
#include <linux/usb/input.h>
#define DRIVER_AUTHOR "Marko Friedemann <mfr@bmx-chemnitz.de>"
@@ -97,6 +98,15 @@
#define XTYPE_XBOX360W 2
#define XTYPE_UNKNOWN 3
+/* queue for interrupt out transfers */
+#define XMIT_SIZE 256
+#define XMIT_INC(var, n) \
+ do { \
+ var += n; \
+ var &= XMIT_SIZE - 1; \
+ } while (0)
+#define XPAD_XMIT_RUNNING 0
+
static bool dpad_to_buttons;
module_param(dpad_to_buttons, bool, S_IRUGO);
MODULE_PARM_DESC(dpad_to_buttons, "Map D-PAD to buttons rather than axes for unknown pads");
@@ -282,7 +292,11 @@ struct usb_xpad {
struct urb *irq_out; /* urb for interrupt out report */
unsigned char *odata; /* output data */
dma_addr_t odata_dma;
- struct mutex odata_mutex;
+
+ struct circ_buf xmit; /* queue for interrupt out transfers */
+ unsigned char xmit_data[XMIT_SIZE];
+ unsigned long xmit_flags[1];
+ spinlock_t xmit_lock;
#endif
#if defined(CONFIG_JOYSTICK_XPAD_LEDS)
@@ -536,38 +550,133 @@ static void xpad_bulk_out(struct urb *urb)
}
#if defined(CONFIG_JOYSTICK_XPAD_FF) || defined(CONFIG_JOYSTICK_XPAD_LEDS)
+void xpad_irq_xmit(struct usb_xpad *xpad)
+{
+ int length, c, err;
+ unsigned long flags;
+
+ spin_lock_irqsave(&xpad->xmit_lock, flags);
+
+ if (xpad->xmit.head == xpad->xmit.tail) {
+ clear_bit(XPAD_XMIT_RUNNING, xpad->xmit_flags);
+ goto out_unlock;
+ }
+
+ /* copy packet length */
+ length = xpad->xmit.buf[xpad->xmit.tail];
+ XMIT_INC(xpad->xmit.tail, 1);
+
+ xpad->irq_out->transfer_buffer_length = length;
+
+ /* copy packet data */
+ c = CIRC_CNT_TO_END(xpad->xmit.head, xpad->xmit.tail, XMIT_SIZE);
+ if (length < c)
+ c = length;
+
+ memcpy(xpad->odata,
+ &xpad->xmit.buf[xpad->xmit.tail],
+ c);
+ if (length != c) {
+ memcpy(xpad->odata + c,
+ &xpad->xmit.buf[0],
+ length - c);
+ }
+ XMIT_INC(xpad->xmit.tail, length);
+
+ err = usb_submit_urb(xpad->irq_out, GFP_ATOMIC);
+ if (err < 0) {
+ clear_bit(XPAD_XMIT_RUNNING, xpad->xmit_flags);
+
+ /* -ENODEV: device disconnected */
+ if (err == -ENODEV)
+ goto out_unlock;
+
+ dev_warn(&xpad->intf->dev, "usb_submit_urb failed %d\n", err);
+ }
+ /*
+ * The XPAD_XMIT_RUNNING bit is not cleared here. That's intended.
+ * As long as the urb completion handler is not called, the transmiting
+ * is considered to be running
+ */
+out_unlock:
+ spin_unlock_irqrestore(&xpad->xmit_lock, flags);
+}
+
+int xpad_send_packet(struct usb_xpad *xpad, u8 length, u8 *data)
+{
+ int head, tail, empty, c;
+ unsigned long flags;
+
+ spin_lock_irqsave(&xpad->xmit_lock, flags);
+
+ /* update head and tail of xmit buffer */
+ head = xpad->xmit.head;
+ tail = xpad->xmit.tail;
+
+ if (CIRC_SPACE(head, tail, XMIT_SIZE) < length + 1) {
+ dev_warn(&xpad->dev->dev,
+ "not enough space in xmit buffer to send new packet\n");
+ spin_unlock_irqrestore(&xpad->xmit_lock, flags);
+ return -1;
+ }
+
+ empty = head == tail;
+ XMIT_INC(xpad->xmit.head, length + 1);
+
+ /* store packet length in xmit buffer */
+ xpad->xmit.buf[head] = length;
+ XMIT_INC(head, 1);
+
+ /* store packet data in xmit buffer */
+ c = CIRC_SPACE_TO_END(head, tail, XMIT_SIZE);
+ if (length < c)
+ c = length;
+
+ memcpy(&xpad->xmit.buf[head],
+ data,
+ c);
+ if (length != c) {
+ memcpy(&xpad->xmit.buf[0],
+ data + c,
+ length - c);
+ }
+ XMIT_INC(head, length);
+
+ spin_unlock_irqrestore(&xpad->xmit_lock, flags);
+
+ /* if necessary, start the transmission */
+ if (empty && !test_and_set_bit(XPAD_XMIT_RUNNING, xpad->xmit_flags))
+ xpad_irq_xmit(xpad);
+
+ return 0;
+}
+
static void xpad_irq_out(struct urb *urb)
{
struct usb_xpad *xpad = urb->context;
struct device *dev = &xpad->intf->dev;
- int retval, status;
+ int status;
status = urb->status;
switch (status) {
case 0:
/* success */
+ xpad_irq_xmit(xpad);
return;
-
case -ECONNRESET:
case -ENOENT:
case -ESHUTDOWN:
/* this urb is terminated, clean up */
dev_dbg(dev, "%s - urb shutting down with status: %d\n",
__func__, status);
- return;
-
+ break;
default:
dev_dbg(dev, "%s - nonzero urb status received: %d\n",
__func__, status);
- goto exit;
}
-exit:
- retval = usb_submit_urb(urb, GFP_ATOMIC);
- if (retval)
- dev_err(dev, "%s - usb_submit_urb failed with result %d\n",
- __func__, retval);
+ clear_bit(XPAD_XMIT_RUNNING, xpad->xmit_flags);
}
static int xpad_init_output(struct usb_interface *intf, struct usb_xpad *xpad)
@@ -585,7 +694,8 @@ static int xpad_init_output(struct usb_interface *intf, struct usb_xpad *xpad)
goto fail1;
}
- mutex_init(&xpad->odata_mutex);
+ spin_lock_init(&xpad->xmit_lock);
+ xpad->xmit.buf = xpad->xmit_data;
xpad->irq_out = usb_alloc_urb(0, GFP_KERNEL);
if (!xpad->irq_out) {
@@ -631,6 +741,7 @@ static void xpad_stop_output(struct usb_xpad *xpad) {}
static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect *effect)
{
struct usb_xpad *xpad = input_get_drvdata(dev);
+ u8 pdata[12];
if (effect->type == FF_RUMBLE) {
__u16 strong = effect->u.rumble.strong_magnitude;
@@ -639,45 +750,39 @@ static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect
switch (xpad->xtype) {
case XTYPE_XBOX:
- xpad->odata[0] = 0x00;
- xpad->odata[1] = 0x06;
- xpad->odata[2] = 0x00;
- xpad->odata[3] = strong / 256; /* left actuator */
- xpad->odata[4] = 0x00;
- xpad->odata[5] = weak / 256; /* right actuator */
- xpad->irq_out->transfer_buffer_length = 6;
-
- return usb_submit_urb(xpad->irq_out, GFP_ATOMIC);
+ pdata[0] = 0x00;
+ pdata[1] = 0x06;
+ pdata[2] = 0x00;
+ pdata[3] = strong / 256; /* left actuator */
+ pdata[4] = 0x00;
+ pdata[5] = weak / 256; /* right actuator */
+ return xpad_send_packet(xpad, 6, pdata);
case XTYPE_XBOX360:
- xpad->odata[0] = 0x00;
- xpad->odata[1] = 0x08;
- xpad->odata[2] = 0x00;
- xpad->odata[3] = strong / 256; /* left actuator? */
- xpad->odata[4] = weak / 256; /* right actuator? */
- xpad->odata[5] = 0x00;
- xpad->odata[6] = 0x00;
- xpad->odata[7] = 0x00;
- xpad->irq_out->transfer_buffer_length = 8;
-
- return usb_submit_urb(xpad->irq_out, GFP_ATOMIC);
+ pdata[0] = 0x00;
+ pdata[1] = 0x08;
+ pdata[2] = 0x00;
+ pdata[3] = strong / 256; /* left actuator? */
+ pdata[4] = weak / 256; /* right actuator? */
+ pdata[5] = 0x00;
+ pdata[6] = 0x00;
+ pdata[7] = 0x00;
+ return xpad_send_packet(xpad, 8, pdata);
case XTYPE_XBOX360W:
- xpad->odata[0] = 0x00;
- xpad->odata[1] = 0x01;
- xpad->odata[2] = 0x0F;
- xpad->odata[3] = 0xC0;
- xpad->odata[4] = 0x00;
- xpad->odata[5] = strong / 256;
- xpad->odata[6] = weak / 256;
- xpad->odata[7] = 0x00;
- xpad->odata[8] = 0x00;
- xpad->odata[9] = 0x00;
- xpad->odata[10] = 0x00;
- xpad->odata[11] = 0x00;
- xpad->irq_out->transfer_buffer_length = 12;
-
- return usb_submit_urb(xpad->irq_out, GFP_ATOMIC);
+ pdata[0] = 0x00;
+ pdata[1] = 0x01;
+ pdata[2] = 0x0F;
+ pdata[3] = 0xC0;
+ pdata[4] = 0x00;
+ pdata[5] = strong / 256;
+ pdata[6] = weak / 256;
+ pdata[7] = 0x00;
+ pdata[8] = 0x00;
+ pdata[9] = 0x00;
+ pdata[10] = 0x00;
+ pdata[11] = 0x00;
+ return xpad_send_packet(xpad, 12, pdata);
default:
dev_dbg(&xpad->dev->dev,
@@ -715,14 +820,13 @@ struct xpad_led {
static void xpad_send_led_command(struct usb_xpad *xpad, int command)
{
+ u8 pdata[3];
+
if (command >= 0 && command < 14) {
- mutex_lock(&xpad->odata_mutex);
- xpad->odata[0] = 0x01;
- xpad->odata[1] = 0x03;
- xpad->odata[2] = command;
- xpad->irq_out->transfer_buffer_length = 3;
- usb_submit_urb(xpad->irq_out, GFP_KERNEL);
- mutex_unlock(&xpad->odata_mutex);
+ pdata[0] = 0x01;
+ pdata[1] = 0x03;
+ pdata[2] = command;
+ xpad_send_packet(xpad, 3, pdata);
}
}
--
1.9.1
^ permalink raw reply related
* Re: [PATCH 00/15] iio: adc: at91 cleanups and atmel_tsadcc removal
From: Jonathan Cameron @ 2014-03-22 12:31 UTC (permalink / raw)
To: Nicolas Ferre, Alexandre Belloni, Dmitry Torokhov,
ARM Maintainers
Cc: linux-iio, linux-kernel, linux-input, Gregory Clement,
Maxime Ripard, Jean-Christophe Plagniol-Villard, linux-arm-kernel
In-Reply-To: <53296F2B.3010406@atmel.com>
On 19/03/14 10:19, Nicolas Ferre wrote:
> On 16/03/2014 19:30, Jonathan Cameron :
>> On 05/03/14 17:35, Alexandre Belloni wrote:
>>> This patch set is a followup of my previous series: iio: adc: at91 fixes
>>>
>>> I'm sorry it is so long, I made sure this is bissectable.
>> It's a nice series, don't apologise for doing things right!
>>
>> Clearly there are a few outstanding snippets from the reviews so far, but we
>> are getting to the point where we need to agree on a path for this. I'm
>> happy to take the whole series through the IIO tree if Dmitry and the at91
>> maintainers are happy with that. If it is considered two invasive form the
>> atmel side then I'm perfectly happy with someone to take it through the relevant
>> platform tree instead.
>
> Jonathan, Dmitry,
>
> Considering the amount of AT91 files touched by this series, I would
> like to take everything through Atmel's git tree and later on arm-soc.
Sure. Just as a warning, there are one or two patches queued up already that this
sits on top of. Give a spot of bad timing these are too late to go in as fixes
before the release. Might be worth waiting on those hitting Linus' tree sometime
after rc1 before taking this series.
>
>
>> I'm obviously not going to pick it up at all until it has a complete set
>> of at91 maintainer acks though!
>>
>> Realistically it's missed the coming merge window anyway, so we have a fair
>> bit of time.
>
> Yes, I will review Alexandre's v2 series.
>
> Thanks a lot for your review and for having thought about the path that
> these patches would take.
>
> Bye,
>
>
>> Jonathan
>>>
>>> 1-3) The first 3 patches are cleaning up the patform_data used for at91_adc.
>>>
>>> 4-5) Then touchscreen support for older ADCs is added, this allows to use that
>>> for the sam9m10g45ek.
>>>
>>> 6) Following those modifications, the mach/at91_adc.h is not used anywhere but
>>> in the at91_adc driver so it is remove and its content (register definitions)
>>> placed directly in the driver.
>>>
>>> 7-9) at91sam9rl support is added to at91_adc and is used for the at91sam9rl
>>> based boards.
>>>
>>> 10-11) Prepare the atmel_tsadcc removal by switching sam9rl and sam9g45 to use
>>> only at91_adc instead of atmel_tsadcc.
>>>
>>> 12-15) atmel_tsadcc removal
>>>
>>>
>>> Alexandre Belloni (15):
>>> ARM: at91: sam9g45: remove unused platform_data
>>> ARM: at91: sam9260: remove unused platform_data
>>> iio: adc: at91: cleanup platform_data
>>> iio: adc: at91_adc: Add support for touchscreens without TSMR
>>> ARM: at91: sam9m10g45ek: Add touchscreen support through at91_adc
>>> iio: adc: at91: remove unused include from include/mach
>>> iio: adc: at91: add sam9rl support
>>> ARM: at91: sam9rl: add at91_adc to support adc and touchscreen
>>> ARM: at91: sam9rlek add touchscreen support through at91_adc
>>> ARM: at91: sam9g45: switch from atmel_tsadcc to at91_adc
>>> ARM: at91: sam9rl: switch from atmel_tsadcc to at91_adc
>>> ARM: at91: remove atmel_tsadcc platform_data
>>> ARM: at91: remove atmel_tsadcc from sama5_defconfig
>>> Input: atmel_tsadcc: remove driver
>>> ARM: at91/dt: at91-cosino_mega2560 remove useless tsadcc node
>>>
>>> MAINTAINERS | 6 -
>>> arch/arm/boot/dts/at91-cosino_mega2560.dts | 5 -
>>> arch/arm/configs/at91sam9g45_defconfig | 3 +-
>>> arch/arm/configs/at91sam9rl_defconfig | 3 +-
>>> arch/arm/configs/sama5_defconfig | 1 -
>>> arch/arm/mach-at91/at91sam9260_devices.c | 10 -
>>> arch/arm/mach-at91/at91sam9g45.c | 2 +-
>>> arch/arm/mach-at91/at91sam9g45_devices.c | 63 +----
>>> arch/arm/mach-at91/at91sam9rl.c | 7 +
>>> arch/arm/mach-at91/at91sam9rl_devices.c | 83 +++++--
>>> arch/arm/mach-at91/board-sam9m10g45ek.c | 16 +-
>>> arch/arm/mach-at91/board-sam9rlek.c | 16 +-
>>> arch/arm/mach-at91/board.h | 3 -
>>> arch/arm/mach-at91/include/mach/at91_adc.h | 107 ---------
>>> drivers/iio/adc/at91_adc.c | 339 +++++++++++++++++++++++----
>>> drivers/input/touchscreen/Kconfig | 12 -
>>> drivers/input/touchscreen/Makefile | 1 -
>>> drivers/input/touchscreen/atmel_tsadcc.c | 358 -----------------------------
>>> include/linux/platform_data/at91_adc.h | 27 +--
>>> include/linux/platform_data/atmel.h | 7 -
>>> 20 files changed, 380 insertions(+), 689 deletions(-)
>>> delete mode 100644 arch/arm/mach-at91/include/mach/at91_adc.h
>>> delete mode 100644 drivers/input/touchscreen/atmel_tsadcc.c
>>>
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>
>
^ permalink raw reply
* Re: [PATCH v2 04/15] iio: adc: at91_adc: Add support for touchscreens without TSMR
From: Jonathan Cameron @ 2014-03-22 12:33 UTC (permalink / raw)
To: Alexandre Belloni, Nicolas Ferre, Dmitry Torokhov
Cc: Jean-Christophe Plagniol-Villard, linux-kernel, linux-arm-kernel,
linux-iio, linux-input, Maxime Ripard, Gregory Clement
In-Reply-To: <1395064628-644-5-git-send-email-alexandre.belloni@free-electrons.com>
On 17/03/14 13:56, Alexandre Belloni wrote:
> Old ADCs, as present on the sam9rl and the sam9g45 don't have a TSMR register
> and the touchscreen support should be handled differently.
>
You cleared up my one query on this in response to the previous version.
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
> arch/arm/mach-at91/include/mach/at91_adc.h | 13 ++
> drivers/iio/adc/at91_adc.c | 200 ++++++++++++++++++++++-------
> include/linux/platform_data/at91_adc.h | 8 ++
> 3 files changed, 174 insertions(+), 47 deletions(-)
>
> diff --git a/arch/arm/mach-at91/include/mach/at91_adc.h b/arch/arm/mach-at91/include/mach/at91_adc.h
> index c287307b9a3b..7d80396346b2 100644
> --- a/arch/arm/mach-at91/include/mach/at91_adc.h
> +++ b/arch/arm/mach-at91/include/mach/at91_adc.h
> @@ -20,6 +20,9 @@
> #define AT91_ADC_START (1 << 1) /* Start Conversion */
>
> #define AT91_ADC_MR 0x04 /* Mode Register */
> +#define AT91_ADC_TSAMOD (3 << 0) /* ADC mode */
> +#define AT91_ADC_TSAMOD_ADC_ONLY_MODE (0 << 0) /* ADC Mode */
> +#define AT91_ADC_TSAMOD_TS_ONLY_MODE (1 << 0) /* Touch Screen Only Mode */
> #define AT91_ADC_TRGEN (1 << 0) /* Trigger Enable */
> #define AT91_ADC_TRGSEL (7 << 1) /* Trigger Selection */
> #define AT91_ADC_TRGSEL_TC0 (0 << 1)
> @@ -28,6 +31,7 @@
> #define AT91_ADC_TRGSEL_EXTERNAL (6 << 1)
> #define AT91_ADC_LOWRES (1 << 4) /* Low Resolution */
> #define AT91_ADC_SLEEP (1 << 5) /* Sleep Mode */
> +#define AT91_ADC_PENDET (1 << 6) /* Pen contact detection enable */
> #define AT91_ADC_PRESCAL_9260 (0x3f << 8) /* Prescalar Rate Selection */
> #define AT91_ADC_PRESCAL_9G45 (0xff << 8)
> #define AT91_ADC_PRESCAL_(x) ((x) << 8)
> @@ -37,6 +41,12 @@
> #define AT91_ADC_STARTUP_(x) ((x) << 16)
> #define AT91_ADC_SHTIM (0xf << 24) /* Sample & Hold Time */
> #define AT91_ADC_SHTIM_(x) ((x) << 24)
> +#define AT91_ADC_PENDBC (0x0f << 28) /* Pen Debounce time */
> +#define AT91_ADC_PENDBC_(x) ((x) << 28)
> +
> +#define AT91_ADC_TSR 0x0C
> +#define AT91_ADC_TSR_SHTIM (0xf << 24) /* Sample & Hold Time */
> +#define AT91_ADC_TSR_SHTIM_(x) ((x) << 24)
>
> #define AT91_ADC_CHER 0x10 /* Channel Enable Register */
> #define AT91_ADC_CHDR 0x14 /* Channel Disable Register */
> @@ -60,6 +70,8 @@
> #define AT91_ADC_IER 0x24 /* Interrupt Enable Register */
> #define AT91_ADC_IDR 0x28 /* Interrupt Disable Register */
> #define AT91_ADC_IMR 0x2C /* Interrupt Mask Register */
> +#define AT91RL_ADC_IER_PEN (1 << 20)
> +#define AT91RL_ADC_IER_NOPEN (1 << 21)
> #define AT91_ADC_IER_PEN (1 << 29)
> #define AT91_ADC_IER_NOPEN (1 << 30)
> #define AT91_ADC_IER_XRDY (1 << 20)
> @@ -102,6 +114,7 @@
> #define AT91_ADC_TRGR_TRGPER (0xffff << 16)
> #define AT91_ADC_TRGR_TRGPER_(x) ((x) << 16)
> #define AT91_ADC_TRGR_TRGMOD (0x7 << 0)
> +#define AT91_ADC_TRGR_NONE (0 << 0)
> #define AT91_ADC_TRGR_MOD_PERIOD_TRIG (5 << 0)
>
> #endif
> diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> index 1beae65aef2c..c0e4206e34e5 100644
> --- a/drivers/iio/adc/at91_adc.c
> +++ b/drivers/iio/adc/at91_adc.c
> @@ -46,6 +46,10 @@
> #define TOUCH_SAMPLE_PERIOD_US 2000 /* 2ms */
> #define TOUCH_PEN_DETECT_DEBOUNCE_US 200
>
> +#define MAX_RLPOS_BITS 10
> +#define TOUCH_SAMPLE_PERIOD_US_RL 10000 /* 10ms, the SoC can't keep up with 2ms */
> +#define TOUCH_SHTIM 0xa
> +
> /**
> * struct at91_adc_reg_desc - Various informations relative to registers
> * @channel_base: Base offset for the channel data registers
> @@ -83,12 +87,6 @@ struct at91_adc_caps {
> struct at91_adc_reg_desc registers;
> };
>
> -enum atmel_adc_ts_type {
> - ATMEL_ADC_TOUCHSCREEN_NONE = 0,
> - ATMEL_ADC_TOUCHSCREEN_4WIRE = 4,
> - ATMEL_ADC_TOUCHSCREEN_5WIRE = 5,
> -};
> -
> struct at91_adc_state {
> struct clk *adc_clk;
> u16 *buffer;
> @@ -133,6 +131,11 @@ struct at91_adc_state {
>
> u16 ts_sample_period_val;
> u32 ts_pressure_threshold;
> + u16 ts_pendbc;
> +
> + bool ts_bufferedmeasure;
> + u32 ts_prev_absx;
> + u32 ts_prev_absy;
> };
>
> static irqreturn_t at91_adc_trigger_handler(int irq, void *p)
> @@ -239,7 +242,72 @@ static int at91_ts_sample(struct at91_adc_state *st)
> return 0;
> }
>
> -static irqreturn_t at91_adc_interrupt(int irq, void *private)
> +static irqreturn_t at91_adc_rl_interrupt(int irq, void *private)
> +{
> + struct iio_dev *idev = private;
> + struct at91_adc_state *st = iio_priv(idev);
> + u32 status = at91_adc_readl(st, st->registers->status_register);
> + unsigned int reg;
> +
> + status &= at91_adc_readl(st, AT91_ADC_IMR);
> + if (status & st->registers->drdy_mask)
> + handle_adc_eoc_trigger(irq, idev);
> +
> + if (status & AT91RL_ADC_IER_PEN) {
> + /* Disabling pen debounce is required to get a NOPEN irq */
> + reg = at91_adc_readl(st, AT91_ADC_MR);
> + reg &= ~AT91_ADC_PENDBC;
> + at91_adc_writel(st, AT91_ADC_MR, reg);
> +
> + at91_adc_writel(st, AT91_ADC_IDR, AT91RL_ADC_IER_PEN);
> + at91_adc_writel(st, AT91_ADC_IER, AT91RL_ADC_IER_NOPEN
> + | AT91_ADC_EOC(3));
> + /* Set up period trigger for sampling */
> + at91_adc_writel(st, st->registers->trigger_register,
> + AT91_ADC_TRGR_MOD_PERIOD_TRIG |
> + AT91_ADC_TRGR_TRGPER_(st->ts_sample_period_val));
> + } else if (status & AT91RL_ADC_IER_NOPEN) {
> + reg = at91_adc_readl(st, AT91_ADC_MR);
> + reg |= AT91_ADC_PENDBC_(st->ts_pendbc) & AT91_ADC_PENDBC;
> + at91_adc_writel(st, AT91_ADC_MR, reg);
> + at91_adc_writel(st, st->registers->trigger_register,
> + AT91_ADC_TRGR_NONE);
> +
> + at91_adc_writel(st, AT91_ADC_IDR, AT91RL_ADC_IER_NOPEN
> + | AT91_ADC_EOC(3));
> + at91_adc_writel(st, AT91_ADC_IER, AT91RL_ADC_IER_PEN);
> + st->ts_bufferedmeasure = false;
> + input_report_key(st->ts_input, BTN_TOUCH, 0);
> + input_sync(st->ts_input);
> + } else if (status & AT91_ADC_EOC(3)) {
> + /* Conversion finished */
> + if (st->ts_bufferedmeasure) {
> + /*
> + * Last measurement is always discarded, since it can
> + * be erroneous.
> + * Always report previous measurement
> + */
> + input_report_abs(st->ts_input, ABS_X, st->ts_prev_absx);
> + input_report_abs(st->ts_input, ABS_Y, st->ts_prev_absy);
> + input_report_key(st->ts_input, BTN_TOUCH, 1);
> + input_sync(st->ts_input);
> + } else
> + st->ts_bufferedmeasure = true;
> +
> + /* Now make new measurement */
> + st->ts_prev_absx = at91_adc_readl(st, AT91_ADC_CHAN(st, 3))
> + << MAX_RLPOS_BITS;
> + st->ts_prev_absx /= at91_adc_readl(st, AT91_ADC_CHAN(st, 2));
> +
> + st->ts_prev_absy = at91_adc_readl(st, AT91_ADC_CHAN(st, 1))
> + << MAX_RLPOS_BITS;
> + st->ts_prev_absy /= at91_adc_readl(st, AT91_ADC_CHAN(st, 0));
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t at91_adc_9x5_interrupt(int irq, void *private)
> {
> struct iio_dev *idev = private;
> struct at91_adc_state *st = iio_priv(idev);
> @@ -672,6 +740,8 @@ static int at91_adc_probe_dt_ts(struct device_node *node,
> return -EINVAL;
> }
>
> + if (!st->caps->has_tsmr)
> + return 0;
> prop = 0;
> of_property_read_u32(node, "atmel,adc-ts-pressure-threshold", &prop);
> st->ts_pressure_threshold = prop;
> @@ -795,6 +865,7 @@ static int at91_adc_probe_pdata(struct at91_adc_state *st,
> st->trigger_number = pdata->trigger_number;
> st->trigger_list = pdata->trigger_list;
> st->registers = &st->caps->registers;
> + st->touchscreen_type = pdata->touchscreen_type;
>
> return 0;
> }
> @@ -809,7 +880,10 @@ static int atmel_ts_open(struct input_dev *dev)
> {
> struct at91_adc_state *st = input_get_drvdata(dev);
>
> - at91_adc_writel(st, AT91_ADC_IER, AT91_ADC_IER_PEN);
> + if (st->caps->has_tsmr)
> + at91_adc_writel(st, AT91_ADC_IER, AT91_ADC_IER_PEN);
> + else
> + at91_adc_writel(st, AT91_ADC_IER, AT91RL_ADC_IER_PEN);
> return 0;
> }
>
> @@ -817,45 +891,61 @@ static void atmel_ts_close(struct input_dev *dev)
> {
> struct at91_adc_state *st = input_get_drvdata(dev);
>
> - at91_adc_writel(st, AT91_ADC_IDR, AT91_ADC_IER_PEN);
> + if (st->caps->has_tsmr)
> + at91_adc_writel(st, AT91_ADC_IDR, AT91_ADC_IER_PEN);
> + else
> + at91_adc_writel(st, AT91_ADC_IDR, AT91RL_ADC_IER_PEN);
> }
>
> static int at91_ts_hw_init(struct at91_adc_state *st, u32 adc_clk_khz)
> {
> - u32 reg = 0, pendbc;
> + u32 reg = 0;
> int i = 0;
>
> - if (st->touchscreen_type == ATMEL_ADC_TOUCHSCREEN_4WIRE)
> - reg = AT91_ADC_TSMR_TSMODE_4WIRE_PRESS;
> - else
> - reg = AT91_ADC_TSMR_TSMODE_5WIRE;
> -
> /* a Pen Detect Debounce Time is necessary for the ADC Touch to avoid
> * pen detect noise.
> * The formula is : Pen Detect Debounce Time = (2 ^ pendbc) / ADCClock
> */
> - pendbc = round_up(TOUCH_PEN_DETECT_DEBOUNCE_US * adc_clk_khz / 1000, 1);
> + st->ts_pendbc = round_up(TOUCH_PEN_DETECT_DEBOUNCE_US * adc_clk_khz /
> + 1000, 1);
>
> - while (pendbc >> ++i)
> + while (st->ts_pendbc >> ++i)
> ; /* Empty! Find the shift offset */
> - if (abs(pendbc - (1 << i)) < abs(pendbc - (1 << (i - 1))))
> - pendbc = i;
> + if (abs(st->ts_pendbc - (1 << i)) < abs(st->ts_pendbc - (1 << (i - 1))))
> + st->ts_pendbc = i;
> else
> - pendbc = i - 1;
> + st->ts_pendbc = i - 1;
>
> - if (st->caps->has_tsmr) {
> - reg |= AT91_ADC_TSMR_TSAV_(st->caps->ts_filter_average)
> - & AT91_ADC_TSMR_TSAV;
> - reg |= AT91_ADC_TSMR_PENDBC_(pendbc) & AT91_ADC_TSMR_PENDBC;
> - reg |= AT91_ADC_TSMR_NOTSDMA;
> - reg |= AT91_ADC_TSMR_PENDET_ENA;
> - reg |= 0x03 << 8; /* TSFREQ, need bigger than TSAV */
> -
> - at91_adc_writel(st, AT91_ADC_TSMR, reg);
> - } else {
> - /* TODO: for 9g45 which has no TSMR */
> + if (!st->caps->has_tsmr) {
> + reg = at91_adc_readl(st, AT91_ADC_MR);
> + reg |= AT91_ADC_TSAMOD_TS_ONLY_MODE | AT91_ADC_PENDET;
> +
> + reg |= AT91_ADC_PENDBC_(st->ts_pendbc) & AT91_ADC_PENDBC;
> + at91_adc_writel(st, AT91_ADC_MR, reg);
> +
> + reg = AT91_ADC_TSR_SHTIM_(TOUCH_SHTIM) & AT91_ADC_TSR_SHTIM;
> + at91_adc_writel(st, AT91_ADC_TSR, reg);
> +
> + st->ts_sample_period_val = round_up((TOUCH_SAMPLE_PERIOD_US_RL *
> + adc_clk_khz / 1000) - 1, 1);
> +
> + return 0;
> }
>
> + if (st->touchscreen_type == ATMEL_ADC_TOUCHSCREEN_4WIRE)
> + reg = AT91_ADC_TSMR_TSMODE_4WIRE_PRESS;
> + else
> + reg = AT91_ADC_TSMR_TSMODE_5WIRE;
> +
> + reg |= AT91_ADC_TSMR_TSAV_(st->caps->ts_filter_average)
> + & AT91_ADC_TSMR_TSAV;
> + reg |= AT91_ADC_TSMR_PENDBC_(st->ts_pendbc) & AT91_ADC_TSMR_PENDBC;
> + reg |= AT91_ADC_TSMR_NOTSDMA;
> + reg |= AT91_ADC_TSMR_PENDET_ENA;
> + reg |= 0x03 << 8; /* TSFREQ, needs to be bigger than TSAV */
> +
> + at91_adc_writel(st, AT91_ADC_TSMR, reg);
> +
> /* Change adc internal resistor value for better pen detection,
> * default value is 100 kOhm.
> * 0 = 200 kOhm, 1 = 150 kOhm, 2 = 100 kOhm, 3 = 50 kOhm
> @@ -864,7 +954,7 @@ static int at91_ts_hw_init(struct at91_adc_state *st, u32 adc_clk_khz)
> at91_adc_writel(st, AT91_ADC_ACR, st->caps->ts_pen_detect_sensitivity
> & AT91_ADC_ACR_PENDETSENS);
>
> - /* Sample Peroid Time = (TRGPER + 1) / ADCClock */
> + /* Sample Period Time = (TRGPER + 1) / ADCClock */
> st->ts_sample_period_val = round_up((TOUCH_SAMPLE_PERIOD_US *
> adc_clk_khz / 1000) - 1, 1);
>
> @@ -893,17 +983,37 @@ static int at91_ts_register(struct at91_adc_state *st,
> __set_bit(EV_ABS, input->evbit);
> __set_bit(EV_KEY, input->evbit);
> __set_bit(BTN_TOUCH, input->keybit);
> - input_set_abs_params(input, ABS_X, 0, (1 << MAX_POS_BITS) - 1, 0, 0);
> - input_set_abs_params(input, ABS_Y, 0, (1 << MAX_POS_BITS) - 1, 0, 0);
> - input_set_abs_params(input, ABS_PRESSURE, 0, 0xffffff, 0, 0);
> + if (st->caps->has_tsmr) {
> + input_set_abs_params(input, ABS_X, 0, (1 << MAX_POS_BITS) - 1,
> + 0, 0);
> + input_set_abs_params(input, ABS_Y, 0, (1 << MAX_POS_BITS) - 1,
> + 0, 0);
> + input_set_abs_params(input, ABS_PRESSURE, 0, 0xffffff, 0, 0);
> + } else {
> + if (st->touchscreen_type != ATMEL_ADC_TOUCHSCREEN_4WIRE) {
> + dev_err(&pdev->dev,
> + "This touchscreen controller only support 4 wires\n");
> + ret = -EINVAL;
> + goto err;
> + }
> +
> + input_set_abs_params(input, ABS_X, 0, (1 << MAX_RLPOS_BITS) - 1,
> + 0, 0);
> + input_set_abs_params(input, ABS_Y, 0, (1 << MAX_RLPOS_BITS) - 1,
> + 0, 0);
> + }
>
> st->ts_input = input;
> input_set_drvdata(input, st);
>
> ret = input_register_device(input);
> if (ret)
> - input_free_device(st->ts_input);
> + goto err;
> +
> + return ret;
>
> +err:
> + input_free_device(st->ts_input);
> return ret;
> }
>
> @@ -962,11 +1072,13 @@ static int at91_adc_probe(struct platform_device *pdev)
> */
> at91_adc_writel(st, AT91_ADC_CR, AT91_ADC_SWRST);
> at91_adc_writel(st, AT91_ADC_IDR, 0xFFFFFFFF);
> - ret = request_irq(st->irq,
> - at91_adc_interrupt,
> - 0,
> - pdev->dev.driver->name,
> - idev);
> +
> + if (st->caps->has_tsmr)
> + ret = request_irq(st->irq, at91_adc_9x5_interrupt, 0,
> + pdev->dev.driver->name, idev);
> + else
> + ret = request_irq(st->irq, at91_adc_rl_interrupt, 0,
> + pdev->dev.driver->name, idev);
> if (ret) {
> dev_err(&pdev->dev, "Failed to allocate IRQ.\n");
> return ret;
> @@ -1070,12 +1182,6 @@ static int at91_adc_probe(struct platform_device *pdev)
> goto error_disable_adc_clk;
> }
> } else {
> - if (!st->caps->has_tsmr) {
> - dev_err(&pdev->dev, "We don't support non-TSMR adc\n");
> - ret = -ENODEV;
> - goto error_disable_adc_clk;
> - }
> -
> ret = at91_ts_register(st, pdev);
> if (ret)
> goto error_disable_adc_clk;
> diff --git a/include/linux/platform_data/at91_adc.h b/include/linux/platform_data/at91_adc.h
> index fcf73879dbfe..7819fc787731 100644
> --- a/include/linux/platform_data/at91_adc.h
> +++ b/include/linux/platform_data/at91_adc.h
> @@ -7,6 +7,12 @@
> #ifndef _AT91_ADC_H_
> #define _AT91_ADC_H_
>
> +enum atmel_adc_ts_type {
> + ATMEL_ADC_TOUCHSCREEN_NONE = 0,
> + ATMEL_ADC_TOUCHSCREEN_4WIRE = 4,
> + ATMEL_ADC_TOUCHSCREEN_5WIRE = 5,
> +};
> +
> /**
> * struct at91_adc_trigger - description of triggers
> * @name: name of the trigger advertised to the user
> @@ -28,6 +34,7 @@ struct at91_adc_trigger {
> * @trigger_number: Number of triggers available in the ADC
> * @use_external_triggers: does the board has external triggers availables
> * @vref: Reference voltage for the ADC in millivolts
> + * @touchscreen_type: If a touchscreen is connected, its type (4 or 5 wires)
> */
> struct at91_adc_data {
> unsigned long channels_used;
> @@ -36,6 +43,7 @@ struct at91_adc_data {
> u8 trigger_number;
> bool use_external_triggers;
> u16 vref;
> + enum atmel_adc_ts_type touchscreen_type;
> };
>
> extern void __init at91_add_device_adc(struct at91_adc_data *data);
>
^ permalink raw reply
* Re: [PATCH v2 05/15] ARM: at91: sam9m10g45ek: Add touchscreen support through at91_adc
From: Jonathan Cameron @ 2014-03-22 12:34 UTC (permalink / raw)
To: Alexandre Belloni, Nicolas Ferre, Dmitry Torokhov
Cc: Jean-Christophe Plagniol-Villard, linux-kernel, linux-arm-kernel,
linux-iio, linux-input, Maxime Ripard, Gregory Clement
In-Reply-To: <1395064628-644-6-git-send-email-alexandre.belloni@free-electrons.com>
On 17/03/14 13:56, Alexandre Belloni wrote:
> at91_adc now supports reading a touchscreen for ADCs without a TSMR register.
> Enable touchscreen support through at91_adc. This allows to use both a
> touchscreen and the remaining ADC channel at the same time.
>
> Also, lower the clock for the ADC as it allows to have more stable reads and
> this is the speed used by atmel_tsadcc.
> It lowers the maximum throughput rate from 440000 samples per second to 12958
> samples per second. It shouldn't be an issue as the CPU is not able to keep up
> reading samples at that frequency.
Thanks for adding this detail.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
> arch/arm/mach-at91/at91sam9g45.c | 2 +-
> arch/arm/mach-at91/board-sam9m10g45ek.c | 1 +
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-at91/at91sam9g45.c b/arch/arm/mach-at91/at91sam9g45.c
> index 2f455ce35268..3c519a7a938f 100644
> --- a/arch/arm/mach-at91/at91sam9g45.c
> +++ b/arch/arm/mach-at91/at91sam9g45.c
> @@ -181,7 +181,7 @@ static struct clk vdec_clk = {
> static struct clk adc_op_clk = {
> .name = "adc_op_clk",
> .type = CLK_TYPE_PERIPHERAL,
> - .rate_hz = 13200000,
> + .rate_hz = 300000,
> };
>
> /* AES/TDES/SHA clock - Only for sam9m11/sam9g56 */
> diff --git a/arch/arm/mach-at91/board-sam9m10g45ek.c b/arch/arm/mach-at91/board-sam9m10g45ek.c
> index ef39078c8ce2..4d775b796ce4 100644
> --- a/arch/arm/mach-at91/board-sam9m10g45ek.c
> +++ b/arch/arm/mach-at91/board-sam9m10g45ek.c
> @@ -315,6 +315,7 @@ static struct at91_adc_data ek_adc_data = {
> .channels_used = BIT(0) | BIT(1) | BIT(2) | BIT(3) | BIT(4) | BIT(5) | BIT(6) | BIT(7),
> .use_external_triggers = true,
> .vref = 3300,
> + .touchscreen_type = ATMEL_ADC_TOUCHSCREEN_4WIRE,
> };
>
> /*
>
^ permalink raw reply
* Re: [PATCH v2 08/15] ARM: at91: sam9rl: add at91_adc to support adc and touchscreen
From: Jonathan Cameron @ 2014-03-22 12:35 UTC (permalink / raw)
To: Alexandre Belloni, Nicolas Ferre, Dmitry Torokhov
Cc: Jean-Christophe Plagniol-Villard, linux-kernel, linux-arm-kernel,
linux-iio, linux-input, Maxime Ripard, Gregory Clement
In-Reply-To: <1395064628-644-9-git-send-email-alexandre.belloni@free-electrons.com>
On 17/03/14 13:57, Alexandre Belloni wrote:
> The ADC clock needs to be defined to enable the at91_adc driver. It is defined
> to the same speed that is used for atmel_tsadcc.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Looks good to me, but out of my area of expertise so I won't ack it.
> ---
> arch/arm/mach-at91/at91sam9rl.c | 7 +++
> arch/arm/mach-at91/at91sam9rl_devices.c | 85 +++++++++++++++++++++++++++++++++
> 2 files changed, 92 insertions(+)
>
> diff --git a/arch/arm/mach-at91/at91sam9rl.c b/arch/arm/mach-at91/at91sam9rl.c
> index 3651517abedf..753ba4893836 100644
> --- a/arch/arm/mach-at91/at91sam9rl.c
> +++ b/arch/arm/mach-at91/at91sam9rl.c
> @@ -151,6 +151,11 @@ static struct clk ac97_clk = {
> .pmc_mask = 1 << AT91SAM9RL_ID_AC97C,
> .type = CLK_TYPE_PERIPHERAL,
> };
> +static struct clk adc_op_clk = {
> + .name = "adc_op_clk",
> + .type = CLK_TYPE_PERIPHERAL,
> + .rate_hz = 1000000,
> +};
>
> static struct clk *periph_clocks[] __initdata = {
> &pioA_clk,
> @@ -176,6 +181,7 @@ static struct clk *periph_clocks[] __initdata = {
> &udphs_clk,
> &lcdc_clk,
> &ac97_clk,
> + &adc_op_clk,
> // irq0
> };
>
> @@ -196,6 +202,7 @@ static struct clk_lookup periph_clocks_lookups[] = {
> CLKDEV_CON_ID("pioB", &pioB_clk),
> CLKDEV_CON_ID("pioC", &pioC_clk),
> CLKDEV_CON_ID("pioD", &pioD_clk),
> + CLKDEV_CON_ID("adc_clk", &tsc_clk),
> };
>
> static struct clk_lookup usart_clocks_lookups[] = {
> diff --git a/arch/arm/mach-at91/at91sam9rl_devices.c b/arch/arm/mach-at91/at91sam9rl_devices.c
> index a698bdab2cce..b836dada942c 100644
> --- a/arch/arm/mach-at91/at91sam9rl_devices.c
> +++ b/arch/arm/mach-at91/at91sam9rl_devices.c
> @@ -22,6 +22,7 @@
> #include <mach/at91_matrix.h>
> #include <mach/at91sam9_smc.h>
> #include <linux/platform_data/dma-atmel.h>
> +#include <linux/platform_data/at91_adc.h>
>
> #include "board.h"
> #include "generic.h"
> @@ -656,6 +657,90 @@ void __init at91_add_device_tsadcc(struct at91_tsadcc_data *data)
> void __init at91_add_device_tsadcc(struct at91_tsadcc_data *data) {}
> #endif
>
> +/* --------------------------------------------------------------------
> + * ADC and Touchscreen
> + * -------------------------------------------------------------------- */
> +
> +#if IS_ENABLED(CONFIG_AT91_ADC)
> +static struct at91_adc_data adc_data;
> +
> +static struct resource adc_resources[] = {
> + [0] = {
> + .start = AT91SAM9RL_BASE_TSC,
> + .end = AT91SAM9RL_BASE_TSC + SZ_16K - 1,
> + .flags = IORESOURCE_MEM,
> + },
> + [1] = {
> + .start = NR_IRQS_LEGACY + AT91SAM9RL_ID_TSC,
> + .end = NR_IRQS_LEGACY + AT91SAM9RL_ID_TSC,
> + .flags = IORESOURCE_IRQ,
> + }
> +};
> +
> +static struct platform_device at91_adc_device = {
> + .name = "at91sam9rl-adc",
> + .id = -1,
> + .dev = {
> + .platform_data = &adc_data,
> + },
> + .resource = adc_resources,
> + .num_resources = ARRAY_SIZE(adc_resources),
> +};
> +
> +static struct at91_adc_trigger at91_adc_triggers[] = {
> + [0] = {
> + .name = "external-rising",
> + .value = 1,
> + .is_external = true,
> + },
> + [1] = {
> + .name = "external-falling",
> + .value = 2,
> + .is_external = true,
> + },
> + [2] = {
> + .name = "external-any",
> + .value = 3,
> + .is_external = true,
> + },
> + [3] = {
> + .name = "continuous",
> + .value = 6,
> + .is_external = false,
> + },
> +};
> +
> +void __init at91_add_device_adc(struct at91_adc_data *data)
> +{
> + if (!data)
> + return;
> +
> + if (test_bit(0, &data->channels_used))
> + at91_set_A_periph(AT91_PIN_PA17, 0);
> + if (test_bit(1, &data->channels_used))
> + at91_set_A_periph(AT91_PIN_PA18, 0);
> + if (test_bit(2, &data->channels_used))
> + at91_set_A_periph(AT91_PIN_PA19, 0);
> + if (test_bit(3, &data->channels_used))
> + at91_set_A_periph(AT91_PIN_PA20, 0);
> + if (test_bit(4, &data->channels_used))
> + at91_set_A_periph(AT91_PIN_PD6, 0);
> + if (test_bit(5, &data->channels_used))
> + at91_set_A_periph(AT91_PIN_PD7, 0);
> +
> + if (data->use_external_triggers)
> + at91_set_A_periph(AT91_PIN_PB15, 0);
> +
> + data->startup_time = 40;
> + data->trigger_number = 4;
> + data->trigger_list = at91_adc_triggers;
> +
> + adc_data = *data;
> + platform_device_register(&at91_adc_device);
> +}
> +#else
> +void __init at91_add_device_adc(struct at91_adc_data *data) {}
> +#endif
>
> /* --------------------------------------------------------------------
> * RTC
>
^ permalink raw reply
* Re: [PATCH v2 06/15] iio: adc: at91: remove unused include from include/mach
From: Jonathan Cameron @ 2014-03-22 12:34 UTC (permalink / raw)
To: Alexandre Belloni, Nicolas Ferre, Dmitry Torokhov
Cc: Jean-Christophe Plagniol-Villard,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard,
Gregory Clement
In-Reply-To: <1395064628-644-7-git-send-email-alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
On 17/03/14 13:56, Alexandre Belloni wrote:
> That include file is now only used by the at91_adc driver, remove it from
> include/mach for better driver separation.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Acked-by: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
> arch/arm/mach-at91/include/mach/at91_adc.h | 120 -----------------------------
> drivers/iio/adc/at91_adc.c | 103 ++++++++++++++++++++++++-
> 2 files changed, 102 insertions(+), 121 deletions(-)
> delete mode 100644 arch/arm/mach-at91/include/mach/at91_adc.h
>
> diff --git a/arch/arm/mach-at91/include/mach/at91_adc.h b/arch/arm/mach-at91/include/mach/at91_adc.h
> deleted file mode 100644
> index 7d80396346b2..000000000000
> --- a/arch/arm/mach-at91/include/mach/at91_adc.h
> +++ /dev/null
> @@ -1,120 +0,0 @@
> -/*
> - * arch/arm/mach-at91/include/mach/at91_adc.h
> - *
> - * Copyright (C) SAN People
> - *
> - * Analog-to-Digital Converter (ADC) registers.
> - * Based on AT91SAM9260 datasheet revision D.
> - *
> - * 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
> - * (at your option) any later version.
> - */
> -
> -#ifndef AT91_ADC_H
> -#define AT91_ADC_H
> -
> -#define AT91_ADC_CR 0x00 /* Control Register */
> -#define AT91_ADC_SWRST (1 << 0) /* Software Reset */
> -#define AT91_ADC_START (1 << 1) /* Start Conversion */
> -
> -#define AT91_ADC_MR 0x04 /* Mode Register */
> -#define AT91_ADC_TSAMOD (3 << 0) /* ADC mode */
> -#define AT91_ADC_TSAMOD_ADC_ONLY_MODE (0 << 0) /* ADC Mode */
> -#define AT91_ADC_TSAMOD_TS_ONLY_MODE (1 << 0) /* Touch Screen Only Mode */
> -#define AT91_ADC_TRGEN (1 << 0) /* Trigger Enable */
> -#define AT91_ADC_TRGSEL (7 << 1) /* Trigger Selection */
> -#define AT91_ADC_TRGSEL_TC0 (0 << 1)
> -#define AT91_ADC_TRGSEL_TC1 (1 << 1)
> -#define AT91_ADC_TRGSEL_TC2 (2 << 1)
> -#define AT91_ADC_TRGSEL_EXTERNAL (6 << 1)
> -#define AT91_ADC_LOWRES (1 << 4) /* Low Resolution */
> -#define AT91_ADC_SLEEP (1 << 5) /* Sleep Mode */
> -#define AT91_ADC_PENDET (1 << 6) /* Pen contact detection enable */
> -#define AT91_ADC_PRESCAL_9260 (0x3f << 8) /* Prescalar Rate Selection */
> -#define AT91_ADC_PRESCAL_9G45 (0xff << 8)
> -#define AT91_ADC_PRESCAL_(x) ((x) << 8)
> -#define AT91_ADC_STARTUP_9260 (0x1f << 16) /* Startup Up Time */
> -#define AT91_ADC_STARTUP_9G45 (0x7f << 16)
> -#define AT91_ADC_STARTUP_9X5 (0xf << 16)
> -#define AT91_ADC_STARTUP_(x) ((x) << 16)
> -#define AT91_ADC_SHTIM (0xf << 24) /* Sample & Hold Time */
> -#define AT91_ADC_SHTIM_(x) ((x) << 24)
> -#define AT91_ADC_PENDBC (0x0f << 28) /* Pen Debounce time */
> -#define AT91_ADC_PENDBC_(x) ((x) << 28)
> -
> -#define AT91_ADC_TSR 0x0C
> -#define AT91_ADC_TSR_SHTIM (0xf << 24) /* Sample & Hold Time */
> -#define AT91_ADC_TSR_SHTIM_(x) ((x) << 24)
> -
> -#define AT91_ADC_CHER 0x10 /* Channel Enable Register */
> -#define AT91_ADC_CHDR 0x14 /* Channel Disable Register */
> -#define AT91_ADC_CHSR 0x18 /* Channel Status Register */
> -#define AT91_ADC_CH(n) (1 << (n)) /* Channel Number */
> -
> -#define AT91_ADC_SR 0x1C /* Status Register */
> -#define AT91_ADC_EOC(n) (1 << (n)) /* End of Conversion on Channel N */
> -#define AT91_ADC_OVRE(n) (1 << ((n) + 8))/* Overrun Error on Channel N */
> -#define AT91_ADC_DRDY (1 << 16) /* Data Ready */
> -#define AT91_ADC_GOVRE (1 << 17) /* General Overrun Error */
> -#define AT91_ADC_ENDRX (1 << 18) /* End of RX Buffer */
> -#define AT91_ADC_RXFUFF (1 << 19) /* RX Buffer Full */
> -
> -#define AT91_ADC_SR_9X5 0x30 /* Status Register for 9x5 */
> -#define AT91_ADC_SR_DRDY_9X5 (1 << 24) /* Data Ready */
> -
> -#define AT91_ADC_LCDR 0x20 /* Last Converted Data Register */
> -#define AT91_ADC_LDATA (0x3ff)
> -
> -#define AT91_ADC_IER 0x24 /* Interrupt Enable Register */
> -#define AT91_ADC_IDR 0x28 /* Interrupt Disable Register */
> -#define AT91_ADC_IMR 0x2C /* Interrupt Mask Register */
> -#define AT91RL_ADC_IER_PEN (1 << 20)
> -#define AT91RL_ADC_IER_NOPEN (1 << 21)
> -#define AT91_ADC_IER_PEN (1 << 29)
> -#define AT91_ADC_IER_NOPEN (1 << 30)
> -#define AT91_ADC_IER_XRDY (1 << 20)
> -#define AT91_ADC_IER_YRDY (1 << 21)
> -#define AT91_ADC_IER_PRDY (1 << 22)
> -#define AT91_ADC_ISR_PENS (1 << 31)
> -
> -#define AT91_ADC_CHR(n) (0x30 + ((n) * 4)) /* Channel Data Register N */
> -#define AT91_ADC_DATA (0x3ff)
> -
> -#define AT91_ADC_CDR0_9X5 (0x50) /* Channel Data Register 0 for 9X5 */
> -
> -#define AT91_ADC_ACR 0x94 /* Analog Control Register */
> -#define AT91_ADC_ACR_PENDETSENS (0x3 << 0) /* pull-up resistor */
> -
> -#define AT91_ADC_TSMR 0xB0
> -#define AT91_ADC_TSMR_TSMODE (3 << 0) /* Touch Screen Mode */
> -#define AT91_ADC_TSMR_TSMODE_NONE (0 << 0)
> -#define AT91_ADC_TSMR_TSMODE_4WIRE_NO_PRESS (1 << 0)
> -#define AT91_ADC_TSMR_TSMODE_4WIRE_PRESS (2 << 0)
> -#define AT91_ADC_TSMR_TSMODE_5WIRE (3 << 0)
> -#define AT91_ADC_TSMR_TSAV (3 << 4) /* Averages samples */
> -#define AT91_ADC_TSMR_TSAV_(x) ((x) << 4)
> -#define AT91_ADC_TSMR_SCTIM (0x0f << 16) /* Switch closure time */
> -#define AT91_ADC_TSMR_PENDBC (0x0f << 28) /* Pen Debounce time */
> -#define AT91_ADC_TSMR_PENDBC_(x) ((x) << 28)
> -#define AT91_ADC_TSMR_NOTSDMA (1 << 22) /* No Touchscreen DMA */
> -#define AT91_ADC_TSMR_PENDET_DIS (0 << 24) /* Pen contact detection disable */
> -#define AT91_ADC_TSMR_PENDET_ENA (1 << 24) /* Pen contact detection enable */
> -
> -#define AT91_ADC_TSXPOSR 0xB4
> -#define AT91_ADC_TSYPOSR 0xB8
> -#define AT91_ADC_TSPRESSR 0xBC
> -
> -#define AT91_ADC_TRGR_9260 AT91_ADC_MR
> -#define AT91_ADC_TRGR_9G45 0x08
> -#define AT91_ADC_TRGR_9X5 0xC0
> -
> -/* Trigger Register bit field */
> -#define AT91_ADC_TRGR_TRGPER (0xffff << 16)
> -#define AT91_ADC_TRGR_TRGPER_(x) ((x) << 16)
> -#define AT91_ADC_TRGR_TRGMOD (0x7 << 0)
> -#define AT91_ADC_TRGR_NONE (0 << 0)
> -#define AT91_ADC_TRGR_MOD_PERIOD_TRIG (5 << 0)
> -
> -#endif
> diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> index c0e4206e34e5..a51b8badbf8e 100644
> --- a/drivers/iio/adc/at91_adc.c
> +++ b/drivers/iio/adc/at91_adc.c
> @@ -31,7 +31,108 @@
> #include <linux/iio/trigger_consumer.h>
> #include <linux/iio/triggered_buffer.h>
>
> -#include <mach/at91_adc.h>
> +/* Registers */
> +#define AT91_ADC_CR 0x00 /* Control Register */
> +#define AT91_ADC_SWRST (1 << 0) /* Software Reset */
> +#define AT91_ADC_START (1 << 1) /* Start Conversion */
> +
> +#define AT91_ADC_MR 0x04 /* Mode Register */
> +#define AT91_ADC_TSAMOD (3 << 0) /* ADC mode */
> +#define AT91_ADC_TSAMOD_ADC_ONLY_MODE (0 << 0) /* ADC Mode */
> +#define AT91_ADC_TSAMOD_TS_ONLY_MODE (1 << 0) /* Touch Screen Only Mode */
> +#define AT91_ADC_TRGEN (1 << 0) /* Trigger Enable */
> +#define AT91_ADC_TRGSEL (7 << 1) /* Trigger Selection */
> +#define AT91_ADC_TRGSEL_TC0 (0 << 1)
> +#define AT91_ADC_TRGSEL_TC1 (1 << 1)
> +#define AT91_ADC_TRGSEL_TC2 (2 << 1)
> +#define AT91_ADC_TRGSEL_EXTERNAL (6 << 1)
> +#define AT91_ADC_LOWRES (1 << 4) /* Low Resolution */
> +#define AT91_ADC_SLEEP (1 << 5) /* Sleep Mode */
> +#define AT91_ADC_PENDET (1 << 6) /* Pen contact detection enable */
> +#define AT91_ADC_PRESCAL_9260 (0x3f << 8) /* Prescalar Rate Selection */
> +#define AT91_ADC_PRESCAL_9G45 (0xff << 8)
> +#define AT91_ADC_PRESCAL_(x) ((x) << 8)
> +#define AT91_ADC_STARTUP_9260 (0x1f << 16) /* Startup Up Time */
> +#define AT91_ADC_STARTUP_9G45 (0x7f << 16)
> +#define AT91_ADC_STARTUP_9X5 (0xf << 16)
> +#define AT91_ADC_STARTUP_(x) ((x) << 16)
> +#define AT91_ADC_SHTIM (0xf << 24) /* Sample & Hold Time */
> +#define AT91_ADC_SHTIM_(x) ((x) << 24)
> +#define AT91_ADC_PENDBC (0x0f << 28) /* Pen Debounce time */
> +#define AT91_ADC_PENDBC_(x) ((x) << 28)
> +
> +#define AT91_ADC_TSR 0x0C
> +#define AT91_ADC_TSR_SHTIM (0xf << 24) /* Sample & Hold Time */
> +#define AT91_ADC_TSR_SHTIM_(x) ((x) << 24)
> +
> +#define AT91_ADC_CHER 0x10 /* Channel Enable Register */
> +#define AT91_ADC_CHDR 0x14 /* Channel Disable Register */
> +#define AT91_ADC_CHSR 0x18 /* Channel Status Register */
> +#define AT91_ADC_CH(n) (1 << (n)) /* Channel Number */
> +
> +#define AT91_ADC_SR 0x1C /* Status Register */
> +#define AT91_ADC_EOC(n) (1 << (n)) /* End of Conversion on Channel N */
> +#define AT91_ADC_OVRE(n) (1 << ((n) + 8))/* Overrun Error on Channel N */
> +#define AT91_ADC_DRDY (1 << 16) /* Data Ready */
> +#define AT91_ADC_GOVRE (1 << 17) /* General Overrun Error */
> +#define AT91_ADC_ENDRX (1 << 18) /* End of RX Buffer */
> +#define AT91_ADC_RXFUFF (1 << 19) /* RX Buffer Full */
> +
> +#define AT91_ADC_SR_9X5 0x30 /* Status Register for 9x5 */
> +#define AT91_ADC_SR_DRDY_9X5 (1 << 24) /* Data Ready */
> +
> +#define AT91_ADC_LCDR 0x20 /* Last Converted Data Register */
> +#define AT91_ADC_LDATA (0x3ff)
> +
> +#define AT91_ADC_IER 0x24 /* Interrupt Enable Register */
> +#define AT91_ADC_IDR 0x28 /* Interrupt Disable Register */
> +#define AT91_ADC_IMR 0x2C /* Interrupt Mask Register */
> +#define AT91RL_ADC_IER_PEN (1 << 20)
> +#define AT91RL_ADC_IER_NOPEN (1 << 21)
> +#define AT91_ADC_IER_PEN (1 << 29)
> +#define AT91_ADC_IER_NOPEN (1 << 30)
> +#define AT91_ADC_IER_XRDY (1 << 20)
> +#define AT91_ADC_IER_YRDY (1 << 21)
> +#define AT91_ADC_IER_PRDY (1 << 22)
> +#define AT91_ADC_ISR_PENS (1 << 31)
> +
> +#define AT91_ADC_CHR(n) (0x30 + ((n) * 4)) /* Channel Data Register N */
> +#define AT91_ADC_DATA (0x3ff)
> +
> +#define AT91_ADC_CDR0_9X5 (0x50) /* Channel Data Register 0 for 9X5 */
> +
> +#define AT91_ADC_ACR 0x94 /* Analog Control Register */
> +#define AT91_ADC_ACR_PENDETSENS (0x3 << 0) /* pull-up resistor */
> +
> +#define AT91_ADC_TSMR 0xB0
> +#define AT91_ADC_TSMR_TSMODE (3 << 0) /* Touch Screen Mode */
> +#define AT91_ADC_TSMR_TSMODE_NONE (0 << 0)
> +#define AT91_ADC_TSMR_TSMODE_4WIRE_NO_PRESS (1 << 0)
> +#define AT91_ADC_TSMR_TSMODE_4WIRE_PRESS (2 << 0)
> +#define AT91_ADC_TSMR_TSMODE_5WIRE (3 << 0)
> +#define AT91_ADC_TSMR_TSAV (3 << 4) /* Averages samples */
> +#define AT91_ADC_TSMR_TSAV_(x) ((x) << 4)
> +#define AT91_ADC_TSMR_SCTIM (0x0f << 16) /* Switch closure time */
> +#define AT91_ADC_TSMR_PENDBC (0x0f << 28) /* Pen Debounce time */
> +#define AT91_ADC_TSMR_PENDBC_(x) ((x) << 28)
> +#define AT91_ADC_TSMR_NOTSDMA (1 << 22) /* No Touchscreen DMA */
> +#define AT91_ADC_TSMR_PENDET_DIS (0 << 24) /* Pen contact detection disable */
> +#define AT91_ADC_TSMR_PENDET_ENA (1 << 24) /* Pen contact detection enable */
> +
> +#define AT91_ADC_TSXPOSR 0xB4
> +#define AT91_ADC_TSYPOSR 0xB8
> +#define AT91_ADC_TSPRESSR 0xBC
> +
> +#define AT91_ADC_TRGR_9260 AT91_ADC_MR
> +#define AT91_ADC_TRGR_9G45 0x08
> +#define AT91_ADC_TRGR_9X5 0xC0
> +
> +/* Trigger Register bit field */
> +#define AT91_ADC_TRGR_TRGPER (0xffff << 16)
> +#define AT91_ADC_TRGR_TRGPER_(x) ((x) << 16)
> +#define AT91_ADC_TRGR_TRGMOD (0x7 << 0)
> +#define AT91_ADC_TRGR_NONE (0 << 0)
> +#define AT91_ADC_TRGR_MOD_PERIOD_TRIG (5 << 0)
>
> #define AT91_ADC_CHAN(st, ch) \
> (st->registers->channel_base + (ch * 4))
>
^ permalink raw reply
* Re: [PATCH v2 07/15] iio: adc: at91: add sam9rl support
From: Jonathan Cameron @ 2014-03-22 12:34 UTC (permalink / raw)
To: Alexandre Belloni, Nicolas Ferre, Dmitry Torokhov
Cc: Jean-Christophe Plagniol-Villard,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard,
Gregory Clement
In-Reply-To: <1395064628-644-8-git-send-email-alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
On 17/03/14 13:57, Alexandre Belloni wrote:
> Signed-off-by: Alexandre Belloni <alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Acked-by: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
> drivers/iio/adc/at91_adc.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> index a51b8badbf8e..3b5bacd4d8da 100644
> --- a/drivers/iio/adc/at91_adc.c
> +++ b/drivers/iio/adc/at91_adc.c
> @@ -1346,6 +1346,20 @@ static struct at91_adc_caps at91sam9260_caps = {
> },
> };
>
> +static struct at91_adc_caps at91sam9rl_caps = {
> + .has_ts = true,
> + .calc_startup_ticks = calc_startup_ticks_9260, /* same as 9260 */
> + .num_channels = 6,
> + .registers = {
> + .channel_base = AT91_ADC_CHR(0),
> + .drdy_mask = AT91_ADC_DRDY,
> + .status_register = AT91_ADC_SR,
> + .trigger_register = AT91_ADC_TRGR_9G45,
> + .mr_prescal_mask = AT91_ADC_PRESCAL_9260,
> + .mr_startup_mask = AT91_ADC_STARTUP_9G45,
> + },
> +};
> +
> static struct at91_adc_caps at91sam9g45_caps = {
> .has_ts = true,
> .calc_startup_ticks = calc_startup_ticks_9260, /* same as 9260 */
> @@ -1380,6 +1394,7 @@ static struct at91_adc_caps at91sam9x5_caps = {
>
> static const struct of_device_id at91_adc_dt_ids[] = {
> { .compatible = "atmel,at91sam9260-adc", .data = &at91sam9260_caps },
> + { .compatible = "atmel,at91sam9rl-adc", .data = &at91sam9rl_caps },
> { .compatible = "atmel,at91sam9g45-adc", .data = &at91sam9g45_caps },
> { .compatible = "atmel,at91sam9x5-adc", .data = &at91sam9x5_caps },
> {},
> @@ -1391,6 +1406,9 @@ static const struct platform_device_id at91_adc_ids[] = {
> .name = "at91sam9260-adc",
> .driver_data = (unsigned long)&at91sam9260_caps,
> }, {
> + .name = "at91sam9rl-adc",
> + .driver_data = (unsigned long)&at91sam9rl_caps,
> + }, {
> .name = "at91sam9g45-adc",
> .driver_data = (unsigned long)&at91sam9g45_caps,
> }, {
>
^ permalink raw reply
* Re: [PATCH v2 12/15] ARM: at91: remove atmel_tsadcc platform_data
From: Jonathan Cameron @ 2014-03-22 12:39 UTC (permalink / raw)
To: Alexandre Belloni, Nicolas Ferre, Dmitry Torokhov
Cc: Jean-Christophe Plagniol-Villard,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard,
Gregory Clement
In-Reply-To: <1395064628-644-13-git-send-email-alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
On 17/03/14 13:57, Alexandre Belloni wrote:
> Signed-off-by: Alexandre Belloni <alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
See below.
> ---
> arch/arm/mach-at91/board.h | 3 ---
> include/linux/platform_data/atmel.h | 7 -------
> 2 files changed, 10 deletions(-)
>
> diff --git a/arch/arm/mach-at91/board.h b/arch/arm/mach-at91/board.h
> index 6c08b341167d..4e773b55bc2d 100644
> --- a/arch/arm/mach-at91/board.h
> +++ b/arch/arm/mach-at91/board.h
> @@ -118,9 +118,6 @@ struct isi_platform_data;
> extern void __init at91_add_device_isi(struct isi_platform_data *data,
> bool use_pck_as_mck);
>
> - /* Touchscreen Controller */
> -extern void __init at91_add_device_tsadcc(struct at91_tsadcc_data *data);
> -
> /* CAN */
> extern void __init at91_add_device_can(struct at91_can_data *data);
>
> diff --git a/include/linux/platform_data/atmel.h b/include/linux/platform_data/atmel.h
> index cea9f70133c5..4b452c6a2f7b 100644
> --- a/include/linux/platform_data/atmel.h
> +++ b/include/linux/platform_data/atmel.h
> @@ -86,13 +86,6 @@ struct atmel_uart_data {
> struct serial_rs485 rs485; /* rs485 settings */
> };
>
> - /* Touchscreen Controller */
> -struct at91_tsadcc_data {
> - unsigned int adc_clock;
> - u8 pendet_debounce;
> - u8 ts_sample_hold_time;
> -};
> -
I believe this is still in use in the driver you don't remove until patch 14.
Probably best to reorder the patches...
> /* CAN */
> struct at91_can_data {
> void (*transceiver_switch)(int on);
>
^ permalink raw reply
* Re: Re: [PATCH v2 2/8] mfd: AXP20x: Add bindings documentation
From: Carlo Caione @ 2014-03-22 14:11 UTC (permalink / raw)
To: Maxime Ripard
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
hdegoede-H+wXaHxf7aLQT0dZR+AlfA, emilio-0Z03zUJReD5OxF6Tv1QG9Q,
wens-jdAy2FN1RRM, sameo-VuQAYsv1563Yd54FQh9/CA,
lee.jones-QSEj5FYQhm4dnm+yROfE0A,
devicetree-u79uwXL29TY76Z2rM5mHXA,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <20140318084505.GT27873@lukather>
On Tue, Mar 18, 2014 at 09:45:05AM +0100, Maxime Ripard wrote:
> On Sat, Mar 15, 2014 at 04:43:39PM +0100, Carlo Caione wrote:
> > Bindings documentation for the AXP20x driver. In this file also two
> > sub-nodes (PEK and regulators) are documented.
>
> PEK doesn't look to be documented, either in this patch, or any other.
Residue from v1. To be deleted.
> > Signed-off-by: Carlo Caione <carlo-KA+7E9HrN00dnm+yROfE0A@public.gmane.org>
> > ---
> > Documentation/devicetree/bindings/mfd/axp20x.txt | 83 ++++++++++++++++++++++
> > .../devicetree/bindings/vendor-prefixes.txt | 1 +
>
> I don't really know what the DT maintainers are expecting here, but I
> would have done two patches.
Uhm, no idea (and no feedback). I'll split it.
> > 2 files changed, 84 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/mfd/axp20x.txt
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt
> > new file mode 100644
> > index 0000000..982aefe
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/axp20x.txt
> > @@ -0,0 +1,83 @@
> > +* axp20x device tree bindings
> > +
> > +The axp20x family current members :-
> > +axp202 (X-Powers)
> > +axp209 (X-Powers)
> > +
> > +Required properties:
> > +- compatible : Should be "x-powers,axp202" or "x-powers,axp209"
> > +- interrupt-controller : axp20x has its own internal IRQs
> > +- #interrupt-cells : Should be set to 1
> > +- interrupt-parent : The parent interrupt controller
> > +- interrupts : Interrupt specifiers for interrupt sources
> > +- reg : The I2C slave address for the AXP chip
> > +
> > +Sub-nodes:
> > +* regulators : Contain the regulator nodes. The regulators are bound using
> > + their name as listed here: dcdc2, dcdc3, ldo1, ldo2, ldo3,
> > + ldo4, ldo5.
> > + The bindings details of individual regulator device can be found in:
> > + Documentation/devicetree/bindings/regulator/regulator.txt with the
> > + exception of:
> > +
> > + - dcdc-freq : defines the work frequency of DC-DC in KHz
> > + (range: 750-1875). Default: 1.5MHz
> > + - dcdc-workmode : Optional. 1 for PWM mode, 0 for AUTO mode
> > + Default: AUTO mode
>
> Those two are x-powers specific, or would it make sense to have them
> in other drivers too?
>
> If the former, please add the x-powers prefix.
AFAIK this is AXP specific. I'll add the prefix.
> > +
> > +Example:
> > +
> > +axp: axp20x@34 {
> > + reg = <0x34>;
> > + interrupt-parent = <&nmi_intc>;
> > + interrupts = <0 8>;
> > +
> > + compatible = "x-powers,axp209";
> > + interrupt-controller;
> > + #interrupt-cells = <1>;
> > +
> > + regulators {
>
> Do we really need that subnode ? it looks useless, and we already know
> that we are defining regulators here.
What do you mean? We are defining the MFD and regulators are just one of
the subsystem here. Moveover I'm using the "regulators" node in the
regulators driver (using of_find_node_by_name()) to get the regulators
configuration.
> > + dcdc-freq = "1500";
>
> That frequency is defined at the same level than the dcdc-workmode
> property, yet they both seem to be placed at different levels.
They are at different level. dcdc-freq is valid for all the dcdc,
whereas dcdc-workmode is dcdc-specific.
I'll clarify the point.
> > +
> > + axp_dcdc2: dcdc2 {
> > + regulator-min-microvolt = <700000>;
> > + regulator-max-microvolt = <2275000>;
> > + dcdc-workmode = <0>;
> > + regulator-always-on;
> > + };
> > +
> > + axp_dcdc3: dcdc3 {
> > + regulator-min-microvolt = <700000>;
> > + regulator-max-microvolt = <3500000>;
> > + dcdc-workmode = <0>;
>
> It looks like those are at their default values?
Yes. To be fixed.
Thanks,
--
Carlo Caione
^ permalink raw reply
* Re: Re: [PATCH v2 6/8] ARM: sunxi: dt: Add x-powers-axp209.dtsi file
From: Carlo Caione @ 2014-03-22 14:31 UTC (permalink / raw)
To: Maxime Ripard
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
hdegoede-H+wXaHxf7aLQT0dZR+AlfA, emilio-0Z03zUJReD5OxF6Tv1QG9Q,
wens-jdAy2FN1RRM, sameo-VuQAYsv1563Yd54FQh9/CA,
lee.jones-QSEj5FYQhm4dnm+yROfE0A,
devicetree-u79uwXL29TY76Z2rM5mHXA,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <20140318090450.GV27873@lukather>
On Tue, Mar 18, 2014 at 10:04:50AM +0100, Maxime Ripard wrote:
> On Sat, Mar 15, 2014 at 04:43:43PM +0100, Carlo Caione wrote:
> > This dtsi describes the axp209 PMIC, and is to be included from inside
> > the i2c controller node to which the axp209 is connected.
> >
> > Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > Signed-off-by: Carlo Caione <carlo-KA+7E9HrN00dnm+yROfE0A@public.gmane.org>
> > ---
> > arch/arm/boot/dts/x-powers-axp209.dtsi | 60 ++++++++++++++++++++++++++++++++++
> > 1 file changed, 60 insertions(+)
> > create mode 100644 arch/arm/boot/dts/x-powers-axp209.dtsi
> >
> > diff --git a/arch/arm/boot/dts/x-powers-axp209.dtsi b/arch/arm/boot/dts/x-powers-axp209.dtsi
> > new file mode 100644
> > index 0000000..d272e67
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/x-powers-axp209.dtsi
> > @@ -0,0 +1,60 @@
> > +/*
> > + * x-powers,axp209 common code to be include from inside the axp209 node
> > + *
> > + * Copyright 2014 - Carlo Caione <carlo-KA+7E9HrN00dnm+yROfE0A@public.gmane.org>
> > + *
> > + * The code contained herein is licensed under the GNU General Public
> > + * License. You may obtain a copy of the GNU General Public License
> > + * Version 2 or later at the following locations:
> > + *
> > + * http://www.opensource.org/licenses/gpl-license.html
> > + * http://www.gnu.org/copyleft/gpl.html
> > + */
> > +
>
> I didn't even know such a thing was possible :)
> Nice hack.
Kudos to Hans ;)
>
> > + compatible = "x-powers,axp209";
> > + interrupt-controller;
> > + #interrupt-cells = <1>;
>
> However, I'd move this out of it, and in the board file, so that we
> actually get an idea by looking at the board DTS of what device we are
> actually registering at this given address, and what it's capable of.
Do you mean the whole dtsi or just those three lines?
--
Carlo Caione
^ permalink raw reply
* Re: Re: [PATCH v2 1/8] mfd: AXP20x: Add mfd driver for AXP20x PMIC
From: Carlo Caione @ 2014-03-22 16:51 UTC (permalink / raw)
To: Lee Jones
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
hdegoede-H+wXaHxf7aLQT0dZR+AlfA, emilio-0Z03zUJReD5OxF6Tv1QG9Q,
wens-jdAy2FN1RRM, sameo-VuQAYsv1563Yd54FQh9/CA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <20140318155919.GS25478@lee--X1>
On Tue, Mar 18, 2014 at 03:59:19PM +0000, Lee Jones wrote:
> > +static struct mfd_cell axp20x_cells[] = {
> > + {
> > + .name = "axp20x-pek",
> > + .num_resources = ARRAY_SIZE(axp20x_pek_resources),
> > + .resources = axp20x_pek_resources,
> > + }, {
> > + .name = "axp20x-regulator",
> > + },
> > +};
>
> nit: The format of these two structs are inconsistent.
Uhm, what do you mean?
> > +static int axp20x_i2c_probe(struct i2c_client *i2c,
> > + const struct i2c_device_id *id)
> > +{
> > + struct axp20x_dev *axp20x;
> > + const struct of_device_id *of_id;
> > + int ret;
> > +
> > + axp20x = devm_kzalloc(&i2c->dev, sizeof(*axp20x), GFP_KERNEL);
> > + if (!axp20x)
> > + return -ENOMEM;
> > +
> > + of_id = of_match_device(axp20x_of_match, &i2c->dev);
> > + if (!of_id) {
> > + dev_err(&i2c->dev, "Unable to setup AXP20X data\n");
> > + return -ENODEV;
> > + }
> > + axp20x->variant = (int) of_id->data;
>
> No need to cast from void *.
My compiler says otherwise :)
> [...]
>
> > +static const struct i2c_device_id axp20x_i2c_id[] = {
> > + { "axp202", AXP202_ID },
> > + { "axp209", AXP209_ID },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(i2c, axp20x_i2c_id);
>
> Isn't this redundant now that you're using of_id?
It is not. Even it is unused it has to be in the driver otherwise the
driver is not loaded. Probably it is something wrong in the I2C core.
Best,
--
Carlo Caione
^ permalink raw reply
* Re: Re: [PATCH v2 1/8] mfd: AXP20x: Add mfd driver for AXP20x PMIC
From: Mark Brown @ 2014-03-22 17:33 UTC (permalink / raw)
To: Carlo Caione
Cc: Lee Jones, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
hdegoede-H+wXaHxf7aLQT0dZR+AlfA, emilio-0Z03zUJReD5OxF6Tv1QG9Q,
wens-jdAy2FN1RRM, sameo-VuQAYsv1563Yd54FQh9/CA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
lgirdwood-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <20140322165132.GC20668-bi+AKbBUZKZeIdyRz4JgOMwOAu8XWILU@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 583 bytes --]
On Sat, Mar 22, 2014 at 05:51:32PM +0100, Carlo Caione wrote:
> On Tue, Mar 18, 2014 at 03:59:19PM +0000, Lee Jones wrote:
> > > + of_id = of_match_device(axp20x_of_match, &i2c->dev);
> > > + if (!of_id) {
> > > + dev_err(&i2c->dev, "Unable to setup AXP20X data\n");
> > > + return -ENODEV;
> > > + }
> > > + axp20x->variant = (int) of_id->data;
> > No need to cast from void *.
> My compiler says otherwise :)
Are you sure your compiler isn't correctly telling you not to cast away
const? If the compiler is complaining about a cast on void then it's
spotted a definite bug.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: Re: [PATCH v2 1/8] mfd: AXP20x: Add mfd driver for AXP20x PMIC
From: Carlo Caione @ 2014-03-22 18:05 UTC (permalink / raw)
To: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
Cc: Lee Jones, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
hdegoede-H+wXaHxf7aLQT0dZR+AlfA, emilio-0Z03zUJReD5OxF6Tv1QG9Q,
wens-jdAy2FN1RRM, sameo-VuQAYsv1563Yd54FQh9/CA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
lgirdwood-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <20140322173313.GU552-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
On Sat, Mar 22, 2014 at 6:33 PM, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Sat, Mar 22, 2014 at 05:51:32PM +0100, Carlo Caione wrote:
>> On Tue, Mar 18, 2014 at 03:59:19PM +0000, Lee Jones wrote:
>
>> > > + of_id = of_match_device(axp20x_of_match, &i2c->dev);
>> > > + if (!of_id) {
>> > > + dev_err(&i2c->dev, "Unable to setup AXP20X data\n");
>> > > + return -ENODEV;
>> > > + }
>> > > + axp20x->variant = (int) of_id->data;
>
>> > No need to cast from void *.
>
>> My compiler says otherwise :)
>
> Are you sure your compiler isn't correctly telling you not to cast away
> const? If the compiler is complaining about a cast on void then it's
> spotted a definite bug.
drivers/mfd/axp20x.c:172:18: warning: assignment makes integer from
pointer without a cast [enabled by default]
--
Carlo Caione
^ permalink raw reply
* Re: Re: [PATCH v2 1/8] mfd: AXP20x: Add mfd driver for AXP20x PMIC
From: Dmitry Torokhov @ 2014-03-22 18:42 UTC (permalink / raw)
To: Carlo Caione
Cc: Mark Brown, Lee Jones,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
hdegoede-H+wXaHxf7aLQT0dZR+AlfA, emilio-0Z03zUJReD5OxF6Tv1QG9Q,
wens-jdAy2FN1RRM, sameo-VuQAYsv1563Yd54FQh9/CA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
lgirdwood-Re5JQEeQqe8AvxtiuMwx3w,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <CAOQ7t2Yg5pp_rXYfsdnHpy-zGFGimzT3EW15P_0ugYMtgPtOrw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Sat, Mar 22, 2014 at 07:13:36PM +0100, Carlo Caione wrote:
> On Sat, Mar 22, 2014 at 6:33 PM, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > On Sat, Mar 22, 2014 at 05:51:32PM +0100, Carlo Caione wrote:
> >> On Tue, Mar 18, 2014 at 03:59:19PM +0000, Lee Jones wrote:
> >
> >> > > + of_id = of_match_device(axp20x_of_match, &i2c->dev);
> >> > > + if (!of_id) {
> >> > > + dev_err(&i2c->dev, "Unable to setup AXP20X data\n");
> >> > > + return -ENODEV;
> >> > > + }
> >> > > + axp20x->variant = (int) of_id->data;
> >
> >> > No need to cast from void *.
> >
> >> My compiler says otherwise :)
> >
> > Are you sure your compiler isn't correctly telling you not to cast away
> > const? If the compiler is complaining about a cast on void then it's
> > spotted a definite bug.
>
> [resend since gmail decided to not put broonie in CC]
>
> drivers/mfd/axp20x.c:172:18: warning: assignment makes integer
> frompointer without a cast [enabled by default]
You need to cast to long, otherwise you will get warnings when compiling
on 64 bit arches.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: Re: [PATCH v2 1/8] mfd: AXP20x: Add mfd driver for AXP20x PMIC
From: Carlo Caione @ 2014-03-22 19:08 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Mark Brown, Lee Jones,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
hdegoede-H+wXaHxf7aLQT0dZR+AlfA, emilio-0Z03zUJReD5OxF6Tv1QG9Q,
wens-jdAy2FN1RRM, sameo-VuQAYsv1563Yd54FQh9/CA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
lgirdwood-Re5JQEeQqe8AvxtiuMwx3w,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20140322184201.GA27611-WlK9ik9hQGAhIp7JRqBPierSzoNAToWh@public.gmane.org>
On Sat, Mar 22, 2014 at 11:42:01AM -0700, Dmitry Torokhov wrote:
> On Sat, Mar 22, 2014 at 07:13:36PM +0100, Carlo Caione wrote:
> > On Sat, Mar 22, 2014 at 6:33 PM, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > > On Sat, Mar 22, 2014 at 05:51:32PM +0100, Carlo Caione wrote:
> > >> On Tue, Mar 18, 2014 at 03:59:19PM +0000, Lee Jones wrote:
> > >
> > >> > > + of_id = of_match_device(axp20x_of_match, &i2c->dev);
> > >> > > + if (!of_id) {
> > >> > > + dev_err(&i2c->dev, "Unable to setup AXP20X data\n");
> > >> > > + return -ENODEV;
> > >> > > + }
> > >> > > + axp20x->variant = (int) of_id->data;
> > >
> > >> > No need to cast from void *.
> > >
> > >> My compiler says otherwise :)
> > >
> > > Are you sure your compiler isn't correctly telling you not to cast away
> > > const? If the compiler is complaining about a cast on void then it's
> > > spotted a definite bug.
> >
> > [resend since gmail decided to not put broonie in CC]
> >
> > drivers/mfd/axp20x.c:172:18: warning: assignment makes integer
> > frompointer without a cast [enabled by default]
>
> You need to cast to long, otherwise you will get warnings when compiling
> on 64 bit arches.
Actually no warning also with (int) cast.
--
Carlo Caione
^ permalink raw reply
* Re: [linux-sunxi] Re: [PATCH v2 1/8] mfd: AXP20x: Add mfd driver for AXP20x PMIC
From: Carlo Caione @ 2014-03-22 18:13 UTC (permalink / raw)
To: Mark Brown
Cc: Lee Jones, linux-arm-kernel, maxime.ripard, hdegoede, emilio,
wens, sameo, devicetree, dmitry.torokhov, linux-input, linux-doc,
lgirdwood, linux-sunxi
In-Reply-To: <20140322173313.GU552@sirena.org.uk>
On Sat, Mar 22, 2014 at 6:33 PM, Mark Brown <broonie@kernel.org> wrote:
> On Sat, Mar 22, 2014 at 05:51:32PM +0100, Carlo Caione wrote:
>> On Tue, Mar 18, 2014 at 03:59:19PM +0000, Lee Jones wrote:
>
>> > > + of_id = of_match_device(axp20x_of_match, &i2c->dev);
>> > > + if (!of_id) {
>> > > + dev_err(&i2c->dev, "Unable to setup AXP20X data\n");
>> > > + return -ENODEV;
>> > > + }
>> > > + axp20x->variant = (int) of_id->data;
>
>> > No need to cast from void *.
>
>> My compiler says otherwise :)
>
> Are you sure your compiler isn't correctly telling you not to cast away
> const? If the compiler is complaining about a cast on void then it's
> spotted a definite bug.
[resend since gmail decided to not put broonie in CC]
drivers/mfd/axp20x.c:172:18: warning: assignment makes integer
frompointer without a cast [enabled by default]
--
Carlo Caione
^ permalink raw reply
* Re: Re: [PATCH v2 1/8] mfd: AXP20x: Add mfd driver for AXP20x PMIC
From: Mark Brown @ 2014-03-22 19:29 UTC (permalink / raw)
To: Carlo Caione
Cc: Dmitry Torokhov, Lee Jones,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
hdegoede-H+wXaHxf7aLQT0dZR+AlfA, emilio-0Z03zUJReD5OxF6Tv1QG9Q,
wens-jdAy2FN1RRM, sameo-VuQAYsv1563Yd54FQh9/CA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
lgirdwood-Re5JQEeQqe8AvxtiuMwx3w,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20140322190803.GA20987-bi+AKbBUZKZeIdyRz4JgOMwOAu8XWILU@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 590 bytes --]
On Sat, Mar 22, 2014 at 08:08:03PM +0100, Carlo Caione wrote:
> On Sat, Mar 22, 2014 at 11:42:01AM -0700, Dmitry Torokhov wrote:
> > > drivers/mfd/axp20x.c:172:18: warning: assignment makes integer
> > > frompointer without a cast [enabled by default]
> > You need to cast to long, otherwise you will get warnings when compiling
> > on 64 bit arches.
> Actually no warning also with (int) cast.
That's because you squashed it to something smaller than a long using
the cast. The other option is to make the variable you're assigning to
suitable to hold the value - intptr_t or a long.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v4 0/3] input: appletouch: fixes for jagged/uneven cursor movement
From: Clinton Sprain @ 2014-03-23 13:34 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg
In-Reply-To: <5320EA03.2040206@gmail.com>
Ping?
On 03/12/2014 06:13 PM, Clinton Sprain wrote:
> input: appletouch: fixes for jagged/uneven cursor movement
>
> The appletouch driver can make some touchpads feel insensitive, with movement that tends to jerk in steps. This is particularly evident when moving diagonally. There is also undesirable movement when additional fingers enter or leave the touchpad.
>
> These patches address these issues by dialing back the fuzz setting, by implementing sensor data smoothing and by discarding movements that directly coincide with a change in the number of fingers detected on the touchpad.
>
> Changes since v3:
>
> 1: No change
> 2: Fix loop counter in atp_calculate_abs
> 3: No change
>
> Clinton Sprain (3):
> input: appletouch: dial back fuzz setting
> input: appletouch: implement sensor data smoothing
> input: appletouch: fix jumps when additional fingers are detected
>
> drivers/input/mouse/appletouch.c | 114 +++++++++++++++++++++++++++-----------
> 1 file changed, 83 insertions(+), 31 deletions(-)
>
^ permalink raw reply
* Re: [PATCH v4 2/3] input: appletouch: implement sensor data smoothing
From: Henrik Rydberg @ 2014-03-23 13:59 UTC (permalink / raw)
To: Clinton Sprain, linux-input; +Cc: Dmitry Torokhov
In-Reply-To: <5320EAC5.5000506@gmail.com>
Hi Clinton,
> input: appletouch: implement sensor data smoothing
>
> Use smoothed version of sensor array data to calculate movement and add weight
> to prior values when calculating average. This gives more granular and more
> predictable movement.
>
> Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
> ---
> drivers/input/mouse/appletouch.c | 72 ++++++++++++++++++++++++++++----------
> 1 file changed, 53 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
> index 2745832..e00f126 100644
> --- a/drivers/input/mouse/appletouch.c
> +++ b/drivers/input/mouse/appletouch.c
> @@ -332,7 +332,11 @@ static void atp_reinit(struct work_struct *work)
> static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
> int *z, int *fingers)
> {
> - int i;
> + int i, k;
> + int smooth[nb_sensors + 8];
> + int smooth_tmp[nb_sensors + 8];
> + int scale = 12;
> +
> /* values to calculate mean */
> int pcum = 0, psum = 0;
> int is_increasing = 0;
> @@ -344,9 +348,6 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
> if (is_increasing)
> is_increasing = 0;
>
> - continue;
> - }
> -
> /*
> * Makes the finger detection more versatile. For example,
> * two fingers with no gap will be detected. Also, my
> @@ -361,27 +362,60 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
> *
> * - Jason Parekh <jasonparekh@gmail.com>
> */
> - if (i < 1 ||
> +
> + } else if (i < 1 ||
> (!is_increasing && xy_sensors[i - 1] < xy_sensors[i])) {
> (*fingers)++;
> is_increasing = 1;
> } else if (i > 0 && (xy_sensors[i - 1] - xy_sensors[i] > threshold)) {
> is_increasing = 0;
> }
> + }
>
> - /*
> - * Subtracts threshold so a high sensor that just passes the
> - * threshold won't skew the calculated absolute coordinate.
> - * Fixes an issue where slowly moving the mouse would
> - * occasionally jump a number of pixels (slowly moving the
> - * finger makes this issue most apparent.)
> - */
> - pcum += (xy_sensors[i] - threshold) * i;
> - psum += (xy_sensors[i] - threshold);
> + /*
> + * Use a smoothed version of sensor data for movement calculations, to
> + * combat noise without needing to rely so heavily on a threshold.
> + * This improves tracking.
> + *
> + * The smoothed array is bigger than the original so that the smoothing
> + * doesn't result in edge values being truncated.
> + */
> +
> + for (i = 0; i < 4; i++)
> + smooth[i] = 0;
> + for (i = nb_sensors + 4; i < nb_sensors + 8; i++)
> + smooth[i] = 0;
> +
> + /* Pull base values, scaled up to help avoid truncation errors. */
> +
> + for (i = 0; i < nb_sensors; i++)
> + smooth[i + 4] = xy_sensors[i] << scale;
> +
> + for (k = 0; k < 4; k++) {
> +
> + /* Handle edge. */
> + smooth_tmp[0] = (smooth[0] + smooth[1]) >> 1;
> +
> + /* Average values with neighbors. */
> + for (i = 1; i < nb_sensors + 7; i++)
> + smooth_tmp[i] = (smooth[i - 1] + smooth[i] * 2 + smooth[i + 1]) >> 2;
> +
> + /* Handle other edge. */
> + smooth_tmp[nb_sensors + 7] = (smooth[nb_sensors + 7] + smooth[nb_sensors + 6]) >> 1;
> +
> + for (i = 0; i < nb_sensors + 8; i++)
> + smooth[i] = smooth_tmp[i];
> + }
> +
> + for (i = 0; i < nb_sensors + 8; i++) {
> + if ((smooth[i] >> scale) > 0) { /* Skip individual values if */
> + pcum += (smooth[i]) * i; /* they are small enough to */
> + psum += (smooth[i]); /* be truncated to 0 by our */
> + } /* scale; mostly just noise. */
> }
>
> if (psum > 0) {
> - *z = psum;
> + *z = psum >> scale; /* Scale down pressure output. */
> return pcum * fact / psum;
> }
>
> @@ -559,8 +593,8 @@ static void atp_complete_geyser_1_2(struct urb *urb)
>
> if (x && y) {
> if (dev->x_old != -1) {
> - x = (dev->x_old * 3 + x) >> 2;
> - y = (dev->y_old * 3 + y) >> 2;
> + x = (dev->x_old * 7 + x) >> 3;
> + y = (dev->y_old * 7 + y) >> 3;
> dev->x_old = x;
> dev->y_old = y;
>
> @@ -671,8 +705,8 @@ static void atp_complete_geyser_3_4(struct urb *urb)
>
> if (x && y) {
> if (dev->x_old != -1) {
> - x = (dev->x_old * 3 + x) >> 2;
> - y = (dev->y_old * 3 + y) >> 2;
> + x = (dev->x_old * 7 + x) >> 3;
> + y = (dev->y_old * 7 + y) >> 3;
> dev->x_old = x;
> dev->y_old = y;
>
>
Reviewed-by: Henrik Rydberg <rydberg@euromail.se>
Thanks,
Henrik
^ permalink raw reply
* Re: [PATCH v4 0/3] input: appletouch: fixes for jagged/uneven cursor movement
From: Henrik Rydberg @ 2014-03-23 14:02 UTC (permalink / raw)
To: Clinton Sprain, linux-input; +Cc: Dmitry Torokhov
In-Reply-To: <5320EA03.2040206@gmail.com>
> input: appletouch: fixes for jagged/uneven cursor movement
>
> The appletouch driver can make some touchpads feel insensitive, with movement that tends to jerk in steps. This is particularly evident when moving diagonally. There is also undesirable movement when additional fingers enter or leave the touchpad.
>
> These patches address these issues by dialing back the fuzz setting, by implementing sensor data smoothing and by discarding movements that directly coincide with a change in the number of fingers detected on the touchpad.
Thanks Clinton, for resolving this long-standing nuisance. It all looks good to
me now.
Henrik
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox