>
> From bc325f4abbcbd7e4032ce148d7a6850eb2a754c8 Mon Sep 17 00:00:00 2001
> From: Yunpeng Gao <[email protected]>
> Date: Fri, 17 Dec 2010 19:00:10 +0800
> Subject: [PATCH] Implement the Medfield eMMC mutex (Dekker Algorithm)
> acquire/release APIs
The subject line is too long. You should restraint your subject lines to 80
chars, and avoid special chars line ( ) /
When your commit is converted to a patch, the name will be truncated to 80
chars, and special chars removed.
Good subject would be
Implement the Medfield eMMC mutex APIs
>
> Changes compared with the old implementation:
> 1. Remove the card re-init operation if last owner is SCU.
> 2. Add a usage counter to manage the acquire/release pair on IA side.
Dont put change-from-last-submission comment in patch comment.
You should rather put.
1) Problem you want to resolve.
2) How you resolve it.
One should understand rationals of your patch without having to look in mailing
lists for original discussion.
>
> +static int intel_mfld_emmc0_probe(struct sdhci_pci_chip *chip)
> +{
> + if (chip->pdev->revision == 0) /* Penwell A0 */
> + chip->quirks |= SDHCI_QUIRK_MFD_EMMC_SDIO_RESTRICTION;
A0 references should be removed when you will push that upstream.
> + else if (chip->pdev->revision == 4) /* Penwell B0 */
> + /*
> + * The eMMC mutex (Dekker algorithm) support in SCU firmware
> + * is only available for Penwell B0.
> + */
> + chip->quirks |= SDHCI_QUIRK_NEED_DEKKER_MUTEX;
> +
> + return 0;
> +}
> +
> +/*
> + * Get the base address in shared SRAM for eMMC mutex
> + * (Dekker's algorithm) through IPC call.
> + *
> + * Please note it'll always return 0 whether the address requesting
> + * success or not. So, the mmc driver will still work well if the scu
> + * firmware is not ready yet.
Then if this always work, you dont need to detect A0 in previous function.
A0 does not support the IPC so it will just work as B0 with old FW.
> +/* Implement the Dekker's algorithm on the IA processor side.
Should use kernel doc style comments
Should say that this function sleeps.
> + *
> + * Return value:
> + * 0 - Acquried the ownership successfully. The last owner is IA
> + * 1 - Acquried the ownership succesffully. The last owenr is SCU
> + * -EBUSY - failed to acquire ownership within the timeout period.
> + */
> +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;
> +
> + atomic_inc(&host->usage_cnt);
What if two IA threads are entering in this loop simulteanously?
> +
> + 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)) {
Access macro might make this easier to read like
READ_DEKKER_SCU_REQ(host)
> + 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();
Why cpu_relax() here, and msleep(10) for t2? Needs short inline comments.
> + }
> +
> + if (readl(host->sram_addr + DEKKER_EMMC_OWNER_OFFSET) ==
> + DEKKER_OWNER_IA)
> + return 1; /* Tell caller to re-config the host controller */
> +
> + 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;
> +
> + if (atomic_dec_and_test(&host->usage_cnt)) {
> + 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));
> + }
> +}
> +
> +/*
> + * Re-configure host controller registers here since SCU firmware
> + * has possibly changed some registers already
> + */
> +static void sdhci_reconfig_host_controller(struct mmc_host *mmc)
> +{
> + struct sdhci_host *host;
> +
> + host = mmc_priv(mmc);
> +
> + if (!((host->quirks & SDHCI_QUIRK_NEED_DEKKER_MUTEX) &&
> + (host->sram_addr)))
> + return;
> +
> + sdhci_init(host, 0);
> + sdhci_set_ios(host->mmc, &host->mmc->ios);
> +}
> +
> 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,
> + .reconfig_host_controller = sdhci_reconfig_host_controller,
> };
>
>
> /*****************************************************************************\
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 01e4886..abb2cab 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -117,6 +117,11 @@ 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);
> + void (*reconfig_host_controller)(struct mmc_host *host);
> };
>
> struct mmc_card;
> diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
> index dc712bb..5ae4eea 100644
> --- a/include/linux/mmc/sdhci.h
> +++ b/include/linux/mmc/sdhci.h
> @@ -91,11 +91,23 @@ struct sdhci_host {
> /* Controller of Medfield specific restriction */
> #define SDHCI_QUIRK_MFD_SD_RESTRICTION (1ULL<<33)
> #define SDHCI_QUIRK_MFD_EMMC_SDIO_RESTRICTION (1ULL<<34)
> -
> +/* One controller port will be accessed by driver and fw at the same time */
> +#define SDHCI_QUIRK_NEED_DEKKER_MUTEX (1ULL<<35)
>
> int irq; /* Device IRQ */
> void __iomem *ioaddr; /* Mapped address */
>
> + /* XXX: SCU/X86 mutex variables base address in shared SRAM */
> + void __iomem *sram_addr; /* Shared SRAM address */
> +
> +#define DEKKER_EMMC_OWNER_OFFSET 0
> +#define DEKKER_IA_REQ_OFFSET 0x04
> +#define DEKKER_SCU_REQ_OFFSET 0x08
> +#define DEKKER_OWNER_IA 0
> +#define DEKKER_OWNER_SCU 1
> +
> + atomic_t usage_cnt; /* eMMC mutex usage count */
> +
> const struct sdhci_ops *ops; /* Low level hw interface */
>
> struct regulator *vmmc; /* Power regulator */
> --
> 1.5.4.5
>
> _______________________________________________
> MeeGo-kernel mailing list
> [email protected]
> http://lists.meego.com/listinfo/meego-kernel
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
_______________________________________________
MeeGo-kernel mailing list
[email protected]
http://lists.meego.com/listinfo/meego-kernel
