| Message ID | 20250204190100.2210863-1-hugo@hugovil.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Input: cyttsp5 - ensure minimum reset pulse width | expand |
On Wed, 5 Feb 2025, at 5:01 AM, Hugo Villeneuve wrote: > From: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > The current reset pulse width is measured to be 5us on a > Renesas RZ/G2L SOM. The manufacturer's minimum reset pulse width is > specified as 10us. > > Extend reset pulse width to make sure it is long enough on all platforms. > > Also reword confusing comments about reset pin assertion. > > Fixes: 5b0c03e24a06 ("Input: Add driver for Cypress Generation 5 touchscreen") > Cc: <stable@vger.kernel.org> > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> Acked-by: Alistair Francis <alistair@alistair23.me> > --- > drivers/input/touchscreen/cyttsp5.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/input/touchscreen/cyttsp5.c b/drivers/input/touchscreen/cyttsp5.c > index eafe5a9b8964..bb09e84d0e92 100644 > --- a/drivers/input/touchscreen/cyttsp5.c > +++ b/drivers/input/touchscreen/cyttsp5.c > @@ -870,13 +870,16 @@ static int cyttsp5_probe(struct device *dev, struct regmap *regmap, int irq, > ts->input->phys = ts->phys; > input_set_drvdata(ts->input, ts); > > - /* Reset the gpio to be in a reset state */ > + /* Assert gpio to be in a reset state */ > ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); > if (IS_ERR(ts->reset_gpio)) { > error = PTR_ERR(ts->reset_gpio); > dev_err(dev, "Failed to request reset gpio, error %d\n", error); > return error; > } > + > + fsleep(1000); /* Ensure long-enough reset pulse (minimum 10us). */ > + > gpiod_set_value_cansleep(ts->reset_gpio, 0); > > /* Need a delay to have device up */ > > base-commit: 0de63bb7d91975e73338300a57c54b93d3cc151c > -- > 2.39.5 > >
On Tue, Feb 04, 2025 at 02:01:00PM -0500, Hugo Villeneuve wrote: > From: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > The current reset pulse width is measured to be 5us on a > Renesas RZ/G2L SOM. The manufacturer's minimum reset pulse width is > specified as 10us. > > Extend reset pulse width to make sure it is long enough on all platforms. > > Also reword confusing comments about reset pin assertion. > > Fixes: 5b0c03e24a06 ("Input: Add driver for Cypress Generation 5 touchscreen") > Cc: <stable@vger.kernel.org> > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> > --- > drivers/input/touchscreen/cyttsp5.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/input/touchscreen/cyttsp5.c b/drivers/input/touchscreen/cyttsp5.c > index eafe5a9b8964..bb09e84d0e92 100644 > --- a/drivers/input/touchscreen/cyttsp5.c > +++ b/drivers/input/touchscreen/cyttsp5.c > @@ -870,13 +870,16 @@ static int cyttsp5_probe(struct device *dev, struct regmap *regmap, int irq, > ts->input->phys = ts->phys; > input_set_drvdata(ts->input, ts); > > - /* Reset the gpio to be in a reset state */ > + /* Assert gpio to be in a reset state */ > ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); > if (IS_ERR(ts->reset_gpio)) { > error = PTR_ERR(ts->reset_gpio); > dev_err(dev, "Failed to request reset gpio, error %d\n", error); > return error; > } > + > + fsleep(1000); /* Ensure long-enough reset pulse (minimum 10us). */ > + If the manufacturer specified that 10us is enough why do we want to wait 100 times longer? Thanks.
On Wed, 5 Feb 2025 07:33:36 -0800 Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > On Tue, Feb 04, 2025 at 02:01:00PM -0500, Hugo Villeneuve wrote: > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > > > The current reset pulse width is measured to be 5us on a > > Renesas RZ/G2L SOM. The manufacturer's minimum reset pulse width is > > specified as 10us. > > > > Extend reset pulse width to make sure it is long enough on all platforms. > > > > Also reword confusing comments about reset pin assertion. > > > > Fixes: 5b0c03e24a06 ("Input: Add driver for Cypress Generation 5 touchscreen") > > Cc: <stable@vger.kernel.org> > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > --- > > drivers/input/touchscreen/cyttsp5.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/input/touchscreen/cyttsp5.c b/drivers/input/touchscreen/cyttsp5.c > > index eafe5a9b8964..bb09e84d0e92 100644 > > --- a/drivers/input/touchscreen/cyttsp5.c > > +++ b/drivers/input/touchscreen/cyttsp5.c > > @@ -870,13 +870,16 @@ static int cyttsp5_probe(struct device *dev, struct regmap *regmap, int irq, > > ts->input->phys = ts->phys; > > input_set_drvdata(ts->input, ts); > > > > - /* Reset the gpio to be in a reset state */ > > + /* Assert gpio to be in a reset state */ > > ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); > > if (IS_ERR(ts->reset_gpio)) { > > error = PTR_ERR(ts->reset_gpio); > > dev_err(dev, "Failed to request reset gpio, error %d\n", error); > > return error; > > } > > + > > + fsleep(1000); /* Ensure long-enough reset pulse (minimum 10us). */ > > + > > If the manufacturer specified that 10us is enough why do we want to wait > 100 times longer? Hi, 10us would do fine. I simply put 1ms because it is then much easier to see the reset pulse on an oscilloscope when correlating it with other signals. Hugo.
diff --git a/drivers/input/touchscreen/cyttsp5.c b/drivers/input/touchscreen/cyttsp5.c index eafe5a9b8964..bb09e84d0e92 100644 --- a/drivers/input/touchscreen/cyttsp5.c +++ b/drivers/input/touchscreen/cyttsp5.c @@ -870,13 +870,16 @@ static int cyttsp5_probe(struct device *dev, struct regmap *regmap, int irq, ts->input->phys = ts->phys; input_set_drvdata(ts->input, ts); - /* Reset the gpio to be in a reset state */ + /* Assert gpio to be in a reset state */ ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); if (IS_ERR(ts->reset_gpio)) { error = PTR_ERR(ts->reset_gpio); dev_err(dev, "Failed to request reset gpio, error %d\n", error); return error; } + + fsleep(1000); /* Ensure long-enough reset pulse (minimum 10us). */ + gpiod_set_value_cansleep(ts->reset_gpio, 0); /* Need a delay to have device up */