| Message ID | 1611002626-5889-10-git-send-email-jeff@labundy.com |
|---|---|
| State | New |
| Headers | show |
| Series | input: iqs5xx: Minor enhancements and optimizations | expand |
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
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 --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);
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(-)