On Wed, Nov 17, 2010 at 10:27:38AM +0800, Chuanxiao.Dong wrote:
> >From 8e339045eed04e60b9c3a4c4a8965c81503222bc Mon Sep 17 00:00:00 2001
> From: Chuanxiao Dong <[email protected]>
> Date: Sun, 14 Nov 2010 12:10:18 +0800
> Subject: [PATCH 2/3] mmc: add two callbacks in mmc_host_ops to implement
> Dekker
>
> Dekker algorithm is used to check whether the IA can get the
> host controller ownership before access it.
> This algorithm will be also used in SCU side.
>
> Signed-off-by: Yunpeng Gao <[email protected]>
> Signed-off-by: Chuanxiao Dong <[email protected]>
> ---
> drivers/mmc/host/sdhci.c | 154
> ++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/mmc/host.h | 4 +
> 2 files changed, 158 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index f6a2b8a..0fdd93f 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1328,11 +1328,165 @@ out:
> spin_unlock_irqrestore(&host->lock, flags);
> }
>
> +/*
> + * One of the Medfield eMMC controller (PCI device id 0x0823, SDIO3) is
> + * a shared resource used by the SCU and the IA processors. SCU primarily
> + * uses the eMMC host controller to access the eMMC device's Boot Partition,
> + * while the IA CPU uses the eMMC host controller to access the eMMC device's
> + * User Partition.
> + *
> + * After the SCU hands off the system to the IA processor, the IA processor
> + * assumes ownership to the eMMC host controller. Due to absence of any
> + * arbitration at the eMMC host controller, this could result in concurrent
> + * eMMC host accesses resulting in bus contention and garbage data ending up
> + * in either of the partitions.
> + * To circumvent this from happening, eMMC host controller locking mechanism
> + * is employed, where at any one given time, only one agent, SCU or IA, may
> be
> + * allowed to access the host. This is achieved by implementing Dekker's
> + * Algorithm (http://en.wikipedia.org/wiki/Dekker's_algorithm) between the
> + * two processors.
> + *
> + * Before handing off the system to the IA processor, SCU must set up three
> + * housekeeping mutex variables allocated in the shared SRAM as follows:
> + *
> + * eMMC_Owner = IA (SCU and IA processors - RW, 32bit)
> + * IA_Req = FALSE (IA -RW, SCU - RO, 32bit)
> + * SCU_Req = FALSE (IA - RO, SCU - R/W, 32bit)
> + *
> + * There is no hardware based access control to these variables and so code
> + * executing on SCU and IA processors must follow below access rules
> + * (Dekker's algorithm):
> + *
> + * -----------------------------------------
> + * SCU Processor Implementation
> + * -----------------------------------------
> + * SCU_Req = TRUE;
> + * while (IA_Req == TRUE) {
> + * if (eMMC_Owner != SCU){
> + * SCU_Req = FALSE;
> + * while (eMMC_Owner != SCU);
> + * SCU_Req = TRUE;
> + * }
> + * }
Your pseudo-code is wrong and doesn't work. Please fix it.
> + * // SCU now performs eMMC transactions here
> + * ...
> + * // When done, relinquish control to IA
> + * eMMC_Owner = IA;
> + * SCU_Req = FALSE;
> + *
> + * -----------------------------------------
> + * IA Processor Implementation
> + * -----------------------------------------
> + * IA_Req = TRUE;
> + * while (SCU_Req == TRUE) {
> + * if (eMMC_Owner != IA){
> + * IA_Req = FALSE;
> + * while (eMMC_Owner != IA);
> + * IA_Req = TRUE;
> + * }
> + * }
Again with the infinate loops in pseudo code, hopefully your
implemtation is better.
> + * //IA now performs eMMC transactions here
> + * ...
> + * //When done, relinquish control to SCU
> + * eMMC_Owner = SCU;
> + * IA_Req = FALSE;
> + *
> + * ----------------------------------------
> +*/
> +
> +/* Implement the Dekker's algorithm on the IA process side */
> +static int sdhci_acquire_ownership(struct mmc_host *mmc)
> +{
> + struct sdhci_host *host;
> + unsigned long t1, t2;
> +
> + host = mmc_priv(mmc);
> +
> + if (!((host->quirks & SDHCI_QUIRK_NEED_DEKKER_MUTEX) &&
> + (host->sram_addr)))
> + return 0;
> +
> + DBG("Acquire ownership - eMMC owner: %d, IA req: %d, SCU req: %d\n",
> + readl(host->sram_addr + DEKKER_EMMC_OWNER_OFFSET),
> + readl(host->sram_addr + DEKKER_IA_REQ_OFFSET),
> + readl(host->sram_addr + DEKKER_SCU_REQ_OFFSET));
> +
> + writel(1, host->sram_addr + DEKKER_IA_REQ_OFFSET);
> +
> + t1 = jiffies + 10 * HZ;
> + t2 = 500;
> +
> + while (readl(host->sram_addr + DEKKER_SCU_REQ_OFFSET)) {
> + if (readl(host->sram_addr + DEKKER_EMMC_OWNER_OFFSET) !=
> + DEKKER_OWNER_IA) {
> + writel(0, host->sram_addr + DEKKER_IA_REQ_OFFSET);
> + while (t2) {
> + if (readl(host->sram_addr +
> + DEKKER_EMMC_OWNER_OFFSET) ==
> + DEKKER_OWNER_IA)
> + break;
> + msleep(10);
> + t2--;
> + }
> + if (t2) {
> + writel(1, host->sram_addr +
> + DEKKER_IA_REQ_OFFSET);
> + } else {
> + pr_err("eMMC mutex timeout (owner)!\n");
> + goto timeout;
> + }
> + }
> + if (time_after(jiffies, t1)) {
> + pr_err("eMMC mutex timeout (req)!\n");
> + goto timeout;
> + }
> + cpu_relax();
Are you sure this is all you need to do here? No sleeping? That's a
very fast loop to be spinning in, even if you are trying to give the cpu
back.
> + }
> +
> + if (readl(host->sram_addr + DEKKER_EMMC_OWNER_OFFSET) ==
> + DEKKER_OWNER_IA) {
> + /* means IA got the relationship from SCU, so driver need
> + * to do something */
> + /* Re-init host controller register here since SCU FW possibly
> + * have changed some registers already. */
> + sdhci_init(host, 0);
> + /* Tell core layer to do card re-init */
> + return 1;
> + }
> +
> + return 0;
> +timeout:
> + writel(DEKKER_OWNER_SCU, host->sram_addr + DEKKER_EMMC_OWNER_OFFSET);
> + writel(0, host->sram_addr + DEKKER_IA_REQ_OFFSET);
> + return -EBUSY;
> +}
> +
> +static void sdhci_release_ownership(struct mmc_host *mmc)
> +{
> + struct sdhci_host *host;
> +
> + host = mmc_priv(mmc);
> +
> + if (!((host->quirks & SDHCI_QUIRK_NEED_DEKKER_MUTEX) &&
> + (host->sram_addr)))
> + return;
> +
> + writel(DEKKER_OWNER_SCU, host->sram_addr + DEKKER_EMMC_OWNER_OFFSET);
> + writel(0, host->sram_addr + DEKKER_IA_REQ_OFFSET);
> +
> + DBG("Exit ownership - eMMC owner: %d, IA req: %d, SCU req: %d\n",
> + readl(host->sram_addr + DEKKER_EMMC_OWNER_OFFSET),
> + readl(host->sram_addr + DEKKER_IA_REQ_OFFSET),
> + readl(host->sram_addr + DEKKER_SCU_REQ_OFFSET));
> +}
> +
> static const struct mmc_host_ops sdhci_ops = {
> .request = sdhci_request,
> .set_ios = sdhci_set_ios,
> .get_ro = sdhci_get_ro,
> .enable_sdio_irq = sdhci_enable_sdio_irq,
> + .acquire_ownership = sdhci_acquire_ownership,
> + .release_ownership = sdhci_release_ownership,
Two new callbacks just for some broken hardware? That seems quite a
lot. There's no other quirk mechanism for these types of devices?
> };
>
>
> /*****************************************************************************\
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 01e4886..498037b 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -117,6 +117,10 @@ struct mmc_host_ops {
>
> /* optional callback for HC quirks */
> void (*init_card)(struct mmc_host *host, struct mmc_card *card);
> +
> + /* optional callback for HC mutex (Dekker algorithm) */
> + int (*acquire_ownership)(struct mmc_host *host);
> + void (*release_ownership)(struct mmc_host *host);
But nothing is actually calling these callbacks. Something seems wrong
here.
thanks,
greg k-h
_______________________________________________
MeeGo-kernel mailing list
[email protected]
http://lists.meego.com/listinfo/meego-kernel