Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCH RESEND v4 3/4] ARM: dts: sunxi: Add R_LRADC support for A83T
From: Maxime Ripard @ 2019-04-08  8:00 UTC (permalink / raw)
  To: megous-5qf/QAjKc83QT0dZR+AlfA
  Cc: wens-jdAy2FN1RRM, robh-DgEjT+Ai2ygdnm+yROfE0A, Dmitry Torokhov,
	mark.rutland-5wv7dgnIgG8, Ziping Chen,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	hdegoede-H+wXaHxf7aLQT0dZR+AlfA
In-Reply-To: <20190327023339.25975-4-megous-5qf/QAjKc83QT0dZR+AlfA@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 525 bytes --]

On Wed, Mar 27, 2019 at 03:33:38AM +0100, megous-5qf/QAjKc83QT0dZR+AlfA@public.gmane.org wrote:
> From: Ziping Chen <techping.chan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> Allwinner A83T SoC has a low res adc like the one in Allwinner A10 SoC.
> Now the driver has been modified to support it.
>
> Add support for it.
>
> Signed-off-by: Ziping Chen <techping.chan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

It's missing your SoB.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* Re: [PATCH v4 1/2] drm/bridge: sil_sii8620: make remote control optional.
From: Andrzej Hajda @ 2019-04-08  5:49 UTC (permalink / raw)
  To: Ronald Tschalär, Dmitry Torokhov, Henrik Rydberg,
	Andy Shevchenko, Greg Kroah-Hartman
  Cc: Lukas Wunner, Federico Lorenzi, linux-input, linux-kernel,
	Inki Dae, Laurent Pinchart
In-Reply-To: <20190407050358.2976-2-ronald@innovation.ch>

On 07.04.2019 07:03, 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).
>
> 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>
Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>

 --
Regards
Andrzej

^ permalink raw reply

* [PATCH v2] Input: i8042 - disable KBD port on Razer Blade Stealth V2 (2017 model)
From: Lyude Paul @ 2019-04-07 23:46 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: stable, Chen-Yu Tsai, linux-input, linux-kernel
In-Reply-To: <20190407213735.10658-1-lyude@redhat.com>

The late 2017 model of the Razer Blade Stealth has a built-in USB
keyboard, but for some reason the BIOS exposes an i8042 controller with
a connected KBD port. While this fake AT Keyboard device doesn't appear
to report any events, attempting to change the state of the caps lock
LED on it from on to off causes the entire system to hang.

So, introduce a quirk table for disabling keyboard probing by default,
i8042_dmi_nokbd_table, and add this specific model of Razer laptop to
that table.

Changes since v1:
- Correct model (should be V2, 2017)

Signed-off-by: Lyude Paul <lyude@redhat.com>
Cc: stable@vger.kernel.org
---
 drivers/input/serio/i8042-x86ia64io.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h
index 136f6e7bf797..f65570691980 100644
--- a/drivers/input/serio/i8042-x86ia64io.h
+++ b/drivers/input/serio/i8042-x86ia64io.h
@@ -884,6 +884,22 @@ static const struct dmi_system_id __initconst i8042_dmi_kbdreset_table[] = {
 	{ }
 };
 
+static const struct dmi_system_id i8042_dmi_nokbd_table[] __initconst = {
+	{
+		/*
+		 * Razer Blade Stealth V2 (2017 model) - Keyboard is on USB
+		 * but the system exposes a fake AT keyboard that crashes the
+		 * system if the caps lock LED is changed
+		 */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Razer"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Blade Stealth"),
+			DMI_MATCH(DMI_PRODUCT_VERSION, "2.04"),
+		},
+	},
+	{ }
+};
+
 #endif /* CONFIG_X86 */
 
 #ifdef CONFIG_PNP
@@ -1040,6 +1056,9 @@ static int __init i8042_pnp_init(void)
 #ifdef CONFIG_X86
 	if (dmi_check_system(i8042_dmi_nopnp_table))
 		i8042_nopnp = true;
+
+	if (dmi_check_system(i8042_dmi_nokbd_table))
+		i8042_nokbd = true;
 #endif
 
 	if (i8042_nopnp) {
-- 
2.20.1

^ permalink raw reply related

* Re: [PATCH] Input: i8042 - disable KBD port on Late-2016 Razer Blade Stealth
From: Lyude Paul @ 2019-04-07 22:54 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: stable, Chen-Yu Tsai, linux-input, linux-kernel
In-Reply-To: <20190407221034.GA162359@dtor-ws>

On Sun, 2019-04-07 at 15:10 -0700, Dmitry Torokhov wrote:
> Hi Lyude,
> 
> On Sun, Apr 07, 2019 at 05:37:34PM -0400, Lyude Paul wrote:
> > The late 2016 model of the Razer Blade Stealth has a built-in USB
> > keyboard, but for some reason the BIOS exposes an i8042 controller with
> > a connected KBD port. While this fake AT Keyboard device doesn't appear
> > to report any events, attempting to change the state of the caps lock
> > LED on it from on to off causes the entire system to hang.
> > 
> > So, introduce a quirk table for disabling keyboard probing by default,
> > i8042_dmi_nokbd_table, and add this specific model of Razer laptop to
> > that table.
> 
> What does dmesg show about i8042 for this device? Especially line "PNP:
> PS/2 Controller  ..."?
> 

Apr 07 18:42:46 malachite kernel: i8042: PNP: No PS/2 controller found.
Apr 07 18:42:46 malachite kernel: i8042: Probing ports directly.
Apr 07 18:42:46 malachite kernel: serio: i8042 KBD port at 0x60,0x64 irq 1
Apr 07 18:42:46 malachite kernel: serio: i8042 AUX port at 0x60,0x64 irq 12
Apr 07 18:42:46 malachite kernel: mousedev: PS/2 mouse device common for all
mice
Apr 07 18:42:46 malachite kernel: rtc_cmos 00:01: RTC can wake from S4
Apr 07 18:42:46 malachite kernel: rtc_cmos 00:01: registered as rtc0
Apr 07 18:42:46 malachite kernel: rtc_cmos 00:01: alarms up to one month, y3k,
242 bytes nvram, hpet irqs
Apr 07 18:42:46 malachite kernel: device-mapper: uevent: version 1.0.3
Apr 07 18:42:46 malachite kernel: device-mapper: ioctl: 4.39.0-ioctl (2018-04-
03) initialised: dm-devel@redhat.com
Apr 07 18:42:46 malachite kernel: intel_pstate: Intel P-state driver
initializing
Apr 07 18:42:46 malachite kernel: intel_pstate: HWP enabled
Apr 07 18:42:46 malachite kernel: hidraw: raw HID events driver (C) Jiri
Kosina
Apr 07 18:42:46 malachite kernel: usbcore: registered new interface driver
usbhid
Apr 07 18:42:46 malachite kernel: usbhid: USB HID core driver
Apr 07 18:42:46 malachite kernel: intel_pmc_core:  initialized
Apr 07 18:42:46 malachite kernel: drop_monitor: Initializing network drop
monitor service
Apr 07 18:42:46 malachite kernel: Initializing XFRM netlink socket
Apr 07 18:42:46 malachite kernel: NET: Registered protocol family 10
Apr 07 18:42:46 malachite kernel: Segment Routing with IPv6
Apr 07 18:42:46 malachite kernel: mip6: Mobile IPv6
Apr 07 18:42:46 malachite kernel: NET: Registered protocol family 17
Apr 07 18:42:46 malachite kernel: RAS: Correctable Errors collector
initialized.
Apr 07 18:42:46 malachite kernel: microcode: sig=0x806e9, pf=0x80,
revision=0x8e
Apr 07 18:42:46 malachite kernel: microcode: Microcode Update Driver: v2.2.
Apr 07 18:42:46 malachite kernel: AVX2 version of gcm_enc/dec engaged.
Apr 07 18:42:46 malachite kernel: AES CTR mode by8 optimization enabled
Apr 07 18:42:46 malachite kernel: battery: ACPI: Battery Slot [BAT0] (battery
present)
Apr 07 18:42:46 malachite kernel: sched_clock: Marking stable (1166896928,
-586991)->(1172494398, -6184461)
Apr 07 18:42:46 malachite kernel: registered taskstats version 1
Apr 07 18:42:46 malachite kernel: Loading compiled-in X.509 certificates
Apr 07 18:42:46 malachite kernel: Loaded X.509 cert 'Fedora kernel signing
key: eb55b2be431426c78789899a96d617d82132041e'
Apr 07 18:42:46 malachite kernel: zswap: loaded using pool lzo/zbud
Apr 07 18:42:46 malachite kernel: Key type big_key registered
Apr 07 18:42:46 malachite kernel: Key type encrypted registered
Apr 07 18:42:46 malachite kernel: integrity: Loading X.509 certificate:
UEFI:db
Apr 07 18:42:46 malachite kernel: integrity: Loaded X.509 cert 'Microsoft
Corporation UEFI CA 2011: 13adbf4309bd82709c8cd54f316ed522988a1bd4'
Apr 07 18:42:46 malachite kernel: integrity: Loading X.509 certificate:
UEFI:db
Apr 07 18:42:46 malachite kernel: integrity: Loaded X.509 cert 'Microsoft
Windows Production PCA 2011: a92902398e16c49778cd90f99e4f9ae17c55af53'
Apr 07 18:42:46 malachite kernel: ima: Allocated hash algorithm: sha1
Apr 07 18:42:46 malachite kernel: input: AT Raw Set 2 keyboard as
/devices/platform/i8042/serio0/input/input4

Also: After doing a bit more research on wikipedia I just noticed that
this laptop is actually a late 2017 model, so I'll respond with a respun
patch in just a sec.

> Thanks.
> 
-- 
Cheers,
	Lyude Paul

^ permalink raw reply

* Re: [PATCH v2 2/3] Input: add a driver for GPIO controllable vibrators
From: Dmitry Torokhov @ 2019-04-07 22:18 UTC (permalink / raw)
  To: Luca Weiss
  Cc: Rob Herring, Mark Rutland, Mauro Carvalho Chehab,
	Pascal PAILLET-LME, Coly Li, Lee Jones, Xiaotong Lu, Brian Masney,
	Rob Herring, Baolin Wang, Andy Gross, David Brown,
	open list:ARM/QUALCOMM SUPPORT,
	open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list
In-Reply-To: <20190407155841.27354-2-luca@z3ntu.xyz>

Hi Luca,

On Sun, Apr 07, 2019 at 05:58:41PM +0200, 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>
> ---
>  drivers/input/misc/Kconfig      |  12 ++
>  drivers/input/misc/Makefile     |   1 +
>  drivers/input/misc/gpio-vibra.c | 214 ++++++++++++++++++++++++++++++++
>  3 files changed, 227 insertions(+)
>  create mode 100644 drivers/input/misc/gpio-vibra.c
> 
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index e15ed1bb8558..6dfe9e2fe5b1 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -290,6 +290,18 @@ config INPUT_GPIO_DECODER
>  	 To compile this driver as a module, choose M here: the module
>  	 will be called gpio_decoder.
>  
> +config INPUT_GPIO_VIBRA
> +	tristate "GPIO vibrator support"
> +	depends on GPIOLIB || COMPILE_TEST
> +	select INPUT_FF_MEMLESS
> +	help
> +	  Say Y here to get support for GPIO based vibrator devices.
> +
> +	  If unsure, say N.
> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called gpio-vibra.
> +
>  config INPUT_IXP4XX_BEEPER
>  	tristate "IXP4XX Beeper support"
>  	depends on ARCH_IXP4XX
> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> index b936c5b1d4ac..f38ebbdb05e2 100644
> --- a/drivers/input/misc/Makefile
> +++ b/drivers/input/misc/Makefile
> @@ -36,6 +36,7 @@ obj-$(CONFIG_INPUT_DRV2667_HAPTICS)	+= drv2667.o
>  obj-$(CONFIG_INPUT_GP2A)		+= gp2ap002a00f.o
>  obj-$(CONFIG_INPUT_GPIO_BEEPER)		+= gpio-beeper.o
>  obj-$(CONFIG_INPUT_GPIO_DECODER)	+= gpio_decoder.o
> +obj-$(CONFIG_INPUT_GPIO_VIBRA)		+= gpio-vibra.o
>  obj-$(CONFIG_INPUT_HISI_POWERKEY)	+= hisi_powerkey.o
>  obj-$(CONFIG_HP_SDC_RTC)		+= hp_sdc_rtc.o
>  obj-$(CONFIG_INPUT_IMS_PCU)		+= ims-pcu.o
> diff --git a/drivers/input/misc/gpio-vibra.c b/drivers/input/misc/gpio-vibra.c
> new file mode 100644
> index 000000000000..14f9534668c8
> --- /dev/null
> +++ b/drivers/input/misc/gpio-vibra.c
> @@ -0,0 +1,214 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + *  GPIO vibrator driver
> + *
> + *  Copyright (C) 2019 Luca Weiss <luca@z3ntu.xyz>
> + *
> + *  Based on PWM vibrator driver:
> + *  Copyright (C) 2017 Collabora Ltd.
> + *
> + *  Based on previous work from:
> + *  Copyright (C) 2012 Dmitry Torokhov <dmitry.torokhov@gmail.com>
> + *
> + *  Based on PWM beeper driver:
> + *  Copyright (C) 2010, Lars-Peter Clausen <lars@metafoo.de>
> + */
> +
> +#include <linux/gpio/consumer.h>
> +#include <linux/input.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/slab.h>
> +
> +enum vibrator_state {
> +	VIBRATOR_OFF,
> +	VIBRATOR_ON
> +};

I'd probably go with simple "bool running".

> +
> +struct gpio_vibrator {
> +	struct input_dev *input;
> +	struct gpio_desc *gpio;
> +	struct regulator *vcc;
> +
> +	struct work_struct play_work;
> +	enum vibrator_state state;
> +	bool vcc_on;
> +};
> +
> +static int gpio_vibrator_start(struct gpio_vibrator *vibrator)
> +{
> +	struct device *pdev = vibrator->input->dev.parent;
> +	int err;
> +
> +	if (!vibrator->vcc_on) {
> +		err = regulator_enable(vibrator->vcc);
> +		if (err) {
> +			dev_err(pdev, "failed to enable regulator: %d", err);
> +			return err;
> +		}
> +		vibrator->vcc_on = true;
> +	}
> +
> +	gpiod_set_value(vibrator->gpio, 1);

Since this is called by work item and thus from context that can sleep,
we can use gpiod_set_value_cansleep() so that "slow" GPIOs can be used
with the driver.

> +
> +	return 0;
> +}
> +
> +static void gpio_vibrator_stop(struct gpio_vibrator *vibrator)
> +{
> +	gpiod_set_value(vibrator->gpio, 0);
> +
> +	if (vibrator->vcc_on) {
> +		regulator_disable(vibrator->vcc);
> +		vibrator->vcc_on = false;
> +	}
> +}
> +
> +static void gpio_vibrator_play_work(struct work_struct *work)
> +{
> +	struct gpio_vibrator *vibrator = container_of(work,
> +					struct gpio_vibrator, play_work);
> +
> +	if (vibrator->state == VIBRATOR_ON)
> +		gpio_vibrator_start(vibrator);
> +	else
> +		gpio_vibrator_stop(vibrator);
> +}
> +
> +static int gpio_vibrator_play_effect(struct input_dev *dev, void *data,
> +				    struct ff_effect *effect)
> +{
> +	struct gpio_vibrator *vibrator = input_get_drvdata(dev);
> +
> +	int level = effect->u.rumble.strong_magnitude;
> +
> +	if (!level)
> +		level = effect->u.rumble.weak_magnitude;
> +
> +	if (level)
> +		vibrator->state = VIBRATOR_ON;
> +	else
> +		vibrator->state = VIBRATOR_OFF;
> +
> +	schedule_work(&vibrator->play_work);
> +
> +	return 0;
> +}
> +
> +static void gpio_vibrator_close(struct input_dev *input)
> +{
> +	struct gpio_vibrator *vibrator = input_get_drvdata(input);
> +
> +	cancel_work_sync(&vibrator->play_work);
> +	gpio_vibrator_stop(vibrator);

Do you want to update vibrator->state here?

> +}
> +
> +static int gpio_vibrator_probe(struct platform_device *pdev)
> +{
> +	struct gpio_vibrator *vibrator;
> +	int err;
> +
> +	vibrator = devm_kzalloc(&pdev->dev, sizeof(*vibrator), GFP_KERNEL);
> +	if (!vibrator)
> +		return -ENOMEM;
> +
> +	vibrator->input = devm_input_allocate_device(&pdev->dev);
> +	if (!vibrator->input)
> +		return -ENOMEM;
> +
> +	vibrator->vcc = devm_regulator_get(&pdev->dev, "vcc");
> +	err = PTR_ERR_OR_ZERO(vibrator->vcc);
> +	if (err) {
> +		if (err != -EPROBE_DEFER)
> +			dev_err(&pdev->dev, "Failed to request regulator: %d",
> +				err);
> +		return err;
> +	}
> +
> +	vibrator->gpio = devm_gpiod_get(&pdev->dev, "enable", GPIOD_OUT_LOW);
> +	err = PTR_ERR_OR_ZERO(vibrator->gpio);
> +	if (err) {
> +		if (err != -EPROBE_DEFER)
> +			dev_err(&pdev->dev, "Failed to request main gpio: %d",
> +				err);
> +		return err;
> +	}
> +
> +	INIT_WORK(&vibrator->play_work, gpio_vibrator_play_work);
> +
> +	vibrator->input->name = "gpio-vibrator";
> +	vibrator->input->id.bustype = BUS_HOST;
> +	vibrator->input->dev.parent = &pdev->dev;

No need to set as you used devm_input_device_alloc().

> +	vibrator->input->close = gpio_vibrator_close;
> +
> +	input_set_drvdata(vibrator->input, vibrator);
> +	input_set_capability(vibrator->input, EV_FF, FF_RUMBLE);
> +
> +	err = input_ff_create_memless(vibrator->input, NULL,
> +				      gpio_vibrator_play_effect);
> +	if (err) {
> +		dev_err(&pdev->dev, "Couldn't create FF dev: %d", err);
> +		return err;
> +	}
> +
> +	err = input_register_device(vibrator->input);
> +	if (err) {
> +		dev_err(&pdev->dev, "Couldn't register input dev: %d", err);
> +		return err;
> +	}
> +
> +	platform_set_drvdata(pdev, vibrator);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused gpio_vibrator_suspend(struct device *dev)
> +{
> +	struct gpio_vibrator *vibrator = dev_get_drvdata(dev);
> +
> +	cancel_work_sync(&vibrator->play_work);
> +	if (vibrator->state == VIBRATOR_ON)
> +		gpio_vibrator_stop(vibrator);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused gpio_vibrator_resume(struct device *dev)
> +{
> +	struct gpio_vibrator *vibrator = dev_get_drvdata(dev);
> +
> +	if (vibrator->state == VIBRATOR_ON)
> +		gpio_vibrator_start(vibrator);
> +
> +	return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(gpio_vibrator_pm_ops,
> +			 gpio_vibrator_suspend, gpio_vibrator_resume);
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id gpio_vibra_dt_match_table[] = {
> +	{ .compatible = "gpio-vibrator" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, gpio_vibra_dt_match_table);
> +#endif
> +
> +static struct platform_driver gpio_vibrator_driver = {
> +	.probe	= gpio_vibrator_probe,
> +	.driver	= {
> +		.name	= "gpio-vibrator",
> +		.pm	= &gpio_vibrator_pm_ops,
> +		.of_match_table = of_match_ptr(gpio_vibra_dt_match_table),
> +	},
> +};
> +module_platform_driver(gpio_vibrator_driver);
> +
> +MODULE_AUTHOR("Luca Weiss <luca@z3ntu.xy>");
> +MODULE_DESCRIPTION("GPIO vibrator driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:gpio-vibrator");
> -- 
> 2.21.0
> 

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] Input: i8042 - disable KBD port on Late-2016 Razer Blade Stealth
From: Dmitry Torokhov @ 2019-04-07 22:10 UTC (permalink / raw)
  To: Lyude Paul; +Cc: stable, Chen-Yu Tsai, linux-input, linux-kernel
In-Reply-To: <20190407213735.10658-1-lyude@redhat.com>

Hi Lyude,

On Sun, Apr 07, 2019 at 05:37:34PM -0400, Lyude Paul wrote:
> The late 2016 model of the Razer Blade Stealth has a built-in USB
> keyboard, but for some reason the BIOS exposes an i8042 controller with
> a connected KBD port. While this fake AT Keyboard device doesn't appear
> to report any events, attempting to change the state of the caps lock
> LED on it from on to off causes the entire system to hang.
> 
> So, introduce a quirk table for disabling keyboard probing by default,
> i8042_dmi_nokbd_table, and add this specific model of Razer laptop to
> that table.

What does dmesg show about i8042 for this device? Especially line "PNP:
PS/2 Controller  ..."?

Thanks.

-- 
Dmitry

^ permalink raw reply

* [PATCH] Input: i8042 - disable KBD port on Late-2016 Razer Blade Stealth
From: Lyude Paul @ 2019-04-07 21:37 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: stable, Chen-Yu Tsai, linux-input, linux-kernel

The late 2016 model of the Razer Blade Stealth has a built-in USB
keyboard, but for some reason the BIOS exposes an i8042 controller with
a connected KBD port. While this fake AT Keyboard device doesn't appear
to report any events, attempting to change the state of the caps lock
LED on it from on to off causes the entire system to hang.

So, introduce a quirk table for disabling keyboard probing by default,
i8042_dmi_nokbd_table, and add this specific model of Razer laptop to
that table.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Cc: stable@vger.kernel.org
---
 drivers/input/serio/i8042-x86ia64io.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h
index 136f6e7bf797..888f5f6feebf 100644
--- a/drivers/input/serio/i8042-x86ia64io.h
+++ b/drivers/input/serio/i8042-x86ia64io.h
@@ -884,6 +884,22 @@ static const struct dmi_system_id __initconst i8042_dmi_kbdreset_table[] = {
 	{ }
 };
 
+static const struct dmi_system_id i8042_dmi_nokbd_table[] __initconst = {
+	{
+		/*
+		 * Razer Blade Stealth (Late 2016 model) - Keyboard is on USB
+		 * but the system exposes a fake AT keyboard that crashes the
+		 * system if the caps lock LED is changed
+		 */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Razer"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Blade Stealth"),
+			DMI_MATCH(DMI_PRODUCT_VERSION, "2.04"),
+		},
+	},
+	{ }
+};
+
 #endif /* CONFIG_X86 */
 
 #ifdef CONFIG_PNP
@@ -1040,6 +1056,9 @@ static int __init i8042_pnp_init(void)
 #ifdef CONFIG_X86
 	if (dmi_check_system(i8042_dmi_nopnp_table))
 		i8042_nopnp = true;
+
+	if (dmi_check_system(i8042_dmi_nokbd_table))
+		i8042_nokbd = true;
 #endif
 
 	if (i8042_nopnp) {
-- 
2.20.1

^ permalink raw reply related

* [PATCH v2 3/3] ARM: dts: msm8974-FP2: Add vibration motor
From: Luca Weiss @ 2019-04-07 15:58 UTC (permalink / raw)
  Cc: Dmitry Torokhov, Rob Herring, Mark Rutland, Mauro Carvalho Chehab,
	Pascal PAILLET-LME, Coly Li, Lee Jones, Xiaotong Lu, Brian Masney,
	Rob Herring, Baolin Wang, Andy Gross, David Brown,
	open list:ARM/QUALCOMM SUPPORT,
	open list:INPUT KEYBOARD, MOUSE, JOYSTICK , TOUCHSCREEN...,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, ope
In-Reply-To: <20190407155841.27354-1-luca@z3ntu.xyz>

Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
 arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts b/arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts
index 643c57f84818..bf402ae39226 100644
--- a/arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts
+++ b/arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts
@@ -50,6 +50,12 @@
 		};
 	};
 
+	vibrator {
+		compatible = "gpio-vibrator";
+		enable-gpios = <&msmgpio 86 GPIO_ACTIVE_HIGH>;
+		vcc-supply = <&pm8941_l18>;
+	};
+
 	smd {
 		rpm {
 			rpm_requests {
-- 
2.21.0

^ permalink raw reply related

* [PATCH v2 2/3] Input: add a driver for GPIO controllable vibrators
From: Luca Weiss @ 2019-04-07 15:58 UTC (permalink / raw)
  Cc: Dmitry Torokhov, Rob Herring, Mark Rutland, Mauro Carvalho Chehab,
	Pascal PAILLET-LME, Coly Li, Lee Jones, Xiaotong Lu, Brian Masney,
	Rob Herring, Baolin Wang, Andy Gross, David Brown,
	open list:ARM/QUALCOMM SUPPORT,
	open list:INPUT KEYBOARD, MOUSE, JOYSTICK , TOUCHSCREEN...,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, ope
In-Reply-To: <20190407155841.27354-1-luca@z3ntu.xyz>

Provide a simple driver for GPIO controllable vibrators.
It will be used by the Fairphone 2.

Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
 drivers/input/misc/Kconfig      |  12 ++
 drivers/input/misc/Makefile     |   1 +
 drivers/input/misc/gpio-vibra.c | 214 ++++++++++++++++++++++++++++++++
 3 files changed, 227 insertions(+)
 create mode 100644 drivers/input/misc/gpio-vibra.c

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index e15ed1bb8558..6dfe9e2fe5b1 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -290,6 +290,18 @@ config INPUT_GPIO_DECODER
 	 To compile this driver as a module, choose M here: the module
 	 will be called gpio_decoder.
 
+config INPUT_GPIO_VIBRA
+	tristate "GPIO vibrator support"
+	depends on GPIOLIB || COMPILE_TEST
+	select INPUT_FF_MEMLESS
+	help
+	  Say Y here to get support for GPIO based vibrator devices.
+
+	  If unsure, say N.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called gpio-vibra.
+
 config INPUT_IXP4XX_BEEPER
 	tristate "IXP4XX Beeper support"
 	depends on ARCH_IXP4XX
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index b936c5b1d4ac..f38ebbdb05e2 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_INPUT_DRV2667_HAPTICS)	+= drv2667.o
 obj-$(CONFIG_INPUT_GP2A)		+= gp2ap002a00f.o
 obj-$(CONFIG_INPUT_GPIO_BEEPER)		+= gpio-beeper.o
 obj-$(CONFIG_INPUT_GPIO_DECODER)	+= gpio_decoder.o
+obj-$(CONFIG_INPUT_GPIO_VIBRA)		+= gpio-vibra.o
 obj-$(CONFIG_INPUT_HISI_POWERKEY)	+= hisi_powerkey.o
 obj-$(CONFIG_HP_SDC_RTC)		+= hp_sdc_rtc.o
 obj-$(CONFIG_INPUT_IMS_PCU)		+= ims-pcu.o
diff --git a/drivers/input/misc/gpio-vibra.c b/drivers/input/misc/gpio-vibra.c
new file mode 100644
index 000000000000..14f9534668c8
--- /dev/null
+++ b/drivers/input/misc/gpio-vibra.c
@@ -0,0 +1,214 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  GPIO vibrator driver
+ *
+ *  Copyright (C) 2019 Luca Weiss <luca@z3ntu.xyz>
+ *
+ *  Based on PWM vibrator driver:
+ *  Copyright (C) 2017 Collabora Ltd.
+ *
+ *  Based on previous work from:
+ *  Copyright (C) 2012 Dmitry Torokhov <dmitry.torokhov@gmail.com>
+ *
+ *  Based on PWM beeper driver:
+ *  Copyright (C) 2010, Lars-Peter Clausen <lars@metafoo.de>
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/input.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+
+enum vibrator_state {
+	VIBRATOR_OFF,
+	VIBRATOR_ON
+};
+
+struct gpio_vibrator {
+	struct input_dev *input;
+	struct gpio_desc *gpio;
+	struct regulator *vcc;
+
+	struct work_struct play_work;
+	enum vibrator_state state;
+	bool vcc_on;
+};
+
+static int gpio_vibrator_start(struct gpio_vibrator *vibrator)
+{
+	struct device *pdev = vibrator->input->dev.parent;
+	int err;
+
+	if (!vibrator->vcc_on) {
+		err = regulator_enable(vibrator->vcc);
+		if (err) {
+			dev_err(pdev, "failed to enable regulator: %d", err);
+			return err;
+		}
+		vibrator->vcc_on = true;
+	}
+
+	gpiod_set_value(vibrator->gpio, 1);
+
+	return 0;
+}
+
+static void gpio_vibrator_stop(struct gpio_vibrator *vibrator)
+{
+	gpiod_set_value(vibrator->gpio, 0);
+
+	if (vibrator->vcc_on) {
+		regulator_disable(vibrator->vcc);
+		vibrator->vcc_on = false;
+	}
+}
+
+static void gpio_vibrator_play_work(struct work_struct *work)
+{
+	struct gpio_vibrator *vibrator = container_of(work,
+					struct gpio_vibrator, play_work);
+
+	if (vibrator->state == VIBRATOR_ON)
+		gpio_vibrator_start(vibrator);
+	else
+		gpio_vibrator_stop(vibrator);
+}
+
+static int gpio_vibrator_play_effect(struct input_dev *dev, void *data,
+				    struct ff_effect *effect)
+{
+	struct gpio_vibrator *vibrator = input_get_drvdata(dev);
+
+	int level = effect->u.rumble.strong_magnitude;
+
+	if (!level)
+		level = effect->u.rumble.weak_magnitude;
+
+	if (level)
+		vibrator->state = VIBRATOR_ON;
+	else
+		vibrator->state = VIBRATOR_OFF;
+
+	schedule_work(&vibrator->play_work);
+
+	return 0;
+}
+
+static void gpio_vibrator_close(struct input_dev *input)
+{
+	struct gpio_vibrator *vibrator = input_get_drvdata(input);
+
+	cancel_work_sync(&vibrator->play_work);
+	gpio_vibrator_stop(vibrator);
+}
+
+static int gpio_vibrator_probe(struct platform_device *pdev)
+{
+	struct gpio_vibrator *vibrator;
+	int err;
+
+	vibrator = devm_kzalloc(&pdev->dev, sizeof(*vibrator), GFP_KERNEL);
+	if (!vibrator)
+		return -ENOMEM;
+
+	vibrator->input = devm_input_allocate_device(&pdev->dev);
+	if (!vibrator->input)
+		return -ENOMEM;
+
+	vibrator->vcc = devm_regulator_get(&pdev->dev, "vcc");
+	err = PTR_ERR_OR_ZERO(vibrator->vcc);
+	if (err) {
+		if (err != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "Failed to request regulator: %d",
+				err);
+		return err;
+	}
+
+	vibrator->gpio = devm_gpiod_get(&pdev->dev, "enable", GPIOD_OUT_LOW);
+	err = PTR_ERR_OR_ZERO(vibrator->gpio);
+	if (err) {
+		if (err != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "Failed to request main gpio: %d",
+				err);
+		return err;
+	}
+
+	INIT_WORK(&vibrator->play_work, gpio_vibrator_play_work);
+
+	vibrator->input->name = "gpio-vibrator";
+	vibrator->input->id.bustype = BUS_HOST;
+	vibrator->input->dev.parent = &pdev->dev;
+	vibrator->input->close = gpio_vibrator_close;
+
+	input_set_drvdata(vibrator->input, vibrator);
+	input_set_capability(vibrator->input, EV_FF, FF_RUMBLE);
+
+	err = input_ff_create_memless(vibrator->input, NULL,
+				      gpio_vibrator_play_effect);
+	if (err) {
+		dev_err(&pdev->dev, "Couldn't create FF dev: %d", err);
+		return err;
+	}
+
+	err = input_register_device(vibrator->input);
+	if (err) {
+		dev_err(&pdev->dev, "Couldn't register input dev: %d", err);
+		return err;
+	}
+
+	platform_set_drvdata(pdev, vibrator);
+
+	return 0;
+}
+
+static int __maybe_unused gpio_vibrator_suspend(struct device *dev)
+{
+	struct gpio_vibrator *vibrator = dev_get_drvdata(dev);
+
+	cancel_work_sync(&vibrator->play_work);
+	if (vibrator->state == VIBRATOR_ON)
+		gpio_vibrator_stop(vibrator);
+
+	return 0;
+}
+
+static int __maybe_unused gpio_vibrator_resume(struct device *dev)
+{
+	struct gpio_vibrator *vibrator = dev_get_drvdata(dev);
+
+	if (vibrator->state == VIBRATOR_ON)
+		gpio_vibrator_start(vibrator);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(gpio_vibrator_pm_ops,
+			 gpio_vibrator_suspend, gpio_vibrator_resume);
+
+#ifdef CONFIG_OF
+static const struct of_device_id gpio_vibra_dt_match_table[] = {
+	{ .compatible = "gpio-vibrator" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, gpio_vibra_dt_match_table);
+#endif
+
+static struct platform_driver gpio_vibrator_driver = {
+	.probe	= gpio_vibrator_probe,
+	.driver	= {
+		.name	= "gpio-vibrator",
+		.pm	= &gpio_vibrator_pm_ops,
+		.of_match_table = of_match_ptr(gpio_vibra_dt_match_table),
+	},
+};
+module_platform_driver(gpio_vibrator_driver);
+
+MODULE_AUTHOR("Luca Weiss <luca@z3ntu.xy>");
+MODULE_DESCRIPTION("GPIO vibrator driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:gpio-vibrator");
-- 
2.21.0

^ permalink raw reply related

* [PATCH v2 1/3] dt-bindings: input: add GPIO controllable vibrator
From: Luca Weiss @ 2019-04-07 15:58 UTC (permalink / raw)
  Cc: Dmitry Torokhov, Rob Herring, Mark Rutland, Mauro Carvalho Chehab,
	Pascal PAILLET-LME, Coly Li, Lee Jones, Xiaotong Lu, Brian Masney,
	Rob Herring, Baolin Wang, Andy Gross, David Brown,
	open list:ARM/QUALCOMM SUPPORT,
	open list:INPUT KEYBOARD, MOUSE, JOYSTICK , TOUCHSCREEN...,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, ope

Provide a simple driver for GPIO controllable vibrators.
It will be used by the Fairphone 2.

Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
Changes from v1:
- Mark vcc-supply as optional

 .../bindings/input/gpio-vibrator.txt          | 20 +++++++++++++++++++
 1 file changed, 20 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..93e5a8e7622d
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/gpio-vibrator.txt
@@ -0,0 +1,20 @@
+* GPIO vibrator device tree bindings
+
+Registers a GPIO device as vibrator, where the vibration motor just has the
+capability to turn on or off. If the device is connected to a pwm, you should
+use the pwm-vibrator driver instead.
+
+Required properties:
+- compatible: should contain "gpio-vibrator"
+- enable-gpios: Should contain a GPIO handle
+
+Optional properties:
+- vcc-supply: Phandle for the regulator supplying power
+
+Example from Fairphone 2:
+
+vibrator {
+	compatible = "gpio-vibrator";
+	enable-gpios = <&msmgpio 86 GPIO_ACTIVE_HIGH>;
+	vcc-supply = <&pm8941_l18>;
+};
-- 
2.21.0

^ permalink raw reply related

* Re: [RFC v2] iio: input-bridge: optionally bridge iio acceleometers to create a /dev/input interface
From: Jonathan Cameron @ 2019-04-07 12:30 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: Dmitry Torokhov, Eric Piel, linux-input, letux-kernel, kernel,
	Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	linux-kernel, linux-iio
In-Reply-To: <195994ebff28de22eae872df134d086c761b83b8.1554026986.git.hns@goldelico.com>

On Sun, 31 Mar 2019 12:09:46 +0200
"H. Nikolaus Schaller" <hns@goldelico.com> wrote:

Hi Nikolaus,

I'm probably going to repeat a few things I sent for v1 as the audience has
expanded somewhat!

Good to see this moving forwards though as there has been at least some demand
for it going way back to the early days of IIO.

> Some user spaces (e.g. some Android devices) use /dev/input/event* for handling
> the 3D position of the device with respect to the center of gravity (earth).
> This can be used for gaming input, auto-rotation of screens etc.
> 
> This interface should be the standard for such use cases because it is an abstraction
> of how orientation data is acquired from sensor chips. Sensor chips may be connected
> through different interfaces and in different positions. They may also have different
> parameters. And, if a chip is replaced by a different one, the values reported by
> the device position interface should remain the same, provided the device tree reflects
> the changed chip.
> 
> This did initially lead to input accelerometer drivers like drivers/input/misc/bma150.c
> or drivers/misc/lis3lv02d/
> 
> But nowadays, new accelerometer chips mostly get iio drivers and rarely input drivers.
> 
> Therefore we need something like a protocol stack which bridges raw data and input devices.
> It can be seen as a similar layering like TCP/IP vs. bare Ethernet. Or keyboard
> input events vs. raw gpio or raw USB access.
> 
> This patch bridges the gap between raw iio data and the input device abstraction
> so that accelerometer measurements can additionally be presented as X/Y/Z accelerometer
> channels (INPUT_PROP_ACCELEROMETER) through /dev/input/event*.
> 
> There are no special requirements or changes needed for an iio driver.
> 
> There is no need to define a mapping (e.g. in device tree).
This worries me, as it inherently means we end up with this interface being
registered in cases where it makes no sense.  A lot of generic distros get
used across widely differing use cases.

I think we need some deliberate userspace interaction to instantiate
one of these rather than 'always doing it'.

As I mentioned in V1, look at the possibility of a configfs based method
to build the map.  It's easy for userspace to work out what makes sense to
map in principle.  There may be some missing info that we also need to
look to expose.

In general, userspace created channel maps would be very useful for
other things such as maker type boards where they can plug all sorts
of odd things into ADC channels for example.

> 
> This driver simply collects the first 3 accelerometer channels as X, Y and Z.
> If only 1 or 2 channels are available, they are used for X and Y only. Additional
> channels are ignored.
> 
> Scaling is done automatically so that 1g is represented by value 256 and
> range is assumed to be -511 .. +511 which gives a reasonable precision as an
> input device.

Why do we do this, rather than letting input deal with it?  Input is used
to widely differing scales IIRC

> 
> If a mount-matrix is provided by the iio driver, it is also taken into account
> so that the input event automatically gets the correct orientation with respect
> to the device.
> 
> If this extension is not configured into the kernel it takes no resources (except
> source code).
> 
> If it is configured, but there is no accelerometer, there is only a tiny penalty
> for scanning for accelerometer channels once during probe of each iio device.
> 
> If it runs, the driver polls the device(s) once every 100 ms. A mode where the
> iio device defines the update rate is not implemented and for further study.
> 
> If there is no user-space client, polling is not running.
> 
> The driver is capable to handle multiple iio accelerometers and they are
> presented by unique /dev/input/event* files. The iio chip name is used to define
> the input device name so that it can be identified (e.g. by udev rules or evtest).
> 
> Here is some example what you can expect from the driver (device:
> arch/arm/boot/dts/omap3-gta04a5.dts):
> 
> root@letux:~# dmesg|fgrep iio
> [    6.324584] input: iio-bridge: bmc150_accel as /devices/platform/68000000.ocp/48072000.i2c/i2c-1/1-0010/iio:device1/input/input5
> [    6.516632] input: iio-bridge: bno055 as /devices/platform/68000000.ocp/48072000.i2c/i2c-1/1-0029/iio:device3/input/input7
> root@letux:~# evtest /dev/input/event5 | head -19
> Input driver version is 1.0.1
> Input device ID: bus 0x0 vendor 0x0 product 0x0 version 0x0
> Input device name: "iio-bridge: bmc150_accel"
> Supported events:
>   Event type 0 (EV_SYN)
>   Event type 3 (EV_ABS)
>     Event code 0 (ABS_X)
>       Value      8
>       Min     -511
>       Max      511
>     Event code 1 (ABS_Y)
>       Value    -44
>       Min     -511
>       Max      511
>     Event code 2 (ABS_Z)
>       Value   -265
>       Min     -511
>       Max      511
> Properties:
> root@letux:~# evtest /dev/input/event7 | head -19
> Input driver version is 1.0.1
> Input device ID: bus 0x0 vendor 0x0 product 0x0 version 0x0
> Input device name: "iio-bridge: bno055"
> Supported events:
>   Event type 0 (EV_SYN)
>   Event type 3 (EV_ABS)
>     Event code 0 (ABS_X)
>       Value     -6
>       Min     -511
>       Max      511
>     Event code 1 (ABS_Y)
>       Value     17
>       Min     -511
>       Max      511
>     Event code 2 (ABS_Z)
>       Value   -250
>       Min     -511
>       Max      511
> Properties:
> root@letux:~# 
> 
> Although the sensor chips are mounted with different axis orientation,
> the application of the mount matrix provides equivalent (despite noise
> and precision) information on device orientation.
> 
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
>  drivers/iio/Kconfig                    |   7 +
>  drivers/iio/Makefile                   |   1 +
>  drivers/iio/industrialio-core.c        |  12 +
>  drivers/iio/industrialio-inputbridge.c | 295 +++++++++++++++++++++++++
>  drivers/iio/industrialio-inputbridge.h |  28 +++
>  5 files changed, 343 insertions(+)
>  create mode 100644 drivers/iio/industrialio-inputbridge.c
>  create mode 100644 drivers/iio/industrialio-inputbridge.h
> 
> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
> index d08aeb41cd07..d85afe002613 100644
> --- a/drivers/iio/Kconfig
> +++ b/drivers/iio/Kconfig
> @@ -68,6 +68,13 @@ config IIO_TRIGGERED_EVENT
>  	help
>  	  Provides helper functions for setting up triggered events.
>  
> +config IIO_INPUT_BRIDGE
> +	bool "Enable accelerometer bridge to input driver"

Dependency on input?

> +	help
> +	  Provides a /dev/input/event* device for accelerometers
> +	  to use as a 3D input device, e.g. for gaming or auto-rotation
> +	  of screen contents.
> +
>  source "drivers/iio/accel/Kconfig"
>  source "drivers/iio/adc/Kconfig"
>  source "drivers/iio/afe/Kconfig"
> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
> index cb5993251381..d695e5a27da5 100644
> --- a/drivers/iio/Makefile
> +++ b/drivers/iio/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_IIO) += industrialio.o
>  industrialio-y := industrialio-core.o industrialio-event.o inkern.o
>  industrialio-$(CONFIG_IIO_BUFFER) += industrialio-buffer.o
>  industrialio-$(CONFIG_IIO_TRIGGER) += industrialio-trigger.o
> +industrialio-$(CONFIG_IIO_INPUT_BRIDGE) += industrialio-inputbridge.o
>  
>  obj-$(CONFIG_IIO_CONFIGFS) += industrialio-configfs.o
>  obj-$(CONFIG_IIO_SW_DEVICE) += industrialio-sw-device.o
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 4700fd5d8c90..81f412b41a78 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -29,6 +29,7 @@
>  #include <linux/iio/iio.h>
>  #include "iio_core.h"
>  #include "iio_core_trigger.h"
> +#include "industrialio-inputbridge.h"
>  #include <linux/iio/sysfs.h>
>  #include <linux/iio/events.h>
>  #include <linux/iio/buffer.h>
> @@ -1723,6 +1724,15 @@ int __iio_device_register(struct iio_dev *indio_dev, struct module *this_mod)
>  	if (ret < 0)
>  		goto error_unreg_eventset;
>  
> +	ret = iio_device_register_inputbridge(indio_dev);
> +	if (ret) {
> +		dev_err(indio_dev->dev.parent,
> +			"Failed to register as input driver\n");
> +		device_del(&indio_dev->dev);
> +
This doesn't look like balanced error handling given the goto in the previous
case.  If we are treating this as an error we need to unwind the whole
of this function properly.

> +		return ret;
> +	}
> +
>  	return 0;
>  
>  error_unreg_eventset:
> @@ -1745,6 +1755,8 @@ void iio_device_unregister(struct iio_dev *indio_dev)
>  {
>  	mutex_lock(&indio_dev->info_exist_lock);
>  
> +	iio_device_unregister_inputbridge(indio_dev);
> +
>  	cdev_device_del(&indio_dev->chrdev, &indio_dev->dev);
>  
>  	iio_device_unregister_debugfs(indio_dev);
> diff --git a/drivers/iio/industrialio-inputbridge.c b/drivers/iio/industrialio-inputbridge.c
> new file mode 100644
> index 000000000000..dd672e25bc70
> --- /dev/null
> +++ b/drivers/iio/industrialio-inputbridge.c
> @@ -0,0 +1,295 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * The Industrial I/O core, bridge to input devices
> + *
> + * Copyright (c) 2016-2019 Golden Delicious Computers GmbH&Co. KG
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
No need for the boiler plate if you have SPDX.  That is one of the
advantages!

> + */
> +
> +#include <linux/iio/consumer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/types.h>
> +#include <linux/input.h>
> +#include <linux/input-polldev.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +
> +#include "industrialio-inputbridge.h"
> +
> +/* currently, only polling is implemented */
> +#define POLLING_MSEC	100
> +
> +struct iio_input_map {
> +	struct input_polled_dev *poll_dev;	/* the input device */
> +	struct iio_channel channels[3];		/* x, y, z channels */
> +	struct matrix {
> +		int mxx, myx, mzx;	/* fixed point mount-matrix */
> +		int mxy, myy, mzy;
> +		int mxz, myz, mzz;
> +	} matrix;
> +};
> +
> +static inline struct iio_input_map *to_iio_input_map(
> +		struct iio_channel *channel)
> +{
> +	return (struct iio_input_map *) channel->data;
> +}
> +
> +/* minimum and maximum range we want to report */
> +#define ABSMAX_ACC_VAL		(512 - 1)
> +#define ABSMIN_ACC_VAL		-(ABSMAX_ACC_VAL)
> +
> +/* scale processed iio values so that 1g maps to ABSMAX_ACC_VAL / 2 */
> +#define SCALE			((100 * ABSMAX_ACC_VAL) / (2 * 981))
> +
> +/*
> + * convert float string to scaled fixed point format, e.g.
> + *   1		-> 1000		(value passed as unit)
> + *   1.23	-> 1230
> + *   0.1234	->  123
> + *   -.01234	->  -12
> + */
> +
> +static int32_t atofix(const char *str, uint32_t unit)
> +{
> +	int32_t mantissa = 0;
> +	bool sign = false;
> +	bool decimal = false;
> +	int32_t divisor = 1;
> +
> +	if (*str == '-')
> +		sign = true, str++;
> +	while (*str && divisor < unit) {
> +		if (*str >= '0' && *str <= '9') {
> +			mantissa = 10 * mantissa + (*str - '0');
> +			if (decimal)
> +				divisor *= 10;
> +		} else if (*str == '.')
> +			decimal = true;
> +		else
> +			return 0;	/* error */
> +		str++;
> +	}
> +
> +	mantissa = (mantissa * unit) / divisor;
> +	if (sign)
> +		mantissa = -mantissa;
> +
> +	return mantissa;
> +}
> +
> +static void iio_apply_matrix(struct matrix *m, int *in, int *out, uint32_t unit)
> +{
> +	/* apply mount matrix */
> +	out[0] = (m->mxx * in[0] + m->myx * in[1] + m->mzx * in[2]) / unit;
> +	out[1] = (m->mxy * in[0] + m->myy * in[1] + m->mzy * in[2]) / unit;
> +	out[2] = (m->mxz * in[0] + m->myz * in[1] + m->mzz * in[2]) / unit;
> +}
> +
> +#define FIXED_POINT_UNIT	1000	/* seems reasonable for accelerometer input */
> +
> +static void iio_accel_poll(struct input_polled_dev *dev)
> +{
> +	struct iio_input_map *map = dev->private;
> +	struct input_dev *input = dev->input;
> +
> +	int values[3];		/* values while processing */
> +	int aligned_values[3];	/* mount matrix applied */
> +
> +	int cindex = 0;
> +
> +printk("%s: map=%px input=%px\n", __func__, map, input);
Remember to tidy these debug statements up at some point.

> +
> +	while (cindex < ARRAY_SIZE(values)) {
> +		struct iio_channel *channel =
> +			&map->channels[cindex];
> +		int val;
> +		int ret;
> +
> +		if (!channel->indio_dev) {
> +			values[cindex] = 0;
> +			continue;
> +		}
> +
> +		ret = iio_read_channel_raw(channel, &val);
> +
> +		if (ret < 0) {
> +			pr_err("%s(): channel read error %d\n",
> +				__func__, cindex);
> +			return;
> +		}
> +
> +		ret = iio_convert_raw_to_processed(channel, val,
> +				 &values[cindex], SCALE);
> +
> +		if (ret < 0) {
> +			pr_err("%s(): channel processing error\n",
> +				__func__);
> +			return;
> +		}
> +
> +		cindex++;
> +	}
> +
> +	iio_apply_matrix(&map->matrix, values, aligned_values, FIXED_POINT_UNIT);
> +
> +	input_report_abs(input, ABS_X, aligned_values[0]);
> +	input_report_abs(input, ABS_Y, aligned_values[1]);
> +	input_report_abs(input, ABS_Z, aligned_values[2]);
> +	input_sync(input);
> +}
> +
> +static int dindex=0;	/* assign unique names to accel/input devices */
Build something from the iio device IDA perhaps?  Those are unique
as well.  Useful to know which one this is linked to.

> +
> +static int iio_input_register_accel_channel(struct iio_dev *indio_dev,
> +		 const struct iio_chan_spec *chan)
> +{ /* we found some accelerometer channel */
> +	int ret;
> +	int cindex;
> +	struct iio_input_map *map = iio_device_get_drvdata(indio_dev);

Don't do that.   That is in the domain of the device driver and so
will sometimes already be in use. 

> +
> +printk("%s: map=%px\n", __func__, map);
> +
> +	if (!map) {
> +		struct input_polled_dev *poll_dev;
> +		const struct iio_chan_spec_ext_info *ext_info;
> +
> +		map = devm_kzalloc(&indio_dev->dev, sizeof(struct iio_input_map), GFP_KERNEL);
> +		if (!map)
> +			return -ENOMEM;
> +
> +		iio_device_set_drvdata(indio_dev, map);
> +
> +		poll_dev = devm_input_allocate_polled_device(&indio_dev->dev);
> +		if (!poll_dev)
> +			return -ENOMEM;
> +
> +		poll_dev->private = map;
> +		poll_dev->poll = iio_accel_poll;
> +		poll_dev->poll_interval = POLLING_MSEC;
> +
> +		poll_dev->input->name = kasprintf(GFP_KERNEL, "iio-bridge: %s",
> +						    indio_dev->name);
> +		poll_dev->input->phys = kasprintf(GFP_KERNEL, "accel/input%d",
> +						    dindex++);
> +
> +// do we need something like this?
> +//		poll_dev->input->id.bustype = BUS_IIO;
> +//		poll_dev->input->id.vendor = 0x0001;
> +//		poll_dev->input->id.product = 0x0001;
> +//		poll_dev->input->id.version = 0x0001;
> +
> +		set_bit(INPUT_PROP_ACCELEROMETER, poll_dev->input->propbit);
> +		poll_dev->input->evbit[0] = BIT_MASK(EV_ABS);
> +		input_alloc_absinfo(poll_dev->input);
> +		input_set_abs_params(poll_dev->input, ABS_X, ABSMIN_ACC_VAL,
> +					ABSMAX_ACC_VAL, 0, 0);
> +		input_set_abs_params(poll_dev->input, ABS_Y, ABSMIN_ACC_VAL,
> +					ABSMAX_ACC_VAL, 0, 0);
> +		input_set_abs_params(poll_dev->input, ABS_Z, ABSMIN_ACC_VAL,
> +					ABSMAX_ACC_VAL, 0, 0);
> +
> +		map->poll_dev = poll_dev;
> +
> +		ret = input_register_polled_device(poll_dev);
> +
> +		if (ret < 0) {
> +			kfree(poll_dev->input->name);
> +			kfree(poll_dev->input->phys);
> +			return ret;
> +		}
> +
> +		/* assume all channels of a device share the same matrix */
> +
> +		ext_info = chan->ext_info;
> +		for (; ext_info && ext_info->name; ext_info++) {
> +			if (strcmp(ext_info->name, "mount_matrix") == 0)
> +				break;
> +		}
> +
> +		if (ext_info && ext_info->name) {
> +			/* matrix found */
> +			uintptr_t priv = ext_info->private;
> +			const struct iio_mount_matrix *mtx;
> +
> +			mtx = ((iio_get_mount_matrix_t *) priv)(indio_dev,
> +								chan);
> +
> +			map->matrix.mxx = atofix(mtx->rotation[0], FIXED_POINT_UNIT);
> +			map->matrix.myx = atofix(mtx->rotation[1], FIXED_POINT_UNIT);
> +			map->matrix.mzx = atofix(mtx->rotation[2], FIXED_POINT_UNIT);
> +			map->matrix.mxy = atofix(mtx->rotation[3], FIXED_POINT_UNIT);
> +			map->matrix.myy = atofix(mtx->rotation[4], FIXED_POINT_UNIT);
> +			map->matrix.mzy = atofix(mtx->rotation[5], FIXED_POINT_UNIT);
> +			map->matrix.mxz = atofix(mtx->rotation[6], FIXED_POINT_UNIT);
> +			map->matrix.myz = atofix(mtx->rotation[7], FIXED_POINT_UNIT);
> +			map->matrix.mzz = atofix(mtx->rotation[8], FIXED_POINT_UNIT);
> +		} else {
> +			map->matrix.mxx = FIXED_POINT_UNIT;
> +			map->matrix.myx = 0;
> +			map->matrix.mzx = 0;
> +			map->matrix.mxy = 0;
> +			map->matrix.myy = FIXED_POINT_UNIT;
> +			map->matrix.mzy = 0;
> +			map->matrix.mxz = 0;
> +			map->matrix.myz = 0;
> +			map->matrix.mzz = FIXED_POINT_UNIT;
> +		}
> +	}
> +
> +// brauchen wir das noch? Oder nehmen wir einfach an dass es 3 Kanäle gibt?
> +
> +	/* find free channel within this device */
> +
> +	for (cindex = 0; cindex < ARRAY_SIZE(map->channels); cindex++) {
> +		if (!map->channels[cindex].indio_dev)
> +			break;
> +	}
> +
> +	/* check if we already have collected enough channels */
> +	if (cindex == ARRAY_SIZE(map->channels))
> +		return 0;	/* silently ignore */
> +
> +	map->channels[cindex].indio_dev = indio_dev;
> +	map->channels[cindex].channel = chan;
> +	map->channels[cindex].data = map;
> +
> +	return 0;
> +}
> +
> +int iio_device_register_inputbridge(struct iio_dev *indio_dev)
> +{
> +	int i;
> +
> +	for (i = 0; i < indio_dev->num_channels; i++) {
> +		const struct iio_chan_spec *chan =
> +				&indio_dev->channels[i];
> +
> +		if (chan->type == IIO_ACCEL) {
> +			int r = iio_input_register_accel_channel(indio_dev,
> +								 chan);
It would be cleaner (and safer) to go find all the necessary channels then
set up the map in one go, rather that iterating and trying to build it
in a sequential fashion.

So move the search loop inside and have something like.

iio_input_find_accel_channel(indio_dev, chan, &numchans);
iio_input_register_device(indio_dev, chan, numchans);
 
> +
> +			if (r < 0)
> +				return r;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +void iio_device_unregister_inputbridge(struct iio_dev *indio_dev)
> +{
> +	struct iio_input_map *map = iio_device_get_drvdata(indio_dev);
> +	struct input_dev *input = map->poll_dev->input;
> +
> +	kfree(input->name);
> +	kfree(input->phys);
> +	input_unregister_polled_device(map->poll_dev);
> +}
> +
> +MODULE_AUTHOR("H. Nikolaus Schaller <hns@goldelico.com>");
> +MODULE_DESCRIPTION("Bridge to present Industrial I/O accelerometers as properly oriented Input devices");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/industrialio-inputbridge.h b/drivers/iio/industrialio-inputbridge.h
> new file mode 100644
> index 000000000000..1363b10ab3f7
> --- /dev/null
> +++ b/drivers/iio/industrialio-inputbridge.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * The Industrial I/O core, bridge to input devices
> + *
> + * Copyright (c) 2016-2019 Golden Delicious Computers GmbH&Co. KG
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + */
> +
> +#if defined(CONFIG_IIO_INPUT_BRIDGE)
> +
> +extern int iio_device_register_inputbridge(struct iio_dev *indio_dev);
> +extern void iio_device_unregister_inputbridge(struct iio_dev *indio_dev);
> +
> +#else
> +
> +static inline int iio_device_register_inputbridge(struct iio_dev *indio_dev)
> +{
> +	return 0;
> +}
> +
> +static inline void iio_device_unregister_inputbridge(struct iio_dev *indio_dev)
> +{
> +}
> +
> +#endif

^ permalink raw reply

* Re: [PATCH 3/3] iio: Add PAT9125 optical tracker sensor
From: Jonathan Cameron @ 2019-04-07 10:20 UTC (permalink / raw)
  To: Alexandre Mergnat
  Cc: robh+dt, mark.rutland, knaack.h, lars, pmeerw, linux-kernel,
	linux-iio, baylibre-upstreaming, Dmitry Torokhov, linux-input
In-Reply-To: <1554456870-8104-4-git-send-email-amergnat@baylibre.com>

On Fri,  5 Apr 2019 09:34:30 +0000
Alexandre Mergnat <amergnat@baylibre.com> wrote:

> This adds support for PixArt Imaging’s miniature low power
> optical navigation chip using LASER light source
> enabling digital surface tracking.

Hi Alexandre,

So I have no problem with this as an IIO driver, but for devices that
are somewhat 'on the edge' I always like to get a clear answer to the
question: Why not input?

I would also argue that, to actually be 'useful' we would typically need
some representation of the 'mechanicals' that are providing the motion
being measured.  Looking at the datasheet this includes, rotating shafts
(side or end on), disk edges and flat surface tracking (mouse like).

That's easy enough to do with the iio in kernel consumer interface. These
are similar to when we handle analog electronic front ends.

I you can, please describe what it is being used for in your application
as that may give us somewhere to start!

+ CC Dmitry and linux-input.

> 
> This IIO driver allow to read delta position on 2 axis (X and Y).
> The values can be taken through ponctual "read_raw" which will issue
> a read in the device registers to return the deltas or subscribe
> to the data buffer feed automaticaly by a new value using a
> trigger gpio. The buffer payload is: |16 bits delta X|16 bits delta Y|timestamp|.
> The possible I2C adresses are 0x73, 0x75 and 0x79.
> 
> Unfortunately, the device configuration must be hardcoded in the initialization
> function and can't be changed "on-the-fly" in user space due to the lack of
> configuration interface.
> 
> The "ot" directory is added to coutain Optical Tracker drivers.
From a shared interface point of view we don't really care 'how', but rather 'what'.
So arguably this is actually measuring differential position, or sort of
'velocity'.  I think it should probably be mapped to simple position though.  See
below.

> 
> Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com>

Comments inline.

> ---
>  drivers/iio/Kconfig      |   1 +
>  drivers/iio/Makefile     |   1 +
>  drivers/iio/ot/Kconfig   |  16 ++
>  drivers/iio/ot/Makefile  |   6 +
>  drivers/iio/ot/pat9125.c | 407 +++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 431 insertions(+)
>  create mode 100644 drivers/iio/ot/Kconfig
>  create mode 100644 drivers/iio/ot/Makefile
>  create mode 100644 drivers/iio/ot/pat9125.c
> 
> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
> index d08aeb4..bdf1bd0 100644
> --- a/drivers/iio/Kconfig
> +++ b/drivers/iio/Kconfig
> @@ -86,6 +86,7 @@ source "drivers/iio/light/Kconfig"
>  source "drivers/iio/magnetometer/Kconfig"
>  source "drivers/iio/multiplexer/Kconfig"
>  source "drivers/iio/orientation/Kconfig"
> +source "drivers/iio/ot/Kconfig"
>  if IIO_TRIGGER
>     source "drivers/iio/trigger/Kconfig"
>  endif #IIO_TRIGGER
> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
> index cb59932..fdda2e1 100644
> --- a/drivers/iio/Makefile
> +++ b/drivers/iio/Makefile
> @@ -32,6 +32,7 @@ obj-y += light/
>  obj-y += magnetometer/
>  obj-y += multiplexer/
>  obj-y += orientation/
> +obj-y += ot/
>  obj-y += potentiometer/
>  obj-y += potentiostat/
>  obj-y += pressure/
> diff --git a/drivers/iio/ot/Kconfig b/drivers/iio/ot/Kconfig
> new file mode 100644
> index 0000000..3d17fda
> --- /dev/null
> +++ b/drivers/iio/ot/Kconfig
> @@ -0,0 +1,16 @@
> +#
> +# Optical tracker sensors
> +#
> +# When adding new entries keep the list in alphabetical order
> +
> +menu "Optical tracker sensors"
> +
> +config PAT9125
> +	tristate "Optical tracker PAT9125 I2C driver"
> +	depends on I2C
> +	select IIO_BUFFER
> +	help
> +	  Say yes here to build support for PAT9125 optical tracker
> +	  sensors.
> +
> +endmenu
> diff --git a/drivers/iio/ot/Makefile b/drivers/iio/ot/Makefile
> new file mode 100644
> index 0000000..cf29491
> --- /dev/null
> +++ b/drivers/iio/ot/Makefile
> @@ -0,0 +1,6 @@
> +#
> +# Makefile for industrial I/O Optical tracker sensor drivers
> +#
> +
> +# When adding new entries keep the list in alphabetical order
> +obj-$(CONFIG_PAT9125) += pat9125.o
> diff --git a/drivers/iio/ot/pat9125.c b/drivers/iio/ot/pat9125.c
> new file mode 100644
> index 0000000..f416bfa
> --- /dev/null
> +++ b/drivers/iio/ot/pat9125.c
> @@ -0,0 +1,407 @@
> +// SPDX-License-Identifier: (GPL-2.0)
> +/*
> + * Copyright (C) 2018 BayLibre, SAS

Probably going to need to add 2019!

> + * Author: Alexandre Mergnat <amergnat@baylibre.com>
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/regmap.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/slab.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/events.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/kfifo_buf.h>
> +
> +/* I2C Address function to ID pin*/
> +#define PAT9125_I2C_ADDR_HI		0x73
> +#define PAT9125_I2C_ADDR_LO		0x75
> +#define PAT9125_I2C_ADDR_NC		0x79
> +
> +/* Registers */
> +#define PAT9125_PRD_ID1_REG		0x00
> +#define PAT9125_PRD_ID2_REG		0x01
> +#define PAT9125_MOTION_STATUS_REG	0x02
> +#define PAT9125_DELTA_X_LO_REG		0x03
> +#define PAT9125_DELTA_Y_LO_REG		0x04
> +#define PAT9125_OP_MODE_REG		0x05
> +#define PAT9125_CONFIG_REG		0x06
> +#define PAT9125_WRITE_PROTEC_REG	0x09
> +#define PAT9125_SLEEP1_REG		0x0A
> +#define PAT9125_SLEEP2_REG		0x0B
> +#define PAT9125_RES_X_REG		0x0D
> +#define PAT9125_RES_Y_REG		0x0E
> +#define PAT9125_DELTA_XY_HI_REG		0x12
> +#define PAT9125_SHUTER_REG		0x14
> +#define PAT9125_FRAME_AVG_REG		0x17
> +#define PAT9125_ORIENTATION_REG		0x19
> +
> +/* Masks */
> +#define PAT9125_VALID_MOTION_DATA_MASK	0x80
> +#define PAT9125_RESET_MASK		0x80

BIT?

> +
> +/* Registers' values */
> +#define PAT9125_SENSOR_ID_VAL			0x31
> +#define PAT9125_DISABLE_WRITE_PROTECT_VAL	0x5A
> +#define PAT9125_ENABLE_WRITE_PROTECT_VAL	0x00
> +#define PAT9125_CPI_RESOLUTION_X_VAL		0x65
> +#define PAT9125_CPI_RESOLUTION_Y_VAL		0xFF
> +
> +/* Default Value of sampled value size */
> +#define PAT9125_SAMPLED_VAL_BIT_SIZE		12
These defines actually mostly add to obscure what is going on.
In my mind, the values themselves are clearer than the defines.

> +#define PAT9125_SAMPLED_VAL_BYTE_SIZE		2	/* 12 bits by default */
> +#define PAT9125_TIMESTAMP_BYTE_SIZE		8	/* 64 bits */
> +#define PAT9125_PAYLOAD_BYTE_SIZE \
> +	(2 * PAT9125_SAMPLED_VAL_BYTE_SIZE + PAT9125_TIMESTAMP_BYTE_SIZE)
> +#define PAT9125_REALBITS_XY_VAL \
> +	(2 * PAT9125_SAMPLED_VAL_BIT_SIZE)
> +#define PAT9125_STORAGEBITS_XY_VAL \
> +	(2 * PAT9125_SAMPLED_VAL_BYTE_SIZE)
> +
> +struct pat9125_data {
> +	struct i2c_client *client;
I think client is only used to get dev for debug message purposes.
you can get that in a number of other ways including from the regmap.

Hence drop client from this structure.

> +	struct regmap *regmap;
> +	s16 delta_x;
> +	s16 delta_y;
> +};
> +
> +static const struct iio_event_spec pat9125_event = {
> +	.type = IIO_EV_TYPE_THRESH,
> +	.dir = IIO_EV_DIR_RISING,
> +	.mask_separate = BIT(IIO_EV_INFO_VALUE),
> +};
> +
> +static const struct iio_chan_spec pat9125_channels[] = {
> +	{
> +		.type = IIO_DISTANCE,
> +		.modified = 1,
> +		.channel2 = IIO_MOD_X_AND_Y,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +		.address = 0,
> +		.scan_index = 0,
> +		.scan_type = {
> +			.sign = 's',			\
> +			.realbits = 24,			\
That is not how mod_x_and_y is used.  It only exists for events, not
to let you define your own version of channels with two elements.

In IIO these are two separate channels and must be represented as
such.

There is another problem here, in what type of channel this actually
is.  Now I may have the following wrong as the datasheet is less than
super clear!.

It's not outputting position at all, but rather a delta value from
when it was either:
1. last read? (not sure on this)
2. last fired an interrupt?
3. last took a sample? (fixed sampling frequency). 

If 1 or 2, I would suggest that you provide absolute position to
Linux.  So add the value to a software counter and provide that.
32 bits should be plenty of resolution for that.

If 3, it's a velocity sensor so report it as velocity not position.

If it is actually not possible to report the two channels separately
then don't report them at all except via the buffered interface and
set the available scan masks so that both are on.

> +			.storagebits = 32,		\
> +			.shift = 8,			\
> +			.endianness = IIO_BE,		\
> +		},
> +		.event_spec = &pat9125_event,
> +		.num_event_specs = 1,
> +	},
> +	IIO_CHAN_SOFT_TIMESTAMP(1),
> +};
> +
> +static int pat9125_read_delta(struct pat9125_data *data)
> +{
> +	struct regmap *regmap = data->regmap;
> +	int status = 0;
> +	int val_x = 0;
> +	int val_y = 0;
> +	int val_xy = 0;
val_xy is a confusing name.  perhaps val_high_nibbles?

> +	int r;
ret, be consistent with naming.

> +
> +	r = regmap_read(regmap, PAT9125_MOTION_STATUS_REG, &status);
> +	if (r < 0)
> +		return r;
> +
> +	/* Check motion bit in bit7 */
Why?  This is reading a value, if there is no motion, then is the
right answer not to set it to such? i.e. 0.

Also we carefully used a macro to hide the fact it is bit7 so don't put
it in the comment ;)

> +	if (status & PAT9125_VALID_MOTION_DATA_MASK) {
> +		r = regmap_read(regmap, PAT9125_DELTA_X_LO_REG, &val_x);
> +		if (r < 0)
> +			return r;

One thing that isn't clear from the datasheet I am looking at is whether this
is potentially racey.  What stops an update whilst we are reading?

> +
> +		r = regmap_read(regmap, PAT9125_DELTA_Y_LO_REG, &val_y);
> +		if (r < 0)
> +			return r;
> +
> +		r = regmap_read(regmap, PAT9125_DELTA_XY_HI_REG, &val_xy);
> +		if (r < 0)
> +			return r;
> +
> +		data->delta_x = val_x | ((val_xy << 4) & 0xF00);
> +		data->delta_y = val_y | ((val_xy << 8) & 0xF00);
> +
> +		if (data->delta_x & 0x800)
> +			data->delta_x |= 0xF000;
This looks like sign extension to me.  We have standard functions for
that.sign_extend32.  Note that it's actually cheaper to do a 32 bit sign extend
to 32 bits than it is to 16, then assign back to 16 if you want to.

> +
> +		if (data->delta_y & 0x800)
> +			data->delta_y |= 0xF000;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * pat9125_read_raw() - Sample and return the value(s)
> + * function to the associated channel info enum.
> + **/
> +static int pat9125_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int *val, int *val2, long mask)
> +{
> +	struct pat9125_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = pat9125_read_delta(data);
> +		if (ret)
> +			return ret;
> +
> +		*val = data->delta_x;
> +		*val2 = data->delta_y;

That's not how the IIO interface works.  You can't just push values
into different fields.  These are two axis, and are independent.
If you absolutely have to read them together then it must be done with
the buffered interface and we simply don't provide a read raw interface at
all.

> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +/**
> + * pat9125_read_event_value() - return last sampled value.
> + **/
> +static int pat9125_read_event_value(struct iio_dev *indio_dev,
> +				    const struct iio_chan_spec *chan,
> +				    enum iio_event_type type,
> +				    enum iio_event_direction dir,
> +				    enum iio_event_info info,
> +				    int *val,
> +				    int *val2)
> +{
> +	struct pat9125_data *data = iio_priv(indio_dev);
> +
> +	switch (info) {
> +	case IIO_EV_INFO_VALUE:
> +		val[0] = data->delta_x;
> +		val[1] = data->delta_y;

Silly question for you.  What happens if you set the delta values to 0?
Do we get an interrupt which is effectively data ready?
If we do, you might want to think about a scheme where that is an option.
As things currently stand we have a confusing interface where changing this
threshold effects the buffered data output.   That should only be the
case if this interface is for a trigger, not an event.

> +		*val2 = 2;
> +		return IIO_VAL_INT_MULTIPLE;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +/**
> + * pat9125_event_handler() - handling ring and non ring events
> + * @irq: The irq being handled.
> + * @private: struct iio_device pointer for the device.
This blank line doesn't add anything to readability so tidy it up.
> + *
> + */
> +static irqreturn_t pat9125_event_handler(int irq, void *private)
> +{
> +	struct iio_dev *indio_dev = private;
> +	struct pat9125_data *data = iio_priv(indio_dev);
> +	int ret = 0;

ret is always set below, so don't initialise.

> +	u8 *payload;
> +	s64 last_timestamp = iio_get_time_ns(indio_dev);
> +
> +	payload = kmalloc(sizeof(s64) + 2 *
> +		  PAT9125_SAMPLED_VAL_BYTE_SIZE, GFP_KERNEL);
Why on the stack?  It's small and last thing you want to
do is a memory allocation ever time.

Also, you haven't allowed for the required alignment of the
timestamp. This needs to be a multiple of 8 bytes.
 
> +
> +	ret = pat9125_read_delta(data);
> +	if (ret)
> +		return ret;
> +	memcpy(&payload[0], &data->delta_x, 2);

Make payload and array of u16 and you can just assign directly.

> +	memcpy(&payload[2], &data->delta_y, 2);
> +
> +	iio_push_to_buffers_with_timestamp(indio_dev, payload, last_timestamp);

Hmm. This is not how IIO is normally used.  So the oddity here is the sensor
only produces an interrupt on a change of value.

So in IIO terms this would normally be considered a 'value change trigger'
driving normal buffered sampling.  I think it is also perfectly valid
if someone wants evenly spaced samples to read on demand?  If so
then you definitely want to allow use of high resolution timer triggers
and similar. 

So, create a trigger that is base don the 'event' - there are other drivers
doing this.  It's possible that you might want that 'trigger' to also
call the iio event interface, but usually only if there are usecases where
knowing "motion happened" is useful rather than what the motion was.

Then use the triggered_buffer helpers to set up all the data capture side
of things.

> +	iio_push_event(indio_dev,
> +		       IIO_MOD_EVENT_CODE(IIO_DISTANCE,
> +					  0,
> +					  IIO_MOD_X_AND_Y,
That one is wrong btw, surely it is IIO_MOD_X_OR_Y. You aren't
triggering on motion in 'both X and Y' directions, motion in one
of them is enough.

> +					  IIO_EV_TYPE_THRESH,
> +					  IIO_EV_DIR_RISING),
> +		       last_timestamp);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int pat9125_configure_ring(struct iio_dev *indio_dev)
> +{
> +	struct iio_buffer *buffer;
> +	struct pat9125_data *data = iio_priv(indio_dev);
> +
> +	buffer = devm_iio_kfifo_allocate(&data->client->dev);
> +	if (!buffer)
> +		return -ENOMEM;
Hmm. Direct use of kfifo's is fairly unusual as it normally means there
is something unusual about how the device is being mapped to the IIO
elements.

> +
> +	iio_device_attach_buffer(indio_dev, buffer);
> +	indio_dev->modes |= INDIO_BUFFER_SOFTWARE;
> +
> +	return 0;
> +}
> +
> +static const struct regmap_config pat9125_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +};
> +
> +static const struct iio_info pat9125_info = {
> +	.read_raw = pat9125_read_raw,
> +	.read_event_value = &pat9125_read_event_value,
> +};
> +
> +static int pat9125_probe(struct i2c_client *client,
> +			 const struct i2c_device_id *id)
> +{
> +	struct pat9125_data *data;
> +	struct iio_dev *indio_dev;
> +	int r, sensor_pid;
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> +	if (!indio_dev) {
> +		dev_err(&client->dev, "IIO device allocation failed");
> +		return -ENOMEM;
> +	}
> +
> +	data = iio_priv(indio_dev);
> +	data->client = client;
> +
> +	indio_dev->dev.parent = &client->dev;
> +	indio_dev->name = id->name;
> +	indio_dev->channels = pat9125_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(pat9125_channels);
> +	indio_dev->info = &pat9125_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	r = pat9125_configure_ring(indio_dev);

I'd rename this, its a fifo, rather than a ring (it was a ring
early in IIO's history so the naming still kicks around
in a few drivers).

> +	if (r < 0) {
> +		dev_err(&client->dev, "FIFO buffer allocation failed");
> +		return r;
> +	}
> +
> +	data->regmap = devm_regmap_init_i2c(client, &pat9125_regmap_config);
> +	if (IS_ERR(data->regmap)) {
> +		dev_err(&client->dev,
> +			"regmap init failed %ld",
> +			PTR_ERR(data->regmap));

This is wrapped onto more lines than necessary.  Kernel coding convention
is to keep as many parameters on each line as fit under 80 chars.

> +		return PTR_ERR(data->regmap);
> +	}
> +
> +	/* Check device ID */
> +	r = regmap_read(data->regmap, PAT9125_PRD_ID1_REG, &sensor_pid);
> +	if (r < 0)
> +		goto reg_access_fail;
> +	if (sensor_pid != PAT9125_SENSOR_ID_VAL)
> +		return -ENODEV;
> +
> +	/* Software reset (i.e. set bit7 to 1).
Multiline, comment style in most of the kernel, including IIO is
/*
 * Soft reset...
 */

> +	 * It will reset to 0 automatically
> +	 */
> +	r = regmap_write_bits(data->regmap,
> +			      PAT9125_CONFIG_REG,
> +			      PAT9125_RESET_MASK,
> +			      1);
> +	if (r < 0)
> +		goto reg_access_fail;
> +
> +	/* Delay 20ms */
> +	msleep(20);
> +
> +	/* Disable write protect */
Don't add comments for things that are obvious from the code.
(i.e. the sleep above and this one).

> +	r = regmap_write(data->regmap,
> +			 PAT9125_WRITE_PROTEC_REG,
> +			 PAT9125_DISABLE_WRITE_PROTECT_VAL);
> +	if (r < 0)
> +		goto reg_access_fail;
> +
> +	/* Set X-axis resolution (depends on application) */
> +	r = regmap_write(data->regmap,
> +			 PAT9125_RES_X_REG,
> +			 0x0A);

Need to control this from somewhere. Device tree binding perhaps?

> +	if (r < 0)
> +		goto reg_access_fail;
> +
> +	/* Set Y-axis resolution (depends on application) */
> +	r = regmap_write(data->regmap,
> +			 PAT9125_RES_Y_REG,
> +			 0x0A);
> +	if (r < 0)
> +		goto reg_access_fail;
> +
> +	/* Enable write protection */
> +	r = regmap_write(data->regmap,
> +			 PAT9125_WRITE_PROTEC_REG,
> +			 PAT9125_ENABLE_WRITE_PROTECT_VAL);
> +	if (r < 0)
> +		goto reg_access_fail;
> +
> +	r = devm_iio_device_register(&client->dev, indio_dev);
> +	if (r) {
> +		dev_err(&client->dev, "IIO device register failed");
> +		return r;
> +	}
> +
> +	i2c_set_clientdata(client, indio_dev);
> +
> +	dev_info(&client->dev, "%s: sensor '%s'\n",
> +		 dev_name(&indio_dev->dev),
> +		 client->name);
> +
> +	/* Make read to reset motion bit status */
> +	r = pat9125_read_delta(data);
> +	if (r)
> +		goto reg_access_fail;
> +
> +	/* Init GPIO IRQ */
> +	if (client->irq) {
> +		r = devm_request_threaded_irq(&client->dev,
> +					      client->irq,
> +					      NULL,
> +					      pat9125_event_handler,
> +					      IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> +					      "pat9125",
> +					      indio_dev);
> +		if (r)
> +			return r;
> +	}
> +	return 0;
> +
> +reg_access_fail:
This just makes the code flow harder to follow.  If you want to add
the print out, just replicate it, with information for example on 
which register access failed.

Only do goto, return paths if there is actual code to run.

> +	dev_err(&client->dev, "register access failed %d", r);
> +	return r;
> +}
> +
> +static int pat9125_remove(struct i2c_client *client)
> +{
> +	dev_info(&client->dev, "PAT9125 removed\n");
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id pat9125_id[] = {
> +	{ "pat9125", 0 },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(i2c, pat9125_id);
> +
> +static const unsigned short normal_i2c[] = {
> +	PAT9125_I2C_ADDR_HI,
> +	PAT9125_I2C_ADDR_LO,
> +	PAT9125_I2C_ADDR_NC,
> +	I2C_CLIENT_END
> +};
> +
> +static struct i2c_driver pat9125_driver = {
> +	.driver = {
> +		.name = "pat9125",
> +	},
> +	.probe = pat9125_probe,
> +	.remove = pat9125_remove,
> +	.address_list = normal_i2c,
> +	.id_table = pat9125_id,
> +};
> +
> +module_i2c_driver(pat9125_driver);
> +
> +MODULE_AUTHOR("Alexandre Mergnat <amergnat@baylibre.com>");
> +MODULE_DESCRIPTION("Optical Tracking sensor");
> +MODULE_LICENSE("GPL");
> +
Clean up this blank line.

Thanks,

Jonathan

^ permalink raw reply

* [PATCH v4 2/2] Input: add Apple SPI keyboard and trackpad driver.
From: Ronald Tschalär @ 2019-04-07  5:03 UTC (permalink / raw)
  To: Dmitry Torokhov, Henrik Rydberg, Andy Shevchenko,
	Greg Kroah-Hartman
  Cc: Lukas Wunner, Federico Lorenzi, linux-input, linux-kernel
In-Reply-To: <20190407050358.2976-1-ronald@innovation.ch>

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.

CC: Federico Lorenzi <federico@travelground.com>
CC: Lukas Wunner <lukas@wunner.de>
CC: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://bugzilla.kernel.org/show_bug.cgi?id=99891
Link: https://bugzilla.kernel.org/show_bug.cgi?id=108331
Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
---
 drivers/input/keyboard/Kconfig          |   15 +
 drivers/input/keyboard/Makefile         |    1 +
 drivers/input/keyboard/applespi.c       | 1999 +++++++++++++++++++++++
 drivers/input/keyboard/applespi.h       |   29 +
 drivers/input/keyboard/applespi_trace.h |   94 ++
 5 files changed, 2138 insertions(+)
 create mode 100644 drivers/input/keyboard/applespi.c
 create mode 100644 drivers/input/keyboard/applespi.h
 create mode 100644 drivers/input/keyboard/applespi_trace.h

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index a878351f1643..d0a9e7fa2508 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -70,6 +70,21 @@ config KEYBOARD_AMIGA
 config ATARI_KBD_CORE
 	bool
 
+config KEYBOARD_APPLESPI
+	tristate "Apple SPI keyboard and trackpad"
+	depends on ACPI && EFI
+	depends on SPI
+	depends on X86 || COMPILE_TEST
+	imply SPI_PXA2XX
+	imply SPI_PXA2XX_PCI
+	imply MFD_INTEL_LPSS_PCI
+	help
+	  Say Y here if you are running Linux on any Apple MacBook8,1 or later,
+	  or any MacBookPro13,* or MacBookPro14,*.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called applespi.
+
 config KEYBOARD_ATARI
 	tristate "Atari keyboard"
 	depends on ATARI
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 182e92985dbf..9283fee2505a 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_KEYBOARD_ADP5520)		+= adp5520-keys.o
 obj-$(CONFIG_KEYBOARD_ADP5588)		+= adp5588-keys.o
 obj-$(CONFIG_KEYBOARD_ADP5589)		+= adp5589-keys.o
 obj-$(CONFIG_KEYBOARD_AMIGA)		+= amikbd.o
+obj-$(CONFIG_KEYBOARD_APPLESPI)		+= applespi.o
 obj-$(CONFIG_KEYBOARD_ATARI)		+= atakbd.o
 obj-$(CONFIG_KEYBOARD_ATKBD)		+= atkbd.o
 obj-$(CONFIG_KEYBOARD_BCM)		+= bcm-keypad.o
diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
new file mode 100644
index 000000000000..f49bc6e544d7
--- /dev/null
+++ b/drivers/input/keyboard/applespi.c
@@ -0,0 +1,1999 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * MacBook (Pro) SPI keyboard and touchpad driver
+ *
+ * Copyright (c) 2015-2018 Federico Lorenzi
+ * Copyright (c) 2017-2018 Ronald Tschalär
+ */
+
+/*
+ * The keyboard and touchpad controller on the MacBookAir6, MacBookPro12,
+ * MacBook8 and newer can be driven either by USB or SPI. However the USB
+ * pins are only connected on the MacBookAir6 and 7 and the MacBookPro12.
+ * All others need this driver. The interface is selected using ACPI methods:
+ *
+ * * UIEN ("USB Interface Enable"): If invoked with argument 1, disables SPI
+ *   and enables USB. If invoked with argument 0, disables USB.
+ * * UIST ("USB Interface Status"): Returns 1 if USB is enabled, 0 otherwise.
+ * * SIEN ("SPI Interface Enable"): If invoked with argument 1, disables USB
+ *   and enables SPI. If invoked with argument 0, disables SPI.
+ * * SIST ("SPI Interface Status"): Returns 1 if SPI is enabled, 0 otherwise.
+ * * ISOL: Resets the four GPIO pins used for SPI. Intended to be invoked with
+ *   argument 1, then once more with argument 0.
+ *
+ * UIEN and UIST are only provided on models where the USB pins are connected.
+ *
+ * SPI-based Protocol
+ * ------------------
+ *
+ * The device and driver exchange messages (struct message); each message is
+ * encapsulated in one or more packets (struct spi_packet). There are two types
+ * of exchanges: reads, and writes. A read is signaled by a GPE, upon which one
+ * message can be read from the device. A write exchange consists of writing a
+ * command message, immediately reading a short status packet, and then, upon
+ * receiving a GPE, reading the response message. Write exchanges cannot be
+ * interleaved, i.e. a new write exchange must not be started till the previous
+ * write exchange is complete. Whether a received message is part of a read or
+ * write exchange is indicated in the encapsulating packet's flags field.
+ *
+ * A single message may be too large to fit in a single packet (which has a
+ * fixed, 256-byte size). In that case it will be split over multiple,
+ * consecutive packets.
+ */
+
+#include <linux/acpi.h>
+#include <linux/crc16.h>
+#include <linux/debugfs.h>
+#include <linux/delay.h>
+#include <linux/efi.h>
+#include <linux/input.h>
+#include <linux/input/mt.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/spinlock.h>
+#include <linux/spi/spi.h>
+#include <linux/wait.h>
+#include <linux/workqueue.h>
+
+#include <asm/barrier.h>
+#include <asm/unaligned.h>
+
+#define CREATE_TRACE_POINTS
+#include "applespi.h"
+#include "applespi_trace.h"
+
+#define APPLESPI_PACKET_SIZE	256
+#define APPLESPI_STATUS_SIZE	4
+
+#define PACKET_TYPE_READ	0x20
+#define PACKET_TYPE_WRITE	0x40
+#define PACKET_DEV_KEYB		0x01
+#define PACKET_DEV_TPAD		0x02
+#define PACKET_DEV_INFO		0xd0
+
+#define MAX_ROLLOVER		6
+
+#define MAX_FINGERS		11
+#define MAX_FINGER_ORIENTATION	16384
+#define MAX_PKTS_PER_MSG	2
+
+#define KBD_BL_LEVEL_MIN	32U
+#define KBD_BL_LEVEL_MAX	255U
+#define KBD_BL_LEVEL_SCALE	1000000U
+#define KBD_BL_LEVEL_ADJ	\
+	((KBD_BL_LEVEL_MAX - KBD_BL_LEVEL_MIN) * KBD_BL_LEVEL_SCALE / 255U)
+
+#define EFI_BL_LEVEL_NAME	L"KeyboardBacklightLevel"
+#define EFI_BL_LEVEL_GUID	EFI_GUID(0xa076d2af, 0x9678, 0x4386, 0x8b, 0x58, 0x1f, 0xc8, 0xef, 0x04, 0x16, 0x19)
+
+#define APPLE_FLAG_FKEY		0x01
+
+#define SPI_RW_CHG_DELAY_US	100	/* from experimentation, in µs */
+
+#define SYNAPTICS_VENDOR_ID	0x06cb
+
+static unsigned int fnmode = 1;
+module_param(fnmode, uint, 0644);
+MODULE_PARM_DESC(fnmode, "Mode of Fn key on Apple keyboards (0 = disabled, [1] = fkeyslast, 2 = fkeysfirst)");
+
+static unsigned int fnremap;
+module_param(fnremap, uint, 0644);
+MODULE_PARM_DESC(fnremap, "Remap Fn key ([0] = no-remap; 1 = left-ctrl, 2 = left-shift, 3 = left-alt, 4 = left-meta, 6 = right-shift, 7 = right-alt, 8 = right-meta)");
+
+static bool iso_layout;
+module_param(iso_layout, bool, 0644);
+MODULE_PARM_DESC(iso_layout, "Enable/Disable hardcoded ISO-layout of the keyboard. ([0] = disabled, 1 = enabled)");
+
+static char touchpad_dimensions[40];
+module_param_string(touchpad_dimensions, touchpad_dimensions,
+		    sizeof(touchpad_dimensions), 0444);
+MODULE_PARM_DESC(touchpad_dimensions, "The pixel dimensions of the touchpad, as XxY+W+H .");
+
+/**
+ * struct keyboard_protocol - keyboard message.
+ * message.type = 0x0110, message.length = 0x000a
+ *
+ * @unknown1:		unknown
+ * @modifiers:		bit-set of modifier/control keys pressed
+ * @unknown2:		unknown
+ * @keys_pressed:	the (non-modifier) keys currently pressed
+ * @fn_pressed:		whether the fn key is currently pressed
+ * @crc16:		crc over the whole message struct (message header +
+ *			this struct) minus this @crc16 field
+ */
+struct keyboard_protocol {
+	__u8			unknown1;
+	__u8			modifiers;
+	__u8			unknown2;
+	__u8			keys_pressed[MAX_ROLLOVER];
+	__u8			fn_pressed;
+	__le16			crc16;
+};
+
+/**
+ * struct tp_finger - single trackpad finger structure, le16-aligned
+ *
+ * @origin:		zero when switching track finger
+ * @abs_x:		absolute x coodinate
+ * @abs_y:		absolute y coodinate
+ * @rel_x:		relative x coodinate
+ * @rel_y:		relative y coodinate
+ * @tool_major:		tool area, major axis
+ * @tool_minor:		tool area, minor axis
+ * @orientation:	16384 when point, else 15 bit angle
+ * @touch_major:	touch area, major axis
+ * @touch_minor:	touch area, minor axis
+ * @unused:		zeros
+ * @pressure:		pressure on forcetouch touchpad
+ * @multi:		one finger: varies, more fingers: constant
+ * @crc16:		on last finger: crc over the whole message struct
+ *			(i.e. message header + this struct) minus the last
+ *			@crc16 field; unknown on all other fingers.
+ */
+struct tp_finger {
+	__le16 origin;
+	__le16 abs_x;
+	__le16 abs_y;
+	__le16 rel_x;
+	__le16 rel_y;
+	__le16 tool_major;
+	__le16 tool_minor;
+	__le16 orientation;
+	__le16 touch_major;
+	__le16 touch_minor;
+	__le16 unused[2];
+	__le16 pressure;
+	__le16 multi;
+	__le16 crc16;
+};
+
+/**
+ * struct touchpad_protocol - touchpad message.
+ * message.type = 0x0210
+ *
+ * @unknown1:		unknown
+ * @clicked:		1 if a button-click was detected, 0 otherwise
+ * @unknown2:		unknown
+ * @number_of_fingers:	the number of fingers being reported in @fingers
+ * @clicked2:		same as @clicked
+ * @unknown3:		unknown
+ * @fingers:		the data for each finger
+ */
+struct touchpad_protocol {
+	__u8			unknown1[1];
+	__u8			clicked;
+	__u8			unknown2[28];
+	__u8			number_of_fingers;
+	__u8			clicked2;
+	__u8			unknown3[16];
+	struct tp_finger	fingers[0];
+};
+
+/**
+ * struct command_protocol_tp_info - get touchpad info.
+ * message.type = 0x1020, message.length = 0x0000
+ *
+ * @crc16:		crc over the whole message struct (message header +
+ *			this struct) minus this @crc16 field
+ */
+struct command_protocol_tp_info {
+	__le16			crc16;
+};
+
+/**
+ * struct touchpad_info - touchpad info response.
+ * message.type = 0x1020, message.length = 0x006e
+ *
+ * @unknown1:		unknown
+ * @model_flags:	flags (vary by model number, but significance otherwise
+ *			unknown)
+ * @model_no:		the touchpad model number
+ * @unknown2:		unknown
+ * @crc16:		crc over the whole message struct (message header +
+ *			this struct) minus this @crc16 field
+ */
+struct touchpad_info_protocol {
+	__u8			unknown1[105];
+	__u8			model_flags;
+	__u8			model_no;
+	__u8			unknown2[3];
+	__le16			crc16;
+};
+
+/**
+ * struct command_protocol_mt_init - initialize multitouch.
+ * message.type = 0x0252, message.length = 0x0002
+ *
+ * @cmd:		value: 0x0102
+ * @crc16:		crc over the whole message struct (message header +
+ *			this struct) minus this @crc16 field
+ */
+struct command_protocol_mt_init {
+	__le16			cmd;
+	__le16			crc16;
+};
+
+/**
+ * struct command_protocol_capsl - toggle caps-lock led
+ * message.type = 0x0151, message.length = 0x0002
+ *
+ * @unknown:		value: 0x01 (length?)
+ * @led:		0 off, 2 on
+ * @crc16:		crc over the whole message struct (message header +
+ *			this struct) minus this @crc16 field
+ */
+struct command_protocol_capsl {
+	__u8			unknown;
+	__u8			led;
+	__le16			crc16;
+};
+
+/**
+ * struct command_protocol_bl - set keyboard backlight brightness
+ * message.type = 0xB051, message.length = 0x0006
+ *
+ * @const1:		value: 0x01B0
+ * @level:		the brightness level to set
+ * @const2:		value: 0x0001 (backlight off), 0x01F4 (backlight on)
+ * @crc16:		crc over the whole message struct (message header +
+ *			this struct) minus this @crc16 field
+ */
+struct command_protocol_bl {
+	__le16			const1;
+	__le16			level;
+	__le16			const2;
+	__le16			crc16;
+};
+
+/**
+ * struct message - a complete spi message.
+ *
+ * Each message begins with fixed header, followed by a message-type specific
+ * payload, and ends with a 16-bit crc. Because of the varying lengths of the
+ * payload, the crc is defined at the end of each payload struct, rather than
+ * in this struct.
+ *
+ * @type:	the message type
+ * @zero:	always 0
+ * @counter:	incremented on each message, rolls over after 255; there is a
+ *		separate counter for each message type.
+ * @rsp_buf_len:response buffer length (the exact nature of this field is quite
+ *		speculative). On a request/write this is often the same as
+ *		@length, though in some cases it has been seen to be much larger
+ *		(e.g. 0x400); on a response/read this the same as on the
+ *		request; for reads that are not responses it is 0.
+ * @length:	length of the remainder of the data in the whole message
+ *		structure (after re-assembly in case of being split over
+ *		multiple spi-packets), minus the trailing crc. The total size
+ *		of the message struct is therefore @length + 10.
+ */
+struct message {
+	__le16		type;
+	__u8		zero;
+	__u8		counter;
+	__le16		rsp_buf_len;
+	__le16		length;
+	union {
+		struct keyboard_protocol	keyboard;
+		struct touchpad_protocol	touchpad;
+		struct touchpad_info_protocol	tp_info;
+		struct command_protocol_tp_info	tp_info_command;
+		struct command_protocol_mt_init	init_mt_command;
+		struct command_protocol_capsl	capsl_command;
+		struct command_protocol_bl	bl_command;
+		__u8				data[0];
+	};
+};
+
+/* type + zero + counter + rsp_buf_len + length */
+#define MSG_HEADER_SIZE	8
+
+/**
+ * struct spi_packet - a complete spi packet; always 256 bytes. This carries
+ * the (parts of the) message in the data. But note that this does not
+ * necessarily contain a complete message, as in some cases (e.g. many
+ * fingers pressed) the message is split over multiple packets (see the
+ * @offset, @remaining, and @length fields). In general the data parts in
+ * spi_packet's are concatenated until @remaining is 0, and the result is an
+ * message.
+ *
+ * @flags:	0x40 = write (to device), 0x20 = read (from device); note that
+ *		the response to a write still has 0x40.
+ * @device:	1 = keyboard, 2 = touchpad
+ * @offset:	specifies the offset of this packet's data in the complete
+ *		message; i.e. > 0 indicates this is a continuation packet (in
+ *		the second packet for a message split over multiple packets
+ *		this would then be the same as the @length in the first packet)
+ * @remaining:	number of message bytes remaining in subsequents packets (in
+ *		the first packet of a message split over two packets this would
+ *		then be the same as the @length in the second packet)
+ * @length:	length of the valid data in the @data in this packet
+ * @data:	all or part of a message
+ * @crc16:	crc over this whole structure minus this @crc16 field. This
+ *		covers just this packet, even on multi-packet messages (in
+ *		contrast to the crc in the message).
+ */
+struct spi_packet {
+	__u8			flags;
+	__u8			device;
+	__le16			offset;
+	__le16			remaining;
+	__le16			length;
+	__u8			data[246];
+	__le16			crc16;
+};
+
+struct spi_settings {
+	u64	spi_cs_delay;		/* cs-to-clk delay in us */
+	u64	reset_a2r_usec;		/* active-to-receive delay? */
+	u64	reset_rec_usec;		/* ? (cur val: 10) */
+};
+
+/* this mimics struct drm_rect */
+struct applespi_tp_info {
+	int	x_min;
+	int	y_min;
+	int	x_max;
+	int	y_max;
+};
+
+struct applespi_data {
+	struct spi_device		*spi;
+	struct spi_settings		spi_settings;
+	struct input_dev		*keyboard_input_dev;
+	struct input_dev		*touchpad_input_dev;
+
+	u8				*tx_buffer;
+	u8				*tx_status;
+	u8				*rx_buffer;
+
+	u8				*msg_buf;
+	unsigned int			saved_msg_len;
+
+	struct applespi_tp_info		tp_info;
+
+	u8				last_keys_pressed[MAX_ROLLOVER];
+	u8				last_keys_fn_pressed[MAX_ROLLOVER];
+	u8				last_fn_pressed;
+	struct input_mt_pos		pos[MAX_FINGERS];
+	int				slots[MAX_FINGERS];
+	acpi_handle			handle;
+	int				gpe;
+	acpi_handle			sien;
+	acpi_handle			sist;
+
+	struct spi_transfer		dl_t;
+	struct spi_transfer		rd_t;
+	struct spi_message		rd_m;
+
+	struct spi_transfer		ww_t;
+	struct spi_transfer		wd_t;
+	struct spi_transfer		wr_t;
+	struct spi_transfer		st_t;
+	struct spi_message		wr_m;
+
+	bool				want_tp_info_cmd;
+	bool				want_mt_init_cmd;
+	bool				want_cl_led_on;
+	bool				have_cl_led_on;
+	unsigned int			want_bl_level;
+	unsigned int			have_bl_level;
+	unsigned int			cmd_msg_cntr;
+	/* lock to protect the above parameters and flags below */
+	spinlock_t			cmd_msg_lock;
+	bool				cmd_msg_queued;
+	enum applespi_evt_type		cmd_evt_type;
+
+	struct led_classdev		backlight_info;
+
+	bool				suspended;
+	bool				drain;
+	wait_queue_head_t		drain_complete;
+	bool				read_active;
+	bool				write_active;
+
+	struct work_struct		work;
+	struct touchpad_info_protocol	rcvd_tp_info;
+
+	struct dentry			*debugfs_root;
+	bool				debug_tp_dim;
+	char				tp_dim_val[40];
+	int				tp_dim_min_x;
+	int				tp_dim_max_x;
+	int				tp_dim_min_y;
+	int				tp_dim_max_y;
+};
+
+static const unsigned char applespi_scancodes[] = {
+	0, 0, 0, 0,
+	KEY_A, KEY_B, KEY_C, KEY_D, KEY_E, KEY_F, KEY_G, KEY_H, KEY_I, KEY_J,
+	KEY_K, KEY_L, KEY_M, KEY_N, KEY_O, KEY_P, KEY_Q, KEY_R, KEY_S, KEY_T,
+	KEY_U, KEY_V, KEY_W, KEY_X, KEY_Y, KEY_Z,
+	KEY_1, KEY_2, KEY_3, KEY_4, KEY_5, KEY_6, KEY_7, KEY_8, KEY_9, KEY_0,
+	KEY_ENTER, KEY_ESC, KEY_BACKSPACE, KEY_TAB, KEY_SPACE, KEY_MINUS,
+	KEY_EQUAL, KEY_LEFTBRACE, KEY_RIGHTBRACE, KEY_BACKSLASH, 0,
+	KEY_SEMICOLON, KEY_APOSTROPHE, KEY_GRAVE, KEY_COMMA, KEY_DOT, KEY_SLASH,
+	KEY_CAPSLOCK,
+	KEY_F1, KEY_F2, KEY_F3, KEY_F4, KEY_F5, KEY_F6, KEY_F7, KEY_F8, KEY_F9,
+	KEY_F10, KEY_F11, KEY_F12, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+	KEY_RIGHT, KEY_LEFT, KEY_DOWN, KEY_UP,
+	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, KEY_102ND,
+	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, KEY_RO, 0, KEY_YEN, 0, 0, 0, 0, 0,
+	0, KEY_KATAKANAHIRAGANA, KEY_MUHENKAN
+};
+
+/*
+ * This must have exactly as many entries as there are bits in
+ * struct keyboard_protocol.modifiers .
+ */
+static const unsigned char applespi_controlcodes[] = {
+	KEY_LEFTCTRL,
+	KEY_LEFTSHIFT,
+	KEY_LEFTALT,
+	KEY_LEFTMETA,
+	0,
+	KEY_RIGHTSHIFT,
+	KEY_RIGHTALT,
+	KEY_RIGHTMETA
+};
+
+struct applespi_key_translation {
+	u16 from;
+	u16 to;
+	u8 flags;
+};
+
+static const struct applespi_key_translation applespi_fn_codes[] = {
+	{ KEY_BACKSPACE, KEY_DELETE },
+	{ KEY_ENTER,	KEY_INSERT },
+	{ KEY_F1,	KEY_BRIGHTNESSDOWN,	APPLE_FLAG_FKEY },
+	{ KEY_F2,	KEY_BRIGHTNESSUP,	APPLE_FLAG_FKEY },
+	{ KEY_F3,	KEY_SCALE,		APPLE_FLAG_FKEY },
+	{ KEY_F4,	KEY_DASHBOARD,		APPLE_FLAG_FKEY },
+	{ KEY_F5,	KEY_KBDILLUMDOWN,	APPLE_FLAG_FKEY },
+	{ KEY_F6,	KEY_KBDILLUMUP,		APPLE_FLAG_FKEY },
+	{ KEY_F7,	KEY_PREVIOUSSONG,	APPLE_FLAG_FKEY },
+	{ KEY_F8,	KEY_PLAYPAUSE,		APPLE_FLAG_FKEY },
+	{ KEY_F9,	KEY_NEXTSONG,		APPLE_FLAG_FKEY },
+	{ KEY_F10,	KEY_MUTE,		APPLE_FLAG_FKEY },
+	{ KEY_F11,	KEY_VOLUMEDOWN,		APPLE_FLAG_FKEY },
+	{ KEY_F12,	KEY_VOLUMEUP,		APPLE_FLAG_FKEY },
+	{ KEY_RIGHT,	KEY_END },
+	{ KEY_LEFT,	KEY_HOME },
+	{ KEY_DOWN,	KEY_PAGEDOWN },
+	{ KEY_UP,	KEY_PAGEUP },
+	{ }
+};
+
+static const struct applespi_key_translation apple_iso_keyboard[] = {
+	{ KEY_GRAVE,	KEY_102ND },
+	{ KEY_102ND,	KEY_GRAVE },
+	{ }
+};
+
+struct applespi_tp_model_info {
+	u16			model;
+	struct applespi_tp_info	tp_info;
+};
+
+static const struct applespi_tp_model_info applespi_tp_models[] = {
+	{
+		.model = 0x04,	/* MB8 MB9 MB10 */
+		.tp_info = { -5087, -182, 5579, 6089 },
+	},
+	{
+		.model = 0x05,	/* MBP13,1 MBP13,2 MBP14,1 MBP14,2 */
+		.tp_info = { -6243, -170, 6749, 7685 },
+	},
+	{
+		.model = 0x06,	/* MBP13,3 MBP14,3 */
+		.tp_info = { -7456, -163, 7976, 9283 },
+	},
+	{}
+};
+
+typedef void (*applespi_trace_fun)(enum applespi_evt_type,
+				   enum applespi_pkt_type, u8 *, size_t);
+
+static applespi_trace_fun applespi_get_trace_fun(enum applespi_evt_type type)
+{
+	switch (type) {
+	case ET_CMD_TP_INI:
+		return trace_applespi_tp_ini_cmd;
+	case ET_CMD_BL:
+		return trace_applespi_backlight_cmd;
+	case ET_CMD_CL:
+		return trace_applespi_caps_lock_cmd;
+	case ET_RD_KEYB:
+		return trace_applespi_keyboard_data;
+	case ET_RD_TPAD:
+		return trace_applespi_touchpad_data;
+	case ET_RD_UNKN:
+		return trace_applespi_unknown_data;
+	default:
+		WARN_ONCE(1, "Unknown msg type %d", type);
+		return trace_applespi_unknown_data;
+	}
+}
+
+static void applespi_setup_read_txfrs(struct applespi_data *applespi)
+{
+	struct spi_message *msg = &applespi->rd_m;
+	struct spi_transfer *dl_t = &applespi->dl_t;
+	struct spi_transfer *rd_t = &applespi->rd_t;
+
+	memset(dl_t, 0, sizeof(*dl_t));
+	memset(rd_t, 0, sizeof(*rd_t));
+
+	dl_t->delay_usecs = applespi->spi_settings.spi_cs_delay;
+
+	rd_t->rx_buf = applespi->rx_buffer;
+	rd_t->len = APPLESPI_PACKET_SIZE;
+
+	spi_message_init(msg);
+	spi_message_add_tail(dl_t, msg);
+	spi_message_add_tail(rd_t, msg);
+}
+
+static void applespi_setup_write_txfrs(struct applespi_data *applespi)
+{
+	struct spi_message *msg = &applespi->wr_m;
+	struct spi_transfer *wt_t = &applespi->ww_t;
+	struct spi_transfer *dl_t = &applespi->wd_t;
+	struct spi_transfer *wr_t = &applespi->wr_t;
+	struct spi_transfer *st_t = &applespi->st_t;
+
+	memset(wt_t, 0, sizeof(*wt_t));
+	memset(dl_t, 0, sizeof(*dl_t));
+	memset(wr_t, 0, sizeof(*wr_t));
+	memset(st_t, 0, sizeof(*st_t));
+
+	/*
+	 * All we need here is a delay at the beginning of the message before
+	 * asserting cs. But the current spi API doesn't support this, so we
+	 * end up with an extra unnecessary (but harmless) cs assertion and
+	 * deassertion.
+	 */
+	wt_t->delay_usecs = SPI_RW_CHG_DELAY_US;
+	wt_t->cs_change = 1;
+
+	dl_t->delay_usecs = applespi->spi_settings.spi_cs_delay;
+
+	wr_t->tx_buf = applespi->tx_buffer;
+	wr_t->len = APPLESPI_PACKET_SIZE;
+	wr_t->delay_usecs = SPI_RW_CHG_DELAY_US;
+
+	st_t->rx_buf = applespi->tx_status;
+	st_t->len = APPLESPI_STATUS_SIZE;
+
+	spi_message_init(msg);
+	spi_message_add_tail(wt_t, msg);
+	spi_message_add_tail(dl_t, msg);
+	spi_message_add_tail(wr_t, msg);
+	spi_message_add_tail(st_t, msg);
+}
+
+static int applespi_async(struct applespi_data *applespi,
+			  struct spi_message *message, void (*complete)(void *))
+{
+	message->complete = complete;
+	message->context = applespi;
+
+	return spi_async(applespi->spi, message);
+}
+
+static inline bool applespi_check_write_status(struct applespi_data *applespi,
+					       int sts)
+{
+	static u8 status_ok[] = { 0xac, 0x27, 0x68, 0xd5 };
+
+	if (sts < 0) {
+		dev_warn(&(applespi)->spi->dev, "Error writing to device: %d\n",
+			 sts);
+		return false;
+	}
+
+	if (memcmp(applespi->tx_status, status_ok, APPLESPI_STATUS_SIZE)) {
+		dev_warn(&(applespi)->spi->dev,
+			 "Error writing to device: %*ph\n",
+			 APPLESPI_STATUS_SIZE, applespi->tx_status);
+		return false;
+	}
+
+	return true;
+}
+
+static int applespi_get_spi_settings(struct applespi_data *applespi)
+{
+	struct acpi_device *adev = ACPI_COMPANION(&(applespi)->spi->dev);
+	const union acpi_object *o;
+	struct spi_settings *settings = &applespi->spi_settings;
+
+	if (!acpi_dev_get_property(adev, "spiCSDelay", ACPI_TYPE_BUFFER, &o))
+		settings->spi_cs_delay = *(u64 *)o->buffer.pointer;
+	else
+		dev_warn(&(applespi)->spi->dev,
+			 "Property spiCSDelay not found\n");
+
+	if (!acpi_dev_get_property(adev, "resetA2RUsec", ACPI_TYPE_BUFFER, &o))
+		settings->reset_a2r_usec = *(u64 *)o->buffer.pointer;
+	else
+		dev_warn(&(applespi)->spi->dev,
+			 "Property resetA2RUsec not found\n");
+
+	if (!acpi_dev_get_property(adev, "resetRecUsec", ACPI_TYPE_BUFFER, &o))
+		settings->reset_rec_usec = *(u64 *)o->buffer.pointer;
+	else
+		dev_warn(&(applespi)->spi->dev,
+			 "Property resetRecUsec not found\n");
+
+	dev_dbg(&(applespi)->spi->dev,
+		"SPI settings: spi_cs_delay=%llu reset_a2r_usec=%llu reset_rec_usec=%llu\n",
+		settings->spi_cs_delay, settings->reset_a2r_usec,
+		settings->reset_rec_usec);
+
+	return 0;
+}
+
+static int applespi_setup_spi(struct applespi_data *applespi)
+{
+	int sts;
+
+	sts = applespi_get_spi_settings(applespi);
+	if (sts)
+		return sts;
+
+	spin_lock_init(&applespi->cmd_msg_lock);
+	init_waitqueue_head(&applespi->drain_complete);
+
+	return 0;
+}
+
+static int applespi_enable_spi(struct applespi_data *applespi)
+{
+	acpi_status acpi_sts;
+	unsigned long long spi_status;
+
+	/* check if SPI is already enabled, so we can skip the delay below */
+	acpi_sts = acpi_evaluate_integer(applespi->sist, NULL, NULL,
+					 &spi_status);
+	if (ACPI_SUCCESS(acpi_sts) && spi_status)
+		return 0;
+
+	/* SIEN(1) will enable SPI communication */
+	acpi_sts = acpi_execute_simple_method(applespi->sien, NULL, 1);
+	if (ACPI_FAILURE(acpi_sts)) {
+		dev_err(&(applespi)->spi->dev, "SIEN failed: %s\n",
+			acpi_format_exception(acpi_sts));
+		return -ENODEV;
+	}
+
+	/*
+	 * Allow the SPI interface to come up before returning. Without this
+	 * delay, the SPI commands to enable multitouch mode may not reach
+	 * the trackpad controller, causing pointer movement to break upon
+	 * resume from sleep.
+	 */
+	msleep(50);
+
+	return 0;
+}
+
+static int applespi_send_cmd_msg(struct applespi_data *applespi);
+
+static void applespi_msg_complete(struct applespi_data *applespi,
+				  bool is_write_msg, bool is_read_compl)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
+
+	if (is_read_compl)
+		applespi->read_active = false;
+	if (is_write_msg)
+		applespi->write_active = false;
+
+	if (applespi->drain && !applespi->write_active)
+		wake_up_all(&applespi->drain_complete);
+
+	if (is_write_msg) {
+		applespi->cmd_msg_queued = false;
+		applespi_send_cmd_msg(applespi);
+	}
+
+	spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
+}
+
+static void applespi_async_write_complete(void *context)
+{
+	struct applespi_data *applespi = context;
+	enum applespi_evt_type evt_type = applespi->cmd_evt_type;
+
+	applespi_get_trace_fun(evt_type)(evt_type, PT_WRITE,
+					 applespi->tx_buffer,
+					 APPLESPI_PACKET_SIZE);
+	applespi_get_trace_fun(evt_type)(evt_type, PT_STATUS,
+					 applespi->tx_status,
+					 APPLESPI_STATUS_SIZE);
+
+	if (!applespi_check_write_status(applespi, applespi->wr_m.status)) {
+		/*
+		 * If we got an error, we presumably won't get the expected
+		 * response message either.
+		 */
+		applespi_msg_complete(applespi, true, false);
+	}
+}
+
+static int applespi_send_cmd_msg(struct applespi_data *applespi)
+{
+	u16 crc;
+	int sts;
+	struct spi_packet *packet = (struct spi_packet *)applespi->tx_buffer;
+	struct message *message = (struct message *)packet->data;
+	u16 msg_len;
+	u8 device;
+
+	/* check if draining */
+	if (applespi->drain)
+		return 0;
+
+	/* check whether send is in progress */
+	if (applespi->cmd_msg_queued)
+		return 0;
+
+	/* set up packet */
+	memset(packet, 0, APPLESPI_PACKET_SIZE);
+
+	/* are we processing init commands? */
+	if (applespi->want_tp_info_cmd) {
+		applespi->want_tp_info_cmd = false;
+		applespi->want_mt_init_cmd = true;
+		applespi->cmd_evt_type = ET_CMD_TP_INI;
+
+		/* build init command */
+		device = PACKET_DEV_INFO;
+
+		message->type = cpu_to_le16(0x1020);
+		msg_len = sizeof(message->tp_info_command);
+
+		message->zero = 0x02;
+		message->rsp_buf_len = cpu_to_le16(0x0200);
+
+	} else if (applespi->want_mt_init_cmd) {
+		applespi->want_mt_init_cmd = false;
+		applespi->cmd_evt_type = ET_CMD_TP_INI;
+
+		/* build init command */
+		device = PACKET_DEV_TPAD;
+
+		message->type = cpu_to_le16(0x0252);
+		msg_len = sizeof(message->init_mt_command);
+
+		message->init_mt_command.cmd = cpu_to_le16(0x0102);
+
+	/* do we need caps-lock command? */
+	} else if (applespi->want_cl_led_on != applespi->have_cl_led_on) {
+		applespi->have_cl_led_on = applespi->want_cl_led_on;
+		applespi->cmd_evt_type = ET_CMD_CL;
+
+		/* build led command */
+		device = PACKET_DEV_KEYB;
+
+		message->type = cpu_to_le16(0x0151);
+		msg_len = sizeof(message->capsl_command);
+
+		message->capsl_command.unknown = 0x01;
+		message->capsl_command.led = applespi->have_cl_led_on ? 2 : 0;
+
+	/* do we need backlight command? */
+	} else if (applespi->want_bl_level != applespi->have_bl_level) {
+		applespi->have_bl_level = applespi->want_bl_level;
+		applespi->cmd_evt_type = ET_CMD_BL;
+
+		/* build command buffer */
+		device = PACKET_DEV_KEYB;
+
+		message->type = cpu_to_le16(0xB051);
+		msg_len = sizeof(message->bl_command);
+
+		message->bl_command.const1 = cpu_to_le16(0x01B0);
+		message->bl_command.level =
+				cpu_to_le16(applespi->have_bl_level);
+
+		if (applespi->have_bl_level > 0)
+			message->bl_command.const2 = cpu_to_le16(0x01F4);
+		else
+			message->bl_command.const2 = cpu_to_le16(0x0001);
+
+	/* everything's up-to-date */
+	} else {
+		return 0;
+	}
+
+	/* finalize packet */
+	packet->flags = PACKET_TYPE_WRITE;
+	packet->device = device;
+	packet->length = cpu_to_le16(MSG_HEADER_SIZE + msg_len);
+
+	message->counter = applespi->cmd_msg_cntr++ % (U8_MAX + 1);
+
+	message->length = cpu_to_le16(msg_len - 2);
+	if (!message->rsp_buf_len)
+		message->rsp_buf_len = message->length;
+
+	crc = crc16(0, (u8 *)message, le16_to_cpu(packet->length) - 2);
+	put_unaligned_le16(crc, &message->data[msg_len - 2]);
+
+	crc = crc16(0, (u8 *)packet, sizeof(*packet) - 2);
+	packet->crc16 = cpu_to_le16(crc);
+
+	/* send command */
+	sts = applespi_async(applespi, &applespi->wr_m,
+			     applespi_async_write_complete);
+	if (sts) {
+		dev_warn(&(applespi)->spi->dev,
+			 "Error queueing async write to device: %d\n", sts);
+		return sts;
+	}
+
+	applespi->cmd_msg_queued = true;
+	applespi->write_active = true;
+
+	return 0;
+}
+
+static void applespi_init(struct applespi_data *applespi, bool is_resume)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
+
+	if (is_resume)
+		applespi->want_mt_init_cmd = true;
+	else
+		applespi->want_tp_info_cmd = true;
+	applespi_send_cmd_msg(applespi);
+
+	spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
+}
+
+static int applespi_set_capsl_led(struct applespi_data *applespi,
+				  bool capslock_on)
+{
+	unsigned long flags;
+	int sts;
+
+	spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
+
+	applespi->want_cl_led_on = capslock_on;
+	sts = applespi_send_cmd_msg(applespi);
+
+	spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
+
+	return sts;
+}
+
+static void applespi_set_bl_level(struct led_classdev *led_cdev,
+				  enum led_brightness value)
+{
+	struct applespi_data *applespi =
+		container_of(led_cdev, struct applespi_data, backlight_info);
+	unsigned long flags;
+	int sts;
+
+	spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
+
+	if (value == 0) {
+		applespi->want_bl_level = value;
+	} else {
+		/*
+		 * The backlight does not turn on till level 32, so we scale
+		 * the range here so that from a user's perspective it turns
+		 * on at 1.
+		 */
+		applespi->want_bl_level =
+			((value * KBD_BL_LEVEL_ADJ) / KBD_BL_LEVEL_SCALE +
+			 KBD_BL_LEVEL_MIN);
+	}
+
+	sts = applespi_send_cmd_msg(applespi);
+
+	spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
+}
+
+static int applespi_event(struct input_dev *dev, unsigned int type,
+			  unsigned int code, int value)
+{
+	struct applespi_data *applespi = input_get_drvdata(dev);
+
+	switch (type) {
+	case EV_LED:
+		applespi_set_capsl_led(applespi, !!test_bit(LED_CAPSL, dev->led));
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+/* lifted from the BCM5974 driver and renamed from raw2int */
+/* convert 16-bit little endian to signed integer */
+static inline int le16_to_int(__le16 x)
+{
+	return (signed short)le16_to_cpu(x);
+}
+
+static void applespi_debug_update_dimensions(struct applespi_data *applespi,
+					     const struct tp_finger *f)
+{
+	#define UPDATE_DIMENSIONS(val, op, last) \
+		do { \
+			if (le16_to_int(val) op last) \
+				last = le16_to_int(val); \
+		} while (0)
+
+	UPDATE_DIMENSIONS(f->abs_x, <, applespi->tp_dim_min_x);
+	UPDATE_DIMENSIONS(f->abs_x, >, applespi->tp_dim_max_x);
+	UPDATE_DIMENSIONS(f->abs_y, <, applespi->tp_dim_min_y);
+	UPDATE_DIMENSIONS(f->abs_y, >, applespi->tp_dim_max_y);
+
+	#undef UPDATE_DIMENSIONS
+}
+
+static int applespi_tp_dim_open(struct inode *inode, struct file *file)
+{
+	struct applespi_data *applespi = inode->i_private;
+
+	file->private_data = applespi;
+
+	snprintf(applespi->tp_dim_val, sizeof(applespi->tp_dim_val),
+		 "0x%.4x %dx%d+%u+%u\n",
+		 applespi->touchpad_input_dev->id.product,
+		 applespi->tp_dim_min_x, applespi->tp_dim_min_y,
+		 applespi->tp_dim_max_x - applespi->tp_dim_min_x,
+		 applespi->tp_dim_max_y - applespi->tp_dim_min_y);
+
+	return nonseekable_open(inode, file);
+}
+
+static ssize_t applespi_tp_dim_read(struct file *file, char __user *buf,
+				    size_t len, loff_t *off)
+{
+	struct applespi_data *applespi = file->private_data;
+
+	return simple_read_from_buffer(buf, len, off, applespi->tp_dim_val,
+				       strlen(applespi->tp_dim_val));
+}
+
+static const struct file_operations applespi_tp_dim_fops = {
+	.owner = THIS_MODULE,
+	.open = applespi_tp_dim_open,
+	.read = applespi_tp_dim_read,
+	.llseek = no_llseek,
+};
+
+static void report_finger_data(struct input_dev *input, int slot,
+			       const struct input_mt_pos *pos,
+			       const struct tp_finger *f)
+{
+	input_mt_slot(input, slot);
+	input_mt_report_slot_state(input, MT_TOOL_FINGER, true);
+
+	input_report_abs(input, ABS_MT_TOUCH_MAJOR,
+			 le16_to_int(f->touch_major) << 1);
+	input_report_abs(input, ABS_MT_TOUCH_MINOR,
+			 le16_to_int(f->touch_minor) << 1);
+	input_report_abs(input, ABS_MT_WIDTH_MAJOR,
+			 le16_to_int(f->tool_major) << 1);
+	input_report_abs(input, ABS_MT_WIDTH_MINOR,
+			 le16_to_int(f->tool_minor) << 1);
+	input_report_abs(input, ABS_MT_ORIENTATION,
+			 MAX_FINGER_ORIENTATION - le16_to_int(f->orientation));
+	input_report_abs(input, ABS_MT_POSITION_X, pos->x);
+	input_report_abs(input, ABS_MT_POSITION_Y, pos->y);
+}
+
+static void report_tp_state(struct applespi_data *applespi,
+			    struct touchpad_protocol *t)
+{
+	const struct tp_finger *f;
+	struct input_dev *input;
+	const struct applespi_tp_info *tp_info = &applespi->tp_info;
+	int i, n;
+
+	/* touchpad_input_dev is set async in worker */
+	input = smp_load_acquire(&applespi->touchpad_input_dev);
+	if (!input)
+		return;	/* touchpad isn't initialized yet */
+
+	n = 0;
+
+	for (i = 0; i < t->number_of_fingers; i++) {
+		f = &t->fingers[i];
+		if (le16_to_int(f->touch_major) == 0)
+			continue;
+		applespi->pos[n].x = le16_to_int(f->abs_x);
+		applespi->pos[n].y = tp_info->y_min + tp_info->y_max -
+				     le16_to_int(f->abs_y);
+		n++;
+
+		if (applespi->debug_tp_dim)
+			applespi_debug_update_dimensions(applespi, f);
+	}
+
+	input_mt_assign_slots(input, applespi->slots, applespi->pos, n, 0);
+
+	for (i = 0; i < n; i++)
+		report_finger_data(input, applespi->slots[i],
+				   &applespi->pos[i], &t->fingers[i]);
+
+	input_mt_sync_frame(input);
+	input_report_key(input, BTN_LEFT, t->clicked);
+
+	input_sync(input);
+}
+
+static const struct applespi_key_translation *
+applespi_find_translation(const struct applespi_key_translation *table, u16 key)
+{
+	const struct applespi_key_translation *trans;
+
+	for (trans = table; trans->from; trans++)
+		if (trans->from == key)
+			return trans;
+
+	return NULL;
+}
+
+static unsigned int applespi_translate_fn_key(unsigned int key, int fn_pressed)
+{
+	const struct applespi_key_translation *trans;
+	int do_translate;
+
+	trans = applespi_find_translation(applespi_fn_codes, key);
+	if (trans) {
+		if (trans->flags & APPLE_FLAG_FKEY)
+			do_translate = (fnmode == 2 && fn_pressed) ||
+				       (fnmode == 1 && !fn_pressed);
+		else
+			do_translate = fn_pressed;
+
+		if (do_translate)
+			key = trans->to;
+	}
+
+	return key;
+}
+
+static unsigned int applespi_translate_iso_layout(unsigned int key)
+{
+	const struct applespi_key_translation *trans;
+
+	trans = applespi_find_translation(apple_iso_keyboard, key);
+	if (trans)
+		key = trans->to;
+
+	return key;
+}
+
+static unsigned int applespi_code_to_key(u8 code, int fn_pressed)
+{
+	unsigned int key = applespi_scancodes[code];
+
+	if (fnmode)
+		key = applespi_translate_fn_key(key, fn_pressed);
+	if (iso_layout)
+		key = applespi_translate_iso_layout(key);
+	return key;
+}
+
+static void
+applespi_remap_fn_key(struct keyboard_protocol *keyboard_protocol)
+{
+	unsigned char tmp;
+
+	if (!fnremap || fnremap > ARRAY_SIZE(applespi_controlcodes) ||
+	    !applespi_controlcodes[fnremap - 1])
+		return;
+
+	tmp = keyboard_protocol->fn_pressed;
+	keyboard_protocol->fn_pressed =
+			!!(keyboard_protocol->modifiers & BIT(fnremap - 1));
+	if (tmp)
+		keyboard_protocol->modifiers |= BIT(fnremap - 1);
+	else
+		keyboard_protocol->modifiers &= ~BIT(fnremap - 1);
+}
+
+static void
+applespi_handle_keyboard_event(struct applespi_data *applespi,
+			       struct keyboard_protocol *keyboard_protocol)
+{
+	int i, j;
+	unsigned int key;
+	bool still_pressed;
+	bool is_overflow;
+
+	compiletime_assert(ARRAY_SIZE(applespi_controlcodes) ==
+			   sizeof_field(struct keyboard_protocol, modifiers) * 8,
+			   "applespi_controlcodes has wrong number of entries");
+
+	/* check for rollover overflow, which is signalled by all keys == 1 */
+	is_overflow = true;
+
+	for (i = 0; i < MAX_ROLLOVER; i++) {
+		if (keyboard_protocol->keys_pressed[i] != 1) {
+			is_overflow = false;
+			break;
+		}
+	}
+
+	if (is_overflow)
+		return;
+
+	/* remap fn key if desired */
+	applespi_remap_fn_key(keyboard_protocol);
+
+	/* check released keys */
+	for (i = 0; i < MAX_ROLLOVER; i++) {
+		still_pressed = false;
+		for (j = 0; j < MAX_ROLLOVER; j++) {
+			if (applespi->last_keys_pressed[i] ==
+			    keyboard_protocol->keys_pressed[j]) {
+				still_pressed = true;
+				break;
+			}
+		}
+
+		if (still_pressed)
+			continue;
+
+		key = applespi_code_to_key(applespi->last_keys_pressed[i],
+					   applespi->last_keys_fn_pressed[i]);
+		input_report_key(applespi->keyboard_input_dev, key, 0);
+		applespi->last_keys_fn_pressed[i] = 0;
+	}
+
+	/* check pressed keys */
+	for (i = 0; i < MAX_ROLLOVER; i++) {
+		if (keyboard_protocol->keys_pressed[i] <
+				ARRAY_SIZE(applespi_scancodes) &&
+		    keyboard_protocol->keys_pressed[i] > 0) {
+			key = applespi_code_to_key(
+					keyboard_protocol->keys_pressed[i],
+					keyboard_protocol->fn_pressed);
+			input_report_key(applespi->keyboard_input_dev, key, 1);
+			applespi->last_keys_fn_pressed[i] =
+					keyboard_protocol->fn_pressed;
+		}
+	}
+
+	/* check control keys */
+	for (i = 0; i < ARRAY_SIZE(applespi_controlcodes); i++) {
+		if (keyboard_protocol->modifiers & BIT(i))
+			input_report_key(applespi->keyboard_input_dev,
+					 applespi_controlcodes[i], 1);
+		else
+			input_report_key(applespi->keyboard_input_dev,
+					 applespi_controlcodes[i], 0);
+	}
+
+	/* check function key */
+	if (keyboard_protocol->fn_pressed && !applespi->last_fn_pressed)
+		input_report_key(applespi->keyboard_input_dev, KEY_FN, 1);
+	else if (!keyboard_protocol->fn_pressed && applespi->last_fn_pressed)
+		input_report_key(applespi->keyboard_input_dev, KEY_FN, 0);
+	applespi->last_fn_pressed = keyboard_protocol->fn_pressed;
+
+	/* done */
+	input_sync(applespi->keyboard_input_dev);
+	memcpy(&applespi->last_keys_pressed, keyboard_protocol->keys_pressed,
+	       sizeof(applespi->last_keys_pressed));
+}
+
+static const struct applespi_tp_info *applespi_find_touchpad_info(__u8 model)
+{
+	const struct applespi_tp_model_info *info;
+
+	for (info = applespi_tp_models; info->model; info++) {
+		if (info->model == model)
+			return &info->tp_info;
+	}
+
+	return NULL;
+}
+
+static int
+applespi_register_touchpad_device(struct applespi_data *applespi,
+				  struct touchpad_info_protocol *rcvd_tp_info)
+{
+	const struct applespi_tp_info *tp_info;
+	struct input_dev *touchpad_input_dev;
+	int sts;
+
+	/* set up touchpad dimensions */
+	tp_info = applespi_find_touchpad_info(rcvd_tp_info->model_no);
+	if (!tp_info) {
+		dev_warn(&(applespi)->spi->dev,
+			 "Unknown touchpad model %x - falling back to MB8 touchpad\n",
+			 rcvd_tp_info->model_no);
+		tp_info = &applespi_tp_models[0].tp_info;
+	}
+
+	applespi->tp_info = *tp_info;
+
+	if (touchpad_dimensions[0]) {
+		int x, y, w, h;
+
+		sts = sscanf(touchpad_dimensions, "%dx%d+%u+%u", &x, &y, &w, &h);
+		if (sts == 4) {
+			dev_info(&(applespi)->spi->dev,
+				 "Overriding touchpad dimensions from module param\n");
+			applespi->tp_info.x_min = x;
+			applespi->tp_info.y_min = y;
+			applespi->tp_info.x_max = x + w;
+			applespi->tp_info.y_max = y + h;
+		} else {
+			dev_warn(&(applespi)->spi->dev,
+				 "Invalid touchpad dimensions '%s': must be in the form XxY+W+H\n",
+				 touchpad_dimensions);
+			touchpad_dimensions[0] = '\0';
+		}
+	}
+	if (!touchpad_dimensions[0]) {
+		snprintf(touchpad_dimensions, sizeof(touchpad_dimensions),
+			 "%dx%d+%u+%u",
+			 applespi->tp_info.x_min,
+			 applespi->tp_info.y_min,
+			 applespi->tp_info.x_max - applespi->tp_info.x_min,
+			 applespi->tp_info.y_max - applespi->tp_info.y_min);
+	}
+
+	/* create touchpad input device */
+	touchpad_input_dev = devm_input_allocate_device(&(applespi)->spi->dev);
+	if (!touchpad_input_dev) {
+		dev_err(&(applespi)->spi->dev,
+			"Failed to allocate touchpad input device\n");
+		return -ENOMEM;
+	}
+
+	touchpad_input_dev->name = "Apple SPI Touchpad";
+	touchpad_input_dev->phys = "applespi/input1";
+	touchpad_input_dev->dev.parent = &(applespi)->spi->dev;
+	touchpad_input_dev->id.bustype = BUS_SPI;
+	touchpad_input_dev->id.vendor = SYNAPTICS_VENDOR_ID;
+	touchpad_input_dev->id.product =
+			rcvd_tp_info->model_no << 8 | rcvd_tp_info->model_flags;
+
+	/* basic properties */
+	input_set_capability(touchpad_input_dev, EV_REL, REL_X);
+	input_set_capability(touchpad_input_dev, EV_REL, REL_Y);
+
+	__set_bit(INPUT_PROP_POINTER, touchpad_input_dev->propbit);
+	__set_bit(INPUT_PROP_BUTTONPAD, touchpad_input_dev->propbit);
+
+	/* finger touch area */
+	input_set_abs_params(touchpad_input_dev, ABS_MT_TOUCH_MAJOR,
+			     0, 5000, 0, 0);
+	input_set_abs_params(touchpad_input_dev, ABS_MT_TOUCH_MINOR,
+			     0, 5000, 0, 0);
+
+	/* finger approach area */
+	input_set_abs_params(touchpad_input_dev, ABS_MT_WIDTH_MAJOR,
+			     0, 5000, 0, 0);
+	input_set_abs_params(touchpad_input_dev, ABS_MT_WIDTH_MINOR,
+			     0, 5000, 0, 0);
+
+	/* finger orientation */
+	input_set_abs_params(touchpad_input_dev, ABS_MT_ORIENTATION,
+			     -MAX_FINGER_ORIENTATION, MAX_FINGER_ORIENTATION,
+			     0, 0);
+
+	/* finger position */
+	input_set_abs_params(touchpad_input_dev, ABS_MT_POSITION_X,
+			     applespi->tp_info.x_min, applespi->tp_info.x_max,
+			     0, 0);
+	input_set_abs_params(touchpad_input_dev, ABS_MT_POSITION_Y,
+			     applespi->tp_info.y_min, applespi->tp_info.y_max,
+			     0, 0);
+
+	/* touchpad button */
+	input_set_capability(touchpad_input_dev, EV_KEY, BTN_LEFT);
+
+	/* multitouch */
+	input_mt_init_slots(touchpad_input_dev, MAX_FINGERS,
+			    INPUT_MT_POINTER | INPUT_MT_DROP_UNUSED |
+			    INPUT_MT_TRACK);
+
+	/* register input device */
+	sts = input_register_device(touchpad_input_dev);
+	if (sts) {
+		dev_err(&(applespi)->spi->dev,
+			"Unable to register touchpad input device (%d)\n", sts);
+		return sts;
+	}
+
+	/* touchpad_input_dev is read async in spi callback */
+	smp_store_release(&applespi->touchpad_input_dev, touchpad_input_dev);
+
+	return 0;
+}
+
+static void applespi_worker(struct work_struct *work)
+{
+	struct applespi_data *applespi =
+		container_of(work, struct applespi_data, work);
+
+	applespi_register_touchpad_device(applespi, &applespi->rcvd_tp_info);
+}
+
+static void applespi_handle_cmd_response(struct applespi_data *applespi,
+					 struct spi_packet *packet,
+					 struct message *message)
+{
+	if (packet->device == PACKET_DEV_INFO &&
+	    le16_to_cpu(message->type) == 0x1020) {
+		/*
+		 * We're not allowed to sleep here, but registering an input
+		 * device can sleep.
+		 */
+		applespi->rcvd_tp_info = message->tp_info;
+		schedule_work(&applespi->work);
+		return;
+	}
+
+	if (le16_to_cpu(message->length) != 0x0000) {
+		dev_warn_ratelimited(&(applespi)->spi->dev,
+				     "Received unexpected write response: length=%x\n",
+				     le16_to_cpu(message->length));
+		return;
+	}
+
+	if (packet->device == PACKET_DEV_TPAD &&
+	    le16_to_cpu(message->type) == 0x0252 &&
+	    le16_to_cpu(message->rsp_buf_len) == 0x0002)
+		dev_info(&(applespi)->spi->dev, "modeswitch done.\n");
+}
+
+static bool applespi_verify_crc(struct applespi_data *applespi, u8 *buffer,
+				size_t buflen)
+{
+	u16 crc;
+
+	crc = crc16(0, buffer, buflen);
+	if (crc) {
+		dev_warn_ratelimited(&(applespi)->spi->dev,
+				     "Received corrupted packet (crc mismatch)\n");
+		trace_applespi_bad_crc(ET_RD_CRC, READ, buffer, buflen);
+
+		return false;
+	}
+
+	return true;
+}
+
+static void applespi_debug_print_read_packet(struct applespi_data *applespi,
+					     struct spi_packet *packet)
+{
+	unsigned int evt_type;
+
+	if (packet->flags == PACKET_TYPE_READ &&
+	    packet->device == PACKET_DEV_KEYB)
+		evt_type = ET_RD_KEYB;
+	else if (packet->flags == PACKET_TYPE_READ &&
+		 packet->device == PACKET_DEV_TPAD)
+		evt_type = ET_RD_TPAD;
+	else if (packet->flags == PACKET_TYPE_WRITE)
+		evt_type = applespi->cmd_evt_type;
+	else
+		evt_type = ET_RD_UNKN;
+
+	applespi_get_trace_fun(evt_type)(evt_type, PT_READ, applespi->rx_buffer,
+					 APPLESPI_PACKET_SIZE);
+}
+
+static void applespi_got_data(struct applespi_data *applespi)
+{
+	struct spi_packet *packet;
+	struct message *message;
+	unsigned int msg_len;
+	unsigned int off;
+	unsigned int rem;
+	unsigned int len;
+
+	/* process packet header */
+	if (!applespi_verify_crc(applespi, applespi->rx_buffer,
+				 APPLESPI_PACKET_SIZE)) {
+		unsigned long flags;
+
+		spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
+
+		if (applespi->drain) {
+			applespi->read_active = false;
+			applespi->write_active = false;
+
+			wake_up_all(&applespi->drain_complete);
+		}
+
+		spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
+
+		return;
+	}
+
+	packet = (struct spi_packet *)applespi->rx_buffer;
+
+	applespi_debug_print_read_packet(applespi, packet);
+
+	off = le16_to_cpu(packet->offset);
+	rem = le16_to_cpu(packet->remaining);
+	len = le16_to_cpu(packet->length);
+
+	if (len > sizeof(packet->data)) {
+		dev_warn_ratelimited(&(applespi)->spi->dev,
+				     "Received corrupted packet (invalid packet length %u)\n",
+				     len);
+		goto msg_complete;
+	}
+
+	/* handle multi-packet messages */
+	if (rem > 0 || off > 0) {
+		if (off != applespi->saved_msg_len) {
+			dev_warn_ratelimited(&(applespi)->spi->dev,
+					     "Received unexpected offset (got %u, expected %u)\n",
+					     off, applespi->saved_msg_len);
+			goto msg_complete;
+		}
+
+		if (off + rem > MAX_PKTS_PER_MSG * APPLESPI_PACKET_SIZE) {
+			dev_warn_ratelimited(&(applespi)->spi->dev,
+					     "Received message too large (size %u)\n",
+					     off + rem);
+			goto msg_complete;
+		}
+
+		if (off + len > MAX_PKTS_PER_MSG * APPLESPI_PACKET_SIZE) {
+			dev_warn_ratelimited(&(applespi)->spi->dev,
+					     "Received message too large (size %u)\n",
+					     off + len);
+			goto msg_complete;
+		}
+
+		memcpy(applespi->msg_buf + off, &packet->data, len);
+		applespi->saved_msg_len += len;
+
+		if (rem > 0)
+			return;
+
+		message = (struct message *)applespi->msg_buf;
+		msg_len = applespi->saved_msg_len;
+	} else {
+		message = (struct message *)&packet->data;
+		msg_len = len;
+	}
+
+	/* got complete message - verify */
+	if (!applespi_verify_crc(applespi, (u8 *)message, msg_len))
+		goto msg_complete;
+
+	if (le16_to_cpu(message->length) != msg_len - MSG_HEADER_SIZE - 2) {
+		dev_warn_ratelimited(&(applespi)->spi->dev,
+				     "Received corrupted packet (invalid message length %u - expected %u)\n",
+				     le16_to_cpu(message->length),
+				     msg_len - MSG_HEADER_SIZE - 2);
+		goto msg_complete;
+	}
+
+	/* handle message */
+	if (packet->flags == PACKET_TYPE_READ &&
+	    packet->device == PACKET_DEV_KEYB) {
+		applespi_handle_keyboard_event(applespi, &message->keyboard);
+
+	} else if (packet->flags == PACKET_TYPE_READ &&
+		   packet->device == PACKET_DEV_TPAD) {
+		struct touchpad_protocol *tp;
+		size_t tp_len;
+
+		tp = &message->touchpad;
+		tp_len = sizeof(*tp) +
+			 tp->number_of_fingers * sizeof(tp->fingers[0]);
+
+		if (le16_to_cpu(message->length) + 2 != tp_len) {
+			dev_warn_ratelimited(&(applespi)->spi->dev,
+					     "Received corrupted packet (invalid message length %u - num-fingers %u, tp-len %zu)\n",
+					     le16_to_cpu(message->length),
+					     tp->number_of_fingers, tp_len);
+			goto msg_complete;
+		}
+
+		if (tp->number_of_fingers > MAX_FINGERS) {
+			dev_warn_ratelimited(&(applespi)->spi->dev,
+					     "Number of reported fingers (%u) exceeds max (%u))\n",
+					     tp->number_of_fingers,
+					     MAX_FINGERS);
+			tp->number_of_fingers = MAX_FINGERS;
+		}
+
+		report_tp_state(applespi, tp);
+
+	} else if (packet->flags == PACKET_TYPE_WRITE) {
+		applespi_handle_cmd_response(applespi, packet, message);
+	}
+
+msg_complete:
+	applespi->saved_msg_len = 0;
+
+	applespi_msg_complete(applespi, packet->flags == PACKET_TYPE_WRITE,
+			      true);
+}
+
+static void applespi_async_read_complete(void *context)
+{
+	struct applespi_data *applespi = context;
+
+	if (applespi->rd_m.status < 0) {
+		dev_warn(&(applespi)->spi->dev,
+			 "Error reading from device: %d\n",
+			 applespi->rd_m.status);
+		/*
+		 * We don't actually know if this was a pure read, or a response
+		 * to a write. But this is a rare error condition that should
+		 * never occur, so clearing both flags to avoid deadlock.
+		 */
+		applespi_msg_complete(applespi, true, true);
+	} else {
+		applespi_got_data(applespi);
+	}
+
+	acpi_finish_gpe(NULL, applespi->gpe);
+}
+
+static u32 applespi_notify(acpi_handle gpe_device, u32 gpe, void *context)
+{
+	struct applespi_data *applespi = context;
+	int sts;
+	unsigned long flags;
+
+	trace_applespi_irq_received(ET_RD_IRQ, PT_READ);
+
+	spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
+
+	if (!applespi->suspended) {
+		sts = applespi_async(applespi, &applespi->rd_m,
+				     applespi_async_read_complete);
+		if (sts)
+			dev_warn(&(applespi)->spi->dev,
+				 "Error queueing async read to device: %d\n",
+				 sts);
+		else
+			applespi->read_active = true;
+	}
+
+	spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
+
+	return ACPI_INTERRUPT_HANDLED;
+}
+
+static int applespi_get_saved_bl_level(struct applespi_data *applespi)
+{
+	struct efivar_entry *efivar_entry;
+	u16 efi_data = 0;
+	unsigned long efi_data_len;
+	int sts;
+
+	efivar_entry = kmalloc(sizeof(*efivar_entry), GFP_KERNEL);
+	if (!efivar_entry)
+		return -ENOMEM;
+
+	memcpy(efivar_entry->var.VariableName, EFI_BL_LEVEL_NAME,
+	       sizeof(EFI_BL_LEVEL_NAME));
+	efivar_entry->var.VendorGuid = EFI_BL_LEVEL_GUID;
+	efi_data_len = sizeof(efi_data);
+
+	sts = efivar_entry_get(efivar_entry, NULL, &efi_data_len, &efi_data);
+	if (sts && sts != -ENOENT)
+		dev_warn(&(applespi)->spi->dev,
+			 "Error getting backlight level from EFI vars: %d\n",
+			 sts);
+
+	kfree(efivar_entry);
+
+	return sts ? sts : efi_data;
+}
+
+static void applespi_save_bl_level(struct applespi_data *applespi,
+				   unsigned int level)
+{
+	efi_guid_t efi_guid;
+	u32 efi_attr;
+	unsigned long efi_data_len;
+	u16 efi_data;
+	int sts;
+
+	/* Save keyboard backlight level */
+	efi_guid = EFI_BL_LEVEL_GUID;
+	efi_data = (u16)level;
+	efi_data_len = sizeof(efi_data);
+	efi_attr = EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS |
+		   EFI_VARIABLE_RUNTIME_ACCESS;
+
+	sts = efivar_entry_set_safe(EFI_BL_LEVEL_NAME, efi_guid, efi_attr, true,
+				    efi_data_len, &efi_data);
+	if (sts)
+		dev_warn(&(applespi)->spi->dev,
+			 "Error saving backlight level to EFI vars: %d\n", sts);
+}
+
+static int applespi_probe(struct spi_device *spi)
+{
+	struct applespi_data *applespi;
+	acpi_status acpi_sts;
+	int sts, i;
+	unsigned long long gpe, usb_status;
+
+	/* check if the USB interface is present and enabled already */
+	acpi_sts = acpi_evaluate_integer(ACPI_HANDLE(&spi->dev), "UIST", NULL,
+					 &usb_status);
+	if (ACPI_SUCCESS(acpi_sts) && usb_status) {
+		/* let the USB driver take over instead */
+		dev_info(&spi->dev, "USB interface already enabled\n");
+		return -ENODEV;
+	}
+
+	/* allocate driver data */
+	applespi = devm_kzalloc(&spi->dev, sizeof(*applespi), GFP_KERNEL);
+	if (!applespi)
+		return -ENOMEM;
+
+	applespi->spi = spi;
+	applespi->handle = ACPI_HANDLE(&spi->dev);
+
+	INIT_WORK(&applespi->work, applespi_worker);
+
+	/* store the driver data */
+	spi_set_drvdata(spi, applespi);
+
+	/* create our buffers */
+	applespi->tx_buffer = devm_kmalloc(&spi->dev, APPLESPI_PACKET_SIZE,
+					   GFP_KERNEL);
+	applespi->tx_status = devm_kmalloc(&spi->dev, APPLESPI_STATUS_SIZE,
+					   GFP_KERNEL);
+	applespi->rx_buffer = devm_kmalloc(&spi->dev, APPLESPI_PACKET_SIZE,
+					   GFP_KERNEL);
+	applespi->msg_buf = devm_kmalloc_array(&spi->dev, MAX_PKTS_PER_MSG,
+					       APPLESPI_PACKET_SIZE,
+					       GFP_KERNEL);
+
+	if (!applespi->tx_buffer || !applespi->tx_status ||
+	    !applespi->rx_buffer || !applespi->msg_buf)
+		return -ENOMEM;
+
+	/* set up our spi messages */
+	applespi_setup_read_txfrs(applespi);
+	applespi_setup_write_txfrs(applespi);
+
+	/* cache ACPI method handles */
+	if (ACPI_FAILURE(acpi_get_handle(applespi->handle, "SIEN",
+					 &applespi->sien)) ||
+	    ACPI_FAILURE(acpi_get_handle(applespi->handle, "SIST",
+					 &applespi->sist))) {
+		dev_err(&(applespi)->spi->dev,
+			"Failed to get required ACPI method handles\n");
+		return -ENODEV;
+	}
+
+	/* switch on the SPI interface */
+	sts = applespi_setup_spi(applespi);
+	if (sts)
+		return sts;
+
+	sts = applespi_enable_spi(applespi);
+	if (sts)
+		return sts;
+
+	/* setup the keyboard input dev */
+	applespi->keyboard_input_dev = devm_input_allocate_device(&spi->dev);
+
+	if (!applespi->keyboard_input_dev)
+		return -ENOMEM;
+
+	applespi->keyboard_input_dev->name = "Apple SPI Keyboard";
+	applespi->keyboard_input_dev->phys = "applespi/input0";
+	applespi->keyboard_input_dev->dev.parent = &spi->dev;
+	applespi->keyboard_input_dev->id.bustype = BUS_SPI;
+
+	applespi->keyboard_input_dev->evbit[0] =
+			BIT_MASK(EV_KEY) | BIT_MASK(EV_LED) | BIT_MASK(EV_REP);
+	applespi->keyboard_input_dev->ledbit[0] = BIT_MASK(LED_CAPSL);
+
+	input_set_drvdata(applespi->keyboard_input_dev, applespi);
+	applespi->keyboard_input_dev->event = applespi_event;
+
+	for (i = 0; i < ARRAY_SIZE(applespi_scancodes); i++)
+		if (applespi_scancodes[i])
+			input_set_capability(applespi->keyboard_input_dev,
+					     EV_KEY, applespi_scancodes[i]);
+
+	for (i = 0; i < ARRAY_SIZE(applespi_controlcodes); i++)
+		if (applespi_controlcodes[i])
+			input_set_capability(applespi->keyboard_input_dev,
+					     EV_KEY, applespi_controlcodes[i]);
+
+	for (i = 0; i < ARRAY_SIZE(applespi_fn_codes); i++)
+		if (applespi_fn_codes[i].to)
+			input_set_capability(applespi->keyboard_input_dev,
+					     EV_KEY, applespi_fn_codes[i].to);
+
+	input_set_capability(applespi->keyboard_input_dev, EV_KEY, KEY_FN);
+
+	sts = input_register_device(applespi->keyboard_input_dev);
+	if (sts) {
+		dev_err(&(applespi)->spi->dev,
+			"Unable to register keyboard input device (%d)\n", sts);
+		return -ENODEV;
+	}
+
+	/*
+	 * The applespi device doesn't send interrupts normally (as is described
+	 * in its DSDT), but rather seems to use ACPI GPEs.
+	 */
+	acpi_sts = acpi_evaluate_integer(applespi->handle, "_GPE", NULL, &gpe);
+	if (ACPI_FAILURE(acpi_sts)) {
+		dev_err(&(applespi)->spi->dev,
+			"Failed to obtain GPE for SPI slave device: %s\n",
+			acpi_format_exception(acpi_sts));
+		return -ENODEV;
+	}
+	applespi->gpe = (int)gpe;
+
+	acpi_sts = acpi_install_gpe_handler(NULL, applespi->gpe,
+					    ACPI_GPE_LEVEL_TRIGGERED,
+					    applespi_notify, applespi);
+	if (ACPI_FAILURE(acpi_sts)) {
+		dev_err(&(applespi)->spi->dev,
+			"Failed to install GPE handler for GPE %d: %s\n",
+			applespi->gpe, acpi_format_exception(acpi_sts));
+		return -ENODEV;
+	}
+
+	applespi->suspended = false;
+
+	acpi_sts = acpi_enable_gpe(NULL, applespi->gpe);
+	if (ACPI_FAILURE(acpi_sts)) {
+		dev_err(&(applespi)->spi->dev,
+			"Failed to enable GPE handler for GPE %d: %s\n",
+			applespi->gpe, acpi_format_exception(acpi_sts));
+		acpi_remove_gpe_handler(NULL, applespi->gpe, applespi_notify);
+		return -ENODEV;
+	}
+
+	/* trigger touchpad setup */
+	applespi_init(applespi, false);
+
+	/*
+	 * By default this device is not enabled for wakeup; but USB keyboards
+	 * generally are, so the expectation is that by default the keyboard
+	 * will wake the system.
+	 */
+	device_wakeup_enable(&spi->dev);
+
+	/* set up keyboard-backlight */
+	sts = applespi_get_saved_bl_level(applespi);
+	if (sts >= 0)
+		applespi_set_bl_level(&applespi->backlight_info, sts);
+
+	applespi->backlight_info.name            = "spi::kbd_backlight";
+	applespi->backlight_info.default_trigger = "kbd-backlight";
+	applespi->backlight_info.brightness_set  = applespi_set_bl_level;
+
+	sts = devm_led_classdev_register(&spi->dev, &applespi->backlight_info);
+	if (sts)
+		dev_warn(&(applespi)->spi->dev,
+			 "Unable to register keyboard backlight class dev (%d)\n",
+			 sts);
+
+	/* set up debugfs entries for touchpad dimensions logging */
+	applespi->debugfs_root = debugfs_create_dir("applespi", NULL);
+	if (IS_ERR(applespi->debugfs_root)) {
+		if (PTR_ERR(applespi->debugfs_root) != -ENODEV)
+			dev_warn(&(applespi)->spi->dev,
+				 "Error creating debugfs root entry (%ld)\n",
+				 PTR_ERR(applespi->debugfs_root));
+	} else {
+		struct dentry *ret;
+
+		ret = debugfs_create_bool("enable_tp_dim", 0600,
+					  applespi->debugfs_root,
+					  &applespi->debug_tp_dim);
+		if (IS_ERR(ret))
+			dev_warn(&(applespi)->spi->dev,
+				 "Error creating debugfs entry enable_tp_dim (%ld)\n",
+				 PTR_ERR(ret));
+
+		ret = debugfs_create_file("tp_dim", 0400,
+					  applespi->debugfs_root, applespi,
+					  &applespi_tp_dim_fops);
+		if (IS_ERR(ret))
+			dev_warn(&(applespi)->spi->dev,
+				 "Error creating debugfs entry tp_dim (%ld)\n",
+				 PTR_ERR(ret));
+	}
+
+	return 0;
+}
+
+static void applespi_drain_writes(struct applespi_data *applespi)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
+
+	applespi->drain = true;
+	wait_event_lock_irq(applespi->drain_complete, !applespi->write_active,
+			    applespi->cmd_msg_lock);
+
+	spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
+}
+
+static void applespi_drain_reads(struct applespi_data *applespi)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
+
+	wait_event_lock_irq(applespi->drain_complete, !applespi->read_active,
+			    applespi->cmd_msg_lock);
+
+	applespi->suspended = true;
+
+	spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
+}
+
+static int applespi_remove(struct spi_device *spi)
+{
+	struct applespi_data *applespi = spi_get_drvdata(spi);
+
+	applespi_drain_writes(applespi);
+
+	acpi_disable_gpe(NULL, applespi->gpe);
+	acpi_remove_gpe_handler(NULL, applespi->gpe, applespi_notify);
+	device_wakeup_disable(&spi->dev);
+
+	applespi_drain_reads(applespi);
+
+	debugfs_remove_recursive(applespi->debugfs_root);
+
+	return 0;
+}
+
+static void applespi_shutdown(struct spi_device *spi)
+{
+	struct applespi_data *applespi = spi_get_drvdata(spi);
+
+	applespi_save_bl_level(applespi, applespi->have_bl_level);
+}
+
+static int applespi_poweroff_late(struct device *dev)
+{
+	struct spi_device *spi = to_spi_device(dev);
+	struct applespi_data *applespi = spi_get_drvdata(spi);
+
+	applespi_save_bl_level(applespi, applespi->have_bl_level);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int applespi_suspend(struct device *dev)
+{
+	struct spi_device *spi = to_spi_device(dev);
+	struct applespi_data *applespi = spi_get_drvdata(spi);
+	acpi_status acpi_sts;
+	int sts;
+
+	/* turn off caps-lock - it'll stay on otherwise */
+	sts = applespi_set_capsl_led(applespi, false);
+	if (sts)
+		dev_warn(&(applespi)->spi->dev,
+			 "Failed to turn off caps-lock led (%d)\n", sts);
+
+	applespi_drain_writes(applespi);
+
+	/* disable the interrupt */
+	acpi_sts = acpi_disable_gpe(NULL, applespi->gpe);
+	if (ACPI_FAILURE(acpi_sts))
+		dev_err(&(applespi)->spi->dev,
+			"Failed to disable GPE handler for GPE %d: %s\n",
+			applespi->gpe, acpi_format_exception(acpi_sts));
+
+	applespi_drain_reads(applespi);
+
+	return 0;
+}
+
+static int applespi_resume(struct device *dev)
+{
+	struct spi_device *spi = to_spi_device(dev);
+	struct applespi_data *applespi = spi_get_drvdata(spi);
+	acpi_status acpi_sts;
+	unsigned long flags;
+
+	/* ensure our flags and state reflect a newly resumed device */
+	spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
+
+	applespi->drain = false;
+	applespi->have_cl_led_on = false;
+	applespi->have_bl_level = 0;
+	applespi->cmd_msg_queued = false;
+	applespi->read_active = false;
+	applespi->write_active = false;
+
+	applespi->suspended = false;
+
+	spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
+
+	/* switch on the SPI interface */
+	applespi_enable_spi(applespi);
+
+	/* re-enable the interrupt */
+	acpi_sts = acpi_enable_gpe(NULL, applespi->gpe);
+	if (ACPI_FAILURE(acpi_sts))
+		dev_err(&(applespi)->spi->dev,
+			"Failed to re-enable GPE handler for GPE %d: %s\n",
+			applespi->gpe, acpi_format_exception(acpi_sts));
+
+	/* switch the touchpad into multitouch mode */
+	applespi_init(applespi, true);
+
+	return 0;
+}
+#endif
+
+static const struct acpi_device_id applespi_acpi_match[] = {
+	{ "APP000D", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, applespi_acpi_match);
+
+const struct dev_pm_ops applespi_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(applespi_suspend, applespi_resume)
+	.poweroff_late	= applespi_poweroff_late,
+};
+
+static struct spi_driver applespi_driver = {
+	.driver		= {
+		.name			= "applespi",
+		.acpi_match_table	= applespi_acpi_match,
+		.pm			= &applespi_pm_ops,
+	},
+	.probe		= applespi_probe,
+	.remove		= applespi_remove,
+	.shutdown	= applespi_shutdown,
+};
+
+module_spi_driver(applespi_driver)
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("MacBook(Pro) SPI Keyboard/Touchpad driver");
+MODULE_AUTHOR("Federico Lorenzi");
+MODULE_AUTHOR("Ronald Tschalär");
diff --git a/drivers/input/keyboard/applespi.h b/drivers/input/keyboard/applespi.h
new file mode 100644
index 000000000000..7f5ab10c597a
--- /dev/null
+++ b/drivers/input/keyboard/applespi.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * MacBook (Pro) SPI keyboard and touchpad driver
+ *
+ * Copyright (c) 2015-2019 Federico Lorenzi
+ * Copyright (c) 2017-2019 Ronald Tschalär
+ */
+
+#ifndef _APPLESPI_H_
+#define _APPLESPI_H_
+
+enum applespi_evt_type {
+	ET_CMD_TP_INI = BIT(0),
+	ET_CMD_BL = BIT(1),
+	ET_CMD_CL = BIT(2),
+	ET_RD_KEYB = BIT(8),
+	ET_RD_TPAD = BIT(9),
+	ET_RD_UNKN = BIT(10),
+	ET_RD_IRQ = BIT(11),
+	ET_RD_CRC = BIT(12),
+};
+
+enum applespi_pkt_type {
+	PT_READ,
+	PT_WRITE,
+	PT_STATUS,
+};
+
+#endif /* _APPLESPI_H_ */
diff --git a/drivers/input/keyboard/applespi_trace.h b/drivers/input/keyboard/applespi_trace.h
new file mode 100644
index 000000000000..5e965e1974c7
--- /dev/null
+++ b/drivers/input/keyboard/applespi_trace.h
@@ -0,0 +1,94 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * MacBook (Pro) SPI keyboard and touchpad driver
+ *
+ * Copyright (c) 2015-2019 Federico Lorenzi
+ * Copyright (c) 2017-2019 Ronald Tschalär
+ */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM applespi
+
+#if !defined(_APPLESPI_TRACE_H_) || defined(TRACE_HEADER_MULTI_READ)
+#define _APPLESPI_TRACE_H_
+
+#include <linux/types.h>
+#include <linux/tracepoint.h>
+
+#include "applespi.h"
+
+DECLARE_EVENT_CLASS(dump_message_template,
+	TP_PROTO(enum applespi_evt_type evt_type,
+		 enum applespi_pkt_type pkt_type,
+		 u8 *buf,
+		 size_t len),
+
+	TP_ARGS(evt_type, pkt_type, buf, len),
+
+	TP_STRUCT__entry(
+		__field(enum applespi_evt_type, evt_type)
+		__field(enum applespi_pkt_type, pkt_type)
+		__field(size_t, len)
+		__dynamic_array(u8, buf, len)
+	),
+
+	TP_fast_assign(
+		__entry->evt_type = evt_type;
+		__entry->pkt_type = pkt_type;
+		__entry->len = len;
+		memcpy(__get_dynamic_array(buf), buf, len);
+	),
+
+	TP_printk("%-6s: %s",
+		  __print_symbolic(__entry->pkt_type,
+				   { PT_READ, "read" },
+				   { PT_WRITE, "write" },
+				   { PT_STATUS, "status" }
+		  ),
+		  __print_hex(__get_dynamic_array(buf), __entry->len))
+);
+
+#define DEFINE_DUMP_MESSAGE_EVENT(name)			\
+DEFINE_EVENT(dump_message_template, name,		\
+	TP_PROTO(enum applespi_evt_type evt_type,	\
+		 enum applespi_pkt_type pkt_type,	\
+		 u8 *buf,				\
+		 size_t len),				\
+	TP_ARGS(evt_type, pkt_type, buf, len)		\
+)
+
+DEFINE_DUMP_MESSAGE_EVENT(applespi_tp_ini_cmd);
+DEFINE_DUMP_MESSAGE_EVENT(applespi_backlight_cmd);
+DEFINE_DUMP_MESSAGE_EVENT(applespi_caps_lock_cmd);
+DEFINE_DUMP_MESSAGE_EVENT(applespi_keyboard_data);
+DEFINE_DUMP_MESSAGE_EVENT(applespi_touchpad_data);
+DEFINE_DUMP_MESSAGE_EVENT(applespi_unknown_data);
+DEFINE_DUMP_MESSAGE_EVENT(applespi_bad_crc);
+
+TRACE_EVENT(applespi_irq_received,
+	TP_PROTO(enum applespi_evt_type evt_type,
+		 enum applespi_pkt_type pkt_type),
+
+	TP_ARGS(evt_type, pkt_type),
+
+	TP_STRUCT__entry(
+		__field(enum applespi_evt_type, evt_type)
+		__field(enum applespi_pkt_type, pkt_type)
+	),
+
+	TP_fast_assign(
+		__entry->evt_type = evt_type;
+		__entry->pkt_type = pkt_type;
+	),
+
+	"\n"
+);
+
+#endif /* _APPLESPI_TRACE_H_ */
+
+/* This part must be outside protection */
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH ../../drivers/input/keyboard
+#define TRACE_INCLUDE_FILE applespi_trace
+#include <trace/define_trace.h>
+
-- 
2.20.1

^ permalink raw reply related

* [PATCH v4 1/2] drm/bridge: sil_sii8620: make remote control optional.
From: Ronald Tschalär @ 2019-04-07  5:03 UTC (permalink / raw)
  To: Dmitry Torokhov, Henrik Rydberg, Andy Shevchenko,
	Greg Kroah-Hartman
  Cc: Lukas Wunner, Federico Lorenzi, linux-input, linux-kernel,
	Inki Dae, Andrzej Hajda, Laurent Pinchart
In-Reply-To: <20190407050358.2976-1-ronald@innovation.ch>

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).

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>
---
 drivers/gpu/drm/bridge/Kconfig       |  3 +--
 drivers/gpu/drm/bridge/sil-sii8620.c | 21 +++++++++++++++++++++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 2fee47b0d50b..9cf07105b73a 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -85,8 +85,7 @@ config DRM_SIL_SII8620
 	depends on OF
 	select DRM_KMS_HELPER
 	imply EXTCON
-	select INPUT
-	select RC_CORE
+	imply RC_CORE
 	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 a6e8f4591e63..f8dfbf01caa8 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;
@@ -1757,6 +1759,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);
@@ -1775,6 +1778,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)
 {
@@ -2098,6 +2107,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;
@@ -2127,6 +2137,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)
 {
@@ -2213,12 +2228,18 @@ static int sii8620_attach(struct drm_bridge *bridge)
 	return sii8620_clear_error(ctx);
 }
 
+#if IS_ENABLED(CONFIG_RC_CORE)
 static void sii8620_detach(struct drm_bridge *bridge)
 {
 	struct sii8620 *ctx = bridge_to_sii8620(bridge);
 
 	rc_unregister_device(ctx->rc_dev);
 }
+#else
+static void sii8620_detach(struct drm_bridge *bridge)
+{
+}
+#endif
 
 static int sii8620_is_packing_required(struct sii8620 *ctx,
 				       const struct drm_display_mode *mode)
-- 
2.20.1

^ permalink raw reply related

* [PATCH v4 0/2] Add Apple SPI keyboard and trackpad driver
From: Ronald Tschalär @ 2019-04-07  5:03 UTC (permalink / raw)
  To: Dmitry Torokhov, Henrik Rydberg, Andy Shevchenko,
	Greg Kroah-Hartman
  Cc: Lukas Wunner, Federico Lorenzi, linux-input, linux-kernel

This changeset adds a driver for the SPI keyboard and trackpad on recent
MacBook's and MacBook Pro's. The driver has seen a fair amount of use
over the last 2 years (basically anybody running linux on these
machines), with only relatively small changes in the last year or so.
For those interested, the driver development has been hosted at
https://github.com/cb22/macbook12-spi-driver/ (as well as my clone at
https://github.com/roadrunner2/macbook12-spi-driver/).

The first patch fixes a problem during config and is provided for
anybody wanting to compile the driver while it's being reviewed here.
The patch is the latest version that has been submitted for review
at dri-devel; the intent is to get an Ack and permission to merge it
through the input subsystem tree here as part of this patch series.

The second patch contains the new applespi driver.

Changes in v4:
  Applied all feedback from review by Andy Shevchenko and Greg
  Kroah-Hartman, including:
  - minor include path fix
  - moved debug logging to debugfs and tracepoints
  The full set of changes to applespi can be viewed at
  https://github.com/roadrunner2/macbook12-spi-driver/ as individual
  commits 3a6262e..daf60f8 in the upstreaming-review branch.

Ronald Tschalär (2):
  drm/bridge: sil_sii8620: make remote control optional.
  Input: add Apple SPI keyboard and trackpad driver.

 drivers/gpu/drm/bridge/Kconfig          |    3 +-
 drivers/gpu/drm/bridge/sil-sii8620.c    |   21 +
 drivers/input/keyboard/Kconfig          |   15 +
 drivers/input/keyboard/Makefile         |    1 +
 drivers/input/keyboard/applespi.c       | 1999 +++++++++++++++++++++++
 drivers/input/keyboard/applespi.h       |   29 +
 drivers/input/keyboard/applespi_trace.h |   94 ++
 7 files changed, 2160 insertions(+), 2 deletions(-)
 create mode 100644 drivers/input/keyboard/applespi.c
 create mode 100644 drivers/input/keyboard/applespi.h
 create mode 100644 drivers/input/keyboard/applespi_trace.h

-- 
2.20.1

^ 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-04-07  1:46 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: <20190402063326.GB7218@kroah.com>


> On Mon, Apr 01, 2019 at 07:47:14PM -0700, Life is hard, and then you die wrote:
> > 
> > On Thu, Mar 28, 2019 at 12:29:52PM +0100, Greg Kroah-Hartman wrote:
> > > On Thu, Mar 28, 2019 at 03:27:55AM -0700, Life is hard, and then you die wrote:
> > > > 
> > > > On Thu, Mar 28, 2019 at 06:29:17AM +0100, Greg Kroah-Hartman wrote:
> > > > > 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(+)
> > [snip]
> > > > > > > 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 :)
> > > > 
> > > > I am aware of dynamic debugging, but there are several issues with it
> > > > from the perspective of the logging I'm doing here (I mentioned these
> > > > in response to an earlier review already):
> > > > 
> > > >   1. Dynamic debugging can't be enabled at a function or line level on
> > > >      the kernel command line, so this means that to debug issues
> > > >      during boot you have to enable everything, which can be way too
> > > >      verbose
> > > 
> > > You can, and should enable it at a function or line level by writing to
> > > the proper kernel file in debugfs.
> > > 
> > > You have read Documentation/admin-guide/dynamic-debug-howto.rst, right?
> > 
> > Yes, and I've played with the parameters quite a bit.
> > 
> > > No need to do anything on the command line, that's so old-school :)
> > 
> > Sorry if I'm being unduly dense, but then how to enable debugging
> > during early boot? The only other alternative I see is modifying the
> > initrd, and asking folks to do that is noticeably more complicated
> > than having them add something to the command line in grub. So from my
> > perspective I find kernel params far from old-school :-)
> 
> You can do dynamic debugging from the kernel command line, if your code
> is built into the kernel (but why would a tiny driver under testing like
> this, not be built into the kernel?) what specifically did not work for you?

This may be part of our disconnect: there's almost no reason (and
several downsides) to building it into the kernel instead of as a
module:

- When developing, being able to just reload the module instead of
  rebooting is just so much faster and more convenient.

- For other users, having them build the driver as a dkms managed
  module is also by far the simplest and least error-prone approach -
  explaning to users how to rebuild their kernel (something most of
  them have never done) takes a bunch of time and effort on both
  sides for essentially no gain.

- Once the driver is part of the regular kernel, practically all
  distro's will build it as a module (at least I'm fairly certain
  about this).

In the case where a problem happens during early boot, if I can
reproduce myself then of course I can build the driver into the kernel
and debug it; but if I need others to test/debug things, then this is
a much harder sell.

In my experience, every time you make something even a little harder
for a user to do, the chance of them doing it and you getting useful
feedback goes down exponentially and the amount of work you need to do
to handhold them goes up significantly. So I think it's absolutely
vital to try and keep things as simple as possible.

> > > >   2. The expressions to enable the individual logging statements are
> > > >      quite brittle (in particular the line-based ones) since they
> > > >      (may) change any time the code is changed - having stable
> > > >      commands to give to users and put in documentation (e.g.
> > > >      "echo 0x200 > /sys/module/applespi/parameters/debug") is
> > > >      quite valuable.
> > > > 
> > > >      One way to work around this would be to put every single logging
> > > >      statement in a function of its own, thereby ensuring a stable
> > > >      name is associated with each one.
> > > 
> > > Again, read the documentation, this works today as-is.
> > 
> > I have read it (we're talking about the dynamic debug docs here), but
> > I just don't see how it addresses this in any way.
> 
> You can enable/disable logging per-function, which is what you want,
> right?

Yes'ish: the problem is that if they are just part of regular
functions, A) the chances are higher that the function may get renamed
and hence any existing debug instructions broken, and B) this doesn't
work if there are multiple debug statements in a function. Hence my
assertion that for this work well (and yes, it can work well) you
basically need to create a separate function for each debug statement
(which that contains just that debug statement) (a sort of stable
labelling of each debug statement).


  Cheers,

  Ronald

^ permalink raw reply

* [PATCH v3] drm/bridge: sil_sii8620: make remote control optional.
From: Ronald Tschalär @ 2019-04-07  1:30 UTC (permalink / raw)
  To: Andrzej Hajda, Inki Dae
  Cc: Laurent Pinchart, Dmitry Torokhov, Lukas Wunner, dri-devel,
	linux-input, linux-kernel, Laurent Pinchart
In-Reply-To: <20190125013355.GA6722@innovation.ch>

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).

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>
---
Changes in v3, from review by Andrzej Hajda:
 - imply RC_CORE in Kconfig to avoid the possibility of sii8620 being
   built-in but rc-core being a module
 - completely stub out sii8620_detach() to make it consistent with the
   approach taken for the other rc-related functions

 drivers/gpu/drm/bridge/Kconfig       |  3 +--
 drivers/gpu/drm/bridge/sil-sii8620.c | 21 +++++++++++++++++++++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 8840f396a7b6..298189067929 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -86,8 +86,7 @@ config DRM_SIL_SII8620
 	depends on OF
 	select DRM_KMS_HELPER
 	imply EXTCON
-	select INPUT
-	select RC_CORE
+	imply RC_CORE
 	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..9cac00579414 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)
 {
@@ -2212,12 +2227,18 @@ static int sii8620_attach(struct drm_bridge *bridge)
 	return sii8620_clear_error(ctx);
 }
 
+#if IS_ENABLED(CONFIG_RC_CORE)
 static void sii8620_detach(struct drm_bridge *bridge)
 {
 	struct sii8620 *ctx = bridge_to_sii8620(bridge);
 
 	rc_unregister_device(ctx->rc_dev);
 }
+#else
+static void sii8620_detach(struct drm_bridge *bridge)
+{
+}
+#endif
 
 static int sii8620_is_packing_required(struct sii8620 *ctx,
 				       const struct drm_display_mode *mode)
-- 
2.20.1

^ permalink raw reply related

* Re: Re: [PATCH RESEND v4 1/4] input: sun4i-a10-lradc-keys: Add support for A83T
From: 'Ondřej Jirman' via linux-sunxi @ 2019-04-06 16:57 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8, wens-jdAy2FN1RRM,
	robh-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	Ziping Chen, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	hdegoede-H+wXaHxf7aLQT0dZR+AlfA
In-Reply-To: <20190404184915.GB116898@dtor-ws>

Hello,

