Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCH v2 07/10] ARM: OMAP1: ams-delta FIQ: Keep serio input GPIOs requested
From: Dmitry Torokhov @ 2018-06-22  0:10 UTC (permalink / raw)
  To: Janusz Krzysztofik
  Cc: Tony Lindgren, Aaro Koskinen, David S . Miller ,
	Mauro Carvalho Chehab, Greg Kroah-Hartman, Andrew Morton,
	Randy Dunlap, Linus Walleij, Mark Brown, Liam Girdwood,
	linux-arm-kernel, linux-omap, linux-input, linux-gpio,
	linux-kernel
In-Reply-To: <20180621224128.17623-7-jmkrzyszt@gmail.com>

On Fri, Jun 22, 2018 at 12:41:25AM +0200, Janusz Krzysztofik wrote:
> From the very beginning, input GPIO pins of ams-delta serio port have
> been used by FIQ handler, not serio driver.
> 
> Don't request those pins from the ams-delta-serio driver any longer,
> instead keep them requested and initialized by the FIQ initialization
> routine which already requests them and releases while identifying GPIO
> IRQs.
> 
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>

Input bits look good.

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

> ---
> Changelog:
> v2: rebased on v4.18-rc1, no conflicts
> 
>  arch/arm/mach-omap1/ams-delta-fiq.c   | 42 ++++++++++++++++++++++++++++++-----
>  drivers/input/serio/ams_delta_serio.c | 30 ++-----------------------
>  2 files changed, 39 insertions(+), 33 deletions(-)
> 
> diff --git a/arch/arm/mach-omap1/ams-delta-fiq.c b/arch/arm/mach-omap1/ams-delta-fiq.c
> index 1d54a6177f14..5a6c59ac9b5f 100644
> --- a/arch/arm/mach-omap1/ams-delta-fiq.c
> +++ b/arch/arm/mach-omap1/ams-delta-fiq.c
> @@ -45,6 +45,11 @@ static struct irq_chip *irq_chip;
>  static struct irq_data *irq_data[16];
>  static unsigned int irq_counter[16];
>  
> +static const char *pin_name[16] __initconst = {
> +	[AMS_DELTA_GPIO_PIN_KEYBRD_DATA]	= "keybrd_data",
> +	[AMS_DELTA_GPIO_PIN_KEYBRD_CLK]		= "keybrd_clk",
> +};
> +
>  static irqreturn_t deferred_fiq(int irq, void *dev_id)
>  {
>  	struct irq_data *d;
> @@ -80,7 +85,7 @@ static irqreturn_t deferred_fiq(int irq, void *dev_id)
>  
>  void __init ams_delta_init_fiq(struct gpio_chip *chip)
>  {
> -	struct gpio_desc *gpiod;
> +	struct gpio_desc *gpiod, *data = NULL, *clk = NULL;
>  	void *fiqhandler_start;
>  	unsigned int fiqhandler_length;
>  	struct pt_regs FIQ_regs;
> @@ -96,7 +101,7 @@ void __init ams_delta_init_fiq(struct gpio_chip *chip)
>  	}
>  
>  	for (i = 0; i < ARRAY_SIZE(irq_data); i++) {
> -		gpiod = gpiochip_request_own_desc(chip, i, NULL);
> +		gpiod = gpiochip_request_own_desc(chip, i, pin_name[i]);
>  		if (IS_ERR(gpiod)) {
>  			pr_err("%s: failed to get GPIO pin %d (%ld)\n",
>  			       __func__, i, PTR_ERR(gpiod));
> @@ -105,8 +110,27 @@ void __init ams_delta_init_fiq(struct gpio_chip *chip)
>  		/* Store irq_data location for IRQ handler use */
>  		irq_data[i] = irq_get_irq_data(gpiod_to_irq(gpiod));
>  
> -		gpiochip_free_own_desc(gpiod);
> +		/*
> +		 * FIQ handler takes full control over serio data and clk GPIO
> +		 * pins.  Initiaize them and keep requested so nobody can
> +		 * interfere.  Fail if any of those two couldn't be requested.
> +		 */
> +		switch (i) {
> +		case AMS_DELTA_GPIO_PIN_KEYBRD_DATA:
> +			data = gpiod;
> +			gpiod_direction_input(data);
> +			break;
> +		case AMS_DELTA_GPIO_PIN_KEYBRD_CLK:
> +			clk = gpiod;
> +			gpiod_direction_input(clk);
> +			break;
> +		default:
> +			gpiochip_free_own_desc(gpiod);
> +			break;
> +		}
>  	}
> +	if (!data || !clk)
> +		goto out_gpio;
>  
>  	fiqhandler_start = &qwerty_fiqin_start;
>  	fiqhandler_length = &qwerty_fiqin_end - &qwerty_fiqin_start;
> @@ -117,7 +141,7 @@ void __init ams_delta_init_fiq(struct gpio_chip *chip)
>  	if (retval) {
>  		pr_err("ams_delta_init_fiq(): couldn't claim FIQ, ret=%d\n",
>  				retval);
> -		return;
> +		goto out_gpio;
>  	}
>  
>  	retval = request_irq(INT_DEFERRED_FIQ, deferred_fiq,
> @@ -125,7 +149,7 @@ void __init ams_delta_init_fiq(struct gpio_chip *chip)
>  	if (retval < 0) {
>  		pr_err("Failed to get deferred_fiq IRQ, ret=%d\n", retval);
>  		release_fiq(&fh);
> -		return;
> +		goto out_gpio;
>  	}
>  	/*
>  	 * Since no set_type() method is provided by OMAP irq chip,
> @@ -175,4 +199,12 @@ void __init ams_delta_init_fiq(struct gpio_chip *chip)
>  	offset = IRQ_ILR0_REG_OFFSET + (INT_GPIO_BANK1 - NR_IRQS_LEGACY) * 0x4;
>  	val = omap_readl(OMAP_IH1_BASE + offset) | 1;
>  	omap_writel(val, OMAP_IH1_BASE + offset);
> +
> +	return;
> +
> +out_gpio:
> +	if (data)
> +		gpiochip_free_own_desc(data);
> +	if (clk)
> +		gpiochip_free_own_desc(clk);
>  }
> diff --git a/drivers/input/serio/ams_delta_serio.c b/drivers/input/serio/ams_delta_serio.c
> index b955c6a72e99..7952a29f9540 100644
> --- a/drivers/input/serio/ams_delta_serio.c
> +++ b/drivers/input/serio/ams_delta_serio.c
> @@ -110,19 +110,6 @@ static void ams_delta_serio_close(struct serio *serio)
>  	regulator_disable(priv->vcc);
>  }
>  
> -static const struct gpio ams_delta_gpios[] __initconst_or_module = {
> -	{
> -		.gpio	= AMS_DELTA_GPIO_PIN_KEYBRD_DATA,
> -		.flags	= GPIOF_DIR_IN,
> -		.label	= "serio-data",
> -	},
> -	{
> -		.gpio	= AMS_DELTA_GPIO_PIN_KEYBRD_CLK,
> -		.flags	= GPIOF_DIR_IN,
> -		.label	= "serio-clock",
> -	},
> -};
> -
>  static int ams_delta_serio_init(struct platform_device *pdev)
>  {
>  	struct ams_delta_serio *priv;
> @@ -133,13 +120,6 @@ static int ams_delta_serio_init(struct platform_device *pdev)
>  	if (!priv)
>  		return -ENOMEM;
>  
> -	err = gpio_request_array(ams_delta_gpios,
> -				ARRAY_SIZE(ams_delta_gpios));
> -	if (err) {
> -		dev_err(&pdev->dev, "Couldn't request gpio pins\n");
> -		goto serio;
> -	}
> -
>  	priv->vcc = devm_regulator_get(&pdev->dev, "vcc");
>  	if (IS_ERR(priv->vcc)) {
>  		err = PTR_ERR(priv->vcc);
> @@ -157,7 +137,7 @@ static int ams_delta_serio_init(struct platform_device *pdev)
>  		 */
>  		if (err == -ENODEV)
>  			err = -EPROBE_DEFER;
> -		goto gpio;
> +		return err;
>  	}
>  
>  	err = request_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK),
> @@ -165,7 +145,7 @@ static int ams_delta_serio_init(struct platform_device *pdev)
>  			DRIVER_NAME, priv);
>  	if (err < 0) {
>  		dev_err(&pdev->dev, "IRQ request failed (%d)\n", err);
> -		goto gpio;
> +		return err;
>  	}
>  	/*
>  	 * Since GPIO register handling for keyboard clock pin is performed
> @@ -201,10 +181,6 @@ static int ams_delta_serio_init(struct platform_device *pdev)
>  
>  irq:
>  	free_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK), priv);
> -gpio:
> -	gpio_free_array(ams_delta_gpios,
> -			ARRAY_SIZE(ams_delta_gpios));
> -serio:
>  	return err;
>  }
>  
> @@ -214,8 +190,6 @@ static int ams_delta_serio_exit(struct platform_device *pdev)
>  
>  	serio_unregister_port(priv->serio);
>  	free_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK), 0);
> -	gpio_free_array(ams_delta_gpios,
> -			ARRAY_SIZE(ams_delta_gpios));
>  
>  	return 0;
>  }
> -- 
> 2.16.4
> 

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v2 04/10] Input: ams_delta_serio: Replace power GPIO with regulator
From: Dmitry Torokhov @ 2018-06-22  0:09 UTC (permalink / raw)
  To: Janusz Krzysztofik
  Cc: Tony Lindgren, Aaro Koskinen, David S . Miller ,
	Mauro Carvalho Chehab, Greg Kroah-Hartman, Andrew Morton,
	Randy Dunlap, Linus Walleij, Mark Brown, Liam Girdwood,
	linux-arm-kernel, linux-omap, linux-input, linux-gpio,
	linux-kernel
In-Reply-To: <20180621224128.17623-4-jmkrzyszt@gmail.com>

On Fri, Jun 22, 2018 at 12:41:22AM +0200, Janusz Krzysztofik wrote:
> Modify the driver so it no longer requests and manipulates the
> "keybrd_pwr" GPIO pin but a "vcc" regulator supply instead.
> 
> For this to work with Amstrad Delta, define a regulator over the
> "keybrd_pwr" GPIO pin with the "vcc" supply for ams-delta-serio device
> and register it from the board file.  Both assign an absulute GPIO
> number to the soon depreciated .gpio member of the regulator config
> structure, and also build and register a GPIO lookup table so it is
> ready for use by the regulator driver as soon as its upcoming update
> is applied.
> 
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>

Input bits look good.

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

> ---
> Changelog:
> v2: - extended comment above error code conversion, thanks Dmitry for 
>       requesting that,
>     - rebased on v4.18-rc1, no conflicts.
> 
>  arch/arm/mach-omap1/board-ams-delta.c | 63 +++++++++++++++++++++++++++++++++--
>  drivers/input/serio/ams_delta_serio.c | 37 +++++++++++++++-----
>  2 files changed, 89 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
> index 2119d2d3ba84..706eb2f9301d 100644
> --- a/arch/arm/mach-omap1/board-ams-delta.c
> +++ b/arch/arm/mach-omap1/board-ams-delta.c
> @@ -509,6 +509,46 @@ static struct platform_device ams_delta_serio_device = {
>  	.id		= PLATFORM_DEVID_NONE,
>  };
>  
> +static struct regulator_consumer_supply keybrd_pwr_consumers[] = {
> +	/*
> +	 * Initialize supply .dev_name with NULL.  It will be replaced
> +	 * with serio dev_name() as soon as the serio device is registered.
> +	 */
> +	REGULATOR_SUPPLY("vcc", NULL),
> +};
> +
> +static struct regulator_init_data keybrd_pwr_initdata = {
> +	.constraints		= {
> +		.valid_ops_mask		= REGULATOR_CHANGE_STATUS,
> +	},
> +	.num_consumer_supplies	= ARRAY_SIZE(keybrd_pwr_consumers),
> +	.consumer_supplies	= keybrd_pwr_consumers,
> +};
> +
> +static struct fixed_voltage_config keybrd_pwr_config = {
> +	.supply_name		= "keybrd_pwr",
> +	.microvolts		= 5000000,
> +	.gpio			= AMS_DELTA_GPIO_PIN_KEYBRD_PWR,
> +	.enable_high		= 1,
> +	.init_data		= &keybrd_pwr_initdata,
> +};
> +
> +static struct platform_device keybrd_pwr_device = {
> +	.name	= "reg-fixed-voltage",
> +	.id	= PLATFORM_DEVID_AUTO,
> +	.dev	= {
> +		.platform_data	= &keybrd_pwr_config,
> +	},
> +};
> +
> +static struct gpiod_lookup_table keybrd_pwr_gpio_table = {
> +	.table = {
> +		GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_KEYBRD_PWR, NULL,
> +			    GPIO_ACTIVE_HIGH),
> +		{ },
> +	},
> +};
> +
>  static struct platform_device *ams_delta_devices[] __initdata = {
>  	&latch1_gpio_device,
>  	&latch2_gpio_device,
> @@ -526,6 +566,7 @@ static struct platform_device *late_devices[] __initdata = {
>  
>  static struct gpiod_lookup_table *ams_delta_gpio_tables[] __initdata = {
>  	&ams_delta_audio_gpio_table,
> +	&keybrd_pwr_gpio_table,
>  };
>  
>  static struct gpiod_lookup_table *late_gpio_tables[] __initdata = {
> @@ -566,12 +607,30 @@ static void __init ams_delta_init(void)
>  	platform_add_devices(ams_delta_devices, ARRAY_SIZE(ams_delta_devices));
>  
>  	/*
> -	 * As soon as devices have been registered, assign their dev_names
> -	 * to respective GPIO lookup tables before they are added.
> +	 * As soon as regulator consumers have been registered, assign their
> +	 * dev_names to consumer supply entries of respective regulators.
> +	 */
> +	keybrd_pwr_consumers[0].dev_name =
> +			dev_name(&ams_delta_serio_device.dev);
> +
> +	/*
> +	 * Once consumer supply entries are populated with dev_names,
> +	 * register regulator devices.  At this stage only the keyboard
> +	 * power regulator has its consumer supply table fully populated.
> +	 */
> +	platform_device_register(&keybrd_pwr_device);
> +
> +	/*
> +	 * As soon as GPIO consumers have been registered, assign
> +	 * their dev_names to respective GPIO lookup tables.
>  	 */
>  	ams_delta_audio_gpio_table.dev_id =
>  			dev_name(&ams_delta_audio_device.dev);
> +	keybrd_pwr_gpio_table.dev_id = dev_name(&keybrd_pwr_device.dev);
>  
> +	/*
> +	 * Once GPIO lookup tables are populated with dev_names, register them.
> +	 */
>  	gpiod_add_lookup_tables(ams_delta_gpio_tables,
>  				ARRAY_SIZE(ams_delta_gpio_tables));
>  
> diff --git a/drivers/input/serio/ams_delta_serio.c b/drivers/input/serio/ams_delta_serio.c
> index 551a4fa73fe4..854d0d3ada52 100644
> --- a/drivers/input/serio/ams_delta_serio.c
> +++ b/drivers/input/serio/ams_delta_serio.c
> @@ -23,6 +23,7 @@
>  #include <linux/gpio.h>
>  #include <linux/irq.h>
>  #include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
>  #include <linux/serio.h>
>  #include <linux/slab.h>
>  #include <linux/module.h>
> @@ -39,6 +40,7 @@ MODULE_LICENSE("GPL");
>  
>  struct ams_delta_serio {
>  	struct serio *serio;
> +	struct regulator *vcc;
>  };
>  
>  static int check_data(struct serio *serio, int data)
> @@ -94,16 +96,18 @@ static irqreturn_t ams_delta_serio_interrupt(int irq, void *dev_id)
>  
>  static int ams_delta_serio_open(struct serio *serio)
>  {
> -	/* enable keyboard */
> -	gpio_set_value(AMS_DELTA_GPIO_PIN_KEYBRD_PWR, 1);
> +	struct ams_delta_serio *priv = serio->port_data;
>  
> -	return 0;
> +	/* enable keyboard */
> +	return regulator_enable(priv->vcc);
>  }
>  
>  static void ams_delta_serio_close(struct serio *serio)
>  {
> +	struct ams_delta_serio *priv = serio->port_data;
> +
>  	/* disable keyboard */
> -	gpio_set_value(AMS_DELTA_GPIO_PIN_KEYBRD_PWR, 0);
> +	regulator_disable(priv->vcc);
>  }
>  
>  static const struct gpio ams_delta_gpios[] __initconst_or_module = {
> @@ -117,11 +121,6 @@ static const struct gpio ams_delta_gpios[] __initconst_or_module = {
>  		.flags	= GPIOF_DIR_IN,
>  		.label	= "serio-clock",
>  	},
> -	{
> -		.gpio	= AMS_DELTA_GPIO_PIN_KEYBRD_PWR,
> -		.flags	= GPIOF_OUT_INIT_LOW,
> -		.label	= "serio-power",
> -	},
>  	{
>  		.gpio	= AMS_DELTA_GPIO_PIN_KEYBRD_DATAOUT,
>  		.flags	= GPIOF_OUT_INIT_LOW,
> @@ -146,6 +145,26 @@ static int ams_delta_serio_init(struct platform_device *pdev)
>  		goto serio;
>  	}
>  
> +	priv->vcc = devm_regulator_get(&pdev->dev, "vcc");
> +	if (IS_ERR(priv->vcc)) {
> +		err = PTR_ERR(priv->vcc);
> +		dev_err(&pdev->dev, "regulator request failed (%d)\n", err);
> +		/*
> +		 * When running on a non-dt platform and requested regulator
> +		 * is not available, devm_regulator_get() never returns
> +		 * -EPROBE_DEFER as it is not able to justify if the regulator
> +		 * may still appear later.  On the other hand, the board can
> +		 * still set full constriants flag at late_initcall in order
> +		 * to instruct devm_regulator_get() to returnn a dummy one
> +		 * if sufficient.  Hence, if we get -ENODEV here, let's convert
> +		 * it to -EPROBE_DEFER and wait for the board to decide or
> +		 * let Deferred Probe infrastructure handle this error.
> +		 */
> +		if (err == -ENODEV)
> +			err = -EPROBE_DEFER;
> +		goto gpio;
> +	}
> +
>  	err = request_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK),
>  			ams_delta_serio_interrupt, IRQ_TYPE_EDGE_RISING,
>  			DRIVER_NAME, priv);
> -- 
> 2.16.4
> 

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v2 03/10] Input: ams_delta_serio: use private structure
From: Dmitry Torokhov @ 2018-06-22  0:07 UTC (permalink / raw)
  To: Janusz Krzysztofik
  Cc: Tony Lindgren, Aaro Koskinen, David S . Miller ,
	Mauro Carvalho Chehab, Greg Kroah-Hartman, Andrew Morton,
	Randy Dunlap, Linus Walleij, Mark Brown, Liam Girdwood,
	linux-arm-kernel, linux-omap, linux-input, linux-gpio,
	linux-kernel
In-Reply-To: <20180621224128.17623-3-jmkrzyszt@gmail.com>

On Fri, Jun 22, 2018 at 12:41:21AM +0200, Janusz Krzysztofik wrote:
> Introduce a driver private structure and allocate it on device probe.
> For now, use it instead of a static variable for storing a pointer to
> serio structure.  Subsequent patches will populate it with more members
> as needed.
> 
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

> ---
> Changelog:
> v2: rebased on v4.18-rc1, no conflicts
> 
>  drivers/input/serio/ams_delta_serio.c | 69 ++++++++++++++++++++++-------------
>  1 file changed, 43 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/input/serio/ams_delta_serio.c b/drivers/input/serio/ams_delta_serio.c
> index a2a7fa19bf49..551a4fa73fe4 100644
> --- a/drivers/input/serio/ams_delta_serio.c
> +++ b/drivers/input/serio/ams_delta_serio.c
> @@ -37,17 +37,17 @@ MODULE_AUTHOR("Matt Callow");
>  MODULE_DESCRIPTION("AMS Delta (E3) keyboard port driver");
>  MODULE_LICENSE("GPL");
>  
> -static struct serio *ams_delta_serio;
> +struct ams_delta_serio {
> +	struct serio *serio;
> +};
>  
> -static int check_data(int data)
> +static int check_data(struct serio *serio, int data)
>  {
>  	int i, parity = 0;
>  
>  	/* check valid stop bit */
>  	if (!(data & 0x400)) {
> -		dev_warn(&ams_delta_serio->dev,
> -				"invalid stop bit, data=0x%X\n",
> -				data);
> +		dev_warn(&serio->dev, "invalid stop bit, data=0x%X\n", data);
>  		return SERIO_FRAME;
>  	}
>  	/* calculate the parity */
> @@ -57,9 +57,9 @@ static int check_data(int data)
>  	}
>  	/* it should be odd */
>  	if (!(parity & 0x01)) {
> -		dev_warn(&ams_delta_serio->dev,
> -				"parity check failed, data=0x%X parity=0x%X\n",
> -				data, parity);
> +		dev_warn(&serio->dev,
> +			 "parity check failed, data=0x%X parity=0x%X\n", data,
> +			 parity);
>  		return SERIO_PARITY;
>  	}
>  	return 0;
> @@ -67,6 +67,7 @@ static int check_data(int data)
>  
>  static irqreturn_t ams_delta_serio_interrupt(int irq, void *dev_id)
>  {
> +	struct ams_delta_serio *priv = dev_id;
>  	int *circ_buff = &fiq_buffer[FIQ_CIRC_BUFF];
>  	int data, dfl;
>  	u8 scancode;
> @@ -84,9 +85,9 @@ static irqreturn_t ams_delta_serio_interrupt(int irq, void *dev_id)
>  		if (fiq_buffer[FIQ_HEAD_OFFSET] == fiq_buffer[FIQ_BUF_LEN])
>  			fiq_buffer[FIQ_HEAD_OFFSET] = 0;
>  
> -		dfl = check_data(data);
> +		dfl = check_data(priv->serio, data);
>  		scancode = (u8) (data >> 1) & 0xFF;
> -		serio_interrupt(ams_delta_serio, scancode, dfl);
> +		serio_interrupt(priv->serio, scancode, dfl);
>  	}
>  	return IRQ_HANDLED;
>  }
> @@ -130,21 +131,14 @@ static const struct gpio ams_delta_gpios[] __initconst_or_module = {
>  
>  static int ams_delta_serio_init(struct platform_device *pdev)
>  {
> +	struct ams_delta_serio *priv;
> +	struct serio *serio;
>  	int err;
>  
> -	ams_delta_serio = kzalloc(sizeof(struct serio), GFP_KERNEL);
> -	if (!ams_delta_serio)
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
>  		return -ENOMEM;
>  
> -	ams_delta_serio->id.type = SERIO_8042;
> -	ams_delta_serio->open = ams_delta_serio_open;
> -	ams_delta_serio->close = ams_delta_serio_close;
> -	strlcpy(ams_delta_serio->name, "AMS DELTA keyboard adapter",
> -			sizeof(ams_delta_serio->name));
> -	strlcpy(ams_delta_serio->phys, dev_name(&pdev->dev),
> -			sizeof(ams_delta_serio->phys));
> -	ams_delta_serio->dev.parent = &pdev->dev;
> -
>  	err = gpio_request_array(ams_delta_gpios,
>  				ARRAY_SIZE(ams_delta_gpios));
>  	if (err) {
> @@ -154,7 +148,7 @@ static int ams_delta_serio_init(struct platform_device *pdev)
>  
>  	err = request_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK),
>  			ams_delta_serio_interrupt, IRQ_TYPE_EDGE_RISING,
> -			DRIVER_NAME, 0);
> +			DRIVER_NAME, priv);
>  	if (err < 0) {
>  		dev_err(&pdev->dev, "IRQ request failed (%d)\n", err);
>  		goto gpio;
> @@ -167,21 +161,44 @@ static int ams_delta_serio_init(struct platform_device *pdev)
>  	irq_set_handler(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK),
>  			handle_simple_irq);
>  
> -	serio_register_port(ams_delta_serio);
> -	dev_info(&ams_delta_serio->dev, "%s\n", ams_delta_serio->name);
> +	serio = kzalloc(sizeof(*serio), GFP_KERNEL);
> +	if (!serio) {
> +		err = -ENOMEM;
> +		goto irq;
> +	}
> +
> +	priv->serio = serio;
> +
> +	serio->id.type = SERIO_8042;
> +	serio->open = ams_delta_serio_open;
> +	serio->close = ams_delta_serio_close;
> +	strlcpy(serio->name, "AMS DELTA keyboard adapter", sizeof(serio->name));
> +	strlcpy(serio->phys, dev_name(&pdev->dev), sizeof(serio->phys));
> +	serio->dev.parent = &pdev->dev;
> +	serio->port_data = priv;
> +
> +	serio_register_port(serio);
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	dev_info(&serio->dev, "%s\n", serio->name);
>  
>  	return 0;
> +
> +irq:
> +	free_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK), priv);
>  gpio:
>  	gpio_free_array(ams_delta_gpios,
>  			ARRAY_SIZE(ams_delta_gpios));
>  serio:
> -	kfree(ams_delta_serio);
>  	return err;
>  }
>  
>  static int ams_delta_serio_exit(struct platform_device *pdev)
>  {
> -	serio_unregister_port(ams_delta_serio);
> +	struct ams_delta_serio *priv = platform_get_drvdata(pdev);
> +
> +	serio_unregister_port(priv->serio);
>  	free_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK), 0);
>  	gpio_free_array(ams_delta_gpios,
>  			ARRAY_SIZE(ams_delta_gpios));
> -- 
> 2.16.4
> 

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v2 02/10] Input: ams_delta_serio: convert to platform driver
From: Dmitry Torokhov @ 2018-06-22  0:05 UTC (permalink / raw)
  To: Janusz Krzysztofik
  Cc: Tony Lindgren, Aaro Koskinen, David S . Miller ,
	Mauro Carvalho Chehab, Greg Kroah-Hartman, Andrew Morton,
	Randy Dunlap, Linus Walleij, Mark Brown, Liam Girdwood,
	linux-arm-kernel, linux-omap, linux-input, linux-gpio,
	linux-kernel
