Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH v2 2/3] Input: ti_am335x_tsc - Ack pending IRQs at probe and before suspend
From: Vignesh R @ 2018-04-14  9:51 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Vignesh R, Grygorii Strashko, linux-input, linux-kernel,
	linux-omap, Tony Lindgren
In-Reply-To: <20180414095153.32060-1-vigneshr@ti.com>

From: Grygorii Strashko <grygorii.strashko@ti.com>

It is seen that just enabling the TSC module triggers a HW_PEN IRQ
without any interaction with touchscreen by user. This results in first
suspend/resume sequence to fail as system immediately wakes up from
suspend as soon as HW_PEN IRQ is enabled in suspend handler due to the
pending IRQ. Therefore clear all IRQs at probe and also in suspend
callback for sanity.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
Signed-off-by: Vignesh R <vigneshr@ti.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
---

v2: Add Acks from v1.

 drivers/input/touchscreen/ti_am335x_tsc.c | 2 ++
 include/linux/mfd/ti_am335x_tscadc.h      | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index 810e05c9c4f5..dcd9db768169 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -439,6 +439,7 @@ static int titsc_probe(struct platform_device *pdev)
 			dev_err(&pdev->dev, "irq wake enable failed.\n");
 	}
 
+	titsc_writel(ts_dev, REG_IRQSTATUS, IRQENB_MASK);
 	titsc_writel(ts_dev, REG_IRQENABLE, IRQENB_FIFO0THRES);
 	titsc_writel(ts_dev, REG_IRQENABLE, IRQENB_EOS);
 	err = titsc_config_wires(ts_dev);
@@ -504,6 +505,7 @@ static int __maybe_unused titsc_suspend(struct device *dev)
 
 	tscadc_dev = ti_tscadc_dev_get(to_platform_device(dev));
 	if (device_may_wakeup(tscadc_dev->dev)) {
+		titsc_writel(ts_dev, REG_IRQSTATUS, IRQENB_MASK);
 		idle = titsc_readl(ts_dev, REG_IRQENABLE);
 		titsc_writel(ts_dev, REG_IRQENABLE,
 				(idle | IRQENB_HW_PEN));
diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index b9a53e013bff..1a6a34f726cc 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -63,6 +63,7 @@
 #define IRQENB_FIFO1OVRRUN	BIT(6)
 #define IRQENB_FIFO1UNDRFLW	BIT(7)
 #define IRQENB_PENUP		BIT(9)
+#define IRQENB_MASK		(0x7FF)
 
 /* Step Configuration */
 #define STEPCONFIG_MODE_MASK	(3 << 0)
-- 
2.17.0

^ permalink raw reply related

* [PATCH v2 1/3] Input: ti_am335x_tsc - Mark IRQ as wakeup capable
From: Vignesh R @ 2018-04-14  9:51 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Vignesh R, Grygorii Strashko, linux-input, linux-kernel,
	linux-omap, Tony Lindgren
In-Reply-To: <20180414095153.32060-1-vigneshr@ti.com>

On AM335x, ti_am335x_tsc can wake up the system from suspend, mark the
IRQ as wakeup capable, so that device irq is not disabled during system
suspend.

Signed-off-by: Vignesh R <vigneshr@ti.com>
---

v2: No changes

 drivers/input/touchscreen/ti_am335x_tsc.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index f1043ae71dcc..810e05c9c4f5 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -27,6 +27,7 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/sort.h>
+#include <linux/pm_wakeirq.h>
 
 #include <linux/mfd/ti_am335x_tscadc.h>
 
@@ -432,6 +433,12 @@ static int titsc_probe(struct platform_device *pdev)
 		goto err_free_mem;
 	}
 
+	if (device_may_wakeup(tscadc_dev->dev)) {
+		err = dev_pm_set_wake_irq(tscadc_dev->dev, ts_dev->irq);
+		if (err)
+			dev_err(&pdev->dev, "irq wake enable failed.\n");
+	}
+
 	titsc_writel(ts_dev, REG_IRQENABLE, IRQENB_FIFO0THRES);
 	titsc_writel(ts_dev, REG_IRQENABLE, IRQENB_EOS);
 	err = titsc_config_wires(ts_dev);
@@ -462,6 +469,7 @@ static int titsc_probe(struct platform_device *pdev)
 	return 0;
 
 err_free_irq:
+	dev_pm_clear_wake_irq(tscadc_dev->dev);
 	free_irq(ts_dev->irq, ts_dev);
 err_free_mem:
 	input_free_device(input_dev);
@@ -474,6 +482,7 @@ static int titsc_remove(struct platform_device *pdev)
 	struct titsc *ts_dev = platform_get_drvdata(pdev);
 	u32 steps;
 
+	dev_pm_clear_wake_irq(ts_dev->mfd_tscadc->dev);
 	free_irq(ts_dev->irq, ts_dev);
 
 	/* total steps followed by the enable mask */
-- 
2.17.0

^ permalink raw reply related

* [PATCH v2 0/3] ti_am335x_tsc: Fix suspend/resume
From: Vignesh R @ 2018-04-14  9:51 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Vignesh R, Grygorii Strashko, linux-input, linux-kernel,
	linux-omap, Tony Lindgren

This patch series fixes couple of issues wrt suspend/resume with TI AM335x
TSC driver. Disable and clear any pending IRQs before suspend, and
handle case where TSC wakeup would fail, if there were touch events
during suspend.

v2:
Rebase onto latest linux-next.
v1:https://lkml.org/lkml/2016/5/16/150

Grygorii Strashko (2):
  Input: ti_am335x_tsc - Ack pending IRQs at probe and before suspend
  Input: ti_am335x_tsc - Prevent system suspend when TSC is in use

Vignesh R (1):
  Input: ti_am335x_tsc - Mark IRQ as wakeup capable

 drivers/input/touchscreen/ti_am335x_tsc.c | 14 ++++++++++++++
 include/linux/mfd/ti_am335x_tscadc.h      |  1 +
 2 files changed, 15 insertions(+)

-- 
2.17.0

^ permalink raw reply

* Re: [PATCH 2/2] input: touchscreen mc13xxx: Add mc34708 support
From: Dmitry Torokhov @ 2018-04-13 22:27 UTC (permalink / raw)
  To: Lukasz Majewski; +Cc: linux-kernel, linux-input, Sascha Hauer
In-Reply-To: <20180411141340.30939-2-lukma@denx.de>

On Wed, Apr 11, 2018 at 04:13:40PM +0200, Lukasz Majewski wrote:
> From: Sascha Hauer <s.hauer@pengutronix.de>
> 
> The mc34708 has a different bit to enable pen detection. This
> adds the driver data and devtype necessary to probe the device
> and to distinguish between the mc13783 and the mc34708.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
> 
> ---
> Changes from the original patch:
> - Simplify the mcXXXXX_set_pen_detection functions
> - Fix checkpatch warnings
> ---
>  drivers/input/touchscreen/mc13783_ts.c | 65 +++++++++++++++++++++++++++++++---
>  1 file changed, 61 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/mc13783_ts.c b/drivers/input/touchscreen/mc13783_ts.c
> index 610f10e6aadf..9fcc7069f633 100644
> --- a/drivers/input/touchscreen/mc13783_ts.c
> +++ b/drivers/input/touchscreen/mc13783_ts.c
> @@ -33,6 +33,8 @@ MODULE_PARM_DESC(sample_tolerance,
>  		"is supposed to be wrong and is discarded.  Set to 0 to "
>  		"disable this check.");
>  
> +struct mc13xxx_driver_data;
> +
>  struct mc13783_ts_priv {
>  	struct input_dev *idev;
>  	struct mc13xxx *mc13xxx;
> @@ -40,6 +42,43 @@ struct mc13783_ts_priv {
>  	unsigned int sample[4];
>  	u8 ato;
>  	bool atox;
> +	struct mc13xxx_driver_data *drvdata;
> +};
> +
> +enum mc13xxx_type {
> +	MC13XXX_TYPE_MC13783,
> +	MC13XXX_TYPE_MC34708,
> +};
> +
> +struct mc13xxx_driver_data {
> +	int (*set_pen_detection)(struct mc13783_ts_priv *priv, bool enable);
> +	enum mc13xxx_type type;
> +};
> +
> +static int mc13783_set_pen_detection(struct mc13783_ts_priv *priv, bool enable)
> +{
> +	return mc13xxx_reg_rmw(priv->mc13xxx, MC13XXX_ADC0,
> +			       MC13XXX_ADC0_TSMOD_MASK,
> +			       enable ? MC13XXX_ADC0_TSMOD0 : 0);
> +}
> +
> +#define MC34708_ADC0_TSPENDETEN		(1 << 20)
> +
> +static int mc34708_set_pen_detection(struct mc13783_ts_priv *priv, bool enable)
> +{
> +	return mc13xxx_reg_rmw(priv->mc13xxx, MC13XXX_ADC0,
> +			       MC34708_ADC0_TSPENDETEN,
> +			       enable ? MC34708_ADC0_TSPENDETEN : 0);
> +}

Instead of having separate functions, can we have register mask and
value in mc13xxx_driver_data structure?

> +
> +static struct mc13xxx_driver_data mc13783_driver_data = {
> +	.type = MC13XXX_TYPE_MC13783,
> +	.set_pen_detection = mc13783_set_pen_detection,
> +};
> +
> +static struct mc13xxx_driver_data mc34708_driver_data = {
> +	.type = MC13XXX_TYPE_MC34708,
> +	.set_pen_detection = mc34708_set_pen_detection,
>  };
>  
>  static irqreturn_t mc13783_ts_handler(int irq, void *data)
> @@ -96,6 +135,10 @@ static void mc13783_ts_report_sample(struct mc13783_ts_priv *priv)
>  
>  	cr0 = (cr0 + cr1) / 2;
>  
> +	if (priv->drvdata->type == MC13XXX_TYPE_MC34708)
> +		if (cr0 > 4080)
> +			cr0 = 0;

4080 is the max resistance for MC34708, right? I'd put it into drvdata
as well (and 4096 for the rest), and used

	input_report_abs(idev, ABS_PRESSURE,
			 cr0 ? priv->drvdata->max_resistance - cr0 : 0);

down below.

> +
>  	if (!cr0 || !sample_tolerance ||
>  			(x2 - x0 < sample_tolerance &&
>  			 y2 - y0 < sample_tolerance)) {
> @@ -148,8 +191,7 @@ static int mc13783_ts_open(struct input_dev *dev)
>  	if (ret)
>  		goto out;
>  
> -	ret = mc13xxx_reg_rmw(priv->mc13xxx, MC13XXX_ADC0,
> -			MC13XXX_ADC0_TSMOD_MASK, MC13XXX_ADC0_TSMOD0);
> +	ret = priv->drvdata->set_pen_detection(priv, 1);
>  	if (ret)
>  		mc13xxx_irq_free(priv->mc13xxx, MC13XXX_IRQ_TS, priv);
>  out:
> @@ -162,8 +204,7 @@ static void mc13783_ts_close(struct input_dev *dev)
>  	struct mc13783_ts_priv *priv = input_get_drvdata(dev);
>  
>  	mc13xxx_lock(priv->mc13xxx);
> -	mc13xxx_reg_rmw(priv->mc13xxx, MC13XXX_ADC0,
> -			MC13XXX_ADC0_TSMOD_MASK, 0);
> +	priv->drvdata->set_pen_detection(priv, 0);
>  	mc13xxx_irq_free(priv->mc13xxx, MC13XXX_IRQ_TS, priv);
>  	mc13xxx_unlock(priv->mc13xxx);
>  
> @@ -175,6 +216,7 @@ static int __init mc13783_ts_probe(struct platform_device *pdev)
>  	struct mc13783_ts_priv *priv;
>  	struct mc13xxx_ts_platform_data *pdata = dev_get_platdata(&pdev->dev);
>  	struct input_dev *idev;
> +	const struct platform_device_id *id = platform_get_device_id(pdev);
>  	int ret = -ENOMEM;
>  
>  	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> @@ -185,6 +227,7 @@ static int __init mc13783_ts_probe(struct platform_device *pdev)
>  	INIT_DELAYED_WORK(&priv->work, mc13783_ts_work);
>  	priv->mc13xxx = dev_get_drvdata(pdev->dev.parent);
>  	priv->idev = idev;
> +	priv->drvdata = (void *)id->driver_data;
>  
>  	if (pdata) {
>  		priv->atox = pdata->atox;
> @@ -231,7 +274,21 @@ static int mc13783_ts_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static const struct platform_device_id mc13xxx_ts_idtable[] = {
> +	{
> +		.name = "mc13783-ts",
> +		.driver_data = (kernel_ulong_t)&mc13783_driver_data,
> +	}, {
> +		.name = "mc34708-ts",
> +		.driver_data = (kernel_ulong_t)&mc34708_driver_data,
> +	}, {
> +		/* sentinel */
> +	}
> +};
> +MODULE_DEVICE_TABLE(platform, mc13xxx_ts_idtable);
> +
>  static struct platform_driver mc13783_ts_driver = {
> +	.id_table	= mc13xxx_ts_idtable,
>  	.remove		= mc13783_ts_remove,
>  	.driver		= {
>  		.name	= MC13783_TS_NAME,
> -- 
> 2.11.0
> 

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] Input: i8042 - Fix KBD port cannot wake up system from suspend-to-idle
From: Dmitry Torokhov @ 2018-04-13 21:57 UTC (permalink / raw)
  To: Kai-Heng Feng; +Cc: linux-input, linux-kernel
In-Reply-To: <20180411085905.22272-1-kai.heng.feng@canonical.com>

On Wed, Apr 11, 2018 at 04:59:05PM +0800, Kai-Heng Feng wrote:
> Commit f13b2065de81 ("Input: i8042 - allow KBD and AUX ports to wake up
> from suspend-to-idle") make system in s2idle can be woken up by i8042
> keyboard, but it's disabled by default.
> 
> In commit 3e6e15a862a0 ("Input: enable remote wakeup for PNP i8042
> keyboard ports") states that "Keyboard ports are always supposed to be
> wakeup-enabled", it should be enabled by default. Keyboard wakeup from
> s2idles is also the default behavior for other OSes.
> 
> But right now we can't wake up the system by keyboard, from s2idle.
> 
> In i8042_probe(), device_set_wakeup_enable(), which gets called by
> i8042_pnp_kbd_probe(), runs before device_set_wakeup_capable(), which
> gets called by i8042_register_ports(). So device_set_wakeup_enable()
> doesn't really enable wakeup for keyboard.

You are talking about 2 different devices here, one representing PNP and
another KBD serio port. Unfortunately there is not really a link between
the 2.

> 
> We can enable keyboard wakeup in i8042_register_ports() directly.

No, the world is not all x86, what makes sense for x86 does not
necessarily work for other architectures. We need to come up with a way
for tying PNP devices and i8042 ports for x86 case.

> 
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
>  drivers/input/serio/i8042-x86ia64io.h | 3 ---
>  drivers/input/serio/i8042.c           | 4 ++++
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h
> index b353d494ad40..e3def9195c2a 100644
> --- a/drivers/input/serio/i8042-x86ia64io.h
> +++ b/drivers/input/serio/i8042-x86ia64io.h
> @@ -925,9 +925,6 @@ static int i8042_pnp_kbd_probe(struct pnp_dev *dev, const struct pnp_device_id *
>  	i8042_pnp_id_to_string(dev->id, i8042_kbd_firmware_id,
>  			       sizeof(i8042_kbd_firmware_id));
>  
> -	/* Keyboard ports are always supposed to be wakeup-enabled */
> -	device_set_wakeup_enable(&dev->dev, true);
> -
>  	i8042_pnp_kbd_devices++;
>  	return 0;
>  }
> diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
> index 824f4c1c1f31..21a16b757931 100644
> --- a/drivers/input/serio/i8042.c
> +++ b/drivers/input/serio/i8042.c
> @@ -1400,6 +1400,10 @@ static void __init i8042_register_ports(void)
>  				i8042_ports[i].irq);
>  			serio_register_port(serio);
>  			device_set_wakeup_capable(&serio->dev, true);
> +
> +			/* Keyboard ports are always supposed to be wakeup-enabled */
> +			if (i == I8042_KBD_PORT_NO)
> +				device_wakeup_enable(&serio->dev);
>  		}
>  	}
>  }
> -- 
> 2.17.0
> 

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] Input: atmel_mxt_ts - add missing compatible strings to OF device table
From: Rob Herring @ 2018-04-13 21:11 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: linux-kernel, devicetree, Alexandre Belloni, Dmitry Torokhov,
	linux-input, Nicolas Ferre, Nick Dyer, Mark Rutland,
	linux-arm-kernel
In-Reply-To: <20180410095340.24074-1-javierm@redhat.com>

On Tue, Apr 10, 2018 at 11:53:40AM +0200, Javier Martinez Canillas wrote:
> Commit af503716ac14 ("i2c: core: report OF style module alias for devices
> registered via OF") fixed how the I2C core reports the module alias when
> devices are registered via OF.
> 
> But the atmel_mxt_ts driver only has an "atmel,maxtouch" compatible in its
> OF device ID table, so if a Device Tree is using a different one, autoload
> won't be working for the module (the matching works because the I2C device
> ID table is used as a fallback).
> 
> So add compatible strings for each of the entries in the I2C device table.
> 
> Fixes: af503716ac14 ("i2c: core: report OF style module alias for devices registered via OF")
> Reported-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
> 
>  Documentation/devicetree/bindings/input/atmel,maxtouch.txt | 6 +++++-
>  drivers/input/touchscreen/atmel_mxt_ts.c                   | 4 ++++
>  2 files changed, 9 insertions(+), 1 deletion(-)

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH v3 05/11] dt-bindings: input: touchscreen: resistive-adc-touch: create bindings
From: Rob Herring @ 2018-04-13 18:34 UTC (permalink / raw)
  To: Eugen Hristev
  Cc: jic23, ludovic.desroches, alexandre.belloni, linux-arm-kernel,
	devicetree, linux-kernel, linux-iio, linux-input, nicolas.ferre,
	dmitry.torokhov
