2017-12-08 21:54:55

by Mylène Josserand

[permalink] [raw]
Subject: [PATCH 0/5] sun8i-a83t: Add touchscreen support on TBS A711

Hello everyone,

This patch series adds touchscreen support (FocalTech EDT-FT5x06
Polytouch) for TBS A711 (Allwinner sun8i-a83t SoC).
This touchscreen is using i2c so this series adds support of this bus
on A83T.

Series information:
- Based on last linux-next (next-20171208)
- Had dependencies on AXP209 Quentin Schultz's series:
https://www.spinics.net/lists/linux-gpio/msg26913.htmlx

Patch 01: Add i2c0 node on a83t device tree.
Patch 02: Add i2c0 pin group.
Patch 03: Enable i2c0 on TBS A711.
Patch 04: Add support for regulator in the FocalTech touchscreen driver
because A711 tablet is using a regulator to power-up the touchscreen.
Patch 05: Add the touchscreen node in A711 TBS tablet.

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

Mylène Josserand (5):
arm: dts: sun8i: a83t: Add I2C0 node
arm: dts: sun8i: a83t: Add I2C0 pins group
arm: dts: sun8i: a83t: a711: Enable I2C0
Input: edt-ft5x06 - Add support for regulator
arm: dts: sun8i: a83t: a711: Add touchscreen node

arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts | 18 +++++++++++++++++
arch/arm/boot/dts/sun8i-a83t.dtsi | 16 +++++++++++++++
drivers/input/touchscreen/edt-ft5x06.c | 33 +++++++++++++++++++++++++++++++
3 files changed, 67 insertions(+)

--
2.11.0


2017-12-08 21:55:03

by Mylène Josserand

[permalink] [raw]
Subject: [PATCH 1/5] arm: dts: sun8i: a83t: Add I2C0 node

Add I2C0 node for A83T.

Signed-off-by: Mylène Josserand <[email protected]>
---
arch/arm/boot/dts/sun8i-a83t.dtsi | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index 19acae1b4089..848cf3f19962 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -177,6 +177,17 @@
#dma-cells = <1>;
};

+ i2c0: i2c@01c2ac00 {
+ compatible = "allwinner,sun6i-a31-i2c";
+ reg = <0x01c2ac00 0x400>;
+ interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_BUS_I2C0>;
+ resets = <&ccu RST_BUS_I2C0>;
+ status = "disabled";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
mmc0: mmc@1c0f000 {
compatible = "allwinner,sun8i-a83t-mmc",
"allwinner,sun7i-a20-mmc";
--
2.11.0

2017-12-08 21:55:07

by Mylène Josserand

[permalink] [raw]
Subject: [PATCH 3/5] arm: dts: sun8i: a83t: a711: Enable I2C0

The A711 has a touchscreen connected by I2C0.
Enable only I2C0 node for the moment.

Signed-off-by: Mylène Josserand <[email protected]>
---
arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts | 7 +++++++
1 file changed, 7 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..84cca1a48cea 100644
--- a/arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts
+++ b/arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts
@@ -105,6 +105,13 @@
status = "okay";
};

+&i2c0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c0_pins>;
+ clock-frequency = <400000>;
+ status = "okay";
+};
+
&mmc0 {
vmmc-supply = <&reg_dcdc1>;
pinctrl-names = "default";
--
2.11.0

2017-12-08 21:55:10

by Mylène Josserand

[permalink] [raw]
Subject: [PATCH 5/5] arm: dts: sun8i: a83t: a711: Add touchscreen node

Tha A711 tablet has a FocalTech EDT-FT5x06 Polytouch touchscreen.
It is connected via I2C0 so add his node in i2c0's node.

The reset line is PD5, the interrupt line is PL7 and the power
supply is the ldo_io0 regulator.

Signed-off-by: Mylène Josserand <[email protected]>
---
arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts b/arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts
index 84cca1a48cea..738359917a34 100644
--- a/arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts
+++ b/arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts
@@ -110,6 +110,17 @@
pinctrl-0 = <&i2c0_pins>;
clock-frequency = <400000>;
status = "okay";
+
+ polytouch: edt-ft5x06@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>;
+ power-supply = <&reg_ldo_io0>;
+ touchscreen-size-x = <1024>;
+ touchscreen-size-y = <600>;
+ };
};

