Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH] Input: elantech - mark expected switch fall-through
From: Gustavo A. R. Silva @ 2019-07-24 17:52 UTC (permalink / raw)
  To: Dmitry Torokhov, Kai-Heng Feng
  Cc: linux-input, linux-kernel, Gustavo A. R. Silva

In preparation to enabling -Wimplicit-fallthrough, mark switch
cases where we are expecting to fall through.

This patch fixes the following warning:

drivers/input/mouse/elantech.c: In function 'elantech_use_host_notify':
drivers/input/mouse/elantech.c:1843:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
   if (dmi_get_bios_year() >= 2018)
      ^
drivers/input/mouse/elantech.c:1845:2: note: here
  default:
  ^~~~~~~

Warning level 3 was used: -Wimplicit-fallthrough=3

This patch is part of the ongoing efforts to enable
-Wimplicit-fallthrough.

Notice that -Wimplicit-fallthrough will be globally
enabled in v5.3.

Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/input/mouse/elantech.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 73544776a9ed..04fe43440a3c 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -1842,6 +1842,7 @@ static bool elantech_use_host_notify(struct psmouse *psmouse,
 		/* SMbus implementation is stable since 2018 */
 		if (dmi_get_bios_year() >= 2018)
 			return true;
+		/* fall through */
 	default:
 		psmouse_dbg(psmouse,
 			    "Ignoring SMBus bus provider %d\n", info->bus);
-- 
2.22.0

^ permalink raw reply related

* Reminder: 3 open syzbot bugs in hid subsystem
From: Eric Biggers @ 2019-07-24  2:41 UTC (permalink / raw)
  To: linux-input, Jiri Kosina, Benjamin Tissoires; +Cc: linux-kernel, syzkaller-bugs

[This email was generated by a script.  Let me know if you have any suggestions
to make it better, or if you want it re-generated with the latest status.]

Of the currently open syzbot reports against the upstream kernel, I've manually
marked 3 of them as possibly being bugs in the hid subsystem.  I've listed these
reports below, sorted by an algorithm that tries to list first the reports most
likely to be still valid, important, and actionable.

Of these 3 bugs, 2 were seen in mainline in the last week.

If you believe a bug is no longer valid, please close the syzbot report by
sending a '#syz fix', '#syz dup', or '#syz invalid' command in reply to the
original thread, as explained at https://goo.gl/tpsmEJ#status

If you believe I misattributed a bug to the hid subsystem, please let me know,
and if possible forward the report to the correct people or mailing list.

Here are the bugs:

--------------------------------------------------------------------------------
Title:              KASAN: use-after-free Read in hidraw_ioctl
Last occurred:      0 days ago
Reported:           0 days ago
Branches:           Mainline (with usb-fuzzer patches)
Dashboard link:     https://syzkaller.appspot.com/bug?id=c7e345ba243bc4476aae52a3354ccbd2a90e344e
Original thread:    https://lkml.kernel.org/lkml/000000000000c07378058e589a29@google.com/T/#u

This bug has a C reproducer.

No one has replied to the original thread for this bug yet.

This looks like a bug in a hid USB driver.

If you fix this bug, please add the following tag to the commit:
    Reported-by: syzbot+ded1794a717e3b235226@syzkaller.appspotmail.com

If you send any email or patch for this bug, please reply to the original
thread.  For the git send-email command to use, or tips on how to reply if the
thread isn't in your mailbox, see the "Reply instructions" at
https://lkml.kernel.org/r/000000000000c07378058e589a29@google.com

--------------------------------------------------------------------------------
Title:              KASAN: use-after-free Read in usbhid_power
Last occurred:      0 days ago
Reported:           0 days ago
Branches:           Mainline (with usb-fuzzer patches)
Dashboard link:     https://syzkaller.appspot.com/bug?id=36143971c5b9b0341ad4018313375a5a40cb52c8
Original thread:    https://lkml.kernel.org/lkml/000000000000bb4247058e589a20@google.com/T/#u

This bug has a C reproducer.

No one has replied to the original thread for this bug yet.

This looks like a bug in a hid USB driver.

If you fix this bug, please add the following tag to the commit:
    Reported-by: syzbot+ef5de9c4f99c4edb4e49@syzkaller.appspotmail.com

If you send any email or patch for this bug, please reply to the original
thread.  For the git send-email command to use, or tips on how to reply if the
thread isn't in your mailbox, see the "Reply instructions" at
https://lkml.kernel.org/r/000000000000bb4247058e589a20@google.com

--------------------------------------------------------------------------------
Title:              INFO: task hung in fsnotify_connector_destroy_workfn (2)
Last occurred:      32 days ago
Reported:           311 days ago
Branches:           Mainline and others
Dashboard link:     https://syzkaller.appspot.com/bug?id=d6011f00f49a2253c15a60ac102b2ea79e3ee8de
Original thread:    https://lkml.kernel.org/lkml/0000000000006364200575dfc280@google.com/T/#u

This bug has a syzkaller reproducer only.

The original thread for this bug received 7 replies; the last was 301 days ago.

If you fix this bug, please add the following tag to the commit:
    Reported-by: syzbot+6fb572170402d311dd39@syzkaller.appspotmail.com

If you send any email or patch for this bug, please consider replying to the
original thread.  For the git send-email command to use, or tips on how to reply
if the thread isn't in your mailbox, see the "Reply instructions" at
https://lkml.kernel.org/r/0000000000006364200575dfc280@google.com

^ permalink raw reply

* Reminder: 8 open syzbot bugs in input subsystem
From: Eric Biggers @ 2019-07-24  2:27 UTC (permalink / raw)
  To: linux-input, Dmitry Torokhov; +Cc: linux-kernel, syzkaller-bugs

[This email was generated by a script.  Let me know if you have any suggestions
to make it better, or if you want it re-generated with the latest status.]

Of the currently open syzbot reports against the upstream kernel, I've manually
marked 8 of them as possibly being bugs in the input subsystem.  I've listed
these reports below, sorted by an algorithm that tries to list first the reports
most likely to be still valid, important, and actionable.

Of these 8 bugs, 6 were seen in mainline in the last week.

If you believe a bug is no longer valid, please close the syzbot report by
sending a '#syz fix', '#syz dup', or '#syz invalid' command in reply to the
original thread, as explained at https://goo.gl/tpsmEJ#status

If you believe I misattributed a bug to the input subsystem, please let me know,
and if possible forward the report to the correct people or mailing list.

Here are the bugs:

--------------------------------------------------------------------------------
Title:              WARNING in aiptek_open/usb_submit_urb
Last occurred:      0 days ago
Reported:           46 days ago
Branches:           Mainline (with usb-fuzzer patches)
Dashboard link:     https://syzkaller.appspot.com/bug?id=0e35393fd821f0570b2a1663a01ac7bdcd15046a
Original thread:    https://lkml.kernel.org/lkml/0000000000001abc1c058ab95b3e@google.com/T/#u

This bug has a C reproducer.

No one has replied to the original thread for this bug yet.

This looks like a bug in an input USB driver.

If you fix this bug, please add the following tag to the commit:
    Reported-by: syzbot+75cccf2b7da87fb6f84b@syzkaller.appspotmail.com

If you send any email or patch for this bug, please consider replying to the
original thread.  For the git send-email command to use, or tips on how to reply
if the thread isn't in your mailbox, see the "Reply instructions" at
https://lkml.kernel.org/r/0000000000001abc1c058ab95b3e@google.com

--------------------------------------------------------------------------------
Title:              INFO: trying to register non-static key in usbtouch_open
Last occurred:      0 days ago
Reported:           0 days ago
Branches:           Mainline (with usb-fuzzer patches)
Dashboard link:     https://syzkaller.appspot.com/bug?id=19bb4d1c56f91465a4a9f5396f0607d487947838
Original thread:    https://lkml.kernel.org/lkml/000000000000b69261058e589a1b@google.com/T/#u

This bug has a C reproducer.

No one has replied to the original thread for this bug yet.

This looks like a bug in an input USB driver.

If you fix this bug, please add the following tag to the commit:
    Reported-by: syzbot+f9c21a30eb9d374e30c1@syzkaller.appspotmail.com

If you send any email or patch for this bug, please reply to the original
thread.  For the git send-email command to use, or tips on how to reply if the
thread isn't in your mailbox, see the "Reply instructions" at
https://lkml.kernel.org/r/000000000000b69261058e589a1b@google.com

--------------------------------------------------------------------------------
Title:              WARNING in kbtab_open/usb_submit_urb
Last occurred:      0 days ago
Reported:           0 days ago
Branches:           Mainline (with usb-fuzzer patches)
Dashboard link:     https://syzkaller.appspot.com/bug?id=ad3a473cad38ce6fbd413eac666501abdc471d31
Original thread:    https://lkml.kernel.org/lkml/000000000000fa7ce2058e5c8318@google.com/T/#u

This bug has a C reproducer.

No one has replied to the original thread for this bug yet.

This looks like a bug in an input USB driver.

If you fix this bug, please add the following tag to the commit:
    Reported-by: syzbot+c7df50363aaff50aa363@syzkaller.appspotmail.com

If you send any email or patch for this bug, please reply to the original
thread.  For the git send-email command to use, or tips on how to reply if the
thread isn't in your mailbox, see the "Reply instructions" at
https://lkml.kernel.org/r/000000000000fa7ce2058e5c8318@google.com

--------------------------------------------------------------------------------
Title:              WARNING in iforce_get_id_packet/usb_submit_urb
Last occurred:      4 days ago
Reported:           13 days ago
Branches:           Mainline (with usb-fuzzer patches)
Dashboard link:     https://syzkaller.appspot.com/bug?id=bd1dc4784ff2c89f9d35cd17b02a4a0e1baa1df4
Original thread:    https://lkml.kernel.org/lkml/000000000000a901ed058d51adc3@google.com/T/#u

This bug has a C reproducer.

No one has replied to the original thread for this bug yet.

This looks like a bug in an input USB driver.

If you fix this bug, please add the following tag to the commit:
    Reported-by: syzbot+9584b712baf1965b590c@syzkaller.appspotmail.com

If you send any email or patch for this bug, please reply to the original
thread.  For the git send-email command to use, or tips on how to reply if the
thread isn't in your mailbox, see the "Reply instructions" at
https://lkml.kernel.org/r/000000000000a901ed058d51adc3@google.com

--------------------------------------------------------------------------------
Title:              KASAN: use-after-free Read in usb_anchor_resume_wakeups
Last occurred:      0 days ago
Reported:           14 days ago
Branches:           Mainline (with usb-fuzzer patches)
Dashboard link:     https://syzkaller.appspot.com/bug?id=21616f648e5fb8ca17de9b869fec3b27270b9edb
Original thread:    https://lkml.kernel.org/lkml/000000000000e9312b058d3eadb8@google.com/T/#u

Unfortunately, this bug does not have a reproducer.

No one has replied to the original thread for this bug yet.

This looks like a bug in an input USB driver.

If you fix this bug, please add the following tag to the commit:
    Reported-by: syzbot+58e201002fe1e775e1ae@syzkaller.appspotmail.com

If you send any email or patch for this bug, please reply to the original
thread.  For the git send-email command to use, or tips on how to reply if the
thread isn't in your mailbox, see the "Reply instructions" at
https://lkml.kernel.org/r/000000000000e9312b058d3eadb8@google.com

--------------------------------------------------------------------------------
Title:              KASAN: use-after-free Write in usb_anchor_resume_wakeups
Last occurred:      2 days ago
Reported:           14 days ago
Branches:           Mainline (with usb-fuzzer patches)
Dashboard link:     https://syzkaller.appspot.com/bug?id=f51395a36f8f9cc43f9538a1c961b87ff7dff3d3
Original thread:    https://lkml.kernel.org/lkml/000000000000ede4ad058d3ead9e@google.com/T/#u

Unfortunately, this bug does not have a reproducer.

No one has replied to the original thread for this bug yet.

This looks like a bug in an input USB driver.

If you fix this bug, please add the following tag to the commit:
    Reported-by: syzbot+6c355f8d5f11884fa38e@syzkaller.appspotmail.com

If you send any email or patch for this bug, please reply to the original
thread.  For the git send-email command to use, or tips on how to reply if the
thread isn't in your mailbox, see the "Reply instructions" at
https://lkml.kernel.org/r/000000000000ede4ad058d3ead9e@google.com

--------------------------------------------------------------------------------
Title:              INFO: trying to register non-static key in usbtouch_reset_resume
Last occurred:      33 days ago
Reported:           57 days ago
Branches:           Mainline (with usb-fuzzer patches)
Dashboard link:     https://syzkaller.appspot.com/bug?id=64fd387d8358406dc0037511ee44db159f6f1605
Original thread:    https://lkml.kernel.org/lkml/0000000000005463aa0589dcfb85@google.com/T/#u

This bug has a C reproducer.

No one has replied to the original thread for this bug yet.

This looks like a bug in an input USB driver.

If you fix this bug, please add the following tag to the commit:
    Reported-by: syzbot+933daad9be4e67ba91a9@syzkaller.appspotmail.com

If you send any email or patch for this bug, please consider replying to the
original thread.  For the git send-email command to use, or tips on how to reply
if the thread isn't in your mailbox, see the "Reply instructions" at
https://lkml.kernel.org/r/0000000000005463aa0589dcfb85@google.com

--------------------------------------------------------------------------------
Title:              INFO: task hung in evdev_release
Last occurred:      273 days ago
Reported:           280 days ago
Branches:           Mainline and others
Dashboard link:     https://syzkaller.appspot.com/bug?id=ebbbff1dcac574b81f9fd5e07100a4879e5bf53d
Original thread:    https://lkml.kernel.org/lkml/000000000000f1be430578524a20@google.com/T/#u

This bug has a syzkaller reproducer only.

syzbot has bisected this bug, but I think the bisection result is incorrect.

The original thread for this bug received 1 reply, 113 days ago.

If you fix this bug, please add the following tag to the commit:
    Reported-by: syzbot+a979743610b4755d4d57@syzkaller.appspotmail.com

If you send any email or patch for this bug, please consider replying to the
original thread.  For the git send-email command to use, or tips on how to reply
if the thread isn't in your mailbox, see the "Reply instructions" at
https://lkml.kernel.org/r/000000000000f1be430578524a20@google.com

^ permalink raw reply

* [PATCH] HID: wacom: fix bit shift for Cintiq Companion 2
From: Aaron Armstrong Skomra @ 2019-07-23 18:09 UTC (permalink / raw)
  To: linux-input, jikos, benjamin.tissoires, pinglinux, jason.gerecke
  Cc: Aaron Armstrong Skomra, # v4 . 5+

The bit indicating BTN_6 on this device is overshifted
by 2 bits, resulting in the incorrect button being
reported.

Also fix copy-paste mistake in comments.

Signed-off-by: Aaron Armstrong Skomra <aaron.skomra@wacom.com>
Reviewed-by: Ping Cheng <ping.cheng@wacom.com>
Link: https://github.com/linuxwacom/xf86-input-wacom/issues/71
Fixes: c7f0522a1ad1 ("HID: wacom: Slim down wacom_intuos_pad processing")
Cc: <stable@vger.kernel.org> # v4.5+
---
 drivers/hid/wacom_wac.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 8fc36a28081b..7a8ddc999a8e 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -533,14 +533,14 @@ static int wacom_intuos_pad(struct wacom_wac *wacom)
 		 */
 		buttons = (data[4] << 1) | (data[3] & 0x01);
 	} else if (features->type == CINTIQ_COMPANION_2) {
-		/* d-pad right  -> data[4] & 0x10
-		 * d-pad up     -> data[4] & 0x20
-		 * d-pad left   -> data[4] & 0x40
-		 * d-pad down   -> data[4] & 0x80
-		 * d-pad center -> data[3] & 0x01
+		/* d-pad right  -> data[2] & 0x10
+		 * d-pad up     -> data[2] & 0x20
+		 * d-pad left   -> data[2] & 0x40
+		 * d-pad down   -> data[2] & 0x80
+		 * d-pad center -> data[1] & 0x01
 		 */
 		buttons = ((data[2] >> 4) << 7) |
-		          ((data[1] & 0x04) << 6) |
+		          ((data[1] & 0x04) << 4) |
 		          ((data[2] & 0x0F) << 2) |
 		          (data[1] & 0x03);
 	} else if (features->type >= INTUOS5S && features->type <= INTUOSPL) {
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH] Input: Apple SPI keyboard needs CRC16
From: Dmitry Torokhov @ 2019-07-23 13:42 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Ronald Tschalär, Andy Shevchenko, linux-input, linux-kernel
In-Reply-To: <20190723115905.2092687-1-arnd@arndb.de>

