2010-04-27 19:06:17

by Andrew Morton

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

On Tue, 30 Mar 2010 19:55:01 -0400
Andrew Morton <[email protected]> wrote:

> On Wed, 31 Mar 2010 11:41:54 +0900 InKi Dae <[email protected]> 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?


2010-04-29 03:30:22

by Inki Dae

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

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 <[email protected]>:
> On Tue, 30 Mar 2010 19:55:01 -0400
> Andrew Morton <[email protected]> wrote:
>
>> On Wed, 31 Mar 2010 11:41:54 +0900 InKi Dae <[email protected]> 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?
>


Attachments:
s6e63m0_second.patch (2.88 kB)