In-Reply-To: <1523350677-27106-6-git-send-email-eugen.hristev@microchip.com>

On Tue, Apr 10, 2018 at 11:57:51AM +0300, Eugen Hristev wrote:
> Added bindings for generic resistive touchscreen ADC.
> 
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
> ---
> Changes in v3:
>  - renamed file and compatible to exclude "generic" keyword
>  - removed the pressure threshold property, added it as a common
> touchscreen property in the touchscreen common bindings in a separate
> commit.
> 
> Changes in v2:
>  - modified bindings to have a generic resistive touchscreen adc driver
> instead of specific architecture one.
> 
>  .../input/touchscreen/resistive-adc-touch.txt      | 30 ++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/resistive-adc-touch.txt

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH v3 04/11] dt-bindings: input: touchscreen: add pressure threshold touchscreen property
From: Rob Herring @ 2018-04-13 18:33 UTC (permalink / raw)
  To: Eugen Hristev
  Cc: jic23, ludovic.desroches, alexandre.belloni, linux-arm-kernel,
	devicetree, linux-kernel, linux-iio, linux-input, nicolas.ferre,
	dmitry.torokhov
In-Reply-To: <1523350677-27106-5-git-send-email-eugen.hristev@microchip.com>

On Tue, Apr 10, 2018 at 11:57:50AM +0300, Eugen Hristev wrote:
> Add a common touchscreen optional property that will specify
> the minimum pressure applied to the screen that is needed
> such that the driver will report the touch event.
> 
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
> ---
>  Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt | 3 +++
>  1 file changed, 3 insertions(+)

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH v2 3/9] Input: elan_i2c - add trackstick report
From: Rob Herring @ 2018-04-13 17:36 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Dmitry Torokhov, kt.liao, Oliver Haessler, Benjamin Berg,
	devicetree, linux-input, linux-kernel
In-Reply-To: <20180409091051.2944-4-benjamin.tissoires@redhat.com>

On Mon, Apr 09, 2018 at 11:10:45AM +0200, Benjamin Tissoires wrote:
> The Elan touchpads over I2C/SMBus also can handle a
> trackstick. Unfortunately, nothing tells us if the
> device supports trackstick (the information lies in
> the PS/2 node), so rely on a platform data to enable
> or not the trackstick node.
> 
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=1313939
> 
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> 
> ---
> 
> changes in v2:
> - use of generic device property instead of platform data so
>   device tree can also make use of it
> ---
>  .../devicetree/bindings/input/elan_i2c.txt         |  1 +

Acked-by: Rob Herring <robh@kernel.org>

>  drivers/input/mouse/elan_i2c_core.c                | 90 +++++++++++++++++++++-
>  2 files changed, 88 insertions(+), 3 deletions(-)

^ permalink raw reply

* Re: [PATCH 2/2] input: touchscreen mc13xxx: Add mc34708 support
From: Lukasz Majewski @ 2018-04-12 14:45 UTC (permalink / raw)
  To: Joe Perches; +Cc: Dmitry Torokhov, linux-kernel, linux-input, Sascha Hauer
In-Reply-To: <4690e30b3c4b97847123295f1994c6eafe4e0cbb.camel@perches.com>

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

On Wed, 11 Apr 2018 09:01:01 -0700
Joe Perches <joe@perches.com> wrote:

> On Wed, 2018-04-11 at 16:13 +0200, Lukasz Majewski wrote:
> > From: Sascha Hauer <s.hauer@pengutronix.de>
> > 
> > The mc34708 has a different bit to enable pen detection. This
> > adds the driver data and devtype necessary to probe the device
> > and to distinguish between the mc13783 and the mc34708.  
> 
> style trivia:
> 
> > diff --git a/drivers/input/touchscreen/mc13783_ts.c
> > b/drivers/input/touchscreen/mc13783_ts.c  
> []
> > @@ -96,6 +135,10 @@ static void mc13783_ts_report_sample(struct
> > mc13783_ts_priv *priv) 
> >  	cr0 = (cr0 + cr1) / 2;
> >  
> > +	if (priv->drvdata->type == MC13XXX_TYPE_MC34708)
> > +		if (cr0 > 4080)
> > +			cr0 = 0;  
> 
> For easy of reading, this multiple if block should either
> use braces around the first if like:
> 
> 	if (foo) {
> 		if (bar)
> 			single_statement;
> 	}
> 
> or be written
> 
> 	if (foo && bar)
> 		single_statement;
> 
> I generally prefer the latter style.
> 

+1.

I will fix it in v2.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH 0/4] HID: alps: Fix some bugs and improve code around 't4_read_write_register()'
From: Jiri Kosina @ 2018-04-12 13:11 UTC (permalink / raw)
  To: Christophe JAILLET
  Cc: masaki.ota, benjamin.tissoires, linux-input, linux-kernel,
	kernel-janitors
In-Reply-To: <nycvar.YFH.7.76.1803271405420.15778@cbobk.fhfr.pm>

On Tue, 27 Mar 2018, Jiri Kosina wrote:

> > These patches are untested. Especially, patch 1 slightly changes the behavior
> > of 't4_read_write_register()'.
> > This looks logical to me, but please, review it carefully.
> > 
> > Christophe JAILLET (4):
> >   HID: alps: Report an error if we receive invalid data in
> >     't4_read_write_register()'
> >   HID: alps: Save a memory allocation in 't4_read_write_register()' when
> >     writing data
> >   HID: alps: Check errors returned by 't4_read_write_register()'
> >   HID: alps: Fix some style in 't4_read_write_register()'
> > 
> >  drivers/hid/hid-alps.c | 27 ++++++++++++++++++++++-----
> >  1 file changed, 22 insertions(+), 5 deletions(-)
> 
> Masaki-san,
> 
> do you have any comments to Christophe's patchset please?

If there is no feedback, I'll queue the set for 4.18.

Thanks,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH] HID: i2c-hid: Fix resume issue on Raydium touchscreen device
From: Jiri Kosina @ 2018-04-12 13:06 UTC (permalink / raw)
  To: Aaron Ma; +Cc: benjamin.tissoires, linux-input, linux-kernel
In-Reply-To: <1523259691-7672-1-git-send-email-aaron.ma@canonical.com>

On Mon, 9 Apr 2018, Aaron Ma wrote:

> When Rayd touchscreen resumed from S3, it issues too many errors like:
> i2c_hid i2c-RAYD0001:00: i2c_hid_get_input: incomplete report (58/5442)
> 
> And all the report data are corrupted, touchscreen is unresponsive.
> 
> Fix this by re-sending report description command after resume.
> Add device ID as a quirk.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH] HID: wacom: bluetooth: send exit report for recent Bluetooth devices
From: Jiri Kosina @ 2018-04-12 12:05 UTC (permalink / raw)
  To: Aaron Armstrong Skomra
  Cc: linux-input, pinglinux, killertofu, # 4 . 11,
	Aaron Armstrong Skomra
In-Reply-To: <1522877051-3943-1-git-send-email-skomra@gmail.com>

On Wed, 4 Apr 2018, Aaron Armstrong Skomra wrote:

> The code path for recent Bluetooth devices omits an exit report which
> resets all the values of the device.
> 
> Fixes: 4922cd26f0 ("HID: wacom: Support 2nd-gen Intuos Pro's Bluetooth classic interface")
> Cc: <stable@vger.kernel.org> # 4.11
> Signed-off-by: Aaron Armstrong Skomra <aaron.skomra@wacom.com>
> Reviewed-by: Ping Cheng <ping.cheng@wacom.com>

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH v6 23/30] drm/rockchip: analogix_dp: Do not call Analogix code before bind
From: Andrzej Hajda @ 2018-04-12  9:54 UTC (permalink / raw)
  To: Enric Balletbo i Serra, architt, inki.dae, thierry.reding, hjc,
	seanpaul, airlied, tfiga, heiko
  Cc: dri-devel, dianders, Laurent.pinchart, ykk, kernel, m.szyprowski,
	linux-samsung-soc, rydberg, krzk, linux-rockchip, kgene,
	linux-input, orjan.eide, wxt, jeffy.chen, linux-arm-kernel,
	mark.yao, wzz, hl, jingoohan1, sw0312.kim, linux-kernel,
	kyungmin.park, kuankuan.y, hshi
In-Reply-To: <20180405095000.9756-24-enric.balletbo@collabora.com>

On 05.04.2018 11:49, Enric Balletbo i Serra wrote:
> From: Tomasz Figa <tfiga@chromium.org>
>
> Driver callbacks, such as system suspend or resume can be called any
> time, specifically they can be called before the component bind
> callback. Let's use dp->adp pointer as a safeguard and skip calling
> Analogix entry points if it is an ERR_PTR().

From purity PoV I would store either 0 either valid pointer in dp->adp,
but functionally it should be the same.

Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>

 --
Regards
Andrzej


