Re: [PATCH v4 2/3] mtd: spi-nor: intel-spi: Convert to SPI MEM

[Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>]

On Thu, 18 Nov 2021 16:05:42 +0300
Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx> wrote:

> +static bool intel_spi_cmp_mem_op(const struct intel_spi_mem_op *iop,
> +				 const struct spi_mem_op *op)
> +{
> +	if (iop->mem_op.cmd.nbytes != op->cmd.nbytes ||
> +	    iop->mem_op.cmd.buswidth != op->cmd.buswidth ||
> +	    iop->mem_op.cmd.dtr != op->cmd.dtr ||
> +	    iop->mem_op.cmd.opcode != op->cmd.opcode)
> +		return false;
> +
> +	if (iop->mem_op.addr.nbytes) {
> +		if (iop->mem_op.addr.nbytes != op->addr.nbytes ||
> +		    iop->mem_op.addr.dtr != op->addr.dtr)
> +			return false;
> +	}

Hm, are you sure you want to allow op->addr.nbytes > 0 when
iop->mem_op.addr.nbytes == 0? Feels like the command should be reported
as unsupported in that case. Unless 0 is a wildcard meaning 'any', but
that would be confusing, since operations with 0 address bytes are
valid, and I actually expect the number of address cycles to be fixed 
or bounded.

> +
> +	if (iop->mem_op.data.dir != op->data.dir ||
> +	    iop->mem_op.data.dtr != op->data.dtr)
> +		return false;
> +
> +	if (iop->mem_op.data.dir != SPI_MEM_NO_DATA) {
> +		if (iop->mem_op.data.buswidth != op->data.buswidth)
> +			return false;
> +	}
> +
> +	return true;
> +}