diff mbox series

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

Message ID 20210105134508.225702-10-tmaimon77@gmail.com
State New
Headers show
Series Add NPCM7xx patches to dev-5.8 | expand

Commit Message

Tomer Maimon Jan. 5, 2021, 1:45 p.m. UTC 
Modify the IRQ handler in the NPCM PSPI
driver to support SPI full duplex communication.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 drivers/spi/spi-npcm-pspi.c | 75 +++++++++++++++----------------------
 1 file changed, 30 insertions(+), 45 deletions(-)

Comments

Joel Stanley Jan. 11, 2021, 1:04 a.m. UTC | #1 
On Tue, 5 Jan 2021 at 13:45, Tomer Maimon <tmaimon77@gmail.com> wrote:
>
> Modify the IRQ handler in the NPCM PSPI
> driver to support SPI full duplex communication.
>
> Signed-off-by: Tomer Maimon <tmaimon77@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
>
Avi Fishman Jan. 11, 2021, 10:58 a.m. UTC | #2 
On Mon, Jan 11, 2021 at 3:05 AM Joel Stanley <joel@jms.id.au> wrote:
>
> On Tue, 5 Jan 2021 at 13:45, Tomer Maimon <tmaimon77@gmail.com> wrote:
> >
> > Modify the IRQ handler in the NPCM PSPI
> > driver to support SPI full duplex communication.
> >
> > Signed-off-by: Tomer Maimon <tmaimon77@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
> >
diff mbox series

Patch

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