Re: [U-Boot] [PATCH v2 10/19] spi: mpc8xxx: Simplify logic a bit

[Joakim Tjernlund]

On Mon, 2019-04-29 at 01:58 +0530, Jagan Teki wrote:
> 
> From: Mario Six <mario....@gdsys.cc>
> 
> We do nothing in the loop if the "not empty" event was not detected. To
> simplify the logic, check if this is the case, and skip the execution of
> the loop early to reduce the nesting level and flag checking.

Looked at the driver to refresh memory and noticed:
if (charSize == 32) {
        /* Advance output buffer by 32 bits */
        din += 4;
}
which suggests that only 32 bit char will increase the din ptr so does other 
bitlens
work for reading?

 Jocke

> 
> Signed-off-by: Mario Six <mario....@gdsys.cc>
> ---
>  drivers/spi/mpc8xxx_spi.c | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/spi/mpc8xxx_spi.c b/drivers/spi/mpc8xxx_spi.c
> index 962ef710f8..a2e698ea17 100644
> --- a/drivers/spi/mpc8xxx_spi.c
> +++ b/drivers/spi/mpc8xxx_spi.c
> @@ -149,25 +149,28 @@ int spi_xfer(struct spi_slave *slave, uint bitlen, 
> const void *dout, void *din,
>                         bool have_ne = event & SPI_EV_NE;
>                         bool have_nf = event & SPI_EV_NF;
> 
> -                       if (have_ne) {
> -                               tmpdin = in_be32(&spi->rx);
> -                               setbits_be32(&spi->event, SPI_EV_NE);
> -
> -                               *(u32 *)din = (tmpdin << (32 - char_size));
> -                               if (char_size == 32) {
> -                                       /* Advance output buffer by 32 bits */
> -                                       din += 4;
> -                               }
> +                       if (!have_ne)
> +                               continue;
> +
> +                       tmpdin = in_be32(&spi->rx);
> +                       setbits_be32(&spi->event, SPI_EV_NE);
> +
> +                       *(u32 *)din = (tmpdin << (32 - char_size));
> +                       if (char_size == 32) {
> +                               /* Advance output buffer by 32 bits */
> +                               din += 4;
>                         }
> +
>                         /*
>                          * Only bail when we've had both NE and NF events.
>                          * This will cause timeouts on RO devices, so maybe
>                          * in the future put an arbitrary delay after writing
>                          * the device.  Arbitrary delays suck, though...
>                          */
> -                       if (have_ne && have_nf)
> +                       if (have_nf)
>                                 break;
>                 }
> +
>                 if (tm >= SPI_TIMEOUT)
>                         debug("*** %s: Time out during SPI transfer\n",
>                               __func__);
> --
> 2.18.0.321.gffc6fa0e3
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://nam03.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.denx.de%2Flistinfo%2Fu-boot&amp;data=02%7C01%7Cjoakim.tjernlund%40infinera.com%7Cb423ca475f53471860b308d6cc195be8%7C285643de5f5b4b03a1530ae2dc8aaf77%7C1%7C0%7C636920806635383891&amp;sdata=PxiqErmkjcpBVL4yBUi2UYiJ5oqtBTI4fCnb4XBTpmE%3D&amp;reserved=0

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot