* RE: [RESEND] input: keyboard: imx: make sure keyboard can always wake up system
From: Anson Huang @ 2019-05-09 1:41 UTC (permalink / raw)
To: dmitry.torokhov@gmail.com, shawnguo@kernel.org,
s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com,
linux-input@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Cc: dl-linux-imx
In-Reply-To: <DB3PR0402MB39167BC7D996F4FF70B5DD2FF53D0@DB3PR0402MB3916.eurprd04.prod.outlook.com>
Ping...
> -----Original Message-----
> From: Anson Huang
> Sent: Thursday, April 25, 2019 9:50 AM
> To: dmitry.torokhov@gmail.com; shawnguo@kernel.org;
> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> linux-input@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org
> Cc: dl-linux-imx <linux-imx@nxp.com>
> Subject: RE: [RESEND] input: keyboard: imx: make sure keyboard can always
> wake up system
>
> Gentle ping...
>
> > -----Original Message-----
> > From: Anson Huang
> > Sent: Thursday, April 4, 2019 9:40 AM
> > To: dmitry.torokhov@gmail.com; shawnguo@kernel.org;
> > s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> > linux-input@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > linux- kernel@vger.kernel.org
> > Cc: dl-linux-imx <linux-imx@nxp.com>
> > Subject: [RESEND] input: keyboard: imx: make sure keyboard can always
> > wake up system
> >
> > There are several scenarios that keyboard can NOT wake up system from
> > suspend, e.g., if a keyboard is depressed between system device
> > suspend phase and device noirq suspend phase, the keyboard ISR will be
> > called and both keyboard depress and release interrupts will be
> > disabled, then keyboard will no longer be able to wake up system.
> > Another scenario would be, if a keyboard is kept depressed, and then
> > system goes into suspend, the expected behavior would be when keyboard
> > is released, system will be waked up, but current implementation can
> > NOT achieve that, because both depress and release interrupts are
> > disabled in ISR, and the event check is still in progress.
> >
> > To fix these issues, need to make sure keyboard's depress or release
> > interrupt is enabled after noirq device suspend phase, this patch
> > moves the suspend/resume callback to noirq suspend/resume phase, and
> > enable the corresponding interrupt according to current keyboard status.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> > drivers/input/keyboard/imx_keypad.c | 18 ++++++++++++++----
> > 1 file changed, 14 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/input/keyboard/imx_keypad.c
> > b/drivers/input/keyboard/imx_keypad.c
> > index cf08f4a..97500a2 100644
> > --- a/drivers/input/keyboard/imx_keypad.c
> > +++ b/drivers/input/keyboard/imx_keypad.c
> > @@ -524,11 +524,12 @@ static int imx_keypad_probe(struct
> > platform_device *pdev)
> > return 0;
> > }
> >
> > -static int __maybe_unused imx_kbd_suspend(struct device *dev)
> > +static int __maybe_unused imx_kbd_noirq_suspend(struct device *dev)
> > {
> > struct platform_device *pdev = to_platform_device(dev);
> > struct imx_keypad *kbd = platform_get_drvdata(pdev);
> > struct input_dev *input_dev = kbd->input_dev;
> > + unsigned short reg_val = readw(kbd->mmio_base + KPSR);
> >
> > /* imx kbd can wake up system even clock is disabled */
> > mutex_lock(&input_dev->mutex);
> > @@ -538,13 +539,20 @@ static int __maybe_unused
> imx_kbd_suspend(struct
> > device *dev)
> >
> > mutex_unlock(&input_dev->mutex);
> >
> > - if (device_may_wakeup(&pdev->dev))
> > + if (device_may_wakeup(&pdev->dev)) {
> > + if (reg_val & KBD_STAT_KPKD)
> > + reg_val |= KBD_STAT_KRIE;
> > + if (reg_val & KBD_STAT_KPKR)
> > + reg_val |= KBD_STAT_KDIE;
> > + writew(reg_val, kbd->mmio_base + KPSR);
> > +
> > enable_irq_wake(kbd->irq);
> > + }
> >
> > return 0;
> > }
> >
> > -static int __maybe_unused imx_kbd_resume(struct device *dev)
> > +static int __maybe_unused imx_kbd_noirq_resume(struct device *dev)
> > {
> > struct platform_device *pdev = to_platform_device(dev);
> > struct imx_keypad *kbd = platform_get_drvdata(pdev); @@ -568,7
> > +576,9 @@ static int __maybe_unused imx_kbd_resume(struct device
> *dev)
> > return ret;
> > }
> >
> > -static SIMPLE_DEV_PM_OPS(imx_kbd_pm_ops, imx_kbd_suspend,
> > imx_kbd_resume);
> > +static const struct dev_pm_ops imx_kbd_pm_ops = {
> > + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(imx_kbd_noirq_suspend,
> > +imx_kbd_noirq_resume) };
> >
> > static struct platform_driver imx_keypad_driver = {
> > .driver = {
> > --
> > 2.7.4
^ permalink raw reply
* [GIT PULL] Immutable branch between MFD, GPIO, Input, LEDs and Power due for the v5.2 merge window
From: Lee Jones @ 2019-05-08 11:11 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Rob Herring, Mark Rutland, Linus Walleij, Dmitry Torokhov,
Jacek Anaszewski, Pavel Machek, Sebastian Reichel, Liam Girdwood,
Greg Kroah-Hartman, linux-kernel, linux-gpio, devicetree,
linux-input, linux-leds, linux-pm, Bartosz Golaszewski
In-Reply-To: <20190423090451.23711-1-brgl@bgdev.pl>
Enjoy!
The following changes since commit e93c9c99a629c61837d5a7fc2120cd2b6c70dbdd:
Linux 5.1 (2019-05-05 17:42:58 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git ib-mfd-gpio-input-leds-power-v5.2
for you to fetch changes up to 796fad0101d370567c2968bd933b865aa57efaa3:
MAINTAINERS: Add an entry for MAX77650 PMIC driver (2019-05-08 12:07:12 +0100)
----------------------------------------------------------------
Immutable branch between MFD, GPIO, Input, LEDs and Power due for the v5.2 merge window
----------------------------------------------------------------
Bartosz Golaszewski (11):
dt-bindings: mfd: Add DT bindings for max77650
dt-bindings: power: supply: Add DT bindings for max77650
dt-bindings: leds: Add DT bindings for max77650
dt-bindings: input: Add DT bindings for max77650
mfd: mfd-core: Document mfd_add_devices()
mfd: Add new driver for MAX77650 PMIC
power: supply: max77650: Add support for battery charger
gpio: max77650: Add GPIO support
leds: max77650: Add LEDs support
input: max77650: Add onkey support
MAINTAINERS: Add an entry for MAX77650 PMIC driver
.../devicetree/bindings/input/max77650-onkey.txt | 26 ++
.../devicetree/bindings/leds/leds-max77650.txt | 57 ++++
Documentation/devicetree/bindings/mfd/max77650.txt | 46 +++
.../bindings/power/supply/max77650-charger.txt | 28 ++
MAINTAINERS | 14 +
drivers/gpio/Kconfig | 7 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-max77650.c | 190 +++++++++++
drivers/input/misc/Kconfig | 9 +
drivers/input/misc/Makefile | 1 +
drivers/input/misc/max77650-onkey.c | 121 +++++++
drivers/leds/Kconfig | 6 +
drivers/leds/Makefile | 1 +
drivers/leds/leds-max77650.c | 147 ++++++++
drivers/mfd/Kconfig | 14 +
drivers/mfd/Makefile | 1 +
drivers/mfd/max77650.c | 232 +++++++++++++
drivers/mfd/mfd-core.c | 13 +
drivers/power/supply/Kconfig | 7 +
drivers/power/supply/Makefile | 1 +
drivers/power/supply/max77650-charger.c | 368 +++++++++++++++++++++
include/linux/mfd/max77650.h | 59 ++++
22 files changed, 1349 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/max77650-onkey.txt
create mode 100644 Documentation/devicetree/bindings/leds/leds-max77650.txt
create mode 100644 Documentation/devicetree/bindings/mfd/max77650.txt
create mode 100644 Documentation/devicetree/bindings/power/supply/max77650-charger.txt
create mode 100644 drivers/gpio/gpio-max77650.c
create mode 100644 drivers/input/misc/max77650-onkey.c
create mode 100644 drivers/leds/leds-max77650.c
create mode 100644 drivers/mfd/max77650.c
create mode 100644 drivers/power/supply/max77650-charger.c
create mode 100644 include/linux/mfd/max77650.h
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v10 06/11] mfd: max77650: new core mfd driver
From: Lee Jones @ 2019-05-08 11:01 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Rob Herring, Mark Rutland, Linus Walleij, Dmitry Torokhov,
Jacek Anaszewski, Pavel Machek, Sebastian Reichel, Liam Girdwood,
Greg Kroah-Hartman, linux-kernel, linux-gpio, devicetree,
linux-input, linux-leds, linux-pm, Bartosz Golaszewski
In-Reply-To: <20190423090451.23711-7-brgl@bgdev.pl>
On Tue, 23 Apr 2019, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Add the core mfd driver for max77650 PMIC. We define five sub-devices
> for which the drivers will be added in subsequent patches.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
> drivers/mfd/Kconfig | 14 +++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/max77650.c | 232 +++++++++++++++++++++++++++++++++++
> include/linux/mfd/max77650.h | 59 +++++++++
> 4 files changed, 306 insertions(+)
> create mode 100644 drivers/mfd/max77650.c
> create mode 100644 include/linux/mfd/max77650.h
For my own reference:
Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v2 2/4] mfd: ioc3: Add driver for SGI IOC3 chip
From: Lee Jones @ 2019-05-08 10:23 UTC (permalink / raw)
To: Thomas Bogendoerfer
Cc: Ralf Baechle, Paul Burton, James Hogan, Dmitry Torokhov,
David S. Miller, Alessandro Zummo, Alexandre Belloni,
Greg Kroah-Hartman, Jiri Slaby, linux-mips, linux-kernel,
linux-input, netdev, linux-rtc, linux-serial
In-Reply-To: <20190409154610.6735-3-tbogendoerfer@suse.de>
On Tue, 09 Apr 2019, Thomas Bogendoerfer wrote:
> SGI IOC3 chip has integrated ethernet, keyboard and mouse interface.
> It also supports connecting a SuperIO chip for serial and parallel
> interfaces. IOC3 is used inside various SGI systemboards and add-on
> cards with different equipped external interfaces.
>
> Support for ethernet and serial interfaces were implemented inside
> the network driver. This patchset moves out the not network related
> parts to a new MFD driver, which takes care of card detection,
> setup of platform devices and interrupt distribution for the subdevices.
>
> Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
> ---
> arch/mips/include/asm/sn/ioc3.h | 345 +++---
> arch/mips/sgi-ip27/ip27-timer.c | 20 -
> drivers/mfd/Kconfig | 13 +
> drivers/mfd/Makefile | 1 +
> drivers/mfd/ioc3.c | 802 ++++++++++++++
> drivers/net/ethernet/sgi/Kconfig | 4 +-
> drivers/net/ethernet/sgi/ioc3-eth.c | 1867 ++++++++++++---------------------
> drivers/tty/serial/8250/8250_ioc3.c | 98 ++
> drivers/tty/serial/8250/Kconfig | 11 +
> drivers/tty/serial/8250/Makefile | 1 +
> include/linux/platform_data/ioc3eth.h | 15 +
> 11 files changed, 1762 insertions(+), 1415 deletions(-)
> create mode 100644 drivers/mfd/ioc3.c
> create mode 100644 drivers/tty/serial/8250/8250_ioc3.c
> create mode 100644 include/linux/platform_data/ioc3eth.h
[...]
> +config SGI_MFD_IOC3
> + tristate "SGI IOC3 core driver"
> + depends on PCI && MIPS
> + select MFD_CORE
> + help
> + This option enables basic support for the SGI IOC3-based
> + controller cards. This option does not enable any specific
> + functions on such a card, but provides necessary infrastructure
> + for other drivers to utilize.
> +
> + If you have an SGI Origin, Octane, or a PCI IOC3 card,
> + then say Y. Otherwise say N.
> +
> endmenu
> endif
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index b4569ed7f3f3..07255e499129 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -246,4 +246,5 @@ obj-$(CONFIG_MFD_MXS_LRADC) += mxs-lradc.o
> obj-$(CONFIG_MFD_SC27XX_PMIC) += sprd-sc27xx-spi.o
> obj-$(CONFIG_RAVE_SP_CORE) += rave-sp.o
> obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o
> +obj-$(CONFIG_SGI_MFD_IOC3) += ioc3.o
>
> diff --git a/drivers/mfd/ioc3.c b/drivers/mfd/ioc3.c
> new file mode 100644
> index 000000000000..ad715805b16e
> --- /dev/null
> +++ b/drivers/mfd/ioc3.c
> @@ -0,0 +1,802 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * SGI IOC3 multifunction device driver
> + *
> + * Copyright (C) 2018, 2019 Thomas Bogendoerfer <tbogendoerfer@suse.de>
> + *
> + * Based on work by:
> + * Stanislaw Skowronek <skylark@unaligned.org>
> + * Joshua Kinard <kumba@gentoo.org>
> + * Brent Casavant <bcasavan@sgi.com> - IOC4 master driver
> + * Pat Gefre <pfg@sgi.com> - IOC3 serial port IRQ demuxer
> + */
> +
> +#include <linux/errno.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/interrupt.h>
> +#include <linux/delay.h>
> +#include <linux/mfd/core.h>
> +#include <linux/platform_device.h>
> +#include <linux/platform_data/ioc3eth.h>
These should be in alphabetical order.
> +#include <asm/sn/ioc3.h>
> +#include <asm/pci/bridge.h>
> +
> +#define IOC3_ETH BIT(0)
> +#define IOC3_SER BIT(1)
> +#define IOC3_PAR BIT(2)
> +#define IOC3_KBD BIT(3)
> +#define IOC3_M48T35 BIT(4)
> +
> +static int ioc3_serial_id = 1;
> +static int ioc3_eth_id = 1;
> +static int ioc3_kbd_id = 1;
> +static struct mfd_cell ioc3_mfd_cells[5];
> +
> +struct ioc3_board_info {
> + const char *name;
> + int irq_offset;
> + int funcs;
> +};
> +
> +struct ioc3_priv_data {
> + struct ioc3_board_info *info;
> + struct irq_domain *domain;
> + struct ioc3 __iomem *regs;
> + struct pci_dev *pdev;
> + char nic_part[32];
> + char nic_mac[6];
> + int irq_io;
> +};
> +
> +#define MCR_PACK(pulse, sample) (((pulse) << 10) | ((sample) << 2))
> +
> +static int ioc3_nic_wait(u32 __iomem *mcr)
> +{
> + u32 mcr_val;
> +
> + do {
> + mcr_val = readl(mcr);
> + } while (!(mcr_val & 2));
Why 2? Is bit 2 always set on a successful read?
Either way, you should provide a comment to improve readability.
> + return (mcr_val & 1);
Reads just a bit at a time? Again, a comment please.
> +}
> +
> +static int ioc3_nic_reset(u32 __iomem *mcr)
> +{
> + int presence;
> + unsigned long flags;
> +
> + local_irq_save(flags);
> + writel(MCR_PACK(520, 65), mcr);
What do these magic values mean? Best to define them.
> + presence = ioc3_nic_wait(mcr);
> + local_irq_restore(flags);
> +
> + udelay(500);
What are you waiting for? Why 500us?
> + return presence;
> +}
> +
> +static int ioc3_nic_read_bit(u32 __iomem *mcr)
> +{
> + int result;
> + unsigned long flags;
> +
> + local_irq_save(flags);
> + writel(MCR_PACK(6, 13), mcr);
Why 6 and 13?
Define all magic values please.
> + result = ioc3_nic_wait(mcr);
> + local_irq_restore(flags);
> +
> + udelay(100);
Why 100?
> + return result;
> +}
> +
> +static u32 ioc3_nic_read_byte(u32 __iomem *mcr)
> +{
> + u32 result = 0;
> + int i;
> +
> + for (i = 0; i < 8; i++)
> + result = ((result >> 1) | (ioc3_nic_read_bit(mcr) << 7));
> +
> + return result;
> +}
> +
> +static void ioc3_nic_write_bit(u32 __iomem *mcr, int bit)
> +{
> + if (bit)
> + writel(MCR_PACK(6, 110), mcr);
> + else
> + writel(MCR_PACK(80, 30), mcr);
> +
> + ioc3_nic_wait(mcr);
> +}
> +
> +static void ioc3_nic_write_byte(u32 __iomem *mcr, int byte)
> +{
> + int i;
> +
> + for (i = 0; i < 8; i++) {
> + ioc3_nic_write_bit(mcr, byte & 1);
> + byte >>= 1;
> + }
> +}
> +
> +static u64 ioc3_nic_find(u32 __iomem *mcr, int *last, u64 addr)
> +{
> + int a, b, index, disc;
> +
> + ioc3_nic_reset(mcr);
> +
> + /* Search ROM. */
> + ioc3_nic_write_byte(mcr, 0xf0);
What does 0xf0 mean?
Please define it and all magic numbers that follow.
> + /* Algorithm from ``Book of iButton Standards''. */
That's great, but what is it actually doing. Please provide suitable
comments such that the reader doesn't have to painstakingly work it
all out.
> + for (index = 0, disc = 0; index < 64; index++) {
> + a = ioc3_nic_read_bit(mcr);
> + b = ioc3_nic_read_bit(mcr);
> +
> + if (unlikely(a && b)) {
> + pr_warn("ioc3: NIC search failed.\n");
> + *last = 0;
> + return 0;
> + }
> +
> + if (!a && !b) {
> + if (index == *last)
> + addr |= 1UL << index;
> + else if (index > *last) {
> + addr &= ~(1UL << index);
> + disc = index;
> + } else if ((addr & (1UL << index)) == 0)
> + disc = index;
> + ioc3_nic_write_bit(mcr, (addr >> index) & 1);
> + continue;
> + } else {
> + if (a)
> + addr |= (1UL << index);
> + else
> + addr &= ~(1UL << index);
> + ioc3_nic_write_bit(mcr, a);
> + continue;
> + }
> + }
> + *last = disc;
> + return addr;
> +}
> +
> +static void ioc3_nic_addr(u32 __iomem *mcr, u64 addr)
> +{
> + int index;
> +
> + ioc3_nic_reset(mcr);
> + ioc3_nic_write_byte(mcr, 0xf0);
> +
> + for (index = 0; index < 64; index++) {
> + ioc3_nic_read_bit(mcr);
> + ioc3_nic_read_bit(mcr);
> + ioc3_nic_write_bit(mcr, ((addr >> index) & 1));
> + }
What is this function doing? Setting the NIC address?
Why are the 2 sequential reads required before each bit write?
> +}
> +
> +static void crc16_byte(u32 *crc, u8 db)
> +{
> + int i;
> +
> + for (i = 0; i < 8; i++) {
> + *crc <<= 1;
> + if ((db ^ (*crc >> 16)) & 1)
> + *crc ^= 0x8005;
> + db >>= 1;
> + }
> + *crc &= 0xffff;
> +}
> +
> +static u32 crc16_area(u8 *dbs, int size, u32 crc)
> +{
> + while (size--)
> + crc16_byte(&crc, *(dbs++));
> +
> + return crc;
> +}
> +
> +static void crc8_byte(u32 *crc, u8 db)
> +{
> + int i, f;
> +
> + for (i = 0; i < 8; i++) {
> + f = ((*crc ^ db) & 1);
> + *crc >>= 1;
> + db >>= 1;
> + if (f)
> + *crc ^= 0x8c;
> + }
> + *crc &= 0xff;
> +}
> +
> +static u32 crc8_addr(u64 addr)
> +{
> + u32 crc = 0;
> + int i;
> +
> + for (i = 0; i < 64; i += 8)
> + crc8_byte(&crc, addr >> i);
> + return crc;
> +}
Not looked into these in any detail, but are you not able to use the
CRC functions already provided by the kernel?
> +static void ioc3_read_redir_page(u32 __iomem *mcr, u64 addr, int page,
What is a redir page?
> + u8 *redir, u8 *data)
> +{
> + int loops = 16, i;
> +
> + while (redir[page] != 0xff) {
> + page = (redir[page] ^ 0xff);
> + loops--;
> + if (unlikely(loops < 0)) {
> + pr_err("ioc3: NIC circular redirection\n");
> + return;
> + }
> + }
> +
> + loops = 3;
> + while (loops > 0) {
> + ioc3_nic_addr(mcr, addr);
> + ioc3_nic_write_byte(mcr, 0xf0);
> + ioc3_nic_write_byte(mcr, (page << 5) & 0xe0);
> + ioc3_nic_write_byte(mcr, (page >> 3) & 0x1f);
> +
> + for (i = 0; i < 0x20; i++)
> + data[i] = ioc3_nic_read_byte(mcr);
> +
> + if (crc16_area(data, 0x20, 0) == 0x800d)
> + return;
> +
> + loops--;
> + }
> +
> + pr_err("ioc3: CRC error in data page\n");
> + for (i = 0; i < 0x20; i++)
> + data[i] = 0x00;
Comments required throughout.
> +}
> +
> +static void ioc3_read_redir_map(u32 __iomem *mcr, u64 addr, u8 *redir)
> +{
> + int i, j, crc_ok, loops = 3;
> + u32 crc;
> +
> + while (loops > 0) {
> + crc_ok = 1;
> + ioc3_nic_addr(mcr, addr);
> + ioc3_nic_write_byte(mcr, 0xaa);
> + ioc3_nic_write_byte(mcr, 0x00);
> + ioc3_nic_write_byte(mcr, 0x01);
> +
> + for (i = 0; i < 64; i += 8) {
> + for (j = 0; j < 8; j++)
> + redir[i + j] = ioc3_nic_read_byte(mcr);
> +
> + crc = crc16_area(redir + i, 8, i == 0 ? 0x8707 : 0);
> +
> + crc16_byte(&crc, ioc3_nic_read_byte(mcr));
> + crc16_byte(&crc, ioc3_nic_read_byte(mcr));
> +
> + if (crc != 0x800d)
> + crc_ok = 0;
> + }
> + if (crc_ok)
> + return;
> + loops--;
> + }
> + pr_err("ioc3: CRC error in redirection page\n");
> + for (i = 0; i < 64; i++)
> + redir[i] = 0xff;
As above.
> +}
> +
> +static void ioc3_read_nic(struct ioc3_priv_data *ipd, u32 __iomem *mcr,
> + u64 addr)
> +{
> + u8 redir[64];
> + u8 data[64], part[32];
> + int i, j;
> +
> + /* Read redirections */
> + ioc3_read_redir_map(mcr, addr, redir);
> +
> + /* Read data pages */
> + ioc3_read_redir_page(mcr, addr, 0, redir, data);
> + ioc3_read_redir_page(mcr, addr, 1, redir, (data + 32));
> +
> + /* Assemble the part # */
> + j = 0;
> + for (i = 0; i < 19; i++)
> + if (data[i + 11] != ' ')
> + part[j++] = data[i + 11];
> +
> + for (i = 0; i < 6; i++)
> + if (data[i + 32] != ' ')
> + part[j++] = data[i + 32];
> +
> + part[j] = 0;
> +
> + /* Skip Octane (IP30) power supplies */
> + if (!(strncmp(part, "060-0035-", 9)) ||
> + !(strncmp(part, "060-0038-", 9)) ||
> + !(strncmp(part, "060-0028-", 9)))
> + return;
> +
> + strlcpy(ipd->nic_part, part, sizeof(ipd->nic_part));
> +}
> +
> +static void ioc3_read_mac(struct ioc3_priv_data *ipd, u64 addr)
> +{
> + int i, loops = 3;
> + u8 data[13];
> + u32 __iomem *mcr = &ipd->regs->mcr;
> +
> + while (loops > 0) {
> + ioc3_nic_addr(mcr, addr);
> + ioc3_nic_write_byte(mcr, 0xf0);
> + ioc3_nic_write_byte(mcr, 0x00);
> + ioc3_nic_write_byte(mcr, 0x00);
> + ioc3_nic_read_byte(mcr);
> +
> + for (i = 0; i < 13; i++)
> + data[i] = ioc3_nic_read_byte(mcr);
> +
> + if (crc16_area(data, 13, 0) == 0x800d) {
> + for (i = 10; i > 4; i--)
> + ipd->nic_mac[10 - i] = data[i];
> + return;
> + }
> + loops--;
> + }
> +
> + pr_err("ioc3: CRC error in MAC address\n");
> + for (i = 0; i < 6; i++)
> + ipd->nic_mac[i] = 0x00;
> +}
> +
> +static void ioc3_probe_nic(struct ioc3_priv_data *ipd, u32 __iomem *mcr)
> +{
> + int save = 0, loops = 3;
> + u64 first, addr;
> +
> + while (loops > 0) {
> + ipd->nic_part[0] = 0;
> + first = ioc3_nic_find(mcr, &save, 0);
> + addr = first;
> +
> + if (unlikely(!first))
> + return;
> +
> + while (1) {
> + if (crc8_addr(addr))
> + break;
> +
> + switch (addr & 0xff) {
> + case 0x0b:
> + ioc3_read_nic(ipd, mcr, addr);
> + break;
> + case 0x09:
> + case 0x89:
> + case 0x91:
> + ioc3_read_mac(ipd, addr);
> + break;
> + }
> +
> + addr = ioc3_nic_find(mcr, &save, addr);
> + if (addr == first)
> + return;
> + }
> + loops--;
> + }
> + pr_err("ioc3: CRC error in NIC address\n");
> +}
This all looks like networking code. If this is the case, it should
be moved to drivers/networking or similar.
> +static void ioc3_irq_ack(struct irq_data *d)
> +{
> + struct ioc3_priv_data *ipd = irq_data_get_irq_chip_data(d);
> + unsigned int hwirq = irqd_to_hwirq(d);
> +
> + writel(BIT(hwirq), &ipd->regs->sio_ir);
> +}
> +
> +static void ioc3_irq_mask(struct irq_data *d)
> +{
> + struct ioc3_priv_data *ipd = irq_data_get_irq_chip_data(d);
> + unsigned int hwirq = irqd_to_hwirq(d);
> +
> + writel(BIT(hwirq), &ipd->regs->sio_iec);
> +}
> +
> +static void ioc3_irq_unmask(struct irq_data *d)
> +{
> + struct ioc3_priv_data *ipd = irq_data_get_irq_chip_data(d);
> + unsigned int hwirq = irqd_to_hwirq(d);
> +
> + writel(BIT(hwirq), &ipd->regs->sio_ies);
> +}
> +
> +static struct irq_chip ioc3_irq_chip = {
> + .name = "IOC3",
> + .irq_ack = ioc3_irq_ack,
> + .irq_mask = ioc3_irq_mask,
> + .irq_unmask = ioc3_irq_unmask,
> +};
> +
> +#define IOC3_LVL_MASK (BIT(0) | BIT(2) | BIT(6) | BIT(9) | BIT(11) | BIT(15))
You need to define what each of these bits are. BIT(2) doesn't tell
the reader anything, meaning the code is unreadable.
> +static int ioc3_irq_domain_map(struct irq_domain *d, unsigned int irq,
> + irq_hw_number_t hwirq)
> +{
> + if (BIT(hwirq) & IOC3_LVL_MASK)
Comment needed.
> + irq_set_chip_and_handler(irq, &ioc3_irq_chip, handle_level_irq);
> + else
> + irq_set_chip_and_handler(irq, &ioc3_irq_chip, handle_edge_irq);
> +
> + irq_set_chip_data(irq, d->host_data);
> + return 0;
> +}
> +
> +
Drop the superfluous '\n'.
> +static const struct irq_domain_ops ioc3_irq_domain_ops = {
> + .map = ioc3_irq_domain_map,
> +};
> +
> +static void ioc3_irq_handler(struct irq_desc *desc)
> +{
> + struct irq_domain *domain = irq_desc_get_handler_data(desc);
> + struct ioc3_priv_data *ipd = domain->host_data;
> + struct ioc3 __iomem *regs = ipd->regs;
> + unsigned int irq = 0;
Why does these need to be pre-assigned?
> + u32 pending;
> +
> + pending = readl(®s->sio_ir);
> + pending &= readl(®s->sio_ies);
Comment required.
> + if (pending)
> + irq = irq_find_mapping(domain, __ffs(pending));
> + else if (!ipd->info->irq_offset &&
> + (readl(®s->eth.eisr) & readl(®s->eth.eier)))
Comment required.
> + irq = irq_find_mapping(domain, 23);
> +
> + if (irq)
> + generic_handle_irq(irq);
> + else
> + spurious_interrupt();
> +}
> +
> +static struct resource ioc3_uarta_resources[] = {
> + DEFINE_RES_MEM(offsetof(struct ioc3, sregs.uarta),
You are the first user of offsetof() in MFD. Could you tell me why
it's required please?
> + sizeof_field(struct ioc3, sregs.uarta)),
> + DEFINE_RES_IRQ(6)
Please define all magic numbers.
> +};
> +
> +static struct resource ioc3_uartb_resources[] = {
> + DEFINE_RES_MEM(offsetof(struct ioc3, sregs.uartb),
> + sizeof_field(struct ioc3, sregs.uartb)),
> + DEFINE_RES_IRQ(15)
> +};
> +
> +static struct resource ioc3_kbd_resources[] = {
> + DEFINE_RES_MEM(offsetof(struct ioc3, serio),
> + sizeof_field(struct ioc3, serio)),
> + DEFINE_RES_IRQ(22)
> +};
> +
> +static struct resource ioc3_eth_resources[] = {
> + DEFINE_RES_MEM(offsetof(struct ioc3, eth),
> + sizeof_field(struct ioc3, eth)),
> + DEFINE_RES_MEM(offsetof(struct ioc3, ssram),
> + sizeof_field(struct ioc3, ssram)),
> + DEFINE_RES_IRQ(0)
> +};
> +
> +static struct ioc3eth_platform_data ioc3_eth_platform_data;
I doubt you'll need this in here. The MAC address info will need to
be moved out to a networking driver of some sort.
> +#ifdef CONFIG_SGI_IP27
#ifery in C files is highly discouraged. Please remove them.
> +static struct resource ioc3_rtc_resources[] = {
> + DEFINE_RES_MEM(IOC3_BYTEBUS_DEV0, 32768)
Define all magic numbers please.
> +};
> +
> +static struct ioc3_board_info ip27_baseio_info = {
> + .name = "IP27 BaseIO",
> + .funcs = IOC3_ETH | IOC3_SER | IOC3_M48T35,
> + .irq_offset = 2
> +};
> +
> +static struct ioc3_board_info ip27_baseio6g_info = {
> + .name = "IP27 BaseIO6G",
> + .funcs = IOC3_ETH | IOC3_SER | IOC3_KBD | IOC3_M48T35,
> + .irq_offset = 2
> +};
> +
> +static struct ioc3_board_info ip27_mio_info = {
> + .name = "MIO",
> + .funcs = IOC3_SER | IOC3_PAR | IOC3_KBD,
> + .irq_offset = 0
> +};
> +
> +static struct ioc3_board_info ip29_baseio_info = {
> + .name = "IP29 System Board",
> + .funcs = IOC3_ETH | IOC3_SER | IOC3_PAR | IOC3_KBD | IOC3_M48T35,
> + .irq_offset = 1
> +};
> +
> +#endif /* CONFIG_SGI_IP27 */
> +
> +static struct ioc3_board_info ioc3_menet_info = {
> + .name = "MENET",
> + .funcs = IOC3_ETH | IOC3_SER,
> + .irq_offset = 4
> +};
> +
> +static struct ioc3_board_info ioc3_cad_duo_info = {
> + .name = "CAD DUO",
> + .funcs = IOC3_ETH | IOC3_KBD,
> + .irq_offset = 0
> +};
Please drop all of these and statically create the MFD cells like
almost all other MFD drivers do.
> +#define IOC3_BOARD(_partno, _info) { .info = _info, .match = _partno }
> +
> +static struct {
> + struct ioc3_board_info *info;
> + const char *match;
> +} ioc3_boards[] = {
> +#ifdef CONFIG_SGI_IP27
> + IOC3_BOARD("030-0734-", &ip27_baseio6g_info),
> + IOC3_BOARD("030-1023-", &ip27_baseio_info),
> + IOC3_BOARD("030-1124-", &ip27_baseio_info),
> + IOC3_BOARD("030-1025-", &ip29_baseio_info),
> + IOC3_BOARD("030-1244-", &ip29_baseio_info),
> + IOC3_BOARD("030-1389-", &ip29_baseio_info),
> + IOC3_BOARD("030-0880-", &ip27_mio_info),
> +#endif
> + IOC3_BOARD("030-0873-", &ioc3_menet_info),
> + IOC3_BOARD("030-1155-", &ioc3_cad_duo_info),
> +};
> +
> +static int ioc3_identify(struct ioc3_priv_data *idp)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(ioc3_boards); i++)
> + if (!strncmp(idp->nic_part, ioc3_boards[i].match,
> + strlen(ioc3_boards[i].match))) {
> + idp->info = ioc3_boards[i].info;
> + return 0;
> + }
> +
> + return -1;
Please return a proper Linux error code.
> +}
> +
> +static void ioc3_create_devices(struct ioc3_priv_data *ipd)
> +{
> + struct mfd_cell *cell;
> +
> + memset(ioc3_mfd_cells, 0, sizeof(ioc3_mfd_cells));
> + cell = ioc3_mfd_cells;
> +
> + if (ipd->info->funcs & IOC3_ETH) {
> + memcpy(ioc3_eth_platform_data.mac_addr, ipd->nic_mac,
> + sizeof(ioc3_eth_platform_data.mac_addr));
Better to pull the MAC address from within the Ethernet driver.
> + cell->name = "ioc3-eth";
> + cell->id = ioc3_eth_id++;
> + cell->resources = ioc3_eth_resources;
> + cell->num_resources = ARRAY_SIZE(ioc3_eth_resources);
> + cell->platform_data = &ioc3_eth_platform_data;
> + cell->pdata_size = sizeof(ioc3_eth_platform_data);
Please define all of this in a static struct, external to this
function.
> + if (ipd->info->irq_offset) {
> + /*
> + * Ethernet interrupt is on an extra interrupt
> + * not inside the irq domain, so we need an
> + * extra mfd_add_devices without the domain
> + * argument
> + */
> + ioc3_eth_resources[2].start = ipd->pdev->irq;
> + ioc3_eth_resources[2].end = ipd->pdev->irq;
Using '2' is fragile.
In this case, seeing as you're using the parent's IRQ, you can obtain
the IRQ using the usual platform_*() handlers, using pdev->parent.
> + mfd_add_devices(&ipd->pdev->dev, -1, cell, 1,
Don't use -1, use the defines instead.
Instead of 1, use ARRAY_SIZE() once the cells are defined statically.
> + &ipd->pdev->resource[0], 0, NULL);
> + memset(cell, 0, sizeof(*cell));
> + } else {
> + /* fake hwirq in domain */
What does this mean?
> + ioc3_eth_resources[2].start = 23;
> + ioc3_eth_resources[2].end = 23;
> + cell++;
> + }
> + }
> + if (ipd->info->funcs & IOC3_SER) {
> + writel(GPCR_UARTA_MODESEL | GPCR_UARTB_MODESEL,
> + &ipd->regs->gpcr_s);
> + writel(0, &ipd->regs->gppr[6]);
> + writel(0, &ipd->regs->gppr[7]);
> + udelay(100);
> + writel(readl(&ipd->regs->port_a.sscr) & ~SSCR_DMA_EN,
> + &ipd->regs->port_a.sscr);
> + writel(readl(&ipd->regs->port_b.sscr) & ~SSCR_DMA_EN,
> + &ipd->regs->port_b.sscr);
> + udelay(1000);
No idea what any of this does.
It looks like it belongs in the serial driver (and needs comments).
> + cell->name = "ioc3-serial8250";
> + cell->id = ioc3_serial_id++;
> + cell->resources = ioc3_uarta_resources;
> + cell->num_resources = ARRAY_SIZE(ioc3_uarta_resources);
> + cell++;
> + cell->name = "ioc3-serial8250";
> + cell->id = ioc3_serial_id++;
> + cell->resources = ioc3_uartb_resources;
> + cell->num_resources = ARRAY_SIZE(ioc3_uartb_resources);
> + cell++;
Please define this statically.
> + }
> + if (ipd->info->funcs & IOC3_KBD) {
> + cell->name = "ioc3-kbd",
> + cell->id = ioc3_kbd_id++;
> + cell->resources = ioc3_kbd_resources,
> + cell->num_resources = ARRAY_SIZE(ioc3_kbd_resources),
> + cell++;
As above.
> + }
> +#if defined(CONFIG_SGI_IP27)
What is this? Can't you obtain this dynamically by probing the H/W?
> + if (ipd->info->funcs & IOC3_M48T35) {
> + cell->name = "rtc-m48t35";
> + cell->id = -1;
> + cell->resources = ioc3_rtc_resources;
> + cell->num_resources = ARRAY_SIZE(ioc3_rtc_resources);
> + cell++;
Static please.
> + }
> +#endif
> + mfd_add_devices(&ipd->pdev->dev, -1, ioc3_mfd_cells,
Use the defines.
> + cell - ioc3_mfd_cells, &ipd->pdev->resource[0],
?
> + 0, ipd->domain);
> +}
> +
> +static int ioc3_mfd_probe(struct pci_dev *pdev,
> + const struct pci_device_id *pci_id)
> +{
> + struct ioc3_priv_data *ipd;
> + int err, ret = -ENODEV, io_irqno;
> + struct ioc3 __iomem *regs;
> + struct irq_domain *domain;
> + struct fwnode_handle *fn;
> +
> + err = pci_enable_device(pdev);
> + if (err)
> + return err;
> +
> + pci_write_config_byte(pdev, PCI_LATENCY_TIMER, 64);
> + pci_set_master(pdev);
> +
> + ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
> + if (ret) {
> + dev_warn(&pdev->dev, "Warning: couldn_t set 64-bit DMA mask\n");
> + ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> + if (ret) {
> + dev_err(&pdev->dev, "Can't set DMA mask, aborting\n");
> + return ret;
> + }
> + }
> +
> + /* Set up per-IOC3 data */
> + ipd = kzalloc(sizeof(struct ioc3_priv_data), GFP_KERNEL);
Does PCI not provide managed (devm_*() like) helpers?
> + if (!ipd) {
> + ret = -ENOMEM;
> + goto out_disable_device;
> + }
> + ipd->pdev = pdev;
'\n'
> + /*
> + * Map all IOC3 registers. These are shared between subdevices
> + * so the main IOC3 module manages them.
> + */
> + regs = pci_ioremap_bar(pdev, 0);
> + if (!regs) {
> + pr_warn("ioc3: Unable to remap PCI BAR for %s.\n",
> + pci_name(pdev));
> + goto out_free_ipd;
> + }
> + ipd->regs = regs;
> +
> + /* Track PCI-device specific data */
> + pci_set_drvdata(pdev, ipd);
Do you don't need 'ipd->pdev = pdev' then.
> + writel(GPCR_MLAN_EN, &ipd->regs->gpcr_s);
> + ioc3_probe_nic(ipd, ®s->mcr);
This should probably be moved to the networking driver.
> +#ifdef CONFIG_SGI_IP27
No #ifery in MFD c-files please.
> + /* BaseIO NIC is attached to bridge */
> + if (!ipd->nic_part[0]) {
> + struct bridge_controller *bc = BRIDGE_CONTROLLER(pdev->bus);
> +
> + ioc3_probe_nic(ipd, &bc->base->b_nic);
> + }
> +#endif
> +
> + if (ioc3_identify(ipd)) {
> + pr_err("ioc3: part: [%s] unknown card\n", ipd->nic_part);
> + goto out_iounmap;
> + }
> +
> + pr_info("ioc3: part: [%s] %s\n", ipd->nic_part, ipd->info->name);
> +
> + /* Clear IRQs */
A comment! I just fell off my chair! =;-)
> + writel(~0, ®s->sio_iec);
> + writel(~0, ®s->sio_ir);
> + writel(0, ®s->eth.eier);
> + writel(~0, ®s->eth.eisr);
> +
> + if (ipd->info->irq_offset) {
What does this really signify?
> + struct pci_host_bridge *hbrg = pci_find_host_bridge(pdev->bus);
> +
> + io_irqno = hbrg->map_irq(pdev,
> + PCI_SLOT(pdev->devfn) + ipd->info->irq_offset, 0);
> + } else {
> + io_irqno = pdev->irq;
> + }
> + ipd->irq_io = io_irqno;
> +
> + fn = irq_domain_alloc_named_fwnode("IOC3");
> + if (!fn)
> + goto out_iounmap;
> +
> + domain = irq_domain_create_linear(fn, 24, &ioc3_irq_domain_ops, ipd);
> + irq_domain_free_fwnode(fn);
> + if (!domain)
> + goto out_iounmap;
> + ipd->domain = domain;
> +
> + irq_set_chained_handler_and_data(io_irqno, ioc3_irq_handler, domain);
> +
> + ioc3_create_devices(ipd);
> +
> + return 0;
> +
> +out_iounmap:
> + iounmap(ipd->regs);
> +
> +out_free_ipd:
> + kfree(ipd);
> +
> +out_disable_device:
> + pci_disable_device(pdev);
> + return ret;
> +}
> +
> +static void ioc3_mfd_remove(struct pci_dev *pdev)
> +{
> + struct ioc3_priv_data *ipd;
> +
> + ipd = pci_get_drvdata(pdev);
> +
> + /* Clear and disable all IRQs */
> + writel(~0, &ipd->regs->sio_iec);
> + writel(~0, &ipd->regs->sio_ir);
> +
> + /* Release resources */
> + irq_domain_remove(ipd->domain);
> + free_irq(ipd->irq_io, (void *)ipd);
> + iounmap(ipd->regs);
> +
> + pci_disable_device(pdev);
> +
> + kfree(ipd);
> +}
> +
> +static struct pci_device_id ioc3_mfd_id_table[] = {
> + { PCI_VENDOR_ID_SGI, PCI_DEVICE_ID_SGI_IOC3, PCI_ANY_ID, PCI_ANY_ID },
> + { 0, },
> +};
> +MODULE_DEVICE_TABLE(pci, ioc3_mfd_id_table);
> +
> +static struct pci_driver ioc3_mfd_driver = {
> + .name = "IOC3",
> + .id_table = ioc3_mfd_id_table,
> + .probe = ioc3_mfd_probe,
> + .remove = ioc3_mfd_remove,
> +};
> +
> +module_pci_driver(ioc3_mfd_driver);
> +
> +MODULE_AUTHOR("Thomas Bogendoerfer <tbogendoerfer@suse.de>");
> +MODULE_DESCRIPTION("SGI IOC3 MFD driver");
> +MODULE_LICENSE("GPL");
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH] Input: libps2 - mark expected switch fall-through
From: Gustavo A. R. Silva @ 2019-05-07 21:37 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, Kees Cook
In-Reply-To: <20190507192744.GA248929@dtor-ws>
On 5/7/19 2:27 PM, Dmitry Torokhov wrote:
> On Tue, May 07, 2019 at 01:24:09PM -0500, Gustavo A. R. Silva wrote:
>> In preparation to enabling -Wimplicit-fallthrough, mark switch
>> cases where we are expecting to fall through.
>>
>> This patch fixes the following warning:
>>
>> drivers/input/serio/libps2.c: In function ‘ps2_handle_ack’:
>> drivers/input/serio/libps2.c:407:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
>> if (ps2dev->flags & PS2_FLAG_NAK) {
>> ^
>> drivers/input/serio/libps2.c:417:2: note: here
>> case 0x00:
>> ^~~~
>>
>> Warning level 3 was used: -Wimplicit-fallthrough=3
>>
>> This patch is part of the ongoing efforts to enable
>> -Wimplicit-fallthrough.
>>
>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>
> Applied, thank you.
>
Awesome. :)
Thanks, Dmitry.
--
Gustavo
^ permalink raw reply
* Re: [PATCH] Input: libps2 - mark expected switch fall-through
From: Dmitry Torokhov @ 2019-05-07 19:27 UTC (permalink / raw)
To: Gustavo A. R. Silva; +Cc: linux-input, linux-kernel, Kees Cook
In-Reply-To: <20190507182409.GA11027@embeddedor>
On Tue, May 07, 2019 at 01:24:09PM -0500, Gustavo A. R. Silva wrote:
> In preparation to enabling -Wimplicit-fallthrough, mark switch
> cases where we are expecting to fall through.
>
> This patch fixes the following warning:
>
> drivers/input/serio/libps2.c: In function ‘ps2_handle_ack’:
> drivers/input/serio/libps2.c:407:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
> if (ps2dev->flags & PS2_FLAG_NAK) {
> ^
> drivers/input/serio/libps2.c:417:2: note: here
> case 0x00:
> ^~~~
>
> Warning level 3 was used: -Wimplicit-fallthrough=3
>
> This patch is part of the ongoing efforts to enable
> -Wimplicit-fallthrough.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Applied, thank you.
> ---
> drivers/input/serio/libps2.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
> index e6a07e68d1ff..22b8e05aa36c 100644
> --- a/drivers/input/serio/libps2.c
> +++ b/drivers/input/serio/libps2.c
> @@ -409,6 +409,7 @@ bool ps2_handle_ack(struct ps2dev *ps2dev, u8 data)
> ps2dev->nak = PS2_RET_ERR;
> break;
> }
> + /* Fall through */
>
> /*
> * Workaround for mice which don't ACK the Get ID command.
> --
> 2.21.0
>
--
Dmitry
^ permalink raw reply
* [PATCH 3/3] HID: wacom: Sync INTUOSP2_BT touch state after each frame if necessary
From: Gerecke, Jason @ 2019-05-07 18:53 UTC (permalink / raw)
To: linux-input, Benjamin Tissoires
Cc: Ping Cheng, Aaron Armstrong Skomra, Jason Gerecke, stable
In-Reply-To: <20190507185322.7168-1-jason.gerecke@wacom.com>
From: Jason Gerecke <jason.gerecke@wacom.com>
The Bluetooth interface of the 2nd-gen Intuos Pro batches together four
independent "frames" of finger data into a single report. Each frame
is essentially equivalent to a single USB report, with the up-to-10
fingers worth of information being spread across two frames. At the
moment the driver only calls `input_sync` after processing all four
frames have been processed, which can result in the driver sending
multiple updates for a single slot within the same SYN_REPORT. This
can confuse userspace, so modify the driver to sync more often if
necessary (i.e., after reporting the state of all fingers).
Fixes: 4922cd26f0 ("HID: wacom: Support 2nd-gen Intuos Pro's Bluetooth classic interface")
Cc: <stable@vger.kernel.org> # 4.11+
Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
---
drivers/hid/wacom_wac.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index e848445236d8..09b8e4aac82f 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -1371,11 +1371,17 @@ static void wacom_intuos_pro2_bt_touch(struct wacom_wac *wacom)
if (wacom->num_contacts_left <= 0) {
wacom->num_contacts_left = 0;
wacom->shared->touch_down = wacom_wac_finger_count_touches(wacom);
+ input_sync(touch_input);
}
}
- input_report_switch(touch_input, SW_MUTE_DEVICE, !(data[281] >> 7));
- input_sync(touch_input);
+ if (wacom->num_contacts_left == 0) {
+ // Be careful that we don't accidentally call input_sync with
+ // only a partial set of fingers of processed
+ input_report_switch(touch_input, SW_MUTE_DEVICE, !(data[281] >> 7));
+ input_sync(touch_input);
+ }
+
}
static void wacom_intuos_pro2_bt_pad(struct wacom_wac *wacom)
--
2.21.0
^ permalink raw reply related
* [PATCH 2/3] HID: wacom: Correct button numbering 2nd-gen Intuos Pro over Bluetooth
From: Gerecke, Jason @ 2019-05-07 18:53 UTC (permalink / raw)
To: linux-input, Benjamin Tissoires
Cc: Ping Cheng, Aaron Armstrong Skomra, Jason Gerecke, stable,
Aaron Skomra
In-Reply-To: <20190507185322.7168-1-jason.gerecke@wacom.com>
From: Jason Gerecke <jason.gerecke@wacom.com>
The button numbering of the 2nd-gen Intuos Pro is not consistent between
the USB and Bluetooth interfaces. Over USB, the HID_GENERIC codepath
enumerates the eight ExpressKeys first (BTN_0 - BTN_7) followed by the
center modeswitch button (BTN_8). The Bluetooth codepath, however, has
the center modeswitch button as BTN_0 and the the eight ExpressKeys as
BTN_1 - BTN_8. To ensure userspace button mappings do not change
depending on how the tablet is connected, modify the Bluetooth codepath
to report buttons in the same order as USB.
To ensure the mode switch LED continues to toggle in response to the
mode switch button, the `wacom_is_led_toggled` function also requires
a small update.
Link: https://github.com/linuxwacom/input-wacom/pull/79
Fixes: 4922cd26f0 ("HID: wacom: Support 2nd-gen Intuos Pro's Bluetooth classic interface")
Cc: <stable@vger.kernel.org> # 4.11+
Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
Reviewed-by: Aaron Skomra <aaron.skomra@wacom.com>
---
drivers/hid/wacom_wac.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index af62a630fee9..e848445236d8 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -1383,7 +1383,7 @@ static void wacom_intuos_pro2_bt_pad(struct wacom_wac *wacom)
struct input_dev *pad_input = wacom->pad_input;
unsigned char *data = wacom->data;
- int buttons = (data[282] << 1) | ((data[281] >> 6) & 0x01);
+ int buttons = data[282] | ((data[281] & 0x40) << 2);
int ring = data[285] & 0x7F;
bool ringstatus = data[285] & 0x80;
bool prox = buttons || ringstatus;
@@ -3832,7 +3832,7 @@ static void wacom_24hd_update_leds(struct wacom *wacom, int mask, int group)
static bool wacom_is_led_toggled(struct wacom *wacom, int button_count,
int mask, int group)
{
- int button_per_group;
+ int group_button;
/*
* 21UX2 has LED group 1 to the left and LED group 0
@@ -3842,9 +3842,12 @@ static bool wacom_is_led_toggled(struct wacom *wacom, int button_count,
if (wacom->wacom_wac.features.type == WACOM_21UX2)
group = 1 - group;
- button_per_group = button_count/wacom->led.count;
+ group_button = group * (button_count/wacom->led.count);
- return mask & (1 << (group * button_per_group));
+ if (wacom->wacom_wac.features.type == INTUOSP2_BT)
+ group_button = 8;
+
+ return mask & (1 << group_button);
}
static void wacom_update_led(struct wacom *wacom, int button_count, int mask,
--
2.21.0
^ permalink raw reply related
* [PATCH 1/3] HID: wacom: Send BTN_TOUCH in response to INTUOSP2_BT eraser contact
From: Gerecke, Jason @ 2019-05-07 18:53 UTC (permalink / raw)
To: linux-input, Benjamin Tissoires
Cc: Ping Cheng, Aaron Armstrong Skomra, Jason Gerecke, stable,
Aaron Skomra
From: Jason Gerecke <jason.gerecke@wacom.com>
The Bluetooth reports from the 2nd-gen Intuos Pro have separate bits for
indicating if the tip or eraser is in contact with the tablet. At the
moment, only the tip contact bit controls the state of the BTN_TOUCH
event. This prevents the eraser from working as expected. This commit
changes the driver to send BTN_TOUCH whenever either the tip or eraser
contact bit is set.
Fixes: 4922cd26f0 ("HID: wacom: Support 2nd-gen Intuos Pro's Bluetooth classic interface")
Cc: <stable@vger.kernel.org> # 4.11+
Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
Reviewed-by: Aaron Skomra <aaron.skomra@wacom.com>
---
drivers/hid/wacom_wac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 613342bb9d6b..af62a630fee9 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -1301,7 +1301,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
range ? frame[7] : wacom->features.distance_max);
}
- input_report_key(pen_input, BTN_TOUCH, frame[0] & 0x01);
+ input_report_key(pen_input, BTN_TOUCH, frame[0] & 0x09);
input_report_key(pen_input, BTN_STYLUS, frame[0] & 0x02);
input_report_key(pen_input, BTN_STYLUS2, frame[0] & 0x04);
--
2.21.0
^ permalink raw reply related
* Re: [PATCH 1/2] HID: wacom: Don't set tool type until we're in range
From: Jason Gerecke @ 2019-05-07 18:43 UTC (permalink / raw)
To: Linux Input, Benjamin Tissoires
Cc: Ping Cheng, Aaron Armstrong Skomra, Jason Gerecke, # v3.17+,
Aaron Armstrong Skomra
In-Reply-To: <20190424221258.19992-1-jason.gerecke@wacom.com>
Haven't heard anything back from you about this patch set, Benjamin.
Just making sure it doesn't get lost down a crack :)
Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one /
(That is to say, eight) to the two, /
But you can’t take seven from three, /
So you look at the sixty-fours....
On Wed, Apr 24, 2019 at 3:13 PM Gerecke, Jason <killertofu@gmail.com> wrote:
>
> From: Jason Gerecke <jason.gerecke@wacom.com>
>
> The serial number and tool type information that is reported by the tablet
> while a pen is merely "in prox" instead of fully "in range" can be stale
> and cause us to report incorrect tool information. Serial number, tool
> type, and other information is only valid once the pen comes fully in range
> so we should be careful to not use this information until that point.
>
> In particular, this issue may cause the driver to incorectly report
> BTN_TOOL_RUBBER after switching from the eraser tool back to the pen.
>
> Fixes: a48324de6d ("HID: wacom: Bluetooth IRQ for Intuos Pro should handle prox/range")
> Cc: <stable@vger.kernel.org> # 4.11+
> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> Reviewed-by: Aaron Armstrong Skomra <aaron.skomra@wacom.com>
> ---
> drivers/hid/wacom_wac.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index 747730d32ab6..4c1bc239207e 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -1236,13 +1236,13 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
> /* Add back in missing bits of ID for non-USI pens */
> wacom->id[0] |= (wacom->serial[0] >> 32) & 0xFFFFF;
> }
> - wacom->tool[0] = wacom_intuos_get_tool_type(wacom_intuos_id_mangle(wacom->id[0]));
>
> for (i = 0; i < pen_frames; i++) {
> unsigned char *frame = &data[i*pen_frame_len + 1];
> bool valid = frame[0] & 0x80;
> bool prox = frame[0] & 0x40;
> bool range = frame[0] & 0x20;
> + bool invert = frame[0] & 0x10;
>
> if (!valid)
> continue;
> @@ -1251,9 +1251,24 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
> wacom->shared->stylus_in_proximity = false;
> wacom_exit_report(wacom);
> input_sync(pen_input);
> +
> + wacom->tool[0] = 0;
> + wacom->id[0] = 0;
> + wacom->serial[0] = 0;
> return;
> }
> +
> if (range) {
> + if (!wacom->tool[0]) { /* first in range */
> + /* Going into range select tool */
> + if (invert)
> + wacom->tool[0] = BTN_TOOL_RUBBER;
> + else if (wacom->id[0])
> + wacom->tool[0] = wacom_intuos_get_tool_type(wacom->id[0]);
> + else
> + wacom->tool[0] = BTN_TOOL_PEN;
> + }
> +
> input_report_abs(pen_input, ABS_X, get_unaligned_le16(&frame[1]));
> input_report_abs(pen_input, ABS_Y, get_unaligned_le16(&frame[3]));
>
> --
> 2.21.0
>
^ permalink raw reply
* [PATCH] Input: libps2 - mark expected switch fall-through
From: Gustavo A. R. Silva @ 2019-05-07 18:24 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, Gustavo A. R. Silva, Kees Cook
In preparation to enabling -Wimplicit-fallthrough, mark switch
cases where we are expecting to fall through.
This patch fixes the following warning:
drivers/input/serio/libps2.c: In function ‘ps2_handle_ack’:
drivers/input/serio/libps2.c:407:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
if (ps2dev->flags & PS2_FLAG_NAK) {
^
drivers/input/serio/libps2.c:417:2: note: here
case 0x00:
^~~~
Warning level 3 was used: -Wimplicit-fallthrough=3
This patch is part of the ongoing efforts to enable
-Wimplicit-fallthrough.
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
drivers/input/serio/libps2.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
index e6a07e68d1ff..22b8e05aa36c 100644
--- a/drivers/input/serio/libps2.c
+++ b/drivers/input/serio/libps2.c
@@ -409,6 +409,7 @@ bool ps2_handle_ack(struct ps2dev *ps2dev, u8 data)
ps2dev->nak = PS2_RET_ERR;
break;
}
+ /* Fall through */
/*
* Workaround for mice which don't ACK the Get ID command.
--
2.21.0
^ permalink raw reply related
* Re: [PATCH 1/3] mfd: apple-ibridge: Add Apple iBridge MFD driver.
From: Lee Jones @ 2019-05-07 12:24 UTC (permalink / raw)
To: Ronald Tschalär
Cc: Jiri Kosina, Benjamin Tissoires, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, linux-input,
linux-iio, linux-kernel
In-Reply-To: <20190422031251.11968-2-ronald@innovation.ch>
On Sun, 21 Apr 2019, Ronald Tschalär wrote:
> The iBridge device provides access to several devices, including:
> - the Touch Bar
> - the iSight webcam
> - the light sensor
> - the fingerprint sensor
>
> This driver provides the core support for managing the iBridge device
> and the access to the underlying devices. In particular, since the
> functionality for the touch bar and light sensor is exposed via USB HID
> interfaces, and the same HID device is used for multiple functions, this
> driver provides a multiplexing layer that allows multiple HID drivers to
> be registered for a given HID device. This allows the touch bar and ALS
> driver to be separated out into their own modules.
>
> Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
> ---
> drivers/mfd/Kconfig | 15 +
> drivers/mfd/Makefile | 1 +
> drivers/mfd/apple-ibridge.c | 883 ++++++++++++++++++++++++++++++
I haven't taken a thorough look through, but I can tell you that the
vast majority of what you're trying to do here does not belong in
MFD. MFD drivers are used to register child devices. Almost all
functionality or 'real work' should be contained in the drivers the
MFD registers, not in the MFD parent itself. You will need to move
all 'real work' out into the subordinate device drivers for
acceptance.
> include/linux/mfd/apple-ibridge.h | 39 ++
> 4 files changed, 938 insertions(+)
> create mode 100644 drivers/mfd/apple-ibridge.c
> create mode 100644 include/linux/mfd/apple-ibridge.h
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH][next] Input: qt1050: fix less than zero comparison on an unsigned int
From: Colin Ian King @ 2019-05-07 8:46 UTC (permalink / raw)
To: Marco Felsch
Cc: Dmitry Torokhov, Rob Herring, linux-input, kernel-janitors,
linux-kernel
In-Reply-To: <20190507083214.rcew5cjfvlbwbov5@pengutronix.de>
On 07/05/2019 09:32, Marco Felsch wrote:
> Hi Ian,
>
> On 19-05-07 09:21, Colin King wrote:
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> Currently the less than zero comparison of val is always false because
>> val is an unsigned int. Fix this by making val a signed int.
>
> Thanks for covering that, was an copy 'n' paste failure..
>>
>> Addresses-Coverity: ("Unsigned compared against zero")
>> Fixes: a33ff45923c8 ("Input: qt1050 - add Microchip AT42QT1050 support")
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>> drivers/input/keyboard/qt1050.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/input/keyboard/qt1050.c b/drivers/input/keyboard/qt1050.c
>> index 6b1603cb7515..4debddb13972 100644
>> --- a/drivers/input/keyboard/qt1050.c
>> +++ b/drivers/input/keyboard/qt1050.c
>> @@ -222,7 +222,7 @@ static struct regmap_config qt1050_regmap_config = {
>>
>> static bool qt1050_identify(struct qt1050_priv *ts)
>> {
>> - unsigned int val;
>> + int val;
>
> I think the proper solution is to add a ret val, because this covers the
> success/fail. I will send a patch to fix this.
OK, thanks for the follow-up fix.
Regards,
Colin
>
> Regards,
> Marco
>
>> /* Read Chip ID */
>> regmap_read(ts->regmap, QT1050_CHIP_ID, &val);
>> --
>> 2.20.1
>>
>>
>
^ permalink raw reply
* Re: [PATCH][next] Input: qt1050: fix less than zero comparison on an unsigned int
From: Marco Felsch @ 2019-05-07 8:32 UTC (permalink / raw)
To: Colin King
Cc: Dmitry Torokhov, Rob Herring, linux-input, kernel-janitors,
linux-kernel
In-Reply-To: <20190507082135.21538-1-colin.king@canonical.com>
Hi Ian,
On 19-05-07 09:21, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Currently the less than zero comparison of val is always false because
> val is an unsigned int. Fix this by making val a signed int.
Thanks for covering that, was an copy 'n' paste failure..
>
> Addresses-Coverity: ("Unsigned compared against zero")
> Fixes: a33ff45923c8 ("Input: qt1050 - add Microchip AT42QT1050 support")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/input/keyboard/qt1050.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/keyboard/qt1050.c b/drivers/input/keyboard/qt1050.c
> index 6b1603cb7515..4debddb13972 100644
> --- a/drivers/input/keyboard/qt1050.c
> +++ b/drivers/input/keyboard/qt1050.c
> @@ -222,7 +222,7 @@ static struct regmap_config qt1050_regmap_config = {
>
> static bool qt1050_identify(struct qt1050_priv *ts)
> {
> - unsigned int val;
> + int val;
I think the proper solution is to add a ret val, because this covers the
success/fail. I will send a patch to fix this.
Regards,
Marco
> /* Read Chip ID */
> regmap_read(ts->regmap, QT1050_CHIP_ID, &val);
> --
> 2.20.1
>
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH][next] Input: qt1050: fix less than zero comparison on an unsigned int
From: Dan Carpenter @ 2019-05-07 8:27 UTC (permalink / raw)
To: Colin King
Cc: Dmitry Torokhov, Rob Herring, Marco Felsch, linux-input,
kernel-janitors, linux-kernel
In-Reply-To: <20190507082135.21538-1-colin.king@canonical.com>
On Tue, May 07, 2019 at 09:21:35AM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Currently the less than zero comparison of val is always false because
> val is an unsigned int. Fix this by making val a signed int.
>
> Addresses-Coverity: ("Unsigned compared against zero")
> Fixes: a33ff45923c8 ("Input: qt1050 - add Microchip AT42QT1050 support")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/input/keyboard/qt1050.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/keyboard/qt1050.c b/drivers/input/keyboard/qt1050.c
> index 6b1603cb7515..4debddb13972 100644
> --- a/drivers/input/keyboard/qt1050.c
> +++ b/drivers/input/keyboard/qt1050.c
> @@ -222,7 +222,7 @@ static struct regmap_config qt1050_regmap_config = {
>
> static bool qt1050_identify(struct qt1050_priv *ts)
> {
> - unsigned int val;
> + int val;
This code is checking the wrong thing anyway. It should be:
int ret;
ret = regmap_read(&val);
if (ret)
return false;
regards,
dan carpenter
^ permalink raw reply
* [PATCH][next] Input: qt1050: fix less than zero comparison on an unsigned int
From: Colin King @ 2019-05-07 8:21 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Marco Felsch, linux-input
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Currently the less than zero comparison of val is always false because
val is an unsigned int. Fix this by making val a signed int.
Addresses-Coverity: ("Unsigned compared against zero")
Fixes: a33ff45923c8 ("Input: qt1050 - add Microchip AT42QT1050 support")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/input/keyboard/qt1050.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/keyboard/qt1050.c b/drivers/input/keyboard/qt1050.c
index 6b1603cb7515..4debddb13972 100644
--- a/drivers/input/keyboard/qt1050.c
+++ b/drivers/input/keyboard/qt1050.c
@@ -222,7 +222,7 @@ static struct regmap_config qt1050_regmap_config = {
static bool qt1050_identify(struct qt1050_priv *ts)
{
- unsigned int val;
+ int val;
/* Read Chip ID */
regmap_read(ts->regmap, QT1050_CHIP_ID, &val);
--
2.20.1
^ permalink raw reply related
* Re: [PATCH -next] hid: fix hid-logitech-dj build error
From: Benjamin Tissoires @ 2019-05-07 7:04 UTC (permalink / raw)
To: Randy Dunlap
Cc: LKML, Andrew Morton, linux-input@vger.kernel.org, Jiri Kosina,
kbuild test robot
In-Reply-To: <e2154a13-0ec5-d39c-52cf-db98867b0496@infradead.org>
Hi Randy,
On Tue, May 7, 2019 at 3:12 AM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> From: Randy Dunlap <rdunlap@infradead.org>
>
> Fix build error in hid-logitech-dj by making it depend on
> USB_HID, like several other HID drivers do.
>
> Fixes this build error:
>
> ERROR: "usb_hid_driver" [drivers/hid/hid-logitech-dj.ko] undefined!
>
> Reported-by: kbuild test robot <lkp@intel.com>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Cc: Jiri Kosina <jikos@kernel.org>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Cc: linux-input@vger.kernel.org
> Cc: Andrew Morton <akpm@linux-foundation.org>
> ---
> drivers/hid/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> --- mmotm-2019-0425-1630.orig/drivers/hid/Kconfig
> +++ mmotm-2019-0425-1630/drivers/hid/Kconfig
> @@ -522,6 +522,7 @@ config HID_LOGITECH
> config HID_LOGITECH_DJ
> tristate "Logitech Unifying receivers full support"
> depends on HIDRAW
> + depends on USB_HID
this is already scheduled in the HID PR:
https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/commit/?h=for-linus&id=c08f38e9fd0b5486957ed42438ec8fa9b6ebbf4f
Cheers,
Benjamin
> depends on HID_LOGITECH
> select HID_LOGITECH_HIDPP
> ---help---
>
>
^ permalink raw reply
* [PATCH AUTOSEL 3.18 02/10] HID: input: add mapping for keyboard Brightness Up/Down/Toggle keys
From: Sasha Levin @ 2019-05-07 5:42 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Dmitry Torokhov, Sasha Levin, linux-input
In-Reply-To: <20190507054247.537-1-sashal@kernel.org>
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
[ Upstream commit 7975a1d6a7afeb3eb61c971a153d24dd8fa032f3 ]
According to HUTRR73 usages 0x79, 0x7a and 0x7c from the consumer page
correspond to Brightness Up/Down/Toggle keys, so let's add the mappings.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-input.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index bb870ee75a90..b7d5a8835424 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -745,6 +745,10 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
case 0x074: map_key_clear(KEY_BRIGHTNESS_MAX); break;
case 0x075: map_key_clear(KEY_BRIGHTNESS_AUTO); break;
+ case 0x079: map_key_clear(KEY_KBDILLUMUP); break;
+ case 0x07a: map_key_clear(KEY_KBDILLUMDOWN); break;
+ case 0x07c: map_key_clear(KEY_KBDILLUMTOGGLE); break;
+
case 0x082: map_key_clear(KEY_VIDEO_NEXT); break;
case 0x083: map_key_clear(KEY_LAST); break;
case 0x084: map_key_clear(KEY_ENTER); break;
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.4 06/14] Input: snvs_pwrkey - initialize necessary driver data before enabling IRQ
From: Sasha Levin @ 2019-05-07 5:42 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Anson Huang, Anson Huang, Dmitry Torokhov, Sasha Levin,
linux-input
In-Reply-To: <20190507054218.340-1-sashal@kernel.org>
From: Anson Huang <anson.huang@nxp.com>
[ Upstream commit bf2a7ca39fd3ab47ef71c621a7ee69d1813b1f97 ]
SNVS IRQ is requested before necessary driver data initialized,
if there is a pending IRQ during driver probe phase, kernel
NULL pointer panic will occur in IRQ handler. To avoid such
scenario, just initialize necessary driver data before enabling
IRQ. This patch is inspired by NXP's internal kernel tree.
Fixes: d3dc6e232215 ("input: keyboard: imx: add snvs power key driver")
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/input/keyboard/snvs_pwrkey.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboard/snvs_pwrkey.c
index 9adf13a5864a..57143365e945 100644
--- a/drivers/input/keyboard/snvs_pwrkey.c
+++ b/drivers/input/keyboard/snvs_pwrkey.c
@@ -156,6 +156,9 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
return error;
}
+ pdata->input = input;
+ platform_set_drvdata(pdev, pdata);
+
error = devm_request_irq(&pdev->dev, pdata->irq,
imx_snvs_pwrkey_interrupt,
0, pdev->name, pdev);
@@ -172,9 +175,6 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
return error;
}
- pdata->input = input;
- platform_set_drvdata(pdev, pdata);
-
device_init_wakeup(&pdev->dev, pdata->wakeup);
return 0;
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.4 03/14] HID: input: add mapping for keyboard Brightness Up/Down/Toggle keys
From: Sasha Levin @ 2019-05-07 5:42 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Dmitry Torokhov, Sasha Levin, linux-input
In-Reply-To: <20190507054218.340-1-sashal@kernel.org>
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
[ Upstream commit 7975a1d6a7afeb3eb61c971a153d24dd8fa032f3 ]
According to HUTRR73 usages 0x79, 0x7a and 0x7c from the consumer page
correspond to Brightness Up/Down/Toggle keys, so let's add the mappings.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-input.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 01b41ff43056..ee3c66c02043 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -783,6 +783,10 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
case 0x074: map_key_clear(KEY_BRIGHTNESS_MAX); break;
case 0x075: map_key_clear(KEY_BRIGHTNESS_AUTO); break;
+ case 0x079: map_key_clear(KEY_KBDILLUMUP); break;
+ case 0x07a: map_key_clear(KEY_KBDILLUMDOWN); break;
+ case 0x07c: map_key_clear(KEY_KBDILLUMTOGGLE); break;
+
case 0x082: map_key_clear(KEY_VIDEO_NEXT); break;
case 0x083: map_key_clear(KEY_LAST); break;
case 0x084: map_key_clear(KEY_ENTER); break;
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.4 02/14] HID: input: add mapping for Expose/Overview key
From: Sasha Levin @ 2019-05-07 5:42 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Dmitry Torokhov, Sasha Levin, linux-input
In-Reply-To: <20190507054218.340-1-sashal@kernel.org>
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
[ Upstream commit 96dd86871e1fffbc39e4fa61c9c75ec54ee9af0f ]
According to HUTRR77 usage 0x29f from the consumer page is reserved for
the Desktop application to present all running user’s application windows.
Linux defines KEY_SCALE to request Compiz Scale (Expose) mode, so let's
add the mapping.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-input.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 8d74e691ac90..01b41ff43056 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -913,6 +913,8 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
case 0x2cb: map_key_clear(KEY_KBDINPUTASSIST_ACCEPT); break;
case 0x2cc: map_key_clear(KEY_KBDINPUTASSIST_CANCEL); break;
+ case 0x29f: map_key_clear(KEY_SCALE); break;
+
default: map_key_clear(KEY_UNKNOWN);
}
break;
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.9 25/25] Input: synaptics-rmi4 - fix possible double free
From: Sasha Levin @ 2019-05-07 5:41 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Pan Bian, Dmitry Torokhov, Sasha Levin, linux-input
In-Reply-To: <20190507054123.32514-1-sashal@kernel.org>
From: Pan Bian <bianpan2016@163.com>
[ Upstream commit bce1a78423961fce676ac65540a31b6ffd179e6d ]
The RMI4 function structure has been released in rmi_register_function
if error occurs. However, it will be released again in the function
rmi_create_function, which may result in a double-free bug.
Signed-off-by: Pan Bian <bianpan2016@163.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/input/rmi4/rmi_driver.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index 4a88312fbd25..65038dcc7613 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -772,7 +772,7 @@ static int rmi_create_function(struct rmi_device *rmi_dev,
error = rmi_register_function(fn);
if (error)
- goto err_put_fn;
+ return error;
if (pdt->function_number == 0x01)
data->f01_container = fn;
@@ -780,10 +780,6 @@ static int rmi_create_function(struct rmi_device *rmi_dev,
list_add_tail(&fn->node, &data->function_list);
return RMI_SCAN_CONTINUE;
-
-err_put_fn:
- put_device(&fn->dev);
- return error;
}
int rmi_driver_suspend(struct rmi_device *rmi_dev)
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.9 09/25] Input: snvs_pwrkey - initialize necessary driver data before enabling IRQ
From: Sasha Levin @ 2019-05-07 5:41 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Anson Huang, Anson Huang, Dmitry Torokhov, Sasha Levin,
linux-input
In-Reply-To: <20190507054123.32514-1-sashal@kernel.org>
From: Anson Huang <anson.huang@nxp.com>
[ Upstream commit bf2a7ca39fd3ab47ef71c621a7ee69d1813b1f97 ]
SNVS IRQ is requested before necessary driver data initialized,
if there is a pending IRQ during driver probe phase, kernel
NULL pointer panic will occur in IRQ handler. To avoid such
scenario, just initialize necessary driver data before enabling
IRQ. This patch is inspired by NXP's internal kernel tree.
Fixes: d3dc6e232215 ("input: keyboard: imx: add snvs power key driver")
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/input/keyboard/snvs_pwrkey.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboard/snvs_pwrkey.c
index 7544888c4749..b8dbde746b4e 100644
--- a/drivers/input/keyboard/snvs_pwrkey.c
+++ b/drivers/input/keyboard/snvs_pwrkey.c
@@ -156,6 +156,9 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
return error;
}
+ pdata->input = input;
+ platform_set_drvdata(pdev, pdata);
+
error = devm_request_irq(&pdev->dev, pdata->irq,
imx_snvs_pwrkey_interrupt,
0, pdev->name, pdev);
@@ -171,9 +174,6 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
return error;
}
- pdata->input = input;
- platform_set_drvdata(pdev, pdata);
-
device_init_wakeup(&pdev->dev, pdata->wakeup);
return 0;
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.9 05/25] HID: input: add mapping for "Toggle Display" key
From: Sasha Levin @ 2019-05-07 5:41 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Dmitry Torokhov, Sasha Levin, linux-input
In-Reply-To: <20190507054123.32514-1-sashal@kernel.org>
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
[ Upstream commit c01908a14bf735b871170092807c618bb9dae654 ]
According to HUT 1.12 usage 0xb5 from the generic desktop page is reserved
for switching between external and internal display, so let's add the
mapping.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-input.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 302a24931147..9f7b1cf726a8 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -607,6 +607,14 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
break;
}
+ if ((usage->hid & 0xf0) == 0xb0) { /* SC - Display */
+ switch (usage->hid & 0xf) {
+ case 0x05: map_key_clear(KEY_SWITCHVIDEOMODE); break;
+ default: goto ignore;
+ }
+ break;
+ }
+
/*
* Some lazy vendors declare 255 usages for System Control,
* leading to the creation of ABS_X|Y axis and too many others.
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.9 04/25] HID: input: add mapping for keyboard Brightness Up/Down/Toggle keys
From: Sasha Levin @ 2019-05-07 5:41 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Dmitry Torokhov, Sasha Levin, linux-input
In-Reply-To: <20190507054123.32514-1-sashal@kernel.org>
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
[ Upstream commit 7975a1d6a7afeb3eb61c971a153d24dd8fa032f3 ]
According to HUTRR73 usages 0x79, 0x7a and 0x7c from the consumer page
correspond to Brightness Up/Down/Toggle keys, so let's add the mappings.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-input.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index d31725c4e7b1..302a24931147 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -802,6 +802,10 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
case 0x074: map_key_clear(KEY_BRIGHTNESS_MAX); break;
case 0x075: map_key_clear(KEY_BRIGHTNESS_AUTO); break;
+ case 0x079: map_key_clear(KEY_KBDILLUMUP); break;
+ case 0x07a: map_key_clear(KEY_KBDILLUMDOWN); break;
+ case 0x07c: map_key_clear(KEY_KBDILLUMTOGGLE); break;
+
case 0x082: map_key_clear(KEY_VIDEO_NEXT); break;
case 0x083: map_key_clear(KEY_LAST); break;
case 0x084: map_key_clear(KEY_ENTER); break;
--
2.20.1
^ 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