* Re: [PATCH v3] mmc: sunxi: Handle the 'New Timing' mode
From: Maxime Ripard @ 2016-10-11 14:55 UTC (permalink / raw)
To: Jean-Francois Moine
Cc: Ulf Hansson, Chen-Yu Tsai, Michael Turquette,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-mmc-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20160830193202.4baaefd508a5d20d48b2cbed-GANU6spQydw@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2252 bytes --]
On Tue, Aug 30, 2016 at 07:32:02PM +0200, Jean-Francois Moine wrote:
> On Tue, 30 Aug 2016 18:26:13 +0200
> Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>
> > > There are 2 flags saying that the new timing is used:
> > > - the bit 'mode select' in the clock register, and
> > > - the bit 'new timing' in the MMC register.
> > > Both bits must be set/reset at the same time, otherwise the device
> > > does not work (tested with wifi and eMMC in H3 and A83T boards).
> > > So, some synchronization must exist.
> > >
> > > The previous versions was using a DT property for the MMC and a flag
> > > in the clock driver. This did work with a correct configuration
> > > on both sides, but experiment showed that it was easy to do an error.
> >
> > I still believe that we will need a property, at least to identify on
> > which we can try the new mode, and on which clocks it's irrelevant (at
> > least for the A33 and A83T).
>
> As told above, setting the new mode on side (clock or MMC) and not on
> the other one prevents the devices to work. Then, it is safer to have
> the new mode capability flag only once for both sides.
>
> Now, as the clocks are defined by memory tables and not by the DT, it
> seems natural to have the flag on the clock side.
You can look at it from two sides.
Either you try to switch to the new mode all the time, or you try to
do it only for MMC controllers (and their associated clocks) that
support it.
In the former case, you'll need to ignore a failure in the switch
(mostly if the clock doesn't support it) only for the MMC controllers
that do not have that mode. For the controllers that support it, you
cannot ignore that error anymore. So you have to have some way to
identify in which case you are.
And you need to have that in the former case too, so there's really no
way you can go without such a flag.
> > However, I also believe we should make that mode switching explicit
> > through a function call, instead of relying on some side effect (of
> > some non-upstream code).
>
> Do you mean a direct call from the MMC driver to the clock driver?
Yes.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC
From: Adrian Hunter @ 2016-10-11 12:39 UTC (permalink / raw)
To: Gregory CLEMENT, Ulf Hansson, linux-mmc
Cc: Hilbert Zhang, Andrew Lunn, Romain Perier, Liuliu Zhao, Peng Zhu,
Nadav Haklai, Ziji Hu, Victor Gu, Doug Jones, Jisheng Zhang,
Yehuda Yitschak, Marcin Wojtas, Xueping Liu, Shiwu Zhang, Yu Cao,
Sebastian Hesselbarth, devicetree, Jason Cooper, Hanna Hawa,
Kostya Porotchkin, Rob Herring, Ryan Gao, Wei(SOCP) Liu,
linux-arm-kernel, Thomas Petazzoni <thomas.petazzo>
In-Reply-To: <e08ffb085d02a784b28456ac47fa4dc6540a9139.1475853198.git-series.gregory.clement@free-electrons.com>
On 07/10/16 18:22, Gregory CLEMENT wrote:
> From: Ziji Hu <huziji@marvell.com>
>
> Marvell Xenon eMMC/SD/SDIO Host Controller contains PHY.
> Three types of PHYs are supported.
>
> Add support to multiple types of PHYs init and configuration.
> Add register definitions of PHYs.
>
> Signed-off-by: Hu Ziji <huziji@marvell.com>
> Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
> MAINTAINERS | 1 +-
> drivers/mmc/host/Makefile | 2 +-
> drivers/mmc/host/sdhci-xenon-phy.c | 1141 +++++++++++++++++++++++++++++-
> drivers/mmc/host/sdhci-xenon-phy.h | 157 ++++-
> drivers/mmc/host/sdhci-xenon.c | 4 +-
> drivers/mmc/host/sdhci-xenon.h | 17 +-
> 6 files changed, 1321 insertions(+), 1 deletion(-)
> create mode 100644 drivers/mmc/host/sdhci-xenon-phy.c
> create mode 100644 drivers/mmc/host/sdhci-xenon-phy.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 859420e5dfd3..b5673c2ee5f2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7583,6 +7583,7 @@ M: Ziji Hu <huziji@marvell.com>
> L: linux-mmc@vger.kernel.org
> S: Supported
> F: drivers/mmc/host/sdhci-xenon.*
> +F: drivers/mmc/host/sdhci-xenon-phy.*
> F: Documentation/devicetree/bindings/mmc/marvell,sdhci-xenon.txt
>
> MATROX FRAMEBUFFER DRIVER
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index 75eaf743486c..4f2854556ff7 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -82,4 +82,4 @@ ifeq ($(CONFIG_CB710_DEBUG),y)
> endif
>
> obj-$(CONFIG_MMC_SDHCI_XENON) += sdhci-xenon-driver.o
> -sdhci-xenon-driver-y += sdhci-xenon.o
> +sdhci-xenon-driver-y += sdhci-xenon.o sdhci-xenon-phy.o
> diff --git a/drivers/mmc/host/sdhci-xenon-phy.c b/drivers/mmc/host/sdhci-xenon-phy.c
> new file mode 100644
> index 000000000000..4eb8fea1bec9
> --- /dev/null
> +++ b/drivers/mmc/host/sdhci-xenon-phy.c
<SNIP>
> +static int __xenon_emmc_delay_adj_test(struct mmc_card *card)
> +{
> + int err;
> + u8 *ext_csd = NULL;
> +
> + err = mmc_get_ext_csd(card, &ext_csd);
> + kfree(ext_csd);
> +
> + return err;
> +}
> +
> +static int __xenon_sdio_delay_adj_test(struct mmc_card *card)
> +{
> + struct mmc_command cmd = {0};
> + int err;
> +
> + cmd.opcode = SD_IO_RW_DIRECT;
> + cmd.flags = MMC_RSP_R5 | MMC_CMD_AC;
> +
> + err = mmc_wait_for_cmd(card->host, &cmd, 0);
> + if (err)
> + return err;
> +
> + if (cmd.resp[0] & R5_ERROR)
> + return -EIO;
> + if (cmd.resp[0] & R5_FUNCTION_NUMBER)
> + return -EINVAL;
> + if (cmd.resp[0] & R5_OUT_OF_RANGE)
> + return -ERANGE;
> + return 0;
> +}
> +
> +static int __xenon_sd_delay_adj_test(struct mmc_card *card)
> +{
> + struct mmc_command cmd = {0};
> + int err;
> +
> + cmd.opcode = MMC_SEND_STATUS;
> + cmd.arg = card->rca << 16;
> + cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
> +
> + err = mmc_wait_for_cmd(card->host, &cmd, 0);
> + return err;
> +}
> +
> +static int xenon_delay_adj_test(struct mmc_card *card)
> +{
> + WARN_ON(!card);
> + WARN_ON(!card->host);
> +
> + if (mmc_card_mmc(card))
> + return __xenon_emmc_delay_adj_test(card);
> + else if (mmc_card_sd(card))
> + return __xenon_sd_delay_adj_test(card);
> + else if (mmc_card_sdio(card))
> + return __xenon_sdio_delay_adj_test(card);
> + else
> + return -EINVAL;
> +}
So you are issuing commands from the ->set_ios() callback. I would want to
get Ulf's OK for that before going further.
One thing: you will need to ensure you don't trigger get HS400 re-tuning
because it will call back into ->set_ios().
And you have the problem that you need to get a reference to the card before
the card device has been added. As I wrote in response to the previous
patch, you should get Ulf's help with that too.
^ permalink raw reply
* Re: [PATCH 6/10] mmc: sdhci-xenon: Add Marvell Xenon SDHC core functionality
From: Adrian Hunter @ 2016-10-11 12:37 UTC (permalink / raw)
To: Gregory CLEMENT, Ulf Hansson, linux-mmc
Cc: Hilbert Zhang, Andrew Lunn, Romain Perier, Liuliu Zhao, Peng Zhu,
Nadav Haklai, Ziji Hu, Victor Gu, Doug Jones, Jisheng Zhang,
Yehuda Yitschak, Marcin Wojtas, Xueping Liu, Shiwu Zhang, Yu Cao,
Sebastian Hesselbarth, devicetree, Jason Cooper, Hanna Hawa,
Kostya Porotchkin, Rob Herring, Ryan Gao, Wei(SOCP) Liu,
linux-arm-kernel, Thomas Petazzoni <thomas.petazzo>
In-Reply-To: <ecde22fd342b0b2001ff5676ea1e67b7226d6946.1475853198.git-series.gregory.clement@free-electrons.com>
On 07/10/16 18:22, Gregory CLEMENT wrote:
> From: Ziji Hu <huziji@marvell.com>
>
> Add Xenon eMMC/SD/SDIO host controller core functionality.
> Add Xenon specific intialization process.
> Add Xenon specific mmc_host_ops APIs.
> Add Xenon specific register definitions.
>
> Add CONFIG_MMC_SDHCI_XENON support in drivers/mmc/host/Kconfig.
>
> Marvell Xenon SDHC conforms to SD Physical Layer Specification
> Version 3.01 and is designed according to the guidelines provided
> in the SD Host Controller Standard Specification Version 3.00.
>
> Signed-off-by: Hu Ziji <huziji@marvell.com>
> Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
I looked at a couple of things but you need to sort out the issues with
card_candidate before going further.
> ---
> MAINTAINERS | 1 +-
> drivers/mmc/host/Kconfig | 9 +-
> drivers/mmc/host/Makefile | 3 +-
> drivers/mmc/host/sdhci-xenon.c | 599 ++++++++++++++++++++++++++++++++++-
> drivers/mmc/host/sdhci-xenon.h | 134 ++++++++-
> 5 files changed, 746 insertions(+), 0 deletions(-)
> create mode 100644 drivers/mmc/host/sdhci-xenon.c
> create mode 100644 drivers/mmc/host/sdhci-xenon.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4aa0eac9bfc7..859420e5dfd3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7582,6 +7582,7 @@ MARVELL XENON MMC/SD/SDIO HOST CONTROLLER DRIVER
> M: Ziji Hu <huziji@marvell.com>
> L: linux-mmc@vger.kernel.org
> S: Supported
> +F: drivers/mmc/host/sdhci-xenon.*
> F: Documentation/devicetree/bindings/mmc/marvell,sdhci-xenon.txt
>
> MATROX FRAMEBUFFER DRIVER
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 5274f503a39a..85a53623526a 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -798,3 +798,12 @@ config MMC_SDHCI_BRCMSTB
> Broadcom STB SoCs.
>
> If unsure, say Y.
> +
> +config MMC_SDHCI_XENON
> + tristate "Marvell Xenon eMMC/SD/SDIO SDHCI driver"
> + depends on MMC_SDHCI && MMC_SDHCI_PLTFM
> + help
> + This selects Marvell Xenon eMMC/SD/SDIO SDHCI.
> + If you have a machine with integrated Marvell Xenon SDHC IP,
> + say Y or M here.
> + If unsure, say N.
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index e2bdaaf43184..75eaf743486c 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -80,3 +80,6 @@ obj-$(CONFIG_MMC_SDHCI_BRCMSTB) += sdhci-brcmstb.o
> ifeq ($(CONFIG_CB710_DEBUG),y)
> CFLAGS-cb710-mmc += -DDEBUG
> endif
> +
> +obj-$(CONFIG_MMC_SDHCI_XENON) += sdhci-xenon-driver.o
> +sdhci-xenon-driver-y += sdhci-xenon.o
> diff --git a/drivers/mmc/host/sdhci-xenon.c b/drivers/mmc/host/sdhci-xenon.c
> new file mode 100644
> index 000000000000..03ba183494d3
> --- /dev/null
> +++ b/drivers/mmc/host/sdhci-xenon.c
> @@ -0,0 +1,599 @@
> +/*
> + * Driver for Marvell SOC Platform Group Xenon SDHC as a platform device
> + *
> + * Copyright (C) 2016 Marvell, All Rights Reserved.
> + *
> + * Author: Hu Ziji <huziji@marvell.com>
> + * Date: 2016-8-24
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation version 2.
> + *
> + * Inspired by Jisheng Zhang <jszhang@marvell.com>
> + * Special thanks to Video BG4 project team.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/mmc/host.h>
> +#include <linux/mmc/mmc.h>
> +#include <linux/mmc/sdio.h>
> +#include <linux/mmc/card.h>
> +#include <linux/mmc/host.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +
> +#include "sdhci-pltfm.h"
> +#include "sdhci.h"
> +#include "sdhci-xenon.h"
> +
> +/* Set SDCLK-off-while-idle */
> +static void xenon_set_sdclk_off_idle(struct sdhci_host *host,
> + unsigned char slot_idx, bool enable)
> +{
> + u32 reg;
> + u32 mask;
> +
> + reg = sdhci_readl(host, SDHC_SYS_OP_CTRL);
> + /* Get the bit shift basing on the slot index */
> + mask = (0x1 << (SDCLK_IDLEOFF_ENABLE_SHIFT + slot_idx));
> + if (enable)
> + reg |= mask;
> + else
> + reg &= ~mask;
> +
> + sdhci_writel(host, reg, SDHC_SYS_OP_CTRL);
> +}
> +
> +/* Enable/Disable the Auto Clock Gating function */
> +static void xenon_set_acg(struct sdhci_host *host, bool enable)
> +{
> + u32 reg;
> +
> + reg = sdhci_readl(host, SDHC_SYS_OP_CTRL);
> + if (enable)
> + reg &= ~AUTO_CLKGATE_DISABLE_MASK;
> + else
> + reg |= AUTO_CLKGATE_DISABLE_MASK;
> + sdhci_writel(host, reg, SDHC_SYS_OP_CTRL);
> +}
> +
> +/* Enable this slot */
> +static void xenon_enable_slot(struct sdhci_host *host,
> + unsigned char slot_idx)
> +{
> + u32 reg;
> +
> + reg = sdhci_readl(host, SDHC_SYS_OP_CTRL);
> + reg |= (BIT(slot_idx) << SLOT_ENABLE_SHIFT);
> + sdhci_writel(host, reg, SDHC_SYS_OP_CTRL);
> +
> + /*
> + * Manually set the flag which all the slots require,
> + * including SD, eMMC, SDIO
> + */
> + host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY;
> +}
> +
> +/* Disable this slot */
> +static void xenon_disable_slot(struct sdhci_host *host,
> + unsigned char slot_idx)
> +{
> + u32 reg;
> +
> + reg = sdhci_readl(host, SDHC_SYS_OP_CTRL);
> + reg &= ~(BIT(slot_idx) << SLOT_ENABLE_SHIFT);
> + sdhci_writel(host, reg, SDHC_SYS_OP_CTRL);
> +}
> +
> +/* Enable Parallel Transfer Mode */
> +static void xenon_enable_slot_parallel_tran(struct sdhci_host *host,
> + unsigned char slot_idx)
> +{
> + u32 reg;
> +
> + reg = sdhci_readl(host, SDHC_SYS_EXT_OP_CTRL);
> + reg |= BIT(slot_idx);
> + sdhci_writel(host, reg, SDHC_SYS_EXT_OP_CTRL);
> +}
> +
> +static void xenon_slot_tuning_setup(struct sdhci_host *host)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> + u32 reg;
> +
> + /* Disable the Re-Tuning Request functionality */
> + reg = sdhci_readl(host, SDHC_SLOT_RETUNING_REQ_CTRL);
> + reg &= ~RETUNING_COMPATIBLE;
> + sdhci_writel(host, reg, SDHC_SLOT_RETUNING_REQ_CTRL);
> +
> + /* Disbale the Re-tuning Event Signal Enable */
Disbale -> Disable
> + reg = sdhci_readl(host, SDHCI_SIGNAL_ENABLE);
> + reg &= ~SDHCI_INT_RETUNE;
> + sdhci_writel(host, reg, SDHCI_SIGNAL_ENABLE);
> +
> + /* Force to use Tuning Mode 1 */
> + host->tuning_mode = SDHCI_TUNING_MODE_1;
> + /* Set re-tuning period */
> + host->tuning_count = 1 << (priv->tuning_count - 1);
> +}
> +
> +/*
> + * Operations inside struct sdhci_ops
> + */
> +/* Recover the Register Setting cleared during SOFTWARE_RESET_ALL */
> +static void sdhci_xenon_reset_exit(struct sdhci_host *host,
> + unsigned char slot_idx, u8 mask)
> +{
> + /* Only SOFTWARE RESET ALL will clear the register setting */
> + if (!(mask & SDHCI_RESET_ALL))
> + return;
> +
> + /* Disable tuning request and auto-retuing again */
retuing -> retuning
> + xenon_slot_tuning_setup(host);
> +
> + xenon_set_acg(host, true);
> +
> + xenon_set_sdclk_off_idle(host, slot_idx, false);
> +}
> +
> +static void sdhci_xenon_reset(struct sdhci_host *host, u8 mask)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +
> + sdhci_reset(host, mask);
> + sdhci_xenon_reset_exit(host, priv->slot_idx, mask);
> +}
> +
> +/*
> + * Xenon defines different values for HS200 and SDR104
> + * in Host_Control_2
> + */
> +static void xenon_set_uhs_signaling(struct sdhci_host *host,
> + unsigned int timing)
> +{
> + u16 ctrl_2;
> +
> + ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> + /* Select Bus Speed Mode for host */
> + ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
> + if (timing == MMC_TIMING_MMC_HS200)
> + ctrl_2 |= XENON_SDHCI_CTRL_HS200;
> + else if (timing == MMC_TIMING_UHS_SDR104)
> + ctrl_2 |= SDHCI_CTRL_UHS_SDR104;
> + else if (timing == MMC_TIMING_UHS_SDR12)
> + ctrl_2 |= SDHCI_CTRL_UHS_SDR12;
> + else if (timing == MMC_TIMING_UHS_SDR25)
> + ctrl_2 |= SDHCI_CTRL_UHS_SDR25;
> + else if (timing == MMC_TIMING_UHS_SDR50)
> + ctrl_2 |= SDHCI_CTRL_UHS_SDR50;
> + else if ((timing == MMC_TIMING_UHS_DDR50) ||
> + (timing == MMC_TIMING_MMC_DDR52))
> + ctrl_2 |= SDHCI_CTRL_UHS_DDR50;
> + else if (timing == MMC_TIMING_MMC_HS400)
> + ctrl_2 |= XENON_SDHCI_CTRL_HS400;
> + sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
> +}
> +
> +static const struct sdhci_ops sdhci_xenon_ops = {
> + .set_clock = sdhci_set_clock,
> + .set_bus_width = sdhci_set_bus_width,
> + .reset = sdhci_xenon_reset,
> + .set_uhs_signaling = xenon_set_uhs_signaling,
> + .get_max_clock = sdhci_pltfm_clk_get_max_clock,
> +};
> +
> +static const struct sdhci_pltfm_data sdhci_xenon_pdata = {
> + .ops = &sdhci_xenon_ops,
> + .quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC |
> + SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12 |
> + SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER |
> + SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
> +};
> +
> +/*
> + * Xenon Specific Operations in mmc_host_ops
> + */
> +static void xenon_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> +{
> + struct sdhci_host *host = mmc_priv(mmc);
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> + unsigned long flags;
> +
> + /*
> + * HS400/HS200/eMMC HS doesn't have Preset Value register.
> + * However, sdhci_set_ios will read HS400/HS200 Preset register.
> + * Disable Preset Value register for HS400/HS200.
> + * eMMC HS with preset_enabled set will trigger a bug in
> + * get_preset_value().
> + */
> + spin_lock_irqsave(&host->lock, flags);
> + if ((ios->timing == MMC_TIMING_MMC_HS400) ||
> + (ios->timing == MMC_TIMING_MMC_HS200) ||
> + (ios->timing == MMC_TIMING_MMC_HS)) {
> + host->preset_enabled = false;
> + host->quirks2 |= SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
> + } else {
> + host->quirks2 &= ~SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
> + }
> + spin_unlock_irqrestore(&host->lock, flags);
> +
> + sdhci_set_ios(mmc, ios);
> +
> + if (host->clock > DEFAULT_SDCLK_FREQ) {
> + spin_lock_irqsave(&host->lock, flags);
> + xenon_set_sdclk_off_idle(host, priv->slot_idx, true);
> + spin_unlock_irqrestore(&host->lock, flags);
> + }
> +}
> +
> +static int __emmc_signal_voltage_switch(struct mmc_host *mmc,
> + const unsigned char signal_voltage)
> +{
> + u32 ctrl;
> + unsigned char voltage_code;
> + struct sdhci_host *host = mmc_priv(mmc);
> +
> + if (signal_voltage == MMC_SIGNAL_VOLTAGE_330)
> + voltage_code = EMMC_VCCQ_3_3V;
> + else if (signal_voltage == MMC_SIGNAL_VOLTAGE_180)
> + voltage_code = EMMC_VCCQ_1_8V;
> + else
> + return -EINVAL;
> +
> + /*
> + * This host is for eMMC, XENON self-defined
> + * eMMC slot control register should be accessed
> + * instead of Host Control 2
> + */
> + ctrl = sdhci_readl(host, SDHC_SLOT_EMMC_CTRL);
> + ctrl &= ~EMMC_VCCQ_MASK;
> + ctrl |= voltage_code;
> + sdhci_writel(host, ctrl, SDHC_SLOT_EMMC_CTRL);
> +
> + /* There is no standard to determine this waiting period */
> + usleep_range(1000, 2000);
> +
> + /* Check whether io voltage switch is done */
> + ctrl = sdhci_readl(host, SDHC_SLOT_EMMC_CTRL);
> + ctrl &= EMMC_VCCQ_MASK;
> + /*
> + * This bit is set only when regulator feeds back the voltage switch
> + * results to Xenon SDHC.
> + * However, in actaul implementation, regulator might not provide
> + * this feedback.
> + * Thus we shall not rely on this bit to determine if switch failed.
> + * If the bit is not set, just throw a message.
> + * Besides, error code should not be returned.
> + */
> + if (ctrl != voltage_code)
> + dev_info(mmc_dev(mmc), "fail to detect eMMC signal voltage stable\n");
> + return 0;
> +}
> +
> +static int xenon_emmc_signal_voltage_switch(struct mmc_host *mmc,
> + struct mmc_ios *ios)
> +{
> + unsigned char voltage = ios->signal_voltage;
> +
> + if ((voltage == MMC_SIGNAL_VOLTAGE_330) ||
> + (voltage == MMC_SIGNAL_VOLTAGE_180))
> + return __emmc_signal_voltage_switch(mmc, voltage);
> +
> + dev_err(mmc_dev(mmc), "Unsupported signal voltage: %d\n",
> + voltage);
> + return -EINVAL;
> +}
> +
> +static int xenon_start_signal_voltage_switch(struct mmc_host *mmc,
> + struct mmc_ios *ios)
> +{
> + struct sdhci_host *host = mmc_priv(mmc);
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +
> + /*
> + * Before SD/SDIO set signal voltage, SD bus clock should be
> + * disabled. However, sdhci_set_clock will also disable the Internal
> + * clock in mmc_set_signal_voltage().
> + * If Internal clock is disabled, the 3.3V/1.8V bit can not be updated.
> + * Thus here manually enable internal clock.
> + *
> + * After switch completes, it is unnecessary to disable internal clock,
> + * since keeping internal clock active obeys SD spec.
> + */
> + enable_xenon_internal_clk(host);
> +
> + if (priv->card_candidate) {
mmc_power_up() calls __mmc_set_signal_voltage() calls
host->ops->start_signal_voltage_switch so priv->card_candidate could be an
invalid reference to an old card.
So that's not going to work if the card changes - not only for removable
cards but even for eMMC if init fails and retries.
> + if (mmc_card_mmc(priv->card_candidate))
> + return xenon_emmc_signal_voltage_switch(mmc, ios);
So if all you need to know is whether it is a eMMC, why can't DT tell you?
> + }
> +
> + return sdhci_start_signal_voltage_switch(mmc, ios);
> +}
> +
> +/*
> + * After determining which slot is used for SDIO,
> + * some additional task is required.
> + */
> +static void xenon_init_card(struct mmc_host *mmc, struct mmc_card *card)
> +{
> + struct sdhci_host *host = mmc_priv(mmc);
> + u32 reg;
> + u8 slot_idx;
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +
> + /* Link the card for delay adjustment */
> + priv->card_candidate = card;
You really need a better way to get the card. I suggest you take up the
issue with Ulf. One possibility is to have mmc core set host->card = card
much earlier.
> + /* Set tuning functionality of this slot */
> + xenon_slot_tuning_setup(host);
> +
> + slot_idx = priv->slot_idx;
> + if (!mmc_card_sdio(card)) {
> + /* Re-enable the Auto-CMD12 cap flag. */
> + host->quirks |= SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12;
> + host->flags |= SDHCI_AUTO_CMD12;
> +
> + /* Clear SDIO Card Inserted indication */
> + reg = sdhci_readl(host, SDHC_SYS_CFG_INFO);
> + reg &= ~(1 << (slot_idx + SLOT_TYPE_SDIO_SHIFT));
> + sdhci_writel(host, reg, SDHC_SYS_CFG_INFO);
> +
> + if (mmc_card_mmc(card)) {
> + mmc->caps |= MMC_CAP_NONREMOVABLE;
> + if (!(host->quirks2 & SDHCI_QUIRK2_NO_1_8_V))
> + mmc->caps |= MMC_CAP_1_8V_DDR;
> + /*
> + * Force to clear BUS_TEST to
> + * skip bus_test_pre and bus_test_post
> + */
> + mmc->caps &= ~MMC_CAP_BUS_WIDTH_TEST;
> + mmc->caps2 |= MMC_CAP2_HC_ERASE_SZ |
> + MMC_CAP2_PACKED_CMD;
> + if (mmc->caps & MMC_CAP_8_BIT_DATA)
> + mmc->caps2 |= MMC_CAP2_HS400_1_8V;
> + }
> + } else {
> + /*
> + * Delete the Auto-CMD12 cap flag.
> + * Otherwise, when sending multi-block CMD53,
> + * Driver will set Transfer Mode Register to enable Auto CMD12.
> + * However, SDIO device cannot recognize CMD12.
> + * Thus SDHC will time-out for waiting for CMD12 response.
> + */
> + host->quirks &= ~SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12;
> + host->flags &= ~SDHCI_AUTO_CMD12;
sdhci_set_transfer_mode() won't enable auto-CMD12 for CMD53 anyway, so is
this needed?
> +
> + /*
> + * Set SDIO Card Inserted indication
> + * to inform that the current slot is for SDIO
> + */
> + reg = sdhci_readl(host, SDHC_SYS_CFG_INFO);
> + reg |= (1 << (slot_idx + SLOT_TYPE_SDIO_SHIFT));
> + sdhci_writel(host, reg, SDHC_SYS_CFG_INFO);
> + }
> +}
> +
> +static int xenon_execute_tuning(struct mmc_host *mmc, u32 opcode)
> +{
> + struct sdhci_host *host = mmc_priv(mmc);
> +
> + if (host->timing == MMC_TIMING_UHS_DDR50)
> + return 0;
> +
> + return sdhci_execute_tuning(mmc, opcode);
> +}
> +
> +static void xenon_replace_mmc_host_ops(struct sdhci_host *host)
> +{
> + host->mmc_host_ops.set_ios = xenon_set_ios;
> + host->mmc_host_ops.start_signal_voltage_switch =
> + xenon_start_signal_voltage_switch;
> + host->mmc_host_ops.init_card = xenon_init_card;
> + host->mmc_host_ops.execute_tuning = xenon_execute_tuning;
> +}
> +
> +static int xenon_probe_dt(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct sdhci_host *host = platform_get_drvdata(pdev);
> + struct mmc_host *mmc = host->mmc;
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> + int err;
> + u32 slot_idx, nr_slot;
> + u32 tuning_count;
> + u32 reg;
> +
> + /* Standard MMC property */
> + err = mmc_of_parse(mmc);
> + if (err)
> + return err;
> +
> + /* Standard SDHCI property */
> + sdhci_get_of_property(pdev);
> +
> + /*
> + * Xenon Specific property:
> + * slotno: the index of slot. Refer to SDHC_SYS_CFG_INFO register
> + * tuning-count: the interval between re-tuning
> + * PHY type: "sdhc phy", "emmc phy 5.0" or "emmc phy 5.1"
> + */
> + if (!of_property_read_u32(np, "xenon,slotno", &slot_idx)) {
> + nr_slot = sdhci_readl(host, SDHC_SYS_CFG_INFO);
> + nr_slot &= NR_SUPPORTED_SLOT_MASK;
> + if (unlikely(slot_idx > nr_slot)) {
> + dev_err(mmc_dev(mmc), "Slot Index %d exceeds Number of slots %d\n",
> + slot_idx, nr_slot);
> + return -EINVAL;
> + }
> + } else {
> + priv->slot_idx = 0x0;
> + }
> +
> + if (!of_property_read_u32(np, "xenon,tuning-count", &tuning_count)) {
> + if (unlikely(tuning_count >= TMR_RETUN_NO_PRESENT)) {
> + dev_err(mmc_dev(mmc), "Wrong Re-tuning Count. Set default value %d\n",
> + DEF_TUNING_COUNT);
> + tuning_count = DEF_TUNING_COUNT;
> + }
> + } else {
> + priv->tuning_count = DEF_TUNING_COUNT;
> + }
> +
> + if (of_property_read_bool(np, "xenon,mask-conflict-err")) {
> + reg = sdhci_readl(host, SDHC_SYS_EXT_OP_CTRL);
> + reg |= MASK_CMD_CONFLICT_ERROR;
> + sdhci_writel(host, reg, SDHC_SYS_EXT_OP_CTRL);
> + }
> +
> + return err;
> +}
> +
> +static int xenon_slot_probe(struct sdhci_host *host)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> + u8 slot_idx = priv->slot_idx;
> +
> + /* Enable slot */
> + xenon_enable_slot(host, slot_idx);
> +
> + /* Enable ACG */
> + xenon_set_acg(host, true);
> +
> + /* Enable Parallel Transfer Mode */
> + xenon_enable_slot_parallel_tran(host, slot_idx);
> +
> + priv->timing = MMC_TIMING_FAKE;
> + priv->clock = 0;
> +
> + return 0;
> +}
> +
> +static void xenon_slot_remove(struct sdhci_host *host)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> + u8 slot_idx = priv->slot_idx;
> +
> + /* disable slot */
> + xenon_disable_slot(host, slot_idx);
> +}
> +
> +static int sdhci_xenon_probe(struct platform_device *pdev)
> +{
> + struct sdhci_pltfm_host *pltfm_host;
> + struct sdhci_host *host;
> + struct clk *clk, *axi_clk;
> + struct sdhci_xenon_priv *priv;
> + int err;
> +
> + host = sdhci_pltfm_init(pdev, &sdhci_xenon_pdata,
> + sizeof(struct sdhci_xenon_priv));
> + if (IS_ERR(host))
> + return PTR_ERR(host);
> +
> + pltfm_host = sdhci_priv(host);
> + priv = sdhci_pltfm_priv(pltfm_host);
> +
> + xenon_set_acg(host, false);
> +
> + /*
> + * Link Xenon specific mmc_host_ops function,
> + * to replace standard ones in sdhci_ops.
> + */
> + xenon_replace_mmc_host_ops(host);
> +
> + clk = devm_clk_get(&pdev->dev, "core");
> + if (IS_ERR(clk)) {
> + dev_err(&pdev->dev, "Failed to setup input clk.\n");
> + err = PTR_ERR(clk);
> + goto free_pltfm;
> + }
> + clk_prepare_enable(clk);
> + pltfm_host->clk = clk;
> +
> + /*
> + * Some SOCs require additional clock to
> + * manage AXI bus clock.
> + * It is optional.
> + */
> + axi_clk = devm_clk_get(&pdev->dev, "axi");
> + if (!IS_ERR(axi_clk)) {
> + clk_prepare_enable(axi_clk);
> + priv->axi_clk = axi_clk;
> + }
> +
> + err = xenon_probe_dt(pdev);
> + if (err)
> + goto err_clk;
> +
> + err = xenon_slot_probe(host);
> + if (err)
> + goto err_clk;
> +
> + err = sdhci_add_host(host);
> + if (err)
> + goto remove_slot;
> +
> + return 0;
> +
> +remove_slot:
> + xenon_slot_remove(host);
> +err_clk:
> + clk_disable_unprepare(pltfm_host->clk);
> + if (!IS_ERR(axi_clk))
> + clk_disable_unprepare(axi_clk);
> +free_pltfm:
> + sdhci_pltfm_free(pdev);
> + return err;
> +}
> +
> +static int sdhci_xenon_remove(struct platform_device *pdev)
> +{
> + struct sdhci_host *host = platform_get_drvdata(pdev);
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> + int dead = (readl(host->ioaddr + SDHCI_INT_STATUS) == 0xFFFFFFFF);
> +
> + xenon_slot_remove(host);
> +
> + sdhci_remove_host(host, dead);
> +
> + clk_disable_unprepare(pltfm_host->clk);
> + clk_disable_unprepare(priv->axi_clk);
> +
> + sdhci_pltfm_free(pdev);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id sdhci_xenon_dt_ids[] = {
> + { .compatible = "marvell,sdhci-xenon",},
> + { .compatible = "marvell,armada-3700-sdhci",},
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, sdhci_xenon_dt_ids);
> +
> +static struct platform_driver sdhci_xenon_driver = {
> + .driver = {
> + .name = "sdhci-xenon",
> + .of_match_table = sdhci_xenon_dt_ids,
> + .pm = &sdhci_pltfm_pmops,
> + },
> + .probe = sdhci_xenon_probe,
> + .remove = sdhci_xenon_remove,
> +};
> +
> +module_platform_driver(sdhci_xenon_driver);
> +
> +MODULE_DESCRIPTION("SDHCI platform driver for Marvell Xenon SDHC");
> +MODULE_AUTHOR("Hu Ziji <huziji@marvell.com>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/mmc/host/sdhci-xenon.h b/drivers/mmc/host/sdhci-xenon.h
> new file mode 100644
> index 000000000000..c2370493fbe8
> --- /dev/null
> +++ b/drivers/mmc/host/sdhci-xenon.h
> @@ -0,0 +1,134 @@
> +/*
> + * Copyright (C) 2016 Marvell, All Rights Reserved.
> + *
> + * Author: Hu Ziji <huziji@marvell.com>
> + * Date: 2016-8-24
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation version 2.
> + */
> +#ifndef SDHCI_XENON_H_
> +#define SDHCI_XENON_H_
> +
> +#include <linux/clk.h>
> +#include <linux/mmc/card.h>
> +#include <linux/of.h>
> +#include "sdhci.h"
> +
> +/* Register Offset of SD Host Controller SOCP self-defined register */
> +#define SDHC_SYS_CFG_INFO 0x0104
> +#define SLOT_TYPE_SDIO_SHIFT 24
> +#define SLOT_TYPE_EMMC_MASK 0xFF
> +#define SLOT_TYPE_EMMC_SHIFT 16
> +#define SLOT_TYPE_SD_SDIO_MMC_MASK 0xFF
> +#define SLOT_TYPE_SD_SDIO_MMC_SHIFT 8
> +#define NR_SUPPORTED_SLOT_MASK 0x7
> +
> +#define SDHC_SYS_OP_CTRL 0x0108
> +#define AUTO_CLKGATE_DISABLE_MASK BIT(20)
> +#define SDCLK_IDLEOFF_ENABLE_SHIFT 8
> +#define SLOT_ENABLE_SHIFT 0
> +
> +#define SDHC_SYS_EXT_OP_CTRL 0x010C
> +#define MASK_CMD_CONFLICT_ERROR BIT(8)
> +
> +#define SDHC_SLOT_OP_STATUS_CTRL 0x0128
> +#define DELAY_90_DEGREE_MASK_EMMC5 BIT(7)
> +#define DELAY_90_DEGREE_SHIFT_EMMC5 7
> +#define EMMC_5_0_PHY_FIXED_DELAY_MASK 0x7F
> +#define EMMC_PHY_FIXED_DELAY_MASK 0xFF
> +#define EMMC_PHY_FIXED_DELAY_WINDOW_MIN (EMMC_PHY_FIXED_DELAY_MASK >> 3)
> +#define SDH_PHY_FIXED_DELAY_MASK 0x1FF
> +#define SDH_PHY_FIXED_DELAY_WINDOW_MIN (SDH_PHY_FIXED_DELAY_MASK >> 4)
> +
> +#define TUN_CONSECUTIVE_TIMES_SHIFT 16
> +#define TUN_CONSECUTIVE_TIMES_MASK 0x7
> +#define TUN_CONSECUTIVE_TIMES 0x4
> +#define TUNING_STEP_SHIFT 12
> +#define TUNING_STEP_MASK 0xF
> +#define TUNING_STEP_DIVIDER BIT(6)
> +
> +#define FORCE_SEL_INVERSE_CLK_SHIFT 11
> +
> +#define SDHC_SLOT_EMMC_CTRL 0x0130
> +#define ENABLE_DATA_STROBE BIT(24)
> +#define SET_EMMC_RSTN BIT(16)
> +#define DISABLE_RD_DATA_CRC BIT(14)
> +#define DISABLE_CRC_STAT_TOKEN BIT(13)
> +#define EMMC_VCCQ_MASK 0x3
> +#define EMMC_VCCQ_1_8V 0x1
> +#define EMMC_VCCQ_3_3V 0x3
> +
> +#define SDHC_SLOT_RETUNING_REQ_CTRL 0x0144
> +/* retuning compatible */
> +#define RETUNING_COMPATIBLE 0x1
> +
> +#define SDHC_SLOT_EXT_PRESENT_STATE 0x014C
> +#define LOCK_STATE 0x1
> +
> +#define SDHC_SLOT_DLL_CUR_DLY_VAL 0x0150
> +
> +/* Tuning Parameter */
> +#define TMR_RETUN_NO_PRESENT 0xF
> +#define DEF_TUNING_COUNT 0x9
> +
> +#define MMC_TIMING_FAKE 0xFF
> +
> +#define DEFAULT_SDCLK_FREQ (400000)
> +
> +/* Xenon specific Mode Select value */
> +#define XENON_SDHCI_CTRL_HS200 0x5
> +#define XENON_SDHCI_CTRL_HS400 0x6
> +
> +struct sdhci_xenon_priv {
> + /*
> + * The bus_width, timing, and clock fields in below
> + * record the current setting of Xenon SDHC.
> + * Driver will call a Sampling Fixed Delay Adjustment
> + * if any setting is changed.
> + */
> + unsigned char bus_width;
> + unsigned char timing;
> + unsigned char tuning_count;
> + unsigned int clock;
> + struct clk *axi_clk;
> +
> + /* Slot idx */
> + u8 slot_idx;
> +
> + /*
> + * When initializing card, Xenon has to determine card type and
> + * adjust Sampling Fixed delay.
> + * However, at that time, card structure is not linked to mmc_host.
> + * Thus a card pointer is added here to provide
> + * the delay adjustment function with the card structure
> + * of the card during initialization
> + */
> + struct mmc_card *card_candidate;
> +};
> +
> +static inline int enable_xenon_internal_clk(struct sdhci_host *host)
> +{
> + u32 reg;
> + u8 timeout;
> +
> + reg = sdhci_readl(host, SDHCI_CLOCK_CONTROL);
> + reg |= SDHCI_CLOCK_INT_EN;
> + sdhci_writel(host, reg, SDHCI_CLOCK_CONTROL);
> + /* Wait max 20 ms */
> + timeout = 20;
> + while (!((reg = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
> + & SDHCI_CLOCK_INT_STABLE)) {
> + if (timeout == 0) {
> + pr_err("%s: Internal clock never stabilised.\n",
> + mmc_hostname(host->mmc));
> + return -ETIMEDOUT;
> + }
> + timeout--;
> + mdelay(1);
> + }
> +
> + return 0;
> +}
> +#endif
>
^ permalink raw reply
* Re: Enabling MMC BKOPs in kernel based on host caps
From: Alex Lemberg @ 2016-10-11 9:58 UTC (permalink / raw)
To: Ravikumar, Shawn Lin, Ravikumar Kattekola,
linux-mmc@vger.kernel.org
In-Reply-To: <29d94a8f-935e-b313-a562-dc15909b78d1@ti.com>
Hi Ravikumar,
[…]
>Should the code to enable BKOPS be wrapped inside a CONFIG_ENABLE_BKOPS
>in order to give the individual an option to change the default behavior?
In case both storage device and the driver decide to enable BKOPS
based on internal status, params and registers, I don’t think the user should
have such an option in config?
But anyway, I would let Ulf and Shawn to comment on this.
Thanks,
Alex
[…]
^ permalink raw reply
* Re: [PATCH v5 03/12] mmc: sdhci-msm: add pltfm_data support to get clk-rates from DT
From: Rob Herring @ 2016-10-11 12:31 UTC (permalink / raw)
To: Ritesh Harjani
Cc: Ulf Hansson, linux-mmc@vger.kernel.org, Adrian Hunter, Shawn Lin,
David Brown, Andy Gross, devicetree@vger.kernel.org,
linux-arm-msm, georgi.djakov, alex.lemberg, mateusz.nowak,
Yuliy.Izrailov, asutoshd, David Griego, Sahitya Tummala, venkatg,
Stephen Boyd, Bjorn Andersson, pramod.gurav
In-Reply-To: <9db96ae7-9a5e-1494-2371-e5b346a08155@codeaurora.org>
On Tue, Oct 11, 2016 at 4:06 AM, Ritesh Harjani <riteshh@codeaurora.org> wrote:
> Hi Rob
>
>
> On 10/11/2016 12:59 AM, Rob Herring wrote:
>>
>> On Mon, Oct 10, 2016 at 11:07 AM, Ritesh Harjani <riteshh@codeaurora.org>
>> wrote:
>>>
>>> Hi Rob,
>>>
>>> Thanks for review.
>>>
>>> On 10/10/2016 6:27 PM, Rob Herring wrote:
>>>>
>>>>
>>>> On Wed, Oct 05, 2016 at 08:10:31PM +0530, Ritesh Harjani wrote:
>>>>>
>>>>>
>>>>> This adds support for sdhc-msm controllers to get supported
>>>>> clk-rates from DT. sdhci-msm would need it's own set_clock
>>>>> ops to be implemented. For this, supported clk-rates needs
>>>>> to be populated in sdhci_msm_pltfm_data.
>>>>>
>>>>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>>>>> ---
>>>>> .../devicetree/bindings/mmc/sdhci-msm.txt | 1 +
>>>>> drivers/mmc/host/sdhci-msm.c | 48
>>>>> ++++++++++++++++++++++
>>>>> 2 files changed, 49 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>>>> b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>>>> index 485483a..6a83b38 100644
>>>>> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>>>> @@ -17,6 +17,7 @@ Required properties:
>>>>> "iface" - Main peripheral bus clock (PCLK/HCLK - AHB Bus clock)
>>>>> (required)
>>>>> "core" - SDC MMC clock (MCLK) (required)
>>>>> "bus" - SDCC bus voter clock (optional)
>>>>> +- clk-rates: Array of supported GCC clock frequencies for sdhc, Units
>>>>> -
>>>>> Hz.
>>>>
>>>>
>>>>
>>>> Why can't some combination of assigned-clock-rates and querying the
>>>> clock provider for rates be used here?
>>>
>>>
>>> From what I understood, assigned-clock-rates would only work for setting
>>> some default clock rates for certain clocks by calling
>>> of_clk_set_defaults.
>>>
>>> Whereas the requirement here is -
>>> That since SDHC msm directly controls the clk(core clock) at source, it's
>>> sdhci-msm driver needs to know the supported clk-rates by the underlying
>>> platform to configure the nearest floor value supported on this platform
>>> (when the request arrives from the core layer to switch the clock).
>>
>>
>> Why does clk_round_rate not work for you? That will round down to the
>> nearest frequency supported.
>
> clk_round_rate will round off to nearest supported "ceil" frequency.
> But we require nearest rounded off "floor" frequency.
Then fix the clk framework to do what you want. This doesn't need to be in DT.
Rob
^ permalink raw reply
* Re: [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Ziji Hu @ 2016-10-11 10:03 UTC (permalink / raw)
To: Rob Herring, Gregory CLEMENT
Cc: Ulf Hansson, Adrian Hunter, linux-mmc, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, devicetree, Thomas Petazzoni,
linux-arm-kernel, Jack(SH) Zhu, Jimmy Xu, Jisheng Zhang,
Nadav Haklai, Ryan Gao, Doug Jones, Shiwu Zhang, Victor Gu,
Wei(SOCP) Liu, Wilson Ding, Xueping Liu, Hilbert Zhang
In-Reply-To: <20161010213417.GA11795@rob-hp-laptop>
Hi Rob,
Thanks a for the review.
It is really helpful to me.
On 2016/10/11 5:34, Rob Herring wrote:
> On Fri, Oct 07, 2016 at 05:22:51PM +0200, Gregory CLEMENT wrote:
>> From: Ziji Hu <huziji@marvell.com>
>>
>> Marvell Xenon SDHC can support eMMC/SD/SDIO.
>> Add Xenon-specific properties.
>> Also add properties for Xenon PHY setting.
>>
>> Signed-off-by: Hu Ziji <huziji@marvell.com>
>> Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> ---
>> Documentation/devicetree/bindings/mmc/marvell,sdhci-xenon.txt | 164 +++++++-
>> MAINTAINERS | 1 +-
>> 2 files changed, 165 insertions(+), 0 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/mmc/marvell,sdhci-xenon.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/marvell,sdhci-xenon.txt b/Documentation/devicetree/bindings/mmc/marvell,sdhci-xenon.txt
>> new file mode 100644
>> index 000000000000..8b25ad28ebbd
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mmc/marvell,sdhci-xenon.txt
>> @@ -0,0 +1,164 @@
>> +Marvell's Xenon SDHCI Controller device tree bindings
>> +This file documents differences between the core mmc properties
>> +described by mmc.txt and the properties used by the Xenon implementation.
>> +
>> +A single Xenon IP can support multiple slots.
>> +Each slot acts as an independent SDHC. It owns independent resources, such
>> +as register sets clock and PHY.
>
> Is the phy really part of the same block?
>
Each SDHC slot owns its PHY. It is part of a SDHC slot.
It is independent to another SDHC slot.
>> +Each slot should have an independent device tree node.
>> +
>> +Required Properties:
>> +- compatible: should be "marvell,sdhci-xenon" or "marvell,armada-3700-sdhci".
>
> Perhaps some consistent ordering (w/ -sdhci on the end).
Sure.
I will adjust the ordering.
>
>> +
>> +- Input Clock Name
>
> Your formatting of properties is a bit strange. Please restructure like
> most bindings so the property names are before all the description.
>
OK.
I will fix the format.
>> + Some SOCs require additional clock for AXI bus.
>
> Those SoCs should have a specific compatible string and you need to
> define which compatible strings have 2 clocks vs. 1 clock.
>
Actually, I copy this implementation from another Marvell SDIO Host Controller, sdhci-pxa.
It is in sdhci-pxa.txt.
I would like to know if it is still acceptable.
>> + The input clock for Xenon IP core should be named as "core".
>> + The optional AXI clock should be named as "axi".
>> + - clocks = <&core_clk>, <&axi_clock>;
>> + - clock-names = "core", "axi";
>> +
>> +- Register Set Size
>
> Is this a property name?
Sorry, it isn't.
I will fix the format.
>
>> + Different Xenon SDHC release has different register set size.
>> + The specific size should also refer to the SOC implementation.
>> +
>> +Optional Properties:
>> +- Slot Index
>> + A single Xenon IP can support multiple slots.
>> + During initialization, each slot should set corresponding setting bit in
>> + some Xenon-specific registers. The corresponding bit is determined by
>> + this property.
>> + - xenon,slotno = <slot_index>;
>
> Slots should probably be represented as child nodes with the reg
> property being the slot number.
Since each SDHC slot is independent, I find it is more convenient to implement each one as independent SD host/MMC host instant.
Otherwise, a main function should loop and initialize each slot, like sdhci-pci. I prefer to avoiding such a unnecessary main function.
It is very hard to determine the slot number by reg property.
Xenon slots are likely to be different types. 1st slot might be eMMC and 2nd one might be SD. They might have different register size.
The register size might also varies in different Xenon versions.
>
> Also, xenon is not a vendor prefix.
>
Yes. The issue is that there are multiple Marvell SD Host Controllers existing in kernel.
If marvell is used as a prefix here, I concern that it might be confused with other Marvell sdhc.
Can I use a combination of marvell and xenon as a prefix, such as mrvl-xenon?
>> + If this property is not provided, Xenon IP should contain only one slot
>> + and the slot index will be 0x0 by default.
>> +
>> +- PHY Type
>
> You're going to need to come of with a common binding for this.
>
Could you please provide more details about the "common binding" here?
The PHY Type property is Xenon-specific, instead of a standard or a spec.
Thus I cannot find a common property to stand for it.
>> + Xenon support mutilple types of PHYs.
>> + To select eMMC 5.1 PHY, set:
>> + - xenon,phy-type = "emmc 5.1 phy"
>> + eMMC 5.1 PHY is the default choice if this property is not provided.
>> + To select eMMC 5.0 PHY, set:
>> + - xenon,phy-type = "emmc 5.0 phy"
>> + To select SDH PHY, set:
>> + - xenon,phy-type = "sdh phy"
>> + Please note that eMMC PHY is a general PHY for eMMC/SD/SDIO, other than for
>> + eMMC only.
>> +
>> +- Customized eMMC PHY Parameters
>> + Some boards require different values of some specific eMMC PHY parameters.
>> + Some SOCs also require specific workaround to set eMMC PHY.
>> + These properties enable diverse boards to customize the eMMC PHY.
>> + The supported eMMC PHY parameters are listed in below. All those properties
>> + are only available for eMMC PHY 5.1 and eMMC PHY 5.0.
>> + ZNR
>> + valid range = [0:0x1F].
>> + ZNR is set as 0xF by default if this property is not provided.
>> + - xenon,phy-znr = <value>;
>> +
>> + ZPR
>> + valid range = [0:0x1F].
>> + ZPR is set as 0xF by default if this property is not provided.
>> + - xenon,phy-zpr = <value>;
>
> marvell is the vendor prefix.
>
>> +
>> + Number of successful tuning times
>> + Set the number of required consecutive successful sampling points used to
>> + identify a valid sampling window, in tuning process.
>> + Valid range = [1:7]. Set as 0x4 by default if this property is not provided.
>> + - xenon,phy-nr-tun-times = <nr_times>;
>> +
>> + Divider for TUN_STEP
>> + Set the divider for calculating TUN_STEP.
>> + Set as 64 by default if this property is not provided.
>> + - xenon,phy-tun-step-divider = <divider>;
>> +
>> + Force PHY into slow mode.
>> + Only available when bus frequency lower than 50MHz in SDR mde.
>> + Disabled by default. Please do not enable it unless it is necessary.
>> + - xenon,phy-slow-mode;
>> +
>> +- Mask Conflict Error Report
>> + Disable Conflict Error alert on some SOC. Disabled by default.
>> + xenon,mask-conflict-err;
>> +
>> +- Re-tuning Counter
>> + Xenon SDHC SOC usually doesn't provide re-tuning counter in
>> + Capabilities Register 3 Bit[11:8].
>> + This property provides the re-tuning counter.
>> + xenon,tuning-count = <count>;
>> + If this property is not set, default re-tuning counter will
>> + be set as 0x9 in driver.
>> +
>> +- SOC PHY PAD Voltage Control register
>> + Some SOCs have SOC PHY PAD Voltage Control register outside Xenon IP.
>> + This register sets SOC PHY PAD Voltage to keep aligh with Vccq.
>> + Two properties provide information of this control register.
>> + These two properties are only valid when "marvell,armada-3700-sdhci"
>> + is selected. Both of them must be provided when "marvell,armada-3700-sdhci"
>> + is selected.
>> + - xenon,pad-type
>> + Two types: "sd" and "fixed-1-8v".
>> + If "sd" is slected, SOC PHY PAD is set as 3.3V at the beginning and is
>> + switched to 1.8V when SD in UHS-I.
>> + If "fixed-1-8v" is slected, SOC PHY PAD is fixed 1.8V, such as for eMMC.
>
> You should be able to existing, common properties for i/o voltage
> capabilities/constraints.
>
The above property is for a special SOC platform in Marvell.
It is irrelevant to common PHY framework or standard MMC bindings.
Thus I cannot find a existing and common property to represent it.
Thank you.
Best regards,
Hu Ziji
>> + - reg
>> + Physical address and size of SOC PHY PAD register.
>> + Append after Xenon SDHC register space, as a second register field.
>> +
>> + Please follow the examples with compatible "marvell,armada-3700-sdhci"
>> + in below.
>> +
>> +Example:
>> +- For eMMC slot:
>> +
>> + sdhci@aa0000 {
>> + compatible = "marvell,sdhci-xenon";
>> + reg = <0xaa0000 0x1000>;
>> + interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>
>> + clocks = <&emmcclk>;
>> + clock-names = "core";
>> + xenon,slotno = <0>;
>> + xenon,phy-type = "emmc 5.1 phy";
>> + bus-width = <8>;
>> + tuning-count = <11>;
>> + };
>> +
>> +- For SD/SDIO slot:
>> +
>> + sdhci@ab0000 {
>> + compatible = "marvell,sdhci-xenon";
>> + reg = <0xab0000 0x1000>;
>> + interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>
>> + vqmmc-supply = <&sd_regulator>;
>> + clocks = <&sdclk>;
>> + clock-names = "core";
>> + bus-width = <4>;
>> + tuning-count = <9>;
>> + };
>> +
>> +- For eMMC slot with compatible "marvell,armada-3700-sdhci":
>> +
>> + sdhci@aa0000 {
>> + compatible = "marvell,armada-3700-sdhci";
>> + reg = <0xaa0000 0x1000>,
>> + <phy_addr 0x4>;
>> + interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>
>> + clocks = <&emmcclk>;
>> + clock-names = "core";
>> + bus-width = <8>;
>> +
>> + xenon,pad-type = "fixed-1-8v";
>> + };
>> +
>> +- For SD/SDIO slot with compatible "marvell,armada-3700-sdhci":
>> +
>> + sdhci@ab0000 {
>> + compatible = "marvell,armada-3700-sdhci";
>> + reg = <0xab0000 0x1000>,
>> + <phy_addr 0x4>;
>> + interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>
>> + vqmmc-supply = <&sd_regulator>;
>> + clocks = <&sdclk>;
>> + clock-names = "core";
>> + bus-width = <4>;
>> +
>> + xenon,pad-type = "sd";
>> + };
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 89adcd57aa25..4aa0eac9bfc7 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -7582,6 +7582,7 @@ MARVELL XENON MMC/SD/SDIO HOST CONTROLLER DRIVER
>> M: Ziji Hu <huziji@marvell.com>
>> L: linux-mmc@vger.kernel.org
>> S: Supported
>> +F: Documentation/devicetree/bindings/mmc/marvell,sdhci-xenon.txt
>>
>> MATROX FRAMEBUFFER DRIVER
>> L: linux-fbdev@vger.kernel.org
>> --
>> git-series 0.8.10
^ permalink raw reply
* Re: [PATCH] mmc: sdhci: put together into one condition checking
From: Adrian Hunter @ 2016-10-11 9:28 UTC (permalink / raw)
To: Jaehoon Chung, linux-mmc; +Cc: ulf.hansson
In-Reply-To: <1475816923-24532-1-git-send-email-jh80.chung@samsung.com>
On 07/10/16 08:08, Jaehoon Chung wrote:
> value of ios->timing is not related with SDCHI v3.0.
> If Controller version is v3.0, SDHCI_QUIRK_NO_HISPD_BIT is meaningless.
> To prevent the setting wrong bit moves into one codntion checking.
codntion -> condition
> (e.g sdhci-s3c doesn't use SDHCI_CTRL_HISPD bit, instead using this bit as
> other purpose.)
>
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> drivers/mmc/host/sdhci.c | 19 ++++++++-----------
> 1 file changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 4805566..f854c66 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1619,7 +1619,14 @@ static void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
>
> if ((ios->timing == MMC_TIMING_SD_HS ||
> - ios->timing == MMC_TIMING_MMC_HS)
> + ios->timing == MMC_TIMING_MMC_HS ||
> + ios->timing == MMC_TIMING_MMC_HS400 ||
> + ios->timing == MMC_TIMING_MMC_HS200 ||
> + ios->timing == MMC_TIMING_MMC_DDR52 ||
> + ios->timing == MMC_TIMING_UHS_SDR50 ||
> + ios->timing == MMC_TIMING_UHS_SDR104 ||
> + ios->timing == MMC_TIMING_UHS_DDR50 ||
> + ios->timing == MMC_TIMING_UHS_SDR25)
> && !(host->quirks & SDHCI_QUIRK_NO_HISPD_BIT))
> ctrl |= SDHCI_CTRL_HISPD;
> else
> @@ -1628,16 +1635,6 @@ static void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> if (host->version >= SDHCI_SPEC_300) {
> u16 clk, ctrl_2;
>
> - /* In case of UHS-I modes, set High Speed Enable */
> - if ((ios->timing == MMC_TIMING_MMC_HS400) ||
> - (ios->timing == MMC_TIMING_MMC_HS200) ||
> - (ios->timing == MMC_TIMING_MMC_DDR52) ||
> - (ios->timing == MMC_TIMING_UHS_SDR50) ||
> - (ios->timing == MMC_TIMING_UHS_SDR104) ||
> - (ios->timing == MMC_TIMING_UHS_DDR50) ||
> - (ios->timing == MMC_TIMING_UHS_SDR25))
> - ctrl |= SDHCI_CTRL_HISPD;
> -
> if (!host->preset_enabled) {
> sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
> /*
>
^ permalink raw reply
* Re: [PATCH v2] sdhci-esdhc-imx: Correct two register accesses
From: Adrian Hunter @ 2016-10-11 9:18 UTC (permalink / raw)
To: Aaron Brice, ulf.hansson, aisheng.dong
Cc: linux-mmc, linux-kernel, linux-arm-kernel, Dave Russell
In-Reply-To: <1476124792-18441-1-git-send-email-aaron.brice@datasoft.com>
On 10/10/16 21:39, Aaron Brice wrote:
> - The DMA error interrupt bit is in a different position as
> compared to the sdhci standard. This is accounted for in
> many cases, but not handled in the case of clearing the
> INT_STATUS register by writing a 1 to that location.
> - The HOST_CONTROL register is very different as compared to
> the sdhci standard. This is accounted for in the write
> case, but not when read back out (which it is in the sdhci
> code).
>
> Signed-off-by: Dave Russell <david.russell@datasoft.com>
> Signed-off-by: Aaron Brice <aaron.brice@datasoft.com>
> Acked-by: Dong Aisheng <aisheng.dong@nxp.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
^ permalink raw reply
* Re: [PATCH v5 03/12] mmc: sdhci-msm: add pltfm_data support to get clk-rates from DT
From: Ritesh Harjani @ 2016-10-11 9:06 UTC (permalink / raw)
To: Rob Herring
Cc: Ulf Hansson, linux-mmc@vger.kernel.org, Adrian Hunter, Shawn Lin,
David Brown, Andy Gross, devicetree@vger.kernel.org,
linux-arm-msm, georgi.djakov, alex.lemberg, mateusz.nowak,
Yuliy.Izrailov, asutoshd, David Griego, Sahitya Tummala, venkatg,
Stephen Boyd, Bjorn Andersson, pramod.gurav
In-Reply-To: <CAL_JsqKfBjR9sTqzOMdM-ifDJiL_2RNrjAn3cvezvisLqZu-Wg@mail.gmail.com>
Hi Rob
On 10/11/2016 12:59 AM, Rob Herring wrote:
> On Mon, Oct 10, 2016 at 11:07 AM, Ritesh Harjani <riteshh@codeaurora.org> wrote:
>> Hi Rob,
>>
>> Thanks for review.
>>
>> On 10/10/2016 6:27 PM, Rob Herring wrote:
>>>
>>> On Wed, Oct 05, 2016 at 08:10:31PM +0530, Ritesh Harjani wrote:
>>>>
>>>> This adds support for sdhc-msm controllers to get supported
>>>> clk-rates from DT. sdhci-msm would need it's own set_clock
>>>> ops to be implemented. For this, supported clk-rates needs
>>>> to be populated in sdhci_msm_pltfm_data.
>>>>
>>>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>>>> ---
>>>> .../devicetree/bindings/mmc/sdhci-msm.txt | 1 +
>>>> drivers/mmc/host/sdhci-msm.c | 48
>>>> ++++++++++++++++++++++
>>>> 2 files changed, 49 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>>> b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>>> index 485483a..6a83b38 100644
>>>> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>>> @@ -17,6 +17,7 @@ Required properties:
>>>> "iface" - Main peripheral bus clock (PCLK/HCLK - AHB Bus clock)
>>>> (required)
>>>> "core" - SDC MMC clock (MCLK) (required)
>>>> "bus" - SDCC bus voter clock (optional)
>>>> +- clk-rates: Array of supported GCC clock frequencies for sdhc, Units -
>>>> Hz.
>>>
>>>
>>> Why can't some combination of assigned-clock-rates and querying the
>>> clock provider for rates be used here?
>>
>> From what I understood, assigned-clock-rates would only work for setting
>> some default clock rates for certain clocks by calling
>> of_clk_set_defaults.
>>
>> Whereas the requirement here is -
>> That since SDHC msm directly controls the clk(core clock) at source, it's
>> sdhci-msm driver needs to know the supported clk-rates by the underlying
>> platform to configure the nearest floor value supported on this platform
>> (when the request arrives from the core layer to switch the clock).
>
> Why does clk_round_rate not work for you? That will round down to the
> nearest frequency supported.
clk_round_rate will round off to nearest supported "ceil" frequency.
But we require nearest rounded off "floor" frequency.
>
>> Hence the table of clk-rates is provided for sdhci-msm.
>>
>>>
>>> Minimally this would need unit suffix and either be made common or have
>>> a vendor prefix.
>>
>> Sure will this work in that case - "qcom-clk-rates"
>
> "qcom,clk-rates", but I'm not yet convinced this is right.
>
> Rob
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH v5 11/12] mmc: sdhci-msm: Add calibration tuning for CDCLP533 circuit
From: Ritesh Harjani @ 2016-10-11 9:09 UTC (permalink / raw)
To: Adrian Hunter
Cc: ulf.hansson, linux-mmc, shawn.lin, david.brown, andy.gross,
devicetree, linux-arm-msm, georgi.djakov, alex.lemberg,
mateusz.nowak, Yuliy.Izrailov, asutoshd, david.griego, stummala,
venkatg, sboyd, bjorn.andersson, pramod.gurav
In-Reply-To: <5554e165-4ace-1409-b544-d1cd0a090acf@intel.com>
Hi Adrian,
On 10/11/2016 12:09 PM, Adrian Hunter wrote:
> On 10/10/16 18:42, Ritesh Harjani wrote:
>> Hi Adrian,
>>
>> On 10/10/2016 6:19 PM, Adrian Hunter wrote:
>>> On 05/10/16 17:40, Ritesh Harjani wrote:
>>>> From: Venkat Gopalakrishnan <venkatg@codeaurora.org>
>>>>
>>>> In HS400 mode a new RCLK is introduced on the interface for read data
>>>> transfers. The eMMC5.0 device transmits the read data to the host with
>>>> respect to rising and falling edges of RCLK. In order to ensure correct
>>>> operation of read data transfers in HS400 mode, the incoming RX data
>>>> needs to be sampled by delayed version of RCLK.
>>>>
>>>> The CDCLP533 delay circuit shifts the RCLK by T/4. It needs to be
>>>> initialized, configured and enabled once during HS400 mode switch and
>>>> when operational voltage/clock is changed.
>>>>
>>>> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
>>>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>>>> ---
>>>> drivers/mmc/host/sdhci-msm.c | 178
>>>> +++++++++++++++++++++++++++++++++++++++++++
>>>> 1 file changed, 178 insertions(+)
>>>>
>>>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>>>> index 612fa82..dbf80a9c 100644
>>>> --- a/drivers/mmc/host/sdhci-msm.c
>>>> +++ b/drivers/mmc/host/sdhci-msm.c
>>>> @@ -57,6 +57,7 @@
>>>> #define CORE_DLL_PDN BIT(29)
>>>> #define CORE_DLL_RST BIT(30)
>>>> #define CORE_DLL_CONFIG 0x100
>>>> +#define CORE_CMD_DAT_TRACK_SEL BIT(0)
>>>> #define CORE_DLL_STATUS 0x108
>>>>
>>>> #define CORE_DLL_CONFIG_2 0x1b4
>>>> @@ -72,8 +73,36 @@
>>>> #define CORE_HC_SELECT_IN_HS400 (6 << 19)
>>>> #define CORE_HC_SELECT_IN_MASK (7 << 19)
>>>>
>>>> +#define CORE_CSR_CDC_CTLR_CFG0 0x130
>>>> +#define CORE_SW_TRIG_FULL_CALIB BIT(16)
>>>> +#define CORE_HW_AUTOCAL_ENA BIT(17)
>>>> +
>>>> +#define CORE_CSR_CDC_CTLR_CFG1 0x134
>>>> +#define CORE_CSR_CDC_CAL_TIMER_CFG0 0x138
>>>> +#define CORE_TIMER_ENA BIT(16)
>>>> +
>>>> +#define CORE_CSR_CDC_CAL_TIMER_CFG1 0x13C
>>>> +#define CORE_CSR_CDC_REFCOUNT_CFG 0x140
>>>> +#define CORE_CSR_CDC_COARSE_CAL_CFG 0x144
>>>> +#define CORE_CDC_OFFSET_CFG 0x14C
>>>> +#define CORE_CSR_CDC_DELAY_CFG 0x150
>>>> +#define CORE_CDC_SLAVE_DDA_CFG 0x160
>>>> +#define CORE_CSR_CDC_STATUS0 0x164
>>>> +#define CORE_CALIBRATION_DONE BIT(0)
>>>> +
>>>> +#define CORE_CDC_ERROR_CODE_MASK 0x7000000
>>>> +
>>>> +#define CORE_CSR_CDC_GEN_CFG 0x178
>>>> +#define CORE_CDC_SWITCH_BYPASS_OFF BIT(0)
>>>> +#define CORE_CDC_SWITCH_RC_EN BIT(1)
>>>> +
>>>> +#define CORE_DDR_200_CFG 0x184
>>>> +#define CORE_CDC_T4_DLY_SEL BIT(0)
>>>> +#define CORE_START_CDC_TRAFFIC BIT(6)
>>>> +
>>>> #define CORE_VENDOR_SPEC_CAPABILITIES0 0x11c
>>>>
>>>> +#define INVALID_TUNING_PHASE -1
>>>> #define TCXO_FREQ 19200000
>>>> #define SDHCI_MSM_MIN_CLOCK 400000
>>>> #define CORE_FREQ_100MHZ (100 * 1000 * 1000)
>>>> @@ -97,6 +126,7 @@ struct sdhci_msm_host {
>>>> bool use_14lpp_dll_reset;
>>>> bool tuning_done;
>>>> bool calibration_done;
>>>> + u8 saved_tuning_phase;
>>>> };
>>>>
>>>> /* Platform specific tuning */
>>>> @@ -426,6 +456,136 @@ static int msm_init_cm_dll(struct sdhci_host *host)
>>>> return 0;
>>>> }
>>>>
>>>> +static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
>>>> +{
>>>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>>> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>>>> + u32 wait_cnt, config;
>>>> + int ret;
>>>> +
>>>> + pr_debug("%s: Enter %s\n", mmc_hostname(host->mmc), __func__);
>>>> +
>>>> + /*
>>>> + * Retuning in HS400 (DDR mode) will fail, just reset the
>>>> + * tuning block and restore the saved tuning phase.
>>>> + */
>>>> + ret = msm_init_cm_dll(host);
>>>> + if (ret)
>>>> + goto out;
>>>> +
>>>> + /* Set the selected phase in delay line hw block */
>>>> + ret = msm_config_cm_dll_phase(host, msm_host->saved_tuning_phase);
>>>> + if (ret)
>>>> + goto out;
>>>> +
>>>> + /* Write 1 to CMD_DAT_TRACK_SEL field in DLL_CONFIG */
>>>> + config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
>>>> + config |= CORE_CMD_DAT_TRACK_SEL;
>>>> + writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
>>>> +
>>>> + /* Write 0 to CDC_T4_DLY_SEL field in VENDOR_SPEC_DDR200_CFG */
>>>> + config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
>>>> + config &= ~CORE_CDC_T4_DLY_SEL;
>>>> + writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
>>>> +
>>>> + /* Write 0 to CDC_SWITCH_BYPASS_OFF field in CORE_CSR_CDC_GEN_CFG */
>>>> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_GEN_CFG);
>>>> + config &= ~CORE_CDC_SWITCH_BYPASS_OFF;
>>>> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_GEN_CFG);
>>>> +
>>>> + /* Write 1 to CDC_SWITCH_RC_EN field in CORE_CSR_CDC_GEN_CFG */
>>>> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_GEN_CFG);
>>>> + config |= CORE_CDC_SWITCH_RC_EN;
>>>> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_GEN_CFG);
>>>> +
>>>> + /* Write 0 to START_CDC_TRAFFIC field in CORE_DDR200_CFG */
>>>> + config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
>>>> + config &= ~CORE_START_CDC_TRAFFIC;
>>>> + writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
>>>> +
>>>> + /*
>>>> + * Perform CDC Register Initialization Sequence
>>>> + *
>>>> + * CORE_CSR_CDC_CTLR_CFG0 0x11800EC
>>>> + * CORE_CSR_CDC_CTLR_CFG1 0x3011111
>>>> + * CORE_CSR_CDC_CAL_TIMER_CFG0 0x1201000
>>>> + * CORE_CSR_CDC_CAL_TIMER_CFG1 0x4
>>>> + * CORE_CSR_CDC_REFCOUNT_CFG 0xCB732020
>>>> + * CORE_CSR_CDC_COARSE_CAL_CFG 0xB19
>>>> + * CORE_CSR_CDC_DELAY_CFG 0x3AC
>>>> + * CORE_CDC_OFFSET_CFG 0x0
>>>> + * CORE_CDC_SLAVE_DDA_CFG 0x16334
>>>> + */
>>>> +
>>>> + writel_relaxed(0x11800EC, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>>>> + writel_relaxed(0x3011111, host->ioaddr + CORE_CSR_CDC_CTLR_CFG1);
>>>> + writel_relaxed(0x1201000, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
>>>> + writel_relaxed(0x4, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG1);
>>>> + writel_relaxed(0xCB732020, host->ioaddr + CORE_CSR_CDC_REFCOUNT_CFG);
>>>> + writel_relaxed(0xB19, host->ioaddr + CORE_CSR_CDC_COARSE_CAL_CFG);
>>>> + writel_relaxed(0x3AC, host->ioaddr + CORE_CSR_CDC_DELAY_CFG);
>>>> + writel_relaxed(0x0, host->ioaddr + CORE_CDC_OFFSET_CFG);
>>>> + writel_relaxed(0x16334, host->ioaddr + CORE_CDC_SLAVE_DDA_CFG);
>>>> +
>>>> + /* CDC HW Calibration */
>>>> +
>>>> + /* Write 1 to SW_TRIG_FULL_CALIB field in CORE_CSR_CDC_CTLR_CFG0 */
>>>> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>>>> + config |= CORE_SW_TRIG_FULL_CALIB;
>>>> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>>>> +
>>>> + /* Write 0 to SW_TRIG_FULL_CALIB field in CORE_CSR_CDC_CTLR_CFG0 */
>>>> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>>>> + config &= ~CORE_SW_TRIG_FULL_CALIB;
>>>> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>>>> +
>>>> + /* Write 1 to HW_AUTOCAL_ENA field in CORE_CSR_CDC_CTLR_CFG0 */
>>>> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>>>> + config |= CORE_HW_AUTOCAL_ENA;
>>>> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>>>> +
>>>> + /* Write 1 to TIMER_ENA field in CORE_CSR_CDC_CAL_TIMER_CFG0 */
>>>> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
>>>> + config |= CORE_TIMER_ENA;
>>>> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
>>>> +
>>>> + wmb(); /* drain writebuffer */
>>>> +
>>>> + /* Poll on CALIBRATION_DONE field in CORE_CSR_CDC_STATUS0 to be 1 */
>>>> + wait_cnt = 50;
>>>> + while (!(readl_relaxed(host->ioaddr + CORE_CSR_CDC_STATUS0)
>>>> + & CORE_CALIBRATION_DONE)) {
>>>> + /* max. wait for 50us sec for CALIBRATION_DONE bit to be set */
>>>> + if (--wait_cnt == 0) {
>>>> + pr_err("%s: %s: CDC Calibration was not completed\n",
>>>> + mmc_hostname(host->mmc), __func__);
>>>> + ret = -ETIMEDOUT;
>>>> + goto out;
>>>> + }
>>>> + /* wait for 1us before polling again */
>>>> + udelay(1);
>>>> + }
>>>> +
>>>> + /* Verify CDC_ERROR_CODE field in CORE_CSR_CDC_STATUS0 is 0 */
>>>> + ret = readl_relaxed(host->ioaddr + CORE_CSR_CDC_STATUS0)
>>>> + & CORE_CDC_ERROR_CODE_MASK;
>>>> + if (ret) {
>>>> + pr_err("%s: %s: CDC Error Code %d\n",
>>>> + mmc_hostname(host->mmc), __func__, ret);
>>>> + ret = -EINVAL;
>>>> + goto out;
>>>> + }
>>>> +
>>>> + /* Write 1 to START_CDC_TRAFFIC field in CORE_DDR200_CFG */
>>>> + config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
>>>> + config |= CORE_START_CDC_TRAFFIC;
>>>> + writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
>>>> +out:
>>>> + pr_debug("%s: Exit %s, ret:%d\n", mmc_hostname(host->mmc),
>>>> + __func__, ret);
>>>> + return ret;
>>>> +}
>>>> +
>>>> static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
>>>> {
>>>> int tuning_seq_cnt = 3;
>>>> @@ -433,6 +593,8 @@ static int sdhci_msm_execute_tuning(struct sdhci_host
>>>> *host, u32 opcode)
>>>> int rc;
>>>> struct mmc_host *mmc = host->mmc;
>>>> struct mmc_ios ios = host->mmc->ios;
>>>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>>> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>>>>
>>>> /*
>>>> * Tuning is required for SDR104, HS200 and HS400 cards and
>>>> @@ -457,6 +619,7 @@ retry:
>>>> if (rc)
>>>> return rc;
>>>>
>>>> + msm_host->saved_tuning_phase = phase;
>>>> rc = mmc_send_tuning(mmc, opcode, NULL);
>>>> if (!rc) {
>>>> /* Tuning is successful at this tuning point */
>>>> @@ -492,6 +655,8 @@ retry:
>>>> rc = -EIO;
>>>> }
>>>>
>>>> + if (!rc)
>>>> + msm_host->tuning_done = true;
>>>> return rc;
>>>> }
>>>>
>>>> @@ -565,6 +730,17 @@ static void sdhci_msm_set_uhs_signaling(struct
>>>> sdhci_host *host,
>>>> dev_dbg(mmc_dev(mmc), "%s: clock=%u uhs=%u ctrl_2=0x%x\n",
>>>> mmc_hostname(host->mmc), host->clock, uhs, ctrl_2);
>>>> sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
>>>> +
>>>> + spin_unlock_irq(&host->lock);
>>>> +
>>>> + /* CDCLP533 HW calibration is only required for HS400 mode*/
>>>> + if (host->clock > CORE_FREQ_100MHZ &&
>>>> + msm_host->tuning_done && !msm_host->calibration_done &&
>>>> + (mmc->ios.timing == MMC_TIMING_MMC_HS400))
>>>> + if (!sdhci_msm_cdclp533_calibration(host))
>>>> + msm_host->calibration_done = true;
>>>> +
>>>> + spin_lock_irq(&host->lock);
>>>> }
>>>>
>>>> static void sdhci_msm_voltage_switch(struct sdhci_host *host)
>>>> @@ -907,6 +1083,8 @@ static int sdhci_msm_probe(struct platform_device
>>>> *pdev)
>>>>
>>>> sdhci_msm_populate_dt(&pdev->dev, msm_host);
>>>>
>>>> + msm_host->saved_tuning_phase = INVALID_TUNING_PHASE;
>>>
>>> There is never a check for INVALID_TUNING_PHASE which begs the question: why
>>> have it?
>> phase value can be between 0x0 to 0xf. So during probe saved_tuning_phase is
>> getting initialized with -1 value.
>>
>> Let me know if any concern, we can remove it as well.
>> But wont it look incorrect if we initialize it with some valid phase value
>> before even tuning is completed?
>
> INVALID_TUNING_PHASE is fine, I would just expect it be checked e.g. in
> msm_config_cm_dll_phase
>
> if (phase > 0xf)
> return -EINVAL;
Sure will add these checks.
>
>>
>>>
>>>> +
>>>> /* Setup SDCC bus voter clock. */
>>>> msm_host->bus_clk = devm_clk_get(&pdev->dev, "bus");
>>>> if (!IS_ERR(msm_host->bus_clk)) {
>>>>
>>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH v5 11/12] mmc: sdhci-msm: Add calibration tuning for CDCLP533 circuit
From: Adrian Hunter @ 2016-10-11 6:39 UTC (permalink / raw)
To: Ritesh Harjani
Cc: ulf.hansson-QSEj5FYQhm4dnm+yROfE0A,
linux-mmc-u79uwXL29TY76Z2rM5mHXA,
shawn.lin-TNX95d0MmH7DzftRWevZcw,
david.brown-QSEj5FYQhm4dnm+yROfE0A,
andy.gross-QSEj5FYQhm4dnm+yROfE0A,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
georgi.djakov-QSEj5FYQhm4dnm+yROfE0A,
alex.lemberg-XdAiOPVOjttBDgjK7y7TUQ,
mateusz.nowak-ral2JQCrhuEAvxtiuMwx3w,
Yuliy.Izrailov-XdAiOPVOjttBDgjK7y7TUQ,
asutoshd-sgV2jX0FEOL9JmXXK+q4OQ,
david.griego-QSEj5FYQhm4dnm+yROfE0A,
stummala-sgV2jX0FEOL9JmXXK+q4OQ, venkatg-sgV2jX0FEOL9JmXXK+q4OQ,
sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A,
pramod.gurav-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <6993d3a2-7961-2507-60d2-153c14e0bc17-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On 10/10/16 18:42, Ritesh Harjani wrote:
> Hi Adrian,
>
> On 10/10/2016 6:19 PM, Adrian Hunter wrote:
>> On 05/10/16 17:40, Ritesh Harjani wrote:
>>> From: Venkat Gopalakrishnan <venkatg-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>>
>>> In HS400 mode a new RCLK is introduced on the interface for read data
>>> transfers. The eMMC5.0 device transmits the read data to the host with
>>> respect to rising and falling edges of RCLK. In order to ensure correct
>>> operation of read data transfers in HS400 mode, the incoming RX data
>>> needs to be sampled by delayed version of RCLK.
>>>
>>> The CDCLP533 delay circuit shifts the RCLK by T/4. It needs to be
>>> initialized, configured and enabled once during HS400 mode switch and
>>> when operational voltage/clock is changed.
>>>
>>> Signed-off-by: Venkat Gopalakrishnan <venkatg-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>> Signed-off-by: Ritesh Harjani <riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>> ---
>>> drivers/mmc/host/sdhci-msm.c | 178
>>> +++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 178 insertions(+)
>>>
>>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>>> index 612fa82..dbf80a9c 100644
>>> --- a/drivers/mmc/host/sdhci-msm.c
>>> +++ b/drivers/mmc/host/sdhci-msm.c
>>> @@ -57,6 +57,7 @@
>>> #define CORE_DLL_PDN BIT(29)
>>> #define CORE_DLL_RST BIT(30)
>>> #define CORE_DLL_CONFIG 0x100
>>> +#define CORE_CMD_DAT_TRACK_SEL BIT(0)
>>> #define CORE_DLL_STATUS 0x108
>>>
>>> #define CORE_DLL_CONFIG_2 0x1b4
>>> @@ -72,8 +73,36 @@
>>> #define CORE_HC_SELECT_IN_HS400 (6 << 19)
>>> #define CORE_HC_SELECT_IN_MASK (7 << 19)
>>>
>>> +#define CORE_CSR_CDC_CTLR_CFG0 0x130
>>> +#define CORE_SW_TRIG_FULL_CALIB BIT(16)
>>> +#define CORE_HW_AUTOCAL_ENA BIT(17)
>>> +
>>> +#define CORE_CSR_CDC_CTLR_CFG1 0x134
>>> +#define CORE_CSR_CDC_CAL_TIMER_CFG0 0x138
>>> +#define CORE_TIMER_ENA BIT(16)
>>> +
>>> +#define CORE_CSR_CDC_CAL_TIMER_CFG1 0x13C
>>> +#define CORE_CSR_CDC_REFCOUNT_CFG 0x140
>>> +#define CORE_CSR_CDC_COARSE_CAL_CFG 0x144
>>> +#define CORE_CDC_OFFSET_CFG 0x14C
>>> +#define CORE_CSR_CDC_DELAY_CFG 0x150
>>> +#define CORE_CDC_SLAVE_DDA_CFG 0x160
>>> +#define CORE_CSR_CDC_STATUS0 0x164
>>> +#define CORE_CALIBRATION_DONE BIT(0)
>>> +
>>> +#define CORE_CDC_ERROR_CODE_MASK 0x7000000
>>> +
>>> +#define CORE_CSR_CDC_GEN_CFG 0x178
>>> +#define CORE_CDC_SWITCH_BYPASS_OFF BIT(0)
>>> +#define CORE_CDC_SWITCH_RC_EN BIT(1)
>>> +
>>> +#define CORE_DDR_200_CFG 0x184
>>> +#define CORE_CDC_T4_DLY_SEL BIT(0)
>>> +#define CORE_START_CDC_TRAFFIC BIT(6)
>>> +
>>> #define CORE_VENDOR_SPEC_CAPABILITIES0 0x11c
>>>
>>> +#define INVALID_TUNING_PHASE -1
>>> #define TCXO_FREQ 19200000
>>> #define SDHCI_MSM_MIN_CLOCK 400000
>>> #define CORE_FREQ_100MHZ (100 * 1000 * 1000)
>>> @@ -97,6 +126,7 @@ struct sdhci_msm_host {
>>> bool use_14lpp_dll_reset;
>>> bool tuning_done;
>>> bool calibration_done;
>>> + u8 saved_tuning_phase;
>>> };
>>>
>>> /* Platform specific tuning */
>>> @@ -426,6 +456,136 @@ static int msm_init_cm_dll(struct sdhci_host *host)
>>> return 0;
>>> }
>>>
>>> +static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
>>> +{
>>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>>> + u32 wait_cnt, config;
>>> + int ret;
>>> +
>>> + pr_debug("%s: Enter %s\n", mmc_hostname(host->mmc), __func__);
>>> +
>>> + /*
>>> + * Retuning in HS400 (DDR mode) will fail, just reset the
>>> + * tuning block and restore the saved tuning phase.
>>> + */
>>> + ret = msm_init_cm_dll(host);
>>> + if (ret)
>>> + goto out;
>>> +
>>> + /* Set the selected phase in delay line hw block */
>>> + ret = msm_config_cm_dll_phase(host, msm_host->saved_tuning_phase);
>>> + if (ret)
>>> + goto out;
>>> +
>>> + /* Write 1 to CMD_DAT_TRACK_SEL field in DLL_CONFIG */
>>> + config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
>>> + config |= CORE_CMD_DAT_TRACK_SEL;
>>> + writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
>>> +
>>> + /* Write 0 to CDC_T4_DLY_SEL field in VENDOR_SPEC_DDR200_CFG */
>>> + config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
>>> + config &= ~CORE_CDC_T4_DLY_SEL;
>>> + writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
>>> +
>>> + /* Write 0 to CDC_SWITCH_BYPASS_OFF field in CORE_CSR_CDC_GEN_CFG */
>>> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_GEN_CFG);
>>> + config &= ~CORE_CDC_SWITCH_BYPASS_OFF;
>>> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_GEN_CFG);
>>> +
>>> + /* Write 1 to CDC_SWITCH_RC_EN field in CORE_CSR_CDC_GEN_CFG */
>>> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_GEN_CFG);
>>> + config |= CORE_CDC_SWITCH_RC_EN;
>>> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_GEN_CFG);
>>> +
>>> + /* Write 0 to START_CDC_TRAFFIC field in CORE_DDR200_CFG */
>>> + config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
>>> + config &= ~CORE_START_CDC_TRAFFIC;
>>> + writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
>>> +
>>> + /*
>>> + * Perform CDC Register Initialization Sequence
>>> + *
>>> + * CORE_CSR_CDC_CTLR_CFG0 0x11800EC
>>> + * CORE_CSR_CDC_CTLR_CFG1 0x3011111
>>> + * CORE_CSR_CDC_CAL_TIMER_CFG0 0x1201000
>>> + * CORE_CSR_CDC_CAL_TIMER_CFG1 0x4
>>> + * CORE_CSR_CDC_REFCOUNT_CFG 0xCB732020
>>> + * CORE_CSR_CDC_COARSE_CAL_CFG 0xB19
>>> + * CORE_CSR_CDC_DELAY_CFG 0x3AC
>>> + * CORE_CDC_OFFSET_CFG 0x0
>>> + * CORE_CDC_SLAVE_DDA_CFG 0x16334
>>> + */
>>> +
>>> + writel_relaxed(0x11800EC, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>>> + writel_relaxed(0x3011111, host->ioaddr + CORE_CSR_CDC_CTLR_CFG1);
>>> + writel_relaxed(0x1201000, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
>>> + writel_relaxed(0x4, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG1);
>>> + writel_relaxed(0xCB732020, host->ioaddr + CORE_CSR_CDC_REFCOUNT_CFG);
>>> + writel_relaxed(0xB19, host->ioaddr + CORE_CSR_CDC_COARSE_CAL_CFG);
>>> + writel_relaxed(0x3AC, host->ioaddr + CORE_CSR_CDC_DELAY_CFG);
>>> + writel_relaxed(0x0, host->ioaddr + CORE_CDC_OFFSET_CFG);
>>> + writel_relaxed(0x16334, host->ioaddr + CORE_CDC_SLAVE_DDA_CFG);
>>> +
>>> + /* CDC HW Calibration */
>>> +
>>> + /* Write 1 to SW_TRIG_FULL_CALIB field in CORE_CSR_CDC_CTLR_CFG0 */
>>> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>>> + config |= CORE_SW_TRIG_FULL_CALIB;
>>> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>>> +
>>> + /* Write 0 to SW_TRIG_FULL_CALIB field in CORE_CSR_CDC_CTLR_CFG0 */
>>> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>>> + config &= ~CORE_SW_TRIG_FULL_CALIB;
>>> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>>> +
>>> + /* Write 1 to HW_AUTOCAL_ENA field in CORE_CSR_CDC_CTLR_CFG0 */
>>> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>>> + config |= CORE_HW_AUTOCAL_ENA;
>>> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>>> +
>>> + /* Write 1 to TIMER_ENA field in CORE_CSR_CDC_CAL_TIMER_CFG0 */
>>> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
>>> + config |= CORE_TIMER_ENA;
>>> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
>>> +
>>> + wmb(); /* drain writebuffer */
>>> +
>>> + /* Poll on CALIBRATION_DONE field in CORE_CSR_CDC_STATUS0 to be 1 */
>>> + wait_cnt = 50;
>>> + while (!(readl_relaxed(host->ioaddr + CORE_CSR_CDC_STATUS0)
>>> + & CORE_CALIBRATION_DONE)) {
>>> + /* max. wait for 50us sec for CALIBRATION_DONE bit to be set */
>>> + if (--wait_cnt == 0) {
>>> + pr_err("%s: %s: CDC Calibration was not completed\n",
>>> + mmc_hostname(host->mmc), __func__);
>>> + ret = -ETIMEDOUT;
>>> + goto out;
>>> + }
>>> + /* wait for 1us before polling again */
>>> + udelay(1);
>>> + }
>>> +
>>> + /* Verify CDC_ERROR_CODE field in CORE_CSR_CDC_STATUS0 is 0 */
>>> + ret = readl_relaxed(host->ioaddr + CORE_CSR_CDC_STATUS0)
>>> + & CORE_CDC_ERROR_CODE_MASK;
>>> + if (ret) {
>>> + pr_err("%s: %s: CDC Error Code %d\n",
>>> + mmc_hostname(host->mmc), __func__, ret);
>>> + ret = -EINVAL;
>>> + goto out;
>>> + }
>>> +
>>> + /* Write 1 to START_CDC_TRAFFIC field in CORE_DDR200_CFG */
>>> + config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
>>> + config |= CORE_START_CDC_TRAFFIC;
>>> + writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
>>> +out:
>>> + pr_debug("%s: Exit %s, ret:%d\n", mmc_hostname(host->mmc),
>>> + __func__, ret);
>>> + return ret;
>>> +}
>>> +
>>> static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
>>> {
>>> int tuning_seq_cnt = 3;
>>> @@ -433,6 +593,8 @@ static int sdhci_msm_execute_tuning(struct sdhci_host
>>> *host, u32 opcode)
>>> int rc;
>>> struct mmc_host *mmc = host->mmc;
>>> struct mmc_ios ios = host->mmc->ios;
>>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>>>
>>> /*
>>> * Tuning is required for SDR104, HS200 and HS400 cards and
>>> @@ -457,6 +619,7 @@ retry:
>>> if (rc)
>>> return rc;
>>>
>>> + msm_host->saved_tuning_phase = phase;
>>> rc = mmc_send_tuning(mmc, opcode, NULL);
>>> if (!rc) {
>>> /* Tuning is successful at this tuning point */
>>> @@ -492,6 +655,8 @@ retry:
>>> rc = -EIO;
>>> }
>>>
>>> + if (!rc)
>>> + msm_host->tuning_done = true;
>>> return rc;
>>> }
>>>
>>> @@ -565,6 +730,17 @@ static void sdhci_msm_set_uhs_signaling(struct
>>> sdhci_host *host,
>>> dev_dbg(mmc_dev(mmc), "%s: clock=%u uhs=%u ctrl_2=0x%x\n",
>>> mmc_hostname(host->mmc), host->clock, uhs, ctrl_2);
>>> sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
>>> +
>>> + spin_unlock_irq(&host->lock);
>>> +
>>> + /* CDCLP533 HW calibration is only required for HS400 mode*/
>>> + if (host->clock > CORE_FREQ_100MHZ &&
>>> + msm_host->tuning_done && !msm_host->calibration_done &&
>>> + (mmc->ios.timing == MMC_TIMING_MMC_HS400))
>>> + if (!sdhci_msm_cdclp533_calibration(host))
>>> + msm_host->calibration_done = true;
>>> +
>>> + spin_lock_irq(&host->lock);
>>> }
>>>
>>> static void sdhci_msm_voltage_switch(struct sdhci_host *host)
>>> @@ -907,6 +1083,8 @@ static int sdhci_msm_probe(struct platform_device
>>> *pdev)
>>>
>>> sdhci_msm_populate_dt(&pdev->dev, msm_host);
>>>
>>> + msm_host->saved_tuning_phase = INVALID_TUNING_PHASE;
>>
>> There is never a check for INVALID_TUNING_PHASE which begs the question: why
>> have it?
> phase value can be between 0x0 to 0xf. So during probe saved_tuning_phase is
> getting initialized with -1 value.
>
> Let me know if any concern, we can remove it as well.
> But wont it look incorrect if we initialize it with some valid phase value
> before even tuning is completed?
INVALID_TUNING_PHASE is fine, I would just expect it be checked e.g. in
msm_config_cm_dll_phase
if (phase > 0xf)
return -EINVAL;
>
>>
>>> +
>>> /* Setup SDCC bus voter clock. */
>>> msm_host->bus_clk = devm_clk_get(&pdev->dev, "bus");
>>> if (!IS_ERR(msm_host->bus_clk)) {
>>>
>>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Rob Herring @ 2016-10-10 21:34 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Ulf Hansson, Adrian Hunter, linux-mmc, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, devicetree, Thomas Petazzoni,
linux-arm-kernel, Ziji Hu, Jack(SH) Zhu, Jimmy Xu, Jisheng Zhang,
Nadav Haklai, Ryan Gao, Doug Jones, Shiwu Zhang, Victor Gu,
Wei(SOCP) Liu, Wilson Ding, Xueping Liu, Hilbert
In-Reply-To: <fc0ca2b3140287cbccf5c176a12b7dce25d06a47.1475853198.git-series.gregory.clement@free-electrons.com>
On Fri, Oct 07, 2016 at 05:22:51PM +0200, Gregory CLEMENT wrote:
> From: Ziji Hu <huziji@marvell.com>
>
> Marvell Xenon SDHC can support eMMC/SD/SDIO.
> Add Xenon-specific properties.
> Also add properties for Xenon PHY setting.
>
> Signed-off-by: Hu Ziji <huziji@marvell.com>
> Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
> Documentation/devicetree/bindings/mmc/marvell,sdhci-xenon.txt | 164 +++++++-
> MAINTAINERS | 1 +-
> 2 files changed, 165 insertions(+), 0 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/mmc/marvell,sdhci-xenon.txt
>
> diff --git a/Documentation/devicetree/bindings/mmc/marvell,sdhci-xenon.txt b/Documentation/devicetree/bindings/mmc/marvell,sdhci-xenon.txt
> new file mode 100644
> index 000000000000..8b25ad28ebbd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/marvell,sdhci-xenon.txt
> @@ -0,0 +1,164 @@
> +Marvell's Xenon SDHCI Controller device tree bindings
> +This file documents differences between the core mmc properties
> +described by mmc.txt and the properties used by the Xenon implementation.
> +
> +A single Xenon IP can support multiple slots.
> +Each slot acts as an independent SDHC. It owns independent resources, such
> +as register sets clock and PHY.
Is the phy really part of the same block?
> +Each slot should have an independent device tree node.
> +
> +Required Properties:
> +- compatible: should be "marvell,sdhci-xenon" or "marvell,armada-3700-sdhci".
Perhaps some consistent ordering (w/ -sdhci on the end).
> +
> +- Input Clock Name
Your formatting of properties is a bit strange. Please restructure like
most bindings so the property names are before all the description.
> + Some SOCs require additional clock for AXI bus.
Those SoCs should have a specific compatible string and you need to
define which compatible strings have 2 clocks vs. 1 clock.
> + The input clock for Xenon IP core should be named as "core".
> + The optional AXI clock should be named as "axi".
> + - clocks = <&core_clk>, <&axi_clock>;
> + - clock-names = "core", "axi";
> +
> +- Register Set Size
Is this a property name?
> + Different Xenon SDHC release has different register set size.
> + The specific size should also refer to the SOC implementation.
> +
> +Optional Properties:
> +- Slot Index
> + A single Xenon IP can support multiple slots.
> + During initialization, each slot should set corresponding setting bit in
> + some Xenon-specific registers. The corresponding bit is determined by
> + this property.
> + - xenon,slotno = <slot_index>;
Slots should probably be represented as child nodes with the reg
property being the slot number.
Also, xenon is not a vendor prefix.
> + If this property is not provided, Xenon IP should contain only one slot
> + and the slot index will be 0x0 by default.
> +
> +- PHY Type
You're going to need to come of with a common binding for this.
> + Xenon support mutilple types of PHYs.
> + To select eMMC 5.1 PHY, set:
> + - xenon,phy-type = "emmc 5.1 phy"
> + eMMC 5.1 PHY is the default choice if this property is not provided.
> + To select eMMC 5.0 PHY, set:
> + - xenon,phy-type = "emmc 5.0 phy"
> + To select SDH PHY, set:
> + - xenon,phy-type = "sdh phy"
> + Please note that eMMC PHY is a general PHY for eMMC/SD/SDIO, other than for
> + eMMC only.
> +
> +- Customized eMMC PHY Parameters
> + Some boards require different values of some specific eMMC PHY parameters.
> + Some SOCs also require specific workaround to set eMMC PHY.
> + These properties enable diverse boards to customize the eMMC PHY.
> + The supported eMMC PHY parameters are listed in below. All those properties
> + are only available for eMMC PHY 5.1 and eMMC PHY 5.0.
> + ZNR
> + valid range = [0:0x1F].
> + ZNR is set as 0xF by default if this property is not provided.
> + - xenon,phy-znr = <value>;
> +
> + ZPR
> + valid range = [0:0x1F].
> + ZPR is set as 0xF by default if this property is not provided.
> + - xenon,phy-zpr = <value>;
marvell is the vendor prefix.
> +
> + Number of successful tuning times
> + Set the number of required consecutive successful sampling points used to
> + identify a valid sampling window, in tuning process.
> + Valid range = [1:7]. Set as 0x4 by default if this property is not provided.
> + - xenon,phy-nr-tun-times = <nr_times>;
> +
> + Divider for TUN_STEP
> + Set the divider for calculating TUN_STEP.
> + Set as 64 by default if this property is not provided.
> + - xenon,phy-tun-step-divider = <divider>;
> +
> + Force PHY into slow mode.
> + Only available when bus frequency lower than 50MHz in SDR mde.
> + Disabled by default. Please do not enable it unless it is necessary.
> + - xenon,phy-slow-mode;
> +
> +- Mask Conflict Error Report
> + Disable Conflict Error alert on some SOC. Disabled by default.
> + xenon,mask-conflict-err;
> +
> +- Re-tuning Counter
> + Xenon SDHC SOC usually doesn't provide re-tuning counter in
> + Capabilities Register 3 Bit[11:8].
> + This property provides the re-tuning counter.
> + xenon,tuning-count = <count>;
> + If this property is not set, default re-tuning counter will
> + be set as 0x9 in driver.
> +
> +- SOC PHY PAD Voltage Control register
> + Some SOCs have SOC PHY PAD Voltage Control register outside Xenon IP.
> + This register sets SOC PHY PAD Voltage to keep aligh with Vccq.
> + Two properties provide information of this control register.
> + These two properties are only valid when "marvell,armada-3700-sdhci"
> + is selected. Both of them must be provided when "marvell,armada-3700-sdhci"
> + is selected.
> + - xenon,pad-type
> + Two types: "sd" and "fixed-1-8v".
> + If "sd" is slected, SOC PHY PAD is set as 3.3V at the beginning and is
> + switched to 1.8V when SD in UHS-I.
> + If "fixed-1-8v" is slected, SOC PHY PAD is fixed 1.8V, such as for eMMC.
You should be able to existing, common properties for i/o voltage
capabilities/constraints.
> + - reg
> + Physical address and size of SOC PHY PAD register.
> + Append after Xenon SDHC register space, as a second register field.
> +
> + Please follow the examples with compatible "marvell,armada-3700-sdhci"
> + in below.
> +
> +Example:
> +- For eMMC slot:
> +
> + sdhci@aa0000 {
> + compatible = "marvell,sdhci-xenon";
> + reg = <0xaa0000 0x1000>;
> + interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>
> + clocks = <&emmcclk>;
> + clock-names = "core";
> + xenon,slotno = <0>;
> + xenon,phy-type = "emmc 5.1 phy";
> + bus-width = <8>;
> + tuning-count = <11>;
> + };
> +
> +- For SD/SDIO slot:
> +
> + sdhci@ab0000 {
> + compatible = "marvell,sdhci-xenon";
> + reg = <0xab0000 0x1000>;
> + interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>
> + vqmmc-supply = <&sd_regulator>;
> + clocks = <&sdclk>;
> + clock-names = "core";
> + bus-width = <4>;
> + tuning-count = <9>;
> + };
> +
> +- For eMMC slot with compatible "marvell,armada-3700-sdhci":
> +
> + sdhci@aa0000 {
> + compatible = "marvell,armada-3700-sdhci";
> + reg = <0xaa0000 0x1000>,
> + <phy_addr 0x4>;
> + interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>
> + clocks = <&emmcclk>;
> + clock-names = "core";
> + bus-width = <8>;
> +
> + xenon,pad-type = "fixed-1-8v";
> + };
> +
> +- For SD/SDIO slot with compatible "marvell,armada-3700-sdhci":
> +
> + sdhci@ab0000 {
> + compatible = "marvell,armada-3700-sdhci";
> + reg = <0xab0000 0x1000>,
> + <phy_addr 0x4>;
> + interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>
> + vqmmc-supply = <&sd_regulator>;
> + clocks = <&sdclk>;
> + clock-names = "core";
> + bus-width = <4>;
> +
> + xenon,pad-type = "sd";
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 89adcd57aa25..4aa0eac9bfc7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7582,6 +7582,7 @@ MARVELL XENON MMC/SD/SDIO HOST CONTROLLER DRIVER
> M: Ziji Hu <huziji@marvell.com>
> L: linux-mmc@vger.kernel.org
> S: Supported
> +F: Documentation/devicetree/bindings/mmc/marvell,sdhci-xenon.txt
>
> MATROX FRAMEBUFFER DRIVER
> L: linux-fbdev@vger.kernel.org
> --
> git-series 0.8.10
^ permalink raw reply
* Re: [PATCH v5 03/12] mmc: sdhci-msm: add pltfm_data support to get clk-rates from DT
From: Rob Herring @ 2016-10-10 19:29 UTC (permalink / raw)
To: Ritesh Harjani
Cc: Ulf Hansson, linux-mmc@vger.kernel.org, Adrian Hunter, Shawn Lin,
David Brown, Andy Gross, devicetree@vger.kernel.org,
linux-arm-msm, georgi.djakov, alex.lemberg, mateusz.nowak,
Yuliy.Izrailov, asutoshd, David Griego, Sahitya Tummala, venkatg,
Stephen Boyd, Bjorn Andersson, pramod.gurav
In-Reply-To: <1a7f9c09-70a6-da2a-ca84-78a0331e3b4d@codeaurora.org>
On Mon, Oct 10, 2016 at 11:07 AM, Ritesh Harjani <riteshh@codeaurora.org> wrote:
> Hi Rob,
>
> Thanks for review.
>
> On 10/10/2016 6:27 PM, Rob Herring wrote:
>>
>> On Wed, Oct 05, 2016 at 08:10:31PM +0530, Ritesh Harjani wrote:
>>>
>>> This adds support for sdhc-msm controllers to get supported
>>> clk-rates from DT. sdhci-msm would need it's own set_clock
>>> ops to be implemented. For this, supported clk-rates needs
>>> to be populated in sdhci_msm_pltfm_data.
>>>
>>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>>> ---
>>> .../devicetree/bindings/mmc/sdhci-msm.txt | 1 +
>>> drivers/mmc/host/sdhci-msm.c | 48
>>> ++++++++++++++++++++++
>>> 2 files changed, 49 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>> b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>> index 485483a..6a83b38 100644
>>> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>>> @@ -17,6 +17,7 @@ Required properties:
>>> "iface" - Main peripheral bus clock (PCLK/HCLK - AHB Bus clock)
>>> (required)
>>> "core" - SDC MMC clock (MCLK) (required)
>>> "bus" - SDCC bus voter clock (optional)
>>> +- clk-rates: Array of supported GCC clock frequencies for sdhc, Units -
>>> Hz.
>>
>>
>> Why can't some combination of assigned-clock-rates and querying the
>> clock provider for rates be used here?
>
> From what I understood, assigned-clock-rates would only work for setting
> some default clock rates for certain clocks by calling
> of_clk_set_defaults.
>
> Whereas the requirement here is -
> That since SDHC msm directly controls the clk(core clock) at source, it's
> sdhci-msm driver needs to know the supported clk-rates by the underlying
> platform to configure the nearest floor value supported on this platform
> (when the request arrives from the core layer to switch the clock).
Why does clk_round_rate not work for you? That will round down to the
nearest frequency supported.
> Hence the table of clk-rates is provided for sdhci-msm.
>
>>
>> Minimally this would need unit suffix and either be made common or have
>> a vendor prefix.
>
> Sure will this work in that case - "qcom-clk-rates"
"qcom,clk-rates", but I'm not yet convinced this is right.
Rob
^ permalink raw reply
* [PATCH v2] sdhci-esdhc-imx: Correct two register accesses
From: Aaron Brice @ 2016-10-10 18:39 UTC (permalink / raw)
To: adrian.hunter, ulf.hansson, aisheng.dong
Cc: linux-mmc, linux-kernel, linux-arm-kernel, Dave Russell
- The DMA error interrupt bit is in a different position as
compared to the sdhci standard. This is accounted for in
many cases, but not handled in the case of clearing the
INT_STATUS register by writing a 1 to that location.
- The HOST_CONTROL register is very different as compared to
the sdhci standard. This is accounted for in the write
case, but not when read back out (which it is in the sdhci
code).
Signed-off-by: Dave Russell <david.russell@datasoft.com>
Signed-off-by: Aaron Brice <aaron.brice@datasoft.com>
Acked-by: Dong Aisheng <aisheng.dong@nxp.com>
---
v1 -> v2:
- rename long_val to val
drivers/mmc/host/sdhci-esdhc-imx.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 1f54fd8..7123ef9 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -346,7 +346,8 @@ static void esdhc_writel_le(struct sdhci_host *host, u32 val, int reg)
struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
u32 data;
- if (unlikely(reg == SDHCI_INT_ENABLE || reg == SDHCI_SIGNAL_ENABLE)) {
+ if (unlikely(reg == SDHCI_INT_ENABLE || reg == SDHCI_SIGNAL_ENABLE ||
+ reg == SDHCI_INT_STATUS)) {
if ((val & SDHCI_INT_CARD_INT) && !esdhc_is_usdhc(imx_data)) {
/*
* Clear and then set D3CD bit to avoid missing the
@@ -555,6 +556,25 @@ static void esdhc_writew_le(struct sdhci_host *host, u16 val, int reg)
esdhc_clrset_le(host, 0xffff, val, reg);
}
+static u8 esdhc_readb_le(struct sdhci_host *host, int reg)
+{
+ u8 ret;
+ u32 val;
+
+ switch (reg) {
+ case SDHCI_HOST_CONTROL:
+ val = readl(host->ioaddr + reg);
+
+ ret = val & SDHCI_CTRL_LED;
+ ret |= (val >> 5) & SDHCI_CTRL_DMA_MASK;
+ ret |= (val & ESDHC_CTRL_4BITBUS);
+ ret |= (val & ESDHC_CTRL_8BITBUS) << 3;
+ return ret;
+ }
+
+ return readb(host->ioaddr + reg);
+}
+
static void esdhc_writeb_le(struct sdhci_host *host, u8 val, int reg)
{
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -947,6 +967,7 @@ static void esdhc_set_timeout(struct sdhci_host *host, struct mmc_command *cmd)
static struct sdhci_ops sdhci_esdhc_ops = {
.read_l = esdhc_readl_le,
.read_w = esdhc_readw_le,
+ .read_b = esdhc_readb_le,
.write_l = esdhc_writel_le,
.write_w = esdhc_writew_le,
.write_b = esdhc_writeb_le,
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v5 03/12] mmc: sdhci-msm: add pltfm_data support to get clk-rates from DT
From: Ritesh Harjani @ 2016-10-10 16:07 UTC (permalink / raw)
To: Rob Herring
Cc: ulf.hansson, linux-mmc, adrian.hunter, shawn.lin, david.brown,
andy.gross, devicetree, linux-arm-msm, georgi.djakov,
alex.lemberg, mateusz.nowak, Yuliy.Izrailov, asutoshd,
david.griego, stummala, venkatg, sboyd, bjorn.andersson,
pramod.gurav
In-Reply-To: <20161010125738.GA26940@rob-hp-laptop>
Hi Rob,
Thanks for review.
On 10/10/2016 6:27 PM, Rob Herring wrote:
> On Wed, Oct 05, 2016 at 08:10:31PM +0530, Ritesh Harjani wrote:
>> This adds support for sdhc-msm controllers to get supported
>> clk-rates from DT. sdhci-msm would need it's own set_clock
>> ops to be implemented. For this, supported clk-rates needs
>> to be populated in sdhci_msm_pltfm_data.
>>
>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>> ---
>> .../devicetree/bindings/mmc/sdhci-msm.txt | 1 +
>> drivers/mmc/host/sdhci-msm.c | 48 ++++++++++++++++++++++
>> 2 files changed, 49 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>> index 485483a..6a83b38 100644
>> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
>> @@ -17,6 +17,7 @@ Required properties:
>> "iface" - Main peripheral bus clock (PCLK/HCLK - AHB Bus clock) (required)
>> "core" - SDC MMC clock (MCLK) (required)
>> "bus" - SDCC bus voter clock (optional)
>> +- clk-rates: Array of supported GCC clock frequencies for sdhc, Units - Hz.
>
> Why can't some combination of assigned-clock-rates and querying the
> clock provider for rates be used here?
From what I understood, assigned-clock-rates would only work for
setting some default clock rates for certain clocks by calling
of_clk_set_defaults.
Whereas the requirement here is -
That since SDHC msm directly controls the clk(core clock) at source,
it's sdhci-msm driver needs to know the supported clk-rates by the
underlying platform to configure the nearest floor value supported on
this platform (when the request arrives from the core layer to switch
the clock).
Hence the table of clk-rates is provided for sdhci-msm.
>
> Minimally this would need unit suffix and either be made common or have
> a vendor prefix.
Sure will this work in that case - "qcom-clk-rates"
>
> Rob
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH v5 12/12] sdhci: sdhci-msm: update dll configuration
From: Ritesh Harjani @ 2016-10-10 15:54 UTC (permalink / raw)
To: Adrian Hunter, ulf.hansson, linux-mmc, shawn.lin
Cc: david.brown, andy.gross, devicetree, linux-arm-msm, georgi.djakov,
alex.lemberg, mateusz.nowak, Yuliy.Izrailov, asutoshd,
david.griego, stummala, venkatg, sboyd, bjorn.andersson,
pramod.gurav, Krishna Konda
In-Reply-To: <0d0df134-63ed-18c4-818e-1fae1b114b2c@intel.com>
Hi Adrian,
On 10/10/2016 6:57 PM, Adrian Hunter wrote:
> On 05/10/16 17:40, Ritesh Harjani wrote:
>> The newer msm sdhci's cores use a different DLL hardware for HS400.
>> Update the configuration and calibration of the newer DLL block.
>>
>> The HS400 DLL block used previously is CDC LP 533 and requires
>> programming multiple registers and waiting for configuration to
>> complete and then enable it. It has about 18 register writes and
>> two register reads.
>>
>> The newer HS400 DLL block is SDC4 DLL and requires two register
>> writes for configuration and one register read to confirm that it
>> is initialized. There is an additional register write to enable
>> the power save mode for SDC4 DLL block.
>>
>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>> Signed-off-by: Krishna Konda <kkonda@codeaurora.org>
>> ---
>> drivers/mmc/host/sdhci-msm.c | 141 ++++++++++++++++++++++++++++++++++++++-----
>> 1 file changed, 127 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index dbf80a9c..ddc8dc9 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -19,6 +19,7 @@
>> #include <linux/delay.h>
>> #include <linux/mmc/mmc.h>
>> #include <linux/slab.h>
>> +#include <linux/iopoll.h>
>>
>> #include "sdhci-pltfm.h"
>>
>> @@ -50,6 +51,7 @@
>> #define INT_MASK 0xf
>> #define MAX_PHASES 16
>> #define CORE_DLL_LOCK BIT(7)
>> +#define CORE_DDR_DLL_LOCK BIT(11)
>> #define CORE_DLL_EN BIT(16)
>> #define CORE_CDR_EN BIT(17)
>> #define CORE_CK_OUT_EN BIT(18)
>> @@ -61,6 +63,7 @@
>> #define CORE_DLL_STATUS 0x108
>>
>> #define CORE_DLL_CONFIG_2 0x1b4
>> +#define CORE_DDR_CAL_EN BIT(0)
>> #define CORE_FLL_CYCLE_CNT BIT(18)
>> #define CORE_DLL_CLOCK_DISABLE BIT(21)
>>
>> @@ -99,6 +102,11 @@
>> #define CORE_DDR_200_CFG 0x184
>> #define CORE_CDC_T4_DLY_SEL BIT(0)
>> #define CORE_START_CDC_TRAFFIC BIT(6)
>> +#define CORE_VENDOR_SPEC3 0x1b0
>> +#define CORE_PWRSAVE_DLL BIT(3)
>> +
>> +#define CORE_DDR_CONFIG 0x1b8
>> +#define DDR_CONFIG_POR_VAL 0x80040853
>>
>> #define CORE_VENDOR_SPEC_CAPABILITIES0 0x11c
>>
>> @@ -127,6 +135,7 @@ struct sdhci_msm_host {
>> bool tuning_done;
>> bool calibration_done;
>> u8 saved_tuning_phase;
>> + bool use_cdclp533;
>> };
>>
>> /* Platform specific tuning */
>> @@ -460,7 +469,7 @@ static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
>> {
>> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> - u32 wait_cnt, config;
>> + u32 config, calib_done;
>> int ret;
>>
>> pr_debug("%s: Enter %s\n", mmc_hostname(host->mmc), __func__);
>> @@ -552,18 +561,13 @@ static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
>> wmb(); /* drain writebuffer */
>>
>> /* Poll on CALIBRATION_DONE field in CORE_CSR_CDC_STATUS0 to be 1 */
>> - wait_cnt = 50;
>> - while (!(readl_relaxed(host->ioaddr + CORE_CSR_CDC_STATUS0)
>> - & CORE_CALIBRATION_DONE)) {
>> - /* max. wait for 50us sec for CALIBRATION_DONE bit to be set */
>> - if (--wait_cnt == 0) {
>> - pr_err("%s: %s: CDC Calibration was not completed\n",
>> + ret = readl_poll_timeout(host->ioaddr + CORE_CSR_CDC_STATUS0,
>
> This code was added in a previous patch, so it would make more sense to make
> it use readl_poll_timeout in the first place. Was there a reason to use
> readl_poll_timeout instead of readl_relaxed_poll_timeout()?
Sure will make the change in previous patch.
Thanks for pointing out about readl_relaxed_poll_timeout.
I think I only missed it, will check once again.
>
>> + calib_done, (calib_done & CORE_CALIBRATION_DONE), 1, 50);
>> +
>> + if (ret == -ETIMEDOUT) {
>> + pr_err("%s: %s: CDC Calibration was not completed\n",
>> mmc_hostname(host->mmc), __func__);
>> - ret = -ETIMEDOUT;
>> - goto out;
>> - }
>> - /* wait for 1us before polling again */
>> - udelay(1);
>> + goto out;
>> }
>>
>> /* Verify CDC_ERROR_CODE field in CORE_CSR_CDC_STATUS0 is 0 */
>> @@ -586,6 +590,86 @@ out:
>> return ret;
>> }
>>
>> +static int sdhci_msm_cm_dll_sdc4_calibration(struct sdhci_host *host)
>> +{
>> + u32 dll_status, config;
>> + int ret;
>> +
>> + pr_debug("%s: Enter %s\n", mmc_hostname(host->mmc), __func__);
>> +
>> + /*
>> + * Currently the CORE_DDR_CONFIG register defaults to desired
>> + * configuration on reset. Currently reprogramming the power on
>> + * reset (POR) value in case it might have been modified by
>> + * bootloaders. In the future, if this changes, then the desired
>> + * values will need to be programmed appropriately.
>> + */
>> + writel_relaxed(DDR_CONFIG_POR_VAL, host->ioaddr + CORE_DDR_CONFIG);
>> +
>> + /* Write 1 to DDR_CAL_EN field in CORE_DLL_CONFIG_2 */
>> + config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG_2);
>> + config |= CORE_DDR_CAL_EN;
>> + writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG_2);
>> +
>> + /* Poll on DDR_DLL_LOCK bit in CORE_DLL_STATUS to be set */
>> + ret = readl_poll_timeout(host->ioaddr + CORE_DLL_STATUS,
>
> Was there a reason to use readl_poll_timeout instead of
> readl_relaxed_poll_timeout()?
Sure will check it.
>
>> + dll_status, (dll_status & CORE_DDR_DLL_LOCK), 10, 1000);
>> +
>> + if (ret == -ETIMEDOUT) {
>> + pr_err("%s: %s: CM_DLL_SDC4 Calibration was not completed\n",
>> + mmc_hostname(host->mmc), __func__);
>> + goto out;
>> + }
>> +
>> + /* set CORE_PWRSAVE_DLL bit in CORE_VENDOR_SPEC3 */
>> + config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC3);
>> + config |= CORE_PWRSAVE_DLL;
>> + writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC3);
>> + wmb(); /* drain writebuffer */
>> +out:
>> + pr_debug("%s: Exit %s, ret:%d\n", mmc_hostname(host->mmc),
>> + __func__, ret);
>> + return ret;
>> +}
>> +
>> +static int sdhci_msm_hs400_dll_calibration(struct sdhci_host *host)
>> +{
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> + int ret;
>> + u32 config;
>> +
>> + pr_debug("%s: Enter %s\n", mmc_hostname(host->mmc), __func__);
>> +
>> + /*
>> + * Retuning in HS400 (DDR mode) will fail, just reset the
>> + * tuning block and restore the saved tuning phase.
>> + */
>> + ret = msm_init_cm_dll(host);
>> + if (ret)
>> + goto out;
>> +
>> + /* Set the selected phase in delay line hw block */
>> + ret = msm_config_cm_dll_phase(host, msm_host->saved_tuning_phase);
>> + if (ret)
>> + goto out;
>> +
>> + /* Write 1 to CMD_DAT_TRACK_SEL field in DLL_CONFIG */
>> + config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
>> + config |= CORE_CMD_DAT_TRACK_SEL;
>> + writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
>> + if (msm_host->use_cdclp533)
>> + /* Calibrate CDCLP533 DLL HW */
>> + ret = sdhci_msm_cdclp533_calibration(host);
>
> sdhci_msm_cdclp533_calibration() does some of the steps above all over
> again. Is that intended?
Yes, this was as per the HW sequence.
>
>
>> + else
>> + /* Calibrate CM_DLL_SDC4 HW */
>> + ret = sdhci_msm_cm_dll_sdc4_calibration(host);
>> +out:
>> + pr_debug("%s: Exit %s, ret:%d\n", mmc_hostname(host->mmc),
>> + __func__, ret);
>> + return ret;
>> +}
>> +
>> static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
>> {
>> int tuning_seq_cnt = 3;
>> @@ -737,7 +821,7 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
>> if (host->clock > CORE_FREQ_100MHZ &&
>> msm_host->tuning_done && !msm_host->calibration_done &&
>> (mmc->ios.timing == MMC_TIMING_MMC_HS400))
>> - if (!sdhci_msm_cdclp533_calibration(host))
>> + if (!sdhci_msm_hs400_dll_calibration(host))
>> msm_host->calibration_done = true;
>>
>> spin_lock_irq(&host->lock);
>> @@ -883,7 +967,7 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> struct mmc_ios curr_ios = host->mmc->ios;
>> - u32 msm_clock, config;
>> + u32 msm_clock, config, dll_lock;
>> int rc;
>>
>> if (!clock)
>> @@ -942,7 +1026,29 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>> config |= CORE_HC_SELECT_IN_EN;
>> writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
>> }
>> + if (!msm_host->clk_rate && !msm_host->use_cdclp533) {
>> + /*
>> + * Poll on DLL_LOCK and DDR_DLL_LOCK bits in
>> + * CORE_DLL_STATUS to be set. This should get set
>> + * with in 15 us at 200 MHz.
>
> 'with in' -> 'within'
Done.
>
>> + */
>> + rc = readl_poll_timeout(host->ioaddr + CORE_DLL_STATUS,
>
> Was there a reason to use readl_poll_timeout instead of
> readl_relaxed_poll_timeout()?
Sure will check it.
>
>> + dll_lock, (dll_lock & (CORE_DLL_LOCK |
>> + CORE_DDR_DLL_LOCK)), 10, 1000);
>
> The comment says 'DLL_LOCK and DDR_DLL_LOCK' but the logic looks 'DLL_LOCK
> or DDR_DLL_LOCK'
Ok, I will fix the comment.
will double confirm the HW spec as well.
>
>> + if (rc == -ETIMEDOUT)
>> + pr_err("%s: Unable to get DLL_LOCK/DDR_DLL_LOCK, dll_status: 0x%08x\n",
>> + mmc_hostname(host->mmc), dll_lock);
>> + }
>> } else {
>> + if (!msm_host->use_cdclp533) {
>> + /* set CORE_PWRSAVE_DLL bit in CORE_VENDOR_SPEC3 */
>> + config = readl_relaxed(host->ioaddr +
>> + CORE_VENDOR_SPEC3);
>> + config &= ~CORE_PWRSAVE_DLL;
>> + writel_relaxed(config, host->ioaddr +
>> + CORE_VENDOR_SPEC3);
>> + }
>> +
>> /* Select the default clock (free running MCLK) */
>> config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
>> config &= ~CORE_HC_MCLK_SEL_MASK;
>> @@ -1172,6 +1278,13 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>> msm_host->use_14lpp_dll_reset = true;
>>
>> /*
>> + * SDCC 5 controller with major version 1, minor version 0x34 and later
>> + * with HS 400 mode support will use CM DLL instead of CDC LP 533 DLL.
>> + */
>> + if ((core_major == 1) && (core_minor < 0x34))
>> + msm_host->use_cdclp533 = true;
>> +
>> + /*
>> * Support for some capabilities is not advertised by newer
>> * controller versions and must be explicitly enabled.
>> */
>>
>
^ permalink raw reply
* Re: [PATCH v5 11/12] mmc: sdhci-msm: Add calibration tuning for CDCLP533 circuit
From: Ritesh Harjani @ 2016-10-10 15:42 UTC (permalink / raw)
To: Adrian Hunter, ulf.hansson-QSEj5FYQhm4dnm+yROfE0A,
linux-mmc-u79uwXL29TY76Z2rM5mHXA,
shawn.lin-TNX95d0MmH7DzftRWevZcw
Cc: david.brown-QSEj5FYQhm4dnm+yROfE0A,
andy.gross-QSEj5FYQhm4dnm+yROfE0A,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
georgi.djakov-QSEj5FYQhm4dnm+yROfE0A,
alex.lemberg-XdAiOPVOjttBDgjK7y7TUQ,
mateusz.nowak-ral2JQCrhuEAvxtiuMwx3w,
Yuliy.Izrailov-XdAiOPVOjttBDgjK7y7TUQ,
asutoshd-sgV2jX0FEOL9JmXXK+q4OQ,
david.griego-QSEj5FYQhm4dnm+yROfE0A,
stummala-sgV2jX0FEOL9JmXXK+q4OQ, venkatg-sgV2jX0FEOL9JmXXK+q4OQ,
sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A,
pramod.gurav-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <183c2e6a-179b-b042-aef9-d1e5cb90b17d-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Hi Adrian,
On 10/10/2016 6:19 PM, Adrian Hunter wrote:
> On 05/10/16 17:40, Ritesh Harjani wrote:
>> From: Venkat Gopalakrishnan <venkatg-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>
>> In HS400 mode a new RCLK is introduced on the interface for read data
>> transfers. The eMMC5.0 device transmits the read data to the host with
>> respect to rising and falling edges of RCLK. In order to ensure correct
>> operation of read data transfers in HS400 mode, the incoming RX data
>> needs to be sampled by delayed version of RCLK.
>>
>> The CDCLP533 delay circuit shifts the RCLK by T/4. It needs to be
>> initialized, configured and enabled once during HS400 mode switch and
>> when operational voltage/clock is changed.
>>
>> Signed-off-by: Venkat Gopalakrishnan <venkatg-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> Signed-off-by: Ritesh Harjani <riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> ---
>> drivers/mmc/host/sdhci-msm.c | 178 +++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 178 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index 612fa82..dbf80a9c 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -57,6 +57,7 @@
>> #define CORE_DLL_PDN BIT(29)
>> #define CORE_DLL_RST BIT(30)
>> #define CORE_DLL_CONFIG 0x100
>> +#define CORE_CMD_DAT_TRACK_SEL BIT(0)
>> #define CORE_DLL_STATUS 0x108
>>
>> #define CORE_DLL_CONFIG_2 0x1b4
>> @@ -72,8 +73,36 @@
>> #define CORE_HC_SELECT_IN_HS400 (6 << 19)
>> #define CORE_HC_SELECT_IN_MASK (7 << 19)
>>
>> +#define CORE_CSR_CDC_CTLR_CFG0 0x130
>> +#define CORE_SW_TRIG_FULL_CALIB BIT(16)
>> +#define CORE_HW_AUTOCAL_ENA BIT(17)
>> +
>> +#define CORE_CSR_CDC_CTLR_CFG1 0x134
>> +#define CORE_CSR_CDC_CAL_TIMER_CFG0 0x138
>> +#define CORE_TIMER_ENA BIT(16)
>> +
>> +#define CORE_CSR_CDC_CAL_TIMER_CFG1 0x13C
>> +#define CORE_CSR_CDC_REFCOUNT_CFG 0x140
>> +#define CORE_CSR_CDC_COARSE_CAL_CFG 0x144
>> +#define CORE_CDC_OFFSET_CFG 0x14C
>> +#define CORE_CSR_CDC_DELAY_CFG 0x150
>> +#define CORE_CDC_SLAVE_DDA_CFG 0x160
>> +#define CORE_CSR_CDC_STATUS0 0x164
>> +#define CORE_CALIBRATION_DONE BIT(0)
>> +
>> +#define CORE_CDC_ERROR_CODE_MASK 0x7000000
>> +
>> +#define CORE_CSR_CDC_GEN_CFG 0x178
>> +#define CORE_CDC_SWITCH_BYPASS_OFF BIT(0)
>> +#define CORE_CDC_SWITCH_RC_EN BIT(1)
>> +
>> +#define CORE_DDR_200_CFG 0x184
>> +#define CORE_CDC_T4_DLY_SEL BIT(0)
>> +#define CORE_START_CDC_TRAFFIC BIT(6)
>> +
>> #define CORE_VENDOR_SPEC_CAPABILITIES0 0x11c
>>
>> +#define INVALID_TUNING_PHASE -1
>> #define TCXO_FREQ 19200000
>> #define SDHCI_MSM_MIN_CLOCK 400000
>> #define CORE_FREQ_100MHZ (100 * 1000 * 1000)
>> @@ -97,6 +126,7 @@ struct sdhci_msm_host {
>> bool use_14lpp_dll_reset;
>> bool tuning_done;
>> bool calibration_done;
>> + u8 saved_tuning_phase;
>> };
>>
>> /* Platform specific tuning */
>> @@ -426,6 +456,136 @@ static int msm_init_cm_dll(struct sdhci_host *host)
>> return 0;
>> }
>>
>> +static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
>> +{
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> + u32 wait_cnt, config;
>> + int ret;
>> +
>> + pr_debug("%s: Enter %s\n", mmc_hostname(host->mmc), __func__);
>> +
>> + /*
>> + * Retuning in HS400 (DDR mode) will fail, just reset the
>> + * tuning block and restore the saved tuning phase.
>> + */
>> + ret = msm_init_cm_dll(host);
>> + if (ret)
>> + goto out;
>> +
>> + /* Set the selected phase in delay line hw block */
>> + ret = msm_config_cm_dll_phase(host, msm_host->saved_tuning_phase);
>> + if (ret)
>> + goto out;
>> +
>> + /* Write 1 to CMD_DAT_TRACK_SEL field in DLL_CONFIG */
>> + config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
>> + config |= CORE_CMD_DAT_TRACK_SEL;
>> + writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
>> +
>> + /* Write 0 to CDC_T4_DLY_SEL field in VENDOR_SPEC_DDR200_CFG */
>> + config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
>> + config &= ~CORE_CDC_T4_DLY_SEL;
>> + writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
>> +
>> + /* Write 0 to CDC_SWITCH_BYPASS_OFF field in CORE_CSR_CDC_GEN_CFG */
>> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_GEN_CFG);
>> + config &= ~CORE_CDC_SWITCH_BYPASS_OFF;
>> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_GEN_CFG);
>> +
>> + /* Write 1 to CDC_SWITCH_RC_EN field in CORE_CSR_CDC_GEN_CFG */
>> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_GEN_CFG);
>> + config |= CORE_CDC_SWITCH_RC_EN;
>> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_GEN_CFG);
>> +
>> + /* Write 0 to START_CDC_TRAFFIC field in CORE_DDR200_CFG */
>> + config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
>> + config &= ~CORE_START_CDC_TRAFFIC;
>> + writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
>> +
>> + /*
>> + * Perform CDC Register Initialization Sequence
>> + *
>> + * CORE_CSR_CDC_CTLR_CFG0 0x11800EC
>> + * CORE_CSR_CDC_CTLR_CFG1 0x3011111
>> + * CORE_CSR_CDC_CAL_TIMER_CFG0 0x1201000
>> + * CORE_CSR_CDC_CAL_TIMER_CFG1 0x4
>> + * CORE_CSR_CDC_REFCOUNT_CFG 0xCB732020
>> + * CORE_CSR_CDC_COARSE_CAL_CFG 0xB19
>> + * CORE_CSR_CDC_DELAY_CFG 0x3AC
>> + * CORE_CDC_OFFSET_CFG 0x0
>> + * CORE_CDC_SLAVE_DDA_CFG 0x16334
>> + */
>> +
>> + writel_relaxed(0x11800EC, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>> + writel_relaxed(0x3011111, host->ioaddr + CORE_CSR_CDC_CTLR_CFG1);
>> + writel_relaxed(0x1201000, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
>> + writel_relaxed(0x4, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG1);
>> + writel_relaxed(0xCB732020, host->ioaddr + CORE_CSR_CDC_REFCOUNT_CFG);
>> + writel_relaxed(0xB19, host->ioaddr + CORE_CSR_CDC_COARSE_CAL_CFG);
>> + writel_relaxed(0x3AC, host->ioaddr + CORE_CSR_CDC_DELAY_CFG);
>> + writel_relaxed(0x0, host->ioaddr + CORE_CDC_OFFSET_CFG);
>> + writel_relaxed(0x16334, host->ioaddr + CORE_CDC_SLAVE_DDA_CFG);
>> +
>> + /* CDC HW Calibration */
>> +
>> + /* Write 1 to SW_TRIG_FULL_CALIB field in CORE_CSR_CDC_CTLR_CFG0 */
>> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>> + config |= CORE_SW_TRIG_FULL_CALIB;
>> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>> +
>> + /* Write 0 to SW_TRIG_FULL_CALIB field in CORE_CSR_CDC_CTLR_CFG0 */
>> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>> + config &= ~CORE_SW_TRIG_FULL_CALIB;
>> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>> +
>> + /* Write 1 to HW_AUTOCAL_ENA field in CORE_CSR_CDC_CTLR_CFG0 */
>> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>> + config |= CORE_HW_AUTOCAL_ENA;
>> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
>> +
>> + /* Write 1 to TIMER_ENA field in CORE_CSR_CDC_CAL_TIMER_CFG0 */
>> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
>> + config |= CORE_TIMER_ENA;
>> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
>> +
>> + wmb(); /* drain writebuffer */
>> +
>> + /* Poll on CALIBRATION_DONE field in CORE_CSR_CDC_STATUS0 to be 1 */
>> + wait_cnt = 50;
>> + while (!(readl_relaxed(host->ioaddr + CORE_CSR_CDC_STATUS0)
>> + & CORE_CALIBRATION_DONE)) {
>> + /* max. wait for 50us sec for CALIBRATION_DONE bit to be set */
>> + if (--wait_cnt == 0) {
>> + pr_err("%s: %s: CDC Calibration was not completed\n",
>> + mmc_hostname(host->mmc), __func__);
>> + ret = -ETIMEDOUT;
>> + goto out;
>> + }
>> + /* wait for 1us before polling again */
>> + udelay(1);
>> + }
>> +
>> + /* Verify CDC_ERROR_CODE field in CORE_CSR_CDC_STATUS0 is 0 */
>> + ret = readl_relaxed(host->ioaddr + CORE_CSR_CDC_STATUS0)
>> + & CORE_CDC_ERROR_CODE_MASK;
>> + if (ret) {
>> + pr_err("%s: %s: CDC Error Code %d\n",
>> + mmc_hostname(host->mmc), __func__, ret);
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> +
>> + /* Write 1 to START_CDC_TRAFFIC field in CORE_DDR200_CFG */
>> + config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
>> + config |= CORE_START_CDC_TRAFFIC;
>> + writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
>> +out:
>> + pr_debug("%s: Exit %s, ret:%d\n", mmc_hostname(host->mmc),
>> + __func__, ret);
>> + return ret;
>> +}
>> +
>> static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
>> {
>> int tuning_seq_cnt = 3;
>> @@ -433,6 +593,8 @@ static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
>> int rc;
>> struct mmc_host *mmc = host->mmc;
>> struct mmc_ios ios = host->mmc->ios;
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>>
>> /*
>> * Tuning is required for SDR104, HS200 and HS400 cards and
>> @@ -457,6 +619,7 @@ retry:
>> if (rc)
>> return rc;
>>
>> + msm_host->saved_tuning_phase = phase;
>> rc = mmc_send_tuning(mmc, opcode, NULL);
>> if (!rc) {
>> /* Tuning is successful at this tuning point */
>> @@ -492,6 +655,8 @@ retry:
>> rc = -EIO;
>> }
>>
>> + if (!rc)
>> + msm_host->tuning_done = true;
>> return rc;
>> }
>>
>> @@ -565,6 +730,17 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
>> dev_dbg(mmc_dev(mmc), "%s: clock=%u uhs=%u ctrl_2=0x%x\n",
>> mmc_hostname(host->mmc), host->clock, uhs, ctrl_2);
>> sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
>> +
>> + spin_unlock_irq(&host->lock);
>> +
>> + /* CDCLP533 HW calibration is only required for HS400 mode*/
>> + if (host->clock > CORE_FREQ_100MHZ &&
>> + msm_host->tuning_done && !msm_host->calibration_done &&
>> + (mmc->ios.timing == MMC_TIMING_MMC_HS400))
>> + if (!sdhci_msm_cdclp533_calibration(host))
>> + msm_host->calibration_done = true;
>> +
>> + spin_lock_irq(&host->lock);
>> }
>>
>> static void sdhci_msm_voltage_switch(struct sdhci_host *host)
>> @@ -907,6 +1083,8 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>>
>> sdhci_msm_populate_dt(&pdev->dev, msm_host);
>>
>> + msm_host->saved_tuning_phase = INVALID_TUNING_PHASE;
>
> There is never a check for INVALID_TUNING_PHASE which begs the question: why
> have it?
phase value can be between 0x0 to 0xf. So during probe
saved_tuning_phase is getting initialized with -1 value.
Let me know if any concern, we can remove it as well.
But wont it look incorrect if we initialize it with some valid phase
value before even tuning is completed?
>
>> +
>> /* Setup SDCC bus voter clock. */
>> msm_host->bus_clk = devm_clk_get(&pdev->dev, "bus");
>> if (!IS_ERR(msm_host->bus_clk)) {
>>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 10/12] mmc: sdhci-msm: Add HS400 platform support
From: Ritesh Harjani @ 2016-10-10 15:26 UTC (permalink / raw)
To: Adrian Hunter, ulf.hansson, linux-mmc, shawn.lin
Cc: david.brown, andy.gross, devicetree, linux-arm-msm, georgi.djakov,
alex.lemberg, mateusz.nowak, Yuliy.Izrailov, asutoshd,
david.griego, stummala, venkatg, sboyd, bjorn.andersson,
pramod.gurav
In-Reply-To: <0596e37d-0d8d-0f8e-8884-4404e2979593@intel.com>
Hi Adrian,
Thanks for the complete review.
On 10/10/2016 5:38 PM, Adrian Hunter wrote:
> On 05/10/16 17:40, Ritesh Harjani wrote:
>> From: Venkat Gopalakrishnan <venkatg@codeaurora.org>
>>
>> The following msm platform specific changes are added to support HS400.
>> - Allow tuning for HS400 mode.
>> - Configure HS400 timing mode using the VENDOR_SPECIFIC_FUNC register.
>>
>> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>> ---
>> drivers/mmc/host/sdhci-msm.c | 124 +++++++++++++++++++++++++++++++++++++++----
>> 1 file changed, 113 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index eb1a9e3..612fa82 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -31,6 +31,7 @@
>> #define HC_MODE_EN 0x1
>> #define CORE_POWER 0x0
>> #define CORE_SW_RST BIT(7)
>> +#define FF_CLK_SW_RST_DIS BIT(13)
>>
>> #define CORE_PWRCTL_STATUS 0xdc
>> #define CORE_PWRCTL_MASK 0xe0
>> @@ -64,11 +65,18 @@
>>
>> #define CORE_VENDOR_SPEC 0x10c
>> #define CORE_CLK_PWRSAVE BIT(1)
>> +#define CORE_HC_MCLK_SEL_DFLT (2 << 8)
>> +#define CORE_HC_MCLK_SEL_HS400 (3 << 8)
>> +#define CORE_HC_MCLK_SEL_MASK (3 << 8)
>> +#define CORE_HC_SELECT_IN_EN (1 << 18)
>> +#define CORE_HC_SELECT_IN_HS400 (6 << 19)
>> +#define CORE_HC_SELECT_IN_MASK (7 << 19)
>>
>> #define CORE_VENDOR_SPEC_CAPABILITIES0 0x11c
>>
>> #define TCXO_FREQ 19200000
>> #define SDHCI_MSM_MIN_CLOCK 400000
>> +#define CORE_FREQ_100MHZ (100 * 1000 * 1000)
>>
>> #define CDR_SELEXT_SHIFT 20
>> #define CDR_SELEXT_MASK (0xf << CDR_SELEXT_SHIFT)
>> @@ -87,6 +95,8 @@ struct sdhci_msm_host {
>> u32 clk_rate;
>> struct mmc_host *mmc;
>> bool use_14lpp_dll_reset;
>> + bool tuning_done;
>> + bool calibration_done;
>> };
>>
>> /* Platform specific tuning */
>> @@ -175,8 +185,8 @@ out:
>> * Find out the greatest range of consecuitive selected
>> * DLL clock output phases that can be used as sampling
>> * setting for SD3.0 UHS-I card read operation (in SDR104
>> - * timing mode) or for eMMC4.5 card read operation (in HS200
>> - * timing mode).
>> + * timing mode) or for eMMC4.5 card read operation (in
>> + * HS400/HS200 timing mode).
>> * Select the 3/4 of the range and configure the DLL with the
>> * selected DLL clock output phase.
>> */
>> @@ -428,9 +438,10 @@ static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
>> * Tuning is required for SDR104, HS200 and HS400 cards and
>> * if clock frequency is greater than 100MHz in these modes.
>> */
>> - if (host->clock <= 100 * 1000 * 1000 ||
>> - !((ios.timing == MMC_TIMING_MMC_HS200) ||
>> - (ios.timing == MMC_TIMING_UHS_SDR104)))
>> + if (host->clock <= CORE_FREQ_100MHZ ||
>> + !((ios.timing == MMC_TIMING_MMC_HS400) ||
>> + (ios.timing == MMC_TIMING_MMC_HS200) ||
>> + (ios.timing == MMC_TIMING_UHS_SDR104)))
>
> Don't need () around ios.timing == MMC_TIMING_MMC_HS400 etc
Sure. Done.
>
>> return 0;
>>
>> retry:
>> @@ -488,7 +499,10 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
>> unsigned int uhs)
>> {
>> struct mmc_host *mmc = host->mmc;
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> u16 ctrl_2;
>> + u32 config;
>>
>> ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>> /* Select Bus Speed Mode for host */
>> @@ -503,6 +517,7 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
>> case MMC_TIMING_UHS_SDR50:
>> ctrl_2 |= SDHCI_CTRL_UHS_SDR50;
>> break;
>> + case MMC_TIMING_MMC_HS400:
>> case MMC_TIMING_MMC_HS200:
>> case MMC_TIMING_UHS_SDR104:
>> ctrl_2 |= SDHCI_CTRL_UHS_SDR104;
>> @@ -519,11 +534,33 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
>> * provide feedback clock, the mode selection can be any value less
>> * than 3'b011 in bits [2:0] of HOST CONTROL2 register.
>> */
>> - if (host->clock <= 100000000 &&
>> - (uhs == MMC_TIMING_MMC_HS400 ||
>> - uhs == MMC_TIMING_MMC_HS200 ||
>> - uhs == MMC_TIMING_UHS_SDR104))
>> - ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
>> + if (host->clock <= CORE_FREQ_100MHZ) {
>> + if ((uhs == MMC_TIMING_MMC_HS400) ||
>> + (uhs == MMC_TIMING_MMC_HS200) ||
>> + (uhs == MMC_TIMING_UHS_SDR104))
>> + ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
>> + /*
>> + * Make sure DLL is disabled when not required
>> + *
>> + * Write 1 to DLL_RST bit of DLL_CONFIG register
>> + */
>> + config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
>> + config |= CORE_DLL_RST;
>> + writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
>> +
>> + /* Write 1 to DLL_PDN bit of DLL_CONFIG register */
>> + config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
>> + config |= CORE_DLL_PDN;
>> + writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
>> +
>> + wmb(); /* drain writebuffer */
>
> Memory barrier could use more explanation.
Ok.
>
>> +
>> + /*
>> + * The DLL needs to be restored and CDCLP533 recalibrated
>> + * when the clock frequency is set back to 400MHz.
>> + */
>> + msm_host->calibration_done = false;
>> + }
>>
>> dev_dbg(mmc_dev(mmc), "%s: clock=%u uhs=%u ctrl_2=0x%x\n",
>> mmc_hostname(host->mmc), host->clock, uhs, ctrl_2);
>> @@ -670,7 +707,7 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> struct mmc_ios curr_ios = host->mmc->ios;
>> - u32 msm_clock;
>> + u32 msm_clock, config;
>> int rc;
>>
>> if (!clock)
>> @@ -691,6 +728,66 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>> }
>> msm_clock = sdhci_msm_get_msm_clk_rate(host, clock);
>>
>> + /*
>> + * In general all timing modes are controlled via UHS mode select in
>> + * Host Control2 register. eMMC specific HS200/HS400 doesn't have
>> + * their respective modes defined here, hence we use these values.
>> + *
>> + * HS200 - SDR104 (Since they both are equivalent in functionality)
>> + * HS400 - This involves multiple configurations
>> + * Initially SDR104 - when tuning is required as HS200
>> + * Then when switching to DDR @ 400MHz (HS400) we use
>> + * the vendor specific HC_SELECT_IN to control the mode.
>> + *
>> + * In addition to controlling the modes we also need to select the
>> + * correct input clock for DLL depending on the mode.
>> + *
>> + * HS400 - divided clock (free running MCLK/2)
>> + * All other modes - default (free running MCLK)
>> + */
>> + if (curr_ios.timing == MMC_TIMING_MMC_HS400) {
>> + /* Select the divided clock (free running MCLK/2) */
>> + config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
>> + config &= ~CORE_HC_MCLK_SEL_MASK;
>> + config |= CORE_HC_MCLK_SEL_HS400;
>> +
>> + writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
>> + /*
>> + * Select HS400 mode using the HC_SELECT_IN from VENDOR SPEC
>> + * register
>> + */
>> + if (msm_host->tuning_done && !msm_host->calibration_done) {
>
>
> In this patch, tuning_done and calibration_done are never true. Is that
> intended?
Select HS400 which is done below should happen only after the tuning is
performed. Hence the check of that variable above.
We set tuning_done = true in the next patch.
Please let me know if any concerns.
Similarly calibration_done is also getting set to true in the next patch.
>
>
>> + /*
>> + * Write 0x6 to HC_SELECT_IN and 1 to HC_SELECT_IN_EN
>> + * field in VENDOR_SPEC_FUNC
>> + */
>> + config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
>> + config |= CORE_HC_SELECT_IN_HS400;
>> + config |= CORE_HC_SELECT_IN_EN;
>> + writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
>> + }
>> + } else {
>> + /* Select the default clock (free running MCLK) */
>> + config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
>> + config &= ~CORE_HC_MCLK_SEL_MASK;
>> + config |= CORE_HC_MCLK_SEL_DFLT;
>> + writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
>> +
>> + /*
>> + * Disable HC_SELECT_IN to be able to use the UHS mode select
>> + * configuration from Host Control2 register for all other
>> + * modes.
>> + *
>> + * Write 0 to HC_SELECT_IN and HC_SELECT_IN_EN field
>> + * in VENDOR_SPEC_FUNC
>> + */
>> + config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
>> + config &= ~CORE_HC_SELECT_IN_EN;
>> + config &= ~CORE_HC_SELECT_IN_MASK;
>> + writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
>> + }
>> + wmb(); /* drain writebuffer */
>
> Memory barrier could use more explanation.
Ok. Sure.
>
>> +
>> if ((msm_clock != msm_host->clk_rate) && msm_host->clk_table) {
>> rc = clk_set_rate(msm_host->clk, msm_clock);
>> if (rc) {
>> @@ -876,6 +973,11 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>> /* Set HC_MODE_EN bit in HC_MODE register */
>> writel_relaxed(HC_MODE_EN, (msm_host->core_mem + CORE_HC_MODE));
>>
>> + /* Set FF_CLK_SW_RST_DIS bit in HC_MODE register */
>> + config = readl_relaxed(msm_host->core_mem + CORE_HC_MODE);
>> + config |= FF_CLK_SW_RST_DIS;
>> + writel_relaxed(config, msm_host->core_mem + CORE_HC_MODE);
>> +
>> host_version = readw_relaxed((host->ioaddr + SDHCI_HOST_VERSION));
>> dev_dbg(&pdev->dev, "Host Version: 0x%x Vendor Version 0x%x\n",
>> host_version, ((host_version & SDHCI_VENDOR_VER_MASK) >>
>>
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH v5 12/12] sdhci: sdhci-msm: update dll configuration
From: Adrian Hunter @ 2016-10-10 13:27 UTC (permalink / raw)
To: Ritesh Harjani, ulf.hansson, linux-mmc, shawn.lin
Cc: david.brown, andy.gross, devicetree, linux-arm-msm, georgi.djakov,
alex.lemberg, mateusz.nowak, Yuliy.Izrailov, asutoshd,
david.griego, stummala, venkatg, sboyd, bjorn.andersson,
pramod.gurav, Krishna Konda
In-Reply-To: <1475678440-3525-13-git-send-email-riteshh@codeaurora.org>
On 05/10/16 17:40, Ritesh Harjani wrote:
> The newer msm sdhci's cores use a different DLL hardware for HS400.
> Update the configuration and calibration of the newer DLL block.
>
> The HS400 DLL block used previously is CDC LP 533 and requires
> programming multiple registers and waiting for configuration to
> complete and then enable it. It has about 18 register writes and
> two register reads.
>
> The newer HS400 DLL block is SDC4 DLL and requires two register
> writes for configuration and one register read to confirm that it
> is initialized. There is an additional register write to enable
> the power save mode for SDC4 DLL block.
>
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
> Signed-off-by: Krishna Konda <kkonda@codeaurora.org>
> ---
> drivers/mmc/host/sdhci-msm.c | 141 ++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 127 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index dbf80a9c..ddc8dc9 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -19,6 +19,7 @@
> #include <linux/delay.h>
> #include <linux/mmc/mmc.h>
> #include <linux/slab.h>
> +#include <linux/iopoll.h>
>
> #include "sdhci-pltfm.h"
>
> @@ -50,6 +51,7 @@
> #define INT_MASK 0xf
> #define MAX_PHASES 16
> #define CORE_DLL_LOCK BIT(7)
> +#define CORE_DDR_DLL_LOCK BIT(11)
> #define CORE_DLL_EN BIT(16)
> #define CORE_CDR_EN BIT(17)
> #define CORE_CK_OUT_EN BIT(18)
> @@ -61,6 +63,7 @@
> #define CORE_DLL_STATUS 0x108
>
> #define CORE_DLL_CONFIG_2 0x1b4
> +#define CORE_DDR_CAL_EN BIT(0)
> #define CORE_FLL_CYCLE_CNT BIT(18)
> #define CORE_DLL_CLOCK_DISABLE BIT(21)
>
> @@ -99,6 +102,11 @@
> #define CORE_DDR_200_CFG 0x184
> #define CORE_CDC_T4_DLY_SEL BIT(0)
> #define CORE_START_CDC_TRAFFIC BIT(6)
> +#define CORE_VENDOR_SPEC3 0x1b0
> +#define CORE_PWRSAVE_DLL BIT(3)
> +
> +#define CORE_DDR_CONFIG 0x1b8
> +#define DDR_CONFIG_POR_VAL 0x80040853
>
> #define CORE_VENDOR_SPEC_CAPABILITIES0 0x11c
>
> @@ -127,6 +135,7 @@ struct sdhci_msm_host {
> bool tuning_done;
> bool calibration_done;
> u8 saved_tuning_phase;
> + bool use_cdclp533;
> };
>
> /* Platform specific tuning */
> @@ -460,7 +469,7 @@ static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
> {
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> - u32 wait_cnt, config;
> + u32 config, calib_done;
> int ret;
>
> pr_debug("%s: Enter %s\n", mmc_hostname(host->mmc), __func__);
> @@ -552,18 +561,13 @@ static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
> wmb(); /* drain writebuffer */
>
> /* Poll on CALIBRATION_DONE field in CORE_CSR_CDC_STATUS0 to be 1 */
> - wait_cnt = 50;
> - while (!(readl_relaxed(host->ioaddr + CORE_CSR_CDC_STATUS0)
> - & CORE_CALIBRATION_DONE)) {
> - /* max. wait for 50us sec for CALIBRATION_DONE bit to be set */
> - if (--wait_cnt == 0) {
> - pr_err("%s: %s: CDC Calibration was not completed\n",
> + ret = readl_poll_timeout(host->ioaddr + CORE_CSR_CDC_STATUS0,
This code was added in a previous patch, so it would make more sense to make
it use readl_poll_timeout in the first place. Was there a reason to use
readl_poll_timeout instead of readl_relaxed_poll_timeout()?
> + calib_done, (calib_done & CORE_CALIBRATION_DONE), 1, 50);
> +
> + if (ret == -ETIMEDOUT) {
> + pr_err("%s: %s: CDC Calibration was not completed\n",
> mmc_hostname(host->mmc), __func__);
> - ret = -ETIMEDOUT;
> - goto out;
> - }
> - /* wait for 1us before polling again */
> - udelay(1);
> + goto out;
> }
>
> /* Verify CDC_ERROR_CODE field in CORE_CSR_CDC_STATUS0 is 0 */
> @@ -586,6 +590,86 @@ out:
> return ret;
> }
>
> +static int sdhci_msm_cm_dll_sdc4_calibration(struct sdhci_host *host)
> +{
> + u32 dll_status, config;
> + int ret;
> +
> + pr_debug("%s: Enter %s\n", mmc_hostname(host->mmc), __func__);
> +
> + /*
> + * Currently the CORE_DDR_CONFIG register defaults to desired
> + * configuration on reset. Currently reprogramming the power on
> + * reset (POR) value in case it might have been modified by
> + * bootloaders. In the future, if this changes, then the desired
> + * values will need to be programmed appropriately.
> + */
> + writel_relaxed(DDR_CONFIG_POR_VAL, host->ioaddr + CORE_DDR_CONFIG);
> +
> + /* Write 1 to DDR_CAL_EN field in CORE_DLL_CONFIG_2 */
> + config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG_2);
> + config |= CORE_DDR_CAL_EN;
> + writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG_2);
> +
> + /* Poll on DDR_DLL_LOCK bit in CORE_DLL_STATUS to be set */
> + ret = readl_poll_timeout(host->ioaddr + CORE_DLL_STATUS,
Was there a reason to use readl_poll_timeout instead of
readl_relaxed_poll_timeout()?
> + dll_status, (dll_status & CORE_DDR_DLL_LOCK), 10, 1000);
> +
> + if (ret == -ETIMEDOUT) {
> + pr_err("%s: %s: CM_DLL_SDC4 Calibration was not completed\n",
> + mmc_hostname(host->mmc), __func__);
> + goto out;
> + }
> +
> + /* set CORE_PWRSAVE_DLL bit in CORE_VENDOR_SPEC3 */
> + config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC3);
> + config |= CORE_PWRSAVE_DLL;
> + writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC3);
> + wmb(); /* drain writebuffer */
> +out:
> + pr_debug("%s: Exit %s, ret:%d\n", mmc_hostname(host->mmc),
> + __func__, ret);
> + return ret;
> +}
> +
> +static int sdhci_msm_hs400_dll_calibration(struct sdhci_host *host)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> + int ret;
> + u32 config;
> +
> + pr_debug("%s: Enter %s\n", mmc_hostname(host->mmc), __func__);
> +
> + /*
> + * Retuning in HS400 (DDR mode) will fail, just reset the
> + * tuning block and restore the saved tuning phase.
> + */
> + ret = msm_init_cm_dll(host);
> + if (ret)
> + goto out;
> +
> + /* Set the selected phase in delay line hw block */
> + ret = msm_config_cm_dll_phase(host, msm_host->saved_tuning_phase);
> + if (ret)
> + goto out;
> +
> + /* Write 1 to CMD_DAT_TRACK_SEL field in DLL_CONFIG */
> + config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
> + config |= CORE_CMD_DAT_TRACK_SEL;
> + writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
> + if (msm_host->use_cdclp533)
> + /* Calibrate CDCLP533 DLL HW */
> + ret = sdhci_msm_cdclp533_calibration(host);
sdhci_msm_cdclp533_calibration() does some of the steps above all over
again. Is that intended?
> + else
> + /* Calibrate CM_DLL_SDC4 HW */
> + ret = sdhci_msm_cm_dll_sdc4_calibration(host);
> +out:
> + pr_debug("%s: Exit %s, ret:%d\n", mmc_hostname(host->mmc),
> + __func__, ret);
> + return ret;
> +}
> +
> static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
> {
> int tuning_seq_cnt = 3;
> @@ -737,7 +821,7 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
> if (host->clock > CORE_FREQ_100MHZ &&
> msm_host->tuning_done && !msm_host->calibration_done &&
> (mmc->ios.timing == MMC_TIMING_MMC_HS400))
> - if (!sdhci_msm_cdclp533_calibration(host))
> + if (!sdhci_msm_hs400_dll_calibration(host))
> msm_host->calibration_done = true;
>
> spin_lock_irq(&host->lock);
> @@ -883,7 +967,7 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> struct mmc_ios curr_ios = host->mmc->ios;
> - u32 msm_clock, config;
> + u32 msm_clock, config, dll_lock;
> int rc;
>
> if (!clock)
> @@ -942,7 +1026,29 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
> config |= CORE_HC_SELECT_IN_EN;
> writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
> }
> + if (!msm_host->clk_rate && !msm_host->use_cdclp533) {
> + /*
> + * Poll on DLL_LOCK and DDR_DLL_LOCK bits in
> + * CORE_DLL_STATUS to be set. This should get set
> + * with in 15 us at 200 MHz.
'with in' -> 'within'
> + */
> + rc = readl_poll_timeout(host->ioaddr + CORE_DLL_STATUS,
Was there a reason to use readl_poll_timeout instead of
readl_relaxed_poll_timeout()?
> + dll_lock, (dll_lock & (CORE_DLL_LOCK |
> + CORE_DDR_DLL_LOCK)), 10, 1000);
The comment says 'DLL_LOCK and DDR_DLL_LOCK' but the logic looks 'DLL_LOCK
or DDR_DLL_LOCK'
> + if (rc == -ETIMEDOUT)
> + pr_err("%s: Unable to get DLL_LOCK/DDR_DLL_LOCK, dll_status: 0x%08x\n",
> + mmc_hostname(host->mmc), dll_lock);
> + }
> } else {
> + if (!msm_host->use_cdclp533) {
> + /* set CORE_PWRSAVE_DLL bit in CORE_VENDOR_SPEC3 */
> + config = readl_relaxed(host->ioaddr +
> + CORE_VENDOR_SPEC3);
> + config &= ~CORE_PWRSAVE_DLL;
> + writel_relaxed(config, host->ioaddr +
> + CORE_VENDOR_SPEC3);
> + }
> +
> /* Select the default clock (free running MCLK) */
> config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
> config &= ~CORE_HC_MCLK_SEL_MASK;
> @@ -1172,6 +1278,13 @@ static int sdhci_msm_probe(struct platform_device *pdev)
> msm_host->use_14lpp_dll_reset = true;
>
> /*
> + * SDCC 5 controller with major version 1, minor version 0x34 and later
> + * with HS 400 mode support will use CM DLL instead of CDC LP 533 DLL.
> + */
> + if ((core_major == 1) && (core_minor < 0x34))
> + msm_host->use_cdclp533 = true;
> +
> + /*
> * Support for some capabilities is not advertised by newer
> * controller versions and must be explicitly enabled.
> */
>
^ permalink raw reply
* Re: [PATCH v5 03/12] mmc: sdhci-msm: add pltfm_data support to get clk-rates from DT
From: Rob Herring @ 2016-10-10 12:57 UTC (permalink / raw)
To: Ritesh Harjani
Cc: ulf.hansson-QSEj5FYQhm4dnm+yROfE0A,
linux-mmc-u79uwXL29TY76Z2rM5mHXA,
adrian.hunter-ral2JQCrhuEAvxtiuMwx3w,
shawn.lin-TNX95d0MmH7DzftRWevZcw,
david.brown-QSEj5FYQhm4dnm+yROfE0A,
andy.gross-QSEj5FYQhm4dnm+yROfE0A,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
georgi.djakov-QSEj5FYQhm4dnm+yROfE0A,
alex.lemberg-XdAiOPVOjttBDgjK7y7TUQ,
mateusz.nowak-ral2JQCrhuEAvxtiuMwx3w,
Yuliy.Izrailov-XdAiOPVOjttBDgjK7y7TUQ,
asutoshd-sgV2jX0FEOL9JmXXK+q4OQ,
david.griego-QSEj5FYQhm4dnm+yROfE0A,
stummala-sgV2jX0FEOL9JmXXK+q4OQ, venkatg-sgV2jX0FEOL9JmXXK+q4OQ,
sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A,
pramod.gurav-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <1475678440-3525-4-git-send-email-riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On Wed, Oct 05, 2016 at 08:10:31PM +0530, Ritesh Harjani wrote:
> This adds support for sdhc-msm controllers to get supported
> clk-rates from DT. sdhci-msm would need it's own set_clock
> ops to be implemented. For this, supported clk-rates needs
> to be populated in sdhci_msm_pltfm_data.
>
> Signed-off-by: Ritesh Harjani <riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
> .../devicetree/bindings/mmc/sdhci-msm.txt | 1 +
> drivers/mmc/host/sdhci-msm.c | 48 ++++++++++++++++++++++
> 2 files changed, 49 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> index 485483a..6a83b38 100644
> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> @@ -17,6 +17,7 @@ Required properties:
> "iface" - Main peripheral bus clock (PCLK/HCLK - AHB Bus clock) (required)
> "core" - SDC MMC clock (MCLK) (required)
> "bus" - SDCC bus voter clock (optional)
> +- clk-rates: Array of supported GCC clock frequencies for sdhc, Units - Hz.
Why can't some combination of assigned-clock-rates and querying the
clock provider for rates be used here?
Minimally this would need unit suffix and either be made common or have
a vendor prefix.
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 11/12] mmc: sdhci-msm: Add calibration tuning for CDCLP533 circuit
From: Adrian Hunter @ 2016-10-10 12:49 UTC (permalink / raw)
To: Ritesh Harjani, ulf.hansson, linux-mmc, shawn.lin
Cc: david.brown, andy.gross, devicetree, linux-arm-msm, georgi.djakov,
alex.lemberg, mateusz.nowak, Yuliy.Izrailov, asutoshd,
david.griego, stummala, venkatg, sboyd, bjorn.andersson,
pramod.gurav
In-Reply-To: <1475678440-3525-12-git-send-email-riteshh@codeaurora.org>
On 05/10/16 17:40, Ritesh Harjani wrote:
> From: Venkat Gopalakrishnan <venkatg@codeaurora.org>
>
> In HS400 mode a new RCLK is introduced on the interface for read data
> transfers. The eMMC5.0 device transmits the read data to the host with
> respect to rising and falling edges of RCLK. In order to ensure correct
> operation of read data transfers in HS400 mode, the incoming RX data
> needs to be sampled by delayed version of RCLK.
>
> The CDCLP533 delay circuit shifts the RCLK by T/4. It needs to be
> initialized, configured and enabled once during HS400 mode switch and
> when operational voltage/clock is changed.
>
> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
> ---
> drivers/mmc/host/sdhci-msm.c | 178 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 178 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 612fa82..dbf80a9c 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -57,6 +57,7 @@
> #define CORE_DLL_PDN BIT(29)
> #define CORE_DLL_RST BIT(30)
> #define CORE_DLL_CONFIG 0x100
> +#define CORE_CMD_DAT_TRACK_SEL BIT(0)
> #define CORE_DLL_STATUS 0x108
>
> #define CORE_DLL_CONFIG_2 0x1b4
> @@ -72,8 +73,36 @@
> #define CORE_HC_SELECT_IN_HS400 (6 << 19)
> #define CORE_HC_SELECT_IN_MASK (7 << 19)
>
> +#define CORE_CSR_CDC_CTLR_CFG0 0x130
> +#define CORE_SW_TRIG_FULL_CALIB BIT(16)
> +#define CORE_HW_AUTOCAL_ENA BIT(17)
> +
> +#define CORE_CSR_CDC_CTLR_CFG1 0x134
> +#define CORE_CSR_CDC_CAL_TIMER_CFG0 0x138
> +#define CORE_TIMER_ENA BIT(16)
> +
> +#define CORE_CSR_CDC_CAL_TIMER_CFG1 0x13C
> +#define CORE_CSR_CDC_REFCOUNT_CFG 0x140
> +#define CORE_CSR_CDC_COARSE_CAL_CFG 0x144
> +#define CORE_CDC_OFFSET_CFG 0x14C
> +#define CORE_CSR_CDC_DELAY_CFG 0x150
> +#define CORE_CDC_SLAVE_DDA_CFG 0x160
> +#define CORE_CSR_CDC_STATUS0 0x164
> +#define CORE_CALIBRATION_DONE BIT(0)
> +
> +#define CORE_CDC_ERROR_CODE_MASK 0x7000000
> +
> +#define CORE_CSR_CDC_GEN_CFG 0x178
> +#define CORE_CDC_SWITCH_BYPASS_OFF BIT(0)
> +#define CORE_CDC_SWITCH_RC_EN BIT(1)
> +
> +#define CORE_DDR_200_CFG 0x184
> +#define CORE_CDC_T4_DLY_SEL BIT(0)
> +#define CORE_START_CDC_TRAFFIC BIT(6)
> +
> #define CORE_VENDOR_SPEC_CAPABILITIES0 0x11c
>
> +#define INVALID_TUNING_PHASE -1
> #define TCXO_FREQ 19200000
> #define SDHCI_MSM_MIN_CLOCK 400000
> #define CORE_FREQ_100MHZ (100 * 1000 * 1000)
> @@ -97,6 +126,7 @@ struct sdhci_msm_host {
> bool use_14lpp_dll_reset;
> bool tuning_done;
> bool calibration_done;
> + u8 saved_tuning_phase;
> };
>
> /* Platform specific tuning */
> @@ -426,6 +456,136 @@ static int msm_init_cm_dll(struct sdhci_host *host)
> return 0;
> }
>
> +static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> + u32 wait_cnt, config;
> + int ret;
> +
> + pr_debug("%s: Enter %s\n", mmc_hostname(host->mmc), __func__);
> +
> + /*
> + * Retuning in HS400 (DDR mode) will fail, just reset the
> + * tuning block and restore the saved tuning phase.
> + */
> + ret = msm_init_cm_dll(host);
> + if (ret)
> + goto out;
> +
> + /* Set the selected phase in delay line hw block */
> + ret = msm_config_cm_dll_phase(host, msm_host->saved_tuning_phase);
> + if (ret)
> + goto out;
> +
> + /* Write 1 to CMD_DAT_TRACK_SEL field in DLL_CONFIG */
> + config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
> + config |= CORE_CMD_DAT_TRACK_SEL;
> + writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
> +
> + /* Write 0 to CDC_T4_DLY_SEL field in VENDOR_SPEC_DDR200_CFG */
> + config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
> + config &= ~CORE_CDC_T4_DLY_SEL;
> + writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
> +
> + /* Write 0 to CDC_SWITCH_BYPASS_OFF field in CORE_CSR_CDC_GEN_CFG */
> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_GEN_CFG);
> + config &= ~CORE_CDC_SWITCH_BYPASS_OFF;
> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_GEN_CFG);
> +
> + /* Write 1 to CDC_SWITCH_RC_EN field in CORE_CSR_CDC_GEN_CFG */
> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_GEN_CFG);
> + config |= CORE_CDC_SWITCH_RC_EN;
> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_GEN_CFG);
> +
> + /* Write 0 to START_CDC_TRAFFIC field in CORE_DDR200_CFG */
> + config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
> + config &= ~CORE_START_CDC_TRAFFIC;
> + writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
> +
> + /*
> + * Perform CDC Register Initialization Sequence
> + *
> + * CORE_CSR_CDC_CTLR_CFG0 0x11800EC
> + * CORE_CSR_CDC_CTLR_CFG1 0x3011111
> + * CORE_CSR_CDC_CAL_TIMER_CFG0 0x1201000
> + * CORE_CSR_CDC_CAL_TIMER_CFG1 0x4
> + * CORE_CSR_CDC_REFCOUNT_CFG 0xCB732020
> + * CORE_CSR_CDC_COARSE_CAL_CFG 0xB19
> + * CORE_CSR_CDC_DELAY_CFG 0x3AC
> + * CORE_CDC_OFFSET_CFG 0x0
> + * CORE_CDC_SLAVE_DDA_CFG 0x16334
> + */
> +
> + writel_relaxed(0x11800EC, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
> + writel_relaxed(0x3011111, host->ioaddr + CORE_CSR_CDC_CTLR_CFG1);
> + writel_relaxed(0x1201000, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
> + writel_relaxed(0x4, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG1);
> + writel_relaxed(0xCB732020, host->ioaddr + CORE_CSR_CDC_REFCOUNT_CFG);
> + writel_relaxed(0xB19, host->ioaddr + CORE_CSR_CDC_COARSE_CAL_CFG);
> + writel_relaxed(0x3AC, host->ioaddr + CORE_CSR_CDC_DELAY_CFG);
> + writel_relaxed(0x0, host->ioaddr + CORE_CDC_OFFSET_CFG);
> + writel_relaxed(0x16334, host->ioaddr + CORE_CDC_SLAVE_DDA_CFG);
> +
> + /* CDC HW Calibration */
> +
> + /* Write 1 to SW_TRIG_FULL_CALIB field in CORE_CSR_CDC_CTLR_CFG0 */
> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
> + config |= CORE_SW_TRIG_FULL_CALIB;
> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
> +
> + /* Write 0 to SW_TRIG_FULL_CALIB field in CORE_CSR_CDC_CTLR_CFG0 */
> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
> + config &= ~CORE_SW_TRIG_FULL_CALIB;
> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
> +
> + /* Write 1 to HW_AUTOCAL_ENA field in CORE_CSR_CDC_CTLR_CFG0 */
> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
> + config |= CORE_HW_AUTOCAL_ENA;
> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
> +
> + /* Write 1 to TIMER_ENA field in CORE_CSR_CDC_CAL_TIMER_CFG0 */
> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
> + config |= CORE_TIMER_ENA;
> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
> +
> + wmb(); /* drain writebuffer */
> +
> + /* Poll on CALIBRATION_DONE field in CORE_CSR_CDC_STATUS0 to be 1 */
> + wait_cnt = 50;
> + while (!(readl_relaxed(host->ioaddr + CORE_CSR_CDC_STATUS0)
> + & CORE_CALIBRATION_DONE)) {
> + /* max. wait for 50us sec for CALIBRATION_DONE bit to be set */
> + if (--wait_cnt == 0) {
> + pr_err("%s: %s: CDC Calibration was not completed\n",
> + mmc_hostname(host->mmc), __func__);
> + ret = -ETIMEDOUT;
> + goto out;
> + }
> + /* wait for 1us before polling again */
> + udelay(1);
> + }
> +
> + /* Verify CDC_ERROR_CODE field in CORE_CSR_CDC_STATUS0 is 0 */
> + ret = readl_relaxed(host->ioaddr + CORE_CSR_CDC_STATUS0)
> + & CORE_CDC_ERROR_CODE_MASK;
> + if (ret) {
> + pr_err("%s: %s: CDC Error Code %d\n",
> + mmc_hostname(host->mmc), __func__, ret);
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + /* Write 1 to START_CDC_TRAFFIC field in CORE_DDR200_CFG */
> + config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
> + config |= CORE_START_CDC_TRAFFIC;
> + writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
> +out:
> + pr_debug("%s: Exit %s, ret:%d\n", mmc_hostname(host->mmc),
> + __func__, ret);
> + return ret;
> +}
> +
> static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
> {
> int tuning_seq_cnt = 3;
> @@ -433,6 +593,8 @@ static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
> int rc;
> struct mmc_host *mmc = host->mmc;
> struct mmc_ios ios = host->mmc->ios;
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>
> /*
> * Tuning is required for SDR104, HS200 and HS400 cards and
> @@ -457,6 +619,7 @@ retry:
> if (rc)
> return rc;
>
> + msm_host->saved_tuning_phase = phase;
> rc = mmc_send_tuning(mmc, opcode, NULL);
> if (!rc) {
> /* Tuning is successful at this tuning point */
> @@ -492,6 +655,8 @@ retry:
> rc = -EIO;
> }
>
> + if (!rc)
> + msm_host->tuning_done = true;
> return rc;
> }
>
> @@ -565,6 +730,17 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
> dev_dbg(mmc_dev(mmc), "%s: clock=%u uhs=%u ctrl_2=0x%x\n",
> mmc_hostname(host->mmc), host->clock, uhs, ctrl_2);
> sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
> +
> + spin_unlock_irq(&host->lock);
> +
> + /* CDCLP533 HW calibration is only required for HS400 mode*/
> + if (host->clock > CORE_FREQ_100MHZ &&
> + msm_host->tuning_done && !msm_host->calibration_done &&
> + (mmc->ios.timing == MMC_TIMING_MMC_HS400))
> + if (!sdhci_msm_cdclp533_calibration(host))
> + msm_host->calibration_done = true;
> +
> + spin_lock_irq(&host->lock);
> }
>
> static void sdhci_msm_voltage_switch(struct sdhci_host *host)
> @@ -907,6 +1083,8 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>
> sdhci_msm_populate_dt(&pdev->dev, msm_host);
>
> + msm_host->saved_tuning_phase = INVALID_TUNING_PHASE;
There is never a check for INVALID_TUNING_PHASE which begs the question: why
have it?
> +
> /* Setup SDCC bus voter clock. */
> msm_host->bus_clk = devm_clk_get(&pdev->dev, "bus");
> if (!IS_ERR(msm_host->bus_clk)) {
>
^ permalink raw reply
* Re: [PATCH 0/4] mmc: sdhci: Minor fixes
From: Ulf Hansson @ 2016-10-10 12:38 UTC (permalink / raw)
To: Adrian Hunter; +Cc: linux-mmc
In-Reply-To: <1475658684-32312-1-git-send-email-adrian.hunter@intel.com>
On 5 October 2016 at 11:11, Adrian Hunter <adrian.hunter@intel.com> wrote:
> Hi
>
> Here are a couple of minor fixes. While it would be good to have them in
> 4.9 or a 4.9-rc, they are not urgent.
>
>
> Adrian Hunter (4):
> mmc: sdhci: Fix SDHCI_QUIRK2_STOP_WITH_TC
> mmc: sdhci: Rename sdhci_set_power() to sdhci_set_power_noreg()
> mmc: sdhci-pci: Let devices define their own sdhci_ops
> mmc: sdhci-pci: Fix bus power failing to enable for some Intel controllers
>
> drivers/mmc/host/sdhci-pci-core.c | 54 ++++++++++++++++++++++++++++++++++++++-
> drivers/mmc/host/sdhci-pci.h | 2 ++
> drivers/mmc/host/sdhci-pxav3.c | 2 +-
> drivers/mmc/host/sdhci.c | 40 ++++++++++++++---------------
> drivers/mmc/host/sdhci.h | 2 ++
> 5 files changed, 77 insertions(+), 23 deletions(-)
>
>
> Regards
> Adrian
Thanks, applied for fixes!
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH] mmc: core, annotate cmd_hdr as __le32
From: Ulf Hansson @ 2016-10-10 12:38 UTC (permalink / raw)
To: Jiri Slaby; +Cc: linux-kernel@vger.kernel.org, linux-mmc, # 4.0+
In-Reply-To: <20161003085828.15913-1-jslaby@suse.cz>
+ stable
On 3 October 2016 at 10:58, Jiri Slaby <jslaby@suse.cz> wrote:
> Commit f68381a70b (mmc: block: fix packed command header endianness)
> correctly fixed endianness handling of packed_cmd_hdr in
> mmc_blk_packed_hdr_wrq_prep.
>
> But now, sparse complains about incorrect types:
> drivers/mmc/card/block.c:1613:27: sparse: incorrect type in assignment (different base types)
> drivers/mmc/card/block.c:1613:27: expected unsigned int [unsigned] [usertype] <noident>
> drivers/mmc/card/block.c:1613:27: got restricted __le32 [usertype] <noident>
> ...
>
> So annotate cmd_hdr properly using __le32 to make everyone happy.
>
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: linux-mmc@vger.kernel.org
Checkpatch reported an error regarding the changelog, I fixed it up so
no worries! I decided to also added a fixes and stable tag.
Thanks, applied for fixes!
Kind regards
Uffe
> ---
> drivers/mmc/card/block.c | 2 +-
> drivers/mmc/card/queue.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index c3335112e68c..9b71414526e7 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -1786,7 +1786,7 @@ static void mmc_blk_packed_hdr_wrq_prep(struct mmc_queue_req *mqrq,
> struct mmc_blk_data *md = mq->data;
> struct mmc_packed *packed = mqrq->packed;
> bool do_rel_wr, do_data_tag;
> - u32 *packed_cmd_hdr;
> + __le32 *packed_cmd_hdr;
> u8 hdr_blocks;
> u8 i = 1;
>
> diff --git a/drivers/mmc/card/queue.h b/drivers/mmc/card/queue.h
> index 3c15a75bae86..342f1e3f301e 100644
> --- a/drivers/mmc/card/queue.h
> +++ b/drivers/mmc/card/queue.h
> @@ -31,7 +31,7 @@ enum mmc_packed_type {
>
> struct mmc_packed {
> struct list_head list;
> - u32 cmd_hdr[1024];
> + __le32 cmd_hdr[1024];
> unsigned int blocks;
> u8 nr_entries;
> u8 retries;
> --
> 2.10.0
>
^ permalink raw reply
* Re: [PATCH v3 3/3] mmc: sdhci-of-arasan: add sdhci_arasan_voltage_switch for arasan,5.1
From: Ulf Hansson @ 2016-10-10 12:38 UTC (permalink / raw)
To: Shawn Lin
Cc: open list:ARM/Rockchip SoC..., Ziyuan Xu, linux-mmc,
Adrian Hunter, Doug Anderson
In-Reply-To: <1475216340-15344-3-git-send-email-shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
On 30 September 2016 at 08:19, Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote:
> Per the vendor's requirement, we shouldn't do any setting for
> 1.8V Signaling Enable, otherwise the interaction/behaviour between
> phy and controller will be undefined. Mostly it works fine if we do
> that, but we still see failures. Anyway, let's fix it to meet the
> vendor's requirement. The error log looks like:
>
> [ 93.405085] mmc1: unexpected status 0x800900 after switch
> [ 93.408474] mmc1: switch to bus width 1 failed
> [ 93.408482] mmc1: mmc_select_hs200 failed, error -110
> [ 93.408492] mmc1: error -110 during resume (card was removed?)
> [ 93.408705] PM: resume of devices complete after 213.453 msecs
>
> Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>
> Acked-by: Adrian Hunter <adrian.hunter-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Reviewed-by: Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Thanks, applied for fixes!
Kind regards
Uffe
> ---
>
> Changes in v3:
> - add tag from Adrian and Doug
>
> Changes in v2: None
>
> drivers/mmc/host/sdhci-of-arasan.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
> index da8e40a..1573a8d 100644
> --- a/drivers/mmc/host/sdhci-of-arasan.c
> +++ b/drivers/mmc/host/sdhci-of-arasan.c
> @@ -265,6 +265,28 @@ void sdhci_arasan_reset(struct sdhci_host *host, u8 mask)
> }
> }
>
> +static int sdhci_arasan_voltage_switch(struct mmc_host *mmc,
> + struct mmc_ios *ios)
> +{
> + switch (ios->signal_voltage) {
> + case MMC_SIGNAL_VOLTAGE_180:
> + /*
> + * Plese don't switch to 1V8 as arasan,5.1 doesn't
> + * actually refer to this setting to indicate the
> + * signal voltage and the state machine will be broken
> + * actually if we force to enable 1V8. That's something
> + * like broken quirk but we could work around here.
> + */
> + return 0;
> + case MMC_SIGNAL_VOLTAGE_330:
> + case MMC_SIGNAL_VOLTAGE_120:
> + /* We don't support 3V3 and 1V2 */
> + break;
> + }
> +
> + return -EINVAL;
> +}
> +
> static struct sdhci_ops sdhci_arasan_ops = {
> .set_clock = sdhci_arasan_set_clock,
> .get_max_clock = sdhci_pltfm_clk_get_max_clock,
> @@ -661,6 +683,8 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
>
> host->mmc_host_ops.hs400_enhanced_strobe =
> sdhci_arasan_hs400_enhanced_strobe;
> + host->mmc_host_ops.start_signal_voltage_switch =
> + sdhci_arasan_voltage_switch;
> }
>
> ret = sdhci_add_host(host);
> --
> 2.3.7
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 2/3] mmc: core: changes frequency to hs_max_dtr when selecting hs400es
From: Ulf Hansson @ 2016-10-10 12:38 UTC (permalink / raw)
To: Shawn Lin
Cc: open list:ARM/Rockchip SoC..., Ziyuan Xu, linux-mmc,
Adrian Hunter, Doug Anderson
In-Reply-To: <1475216340-15344-2-git-send-email-shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
On 30 September 2016 at 08:18, Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote:
> Per JESD84-B51 P49, Host need to change frequency to <=52MHz
> after setting HS_TIMING to 0x1, and host may changes frequency
> to <= 200MHz after setting HS_TIMING to 0x3. That means the card
> expects the clock rate to increase from the current used f_init
> (which is less than 400KHz, but still being less than 52MHz) to
> 52MHz, otherwise we find some eMMC devices significantly report
> failure when sending status.
>
> Reported-by: Xiao Yao <xiaoyao-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>
> Reviewed-by: Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Thanks, applied for fixes!
Kind regards
Uffe
> ---
>
> Changes in v3:
> - add Doug's tag and fix the wrong page index of spec
>
> Changes in v2:
> - improve the changelog
>
> drivers/mmc/core/mmc.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index f4ed5ac..39fc5b2 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1282,6 +1282,8 @@ static int mmc_select_hs400es(struct mmc_card *card)
> if (err)
> goto out_err;
>
> + mmc_set_clock(host, card->ext_csd.hs_max_dtr);
> +
> err = mmc_switch_status(card);
> if (err)
> goto out_err;
> --
> 2.3.7
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox