* Re: [PATCH v2 2/2] Input: add Apple SPI keyboard and trackpad driver.
From: Andy Shevchenko @ 2019-02-26 8:51 UTC (permalink / raw)
To: Life is hard, and then you die
Cc: Dmitry Torokhov, Henrik Rydberg, Lukas Wunner, Federico Lorenzi,
linux-input, linux-kernel
In-Reply-To: <20190225080529.GA26142@innovation.ch>
On Mon, Feb 25, 2019 at 12:05:29AM -0800, Life is hard, and then you die wrote:
>
> On Thu, Feb 21, 2019 at 02:56:09AM -0800, Ronald Tschalär wrote:
> > The keyboard and trackpad on recent MacBook's (since 8,1) and
> > MacBookPro's (13,* and 14,*) are attached to an SPI controller instead
> > of USB, as previously. The higher level protocol is not publicly
> > documented and hence has been reverse engineered. As a consequence there
> > are still a number of unknown fields and commands. However, the known
> > parts have been working well and received extensive testing and use.
> >
> > In order for this driver to work, the proper SPI drivers need to be
> > loaded too; for MB8,1 these are spi_pxa2xx_platform and spi_pxa2xx_pci;
> > for all others they are spi_pxa2xx_platform and intel_lpss_pci. For this
> > reason enabling this driver in the config implies enabling the above
> > drivers.
> > +/**
> > + * This is a reduced version of print_hex_dump() that uses dev_printk().
> > + */
...and this should follow kernel doc as stated by comment style.
> > +static void dev_print_hex_dump(const char *level, const struct device *dev,
> > + const char *prefix_str,
> > + int rowsize, int groupsize,
> > + const void *buf, size_t len, bool ascii)
> > +{
> > + const u8 *ptr = buf;
> > + int i, linelen, remaining = len;
> > + unsigned char linebuf[32 * 3 + 2 + 32 + 1];
> > +
> > + if (rowsize != 16 && rowsize != 32)
> > + rowsize = 16;
> > +
> > + for (i = 0; i < len; i += rowsize) {
> > + linelen = min(remaining, rowsize);
> > + remaining -= rowsize;
> > +
> > + hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize,
> > + linebuf, sizeof(linebuf), ascii);
> > +
> > + dev_printk(level, dev, "%s%s\n", prefix_str, linebuf);
> > + }
> > +}
>
> Apologies, I should've have fixed this before posting v2: I'll
> introduce an additional patch to add this function to the core to
> avoid duplication and because I presume this may be useful for others
> too.
Yes, makes sense.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 2/2] Input: add Apple SPI keyboard and trackpad driver.
From: Andy Shevchenko @ 2019-02-26 9:20 UTC (permalink / raw)
To: Ronald Tschalär
Cc: Dmitry Torokhov, Henrik Rydberg, Lukas Wunner, Federico Lorenzi,
linux-input, linux-kernel
In-Reply-To: <20190221105609.5710-3-ronald@innovation.ch>
On Thu, Feb 21, 2019 at 02:56:09AM -0800, Ronald Tschalär wrote:
> The keyboard and trackpad on recent MacBook's (since 8,1) and
> MacBookPro's (13,* and 14,*) are attached to an SPI controller instead
> of USB, as previously. The higher level protocol is not publicly
> documented and hence has been reverse engineered. As a consequence there
> are still a number of unknown fields and commands. However, the known
> parts have been working well and received extensive testing and use.
>
> In order for this driver to work, the proper SPI drivers need to be
> loaded too; for MB8,1 these are spi_pxa2xx_platform and spi_pxa2xx_pci;
> for all others they are spi_pxa2xx_platform and intel_lpss_pci. For this
> reason enabling this driver in the config implies enabling the above
> drivers.
> +config KEYBOARD_APPLESPI
> + tristate "Apple SPI keyboard and trackpad"
> + depends on ACPI && SPI && EFI
I would rather want to see separate line for SPI...
> + depends on X86 || COMPILE_TEST
...like here
depends on SPI
> + imply SPI_PXA2XX
> + imply SPI_PXA2XX_PCI
> + imply MFD_INTEL_LPSS_PCI
> + help
> + Say Y here if you are running Linux on any Apple MacBook8,1 or later,
> + or any MacBookPro13,* or MacBookPro14,*.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called applespi.
/* Perhaps a comment here that this mimics struct drm_rect */
> +struct applespi_tp_info {
> + int x_min;
> + int y_min;
> + int x_max;
> + int y_max;
> +};
...see above
> +static inline bool applespi_check_write_status(struct applespi_data *applespi,
> + int sts)
> +{
> + static u8 status_ok[] = { 0xac, 0x27, 0x68, 0xd5 };
> +
> + if (sts < 0) {
> + dev_warn(DEV(applespi), "Error writing to device: %d\n", sts);
> + return false;
> + }
> +
> + if (memcmp(applespi->tx_status, status_ok, APPLESPI_STATUS_SIZE)) {
> + dev_warn(DEV(applespi), "Error writing to device: %*ph\n",
Hmm... DEV() is too generic name for custom macro. And frankly I don't think
it's good to have in the first place.
> + APPLESPI_STATUS_SIZE, applespi->tx_status);
> + return false;
> + }
> +
> + return true;
> +}
> +static void applespi_set_bl_level(struct led_classdev *led_cdev,
> + enum led_brightness value)
> +{
> + struct applespi_data *applespi =
> + container_of(led_cdev, struct applespi_data, backlight_info);
> + unsigned long flags;
> + int sts;
> +
> + spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
> +
> + if (value == 0)
> + applespi->want_bl_level = value;
> + else
> + /*
> + * The backlight does not turn on till level 32, so we scale
> + * the range here so that from a user's perspective it turns
> + * on at 1.
> + */
> + applespi->want_bl_level =
> + ((value * KBD_BL_LEVEL_ADJ) / KBD_BL_LEVEL_SCALE +
> + KBD_BL_LEVEL_MIN);
Fir multi-line conditionals the style is
if () {
} else {
}
> +
> + sts = applespi_send_cmd_msg(applespi);
> +
> + spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
> +}
> +static void
> +applespi_remap_fn_key(struct keyboard_protocol *keyboard_protocol)
> +{
> + unsigned char tmp;
> + unsigned long *modifiers =
> + (unsigned long *)&keyboard_protocol->modifiers;
I would leave it on one online despite checkpatch warning (also, instead of
(unsigned long *) the (void *) might be used as a small trick).
> +
> + if (!fnremap || fnremap > ARRAY_SIZE(applespi_controlcodes) ||
> + !applespi_controlcodes[fnremap - 1])
> + return;
> +
> + tmp = keyboard_protocol->fn_pressed;
> + keyboard_protocol->fn_pressed = test_bit(fnremap - 1, modifiers);
> + if (tmp)
> + __set_bit(fnremap - 1, modifiers);
> + else
> + __clear_bit(fnremap - 1, modifiers);
Oh, this is not good. modifiers should be really unsigned long bounary,
otherwise it is potential overflow.
Best to fix is to define them as unsigned long in the first place.
> +}
> + applespi->last_keys_fn_pressed[i]);
> + input_report_key(applespi->keyboard_input_dev, key, 0);
> + applespi->last_keys_fn_pressed[i] = 0;
> + }
> + for (i = 0; i < MAX_MODIFIERS; i++) {
> + u8 *modifiers = &keyboard_protocol->modifiers;
> +
> + if (test_bit(i, (unsigned long *)modifiers))
Oh, this is not good idea, see above.
> + if (touchpad_dimensions[0]) {
> + int x, y, w, h;
> +
> + if (sscanf(touchpad_dimensions, "%dx%d+%u+%u", &x, &y, &w, &h)
> + == 4) {
I would leave this on one line. Or use temporary variable
int ret;
ret = sscanf();
if (ret ...) {
> + dev_info(DEV(applespi),
> + "Overriding touchpad dimensions from module param\n");
> + applespi->tp_info.x_min = x;
> + applespi->tp_info.y_min = y;
> + applespi->tp_info.x_max = x + w;
> + applespi->tp_info.y_max = y + h;
> + } else {
> + dev_warn(DEV(applespi),
> + "Invalid touchpad dimensions '%s': must be in the form XxY+W+H\n",
> + touchpad_dimensions);
> + touchpad_dimensions[0] = '\0';
> + }
> + }
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH] HID: quirks: use correct format chars in dbg_hid
From: Louis Taylor @ 2019-02-26 23:48 UTC (permalink / raw)
To: jikos
Cc: benjamin.tissoires, linux-input, linux-kernel, clang-built-linux,
ndesaulniers, Louis Taylor
When building with -Wformat, clang warns:
drivers/hid/hid-quirks.c:1075:27: warning: format specifies type
'unsigned short' but the argument has type '__u32' (aka 'unsigned int')
[-Wformat]
bl_entry->driver_data, bl_entry->vendor,
^~~~~~~~~~~~~~~~
./include/linux/hid.h:1170:48: note: expanded from macro 'dbg_hid'
printk(KERN_DEBUG "%s: " format, __FILE__, ##arg); \
~~~~~~ ^~~
drivers/hid/hid-quirks.c:1076:4: warning: format specifies type
'unsigned short' but the argument has type '__u32' (aka 'unsigned int')
[-Wformat]
bl_entry->product);
^~~~~~~~~~~~~~~~~
./include/linux/hid.h:1170:48: note: expanded from macro 'dbg_hid'
printk(KERN_DEBUG "%s: " format, __FILE__, ##arg); \
~~~~~~ ^~~
drivers/hid/hid-quirks.c:1242:12: warning: format specifies type
'unsigned short' but the argument has type '__u32' (aka 'unsigned int')
[-Wformat]
quirks, hdev->vendor, hdev->product);
^~~~~~~~~~~~
./include/linux/hid.h:1170:48: note: expanded from macro 'dbg_hid'
printk(KERN_DEBUG "%s: " format, __FILE__, ##arg); \
~~~~~~ ^~~
drivers/hid/hid-quirks.c:1242:26: warning: format specifies type
'unsigned short' but the argument has type '__u32' (aka 'unsigned int')
[-Wformat]
quirks, hdev->vendor, hdev->product);
^~~~~~~~~~~~~
./include/linux/hid.h:1170:48: note: expanded from macro 'dbg_hid'
printk(KERN_DEBUG "%s: " format, __FILE__, ##arg); \
~~~~~~ ^~~
4 warnings generated.
This patch fixes the format strings to use the correct format type for unsigned
ints.
Link: https://github.com/ClangBuiltLinux/linux/issues/378
Signed-off-by: Louis Taylor <louis@kragniz.eu>
---
drivers/hid/hid-quirks.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 94088c0ed68a..b4e49e1b6f4a 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -1071,7 +1071,7 @@ static struct hid_device_id *hid_exists_dquirk(const struct hid_device *hdev)
}
if (bl_entry != NULL)
- dbg_hid("Found dynamic quirk 0x%lx for HID device 0x%hx:0x%hx\n",
+ dbg_hid("Found dynamic quirk 0x%lx for HID device 0x%x:0x%x\n",
bl_entry->driver_data, bl_entry->vendor,
bl_entry->product);
@@ -1238,7 +1238,7 @@ static unsigned long hid_gets_squirk(const struct hid_device *hdev)
quirks |= bl_entry->driver_data;
if (quirks)
- dbg_hid("Found squirk 0x%lx for HID device 0x%hx:0x%hx\n",
+ dbg_hid("Found squirk 0x%lx for HID device 0x%x:0x%x\n",
quirks, hdev->vendor, hdev->product);
return quirks;
}
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] HID: quirks: use correct format chars in dbg_hid
From: Nick Desaulniers @ 2019-02-27 0:27 UTC (permalink / raw)
To: Louis Taylor
Cc: Jiri Kosina, benjamin.tissoires, linux-input, LKML,
clang-built-linux, Jon Flatley, Matthias Männich
In-Reply-To: <20190226234853.20441-1-louis@kragniz.eu>
On Tue, Feb 26, 2019 at 3:50 PM Louis Taylor <louis@kragniz.eu> wrote:
>
> When building with -Wformat, clang warns:
>
> drivers/hid/hid-quirks.c:1075:27: warning: format specifies type
> 'unsigned short' but the argument has type '__u32' (aka 'unsigned int')
> [-Wformat]
> bl_entry->driver_data, bl_entry->vendor,
> ^~~~~~~~~~~~~~~~
> ./include/linux/hid.h:1170:48: note: expanded from macro 'dbg_hid'
> printk(KERN_DEBUG "%s: " format, __FILE__, ##arg); \
> ~~~~~~ ^~~
> drivers/hid/hid-quirks.c:1076:4: warning: format specifies type
> 'unsigned short' but the argument has type '__u32' (aka 'unsigned int')
> [-Wformat]
> bl_entry->product);
> ^~~~~~~~~~~~~~~~~
> ./include/linux/hid.h:1170:48: note: expanded from macro 'dbg_hid'
> printk(KERN_DEBUG "%s: " format, __FILE__, ##arg); \
> ~~~~~~ ^~~
> drivers/hid/hid-quirks.c:1242:12: warning: format specifies type
> 'unsigned short' but the argument has type '__u32' (aka 'unsigned int')
> [-Wformat]
> quirks, hdev->vendor, hdev->product);
> ^~~~~~~~~~~~
> ./include/linux/hid.h:1170:48: note: expanded from macro 'dbg_hid'
> printk(KERN_DEBUG "%s: " format, __FILE__, ##arg); \
> ~~~~~~ ^~~
> drivers/hid/hid-quirks.c:1242:26: warning: format specifies type
> 'unsigned short' but the argument has type '__u32' (aka 'unsigned int')
> [-Wformat]
> quirks, hdev->vendor, hdev->product);
> ^~~~~~~~~~~~~
> ./include/linux/hid.h:1170:48: note: expanded from macro 'dbg_hid'
> printk(KERN_DEBUG "%s: " format, __FILE__, ##arg); \
> ~~~~~~ ^~~
> 4 warnings generated.
>
> This patch fixes the format strings to use the correct format type for unsigned
> ints.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/378
> Signed-off-by: Louis Taylor <louis@kragniz.eu>
> ---
> drivers/hid/hid-quirks.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index 94088c0ed68a..b4e49e1b6f4a 100644
> --- a/drivers/hid/hid-quirks.c
> +++ b/drivers/hid/hid-quirks.c
> @@ -1071,7 +1071,7 @@ static struct hid_device_id *hid_exists_dquirk(const struct hid_device *hdev)
> }
>
> if (bl_entry != NULL)
> - dbg_hid("Found dynamic quirk 0x%lx for HID device 0x%hx:0x%hx\n",
> + dbg_hid("Found dynamic quirk 0x%lx for HID device 0x%x:0x%x\n",
%h is for short ints, include/linux/mod_devicetable.h declares struct
hid_device_id (bl_entry is an instance of struct hid_device_id)
unconditionally as:
147 struct hid_device_id {
...
150 __u32 vendor;
151 __u32 product;
...
153 };
yep; LGTM
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Thank you for the patch! There's 3 more of these in
drivers/hid/i2c-hid/i2c-hid-core.c if your looking to clean up some
more!
> bl_entry->driver_data, bl_entry->vendor,
> bl_entry->product);
>
> @@ -1238,7 +1238,7 @@ static unsigned long hid_gets_squirk(const struct hid_device *hdev)
> quirks |= bl_entry->driver_data;
>
> if (quirks)
> - dbg_hid("Found squirk 0x%lx for HID device 0x%hx:0x%hx\n",
> + dbg_hid("Found squirk 0x%lx for HID device 0x%x:0x%x\n",
> quirks, hdev->vendor, hdev->product);
> return quirks;
> }
> --
> 2.20.1
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply
* Re: [PATCH v2 2/2] Input: add Apple SPI keyboard and trackpad driver.
From: Life is hard, and then you die @ 2019-02-27 7:29 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Dmitry Torokhov, Henrik Rydberg, Lukas Wunner, Federico Lorenzi,
linux-input, linux-kernel
In-Reply-To: <20190226092059.GV9224@smile.fi.intel.com>
On Tue, Feb 26, 2019 at 11:20:59AM +0200, Andy Shevchenko wrote:
> On Thu, Feb 21, 2019 at 02:56:09AM -0800, Ronald Tschalär wrote:
> > The keyboard and trackpad on recent MacBook's (since 8,1) and
> > MacBookPro's (13,* and 14,*) are attached to an SPI controller instead
> > of USB, as previously. The higher level protocol is not publicly
> > documented and hence has been reverse engineered. As a consequence there
> > are still a number of unknown fields and commands. However, the known
> > parts have been working well and received extensive testing and use.
> >
> > In order for this driver to work, the proper SPI drivers need to be
> > loaded too; for MB8,1 these are spi_pxa2xx_platform and spi_pxa2xx_pci;
> > for all others they are spi_pxa2xx_platform and intel_lpss_pci. For this
> > reason enabling this driver in the config implies enabling the above
> > drivers.
>
> > +config KEYBOARD_APPLESPI
> > + tristate "Apple SPI keyboard and trackpad"
>
> > + depends on ACPI && SPI && EFI
>
> I would rather want to see separate line for SPI...
>
> > + depends on X86 || COMPILE_TEST
>
> ...like here
>
> depends on SPI
Sure. Generally, what is the criteria/rule here for splitting
conjunctions into separate 'depends'?
[snip]
> + #define DEV(applespi) (&(applespi)->spi->dev)
[snip]
> > + if (memcmp(applespi->tx_status, status_ok, APPLESPI_STATUS_SIZE)) {
>
> > + dev_warn(DEV(applespi), "Error writing to device: %*ph\n",
>
> Hmm... DEV() is too generic name for custom macro. And frankly I don't think
> it's good to have in the first place.
Yeah, I've been having trouble coming up with a better (but still
succinct) name - CORE_DEV()? RAW_DEV()? DEV_OF()? However, because
this expression is used in many places throughout the driver (mostly,
but not only, for logging statements) I feel like it's good to factor
it out. But I'll defer to your .
[snip]
> > +static void
> > +applespi_remap_fn_key(struct keyboard_protocol *keyboard_protocol)
> > +{
> > + unsigned char tmp;
>
> > + unsigned long *modifiers =
> > + (unsigned long *)&keyboard_protocol->modifiers;
>
> I would leave it on one online despite checkpatch warning (also, instead of
> (unsigned long *) the (void *) might be used as a small trick).
>
> > +
> > + if (!fnremap || fnremap > ARRAY_SIZE(applespi_controlcodes) ||
> > + !applespi_controlcodes[fnremap - 1])
> > + return;
> > +
> > + tmp = keyboard_protocol->fn_pressed;
> > + keyboard_protocol->fn_pressed = test_bit(fnremap - 1, modifiers);
> > + if (tmp)
>
> > + __set_bit(fnremap - 1, modifiers);
> > + else
> > + __clear_bit(fnremap - 1, modifiers);
>
> Oh, this is not good. modifiers should be really unsigned long bounary,
> otherwise it is potential overflow.
>
> Best to fix is to define them as unsigned long in the first place.
Can't do that directly, because keyboard_protocol->modifiers is a
field in the data received from the device, i.e. defined by that
protocol. Instead I could make a copy of the modifiers and pass that
around separately (i.e. in addition to the keyboard_protocol struct).
However, the implied size assertions here would basically still apply:
MAX_MODIFIERS == sizeof(keyboard_protocol->modifiers) * 8
ARRAY_SIZE(applespi_controlcodes) == sizeof(keyboard_protocol->modifiers) * 8
(hmm, MAX_MODIFIERS is really redundant - getting rid of it...)
Would using compiletime_assert()'s be an acceptable alternate approach
here? It would serve to both document the size constraint and to
protect against overflow due to an error in some future edit. E.g.
applespi_remap_fn_key(struct keyboard_protocol *keyboard_protocol)
{
unsigned char tmp;
unsigned long *modifiers = (void *)&keyboard_protocol->modifiers;
+
+ compiletime_assert(ARRAY_SIZE(applespi_controlcodes) ==
+ sizeof_field(struct keyboard_protocol, modifiers) * 8,
+ "applespi_controlcodes has wrong number of entries");
if (!fnremap || fnremap > ARRAY_SIZE(applespi_controlcodes) ||
!applespi_controlcodes[fnremap - 1])
return;
tmp = keyboard_protocol->fn_pressed;
keyboard_protocol->fn_pressed = test_bit(fnremap - 1, modifiers);
if (tmp)
__set_bit(fnremap - 1, modifiers);
else
__clear_bit(fnremap - 1, modifiers);
}
> > +}
>
> > + applespi->last_keys_fn_pressed[i]);
> > + input_report_key(applespi->keyboard_input_dev, key, 0);
> > + applespi->last_keys_fn_pressed[i] = 0;
> > + }
>
> > + for (i = 0; i < MAX_MODIFIERS; i++) {
>
> > + u8 *modifiers = &keyboard_protocol->modifiers;
> > +
> > + if (test_bit(i, (unsigned long *)modifiers))
>
> Oh, this is not good idea, see above.
See above. (I presume duplicating the compiletime_assert() here isn't
necessary, if going that route?)
Cheers,
Ronald
^ permalink raw reply
* Re: [PATCH] HID: quirks: use correct format chars in dbg_hid
From: Benjamin Tissoires @ 2019-02-27 9:57 UTC (permalink / raw)
To: Louis Taylor
Cc: Jiri Kosina, open list:HID CORE LAYER, lkml, clang-built-linux,
ndesaulniers
In-Reply-To: <20190226234853.20441-1-louis@kragniz.eu>
On Wed, Feb 27, 2019 at 12:50 AM Louis Taylor <louis@kragniz.eu> wrote:
>
> When building with -Wformat, clang warns:
>
> drivers/hid/hid-quirks.c:1075:27: warning: format specifies type
> 'unsigned short' but the argument has type '__u32' (aka 'unsigned int')
> [-Wformat]
> bl_entry->driver_data, bl_entry->vendor,
> ^~~~~~~~~~~~~~~~
> ./include/linux/hid.h:1170:48: note: expanded from macro 'dbg_hid'
> printk(KERN_DEBUG "%s: " format, __FILE__, ##arg); \
> ~~~~~~ ^~~
> drivers/hid/hid-quirks.c:1076:4: warning: format specifies type
> 'unsigned short' but the argument has type '__u32' (aka 'unsigned int')
> [-Wformat]
> bl_entry->product);
> ^~~~~~~~~~~~~~~~~
> ./include/linux/hid.h:1170:48: note: expanded from macro 'dbg_hid'
> printk(KERN_DEBUG "%s: " format, __FILE__, ##arg); \
> ~~~~~~ ^~~
> drivers/hid/hid-quirks.c:1242:12: warning: format specifies type
> 'unsigned short' but the argument has type '__u32' (aka 'unsigned int')
> [-Wformat]
> quirks, hdev->vendor, hdev->product);
> ^~~~~~~~~~~~
> ./include/linux/hid.h:1170:48: note: expanded from macro 'dbg_hid'
> printk(KERN_DEBUG "%s: " format, __FILE__, ##arg); \
> ~~~~~~ ^~~
> drivers/hid/hid-quirks.c:1242:26: warning: format specifies type
> 'unsigned short' but the argument has type '__u32' (aka 'unsigned int')
> [-Wformat]
> quirks, hdev->vendor, hdev->product);
> ^~~~~~~~~~~~~
> ./include/linux/hid.h:1170:48: note: expanded from macro 'dbg_hid'
> printk(KERN_DEBUG "%s: " format, __FILE__, ##arg); \
> ~~~~~~ ^~~
> 4 warnings generated.
>
> This patch fixes the format strings to use the correct format type for unsigned
> ints.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/378
> Signed-off-by: Louis Taylor <louis@kragniz.eu>
> ---
> drivers/hid/hid-quirks.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index 94088c0ed68a..b4e49e1b6f4a 100644
> --- a/drivers/hid/hid-quirks.c
> +++ b/drivers/hid/hid-quirks.c
> @@ -1071,7 +1071,7 @@ static struct hid_device_id *hid_exists_dquirk(const struct hid_device *hdev)
> }
>
> if (bl_entry != NULL)
> - dbg_hid("Found dynamic quirk 0x%lx for HID device 0x%hx:0x%hx\n",
> + dbg_hid("Found dynamic quirk 0x%lx for HID device 0x%x:0x%x\n",
Can you make it %04x instead?
The VID/PID are usually 4 hex chars, and without the '04' format,
you'll end up having a varying length result, which is not that nice.
Cheers,
Benjamin
> bl_entry->driver_data, bl_entry->vendor,
> bl_entry->product);
>
> @@ -1238,7 +1238,7 @@ static unsigned long hid_gets_squirk(const struct hid_device *hdev)
> quirks |= bl_entry->driver_data;
>
> if (quirks)
> - dbg_hid("Found squirk 0x%lx for HID device 0x%hx:0x%hx\n",
> + dbg_hid("Found squirk 0x%lx for HID device 0x%x:0x%x\n",
> quirks, hdev->vendor, hdev->product);
> return quirks;
> }
> --
> 2.20.1
>
^ permalink raw reply
* [PATCH v2] HID: quirks: use correct format chars in dbg_hid
From: Louis Taylor @ 2019-02-27 11:07 UTC (permalink / raw)
To: jikos
Cc: benjamin.tissoires, linux-input, linux-kernel, clang-built-linux,
ndesaulniers, Louis Taylor
In-Reply-To: <20190226234853.20441-1-louis@kragniz.eu>
When building with -Wformat, clang warns:
drivers/hid/hid-quirks.c:1075:27: warning: format specifies type
'unsigned short' but the argument has type '__u32' (aka 'unsigned int')
[-Wformat]
bl_entry->driver_data, bl_entry->vendor,
^~~~~~~~~~~~~~~~
./include/linux/hid.h:1170:48: note: expanded from macro 'dbg_hid'
printk(KERN_DEBUG "%s: " format, __FILE__, ##arg); \
~~~~~~ ^~~
drivers/hid/hid-quirks.c:1076:4: warning: format specifies type
'unsigned short' but the argument has type '__u32' (aka 'unsigned int')
[-Wformat]
bl_entry->product);
^~~~~~~~~~~~~~~~~
./include/linux/hid.h:1170:48: note: expanded from macro 'dbg_hid'
printk(KERN_DEBUG "%s: " format, __FILE__, ##arg); \
~~~~~~ ^~~
drivers/hid/hid-quirks.c:1242:12: warning: format specifies type
'unsigned short' but the argument has type '__u32' (aka 'unsigned int')
[-Wformat]
quirks, hdev->vendor, hdev->product);
^~~~~~~~~~~~
./include/linux/hid.h:1170:48: note: expanded from macro 'dbg_hid'
printk(KERN_DEBUG "%s: " format, __FILE__, ##arg); \
~~~~~~ ^~~
drivers/hid/hid-quirks.c:1242:26: warning: format specifies type
'unsigned short' but the argument has type '__u32' (aka 'unsigned int')
[-Wformat]
quirks, hdev->vendor, hdev->product);
^~~~~~~~~~~~~
./include/linux/hid.h:1170:48: note: expanded from macro 'dbg_hid'
printk(KERN_DEBUG "%s: " format, __FILE__, ##arg); \
~~~~~~ ^~~
4 warnings generated.
This patch fixes the format strings to use the correct format type for unsigned
ints.
Link: https://github.com/ClangBuiltLinux/linux/issues/378
Signed-off-by: Louis Taylor <louis@kragniz.eu>
---
v2: change format string to use %04x instead of %x
drivers/hid/hid-quirks.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 94088c0ed68a..b608a57b7908 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -1071,7 +1071,7 @@ static struct hid_device_id *hid_exists_dquirk(const struct hid_device *hdev)
}
if (bl_entry != NULL)
- dbg_hid("Found dynamic quirk 0x%lx for HID device 0x%hx:0x%hx\n",
+ dbg_hid("Found dynamic quirk 0x%lx for HID device 0x%04x:0x%04x\n",
bl_entry->driver_data, bl_entry->vendor,
bl_entry->product);
@@ -1238,7 +1238,7 @@ static unsigned long hid_gets_squirk(const struct hid_device *hdev)
quirks |= bl_entry->driver_data;
if (quirks)
- dbg_hid("Found squirk 0x%lx for HID device 0x%hx:0x%hx\n",
+ dbg_hid("Found squirk 0x%lx for HID device 0x%04x:0x%04x\n",
quirks, hdev->vendor, hdev->product);
return quirks;
}
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v2] HID: quirks: use correct format chars in dbg_hid
From: Nick Desaulniers @ 2019-02-27 19:42 UTC (permalink / raw)
To: Louis Taylor
Cc: Jiri Kosina, benjamin.tissoires, linux-input, LKML,
clang-built-linux
In-Reply-To: <20190227110720.27329-1-louis@kragniz.eu>
On Wed, Feb 27, 2019 at 3:08 AM Louis Taylor <louis@kragniz.eu> wrote:
>
> When building with -Wformat, clang warns:
>
> drivers/hid/hid-quirks.c:1075:27: warning: format specifies type
> 'unsigned short' but the argument has type '__u32' (aka 'unsigned int')
> [-Wformat]
> bl_entry->driver_data, bl_entry->vendor,
> ^~~~~~~~~~~~~~~~
> ./include/linux/hid.h:1170:48: note: expanded from macro 'dbg_hid'
> printk(KERN_DEBUG "%s: " format, __FILE__, ##arg); \
> ~~~~~~ ^~~
> drivers/hid/hid-quirks.c:1076:4: warning: format specifies type
> 'unsigned short' but the argument has type '__u32' (aka 'unsigned int')
> [-Wformat]
> bl_entry->product);
> ^~~~~~~~~~~~~~~~~
> ./include/linux/hid.h:1170:48: note: expanded from macro 'dbg_hid'
> printk(KERN_DEBUG "%s: " format, __FILE__, ##arg); \
> ~~~~~~ ^~~
> drivers/hid/hid-quirks.c:1242:12: warning: format specifies type
> 'unsigned short' but the argument has type '__u32' (aka 'unsigned int')
> [-Wformat]
> quirks, hdev->vendor, hdev->product);
> ^~~~~~~~~~~~
> ./include/linux/hid.h:1170:48: note: expanded from macro 'dbg_hid'
> printk(KERN_DEBUG "%s: " format, __FILE__, ##arg); \
> ~~~~~~ ^~~
> drivers/hid/hid-quirks.c:1242:26: warning: format specifies type
> 'unsigned short' but the argument has type '__u32' (aka 'unsigned int')
> [-Wformat]
> quirks, hdev->vendor, hdev->product);
> ^~~~~~~~~~~~~
> ./include/linux/hid.h:1170:48: note: expanded from macro 'dbg_hid'
> printk(KERN_DEBUG "%s: " format, __FILE__, ##arg); \
> ~~~~~~ ^~~
> 4 warnings generated.
>
> This patch fixes the format strings to use the correct format type for unsigned
> ints.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/378
> Signed-off-by: Louis Taylor <louis@kragniz.eu>
Thanks for following up on the feedback.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>
> v2: change format string to use %04x instead of %x
>
> drivers/hid/hid-quirks.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index 94088c0ed68a..b608a57b7908 100644
> --- a/drivers/hid/hid-quirks.c
> +++ b/drivers/hid/hid-quirks.c
> @@ -1071,7 +1071,7 @@ static struct hid_device_id *hid_exists_dquirk(const struct hid_device *hdev)
> }
>
> if (bl_entry != NULL)
> - dbg_hid("Found dynamic quirk 0x%lx for HID device 0x%hx:0x%hx\n",
> + dbg_hid("Found dynamic quirk 0x%lx for HID device 0x%04x:0x%04x\n",
> bl_entry->driver_data, bl_entry->vendor,
> bl_entry->product);
>
> @@ -1238,7 +1238,7 @@ static unsigned long hid_gets_squirk(const struct hid_device *hdev)
> quirks |= bl_entry->driver_data;
>
> if (quirks)
> - dbg_hid("Found squirk 0x%lx for HID device 0x%hx:0x%hx\n",
> + dbg_hid("Found squirk 0x%lx for HID device 0x%04x:0x%04x\n",
> quirks, hdev->vendor, hdev->product);
> return quirks;
> }
> --
> 2.20.1
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply
* [RFC/RFT] HID: primax: Fix wireless keyboards descriptor
From: Nicolas Saenz Julienne @ 2019-02-28 13:55 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: oneukum, Nicolas Saenz Julienne, linux-input, linux-kernel
A whole set of Primax manufactured wireless keyboards won't work out of
the box as they provide wrong HID descriptors. In this case the offense
being defining the "Usage Maximum/Minimum" local items before the "Usage
Page". This will make the parser use the previous "Usage Page" on those
local items, generating a wrong HID report. This is not a parser error
as the spec clearly states that "Usage Page" applies to any item that
follows it (see 6.2.2.7).
In other words, we get this:
15 00 Logical Minimum (0),
26 ff 00 Logical Maximum (255),
19 00 Usage Minimum (00h),
2a ff 00 Usage Maximum (FFh),
05 07 Usage Page (Keyboard), ; Keyboard/keypad (07h)
75 08 Report Size (8),
95 06 Report Count (6),
81 00 Input,
Yet they meant this:
15 00 Logical Minimum (0),
26 ff 00 Logical Maximum (255),
05 07 Usage Page (Keyboard), ; Keyboard/keypad (07h)
19 00 Usage Minimum (00h),
2a ff 00 Usage Maximum (FFh),
75 08 Report Size (8),
95 06 Report Count (6),
81 00 Input,
This patch fixes the issue by overwriting the offending report with a
correct one.
This was made thanks to the user's answers provided here, also full HID
descriptors are available there:
https://unix.stackexchange.com/questions/377830/linux-hid-driver-for-primax-wireless-keyboards/
Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
NOTE: This is an RFC as I'm unable to test my fix. Apart from the
devices mentioned in the patch there is a whole list of them, yet I
didn't include them as I was unable to access their HID descriptors.
drivers/hid/hid-ids.h | 3 +++
drivers/hid/hid-primax.c | 30 +++++++++++++++++++++++++++++-
drivers/hid/hid-quirks.c | 3 +++
3 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 24f846d67478..b39f9f36d41f 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -714,6 +714,7 @@
#define USB_DEVICE_ID_LENOVO_TPPRODOCK 0x6067
#define USB_DEVICE_ID_LENOVO_X1_COVER 0x6085
#define USB_DEVICE_ID_LENOVO_X1_TAB 0x60a3
+#define USB_DEVICE_ID_LENOVO_WIRELESS_PROFESSIONAL 0x609b
#define USB_VENDOR_ID_LG 0x1fd2
#define USB_DEVICE_ID_LG_MULTITOUCH 0x0064
@@ -1225,6 +1226,8 @@
#define USB_DEVICE_ID_PRIMAX_REZEL 0x4e72
#define USB_DEVICE_ID_PRIMAX_PIXART_MOUSE_4D0F 0x4d0f
#define USB_DEVICE_ID_PRIMAX_PIXART_MOUSE_4E22 0x4e22
+#define USB_DEVICE_ID_PRIMAX_WIRELESS_KEYBOARD_4E63 0x4e63
+#define USB_DEVICE_ID_PRIMAX_WIRELESS_KEYBOARD_4E80 0x4e80
#define USB_VENDOR_ID_RISO_KAGAKU 0x1294 /* Riso Kagaku Corp. */
diff --git a/drivers/hid/hid-primax.c b/drivers/hid/hid-primax.c
index 3a1c3c4c50dc..033f6660e8b6 100644
--- a/drivers/hid/hid-primax.c
+++ b/drivers/hid/hid-primax.c
@@ -2,9 +2,11 @@
* HID driver for primax and similar keyboards with in-band modifiers
*
* Copyright 2011 Google Inc. All Rights Reserved
+ * Copyright 2019 Nicolas Saenz Julienne
*
- * Author:
+ * Authors:
* Terry Lambert <tlambert@google.com>
+ * Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
*
* This software is licensed under the terms of the GNU General Public
* License version 2, as published by the Free Software Foundation, and
@@ -27,6 +29,9 @@ static int px_raw_event(struct hid_device *hid, struct hid_report *report,
{
int idx = size;
+ if (hid->product != USB_DEVICE_ID_PRIMAX_KEYBOARD)
+ return 0;
+
switch (report->id) {
case 0: /* keyboard input */
/*
@@ -64,8 +69,29 @@ static int px_raw_event(struct hid_device *hid, struct hid_report *report,
return 0;
}
+/*
+ * Some Primax manufactured keyboards set "Usage Minimum/Maximum" values before
+ * setting the "Usage Page". This is not supported as per spec 6.2.2.7.
+ */
+static __u8 *px_fixup(struct hid_device *hid, __u8 *rdesc, unsigned int *rsize)
+{
+ __u8 wrong_report[] = { 0x19, 0x00, 0x2a, 0xff, 0x05, 0x07 };
+ __u8 fixed_report[] = { 0x05, 0x07, 0x19, 0x00, 0x2a, 0xff };
+
+ if (*rsize > 57 &&
+ !memcmp(&rdesc[51], wrong_report, ARRAY_SIZE(wrong_report))) {
+ hid_info(hid, "fixing up Primax report descriptor\n");
+ memcpy(&rdesc[51], fixed_report, ARRAY_SIZE(fixed_report));
+ }
+
+ return rdesc;
+}
+
static const struct hid_device_id px_devices[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_KEYBOARD) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_WIRELESS_KEYBOARD_4E63) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_WIRELESS_KEYBOARD_4E80) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_WIRELESS_PROFESSIONAL) },
{ }
};
MODULE_DEVICE_TABLE(hid, px_devices);
@@ -74,8 +100,10 @@ static struct hid_driver px_driver = {
.name = "primax",
.id_table = px_devices,
.raw_event = px_raw_event,
+ .report_fixup = px_fixup,
};
module_hid_driver(px_driver);
MODULE_AUTHOR("Terry Lambert <tlambert@google.com>");
+MODULE_AUTHOR("Nicolas Saenz Julienne <nsaenzjulienne@suse.de");
MODULE_LICENSE("GPL");
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 94088c0ed68a..9fd256fea231 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -563,6 +563,9 @@ static const struct hid_device_id hid_have_special_driver[] = {
#endif
#if IS_ENABLED(CONFIG_HID_PRIMAX)
{ HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_KEYBOARD) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_WIRELESS_KEYBOARD_4E63) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_WIRELESS_KEYBOARD_4E80) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_WIRELESS_PROFESSIONAL) },
#endif
#if IS_ENABLED(CONFIG_HID_PRODIKEYS)
{ HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS, USB_DEVICE_ID_PRODIKEYS_PCMIDI) },
--
2.20.1
^ permalink raw reply related
* RE: [RFC/RFT] HID: primax: Fix wireless keyboards descriptor
From: Junge, Terry @ 2019-02-28 17:02 UTC (permalink / raw)
To: Nicolas Saenz Julienne, Jiri Kosina, Benjamin Tissoires
Cc: oneukum@suse.de, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20190228135556.14713-1-nsaenzjulienne@suse.de>
This could also be a parser error. In the HID specification section 6.2.2.8 it states that the last declared Usage Page is applied to Usages when the Main item is encountered.
"If the bSize field = 1 or 2 then the Usage is interpreted as an unsigned value
that selects a Usage ID on the currently defined Usage Page. When the parser
encounters a main item it concatenates the last declared Usage Page with a
Usage to form a complete usage value. Extended usages can be used to
override the currently defined Usage Page for individual usages."
-----Original Message-----
From: linux-input-owner@vger.kernel.org <linux-input-owner@vger.kernel.org> On Behalf Of Nicolas Saenz Julienne
Sent: Thursday, February 28, 2019 5:56 AM
To: Jiri Kosina <jikos@kernel.org>; Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: oneukum@suse.de; Nicolas Saenz Julienne <nsaenzjulienne@suse.de>; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: [RFC/RFT] HID: primax: Fix wireless keyboards descriptor
A whole set of Primax manufactured wireless keyboards won't work out of the box as they provide wrong HID descriptors. In this case the offense being defining the "Usage Maximum/Minimum" local items before the "Usage Page". This will make the parser use the previous "Usage Page" on those local items, generating a wrong HID report. This is not a parser error as the spec clearly states that "Usage Page" applies to any item that follows it (see 6.2.2.7).
In other words, we get this:
15 00 Logical Minimum (0),
26 ff 00 Logical Maximum (255),
19 00 Usage Minimum (00h),
2a ff 00 Usage Maximum (FFh),
05 07 Usage Page (Keyboard), ; Keyboard/keypad (07h)
75 08 Report Size (8),
95 06 Report Count (6),
81 00 Input,
Yet they meant this:
15 00 Logical Minimum (0),
26 ff 00 Logical Maximum (255),
05 07 Usage Page (Keyboard), ; Keyboard/keypad (07h)
19 00 Usage Minimum (00h),
2a ff 00 Usage Maximum (FFh),
75 08 Report Size (8),
95 06 Report Count (6),
81 00 Input,
This patch fixes the issue by overwriting the offending report with a correct one.
This was made thanks to the user's answers provided here, also full HID descriptors are available there:
https://unix.stackexchange.com/questions/377830/linux-hid-driver-for-primax-wireless-keyboards/
Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
NOTE: This is an RFC as I'm unable to test my fix. Apart from the
devices mentioned in the patch there is a whole list of them, yet I
didn't include them as I was unable to access their HID descriptors.
drivers/hid/hid-ids.h | 3 +++
drivers/hid/hid-primax.c | 30 +++++++++++++++++++++++++++++- drivers/hid/hid-quirks.c | 3 +++
3 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 24f846d67478..b39f9f36d41f 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -714,6 +714,7 @@
#define USB_DEVICE_ID_LENOVO_TPPRODOCK 0x6067
#define USB_DEVICE_ID_LENOVO_X1_COVER 0x6085
#define USB_DEVICE_ID_LENOVO_X1_TAB 0x60a3
+#define USB_DEVICE_ID_LENOVO_WIRELESS_PROFESSIONAL 0x609b
#define USB_VENDOR_ID_LG 0x1fd2
#define USB_DEVICE_ID_LG_MULTITOUCH 0x0064
@@ -1225,6 +1226,8 @@
#define USB_DEVICE_ID_PRIMAX_REZEL 0x4e72
#define USB_DEVICE_ID_PRIMAX_PIXART_MOUSE_4D0F 0x4d0f
#define USB_DEVICE_ID_PRIMAX_PIXART_MOUSE_4E22 0x4e22
+#define USB_DEVICE_ID_PRIMAX_WIRELESS_KEYBOARD_4E63 0x4e63
+#define USB_DEVICE_ID_PRIMAX_WIRELESS_KEYBOARD_4E80 0x4e80
#define USB_VENDOR_ID_RISO_KAGAKU 0x1294 /* Riso Kagaku Corp. */
diff --git a/drivers/hid/hid-primax.c b/drivers/hid/hid-primax.c index 3a1c3c4c50dc..033f6660e8b6 100644
--- a/drivers/hid/hid-primax.c
+++ b/drivers/hid/hid-primax.c
@@ -2,9 +2,11 @@
* HID driver for primax and similar keyboards with in-band modifiers
*
* Copyright 2011 Google Inc. All Rights Reserved
+ * Copyright 2019 Nicolas Saenz Julienne
*
- * Author:
+ * Authors:
* Terry Lambert <tlambert@google.com>
+ * Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
*
* This software is licensed under the terms of the GNU General Public
* License version 2, as published by the Free Software Foundation, and @@ -27,6 +29,9 @@ static int px_raw_event(struct hid_device *hid, struct hid_report *report, {
int idx = size;
+ if (hid->product != USB_DEVICE_ID_PRIMAX_KEYBOARD)
+ return 0;
+
switch (report->id) {
case 0: /* keyboard input */
/*
@@ -64,8 +69,29 @@ static int px_raw_event(struct hid_device *hid, struct hid_report *report,
return 0;
}
+/*
+ * Some Primax manufactured keyboards set "Usage Minimum/Maximum"
+values before
+ * setting the "Usage Page". This is not supported as per spec 6.2.2.7.
+ */
+static __u8 *px_fixup(struct hid_device *hid, __u8 *rdesc, unsigned int
+*rsize) {
+ __u8 wrong_report[] = { 0x19, 0x00, 0x2a, 0xff, 0x05, 0x07 };
+ __u8 fixed_report[] = { 0x05, 0x07, 0x19, 0x00, 0x2a, 0xff };
+
+ if (*rsize > 57 &&
+ !memcmp(&rdesc[51], wrong_report, ARRAY_SIZE(wrong_report))) {
+ hid_info(hid, "fixing up Primax report descriptor\n");
+ memcpy(&rdesc[51], fixed_report, ARRAY_SIZE(fixed_report));
+ }
+
+ return rdesc;
+}
+
static const struct hid_device_id px_devices[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_KEYBOARD) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_WIRELESS_KEYBOARD_4E63) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_WIRELESS_KEYBOARD_4E80) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO,
+USB_DEVICE_ID_LENOVO_WIRELESS_PROFESSIONAL) },
{ }
};
MODULE_DEVICE_TABLE(hid, px_devices);
@@ -74,8 +100,10 @@ static struct hid_driver px_driver = {
.name = "primax",
.id_table = px_devices,
.raw_event = px_raw_event,
+ .report_fixup = px_fixup,
};
module_hid_driver(px_driver);
MODULE_AUTHOR("Terry Lambert <tlambert@google.com>");
+MODULE_AUTHOR("Nicolas Saenz Julienne <nsaenzjulienne@suse.de");
MODULE_LICENSE("GPL");
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c index 94088c0ed68a..9fd256fea231 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -563,6 +563,9 @@ static const struct hid_device_id hid_have_special_driver[] = { #endif #if IS_ENABLED(CONFIG_HID_PRIMAX)
{ HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_KEYBOARD) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_WIRELESS_KEYBOARD_4E63) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_WIRELESS_KEYBOARD_4E80) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO,
+USB_DEVICE_ID_LENOVO_WIRELESS_PROFESSIONAL) },
#endif
#if IS_ENABLED(CONFIG_HID_PRODIKEYS)
{ HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS, USB_DEVICE_ID_PRODIKEYS_PCMIDI) },
--
2.20.1
^ permalink raw reply
* Re: [RFC/RFT] HID: primax: Fix wireless keyboards descriptor
From: Nicolas Saenz Julienne @ 2019-02-28 18:01 UTC (permalink / raw)
To: Junge, Terry, Jiri Kosina, Benjamin Tissoires
Cc: oneukum@suse.de, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <BYAPR02MB56078ADD429292DF89F8C694ED750@BYAPR02MB5607.namprd02.prod.outlook.com>
[-- Attachment #1: Type: text/plain, Size: 1633 bytes --]
On Thu, 2019-02-28 at 17:02 +0000, Junge, Terry wrote:
> This could also be a parser error. In the HID specification section 6.2.2.8 it
> states that the last declared Usage Page is applied to Usages when the Main
> item is encountered.
>
> "If the bSize field = 1 or 2 then the Usage is interpreted as an unsigned
> value
> that selects a Usage ID on the currently defined Usage Page. When the parser
> encounters a main item it concatenates the last declared Usage Page with a
> Usage to form a complete usage value. Extended usages can be used to
> override the currently defined Usage Page for individual usages."
>
Hi Terry, thanks for the comment.
Just for the record the paragraph I cited on my patch is the following:
6.2.2.7 Global Items
[...]
Usage Page: Unsigned integer specifying the current Usage Page. Since a
usage are 32 bit values, Usage Page items can be used to conserve space
in a report descriptor by setting the high order 16 bits of a
subsequent usages. Any usage that follows which is defines* 16 bits or
less is interpreted as a Usage ID and concatenated with the Usage Page
to form a 32 bit Usage.
* This is a spec errata, I belive it should say "defined"
As you can see they use the word "follows" which in my opinion contradicts the
paragraph you pointed out. That said I may be wrong, I'm not too good at
reading specs :).
I checked the HID parser and it's indeed written assuming local items are
preceded by a Usage Page. I'd be glad to fix it there, but it would be nice to
have the mantainer's opinion on the matter first.
Regards,
Nicolas
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH] Input: fix oops in input_to_handler
From: Zhipeng Xie @ 2019-03-01 4:13 UTC (permalink / raw)
To: dmitry.torokhov, rydberg
Cc: linux-input, linux-kernel, kernel.openeuler, euleros,
huawei.libin, yangyingliang, xiezhipeng1
we got the following boot crash:
[ 36.086344] Internal error: Oops: 96000004 [#1] SMP
[ 36.091371] CPU: 32 PID: 0 Comm: swapper/32 Tainted: G OE 4.19.25-vhulk1901.1.0.h111.aarch64 #1
[ 36.101444] Hardware name: Huawei TaiShan 2280 /BC11SPCD, BIOS 1.58 10/24/2018
[ 36.108860] pstate: 20000085 (nzCv daIf -PAN -UAO)
[ 36.113727] pc : input_to_handler+0x2c/0x148
[ 36.118058] lr : input_pass_values.part.2+0x148/0x168
[ 36.123177] sp : ffff000100103ba0
[ 36.126535] x29: ffff000100103ba0 x28: ffff801fb2ac5958
[ 36.131924] x27: ffff000009799000 x26: 0000000000000001
[ 36.137375] x25: 0000000000000000 x24: ffff801fb5b57c00
[ 36.142822] x23: 000000020987f3d0 x22: ffff801faf427e00
[ 36.148211] x21: 0000000000000003 x20: 0000000000000003
[ 36.153599] x19: ffff801faf427e00 x18: 000000000000000e
[ 36.158986] x17: 000000000000000e x16: 0000000000000007
[ 36.164374] x15: 0000000000000001 x14: 0000000000000019
[ 36.169762] x13: 0000000000000033 x12: 000000000000004c
[ 36.175150] x11: 0000000000000068 x10: ffff000008dfa290
[ 36.180538] x9 : 000000000000007d x8 : 0000000000000000
[ 36.185925] x7 : 0000000000000053 x6 : 0000000000000000
[ 36.191313] x5 : 0000000000000000 x4 : 0000000000000000
[ 36.196700] x3 : 0000000000000010 x2 : 0000000000000003
[ 36.202088] x1 : ffff801fb5b57c00 x0 : ffff000008a120a0
[ 36.207477] Process swapper/32 (pid: 0, stack limit = 0x0000000032f86b58)
[ 36.214361] Call trace:
[ 36.216840] input_to_handler+0x2c/0x148
[ 36.220816] input_pass_values.part.2+0x148/0x168
[ 36.225582] input_handle_event+0x130/0x5b8
[ 36.229823] i6.242013] hid_input_report+0x128/0x1b0
[ 36.246076] hid_irq_in+0x240/0x298
[ 36.249613] __usb_hcd_giveback_urb+0x9c/0x130
[ 36.257460] usb_giveback_urb_bh+0xf4/0x198
[ 36.265127] tasklet_action_common.isra.6+0x94/0x160
[ 36.273458] tasklet_hi_action+0x2c/0x38
[ 36.280597] __do_softirq+0x118/0x314
[ 36.287494] irq_exit+0xa4/0xe8
[ 36.293682] __handle_domain_irq+0x6c/0xc0
[ 36.300783] gic_handle_irq+0x6c/0x170
[ 36.307296] el1_irq+0xb8/0x140
[ 36.313283] arch_cpu_idle+0x38/0x1c0
[ 36.319776] default_idle_call+0x24/0x44
[ 36.326527] do_idle+0x1ec/0x2d0
[ 36.332503] cpu_startup_entry+0x28/0x30
[ 36.339161] secondary_start_kernel+0x194/0x1d8
We need to check the input_handler before referencing its members.
Signed-off-by: Zhipeng Xie <xiezhipeng1@huawei.com>
---
drivers/input/input.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 3304aaa..b768d14 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -100,6 +100,9 @@ static unsigned int input_to_handler(struct input_handle *handle,
struct input_value *end = vals;
struct input_value *v;
+ if (!handler)
+ return 0;
+
if (handler->filter) {
for (v = vals; v != vals + count; v++) {
if (handler->filter(handle, v->type, v->code, v->value))
--
1.7.12.4
^ permalink raw reply related
* Re: [RFC/RFT] HID: primax: Fix wireless keyboards descriptor
From: Benjamin Tissoires @ 2019-03-01 9:48 UTC (permalink / raw)
To: Nicolas Saenz Julienne
Cc: Junge, Terry, Jiri Kosina, oneukum@suse.de,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <a6ecd59f99aa64355ac7c176aa4310d32ce8969e.camel@suse.de>
On Thu, Feb 28, 2019 at 7:01 PM Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> On Thu, 2019-02-28 at 17:02 +0000, Junge, Terry wrote:
> > This could also be a parser error. In the HID specification section 6.2.2.8 it
> > states that the last declared Usage Page is applied to Usages when the Main
> > item is encountered.
> >
> > "If the bSize field = 1 or 2 then the Usage is interpreted as an unsigned
> > value
> > that selects a Usage ID on the currently defined Usage Page. When the parser
> > encounters a main item it concatenates the last declared Usage Page with a
> > Usage to form a complete usage value. Extended usages can be used to
> > override the currently defined Usage Page for individual usages."
> >
>
> Hi Terry, thanks for the comment.
> Just for the record the paragraph I cited on my patch is the following:
>
> 6.2.2.7 Global Items
>
> [...]
>
> Usage Page: Unsigned integer specifying the current Usage Page. Since a
> usage are 32 bit values, Usage Page items can be used to conserve space
> in a report descriptor by setting the high order 16 bits of a
> subsequent usages. Any usage that follows which is defines* 16 bits or
> less is interpreted as a Usage ID and concatenated with the Usage Page
> to form a 32 bit Usage.
>
> * This is a spec errata, I belive it should say "defined"
>
> As you can see they use the word "follows" which in my opinion contradicts the
> paragraph you pointed out. That said I may be wrong, I'm not too good at
> reading specs :).
I think you are both right (that is some decision making skills :-P).
I never saw a case where the Usage Page was after the Usage. And I
would lean towards Nicolas interpretation.
However, Terry's point is valid too, and by re-reading the two
paragraphs, one could argue that the "follows" of 6.2.2.7 could be
applied when the Main item is processed as in 6.2.2.8.
So I am going to base my decision on the "reference" driver. How well
behaves Windows with this particular keyboard?
If it behaves properly, then we better use the 6.2.2.8 version where
the Usage Page is only appended when there is a Main item. This means
we need to remember what size was the last Usage (and Min/Max), to
know if we should concatenate the 2.
Note that for every change that involves the generic parser, I'd like
a few tests added to `tests/test_keyboard.py in
https://gitlab.freedesktop.org/libevdev/hid-tools
Also note that the HID parser implementation in hid-tools follows the
kernel, so we might need to adjust it there too if we want to add high
level events. If you directly call `.call_input_event()` with raw
events, it should be fine.
>
> I checked the HID parser and it's indeed written assuming local items are
> preceded by a Usage Page. I'd be glad to fix it there, but it would be nice to
> have the mantainer's opinion on the matter first.
>
You've now got the opinion of one of the 2 maintainers, hope this helps :)
Cheers,
Benjamin
^ permalink raw reply
* [PATCH] platform/x86: touchscreen_dmi: Add info for the CHUWI Hi10 Air tablet
From: me @ 2019-03-01 16:40 UTC (permalink / raw)
Cc: Christian Oder, Hans de Goede, Darren Hart, Andy Shevchenko,
linux-input, platform-driver-x86, linux-kernel
From: Christian Oder <me@myself5.de>
Add touchscreen info for the CHUWUI Hi10 Air tablet.
Signed-off-by: Christian Oder <me@myself5.de>
---
drivers/platform/x86/touchscreen_dmi.c | 27 ++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/platform/x86/touchscreen_dmi.c b/drivers/platform/x86/touchscreen_dmi.c
index 167a156e3cc7..2d56ff7c8230 100644
--- a/drivers/platform/x86/touchscreen_dmi.c
+++ b/drivers/platform/x86/touchscreen_dmi.c
@@ -72,6 +72,25 @@ static const struct ts_dmi_data chuwi_hi8_pro_data = {
.properties = chuwi_hi8_pro_props,
};
+static const struct property_entry chuwi_hi10_air_props[] = {
+ PROPERTY_ENTRY_U32("touchscreen-size-x", 1981),
+ PROPERTY_ENTRY_U32("touchscreen-size-y", 1271),
+ PROPERTY_ENTRY_U32("touchscreen-min-x", 99),
+ PROPERTY_ENTRY_U32("touchscreen-min-y", 9),
+ PROPERTY_ENTRY_BOOL("touchscreen-swapped-x-y"),
+ PROPERTY_ENTRY_U32("touchscreen-fuzz-x", 5),
+ PROPERTY_ENTRY_U32("touchscreen-fuzz-y", 4),
+ PROPERTY_ENTRY_STRING("firmware-name", "gsl1680-chuwi-hi10-air.fw"),
+ PROPERTY_ENTRY_U32("silead,max-fingers", 10),
+ PROPERTY_ENTRY_BOOL("silead,home-button"),
+ { }
+};
+
+static const struct ts_dmi_data chuwi_hi10_air_data = {
+ .acpi_name = "MSSL1680:00",
+ .properties = chuwi_hi10_air_props,
+};
+
static const struct property_entry chuwi_vi8_props[] = {
PROPERTY_ENTRY_U32("touchscreen-min-x", 4),
PROPERTY_ENTRY_U32("touchscreen-min-y", 6),
@@ -546,6 +565,14 @@ static const struct dmi_system_id touchscreen_dmi_table[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "X1D3_C806N"),
},
},
+ {
+ /* Chuwi Hi10 Air */
+ .driver_data = (void *)&chuwi_hi10_air_data,
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Hampoo"),
+ DMI_MATCH(DMI_PRODUCT_SKU, "P1W6_C109D_B"),
+ },
+ },
{
/* Chuwi Vi8 (CWI506) */
.driver_data = (void *)&chuwi_vi8_data,
--
2.20.1
^ permalink raw reply related
* [PATCH 1/2] dt-bindings: input: add GPIO controllable vibrator
From: Luca Weiss @ 2019-03-02 14:11 UTC (permalink / raw)
Cc: Luca Weiss, Dmitry Torokhov, Rob Herring, Mark Rutland,
open list:INPUT KEYBOARD, MOUSE, JOYSTICK , TOUCHSCREEN...,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
Provide a simple driver for GPIO controllable vibrators.
It will be used by the Fairphone 2.
Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
.../devicetree/bindings/input/gpio-vibrator.txt | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/gpio-vibrator.txt
diff --git a/Documentation/devicetree/bindings/input/gpio-vibrator.txt b/Documentation/devicetree/bindings/input/gpio-vibrator.txt
new file mode 100644
index 000000000000..9e2e9acf497b
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/gpio-vibrator.txt
@@ -0,0 +1,16 @@
+* GPIO vibrator device tree bindings
+
+Registers a GPIO device as vibrator, where the vibration motor just has the capability to turn on or off. If the device is connected to a pwm, you should use the pwm-vibrator driver instead.
+
+Required properties:
+- compatible: should contain "gpio-vibrator"
+- enable-gpios: Should contain a GPIO handle
+- vcc-supply: Phandle for the regulator supplying power
+
+Example from Fairphone 2:
+
+vibrator {
+ compatible = "gpio-vibrator";
+ enable-gpios = <&msmgpio 86 GPIO_ACTIVE_HIGH>;
+ vcc-supply = <&pm8941_l18>;
+};
--
2.21.0
^ permalink raw reply related
* [PATCH 2/2] Input: add a driver for GPIO controllable vibrators
From: Luca Weiss @ 2019-03-02 14:11 UTC (permalink / raw)
Cc: Luca Weiss, Dmitry Torokhov, Xiaotong Lu, Coly Li, Stephen Boyd,
Lucas Stach, Mauro Carvalho Chehab, Andrey Smirnov, Arnd Bergmann,
Aaron Wu, Rob Herring, open list,
open list:INPUT KEYBOARD, MOUSE, JOYSTICK , TOUCHSCREEN...
In-Reply-To: <20190302141132.21160-1-luca@z3ntu.xyz>
Provide a simple driver for GPIO controllable vibrators.
It will be used by the Fairphone 2.
Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
drivers/input/misc/Kconfig | 12 ++
drivers/input/misc/Makefile | 1 +
drivers/input/misc/gpio-vibra.c | 214 ++++++++++++++++++++++++++++++++
3 files changed, 227 insertions(+)
create mode 100644 drivers/input/misc/gpio-vibra.c
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index ca59a2be9bc5..77480268fef2 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -280,6 +280,18 @@ config INPUT_GPIO_DECODER
To compile this driver as a module, choose M here: the module
will be called gpio_decoder.
+config INPUT_GPIO_VIBRA
+ tristate "GPIO vibrator support"
+ depends on GPIOLIB || COMPILE_TEST
+ select INPUT_FF_MEMLESS
+ help
+ Say Y here to get support for GPIO based vibrator devices.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the module will be
+ called gpio-vibra.
+
config INPUT_IXP4XX_BEEPER
tristate "IXP4XX Beeper support"
depends on ARCH_IXP4XX
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 9d0f9d1ff68f..79edbad44cf3 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_INPUT_DRV2667_HAPTICS) += drv2667.o
obj-$(CONFIG_INPUT_GP2A) += gp2ap002a00f.o
obj-$(CONFIG_INPUT_GPIO_BEEPER) += gpio-beeper.o
obj-$(CONFIG_INPUT_GPIO_DECODER) += gpio_decoder.o
+obj-$(CONFIG_INPUT_GPIO_VIBRA) += gpio-vibra.o
obj-$(CONFIG_INPUT_HISI_POWERKEY) += hisi_powerkey.o
obj-$(CONFIG_HP_SDC_RTC) += hp_sdc_rtc.o
obj-$(CONFIG_INPUT_IMS_PCU) += ims-pcu.o
diff --git a/drivers/input/misc/gpio-vibra.c b/drivers/input/misc/gpio-vibra.c
new file mode 100644
index 000000000000..14f9534668c8
--- /dev/null
+++ b/drivers/input/misc/gpio-vibra.c
@@ -0,0 +1,214 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * GPIO vibrator driver
+ *
+ * Copyright (C) 2019 Luca Weiss <luca@z3ntu.xyz>
+ *
+ * Based on PWM vibrator driver:
+ * Copyright (C) 2017 Collabora Ltd.
+ *
+ * Based on previous work from:
+ * Copyright (C) 2012 Dmitry Torokhov <dmitry.torokhov@gmail.com>
+ *
+ * Based on PWM beeper driver:
+ * Copyright (C) 2010, Lars-Peter Clausen <lars@metafoo.de>
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/input.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+
+enum vibrator_state {
+ VIBRATOR_OFF,
+ VIBRATOR_ON
+};
+
+struct gpio_vibrator {
+ struct input_dev *input;
+ struct gpio_desc *gpio;
+ struct regulator *vcc;
+
+ struct work_struct play_work;
+ enum vibrator_state state;
+ bool vcc_on;
+};
+
+static int gpio_vibrator_start(struct gpio_vibrator *vibrator)
+{
+ struct device *pdev = vibrator->input->dev.parent;
+ int err;
+
+ if (!vibrator->vcc_on) {
+ err = regulator_enable(vibrator->vcc);
+ if (err) {
+ dev_err(pdev, "failed to enable regulator: %d", err);
+ return err;
+ }
+ vibrator->vcc_on = true;
+ }
+
+ gpiod_set_value(vibrator->gpio, 1);
+
+ return 0;
+}
+
+static void gpio_vibrator_stop(struct gpio_vibrator *vibrator)
+{
+ gpiod_set_value(vibrator->gpio, 0);
+
+ if (vibrator->vcc_on) {
+ regulator_disable(vibrator->vcc);
+ vibrator->vcc_on = false;
+ }
+}
+
+static void gpio_vibrator_play_work(struct work_struct *work)
+{
+ struct gpio_vibrator *vibrator = container_of(work,
+ struct gpio_vibrator, play_work);
+
+ if (vibrator->state == VIBRATOR_ON)
+ gpio_vibrator_start(vibrator);
+ else
+ gpio_vibrator_stop(vibrator);
+}
+
+static int gpio_vibrator_play_effect(struct input_dev *dev, void *data,
+ struct ff_effect *effect)
+{
+ struct gpio_vibrator *vibrator = input_get_drvdata(dev);
+
+ int level = effect->u.rumble.strong_magnitude;
+
+ if (!level)
+ level = effect->u.rumble.weak_magnitude;
+
+ if (level)
+ vibrator->state = VIBRATOR_ON;
+ else
+ vibrator->state = VIBRATOR_OFF;
+
+ schedule_work(&vibrator->play_work);
+
+ return 0;
+}
+
+static void gpio_vibrator_close(struct input_dev *input)
+{
+ struct gpio_vibrator *vibrator = input_get_drvdata(input);
+
+ cancel_work_sync(&vibrator->play_work);
+ gpio_vibrator_stop(vibrator);
+}
+
+static int gpio_vibrator_probe(struct platform_device *pdev)
+{
+ struct gpio_vibrator *vibrator;
+ int err;
+
+ vibrator = devm_kzalloc(&pdev->dev, sizeof(*vibrator), GFP_KERNEL);
+ if (!vibrator)
+ return -ENOMEM;
+
+ vibrator->input = devm_input_allocate_device(&pdev->dev);
+ if (!vibrator->input)
+ return -ENOMEM;
+
+ vibrator->vcc = devm_regulator_get(&pdev->dev, "vcc");
+ err = PTR_ERR_OR_ZERO(vibrator->vcc);
+ if (err) {
+ if (err != -EPROBE_DEFER)
+ dev_err(&pdev->dev, "Failed to request regulator: %d",
+ err);
+ return err;
+ }
+
+ vibrator->gpio = devm_gpiod_get(&pdev->dev, "enable", GPIOD_OUT_LOW);
+ err = PTR_ERR_OR_ZERO(vibrator->gpio);
+ if (err) {
+ if (err != -EPROBE_DEFER)
+ dev_err(&pdev->dev, "Failed to request main gpio: %d",
+ err);
+ return err;
+ }
+
+ INIT_WORK(&vibrator->play_work, gpio_vibrator_play_work);
+
+ vibrator->input->name = "gpio-vibrator";
+ vibrator->input->id.bustype = BUS_HOST;
+ vibrator->input->dev.parent = &pdev->dev;
+ vibrator->input->close = gpio_vibrator_close;
+
+ input_set_drvdata(vibrator->input, vibrator);
+ input_set_capability(vibrator->input, EV_FF, FF_RUMBLE);
+
+ err = input_ff_create_memless(vibrator->input, NULL,
+ gpio_vibrator_play_effect);
+ if (err) {
+ dev_err(&pdev->dev, "Couldn't create FF dev: %d", err);
+ return err;
+ }
+
+ err = input_register_device(vibrator->input);
+ if (err) {
+ dev_err(&pdev->dev, "Couldn't register input dev: %d", err);
+ return err;
+ }
+
+ platform_set_drvdata(pdev, vibrator);
+
+ return 0;
+}
+
+static int __maybe_unused gpio_vibrator_suspend(struct device *dev)
+{
+ struct gpio_vibrator *vibrator = dev_get_drvdata(dev);
+
+ cancel_work_sync(&vibrator->play_work);
+ if (vibrator->state == VIBRATOR_ON)
+ gpio_vibrator_stop(vibrator);
+
+ return 0;
+}
+
+static int __maybe_unused gpio_vibrator_resume(struct device *dev)
+{
+ struct gpio_vibrator *vibrator = dev_get_drvdata(dev);
+
+ if (vibrator->state == VIBRATOR_ON)
+ gpio_vibrator_start(vibrator);
+
+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(gpio_vibrator_pm_ops,
+ gpio_vibrator_suspend, gpio_vibrator_resume);
+
+#ifdef CONFIG_OF
+static const struct of_device_id gpio_vibra_dt_match_table[] = {
+ { .compatible = "gpio-vibrator" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, gpio_vibra_dt_match_table);
+#endif
+
+static struct platform_driver gpio_vibrator_driver = {
+ .probe = gpio_vibrator_probe,
+ .driver = {
+ .name = "gpio-vibrator",
+ .pm = &gpio_vibrator_pm_ops,
+ .of_match_table = of_match_ptr(gpio_vibra_dt_match_table),
+ },
+};
+module_platform_driver(gpio_vibrator_driver);
+
+MODULE_AUTHOR("Luca Weiss <luca@z3ntu.xy>");
+MODULE_DESCRIPTION("GPIO vibrator driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:gpio-vibrator");
--
2.21.0
^ permalink raw reply related
* [PATCH][next] HID: uclogic: remove redudant duplicated null check on ver_ptr
From: Colin King @ 2019-03-02 22:23 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Currently ver_ptr is being null checked twice, once before calling
usb_string and once afterwards. The second null check is redundant
and can be removed, remove it.
Detected by CoverityScan, CID#1477308 ("Logically dead code")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/hid/hid-uclogic-params.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/hid/hid-uclogic-params.c b/drivers/hid/hid-uclogic-params.c
index 7710d9f957da..0187c9f8fc22 100644
--- a/drivers/hid/hid-uclogic-params.c
+++ b/drivers/hid/hid-uclogic-params.c
@@ -735,10 +735,6 @@ static int uclogic_params_huion_init(struct uclogic_params *params,
goto cleanup;
}
rc = usb_string(udev, 201, ver_ptr, ver_len);
- if (ver_ptr == NULL) {
- rc = -ENOMEM;
- goto cleanup;
- }
if (rc == -EPIPE) {
*ver_ptr = '\0';
} else if (rc < 0) {
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] platform/x86: touchscreen_dmi: Add info for the CHUWI Hi10 Air tablet
From: Hans de Goede @ 2019-03-03 10:55 UTC (permalink / raw)
To: me
Cc: Darren Hart, Andy Shevchenko, linux-input, platform-driver-x86,
linux-kernel
In-Reply-To: <20190301164002.6031-1-me@myself5.de>
Hi,
On 01-03-19 17:40, me@myself5.de wrote:
> From: Christian Oder <me@myself5.de>
>
> Add touchscreen info for the CHUWUI Hi10 Air tablet.
>
> Signed-off-by: Christian Oder <me@myself5.de>
Thank you for the patch, patch looks good to me:
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Regards,
Hans
> ---
> drivers/platform/x86/touchscreen_dmi.c | 27 ++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/platform/x86/touchscreen_dmi.c b/drivers/platform/x86/touchscreen_dmi.c
> index 167a156e3cc7..2d56ff7c8230 100644
> --- a/drivers/platform/x86/touchscreen_dmi.c
> +++ b/drivers/platform/x86/touchscreen_dmi.c
> @@ -72,6 +72,25 @@ static const struct ts_dmi_data chuwi_hi8_pro_data = {
> .properties = chuwi_hi8_pro_props,
> };
>
> +static const struct property_entry chuwi_hi10_air_props[] = {
> + PROPERTY_ENTRY_U32("touchscreen-size-x", 1981),
> + PROPERTY_ENTRY_U32("touchscreen-size-y", 1271),
> + PROPERTY_ENTRY_U32("touchscreen-min-x", 99),
> + PROPERTY_ENTRY_U32("touchscreen-min-y", 9),
> + PROPERTY_ENTRY_BOOL("touchscreen-swapped-x-y"),
> + PROPERTY_ENTRY_U32("touchscreen-fuzz-x", 5),
> + PROPERTY_ENTRY_U32("touchscreen-fuzz-y", 4),
> + PROPERTY_ENTRY_STRING("firmware-name", "gsl1680-chuwi-hi10-air.fw"),
> + PROPERTY_ENTRY_U32("silead,max-fingers", 10),
> + PROPERTY_ENTRY_BOOL("silead,home-button"),
> + { }
> +};
> +
> +static const struct ts_dmi_data chuwi_hi10_air_data = {
> + .acpi_name = "MSSL1680:00",
> + .properties = chuwi_hi10_air_props,
> +};
> +
> static const struct property_entry chuwi_vi8_props[] = {
> PROPERTY_ENTRY_U32("touchscreen-min-x", 4),
> PROPERTY_ENTRY_U32("touchscreen-min-y", 6),
> @@ -546,6 +565,14 @@ static const struct dmi_system_id touchscreen_dmi_table[] = {
> DMI_MATCH(DMI_PRODUCT_NAME, "X1D3_C806N"),
> },
> },
> + {
> + /* Chuwi Hi10 Air */
> + .driver_data = (void *)&chuwi_hi10_air_data,
> + .matches = {
> + DMI_MATCH(DMI_BOARD_VENDOR, "Hampoo"),
> + DMI_MATCH(DMI_PRODUCT_SKU, "P1W6_C109D_B"),
> + },
> + },
> {
> /* Chuwi Vi8 (CWI506) */
> .driver_data = (void *)&chuwi_vi8_data,
>
^ permalink raw reply
* [PATCH v2] platform/x86: touchscreen_dmi: Add info for the CHUWI Hi10 Air tablet
From: Christian Oder @ 2019-03-03 13:47 UTC (permalink / raw)
To: hdegoede; +Cc: andy, dvhart, linux-input, linux-kernel, me, platform-driver-x86
In-Reply-To: <c6b0c2ef-eed9-ffae-e974-92260d5356aa@redhat.com>
Add touchscreen info for the CHUWUI Hi10 Air tablet.
Signed-off-by: Christian Oder <me@myself5.de>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
- added review tag
drivers/platform/x86/touchscreen_dmi.c | 27 ++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/platform/x86/touchscreen_dmi.c b/drivers/platform/x86/touchscreen_dmi.c
index 167a156e3cc7..2d56ff7c8230 100644
--- a/drivers/platform/x86/touchscreen_dmi.c
+++ b/drivers/platform/x86/touchscreen_dmi.c
@@ -72,6 +72,25 @@ static const struct ts_dmi_data chuwi_hi8_pro_data = {
.properties = chuwi_hi8_pro_props,
};
+static const struct property_entry chuwi_hi10_air_props[] = {
+ PROPERTY_ENTRY_U32("touchscreen-size-x", 1981),
+ PROPERTY_ENTRY_U32("touchscreen-size-y", 1271),
+ PROPERTY_ENTRY_U32("touchscreen-min-x", 99),
+ PROPERTY_ENTRY_U32("touchscreen-min-y", 9),
+ PROPERTY_ENTRY_BOOL("touchscreen-swapped-x-y"),
+ PROPERTY_ENTRY_U32("touchscreen-fuzz-x", 5),
+ PROPERTY_ENTRY_U32("touchscreen-fuzz-y", 4),
+ PROPERTY_ENTRY_STRING("firmware-name", "gsl1680-chuwi-hi10-air.fw"),
+ PROPERTY_ENTRY_U32("silead,max-fingers", 10),
+ PROPERTY_ENTRY_BOOL("silead,home-button"),
+ { }
+};
+
+static const struct ts_dmi_data chuwi_hi10_air_data = {
+ .acpi_name = "MSSL1680:00",
+ .properties = chuwi_hi10_air_props,
+};
+
static const struct property_entry chuwi_vi8_props[] = {
PROPERTY_ENTRY_U32("touchscreen-min-x", 4),
PROPERTY_ENTRY_U32("touchscreen-min-y", 6),
@@ -546,6 +565,14 @@ static const struct dmi_system_id touchscreen_dmi_table[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "X1D3_C806N"),
},
},
+ {
+ /* Chuwi Hi10 Air */
+ .driver_data = (void *)&chuwi_hi10_air_data,
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Hampoo"),
+ DMI_MATCH(DMI_PRODUCT_SKU, "P1W6_C109D_B"),
+ },
+ },
{
/* Chuwi Vi8 (CWI506) */
.driver_data = (void *)&chuwi_vi8_data,
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v2] platform/x86: touchscreen_dmi: Add info for the CHUWI Hi10 Air tablet
From: Hans de Goede @ 2019-03-03 13:59 UTC (permalink / raw)
To: Christian Oder
Cc: andy, dvhart, linux-input, linux-kernel, platform-driver-x86
In-Reply-To: <20190303134727.6680-1-me@myself5.de>
Hi Christian,
On 03-03-19 14:47, Christian Oder wrote:
> Add touchscreen info for the CHUWUI Hi10 Air tablet.
>
> Signed-off-by: Christian Oder <me@myself5.de>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> - added review tag
A quick note for any patches you may submit in the future, there
is no need to send a v2 just to add someones Reviewed-by or Acked-by,
these will be picked up automatically by the subsys-maintainer when
they add the patch to their tree.
Regards,
Hans
>
> drivers/platform/x86/touchscreen_dmi.c | 27 ++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/platform/x86/touchscreen_dmi.c b/drivers/platform/x86/touchscreen_dmi.c
> index 167a156e3cc7..2d56ff7c8230 100644
> --- a/drivers/platform/x86/touchscreen_dmi.c
> +++ b/drivers/platform/x86/touchscreen_dmi.c
> @@ -72,6 +72,25 @@ static const struct ts_dmi_data chuwi_hi8_pro_data = {
> .properties = chuwi_hi8_pro_props,
> };
>
> +static const struct property_entry chuwi_hi10_air_props[] = {
> + PROPERTY_ENTRY_U32("touchscreen-size-x", 1981),
> + PROPERTY_ENTRY_U32("touchscreen-size-y", 1271),
> + PROPERTY_ENTRY_U32("touchscreen-min-x", 99),
> + PROPERTY_ENTRY_U32("touchscreen-min-y", 9),
> + PROPERTY_ENTRY_BOOL("touchscreen-swapped-x-y"),
> + PROPERTY_ENTRY_U32("touchscreen-fuzz-x", 5),
> + PROPERTY_ENTRY_U32("touchscreen-fuzz-y", 4),
> + PROPERTY_ENTRY_STRING("firmware-name", "gsl1680-chuwi-hi10-air.fw"),
> + PROPERTY_ENTRY_U32("silead,max-fingers", 10),
> + PROPERTY_ENTRY_BOOL("silead,home-button"),
> + { }
> +};
> +
> +static const struct ts_dmi_data chuwi_hi10_air_data = {
> + .acpi_name = "MSSL1680:00",
> + .properties = chuwi_hi10_air_props,
> +};
> +
> static const struct property_entry chuwi_vi8_props[] = {
> PROPERTY_ENTRY_U32("touchscreen-min-x", 4),
> PROPERTY_ENTRY_U32("touchscreen-min-y", 6),
> @@ -546,6 +565,14 @@ static const struct dmi_system_id touchscreen_dmi_table[] = {
> DMI_MATCH(DMI_PRODUCT_NAME, "X1D3_C806N"),
> },
> },
> + {
> + /* Chuwi Hi10 Air */
> + .driver_data = (void *)&chuwi_hi10_air_data,
> + .matches = {
> + DMI_MATCH(DMI_BOARD_VENDOR, "Hampoo"),
> + DMI_MATCH(DMI_PRODUCT_SKU, "P1W6_C109D_B"),
> + },
> + },
> {
> /* Chuwi Vi8 (CWI506) */
> .driver_data = (void *)&chuwi_vi8_data,
>
^ permalink raw reply
* [PATCH 0/9] HID: intel-ish-hid: Clean up external interfaces
From: Srinivas Pandruvada @ 2019-03-03 16:46 UTC (permalink / raw)
To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Srinivas Pandruvada
FOR kernel v5.2+.
No functional changes are expected with this series. I am posting this now
because of usage of ISH in ChromeOS Embedded Controller.
https://lkml.org/lkml/2019/2/24/26
I want to make sure that API is restricted before more development and posting
there.
Currently only one ISH client is using ISH transport. But it is changing now
with the development of other clients using ISH transport. Some of these
clients which are targeted for Linux based OS only laptops, are not using
HID to export sensors. As more clients are getting developed it is important
that the external interface for ISH transport only allows what clients need.
Currently the header files used by clients "client.h" and "ishtp-dev.h"
include other ISH header files. Also clients access fields from structure
which also has other fields which are only used by ISH transort.
So this series introduces one header file "linux/intel-ish-client-if.h".
This header files doesn't include any other ISH transport header files.
There are interface functions defined so that clients never have to directly
access any ISH transort structures.
Also clients don't have to match there GUID in probe. They will be only
probbed if their GUID matches, which is passed as driver registry.
Hong Liu (1):
HID: intel-ish-hid: Add match callback to ishtp bus type
Srinivas Pandruvada (8):
HID: intel-ish-hid: Hide members of struct ishtp_cl_device
HID: intel-ish-hid: Simplify ishtp_cl_link()
HID: intel-ish-hid: Move driver registry functions
HID: intel-ish-hid: Store ishtp_cl_device instance in device
HID: intel-ish-hid: Move the common functions from client.h
HID: intel-ish-hid: Add interface functions for struct ishtp_cl
HID: intel-ish-hid: Move functions related to bus and device
HID: intel-ish-hid: Use the new interface functions in HID ish client
drivers/hid/intel-ish-hid/ishtp-hid-client.c | 131 ++++++++++---------
drivers/hid/intel-ish-hid/ishtp-hid.c | 6 +-
drivers/hid/intel-ish-hid/ishtp-hid.h | 6 +-
drivers/hid/intel-ish-hid/ishtp/bus.c | 83 +++++++++++-
drivers/hid/intel-ish-hid/ishtp/bus.h | 37 +-----
drivers/hid/intel-ish-hid/ishtp/client.c | 60 +++++++--
drivers/hid/intel-ish-hid/ishtp/client.h | 24 ----
drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h | 31 -----
include/linux/intel-ish-client-if.h | 110 ++++++++++++++++
9 files changed, 310 insertions(+), 178 deletions(-)
create mode 100644 include/linux/intel-ish-client-if.h
--
2.17.2
^ permalink raw reply
* [PATCH 1/9] HID: intel-ish-hid: Add match callback to ishtp bus type
From: Srinivas Pandruvada @ 2019-03-03 16:46 UTC (permalink / raw)
To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Hong Liu
In-Reply-To: <20190303164654.29400-1-srinivas.pandruvada@linux.intel.com>
From: Hong Liu <hong.liu@intel.com>
Currently we depend on the guid check in ishtp_cl_driver.probe to match
the device and driver. However Linux device core first calls the match()
callback to decide the matching of driver and device, and then does some
preparation before calling the driver probe function. If we return error
in the driver probe, it needs to tear down all the preparation work and
retry with next driver.
Adding the match callback can avoid the unnecessary entry into unmatched
driver probe function for ishtp clients reported by FW.
Signed-off-by: Hong Liu <hong.liu@intel.com>
---
drivers/hid/intel-ish-hid/ishtp-hid-client.c | 5 +----
drivers/hid/intel-ish-hid/ishtp/bus.c | 21 ++++++++++++++++++++
drivers/hid/intel-ish-hid/ishtp/bus.h | 1 +
3 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
index 30fe0c5e6fad..fbb9d85ba6df 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
@@ -788,10 +788,6 @@ static int hid_ishtp_cl_probe(struct ishtp_cl_device *cl_device)
if (!cl_device)
return -ENODEV;
- if (!guid_equal(&hid_ishtp_guid,
- &cl_device->fw_client->props.protocol_name))
- return -ENODEV;
-
client_data = devm_kzalloc(&cl_device->dev, sizeof(*client_data),
GFP_KERNEL);
if (!client_data)
@@ -922,6 +918,7 @@ static const struct dev_pm_ops hid_ishtp_pm_ops = {
static struct ishtp_cl_driver hid_ishtp_cl_driver = {
.name = "ish-hid",
+ .guid = &hid_ishtp_guid,
.probe = hid_ishtp_cl_probe,
.remove = hid_ishtp_cl_remove,
.reset = hid_ishtp_cl_reset,
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c
index d5f4b6438d86..6348fee8aadc 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.c
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
@@ -219,6 +219,26 @@ static int ishtp_cl_device_probe(struct device *dev)
return driver->probe(device);
}
+/**
+ * ishtp_cl_bus_match() - Bus match() callback
+ * @dev: the device structure
+ * @drv: the driver structure
+ *
+ * This is a bus match callback, called when a new ishtp_cl_device is
+ * registered during ishtp bus client enumeration. Use the guid_t in
+ * drv and dev to decide whether they match or not.
+ *
+ * Return: 1 if dev & drv matches, 0 otherwise.
+ */
+static int ishtp_cl_bus_match(struct device *dev, struct device_driver *drv)
+{
+ struct ishtp_cl_device *device = to_ishtp_cl_device(dev);
+ struct ishtp_cl_driver *driver = to_ishtp_cl_driver(drv);
+
+ return guid_equal(driver->guid,
+ &device->fw_client->props.protocol_name);
+}
+
/**
* ishtp_cl_device_remove() - Bus remove() callback
* @dev: the device structure
@@ -372,6 +392,7 @@ static struct bus_type ishtp_cl_bus_type = {
.name = "ishtp",
.dev_groups = ishtp_cl_dev_groups,
.probe = ishtp_cl_device_probe,
+ .match = ishtp_cl_bus_match,
.remove = ishtp_cl_device_remove,
.pm = &ishtp_cl_bus_dev_pm_ops,
.uevent = ishtp_cl_uevent,
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.h b/drivers/hid/intel-ish-hid/ishtp/bus.h
index 4cf7ad586c37..c96e7fb42f01 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.h
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.h
@@ -64,6 +64,7 @@ struct ishtp_cl_device {
struct ishtp_cl_driver {
struct device_driver driver;
const char *name;
+ const guid_t *guid;
int (*probe)(struct ishtp_cl_device *dev);
int (*remove)(struct ishtp_cl_device *dev);
int (*reset)(struct ishtp_cl_device *dev);
--
2.17.2
^ permalink raw reply related
* [PATCH 2/9] HID: intel-ish-hid: Hide members of struct ishtp_cl_device
From: Srinivas Pandruvada @ 2019-03-03 16:46 UTC (permalink / raw)
To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Srinivas Pandruvada
In-Reply-To: <20190303164654.29400-1-srinivas.pandruvada@linux.intel.com>
ISH clients don't need to access any field of struct ishtp_cl_device. To
avoid this create an interface functions instead where it is required.
In the case of ishtp_cl_allocate(), modify the parameters so that the
clients don't have to dereference.
Clients can also use tracing, here a new interface is added to get the
common trace function pointer, instead of direct call.
The new interface functions defined in one external header file, named
intel-ish-client-if.h. This is the only header files all ISHTP clients
must include.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/hid/intel-ish-hid/ishtp-hid-client.c | 56 +++++++++++---------
drivers/hid/intel-ish-hid/ishtp-hid.c | 4 +-
drivers/hid/intel-ish-hid/ishtp-hid.h | 2 +-
drivers/hid/intel-ish-hid/ishtp/bus.c | 27 ++++++++++
drivers/hid/intel-ish-hid/ishtp/client.c | 4 +-
drivers/hid/intel-ish-hid/ishtp/client.h | 2 +-
include/linux/intel-ish-client-if.h | 18 +++++++
7 files changed, 84 insertions(+), 29 deletions(-)
create mode 100644 include/linux/intel-ish-client-if.h
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
index fbb9d85ba6df..ffed7c91bebf 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
@@ -15,6 +15,7 @@
#include <linux/module.h>
#include <linux/hid.h>
+#include <linux/intel-ish-client-if.h>
#include <linux/sched.h>
#include "ishtp/ishtp-dev.h"
#include "ishtp/client.h"
@@ -24,6 +25,8 @@
#define HID_CL_RX_RING_SIZE 32
#define HID_CL_TX_RING_SIZE 16
+#define cl_data_to_dev(client_data) ishtp_device(client_data->cl_device)
+
/**
* report_bad_packets() - Report bad packets
* @hid_ishtp_cl: Client instance to get stats
@@ -39,7 +42,7 @@ static void report_bad_packet(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
struct hostif_msg *recv_msg = recv_buf;
struct ishtp_cl_data *client_data = hid_ishtp_cl->client_data;
- dev_err(&client_data->cl_device->dev, "[hid-ish]: BAD packet %02X\n"
+ dev_err(cl_data_to_dev(client_data), "[hid-ish]: BAD packet %02X\n"
"total_bad=%u cur_pos=%u\n"
"[%02X %02X %02X %02X]\n"
"payload_len=%u\n"
@@ -83,7 +86,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
do {
if (cur_pos + sizeof(struct hostif_msg) > total_len) {
- dev_err(&client_data->cl_device->dev,
+ dev_err(cl_data_to_dev(client_data),
"[hid-ish]: error, received %u which is less than data header %u\n",
(unsigned int)data_len,
(unsigned int)sizeof(struct hostif_msg_hdr));
@@ -122,12 +125,12 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
client_data->hid_dev_count = (unsigned int)*payload;
if (!client_data->hid_devices)
client_data->hid_devices = devm_kcalloc(
- &client_data->cl_device->dev,
+ cl_data_to_dev(client_data),
client_data->hid_dev_count,
sizeof(struct device_info),
GFP_KERNEL);
if (!client_data->hid_devices) {
- dev_err(&client_data->cl_device->dev,
+ dev_err(cl_data_to_dev(client_data),
"Mem alloc failed for hid device info\n");
wake_up_interruptible(&client_data->init_wait);
break;
@@ -135,7 +138,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
for (i = 0; i < client_data->hid_dev_count; ++i) {
if (1 + sizeof(struct device_info) * i >=
payload_len) {
- dev_err(&client_data->cl_device->dev,
+ dev_err(cl_data_to_dev(client_data),
"[hid-ish]: [ENUM_DEVICES]: content size %zu is bigger than payload_len %zu\n",
1 + sizeof(struct device_info)
* i, payload_len);
@@ -170,7 +173,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
}
if (!client_data->hid_descr[curr_hid_dev])
client_data->hid_descr[curr_hid_dev] =
- devm_kmalloc(&client_data->cl_device->dev,
+ devm_kmalloc(cl_data_to_dev(client_data),
payload_len, GFP_KERNEL);
if (client_data->hid_descr[curr_hid_dev]) {
memcpy(client_data->hid_descr[curr_hid_dev],
@@ -195,7 +198,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
}
if (!client_data->report_descr[curr_hid_dev])
client_data->report_descr[curr_hid_dev] =
- devm_kmalloc(&client_data->cl_device->dev,
+ devm_kmalloc(cl_data_to_dev(client_data),
payload_len, GFP_KERNEL);
if (client_data->report_descr[curr_hid_dev]) {
memcpy(client_data->report_descr[curr_hid_dev],
@@ -501,12 +504,12 @@ static int ishtp_enum_enum_devices(struct ishtp_cl *hid_ishtp_cl)
sizeof(struct hostif_msg));
}
if (!client_data->enum_devices_done) {
- dev_err(&client_data->cl_device->dev,
+ dev_err(cl_data_to_dev(client_data),
"[hid-ish]: timed out waiting for enum_devices\n");
return -ETIMEDOUT;
}
if (!client_data->hid_devices) {
- dev_err(&client_data->cl_device->dev,
+ dev_err(cl_data_to_dev(client_data),
"[hid-ish]: failed to allocate HID dev structures\n");
return -ENOMEM;
}
@@ -549,13 +552,13 @@ static int ishtp_get_hid_descriptor(struct ishtp_cl *hid_ishtp_cl, int index)
client_data->hid_descr_done,
3 * HZ);
if (!client_data->hid_descr_done) {
- dev_err(&client_data->cl_device->dev,
+ dev_err(cl_data_to_dev(client_data),
"[hid-ish]: timed out for hid_descr_done\n");
return -EIO;
}
if (!client_data->hid_descr[index]) {
- dev_err(&client_data->cl_device->dev,
+ dev_err(cl_data_to_dev(client_data),
"[hid-ish]: allocation HID desc fail\n");
return -ENOMEM;
}
@@ -596,12 +599,12 @@ static int ishtp_get_report_descriptor(struct ishtp_cl *hid_ishtp_cl,
client_data->report_descr_done,
3 * HZ);
if (!client_data->report_descr_done) {
- dev_err(&client_data->cl_device->dev,
+ dev_err(cl_data_to_dev(client_data),
"[hid-ish]: timed out for report descr\n");
return -EIO;
}
if (!client_data->report_descr[index]) {
- dev_err(&client_data->cl_device->dev,
+ dev_err(cl_data_to_dev(client_data),
"[hid-ish]: failed to alloc report descr\n");
return -ENOMEM;
}
@@ -631,12 +634,12 @@ static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl, int reset)
int i;
int rv;
- dev_dbg(&client_data->cl_device->dev, "%s\n", __func__);
+ dev_dbg(cl_data_to_dev(client_data), "%s\n", __func__);
hid_ishtp_trace(client_data, "%s reset flag: %d\n", __func__, reset);
rv = ishtp_cl_link(hid_ishtp_cl, ISHTP_HOST_CLIENT_ID_ANY);
if (rv) {
- dev_err(&client_data->cl_device->dev,
+ dev_err(cl_data_to_dev(client_data),
"ishtp_cl_link failed\n");
return -ENOMEM;
}
@@ -651,7 +654,7 @@ static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl, int reset)
fw_client = ishtp_fw_cl_get_client(dev, &hid_ishtp_guid);
if (!fw_client) {
- dev_err(&client_data->cl_device->dev,
+ dev_err(cl_data_to_dev(client_data),
"ish client uuid not found\n");
return -ENOENT;
}
@@ -661,7 +664,7 @@ static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl, int reset)
rv = ishtp_cl_connect(hid_ishtp_cl);
if (rv) {
- dev_err(&client_data->cl_device->dev,
+ dev_err(cl_data_to_dev(client_data),
"client connect fail\n");
goto err_cl_unlink;
}
@@ -692,7 +695,7 @@ static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl, int reset)
if (!reset) {
rv = ishtp_hid_probe(i, client_data);
if (rv) {
- dev_err(&client_data->cl_device->dev,
+ dev_err(cl_data_to_dev(client_data),
"[hid-ish]: HID probe for #%u failed: %d\n",
i, rv);
goto err_cl_disconnect;
@@ -748,7 +751,7 @@ static void hid_ishtp_cl_reset_handler(struct work_struct *work)
hid_ishtp_cl_deinit(hid_ishtp_cl);
- hid_ishtp_cl = ishtp_cl_allocate(cl_device->ishtp_dev);
+ hid_ishtp_cl = ishtp_cl_allocate(cl_device);
if (!hid_ishtp_cl)
return;
@@ -762,15 +765,17 @@ static void hid_ishtp_cl_reset_handler(struct work_struct *work)
rv = hid_ishtp_cl_init(hid_ishtp_cl, 1);
if (!rv)
break;
- dev_err(&client_data->cl_device->dev, "Retry reset init\n");
+ dev_err(cl_data_to_dev(client_data), "Retry reset init\n");
}
if (rv) {
- dev_err(&client_data->cl_device->dev, "Reset Failed\n");
+ dev_err(cl_data_to_dev(client_data), "Reset Failed\n");
hid_ishtp_trace(client_data, "%s Failed hid_ishtp_cl %p\n",
__func__, hid_ishtp_cl);
}
}
+void (*hid_print_trace)(void *dev, const char *format, ...);
+
/**
* hid_ishtp_cl_probe() - ISHTP client driver probe
* @cl_device: ISHTP client device instance
@@ -788,12 +793,13 @@ static int hid_ishtp_cl_probe(struct ishtp_cl_device *cl_device)
if (!cl_device)
return -ENODEV;
- client_data = devm_kzalloc(&cl_device->dev, sizeof(*client_data),
+ client_data = devm_kzalloc(ishtp_device(cl_device),
+ sizeof(*client_data),
GFP_KERNEL);
if (!client_data)
return -ENOMEM;
- hid_ishtp_cl = ishtp_cl_allocate(cl_device->ishtp_dev);
+ hid_ishtp_cl = ishtp_cl_allocate(cl_device);
if (!hid_ishtp_cl)
return -ENOMEM;
@@ -807,6 +813,8 @@ static int hid_ishtp_cl_probe(struct ishtp_cl_device *cl_device)
INIT_WORK(&client_data->work, hid_ishtp_cl_reset_handler);
+ hid_print_trace = ishtp_trace_callback(cl_device);
+
rv = hid_ishtp_cl_init(hid_ishtp_cl, 0);
if (rv) {
ishtp_cl_free(hid_ishtp_cl);
@@ -833,7 +841,7 @@ static int hid_ishtp_cl_remove(struct ishtp_cl_device *cl_device)
hid_ishtp_trace(client_data, "%s hid_ishtp_cl %p\n", __func__,
hid_ishtp_cl);
- dev_dbg(&cl_device->dev, "%s\n", __func__);
+ dev_dbg(ishtp_device(cl_device), "%s\n", __func__);
hid_ishtp_cl->state = ISHTP_CL_DISCONNECTING;
ishtp_cl_disconnect(hid_ishtp_cl);
ishtp_put_device(cl_device);
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid.c b/drivers/hid/intel-ish-hid/ishtp-hid.c
index bc4c536f3c0d..cb71ad43ea3b 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid.c
@@ -14,6 +14,7 @@
*/
#include <linux/hid.h>
+#include <linux/intel-ish-client-if.h>
#include <uapi/linux/input.h>
#include "ishtp/client.h"
#include "ishtp-hid.h"
@@ -204,7 +205,8 @@ int ishtp_hid_probe(unsigned int cur_hid_dev,
hid->ll_driver = &ishtp_hid_ll_driver;
hid->bus = BUS_INTEL_ISHTP;
- hid->dev.parent = &client_data->cl_device->dev;
+ hid->dev.parent = ishtp_device(client_data->cl_device);
+
hid->version = le16_to_cpu(ISH_HID_VERSION);
hid->vendor = le16_to_cpu(client_data->hid_devices[cur_hid_dev].vid);
hid->product = le16_to_cpu(client_data->hid_devices[cur_hid_dev].pid);
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid.h b/drivers/hid/intel-ish-hid/ishtp-hid.h
index 1cd07a441cd4..e1b00f5125c7 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid.h
+++ b/drivers/hid/intel-ish-hid/ishtp-hid.h
@@ -24,7 +24,7 @@
#define IS_RESPONSE 0x80
/* Used to dump to Linux trace buffer, if enabled */
-#define hid_ishtp_trace(client, ...) \
+#define hid_ishtp_trace(client, ...) \
client->cl_device->ishtp_dev->print_log(\
client->cl_device->ishtp_dev, __VA_ARGS__)
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c
index 6348fee8aadc..308853eab91c 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.c
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
@@ -827,6 +827,33 @@ int ishtp_use_dma_transfer(void)
return ishtp_use_dma;
}
+/**
+ * ishtp_device() - Return device pointer
+ *
+ * This interface is used to return device pointer from ishtp_cl_device
+ * instance.
+ *
+ * Return: device *.
+ */
+struct device *ishtp_device(struct ishtp_cl_device *device)
+{
+ return &device->dev;
+}
+EXPORT_SYMBOL(ishtp_device);
+
+/**
+ * ishtp_trace_callback() - Return trace callback
+ *
+ * This interface is used to return trace callback function pointer.
+ *
+ * Return: void *.
+ */
+void *ishtp_trace_callback(struct ishtp_cl_device *cl_device)
+{
+ return cl_device->ishtp_dev->print_log;
+}
+EXPORT_SYMBOL(ishtp_trace_callback);
+
/**
* ishtp_bus_register() - Function to register bus
*
diff --git a/drivers/hid/intel-ish-hid/ishtp/client.c b/drivers/hid/intel-ish-hid/ishtp/client.c
index faeccdb1475b..760e48a368a8 100644
--- a/drivers/hid/intel-ish-hid/ishtp/client.c
+++ b/drivers/hid/intel-ish-hid/ishtp/client.c
@@ -126,7 +126,7 @@ static void ishtp_cl_init(struct ishtp_cl *cl, struct ishtp_device *dev)
*
* Return: The allocated client instance or NULL on failure
*/
-struct ishtp_cl *ishtp_cl_allocate(struct ishtp_device *dev)
+struct ishtp_cl *ishtp_cl_allocate(struct ishtp_cl_device *cl_device)
{
struct ishtp_cl *cl;
@@ -134,7 +134,7 @@ struct ishtp_cl *ishtp_cl_allocate(struct ishtp_device *dev)
if (!cl)
return NULL;
- ishtp_cl_init(cl, dev);
+ ishtp_cl_init(cl, cl_device->ishtp_dev);
return cl;
}
EXPORT_SYMBOL(ishtp_cl_allocate);
diff --git a/drivers/hid/intel-ish-hid/ishtp/client.h b/drivers/hid/intel-ish-hid/ishtp/client.h
index e0df3eb611e6..992625891a6c 100644
--- a/drivers/hid/intel-ish-hid/ishtp/client.h
+++ b/drivers/hid/intel-ish-hid/ishtp/client.h
@@ -170,7 +170,7 @@ static inline bool ishtp_cl_cmp_id(const struct ishtp_cl *cl1,
}
/* exported functions from ISHTP under client management scope */
-struct ishtp_cl *ishtp_cl_allocate(struct ishtp_device *dev);
+struct ishtp_cl *ishtp_cl_allocate(struct ishtp_cl_device *cl_device);
void ishtp_cl_free(struct ishtp_cl *cl);
int ishtp_cl_link(struct ishtp_cl *cl, int id);
void ishtp_cl_unlink(struct ishtp_cl *cl);
diff --git a/include/linux/intel-ish-client-if.h b/include/linux/intel-ish-client-if.h
new file mode 100644
index 000000000000..11e285172735
--- /dev/null
+++ b/include/linux/intel-ish-client-if.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Intel ISH client Interface definitions
+ *
+ * Copyright (c) 2019, Intel Corporation.
+ */
+
+#ifndef _INTEL_ISH_CLIENT_IF_H_
+#define _INTEL_ISH_CLIENT_IF_H_
+
+struct ishtp_cl_device;
+
+/* Get the device * from ishtp device instance */
+struct device *ishtp_device(struct ishtp_cl_device *cl_device);
+/* Trace interface for clients */
+void *ishtp_trace_callback(struct ishtp_cl_device *cl_device);
+
+#endif /* _INTEL_ISH_CLIENT_IF_H_ */
--
2.17.2
^ permalink raw reply related
* [PATCH 3/9] HID: intel-ish-hid: Simplify ishtp_cl_link()
From: Srinivas Pandruvada @ 2019-03-03 16:46 UTC (permalink / raw)
To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Srinivas Pandruvada
In-Reply-To: <20190303164654.29400-1-srinivas.pandruvada@linux.intel.com>
All callers will only use ISHTP_HOST_CLIENT_ID_ANY, so get rid of
option to pass this additional id.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/hid/intel-ish-hid/ishtp-hid-client.c | 2 +-
drivers/hid/intel-ish-hid/ishtp/client.c | 14 ++++----------
drivers/hid/intel-ish-hid/ishtp/client.h | 2 +-
3 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
index ffed7c91bebf..287d71338b91 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
@@ -637,7 +637,7 @@ static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl, int reset)
dev_dbg(cl_data_to_dev(client_data), "%s\n", __func__);
hid_ishtp_trace(client_data, "%s reset flag: %d\n", __func__, reset);
- rv = ishtp_cl_link(hid_ishtp_cl, ISHTP_HOST_CLIENT_ID_ANY);
+ rv = ishtp_cl_link(hid_ishtp_cl);
if (rv) {
dev_err(cl_data_to_dev(client_data),
"ishtp_cl_link failed\n");
diff --git a/drivers/hid/intel-ish-hid/ishtp/client.c b/drivers/hid/intel-ish-hid/ishtp/client.c
index 760e48a368a8..657b46dcefa6 100644
--- a/drivers/hid/intel-ish-hid/ishtp/client.c
+++ b/drivers/hid/intel-ish-hid/ishtp/client.c
@@ -168,9 +168,6 @@ EXPORT_SYMBOL(ishtp_cl_free);
/**
* ishtp_cl_link() - Reserve a host id and link the client instance
* @cl: client device instance
- * @id: host client id to use. It can be ISHTP_HOST_CLIENT_ID_ANY if any
- * id from the available can be used
- *
*
* This allocates a single bit in the hostmap. This function will make sure
* that not many client sessions are opened at the same time. Once allocated
@@ -179,11 +176,11 @@ EXPORT_SYMBOL(ishtp_cl_free);
*
* Return: 0 or error code on failure
*/
-int ishtp_cl_link(struct ishtp_cl *cl, int id)
+int ishtp_cl_link(struct ishtp_cl *cl)
{
struct ishtp_device *dev;
- unsigned long flags, flags_cl;
- int ret = 0;
+ unsigned long flags, flags_cl;
+ int id, ret = 0;
if (WARN_ON(!cl || !cl->dev))
return -EINVAL;
@@ -197,10 +194,7 @@ int ishtp_cl_link(struct ishtp_cl *cl, int id)
goto unlock_dev;
}
- /* If Id is not assigned get one*/
- if (id == ISHTP_HOST_CLIENT_ID_ANY)
- id = find_first_zero_bit(dev->host_clients_map,
- ISHTP_CLIENTS_MAX);
+ id = find_first_zero_bit(dev->host_clients_map, ISHTP_CLIENTS_MAX);
if (id >= ISHTP_CLIENTS_MAX) {
spin_unlock_irqrestore(&dev->device_lock, flags);
diff --git a/drivers/hid/intel-ish-hid/ishtp/client.h b/drivers/hid/intel-ish-hid/ishtp/client.h
index 992625891a6c..afa8b1f521d0 100644
--- a/drivers/hid/intel-ish-hid/ishtp/client.h
+++ b/drivers/hid/intel-ish-hid/ishtp/client.h
@@ -172,7 +172,7 @@ static inline bool ishtp_cl_cmp_id(const struct ishtp_cl *cl1,
/* exported functions from ISHTP under client management scope */
struct ishtp_cl *ishtp_cl_allocate(struct ishtp_cl_device *cl_device);
void ishtp_cl_free(struct ishtp_cl *cl);
-int ishtp_cl_link(struct ishtp_cl *cl, int id);
+int ishtp_cl_link(struct ishtp_cl *cl);
void ishtp_cl_unlink(struct ishtp_cl *cl);
int ishtp_cl_disconnect(struct ishtp_cl *cl);
int ishtp_cl_connect(struct ishtp_cl *cl);
--
2.17.2
^ permalink raw reply related
* [PATCH 4/9] HID: intel-ish-hid: Move driver registry functions
From: Srinivas Pandruvada @ 2019-03-03 16:46 UTC (permalink / raw)
To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Srinivas Pandruvada
In-Reply-To: <20190303164654.29400-1-srinivas.pandruvada@linux.intel.com>
Move the driver registry with the ishtp bus to the common interface
file, which clients can include.
Also rename __ishtp_cl_driver_register() to ishtp_cl_driver_register()
and removed define for ishtp_cl_driver_register.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/hid/intel-ish-hid/ishtp-hid-client.c | 2 +-
drivers/hid/intel-ish-hid/ishtp/bus.c | 8 +++---
drivers/hid/intel-ish-hid/ishtp/bus.h | 27 +-------------------
include/linux/intel-ish-client-if.h | 25 ++++++++++++++++++
4 files changed, 31 insertions(+), 31 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
index 287d71338b91..9084fea4c247 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
@@ -938,7 +938,7 @@ static int __init ish_hid_init(void)
int rv;
/* Register ISHTP client device driver with ISHTP Bus */
- rv = ishtp_cl_driver_register(&hid_ishtp_cl_driver);
+ rv = ishtp_cl_driver_register(&hid_ishtp_cl_driver, THIS_MODULE);
return rv;
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c
index 308853eab91c..95a97534fcda 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.c
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
@@ -485,7 +485,7 @@ static void ishtp_bus_remove_device(struct ishtp_cl_device *device)
}
/**
- * __ishtp_cl_driver_register() - Client driver register
+ * ishtp_cl_driver_register() - Client driver register
* @driver: the client driver instance
* @owner: Owner of this driver module
*
@@ -494,8 +494,8 @@ static void ishtp_bus_remove_device(struct ishtp_cl_device *device)
*
* Return: Return value of driver_register or -ENODEV if not ready
*/
-int __ishtp_cl_driver_register(struct ishtp_cl_driver *driver,
- struct module *owner)
+int ishtp_cl_driver_register(struct ishtp_cl_driver *driver,
+ struct module *owner)
{
int err;
@@ -512,7 +512,7 @@ int __ishtp_cl_driver_register(struct ishtp_cl_driver *driver,
return 0;
}
-EXPORT_SYMBOL(__ishtp_cl_driver_register);
+EXPORT_SYMBOL(ishtp_cl_driver_register);
/**
* ishtp_cl_driver_unregister() - Client driver unregister
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.h b/drivers/hid/intel-ish-hid/ishtp/bus.h
index c96e7fb42f01..4aed195719de 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.h
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.h
@@ -17,6 +17,7 @@
#include <linux/device.h>
#include <linux/mod_devicetable.h>
+#include <linux/intel-ish-client-if.h>
struct ishtp_cl;
struct ishtp_cl_device;
@@ -52,26 +53,6 @@ struct ishtp_cl_device {
void (*event_cb)(struct ishtp_cl_device *device);
};
-/**
- * struct ishtp_cl_device - ISHTP device handle
- * @driver: driver instance on a bus
- * @name: Name of the device for probe
- * @probe: driver callback for device probe
- * @remove: driver callback on device removal
- *
- * Client drivers defines to get probed/removed for ISHTP client device.
- */
-struct ishtp_cl_driver {
- struct device_driver driver;
- const char *name;
- const guid_t *guid;
- int (*probe)(struct ishtp_cl_device *dev);
- int (*remove)(struct ishtp_cl_device *dev);
- int (*reset)(struct ishtp_cl_device *dev);
- const struct dev_pm_ops *pm;
-};
-
-
int ishtp_bus_new_client(struct ishtp_device *dev);
void ishtp_remove_all_clients(struct ishtp_device *dev);
int ishtp_cl_device_bind(struct ishtp_cl *cl);
@@ -105,12 +86,6 @@ void ishtp_get_device(struct ishtp_cl_device *);
void ishtp_set_drvdata(struct ishtp_cl_device *cl_device, void *data);
void *ishtp_get_drvdata(struct ishtp_cl_device *cl_device);
-int __ishtp_cl_driver_register(struct ishtp_cl_driver *driver,
- struct module *owner);
-#define ishtp_cl_driver_register(driver) \
- __ishtp_cl_driver_register(driver, THIS_MODULE)
-void ishtp_cl_driver_unregister(struct ishtp_cl_driver *driver);
-
int ishtp_register_event_cb(struct ishtp_cl_device *device,
void (*read_cb)(struct ishtp_cl_device *));
int ishtp_fw_cl_by_uuid(struct ishtp_device *dev, const guid_t *cuuid);
diff --git a/include/linux/intel-ish-client-if.h b/include/linux/intel-ish-client-if.h
index 11e285172735..abc0b8122f07 100644
--- a/include/linux/intel-ish-client-if.h
+++ b/include/linux/intel-ish-client-if.h
@@ -10,6 +10,31 @@
struct ishtp_cl_device;
+/**
+ * struct ishtp_cl_device - ISHTP device handle
+ * @driver: driver instance on a bus
+ * @name: Name of the device for probe
+ * @probe: driver callback for device probe
+ * @remove: driver callback on device removal
+ *
+ * Client drivers defines to get probed/removed for ISHTP client device.
+ */
+struct ishtp_cl_driver {
+ struct device_driver driver;
+ const char *name;
+ const guid_t *guid;
+ int (*probe)(struct ishtp_cl_device *dev);
+ int (*remove)(struct ishtp_cl_device *dev);
+ int (*reset)(struct ishtp_cl_device *dev);
+ const struct dev_pm_ops *pm;
+};
+
+int ishtp_cl_driver_register(struct ishtp_cl_driver *driver,
+ struct module *owner);
+void ishtp_cl_driver_unregister(struct ishtp_cl_driver *driver);
+int ishtp_register_event_cb(struct ishtp_cl_device *device,
+ void (*read_cb)(struct ishtp_cl_device *));
+
/* Get the device * from ishtp device instance */
struct device *ishtp_device(struct ishtp_cl_device *cl_device);
/* Trace interface for clients */
--
2.17.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox