* [PATCH] backlight: lm3630a: bump REG_MAX value to 0x50 instead of 0x1F
@ 2017-06-21 5:36 ` Bhushan Shah
0 siblings, 0 replies; 12+ messages in thread
From: Bhushan Shah @ 2017-06-21 5:24 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han,
Bartlomiej Zolnierkiewicz, linux-fbdev, linux-kernel
Cc: Bhushan Shah
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>
---
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..42496b6c09d0 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, FLTR_STR, 0x03);
/* set Cofig. register */
rval |= lm3630a_update(pchip, REG_CONFIG, 0x07, pdata->pwm_ctrl);
/* set boost control */
--
2.13.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2] backlight: lm3630a: bump REG_MAX value to 0x50 instead of 0x1F
2017-06-21 5:36 ` Bhushan Shah
@ 2017-06-21 5:43 ` Bhushan Shah
-1 siblings, 0 replies; 12+ messages in thread
From: Bhushan Shah @ 2017-06-21 5:31 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han,
Bartlomiej Zolnierkiewicz, linux-fbdev, linux-kernel
Cc: Bhushan Shah
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(-)
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 */
--
2.13.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH] backlight: lm3630a: bump REG_MAX value to 0x50 instead of 0x1F
@ 2017-06-21 5:36 ` Bhushan Shah
0 siblings, 0 replies; 12+ messages in thread
From: Bhushan Shah @ 2017-06-21 5:36 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han,
Bartlomiej Zolnierkiewicz, linux-fbdev, linux-kernel
Cc: Bhushan Shah
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>
---
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..42496b6c09d0 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, FLTR_STR, 0x03);
/* set Cofig. register */
rval |= lm3630a_update(pchip, REG_CONFIG, 0x07, pdata->pwm_ctrl);
/* set boost control */
--
2.13.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2] backlight: lm3630a: bump REG_MAX value to 0x50 instead of 0x1F
@ 2017-06-21 5:43 ` Bhushan Shah
0 siblings, 0 replies; 12+ messages in thread
From: Bhushan Shah @ 2017-06-21 5:43 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han,
Bartlomiej Zolnierkiewicz, linux-fbdev, linux-kernel
Cc: Bhushan Shah
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(-)
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 */
--
2.13.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2] backlight: lm3630a: bump REG_MAX value to 0x50 instead of 0x1F
2017-06-21 5:43 ` Bhushan Shah
@ 2017-06-21 9:37 ` Daniel Thompson
-1 siblings, 0 replies; 12+ messages in thread
From: Daniel Thompson @ 2017-06-21 9:37 UTC (permalink / raw)
To: Bhushan Shah, Lee Jones, Jingoo Han, Bartlomiej Zolnierkiewicz,
linux-fbdev, linux-kernel
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 */
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] backlight: lm3630a: bump REG_MAX value to 0x50 instead of 0x1F
@ 2017-06-21 9:37 ` Daniel Thompson
0 siblings, 0 replies; 12+ messages in thread
From: Daniel Thompson @ 2017-06-21 9:37 UTC (permalink / raw)
To: Bhushan Shah, Lee Jones, Jingoo Han, Bartlomiej Zolnierkiewicz,
linux-fbdev, linux-kernel
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 */
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] backlight: lm3630a: bump REG_MAX value to 0x50 instead of 0x1F
2017-06-21 9:37 ` Daniel Thompson
@ 2017-06-21 11:04 ` Bhushan Shah
-1 siblings, 0 replies; 12+ messages in thread
From: Bhushan Shah @ 2017-06-21 10:52 UTC (permalink / raw)
To: Daniel Thompson
Cc: Lee Jones, Jingoo Han, Bartlomiej Zolnierkiewicz, Bhushan Shah,
linux-fbdev, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2717 bytes --]
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 */
> >
>
--
Bhushan Shah
http://blog.bshah.in
IRC Nick : bshah on Freenode
GPG key fingerprint : 0AAC 775B B643 7A8D 9AF7 A3AC FE07 8411 7FBC E11D
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] backlight: lm3630a: bump REG_MAX value to 0x50 instead of 0x1F
2017-06-21 11:04 ` Bhushan Shah
@ 2017-06-21 11:03 ` Daniel Thompson
-1 siblings, 0 replies; 12+ messages in thread
From: Daniel Thompson @ 2017-06-21 11:03 UTC (permalink / raw)
To: Bhushan Shah
Cc: Lee Jones, Jingoo Han, Bartlomiej Zolnierkiewicz, linux-fbdev,
linux-kernel
On 21/06/17 11:52, Bhushan Shah wrote:
> 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?
Depends if we chase the seeds of disaster or problems user experience.
For sure I don't really attach very much blame to 2a0c316bf3cc. However
that *is* the commit from which the probe() will have stopped working
and is therefore is probably what would be picked out by a bisect... if
you had the patience to get a 2013 upstream kernel running on your Nexus ;-)
The seeds of disaster come from 28e64a68a2ef ("backlight: lm3630: apply
chip revision") which was the commit that (didn't) add support for this
register.
Personally I think it would be OK to put in two commits both commits in
the Fixes: (if you really want to be even handed explain in the patch
description).
>
>>
>> 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.
Thanks.
>
>>
>>> ---
>>>
>>> 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 */
>>>
>>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] backlight: lm3630a: bump REG_MAX value to 0x50 instead of 0x1F
@ 2017-06-21 11:03 ` Daniel Thompson
0 siblings, 0 replies; 12+ messages in thread
From: Daniel Thompson @ 2017-06-21 11:03 UTC (permalink / raw)
To: Bhushan Shah
Cc: Lee Jones, Jingoo Han, Bartlomiej Zolnierkiewicz, linux-fbdev,
linux-kernel
On 21/06/17 11:52, Bhushan Shah wrote:
> 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?
Depends if we chase the seeds of disaster or problems user experience.
For sure I don't really attach very much blame to 2a0c316bf3cc. However
that *is* the commit from which the probe() will have stopped working
and is therefore is probably what would be picked out by a bisect... if
you had the patience to get a 2013 upstream kernel running on your Nexus ;-)
The seeds of disaster come from 28e64a68a2ef ("backlight: lm3630: apply
chip revision") which was the commit that (didn't) add support for this
register.
Personally I think it would be OK to put in two commits both commits in
the Fixes: (if you really want to be even handed explain in the patch
description).
>
>>
>> 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.
Thanks.
>
>>
>>> ---
>>>
>>> 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 */
>>>
>>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] backlight: lm3630a: bump REG_MAX value to 0x50 instead of 0x1F
@ 2017-06-21 11:04 ` Bhushan Shah
0 siblings, 0 replies; 12+ messages in thread
From: Bhushan Shah @ 2017-06-21 11:04 UTC (permalink / raw)
To: Daniel Thompson
Cc: Lee Jones, Jingoo Han, Bartlomiej Zolnierkiewicz, Bhushan Shah,
linux-fbdev, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2717 bytes --]
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 */
> >
>
--
Bhushan Shah
http://blog.bshah.in
IRC Nick : bshah on Freenode
GPG key fingerprint : 0AAC 775B B643 7A8D 9AF7 A3AC FE07 8411 7FBC E11D
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] backlight: lm3630a: bump REG_MAX value to 0x50 instead of 0x1F
2017-06-21 5:36 ` Bhushan Shah
@ 2017-06-22 0:46 ` kbuild test robot
-1 siblings, 0 replies; 12+ messages in thread
From: kbuild test robot @ 2017-06-22 0:46 UTC (permalink / raw)
To: Bhushan Shah
Cc: kbuild-all, Lee Jones, Daniel Thompson, Jingoo Han,
Bartlomiej Zolnierkiewicz, linux-fbdev, linux-kernel,
Bhushan Shah
[-- Attachment #1: Type: text/plain, Size: 1558 bytes --]
Hi Bhushan,
[auto build test ERROR on v4.12-rc6]
[also build test ERROR on next-20170621]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Bhushan-Shah/backlight-lm3630a-bump-REG_MAX-value-to-0x50-instead-of-0x1F/20170622-074946
config: i386-randconfig-x011-06190614 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers/video/backlight/lm3630a_bl.c: In function 'lm3630a_chip_init':
>> drivers/video/backlight/lm3630a_bl.c:84:30: error: 'FLTR_STR' undeclared (first use in this function)
rval = lm3630a_write(pchip, FLTR_STR, 0x03);
^~~~~~~~
drivers/video/backlight/lm3630a_bl.c:84:30: note: each undeclared identifier is reported only once for each function it appears in
vim +/FLTR_STR +84 drivers/video/backlight/lm3630a_bl.c
78 {
79 int rval;
80 struct lm3630a_platform_data *pdata = pchip->pdata;
81
82 usleep_range(1000, 2000);
83 /* set Filter Strength Register */
> 84 rval = lm3630a_write(pchip, FLTR_STR, 0x03);
85 /* set Cofig. register */
86 rval |= lm3630a_update(pchip, REG_CONFIG, 0x07, pdata->pwm_ctrl);
87 /* set boost control */
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 22990 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] backlight: lm3630a: bump REG_MAX value to 0x50 instead of 0x1F
@ 2017-06-22 0:46 ` kbuild test robot
0 siblings, 0 replies; 12+ messages in thread
From: kbuild test robot @ 2017-06-22 0:46 UTC (permalink / raw)
To: linux-fbdev
[-- Attachment #1: Type: text/plain, Size: 1558 bytes --]
Hi Bhushan,
[auto build test ERROR on v4.12-rc6]
[also build test ERROR on next-20170621]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Bhushan-Shah/backlight-lm3630a-bump-REG_MAX-value-to-0x50-instead-of-0x1F/20170622-074946
config: i386-randconfig-x011-06190614 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers/video/backlight/lm3630a_bl.c: In function 'lm3630a_chip_init':
>> drivers/video/backlight/lm3630a_bl.c:84:30: error: 'FLTR_STR' undeclared (first use in this function)
rval = lm3630a_write(pchip, FLTR_STR, 0x03);
^~~~~~~~
drivers/video/backlight/lm3630a_bl.c:84:30: note: each undeclared identifier is reported only once for each function it appears in
vim +/FLTR_STR +84 drivers/video/backlight/lm3630a_bl.c
78 {
79 int rval;
80 struct lm3630a_platform_data *pdata = pchip->pdata;
81
82 usleep_range(1000, 2000);
83 /* set Filter Strength Register */
> 84 rval = lm3630a_write(pchip, FLTR_STR, 0x03);
85 /* set Cofig. register */
86 rval |= lm3630a_update(pchip, REG_CONFIG, 0x07, pdata->pwm_ctrl);
87 /* set boost control */
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 22990 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-06-22 0:47 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-21 5:24 [PATCH] backlight: lm3630a: bump REG_MAX value to 0x50 instead of 0x1F Bhushan Shah
2017-06-21 5:36 ` Bhushan Shah
2017-06-21 5:31 ` [PATCH v2] " Bhushan Shah
2017-06-21 5:43 ` Bhushan Shah
2017-06-21 9:37 ` Daniel Thompson
2017-06-21 9:37 ` Daniel Thompson
2017-06-21 10:52 ` Bhushan Shah
2017-06-21 11:04 ` Bhushan Shah
2017-06-21 11:03 ` Daniel Thompson
2017-06-21 11:03 ` Daniel Thompson
2017-06-22 0:46 ` [PATCH] " kbuild test robot
2017-06-22 0:46 ` kbuild test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.