[U-Boot] [PATCH 2/3] mmc: sdhci: add the DMA select for SDMA

[Rommel Custodio]

Dear Jaehoon Chung, All,

Jaehoon Chung <jh80.chung <at> samsung.com> writes:

> 
> Hi Rommel,
> 
> Thanks for report.
> 
> On 11/05/2012 10:39 PM, Rommel G Custodio wrote:
> > Dear All, Jaehoon Chung
> > 
> > On 2012.09/21, Jaehoon Chung wrote:
> >> In host-control register, DMA select bit field is present.
> >> BUt in sdhci.c, didn't select for DMA.
> > 
> > This is an FYI.
> > Maybe others will encounter the same problem I had.
> > 
> > This code has recently been mainlined and it has caused some problems in
> > the IP that I am working with. The IP hangs when this code is enabled.
> How did you hang your IP when this code is enabled?

I read your e-mail last night. Sorry for the delayed reply.
Posting from gmane instead of my usual mailer, sorry for any readability issues.

> Before enabled, also set to SDMA_MODE at HOST_CONTROL register.
> Just More exactly  to set SDMA mode is used this code.
> Is your IP using other DMA mode?
The IP supports SDMA and ADMA but u-boot only supports SDMA.
The same driver/wrapper in Linux and u-boot. I've verified both DMA options
work correctly in Linux.


> Just i want to know your hang case.

** Sorry I deleted the patch in my previous reply.
But this is the gist of the patch, with inline comments.

+#ifdef CONFIG_MMC_SDMA
+	unsigned char ctrl;
(1) Is this correct data type?

+	ctrl = sdhci_readl(host, SDHCI_HOST_CONTROL);
(2) Is this the correct operation you want to use?

+	ctrl &= ~SDHCI_CTRL_DMA_MASK;
+	ctrl |= SDHCI_CTRL_SDMA;
+	sdhci_writel(host, ctrl, SDHCI_HOST_CONTROL);
(3) Is this the correct operation you want to use?
After this offset 028h becomes 0x00000000.
Power and voltage has been reset by this write it seems.
The IP stops responding because of this.

+#endif

There is two options, fix the data type or fix the APIs used.


*** Other comments:
Is this really needed _inside_ sdhci_transfer_data()? Can't it be implemented
inside sdhci_init() instead? Setting the DMA on the host controller should be
a one-shot deal, right?

All the best,
Rommel

> 
> Best Regards,
> Jaehoon Chung
> > 
> >> if set CONFIG_MMC_SDMA, we need to set SDMA-select bit.
> > 
> > The bit in the SDHCI specs show that 00b is the code for choosing SDMA.
> > So this code is not really necessary as long as sdhci_init() or 
sdhci_set_ios(),
> > the API that modifies SDHCI_HOST_CONTROL, does not touch the DMA bits. Right 
now,
> > by looking at the code, sdhci_init() nor sdhci_set_ios() does not modify the 
DMA
> > bits.
> > 
> >>
> >> Signed-off-by: Jaehoon Chung <jh80.chung at samsung.com>
> >> Signed-off-by: Kyungmin Park <kyungmin.park at samsung.com>
> > 
> > All the best,
> > Rommel
> > 
>