linux-kernel - Re: [PATCH v2 2/4] iio: adc: max1363: make use of iio_is_soft_ts

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: <20241219174500.18038922@jic23-huawei>
Date: Thu, 19 Dec 2024 17:45:00 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Vasileios Amoiridis <vassilisamir@...il.com>
Cc: lars@...afoo.de, krzysztof.kozlowski@...aro.org, nuno.sa@...log.com,
 u.kleine-koenig@...libre.com, abhashkumarjha123@...il.com,
 jstephan@...libre.com, dlechner@...libre.com, jackoalan@...il.com,
 k.wrona@...sung.com, linux-iio@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/4] iio: adc: max1363: make use of
 iio_is_soft_ts_enabled()

On Sat, 14 Dec 2024 20:14:19 +0100
Vasileios Amoiridis <vassilisamir@...il.com> wrote:

Patch needs a new title.  I'll fix it up.

Use a small fixed size buffer to replace dynamic allocation

with that, applied.

Thanks,


> Drop the recurrent allocation of the data buffer from the trigger
> handler and put it in the iio_priv(). This way, the maximum amount of
> channels is always allocated in favor of simpler code and drop
> of usage of the internal private variable "scan_timestamp" of the
> struct iio_dev.
> 
> Signed-off-by: Vasileios Amoiridis <vassilisamir@...il.com>
> ---
>  drivers/iio/adc/max1363.c | 30 +++++++++---------------------
>  1 file changed, 9 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/iio/adc/max1363.c b/drivers/iio/adc/max1363.c
> index 9a0baea08ab6..e8d731bc34e0 100644
> --- a/drivers/iio/adc/max1363.c
> +++ b/drivers/iio/adc/max1363.c
> @@ -161,6 +161,7 @@ struct max1363_chip_info {
>   * @vref_uv:		Actual (external or internal) reference voltage
>   * @send:		function used to send data to the chip
>   * @recv:		function used to receive data from the chip
> + * @data:		buffer to store channel data and timestamp
>   */
>  struct max1363_state {
>  	struct i2c_client		*client;
> @@ -186,6 +187,10 @@ struct max1363_state {
>  						const char *buf, int count);
>  	int				(*recv)(const struct i2c_client *client,
>  						char *buf, int count);
> +	struct {
> +		u8 buf[MAX1363_MAX_CHANNELS * 2];
> +		aligned_s64 ts;
> +	} data;
>  };
>  
>  #define MAX1363_MODE_SINGLE(_num, _mask) {				\
> @@ -1462,22 +1467,10 @@ static irqreturn_t max1363_trigger_handler(int irq, void *p)
>  	struct iio_poll_func *pf = p;
>  	struct iio_dev *indio_dev = pf->indio_dev;
>  	struct max1363_state *st = iio_priv(indio_dev);
> -	__u8 *rxbuf;
>  	int b_sent;
> -	size_t d_size;
>  	unsigned long numvals = bitmap_weight(st->current_mode->modemask,
>  					      MAX1363_MAX_CHANNELS);
>  
> -	/* Ensure the timestamp is 8 byte aligned */
> -	if (st->chip_info->bits != 8)
> -		d_size = numvals*2;
> -	else
> -		d_size = numvals;
> -	if (indio_dev->scan_timestamp) {
> -		d_size += sizeof(s64);
> -		if (d_size % sizeof(s64))
> -			d_size += sizeof(s64) - (d_size % sizeof(s64));
> -	}
>  	/* Monitor mode prevents reading. Whilst not currently implemented
>  	 * might as well have this test in here in the meantime as it does
>  	 * no harm.
> @@ -1485,21 +1478,16 @@ static irqreturn_t max1363_trigger_handler(int irq, void *p)
>  	if (numvals == 0)
>  		goto done;
>  
> -	rxbuf = kmalloc(d_size,	GFP_KERNEL);
> -	if (rxbuf == NULL)
> -		goto done;
>  	if (st->chip_info->bits != 8)
> -		b_sent = st->recv(st->client, rxbuf, numvals * 2);
> +		b_sent = st->recv(st->client, st->data.buf, numvals * 2);
>  	else
> -		b_sent = st->recv(st->client, rxbuf, numvals);
> +		b_sent = st->recv(st->client, st->data.buf, numvals);
>  	if (b_sent < 0)
> -		goto done_free;
> +		goto done;
>  
> -	iio_push_to_buffers_with_timestamp(indio_dev, rxbuf,
> +	iio_push_to_buffers_with_timestamp(indio_dev, &st->data,
>  					   iio_get_time_ns(indio_dev));
>  
> -done_free:
> -	kfree(rxbuf);
>  done:
>  	iio_trigger_notify_done(indio_dev->trig);
>