| Message ID | 1480438754-14794-2-git-send-email-florian.vaussard@heig-vd.ch (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Tue, Nov 29, 2016 at 05:59:13PM +0100, Florian Vaussard wrote: > Using regmap_update_bits(..., mask, 1) with 'mask' following (1 << k) > and k greater than 0 is wrong. Indeed, _regmap_update_bits will perform > (mask & 1), which results in 0 if LSB of mask is 0. Thus the call > regmap_update_bits(..., mask, 1) is in reality equivalent to > regmap_update_bits(..., mask, 0). > > In such a case, the correct use is regmap_update_bits(..., mask, mask). > > This driver is performing such a mistake with the DRV2665_STANDBY mask, > which equals BIT(6). Fix the driver to make it consistent with the API, > and fix the alignment problem at the same time. Please note that this > change is untested, as I do not have this piece of hardware. Testers > are welcome! > > Signed-off-by: Florian Vaussard <florian.vaussard@heig-vd.ch> Applied, thank you. > --- > drivers/input/misc/drv2665.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/input/misc/drv2665.c b/drivers/input/misc/drv2665.c > index ef9bc12..b0f46e7 100644 > --- a/drivers/input/misc/drv2665.c > +++ b/drivers/input/misc/drv2665.c > @@ -126,7 +126,8 @@ static void drv2665_close(struct input_dev *input) > cancel_work_sync(&haptics->work); > > error = regmap_update_bits(haptics->regmap, > - DRV2665_CTRL_2, DRV2665_STANDBY, 1); > + DRV2665_CTRL_2, DRV2665_STANDBY, > + DRV2665_STANDBY); > if (error) > dev_err(&haptics->client->dev, > "Failed to enter standby mode: %d\n", error); > @@ -240,7 +241,7 @@ static int __maybe_unused drv2665_suspend(struct device *dev) > > if (haptics->input_dev->users) { > ret = regmap_update_bits(haptics->regmap, DRV2665_CTRL_2, > - DRV2665_STANDBY, 1); > + DRV2665_STANDBY, DRV2665_STANDBY); > if (ret) { > dev_err(dev, "Failed to set standby mode\n"); > regulator_disable(haptics->regulator); > -- > 2.5.5 >
diff --git a/drivers/input/misc/drv2665.c b/drivers/input/misc/drv2665.c index ef9bc12..b0f46e7 100644 --- a/drivers/input/misc/drv2665.c +++ b/drivers/input/misc/drv2665.c @@ -126,7 +126,8 @@ static void drv2665_close(struct input_dev *input) cancel_work_sync(&haptics->work); error = regmap_update_bits(haptics->regmap, - DRV2665_CTRL_2, DRV2665_STANDBY, 1); + DRV2665_CTRL_2, DRV2665_STANDBY, + DRV2665_STANDBY); if (error) dev_err(&haptics->client->dev, "Failed to enter standby mode: %d\n", error); @@ -240,7 +241,7 @@ static int __maybe_unused drv2665_suspend(struct device *dev) if (haptics->input_dev->users) { ret = regmap_update_bits(haptics->regmap, DRV2665_CTRL_2, - DRV2665_STANDBY, 1); + DRV2665_STANDBY, DRV2665_STANDBY); if (ret) { dev_err(dev, "Failed to set standby mode\n"); regulator_disable(haptics->regulator);
Using regmap_update_bits(..., mask, 1) with 'mask' following (1 << k) and k greater than 0 is wrong. Indeed, _regmap_update_bits will perform (mask & 1), which results in 0 if LSB of mask is 0. Thus the call regmap_update_bits(..., mask, 1) is in reality equivalent to regmap_update_bits(..., mask, 0). In such a case, the correct use is regmap_update_bits(..., mask, mask). This driver is performing such a mistake with the DRV2665_STANDBY mask, which equals BIT(6). Fix the driver to make it consistent with the API, and fix the alignment problem at the same time. Please note that this change is untested, as I do not have this piece of hardware. Testers are welcome! Signed-off-by: Florian Vaussard <florian.vaussard@heig-vd.ch> --- drivers/input/misc/drv2665.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)