Hi Simon,
> > I would very much prefer if the driver functioned well without
> > such settings, since they complicate userspace and are not likely
> > to ever go away. Oh well.
>
> I would prefer to have the touchscreen adjust itself as well,
> unfortunately this is not available and you definitely need different
> settings depending for different touch setups.
>
> Would an ioctl() be more acceptable? Would make it harder to adjust it
> in startup scripts etc. though.
The format (ioctl, sysfs or whatnot) is of no consequence, it is the
a) special handling of certain drivers and b) forcing userspace to
tune the driver that makes maintenance problematic in the long
run. With every screen project ending in its own solution, this
quickly becomes a problem, both in the kernel and in
userspace. Preventing such predicaments is my concern.
>
> >> + if (error) { + dev_err(&tsdata->client->dev, + "Unable to
> >> write to fetch data, error: %d\n", error); + goto out; + }
> >
> > No risk of flooding the logs here? Perhaps rate-limit the outputs?
>
> Hmm, possible. Can you point me to a driver that does this in a sane
> fashion?
Different methods all over the kernel; there is dev_err_ratelimited(),
pr_warn_once(), printk_once(), special solutions... take you pick.
>
> >> + for (i = 0; i < MAX_SUPPORT_POINTS; i++) { + u8 *buf =
> >> &rdbuf[i * 4]; + bool down; + + type = buf[5] >> 6; + /*
> >> ignore Reserved events */ + if (type == TOUCH_EVENT_RESERVED) +
> >> continue;
> >
> > As per the implementation by Olivier, it seems these touches may
> > get stuck in the down position. No?
>
> Not if you do the loop over all 5 event entries in the report. The
> n_touches field really contains the number of fingers on the touch,
> not the number of events in the report. Since the "down"-events
> conveniently are sorted to the beginning of the report for the type A
> protocol it was enough to to just iterate over these (we ignored the
> UP-events anyway). Now that we need them we just iterate over all of
> the events and there they are.
Ok, good.
>
> >> + tsdata->threshold = edt_ft5x06_i2c_register_read(tsdata, +
> >> WORK_REGISTER_THRESHOLD); + tsdata->gain =
> >> edt_ft5x06_i2c_register_read(tsdata, + WORK_REGISTER_GAIN);
> >> + tsdata->offset = edt_ft5x06_i2c_register_read(tsdata, +
> >> WORK_REGISTER_OFFSET); + tsdata->report_rate =
> >> edt_ft5x06_i2c_register_read(tsdata, +
> >> WORK_REGISTER_REPORT_RATE); + tsdata->num_x =
> >> edt_ft5x06_i2c_register_read(tsdata, +
> >> WORK_REGISTER_NUM_X); + tsdata->num_y =
> >> edt_ft5x06_i2c_register_read(tsdata, +
> >> WORK_REGISTER_NUM_Y); + + dev_dbg(&client->dev, + "Model \"%s\",
> >> Rev. \"%s\", %dx%d sensors\n", + tsdata->name, fw_version,
> >> tsdata->num_x, tsdata->num_y); + + input->name = tsdata->name; +
> >> input->id.bustype = BUS_I2C; + input->dev.parent = &client->dev;
> >> + + __set_bit(EV_SYN, input->evbit); + __set_bit(EV_KEY,
> >> input->evbit); + __set_bit(EV_ABS, input->evbit); +
> >> __set_bit(BTN_TOUCH, input->keybit); +
> >> input_set_abs_params(input, ABS_X, 0, tsdata->num_x * 64 - 1, 0,
> >> 0); + input_set_abs_params(input, ABS_Y, 0, tsdata->num_y * 64 -
> >> 1, 0, 0); + input_set_abs_params(input, ABS_MT_POSITION_X, +
> >> 0, tsdata->num_x * 64 - 1, 0, 0); + input_set_abs_params(input,
> >> ABS_MT_POSITION_Y, + 0, tsdata->num_y * 64 - 1, 0, 0); +
> >> input_mt_init_slots(input, MAX_SUPPORT_POINTS);
> >
> > No error checking here?
>
> I guess you're referring to the _register_read()s? Yeah, they probably
> could use some.
I was thinking of input_mt_init_slots(). Yes, not all drivers check it
because the kernel wont crash, but it wont function as planned either.
Thanks,
Henrik
--
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
