Re: [PATCH] added S6E63M0 AMOLED LCD Panel driver.

[InKi Dae <daeinki@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1384 bytes --]

I'm sorry for being late.

this is second patch that your concern is solved.

Please review this patch.

Thank you.

Best Regards,
InKi Dae.

2010/4/28 Andrew Morton <akpm@linux-foundation.org>:
> On Tue, 30 Mar 2010 19:55:01 -0400
> Andrew Morton <akpm@linux-foundation.org> wrote:
>
>> On Wed, 31 Mar 2010 11:41:54 +0900 InKi Dae <daeinki@gmail.com> wrote:
>>
>> > Hi Andrew,
>> >
>> > all the calls to s6e63m0_panel_send_sequence() would return -EINVAL.
>> > by api_async() of driver/spi/spi.c
>>
>> No, spi_async() does
>>
>>       master->transfer(spi, message);
>>
>> which can return at least EIO, EINPROGRESS, EINVAL or ETIMEDOUT.
>>
>> > so I think that those return values aren't changed to other.
>> >
>> > and final step is to check only whether the return value is 0 or not.
>> > if you still think that this code has minor problem or you want it to
>> > be corrected
>> > then I will patch this code to be corrected anytime.
>>
>> It's a bug.
>>
>> Also s6e63m0_power_on() is sloppy.  It again or's together disparate
>> errnos.  Then if _anything_ failed it returns hardwired -EIO, but it
>> should instead propagate the callee's errno back up to the caller.
>>
>> And s6e63m0_power_on() can return -EFAULT in several places, which is
>> nonsensical.
>>
>> None of this is very critical, just ... sloppy.
>>
>
> ping?
>

[-- Attachment #2: s6e63m0_second.patch --]
[-- Type: application/octet-stream, Size: 2954 bytes --]

diff --git a/drivers/video/backlight/s6e63m0.c b/drivers/video/backlight/s6e63m0.c
index e4793df..254a099 100644
--- a/drivers/video/backlight/s6e63m0.c
+++ b/drivers/video/backlight/s6e63m0.c
@@ -420,13 +420,25 @@ static int _s6e63m0_gamma_ctl(struct s6e63m0 *lcd, const unsigned int *gamma)
 
 	/* disable gamma table updating. */
 	ret = s6e63m0_spi_write(lcd, 0xfa, 0x00);
+	if (ret) {
+		dev_err(lcd->dev, "failed to disable gamma table updating.\n");
+		goto gamma_err;
+	}
 
-	for (i = 0 ; i < GAMMA_TABLE_COUNT; i++)
-		ret |= s6e63m0_spi_write(lcd, DATA_ONLY, gamma[i]);
+	for (i = 0 ; i < GAMMA_TABLE_COUNT; i++) {
+		ret = s6e63m0_spi_write(lcd, DATA_ONLY, gamma[i]);
+		if (ret) {
+			dev_err(lcd->dev, "failed to set gamma table.\n");
+			goto gamma_err;
+		}
+	}
 
 	/* update gamma table. */
-	ret |= s6e63m0_spi_write(lcd, 0xfa, 0x01);
+	ret = s6e63m0_spi_write(lcd, 0xfa, 0x01);
+	if (ret)
+		dev_err(lcd->dev, "failed to update gamma table.\n");
 
+gamma_err:
 	return ret;
 }
 
@@ -442,24 +454,38 @@ static int s6e63m0_gamma_ctl(struct s6e63m0 *lcd, int gamma)
 
 static int s6e63m0_ldi_init(struct s6e63m0 *lcd)
 {
-	int ret;
+	int ret, i;
+	const unsigned short *init_seq[] = {
+		SEQ_PANEL_CONDITION_SET,
+		SEQ_DISPLAY_CONDITION_SET,
+		SEQ_GAMMA_SETTING,
+		SEQ_ETC_CONDITION_SET,
+		SEQ_ACL_ON,
+		SEQ_ELVSS_ON,
+	};
 
-	ret = s6e63m0_panel_send_sequence(lcd, SEQ_PANEL_CONDITION_SET);
-	ret |= s6e63m0_panel_send_sequence(lcd, SEQ_DISPLAY_CONDITION_SET);
-	ret |= s6e63m0_panel_send_sequence(lcd, SEQ_GAMMA_SETTING);
-	ret |= s6e63m0_panel_send_sequence(lcd, SEQ_ETC_CONDITION_SET);
-	ret |= s6e63m0_panel_send_sequence(lcd, SEQ_ACL_ON);
-	ret |= s6e63m0_panel_send_sequence(lcd, SEQ_ELVSS_ON);
+	for (i = 0; i < ARRAY_SIZE(init_seq); i++) {
+		ret = s6e63m0_panel_send_sequence(lcd, init_seq[i]);
+		if (ret)
+			break;
+	}
 
 	return ret;
 }
 
 static int s6e63m0_ldi_enable(struct s6e63m0 *lcd)
 {
-	int ret = 0;
+	int ret = 0, i;
+	const unsigned short *enable_seq[] = {
+		SEQ_STAND_BY_OFF,
+		SEQ_DISPLAY_ON,
+	};
 
-	ret = s6e63m0_panel_send_sequence(lcd, SEQ_STAND_BY_OFF);
-	ret = s6e63m0_panel_send_sequence(lcd, SEQ_DISPLAY_ON);
+	for (i = 0; i < ARRAY_SIZE(enable_seq); i++) {
+		ret = s6e63m0_panel_send_sequence(lcd, enable_seq[i]);
+		if (ret)
+			break;
+	}
 
 	return ret;
 }
@@ -508,12 +534,22 @@ static int s6e63m0_power_on(struct s6e63m0 *lcd)
 	}
 
 	ret = s6e63m0_ldi_init(lcd);
-	ret |= s6e63m0_ldi_enable(lcd);
+	if (ret) {
+		dev_err(lcd->dev, "failed to initialize ldi.\n");
+		return ret;
+	}
+
+	ret = s6e63m0_ldi_enable(lcd);
+	if (ret) {
+		dev_err(lcd->dev, "failed to enable ldi.\n");
+		return ret;
+	}
+
 	/* set brightness to current value after power on or resume. */
-	ret |= s6e63m0_gamma_ctl(lcd, bd->props.brightness);
+	ret = s6e63m0_gamma_ctl(lcd, bd->props.brightness);
 	if (ret) {
 		dev_err(lcd->dev, "lcd gamma setting failed.\n");
-		return -EIO;
+		return ret;
 	}
 
 	return 0;