Hi Michał,
On Fri, Dec 11, 2020 at 07:53:56AM +0100, Michał Mirosław wrote:
> +
> + if (!phy_x || !phy_y) {
> + dev_warn(&client->dev,
> + "invalid size data: %d x %d mm\n",
> + phy_x, phy_y);
> + return 0;
Given we are not treating this as hard error mind dropping this "return"
and making the below an "else" branch?
> + }
> +
> + dev_dbg(&client->dev, "phy_x=%d, phy_y=%d\n", phy_x, phy_y);
> +
> + /* calculate resolution from size */
> + ts->x_max = 2240-1;
> + ts->x_res = DIV_ROUND_CLOSEST(ts->prop.max_x, phy_x);
> +
> + ts->y_max = 1408-1;
> + ts->y_res = DIV_ROUND_CLOSEST(ts->prop.max_y, phy_y);
> +
> + return 0;
> +}
> +
>
> + if (client->dev.of_node)
> + ts->chip_id = (uintptr_t)of_device_get_match_data(&client->dev);
Why don't we add EKTH3500 tag to the ACPI entry and make this
unconditional device_get_match_data()?
> +
> ts->vcc33 = devm_regulator_get(&client->dev, "vcc33");
> if (IS_ERR(ts->vcc33)) {
> error = PTR_ERR(ts->vcc33);
> @@ -1495,7 +1568,8 @@ MODULE_DEVICE_TABLE(acpi, elants_acpi_id);
>
> #ifdef CONFIG_OF
> static const struct of_device_id elants_of_match[] = {
> - { .compatible = "elan,ekth3500" },
> + { .compatible = "elan,ekth3500", .data = (void *)EKTH3500 },
> + { .compatible = "elan,ektf3624", .data = (void *)EKTF3624 },
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, elants_of_match);
> --
> 2.20.1
>
Thanks.
--
Dmitry