On Thu, Apr 04, 2019 at 11:49:15AM -0700, Dmitry Torokhov wrote:
> On Wed, Mar 27, 2019 at 03:33:36AM +0100, megous-5qf/QAjKc83QT0dZR+AlfA@public.gmane.org wrote:
> > From: Ziping Chen <techping.chan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > 
> > Allwinner A83T SoC has a low res adc like the one in Allwinner A10 SoC,
> > however, the A10 SoC's vref of lradc internally is divided by 2/3 and
> > the A83T SoC's vref of lradc internally is divided by 3/4, thus add
> > a hardware variant for it to be compatible with various devices.
> > 
> > Signed-off-by: Ziping Chen <techping.chan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Acked-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> 
> Applied, this and binding patch; the dts changes should go through other
> relevant trees I believe.

Thank you very much. Yes, the rest should go through the sunxi tree.

regards,
   Ondrej

> Thanks.
> 
> -- 
> Dmitry
> 
> -- 
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
> For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [PATCH v8 02/11] dt-bindings: power: supply: add DT bindings for max77650
From: Rob Herring @ 2019-04-06  7:07 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Mark Rutland, Linus Walleij, Dmitry Torokhov, Jacek Anaszewski,
	Pavel Machek, Lee Jones, Sebastian Reichel, Liam Girdwood,
	Greg Kroah-Hartman, linux-kernel, linux-gpio, devicetree,
	linux-input, linux-leds, linux-pm, Bartosz Golaszewski
