* [PATCH v2 0/2] sun8i-a83t: Add touchscreen support on TBS A711
@ 2017-12-28 16:33 Mylène Josserand
2017-12-28 16:33 ` [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator Mylène Josserand
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Mylène Josserand @ 2017-12-28 16:33 UTC (permalink / raw)
To: dmitry.torokhov, robh+dt, mark.rutland, linux, maxime.ripard, wens
Cc: linux-arm-kernel, linux-input, devicetree, linux-kernel,
mylene.josserand, thomas.petazzoni, quentin.schulz
Hello everyone,
This is a V2 of the patch series that adds touchscreen support
(FocalTech EDT-FT5x06 Polytouch) for TBS A711 (Allwinner sun8i-a83t SoC).
Based on last linux-next (next-20171222).
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 i2c0 and touchscreen's node for A711 TBS tablet.
Thank you in advance for any review.
Best regards,
Mylène
Mylène Josserand (2):
Input: edt-ft5x06 - Add support for regulator
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 | 33 ++++++++++++++++++++++
3 files changed, 50 insertions(+)
--
2.11.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator
2017-12-28 16:33 [PATCH v2 0/2] sun8i-a83t: Add touchscreen support on TBS A711 Mylène Josserand
@ 2017-12-28 16:33 ` Mylène Josserand
2017-12-30 3:15 ` [PATCH] Input: fix semicolon.cocci warnings kbuild test robot
` (4 more replies)
2017-12-28 16:33 ` [PATCH v2 2/2] arm: dts: sun8i: a83t: a711: Add touchscreen node Mylène Josserand
[not found] ` <20171228163336.28131-1-mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2 siblings, 5 replies; 14+ messages in thread
From: Mylène Josserand @ 2017-12-28 16:33 UTC (permalink / raw)
To: dmitry.torokhov, robh+dt, mark.rutland, linux, maxime.ripard, wens
Cc: linux-arm-kernel, linux-input, devicetree, linux-kernel,
mylene.josserand, thomas.petazzoni, quentin.schulz
Add the support of regulator to use it as VCC source.
Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
---
.../bindings/input/touchscreen/edt-ft5x06.txt | 1 +
drivers/input/touchscreen/edt-ft5x06.c | 33 ++++++++++++++++++++++
2 files changed, 34 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 c53a3d7239e7..5ee14a25a382 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;
@@ -993,6 +995,23 @@ 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);
+ dev_err(&client->dev, "failed to request regulator: %d\n",
+ error);
+ return error;
+ };
+
+ if (tsdata->vcc) {
+ 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)) {
@@ -1122,20 +1141,34 @@ 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);
+ if (tsdata->vcc)
+ 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);
+ if (tsdata->vcc) {
+ 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 [flat|nested] 14+ messages in thread
* [PATCH v2 2/2] arm: dts: sun8i: a83t: a711: Add touchscreen node
2017-12-28 16:33 [PATCH v2 0/2] sun8i-a83t: Add touchscreen support on TBS A711 Mylène Josserand
2017-12-28 16:33 ` [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator Mylène Josserand
@ 2017-12-28 16:33 ` Mylène Josserand
[not found] ` <20171228163336.28131-1-mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2 siblings, 0 replies; 14+ messages in thread
From: Mylène Josserand @ 2017-12-28 16:33 UTC (permalink / raw)
To: dmitry.torokhov, robh+dt, mark.rutland, linux, maxime.ripard, wens
Cc: thomas.petazzoni, devicetree, linux-kernel, quentin.schulz,
linux-input, mylene.josserand, linux-arm-kernel
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@free-electrons.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 a021ee6da396..7840f9aa9094 100644
--- a/arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts
+++ b/arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts
@@ -105,6 +105,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 = <®_ldo_io0>;
+ touchscreen-size-x = <1024>;
+ touchscreen-size-y = <600>;
+ };
+};
+
&mmc0 {
vmmc-supply = <®_dcdc1>;
pinctrl-names = "default";
--
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 related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 0/2] sun8i-a83t: Add touchscreen support on TBS A711
[not found] ` <20171228163336.28131-1-mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
@ 2017-12-28 16:46 ` Mylene JOSSERAND
0 siblings, 0 replies; 14+ messages in thread
From: Mylene JOSSERAND @ 2017-12-28 16:46 UTC (permalink / raw)
To: dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-I+IVW8TIWO2tmTQ+vhA3Yw,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8, wens-jdAy2FN1RRM
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-input-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8
Hello,
Le Thu, 28 Dec 2017 17:33:34 +0100,
Mylène Josserand <mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> a écrit :
> Hello everyone,
>
> This is a V2 of the patch series that adds touchscreen support
> (FocalTech EDT-FT5x06 Polytouch) for TBS A711 (Allwinner sun8i-a83t SoC).
> Based on last linux-next (next-20171222).
>
> 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.
I notice that I forgot the second review of Dmitry about reset/wake
gpios so I will send a V3 with the modifications.
Thanks,
Mylène
> - 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 i2c0 and touchscreen's node for A711 TBS tablet.
>
> Thank you in advance for any review.
> Best regards,
> Mylène
>
> Mylène Josserand (2):
> Input: edt-ft5x06 - Add support for regulator
> 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 | 33 ++++++++++++++++++++++
> 3 files changed, 50 insertions(+)
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator
2017-12-28 16:33 ` [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator Mylène Josserand
2017-12-30 3:15 ` [PATCH] Input: fix semicolon.cocci warnings kbuild test robot
@ 2017-12-30 3:15 ` kbuild test robot
2018-01-03 19:48 ` Rob Herring
` (2 subsequent siblings)
4 siblings, 0 replies; 14+ messages in thread
From: kbuild test robot @ 2017-12-30 3:15 UTC (permalink / raw)
Cc: kbuild-all, dmitry.torokhov, robh+dt, mark.rutland, linux,
maxime.ripard, wens, linux-arm-kernel, linux-input, devicetree,
linux-kernel, mylene.josserand, thomas.petazzoni, quentin.schulz
Hi Mylène,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on robh/for-next]
[also build test WARNING on v4.15-rc5 next-20171222]
[cannot apply to input/next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Myl-ne-Josserand/sun8i-a83t-Add-touchscreen-support-on-TBS-A711/20171230-091331
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
coccinelle warnings: (new ones prefixed by >>)
>> drivers/input/touchscreen/edt-ft5x06.c:1004:2-3: Unneeded semicolon
Please review and possibly fold the followup patch.
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] Input: fix semicolon.cocci warnings
2017-12-28 16:33 ` [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator Mylène Josserand
@ 2017-12-30 3:15 ` kbuild test robot
2017-12-30 3:15 ` [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator kbuild test robot
` (3 subsequent siblings)
4 siblings, 0 replies; 14+ messages in thread
From: kbuild test robot @ 2017-12-30 3:15 UTC (permalink / raw)
Cc: kbuild-all, dmitry.torokhov, robh+dt, mark.rutland, linux,
maxime.ripard, wens, linux-arm-kernel, linux-input, devicetree,
linux-kernel, mylene.josserand, thomas.petazzoni, quentin.schulz
From: Fengguang Wu <fengguang.wu@intel.com>
drivers/input/touchscreen/edt-ft5x06.c:1004:2-3: Unneeded semicolon
Remove unneeded semicolon.
Generated by: scripts/coccinelle/misc/semicolon.cocci
Fixes: 5969d946e8aa ("Input: edt-ft5x06 - Add support for regulator")
CC: Mylène Josserand <mylene.josserand@free-electrons.com>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---
edt-ft5x06.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -1001,7 +1001,7 @@ static int edt_ft5x06_ts_probe(struct i2
dev_err(&client->dev, "failed to request regulator: %d\n",
error);
return error;
- };
+ }
if (tsdata->vcc) {
error = regulator_enable(tsdata->vcc);
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator
2017-12-28 16:33 ` [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator Mylène Josserand
2017-12-30 3:15 ` [PATCH] Input: fix semicolon.cocci warnings kbuild test robot
2017-12-30 3:15 ` [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator kbuild test robot
@ 2018-01-03 19:48 ` Rob Herring
2018-01-22 12:51 ` Maxime Ripard
[not found] ` <20171228163336.28131-2-mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
4 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2018-01-03 19:48 UTC (permalink / raw)
To: Mylène Josserand
Cc: dmitry.torokhov, mark.rutland, linux, maxime.ripard, wens,
linux-arm-kernel, linux-input, devicetree, linux-kernel,
thomas.petazzoni, quentin.schulz
On Thu, Dec 28, 2017 at 05:33:35PM +0100, Mylène Josserand wrote:
> Add the support of regulator to use it as VCC source.
>
> Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
> ---
> .../bindings/input/touchscreen/edt-ft5x06.txt | 1 +
> drivers/input/touchscreen/edt-ft5x06.c | 33 ++++++++++++++++++++++
> 2 files changed, 34 insertions(+)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator
2017-12-28 16:33 ` [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator Mylène Josserand
` (2 preceding siblings ...)
2018-01-03 19:48 ` Rob Herring
@ 2018-01-22 12:51 ` Maxime Ripard
[not found] ` <20171228163336.28131-2-mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
4 siblings, 0 replies; 14+ messages in thread
From: Maxime Ripard @ 2018-01-22 12:51 UTC (permalink / raw)
To: dmitry.torokhov, Mylène Josserand
Cc: robh+dt, mark.rutland, linux, wens, linux-arm-kernel,
linux-input, devicetree, linux-kernel, thomas.petazzoni,
quentin.schulz
[-- Attachment #1: Type: text/plain, Size: 325 bytes --]
Dmitry,
On Thu, Dec 28, 2017 at 05:33:35PM +0100, Mylène Josserand wrote:
> Add the support of regulator to use it as VCC source.
>
> Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
Ping?
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator
[not found] ` <20171228163336.28131-2-mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
@ 2018-01-22 17:42 ` Dmitry Torokhov
2018-01-23 8:04 ` Lothar Waßmann
2018-01-23 9:10 ` Mylene Josserand
0 siblings, 2 replies; 14+ messages in thread
From: Dmitry Torokhov @ 2018-01-22 17:42 UTC (permalink / raw)
To: Mylène Josserand
Cc: Rob Herring, Mark Rutland, Russell King - ARM Linux,
Maxime Ripard, Chen-Yu Tsai,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-input-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, lkml, Thomas Petazzoni,
Quentin Schulz
Hi Mylène,
On Thu, Dec 28, 2017 at 8:33 AM, Mylène Josserand
<mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> Add the support of regulator to use it as VCC source.
>
> Signed-off-by: Mylène Josserand <mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
> .../bindings/input/touchscreen/edt-ft5x06.txt | 1 +
> drivers/input/touchscreen/edt-ft5x06.c | 33 ++++++++++++++++++++++
> 2 files changed, 34 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 c53a3d7239e7..5ee14a25a382 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;
> @@ -993,6 +995,23 @@ 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);
> + dev_err(&client->dev, "failed to request regulator: %d\n",
> + error);
> + return error;
> + };
As 0-day pounted out, this semicolon is not needed.
> +
> + if (tsdata->vcc) {
You do not need to check for non-NULL here, devm_regulator_get() wil
lnever give you a NULL. If regulator is not defined in DT/board
mappings, then dummy regulator will be provided. You can call
regulator_enable() and regulator_disable() and other regulator APIs
with dummy regulator.
> + 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)) {
> @@ -1122,20 +1141,34 @@ 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);
>
> + if (tsdata->vcc)
Same here.
> + 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);
>
> + if (tsdata->vcc) {
And here.
> + ret = regulator_enable(tsdata->vcc);
> + if (ret < 0) {
> + dev_err(dev, "failed to enable vcc: %d\n", ret);
> + return ret;
> + }
Since power to the device may have been cut, I think you need to
restore the register settings to whatever it was (factory vs work
mode, threshold, gain and offset registers, etc, etc).
> + }
> +
> return 0;
> }
>
> --
> 2.11.0
>
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator
2018-01-22 17:42 ` Dmitry Torokhov
@ 2018-01-23 8:04 ` Lothar Waßmann
2018-01-23 9:13 ` Mylene Josserand
2018-01-23 9:10 ` Mylene Josserand
1 sibling, 1 reply; 14+ messages in thread
From: Lothar Waßmann @ 2018-01-23 8:04 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Mylène Josserand, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA, Russell King - ARM Linux,
lkml, Quentin Schulz, Chen-Yu Tsai, Rob Herring,
linux-input-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard,
Thomas Petazzoni,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Hi,
On Mon, 22 Jan 2018 09:42:08 -0800 Dmitry Torokhov wrote:
> Hi Mylène,
>
> On Thu, Dec 28, 2017 at 8:33 AM, Mylène Josserand
> <mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> > Add the support of regulator to use it as VCC source.
> >
> > Signed-off-by: Mylène Josserand <mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> > ---
> > .../bindings/input/touchscreen/edt-ft5x06.txt | 1 +
> > drivers/input/touchscreen/edt-ft5x06.c | 33 ++++++++++++++++++++++
> > 2 files changed, 34 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 c53a3d7239e7..5ee14a25a382 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;
> > @@ -993,6 +995,23 @@ 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);
> > + dev_err(&client->dev, "failed to request regulator: %d\n",
> > + error);
>
I would check for -EPROBE_DEFER here and omit the error message in this
case.
Lothar Waßmann
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator
2018-01-22 17:42 ` Dmitry Torokhov
2018-01-23 8:04 ` Lothar Waßmann
@ 2018-01-23 9:10 ` Mylene Josserand
2018-01-23 17:58 ` Dmitry Torokhov
1 sibling, 1 reply; 14+ messages in thread
From: Mylene Josserand @ 2018-01-23 9:10 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rob Herring, Mark Rutland, Russell King - ARM Linux,
Maxime Ripard, Chen-Yu Tsai, linux-arm-kernel, linux-input,
devicetree, lkml, Thomas Petazzoni, Quentin Schulz
Hello Dimitry,
Thank you for the review!
Le Mon, 22 Jan 2018 09:42:08 -0800,
Dmitry Torokhov <dmitry.torokhov@gmail.com> a écrit :
> Hi Mylène,
>
> On Thu, Dec 28, 2017 at 8:33 AM, Mylène Josserand
> <mylene.josserand@free-electrons.com> wrote:
> > Add the support of regulator to use it as VCC source.
> >
> > Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
> > ---
> > .../bindings/input/touchscreen/edt-ft5x06.txt | 1 +
> > drivers/input/touchscreen/edt-ft5x06.c | 33 ++++++++++++++++++++++
> > 2 files changed, 34 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 c53a3d7239e7..5ee14a25a382 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;
> > @@ -993,6 +995,23 @@ 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);
> > + dev_err(&client->dev, "failed to request regulator: %d\n",
> > + error);
> > + return error;
> > + };
>
> As 0-day pounted out, this semicolon is not needed.
Yes, thanks, I will fix that in next version.
>
> > +
> > + if (tsdata->vcc) {
>
> You do not need to check for non-NULL here, devm_regulator_get() wil
> lnever give you a NULL. If regulator is not defined in DT/board
> mappings, then dummy regulator will be provided. You can call
> regulator_enable() and regulator_disable() and other regulator APIs
> with dummy regulator.
Okay, thanks for the explanation, I will remove that.
>
> > + 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)) {
> > @@ -1122,20 +1141,34 @@ 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);
> >
> > + if (tsdata->vcc)
>
> Same here.
yep
>
> > + 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);
> >
> > + if (tsdata->vcc) {
>
> And here.
yep
>
> > + ret = regulator_enable(tsdata->vcc);
> > + if (ret < 0) {
> > + dev_err(dev, "failed to enable vcc: %d\n", ret);
> > + return ret;
> > + }
>
> Since power to the device may have been cut, I think you need to
> restore the register settings to whatever it was (factory vs work
> mode, threshold, gain and offset registers, etc, etc).
Okay. Could you tell me how can I do that?
>
> > + }
> > +
> > return 0;
> > }
> >
> > --
> > 2.11.0
> >
>
> Thanks.
>
About your V2's review, you suggested to add support for wake/reset in
suspend/resume (that I forgot in this version). I wanted to add it but
with my board, I can't test suspend/resume. What should I do about
that?
If I send a V3 in next few days, do you think you will have time to
merge it for v4.16?
Thank you in advance,
Best regards,
Mylène
--
Mylène Josserand, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator
2018-01-23 8:04 ` Lothar Waßmann
@ 2018-01-23 9:13 ` Mylene Josserand
0 siblings, 0 replies; 14+ messages in thread
From: Mylene Josserand @ 2018-01-23 9:13 UTC (permalink / raw)
To: Lothar Waßmann
Cc: Dmitry Torokhov, Mark Rutland, devicetree,
Russell King - ARM Linux, lkml, Quentin Schulz, Chen-Yu Tsai,
Rob Herring, linux-input, Maxime Ripard, Thomas Petazzoni,
linux-arm-kernel
Hello Lothar,
Le Tue, 23 Jan 2018 09:04:14 +0100,
Lothar Waßmann <LW@KARO-electronics.de> a écrit :
> Hi,
>
> On Mon, 22 Jan 2018 09:42:08 -0800 Dmitry Torokhov wrote:
> > Hi Mylène,
> >
> > On Thu, Dec 28, 2017 at 8:33 AM, Mylène Josserand
> > <mylene.josserand@free-electrons.com> wrote:
> > > Add the support of regulator to use it as VCC source.
> > >
> > > Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
> > > ---
> > > .../bindings/input/touchscreen/edt-ft5x06.txt | 1 +
> > > drivers/input/touchscreen/edt-ft5x06.c | 33 ++++++++++++++++++++++
> > > 2 files changed, 34 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 c53a3d7239e7..5ee14a25a382 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;
> > > @@ -993,6 +995,23 @@ 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);
> > > + dev_err(&client->dev, "failed to request regulator: %d\n",
> > > + error);
> >
> I would check for -EPROBE_DEFER here and omit the error message in this
> case.
>
>
> Lothar Waßmann
Sure, I will add this case, thank you for the review.
Best regards,
--
Mylène Josserand, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator
2018-01-23 9:10 ` Mylene Josserand
@ 2018-01-23 17:58 ` Dmitry Torokhov
2018-01-26 10:11 ` Mylene Josserand
0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Torokhov @ 2018-01-23 17:58 UTC (permalink / raw)
To: Mylene Josserand
Cc: Rob Herring, Mark Rutland, Russell King - ARM Linux,
Maxime Ripard, Chen-Yu Tsai, linux-arm-kernel, linux-input,
devicetree, lkml, Thomas Petazzoni, Quentin Schulz
On Tue, Jan 23, 2018 at 10:10:55AM +0100, Mylene Josserand wrote:
> Hello Dimitry,
>
> Thank you for the review!
>
> Le Mon, 22 Jan 2018 09:42:08 -0800,
> Dmitry Torokhov <dmitry.torokhov@gmail.com> a écrit :
>
> > Hi Mylène,
> >
> > On Thu, Dec 28, 2017 at 8:33 AM, Mylène Josserand
> > <mylene.josserand@free-electrons.com> wrote:
> > > Add the support of regulator to use it as VCC source.
> > >
> > > Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
> > > ---
> > > .../bindings/input/touchscreen/edt-ft5x06.txt | 1 +
> > > drivers/input/touchscreen/edt-ft5x06.c | 33 ++++++++++++++++++++++
> > > 2 files changed, 34 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 c53a3d7239e7..5ee14a25a382 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;
> > > @@ -993,6 +995,23 @@ 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);
> > > + dev_err(&client->dev, "failed to request regulator: %d\n",
> > > + error);
> > > + return error;
> > > + };
> >
> > As 0-day pounted out, this semicolon is not needed.
>
> Yes, thanks, I will fix that in next version.
>
> >
> > > +
> > > + if (tsdata->vcc) {
> >
> > You do not need to check for non-NULL here, devm_regulator_get() wil
> > lnever give you a NULL. If regulator is not defined in DT/board
> > mappings, then dummy regulator will be provided. You can call
> > regulator_enable() and regulator_disable() and other regulator APIs
> > with dummy regulator.
>
> Okay, thanks for the explanation, I will remove that.
>
> >
> > > + 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)) {
> > > @@ -1122,20 +1141,34 @@ 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);
> > >
> > > + if (tsdata->vcc)
> >
> > Same here.
>
> yep
>
> >
> > > + 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);
> > >
> > > + if (tsdata->vcc) {
> >
> > And here.
>
> yep
>
> >
> > > + ret = regulator_enable(tsdata->vcc);
> > > + if (ret < 0) {
> > > + dev_err(dev, "failed to enable vcc: %d\n", ret);
> > > + return ret;
> > > + }
> >
> > Since power to the device may have been cut, I think you need to
> > restore the register settings to whatever it was (factory vs work
> > mode, threshold, gain and offset registers, etc, etc).
>
> Okay. Could you tell me how can I do that?
If you take a look at edt_ft5x06_work_mode() at the end ther eis code to
restore parameters. You want to factor it out and apply to the device
when resuming. You probably also want to see what mode (factory/normal)
the device was before suspend and restore the previous mode on resume.
>
> >
> > > + }
> > > +
> > > return 0;
> > > }
> > >
> > > --
> > > 2.11.0
> > >
> >
> > Thanks.
> >
>
> About your V2's review, you suggested to add support for wake/reset in
> suspend/resume (that I forgot in this version). I wanted to add it but
> with my board, I can't test suspend/resume. What should I do about
> that?
See if Simon Budig <simon.budig@kernelconcepts.de> can help us here.
>
> If I send a V3 in next few days, do you think you will have time to
> merge it for v4.16?
It all depends on the patch shape...
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator
2018-01-23 17:58 ` Dmitry Torokhov
@ 2018-01-26 10:11 ` Mylene Josserand
0 siblings, 0 replies; 14+ messages in thread
From: Mylene Josserand @ 2018-01-26 10:11 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rob Herring, Mark Rutland, Russell King - ARM Linux,
Maxime Ripard, Chen-Yu Tsai,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-input-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, lkml, Thomas Petazzoni,
Quentin Schulz, simon.budig-t93Ne7XHvje5bSeCtf/tX7NAH6kLmebB
Hello Dmitry,
Le Tue, 23 Jan 2018 09:58:29 -0800,
Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> a écrit :
> On Tue, Jan 23, 2018 at 10:10:55AM +0100, Mylene Josserand wrote:
> > Hello Dimitry,
> >
> > Thank you for the review!
> >
> > Le Mon, 22 Jan 2018 09:42:08 -0800,
> > Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> a écrit :
> >
> > > Hi Mylène,
> > >
> > > On Thu, Dec 28, 2017 at 8:33 AM, Mylène Josserand
> > > <mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> > > > Add the support of regulator to use it as VCC source.
> > > >
> > > > Signed-off-by: Mylène Josserand <mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> > > > ---
> > > > .../bindings/input/touchscreen/edt-ft5x06.txt | 1 +
> > > > drivers/input/touchscreen/edt-ft5x06.c | 33 ++++++++++++++++++++++
> > > > 2 files changed, 34 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 c53a3d7239e7..5ee14a25a382 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;
> > > > @@ -993,6 +995,23 @@ 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);
> > > > + dev_err(&client->dev, "failed to request regulator: %d\n",
> > > > + error);
> > > > + return error;
> > > > + };
> > >
> > > As 0-day pounted out, this semicolon is not needed.
> >
> > Yes, thanks, I will fix that in next version.
> >
> > >
> > > > +
> > > > + if (tsdata->vcc) {
> > >
> > > You do not need to check for non-NULL here, devm_regulator_get() wil
> > > lnever give you a NULL. If regulator is not defined in DT/board
> > > mappings, then dummy regulator will be provided. You can call
> > > regulator_enable() and regulator_disable() and other regulator APIs
> > > with dummy regulator.
> >
> > Okay, thanks for the explanation, I will remove that.
> >
> > >
> > > > + 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)) {
> > > > @@ -1122,20 +1141,34 @@ 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);
> > > >
> > > > + if (tsdata->vcc)
> > >
> > > Same here.
> >
> > yep
> >
> > >
> > > > + 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);
> > > >
> > > > + if (tsdata->vcc) {
> > >
> > > And here.
> >
> > yep
> >
> > >
> > > > + ret = regulator_enable(tsdata->vcc);
> > > > + if (ret < 0) {
> > > > + dev_err(dev, "failed to enable vcc: %d\n", ret);
> > > > + return ret;
> > > > + }
> > >
> > > Since power to the device may have been cut, I think you need to
> > > restore the register settings to whatever it was (factory vs work
> > > mode, threshold, gain and offset registers, etc, etc).
> >
> > Okay. Could you tell me how can I do that?
>
> If you take a look at edt_ft5x06_work_mode() at the end ther eis code to
> restore parameters. You want to factor it out and apply to the device
> when resuming. You probably also want to see what mode (factory/normal)
> the device was before suspend and restore the previous mode on resume.
Okay, thank you for your help.
About the factory/normal mode to be sure that I understand correctly,
in the resume, I should use something like "edt_ft5x06_debugfs_mode_set"
to check the "tsdata->factory_mode" and call "edt_ft5x06_factory_mode"
or "edt_ft5x06_work_mode", is not it?
As the current suspend/resume code is only enabling/disabling an irq, I
would like to understand exactly the modifications you would like.
>
> >
> > >
> > > > + }
> > > > +
> > > > return 0;
> > > > }
> > > >
> > > > --
> > > > 2.11.0
> > > >
> > >
> > > Thanks.
> > >
> >
> > About your V2's review, you suggested to add support for wake/reset in
> > suspend/resume (that I forgot in this version). I wanted to add it but
> > with my board, I can't test suspend/resume. What should I do about
> > that?
>
> See if Simon Budig <simon.budig-t93Ne7XHvje5bSeCtf/tX7NAH6kLmebB@public.gmane.org> can help us here.
Thank you, I contacted Simon (added him in CC) and unfortunately, he
doesn't have a setup with working suspend/resume so he can't help me
on that.
My patches are ready (except for factory/normal mode, see above) but as
I can't test suspend/resume, I prefer not to send them without testing
it..
>
> >
> > If I send a V3 in next few days, do you think you will have time to
> > merge it for v4.16?
>
> It all depends on the patch shape...
Sure I understand.
Best regards,
--
Mylène Josserand, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2018-01-26 10:11 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-28 16:33 [PATCH v2 0/2] sun8i-a83t: Add touchscreen support on TBS A711 Mylène Josserand
2017-12-28 16:33 ` [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator Mylène Josserand
2017-12-30 3:15 ` [PATCH] Input: fix semicolon.cocci warnings kbuild test robot
2017-12-30 3:15 ` [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator kbuild test robot
2018-01-03 19:48 ` Rob Herring
2018-01-22 12:51 ` Maxime Ripard
[not found] ` <20171228163336.28131-2-mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2018-01-22 17:42 ` Dmitry Torokhov
2018-01-23 8:04 ` Lothar Waßmann
2018-01-23 9:13 ` Mylene Josserand
2018-01-23 9:10 ` Mylene Josserand
2018-01-23 17:58 ` Dmitry Torokhov
2018-01-26 10:11 ` Mylene Josserand
2017-12-28 16:33 ` [PATCH v2 2/2] arm: dts: sun8i: a83t: a711: Add touchscreen node Mylène Josserand
[not found] ` <20171228163336.28131-1-mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2017-12-28 16:46 ` [PATCH v2 0/2] sun8i-a83t: Add touchscreen support on TBS A711 Mylene JOSSERAND
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).