LinuxLists.cc - [PATCH 0/2] Input: cyttsp5

2022-11-17 19:29:29

by Lin, Meng-Bo

[permalink] [raw]

Subject: [PATCH 0/2] Input: cyttsp5 - add vddio regulator

2022-11-17 19:29:44

by Lin, Meng-Bo

[permalink] [raw]

Subject: [PATCH 1/2] dt-bindings: input: cyttsp5: document vddio-supply

2022-11-17 19:30:06

by Lin, Meng-Bo

[permalink] [raw]

Subject: [PATCH 2/2] Input: cyttsp5 - add vddio regulator

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 <[email protected]>
---
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

2022-11-17 21:27:31

by Christophe JAILLET

[permalink] [raw]

Subject: Re: [PATCH 2/2] Input: cyttsp5 - add vddio regulator

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/[email protected]>
> ---
> 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) {

2022-11-17 22:35:16

by Dmitry Torokhov

[permalink] [raw]

Subject: Re: [PATCH 2/2] Input: cyttsp5 - add vddio regulator

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/[email protected]>
> > ---
> > 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

2022-11-17 22:37:18

by Dmitry Torokhov

[permalink] [raw]

Subject: Re: [PATCH 2/2] Input: cyttsp5 - add vddio regulator

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

2022-11-17 22:51:46

by Lin, Meng-Bo

[permalink] [raw]

Subject: Re: [PATCH 2/2] Input: cyttsp5 - add vddio regulator

Hi Dmitry,

> This needs binding update.

Please have a look at the binding update in
https://lore.kernel.org/all/[email protected]/
if you don't receive it.

Regards,
Lin

2022-11-18 08:18:29

by Krzysztof Kozlowski

[permalink] [raw]

Subject: Re: [PATCH 1/2] dt-bindings: input: cyttsp5: document vddio-supply

On 17/11/2022 20:05, Lin, Meng-Bo wrote:
> The Samsung touchscreen controllers are often used with external pull-up

Samsung or Cypress?

> 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.
>
> Document support for a "vddio-supply" that is enabled by the cyttsp5
> driver so that the regulator gets enabled when needed.
>

Acked-by: Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof

2022-11-19 00:04:19

by Alistair Francis

[permalink] [raw]

Subject: Re: [PATCH 1/2] dt-bindings: input: cyttsp5: document vddio-supply

2022-11-19 00:39:01

by Alistair Francis

[permalink] [raw]

Subject: Re: [PATCH 2/2] Input: cyttsp5 - add vddio regulator

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 <[email protected]>

Acked-by: Alistair Francis <[email protected]>

> ---
> 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
>
>
>