Hi Michael,
On Thu, Oct 01, 2020 at 02:29:48PM +0200, michael.srba@xxxxxxxxx wrote:
> From: Michael Srba <Michael.Srba@xxxxxxxxx>
>
> Add support for the bt541 touchscreen IC from zinitix, loosely based on
> downstream driver. The driver currently supports multitouch (5 touch points).
> The bt541 seems to support touch keys, but the support was not added because
> that functionality is not being utilized by the touchscreen used for testing.
> Based on the similartities between downstream drivers, it seems likely that
> other similar touchscreen ICs can be supported with this driver in the future.
>
> Signed-off-by: Michael Srba <Michael.Srba@xxxxxxxxx>
> ---
> changes in v2: applied fixes per recommendation, added support for suspend/resume handling
> changes in v3: added support for working in different touch point report modes
> (modes 1 and 2 are now supported). mode 2 seems to work just fine
> on Samsung Galaxy A3 (2015), and also works on Samsung Galaxy S4
> Mini Value Edition (where mode 1 doesn't seem to work because
> of different firmware version). It is expected that other fw
> versions, and models other than bt541, may have either mode broken
> (vendor doesn't use it -> they don't care) .
> changes in v4: - removed mode 1 for now to simplify the code and improve it's chances
> to get accepted.
> - added runtime pm to save power while not using the ts
> - refactored i2c helper functions
> -
> changes in v5: - send the actual intended V4 (sorry)
> changes in v6: - remove unused include
> - other fixes per reccomendation
Applied with some edits:
> +
> +static int zinitix_read_data(struct i2c_client *client, u16 reg, u8 *values, size_t length)
> +{
> + int ret;
> + __le16 reg_le = cpu_to_le16(reg);
> +
Added a comment that single i2c_transfer will not work.
> + ret = i2c_master_send(client, (u8 *)®_le, sizeof(reg_le));
> + if (ret != sizeof(reg_le))
> + return ret < 0 ? ret : -EIO;
> +
> + ret = i2c_master_recv(client, values, length);
> + if (ret != length)
> + return ret;
Changed to:
return ret < 0 ? ret : -EIO;
> + error = zinitix_write_u16(client, BT541_X_RESOLUTION, (u16)bt541->prop.max_x);
No need for explicit cast as far as I can see, removed.
> +
> + for (i = 0; i < MAX_SUPPORTED_FINGER_NUM; i++) {
> + if (!(touch_event.point_coord[i].sub_status & SUB_BIT_EXIST))
> + continue;
> +
> + input_mt_slot(bt541->input_dev, i);
> + input_mt_report_slot_state(bt541->input_dev, MT_TOOL_FINGER, true);
> + touchscreen_report_pos(bt541->input_dev, &bt541->prop,
> + le16_to_cpu(touch_event.point_coord[i].x),
> + le16_to_cpu(touch_event.point_coord[i].y), true);
> + input_report_abs(bt541->input_dev, ABS_MT_TOUCH_MAJOR,
> + touch_event.point_coord[i].width);
Factored out into a helper.
> +
> + error = input_register_device(bt541->input_dev);
> + if (error) {
> + dev_err(&bt541->client->dev,
> + "Failed to register input device: %d", error);
> + return error;
> + }
> +
> + input_set_drvdata(bt541->input_dev, bt541);
This is too late, as open and close that use input_get_drvdata() can be
called before input_register_device() returns. Moved earlier.
> +
> + return 0;
> +}
> +
> +static int zinitix_ts_probe(struct i2c_client *client, const struct i2c_device_id *id)
Dropped the last argument and used as probe_new() callback.
> +
> +static int __maybe_unused zinitix_suspend(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct bt541_ts_data *bt541 = i2c_get_clientdata(client);
> +
> + /* Release all fingers */
> + input_mt_sync_frame(bt541->input_dev);
> + input_sync(bt541->input_dev);
As I mentioned, if we need this, this should be done in input core and
not in each individual driver.
I dropped this chunk for now.
Thanks.
--
Dmitry
