On 12/22/18 3:03 AM, Marek Vasut wrote:
> On 12/22/2018 02:06 AM, Dmitry Torokhov wrote:
>> On Fri, Dec 21, 2018 at 09:47:34PM +0100, Marek Vasut wrote:
>>> 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 ?
>>
>> This really depends on the CPU and I am sure we will not have to live
>> with sucky CPUs for the rest of our lives. C++ crowd will want their
>> performance back. We should produce the most readable code and then rely
>> on the crazy architecture/compiler folks to fix up our mess as needed ;)
>>
>> So indirect calls via a const ops structure please.
>
> I'll have to live with that on the CortexA9 system I use that
> touchscreen at though. And this is a register IO, so it's a critical
> path IMO. I am a bit opposed to this ...
I tried to add the callback, but the resulting code looks horrible, so
I'll skip this part.
--
Best regards,
Marek Vasut
