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
Add the support of regulator to use it as VCC source.
Signed-off-by: Mylène Josserand <[email protected]>
---
.../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
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 <[email protected]>
---
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 = <®_ldo_io0>;
+ touchscreen-size-x = <1024>;
+ touchscreen-size-y = <600>;
+ };
+};
+
&mmc0 {
vmmc-supply = <®_dcdc1>;
pinctrl-names = "default";
--
2.11.0
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 <[email protected]>
---
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
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 <[email protected]>
> ---
> .../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
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
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 <[email protected]>
> ---
> .../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(+)
On Thu, Jul 19, 2018 at 12:46:45AM +0200, Ondřej Jirman wrote:
> 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 <[email protected]>
> > ---
> > .../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)) {
You need to disable regulator here. We do not have
devm_regulator_enable() (and Mark had some concerns about mixing managed
and unmanaged APIs for regulators so we can't simply introduce it),
so I'd recommend using devm_add_action_or_reset() and iunstall custom
action to turn off regulator.
> > @@ -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?
Exactly, it should be in an "else" branch.
Thanks.
--
Dmitry
Hello,
On Fri, 20 Jul 2018 07:43:55 -0600
Rob Herring <[email protected]> wrote:
> 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 <[email protected]>
> > ---
> > .../bindings/input/touchscreen/edt-ft5x06.txt | 1 +
>
> Please add acks when posting new versions.
Yes, sorry, I missed it. I will be more careful, next time.
Best regards,
--
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
Hello,
Thank you for your review.
On Thu, 19 Jul 2018 00:46:45 +0200
Ondřej Jirman <[email protected]> wrote:
> 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 <[email protected]>
> > ---
> > .../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?
True, thank you for pointing it out.
>
> > 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.
ack
>
> Regards,
> o.
>
> > return 0;
> > }
> >
> > --
> > 2.11.0
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Best regards,
--
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
Hello Dmitry,
Thank you for your review!
On Mon, 23 Jul 2018 15:39:26 -0700
Dmitry Torokhov <[email protected]> wrote:
> On Thu, Jul 19, 2018 at 12:46:45AM +0200, Ondřej Jirman wrote:
> > 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 <[email protected]>
> > > ---
> > > .../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)) {
>
> You need to disable regulator here. We do not have
> devm_regulator_enable() (and Mark had some concerns about mixing managed
> and unmanaged APIs for regulators so we can't simply introduce it),
> so I'd recommend using devm_add_action_or_reset() and iunstall custom
> action to turn off regulator.
Yep, I see. Thank you about "devm_add_action_or_reset", I did not know
this function. I will add it in next version.
>
> > > @@ -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?
>
> Exactly, it should be in an "else" branch.
>
> Thanks.
>
I see, I will add it in v4.
Best regards,
--
Mylène Josserand, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com