* Re: [PATCH 2/2] Input: cyttsp5 - add vddio regulator
2022-11-17 19:05 ` [PATCH 2/2] Input: cyttsp5 - add vddio regulator Lin, Meng-Bo
@ 2022-11-17 21:16 ` Christophe JAILLET
2022-11-17 22:15 ` Dmitry Torokhov
2022-11-17 22:17 ` Dmitry Torokhov
2022-11-18 23:29 ` Alistair
2 siblings, 1 reply; 10+ messages in thread
From: Christophe JAILLET @ 2022-11-17 21:16 UTC (permalink / raw)
To: linmengbo0689
Cc: alistair, devicetree, dmitry.torokhov, krzysztof.kozlowski+dt,
linus.walleij, linux-input, linux-kernel, robh+dt
Le 17/11/2022 à 20:05, Lin, Meng-Bo a écrit :
> The Samsung touchscreen controllers are often used with external pull-up
> for the interrupt line and the I2C lines, so we might need to enable
> a regulator to bring the lines into usable state. Otherwise, this might
> cause spurious interrupts and reading from I2C will fail.
>
> Implement support for a "vddio-supply" that is enabled by the cyttsp5
> driver so that the regulator gets enabled when needed.
>
> Signed-off-by: Lin, Meng-Bo <linmengbo0689-g/b1ySJe57IN+BqQ9rBEUg@public.gmane.org>
> ---
> drivers/input/touchscreen/cyttsp5.c | 19 ++++++++++++-------
> 1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/input/touchscreen/cyttsp5.c b/drivers/input/touchscreen/cyttsp5.c
> index 24ab1df9fc07..d02fdb940edf 100644
> --- a/drivers/input/touchscreen/cyttsp5.c
> +++ b/drivers/input/touchscreen/cyttsp5.c
> @@ -190,7 +190,7 @@ struct cyttsp5 {
> int num_prv_rec;
> struct regmap *regmap;
> struct touchscreen_properties prop;
> - struct regulator *vdd;
> + struct regulator_bulk_data supplies[2];
> };
>
> /*
> @@ -767,7 +767,7 @@ static void cyttsp5_cleanup(void *data)
> {
> struct cyttsp5 *ts = data;
>
> - regulator_disable(ts->vdd);
> + regulator_bulk_disable(ARRAY_SIZE(ts->supplies), ts->supplies);
> }
>
> static int cyttsp5_probe(struct device *dev, struct regmap *regmap, int irq,
> @@ -790,9 +790,12 @@ static int cyttsp5_probe(struct device *dev, struct regmap *regmap, int irq,
> init_completion(&ts->cmd_done);
>
> /* Power up the device */
> - ts->vdd = devm_regulator_get(dev, "vdd");
> - if (IS_ERR(ts->vdd)) {
> - error = PTR_ERR(ts->vdd);
> + ts->supplies[0].supply = "vdd";
> + ts->supplies[1].supply = "vddio";
> + error = devm_regulator_bulk_get(dev, ARRAY_SIZE(ts->supplies),
> + ts->supplies);
> + if (error < 0) {
> + dev_err(ts->dev, "Failed to get regulators, error %d\n", error);
Hi,
dev_err_probe()?
I think that devm_regulator_bulk_get() can return -EPROBE_DEFER;
> return error;
> }
>
> @@ -800,9 +803,11 @@ static int cyttsp5_probe(struct device *dev, struct regmap *regmap, int irq,
> if (error)
> return error;
>
> - error = regulator_enable(ts->vdd);
> - if (error)
> + error = regulator_bulk_enable(ARRAY_SIZE(ts->supplies), ts->supplies);
> + if (error < 0) {
> + dev_err(ts->dev, "Failed to enable regulators, error %d\n", error);
Eventually, the same here in order to be consistent.
CJ
> return error;
> + }
>
> ts->input = devm_input_allocate_device(dev);
> if (!ts->input) {
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 2/2] Input: cyttsp5 - add vddio regulator
2022-11-17 21:16 ` Christophe JAILLET
@ 2022-11-17 22:15 ` Dmitry Torokhov
0 siblings, 0 replies; 10+ messages in thread
From: Dmitry Torokhov @ 2022-11-17 22:15 UTC (permalink / raw)
To: Christophe JAILLET
Cc: linmengbo0689, alistair, devicetree, krzysztof.kozlowski+dt,
linus.walleij, linux-input, linux-kernel, robh+dt
On Thu, Nov 17, 2022 at 10:16:40PM +0100, Christophe JAILLET wrote:
> Le 17/11/2022 à 20:05, Lin, Meng-Bo a écrit :
> > The Samsung touchscreen controllers are often used with external pull-up
> > for the interrupt line and the I2C lines, so we might need to enable
> > a regulator to bring the lines into usable state. Otherwise, this might
> > cause spurious interrupts and reading from I2C will fail.
> >
> > Implement support for a "vddio-supply" that is enabled by the cyttsp5
> > driver so that the regulator gets enabled when needed.
> >
> > Signed-off-by: Lin, Meng-Bo <linmengbo0689-g/b1ySJe57IN+BqQ9rBEUg@public.gmane.org>
> > ---
> > drivers/input/touchscreen/cyttsp5.c | 19 ++++++++++++-------
> > 1 file changed, 12 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/input/touchscreen/cyttsp5.c b/drivers/input/touchscreen/cyttsp5.c
> > index 24ab1df9fc07..d02fdb940edf 100644
> > --- a/drivers/input/touchscreen/cyttsp5.c
> > +++ b/drivers/input/touchscreen/cyttsp5.c
> > @@ -190,7 +190,7 @@ struct cyttsp5 {
> > int num_prv_rec;
> > struct regmap *regmap;
> > struct touchscreen_properties prop;
> > - struct regulator *vdd;
> > + struct regulator_bulk_data supplies[2];
> > };
> > /*
> > @@ -767,7 +767,7 @@ static void cyttsp5_cleanup(void *data)
> > {
> > struct cyttsp5 *ts = data;
> > - regulator_disable(ts->vdd);
> > + regulator_bulk_disable(ARRAY_SIZE(ts->supplies), ts->supplies);
> > }
> > static int cyttsp5_probe(struct device *dev, struct regmap *regmap, int irq,
> > @@ -790,9 +790,12 @@ static int cyttsp5_probe(struct device *dev, struct regmap *regmap, int irq,
> > init_completion(&ts->cmd_done);
> > /* Power up the device */
> > - ts->vdd = devm_regulator_get(dev, "vdd");
> > - if (IS_ERR(ts->vdd)) {
> > - error = PTR_ERR(ts->vdd);
> > + ts->supplies[0].supply = "vdd";
> > + ts->supplies[1].supply = "vddio";
> > + error = devm_regulator_bulk_get(dev, ARRAY_SIZE(ts->supplies),
> > + ts->supplies);
> > + if (error < 0) {
> > + dev_err(ts->dev, "Failed to get regulators, error %d\n", error);
>
> Hi,
>
> dev_err_probe()?
> I think that devm_regulator_bulk_get() can return -EPROBE_DEFER;
No, I'd rather we avoid dev_err_probe().
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] Input: cyttsp5 - add vddio regulator
2022-11-17 19:05 ` [PATCH 2/2] Input: cyttsp5 - add vddio regulator Lin, Meng-Bo
2022-11-17 21:16 ` Christophe JAILLET
@ 2022-11-17 22:17 ` Dmitry Torokhov
2022-11-17 22:39 ` Lin, Meng-Bo
2022-11-18 23:29 ` Alistair
2 siblings, 1 reply; 10+ messages in thread
From: Dmitry Torokhov @ 2022-11-17 22:17 UTC (permalink / raw)
To: Lin, Meng-Bo
Cc: linux-input, Rob Herring, Krzysztof Kozlowski, Linus Walleij,
Alistair Francis, devicetree, linux-kernel
On Thu, Nov 17, 2022 at 07:05:41PM +0000, Lin, Meng-Bo wrote:
> The Samsung touchscreen controllers are often used with external pull-up
> for the interrupt line and the I2C lines, so we might need to enable
> a regulator to bring the lines into usable state. Otherwise, this might
> cause spurious interrupts and reading from I2C will fail.
>
> Implement support for a "vddio-supply" that is enabled by the cyttsp5
> driver so that the regulator gets enabled when needed.
This needs binding update.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] Input: cyttsp5 - add vddio regulator
2022-11-17 22:17 ` Dmitry Torokhov
@ 2022-11-17 22:39 ` Lin, Meng-Bo
0 siblings, 0 replies; 10+ messages in thread
From: Lin, Meng-Bo @ 2022-11-17 22:39 UTC (permalink / raw)
To: dmitry.torokhov
Cc: alistair, devicetree, krzysztof.kozlowski+dt, linmengbo0689,
linus.walleij, linux-input, linux-kernel, robh+dt
Hi Dmitry,
> This needs binding update.
Please have a look at the binding update in
https://lore.kernel.org/all/20221117190507.87535-2-linmengbo0689@protonmail.com/
if you don't receive it.
Regards,
Lin
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] Input: cyttsp5 - add vddio regulator
2022-11-17 19:05 ` [PATCH 2/2] Input: cyttsp5 - add vddio regulator Lin, Meng-Bo
2022-11-17 21:16 ` Christophe JAILLET
2022-11-17 22:17 ` Dmitry Torokhov
@ 2022-11-18 23:29 ` Alistair
2 siblings, 0 replies; 10+ messages in thread
From: Alistair @ 2022-11-18 23:29 UTC (permalink / raw)
To: Lin, Meng-Bo, linux-input
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Linus Walleij,
devicetree, Linux Kernel Mailing List
On Fri, 18 Nov 2022, at 5:05 AM, Lin, Meng-Bo wrote:
> The Samsung touchscreen controllers are often used with external pull-up
> for the interrupt line and the I2C lines, so we might need to enable
> a regulator to bring the lines into usable state. Otherwise, this might
> cause spurious interrupts and reading from I2C will fail.
>
> Implement support for a "vddio-supply" that is enabled by the cyttsp5
> driver so that the regulator gets enabled when needed.
>
> Signed-off-by: Lin, Meng-Bo <linmengbo0689@protonmail.com>
Acked-by: Alistair Francis <alistair@alistair23.me>
> ---
> drivers/input/touchscreen/cyttsp5.c | 19 ++++++++++++-------
> 1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/input/touchscreen/cyttsp5.c b/drivers/input/touchscreen/cyttsp5.c
> index 24ab1df9fc07..d02fdb940edf 100644
> --- a/drivers/input/touchscreen/cyttsp5.c
> +++ b/drivers/input/touchscreen/cyttsp5.c
> @@ -190,7 +190,7 @@ struct cyttsp5 {
> int num_prv_rec;
> struct regmap *regmap;
> struct touchscreen_properties prop;
> - struct regulator *vdd;
> + struct regulator_bulk_data supplies[2];
> };
>
> /*
> @@ -767,7 +767,7 @@ static void cyttsp5_cleanup(void *data)
> {
> struct cyttsp5 *ts = data;
>
> - regulator_disable(ts->vdd);
> + regulator_bulk_disable(ARRAY_SIZE(ts->supplies), ts->supplies);
> }
>
> static int cyttsp5_probe(struct device *dev, struct regmap *regmap, int irq,
> @@ -790,9 +790,12 @@ static int cyttsp5_probe(struct device *dev, struct regmap *regmap, int irq,
> init_completion(&ts->cmd_done);
>
> /* Power up the device */
> - ts->vdd = devm_regulator_get(dev, "vdd");
> - if (IS_ERR(ts->vdd)) {
> - error = PTR_ERR(ts->vdd);
> + ts->supplies[0].supply = "vdd";
> + ts->supplies[1].supply = "vddio";
> + error = devm_regulator_bulk_get(dev, ARRAY_SIZE(ts->supplies),
> + ts->supplies);
> + if (error < 0) {
> + dev_err(ts->dev, "Failed to get regulators, error %d\n", error);
> return error;
> }
>
> @@ -800,9 +803,11 @@ static int cyttsp5_probe(struct device *dev, struct regmap *regmap, int irq,
> if (error)
> return error;
>
> - error = regulator_enable(ts->vdd);
> - if (error)
> + error = regulator_bulk_enable(ARRAY_SIZE(ts->supplies), ts->supplies);
> + if (error < 0) {
> + dev_err(ts->dev, "Failed to enable regulators, error %d\n", error);
> return error;
> + }
>
> ts->input = devm_input_allocate_device(dev);
> if (!ts->input) {
> --
> 2.30.2
>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread