Re: [PATCH RESEND v8 3/4] input: elants: read touchscreen size for EKTF3624

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

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