[PATCH] backlight: lm3630a: bump REG_MAX value to 0x50 instead of 0x1F

Subsystems: backlight class/subsystem, framebuffer layer, the rest

STALE3336d

6 messages, 3 authors, 2017-06-22 · open the first message on its own page

[PATCH] backlight: lm3630a: bump REG_MAX value to 0x50 instead of 0x1F

From: Bhushan Shah <hidden>
Date: 2017-06-21 05:30:34

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 <redacted>
Suggested-by: Bjorn Andersson <redacted>
---
 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

[PATCH v2] backlight: lm3630a: bump REG_MAX value to 0x50 instead of 0x1F

From: Bhushan Shah <hidden>
Date: 2017-06-21 05:35:39

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 <redacted>
Suggested-by: Bjorn Andersson <redacted>
---

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

Re: [PATCH v2] backlight: lm3630a: bump REG_MAX value to 0x50 instead of 0x1F

From: Daniel Thompson <hidden>
Date: 2017-06-21 09:37:49

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 <redacted>
Suggested-by: Bjorn Andersson <redacted>
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!
quoted hunk
---

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.
quoted hunk
+#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 */

Re: [PATCH v2] backlight: lm3630a: bump REG_MAX value to 0x50 instead of 0x1F

From: Daniel Thompson <hidden>
Date: 2017-06-21 11:03:36

On 21/06/17 11:52, Bhushan Shah wrote:
On Wed, Jun 21, 2017 at 10:37:43AM +0100, Daniel Thompson wrote:
quoted
On 21/06/17 06:31, Bhushan Shah wrote:
quoted
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 <redacted>
Suggested-by: Bjorn Andersson <redacted>
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).
quoted
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.

quoted
quoted
---

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.
quoted
Daniel.
quoted
+#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 */

Re: [PATCH v2] backlight: lm3630a: bump REG_MAX value to 0x50 instead of 0x1F

From: Bhushan Shah <hidden>
Date: 2017-06-21 11:04:32

On Wed, Jun 21, 2017 at 10:37:43AM +0100, Daniel Thompson wrote:
On 21/06/17 06:31, Bhushan Shah wrote:
quoted
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 <redacted>
Suggested-by: Bjorn Andersson <redacted>
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.
quoted
---

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.
quoted
+#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

Re: [PATCH] backlight: lm3630a: bump REG_MAX value to 0x50 instead of 0x1F

From: kbuild test robot <hidden>
Date: 2017-06-22 00:47:25

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':
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help