>
> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>
>  drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> index 23317a2269e1..6d45d62466b3 100644
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> @@ -368,6 +368,8 @@ static void rockchip_dp_unbind(struct device *dev, struct device *master,
>  	analogix_dp_unbind(dp->adp);
>  	rockchip_drm_psr_unregister(&dp->encoder);
>  	dp->encoder.funcs->destroy(&dp->encoder);
> +
> +	dp->adp = ERR_PTR(-ENODEV);
>  }
>  
>  static const struct component_ops rockchip_dp_component_ops = {
> @@ -391,6 +393,7 @@ static int rockchip_dp_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	dp->dev = dev;
> +	dp->adp = ERR_PTR(-ENODEV);
>  	dp->plat_data.panel = panel;
>  
>  	ret = rockchip_dp_of_probe(dp);
> @@ -414,6 +417,9 @@ static int rockchip_dp_suspend(struct device *dev)
>  {
>  	struct rockchip_dp_device *dp = dev_get_drvdata(dev);
>  
> +	if (IS_ERR(dp->adp))
> +		return 0;
> +
>  	return analogix_dp_suspend(dp->adp);
>  }
>  
> @@ -421,6 +427,9 @@ static int rockchip_dp_resume(struct device *dev)
>  {
>  	struct rockchip_dp_device *dp = dev_get_drvdata(dev);
>  
> +	if (IS_ERR(dp->adp))
> +		return 0;
> +
>  	return analogix_dp_resume(dp->adp);
>  }
>  #endif


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH] Input: leds - fix out of bound access
From: Peter Hutterer @ 2018-04-12  6:20 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-input, linux-kernel, Tasos Sahanidis, Samuel Thibault
In-Reply-To: <20180406181242.GA225849@dtor-ws>

On Fri, Apr 06, 2018 at 11:12:42AM -0700, Dmitry Torokhov wrote:
> UI_SET_LEDBIT ioctl() causes the following KASAN splat when used with
> led > LED_CHARGING:
> 
> [ 1274.663418] BUG: KASAN: slab-out-of-bounds in input_leds_connect+0x611/0x730 [input_leds]
> [ 1274.663426] Write of size 8 at addr ffff88003377b2c0 by task ckb-next-daemon/5128
> 
> This happens because we were writing to the led structure before making
> sure that it exists.
> 
> Reported-by: Tasos Sahanidis <tasos@tasossah.com>
> Tested-by: Tasos Sahanidis <tasos@tasossah.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>

Cheers,
   Peter

