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

[Mika Westerberg <mika.westerberg@linux.intel.com>]

Hi,

On Thu, Dec 16, 2021 at 11:51:00AM +0100, Boris Brezillon wrote:
> On Thu, 18 Nov 2021 16:05:42 +0300
> Mika Westerberg <mika.westerberg@linux.intel.com> 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.

Indeed. I will change it to:

	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;
	} else if (op->addr.nbytes > 0) {
		return false;
	}

in v5 if that's what you meant.