&mmc0 {
--
2.11.0

2017-12-08 21:55:53

by Mylène Josserand

[permalink] [raw]
Subject: [PATCH 4/5] Input: edt-ft5x06 - Add support for regulator

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

Signed-off-by: Mylène Josserand <[email protected]>
---
drivers/input/touchscreen/edt-ft5x06.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)

diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index c53a3d7239e7..44b0e04a8f35 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 *supply;

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->supply = devm_regulator_get_optional(&client->dev, "power");
+ if (IS_ERR(tsdata->supply)) {
+ error = PTR_ERR(tsdata->supply);
+ dev_err(&client->dev, "failed to request regulator: %d\n",
+ error);
+ return error;
+ };
+
+ if (tsdata->supply) {
+ error = regulator_enable(tsdata->supply);
+ if (error < 0) {
+ dev_err(&client->dev, "failed to enable supply: %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->supply)
+ regulator_disable(tsdata->supply);
+
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->supply) {
+ ret = regulator_enable(tsdata->supply);
+ if (ret < 0) {
+ dev_err(dev, "failed to enable supply: %d\n", ret);
+ return ret;
+ }
+ }
+
return 0;
}

--
2.11.0

2017-12-08 21:56:12

by Mylène Josserand

[permalink] [raw]
Subject: [PATCH 2/5] arm: dts: sun8i: a83t: Add I2C0 pins group

Add pins group to configure it as I2C0.

Signed-off-by: Mylène Josserand <[email protected]>
---
arch/arm/boot/dts/sun8i-a83t.dtsi | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index 848cf3f19962..3bce6bd3dc79 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -347,6 +347,11 @@
#interrupt-cells = <3>;
#gpio-cells = <3>;

+ i2c0_pins: i2c0-pins {
+ pins = "PH0", "PH1";
+ function = "i2c0";
+ };
+
mmc0_pins: mmc0-pins {
pins = "PF0", "PF1", "PF2",
"PF3", "PF4", "PF5";
--
2.11.0

2017-12-08 22:02:05

by Mylène Josserand

[permalink] [raw]
Subject: Re: [PATCH 0/5] sun8i-a83t: Add touchscreen support on TBS A711

Hi,

Le Fri, 8 Dec 2017 22:54:14 +0100,
Mylène Josserand <[email protected]> a écrit :

> Hello everyone,
>
> This patch series adds touchscreen support (FocalTech EDT-FT5x06
> Polytouch) for TBS A711 (Allwinner sun8i-a83t SoC).
> This touchscreen is using i2c so this series adds support of this bus
> on A83T.
>
> Series information:
> - Based on last linux-next (next-20171208)
> - Had dependencies on AXP209 Quentin Schultz's series:
> https://www.spinics.net/lists/linux-gpio/msg26913.htmlx

I forgot to mention a dependency on Maxime Ripard's patch that
reinstate AXP813 PMIC:
https://patchwork.kernel.org/patch/10077037/

Thanks,

Mylène

>
> Patch 01: Add i2c0 node on a83t device tree.
> Patch 02: Add i2c0 pin group.
> Patch 03: Enable i2c0 on TBS A711.
> Patch 04: Add support for regulator in the FocalTech touchscreen
> driver because A711 tablet is using a regulator to power-up the
> touchscreen. Patch 05: Add the touchscreen node in A711 TBS tablet.
>
> Thank you in advance for any review.
> Best regards,
> Mylène
>
> Mylène Josserand (5):
> arm: dts: sun8i: a83t: Add I2C0 node
> arm: dts: sun8i: a83t: Add I2C0 pins group
> arm: dts: sun8i: a83t: a711: Enable I2C0
> Input: edt-ft5x06 - Add support for regulator
> arm: dts: sun8i: a83t: a711: Add touchscreen node
>
> arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts | 18 +++++++++++++++++
> arch/arm/boot/dts/sun8i-a83t.dtsi | 16 +++++++++++++++
> drivers/input/touchscreen/edt-ft5x06.c | 33
> +++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+)
>

2017-12-09 01:16:37

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH 4/5] Input: edt-ft5x06 - Add support for regulator

