Re: [PATCH v9 1/3] Input: cyttsp - Cypress TTSP capacitive multi-touch screen support

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Wed, Jan 18, 2012 at 11:35:28AM +0100, Javier Martinez Canillas wrote:
> On Wed, Jan 18, 2012 at 10:20 AM, Javier Martinez Canillas
> <martinez.javier@gmail.com> wrote:
> > On Wed, Jan 18, 2012 at 10:12 AM, Dmitry Torokhov
> > <dmitry.torokhov@gmail.com> wrote:
> >> Hi Javier,
> >>
> >> On Sat, Jan 14, 2012 at 03:32:51AM +0100, Javier Martinez Canillas wrote:
> >>> +
> >>> +static int __cyttsp_disable(struct cyttsp *ts)
> >>> +{
> >>> +     u8 sleep_mode = 0;
> >>> +     int retval = 0;
> >>> +
> >>> +     if (ts->pdata->use_sleep && ts->power_state == CY_ACTIVE_STATE) {
> >>> +             sleep_mode = ts->pdata->use_sleep;
> >>> +             retval = ttsp_write_block_data(ts, CY_REG_BASE,
> >>> +                                            sizeof(sleep_mode), &sleep_mode);
> >>> +             if (retval >= 0) {
> >>> +                     ts->power_state = CY_SLEEP_STATE;
> >>> +                     disable_irq(ts->irq);
> >>> +             }
> >>
> >> I thought we agreed that we should put the device to sleep
> >> unconditionally and not have use_sleep option?
> >
> > Yes, we did. Sorry, I've missed that. I'll change it and resend.
> >
> >>
> >> Thanks.
> >>
> >> --
> >> Dmitry
> >> --
> 
> Thanks Dmitry for the review.
> 
> Any more issues you have found? So I can fix all of them for the next
> version of the patch.

I am still not quite happy with the flow of power_on function. Maybe you
should fold cyttsp_bl_app_valid() into it to avoid dealing with negative,
0 and 1 return codes. If you also pull printing of the status out of it
and into cyttsp_open() you'll be able to do early returns and avoid
Fortran-style control flow of gotos.

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html