* Re: uinput: waiting for UI_FF_UPLOAD events will not inform user when allocation is required
From: Dmitry Torokhov @ 2023-10-13 21:59 UTC (permalink / raw)
To: John Salamon; +Cc: rydberg, linux-input, linux-kernel
In-Reply-To: <CA+fyA4RABYNPZZSk9+9U51u53kbSzqgwdi1KDDGRxXi8q5TtxQ@mail.gmail.com>
Hi John,
On Tue, Oct 10, 2023 at 05:38:27PM +1030, John Salamon wrote:
> Currently the "fake" input events generated by uinput in response to
> effect uploads will return an effect with an id that has already been
> handled by input_ff_upload in ff-core.c, which can modify the effect
> id. This causes a problem specifically when the effect originally
> uploaded via the EVIOCSFF ioctl contained an effect with -1, as the
> userspace code handling UI_FF_UPLOAD receives an effect with an id
> other than -1, and therefore will not know an allocation was
> requested.
The kernel never changes ID of an existing effect, the only time ID is
changed is when userspace indicates that a new effect should be created
by setting effect ID to -1.
The handler of force feedback effects should know what effects (with
what IDs) have been uploaded to the device so far, so whenever it sees a
request for an effect with previously unseen effect_id it should
recognize this as a signal that a new effect/id has been allocated by
the kernel.
>
> I notice that the "old" field on the ff_effect struct is set to NULL
> when the -1 id is changed (in input_ff_upload), which can serve as a
> flag that an allocation was requested. If it is the intention is that
> uinput users check if old == NULL to know when allocations are needed
> I think uinput documentation should describe this.
No, not really, as explained above.
>
> I first noticed this using python-evdev, see my issue report here:
> https://github.com/gvalkov/python-evdev/issues/199
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] Input: bcm5974 - check endpoint type before starting traffic
From: Dmitry Torokhov @ 2023-10-14 3:03 UTC (permalink / raw)
To: Javier Carrasco
Cc: John Horan, Henrik Rydberg, linux-input, linux-kernel,
syzbot+348331f63b034f89b622
In-Reply-To: <20231007-topic-bcm5974_bulk-v2-1-021131c83efb@gmail.com>
Hi Javier,
On Thu, Oct 12, 2023 at 06:51:49PM +0200, Javier Carrasco wrote:
>
> +static bool bcm5974_ep_is_int_in(struct usb_host_interface *iface, int addr)
> +{
> + struct usb_endpoint_descriptor *endpoint;
> + int i;
> +
> + for (i = 0; i < iface->desc.bNumEndpoints; i++) {
> + endpoint = &iface->endpoint[i].desc;
> + if (endpoint->bEndpointAddress == addr) {
> + if (usb_endpoint_is_int_in(endpoint))
> + return true;
> + }
> + }
> + return false;
> +}
This essentially reimplements usb_find_endpoint() in a sense, so can we
instead do:
ep = usb_find_endpoint(iface, addr);
if (!ep || !usb_endpoint_is_int_in(ep)) {
dev_err(...);
return ...;
}
Also it looks like the handling of button endpoint is interleaved with
the trackpad endpoint, I wonder if it would not be better if we have a
separate "if (cfg->tp_type == TYPE1)" where we would do the check,
allocate URB, and did all the rest of set up for button transfers.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: Annotate struct ff_device with __counted_by
From: Dmitry Torokhov @ 2023-10-14 3:04 UTC (permalink / raw)
To: Kees Cook
Cc: Gustavo A. R. Silva, linux-input, linux-hardening,
Nathan Chancellor, Nick Desaulniers, Tom Rix, linux-kernel, llvm
In-Reply-To: <20231006201739.work.350-kees@kernel.org>
On Fri, Oct 06, 2023 at 01:17:39PM -0700, Kees Cook wrote:
> Prepare for the coming implementation by GCC and Clang of the __counted_by
> attribute. Flexible array members annotated with __counted_by can have
> their accesses bounds-checked at run-time via CONFIG_UBSAN_BOUNDS (for
> array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
> functions).
>
> As found with Coccinelle[1], add __counted_by for struct ff_device.
>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> Cc: linux-input@vger.kernel.org
> Cc: linux-hardening@vger.kernel.org
> Link: https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci [1]
> Signed-off-by: Kees Cook <keescook@chromium.org>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: tegra: Use device_get_match_data()
From: Dmitry Torokhov @ 2023-10-14 3:09 UTC (permalink / raw)
To: Rob Herring
Cc: Laxman Dewangan, Thierry Reding, Jonathan Hunter, linux-input,
linux-tegra, linux-kernel
In-Reply-To: <20231006224432.442709-1-robh@kernel.org>
On Fri, Oct 06, 2023 at 05:44:32PM -0500, Rob Herring wrote:
> Use preferred device_get_match_data() instead of of_match_device() to
> get the driver match data. With this, adjust the includes to explicitly
> include the correct headers.
>
> Signed-off-by: Rob Herring <robh@kernel.org>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3] Input: powermate - fix use-after-free in powermate_config_complete
From: Dmitry Torokhov @ 2023-10-14 3:13 UTC (permalink / raw)
To: Javier Carrasco; +Cc: linux-input, linux-kernel, syzbot+0434ac83f907a1dbdd1e
In-Reply-To: <20230916-topic-powermate_use_after_free-v3-1-64412b81a7a2@gmail.com>
On Wed, Oct 04, 2023 at 04:36:13PM +0200, Javier Carrasco wrote:
> syzbot has found a use-after-free bug [1] in the powermate driver. This
> happens when the device is disconnected, which leads to a memory free
> from the powermate_device struct.
> When an asynchronous control message completes after the kfree and its
> callback is invoked, the lock does not exist anymore and hence the bug.
>
> Use usb_kill_urb() on pm->config to cancel any in-progress requests upon
> device disconnection.
>
> [1] https://syzkaller.appspot.com/bug?extid=0434ac83f907a1dbdd1e
>
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> Reported-by: syzbot+0434ac83f907a1dbdd1e@syzkaller.appspotmail.com
Applied, thank you.
--
Dmitry
^ permalink raw reply
* [git pull] Input updates for v6.6-rc5
From: Dmitry Torokhov @ 2023-10-14 4:15 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 tags/input-for-v6.6-rc5
to receive updates for the input subsystem. You will get:
- a reworked way for handling reset delay on SMBus-connected Synaptics
touchpads (the original one, while being correct, uncovered an old bug
in fallback to PS/2 code that was fixed separately; the new one however
avoids having delay in serio port "fast" resume, and instead has the
wait in the RMI4 code)
- a fix for potential crashes when devices with Elan controllers (and
Synaptics) fall back to PS/2 code. Can't be hit without the original
patch above, but still good to have it fixed
- a couple new device IDs in xpad Xbox driver
- another quirk for Goodix driver to deal with stuff vendors put in ACPI
tables
- a fix for use-after-free on disconnect for powermate driver
- a quirk to not initialize PS/2 mouse port on Fujitsu Lifebook E5411
laptop as it makes keyboard not usable and the device uses
hid-over-i2c touchpad anyways.
Changelog:
---------
Dmitry Torokhov (2):
Revert "Input: psmouse - add delay when deactivating for SMBus mode"
Input: synaptics-rmi4 - handle reset delay when using SMBus trsnsport
Hans de Goede (1):
Input: goodix - ensure int GPIO is in input for gpio_count == 1 && gpio_int_idx == 0 case
Javier Carrasco (1):
Input: powermate - fix use-after-free in powermate_config_complete
Jeffery Miller (1):
Input: psmouse - fix fast_reconnect function for PS/2 mode
Matthias Berndt (1):
Input: xpad - add PXN V900 support
Max Nguyen (1):
Input: xpad - add HyperX Clutch Gladiate Support
Szilard Fabian (1):
Input: i8042 - add Fujitsu Lifebook E5411 to i8042 quirk table
Diffstat:
--------
drivers/input/joystick/xpad.c | 4 +++
drivers/input/misc/powermate.c | 1 +
drivers/input/mouse/elantech.c | 1 +
drivers/input/mouse/psmouse-smbus.c | 19 +++++--------
drivers/input/mouse/synaptics.c | 2 ++
drivers/input/rmi4/rmi_smbus.c | 50 ++++++++++++++++++++---------------
drivers/input/serio/i8042-acpipnpio.h | 8 ++++++
drivers/input/touchscreen/goodix.c | 19 +++++++++++++
8 files changed, 70 insertions(+), 34 deletions(-)
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3 09/16] platform/x86/amd/pmf: Add facility to dump TA inputs
From: kernel test robot @ 2023-10-14 4:16 UTC (permalink / raw)
To: Shyam Sundar S K, hdegoede, markgross, ilpo.jarvinen,
basavaraj.natikar, jikos, benjamin.tissoires, alexander.deucher,
christian.koenig, Xinhui.Pan, airlied, daniel
Cc: oe-kbuild-all, Shyam Sundar S K, amd-gfx, platform-driver-x86,
dri-devel, Patil.Reddy, linux-input, mario.limonciello
In-Reply-To: <20231010125917.138225-10-Shyam-sundar.S-k@amd.com>
Hi Shyam,
kernel test robot noticed the following build warnings:
[auto build test WARNING on hid/for-next]
[also build test WARNING on drm-misc/drm-misc-next linus/master v6.6-rc5 next-20231013]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Shyam-Sundar-S-K/platform-x86-amd-pmf-Add-PMF-TEE-interface/20231010-210347
base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link: https://lore.kernel.org/r/20231010125917.138225-10-Shyam-sundar.S-k%40amd.com
patch subject: [PATCH v3 09/16] platform/x86/amd/pmf: Add facility to dump TA inputs
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20231014/202310141247.22Coajca-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231014/202310141247.22Coajca-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310141247.22Coajca-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from include/linux/printk.h:564,
from include/linux/kernel.h:30,
from arch/x86/include/asm/percpu.h:27,
from arch/x86/include/asm/preempt.h:6,
from include/linux/preempt.h:79,
from include/linux/spinlock.h:56,
from include/linux/mmzone.h:8,
from include/linux/gfp.h:7,
from include/linux/slab.h:16,
from include/linux/resource_ext.h:11,
from include/linux/acpi.h:13,
from drivers/platform/x86/amd/pmf/pmf.h:14,
from drivers/platform/x86/amd/pmf/sps.c:11:
drivers/platform/x86/amd/pmf/sps.c: In function 'amd_pmf_dump_sps_defaults':
drivers/platform/x86/amd/pmf/sps.c:50:65: error: implicit declaration of function 'source_as_str' [-Werror=implicit-function-declaration]
50 | pr_debug("--- Source:%s Mode:%s ---\n", source_as_str(i), slider_as_str(j));
| ^~~~~~~~~~~~~
include/linux/dynamic_debug.h:224:29: note: in definition of macro '__dynamic_func_call_cls'
224 | func(&id, ##__VA_ARGS__); \
| ^~~~~~~~~~~
include/linux/dynamic_debug.h:250:9: note: in expansion of macro '_dynamic_func_call_cls'
250 | _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call'
269 | _dynamic_func_call(fmt, __dynamic_pr_debug, \
| ^~~~~~~~~~~~~~~~~~
include/linux/printk.h:579:9: note: in expansion of macro 'dynamic_pr_debug'
579 | dynamic_pr_debug(fmt, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~
drivers/platform/x86/amd/pmf/sps.c:50:25: note: in expansion of macro 'pr_debug'
50 | pr_debug("--- Source:%s Mode:%s ---\n", source_as_str(i), slider_as_str(j));
| ^~~~~~~~
>> drivers/platform/x86/amd/pmf/sps.c:50:34: warning: format '%s' expects argument of type 'char *', but argument 3 has type 'int' [-Wformat=]
50 | pr_debug("--- Source:%s Mode:%s ---\n", source_as_str(i), slider_as_str(j));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/printk.h:345:21: note: in definition of macro 'pr_fmt'
345 | #define pr_fmt(fmt) fmt
| ^~~
include/linux/dynamic_debug.h:248:9: note: in expansion of macro '__dynamic_func_call_cls'
248 | __dynamic_func_call_cls(__UNIQUE_ID(ddebug), cls, fmt, func, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:250:9: note: in expansion of macro '_dynamic_func_call_cls'
250 | _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call'
269 | _dynamic_func_call(fmt, __dynamic_pr_debug, \
| ^~~~~~~~~~~~~~~~~~
include/linux/printk.h:579:9: note: in expansion of macro 'dynamic_pr_debug'
579 | dynamic_pr_debug(fmt, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~
drivers/platform/x86/amd/pmf/sps.c:50:25: note: in expansion of macro 'pr_debug'
50 | pr_debug("--- Source:%s Mode:%s ---\n", source_as_str(i), slider_as_str(j));
| ^~~~~~~~
drivers/platform/x86/amd/pmf/sps.c:50:47: note: format string is defined here
50 | pr_debug("--- Source:%s Mode:%s ---\n", source_as_str(i), slider_as_str(j));
| ~^
| |
| char *
| %d
cc1: some warnings being treated as errors
vim +50 drivers/platform/x86/amd/pmf/sps.c
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 41
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 42 static void amd_pmf_dump_sps_defaults(struct amd_pmf_static_slider_granular *data)
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 43 {
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 44 int i, j;
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 45
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 46 pr_debug("Static Slider Data - BEGIN\n");
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 47
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 48 for (i = 0; i < POWER_SOURCE_MAX; i++) {
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 49 for (j = 0; j < POWER_MODE_MAX; j++) {
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 @50 pr_debug("--- Source:%s Mode:%s ---\n", source_as_str(i), slider_as_str(j));
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 51 pr_debug("SPL: %u mW\n", data->prop[i][j].spl);
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 52 pr_debug("SPPT: %u mW\n", data->prop[i][j].sppt);
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 53 pr_debug("SPPT_ApuOnly: %u mW\n", data->prop[i][j].sppt_apu_only);
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 54 pr_debug("FPPT: %u mW\n", data->prop[i][j].fppt);
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 55 pr_debug("STTMinLimit: %u mW\n", data->prop[i][j].stt_min);
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 56 pr_debug("STT_SkinTempLimit_APU: %u C\n",
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 57 data->prop[i][j].stt_skin_temp[STT_TEMP_APU]);
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 58 pr_debug("STT_SkinTempLimit_HS2: %u C\n",
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 59 data->prop[i][j].stt_skin_temp[STT_TEMP_HS2]);
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 60 }
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 61 }
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 62
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 63 pr_debug("Static Slider Data - END\n");
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 64 }
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 65 #else
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 66 static void amd_pmf_dump_sps_defaults(struct amd_pmf_static_slider_granular *data) {}
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 67 #endif
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 68
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [git pull] Input updates for v6.6-rc5
From: pr-tracker-bot @ 2023-10-14 6:32 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Linus Torvalds, linux-kernel, linux-input
In-Reply-To: <ZSoV4d4p8RKxEVeG@google.com>
The pull request you sent on Fri, 13 Oct 2023 21:15:29 -0700:
> git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git tags/input-for-v6.6-rc5
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/727fb83765049981e342db4c5a8b51aca72201d8
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: uinput: waiting for UI_FF_UPLOAD events will not inform user when allocation is required
From: John Salamon @ 2023-10-14 6:32 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: rydberg, linux-input, linux-kernel
In-Reply-To: <ZSm9zeepuZ94A-ZI@google.com>
Hi Dmitry,
Thank you for the clarification.
> The handler of force feedback effects should know what effects (with
> what IDs) have been uploaded to the device so far, so whenever it sees a
> request for an effect with previously unseen effect_id it should
> recognize this as a signal that a new effect/id has been allocated by
> the kernel.
In that case, would you consider accepting a patch adding a sentence
or two explaining that handlers will need to keep track of effect IDs
in this manner? Either in uinput documentation or just in the comment
describing how to implement upload_effect() in uinput.h. Something
along the lines of "Effects received here will never have an ID of -1.
Handlers of these effects must recognize previously unseen effect IDs
to know when a new effect has been allocated.".
Cheers,
John
On Sat, Oct 14, 2023 at 8:29 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> Hi John,
>
> On Tue, Oct 10, 2023 at 05:38:27PM +1030, John Salamon wrote:
> > Currently the "fake" input events generated by uinput in response to
> > effect uploads will return an effect with an id that has already been
> > handled by input_ff_upload in ff-core.c, which can modify the effect
> > id. This causes a problem specifically when the effect originally
> > uploaded via the EVIOCSFF ioctl contained an effect with -1, as the
> > userspace code handling UI_FF_UPLOAD receives an effect with an id
> > other than -1, and therefore will not know an allocation was
> > requested.
>
> The kernel never changes ID of an existing effect, the only time ID is
> changed is when userspace indicates that a new effect should be created
> by setting effect ID to -1.
>
> The handler of force feedback effects should know what effects (with
> what IDs) have been uploaded to the device so far, so whenever it sees a
> request for an effect with previously unseen effect_id it should
> recognize this as a signal that a new effect/id has been allocated by
> the kernel.
>
> >
> > I notice that the "old" field on the ff_effect struct is set to NULL
> > when the -1 id is changed (in input_ff_upload), which can serve as a
> > flag that an allocation was requested. If it is the intention is that
> > uinput users check if old == NULL to know when allocations are needed
> > I think uinput documentation should describe this.
>
> No, not really, as explained above.
>
> >
> > I first noticed this using python-evdev, see my issue report here:
> > https://github.com/gvalkov/python-evdev/issues/199
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* Re: [PATCH v2] Input: bcm5974 - check endpoint type before starting traffic
From: Javier Carrasco @ 2023-10-14 7:57 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: John Horan, Henrik Rydberg, linux-input, linux-kernel,
syzbot+348331f63b034f89b622
In-Reply-To: <ZSoFDLv8_CG1SGN6@google.com>
Hi Dmitry,
On 14.10.23 05:03, Dmitry Torokhov wrote:
> Hi Javier,
>
> On Thu, Oct 12, 2023 at 06:51:49PM +0200, Javier Carrasco wrote:
>>
>> +static bool bcm5974_ep_is_int_in(struct usb_host_interface *iface, int addr)
>> +{
>> + struct usb_endpoint_descriptor *endpoint;
>> + int i;
>> +
>> + for (i = 0; i < iface->desc.bNumEndpoints; i++) {
>> + endpoint = &iface->endpoint[i].desc;
>> + if (endpoint->bEndpointAddress == addr) {
>> + if (usb_endpoint_is_int_in(endpoint))
>> + return true;
>> + }
>> + }
>> + return false;
>> +}
>
> This essentially reimplements usb_find_endpoint() in a sense, so can we
> instead do:
>
> ep = usb_find_endpoint(iface, addr);
> if (!ep || !usb_endpoint_is_int_in(ep)) {
> dev_err(...);
> return ...;
> }
>
Thanks for your feedback. usb_find_endpoint is a static function from
the usb core and in principle is not available here, but your code
snippet seems to reinterpret usb_check_int_endpoints() for a single
address. I would suggest using usb_check_int_endpoints and pass both
addresses at the same time (if both exist, of course).
>
> Also it looks like the handling of button endpoint is interleaved with
> the trackpad endpoint, I wonder if it would not be better if we have a
> separate "if (cfg->tp_type == TYPE1)" where we would do the check,
> allocate URB, and did all the rest of set up for button transfers.
Using usb_check_int_endpoints would solve this immediately.
>
> Thanks.
>
Thanks and best regards,
Javier Carrasco
^ permalink raw reply
* [PATCH v3] Input: bcm5974 - check endpoint type before starting traffic
From: Javier Carrasco @ 2023-10-14 10:20 UTC (permalink / raw)
To: John Horan, Henrik Rydberg, Dmitry Torokhov
Cc: linux-input, linux-kernel, Javier Carrasco,
syzbot+348331f63b034f89b622
syzbot has found a type mismatch between a USB pipe and the transfer
endpoint, which is triggered by the bcm5974 driver[1].
This driver expects the device to provide input interrupt endpoints and
if that is not the case, the driver registration should terminate.
Repros are available to reproduce this issue with a certain setup for
the dummy_hcd, leading to an interrupt/bulk mismatch which is caught in
the USB core after calling usb_submit_urb() with the following message:
"BOGUS urb xfer, pipe 1 != type 3"
Some other device drivers (like the appletouch driver bcm5974 is mainly
based on) provide some checking mechanism to make sure that an IN
interrupt endpoint is available. In this particular case the endpoint
addresses are provided by a config table, so the checking can be
targeted to the provided endpoints.
Add some basic checking to guarantee that the endpoints available match
the expected type for both the trackpad and button endpoints.
This issue was only found for the trackpad endpoint, but the checking
has been added to the button endpoint as well for the same reasons.
Given that there was never a check for the endpoint type, this bug has
been there since the first implementation of the driver (f89bd95c5c94).
[1] https://syzkaller.appspot.com/bug?extid=348331f63b034f89b622
Fixes: f89bd95c5c94 ("Input: bcm5974 - add driver for Macbook Air and Pro Penryn touchpads")
Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
Reported-and-tested-by: syzbot+348331f63b034f89b622@syzkaller.appspotmail.com
---
Changes in v3:
- Use usb_check_int_endpoints() to validate the endpoints.
- Link to v2: https://lore.kernel.org/r/20231007-topic-bcm5974_bulk-v2-1-021131c83efb@gmail.com
Changes in v2:
- Keep error = -ENOMEM for the rest of the probe and return -ENODEV if
the endpoint check fails.
- Check function returns now bool and was renamed (_is_ for
bool-returning functions).
- Link to v1: https://lore.kernel.org/r/20231007-topic-bcm5974_bulk-v1-1-355be9f8ad80@gmail.com
---
drivers/input/mouse/bcm5974.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/input/mouse/bcm5974.c b/drivers/input/mouse/bcm5974.c
index ca150618d32f..953992b458e9 100644
--- a/drivers/input/mouse/bcm5974.c
+++ b/drivers/input/mouse/bcm5974.c
@@ -19,6 +19,7 @@
* Copyright (C) 2006 Nicolas Boichat (nicolas@boichat.ch)
*/
+#include "linux/usb.h"
#include <linux/kernel.h>
#include <linux/errno.h>
#include <linux/slab.h>
@@ -193,6 +194,8 @@ enum tp_type {
/* list of device capability bits */
#define HAS_INTEGRATED_BUTTON 1
+/* maximum number of supported endpoints (currently trackpad and button) */
+#define MAX_ENDPOINTS 2
/* trackpad finger data block size */
#define FSIZE_TYPE1 (14 * sizeof(__le16))
@@ -891,6 +894,18 @@ static int bcm5974_resume(struct usb_interface *iface)
return error;
}
+static bool bcm5974_check_endpoints(struct usb_interface *iface,
+ const struct bcm5974_config *cfg)
+{
+ u8 ep_addr[MAX_ENDPOINTS + 1] = {0};
+
+ ep_addr[0] = cfg->tp_ep;
+ if (cfg->tp_type == TYPE1)
+ ep_addr[1] = cfg->bt_ep;
+
+ return usb_check_int_endpoints(iface, ep_addr);
+}
+
static int bcm5974_probe(struct usb_interface *iface,
const struct usb_device_id *id)
{
@@ -903,6 +918,11 @@ static int bcm5974_probe(struct usb_interface *iface,
/* find the product index */
cfg = bcm5974_get_config(udev);
+ if (!bcm5974_check_endpoints(iface, cfg)) {
+ dev_err(&iface->dev, "Unexpected non-int endpoint\n");
+ return -ENODEV;
+ }
+
/* allocate memory for our device state and initialize it */
dev = kzalloc(sizeof(struct bcm5974), GFP_KERNEL);
input_dev = input_allocate_device();
---
base-commit: 401644852d0b2a278811de38081be23f74b5bb04
change-id: 20231007-topic-bcm5974_bulk-c66b743ba7ba
Best regards,
--
Javier Carrasco <javier.carrasco.cruz@gmail.com>
^ permalink raw reply related
* Help getting ELAN066C touchpad working in Linux
From: jdieter @ 2023-10-14 17:12 UTC (permalink / raw)
To: Dmitry Torokhov, Hans de Goede, linux-input
[-- Attachment #1: Type: text/plain, Size: 2238 bytes --]
Hi Dmitry, Hans, input mailing list,
I have two Lenovo 300e laptops, one running Windows and the other
Fedora. The laptop has a touchscreen, a stylus and a touchpad. The
touch screen and stylus work perfectly, but, under Fedora, the touchpad
doesn't work at all and isn't detected by the kernel.
The device shows up in /sys/bus/acpi/devices/ELAN066C:00/ with a
modalias of acpi:ELAN066C:PNP0C50:.
I'm not sure if the touchpad should be using elan_i2c or i2c_hid_acpi.
In a somewhat naive attempt to get something to happen, I added:
```
{ "ELAN066C", 0 }, /* Lenovo 100e/300e 2nd gen AMD */
```
to include/linux/input/elan-i2c-ids.h, but the kernel still doesn't
auto-load the module.
I can get elan_i2c to recognize something if I run:
# echo elan_i2c 0x15 > /sys/bus/i2c/devices/i2c-1/new_device
but the IRQ defaults to 0, and I get an error about an IRQ conflict:
```
[ 2317.791897] i2c i2c-1: delete_device: Deleting device elan_i2c at
0x15
[ 2319.317468] i2c i2c-1: new_device: Instantiated device elan_i2c at
0x15
[ 2319.317662] elan_i2c 1-0015: supply vcc not found, using dummy
regulator
[ 2319.433176] elan_i2c 1-0015: Elan Touchpad: Module ID: 0x00be,
Firmware: 0x0003, Sample: 0x0003, IAP: 0x000e
[ 2319.433184] elan_i2c 1-0015: Elan Touchpad Extra Information:
Max ABS X,Y: 3206,1828
Width X,Y: 152,152
Resolution X,Y: 31,31 (dots/mm)
ic type: 0xa
info pattern: 0x0
[ 2319.433339] genirq: Flags mismatch irq 0. 00002008 (elan_i2c) vs.
00015a00 (timer)
```
By manually setting the IRQ to an unused value in the driver, I can at
least get the driver to create a new mouse device in /dev/input, but
the touchpad still doesn't work.
At this point, I don't know if I'm really close or barking up the
completely wrong tree.
A bug report from earlier this year about an identical system is at:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1976556
Attached is the dsl of the device in ACPI.
I would really appreciate any help in figuring out how to get this
working. Please let me know if there's anything else you need from me.
Thanks,
Jonathan
[-- Attachment #2: TDP0.dsl --]
[-- Type: text/x-dsl, Size: 3654 bytes --]
Device (TPD0)
{
Name (_HID, "ELAN066C") // _HID: Hardware ID
Name (_CID, "PNP0C50" /* HID Protocol Device (I2C bus) */) // _CID: Compatible ID
ICMS = 0x0A
DCMS = 0x01
ICMS = 0x20
CDAT = DCMS /* \DCMS */
If ((CDAT == 0x02))
{
_HID = "SYNA2392"
}
If ((CDAT == 0x01))
{
_HID = "ELAN066C"
}
Name (PPRW, Package (0x02)
{
0x16,
0x03
})
Method (_PRW, 0, NotSerialized) // _PRW: Power Resources for Wake
{
Return (PPRW) /* \_SB_.I2CD.TPD0.PPRW */
}
Method (_STA, 0, NotSerialized) // _STA: Status
{
If ((CDAT == 0x00))
{
Return (0x00)
}
Else
{
Return (0x0F)
}
}
Method (_DSM, 4, Serialized) // _DSM: Device-Specific Method
{
If ((Arg0 == ToUUID ("3cdff6f7-4267-4555-ad05-b30a3d8938de") /* HID I2C Device */))
{
Switch (ToInteger (Arg2))
{
Case (0x00)
{
Switch (ToInteger (Arg1))
{
Case (0x01)
{
Return (Buffer (0x01)
{
0x03 // .
})
}
Default
{
Return (Buffer (0x01)
{
0x00 // .
})
}
}
}
Case (0x01)
{
If ((^^^PCI0.LPC0.H_EC.ECRD (RefOf (^^^PCI0.LPC0.H_EC.TPTY)) == 0x01))
{
Return (0x01)
}
If ((^^^PCI0.LPC0.H_EC.ECRD (RefOf (^^^PCI0.LPC0.H_EC.TPTY)) == 0x02))
{
Return (0x20)
}
}
Default
{
Return (0x00)
}
}
}
Else
{
Return (Buffer (0x01)
{
0x00 // .
})
}
}
Method (_CRS, 0, NotSerialized) // _CRS: Current Resource Settings
{
Name (SBFG, ResourceTemplate ()
{
GpioInt (Level, ActiveLow, ExclusiveAndWake, PullUp, 0x0000,
"\\_SB.GPIO", 0x00, ResourceConsumer, ,
)
{ // Pin list
0x0009
}
})
If ((^^^PCI0.LPC0.H_EC.ECRD (RefOf (^^^PCI0.LPC0.H_EC.TPTY)) == 0x01))
{
Name (SBFB, ResourceTemplate ()
{
I2cSerialBusV2 (0x0015, ControllerInitiated, 0x00061A80,
AddressingMode7Bit, "\\_SB.I2CD",
0x00, ResourceConsumer, , Exclusive,
)
})
Return (ConcatenateResTemplate (SBFB, SBFG))
}
If ((^^^PCI0.LPC0.H_EC.ECRD (RefOf (^^^PCI0.LPC0.H_EC.TPTY)) == 0x02))
{
Name (SBFC, ResourceTemplate ()
{
I2cSerialBusV2 (0x002C, ControllerInitiated, 0x00061A80,
AddressingMode7Bit, "\\_SB.I2CD",
0x00, ResourceConsumer, , Exclusive,
)
})
Return (ConcatenateResTemplate (SBFC, SBFG))
}
}
}
^ permalink raw reply
* Re: Help getting ELAN066C touchpad working in Linux
From: Hans de Goede @ 2023-10-14 19:39 UTC (permalink / raw)
To: jdieter, Dmitry Torokhov, linux-input
In-Reply-To: <27131a7fae2794a63f4f285a20e41116ba4198f3.camel@gmail.com>
Hi Jonathan,
For starters, did you see this in dmesg? :
[ 0.422648] i8042: PNP: PS/2 Controller [PNP0303:KBC0] at 0x60,0x64 irq 1
[ 0.422652] i8042: PNP: PS/2 appears to have AUX port disabled, if this is incorrect please boot with i8042.nopnp
And did you try booting with "i8042.nopnp" ?
Also what is the output of:
ls -l /sys/bus/i2c/devices
and of:
ls -l /sys/bus/acpi/devices/ELAN066C:00/
and of:
cat /sys/bus/acpi/devices/ELAN066C:00/status
?
Regards,
Hans
On 10/14/23 19:12, jdieter@gmail.com wrote:
> Hi Dmitry, Hans, input mailing list,
>
> I have two Lenovo 300e laptops, one running Windows and the other
> Fedora. The laptop has a touchscreen, a stylus and a touchpad. The
> touch screen and stylus work perfectly, but, under Fedora, the touchpad
> doesn't work at all and isn't detected by the kernel.
>
> The device shows up in /sys/bus/acpi/devices/ELAN066C:00/ with a
> modalias of acpi:ELAN066C:PNP0C50:.
>
> I'm not sure if the touchpad should be using elan_i2c or i2c_hid_acpi.
> In a somewhat naive attempt to get something to happen, I added:
> ```
> { "ELAN066C", 0 }, /* Lenovo 100e/300e 2nd gen AMD */
> ```
> to include/linux/input/elan-i2c-ids.h, but the kernel still doesn't
> auto-load the module.
>
> I can get elan_i2c to recognize something if I run:
> # echo elan_i2c 0x15 > /sys/bus/i2c/devices/i2c-1/new_device
> but the IRQ defaults to 0, and I get an error about an IRQ conflict:
>
> ```
> [ 2317.791897] i2c i2c-1: delete_device: Deleting device elan_i2c at
> 0x15
> [ 2319.317468] i2c i2c-1: new_device: Instantiated device elan_i2c at
> 0x15
> [ 2319.317662] elan_i2c 1-0015: supply vcc not found, using dummy
> regulator
> [ 2319.433176] elan_i2c 1-0015: Elan Touchpad: Module ID: 0x00be,
> Firmware: 0x0003, Sample: 0x0003, IAP: 0x000e
> [ 2319.433184] elan_i2c 1-0015: Elan Touchpad Extra Information:
> Max ABS X,Y: 3206,1828
> Width X,Y: 152,152
> Resolution X,Y: 31,31 (dots/mm)
> ic type: 0xa
> info pattern: 0x0
> [ 2319.433339] genirq: Flags mismatch irq 0. 00002008 (elan_i2c) vs.
> 00015a00 (timer)
> ```
>
> By manually setting the IRQ to an unused value in the driver, I can at
> least get the driver to create a new mouse device in /dev/input, but
> the touchpad still doesn't work.
>
> At this point, I don't know if I'm really close or barking up the
> completely wrong tree.
>
> A bug report from earlier this year about an identical system is at:
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1976556
>
> Attached is the dsl of the device in ACPI.
>
> I would really appreciate any help in figuring out how to get this
> working. Please let me know if there's anything else you need from me.
>
> Thanks,
>
> Jonathan
^ permalink raw reply
* Re: Help getting ELAN066C touchpad working in Linux
From: Jonathan Dieter @ 2023-10-14 20:22 UTC (permalink / raw)
To: Hans de Goede, Dmitry Torokhov, linux-input
In-Reply-To: <f5016787-c4c7-c8d9-f208-6e355d8b7db0@redhat.com>
On Sat, 2023-10-14 at 21:39 +0200, Hans de Goede wrote:
> Hi Jonathan,
>
> For starters, did you see this in dmesg? :
>
> [ 0.422648] i8042: PNP: PS/2 Controller [PNP0303:KBC0] at
> 0x60,0x64 irq 1
> [ 0.422652] i8042: PNP: PS/2 appears to have AUX port disabled, if
> this is incorrect please boot with i8042.nopnp
>
> And did you try booting with "i8042.nopnp" ?
>
> Also what is the output of:
>
> ls -l /sys/bus/i2c/devices
>
> and of:
>
> ls -l /sys/bus/acpi/devices/ELAN066C:00/
>
> and of:
>
> cat /sys/bus/acpi/devices/ELAN066C:00/status
>
> ?
Hey Hans,
Yeah, I did see that in my dmesg and did try i8042.nopnp by itself and
with combinations of pci=nocrs and the other i8042 options, with no
effect except breaking audio.
# ls -l /sys/bus/i2c/devices
total 0
lrwxrwxrwx. 1 root root 0 Oct 14 2023 i2c-0 -> ../../../devices/platform/AMDI0010:00/i2c-0
lrwxrwxrwx. 1 root root 0 Oct 14 2023 i2c-1 -> ../../../devices/platform/AMDI0010:01/i2c-1
lrwxrwxrwx. 1 root root 0 Oct 14 21:11 i2c-2 -> ../../../devices/pci0000:00/0000:00:08.1/0000:04:00.0/i2c-2
lrwxrwxrwx. 1 root root 0 Oct 14 21:11 i2c-3 -> ../../../devices/pci0000:00/0000:00:08.1/0000:04:00.0/i2c-3
lrwxrwxrwx. 1 root root 0 Oct 14 21:11 i2c-4 -> ../../../devices/pci0000:00/0000:00:08.1/0000:04:00.0/i2c-4
lrwxrwxrwx. 1 root root 0 Oct 14 21:11 i2c-5 -> ../../../devices/pci0000:00/0000:00:08.1/0000:04:00.0/drm/card1/card1-eDP-1/i2c-5
lrwxrwxrwx. 1 root root 0 Oct 14 21:11 i2c-6 -> ../../../devices/pci0000:00/0000:00:08.1/0000:04:00.0/drm/card1/card1-DP-1/i2c-6
lrwxrwxrwx. 1 root root 0 Oct 14 21:15 i2c-7 -> ../../../devices/pci0000:00/0000:00:14.0/i2c-7
lrwxrwxrwx. 1 root root 0 Oct 14 21:15 i2c-8 -> ../../../devices/pci0000:00/0000:00:14.0/i2c-8
lrwxrwxrwx. 1 root root 0 Oct 14 21:15 i2c-9 -> ../../../devices/pci0000:00/0000:00:14.0/i2c-9
lrwxrwxrwx. 1 root root 0 Oct 14 2023 i2c-ELAN238E:00 -> ../../../devices/platform/AMDI0010:00/i2c-0/i2c-ELAN238E:00
# ls -l /sys/bus/acpi/devices/ELAN066C:00/
total 0
-r--r--r--. 1 root root 4096 Oct 14 21:16 hid
-r--r--r--. 1 root root 4096 Oct 14 21:16 modalias
-r--r--r--. 1 root root 4096 Oct 14 21:16 path
lrwxrwxrwx. 1 root root 0 Oct 14 21:16 physical_node -> ../../../../platform/AMDI0010:01/ELAN066C:00
drwxr-xr-x. 2 root root 0 Oct 14 21:16 power
-r--r--r--. 1 root root 4096 Oct 14 21:16 status
lrwxrwxrwx. 1 root root 0 Oct 14 2023 subsystem -> ../../../../../bus/acpi
-rw-r--r--. 1 root root 4096 Oct 14 2023 uevent
# cat /sys/bus/acpi/devices/ELAN066C:00/status
15
The above is exactly the same on a clean Fedora 6.5.6 kernel and my
custom build with the alias added to elan_i2c.
Thanks for the swift reply!
Jonathan
^ permalink raw reply
* Re: Help getting ELAN066C touchpad working in Linux
From: Hans de Goede @ 2023-10-14 20:30 UTC (permalink / raw)
To: Jonathan Dieter, Dmitry Torokhov, linux-input
In-Reply-To: <4a1de7c3dfd7c2d2ced08c6d2b589555c1205dab.camel@gmail.com>
Hi,
On 10/14/23 22:22, Jonathan Dieter wrote:
> On Sat, 2023-10-14 at 21:39 +0200, Hans de Goede wrote:
>> Hi Jonathan,
>>
>> For starters, did you see this in dmesg? :
>>
>> [ 0.422648] i8042: PNP: PS/2 Controller [PNP0303:KBC0] at
>> 0x60,0x64 irq 1
>> [ 0.422652] i8042: PNP: PS/2 appears to have AUX port disabled, if
>> this is incorrect please boot with i8042.nopnp
>>
>> And did you try booting with "i8042.nopnp" ?
>>
>> Also what is the output of:
>>
>> ls -l /sys/bus/i2c/devices
>>
>> and of:
>>
>> ls -l /sys/bus/acpi/devices/ELAN066C:00/
>>
>> and of:
>>
>> cat /sys/bus/acpi/devices/ELAN066C:00/status
>>
>> ?
>
> Hey Hans,
>
> Yeah, I did see that in my dmesg and did try i8042.nopnp by itself and
> with combinations of pci=nocrs and the other i8042 options, with no
> effect except breaking audio.
>
> # ls -l /sys/bus/i2c/devices
> total 0
> lrwxrwxrwx. 1 root root 0 Oct 14 2023 i2c-0 -> ../../../devices/platform/AMDI0010:00/i2c-0
> lrwxrwxrwx. 1 root root 0 Oct 14 2023 i2c-1 -> ../../../devices/platform/AMDI0010:01/i2c-1
> lrwxrwxrwx. 1 root root 0 Oct 14 21:11 i2c-2 -> ../../../devices/pci0000:00/0000:00:08.1/0000:04:00.0/i2c-2
> lrwxrwxrwx. 1 root root 0 Oct 14 21:11 i2c-3 -> ../../../devices/pci0000:00/0000:00:08.1/0000:04:00.0/i2c-3
> lrwxrwxrwx. 1 root root 0 Oct 14 21:11 i2c-4 -> ../../../devices/pci0000:00/0000:00:08.1/0000:04:00.0/i2c-4
> lrwxrwxrwx. 1 root root 0 Oct 14 21:11 i2c-5 -> ../../../devices/pci0000:00/0000:00:08.1/0000:04:00.0/drm/card1/card1-eDP-1/i2c-5
> lrwxrwxrwx. 1 root root 0 Oct 14 21:11 i2c-6 -> ../../../devices/pci0000:00/0000:00:08.1/0000:04:00.0/drm/card1/card1-DP-1/i2c-6
> lrwxrwxrwx. 1 root root 0 Oct 14 21:15 i2c-7 -> ../../../devices/pci0000:00/0000:00:14.0/i2c-7
> lrwxrwxrwx. 1 root root 0 Oct 14 21:15 i2c-8 -> ../../../devices/pci0000:00/0000:00:14.0/i2c-8
> lrwxrwxrwx. 1 root root 0 Oct 14 21:15 i2c-9 -> ../../../devices/pci0000:00/0000:00:14.0/i2c-9
> lrwxrwxrwx. 1 root root 0 Oct 14 2023 i2c-ELAN238E:00 -> ../../../devices/platform/AMDI0010:00/i2c-0/i2c-ELAN238E:00
>
> # ls -l /sys/bus/acpi/devices/ELAN066C:00/
> total 0
> -r--r--r--. 1 root root 4096 Oct 14 21:16 hid
> -r--r--r--. 1 root root 4096 Oct 14 21:16 modalias
> -r--r--r--. 1 root root 4096 Oct 14 21:16 path
> lrwxrwxrwx. 1 root root 0 Oct 14 21:16 physical_node -> ../../../../platform/AMDI0010:01/ELAN066C:00
Ok this looks like a platform-device is being instantiated for the touchpad
rather then an i2c_client.
Can you do:
ls -l /sys/bus/platform/devices | grep ELAN066C
and let me know the output of that ?
If that confirms that a platform device is being instantiated
then the issue likely is that neither of the 2 if-s here:
Method (_CRS, 0, NotSerialized) // _CRS: Current Resource Settings
{
Name (SBFG, ResourceTemplate ()
{
GpioInt (Level, ActiveLow, ExclusiveAndWake, PullUp, 0x0000,
"\\_SB.GPIO", 0x00, ResourceConsumer, ,
)
{ // Pin list
0x0009
}
})
If ((^^^PCI0.LPC0.H_EC.ECRD (RefOf (^^^PCI0.LPC0.H_EC.TPTY)) == 0x01))
{
Name (SBFB, ResourceTemplate ()
{
I2cSerialBusV2 (0x0015, ControllerInitiated, 0x00061A80,
AddressingMode7Bit, "\\_SB.I2CD",
0x00, ResourceConsumer, , Exclusive,
)
})
Return (ConcatenateResTemplate (SBFB, SBFG))
}
If ((^^^PCI0.LPC0.H_EC.ECRD (RefOf (^^^PCI0.LPC0.H_EC.TPTY)) == 0x02))
{
Name (SBFC, ResourceTemplate ()
{
I2cSerialBusV2 (0x002C, ControllerInitiated, 0x00061A80,
AddressingMode7Bit, "\\_SB.I2CD",
0x00, ResourceConsumer, , Exclusive,
)
})
Return (ConcatenateResTemplate (SBFC, SBFG))
}
}
Returns true causing no i2c-client to get instantiated.
If that is the case the first thing to do is try a DSDT overlay replacing the
first if:
If ((^^^PCI0.LPC0.H_EC.ECRD (RefOf (^^^PCI0.LPC0.H_EC.TPTY)) == 0x01))
{
...
}
With dropping the If so you get:
{
...
}
See: https://docs.kernel.org/admin-guide/acpi/initrd_table_override.html
And then specifically the "How does it work" section.
Regards,
Hans
^ permalink raw reply
* Re: Help getting ELAN066C touchpad working in Linux
From: Jonathan Dieter @ 2023-10-14 21:16 UTC (permalink / raw)
To: Hans de Goede, Dmitry Torokhov, linux-input
In-Reply-To: <fdfc2766-0885-7cbe-0e96-6358a08aa3c7@redhat.com>
On Sat, 2023-10-14 at 22:30 +0200, Hans de Goede wrote:
> On 10/14/23 22:22, Jonathan Dieter wrote:
> > # ls -l /sys/bus/i2c/devices
> > total 0
> > lrwxrwxrwx. 1 root root 0 Oct 14 2023 i2c-0 ->
> > ../../../devices/platform/AMDI0010:00/i2c-0
> > lrwxrwxrwx. 1 root root 0 Oct 14 2023 i2c-1 ->
> > ../../../devices/platform/AMDI0010:01/i2c-1
> > lrwxrwxrwx. 1 root root 0 Oct 14 21:11 i2c-2 ->
> > ../../../devices/pci0000:00/0000:00:08.1/0000:04:00.0/i2c-2
> > lrwxrwxrwx. 1 root root 0 Oct 14 21:11 i2c-3 ->
> > ../../../devices/pci0000:00/0000:00:08.1/0000:04:00.0/i2c-3
> > lrwxrwxrwx. 1 root root 0 Oct 14 21:11 i2c-4 ->
> > ../../../devices/pci0000:00/0000:00:08.1/0000:04:00.0/i2c-4
> > lrwxrwxrwx. 1 root root 0 Oct 14 21:11 i2c-5 ->
> > ../../../devices/pci0000:00/0000:00:08.1/0000:04:00.0/drm/card1/car
> > d1-eDP-1/i2c-5
> > lrwxrwxrwx. 1 root root 0 Oct 14 21:11 i2c-6 ->
> > ../../../devices/pci0000:00/0000:00:08.1/0000:04:00.0/drm/card1/car
> > d1-DP-1/i2c-6
> > lrwxrwxrwx. 1 root root 0 Oct 14 21:15 i2c-7 ->
> > ../../../devices/pci0000:00/0000:00:14.0/i2c-7
> > lrwxrwxrwx. 1 root root 0 Oct 14 21:15 i2c-8 ->
> > ../../../devices/pci0000:00/0000:00:14.0/i2c-8
> > lrwxrwxrwx. 1 root root 0 Oct 14 21:15 i2c-9 ->
> > ../../../devices/pci0000:00/0000:00:14.0/i2c-9
> > lrwxrwxrwx. 1 root root 0 Oct 14 2023 i2c-ELAN238E:00 ->
> > ../../../devices/platform/AMDI0010:00/i2c-0/i2c-ELAN238E:00
> >
> > # ls -l /sys/bus/acpi/devices/ELAN066C:00/
> > total 0
> > -r--r--r--. 1 root root 4096 Oct 14 21:16 hid
> > -r--r--r--. 1 root root 4096 Oct 14 21:16 modalias
> > -r--r--r--. 1 root root 4096 Oct 14 21:16 path
> > lrwxrwxrwx. 1 root root 0 Oct 14 21:16 physical_node ->
> > ../../../../platform/AMDI0010:01/ELAN066C:00
>
> Ok this looks like a platform-device is being instantiated for the
> touchpad
> rather then an i2c_client.
>
> Can you do:
>
> ls -l /sys/bus/platform/devices | grep ELAN066C
>
> and let me know the output of that ?
>
> If that confirms that a platform device is being instantiated
> then the issue likely is that neither of the 2 if-s here:
>
...
>
> Returns true causing no i2c-client to get instantiated.
>
>
> If that is the case the first thing to do is try a DSDT overlay
> replacing the
> first if:
>
> If ((^^^PCI0.LPC0.H_EC.ECRD (RefOf (^^^PCI0.LPC0.H_EC.TPTY))
> == 0x01))
> {
> ...
> }
>
> With dropping the If so you get:
>
> {
> ...
> }
>
> See:
> https://docs.kernel.org/admin-guide/acpi/initrd_table_override.html
>
> And then specifically the "How does it work" section.
Hans,
Thank you so much! Adding a DSDT overlay worked perfectly! The
touchpad is functioning exactly as you would expect (using the
i2c_hid_acpi driver).
# ls -l /sys/bus/i2c/devices/
total 0
lrwxrwxrwx. 1 root root 0 Oct 14 2023 i2c-0 -> ../../../devices/platform/AMDI0010:00/i2c-0
lrwxrwxrwx. 1 root root 0 Oct 14 2023 i2c-1 -> ../../../devices/platform/AMDI0010:01/i2c-1
lrwxrwxrwx. 1 root root 0 Oct 14 22:02 i2c-2 -> ../../../devices/pci0000:00/0000:00:08.1/0000:04:00.0/i2c-2
lrwxrwxrwx. 1 root root 0 Oct 14 22:02 i2c-3 -> ../../../devices/pci0000:00/0000:00:08.1/0000:04:00.0/i2c-3
lrwxrwxrwx. 1 root root 0 Oct 14 22:02 i2c-4 -> ../../../devices/pci0000:00/0000:00:08.1/0000:04:00.0/i2c-4
lrwxrwxrwx. 1 root root 0 Oct 14 22:02 i2c-5 -> ../../../devices/pci0000:00/0000:00:08.1/0000:04:00.0/drm/card1/card1-eDP-1/i2c-5
lrwxrwxrwx. 1 root root 0 Oct 14 22:02 i2c-6 -> ../../../devices/pci0000:00/0000:00:08.1/0000:04:00.0/drm/card1/card1-DP-1/i2c-6
lrwxrwxrwx. 1 root root 0 Oct 14 22:03 i2c-7 -> ../../../devices/pci0000:00/0000:00:14.0/i2c-7
lrwxrwxrwx. 1 root root 0 Oct 14 22:03 i2c-8 -> ../../../devices/pci0000:00/0000:00:14.0/i2c-8
lrwxrwxrwx. 1 root root 0 Oct 14 22:03 i2c-9 -> ../../../devices/pci0000:00/0000:00:14.0/i2c-9
lrwxrwxrwx. 1 root root 0 Oct 14 2023 i2c-ELAN066C:00 -> ../../../devices/platform/AMDI0010:01/i2c-1/i2c-ELAN066C:00
lrwxrwxrwx. 1 root root 0 Oct 14 2023 i2c-ELAN238E:00 -> ../../../devices/platform/AMDI0010:00/i2c-0/i2c-ELAN238E:00
# ls -l /sys/bus/acpi/devices/ELAN066C:00/
total 0
-r--r--r--. 1 root root 4096 Oct 14 22:10 hid
-r--r--r--. 1 root root 4096 Oct 14 22:10 modalias
-r--r--r--. 1 root root 4096 Oct 14 22:10 path
lrwxrwxrwx. 1 root root 0 Oct 14 22:10 physical_node -> ../../../../platform/AMDI0010:01/i2c-1/i2c-ELAN066C:00
drwxr-xr-x. 2 root root 0 Oct 14 22:10 power
-r--r--r--. 1 root root 4096 Oct 14 22:10 status
lrwxrwxrwx. 1 root root 0 Oct 14 2023 subsystem -> ../../../../../bus/acpi
-rw-r--r--. 1 root root 4096 Oct 14 2023 uevent
drwxr-xr-x. 3 root root 0 Oct 14 2023 wakeup
Is there some way we can get a fix in the kernel for this buggy ACPI table or should I be asking Lenovo for a fix?
Thanks again,
Jonathan
^ permalink raw reply
* Re: Help getting ELAN066C touchpad working in Linux
From: Hans de Goede @ 2023-10-15 9:25 UTC (permalink / raw)
To: Jonathan Dieter, Dmitry Torokhov, linux-input
In-Reply-To: <f37f08d92587b7ecdbc32cc071dbabe7738695a2.camel@gmail.com>
Hi Jonathan,
On 10/14/23 23:16, Jonathan Dieter wrote:
> On Sat, 2023-10-14 at 22:30 +0200, Hans de Goede wrote:
<snip>
> Hans,
>
> Thank you so much! Adding a DSDT overlay worked perfectly! The
> touchpad is functioning exactly as you would expect (using the
> i2c_hid_acpi driver).
That is good to hear, so that confirms that this line:
If ((^^^PCI0.LPC0.H_EC.ECRD (RefOf (^^^PCI0.LPC0.H_EC.TPTY)) == 0x01))
In the DSDT is not functioning as it should under Linux.
<snip ls -l /sys/bus/i2c/devices/ which looks good now>
> Is there some way we can get a fix in the kernel for this buggy ACPI table or should I be asking Lenovo for a fix?
I assume (and it seems so) that the:
If ((^^^PCI0.LPC0.H_EC.ECRD (RefOf (^^^PCI0.LPC0.H_EC.TPTY)) == 0x01))
line in the DSDT does work under Windows. So now we need to figure out
why it does work under Windows and not under Linux.
Can you please run:
sudo acpidump -o acpidump.txt
And then reply to this email with the generated acpidump.txt attached ?
Note please drop the linux-input list from the Cc when sending the acpidump.
Then I'll take a look at the full DSDT and we'll see from there.
As for getting a BIOS fix for this, I think that the chances of Lenovo fixing this when asked are not good.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH v3 09/16] platform/x86/amd/pmf: Add facility to dump TA inputs
From: kernel test robot @ 2023-10-15 10:05 UTC (permalink / raw)
To: Shyam Sundar S K, hdegoede, markgross, ilpo.jarvinen,
basavaraj.natikar, jikos, benjamin.tissoires, alexander.deucher,
christian.koenig, Xinhui.Pan, airlied, daniel
Cc: oe-kbuild-all, Shyam Sundar S K, amd-gfx, platform-driver-x86,
dri-devel, Patil.Reddy, linux-input, mario.limonciello
In-Reply-To: <20231010125917.138225-10-Shyam-sundar.S-k@amd.com>
Hi Shyam,
kernel test robot noticed the following build errors:
[auto build test ERROR on hid/for-next]
[also build test ERROR on drm-misc/drm-misc-next linus/master v6.6-rc5 next-20231013]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Shyam-Sundar-S-K/platform-x86-amd-pmf-Add-PMF-TEE-interface/20231010-210347
base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link: https://lore.kernel.org/r/20231010125917.138225-10-Shyam-sundar.S-k%40amd.com
patch subject: [PATCH v3 09/16] platform/x86/amd/pmf: Add facility to dump TA inputs
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20231015/202310151704.ofb4HSxK-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231015/202310151704.ofb4HSxK-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310151704.ofb4HSxK-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from include/linux/printk.h:564,
from include/linux/kernel.h:30,
from arch/x86/include/asm/percpu.h:27,
from arch/x86/include/asm/preempt.h:6,
from include/linux/preempt.h:79,
from include/linux/spinlock.h:56,
from include/linux/mmzone.h:8,
from include/linux/gfp.h:7,
from include/linux/slab.h:16,
from include/linux/resource_ext.h:11,
from include/linux/acpi.h:13,
from drivers/platform/x86/amd/pmf/pmf.h:14,
from drivers/platform/x86/amd/pmf/sps.c:11:
drivers/platform/x86/amd/pmf/sps.c: In function 'amd_pmf_dump_sps_defaults':
>> drivers/platform/x86/amd/pmf/sps.c:50:65: error: implicit declaration of function 'source_as_str' [-Werror=implicit-function-declaration]
50 | pr_debug("--- Source:%s Mode:%s ---\n", source_as_str(i), slider_as_str(j));
| ^~~~~~~~~~~~~
include/linux/dynamic_debug.h:224:29: note: in definition of macro '__dynamic_func_call_cls'
224 | func(&id, ##__VA_ARGS__); \
| ^~~~~~~~~~~
include/linux/dynamic_debug.h:250:9: note: in expansion of macro '_dynamic_func_call_cls'
250 | _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call'
269 | _dynamic_func_call(fmt, __dynamic_pr_debug, \
| ^~~~~~~~~~~~~~~~~~
include/linux/printk.h:579:9: note: in expansion of macro 'dynamic_pr_debug'
579 | dynamic_pr_debug(fmt, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~
drivers/platform/x86/amd/pmf/sps.c:50:25: note: in expansion of macro 'pr_debug'
50 | pr_debug("--- Source:%s Mode:%s ---\n", source_as_str(i), slider_as_str(j));
| ^~~~~~~~
drivers/platform/x86/amd/pmf/sps.c:50:34: warning: format '%s' expects argument of type 'char *', but argument 3 has type 'int' [-Wformat=]
50 | pr_debug("--- Source:%s Mode:%s ---\n", source_as_str(i), slider_as_str(j));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/printk.h:345:21: note: in definition of macro 'pr_fmt'
345 | #define pr_fmt(fmt) fmt
| ^~~
include/linux/dynamic_debug.h:248:9: note: in expansion of macro '__dynamic_func_call_cls'
248 | __dynamic_func_call_cls(__UNIQUE_ID(ddebug), cls, fmt, func, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:250:9: note: in expansion of macro '_dynamic_func_call_cls'
250 | _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call'
269 | _dynamic_func_call(fmt, __dynamic_pr_debug, \
| ^~~~~~~~~~~~~~~~~~
include/linux/printk.h:579:9: note: in expansion of macro 'dynamic_pr_debug'
579 | dynamic_pr_debug(fmt, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~
drivers/platform/x86/amd/pmf/sps.c:50:25: note: in expansion of macro 'pr_debug'
50 | pr_debug("--- Source:%s Mode:%s ---\n", source_as_str(i), slider_as_str(j));
| ^~~~~~~~
drivers/platform/x86/amd/pmf/sps.c:50:47: note: format string is defined here
50 | pr_debug("--- Source:%s Mode:%s ---\n", source_as_str(i), slider_as_str(j));
| ~^
| |
| char *
| %d
cc1: some warnings being treated as errors
vim +/source_as_str +50 drivers/platform/x86/amd/pmf/sps.c
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 41
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 42 static void amd_pmf_dump_sps_defaults(struct amd_pmf_static_slider_granular *data)
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 43 {
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 44 int i, j;
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 45
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 46 pr_debug("Static Slider Data - BEGIN\n");
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 47
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 48 for (i = 0; i < POWER_SOURCE_MAX; i++) {
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 49 for (j = 0; j < POWER_MODE_MAX; j++) {
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 @50 pr_debug("--- Source:%s Mode:%s ---\n", source_as_str(i), slider_as_str(j));
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 51 pr_debug("SPL: %u mW\n", data->prop[i][j].spl);
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 52 pr_debug("SPPT: %u mW\n", data->prop[i][j].sppt);
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 53 pr_debug("SPPT_ApuOnly: %u mW\n", data->prop[i][j].sppt_apu_only);
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 54 pr_debug("FPPT: %u mW\n", data->prop[i][j].fppt);
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 55 pr_debug("STTMinLimit: %u mW\n", data->prop[i][j].stt_min);
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 56 pr_debug("STT_SkinTempLimit_APU: %u C\n",
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 57 data->prop[i][j].stt_skin_temp[STT_TEMP_APU]);
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 58 pr_debug("STT_SkinTempLimit_HS2: %u C\n",
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 59 data->prop[i][j].stt_skin_temp[STT_TEMP_HS2]);
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 60 }
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 61 }
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 62
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 63 pr_debug("Static Slider Data - END\n");
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 64 }
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 65 #else
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 66 static void amd_pmf_dump_sps_defaults(struct amd_pmf_static_slider_granular *data) {}
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 67 #endif
a82ebb3d800d7b Shyam Sundar S K 2023-05-10 68
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [dtor-input:for-linus] BUILD SUCCESS 5c15c60e7be615f05a45cd905093a54b11f461bc
From: kernel test robot @ 2023-10-15 11:34 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus
branch HEAD: 5c15c60e7be615f05a45cd905093a54b11f461bc Input: powermate - fix use-after-free in powermate_config_complete
elapsed time: 1877m
configs tested: 118
configs skipped: 2
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allnoconfig gcc
alpha allyesconfig gcc
alpha defconfig gcc
arc allmodconfig gcc
arc allnoconfig gcc
arc allyesconfig gcc
arc defconfig gcc
arc randconfig-001-20231014 gcc
arm allmodconfig gcc
arm allnoconfig gcc
arm allyesconfig gcc
arm defconfig gcc
arm jornada720_defconfig gcc
arm netwinder_defconfig clang
arm randconfig-001-20231014 gcc
arm64 allmodconfig gcc
arm64 allnoconfig gcc
arm64 allyesconfig gcc
arm64 defconfig gcc
csky allmodconfig gcc
csky allnoconfig gcc
csky allyesconfig gcc
csky defconfig gcc
i386 allmodconfig gcc
i386 allnoconfig gcc
i386 allyesconfig gcc
i386 debian-10.3 gcc
i386 defconfig gcc
i386 randconfig-001-20231014 gcc
i386 randconfig-002-20231014 gcc
i386 randconfig-003-20231014 gcc
i386 randconfig-004-20231014 gcc
i386 randconfig-005-20231014 gcc
i386 randconfig-006-20231014 gcc
loongarch allmodconfig gcc
loongarch allnoconfig gcc
loongarch allyesconfig gcc
loongarch defconfig gcc
loongarch randconfig-001-20231014 gcc
m68k allmodconfig gcc
m68k allnoconfig gcc
m68k allyesconfig gcc
m68k defconfig gcc
microblaze allmodconfig gcc
microblaze allnoconfig gcc
microblaze allyesconfig gcc
microblaze defconfig gcc
mips allmodconfig gcc
mips allnoconfig gcc
mips allyesconfig gcc
mips bmips_stb_defconfig clang
mips cu1000-neo_defconfig clang
mips gpr_defconfig gcc
mips maltasmvp_eva_defconfig gcc
mips rbtx49xx_defconfig clang
nios2 allmodconfig gcc
nios2 allnoconfig gcc
nios2 allyesconfig gcc
nios2 defconfig gcc
openrisc allmodconfig gcc
openrisc allnoconfig gcc
openrisc allyesconfig gcc
openrisc defconfig gcc
parisc allmodconfig gcc
parisc allnoconfig gcc
parisc allyesconfig gcc
parisc defconfig gcc
parisc64 defconfig gcc
powerpc allmodconfig gcc
powerpc allnoconfig gcc
powerpc allyesconfig gcc
powerpc bluestone_defconfig clang
powerpc canyonlands_defconfig gcc
powerpc mpc834x_itx_defconfig gcc
riscv allmodconfig gcc
riscv allnoconfig gcc
riscv allyesconfig gcc
riscv defconfig gcc
riscv randconfig-001-20231014 gcc
riscv rv32_defconfig gcc
s390 alldefconfig clang
s390 allmodconfig gcc
s390 allnoconfig gcc
s390 allyesconfig gcc
s390 defconfig gcc
s390 randconfig-001-20231014 gcc
sh allmodconfig gcc
sh allnoconfig gcc
sh allyesconfig gcc
sh defconfig gcc
sh rts7751r2dplus_defconfig gcc
sparc allmodconfig gcc
sparc allnoconfig gcc
sparc allyesconfig gcc
sparc defconfig gcc
sparc randconfig-001-20231014 gcc
sparc64 allmodconfig gcc
sparc64 allyesconfig gcc
sparc64 defconfig gcc
um allmodconfig clang
um allnoconfig clang
um allyesconfig clang
um defconfig gcc
um i386_defconfig gcc
um x86_64_defconfig gcc
x86_64 allnoconfig gcc
x86_64 allyesconfig gcc
x86_64 defconfig gcc
x86_64 randconfig-001-20231014 gcc
x86_64 randconfig-002-20231014 gcc
x86_64 randconfig-003-20231014 gcc
x86_64 randconfig-004-20231014 gcc
x86_64 randconfig-005-20231014 gcc
x86_64 randconfig-006-20231014 gcc
x86_64 rhel-8.3-rust clang
x86_64 rhel-8.3 gcc
xtensa allnoconfig gcc
xtensa allyesconfig gcc
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [dtor-input:next] BUILD SUCCESS 6cd256694afe2a0a94f62418fea95279de2b7ada
From: kernel test robot @ 2023-10-15 14:37 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
branch HEAD: 6cd256694afe2a0a94f62418fea95279de2b7ada Input: tegra-kbc - use device_get_match_data()
elapsed time: 2060m
configs tested: 100
configs skipped: 2
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allnoconfig gcc
alpha allyesconfig gcc
alpha defconfig gcc
arc allmodconfig gcc
arc allnoconfig gcc
arc allyesconfig gcc
arc defconfig gcc
arc randconfig-001-20231014 gcc
arm allmodconfig gcc
arm allnoconfig gcc
arm allyesconfig gcc
arm defconfig gcc
arm randconfig-001-20231014 gcc
arm64 allmodconfig gcc
arm64 allnoconfig gcc
arm64 allyesconfig gcc
arm64 defconfig gcc
csky allmodconfig gcc
csky allnoconfig gcc
csky allyesconfig gcc
csky defconfig gcc
i386 defconfig gcc
i386 randconfig-001-20231014 gcc
i386 randconfig-002-20231014 gcc
i386 randconfig-003-20231014 gcc
i386 randconfig-004-20231014 gcc
i386 randconfig-005-20231014 gcc
i386 randconfig-006-20231014 gcc
loongarch allmodconfig gcc
loongarch allnoconfig gcc
loongarch allyesconfig gcc
loongarch defconfig gcc
loongarch randconfig-001-20231014 gcc
m68k allmodconfig gcc
m68k allnoconfig gcc
m68k allyesconfig gcc
m68k defconfig gcc
microblaze allmodconfig gcc
microblaze allnoconfig gcc
microblaze allyesconfig gcc
microblaze defconfig gcc
mips allmodconfig gcc
mips allnoconfig gcc
mips allyesconfig gcc
nios2 allmodconfig gcc
nios2 allnoconfig gcc
nios2 allyesconfig gcc
nios2 defconfig gcc
openrisc allmodconfig gcc
openrisc allnoconfig gcc
openrisc allyesconfig gcc
openrisc defconfig gcc
parisc allmodconfig gcc
parisc allnoconfig gcc
parisc allyesconfig gcc
parisc defconfig gcc
parisc64 defconfig gcc
powerpc allmodconfig gcc
powerpc allnoconfig gcc
powerpc allyesconfig gcc
riscv allmodconfig gcc
riscv allnoconfig gcc
riscv allyesconfig gcc
riscv defconfig gcc
riscv randconfig-001-20231014 gcc
riscv rv32_defconfig gcc
s390 allmodconfig gcc
s390 allnoconfig gcc
s390 allyesconfig gcc
s390 defconfig gcc
s390 randconfig-001-20231014 gcc
sh allmodconfig gcc
sh allnoconfig gcc
sh allyesconfig gcc
sh defconfig gcc
sparc allmodconfig gcc
sparc allnoconfig gcc
sparc allyesconfig gcc
sparc defconfig gcc
sparc randconfig-001-20231014 gcc
sparc64 allmodconfig gcc
sparc64 allyesconfig gcc
sparc64 defconfig gcc
um allmodconfig clang
um allnoconfig clang
um allyesconfig clang
um defconfig gcc
um i386_defconfig gcc
um x86_64_defconfig gcc
x86_64 allnoconfig gcc
x86_64 allyesconfig gcc
x86_64 defconfig gcc
x86_64 randconfig-001-20231014 gcc
x86_64 randconfig-002-20231014 gcc
x86_64 randconfig-003-20231014 gcc
x86_64 randconfig-004-20231014 gcc
x86_64 randconfig-005-20231014 gcc
x86_64 randconfig-006-20231014 gcc
x86_64 rhel-8.3-rust clang
x86_64 rhel-8.3 gcc
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [PATCH v2 0/7] Input: synaptics-rmi4: add quirks for third party touchscreen controllers
From: Caleb Connolly @ 2023-10-15 21:11 UTC (permalink / raw)
To: Dmitry Torokhov, Vincent Huang
Cc: methanal, linux-input, devicetree, phone-devel,
~postmarketos/upstreaming, Caleb Connolly, Jason A. Donenfeld,
Matthias Schiffer, Rob Herring, Krzysztof Kozlowski, Conor Dooley
With the growing popularity of running upstream Linux on mobile devices,
we're beginning to run into more and more edgecases. The OnePlus 6 is a
fairly well supported 2018 era smartphone, selling over a million units
in it's first 22 days. With this level of popularity, it's almost
inevitable that we get third party replacement displays, and as a
result, replacement touchscreen controllers.
The OnePlus 6 shipped with an extremely usecase specific touchscreen
driver, it implemented only the bare minimum parts of the highly generic
rmi4 protocol, instead hardcoding most of the register addresses.
As a result, the third party touchscreen controllers that are often
found in replacement screens, implement only the registers that the
downstream driver reads from. They additionally have other restrictions
such as heavy penalties on unaligned reads.
This series attempts to implement the necessary workaround to support
some of these chips with the rmi4 driver. Although it's worth noting
that at the time of writing there are other unofficial controllers in
the wild that don't work even with these patches.
We have been shipping these patches in postmarketOS for the last several
months, and they are known to not cause any regressions on the OnePlus
6/6T (with the official Synaptics controller), however I don't own any
other rmi4 hardware to further validate this.
These patches were contributed by a community developer who has given
permission for me to submit them on their behalf.
---
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Vincent Huang <vincent.huang@tw.synaptics.com>
Cc: methanal <baclofen@tuta.io>
Cc: linux-input@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: phone-devel@vger.kernel.org
Cc: ~postmarketos/upstreaming@lists.sr.ht
---
Changes since v1:
- Improve dt-bindings patch (thanks Rob)
- Add missing cast in patch 5 to fix the pointer arithmetic
- Link to v1: https://lore.kernel.org/r/20230929-caleb-rmi4-quirks-v1-0-cc3c703f022d@linaro.org
---
Caleb Connolly (2):
dt-bindings: input: syna,rmi4: document syna,pdt-fallback-desc
Input: synaptics-rmi4 - handle duplicate/unknown PDT entries
methanal (5):
Input: synaptics-rmi4 - f12: use hardcoded values for aftermarket touch ICs
Input: synaptics-rmi4 - f55: handle zero electrode count
Input: synaptics-rmi4 - don't do unaligned reads in IRQ context
Input: synaptics-rmi4 - read product ID on aftermarket touch ICs
Input: synaptics-rmi4 - support fallback values for PDT descriptor bytes
.../devicetree/bindings/input/syna,rmi4.yaml | 15 +++
drivers/input/rmi4/rmi_driver.c | 134 ++++++++++++++++++---
drivers/input/rmi4/rmi_driver.h | 8 ++
drivers/input/rmi4/rmi_f01.c | 14 +++
drivers/input/rmi4/rmi_f12.c | 117 ++++++++++++++----
drivers/input/rmi4/rmi_f55.c | 5 +
include/linux/rmi.h | 3 +
7 files changed, 252 insertions(+), 44 deletions(-)
---
base-commit: b0d95ff7653ef6ace66a24d6c09147d0731825ce
// Caleb (they/them)
^ permalink raw reply
* [PATCH v2 1/7] dt-bindings: input: syna,rmi4: document syna,pdt-fallback-desc
From: Caleb Connolly @ 2023-10-15 21:11 UTC (permalink / raw)
To: Dmitry Torokhov, Vincent Huang
Cc: methanal, linux-input, devicetree, phone-devel,
~postmarketos/upstreaming, Caleb Connolly, Jason A. Donenfeld,
Matthias Schiffer, Rob Herring, Krzysztof Kozlowski, Conor Dooley
In-Reply-To: <20230929-caleb-rmi4-quirks-v2-0-b227ac498d88@linaro.org>
This new property allows devices to specify some register values which
are missing on units with third party replacement displays. These
displays use unofficial touch ICs which only implement a subset of the
RMI4 specification.
Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
To: "Jason A. Donenfeld" <Jason@zx2c4.com>
To: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
To: Rob Herring <robh+dt@kernel.org>
To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
To: Conor Dooley <conor+dt@kernel.org>
Cc: devicetree@vger.kernel.org
---
Documentation/devicetree/bindings/input/syna,rmi4.yaml | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/syna,rmi4.yaml b/Documentation/devicetree/bindings/input/syna,rmi4.yaml
index 4d4e1a8e36be..1f4a2179e4d3 100644
--- a/Documentation/devicetree/bindings/input/syna,rmi4.yaml
+++ b/Documentation/devicetree/bindings/input/syna,rmi4.yaml
@@ -49,6 +49,21 @@ properties:
description:
Delay to wait after powering on the device.
+ syna,pdt-fallback-desc:
+ $ref: /schemas/types.yaml#/definitions/uint8-matrix
+ description:
+ An array of pairs of function number and version. These are used
+ on some devices with replacement displays that use unofficial touch
+ controllers. These controllers do report the properties of their Page
+ Descriptor Table (PDT) entries, but leave the function_number and
+ function_version registers blank. These values should match exactly
+ the 5th and 4th bytes of each PDT entry from the original display's
+ touch controller.
+ items:
+ items:
+ - description: The 5th byte of the PDT entry
+ - description: The 4th byte of the PDT entry
+
vdd-supply: true
vio-supply: true
--
2.42.0
^ permalink raw reply related
* [PATCH v2 2/7] Input: synaptics-rmi4 - handle duplicate/unknown PDT entries
From: Caleb Connolly @ 2023-10-15 21:11 UTC (permalink / raw)
To: Dmitry Torokhov, Vincent Huang
Cc: methanal, linux-input, devicetree, phone-devel,
~postmarketos/upstreaming, Caleb Connolly
In-Reply-To: <20230929-caleb-rmi4-quirks-v2-0-b227ac498d88@linaro.org>
Some third party rmi4-compatible ICs don't expose their PDT entries
very well. Add a few checks to skip duplicate entries as well as entries
for unsupported functions.
This is required to support some phones with third party displays.
Validated on a stock OnePlus 6T (original parts):
manufacturer: Synaptics, product: S3706B, fw id: 2852315
Co-developed-by: methanal <baclofen@tuta.io>
Signed-off-by: methanal <baclofen@tuta.io>
Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
drivers/input/rmi4/rmi_driver.c | 47 +++++++++++++++++++++++++++++++++++------
drivers/input/rmi4/rmi_driver.h | 6 ++++++
2 files changed, 47 insertions(+), 6 deletions(-)
diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index 258d5fe3d395..cd3e4e77ab9b 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -493,12 +493,44 @@ static void rmi_driver_copy_pdt_to_fd(const struct pdt_entry *pdt,
fd->function_version = pdt->function_version;
}
+static bool rmi_pdt_entry_is_valid(struct rmi_device *rmi_dev,
+ struct pdt_scan_state *state, u8 fn)
+{
+ int i;
+
+ switch (fn) {
+ case 0x01:
+ case 0x03:
+ case 0x11:
+ case 0x12:
+ case 0x30:
+ case 0x34:
+ case 0x3a:
+ case 0x54:
+ case 0x55:
+ break;
+
+ default:
+ rmi_dbg(RMI_DEBUG_CORE, &rmi_dev->dev,
+ "PDT has unknown function number %#02x\n", fn);
+ return false;
+ }
+
+ for (i = 0; i < state->pdt_count; i++) {
+ if (state->pdts[i] == fn)
+ return false;
+ }
+
+ state->pdts[state->pdt_count++] = fn;
+ return true;
+}
+
#define RMI_SCAN_CONTINUE 0
#define RMI_SCAN_DONE 1
static int rmi_scan_pdt_page(struct rmi_device *rmi_dev,
int page,
- int *empty_pages,
+ struct pdt_scan_state *state,
void *ctx,
int (*callback)(struct rmi_device *rmi_dev,
void *ctx,
@@ -521,6 +553,9 @@ static int rmi_scan_pdt_page(struct rmi_device *rmi_dev,
if (RMI4_END_OF_PDT(pdt_entry.function_number))
break;
+ if (!rmi_pdt_entry_is_valid(rmi_dev, state, pdt_entry.function_number))
+ continue;
+
retval = callback(rmi_dev, ctx, &pdt_entry);
if (retval != RMI_SCAN_CONTINUE)
return retval;
@@ -531,11 +566,11 @@ static int rmi_scan_pdt_page(struct rmi_device *rmi_dev,
* or more is found, stop scanning.
*/
if (addr == pdt_start)
- ++*empty_pages;
+ ++state->empty_pages;
else
- *empty_pages = 0;
+ state->empty_pages = 0;
- return (data->bootloader_mode || *empty_pages >= 2) ?
+ return (data->bootloader_mode || state->empty_pages >= 2) ?
RMI_SCAN_DONE : RMI_SCAN_CONTINUE;
}
@@ -544,11 +579,11 @@ int rmi_scan_pdt(struct rmi_device *rmi_dev, void *ctx,
void *ctx, const struct pdt_entry *entry))
{
int page;
- int empty_pages = 0;
+ struct pdt_scan_state state = {0, 0, {0}};
int retval = RMI_SCAN_DONE;
for (page = 0; page <= RMI4_MAX_PAGE; page++) {
- retval = rmi_scan_pdt_page(rmi_dev, page, &empty_pages,
+ retval = rmi_scan_pdt_page(rmi_dev, page, &state,
ctx, callback);
if (retval != RMI_SCAN_CONTINUE)
break;
diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
index 1c6c6086c0e5..e1a5412f2f8f 100644
--- a/drivers/input/rmi4/rmi_driver.h
+++ b/drivers/input/rmi4/rmi_driver.h
@@ -46,6 +46,12 @@ struct pdt_entry {
u8 function_number;
};
+struct pdt_scan_state {
+ u8 empty_pages;
+ u8 pdt_count;
+ u8 pdts[254];
+};
+
#define RMI_REG_DESC_PRESENSE_BITS (32 * BITS_PER_BYTE)
#define RMI_REG_DESC_SUBPACKET_BITS (37 * BITS_PER_BYTE)
--
2.42.0
^ permalink raw reply related
* [PATCH v2 4/7] Input: synaptics-rmi4 - f55: handle zero electrode count
From: Caleb Connolly @ 2023-10-15 21:11 UTC (permalink / raw)
To: Dmitry Torokhov, Vincent Huang
Cc: methanal, linux-input, devicetree, phone-devel,
~postmarketos/upstreaming, Caleb Connolly
In-Reply-To: <20230929-caleb-rmi4-quirks-v2-0-b227ac498d88@linaro.org>
From: methanal <baclofen@tuta.io>
Some third party ICs claim to support f55 but report an electrode count
of 0. Catch this and bail out early so that we don't confuse the i2c bus
with 0 sized reads.
Signed-off-by: methanal <baclofen@tuta.io>
[simplify code, adjust wording]
Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
drivers/input/rmi4/rmi_f55.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/input/rmi4/rmi_f55.c b/drivers/input/rmi4/rmi_f55.c
index 488adaca4dd0..ad2ef14ae9f4 100644
--- a/drivers/input/rmi4/rmi_f55.c
+++ b/drivers/input/rmi4/rmi_f55.c
@@ -52,6 +52,11 @@ static int rmi_f55_detect(struct rmi_function *fn)
f55->num_rx_electrodes = f55->qry[F55_NUM_RX_OFFSET];
f55->num_tx_electrodes = f55->qry[F55_NUM_TX_OFFSET];
+ if (!f55->num_rx_electrodes || !f55->num_tx_electrodes) {
+ rmi_dbg(RMI_DEBUG_FN, &fn->dev,
+ "F55 query returned no electrodes, giving up\n");
+ return 0;
+ }
f55->cfg_num_rx_electrodes = f55->num_rx_electrodes;
f55->cfg_num_tx_electrodes = f55->num_rx_electrodes;
--
2.42.0
^ permalink raw reply related
* [PATCH v2 3/7] Input: synaptics-rmi4 - f12: use hardcoded values for aftermarket touch ICs
From: Caleb Connolly @ 2023-10-15 21:11 UTC (permalink / raw)
To: Dmitry Torokhov, Vincent Huang
Cc: methanal, linux-input, devicetree, phone-devel,
~postmarketos/upstreaming, Caleb Connolly
In-Reply-To: <20230929-caleb-rmi4-quirks-v2-0-b227ac498d88@linaro.org>
From: methanal <baclofen@tuta.io>
Some replacement displays include third-party touch ICs which are
devoid of register descriptors. Create a fake data register descriptor
for such ICs and provide hardcoded default values.
It isn't possible to reliably determine if the touch IC is original or
not, so these fallback values are offered as an alternative to the error
path when register descriptors aren't available.
Signed-off-by: methanal <baclofen@tuta.io>
[changes for readability / codeflow, checkpatch fixes]
Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
drivers/input/rmi4/rmi_f12.c | 117 +++++++++++++++++++++++++++++++++----------
1 file changed, 91 insertions(+), 26 deletions(-)
diff --git a/drivers/input/rmi4/rmi_f12.c b/drivers/input/rmi4/rmi_f12.c
index 7e97944f7616..719ee3b42550 100644
--- a/drivers/input/rmi4/rmi_f12.c
+++ b/drivers/input/rmi4/rmi_f12.c
@@ -195,6 +195,41 @@ static void rmi_f12_process_objects(struct f12_data *f12, u8 *data1, int size)
rmi_2d_sensor_abs_report(sensor, &sensor->objs[i], i);
}
+static void rmi_f12_set_hardcoded_desc(struct rmi_function *fn, struct f12_data *f12)
+{
+ struct rmi_2d_sensor *sensor = &f12->sensor;
+ struct rmi_register_desc_item *reg_desc;
+
+ /* We have no f12->data_reg_desc, so the pkt_size is 0, override it with
+ * a somewhat sensible default (this corresponds to 10 fingers).
+ */
+ sensor->pkt_size = 88;
+
+ /*
+ * There are no register descriptors to get these values from.
+ * We set them to high values to either be overwritten by the clip
+ * properties from devicetree, or to just not get in the way.
+ */
+ sensor->max_x = 65535;
+ sensor->max_y = 65535;
+
+ /*
+ * Create the Data1 register descriptor so that touch events
+ * can work properly.
+ */
+ reg_desc = devm_kcalloc(&fn->dev, 1,
+ sizeof(struct rmi_register_desc_item), GFP_KERNEL);
+ reg_desc->reg = 1;
+ reg_desc->reg_size = 80;
+ reg_desc->num_subpackets = 10;
+
+ f12->data1 = reg_desc;
+ f12->data1_offset = 0;
+ sensor->nbr_fingers = reg_desc->num_subpackets;
+ sensor->report_abs = 1;
+ sensor->attn_size += reg_desc->reg_size;
+}
+
static irqreturn_t rmi_f12_attention(int irq, void *ctx)
{
int retval;
@@ -315,6 +350,40 @@ static int rmi_f12_config(struct rmi_function *fn)
return 0;
}
+static int rmi_f12_sensor_init(struct rmi_function *fn, struct f12_data *f12)
+{
+ struct rmi_2d_sensor *sensor = &f12->sensor;
+
+ sensor->fn = fn;
+ f12->data_addr = fn->fd.data_base_addr;
+
+ /* On quirky devices that don't have a data_reg_desc we hardcode the packet
+ * in rmi_f12_set_hardcoded_desc(). Make sure not to set it to 0 here.
+ */
+ if (!sensor->pkt_size)
+ sensor->pkt_size = rmi_register_desc_calc_size(&f12->data_reg_desc);
+
+ sensor->axis_align =
+ f12->sensor_pdata.axis_align;
+
+ sensor->x_mm = f12->sensor_pdata.x_mm;
+ sensor->y_mm = f12->sensor_pdata.y_mm;
+ sensor->dribble = f12->sensor_pdata.dribble;
+
+ if (sensor->sensor_type == rmi_sensor_default)
+ sensor->sensor_type =
+ f12->sensor_pdata.sensor_type;
+
+ rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%s: data packet size: %d\n", __func__,
+ sensor->pkt_size);
+
+ sensor->data_pkt = devm_kzalloc(&fn->dev, sensor->pkt_size, GFP_KERNEL);
+ if (!sensor->data_pkt)
+ return -ENOMEM;
+
+ return 0;
+}
+
static int rmi_f12_probe(struct rmi_function *fn)
{
struct f12_data *f12;
@@ -328,6 +397,7 @@ static int rmi_f12_probe(struct rmi_function *fn)
struct rmi_driver_data *drvdata = dev_get_drvdata(&rmi_dev->dev);
u16 data_offset = 0;
int mask_size;
+ bool hardcoded_desc_quirk = false;
rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%s\n", __func__);
@@ -342,9 +412,9 @@ static int rmi_f12_probe(struct rmi_function *fn)
++query_addr;
if (!(buf & BIT(0))) {
- dev_err(&fn->dev,
- "Behavior of F12 without register descriptors is undefined.\n");
- return -ENODEV;
+ rmi_dbg(RMI_DEBUG_FN, &fn->dev,
+ "No register descriptors defined for F12, using fallback\n");
+ hardcoded_desc_quirk = true;
}
f12 = devm_kzalloc(&fn->dev, sizeof(struct f12_data) + mask_size * 2,
@@ -352,6 +422,8 @@ static int rmi_f12_probe(struct rmi_function *fn)
if (!f12)
return -ENOMEM;
+ dev_set_drvdata(&fn->dev, f12);
+
f12->abs_mask = (unsigned long *)((char *)f12
+ sizeof(struct f12_data));
f12->rel_mask = (unsigned long *)((char *)f12
@@ -370,6 +442,18 @@ static int rmi_f12_probe(struct rmi_function *fn)
f12->sensor_pdata = pdata->sensor_pdata;
}
+ sensor = &f12->sensor;
+
+ if (hardcoded_desc_quirk) {
+ rmi_f12_set_hardcoded_desc(fn, f12);
+
+ ret = rmi_f12_sensor_init(fn, f12);
+ if (ret)
+ return ret;
+
+ goto skip_register_desc;
+ }
+
ret = rmi_read_register_desc(rmi_dev, query_addr,
&f12->query_reg_desc);
if (ret) {
@@ -400,29 +484,9 @@ static int rmi_f12_probe(struct rmi_function *fn)
}
query_addr += 3;
- sensor = &f12->sensor;
- sensor->fn = fn;
- f12->data_addr = fn->fd.data_base_addr;
- sensor->pkt_size = rmi_register_desc_calc_size(&f12->data_reg_desc);
-
- sensor->axis_align =
- f12->sensor_pdata.axis_align;
-
- sensor->x_mm = f12->sensor_pdata.x_mm;
- sensor->y_mm = f12->sensor_pdata.y_mm;
- sensor->dribble = f12->sensor_pdata.dribble;
-
- if (sensor->sensor_type == rmi_sensor_default)
- sensor->sensor_type =
- f12->sensor_pdata.sensor_type;
-
- rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%s: data packet size: %d\n", __func__,
- sensor->pkt_size);
- sensor->data_pkt = devm_kzalloc(&fn->dev, sensor->pkt_size, GFP_KERNEL);
- if (!sensor->data_pkt)
- return -ENOMEM;
-
- dev_set_drvdata(&fn->dev, f12);
+ ret = rmi_f12_sensor_init(fn, f12);
+ if (ret)
+ return ret;
ret = rmi_f12_read_sensor_tuning(f12);
if (ret)
@@ -520,6 +584,7 @@ static int rmi_f12_probe(struct rmi_function *fn)
data_offset += item->reg_size;
}
+skip_register_desc:
/* allocate the in-kernel tracking buffers */
sensor->tracking_pos = devm_kcalloc(&fn->dev,
sensor->nbr_fingers, sizeof(struct input_mt_pos),
--
2.42.0
^ permalink raw reply related
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