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

Mika Westerberg
2021-12-16T16:38:24Z

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.