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

[Joel Stanley]

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?

> -
>         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
>