[PATCH linux dev-5.8 v2 09/11] spi: npcm-pspi: Add full duplex support

[Avi Fishman]

On Mon, Jan 11, 2021 at 3:05 AM Joel Stanley <joel at jms.id.au> wrote:
>
> On Tue, 5 Jan 2021 at 13:45, Tomer Maimon <tmaimon77 at gmail.com> wrote:
> >
> > Modify the IRQ handler in the NPCM PSPI
> > driver to support SPI full duplex communication.
> >
> > Signed-off-by: Tomer Maimon <tmaimon77 at gmail.com>
> > ---
> >  drivers/spi/spi-npcm-pspi.c | 75 +++++++++++++++----------------------
> >  1 file changed, 30 insertions(+), 45 deletions(-)
> >
> > diff --git a/drivers/spi/spi-npcm-pspi.c b/drivers/spi/spi-npcm-pspi.c
> > index 87cd0233c60b..92fae0b23eb1 100644
> > --- a/drivers/spi/spi-npcm-pspi.c
> > +++ b/drivers/spi/spi-npcm-pspi.c
> > @@ -197,22 +197,22 @@ static void npcm_pspi_setup_transfer(struct spi_device *spi,
> >  static void npcm_pspi_send(struct npcm_pspi *priv)
> >  {
> >         int wsize;
> > -       u16 val;
> > +       u16 val = 0;
> >
> >         wsize = min(bytes_per_word(priv->bits_per_word), priv->tx_bytes);
> >         priv->tx_bytes -= wsize;
> >
> > -       if (!priv->tx_buf)
> > -               return;
>
> It looks like you're removing this check and instead doing it inside
> each case. That seems like a waste, why not leave the single check in?

Even if the tx_buf is NULL, I still need to do the write to
NPCM_PSPI_DATA, since only this write triggers the clock and I need
the clocks for the rx_buf.
and this is why I initialized 'val = 0'

>
> > -
> >         switch (wsize) {
> >         case 1:
> > -               val = *priv->tx_buf++;
> > +               if (priv->tx_buf)
> > +                       val = *priv->tx_buf++;
> >                 iowrite8(val, NPCM_PSPI_DATA + priv->base);
> >                 break;
> >         case 2:
> > -               val = *priv->tx_buf++;
> > -               val = *priv->tx_buf++ | (val << 8);
> > +               if (priv->tx_buf) {
> > +                       val = *priv->tx_buf++;
> > +                       val = *priv->tx_buf++ | (val << 8);
> > +               }
> >                 iowrite16(val, NPCM_PSPI_DATA + priv->base);
> >                 break;
> >         default:
> > @@ -224,22 +224,24 @@ static void npcm_pspi_send(struct npcm_pspi *priv)
> >  static void npcm_pspi_recv(struct npcm_pspi *priv)
> >  {
> >         int rsize;
> > -       u16 val;
> > +       u16 val_16;
> > +       u8  val_8;
> >
> >         rsize = min(bytes_per_word(priv->bits_per_word), priv->rx_bytes);
> >         priv->rx_bytes -= rsize;
> >
> > -       if (!priv->rx_buf)
> > -               return;
> > -
> >         switch (rsize) {
> >         case 1:
> > -               *priv->rx_buf++ = ioread8(priv->base + NPCM_PSPI_DATA);
> > +               val_8 = ioread8(priv->base + NPCM_PSPI_DATA);
> > +               if (priv->rx_buf)
> > +                       *priv->rx_buf++ = val_8;
> >                 break;
> >         case 2:
> > -               val = ioread16(priv->base + NPCM_PSPI_DATA);
> > -               *priv->rx_buf++ = (val >> 8);
> > -               *priv->rx_buf++ = val & 0xff;
> > +               val_16 = ioread16(priv->base + NPCM_PSPI_DATA);
> > +               if (priv->rx_buf) {
> > +                       *priv->rx_buf++ = (val_16 >> 8);
> > +                       *priv->rx_buf++ = val_16 & 0xff;
> > +               }
> >                 break;
> >         default:
> >                 WARN_ON_ONCE(1);
> > @@ -298,43 +300,26 @@ static irqreturn_t npcm_pspi_handler(int irq, void *dev_id)
> >         struct npcm_pspi *priv = dev_id;
> >         u8 stat;
> >
> > -       stat = ioread8(priv->base + NPCM_PSPI_STAT);
> > -
> >         if (!priv->tx_buf && !priv->rx_buf)
> >                 return IRQ_NONE;
> >
> > -       if (priv->tx_buf) {
> > -               if (stat & NPCM_PSPI_STAT_RBF) {
> > -                       ioread8(NPCM_PSPI_DATA + priv->base);
> > -                       if (priv->tx_bytes == 0) {
> > -                               npcm_pspi_disable(priv);
> > -                               complete(&priv->xfer_done);
> > -                               return IRQ_HANDLED;
> > -                       }
> > -               }
> > -
> > -               if ((stat & NPCM_PSPI_STAT_BSY) == 0)
> > -                       if (priv->tx_bytes)
> > -                               npcm_pspi_send(priv);
> > +       if (priv->tx_bytes == 0 && priv->rx_bytes == 0) {
> > +               npcm_pspi_disable(priv);
> > +               complete(&priv->xfer_done);
> > +               return IRQ_HANDLED;
> >         }
> >
> > -       if (priv->rx_buf) {
> > -               if (stat & NPCM_PSPI_STAT_RBF) {
> > -                       if (!priv->rx_bytes)
> > -                               return IRQ_NONE;
> > -
> > -                       npcm_pspi_recv(priv);
> > +       stat = ioread8(priv->base + NPCM_PSPI_STAT);
> >
> > -                       if (!priv->rx_bytes) {
> > -                               npcm_pspi_disable(priv);
> > -                               complete(&priv->xfer_done);
> > -                               return IRQ_HANDLED;
> > -                       }
> > -               }
> > +       /*
> > +        * first we do the read since if we do the write we previous read might
> > +        * be lost (indeed low chances)
> > +        */
> > +       if ((stat & NPCM_PSPI_STAT_RBF) && priv->rx_bytes)
> > +               npcm_pspi_recv(priv);
> >
> > -               if (((stat & NPCM_PSPI_STAT_BSY) == 0) && !priv->tx_buf)
> > -                       iowrite8(0x0, NPCM_PSPI_DATA + priv->base);
> > -       }
> > +       if (((stat & NPCM_PSPI_STAT_BSY) == 0) && priv->tx_bytes)
> > +               npcm_pspi_send(priv);
> >
> >         return IRQ_HANDLED;
> >  }
> > --
> > 2.22.0
> >



-- 
Regards,
Avi