Re: [PATCH v3 2/7] Input: synaptics-rmi4 - handle duplicate/unknown PDT entries

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

Hi David,

On Sat, Mar 08, 2025 at 03:08:38PM +0100, David Heidelberg via B4 Relay wrote:
> From: Caleb Connolly <caleb.connolly@linaro.org>
> 
> Some third party rmi4-compatible ICs don't expose their PDT entries
> very well. Add a few checks to skip duplicate entries as well as entries
> for unsupported functions.
> 
> This is required to support some phones with third party displays.
> 
> Validated on a stock OnePlus 6T (original parts):
> manufacturer: Synaptics, product: S3706B, fw id: 2852315
> 
> Co-developed-by: methanal <baclofen@tuta.io>
> Signed-off-by: methanal <baclofen@tuta.io>
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> Signed-off-by: David Heidelberg <david@ixit.cz>
> ---
>  drivers/input/rmi4/rmi_driver.c | 47 +++++++++++++++++++++++++++++++++++------
>  drivers/input/rmi4/rmi_driver.h |  6 ++++++
>  2 files changed, 47 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
> index 2168b6cd7167334d44553c9c566f870a4e034179..51c23a407b2731d5b6eaefe9cae6288f97316e34 100644
> --- a/drivers/input/rmi4/rmi_driver.c
> +++ b/drivers/input/rmi4/rmi_driver.c
> @@ -493,12 +493,44 @@ static void rmi_driver_copy_pdt_to_fd(const struct pdt_entry *pdt,
>  	fd->function_version = pdt->function_version;
>  }
>  
> +static bool rmi_pdt_entry_is_valid(struct rmi_device *rmi_dev,
> +				   struct pdt_scan_state *state, u8 fn)
> +{
> +	unsigned int i;
> +
> +	switch (fn) {
> +	case 0x01:
> +	case 0x03:
> +	case 0x11:
> +	case 0x12:
> +	case 0x30:
> +	case 0x34:
> +	case 0x3a:
> +	case 0x54:
> +	case 0x55:

This mean that we need to update this code any time there is new
function introduced. I'd rather we did not do that. The driver should be
able to handle unknown functions.

> +		break;
> +
> +	default:
> +		rmi_dbg(RMI_DEBUG_CORE, &rmi_dev->dev,
> +			"PDT has unknown function number %#02x\n", fn);
> +		return false;
> +	}
> +
> +	for (i = 0; i < state->pdt_count; i++) {
> +		if (state->pdts[i] == fn)
> +			return false;
> +	}
> +
> +	state->pdts[state->pdt_count++] = fn;

Duplicate detection could be handled thorough a bitmap.

Thanks.

-- 
Dmitry