Add support for the VDD and IOVDD power supply inputs. This allows the
chip to share its supplies with other components (e.g. panel) and manage
them.
Signed-off-by: Mike Looijmans <[email protected]>
---
.../input/touchscreen/sitronix,st1232.yaml | 6 +++
drivers/input/touchscreen/st1232.c | 54 ++++++++++++++++---
2 files changed, 52 insertions(+), 8 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml b/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
index 1d8ca19fd37a..240be8d49232 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
@@ -28,6 +28,12 @@ properties:
description: A phandle to the reset GPIO
maxItems: 1
+ vdd-supply:
+ description: Power supply regulator for the chip
+
+ vddio-supply:
+ description: Power supply regulator for the I2C bus
+
required:
- compatible
- reg
diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c
index e38ba3e4f183..d9c9f6f1f11a 100644
--- a/drivers/input/touchscreen/st1232.c
+++ b/drivers/input/touchscreen/st1232.c
@@ -44,6 +44,11 @@
#define REG_XY_COORDINATES 0x12
#define ST_TS_MAX_FINGERS 10
+enum st1232_regulators {
+ ST1232_REGULATOR_VDD,
+ ST1232_REGULATOR_IOVDD,
+};
+
struct st_chip_info {
bool have_z;
u16 max_area;
@@ -56,6 +61,7 @@ struct st1232_ts_data {
struct touchscreen_properties prop;
struct dev_pm_qos_request low_latency_req;
struct gpio_desc *reset_gpio;
+ struct regulator_bulk_data regulators[2];
const struct st_chip_info *chip_info;
int read_buf_len;
u8 *read_buf;
@@ -197,17 +203,36 @@ static irqreturn_t st1232_ts_irq_handler(int irq, void *dev_id)
return IRQ_HANDLED;
}
-static void st1232_ts_power(struct st1232_ts_data *ts, bool poweron)
+static int st1232_ts_power_on(struct st1232_ts_data *ts)
+{
+ int err;
+
+ err = regulator_bulk_enable(ARRAY_SIZE(ts->regulators), ts->regulators);
+ if (err)
+ return err;
+
+ usleep_range(5000, 6000);
+
+ if (ts->reset_gpio)
+ gpiod_set_value_cansleep(ts->reset_gpio, 0);
+
+ return 0;
+}
+
+static void st1232_ts_power_off(struct st1232_ts_data *ts)
{
if (ts->reset_gpio)
- gpiod_set_value_cansleep(ts->reset_gpio, !poweron);
+ gpiod_set_value_cansleep(ts->reset_gpio, 1);
+ usleep_range(100, 150);
+ regulator_bulk_disable(ARRAY_SIZE(ts->regulators), ts->regulators);
}
-static void st1232_ts_power_off(void *data)
+static void st1232_ts_power_off_action(void *data)
{
- st1232_ts_power(data, false);
+ st1232_ts_power_off(data);
}
+
static const struct st_chip_info st1232_chip_info = {
.have_z = true,
.max_area = 0xff,
@@ -266,6 +291,14 @@ static int st1232_ts_probe(struct i2c_client *client,
ts->client = client;
ts->input_dev = input_dev;
+ ts->regulators[ST1232_REGULATOR_VDD].supply = "vdd";
+ ts->regulators[ST1232_REGULATOR_IOVDD].supply = "iovdd";
+ error = devm_regulator_bulk_get(&client->dev,
+ ARRAY_SIZE(ts->regulators),
+ ts->regulators);
+ if (error)
+ return error;
+
ts->reset_gpio = devm_gpiod_get_optional(&client->dev, NULL,
GPIOD_OUT_HIGH);
if (IS_ERR(ts->reset_gpio)) {
@@ -275,9 +308,14 @@ static int st1232_ts_probe(struct i2c_client *client,
return error;
}
- st1232_ts_power(ts, true);
+ error = st1232_ts_power_on(ts);
+ if (error) {
+ dev_err(&client->dev, "Failed to power on: %d\n", error);
+ return error;
+ }
- error = devm_add_action_or_reset(&client->dev, st1232_ts_power_off, ts);
+ error = devm_add_action_or_reset(&client->dev,
+ st1232_ts_power_off_action, ts);
if (error) {
dev_err(&client->dev,
"Failed to install power off action: %d\n", error);
@@ -348,7 +386,7 @@ static int __maybe_unused st1232_ts_suspend(struct device *dev)
disable_irq(client->irq);
if (!device_may_wakeup(&client->dev))
- st1232_ts_power(ts, false);
+ st1232_ts_power_off(ts);
return 0;
}
@@ -359,7 +397,7 @@ static int __maybe_unused st1232_ts_resume(struct device *dev)
struct st1232_ts_data *ts = i2c_get_clientdata(client);
if (!device_may_wakeup(&client->dev))
- st1232_ts_power(ts, true);
+ st1232_ts_power_on(ts);
enable_irq(client->irq);
--
2.17.1
Hi Mike,
On Tue, May 24, 2022 at 10:12:16AM +0200, Mike Looijmans wrote:
> Add support for the VDD and IOVDD power supply inputs. This allows the
> chip to share its supplies with other components (e.g. panel) and manage
> them.
>
> Signed-off-by: Mike Looijmans <[email protected]>
> ---
> .../input/touchscreen/sitronix,st1232.yaml | 6 +++
> drivers/input/touchscreen/st1232.c | 54 ++++++++++++++++---
> 2 files changed, 52 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml b/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
> index 1d8ca19fd37a..240be8d49232 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
> +++ b/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
> @@ -28,6 +28,12 @@ properties:
> description: A phandle to the reset GPIO
> maxItems: 1
>
> + vdd-supply:
> + description: Power supply regulator for the chip
> +
> + vddio-supply:
> + description: Power supply regulator for the I2C bus
> +
> required:
> - compatible
> - reg
> diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c
> index e38ba3e4f183..d9c9f6f1f11a 100644
> --- a/drivers/input/touchscreen/st1232.c
> +++ b/drivers/input/touchscreen/st1232.c
> @@ -44,6 +44,11 @@
> #define REG_XY_COORDINATES 0x12
> #define ST_TS_MAX_FINGERS 10
>
> +enum st1232_regulators {
> + ST1232_REGULATOR_VDD,
> + ST1232_REGULATOR_IOVDD,
> +};
> +
> struct st_chip_info {
> bool have_z;
> u16 max_area;
> @@ -56,6 +61,7 @@ struct st1232_ts_data {
> struct touchscreen_properties prop;
> struct dev_pm_qos_request low_latency_req;
> struct gpio_desc *reset_gpio;
> + struct regulator_bulk_data regulators[2];
> const struct st_chip_info *chip_info;
> int read_buf_len;
> u8 *read_buf;
> @@ -197,17 +203,36 @@ static irqreturn_t st1232_ts_irq_handler(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> -static void st1232_ts_power(struct st1232_ts_data *ts, bool poweron)
> +static int st1232_ts_power_on(struct st1232_ts_data *ts)
> +{
> + int err;
> +
> + err = regulator_bulk_enable(ARRAY_SIZE(ts->regulators), ts->regulators);
> + if (err)
> + return err;
Does it really make sense to try and handle regulators when reset gpio
is not defined? Would it not be better to tie them to the presence of
reset gpio to make sure we implement proper power on sequence?
> +
> + usleep_range(5000, 6000);
> +
> + if (ts->reset_gpio)
> + gpiod_set_value_cansleep(ts->reset_gpio, 0);
> +
> + return 0;
> +}
Thanks.
--
Dmitry
Comment inlined below (mailserver injects signature halfway through the
mail usually).
Met vriendelijke groet / kind regards,
Mike Looijmans
System Expert
TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands
T: +31 (0) 499 33 69 69
E: [email protected]
W: http://www.topic.nl
Please consider the environment before printing this e-mail
On 28-05-2022 06:47, Dmitry Torokhov wrote:
> Hi Mike,
>
> On Tue, May 24, 2022 at 10:12:16AM +0200, Mike Looijmans wrote:
>> Add support for the VDD and IOVDD power supply inputs. This allows the
>> chip to share its supplies with other components (e.g. panel) and manage
>> them.
>>
>> Signed-off-by: Mike Looijmans <[email protected]>
>> ---
>> .../input/touchscreen/sitronix,st1232.yaml | 6 +++
>> drivers/input/touchscreen/st1232.c | 54 ++++++++++++++++---
>> 2 files changed, 52 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml b/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
>> index 1d8ca19fd37a..240be8d49232 100644
>> --- a/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
>> +++ b/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
>> @@ -28,6 +28,12 @@ properties:
>> description: A phandle to the reset GPIO
>> maxItems: 1
>>
>> + vdd-supply:
>> + description: Power supply regulator for the chip
>> +
>> + vddio-supply:
>> + description: Power supply regulator for the I2C bus
>> +
>> required:
>> - compatible
>> - reg
>> diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c
>> index e38ba3e4f183..d9c9f6f1f11a 100644
>> --- a/drivers/input/touchscreen/st1232.c
>> +++ b/drivers/input/touchscreen/st1232.c
>> @@ -44,6 +44,11 @@
>> #define REG_XY_COORDINATES 0x12
>> #define ST_TS_MAX_FINGERS 10
>>
>> +enum st1232_regulators {
>> + ST1232_REGULATOR_VDD,
>> + ST1232_REGULATOR_IOVDD,
>> +};
>> +
>> struct st_chip_info {
>> bool have_z;
>> u16 max_area;
>> @@ -56,6 +61,7 @@ struct st1232_ts_data {
>> struct touchscreen_properties prop;
>> struct dev_pm_qos_request low_latency_req;
>> struct gpio_desc *reset_gpio;
>> + struct regulator_bulk_data regulators[2];
>> const struct st_chip_info *chip_info;
>> int read_buf_len;
>> u8 *read_buf;
>> @@ -197,17 +203,36 @@ static irqreturn_t st1232_ts_irq_handler(int irq, void *dev_id)
>> return IRQ_HANDLED;
>> }
>>
>> -static void st1232_ts_power(struct st1232_ts_data *ts, bool poweron)
>> +static int st1232_ts_power_on(struct st1232_ts_data *ts)
>> +{
>> + int err;
>> +
>> + err = regulator_bulk_enable(ARRAY_SIZE(ts->regulators), ts->regulators);
>> + if (err)
>> + return err;
> Does it really make sense to try and handle regulators when reset gpio
> is not defined? Would it not be better to tie them to the presence of
> reset gpio to make sure we implement proper power on sequence?
I thought that's what we're doing here. The datasheet says 5ms between
power-good and reset de-assert. Whether or not the hardware guys
bothered to actually connect the reset is out of our hands. The
regulator is not mandatory either, we'll get a dummy supply from the
framework when not defined.
The main use case here is that if for example the panel and touchscreen
share a power supply, they can now turn off the power supply when not in
use.
>
>> +
>> + usleep_range(5000, 6000);
>> +
>> + if (ts->reset_gpio)
>> + gpiod_set_value_cansleep(ts->reset_gpio, 0);
>> +
>> + return 0;
>> +}
> Thanks.
>
--
Mike Looijmans
On Tue, May 24, 2022 at 10:12:16AM +0200, Mike Looijmans wrote:
> Add support for the VDD and IOVDD power supply inputs. This allows the
> chip to share its supplies with other components (e.g. panel) and manage
> them.
>
> Signed-off-by: Mike Looijmans <[email protected]>
> ---
> .../input/touchscreen/sitronix,st1232.yaml | 6 +++
Separate patch please.
> drivers/input/touchscreen/st1232.c | 54 ++++++++++++++++---
> 2 files changed, 52 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml b/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
> index 1d8ca19fd37a..240be8d49232 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
> +++ b/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
> @@ -28,6 +28,12 @@ properties:
> description: A phandle to the reset GPIO
> maxItems: 1
>
> + vdd-supply:
> + description: Power supply regulator for the chip
> +
> + vddio-supply:
> + description: Power supply regulator for the I2C bus
> +
> required:
> - compatible
> - reg
> diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c
> index e38ba3e4f183..d9c9f6f1f11a 100644
> --- a/drivers/input/touchscreen/st1232.c
> +++ b/drivers/input/touchscreen/st1232.c
> @@ -44,6 +44,11 @@
> #define REG_XY_COORDINATES 0x12
> #define ST_TS_MAX_FINGERS 10
>
> +enum st1232_regulators {
> + ST1232_REGULATOR_VDD,
> + ST1232_REGULATOR_IOVDD,
> +};
> +
> struct st_chip_info {
> bool have_z;
> u16 max_area;
> @@ -56,6 +61,7 @@ struct st1232_ts_data {
> struct touchscreen_properties prop;
> struct dev_pm_qos_request low_latency_req;
> struct gpio_desc *reset_gpio;
> + struct regulator_bulk_data regulators[2];
> const struct st_chip_info *chip_info;
> int read_buf_len;
> u8 *read_buf;
> @@ -197,17 +203,36 @@ static irqreturn_t st1232_ts_irq_handler(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> -static void st1232_ts_power(struct st1232_ts_data *ts, bool poweron)
> +static int st1232_ts_power_on(struct st1232_ts_data *ts)
> +{
> + int err;
> +
> + err = regulator_bulk_enable(ARRAY_SIZE(ts->regulators), ts->regulators);
> + if (err)
> + return err;
> +
> + usleep_range(5000, 6000);
> +
> + if (ts->reset_gpio)
> + gpiod_set_value_cansleep(ts->reset_gpio, 0);
> +
> + return 0;
> +}
> +
> +static void st1232_ts_power_off(struct st1232_ts_data *ts)
> {
> if (ts->reset_gpio)
> - gpiod_set_value_cansleep(ts->reset_gpio, !poweron);
> + gpiod_set_value_cansleep(ts->reset_gpio, 1);
> + usleep_range(100, 150);
> + regulator_bulk_disable(ARRAY_SIZE(ts->regulators), ts->regulators);
> }
>
> -static void st1232_ts_power_off(void *data)
> +static void st1232_ts_power_off_action(void *data)
> {
> - st1232_ts_power(data, false);
> + st1232_ts_power_off(data);
> }
>
> +
> static const struct st_chip_info st1232_chip_info = {
> .have_z = true,
> .max_area = 0xff,
> @@ -266,6 +291,14 @@ static int st1232_ts_probe(struct i2c_client *client,
> ts->client = client;
> ts->input_dev = input_dev;
>
> + ts->regulators[ST1232_REGULATOR_VDD].supply = "vdd";
> + ts->regulators[ST1232_REGULATOR_IOVDD].supply = "iovdd";
> + error = devm_regulator_bulk_get(&client->dev,
> + ARRAY_SIZE(ts->regulators),
> + ts->regulators);
> + if (error)
> + return error;
> +
> ts->reset_gpio = devm_gpiod_get_optional(&client->dev, NULL,
> GPIOD_OUT_HIGH);
> if (IS_ERR(ts->reset_gpio)) {
> @@ -275,9 +308,14 @@ static int st1232_ts_probe(struct i2c_client *client,
> return error;
> }
>
> - st1232_ts_power(ts, true);
> + error = st1232_ts_power_on(ts);
> + if (error) {
> + dev_err(&client->dev, "Failed to power on: %d\n", error);
> + return error;
> + }
>
> - error = devm_add_action_or_reset(&client->dev, st1232_ts_power_off, ts);
> + error = devm_add_action_or_reset(&client->dev,
> + st1232_ts_power_off_action, ts);
> if (error) {
> dev_err(&client->dev,
> "Failed to install power off action: %d\n", error);
> @@ -348,7 +386,7 @@ static int __maybe_unused st1232_ts_suspend(struct device *dev)
> disable_irq(client->irq);
>
> if (!device_may_wakeup(&client->dev))
> - st1232_ts_power(ts, false);
> + st1232_ts_power_off(ts);
>
> return 0;
> }
> @@ -359,7 +397,7 @@ static int __maybe_unused st1232_ts_resume(struct device *dev)
> struct st1232_ts_data *ts = i2c_get_clientdata(client);
>
> if (!device_may_wakeup(&client->dev))
> - st1232_ts_power(ts, true);
> + st1232_ts_power_on(ts);
>
> enable_irq(client->irq);
>
> --
> 2.17.1
>
>