In-Reply-To: <20190403090108.24788-3-brgl@bgdev.pl>

On Wed, Apr 03, 2019 at 11:00:59AM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> Add the DT binding document for the battery charger module of max77650.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  .../power/supply/max77650-charger.txt         | 27 +++++++++++++++++++
>  1 file changed, 27 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/supply/max77650-charger.txt
> 
> diff --git a/Documentation/devicetree/bindings/power/supply/max77650-charger.txt b/Documentation/devicetree/bindings/power/supply/max77650-charger.txt
> new file mode 100644
> index 000000000000..fef188144386
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/max77650-charger.txt
> @@ -0,0 +1,27 @@
> +Battery charger driver for MAX77650 PMIC from Maxim Integrated.
> +
> +This module is part of the MAX77650 MFD device. For more details
> +see Documentation/devicetree/bindings/mfd/max77650.txt.
> +
> +The charger is represented as a sub-node of the PMIC node on the device tree.
> +
> +Required properties:
> +--------------------
> +- compatible:		Must be "maxim,max77650-charger"
> +
> +Optional properties:
> +--------------------
> +- min-microvolt:	Minimum CHGIN regulation voltage (in microvolts). Must be
> +			one of: 4000000, 4100000, 4200000, 4300000, 4400000,
> +			4500000, 4600000, 4700000.
> +- current-limit-microamp:	CHGIN input current limit (in microamps). Must
> +				be one of: 95000, 190000, 285000, 380000, 475000.

