backlight: corgi_lcd: Fix WARN_ON() when calling corgi_bl_set_intensity.

From: Marko Katic <dromede.gmail.com>


Signed-off-by: Marko Katic <redacted>
---
 drivers/video/backlight/corgi_lcd.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/backlight/corgi_lcd.c b/drivers/video/backlight/corgi_lcd.c
index c781768..8b002d7 100644
--- a/drivers/video/backlight/corgi_lcd.c
+++ b/drivers/video/backlight/corgi_lcd.c

@@ -409,10 +409,10 @@ static int corgi_bl_set_intensity(struct corgi_lcd *lcd, int intensity)
 	cont = !!(intensity & 0x20) ^ lcd->gpio_backlight_cont_inverted;
 
 	if (gpio_is_valid(lcd->gpio_backlight_cont))
-		gpio_set_value(lcd->gpio_backlight_cont, cont);
+		gpio_set_value_cansleep(lcd->gpio_backlight_cont, cont);
 
 	if (gpio_is_valid(lcd->gpio_backlight_on))
-		gpio_set_value(lcd->gpio_backlight_on, intensity);
+		gpio_set_value_cansleep(lcd->gpio_backlight_on, intensity);
 
 	if (lcd->kick_battery)
 		lcd->kick_battery();

-- 
1.7.4.1

On Thursday, November 29, 2012 6:47 AM, Marko Katic wrote

Subject: [PATCH] backlight: corgi_lcd: Fix WARN_ON() when calling corgi_bl_set_intensity.

CC'ed Andrew Morton.


Hi Marko Katic,
The commit subject and commit message are not clear.

How about using subject as below?
backlight: corgi_lcd: use gpio_set_value_cansleep()

In addition, 'Fix WARN_ON() when calling corgi_bl_set_intensity'
would be the reason why you submit this patch.
Please make the commit message more detail.

Also, I have a question on gpio driver.
In order to trigger WARN_ON(chip->can_sleep), 'can_sleep' should be
set as 1. However, I cannot find 'can_sleep = 1' in the PXA gpio driver.
What gpio driver do you use to test corgi_lcd driver?


Best regards,
Jingoo Han

On Thu, Nov 29, 2012 at 6:30 AM, Jingoo Han [off-list ref] wrote:

On Thursday, November 29, 2012 6:47 AM, Marko Katic wrote

quoted

Subject: [PATCH] backlight: corgi_lcd: Fix WARN_ON() when calling corgi_bl_set_intensity.

CC'ed Andrew Morton.


Hi Marko Katic,
The commit subject and commit message are not clear.

How about using subject as below?
backlight: corgi_lcd: use gpio_set_value_cansleep()

In addition, 'Fix WARN_ON() when calling corgi_bl_set_intensity'
would be the reason why you submit this patch.
Please make the commit message more detail.

Also, I have a question on gpio driver.
In order to trigger WARN_ON(chip->can_sleep), 'can_sleep' should be
set as 1. However, I cannot find 'can_sleep = 1' in the PXA gpio driver.
What gpio driver do you use to test corgi_lcd driver?


Best regards,
Jingoo Han

Well, the commit message was short because i thought it was a quick
and obvious fix.
But it doesn't really matter now since you raise a valid point with
your question.
There are two classes of devices that use this lcd, corgi and spitz
(both are in mach-pxa tree). Both have
several variants. All of them use the SCOOP chip for backlight control
(arm/common/scoop.c) except akita which
uses the max7310 gpio expander for backlight control. The SCOOP chip
driver doesn't set can_sleep but the
max7310 does. So this patch is probably only valid for akita machines.
I'll test this further and post a revised patch
soon.

On Thu, Nov 29, 2012 at 02:09:51PM +0100, Marko Katić wrote:

Well, the commit message was short because i thought it was a quick
and obvious fix.

Don't always expect the person who ends up applying your patch to know
what your patch is doing.  Don't expect people who are looking back
through the git history to look at the patch to work out whether your
commit is relevant to them.  I suspect Andrew Morton doesn't know what
a "SCOOP" or "AKITA" is...

Commit messages are there not only to describe the change, but also say
why the change is necessary.  Think about it as your chance to "sell"
the patch.

One good step would be to include the warning message dump from the
kernel (your subject line says you're hitting a WARN_ON so include
that.)  At least then people can see your starting point for the patch.