RE: [PATCH v4 6/14] input: cyapa: add gen3 trackpad device basic functions support

From: Dudley Du
Date: Sun Aug 24 2014 - 22:27:39 EST


Hi Dmitry,

> -----Original Message-----
> From: Dmitry Torokhov [mailto:dmitry.torokhov@xxxxxxxxx]
> Sent: Saturday, August 23, 2014 7:55 AM
> To: Dudley Du
> Cc: Rafael J. Wysocki; Benson Leung; Patrik Fimml; linux-input@xxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; Dudley Du
> Subject: Re: [PATCH v4 6/14] input: cyapa: add gen3 trackpad device basic
> functions support
>
> On Thu, Jul 17, 2014 at 02:53:48PM +0800, Dudley Du wrote:
> > Based on the cyapa core, add the gen3 trackpad device's basic functions
> > supported, so gen3 trackpad device can work with kernel input system.
> > The basic function is absolutely same as previous cyapa driver only
> > support gen3 trackpad device.
> > TEST=test on Chromebooks.
> >
> > Signed-off-by: Dudley Du <dudl@xxxxxxxxxxx>
> > ---
> > drivers/input/mouse/Makefile | 2 +-
> > drivers/input/mouse/cyapa.c | 96 ++++-
> > drivers/input/mouse/cyapa.h | 1 +
> > drivers/input/mouse/cyapa_gen3.c | 784
> ++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 881 insertions(+), 2 deletions(-)
> > create mode 100644 drivers/input/mouse/cyapa_gen3.c
> >
> > diff --git a/drivers/input/mouse/Makefile b/drivers/input/mouse/Makefile
> > index 8608eb7..63b42e0 100644
> > --- a/drivers/input/mouse/Makefile
> > +++ b/drivers/input/mouse/Makefile
> > @@ -35,4 +35,4 @@ psmouse-$(CONFIG_MOUSE_PS2_TRACKPOINT) += trackpoint.o
> > psmouse-$(CONFIG_MOUSE_PS2_TOUCHKIT) += touchkit_ps2.o
> > psmouse-$(CONFIG_MOUSE_PS2_CYPRESS) += cypress_ps2.o
> >
> > -cyapatp-y := cyapa.o
> > +cyapatp-y := cyapa.o cyapa_gen3.o
> > diff --git a/drivers/input/mouse/cyapa.c b/drivers/input/mouse/cyapa.c
> > index ae24b02..5c62503 100644
> > --- a/drivers/input/mouse/cyapa.c
> > +++ b/drivers/input/mouse/cyapa.c
> > @@ -113,6 +113,15 @@ ssize_t cyapa_i2c_write(struct cyapa *cyapa, u8 reg,
> >
> > void cyapa_default_irq_handler(struct cyapa *cyapa)
> > {
> > + bool cont;
> > +
> > + /* Interrupt triggerred by command response in detecting. */
> > + cont = true;
> > + if (cyapa_gen3_ops.irq_cmd_handler)
> > + cont = cyapa_gen3_ops.irq_cmd_handler(cyapa);
>
> Why not simply
>
> cont = cyapa->ops->irq_cmd_handler(cyapa)?


When this the default irq handler is called, at this time, means the device
haven't been initialized yet, the cyapa->ops hasn't been setup correctly, and
the cyapa->ops->irq_cmd_handler is NULL.
So currently, maybe the driver is still on detecting, which device module should
be setup to cyapa->ops is unknown yet. But like gen5 device, it will execute commands
that based on interrupt signals, so the irq_cmd_handler() must be executed
for each module to complete the possible command response when an interrupt comes.
So, here, for each device module, their irq_cmd_handler() should be called directly.


>
>
> > + if (!cont)
> > + return;
> > +
> > /*
> > * Do redetecting when device states is still unknown and
> > * interrupt envent is received from device.
> > @@ -252,6 +261,9 @@ static int cyapa_check_is_operational(struct cyapa
> *cyapa)
> > return ret;
> >
> > switch (cyapa->gen) {
> > + case CYAPA_GEN3:
> > + cyapa->ops = &cyapa_gen3_ops;
> > + break;
> > default:
> > cyapa->ops = &cyapa_default_ops;
> > cyapa->gen = CYAPA_GEN_UNKNOWN;
> > @@ -314,9 +326,85 @@ out:
> > */
> > static int cyapa_get_state(struct cyapa *cyapa)
> > {
> > + int ret;
> > + u8 status[BL_STATUS_SIZE];
> > + u8 cmd[32];
> > + /* The i2c address of gen4 and gen5 trackpad device must be even. */
> > + bool even_addr = ((cyapa->client->addr & 0x0001) == 0);
> > + bool smbus = false;
> > + int retries = 2;
> > +
> > cyapa->state = CYAPA_STATE_NO_DEVICE;
> >
> > - return -ENODEV;
> > + /*
> > + * Get trackpad status by reading 3 registers starting from 0.
> > + * If the device is in the bootloader, this will be BL_HEAD.
> > + * If the device is in operation mode, this will be the DATA regs.
> > + *
> > + */
> > + ret = cyapa_i2c_reg_read_block(cyapa, BL_HEAD_OFFSET, BL_STATUS_SIZE,
> > + status);
> > +
> > + /*
> > + * On smbus systems in OP mode, the i2c_reg_read will fail with
> > + * -ETIMEDOUT. In this case, try again using the smbus equivalent
> > + * command. This should return a BL_HEAD indicating CYAPA_STATE_OP.
> > + */
> > + if (cyapa->smbus && (ret == -ETIMEDOUT || ret == -ENXIO)) {
> > + if (!even_addr)
> > + ret = cyapa_read_block(cyapa,
> > + CYAPA_CMD_BL_STATUS, status);
> > + smbus = true;
> > + }
> > + if (ret != BL_STATUS_SIZE)
> > + goto error;
> > +
> > + /*
> > + * Detect trackpad protocol based on characristic registers and bits.
> > + */
> > + do {
> > + cyapa->status[REG_OP_STATUS] = status[REG_OP_STATUS];
> > + cyapa->status[REG_BL_STATUS] = status[REG_BL_STATUS];
> > + cyapa->status[REG_BL_ERROR] = status[REG_BL_ERROR];
> > +
> > + if (cyapa->gen == CYAPA_GEN_UNKNOWN ||
> > + cyapa->gen == CYAPA_GEN3) {
> > + ret = cyapa_gen3_ops.state_parse(cyapa,
> > + status, BL_STATUS_SIZE);
> > + if (ret == 0)
> > + goto out_detected;
> > + }
> > +
> > + /*
> > + * Cannot detect communication protocol based on current
> > + * charateristic registers and bits.
> > + * So write error command to do further detection.
> > + * this method only valid on I2C bus.
> > + * for smbus interface, it won't have overwrite issue.
> > + */
> > + if (!smbus) {
> > + cmd[0] = 0x00;
> > + cmd[1] = 0x00;
> > + ret = cyapa_i2c_write(cyapa, 0, 2, cmd);
> > + if (ret)
> > + goto error;
> > +
> > + msleep(50);
> > +
> > + ret = cyapa_i2c_read(cyapa, BL_HEAD_OFFSET,
> > + BL_STATUS_SIZE, status);
> > + if (ret < 0)
> > + goto error;
> > + }
> > + } while (--retries > 0 && !smbus);
> > +
> > + goto error;
> > +
> > +out_detected:
> > + return 0;
> > +
> > +error:
> > + return (ret < 0) ? ret : -EAGAIN;
> > }
> >
> > /*
> > @@ -994,11 +1082,17 @@ static void cyapa_detect_and_start(void *data,
> async_cookie_t cookie)
> >
> > static int cyapa_tp_modules_init(struct cyapa *cyapa)
> > {
> > + if (cyapa_gen3_ops.initialize)
> > + return cyapa_gen3_ops.initialize(cyapa);
>
> Same here.

Same reason as above.

>
> > +
> > return 0;
> > }
> >
> > static int cyapa_tp_modules_uninit(struct cyapa *cyapa)
> > {
> > + if (cyapa_gen3_ops.uninitialize)
> > + return cyapa_gen3_ops.uninitialize(cyapa);
>
> And here.
>
> Thanks.
>
> --
> Dmitry

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/