* Re: ALPS Trackpoint & pressure
From: Peter Hutterer @ 2018-02-06 0:29 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Pali Rohár, Masaki Ota, linux-input, linux-kernel
In-Reply-To: <20180205224955.GB46524@dtor-ws>
On Mon, Feb 05, 2018 at 02:49:55PM -0800, Dmitry Torokhov wrote:
> On Sun, Feb 04, 2018 at 04:08:39PM +0100, Pali Rohár wrote:
> > Hi! Now playing again with trackpoint connected to ALPS rushmore
> > touchpad and I'm seeking a nice feature. Via ALPS PS/2 protocol it
> > reports pressure of trackpoint. Parser for it is already implemented in
> > alps.c and value is assigned to variable "z". When I just move
> > trackpoint z is zero, when I push trackpoint while moving, then z is
> > number higher, maximally 32. Variable "z" is set, but unused.
> >
> > Do we have some input interface which can be used to report this
> > pressure of trackpoint to userspace? I can use this feature e.g. as
> > additional button...
>
> We could either do the conversion in kernel and emit BTN_LEFT, or
> report ABS_PRESSURE and see if userspace will be able to handle
> REL_X/REL_Y/ABS_PRESSURE device.
>
> Adding Peter.
judging by trackpoint history, I'd leave the pressure->click conversion to
userspace because every trackpoint may need a different threshold setting.
"easier" to have this in userspace with dmi matches etc. plus, converting to
BTN_LEFT in the kernel means we cannot use it as a separate interaction
anymore.
That aside, I think exporting ABS_PRESSURE is fine, that's what it's there
for. Nothing will use it for now though, tbh not sure yet how that would be
exported from libinput. but worth filing a bug for, please assign it to me.
Cheers,
Peter
^ permalink raw reply
* Re: [BUG] drivers/input/misc/pcap: a possible sleep-in-atomic bug in pcap_keys_handler
From: Dmitry Torokhov @ 2018-02-05 23:25 UTC (permalink / raw)
To: Jia-Ju Bai, Daniel Ribeiro; +Cc: ilya.muromec, linux-input, linux-kernel
In-Reply-To: <9af81fe8-e07c-9161-2d85-3eb23a198ec9@gmail.com>
On Tue, Dec 12, 2017 at 04:32:01PM +0800, Jia-Ju Bai wrote:
> According to drivers/input/misc/pcap_keys.c, the kernel module may sleep in
> the interrupt handler.
> The function call path is:
> pcap_keys_handler (interrupt handler)
> ezx_pcap_read
> mutex_lock --> may sleep
>
> I do not find a good way to fix it, so I only report.
> This possible bug is found by my static analysis tool (DSAC) and checked by
> my code review.
Let's see if Daniel's email still work. I am not sure how it worked to
begin with, given that we've been taking that mutex since very
beginning, and pcap_isr_work() disables local interrupts...
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3] Input: matrix_keypad - fix keypad does not response
From: Dmitry Torokhov @ 2018-02-05 23:07 UTC (permalink / raw)
To: Zhang Bo; +Cc: DRivshin, robh, linux-input, linux-kernel, zhang.bo19
In-Reply-To: <20180204001922.32084-1-zbsdta@126.com>
On Sun, Feb 04, 2018 at 08:19:22AM +0800, Zhang Bo wrote:
> If matrix_keypad_stop() is calling and the keypad interrupt is triggered,
> disable_row_irqs() is called by both matrix_keypad_interrupt() and
> matrix_keypad_stop() at the same time. then disable_row_irqs() is called
> twice, and the device enter suspend state before keypad->work is executed.
> At this condition the device will start keypad and enable irq once after
> resume. and then irqs are disabled yet because irqs are disabled twice and
> only enable once.
>
> Take lock around keypad->stopped to ensure irqs operation is in atomic
> operation.
>
> Signed-off-by: Zhang Bo <zbsdta@126.com>
Applied, thank you.
> ---
> Changes in v3:
> - delete no needed lock protection, only add lock in matrix_keypad_stop().
> Changes in v2:
> - Change commit message and full name in the signed-off-by tag.
>
> drivers/input/keyboard/matrix_keypad.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
> index 1f316d66e6f7..41614c185918 100644
> --- a/drivers/input/keyboard/matrix_keypad.c
> +++ b/drivers/input/keyboard/matrix_keypad.c
> @@ -218,8 +218,10 @@ static void matrix_keypad_stop(struct input_dev *dev)
> {
> struct matrix_keypad *keypad = input_get_drvdata(dev);
>
> + spin_lock_irq(&keypad->lock);
> keypad->stopped = true;
> - mb();
> + spin_unlock_irq(&keypad->lock);
> +
> flush_work(&keypad->work.work);
> /*
> * matrix_keypad_scan() will leave IRQs enabled;
> --
> 2.14.3
>
>
--
Dmitry
^ permalink raw reply
* Re: ALPS Trackpoint & pressure
From: Dmitry Torokhov @ 2018-02-05 22:49 UTC (permalink / raw)
To: Pali Rohár, Peter Hutterer; +Cc: Masaki Ota, linux-input, linux-kernel
In-Reply-To: <20180204150839.b5qm4wb26vic35st@pali>
On Sun, Feb 04, 2018 at 04:08:39PM +0100, Pali Rohár wrote:
> Hi! Now playing again with trackpoint connected to ALPS rushmore
> touchpad and I'm seeking a nice feature. Via ALPS PS/2 protocol it
> reports pressure of trackpoint. Parser for it is already implemented in
> alps.c and value is assigned to variable "z". When I just move
> trackpoint z is zero, when I push trackpoint while moving, then z is
> number higher, maximally 32. Variable "z" is set, but unused.
>
> Do we have some input interface which can be used to report this
> pressure of trackpoint to userspace? I can use this feature e.g. as
> additional button...
We could either do the conversion in kernel and emit BTN_LEFT, or
report ABS_PRESSURE and see if userspace will be able to handle
REL_X/REL_Y/ABS_PRESSURE device.
Adding Peter.
--
Dmitry
^ permalink raw reply
* Re: hid-sensor-accel-3d: Delete an error message for a failed memory allocation in hid_accel_3d_probe()
From: Jonathan Cameron @ 2018-02-05 21:51 UTC (permalink / raw)
To: SF Markus Elfring, Jonathan Cameron,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA
Cc: Hartmut Knaack, Jiri Kosina, Lars-Peter Clausen,
Peter Meerwald-Stadler, Srinivas Pandruvada, LKML,
kernel-janitors-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <9420fc82-1a37-3601-bafe-f57ef953bfcd-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
On 5 February 2018 18:26:59 GMT, SF Markus Elfring <elfring-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org> wrote:
>> If making changes like this I would suggest only sending one until
>> you have have a response from the relevant maintainer.
>
>The corresponding feedback can become more positive for such
>a transformation pattern after a while, can't it?
Not in that sort of time period.
>
>
>> It would save you time as often these sorts of changes are
>> a matter of personal taste and weighing up of costs vs gains
>> - hence it is not obvious that they will be accepted.
>
>Can the wording “WARNING: Possible unnecessary 'out of memory' message”
>(from the script “checkpatch.pl”) be another incentive?
No. The key word is possible. Check patch is dumb and often gives false positives.
>
>Regards,
>Markus
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply
* Re: [PATCH 4/5] add V4L2 control functions
From: Hans Verkuil @ 2018-02-05 21:40 UTC (permalink / raw)
To: Florian Echtler; +Cc: linux-media, linux-input, modin
In-Reply-To: <alpine.DEB.2.10.1802052234090.18874@butterbrot>
On 02/05/2018 10:36 PM, Florian Echtler wrote:
> Hello Hans,
>
> On Mon, 5 Feb 2018, Hans Verkuil wrote:
>> On 02/05/2018 03:29 PM, Florian Echtler wrote:
>>> +
>>> +static int sur40_vidioc_queryctrl(struct file *file, void *fh,
>>> + struct v4l2_queryctrl *qc)
>>
>> Sorry, but this is very wrong. Use the control framework instead. See
>> https://hverkuil.home.xs4all.nl/spec/kapi/v4l2-controls.html and pretty much all
>> media drivers (with the exception of uvc). See for example this driver:
>> drivers/media/pci/tw68/tw68-video.c (randomly chosen).
>>
>> It actually makes life a lot easier for you as you don't have to perform any
>> range checks and all control ioctls are automatically supported for you.
>
> thanks for the quick reply, I wasn't aware of that framework at all :-)
> Will certainly use it.
>
> What's the earliest kernel version this is supported on, in case we want
> to backport this for our standalone module, too?
Initial commit was in August 2010, so it's been around for quite some time :-)
Regards,
Hans
^ permalink raw reply
* Re: [PATCH 4/5] add V4L2 control functions
From: Florian Echtler @ 2018-02-05 21:36 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, linux-input, modin
In-Reply-To: <4835be2b-238c-bb27-e9e5-98642ae76733@xs4all.nl>
Hello Hans,
On Mon, 5 Feb 2018, Hans Verkuil wrote:
> On 02/05/2018 03:29 PM, Florian Echtler wrote:
>> +
>> +static int sur40_vidioc_queryctrl(struct file *file, void *fh,
>> + struct v4l2_queryctrl *qc)
>
> Sorry, but this is very wrong. Use the control framework instead. See
> https://hverkuil.home.xs4all.nl/spec/kapi/v4l2-controls.html and pretty much all
> media drivers (with the exception of uvc). See for example this driver:
> drivers/media/pci/tw68/tw68-video.c (randomly chosen).
>
> It actually makes life a lot easier for you as you don't have to perform any
> range checks and all control ioctls are automatically supported for you.
thanks for the quick reply, I wasn't aware of that framework at all :-)
Will certainly use it.
What's the earliest kernel version this is supported on, in case we want
to backport this for our standalone module, too?
Best regards, Florian
--
"_Nothing_ brightens up my morning. Coffee simply provides a shade of
grey just above the pitch-black of the infinite depths of the _abyss_."
^ permalink raw reply
* [PATCH 4/4] HID: logitech-dj: Fix a typo in a comment line
From: SF Markus Elfring @ 2018-02-05 21:30 UTC (permalink / raw)
To: linux-input, Benjamin Tissoires, Jiri Kosina; +Cc: LKML, kernel-janitors
In-Reply-To: <12e9f2fd-5e85-fe8b-5eec-cd1712b07166@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 5 Feb 2018 22:14:49 +0100
Adjust a word in this description.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/hid/hid-logitech-dj.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index f1496c6303ce..de1859fade5c 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -318,7 +318,7 @@ static const char hidpp_descriptor[] = {
*
* Right now, RF report types have the same report types (or report id's)
* than the hid report created from those RF reports. In the future
- * this doesnt have to be true.
+ * this doesn't have to be true.
*
* For instance, RF report type 0x01 which has a size of 8 bytes, corresponds
* to hid report id 0x01, this is standard keyboard. Same thing applies to mice
--
2.16.1
^ permalink raw reply related
* [PATCH 3/4] HID: logitech-dj: Combine substrings for seven messages
From: SF Markus Elfring @ 2018-02-05 21:28 UTC (permalink / raw)
To: linux-input, Benjamin Tissoires, Jiri Kosina; +Cc: LKML, kernel-janitors
In-Reply-To: <12e9f2fd-5e85-fe8b-5eec-cd1712b07166@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 5 Feb 2018 22:06:20 +0100
The script "checkpatch.pl" pointed information out like the following.
WARNING: quoted string split across lines
Thus fix the affected source code places.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/hid/hid-logitech-dj.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 0da00d256ca7..f1496c6303ce 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -463,16 +463,17 @@ static void delayedwork_callback(struct work_struct *work)
sizeof(struct dj_report));
if (count != sizeof(struct dj_report)) {
- dev_err(&djrcv_dev->hdev->dev, "%s: workitem triggered without "
- "notifications available\n", __func__);
+ dev_err(&djrcv_dev->hdev->dev,
+ "%s: workitem triggered without notifications available\n",
+ __func__);
spin_unlock_irqrestore(&djrcv_dev->lock, flags);
return;
}
if (!kfifo_is_empty(&djrcv_dev->notif_fifo)) {
if (schedule_work(&djrcv_dev->work) == 0) {
- dbg_hid("%s: did not schedule the work item, was "
- "already queued\n", __func__);
+ dbg_hid("%s: did not schedule the work item, was already queued\n",
+ __func__);
}
}
@@ -502,8 +503,8 @@ static void delayedwork_callback(struct work_struct *work)
break;
}
dev_err(&djrcv_dev->hdev->dev,
- "%s:logi_dj_recv_query_paired_devices "
- "error:%d\n", __func__, retval);
+ "%s:logi_dj_recv_query_paired_devices error:%d\n",
+ __func__, retval);
}
dbg_hid("%s: unexpected report type\n", __func__);
}
@@ -517,8 +518,8 @@ static void logi_dj_recv_queue_notification(struct dj_receiver_dev *djrcv_dev,
kfifo_in(&djrcv_dev->notif_fifo, dj_report, sizeof(struct dj_report));
if (schedule_work(&djrcv_dev->work) == 0) {
- dbg_hid("%s: did not schedule the work item, was already "
- "queued\n", __func__);
+ dbg_hid("%s: did not schedule the work item, was already queued\n",
+ __func__);
}
}
@@ -541,8 +542,7 @@ static void logi_dj_recv_forward_null_report(struct dj_receiver_dev *djrcv_dev,
HID_INPUT_REPORT,
reportbuffer,
hid_reportid_size_map[i], 1)) {
- dbg_hid("hid_input_report error sending null "
- "report\n");
+ dbg_hid("hid_input_report error sending null report\n");
}
}
}
@@ -770,8 +770,8 @@ static int logi_dj_ll_parse(struct hid_device *hid)
}
if (djdev->reports_supported & STD_MOUSE) {
- dbg_hid("%s: sending a mouse descriptor, reports_supported: "
- "%x\n", __func__, djdev->reports_supported);
+ dbg_hid("%s: sending a mouse descriptor, reports_supported: %x\n",
+ __func__, djdev->reports_supported);
rdcat(rdesc, &rsize, mse_descriptor, sizeof(mse_descriptor));
}
@@ -1068,8 +1068,9 @@ static int logi_dj_probe(struct hid_device *hdev,
retval = logi_dj_recv_query_paired_devices(djrcv_dev);
if (retval < 0) {
- dev_err(&hdev->dev, "%s:logi_dj_recv_query_paired_devices "
- "error:%d\n", __func__, retval);
+ dev_err(&hdev->dev,
+ "%s:logi_dj_recv_query_paired_devices error:%d\n",
+ __func__, retval);
goto logi_dj_recv_query_paired_devices_failed;
}
--
2.16.1
^ permalink raw reply related
* [PATCH 2/4] HID: logitech-dj: Improve a size determination in four functions
From: SF Markus Elfring @ 2018-02-05 21:27 UTC (permalink / raw)
To: linux-input, Benjamin Tissoires, Jiri Kosina; +Cc: LKML, kernel-janitors
In-Reply-To: <12e9f2fd-5e85-fe8b-5eec-cd1712b07166@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 5 Feb 2018 21:47:45 +0100
Replace the specification of data structures by pointer dereferences
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/hid/hid-logitech-dj.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 4c92b2cb5768..0da00d256ca7 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -417,7 +417,7 @@ static void logi_dj_recv_add_djhid_device(struct dj_receiver_dev *djrcv_dev,
snprintf(tmpstr, sizeof(tmpstr), ":%d", dj_report->device_index);
strlcat(dj_hiddev->phys, tmpstr, sizeof(dj_hiddev->phys));
- dj_dev = kzalloc(sizeof(struct dj_device), GFP_KERNEL);
+ dj_dev = kzalloc(sizeof(*dj_dev), GFP_KERNEL);
if (!dj_dev)
goto dj_device_allocate_fail;
@@ -611,7 +611,7 @@ static int logi_dj_recv_query_paired_devices(struct dj_receiver_dev *djrcv_dev)
if (djrcv_dev->querying_devices)
return 0;
- dj_report = kzalloc(sizeof(struct dj_report), GFP_KERNEL);
+ dj_report = kzalloc(sizeof(*dj_report), GFP_KERNEL);
if (!dj_report)
return -ENOMEM;
dj_report->report_id = REPORT_ID_DJ_SHORT;
@@ -627,11 +627,10 @@ static int logi_dj_recv_switch_to_dj_mode(struct dj_receiver_dev *djrcv_dev,
unsigned timeout)
{
struct hid_device *hdev = djrcv_dev->hdev;
- struct dj_report *dj_report;
u8 *buf;
int retval;
+ struct dj_report *dj_report = kzalloc(sizeof(*dj_report), GFP_KERNEL);
- dj_report = kzalloc(sizeof(struct dj_report), GFP_KERNEL);
if (!dj_report)
return -ENOMEM;
dj_report->report_id = REPORT_ID_DJ_SHORT;
@@ -1005,8 +1004,7 @@ static int logi_dj_probe(struct hid_device *hdev,
}
/* Treat interface 2 */
-
- djrcv_dev = kzalloc(sizeof(struct dj_receiver_dev), GFP_KERNEL);
+ djrcv_dev = kzalloc(sizeof(*djrcv_dev), GFP_KERNEL);
if (!djrcv_dev)
return -ENOMEM;
--
2.16.1
^ permalink raw reply related
* [PATCH 1/4] HID: logitech-dj: Delete an error message for a failed memory allocation in two functions
From: SF Markus Elfring @ 2018-02-05 21:26 UTC (permalink / raw)
To: linux-input, Benjamin Tissoires, Jiri Kosina; +Cc: LKML, kernel-janitors
In-Reply-To: <12e9f2fd-5e85-fe8b-5eec-cd1712b07166@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 5 Feb 2018 21:36:18 +0100
Omit an extra message for a memory allocation failure in these functions.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/hid/hid-logitech-dj.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 826fa1e1c8d9..4c92b2cb5768 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -418,12 +418,8 @@ static void logi_dj_recv_add_djhid_device(struct dj_receiver_dev *djrcv_dev,
strlcat(dj_hiddev->phys, tmpstr, sizeof(dj_hiddev->phys));
dj_dev = kzalloc(sizeof(struct dj_device), GFP_KERNEL);
-
- if (!dj_dev) {
- dev_err(&djrcv_hdev->dev, "%s: failed allocating dj_device\n",
- __func__);
+ if (!dj_dev)
goto dj_device_allocate_fail;
- }
dj_dev->reports_supported = get_unaligned_le32(
dj_report->report_params + DEVICE_PAIRED_RF_REPORT_TYPE);
@@ -1011,11 +1007,9 @@ static int logi_dj_probe(struct hid_device *hdev,
/* Treat interface 2 */
djrcv_dev = kzalloc(sizeof(struct dj_receiver_dev), GFP_KERNEL);
- if (!djrcv_dev) {
- dev_err(&hdev->dev,
- "%s:failed allocating dj_receiver_dev\n", __func__);
+ if (!djrcv_dev)
return -ENOMEM;
- }
+
djrcv_dev->hdev = hdev;
INIT_WORK(&djrcv_dev->work, delayedwork_callback);
spin_lock_init(&djrcv_dev->lock);
--
2.16.1
^ permalink raw reply related
* [PATCH 0/4] HID: logitech-dj: Adjustments for some function implementations
From: SF Markus Elfring @ 2018-02-05 21:24 UTC (permalink / raw)
To: linux-input, Benjamin Tissoires, Jiri Kosina; +Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 5 Feb 2018 22:22:11 +0100
A few update suggestions were taken into account
from static source code analysis.
Markus Elfring (4):
Delete an error message for a failed memory allocation in two functions
Improve a size determination in four functions
Combine substrings for seven messages
Fix a typo in a comment line
drivers/hid/hid-logitech-dj.c | 53 +++++++++++++++++++------------------------
1 file changed, 23 insertions(+), 30 deletions(-)
--
2.16.1
^ permalink raw reply
* Re: hid-sensor-accel-3d: Delete an error message for a failed memory allocation in hid_accel_3d_probe()
From: SF Markus Elfring @ 2018-02-05 18:26 UTC (permalink / raw)
To: Jonathan Cameron, linux-iio, linux-input
Cc: Hartmut Knaack, Jiri Kosina, Lars-Peter Clausen,
Peter Meerwald-Stadler, Srinivas Pandruvada, LKML,
kernel-janitors
In-Reply-To: <20180204112346.0977e938@archlinux>
> If making changes like this I would suggest only sending one until
> you have have a response from the relevant maintainer.
The corresponding feedback can become more positive for such
a transformation pattern after a while, can't it?
> It would save you time as often these sorts of changes are
> a matter of personal taste and weighing up of costs vs gains
> - hence it is not obvious that they will be accepted.
Can the wording “WARNING: Possible unnecessary 'out of memory' message”
(from the script “checkpatch.pl”) be another incentive?
Regards,
Markus
^ permalink raw reply
* Re: [PATCH 5/5] add default control values as module parameters
From: Hans Verkuil @ 2018-02-05 14:56 UTC (permalink / raw)
To: Florian Echtler, linux-media; +Cc: linux-input, modin
In-Reply-To: <1517840981-12280-6-git-send-email-floe@butterbrot.org>
On 02/05/2018 03:29 PM, Florian Echtler wrote:
> Signed-off-by: Florian Echtler <floe@butterbrot.org>
Please add a change log when you make a patch.
I for one would like to know why this has to be supplied as a module option.
Some documentation in the code would be helpful as well (e.g. I have no idea
what a 'vsvideo' is).
Regards,
Hans
> ---
> drivers/input/touchscreen/sur40.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
> index c4b7cf1..d612f3f 100644
> --- a/drivers/input/touchscreen/sur40.c
> +++ b/drivers/input/touchscreen/sur40.c
> @@ -173,6 +173,14 @@ int sur40_v4l2_contrast = SUR40_CONTRAST_DEF; /* blacklevel */
> int sur40_v4l2_gain = SUR40_GAIN_DEF; /* gain */
> int sur40_v4l2_backlight = 1; /* preprocessor */
>
> +/* module parameters */
> +static uint irlevel = SUR40_BRIGHTNESS_DEF;
> +module_param(irlevel, uint, 0644);
> +MODULE_PARM_DESC(irlevel, "set default irlevel");
> +static uint vsvideo = SUR40_VSVIDEO_DEF;
> +module_param(vsvideo, uint, 0644);
> +MODULE_PARM_DESC(vsvideo, "set default vsvideo");
> +
> static const struct v4l2_pix_format sur40_pix_format[] = {
> {
> .pixelformat = V4L2_TCH_FMT_TU08,
> @@ -372,6 +380,11 @@ static void sur40_open(struct input_polled_dev *polldev)
>
> dev_dbg(sur40->dev, "open\n");
> sur40_init(sur40);
> +
> + /* set default values */
> + sur40_set_irlevel(sur40, irlevel);
> + sur40_set_vsvideo(sur40, vsvideo);
> + sur40_set_preprocessor(sur40, SUR40_BACKLIGHT_DEF);
> }
>
> /* Disable device, polling has stopped. */
>
^ permalink raw reply
* Re: [PATCH 4/5] add V4L2 control functions
From: Hans Verkuil @ 2018-02-05 14:54 UTC (permalink / raw)
To: Florian Echtler, linux-media; +Cc: linux-input, modin
In-Reply-To: <1517840981-12280-5-git-send-email-floe@butterbrot.org>
On 02/05/2018 03:29 PM, Florian Echtler wrote:
> Signed-off-by: Florian Echtler <floe@butterbrot.org>
> ---
> drivers/input/touchscreen/sur40.c | 114 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 114 insertions(+)
>
> diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
> index 63c7264b..c4b7cf1 100644
> --- a/drivers/input/touchscreen/sur40.c
> +++ b/drivers/input/touchscreen/sur40.c
> @@ -953,6 +953,119 @@ static int sur40_vidioc_g_fmt(struct file *file, void *priv,
> return 0;
> }
>
> +
> +static int sur40_vidioc_queryctrl(struct file *file, void *fh,
> + struct v4l2_queryctrl *qc)
> +{
> +
> + switch (qc->id) {
> + case V4L2_CID_BRIGHTNESS:
> + qc->flags = 0;
> + sprintf(qc->name, "Brightness");
> + qc->type = V4L2_CTRL_TYPE_INTEGER;
> + qc->minimum = SUR40_BRIGHTNESS_MIN;
> + qc->default_value = SUR40_BRIGHTNESS_DEF;
> + qc->maximum = SUR40_BRIGHTNESS_MAX;
> + qc->step = 8;
> + return 0;
> + case V4L2_CID_CONTRAST:
> + qc->flags = 0;
> + sprintf(qc->name, "Contrast");
> + qc->type = V4L2_CTRL_TYPE_INTEGER;
> + qc->minimum = SUR40_CONTRAST_MIN;
> + qc->default_value = SUR40_CONTRAST_DEF;
> + qc->maximum = SUR40_CONTRAST_MAX;
> + qc->step = 1;
> + return 0;
> + case V4L2_CID_GAIN:
> + qc->flags = 0;
> + sprintf(qc->name, "Gain");
> + qc->type = V4L2_CTRL_TYPE_INTEGER;
> + qc->minimum = SUR40_GAIN_MIN;
> + qc->default_value = SUR40_GAIN_DEF;
> + qc->maximum = SUR40_GAIN_MAX;
> + qc->step = 1;
> + return 0;
> + case V4L2_CID_BACKLIGHT_COMPENSATION:
> + qc->flags = 0;
> + sprintf(qc->name, "Preprocessor");
> + qc->type = V4L2_CTRL_TYPE_INTEGER;
> + qc->minimum = SUR40_BACKLIGHT_MIN;
> + qc->default_value = SUR40_BACKLIGHT_DEF;
> + qc->maximum = SUR40_BACKLIGHT_MAX;
> + qc->step = 1;
> + return 0;
> + default:
> + qc->flags = V4L2_CTRL_FLAG_DISABLED;
> + return -EINVAL;
> + }
> +}
> +
> +static int sur40_vidioc_g_ctrl(struct file *file, void *fh,
> + struct v4l2_control *ctrl)
> +{
> +
> + switch (ctrl->id) {
> + case V4L2_CID_BRIGHTNESS:
> + ctrl->value = sur40_v4l2_brightness;
> + return 0;
> + case V4L2_CID_CONTRAST:
> + ctrl->value = sur40_v4l2_contrast;
> + return 0;
> + case V4L2_CID_GAIN:
> + ctrl->value = sur40_v4l2_gain;
> + return 0;
> + case V4L2_CID_BACKLIGHT_COMPENSATION:
> + ctrl->value = sur40_v4l2_backlight;
> + return 0;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int sur40_vidioc_s_ctrl(struct file *file, void *fh,
> + struct v4l2_control *ctrl)
> +{
> + u8 value = 0;
> + struct sur40_state *sur40 = video_drvdata(file);
> +
> + switch (ctrl->id) {
> + case V4L2_CID_BRIGHTNESS:
> + sur40_v4l2_brightness = ctrl->value;
> + if (sur40_v4l2_brightness < SUR40_BRIGHTNESS_MIN)
> + sur40_v4l2_brightness = SUR40_BRIGHTNESS_MIN;
> + else if (sur40_v4l2_brightness > SUR40_BRIGHTNESS_MAX)
> + sur40_v4l2_brightness = SUR40_BRIGHTNESS_MAX;
> + sur40_set_irlevel(sur40, sur40_v4l2_brightness);
> + return 0;
> + case V4L2_CID_CONTRAST:
> + sur40_v4l2_contrast = ctrl->value;
> + if (sur40_v4l2_contrast < SUR40_CONTRAST_MIN)
> + sur40_v4l2_contrast = SUR40_CONTRAST_MIN;
> + else if (sur40_v4l2_contrast > SUR40_CONTRAST_MAX)
> + sur40_v4l2_contrast = SUR40_CONTRAST_MAX;
> + value = (sur40_v4l2_contrast << 4) + sur40_v4l2_gain;
> + sur40_set_vsvideo(sur40, value);
> + return 0;
> + case V4L2_CID_GAIN:
> + sur40_v4l2_gain = ctrl->value;
> + if (sur40_v4l2_gain < SUR40_GAIN_MIN)
> + sur40_v4l2_gain = SUR40_GAIN_MIN;
> + else if (sur40_v4l2_gain > SUR40_GAIN_MAX)
> + sur40_v4l2_gain = SUR40_GAIN_MAX;
> + value = (sur40_v4l2_contrast << 4) + sur40_v4l2_gain;
> + sur40_set_vsvideo(sur40, value);
> + return 0;
> + case V4L2_CID_BACKLIGHT_COMPENSATION:
> + sur40_v4l2_backlight = ctrl->value;
> + sur40_set_preprocessor(sur40, sur40_v4l2_backlight);
> + return 0;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +
> static int sur40_ioctl_parm(struct file *file, void *priv,
> struct v4l2_streamparm *p)
> {
> @@ -1071,6 +1181,10 @@ static const struct v4l2_ioctl_ops sur40_video_ioctl_ops = {
> .vidioc_g_input = sur40_vidioc_g_input,
> .vidioc_s_input = sur40_vidioc_s_input,
>
> + .vidioc_queryctrl = sur40_vidioc_queryctrl,
> + .vidioc_g_ctrl = sur40_vidioc_g_ctrl,
> + .vidioc_s_ctrl = sur40_vidioc_s_ctrl,
> +
> .vidioc_reqbufs = vb2_ioctl_reqbufs,
> .vidioc_create_bufs = vb2_ioctl_create_bufs,
> .vidioc_querybuf = vb2_ioctl_querybuf,
>
Sorry, but this is very wrong. Use the control framework instead. See
https://hverkuil.home.xs4all.nl/spec/kapi/v4l2-controls.html and pretty much all
media drivers (with the exception of uvc). See for example this driver:
drivers/media/pci/tw68/tw68-video.c (randomly chosen).
It actually makes life a lot easier for you as you don't have to perform any
range checks and all control ioctls are automatically supported for you.
Regards,
Hans
^ permalink raw reply
* [PATCH 1/5] add missing blob structure tag field
From: Florian Echtler @ 2018-02-05 14:29 UTC (permalink / raw)
To: linux-media, hverkuil; +Cc: linux-input, modin, Florian Echtler
In-Reply-To: <1517840981-12280-1-git-send-email-floe@butterbrot.org>
Signed-off-by: Florian Echtler <floe@butterbrot.org>
---
drivers/input/touchscreen/sur40.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index f16f835..8375b06 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -81,7 +81,10 @@ struct sur40_blob {
__le32 area; /* size in pixels/pressure (?) */
- u8 padding[32];
+ u8 padding[24];
+
+ __le32 tag_id; /* valid when type == 0x04 (SUR40_TAG) */
+ __le32 unknown;
} __packed;
--
2.7.4
^ permalink raw reply related
* [PATCH 4/5] add V4L2 control functions
From: Florian Echtler @ 2018-02-05 14:29 UTC (permalink / raw)
To: linux-media, hverkuil; +Cc: linux-input, modin, Florian Echtler
In-Reply-To: <1517840981-12280-1-git-send-email-floe@butterbrot.org>
Signed-off-by: Florian Echtler <floe@butterbrot.org>
---
drivers/input/touchscreen/sur40.c | 114 ++++++++++++++++++++++++++++++++++++++
1 file changed, 114 insertions(+)
diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index 63c7264b..c4b7cf1 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -953,6 +953,119 @@ static int sur40_vidioc_g_fmt(struct file *file, void *priv,
return 0;
}
+
+static int sur40_vidioc_queryctrl(struct file *file, void *fh,
+ struct v4l2_queryctrl *qc)
+{
+
+ switch (qc->id) {
+ case V4L2_CID_BRIGHTNESS:
+ qc->flags = 0;
+ sprintf(qc->name, "Brightness");
+ qc->type = V4L2_CTRL_TYPE_INTEGER;
+ qc->minimum = SUR40_BRIGHTNESS_MIN;
+ qc->default_value = SUR40_BRIGHTNESS_DEF;
+ qc->maximum = SUR40_BRIGHTNESS_MAX;
+ qc->step = 8;
+ return 0;
+ case V4L2_CID_CONTRAST:
+ qc->flags = 0;
+ sprintf(qc->name, "Contrast");
+ qc->type = V4L2_CTRL_TYPE_INTEGER;
+ qc->minimum = SUR40_CONTRAST_MIN;
+ qc->default_value = SUR40_CONTRAST_DEF;
+ qc->maximum = SUR40_CONTRAST_MAX;
+ qc->step = 1;
+ return 0;
+ case V4L2_CID_GAIN:
+ qc->flags = 0;
+ sprintf(qc->name, "Gain");
+ qc->type = V4L2_CTRL_TYPE_INTEGER;
+ qc->minimum = SUR40_GAIN_MIN;
+ qc->default_value = SUR40_GAIN_DEF;
+ qc->maximum = SUR40_GAIN_MAX;
+ qc->step = 1;
+ return 0;
+ case V4L2_CID_BACKLIGHT_COMPENSATION:
+ qc->flags = 0;
+ sprintf(qc->name, "Preprocessor");
+ qc->type = V4L2_CTRL_TYPE_INTEGER;
+ qc->minimum = SUR40_BACKLIGHT_MIN;
+ qc->default_value = SUR40_BACKLIGHT_DEF;
+ qc->maximum = SUR40_BACKLIGHT_MAX;
+ qc->step = 1;
+ return 0;
+ default:
+ qc->flags = V4L2_CTRL_FLAG_DISABLED;
+ return -EINVAL;
+ }
+}
+
+static int sur40_vidioc_g_ctrl(struct file *file, void *fh,
+ struct v4l2_control *ctrl)
+{
+
+ switch (ctrl->id) {
+ case V4L2_CID_BRIGHTNESS:
+ ctrl->value = sur40_v4l2_brightness;
+ return 0;
+ case V4L2_CID_CONTRAST:
+ ctrl->value = sur40_v4l2_contrast;
+ return 0;
+ case V4L2_CID_GAIN:
+ ctrl->value = sur40_v4l2_gain;
+ return 0;
+ case V4L2_CID_BACKLIGHT_COMPENSATION:
+ ctrl->value = sur40_v4l2_backlight;
+ return 0;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int sur40_vidioc_s_ctrl(struct file *file, void *fh,
+ struct v4l2_control *ctrl)
+{
+ u8 value = 0;
+ struct sur40_state *sur40 = video_drvdata(file);
+
+ switch (ctrl->id) {
+ case V4L2_CID_BRIGHTNESS:
+ sur40_v4l2_brightness = ctrl->value;
+ if (sur40_v4l2_brightness < SUR40_BRIGHTNESS_MIN)
+ sur40_v4l2_brightness = SUR40_BRIGHTNESS_MIN;
+ else if (sur40_v4l2_brightness > SUR40_BRIGHTNESS_MAX)
+ sur40_v4l2_brightness = SUR40_BRIGHTNESS_MAX;
+ sur40_set_irlevel(sur40, sur40_v4l2_brightness);
+ return 0;
+ case V4L2_CID_CONTRAST:
+ sur40_v4l2_contrast = ctrl->value;
+ if (sur40_v4l2_contrast < SUR40_CONTRAST_MIN)
+ sur40_v4l2_contrast = SUR40_CONTRAST_MIN;
+ else if (sur40_v4l2_contrast > SUR40_CONTRAST_MAX)
+ sur40_v4l2_contrast = SUR40_CONTRAST_MAX;
+ value = (sur40_v4l2_contrast << 4) + sur40_v4l2_gain;
+ sur40_set_vsvideo(sur40, value);
+ return 0;
+ case V4L2_CID_GAIN:
+ sur40_v4l2_gain = ctrl->value;
+ if (sur40_v4l2_gain < SUR40_GAIN_MIN)
+ sur40_v4l2_gain = SUR40_GAIN_MIN;
+ else if (sur40_v4l2_gain > SUR40_GAIN_MAX)
+ sur40_v4l2_gain = SUR40_GAIN_MAX;
+ value = (sur40_v4l2_contrast << 4) + sur40_v4l2_gain;
+ sur40_set_vsvideo(sur40, value);
+ return 0;
+ case V4L2_CID_BACKLIGHT_COMPENSATION:
+ sur40_v4l2_backlight = ctrl->value;
+ sur40_set_preprocessor(sur40, sur40_v4l2_backlight);
+ return 0;
+ default:
+ return -EINVAL;
+ }
+}
+
+
static int sur40_ioctl_parm(struct file *file, void *priv,
struct v4l2_streamparm *p)
{
@@ -1071,6 +1181,10 @@ static const struct v4l2_ioctl_ops sur40_video_ioctl_ops = {
.vidioc_g_input = sur40_vidioc_g_input,
.vidioc_s_input = sur40_vidioc_s_input,
+ .vidioc_queryctrl = sur40_vidioc_queryctrl,
+ .vidioc_g_ctrl = sur40_vidioc_g_ctrl,
+ .vidioc_s_ctrl = sur40_vidioc_s_ctrl,
+
.vidioc_reqbufs = vb2_ioctl_reqbufs,
.vidioc_create_bufs = vb2_ioctl_create_bufs,
.vidioc_querybuf = vb2_ioctl_querybuf,
--
2.7.4
^ permalink raw reply related
* [PATCH 5/5] add default control values as module parameters
From: Florian Echtler @ 2018-02-05 14:29 UTC (permalink / raw)
To: linux-media, hverkuil; +Cc: linux-input, modin, Florian Echtler
In-Reply-To: <1517840981-12280-1-git-send-email-floe@butterbrot.org>
Signed-off-by: Florian Echtler <floe@butterbrot.org>
---
drivers/input/touchscreen/sur40.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index c4b7cf1..d612f3f 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -173,6 +173,14 @@ int sur40_v4l2_contrast = SUR40_CONTRAST_DEF; /* blacklevel */
int sur40_v4l2_gain = SUR40_GAIN_DEF; /* gain */
int sur40_v4l2_backlight = 1; /* preprocessor */
+/* module parameters */
+static uint irlevel = SUR40_BRIGHTNESS_DEF;
+module_param(irlevel, uint, 0644);
+MODULE_PARM_DESC(irlevel, "set default irlevel");
+static uint vsvideo = SUR40_VSVIDEO_DEF;
+module_param(vsvideo, uint, 0644);
+MODULE_PARM_DESC(vsvideo, "set default vsvideo");
+
static const struct v4l2_pix_format sur40_pix_format[] = {
{
.pixelformat = V4L2_TCH_FMT_TU08,
@@ -372,6 +380,11 @@ static void sur40_open(struct input_polled_dev *polldev)
dev_dbg(sur40->dev, "open\n");
sur40_init(sur40);
+
+ /* set default values */
+ sur40_set_irlevel(sur40, irlevel);
+ sur40_set_vsvideo(sur40, vsvideo);
+ sur40_set_preprocessor(sur40, SUR40_BACKLIGHT_DEF);
}
/* Disable device, polling has stopped. */
--
2.7.4
^ permalink raw reply related
* [PATCH 3/5] add video control register handlers
From: Florian Echtler @ 2018-02-05 14:29 UTC (permalink / raw)
To: linux-media, hverkuil; +Cc: linux-input, modin, Florian Echtler
In-Reply-To: <1517840981-12280-1-git-send-email-floe@butterbrot.org>
Signed-off-by: Florian Echtler <floe@butterbrot.org>
---
drivers/input/touchscreen/sur40.c | 70 +++++++++++++++++++++++++++++++++++++++
1 file changed, 70 insertions(+)
diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index 0dbb004..63c7264b 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -247,6 +255,80 @@ static int sur40_command(struct sur40_state *dev,
0x00, index, buffer, size, 1000);
}
+/* poke a byte in the panel register space */
+static int sur40_poke(struct sur40_state *dev, u8 offset, u8 value)
+{
+ int result;
+ u8 index = 0x96; // 0xae for permanent write
+
+ result = usb_control_msg(dev->usbdev, usb_sndctrlpipe(dev->usbdev, 0),
+ SUR40_POKE, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
+ 0x32, index, NULL, 0, 1000);
+ if (result < 0)
+ goto error;
+ msleep(5);
+
+ result = usb_control_msg(dev->usbdev, usb_sndctrlpipe(dev->usbdev, 0),
+ SUR40_POKE, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
+ 0x72, offset, NULL, 0, 1000);
+ if (result < 0)
+ goto error;
+ msleep(5);
+
+ result = usb_control_msg(dev->usbdev, usb_sndctrlpipe(dev->usbdev, 0),
+ SUR40_POKE, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
+ 0xb2, value, NULL, 0, 1000);
+ if (result < 0)
+ goto error;
+ msleep(5);
+
+error:
+ return result;
+}
+
+static int sur40_set_preprocessor(struct sur40_state *dev, u8 value)
+{
+ u8 setting_07[2] = { 0x01, 0x00 };
+ u8 setting_17[2] = { 0x85, 0x80 };
+ int result;
+
+ if (value > 1)
+ return -ERANGE;
+
+ result = usb_control_msg(dev->usbdev, usb_sndctrlpipe(dev->usbdev, 0),
+ SUR40_POKE, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
+ 0x07, setting_07[value], NULL, 0, 1000);
+ if (result < 0)
+ goto error;
+ msleep(5);
+
+ result = usb_control_msg(dev->usbdev, usb_sndctrlpipe(dev->usbdev, 0),
+ SUR40_POKE, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT,
+ 0x17, setting_17[value], NULL, 0, 1000);
+ if (result < 0)
+ goto error;
+ msleep(5);
+
+error:
+ return result;
+}
+
+static void sur40_set_vsvideo(struct sur40_state *handle, u8 value)
+{
+ int i;
+
+ for (i = 0; i < 4; i++)
+ sur40_poke(handle, 0x1c+i, value);
+}
+
+static void sur40_set_irlevel(struct sur40_state *handle, u8 value)
+{
+ int i;
+
+ for (i = 0; i < 8; i++)
+ sur40_poke(handle, 0x08+(2*i), value);
+}
+
/* Initialization routine, called from sur40_open */
static int sur40_init(struct sur40_state *dev)
{
--
2.7.4
^ permalink raw reply related
* [PATCH 2/5] add video control register definitions
From: Florian Echtler @ 2018-02-05 14:29 UTC (permalink / raw)
To: linux-media, hverkuil; +Cc: linux-input, modin, Florian Echtler
In-Reply-To: <1517840981-12280-1-git-send-email-floe@butterbrot.org>
Signed-off-by: Florian Echtler <floe@butterbrot.org>
---
drivers/input/touchscreen/sur40.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index 8375b06..0dbb004 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -149,6 +149,30 @@ struct sur40_image_header {
#define SUR40_TOUCH 0x02
#define SUR40_TAG 0x04
+/* video controls */
+#define SUR40_BRIGHTNESS_MAX 0xFF
+#define SUR40_BRIGHTNESS_MIN 0x00
+#define SUR40_BRIGHTNESS_DEF 0xFF
+
+#define SUR40_CONTRAST_MAX 0x0F
+#define SUR40_CONTRAST_MIN 0x00
+#define SUR40_CONTRAST_DEF 0x0A
+
+#define SUR40_GAIN_MAX 0x09
+#define SUR40_GAIN_MIN 0x00
+#define SUR40_GAIN_DEF 0x08
+
+#define SUR40_VSVIDEO_DEF 0x86
+
+#define SUR40_BACKLIGHT_MAX 0x01
+#define SUR40_BACKLIGHT_MIN 0x00
+#define SUR40_BACKLIGHT_DEF 0x01
+
+int sur40_v4l2_brightness = SUR40_BRIGHTNESS_DEF; /* infrared */
+int sur40_v4l2_contrast = SUR40_CONTRAST_DEF; /* blacklevel */
+int sur40_v4l2_gain = SUR40_GAIN_DEF; /* gain */
+int sur40_v4l2_backlight = 1; /* preprocessor */
+
static const struct v4l2_pix_format sur40_pix_format[] = {
{
.pixelformat = V4L2_TCH_FMT_TU08,
--
2.7.4
^ permalink raw reply related
* [PATCH 0/5] [RFC] add video controls to SUR40 driver
From: Florian Echtler @ 2018-02-05 14:29 UTC (permalink / raw)
To: linux-media, hverkuil; +Cc: linux-input, modin
The SUR40 (aka Pixelsense) has internal registers that expose sensor
parameters such as brightness, gain etc. This patch creates V4L2
control items and maps them to the appropriate parameters.
This is an initial submission for review, comments welcome!
Best regards, Florian
^ permalink raw reply
* Re: [PATCH] Input: synaptics - Lenovo Carbon X1 Gen5 (2017) devices should use RMI
From: Damjan Georgievski @ 2018-02-05 11:56 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
In-Reply-To: <20180202205050.i5pkl3rji45oovxb@dtor-ws>
>> >>> Second issues is, trackpoint is reported as "ImPS/2 Generic Wheel
>> >>> Mouse". Middle button scrolling is not enabled by default.
>> >>
>> >> I think this will be fixed by:
>> >>
>> >> https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git/patch/?id=ec667683c532c93fb41e100e5d61a518971060e2
>> >>
>> >
>> > Interesting, with this patch, the trackpoint appeared sooner, but
>> > still late (at the 44 second mark):
>>
>> but that seems sporadic. on second boot, it didn't show up until I
>> reloaded the psmouse module
>>
>> any ideas how to debug this issue further? possibly before the change
>> goes upstream.
>
> Damjan,
>
> Since my other patches seem to have fixed the trackpoint detection
> issue, mind giving your "tested-by" to this patch so I can get it in?
let me just run some more tests and I'll get back to you soon.
I'm getting some weird clicks on the trackpad, I want to compare with
vanilla kernel.
Dimitry, what's best for testing, 4.15.1 + your patches or should I use 4.16rc1?
--
damjan
^ permalink raw reply
* Re: ALPS touchpad ot correctly recognized: GlidePoint vs DualPoint
From: Juanito @ 2018-02-05 11:19 UTC (permalink / raw)
To: Pali Rohár
Cc: Masaki Ota, Dmitry Torokhov, linux-input@vger.kernel.org,
Paul Donohue
In-Reply-To: <131d0bb6-4861-95a0-3963-790c91cb3912@posteo.net>
[-- Attachment #1.1: Type: text/plain, Size: 3995 bytes --]
On 02/05/2018 08:48 AM, Juanito wrote:
> On 02/04/2018 09:21 PM, Pali Rohár wrote:
>> On Sunday 04 February 2018 20:39:06 Juanito wrote:
>>> On 02/04/2018 07:16 PM, Pali Rohár wrote:
>>>> On Monday 11 September 2017 13:26:30 Juanito wrote:
>>>>> Hi Masaki Ota,
>>>>>
>>>>> Thanks!
>>>>>
>>>>> On 11.09.2017 04:38, Masaki Ota wrote:
>>>>>> Hi, Juanito,
>>>>>>
>>>>>> In my information, ALPS Touchpad is used on Thinkpad E series and L series.
>>>>>> I don't know the device that is ALPS Touchpad + other vendor TrackStick.
>>>>>> But Lenovo might use ALPS Touchpad on such a combination.
>>>>>>
>>>>>
>>>>> Well, that is probably my fault, as it is not the original touchpad that
>>>>> was delievered with the laptop. I bought the touchpad (that included
>>>>> the three buttons) separately because I didn't like the clickpad that
>>>>> came with my laptop.
>>>>
>>>> Hi Juanito,
>>>>
>>>
>>> Hi Pali,
>>>
>>>> if you are still want to play with your touchpad hardware, I have a good
>>>> news for your.
>>>>
>>>
>>> Yeah! I'd love to get it to work! It's just I've been "working" on some
>>> other projects and my last kernel builds didn't work at all so I gave up
>>> and never got back to it.
>>>
>>> Thank you very much for that!
>>>
>>>> It looks like that at least ALPS rushmore touchpads allow to receive RAW
>>>> PS/2 packets from trackstick to host kernel, without modifying them by
>>>> touchpad. Plus I was able to tell ALPS touchpad to start "mixing" those
>>>> RAW trackstick PS/2 packets with native touchpad packets.
>>>>
>>>> On my configuration trackstick in RAW PS/2 mode by default talks with
>>>> standard bare 3 byte PS/2 protocol and touchpad in 6 byte ALPS protocol.
>>>> alps.c/psmouse.ko is already able to process and parse such mixed
>>>> packets. And trackstick can be switched to some extended 4 byte
>>>> protocol...
>>>>
>>>> All this happen when passthrough mode is enabled.
>>>>
>>>> From my understanding it seems that in normal mode, touchpad and
>>>> trackstick communicate with that 4 byte protocol and touchpad converts
>>>> it into 6 byte ALPS protocol and then send to kernel.
>>>>
>>>> On thinkpads trackstick communicate with TPPS/2 protcol and above 4
>>>> byte. So in my opinion ALPS touchpad by default cannot understand it.
>>>> But you should be able to enter passthrough mode and then you would
>>>> receive that TPPS/2 in alps kernel code.
>>>>
>>>
>>> I don't really understand what you mean. Do I "just" have to set it to
>>> passthrough mode? How exactly can I do that?
>>
>> Look at function alps_passthrough_mode_v3(). And try to call it after
>> touchpad is initialized.
>>
>
> Cool, I'll give this a try!
>
>> You can also look which alps_command_mode_write_reg() functions are
>> called for your touchpad and maybe try to figure out what those the
>> registers can enabled/disable.
>>
>> On http://www.cirque.com/gen4-dev-resources you can find document named
>> GP-AN- 130823 INTERFACING TO GEN4 OVER I2C (PDF) which contains
>> some description of those registers (in section 7).
>>
>> Register C2C8, bit 0 has description "PS2AuxControl.CommandPassThruEnabled"
>>
>> If trackstick is not detected, it seems you can "force" enable it by
>> setting bit 7 "PS2AuxControl.AuxDevicePresent" in same register.
>>
>>> Again, thank you very much!
>>
>> Have you tried to read from that register which Masaki Ota talked in
>> previous emails?
>>
>
> I tried but I just didn't manage to get it to work, at all and gave up
> (I really am a newbie in this whole kernel world). Maybe a fresh start
> helps here.
>
Oh, oh! Currently, my touchpad is just being recognized as a PS/2
Generic Mouse.
On both debian stretch kernel (4.9.0-5-amd64) and on a 4.15.0+ compiled
by me.
I'll try to see if I can find out why this is happening.
Do have any idea why this might be the case (it used to be recognized as
ALPS)?
Cheers,
Juanito
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] input: touchscreen: ar1021_i2c: enable touch mode during open
From: Martin Kepplinger @ 2018-02-05 10:40 UTC (permalink / raw)
To: Christian Gmeiner; +Cc: Dmitry Torokhov, linux-input, LKML
In-Reply-To: <CAH9NwWeom2H89D5hUixxSL28YgDna_WS26boayzUBAPyVFUvTA@mail.gmail.com>
Martin Kepplinger | Entwicklung Software
GINZINGER ELECTRONIC SYSTEMS GMBH
Tel.: +43 7723 5422 157
Mail: martin.kepplinger@ginzinger.com
Web: www.ginzinger.com
On 2018-02-05 11:07, Christian Gmeiner wrote:
> Hi all.
>
> 2017-04-27 14:22 GMT+02:00 Martin Kepplinger <martin.kepplinger@ginzinger.com>:
>> The device could as well be in command mode, in which this driver cannot
>> handle the device. When opening the device, let's make sure the device
>> will be in the mode we expect it to be for this driver.
>>
>
> I run into issues caused by this change. It turns out that the device
> is non-functional
> after some warm-reboots and as a result I am not able to use xorg's
> evdev driver.
> So I have some questions about this change:
>
> * Should we enable irq before calling i2c_master_send(..) as the chip raises an
> irq if the command was processed?
>
> * Would it be enough to send this command only once during driver
> lifetime? I can
> see that on my system open gets called 3 times during boot-up.
It would. See below for my thought on this change.
>
> * What are the circumstances the touch device would be in an other state? In the
> official kernel driver the userspace can send commands via sysfs.
> Also the driver
> does set the touch enable mode as this patch does.
I did this change as the device was once non-functional unexpectedly
because it wasn't in touch mode. We can set touch mode during open() or
probe() but I figured during open() would keep the driver working even
when others would use the device in command mode.
Does your problem go away when you revert this change or put it into
probe()?
martin
________________________________________
Ginzinger electronic systems GmbH
Gewerbegebiet Pirath 16
4952 Weng im Innkreis
www.ginzinger.com
Firmenbuchnummer: FN 364958d
Firmenbuchgericht: Ried im Innkreis
UID-Nr.: ATU66521089
^ permalink raw reply
* Re: [PATCH 1/2] input: touchscreen: ar1021_i2c: enable touch mode during open
From: Christian Gmeiner @ 2018-02-05 10:07 UTC (permalink / raw)
To: Martin Kepplinger; +Cc: Dmitry Torokhov, linux-input, LKML
In-Reply-To: <1493295756-17812-1-git-send-email-martin.kepplinger@ginzinger.com>
Hi all.
2017-04-27 14:22 GMT+02:00 Martin Kepplinger <martin.kepplinger@ginzinger.com>:
> The device could as well be in command mode, in which this driver cannot
> handle the device. When opening the device, let's make sure the device
> will be in the mode we expect it to be for this driver.
>
I run into issues caused by this change. It turns out that the device
is non-functional
after some warm-reboots and as a result I am not able to use xorg's
evdev driver.
So I have some questions about this change:
* Should we enable irq before calling i2c_master_send(..) as the chip raises an
irq if the command was processed?
* Would it be enough to send this command only once during driver
lifetime? I can
see that on my system open gets called 3 times during boot-up.
* What are the circumstances the touch device would be in an other state? In the
official kernel driver the userspace can send commands via sysfs.
Also the driver
does set the touch enable mode as this patch does.
> Signed-off-by: Martin Kepplinger <martin.kepplinger@ginzinger.com>
> ---
> drivers/input/touchscreen/ar1021_i2c.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/input/touchscreen/ar1021_i2c.c b/drivers/input/touchscreen/ar1021_i2c.c
> index 1a94d8b..2a76231 100644
> --- a/drivers/input/touchscreen/ar1021_i2c.c
> +++ b/drivers/input/touchscreen/ar1021_i2c.c
> @@ -18,6 +18,12 @@
> #define AR1021_MAX_X 4095
> #define AR1021_MAX_Y 4095
>
> +#define AR1021_CMD 0x55
> +#define AR1021_TOUCH 0x80
> +
> +#define AR1021_CMD_ENABLE_TOUCH 0x12
> +#define AR1021_CMD_DISABLE_TOUCH 0x13
> +
> struct ar1021_i2c {
> struct i2c_client *client;
> struct input_dev *input;
> @@ -58,6 +64,15 @@ static int ar1021_i2c_open(struct input_dev *dev)
> {
> struct ar1021_i2c *ar1021 = input_get_drvdata(dev);
> struct i2c_client *client = ar1021->client;
> + int error;
> + u8 cmd_enable_touch[3] = {AR1021_CMD,
> + 0x01, /* number of bytes after this */
> + AR1021_CMD_ENABLE_TOUCH };
> +
> + error = i2c_master_send(ar1021->client, cmd_enable_touch,
> + sizeof(cmd_enable_touch));
> + if (error < 0)
> + return error;
>
> enable_irq(client->irq);
>
> --
> 2.1.4
>
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info
^ 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