On 12/21/2018 09:22 AM, Dmitry Torokhov wrote:
> On Fri, Dec 21, 2018 at 03:27:08AM +0100, Marek Vasut wrote:
>> On 12/21/2018 02:55 AM, Dmitry Torokhov wrote:
>>> On Thu, Dec 20, 2018 at 09:43:05PM +0100, Marek Vasut wrote:
>>>> Add support for ILI251x touch controller. This controller is similar
>>>> to the ILI210x, except for the following differences:
>>>> - Does not support I2C R-W transfer, Read must be followed by an
>>>> obscenely long delay, and then followed by Write
>>>> - Does support 10 simultaneous touch inputs.
>>>> - Touch data format is slightly different, pressure reporting does not
>>>> work although the touch data contain such information.
>>>>
>>>> Signed-off-by: Marek Vasut <marex@xxxxxxx>
>>>> Cc: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
>>>> Cc: Henrik Rydberg <rydberg@xxxxxxxxxxx>
>>>> Cc: Olivier Sobrie <olivier@xxxxxxxxx>
>>>> Cc: Philipp Puschmann <pp@xxxxxxxxx>
>>>> To: linux-input@xxxxxxxxxxxxxxx
>>>> ---
>>>> V2: - Implement delayed work for ILI251x
>>>> - Fix operation with >6 fingers in ili210x_work
>>>> ---
>>>> drivers/input/touchscreen/ili210x.c | 128 ++++++++++++++++++++++++----
>>>> 1 file changed, 113 insertions(+), 15 deletions(-)
>>>>
>>>> diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
>>>> index bb77d37aaaba..5099653dfb88 100644
>>>> --- a/drivers/input/touchscreen/ili210x.c
>>>> +++ b/drivers/input/touchscreen/ili210x.c
>>>> @@ -6,8 +6,10 @@
>>>> #include <linux/input/mt.h>
>>>> #include <linux/delay.h>
>>>> #include <linux/gpio/consumer.h>
>>>> +#include <linux/of_device.h>
>>>>
>>>> -#define MAX_TOUCHES 2
>>>> +#define ILI210X_TOUCHES 2
>>>> +#define ILI251X_TOUCHES 10
>>>> #define DEFAULT_POLL_PERIOD 20
>>>>
>>>> /* Touchscreen commands */
>>>> @@ -31,17 +33,25 @@ struct firmware_version {
>>>> u8 minor;
>>>> } __packed;
>>>>
>>>> +enum ili2xxx_model {
>>>> + MODEL_ILI210X,
>>>> + MODEL_ILI251X,
>>>> +};
>>>> +
>>>> struct ili210x {
>>>> struct i2c_client *client;
>>>> struct input_dev *input;
>>>> unsigned int poll_period;
>>>> struct delayed_work dwork;
>>>> struct gpio_desc *reset_gpio;
>>>> + enum ili2xxx_model model;
>>>> + unsigned int max_touches;
>>>> };
>>>>
>>>> static int ili210x_read_reg(struct i2c_client *client, u8 reg, void *buf,
>>>> size_t len)
>>>> {
>>>> + struct ili210x *priv = i2c_get_clientdata(client);
>>>> struct i2c_msg msg[2] = {
>>>> {
>>>> .addr = client->addr,
>>>> @@ -57,7 +67,38 @@ static int ili210x_read_reg(struct i2c_client *client, u8 reg, void *buf,
>>>> }
>>>> };
>>>>
>>>> - if (i2c_transfer(client->adapter, msg, 2) != 2) {
>>>> + if (priv->model == MODEL_ILI251X) {
>>>
>>> Instead of doing conditional maybe define "ops" structure and tie it to
>>> i2c and of table entries and call via pointers here and in coordinate
>>> processing?
>>
>> Only for the read function or others as well ?
>
> I think all where you currently branch depending on the model.
I had a discussion with netdev people and they mentioned these indirect
function calls now have a lot of overhead due to spectre/meltdown
mitigations. Do we care here or shall I just use those indirect calls ?
--
Best regards,
Marek Vasut