Hi Myl?ne,

On Fri, Dec 08, 2017 at 10:54:18PM +0100, Myl?ne Josserand wrote:
> Add the support of regulator to use them as VCC source.
>
> Signed-off-by: Myl?ne Josserand <[email protected]>
> ---
> drivers/input/touchscreen/edt-ft5x06.c | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> index c53a3d7239e7..44b0e04a8f35 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 *supply;
>
> 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->supply = devm_regulator_get_optional(&client->dev, "power");

I'd prefer if we used devm_regulator_get() instead. On a
fully-constrained systems a missing regulator will be substituted with a
dummy one that you can then use in regulator_enable() and
regulator_disable(). The _optional regulator API is reserved for cases
where some of chip functionality is optional and you can engage it by
powering up parts of the chip. The reset is not such case: it is always
present, but may not be exposed to the OS to control.

I think the preference is to call the supply by the name is a datasheet,
something like "vcc" or "vdd", etc.

Thanks.

--
Dmitry

2017-12-09 01:24:53

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH 4/5] Input: edt-ft5x06 - Add support for regulator

On Fri, Dec 08, 2017 at 05:16:29PM -0800, Dmitry Torokhov wrote:
> Hi Myl?ne,
>
> On Fri, Dec 08, 2017 at 10:54:18PM +0100, Myl?ne Josserand wrote:
> > Add the support of regulator to use them as VCC source.
> >
> > Signed-off-by: Myl?ne Josserand <[email protected]>
> > ---
> > drivers/input/touchscreen/edt-ft5x06.c | 33 +++++++++++++++++++++++++++++++++
> > 1 file changed, 33 insertions(+)
> >
> > diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> > index c53a3d7239e7..44b0e04a8f35 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 *supply;
> >
> > 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->supply = devm_regulator_get_optional(&client->dev, "power");
>
> I'd prefer if we used devm_regulator_get() instead. On a
> fully-constrained systems a missing regulator will be substituted with a
> dummy one that you can then use in regulator_enable() and
> regulator_disable(). The _optional regulator API is reserved for cases
> where some of chip functionality is optional and you can engage it by
> powering up parts of the chip. The reset is not such case: it is always
> present, but may not be exposed to the OS to control.
>
> I think the preference is to call the supply by the name is a datasheet,
> something like "vcc" or "vdd", etc.

Looking at this some more, you need to make sure your new regulator
support plays well with reset/wake GPIOs. I.e. you probably want to
properly bring the chip out of reset in edt_ft5x06_ts_resume() and maybe
driver the reset line low in suspend to make sure you do not leak into
the unpowered chip?

Also, please update
Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt with
the additional binding data and cc device tree folks and Rob Herring.

Thanks.

--
Dmitry

2017-12-11 07:15:18

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH 1/5] arm: dts: sun8i: a83t: Add I2C0 node

Hi,