On Tue, Jul 23, 2019 at 01:58:45PM +0200, Arnd Bergmann wrote:
> In some rare randconfig builds, CRC16 is disabled, which leads
> to a link error:
> 
> drivers/input/keyboard/applespi.o: In function `applespi_send_cmd_msg':
> applespi.c:(.text+0x449f): undefined reference to `crc16'
> drivers/input/keyboard/applespi.o: In function `applespi_verify_crc':
> applespi.c:(.text+0x7538): undefined reference to `crc16'
> 
> This symbol is meant to be selected for each user in Kconfig,
> so do that here as well.
> 
> Fixes: 038b1a05eae6 ("Input: add Apple SPI keyboard and trackpad driver")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Applied, thank you.

> ---
>  drivers/input/keyboard/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index 8e9c3ea9d5e7..ebb19e21473e 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -76,6 +76,7 @@ config KEYBOARD_APPLESPI
>  	depends on ACPI && EFI
>  	depends on SPI
>  	depends on X86 || COMPILE_TEST
> +	select CRC16
>  	help
>  	  Say Y here if you are running Linux on any Apple MacBook8,1 or later,
>  	  or any MacBookPro13,* or MacBookPro14,*.
> -- 
> 2.20.0
> 

-- 
Dmitry

^ permalink raw reply

* [PATCH] Input: Apple SPI keyboard needs CRC16
From: Arnd Bergmann @ 2019-07-23 11:58 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Arnd Bergmann, Ronald Tschalär, Andy Shevchenko, linux-input,
	linux-kernel

In some rare randconfig builds, CRC16 is disabled, which leads
to a link error:

drivers/input/keyboard/applespi.o: In function `applespi_send_cmd_msg':
applespi.c:(.text+0x449f): undefined reference to `crc16'
drivers/input/keyboard/applespi.o: In function `applespi_verify_crc':
applespi.c:(.text+0x7538): undefined reference to `crc16'

