linux-kernel - Re: [PATCH v1 1/2] spi: amlogic-spifc-a1: implement adjust_op

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: <ZKVDMV95gZN4oQjx@surfacebook>
Date:   Wed, 5 Jul 2023 13:17:21 +0300
From:   andy.shevchenko@...il.com
To:     Martin Kurbanov <mmkurbanov@...rdevices.ru>
Cc:     Mark Brown <broonie@...nel.org>,
        Neil Armstrong <neil.armstrong@...aro.org>,
        linux-spi@...r.kernel.org, linux-amlogic@...ts.infradead.org,
        linux-kernel@...r.kernel.org, kernel@...rdevices.ru
Subject: Re: [PATCH v1 1/2] spi: amlogic-spifc-a1: implement adjust_op_size()

Mon, Jul 03, 2023 at 12:45:17PM +0300, Martin Kurbanov kirjoitti:
> This enhancement eliminates the need for a loop in the
> amlogic_spifc_a1_exec_op() function and allows the SPI core to
> dynamically divide transactions into appropriately sized chunks.

...

> +	if (!data_size) {

Why not positive check?

> +		ret = amlogic_spifc_a1_request(spifc, false);
> +	} else {
> +		u32 mode = ilog2(op->data.buswidth);
>  
>  		writel(0, spifc->base + SPIFC_A1_USER_DBUF_ADDR_REG);
>  
>  		if (op->data.dir == SPI_MEM_DATA_IN)
> -			ret = amlogic_spifc_a1_read(spifc,
> -						    op->data.buf.in + off,
> -						    block_size, dmode);
> +			ret = amlogic_spifc_a1_read(spifc, op->data.buf.in,
> +						    data_size, mode);
>  		else
> -			ret = amlogic_spifc_a1_write(spifc,
> -						     op->data.buf.out + off,
> -						     block_size, dmode);
> -
> -		nbytes -= block_size;
> -		off += block_size;
> -	} while (nbytes != 0 && !ret);
> +			ret = amlogic_spifc_a1_write(spifc, op->data.buf.out,
> +						     data_size, mode);
> +	}

...

> +static int amlogic_spifc_a1_adjust_op_size(struct spi_mem *mem,
> +					   struct spi_mem_op *op)
> +{
> +	op->data.nbytes = min_t(u32, op->data.nbytes, SPIFC_A1_BUFFER_SIZE);

I would rather use min() for strict type checking.
In this case it will require to have U/u suffix in the SPIFC_A1_BUFFER_SIZE
definition.

> +	return 0;
> +}

-- 
With Best Regards,
Andy Shevchenko