On Fri, Dec 08, 2017 at 10:54:15PM +0100, Myl?ne Josserand wrote:
> Add I2C0 node for A83T.
>
> Signed-off-by: Myl?ne Josserand <[email protected]>
> ---
> arch/arm/boot/dts/sun8i-a83t.dtsi | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
> index 19acae1b4089..848cf3f19962 100644
> --- a/arch/arm/boot/dts/sun8i-a83t.dtsi
> +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
> @@ -177,6 +177,17 @@
> #dma-cells = <1>;
> };
>
> + i2c0: i2c@01c2ac00 {

Drop the leading 0, it generates a warning in dtc.

> + compatible = "allwinner,sun6i-a31-i2c";

Can you add a SoC-specific compatible there please?

> + reg = <0x01c2ac00 0x400>;

And you should order your nodes by physical address.

Thanks!
Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


Attachments:
(No filename) (921.00 B)
signature.asc (833.00 B)
Download all attachments

2017-12-11 07:16:38

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH 5/5] arm: dts: sun8i: a83t: a711: Add touchscreen node

Hi,

On Fri, Dec 08, 2017 at 10:54:19PM +0100, Myl?ne Josserand wrote:
> Tha A711 tablet has a FocalTech EDT-FT5x06 Polytouch touchscreen.
> It is connected via I2C0 so add his node in i2c0's node.
>
> The reset line is PD5, the interrupt line is PL7 and the power
> supply is the ldo_io0 regulator.
>
> Signed-off-by: Myl?ne Josserand <[email protected]>
> ---
> arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts b/arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts
> index 84cca1a48cea..738359917a34 100644
> --- a/arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts
> +++ b/arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts
> @@ -110,6 +110,17 @@
> pinctrl-0 = <&i2c0_pins>;
> clock-frequency = <400000>;
> status = "okay";
> +
> + polytouch: edt-ft5x06@38 {

You're not using that label anywhere, so you can drop it, and the node
name must be the class of the device.

Thanks!
Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


Attachments:
(No filename) (1.07 kB)
signature.asc (833.00 B)
Download all attachments

2017-12-11 07:18:05

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH 3/5] arm: dts: sun8i: a83t: a711: Enable I2C0

Hi,

On Fri, Dec 08, 2017 at 10:54:17PM +0100, Myl?ne Josserand wrote:
> The A711 has a touchscreen connected by I2C0.
> Enable only I2C0 node for the moment.
>
> Signed-off-by: Myl?ne Josserand <[email protected]>
> ---
> arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts | 7 +++++++
> 1 file changed, 7 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..84cca1a48cea 100644
> --- a/arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts
> +++ b/arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts
> @@ -105,6 +105,13 @@
> status = "okay";
> };
>
> +&i2c0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c0_pins>;

If there's only one set of pins, maybe we should force that muxing in
the DTSI?

Thanks!
Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


Attachments:
(No filename) (900.00 B)
signature.asc (833.00 B)
Download all attachments

2017-12-11 08:31:52

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH 1/5] arm: dts: sun8i: a83t: Add I2C0 node

On Mon, Dec 11, 2017 at 3:15 PM, Maxime Ripard
<[email protected]> wrote:
> Hi,
>
> On Fri, Dec 08, 2017 at 10:54:15PM +0100, Mylène Josserand wrote:
>> Add I2C0 node for A83T.
>>
>> Signed-off-by: Mylène Josserand <[email protected]>
>> ---
>> arch/arm/boot/dts/sun8i-a83t.dtsi | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
>> index 19acae1b4089..848cf3f19962 100644
>> --- a/arch/arm/boot/dts/sun8i-a83t.dtsi
>> +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
>> @@ -177,6 +177,17 @@
>> #dma-cells = <1>;
>> };
>>
>> + i2c0: i2c@01c2ac00 {
>
> Drop the leading 0, it generates a warning in dtc.
>
>> + compatible = "allwinner,sun6i-a31-i2c";
>
> Can you add a SoC-specific compatible there please?
>
>> + reg = <0x01c2ac00 0x400>;
>
> And you should order your nodes by physical address.

I have a similar patch in my A83T I2S branch:

https://github.com/wens/linux/commit/8b76a3e555b39a06f3f8182e4ff5645de4c4cbc3

ChenYu