linux-kernel - Re: [PATCH v6 3/3] iio: proximity: Add driver support for TYHX's HX9023S capacitive proximity sensor

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: <20240623125621.07d1617c@jic23-huawei>
Date: Sun, 23 Jun 2024 12:56:21 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Yasin Lee <yasin.lee.x@...il.com>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Lars-Peter Clausen <lars@...afoo.de>,
 yasin.lee.x@...look.com, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-iio@...r.kernel.org
Subject: Re: [PATCH v6 3/3] iio: proximity: Add driver support for TYHX's
 HX9023S capacitive proximity sensor

On Fri, 21 Jun 2024 15:40:51 +0800
Yasin Lee <yasin.lee.x@...il.com> wrote:

> A SAR sensor from NanjingTianyihexin Electronics Ltd.
> 
> The device has the following entry points:
> 
> Usual frequency:
> - sampling_frequency
> 
> Instant reading of current values for different sensors:
> - in_proximity0_raw
> - in_proximity1_raw
> - in_proximity2_raw
> - in_proximity3_raw
> - in_proximity4_raw
> and associated events in events/
> 
> Signed-off-by: Yasin Lee <yasin.lee.x@...il.com>

Hi Yasin,

Some good reviews in already for this version, so I only took a quick look
this time. It seems to be in a reasonable state now.

Jonathan

> diff --git a/drivers/iio/proximity/hx9023s.c b/drivers/iio/proximity/hx9023s.c
> new file mode 100644
> index 000000000000..1d8cb9a05d8a
> --- /dev/null
> +++ b/drivers/iio/proximity/hx9023s.c
>
> +struct hx9023s_data {
> +	struct iio_trigger *trig;
> +	struct regmap *regmap;
> +	unsigned long chan_prox_stat;
> +	unsigned long chan_read;
> +	unsigned long chan_event;
> +	unsigned long ch_en_stat;
> +	unsigned long chan_in_use;
> +	unsigned int prox_state_reg;
> +	bool trigger_enabled;
> +
> +	struct {
> +		__le16 channels[HX9023S_CH_NUM];
> +		s64 ts __aligned(8);
> +	} buffer;
> +
> +	struct mutex mutex;

Add a comment explaining the data this mutex is protecting
(that may be in this structure, or for example on the device)

> +	struct hx9023s_ch_data ch_data[HX9023S_CH_NUM];
> +};


> +
> +static int hx9023s_sample(struct hx9023s_data *data)
> +{
...
> +
> +	for (i = 0; i < HX9023S_CH_NUM; i++) {
> +		value = get_unaligned_le16(&rx_buf[i * data_size + 1]);
> +		value = sign_extend32(value, 15);
> +		data->ch_data[i].lp = 0;
> +		data->ch_data[i].diff = 0;
> +		if (data->ch_data[i].sel_lp == true)
> +			data->ch_data[i].lp = value;
> +		if (data->ch_data[i].sel_diff == true)
> 
Run checkpatch.pl --strict and it will probably moan about these.
if (data->ch_data[i].sel_diff) is the same thing so just use that.