linux-kernel - Re: [PATCH V2] Input: synaptics-rmi4- Add a new feature for Forcepad.

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]

Message-ID: <6sjnlz2zcstrsjgh5qxfmswlvwyjm5wiyz4wtlndprskw2aocr@icqoimso45wd>
Date: Wed, 23 Jul 2025 09:30:26 -0700
From: Dmitry Torokhov <dmitry.torokhov@...il.com>
To: Marge Yang <marge.yang@...synaptics.com>
Cc: linux-input@...r.kernel.org, linux-kernel@...r.kernel.org, 
	david.chiu@...synaptics.com, derek.cheng@...synaptics.com, sam.tsai@...aptics.com, 
	vincent.huang@...synaptics.com
Subject: Re: [PATCH V2] Input: synaptics-rmi4- Add a new feature for Forcepad.

Hi Marge,

On Wed, Jul 16, 2025 at 03:36:48AM +0000, Marge Yang wrote:
> +	f21->sensor_count = fn->fd.query_base_addr & (BIT(0) | BIT(1) | BIT(2) | BIT(3));

We could either use GENMASK or just 0x0f. BIT() is for individual bits.

> +
> +	if (fn->fd.query_base_addr & BIT(5)) {
> +		if (fn->fd.query_base_addr & BIT(6))
> +			f21->query15_offset = 2;
> +		else
> +			f21->query15_offset = 1;
> +
> +		rmi_read_block(fn->rmi_dev, fn->fd.query_base_addr + f21->query15_offset,
> +					f21->data_regs, 1);
> +		f21->max_number_Of_finger = f21->data_regs[0] & 0x0F;
> +	} else {
> +		dev_info(&fn->dev, "f21_query15 doesn't support.\n");
> +		f21->query15_offset = 0;
> +		f21->max_number_Of_finger = 5;
> +	}
> +
> +	if (fn->fd.query_base_addr & BIT(6)) {

Just double-checking - should it be BIT(5) give that reading of number of fingers
is gated by BIT(5) in the block above.

> +		dev_info(&fn->dev, "Support new F21 feature.\n");
> +		/*Each finger uses one byte, and the button state uses one byte.*/
> +		f21->attn_data_size = f21->max_number_Of_finger + 1;
> +		f21->attn_data_index_for_button = f21->attn_data_size - 1;
> +		/*
> +		 * Each sensor uses two bytes, the button state uses one byte,
> +		 * and each finger uses two bytes.
> +		 */
> +		f21->data_reg_size = f21->sensor_count * 2 + 1 +
> +								f21->max_number_Of_finger * 2;
> +		f21->data_reg_index_for_button = f21->sensor_count * 2;
> +	} else {
> +		dev_info(&fn->dev, "Support old F21 feature.\n");
> +		/*Each finger uses two bytes, and the button state uses one byte.*/
> +		f21->attn_data_size = f21->sensor_count * 2 + 1;
> +		f21->attn_data_index_for_button = f21->attn_data_size - 1;
> +		/*Each finger uses two bytes, and the button state uses one byte.*/
> +		f21->data_reg_size = f21->sensor_count * 2 + 1;
> +		f21->data_reg_index_for_button = f21->data_reg_size - 1;

The block is duplicated?

No need to resubmit the patch, please just provide the answer to the
above questions.

Thanks.

-- 
Dmitry