diff mbox series

[09/10] input: iqs5xx: Make reset GPIO optional

Message ID 1611002626-5889-10-git-send-email-jeff@labundy.com
State New
Headers show
Series input: iqs5xx: Minor enhancements and optimizations | expand

Commit Message

Jeff LaBundy Jan. 18, 2021, 8:43 p.m. UTC
The device's hardware reset pin is only required if the platform
must be able to update the device's firmware on the fly.

As such, demote the reset GPIO to optional in support of devices
that ship with pre-programmed firmware and don't route the reset
pin back to the SOC.

If user space attempts to push updated firmware which would rely
upon the reset pin to wake the bootloader, attempts to reach the
bootloader are simply NAK'd and the device resumes normally.

Signed-off-by: Jeff LaBundy <jeff@labundy.com>
---
 drivers/input/touchscreen/iqs5xx.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Dmitry Torokhov Jan. 25, 2021, 4:43 a.m. UTC | #1
On Mon, Jan 18, 2021 at 02:43:45PM -0600, Jeff LaBundy wrote:
> The device's hardware reset pin is only required if the platform

> must be able to update the device's firmware on the fly.

> 

> As such, demote the reset GPIO to optional in support of devices

> that ship with pre-programmed firmware and don't route the reset

> pin back to the SOC.

> 

> If user space attempts to push updated firmware which would rely

> upon the reset pin to wake the bootloader, attempts to reach the

> bootloader are simply NAK'd and the device resumes normally.


Can we maybe make the firmware attribute invisible in this case? Or
return early instead of failing to enter bootloader mode?

Thanks.

-- 
Dmitry
Jeff LaBundy Jan. 26, 2021, 3:10 a.m. UTC | #2
Hi Dmitry,

On Sun, Jan 24, 2021 at 08:43:46PM -0800, Dmitry Torokhov wrote:
> On Mon, Jan 18, 2021 at 02:43:45PM -0600, Jeff LaBundy wrote:

> > The device's hardware reset pin is only required if the platform

> > must be able to update the device's firmware on the fly.

> > 

> > As such, demote the reset GPIO to optional in support of devices

> > that ship with pre-programmed firmware and don't route the reset

> > pin back to the SOC.

> > 

> > If user space attempts to push updated firmware which would rely

> > upon the reset pin to wake the bootloader, attempts to reach the

> > bootloader are simply NAK'd and the device resumes normally.

> 

> Can we maybe make the firmware attribute invisible in this case? Or

> return early instead of failing to enter bootloader mode?


I almost sent the second alternative, but instead I liked the idea of
restricting the check for reset_gpio to the only method that actually
uses it. That way, nothing outside of iqs5xx_reset() has to check for
the GPIO and can just fail gracefully in its absence.

That being said, either of your suggestions work just as well; let me
take another stab at it.

> 

> Thanks.

> 

> -- 

> Dmitry


Kind regards,
Jeff LaBundy
diff mbox series

Patch

diff --git a/drivers/input/touchscreen/iqs5xx.c b/drivers/input/touchscreen/iqs5xx.c
index babd1f1..dac132b 100644
--- a/drivers/input/touchscreen/iqs5xx.c
+++ b/drivers/input/touchscreen/iqs5xx.c
@@ -242,6 +242,9 @@  static void iqs5xx_reset(struct i2c_client *client)
 {
 	struct iqs5xx_private *iqs5xx = i2c_get_clientdata(client);
 
+	if (!iqs5xx->reset_gpio)
+		return;
+
 	gpiod_set_value_cansleep(iqs5xx->reset_gpio, 1);
 	usleep_range(200, 300);
 
@@ -1045,8 +1048,8 @@  static int iqs5xx_probe(struct i2c_client *client,
 	i2c_set_clientdata(client, iqs5xx);
 	iqs5xx->client = client;
 
-	iqs5xx->reset_gpio = devm_gpiod_get(&client->dev,
-					    "reset", GPIOD_OUT_LOW);
+	iqs5xx->reset_gpio = devm_gpiod_get_optional(&client->dev,
+						     "reset", GPIOD_OUT_LOW);
 	if (IS_ERR(iqs5xx->reset_gpio)) {
 		error = PTR_ERR(iqs5xx->reset_gpio);
 		dev_err(&client->dev, "Failed to request GPIO: %d\n", error);