| Message ID | 20170621053152.3008-1-bshah@kde.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 21/06/17 06:31, Bhushan Shah wrote: > In the lm3630a_chip_init we try to write to 0x50 register, which is > higher value then the max_register value, this resulted in regmap_write > return -EIO. > > Fix this by bumping REG_MAX value to 0x50. > > Signed-off-by: Bhushan Shah <bshah@kde.org> > Suggested-by: Bjorn Andersson <bjorn.andersson@linaro.org> Can we get a "Fixes" on this? It looks to me like it has been broken since 2a0c316bf3cc ("fix signedness bug in lm3630a_chip_init()"). Also I assume you find this by trying to use the driver on real hardware? If so can you confirm what you tested on in the patch description. As far as I can tell the code to set the filter strength has never worked, so you'll be the first user of it! > --- > > Changes since v1: > - Fix the lm3630a_write call to use proper value (sent worng patch earlier) > > drivers/video/backlight/lm3630a_bl.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c > index 60d6c2ac87aa..b641f706dbc9 100644 > --- a/drivers/video/backlight/lm3630a_bl.c > +++ b/drivers/video/backlight/lm3630a_bl.c > @@ -31,7 +31,8 @@ > #define REG_FAULT 0x0B > #define REG_PWM_OUTLOW 0x12 > #define REG_PWM_OUTHIGH 0x13 > -#define REG_MAX 0x1F > +#define REG_FLTR_STR 0x50 Can we expand this to REG_FILTER_STRENGTH? Daniel. > +#define REG_MAX 0x50 > > #define INT_DEBOUNCE_MSEC 10 > struct lm3630a_chip { > @@ -80,7 +81,7 @@ static int lm3630a_chip_init(struct lm3630a_chip *pchip) > > usleep_range(1000, 2000); > /* set Filter Strength Register */ > - rval = lm3630a_write(pchip, 0x50, 0x03); > + rval = lm3630a_write(pchip, REG_FLTR_STR, 0x03); > /* set Cofig. register */ > rval |= lm3630a_update(pchip, REG_CONFIG, 0x07, pdata->pwm_ctrl); > /* set boost control */ > -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 21, 2017 at 10:37:43AM +0100, Daniel Thompson wrote: > On 21/06/17 06:31, Bhushan Shah wrote: > > In the lm3630a_chip_init we try to write to 0x50 register, which is > > higher value then the max_register value, this resulted in regmap_write > > return -EIO. > > > > Fix this by bumping REG_MAX value to 0x50. > > > Signed-off-by: Bhushan Shah <bshah@kde.org> > > Suggested-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > Can we get a "Fixes" on this? It looks to me like it has been broken since > 2a0c316bf3cc ("fix signedness bug in lm3630a_chip_init()"). I don't think 2a0c316bf3cc is the right commit to mention in Fixes tag? Because it was broken since the introduction of chip revision in commit 28e64a68a2ef ("backlight: lm3630: apply chip revision"). commit 2a0c316bf3cc just made it error out correctly instead of failing silently. What do you think? > > Also I assume you find this by trying to use the driver on real hardware? If > so can you confirm what you tested on in the patch description. As far as I > can tell the code to set the filter strength has never worked, so you'll be > the first user of it! Yes I hit this problem by trying it on LGE Nexus 5 (hammerhead). I will mention this in the v3 revision of patch. > > > --- > > > > Changes since v1: > > - Fix the lm3630a_write call to use proper value (sent worng patch earlier) > > > > drivers/video/backlight/lm3630a_bl.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c > > index 60d6c2ac87aa..b641f706dbc9 100644 > > --- a/drivers/video/backlight/lm3630a_bl.c > > +++ b/drivers/video/backlight/lm3630a_bl.c > > @@ -31,7 +31,8 @@ > > #define REG_FAULT 0x0B > > #define REG_PWM_OUTLOW 0x12 > > #define REG_PWM_OUTHIGH 0x13 > > -#define REG_MAX 0x1F > > +#define REG_FLTR_STR 0x50 > > Can we expand this to REG_FILTER_STRENGTH? Sure, will include in Patch v3. > > Daniel. > > > +#define REG_MAX 0x50 > > #define INT_DEBOUNCE_MSEC 10 > > struct lm3630a_chip { > > @@ -80,7 +81,7 @@ static int lm3630a_chip_init(struct lm3630a_chip *pchip) > > usleep_range(1000, 2000); > > /* set Filter Strength Register */ > > - rval = lm3630a_write(pchip, 0x50, 0x03); > > + rval = lm3630a_write(pchip, REG_FLTR_STR, 0x03); > > /* set Cofig. register */ > > rval |= lm3630a_update(pchip, REG_CONFIG, 0x07, pdata->pwm_ctrl); > > /* set boost control */ > > >
diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c index 60d6c2ac87aa..b641f706dbc9 100644 --- a/drivers/video/backlight/lm3630a_bl.c +++ b/drivers/video/backlight/lm3630a_bl.c @@ -31,7 +31,8 @@ #define REG_FAULT 0x0B #define REG_PWM_OUTLOW 0x12 #define REG_PWM_OUTHIGH 0x13 -#define REG_MAX 0x1F +#define REG_FLTR_STR 0x50 +#define REG_MAX 0x50 #define INT_DEBOUNCE_MSEC 10 struct lm3630a_chip { @@ -80,7 +81,7 @@ static int lm3630a_chip_init(struct lm3630a_chip *pchip) usleep_range(1000, 2000); /* set Filter Strength Register */ - rval = lm3630a_write(pchip, 0x50, 0x03); + rval = lm3630a_write(pchip, REG_FLTR_STR, 0x03); /* set Cofig. register */ rval |= lm3630a_update(pchip, REG_CONFIG, 0x07, pdata->pwm_ctrl); /* set boost control */
In the lm3630a_chip_init we try to write to 0x50 register, which is higher value then the max_register value, this resulted in regmap_write return -EIO. Fix this by bumping REG_MAX value to 0x50. Signed-off-by: Bhushan Shah <bshah@kde.org> Suggested-by: Bjorn Andersson <bjorn.andersson@linaro.org> --- Changes since v1: - Fix the lm3630a_write call to use proper value (sent worng patch earlier) drivers/video/backlight/lm3630a_bl.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)