Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH v1 03/10] Input: atmel_mxt_ts - remove duplicate setup of ABS_MT_PRESSURE
From: Nick Dyer @ 2018-07-20 21:51 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-kernel, linux-input, Chris Healy, Nikita Yushchenko,
	Lucas Stach, Nick Dyer
In-Reply-To: <20180720215122.23558-1-nick@shmanahar.org>

From: Nick Dyer <nick.dyer@itdev.co.uk>

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 9555947a2d46..dcafb812ee7e 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -2055,12 +2055,6 @@ static int mxt_initialize_input_device(struct mxt_data *data)
 				     0, 255, 0, 0);
 	}
 
-	if (data->multitouch == MXT_TOUCH_MULTITOUCHSCREEN_T100 &&
-	    data->t100_aux_ampl) {
-		input_set_abs_params(input_dev, ABS_MT_PRESSURE,
-				     0, 255, 0, 0);
-	}
-
 	if (data->multitouch == MXT_TOUCH_MULTITOUCHSCREEN_T100 &&
 	    data->t100_aux_vect) {
 		input_set_abs_params(input_dev, ABS_MT_ORIENTATION,
-- 
2.17.1

^ permalink raw reply related

* [PATCH v1 02/10] Input: atmel_mxt_ts - use BIT() macro everywhere
From: Nick Dyer @ 2018-07-20 21:51 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-kernel, linux-input, Chris Healy, Nikita Yushchenko,
	Lucas Stach, Nick Dyer
In-Reply-To: <20180720215122.23558-1-nick@shmanahar.org>

From: Nick Dyer <nick.dyer@itdev.co.uk>

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 36 ++++++++++++------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 48c5ccab00a0..9555947a2d46 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -88,12 +88,12 @@
 #define MXT_COMMAND_DIAGNOSTIC	5
 
 /* Define for T6 status byte */
-#define MXT_T6_STATUS_RESET	(1 << 7)
-#define MXT_T6_STATUS_OFL	(1 << 6)
-#define MXT_T6_STATUS_SIGERR	(1 << 5)
-#define MXT_T6_STATUS_CAL	(1 << 4)
-#define MXT_T6_STATUS_CFGERR	(1 << 3)
-#define MXT_T6_STATUS_COMSERR	(1 << 2)
+#define MXT_T6_STATUS_RESET	BIT(7)
+#define MXT_T6_STATUS_OFL	BIT(6)
+#define MXT_T6_STATUS_SIGERR	BIT(5)
+#define MXT_T6_STATUS_CAL	BIT(4)
+#define MXT_T6_STATUS_CFGERR	BIT(3)
+#define MXT_T6_STATUS_COMSERR	BIT(2)
 
 /* MXT_GEN_POWER_T7 field */
 struct t7_config {
@@ -112,14 +112,14 @@ struct t7_config {
 #define MXT_T9_RANGE		18
 
 /* MXT_TOUCH_MULTI_T9 status */
-#define MXT_T9_UNGRIP		(1 << 0)
-#define MXT_T9_SUPPRESS		(1 << 1)
-#define MXT_T9_AMP		(1 << 2)
-#define MXT_T9_VECTOR		(1 << 3)
-#define MXT_T9_MOVE		(1 << 4)
-#define MXT_T9_RELEASE		(1 << 5)
-#define MXT_T9_PRESS		(1 << 6)
-#define MXT_T9_DETECT		(1 << 7)
+#define MXT_T9_UNGRIP		BIT(0)
+#define MXT_T9_SUPPRESS		BIT(1)
+#define MXT_T9_AMP		BIT(2)
+#define MXT_T9_VECTOR		BIT(3)
+#define MXT_T9_MOVE		BIT(4)
+#define MXT_T9_RELEASE		BIT(5)
+#define MXT_T9_PRESS		BIT(6)
+#define MXT_T9_DETECT		BIT(7)
 
 struct t9_range {
 	__le16 x;
@@ -127,9 +127,9 @@ struct t9_range {
 } __packed;
 
 /* MXT_TOUCH_MULTI_T9 orient */
-#define MXT_T9_ORIENT_SWITCH	(1 << 0)
-#define MXT_T9_ORIENT_INVERTX	(1 << 1)
-#define MXT_T9_ORIENT_INVERTY	(1 << 2)
+#define MXT_T9_ORIENT_SWITCH	BIT(0)
+#define MXT_T9_ORIENT_INVERTX	BIT(1)
+#define MXT_T9_ORIENT_INVERTY	BIT(2)
 
 /* MXT_SPT_COMMSCONFIG_T18 */
 #define MXT_COMMS_CTRL		0
@@ -214,7 +214,7 @@ enum t100_type {
 #define MXT_FRAME_CRC_PASS	0x04
 #define MXT_APP_CRC_FAIL	0x40	/* valid 7 8 bit only */
 #define MXT_BOOT_STATUS_MASK	0x3f
-#define MXT_BOOT_EXTENDED_ID	(1 << 5)
+#define MXT_BOOT_EXTENDED_ID	BIT(5)
 #define MXT_BOOT_ID_MASK	0x1f
 
 /* Touchscreen absolute values */
-- 
2.17.1

^ permalink raw reply related

* [PATCH v1 01/10] Input: atmel_mxt_ts - only use first T9 instance
From: Nick Dyer @ 2018-07-20 21:51 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-kernel, linux-input, Chris Healy, Nikita Yushchenko,
	Lucas Stach, Nick Dyer

From: Nick Dyer <nick.dyer@itdev.co.uk>

The driver only registers one input device, which uses the screen
parameters from the first T9 instance. The first T63 instance also uses
those parameters.

It is incorrect to send input reports from the second instances of these
objects if they are enabled: the input scaling will be wrong and the
positions will be mashed together.

This also causes problems on Android if the number of slots exceeds 32.

In the future, this could be handled by looking for enabled touch object
instances and creating an input device for each one.

Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk>
Acked-by: Benson Leung <bleung@chromium.org>
Acked-by: Yufeng Shen <miletus@chromium.org>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 54fe190fd4bc..48c5ccab00a0 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -1658,10 +1658,11 @@ static int mxt_parse_object_table(struct mxt_data *data,
 			break;
 		case MXT_TOUCH_MULTI_T9:
 			data->multitouch = MXT_TOUCH_MULTI_T9;
+			/* Only handle messages from first T9 instance */
 			data->T9_reportid_min = min_id;
-			data->T9_reportid_max = max_id;
-			data->num_touchids = object->num_report_ids
-						* mxt_obj_instances(object);
+			data->T9_reportid_max = min_id +
+						object->num_report_ids - 1;
+			data->num_touchids = object->num_report_ids;
 			break;
 		case MXT_SPT_MESSAGECOUNT_T44:
 			data->T44_address = object->start_address;
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH v3 1/3] Input: edt-ft5x06 - Add support for regulator
From: Rob Herring @ 2018-07-20 13:43 UTC (permalink / raw)
  To: Mylène Josserand
  Cc: dmitry.torokhov, mark.rutland, maxime.ripard, wens, linux-input,
	devicetree, linux-kernel, linux-arm-kernel, thomas.petazzoni
In-Reply-To: <20180718182719.29663-2-mylene.josserand@bootlin.com>

On Wed, Jul 18, 2018 at 08:27:17PM +0200, Mylène Josserand wrote:
> Add the support of regulator to use it as VCC source.
> 
> Signed-off-by: Mylène Josserand <mylene.josserand@bootlin.com>
> ---
>  .../bindings/input/touchscreen/edt-ft5x06.txt      |  1 +

Please add acks when posting new versions.

>  drivers/input/touchscreen/edt-ft5x06.c             | 29 ++++++++++++++++++++++
>  2 files changed, 30 insertions(+)

^ permalink raw reply

* [PATCH v2] drivers: input: Remove paragraphs about GPL due to SPDX-tag
From: Christian Thoms @ 2018-07-20 12:36 UTC (permalink / raw)
  To: dmitry.torokhov, rydberg, linux-input, linux-kernel
  Cc: Christian Thoms, linux-kernel, Sebastian Hillert

By adding the SPDX-License identifier, the paragraphs about the GPL are not
longer necessary as stated by Dmitry.

Signed-off-by: Christian Thoms <christian.thoms93@gmail.com>
Signed-off-by: Sebastian Hillert <sebastian.hillert1@gmail.com>
---
 drivers/input/evbug.c         | 15 ++-------------
 drivers/input/ff-core.c       | 11 -----------
 drivers/input/ff-memless.c    | 11 -----------
 drivers/input/matrix-keymap.c |  5 -----
 4 files changed, 2 insertions(+), 40 deletions(-)

diff --git a/drivers/input/evbug.c b/drivers/input/evbug.c
index 337c0ff..4de7431 100644
--- a/drivers/input/evbug.c
+++ b/drivers/input/evbug.c
@@ -1,22 +1,11 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
- *  Copyright (c) 1999-2001 Vojtech Pavlik
- */
-
-/*
  *  Input driver event debug module - dumps all events into syslog
+ *
+ *  Copyright (c) 1999-2001 Vojtech Pavlik
  */
 
 /*
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
- *
  * Should you need to contact me, the author, you can do so either by
  * e-mail - mail your message to <vojtech@ucw.cz>, or by paper mail:
  * Vojtech Pavlik, Simunkova 1594, Prague 8, 182 00 Czech Republic
diff --git a/drivers/input/ff-core.c b/drivers/input/ff-core.c
index 3086166..0a33737 100644
--- a/drivers/input/ff-core.c
+++ b/drivers/input/ff-core.c
@@ -6,17 +6,6 @@
  *  Copyright (c) 2006 Dmitry Torokhov <dtor@mail.ru>
  */
 
-/*
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
- */
-
 /* #define DEBUG */
 
 #include <linux/input.h>
diff --git a/drivers/input/ff-memless.c b/drivers/input/ff-memless.c
index f25377d..e16f121 100644
--- a/drivers/input/ff-memless.c
+++ b/drivers/input/ff-memless.c
@@ -6,17 +6,6 @@
  *  Copyright (c) 2006 Dmitry Torokhov <dtor@mail.ru>
  */
 
-/*
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
- */
-
 /* #define DEBUG */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
diff --git a/drivers/input/matrix-keymap.c b/drivers/input/matrix-keymap.c
index c2cc853..525675c 100644
--- a/drivers/input/matrix-keymap.c
+++ b/drivers/input/matrix-keymap.c
@@ -6,11 +6,6 @@
  *
  * Author:
  *	Olof Johansson <olof@lixom.net>
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
  */
 
 #include <linux/device.h>
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH v2 1/2] Input: atmel_mxt_ts: Add support for  optional regulators.
From: Nick Dyer @ 2018-07-19 20:54 UTC (permalink / raw)
  To: Paweł Chmiel
  Cc: dmitry.torokhov, robh+dt, mark.rutland, nicolas.ferre,
	alexandre.belloni, linux-input, devicetree, linux-arm-kernel,
	linux-kernel
In-Reply-To: <2353752.vfvcsHJLR6@acerlaptop>

On Wed, Jul 18, 2018 at 06:21:30PM +0200, Paweł Chmiel wrote:
> On Tuesday, July 17, 2018 10:00:05 PM CEST Nick Dyer wrote:
> > On Tue, Jul 17, 2018 at 08:16:25PM +0200, Paweł Chmiel wrote:
> > > This patch adds optional regulators, which can be used to power
> > > up touchscreen. After enabling regulators, we need to wait 150msec.
> > > This value is taken from official driver.
> > > 
> > > It was tested on Samsung Galaxy i9000 (based on Samsung S5PV210 SOC).
> > > 
> > > Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> > > ---
> > > Changes from v1:
> > >   - Enable regulators only if reset_gpio is present.
> > >   - Switch from devm_regulator_get_optional to devm_regulator_get
> > > ---
> > >  drivers/input/touchscreen/atmel_mxt_ts.c | 46 ++++++++++++++++++++++++++++++--
> > >  1 file changed, 44 insertions(+), 2 deletions(-)
> > 
> > Hi Pawel-
> > 
> > I see you've borrowed some of the logic from the patch I wrote a while
> > back (see https://github.com/ndyer/linux/commit/8e9687e41ed062 )
> Actually, i was looking at https://github.com/atmel-maxtouch/linux/blob/maxtouch-v3.14/drivers/input/touchscreen/atmel_mxt_ts.c (and didn't saw Your patch till now).
> Are You going to submit it? (it has more functionalities - for example
> suspend mode read from device tree).

Getting that work upstream has stalled for a couple of years because I
changed jobs. I have actually started recently to dust it off again, it
was later on in my queue but if you have the time to work on it that is
great.

> > The correct behaviour according to Atmel should be:
> > 
> > * Make RESET zero
> > * Bring up regulators
> > * Wait for a period to settle (150 msec)
> > * Release RESET
> > * Wait for 100 msec whilst device gets through bootloader
> > * Wait for CHG line assert before reading info block
> > 
> > I can't see the first and last steps in your patch at present.
> About first step - reset_gpio is readed by using
> devm_gpiod_get_optional with GPIOD_OUT_LOW flag, so i think (but might
> be wrong)  that we don't need to set this gpio value again to 0 before
> enabling regulators,

I see what you mean - that is fair enough.

> since currently only place where reset_gpio is used is in driver probe
> (in Your patch it is used in other cases/places - for example in
> mxt_start/stop, when we enable regulators).
> About missing wait after releasing reset, shouldn't this be separate
> patch (since currently driver is not doing it)? I can prepare it and
> send with other in next version.

According to the maxtouch documentation, it isn't ready for comms until
the firmware asserts the CHG line. I've seen a bunch of devices that get
by without an explicit wait because the board file does the power on,
and by the time the driver gets to probe it's a few hundred ms later
anyway, so it doesn't matter. But if we put it all in the driver, it
will attempt to read the info block straight after the 100 msec delay
without waiting for CHG, and I suspect we'll end up with occasional
probe failures. It'll depend on the maxtouch device, though: they have a
range of different power on timings.

Which platform are you doing this for? Is it a Chromebook?

> Thanks for feedback
> > 
> > The only downside with this approach is that there are a lot of
> > delays during driver probe, but I believe the asynchronous probe stuff
> > that's landed since I wrote the original patch should address that.
> > 
> > cheers
> > 
> > Nick
> > 
> > >  	}
> > > @@ -3116,6 +3154,10 @@ static int mxt_remove(struct i2c_client *client)
> > >  	struct mxt_data *data = i2c_get_clientdata(client);
> > >  
> > >  	disable_irq(data->irq);
> > > +	if (data->reset_gpio) {
> > > +		regulator_disable(data->avdd_reg);
> > > +		regulator_disable(data->vdd_reg);
> > > +	}
> > >  	sysfs_remove_group(&client->dev.kobj, &mxt_attr_group);
> > >  	mxt_free_input_device(data);
> > >  	mxt_free_object_table(data);
> > 
> 
> 

^ permalink raw reply

* Re: [PATCH v7 4/4] kexec_file: Load kernel at top of system RAM if required
From: Andrew Morton @ 2018-07-19 19:44 UTC (permalink / raw)
  To: Baoquan He
  Cc: nicolas.pitre-QSEj5FYQhm4dnm+yROfE0A, brijesh.singh-5C7GfCeVMHo,
	devicetree-u79uwXL29TY76Z2rM5mHXA, airlied-cv59FeDIM0c,
	linux-pci-u79uwXL29TY76Z2rM5mHXA,
	richard.weiyang-Re5JQEeQqe8AvxtiuMwx3w,
	jcmvbkbc-Re5JQEeQqe8AvxtiuMwx3w,
	baiyaowei-0p4V/sDNsUmm0O/7XYngnFaTQe2KTcn/,
	kys-0li6OtcxBFHby3iVrkZq2A, frowand.list-Re5JQEeQqe8AvxtiuMwx3w,
	tglx-hfZtesqFncYOwBW4kG4KsQ, lorenzo.pieralisi-5wv7dgnIgG8,
	sthemmin-0li6OtcxBFHby3iVrkZq2A,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw,
	patrik.r.jakobsson-Re5JQEeQqe8AvxtiuMwx3w,
	andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	gustavo-THi1TnShQwVAfugRpC6u6w, bp-l3A5Bk7waGM,
	dyoung-H+wXaHxf7aLQT0dZR+AlfA, thomas.lendacky-5C7GfCeVMHo,
	haiyangz-0li6OtcxBFHby3iVrkZq2A,
	maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA,
	josh-iaAMLnmF4UmaiuxdJuQwMA, jglisse-H+wXaHxf7aLQT0dZR+AlfA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, seanpaul-F7+t8E8rja9g9hUCZPvPmw,
	bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, yinghai-DgEjT+Ai2ygdnm+yROfE0A,
	jonathan.derrick-ral2JQCrhuEAvxtiuMwx3w,
	chris-YvXeqwSYzG2sTnJN9+BGXg, monstr-pSz03upnqPeHXe+LvDLADg,
	linux-parisc-u79uwXL29TY76Z2rM5mHXA,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
	kexec-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel
In-Reply-To: <20180719151753.GB7147-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>

On Thu, 19 Jul 2018 23:17:53 +0800 Baoquan He <bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> Hi Andrew,
> 
> On 07/18/18 at 03:33pm, Andrew Morton wrote:
> > On Wed, 18 Jul 2018 10:49:44 +0800 Baoquan He <bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > 
> > > For kexec_file loading, if kexec_buf.top_down is 'true', the memory which
> > > is used to load kernel/initrd/purgatory is supposed to be allocated from
> > > top to down. This is what we have been doing all along in the old kexec
> > > loading interface and the kexec loading is still default setting in some
> > > distributions. However, the current kexec_file loading interface doesn't
> > > do like this. The function arch_kexec_walk_mem() it calls ignores checking
> > > kexec_buf.top_down, but calls walk_system_ram_res() directly to go through
> > > all resources of System RAM from bottom to up, to try to find memory region
> > > which can contain the specific kexec buffer, then call locate_mem_hole_callback()
> > > to allocate memory in that found memory region from top to down. This brings
> > > confusion especially when KASLR is widely supported , users have to make clear
> > > why kexec/kdump kernel loading position is different between these two
> > > interfaces in order to exclude unnecessary noises. Hence these two interfaces
> > > need be unified on behaviour.
> > 
> > As far as I can tell, the above is the whole reason for the patchset,
> > yes?  To avoid confusing users.
> 
> 
> In fact, it's not just trying to avoid confusing users. Kexec loading
> and kexec_file loading are just do the same thing in essence. Just we
> need do kernel image verification on uefi system, have to port kexec
> loading code to kernel. 
> 
> Kexec has been a formal feature in our distro, and customers owning
> those kind of very large machine can make use of this feature to speed
> up the reboot process. On uefi machine, the kexec_file loading will
> search place to put kernel under 4G from top to down. As we know, the
> 1st 4G space is DMA32 ZONE, dma, pci mmcfg, bios etc all try to consume
> it. It may have possibility to not be able to find a usable space for
> kernel/initrd. From the top down of the whole memory space, we don't
> have this worry. 
> 
> And at the first post, I just posted below with AKASHI's
> walk_system_ram_res_rev() version. Later you suggested to use
> list_head to link child sibling of resource, see what the code change
> looks like.
> http://lkml.kernel.org/r/20180322033722.9279-1-bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
> 
> Then I posted v2
> http://lkml.kernel.org/r/20180408024724.16812-1-bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
> Rob Herring mentioned that other components which has this tree struct
> have planned to do the same thing, replacing the singly linked list with
> list_head to link resource child sibling. Just quote Rob's words as
> below. I think this could be another reason.
> 
> ~~~~~ From Rob
> 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.

Please let's get all this into the changelogs?

> > 
> > Is that sufficient?  Can we instead simplify their lives by providing
> > better documentation or informative printks or better Kconfig text,
> > etc?
> > 
> > And who *are* the people who are performing this configuration?  Random
> > system administrators?  Linux distro engineers?  If the latter then
> > they presumably aren't easily confused!
> 
> Kexec was invented for kernel developer to speed up their kernel
> rebooting. Now high end sever admin, kernel developer and QE are also
> keen to use it to reboot large box for faster feature testing, bug
> debugging. Kernel dev could know this well, about kernel loading
> position, admin or QE might not be aware of it very well. 
> 
> > 
> > In other words, I'm trying to understand how much benefit this patchset
> > will provide to our users as a whole.
> 
> Understood. The list_head replacing patch truly involes too many code
> changes, it's risky. I am willing to try any idea from reviewers, won't
> persuit they have to be accepted finally. If don't have a try, we don't
> know what it looks like, and what impact it may have. I am fine to take
> AKASHI's simple version of walk_system_ram_res_rev() to lower risk, even
> though it could be a little bit low efficient.

The larger patch produces a better result.  We can handle it ;)

^ permalink raw reply

* Re: [PATCH v7 1/4] resource: Move reparent_resources() to kernel/resource.c and make it public
From: Baoquan He @ 2018-07-19 15:18 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Nicolas Pitre, brijesh.singh-5C7GfCeVMHo, devicetree,
	David Airlie, linux-pci-u79uwXL29TY76Z2rM5mHXA,
	richard.weiyang-Re5JQEeQqe8AvxtiuMwx3w, Max Filippov,
	Paul Mackerras, baiyaowei-0p4V/sDNsUmm0O/7XYngnFaTQe2KTcn/,
	KY Srinivasan, Frank Rowand, Lorenzo Pieralisi, Stephen Hemminger,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Michael Ellerman,
	Patrik Jakobsson, linux-input, Gustavo Padovan, Borislav Petkov,
	Dave Young, Tom Lendacky, Haiyang Zhang, Maarten Lankhorst,
	Josh Triplett
In-Reply-To: <CAHp75Vf2yEwHhEhhQH2XN+pOQ=-skiAHZ=FgLnfVV8vcm59qeQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On 07/18/18 at 07:37pm, Andy Shevchenko wrote:
> On Wed, Jul 18, 2018 at 7:36 PM, Andy Shevchenko
> <andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > On Wed, Jul 18, 2018 at 5:49 AM, Baoquan He <bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> >> reparent_resources() is duplicated in arch/microblaze/pci/pci-common.c
> >> and arch/powerpc/kernel/pci-common.c, so move it to kernel/resource.c
> >> so that it's shared.
> 
> >> + * Returns 0 on success, -ENOTSUPP if child resource is not completely
> >> + * contained by 'res', -ECANCELED if no any conflicting entry found.
> 
> You also can refer to constants by prefixing them with %, e.g. %-ENOTSUPP.
> But this is up to you completely.

Thanks, will fix when repost. 

^ permalink raw reply

* Re: [PATCH v7 4/4] kexec_file: Load kernel at top of system RAM if required
From: Baoquan He @ 2018-07-19 15:17 UTC (permalink / raw)
  To: Andrew Morton
  Cc: nicolas.pitre-QSEj5FYQhm4dnm+yROfE0A, brijesh.singh-5C7GfCeVMHo,
	devicetree-u79uwXL29TY76Z2rM5mHXA, airlied-cv59FeDIM0c,
	linux-pci-u79uwXL29TY76Z2rM5mHXA,
	richard.weiyang-Re5JQEeQqe8AvxtiuMwx3w,
	jcmvbkbc-Re5JQEeQqe8AvxtiuMwx3w,
	baiyaowei-0p4V/sDNsUmm0O/7XYngnFaTQe2KTcn/,
	kys-0li6OtcxBFHby3iVrkZq2A, frowand.list-Re5JQEeQqe8AvxtiuMwx3w,
	tglx-hfZtesqFncYOwBW4kG4KsQ, lorenzo.pieralisi-5wv7dgnIgG8,
	sthemmin-0li6OtcxBFHby3iVrkZq2A,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw,
	patrik.r.jakobsson-Re5JQEeQqe8AvxtiuMwx3w,
	andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	gustavo-THi1TnShQwVAfugRpC6u6w, bp-l3A5Bk7waGM,
	dyoung-H+wXaHxf7aLQT0dZR+AlfA, thomas.lendacky-5C7GfCeVMHo,
	haiyangz-0li6OtcxBFHby3iVrkZq2A,
	maarten.lankhorst-VuQAYsv1563Yd54FQh9/CA,
	josh-iaAMLnmF4UmaiuxdJuQwMA, jglisse-H+wXaHxf7aLQT0dZR+AlfA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, seanpaul-F7+t8E8rja9g9hUCZPvPmw,
	bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, yinghai-DgEjT+Ai2ygdnm+yROfE0A,
	jonathan.derrick-ral2JQCrhuEAvxtiuMwx3w,
	chris-YvXeqwSYzG2sTnJN9+BGXg, monstr-pSz03upnqPeHXe+LvDLADg,
	linux-parisc-u79uwXL29TY76Z2rM5mHXA,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
	kexec-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel
In-Reply-To: <20180718153326.b795e9ea7835432a56cd7011-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>

Hi Andrew,

On 07/18/18 at 03:33pm, Andrew Morton wrote:
> On Wed, 18 Jul 2018 10:49:44 +0800 Baoquan He <bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> 
> > For kexec_file loading, if kexec_buf.top_down is 'true', the memory which
> > is used to load kernel/initrd/purgatory is supposed to be allocated from
> > top to down. This is what we have been doing all along in the old kexec
> > loading interface and the kexec loading is still default setting in some
> > distributions. However, the current kexec_file loading interface doesn't
> > do like this. The function arch_kexec_walk_mem() it calls ignores checking
> > kexec_buf.top_down, but calls walk_system_ram_res() directly to go through
> > all resources of System RAM from bottom to up, to try to find memory region
> > which can contain the specific kexec buffer, then call locate_mem_hole_callback()
> > to allocate memory in that found memory region from top to down. This brings
> > confusion especially when KASLR is widely supported , users have to make clear
> > why kexec/kdump kernel loading position is different between these two
> > interfaces in order to exclude unnecessary noises. Hence these two interfaces
> > need be unified on behaviour.
> 
> As far as I can tell, the above is the whole reason for the patchset,
> yes?  To avoid confusing users.


In fact, it's not just trying to avoid confusing users. Kexec loading
and kexec_file loading are just do the same thing in essence. Just we
need do kernel image verification on uefi system, have to port kexec
loading code to kernel. 

Kexec has been a formal feature in our distro, and customers owning
those kind of very large machine can make use of this feature to speed
up the reboot process. On uefi machine, the kexec_file loading will
search place to put kernel under 4G from top to down. As we know, the
1st 4G space is DMA32 ZONE, dma, pci mmcfg, bios etc all try to consume
it. It may have possibility to not be able to find a usable space for
kernel/initrd. From the top down of the whole memory space, we don't
have this worry. 

And at the first post, I just posted below with AKASHI's
walk_system_ram_res_rev() version. Later you suggested to use
list_head to link child sibling of resource, see what the code change
looks like.
http://lkml.kernel.org/r/20180322033722.9279-1-bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org

Then I posted v2
http://lkml.kernel.org/r/20180408024724.16812-1-bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
Rob Herring mentioned that other components which has this tree struct
have planned to do the same thing, replacing the singly linked list with
list_head to link resource child sibling. Just quote Rob's words as
below. I think this could be another reason.

~~~~~ From Rob
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.

> 
> Is that sufficient?  Can we instead simplify their lives by providing
> better documentation or informative printks or better Kconfig text,
> etc?
> 
> And who *are* the people who are performing this configuration?  Random
> system administrators?  Linux distro engineers?  If the latter then
> they presumably aren't easily confused!

Kexec was invented for kernel developer to speed up their kernel
rebooting. Now high end sever admin, kernel developer and QE are also
keen to use it to reboot large box for faster feature testing, bug
debugging. Kernel dev could know this well, about kernel loading
position, admin or QE might not be aware of it very well. 

> 
> In other words, I'm trying to understand how much benefit this patchset
> will provide to our users as a whole.

Understood. The list_head replacing patch truly involes too many code
changes, it's risky. I am willing to try any idea from reviewers, won't
persuit they have to be accepted finally. If don't have a try, we don't
know what it looks like, and what impact it may have. I am fine to take
AKASHI's simple version of walk_system_ram_res_rev() to lower risk, even
though it could be a little bit low efficient.

Thanks
Baoquan

^ permalink raw reply

* Re: [PATCH v2] HID: intel-ish-hid: Prevent loading of driver on Mehlow
From: Benjamin Tissoires @ 2018-07-19  9:39 UTC (permalink / raw)
  To: Srinivas Pandruvada; +Cc: Jiri Kosina, open list:HID CORE LAYER, lkml
In-Reply-To: <20180718193200.17944-1-srinivas.pandruvada@linux.intel.com>

On Wed, Jul 18, 2018 at 9:32 PM Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
>
> On Mehlow Xeon-E workstation, ISH PCI device is enabled but without ISH
> firmware. Here the ISH device PCI device id was reused for some non Linux
> storage drivers. So this was not done for enabling ISH. But this has a
> undesirable side effect for Linux.
>
> Here the ISH driver will be loaded via PCI enumeration and will try to do
> reset sequence. But reset sequence will wait till timeout as there is no
> real ISH firmware is present to take action. This delay will add to boot
> time of Linux (This platform will still continue to boot after this
> timeout).
>
> To avoid this boot delay we need to prevent loading of ISH drivers on
> this platform. So we need to have hack to avoid treating this device as
> ISH on this platform. To identify this workstation, we need some runtime
> method. Luckily there are special PCI id on this workstation to
> distinguish from the client version of this platform. On client version,
> the ISH is supported using same PCI device id. So this change look for
> the presence of PCI device IDs A309 and A30A and exit.
>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
> v2
>  Replaced pci_get_device with pci_dev_present to check in a loop as
> suggested by Benjamin.

That's even better than what I suggested :)

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

Cheers,
Benjamin

>
>  drivers/hid/intel-ish-hid/ipc/pci-ish.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> index a2c53ea3b5ed..4a55eab39b88 100644
> --- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> +++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> @@ -95,6 +95,13 @@ static int ish_init(struct ishtp_device *dev)
>         return 0;
>  }
>
> +static const struct pci_device_id ish_invalid_pci_ids[] = {
> +       /* Mehlow platform special pci ids */
> +       {PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0xA309)},
> +       {PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0xA30A)},
> +       {}
> +};
> +
>  /**
>   * ish_probe() - PCI driver probe callback
>   * @pdev:      pci device
> @@ -110,6 +117,10 @@ static int ish_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>         struct ish_hw *hw;
>         int     ret;
>
> +       /* Check for invalid platforms for ISH support */
> +       if (pci_dev_present(ish_invalid_pci_ids))
> +               return -ENODEV;
> +
>         /* enable pci dev */
>         ret = pci_enable_device(pdev);
>         if (ret) {
> --
> 2.17.1
>

^ permalink raw reply

* Re: [PATCH v3 1/3] Input: edt-ft5x06 - Add support for regulator
From: Ondřej Jirman @ 2018-07-18 22:46 UTC (permalink / raw)
  To: Mylène Josserand
  Cc: dmitry.torokhov, robh+dt, mark.rutland, maxime.ripard, wens,
	devicetree, linux-kernel, thomas.petazzoni, linux-input,
	linux-arm-kernel
In-Reply-To: <20180718182719.29663-2-mylene.josserand@bootlin.com>

Hello Mylène,

On Wed, Jul 18, 2018 at 08:27:17PM +0200, Mylène Josserand wrote:
> Add the support of regulator to use it as VCC source.
> 
> Signed-off-by: Mylène Josserand <mylene.josserand@bootlin.com>
> ---
>  .../bindings/input/touchscreen/edt-ft5x06.txt      |  1 +
>  drivers/input/touchscreen/edt-ft5x06.c             | 29 ++++++++++++++++++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> index 025cf8c9324a..48e975b9c1aa 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> @@ -30,6 +30,7 @@ Required properties:
>  Optional properties:
>   - reset-gpios: GPIO specification for the RESET input
>   - wake-gpios:  GPIO specification for the WAKE input
> + - vcc-supply:  Regulator that supplies the touchscreen
>  
>   - pinctrl-names: should be "default"
>   - pinctrl-0:   a phandle pointing to the pin settings for the
> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> index 1e18ca0d1b4e..aa94494b06b5 100644
> --- a/drivers/input/touchscreen/edt-ft5x06.c
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
> @@ -39,6 +39,7 @@
>  #include <linux/input/mt.h>
>  #include <linux/input/touchscreen.h>
>  #include <linux/of_device.h>
> +#include <linux/regulator/consumer.h>
>  
>  #define WORK_REGISTER_THRESHOLD		0x00
>  #define WORK_REGISTER_REPORT_RATE	0x08
> @@ -91,6 +92,7 @@ struct edt_ft5x06_ts_data {
>  	struct touchscreen_properties prop;
>  	u16 num_x;
>  	u16 num_y;
> +	struct regulator *vcc;
>  
>  	struct gpio_desc *reset_gpio;
>  	struct gpio_desc *wake_gpio;
> @@ -991,6 +993,22 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
>  
>  	tsdata->max_support_points = chip_data->max_support_points;
>  
> +	tsdata->vcc = devm_regulator_get(&client->dev, "vcc");
> +	if (IS_ERR(tsdata->vcc)) {
> +		error = PTR_ERR(tsdata->vcc);
> +		if (error != -EPROBE_DEFER)
> +			dev_err(&client->dev, "failed to request regulator: %d\n",
> +				error);
> +		return error;
> +	}
> +
> +	error = regulator_enable(tsdata->vcc);
> +	if (error < 0) {
> +		dev_err(&client->dev, "failed to enable vcc: %d\n",
> +			error);
> +		return error;
> +	}
> +
>  	tsdata->reset_gpio = devm_gpiod_get_optional(&client->dev,
>  						     "reset", GPIOD_OUT_HIGH);
>  	if (IS_ERR(tsdata->reset_gpio)) {
> @@ -1120,20 +1138,31 @@ static int edt_ft5x06_ts_remove(struct i2c_client *client)
>  static int __maybe_unused edt_ft5x06_ts_suspend(struct device *dev)
>  {
>  	struct i2c_client *client = to_i2c_client(dev);
> +	struct edt_ft5x06_ts_data *tsdata = i2c_get_clientdata(client);
>  
>  	if (device_may_wakeup(dev))
>  		enable_irq_wake(client->irq);
>  
> +	regulator_disable(tsdata->vcc);
> +

How will the touchscreen wakeup the system with interrupt if you power it off
on suspend? Perhaps guard this with device_may_wakeup() too? 

>  	return 0;
>  }
>  
>  static int __maybe_unused edt_ft5x06_ts_resume(struct device *dev)
>  {
>  	struct i2c_client *client = to_i2c_client(dev);
> +	struct edt_ft5x06_ts_data *tsdata = i2c_get_clientdata(client);
> +	int ret;
>  
>  	if (device_may_wakeup(dev))
>  		disable_irq_wake(client->irq);
>  
> +	ret = regulator_enable(tsdata->vcc);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to enable vcc: %d\n", ret);
> +		return ret;
> +	}
> +

Ditto.

Regards,
  o.

>  	return 0;
>  }
>  
> -- 
> 2.11.0
> 
> 
> _______________________________________________
> 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 v7 4/4] kexec_file: Load kernel at top of system RAM if required
From: Andrew Morton @ 2018-07-18 22:33 UTC (permalink / raw)
  To: Baoquan He
  Cc: linux-kernel, robh+dt, dan.j.williams, nicolas.pitre, josh,
	fengguang.wu, bp, andy.shevchenko, patrik.r.jakobsson, airlied,
	kys, haiyangz, sthemmin, dmitry.torokhov, frowand.list,
	keith.busch, jonathan.derrick, lorenzo.pieralisi, bhelgaas, tglx,
	brijesh.singh, jglisse, thomas.lendacky, gregkh, baiyaowei,
	richard.weiyang, devel, linux-input, linux-nvdimm, devicetree,
	linux-pci, ebiederm
In-Reply-To: <20180718024944.577-5-bhe@redhat.com>

On Wed, 18 Jul 2018 10:49:44 +0800 Baoquan He <bhe@redhat.com> wrote:

> For kexec_file loading, if kexec_buf.top_down is 'true', the memory which
> is used to load kernel/initrd/purgatory is supposed to be allocated from
> top to down. This is what we have been doing all along in the old kexec
> loading interface and the kexec loading is still default setting in some
> distributions. However, the current kexec_file loading interface doesn't
> do like this. The function arch_kexec_walk_mem() it calls ignores checking
> kexec_buf.top_down, but calls walk_system_ram_res() directly to go through
> all resources of System RAM from bottom to up, to try to find memory region
> which can contain the specific kexec buffer, then call locate_mem_hole_callback()
> to allocate memory in that found memory region from top to down. This brings
> confusion especially when KASLR is widely supported , users have to make clear
> why kexec/kdump kernel loading position is different between these two
> interfaces in order to exclude unnecessary noises. Hence these two interfaces
> need be unified on behaviour.

As far as I can tell, the above is the whole reason for the patchset,
yes?  To avoid confusing users.

Is that sufficient?  Can we instead simplify their lives by providing
better documentation or informative printks or better Kconfig text,
etc?

And who *are* the people who are performing this configuration?  Random
system administrators?  Linux distro engineers?  If the latter then
they presumably aren't easily confused!

In other words, I'm trying to understand how much benefit this patchset
will provide to our users as a whole.

^ permalink raw reply

* [PATCH v2] HID: intel-ish-hid: Prevent loading of driver on Mehlow
From: Srinivas Pandruvada @ 2018-07-18 19:32 UTC (permalink / raw)
  To: jikos; +Cc: benjamin.tissoires, linux-input, linux-kernel,
	Srinivas Pandruvada

On Mehlow Xeon-E workstation, ISH PCI device is enabled but without ISH
firmware. Here the ISH device PCI device id was reused for some non Linux
storage drivers. So this was not done for enabling ISH. But this has a
undesirable side effect for Linux.

Here the ISH driver will be loaded via PCI enumeration and will try to do
reset sequence. But reset sequence will wait till timeout as there is no
real ISH firmware is present to take action. This delay will add to boot
time of Linux (This platform will still continue to boot after this
timeout).

To avoid this boot delay we need to prevent loading of ISH drivers on
this platform. So we need to have hack to avoid treating this device as
ISH on this platform. To identify this workstation, we need some runtime
method. Luckily there are special PCI id on this workstation to
distinguish from the client version of this platform. On client version,
the ISH is supported using same PCI device id. So this change look for
the presence of PCI device IDs A309 and A30A and exit.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
v2
 Replaced pci_get_device with pci_dev_present to check in a loop as
suggested by Benjamin.

 drivers/hid/intel-ish-hid/ipc/pci-ish.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
index a2c53ea3b5ed..4a55eab39b88 100644
--- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
+++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
@@ -95,6 +95,13 @@ static int ish_init(struct ishtp_device *dev)
 	return 0;
 }
 
+static const struct pci_device_id ish_invalid_pci_ids[] = {
+	/* Mehlow platform special pci ids */
+	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0xA309)},
+	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0xA30A)},
+	{}
+};
+
 /**
  * ish_probe() - PCI driver probe callback
  * @pdev:	pci device
@@ -110,6 +117,10 @@ static int ish_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	struct ish_hw *hw;
 	int	ret;
 
+	/* Check for invalid platforms for ISH support */
+	if (pci_dev_present(ish_invalid_pci_ids))
+		return -ENODEV;
+
 	/* enable pci dev */
 	ret = pci_enable_device(pdev);
 	if (ret) {
-- 
2.17.1

^ permalink raw reply related

* [PATCH v3 3/3] arm: dts: sun8i: a83t: a711: Add touchscreen node
From: Mylène Josserand @ 2018-07-18 18:27 UTC (permalink / raw)
  To: dmitry.torokhov, robh+dt, mark.rutland, maxime.ripard, wens
  Cc: linux-input, devicetree, linux-kernel, linux-arm-kernel,
	mylene.josserand, thomas.petazzoni
In-Reply-To: <20180718182719.29663-1-mylene.josserand@bootlin.com>

Tha A711 tablet has a FocalTech EDT-FT5x06 Polytouch touchscreen.
It is connected via I2C0. The reset line is PD5, the interrupt
line is PL7 and the VCC supply is the ldo_io0 regulator.

Signed-off-by: Mylène Josserand <mylene.josserand@bootlin.com>
---
 arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts b/arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts
index 1537ce148cc1..dc7b94a6c068 100644
--- a/arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts
+++ b/arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts
@@ -156,6 +156,22 @@
 	status = "okay";
 };
 