In-Reply-To: <20180621224128.17623-2-jmkrzyszt@gmail.com>

On Fri, Jun 22, 2018 at 12:41:20AM +0200, Janusz Krzysztofik wrote:
> Convert the driver to an "ams-delta-serio" platform driver.  For it to
> be used with Amstrad Delta, register an "ams-delta-serio" platform
> device from the board init file.
> 
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
> ---
> Changelog:
> v2: rebased on v4.18-rc1, no conflicts

Input bits look good to me.

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

> 
>  arch/arm/mach-omap1/board-ams-delta.c |  6 ++++++
>  drivers/input/serio/ams_delta_serio.c | 34 +++++++++++++++++++++-------------
>  2 files changed, 27 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
> index 18e0ff437b27..2119d2d3ba84 100644
> --- a/arch/arm/mach-omap1/board-ams-delta.c
> +++ b/arch/arm/mach-omap1/board-ams-delta.c
> @@ -504,12 +504,18 @@ static struct platform_device cx20442_codec_device = {
>  	.id     = -1,
>  };
>  
> +static struct platform_device ams_delta_serio_device = {
> +	.name		= "ams-delta-serio",
> +	.id		= PLATFORM_DEVID_NONE,
> +};
> +
>  static struct platform_device *ams_delta_devices[] __initdata = {
>  	&latch1_gpio_device,
>  	&latch2_gpio_device,
>  	&ams_delta_kp_device,
>  	&ams_delta_camera_device,
>  	&ams_delta_audio_device,
> +	&ams_delta_serio_device,
>  };
>  
>  static struct platform_device *late_devices[] __initdata = {
> diff --git a/drivers/input/serio/ams_delta_serio.c b/drivers/input/serio/ams_delta_serio.c
> index 3df501c3421b..a2a7fa19bf49 100644
> --- a/drivers/input/serio/ams_delta_serio.c
> +++ b/drivers/input/serio/ams_delta_serio.c
> @@ -22,15 +22,17 @@
>   */
>  #include <linux/gpio.h>
>  #include <linux/irq.h>
> +#include <linux/platform_device.h>
>  #include <linux/serio.h>
>  #include <linux/slab.h>
>  #include <linux/module.h>
>  
> -#include <asm/mach-types.h>
>  #include <mach/board-ams-delta.h>
>  
>  #include <mach/ams-delta-fiq.h>
>  
> +#define DRIVER_NAME	"ams-delta-serio"
> +
>  MODULE_AUTHOR("Matt Callow");
>  MODULE_DESCRIPTION("AMS Delta (E3) keyboard port driver");
>  MODULE_LICENSE("GPL");
> @@ -126,13 +128,10 @@ static const struct gpio ams_delta_gpios[] __initconst_or_module = {
>  	},
>  };
>  
> -static int __init ams_delta_serio_init(void)
> +static int ams_delta_serio_init(struct platform_device *pdev)
>  {
>  	int err;
>  
> -	if (!machine_is_ams_delta())
> -		return -ENODEV;
> -
>  	ams_delta_serio = kzalloc(sizeof(struct serio), GFP_KERNEL);
>  	if (!ams_delta_serio)
>  		return -ENOMEM;
> @@ -142,22 +141,22 @@ static int __init ams_delta_serio_init(void)
>  	ams_delta_serio->close = ams_delta_serio_close;
>  	strlcpy(ams_delta_serio->name, "AMS DELTA keyboard adapter",
>  			sizeof(ams_delta_serio->name));
> -	strlcpy(ams_delta_serio->phys, "GPIO/serio0",
> +	strlcpy(ams_delta_serio->phys, dev_name(&pdev->dev),
>  			sizeof(ams_delta_serio->phys));
> +	ams_delta_serio->dev.parent = &pdev->dev;
>  
>  	err = gpio_request_array(ams_delta_gpios,
>  				ARRAY_SIZE(ams_delta_gpios));
>  	if (err) {
> -		pr_err("ams_delta_serio: Couldn't request gpio pins\n");
> +		dev_err(&pdev->dev, "Couldn't request gpio pins\n");
>  		goto serio;
>  	}
>  
>  	err = request_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK),
>  			ams_delta_serio_interrupt, IRQ_TYPE_EDGE_RISING,
> -			"ams-delta-serio", 0);
> +			DRIVER_NAME, 0);
>  	if (err < 0) {
> -		pr_err("ams_delta_serio: couldn't request gpio interrupt %d\n",
> -				gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK));
> +		dev_err(&pdev->dev, "IRQ request failed (%d)\n", err);
>  		goto gpio;
>  	}
>  	/*
> @@ -179,13 +178,22 @@ static int __init ams_delta_serio_init(void)
>  	kfree(ams_delta_serio);
>  	return err;
>  }
> -module_init(ams_delta_serio_init);
>  
> -static void __exit ams_delta_serio_exit(void)
> +static int ams_delta_serio_exit(struct platform_device *pdev)
>  {
>  	serio_unregister_port(ams_delta_serio);
>  	free_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK), 0);
>  	gpio_free_array(ams_delta_gpios,
>  			ARRAY_SIZE(ams_delta_gpios));
> +
> +	return 0;
>  }
> -module_exit(ams_delta_serio_exit);
> +
> +static struct platform_driver ams_delta_serio_driver = {
> +	.probe	= ams_delta_serio_init,
> +	.remove	= ams_delta_serio_exit,
> +	.driver	= {
> +		.name	= DRIVER_NAME
> +	},
> +};
> +module_platform_driver(ams_delta_serio_driver);
> -- 
> 2.16.4
> 

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v2] HID: i2c-hid: Add vddl regulator control
From: Dmitry Torokhov @ 2018-06-21 23:58 UTC (permalink / raw)
  To: swboyd
  Cc: Jiri Kosina, lkml, open list:HID CORE LAYER, Benjamin Tissoires,
	Hans de Goede, andriy.shevchenko, Doug Anderson, devicetree
In-Reply-To: <20180621234138.30537-1-swboyd@chromium.org>

On Thu, Jun 21, 2018 at 4:41 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> Some wacom w9013 devices have a vddl supply for "low valtage"
> requirements. Add support in this driver to turn on this low voltage
> supply. We can also drop a handful of error messages because the
> regulator core is already printing an error when bulk regulators fail to
> enable or disable.
>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Dmitry Torokhov <dtor@chromium.org>
> Cc: Doug Anderson <dianders@chromium.org>
> Acked-by: Rob Herring <robh@kernel.org>
> Cc: <devicetree@vger.kernel.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>
> Changes from v1:
>  * Use the bulk regulator APIs instead of open coding it (Dmitry)
>
>  .../bindings/input/hid-over-i2c.txt           |  3 +-
>  drivers/hid/i2c-hid/i2c-hid.c                 | 62 +++++++++----------
>  include/linux/platform_data/i2c-hid.h         |  7 +--
>  3 files changed, 36 insertions(+), 36 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/input/hid-over-i2c.txt b/Documentation/devicetree/bindings/input/hid-over-i2c.txt
> index 4d3da9d91de4..89e6ab89ba38 100644
> --- a/Documentation/devicetree/bindings/input/hid-over-i2c.txt
> +++ b/Documentation/devicetree/bindings/input/hid-over-i2c.txt
> @@ -26,7 +26,8 @@ device-specific compatible properties, which should be used in addition to the
>
>  - compatible:
>    * "wacom,w9013" (Wacom W9013 digitizer). Supports:
> -    - vdd-supply
> +    - vdd-supply (3.3V)
> +    - vddl-supply (1.8V)
>      - post-power-on-delay-ms
>
>  - vdd-supply: phandle of the regulator that provides the supply voltage.
> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> index c1652bb7bd15..1d83531454c5 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.c
> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> @@ -1002,18 +1002,21 @@ static int i2c_hid_probe(struct i2c_client *client,
>                 return client->irq;
>         }
>
> -       ihid = kzalloc(sizeof(struct i2c_hid), GFP_KERNEL);
> +       ihid = devm_kzalloc(&client->dev, sizeof(*ihid), GFP_KERNEL);

The patch looks good, but I think this chunk needs to be split out.

Thanks,
Dmitry

^ permalink raw reply

* [PATCH v2] HID: i2c-hid: Add vddl regulator control
From: Stephen Boyd @ 2018-06-21 23:41 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: linux-kernel, linux-input, Benjamin Tissoires, Hans de Goede,
	Andy Shevchenko, Dmitry Torokhov, Doug Anderson, devicetree

Some wacom w9013 devices have a vddl supply for "low valtage"
requirements. Add support in this driver to turn on this low voltage
supply. We can also drop a handful of error messages because the
regulator core is already printing an error when bulk regulators fail to
enable or disable.

Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Dmitry Torokhov <dtor@chromium.org>
Cc: Doug Anderson <dianders@chromium.org>
Acked-by: Rob Herring <robh@kernel.org>
Cc: <devicetree@vger.kernel.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---

Changes from v1:
 * Use the bulk regulator APIs instead of open coding it (Dmitry)

 .../bindings/input/hid-over-i2c.txt           |  3 +-
 drivers/hid/i2c-hid/i2c-hid.c                 | 62 +++++++++----------
 include/linux/platform_data/i2c-hid.h         |  7 +--
 3 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/hid-over-i2c.txt b/Documentation/devicetree/bindings/input/hid-over-i2c.txt
index 4d3da9d91de4..89e6ab89ba38 100644
--- a/Documentation/devicetree/bindings/input/hid-over-i2c.txt
+++ b/Documentation/devicetree/bindings/input/hid-over-i2c.txt
@@ -26,7 +26,8 @@ device-specific compatible properties, which should be used in addition to the
 
 - compatible:
   * "wacom,w9013" (Wacom W9013 digitizer). Supports:
-    - vdd-supply
+    - vdd-supply (3.3V)
+    - vddl-supply (1.8V)
     - post-power-on-delay-ms
 
 - vdd-supply: phandle of the regulator that provides the supply voltage.
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index c1652bb7bd15..1d83531454c5 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -1002,18 +1002,21 @@ static int i2c_hid_probe(struct i2c_client *client,
 		return client->irq;
 	}
 
-	ihid = kzalloc(sizeof(struct i2c_hid), GFP_KERNEL);
+	ihid = devm_kzalloc(&client->dev, sizeof(*ihid), GFP_KERNEL);
 	if (!ihid)
 		return -ENOMEM;
 
 	if (client->dev.of_node) {
 		ret = i2c_hid_of_probe(client, &ihid->pdata);
 		if (ret)
-			goto err;
+			return ret;
 	} else if (!platform_data) {
 		ret = i2c_hid_acpi_pdata(client, &ihid->pdata);
-		if (ret)
-			goto err;
+		if (ret) {
+			dev_err(&client->dev,
+				"HID register address not provided\n");
+			return ret;
+		}
 	} else {
 		ihid->pdata = *platform_data;
 	}
@@ -1021,21 +1024,20 @@ static int i2c_hid_probe(struct i2c_client *client,
 	/* Parse platform agnostic common properties from ACPI / device tree */
 	i2c_hid_fwnode_probe(client, &ihid->pdata);
 
-	ihid->pdata.supply = devm_regulator_get(&client->dev, "vdd");
-	if (IS_ERR(ihid->pdata.supply)) {
-		ret = PTR_ERR(ihid->pdata.supply);
-		if (ret != -EPROBE_DEFER)
-			dev_err(&client->dev, "Failed to get regulator: %d\n",
-				ret);
-		goto err;
-	}
+	ihid->pdata.supplies[0].supply = "vdd";
+	ihid->pdata.supplies[1].supply = "vddl";
+
+	ret = devm_regulator_bulk_get(&client->dev,
+				      ARRAY_SIZE(ihid->pdata.supplies),
+				      ihid->pdata.supplies);
+	if (ret)
+		return ret;
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(ihid->pdata.supplies),
+				    ihid->pdata.supplies);
+	if (ret < 0)
+		return ret;
 
-	ret = regulator_enable(ihid->pdata.supply);
-	if (ret < 0) {
-		dev_err(&client->dev, "Failed to enable regulator: %d\n",
-			ret);
-		goto err;
-	}
 	if (ihid->pdata.post_power_delay_ms)
 		msleep(ihid->pdata.post_power_delay_ms);
 
@@ -1122,11 +1124,9 @@ static int i2c_hid_probe(struct i2c_client *client,
 	pm_runtime_disable(&client->dev);
 
 err_regulator:
-	regulator_disable(ihid->pdata.supply);
-
-err:
+	regulator_bulk_disable(ARRAY_SIZE(ihid->pdata.supplies),
+			       ihid->pdata.supplies);
 	i2c_hid_free_buffers(ihid);
-	kfree(ihid);
 	return ret;
 }
 
@@ -1148,9 +1148,8 @@ static int i2c_hid_remove(struct i2c_client *client)
 	if (ihid->bufsize)
 		i2c_hid_free_buffers(ihid);
 
-	regulator_disable(ihid->pdata.supply);
-
-	kfree(ihid);
+	regulator_bulk_disable(ARRAY_SIZE(ihid->pdata.supplies),
+			       ihid->pdata.supplies);
 
 	return 0;
 }
@@ -1201,9 +1200,8 @@ static int i2c_hid_suspend(struct device *dev)
 			hid_warn(hid, "Failed to enable irq wake: %d\n",
 				wake_status);
 	} else {
-		ret = regulator_disable(ihid->pdata.supply);
-		if (ret < 0)
-			hid_warn(hid, "Failed to disable supply: %d\n", ret);
+		regulator_bulk_disable(ARRAY_SIZE(ihid->pdata.supplies),
+				       ihid->pdata.supplies);
 	}
 
 	return 0;