To repeat what I said on v6, these should be common properties IMO (and 
therefore documented as such).

> +
> +Example:
> +--------
> +
> +	charger {
> +		compatible = "maxim,max77650-charger";
> +		min-microvolt = <4200000>;
> +		curr-lim-microamp = <285000>;
> +	};
> -- 
> 2.21.0
> 

^ permalink raw reply

* Re: [PATCH] platform/x86: touchscreen_dmi: Add info for Myria MY8307 2-in-1
From: Andy Shevchenko @ 2019-04-05 14:22 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Gabriel Lazar, Darren Hart, Andy Shevchenko, linux-input,
	Platform Driver, Linux Kernel Mailing List
In-Reply-To: <12a67eb2-18ff-dd9f-4c48-4d01a01a2aa7@redhat.com>

On Fri, Mar 15, 2019 at 7:47 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 3/15/19 2:31 PM, Gabriel Lazar wrote:
> > Add touchscreen platform data for the Myrya MY8307 2-in-1 laptop.
> >
> > Signed-off-by: Gabriel Lazar <gabriel.lazar@com.utcluj.ro>
>
> Patch looks good to me:
>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Pushed to my review and testing queue, thanks!

>
> Regards,
>
> Hans
>
>
>
> > ---
> >   drivers/platform/x86/touchscreen_dmi.c | 25 +++++++++++++++++++++++++
> >   1 file changed, 25 insertions(+)
> >
> > diff --git a/drivers/platform/x86/touchscreen_dmi.c b/drivers/platform/x86/touchscreen_dmi.c
> > index 2d56ff7c8230..3319f0cbb558 100644
> > --- a/drivers/platform/x86/touchscreen_dmi.c
> > +++ b/drivers/platform/x86/touchscreen_dmi.c
> > @@ -265,6 +265,23 @@ static const struct ts_dmi_data jumper_ezpad_mini3_data = {
> >       .properties     = jumper_ezpad_mini3_props,
> >   };
> >
> > +static const struct property_entry myria_my8307_props[] = {
> > +     PROPERTY_ENTRY_U32("touchscreen-size-x", 1720),
> > +     PROPERTY_ENTRY_U32("touchscreen-size-y", 1140),
> > +     PROPERTY_ENTRY_BOOL("touchscreen-inverted-x"),
> > +     PROPERTY_ENTRY_BOOL("touchscreen-inverted-y"),
> > +     PROPERTY_ENTRY_BOOL("touchscreen-swapped-x-y"),
> > +     PROPERTY_ENTRY_STRING("firmware-name", "gsl1680-myria-my8307.fw"),
> > +     PROPERTY_ENTRY_U32("silead,max-fingers", 10),
> > +     PROPERTY_ENTRY_BOOL("silead,home-button"),
> > +     { }
> > +};
> > +
> > +static const struct ts_dmi_data myria_my8307_data = {
> > +     .acpi_name      = "MSSL1680:00",
> > +     .properties     = myria_my8307_props,
> > +};
> > +
> >   static const struct property_entry onda_obook_20_plus_props[] = {
> >       PROPERTY_ENTRY_U32("touchscreen-size-x", 1728),
> >       PROPERTY_ENTRY_U32("touchscreen-size-y", 1148),
> > @@ -690,6 +707,14 @@ static const struct dmi_system_id touchscreen_dmi_table[] = {
> >                       DMI_MATCH(DMI_PRODUCT_NAME, "FlexBook edge11 - M-FBE11"),
> >               },
> >       },
> > +     {
> > +             /* Myria MY8307 */
> > +             .driver_data = (void *)&myria_my8307_data,
> > +             .matches = {
> > +                     DMI_MATCH(DMI_SYS_VENDOR, "Complet Electro Serv"),
> > +                     DMI_MATCH(DMI_PRODUCT_NAME, "MY8307"),
> > +             },
> > +     },
> >       {
> >               /* Onda oBook 20 Plus */
> >               .driver_data = (void *)&onda_obook_20_plus_data,
> >



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH RESEND v4 1/4] input: sun4i-a10-lradc-keys: Add support for A83T
From: Dmitry Torokhov @ 2019-04-04 18:49 UTC (permalink / raw)
  To: megous-5qf/QAjKc83QT0dZR+AlfA
  Cc: maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8, wens-jdAy2FN1RRM,
	robh-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	Ziping Chen, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	hdegoede-H+wXaHxf7aLQT0dZR+AlfA
In-Reply-To: <20190327023339.25975-2-megous-5qf/QAjKc83QT0dZR+AlfA@public.gmane.org>

On Wed, Mar 27, 2019 at 03:33:36AM +0100, megous-5qf/QAjKc83QT0dZR+AlfA@public.gmane.org wrote:
> From: Ziping Chen <techping.chan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> Allwinner A83T SoC has a low res adc like the one in Allwinner A10 SoC,
> however, the A10 SoC's vref of lradc internally is divided by 2/3 and
> the A83T SoC's vref of lradc internally is divided by 3/4, thus add
> a hardware variant for it to be compatible with various devices.
> 
> Signed-off-by: Ziping Chen <techping.chan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Acked-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Applied, this and binding patch; the dts changes should go through other
relevant trees I believe.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v8 09/11] leds: max77650: add LEDs support
From: Pavel Machek @ 2019-04-04 13:22 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Rob Herring, Mark Rutland, Linus Walleij, Dmitry Torokhov,
	Jacek Anaszewski, Lee Jones, Sebastian Reichel, Liam Girdwood,
	Greg Kroah-Hartman, linux-kernel, linux-gpio, devicetree,
	linux-input, linux-leds, linux-pm, Bartosz Golaszewski
In-Reply-To: <20190403090108.24788-10-brgl@bgdev.pl>