+&i2c0 {
+	clock-frequency = <400000>;
+	status = "okay";
+
+	touchscreen@38 {
+		compatible = "edt,edt-ft5x06";
+		reg = <0x38>;
+		interrupt-parent = <&r_pio>;
+		interrupts = <0 7 IRQ_TYPE_EDGE_FALLING>;
+		reset-gpios = <&pio 3 5 GPIO_ACTIVE_LOW>;
+		vcc-supply = <&reg_ldo_io0>;
+		touchscreen-size-x = <1024>;
+		touchscreen-size-y = <600>;
+	};
+};
+
 &mmc0 {
 	vmmc-supply = <&reg_dcdc1>;
 	pinctrl-names = "default";
-- 
2.11.0

^ permalink raw reply related

* [PATCH v3 2/3] Input: edt-ft5x06 - Set wake/reset values on resume/suspend
From: Mylène Josserand @ 2018-07-18 18:27 UTC (permalink / raw)
  To: dmitry.torokhov, robh+dt, mark.rutland, maxime.ripard, wens
  Cc: linux-input, devicetree, linux-kernel, linux-arm-kernel,
	mylene.josserand, thomas.petazzoni
In-Reply-To: <20180718182719.29663-1-mylene.josserand@bootlin.com>

On resume and suspend, set the value of wake and reset gpios
to be sure that we are in a know state after suspending/resuming.

Signed-off-by: Mylène Josserand <mylene.josserand@bootlin.com>
---
 drivers/input/touchscreen/edt-ft5x06.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index aa94494b06b5..b013d97006e6 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -1143,6 +1143,12 @@ static int __maybe_unused edt_ft5x06_ts_suspend(struct device *dev)
 	if (device_may_wakeup(dev))
 		enable_irq_wake(client->irq);
 
+	if (tsdata->wake_gpio)
+		gpiod_set_value(tsdata->wake_gpio, 0);
+
+	if (tsdata->reset_gpio)
+		gpiod_set_value(tsdata->reset_gpio, 1);
+
 	regulator_disable(tsdata->vcc);
 
 	return 0;
@@ -1157,6 +1163,12 @@ static int __maybe_unused edt_ft5x06_ts_resume(struct device *dev)
 	if (device_may_wakeup(dev))
 		disable_irq_wake(client->irq);
 
+	if (tsdata->wake_gpio)
+		gpiod_set_value(tsdata->wake_gpio, 1);
+
+	if (tsdata->reset_gpio)
+		gpiod_set_value(tsdata->reset_gpio, 0);
+
 	ret = regulator_enable(tsdata->vcc);
 	if (ret < 0) {
 		dev_err(dev, "failed to enable vcc: %d\n", ret);
-- 
2.11.0

^ permalink raw reply related

* [PATCH v3 1/3] Input: edt-ft5x06 - Add support for regulator
From: Mylène Josserand @ 2018-07-18 18:27 UTC (permalink / raw)
  To: dmitry.torokhov, robh+dt, mark.rutland, maxime.ripard, wens
  Cc: linux-input, devicetree, linux-kernel, linux-arm-kernel,
	mylene.josserand, thomas.petazzoni
In-Reply-To: <20180718182719.29663-1-mylene.josserand@bootlin.com>

Add the support of regulator to use it as VCC source.

Signed-off-by: Mylène Josserand <mylene.josserand@bootlin.com>
---
 .../bindings/input/touchscreen/edt-ft5x06.txt      |  1 +
 drivers/input/touchscreen/edt-ft5x06.c             | 29 ++++++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
index 025cf8c9324a..48e975b9c1aa 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
@@ -30,6 +30,7 @@ Required properties:
 Optional properties:
  - reset-gpios: GPIO specification for the RESET input
  - wake-gpios:  GPIO specification for the WAKE input
+ - vcc-supply:  Regulator that supplies the touchscreen
 
  - pinctrl-names: should be "default"
  - pinctrl-0:   a phandle pointing to the pin settings for the
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index 1e18ca0d1b4e..aa94494b06b5 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -39,6 +39,7 @@
 #include <linux/input/mt.h>
 #include <linux/input/touchscreen.h>
 #include <linux/of_device.h>
+#include <linux/regulator/consumer.h>
 
 #define WORK_REGISTER_THRESHOLD		0x00
 #define WORK_REGISTER_REPORT_RATE	0x08
@@ -91,6 +92,7 @@ struct edt_ft5x06_ts_data {
 	struct touchscreen_properties prop;
 	u16 num_x;
 	u16 num_y;
+	struct regulator *vcc;
 
 	struct gpio_desc *reset_gpio;
 	struct gpio_desc *wake_gpio;
@@ -991,6 +993,22 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
 
 	tsdata->max_support_points = chip_data->max_support_points;
 
+	tsdata->vcc = devm_regulator_get(&client->dev, "vcc");
+	if (IS_ERR(tsdata->vcc)) {
+		error = PTR_ERR(tsdata->vcc);
+		if (error != -EPROBE_DEFER)
+			dev_err(&client->dev, "failed to request regulator: %d\n",
+				error);
+		return error;
+	}
+
+	error = regulator_enable(tsdata->vcc);
+	if (error < 0) {
+		dev_err(&client->dev, "failed to enable vcc: %d\n",
+			error);
+		return error;
+	}
+
 	tsdata->reset_gpio = devm_gpiod_get_optional(&client->dev,
 						     "reset", GPIOD_OUT_HIGH);
 	if (IS_ERR(tsdata->reset_gpio)) {
@@ -1120,20 +1138,31 @@ static int edt_ft5x06_ts_remove(struct i2c_client *client)
 static int __maybe_unused edt_ft5x06_ts_suspend(struct device *dev)
 {
 	struct i2c_client *client = to_i2c_client(dev);
+	struct edt_ft5x06_ts_data *tsdata = i2c_get_clientdata(client);
 
 	if (device_may_wakeup(dev))
 		enable_irq_wake(client->irq);
 
+	regulator_disable(tsdata->vcc);
+
 	return 0;
 }
 
 static int __maybe_unused edt_ft5x06_ts_resume(struct device *dev)
 {
 	struct i2c_client *client = to_i2c_client(dev);
+	struct edt_ft5x06_ts_data *tsdata = i2c_get_clientdata(client);
+	int ret;
 
 	if (device_may_wakeup(dev))
 		disable_irq_wake(client->irq);
 
+	ret = regulator_enable(tsdata->vcc);
+	if (ret < 0) {
+		dev_err(dev, "failed to enable vcc: %d\n", ret);
+		return ret;
+	}
+
 	return 0;
 }
 
-- 
2.11.0

^ permalink raw reply related

* [PATCH v3 0/3] sun8i-a83t: Add touchscreen support on TBS A711
From: Mylène Josserand @ 2018-07-18 18:27 UTC (permalink / raw)
  To: dmitry.torokhov, robh+dt, mark.rutland, maxime.ripard, wens
  Cc: linux-input, devicetree, linux-kernel, linux-arm-kernel,
	mylene.josserand, thomas.petazzoni

Hello everyone,

This is a V3 of the patch series that adds touchscreen support
(FocalTech EDT-FT5x06 Polytouch) for TBS A711 (Allwinner sun8i-a83t SoC).
Based on last master of linux-input tree.

Since I can't test the suspend/resume functions there is no updates in
this series about factory/normal mode.

Changes since v2:
   - Remove the check if regulator is NULL (Dmitry Torokhov's review)
   - Add EPROBE_DEFER error not to print a error message (Lothar Waßmann's review)
   - Add set wake/reset on suspend/resume.
Changes since v1:
   - Remove patches 01 and 02 as Chen-Yu Tsai sent a similar patch:
   https://patchwork.kernel.org/patch/10111431/
   and it is merged on last next-20171222.
   (See commit f066f46ce5a5 "ARM: dts: sun8i: a83t: Add I2C device nodes and pinmux settings")
   - Update regulator according to Dmitry Torokhov's review: remove "optional"
   suffix while retrieving the regulator, rename it into "vcc" instead of
   "power" and add bindings documentation.
   - Update device tree according to Maxime Ripard's review: remove the
   label and rename the node.
   - Squash patch 03 with patch 05 to add I2C0 and touchscreen's node
   in one patch (see patch 02).

Patch 01: Add support for regulator in the FocalTech touchscreen driver
because A711 tablet is using a regulator to power-up the touchscreen.
Patch 02: Add a set wake/reset values on resume and suspend.
Patch 03: Add i2c0 and touchscreen's node for A711 TBS tablet.

Thank you in advance for any review.
Best regards,
Mylène

Mylène Josserand (3):
  Input: edt-ft5x06 - Add support for regulator
  Input: edt-ft5x06 - Set wake/reset values on resume/suspend
  arm: dts: sun8i: a83t: a711: Add touchscreen node

 .../bindings/input/touchscreen/edt-ft5x06.txt      |  1 +
 arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts          | 16 +++++++++
 drivers/input/touchscreen/edt-ft5x06.c             | 41 ++++++++++++++++++++++
 3 files changed, 58 insertions(+)

-- 
2.11.0

^ permalink raw reply

* Re: [PATCH] Input: i8042 - add Lenovo LaVie Z to the i8042 reset list
From: Dmitry Torokhov @ 2018-07-18 17:28 UTC (permalink / raw)
  To: Chen-Yu Tsai; +Cc: linux-input, linux-kernel, stable
In-Reply-To: <20180705130849.10479-1-wens@csie.org>

On Thu, Jul 05, 2018 at 09:08:49PM +0800, Chen-Yu Tsai wrote:
> The Lenovo LaVie Z laptop requires i8042 to be reset in order to
> consistently detect its Elantech touchpad. The nomux and kbdreset
> quirks are not sufficient.
> 
> It's possible the other LaVie Z models from NEC require this as well.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Applied, thank you.

> ---
>  drivers/input/serio/i8042-x86ia64io.h | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h
> index b353d494ad40..136f6e7bf797 100644
> --- a/drivers/input/serio/i8042-x86ia64io.h
> +++ b/drivers/input/serio/i8042-x86ia64io.h
> @@ -527,6 +527,13 @@ static const struct dmi_system_id __initconst i8042_dmi_nomux_table[] = {
>  			DMI_MATCH(DMI_PRODUCT_NAME, "N24_25BU"),
>  		},
>  	},
> +	{
> +		/* Lenovo LaVie Z */
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> +			DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo LaVie Z"),
> +		},
> +	},
>  	{ }
>  };
>  
> -- 
> 2.18.0
> 

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 0/5] ti_am335x_tsc: Enable wakeup capability
From: Dmitry Torokhov @ 2018-07-18 17:09 UTC (permalink / raw)
  To: Lee Jones
  Cc: Vignesh R, Jonathan Cameron, linux-iio, linux-omap, linux-kernel,
	linux-input
In-Reply-To: <20180718074736.GA4641@dell>

On Wed, Jul 18, 2018 at 08:47:36AM +0100, Lee Jones wrote:
> On Tue, 17 Jul 2018, Vignesh R wrote:
> 
> > Hi Dmitry,
> > 
> > On Saturday 30 June 2018 04:03 PM, Vignesh R wrote:
> > > On AM335x, resistive TSC can wakeup the system from low power state.
> > > Currently, parent MFD device is marked as wakeup source, which is
> > > inaccurate as its the touch event generated by TSC thats the wakeup
> > > source. This series moves all wakeup related calls to operate on TSC
> > > device instead of MFD. It also marks TSC IRQ as wakeup capable, so that
> > > its not disabled during system suspend.
> > > 
> > > This series is based on Dmitry's comments here:
> > > https://lkml.org/lkml/2018/4/24/65
> > > 
> > > There are many new patches in this series, hence did not mark this as v4.
> > > 
> > > Vignesh R (5):
> > >   mfd: ti_am335x_tscadc: Don't mark TSCADC MFD as wakeup capable
> > >   Input: ti_am335x_tsc: Mark TSC device as wakeup source
> > >   mfd: ti_am335x_tscadc: Keep ADC interface on if child is wakeup
> > >     capable
> > >   iio: adc: ti_am335x_adc: Disable ADC during suspend unconditionally
> > >   Input: ti_am335x_tsc: Mark IRQ as wakeup capable
> > > 
> > >  drivers/iio/adc/ti_am335x_adc.c           | 12 ++++--------
> > >  drivers/input/touchscreen/ti_am335x_tsc.c | 22 +++++++++++++++++-----
> > >  drivers/mfd/ti_am335x_tscadc.c            | 14 +++++++++++++-
> > >  3 files changed, 34 insertions(+), 14 deletions(-)
> > > 
> > 
> > Gentle ping... Could you review/pick this series? MFD amd IIO bits are
> > already ACKed
> 
> MFD patches are reviewed "for my own reference" meaning that we
> haven't yet agreed on a merge plan yet.

I think this series makes sense to be pushed through a single tree as
opposed to being spread between 3, even if it could technically be
possible. It looks like Jonathan is fine with going it through either
his or some other tree, I am fine with it going through MFD. I just
Acked the 2 input-related patches.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 2/5] Input: ti_am335x_tsc: Mark TSC device as wakeup source
From: Dmitry Torokhov @ 2018-07-18 17:06 UTC (permalink / raw)
  To: Vignesh R
  Cc: Jonathan Cameron, Lee Jones, linux-iio, linux-omap, linux-kernel,
	linux-input
In-Reply-To: <20180630103318.25355-3-vigneshr@ti.com>

On Sat, Jun 30, 2018 at 04:03:15PM +0530, Vignesh R wrote:
> Instead of TSCADC MFD device, mark TSC as wakeup source and change all
> wakeup related PM calls to operate on TSC device.
> 
> Signed-off-by: Vignesh R <vigneshr@ti.com>

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

> ---
>  drivers/input/touchscreen/ti_am335x_tsc.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
> index b86c1e5fbc11..a4f25a915ffc 100644
> --- a/drivers/input/touchscreen/ti_am335x_tsc.c
> +++ b/drivers/input/touchscreen/ti_am335x_tsc.c
> @@ -46,6 +46,7 @@ static const int config_pins[] = {
>  struct titsc {
>  	struct input_dev	*input;
>  	struct ti_tscadc_dev	*mfd_tscadc;
> +	struct device		*dev;
>  	unsigned int		irq;
>  	unsigned int		wires;
>  	unsigned int		x_plate_resistance;
> @@ -276,7 +277,7 @@ static irqreturn_t titsc_irq(int irq, void *dev)
>  	if (status & IRQENB_HW_PEN) {
>  		ts_dev->pen_down = true;
>  		irqclr |= IRQENB_HW_PEN;
> -		pm_stay_awake(ts_dev->mfd_tscadc->dev);
> +		pm_stay_awake(ts_dev->dev);
>  	}
>  
>  	if (status & IRQENB_PENUP) {
> @@ -286,7 +287,7 @@ static irqreturn_t titsc_irq(int irq, void *dev)
>  			input_report_key(input_dev, BTN_TOUCH, 0);
>  			input_report_abs(input_dev, ABS_PRESSURE, 0);
>  			input_sync(input_dev);
> -			pm_relax(ts_dev->mfd_tscadc->dev);
> +			pm_relax(ts_dev->dev);
>  		} else {
>  			ts_dev->pen_down = true;
>  		}
> @@ -422,6 +423,7 @@ static int titsc_probe(struct platform_device *pdev)
>  	ts_dev->mfd_tscadc = tscadc_dev;
>  	ts_dev->input = input_dev;
>  	ts_dev->irq = tscadc_dev->irq;
> +	ts_dev->dev = &pdev->dev;
>  
>  	err = titsc_parse_dt(pdev, ts_dev);
>  	if (err) {
> @@ -436,6 +438,8 @@ static int titsc_probe(struct platform_device *pdev)
>  		goto err_free_mem;
>  	}
>  
> +	device_init_wakeup(&pdev->dev, true);
> +
>  	titsc_writel(ts_dev, REG_IRQSTATUS, TSC_IRQENB_MASK);
>  	titsc_writel(ts_dev, REG_IRQENABLE, IRQENB_FIFO0THRES);
>  	titsc_writel(ts_dev, REG_IRQENABLE, IRQENB_EOS);
> @@ -467,6 +471,7 @@ static int titsc_probe(struct platform_device *pdev)
>  	return 0;
>  
>  err_free_irq:
> +	device_init_wakeup(&pdev->dev, false);
>  	free_irq(ts_dev->irq, ts_dev);
>  err_free_mem:
>  	input_free_device(input_dev);
> @@ -479,6 +484,7 @@ static int titsc_remove(struct platform_device *pdev)
>  	struct titsc *ts_dev = platform_get_drvdata(pdev);
>  	u32 steps;
>  
> +	device_init_wakeup(&pdev->dev, false);
>  	free_irq(ts_dev->irq, ts_dev);
>  
>  	/* total steps followed by the enable mask */
> @@ -499,7 +505,7 @@ static int __maybe_unused titsc_suspend(struct device *dev)
>  	unsigned int idle;
>  
>  	tscadc_dev = ti_tscadc_dev_get(to_platform_device(dev));
> -	if (device_may_wakeup(tscadc_dev->dev)) {
> +	if (device_may_wakeup(dev)) {
>  		titsc_writel(ts_dev, REG_IRQSTATUS, TSC_IRQENB_MASK);
>  		idle = titsc_readl(ts_dev, REG_IRQENABLE);
>  		titsc_writel(ts_dev, REG_IRQENABLE,
> @@ -515,11 +521,11 @@ static int __maybe_unused titsc_resume(struct device *dev)
>  	struct ti_tscadc_dev *tscadc_dev;
>  
>  	tscadc_dev = ti_tscadc_dev_get(to_platform_device(dev));
> -	if (device_may_wakeup(tscadc_dev->dev)) {
> +	if (device_may_wakeup(dev)) {
>  		titsc_writel(ts_dev, REG_IRQWAKEUP,
>  				0x00);
>  		titsc_writel(ts_dev, REG_IRQCLR, IRQENB_HW_PEN);
> -		pm_relax(ts_dev->mfd_tscadc->dev);
> +		pm_relax(dev);
>  	}
>  	titsc_step_config(ts_dev);
>  	titsc_writel(ts_dev, REG_FIFO0THR,
> -- 
> 2.18.0
> 

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 5/5] Input: ti_am335x_tsc: Mark IRQ as wakeup capable
From: Dmitry Torokhov @ 2018-07-18 17:06 UTC (permalink / raw)
  To: Vignesh R
  Cc: Jonathan Cameron, Lee Jones, linux-iio, linux-omap, linux-kernel,
	linux-input
In-Reply-To: <20180630103318.25355-6-vigneshr@ti.com>

On Sat, Jun 30, 2018 at 04:03:18PM +0530, Vignesh R wrote:
> 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>

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

> ---
>  drivers/input/touchscreen/ti_am335x_tsc.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
> index a4f25a915ffc..9e8684ab48f4 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>
>  
> @@ -439,6 +440,9 @@ static int titsc_probe(struct platform_device *pdev)
>  	}
>  
>  	device_init_wakeup(&pdev->dev, true);
> +	err = dev_pm_set_wake_irq(&pdev->dev, ts_dev->irq);
> +	if (err)
> +		dev_err(&pdev->dev, "irq wake enable failed.\n");
>  
>  	titsc_writel(ts_dev, REG_IRQSTATUS, TSC_IRQENB_MASK);
>  	titsc_writel(ts_dev, REG_IRQENABLE, IRQENB_FIFO0THRES);
> @@ -471,6 +475,7 @@ static int titsc_probe(struct platform_device *pdev)
>  	return 0;
>  
>  err_free_irq:
> +	dev_pm_clear_wake_irq(&pdev->dev);
>  	device_init_wakeup(&pdev->dev, false);
>  	free_irq(ts_dev->irq, ts_dev);
>  err_free_mem:
> @@ -484,6 +489,7 @@ static int titsc_remove(struct platform_device *pdev)
>  	struct titsc *ts_dev = platform_get_drvdata(pdev);
>  	u32 steps;
>  
> +	dev_pm_clear_wake_irq(&pdev->dev);
>  	device_init_wakeup(&pdev->dev, false);
>  	free_irq(ts_dev->irq, ts_dev);
>  
> -- 
> 2.18.0
> 

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] input: gpio_keys: add missing include to gpio_keys.h
From: Dmitry Torokhov @ 2018-07-18 16:48 UTC (permalink / raw)
  To: Matti Vaittinen; +Cc: robh, gregkh, tglx, jeffy.chen, linux-kernel, linux-input
In-Reply-To: <20180627073458.GA27800@localhost.localdomain>

On Wed, Jun 27, 2018 at 10:34:58AM +0300, Matti Vaittinen wrote:
> gpio_keys.h uses 'bool' - type which is defined in linux/types.h.
> Include this header.
> 
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>

Applied, thank you.

> ---
>  include/linux/gpio_keys.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
> index 7160df54a6fe..3f84aeb81e48 100644
> --- a/include/linux/gpio_keys.h
> +++ b/include/linux/gpio_keys.h
> @@ -2,6 +2,8 @@
>  #ifndef _GPIO_KEYS_H
>  #define _GPIO_KEYS_H
>  
> +#include <linux/types.h>
> +
>  struct device;
>  
>  /**
> -- 
> 2.14.3
> 

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v7 1/4] resource: Move reparent_resources() to kernel/resource.c and make it public
From: Andy Shevchenko @ 2018-07-18 16:37 UTC (permalink / raw)
  To: Baoquan He
  Cc: Nicolas Pitre, brijesh.singh-5C7GfCeVMHo, devicetree,
	David Airlie, linux-pci-u79uwXL29TY76Z2rM5mHXA,
	richard.weiyang-Re5JQEeQqe8AvxtiuMwx3w, Max Filippov,
	Paul Mackerras, baiyaowei-0p4V/sDNsUmm0O/7XYngnFaTQe2KTcn/,
	KY Srinivasan, Frank Rowand, Lorenzo Pieralisi, Stephen Hemminger,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Michael Ellerman,
	Patrik Jakobsson, linux-input, Gustavo Padovan, Borislav Petkov,
	Dave Young, Tom Lendacky, Haiyang Zhang, Maarten Lankhorst,
	Josh Triplett
In-Reply-To: <CAHp75VdO88ydJQ9GHdaDUmAmzL6QHR=US6JiXZ1R_EEA-xWR1Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Wed, Jul 18, 2018 at 7:36 PM, Andy Shevchenko
<andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Wed, Jul 18, 2018 at 5:49 AM, Baoquan He <bhe-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> reparent_resources() is duplicated in arch/microblaze/pci/pci-common.c
>> and arch/powerpc/kernel/pci-common.c, so move it to kernel/resource.c
>> so that it's shared.

>> + * Returns 0 on success, -ENOTSUPP if child resource is not completely
>> + * contained by 'res', -ECANCELED if no any conflicting entry found.

You also can refer to constants by prefixing them with %, e.g. %-ENOTSUPP.
But this is up to you completely.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH v7 1/4] resource: Move reparent_resources() to kernel/resource.c and make it public
From: Andy Shevchenko @ 2018-07-18 16:36 UTC (permalink / raw)
  To: Baoquan He
  Cc: Nicolas Pitre, brijesh.singh, devicetree, David Airlie, linux-pci,
	richard.weiyang, Keith Busch, Max Filippov, Paul Mackerras,
	baiyaowei, Frank Rowand, Dan Williams, Lorenzo Pieralisi,
	Stephen Hemminger, linux-nvdimm, Michael Ellerman,
	Patrik Jakobsson, linux-input, Gustavo Padovan, Borislav Petkov,
	Dave Young, Vivek Goyal, Tom Lendacky, Haiyang Zhang, Maarten
In-Reply-To: <20180718024944.577-2-bhe@redhat.com>

On Wed, Jul 18, 2018 at 5:49 AM, Baoquan He <bhe@redhat.com> wrote:
> reparent_resources() is duplicated in arch/microblaze/pci/pci-common.c
> and arch/powerpc/kernel/pci-common.c, so move it to kernel/resource.c
> so that it's shared.

Some minor stuff.

> +/**
> + * reparent_resources - reparent resource children of parent that res covers
> + * @parent: parent resource descriptor
> + * @res: resource descriptor desired by caller
> + *
> + * Returns 0 on success, -ENOTSUPP if child resource is not completely
> + * contained by 'res', -ECANCELED if no any conflicting entry found.

'res' -> @res

> + *
> + * Reparent resource children of 'parent' that conflict with 'res'

Ditto + 'parent' -> @parent

> + * under 'res', and make 'res' replace those children.

Ditto.

> + */
> +int reparent_resources(struct resource *parent, struct resource *res)
> +{
> +       struct resource *p, **pp;
> +       struct resource **firstpp = NULL;
> +
> +       for (pp = &parent->child; (p = *pp) != NULL; pp = &p->sibling) {
> +               if (p->end < res->start)
> +                       continue;
> +               if (res->end < p->start)
> +                       break;
> +               if (p->start < res->start || p->end > res->end)
> +                       return -ENOTSUPP;       /* not completely contained */
> +               if (firstpp == NULL)
> +                       firstpp = pp;
> +       }
> +       if (firstpp == NULL)
> +               return -ECANCELED; /* didn't find any conflicting entries? */
> +       res->parent = parent;
> +       res->child = *firstpp;
> +       res->sibling = *pp;
> +       *firstpp = res;
> +       *pp = NULL;
> +       for (p = res->child; p != NULL; p = p->sibling) {
> +               p->parent = res;

> +               pr_debug("PCI: Reparented %s %pR under %s\n",
> +                        p->name, p, res->name);

Now, PCI is a bit confusing here.

> +       }
> +       return 0;
> +}
> +EXPORT_SYMBOL(reparent_resources);
> +
>  static void __init __reserve_region_with_split(struct resource *root,
>                 resource_size_t start, resource_size_t end,
>                 const char *name)
> --
> 2.13.6
>



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH v2 1/2] Input: atmel_mxt_ts: Add support for  optional regulators.
From: Paweł Chmiel @ 2018-07-18 16:21 UTC (permalink / raw)
  To: Nick Dyer
  Cc: dmitry.torokhov, robh+dt, mark.rutland, nicolas.ferre,
	alexandre.belloni, linux-input, devicetree, linux-arm-kernel,
	linux-kernel
In-Reply-To: <20180717210005.GA21416@hairyalien>

On Tuesday, July 17, 2018 10:00:05 PM CEST Nick Dyer wrote:
> On Tue, Jul 17, 2018 at 08:16:25PM +0200, Paweł Chmiel wrote:
> > This patch adds optional regulators, which can be used to power
> > up touchscreen. After enabling regulators, we need to wait 150msec.
> > This value is taken from official driver.
> > 
> > It was tested on Samsung Galaxy i9000 (based on Samsung S5PV210 SOC).
> > 
> > Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
> > ---
> > Changes from v1:
> >   - Enable regulators only if reset_gpio is present.
> >   - Switch from devm_regulator_get_optional to devm_regulator_get
> > ---
> >  drivers/input/touchscreen/atmel_mxt_ts.c | 46 ++++++++++++++++++++++++++++++--
> >  1 file changed, 44 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> > index 54fe190fd4bc..005f0fee9fc8 100644
> > --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> > +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> > @@ -27,6 +27,7 @@
> >  #include <linux/interrupt.h>
> >  #include <linux/of.h>
> >  #include <linux/property.h>
> > +#include <linux/regulator/consumer.h>
> >  #include <linux/slab.h>
> >  #include <linux/gpio/consumer.h>
> >  #include <linux/property.h>
> > @@ -194,10 +195,10 @@ enum t100_type {
> >  
> >  /* Delay times */
> >  #define MXT_BACKUP_TIME		50	/* msec */
> > -#define MXT_RESET_GPIO_TIME	20	/* msec */
> >  #define MXT_RESET_INVALID_CHG	100	/* msec */
> >  #define MXT_RESET_TIME		200	/* msec */
> >  #define MXT_RESET_TIMEOUT	3000	/* msec */
> > +#define MXT_REGULATOR_DELAY	150	/* msec */
> >  #define MXT_CRC_TIMEOUT		1000	/* msec */
> >  #define MXT_FW_RESET_TIME	3000	/* msec */
> >  #define MXT_FW_CHG_TIMEOUT	300	/* msec */
> > @@ -310,6 +311,8 @@ struct mxt_data {
> >  	struct t7_config t7_cfg;
> >  	struct mxt_dbg dbg;
> >  	struct gpio_desc *reset_gpio;
> > +	struct regulator *vdd_reg;
> > +	struct regulator *avdd_reg;
> >  
> >  	/* Cached parameters from object table */
> >  	u16 T5_address;
> > @@ -3076,6 +3079,22 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
> >  		return error;
> >  	}
> >  
> > +	data->vdd_reg = devm_regulator_get(&client->dev, "vdd");
> > +	if (IS_ERR(data->vdd_reg)) {
> > +		error = PTR_ERR(data->vdd_reg);
> > +		dev_err(&client->dev, "Failed to get vdd regulator: %d\n",
> > +			error);
> > +		return error;
> > +	}
> > +
> > +	data->avdd_reg = devm_regulator_get(&client->dev, "avdd");
> > +	if (IS_ERR(data->avdd_reg)) {
> > +		error = PTR_ERR(data->avdd_reg);
> > +		dev_err(&client->dev, "Failed to get avdd regulator: %d\n",
> > +			error);
> > +		return error;
> > +	}
> > +
> >  	error = devm_request_threaded_irq(&client->dev, client->irq,
> >  					  NULL, mxt_interrupt, IRQF_ONESHOT,
> >  					  client->name, data);
> > @@ -3087,7 +3106,26 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
> >  	disable_irq(client->irq);
> >  
> >  	if (data->reset_gpio) {
> > -		msleep(MXT_RESET_GPIO_TIME);
> > +		error = regulator_enable(data->vdd_reg);
> > +		if (error) {
> > +			dev_err(&client->dev, "Failed to enable vdd regulator: %d\n",
> > +				error);
> > +			return error;
> > +		}
> > +
> > +		error = regulator_enable(data->avdd_reg);
> > +		if (error) {
> > +			dev_err(&client->dev, "Failed to enable avdd regulator: %d\n",
> > +				error);
> > +			return error;
> > +		}
> > +
> > +		/*
> > +		 * According to maXTouch power sequencing specification, RESET line
> > +		 * must be kept low until some time after regulators come up to
> > +		 * voltage
> > +		 */
> > +		msleep(MXT_REGULATOR_DELAY);
> >  		gpiod_set_value(data->reset_gpio, 1);
> >  		msleep(MXT_RESET_INVALID_CHG);
> 
> Hi Pawel-
> 
> I see you've borrowed some of the logic from the patch I wrote a while
> back (see https://github.com/ndyer/linux/commit/8e9687e41ed062 )
Actually, i was looking at https://github.com/atmel-maxtouch/linux/blob/maxtouch-v3.14/drivers/input/touchscreen/atmel_mxt_ts.c (and didn't saw Your patch till now).
Are You going to submit it? (it has more functionalities - for example suspend mode read from device tree).
> 
> The correct behaviour according to Atmel should be:
> 
> * Make RESET zero
> * Bring up regulators
> * Wait for a period to settle (150 msec)
> * Release RESET
> * Wait for 100 msec whilst device gets through bootloader
> * Wait for CHG line assert before reading info block
> 
> I can't see the first and last steps in your patch at present.
About first step - reset_gpio is readed by using devm_gpiod_get_optional with GPIOD_OUT_LOW flag, so i think (but might be wrong)  that we don't need to set this gpio value again to 0 before enabling regulators,
since currently only place where reset_gpio is used is in driver probe (in Your patch it is used in other cases/places - for example in mxt_start/stop, when we enable regulators).
About missing wait after releasing reset, shouldn't this be separate patch (since currently driver is not doing it)? I can prepare it and send with other in next version.

Thanks for feedback
> 
> The only downside with this approach is that there are a lot of
> delays during driver probe, but I believe the asynchronous probe stuff
> that's landed since I wrote the original patch should address that.
> 
> cheers
> 
> Nick
> 
> >  	}
> > @@ -3116,6 +3154,10 @@ static int mxt_remove(struct i2c_client *client)
> >  	struct mxt_data *data = i2c_get_clientdata(client);
> >  
> >  	disable_irq(data->irq);
> > +	if (data->reset_gpio) {
> > +		regulator_disable(data->avdd_reg);
> > +		regulator_disable(data->vdd_reg);
> > +	}
> >  	sysfs_remove_group(&client->dev.kobj, &mxt_attr_group);
> >  	mxt_free_input_device(data);
> >  	mxt_free_object_table(data);
> 

^ 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