public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Matt Waddel <matt.waddel@linaro.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] MMC: PL180: Fix infinite loop with VExpress extended fifo implementation
Date: Thu, 06 Oct 2011 10:43:20 -0600	[thread overview]
Message-ID: <4E8DDAA8.8070608@linaro.org> (raw)
In-Reply-To: <1317749544.740.27.camel@linaro1>

On 10/04/2011 11:32 AM, Jon Medhurst (Tixy) wrote:
> From: Jon Medhurst <jon.medhurst@linaro.org>
> 
> The new IO FPGA implementation for Versatile Express contains an MMCI
> (PL180) cell with the FIFO extended to 128 words. This causes the
> read_bytes() function to go into an infinite loop; as it will wait for
> for the half-full signal (SDI_STA_RXFIFOBR) if there are more than 8
> words remaining (SDI_FIFO_BURST_SIZE), but it won't receive this signal
> once there are fewer than 64 words left to transfer.
> 
> One possible fix is to add some build time configuration to change
> SDI_FIFO_BURST_SIZE for the new implementation. However, the problematic
> code only seems to exist as a small performance optimisation, so the
> solution implemented by this patch is to simply remove it. The error
> checking following the loop is also removed as this will be handled by
> code further down the function.
> 
> Cc: Andy Fleming <afleming@gmail.com>
> Signed-off-by: Jon Medhurst <jon.medhurst@linaro.org>

Acked-by: Matt Waddel <matt.waddel@linaro.org>

> ---
> 
> If it is desirable to keep the burst read optimisation, then an
> alternative solution would be to keep the loop and add an if clause to
> do a single read if the fifo doesn't have enough for a burst.
> 
> ---
>  drivers/mmc/arm_pl180_mmci.c |   26 --------------------------
>  1 files changed, 0 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/mmc/arm_pl180_mmci.c b/drivers/mmc/arm_pl180_mmci.c
> index ed296ee..e6467a2 100644
> --- a/drivers/mmc/arm_pl180_mmci.c
> +++ b/drivers/mmc/arm_pl180_mmci.c
> @@ -111,7 +111,6 @@ static int do_command(struct mmc *dev, struct mmc_cmd *cmd)
>  static int read_bytes(struct mmc *dev, u32 *dest, u32 blkcount, u32 blksize)
>  {
>         u32 *tempbuff = dest;
> -       int i;
>         u64 xfercount = blkcount * blksize;
>         struct mmc_host *host = dev->priv;
>         u32 status, status_err;
> @@ -121,31 +120,6 @@ static int read_bytes(struct mmc *dev, u32 *dest, u32 blkcount, u32 blksize)
>         status = readl(&host->base->status);
>         status_err = status & (SDI_STA_DCRCFAIL | SDI_STA_DTIMEOUT |
>                                SDI_STA_RXOVERR);
> -       while (!status_err &&
> -              (xfercount >= SDI_FIFO_BURST_SIZE * sizeof(u32))) {
> -               if (status & SDI_STA_RXFIFOBR) {
> -                       for (i = 0; i < SDI_FIFO_BURST_SIZE; i++)
> -                               *(tempbuff + i) = readl(&host->base->fifo);
> -                       tempbuff += SDI_FIFO_BURST_SIZE;
> -                       xfercount -= SDI_FIFO_BURST_SIZE * sizeof(u32);
> -               }
> -               status = readl(&host->base->status);
> -               status_err = status &
> -                       (SDI_STA_DCRCFAIL | SDI_STA_DTIMEOUT | SDI_STA_RXOVERR);
> -       }
> -
> -       if (status & SDI_STA_DTIMEOUT) {
> -               printf("Read data timed out, xfercount: %llu, status: 0x%08X\n",
> -                       xfercount, status);
> -               return -ETIMEDOUT;
> -       } else if (status & SDI_STA_DCRCFAIL) {
> -               printf("Read data blk CRC error: 0x%x\n", status);
> -               return -EILSEQ;
> -       } else if (status & SDI_STA_RXOVERR) {
> -               printf("Read data RX overflow error\n");
> -               return -EIO;
> -       }
> -
>         while ((!status_err) && (xfercount >= sizeof(u32))) {
>                 if (status & SDI_STA_RXDAVL) {
>                         *(tempbuff) = readl(&host->base->fifo);


-- 
Linaro.org ? Open source software for ARM SoCs
Follow Linaro:
http://www.facebook.com/pages/Linaro/155974581091106
http://twitter.com/#!/linaroorg
http://www.linaro.org/linaro-blog/

  parent reply	other threads:[~2011-10-06 16:43 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-04 17:32 [U-Boot] [PATCH] MMC: PL180: Fix infinite loop with VExpress extended fifo implementation Jon Medhurst
2011-10-05  9:30 ` Pawel Moll
2011-10-05  9:48   ` Jon Medhurst
2011-10-05  9:58     ` Pawel Moll
2011-10-05 10:14       ` Jon Medhurst
2011-10-06 16:43 ` Matt Waddel [this message]
2011-11-03  1:39 ` Andy Fleming
2011-11-04 13:06   ` [U-Boot] [PATCH v2] " Jon Medhurst
2011-11-08 20:46     ` Andy Fleming

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4E8DDAA8.8070608@linaro.org \
    --to=matt.waddel@linaro.org \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox