linux-kernel - Re: [PATCH v4 2/2] Input: add MStar MSG2638 touchscreen driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 20 Feb 2021 15:23:51 -0800
From:   Dmitry Torokhov <dmitry.torokhov@...il.com>
To:     Vincent Knecht <vincent.knecht@...loo.org>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Henrik Rydberg <rydberg@...math.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Michael Srba <Michael.Srba@...nam.cz>,
        linux-input@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, ~postmarketos/upstreaming@...ts.sr.ht
Subject: Re: [PATCH v4 2/2] Input: add MStar MSG2638 touchscreen driver

Hi Vincent,

On Wed, Feb 10, 2021 at 06:33:52PM +0100, Vincent Knecht wrote:
> +
> +	for (i = 0; i < MAX_SUPPORTED_FINGER_NUM; i++) {
> +		p = &touch_event.pkt[i];
> +		/* Ignore non-pressed finger data */
> +		if (p->xy_hi == 0xFF && p->x_low == 0xFF && p->y_low == 0xFF)
> +			continue;
> +
> +		coord.x = (((p->xy_hi & 0xF0) << 4) | p->x_low) * msg2638->prop.max_x / TPD_WIDTH;
> +		coord.y = (((p->xy_hi & 0x0F) << 8) | p->y_low) * msg2638->prop.max_y / TPD_HEIGHT;
> +		msg2638_report_finger(msg2638, i, &coord);

We do not scale the coordinates in the kernel. Rather we provide
resolution, if known, and min/max coordinates reported by the hardware,
and let userspace handle the rest.

> +static int __maybe_unused msg2638_suspend(struct device *dev)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct msg2638_ts_data *msg2638 = i2c_get_clientdata(client);
> +
> +	mutex_lock(&msg2638->input_dev->mutex);
> +
> +	if (input_device_enabled(msg2638->input_dev))
> +		msg2638_stop(msg2638);

I believe that you should power down the device only if it is not
configures as wakeup source. In fact (and I think most drivers are
wrong in this), you may want to power up the device if it is a wakeup
source and it does not have any users.

Thanks.

-- 
Dmitry