@@ -1218,9 +1216,11 @@ static int i2c_hid_resume(struct device *dev)
 	int wake_status;
 
 	if (!device_may_wakeup(&client->dev)) {
-		ret = regulator_enable(ihid->pdata.supply);
-		if (ret < 0)
-			hid_warn(hid, "Failed to enable supply: %d\n", ret);
+		ret = regulator_bulk_enable(ARRAY_SIZE(ihid->pdata.supplies),
+					    ihid->pdata.supplies);
+		if (ret)
+			hid_warn(hid, "Failed to enable supplies: %d\n", ret);
+
 		if (ihid->pdata.post_power_delay_ms)
 			msleep(ihid->pdata.post_power_delay_ms);
 	} else if (ihid->irq_wake_enabled) {
diff --git a/include/linux/platform_data/i2c-hid.h b/include/linux/platform_data/i2c-hid.h
index 1fb088239d12..c628bb5e1061 100644
--- a/include/linux/platform_data/i2c-hid.h
+++ b/include/linux/platform_data/i2c-hid.h
@@ -12,14 +12,13 @@
 #ifndef __LINUX_I2C_HID_H
 #define __LINUX_I2C_HID_H
 
+#include <linux/regulator/consumer.h>
 #include <linux/types.h>
 
-struct regulator;
-
 /**
  * struct i2chid_platform_data - used by hid over i2c implementation.
  * @hid_descriptor_address: i2c register where the HID descriptor is stored.
- * @supply: regulator for powering on the device.
+ * @supplies: regulators for powering on the device.
  * @post_power_delay_ms: delay after powering on before device is usable.
  *
  * Note that it is the responsibility of the platform driver (or the acpi 5.0
@@ -35,7 +34,7 @@ struct regulator;
  */
 struct i2c_hid_platform_data {
 	u16 hid_descriptor_address;
-	struct regulator *supply;
+	struct regulator_bulk_data supplies[2];
 	int post_power_delay_ms;
 };
 
-- 
Sent by a computer through tubes

^ permalink raw reply related

* [PATCH v2 10/10] Input: ams_delta_serio: Get FIQ buffer from platform_data
From: Janusz Krzysztofik @ 2018-06-21 22:41 UTC (permalink / raw)
  To: Dmitry Torokhov, Tony Lindgren, Aaro Koskinen
  Cc: David S . Miller , Mauro Carvalho Chehab, Greg Kroah-Hartman,
	Andrew Morton, Randy Dunlap, Linus Walleij, Mark Brown,
	Liam Girdwood, linux-arm-kernel, linux-omap, linux-input,
	linux-gpio, linux-kernel, Janusz Krzysztofik
In-Reply-To: <20180621224128.17623-1-jmkrzyszt@gmail.com>

Instead of exporting the FIQ buffer symbol to be used in
ams-delta-serio driver, pass it to the driver as platform_data.

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
Changelog:
v2: rebased on v4.18-rc1, no conflicts

 arch/arm/mach-omap1/ams-delta-fiq.c         |  6 +++---
 arch/arm/mach-omap1/board-ams-delta.c       |  8 ++++++++
 drivers/input/serio/ams_delta_serio.c       | 20 +++++++++++++-------
 include/linux/platform_data/ams-delta-fiq.h |  4 ----
 4 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-omap1/ams-delta-fiq.c b/arch/arm/mach-omap1/ams-delta-fiq.c
index 82ca4246a5e4..b0dc7ddf5877 100644
--- a/arch/arm/mach-omap1/ams-delta-fiq.c
+++ b/arch/arm/mach-omap1/ams-delta-fiq.c
@@ -40,8 +40,7 @@ static struct fiq_handler fh = {
  * keystrokes received from the qwerty keyboard.  See
  * <linux/platform_data/ams-delta-fiq.h> for details of offsets.
  */
-unsigned int fiq_buffer[1024];
-EXPORT_SYMBOL(fiq_buffer);
+static unsigned int fiq_buffer[1024];
 
 static struct irq_chip *irq_chip;
 static struct irq_data *irq_data[16];
@@ -203,9 +202,10 @@ void __init ams_delta_init_fiq(struct gpio_chip *chip,
 	val = omap_readl(OMAP_IH1_BASE + offset) | 1;
 	omap_writel(val, OMAP_IH1_BASE + offset);
 
-	/* Initialize serio device IRQ resource */
+	/* Initialize serio device IRQ resource and platform_data */
 	serio->resource[0].start = gpiod_to_irq(clk);
 	serio->resource[0].end = serio->resource[0].start;
+	serio->dev.platform_data = fiq_buffer;
 
 	/*
 	 * Since FIQ handler performs handling of GPIO registers for
diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index 84177ba3e39a..772892487827 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -520,6 +520,14 @@ static struct resource ams_delta_serio_resources[] = {
 static struct platform_device ams_delta_serio_device = {
 	.name		= "ams-delta-serio",
 	.id		= PLATFORM_DEVID_NONE,
+	.dev		= {
+		/*
+		 * Initialize .platform_data explicitly with NULL to
+		 * indicate it is going to be used.  It will be replaced
+		 * with FIQ buffer address as soon as FIQ is initialized.
+		 */
+		.platform_data = NULL,
+	},
 	.num_resources	= ARRAY_SIZE(ams_delta_serio_resources),
 	.resource	= ams_delta_serio_resources,
 };
diff --git a/drivers/input/serio/ams_delta_serio.c b/drivers/input/serio/ams_delta_serio.c
index c1f8226f172e..f8663d7891f2 100644
--- a/drivers/input/serio/ams_delta_serio.c
+++ b/drivers/input/serio/ams_delta_serio.c
@@ -37,6 +37,7 @@ MODULE_LICENSE("GPL");
 struct ams_delta_serio {
 	struct serio *serio;
 	struct regulator *vcc;
+	unsigned int *fiq_buffer;
 };
 
 static int check_data(struct serio *serio, int data)
@@ -66,22 +67,23 @@ static int check_data(struct serio *serio, int data)
 static irqreturn_t ams_delta_serio_interrupt(int irq, void *dev_id)
 {
 	struct ams_delta_serio *priv = dev_id;
-	int *circ_buff = &fiq_buffer[FIQ_CIRC_BUFF];
+	int *circ_buff = &priv->fiq_buffer[FIQ_CIRC_BUFF];
 	int data, dfl;
 	u8 scancode;
 
-	fiq_buffer[FIQ_IRQ_PEND] = 0;
+	priv->fiq_buffer[FIQ_IRQ_PEND] = 0;
 
 	/*
 	 * Read data from the circular buffer, check it
 	 * and then pass it on the serio
 	 */
-	while (fiq_buffer[FIQ_KEYS_CNT] > 0) {
+	while (priv->fiq_buffer[FIQ_KEYS_CNT] > 0) {
 
-		data = circ_buff[fiq_buffer[FIQ_HEAD_OFFSET]++];
-		fiq_buffer[FIQ_KEYS_CNT]--;
-		if (fiq_buffer[FIQ_HEAD_OFFSET] == fiq_buffer[FIQ_BUF_LEN])
-			fiq_buffer[FIQ_HEAD_OFFSET] = 0;
+		data = circ_buff[priv->fiq_buffer[FIQ_HEAD_OFFSET]++];
+		priv->fiq_buffer[FIQ_KEYS_CNT]--;
+		if (priv->fiq_buffer[FIQ_HEAD_OFFSET] ==
+		    priv->fiq_buffer[FIQ_BUF_LEN])
+			priv->fiq_buffer[FIQ_HEAD_OFFSET] = 0;
 
 		dfl = check_data(priv->serio, data);
 		scancode = (u8) (data >> 1) & 0xFF;
@@ -116,6 +118,10 @@ static int ams_delta_serio_init(struct platform_device *pdev)
 	if (!priv)
 		return -ENOMEM;
 
+	priv->fiq_buffer = pdev->dev.platform_data;
+	if (!priv->fiq_buffer)
+		return -EINVAL;
+
 	priv->vcc = devm_regulator_get(&pdev->dev, "vcc");
 	if (IS_ERR(priv->vcc)) {
 		err = PTR_ERR(priv->vcc);
diff --git a/include/linux/platform_data/ams-delta-fiq.h b/include/linux/platform_data/ams-delta-fiq.h
index dc0f835ea918..cf4589ccb720 100644
--- a/include/linux/platform_data/ams-delta-fiq.h
+++ b/include/linux/platform_data/ams-delta-fiq.h
@@ -55,8 +55,4 @@
 
 #define FIQ_CIRC_BUFF		30      /*Start of circular buffer */
 
-#ifndef __ASSEMBLER__
-extern unsigned int fiq_buffer[];
-#endif
-
 #endif
-- 
2.16.4

^ permalink raw reply related

* [PATCH v2 09/10] Input: ams_delta_serio: use IRQ resource
From: Janusz Krzysztofik @ 2018-06-21 22:41 UTC (permalink / raw)
  To: Dmitry Torokhov, Tony Lindgren, Aaro Koskinen
  Cc: David S . Miller , Mauro Carvalho Chehab, Greg Kroah-Hartman,
	Andrew Morton, Randy Dunlap, Linus Walleij, Mark Brown,
	Liam Girdwood, linux-arm-kernel, linux-omap, linux-input,
	linux-gpio, linux-kernel, Janusz Krzysztofik
In-Reply-To: <20180621224128.17623-1-jmkrzyszt@gmail.com>

The driver still obtains IRQ number from a hardcoded GPIO.  Use IRQ
resource instead.

For this to work on Amstrad Delta, add the IRQ resource to
ams-delta-serio platform device structure.  Obtain the IRQ number
assigned to "keybrd_clk" GPIO pin from FIQ initialization routine.

As a benefit, the driver no longer needs to include
<mach/board-ams-delta.h>.

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
Changelog:
v2: - obsolete assignment of handle_simple_irq() removed from the driver
      and replaced with a comment in the FIQ init file,
    - rebased on v4.18-rc1, no conflicts.

 arch/arm/mach-omap1/ams-delta-fiq.c   | 20 +++++++++++++++++++-
 arch/arm/mach-omap1/ams-delta-fiq.h   |  3 ++-
 arch/arm/mach-omap1/board-ams-delta.c | 17 ++++++++++++++++-
 drivers/input/serio/ams_delta_serio.c | 32 +++++++++-----------------------
 4 files changed, 46 insertions(+), 26 deletions(-)

diff --git a/arch/arm/mach-omap1/ams-delta-fiq.c b/arch/arm/mach-omap1/ams-delta-fiq.c
index e72935034d42..82ca4246a5e4 100644
--- a/arch/arm/mach-omap1/ams-delta-fiq.c
+++ b/arch/arm/mach-omap1/ams-delta-fiq.c
@@ -20,6 +20,7 @@
 #include <linux/module.h>
 #include <linux/io.h>
 #include <linux/platform_data/ams-delta-fiq.h>
+#include <linux/platform_device.h>
 
 #include <mach/board-ams-delta.h>
 
@@ -84,7 +85,8 @@ static irqreturn_t deferred_fiq(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-void __init ams_delta_init_fiq(struct gpio_chip *chip)
+void __init ams_delta_init_fiq(struct gpio_chip *chip,
+			       struct platform_device *serio)
 {
 	struct gpio_desc *gpiod, *data = NULL, *clk = NULL;
 	void *fiqhandler_start;
@@ -201,6 +203,22 @@ void __init ams_delta_init_fiq(struct gpio_chip *chip)
 	val = omap_readl(OMAP_IH1_BASE + offset) | 1;
 	omap_writel(val, OMAP_IH1_BASE + offset);
 
+	/* Initialize serio device IRQ resource */
+	serio->resource[0].start = gpiod_to_irq(clk);
+	serio->resource[0].end = serio->resource[0].start;
+
+	/*
+	 * Since FIQ handler performs handling of GPIO registers for
+	 * "keybrd_clk" IRQ pin, ams_delta_serio driver used to set
+	 * handle_simple_irq() as active IRQ handler for that pin to avoid
+	 * bad interaction with gpio-omap driver.  This is no longer needed
+	 * as handle_simple_irq() is now the default handler for OMAP GPIO
+	 * edge interrupts.
+	 * This comment replaces the obsolete code which has been removed
+	 * from the ams_delta_serio driver and stands here only as a reminder
+	 * of that dependency on gpio-omap driver behavior.
+	 */
+
 	return;
 
 out_gpio:
diff --git a/arch/arm/mach-omap1/ams-delta-fiq.h b/arch/arm/mach-omap1/ams-delta-fiq.h
index 3f691d68aa62..fd76df3cce37 100644
--- a/arch/arm/mach-omap1/ams-delta-fiq.h
+++ b/arch/arm/mach-omap1/ams-delta-fiq.h
@@ -35,7 +35,8 @@
 #ifndef __ASSEMBLER__
 extern unsigned char qwerty_fiqin_start, qwerty_fiqin_end;
 
-extern void __init ams_delta_init_fiq(struct gpio_chip *chip);
+extern void __init ams_delta_init_fiq(struct gpio_chip *chip,
+				      struct platform_device *pdev);
 #endif
 
 #endif
diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index fe9a3e7cbfeb..84177ba3e39a 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -504,9 +504,24 @@ static struct platform_device cx20442_codec_device = {
 	.id     = -1,
 };
 
+static struct resource ams_delta_serio_resources[] = {
+	{
+		.flags	= IORESOURCE_IRQ,
+		/*
+		 * Initialize IRQ resource with invalid IRQ number.
+		 * It will be replaced with dynamically allocated GPIO IRQ
+		 * obtained from GPIO chip as soon as the chip is available.
+		 */
+		.start	= -EINVAL,
+		.end	= -EINVAL,
+	},
+};
+
 static struct platform_device ams_delta_serio_device = {
 	.name		= "ams-delta-serio",
 	.id		= PLATFORM_DEVID_NONE,
+	.num_resources	= ARRAY_SIZE(ams_delta_serio_resources),
+	.resource	= ams_delta_serio_resources,
 };
 
 static struct regulator_consumer_supply keybrd_pwr_consumers[] = {
@@ -615,7 +630,7 @@ static void __init omap_gpio_deps_init(void)
 		return;
 	}
 
-	ams_delta_init_fiq(chip);
+	ams_delta_init_fiq(chip, &ams_delta_serio_device);
 }
 
 static void __init ams_delta_init(void)
diff --git a/drivers/input/serio/ams_delta_serio.c b/drivers/input/serio/ams_delta_serio.c
index 2602f7cff5ae..c1f8226f172e 100644
--- a/drivers/input/serio/ams_delta_serio.c
+++ b/drivers/input/serio/ams_delta_serio.c
@@ -20,7 +20,6 @@
  * However, when used with the E3 mailboard that producecs non-standard
  * scancodes, a custom key table must be prepared and loaded from userspace.
  */
-#include <linux/gpio.h>
 #include <linux/irq.h>
 #include <linux/platform_data/ams-delta-fiq.h>
 #include <linux/platform_device.h>
@@ -29,8 +28,6 @@
 #include <linux/slab.h>
 #include <linux/module.h>
 
-#include <mach/board-ams-delta.h>
-
 #define DRIVER_NAME	"ams-delta-serio"
 
 MODULE_AUTHOR("Matt Callow");
@@ -113,7 +110,7 @@ static int ams_delta_serio_init(struct platform_device *pdev)
 {
 	struct ams_delta_serio *priv;
 	struct serio *serio;
-	int err;
+	int irq, err;
 
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -139,26 +136,20 @@ static int ams_delta_serio_init(struct platform_device *pdev)
 		return err;
 	}
 
-	err = request_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK),
-			ams_delta_serio_interrupt, IRQ_TYPE_EDGE_RISING,
-			DRIVER_NAME, priv);
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return -ENXIO;
+
+	err = devm_request_irq(&pdev->dev, irq, ams_delta_serio_interrupt,
+			       IRQ_TYPE_EDGE_RISING, DRIVER_NAME, priv);
 	if (err < 0) {
 		dev_err(&pdev->dev, "IRQ request failed (%d)\n", err);
 		return err;
 	}
-	/*
-	 * Since GPIO register handling for keyboard clock pin is performed
-	 * at FIQ level, switch back from edge to simple interrupt handler
-	 * to avoid bad interaction.
-	 */
-	irq_set_handler(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK),
-			handle_simple_irq);
 
 	serio = kzalloc(sizeof(*serio), GFP_KERNEL);
-	if (!serio) {
-		err = -ENOMEM;
-		goto irq;
-	}
+	if (!serio)
+		return -ENOMEM;
 
 	priv->serio = serio;
 
@@ -177,10 +168,6 @@ static int ams_delta_serio_init(struct platform_device *pdev)
 	dev_info(&serio->dev, "%s\n", serio->name);
 
 	return 0;
-
-irq:
-	free_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK), priv);
-	return err;
 }
 
 static int ams_delta_serio_exit(struct platform_device *pdev)
@@ -188,7 +175,6 @@ static int ams_delta_serio_exit(struct platform_device *pdev)
 	struct ams_delta_serio *priv = platform_get_drvdata(pdev);
 
 	serio_unregister_port(priv->serio);
-	free_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK), 0);
 
 	return 0;
 }
-- 
2.16.4

^ permalink raw reply related

* [PATCH v2 08/10] ARM: OMAP1: Get rid of <mach/ams-delta-fiq.h>
From: Janusz Krzysztofik @ 2018-06-21 22:41 UTC (permalink / raw)
  To: Dmitry Torokhov, Tony Lindgren, Aaro Koskinen
  Cc: David S . Miller , Mauro Carvalho Chehab, Greg Kroah-Hartman,
	Andrew Morton, Randy Dunlap, Linus Walleij, Mark Brown,
	Liam Girdwood, linux-arm-kernel, linux-omap, linux-input,
	linux-gpio, linux-kernel, Janusz Krzysztofik
In-Reply-To: <20180621224128.17623-1-jmkrzyszt@gmail.com>

Split the header file into two parts and move them to directories where
they belong.

Information on internal structure of FIQ buffer is moved to
<linux/platform_data/ams-delta-fiq.h> for ams-delta-serio driver use.

Other information used by ams-delta board init file and FIQ code is
made local to mach-omap1 root directory.

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
Changelog:
v2: rebased on v4.18-rc1, no conflicts

 MAINTAINERS                                        |  1 +
 arch/arm/mach-omap1/ams-delta-fiq-handler.S        |  5 +--
 arch/arm/mach-omap1/ams-delta-fiq.c                |  7 ++--
 arch/arm/mach-omap1/ams-delta-fiq.h                | 41 ++++++++++++++++++++++
 arch/arm/mach-omap1/board-ams-delta.c              |  2 +-
 drivers/input/serio/ams_delta_serio.c              |  3 +-
 .../linux/platform_data}/ams-delta-fiq.h           | 27 +++-----------
 7 files changed, 56 insertions(+), 30 deletions(-)
 create mode 100644 arch/arm/mach-omap1/ams-delta-fiq.h
 rename {arch/arm/mach-omap1/include/mach => include/linux/platform_data}/ams-delta-fiq.h (71%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9d5eeff51b5f..c78b7dc42a81 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10391,6 +10391,7 @@ F:	arch/arm/plat-omap/
 F:	arch/arm/configs/omap1_defconfig
 F:	drivers/i2c/busses/i2c-omap.c
 F:	include/linux/platform_data/i2c-omap.h
+F:	include/linux/platform_data/ams-delta-fiq.h
 
 OMAP2+ SUPPORT
 M:	Tony Lindgren <tony@atomide.com>
diff --git a/arch/arm/mach-omap1/ams-delta-fiq-handler.S b/arch/arm/mach-omap1/ams-delta-fiq-handler.S
index bf608441b357..ddc27638ba2a 100644
--- a/arch/arm/mach-omap1/ams-delta-fiq-handler.S
+++ b/arch/arm/mach-omap1/ams-delta-fiq-handler.S
@@ -14,11 +14,12 @@
  */
 
 #include <linux/linkage.h>
-#include <asm/assembler.h>
+#include <linux/platform_data/ams-delta-fiq.h>
 
+#include <asm/assembler.h>
 #include <mach/board-ams-delta.h>
-#include <mach/ams-delta-fiq.h>
 
+#include "ams-delta-fiq.h"
 #include "iomap.h"
 #include "soc.h"
 
diff --git a/arch/arm/mach-omap1/ams-delta-fiq.c b/arch/arm/mach-omap1/ams-delta-fiq.c
index 5a6c59ac9b5f..e72935034d42 100644
--- a/arch/arm/mach-omap1/ams-delta-fiq.c
+++ b/arch/arm/mach-omap1/ams-delta-fiq.c
@@ -19,12 +19,13 @@
 #include <linux/irq.h>
 #include <linux/module.h>
 #include <linux/io.h>
+#include <linux/platform_data/ams-delta-fiq.h>
 
 #include <mach/board-ams-delta.h>
 
 #include <asm/fiq.h>
 
-#include <mach/ams-delta-fiq.h>
+#include "ams-delta-fiq.h"
 
 static struct fiq_handler fh = {
 	.name	= "ams-delta-fiq"
@@ -35,8 +36,8 @@ static struct fiq_handler fh = {
  * The FIQ and IRQ isrs can both read and write it.
  * It is structured as a header section several 32bit slots,
  * followed by the circular buffer where the FIQ isr stores
- * keystrokes received from the qwerty keyboard.
- * See ams-delta-fiq.h for details of offsets.
+ * keystrokes received from the qwerty keyboard.  See
+ * <linux/platform_data/ams-delta-fiq.h> for details of offsets.
  */
 unsigned int fiq_buffer[1024];
 EXPORT_SYMBOL(fiq_buffer);
diff --git a/arch/arm/mach-omap1/ams-delta-fiq.h b/arch/arm/mach-omap1/ams-delta-fiq.h
new file mode 100644
index 000000000000..3f691d68aa62
--- /dev/null
+++ b/arch/arm/mach-omap1/ams-delta-fiq.h
@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * arch/arm/mach-omap1/ams-delta-fiq.h
+ *
+ * Taken from the original Amstrad modifications to fiq.h
+ *
+ * Copyright (c) 2004 Amstrad Plc
+ * Copyright (c) 2006 Matt Callow
+ * Copyright (c) 2010 Janusz Krzysztofik
+ *
+ * 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.
+ */
+#ifndef __AMS_DELTA_FIQ_H
+#define __AMS_DELTA_FIQ_H
+
+#include <mach/irqs.h>
+
+/*
+ * Interrupt number used for passing control from FIQ to IRQ.
+ * IRQ12, described as reserved, has been selected.
+ */
+#define INT_DEFERRED_FIQ	INT_1510_RES12
+/*
+ * Base address of an interrupt handler that the INT_DEFERRED_FIQ belongs to.
+ */
+#if (INT_DEFERRED_FIQ < IH2_BASE)
+#define DEFERRED_FIQ_IH_BASE	OMAP_IH1_BASE
+#else
+#define DEFERRED_FIQ_IH_BASE	OMAP_IH2_BASE
+#endif
+
+#ifndef __ASSEMBLER__
+extern unsigned char qwerty_fiqin_start, qwerty_fiqin_end;
+
+extern void __init ams_delta_init_fiq(struct gpio_chip *chip);
+#endif
+
+#endif
diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index f15c0793c34b..fe9a3e7cbfeb 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -41,10 +41,10 @@
 #include <mach/mux.h>
 
 #include <mach/hardware.h>
-#include <mach/ams-delta-fiq.h>
 #include "camera.h"
 #include <mach/usb.h>
 
+#include "ams-delta-fiq.h"
 #include "iomap.h"
 #include "common.h"
 
diff --git a/drivers/input/serio/ams_delta_serio.c b/drivers/input/serio/ams_delta_serio.c
index 7952a29f9540..2602f7cff5ae 100644
--- a/drivers/input/serio/ams_delta_serio.c
+++ b/drivers/input/serio/ams_delta_serio.c
@@ -22,6 +22,7 @@
  */
 #include <linux/gpio.h>
 #include <linux/irq.h>
+#include <linux/platform_data/ams-delta-fiq.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
 #include <linux/serio.h>
@@ -30,8 +31,6 @@
 
 #include <mach/board-ams-delta.h>
 
-#include <mach/ams-delta-fiq.h>
-
 #define DRIVER_NAME	"ams-delta-serio"
 
 MODULE_AUTHOR("Matt Callow");
diff --git a/arch/arm/mach-omap1/include/mach/ams-delta-fiq.h b/include/linux/platform_data/ams-delta-fiq.h
similarity index 71%
rename from arch/arm/mach-omap1/include/mach/ams-delta-fiq.h
rename to include/linux/platform_data/ams-delta-fiq.h
index a9769ff396bc..dc0f835ea918 100644
--- a/arch/arm/mach-omap1/include/mach/ams-delta-fiq.h
+++ b/include/linux/platform_data/ams-delta-fiq.h
@@ -1,5 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
 /*
- * arch/arm/mach-omap1/include/ams-delta-fiq.h
+ * include/linux/platform_data/ams-delta-fiq.h
  *
  * Taken from the original Amstrad modifications to fiq.h
  *
@@ -11,24 +13,8 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
-#ifndef __AMS_DELTA_FIQ_H
-#define __AMS_DELTA_FIQ_H
-
-#include <mach/irqs.h>
-
-/*
- * Interrupt number used for passing control from FIQ to IRQ.
- * IRQ12, described as reserved, has been selected.
- */
-#define INT_DEFERRED_FIQ	INT_1510_RES12
-/*
- * Base address of an interrupt handler that the INT_DEFERRED_FIQ belongs to.
- */
-#if (INT_DEFERRED_FIQ < IH2_BASE)
-#define DEFERRED_FIQ_IH_BASE	OMAP_IH1_BASE
-#else
-#define DEFERRED_FIQ_IH_BASE	OMAP_IH2_BASE
-#endif
+#ifndef __LINUX_PLATFORM_DATA_AMS_DELTA_FIQ_H
+#define __LINUX_PLATFORM_DATA_AMS_DELTA_FIQ_H
 
 /*
  * These are the offsets from the beginning of the fiq_buffer. They are put here
@@ -71,9 +57,6 @@
 
 #ifndef __ASSEMBLER__
 extern unsigned int fiq_buffer[];
-extern unsigned char qwerty_fiqin_start, qwerty_fiqin_end;
-
-extern void __init ams_delta_init_fiq(struct gpio_chip *chip);
 #endif
 
 #endif
-- 
2.16.4

^ permalink raw reply related

* [PATCH v2 07/10] ARM: OMAP1: ams-delta FIQ: Keep serio input GPIOs requested
From: Janusz Krzysztofik @ 2018-06-21 22:41 UTC (permalink / raw)
  To: Dmitry Torokhov, Tony Lindgren, Aaro Koskinen
  Cc: David S . Miller , Mauro Carvalho Chehab, Greg Kroah-Hartman,
	Andrew Morton, Randy Dunlap, Linus Walleij, Mark Brown,
	Liam Girdwood, linux-arm-kernel, linux-omap, linux-input,
	linux-gpio, linux-kernel, Janusz Krzysztofik
In-Reply-To: <20180621224128.17623-1-jmkrzyszt@gmail.com>

>From the very beginning, input GPIO pins of ams-delta serio port have
been used by FIQ handler, not serio driver.

Don't request those pins from the ams-delta-serio driver any longer,
instead keep them requested and initialized by the FIQ initialization
routine which already requests them and releases while identifying GPIO
IRQs.

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
Changelog:
v2: rebased on v4.18-rc1, no conflicts

 arch/arm/mach-omap1/ams-delta-fiq.c   | 42 ++++++++++++++++++++++++++++++-----
 drivers/input/serio/ams_delta_serio.c | 30 ++-----------------------
 2 files changed, 39 insertions(+), 33 deletions(-)

diff --git a/arch/arm/mach-omap1/ams-delta-fiq.c b/arch/arm/mach-omap1/ams-delta-fiq.c
index 1d54a6177f14..5a6c59ac9b5f 100644
--- a/arch/arm/mach-omap1/ams-delta-fiq.c
+++ b/arch/arm/mach-omap1/ams-delta-fiq.c
@@ -45,6 +45,11 @@ static struct irq_chip *irq_chip;
 static struct irq_data *irq_data[16];
 static unsigned int irq_counter[16];
 
+static const char *pin_name[16] __initconst = {
+	[AMS_DELTA_GPIO_PIN_KEYBRD_DATA]	= "keybrd_data",
+	[AMS_DELTA_GPIO_PIN_KEYBRD_CLK]		= "keybrd_clk",
+};
+
 static irqreturn_t deferred_fiq(int irq, void *dev_id)
 {
 	struct irq_data *d;
@@ -80,7 +85,7 @@ static irqreturn_t deferred_fiq(int irq, void *dev_id)
 
 void __init ams_delta_init_fiq(struct gpio_chip *chip)
 {
-	struct gpio_desc *gpiod;
+	struct gpio_desc *gpiod, *data = NULL, *clk = NULL;
 	void *fiqhandler_start;
 	unsigned int fiqhandler_length;
 	struct pt_regs FIQ_regs;
@@ -96,7 +101,7 @@ void __init ams_delta_init_fiq(struct gpio_chip *chip)
 	}
 
 	for (i = 0; i < ARRAY_SIZE(irq_data); i++) {
-		gpiod = gpiochip_request_own_desc(chip, i, NULL);
+		gpiod = gpiochip_request_own_desc(chip, i, pin_name[i]);
 		if (IS_ERR(gpiod)) {
 			pr_err("%s: failed to get GPIO pin %d (%ld)\n",
 			       __func__, i, PTR_ERR(gpiod));
@@ -105,8 +110,27 @@ void __init ams_delta_init_fiq(struct gpio_chip *chip)
 		/* Store irq_data location for IRQ handler use */
 		irq_data[i] = irq_get_irq_data(gpiod_to_irq(gpiod));
 
-		gpiochip_free_own_desc(gpiod);
+		/*
+		 * FIQ handler takes full control over serio data and clk GPIO
+		 * pins.  Initiaize them and keep requested so nobody can
+		 * interfere.  Fail if any of those two couldn't be requested.
+		 */
+		switch (i) {
+		case AMS_DELTA_GPIO_PIN_KEYBRD_DATA:
+			data = gpiod;
+			gpiod_direction_input(data);
+			break;
+		case AMS_DELTA_GPIO_PIN_KEYBRD_CLK:
+			clk = gpiod;
+			gpiod_direction_input(clk);
+			break;
+		default:
+			gpiochip_free_own_desc(gpiod);
+			break;
+		}
 	}
+	if (!data || !clk)
+		goto out_gpio;
 
 	fiqhandler_start = &qwerty_fiqin_start;
 	fiqhandler_length = &qwerty_fiqin_end - &qwerty_fiqin_start;
@@ -117,7 +141,7 @@ void __init ams_delta_init_fiq(struct gpio_chip *chip)
 	if (retval) {
 		pr_err("ams_delta_init_fiq(): couldn't claim FIQ, ret=%d\n",
 				retval);
-		return;
+		goto out_gpio;
 	}
 
 	retval = request_irq(INT_DEFERRED_FIQ, deferred_fiq,
@@ -125,7 +149,7 @@ void __init ams_delta_init_fiq(struct gpio_chip *chip)
 	if (retval < 0) {
 		pr_err("Failed to get deferred_fiq IRQ, ret=%d\n", retval);
 		release_fiq(&fh);
-		return;
+		goto out_gpio;
 	}
 	/*
 	 * Since no set_type() method is provided by OMAP irq chip,
@@ -175,4 +199,12 @@ void __init ams_delta_init_fiq(struct gpio_chip *chip)
 	offset = IRQ_ILR0_REG_OFFSET + (INT_GPIO_BANK1 - NR_IRQS_LEGACY) * 0x4;
 	val = omap_readl(OMAP_IH1_BASE + offset) | 1;
 	omap_writel(val, OMAP_IH1_BASE + offset);
+
+	return;
+
+out_gpio:
+	if (data)
+		gpiochip_free_own_desc(data);
+	if (clk)
+		gpiochip_free_own_desc(clk);
 }
diff --git a/drivers/input/serio/ams_delta_serio.c b/drivers/input/serio/ams_delta_serio.c
index b955c6a72e99..7952a29f9540 100644
--- a/drivers/input/serio/ams_delta_serio.c
+++ b/drivers/input/serio/ams_delta_serio.c
@@ -110,19 +110,6 @@ static void ams_delta_serio_close(struct serio *serio)
 	regulator_disable(priv->vcc);
 }
 
-static const struct gpio ams_delta_gpios[] __initconst_or_module = {
-	{
-		.gpio	= AMS_DELTA_GPIO_PIN_KEYBRD_DATA,
-		.flags	= GPIOF_DIR_IN,
-		.label	= "serio-data",
-	},
-	{
-		.gpio	= AMS_DELTA_GPIO_PIN_KEYBRD_CLK,
-		.flags	= GPIOF_DIR_IN,
-		.label	= "serio-clock",
-	},
-};
-
 static int ams_delta_serio_init(struct platform_device *pdev)
 {
 	struct ams_delta_serio *priv;
@@ -133,13 +120,6 @@ static int ams_delta_serio_init(struct platform_device *pdev)
 	if (!priv)
 		return -ENOMEM;
 
-	err = gpio_request_array(ams_delta_gpios,
-				ARRAY_SIZE(ams_delta_gpios));
-	if (err) {
-		dev_err(&pdev->dev, "Couldn't request gpio pins\n");
-		goto serio;
-	}
-
 	priv->vcc = devm_regulator_get(&pdev->dev, "vcc");
 	if (IS_ERR(priv->vcc)) {
 		err = PTR_ERR(priv->vcc);
@@ -157,7 +137,7 @@ static int ams_delta_serio_init(struct platform_device *pdev)
 		 */
 		if (err == -ENODEV)
 			err = -EPROBE_DEFER;
-		goto gpio;
+		return err;
 	}
 
 	err = request_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK),
@@ -165,7 +145,7 @@ static int ams_delta_serio_init(struct platform_device *pdev)
 			DRIVER_NAME, priv);
 	if (err < 0) {
 		dev_err(&pdev->dev, "IRQ request failed (%d)\n", err);
-		goto gpio;
+		return err;
 	}
 	/*
 	 * Since GPIO register handling for keyboard clock pin is performed
@@ -201,10 +181,6 @@ static int ams_delta_serio_init(struct platform_device *pdev)
 
 irq:
 	free_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK), priv);
-gpio:
-	gpio_free_array(ams_delta_gpios,
-			ARRAY_SIZE(ams_delta_gpios));
-serio:
 	return err;
 }
 
@@ -214,8 +190,6 @@ static int ams_delta_serio_exit(struct platform_device *pdev)
 
 	serio_unregister_port(priv->serio);
 	free_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK), 0);
-	gpio_free_array(ams_delta_gpios,
-			ARRAY_SIZE(ams_delta_gpios));
 
 	return 0;
 }
-- 
2.16.4

^ permalink raw reply related

* [PATCH v2 06/10] ARM: OMAP1: ams-delta FIQ: don't use static GPIO numbers
From: Janusz Krzysztofik @ 2018-06-21 22:41 UTC (permalink / raw)
  To: Dmitry Torokhov, Tony Lindgren, Aaro Koskinen
  Cc: David S . Miller , Mauro Carvalho Chehab, Greg Kroah-Hartman,
	Andrew Morton, Randy Dunlap, Linus Walleij, Mark Brown,
	Liam Girdwood, linux-arm-kernel, linux-omap, linux-input,
	linux-gpio, linux-kernel, Janusz Krzysztofik
In-Reply-To: <20180621224128.17623-1-jmkrzyszt@gmail.com>

With introduction of GPIO lookup tables to Amstrad Delta board init
file, semantics of symbols representing OMAP GPIO pins defined in
<mach/board-ams-delta.h> changed from statically assigned global GPIO
numbers to hardware pin numbers local to OMAP "gpio-0-15" chip.

This patch modifies deferred FIQ interrupt handler so it no longer uses
static GPIO numbers in favour of IRQ data descriptors obtained at FIQ
initialization time from descriptor of the GPIO chip with use of its
hardware pin numbers.  The chip descriptor is passed from the board
init file.

As a benefit, the deferred FIQ handler should work faster.

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
Changelog:
v2: rebased on v4.18-rc1, no conflicts

 arch/arm/mach-omap1/ams-delta-fiq.c              | 48 +++++++++++++++++-------
 arch/arm/mach-omap1/board-ams-delta.c            | 41 +++++++++++++++++++-
 arch/arm/mach-omap1/include/mach/ams-delta-fiq.h |  2 +-
 3 files changed, 74 insertions(+), 17 deletions(-)

diff --git a/arch/arm/mach-omap1/ams-delta-fiq.c b/arch/arm/mach-omap1/ams-delta-fiq.c
index d7ca9e2b40d2..1d54a6177f14 100644
--- a/arch/arm/mach-omap1/ams-delta-fiq.c
+++ b/arch/arm/mach-omap1/ams-delta-fiq.c
@@ -13,7 +13,8 @@
  * under the terms of the GNU General Public License version 2 as published by
  * the Free Software Foundation.
  */
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
 #include <linux/module.h>