This symbol is meant to be selected for each user in Kconfig,
so do that here as well.

Fixes: 038b1a05eae6 ("Input: add Apple SPI keyboard and trackpad driver")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/input/keyboard/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 8e9c3ea9d5e7..ebb19e21473e 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -76,6 +76,7 @@ config KEYBOARD_APPLESPI
 	depends on ACPI && EFI
 	depends on SPI
 	depends on X86 || COMPILE_TEST
+	select CRC16
 	help
 	  Say Y here if you are running Linux on any Apple MacBook8,1 or later,
 	  or any MacBookPro13,* or MacBookPro14,*.
-- 
2.20.0

^ permalink raw reply related

* Re: [PATCH v3 2/2] Input: soc_button_array - Add support for newer surface devices
From: Maximilian Luz @ 2019-07-23 11:29 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: linux-kernel, linux-input, platform-driver-x86, Dmitry Torokhov,
	Hans de Goede, Chen Yu, Darren Hart, Andy Shevchenko,
	Benjamin Tissoires
In-Reply-To: <a2f75544-54ce-abce-56a4-ca226dbed51f@metux.net>

On 7/22/19 10:00 AM, Enrico Weigelt, metux IT consult wrote:
> On 20.07.19 17:05, Maximilian Luz wrote:
>> Power and volume button support for 5th and 6th generation Microsoft
>> Surface devices via soc_button_array.
>>
>> Note that these devices use the same MSHW0040 device as on the Surface
>> Pro 4, however the implementation is different (GPIOs vs. ACPI
>> notifications). Thus some checking is required to ensure we only load
>> this driver on the correct devices.
> 
> Could this also used on the older (pre pro4) devices (also using the
> gpios directly, and leave off acpi notifications) ?

As far as I can tell, no. The Pro 4 and Pro 3 don't have any GPIOs on
MSHW0028/MSHW0040. Book 1 has GPIOs but for a different purpose. The Pro
2 has a standard PNP0C0C power button (no idea how the volume buttons
are handled there, but also seems to be different from what I can gather
from DSDT). I can't say anything for the Pro 1 and non-Pro devices.

Maximilian

^ permalink raw reply

* [PATCH] HID: intel-ish-hid: Use dev_get_drvdata where possible
From: Chuhong Yuan @ 2019-07-23 11:10 UTC (permalink / raw)
  Cc: Srinivas Pandruvada, Jiri Kosina, Benjamin Tissoires, linux-input,
	linux-kernel, Chuhong Yuan

Instead of using to_pci_dev + pci_get_drvdata,
use dev_get_drvdata to make code simpler.

Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
---
 drivers/hid/intel-ish-hid/ipc/pci-ish.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
index aa80b4d3b740..7047c14516c9 100644
--- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
+++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
@@ -204,8 +204,7 @@ static struct device __maybe_unused *ish_resume_device;
  */
 static void __maybe_unused ish_resume_handler(struct work_struct *work)
 {
-	struct pci_dev *pdev = to_pci_dev(ish_resume_device);
-	struct ishtp_device *dev = pci_get_drvdata(pdev);
+	struct ishtp_device *dev = dev_get_drvdata(ish_resume_device);
 	uint32_t fwsts;
 	int ret;
 
-- 
2.20.1

^ permalink raw reply related

* Re: [PATCH v3] Input: elantech: Enable SMBus on new (2018+) systems
From: Kai-Heng Feng @ 2019-07-23  6:45 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: benjamin.tissoires, linux-input, linux-kernel, Your Name
In-Reply-To: <20190722081739.GA804@penguin>

Hi Dmitry,

at 16:17, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> Hi Kai-Heng,
>
> On Mon, Jul 22, 2019 at 03:40:55PM +0800, Kai-Heng Feng wrote:
>> There are some new HP laptops with Elantech touchpad don't support
>> multitouch.
>>
>> Currently we use ETP_NEW_IC_SMBUS_HOST_NOTIFY() to check if SMBus is
>> supported, but in addition to firmware version, the bus type also
>> informs us if the IC can support SMBus, so also check that.
>>
>> In case of breaking old ICs, only enables SMBus on systems manufactured
>> after 2018, alongsides aforementioned checks.
>>
>> Lastly, consolidats all check into elantech_use_host_notify() and use it
>> to determine whether to use PS/2 or SMBus.
>>
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>> Signed-off-by: Your Name <you@example.com>
>
> I do not think "Your Name" should be signing DCO here :)

Yes I don’t know how I messed that up...

>
>> +static bool elantech_use_host_notify(struct psmouse *psmouse,
>> +				     struct elantech_device_info *info)
>> +{
>> +	bool host_notify = false;
>> +
>> +	if (ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version))
>> +		host_notify = true;
>> +	else {
>> +		switch (info->bus) {
>> +		case ETP_BUS_PS2_ONLY:
>> +			/* expected case */
>> +			break;
>> +		case ETP_BUS_SMB_ALERT_ONLY:
>> +			/* fall-through  */
>> +		case ETP_BUS_PS2_SMB_ALERT:
>> +			psmouse_dbg(psmouse, "Ignoring SMBus provider  
>> through alert protocol.\n");
>> +			break;
>> +		case ETP_BUS_SMB_HST_NTFY_ONLY:
>> +			/* fall-through  */
>> +		case ETP_BUS_PS2_SMB_HST_NTFY:
>> +			/* SMbus implementation is stable since 2018 */
>> +			if (dmi_get_bios_year() >= 2018)
>> +				host_notify = true;
>> +			break;
>> +		default:
>> +			psmouse_dbg(psmouse,
>> +				    "Ignoring SMBus bus provider %d.\n",
>> +				    info->bus);
>> +		}
>> +	}
>
> I think this is way too verbose. How about a bit more condensed form:

The one revised by you is more succinct. Please use yours instead :)

Thanks!

Kai-Heng

