* Re: [PATCH 1/2] Input: synaptics-rmi4 - clear irqs before set irqs
From: Aaron Ma @ 2019-03-28 6:02 UTC (permalink / raw)
To: Christopher Heiny, dmitry.torokhov@gmail.com,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
Andrew Duggan, benjamin.tissoires@redhat.com
In-Reply-To: <a0f21198-5715-8858-8368-51e43092181e@canonical.com>
Hi Dmitry and Chiristopher:
Do you have any suggestion about these 2 patches?
Many users confirmed that they fixed issues of Trackpoint/Touchpad after S3.
Will you consider them be accepted?
Thanks,
Aaron
^ permalink raw reply
* Re: [PATCH v3 3/4] driver core: add dev_print_hex_dump() logging function.
From: Greg Kroah-Hartman @ 2019-03-28 5:29 UTC (permalink / raw)
To: Life is hard, and then you die
Cc: Dmitry Torokhov, Henrik Rydberg, Andy Shevchenko,
Sergey Senozhatsky, Steven Rostedt, Rafael J. Wysocki,
Lukas Wunner, Federico Lorenzi, linux-input, linux-kernel
In-Reply-To: <20190328002817.GF24753@innovation.ch>
On Wed, Mar 27, 2019 at 05:28:17PM -0700, Life is hard, and then you die wrote:
>
> On Wed, Mar 27, 2019 at 11:37:57AM +0900, Greg Kroah-Hartman wrote:
> > On Tue, Mar 26, 2019 at 06:48:06PM -0700, Ronald Tschalär wrote:
> > > This is the dev_xxx() analog to print_hex_dump(), using dev_printk()
> > > instead of straight printk() to match other dev_xxx() logging functions.
> > > ---
> > > drivers/base/core.c | 43 ++++++++++++++++++++++++++++++++++++++++++
> > > include/linux/device.h | 15 +++++++++++++++
> > > 2 files changed, 58 insertions(+)
> >
> > No signed-off-by?
>
> Aargh! Apologies, fixed for the future.
>
> > Anyway, no, please do not do this. Please do not dump large hex values
> > like this to the kernel log, it does not help anyone.
> >
> > You can do this while debugging, sure, but not for "real" kernel code.
>
> As used by this driver, it is definitely called for debugging only and
> must be explicitly enabled via a module param. But having the ability
> for folks to easily generate and print out debugging info has proven
> quite valuable.
We have dynamic debugging, no need for module parameters at all. This
isn't the 1990's anymore :)
thanks,
greg k-h
^ permalink raw reply
* Re: INFO: task hung in evdev_release
From: syzbot @ 2019-03-28 1:24 UTC (permalink / raw)
To: alsa-devel, broonie, devicetree, dmitry.torokhov, jbrunet,
lgirdwood, linux-input, linux-kernel, mark.rutland, robh+dt,
rydberg, syzkaller-bugs
In-Reply-To: <000000000000f1be430578524a20@google.com>
syzbot has bisected this bug to:
commit e32d99af6830c9a8f37b4f2637ef0cdc60fa79fb
Author: Jerome Brunet <jbrunet@baylibre.com>
Date: Tue Jul 17 15:42:50 2018 +0000
ASoC: meson: add axg fifos DT binding documentation
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=13fba69b200000
start commit: 9dcd936c Merge tag 'for-4.19/dm-fixes-4' of git://git.kern..
git tree: upstream
final crash: https://syzkaller.appspot.com/x/report.txt?x=1007a69b200000
console output: https://syzkaller.appspot.com/x/log.txt?x=17fba69b200000
kernel config: https://syzkaller.appspot.com/x/.config?x=88e9a8a39dc0be2d
dashboard link: https://syzkaller.appspot.com/bug?extid=a979743610b4755d4d57
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10fcd826400000
Reported-by: syzbot+a979743610b4755d4d57@syzkaller.appspotmail.com
Fixes: e32d99af6830 ("ASoC: meson: add axg fifos DT binding documentation")
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
^ permalink raw reply
* Re: [PATCH v2] drm/bridge: sil_sii8620: make remote control optional.
From: Life is hard, and then you die @ 2019-03-28 1:07 UTC (permalink / raw)
To: Andrzej Hajda
Cc: Inki Dae, Laurent Pinchart, Dmitry Torokhov, Lukas Wunner,
dri-devel, linux-input, linux-kernel
In-Reply-To: <d7e8493b-a1fc-3f7d-c715-5f72f15f9320@samsung.com>
On Mon, Jan 28, 2019 at 11:53:38AM +0100, Andrzej Hajda wrote:
> On 25.01.2019 02:33, Ronald Tschalär wrote:
> > commit d6abe6df706c (drm/bridge: sil_sii8620: do not have a dependency
> > of RC_CORE) changed the driver to select both RC_CORE and INPUT.
> > However, this causes problems with other drivers, in particular an input
> > driver that depends on MFD_INTEL_LPSS_PCI (to be added in a separate
> > commit):
> >
> > drivers/clk/Kconfig:9:error: recursive dependency detected!
> > drivers/clk/Kconfig:9: symbol COMMON_CLK is selected by MFD_INTEL_LPSS
> > drivers/mfd/Kconfig:566: symbol MFD_INTEL_LPSS is selected by MFD_INTEL_LPSS_PCI
> > drivers/mfd/Kconfig:580: symbol MFD_INTEL_LPSS_PCI is implied by KEYBOARD_APPLESPI
> > drivers/input/keyboard/Kconfig:73: symbol KEYBOARD_APPLESPI depends on INPUT
> > drivers/input/Kconfig:8: symbol INPUT is selected by DRM_SIL_SII8620
> > drivers/gpu/drm/bridge/Kconfig:83: symbol DRM_SIL_SII8620 depends on DRM_BRIDGE
> > drivers/gpu/drm/bridge/Kconfig:1: symbol DRM_BRIDGE is selected by DRM_PL111
> > drivers/gpu/drm/pl111/Kconfig:1: symbol DRM_PL111 depends on COMMON_CLK
> >
> > According to the docs and general consensus, select should only be used
> > for non user-visible symbols, but both RC_CORE and INPUT are
> > user-visible. Furthermore almost all other references to INPUT
> > throughout the kernel config are depends, not selects. For this reason
> > the first part of this change reverts commit d6abe6df706c.
> >
> > In order to address the original reason for commit d6abe6df706c, namely
> > that not all boards use the remote controller functionality and hence
> > should not need have to deal with RC_CORE, the second part of this
> > change now makes the remote control support in the driver optional and
> > contingent on RC_CORE being defined. And with this the hard dependency
> > on INPUT also goes away as that is only needed if RC_CORE is defined
> > (which in turn already depends on INPUT).
>
>
> Thanks for fixing it, this seems to be a best way to deal with it, more
> comments below.
Sorry I didn't respond to this earlier, but since I wasn't on the
to/cc list and I neglected to check the archives I completely missed
it.
> > CC: Inki Dae <inki.dae@samsung.com>
> > CC: Andrzej Hajda <a.hajda@samsung.com>
> > CC: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
> > ---
> > Resending this, as I somehow managed to forget to cc dri-devel.
> > Apologies for the duplication.
> >
> > Changes in v2:
> > - completely remove dependencies on both RC_CORE and INPUT in Kconfig,
> > - make remote control functionality in driver contingent on RC_CORE
> > being defined
> >
> > drivers/gpu/drm/bridge/Kconfig | 2 --
> > drivers/gpu/drm/bridge/sil-sii8620.c | 17 +++++++++++++++++
> > 2 files changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> > index 2fee47b0d50b..a11198a36005 100644
> > --- a/drivers/gpu/drm/bridge/Kconfig
> > +++ b/drivers/gpu/drm/bridge/Kconfig
> > @@ -85,8 +85,6 @@ config DRM_SIL_SII8620
> > depends on OF
> > select DRM_KMS_HELPER
> > imply EXTCON
> > - select INPUT
> > - select RC_CORE
>
> Shouldn't you put here "imply RC_CORE"? To avoid RC_CORE as module and
> sii8620 built-in.
Good point - fixed.
> > help
> > Silicon Image SII8620 HDMI/MHL bridge chip driver.
> >
> > diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
> > index 0cc293a6ac24..dee47752791e 100644
> > --- a/drivers/gpu/drm/bridge/sil-sii8620.c
> > +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
> > @@ -66,7 +66,9 @@ enum sii8620_mt_state {
> > struct sii8620 {
> > struct drm_bridge bridge;
> > struct device *dev;
> > +#if IS_ENABLED(CONFIG_RC_CORE)
> > struct rc_dev *rc_dev;
> > +#endif
> > struct clk *clk_xtal;
> > struct gpio_desc *gpio_reset;
> > struct gpio_desc *gpio_int;
> > @@ -1756,6 +1758,7 @@ static void sii8620_send_features(struct sii8620 *ctx)
> > sii8620_write_buf(ctx, REG_MDT_XMIT_WRITE_PORT, buf, ARRAY_SIZE(buf));
> > }
> >
> > +#if IS_ENABLED(CONFIG_RC_CORE)
> > static bool sii8620_rcp_consume(struct sii8620 *ctx, u8 scancode)
> > {
> > bool pressed = !(scancode & MHL_RCP_KEY_RELEASED_MASK);
> > @@ -1774,6 +1777,12 @@ static bool sii8620_rcp_consume(struct sii8620 *ctx, u8 scancode)
> >
> > return true;
> > }
> > +#else
> > +static bool sii8620_rcp_consume(struct sii8620 *ctx, u8 scancode)
> > +{
> > + return false;
> > +}
> > +#endif
> >
> > static void sii8620_msc_mr_set_int(struct sii8620 *ctx)
> > {
> > @@ -2097,6 +2106,7 @@ static void sii8620_cable_in(struct sii8620 *ctx)
> > enable_irq(to_i2c_client(ctx->dev)->irq);
> > }
> >
> > +#if IS_ENABLED(CONFIG_RC_CORE)
> > static void sii8620_init_rcp_input_dev(struct sii8620 *ctx)
> > {
> > struct rc_dev *rc_dev;
> > @@ -2126,6 +2136,11 @@ static void sii8620_init_rcp_input_dev(struct sii8620 *ctx)
> > }
> > ctx->rc_dev = rc_dev;
> > }
> > +#else
> > +static void sii8620_init_rcp_input_dev(struct sii8620 *ctx)
> > +{
> > +}
> > +#endif
> >
> > static void sii8620_cable_out(struct sii8620 *ctx)
> > {
> > @@ -2214,9 +2229,11 @@ static int sii8620_attach(struct drm_bridge *bridge)
> >
> > static void sii8620_detach(struct drm_bridge *bridge)
> > {
> > +#if IS_ENABLED(CONFIG_RC_CORE)
> > struct sii8620 *ctx = bridge_to_sii8620(bridge);
> >
> > rc_unregister_device(ctx->rc_dev);
> > +#endif
> > }
>
> In two cases you create stub functions, in the third you put ifdefs
> inside function, some lack of consistency.
Agreed - I'll make a stub for sii8620_detach() too.
> I wonder if it wouldn't be better to create stubs just for rc_core
> functions, the best would be to put stubs into rc-core.h, but if the
> case of 'optional rc-core' is too rare you can put stubs into the driver:
>
> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c
> b/drivers/gpu/drm/bridge/sil-sii8620.c
>
> index a6e8f4591e63..6ca838d30f93 100644
> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
> @@ -35,6 +35,13 @@
>
> #include "sil-sii8620.h"
>
> +#if !IS_ENABLED(CONFIG_RC_CORE)
> +#define rc_unregister_device(dev) ({ (void)(dev); })
> +#define rc_allocate_device(type) ({ NULL; })
> +#define rc_keydown(...) ({ })
> +#define rc_keyup(...) ({ })
> +#endif
> +
> #define SII8620_BURST_BUF_LEN 288
> #define VAL_RX_HDMI_CTRL2_DEFVAL VAL_RX_HDMI_CTRL2_IDLE_CNT(3)
The reason I didn't do this was because it leads to unwanted log
messages: in particular in sii8620_init_rcp_input_dev() the failed
allocation results in an ugly dev_err() message; in the case of
sii8620_rcp_consume() it's a much more benign dev_dbg() though.
But if stubs for rc-core is the preferred approach, I'll certainly
do that instead.
As to how rare 'optional rc-core' is, the only other place I've found
is in drivers/media/usb/dvb-usb-v2/, and there the approach is to #if
out the whole rc related code and stub out the single entry point to
it (get_rc_config). So for now it looks like use for rc-core stubs
would be restricted to this module (if we go that route).
Cheers,
Ronald
^ permalink raw reply
* Re: [PATCH v3 2/4] lib/hexdump.c: factor out generic hexdump formatting for reuse.
From: Life is hard, and then you die @ 2019-03-28 0:34 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Dmitry Torokhov, Henrik Rydberg, Andy Shevchenko,
Sergey Senozhatsky, Steven Rostedt, Greg Kroah-Hartman,
Rafael J. Wysocki, Lukas Wunner, Federico Lorenzi, linux-input,
Linux Kernel Mailing List
In-Reply-To: <CAHp75VebbroyOt6DPnqKVk99RowZsxgjuHbi6HoXf-tAuZLMqA@mail.gmail.com>
On Wed, Mar 27, 2019 at 09:46:48AM +0200, Andy Shevchenko wrote:
> On Wed, Mar 27, 2019 at 3:49 AM Ronald Tschalär <ronald@innovation.ch> wrote:
> >
> > This introduces print_hex_dump_to_cb() which contains all the hexdump
> > formatting minus the actual printk() call, allowing an arbitrary print
> > function to be supplied instead. And print_hex_dump() is re-implemented
> > using print_hex_dump_to_cb().
> >
> > This allows other hex-dump logging functions to be provided which call
> > printk() differently or even log the hexdump somewhere entirely
> > different.
>
> No Sign-off?
Yeah, my screwup.
> In any case, don't do it like this. smaller non-recursive printf() is
> better than one big receursive call.
> When it looks like an optimization, it's actually a regression.
Not sure where you see recursion here - are you referring to the
callback approach? Since dev_printk() ends up calling printk with a
dictionary as well as additional formatting, vs print_hex_dump()'s
stright use of printk, this seemed like the best way accommodate
various possible ways of logging the messages. But as per below I
guess this is moot.
> And yes, debugfs idea is not bad.
So it seems like that is the consensus. As per my other response, I'll
do this then and leave the print_hex_dump() alone.
> P.S. Also check %*ph specifier.
Thanks!
Cheers,
Ronald
^ permalink raw reply
* Re: [PATCH v3 3/4] driver core: add dev_print_hex_dump() logging function.
From: Life is hard, and then you die @ 2019-03-28 0:28 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Dmitry Torokhov, Henrik Rydberg, Andy Shevchenko,
Sergey Senozhatsky, Steven Rostedt, Rafael J. Wysocki,
Lukas Wunner, Federico Lorenzi, linux-input, linux-kernel
In-Reply-To: <20190327023757.GB20766@kroah.com>
On Wed, Mar 27, 2019 at 11:37:57AM +0900, Greg Kroah-Hartman wrote:
> On Tue, Mar 26, 2019 at 06:48:06PM -0700, Ronald Tschalär wrote:
> > This is the dev_xxx() analog to print_hex_dump(), using dev_printk()
> > instead of straight printk() to match other dev_xxx() logging functions.
> > ---
> > drivers/base/core.c | 43 ++++++++++++++++++++++++++++++++++++++++++
> > include/linux/device.h | 15 +++++++++++++++
> > 2 files changed, 58 insertions(+)
>
> No signed-off-by?
Aargh! Apologies, fixed for the future.
> Anyway, no, please do not do this. Please do not dump large hex values
> like this to the kernel log, it does not help anyone.
>
> You can do this while debugging, sure, but not for "real" kernel code.
As used by this driver, it is definitely called for debugging only and
must be explicitly enabled via a module param. But having the ability
for folks to easily generate and print out debugging info has proven
quite valuable.
> Worst case, just create a debugfs file for your device that you can read
> the binary data from if you really need it. For any "normal" operation,
> this is not something that you should ever need.
Ok, can do that.
I'll retract the two print_hex_dump related patches then.
Cheers,
Ronald
^ permalink raw reply
* RE: [PATCH v4] HID: core: move Usage Page concatenation to Main item
From: Junge, Terry @ 2019-03-28 0:27 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: <20190327101848.25477-1-nsaenzjulienne@suse.de>
Hi Nicolas,
V4 looks good to me.
Thanks,
Terry
>-----Original Message-----
>Subject: [PATCH v4] HID: core: move Usage Page concatenation to Main item
>
^ permalink raw reply
* Re: [PATCH v3 4/4] Input: add Apple SPI keyboard and trackpad driver.
From: Life is hard, and then you die @ 2019-03-28 0:24 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Dmitry Torokhov, Henrik Rydberg, Sergey Senozhatsky,
Steven Rostedt, Greg Kroah-Hartman, Rafael J. Wysocki,
Lukas Wunner, Federico Lorenzi, linux-input, linux-kernel
In-Reply-To: <20190327093530.GH9224@smile.fi.intel.com>
On Wed, Mar 27, 2019 at 11:35:30AM +0200, Andy Shevchenko wrote:
> On Tue, Mar 26, 2019 at 06:48:07PM -0700, 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.
[snip]
> > +#include <asm/barrier.h>
>
> > +#include <asm-generic/unaligned.h>
>
> generic?!
>
> #include <asm/unaligned.h>
> should work.
Yes, you're right. I brought myself up-to-speed now on the difference
between the two.
> > +static const char *applespi_debug_facility(unsigned int log_mask)
> > +{
> > + switch (log_mask) {
> > + case DBG_CMD_TP_INI:
> > + return "Touchpad Initialization";
> > + case DBG_CMD_BL:
> > + return "Backlight Command";
> > + case DBG_CMD_CL:
> > + return "Caps-Lock Command";
> > + case DBG_RD_KEYB:
> > + return "Keyboard Event";
> > + case DBG_RD_TPAD:
> > + return "Touchpad Event";
> > + case DBG_RD_UNKN:
> > + return "Unknown Event";
> > + case DBG_RD_IRQ:
> > + return "Interrupt Request";
> > + case DBG_RD_CRC:
> > + return "Corrupted packet";
> > + case DBG_TP_DIM:
> > + return "Touchpad Dimensions";
> > + default:
>
> > + return "-Unrecognized log mask-";
>
> I don't think '-' surroundings are needed, but this is rather minor. Up to you.
I've used that to distinguish an error value from normal values; but
that's not an idiom used in the kernel AFAICT, so I'll remove them.
Thanks.
Cheers,
Ronald
^ permalink raw reply
* Re: [PATCH v3 1/4] drm/bridge: sil_sii8620: depend on INPUT instead of selecting it.
From: Life is hard, and then you die @ 2019-03-28 0:07 UTC (permalink / raw)
To: Andrzej Hajda
Cc: Dmitry Torokhov, Henrik Rydberg, Andy Shevchenko,
Sergey Senozhatsky, Steven Rostedt, Greg Kroah-Hartman,
Rafael J. Wysocki, Lukas Wunner, Federico Lorenzi, linux-input,
linux-kernel, Inki Dae, dri-devel@lists.freedesktop.org
In-Reply-To: <9fd8af5b-20ed-5dca-7d5c-98f2926b9b0c@samsung.com>
Hi Andrzej,
On Wed, Mar 27, 2019 at 03:13:37PM +0100, Andrzej Hajda wrote:
> +cc: dri-devel
>
> On 27.03.2019 02:48, Ronald Tschalär wrote:
> > commit d6abe6df706c66d803e6dd4fe98c1b6b7f125a56 (drm/bridge:
> > sil_sii8620: do not have a dependency of RC_CORE) added a dependency on
> > INPUT. However, this causes problems with other drivers, in particular
> > an input driver that depends on MFD_INTEL_LPSS_PCI (to be added in a
> > future commit):
> >
> > drivers/clk/Kconfig:9:error: recursive dependency detected!
> > drivers/clk/Kconfig:9: symbol COMMON_CLK is selected by MFD_INTEL_LPSS
> > drivers/mfd/Kconfig:566: symbol MFD_INTEL_LPSS is selected by MFD_INTEL_LPSS_PCI
> > drivers/mfd/Kconfig:580: symbol MFD_INTEL_LPSS_PCI is implied by KEYBOARD_APPLESPI
> > drivers/input/keyboard/Kconfig:73: symbol KEYBOARD_APPLESPI depends on INPUT
> > drivers/input/Kconfig:8: symbol INPUT is selected by DRM_SIL_SII8620
> > drivers/gpu/drm/bridge/Kconfig:83: symbol DRM_SIL_SII8620 depends on DRM_BRIDGE
> > drivers/gpu/drm/bridge/Kconfig:1: symbol DRM_BRIDGE is selected by DRM_PL111
> > drivers/gpu/drm/pl111/Kconfig:1: symbol DRM_PL111 depends on COMMON_CLK
> >
> > According to the docs, select should only be used for non-visible
> > symbols. Furthermore almost all other references to INPUT throughout the
> > kernel config are depends, not selects. Hence this change.
> >
> > CC: Inki Dae <inki.dae@samsung.com>
> > CC: Andrzej Hajda <a.hajda@samsung.com>
> > Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
>
> This is drm bridge driver, next time please cc it to dri-devel ML also.
Ok. Though as noted in the cover letter, the patch here is meant as a
placeholder till the real thing being discussed on dri-devel is
finalized. I was trying to avoid cross-posting too much, hence the
separate submission on dri-devel.
> Anyway this is not the solution we have agreed to.
>
> Why have you abandoned the patch [1]? It needed just some minor
> polishing, as I wrote in response to this mail.
>
> [1]:
> https://lore.kernel.org/lkml/20190124082423.23139-1-ronald@innovation.ch/T/#mf620df0b1583096a214d8e2e690387078583472f
It seems your mail client doesn't like me :-) I got neither of your
responses. Sorry, I should've checked the archives when I didn't hear
anything. In any case thank you for your review, and I will update
that patch and send out a new version shortly.
Cheers,
Ronald
> > ---
> > drivers/gpu/drm/bridge/Kconfig | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> > index 2fee47b0d50b..eabedc83f25c 100644
> > --- a/drivers/gpu/drm/bridge/Kconfig
> > +++ b/drivers/gpu/drm/bridge/Kconfig
> > @@ -83,9 +83,9 @@ config DRM_PARADE_PS8622
> > config DRM_SIL_SII8620
> > tristate "Silicon Image SII8620 HDMI/MHL bridge"
> > depends on OF
> > + depends on INPUT
> > select DRM_KMS_HELPER
> > imply EXTCON
> > - select INPUT
> > select RC_CORE
> > help
> > Silicon Image SII8620 HDMI/MHL bridge chip driver.
>
>
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: input: add GPIO controllable vibrator
From: Rob Herring @ 2019-03-27 19:43 UTC (permalink / raw)
To: Luca Weiss
Cc: Dmitry Torokhov, Mark Rutland,
open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
In-Reply-To: <20190302141132.21160-1-luca@z3ntu.xyz>
On Sat, Mar 02, 2019 at 03:11:30PM +0100, Luca Weiss wrote:
> 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.
Need to wrap line.
> +
> +Required properties:
> +- compatible: should contain "gpio-vibrator"
> +- enable-gpios: Should contain a GPIO handle
> +- vcc-supply: Phandle for the regulator supplying power
This should probably be optional. There may not be a s/w controllable
supply.
> +
> +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
* Re: [PATCH v3 4/4] Input: add Apple SPI keyboard and trackpad driver.
From: Andy Shevchenko @ 2019-03-27 19:22 UTC (permalink / raw)
To: Greg Kroah-Hartman, Bartosz Golaszewski
Cc: Ronald Tschalär, Dmitry Torokhov, Henrik Rydberg,
Sergey Senozhatsky, Steven Rostedt, Rafael J. Wysocki,
Lukas Wunner, Federico Lorenzi, linux-input, linux-kernel
In-Reply-To: <20190327184526.GA11095@kroah.com>
On Wed, Mar 27, 2019 at 07:45:26PM +0100, Greg Kroah-Hartman wrote:
> On Wed, Mar 27, 2019 at 11:35:30AM +0200, Andy Shevchenko wrote:
> > On Tue, Mar 26, 2019 at 06:48:07PM -0700, 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.
> >
> > > +// SPDX-License-Identifier: GPL-2.0
> >
> > According to last changes this should be GPL-2.0-only
>
> What "last changes"? "GPL-2.0" is a totally valid SPDX identifier for
> the kernel. Don't buy into the "-only" prefix crud that the newer SPDX
> version adopted for crazy reasons. The in-kernel documentation lists
> the valid identifiers and the version of SPDX we are currently using.
Thanks for this clarification.
// offtopic
Bartosz, so, it seems my first mail has been a correct one: in-kernel
documentation clarifies things for kernel.
// offtopic
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v3 4/4] Input: add Apple SPI keyboard and trackpad driver.
From: Steven Rostedt @ 2019-03-27 19:15 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Andy Shevchenko, Ronald Tschalär, Dmitry Torokhov,
Henrik Rydberg, Sergey Senozhatsky, Rafael J. Wysocki,
Lukas Wunner, Federico Lorenzi, linux-input, linux-kernel
In-Reply-To: <20190327184526.GA11095@kroah.com>
On Wed, 27 Mar 2019 19:45:26 +0100
Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> > > +// SPDX-License-Identifier: GPL-2.0
> >
> > According to last changes this should be GPL-2.0-only
>
> What "last changes"? "GPL-2.0" is a totally valid SPDX identifier for
> the kernel. Don't buy into the "-only" prefix crud that the newer SPDX
> version adopted for crazy reasons. The in-kernel documentation lists
> the valid identifiers and the version of SPDX we are currently using.
According to: LICENSES/preferred/GPL-2.0
> SPDX-URL: https://spdx.org/licenses/GPL-2.0.html
> Usage-Guide:
> To use this license in source code, put one of the following SPDX
> tag/value pairs into a comment according to the placement
> guidelines in the licensing rules documentation.
> For 'GNU General Public License (GPL) version 2 only' use:
> SPDX-License-Identifier: GPL-2.0
> or
> SPDX-License-Identifier: GPL-2.0-only
> For 'GNU General Public License (GPL) version 2 or any later version' use:
> SPDX-License-Identifier: GPL-2.0+
> or
> SPDX-License-Identifier: GPL-2.0-or-later
So, GPL-2.0 is the same as GPL-2.0-only. Changing it from one to the
other doesn't make any difference.
-- Steve
^ permalink raw reply
* Re: [PATCH v3 4/4] Input: add Apple SPI keyboard and trackpad driver.
From: Greg Kroah-Hartman @ 2019-03-27 18:45 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Ronald Tschalär, Dmitry Torokhov, Henrik Rydberg,
Sergey Senozhatsky, Steven Rostedt, Rafael J. Wysocki,
Lukas Wunner, Federico Lorenzi, linux-input, linux-kernel
In-Reply-To: <20190327093530.GH9224@smile.fi.intel.com>
On Wed, Mar 27, 2019 at 11:35:30AM +0200, Andy Shevchenko wrote:
> On Tue, Mar 26, 2019 at 06:48:07PM -0700, 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.
>
> > +// SPDX-License-Identifier: GPL-2.0
>
> According to last changes this should be GPL-2.0-only
What "last changes"? "GPL-2.0" is a totally valid SPDX identifier for
the kernel. Don't buy into the "-only" prefix crud that the newer SPDX
version adopted for crazy reasons. The in-kernel documentation lists
the valid identifiers and the version of SPDX we are currently using.
thanks,
greg k-h
^ permalink raw reply
* [PATCH AUTOSEL 4.9 72/87] HID: intel-ish: ipc: handle PIMR before ish_wakeup also clear PISR busy_clear bit
From: Sasha Levin @ 2019-03-27 18:20 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Song Hongyan, Jiri Kosina, Sasha Levin, linux-input
In-Reply-To: <20190327182040.17444-1-sashal@kernel.org>
From: Song Hongyan <hongyan.song@intel.com>
[ Upstream commit 2edefc056e4f0e6ec9508dd1aca2c18fa320efef ]
Host driver should handle interrupt mask register earlier than wake up ish FW
else there will be conditions when FW interrupt comes, host PIMR register still
not set ready, so move the interrupt mask setting before ish_wakeup.
Clear PISR busy_clear bit in ish_irq_handler. If not clear, there will be
conditions host driver received a busy_clear interrupt (before the busy_clear
mask bit is ready), it will return IRQ_NONE after check_generated_interrupt,
the interrupt will never be cleared, causing the DEVICE not sending following
IRQ.
Since PISR clear should not be called for the CHV device we do this change.
After the change, both ISH2HOST interrupt and busy_clear interrupt will be
considered as interrupt from ISH, busy_clear interrupt will return IRQ_HANDLED
from IPC_IS_BUSY check.
Signed-off-by: Song Hongyan <hongyan.song@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/intel-ish-hid/ipc/ipc.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ipc/ipc.c b/drivers/hid/intel-ish-hid/ipc/ipc.c
index 0c9ac4d5d850..41d44536aa15 100644
--- a/drivers/hid/intel-ish-hid/ipc/ipc.c
+++ b/drivers/hid/intel-ish-hid/ipc/ipc.c
@@ -92,7 +92,10 @@ static bool check_generated_interrupt(struct ishtp_device *dev)
IPC_INT_FROM_ISH_TO_HOST_CHV_AB(pisr_val);
} else {
pisr_val = ish_reg_read(dev, IPC_REG_PISR_BXT);
- interrupt_generated = IPC_INT_FROM_ISH_TO_HOST_BXT(pisr_val);
+ interrupt_generated = !!pisr_val;
+ /* only busy-clear bit is RW, others are RO */
+ if (pisr_val)
+ ish_reg_write(dev, IPC_REG_PISR_BXT, pisr_val);
}
return interrupt_generated;
@@ -795,11 +798,11 @@ int ish_hw_start(struct ishtp_device *dev)
{
ish_set_host_rdy(dev);
+ set_host_ready(dev);
+
/* After that we can enable ISH DMA operation and wakeup ISHFW */
ish_wakeup(dev);
- set_host_ready(dev);
-
/* wait for FW-initiated reset flow */
if (!dev->recvd_hw_ready)
wait_event_interruptible_timeout(dev->wait_hw_ready,
--
2.19.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.9 44/87] HID: intel-ish-hid: avoid binding wrong ishtp_cl_device
From: Sasha Levin @ 2019-03-27 18:19 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Hong Liu, Jiri Kosina, Sasha Levin, linux-input
In-Reply-To: <20190327182040.17444-1-sashal@kernel.org>
From: Hong Liu <hong.liu@intel.com>
[ Upstream commit 0d28f49412405d87d3aae83da255070a46e67627 ]
When performing a warm reset in ishtp bus driver, the ishtp_cl_device
will not be removed, its fw_client still points to the already freed
ishtp_device.fw_clients array.
Later after driver finishing ishtp client enumeration, this dangling
pointer may cause driver to bind the wrong ishtp_cl_device to the new
client, causing wrong callback to be called for messages intended for
the new client.
This helps in development of firmware where frequent switching of
firmwares is required without Linux reboot.
Signed-off-by: Hong Liu <hong.liu@intel.com>
Tested-by: Hongyan Song <hongyan.song@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/intel-ish-hid/ishtp/bus.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c
index 256521509d20..0de18c76f8d4 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.c
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
@@ -628,7 +628,8 @@ int ishtp_cl_device_bind(struct ishtp_cl *cl)
spin_lock_irqsave(&cl->dev->device_list_lock, flags);
list_for_each_entry(cl_device, &cl->dev->device_list,
device_link) {
- if (cl_device->fw_client->client_id == cl->fw_client_id) {
+ if (cl_device->fw_client &&
+ cl_device->fw_client->client_id == cl->fw_client_id) {
cl->device = cl_device;
rv = 0;
break;
@@ -688,6 +689,7 @@ void ishtp_bus_remove_all_clients(struct ishtp_device *ishtp_dev,
spin_lock_irqsave(&ishtp_dev->device_list_lock, flags);
list_for_each_entry_safe(cl_device, n, &ishtp_dev->device_list,
device_link) {
+ cl_device->fw_client = NULL;
if (warm_reset && cl_device->reference_count)
continue;
--
2.19.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.14 103/123] HID: intel-ish: ipc: handle PIMR before ish_wakeup also clear PISR busy_clear bit
From: Sasha Levin @ 2019-03-27 18:16 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Song Hongyan, Jiri Kosina, Sasha Levin, linux-input
In-Reply-To: <20190327181628.15899-1-sashal@kernel.org>
From: Song Hongyan <hongyan.song@intel.com>
[ Upstream commit 2edefc056e4f0e6ec9508dd1aca2c18fa320efef ]
Host driver should handle interrupt mask register earlier than wake up ish FW
else there will be conditions when FW interrupt comes, host PIMR register still
not set ready, so move the interrupt mask setting before ish_wakeup.
Clear PISR busy_clear bit in ish_irq_handler. If not clear, there will be
conditions host driver received a busy_clear interrupt (before the busy_clear
mask bit is ready), it will return IRQ_NONE after check_generated_interrupt,
the interrupt will never be cleared, causing the DEVICE not sending following
IRQ.
Since PISR clear should not be called for the CHV device we do this change.
After the change, both ISH2HOST interrupt and busy_clear interrupt will be
considered as interrupt from ISH, busy_clear interrupt will return IRQ_HANDLED
from IPC_IS_BUSY check.
Signed-off-by: Song Hongyan <hongyan.song@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/intel-ish-hid/ipc/ipc.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ipc/ipc.c b/drivers/hid/intel-ish-hid/ipc/ipc.c
index 9a60ec13cb10..a3106fcc2253 100644
--- a/drivers/hid/intel-ish-hid/ipc/ipc.c
+++ b/drivers/hid/intel-ish-hid/ipc/ipc.c
@@ -91,7 +91,10 @@ static bool check_generated_interrupt(struct ishtp_device *dev)
IPC_INT_FROM_ISH_TO_HOST_CHV_AB(pisr_val);
} else {
pisr_val = ish_reg_read(dev, IPC_REG_PISR_BXT);
- interrupt_generated = IPC_INT_FROM_ISH_TO_HOST_BXT(pisr_val);
+ interrupt_generated = !!pisr_val;
+ /* only busy-clear bit is RW, others are RO */
+ if (pisr_val)
+ ish_reg_write(dev, IPC_REG_PISR_BXT, pisr_val);
}
return interrupt_generated;
@@ -843,11 +846,11 @@ int ish_hw_start(struct ishtp_device *dev)
{
ish_set_host_rdy(dev);
+ set_host_ready(dev);
+
/* After that we can enable ISH DMA operation and wakeup ISHFW */
ish_wakeup(dev);
- set_host_ready(dev);
-
/* wait for FW-initiated reset flow */
if (!dev->recvd_hw_ready)
wait_event_interruptible_timeout(dev->wait_hw_ready,
--
2.19.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.14 063/123] HID: intel-ish-hid: avoid binding wrong ishtp_cl_device
From: Sasha Levin @ 2019-03-27 18:15 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Hong Liu, Jiri Kosina, Sasha Levin, linux-input
In-Reply-To: <20190327181628.15899-1-sashal@kernel.org>
From: Hong Liu <hong.liu@intel.com>
[ Upstream commit 0d28f49412405d87d3aae83da255070a46e67627 ]
When performing a warm reset in ishtp bus driver, the ishtp_cl_device
will not be removed, its fw_client still points to the already freed
ishtp_device.fw_clients array.
Later after driver finishing ishtp client enumeration, this dangling
pointer may cause driver to bind the wrong ishtp_cl_device to the new
client, causing wrong callback to be called for messages intended for
the new client.
This helps in development of firmware where frequent switching of
firmwares is required without Linux reboot.
Signed-off-by: Hong Liu <hong.liu@intel.com>
Tested-by: Hongyan Song <hongyan.song@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/intel-ish-hid/ishtp/bus.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c
index 2623a567ffba..f546635e9ac9 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.c
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
@@ -623,7 +623,8 @@ int ishtp_cl_device_bind(struct ishtp_cl *cl)
spin_lock_irqsave(&cl->dev->device_list_lock, flags);
list_for_each_entry(cl_device, &cl->dev->device_list,
device_link) {
- if (cl_device->fw_client->client_id == cl->fw_client_id) {
+ if (cl_device->fw_client &&
+ cl_device->fw_client->client_id == cl->fw_client_id) {
cl->device = cl_device;
rv = 0;
break;
@@ -683,6 +684,7 @@ void ishtp_bus_remove_all_clients(struct ishtp_device *ishtp_dev,
spin_lock_irqsave(&ishtp_dev->device_list_lock, flags);
list_for_each_entry_safe(cl_device, n, &ishtp_dev->device_list,
device_link) {
+ cl_device->fw_client = NULL;
if (warm_reset && cl_device->reference_count)
continue;
--
2.19.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 187/192] Input: soc_button_array - fix mapping of the 5th GPIO in a PNP0C40 device
From: Sasha Levin @ 2019-03-27 18:10 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Hans de Goede, Dmitry Torokhov, Sasha Levin, linux-input
In-Reply-To: <20190327181025.13507-1-sashal@kernel.org>
From: Hans de Goede <hdegoede@redhat.com>
[ Upstream commit e9eb788f9442d1b5d93efdb30c3be071ce8a22b1 ]
The Microsoft documenation for the PNP0C40 device aka the
"Windows-compatible button array" describes the 5th GpioInt listed in
the resources as: '5. Interrupt corresponding to the "Rotation Lock"
button, if supported'.
Notice this describes the 5th entry as a button while we sofar have been
mapping it to EV_SW, SW_ROTATE_LOCK. On my Point of View TAB P1006W-232
which actually comes with a rotation-lock button, the button indeed is a
button and not a slider/switch. An image search for other Windows tablets
has found 2 more models with a rotation-lock button and on both of those
it too is a push-button and not a slider/switch.
Further evidence can be found in the HUT extension HUTRR52 from Microsoft
which adds rotation lock support to the HUT, which describes 2 different
usages: "0xC9 System Display Rotation Lock Button" and
"0xCA System Display Rotation Lock Slider Switch" note that switch is seen
as a separate thing here and the non switch wording is an exact match for
the "Windows-compatible button array" spec wording.
TL;DR: our current mapping of the 5th GPIO to SW_ROTATE_LOCK is wrong
because the 5th GPIO is for a push-button not a switch.
This commit fixes this by maping the 5th GPIO to KEY_ROTATE_LOCK_TOGGLE.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/input/misc/soc_button_array.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
index 23520df7650f..55cd6e0b409c 100644
--- a/drivers/input/misc/soc_button_array.c
+++ b/drivers/input/misc/soc_button_array.c
@@ -373,7 +373,7 @@ static struct soc_button_info soc_button_PNP0C40[] = {
{ "home", 1, EV_KEY, KEY_LEFTMETA, false, true },
{ "volume_up", 2, EV_KEY, KEY_VOLUMEUP, true, false },
{ "volume_down", 3, EV_KEY, KEY_VOLUMEDOWN, true, false },
- { "rotation_lock", 4, EV_SW, SW_ROTATE_LOCK, false, false },
+ { "rotation_lock", 4, EV_KEY, KEY_ROTATE_LOCK_TOGGLE, false, false },
{ }
};
--
2.19.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 162/192] HID: intel-ish: ipc: handle PIMR before ish_wakeup also clear PISR busy_clear bit
From: Sasha Levin @ 2019-03-27 18:09 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Song Hongyan, Jiri Kosina, Sasha Levin, linux-input
In-Reply-To: <20190327181025.13507-1-sashal@kernel.org>
From: Song Hongyan <hongyan.song@intel.com>
[ Upstream commit 2edefc056e4f0e6ec9508dd1aca2c18fa320efef ]
Host driver should handle interrupt mask register earlier than wake up ish FW
else there will be conditions when FW interrupt comes, host PIMR register still
not set ready, so move the interrupt mask setting before ish_wakeup.
Clear PISR busy_clear bit in ish_irq_handler. If not clear, there will be
conditions host driver received a busy_clear interrupt (before the busy_clear
mask bit is ready), it will return IRQ_NONE after check_generated_interrupt,
the interrupt will never be cleared, causing the DEVICE not sending following
IRQ.
Since PISR clear should not be called for the CHV device we do this change.
After the change, both ISH2HOST interrupt and busy_clear interrupt will be
considered as interrupt from ISH, busy_clear interrupt will return IRQ_HANDLED
from IPC_IS_BUSY check.
Signed-off-by: Song Hongyan <hongyan.song@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/intel-ish-hid/ipc/ipc.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ipc/ipc.c b/drivers/hid/intel-ish-hid/ipc/ipc.c
index bfbca7ec54ce..e00b9dbe220f 100644
--- a/drivers/hid/intel-ish-hid/ipc/ipc.c
+++ b/drivers/hid/intel-ish-hid/ipc/ipc.c
@@ -91,7 +91,10 @@ static bool check_generated_interrupt(struct ishtp_device *dev)
IPC_INT_FROM_ISH_TO_HOST_CHV_AB(pisr_val);
} else {
pisr_val = ish_reg_read(dev, IPC_REG_PISR_BXT);
- interrupt_generated = IPC_INT_FROM_ISH_TO_HOST_BXT(pisr_val);
+ interrupt_generated = !!pisr_val;
+ /* only busy-clear bit is RW, others are RO */
+ if (pisr_val)
+ ish_reg_write(dev, IPC_REG_PISR_BXT, pisr_val);
}
return interrupt_generated;
@@ -843,11 +846,11 @@ int ish_hw_start(struct ishtp_device *dev)
{
ish_set_host_rdy(dev);
+ set_host_ready(dev);
+
/* After that we can enable ISH DMA operation and wakeup ISHFW */
ish_wakeup(dev);
- set_host_ready(dev);
-
/* wait for FW-initiated reset flow */
if (!dev->recvd_hw_ready)
wait_event_interruptible_timeout(dev->wait_hw_ready,
--
2.19.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 095/192] HID: intel-ish-hid: avoid binding wrong ishtp_cl_device
From: Sasha Levin @ 2019-03-27 18:08 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Hong Liu, Jiri Kosina, Sasha Levin, linux-input
In-Reply-To: <20190327181025.13507-1-sashal@kernel.org>
From: Hong Liu <hong.liu@intel.com>
[ Upstream commit 0d28f49412405d87d3aae83da255070a46e67627 ]
When performing a warm reset in ishtp bus driver, the ishtp_cl_device
will not be removed, its fw_client still points to the already freed
ishtp_device.fw_clients array.
Later after driver finishing ishtp client enumeration, this dangling
pointer may cause driver to bind the wrong ishtp_cl_device to the new
client, causing wrong callback to be called for messages intended for
the new client.
This helps in development of firmware where frequent switching of
firmwares is required without Linux reboot.
Signed-off-by: Hong Liu <hong.liu@intel.com>
Tested-by: Hongyan Song <hongyan.song@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/intel-ish-hid/ishtp/bus.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c
index 2623a567ffba..f546635e9ac9 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.c
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
@@ -623,7 +623,8 @@ int ishtp_cl_device_bind(struct ishtp_cl *cl)
spin_lock_irqsave(&cl->dev->device_list_lock, flags);
list_for_each_entry(cl_device, &cl->dev->device_list,
device_link) {
- if (cl_device->fw_client->client_id == cl->fw_client_id) {
+ if (cl_device->fw_client &&
+ cl_device->fw_client->client_id == cl->fw_client_id) {
cl->device = cl_device;
rv = 0;
break;
@@ -683,6 +684,7 @@ void ishtp_bus_remove_all_clients(struct ishtp_device *ishtp_dev,
spin_lock_irqsave(&ishtp_dev->device_list_lock, flags);
list_for_each_entry_safe(cl_device, n, &ishtp_dev->device_list,
device_link) {
+ cl_device->fw_client = NULL;
if (warm_reset && cl_device->reference_count)
continue;
--
2.19.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.0 254/262] Input: soc_button_array - fix mapping of the 5th GPIO in a PNP0C40 device
From: Sasha Levin @ 2019-03-27 18:01 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Hans de Goede, Dmitry Torokhov, Sasha Levin, linux-input
In-Reply-To: <20190327180158.10245-1-sashal@kernel.org>
From: Hans de Goede <hdegoede@redhat.com>
[ Upstream commit e9eb788f9442d1b5d93efdb30c3be071ce8a22b1 ]
The Microsoft documenation for the PNP0C40 device aka the
"Windows-compatible button array" describes the 5th GpioInt listed in
the resources as: '5. Interrupt corresponding to the "Rotation Lock"
button, if supported'.
Notice this describes the 5th entry as a button while we sofar have been
mapping it to EV_SW, SW_ROTATE_LOCK. On my Point of View TAB P1006W-232
which actually comes with a rotation-lock button, the button indeed is a
button and not a slider/switch. An image search for other Windows tablets
has found 2 more models with a rotation-lock button and on both of those
it too is a push-button and not a slider/switch.
Further evidence can be found in the HUT extension HUTRR52 from Microsoft
which adds rotation lock support to the HUT, which describes 2 different
usages: "0xC9 System Display Rotation Lock Button" and
"0xCA System Display Rotation Lock Slider Switch" note that switch is seen
as a separate thing here and the non switch wording is an exact match for
the "Windows-compatible button array" spec wording.
TL;DR: our current mapping of the 5th GPIO to SW_ROTATE_LOCK is wrong
because the 5th GPIO is for a push-button not a switch.
This commit fixes this by maping the 5th GPIO to KEY_ROTATE_LOCK_TOGGLE.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/input/misc/soc_button_array.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
index 23520df7650f..55cd6e0b409c 100644
--- a/drivers/input/misc/soc_button_array.c
+++ b/drivers/input/misc/soc_button_array.c
@@ -373,7 +373,7 @@ static struct soc_button_info soc_button_PNP0C40[] = {
{ "home", 1, EV_KEY, KEY_LEFTMETA, false, true },
{ "volume_up", 2, EV_KEY, KEY_VOLUMEUP, true, false },
{ "volume_down", 3, EV_KEY, KEY_VOLUMEDOWN, true, false },
- { "rotation_lock", 4, EV_SW, SW_ROTATE_LOCK, false, false },
+ { "rotation_lock", 4, EV_KEY, KEY_ROTATE_LOCK_TOGGLE, false, false },
{ }
};
--
2.19.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.0 219/262] HID: intel-ish: ipc: handle PIMR before ish_wakeup also clear PISR busy_clear bit
From: Sasha Levin @ 2019-03-27 18:01 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Song Hongyan, Jiri Kosina, Sasha Levin, linux-input
In-Reply-To: <20190327180158.10245-1-sashal@kernel.org>
From: Song Hongyan <hongyan.song@intel.com>
[ Upstream commit 2edefc056e4f0e6ec9508dd1aca2c18fa320efef ]
Host driver should handle interrupt mask register earlier than wake up ish FW
else there will be conditions when FW interrupt comes, host PIMR register still
not set ready, so move the interrupt mask setting before ish_wakeup.
Clear PISR busy_clear bit in ish_irq_handler. If not clear, there will be
conditions host driver received a busy_clear interrupt (before the busy_clear
mask bit is ready), it will return IRQ_NONE after check_generated_interrupt,
the interrupt will never be cleared, causing the DEVICE not sending following
IRQ.
Since PISR clear should not be called for the CHV device we do this change.
After the change, both ISH2HOST interrupt and busy_clear interrupt will be
considered as interrupt from ISH, busy_clear interrupt will return IRQ_HANDLED
from IPC_IS_BUSY check.
Signed-off-by: Song Hongyan <hongyan.song@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/intel-ish-hid/ipc/ipc.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ipc/ipc.c b/drivers/hid/intel-ish-hid/ipc/ipc.c
index 742191bb24c6..45e33c7ba9a6 100644
--- a/drivers/hid/intel-ish-hid/ipc/ipc.c
+++ b/drivers/hid/intel-ish-hid/ipc/ipc.c
@@ -91,7 +91,10 @@ static bool check_generated_interrupt(struct ishtp_device *dev)
IPC_INT_FROM_ISH_TO_HOST_CHV_AB(pisr_val);
} else {
pisr_val = ish_reg_read(dev, IPC_REG_PISR_BXT);
- interrupt_generated = IPC_INT_FROM_ISH_TO_HOST_BXT(pisr_val);
+ interrupt_generated = !!pisr_val;
+ /* only busy-clear bit is RW, others are RO */
+ if (pisr_val)
+ ish_reg_write(dev, IPC_REG_PISR_BXT, pisr_val);
}
return interrupt_generated;
@@ -839,11 +842,11 @@ int ish_hw_start(struct ishtp_device *dev)
{
ish_set_host_rdy(dev);
+ set_host_ready(dev);
+
/* After that we can enable ISH DMA operation and wakeup ISHFW */
ish_wakeup(dev);
- set_host_ready(dev);
-
/* wait for FW-initiated reset flow */
if (!dev->recvd_hw_ready)
wait_event_interruptible_timeout(dev->wait_hw_ready,
--
2.19.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.0 132/262] HID: intel-ish-hid: avoid binding wrong ishtp_cl_device
From: Sasha Levin @ 2019-03-27 17:59 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Hong Liu, Jiri Kosina, Sasha Levin, linux-input
In-Reply-To: <20190327180158.10245-1-sashal@kernel.org>
From: Hong Liu <hong.liu@intel.com>
[ Upstream commit 0d28f49412405d87d3aae83da255070a46e67627 ]
When performing a warm reset in ishtp bus driver, the ishtp_cl_device
will not be removed, its fw_client still points to the already freed
ishtp_device.fw_clients array.
Later after driver finishing ishtp client enumeration, this dangling
pointer may cause driver to bind the wrong ishtp_cl_device to the new
client, causing wrong callback to be called for messages intended for
the new client.
This helps in development of firmware where frequent switching of
firmwares is required without Linux reboot.
Signed-off-by: Hong Liu <hong.liu@intel.com>
Tested-by: Hongyan Song <hongyan.song@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/intel-ish-hid/ishtp/bus.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c
index 728dc6d4561a..a271d6d169b1 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.c
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
@@ -675,7 +675,8 @@ int ishtp_cl_device_bind(struct ishtp_cl *cl)
spin_lock_irqsave(&cl->dev->device_list_lock, flags);
list_for_each_entry(cl_device, &cl->dev->device_list,
device_link) {
- if (cl_device->fw_client->client_id == cl->fw_client_id) {
+ if (cl_device->fw_client &&
+ cl_device->fw_client->client_id == cl->fw_client_id) {
cl->device = cl_device;
rv = 0;
break;
@@ -735,6 +736,7 @@ void ishtp_bus_remove_all_clients(struct ishtp_device *ishtp_dev,
spin_lock_irqsave(&ishtp_dev->device_list_lock, flags);
list_for_each_entry_safe(cl_device, n, &ishtp_dev->device_list,
device_link) {
+ cl_device->fw_client = NULL;
if (warm_reset && cl_device->reference_count)
continue;
--
2.19.1
^ permalink raw reply related
* Re: [PATCH] HID: intel-ish-hid: ISH firmware loader client driver
From: Nick Crews @ 2019-03-27 16:01 UTC (permalink / raw)
To: Srinivas Pandruvada
Cc: Rushikesh S Kadam, benjamin.tissoires, jikos, jettrink, gwendal,
linux-kernel, linux-input
In-Reply-To: <de917efc4ded94b3fbc70913207881b3a0c10b52.camel@linux.intel.com>
On Tue, Mar 26, 2019 at 8:22 PM Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
>
> On Tue, 2019-03-26 at 18:39 -0600, Nick Crews wrote:
> > Hi Rushikesh, I know I've been reviewing this on Chromium, but I have
> > some more larges-scale design thoughts.
> Hi Nick.
>
> Does this fundamentally change, the way it is done here or can wait for
> subsequent revisions later?
I don't have any official stakes in this, as I'm not the maintainer or
anything, so
I'm just preaching what I think would be good design :)
I think I would like to see most of my suggestions addressed. At the
very least there
are some actual bugs (infinite loops, accessing bad memory, not
reporting all errors)
that need to get fixed. Of course I'm not the one that has to write
or test it, but I imagine
that the one large design change I proposed of where memory is
allocated shouldn't be too
hard either. I worry that "subsequent revisions" to upstream won't
happen, since it's hard enough
to get a patch accepted. Maybe that concern isn't warranted though, I
don't have that
much experience on the LKML.
Is there a really tight deadline for this? If so then I would say we
should apply what
we currently have to the Chromium tree, and upstream the final version
when it's done.
Thanks,
Nick
>
> Thanks,
> Srinivas
>
^ permalink raw reply
* Re: [PATCH v3 1/4] drm/bridge: sil_sii8620: depend on INPUT instead of selecting it.
From: Andrzej Hajda @ 2019-03-27 14:13 UTC (permalink / raw)
To: Ronald Tschalär, Dmitry Torokhov, Henrik Rydberg,
Andy Shevchenko, Sergey Senozhatsky, Steven Rostedt,
Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Lukas Wunner, Federico Lorenzi, linux-input, linux-kernel,
Inki Dae, dri-devel@lists.freedesktop.org
In-Reply-To: <20190327014807.7472-2-ronald@innovation.ch>
+cc: dri-devel
On 27.03.2019 02:48, Ronald Tschalär wrote:
> commit d6abe6df706c66d803e6dd4fe98c1b6b7f125a56 (drm/bridge:
> sil_sii8620: do not have a dependency of RC_CORE) added a dependency on
> INPUT. However, this causes problems with other drivers, in particular
> an input driver that depends on MFD_INTEL_LPSS_PCI (to be added in a
> future commit):
>
> drivers/clk/Kconfig:9:error: recursive dependency detected!
> drivers/clk/Kconfig:9: symbol COMMON_CLK is selected by MFD_INTEL_LPSS
> drivers/mfd/Kconfig:566: symbol MFD_INTEL_LPSS is selected by MFD_INTEL_LPSS_PCI
> drivers/mfd/Kconfig:580: symbol MFD_INTEL_LPSS_PCI is implied by KEYBOARD_APPLESPI
> drivers/input/keyboard/Kconfig:73: symbol KEYBOARD_APPLESPI depends on INPUT
> drivers/input/Kconfig:8: symbol INPUT is selected by DRM_SIL_SII8620
> drivers/gpu/drm/bridge/Kconfig:83: symbol DRM_SIL_SII8620 depends on DRM_BRIDGE
> drivers/gpu/drm/bridge/Kconfig:1: symbol DRM_BRIDGE is selected by DRM_PL111
> drivers/gpu/drm/pl111/Kconfig:1: symbol DRM_PL111 depends on COMMON_CLK
>
> According to the docs, select should only be used for non-visible
> symbols. Furthermore almost all other references to INPUT throughout the
> kernel config are depends, not selects. Hence this change.
>
> CC: Inki Dae <inki.dae@samsung.com>
> CC: Andrzej Hajda <a.hajda@samsung.com>
> Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
This is drm bridge driver, next time please cc it to dri-devel ML also.
Anyway this is not the solution we have agreed to.
Why have you abandoned the patch [1]? It needed just some minor
polishing, as I wrote in response to this mail.
[1]:
https://lore.kernel.org/lkml/20190124082423.23139-1-ronald@innovation.ch/T/#mf620df0b1583096a214d8e2e690387078583472f
Regards
Andrzej
> ---
> drivers/gpu/drm/bridge/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index 2fee47b0d50b..eabedc83f25c 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -83,9 +83,9 @@ config DRM_PARADE_PS8622
> config DRM_SIL_SII8620
> tristate "Silicon Image SII8620 HDMI/MHL bridge"
> depends on OF
> + depends on INPUT
> select DRM_KMS_HELPER
> imply EXTCON
> - select INPUT
> select RC_CORE
> help
> Silicon Image SII8620 HDMI/MHL bridge chip driver.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox