* Re: [PATCH RFC v5 0/6] ARM: pxa: GPIO descriptor conversions
From: Andy Shevchenko @ 2023-10-05 9:14 UTC (permalink / raw)
To: Duje Mihanović
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Dmitry Torokhov, Mark Brown,
linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi
In-Reply-To: <20231004-pxa-gpio-v5-0-d99ae6fceea8@skole.hr>
On Wed, Oct 04, 2023 at 04:56:24PM +0200, Duje Mihanović wrote:
> Hello,
>
> Small series to convert some of the board files in the mach-pxa directory
> to use the new GPIO descriptor interface.
>
> Most notably, the am200epd, am300epd and Spitz matrix keypad among
> others are not converted in this series.
Why is it still RFC?
I believe it's already good enough to be considered as a real material.
OTOH "RFT" might make sense. I'm not sure there are any users on the
planet Earth that have this Sharp device up and running with newest
kernels.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [dtor-input:for-linus] BUILD SUCCESS 423622a90abb243944d1517b9f57db53729e45c4
From: kernel test robot @ 2023-10-05 3:11 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: 423622a90abb243944d1517b9f57db53729e45c4 Input: goodix - ensure int GPIO is in input for gpio_count == 1 && gpio_int_idx == 0 case
elapsed time: 727m
configs tested: 145
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 axs101_defconfig gcc
arc defconfig gcc
arc nsimosci_hs_smp_defconfig gcc
arc randconfig-001-20231004 gcc
arc randconfig-001-20231005 gcc
arm alldefconfig clang
arm allmodconfig gcc
arm allnoconfig gcc
arm allyesconfig gcc
arm bcm2835_defconfig clang
arm defconfig gcc
arm footbridge_defconfig gcc
arm orion5x_defconfig clang
arm pxa168_defconfig clang
arm pxa3xx_defconfig gcc
arm randconfig-001-20231004 gcc
arm randconfig-001-20231005 gcc
arm s5pv210_defconfig clang
arm sunxi_defconfig gcc
arm vf610m4_defconfig 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 buildonly-randconfig-001-20231004 gcc
i386 buildonly-randconfig-001-20231005 gcc
i386 buildonly-randconfig-002-20231004 gcc
i386 buildonly-randconfig-002-20231005 gcc
i386 buildonly-randconfig-003-20231004 gcc
i386 buildonly-randconfig-003-20231005 gcc
i386 buildonly-randconfig-004-20231004 gcc
i386 buildonly-randconfig-004-20231005 gcc
i386 buildonly-randconfig-005-20231004 gcc
i386 buildonly-randconfig-005-20231005 gcc
i386 buildonly-randconfig-006-20231004 gcc
i386 buildonly-randconfig-006-20231005 gcc
i386 debian-10.3 gcc
i386 defconfig gcc
i386 randconfig-001-20231005 gcc
i386 randconfig-002-20231005 gcc
i386 randconfig-003-20231005 gcc
i386 randconfig-004-20231005 gcc
i386 randconfig-005-20231005 gcc
i386 randconfig-006-20231005 gcc
loongarch allmodconfig gcc
loongarch allnoconfig gcc
loongarch allyesconfig gcc
loongarch defconfig gcc
loongarch randconfig-001-20231004 gcc
loongarch randconfig-001-20231005 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 decstation_64_defconfig gcc
mips malta_defconfig clang
nios2 allmodconfig gcc
nios2 allnoconfig gcc
nios2 allyesconfig gcc
nios2 defconfig gcc
openrisc alldefconfig 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 bamboo_defconfig gcc
powerpc ep88xc_defconfig gcc
powerpc klondike_defconfig gcc
powerpc mpc5200_defconfig clang
powerpc skiroot_defconfig clang
powerpc storcenter_defconfig gcc
powerpc wii_defconfig gcc
powerpc xes_mpc85xx_defconfig clang
riscv allmodconfig gcc
riscv allnoconfig clang
riscv allnoconfig gcc
riscv allyesconfig gcc
riscv defconfig gcc
riscv nommu_virt_defconfig clang
riscv rv32_defconfig gcc
s390 alldefconfig clang
s390 allmodconfig gcc
s390 allnoconfig gcc
s390 allyesconfig gcc
s390 defconfig gcc
sh allmodconfig gcc
sh allnoconfig gcc
sh allyesconfig gcc
sh defconfig gcc
sh rts7751r2dplus_defconfig gcc
sh sdk7780_defconfig gcc
sh sh03_defconfig gcc
sparc allmodconfig gcc
sparc allnoconfig gcc
sparc allyesconfig gcc
sparc defconfig 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-20231005 gcc
x86_64 randconfig-002-20231005 gcc
x86_64 randconfig-003-20231005 gcc
x86_64 randconfig-004-20231005 gcc
x86_64 randconfig-005-20231005 gcc
x86_64 randconfig-006-20231005 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
* [PATCH] Input: max77693-haptic - add device-tree compatible strings
From: Marek Szyprowski @ 2023-10-05 11:48 UTC (permalink / raw)
To: linux-input, linux-kernel
Cc: Marek Szyprowski, Dmitry Torokhov, Krzysztof Kozlowski
In-Reply-To: <CGME20231005114823eucas1p2db2015fae87787ca776ad987f2fe6f5d@eucas1p2.samsung.com>
Add the needed device-tree compatible strings to the MAX77693 haptic
driver, so it can be automatically loaded when compiled as a kernel
module and given device-tree contains separate (i.e. 'motor-driver')
node under the main PMIC node.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
drivers/input/misc/max77693-haptic.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/input/misc/max77693-haptic.c b/drivers/input/misc/max77693-haptic.c
index 80f4416ffe2f..2a6e713b32f4 100644
--- a/drivers/input/misc/max77693-haptic.c
+++ b/drivers/input/misc/max77693-haptic.c
@@ -412,6 +412,13 @@ static const struct platform_device_id max77693_haptic_id[] = {
};
MODULE_DEVICE_TABLE(platform, max77693_haptic_id);
+static const struct of_device_id of_max77693_haptic_dt_match[] = {
+ { .compatible = "maxim,max77693-haptic", },
+ { .compatible = "maxim,max77843-haptic", },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, of_max77693_haptic_dt_match);
+
static struct platform_driver max77693_haptic_driver = {
.driver = {
.name = "max77693-haptic",
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v2] HID: i2c-hid: fix handling of unpopulated devices
From: Benjamin Tissoires @ 2023-10-05 7:10 UTC (permalink / raw)
To: Johan Hovold
Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel,
Douglas Anderson, Maxime Ripard
In-Reply-To: <20231002155857.24584-1-johan+linaro@kernel.org>
On Oct 02 2023, Johan Hovold wrote:
> A recent commit reordered probe so that the interrupt line is now
> requested before making sure that the device exists.
>
> This breaks machines like the Lenovo ThinkPad X13s which rely on the
> HID driver to probe second-source devices and only register the variant
> that is actually populated. Specifically, the interrupt line may now
> already be (temporarily) claimed when doing asynchronous probing of the
> touchpad:
>
> genirq: Flags mismatch irq 191. 00082008 (hid-over-i2c) vs. 00082008 (hid-over-i2c)
> i2c_hid_of 21-0015: Could not register for hid-over-i2c interrupt, irq = 191, ret = -16
> i2c_hid_of: probe of 21-0015 failed with error -16
>
> Fix this by restoring the old behaviour of first making sure the device
> exists before requesting the interrupt line.
>
> Note that something like this should probably be implemented also for
> "panel followers", whose actual probe is currently effectively deferred
> until the DRM panel is probed (e.g. by powering down the device after
> making sure it exists and only then register it as a follower).
>
> Fixes: 675cd877c952 ("HID: i2c-hid: Rearrange probe() to power things up later")
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Maxime Ripard <mripard@kernel.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>
> Changes in v2
> - initialise ihid->is_panel_follower sooner to avoid repeated property
> lookups and so that it can be used consistently throughout the driver
> for code that differs for "panel followers"
Patches looks good to me, but I can not test it unfortunately.
Doug, would you mind sending us your Ack or tested-by?
Cheers,
Benjamin
>
> Link to v1: https://lore.kernel.org/lkml/20230918125851.310-1-johan+linaro@kernel.org/
>
>
> drivers/hid/i2c-hid/i2c-hid-core.c | 144 ++++++++++++++++-------------
> 1 file changed, 81 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
> index 9601c0605fd9..2735cd585af0 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -998,45 +998,29 @@ static int i2c_hid_core_resume(struct i2c_hid *ihid)
> return hid_driver_reset_resume(hid);
> }
>
> -/**
> - * __do_i2c_hid_core_initial_power_up() - First time power up of the i2c-hid device.
> - * @ihid: The ihid object created during probe.
> - *
> - * This function is called at probe time.
> - *
> - * The initial power on is where we do some basic validation that the device
> - * exists, where we fetch the HID descriptor, and where we create the actual
> - * HID devices.
> - *
> - * Return: 0 or error code.
> +/*
> + * Check that the device exists and parse the HID descriptor.
> */
> -static int __do_i2c_hid_core_initial_power_up(struct i2c_hid *ihid)
> +static int __i2c_hid_core_probe(struct i2c_hid *ihid)
> {
> struct i2c_client *client = ihid->client;
> struct hid_device *hid = ihid->hid;
> int ret;
>
> - ret = i2c_hid_core_power_up(ihid);
> - if (ret)
> - return ret;
> -
> /* Make sure there is something at this address */
> ret = i2c_smbus_read_byte(client);
> if (ret < 0) {
> i2c_hid_dbg(ihid, "nothing at this address: %d\n", ret);
> - ret = -ENXIO;
> - goto err;
> + return -ENXIO;
> }
>
> ret = i2c_hid_fetch_hid_descriptor(ihid);
> if (ret < 0) {
> dev_err(&client->dev,
> "Failed to fetch the HID Descriptor\n");
> - goto err;
> + return ret;
> }
>
> - enable_irq(client->irq);
> -
> hid->version = le16_to_cpu(ihid->hdesc.bcdVersion);
> hid->vendor = le16_to_cpu(ihid->hdesc.wVendorID);
> hid->product = le16_to_cpu(ihid->hdesc.wProductID);
> @@ -1050,17 +1034,49 @@ static int __do_i2c_hid_core_initial_power_up(struct i2c_hid *ihid)
>
> ihid->quirks = i2c_hid_lookup_quirk(hid->vendor, hid->product);
>
> + return 0;
> +}
> +
> +static int i2c_hid_core_register_hid(struct i2c_hid *ihid)
> +{
> + struct i2c_client *client = ihid->client;
> + struct hid_device *hid = ihid->hid;
> + int ret;
> +
> + enable_irq(client->irq);
> +
> ret = hid_add_device(hid);
> if (ret) {
> if (ret != -ENODEV)
> hid_err(client, "can't add hid device: %d\n", ret);
> - goto err;
> + disable_irq(client->irq);
> + return ret;
> }
>
> return 0;
> +}
> +
> +static int i2c_hid_core_probe_panel_follower(struct i2c_hid *ihid)
> +{
> + int ret;
> +
> + ret = i2c_hid_core_power_up(ihid);
> + if (ret)
> + return ret;
>
> -err:
> + ret = __i2c_hid_core_probe(ihid);
> + if (ret)
> + goto err_power_down;
> +
> + ret = i2c_hid_core_register_hid(ihid);
> + if (ret)
> + goto err_power_down;
> +
> + return 0;
> +
> +err_power_down:
> i2c_hid_core_power_down(ihid);
> +
> return ret;
> }
>
> @@ -1077,7 +1093,7 @@ static void ihid_core_panel_prepare_work(struct work_struct *work)
> * steps.
> */
> if (!hid->version)
> - ret = __do_i2c_hid_core_initial_power_up(ihid);
> + ret = i2c_hid_core_probe_panel_follower(ihid);
> else
> ret = i2c_hid_core_resume(ihid);
>
> @@ -1136,7 +1152,6 @@ static int i2c_hid_core_register_panel_follower(struct i2c_hid *ihid)
> struct device *dev = &ihid->client->dev;
> int ret;
>
> - ihid->is_panel_follower = true;
> ihid->panel_follower.funcs = &i2c_hid_core_panel_follower_funcs;
>
> /*
> @@ -1156,30 +1171,6 @@ static int i2c_hid_core_register_panel_follower(struct i2c_hid *ihid)
> return 0;
> }
>
> -static int i2c_hid_core_initial_power_up(struct i2c_hid *ihid)
> -{
> - /*
> - * If we're a panel follower, we'll register and do our initial power
> - * up when the panel turns on; otherwise we do it right away.
> - */
> - if (drm_is_panel_follower(&ihid->client->dev))
> - return i2c_hid_core_register_panel_follower(ihid);
> - else
> - return __do_i2c_hid_core_initial_power_up(ihid);
> -}
> -
> -static void i2c_hid_core_final_power_down(struct i2c_hid *ihid)
> -{
> - /*
> - * If we're a follower, the act of unfollowing will cause us to be
> - * powered down. Otherwise we need to manually do it.
> - */
> - if (ihid->is_panel_follower)
> - drm_panel_remove_follower(&ihid->panel_follower);
> - else
> - i2c_hid_core_suspend(ihid, true);
> -}
> -
> int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
> u16 hid_descriptor_address, u32 quirks)
> {
> @@ -1211,6 +1202,7 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
> ihid->ops = ops;
> ihid->client = client;
> ihid->wHIDDescRegister = cpu_to_le16(hid_descriptor_address);
> + ihid->is_panel_follower = drm_is_panel_follower(&client->dev);
>
> init_waitqueue_head(&ihid->wait);
> mutex_init(&ihid->reset_lock);
> @@ -1224,14 +1216,10 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
> return ret;
> device_enable_async_suspend(&client->dev);
>
> - ret = i2c_hid_init_irq(client);
> - if (ret < 0)
> - goto err_buffers_allocated;
> -
> hid = hid_allocate_device();
> if (IS_ERR(hid)) {
> ret = PTR_ERR(hid);
> - goto err_irq;
> + goto err_free_buffers;
> }
>
> ihid->hid = hid;
> @@ -1242,19 +1230,42 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
> hid->bus = BUS_I2C;
> hid->initial_quirks = quirks;
>
> - ret = i2c_hid_core_initial_power_up(ihid);
> + /* Power on and probe unless device is a panel follower. */
> + if (!ihid->is_panel_follower) {
> + ret = i2c_hid_core_power_up(ihid);
> + if (ret < 0)
> + goto err_destroy_device;
> +
> + ret = __i2c_hid_core_probe(ihid);
> + if (ret < 0)
> + goto err_power_down;
> + }
> +
> + ret = i2c_hid_init_irq(client);
> + if (ret < 0)
> + goto err_power_down;
> +
> + /*
> + * If we're a panel follower, we'll register when the panel turns on;
> + * otherwise we do it right away.
> + */
> + if (ihid->is_panel_follower)
> + ret = i2c_hid_core_register_panel_follower(ihid);
> + else
> + ret = i2c_hid_core_register_hid(ihid);
> if (ret)
> - goto err_mem_free;
> + goto err_free_irq;
>
> return 0;
>
> -err_mem_free:
> - hid_destroy_device(hid);
> -
> -err_irq:
> +err_free_irq:
> free_irq(client->irq, ihid);
> -
> -err_buffers_allocated:
> +err_power_down:
> + if (!ihid->is_panel_follower)
> + i2c_hid_core_power_down(ihid);
> +err_destroy_device:
> + hid_destroy_device(hid);
> +err_free_buffers:
> i2c_hid_free_buffers(ihid);
>
> return ret;
> @@ -1266,7 +1277,14 @@ void i2c_hid_core_remove(struct i2c_client *client)
> struct i2c_hid *ihid = i2c_get_clientdata(client);
> struct hid_device *hid;
>
> - i2c_hid_core_final_power_down(ihid);
> + /*
> + * If we're a follower, the act of unfollowing will cause us to be
> + * powered down. Otherwise we need to manually do it.
> + */
> + if (ihid->is_panel_follower)
> + drm_panel_remove_follower(&ihid->panel_follower);
> + else
> + i2c_hid_core_suspend(ihid, true);
>
> hid = ihid->hid;
> hid_destroy_device(hid);
> --
> 2.41.0
>
^ permalink raw reply
* [PATCH v3 0/3] selftests/hid: assorted fixes
From: Benjamin Tissoires @ 2023-10-05 15:55 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Justin Stitt,
Nick Desaulniers, Eduard Zingerman
Cc: linux-input, linux-kselftest, linux-kernel, bpf,
Benjamin Tissoires, Shuah Khan
And this is the last(?) revision of this series which should now compile
with or without CONFIG_HID_BPF set.
I had to do changes because [1] was failing
Nick, I kept your Tested-by, even if I made small changes in 1/3. Feel
free to shout if you don't want me to keep it.
Eduard, You helped us a lot in the review of v1 but never sent your
Reviewed-by or Acked-by. Do you want me to add one?
Cheers,
Benjamin
[1] https://gitlab.freedesktop.org/bentiss/hid/-/jobs/49754306
For reference, the v2 cover letter:
| Hi, I am sending this series on behalf of myself and Benjamin Tissoires. There
| existed an initial n=3 patch series which was later expanded to n=4 and
| is now back to n=3 with some fixes added in and rebased against
| mainline.
|
| This patch series aims to ensure that the hid/bpf selftests can be built
| without errors.
|
| Here's Benjamin's initial cover letter for context:
| | These fixes have been triggered by [0]:
| | basically, if you do not recompile the kernel first, and are
| | running on an old kernel, vmlinux.h doesn't have the required
| | symbols and the compilation fails.
| |
| | The tests will fail if you run them on that very same machine,
| | of course, but the binary should compile.
| |
| | And while I was sorting out why it was failing, I realized I
| | could do a couple of improvements on the Makefile.
| |
| | [0] https://lore.kernel.org/linux-input/56ba8125-2c6f-a9c9-d498-0ca1c153dcb2@redhat.com/T/#t
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
Changes in v3:
- Also overwrite all of the enum symbols in patch 1/3
- Link to v2: https://lore.kernel.org/r/20230908-kselftest-09-08-v2-0-0def978a4c1b@google.com
Changes in v2:
- roll Justin's fix into patch 1/3
- add __attribute__((preserve_access_index)) (thanks Eduard)
- rebased onto mainline (2dde18cd1d8fac735875f2e4987f11817cc0bc2c)
- Link to v1: https://lore.kernel.org/r/20230825-wip-selftests-v1-0-c862769020a8@kernel.org
Link: https://github.com/ClangBuiltLinux/linux/issues/1698
Link: https://github.com/ClangBuiltLinux/continuous-integration2/issues/61
---
Benjamin Tissoires (3):
selftests/hid: ensure we can compile the tests on kernels pre-6.3
selftests/hid: do not manually call headers_install
selftests/hid: force using our compiled libbpf headers
tools/testing/selftests/hid/Makefile | 10 ++-
tools/testing/selftests/hid/progs/hid.c | 3 -
.../testing/selftests/hid/progs/hid_bpf_helpers.h | 77 ++++++++++++++++++++++
3 files changed, 81 insertions(+), 9 deletions(-)
---
base-commit: 29aa98d0fe013e2ab62aae4266231b7fb05d47a2
change-id: 20230825-wip-selftests-9a7502b56542
Best regards,
--
Benjamin Tissoires <bentiss@kernel.org>
^ permalink raw reply
* Re: [PATCH RFC v5 0/6] ARM: pxa: GPIO descriptor conversions
From: Duje Mihanović @ 2023-10-05 15:52 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Dmitry Torokhov, Mark Brown,
linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi
In-Reply-To: <ZR5+XWBmg0I7joOg@smile.fi.intel.com>
On Thursday, October 5, 2023 11:14:05 AM CEST Andy Shevchenko wrote:
> On Wed, Oct 04, 2023 at 04:56:24PM +0200, Duje Mihanović wrote:
> > Hello,
> >
> > Small series to convert some of the board files in the mach-pxa directory
> > to use the new GPIO descriptor interface.
> >
> > Most notably, the am200epd, am300epd and Spitz matrix keypad among
> > others are not converted in this series.
>
> Why is it still RFC?
> I believe it's already good enough to be considered as a real material.
> OTOH "RFT" might make sense. I'm not sure there are any users on the
> planet Earth that have this Sharp device up and running with newest
> kernels.
Will add RFT in an eventual v6.
Regards,
Duje
^ permalink raw reply
* [PATCH v3 1/3] selftests/hid: ensure we can compile the tests on kernels pre-6.3
From: Benjamin Tissoires @ 2023-10-05 15:55 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Justin Stitt,
Nick Desaulniers, Eduard Zingerman
Cc: linux-input, linux-kselftest, linux-kernel, bpf,
Benjamin Tissoires
In-Reply-To: <20230825-wip-selftests-v3-0-639963c54109@kernel.org>
For the hid-bpf tests to compile, we need to have the definition of
struct hid_bpf_ctx. This definition is an internal one from the kernel
and it is supposed to be defined in the generated vmlinux.h.
This vmlinux.h header is generated based on the currently running kernel
or if the kernel was already compiled in the tree. If you just compile
the selftests without compiling the kernel beforehand and you are running
on a 6.2 kernel, you'll end up with a vmlinux.h without the hid_bpf_ctx
definition.
Use the clever trick from tools/testing/selftests/bpf/progs/bpf_iter.h
to force the definition of that symbol in case we don't find it in the
BTF and also add __attribute__((preserve_access_index)) to further
support CO-RE functionality for these tests.
Signed-off-by: Justin Stitt <justinstitt@google.com>
Tested-by: Nick Desaulniers <ndesaulniers@google.com> # Build
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
tools/testing/selftests/hid/progs/hid.c | 3 -
.../testing/selftests/hid/progs/hid_bpf_helpers.h | 77 ++++++++++++++++++++++
2 files changed, 77 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/hid/progs/hid.c b/tools/testing/selftests/hid/progs/hid.c
index 88c593f753b5..1e558826b809 100644
--- a/tools/testing/selftests/hid/progs/hid.c
+++ b/tools/testing/selftests/hid/progs/hid.c
@@ -1,8 +1,5 @@
// SPDX-License-Identifier: GPL-2.0
/* Copyright (c) 2022 Red hat */
-#include "vmlinux.h"
-#include <bpf/bpf_helpers.h>
-#include <bpf/bpf_tracing.h>
#include "hid_bpf_helpers.h"
char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
index 4fff31dbe0e7..65e657ac1198 100644
--- a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
+++ b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
@@ -5,6 +5,83 @@
#ifndef __HID_BPF_HELPERS_H
#define __HID_BPF_HELPERS_H
+/* "undefine" structs and enums in vmlinux.h, because we "override" them below */
+#define hid_bpf_ctx hid_bpf_ctx___not_used
+#define hid_report_type hid_report_type___not_used
+#define hid_class_request hid_class_request___not_used
+#define hid_bpf_attach_flags hid_bpf_attach_flags___not_used
+#define HID_INPUT_REPORT HID_INPUT_REPORT___not_used
+#define HID_OUTPUT_REPORT HID_OUTPUT_REPORT___not_used
+#define HID_FEATURE_REPORT HID_FEATURE_REPORT___not_used
+#define HID_REPORT_TYPES HID_REPORT_TYPES___not_used
+#define HID_REQ_GET_REPORT HID_REQ_GET_REPORT___not_used
+#define HID_REQ_GET_IDLE HID_REQ_GET_IDLE___not_used
+#define HID_REQ_GET_PROTOCOL HID_REQ_GET_PROTOCOL___not_used
+#define HID_REQ_SET_REPORT HID_REQ_SET_REPORT___not_used
+#define HID_REQ_SET_IDLE HID_REQ_SET_IDLE___not_used
+#define HID_REQ_SET_PROTOCOL HID_REQ_SET_PROTOCOL___not_used
+#define HID_BPF_FLAG_NONE HID_BPF_FLAG_NONE___not_used
+#define HID_BPF_FLAG_INSERT_HEAD HID_BPF_FLAG_INSERT_HEAD___not_used
+#define HID_BPF_FLAG_MAX HID_BPF_FLAG_MAX___not_used
+
+#include "vmlinux.h"
+
+#undef hid_bpf_ctx
+#undef hid_report_type
+#undef hid_class_request
+#undef hid_bpf_attach_flags
+#undef HID_INPUT_REPORT
+#undef HID_OUTPUT_REPORT
+#undef HID_FEATURE_REPORT
+#undef HID_REPORT_TYPES
+#undef HID_REQ_GET_REPORT
+#undef HID_REQ_GET_IDLE
+#undef HID_REQ_GET_PROTOCOL
+#undef HID_REQ_SET_REPORT
+#undef HID_REQ_SET_IDLE
+#undef HID_REQ_SET_PROTOCOL
+#undef HID_BPF_FLAG_NONE
+#undef HID_BPF_FLAG_INSERT_HEAD
+#undef HID_BPF_FLAG_MAX
+
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include <linux/const.h>
+
+enum hid_report_type {
+ HID_INPUT_REPORT = 0,
+ HID_OUTPUT_REPORT = 1,
+ HID_FEATURE_REPORT = 2,
+
+ HID_REPORT_TYPES,
+};
+
+struct hid_bpf_ctx {
+ __u32 index;
+ const struct hid_device *hid;
+ __u32 allocated_size;
+ enum hid_report_type report_type;
+ union {
+ __s32 retval;
+ __s32 size;
+ };
+} __attribute__((preserve_access_index));
+
+enum hid_class_request {
+ HID_REQ_GET_REPORT = 0x01,
+ HID_REQ_GET_IDLE = 0x02,
+ HID_REQ_GET_PROTOCOL = 0x03,
+ HID_REQ_SET_REPORT = 0x09,
+ HID_REQ_SET_IDLE = 0x0A,
+ HID_REQ_SET_PROTOCOL = 0x0B,
+};
+
+enum hid_bpf_attach_flags {
+ HID_BPF_FLAG_NONE = 0,
+ HID_BPF_FLAG_INSERT_HEAD = _BITUL(0),
+ HID_BPF_FLAG_MAX,
+};
+
/* following are kfuncs exported by HID for HID-BPF */
extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx,
unsigned int offset,
--
2.39.1
^ permalink raw reply related
* [PATCH v3 2/3] selftests/hid: do not manually call headers_install
From: Benjamin Tissoires @ 2023-10-05 15:55 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Justin Stitt,
Nick Desaulniers, Eduard Zingerman
Cc: linux-input, linux-kselftest, linux-kernel, bpf,
Benjamin Tissoires, Shuah Khan
In-Reply-To: <20230825-wip-selftests-v3-0-639963c54109@kernel.org>
"make headers" is a requirement before calling make on the selftests
dir, so we should not have to manually install those headers
Acked-by: Shuah Khan <skhan@linuxfoundation.org>
Tested-by: Nick Desaulniers <ndesaulniers@google.com> # Build
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
tools/testing/selftests/hid/Makefile | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/hid/Makefile b/tools/testing/selftests/hid/Makefile
index 2e986cbf1a46..a28054113f47 100644
--- a/tools/testing/selftests/hid/Makefile
+++ b/tools/testing/selftests/hid/Makefile
@@ -21,7 +21,7 @@ CXX ?= $(CROSS_COMPILE)g++
HOSTPKG_CONFIG := pkg-config
-CFLAGS += -g -O0 -rdynamic -Wall -Werror -I$(KHDR_INCLUDES) -I$(OUTPUT)
+CFLAGS += -g -O0 -rdynamic -Wall -Werror -I$(OUTPUT)
LDLIBS += -lelf -lz -lrt -lpthread
# Silence some warnings when compiled with clang
@@ -65,7 +65,6 @@ BPFTOOLDIR := $(TOOLSDIR)/bpf/bpftool
SCRATCH_DIR := $(OUTPUT)/tools
BUILD_DIR := $(SCRATCH_DIR)/build
INCLUDE_DIR := $(SCRATCH_DIR)/include
-KHDR_INCLUDES := $(SCRATCH_DIR)/uapi/include
BPFOBJ := $(BUILD_DIR)/libbpf/libbpf.a
ifneq ($(CROSS_COMPILE),)
HOST_BUILD_DIR := $(BUILD_DIR)/host
@@ -151,9 +150,6 @@ else
$(Q)cp "$(VMLINUX_H)" $@
endif
-$(KHDR_INCLUDES)/linux/hid.h: $(top_srcdir)/include/uapi/linux/hid.h
- $(MAKE) -C $(top_srcdir) INSTALL_HDR_PATH=$(SCRATCH_DIR)/uapi headers_install
-
$(RESOLVE_BTFIDS): $(HOST_BPFOBJ) | $(HOST_BUILD_DIR)/resolve_btfids \
$(TOOLSDIR)/bpf/resolve_btfids/main.c \
$(TOOLSDIR)/lib/rbtree.c \
@@ -231,7 +227,7 @@ $(BPF_SKELS): %.skel.h: %.bpf.o $(BPFTOOL) | $(OUTPUT)
$(Q)$(BPFTOOL) gen object $(<:.o=.linked1.o) $<
$(Q)$(BPFTOOL) gen skeleton $(<:.o=.linked1.o) name $(notdir $(<:.bpf.o=)) > $@
-$(OUTPUT)/%.o: %.c $(BPF_SKELS) $(KHDR_INCLUDES)/linux/hid.h
+$(OUTPUT)/%.o: %.c $(BPF_SKELS)
$(call msg,CC,,$@)
$(Q)$(CC) $(CFLAGS) -c $(filter %.c,$^) $(LDLIBS) -o $@
--
2.39.1
^ permalink raw reply related
* [PATCH v3 3/3] selftests/hid: force using our compiled libbpf headers
From: Benjamin Tissoires @ 2023-10-05 15:55 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Justin Stitt,
Nick Desaulniers, Eduard Zingerman
Cc: linux-input, linux-kselftest, linux-kernel, bpf,
Benjamin Tissoires
In-Reply-To: <20230825-wip-selftests-v3-0-639963c54109@kernel.org>
Turns out that we were relying on the globally installed headers, not
the ones we freshly compiled.
Add a manual include in CFLAGS to sort this out.
Tested-by: Nick Desaulniers <ndesaulniers@google.com> # Build
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
tools/testing/selftests/hid/Makefile | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/testing/selftests/hid/Makefile b/tools/testing/selftests/hid/Makefile
index a28054113f47..2b5ea18bde38 100644
--- a/tools/testing/selftests/hid/Makefile
+++ b/tools/testing/selftests/hid/Makefile
@@ -22,6 +22,8 @@ CXX ?= $(CROSS_COMPILE)g++
HOSTPKG_CONFIG := pkg-config
CFLAGS += -g -O0 -rdynamic -Wall -Werror -I$(OUTPUT)
+CFLAGS += -I$(OUTPUT)/tools/include
+
LDLIBS += -lelf -lz -lrt -lpthread
# Silence some warnings when compiled with clang
--
2.39.1
^ permalink raw reply related
* Re: [PATCH v2] HID: i2c-hid: fix handling of unpopulated devices
From: Doug Anderson @ 2023-10-05 16:31 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Johan Hovold, Jiri Kosina, Benjamin Tissoires, linux-input,
linux-kernel, Maxime Ripard
In-Reply-To: <cnnjghxj4od6fniotztlgorc7myzya2bsvixkf2cpk4metcipa@r2w2ouob5ths>
Hi,
On Thu, Oct 5, 2023 at 12:10 AM Benjamin Tissoires <bentiss@kernel.org> wrote:
>
> On Oct 02 2023, Johan Hovold wrote:
> > A recent commit reordered probe so that the interrupt line is now
> > requested before making sure that the device exists.
> >
> > This breaks machines like the Lenovo ThinkPad X13s which rely on the
> > HID driver to probe second-source devices and only register the variant
> > that is actually populated. Specifically, the interrupt line may now
> > already be (temporarily) claimed when doing asynchronous probing of the
> > touchpad:
> >
> > genirq: Flags mismatch irq 191. 00082008 (hid-over-i2c) vs. 00082008 (hid-over-i2c)
> > i2c_hid_of 21-0015: Could not register for hid-over-i2c interrupt, irq = 191, ret = -16
> > i2c_hid_of: probe of 21-0015 failed with error -16
> >
> > Fix this by restoring the old behaviour of first making sure the device
> > exists before requesting the interrupt line.
> >
> > Note that something like this should probably be implemented also for
> > "panel followers", whose actual probe is currently effectively deferred
> > until the DRM panel is probed (e.g. by powering down the device after
> > making sure it exists and only then register it as a follower).
> >
> > Fixes: 675cd877c952 ("HID: i2c-hid: Rearrange probe() to power things up later")
> > Cc: Douglas Anderson <dianders@chromium.org>
> > Cc: Maxime Ripard <mripard@kernel.org>
> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> > ---
> >
> > Changes in v2
> > - initialise ihid->is_panel_follower sooner to avoid repeated property
> > lookups and so that it can be used consistently throughout the driver
> > for code that differs for "panel followers"
>
> Patches looks good to me, but I can not test it unfortunately.
>
> Doug, would you mind sending us your Ack or tested-by?
Sure. Patches look OK to me, so if you're good with them then I'm good
with them too.
Reviewed-by: Douglas Anderson <dianders@chromium.org>
I tested this on a board by adding a second i2c-hid device in the
device tree and confirming that things appeared to work OK. I also
tried this with a board setup to use panel-follower (but _not_ two
sources of touchscreens) and that also worked OK. Thus:
Tested-by: Douglas Anderson <dianders@chromium.org>
As expected, combining panel-follower with two sources of touchscreen
_didn't_ work because only one of them is able to acquire the
interrupt. This is fine with me as there is nobody currently doing
that. I'm still of the belief that we need a more complete solution
for that and I'll continue to work on it.
Thanks!
-Doug
^ permalink raw reply
* Re: [PATCH] Input: max77693-haptic - add device-tree compatible strings
From: kernel test robot @ 2023-10-05 17:02 UTC (permalink / raw)
To: Marek Szyprowski, linux-input, linux-kernel
Cc: oe-kbuild-all, Marek Szyprowski, Dmitry Torokhov,
Krzysztof Kozlowski
In-Reply-To: <20231005114816.1101953-1-m.szyprowski@samsung.com>
Hi Marek,
kernel test robot noticed the following build warnings:
[auto build test WARNING on dtor-input/next]
[also build test WARNING on dtor-input/for-linus hid/for-next linus/master v6.6-rc4 next-20231005]
[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/Marek-Szyprowski/Input-max77693-haptic-add-device-tree-compatible-strings/20231005-231602
base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
patch link: https://lore.kernel.org/r/20231005114816.1101953-1-m.szyprowski%40samsung.com
patch subject: [PATCH] Input: max77693-haptic - add device-tree compatible strings
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20231006/202310060002.ucD2eiLJ-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231006/202310060002.ucD2eiLJ-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/202310060002.ucD2eiLJ-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/input/misc/max77693-haptic.c:415:34: warning: 'of_max77693_haptic_dt_match' defined but not used [-Wunused-const-variable=]
415 | static const struct of_device_id of_max77693_haptic_dt_match[] = {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/of_max77693_haptic_dt_match +415 drivers/input/misc/max77693-haptic.c
414
> 415 static const struct of_device_id of_max77693_haptic_dt_match[] = {
416 { .compatible = "maxim,max77693-haptic", },
417 { .compatible = "maxim,max77843-haptic", },
418 { /* sentinel */ },
419 };
420 MODULE_DEVICE_TABLE(of, of_max77693_haptic_dt_match);
421
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v3 0/3] selftests/hid: assorted fixes
From: Justin Stitt @ 2023-10-05 18:00 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Benjamin Tissoires, Shuah Khan, Nick Desaulniers,
Eduard Zingerman, linux-input, linux-kselftest, linux-kernel, bpf,
Shuah Khan
In-Reply-To: <20230825-wip-selftests-v3-0-639963c54109@kernel.org>
On Thu, Oct 5, 2023 at 8:55 AM Benjamin Tissoires <bentiss@kernel.org> wrote:
>
> And this is the last(?) revision of this series which should now compile
> with or without CONFIG_HID_BPF set.
>
> I had to do changes because [1] was failing
>
> Nick, I kept your Tested-by, even if I made small changes in 1/3. Feel
> free to shout if you don't want me to keep it.
>
> Eduard, You helped us a lot in the review of v1 but never sent your
> Reviewed-by or Acked-by. Do you want me to add one?
>
> Cheers,
> Benjamin
>
> [1] https://gitlab.freedesktop.org/bentiss/hid/-/jobs/49754306
>
> For reference, the v2 cover letter:
>
> | Hi, I am sending this series on behalf of myself and Benjamin Tissoires. There
> | existed an initial n=3 patch series which was later expanded to n=4 and
> | is now back to n=3 with some fixes added in and rebased against
> | mainline.
> |
> | This patch series aims to ensure that the hid/bpf selftests can be built
> | without errors.
> |
> | Here's Benjamin's initial cover letter for context:
> | | These fixes have been triggered by [0]:
> | | basically, if you do not recompile the kernel first, and are
> | | running on an old kernel, vmlinux.h doesn't have the required
> | | symbols and the compilation fails.
> | |
> | | The tests will fail if you run them on that very same machine,
> | | of course, but the binary should compile.
> | |
> | | And while I was sorting out why it was failing, I realized I
> | | could do a couple of improvements on the Makefile.
> | |
> | | [0] https://lore.kernel.org/linux-input/56ba8125-2c6f-a9c9-d498-0ca1c153dcb2@redhat.com/T/#t
>
> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> ---
> Changes in v3:
> - Also overwrite all of the enum symbols in patch 1/3
> - Link to v2: https://lore.kernel.org/r/20230908-kselftest-09-08-v2-0-0def978a4c1b@google.com
>
> Changes in v2:
> - roll Justin's fix into patch 1/3
> - add __attribute__((preserve_access_index)) (thanks Eduard)
> - rebased onto mainline (2dde18cd1d8fac735875f2e4987f11817cc0bc2c)
> - Link to v1: https://lore.kernel.org/r/20230825-wip-selftests-v1-0-c862769020a8@kernel.org
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1698
> Link: https://github.com/ClangBuiltLinux/continuous-integration2/issues/61
>
> ---
> Benjamin Tissoires (3):
> selftests/hid: ensure we can compile the tests on kernels pre-6.3
> selftests/hid: do not manually call headers_install
> selftests/hid: force using our compiled libbpf headers
>
> tools/testing/selftests/hid/Makefile | 10 ++-
> tools/testing/selftests/hid/progs/hid.c | 3 -
> .../testing/selftests/hid/progs/hid_bpf_helpers.h | 77 ++++++++++++++++++++++
> 3 files changed, 81 insertions(+), 9 deletions(-)
> ---
> base-commit: 29aa98d0fe013e2ab62aae4266231b7fb05d47a2
> change-id: 20230825-wip-selftests-9a7502b56542
>
> Best regards,
> --
> Benjamin Tissoires <bentiss@kernel.org>
>
Tested entire series.
I can now build the tests using this command:
$ make LLVM=1 -j128 ARCH=x86_64 mrproper headers && make LLVM=1 -j128
ARCH=x86_64 -C tools/testing/selftests TARGETS=hid
Tested-by: Justin Stitt <justinstitt@google.com>
^ permalink raw reply
* [PATCH] HID: logitech-hidpp: Fix kernel crash on receiver USB disconnect
From: Hans de Goede @ 2023-10-05 18:26 UTC (permalink / raw)
To: Filipe Laíns, Bastien Nocera, Jiri Kosina,
Benjamin Tissoires
Cc: Hans de Goede, linux-input, stable
hidpp_connect_event() has *four* time-of-check vs time-of-use (TOCTOU)
races when it races with itself.
hidpp_connect_event() primarily runs from a workqueue but it also runs
on probe() and if a "device-connected" packet is received by the hw
when the thread running hidpp_connect_event() from probe() is waiting on
the hw, then a second thread running hidpp_connect_event() will be
started from the workqueue.
This opens the following races (note the below code is simplified):
1. Retrieving + printing the protocol (harmless race):
if (!hidpp->protocol_major) {
hidpp_root_get_protocol_version()
hidpp->protocol_major = response.rap.params[0];
}
We can actually see this race hit in the dmesg in the abrt output
attached to rhbz#2227968:
[ 3064.624215] logitech-hidpp-device 0003:046D:4071.0049: HID++ 4.5 device connected.
[ 3064.658184] logitech-hidpp-device 0003:046D:4071.0049: HID++ 4.5 device connected.
Testing with extra logging added has shown that after this the 2 threads
take turn grabbing the hw access mutex (send_mutex) so they ping-pong
through all the other TOCTOU cases managing to hit all of them:
2. Updating the name to the HIDPP name (harmless race):
if (hidpp->name == hdev->name) {
...
hidpp->name = new_name;
}
3. Initializing the power_supply class for the battery (problematic!):
hidpp_initialize_battery()
{
if (hidpp->battery.ps)
return 0;
probe_battery(); /* Blocks, threads take turns executing this */
hidpp->battery.desc.properties =
devm_kmemdup(dev, hidpp_battery_props, cnt, GFP_KERNEL);
hidpp->battery.ps =
devm_power_supply_register(&hidpp->hid_dev->dev,
&hidpp->battery.desc, cfg);
}
4. Creating delayed input_device (potentially problematic):
if (hidpp->delayed_input)
return;
hidpp->delayed_input = hidpp_allocate_input(hdev);
The really big problem here is 3. Hitting the race leads to the following
sequence:
hidpp->battery.desc.properties =
devm_kmemdup(dev, hidpp_battery_props, cnt, GFP_KERNEL);
hidpp->battery.ps =
devm_power_supply_register(&hidpp->hid_dev->dev,
&hidpp->battery.desc, cfg);
...
hidpp->battery.desc.properties =
devm_kmemdup(dev, hidpp_battery_props, cnt, GFP_KERNEL);
hidpp->battery.ps =
devm_power_supply_register(&hidpp->hid_dev->dev,
&hidpp->battery.desc, cfg);
So now we have registered 2 power supplies for the same battery,
which looks a bit weird from userspace's pov but this is not even
the really big problem.
Notice how:
1. This is all devm-maganaged
2. The hidpp->battery.desc struct is shared between the 2 power supplies
3. hidpp->battery.desc points to the result from the second devm_kmemdup()
This causes a use after free scenario on USB disconnect of the receiver:
1. The last registered power supply class device gets unregistered
2. The memory from the last devm_kmemdup() call gets freed,
hidpp->battery.desc.properties now points to freed memory
3. The first registered power supply class device gets unregistered,
this involves sending a remove uevent to userspace which invokes
power_supply_uevent() to fill the uevent data
4. power_supply_uevent() uses hidpp->battery.desc.properties which
now points to freed memory leading to backtraces like this one:
Sep 22 20:01:35 eric kernel: BUG: unable to handle page fault for address: ffffb2140e017f08
...
Sep 22 20:01:35 eric kernel: Workqueue: usb_hub_wq hub_event
Sep 22 20:01:35 eric kernel: RIP: 0010:power_supply_uevent+0xee/0x1d0
...
Sep 22 20:01:35 eric kernel: ? asm_exc_page_fault+0x26/0x30
Sep 22 20:01:35 eric kernel: ? power_supply_uevent+0xee/0x1d0
Sep 22 20:01:35 eric kernel: ? power_supply_uevent+0x10d/0x1d0
Sep 22 20:01:35 eric kernel: dev_uevent+0x10f/0x2d0
Sep 22 20:01:35 eric kernel: kobject_uevent_env+0x291/0x680
Sep 22 20:01:35 eric kernel: power_supply_unregister+0x8e/0xa0
Sep 22 20:01:35 eric kernel: release_nodes+0x3d/0xb0
Sep 22 20:01:35 eric kernel: devres_release_group+0xfc/0x130
Sep 22 20:01:35 eric kernel: hid_device_remove+0x56/0xa0
Sep 22 20:01:35 eric kernel: device_release_driver_internal+0x19f/0x200
Sep 22 20:01:35 eric kernel: bus_remove_device+0xc6/0x130
Sep 22 20:01:35 eric kernel: device_del+0x15c/0x3f0
Sep 22 20:01:35 eric kernel: ? __queue_work+0x1df/0x440
Sep 22 20:01:35 eric kernel: hid_destroy_device+0x4b/0x60
Sep 22 20:01:35 eric kernel: logi_dj_remove+0x9a/0x100 [hid_logitech_dj 5c91534a0ead2b65e04dd799a0437e3b99b21bc4]
Sep 22 20:01:35 eric kernel: hid_device_remove+0x44/0xa0
Sep 22 20:01:35 eric kernel: device_release_driver_internal+0x19f/0x200
Sep 22 20:01:35 eric kernel: bus_remove_device+0xc6/0x130
Sep 22 20:01:35 eric kernel: device_del+0x15c/0x3f0
Sep 22 20:01:35 eric kernel: ? __queue_work+0x1df/0x440
Sep 22 20:01:35 eric kernel: hid_destroy_device+0x4b/0x60
Sep 22 20:01:35 eric kernel: usbhid_disconnect+0x47/0x60 [usbhid 727dcc1c0b94e6b4418727a468398ac3bca492f3]
Sep 22 20:01:35 eric kernel: usb_unbind_interface+0x90/0x270
Sep 22 20:01:35 eric kernel: device_release_driver_internal+0x19f/0x200
Sep 22 20:01:35 eric kernel: bus_remove_device+0xc6/0x130
Sep 22 20:01:35 eric kernel: device_del+0x15c/0x3f0
Sep 22 20:01:35 eric kernel: ? kobject_put+0xa0/0x1d0
Sep 22 20:01:35 eric kernel: usb_disable_device+0xcd/0x1e0
Sep 22 20:01:35 eric kernel: usb_disconnect+0xde/0x2c0
Sep 22 20:01:35 eric kernel: usb_disconnect+0xc3/0x2c0
Sep 22 20:01:35 eric kernel: hub_event+0xe80/0x1c10
There have been quite a few bug reports (see Link tags) about this crash.
Fix all the TOCTOU issues, including the really bad power-supply related
system crash on USB disconnect, by making probe() use the workqueue for
running hidpp_connect_event() too, so that it can never run more then once.
Link: https://bugzilla.redhat.com/show_bug.cgi?id=2227221
Link: https://bugzilla.redhat.com/show_bug.cgi?id=2227968
Link: https://bugzilla.redhat.com/show_bug.cgi?id=2227968
Link: https://bugzilla.redhat.com/show_bug.cgi?id=2242189
Link: https://bugzilla.kernel.org/show_bug.cgi?id=217412#c58
Cc: stable@vger.kernel.org
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/hid/hid-logitech-hidpp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index ff077df0babf..a209d51bd247 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4515,7 +4515,8 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
goto hid_hw_init_fail;
}
- hidpp_connect_event(hidpp);
+ schedule_work(&hidpp->work);
+ flush_work(&hidpp->work);
if (will_restart) {
/* Reset the HID node state */
--
2.41.0
^ permalink raw reply related
* Re: [PATCH v3 0/3] selftests/hid: assorted fixes
From: Eduard Zingerman @ 2023-10-05 18:14 UTC (permalink / raw)
To: Benjamin Tissoires, Jiri Kosina, Benjamin Tissoires, Shuah Khan,
Justin Stitt, Nick Desaulniers
Cc: linux-input, linux-kselftest, linux-kernel, bpf, Shuah Khan
In-Reply-To: <20230825-wip-selftests-v3-0-639963c54109@kernel.org>
On Thu, 2023-10-05 at 17:55 +0200, Benjamin Tissoires wrote:
> > And this is the last(?) revision of this series which should now compile
> > with or without CONFIG_HID_BPF set.
> >
> > I had to do changes because [1] was failing
> >
> > Nick, I kept your Tested-by, even if I made small changes in 1/3. Feel
> > free to shout if you don't want me to keep it.
> >
> > Eduard, You helped us a lot in the review of v1 but never sent your
> > Reviewed-by or Acked-by. Do you want me to add one?
Hi Benjamin,
I think there is no need, I just took part in the discussion and that's all.
Feel free to ping me if there is anything BPF related that needs clarification.
Thanks,
Eduard
> >
> > Cheers,
> > Benjamin
> >
> > [1] https://gitlab.freedesktop.org/bentiss/hid/-/jobs/49754306
> >
> > For reference, the v2 cover letter:
> >
> > > > Hi, I am sending this series on behalf of myself and Benjamin Tissoires. There
> > > > existed an initial n=3 patch series which was later expanded to n=4 and
> > > > is now back to n=3 with some fixes added in and rebased against
> > > > mainline.
> > > >
> > > > This patch series aims to ensure that the hid/bpf selftests can be built
> > > > without errors.
> > > >
> > > > Here's Benjamin's initial cover letter for context:
> > > > > > These fixes have been triggered by [0]:
> > > > > > basically, if you do not recompile the kernel first, and are
> > > > > > running on an old kernel, vmlinux.h doesn't have the required
> > > > > > symbols and the compilation fails.
> > > > > >
> > > > > > The tests will fail if you run them on that very same machine,
> > > > > > of course, but the binary should compile.
> > > > > >
> > > > > > And while I was sorting out why it was failing, I realized I
> > > > > > could do a couple of improvements on the Makefile.
> > > > > >
> > > > > > [0] https://lore.kernel.org/linux-input/56ba8125-2c6f-a9c9-d498-0ca1c153dcb2@redhat.com/T/#t
> >
> > Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> > ---
> > Changes in v3:
> > - Also overwrite all of the enum symbols in patch 1/3
> > - Link to v2: https://lore.kernel.org/r/20230908-kselftest-09-08-v2-0-0def978a4c1b@google.com
> >
> > Changes in v2:
> > - roll Justin's fix into patch 1/3
> > - add __attribute__((preserve_access_index)) (thanks Eduard)
> > - rebased onto mainline (2dde18cd1d8fac735875f2e4987f11817cc0bc2c)
> > - Link to v1: https://lore.kernel.org/r/20230825-wip-selftests-v1-0-c862769020a8@kernel.org
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1698
> > Link: https://github.com/ClangBuiltLinux/continuous-integration2/issues/61
> >
> > ---
> > Benjamin Tissoires (3):
> > selftests/hid: ensure we can compile the tests on kernels pre-6.3
> > selftests/hid: do not manually call headers_install
> > selftests/hid: force using our compiled libbpf headers
> >
> > tools/testing/selftests/hid/Makefile | 10 ++-
> > tools/testing/selftests/hid/progs/hid.c | 3 -
> > .../testing/selftests/hid/progs/hid_bpf_helpers.h | 77 ++++++++++++++++++++++
> > 3 files changed, 81 insertions(+), 9 deletions(-)
> > ---
> > base-commit: 29aa98d0fe013e2ab62aae4266231b7fb05d47a2
> > change-id: 20230825-wip-selftests-9a7502b56542
> >
> > Best regards,
^ permalink raw reply
* Re: [PATCH] platform/x86: touchscreen_dmi: Add info for the Positivo C4128B
From: Hans de Goede @ 2023-10-05 19:08 UTC (permalink / raw)
To: Renan Guilherme Lebre Ramos, linux-input
In-Reply-To: <20231004235900.426240-1-japareaggae@gmail.com>
Hi,
On 10/5/23 01:59, Renan Guilherme Lebre Ramos wrote:
> Add information for the Positivo C4128B, a notebook/tablet convertible.
>
> Link: https://github.com/onitake/gsl-firmware/pull/217
> Signed-off-by: Renan Guilherme Lebre Ramos <japareaggae@gmail.com>
Thank you for your patch/series, I've applied this patch
(series) to the pdx86 fixes branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=fixes
Note it will show up in the pdx86 fixes branch once I've pushed
my local branch there, which might take a while.
I will include this patch in my next fixes pull-req to Linus
for the current kernel development cycle.
Regards,
Hans
> ---
> drivers/platform/x86/touchscreen_dmi.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/drivers/platform/x86/touchscreen_dmi.c b/drivers/platform/x86/touchscreen_dmi.c
> index f9301a9382e7..0f6b30285381 100644
> --- a/drivers/platform/x86/touchscreen_dmi.c
> +++ b/drivers/platform/x86/touchscreen_dmi.c
> @@ -756,6 +756,21 @@ static const struct ts_dmi_data pipo_w11_data = {
> .properties = pipo_w11_props,
> };
>
> +static const struct property_entry positivo_c4128b_props[] = {
> + PROPERTY_ENTRY_U32("touchscreen-min-x", 4),
> + PROPERTY_ENTRY_U32("touchscreen-min-y", 13),
> + PROPERTY_ENTRY_U32("touchscreen-size-x", 1915),
> + PROPERTY_ENTRY_U32("touchscreen-size-y", 1269),
> + PROPERTY_ENTRY_STRING("firmware-name", "gsl1680-positivo-c4128b.fw"),
> + PROPERTY_ENTRY_U32("silead,max-fingers", 10),
> + { }
> +};
> +
> +static const struct ts_dmi_data positivo_c4128b_data = {
> + .acpi_name = "MSSL1680:00",
> + .properties = positivo_c4128b_props,
> +};
> +
> static const struct property_entry pov_mobii_wintab_p800w_v20_props[] = {
> PROPERTY_ENTRY_U32("touchscreen-min-x", 32),
> PROPERTY_ENTRY_U32("touchscreen-min-y", 16),
> @@ -1480,6 +1495,14 @@ const struct dmi_system_id touchscreen_dmi_table[] = {
> DMI_MATCH(DMI_BIOS_VERSION, "MOMO.G.WI71C.MABMRBA02"),
> },
> },
> + {
> + /* Positivo C4128B */
> + .driver_data = (void *)&positivo_c4128b_data,
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Positivo Tecnologia SA"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "C4128B-1"),
> + },
> + },
> {
> /* Point of View mobii wintab p800w (v2.0) */
> .driver_data = (void *)&pov_mobii_wintab_p800w_v20_data,
^ permalink raw reply
* [PATCH 3/3] Input: atkbd - set skip_commands = ATKBD_SKIP_GETID for HP laptop 15s-fq* laptops
From: Hans de Goede @ 2023-10-05 20:15 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Hans de Goede, linux-input
In-Reply-To: <20231005201544.26983-1-hdegoede@redhat.com>
On "HP Laptop 15s-fq2xxx" and "HP laptop 15s-fq4xxx" laptops
the keyboard gets confused by Linux probing it and won't work
for the first 2 - 5 minutes or so (waiting for EC watchdog reset?).
Testing has shown that Linux sending ATKBD_CMD_GETID causes
the keyboard to not work for the first 2 - 5 minutes.
Add a quirk setting skip_commands = ATKBD_SKIP_GETID to fix this.
Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2086156
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/input/keyboard/atkbd.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 19a01e763871..7ebd686b61b3 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -1937,6 +1937,19 @@ static const struct dmi_system_id atkbd_dmi_quirk_table[] __initconst = {
.callback = atkbd_skip_commands_fixup,
.driver_data = (void *)ATKBD_SKIP_DEACTIVATE,
},
+ {
+ /*
+ * "HP Laptop 15s-fq2xxx" and "HP laptop 15s-fq4xxx" both get
+ * confused by ATKBD_CMD_GETID.
+ */
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "HP"),
+ /* Partial match to match both systems */
+ DMI_MATCH(DMI_PRODUCT_NAME, "HP Laptop 15s-fq"),
+ },
+ .callback = atkbd_skip_commands_fixup,
+ .driver_data = (void *)ATKBD_SKIP_GETID,
+ },
{ }
};
--
2.41.0
^ permalink raw reply related
* [PATCH 2/3] Input: atkbd - drop atkbd_skip_deactivate flag
From: Hans de Goede @ 2023-10-05 20:15 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Hans de Goede, linux-input
In-Reply-To: <20231005201544.26983-1-hdegoede@redhat.com>
Drop the atkbd_skip_deactivate flag and make the DMI quirk for LG laptops
set atkbd_skip_commands = ATKBD_SKIP_DEACTIVATE instead.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/input/keyboard/atkbd.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 7c16f9cc1e40..19a01e763871 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -257,12 +257,6 @@ static void (*atkbd_platform_fixup)(struct atkbd *, const void *data);
static void *atkbd_platform_fixup_data;
static unsigned int (*atkbd_platform_scancode_fixup)(struct atkbd *, unsigned int);
-/*
- * Certain keyboards to not like ATKBD_CMD_RESET_DIS and stop responding
- * to many commands until full reset (ATKBD_CMD_RESET_BAT) is performed.
- */
-static bool atkbd_skip_deactivate;
-
static ssize_t atkbd_attr_show_helper(struct device *dev, char *buf,
ssize_t (*handler)(struct atkbd *, char *));
static ssize_t atkbd_attr_set_helper(struct device *dev, const char *buf, size_t count,
@@ -855,8 +849,7 @@ static int atkbd_probe(struct atkbd *atkbd)
* Make sure nothing is coming from the keyboard and disturbs our
* internal state.
*/
- if (!atkbd_skip_deactivate)
- atkbd_deactivate(atkbd);
+ atkbd_deactivate(atkbd);
return 0;
}
@@ -1781,9 +1774,12 @@ static int __init atkbd_setup_scancode_fixup(const struct dmi_system_id *id)
return 1;
}
-static int __init atkbd_deactivate_fixup(const struct dmi_system_id *id)
+static int __init atkbd_skip_commands_fixup(const struct dmi_system_id *id)
{
- atkbd_skip_deactivate = true;
+ /* Module parameter overrules quirks */
+ if (!atkbd_skip_commands)
+ atkbd_skip_commands = (long)id->driver_data;
+
return 1;
}
@@ -1931,10 +1927,15 @@ static const struct dmi_system_id atkbd_dmi_quirk_table[] __initconst = {
.driver_data = atkbd_oqo_01plus_scancode_fixup,
},
{
+ /*
+ * LG keyboards seem to not like ATKBD_CMD_RESET_DIS and stop responding
+ * to many commands until full reset (ATKBD_CMD_RESET_BAT) is performed.
+ */
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "LG Electronics"),
},
- .callback = atkbd_deactivate_fixup,
+ .callback = atkbd_skip_commands_fixup,
+ .driver_data = (void *)ATKBD_SKIP_DEACTIVATE,
},
{ }
};
--
2.41.0
^ permalink raw reply related
* [PATCH 0/3] Input: atkbd - add skip_commands module parameter
From: Hans de Goede @ 2023-10-05 20:15 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Hans de Goede, linux-input
Hi all,
While debugging a keyboard issue on some HP laptops adding i8042.dumbkbd
helped to avoid the issue. So one of the commands send by atkbd.c seemed
to be the culprit.
This series a skip_commands option to help debug cases like this by adding
a bit-field which allows disabling a subset of the ps2_command()
calls the atkbd driver makes.
It also replaces the existing atkbd_skip_deactivate flag
with the new parameter and adds a DMI quirk for the HP laptops
to avoid the keyboard issue there.
Regards,
Hans
Hans de Goede (3):
Input: atkbd - add skip_commands module parameter
Input: atkbd - drop atkbd_skip_deactivate flag
Input: atkbd - set skip_commands = ATKBD_SKIP_GETID for HP laptop
15s-fq* laptops
drivers/input/keyboard/atkbd.c | 88 ++++++++++++++++++++++++++--------
1 file changed, 69 insertions(+), 19 deletions(-)
--
2.41.0
^ permalink raw reply
* [PATCH 1/3] Input: atkbd - add skip_commands module parameter
From: Hans de Goede @ 2023-10-05 20:15 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Hans de Goede, linux-input
In-Reply-To: <20231005201544.26983-1-hdegoede@redhat.com>
While debugging a keyboard issue on some HP laptops (see link)
adding i8042.dumbkbd helped to avoid the issue. So one of the commands
send by atkbd.c seemed to be the culprit.
Add a skip_commands option to help debug cases like this by adding
a bit-field which allows disabling a subset of the ps2_command()
calls the atkbd driver makes.
This also avoids the need to add special flags for each command
which atkbd needs to skip on certain models, like e.g. the existing
atkbd_skip_deactivate flag.
Link: https://bugzilla.redhat.com/show_bug.cgi?id=2086156
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
The next patch in this series replaces the atkbd_skip_deactivate flag
with using atbkd_skip_commands to avoid having 2 deactivate skip checks.
---
drivers/input/keyboard/atkbd.c | 52 ++++++++++++++++++++++++++++------
1 file changed, 44 insertions(+), 8 deletions(-)
diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index c92e544c792d..7c16f9cc1e40 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -65,6 +65,10 @@ static bool atkbd_terminal;
module_param_named(terminal, atkbd_terminal, bool, 0);
MODULE_PARM_DESC(terminal, "Enable break codes on an IBM Terminal keyboard connected via AT/PS2");
+static int atkbd_skip_commands;
+module_param_named(skip_commands, atkbd_skip_commands, int, 0444);
+MODULE_PARM_DESC(skip_commands, "Bitfield where each bits skips a specific keyboard cmd (0 - 0x3f)");
+
#define SCANCODE(keymap) ((keymap >> 16) & 0xFFFF)
#define KEYCODE(keymap) (keymap & 0xFFFF)
@@ -182,6 +186,13 @@ static const unsigned short atkbd_unxlate_table[128] = {
#define ATKBD_XL_HANGEUL 0x10
#define ATKBD_XL_HANJA 0x20
+#define ATKBD_SKIP_GETID 0x01l
+#define ATKBD_SKIP_ACTIVATE 0x02l
+#define ATKBD_SKIP_DEACTIVATE 0x04l
+#define ATKBD_SKIP_SETREP 0x08l
+#define ATKBD_SKIP_SETLEDS 0x10l
+#define ATKBD_SKIP_RESET_DEF 0x20l
+
static const struct {
unsigned short keycode;
unsigned char set2;
@@ -592,6 +603,9 @@ static int atkbd_set_repeat_rate(struct atkbd *atkbd)
unsigned char param;
int i = 0, j = 0;
+ if (atkbd_skip_commands & ATKBD_SKIP_SETREP)
+ return 0;
+
while (i < ARRAY_SIZE(period) - 1 && period[i] < dev->rep[REP_PERIOD])
i++;
dev->rep[REP_PERIOD] = period[i];
@@ -609,6 +623,9 @@ static int atkbd_set_leds(struct atkbd *atkbd)
struct input_dev *dev = atkbd->dev;
unsigned char param[2];
+ if (atkbd_skip_commands & ATKBD_SKIP_SETLEDS)
+ return 0;
+
param[0] = (test_bit(LED_SCROLLL, dev->led) ? 1 : 0)
| (test_bit(LED_NUML, dev->led) ? 2 : 0)
| (test_bit(LED_CAPSL, dev->led) ? 4 : 0);
@@ -736,6 +753,9 @@ static int atkbd_activate(struct atkbd *atkbd)
{
struct ps2dev *ps2dev = &atkbd->ps2dev;
+ if (atkbd_skip_commands & ATKBD_SKIP_ACTIVATE)
+ return 0;
+
/*
* Enable the keyboard to receive keystrokes.
*/
@@ -759,6 +779,9 @@ static void atkbd_deactivate(struct atkbd *atkbd)
{
struct ps2dev *ps2dev = &atkbd->ps2dev;
+ if (atkbd_skip_commands & ATKBD_SKIP_DEACTIVATE)
+ return;
+
if (ps2_command(ps2dev, NULL, ATKBD_CMD_RESET_DIS))
dev_err(&ps2dev->serio->dev,
"Failed to deactivate keyboard on %s\n",
@@ -786,6 +809,13 @@ static int atkbd_probe(struct atkbd *atkbd)
"keyboard reset failed on %s\n",
ps2dev->serio->phys);
+/* Only skip probe for translated keyboards to avoid mis-identifying mice */
+ if (atkbd->translated && (atkbd_skip_commands & ATKBD_SKIP_GETID)) {
+ atkbd->id = 0xab00;
+ atkbd_deactivate(atkbd);
+ return 0;
+ }
+
/*
* Then we check the keyboard ID. We should get 0xab83 under normal conditions.
* Some keyboards report different values, but the first byte is always 0xab or
@@ -802,7 +832,8 @@ static int atkbd_probe(struct atkbd *atkbd)
* the LEDs off, which we want anyway.
*/
param[0] = 0;
- if (ps2_command(ps2dev, param, ATKBD_CMD_SETLEDS))
+ if (!(atkbd_skip_commands & ATKBD_SKIP_SETLEDS) &&
+ ps2_command(ps2dev, param, ATKBD_CMD_SETLEDS))
return -1;
atkbd->id = 0xabba;
return 0;
@@ -906,17 +937,21 @@ static int atkbd_reset_state(struct atkbd *atkbd)
* Set the LEDs to a predefined state (all off).
*/
- param[0] = 0;
- if (ps2_command(ps2dev, param, ATKBD_CMD_SETLEDS))
- return -1;
+ if (!(atkbd_skip_commands & ATKBD_SKIP_SETLEDS)) {
+ param[0] = 0;
+ if (ps2_command(ps2dev, param, ATKBD_CMD_SETLEDS))
+ return -1;
+ }
/*
* Set autorepeat to fastest possible.
*/
- param[0] = 0;
- if (ps2_command(ps2dev, param, ATKBD_CMD_SETREP))
- return -1;
+ if (!(atkbd_skip_commands & ATKBD_SKIP_SETREP)) {
+ param[0] = 0;
+ if (ps2_command(ps2dev, param, ATKBD_CMD_SETREP))
+ return -1;
+ }
return 0;
}
@@ -931,7 +966,8 @@ static void atkbd_cleanup(struct serio *serio)
struct atkbd *atkbd = atkbd_from_serio(serio);
atkbd_disable(atkbd);
- ps2_command(&atkbd->ps2dev, NULL, ATKBD_CMD_RESET_DEF);
+ if (!(atkbd_skip_commands & ATKBD_SKIP_RESET_DEF))
+ ps2_command(&atkbd->ps2dev, NULL, ATKBD_CMD_RESET_DEF);
}
--
2.41.0
^ permalink raw reply related
* Re: [PATCH v2] HID: i2c-hid: fix handling of unpopulated devices
From: Dennis Gilmore @ 2023-10-05 22:06 UTC (permalink / raw)
To: johan+linaro, Benjamin Tissoires
Cc: dianders, jikos, linux-input, linux-kernel, mripard,
Dennis Gilmore
In-Reply-To: <20231002155857.24584-1-johan+linaro@kernel.org>
> A recent commit reordered probe so that the interrupt line is now
> requested before making sure that the device exists.
>
> This breaks machines like the Lenovo ThinkPad X13s which rely on the
> HID driver to probe second-source devices and only register the variant
> that is actually populated. Specifically, the interrupt line may now
> already be (temporarily) claimed when doing asynchronous probing of the
> touchpad:
>
> genirq: Flags mismatch irq 191. 00082008 (hid-over-i2c) vs. 00082008 (hid-over-i2c)
> i2c_hid_of 21-0015: Could not register for hid-over-i2c interrupt, irq = 191, ret = -16
> i2c_hid_of: probe of 21-0015 failed with error -16
>
> Fix this by restoring the old behaviour of first making sure the device
> exists before requesting the interrupt line.
>
> Note that something like this should probably be implemented also for
> "panel followers", whose actual probe is currently effectively deferred
> until the DRM panel is probed (e.g. by powering down the device after
> making sure it exists and only then register it as a follower).
>
> Fixes: 675cd877c952 ("HID: i2c-hid: Rearrange probe() to power things up later")
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Maxime Ripard <mripard@kernel.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
Tested-by: Dennis Gilmore <dgilmore@redhat.com>
> ---
>
> Changes in v2
> - initialise ihid->is_panel_follower sooner to avoid repeated property
> lookups and so that it can be used consistently throughout the driver
> for code that differs for "panel followers"
>
> Link to v1: https://lore.kernel.org/lkml/20230918125851.310-1-johan+linaro@kernel.org/
>
>
> drivers/hid/i2c-hid/i2c-hid-core.c | 144 ++++++++++++++++-------------
> 1 file changed, 81 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
> index 9601c0605fd9..2735cd585af0 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -998,45 +998,29 @@ static int i2c_hid_core_resume(struct i2c_hid *ihid)
> return hid_driver_reset_resume(hid);
> }
>
> -/**
> - * __do_i2c_hid_core_initial_power_up() - First time power up of the i2c-hid device.
> - * @ihid: The ihid object created during probe.
> - *
> - * This function is called at probe time.
> - *
> - * The initial power on is where we do some basic validation that the device
> - * exists, where we fetch the HID descriptor, and where we create the actual
> - * HID devices.
> - *
> - * Return: 0 or error code.
> +/*
> + * Check that the device exists and parse the HID descriptor.
> */
> -static int __do_i2c_hid_core_initial_power_up(struct i2c_hid *ihid)
> +static int __i2c_hid_core_probe(struct i2c_hid *ihid)
> {
> struct i2c_client *client = ihid->client;
> struct hid_device *hid = ihid->hid;
> int ret;
>
> - ret = i2c_hid_core_power_up(ihid);
> - if (ret)
> - return ret;
> -
> /* Make sure there is something at this address */
> ret = i2c_smbus_read_byte(client);
> if (ret < 0) {
> i2c_hid_dbg(ihid, "nothing at this address: %d\n", ret);
> - ret = -ENXIO;
> - goto err;
> + return -ENXIO;
> }
>
> ret = i2c_hid_fetch_hid_descriptor(ihid);
> if (ret < 0) {
> dev_err(&client->dev,
> "Failed to fetch the HID Descriptor\n");
> - goto err;
> + return ret;
> }
>
> - enable_irq(client->irq);
> -
> hid->version = le16_to_cpu(ihid->hdesc.bcdVersion);
> hid->vendor = le16_to_cpu(ihid->hdesc.wVendorID);
> hid->product = le16_to_cpu(ihid->hdesc.wProductID);
> @@ -1050,17 +1034,49 @@ static int __do_i2c_hid_core_initial_power_up(struct i2c_hid *ihid)
>
> ihid->quirks = i2c_hid_lookup_quirk(hid->vendor, hid->product);
>
> + return 0;
> +}
> +
> +static int i2c_hid_core_register_hid(struct i2c_hid *ihid)
> +{
> + struct i2c_client *client = ihid->client;
> + struct hid_device *hid = ihid->hid;
> + int ret;
> +
> + enable_irq(client->irq);
> +
> ret = hid_add_device(hid);
> if (ret) {
> if (ret != -ENODEV)
> hid_err(client, "can't add hid device: %d\n", ret);
> - goto err;
> + disable_irq(client->irq);
> + return ret;
> }
>
> return 0;
> +}
> +
> +static int i2c_hid_core_probe_panel_follower(struct i2c_hid *ihid)
> +{
> + int ret;
> +
> + ret = i2c_hid_core_power_up(ihid);
> + if (ret)
> + return ret;
>
> -err:
> + ret = __i2c_hid_core_probe(ihid);
> + if (ret)
> + goto err_power_down;
> +
> + ret = i2c_hid_core_register_hid(ihid);
> + if (ret)
> + goto err_power_down;
> +
> + return 0;
> +
> +err_power_down:
> i2c_hid_core_power_down(ihid);
> +
> return ret;
> }
>
> @@ -1077,7 +1093,7 @@ static void ihid_core_panel_prepare_work(struct work_struct *work)
> * steps.
> */
> if (!hid->version)
> - ret = __do_i2c_hid_core_initial_power_up(ihid);
> + ret = i2c_hid_core_probe_panel_follower(ihid);
> else
> ret = i2c_hid_core_resume(ihid);
>
> @@ -1136,7 +1152,6 @@ static int i2c_hid_core_register_panel_follower(struct i2c_hid *ihid)
> struct device *dev = &ihid->client->dev;
> int ret;
>
> - ihid->is_panel_follower = true;
> ihid->panel_follower.funcs = &i2c_hid_core_panel_follower_funcs;
>
> /*
> @@ -1156,30 +1171,6 @@ static int i2c_hid_core_register_panel_follower(struct i2c_hid *ihid)
> return 0;
> }
>
> -static int i2c_hid_core_initial_power_up(struct i2c_hid *ihid)
> -{
> - /*
> - * If we're a panel follower, we'll register and do our initial power
> - * up when the panel turns on; otherwise we do it right away.
> - */
> - if (drm_is_panel_follower(&ihid->client->dev))
> - return i2c_hid_core_register_panel_follower(ihid);
> - else
> - return __do_i2c_hid_core_initial_power_up(ihid);
> -}
> -
> -static void i2c_hid_core_final_power_down(struct i2c_hid *ihid)
> -{
> - /*
> - * If we're a follower, the act of unfollowing will cause us to be
> - * powered down. Otherwise we need to manually do it.
> - */
> - if (ihid->is_panel_follower)
> - drm_panel_remove_follower(&ihid->panel_follower);
> - else
> - i2c_hid_core_suspend(ihid, true);
> -}
> -
> int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
> u16 hid_descriptor_address, u32 quirks)
> {
> @@ -1211,6 +1202,7 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
> ihid->ops = ops;
> ihid->client = client;
> ihid->wHIDDescRegister = cpu_to_le16(hid_descriptor_address);
> + ihid->is_panel_follower = drm_is_panel_follower(&client->dev);
>
> init_waitqueue_head(&ihid->wait);
> mutex_init(&ihid->reset_lock);
> @@ -1224,14 +1216,10 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
> return ret;
> device_enable_async_suspend(&client->dev);
>
> - ret = i2c_hid_init_irq(client);
> - if (ret < 0)
> - goto err_buffers_allocated;
> -
> hid = hid_allocate_device();
> if (IS_ERR(hid)) {
> ret = PTR_ERR(hid);
> - goto err_irq;
> + goto err_free_buffers;
> }
>
> ihid->hid = hid;
> @@ -1242,19 +1230,42 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
> hid->bus = BUS_I2C;
> hid->initial_quirks = quirks;
>
> - ret = i2c_hid_core_initial_power_up(ihid);
> + /* Power on and probe unless device is a panel follower. */
> + if (!ihid->is_panel_follower) {
> + ret = i2c_hid_core_power_up(ihid);
> + if (ret < 0)
> + goto err_destroy_device;
> +
> + ret = __i2c_hid_core_probe(ihid);
> + if (ret < 0)
> + goto err_power_down;
> + }
> +
> + ret = i2c_hid_init_irq(client);
> + if (ret < 0)
> + goto err_power_down;
> +
> + /*
> + * If we're a panel follower, we'll register when the panel turns on;
> + * otherwise we do it right away.
> + */
> + if (ihid->is_panel_follower)
> + ret = i2c_hid_core_register_panel_follower(ihid);
> + else
> + ret = i2c_hid_core_register_hid(ihid);
> if (ret)
> - goto err_mem_free;
> + goto err_free_irq;
>
> return 0;
>
> -err_mem_free:
> - hid_destroy_device(hid);
> -
> -err_irq:
> +err_free_irq:
> free_irq(client->irq, ihid);
> -
> -err_buffers_allocated:
> +err_power_down:
> + if (!ihid->is_panel_follower)
> + i2c_hid_core_power_down(ihid);
> +err_destroy_device:
> + hid_destroy_device(hid);
> +err_free_buffers:
> i2c_hid_free_buffers(ihid);
>
> return ret;
> @@ -1266,7 +1277,14 @@ void i2c_hid_core_remove(struct i2c_client *client)
> struct i2c_hid *ihid = i2c_get_clientdata(client);
> struct hid_device *hid;
>
> - i2c_hid_core_final_power_down(ihid);
> + /*
> + * If we're a follower, the act of unfollowing will cause us to be
> + * powered down. Otherwise we need to manually do it.
> + */
> + if (ihid->is_panel_follower)
> + drm_panel_remove_follower(&ihid->panel_follower);
> + else
> + i2c_hid_core_suspend(ihid, true);
>
> hid = ihid->hid;
> hid_destroy_device(hid);
> --
> 2.41.0
>
>
>
^ permalink raw reply
* Re: [PATCH] HID: nvidia-shield: Select POWER_SUPPLY Kconfig option
From: Rahul Rameshbabu @ 2023-10-05 23:54 UTC (permalink / raw)
To: Jiri Kosina; +Cc: Benjamin Tissoires, linux-input
In-Reply-To: <nycvar.YFH.7.76.2310042048280.3534@cbobk.fhfr.pm>
On Wed, 04 Oct, 2023 20:49:07 +0200 Jiri Kosina <jikos@kernel.org> wrote:
> On Mon, 18 Sep 2023, Rahul Rameshbabu wrote:
>
>> >> Battery information reported by the driver depends on the power supply
>> >> subsystem. Select the required subsystem when the HID_NVIDIA_SHIELD
>> >> Kconfig option is enabled.
>> >>
>> >> Fixes: 3ab196f88237 ("HID: nvidia-shield: Add battery support for Thunderstrike")
>> >> Signed-off-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
>> >> ---
>> >> drivers/hid/Kconfig | 1 +
>> >> 1 file changed, 1 insertion(+)
>> >>
>> >> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
>> >> index e11c1c803676..dc227f477601 100644
>> >> --- a/drivers/hid/Kconfig
>> >> +++ b/drivers/hid/Kconfig
>> >> @@ -792,6 +792,7 @@ config HID_NVIDIA_SHIELD
>> >> tristate "NVIDIA SHIELD devices"
>> >> depends on USB_HID
>> >> depends on BT_HIDP
>> >> + select POWER_SUPPLY
>> >
>> > Is there a reason not to do it the standard way using 'depends on', and
>> > not vice versa?
>>
>> I originally used 'depends on' for POWER_SUPPLY. I took a look at
>> drivers/hid/Kconfig and saw that all modules that depended on
>> POWER_SUPPLY in the hid subsystem used 'select' instead. I figured I
>> should follow the trend.
>
> You are right.
>
> I still don't like the fact that we are forcefully selecting POWER_SUPPLY
> like this, but that's a 6.7 material.
I agree. This bothered me as well, so I am glad you called it out. There
were some items I wanted to look at for 6.7. Do you want me to add this
to some other patches I am hoping to send out or did you want to take
care of this?
--
Thanks,
Rahul Rameshbabu
^ permalink raw reply
* Re: [PATCH RFC v5 1/6] ARM: pxa: Convert Spitz OHCI to GPIO descriptors
From: Bartosz Golaszewski @ 2023-10-06 6:48 UTC (permalink / raw)
To: Duje Mihanović
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij, Andy Shevchenko,
Dmitry Torokhov, Mark Brown, linux-arm-kernel, linux-kernel,
linux-usb, linux-gpio, linux-input, linux-spi
In-Reply-To: <20231004-pxa-gpio-v5-1-d99ae6fceea8@skole.hr>
On Wed, Oct 4, 2023 at 4:56 PM Duje Mihanović <duje.mihanovic@skole.hr> wrote:
>
> Sharp's Spitz board still uses the legacy GPIO interface for controlling
> a GPIO pin related to the USB host controller.
>
> Convert this function to use the new GPIO descriptor interface.
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
> ---
> arch/arm/mach-pxa/spitz.c | 13 ++++++-------
> drivers/usb/host/ohci-pxa27x.c | 7 +++++++
> 2 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
> index cc691b199429..535e2b2e997b 100644
> --- a/arch/arm/mach-pxa/spitz.c
> +++ b/arch/arm/mach-pxa/spitz.c
> @@ -649,23 +649,22 @@ static inline void spitz_mmc_init(void) {}
> * USB Host
> ******************************************************************************/
> #if defined(CONFIG_USB_OHCI_HCD) || defined(CONFIG_USB_OHCI_HCD_MODULE)
> +GPIO_LOOKUP_SINGLE(spitz_usb_host_gpio_table, "pxa27x-ohci", "gpio-pxa",
> + SPITZ_GPIO_USB_HOST, "usb-host", GPIO_ACTIVE_LOW);
> +
> static int spitz_ohci_init(struct device *dev)
> {
> - int err;
> -
> - err = gpio_request(SPITZ_GPIO_USB_HOST, "USB_HOST");
> - if (err)
> - return err;
> + gpiod_add_lookup_table(&spitz_usb_host_gpio_table);
>
> /* Only Port 2 is connected, setup USB Port 2 Output Control Register */
> UP2OCR = UP2OCR_HXS | UP2OCR_HXOE | UP2OCR_DPPDE | UP2OCR_DMPDE;
>
> - return gpio_direction_output(SPITZ_GPIO_USB_HOST, 1);
> + return 0;
> }
>
> static void spitz_ohci_exit(struct device *dev)
> {
> - gpio_free(SPITZ_GPIO_USB_HOST);
> + gpiod_remove_lookup_table(&spitz_usb_host_gpio_table);
> }
>
> static struct pxaohci_platform_data spitz_ohci_platform_data = {
> diff --git a/drivers/usb/host/ohci-pxa27x.c b/drivers/usb/host/ohci-pxa27x.c
> index 357d9aee38a3..876842b940c0 100644
> --- a/drivers/usb/host/ohci-pxa27x.c
> +++ b/drivers/usb/host/ohci-pxa27x.c
> @@ -121,6 +121,7 @@ struct pxa27x_ohci {
> void __iomem *mmio_base;
> struct regulator *vbus[3];
> bool vbus_enabled[3];
> + struct gpio_desc *usb_host;
> };
>
> #define to_pxa27x_ohci(hcd) (struct pxa27x_ohci *)(hcd_to_ohci(hcd)->priv)
> @@ -447,6 +448,10 @@ static int ohci_hcd_pxa27x_probe(struct platform_device *pdev)
> pxa_ohci = to_pxa27x_ohci(hcd);
> pxa_ohci->clk = usb_clk;
> pxa_ohci->mmio_base = (void __iomem *)hcd->regs;
> + pxa_ohci->usb_host = gpiod_get_optional(&pdev->dev, "usb-host", GPIOD_OUT_LOW);
Any reason not to use devm_gpiod_get_optional()?
Bart
> + if (IS_ERR(pxa_ohci->usb_host))
> + return dev_err_probe(&pdev->dev, PTR_ERR(pxa_ohci->usb_host),
> + "failed to get USB host GPIO\n");
>
> for (i = 0; i < 3; ++i) {
> char name[6];
> @@ -512,6 +517,8 @@ static void ohci_hcd_pxa27x_remove(struct platform_device *pdev)
> for (i = 0; i < 3; ++i)
> pxa27x_ohci_set_vbus_power(pxa_ohci, i, false);
>
> + gpiod_put(pxa_ohci->usb_host);
> +
> usb_put_hcd(hcd);
> }
>
>
> --
> 2.42.0
>
>
^ permalink raw reply
* Re: [PATCH RFC v5 2/6] ARM: pxa: Convert Spitz LEDs to GPIO descriptors
From: Bartosz Golaszewski @ 2023-10-06 6:50 UTC (permalink / raw)
To: Duje Mihanović
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij, Andy Shevchenko,
Dmitry Torokhov, Mark Brown, linux-arm-kernel, linux-kernel,
linux-usb, linux-gpio, linux-input, linux-spi
In-Reply-To: <20231004-pxa-gpio-v5-2-d99ae6fceea8@skole.hr>
On Wed, Oct 4, 2023 at 4:56 PM Duje Mihanović <duje.mihanovic@skole.hr> wrote:
>
> Sharp's Spitz board still uses the legacy GPIO interface for configuring
> its two onboard LEDs.
>
> Convert them to use the GPIO descriptor interface.
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
> ---
> arch/arm/mach-pxa/spitz.c | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
> index 535e2b2e997b..b6a4085e9fb0 100644
> --- a/arch/arm/mach-pxa/spitz.c
> +++ b/arch/arm/mach-pxa/spitz.c
> @@ -452,16 +452,25 @@ static inline void spitz_keys_init(void) {}
> * LEDs
> ******************************************************************************/
> #if defined(CONFIG_LEDS_GPIO) || defined(CONFIG_LEDS_GPIO_MODULE)
> +static struct gpiod_lookup_table spitz_led_gpio_table = {
> + .dev_id = "leds-gpio",
> + .table = {
> + GPIO_LOOKUP_IDX("pxa-gpio", SPITZ_GPIO_LED_ORANGE, NULL, 0,
> + GPIO_ACTIVE_HIGH),
> + GPIO_LOOKUP_IDX("pxa-gpio", SPITZ_GPIO_LED_GREEN, NULL, 1,
> + GPIO_ACTIVE_HIGH),
> + { }
> + }
> +};
> +
> static struct gpio_led spitz_gpio_leds[] = {
> {
> .name = "spitz:amber:charge",
> .default_trigger = "sharpsl-charge",
> - .gpio = SPITZ_GPIO_LED_ORANGE,
> },
> {
> .name = "spitz:green:hddactivity",
> .default_trigger = "disk-activity",
> - .gpio = SPITZ_GPIO_LED_GREEN,
> },
> };
>
> @@ -480,7 +489,12 @@ static struct platform_device spitz_led_device = {
>
> static void __init spitz_leds_init(void)
> {
> + gpiod_add_lookup_table(&spitz_led_gpio_table);
> platform_device_register(&spitz_led_device);
> + spitz_gpio_leds[0].gpiod = gpiod_get_index(&spitz_led_device.dev,
> + NULL, 0, GPIOD_ASIS);
> + spitz_gpio_leds[1].gpiod = gpiod_get_index(&spitz_led_device.dev,
> + NULL, 1, GPIOD_ASIS);
You're not using the con_id you specified in the lookup table. How
about using gpiod_get_array()?
Bart
> }
> #else
> static inline void spitz_leds_init(void) {}
>
> --
> 2.42.0
>
>
^ permalink raw reply
* Re: [PATCH RFC v5 3/6] ARM: pxa: Convert Spitz CF power control to GPIO descriptors
From: Bartosz Golaszewski @ 2023-10-06 6:59 UTC (permalink / raw)
To: Duje Mihanović
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij, Andy Shevchenko,
Dmitry Torokhov, Mark Brown, linux-arm-kernel, linux-kernel,
linux-usb, linux-gpio, linux-input, linux-spi
In-Reply-To: <20231004-pxa-gpio-v5-3-d99ae6fceea8@skole.hr>
On Wed, Oct 4, 2023 at 4:56 PM Duje Mihanović <duje.mihanovic@skole.hr> wrote:
>
> Sharp's Spitz board still uses the legacy GPIO interface for controlling
> the power supply to its CF and SD card slots.
>
> Convert it to use the GPIO descriptor interface.
>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
> ---
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
^ permalink raw reply
* Re: [PATCH RFC v5 4/6] ARM: pxa: Convert reset driver to GPIO descriptors
From: Bartosz Golaszewski @ 2023-10-06 7:07 UTC (permalink / raw)
To: Duje Mihanović
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij, Andy Shevchenko,
Dmitry Torokhov, Mark Brown, linux-arm-kernel, linux-kernel,
linux-usb, linux-gpio, linux-input, linux-spi
In-Reply-To: <20231004-pxa-gpio-v5-4-d99ae6fceea8@skole.hr>
On Wed, Oct 4, 2023 at 4:56 PM Duje Mihanović <duje.mihanovic@skole.hr> wrote:
>
> The PXA reset driver still uses the legacy GPIO interface for
> configuring and asserting the reset pin.
>
> Convert it to use the GPIO descriptor interface.
>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
> ---
> arch/arm/mach-pxa/reset.c | 39 +++++++++++++--------------------------
> arch/arm/mach-pxa/reset.h | 3 +--
> arch/arm/mach-pxa/spitz.c | 6 +++++-
> 3 files changed, 19 insertions(+), 29 deletions(-)
>
> diff --git a/arch/arm/mach-pxa/reset.c b/arch/arm/mach-pxa/reset.c
> index 27293549f8ad..08bc104b9411 100644
> --- a/arch/arm/mach-pxa/reset.c
> +++ b/arch/arm/mach-pxa/reset.c
> @@ -2,7 +2,7 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/delay.h>
> -#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/io.h>
> #include <asm/proc-fns.h>
> #include <asm/system_misc.h>
> @@ -14,33 +14,20 @@
>
> static void do_hw_reset(void);
>
> -static int reset_gpio = -1;
> +static struct gpio_desc *reset_gpio;
>
> -int init_gpio_reset(int gpio, int output, int level)
> +int init_gpio_reset(int output, int level)
> {
> - int rc;
> -
> - rc = gpio_request(gpio, "reset generator");
> - if (rc) {
> - printk(KERN_ERR "Can't request reset_gpio\n");
> - goto out;
> + reset_gpio = gpiod_get(NULL, "reset", GPIOD_ASIS);
> + if (IS_ERR(reset_gpio)) {
> + pr_err("Can't request reset_gpio: %pe\n", reset_gpio);
> + return PTR_ERR(reset_gpio);
> }
>
> if (output)
> - rc = gpio_direction_output(gpio, level);
> + return gpiod_direction_output(reset_gpio, level);
> else
> - rc = gpio_direction_input(gpio);
> - if (rc) {
> - printk(KERN_ERR "Can't configure reset_gpio\n");
> - gpio_free(gpio);
> - goto out;
> - }
> -
> -out:
> - if (!rc)
> - reset_gpio = gpio;
> -
> - return rc;
> + return gpiod_direction_input(reset_gpio);
> }
>
> /*
> @@ -50,16 +37,16 @@ int init_gpio_reset(int gpio, int output, int level)
> */
> static void do_gpio_reset(void)
> {
> - BUG_ON(reset_gpio == -1);
> + BUG_ON(IS_ERR(reset_gpio));
Crashing the kernel on a GPIO error? I guess it just keeps the old
behavior but still...
>
> /* drive it low */
> - gpio_direction_output(reset_gpio, 0);
> + gpiod_direction_output(reset_gpio, 0);
> mdelay(2);
> /* rising edge or drive high */
> - gpio_set_value(reset_gpio, 1);
> + gpiod_set_value(reset_gpio, 1);
> mdelay(2);
> /* falling edge */
> - gpio_set_value(reset_gpio, 0);
> + gpiod_set_value(reset_gpio, 0);
>
> /* give it some time */
> mdelay(10);
> diff --git a/arch/arm/mach-pxa/reset.h b/arch/arm/mach-pxa/reset.h
> index 963dd190bc13..5864f61a0e94 100644
> --- a/arch/arm/mach-pxa/reset.h
> +++ b/arch/arm/mach-pxa/reset.h
> @@ -13,10 +13,9 @@ extern void pxa_register_wdt(unsigned int reset_status);
>
> /**
> * init_gpio_reset() - register GPIO as reset generator
> - * @gpio: gpio nr
> * @output: set gpio as output instead of input during normal work
> * @level: output level
> */
> -extern int init_gpio_reset(int gpio, int output, int level);
> +extern int init_gpio_reset(int output, int level);
>
> #endif /* __ASM_ARCH_RESET_H */
> diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
> index 965354e64c68..26ec29c9cd1b 100644
> --- a/arch/arm/mach-pxa/spitz.c
> +++ b/arch/arm/mach-pxa/spitz.c
> @@ -1024,9 +1024,13 @@ static void spitz_restart(enum reboot_mode mode, const char *cmd)
> spitz_poweroff();
> }
>
> +GPIO_LOOKUP_SINGLE(spitz_reset_gpio_table, NULL, "pxa-gpio",
> + SPITZ_GPIO_ON_RESET, "reset", GPIO_ACTIVE_HIGH);
> +
> static void __init spitz_init(void)
> {
> - init_gpio_reset(SPITZ_GPIO_ON_RESET, 1, 0);
> + gpiod_add_lookup_table(&spitz_reset_gpio_table);
> + init_gpio_reset(1, 0);
> pm_power_off = spitz_poweroff;
>
> PMCR = 0x00;
>
> --
> 2.42.0
>
>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
^ 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