On Tue, Jun 20, 2017 at 12:32:03AM +0200, Bastien Nocera wrote:
> On Sat, 2017-06-17 at 19:42 +1000, Sergei A. Trusov wrote:
> > On some x86 tablets with a goodix touchscreen the windows logo on the
> > front is a capacitive home button. Touching this button results in a
> > touch
> > with bit 4 of the 0th byte set, while normally only the lower 4 bits
> > are used to indicate the number of touches.
> >
> > Detect this and report a KEY_LEFTMETA press when this happens. Note
> > the
> > hardware might support more than one button, the number of a button
> > is
> > reported by the 'id' byte of coor_data. Now we ignore button id.
> >
> > Signed-off-by: Sergei A. Trusov <sergei.a.trusov@xxxxx>
> > ---
> > drivers/input/touchscreen/goodix.c | 20 +++++++++++++++-----
> > 1 file changed, 15 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/input/touchscreen/goodix.c
> > b/drivers/input/touchscreen/goodix.c
> > index 240b16f3ee97..a4c17c461792 100644
> > --- a/drivers/input/touchscreen/goodix.c
> > +++ b/drivers/input/touchscreen/goodix.c
> > @@ -76,6 +76,10 @@ struct goodix_ts_data {
> > #define MAX_CONTACTS_LOC 5
> > #define TRIGGER_LOC 6
> >
> > +#define GOODIX_TOUCH_NUM_MASK 0x0f
> > +#define GOODIX_SOFTBUTTON_MASK 0x10
> > +#define GOODIX_SOFTBUTTON1 KEY_LEFTMETA
> > +
> > static const unsigned long goodix_irq_flags[] = {
> > IRQ_TYPE_EDGE_RISING,
> > IRQ_TYPE_EDGE_FALLING,
> > @@ -208,7 +212,7 @@ static int goodix_ts_read_input_report(struct
> > goodix_ts_data *ts, u8 *data)
> > if (!(data[0] & 0x80))
> > return -EAGAIN;
> >
> > - touch_num = data[0] & 0x0f;
> > + touch_num = data[0] & GOODIX_TOUCH_NUM_MASK;
>
> This should be a separate patch.
>
> > if (touch_num > ts->max_touch_num)
> > return -EPROTO;
> >
> > @@ -223,7 +227,7 @@ static int goodix_ts_read_input_report(struct
> > goodix_ts_data *ts, u8 *data)
> > return error;
> > }
> >
> > - return touch_num;
> > + return data[0];
>
> I really don't like it when we change the meaning of a function's
> return value without changing the function's name, or explaining that
> we're doing that. This should be a separate patch please.
>
> > }
> >
> > static void goodix_ts_report_touch(struct goodix_ts_data *ts, u8
> > *coor_data)
> > @@ -260,13 +264,17 @@ static void goodix_ts_report_touch(struct
> > goodix_ts_data *ts, u8 *coor_data)
> > static void goodix_process_events(struct goodix_ts_data *ts)
> > {
> > u8 point_data[1 + GOODIX_CONTACT_SIZE *
> > GOODIX_MAX_CONTACTS];
> > - int touch_num;
> > + int touch_num, status;
> > int i;
> >
> > - touch_num = goodix_ts_read_input_report(ts, point_data);
> > - if (touch_num < 0)
> > + status = goodix_ts_read_input_report(ts, point_data);
> > + if (status < 0)
> > return;
> >
> > + input_report_key(ts->input_dev, GOODIX_SOFTBUTTON1,
> > + !!(status & GOODIX_SOFTBUTTON_MASK));
> > +
> > + touch_num = status & GOODIX_TOUCH_NUM_MASK;
> > for (i = 0; i < touch_num; i++)
> > goodix_ts_report_touch(ts,
> > &point_data[1 + GOODIX_CONTACT_SIZE
> > * i]);
> > @@ -612,6 +620,8 @@ static int goodix_request_input_dev(struct
> > goodix_ts_data *ts)
> > ts->input_dev->id.product = ts->id;
> > ts->input_dev->id.version = ts->version;
> >
> > + input_set_capability(ts->input_dev, EV_KEY,
> > GOODIX_SOFTBUTTON1);
>
> I don't like this define though, please use KEY_LEFTMETA directly. We
> can change it to a constant when we have more than one softbutton.
>
> General question though, should we not only export and advertise the
> button if it exists on the device?
Yes, this is preferred behavior.
>
> > +
> > error = input_register_device(ts->input_dev);
> > if (error) {
> > dev_err(&ts->client->dev,
> >
--
Dmitry
--
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
