| Message ID | 20190309151853.3140-1-axel.lin@ingics.com |
|---|---|
| State | New |
| Headers | show |
| Series | gpio: adnp: Fix testing wrong value in adnp_gpio_direction_input | expand |
On Sat, Mar 09, 2019 at 11:18:53PM +0800, Axel Lin wrote: > Current code test wrong value so it does not verify if the written > data is correctly read back. Fix it. > > Signed-off-by: Axel Lin <axel.lin@ingics.com> > --- > BTW, I'm wondering why it needs to read back the written vlaue for double > checking if write is success or not. > Without any comment, it looks like a leftover of testing code. > Does it make sense to remove the read after write? The purpose of this was to support read-only and write-only pins in the controller. The controller would not allow a bit to be set (or cleared) if the pin was read- or write-only. And yes, that would've probably deserved a comment. > drivers/gpio/gpio-adnp.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpio/gpio-adnp.c b/drivers/gpio/gpio-adnp.c > index 91b90c0cea73..d334dc302dac 100644 > --- a/drivers/gpio/gpio-adnp.c > +++ b/drivers/gpio/gpio-adnp.c > @@ -132,8 +132,10 @@ static int adnp_gpio_direction_input(struct gpio_chip *chip, unsigned offset) > if (err < 0) > goto out; > > - if (err & BIT(pos)) > + if (value & BIT(pos)) { > err = -EACCES; > + goto out; > + } Perhaps while at it also make this return -EPERM just like it done for adnp_gpio_direction_output()? With that: Reviewed-by: Thierry Reding <thierry.reding@gmail.com>
diff --git a/drivers/gpio/gpio-adnp.c b/drivers/gpio/gpio-adnp.c index 91b90c0cea73..d334dc302dac 100644 --- a/drivers/gpio/gpio-adnp.c +++ b/drivers/gpio/gpio-adnp.c @@ -132,8 +132,10 @@ static int adnp_gpio_direction_input(struct gpio_chip *chip, unsigned offset) if (err < 0) goto out; - if (err & BIT(pos)) + if (value & BIT(pos)) { err = -EACCES; + goto out; + } err = 0;
Current code test wrong value so it does not verify if the written data is correctly read back. Fix it. Signed-off-by: Axel Lin <axel.lin@ingics.com> --- BTW, I'm wondering why it needs to read back the written vlaue for double checking if write is success or not. Without any comment, it looks like a leftover of testing code. Does it make sense to remove the read after write? drivers/gpio/gpio-adnp.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)