[PATCH v8 04/28] spi: spi-mem: add spi_mem_dtr_supports_op()

[Pratyush Yadav]

On 02/04/21 06:31PM, Sean Anderson wrote:
> On 4/1/21 3:31 PM, Pratyush Yadav wrote:
> > spi_mem_default_supports_op() rejects DTR ops by default to ensure that
> > the controller drivers that haven't been updated with DTR support
> > continue to reject them. It also makes sure that controllers that don't
> > support DTR mode at all (which is most of them at the moment) also
> > reject them.
> > 
> > This means that controller drivers that want to support DTR mode can't
> > use spi_mem_default_supports_op(). Driver authors have to roll their own
> > supports_op() function and mimic the buswidth checks. See
> > spi-cadence-quadspi.c for example. Or even worse, driver authors might
> > skip it completely or get it wrong.
> > 
> > Add spi_mem_dtr_supports_op(). It provides a basic sanity check for DTR
> > ops and performs the buswidth requirement check. Move the logic for
> > checking buswidth in spi_mem_default_supports_op() to a separate
> > function so the logic is not repeated twice.
> > 
> > Signed-off-by: Pratyush Yadav <p.yadav at ti.com>
> > ---
> >   drivers/spi/spi-mem.c | 22 +++++++++++++++++++---
> >   include/spi-mem.h     |  2 ++
> >   2 files changed, 21 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
> > index 541cd0e5a7..be1737a2c6 100644
> > --- a/drivers/spi/spi-mem.c
> > +++ b/drivers/spi/spi-mem.c
> > @@ -145,8 +145,8 @@ static int spi_check_buswidth_req(struct spi_slave *slave, u8 buswidth, bool tx)
> >   	return -ENOTSUPP;
> >   }
> > -bool spi_mem_default_supports_op(struct spi_slave *slave,
> > -				 const struct spi_mem_op *op)
> > +static bool spi_mem_check_buswidth(struct spi_slave *slave,
> > +				   const struct spi_mem_op *op)
> >   {
> >   	if (spi_check_buswidth_req(slave, op->cmd.buswidth, true))
> >   		return false;
> > @@ -164,13 +164,29 @@ bool spi_mem_default_supports_op(struct spi_slave *slave,
> >   				   op->data.dir == SPI_MEM_DATA_OUT))
> >   		return false;
> > +	return true;
> > +}
> > +
> > +bool spi_mem_dtr_supports_op(struct spi_slave *slave,
> > +			     const struct spi_mem_op *op)
> > +{
> > +	if (op->cmd.nbytes != 2)
> > +		return false;
> 
> Why does the command bytes need to be 2?

They need to be 2 if the command buswidth is 8 because otherwise the 
command phase will only take up half a cycle. This should have been if 
(op->cmd.buswidth == 8 && op->cmd.nbytes != 2).

> 
> --Sean
> 
> > +
> > +	return spi_mem_check_buswidth(slave, op);
> > +}
> > +EXPORT_SYMBOL_GPL(spi_mem_dtr_supports_op);
> > +
> > +bool spi_mem_default_supports_op(struct spi_slave *slave,
> > +				 const struct spi_mem_op *op)
> > +{
> >   	if (op->cmd.dtr || op->addr.dtr || op->dummy.dtr || op->data.dtr)
> >   		return false;
> >   	if (op->cmd.nbytes != 1)
> >   		return false;>   -	return true;
> > +	return spi_mem_check_buswidth(slave, op);
> >   }
> >   EXPORT_SYMBOL_GPL(spi_mem_default_supports_op);
> > diff --git a/include/spi-mem.h b/include/spi-mem.h
> > index dc53b517c1..37a9128c5b 100644
> > --- a/include/spi-mem.h
> > +++ b/include/spi-mem.h
> > @@ -249,6 +249,8 @@ spi_controller_dma_unmap_mem_op_data(struct spi_controller *ctlr,
> >   int spi_mem_adjust_op_size(struct spi_slave *slave, struct spi_mem_op *op);
> >   bool spi_mem_supports_op(struct spi_slave *slave, const struct spi_mem_op *op);
> > +bool spi_mem_dtr_supports_op(struct spi_slave *slave,
> > +			     const struct spi_mem_op *op);
> >   bool spi_mem_default_supports_op(struct spi_slave *slave,
> >   				 const struct spi_mem_op *op);
> > 
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.