* [PATCH] pwm: pca9685: Fix wrong argument to set MODE1_SLEEP bit @ 2013-06-19 17:27 Axel Lin 2013-06-26 9:51 ` Thierry Reding 2013-06-26 21:33 ` Thierry Reding 0 siblings, 2 replies; 4+ messages in thread From: Axel Lin @ 2013-06-19 17:27 UTC (permalink / raw) To: Thierry Reding; +Cc: Steffen Trumtrar, linux-pwm Current code actually does not set MODE1_SLEEP bit because the new value for bitmask (0x1) is wrong. To set MODE1_SLEEP bit, we should pass MODE1_SLEEP as the new value for bitmask. Signed-off-by: Axel Lin <axel.lin@ingics.com> --- Hi Steffen, I don't have this hardware, can you test if this patch works? Thanks, Axel drivers/pwm/pwm-pca9685.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/pwm/pwm-pca9685.c b/drivers/pwm/pwm-pca9685.c index c9f9e65..3fb775d 100644 --- a/drivers/pwm/pwm-pca9685.c +++ b/drivers/pwm/pwm-pca9685.c @@ -190,7 +190,7 @@ static void pca9685_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm) if (--pca->active_cnt == 0) regmap_update_bits(pca->regmap, PCA9685_MODE1, MODE1_SLEEP, - 0x1); + MODE1_SLEEP); } static const struct pwm_ops pca9685_pwm_ops = { @@ -264,7 +264,8 @@ static int pca9685_pwm_remove(struct i2c_client *client) { struct pca9685 *pca = i2c_get_clientdata(client); - regmap_update_bits(pca->regmap, PCA9685_MODE1, MODE1_SLEEP, 0x1); + regmap_update_bits(pca->regmap, PCA9685_MODE1, MODE1_SLEEP, + MODE1_SLEEP); return pwmchip_remove(&pca->chip); } -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] pwm: pca9685: Fix wrong argument to set MODE1_SLEEP bit 2013-06-19 17:27 [PATCH] pwm: pca9685: Fix wrong argument to set MODE1_SLEEP bit Axel Lin @ 2013-06-26 9:51 ` Thierry Reding 2013-06-26 12:09 ` Steffen Trumtrar 2013-06-26 21:33 ` Thierry Reding 1 sibling, 1 reply; 4+ messages in thread From: Thierry Reding @ 2013-06-26 9:51 UTC (permalink / raw) To: Steffen Trumtrar; +Cc: Axel Lin, linux-pwm [-- Attachment #1: Type: text/plain, Size: 1540 bytes --] On Thu, Jun 20, 2013 at 01:27:27AM +0800, Axel Lin wrote: > Current code actually does not set MODE1_SLEEP bit because the new value for > bitmask (0x1) is wrong. To set MODE1_SLEEP bit, we should pass MODE1_SLEEP > as the new value for bitmask. > > Signed-off-by: Axel Lin <axel.lin@ingics.com> > --- > Hi Steffen, > I don't have this hardware, can you test if this patch works? > Thanks, > Axel > > drivers/pwm/pwm-pca9685.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) Steffen, are you able to test or review this? The patch seems good but I'm not familiar with the hardware nor do I have a board to test it on. Thierry > > diff --git a/drivers/pwm/pwm-pca9685.c b/drivers/pwm/pwm-pca9685.c > index c9f9e65..3fb775d 100644 > --- a/drivers/pwm/pwm-pca9685.c > +++ b/drivers/pwm/pwm-pca9685.c > @@ -190,7 +190,7 @@ static void pca9685_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm) > > if (--pca->active_cnt == 0) > regmap_update_bits(pca->regmap, PCA9685_MODE1, MODE1_SLEEP, > - 0x1); > + MODE1_SLEEP); > } > > static const struct pwm_ops pca9685_pwm_ops = { > @@ -264,7 +264,8 @@ static int pca9685_pwm_remove(struct i2c_client *client) > { > struct pca9685 *pca = i2c_get_clientdata(client); > > - regmap_update_bits(pca->regmap, PCA9685_MODE1, MODE1_SLEEP, 0x1); > + regmap_update_bits(pca->regmap, PCA9685_MODE1, MODE1_SLEEP, > + MODE1_SLEEP); > > return pwmchip_remove(&pca->chip); > } > -- > 1.8.1.2 > > > [-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] pwm: pca9685: Fix wrong argument to set MODE1_SLEEP bit 2013-06-26 9:51 ` Thierry Reding @ 2013-06-26 12:09 ` Steffen Trumtrar 0 siblings, 0 replies; 4+ messages in thread From: Steffen Trumtrar @ 2013-06-26 12:09 UTC (permalink / raw) To: Thierry Reding; +Cc: Axel Lin, linux-pwm On Wed, Jun 26, 2013 at 11:51:07AM +0200, Thierry Reding wrote: > On Thu, Jun 20, 2013 at 01:27:27AM +0800, Axel Lin wrote: > > Current code actually does not set MODE1_SLEEP bit because the new value for > > bitmask (0x1) is wrong. To set MODE1_SLEEP bit, we should pass MODE1_SLEEP > > as the new value for bitmask. > > > > Signed-off-by: Axel Lin <axel.lin@ingics.com> > > --- > > Hi Steffen, > > I don't have this hardware, can you test if this patch works? > > Thanks, > > Axel > > > > drivers/pwm/pwm-pca9685.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > Steffen, > > are you able to test or review this? The patch seems good but I'm not > familiar with the hardware nor do I have a board to test it on. > > Thierry > > > > > diff --git a/drivers/pwm/pwm-pca9685.c b/drivers/pwm/pwm-pca9685.c > > index c9f9e65..3fb775d 100644 > > --- a/drivers/pwm/pwm-pca9685.c > > +++ b/drivers/pwm/pwm-pca9685.c > > @@ -190,7 +190,7 @@ static void pca9685_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm) > > > > if (--pca->active_cnt == 0) > > regmap_update_bits(pca->regmap, PCA9685_MODE1, MODE1_SLEEP, > > - 0x1); > > + MODE1_SLEEP); > > } > > > > static const struct pwm_ops pca9685_pwm_ops = { > > @@ -264,7 +264,8 @@ static int pca9685_pwm_remove(struct i2c_client *client) > > { > > struct pca9685 *pca = i2c_get_clientdata(client); > > > > - regmap_update_bits(pca->regmap, PCA9685_MODE1, MODE1_SLEEP, 0x1); > > + regmap_update_bits(pca->regmap, PCA9685_MODE1, MODE1_SLEEP, > > + MODE1_SLEEP); > > > > return pwmchip_remove(&pca->chip); > > } > > -- Hi! Looks good. I used regmap_update_bits wrong, obviously. I only provided the value (0x1), but did not apply the correct mask to it, which this version (with MODE1_SLEEP) does. So, you may add my Reviewed-by: Steffen Trumtrar <s.trumtrar@pengutronix.de> Thank you Axel for fixing it, Steffen -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] pwm: pca9685: Fix wrong argument to set MODE1_SLEEP bit 2013-06-19 17:27 [PATCH] pwm: pca9685: Fix wrong argument to set MODE1_SLEEP bit Axel Lin 2013-06-26 9:51 ` Thierry Reding @ 2013-06-26 21:33 ` Thierry Reding 1 sibling, 0 replies; 4+ messages in thread From: Thierry Reding @ 2013-06-26 21:33 UTC (permalink / raw) To: Axel Lin; +Cc: Steffen Trumtrar, linux-pwm [-- Attachment #1: Type: text/plain, Size: 536 bytes --] On Thu, Jun 20, 2013 at 01:27:27AM +0800, Axel Lin wrote: > Current code actually does not set MODE1_SLEEP bit because the new value for > bitmask (0x1) is wrong. To set MODE1_SLEEP bit, we should pass MODE1_SLEEP > as the new value for bitmask. > > Signed-off-by: Axel Lin <axel.lin@ingics.com> > --- > Hi Steffen, > I don't have this hardware, can you test if this patch works? > Thanks, > Axel > > drivers/pwm/pwm-pca9685.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) Applied, thanks. Thierry [-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-06-26 21:33 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-06-19 17:27 [PATCH] pwm: pca9685: Fix wrong argument to set MODE1_SLEEP bit Axel Lin 2013-06-26 9:51 ` Thierry Reding 2013-06-26 12:09 ` Steffen Trumtrar 2013-06-26 21:33 ` Thierry Reding
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).