RE: [PATCH V2] Powerpc eSDHC Recover from the ADMA error

[Zhang Haijun-B42677]

Thank you very much.
Pls review.


-----Original Message-----
From: Anton Vorontsov [mailto:cbouatmai...@gmail.com] 
Sent: Wednesday, September 19, 2012 2:09 AM
To: Zhang Haijun-B42677
Cc: linux-mmc@vger.kernel.org; Zhang Haijun-B42677; Huang Changming-R66093
Subject: Re: [PATCH V2] Powerpc eSDHC Recover from the ADMA error

On Wed, Sep 19, 2012 at 01:05:13PM +0800, b42...@freescale.com wrote:
> From: Haijun Zhang <haijun.zh...@freescale.com>
> 
> A-003500: False ADMA Error might be reported when ADMA is used for 
> multiple block read command with Stop at Block Gap. If PROCTL[SABGREQ] 
> is set when the particular block's data is received by the System side 
> logic before entire block(with CRC) data is received by the SD side 
> logic, and also if ADMA descriptor line is fetched at the same time, 
> then DMA engine might report false ADMA error. eSDHC might not be able 
> to Continue(PROCTL[CREQ]=1)after Stop at Block Gap.
> This issue will impact the eSDHC IP VVN2.3.
> 
> 
> Signed-off-by: Haijun Zhang <haijun.zh...@freescale.com>
> Signed-off-by: Jerry Huang <chang-ming.hu...@freescale.com>
> CC: Anton Vorontsov <cbouatmai...@gmail.com>
> ---
> changes for v2:
>       - Invert the condition of the if statement in function workground

Hm.

[...]
> +static void esdhci_of_adma_workaround(struct sdhci_host *host, u32 
> +intmask) {
> +     u32 tmp = in_be32(host->ioaddr + SDHCI_SLOT_INT_STATUS);
> +
> +     tmp = (tmp & SDHCI_VENDOR_VER_MASK) >> SDHCI_VENDOR_VER_SHIFT;
> +     if ((intmask & SDHCI_INT_DATA_END) &&
> +             (intmask & SDHCI_INT_BLK_GAP) &&
> +             (tmp == VENDOR_V_23)) {

I really don't see it inverted. :-)

What I meant was this:

static void esdhci_of_adma_workaround(struct sdhci_host *host, u32 intmask) {
        u32 vendor;
        bool applicable;
        dma_addr_t dmastart;
        dma_addr_t dmanow;

        vendor = in_be32(host->ioaddr + SDHCI_SLOT_INT_STATUS);
        vendor = (vendor & SDHCI_VENDOR_VER_MASK) >> SDHCI_VENDOR_VER_SHIFT;

        applicable = intmask & SDHCI_INT_DATA_END &&
                     intmask & SDHCI_INT_BLK_GAP &&
                     vendor == VENDOR_V_23;
        if (!applicable)
                return;

        host->data->error = 0;
        dmastart = sg_dma_address(host->data->sg);
        dmanow = dmastart + host->data->bytes_xfered;
        ...
        ...
}

This is human-readable and there's no additional indentation.

Thanks,

Anton.

N�����r��y����b�X��ǧv�^�)޺{.n�+����{��g"��^n�r���z���h�����&���G���h�(�階�ݢj"���m������z�ޖ���f���h���~�m