* Re: [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator
From: Rob Herring @ 2018-01-03 19:48 UTC (permalink / raw)
To: Mylène Josserand
Cc: dmitry.torokhov, mark.rutland, linux, maxime.ripard, wens,
linux-arm-kernel, linux-input, devicetree, linux-kernel,
thomas.petazzoni, quentin.schulz
In-Reply-To: <20171228163336.28131-2-mylene.josserand@free-electrons.com>
On Thu, Dec 28, 2017 at 05:33:35PM +0100, Mylène Josserand wrote:
> Add the support of regulator to use it as VCC source.
>
> Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
> ---
> .../bindings/input/touchscreen/edt-ft5x06.txt | 1 +
> drivers/input/touchscreen/edt-ft5x06.c | 33 ++++++++++++++++++++++
> 2 files changed, 34 insertions(+)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH 2/2] HID: i2c-hid: Fix resume issue on Raydium touchscreen device
From: Benjamin Tissoires @ 2018-01-04 10:34 UTC (permalink / raw)
To: Aaron Ma; +Cc: lkml, linux-input, Jiri Kosina
In-Reply-To: <20180102173006.506-2-aaron.ma@canonical.com>
Hi Aaron,
On Tue, Jan 2, 2018 at 6:30 PM, Aaron Ma <aaron.ma@canonical.com> wrote:
> When Rayd touchscreen resumed from S3, it issues too many errors like:
> i2c_hid i2c-RAYD0001:00: i2c_hid_get_input: incomplete report (58/5442)
>
> And all the report data are corrupted, touchscreen is unresponsive.
>
> Fix this by re-sending report description command after resume.
Is this something the Windows driver does unconditionally?
I'd rather not add a quirk if the Windows driver does it all the time,
and hardware manufacturers start relying on it.
Otherwise, the patch looks good, I just want to be sure that we
actually need the quirk or if we should do it all the time.
Cheers,
Benjamin
> Add device ID as a quirk.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> ---
> drivers/hid/hid-ids.h | 3 +++
> drivers/hid/i2c-hid/i2c-hid.c | 13 +++++++++++++
> 2 files changed, 16 insertions(+)
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 5da3d6256d25..753cc10aa699 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -516,6 +516,9 @@
> #define I2C_VENDOR_ID_HANTICK 0x0911
> #define I2C_PRODUCT_ID_HANTICK_5288 0x5288
>
> +#define I2C_VENDOR_ID_RAYD 0x2386
> +#define I2C_PRODUCT_ID_RAYD_3118 0x3118
> +
> #define USB_VENDOR_ID_HANWANG 0x0b57
> #define USB_DEVICE_ID_HANWANG_TABLET_FIRST 0x5000
> #define USB_DEVICE_ID_HANWANG_TABLET_LAST 0x8fff
> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> index 09404ffdb08b..57a447a9d40e 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.c
> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> @@ -47,6 +47,7 @@
> /* quirks to control the device */
> #define I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV BIT(0)
> #define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET BIT(1)
> +#define I2C_HID_QUIRK_RESEND_REPORT_DESCR BIT(2)
>
> /* flags */
> #define I2C_HID_STARTED 0
> @@ -171,6 +172,8 @@ static const struct i2c_hid_quirks {
> I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV },
> { I2C_VENDOR_ID_HANTICK, I2C_PRODUCT_ID_HANTICK_5288,
> I2C_HID_QUIRK_NO_IRQ_AFTER_RESET },
> + { I2C_VENDOR_ID_RAYD, I2C_PRODUCT_ID_RAYD_3118,
> + I2C_HID_QUIRK_RESEND_REPORT_DESCR },
> { 0, 0 }
> };
>
> @@ -1211,6 +1214,16 @@ static int i2c_hid_resume(struct device *dev)
> if (ret)
> return ret;
>
> + /* RAYDIUM device (2386:3118) need to re-send report descr cmd
> + * after resume, after this it will be back normal.
> + * otherwise it issues too many incomplete reports.
> + */
> + if (ihid->quirks & I2C_HID_QUIRK_RESEND_REPORT_DESCR) {
> + ret = i2c_hid_command(client, &hid_report_descr_cmd, NULL, 0);
> + if (!ret)
> + return ret;
> + }
> +
> if (hid->driver && hid->driver->reset_resume) {
> ret = hid->driver->reset_resume(hid);
> return ret;
> --
> 2.14.3
>
^ permalink raw reply
* Re: [PATCH 1/2] HID: core: i2c-hid: fix size check and type usage
From: Benjamin Tissoires @ 2018-01-04 10:44 UTC (permalink / raw)
To: Aaron Ma; +Cc: lkml, linux-input, Jiri Kosina
In-Reply-To: <20180102173006.506-1-aaron.ma@canonical.com>
Hi Aaron,
There are quite some changes I'd like to see in this patch. See below.
On Tue, Jan 2, 2018 at 6:30 PM, Aaron Ma <aaron.ma@canonical.com> wrote:
> When convert char array with signed int, if the inbuf[x] is negative then
> upper bits will be set to 1. Fix this by using u8 instead of char.
>
> ret_size has to be at least 3, hid_input_report use it after minus 2 bytes.
>
> size should be more than 0 to keep memset safe.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> ---
> drivers/hid/hid-core.c | 4 ++--
> drivers/hid/i2c-hid/i2c-hid.c | 10 +++++-----
I'd like to have this patch at least split in 2. One for hid-core, one
for i2c-hid.
Both files are not targeting the same issue, so it would make sense to
not have them in the same patch.
> 2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 0c3f608131cf..992547771d96 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1506,7 +1506,7 @@ int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size,
> if (rsize > HID_MAX_BUFFER_SIZE)
> rsize = HID_MAX_BUFFER_SIZE;
>
> - if (csize < rsize) {
> + if ((csize < rsize) && (csize > 0)) {
I would think a call to this function with a csize <= 0 is a reason to fail.
And actually, I think an other thing we shouold do is changing the
prototype to have an unsigned int for size instead of a simple int.
Tracking the impact of such change might involve a bigger patch than a
simple check here, but we should be able to at least have the compiler
complaining if some driver starts using negative values.
> dbg_hid("report %d is too short, (%d < %d)\n", report->id,
> csize, rsize);
> memset(cdata + csize, 0, rsize - csize);
> @@ -1566,7 +1566,7 @@ int hid_input_report(struct hid_device *hid, int type, u8 *data, int size, int i
> report_enum = hid->report_enum + type;
> hdrv = hid->driver;
>
> - if (!size) {
> + if (size <= 0) {
This should also be solved by changing the prototype of the function.
> dbg_hid("empty report\n");
> ret = -1;
> goto unlock;
> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> index e054ee43c1e2..09404ffdb08b 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.c
> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> @@ -144,10 +144,10 @@ struct i2c_hid {
> * register of the HID
> * descriptor. */
> unsigned int bufsize; /* i2c buffer size */
> - char *inbuf; /* Input buffer */
> - char *rawbuf; /* Raw Input buffer */
> - char *cmdbuf; /* Command buffer */
> - char *argsbuf; /* Command arguments buffer */
> + u8 *inbuf; /* Input buffer */
> + u8 *rawbuf; /* Raw Input buffer */
> + u8 *cmdbuf; /* Command buffer */
> + u8 *argsbuf; /* Command arguments buffer */
>
> unsigned long flags; /* device flags */
> unsigned long quirks; /* Various quirks */
> @@ -473,7 +473,7 @@ static void i2c_hid_get_input(struct i2c_hid *ihid)
>
> ret_size = ihid->inbuf[0] | ihid->inbuf[1] << 8;
>
> - if (!ret_size) {
> + if (ret_size <= 2) {
Please do a separate case, after this one. RESET is acked by a size of
0, a size of 1 or 2 is a bug that needs to be fixed from the HW point
of view.
Cheers,
Benjamin
> /* host or device initiated RESET completed */
> if (test_and_clear_bit(I2C_HID_RESET_PENDING, &ihid->flags))
> wake_up(&ihid->wait);
> --
> 2.14.3
>
^ permalink raw reply
* Re: [PATCH 2/2] HID: i2c-hid: Fix resume issue on Raydium touchscreen device
From: Aaron Ma @ 2018-01-04 12:56 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: lkml, linux-input, Jiri Kosina
In-Reply-To: <CAO-hwJJ3n3YD1+pKvKRRoCX8y-5xZwemKLuLi7aqWSQT=J645A@mail.gmail.com>
Hi Benjamin:
Thanks for reviewing my patches.
This issue only happened on this RayD 2386:3118 touchscreen.
No other devices found, so I think it should be in quirk.
^ permalink raw reply
* Re: [PATCH 1/2] HID: core: i2c-hid: fix size check and type usage
From: Aaron Ma @ 2018-01-04 12:58 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: lkml, linux-input, Jiri Kosina
In-Reply-To: <CAO-hwJ+3Dz3g410cHwz3o_0f=2J2aa=+E5RY2sG28amjw8LcHQ@mail.gmail.com>
I will follow your advice and send V2.
Thanks,
Aaron
^ permalink raw reply
* Re: [PATCH 12/14] iio: adc: at91-sama5d2_adc: support for position and pressure channels
From: Eugen Hristev @ 2018-01-04 15:17 UTC (permalink / raw)
To: Jonathan Cameron
Cc: nicolas.ferre-UWL1GkI3JZL3oGB3hsPCZA,
ludovic.desroches-UWL1GkI3JZL3oGB3hsPCZA,
alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <20171229170236.06bf6394@archlinux>
On 29.12.2017 19:02, Jonathan Cameron wrote:
> On Fri, 22 Dec 2017 17:07:19 +0200
> Eugen Hristev <eugen.hristev-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org> wrote:
>
>> The ADC IP supports position and pressure measurements for a touchpad
>> connected on channels 0,1,2,3 for a 4-wire touchscreen with pressure
>> measurement support.
>> Using the inkern API, a driver can request a trigger and read the
>> channel values from the ADC.
>> The implementation provides a trigger named "touch" which can be
>> connected to a consumer driver.
>> Once a driver connects and attaches a pollfunc to this trigger, the
>> configure trigger callback is called, and then the ADC driver will
>> initialize pad measurement.
>> First step is to enable touchscreen 4wire support and enable
>> pen detect IRQ.
>> Once a pen is detected, a periodic trigger is setup to trigger every
>> 2 ms (e.g.) and sample the resistive touchscreen values. The trigger poll
>> is called, and the consumer driver is then woke up, and it can read the
>> respective channels for the values : X, and Y for position and pressure
>> channel.
>> Because only one trigger can be active in hardware in the same time,
>> while touching the pad, the ADC will block any attempt to use the
>> triggered buffer. Same, conversions using the software trigger are also
>> impossible (since the periodic trigger is setup).
>> If some driver wants to attach while the trigger is in use, it will
>> also fail.
>> Once the pen is not detected anymore, the trigger is free for use (hardware
>> or software trigger, with or without DMA).
>> Channels 0,1,2 and 3 are unavailable if a touchscreen is enabled.
>>
>> Some parts of this patch are based on initial original work by
>> Mohamed Jamsheeth Hajanajubudeen and Bandaru Venkateswara Swamy
>>
> OK, so comments inline.
>
> What I'm missing currently though is an explanation of why the slightly
> more standard arrangement of using a callback buffer doesn't work here.
> The only addition I think you need to do that is to allow a consumer to
> request a particular trigger. I also think some of the other provisions
> could be handled using standard features and slightly reducing the flexibility.
> I don't know for example if it's useful to allow other channels to be
> read when touch is not in progress or not.
>
> So restrictions:
>
> 1. Touch screen channels can only be read when touch is enabled.
> - use the available_scan_masks to control this. Or the callback that lets
> you do the same dynamically.
> 2. You need to push these channels to your consumer driver.
> - register a callback buffer rather than jumping through the hoops to
> insert your own pollfunc. That will call a function in your
> consumer, providing the data from the 3 channels directly.
> 3. You need to make sure it is using the right driver. For that you
> will I think need a new interface.
>
> Various other comments inline. I may well be missing something as this is
> a fair bit of complex code to read - if so then next version should have
> a clear cover letter describing why this more standard approach can't be
> used.
Hello Jonathan and thanks for the review of my patch series,
before starting and working over the required modifications and
suggestions that you sent me, I want to be a little more explicit about
the design of my implementation.
Hope this will clarify some things, and maybe I can as well understand
better what you have in mind to support this feature set.
Why have I picked a pollfunction: We discussed a while back on the
mailing list that you do not have an inkern mechanism to expose the
triggers to other drivers, and that it may be a good idea to have it for
such kind of actually multi function device, instead of having a MFD
driver, an ADC driver, and an Input driver, all sharing the same
register map, the same IRQ , etc, with some kind of synchronization to
avoid stepping on each other for the hardware resource.
So I considered to expose the trigger by attaching and detaching
pollfunctions to it. Which is the main thing what we use a trigger for.
So, what I had in mind, was to create a consumer driver that will
request triggers from the IIO device just like other drivers request
channels (part which is already done in IIO).
In order to do this I had to somehow wake up the consumer driver when
new data was available from the touchscreen. So, having the IRQ only in
the ADC device, and then on Pen detect and No pen detect just start or
stop the periodic trigger, which needs to be polled. The magic part is
that the consumer driver has a poll function already attached to this
trigger, so the poll function is just called every time we have new
data. The poll function is attached as an irq handler, and then we can
reuse all the read_raw data by using a scheduled work from the consumer
driver, to read the channels.
To do this, the ADC registers a special trigger named "touch trigger"
which is never enabled by the ADC driver. Instead, when a pollfunc is
attached to it, the attach function will also configure it with enabled
state. In the ADC, this means to start the touchscreen functionality. If
the touch is requested, it will standby and wait for pen detect IRQ.
Once we have pen detect, we can use a periodic trigger to sample the
touch data, and poll the "touch" trigger. The consumer driver will wake
up and schedule a work , that will use the standard read raw interface
(inkern) that will read three virtual channels (position + pressure).
They are not actual hardware channels, as the touch information is being
received on channels 0,1,2,3, but reading these virtual channels will
read from different registers inside the ADC IP ( x position, y
position, pressure), do some computations on the data, and feed the
consumer with the values , hiding the behind the scenes hardware
specific calculations.
After trigger is polled , the ADC will resume normal functionality, and
the consumer driver will continue to sleep.
We need to have a periodic trigger to sample the data because the actual
analog to digital conversion inside the IP block needs to be triggered.
The touchscreen data measurements cannot happen in hardware without
being triggered. If I try with a hrtimer to get a periodic IRQ to just
read the data, it will never be ready. The datasheet states that the
touchscreen measurements "will be attached to the conversion sequence".
So the periodic trigger is forcing a conversion sequence. This could be
done with a software trigger as well, but why the hassle to start it
every 2 milliseconds (or other time interval), if we can do it by
periodic trigger ?
Once we get the No pen IRQ, we stop the periodic trigger and it can be
used in another purpose (software or external as of now in the driver,
in the future we can add PWM trigger and Timer trigger)
In short, the ADC in Sama5D2 also supports touchscreen, and in
touchscreen mode , 4 of the channels are being used for this purpose.
This however, doesn't stop the ADC to use the other channels . The
hardware has 12 total single channels and they can be paired to have 6
more differential channels. The only thing that is blocked is the
trigger, but only if the pen is touching (when we start the periodic
trigger to sample the touchscreen). If the pen is not touching, an
external trigger or software trigger can be used without any issues (so
why limit the functionality, if this is available from hardware ?).
Because of the reason I discussed above (touchscreen sequence must be
triggered), we cannot use another trigger in the same time.
I see your idea with the callback buffer and it's worth exploring.
Mainly this series was to actually show you what I had in mind about
supporting the resistive touchscreen, and to give you some actually
working code/patch, so we can discuss based on real implementation, not
just suppositions.
You are right in many of the other comments that you said, and I will
come up with a v2 to this series. For now, I need to know if this is a
good or right direction in which I am going, or I should try to change
all the mechanism to callback buffer ? Or maybe I am totally in a bad
direction ?
The requirements are that the consumer driver needs to be somehow woke
up for every new touch data available, and report to the input
subsystem. As it was done before, the at91 old driver, just creates and
registers an input device by itself, and then reports the position and
touches. I was thinking that with this trigger consumer implementation,
things can be better in terms of subsystem separation and support.
Thanks again and let me know of your thoughts,
Eugen
[...]
^ permalink raw reply
* Business Possibility
From: Peter Deng @ 2018-01-04 16:13 UTC (permalink / raw)
To: linux-input
Hello there, My name is Peter Deng a South African citizen and a friend to Mrs Mugabe sister . I got your contact through Korean business online directory. I represent the interest of Mrs Mugabe who wishes to move a total amount of $19 million into a safe account owns by a trusted business man who is capable of accommodating such volume of funds with absolute trust & confidentiality.
We are willing to give 5-10 percent of the total money for this efforts . A non disclosure non circumvent agreement will be signed before the eventual transfer of funds. It is important for to know that the fund owner is going through a period of political turmoil and she is in a precarious position, so this transaction has to be highly secretive & very confidential.
Kindly respond back to me if this is what you can handle .
Regards,
Peter.
^ permalink raw reply
* Re: xps 15 9560 touchpad high interrupts
From: Baruch Siach @ 2018-01-04 19:01 UTC (permalink / raw)
To: Javad Karabi; +Cc: linux-input, linux-i2c
In-Reply-To: <CAEOHGOkZBGuu5mXjyRYQ0jpRhYKkoenqYv+_r1_M2kq0xwtbLg@mail.gmail.com>
Hi Javad Karabi,
On Tue, Jan 02, 2018 at 06:24:54PM -0600, Javad Karabi wrote:
> On Tue, Jan 2, 2018 at 7:46 AM, Baruch Siach <baruch@tkos.co.il> wrote:
> > On Mon, Jan 01, 2018 at 05:18:00PM -0600, Javad Karabi wrote:
> >> On Mon, Jan 1, 2018 at 12:14 AM, Baruch Siach <baruch@tkos.co.il> wrote:
> >> > On Sun, Dec 31, 2017 at 03:58:57PM -0600, Javad Karabi wrote:
> >> >> On Sun, Dec 31, 2017 at 12:25 AM, Baruch Siach <baruch@tkos.co.il> wrote:
> >> >> > Added linux-input list to Cc.
> >> >> >
> >> >> > On Sat, Dec 30, 2017 at 05:10:06PM -0600, Javad Karabi wrote:
> >> >> > > im trying to figure out why i get like 7000 interrupts a second simply by
> >> >> > > resting my finger on the touchpad (not even moving it)
> >> >> > > this is on a xps 15 9560
> >> >> > > and the touchpad is at
> >> >> > > DLL07BE:01 06CB:7A13 Touchpad as
> >> >> > > /devices/pci0000:00/0000:00:15.1/i2c_designware.1/i2c-7/
> >> >> > i2c-DLL07BE:01/0018:06CB:7A13.0009/input/input58
> >> >> > >
> >> >> > > could you provide me with any advice as to where i should look to figure
> >> >> > > this out?
> >> >> > > i have already tried adding code to i2c-designware-platdrv.c
> >> >> > > i added
> >> >> > > dev->clk_freq = 100000;
> >> >> > > in dw_i2c_plat_probe, but it still shoots off thousands of interrupts a
> >> >> > > second (and they are apparently spurious interrupts, atleast accoding
> >> >> > > to /proc/irq/17/spurious
> >> >> > >
> >> >> > > could you provide any guidance at all? would be much appreciated.. i
> >> >> > > would love to fix this issue and get it upstreamed in the kernel if
> >> >> > > possible.
> >> >> > > thank you
> >> >> >
> >> >> > i2c_designware is a I2C bus master driver. It allows the host to
> >> >> > communicate with various devices. Your touchpad is apparently one such
> >> >> > device. But each device on the I2C bus needs its own driver. I have no
> >> >> > idea which driver handles your touchpad device. Maybe someone on the
> >> >> > linux-input list knows.
> >> >> >
> >> >> > Specifically, the interrupts handling has nothing to do with the I2C bus.
> >> >> > Unlike PCIe, I2C provides no in-bus interrupt delivery facility. I2C
> >> >> > devices usually use a separate dedicated interrupt line. So the spurious
> >> >> > interrupts that you see must be handled at the touchpad input driver
> >> >> > level.
> >> >> >
> >> >> > One thing that might help others help you is the version of the kernel you
> >> >> > are running. Please provide the output of 'uname -rv' on your machine.
> >> >>
> >> >> uname -rv
> >> >> 4.15.0-rc5 #2 SMP Thu Dec 28 18:21:06 CST 2017
> >> >>
> >> >> for what its worth, i think it might be hid_multitouch that is handling the
> >> >> touchpad, since when i rmmod it, my touchpad is no longer active.
> >> >
> >> > The hid-multitouch driver handles USB devices, not I2C. The code at
> >> > drivers/hid/hid-multitouch.c shows a few supported USB_VENDOR_ID_SYNAPTICS
> >> > (0x06cb) devices, but the 0x7a13 device ID does not appear there as of
> >> > v4.15-rc6. Maybe your kernel is patched to add support for that device.
> >> >
> >> > I guess that i2c_designware appears on the device hierarchy because the
> >> > "smart" USB hub on your system is controlled over the I2C bus.
> >> >
> >> >> when you say that the touchpad driver handles the irq stuff... i am a
> >> >> little confused because i2c-designware-platdrv.c contains this line:
> >> >> irq = platform_get_irq(pdev, 0);
> >> >
> >> > Do you have an indication that irq 17 belongs to i2c-designware?
> >> >
> >> > The i2c-designware driver uses an interrupt to handle its hardware buffer, and
> >> > to receive transactions status. I2C is a slow protocol, so most controller
> >> > implementations are asynchronous. If this irq misbehaves, then there is most
> >> > likely a problem with the i2c-designware driver.
> >> >
> >> >> i would assume that hid_multitouch would contain irq related code if i am
> >> >> understanding you correctly?
> >> >> am i misunderstanding?
> >> >
> >> > Since hid-multitouch is a USB driver, the irq handle itself is in the USB bus
> >> > driver.
> >>
> >> uname -rv:
> >> 4.15.0-rc5-00248-gf39d7d78b70e #2 SMP Sun Dec 31 18:17:25 CST 2017
> >>
> >> the git commit i am at is f39d7d78b70e0f39facb1e4fab77ad3df5c52a35
> >>
> >> > Do you have an indication that irq 17 belongs to i2c-designware?
> >> from /proc/interrupts:
> >> 17: 47105193 0 0 0 0
> >> 0 0 0 IR-IO-APIC 17-fasteoi idma64.1,
> >> i2c_designware.1
> >>
> >> if i rmmod idma64, the behavior remains the same, so im assuming this
> >> is all i2c_designware thats causing this problem.
> >>
> >> > then there is most likely a problem with the i2c-designware driver.
> >> do you have any idea what the problem might be? i know i should look
> >> in the code for i2c-designware, but why would a touchpad be shooting
> >> off interrupts like that with my finger simply resting on it?
> >
> > It is not the touchpad that shoots the interrupts. It is the i2c-designware
> > hardware module.
> >
> >> i have a theory, could you tell me if this makes sense?
> >> perhaps the touchpad hardware is configured with a very high
> >> sensitivity. so in theory if the sensitivity was decreased, then
> >> resting my finger would not trigger interrupts until i move it a
> >> larger amount?
> >
> > I don't think that the touchpad sensitivity has anything to do with this
> > interrupts storm.
> >
> > There is another possibility. The USB hub controller driver might be shooting
> > i2c transactions at the USB hub. In this case the bug is not in the i2c master
> > driver.
> >
> > Can you identify the driver that initiates the i2c transactions?
> > The USB hub controller driver might be shooting
> > i2c transactions at the USB hub. In this case the bug is not in the i2c master
> > driver.
> im still rather confused about the relationship between the i2c bus
> and the usb bus.
> from what i understand, you have the i2c bus, which has a touchpad,
> and the touchpad sends i2c transactions
> when an event has occured, e.g. i moved my finger on the touchpad...
> is that correct?
No. On the I2C bus only the master initiates transactions. Slaves can only
respond to master queries. But there might be a bug in the USB hub driver that
results in a flood of I2C transactions.
> > Can you identify the driver that initiates the i2c transactions?
> i would be more than happy to identify that driver if you could point
> me in the right direction to figure that out
You can add a rate limited dump_stack() call in i2c_dw_xfer(). This would tell
you where transactions are coming from, and whether their rate is too high.
baruch
--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
^ permalink raw reply
* Re: [PATCH] Revert "Input: trackpoint - add new trackpoint firmware ID"
From: Dmitry Torokhov @ 2018-01-05 0:56 UTC (permalink / raw)
To: Aaron Ma; +Cc: Greg KH, Sebastian Schmidt, linux-input
In-Reply-To: <ac087ae7-54d4-d038-daa7-048c87368d17@canonical.com>
Hi Aaron,
On Tue, Jan 02, 2018 at 09:57:55PM +0800, Aaron Ma wrote:
> No, it is not a regression of this commit.
>
> ThinkPad X1 Yoga 2nd:
> trackpoint (ID: 01)
>
> ThinkPad X1 Yoga 3rd:
> trackpoint (ID: 03)
>
> Both laptop's trackpoints have the same behavior.
> Writing "speed" of sysfs is failed.
>
> Override the ID and force loading drivers/input/mouse/elantech.c,
> it causes too many failure and trackpoint stops work.
Right, because it does not support Elantech *touchpad* protocol, that is
not a surprise.
>
> The ID of "2.4.18 READ SECONDARY ID (x"E1")" in TrackPoint specification
> does not indicate any other vendors but only trackpoint.
Exactly. If ID does not match, it is not an IBM trackpoint device.
> Elantech uses 0x03e9.
> ALPS uses 0x00e6/0x00e7/0x00ec.
>
> Maybe the windows tool's is wrong like Linux driver before.
I am not sure what you mean by that.
Anyway, I played with my Carbons a bit, and it seems that the patch
should indeed be reverted. I believe that neither the Elantech nor ALPS
trackpoints support the IBM trackpoint protocol; none of the extended
features (sensitivity, inertia, etc) work when we register them as
TTPS/2 devices. They should continue to be registered as "Generic PS/2"
as that's that they support.
I understand that you want scroll mode working with trackpoints, but
forcing them to pretend that they are TTPS/2 devices is not the proper
way of doing that. Write udev rules that would set
ID_INPUT_POINTINGSTICK property on all input devices connected to a
pass-through serio ports on LENOVO devices, and you should be set (just
make sure you cover both PS/2 pass-through and RMI pass-through
options).
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] driver: input :touchscreen :Modify Raydium Firmware update input file
From: Dmitry Torokhov @ 2018-01-05 5:53 UTC (permalink / raw)
To: jeffrey.lin
Cc: groeck, keith.tzeng, Katherine.Hsieh, bleung, jeffrey.lin, KP.li,
albert.shieh, calvin.tseng, linux-kernel, linux-input
In-Reply-To: <20171221135122.345-1-jeffrey.lin@raydium.corp-partner.google.com>
Hi Jeffrey,
On Thu, Dec 21, 2017 at 09:51:22PM +0800, jeffrey.lin wrote:
> Modify update firmware to accept alternative file name
>
> Signed-off-by: jeffrey.lin <jeffrey.lin@rad-ic.com>
> ---
> drivers/input/touchscreen/raydium_i2c_ts.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/touchscreen/raydium_i2c_ts.c b/drivers/input/touchscreen/raydium_i2c_ts.c
> index a99fb5cac5a0..439d43c3519c 100644
> --- a/drivers/input/touchscreen/raydium_i2c_ts.c
> +++ b/drivers/input/touchscreen/raydium_i2c_ts.c
> @@ -130,6 +130,7 @@ struct raydium_data {
> struct gpio_desc *reset_gpio;
>
> struct raydium_info info;
> + char fw_file[64];
You do not really need to keep the firmware name in driver data, just
use a temporary in raydium_i2c_fw_update().
>
> struct mutex sysfs_mutex;
>
> @@ -752,12 +753,16 @@ static int raydium_i2c_fw_update(struct raydium_data *ts)
> {
> struct i2c_client *client = ts->client;
> const struct firmware *fw = NULL;
> - const char *fw_file = "raydium.fw";
> int error;
>
> - error = request_firmware(&fw, fw_file, &client->dev);
> + /* Firmware name */
> + snprintf(ts->fw_file, sizeof(ts->fw_file),
> + "raydium_%x.fw", ts->info.hw_ver);
hw_ver is LE32, you need to convert it to CPU endianness before using.
Also it would be better if we used the same encoding for the hardware
version as the one that we use when we output it in sysfs. It makes
userspace life a bit easier I think.
How about the version of the patch below?
Thanks.
--
Dmitry
Input: raydium_i2c_ts - include hardware version in firmware name
From: Jeffrey Lin <jeffrey.lin@rad-ic.com>
Add hardware version to the firmware file name to handle scenarios where
single system image supports variety of devices.
Signed-off-by: Jeffrey Lin <jeffrey.lin@rad-ic.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/touchscreen/raydium_i2c_ts.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/input/touchscreen/raydium_i2c_ts.c b/drivers/input/touchscreen/raydium_i2c_ts.c
index 100538d64fff..d1c09e6a2cb6 100644
--- a/drivers/input/touchscreen/raydium_i2c_ts.c
+++ b/drivers/input/touchscreen/raydium_i2c_ts.c
@@ -752,13 +752,20 @@ static int raydium_i2c_fw_update(struct raydium_data *ts)
{
struct i2c_client *client = ts->client;
const struct firmware *fw = NULL;
- const char *fw_file = "raydium.fw";
+ char *fw_file;
int error;
+ fw_file = kasprintf(GFP_KERNEL, "raydium_%#04x.fw",
+ le32_to_cpu(ts->info.hw_ver));
+ if (!fw_file)
+ return -ENOMEM;
+
+ dev_dbg(&client->dev, "firmware name: %s\n", fw_file);
+
error = request_firmware(&fw, fw_file, &client->dev);
if (error) {
dev_err(&client->dev, "Unable to open firmware %s\n", fw_file);
- return error;
+ goto out_free_fw_file;
}
disable_irq(client->irq);
@@ -787,6 +794,9 @@ static int raydium_i2c_fw_update(struct raydium_data *ts)
release_firmware(fw);
+out_free_fw_file:
+ kfree(fw_file);
+
return error;
}
^ permalink raw reply related
* Re: [PATCH] Input: stmfts,s6sy671 - add SPDX identifier
From: Andi Shyti @ 2018-01-05 9:57 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, Andi Shyti
In-Reply-To: <20171212074149.7844-1-andi.shyti@samsung.com>
Hi Dmitry,
this is a kind ping, would you also mind giving me a feedback to
all the previous patches I sent?
Thanks,
Andi
On Tue, Dec 12, 2017 at 04:41:49PM +0900, Andi Shyti wrote:
> Replace the original license statement with the SPDX identifier.
>
> Update also the copyright owner adding myself as co-owner of the
> copyright.
>
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> ---
> drivers/input/touchscreen/s6sy761.c | 15 +++++----------
> drivers/input/touchscreen/stmfts.c | 15 +++++----------
> 2 files changed, 10 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/input/touchscreen/s6sy761.c b/drivers/input/touchscreen/s6sy761.c
> index 26b1cb8a88ec..675efa93d444 100644
> --- a/drivers/input/touchscreen/s6sy761.c
> +++ b/drivers/input/touchscreen/s6sy761.c
> @@ -1,13 +1,8 @@
> -/*
> - * Copyright (c) 2017 Samsung Electronics Co., Ltd.
> - * Author: Andi Shyti <andi.shyti@samsung.com>
> - *
> - * 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.
> - *
> - * Samsung S6SY761 Touchscreen device driver
> - */
> +// SPDX-License-Identifier: GPL-2.0
> +// Samsung S6SY761 Touchscreen device driver
> +//
> +// Copyright (c) 2017 Samsung Electronics Co., Ltd.
> +// Copyright (c) 2017 Andi Shyti <andi.shyti@samsung.com>
>
> #include <asm/unaligned.h>
> #include <linux/delay.h>
> diff --git a/drivers/input/touchscreen/stmfts.c b/drivers/input/touchscreen/stmfts.c
> index c12d01899939..2a123e20a42e 100644
> --- a/drivers/input/touchscreen/stmfts.c
> +++ b/drivers/input/touchscreen/stmfts.c
> @@ -1,13 +1,8 @@
> -/*
> - * Copyright (c) 2017 Samsung Electronics Co., Ltd.
> - * Author: Andi Shyti <andi.shyti@samsung.com>
> - *
> - * 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.
> - *
> - * STMicroelectronics FTS Touchscreen device driver
> - */
> +// SPDX-License-Identifier: GPL-2.0
> +// STMicroelectronics FTS Touchscreen device driver
> +//
> +// Copyright (c) 2017 Samsung Electronics Co., Ltd.
> +// Copyright (c) 2017 Andi Shyti <andi.shyti@samsung.com>
>
> #include <linux/delay.h>
> #include <linux/i2c.h>
> --
> 2.15.1
>
^ permalink raw reply
* [PATCH 1/2] HID: asus: Add touchpad max x/y and resolution info for the T200TA
From: Hans de Goede @ 2018-01-05 11:09 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires; +Cc: Hans de Goede, linux-input
The Asus T200TA uses the same USB device-id for its keyboard dock as the
T100TA, but the touchpad has a different size and corresponding different
max x/y values.
Add a separate asus_touchpad_info struct for the T200TA and select this
based on the DMI product-name (as we are already doing for the T100HA),
so that we report the correct info to userspace.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/hid/hid-asus.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 6d2894b7d8e7..07525bc99b6a 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -129,6 +129,15 @@ static const struct asus_touchpad_info asus_t100ha_tp = {
.max_contacts = 5,
};
+static const struct asus_touchpad_info asus_t200ta_tp = {
+ .max_x = 3120,
+ .max_y = 1716,
+ .res_x = 30, /* units/mm */
+ .res_y = 28, /* units/mm */
+ .contact_size = 5,
+ .max_contacts = 5,
+};
+
static const struct asus_touchpad_info asus_t100chi_tp = {
.max_x = 2640,
.max_y = 1320,
@@ -617,11 +626,14 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
if (intf->altsetting->desc.bInterfaceNumber == T100_TPAD_INTF) {
drvdata->quirks = QUIRK_SKIP_INPUT_MAPPING;
/*
- * The T100HA uses the same USB-ids as the T100TAF,
- * but has different max_x / max_y values.
+ * The T100HA uses the same USB-ids as the T100TAF and
+ * the T200TA uses the same USB-ids as the T100TA, while
+ * both have different max x/y values as the T100TA[F].
*/
if (dmi_match(DMI_PRODUCT_NAME, "T100HAN"))
drvdata->tp = &asus_t100ha_tp;
+ else if (dmi_match(DMI_PRODUCT_NAME, "T200TA"))
+ drvdata->tp = &asus_t200ta_tp;
else
drvdata->tp = &asus_t100ta_tp;
}
--
2.14.3
^ permalink raw reply related
* [PATCH 2/2] HID: asus: Fix special function keys on T200TA
From: Hans de Goede @ 2018-01-05 11:09 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires; +Cc: Hans de Goede, linux-input
In-Reply-To: <20180105110919.30399-1-hdegoede@redhat.com>
Just like on the T100TA the T200TA HID descriptors for the 0xff32
Asus vendor usage page need a small fixup. But on the T200TA the HID
descriptors are larger because they have descrriptors for one more
(unused) HID report appended.
Extend the T100TA descriptor fixup to also check for the T200TA's
descriptors size.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/hid/hid-asus.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 07525bc99b6a..88b9703318e4 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -715,9 +715,10 @@ static __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc,
hid_info(hdev, "Fixing up Asus notebook report descriptor\n");
rdesc[55] = 0xdd;
}
- /* For the T100TA keyboard dock */
+ /* For the T100TA/T200TA keyboard dock */
if (drvdata->quirks & QUIRK_T100_KEYBOARD &&
- *rsize == 76 && rdesc[73] == 0x81 && rdesc[74] == 0x01) {
+ (*rsize == 76 || *rsize == 101) &&
+ rdesc[73] == 0x81 && rdesc[74] == 0x01) {
hid_info(hdev, "Fixing up Asus T100 keyb report descriptor\n");
rdesc[74] &= ~HID_MAIN_ITEM_CONSTANT;
}
--
2.14.3
^ permalink raw reply related
* RE: [PATCH] driver: input :touchscreen :Modify Raydium Firmware update input file
From: Jeffrey Lin (林義章) @ 2018-01-05 12:12 UTC (permalink / raw)
To: Dmitry Torokhov, jeffrey.lin
Cc: groeck@chromium.org, keith.tzeng@quantatw.com,
Katherine.Hsieh@quantatw.com, bleung@google.com,
KP Li (李昆倍),
Albert Shieh (謝欣瑋),
Calvin Tseng (曾國宗),
linux-kernel@vger.kernel.org, linux-input@vger.kernel.org,
Kevin Chiu (邱俊睿),
Jacky Lin (林冠明)
In-Reply-To: <20180105055338.cogtaty5p5r5gglt@dtor-ws>
Dmitry:
I've verified pass the patch you had modified. Many thanks.
Best Regards
----------------------------------------------------------------------
Jeffrey Lin, 林義章
Raydium Semiconductor Corporation, 瑞鼎科技
Tel:(03)666-1818 Ext.4163
Fax:(03)666-1919
-----Original Message-----
From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
Sent: Friday, January 05, 2018 1:54 PM
To: jeffrey.lin
Cc: groeck@chromium.org; keith.tzeng@quantatw.com; Katherine.Hsieh@quantatw.com; bleung@google.com; Jeffrey Lin (林義章); KP Li (李昆倍); Albert Shieh (謝欣瑋); Calvin Tseng (曾國宗); linux-kernel@vger.kernel.org; linux-input@vger.kernel.org
Subject: Re: [PATCH] driver: input :touchscreen :Modify Raydium Firmware update input file
Hi Jeffrey,
On Thu, Dec 21, 2017 at 09:51:22PM +0800, jeffrey.lin wrote:
> Modify update firmware to accept alternative file name
>
> Signed-off-by: jeffrey.lin <jeffrey.lin@rad-ic.com>
> ---
> drivers/input/touchscreen/raydium_i2c_ts.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/touchscreen/raydium_i2c_ts.c b/drivers/input/touchscreen/raydium_i2c_ts.c
> index a99fb5cac5a0..439d43c3519c 100644
> --- a/drivers/input/touchscreen/raydium_i2c_ts.c
> +++ b/drivers/input/touchscreen/raydium_i2c_ts.c
> @@ -130,6 +130,7 @@ struct raydium_data {
> struct gpio_desc *reset_gpio;
>
> struct raydium_info info;
> + char fw_file[64];
You do not really need to keep the firmware name in driver data, just
use a temporary in raydium_i2c_fw_update().
>
> struct mutex sysfs_mutex;
>
> @@ -752,12 +753,16 @@ static int raydium_i2c_fw_update(struct raydium_data *ts)
> {
> struct i2c_client *client = ts->client;
> const struct firmware *fw = NULL;
> - const char *fw_file = "raydium.fw";
> int error;
>
> - error = request_firmware(&fw, fw_file, &client->dev);
> + /* Firmware name */
> + snprintf(ts->fw_file, sizeof(ts->fw_file),
> + "raydium_%x.fw", ts->info.hw_ver);
hw_ver is LE32, you need to convert it to CPU endianness before using.
Also it would be better if we used the same encoding for the hardware
version as the one that we use when we output it in sysfs. It makes
userspace life a bit easier I think.
How about the version of the patch below?
Thanks.
--
Dmitry
Input: raydium_i2c_ts - include hardware version in firmware name
From: Jeffrey Lin <jeffrey.lin@rad-ic.com>
Add hardware version to the firmware file name to handle scenarios where
single system image supports variety of devices.
Signed-off-by: Jeffrey Lin <jeffrey.lin@rad-ic.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/touchscreen/raydium_i2c_ts.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/input/touchscreen/raydium_i2c_ts.c b/drivers/input/touchscreen/raydium_i2c_ts.c
index 100538d64fff..d1c09e6a2cb6 100644
--- a/drivers/input/touchscreen/raydium_i2c_ts.c
+++ b/drivers/input/touchscreen/raydium_i2c_ts.c
@@ -752,13 +752,20 @@ static int raydium_i2c_fw_update(struct raydium_data *ts)
{
struct i2c_client *client = ts->client;
const struct firmware *fw = NULL;
- const char *fw_file = "raydium.fw";
+ char *fw_file;
int error;
+ fw_file = kasprintf(GFP_KERNEL, "raydium_%#04x.fw",
+ le32_to_cpu(ts->info.hw_ver));
+ if (!fw_file)
+ return -ENOMEM;
+
+ dev_dbg(&client->dev, "firmware name: %s\n", fw_file);
+
error = request_firmware(&fw, fw_file, &client->dev);
if (error) {
dev_err(&client->dev, "Unable to open firmware %s\n", fw_file);
- return error;
+ goto out_free_fw_file;
}
disable_irq(client->irq);
@@ -787,6 +794,9 @@ static int raydium_i2c_fw_update(struct raydium_data *ts)
release_firmware(fw);
+out_free_fw_file:
+ kfree(fw_file);
+
return error;
}
CONFIDENTIALITY AND PROPRIETARY REMINDER:
This message and any attachment may contain confidential information. All rights including intellectual property rights arising out of this correspondence shall belong to Raydium Semiconductor Corp. Any unauthorized disclosure, forwarding, using, modifying, spreading, publishing or copying this email and the content is illegal and strictly prohibited.
If you are not the intended recipient, please notify the sender by replying to this message and delete all copies of it from your system. Thank you for cooperation.
^ permalink raw reply related
* Re: driver: input :touchscreen :Modify Raydium Firmware update input file
From: jeffrey.lin @ 2018-01-05 12:43 UTC (permalink / raw)
To: dmitry.torokhov, groeck, keith.tzeng, Katherine.Hsieh, bleung
Cc: jeffrey.lin, KP.li, albert.shieh, calvin.tseng, linux-kernel,
linux-input
I've tested okay by the patch you suggested.
^ permalink raw reply
* Re: [PATCH] Revert "Input: trackpoint - add new trackpoint firmware ID"
From: Aaron Ma @ 2018-01-05 13:29 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Greg KH, Sebastian Schmidt, linux-input
In-Reply-To: <20180105005659.j2x56opkq74wm7v2@dtor-ws>
Hi Dmitry:
Got the official info from Lenovo:
Lenovo introduced new TrackPoint compatible sticks ( ELAN/Alps/NXP
sticks) from 2016.
These new devices only support the minimum commands described in the
spec, which has been used in the current Windows driver.
Legacy TrackPoint: 0101 – 0E01
ALPS: 0102 – FF02
ELAN:0103 – FF03
NXP: 0104 – FF04
2.4.18 READ SECONDARY ID (x"E1")
This command will read the secondary device ID of the pointing device (2
bytes). The least significant byte is sent first. For the first byte,
the legacy TrackPoint controller from IBM will always return x"01", the
pointing stick from ALPS will always return x"02", the pointing stick
from Elan will always return x"03”, and the pointing stick from NXP will
always return 0x”04". And a second byte which denotes a specific set of
functional specifications. Differing ROM versions are used to denote
changes within a given functional set.
The new devices (include Legacy ID:01) will not support the sysfs like
speed.
So it is not right to revert the commit, it is about to add another 0x04
ID in it.
Old sysfs could be stayed for old legacy device ID:01 or removed.
Aaron
^ permalink raw reply
* Re: [PATCH] Revert "Input: trackpoint - add new trackpoint firmware ID"
From: Dmitry Torokhov @ 2018-01-05 16:23 UTC (permalink / raw)
To: Aaron Ma; +Cc: Greg KH, Sebastian Schmidt, linux-input
In-Reply-To: <598ac534-ed9b-1377-70c5-eec7b7521755@canonical.com>
Hi Aaron,
On Fri, Jan 05, 2018 at 09:29:26PM +0800, Aaron Ma wrote:
> Hi Dmitry:
>
> Got the official info from Lenovo:
> Lenovo introduced new TrackPoint compatible sticks ( ELAN/Alps/NXP
> sticks) from 2016.
> These new devices only support the minimum commands described in the
> spec, which has been used in the current Windows driver.
What is the exact list of the commands supported by each variant?
>
> Legacy TrackPoint: 0101 – 0E01
> ALPS: 0102 – FF02
> ELAN:0103 – FF03
> NXP: 0104 – FF04
>
> 2.4.18 READ SECONDARY ID (x"E1")
> This command will read the secondary device ID of the pointing device (2
> bytes). The least significant byte is sent first. For the first byte,
> the legacy TrackPoint controller from IBM will always return x"01", the
> pointing stick from ALPS will always return x"02", the pointing stick
> from Elan will always return x"03”, and the pointing stick from NXP will
> always return 0x”04". And a second byte which denotes a specific set of
> functional specifications. Differing ROM versions are used to denote
> changes within a given functional set.
Can you/Lenovo share the updated spec?
>
> The new devices (include Legacy ID:01) will not support the sysfs like
> speed.
>
> So it is not right to revert the commit, it is about to add another 0x04
> ID in it.
>
> Old sysfs could be stayed for old legacy device ID:01 or removed.
No, because there are devices that have trackpoints properly
implementing the protocol, before Lenovo started their "innovation".
Do we have any way to distinguish between properly implemented
trackpoints and Lenovo "improved" ones? I played with gen3 Carbon, and
while it does not error out on "speed" attribute, unlike gen5, it still
has no visible effects. Additionally, the "press to select"
functionality seems to be disabled, and trying to enable it via sysfs
results in register content being reverted to the original "disabled"
setting in a second or two. Setting to swap X and Y axes does not work
either, not sure about other bits of that control register.
"sensitivity" does work though, again unlike my gen5.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: stmfts,s6sy671 - add SPDX identifier
From: Dmitry Torokhov @ 2018-01-05 16:49 UTC (permalink / raw)
To: Andi Shyti; +Cc: linux-input, linux-kernel, Andi Shyti
In-Reply-To: <20180105095715.GA6679@gangnam.samsung>
Hi Andi,
On Fri, Jan 05, 2018 at 06:57:15PM +0900, Andi Shyti wrote:
> Hi Dmitry,
>
> this is a kind ping, would you also mind giving me a feedback to
Yes, sorry. Could you please split the patch for each driver
individually? Also, until we have an update to the CodingStyle doc
mandating the C++ style comments, I'd prefer keeping the original style
of comments. So // for the SPDX line and /* */ for the rest.
Thanks!
> all the previous patches I sent?
Could you tell me what reviews I owe you? My mailbox keeps growing :(
>
> Thanks,
> Andi
>
> On Tue, Dec 12, 2017 at 04:41:49PM +0900, Andi Shyti wrote:
> > Replace the original license statement with the SPDX identifier.
> >
> > Update also the copyright owner adding myself as co-owner of the
> > copyright.
> >
> > Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> > ---
> > drivers/input/touchscreen/s6sy761.c | 15 +++++----------
> > drivers/input/touchscreen/stmfts.c | 15 +++++----------
> > 2 files changed, 10 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/input/touchscreen/s6sy761.c b/drivers/input/touchscreen/s6sy761.c
> > index 26b1cb8a88ec..675efa93d444 100644
> > --- a/drivers/input/touchscreen/s6sy761.c
> > +++ b/drivers/input/touchscreen/s6sy761.c
> > @@ -1,13 +1,8 @@
> > -/*
> > - * Copyright (c) 2017 Samsung Electronics Co., Ltd.
> > - * Author: Andi Shyti <andi.shyti@samsung.com>
> > - *
> > - * 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.
> > - *
> > - * Samsung S6SY761 Touchscreen device driver
> > - */
> > +// SPDX-License-Identifier: GPL-2.0
> > +// Samsung S6SY761 Touchscreen device driver
> > +//
> > +// Copyright (c) 2017 Samsung Electronics Co., Ltd.
> > +// Copyright (c) 2017 Andi Shyti <andi.shyti@samsung.com>
> >
> > #include <asm/unaligned.h>
> > #include <linux/delay.h>
> > diff --git a/drivers/input/touchscreen/stmfts.c b/drivers/input/touchscreen/stmfts.c
> > index c12d01899939..2a123e20a42e 100644
> > --- a/drivers/input/touchscreen/stmfts.c
> > +++ b/drivers/input/touchscreen/stmfts.c
> > @@ -1,13 +1,8 @@
> > -/*
> > - * Copyright (c) 2017 Samsung Electronics Co., Ltd.
> > - * Author: Andi Shyti <andi.shyti@samsung.com>
> > - *
> > - * 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.
> > - *
> > - * STMicroelectronics FTS Touchscreen device driver
> > - */
> > +// SPDX-License-Identifier: GPL-2.0
> > +// STMicroelectronics FTS Touchscreen device driver
> > +//
> > +// Copyright (c) 2017 Samsung Electronics Co., Ltd.
> > +// Copyright (c) 2017 Andi Shyti <andi.shyti@samsung.com>
> >
> > #include <linux/delay.h>
> > #include <linux/i2c.h>
> > --
> > 2.15.1
> >
--
Dmitry
^ permalink raw reply
* Re: [PATCH 2/2] HID: asus: Fix special function keys on T200TA
From: Benjamin Tissoires @ 2018-01-05 17:00 UTC (permalink / raw)
To: Hans de Goede; +Cc: Jiri Kosina, linux-input
In-Reply-To: <20180105110919.30399-2-hdegoede@redhat.com>
On Fri, Jan 5, 2018 at 12:09 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Just like on the T100TA the T200TA HID descriptors for the 0xff32
> Asus vendor usage page need a small fixup. But on the T200TA the HID
> descriptors are larger because they have descrriptors for one more
> (unused) HID report appended.
>
> Extend the T100TA descriptor fixup to also check for the T200TA's
> descriptors size.
Sigh, both are:
Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cheers,
Benjamin
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/hid/hid-asus.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 07525bc99b6a..88b9703318e4 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -715,9 +715,10 @@ static __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> hid_info(hdev, "Fixing up Asus notebook report descriptor\n");
> rdesc[55] = 0xdd;
> }
> - /* For the T100TA keyboard dock */
> + /* For the T100TA/T200TA keyboard dock */
> if (drvdata->quirks & QUIRK_T100_KEYBOARD &&
> - *rsize == 76 && rdesc[73] == 0x81 && rdesc[74] == 0x01) {
> + (*rsize == 76 || *rsize == 101) &&
> + rdesc[73] == 0x81 && rdesc[74] == 0x01) {
> hid_info(hdev, "Fixing up Asus T100 keyb report descriptor\n");
> rdesc[74] &= ~HID_MAIN_ITEM_CONSTANT;
> }
> --
> 2.14.3
>
^ permalink raw reply
* [git pull] Input updates for v4.15-rc6
From: Dmitry Torokhov @ 2018-01-05 23:53 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-input
Hi Linus,
Please pull from:
git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus
to receive updates for the input subsystem. Just a few driver fixups,
nothing exciting.
Changelog:
---------
Aaron Ma (1):
Input: elantech - add new icbody type 15
Anthony Kim (1):
Input: hideep - fix compile error due to missing include file
Dmitry Torokhov (1):
Input: elants_i2c - do not clobber interrupt trigger on x86
Oleksandr Andrushchenko (1):
Input: xen-kbdfront - do not advertise multi-touch pressure support
Olof Johansson (1):
Input: joystick/analog - riscv has get_cycles()
Zhen Lei (1):
Input: ims-pcu - fix typo in the error message
Diffstat:
--------
drivers/input/joystick/analog.c | 2 +-
drivers/input/misc/ims-pcu.c | 2 +-
drivers/input/misc/xen-kbdfront.c | 2 --
drivers/input/mouse/elantech.c | 2 +-
drivers/input/touchscreen/elants_i2c.c | 10 +++++++---
drivers/input/touchscreen/hideep.c | 3 +--
6 files changed, 11 insertions(+), 10 deletions(-)
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [RFP] iio: Support of gesture sensor as a standard IIO sensor
From: Alan Cox @ 2018-01-06 0:20 UTC (permalink / raw)
To: Pandruvada, Srinivas
Cc: linux-iio@vger.kernel.org, jic23@kernel.org,
linux-kernel@vger.kernel.org, linux-input
In-Reply-To: <1515197240.26317.65.camel@intel.com>
> >From an IIO sensor point of view A Gesture sensor:
> Outputs
> A pre defined activity type
> WAKE
> TILT
> GLANCE
> PICK_UP
> &
> more
>
> A user defined activity type as "string"
>
> Inputs
> A raw binary cdev interface to download templates/patterns
>
>
> I want to gather more opinions before submitting a RFC patch.
The only question I have is should it appear under IIO or should it be an
input event interface. It feels to me more like an input device in that in
this case while it's not keys or joystick it is still 'please do X'. That
might also make it much easier (in the non-Android space in particular)
to bind these activities to actions in things like web browsers.
(+ linux-input)
Alan
^ permalink raw reply
* Hallo mein lieber Freund,
From: 60629177 @ 2018-01-06 6:14 UTC (permalink / raw)
To: Recipients
Hallo mein lieber Freund,
Ich schreibe dir diese Post mit schweren Tränen In meinen Augen und großem Kummer in meinem Herzen, Mein Name ist Vera Hollin Kvan, und ich kontaktiere dich aus meinem Land Indien Ich möchte dir das sagen, weil ich keine habe Eine andere Möglichkeit, als Ihnen zu sagen, wie ich berührt war, um Sie zu öffnen, heiratete ich mit Herrn Hollin Kvan, der für neun Jahre mit der tunesischen Botschaft in Madrid Spanien arbeitete, bevor er im Jahr 2005 starb. Wir waren elf Jahre ohne verheiratet Kind.
Er starb nach einer kurzen Krankheit, die nur fünf Tage dauerte. Seit seinem Tod entschied ich mich, nicht wieder zu heiraten. Als mein verstorbener Mann noch am Leben war, hinterlegte er die Summe von $ 4.850.000,00USD (Vier Millionen achthundertundfünfzigtausend Dollar) in einer Bank hier in Indien Neu-Delhi, der Hauptstadt Indiens, gegenwärtig dieses Geld ist immer noch in der Bank.
Er stellte dieses Geld für den Export von Gold aus dem Minenfactory in Madrid Spanien zur Verfügung. In letzter Zeit sagte mir mein Doktor, dass ich wegen der Krebserkrankung für die Dauer von sieben Monaten nicht überleben würde. Derjenige, der mich am meisten stört, ist meine Schlaganfall-Krankheit. Nachdem ich meinen Zustand gekannt habe, habe ich beschlossen, dir dieses Geld auszuhändigen, um auf die weniger privilegierten Menschen aufzupassen, du wirst dieses Geld auf die Art und Weise benutzen, wie ich es hier unterrichten werde.
Ich möchte, dass Sie 30 Prozent des gesamten Geldes für Ihren persönlichen Gebrauch aufwenden, während 70% des Geldes an wohltätige Zwecke gehen, Menschen auf der Straße und dem Waisenhaus helfen. Ich bin als Waise aufgewachsen und habe keinen Körper als mein Familienmitglied, nur um zu beenden, dass das Haus Gottes auch erhalten wird. Mache dies, damit Gott meine Sünden vergibt und meine Seele akzeptiert, weil diese Krankheiten mich so sehr leiden. Sobald ich Ihre Antwort erhalten habe, werde ich Ihnen den Kontakt der Bank hier in Delhi Indien geben und ich werde auch den Bankmanager beauftragen, Ihnen einen Vollmachtenbrief auszustellen, der Ihnen den gegenwärtigen Begünstigten des Geldes in der Bank beweisen wird, wenn Sie versichern mir, dass Sie entsprechend handeln werden, wie ich hier angegeben habe.
Ich hoffe auf Ihre Antwort.
Von Vera Hollin kVan
^ permalink raw reply
* [PATCH] input: multi-touch fix for ALPS touchpads ("SS4 plus" variant)
From: Nir Perry @ 2018-01-06 11:54 UTC (permalink / raw)
To: Masaki Ota, Dmitry Torokhov, Pali Rohár; +Cc: linux-kernel, linux-input
Hi all,
I think a minor "typo" bug was accidentally introduced to ALPS
touchpad driver by a previous bug-fix (commit
4a646580f793d19717f7e034c8d473b509c27d49, "Input: ALPS - fix
two-finger scroll breakage in right side on ALPS touchpad").
It breaks how multi-touch events are decoded on some ALPS touchpads,
so for example tapping with three-fingers can no longer be used to
emulate middle-mouse-button (the kernel doesn't recognize this as the
proper event, and doesn't report it correctly to userspace).
This affects touchpads that use SS4 "plus" protocol variant, like
those found on Dell E7270 & E7470 laptops (tested on E7270).
The cause of the problem
------------------------------------------
First, probably due to a typo, the code in alps_decode_ss4_v2() for
case SS4_PACKET_ID_MULTI used inconsistent indices to "f->mt[]". You
can see 0 & 1 are used for the "if" part but 2 & 3 are used for the
"else" part, which I believe is a typo.
Second, in the previous patch, new macros were introduced to decode X
coordinates specific to the SS4 "plus" variant, but the macro to
define the maximum X value wasn't changed accordingly. The macros to
decode X values for "plus" variant are effectively shifted right by 1
bit, but the max wasn't shifted too. This causes the driver to
incorrectly handle "no data" cases, which also interfered with how
multi-touch was handled. To fix it - I created new SS4 "plus" macros
for the max value - SS4_PLUS_MFPACKET_NO_AX &
SS4_PLUS_MFPACKET_NO_AX_BL. To make the change a little more readable,
I moved also the Y-max lines so they are closer to the X-max lines.
To get three-finger tap to work both changes are required.
The included patch was generated against the mainline tree today, but
was also tested against the 4.14 kernel branch. I've included in this
e-mail the people involved with the old patch from August, plus Pali
Rohár who is listed as the ALPS PS/2 touchpad driver reviewer (in the
maintainers file).
Fixes: 4a646580f793d19717f7e034c8d473b509c27d49 ("Input: ALPS - fix
two-finger scroll breakage in right side on ALPS touchpad")
Regards,
Nir
Signed-off-by: Nir Perry <nirperry@gmail.com>
diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 579b899..dbe57da 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -1250,29 +1250,32 @@ static int alps_decode_ss4_v2(struct alps_fields *f,
case SS4_PACKET_ID_MULTI:
if (priv->flags & ALPS_BUTTONPAD) {
if (IS_SS4PLUS_DEV(priv->dev_id)) {
- f->mt[0].x = SS4_PLUS_BTL_MF_X_V2(p, 0);
- f->mt[1].x = SS4_PLUS_BTL_MF_X_V2(p, 1);
+ f->mt[2].x = SS4_PLUS_BTL_MF_X_V2(p, 0);
+ f->mt[3].x = SS4_PLUS_BTL_MF_X_V2(p, 1);
+ no_data_x = SS4_PLUS_MFPACKET_NO_AX_BL;
} else {
f->mt[2].x = SS4_BTL_MF_X_V2(p, 0);
f->mt[3].x = SS4_BTL_MF_X_V2(p, 1);
+ no_data_x = SS4_MFPACKET_NO_AX_BL;
}
+ no_data_y = SS4_MFPACKET_NO_AY_BL;
f->mt[2].y = SS4_BTL_MF_Y_V2(p, 0);
f->mt[3].y = SS4_BTL_MF_Y_V2(p, 1);
- no_data_x = SS4_MFPACKET_NO_AX_BL;
- no_data_y = SS4_MFPACKET_NO_AY_BL;
} else {
if (IS_SS4PLUS_DEV(priv->dev_id)) {
- f->mt[0].x = SS4_PLUS_STD_MF_X_V2(p, 0);
- f->mt[1].x = SS4_PLUS_STD_MF_X_V2(p, 1);
+ f->mt[2].x = SS4_PLUS_STD_MF_X_V2(p, 0);
+ f->mt[3].x = SS4_PLUS_STD_MF_X_V2(p, 1);
+ no_data_x = SS4_PLUS_MFPACKET_NO_AX;
} else {
- f->mt[0].x = SS4_STD_MF_X_V2(p, 0);
- f->mt[1].x = SS4_STD_MF_X_V2(p, 1);
+ f->mt[2].x = SS4_STD_MF_X_V2(p, 0);
+ f->mt[3].x = SS4_STD_MF_X_V2(p, 1);
+ no_data_x = SS4_MFPACKET_NO_AX;
}
+ no_data_y = SS4_MFPACKET_NO_AY;
+
f->mt[2].y = SS4_STD_MF_Y_V2(p, 0);
f->mt[3].y = SS4_STD_MF_Y_V2(p, 1);
- no_data_x = SS4_MFPACKET_NO_AX;
- no_data_y = SS4_MFPACKET_NO_AY;
}
f->first_mp = 0;
diff --git a/drivers/input/mouse/alps.h b/drivers/input/mouse/alps.h
index c80a7c7..3dfae83 100644
--- a/drivers/input/mouse/alps.h
+++ b/drivers/input/mouse/alps.h
@@ -145,6 +145,8 @@ enum SS4_PACKET_ID {
#define SS4_MFPACKET_NO_AY 4080 /* Y-Coordinate value */
#define SS4_MFPACKET_NO_AX_BL 8176 /* Buttonless X-Coordinate value */
#define SS4_MFPACKET_NO_AY_BL 4088 /* Buttonless Y-Coordinate value */
+#define SS4_PLUS_MFPACKET_NO_AX 4080 /* SS4 PLUS, x */
+#define SS4_PLUS_MFPACKET_NO_AX_BL 4088 /* Buttonless SS4 PLUS, x */
/*
* enum V7_PACKET_ID - defines the packet type for V7
^ permalink raw reply related
* Re: [RFP] iio: Support of gesture sensor as a standard IIO sensor
From: Jonathan Cameron @ 2018-01-06 13:12 UTC (permalink / raw)
To: Alan Cox
Cc: Pandruvada, Srinivas,
linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-input-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20180106002024.012f44c0@alans-desktop>
On Sat, 6 Jan 2018 00:20:24 +0000
Alan Cox <gnomes-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org> wrote:
> > >From an IIO sensor point of view A Gesture sensor:
> > Outputs
> > A pre defined activity type
> > WAKE
> > TILT
> > GLANCE
> > PICK_UP
> > &
> > more
> >
> > A user defined activity type as "string"
> >
> > Inputs
> > A raw binary cdev interface to download templates/patterns
> >
> >
> > I want to gather more opinions before submitting a RFC patch.
>
> The only question I have is should it appear under IIO or should it be an
> input event interface. It feels to me more like an input device in that in
> this case while it's not keys or joystick it is still 'please do X'. That
> might also make it much easier (in the non-Android space in particular)
> to bind these activities to actions in things like web browsers.
>
I agree that this may well be an option for many of the gestures specifically
metioned (flicks etc and glyphs).
However, there are other obvious uses of this technology such as step detection
or activity classification (running, walking sitting) that so far have fallen
in the scope of IIO as they aren't really things you expect the device to
perform an an action in response to. Another one of those messy corners
that fall through the gaps!
The drivers/iio/accel/mma9553.c does activity detection, but that isn't
really 'events' in the same way as we have here...
So right answer might be a hybrid of an underlying flexible IIO device
and an input front end for when it makes sense.
We probably need to get the in kernel use of IIO events sorted. Non event
stuff has been sorted for years, but this last corner was never of enough
interest to anyone to actually implement it (it's fairly straight forward
to do).
> (+ linux-input)
>
> Alan
^ permalink raw reply
* Re: [PATCH 12/14] iio: adc: at91-sama5d2_adc: support for position and pressure channels
From: Jonathan Cameron @ 2018-01-06 15:05 UTC (permalink / raw)
To: Eugen Hristev
Cc: devicetree, linux-iio, dmitry.torokhov, linux-kernel,
ludovic.desroches, alexandre.belloni, linux-input,
linux-arm-kernel
In-Reply-To: <25045512-f372-3f25-e84f-ed809db9dde3@microchip.com>
On Thu, 4 Jan 2018 17:17:54 +0200
Eugen Hristev <eugen.hristev@microchip.com> wrote:
> On 29.12.2017 19:02, Jonathan Cameron wrote:
> > On Fri, 22 Dec 2017 17:07:19 +0200
> > Eugen Hristev <eugen.hristev@microchip.com> wrote:
> >
> >> The ADC IP supports position and pressure measurements for a touchpad
> >> connected on channels 0,1,2,3 for a 4-wire touchscreen with pressure
> >> measurement support.
> >> Using the inkern API, a driver can request a trigger and read the
> >> channel values from the ADC.
> >> The implementation provides a trigger named "touch" which can be
> >> connected to a consumer driver.
> >> Once a driver connects and attaches a pollfunc to this trigger, the
> >> configure trigger callback is called, and then the ADC driver will
> >> initialize pad measurement.
> >> First step is to enable touchscreen 4wire support and enable
> >> pen detect IRQ.
> >> Once a pen is detected, a periodic trigger is setup to trigger every
> >> 2 ms (e.g.) and sample the resistive touchscreen values. The trigger poll
> >> is called, and the consumer driver is then woke up, and it can read the
> >> respective channels for the values : X, and Y for position and pressure
> >> channel.
> >> Because only one trigger can be active in hardware in the same time,
> >> while touching the pad, the ADC will block any attempt to use the
> >> triggered buffer. Same, conversions using the software trigger are also
> >> impossible (since the periodic trigger is setup).
> >> If some driver wants to attach while the trigger is in use, it will
> >> also fail.
> >> Once the pen is not detected anymore, the trigger is free for use (hardware
> >> or software trigger, with or without DMA).
> >> Channels 0,1,2 and 3 are unavailable if a touchscreen is enabled.
> >>
> >> Some parts of this patch are based on initial original work by
> >> Mohamed Jamsheeth Hajanajubudeen and Bandaru Venkateswara Swamy
> >>
> > OK, so comments inline.
> >
> > What I'm missing currently though is an explanation of why the slightly
> > more standard arrangement of using a callback buffer doesn't work here.
> > The only addition I think you need to do that is to allow a consumer to
> > request a particular trigger. I also think some of the other provisions
> > could be handled using standard features and slightly reducing the flexibility.
> > I don't know for example if it's useful to allow other channels to be
> > read when touch is not in progress or not.
> >
> > So restrictions:
> >
> > 1. Touch screen channels can only be read when touch is enabled.
> > - use the available_scan_masks to control this. Or the callback that lets
> > you do the same dynamically.
> > 2. You need to push these channels to your consumer driver.
> > - register a callback buffer rather than jumping through the hoops to
> > insert your own pollfunc. That will call a function in your
> > consumer, providing the data from the 3 channels directly.
> > 3. You need to make sure it is using the right driver. For that you
> > will I think need a new interface.
> >
> > Various other comments inline. I may well be missing something as this is
> > a fair bit of complex code to read - if so then next version should have
> > a clear cover letter describing why this more standard approach can't be
> > used.
>
> Hello Jonathan and thanks for the review of my patch series,
>
> before starting and working over the required modifications and
> suggestions that you sent me, I want to be a little more explicit about
> the design of my implementation.
> Hope this will clarify some things, and maybe I can as well understand
> better what you have in mind to support this feature set.
>
> Why have I picked a pollfunction: We discussed a while back on the
> mailing list that you do not have an inkern mechanism to expose the
> triggers to other drivers, and that it may be a good idea to have it for
> such kind of actually multi function device, instead of having a MFD
> driver, an ADC driver, and an Input driver, all sharing the same
> register map, the same IRQ , etc, with some kind of synchronization to
> avoid stepping on each other for the hardware resource.
No disagreement with that principle.
> So I considered to expose the trigger by attaching and detaching
> pollfunctions to it. Which is the main thing what we use a trigger for.
Hmm. It's definitely one approach. But we do already have other drivers
where the trigger is controlled by a consumer and to my mind that
is a cleaner approach as it doesn't short cut the equivalent of
doing it from userspace.
drivers/iio/potentiostat/lmp91000.c does something similar though
for a rather different use. You need your consumer interface
to get the handle to the trigger in this case
(the lmp91000 is actually providing the trigger rather than
consuming it).
>
> So, what I had in mind, was to create a consumer driver that will
> request triggers from the IIO device just like other drivers request
> channels (part which is already done in IIO).
> In order to do this I had to somehow wake up the consumer driver when
> new data was available from the touchscreen. So, having the IRQ only in
> the ADC device, and then on Pen detect and No pen detect just start or
> stop the periodic trigger, which needs to be polled. The magic part is
> that the consumer driver has a poll function already attached to this
> trigger, so the poll function is just called every time we have new
> data. The poll function is attached as an irq handler, and then we can
> reuse all the read_raw data by using a scheduled work from the consumer
> driver, to read the channels.
If you had done this via a callback buffer the only difference is that
the pollfunc would have been a standard one pulling the relevant channels
and passing them on down to the buffer interface which could then decide
what to do with them.
> To do this, the ADC registers a special trigger named "touch trigger"
> which is never enabled by the ADC driver. Instead, when a pollfunc is
> attached to it, the attach function will also configure it with enabled
> state.
Whilst it might not make sense to enable it in the touch screen driver
I'm not sure there is strictly any reason to prevent it being so used.
> In the ADC, this means to start the touchscreen functionality. If
> the touch is requested, it will standby and wait for pen detect IRQ.
> Once we have pen detect, we can use a periodic trigger to sample the
> touch data, and poll the "touch" trigger. The consumer driver will wake
> up and schedule a work , that will use the standard read raw interface
> (inkern) that will read three virtual channels (position + pressure).
> They are not actual hardware channels, as the touch information is being
> received on channels 0,1,2,3, but reading these virtual channels will
> read from different registers inside the ADC IP ( x position, y
> position, pressure), do some computations on the data, and feed the
> consumer with the values , hiding the behind the scenes hardware
> specific calculations.
I wouldn't worry about whether they are real channels or not. This
is really similar to a differential ADC (some of those do the differential
digitally). Light sensors often have a number of 'real' channels used
to derive (via hideous non linear calculations) the illuminance as
it's hard to build a light sensor with the same sensitivity as the human
eye. We have worse 'non real' channels as well such as activity channels
on some the accelerometers that report if it thinks you are walking /
running etc.
> After trigger is polled , the ADC will resume normal functionality, and
> the consumer driver will continue to sleep.
So this is where I'm unsure. Do you actually have a usecase where it
makes the sense to read from the ADC only when there is no touch? Any
system doing that has an obvious denial of service attack - touch the
screen.
> We need to have a periodic trigger to sample the data because the actual
> analog to digital conversion inside the IP block needs to be triggered.
> The touchscreen data measurements cannot happen in hardware without
> being triggered. If I try with a hrtimer to get a periodic IRQ to just
> read the data, it will never be ready. The datasheet states that the
> touchscreen measurements "will be attached to the conversion sequence".
> So the periodic trigger is forcing a conversion sequence. This could be
> done with a software trigger as well, but why the hassle to start it
> every 2 milliseconds (or other time interval), if we can do it by
> periodic trigger ?
Ah, one reason here would be to allow separate consumers to use the
device. In that case you'd run with a periodic trigger all the time
and have two buffers attached, the buffer_cb that is feeding your
touchscreen and another buffer to deal with the other channels
(presumably the standard one an IIO device has when using buffered
interfaces).
The buffer demux would ensure the data from the right channels
ends up in the right place. It makes it look to the buffer
consumer like it is the only thing using / controlling the data
flow.
> Once we get the No pen IRQ, we stop the periodic trigger and it can be
> used in another purpose (software or external as of now in the driver,
> in the future we can add PWM trigger and Timer trigger)
This case isn't really useful though as any other use is denied
access when touch occurs.
I'll summarise what I think would work for this below.
>
> In short, the ADC in Sama5D2 also supports touchscreen, and in
> touchscreen mode , 4 of the channels are being used for this purpose.
> This however, doesn't stop the ADC to use the other channels . The
> hardware has 12 total single channels and they can be paired to have 6
> more differential channels. The only thing that is blocked is the
> trigger, but only if the pen is touching (when we start the periodic
> trigger to sample the touchscreen). If the pen is not touching, an
> external trigger or software trigger can be used without any issues (so
> why limit the functionality, if this is available from hardware ?).
> Because of the reason I discussed above (touchscreen sequence must be
> triggered), we cannot use another trigger in the same time.
>
>
> I see your idea with the callback buffer and it's worth exploring.
> Mainly this series was to actually show you what I had in mind about
> supporting the resistive touchscreen, and to give you some actually
> working code/patch, so we can discuss based on real implementation, not
> just suppositions.
That side of things is fine.
>
> You are right in many of the other comments that you said, and I will
> come up with a v2 to this series. For now, I need to know if this is a
> good or right direction in which I am going, or I should try to change
> all the mechanism to callback buffer ? Or maybe I am totally in a bad
> direction ?
> The requirements are that the consumer driver needs to be somehow woke
> up for every new touch data available, and report to the input
> subsystem. As it was done before, the at91 old driver, just creates and
> registers an input device by itself, and then reports the position and
> touches. I was thinking that with this trigger consumer implementation,
> things can be better in terms of subsystem separation and support.
>
> Thanks again and let me know of your thoughts,
>
> Eugen
So a couple of things come to mind on how I'd structure this.
So what we have (very briefly)
No touch screen case:
* Generic ADC using all sorts of different triggers
Touch screen only case:
* Interrupt to indicate pen on / off
* A need to do a periodic trigger of the device but only
useful when touch is in progress.
Touch screen and other users:
* Interrupt to indicate pen on / off
* Periodic trigger needed for touchscreen when touch in progress.
* Do not have denial of service on other channels.
First two cases are easy enough by having a magic trigger, third
case is harder.
If we have the touchscreen then I would drop support for direct access to
to ADC channels whilst it's in use (so no sysfs - or emulate it if you
really want it by stashing results from scans done when touch is in
progress).
Have your touch screen channels just as normal additional channels,
but only via the buffered interface (no _RAW attribute).
If someone sets up to read them via buffered interface with
a different trigger I think they'll get values - whether they
are right is dependent (if I understand correctly) on whether
there is a touch in progress. So no harm done and it'll make
the logic simpler.
The moment touch is opened and acquires the IIO channels
fix the trigger (may need new interface) to the periodic one
that you were enabling and disabling on touch.
Things get dicey if there is an existing user so you may
have to do it on driver probe rather than open of the input
device if we effectively want touch to have the highest
priority use of the ADC.
If other channels are enabled for buffered mode then note
this in the driver and have the periodic trigger on all the
time (to ensure they keep getting read) This will pass
garbage to your touch screen driver, but it'll remove it due
to the pressure value being too low so no harm there.
Normal path will work for non touch channels (and in theory
the touch ones if they are turned on) via IIO buffer
interface. It'll be restricted in form due to the needs of
the touch driver, but better than nothing and should cover
most usecases.
Now the interrupt on / off on touch bit becomes an optimization
in the case of only the buffer_cb being attached.
I think that fits cleanly in the current IIO framework and
looks more similar to our existing provider consumer approaches.
Still needs the hooks to get hold of the trigger though so
as to be able to tell the ADC which one to use. So rather
than being a trigger consumer interface, it's more of a trigger
configuration interface.. Exact term doesn't matter though.
Jonathan
>
>
>
> [...]
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ 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