Re: [U-Boot] [PATCH] MMC: PL180: Fix infinite loop with VExpress extended fifo implementation
On Tue, Oct 4, 2011 at 12:32 PM, Jon Medhurst (Tixy)
wrote:
> Cc: Andy Fleming
> Signed-off-by: Jon Medhurst
> 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;
Please fix your patch-sending software. This patch has converted all
of the tabs to spaces, and won't apply.
Andy
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] MMC: PL180: Fix infinite loop with VExpress extended fifo implementation
On 10/04/2011 11:32 AM, Jon Medhurst (Tixy) wrote:
> From: Jon Medhurst
>
> 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
> Signed-off-by: Jon Medhurst
Acked-by: Matt Waddel
> ---
>
> 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/
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] MMC: PL180: Fix infinite loop with VExpress extended fifo implementation
On Wed, 2011-10-05 at 10:58 +0100, Pawel Moll wrote: > > That's useful to know. The PL180 code is also used for U8500, I don't > > know if that implements the peripheral ID register; though I guess any > > probing could be limited to vexpress anyway. > > STE have the same "problems" with FIFO size, see drivers/mmc/host/mmci.c > in kernel sources: Yes, and my proposed fix for U-Boot will work with them all because it removes a dependency on the fifo size. -- Tixy ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] MMC: PL180: Fix infinite loop with VExpress extended fifo implementation
> That's useful to know. The PL180 code is also used for U8500, I don't
> know if that implements the peripheral ID register; though I guess any
> probing could be limited to vexpress anyway.
STE have the same "problems" with FIFO size, see drivers/mmc/host/mmci.c
in kernel sources:
static struct variant_data variant_arm = {
.fifosize = 16 * 4,
.fifohalfsize = 8 * 4,
.datalength_bits= 16,
};
static struct variant_data variant_arm_extended_fifo = {
.fifosize = 128 * 4,
.fifohalfsize = 64 * 4,
.datalength_bits= 16,
};
static struct variant_data variant_u300 = {
.fifosize = 16 * 4,
.fifohalfsize = 8 * 4,
.clkreg_enable = MCI_ST_U300_HWFCEN,
.datalength_bits= 16,
.sdio = true,
};
static struct variant_data variant_ux500 = {
.fifosize = 30 * 4,
.fifohalfsize = 8 * 4,
.clkreg = MCI_CLK_ENABLE,
.clkreg_enable = MCI_ST_UX500_HWFCEN,
.datalength_bits= 24,
.sdio = true,
.st_clkdiv = true,
};
static struct variant_data variant_ux500v2 = {
.fifosize = 30 * 4,
.fifohalfsize = 8 * 4,
.clkreg = MCI_CLK_ENABLE,
.clkreg_enable = MCI_ST_UX500_HWFCEN,
.datalength_bits= 24,
.sdio = true,
.st_clkdiv = true,
.blksz_datactrl16 = true,
};
Cheers!
Paweł
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] MMC: PL180: Fix infinite loop with VExpress extended fifo implementation
On Wed, 2011-10-05 at 10:30 +0100, Pawel Moll wrote: > Hi Tixy, > > > One possible fix is to add some build time configuration to change > > SDI_FIFO_BURST_SIZE for the new implementation. > > You can also detect the configuration in runtime, basing on PeriphID: > > http://infocenter.arm.com/help/topic/com.arm.doc.ddi0172a/i1024149.html > > Configuration == 0 (ID == 0x00041180) -> FIFO length = 16 * 4 > Configuration == 1 (ID == 0x01041180) -> FIFO length = 128 * 4 That's useful to know. The PL180 code is also used for U8500, I don't know if that implements the peripheral ID register; though I guess any probing could be limited to vexpress anyway. However, I think that if smaller and simpler code will work on all hardware then that is preferable. -- Tixy ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] MMC: PL180: Fix infinite loop with VExpress extended fifo implementation
Hi Tixy, > One possible fix is to add some build time configuration to change > SDI_FIFO_BURST_SIZE for the new implementation. You can also detect the configuration in runtime, basing on PeriphID: http://infocenter.arm.com/help/topic/com.arm.doc.ddi0172a/i1024149.html Configuration == 0 (ID == 0x00041180) -> FIFO length = 16 * 4 Configuration == 1 (ID == 0x01041180) -> FIFO length = 128 * 4 Cheers! PAweł ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
