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

InKi Dae
2010-04-29T03:30:22Z

[-- 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;