Re: [RFC PATCH 1/6] spi: Extend the core to ease integration of SPI memory controllers

[Boris Brezillon <boris.brezillon@bootlin.com>]

On Wed, 28 Feb 2018 15:51:29 +0800
Peter Pan <peterpansjtu@gmail.com> wrote:

> > +int spi_mem_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
> > +{
> > +       unsigned int tmpbufsize, xferpos = 0, totalxferlen = 0;
> > +       struct spi_controller *ctlr = mem->spi->controller;
> > +       struct spi_transfer xfers[4] = { };
> > +       struct spi_message msg;
> > +       u8 *tmpbuf;
> > +       int ret;
> > +
> > +       if (!spi_mem_supports_op(mem, op))
> > +               return -ENOTSUPP;
> > +
> > +       if (ctlr->mem_ops) {
> > +               if (ctlr->auto_runtime_pm) {
> > +                       ret = pm_runtime_get_sync(ctlr->dev.parent);
> > +                       if (ret < 0) {
> > +                               dev_err(&ctlr->dev,
> > +                                       "Failed to power device: %d\n",
> > +                                       ret);
> > +                               return ret;
> > +                       }
> > +               }
> > +
> > +               mutex_lock(&ctlr->bus_lock_mutex);
> > +               mutex_lock(&ctlr->io_mutex);
> > +               ret = ctlr->mem_ops->exec_op(mem, op);
> > +               mutex_unlock(&ctlr->io_mutex);
> > +               mutex_unlock(&ctlr->bus_lock_mutex);
> > +
> > +               if (ctlr->auto_runtime_pm)
> > +                       pm_runtime_put(ctlr->dev.parent);
> > +
> > +               /*
> > +                * Some controllers only optimize specific paths (typically the
> > +                * read path) and expect the core to use the regular SPI
> > +                * interface in these cases.
> > +                */
> > +               if (!ret || ret != -ENOTSUPP)
> > +                       return ret;
> > +       }
> > +
> > +       tmpbufsize = sizeof(op->cmd.opcode) + op->addr.nbytes +
> > +                    op->dummy.nbytes;  
> 
> Here you are using sizeof(op->cmd.opcode) while the code after this
> assumes opcode is u8(ie. "memcpy(tmpbuf + 1, op->addr.buf, op->addr.nbytes);")
> It may be confused.

Will use sizeof(op->cmd.opcode) everywhere then.

-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com