On Mon, Jun 28, 2021 at 2:47 PM Pratyush Yadav <p.ya...@ti.com> wrote: > > On 28/06/21 12:09PM, Jagan Teki wrote: > > On Sun, Jun 27, 2021 at 2:05 PM Pratyush Yadav <p.ya...@ti.com> wrote: > > > > > > On 26/06/21 02:44PM, Jagan Teki wrote: > > > > On Sat, Jun 26, 2021 at 12:47 AM Pratyush Yadav <p.ya...@ti.com> 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. 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.ya...@ti.com> > > > > > --- > > > > > drivers/spi/spi-mem.c | 32 +++++++++++++++++++++++++++++--- > > > > > include/spi-mem.h | 2 ++ > > > > > 2 files changed, 31 insertions(+), 3 deletions(-) > > > > > > > > > > diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c > > > > > index 541cd0e5a7..9c1ede1b61 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,39 @@ 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.buswidth == 8 && op->cmd.nbytes % 2) > > > > > + return false; > > > > > + > > > > > + if (op->addr.nbytes && op->addr.buswidth == 8 && > > > > > op->addr.nbytes % 2) > > > > > + return false; > > > > > + > > > > > + if (op->dummy.nbytes && op->dummy.buswidth == 8 && > > > > > op->dummy.nbytes % 2) > > > > > + return false; > > > > > + > > > > > + if (op->data.dir != SPI_MEM_NO_DATA && > > > > > + op->dummy.buswidth == 8 && op->data.nbytes % 2) > > > > > + return false; > > > > > + > > > > > + return spi_mem_check_buswidth(slave, op); > > > > > +} > > > > > +EXPORT_SYMBOL_GPL(spi_mem_dtr_supports_op); > > > > > > > > Does this export_symbol is used in U-Boot? > > > > > > It is being used in drivers ported from Linux. I see it being used in > > > spi-mem.c, mtdcore.c, x530_cert_parser.c, nand/core.c, etc. It is > > > defined to an empty macro in include/linux/compat.h. > > > > Ok, thanks for the details. Somehow we need to refactor the code, I > > believe it would require lot of testing but possible. > > Sorry, I don't quite understand what code you want to refactor. Do you > mean the code in this series? Can you be more specific on what you think > needs refactoring? Or do you mean a refactor of spi-mem?
Not in the series, In general in spi-mem, spi-nor. The series is running under CI now. I will send the PR tonight. Jagan.