Messages in this thread | | | Date | Mon, 30 May 2022 07:50:32 +0200 | Subject | Re: [PATCH] Input: st1232 - Support power supply regulators | From | Mike Looijmans <> |
| |
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: mike.looijmans@topicproducts.com W: 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 <mike.looijmans@topic.nl> >> --- >> .../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
| |