RE: [RFC PATCH] mmc: sdhci-of-arasan: Add auto tuning support for ZynqMP Platform
From: Manish Narani
Date: Wed Feb 21 2018 - 01:09:06 EST
Hi Adrian,
> -----Original Message-----
> From: Adrian Hunter [mailto:adrian.hunter@xxxxxxxxx]
> Sent: Friday, February 16, 2018 7:37 PM
> To: Manish Narani <MNARANI@xxxxxxxxxx>; michal.simek@xxxxxxxxxx;
> ulf.hansson@xxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-
> mmc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; mark.rutland@xxxxxxx; robh+dt@xxxxxxxxxx
> Cc: Anirudha Sarangi <anirudh@xxxxxxxxxx>; Srinivas Goud
> <sgoud@xxxxxxxxxx>; Manish Narani <MNARANI@xxxxxxxxxx>
> Subject: Re: [RFC PATCH] mmc: sdhci-of-arasan: Add auto tuning support for
> ZynqMP Platform
>
> On 30/01/18 20:14, Manish Narani wrote:
> > This patch adds support of SD auto tuning for ZynqMP platform. Auto
> > tuning sequence sends tuning block to card when operating in UHS-1
> > modes. This resets the DLL and sends CMD19/CMD21 as a part of the auto
> > tuning process. Once the auto tuning process gets completed, reset the
> > DLL to load the newly obtained SDHC tuned tap value.
>
> How is this different from:
> 1. reset the dll
> 2. call sdhci_execute_tuning
> 3. reset the dll
>
Thanks for your comments. I am looking into this. I will check and let you know on the same.
Thanks,
- Manish
> >
> > Signed-off-by: Manish Narani <mnarani@xxxxxxxxxx>
> > ---
> > .../devicetree/bindings/mmc/arasan,sdhci.txt | 1 +
> > drivers/mmc/host/sdhci-of-arasan.c | 219
> ++++++++++++++++++++-
> > 2 files changed, 219 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> > b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> > index 60481bf..7d29751 100644
> > --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> > +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> > @@ -14,6 +14,7 @@ Required Properties:
> > - "arasan,sdhci-4.9a": generic Arasan SDHCI 4.9a PHY
> > - "arasan,sdhci-5.1": generic Arasan SDHCI 5.1 PHY
> > - "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1": rk3399 eMMC
> > PHY
> > + - "xlnx,zynqmp-8.9a": Xilinx ZynqMP 8.9a PHY
> > For this device it is strongly suggested to include arasan,soc-ctl-syscon.
> > - reg: From mmc bindings: Register location and length.
> > - clocks: From clock bindings: Handles to clock inputs.
> > diff --git a/drivers/mmc/host/sdhci-of-arasan.c
> > b/drivers/mmc/host/sdhci-of-arasan.c
> > index 0720ea7..7673db4 100644
> > --- a/drivers/mmc/host/sdhci-of-arasan.c
> > +++ b/drivers/mmc/host/sdhci-of-arasan.c
> > @@ -24,15 +24,18 @@
> > #include <linux/module.h>
> > #include <linux/of_device.h>
> > #include <linux/phy/phy.h>
> > +#include <linux/mmc/mmc.h>
> > #include <linux/regmap.h>
> > #include "sdhci-pltfm.h"
> > #include <linux/of.h>
> > +#include <linux/firmware/xilinx/zynqmp/firmware.h>
> >
> > #define SDHCI_ARASAN_VENDOR_REGISTER 0x78
> >
> > #define VENDOR_ENHANCED_STROBE BIT(0)
> >
> > #define PHY_CLK_TOO_SLOW_HZ 400000
> > +#define MAX_TUNING_LOOP 40
> >
> > /*
> > * On some SoCs the syscon area has a feature where the upper 16-bits
> > of @@ -88,6 +91,7 @@ struct sdhci_arasan_data {
> > struct sdhci_host *host;
> > struct clk *clk_ahb;
> > struct phy *phy;
> > + u32 device_id;
> > bool is_phy_on;
> >
> > struct clk_hw sdcardclk_hw;
> > @@ -157,6 +161,213 @@ static int sdhci_arasan_syscon_write(struct
> sdhci_host *host,
> > return ret;
> > }
> >
> > +/**
> > + * arasan_zynqmp_dll_reset - Issue the DLL reset.
> > + * @deviceid: Unique Id of device
> > + */
> > +void zynqmp_dll_reset(u8 deviceid)
> > +{
> > + const struct zynqmp_eemi_ops *eemi_ops = get_eemi_ops();
> > +
> > + if (!eemi_ops || !eemi_ops->ioctl)
> > + return;
> > +
> > + /* Issue DLL Reset */
> > + if (deviceid == 0)
> > + eemi_ops->ioctl(NODE_SD_0, IOCTL_SD_DLL_RESET,
> > + PM_DLL_RESET_PULSE, 0, NULL);
> > + else
> > + eemi_ops->ioctl(NODE_SD_1, IOCTL_SD_DLL_RESET,
> > + PM_DLL_RESET_PULSE, 0, NULL); }
> > +
> > +static void arasan_zynqmp_dll_reset(struct sdhci_host *host, u8
> > +deviceid) {
> > + u16 clk;
> > + unsigned long timeout;
> > +
> > + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
> > + clk &= ~(SDHCI_CLOCK_CARD_EN | SDHCI_CLOCK_INT_EN);
> > + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> > +
> > + /* Issue DLL Reset */
> > + zynqmp_dll_reset(deviceid);
> > +
> > + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
> > + clk |= SDHCI_CLOCK_INT_EN;
> > + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> > +
> > + /* Wait max 20 ms */
> > + timeout = 20;
> > + while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
> > + & SDHCI_CLOCK_INT_STABLE)) {
> > + if (timeout == 0) {
> > + dev_err(mmc_dev(host->mmc),
> > + ": Internal clock never stabilised.\n");
> > + return;
> > + }
> > + timeout--;
> > + mdelay(1);
> > + }
> > +
> > + clk |= SDHCI_CLOCK_CARD_EN;
> > + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL); }
> > +
> > +static int arasan_zynqmp_execute_tuning(struct sdhci_host *host, u32
> > +opcode) {
> > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> > + struct sdhci_arasan_data *sdhci_arasan =
> sdhci_pltfm_priv(pltfm_host);
> > + struct mmc_host *mmc = host->mmc;
> > + u16 ctrl;
> > + int tuning_loop_counter = MAX_TUNING_LOOP;
> > + int err = 0;
> > + unsigned long flags;
> > + unsigned int tuning_count = 0;
> > +
> > + spin_lock_irqsave(&host->lock, flags);
> > +
> > + if (host->tuning_mode == SDHCI_TUNING_MODE_1)
> > + tuning_count = host->tuning_count;
> > +
> > + ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> > + ctrl |= SDHCI_CTRL_EXEC_TUNING;
> > + if (host->quirks2 & SDHCI_QUIRK2_TUNING_WORK_AROUND)
> > + ctrl |= SDHCI_CTRL_TUNED_CLK;
> > + sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
> > +
> > + mdelay(1);
> > +
> > + arasan_zynqmp_dll_reset(host, sdhci_arasan->device_id);
> > +
> > + /*
> > + * As per the Host Controller spec v3.00, tuning command
> > + * generates Buffer Read Ready interrupt, so enable that.
> > + *
> > + * Note: The spec clearly says that when tuning sequence
> > + * is being performed, the controller does not generate
> > + * interrupts other than Buffer Read Ready interrupt. But
> > + * to make sure we don't hit a controller bug, we _only_
> > + * enable Buffer Read Ready interrupt here.
> > + */
> > + sdhci_writel(host, SDHCI_INT_DATA_AVAIL, SDHCI_INT_ENABLE);
> > + sdhci_writel(host, SDHCI_INT_DATA_AVAIL, SDHCI_SIGNAL_ENABLE);
> > +
> > + /*
> > + * Issue CMD19 repeatedly till Execute Tuning is set to 0 or the number
> > + * of loops reaches 40 times or a timeout of 150ms occurs.
> > + */
> > + do {
> > + struct mmc_command cmd = {0};
> > + struct mmc_request mrq = {NULL};
> > +
> > + cmd.opcode = opcode;
> > + cmd.arg = 0;
> > + cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
> > + cmd.retries = 0;
> > + cmd.data = NULL;
> > + cmd.mrq = &mrq;
> > + cmd.error = 0;
> > +
> > + if (tuning_loop_counter-- == 0)
> > + break;
> > +
> > + mrq.cmd = &cmd;
> > +
> > + /*
> > + * In response to CMD19, the card sends 64 bytes of tuning
> > + * block to the Host Controller. So we set the block size
> > + * to 64 here.
> > + */
> > + if (cmd.opcode == MMC_SEND_TUNING_BLOCK_HS200) {
> > + if (mmc->ios.bus_width == MMC_BUS_WIDTH_8) {
> > + sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 128),
> > + SDHCI_BLOCK_SIZE);
> > + } else if (mmc->ios.bus_width == MMC_BUS_WIDTH_4) {
> > + sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 64),
> > + SDHCI_BLOCK_SIZE);
> > + }
> > + } else {
> > + sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 64),
> > + SDHCI_BLOCK_SIZE);
> > + }
> > +
> > + /*
> > + * The tuning block is sent by the card to the host controller.
> > + * So we set the TRNS_READ bit in the Transfer Mode register.
> > + * This also takes care of setting DMA Enable and Multi Block
> > + * Select in the same register to 0.
> > + */
> > + sdhci_writew(host, SDHCI_TRNS_READ,
> > + SDHCI_TRANSFER_MODE);
> > +
> > + sdhci_send_command(host, &cmd);
> > +
> > + host->cmd = NULL;
> > +
> > + spin_unlock_irqrestore(&host->lock, flags);
> > + /* Wait for Buffer Read Ready interrupt */
> > + wait_event_interruptible_timeout(host->buf_ready_int,
> > + (host->tuning_done == 1),
> > + msecs_to_jiffies(50));
> > + spin_lock_irqsave(&host->lock, flags);
> > +
> > + if (!host->tuning_done) {
> > + dev_warn(mmc_dev(host->mmc),
> > + ": Timeout for Buffer Read Ready interrupt, back to fixed
> sampling clock\n");
> > + ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> > + ctrl &= ~SDHCI_CTRL_TUNED_CLK;
> > + ctrl &= ~SDHCI_CTRL_EXEC_TUNING;
> > + sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
> > +
> > + err = -EIO;
> > + goto out;
> > + }
> > +
> > + host->tuning_done = 0;
> > +
> > + ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> > +
> > + /* eMMC spec does not require a delay between tuning cycles */
> > + if (opcode == MMC_SEND_TUNING_BLOCK)
> > + mdelay(1);
> > + } while (ctrl & SDHCI_CTRL_EXEC_TUNING);
> > +
> > + /*
> > + * The Host Driver has exhausted the maximum number of loops
> allowed,
> > + * so use fixed sampling frequency.
> > + */
> > + if (tuning_loop_counter < 0) {
> > + ctrl &= ~SDHCI_CTRL_TUNED_CLK;
> > + sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
> > + }
> > + if (!(ctrl & SDHCI_CTRL_TUNED_CLK)) {
> > + dev_warn(mmc_dev(host->mmc),
> > + ": Tuning failed, back to fixed sampling clock\n");
> > + err = -EIO;
> > + } else {
> > + arasan_zynqmp_dll_reset(host, sdhci_arasan->device_id);
> > + }
> > +
> > +out:
> > + /*
> > + * In case tuning fails, host controllers which support
> > + * re-tuning can try tuning again at a later time, when the
> > + * re-tuning timer expires. So for these controllers, we
> > + * return 0. Since there might be other controllers who do not
> > + * have this capability, we return error for them.
> > + */
> > + if (tuning_count)
> > + err = 0;
> > +
> > + host->mmc->retune_period = err ? 0 : tuning_count;
> > +
> > + sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
> > + sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
> > + spin_unlock_irqrestore(&host->lock, flags);
> > +
> > + return err;
> > +}
> > +
> > static void sdhci_arasan_set_clock(struct sdhci_host *host, unsigned
> > int clock) {
> > struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); @@
> > -262,7 +473,7 @@ static int sdhci_arasan_voltage_switch(struct mmc_host
> *mmc,
> > return -EINVAL;
> > }
> >
> > -static const struct sdhci_ops sdhci_arasan_ops = {
> > +static struct sdhci_ops sdhci_arasan_ops = {
> > .set_clock = sdhci_arasan_set_clock,
> > .get_max_clock = sdhci_pltfm_clk_get_max_clock,
> > .get_timeout_clock = sdhci_pltfm_clk_get_max_clock, @@ -371,6
> > +582,7 @@ static const struct of_device_id sdhci_arasan_of_match[] = {
> > { .compatible = "arasan,sdhci-8.9a" },
> > { .compatible = "arasan,sdhci-5.1" },
> > { .compatible = "arasan,sdhci-4.9a" },
> > + { .compatible = "xlnx,zynqmp-8.9a" },
> >
> > { /* sentinel */ }
> > };
> > @@ -642,6 +854,11 @@ static int sdhci_arasan_probe(struct
> platform_device *pdev)
> > goto unreg_clk;
> > }
> >
> > + if (of_device_is_compatible(pdev->dev.of_node, "xlnx,zynqmp-
> 8.9a")) {
> > + sdhci_arasan_ops.platform_execute_tuning =
> > + arasan_zynqmp_execute_tuning;
> > + }
> > +
> > sdhci_arasan->phy = ERR_PTR(-ENODEV);
> > if (of_device_is_compatible(pdev->dev.of_node,
> > "arasan,sdhci-5.1")) {
> > --
> > 2.7.4
> >
> > This email and any attachments are intended for the sole use of the named
> recipient(s) and contain(s) confidential information that may be proprietary,
> privileged or copyrighted under applicable law. If you are not the intended
> recipient, do not read, copy, or forward this email message or any
> attachments. Delete this email message and any attachments immediately.
> >