@@ -40,14 +41,14 @@ static struct fiq_handler fh = {
 unsigned int fiq_buffer[1024];
 EXPORT_SYMBOL(fiq_buffer);
 
+static struct irq_chip *irq_chip;
+static struct irq_data *irq_data[16];
 static unsigned int irq_counter[16];
 
 static irqreturn_t deferred_fiq(int irq, void *dev_id)
 {
+	struct irq_data *d;
 	int gpio, irq_num, fiq_count;
-	struct irq_chip *irq_chip;
-
-	irq_chip = irq_get_chip(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK));
 
 	/*
 	 * For each handled GPIO interrupt, keep calling its interrupt handler
@@ -55,24 +56,21 @@ static irqreturn_t deferred_fiq(int irq, void *dev_id)
 	 */
 	for (gpio = AMS_DELTA_GPIO_PIN_KEYBRD_CLK;
 			gpio <= AMS_DELTA_GPIO_PIN_HOOK_SWITCH; gpio++) {
-		irq_num = gpio_to_irq(gpio);
+		d = irq_data[gpio];
+		irq_num = d->irq;
 		fiq_count = fiq_buffer[FIQ_CNT_INT_00 + gpio];
 
 		if (irq_counter[gpio] < fiq_count &&
 				gpio != AMS_DELTA_GPIO_PIN_KEYBRD_CLK) {
-			struct irq_data *d = irq_get_irq_data(irq_num);
-
 			/*
 			 * handle_simple_irq() that OMAP GPIO edge
 			 * interrupts default to since commit 80ac93c27441
 			 * requires interrupt already acked and unmasked.
 			 */
-			if (irq_chip) {
-				if (irq_chip->irq_ack)
-					irq_chip->irq_ack(d);
-				if (irq_chip->irq_unmask)
-					irq_chip->irq_unmask(d);
-			}
+			if (irq_chip->irq_ack)
+				irq_chip->irq_ack(d);
+			if (irq_chip->irq_unmask)
+				irq_chip->irq_unmask(d);
 		}
 		for (; irq_counter[gpio] < fiq_count; irq_counter[gpio]++)
 			generic_handle_irq(irq_num);
@@ -80,14 +78,36 @@ static irqreturn_t deferred_fiq(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-void __init ams_delta_init_fiq(void)
+void __init ams_delta_init_fiq(struct gpio_chip *chip)
 {
+	struct gpio_desc *gpiod;
 	void *fiqhandler_start;
 	unsigned int fiqhandler_length;
 	struct pt_regs FIQ_regs;
 	unsigned long val, offset;
 	int i, retval;
 
+	/* Store irq_chip location for IRQ handler use */
+	irq_chip = chip->irq.chip;
+	if (!irq_chip) {
+		pr_err("%s: GPIO chip %s is missing IRQ function\n", __func__,
+		       chip->label);
+		return;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(irq_data); i++) {
+		gpiod = gpiochip_request_own_desc(chip, i, NULL);
+		if (IS_ERR(gpiod)) {
+			pr_err("%s: failed to get GPIO pin %d (%ld)\n",
+			       __func__, i, PTR_ERR(gpiod));
+			return;
+		}
+		/* Store irq_data location for IRQ handler use */
+		irq_data[i] = irq_get_irq_data(gpiod_to_irq(gpiod));
+
+		gpiochip_free_own_desc(gpiod);
+	}
+
 	fiqhandler_start = &qwerty_fiqin_start;
 	fiqhandler_length = &qwerty_fiqin_end - &qwerty_fiqin_start;
 	pr_info("Installing fiq handler from %p, length 0x%x\n",
diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index 2821284aa0c9..f15c0793c34b 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -580,6 +580,44 @@ static struct gpiod_hog ams_delta_gpio_hogs[] = {
 	{},
 };
 
+/*
+ * Some drivers may not use GPIO lookup tables but need to be provided
+ * with GPIO numbers.  The same applies to GPIO based IRQ lines - some
+ * drivers may even not use GPIO layer but expect just IRQ numbers.
+ * We could either define GPIO lookup tables then use them on behalf
+ * of those devices, or we can use GPIO driver level methods for
+ * identification of GPIO and IRQ numbers. For the purpose of the latter,
+ * defina a helper function which identifies GPIO chips by their labels.
+ */
+static int gpiochip_match_by_label(struct gpio_chip *chip, void *data)
+{
+	char *label = data;
+
+	return !strcmp(label, chip->label);
+}
+
+/*
+ * The purpose of this function is to take care of proper initialization of
+ * devices and data structures which depend on GPIO lines provided by OMAP GPIO
+ * banks but their drivers don't use GPIO lookup tables or GPIO layer at all.
+ * The function may be called as soon as OMAP GPIO devices are probed.
+ * Since that happens at postcore_initcall, it can be called successfully
+ * from init_machine or later.
+ * Dependent devices may be registered from within this function or later.
+ */
+static void __init omap_gpio_deps_init(void)
+{
+	struct gpio_chip *chip;
+
+	chip = gpiochip_find(OMAP_GPIO_LABEL, gpiochip_match_by_label);
+	if (!chip) {
+		pr_err("%s: OMAP GPIO chip not found\n", __func__);
+		return;
+	}
+
+	ams_delta_init_fiq(chip);
+}
+
 static void __init ams_delta_init(void)
 {
 	/* mux pins for uarts */
@@ -600,6 +638,7 @@ static void __init ams_delta_init(void)
 	omap_cfg_reg(J19_1610_CAM_D6);
 	omap_cfg_reg(J18_1610_CAM_D7);
 
+	omap_gpio_deps_init();
 	gpiod_add_hogs(ams_delta_gpio_hogs);
 
 	omap_serial_init();
@@ -642,8 +681,6 @@ static void __init ams_delta_init(void)
 	gpiod_add_lookup_tables(ams_delta_gpio_tables,
 				ARRAY_SIZE(ams_delta_gpio_tables));
 
-	ams_delta_init_fiq();
-
 	omap_writew(omap_readw(ARM_RSTCT1) | 0x0004, ARM_RSTCT1);
 
 	omapfb_set_lcd_config(&ams_delta_lcd_config);
diff --git a/arch/arm/mach-omap1/include/mach/ams-delta-fiq.h b/arch/arm/mach-omap1/include/mach/ams-delta-fiq.h
index 6dfc3e1210a3..a9769ff396bc 100644
--- a/arch/arm/mach-omap1/include/mach/ams-delta-fiq.h
+++ b/arch/arm/mach-omap1/include/mach/ams-delta-fiq.h
@@ -73,7 +73,7 @@
 extern unsigned int fiq_buffer[];
 extern unsigned char qwerty_fiqin_start, qwerty_fiqin_end;
 
-extern void __init ams_delta_init_fiq(void);
+extern void __init ams_delta_init_fiq(struct gpio_chip *chip);
 #endif
 
 #endif
-- 
2.16.4

^ permalink raw reply related

* [PATCH v2 05/10] ARM: OMAP1: ams-delta: Hog "keybrd_dataout" GPIO pin
From: Janusz Krzysztofik @ 2018-06-21 22:41 UTC (permalink / raw)
  To: Dmitry Torokhov, Tony Lindgren, Aaro Koskinen
  Cc: David S . Miller , Mauro Carvalho Chehab, Greg Kroah-Hartman,
	Andrew Morton, Randy Dunlap, Linus Walleij, Mark Brown,
	Liam Girdwood, linux-arm-kernel, linux-omap, linux-input,
	linux-gpio, linux-kernel, Janusz Krzysztofik
In-Reply-To: <20180621224128.17623-1-jmkrzyszt@gmail.com>

"keybrd_dataout" GPIO pin used to be initialized by ams-delta-serio
driver to a state safe for ams-delta-serio device function and not
changed thereafter.  As such, it may be assumed not under the driver
control and responsibility for its initialization handed over to board
init file.

Introduce a GPIO hog table and take over control of the
"keybrd_dataout" GPIO pin from the ams-delta-serio driver.

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
Changelog:
v2: rebased on v4.18-rc1, no conflicts

 arch/arm/mach-omap1/board-ams-delta.c | 8 ++++++++
 drivers/input/serio/ams_delta_serio.c | 5 -----
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index 706eb2f9301d..2821284aa0c9 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -574,6 +574,12 @@ static struct gpiod_lookup_table *late_gpio_tables[] __initdata = {
 	&ams_delta_nand_gpio_table,
 };
 
+static struct gpiod_hog ams_delta_gpio_hogs[] = {
+	GPIO_HOG(LATCH2_LABEL, LATCH2_PIN_KEYBRD_DATAOUT, "keybrd_dataout",
+		 GPIO_ACTIVE_HIGH, GPIOD_OUT_LOW),
+	{},
+};
+
 static void __init ams_delta_init(void)
 {
 	/* mux pins for uarts */
@@ -594,6 +600,8 @@ static void __init ams_delta_init(void)
 	omap_cfg_reg(J19_1610_CAM_D6);
 	omap_cfg_reg(J18_1610_CAM_D7);
 
+	gpiod_add_hogs(ams_delta_gpio_hogs);
+
 	omap_serial_init();
 	omap_register_i2c_bus(1, 100, NULL, 0);
 
diff --git a/drivers/input/serio/ams_delta_serio.c b/drivers/input/serio/ams_delta_serio.c
index 854d0d3ada52..b955c6a72e99 100644
--- a/drivers/input/serio/ams_delta_serio.c
+++ b/drivers/input/serio/ams_delta_serio.c
@@ -121,11 +121,6 @@ static const struct gpio ams_delta_gpios[] __initconst_or_module = {
 		.flags	= GPIOF_DIR_IN,
 		.label	= "serio-clock",
 	},
-	{
-		.gpio	= AMS_DELTA_GPIO_PIN_KEYBRD_DATAOUT,
-		.flags	= GPIOF_OUT_INIT_LOW,
-		.label	= "serio-dataout",
-	},
 };
 
 static int ams_delta_serio_init(struct platform_device *pdev)
-- 
2.16.4

^ permalink raw reply related

* [PATCH v2 04/10] Input: ams_delta_serio: Replace power GPIO with regulator
From: Janusz Krzysztofik @ 2018-06-21 22:41 UTC (permalink / raw)
  To: Dmitry Torokhov, Tony Lindgren, Aaro Koskinen
  Cc: David S . Miller , Mauro Carvalho Chehab, Greg Kroah-Hartman,
	Andrew Morton, Randy Dunlap, Linus Walleij, Mark Brown,
	Liam Girdwood, linux-arm-kernel, linux-omap, linux-input,
	linux-gpio, linux-kernel, Janusz Krzysztofik
In-Reply-To: <20180621224128.17623-1-jmkrzyszt@gmail.com>

Modify the driver so it no longer requests and manipulates the
"keybrd_pwr" GPIO pin but a "vcc" regulator supply instead.

For this to work with Amstrad Delta, define a regulator over the
"keybrd_pwr" GPIO pin with the "vcc" supply for ams-delta-serio device
and register it from the board file.  Both assign an absulute GPIO
number to the soon depreciated .gpio member of the regulator config
structure, and also build and register a GPIO lookup table so it is
ready for use by the regulator driver as soon as its upcoming update
is applied.

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
Changelog:
v2: - extended comment above error code conversion, thanks Dmitry for 
      requesting that,
    - rebased on v4.18-rc1, no conflicts.

 arch/arm/mach-omap1/board-ams-delta.c | 63 +++++++++++++++++++++++++++++++++--
 drivers/input/serio/ams_delta_serio.c | 37 +++++++++++++++-----
 2 files changed, 89 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index 2119d2d3ba84..706eb2f9301d 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -509,6 +509,46 @@ static struct platform_device ams_delta_serio_device = {
 	.id		= PLATFORM_DEVID_NONE,
 };
 
+static struct regulator_consumer_supply keybrd_pwr_consumers[] = {
+	/*
+	 * Initialize supply .dev_name with NULL.  It will be replaced
+	 * with serio dev_name() as soon as the serio device is registered.
+	 */
+	REGULATOR_SUPPLY("vcc", NULL),
+};
+
+static struct regulator_init_data keybrd_pwr_initdata = {
+	.constraints		= {
+		.valid_ops_mask		= REGULATOR_CHANGE_STATUS,
+	},
+	.num_consumer_supplies	= ARRAY_SIZE(keybrd_pwr_consumers),
+	.consumer_supplies	= keybrd_pwr_consumers,
+};
+
+static struct fixed_voltage_config keybrd_pwr_config = {
+	.supply_name		= "keybrd_pwr",
+	.microvolts		= 5000000,
+	.gpio			= AMS_DELTA_GPIO_PIN_KEYBRD_PWR,
+	.enable_high		= 1,
+	.init_data		= &keybrd_pwr_initdata,
+};
+
+static struct platform_device keybrd_pwr_device = {
+	.name	= "reg-fixed-voltage",
+	.id	= PLATFORM_DEVID_AUTO,
+	.dev	= {
+		.platform_data	= &keybrd_pwr_config,
+	},
+};
+
+static struct gpiod_lookup_table keybrd_pwr_gpio_table = {
+	.table = {
+		GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_KEYBRD_PWR, NULL,
+			    GPIO_ACTIVE_HIGH),
+		{ },
+	},
+};
+
 static struct platform_device *ams_delta_devices[] __initdata = {
 	&latch1_gpio_device,
 	&latch2_gpio_device,
@@ -526,6 +566,7 @@ static struct platform_device *late_devices[] __initdata = {
 
 static struct gpiod_lookup_table *ams_delta_gpio_tables[] __initdata = {
 	&ams_delta_audio_gpio_table,
+	&keybrd_pwr_gpio_table,
 };
 
 static struct gpiod_lookup_table *late_gpio_tables[] __initdata = {
@@ -566,12 +607,30 @@ static void __init ams_delta_init(void)
 	platform_add_devices(ams_delta_devices, ARRAY_SIZE(ams_delta_devices));
 
 	/*
-	 * As soon as devices have been registered, assign their dev_names
-	 * to respective GPIO lookup tables before they are added.
+	 * As soon as regulator consumers have been registered, assign their
+	 * dev_names to consumer supply entries of respective regulators.
+	 */
+	keybrd_pwr_consumers[0].dev_name =
+			dev_name(&ams_delta_serio_device.dev);
+
+	/*
+	 * Once consumer supply entries are populated with dev_names,
+	 * register regulator devices.  At this stage only the keyboard
+	 * power regulator has its consumer supply table fully populated.
+	 */
+	platform_device_register(&keybrd_pwr_device);
+
+	/*
+	 * As soon as GPIO consumers have been registered, assign
+	 * their dev_names to respective GPIO lookup tables.
 	 */
 	ams_delta_audio_gpio_table.dev_id =
 			dev_name(&ams_delta_audio_device.dev);
+	keybrd_pwr_gpio_table.dev_id = dev_name(&keybrd_pwr_device.dev);
 
+	/*
+	 * Once GPIO lookup tables are populated with dev_names, register them.
+	 */
 	gpiod_add_lookup_tables(ams_delta_gpio_tables,
 				ARRAY_SIZE(ams_delta_gpio_tables));
 
diff --git a/drivers/input/serio/ams_delta_serio.c b/drivers/input/serio/ams_delta_serio.c
index 551a4fa73fe4..854d0d3ada52 100644
--- a/drivers/input/serio/ams_delta_serio.c
+++ b/drivers/input/serio/ams_delta_serio.c
@@ -23,6 +23,7 @@
 #include <linux/gpio.h>
 #include <linux/irq.h>
 #include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
 #include <linux/serio.h>
 #include <linux/slab.h>
 #include <linux/module.h>
@@ -39,6 +40,7 @@ MODULE_LICENSE("GPL");
 
 struct ams_delta_serio {
 	struct serio *serio;
+	struct regulator *vcc;
 };
 
 static int check_data(struct serio *serio, int data)
@@ -94,16 +96,18 @@ static irqreturn_t ams_delta_serio_interrupt(int irq, void *dev_id)
 
 static int ams_delta_serio_open(struct serio *serio)
 {
-	/* enable keyboard */
-	gpio_set_value(AMS_DELTA_GPIO_PIN_KEYBRD_PWR, 1);
+	struct ams_delta_serio *priv = serio->port_data;
 
-	return 0;
+	/* enable keyboard */
+	return regulator_enable(priv->vcc);
 }
 
 static void ams_delta_serio_close(struct serio *serio)
 {
+	struct ams_delta_serio *priv = serio->port_data;
+
 	/* disable keyboard */
-	gpio_set_value(AMS_DELTA_GPIO_PIN_KEYBRD_PWR, 0);
+	regulator_disable(priv->vcc);
 }
 
 static const struct gpio ams_delta_gpios[] __initconst_or_module = {
@@ -117,11 +121,6 @@ static const struct gpio ams_delta_gpios[] __initconst_or_module = {
 		.flags	= GPIOF_DIR_IN,
 		.label	= "serio-clock",
 	},
-	{
-		.gpio	= AMS_DELTA_GPIO_PIN_KEYBRD_PWR,
-		.flags	= GPIOF_OUT_INIT_LOW,
-		.label	= "serio-power",
-	},
 	{
 		.gpio	= AMS_DELTA_GPIO_PIN_KEYBRD_DATAOUT,
 		.flags	= GPIOF_OUT_INIT_LOW,
@@ -146,6 +145,26 @@ static int ams_delta_serio_init(struct platform_device *pdev)
 		goto serio;
 	}
 
+	priv->vcc = devm_regulator_get(&pdev->dev, "vcc");
+	if (IS_ERR(priv->vcc)) {
+		err = PTR_ERR(priv->vcc);
+		dev_err(&pdev->dev, "regulator request failed (%d)\n", err);
+		/*
+		 * When running on a non-dt platform and requested regulator
+		 * is not available, devm_regulator_get() never returns
+		 * -EPROBE_DEFER as it is not able to justify if the regulator
+		 * may still appear later.  On the other hand, the board can
+		 * still set full constriants flag at late_initcall in order
+		 * to instruct devm_regulator_get() to returnn a dummy one
+		 * if sufficient.  Hence, if we get -ENODEV here, let's convert
+		 * it to -EPROBE_DEFER and wait for the board to decide or
+		 * let Deferred Probe infrastructure handle this error.
+		 */
+		if (err == -ENODEV)
+			err = -EPROBE_DEFER;
+		goto gpio;
+	}
+
 	err = request_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK),
 			ams_delta_serio_interrupt, IRQ_TYPE_EDGE_RISING,
 			DRIVER_NAME, priv);
-- 
2.16.4

^ permalink raw reply related

* [PATCH v2 03/10] Input: ams_delta_serio: use private structure
From: Janusz Krzysztofik @ 2018-06-21 22:41 UTC (permalink / raw)
  To: Dmitry Torokhov, Tony Lindgren, Aaro Koskinen
  Cc: David S . Miller , Mauro Carvalho Chehab, Greg Kroah-Hartman,
	Andrew Morton, Randy Dunlap, Linus Walleij, Mark Brown,
	Liam Girdwood, linux-arm-kernel, linux-omap, linux-input,
	linux-gpio, linux-kernel, Janusz Krzysztofik
In-Reply-To: <20180621224128.17623-1-jmkrzyszt@gmail.com>

Introduce a driver private structure and allocate it on device probe.
For now, use it instead of a static variable for storing a pointer to
serio structure.  Subsequent patches will populate it with more members
as needed.

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
Changelog:
v2: rebased on v4.18-rc1, no conflicts

 drivers/input/serio/ams_delta_serio.c | 69 ++++++++++++++++++++++-------------
 1 file changed, 43 insertions(+), 26 deletions(-)

diff --git a/drivers/input/serio/ams_delta_serio.c b/drivers/input/serio/ams_delta_serio.c
index a2a7fa19bf49..551a4fa73fe4 100644
--- a/drivers/input/serio/ams_delta_serio.c
+++ b/drivers/input/serio/ams_delta_serio.c
@@ -37,17 +37,17 @@ MODULE_AUTHOR("Matt Callow");
 MODULE_DESCRIPTION("AMS Delta (E3) keyboard port driver");
 MODULE_LICENSE("GPL");
 
-static struct serio *ams_delta_serio;
+struct ams_delta_serio {
+	struct serio *serio;
+};
 
-static int check_data(int data)
+static int check_data(struct serio *serio, int data)
 {
 	int i, parity = 0;
 
 	/* check valid stop bit */
 	if (!(data & 0x400)) {
-		dev_warn(&ams_delta_serio->dev,
-				"invalid stop bit, data=0x%X\n",
-				data);
+		dev_warn(&serio->dev, "invalid stop bit, data=0x%X\n", data);
 		return SERIO_FRAME;
 	}
 	/* calculate the parity */
@@ -57,9 +57,9 @@ static int check_data(int data)
 	}
 	/* it should be odd */
 	if (!(parity & 0x01)) {
-		dev_warn(&ams_delta_serio->dev,
-				"parity check failed, data=0x%X parity=0x%X\n",
-				data, parity);
+		dev_warn(&serio->dev,
+			 "parity check failed, data=0x%X parity=0x%X\n", data,
+			 parity);
 		return SERIO_PARITY;
 	}
 	return 0;
@@ -67,6 +67,7 @@ static int check_data(int data)
 
 static irqreturn_t ams_delta_serio_interrupt(int irq, void *dev_id)
 {
+	struct ams_delta_serio *priv = dev_id;
 	int *circ_buff = &fiq_buffer[FIQ_CIRC_BUFF];
 	int data, dfl;
 	u8 scancode;
@@ -84,9 +85,9 @@ static irqreturn_t ams_delta_serio_interrupt(int irq, void *dev_id)
 		if (fiq_buffer[FIQ_HEAD_OFFSET] == fiq_buffer[FIQ_BUF_LEN])
 			fiq_buffer[FIQ_HEAD_OFFSET] = 0;
 
-		dfl = check_data(data);
+		dfl = check_data(priv->serio, data);
 		scancode = (u8) (data >> 1) & 0xFF;
-		serio_interrupt(ams_delta_serio, scancode, dfl);
+		serio_interrupt(priv->serio, scancode, dfl);
 	}
 	return IRQ_HANDLED;
 }
@@ -130,21 +131,14 @@ static const struct gpio ams_delta_gpios[] __initconst_or_module = {
 
 static int ams_delta_serio_init(struct platform_device *pdev)
 {
+	struct ams_delta_serio *priv;
+	struct serio *serio;
 	int err;
 
-	ams_delta_serio = kzalloc(sizeof(struct serio), GFP_KERNEL);
-	if (!ams_delta_serio)
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
 		return -ENOMEM;
 
-	ams_delta_serio->id.type = SERIO_8042;
-	ams_delta_serio->open = ams_delta_serio_open;
-	ams_delta_serio->close = ams_delta_serio_close;
-	strlcpy(ams_delta_serio->name, "AMS DELTA keyboard adapter",
-			sizeof(ams_delta_serio->name));
-	strlcpy(ams_delta_serio->phys, dev_name(&pdev->dev),
-			sizeof(ams_delta_serio->phys));
-	ams_delta_serio->dev.parent = &pdev->dev;
-
 	err = gpio_request_array(ams_delta_gpios,
 				ARRAY_SIZE(ams_delta_gpios));
 	if (err) {
@@ -154,7 +148,7 @@ static int ams_delta_serio_init(struct platform_device *pdev)
 
 	err = request_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK),
 			ams_delta_serio_interrupt, IRQ_TYPE_EDGE_RISING,
-			DRIVER_NAME, 0);
+			DRIVER_NAME, priv);
 	if (err < 0) {
 		dev_err(&pdev->dev, "IRQ request failed (%d)\n", err);
 		goto gpio;
@@ -167,21 +161,44 @@ static int ams_delta_serio_init(struct platform_device *pdev)
 	irq_set_handler(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK),
 			handle_simple_irq);
 
-	serio_register_port(ams_delta_serio);
-	dev_info(&ams_delta_serio->dev, "%s\n", ams_delta_serio->name);
+	serio = kzalloc(sizeof(*serio), GFP_KERNEL);
+	if (!serio) {
+		err = -ENOMEM;
+		goto irq;
+	}
+
+	priv->serio = serio;
+
+	serio->id.type = SERIO_8042;
+	serio->open = ams_delta_serio_open;
+	serio->close = ams_delta_serio_close;
+	strlcpy(serio->name, "AMS DELTA keyboard adapter", sizeof(serio->name));
+	strlcpy(serio->phys, dev_name(&pdev->dev), sizeof(serio->phys));
+	serio->dev.parent = &pdev->dev;
+	serio->port_data = priv;
+
+	serio_register_port(serio);
+
+	platform_set_drvdata(pdev, priv);
+
+	dev_info(&serio->dev, "%s\n", serio->name);
 
 	return 0;
+
+irq:
+	free_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK), priv);
 gpio:
 	gpio_free_array(ams_delta_gpios,
 			ARRAY_SIZE(ams_delta_gpios));
 serio:
-	kfree(ams_delta_serio);
 	return err;
 }
 
 static int ams_delta_serio_exit(struct platform_device *pdev)
 {
-	serio_unregister_port(ams_delta_serio);
+	struct ams_delta_serio *priv = platform_get_drvdata(pdev);
+
+	serio_unregister_port(priv->serio);
 	free_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK), 0);
 	gpio_free_array(ams_delta_gpios,
 			ARRAY_SIZE(ams_delta_gpios));
-- 
2.16.4

^ permalink raw reply related

* [PATCH v2 02/10] Input: ams_delta_serio: convert to platform driver
From: Janusz Krzysztofik @ 2018-06-21 22:41 UTC (permalink / raw)
  To: Dmitry Torokhov, Tony Lindgren, Aaro Koskinen
  Cc: David S . Miller , Mauro Carvalho Chehab, Greg Kroah-Hartman,
	Andrew Morton, Randy Dunlap, Linus Walleij, Mark Brown,
	Liam Girdwood, linux-arm-kernel, linux-omap, linux-input,
	linux-gpio, linux-kernel, Janusz Krzysztofik
In-Reply-To: <20180621224128.17623-1-jmkrzyszt@gmail.com>

Convert the driver to an "ams-delta-serio" platform driver.  For it to
be used with Amstrad Delta, register an "ams-delta-serio" platform
device from the board init file.

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
Changelog:
v2: rebased on v4.18-rc1, no conflicts

 arch/arm/mach-omap1/board-ams-delta.c |  6 ++++++
 drivers/input/serio/ams_delta_serio.c | 34 +++++++++++++++++++++-------------
 2 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index 18e0ff437b27..2119d2d3ba84 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -504,12 +504,18 @@ static struct platform_device cx20442_codec_device = {
 	.id     = -1,
 };
 
+static struct platform_device ams_delta_serio_device = {
+	.name		= "ams-delta-serio",
+	.id		= PLATFORM_DEVID_NONE,
+};
+
 static struct platform_device *ams_delta_devices[] __initdata = {
 	&latch1_gpio_device,
 	&latch2_gpio_device,
 	&ams_delta_kp_device,
 	&ams_delta_camera_device,
 	&ams_delta_audio_device,
+	&ams_delta_serio_device,
 };
 
 static struct platform_device *late_devices[] __initdata = {
diff --git a/drivers/input/serio/ams_delta_serio.c b/drivers/input/serio/ams_delta_serio.c
index 3df501c3421b..a2a7fa19bf49 100644
--- a/drivers/input/serio/ams_delta_serio.c
+++ b/drivers/input/serio/ams_delta_serio.c
@@ -22,15 +22,17 @@
  */
 #include <linux/gpio.h>
 #include <linux/irq.h>
+#include <linux/platform_device.h>
 #include <linux/serio.h>
 #include <linux/slab.h>
 #include <linux/module.h>
 
-#include <asm/mach-types.h>
 #include <mach/board-ams-delta.h>
 
 #include <mach/ams-delta-fiq.h>
 
+#define DRIVER_NAME	"ams-delta-serio"
+
 MODULE_AUTHOR("Matt Callow");
 MODULE_DESCRIPTION("AMS Delta (E3) keyboard port driver");
 MODULE_LICENSE("GPL");
@@ -126,13 +128,10 @@ static const struct gpio ams_delta_gpios[] __initconst_or_module = {
 	},
 };
 
-static int __init ams_delta_serio_init(void)
+static int ams_delta_serio_init(struct platform_device *pdev)
 {
 	int err;
 
-	if (!machine_is_ams_delta())
-		return -ENODEV;
-
 	ams_delta_serio = kzalloc(sizeof(struct serio), GFP_KERNEL);
 	if (!ams_delta_serio)
 		return -ENOMEM;
@@ -142,22 +141,22 @@ static int __init ams_delta_serio_init(void)
 	ams_delta_serio->close = ams_delta_serio_close;
 	strlcpy(ams_delta_serio->name, "AMS DELTA keyboard adapter",
 			sizeof(ams_delta_serio->name));
-	strlcpy(ams_delta_serio->phys, "GPIO/serio0",
+	strlcpy(ams_delta_serio->phys, dev_name(&pdev->dev),
 			sizeof(ams_delta_serio->phys));
+	ams_delta_serio->dev.parent = &pdev->dev;
 
 	err = gpio_request_array(ams_delta_gpios,
 				ARRAY_SIZE(ams_delta_gpios));
 	if (err) {
-		pr_err("ams_delta_serio: Couldn't request gpio pins\n");
+		dev_err(&pdev->dev, "Couldn't request gpio pins\n");
 		goto serio;
 	}
 
 	err = request_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK),
 			ams_delta_serio_interrupt, IRQ_TYPE_EDGE_RISING,
-			"ams-delta-serio", 0);
+			DRIVER_NAME, 0);
 	if (err < 0) {
-		pr_err("ams_delta_serio: couldn't request gpio interrupt %d\n",
-				gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK));
+		dev_err(&pdev->dev, "IRQ request failed (%d)\n", err);
 		goto gpio;
 	}
 	/*
@@ -179,13 +178,22 @@ static int __init ams_delta_serio_init(void)
 	kfree(ams_delta_serio);
 	return err;
 }
-module_init(ams_delta_serio_init);
 
-static void __exit ams_delta_serio_exit(void)
+static int ams_delta_serio_exit(struct platform_device *pdev)
 {
 	serio_unregister_port(ams_delta_serio);
 	free_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK), 0);
 	gpio_free_array(ams_delta_gpios,
 			ARRAY_SIZE(ams_delta_gpios));
+
+	return 0;
 }
-module_exit(ams_delta_serio_exit);
+
+static struct platform_driver ams_delta_serio_driver = {
+	.probe	= ams_delta_serio_init,
+	.remove	= ams_delta_serio_exit,
+	.driver	= {
+		.name	= DRIVER_NAME
+	},
+};
+module_platform_driver(ams_delta_serio_driver);
-- 
2.16.4

^ permalink raw reply related

* [PATCH v2 01/10] ARM: OMAP1: ams-delta: drop GPIO lookup table for serio device
From: Janusz Krzysztofik @ 2018-06-21 22:41 UTC (permalink / raw)
  To: Dmitry Torokhov, Tony Lindgren, Aaro Koskinen
  Cc: David S . Miller , Mauro Carvalho Chehab, Greg Kroah-Hartman,
	Andrew Morton, Randy Dunlap, Linus Walleij, Mark Brown,
	Liam Girdwood, linux-arm-kernel, linux-omap, linux-input,
	linux-gpio, linux-kernel, Janusz Krzysztofik
In-Reply-To: <20180609140224.32606-1-jmkrzyszt@gmail.com>

GPIO lookup table for ams-delta-serio device was introduced by commit
0486738928bf ("ARM: OMAP1: ams-delta: add GPIO lookup tables").
Unfortunately, a follow up patch "Input: ams_delta_serio: use GPIO
lookup table" was not accepted by subystem maintainer who requested
conversion of the driver to a platform driver, replacepemnt of IRQ GPIO
pin with IRQ resource, replacement of GPIO pin providing keyboard power
with a regulator and removal of remaining GPIO pins from the driver as
not handled by it.

Let's start with removal of the no longer needed GPIO lookup table from
the board init file.

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
Changelog:
v2: rebased on v4.18-rc1, no conflicts

 arch/arm/mach-omap1/board-ams-delta.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index 80f54cb54276..18e0ff437b27 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -504,20 +504,6 @@ static struct platform_device cx20442_codec_device = {
 	.id     = -1,
 };
 
-static struct gpiod_lookup_table ams_delta_serio_gpio_table = {
-	.table = {
-		GPIO_LOOKUP(OMAP_GPIO_LABEL, AMS_DELTA_GPIO_PIN_KEYBRD_DATA,
-			    "data", 0),
-		GPIO_LOOKUP(OMAP_GPIO_LABEL, AMS_DELTA_GPIO_PIN_KEYBRD_CLK,
-			    "clock", 0),
-		GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_KEYBRD_PWR,
-			    "power", 0),
-		GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_KEYBRD_DATAOUT,
-			    "dataout", 0),
-		{ },
-	},
-};
-
 static struct platform_device *ams_delta_devices[] __initdata = {
 	&latch1_gpio_device,
 	&latch2_gpio_device,
@@ -534,7 +520,6 @@ static struct platform_device *late_devices[] __initdata = {
 
 static struct gpiod_lookup_table *ams_delta_gpio_tables[] __initdata = {
 	&ams_delta_audio_gpio_table,
-	&ams_delta_serio_gpio_table,
 };
 
 static struct gpiod_lookup_table *late_gpio_tables[] __initdata = {
@@ -580,10 +565,6 @@ static void __init ams_delta_init(void)
 	 */
 	ams_delta_audio_gpio_table.dev_id =
 			dev_name(&ams_delta_audio_device.dev);