> ---
>  drivers/input/input-leds.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/input/input-leds.c b/drivers/input/input-leds.c
> index 766bf26601163..5f04b2d946350 100644
> --- a/drivers/input/input-leds.c
> +++ b/drivers/input/input-leds.c
> @@ -88,6 +88,7 @@ static int input_leds_connect(struct input_handler *handler,
>  			      const struct input_device_id *id)
>  {
>  	struct input_leds *leds;
> +	struct input_led *led;
>  	unsigned int num_leds;
>  	unsigned int led_code;
>  	int led_no;
> @@ -119,14 +120,13 @@ static int input_leds_connect(struct input_handler *handler,
>  
>  	led_no = 0;
>  	for_each_set_bit(led_code, dev->ledbit, LED_CNT) {
> -		struct input_led *led = &leds->leds[led_no];
> +		if (!input_led_info[led_code].name)
> +			continue;
>  
> +		led = &leds->leds[led_no];
>  		led->handle = &leds->handle;
>  		led->code = led_code;
>  
> -		if (!input_led_info[led_code].name)
> -			continue;
> -
>  		led->cdev.name = kasprintf(GFP_KERNEL, "%s::%s",
>  					   dev_name(&dev->dev),
>  					   input_led_info[led_code].name);
> -- 
> 2.17.0.484.g0c8726318c-goog

^ permalink raw reply

* Re: [PATCH 2/2] input: touchscreen mc13xxx: Add mc34708 support
From: Joe Perches @ 2018-04-11 16:01 UTC (permalink / raw)
  To: Lukasz Majewski, Dmitry Torokhov; +Cc: linux-kernel, linux-input, Sascha Hauer
In-Reply-To: <20180411141340.30939-2-lukma@denx.de>

On Wed, 2018-04-11 at 16:13 +0200, Lukasz Majewski wrote:
> From: Sascha Hauer <s.hauer@pengutronix.de>
> 
> The mc34708 has a different bit to enable pen detection. This
> adds the driver data and devtype necessary to probe the device
> and to distinguish between the mc13783 and the mc34708.

style trivia:

> diff --git a/drivers/input/touchscreen/mc13783_ts.c b/drivers/input/touchscreen/mc13783_ts.c
[]
> @@ -96,6 +135,10 @@ static void mc13783_ts_report_sample(struct mc13783_ts_priv *priv)
>  
>  	cr0 = (cr0 + cr1) / 2;
>  
> +	if (priv->drvdata->type == MC13XXX_TYPE_MC34708)
> +		if (cr0 > 4080)
> +			cr0 = 0;

For easy of reading, this multiple if block should either
use braces around the first if like:

	if (foo) {
		if (bar)
			single_statement;
	}

or be written

	if (foo && bar)
		single_statement;

I generally prefer the latter style.

^ permalink raw reply

* [PATCH 2/2] input: touchscreen mc13xxx: Add mc34708 support
From: Lukasz Majewski @ 2018-04-11 14:13 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-kernel, linux-input, Sascha Hauer, Lukasz Majewski
In-Reply-To: <20180411141340.30939-1-lukma@denx.de>

From: Sascha Hauer <s.hauer@pengutronix.de>

The mc34708 has a different bit to enable pen detection. This
adds the driver data and devtype necessary to probe the device
and to distinguish between the mc13783 and the mc34708.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Lukasz Majewski <lukma@denx.de>

---
Changes from the original patch:
- Simplify the mcXXXXX_set_pen_detection functions
- Fix checkpatch warnings
---
 drivers/input/touchscreen/mc13783_ts.c | 65 +++++++++++++++++++++++++++++++---
 1 file changed, 61 insertions(+), 4 deletions(-)

diff --git a/drivers/input/touchscreen/mc13783_ts.c b/drivers/input/touchscreen/mc13783_ts.c
index 610f10e6aadf..9fcc7069f633 100644
--- a/drivers/input/touchscreen/mc13783_ts.c
+++ b/drivers/input/touchscreen/mc13783_ts.c
@@ -33,6 +33,8 @@ MODULE_PARM_DESC(sample_tolerance,
 		"is supposed to be wrong and is discarded.  Set to 0 to "
 		"disable this check.");
 
+struct mc13xxx_driver_data;
+
 struct mc13783_ts_priv {
 	struct input_dev *idev;
 	struct mc13xxx *mc13xxx;
@@ -40,6 +42,43 @@ struct mc13783_ts_priv {
 	unsigned int sample[4];
 	u8 ato;
 	bool atox;
+	struct mc13xxx_driver_data *drvdata;
+};
+
+enum mc13xxx_type {
+	MC13XXX_TYPE_MC13783,
+	MC13XXX_TYPE_MC34708,
+};
+
+struct mc13xxx_driver_data {
+	int (*set_pen_detection)(struct mc13783_ts_priv *priv, bool enable);
+	enum mc13xxx_type type;
+};
+
+static int mc13783_set_pen_detection(struct mc13783_ts_priv *priv, bool enable)
+{
+	return mc13xxx_reg_rmw(priv->mc13xxx, MC13XXX_ADC0,
+			       MC13XXX_ADC0_TSMOD_MASK,
+			       enable ? MC13XXX_ADC0_TSMOD0 : 0);
+}
+
+#define MC34708_ADC0_TSPENDETEN		(1 << 20)
+
+static int mc34708_set_pen_detection(struct mc13783_ts_priv *priv, bool enable)
+{
+	return mc13xxx_reg_rmw(priv->mc13xxx, MC13XXX_ADC0,
+			       MC34708_ADC0_TSPENDETEN,
+			       enable ? MC34708_ADC0_TSPENDETEN : 0);
+}
+
+static struct mc13xxx_driver_data mc13783_driver_data = {
+	.type = MC13XXX_TYPE_MC13783,
+	.set_pen_detection = mc13783_set_pen_detection,
+};
+
+static struct mc13xxx_driver_data mc34708_driver_data = {
+	.type = MC13XXX_TYPE_MC34708,
+	.set_pen_detection = mc34708_set_pen_detection,
 };
 
 static irqreturn_t mc13783_ts_handler(int irq, void *data)
@@ -96,6 +135,10 @@ static void mc13783_ts_report_sample(struct mc13783_ts_priv *priv)
 
 	cr0 = (cr0 + cr1) / 2;
 
+	if (priv->drvdata->type == MC13XXX_TYPE_MC34708)
+		if (cr0 > 4080)
+			cr0 = 0;
+
 	if (!cr0 || !sample_tolerance ||
 			(x2 - x0 < sample_tolerance &&
 			 y2 - y0 < sample_tolerance)) {
@@ -148,8 +191,7 @@ static int mc13783_ts_open(struct input_dev *dev)
 	if (ret)
 		goto out;
 
-	ret = mc13xxx_reg_rmw(priv->mc13xxx, MC13XXX_ADC0,
-			MC13XXX_ADC0_TSMOD_MASK, MC13XXX_ADC0_TSMOD0);
+	ret = priv->drvdata->set_pen_detection(priv, 1);
 	if (ret)
 		mc13xxx_irq_free(priv->mc13xxx, MC13XXX_IRQ_TS, priv);
 out:
@@ -162,8 +204,7 @@ static void mc13783_ts_close(struct input_dev *dev)
 	struct mc13783_ts_priv *priv = input_get_drvdata(dev);
 
 	mc13xxx_lock(priv->mc13xxx);
-	mc13xxx_reg_rmw(priv->mc13xxx, MC13XXX_ADC0,
-			MC13XXX_ADC0_TSMOD_MASK, 0);
+	priv->drvdata->set_pen_detection(priv, 0);
 	mc13xxx_irq_free(priv->mc13xxx, MC13XXX_IRQ_TS, priv);
 	mc13xxx_unlock(priv->mc13xxx);
 
@@ -175,6 +216,7 @@ static int __init mc13783_ts_probe(struct platform_device *pdev)
 	struct mc13783_ts_priv *priv;
 	struct mc13xxx_ts_platform_data *pdata = dev_get_platdata(&pdev->dev);
 	struct input_dev *idev;
+	const struct platform_device_id *id = platform_get_device_id(pdev);
 	int ret = -ENOMEM;
 
 	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
@@ -185,6 +227,7 @@ static int __init mc13783_ts_probe(struct platform_device *pdev)
 	INIT_DELAYED_WORK(&priv->work, mc13783_ts_work);
 	priv->mc13xxx = dev_get_drvdata(pdev->dev.parent);
 	priv->idev = idev;
+	priv->drvdata = (void *)id->driver_data;
 
 	if (pdata) {
 		priv->atox = pdata->atox;
@@ -231,7 +274,21 @@ static int mc13783_ts_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct platform_device_id mc13xxx_ts_idtable[] = {
+	{
+		.name = "mc13783-ts",
+		.driver_data = (kernel_ulong_t)&mc13783_driver_data,
+	}, {
+		.name = "mc34708-ts",
+		.driver_data = (kernel_ulong_t)&mc34708_driver_data,
+	}, {
+		/* sentinel */
+	}
+};
+MODULE_DEVICE_TABLE(platform, mc13xxx_ts_idtable);
+
 static struct platform_driver mc13783_ts_driver = {
+	.id_table	= mc13xxx_ts_idtable,
 	.remove		= mc13783_ts_remove,
 	.driver		= {
 		.name	= MC13783_TS_NAME,
-- 
2.11.0

^ permalink raw reply related

* [PATCH 1/2] input: touchscreen mc13xxx: Make platform data optional
From: Lukasz Majewski @ 2018-04-11 14:13 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-kernel, linux-input, Sascha Hauer, Lukasz Majewski

From: Sascha Hauer <s.hauer@pengutronix.de>

The platform data once was optional, make it optional again. This
is a first step towards device tree support for the mc13xxx touchscreen
driver.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Lukasz Majewski <lukma@denx.de>
---
Changes from the original patch:
- Commit message typo fixes
---
 drivers/input/touchscreen/mc13783_ts.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/input/touchscreen/mc13783_ts.c b/drivers/input/touchscreen/mc13783_ts.c
index ef64f36c5ffc..610f10e6aadf 100644
--- a/drivers/input/touchscreen/mc13783_ts.c
+++ b/drivers/input/touchscreen/mc13783_ts.c
@@ -38,7 +38,8 @@ struct mc13783_ts_priv {
 	struct mc13xxx *mc13xxx;
 	struct delayed_work work;
 	unsigned int sample[4];
-	struct mc13xxx_ts_platform_data *touch;
+	u8 ato;
+	bool atox;
 };
 
 static irqreturn_t mc13783_ts_handler(int irq, void *data)
@@ -128,7 +129,7 @@ static void mc13783_ts_work(struct work_struct *work)
 
 	if (mc13xxx_adc_do_conversion(priv->mc13xxx,
 				mode, channel,
-				priv->touch->ato, priv->touch->atox,
+				priv->ato, priv->atox,
 				priv->sample) == 0)
 		mc13783_ts_report_sample(priv);
 }
@@ -172,6 +173,7 @@ static void mc13783_ts_close(struct input_dev *dev)
 static int __init mc13783_ts_probe(struct platform_device *pdev)
 {
 	struct mc13783_ts_priv *priv;
+	struct mc13xxx_ts_platform_data *pdata = dev_get_platdata(&pdev->dev);
 	struct input_dev *idev;
 	int ret = -ENOMEM;
 
@@ -183,11 +185,10 @@ static int __init mc13783_ts_probe(struct platform_device *pdev)
 	INIT_DELAYED_WORK(&priv->work, mc13783_ts_work);
 	priv->mc13xxx = dev_get_drvdata(pdev->dev.parent);
 	priv->idev = idev;
-	priv->touch = dev_get_platdata(&pdev->dev);
-	if (!priv->touch) {
-		dev_err(&pdev->dev, "missing platform data\n");
-		ret = -ENODEV;
-		goto err_free_mem;
+
+	if (pdata) {
+		priv->atox = pdata->atox;
+		priv->ato = pdata->ato;
 	}
 
 	idev->name = MC13783_TS_NAME;
-- 
2.11.0

^ permalink raw reply related

* [PATCH v5] Fix modifier keys for Redragon Asura Keyboard
From: Robert Munteanu @ 2018-04-11  9:49 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires
  Cc: linux-kernel, linux-input, Robert Munteanu

Changelog:

- v2: modifier keys work, some combinations are still troublesome
- v3: style cleanup, rebase on top of 4.14
- v4: remove most debugging calls, make init info useful for user,
  rebased on top of 4.15
- v5: fix the HID descriptor as suggested by Benjamin Tissoires,
  use existing USB vendor id, update comment style, add SPDX
  license identifier, rename to hid-redragon, stop registering
  two input devices, rebased on top of 4.16

Signed-off-by: Robert Munteanu <rombert@apache.org>
---
 drivers/hid/Kconfig        |  7 ++++
 drivers/hid/Makefile       |  1 +
 drivers/hid/hid-ids.h      |  1 +
 drivers/hid/hid-quirks.c   |  3 ++
 drivers/hid/hid-redragon.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 101 insertions(+)
 create mode 100644 drivers/hid/hid-redragon.c

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 19c499f5623d..1125e4813716 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -560,6 +560,13 @@ config HID_MAYFLASH
 	Say Y here if you have HJZ Mayflash PS3 game controller adapters
 	and want to enable force feedback support.
 
+config HID_REDRAGON
+	tristate "Redragon keyboards"
+	depends on HID
+	default !EXPERT
+	---help---
+    Support for Redragon keyboards that need fix-ups to work properly.
+
 config HID_MICROSOFT
 	tristate "Microsoft non-fully HID-compliant devices"
 	depends on HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index eb13b9e92d85..a36f3f40ba63 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -84,6 +84,7 @@ hid-picolcd-$(CONFIG_DEBUG_FS)		+= hid-picolcd_debugfs.o
 
 obj-$(CONFIG_HID_PLANTRONICS)	+= hid-plantronics.o
 obj-$(CONFIG_HID_PRIMAX)	+= hid-primax.o
+obj-$(CONFIG_HID_REDRAGON)	+= hid-redragon.o
 obj-$(CONFIG_HID_RETRODE)	+= hid-retrode.o
 obj-$(CONFIG_HID_ROCCAT)	+= hid-roccat.o hid-roccat-common.o \
 	hid-roccat-arvo.o hid-roccat-isku.o hid-roccat-kone.o \
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 9454ac134ce2..41a64d0e91f9 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -599,6 +599,7 @@
 #define USB_VENDOR_ID_JESS		0x0c45
 #define USB_DEVICE_ID_JESS_YUREX	0x1010
 #define USB_DEVICE_ID_ASUS_MD_5112	0x5112
+#define USB_DEVICE_ID_REDRAGON_ASURA	0x760b
 
 #define USB_VENDOR_ID_JESS2		0x0f30
 #define USB_DEVICE_ID_JESS2_COLOR_RUMBLE_PAD 0x0111
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index e92b77fa574a..5f1253f1739a 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -557,6 +557,9 @@ static const struct hid_device_id hid_have_special_driver[] = {
 #if IS_ENABLED(CONFIG_HID_PRODIKEYS)
 	{ HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS, USB_DEVICE_ID_PRODIKEYS_PCMIDI) },
 #endif
+#if IS_ENABLED(CONFIG_HID_REDRAGON)
+	{ HID_USB_DEVICE(USB_VENDOR_ID_JESS,  USB_DEVICE_ID_REDRAGON_ASURA) },
+#endif
 #if IS_ENABLED(CONFIG_HID_RETRODE)
 	{ HID_USB_DEVICE(USB_VENDOR_ID_FUTURE_TECHNOLOGY, USB_DEVICE_ID_RETRODE2) },
 #endif
diff --git a/drivers/hid/hid-redragon.c b/drivers/hid/hid-redragon.c
new file mode 100644
index 000000000000..ff98a5dbb8e2
--- /dev/null
+++ b/drivers/hid/hid-redragon.c
@@ -0,0 +1,89 @@
+/*
+ *  HID driver for Redragon keyboards
+ *
+ *  Copyright (c) 2017 Robert Munteanu
+ *  SPDX-License-Identifier: GPL-2.0
+ */
+
+/*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ */
+
+#include <linux/device.h>
+#include <linux/input.h>
+#include <linux/hid.h>
+#include <linux/module.h>
+#include <linux/log2.h>
+#include <linux/input-event-codes.h>
+
+#include "hid-ids.h"
+
+
+/*
+ * The Redragon Asura keyboard sends an incorrect HID descriptor.
+ * At byte 100 it contains
+ *
+ *   0x81, 0x00
+ *
+ * which is Input (Data, Arr, Abs), but it should be
+ *
+ *   0x81, 0x02
+ *
+ * which is Input (Data, Var, Abs), which is consistent with the way
+ * key codes are generated.
+ */
+
+static __u8 *redragon_report_fixup(struct hid_device *hdev, __u8 *rdesc,
+	unsigned int *rsize)
+{
+	if (*rsize >= 102 && rdesc[100] == 0x81 && rdesc[101] == 0x00) {
+		dev_info(&hdev->dev, "Fixing Redragon ASURA report descriptor.\n");
+		rdesc[101] = 0x02;
+	}
+
+	return rdesc;
+}
+
+static int redragon_probe(struct hid_device *dev,
+	const struct hid_device_id *id)
+{
+	int ret;
+
+	ret = hid_parse(dev);
+	if (ret) {
+		hid_err(dev, "parse failed\n");
+		return ret;
+	}
+
+	/* do not register unused input device */
+	if (dev->maxapplication == 1)
+		return 0;
+
+	ret = hid_hw_start(dev, HID_CONNECT_DEFAULT);
+	if (ret) {
+		hid_err(dev, "hw start failed\n");
+		return ret;
+	}
+
+	return 0;
+}
+static const struct hid_device_id redragon_devices[] = {
+	{HID_USB_DEVICE(USB_VENDOR_ID_JESS, USB_DEVICE_ID_REDRAGON_ASURA)},
+	{}
+};
+
+MODULE_DEVICE_TABLE(hid, redragon_devices);
+
+static struct hid_driver redragon_driver = {
+	.name = "redragon",
+	.id_table = redragon_devices,
+	.report_fixup = redragon_report_fixup,
+	.probe = redragon_probe
+};
+
+module_hid_driver(redragon_driver);
+
+MODULE_LICENSE("GPL");
-- 
2.16.3

^ permalink raw reply related

* [PATCH] Input: i8042 - Fix KBD port cannot wake up system from suspend-to-idle
From: Kai-Heng Feng @ 2018-04-11  8:59 UTC (permalink / raw)
  To: dmitry.torokhov; +Cc: linux-input, linux-kernel, Kai-Heng Feng

Commit f13b2065de81 ("Input: i8042 - allow KBD and AUX ports to wake up
from suspend-to-idle") make system in s2idle can be woken up by i8042
keyboard, but it's disabled by default.

In commit 3e6e15a862a0 ("Input: enable remote wakeup for PNP i8042
keyboard ports") states that "Keyboard ports are always supposed to be
wakeup-enabled", it should be enabled by default. Keyboard wakeup from
s2idles is also the default behavior for other OSes.

But right now we can't wake up the system by keyboard, from s2idle.

In i8042_probe(), device_set_wakeup_enable(), which gets called by
i8042_pnp_kbd_probe(), runs before device_set_wakeup_capable(), which
gets called by i8042_register_ports(). So device_set_wakeup_enable()
doesn't really enable wakeup for keyboard.

We can enable keyboard wakeup in i8042_register_ports() directly.

Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
 drivers/input/serio/i8042-x86ia64io.h | 3 ---
 drivers/input/serio/i8042.c           | 4 ++++
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h
index b353d494ad40..e3def9195c2a 100644
--- a/drivers/input/serio/i8042-x86ia64io.h
+++ b/drivers/input/serio/i8042-x86ia64io.h
@@ -925,9 +925,6 @@ static int i8042_pnp_kbd_probe(struct pnp_dev *dev, const struct pnp_device_id *
 	i8042_pnp_id_to_string(dev->id, i8042_kbd_firmware_id,
 			       sizeof(i8042_kbd_firmware_id));
 
-	/* Keyboard ports are always supposed to be wakeup-enabled */
-	device_set_wakeup_enable(&dev->dev, true);
-
 	i8042_pnp_kbd_devices++;
 	return 0;
 }
diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index 824f4c1c1f31..21a16b757931 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -1400,6 +1400,10 @@ static void __init i8042_register_ports(void)
 				i8042_ports[i].irq);
 			serio_register_port(serio);
 			device_set_wakeup_capable(&serio->dev, true);
+
+			/* Keyboard ports are always supposed to be wakeup-enabled */
+			if (i == I8042_KBD_PORT_NO)
+				device_wakeup_enable(&serio->dev);
 		}
 	}
 }
-- 
2.17.0

^ permalink raw reply related

* RE: [PATCH 0/8] Input: support for latest Lenovo thinkpads (series 80)
From: 廖崇榮 @ 2018-04-11  4:32 UTC (permalink / raw)
  To: 'Benjamin Tissoires'
  Cc: 'Dmitry Torokhov', 'Oliver Haessler',
	'Benjamin Berg', 'open list:HID CORE LAYER',
	'lkml'
In-Reply-To: <CAO-hwJKM51-N=ffO2sfZ6-j=L+qEsUt_bFJ+YOdJboROodPmOg@mail.gmail.com>

Hi Benjamin,

-----Original Message-----
From: Benjamin Tissoires [mailto:benjamin.tissoires@redhat.com] 
Sent: Tuesday, April 10, 2018 3:35 PM
To: 廖崇榮
Cc: Dmitry Torokhov; Oliver Haessler; Benjamin Berg; open list:HID CORE LAYER; lkml
Subject: Re: [PATCH 0/8] Input: support for latest Lenovo thinkpads (series 80)

Hi KT,

On Tue, Apr 10, 2018 at 7:45 AM, 廖崇榮 <kt.liao@emc.com.tw> wrote:
> Hi Benjamin,
>
> Thanks so much for your patch.
>
> I have tested them for Elan Gen5/Gen6(new) touchpad with SMbus/PS2.
> It works fine in my thinkpad so far but I find an issue today after lid-close/open.
>
> I am not sure if you can see it in T480S , I "guess" it may be relative to i2c_i801.
>
> The lid-close will enter deep sleep and cut touchpad power.
> I can see the resume flow after lid-open and SMbus-initial try to request hello package but fail.
> Strangely, I can't see any SMbus host signal on LA scope after power-on.

That's weird. I do not see this, by either closing the lid or directly calling 'systemctl suspend'.
On my system, i2c-i801 is also compiled as a module but psmouse is not (directly in vmlinuz).
[KT] : It's good to know your system doesn't meet this problem. 
     My SMbus laptop is an engineer sample in 2015, and PM tell me that NFC attaches to the same bus.
     I guess it's a single case because it's not a stable platform.

>
> I can't switch to SMbus after rmmod/modprobe psmouse because error happen in elantech_create_smbus.

If the SMBus adapter is failing, it is somewhat expected. Reloading psmouse will force a re-trigger of the SMBus probe function, but if the underlying communication fails, there is no way for psmouse to know it failed, so the PS/2 node will disappear. And the SMBus device will not be there.
[KT] : It switch to PS/2 interface after rmmod/modprobe psmouse.

> It will be recovered only if I rmmod/modprobe i2c_i801 first.

Just to be sure, what happens if you rmmod/modprobe elan_i2c instead of i2c_i801?
[KT] :I tried rmmod/modprobe elan_i2c first , but can't recover touchpad. 
    No error printed in the dmesg. I will add more message to check it later.

And which kernel are you running? On a vanilla 4.16 + Dmitry's next branch I do not see such issues.
[KT]: I use 4.15, I may try 4.16 later.

>
> Do you have any idea about it?

If reloading elan_i2c doesn't fix the situation, it must be in i2c_i801. But this is weird that this happens on your platform but not on my t480s going into S3.
[KT] I think so. As I mention that I can't see bus signal on scope. That make me guess it's the bus adaptor issue.
    It's an early-stage engineer system, power Issue will be expected.

Cheers,
Benjamin

>
> Thanks
> KT
> -----Original Message-----
> From: Benjamin Tissoires [mailto:benjamin.tissoires@redhat.com]
> Sent: Friday, April 06, 2018 2:51 PM
> To: Dmitry Torokhov
> Cc: 廖崇榮; Oliver Haessler; Benjamin Berg; open list:HID CORE LAYER; 
> lkml
> Subject: Re: [PATCH 0/8] Input: support for latest Lenovo thinkpads 
> (series 80)
>
> Hi Dmitry,
>
> On Fri, Apr 6, 2018 at 1:51 AM, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
>> Hi Benjamin,
>>
>> On Thu, Apr 05, 2018 at 03:25:29PM +0200, Benjamin Tissoires wrote:
>>> Hi Dmitry,
>>>
>>> well, this year, Lenovo gave us a surprise and decided to not use 
>>> the same touchpad/trackstick in all its model. And by default, the 
>>> support under Linux is less than ideal.
>>>
>>> Please find a series that should fix those issues. Compared to the 
>>> 60 series, there do not seem to e BIOS table issues this time, and 
>>> suspend/resume works fine thanks to your latest trackstick fixes.
>>>
>>> The T480s is a different beast, as it uses an Elan touchpad.
>>> I have been carrying the patches 3-6 for a while and tested previous 
>>> versions on various Elan PS/2 hardware without an issue as far as I 
>>> could tell. I was lacking tests from users with SMBus as all the 
>>> laptops I tried where puer PS/2.
>>>
>>> Anyway, it would be cool if you could have a look at the series.
>>
>> I am mostly happy with the series, but I would love to hear KT's take 
>> on it.
>
> thanks for the quick review.
> I worked closely with KT for this series. He helped me a lot for the tiny firmware changes that were required. However, quoting his email from Tuesday:
> "There will be a spring vacation in Taiwan from tomorrow." I guess we won't hear from him until the end of next week as we always have a backlog of urgent things to do after holidays...
>
> Cheers,
> Benjamin
>

^ permalink raw reply

* Re: [PATCH] Input: leds - fix out of bound access
From: Sasha Levin @ 2018-04-10 13:50 UTC (permalink / raw)
  To: Sasha Levin, Dmitry Torokhov, linux-input@vger.kernel.org
  Cc: linux-kernel@vger.kernel.org, Tasos Sahanidis,
	stable@vger.kernel.org
In-Reply-To: <20180406181242.GA225849@dtor-ws>

Hi,

[This is an automated email]

This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all

The bot has also determined it's probably a bug fixing patch. (score: 97.7389)

The bot has tested the following trees: v4.16.1, v4.15.16, v4.14.33, v4.9.93, v4.4.127.

v4.16.1: Build OK!
v4.15.16: Build OK!
v4.14.33: Build OK!
v4.9.93: Build OK!
v4.4.127: Build OK!

Please let us know if you'd like to have this patch included in a stable tree.

--
Thanks,
Sasha

^ permalink raw reply

* Re: [PATCH v2 00/10]  Add support for SAMA5D2 touchscreen
From: Jonathan Cameron @ 2018-04-10 13:47 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: Jonathan Cameron, Eugen Hristev, devicetree, alexandre.belloni,
	linux-iio, dmitry.torokhov, linux-kernel, ludovic.desroches,
	linux-input, linux-arm-kernel
In-Reply-To: <20180410073812.xbt3n6oegr23nlff@qschulz>

On Tue, 10 Apr 2018 09:38:12 +0200
Quentin Schulz <quentin.schulz@bootlin.com> wrote:

> Hi Jonathan and Eugen,
> 
> On Fri, Mar 30, 2018 at 02:02:12PM +0100, Jonathan Cameron wrote:
> > On Tue, 27 Mar 2018 15:32:33 +0300
> > Eugen Hristev <eugen.hristev@microchip.com> wrote:
> >   
> > > Hello,
> > > 
> > > This patch series is a rework of my previous series named:
> > > [PATCH 00/14] iio: triggers: add consumer support
> > > 
> > > In few words, this is the implementation of splitting the functionality
> > > of the IP block ADC device in SAMA5D2 SoC from ADC with touchscreen
> > > support. In order to avoid having a MFD device, two separate
> > > drivers that would work on same register base and split the IRQ,etc,
> > > as advised on the mailing list, I created a consumer driver for the
> > > channels, that will connect to the ADC as described in the device tree.
> > > 
> > > I have collected feedback from everyone and here is the result:
> > > I have added a new generic resistive touchscreen driver, which acts
> > > as a iio consumer for the given channels and will create an input
> > > device and report the events. It uses a callback buffer to register
> > > to the IIO device and waits for data to be pushed.
> > > Inside the IIO device, I have kept a similar approach with the first version
> > > of the series, except that now the driver can take multiple buffers, and
> > > will configure the touchscreen part of the hardware device if the specific
> > > channels are requested.
> > > 
> > > The SAMA5D2 ADC driver registers three new channels: two for the
> > > position on the X and Y axis, and one for the touch pressure.
> > > When channels are requested, it will check if the touchscreen channel mask
> > > includes the requested channels (it is possible that the consumer driver
> > > will not request pressure for example). If it's the case, it will work
> > > in touchscreen mode, and will refuse to do usual analog-digital conversion,
> > > because we have a single trigger and the touchscreen needs it.
> > > When the scan mask will include only old channels, the driver will function
> > > in the same way as before. If the scan mask somehow is a mix of the two (the
> > > masks intersect), the driver will refuse to work whatsoever (cannot have both
> > > in the same time).
> > > The driver allows reading raw data for the new channels, if claim direct
> > > mode works: no touchscreen driver requested anything. The new channels can
> > > act like the old ones. However, when requesting these channels, the usual
> > > trigger will not work and will not be enabled. The touchscreen channels
> > > require special trigger and irq configuration: pen detect, no pen detect
> > > and a periodic trigger to sample the touchscreen position and pressure.
> > > If the user attempts to use another trigger while there is a buffer
> > > that already requested the touchscreen channels (thus the trigger), the
> > > driver will refuse to comply.
> > > 
> > > In order to have defines for the channel numbers, I added a bindings include
> > > file that goes on a separate commit :
> > > dt-bindings: iio: adc: at91-sama5d2_adc: add channel specific consumer info
> > > This should go in the same tree with the following commits :
> > >   ARM: dts: at91: sama5d2: add channel cells for ADC device
> > >   ARM: dts: at91: sama5d2: Add resistive touch device
> > > 
> > > as build will break because these commits depend on the binding one
> > > which creates the included header file.
> > > 
> > > After the discussions earlier this year on the mailing list, I hope this
> > > rework of the patches is much better and fulfills all the requirements
> > > for this implementation.  
> > As I said in one of the later patches, I like this a lot.
> > It is a good blend of the moderately nasty handling needed in the ADC
> > driver with a lovely generic input driver.
> > 
> > Very nice!  Hope everyone else agrees ;)
> >   
> 
> I'd love to see a generic touchscreen driver being an iio consumer!
> 
> However, I've already a case that can't be handled unfortunately.
> 
> I posted ~2 years ago a patch series[1] for touchscreen support for
> Allwinner SoCs that are using their ADC (also) for touchscreen purpose.
> 
> It's been a very long time, I'm trying the hardest I can with my
> "IIRC-skills".
> 
> There are several problems:
>   - Data is stored in one register as a queue following this scheme:
>     X @t=0 coord, Y @t=0 coord, X @t=1 coord, Y @t=1 coord, X @t=2
>     coord, Y @t=2 coord, etc...
> 
>     Thus, I suppose I've only one channel and not two for coordinates.

As I read this, no you don't.  You have two channels going through a
very short on chip hw fifo.  They need to be reported as two channels
with data appropriate reformatted to reflect that before being pushed
to the IIO buffer interfaces.

> 
>   - The first data stored after an up event is absolutely unreliable so
>   I have to flush it, thus I need to use another API call to have the
>   touchscreen driver do some logic with a whole queue only when it's
>   after an up event (it doesn't really make sense to do so in the IIO
>   driver, does it?).

If it is garbage data I don't see any real problem with doing it in
the IIO driver.  But obviously I don't have details to hand so there
may be some complexity in this - I'm not sure the IIO driver knows
enough about what is going on to distinguish that this data is bad.

> 
>   - This up event is an interrupt.. that is configurable from the same
>   set of registers than for the ADC, so I need an mfd,
Hmm.  So there are various options here other than using an mfd.

1. Could use the event to 'filter' the trigger being used to drive the
data flow to the touch screen driver. So, as far as the touch screen
driver is concerned it gets good data only when a touch is in progress.
We present it as a 'magic' trigger that can be associated with the
device that happens to have this property.

2. Could actually write the (long missing) in kernel API for IIO events and
support the touch as a threshold falling event and add support for
handling to the touchscreen driver.
> 
> What are your thoughts (and maybe workarounds?) on those issues?
> 
Not trivial and at some point we'll be different enough from the generic
driver that it makes sense to perhaps have a couple of 'classes' of generic
touch screen to cover common options without making thing too complex.
That might be in one driver, and it might not.

> Thanks,
> Quentin
> 
> [1] https://lkml.org/lkml/2016/7/20/156
> 
> > Jonathan
> >   
> > > 
> > > Eugen Hristev (10):
> > >   MAINTAINERS: add generic resistive touchscreen adc
> > >   iio: Add channel for Position Relative
> > >   dt-bindings: input: touchscreen: touch_adc: create bindings
> > >   iio: inkern: add module put/get on iio dev module when requesting
> > >     channels
> > >   iio: adc: at91-sama5d2_adc: fix channel configuration for differential
> > >     channels
> > >   iio: adc: at91-sama5d2_adc: add support for position and pressure
> > >     channels
> > >   input: touchscreen: touch_adc: add generic resistive ADC touchscreen
> > >   dt-bindings: iio: adc: at91-sama5d2_adc: add channel specific consumer
> > >     info
> > >   ARM: dts: at91: sama5d2: add channel cells for ADC device
> > >   ARM: dts: at91: sama5d2: Add resistive touch device
> > > 
> > >  Documentation/ABI/testing/sysfs-bus-iio            |  12 +
> > >  .../bindings/iio/adc/at91-sama5d2_adc.txt          |   9 +
> > >  .../bindings/input/touchscreen/touch_adc.txt       |  33 ++
> > >  MAINTAINERS                                        |   6 +
> > >  arch/arm/boot/dts/sama5d2.dtsi                     |  12 +
> > >  drivers/iio/adc/at91-sama5d2_adc.c                 | 491 ++++++++++++++++++++-
> > >  drivers/iio/industrialio-core.c                    |   1 +
> > >  drivers/iio/inkern.c                               |   8 +-
> > >  drivers/input/touchscreen/Kconfig                  |  13 +
> > >  drivers/input/touchscreen/Makefile                 |   1 +
> > >  drivers/input/touchscreen/touch_adc.c              | 199 +++++++++
> > >  include/dt-bindings/iio/adc/at91-sama5d2_adc.h     |  16 +
> > >  include/uapi/linux/iio/types.h                     |   1 +
> > >  tools/iio/iio_event_monitor.c                      |   2 +
> > >  14 files changed, 791 insertions(+), 13 deletions(-)
> > >  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/touch_adc.txt
> > >  create mode 100644 drivers/input/touchscreen/touch_adc.c
> > >  create mode 100644 include/dt-bindings/iio/adc/at91-sama5d2_adc.h
> > >   
> > 
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel  
> 

^ permalink raw reply

* Re: [PATCH v3 1/3] resource: Use list_head to link resource sibling
From: Baoquan He @ 2018-04-10 13:44 UTC (permalink / raw)
  To: Rob Herring
  Cc: Brijesh Singh, devicetree-u79uwXL29TY76Z2rM5mHXA, David Airlie,
	linux-pci-u79uwXL29TY76Z2rM5mHXA, Wei Yang, Keith Busch,
	Yaowei Bai, K. Y. Srinivasan, Frank Rowand, Thomas Gleixner,
	Lorenzo Pieralisi, Stephen Hemminger, Nicolas Pitre,
	Patrik Jakobsson, linux-input-u79uwXL29TY76Z2rM5mHXA,
	Borislav Petkov, Tom Lendacky, Haiyang Zhang,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Jérôme Glisse,
	Bjorn Helgaas, Jonathan Derrick, Greg Kroah-Hartman
In-Reply-To: <CAL_Jsq+xxa9ep2LK3H6YWhwrZtc-T73Y84ry6Hs-JJ3XEXoGsw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi Rob,

Thanks a lot for looking into this and involve Nico to this thread!

On 04/09/18 at 09:49am, Rob Herring wrote:
> +Nico who has been working on tinification of the kernel.
> 
> On Mon, Apr 9, 2018 at 4:08 AM, Baoquan He <bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > The struct resource uses singly linked list to link siblings. It's not
> > easy to do reverse iteration on sibling list. So replace it with list_head.
> 
> Why is reverse iteration needed?

This is the explanation I made when Andrew helped to review the v1 post:
https://lkml.org/lkml/2018/3/23/78

Because we have been using kexec-tools utility to search available
System RAM space for loading kernel/initrd/purgatory from top to down.
That is done in user space by searching /proc/iomem. While later added
kexec_file interface, the searching code happened in kernel, and it
only search System RAM region bottom up, then take an area in that found
RAM region from top to down. We need unify these two interfaces on
behaviour since they are the same on essense from the users' point of
view, though implementation is different. As you know, the singly linked
list implementation of the current resource's sibling linking, makes the
searching from top to down very hard to satisfy people. 

Below is the v1 post, we make an temporary array to copy iomem_resource's
first level of children, then iterate the array reversedly. Andrew
suggested me to try list_head after reviewing. In fact we can optimize
that patch to only copy resource pointer into array, still the way is
ugly.
https://lkml.org/lkml/2018/3/21/952

Then Wei pasted a patch he had made as below. He didn't mention if he
also has requirement on reversed iteration of resource. That is an O(n*n)
way, from personal feelings, hard to say if it's bettern than v1 post.
https://lkml.org/lkml/2018/3/24/157

That's why I would like to have a try of the list_head linking.

> 
> > And code refactoring makes codes in kernel/resource.c more readable than
> > pointer operation.
> 
> resource_for_each_* helpers could solve that without the size increase.

Nico mentioned llist, that is a linux kernel standard singly linked
list, very elegant code existed. Still can not satisfy the reversed
iteration requirement.

> 
> > Besides, type of member variables of struct resource, sibling and child, are
> > changed from 'struct resource *' to 'struct list_head'. Kernel size will
> > increase because of those statically defined struct resource instances.
> 
> The DT struct device_node also has the same tree structure with
> parent, child, sibling pointers and converting to list_head had been
> on the todo list for a while. ACPI also has some tree walking
> functions (drivers/acpi/acpica/pstree.c). Perhaps there should be a
> common tree struct and helpers defined either on top of list_head or a
> new struct if that saves some size.

We have had many this kind of trees in kernel, the obvious examples is 
task_struct. And struct task_group {} too. They have used list_head
already.
struct task_struct {
	...
	/* Real parent process: */                                                                                 
        struct task_struct __rcu        *real_parent;                                                              
                                                                                                                   
        /* Recipient of SIGCHLD, wait4() reports: */                                                               
        struct task_struct __rcu        *parent;
	struct list_head                children;                                                                  
        struct list_head                sibling;
	...
}

		    root
                ///   |   \\\
               ///    |    \\\
              ///     |     \\\
             ///      |      \\\
          (child)<->(child)<->(child)
                         ///   |   \\\
                     ///       |      \\\
                   ///         |          \\\
                ///            |             \\\
             (grandchild)<->(grandchild)<->(grandchild)

If define an new struct, , like tree_list, or tlist?

struct tlist {
	void			*parent;
	struct list_head        sibling;                                                                                                         
	struct list_head        child;
}

Just a rough idea, feel it might not bring too much benefit compared
with direct list operation.

> 
> >
> > Signed-off-by: Baoquan He <bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > ---
> > v2->v3:
> >   Make sibling() and first_child() global so that they can be called
> >   out of kernel/resource.c to simplify code.
> 
> These should probably be inline functions. Or exported if not.
> 
> >
> >   Fix several code bugs found by kbuild test robot.
> >
> >   Got report from lkp that kernel size increased. It's on purpose since
> >   the type change of sibling and child inside struct resource{}. For
> >   each struct resource variable, it will cost another 16 bytes on x86 64.
> 
> The size increase should be mentioned in the commit log. More
> generally, the size increase is 2 pointers.

Simply mentioned the size increase in this updated version. Yes, 2
pointers increased. Will add it to log ot make it more specifically.

Thanks
Baoquan

> >
> >  arch/sparc/kernel/ioport.c                  |   2 +-
> >  drivers/gpu/drm/drm_memory.c                |   3 +-
> >  drivers/gpu/drm/gma500/gtt.c                |   5 +-
> >  drivers/hv/vmbus_drv.c                      |  52 ++++----
> >  drivers/input/joystick/iforce/iforce-main.c |   4 +-
> >  drivers/nvdimm/e820.c                       |   2 +-
> >  drivers/nvdimm/namespace_devs.c             |   6 +-
> >  drivers/nvdimm/nd.h                         |   5 +-
> >  drivers/of/address.c                        |   4 +-
> >  drivers/parisc/lba_pci.c                    |   4 +-
> >  drivers/pci/host/vmd.c                      |   8 +-
> >  drivers/pci/probe.c                         |   2 +
> >  drivers/pci/setup-bus.c                     |   2 +-
> >  include/linux/ioport.h                      |   6 +-
> >  kernel/resource.c                           | 193 ++++++++++++++--------------
> >  15 files changed, 149 insertions(+), 149 deletions(-)
> >
> > diff --git a/arch/sparc/kernel/ioport.c b/arch/sparc/kernel/ioport.c
> > index 3bcef9ce74df..4e91fbbbedcc 100644
> > --- a/arch/sparc/kernel/ioport.c
> > +++ b/arch/sparc/kernel/ioport.c
> > @@ -669,7 +669,7 @@ static int sparc_io_proc_show(struct seq_file *m, void *v)
> >         struct resource *root = m->private, *r;
> >         const char *nm;
> >
> > -       for (r = root->child; r != NULL; r = r->sibling) {
> > +       list_for_each_entry(r, &root->child, sibling) {
> >                 if ((nm = r->name) == NULL) nm = "???";
> >                 seq_printf(m, "%016llx-%016llx: %s\n",
> >                                 (unsigned long long)r->start,
> > diff --git a/drivers/gpu/drm/drm_memory.c b/drivers/gpu/drm/drm_memory.c
> > index 3c54044214db..53e300a993dc 100644
> > --- a/drivers/gpu/drm/drm_memory.c
> > +++ b/drivers/gpu/drm/drm_memory.c
> > @@ -155,9 +155,8 @@ u64 drm_get_max_iomem(void)
> >         struct resource *tmp;
> >         resource_size_t max_iomem = 0;
> >
> > -       for (tmp = iomem_resource.child; tmp; tmp = tmp->sibling) {
> > +       list_for_each_entry(tmp, &iomem_resource.child, sibling)
> >                 max_iomem = max(max_iomem,  tmp->end);
> > -       }
> >
> >         return max_iomem;
> >  }
> > diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
> > index 3949b0990916..addd3bc009af 100644
> > --- a/drivers/gpu/drm/gma500/gtt.c
> > +++ b/drivers/gpu/drm/gma500/gtt.c
> > @@ -565,7 +565,7 @@ int psb_gtt_init(struct drm_device *dev, int resume)
> >  int psb_gtt_restore(struct drm_device *dev)
> >  {
> >         struct drm_psb_private *dev_priv = dev->dev_private;
> > -       struct resource *r = dev_priv->gtt_mem->child;
> > +       struct resource *r;
> >         struct gtt_range *range;
> >         unsigned int restored = 0, total = 0, size = 0;
> >
> > @@ -573,14 +573,13 @@ int psb_gtt_restore(struct drm_device *dev)
> >         mutex_lock(&dev_priv->gtt_mutex);
> >         psb_gtt_init(dev, 1);
> >
> > -       while (r != NULL) {
> > +       list_for_each_entry(r, &dev_priv->gtt_mem->child, sibling) {
> >                 range = container_of(r, struct gtt_range, resource);
> >                 if (range->pages) {
> >                         psb_gtt_insert(dev, range, 1);
> >                         size += range->resource.end - range->resource.start;
> >                         restored++;
> >                 }
> > -               r = r->sibling;
> >                 total++;
> >         }
> >         mutex_unlock(&dev_priv->gtt_mutex);
> > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> > index bc65c4d79c1f..7ba8a25520d9 100644
> > --- a/drivers/hv/vmbus_drv.c
> > +++ b/drivers/hv/vmbus_drv.c
> > @@ -1413,9 +1413,8 @@ static acpi_status vmbus_walk_resources(struct acpi_resource *res, void *ctx)
> >  {
> >         resource_size_t start = 0;
> >         resource_size_t end = 0;
> > -       struct resource *new_res;
> > +       struct resource *new_res, *tmp;
> >         struct resource **old_res = &hyperv_mmio;
> > -       struct resource **prev_res = NULL;
> >
> >         switch (res->type) {
> >
> > @@ -1462,44 +1461,36 @@ static acpi_status vmbus_walk_resources(struct acpi_resource *res, void *ctx)
> >         /*
> >          * If two ranges are adjacent, merge them.
> >          */
> > -       do {
> > -               if (!*old_res) {
> > -                       *old_res = new_res;
> > -                       break;
> > -               }
> > -
> > -               if (((*old_res)->end + 1) == new_res->start) {
> > -                       (*old_res)->end = new_res->end;
> > +       if (!*old_res) {
> > +               *old_res = new_res;
> > +               return AE_OK;
> > +       }
> > +       tmp = *old_res;
> > +       list_for_each_entry_from(tmp, &tmp->parent->child, sibling) {
> > +               if ((tmp->end + 1) == new_res->start) {
> > +                       tmp->end = new_res->end;
> >                         kfree(new_res);
> >                         break;
> >                 }
> >
> > -               if ((*old_res)->start == new_res->end + 1) {
> > -                       (*old_res)->start = new_res->start;
> > +               if (tmp->start == new_res->end + 1) {
> > +                       tmp->start = new_res->start;
> >                         kfree(new_res);
> >                         break;
> >                 }
> >
> > -               if ((*old_res)->start > new_res->end) {
> > -                       new_res->sibling = *old_res;
> > -                       if (prev_res)
> > -                               (*prev_res)->sibling = new_res;
> > -                       *old_res = new_res;
> > +               if (tmp->start > new_res->end) {
> > +                       list_add(&new_res->sibling, tmp->sibling.prev);
> >                         break;
> >                 }
> > -
> > -               prev_res = old_res;
> > -               old_res = &(*old_res)->sibling;
> > -
> > -       } while (1);
> > +       }
> >
> >         return AE_OK;
> >  }
> >
> >  static int vmbus_acpi_remove(struct acpi_device *device)
> >  {
> > -       struct resource *cur_res;
> > -       struct resource *next_res;
> > +       struct resource *res;
> >
> >         if (hyperv_mmio) {
> >                 if (fb_mmio) {
> > @@ -1508,10 +1499,9 @@ static int vmbus_acpi_remove(struct acpi_device *device)
> >                         fb_mmio = NULL;
> >                 }
> >
> > -               for (cur_res = hyperv_mmio; cur_res; cur_res = next_res) {
> > -                       next_res = cur_res->sibling;
> > -                       kfree(cur_res);
> > -               }
> > +               res = hyperv_mmio;
> > +               list_for_each_entry_from(res, &res->parent->child, sibling)
> > +                       kfree(res);
> >         }
> >
> >         return 0;
> > @@ -1597,7 +1587,8 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj,
> >                 }
> >         }
> >
> > -       for (iter = hyperv_mmio; iter; iter = iter->sibling) {
> > +       iter = hyperv_mmio;
> > +       list_for_each_entry_from(iter, &iter->parent->child, sibling) {
> >                 if ((iter->start >= max) || (iter->end <= min))
> >                         continue;
> >
> > @@ -1640,7 +1631,8 @@ void vmbus_free_mmio(resource_size_t start, resource_size_t size)
> >         struct resource *iter;
> >
> >         down(&hyperv_mmio_lock);
> > -       for (iter = hyperv_mmio; iter; iter = iter->sibling) {
> > +       iter = hyperv_mmio;
> > +       list_for_each_entry_from(iter, &iter->parent->child, sibling) {
> >                 if ((iter->start >= start + size) || (iter->end <= start))
> >                         continue;
> >
> > diff --git a/drivers/input/joystick/iforce/iforce-main.c b/drivers/input/joystick/iforce/iforce-main.c
> > index daeeb4c7e3b0..5c0be27b33ff 100644
> > --- a/drivers/input/joystick/iforce/iforce-main.c
> > +++ b/drivers/input/joystick/iforce/iforce-main.c
> > @@ -305,8 +305,8 @@ int iforce_init_device(struct iforce *iforce)
> >         iforce->device_memory.end = 200;
> >         iforce->device_memory.flags = IORESOURCE_MEM;
> >         iforce->device_memory.parent = NULL;
> > -       iforce->device_memory.child = NULL;
> > -       iforce->device_memory.sibling = NULL;
> > +       INIT_LIST_HEAD(&iforce->device_memory.child);
> > +       INIT_LIST_HEAD(&iforce->device_memory.sibling);
> >
> >  /*
> >   * Wait until device ready - until it sends its first response.
> > diff --git a/drivers/nvdimm/e820.c b/drivers/nvdimm/e820.c
> > index 6f9a6ffd7cde..513e661bb0d8 100644
> > --- a/drivers/nvdimm/e820.c
> > +++ b/drivers/nvdimm/e820.c
> > @@ -53,7 +53,7 @@ static int e820_pmem_probe(struct platform_device *pdev)
> >                 goto err;
> >         platform_set_drvdata(pdev, nvdimm_bus);
> >
> > -       for (p = iomem_resource.child; p ; p = p->sibling) {
> > +       list_for_each_entry(p, &iomem_resource.child, sibling) {
> >                 struct nd_region_desc ndr_desc;
> >
> >                 if (p->desc != IORES_DESC_PERSISTENT_MEMORY_LEGACY)
> > diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c
> > index 658ada497be0..bcbdf5335909 100644
> > --- a/drivers/nvdimm/namespace_devs.c
> > +++ b/drivers/nvdimm/namespace_devs.c
> > @@ -637,7 +637,7 @@ static resource_size_t scan_allocate(struct nd_region *nd_region,
> >   retry:
> >         first = 0;
> >         for_each_dpa_resource(ndd, res) {
> > -               struct resource *next = res->sibling, *new_res = NULL;
> > +               struct resource *next = sibling(res), *new_res = NULL;
> >                 resource_size_t allocate, available = 0;
> >                 enum alloc_loc loc = ALLOC_ERR;
> >                 const char *action;
> > @@ -763,7 +763,7 @@ static resource_size_t scan_allocate(struct nd_region *nd_region,
> >          * an initial "pmem-reserve pass".  Only do an initial BLK allocation
> >          * when none of the DPA space is reserved.
> >          */
> > -       if ((is_pmem || !ndd->dpa.child) && n == to_allocate)
> > +       if ((is_pmem || list_empty(&ndd->dpa.child)) && n == to_allocate)
> >                 return init_dpa_allocation(label_id, nd_region, nd_mapping, n);
> >         return n;
> >  }
> > @@ -779,7 +779,7 @@ static int merge_dpa(struct nd_region *nd_region,
> >   retry:
> >         for_each_dpa_resource(ndd, res) {
> >                 int rc;
> > -               struct resource *next = res->sibling;
> > +               struct resource *next = sibling(res);
> >                 resource_size_t end = res->start + resource_size(res);
> >
> >                 if (!next || strcmp(res->name, label_id->id) != 0
> > diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
> > index 184e070d50a2..1dc0b2370bd1 100644
> > --- a/drivers/nvdimm/nd.h
> > +++ b/drivers/nvdimm/nd.h
> > @@ -102,11 +102,10 @@ unsigned sizeof_namespace_label(struct nvdimm_drvdata *ndd);
> >                 (unsigned long long) (res ? res->start : 0), ##arg)
> >
> >  #define for_each_dpa_resource(ndd, res) \
> > -       for (res = (ndd)->dpa.child; res; res = res->sibling)
> > +       list_for_each_entry(res, &(ndd)->dpa.child, sibling)
> >
> >  #define for_each_dpa_resource_safe(ndd, res, next) \
> > -       for (res = (ndd)->dpa.child, next = res ? res->sibling : NULL; \
> > -                       res; res = next, next = next ? next->sibling : NULL)
> > +       list_for_each_entry_safe(res, next, &(ndd)->dpa.child, sibling)
> >
> >  struct nd_percpu_lane {
> >         int count;
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > index 53349912ac75..e2e25719ab52 100644
> > --- a/drivers/of/address.c
> > +++ b/drivers/of/address.c
> > @@ -330,7 +330,9 @@ int of_pci_range_to_resource(struct of_pci_range *range,
> >  {
> >         int err;
> >         res->flags = range->flags;
> > -       res->parent = res->child = res->sibling = NULL;
> > +       res->parent = NULL;
> > +       INIT_LIST_HEAD(&res->child);
> > +       INIT_LIST_HEAD(&res->sibling);
> >         res->name = np->full_name;
> >
> >         if (res->flags & IORESOURCE_IO) {
> > diff --git a/drivers/parisc/lba_pci.c b/drivers/parisc/lba_pci.c
> > index 69bd98421eb1..84f04418ae0b 100644
> > --- a/drivers/parisc/lba_pci.c
> > +++ b/drivers/parisc/lba_pci.c
> > @@ -170,8 +170,8 @@ lba_dump_res(struct resource *r, int d)
> >         for (i = d; i ; --i) printk(" ");
> >         printk(KERN_DEBUG "%p [%lx,%lx]/%lx\n", r,
> >                 (long)r->start, (long)r->end, r->flags);
> > -       lba_dump_res(r->child, d+2);
> > -       lba_dump_res(r->sibling, d);
> > +       lba_dump_res(first_child(&r->child), d+2);
> > +       lba_dump_res(sibling(r), d);
> >  }
> >
> >
> > diff --git a/drivers/pci/host/vmd.c b/drivers/pci/host/vmd.c
> > index 930a8fa08bd6..c3000af903ea 100644
> > --- a/drivers/pci/host/vmd.c
> > +++ b/drivers/pci/host/vmd.c
> > @@ -520,14 +520,14 @@ static struct pci_ops vmd_ops = {
> >
> >  static void vmd_attach_resources(struct vmd_dev *vmd)
> >  {
> > -       vmd->dev->resource[VMD_MEMBAR1].child = &vmd->resources[1];
> > -       vmd->dev->resource[VMD_MEMBAR2].child = &vmd->resources[2];
> > +       list_add(&vmd->resources[1].sibling, &vmd->dev->resource[VMD_MEMBAR1].child);
> > +       list_add(&vmd->resources[2].sibling, &vmd->dev->resource[VMD_MEMBAR2].child);
> >  }
> >
> >  static void vmd_detach_resources(struct vmd_dev *vmd)
> >  {
> > -       vmd->dev->resource[VMD_MEMBAR1].child = NULL;
> > -       vmd->dev->resource[VMD_MEMBAR2].child = NULL;
> > +       INIT_LIST_HEAD(&vmd->dev->resource[VMD_MEMBAR1].child);
> > +       INIT_LIST_HEAD(&vmd->dev->resource[VMD_MEMBAR2].child);
> >  }
> >
> >  /*
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index ac91b6fd0bcd..d162c77bec29 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -59,6 +59,8 @@ static struct resource *get_pci_domain_busn_res(int domain_nr)
> >         r->res.start = 0;
> >         r->res.end = 0xff;
> >         r->res.flags = IORESOURCE_BUS | IORESOURCE_PCI_FIXED;
> > +       INIT_LIST_HEAD(&r->res.child);
> > +       INIT_LIST_HEAD(&r->res.sibling);
> >
> >         list_add_tail(&r->list, &pci_domain_busn_res_list);
> >
> > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> > index 072784f55ea5..0d5e30004ca6 100644
> > --- a/drivers/pci/setup-bus.c
> > +++ b/drivers/pci/setup-bus.c
> > @@ -2107,7 +2107,7 @@ int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type)
> >                                 continue;
> >
> >                         /* Ignore BARs which are still in use */
> > -                       if (res->child)
> > +                       if (!list_empty(&res->child))
> >                                 continue;
> >
> >                         ret = add_to_list(&saved, bridge, res, 0, 0);
> > diff --git a/include/linux/ioport.h b/include/linux/ioport.h
> > index da0ebaec25f0..03d1510f03e0 100644
> > --- a/include/linux/ioport.h
> > +++ b/include/linux/ioport.h
> > @@ -22,7 +22,8 @@ struct resource {
> >         const char *name;
> >         unsigned long flags;
> >         unsigned long desc;
> > -       struct resource *parent, *sibling, *child;
> > +       struct list_head child, sibling;
> > +       struct resource *parent;
> >  };
> >
> >  /*
> > @@ -189,6 +190,8 @@ extern int allocate_resource(struct resource *root, struct resource *new,
> >                                                        resource_size_t,
> >                                                        resource_size_t),
> >                              void *alignf_data);
> > +extern struct resource *sibling(struct resource *res);
> > +extern struct resource *first_child(struct list_head *head);
> >  struct resource *lookup_resource(struct resource *root, resource_size_t start);
> >  int adjust_resource(struct resource *res, resource_size_t start,
> >                     resource_size_t size);
> > @@ -215,7 +218,6 @@ static inline bool resource_contains(struct resource *r1, struct resource *r2)
> >         return r1->start <= r2->start && r1->end >= r2->end;
> >  }
> >
> > -
> >  /* Convenience shorthand with allocation */
> >  #define request_region(start,n,name)           __request_region(&ioport_resource, (start), (n), (name), 0)
> >  #define request_muxed_region(start,n,name)     __request_region(&ioport_resource, (start), (n), (name), IORESOURCE_MUXED)
> > diff --git a/kernel/resource.c b/kernel/resource.c
> > index e270b5048988..473c624606f9 100644
> > --- a/kernel/resource.c
> > +++ b/kernel/resource.c
> > @@ -31,6 +31,8 @@ struct resource ioport_resource = {
> >         .start  = 0,
> >         .end    = IO_SPACE_LIMIT,
> >         .flags  = IORESOURCE_IO,
> > +       .sibling = LIST_HEAD_INIT(ioport_resource.sibling),
> > +       .child  = LIST_HEAD_INIT(ioport_resource.child),
> >  };
> >  EXPORT_SYMBOL(ioport_resource);
> >
> > @@ -39,6 +41,8 @@ struct resource iomem_resource = {
> >         .start  = 0,
> >         .end    = -1,
> >         .flags  = IORESOURCE_MEM,
> > +       .sibling = LIST_HEAD_INIT(iomem_resource.sibling),
> > +       .child  = LIST_HEAD_INIT(iomem_resource.child),
> >  };
> >  EXPORT_SYMBOL(iomem_resource);
> >
> > @@ -57,20 +61,32 @@ static DEFINE_RWLOCK(resource_lock);
> >   * by boot mem after the system is up. So for reusing the resource entry
> >   * we need to remember the resource.
> >   */
> > -static struct resource *bootmem_resource_free;
> > +static struct list_head bootmem_resource_free = LIST_HEAD_INIT(bootmem_resource_free);
> >  static DEFINE_SPINLOCK(bootmem_resource_lock);
> >
> > +struct resource *sibling(struct resource *res)
> > +{
> > +       if (res->parent && !list_is_last(&res->sibling, &res->parent->child))
> > +               return list_next_entry(res, sibling);
> > +       return NULL;
> > +}
> > +
> > +struct resource *first_child(struct list_head *head)
> > +{
> > +       return list_first_entry_or_null(head, struct resource, sibling);
> > +}
> > +
> >  static struct resource *next_resource(struct resource *p, bool sibling_only)
> >  {
> >         /* Caller wants to traverse through siblings only */
> >         if (sibling_only)
> > -               return p->sibling;
> > +               return sibling(p);
> >
> > -       if (p->child)
> > -               return p->child;
> > -       while (!p->sibling && p->parent)
> > +       if (!list_empty(&p->child))
> > +               return first_child(&p->child);
> > +       while (!sibling(p) && p->parent)
> >                 p = p->parent;
> > -       return p->sibling;
> > +       return sibling(p);
> >  }
> >
> >  static void *r_next(struct seq_file *m, void *v, loff_t *pos)
> > @@ -90,7 +106,7 @@ static void *r_start(struct seq_file *m, loff_t *pos)
> >         struct resource *p = m->private;
> >         loff_t l = 0;
> >         read_lock(&resource_lock);
> > -       for (p = p->child; p && l < *pos; p = r_next(m, p, &l))
> > +       for (p = first_child(&p->child); p && l < *pos; p = r_next(m, p, &l))
> >                 ;
> >         return p;
> >  }
> > @@ -186,8 +202,7 @@ static void free_resource(struct resource *res)
> >
> >         if (!PageSlab(virt_to_head_page(res))) {
> >                 spin_lock(&bootmem_resource_lock);
> > -               res->sibling = bootmem_resource_free;
> > -               bootmem_resource_free = res;
> > +               list_add(&res->sibling, &bootmem_resource_free);
> >                 spin_unlock(&bootmem_resource_lock);
> >         } else {
> >                 kfree(res);
> > @@ -199,10 +214,9 @@ static struct resource *alloc_resource(gfp_t flags)
> >         struct resource *res = NULL;
> >
> >         spin_lock(&bootmem_resource_lock);
> > -       if (bootmem_resource_free) {
> > -               res = bootmem_resource_free;
> > -               bootmem_resource_free = res->sibling;
> > -       }
> > +       res = first_child(&bootmem_resource_free);
> > +       if (res)
> > +               list_del(&res->sibling);
> >         spin_unlock(&bootmem_resource_lock);
> >
> >         if (res)
> > @@ -210,6 +224,8 @@ static struct resource *alloc_resource(gfp_t flags)
> >         else
> >                 res = kzalloc(sizeof(struct resource), flags);
> >
> > +       INIT_LIST_HEAD(&res->child);
> > +       INIT_LIST_HEAD(&res->sibling);
> >         return res;
> >  }
> >
> > @@ -218,7 +234,7 @@ static struct resource * __request_resource(struct resource *root, struct resour
> >  {
> >         resource_size_t start = new->start;
> >         resource_size_t end = new->end;
> > -       struct resource *tmp, **p;
> > +       struct resource *tmp;
> >
> >         if (end < start)
> >                 return root;
> > @@ -226,64 +242,62 @@ static struct resource * __request_resource(struct resource *root, struct resour
> >                 return root;
> >         if (end > root->end)
> >                 return root;
> > -       p = &root->child;
> > -       for (;;) {
> > -               tmp = *p;
> > -               if (!tmp || tmp->start > end) {
> > -                       new->sibling = tmp;
> > -                       *p = new;
> > +
> > +       if (list_empty(&root->child)) {
> > +               list_add(&new->sibling, &root->child);
> > +               new->parent = root;
> > +               INIT_LIST_HEAD(&new->child);
> > +               return NULL;
> > +       }
> > +
> > +       list_for_each_entry(tmp, &root->child, sibling) {
> > +               if (tmp->start > end) {
> > +                       list_add(&new->sibling, tmp->sibling.prev);
> >                         new->parent = root;
> > +                       INIT_LIST_HEAD(&new->child);
> >                         return NULL;
> >                 }
> > -               p = &tmp->sibling;
> >                 if (tmp->end < start)
> >                         continue;
> >                 return tmp;
> >         }
> > +
> > +       list_add_tail(&new->sibling, &root->child);
> > +       new->parent = root;
> > +       INIT_LIST_HEAD(&new->child);
> > +       return NULL;
> >  }
> >
> >  static int __release_resource(struct resource *old, bool release_child)
> >  {
> > -       struct resource *tmp, **p, *chd;
> > +       struct resource *tmp, *next, *chd;
> >
> > -       p = &old->parent->child;
> > -       for (;;) {
> > -               tmp = *p;
> > -               if (!tmp)
> > -                       break;
> > +       list_for_each_entry_safe(tmp, next, &old->parent->child, sibling) {
> >                 if (tmp == old) {
> > -                       if (release_child || !(tmp->child)) {
> > -                               *p = tmp->sibling;
> > +                       if (release_child || list_empty(&tmp->child)) {
> > +                               list_del(&tmp->sibling);
> >                         } else {
> > -                               for (chd = tmp->child;; chd = chd->sibling) {
> > +                               list_for_each_entry(chd, &tmp->child, sibling)
> >                                         chd->parent = tmp->parent;
> > -                                       if (!(chd->sibling))
> > -                                               break;
> > -                               }
> > -                               *p = tmp->child;
> > -                               chd->sibling = tmp->sibling;
> > +                               list_splice(&tmp->child, tmp->sibling.prev);
> > +                               list_del(&tmp->sibling);
> >                         }
> > +
> >                         old->parent = NULL;
> >                         return 0;
> >                 }
> > -               p = &tmp->sibling;
> >         }
> >         return -EINVAL;
> >  }
> >
> >  static void __release_child_resources(struct resource *r)
> >  {
> > -       struct resource *tmp, *p;
> > +       struct resource *tmp, *next;
> >         resource_size_t size;
> >
> > -       p = r->child;
> > -       r->child = NULL;
> > -       while (p) {
> > -               tmp = p;
> > -               p = p->sibling;
> > -
> > +       list_for_each_entry_safe(tmp, next, &r->child, sibling) {
> >                 tmp->parent = NULL;
> > -               tmp->sibling = NULL;
> > +               INIT_LIST_HEAD(&tmp->sibling);
> >                 __release_child_resources(tmp);
> >
> >                 printk(KERN_DEBUG "release child resource %pR\n", tmp);
> > @@ -292,6 +306,8 @@ static void __release_child_resources(struct resource *r)
> >                 tmp->start = 0;
> >                 tmp->end = size - 1;
> >         }
> > +
> > +       INIT_LIST_HEAD(&tmp->child);
> >  }
> >
> >  void release_child_resources(struct resource *r)
> > @@ -376,7 +392,8 @@ static int find_next_iomem_res(struct resource *res, unsigned long desc,
> >
> >         read_lock(&resource_lock);
> >
> > -       for (p = iomem_resource.child; p; p = next_resource(p, sibling_only)) {
> > +       for (p = first_child(&iomem_resource.child); p;
> > +                       p = next_resource(p, sibling_only)) {
> >                 if ((p->flags & res->flags) != res->flags)
> >                         continue;
> >                 if ((desc != IORES_DESC_NONE) && (desc != p->desc))
> > @@ -564,7 +581,7 @@ int region_intersects(resource_size_t start, size_t size, unsigned long flags,
> >         struct resource *p;
> >
> >         read_lock(&resource_lock);
> > -       for (p = iomem_resource.child; p ; p = p->sibling) {
> > +       list_for_each_entry(p, &iomem_resource.child, sibling) {
> >                 bool is_type = (((p->flags & flags) == flags) &&
> >                                 ((desc == IORES_DESC_NONE) ||
> >                                  (desc == p->desc)));
> > @@ -618,7 +635,7 @@ static int __find_resource(struct resource *root, struct resource *old,
> >                          resource_size_t  size,
> >                          struct resource_constraint *constraint)
> >  {
> > -       struct resource *this = root->child;
> > +       struct resource *this = first_child(&root->child);
> >         struct resource tmp = *new, avail, alloc;
> >
> >         tmp.start = root->start;
> > @@ -628,7 +645,7 @@ static int __find_resource(struct resource *root, struct resource *old,
> >          */
> >         if (this && this->start == root->start) {
> >                 tmp.start = (this == old) ? old->start : this->end + 1;
> > -               this = this->sibling;
> > +               this = sibling(this);
> >         }
> >         for(;;) {
> >                 if (this)
> > @@ -663,7 +680,7 @@ next:               if (!this || this->end == root->end)
> >
> >                 if (this != old)
> >                         tmp.start = this->end + 1;
> > -               this = this->sibling;
> > +               this = sibling(this);
> >         }
> >         return -EBUSY;
> >  }
> > @@ -707,7 +724,7 @@ static int reallocate_resource(struct resource *root, struct resource *old,
> >                 goto out;
> >         }
> >
> > -       if (old->child) {
> > +       if (!list_empty(&old->child)) {
> >                 err = -EBUSY;
> >                 goto out;
> >         }
> > @@ -788,7 +805,7 @@ struct resource *lookup_resource(struct resource *root, resource_size_t start)
> >         struct resource *res;
> >
> >         read_lock(&resource_lock);
> > -       for (res = root->child; res; res = res->sibling) {
> > +       list_for_each_entry(res, &root->child, sibling) {
> >                 if (res->start == start)
> >                         break;
> >         }
> > @@ -821,32 +838,27 @@ static struct resource * __insert_resource(struct resource *parent, struct resou
> >                         break;
> >         }
> >
> > -       for (next = first; ; next = next->sibling) {
> > +       for (next = first; ; next = sibling(next)) {
> >                 /* Partial overlap? Bad, and unfixable */
> >                 if (next->start < new->start || next->end > new->end)
> >                         return next;
> > -               if (!next->sibling)
> > +               if (!sibling(next))
> >                         break;
> > -               if (next->sibling->start > new->end)
> > +               if (sibling(next)->start > new->end)
> >                         break;
> >         }
> > -
> >         new->parent = parent;
> > -       new->sibling = next->sibling;
> > -       new->child = first;
> > +       list_add(&new->sibling, &next->sibling);
> > +       INIT_LIST_HEAD(&new->child);
> >
> > -       next->sibling = NULL;
> > -       for (next = first; next; next = next->sibling)
> > +       /*
> > +        * From first to next, they all fall into new's region, so change them
> > +        * as new's children.
> > +        */
> > +       list_cut_position(&new->child, first->sibling.prev, &next->sibling);
> > +       list_for_each_entry(next, &new->child, sibling)
> >                 next->parent = new;
> >
> > -       if (parent->child == first) {
> > -               parent->child = new;
> > -       } else {
> > -               next = parent->child;
> > -               while (next->sibling != first)
> > -                       next = next->sibling;
> > -               next->sibling = new;
> > -       }
> >         return NULL;
> >  }
> >
> > @@ -968,19 +980,17 @@ static int __adjust_resource(struct resource *res, resource_size_t start,
> >         if ((start < parent->start) || (end > parent->end))
> >                 goto out;
> >
> > -       if (res->sibling && (res->sibling->start <= end))
> > +       if (sibling(res) && (sibling(res)->start <= end))
> >                 goto out;
> >
> > -       tmp = parent->child;
> > -       if (tmp != res) {
> > -               while (tmp->sibling != res)
> > -                       tmp = tmp->sibling;
> > +       if (res->sibling.prev != &parent->child) {
> > +               tmp = list_prev_entry(res, sibling);
> >                 if (start <= tmp->end)
> >                         goto out;
> >         }
> >
> >  skip:
> > -       for (tmp = res->child; tmp; tmp = tmp->sibling)
> > +       list_for_each_entry(tmp, &res->child, sibling)
> >                 if ((tmp->start < start) || (tmp->end > end))
> >                         goto out;
> >
> > @@ -1205,34 +1215,32 @@ EXPORT_SYMBOL(__request_region);
> >  void __release_region(struct resource *parent, resource_size_t start,
> >                         resource_size_t n)
> >  {
> > -       struct resource **p;
> > +       struct resource *res;
> >         resource_size_t end;
> >
> > -       p = &parent->child;
> > +       res = first_child(&parent->child);
> >         end = start + n - 1;
> >
> >         write_lock(&resource_lock);
> >
> >         for (;;) {
> > -               struct resource *res = *p;
> > -
> >                 if (!res)
> >                         break;
> >                 if (res->start <= start && res->end >= end) {
> >                         if (!(res->flags & IORESOURCE_BUSY)) {
> > -                               p = &res->child;
> > +                               res = first_child(&res->child);
> >                                 continue;
> >                         }
> >                         if (res->start != start || res->end != end)
> >                                 break;
> > -                       *p = res->sibling;
> > +                       list_del(&res->sibling);
> >                         write_unlock(&resource_lock);
> >                         if (res->flags & IORESOURCE_MUXED)
> >                                 wake_up(&muxed_resource_wait);
> >                         free_resource(res);
> >                         return;
> >                 }
> > -               p = &res->sibling;
> > +               res = sibling(res);
> >         }
> >
> >         write_unlock(&resource_lock);
> > @@ -1267,9 +1275,7 @@ EXPORT_SYMBOL(__release_region);
> >  int release_mem_region_adjustable(struct resource *parent,
> >                         resource_size_t start, resource_size_t size)
> >  {
> > -       struct resource **p;
> > -       struct resource *res;
> > -       struct resource *new_res;
> > +       struct resource *res, *new_res;
> >         resource_size_t end;
> >         int ret = -EINVAL;
> >
> > @@ -1280,16 +1286,16 @@ int release_mem_region_adjustable(struct resource *parent,
> >         /* The alloc_resource() result gets checked later */
> >         new_res = alloc_resource(GFP_KERNEL);
> >
> > -       p = &parent->child;
> > +       res = first_child(&parent->child);
> >         write_lock(&resource_lock);
> >
> > -       while ((res = *p)) {
> > +       while ((res)) {
> >                 if (res->start >= end)
> >                         break;
> >
> >                 /* look for the next resource if it does not fit into */
> >                 if (res->start > start || res->end < end) {
> > -                       p = &res->sibling;
> > +                       res = sibling(res);
> >                         continue;
> >                 }
> >
> > @@ -1297,14 +1303,14 @@ int release_mem_region_adjustable(struct resource *parent,
> >                         break;
> >
> >                 if (!(res->flags & IORESOURCE_BUSY)) {
> > -                       p = &res->child;
> > +                       res = first_child(&res->child);
> >                         continue;
> >                 }
> >
> >                 /* found the target resource; let's adjust accordingly */
> >                 if (res->start == start && res->end == end) {
> >                         /* free the whole entry */
> > -                       *p = res->sibling;
> > +                       list_del(&res->sibling);
> >                         free_resource(res);
> >                         ret = 0;
> >                 } else if (res->start == start && res->end != end) {
> > @@ -1327,14 +1333,13 @@ int release_mem_region_adjustable(struct resource *parent,
> >                         new_res->flags = res->flags;
> >                         new_res->desc = res->desc;
> >                         new_res->parent = res->parent;
> > -                       new_res->sibling = res->sibling;
> > -                       new_res->child = NULL;
> > +                       INIT_LIST_HEAD(&new_res->child);
> >
> >                         ret = __adjust_resource(res, res->start,
> >                                                 start - res->start);
> >                         if (ret)
> >                                 break;
> > -                       res->sibling = new_res;
> > +                       list_add(&new_res->sibling, &res->sibling);
> >                         new_res = NULL;
> >                 }
> >
> > @@ -1515,7 +1520,7 @@ static int __init reserve_setup(char *str)
> >                         res->end = io_start + io_num - 1;
> >                         res->flags |= IORESOURCE_BUSY;
> >                         res->desc = IORES_DESC_NONE;
> > -                       res->child = NULL;
> > +                       INIT_LIST_HEAD(&res->child);
> >                         if (request_resource(parent, res) == 0)
> >                                 reserved = x+1;
> >                 }
> > @@ -1535,7 +1540,7 @@ int iomem_map_sanity_check(resource_size_t addr, unsigned long size)
> >         loff_t l;
> >
> >         read_lock(&resource_lock);
> > -       for (p = p->child; p ; p = r_next(NULL, p, &l)) {
> > +       for (p = first_child(&p->child); p; p = r_next(NULL, p, &l)) {
> >                 /*
> >                  * We can probably skip the resources without
> >                  * IORESOURCE_IO attribute?
> > @@ -1591,7 +1596,7 @@ bool iomem_is_exclusive(u64 addr)
> >         addr = addr & PAGE_MASK;
> >
> >         read_lock(&resource_lock);
> > -       for (p = p->child; p ; p = r_next(NULL, p, &l)) {
> > +       for (p = first_child(&p->child); p; p = r_next(NULL, p, &l)) {
> >                 /*
> >                  * We can probably skip the resources without
> >                  * IORESOURCE_IO attribute?
> > --
> > 2.13.6
> >

^ permalink raw reply

* Re: [PATCH] Input: atmel_mxt_ts - add missing compatible strings to OF device table
From: Enric Balletbo Serra @ 2018-04-10 10:20 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: linux-kernel, devicetree@vger.kernel.org, Alexandre Belloni,
	Rob Herring, Dmitry Torokhov, linux-input, Nicolas Ferre,
	Nick Dyer, Mark Rutland, Linux ARM
In-Reply-To: <20180410095340.24074-1-javierm@redhat.com>

Thanks Javier!

2018-04-10 11:53 GMT+02:00 Javier Martinez Canillas <javierm@redhat.com>:
> Commit af503716ac14 ("i2c: core: report OF style module alias for devices
> registered via OF") fixed how the I2C core reports the module alias when
> devices are registered via OF.
>
> But the atmel_mxt_ts driver only has an "atmel,maxtouch" compatible in its
> OF device ID table, so if a Device Tree is using a different one, autoload
> won't be working for the module (the matching works because the I2C device
> ID table is used as a fallback).
>
> So add compatible strings for each of the entries in the I2C device table.
>
> Fixes: af503716ac14 ("i2c: core: report OF style module alias for devices registered via OF")
> Reported-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
>
>  Documentation/devicetree/bindings/input/atmel,maxtouch.txt | 6 +++++-
>  drivers/input/touchscreen/atmel_mxt_ts.c                   | 4 ++++
>  2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/input/atmel,maxtouch.txt b/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
> index 23e3abc3fdef..cd43fb8bc2ce 100644
> --- a/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
> +++ b/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
> @@ -1,8 +1,12 @@
>  Atmel maXTouch touchscreen/touchpad
>
>  Required properties:
> -- compatible:
> +- compatible: Must be one of the following
> +    atmel,qt602240_ts
> +    atmel,atmel_mxt_ts
> +    atmel,atmel_mxt_tp
>      atmel,maxtouch
> +    atmel,mXT224
>
>  - reg: The I2C address of the device
>
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index 7659bc48f1db..e8ef83f168d6 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -3238,7 +3238,11 @@ static int __maybe_unused mxt_resume(struct device *dev)
>  static SIMPLE_DEV_PM_OPS(mxt_pm_ops, mxt_suspend, mxt_resume);
>
>  static const struct of_device_id mxt_of_match[] = {
> +       { .compatible = "atmel,qt602240_ts", },
> +       { .compatible = "atmel,atmel_mxt_ts", },
> +       { .compatible = "atmel,atmel_mxt_tp", },
>         { .compatible = "atmel,maxtouch", },
> +       { .compatible = "atmel,mXT224", },
>         {},
>  };
>  MODULE_DEVICE_TABLE(of, mxt_of_match);
> --
> 2.14.3
>

The Samsung Chromebook Plus is one of the affected devices, with
current mainline the module is not autoloaded, this patch fixes the
issue.

Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

^ permalink raw reply


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