lkml.org 
[lkml]   [2018]   [Mar]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] Input: add bu21029 touch driver
Date
Hello Dmitry,

> > +++ b/drivers/input/touchscreen/bu21029_ts.c
> > @@ -0,0 +1,456 @@
>
> Please add SPDX tag for the driver.

We will do.

> Please use GPIOD API (and include linux/gpio/consumer.h instead of
> of_gpio.h).

We will do the change.

> > +/* HW_ID1 Register (PAGE=0, ADDR=0x0E, Reset value=0x02, Read only)
>
> Multi-line comments start with empty comment line:
>
> /*
> * Multi
> * line.
> */

Will be fixed.

> > + /* calculate Rz (pressure resistance value) by equation:
> > + * Rz = Rx * (x/Q) * ((z2/z1) - 1), where
> > + * Rx is x-plate resistance,
> > + * Q is the touch screen resolution (8bit = 256, 12bit = 4096)
> > + * x, z1, z2 are the measured positions.
> > + */
> > + rz = z2 - z1;
> > + rz *= x;
> > + rz *= bu21029->x_plate_ohms;
> > + rz /= z1;
> > + rz = DIV_ROUND_CLOSEST(rz, SCALE_12BIT);
> > + if (rz <= bu21029->max_pressure) {
> > + input_report_abs(bu21029->in_dev, ABS_X, x);
> > + input_report_abs(bu21029->in_dev, ABS_Y, y);
> > + input_report_abs(bu21029->in_dev, ABS_PRESSURE, rz);
>
> What is the values of pressure reported when finger is touching the
> surface? IOW is 'rz' pressure or resistance?

Rz is pressure measured in Ohms. That is, it is a resistance which
correlates with finger pressure.

I fear that I do not understand your question. Does ABS_PRESSURE have
to be reported in a specific unit, e.g. milli Newton? We thought that
it is a device specific scale and that it will be converted into a
calibrated value (just like the coordinates) in user space.

> > + if (of_property_read_u32(np, "touchscreen-max-pressure", &val32))
> > + bu21029->max_pressure = MAX_12BIT;
> > + else
> > + bu21029->max_pressure = val32;
>
> Please use infrastructure form include/linux/input/touchscreen.h
> so that you handle different sizes and orientations.

Thank you for this recommendation. We will check it out and send a new
proposal.

> > + in_dev->name = DRIVER_NAME;
> > + in_dev->id.bustype = BUS_I2C;
> > + in_dev->dev.parent = &client->dev;
>
> Not needed with devm_input_allocate_device().

We will have a look at the other drivers.

> > +static int bu21029_remove(struct i2c_client *client)
> > +{
> > + struct bu21029_ts_data *bu21029 = i2c_get_clientdata(client);
> > +
> > + del_timer_sync(&bu21029->timer);
>
> If interrupt comes here kernel will be unhappy. You need to either work
> canceling timer into devm unwid stream (devm_add_action_or_reset()) or
> somehow make sure that you shut off interrupts before canceling the
> timer.

We will work on a solution. Thank you for spotting this.

>
> > +
> > + return 0;
> > +}

> > +static struct i2c_driver bu21029_driver = {
> > + .driver = {
> > + .name = DRIVER_NAME,
> > + .owner = THIS_MODULE,
>
> Not needed.

OK, we will remove the .owner assignment.

Greetings,
Mark Jonas

Building Technologies, Panel Software Fire (BT-FIR/ENG1)
Bosch Sicherheitssysteme GmbH | Postfach 11 11 | 85626 Grasbrunn | GERMANY | www.boschsecurity.com

Sitz: Stuttgart, Registergericht: Amtsgericht Stuttgart HRB 23118
Aufsichtsratsvorsitzender: Stefan Hartung; Geschäftsführung: Gert van Iperen, Andreas Bartz, Thomas Quante, Bernhard Schuster

\
 
 \ /
  Last update: 2018-03-27 08:57    [W:2.694 / U:0.052 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site