-	/*
-	 * No device name is assigned to GPIO lookup table for serio device
-	 * as long as serio driver is not converted to platform device driver.
-	 */
 
 	gpiod_add_lookup_tables(ams_delta_gpio_tables,
 				ARRAY_SIZE(ams_delta_gpio_tables));
-- 
2.16.4

^ permalink raw reply related

* [PATCH v3 12/12] HID: multitouch: handle palm for touchscreens
From: Benjamin Tissoires @ 2018-06-21 12:09 UTC (permalink / raw)
  To: Jiri Kosina, Dmitry Torokhov
  Cc: Mario.Limonciello, Peter Hutterer, linux-input, linux-kernel,
	Benjamin Tissoires
In-Reply-To: <20180621120908.16706-1-benjamin.tissoires@redhat.com>

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Usually, there is no palm rejection for touchscreens. You don't rest
your palm on the touchscreen while interacting with it.
However, some wacom devices do so because you can rest your palm while
interacting with the stylus.

Unfortunately, the spec for touchscreens[1] is less precise than the one
for touchpads[2]. This leads to a situation where it's 'legitimate'
for a touchscreen to provide both tipswitch off and confidence off in the
same report.

Work around that by keeping the slot active for one frame where we report
MT_TOOL_PALM, and then synthesizing the release event in a separate frame.
frame

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
[rebased and new commit message]
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---
changes in v2 (compared to Dmitry's initial submission):
- extracted from the initial submission in a separate patch
- rebased on top of my current series
- add an extra input_mt_sync_frame(input); to release the single touch
  emulation

no changes in v3
---
 drivers/hid/hid-multitouch.c | 52 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 51 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 9c708aa261ee..3a1c2d80729b 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -118,6 +118,9 @@ struct mt_application {
 	int left_button_state;		/* left button state */
 	unsigned int mt_flags;		/* flags to pass to input-mt */
 
+	unsigned long *pending_palm_slots;	/* slots where we reported palm
+						 * and need to release */
+
 	__u8 num_received;	/* how many contacts we received */
 	__u8 num_expected;	/* expected last contact index */
 	__u8 buttons_count;	/* number of physical buttons per touchpad */
@@ -863,6 +866,28 @@ static int mt_compute_slot(struct mt_device *td, struct mt_application *app,
 	return input_mt_get_slot_by_key(input, *slot->contactid);
 }
 
+static void mt_release_pending_palms(struct mt_device *td,
+				     struct mt_application *app,
+				     struct input_dev *input)
+{
+	int slotnum;
+	bool need_sync = false;
+
+	for_each_set_bit(slotnum, app->pending_palm_slots, td->maxcontacts) {
+		clear_bit(slotnum, app->pending_palm_slots);
+
+		input_mt_slot(input, slotnum);
+		input_mt_report_slot_state(input, MT_TOOL_PALM, false);
+
+		need_sync = true;
+	}
+
+	if (need_sync) {
+		input_mt_sync_frame(input);
+		input_sync(input);
+	}
+}
+
 /*
  * this function is called when a whole packet has been received and processed,
  * so that it can decide what to send to the input layer.
@@ -876,6 +901,9 @@ static void mt_sync_frame(struct mt_device *td, struct mt_application *app,
 	input_mt_sync_frame(input);
 	input_event(input, EV_MSC, MSC_TIMESTAMP, app->timestamp);
 	input_sync(input);
+
+	mt_release_pending_palms(td, app, input);
+
 	app->num_received = 0;
 	app->left_button_state = 0;
 
@@ -970,8 +998,23 @@ static int mt_process_slot(struct mt_device *td, struct input_dev *input,
 
 	if (app->application == HID_GD_SYSTEM_MULTIAXIS)
 		tool = MT_TOOL_DIAL;
-	else if (unlikely(!confidence_state))
+	else if (unlikely(!confidence_state)) {
 		tool = MT_TOOL_PALM;
+		if (!active &&
+		    input_mt_is_active(&mt->slots[slotnum])) {
+			/*
+			 * The non-confidence was reported for
+			 * previously valid contact that is also no
+			 * longer valid. We can't simply report
+			 * lift-off as userspace will not be aware
+			 * of non-confidence, so we need to split
+			 * it into 2 events: active MT_TOOL_PALM
+			 * and a separate liftoff.
+			 */
+			active = true;
+			set_bit(slotnum, app->pending_palm_slots);
+		}
+	}
 
 	input_mt_slot(input, slotnum);
 	input_mt_report_slot_state(input, tool, active);
@@ -1206,6 +1249,13 @@ static int mt_touch_input_configured(struct hid_device *hdev,
 	if (td->is_buttonpad)
 		__set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
 
+	app->pending_palm_slots = devm_kcalloc(&hi->input->dev,
+					       BITS_TO_LONGS(td->maxcontacts),
+					       sizeof(long),
+					       GFP_KERNEL);
+	if (!app->pending_palm_slots)
+		return -ENOMEM;
+
 	ret = input_mt_init_slots(input, td->maxcontacts, app->mt_flags);
 	if (ret)
 		return ret;
-- 
2.14.3

^ permalink raw reply related

* [PATCH v3 11/12] HID: multitouch: touchscreens also use confidence reports
From: Benjamin Tissoires @ 2018-06-21 12:09 UTC (permalink / raw)
  To: Jiri Kosina, Dmitry Torokhov
  Cc: Mario.Limonciello, Peter Hutterer, linux-input, linux-kernel,
	Benjamin Tissoires
In-Reply-To: <20180621120908.16706-1-benjamin.tissoires@redhat.com>

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

According to [1] the confidence is used not only by touchpad devices,
but also by touchscreens.

[1] https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/touchscreen-required-hid-top-level-collections

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
[rebased]
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

no changes in v2 (compared to Dmitry's initial submission)
no changes in v3
---
 drivers/hid/hid-multitouch.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 36c8b67bb032..9c708aa261ee 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -734,7 +734,8 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		case HID_DG_CONFIDENCE:
 			if ((cls->name == MT_CLS_WIN_8 ||
 				cls->name == MT_CLS_WIN_8_DUAL) &&
-				field->application == HID_DG_TOUCHPAD)
+				(field->application == HID_DG_TOUCHPAD ||
+				 field->application == HID_DG_TOUCHSCREEN))
 				app->quirks |= MT_QUIRK_CONFIDENCE;
 
 			if (app->quirks & MT_QUIRK_CONFIDENCE)
-- 
2.14.3

^ permalink raw reply related

* [PATCH v3 10/12] HID: multitouch: report MT_TOOL_PALM for non-confident touches
From: Benjamin Tissoires @ 2018-06-21 12:09 UTC (permalink / raw)
  To: Jiri Kosina, Dmitry Torokhov
  Cc: Mario.Limonciello, Peter Hutterer, linux-input, linux-kernel,
	Benjamin Tissoires
In-Reply-To: <20180621120908.16706-1-benjamin.tissoires@redhat.com>

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

According to Microsoft specification [1] for Precision Touchpads (and
Touchscreens) the devices use "confidence" reports to signal accidental
touches, or contacts that are "too large to be a finger". Instead of
simply marking contact inactive in this case (which causes issues if
contact was originally proper and we lost confidence in it later, as
this results in accidental clicks, drags, etc), let's report such
contacts as MT_TOOL_PALM and let userspace decide what to do.
Additionally, let's report contact size for such touches as maximum
allowed for major/minor, which should help userspace that is not yet
aware of MT_TOOL_PALM to still perform palm rejection.

[1] https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/touchpad-windows-precision-touchpad-collection

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
[splitted and rebased]
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

changes in v2:
- dropped the delayed release in case of palm detection. This is
  a FW bug for PTP devices, and should only happen with touchscreens
- move input_set_abs_params(hi->input, ABS_MT_TOOL_TYPE,...) into
  its own test so other devices can add MT_QUIRK_CONFIDENCE to their
  quirks.

no changes in v3
---
 drivers/hid/hid-multitouch.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index a4a274ebfbef..36c8b67bb032 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -736,6 +736,13 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 				cls->name == MT_CLS_WIN_8_DUAL) &&
 				field->application == HID_DG_TOUCHPAD)
 				app->quirks |= MT_QUIRK_CONFIDENCE;
+
+			if (app->quirks & MT_QUIRK_CONFIDENCE)
+				input_set_abs_params(hi->input,
+						     ABS_MT_TOOL_TYPE,
+						     MT_TOOL_FINGER,
+						     MT_TOOL_PALM, 0, 0);
+
 			MT_STORE_FIELD(confidence_state);
 			return 1;
 		case HID_DG_TIPSWITCH:
@@ -958,10 +965,12 @@ static int mt_process_slot(struct mt_device *td, struct input_dev *input,
 	if (quirks & MT_QUIRK_HOVERING)
 		inrange_state = *slot->inrange_state;
 
-	active = (*slot->tip_state || inrange_state) && confidence_state;
+	active = *slot->tip_state || inrange_state;
 
 	if (app->application == HID_GD_SYSTEM_MULTIAXIS)
 		tool = MT_TOOL_DIAL;
+	else if (unlikely(!confidence_state))
+		tool = MT_TOOL_PALM;
 
 	input_mt_slot(input, slotnum);
 	input_mt_report_slot_state(input, tool, active);
@@ -993,11 +1002,20 @@ static int mt_process_slot(struct mt_device *td, struct input_dev *input,
 			orientation = -azimuth;
 		}
 
-		/*
-		 * divided by two to match visual scale of touch
-		 * for devices with this quirk
-		 */
-		if (quirks & MT_QUIRK_TOUCH_SIZE_SCALING) {
+		if (unlikely(!confidence_state)) {
+			/*
+			 * When reporting palm, set contact to maximum
+			 * size to help userspace that does not
+			 * recognize MT_TOOL_PALM to reject contacts
+			 * that are too large.
+			 */
+			major = input_abs_get_max(input, ABS_MT_TOUCH_MAJOR);
+			minor = input_abs_get_max(input, ABS_MT_TOUCH_MINOR);
+		} else if (quirks & MT_QUIRK_TOUCH_SIZE_SCALING) {
+			/*
+			 * divided by two to match visual scale of touch
+			 * for devices with this quirk
+			 */
 			major = major >> 1;
 			minor = minor >> 1;
 		}
-- 
2.14.3

^ permalink raw reply related

* [PATCH v3 09/12] HID: microsoft: support the Surface Dial
From: Benjamin Tissoires @ 2018-06-21 12:09 UTC (permalink / raw)
  To: Jiri Kosina, Dmitry Torokhov
  Cc: Mario.Limonciello, Peter Hutterer, linux-input, linux-kernel,
	Benjamin Tissoires
In-Reply-To: <20180621120908.16706-1-benjamin.tissoires@redhat.com>

The tool works nicely with hid-generic, but it ends up creating 9
different input nodes with most of them only having ABS_MISC set.

Filter the axis out, which reduces the amount of devices to 2. One is
the proper System Multi-axis collection, the other exported device
seems to provide SLEEP and POWER Key, not sure how one can trigger
those events though.

Filtering the ABS_X and ABS_Y axes also prevents udev to detect this as
a touchscreen.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

no changes in v2
no changes in v3
---
 drivers/hid/hid-microsoft.c | 49 +++++++++++++++++++++++++++++++++++++++------
 1 file changed, 43 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-microsoft.c b/drivers/hid/hid-microsoft.c
index 96e7d3231d2f..72d983626afd 100644
--- a/drivers/hid/hid-microsoft.c
+++ b/drivers/hid/hid-microsoft.c
@@ -22,12 +22,13 @@
 
 #include "hid-ids.h"
 
-#define MS_HIDINPUT		0x01
-#define MS_ERGONOMY		0x02
-#define MS_PRESENTER		0x04
-#define MS_RDESC		0x08
-#define MS_NOGET		0x10
-#define MS_DUPLICATE_USAGES	0x20
+#define MS_HIDINPUT		BIT(0)
+#define MS_ERGONOMY		BIT(1)
+#define MS_PRESENTER		BIT(2)
+#define MS_RDESC		BIT(3)
+#define MS_NOGET		BIT(4)
+#define MS_DUPLICATE_USAGES	BIT(5)
+#define MS_SURFACE_DIAL		BIT(6)
 
 static __u8 *ms_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 		unsigned int *rsize)
@@ -130,6 +131,30 @@ static int ms_presenter_8k_quirk(struct hid_input *hi, struct hid_usage *usage,
 	return 1;
 }
 
+static int ms_surface_dial_quirk(struct hid_input *hi, struct hid_field *field,
+		struct hid_usage *usage, unsigned long **bit, int *max)
+{
+	switch (usage->hid & HID_USAGE_PAGE) {
+	case 0xff070000:
+		/* fall-through */
+	case HID_UP_DIGITIZER:
+		/* ignore those axis */
+		return -1;
+	case HID_UP_GENDESK:
+		switch (usage->hid) {
+		case HID_GD_X:
+			/* fall-through */
+		case HID_GD_Y:
+			/* fall-through */
+		case HID_GD_RFKILL_BTN:
+			/* ignore those axis */
+			return -1;
+		}
+	}
+
+	return 0;
+}
+
 static int ms_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		struct hid_field *field, struct hid_usage *usage,
 		unsigned long **bit, int *max)
@@ -146,6 +171,13 @@ static int ms_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 			ms_presenter_8k_quirk(hi, usage, bit, max))
 		return 1;
 
+	if (quirks & MS_SURFACE_DIAL) {
+		int ret = ms_surface_dial_quirk(hi, field, usage, bit, max);
+
+		if (ret)
+			return ret;
+	}
+
 	return 0;
 }
 
@@ -229,6 +261,9 @@ static int ms_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	if (quirks & MS_NOGET)
 		hdev->quirks |= HID_QUIRK_NOGET;
 
+	if (quirks & MS_SURFACE_DIAL)
+		hdev->quirks |= HID_QUIRK_INPUT_PER_APP;
+
 	ret = hid_parse(hdev);
 	if (ret) {
 		hid_err(hdev, "parse failed\n");
@@ -281,6 +316,8 @@ static const struct hid_device_id ms_devices[] = {
 
 	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_PRESENTER_8K_BT),
 		.driver_data = MS_PRESENTER },
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, 0x091B),
+		.driver_data = MS_SURFACE_DIAL },
 	{ }
 };
 MODULE_DEVICE_TABLE(hid, ms_devices);
-- 
2.14.3

^ permalink raw reply related

* [PATCH v3 08/12] HID: core: do not upper bound the collection stack
From: Benjamin Tissoires @ 2018-06-21 12:09 UTC (permalink / raw)
  To: Jiri Kosina, Dmitry Torokhov
  Cc: Mario.Limonciello, Peter Hutterer, linux-input, linux-kernel,
	Benjamin Tissoires
In-Reply-To: <20180621120908.16706-1-benjamin.tissoires@redhat.com>

Looks like 4 was sufficient until now. However, the Surface Dial needs
a stack of 5 and simply fails at probing.
Dynamically add HID_COLLECTION_STACK_SIZE to the size of the stack if
we hit the upper bound.

Checkpatch complains about bare unsigned, so converting those to
'unsigned int' in struct hid_parser

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

no changes in v2
no changes in v3
---
 drivers/hid/hid-core.c | 17 ++++++++++++++---
 include/linux/hid.h    |  9 +++++----
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index a460ec147aee..7afed0c0f9e5 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -128,9 +128,19 @@ static int open_collection(struct hid_parser *parser, unsigned type)
 
 	usage = parser->local.usage[0];
 
-	if (parser->collection_stack_ptr == HID_COLLECTION_STACK_SIZE) {
-		hid_err(parser->device, "collection stack overflow\n");
-		return -EINVAL;
+	if (parser->collection_stack_ptr == parser->collection_stack_size) {
+		unsigned int *collection_stack;
+		unsigned int new_size = parser->collection_stack_size +
+					HID_COLLECTION_STACK_SIZE;
+
+		collection_stack = krealloc(parser->collection_stack,
+					    new_size * sizeof(unsigned int),
+					    GFP_KERNEL);
+		if (!collection_stack)
+			return -ENOMEM;
+
+		parser->collection_stack = collection_stack;
+		parser->collection_stack_size = new_size;
 	}
 
 	if (parser->device->maxcollection == parser->device->collection_size) {
@@ -837,6 +847,7 @@ static int hid_scan_report(struct hid_device *hid)
 		break;
 	}
 
+	kfree(parser->collection_stack);
 	vfree(parser);
 	return 0;
 }
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 2e4498d52a2f..aee281522c6d 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -644,12 +644,13 @@ static inline void hid_set_drvdata(struct hid_device *hdev, void *data)
 struct hid_parser {
 	struct hid_global     global;
 	struct hid_global     global_stack[HID_GLOBAL_STACK_SIZE];
-	unsigned              global_stack_ptr;
+	unsigned int          global_stack_ptr;
 	struct hid_local      local;
-	unsigned              collection_stack[HID_COLLECTION_STACK_SIZE];
-	unsigned              collection_stack_ptr;
+	unsigned int         *collection_stack;
+	unsigned int          collection_stack_ptr;
+	unsigned int          collection_stack_size;
 	struct hid_device    *device;
-	unsigned              scan_flags;
+	unsigned int          scan_flags;
 };
 
 struct hid_class_descriptor {
-- 
2.14.3

^ permalink raw reply related

* [PATCH v3 07/12] HID: input: enable Totem on the Dell Canvas 27
From: Benjamin Tissoires @ 2018-06-21 12:09 UTC (permalink / raw)
  To: Jiri Kosina, Dmitry Torokhov
  Cc: Mario.Limonciello, Peter Hutterer, linux-input, linux-kernel,
	Benjamin Tissoires
In-Reply-To: <20180621120908.16706-1-benjamin.tissoires@redhat.com>

The Dell Canvas 27 has a tool that can be put on the surface and acts
as a dial. The firmware processes the detection of the tool and forward
regular HID reports with X, Y, Azimuth, rotation, width/height.

The firmware also exports Contact ID, Countact Count which may hint that
several totems can be used at the same time (the FW only supports one).

We can tell that MT_TOOL_DIAL will be reported by setting the min/max
of ABS_MT_TOOL_TYPE to MT_TOOL_DIAL.

This tool is aimed at being used by the system and not the applications,
so the user space processing should not go through the regular touch
inputs.
We set INPUT_PROP_DIRECT which applies ID_INPUT_TOUCHSCREEN to this new
type of devices, but we will counter this for the time being with the
special udev hwdb entry mentioned above.

Link: https://bugzilla.redhat.com/show_bug.cgi?id=1511846

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

changes in v2:
- manually declare ABS_MT_TOOL_TYPE and set its min/max to MT_TOOL_DIAL
  to explain which tool it will set

no changes in v3
---
 drivers/hid/hid-input.c      |  3 +++
 drivers/hid/hid-multitouch.c | 48 +++++++++++++++++++++++++++++++-------------
 include/linux/hid.h          |  6 ++++++
 3 files changed, 43 insertions(+), 14 deletions(-)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index ab93dd5927c3..4e94ea3e280a 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1550,6 +1550,9 @@ static struct hid_input *hidinput_allocate(struct hid_device *hid,
 		case HID_GD_WIRELESS_RADIO_CTLS:
 			suffix = "Wireless Radio Control";
 			break;
+		case HID_GD_SYSTEM_MULTIAXIS:
+			suffix = "System Multi Axis";
+			break;
 		default:
 			break;
 		}
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 346e9caef6f3..a4a274ebfbef 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -665,7 +665,8 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 	/*
 	 * Model touchscreens providing buttons as touchpads.
 	 */
-	if ((usage->hid & HID_USAGE_PAGE) == HID_UP_BUTTON) {
+	if (field->application == HID_DG_TOUCHSCREEN &&
+	    (usage->hid & HID_USAGE_PAGE) == HID_UP_BUTTON) {
 		app->mt_flags |= INPUT_MT_POINTER;
 		td->inputmode_value = MT_INPUTMODE_TOUCHPAD;
 	}
@@ -692,6 +693,19 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 
 			set_abs(hi->input, code, field, cls->sn_move);
 
+			/*
+			 * A system multi-axis that exports X and Y has a high
+			 * chance of being used directly on a surface
+			 */
+			if (field->application == HID_GD_SYSTEM_MULTIAXIS) {
+				__set_bit(INPUT_PROP_DIRECT,
+					  hi->input->propbit);
+				input_set_abs_params(hi->input,
+						     ABS_MT_TOOL_TYPE,
+						     MT_TOOL_DIAL,
+						     MT_TOOL_DIAL, 0, 0);
+			}
+
 			return 1;
 		case HID_GD_Y:
 			if (prev_usage && (prev_usage->hid == usage->hid)) {
@@ -725,7 +739,9 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 			MT_STORE_FIELD(confidence_state);
 			return 1;
 		case HID_DG_TIPSWITCH:
-			input_set_capability(hi->input, EV_KEY, BTN_TOUCH);
+			if (field->application != HID_GD_SYSTEM_MULTIAXIS)
+				input_set_capability(hi->input,
+						     EV_KEY, BTN_TOUCH);
 			MT_STORE_FIELD(tip_state);
 			return 1;
 		case HID_DG_CONTACTID:
@@ -802,6 +818,10 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		    field->application == HID_DG_TOUCHPAD &&
 		    (usage->hid & HID_USAGE) > 1)
 			code--;
+
+		if (field->application == HID_GD_SYSTEM_MULTIAXIS)
+			code = BTN_0  + ((usage->hid - 1) & HID_USAGE);
+
 		hid_map_usage(hi, usage, bit, max, EV_KEY, code);
 		input_set_capability(hi->input, EV_KEY, code);
 		return 1;
@@ -899,6 +919,7 @@ static int mt_process_slot(struct mt_device *td, struct input_dev *input,
 	bool inrange_state = false;
 	int active;
 	int slotnum;
+	int tool = MT_TOOL_FINGER;
 
 	if (!slot)
 		return -EINVAL;
@@ -939,8 +960,11 @@ static int mt_process_slot(struct mt_device *td, struct input_dev *input,
 
 	active = (*slot->tip_state || inrange_state) && confidence_state;
 
+	if (app->application == HID_GD_SYSTEM_MULTIAXIS)
+		tool = MT_TOOL_DIAL;
+
 	input_mt_slot(input, slotnum);
-	input_mt_report_slot_state(input, MT_TOOL_FINGER, active);
+	input_mt_report_slot_state(input, tool, active);
 	if (active) {
 		/* this finger is in proximity of the sensor */
 		int wide = (*slot->w > *slot->h);
@@ -1203,6 +1227,7 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 	    field->application != HID_GD_SYSTEM_CONTROL &&
 	    field->application != HID_CP_CONSUMER_CONTROL &&
 	    field->application != HID_GD_WIRELESS_RADIO_CTLS &&
+	    field->application != HID_GD_SYSTEM_MULTIAXIS &&
 	    !(field->application == HID_VD_ASUS_CUSTOM_MEDIA_KEYS &&
 	      application->quirks & MT_QUIRK_ASUS_CUSTOM_UP))
 		return -1;
@@ -1230,9 +1255,7 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		return 1;
 	}
 
-	if (rdata->is_mt_collection &&
-	    (field->application == HID_DG_TOUCHSCREEN ||
-	     field->application == HID_DG_TOUCHPAD))
+	if (rdata->is_mt_collection)
 		return mt_touch_input_mapping(hdev, hi, field, usage, bit, max,
 					      application);
 
@@ -1244,15 +1267,11 @@ static int mt_input_mapped(struct hid_device *hdev, struct hid_input *hi,
 		struct hid_field *field, struct hid_usage *usage,
 		unsigned long **bit, int *max)
 {
-	/*
-	 * some egalax touchscreens have "application == HID_DG_TOUCHSCREEN"
-	 * for the stylus.
-	 */
-	if (field->physical == HID_DG_STYLUS)
-		return 0;
+	struct mt_device *td = hid_get_drvdata(hdev);
+	struct mt_report_data *rdata;
 
-	if (field->application == HID_DG_TOUCHSCREEN ||
-	    field->application == HID_DG_TOUCHPAD) {
+	rdata = mt_find_report_data(td, field->report);
+	if (rdata && rdata->is_mt_collection) {
 		/* We own these mappings, tell hid-input to ignore them */
 		return -1;
 	}
@@ -1460,6 +1479,7 @@ static int mt_input_configured(struct hid_device *hdev, struct hid_input *hi)
 		case HID_GD_SYSTEM_CONTROL:
 		case HID_CP_CONSUMER_CONTROL:
 		case HID_GD_WIRELESS_RADIO_CTLS:
+		case HID_GD_SYSTEM_MULTIAXIS:
 			/* already handled by hid core */
 			break;
 		case HID_DG_TOUCHSCREEN:
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 773bcb1d4044..2e4498d52a2f 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -190,6 +190,12 @@ struct hid_item {
  * http://www.usb.org/developers/hidpage/HUTRR40RadioHIDUsagesFinal.pdf
  */
 #define HID_GD_WIRELESS_RADIO_CTLS	0x0001000c
+/*
+ * System Multi-Axis, see:
+ * http://www.usb.org/developers/hidpage/HUTRR62_-_Generic_Desktop_CA_for_System_Multi-Axis_Controllers.txt
+ */
+#define HID_GD_SYSTEM_MULTIAXIS	0x0001000e
+
 #define HID_GD_X		0x00010030
 #define HID_GD_Y		0x00010031
 #define HID_GD_Z		0x00010032
-- 
2.14.3

^ permalink raw reply related

* [PATCH v3 06/12] HID: multitouch: remove one copy of values
From: Benjamin Tissoires @ 2018-06-21 12:09 UTC (permalink / raw)
  To: Jiri Kosina, Dmitry Torokhov
  Cc: Mario.Limonciello, Peter Hutterer, linux-input, linux-kernel,
	Benjamin Tissoires
In-Reply-To: <20180621120908.16706-1-benjamin.tissoires@redhat.com>

The current way of handling multitouch data is not very straightforward:
- in mt_event() we do nothing
- in mt_report() we:
  - do some gym to fetch the scantime and the contact count
  - then iterate over the input fields where we copy the data to a
    temporary place
  - when we see the last field in a slot, we then use this data to emit
    the input data

A more streamlined way is to first get all of the address in the report
of all fields, and then just pick the fields we are interested in in
mt_report()

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

no changes in v2
no changes in v3
---
 drivers/hid/hid-multitouch.c | 545 +++++++++++++++++++++----------------------
 1 file changed, 264 insertions(+), 281 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index a2c10fc62ef2..346e9caef6f3 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -87,30 +87,34 @@ enum latency_mode {
 #define MT_IO_FLAGS_ACTIVE_SLOTS	1
 #define MT_IO_FLAGS_PENDING_SLOTS	2
 
-struct mt_slot {
-	__s32 x, y, cx, cy, p, w, h, a;
-	__s32 contactid;	/* the device ContactID assigned to this slot */
-	bool touch_state;	/* is the touch valid? */
-	bool inrange_state;	/* is the finger in proximity of the sensor? */
-	bool confidence_state;  /* is the touch made by a finger? */
-	bool has_azimuth;       /* the contact reports azimuth */
+static const bool mtrue = true;		/* default for true */
+static const bool mfalse;		/* default for false */
+static const __s32 mzero;		/* default for 0 */
+
+#define DEFAULT_TRUE	((void *)&mtrue)
+#define DEFAULT_FALSE	((void *)&mfalse)
+#define DEFAULT_ZERO	((void *)&mzero)
+
+struct mt_usages {
+	struct list_head list;
+	__s32 *x, *y, *cx, *cy, *p, *w, *h, *a;
+	__s32 *contactid;	/* the device ContactID assigned to this slot */
+	bool *tip_state;	/* is the touch valid? */
+	bool *inrange_state;	/* is the finger in proximity of the sensor? */
+	bool *confidence_state;	/* is the touch made by a finger? */
 };
 
 struct mt_application {
 	struct list_head list;
 	unsigned int application;
+	struct list_head mt_usages;	/* mt usages list */
 
 	__s32 quirks;
 
-	struct mt_slot curdata;		/* placeholder of incoming data */
-
-	int cc_index;	/* contact count field index in the report */
-	int cc_value_index;	/* contact count value index in the field */
-	int scantime_index;	/* scantime field index in the report */
-	int scantime_val_index;	/* scantime value index in the field */
-	unsigned int last_slot_field;	/* the last field of a slot */
-	bool curvalid;		/* is the current contact valid? */
+	__s32 *scantime;		/* scantime reported */
+	__s32 scantime_logical_max;	/* max value for raw scantime */
 
+	__s32 *raw_cc;			/* contact count in the report */
 	int left_button_state;		/* left button state */
 	unsigned int mt_flags;		/* flags to pass to input-mt */
 
@@ -142,11 +146,6 @@ struct mt_class {
 	bool export_all_inputs;	/* do not ignore mouse, keyboards, etc... */
 };
 
-struct mt_fields {
-	unsigned usages[HID_MAX_FIELDS];
-	unsigned int length;
-};
-
 struct mt_report_data {
 	struct list_head list;
 	struct hid_report *report;
@@ -158,8 +157,6 @@ struct mt_device {
 	struct mt_class mtclass;	/* our mt device class */
 	struct timer_list release_timer;	/* to release sticky fingers */
 	struct hid_device *hdev;	/* hid_device we're attached to */
-	struct mt_fields *fields;	/* temporary placeholder for storing the
-					   multitouch fields */
 	unsigned long mt_io_flags;	/* mt flags (MT_IO_FLAGS_*) */
 	__u8 inputmode_value;	/* InputMode HID feature value */
 	__u8 maxcontacts;
@@ -225,10 +222,11 @@ static void mt_post_parse(struct mt_device *td, struct mt_application *app);
  * to a valid contact that was just read.
  */
 
-static int cypress_compute_slot(struct mt_application *app)
+static int cypress_compute_slot(struct mt_application *application,
+				struct mt_usages *slot)
 {
-	if (app->curdata.contactid != 0 || app->num_received == 0)
-		return app->curdata.contactid;
+	if (*slot->contactid != 0 || application->num_received == 0)
+		return *slot->contactid;
 	else
 		return -1;
 }
@@ -483,6 +481,34 @@ static void set_abs(struct input_dev *input, unsigned int code,
 	input_abs_set_res(input, code, hidinput_calc_abs_res(field, code));
 }
 
+static struct mt_usages *mt_allocate_usage(struct hid_device *hdev,
+					   struct mt_application *application)
+{
+	struct mt_usages *usage;
+
+	usage = devm_kzalloc(&hdev->dev, sizeof(*usage), GFP_KERNEL);
+	if (!usage)
+		return NULL;
+
+	/* set some defaults so we do not need to check for null pointers */
+	usage->x = DEFAULT_ZERO;
+	usage->y = DEFAULT_ZERO;
+	usage->cx = DEFAULT_ZERO;
+	usage->cy = DEFAULT_ZERO;
+	usage->p = DEFAULT_ZERO;
+	usage->w = DEFAULT_ZERO;
+	usage->h = DEFAULT_ZERO;
+	usage->a = DEFAULT_ZERO;
+	usage->contactid = DEFAULT_ZERO;
+	usage->tip_state = DEFAULT_FALSE;
+	usage->inrange_state = DEFAULT_FALSE;
+	usage->confidence_state = DEFAULT_TRUE;
+
+	list_add_tail(&usage->list, &application->mt_usages);
+
+	return usage;
+}
+
 static struct mt_application *mt_allocate_application(struct mt_device *td,
 						      unsigned int application)
 {
@@ -494,6 +520,7 @@ static struct mt_application *mt_allocate_application(struct mt_device *td,
 		return NULL;
 
 	mt_application->application = application;
+	INIT_LIST_HEAD(&mt_application->mt_usages);
 
 	if (application == HID_DG_TOUCHSCREEN)
 		mt_application->mt_flags |= INPUT_MT_DIRECT;
@@ -506,8 +533,8 @@ static struct mt_application *mt_allocate_application(struct mt_device *td,
 		td->inputmode_value = MT_INPUTMODE_TOUCHPAD;
 	}
 
-	mt_application->cc_index = -1;
-	mt_application->scantime_index = -1;
+	mt_application->scantime = DEFAULT_ZERO;
+	mt_application->raw_cc = DEFAULT_ZERO;
 	mt_application->quirks = td->mtclass.quirks;
 
 	list_add_tail(&mt_application->list, &td->applications);
@@ -587,17 +614,45 @@ static struct mt_report_data *mt_find_report_data(struct mt_device *td,
 	return rdata;
 }
 
-static void mt_store_field(struct hid_usage *usage, struct mt_device *td,
-		struct hid_input *hi)
+static void mt_store_field(struct hid_device *hdev,
+			   struct mt_application *application,
+			   __s32 *value,
+			   size_t offset)
 {
-	struct mt_fields *f = td->fields;
+	struct mt_usages *usage;
+	__s32 **target;
+
+	if (list_empty(&application->mt_usages))
+		usage = mt_allocate_usage(hdev, application);
+	else
+		usage = list_last_entry(&application->mt_usages,
+					struct mt_usages,
+					list);
 
-	if (f->length >= HID_MAX_FIELDS)
+	if (!usage)
 		return;
 
-	f->usages[f->length++] = usage->hid;
+	target = (__s32 **)((char *)usage + offset);
+
+	/* the value has already been filled, create a new slot */
+	if (*target != DEFAULT_TRUE &&
+	    *target != DEFAULT_FALSE &&
+	    *target != DEFAULT_ZERO) {
+		usage = mt_allocate_usage(hdev, application);
+		if (!usage)
+			return;
+
+		target = (__s32 **)((char *)usage + offset);
+	}
+
+	*target = value;
 }
 
+#define MT_STORE_FIELD(__name)						\
+	mt_store_field(hdev, app,					\
+		       &field->value[usage->usage_index],		\
+		       offsetof(struct mt_usages, __name))
+
 static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		struct hid_field *field, struct hid_usage *usage,
 		unsigned long **bit, int *max, struct mt_application *app)
@@ -627,24 +682,28 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 	case HID_UP_GENDESK:
 		switch (usage->hid) {
 		case HID_GD_X:
-			if (prev_usage && (prev_usage->hid == usage->hid))
+			if (prev_usage && (prev_usage->hid == usage->hid)) {
 				code = ABS_MT_TOOL_X;
-			else
+				MT_STORE_FIELD(cx);
+			} else {
 				code = ABS_MT_POSITION_X;
+				MT_STORE_FIELD(x);
+			}
 
-			hid_map_usage(hi, usage, bit, max, EV_ABS, code);
 			set_abs(hi->input, code, field, cls->sn_move);
-			mt_store_field(usage, td, hi);
+
 			return 1;
 		case HID_GD_Y:
-			if (prev_usage && (prev_usage->hid == usage->hid))
+			if (prev_usage && (prev_usage->hid == usage->hid)) {
 				code = ABS_MT_TOOL_Y;
-			else
+				MT_STORE_FIELD(cy);
+			} else {
 				code = ABS_MT_POSITION_Y;
+				MT_STORE_FIELD(y);
+			}
 
-			hid_map_usage(hi, usage, bit, max, EV_ABS, code);
 			set_abs(hi->input, code, field, cls->sn_move);
-			mt_store_field(usage, td, hi);
+
 			return 1;
 		}
 		return 0;
@@ -653,40 +712,33 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		switch (usage->hid) {
 		case HID_DG_INRANGE:
 			if (app->quirks & MT_QUIRK_HOVERING) {
-				hid_map_usage(hi, usage, bit, max,
-					EV_ABS, ABS_MT_DISTANCE);
 				input_set_abs_params(hi->input,
 					ABS_MT_DISTANCE, 0, 1, 0, 0);
 			}
-			mt_store_field(usage, td, hi);
+			MT_STORE_FIELD(inrange_state);
 			return 1;
 		case HID_DG_CONFIDENCE:
 			if ((cls->name == MT_CLS_WIN_8 ||
 				cls->name == MT_CLS_WIN_8_DUAL) &&
 				field->application == HID_DG_TOUCHPAD)
 				app->quirks |= MT_QUIRK_CONFIDENCE;
-			mt_store_field(usage, td, hi);
+			MT_STORE_FIELD(confidence_state);
 			return 1;
 		case HID_DG_TIPSWITCH:
-			hid_map_usage(hi, usage, bit, max, EV_KEY, BTN_TOUCH);
 			input_set_capability(hi->input, EV_KEY, BTN_TOUCH);
-			mt_store_field(usage, td, hi);
+			MT_STORE_FIELD(tip_state);
 			return 1;
 		case HID_DG_CONTACTID:
-			mt_store_field(usage, td, hi);
+			MT_STORE_FIELD(contactid);
 			app->touches_by_report++;
 			return 1;
 		case HID_DG_WIDTH:
-			hid_map_usage(hi, usage, bit, max,
-					EV_ABS, ABS_MT_TOUCH_MAJOR);
 			if (!(app->quirks & MT_QUIRK_NO_AREA))
 				set_abs(hi->input, ABS_MT_TOUCH_MAJOR, field,
 					cls->sn_width);
-			mt_store_field(usage, td, hi);
+			MT_STORE_FIELD(w);
 			return 1;
 		case HID_DG_HEIGHT:
-			hid_map_usage(hi, usage, bit, max,
-					EV_ABS, ABS_MT_TOUCH_MINOR);
 			if (!(app->quirks & MT_QUIRK_NO_AREA)) {
 				set_abs(hi->input, ABS_MT_TOUCH_MINOR, field,
 					cls->sn_height);
@@ -700,37 +752,23 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 					input_set_abs_params(hi->input,
 						ABS_MT_ORIENTATION, 0, 1, 0, 0);
 			}
-			mt_store_field(usage, td, hi);
+			MT_STORE_FIELD(h);
 			return 1;
 		case HID_DG_TIPPRESSURE:
-			hid_map_usage(hi, usage, bit, max,
-					EV_ABS, ABS_MT_PRESSURE);
 			set_abs(hi->input, ABS_MT_PRESSURE, field,
 				cls->sn_pressure);
-			mt_store_field(usage, td, hi);
+			MT_STORE_FIELD(p);
 			return 1;
 		case HID_DG_SCANTIME:
-			hid_map_usage(hi, usage, bit, max,
-				EV_MSC, MSC_TIMESTAMP);
 			input_set_capability(hi->input, EV_MSC, MSC_TIMESTAMP);
-			/* Ignore if indexes are out of bounds. */
-			if (field->index >= field->report->maxfield ||
-			    usage->usage_index >= field->report_count)
-				return 1;
-			app->scantime_index = field->index;
-			app->scantime_val_index = usage->usage_index;
+			app->scantime = &field->value[usage->usage_index];
+			app->scantime_logical_max = field->logical_maximum;
 			return 1;
 		case HID_DG_CONTACTCOUNT:
-			/* Ignore if indexes are out of bounds. */
-			if (field->index >= field->report->maxfield ||
-			    usage->usage_index >= field->report_count)
-				return 1;
-			app->cc_index = field->index;
-			app->cc_value_index = usage->usage_index;
+			app->have_contact_count = true;
+			app->raw_cc = &field->value[usage->usage_index];
 			return 1;
 		case HID_DG_AZIMUTH:
-			hid_map_usage(hi, usage, bit, max,
-				EV_ABS, ABS_MT_ORIENTATION);
 			/*
 			 * Azimuth has the range of [0, MAX) representing a full
 			 * revolution. Set ABS_MT_ORIENTATION to a quarter of
@@ -741,11 +779,10 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 				field->logical_maximum / 4,
 				cls->sn_move ?
 				field->logical_maximum / cls->sn_move : 0, 0);
-			mt_store_field(usage, td, hi);
+			MT_STORE_FIELD(a);
 			return 1;
 		case HID_DG_CONTACTMAX:
-			/* we don't set td->last_slot_field as contactcount and
-			 * contact max are global to the report */
+			/* contact max are global to the report */
 			return -1;
 		case HID_DG_TOUCH:
 			/* Legacy devices use TIPSWITCH and not TOUCH.
@@ -778,95 +815,24 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 }
 
 static int mt_compute_slot(struct mt_device *td, struct mt_application *app,
+			   struct mt_usages *slot,
 			   struct input_dev *input)
 {
 	__s32 quirks = app->quirks;
 
 	if (quirks & MT_QUIRK_SLOT_IS_CONTACTID)
-		return app->curdata.contactid;
+		return *slot->contactid;
 
 	if (quirks & MT_QUIRK_CYPRESS)
-		return cypress_compute_slot(app);
+		return cypress_compute_slot(app, slot);
 
 	if (quirks & MT_QUIRK_SLOT_IS_CONTACTNUMBER)
 		return app->num_received;
 
 	if (quirks & MT_QUIRK_SLOT_IS_CONTACTID_MINUS_ONE)
-		return app->curdata.contactid - 1;
+		return *slot->contactid - 1;
 
-	return input_mt_get_slot_by_key(input, app->curdata.contactid);
-}
-
-/*
- * this function is called when a whole contact has been processed,
- * so that it can assign it to a slot and store the data there
- */
-static void mt_complete_slot(struct mt_device *td, struct mt_application *app,
-			     struct input_dev *input)
-{
-	if ((app->quirks & MT_QUIRK_CONTACT_CNT_ACCURATE) &&
-	    app->num_received >= app->num_expected)
-		return;
-
-	if (app->curvalid || (app->quirks & MT_QUIRK_ALWAYS_VALID)) {
-		int active;
-		int slotnum = mt_compute_slot(td, app, input);
-		struct mt_slot *s = &app->curdata;
-		struct input_mt *mt = input->mt;
-
-		if (slotnum < 0 || slotnum >= td->maxcontacts)
-			return;
-
-		if ((app->quirks & MT_QUIRK_IGNORE_DUPLICATES) && mt) {
-			struct input_mt_slot *slot = &mt->slots[slotnum];
-			if (input_mt_is_active(slot) &&
-			    input_mt_is_used(mt, slot))
-				return;
-		}
-
-		if (!(app->quirks & MT_QUIRK_CONFIDENCE))
-			s->confidence_state = true;
-		active = (s->touch_state || s->inrange_state) &&
-							s->confidence_state;
-
-		input_mt_slot(input, slotnum);
-		input_mt_report_slot_state(input, MT_TOOL_FINGER, active);
-		if (active) {
-			/* this finger is in proximity of the sensor */
-			int wide = (s->w > s->h);
-			int major = max(s->w, s->h);
-			int minor = min(s->w, s->h);
-			int orientation = wide;
-
-			if (s->has_azimuth)
-				orientation = s->a;
-
-			/*
-			 * divided by two to match visual scale of touch
-			 * for devices with this quirk
-			 */
-			if (app->quirks & MT_QUIRK_TOUCH_SIZE_SCALING) {
-				major = major >> 1;
-				minor = minor >> 1;
-			}
-
-			input_event(input, EV_ABS, ABS_MT_POSITION_X, s->x);
-			input_event(input, EV_ABS, ABS_MT_POSITION_Y, s->y);
-			input_event(input, EV_ABS, ABS_MT_TOOL_X, s->cx);
-			input_event(input, EV_ABS, ABS_MT_TOOL_Y, s->cy);
-			input_event(input, EV_ABS, ABS_MT_DISTANCE,
-				!s->touch_state);
-			input_event(input, EV_ABS, ABS_MT_ORIENTATION,
-				orientation);
-			input_event(input, EV_ABS, ABS_MT_PRESSURE, s->p);
-			input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major);
-			input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor);
-
-			set_bit(MT_IO_FLAGS_ACTIVE_SLOTS, &td->mt_io_flags);
-		}
-	}
-
-	app->num_received++;
+	return input_mt_get_slot_by_key(input, *slot->contactid);
 }
 
 /*
@@ -892,8 +858,7 @@ static void mt_sync_frame(struct mt_device *td, struct mt_application *app,
 	clear_bit(MT_IO_FLAGS_ACTIVE_SLOTS, &td->mt_io_flags);
 }
 
-static int mt_compute_timestamp(struct mt_application *app,
-				struct hid_field *field, __s32 value)
+static int mt_compute_timestamp(struct mt_application *app, __s32 value)
 {
 	long delta = value - app->prev_scantime;
 	unsigned long jdelta = jiffies_to_usecs(jiffies - app->jiffies);
@@ -901,7 +866,7 @@ static int mt_compute_timestamp(struct mt_application *app,
 	app->jiffies = jiffies;
 
 	if (delta < 0)
-		delta += field->logical_maximum;
+		delta += app->scantime_logical_max;
 
 	/* HID_DG_SCANTIME is expressed in 100us, we want it in us. */
 	delta *= 100;
@@ -923,64 +888,69 @@ static int mt_touch_event(struct hid_device *hid, struct hid_field *field,
 	return 1;
 }
 
-static void mt_process_mt_event(struct hid_device *hid, struct hid_field *field,
-				struct hid_usage *usage, __s32 value,
-				struct mt_application *app, bool first_packet)
+static int mt_process_slot(struct mt_device *td, struct input_dev *input,
+			    struct mt_application *app,
+			    struct mt_usages *slot)
 {
-	struct mt_device *td = hid_get_drvdata(hid);
+	struct input_mt *mt = input->mt;
 	__s32 quirks = app->quirks;
-	struct input_dev *input = field->hidinput->input;
+	bool valid = true;
+	bool confidence_state = true;
+	bool inrange_state = false;
+	int active;
+	int slotnum;
 
-	if (hid->claimed & HID_CLAIMED_INPUT) {
-		switch (usage->hid) {
-		case HID_DG_INRANGE:
-			if (quirks & MT_QUIRK_VALID_IS_INRANGE)
-				app->curvalid = value;
-			if (quirks & MT_QUIRK_HOVERING)
-				app->curdata.inrange_state = value;
-			break;
-		case HID_DG_TIPSWITCH:
-			if (quirks & MT_QUIRK_NOT_SEEN_MEANS_UP)
-				app->curvalid = value;
-			app->curdata.touch_state = value;
-			break;
-		case HID_DG_CONFIDENCE:
-			if (quirks & MT_QUIRK_CONFIDENCE)
-				app->curdata.confidence_state = value;
-			if (quirks & MT_QUIRK_VALID_IS_CONFIDENCE)
-				app->curvalid = value;
-			break;
-		case HID_DG_CONTACTID:
-			app->curdata.contactid = value;
-			break;
-		case HID_DG_TIPPRESSURE:
-			app->curdata.p = value;
-			break;
-		case HID_GD_X:
-			if (usage->code == ABS_MT_TOOL_X)
-				app->curdata.cx = value;
-			else
-				app->curdata.x = value;
-			break;
-		case HID_GD_Y:
-			if (usage->code == ABS_MT_TOOL_Y)
-				app->curdata.cy = value;
-			else
-				app->curdata.y = value;
-			break;
-		case HID_DG_WIDTH:
-			app->curdata.w = value;
-			break;
-		case HID_DG_HEIGHT:
-			app->curdata.h = value;
-			break;
-		case HID_DG_SCANTIME:
-			app->timestamp = mt_compute_timestamp(app, field,
-							      value);
-			break;
-		case HID_DG_CONTACTCOUNT:
-			break;
-		case HID_DG_AZIMUTH:
+	if (!slot)
+		return -EINVAL;
+
+	if ((quirks & MT_QUIRK_CONTACT_CNT_ACCURATE) &&
+	    app->num_received >= app->num_expected)
+		return -EAGAIN;
+
+	if (!(quirks & MT_QUIRK_ALWAYS_VALID)) {
+		if (quirks & MT_QUIRK_VALID_IS_INRANGE)
+			valid = *slot->inrange_state;
+		if (quirks & MT_QUIRK_NOT_SEEN_MEANS_UP)
+			valid = *slot->tip_state;
+		if (quirks & MT_QUIRK_VALID_IS_CONFIDENCE)
+			valid = *slot->confidence_state;
+
+		if (!valid)
+			return 0;
+	}
+
+	slotnum = mt_compute_slot(td, app, slot, input);
+	if (slotnum < 0 || slotnum >= td->maxcontacts)
+		return 0;
+
+	if ((quirks & MT_QUIRK_IGNORE_DUPLICATES) && mt) {
+		struct input_mt_slot *i_slot = &mt->slots[slotnum];
+
+		if (input_mt_is_active(i_slot) &&
+		    input_mt_is_used(mt, i_slot))
+			return -EAGAIN;
+	}
+
+	if (quirks & MT_QUIRK_CONFIDENCE)
+		confidence_state = *slot->confidence_state;
+
+	if (quirks & MT_QUIRK_HOVERING)
+		inrange_state = *slot->inrange_state;
+
+	active = (*slot->tip_state || inrange_state) && confidence_state;
+
+	input_mt_slot(input, slotnum);
+	input_mt_report_slot_state(input, MT_TOOL_FINGER, active);
+	if (active) {
+		/* this finger is in proximity of the sensor */
+		int wide = (*slot->w > *slot->h);
+		int major = max(*slot->w, *slot->h);
+		int minor = min(*slot->w, *slot->h);
+		int orientation = wide;
+		int max_azimuth;
+		int azimuth;
+
+		if (slot->a != DEFAULT_ZERO) {
 			/*
 			 * Azimuth is counter-clockwise and ranges from [0, MAX)
 			 * (a full revolution). Convert it to clockwise ranging
@@ -991,52 +961,76 @@ static void mt_process_mt_event(struct hid_device *hid, struct hid_field *field,
 			 * out of range to [-MAX/2, MAX/2] to report an upside
 			 * down ellipsis.
 			 */
-			if (value > field->logical_maximum / 2)
-				value -= field->logical_maximum;
-			app->curdata.a = -value;
-			app->curdata.has_azimuth = true;
-			break;
-		case HID_DG_TOUCH:
-			/* do nothing */
-			break;
+			azimuth = *slot->a;
+			max_azimuth = input_abs_get_max(input,
+							ABS_MT_ORIENTATION);
+			if (azimuth > max_azimuth * 2)
+				azimuth -= max_azimuth * 4;
+			orientation = -azimuth;
+		}
 
-		default:
-			/*
-			 * For Win8 PTP touchpads we should only look at
-			 * non finger/touch events in the first_packet of
-			 * a (possible) multi-packet frame.
-			 */
-			if ((quirks & MT_QUIRK_WIN8_PTP_BUTTONS) &&
-			    !first_packet)
-				return;
+		/*
+		 * divided by two to match visual scale of touch
+		 * for devices with this quirk
+		 */
+		if (quirks & MT_QUIRK_TOUCH_SIZE_SCALING) {
+			major = major >> 1;
+			minor = minor >> 1;
+		}
 
-			/*
-			 * For Win8 PTP touchpads we map both the clickpad click
-			 * and any "external" left buttons to BTN_LEFT if a
-			 * device claims to have both we need to report 1 for
-			 * BTN_LEFT if either is pressed, so we or all values
-			 * together and report the result in mt_sync_frame().
-			 */
-			if ((quirks & MT_QUIRK_WIN8_PTP_BUTTONS) &&
-			    usage->type == EV_KEY && usage->code == BTN_LEFT) {
-				app->left_button_state |= value;
-				return;
-			}
+		input_event(input, EV_ABS, ABS_MT_POSITION_X, *slot->x);
+		input_event(input, EV_ABS, ABS_MT_POSITION_Y, *slot->y);
+		input_event(input, EV_ABS, ABS_MT_TOOL_X, *slot->cx);
+		input_event(input, EV_ABS, ABS_MT_TOOL_Y, *slot->cy);
+		input_event(input, EV_ABS, ABS_MT_DISTANCE, !*slot->tip_state);
+		input_event(input, EV_ABS, ABS_MT_ORIENTATION, orientation);
+		input_event(input, EV_ABS, ABS_MT_PRESSURE, *slot->p);
+		input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major);
+		input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor);
+
+		set_bit(MT_IO_FLAGS_ACTIVE_SLOTS, &td->mt_io_flags);
+	}
 
-			if (usage->type)
-				input_event(input, usage->type, usage->code,
-						value);
+	return 0;
+}
+
+static void mt_process_mt_event(struct hid_device *hid,
+				struct mt_application *app,
+				struct hid_field *field,
+				struct hid_usage *usage,
+				__s32 value,
+				bool first_packet)
+{
+	__s32 quirks = app->quirks;
+	struct input_dev *input = field->hidinput->input;
+
+	if (!usage->type || !(hid->claimed & HID_CLAIMED_INPUT))
+		return;
+
+	if (quirks & MT_QUIRK_WIN8_PTP_BUTTONS) {
+
+		/*
+		 * For Win8 PTP touchpads we should only look at
+		 * non finger/touch events in the first_packet of a
+		 * (possible) multi-packet frame.
+		 */
+		if (!first_packet)
 			return;
-		}
 
-		if (usage->usage_index + 1 == field->report_count) {
-			/* we only take into account the last report. */
-			if (usage->hid == app->last_slot_field)
-				mt_complete_slot(td, app,
-						 field->hidinput->input);
+		/*
+		 * For Win8 PTP touchpads we map both the clickpad click
+		 * and any "external" left buttons to BTN_LEFT if a
+		 * device claims to have both we need to report 1 for
+		 * BTN_LEFT if either is pressed, so we or all values
+		 * together and report the result in mt_sync_frame().
+		 */
+		if (usage->type == EV_KEY && usage->code == BTN_LEFT) {
+			app->left_button_state |= value;
+			return;
 		}
-
 	}
+
+	input_event(input, usage->type, usage->code, value);
 }
 
 static void mt_touch_report(struct hid_device *hid,
@@ -1046,6 +1040,8 @@ static void mt_touch_report(struct hid_device *hid,
 	struct hid_report *report = rdata->report;
 	struct mt_application *app = rdata->application;
 	struct hid_field *field;
+	struct input_dev *input;
+	struct mt_usages *slot;
 	bool first_packet;
 	unsigned count;
 	int r, n;
@@ -1056,18 +1052,16 @@ static void mt_touch_report(struct hid_device *hid,
 	if (test_and_set_bit(MT_IO_FLAGS_RUNNING, &td->mt_io_flags))
 		return;
 
+	scantime = *app->scantime;
+	app->timestamp = mt_compute_timestamp(app, scantime);
+	if (app->raw_cc != DEFAULT_ZERO)
+		contact_count = *app->raw_cc;
+
 	/*
 	 * Includes multi-packet support where subsequent
 	 * packets are sent with zero contactcount.
 	 */
-	if (app->scantime_index >= 0) {
-		field = report->field[app->scantime_index];
-		scantime = field->value[app->scantime_val_index];
-	}
-	if (app->cc_index >= 0) {
-		field = report->field[app->cc_index];
-		contact_count = field->value[app->cc_value_index];
-
+	if (contact_count >= 0) {
 		/*
 		 * For Win8 PTPs the first packet (td->num_received == 0) may
 		 * have a contactcount of 0 if there only is a button event.
@@ -1086,6 +1080,14 @@ static void mt_touch_report(struct hid_device *hid,
 	app->prev_scantime = scantime;
 
 	first_packet = app->num_received == 0;
+
+	input = report->field[0]->hidinput->input;
+
+	list_for_each_entry(slot, &app->mt_usages, list) {
+		if (!mt_process_slot(td, input, app, slot))
+			app->num_received++;
+	}
+
 	for (r = 0; r < report->maxfield; r++) {
 		field = report->field[r];
 		count = field->report_count;
@@ -1094,12 +1096,13 @@ static void mt_touch_report(struct hid_device *hid,
 			continue;
 
 		for (n = 0; n < count; n++)
-			mt_process_mt_event(hid, field, &field->usage[n],
-					    field->value[n], app, first_packet);
+			mt_process_mt_event(hid, app, field,
+					    &field->usage[n], field->value[n],
+					    first_packet);
 	}
 
 	if (app->num_received >= app->num_expected)
-		mt_sync_frame(td, app, report->field[0]->hidinput->input);
+		mt_sync_frame(td, app, input);
 
 	/*
 	 * Windows 8 specs says 2 things:
@@ -1391,7 +1394,7 @@ static void mt_post_parse_default_settings(struct mt_device *td,
 	__s32 quirks = app->quirks;
 
 	/* unknown serial device needs special quirks */
-	if (app->touches_by_report == 1) {
+	if (list_is_singular(&app->mt_usages)) {
 		quirks |= MT_QUIRK_ALWAYS_VALID;
 		quirks &= ~MT_QUIRK_NOT_SEEN_MEANS_UP;
 		quirks &= ~MT_QUIRK_VALID_IS_INRANGE;
@@ -1404,16 +1407,7 @@ static void mt_post_parse_default_settings(struct mt_device *td,
 
 static void mt_post_parse(struct mt_device *td, struct mt_application *app)
 {
-	struct mt_fields *f = td->fields;
-
-	if (app->touches_by_report > 0) {
-		int field_count_per_touch;
-
-		field_count_per_touch = f->length / app->touches_by_report;
-		app->last_slot_field = f->usages[field_count_per_touch - 1];
-	}
-
-	if (app->cc_index < 0)
+	if (!app->have_contact_count)
 		app->quirks &= ~MT_QUIRK_CONTACT_CNT_ACCURATE;
 }
 
@@ -1596,13 +1590,6 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	INIT_LIST_HEAD(&td->applications);
 	INIT_LIST_HEAD(&td->reports);
 
-	td->fields = devm_kzalloc(&hdev->dev, sizeof(struct mt_fields),
-				  GFP_KERNEL);
-	if (!td->fields) {
-		dev_err(&hdev->dev, "cannot allocate multitouch fields data\n");
-		return -ENOMEM;
-	}
-
 	if (id->vendor == HID_ANY_ID && id->product == HID_ANY_ID)
 		td->serial_maybe = true;
 
@@ -1638,10 +1625,6 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
 
 	mt_set_modes(hdev, HID_LATENCY_NORMAL, true, true);
 
-	/* release .fields memory as it is not used anymore */
-	devm_kfree(&hdev->dev, td->fields);
-	td->fields = NULL;
-
 	return 0;
 }
 
-- 
2.14.3

^ permalink raw reply related

* [PATCH v3 05/12] HID: multitouch: ditch mt_report_id
From: Benjamin Tissoires @ 2018-06-21 12:09 UTC (permalink / raw)
  To: Jiri Kosina, Dmitry Torokhov
  Cc: Mario.Limonciello, Peter Hutterer, linux-input, linux-kernel,
	Benjamin Tissoires
In-Reply-To: <20180621120908.16706-1-benjamin.tissoires@redhat.com>

Now that the driver can handle more than one multitouch collection in
a single HID device, ditch the last bit that contains us to use only
one mt collection.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

no changes in v2
no changes in v3
---
 drivers/hid/hid-multitouch.c | 126 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 94 insertions(+), 32 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index beaac36f61a7..a2c10fc62ef2 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -147,6 +147,13 @@ struct mt_fields {
 	unsigned int length;
 };
 
+struct mt_report_data {
+	struct list_head list;
+	struct hid_report *report;
+	struct mt_application *application;
+	bool is_mt_collection;
+};
+
 struct mt_device {
 	struct mt_class mtclass;	/* our mt device class */
 	struct timer_list release_timer;	/* to release sticky fingers */
@@ -154,13 +161,13 @@ struct mt_device {
 	struct mt_fields *fields;	/* temporary placeholder for storing the
 					   multitouch fields */
 	unsigned long mt_io_flags;	/* mt flags (MT_IO_FLAGS_*) */
-	unsigned mt_report_id;	/* the report ID of the multitouch device */
 	__u8 inputmode_value;	/* InputMode HID feature value */
 	__u8 maxcontacts;
 	bool is_buttonpad;	/* is this device a button pad? */
 	bool serial_maybe;	/* need to check for serial protocol */
 
 	struct list_head applications;
+	struct list_head reports;
 };
 
 static void mt_post_parse_default_settings(struct mt_device *td,
@@ -526,6 +533,60 @@ static struct mt_application *mt_find_application(struct mt_device *td,
 	return mt_application;
 }
 
+static struct mt_report_data *mt_allocate_report_data(struct mt_device *td,
+						      struct hid_report *report)
+{
+	struct mt_report_data *rdata;
+	struct hid_field *field;
+	int r, n;
+
+	rdata = devm_kzalloc(&td->hdev->dev, sizeof(*rdata), GFP_KERNEL);
+	if (!rdata)
+		return NULL;
+
+	rdata->report = report;
+	rdata->application = mt_find_application(td, report->application);
+
+	if (!rdata->application) {
+		devm_kfree(&td->hdev->dev, rdata);
+		return NULL;
+	}
+
+	for (r = 0; r < report->maxfield; r++) {
+		field = report->field[r];
+
+		if (!(HID_MAIN_ITEM_VARIABLE & field->flags))
+			continue;
+
+		for (n = 0; n < field->report_count; n++) {
+			if (field->usage[n].hid == HID_DG_CONTACTID)
+				rdata->is_mt_collection = true;
+		}
+	}
+
+	list_add_tail(&rdata->list, &td->reports);
+
+	return rdata;
+}
+
+static struct mt_report_data *mt_find_report_data(struct mt_device *td,
+						  struct hid_report *report)
+{
+	struct mt_report_data *tmp, *rdata = NULL;
+
+	list_for_each_entry(tmp, &td->reports, list) {
+		if (report == tmp->report) {
+			rdata = tmp;
+			break;
+		}
+	}
+
+	if (!rdata)
+		rdata = mt_allocate_report_data(td, report);
+
+	return rdata;
+}
+
 static void mt_store_field(struct hid_usage *usage, struct mt_device *td,
 		struct hid_input *hi)
 {
@@ -614,7 +675,6 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		case HID_DG_CONTACTID:
 			mt_store_field(usage, td, hi);
 			app->touches_by_report++;
-			td->mt_report_id = field->report->id;
 			return 1;
 		case HID_DG_WIDTH:
 			hid_map_usage(hi, usage, bit, max,
@@ -979,10 +1039,12 @@ static void mt_process_mt_event(struct hid_device *hid, struct hid_field *field,
 	}
 }
 
-static void mt_touch_report(struct hid_device *hid, struct hid_report *report)
+static void mt_touch_report(struct hid_device *hid,
+			    struct mt_report_data *rdata)
 {
 	struct mt_device *td = hid_get_drvdata(hid);
-	struct mt_application *app;
+	struct hid_report *report = rdata->report;
+	struct mt_application *app = rdata->application;
 	struct hid_field *field;
 	bool first_packet;
 	unsigned count;
@@ -994,11 +1056,6 @@ static void mt_touch_report(struct hid_device *hid, struct hid_report *report)
 	if (test_and_set_bit(MT_IO_FLAGS_RUNNING, &td->mt_io_flags))
 		return;
 
-	app = mt_find_application(td, report->application);
-
-	if (!app)
-		return;
-
 	/*
 	 * Includes multi-packet support where subsequent
 	 * packets are sent with zero contactcount.
@@ -1119,8 +1176,15 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 {
 	struct mt_device *td = hid_get_drvdata(hdev);
 	struct mt_application *application;
+	struct mt_report_data *rdata;
+
+	rdata = mt_find_report_data(td, field->report);
+	if (!rdata) {
+		hid_err(hdev, "failed to allocate data for report\n");
+		return 0;
+	}
 
-	application = mt_find_application(td, field->application);
+	application = rdata->application;
 
 	/*
 	 * If mtclass.export_all_inputs is not set, only map fields from
@@ -1163,22 +1227,9 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		return 1;
 	}
 
-	/*
-	 * some egalax touchscreens have "application == HID_DG_TOUCHSCREEN"
-	 * for the stylus.
-	 * The check for mt_report_id ensures we don't process
-	 * HID_DG_CONTACTCOUNT from the pen report as it is outside the physical
-	 * collection, but within the report ID.
-	 */
-	if (field->physical == HID_DG_STYLUS)
-		return 0;
-	else if ((field->physical == 0) &&
-		 (field->report->id != td->mt_report_id) &&
-		 (td->mt_report_id != -1))
-		return 0;
-
-	if (field->application == HID_DG_TOUCHSCREEN ||
-	    field->application == HID_DG_TOUCHPAD)
+	if (rdata->is_mt_collection &&
+	    (field->application == HID_DG_TOUCHSCREEN ||
+	     field->application == HID_DG_TOUCHPAD))
 		return mt_touch_input_mapping(hdev, hi, field, usage, bit, max,
 					      application);
 
@@ -1211,8 +1262,10 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
 				struct hid_usage *usage, __s32 value)
 {
 	struct mt_device *td = hid_get_drvdata(hid);
+	struct mt_report_data *rdata;
 
-	if (field->report->id == td->mt_report_id)
+	rdata = mt_find_report_data(td, field->report);
+	if (rdata && rdata->is_mt_collection)
 		return mt_touch_event(hid, field, usage, value);
 
 	return 0;
@@ -1222,12 +1275,14 @@ static void mt_report(struct hid_device *hid, struct hid_report *report)
 {
 	struct mt_device *td = hid_get_drvdata(hid);
 	struct hid_field *field = report->field[0];
+	struct mt_report_data *rdata;
 
 	if (!(hid->claimed & HID_CLAIMED_INPUT))
 		return;
 
-	if (report->id == td->mt_report_id)
-		return mt_touch_report(hid, report);
+	rdata = mt_find_report_data(td, report);
+	if (rdata && rdata->is_mt_collection)
+		return mt_touch_report(hid, rdata);
 
 	if (field && field->hidinput && field->hidinput->input)
 		input_sync(field->hidinput->input);
@@ -1368,15 +1423,22 @@ static int mt_input_configured(struct hid_device *hdev, struct hid_input *hi)
 	char *name;
 	const char *suffix = NULL;
 	unsigned int application = 0;
+	struct mt_report_data *rdata;
 	struct mt_application *mt_application = NULL;
 	struct hid_report *report;
 	int ret;
 
 	list_for_each_entry(report, &hi->reports, hidinput_list) {
 		application = report->application;
-		mt_application = mt_find_application(td, application);
+		rdata = mt_find_report_data(td, report);
+		if (!rdata) {
+			hid_err(hdev, "failed to allocate data for report\n");
+			return -ENOMEM;
+		}
+
+		mt_application = rdata->application;
 
-		if (report->id == td->mt_report_id) {
+		if (rdata->is_mt_collection) {
 			ret = mt_touch_input_configured(hdev, hi,
 							mt_application);
 			if (ret)
@@ -1529,10 +1591,10 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	td->hdev = hdev;
 	td->mtclass = *mtclass;
 	td->inputmode_value = MT_INPUTMODE_TOUCHSCREEN;
-	td->mt_report_id = -1;
 	hid_set_drvdata(hdev, td);
 
 	INIT_LIST_HEAD(&td->applications);
+	INIT_LIST_HEAD(&td->reports);
 
 	td->fields = devm_kzalloc(&hdev->dev, sizeof(struct mt_fields),
 				  GFP_KERNEL);
-- 
2.14.3

^ permalink raw reply related

* [PATCH v3 04/12] HID: multitouch: store a per application quirks value
From: Benjamin Tissoires @ 2018-06-21 12:09 UTC (permalink / raw)
  To: Jiri Kosina, Dmitry Torokhov
  Cc: Mario.Limonciello, Peter Hutterer, linux-input, linux-kernel,
	Benjamin Tissoires
In-Reply-To: <20180621120908.16706-1-benjamin.tissoires@redhat.com>

If a device has more than one multitouch collection, there is a chance
we need per tool quirks. This is the case for the Totem on the Dell
Canvas.

Note that thesysfs attribute quirks can now get out of sync, but there
should not be much users of it as it's debugging only.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

no changes in v2
no changes in v3
---
 drivers/hid/hid-multitouch.c | 64 ++++++++++++++++++++++----------------------
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index cd207e99d26a..beaac36f61a7 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -99,6 +99,9 @@ struct mt_slot {
 struct mt_application {
 	struct list_head list;
 	unsigned int application;
+
+	__s32 quirks;
+
 	struct mt_slot curdata;		/* placeholder of incoming data */
 
 	int cc_index;	/* contact count field index in the report */
@@ -368,7 +371,6 @@ static ssize_t mt_set_quirks(struct device *dev,
 	struct mt_application *application;
 
 	unsigned long val;
-	bool confidence_found = false;
 
 	if (kstrtoul(buf, 0, &val))
 		return -EINVAL;
@@ -376,13 +378,11 @@ static ssize_t mt_set_quirks(struct device *dev,
 	td->mtclass.quirks = val;
 
 	list_for_each_entry(application, &td->applications, list) {
-		if (application->have_contact_count)
-			confidence_found = true;
+		application->quirks = val;
+		if (!application->have_contact_count)
+			application->quirks &= ~MT_QUIRK_CONTACT_CNT_ACCURATE;
 	}
 
-	if (!confidence_found)
-		td->mtclass.quirks &= ~MT_QUIRK_CONTACT_CNT_ACCURATE;
-
 	return count;
 }
 
@@ -501,6 +501,7 @@ static struct mt_application *mt_allocate_application(struct mt_device *td,
 
 	mt_application->cc_index = -1;
 	mt_application->scantime_index = -1;
+	mt_application->quirks = td->mtclass.quirks;
 
 	list_add_tail(&mt_application->list, &td->applications);
 
@@ -590,7 +591,7 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 	case HID_UP_DIGITIZER:
 		switch (usage->hid) {
 		case HID_DG_INRANGE:
-			if (cls->quirks & MT_QUIRK_HOVERING) {
+			if (app->quirks & MT_QUIRK_HOVERING) {
 				hid_map_usage(hi, usage, bit, max,
 					EV_ABS, ABS_MT_DISTANCE);
 				input_set_abs_params(hi->input,
@@ -602,7 +603,7 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 			if ((cls->name == MT_CLS_WIN_8 ||
 				cls->name == MT_CLS_WIN_8_DUAL) &&
 				field->application == HID_DG_TOUCHPAD)
-				cls->quirks |= MT_QUIRK_CONFIDENCE;
+				app->quirks |= MT_QUIRK_CONFIDENCE;
 			mt_store_field(usage, td, hi);
 			return 1;
 		case HID_DG_TIPSWITCH:
@@ -618,7 +619,7 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		case HID_DG_WIDTH:
 			hid_map_usage(hi, usage, bit, max,
 					EV_ABS, ABS_MT_TOUCH_MAJOR);
-			if (!(cls->quirks & MT_QUIRK_NO_AREA))
+			if (!(app->quirks & MT_QUIRK_NO_AREA))
 				set_abs(hi->input, ABS_MT_TOUCH_MAJOR, field,
 					cls->sn_width);
 			mt_store_field(usage, td, hi);
@@ -626,7 +627,7 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		case HID_DG_HEIGHT:
 			hid_map_usage(hi, usage, bit, max,
 					EV_ABS, ABS_MT_TOUCH_MINOR);
-			if (!(cls->quirks & MT_QUIRK_NO_AREA)) {
+			if (!(app->quirks & MT_QUIRK_NO_AREA)) {
 				set_abs(hi->input, ABS_MT_TOUCH_MINOR, field,
 					cls->sn_height);
 
@@ -700,7 +701,7 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		 * MS PTP spec says that external buttons left and right have
 		 * usages 2 and 3.
 		 */
-		if ((cls->quirks & MT_QUIRK_WIN8_PTP_BUTTONS) &&
+		if ((app->quirks & MT_QUIRK_WIN8_PTP_BUTTONS) &&
 		    field->application == HID_DG_TOUCHPAD &&
 		    (usage->hid & HID_USAGE) > 1)
 			code--;
@@ -719,7 +720,7 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 static int mt_compute_slot(struct mt_device *td, struct mt_application *app,
 			   struct input_dev *input)
 {
-	__s32 quirks = td->mtclass.quirks;
+	__s32 quirks = app->quirks;
 
 	if (quirks & MT_QUIRK_SLOT_IS_CONTACTID)
 		return app->curdata.contactid;
@@ -743,11 +744,11 @@ static int mt_compute_slot(struct mt_device *td, struct mt_application *app,
 static void mt_complete_slot(struct mt_device *td, struct mt_application *app,
 			     struct input_dev *input)
 {
-	if ((td->mtclass.quirks & MT_QUIRK_CONTACT_CNT_ACCURATE) &&
+	if ((app->quirks & MT_QUIRK_CONTACT_CNT_ACCURATE) &&
 	    app->num_received >= app->num_expected)
 		return;
 
-	if (app->curvalid || (td->mtclass.quirks & MT_QUIRK_ALWAYS_VALID)) {
+	if (app->curvalid || (app->quirks & MT_QUIRK_ALWAYS_VALID)) {
 		int active;
 		int slotnum = mt_compute_slot(td, app, input);
 		struct mt_slot *s = &app->curdata;
@@ -756,14 +757,14 @@ static void mt_complete_slot(struct mt_device *td, struct mt_application *app,
 		if (slotnum < 0 || slotnum >= td->maxcontacts)
 			return;
 
-		if ((td->mtclass.quirks & MT_QUIRK_IGNORE_DUPLICATES) && mt) {
+		if ((app->quirks & MT_QUIRK_IGNORE_DUPLICATES) && mt) {
 			struct input_mt_slot *slot = &mt->slots[slotnum];
 			if (input_mt_is_active(slot) &&
 			    input_mt_is_used(mt, slot))
 				return;
 		}
 
-		if (!(td->mtclass.quirks & MT_QUIRK_CONFIDENCE))
+		if (!(app->quirks & MT_QUIRK_CONFIDENCE))
 			s->confidence_state = true;
 		active = (s->touch_state || s->inrange_state) &&
 							s->confidence_state;
@@ -784,7 +785,7 @@ static void mt_complete_slot(struct mt_device *td, struct mt_application *app,
 			 * divided by two to match visual scale of touch
 			 * for devices with this quirk
 			 */
-			if (td->mtclass.quirks & MT_QUIRK_TOUCH_SIZE_SCALING) {
+			if (app->quirks & MT_QUIRK_TOUCH_SIZE_SCALING) {
 				major = major >> 1;
 				minor = minor >> 1;
 			}
@@ -815,7 +816,7 @@ static void mt_complete_slot(struct mt_device *td, struct mt_application *app,
 static void mt_sync_frame(struct mt_device *td, struct mt_application *app,
 			  struct input_dev *input)
 {
-	if (td->mtclass.quirks & MT_QUIRK_WIN8_PTP_BUTTONS)
+	if (app->quirks & MT_QUIRK_WIN8_PTP_BUTTONS)
 		input_event(input, EV_KEY, BTN_LEFT, app->left_button_state);
 
 	input_mt_sync_frame(input);
@@ -867,7 +868,7 @@ static void mt_process_mt_event(struct hid_device *hid, struct hid_field *field,
 				struct mt_application *app, bool first_packet)
 {
 	struct mt_device *td = hid_get_drvdata(hid);
-	__s32 quirks = td->mtclass.quirks;
+	__s32 quirks = app->quirks;
 	struct input_dev *input = field->hidinput->input;
 
 	if (hid->claimed & HID_CLAIMED_INPUT) {
@@ -1017,7 +1018,7 @@ static void mt_touch_report(struct hid_device *hid, struct hid_report *report)
 		 * of a possible multi-packet frame be checking that the
 		 * timestamp has changed.
 		 */
-		if ((td->mtclass.quirks & MT_QUIRK_WIN8_PTP_BUTTONS) &&
+		if ((app->quirks & MT_QUIRK_WIN8_PTP_BUTTONS) &&
 		    app->num_received == 0 &&
 		    app->prev_scantime != scantime)
 			app->num_expected = contact_count;
@@ -1061,7 +1062,7 @@ static void mt_touch_report(struct hid_device *hid, struct hid_report *report)
 	 * only affect laggish machines and the ones that have a firmware
 	 * defect.
 	 */
-	if (td->mtclass.quirks & MT_QUIRK_STICKY_FINGERS) {
+	if (app->quirks & MT_QUIRK_STICKY_FINGERS) {
 		if (test_bit(MT_IO_FLAGS_PENDING_SLOTS, &td->mt_io_flags))
 			mod_timer(&td->release_timer,
 				  jiffies + msecs_to_jiffies(100));
@@ -1091,7 +1092,7 @@ static int mt_touch_input_configured(struct hid_device *hdev,
 	if (cls->is_indirect)
 		app->mt_flags |= INPUT_MT_POINTER;
 
-	if (cls->quirks & MT_QUIRK_NOT_SEEN_MEANS_UP)
+	if (app->quirks & MT_QUIRK_NOT_SEEN_MEANS_UP)
 		app->mt_flags |= INPUT_MT_DROP_UNUSED;
 
 	/* check for clickpads */
@@ -1119,6 +1120,8 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 	struct mt_device *td = hid_get_drvdata(hdev);
 	struct mt_application *application;
 
+	application = mt_find_application(td, field->application);
+
 	/*
 	 * If mtclass.export_all_inputs is not set, only map fields from
 	 * TouchScreen or TouchPad collections. We need to ignore fields
@@ -1134,7 +1137,7 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 	    field->application != HID_CP_CONSUMER_CONTROL &&
 	    field->application != HID_GD_WIRELESS_RADIO_CTLS &&
 	    !(field->application == HID_VD_ASUS_CUSTOM_MEDIA_KEYS &&
-	      td->mtclass.quirks & MT_QUIRK_ASUS_CUSTOM_UP))
+	      application->quirks & MT_QUIRK_ASUS_CUSTOM_UP))
 		return -1;
 
 	/*
@@ -1143,7 +1146,7 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 	 * map usages to input keys.
 	 */
 	if (field->application == HID_VD_ASUS_CUSTOM_MEDIA_KEYS &&
-	    td->mtclass.quirks & MT_QUIRK_ASUS_CUSTOM_UP &&
+	    application->quirks & MT_QUIRK_ASUS_CUSTOM_UP &&
 	    (usage->hid & HID_USAGE_PAGE) == HID_UP_CUSTOM) {
 		set_bit(EV_REP, hi->input->evbit);
 		if (field->flags & HID_MAIN_ITEM_VARIABLE)
@@ -1160,8 +1163,6 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		return 1;
 	}
 
-	application = mt_find_application(td, field->application);
-
 	/*
 	 * some egalax touchscreens have "application == HID_DG_TOUCHSCREEN"
 	 * for the stylus.
@@ -1267,9 +1268,9 @@ static bool mt_need_to_apply_feature(struct hid_device *hdev,
 		return true;
 
 	case HID_DG_CONTACTMAX:
-		if (td->mtclass.maxcontacts) {
+		if (cls->maxcontacts) {
 			max = min_t(int, field->logical_maximum,
-				    td->mtclass.maxcontacts);
+				    cls->maxcontacts);
 			if (field->value[index] != max) {
 				field->value[index] = max;
 				return true;
@@ -1332,7 +1333,7 @@ static void mt_set_modes(struct hid_device *hdev, enum latency_mode latency,
 static void mt_post_parse_default_settings(struct mt_device *td,
 					   struct mt_application *app)
 {
-	__s32 quirks = td->mtclass.quirks;
+	__s32 quirks = app->quirks;
 
 	/* unknown serial device needs special quirks */
 	if (app->touches_by_report == 1) {
@@ -1343,13 +1344,12 @@ static void mt_post_parse_default_settings(struct mt_device *td,
 		quirks &= ~MT_QUIRK_CONTACT_CNT_ACCURATE;
 	}
 
-	td->mtclass.quirks = quirks;
+	app->quirks = quirks;
 }
 
 static void mt_post_parse(struct mt_device *td, struct mt_application *app)
 {
 	struct mt_fields *f = td->fields;
-	struct mt_class *cls = &td->mtclass;
 
 	if (app->touches_by_report > 0) {
 		int field_count_per_touch;
@@ -1359,7 +1359,7 @@ static void mt_post_parse(struct mt_device *td, struct mt_application *app)
 	}
 
 	if (app->cc_index < 0)
-		cls->quirks &= ~MT_QUIRK_CONTACT_CNT_ACCURATE;
+		app->quirks &= ~MT_QUIRK_CONTACT_CNT_ACCURATE;
 }
 
 static int mt_input_configured(struct hid_device *hdev, struct hid_input *hi)
-- 
2.14.3

^ permalink raw reply related


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