>
>
> Input: elantech - enable SMBus on new (2018+) systems
>
> From: Kai-Heng Feng <kai.heng.feng@canonical.com>
>
> There are some new HP laptops with Elantech touchpad that don't support
> multitouch.
>
> Currently we use ETP_NEW_IC_SMBUS_HOST_NOTIFY() to check if SMBus is  
> supported,
> but in addition to firmware version, the bus type also informs us whether  
> the IC
> can support SMBus. To avoid breaking old ICs, we will only enable SMbus  
> support
> based the bus type on systems manufactured after 2018.
>
> Lastly, let's consolidate all checks into elantech_use_host_notify() and  
> use it
> to determine whether to use PS/2 or SMBus.
>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/mouse/elantech.c |   54 +++++++++++++++++++---------------------
>  1 file changed, 25 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/input/mouse/elantech.c  
> b/drivers/input/mouse/elantech.c
> index 2d8434b7b623..73544776a9ed 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -1827,6 +1827,30 @@ static int elantech_create_smbus(struct psmouse  
> *psmouse,
>  				  leave_breadcrumbs);
>  }
>
> +static bool elantech_use_host_notify(struct psmouse *psmouse,
> +				     struct elantech_device_info *info)
> +{
> +	if (ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version))
> +		return true;
> +
> +	switch (info->bus) {
> +	case ETP_BUS_PS2_ONLY:
> +		/* expected case */
> +		break;
> +	case ETP_BUS_SMB_HST_NTFY_ONLY:
> +	case ETP_BUS_PS2_SMB_HST_NTFY:
> +		/* SMbus implementation is stable since 2018 */
> +		if (dmi_get_bios_year() >= 2018)
> +			return true;
> +	default:
> +		psmouse_dbg(psmouse,
> +			    "Ignoring SMBus bus provider %d\n", info->bus);
> +		break;
> +	}
> +
> +	return false;
> +}
> +
>  /**
>   * elantech_setup_smbus - called once the PS/2 devices are enumerated
>   * and decides to instantiate a SMBus InterTouch device.
> @@ -1846,7 +1870,7 @@ static int elantech_setup_smbus(struct psmouse  
> *psmouse,
>  		 * i2c_blacklist_pnp_ids.
>  		 * Old ICs are up to the user to decide.
>  		 */
> -		if (!ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version) ||
> +		if (!elantech_use_host_notify(psmouse, info) ||
>  		    psmouse_matches_pnp_id(psmouse, i2c_blacklist_pnp_ids))
>  			return -ENXIO;
>  	}
> @@ -1866,34 +1890,6 @@ static int elantech_setup_smbus(struct psmouse  
> *psmouse,
>  	return 0;
>  }
>
> -static bool elantech_use_host_notify(struct psmouse *psmouse,
> -				     struct elantech_device_info *info)
> -{
> -	if (ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version))
> -		return true;
> -
> -	switch (info->bus) {
> -	case ETP_BUS_PS2_ONLY:
> -		/* expected case */
> -		break;
> -	case ETP_BUS_SMB_ALERT_ONLY:
> -		/* fall-through  */
> -	case ETP_BUS_PS2_SMB_ALERT:
> -		psmouse_dbg(psmouse, "Ignoring SMBus provider through alert  
> protocol.\n");
> -		break;
> -	case ETP_BUS_SMB_HST_NTFY_ONLY:
> -		/* fall-through  */
> -	case ETP_BUS_PS2_SMB_HST_NTFY:
> -		return true;
> -	default:
> -		psmouse_dbg(psmouse,
> -			    "Ignoring SMBus bus provider %d.\n",
> -			    info->bus);
> -	}
> -
> -	return false;
> -}
> -
>  int elantech_init_smbus(struct psmouse *psmouse)
>  {
>  	struct elantech_device_info info;
>
> Thanks.
>
> -- 
> Dmitry

^ permalink raw reply

* Re: [PATCH v8 3/5] arm64: dts: qcom: Add Lenovo Miix 630
From: Bjorn Andersson @ 2019-07-22 23:34 UTC (permalink / raw)
  To: Jeffrey Hugo
  Cc: agross, benjamin.tissoires, dmitry.torokhov, jikos, hdegoede,
	lee.jones, xnox, robh+dt, mark.rutland, linux-input, devicetree,
	linux-arm-msm, linux-kernel
In-Reply-To: <20190621145450.38741-1-jeffrey.l.hugo@gmail.com>

On Fri 21 Jun 07:54 PDT 2019, Jeffrey Hugo wrote:

> This adds the initial DT for the Lenovo Miix 630 laptop.  Supported
> functionality includes USB (host), microSD-card, keyboard, and trackpad.
> 
> Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>

Applied patch 3-5

Thanks,
Bjorn

> ---
>  arch/arm64/boot/dts/qcom/Makefile             |   1 +
>  .../boot/dts/qcom/msm8998-clamshell.dtsi      | 240 ++++++++++++++++++
>  .../boot/dts/qcom/msm8998-lenovo-miix-630.dts |  30 +++
>  3 files changed, 271 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/qcom/msm8998-clamshell.dtsi
>  create mode 100644 arch/arm64/boot/dts/qcom/msm8998-lenovo-miix-630.dts
> 
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index 21d548f02d39..c3e4307bcbd4 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -6,6 +6,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-mtp.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8992-bullhead-rev-101.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8994-angler-rev-101.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8996-mtp.dtb
> +dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-lenovo-miix-630.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= msm8998-mtp.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-mtp.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= qcs404-evb-1000.dtb
> diff --git a/arch/arm64/boot/dts/qcom/msm8998-clamshell.dtsi b/arch/arm64/boot/dts/qcom/msm8998-clamshell.dtsi
> new file mode 100644
> index 000000000000..9682d4dd7496
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/msm8998-clamshell.dtsi
> @@ -0,0 +1,240 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2019, Jeffrey Hugo. All rights reserved. */
> +
> +/*
> + * Common include for MSM8998 clamshell devices, ie the Lenovo Miix 630,
> + * Asus NovaGo TP370QL, and HP Envy x2.  All three devices are basically the
> + * same, with differences in peripherals.
> + */
> +
> +#include "msm8998.dtsi"
> +#include "pm8998.dtsi"
> +#include "pm8005.dtsi"
> +
> +/ {
> +	chosen {
> +	};
> +
> +	vph_pwr: vph-pwr-regulator {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vph_pwr";
> +		regulator-always-on;
> +		regulator-boot-on;
> +	};
> +};
> +
> +&qusb2phy {
> +	status = "okay";
> +
> +	vdda-pll-supply = <&vreg_l12a_1p8>;
> +	vdda-phy-dpdm-supply = <&vreg_l24a_3p075>;
> +};
> +
> +&rpm_requests {
> +	pm8998-regulators {
> +		compatible = "qcom,rpm-pm8998-regulators";
> +
> +		vdd_s1-supply = <&vph_pwr>;
> +		vdd_s2-supply = <&vph_pwr>;
> +		vdd_s3-supply = <&vph_pwr>;
> +		vdd_s4-supply = <&vph_pwr>;
> +		vdd_s5-supply = <&vph_pwr>;
> +		vdd_s6-supply = <&vph_pwr>;
> +		vdd_s7-supply = <&vph_pwr>;
> +		vdd_s8-supply = <&vph_pwr>;
> +		vdd_s9-supply = <&vph_pwr>;
> +		vdd_s10-supply = <&vph_pwr>;
> +		vdd_s11-supply = <&vph_pwr>;
> +		vdd_s12-supply = <&vph_pwr>;
> +		vdd_s13-supply = <&vph_pwr>;
> +		vdd_l1_l27-supply = <&vreg_s7a_1p025>;
> +		vdd_l2_l8_l17-supply = <&vreg_s3a_1p35>;
> +		vdd_l3_l11-supply = <&vreg_s7a_1p025>;
> +		vdd_l4_l5-supply = <&vreg_s7a_1p025>;
> +		vdd_l6-supply = <&vreg_s5a_2p04>;
> +		vdd_l7_l12_l14_l15-supply = <&vreg_s5a_2p04>;
> +		vdd_l9-supply = <&vph_pwr>;
> +		vdd_l10_l23_l25-supply = <&vph_pwr>;
> +		vdd_l13_l19_l21-supply = <&vph_pwr>;
> +		vdd_l16_l28-supply = <&vph_pwr>;
> +		vdd_l18_l22-supply = <&vph_pwr>;
> +		vdd_l20_l24-supply = <&vph_pwr>;
> +		vdd_l26-supply = <&vreg_s3a_1p35>;
> +		vdd_lvs1_lvs2-supply = <&vreg_s4a_1p8>;
> +
> +		vreg_s3a_1p35: s3 {
> +			regulator-min-microvolt = <1352000>;
> +			regulator-max-microvolt = <1352000>;
> +		};
> +		vreg_s4a_1p8: s4 {
> +			regulator-min-microvolt = <1800000>;
> +			regulator-max-microvolt = <1800000>;
> +			regulator-allow-set-load;
> +		};
> +		vreg_s5a_2p04: s5 {
> +			regulator-min-microvolt = <1904000>;
> +			regulator-max-microvolt = <2040000>;
> +		};
> +		vreg_s7a_1p025: s7 {
> +			regulator-min-microvolt = <900000>;
> +			regulator-max-microvolt = <1028000>;
> +		};
> +		vreg_l1a_0p875: l1 {
> +			regulator-min-microvolt = <880000>;
> +			regulator-max-microvolt = <880000>;
> +			regulator-allow-set-load;
> +		};
> +		vreg_l2a_1p2: l2 {
> +			regulator-min-microvolt = <1200000>;
> +			regulator-max-microvolt = <1200000>;
> +			regulator-allow-set-load;
> +		};
> +		vreg_l3a_1p0: l3 {
> +			regulator-min-microvolt = <1000000>;
> +			regulator-max-microvolt = <1000000>;
> +		};
> +		vreg_l5a_0p8: l5 {
> +			regulator-min-microvolt = <800000>;
> +			regulator-max-microvolt = <800000>;
> +		};
> +		vreg_l6a_1p8: l6 {
> +			regulator-min-microvolt = <1808000>;
> +			regulator-max-microvolt = <1808000>;
> +		};
> +		vreg_l7a_1p8: l7 {
> +			regulator-min-microvolt = <1800000>;
> +			regulator-max-microvolt = <1800000>;
> +		};
> +		vreg_l8a_1p2: l8 {
> +			regulator-min-microvolt = <1200000>;
> +			regulator-max-microvolt = <1200000>;
> +		};
> +		vreg_l9a_1p8: l9 {
> +			regulator-min-microvolt = <1808000>;
> +			regulator-max-microvolt = <2960000>;
> +		};
> +		vreg_l10a_1p8: l10 {
> +			regulator-min-microvolt = <1808000>;
> +			regulator-max-microvolt = <2960000>;
> +		};
> +		vreg_l11a_1p0: l11 {
> +			regulator-min-microvolt = <1000000>;
> +			regulator-max-microvolt = <1000000>;
> +		};
> +		vreg_l12a_1p8: l12 {
> +			regulator-min-microvolt = <1800000>;
> +			regulator-max-microvolt = <1800000>;
> +		};
> +		vreg_l13a_2p95: l13 {
> +			regulator-min-microvolt = <1808000>;
> +			regulator-max-microvolt = <2960000>;
> +		};
> +		vreg_l14a_1p88: l14 {
> +			regulator-min-microvolt = <1880000>;
> +			regulator-max-microvolt = <1880000>;
> +		};
> +		vreg_15a_1p8: l15 {
> +			regulator-min-microvolt = <1800000>;
> +			regulator-max-microvolt = <1800000>;
> +		};
> +		vreg_l16a_2p7: l16 {
> +			regulator-min-microvolt = <2704000>;
> +			regulator-max-microvolt = <2704000>;
> +		};
> +		vreg_l17a_1p3: l17 {
> +			regulator-min-microvolt = <1304000>;
> +			regulator-max-microvolt = <1304000>;
> +		};
> +		vreg_l18a_2p7: l18 {
> +			regulator-min-microvolt = <2704000>;
> +			regulator-max-microvolt = <2704000>;
> +		};
> +		vreg_l19a_3p0: l19 {
> +			regulator-min-microvolt = <3008000>;
> +			regulator-max-microvolt = <3008000>;
> +		};
> +		vreg_l20a_2p95: l20 {
> +			regulator-min-microvolt = <2960000>;
> +			regulator-max-microvolt = <2960000>;
> +			regulator-allow-set-load;
> +		};
> +		vreg_l21a_2p95: l21 {
> +			regulator-min-microvolt = <2960000>;
> +			regulator-max-microvolt = <2960000>;
> +			regulator-allow-set-load;
> +			regulator-system-load = <800000>;
> +		};
> +		vreg_l22a_2p85: l22 {
> +			regulator-min-microvolt = <2864000>;
> +			regulator-max-microvolt = <2864000>;
> +		};
> +		vreg_l23a_3p3: l23 {
> +			regulator-min-microvolt = <3312000>;
> +			regulator-max-microvolt = <3312000>;
> +		};
> +		vreg_l24a_3p075: l24 {
> +			regulator-min-microvolt = <3088000>;
> +			regulator-max-microvolt = <3088000>;
> +		};
> +		vreg_l25a_3p3: l25 {
> +			regulator-min-microvolt = <3104000>;
> +			regulator-max-microvolt = <3312000>;
> +		};
> +		vreg_l26a_1p2: l26 {
> +			regulator-min-microvolt = <1200000>;
> +			regulator-max-microvolt = <1200000>;
> +		};
> +		vreg_l28_3p0: l28 {
> +			regulator-min-microvolt = <3008000>;
> +			regulator-max-microvolt = <3008000>;
> +		};
> +
> +		vreg_lvs1a_1p8: lvs1 {
> +			regulator-min-microvolt = <1800000>;
> +			regulator-max-microvolt = <1800000>;
> +		};
> +
> +		vreg_lvs2a_1p8: lvs2 {
> +			regulator-min-microvolt = <1800000>;
> +			regulator-max-microvolt = <1800000>;
> +		};
> +
> +	};
> +};
> +
> +&tlmm {
> +	gpio-reserved-ranges = <0 4>, <81 4>;
> +
> +	touchpad: touchpad {
> +		config {
> +			pins = "gpio123";
> +			bias-pull-up;           /* pull up */
> +		};
> +	};
> +};
> +
> +&sdhc2 {
> +	status = "okay";
> +
> +	vmmc-supply = <&vreg_l21a_2p95>;
> +	vqmmc-supply = <&vreg_l13a_2p95>;
> +
> +	pinctrl-names = "default", "sleep";
> +	pinctrl-0 = <&sdc2_clk_on  &sdc2_cmd_on  &sdc2_data_on  &sdc2_cd_on>;
> +	pinctrl-1 = <&sdc2_clk_off &sdc2_cmd_off &sdc2_data_off &sdc2_cd_off>;
> +};
> +
> +&usb3 {
> +	status = "okay";
> +};
> +
> +&usb3_dwc3 {
> +	dr_mode = "host"; /* Force to host until we have Type-C hooked up */
> +};
> +
> +&usb3phy {
> +	status = "okay";
> +
> +	vdda-phy-supply = <&vreg_l1a_0p875>;
> +	vdda-pll-supply = <&vreg_l2a_1p2>;
> +};
> diff --git a/arch/arm64/boot/dts/qcom/msm8998-lenovo-miix-630.dts b/arch/arm64/boot/dts/qcom/msm8998-lenovo-miix-630.dts
> new file mode 100644
> index 000000000000..407c6a32911c
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/msm8998-lenovo-miix-630.dts
> @@ -0,0 +1,30 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2019, Jeffrey Hugo. All rights reserved. */
> +
> +/dts-v1/;
> +
> +#include "msm8998-clamshell.dtsi"
> +
> +/ {
> +	model = "Lenovo Miix 630";
> +	compatible = "lenovo,miix-630", "qcom,msm8998";
> +};
> +
> +&blsp1_i2c6 {
> +	status = "okay";
> +
> +	keyboard@3a {
> +		compatible = "hid-over-i2c";
> +		interrupt-parent = <&tlmm>;
> +		interrupts = <0x79 IRQ_TYPE_LEVEL_LOW>;
> +		reg = <0x3a>;
> +		hid-descr-addr = <0x0001>;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&touchpad>;
> +	};
> +};
> +
> +&sdhc2 {
> +	cd-gpios = <&tlmm 95 GPIO_ACTIVE_HIGH>;
> +};
> -- 
> 2.17.1
> 

^ permalink raw reply

* [PATCH v3 3/3] HID: core: only warn once of oversize hid report
From: stillcompiling @ 2019-07-22 21:48 UTC (permalink / raw)
  To: Joe Perches, Jiri Kosina, Benjamin Tissoires,
	open list:HID CORE LAYER, open list
  Cc: Joshua Clayton
In-Reply-To: <20190722214827.29257-1-stillcompiling@gmail.com>

From: Joshua Clayton <stillcompiling@gmail.com>

On HP spectre x360 convertible the message:
hid-sensor-hub 001F:8087:0AC2.0002: hid_field_extract() called with n (192) > 32! (kworker/1:2)
is continually printed many times per second, crowding out all else
Protect dmesg by printing the warning only one time.

Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
 drivers/hid/hid-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 210b81a56e1a..3eaee2c37931 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1311,8 +1311,8 @@ u32 hid_field_extract(const struct hid_device *hid, u8 *report,
 			unsigned offset, unsigned n)
 {
 	if (n > 32) {
-		hid_warn(hid, "hid_field_extract() called with n (%d) > 32! (%s)\n",
-			 n, current->comm);
+		hid_warn_once(hid, "%s() called with n (%d) > 32! (%s)\n",
+			      __func__, n, current->comm);
 		n = 32;
 	}
 
-- 
2.21.0

^ permalink raw reply related

* [PATCH v3 2/3] HID: core: Add printk_once variants to hid_warn() etc
From: stillcompiling @ 2019-07-22 21:48 UTC (permalink / raw)
  To: Joe Perches, Jiri Kosina, Benjamin Tissoires,
	open list:HID CORE LAYER, open list
  Cc: Joshua Clayton
In-Reply-To: <20190722214827.29257-1-stillcompiling@gmail.com>

From: Joshua Clayton <stillcompiling@gmail.com>

hid_warn_once() is needed, add the others as part of the block

Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
 include/linux/hid.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/linux/hid.h b/include/linux/hid.h
index e6c7efdb0458..cd41f209043f 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -1171,4 +1171,15 @@ do {									\
 #define hid_dbg(hid, fmt, ...)				\
 	dev_dbg(&(hid)->dev, fmt, ##__VA_ARGS__)
 
+#define hid_err_once(hid, fmt, ...)			\
+	dev_err_once(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_notice_once(hid, fmt, ...)			\
+	dev_notice_once(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_warn_once(hid, fmt, ...)			\
+	dev_warn_once(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_info_once(hid, fmt, ...)			\
+	dev_info_once(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_dbg_once(hid, fmt, ...)			\
+	dev_dbg_once(&(hid)->dev, fmt, ##__VA_ARGS__)
+
 #endif
-- 
2.21.0

^ permalink raw reply related

* [PATCH v3 1/3] HID: core: reformat and reduce hid_printk macros
From: stillcompiling @ 2019-07-22 21:48 UTC (permalink / raw)
  To: Joe Perches, Jiri Kosina, Benjamin Tissoires,
	open list:HID CORE LAYER, open list
  Cc: Joshua Clayton
In-Reply-To: <20190722163642.10417-2-stillcompiling@gmail.com>

From: Joshua Clayton <stillcompiling@gmail.com>

Reformat hid_printk macros to use standard __VA_ARGS__ syntax
Remove hid_printk(), hid_emerg(), hid_crit(), and hid_alert().
Per Joe Perches these unused and likely never to be used.

Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
 include/linux/hid.h | 32 ++++++++++++--------------------
 1 file changed, 12 insertions(+), 20 deletions(-)

diff --git a/include/linux/hid.h b/include/linux/hid.h
index d770ab1a0479..e6c7efdb0458 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -1154,29 +1154,21 @@ int hid_pidff_init(struct hid_device *hid);
 #define hid_pidff_init NULL
 #endif
 
-#define dbg_hid(format, arg...)						\
+#define dbg_hid(fmt, ...)						\
 do {									\
 	if (hid_debug)							\
-		printk(KERN_DEBUG "%s: " format, __FILE__, ##arg);	\
+		printk(KERN_DEBUG "%s: " fmt, __FILE__, ##__VA_ARGS__);	\
 } while (0)
 
-#define hid_printk(level, hid, fmt, arg...)		\
-	dev_printk(level, &(hid)->dev, fmt, ##arg)
-#define hid_emerg(hid, fmt, arg...)			\
-	dev_emerg(&(hid)->dev, fmt, ##arg)
-#define hid_crit(hid, fmt, arg...)			\
-	dev_crit(&(hid)->dev, fmt, ##arg)
-#define hid_alert(hid, fmt, arg...)			\
-	dev_alert(&(hid)->dev, fmt, ##arg)
-#define hid_err(hid, fmt, arg...)			\
-	dev_err(&(hid)->dev, fmt, ##arg)
-#define hid_notice(hid, fmt, arg...)			\
-	dev_notice(&(hid)->dev, fmt, ##arg)
-#define hid_warn(hid, fmt, arg...)			\
-	dev_warn(&(hid)->dev, fmt, ##arg)
-#define hid_info(hid, fmt, arg...)			\
-	dev_info(&(hid)->dev, fmt, ##arg)
-#define hid_dbg(hid, fmt, arg...)			\
-	dev_dbg(&(hid)->dev, fmt, ##arg)
+#define hid_err(hid, fmt, ...)				\
+	dev_err(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_notice(hid, fmt, ...)			\
+	dev_notice(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_warn(hid, fmt, ...)				\
+	dev_warn(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_info(hid, fmt, ...)				\
+	dev_info(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_dbg(hid, fmt, ...)				\
+	dev_dbg(&(hid)->dev, fmt, ##__VA_ARGS__)
 
 #endif
-- 
2.21.0

^ permalink raw reply related

* Re: [PATCH v2 3/3] HID: core: only warn once of oversize hid report
From: Joshua Clayton @ 2019-07-22 21:43 UTC (permalink / raw)
  To: Joe Perches
  Cc: Jiri Kosina, Benjamin Tissoires, open list:HID CORE LAYER,
	open list
In-Reply-To: <f686e93180d2f91c78801be124e8aac36145d9b0.camel@perches.com>

On Mon, Jul 22, 2019 at 3:30 PM Joe Perches <joe@perches.com> wrote:
>
> On Mon, 2019-07-22 at 15:26 -0600, stillcompiling@gmail.com wrote:
> > From: Joshua Clayton <stillcompiling@gmail.com>
>
> Thanks Joshua
>
> > On HP spectre x360 convertible the message:
> > hid-sensor-hub 001F:8087:0AC2.0002: hid_field_extract() called with n (192) > 32! (kworker/1:2)
> > is continually printed many times per second, crowding out all else
> > Protect dmesg by printing the warning only one time.
> []
> > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> []
> > @@ -1311,8 +1311,8 @@ u32 hid_field_extract(const struct hid_device *hid, u8 *report,
> >                       unsigned offset, unsigned n)
> >  {
> >       if (n > 32) {
> > -             hid_warn(hid, "hid_field_extract() called with n (%d) > 32! (%s)\n",
> > -                      n, current->comm);
> > +             hid_warn_once(hid, "%s() called with n (%d) > 32! (%s)\n",
> > +                           __func__ ,n , current->comm);
>
> All the other bits are fine, but this line is oddly written
> with unusual spacing around 'n'.
>
> Normally it'd be something like:
>
>                 hid_warn_once(hid, "%s: called with n (%d) > 32! (%s)\n",
>                               __func__, n, current->comm);
Gah!
Not only that but I missed a semicolon in patch 1. Will fix, (compile)
and send v3 pdq.
Sorry about the extra noise.
>
>

^ permalink raw reply

* Re: [PATCH v2 3/3] HID: core: only warn once of oversize hid report
From: Joe Perches @ 2019-07-22 21:30 UTC (permalink / raw)
  To: stillcompiling, Jiri Kosina, Benjamin Tissoires,
	open list:HID CORE LAYER, open list
In-Reply-To: <20190722212613.24906-3-stillcompiling@gmail.com>

On Mon, 2019-07-22 at 15:26 -0600, stillcompiling@gmail.com wrote:
> From: Joshua Clayton <stillcompiling@gmail.com>

Thanks Joshua

> On HP spectre x360 convertible the message:
> hid-sensor-hub 001F:8087:0AC2.0002: hid_field_extract() called with n (192) > 32! (kworker/1:2)
> is continually printed many times per second, crowding out all else
> Protect dmesg by printing the warning only one time.
[]
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
[]
> @@ -1311,8 +1311,8 @@ u32 hid_field_extract(const struct hid_device *hid, u8 *report,
>  			unsigned offset, unsigned n)
>  {
>  	if (n > 32) {
> -		hid_warn(hid, "hid_field_extract() called with n (%d) > 32! (%s)\n",
> -			 n, current->comm);
> +		hid_warn_once(hid, "%s() called with n (%d) > 32! (%s)\n",
> +			      __func__ ,n , current->comm);

All the other bits are fine, but this line is oddly written
with unusual spacing around 'n'.

Normally it'd be something like:

		hid_warn_once(hid, "%s: called with n (%d) > 32! (%s)\n",
			      __func__, n, current->comm);

^ permalink raw reply

* [PATCH v2 3/3] HID: core: only warn once of oversize hid report
From: stillcompiling @ 2019-07-22 21:26 UTC (permalink / raw)
  To: Joe Perches, Jiri Kosina, Benjamin Tissoires,
	open list:HID CORE LAYER, open list
  Cc: Joshua Clayton
In-Reply-To: <20190722212613.24906-1-stillcompiling@gmail.com>

From: Joshua Clayton <stillcompiling@gmail.com>

On HP spectre x360 convertible the message:
hid-sensor-hub 001F:8087:0AC2.0002: hid_field_extract() called with n (192) > 32! (kworker/1:2)
is continually printed many times per second, crowding out all else
Protect dmesg by printing the warning only one time.

Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
 drivers/hid/hid-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 210b81a56e1a..0cb53dddf341 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1311,8 +1311,8 @@ u32 hid_field_extract(const struct hid_device *hid, u8 *report,
 			unsigned offset, unsigned n)
 {
 	if (n > 32) {
-		hid_warn(hid, "hid_field_extract() called with n (%d) > 32! (%s)\n",
-			 n, current->comm);
+		hid_warn_once(hid, "%s() called with n (%d) > 32! (%s)\n",
+			      __func__ ,n , current->comm);
 		n = 32;
 	}
 
-- 
2.21.0

^ permalink raw reply related

* [PATCH v2 2/3] HID: core: Add printk_once variants to hid_warn() etc
From: stillcompiling @ 2019-07-22 21:26 UTC (permalink / raw)
  To: Joe Perches, Jiri Kosina, Benjamin Tissoires,
	open list:HID CORE LAYER, open list
  Cc: Joshua Clayton
In-Reply-To: <20190722212613.24906-1-stillcompiling@gmail.com>

From: Joshua Clayton <stillcompiling@gmail.com>

hid_warn_once() is needed, add the others as part of the block

Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
 include/linux/hid.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/linux/hid.h b/include/linux/hid.h
index b5e73331100e..306dde3760a4 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -1171,4 +1171,15 @@ do {									\
 #define hid_dbg(hid, fmt, ...)				\
 	dev_dbg(&(hid)->dev, fmt, ##__VA_ARGS__)
 
+#define hid_err_once(hid, fmt, ...)			\
+	dev_err_once(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_notice_once(hid, fmt, ...)			\
+	dev_notice_once(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_warn_once(hid, fmt, ...)			\
+	dev_warn_once(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_info_once(hid, fmt, ...)			\
+	dev_info_once(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_dbg_once(hid, fmt, ...)			\
+	dev_dbg_once(&(hid)->dev, fmt, ##__VA_ARGS__)
+
 #endif
-- 
2.21.0

^ permalink raw reply related

* [PATCH v2 1/3] HID: core: reformat and reduce hid_printk macros
From: stillcompiling @ 2019-07-22 21:26 UTC (permalink / raw)
  To: Joe Perches, Jiri Kosina, Benjamin Tissoires,
	open list:HID CORE LAYER, open list
  Cc: Joshua Clayton
In-Reply-To: <20190722163642.10417-2-stillcompiling@gmail.com>

From: Joshua Clayton <stillcompiling@gmail.com>

Reformat hid_printk macros to use standard __VA_ARGS__ syntax
Remove hid_printk(), hid_emerg(), hid_crit(), and hid_alert().
Per Joe Perches these unused and likely never to be used.

Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
 include/linux/hid.h | 32 ++++++++++++--------------------
 1 file changed, 12 insertions(+), 20 deletions(-)

diff --git a/include/linux/hid.h b/include/linux/hid.h
index d770ab1a0479..b5e73331100e 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -1154,29 +1154,21 @@ int hid_pidff_init(struct hid_device *hid);
 #define hid_pidff_init NULL
 #endif
 
-#define dbg_hid(format, arg...)						\
+#define dbg_hid(fmt, ...)						\
 do {									\
 	if (hid_debug)							\
-		printk(KERN_DEBUG "%s: " format, __FILE__, ##arg);	\
+		printk(KERN_DEBUG "%s: " fmt, __FILE__, ##__VA_ARGS__)	\
 } while (0)
 
-#define hid_printk(level, hid, fmt, arg...)		\
-	dev_printk(level, &(hid)->dev, fmt, ##arg)
-#define hid_emerg(hid, fmt, arg...)			\
-	dev_emerg(&(hid)->dev, fmt, ##arg)
-#define hid_crit(hid, fmt, arg...)			\
-	dev_crit(&(hid)->dev, fmt, ##arg)
-#define hid_alert(hid, fmt, arg...)			\
-	dev_alert(&(hid)->dev, fmt, ##arg)
-#define hid_err(hid, fmt, arg...)			\
-	dev_err(&(hid)->dev, fmt, ##arg)
-#define hid_notice(hid, fmt, arg...)			\
-	dev_notice(&(hid)->dev, fmt, ##arg)
-#define hid_warn(hid, fmt, arg...)			\
-	dev_warn(&(hid)->dev, fmt, ##arg)
-#define hid_info(hid, fmt, arg...)			\
-	dev_info(&(hid)->dev, fmt, ##arg)
-#define hid_dbg(hid, fmt, arg...)			\
-	dev_dbg(&(hid)->dev, fmt, ##arg)
+#define hid_err(hid, fmt, ...)				\
+	dev_err(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_notice(hid, fmt, ...)			\
+	dev_notice(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_warn(hid, fmt, ...)				\
+	dev_warn(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_info(hid, fmt, ...)				\
+	dev_info(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_dbg(hid, fmt, ...)				\
+	dev_dbg(&(hid)->dev, fmt, ##__VA_ARGS__)
 
 #endif
-- 
2.21.0

^ permalink raw reply related

* Re: [PATCH 2/2] HID: core: only warn once of oversize hid report
From: Joshua Clayton @ 2019-07-22 18:16 UTC (permalink / raw)
  To: Joe Perches
  Cc: Jiri Kosina, Benjamin Tissoires, open list:HID CORE LAYER,
	open list
In-Reply-To: <f7b1a46569a7208eb2be3b5cc3432b0aa97fc3ee.camel@perches.com>

On Mon, Jul 22, 2019 at 11:23 AM Joe Perches <joe@perches.com> wrote:
>
> On Mon, 2019-07-22 at 10:36 -0600, stillcompiling@gmail.com wrote:
> > On HP spectre x360 convertible the message:
> > hid-sensor-hub 001F:8087:0AC2.0002: hid_field_extract() called with n (192) > 32! (kworker/1:2)
> > is continually printed many times per second, crowding out all other kernel logs
> > Protect dmesg by printing the warning only one time.
> []
> > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> []
> > @@ -1311,7 +1311,7 @@ u32 hid_field_extract(const struct hid_device *hid, u8 *report,
> >                       unsigned offset, unsigned n)
> >  {
> >       if (n > 32) {
> > -             hid_warn(hid, "hid_field_extract() called with n (%d) > 32! (%s)\n",
> > +             hid_warn_once(hid, "hid_field_extract() called with n (%d) > 32! (%s)\n",
> >                        n, current->comm);
> >               n = 32;
> >       }
>
> Is this papering over an actual defect somewhere else?

Sort of.
It doesn't correct the underlying issue, but I think this should go in
even along with the real fix.
The dmesg spamming has become a more serious problem for me than the
underlying issue.
Someone had a patch rejected that completely suppressed the message.

>From my limited understanding, the hid spec allows an unlimited size
for an hid report , but the kernel only allocated 32 bits, which was
more than anything used at that time. The 32 bit version is doing some
bit shifting and possibly endian correction with the 32 bit field, so
I was not comfortable just extending it to 192 or 256 bits without a
little more understanding.

> Trivially, this could use "%s: ...", __func__, ...
True. I can make that change.
>

^ permalink raw reply

* [PATCH RESEND] HID: fix error message in hid_open_report()
From: Michał Mirosław @ 2019-07-22 18:02 UTC (permalink / raw)
  To: linux-input; +Cc: Jiri Kosina, Benjamin Tissoires, linux-kernel

On HID report descriptor parsing error the code displays bogus
pointer instead of error offset (subtracts start=NULL from end).
Make the message more useful by displaying correct error offset
and include total buffer size for reference.

This was carried over from ancient times - "Fixed" commit just
promoted the message from DEBUG to ERROR.

Cc: stable@vger.kernel.org
Fixes: 8c3d52fc393b ("HID: make parser more verbose about parsing errors by default")
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

---
 * against v5.2.2

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/hid/hid-core.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 210b81a56e1a..bb74b7ff4e6a 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1139,6 +1139,7 @@ int hid_open_report(struct hid_device *device)
 	__u8 *start;
 	__u8 *buf;
 	__u8 *end;
+	__u8 *next;
 	int ret;
 	static int (*dispatch_type[])(struct hid_parser *parser,
 				      struct hid_item *item) = {
@@ -1192,7 +1193,8 @@ int hid_open_report(struct hid_device *device)
 	device->collection_size = HID_DEFAULT_NUM_COLLECTIONS;
 
 	ret = -EINVAL;
-	while ((start = fetch_item(start, end, &item)) != NULL) {
+	while ((next = fetch_item(start, end, &item)) != NULL) {
+		start = next;
 
 		if (item.format != HID_ITEM_FORMAT_SHORT) {
 			hid_err(device, "unexpected long global item\n");
@@ -1230,7 +1232,8 @@ int hid_open_report(struct hid_device *device)
 		}
 	}
 
-	hid_err(device, "item fetching failed at offset %d\n", (int)(end - start));
+	hid_err(device, "item fetching failed at offset %zu/%zu\n",
+		size - (end - start), size);
 err:
 	kfree(parser->collection_stack);
 alloc_err:
-- 
2.20.1

^ permalink raw reply related

* Re: [PATCH 2/2] HID: core: only warn once of oversize hid report
From: Joe Perches @ 2019-07-22 17:23 UTC (permalink / raw)
  To: stillcompiling, Jiri Kosina, Benjamin Tissoires,
	open list:HID CORE LAYER, open list
In-Reply-To: <20190722163642.10417-2-stillcompiling@gmail.com>

On Mon, 2019-07-22 at 10:36 -0600, stillcompiling@gmail.com wrote:
> On HP spectre x360 convertible the message:
> hid-sensor-hub 001F:8087:0AC2.0002: hid_field_extract() called with n (192) > 32! (kworker/1:2)
> is continually printed many times per second, crowding out all other kernel logs
> Protect dmesg by printing the warning only one time.
[]
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
[]
> @@ -1311,7 +1311,7 @@ u32 hid_field_extract(const struct hid_device *hid, u8 *report,
>  			unsigned offset, unsigned n)
>  {
>  	if (n > 32) {
> -		hid_warn(hid, "hid_field_extract() called with n (%d) > 32! (%s)\n",
> +		hid_warn_once(hid, "hid_field_extract() called with n (%d) > 32! (%s)\n",
>  			 n, current->comm);
>  		n = 32;
>  	}

Is this papering over an actual defect somewhere else?
Trivially, this could use "%s: ...", __func__, ...

^ permalink raw reply

* Re: [PATCH 1/2] HID: core: Add hid printk_once macros
From: Joe Perches @ 2019-07-22 17:11 UTC (permalink / raw)
  To: stillcompiling, Jiri Kosina, Benjamin Tissoires,
	open list:HID CORE LAYER, open list
In-Reply-To: <20190722163642.10417-1-stillcompiling@gmail.com>

On Mon, 2019-07-22 at 10:36 -0600, stillcompiling@gmail.com wrote:
> From: Joshua Clayton <stillcompiling@gmail.com>
> 
> Make available printk_once variants to hid_warn() etc
> 
> Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>

This seems OK, but I suggest a slightly different style:

> diff --git a/include/linux/hid.h b/include/linux/hid.h
[]
> @@ -1179,4 +1179,23 @@ do {									\
>  #define hid_dbg(hid, fmt, arg...)			\
>  	dev_dbg(&(hid)->dev, fmt, ##arg)
>  
> +#define hid_level_once(level, hid, fmt, arg...)		\
> +	dev_level_once(level, &(hid)->dev, fmt, ##arg)

This one is probably not useful in actual code.

> +#define hid_emerg_once(hid, fmt, arg...)		\
> +	dev_emerg_once(&(hid)->dev, fmt, ##arg)

Even though I introduced those macros originally,
it's now a more common style to use:

#define hid_emerg_once(hid, fmt, ...)				\
	dev_emerg_once(&(hid)->dev, fmt, ##__VA_ARGS__)

etc...

And trivially:

hid_printk, hid_emerg, hid_crit, and hid_alert aren't
used at all and could all be removed.

I'm not sure there is a use case for any of them.

Perhaps:
---
 include/linux/hid.h | 39 +++++++++++++++++++++------------------
 1 file changed, 21 insertions(+), 18 deletions(-)

diff --git a/include/linux/hid.h b/include/linux/hid.h
index d770ab1a0479..5d2c4b63954f 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -1160,23 +1160,26 @@ do {									\
 		printk(KERN_DEBUG "%s: " format, __FILE__, ##arg);	\
 } while (0)
 
-#define hid_printk(level, hid, fmt, arg...)		\
-	dev_printk(level, &(hid)->dev, fmt, ##arg)
-#define hid_emerg(hid, fmt, arg...)			\
-	dev_emerg(&(hid)->dev, fmt, ##arg)
-#define hid_crit(hid, fmt, arg...)			\
-	dev_crit(&(hid)->dev, fmt, ##arg)
-#define hid_alert(hid, fmt, arg...)			\
-	dev_alert(&(hid)->dev, fmt, ##arg)
-#define hid_err(hid, fmt, arg...)			\
-	dev_err(&(hid)->dev, fmt, ##arg)
-#define hid_notice(hid, fmt, arg...)			\
-	dev_notice(&(hid)->dev, fmt, ##arg)
-#define hid_warn(hid, fmt, arg...)			\
-	dev_warn(&(hid)->dev, fmt, ##arg)
-#define hid_info(hid, fmt, arg...)			\
-	dev_info(&(hid)->dev, fmt, ##arg)
-#define hid_dbg(hid, fmt, arg...)			\
-	dev_dbg(&(hid)->dev, fmt, ##arg)
+#define hid_err(hid, fmt, ...)						\
+	dev_err(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_notice(hid, fmt, ...)					\
+	dev_notice(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_warn(hid, fmt, ...)						\
+	dev_warn(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_info(hid, fmt, ...)						\
+	dev_info(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_dbg(hid, fmt, ...)						\
+	dev_dbg(&(hid)->dev, fmt, ##__VA_ARGS__)
+
+#define hid_err_once(hid, fmt, ...)					\
+	dev_err_once(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_notice_once(hid, fmt, ...)					\
+	dev_notice_once(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_warn_once(hid, fmt, ...)					\
+	dev_warn_once(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_info_once(hid, fmt, ...)					\
+	dev_info_once(&(hid)->dev, fmt, ##__VA_ARGS__)
+#define hid_dbg_once(hid, fmt, ...)					\
+	dev_dbg_once(&(hid)->dev, fmt, ##__VA_ARGS__)
 
 #endif

^ permalink raw reply related

* [PATCH 2/2] HID: core: only warn once of oversize hid report
From: stillcompiling @ 2019-07-22 16:36 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, open list:HID CORE LAYER,
	open list
  Cc: Joshua Clayton
In-Reply-To: <20190722163642.10417-1-stillcompiling@gmail.com>

From: Joshua Clayton <stillcompiling@gmail.com>

On HP spectre x360 convertible the message:
hid-sensor-hub 001F:8087:0AC2.0002: hid_field_extract() called with n (192) > 32! (kworker/1:2)
is continually printed many times per second, crowding out all other kernel logs
Protect dmesg by printing the warning only one time.

Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
 drivers/hid/hid-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 210b81a56e1a..7afd0422b280 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1311,7 +1311,7 @@ u32 hid_field_extract(const struct hid_device *hid, u8 *report,
 			unsigned offset, unsigned n)
 {
 	if (n > 32) {
-		hid_warn(hid, "hid_field_extract() called with n (%d) > 32! (%s)\n",
+		hid_warn_once(hid, "hid_field_extract() called with n (%d) > 32! (%s)\n",
 			 n, current->comm);
 		n = 32;
 	}
-- 
2.21.0

^ permalink raw reply related

* [PATCH 1/2] HID: core: Add hid printk_once macros
From: stillcompiling @ 2019-07-22 16:36 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, open list:HID CORE LAYER,
	open list
  Cc: Joshua Clayton

From: Joshua Clayton <stillcompiling@gmail.com>

Make available printk_once variants to hid_warn() etc

Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
 include/linux/hid.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/include/linux/hid.h b/include/linux/hid.h
index d770ab1a0479..5b239712c902 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -1179,4 +1179,23 @@ do {									\
 #define hid_dbg(hid, fmt, arg...)			\
 	dev_dbg(&(hid)->dev, fmt, ##arg)
 
+#define hid_level_once(level, hid, fmt, arg...)		\
+	dev_level_once(level, &(hid)->dev, fmt, ##arg)
+#define hid_emerg_once(hid, fmt, arg...)		\
+	dev_emerg_once(&(hid)->dev, fmt, ##arg)
+#define hid_crit_once(hid, fmt, arg...)			\
+	dev_crit_once(&(hid)->dev, fmt, ##arg)
+#define hid_alert_once(hid, fmt, arg...)		\
+	dev_alert_once(&(hid)->dev, fmt, ##arg)
+#define hid_err_once(hid, fmt, arg...)			\
+	dev_err_once(&(hid)->dev, fmt, ##arg)
+#define hid_notice_once(hid, fmt, arg...)		\
+	dev_notice_once(&(hid)->dev, fmt, ##arg)
+#define hid_warn_once(hid, fmt, arg...)			\
+	dev_warn_once(&(hid)->dev, fmt, ##arg)
+#define hid_info_once(hid, fmt, arg...)			\
+	dev_info_once(&(hid)->dev, fmt, ##arg)
+#define hid_dbg_once(hid, fmt, arg...)			\
+	dev_dbg_once(&(hid)->dev, fmt, ##arg)
+
 #endif
-- 
2.21.0

^ permalink raw reply related

* Re: [PATCH v3] Input: elantech: Enable SMBus on new (2018+) systems
From: Dmitry Torokhov @ 2019-07-22  8:17 UTC (permalink / raw)
  To: Kai-Heng Feng; +Cc: benjamin.tissoires, linux-input, linux-kernel, Your Name
In-Reply-To: <20190722074055.22427-1-kai.heng.feng@canonical.com>

Hi Kai-Heng,

On Mon, Jul 22, 2019 at 03:40:55PM +0800, Kai-Heng Feng wrote:
> There are some new HP laptops with Elantech touchpad don't support
> multitouch.
> 
> Currently we use ETP_NEW_IC_SMBUS_HOST_NOTIFY() to check if SMBus is
> supported, but in addition to firmware version, the bus type also
> informs us if the IC can support SMBus, so also check that.
> 
> In case of breaking old ICs, only enables SMBus on systems manufactured
> after 2018, alongsides aforementioned checks.
> 
> Lastly, consolidats all check into elantech_use_host_notify() and use it
> to determine whether to use PS/2 or SMBus.
> 
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Signed-off-by: Your Name <you@example.com>

I do not think "Your Name" should be signing DCO here :)

> +static bool elantech_use_host_notify(struct psmouse *psmouse,
> +				     struct elantech_device_info *info)
> +{
> +	bool host_notify = false;
> +
> +	if (ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version))
> +		host_notify = true;
> +	else {
> +		switch (info->bus) {
> +		case ETP_BUS_PS2_ONLY:
> +			/* expected case */
> +			break;
> +		case ETP_BUS_SMB_ALERT_ONLY:
> +			/* fall-through  */
> +		case ETP_BUS_PS2_SMB_ALERT:
> +			psmouse_dbg(psmouse, "Ignoring SMBus provider through alert protocol.\n");
> +			break;
> +		case ETP_BUS_SMB_HST_NTFY_ONLY:
> +			/* fall-through  */
> +		case ETP_BUS_PS2_SMB_HST_NTFY:
> +			/* SMbus implementation is stable since 2018 */
> +			if (dmi_get_bios_year() >= 2018)
> +				host_notify = true;
> +			break;
> +		default:
> +			psmouse_dbg(psmouse,
> +				    "Ignoring SMBus bus provider %d.\n",
> +				    info->bus);
> +		}
> +	}

I think this is way too verbose. How about a bit more condensed form:


Input: elantech - enable SMBus on new (2018+) systems

From: Kai-Heng Feng <kai.heng.feng@canonical.com>

There are some new HP laptops with Elantech touchpad that don't support
multitouch.

Currently we use ETP_NEW_IC_SMBUS_HOST_NOTIFY() to check if SMBus is supported,
but in addition to firmware version, the bus type also informs us whether the IC
can support SMBus. To avoid breaking old ICs, we will only enable SMbus support
based the bus type on systems manufactured after 2018.

Lastly, let's consolidate all checks into elantech_use_host_notify() and use it
to determine whether to use PS/2 or SMBus.

Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: stable@vger.kernel.org
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/mouse/elantech.c |   54 +++++++++++++++++++---------------------
 1 file changed, 25 insertions(+), 29 deletions(-)

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 2d8434b7b623..73544776a9ed 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -1827,6 +1827,30 @@ static int elantech_create_smbus(struct psmouse *psmouse,
 				  leave_breadcrumbs);
 }
 
+static bool elantech_use_host_notify(struct psmouse *psmouse,
+				     struct elantech_device_info *info)
+{
+	if (ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version))
+		return true;
+
+	switch (info->bus) {
+	case ETP_BUS_PS2_ONLY:
+		/* expected case */
+		break;
+	case ETP_BUS_SMB_HST_NTFY_ONLY:
+	case ETP_BUS_PS2_SMB_HST_NTFY:
+		/* SMbus implementation is stable since 2018 */
+		if (dmi_get_bios_year() >= 2018)
+			return true;
+	default:
+		psmouse_dbg(psmouse,
+			    "Ignoring SMBus bus provider %d\n", info->bus);
+		break;
+	}
+
+	return false;
+}
+
 /**
  * elantech_setup_smbus - called once the PS/2 devices are enumerated
  * and decides to instantiate a SMBus InterTouch device.
@@ -1846,7 +1870,7 @@ static int elantech_setup_smbus(struct psmouse *psmouse,
 		 * i2c_blacklist_pnp_ids.
 		 * Old ICs are up to the user to decide.
 		 */
-		if (!ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version) ||
+		if (!elantech_use_host_notify(psmouse, info) ||
 		    psmouse_matches_pnp_id(psmouse, i2c_blacklist_pnp_ids))
 			return -ENXIO;
 	}
@@ -1866,34 +1890,6 @@ static int elantech_setup_smbus(struct psmouse *psmouse,
 	return 0;
 }
 
-static bool elantech_use_host_notify(struct psmouse *psmouse,
-				     struct elantech_device_info *info)
-{
-	if (ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version))
-		return true;
-
-	switch (info->bus) {
-	case ETP_BUS_PS2_ONLY:
-		/* expected case */
-		break;
-	case ETP_BUS_SMB_ALERT_ONLY:
-		/* fall-through  */
-	case ETP_BUS_PS2_SMB_ALERT:
-		psmouse_dbg(psmouse, "Ignoring SMBus provider through alert protocol.\n");
-		break;
-	case ETP_BUS_SMB_HST_NTFY_ONLY:
-		/* fall-through  */
-	case ETP_BUS_PS2_SMB_HST_NTFY:
-		return true;
-	default:
-		psmouse_dbg(psmouse,
-			    "Ignoring SMBus bus provider %d.\n",
-			    info->bus);
-	}
-
-	return false;
-}
-
 int elantech_init_smbus(struct psmouse *psmouse)
 {
 	struct elantech_device_info info;

Thanks.

-- 
Dmitry

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox