Re: [PATCH 0/7] A few patches to cyttsp

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Nov 14, 2011 at 9:15 AM, Dmitry Torokhov
<dmitry.torokhov@xxxxxxxxx> wrote:
> Hi Javier,
>
> I am sending a few patches that I made while trying to understand the
> Cypress TTSP driver. I am still not quite happy with the SPI read method
> and I am also wondering if cyttsp_power_on could be made less messy.
>

Hi Dmitry,

Thank you very much for the review and the patches. One question, when
sending new versions of the patch-set, do you want me to keep the
history of your patches or should I merge all of them and submit just
three patches (core, i2c, spi) to make easier future revisions?

Sure, Kevin will cleanup the SPI read method and I will make
cyttsp_power_on more cleaner.

> Current open/close methods seem to be not very useful: even though we
> stop interrupts we do not put the chip into low power mode. Is there a
> way to do so?
>

Yes, the controller can be put to sleep (low power mode). I will add
that to the close function for the next version of the patch-set.

> Regarding PM methods - why do we have conditional 'use sleep' and what
> exactly wakeup() method is supposed to do? Why is it a platform method?
>

It was meant to let each board to decide if it wants to go sleep and
to define a specific handler for wakeup. This is for example the board
file of the OMAP Encore machine type (B & N Nook's color)

static struct cyttsp_platform_data cyttsp_i2c_platform_data = {
        .wakeup = cyttsp_i2c_wakeup,
        .init = cyttsp_i2c_init,
.....
}

In the same way it exists an init board specific function that gets
called from cyttsp_core_init() if the board has defined one:

void *cyttsp_core_init(struct cyttsp_bus_ops *bus_ops,
		       struct device *dev, int irq)
{
....
	if (ts->platform_data->init) {
		if (ts->platform_data->init()) {
			dev_dbg(ts->dev, "%s: Error, platform init failed!\n",
				__func__);
			goto error_init;
		}
	}
....
}

This is necessary because each board may need to configure differently
the connection with the chip. For example, OMAP boards will have to
configure the mux pins and so on.

Is this correct? or the driver should not have these handlers and
trust that the platform specific code did all the set up before?

Again, thank you for the review and your comments.

Best regards,

-- 
Javier Martínez Canillas
(+34) 682 39 81 69
Barcelona, Spain
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux