* [PATCH v2 1/2] dt-bindings: input: touchscreen: edt-ft5x06: add iovcc-supply @ 2021-05-10 19:31 Stephan Gerhold 2021-05-10 19:31 ` [PATCH v2 2/2] Input: edt-ft5x06 - add support for iovcc-supply Stephan Gerhold 0 siblings, 1 reply; 10+ messages in thread From: Stephan Gerhold @ 2021-05-10 19:31 UTC (permalink / raw) To: Dmitry Torokhov Cc: linux-input, Rob Herring, devicetree, Simon Budig, Andy Shevchenko, Stephan Gerhold, Ondrej Jirman, Marco Felsch, Rob Herring At the moment, the edt-ft5x06 driver can control a single regulator ("vcc"). However, some FocalTech touch controllers have an additional IOVCC pin that should be supplied with the digital I/O voltage. The I/O voltage might be provided by another regulator that should also be kept on. Otherwise, the touchscreen can randomly stop functioning if the regulator is turned off because no other components still require it. Document (optional) support for controlling the regulator for IOVCC using "iovcc-supply". Cc: Ondrej Jirman <megous@megous.com> Cc: Marco Felsch <m.felsch@pengutronix.de> Acked-by: Rob Herring <robh@kernel.org> Signed-off-by: Stephan Gerhold <stephan@gerhold.net> --- Changes in v2: None, added Rob's Acked-by v1: https://lore.kernel.org/linux-input/20210108192337.563679-1-stephan@gerhold.net/ --- .../devicetree/bindings/input/touchscreen/edt-ft5x06.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml index bfc3a8b5e118..2e8da7470513 100644 --- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml +++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml @@ -56,6 +56,7 @@ properties: wakeup-source: true vcc-supply: true + iovcc-supply: true gain: description: Allows setting the sensitivity in the range from 0 to 31. -- 2.31.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/2] Input: edt-ft5x06 - add support for iovcc-supply 2021-05-10 19:31 [PATCH v2 1/2] dt-bindings: input: touchscreen: edt-ft5x06: add iovcc-supply Stephan Gerhold @ 2021-05-10 19:31 ` Stephan Gerhold 2021-05-10 19:48 ` Ondřej Jirman 0 siblings, 1 reply; 10+ messages in thread From: Stephan Gerhold @ 2021-05-10 19:31 UTC (permalink / raw) To: Dmitry Torokhov Cc: linux-input, Rob Herring, devicetree, Simon Budig, Andy Shevchenko, Stephan Gerhold, Ondrej Jirman, Marco Felsch At the moment, the edt-ft5x06 driver can control a single regulator ("vcc"). However, some FocalTech touch controllers have an additional IOVCC pin that should be supplied with the digital I/O voltage. The I/O voltage might be provided by another regulator that should also be kept on. Otherwise, the touchscreen can randomly stop functioning if the regulator is turned off because no other components still require it. Implement (optional) support for also enabling an "iovcc-supply". The datasheet specifies a delay of ~ 10us before enabling VDD/VCC after IOVCC is enabled, so make sure to enable IOVCC first. Cc: Ondrej Jirman <megous@megous.com> Cc: Marco Felsch <m.felsch@pengutronix.de> Signed-off-by: Stephan Gerhold <stephan@gerhold.net> --- Sorry about the long delay, couldn't find the time to test the new changes :) Changes in v2: - Respect 10us delay between enabling IOVCC and VDD/VCC line (suggested by Marco Felsch) v1: https://lore.kernel.org/linux-input/20210108192337.563679-2-stephan@gerhold.net/ --- drivers/input/touchscreen/edt-ft5x06.c | 34 ++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c index 2eefbc2485bc..d3271856bb5c 100644 --- a/drivers/input/touchscreen/edt-ft5x06.c +++ b/drivers/input/touchscreen/edt-ft5x06.c @@ -104,6 +104,7 @@ struct edt_ft5x06_ts_data { u16 num_x; u16 num_y; struct regulator *vcc; + struct regulator *iovcc; struct gpio_desc *reset_gpio; struct gpio_desc *wake_gpio; @@ -1067,6 +1068,7 @@ static void edt_ft5x06_disable_regulator(void *arg) struct edt_ft5x06_ts_data *data = arg; regulator_disable(data->vcc); + regulator_disable(data->iovcc); } static int edt_ft5x06_ts_probe(struct i2c_client *client, @@ -1107,9 +1109,28 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client, return error; } + tsdata->iovcc = devm_regulator_get(&client->dev, "iovcc"); + if (IS_ERR(tsdata->iovcc)) { + error = PTR_ERR(tsdata->iovcc); + if (error != -EPROBE_DEFER) + dev_err(&client->dev, + "failed to request iovcc regulator: %d\n", error); + return error; + } + + error = regulator_enable(tsdata->iovcc); + if (error < 0) { + dev_err(&client->dev, "failed to enable iovcc: %d\n", error); + return error; + } + + /* Delay enabling VCC for > 10us (T_ivd) after IOVCC */ + usleep_range(10, 100); + error = regulator_enable(tsdata->vcc); if (error < 0) { dev_err(&client->dev, "failed to enable vcc: %d\n", error); + regulator_disable(tsdata->iovcc); return error; } @@ -1289,6 +1310,9 @@ static int __maybe_unused edt_ft5x06_ts_suspend(struct device *dev) ret = regulator_disable(tsdata->vcc); if (ret) dev_warn(dev, "Failed to disable vcc\n"); + ret = regulator_disable(tsdata->iovcc); + if (ret) + dev_warn(dev, "Failed to disable iovcc\n"); return 0; } @@ -1319,9 +1343,19 @@ static int __maybe_unused edt_ft5x06_ts_resume(struct device *dev) gpiod_set_value_cansleep(reset_gpio, 1); usleep_range(5000, 6000); + ret = regulator_enable(tsdata->iovcc); + if (ret) { + dev_err(dev, "Failed to enable iovcc\n"); + return ret; + } + + /* Delay enabling VCC for > 10us (T_ivd) after IOVCC */ + usleep_range(10, 100); + ret = regulator_enable(tsdata->vcc); if (ret) { dev_err(dev, "Failed to enable vcc\n"); + regulator_disable(tsdata->iovcc); return ret; } -- 2.31.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] Input: edt-ft5x06 - add support for iovcc-supply 2021-05-10 19:31 ` [PATCH v2 2/2] Input: edt-ft5x06 - add support for iovcc-supply Stephan Gerhold @ 2021-05-10 19:48 ` Ondřej Jirman 2021-05-10 20:09 ` Andy Shevchenko 2021-05-10 20:16 ` Stephan Gerhold 0 siblings, 2 replies; 10+ messages in thread From: Ondřej Jirman @ 2021-05-10 19:48 UTC (permalink / raw) To: Stephan Gerhold Cc: Dmitry Torokhov, linux-input, Rob Herring, devicetree, Simon Budig, Andy Shevchenko, Marco Felsch Hello Stephan, On Mon, May 10, 2021 at 09:31:08PM +0200, Stephan Gerhold wrote: > At the moment, the edt-ft5x06 driver can control a single regulator > ("vcc"). However, some FocalTech touch controllers have an additional > IOVCC pin that should be supplied with the digital I/O voltage. > > The I/O voltage might be provided by another regulator that should also > be kept on. Otherwise, the touchscreen can randomly stop functioning if > the regulator is turned off because no other components still require it. > > Implement (optional) support for also enabling an "iovcc-supply". > The datasheet specifies a delay of ~ 10us before enabling VDD/VCC > after IOVCC is enabled, so make sure to enable IOVCC first. > > Cc: Ondrej Jirman <megous@megous.com> > Cc: Marco Felsch <m.felsch@pengutronix.de> > Signed-off-by: Stephan Gerhold <stephan@gerhold.net> > --- > Sorry about the long delay, couldn't find the time to test the new changes :) > > Changes in v2: > - Respect 10us delay between enabling IOVCC and VDD/VCC line > (suggested by Marco Felsch) > > v1: https://lore.kernel.org/linux-input/20210108192337.563679-2-stephan@gerhold.net/ > --- > drivers/input/touchscreen/edt-ft5x06.c | 34 ++++++++++++++++++++++++++ > 1 file changed, 34 insertions(+) > > diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c > index 2eefbc2485bc..d3271856bb5c 100644 > --- a/drivers/input/touchscreen/edt-ft5x06.c > +++ b/drivers/input/touchscreen/edt-ft5x06.c > @@ -104,6 +104,7 @@ struct edt_ft5x06_ts_data { > u16 num_x; > u16 num_y; > struct regulator *vcc; > + struct regulator *iovcc; > > struct gpio_desc *reset_gpio; > struct gpio_desc *wake_gpio; > @@ -1067,6 +1068,7 @@ static void edt_ft5x06_disable_regulator(void *arg) > struct edt_ft5x06_ts_data *data = arg; > > regulator_disable(data->vcc); > + regulator_disable(data->iovcc); > } > > static int edt_ft5x06_ts_probe(struct i2c_client *client, > @@ -1107,9 +1109,28 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client, > return error; > } > > + tsdata->iovcc = devm_regulator_get(&client->dev, "iovcc"); > + if (IS_ERR(tsdata->iovcc)) { > + error = PTR_ERR(tsdata->iovcc); > + if (error != -EPROBE_DEFER) > + dev_err(&client->dev, > + "failed to request iovcc regulator: %d\n", error); Please use dev_err_probe instead. If this pattern is used for vcc-supply, maybe change that too. Maybe also consider using a bulk regulator API. Thank you, o. > + return error; > + } > + > + error = regulator_enable(tsdata->iovcc); > + if (error < 0) { > + dev_err(&client->dev, "failed to enable iovcc: %d\n", error); > + return error; > + } > + > + /* Delay enabling VCC for > 10us (T_ivd) after IOVCC */ > + usleep_range(10, 100); > + > error = regulator_enable(tsdata->vcc); > if (error < 0) { > dev_err(&client->dev, "failed to enable vcc: %d\n", error); > + regulator_disable(tsdata->iovcc); > return error; > } > > @@ -1289,6 +1310,9 @@ static int __maybe_unused edt_ft5x06_ts_suspend(struct device *dev) > ret = regulator_disable(tsdata->vcc); > if (ret) > dev_warn(dev, "Failed to disable vcc\n"); > + ret = regulator_disable(tsdata->iovcc); > + if (ret) > + dev_warn(dev, "Failed to disable iovcc\n"); > > return 0; > } > @@ -1319,9 +1343,19 @@ static int __maybe_unused edt_ft5x06_ts_resume(struct device *dev) > gpiod_set_value_cansleep(reset_gpio, 1); > usleep_range(5000, 6000); > > + ret = regulator_enable(tsdata->iovcc); > + if (ret) { > + dev_err(dev, "Failed to enable iovcc\n"); > + return ret; > + } > + > + /* Delay enabling VCC for > 10us (T_ivd) after IOVCC */ > + usleep_range(10, 100); > + > ret = regulator_enable(tsdata->vcc); > if (ret) { > dev_err(dev, "Failed to enable vcc\n"); > + regulator_disable(tsdata->iovcc); > return ret; > } > > -- > 2.31.1 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] Input: edt-ft5x06 - add support for iovcc-supply 2021-05-10 19:48 ` Ondřej Jirman @ 2021-05-10 20:09 ` Andy Shevchenko 2021-05-10 20:17 ` Ondřej Jirman 2021-05-10 20:16 ` Stephan Gerhold 1 sibling, 1 reply; 10+ messages in thread From: Andy Shevchenko @ 2021-05-10 20:09 UTC (permalink / raw) To: Ondřej Jirman Cc: Stephan Gerhold, Dmitry Torokhov, linux-input, Rob Herring, devicetree, Simon Budig, Marco Felsch On Mon, May 10, 2021 at 09:48:48PM +0200, Ondřej Jirman wrote: > On Mon, May 10, 2021 at 09:31:08PM +0200, Stephan Gerhold wrote: > > + tsdata->iovcc = devm_regulator_get(&client->dev, "iovcc"); > > + if (IS_ERR(tsdata->iovcc)) { > > + error = PTR_ERR(tsdata->iovcc); > > + if (error != -EPROBE_DEFER) > > + dev_err(&client->dev, > > + "failed to request iovcc regulator: %d\n", error); > > Please use dev_err_probe instead. If this pattern is used for vcc-supply, maybe > change that too. Maybe also consider using a bulk regulator API. Dmitry seems is having something against it last time I remember it was discussed with him. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] Input: edt-ft5x06 - add support for iovcc-supply 2021-05-10 20:09 ` Andy Shevchenko @ 2021-05-10 20:17 ` Ondřej Jirman 0 siblings, 0 replies; 10+ messages in thread From: Ondřej Jirman @ 2021-05-10 20:17 UTC (permalink / raw) To: Andy Shevchenko Cc: Stephan Gerhold, Dmitry Torokhov, linux-input, Rob Herring, devicetree, Simon Budig, Marco Felsch On Mon, May 10, 2021 at 11:09:24PM +0300, Andy Shevchenko wrote: > On Mon, May 10, 2021 at 09:48:48PM +0200, Ondřej Jirman wrote: > > On Mon, May 10, 2021 at 09:31:08PM +0200, Stephan Gerhold wrote: > > > > + tsdata->iovcc = devm_regulator_get(&client->dev, "iovcc"); > > > + if (IS_ERR(tsdata->iovcc)) { > > > + error = PTR_ERR(tsdata->iovcc); > > > + if (error != -EPROBE_DEFER) > > > + dev_err(&client->dev, > > > + "failed to request iovcc regulator: %d\n", error); > > > > Please use dev_err_probe instead. If this pattern is used for vcc-supply, maybe > > change that too. Maybe also consider using a bulk regulator API. > > Dmitry seems is having something against it last time I remember it was > discussed with him. It basically does the same thing this does, except that you can figure out the failure later on from sysfs more easily just by looking at: /sys/kernel/debug/devices_deferred And you'll see the error message there to help you figure out the dependency that failed. What's to hate about this? :) kind regards, o. > -- > With Best Regards, > Andy Shevchenko > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] Input: edt-ft5x06 - add support for iovcc-supply 2021-05-10 19:48 ` Ondřej Jirman 2021-05-10 20:09 ` Andy Shevchenko @ 2021-05-10 20:16 ` Stephan Gerhold 2021-05-10 21:14 ` Ondřej Jirman 2021-05-11 7:21 ` Andy Shevchenko 1 sibling, 2 replies; 10+ messages in thread From: Stephan Gerhold @ 2021-05-10 20:16 UTC (permalink / raw) To: Ondřej Jirman Cc: Dmitry Torokhov, linux-input, Rob Herring, devicetree, Simon Budig, Andy Shevchenko, Marco Felsch Hi! On Mon, May 10, 2021 at 09:48:48PM +0200, Ondřej Jirman wrote: > Hello Stephan, > > On Mon, May 10, 2021 at 09:31:08PM +0200, Stephan Gerhold wrote: > > At the moment, the edt-ft5x06 driver can control a single regulator > > ("vcc"). However, some FocalTech touch controllers have an additional > > IOVCC pin that should be supplied with the digital I/O voltage. > > > > The I/O voltage might be provided by another regulator that should also > > be kept on. Otherwise, the touchscreen can randomly stop functioning if > > the regulator is turned off because no other components still require it. > > > > Implement (optional) support for also enabling an "iovcc-supply". > > The datasheet specifies a delay of ~ 10us before enabling VDD/VCC > > after IOVCC is enabled, so make sure to enable IOVCC first. > > > > Cc: Ondrej Jirman <megous@megous.com> > > Cc: Marco Felsch <m.felsch@pengutronix.de> > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net> > > --- > > Sorry about the long delay, couldn't find the time to test the new changes :) > > > > Changes in v2: > > - Respect 10us delay between enabling IOVCC and VDD/VCC line > > (suggested by Marco Felsch) > > > > v1: https://lore.kernel.org/linux-input/20210108192337.563679-2-stephan@gerhold.net/ > > --- > > drivers/input/touchscreen/edt-ft5x06.c | 34 ++++++++++++++++++++++++++ > > 1 file changed, 34 insertions(+) > > > > diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c > > index 2eefbc2485bc..d3271856bb5c 100644 > > --- a/drivers/input/touchscreen/edt-ft5x06.c > > +++ b/drivers/input/touchscreen/edt-ft5x06.c > > @@ -104,6 +104,7 @@ struct edt_ft5x06_ts_data { > > u16 num_x; > > u16 num_y; > > struct regulator *vcc; > > + struct regulator *iovcc; > > > > struct gpio_desc *reset_gpio; > > struct gpio_desc *wake_gpio; > > @@ -1067,6 +1068,7 @@ static void edt_ft5x06_disable_regulator(void *arg) > > struct edt_ft5x06_ts_data *data = arg; > > > > regulator_disable(data->vcc); > > + regulator_disable(data->iovcc); > > } > > > > static int edt_ft5x06_ts_probe(struct i2c_client *client, > > @@ -1107,9 +1109,28 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client, > > return error; > > } > > > > + tsdata->iovcc = devm_regulator_get(&client->dev, "iovcc"); > > + if (IS_ERR(tsdata->iovcc)) { > > + error = PTR_ERR(tsdata->iovcc); > > + if (error != -EPROBE_DEFER) > > + dev_err(&client->dev, > > + "failed to request iovcc regulator: %d\n", error); > > Please use dev_err_probe instead. If this pattern is used for vcc-supply, maybe > change that too. Maybe also consider using a bulk regulator API. > I had both of that in v1 but reverted both of that as discussed. I didn't make that very clear in the changelog, sorry about that. :) The reasons were: - Bulk regulator API: AFAICT there is no way to use it while also maintaining the correct enable/disable order plus the 10us delay. See https://lore.kernel.org/linux-input/X%2Fwj+bxe%2FIlznCj6@gerhold.net/ - dev_err_probe(): For some reason the patch set that converted a lot of input drivers (including edt-ft5x06) to dev_err_probe() was never applied: https://lore.kernel.org/linux-input/20200827185829.30096-12-krzk@kernel.org/ I dropped the change from my patch since Andy already mentioned a similar thing back then. Thanks! Stephan ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] Input: edt-ft5x06 - add support for iovcc-supply 2021-05-10 20:16 ` Stephan Gerhold @ 2021-05-10 21:14 ` Ondřej Jirman 2021-05-11 7:21 ` Andy Shevchenko 1 sibling, 0 replies; 10+ messages in thread From: Ondřej Jirman @ 2021-05-10 21:14 UTC (permalink / raw) To: Stephan Gerhold Cc: Dmitry Torokhov, linux-input, Rob Herring, devicetree, Simon Budig, Andy Shevchenko, Marco Felsch On Mon, May 10, 2021 at 10:16:41PM +0200, Stephan Gerhold wrote: > Hi! > > On Mon, May 10, 2021 at 09:48:48PM +0200, Ondřej Jirman wrote: > > Hello Stephan, > > > > On Mon, May 10, 2021 at 09:31:08PM +0200, Stephan Gerhold wrote: > > > At the moment, the edt-ft5x06 driver can control a single regulator > > > ("vcc"). However, some FocalTech touch controllers have an additional > > > IOVCC pin that should be supplied with the digital I/O voltage. > > > > > > The I/O voltage might be provided by another regulator that should also > > > be kept on. Otherwise, the touchscreen can randomly stop functioning if > > > the regulator is turned off because no other components still require it. > > > > > > Implement (optional) support for also enabling an "iovcc-supply". > > > The datasheet specifies a delay of ~ 10us before enabling VDD/VCC > > > after IOVCC is enabled, so make sure to enable IOVCC first. > > > > > > Cc: Ondrej Jirman <megous@megous.com> > > > Cc: Marco Felsch <m.felsch@pengutronix.de> > > > Signed-off-by: Stephan Gerhold <stephan@gerhold.net> > > > --- > > > Sorry about the long delay, couldn't find the time to test the new changes :) > > > > > > Changes in v2: > > > - Respect 10us delay between enabling IOVCC and VDD/VCC line > > > (suggested by Marco Felsch) > > > > > > v1: https://lore.kernel.org/linux-input/20210108192337.563679-2-stephan@gerhold.net/ > > > --- > > > drivers/input/touchscreen/edt-ft5x06.c | 34 ++++++++++++++++++++++++++ > > > 1 file changed, 34 insertions(+) > > > > > > diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c > > > index 2eefbc2485bc..d3271856bb5c 100644 > > > --- a/drivers/input/touchscreen/edt-ft5x06.c > > > +++ b/drivers/input/touchscreen/edt-ft5x06.c > > > @@ -104,6 +104,7 @@ struct edt_ft5x06_ts_data { > > > u16 num_x; > > > u16 num_y; > > > struct regulator *vcc; > > > + struct regulator *iovcc; > > > > > > struct gpio_desc *reset_gpio; > > > struct gpio_desc *wake_gpio; > > > @@ -1067,6 +1068,7 @@ static void edt_ft5x06_disable_regulator(void *arg) > > > struct edt_ft5x06_ts_data *data = arg; > > > > > > regulator_disable(data->vcc); > > > + regulator_disable(data->iovcc); > > > } > > > > > > static int edt_ft5x06_ts_probe(struct i2c_client *client, > > > @@ -1107,9 +1109,28 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client, > > > return error; > > > } > > > > > > + tsdata->iovcc = devm_regulator_get(&client->dev, "iovcc"); > > > + if (IS_ERR(tsdata->iovcc)) { > > > + error = PTR_ERR(tsdata->iovcc); > > > + if (error != -EPROBE_DEFER) > > > + dev_err(&client->dev, > > > + "failed to request iovcc regulator: %d\n", error); > > > > Please use dev_err_probe instead. If this pattern is used for vcc-supply, maybe > > change that too. Maybe also consider using a bulk regulator API. > > > > I had both of that in v1 but reverted both of that as discussed. > I didn't make that very clear in the changelog, sorry about that. :) > > The reasons were: > > - Bulk regulator API: AFAICT there is no way to use it while also > maintaining the correct enable/disable order plus the 10us delay. > See https://lore.kernel.org/linux-input/X%2Fwj+bxe%2FIlznCj6@gerhold.net/ > > - dev_err_probe(): For some reason the patch set that converted a lot of > input drivers (including edt-ft5x06) to dev_err_probe() was never applied: > https://lore.kernel.org/linux-input/20200827185829.30096-12-krzk@kernel.org/ > I dropped the change from my patch since Andy already mentioned > a similar thing back then. Thanks for explanation. regards, o. > Thanks! > Stephan ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] Input: edt-ft5x06 - add support for iovcc-supply 2021-05-10 20:16 ` Stephan Gerhold 2021-05-10 21:14 ` Ondřej Jirman @ 2021-05-11 7:21 ` Andy Shevchenko 2021-05-11 7:42 ` Marco Felsch 2021-05-11 8:50 ` Stephan Gerhold 1 sibling, 2 replies; 10+ messages in thread From: Andy Shevchenko @ 2021-05-11 7:21 UTC (permalink / raw) To: Stephan Gerhold Cc: Ondřej Jirman, Dmitry Torokhov, linux-input, Rob Herring, devicetree, Simon Budig, Marco Felsch On Mon, May 10, 2021 at 10:16:41PM +0200, Stephan Gerhold wrote: > On Mon, May 10, 2021 at 09:48:48PM +0200, Ondřej Jirman wrote: ... > The reasons were: > > - Bulk regulator API: AFAICT there is no way to use it while also > maintaining the correct enable/disable order plus the 10us delay. > See https://lore.kernel.org/linux-input/X%2Fwj+bxe%2FIlznCj6@gerhold.net/ This by the way can be fixed on regulator level (adding some like ranges into bulk structure with timeouts, and if 0, skip them). > - dev_err_probe(): For some reason the patch set that converted a lot of > input drivers (including edt-ft5x06) to dev_err_probe() was never applied: > https://lore.kernel.org/linux-input/20200827185829.30096-12-krzk@kernel.org/ > I dropped the change from my patch since Andy already mentioned > a similar thing back then. This question to Dmitry, because I don't remember any good argument why he doesn't like it. Maybe he can refresh our memories by providing it again. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] Input: edt-ft5x06 - add support for iovcc-supply 2021-05-11 7:21 ` Andy Shevchenko @ 2021-05-11 7:42 ` Marco Felsch 2021-05-11 8:50 ` Stephan Gerhold 1 sibling, 0 replies; 10+ messages in thread From: Marco Felsch @ 2021-05-11 7:42 UTC (permalink / raw) To: Andy Shevchenko Cc: Stephan Gerhold, Ondřej Jirman, Dmitry Torokhov, linux-input, Rob Herring, devicetree, Simon Budig On 21-05-11 10:21, Andy Shevchenko wrote: > On Mon, May 10, 2021 at 10:16:41PM +0200, Stephan Gerhold wrote: > > On Mon, May 10, 2021 at 09:48:48PM +0200, Ondřej Jirman wrote: > > ... > > > The reasons were: > > > > - Bulk regulator API: AFAICT there is no way to use it while also > > maintaining the correct enable/disable order plus the 10us delay. > > See https://lore.kernel.org/linux-input/X%2Fwj+bxe%2FIlznCj6@gerhold.net/ > > This by the way can be fixed on regulator level (adding some like ranges into > bulk structure with timeouts, and if 0, skip them). I would appreciate this :) Regards, Marco ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] Input: edt-ft5x06 - add support for iovcc-supply 2021-05-11 7:21 ` Andy Shevchenko 2021-05-11 7:42 ` Marco Felsch @ 2021-05-11 8:50 ` Stephan Gerhold 1 sibling, 0 replies; 10+ messages in thread From: Stephan Gerhold @ 2021-05-11 8:50 UTC (permalink / raw) To: Andy Shevchenko Cc: Ondřej Jirman, Dmitry Torokhov, linux-input, Rob Herring, devicetree, Simon Budig, Marco Felsch On Tue, May 11, 2021 at 10:21:27AM +0300, Andy Shevchenko wrote: > On Mon, May 10, 2021 at 10:16:41PM +0200, Stephan Gerhold wrote: > > On Mon, May 10, 2021 at 09:48:48PM +0200, Ondřej Jirman wrote: > > > > - Bulk regulator API: AFAICT there is no way to use it while also > > maintaining the correct enable/disable order plus the 10us delay. > > See https://lore.kernel.org/linux-input/X%2Fwj+bxe%2FIlznCj6@gerhold.net/ > > This by the way can be fixed on regulator level (adding some like ranges into > bulk structure with timeouts, and if 0, skip them). > At the moment the bulk regulator API seems specifically designed to enable all the regulators at the same time (with some funky asynchronous scheduling code). I'm not sure if there is a straightforward way to fit in a sequential enable/disable order with potential delays. I'm also not entirely convinced it's worth it in this case. I would say the code in this patch (except for the dev_err_probe()) is still quite easy to read. Encoding the enable/disable order + delays in some bulk regulator struct might actually be more difficult to read. Thanks, Stephan ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-05-11 8:50 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-10 19:31 [PATCH v2 1/2] dt-bindings: input: touchscreen: edt-ft5x06: add iovcc-supply Stephan Gerhold 2021-05-10 19:31 ` [PATCH v2 2/2] Input: edt-ft5x06 - add support for iovcc-supply Stephan Gerhold 2021-05-10 19:48 ` Ondřej Jirman 2021-05-10 20:09 ` Andy Shevchenko 2021-05-10 20:17 ` Ondřej Jirman 2021-05-10 20:16 ` Stephan Gerhold 2021-05-10 21:14 ` Ondřej Jirman 2021-05-11 7:21 ` Andy Shevchenko 2021-05-11 7:42 ` Marco Felsch 2021-05-11 8:50 ` Stephan Gerhold
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.