On Wed 2019-04-03 11:01:06, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> This adds basic support for LEDs for the max77650 PMIC. The device has
> three current sinks for driving LEDs.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>

Acked-by: Pavel Machek <pavel@ucw.cz>


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* Re: [PATCH v8 03/11] dt-bindings: leds: add DT bindings for max77650
From: Pavel Machek @ 2019-04-04 13:22 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Rob Herring, Mark Rutland, Linus Walleij, Dmitry Torokhov,
	Jacek Anaszewski, Lee Jones, Sebastian Reichel, Liam Girdwood,
	Greg Kroah-Hartman, linux-kernel, linux-gpio, devicetree,
	linux-input, linux-leds, linux-pm, Bartosz Golaszewski,
	Rob Herring
In-Reply-To: <20190403090108.24788-4-brgl@bgdev.pl>

On Wed 2019-04-03 11:01:00, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> Add the DT binding document for the LEDs module of max77650.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> Reviewed-by: Rob Herring <robh@kernel.org>

Acked-by: Pavel Machek <pavel@ucw.cz>

^ permalink raw reply

* Re: [PATCH][next] HID: intel-ish-hid: fix spelling mistake "multipe" -> "multiple"
From: Benjamin Tissoires @ 2019-04-04 12:30 UTC (permalink / raw)
  To: Colin King
  Cc: Srinivas Pandruvada, Jiri Kosina, open list:HID CORE LAYER, lkml,
	kernel-janitors
In-Reply-To: <20190404075857.12411-1-colin.king@canonical.com>

On Thu, Apr 4, 2019 at 9:59 AM Colin King <colin.king@canonical.com> wrote:
>
> From: Colin Ian King <colin.king@canonical.com>
>
> There is a spelling mistake in a dev_err message, fix it.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---

Applied to for-5.2/ish

Cheers,
Benjamin

>  drivers/hid/intel-ish-hid/ishtp-fw-loader.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hid/intel-ish-hid/ishtp-fw-loader.c b/drivers/hid/intel-ish-hid/ishtp-fw-loader.c
> index e770e220bd73..22ba21457035 100644
> --- a/drivers/hid/intel-ish-hid/ishtp-fw-loader.c
> +++ b/drivers/hid/intel-ish-hid/ishtp-fw-loader.c
> @@ -525,7 +525,7 @@ static int ish_query_loader_prop(struct ishtp_cl_data *client_data,
>         if ((fw_info->ldr_capability.xfer_mode & LOADER_XFER_MODE_DIRECT_DMA) &&
>             (fw_info->ldr_capability.max_dma_buf_size % L1_CACHE_BYTES)) {
>                 dev_err(cl_data_to_dev(client_data),
> -                       "Shim firmware loader buffer size %d should be multipe of cacheline\n",
> +                       "Shim firmware loader buffer size %d should be multiple of cacheline\n",
>                         fw_info->ldr_capability.max_dma_buf_size);
>                 return -EINVAL;
>         }
> --
> 2.20.1
>

^ permalink raw reply

* Re: [PATCH][next] HID: intel-ish-hid: fix spelling mistake "multipe" -> "multiple"
From: Mukesh Ojha @ 2019-04-04  8:55 UTC (permalink / raw)
  To: Colin King, Srinivas Pandruvada, Jiri Kosina, Benjamin Tissoires,
	linux-input, linux-kernel
  Cc: kernel-janitors
In-Reply-To: <20190404075857.12411-1-colin.king@canonical.com>


On 4/4/2019 1:28 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> There is a spelling mistake in a dev_err message, fix it.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Reviewed-by: Mukesh Ojha <mojha@codeaurora.org>

Cheers,
-Mukesh
> ---
>   drivers/hid/intel-ish-hid/ishtp-fw-loader.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hid/intel-ish-hid/ishtp-fw-loader.c b/drivers/hid/intel-ish-hid/ishtp-fw-loader.c
> index e770e220bd73..22ba21457035 100644
> --- a/drivers/hid/intel-ish-hid/ishtp-fw-loader.c
> +++ b/drivers/hid/intel-ish-hid/ishtp-fw-loader.c
> @@ -525,7 +525,7 @@ static int ish_query_loader_prop(struct ishtp_cl_data *client_data,
>   	if ((fw_info->ldr_capability.xfer_mode & LOADER_XFER_MODE_DIRECT_DMA) &&
>   	    (fw_info->ldr_capability.max_dma_buf_size % L1_CACHE_BYTES)) {
>   		dev_err(cl_data_to_dev(client_data),
> -			"Shim firmware loader buffer size %d should be multipe of cacheline\n",
> +			"Shim firmware loader buffer size %d should be multiple of cacheline\n",
>   			fw_info->ldr_capability.max_dma_buf_size);
>   		return -EINVAL;
>   	}

^ permalink raw reply


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