* 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
* Re: [PATCH v3 1/3] mmc: core: switch to 1V8 or 1V2 for hs400es mode
From: Ulf Hansson @ 2016-10-10 12:38 UTC (permalink / raw)
To: Shawn Lin
Cc: Ziyuan Xu, linux-mmc, Adrian Hunter, # 4.0+, Doug Anderson,
open list:ARM/Rockchip SoC...
In-Reply-To: <1475216340-15344-1-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:
> When introducing hs400es, I didn't notice that we haven't
> switched voltage to 1V2 or 1V8 for it. That happens to work
> as the first controller claiming to support hs400es, arasan(5.1),
> which is designed to only support 1V8. So the voltage is fixed to 1V8.
> But it actually is wrong, and will not fit for other host controllers.
> Let's fix it.
>
> Fixes: commit 81ac2af65793ecf ("mmc: core: implement enhanced strobe support")
> Cc: <stable-u79uwXL29TY76Z2rM5mHXA@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
>
> Changes in v2:
> - fix copy-paste error, mea culpa
>
> drivers/mmc/core/mmc.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 3486bc7..f4ed5ac 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1263,6 +1263,16 @@ static int mmc_select_hs400es(struct mmc_card *card)
> goto out_err;
> }
>
> + if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS400_1_2V)
> + err = __mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_120);
> +
> + if (err && card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS400_1_8V)
> + err = __mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_180);
> +
> + /* If fails try again during next card power cycle */
> + if (err)
> + goto out_err;
> +
> err = mmc_select_bus_width(card);
> if (err < 0)
> 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
* Re: [PATCH 1/2] mmc: block: add missing header dependencies
From: Ulf Hansson @ 2016-10-10 12:37 UTC (permalink / raw)
To: Baoyou Xie
Cc: Arnd Bergmann, tang.qiang007, xie.baoyou, linux-mmc,
linux-kernel@vger.kernel.org, Michal Simek, han.fei,
Sören Brinkmann, Adrian Hunter,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <1475199459-4775-1-git-send-email-baoyou.xie@linaro.org>
On 30 September 2016 at 03:37, Baoyou Xie <baoyou.xie@linaro.org> wrote:
> We get 1 warning when building kernel with W=1:
> drivers/mmc/card/block.c:2147:5: warning: no previous prototype for 'mmc_blk_issue_rq' [-Wmissing-prototypes]
>
> In fact, this function is declared in drivers/mmc/card/block.h,
> so this patch adds missing header dependencies.
>
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
Thanks, applied for fixes!
Kind regards
Uffe
> ---
> drivers/mmc/card/block.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index c333511..0f2cc9f2 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -46,6 +46,7 @@
> #include <asm/uaccess.h>
>
> #include "queue.h"
> +#include "block.h"
>
> MODULE_ALIAS("mmc:block");
> #ifdef MODULE_PARAM_PREFIX
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH -next] mmc: sdhci-of-arasan: Fix non static symbol warning
From: Ulf Hansson @ 2016-10-10 12:37 UTC (permalink / raw)
To: Wei Yongjun
Cc: linux-mmc, Michal Simek, Adrian Hunter, Wei Yongjun,
Sören Brinkmann, linux-arm-kernel@lists.infradead.org
In-Reply-To: <1474818243-19207-1-git-send-email-weiyj.lk@gmail.com>
On 25 September 2016 at 17:44, Wei Yongjun <weiyj.lk@gmail.com> wrote:
> From: Wei Yongjun <weiyongjun1@huawei.com>
>
> Fixes the following sparse warning:
>
> drivers/mmc/host/sdhci-of-arasan.c:253:6: warning:
> symbol 'sdhci_arasan_reset' was not declared. Should it be static?
>
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Thanks, applied for fixes!
Kind regards
Uffe
> ---
> drivers/mmc/host/sdhci-of-arasan.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
> index da8e40a..e263671 100644
> --- a/drivers/mmc/host/sdhci-of-arasan.c
> +++ b/drivers/mmc/host/sdhci-of-arasan.c
> @@ -250,7 +250,7 @@ static void sdhci_arasan_hs400_enhanced_strobe(struct mmc_host *mmc,
> writel(vendor, host->ioaddr + SDHCI_ARASAN_VENDOR_REGISTER);
> }
>
> -void sdhci_arasan_reset(struct sdhci_host *host, u8 mask)
> +static void sdhci_arasan_reset(struct sdhci_host *host, u8 mask)
> {
> u8 ctrl;
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>
>
>
^ permalink raw reply
* Re: [PATCH v5 10/12] mmc: sdhci-msm: Add HS400 platform support
From: Adrian Hunter @ 2016-10-10 12:08 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-11-git-send-email-riteshh@codeaurora.org>
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
> 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.
> +
> + /*
> + * 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?
> + /*
> + * 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.
> +
> 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) >>
>
^ permalink raw reply
* Re: [PATCH v5 07/12] mmc: sdhci-msm: Implement set_clock callback for sdhci-msm
From: Ritesh Harjani @ 2016-10-10 11:17 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: <d35224cf-52e0-5ccc-9596-1c338df41c36-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Hi Adrian,
On 10/10/2016 3:53 PM, Adrian Hunter wrote:
> On 10/10/16 13:16, Adrian Hunter wrote:
>> On 05/10/16 17:40, Ritesh Harjani wrote:
>>> sdhci-msm controller may have different clk-rates for each
>>> bus speed mode. Thus implement set_clock callback for
>>> sdhci-msm driver.
>>>
>>> Signed-off-by: Sahitya Tummala <stummala-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>> Signed-off-by: Ritesh Harjani <riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>> ---
>>> drivers/mmc/host/sdhci-msm.c | 110 ++++++++++++++++++++++++++++++++++++++++++-
>>> 1 file changed, 109 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>>> index 542ddad..9d18cf0 100644
>>> --- a/drivers/mmc/host/sdhci-msm.c
>>> +++ b/drivers/mmc/host/sdhci-msm.c
>>> @@ -84,6 +84,7 @@ struct sdhci_msm_host {
>>> struct clk *bus_clk; /* SDHC bus voter clock */
>>> u32 *clk_table;
>>> int clk_table_sz;
>>> + u32 clk_rate;
>>> struct mmc_host *mmc;
>>> bool use_14lpp_dll_reset;
>>> };
>>> @@ -588,6 +589,113 @@ static unsigned int sdhci_msm_get_min_clock(struct sdhci_host *host)
>>> }
>>> }
>>>
>>> +static unsigned int sdhci_msm_get_msm_clk_rate(struct sdhci_host *host,
>>> + u32 req_clk)
>>> +{
>>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>>> + int count;
>>> + unsigned int sel_clk = -1;
>>> +
>>> + if (!msm_host->clk_table)
>>> + return clk_round_rate(msm_host->clk, ULONG_MAX);
>>> +
>>> + count = msm_host->clk_table_sz;
>>> +
>>> + while (count--) {
>>> + sel_clk = msm_host->clk_table[count];
>>> + if (req_clk >= sel_clk)
>>> + return sel_clk;
>>> + }
>>> +
>>> + return sel_clk;
>>> +}
>>> +
>>> +/**
>>> + * __sdhci_msm_set_clock - sdhci_msm clock control.
>>> + *
>>> + * Description:
>>> + * Implement MSM version of sdhci_set_clock.
>>> + * This is required since MSM controller does not
>>> + * use internal divider and instead directly control
>>> + * the GCC clock as per HW recommendation.
>>> + **/
>>> +void __sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>>> +{
>>> + u16 clk;
>>> + unsigned long timeout;
>>> +
>>> + /*
>>> + * Keep actual_clock as zero -
>>> + * - since there is no divider used so no need of having actual_clock.
>>> + * - MSM controller uses SDCLK for data timeout calculation. If
>>> + * actual_clock is zero, host->clock is taken for calculation.
>>> + */
>>> + host->mmc->actual_clock = 0;
>>> +
>>> + sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
>>> +
>>> + if (clock == 0)
>>> + return;
>>> +
>>> + /*
>>> + * MSM controller do not use clock divider.
>>> + * Thus read SDHCI_CLOCK_CONTROL and only enable
>>> + * clock with no divider value programmed.
>>> + */
>>> + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
>>> +
>>> + clk |= SDHCI_CLOCK_INT_EN;
>>> + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>>> +
>>> + /* Wait max 20 ms */
>>> + timeout = 20;
>>> + while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
>>> + & SDHCI_CLOCK_INT_STABLE)) {
>>> + if (timeout == 0) {
>>> + pr_err("%s: Internal clock never stabilised.\n",
>>> + mmc_hostname(host->mmc));
>>> + return;
>>> + }
>>> + timeout--;
>>> + mdelay(1);
>>> + }
>>> +
>>> + clk |= SDHCI_CLOCK_CARD_EN;
>>> + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>>> +}
>>> +
>>> +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);
>>> + u32 msm_clock;
>>> + int rc;
>>> +
>>> + if (!clock)
>>
>> Wouldn't you still need to set msm_host->clk_rate = clock in this case
Yes. Will do that.
>>
>>> + goto out;
>>> +
>>> + spin_unlock_irq(&host->lock);
>>> + if ((clock != msm_host->clk_rate) && msm_host->clk_table) {
>>> + msm_clock = sdhci_msm_get_msm_clk_rate(host, clock);
>>> + rc = clk_set_rate(msm_host->clk, msm_clock);
>>> + if (rc) {
>>> + pr_err("%s: failed to set clock at rate %u, requested clock rate %u\n",
>>> + mmc_hostname(host->mmc), msm_clock, clock);
>>> + goto out;
>
> 'goto out' leaves spinlock unlocked
Thanks for catching it.
>
>>> + }
>>> + msm_host->clk_rate = clock;
>>> + pr_debug("%s: setting clock at rate %lu\n",
>>> + mmc_hostname(host->mmc), clk_get_rate(msm_host->clk));
>>> + }
>>> +
>>> + spin_lock_irq(&host->lock);
>>> +out:
>>> + if (!msm_host->clk_table)
>>> + return sdhci_set_clock(host, clock);
>>
>> Could put the above 2 lines at the start and then no need to check
>> msm_host->clk_table again.
I wanted to make this change, but then may be because of later patches
which modifies sdhci_msm_set_clock for UHS modes, I did not move this to
top.
I reviewed that part again, I will make the necessary changes.
Thanks.
>>
>>> + __sdhci_msm_set_clock(host, clock);
>>> +}
>>> +
>>> static const struct of_device_id sdhci_msm_dt_match[] = {
>>> { .compatible = "qcom,sdhci-msm-v4" },
>>> {},
>>> @@ -598,7 +706,7 @@ MODULE_DEVICE_TABLE(of, sdhci_msm_dt_match);
>>> static const struct sdhci_ops sdhci_msm_ops = {
>>> .platform_execute_tuning = sdhci_msm_execute_tuning,
>>> .reset = sdhci_reset,
>>> - .set_clock = sdhci_set_clock,
>>> + .set_clock = sdhci_msm_set_clock,
>>> .get_min_clock = sdhci_msm_get_min_clock,
>>> .get_max_clock = sdhci_msm_get_max_clock,
>>> .set_bus_width = sdhci_set_bus_width,
>>>
>>
>>
>
> --
> 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
>
--
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 05/12] mmc: sdhci-msm: Add get_min_clock() and get_max_clock() callback
From: Ritesh Harjani @ 2016-10-10 11:05 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: <54b602df-9c23-dc81-fd51-d04b441b99c0@intel.com>
Hi Adrian,
On 10/10/2016 3:16 PM, Adrian Hunter wrote:
> On 05/10/16 17:40, Ritesh Harjani wrote:
>> This add get_min_clock() and get_max_clock() callback
>> for sdhci-msm. sdhci-msm min/max clocks may be different
>> hence implement these callbacks.
>>
>> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>> ---
>> drivers/mmc/host/sdhci-msm.c | 29 +++++++++++++++++++++++++++++
>> 1 file changed, 29 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index 042ecb2..4e17201 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -68,6 +68,7 @@
>> #define CORE_VENDOR_SPEC_CAPABILITIES0 0x11c
>>
>> #define TCXO_FREQ 19200000
>> +#define SDHCI_MSM_MIN_CLOCK 400000
>>
>> #define CDR_SELEXT_SHIFT 20
>> #define CDR_SELEXT_MASK (0xf << CDR_SELEXT_SHIFT)
>> @@ -561,6 +562,32 @@ static irqreturn_t sdhci_msm_pwr_irq(int irq, void *data)
>> return IRQ_HANDLED;
>> }
>>
>> +static unsigned int sdhci_msm_get_max_clock(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 count;
>> +
>> + if (msm_host->clk_table) {
>
> I don't know if DT allows empty arrays, but if it does, might not
> msm_host->clk_table_sz be zero here.
Sure, I will take care of this as mentioned in the response of previous
patch 03.
>
>> + count = msm_host->clk_table_sz;
>> + return msm_host->clk_table[count - 1];
>> + } else {
>
> 'else' is redundant here.
Done. Will remove it.
>
>> + return clk_round_rate(msm_host->clk, ULONG_MAX);
>> + }
>> +}
>> +
>> +static unsigned int sdhci_msm_get_min_clock(struct sdhci_host *host)
>> +{
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> +
>> + if (msm_host->clk_table) {
>
> Same about possibility of msm_host->clk_table_sz being zero.
Yes, as mentioned. Will take care of this.
>
>> + return msm_host->clk_table[0];
>> + } else {
>
> 'else' is redundant here.
Done.
>
>> + return SDHCI_MSM_MIN_CLOCK;
>> + }
>> +}
>> +
>> static const struct of_device_id sdhci_msm_dt_match[] = {
>> { .compatible = "qcom,sdhci-msm-v4" },
>> {},
>> @@ -572,6 +599,8 @@ static const struct sdhci_ops sdhci_msm_ops = {
>> .platform_execute_tuning = sdhci_msm_execute_tuning,
>> .reset = sdhci_reset,
>> .set_clock = sdhci_set_clock,
>> + .get_min_clock = sdhci_msm_get_min_clock,
>> + .get_max_clock = sdhci_msm_get_max_clock,
>> .set_bus_width = sdhci_set_bus_width,
>> .set_uhs_signaling = sdhci_msm_set_uhs_signaling,
>> .voltage_switch = sdhci_msm_voltage_switch,
>>
>
> --
> 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 03/12] mmc: sdhci-msm: add pltfm_data support to get clk-rates from DT
From: Ritesh Harjani @ 2016-10-10 11:00 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: <7e5c2bfe-0a67-71e2-d083-49d9a712482e-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Hi Adrian,
Thanks for the review. I will address your comments.
Responses inline.
--
Regards
Ritesh
On 10/10/2016 3:05 PM, Adrian Hunter wrote:
> On 05/10/16 17:40, 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.
>>
>> Example:
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index 85ddaae..042ecb2 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -81,6 +81,8 @@ struct sdhci_msm_host {
>> struct clk *clk; /* main SD/MMC bus clock */
>> struct clk *pclk; /* SDHC peripheral bus clock */
>> struct clk *bus_clk; /* SDHC bus voter clock */
>> + u32 *clk_table;
>> + int clk_table_sz;
>> struct mmc_host *mmc;
>> bool use_14lpp_dll_reset;
>> };
>> @@ -582,6 +584,50 @@ static const struct sdhci_pltfm_data sdhci_msm_pdata = {
>> .ops = &sdhci_msm_ops,
>> };
>>
>> +static int sdhci_msm_dt_get_array(struct device *dev, const char *prop_name,
>> + u32 **table, int *size)
>
> It is nice to align to the open parenthesis. Have a look at the checks from
> checkpatch --strict
Sure, will do that.
>
>> +{
>> + struct device_node *np = dev->of_node;
>> + int count, ret;
>> + u32 *arr;
>> +
>> + count = of_property_count_elems_of_size(np, prop_name, sizeof(u32));
>
> Shouldn't this be of_property_count_u32_elems()
Yes, I will make the change.
>
>> + if (count < 0) {
will change this to below to avoid empty property problem in DT which
you mentioned in the next patch.
if (count <= 0) {
>> + dev_warn(dev, "%s: Invalid dt property, err(%d)\n",
>> + prop_name, count);
>> + return count;
>> + }
>> +
>> + arr = kcalloc(count, sizeof(*arr), GFP_KERNEL);
>> + if (!arr)
>> + return -ENOMEM;
>> +
>> + ret = of_property_read_u32_array(np, prop_name, arr, count);
>> + if (ret) {
>> + kfree(arr);
>> + dev_warn(dev, "%s Invalid dt array property, err(%d)\n",
>
> 'err(%d)' is an unusual style for printing error numbers. 'error %d' looks
> better.
Done.
>
> Also in some messages you have 'DT' and others 'dt'. Also here it is '%s'
> but above '%s:', and some messages start with a lower case letter and some
> upper case. Please try to make everything consistent.
>
Sure thanks.
>> + prop_name, ret);
>> + return ret;
>> + }
>> + *table = arr;
>> + *size = count;
>> + return 0;
>> +}
>> +
>> +void sdhci_msm_populate_dt(struct device *dev,
>> + struct sdhci_msm_host *msm_host)
>
> Align to open parenthesis
Done.
>
>> +{
>> + int table_sz = 0;
>> + u32 *table = NULL;
>> +
>> + if (sdhci_msm_dt_get_array(dev, "clk-rates", &table, &table_sz)) {
>> + dev_warn(dev, "failed in DT parsing for supported clk-rates\n");
>> + return;
>> + }
>> + msm_host->clk_table = table;
>> + msm_host->clk_table_sz = table_sz;
>> +}
>> +
>> static int sdhci_msm_probe(struct platform_device *pdev)
>> {
>> struct sdhci_host *host;
>> @@ -608,6 +654,8 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>>
>> sdhci_get_of_property(pdev);
>>
>> + sdhci_msm_populate_dt(&pdev->dev, msm_host);
>> +
>> /* 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
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 08/12] mmc: sdhci-msm: Add clock changes for DDR mode.
From: Adrian Hunter @ 2016-10-10 10:26 UTC (permalink / raw)
To: Ritesh Harjani, 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: <1475678440-3525-9-git-send-email-riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On 05/10/16 17:40, Ritesh Harjani wrote:
> SDHC MSM controller need 2x clock for MCLK at GCC.
> Hence make required changes to have 2x clock for
> DDR timing modes.
>
> Signed-off-by: Ritesh Harjani <riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Acked-by: Adrian Hunter <adrian.hunter-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
> drivers/mmc/host/sdhci-msm.c | 25 ++++++++++++++++++++-----
> 1 file changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 9d18cf0..eb1a9e3 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -669,6 +669,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;
> int rc;
>
> @@ -676,15 +677,29 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
> goto out;
>
> spin_unlock_irq(&host->lock);
> - if ((clock != msm_host->clk_rate) && msm_host->clk_table) {
> - msm_clock = sdhci_msm_get_msm_clk_rate(host, clock);
> + if (((curr_ios.timing == MMC_TIMING_UHS_DDR50) ||
> + (curr_ios.timing == MMC_TIMING_MMC_DDR52) ||
> + (curr_ios.timing == MMC_TIMING_MMC_HS400)) &&
> + msm_host->clk_table) {
> + /*
> + * The SDHC requires internal clock frequency to be double the
> + * actual clock that will be set for DDR mode. The controller
> + * uses the faster clock(100/400MHz) for some of its parts and
> + * send the actual required clock (50/200MHz) to the card.
> + */
> + clock *= 2;
> + }
> + msm_clock = sdhci_msm_get_msm_clk_rate(host, clock);
> +
> + if ((msm_clock != msm_host->clk_rate) && msm_host->clk_table) {
> rc = clk_set_rate(msm_host->clk, msm_clock);
> if (rc) {
> - pr_err("%s: failed to set clock at rate %u, requested clock rate %u\n",
> - mmc_hostname(host->mmc), msm_clock, clock);
> + pr_err("%s: failed to set clock at rate %u at timing %d\n",
> + mmc_hostname(host->mmc), msm_clock,
> + curr_ios.timing);
> goto out;
> }
> - msm_host->clk_rate = clock;
> + msm_host->clk_rate = msm_clock;
> pr_debug("%s: setting clock at rate %lu\n",
> mmc_hostname(host->mmc), clk_get_rate(msm_host->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 07/12] mmc: sdhci-msm: Implement set_clock callback for sdhci-msm
From: Adrian Hunter @ 2016-10-10 10:23 UTC (permalink / raw)
To: Adrian Hunter, 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: <20faeedb-ae65-fab7-f52f-aaec2aebe755@intel.com>
On 10/10/16 13:16, Adrian Hunter wrote:
> On 05/10/16 17:40, Ritesh Harjani wrote:
>> sdhci-msm controller may have different clk-rates for each
>> bus speed mode. Thus implement set_clock callback for
>> sdhci-msm driver.
>>
>> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>> ---
>> drivers/mmc/host/sdhci-msm.c | 110 ++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 109 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index 542ddad..9d18cf0 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -84,6 +84,7 @@ struct sdhci_msm_host {
>> struct clk *bus_clk; /* SDHC bus voter clock */
>> u32 *clk_table;
>> int clk_table_sz;
>> + u32 clk_rate;
>> struct mmc_host *mmc;
>> bool use_14lpp_dll_reset;
>> };
>> @@ -588,6 +589,113 @@ static unsigned int sdhci_msm_get_min_clock(struct sdhci_host *host)
>> }
>> }
>>
>> +static unsigned int sdhci_msm_get_msm_clk_rate(struct sdhci_host *host,
>> + u32 req_clk)
>> +{
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> + int count;
>> + unsigned int sel_clk = -1;
>> +
>> + if (!msm_host->clk_table)
>> + return clk_round_rate(msm_host->clk, ULONG_MAX);
>> +
>> + count = msm_host->clk_table_sz;
>> +
>> + while (count--) {
>> + sel_clk = msm_host->clk_table[count];
>> + if (req_clk >= sel_clk)
>> + return sel_clk;
>> + }
>> +
>> + return sel_clk;
>> +}
>> +
>> +/**
>> + * __sdhci_msm_set_clock - sdhci_msm clock control.
>> + *
>> + * Description:
>> + * Implement MSM version of sdhci_set_clock.
>> + * This is required since MSM controller does not
>> + * use internal divider and instead directly control
>> + * the GCC clock as per HW recommendation.
>> + **/
>> +void __sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
>> +{
>> + u16 clk;
>> + unsigned long timeout;
>> +
>> + /*
>> + * Keep actual_clock as zero -
>> + * - since there is no divider used so no need of having actual_clock.
>> + * - MSM controller uses SDCLK for data timeout calculation. If
>> + * actual_clock is zero, host->clock is taken for calculation.
>> + */
>> + host->mmc->actual_clock = 0;
>> +
>> + sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
>> +
>> + if (clock == 0)
>> + return;
>> +
>> + /*
>> + * MSM controller do not use clock divider.
>> + * Thus read SDHCI_CLOCK_CONTROL and only enable
>> + * clock with no divider value programmed.
>> + */
>> + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
>> +
>> + clk |= SDHCI_CLOCK_INT_EN;
>> + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>> +
>> + /* Wait max 20 ms */
>> + timeout = 20;
>> + while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
>> + & SDHCI_CLOCK_INT_STABLE)) {
>> + if (timeout == 0) {
>> + pr_err("%s: Internal clock never stabilised.\n",
>> + mmc_hostname(host->mmc));
>> + return;
>> + }
>> + timeout--;
>> + mdelay(1);
>> + }
>> +
>> + clk |= SDHCI_CLOCK_CARD_EN;
>> + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>> +}
>> +
>> +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);
>> + u32 msm_clock;
>> + int rc;
>> +
>> + if (!clock)
>
> Wouldn't you still need to set msm_host->clk_rate = clock in this case
>
>> + goto out;
>> +
>> + spin_unlock_irq(&host->lock);
>> + if ((clock != msm_host->clk_rate) && msm_host->clk_table) {
>> + msm_clock = sdhci_msm_get_msm_clk_rate(host, clock);
>> + rc = clk_set_rate(msm_host->clk, msm_clock);
>> + if (rc) {
>> + pr_err("%s: failed to set clock at rate %u, requested clock rate %u\n",
>> + mmc_hostname(host->mmc), msm_clock, clock);
>> + goto out;
'goto out' leaves spinlock unlocked
>> + }
>> + msm_host->clk_rate = clock;
>> + pr_debug("%s: setting clock at rate %lu\n",
>> + mmc_hostname(host->mmc), clk_get_rate(msm_host->clk));
>> + }
>> +
>> + spin_lock_irq(&host->lock);
>> +out:
>> + if (!msm_host->clk_table)
>> + return sdhci_set_clock(host, clock);
>
> Could put the above 2 lines at the start and then no need to check
> msm_host->clk_table again.
>
>> + __sdhci_msm_set_clock(host, clock);
>> +}
>> +
>> static const struct of_device_id sdhci_msm_dt_match[] = {
>> { .compatible = "qcom,sdhci-msm-v4" },
>> {},
>> @@ -598,7 +706,7 @@ MODULE_DEVICE_TABLE(of, sdhci_msm_dt_match);
>> static const struct sdhci_ops sdhci_msm_ops = {
>> .platform_execute_tuning = sdhci_msm_execute_tuning,
>> .reset = sdhci_reset,
>> - .set_clock = sdhci_set_clock,
>> + .set_clock = sdhci_msm_set_clock,
>> .get_min_clock = sdhci_msm_get_min_clock,
>> .get_max_clock = sdhci_msm_get_max_clock,
>> .set_bus_width = sdhci_set_bus_width,
>>
>
>
^ permalink raw reply
* Re: [PATCH v5 07/12] mmc: sdhci-msm: Implement set_clock callback for sdhci-msm
From: Adrian Hunter @ 2016-10-10 10:16 UTC (permalink / raw)
To: Ritesh Harjani, 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: <1475678440-3525-8-git-send-email-riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On 05/10/16 17:40, Ritesh Harjani wrote:
> sdhci-msm controller may have different clk-rates for each
> bus speed mode. Thus implement set_clock callback for
> sdhci-msm driver.
>
> Signed-off-by: Sahitya Tummala <stummala-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Signed-off-by: Ritesh Harjani <riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
> drivers/mmc/host/sdhci-msm.c | 110 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 109 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 542ddad..9d18cf0 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -84,6 +84,7 @@ struct sdhci_msm_host {
> struct clk *bus_clk; /* SDHC bus voter clock */
> u32 *clk_table;
> int clk_table_sz;
> + u32 clk_rate;
> struct mmc_host *mmc;
> bool use_14lpp_dll_reset;
> };
> @@ -588,6 +589,113 @@ static unsigned int sdhci_msm_get_min_clock(struct sdhci_host *host)
> }
> }
>
> +static unsigned int sdhci_msm_get_msm_clk_rate(struct sdhci_host *host,
> + u32 req_clk)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> + int count;
> + unsigned int sel_clk = -1;
> +
> + if (!msm_host->clk_table)
> + return clk_round_rate(msm_host->clk, ULONG_MAX);
> +
> + count = msm_host->clk_table_sz;
> +
> + while (count--) {
> + sel_clk = msm_host->clk_table[count];
> + if (req_clk >= sel_clk)
> + return sel_clk;
> + }
> +
> + return sel_clk;
> +}
> +
> +/**
> + * __sdhci_msm_set_clock - sdhci_msm clock control.
> + *
> + * Description:
> + * Implement MSM version of sdhci_set_clock.
> + * This is required since MSM controller does not
> + * use internal divider and instead directly control
> + * the GCC clock as per HW recommendation.
> + **/
> +void __sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
> +{
> + u16 clk;
> + unsigned long timeout;
> +
> + /*
> + * Keep actual_clock as zero -
> + * - since there is no divider used so no need of having actual_clock.
> + * - MSM controller uses SDCLK for data timeout calculation. If
> + * actual_clock is zero, host->clock is taken for calculation.
> + */
> + host->mmc->actual_clock = 0;
> +
> + sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
> +
> + if (clock == 0)
> + return;
> +
> + /*
> + * MSM controller do not use clock divider.
> + * Thus read SDHCI_CLOCK_CONTROL and only enable
> + * clock with no divider value programmed.
> + */
> + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
> +
> + clk |= SDHCI_CLOCK_INT_EN;
> + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> +
> + /* Wait max 20 ms */
> + timeout = 20;
> + while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
> + & SDHCI_CLOCK_INT_STABLE)) {
> + if (timeout == 0) {
> + pr_err("%s: Internal clock never stabilised.\n",
> + mmc_hostname(host->mmc));
> + return;
> + }
> + timeout--;
> + mdelay(1);
> + }
> +
> + clk |= SDHCI_CLOCK_CARD_EN;
> + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> +}
> +
> +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);
> + u32 msm_clock;
> + int rc;
> +
> + if (!clock)
Wouldn't you still need to set msm_host->clk_rate = clock in this case
> + goto out;
> +
> + spin_unlock_irq(&host->lock);
> + if ((clock != msm_host->clk_rate) && msm_host->clk_table) {
> + msm_clock = sdhci_msm_get_msm_clk_rate(host, clock);
> + rc = clk_set_rate(msm_host->clk, msm_clock);
> + if (rc) {
> + pr_err("%s: failed to set clock at rate %u, requested clock rate %u\n",
> + mmc_hostname(host->mmc), msm_clock, clock);
> + goto out;
> + }
> + msm_host->clk_rate = clock;
> + pr_debug("%s: setting clock at rate %lu\n",
> + mmc_hostname(host->mmc), clk_get_rate(msm_host->clk));
> + }
> +
> + spin_lock_irq(&host->lock);
> +out:
> + if (!msm_host->clk_table)
> + return sdhci_set_clock(host, clock);
Could put the above 2 lines at the start and then no need to check
msm_host->clk_table again.
> + __sdhci_msm_set_clock(host, clock);
> +}
> +
> static const struct of_device_id sdhci_msm_dt_match[] = {
> { .compatible = "qcom,sdhci-msm-v4" },
> {},
> @@ -598,7 +706,7 @@ MODULE_DEVICE_TABLE(of, sdhci_msm_dt_match);
> static const struct sdhci_ops sdhci_msm_ops = {
> .platform_execute_tuning = sdhci_msm_execute_tuning,
> .reset = sdhci_reset,
> - .set_clock = sdhci_set_clock,
> + .set_clock = sdhci_msm_set_clock,
> .get_min_clock = sdhci_msm_get_min_clock,
> .get_max_clock = sdhci_msm_get_max_clock,
> .set_bus_width = sdhci_set_bus_width,
>
--
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 05/12] mmc: sdhci-msm: Add get_min_clock() and get_max_clock() callback
From: Adrian Hunter @ 2016-10-10 9:46 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-6-git-send-email-riteshh@codeaurora.org>
On 05/10/16 17:40, Ritesh Harjani wrote:
> This add get_min_clock() and get_max_clock() callback
> for sdhci-msm. sdhci-msm min/max clocks may be different
> hence implement these callbacks.
>
> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
> ---
> drivers/mmc/host/sdhci-msm.c | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 042ecb2..4e17201 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -68,6 +68,7 @@
> #define CORE_VENDOR_SPEC_CAPABILITIES0 0x11c
>
> #define TCXO_FREQ 19200000
> +#define SDHCI_MSM_MIN_CLOCK 400000
>
> #define CDR_SELEXT_SHIFT 20
> #define CDR_SELEXT_MASK (0xf << CDR_SELEXT_SHIFT)
> @@ -561,6 +562,32 @@ static irqreturn_t sdhci_msm_pwr_irq(int irq, void *data)
> return IRQ_HANDLED;
> }
>
> +static unsigned int sdhci_msm_get_max_clock(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 count;
> +
> + if (msm_host->clk_table) {
I don't know if DT allows empty arrays, but if it does, might not
msm_host->clk_table_sz be zero here.
> + count = msm_host->clk_table_sz;
> + return msm_host->clk_table[count - 1];
> + } else {
'else' is redundant here.
> + return clk_round_rate(msm_host->clk, ULONG_MAX);
> + }
> +}
> +
> +static unsigned int sdhci_msm_get_min_clock(struct sdhci_host *host)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +
> + if (msm_host->clk_table) {
Same about possibility of msm_host->clk_table_sz being zero.
> + return msm_host->clk_table[0];
> + } else {
'else' is redundant here.
> + return SDHCI_MSM_MIN_CLOCK;
> + }
> +}
> +
> static const struct of_device_id sdhci_msm_dt_match[] = {
> { .compatible = "qcom,sdhci-msm-v4" },
> {},
> @@ -572,6 +599,8 @@ static const struct sdhci_ops sdhci_msm_ops = {
> .platform_execute_tuning = sdhci_msm_execute_tuning,
> .reset = sdhci_reset,
> .set_clock = sdhci_set_clock,
> + .get_min_clock = sdhci_msm_get_min_clock,
> + .get_max_clock = sdhci_msm_get_max_clock,
> .set_bus_width = sdhci_set_bus_width,
> .set_uhs_signaling = sdhci_msm_set_uhs_signaling,
> .voltage_switch = sdhci_msm_voltage_switch,
>
^ permalink raw reply
* Re: [PATCH v5 03/12] mmc: sdhci-msm: add pltfm_data support to get clk-rates from DT
From: Adrian Hunter @ 2016-10-10 9:35 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-4-git-send-email-riteshh@codeaurora.org>
On 05/10/16 17:40, 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.
>
> Example:
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 85ddaae..042ecb2 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -81,6 +81,8 @@ struct sdhci_msm_host {
> struct clk *clk; /* main SD/MMC bus clock */
> struct clk *pclk; /* SDHC peripheral bus clock */
> struct clk *bus_clk; /* SDHC bus voter clock */
> + u32 *clk_table;
> + int clk_table_sz;
> struct mmc_host *mmc;
> bool use_14lpp_dll_reset;
> };
> @@ -582,6 +584,50 @@ static const struct sdhci_pltfm_data sdhci_msm_pdata = {
> .ops = &sdhci_msm_ops,
> };
>
> +static int sdhci_msm_dt_get_array(struct device *dev, const char *prop_name,
> + u32 **table, int *size)
It is nice to align to the open parenthesis. Have a look at the checks from
checkpatch --strict
> +{
> + struct device_node *np = dev->of_node;
> + int count, ret;
> + u32 *arr;
> +
> + count = of_property_count_elems_of_size(np, prop_name, sizeof(u32));
Shouldn't this be of_property_count_u32_elems()
> + if (count < 0) {
> + dev_warn(dev, "%s: Invalid dt property, err(%d)\n",
> + prop_name, count);
> + return count;
> + }
> +
> + arr = kcalloc(count, sizeof(*arr), GFP_KERNEL);
> + if (!arr)
> + return -ENOMEM;
> +
> + ret = of_property_read_u32_array(np, prop_name, arr, count);
> + if (ret) {
> + kfree(arr);
> + dev_warn(dev, "%s Invalid dt array property, err(%d)\n",
'err(%d)' is an unusual style for printing error numbers. 'error %d' looks
better.
Also in some messages you have 'DT' and others 'dt'. Also here it is '%s'
but above '%s:', and some messages start with a lower case letter and some
upper case. Please try to make everything consistent.
> + prop_name, ret);
> + return ret;
> + }
> + *table = arr;
> + *size = count;
> + return 0;
> +}
> +
> +void sdhci_msm_populate_dt(struct device *dev,
> + struct sdhci_msm_host *msm_host)
Align to open parenthesis
> +{
> + int table_sz = 0;
> + u32 *table = NULL;
> +
> + if (sdhci_msm_dt_get_array(dev, "clk-rates", &table, &table_sz)) {
> + dev_warn(dev, "failed in DT parsing for supported clk-rates\n");
> + return;
> + }
> + msm_host->clk_table = table;
> + msm_host->clk_table_sz = table_sz;
> +}
> +
> static int sdhci_msm_probe(struct platform_device *pdev)
> {
> struct sdhci_host *host;
> @@ -608,6 +654,8 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>
> sdhci_get_of_property(pdev);
>
> + sdhci_msm_populate_dt(&pdev->dev, msm_host);
> +
> /* 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] mmc: sdhci-pci-core: Tuning mode support for HS200 on AMD Platforms
From: Adrian Hunter @ 2016-10-10 7:55 UTC (permalink / raw)
To: Shyam Sundar S K
Cc: ulf.hansson, linux-mmc, Sen, Pankaj, Shah, Nehal-bakulchandra,
Agrawal, Nitesh-kumar
In-Reply-To: <2ef1e2da-5913-85fe-65a7-0984ac04164d@amd.com>
On 04/10/16 11:42, Shyam Sundar S K wrote:
> This patch adds support for HS200 tuning mode on AMD eMMC-4.5.1
>
> Reviewed-by: Sen, Pankaj <Pankaj.Sen@amd.com>
> Reviewed-by: Shah, Nehal-bakulchandra <Nehal-bakulchandra.Shah@amd.com>
> Signed-off-by: S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>
> ---
> drivers/mmc/host/sdhci-pci-core.c | 182 +++++++++++++++++++++++++++++++++++++-
> 1 file changed, 180 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
> index 897cfd2..5893ec4 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -734,6 +734,7 @@ static const struct sdhci_pci_fixes sdhci_via = {
> .probe = via_probe,
> };
>
> +
Unnecessary blank line
> static int rtsx_probe_slot(struct sdhci_pci_slot *slot)
> {
> slot->host->mmc->caps2 |= MMC_CAP2_HS200;
> @@ -755,6 +756,172 @@ enum amd_chipset_gen {
> AMD_CHIPSET_UNKNOWN,
> };
>
> +struct tuning_descriptor {
It would be nicer to prefix all structure and function names by something
specific to the device e.g. amd_...
> + unsigned char tune_around;
> + bool this_tune_ok;
> + bool last_tune_ok;
> + bool valid_front_end;
> + unsigned char valid_front;
> + unsigned char valid_window_max;
> + unsigned char tune_low_max;
> + unsigned char tune_low;
> + unsigned char valid_window;
> + unsigned char tune_result;
'unsigned char' -> 'u8'
> +};
> +
> +static struct sdhci_ops sdhci_pci_ops;
> +static struct tuning_descriptor tdescriptor;
tdescriptor should not be global. You need somewhere to put private
data. How about this:
From: Adrian Hunter <adrian.hunter@intel.com>
Date: Mon, 10 Oct 2016 10:04:45 +0300
Subject: [PATCH] mmc: sdhci-pci: Let devices define their own private data
Let devices define their own private data to facilitate device-specific
operations. The size of the private structure is specified in the
sdhci_pci_fixes structure, then sdhci_pci_probe_slot() will allocate extra
space for it, and sdhci_pci_priv() can be used to get a reference to it.
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
drivers/mmc/host/sdhci-pci-core.c | 3 ++-
drivers/mmc/host/sdhci-pci.h | 8 ++++++++
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index 1d9e00a00e9f..782c8d25c0c8 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -1646,6 +1646,7 @@ static struct sdhci_pci_slot *sdhci_pci_probe_slot(
struct sdhci_pci_slot *slot;
struct sdhci_host *host;
int ret, bar = first_bar + slotno;
+ size_t priv_size = chip->fixes ? chip->fixes->priv_size : 0;
if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM)) {
dev_err(&pdev->dev, "BAR %d is not iomem. Aborting.\n", bar);
@@ -1667,7 +1668,7 @@ static struct sdhci_pci_slot *sdhci_pci_probe_slot(
return ERR_PTR(-ENODEV);
}
- host = sdhci_alloc_host(&pdev->dev, sizeof(struct sdhci_pci_slot));
+ host = sdhci_alloc_host(&pdev->dev, sizeof(*slot) + priv_size);
if (IS_ERR(host)) {
dev_err(&pdev->dev, "cannot allocate host\n");
return ERR_CAST(host);
diff --git a/drivers/mmc/host/sdhci-pci.h b/drivers/mmc/host/sdhci-pci.h
index 6bccf56bc5ff..6a1be6afe089 100644
--- a/drivers/mmc/host/sdhci-pci.h
+++ b/drivers/mmc/host/sdhci-pci.h
@@ -67,6 +67,7 @@ struct sdhci_pci_fixes {
int (*resume) (struct sdhci_pci_chip *);
const struct sdhci_ops *ops;
+ size_t priv_size;
};
struct sdhci_pci_slot {
@@ -87,6 +88,8 @@ struct sdhci_pci_slot {
struct mmc_card *card,
unsigned int max_dtr, int host_drv,
int card_drv, int *drv_type);
+
+ unsigned long private[0] ____cacheline_aligned;
};
struct sdhci_pci_chip {
@@ -101,4 +104,9 @@ struct sdhci_pci_chip {
struct sdhci_pci_slot *slots[MAX_SLOTS]; /* Pointers to host slots */
};
+static inline void *sdhci_pci_priv(struct sdhci_pci_slot *slot)
+{
+ return (void *)slot->private;
+}
+
#endif /* __SDHCI_PCI_H */
--
1.9.1
> +
> +static int tuning_reset(struct sdhci_host *host)
> +{
> + unsigned int val;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&host->lock, flags);
> +
> + val = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> + val |= SDHCI_CTRL_PRESET_VAL_ENABLE | SDHCI_CTRL_EXEC_TUNING;
> + sdhci_writew(host, val, SDHCI_HOST_CONTROL2);
> +
> + val = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> + val &= ~SDHCI_CTRL_EXEC_TUNING;
> + sdhci_writew(host, val, SDHCI_HOST_CONTROL2);
> +
> + spin_unlock_irqrestore(&host->lock, flags);
> +
> + return 0;
> +}
> +
> +static int config_tuning_phase(struct sdhci_host *host, unsigned char phase)
> +{
> + struct sdhci_pci_slot *slot = sdhci_priv(host);
> + struct pci_dev *pdev = slot->chip->pdev;
> + unsigned int val;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&host->lock, flags);
> +
> + pci_read_config_dword(pdev, 0xb8, &val);
> + val &= ~0x1f;
> + val |= (0x10800 | (phase << 1));
> + pci_write_config_dword(pdev, 0xb8, val);
> +
> + spin_unlock_irqrestore(&host->lock, flags);
> +
> + return 0;
> +}
> +
> +static int find_good_phase(struct sdhci_host *host)
> +{
> + struct tuning_descriptor *td = &tdescriptor;
> + struct sdhci_pci_slot *slot = sdhci_priv(host);
> + struct pci_dev *pdev = slot->chip->pdev;
> + unsigned int val;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&host->lock, flags);
> +
> + if (td->this_tune_ok == false)
> + td->valid_front_end = 1;
> +
> + if (td->valid_front_end)
> + td->valid_front = td->valid_front;
Assigning 'td->valid_front' to itself?
> + else if (td->this_tune_ok)
> + td->valid_front = td->valid_front + 1;
Use '+= 1'
> +
> + if ((!td->this_tune_ok && td->last_tune_ok) ||
> + (td->tune_around == 11)) {
> +
> + if (td->valid_window > td->valid_window_max) {
> + td->valid_window_max = td->valid_window;
> + td->tune_low_max = td->tune_low;
> + }
> + }
> +
> + if (td->this_tune_ok && (!td->last_tune_ok))
> + td->tune_low = td->tune_around;
> + if (!td->this_tune_ok && td->last_tune_ok)
> + td->valid_window = 0x0;
Just use 0 not 0x0.
> + else if (td->this_tune_ok)
> + td->valid_window = td->valid_window + 1;
Use '+= 1'
> +
> + td->last_tune_ok = td->this_tune_ok;
> +
> + if (td->tune_around == 11) {
> + if ((td->valid_front + td->valid_window) >
> + td->valid_window_max) {
> + if (td->valid_front > td->valid_window)
> + td->tune_result = ((td->valid_front -
> + td->valid_window) >> 1);
> + else
> + td->tune_result = td->tune_low +
> + ((td->valid_window + td->valid_front) >> 1);
> + } else {
> + td->tune_result = td->tune_low_max +
> + (td->valid_window_max>>1);
> + }
> +
> +
> + if (td->tune_result > 0x0b)
> + td->tune_result = 0x0b;
> +
> + pci_read_config_dword(pdev, 0xb8, &val);
> + val &= ~0x1f;
> + val |= (0x10800 | (td->tune_result<<1));
> + pci_write_config_dword(pdev, 0xb8, val);
> + }
> +
> + spin_unlock_irqrestore(&host->lock, flags);
> +
> + return 0;
> +}
> +
> +static int amd_execute_tuning(struct sdhci_host *host, u32 opcode)
> +{
> + struct tuning_descriptor *td = &tdescriptor;
> + u8 ctrl;
> +
> + tuning_reset(host);
> + memset(td, 0x0, sizeof(struct tuning_descriptor));
> +
> + if (host->quirks2 & SDHCI_QUIRK2_CLEAR_TRANSFERMODE_REG_BEFORE_CMD)
> + opcode = MMC_SEND_TUNING_BLOCK_HS200;
Why change the opcode?
> +
> + for (td->tune_around = 0; td->tune_around < 12; td->tune_around++) {
> +
> + config_tuning_phase(host, td->tune_around);
> +
> + if (mmc_send_tuning(host->mmc, opcode, NULL)) {
> + td->this_tune_ok = false;
> + host->mmc->need_retune = 0;
> + mdelay(4);
> + ctrl = SDHCI_RESET_CMD | SDHCI_RESET_DATA;
> + sdhci_writeb(host, ctrl, SDHCI_SOFTWARE_RESET);
> + } else {
> + td->this_tune_ok = true;
> + }
> +
> + find_good_phase(host);
It looks like tune_around is a temporary that should be passed as a parameter
to find_good_phase()
> + }
> +
> + host->mmc->retune_period = 0;
> +
> + return 0;
> +}
> +
> +static int amd_enable_manual_tuning(struct sdhci_pci_slot *slot)
> +{
> + struct pci_dev *pdev = slot->chip->pdev;
> + unsigned int val;
> +
> + pci_read_config_dword(pdev, 0xd0, &val);
> + val &= 0xffffffcf;
> + val |= 0x30;
> + pci_write_config_dword(pdev, 0xd0, val);
> +
> + return 0;
> +}
> +
> static int amd_probe(struct sdhci_pci_chip *chip)
> {
> struct pci_dev *smbus_dev;
> @@ -779,14 +946,25 @@ static int amd_probe(struct sdhci_pci_chip *chip)
>
> if ((gen == AMD_CHIPSET_BEFORE_ML) || (gen == AMD_CHIPSET_CZ)) {
> chip->quirks2 |= SDHCI_QUIRK2_CLEAR_TRANSFERMODE_REG_BEFORE_CMD;
> - chip->quirks2 |= SDHCI_QUIRK2_BROKEN_HS200;
> }
>
> return 0;
> }
>
> +static int amd_probe_slot(struct sdhci_pci_slot *slot)
> +{
> + struct sdhci_host *host = slot->host;
> +
> + if (host->quirks2 & SDHCI_QUIRK2_CLEAR_TRANSFERMODE_REG_BEFORE_CMD) {
Please don't use a quirk to identify the chip.
> + sdhci_pci_ops.platform_execute_tuning = amd_execute_tuning;
You can't do that because sdhci_pci_ops is shared with every PCI SDHCI host
controller not just AMD.
Have a look at:
https://marc.info/?l=linux-mmc&m=147565902811041&w=2
and re-base on top of that series.
> + amd_enable_manual_tuning(slot);
> + }
> + return 0;
> +}
> +
> static const struct sdhci_pci_fixes sdhci_amd = {
> .probe = amd_probe,
> + .probe_slot = amd_probe_slot,
> };
>
> static const struct pci_device_id pci_ids[] = {
> @@ -1397,7 +1575,7 @@ static int sdhci_pci_select_drive_strength(struct sdhci_host *host,
> card_drv, drv_type);
> }
>
> -static const struct sdhci_ops sdhci_pci_ops = {
> +static struct sdhci_ops sdhci_pci_ops = {
> .set_clock = sdhci_set_clock,
> .enable_dma = sdhci_pci_enable_dma,
> .set_bus_width = sdhci_pci_set_bus_width,
>
Please run checkpatch --strict and consider it suggestions.
Please consider adding some explanation of the tuning algorithm
and defining some of the constants.
^ permalink raw reply related
* Re: [PATCH] ARM: dts: imx50: imx50-esdhc use imx53-esdhc
From: Uwe Kleine-König @ 2016-10-10 6:47 UTC (permalink / raw)
To: Alexander Kurz
Cc: Vladimir Zapolskiy, Fabio Estevam, linux-mmc, Shawn Guo,
Sascha Hauer, Lucas Stach
In-Reply-To: <alpine.DEB.2.00.1610092241410.19531@blala.de>
Hello,
On Sun, Oct 09, 2016 at 11:09:28PM +0200, Alexander Kurz wrote:
> On Sun, 9 Oct 2016, Vladimir Zapolskiy wrote:
> > On 09.10.2016 18:34, Alexander Kurz wrote:
> > > - compatible = "fsl,imx50-esdhc";
> > > + compatible = "fsl,imx50-esdhc", "fsl,imx53-esdhc";
> > > [...]
> >
> > Regarding DTS change if the controllers are the same I think it may
> > have sense to add "fsl,imx50-esdhc" compatible to imx53.dtsi definitions,
> > but not the other way round.
> Is there some requirement for ascending numbers?
No, usually the older hardware defines the major name[1]. So IMHO the change
under discussion here is fine. Check the output of
git grep imx27 arch/arm/boot/dts/imx25.dtsi
to get a few samples of a similar case. (i.e. imx25 is newer but got
the smaller number.)
Best regards
Uwe
[1] I'd expect that older might also mean "first supported by a
binding".
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [RESEND PATCH 0/9] Init runtime PM support for dw_mmc
From: Shawn Lin @ 2016-10-10 2:30 UTC (permalink / raw)
To: Jaehoon Chung, Ulf Hansson
Cc: shawn.lin, linux-mmc, Doug Anderson, linux-rockchip
In-Reply-To: <9c4dd4aa-af9f-1765-fae6-f279ca45e60e@samsung.com>
在 2016/10/10 10:10, Jaehoon Chung 写道:
> Hi Shawn,
>
> On 10/09/2016 10:58 PM, Shawn Lin wrote:
>> Hi Jaehoon and Ulf,
>>
>> This patch is gonna support runtime PM for dw_mmc.
>> It could support to disable ciu_clk by default and disable
>> biu_clk if the devices are non-removeable, or removeable
>> with gpio-base card detect.
>>
>> Then I remove the system PM since the runtime PM actually
>> does the same thing as it. So I help migrate the dw_mmc variant
>> drivers to use runtime PM pairs and pm_runtime_force_*. Note
>> that I only enable runtime PM for dw_mmc-rockchip as I will
>> leave the decision to the owners of the corresponding drivers.
>> I just tested it on my RK3288 platform with linux-next to make
>> the runtime PM and system PM work fine for my emmc, sd card and
>> sdio. But I don't have hardware to help test other variant drivers.
>> But in theory it should work fine as I mentioned that the runtime
>> PM does the same thing as system PM except for disabling ciu_clk
>> aggressively which should not be related to the variant hosts.
>
> I'm testing this patchset with exynos SoCs. If it's possible to use other SoCs,
Thanks for testing this patchset, I will respin v2 if there are no any
other nitpick from folkz and regression when you test it.
> then I think your patches will be changed more clear than now.
> e.g) Not need to define the each driver's runtime_pm_suspend/resume function.
>
Sure.
>>
>> As you could see that I just extend the slot-gpio a bit, so the
>> ideal way is Ulf could pick them up with Jaehoon's ack. :)
>>
>>
>>
>> Shawn Lin (9):
>> mmc: dw_mmc: add runtime PM callback
>> mmc: dw_mmc-rockchip: add runtime PM support
>> mmc: core: expose the capability of gpio card detect
>> mmc: dw_mmc-rockchip: disable biu clk if possible
>> mmc: dw_mmc-k3: deploay runtime PM facilities
>> mmc: dw_mmc-exynos: deploay runtime PM facilities
>> mmc: dw_mmc-pci: deploay runtime PM facilities
>> mmc: dw_mmc-pltfm: deploay runtime PM facilities
>
> Maybe s/deploay/deploy
Will fix the copy-paste error, mea culpa, but it is quite
interesting that checkpatch.pl didn't yell at it. :)
>
> Best Regards,
> Jaehoon Chung
>
>> mmc: dw_mmc: remove system PM callback
>>
>> drivers/mmc/core/slot-gpio.c | 8 +++++
>> drivers/mmc/host/dw_mmc-exynos.c | 21 ++++++-----
>> drivers/mmc/host/dw_mmc-k3.c | 37 +++++++++----------
>> drivers/mmc/host/dw_mmc-pci.c | 25 ++++++++-----
>> drivers/mmc/host/dw_mmc-pltfm.c | 26 ++++++++------
>> drivers/mmc/host/dw_mmc-rockchip.c | 73 ++++++++++++++++++++++++++++++++++++--
>> drivers/mmc/host/dw_mmc.c | 24 ++++++-------
>> drivers/mmc/host/dw_mmc.h | 6 ++--
>> include/linux/mmc/slot-gpio.h | 1 +
>> 9 files changed, 152 insertions(+), 69 deletions(-)
>>
>
> --
> 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
>
--
Best Regards
Shawn Lin
^ permalink raw reply
* Re: [RESEND PATCH 0/9] Init runtime PM support for dw_mmc
From: Jaehoon Chung @ 2016-10-10 2:10 UTC (permalink / raw)
To: Shawn Lin, Ulf Hansson; +Cc: linux-mmc, Doug Anderson, linux-rockchip
In-Reply-To: <1476021506-869-1-git-send-email-shawn.lin@rock-chips.com>
Hi Shawn,
On 10/09/2016 10:58 PM, Shawn Lin wrote:
> Hi Jaehoon and Ulf,
>
> This patch is gonna support runtime PM for dw_mmc.
> It could support to disable ciu_clk by default and disable
> biu_clk if the devices are non-removeable, or removeable
> with gpio-base card detect.
>
> Then I remove the system PM since the runtime PM actually
> does the same thing as it. So I help migrate the dw_mmc variant
> drivers to use runtime PM pairs and pm_runtime_force_*. Note
> that I only enable runtime PM for dw_mmc-rockchip as I will
> leave the decision to the owners of the corresponding drivers.
> I just tested it on my RK3288 platform with linux-next to make
> the runtime PM and system PM work fine for my emmc, sd card and
> sdio. But I don't have hardware to help test other variant drivers.
> But in theory it should work fine as I mentioned that the runtime
> PM does the same thing as system PM except for disabling ciu_clk
> aggressively which should not be related to the variant hosts.
I'm testing this patchset with exynos SoCs. If it's possible to use other SoCs,
then I think your patches will be changed more clear than now.
e.g) Not need to define the each driver's runtime_pm_suspend/resume function.
>
> As you could see that I just extend the slot-gpio a bit, so the
> ideal way is Ulf could pick them up with Jaehoon's ack. :)
>
>
>
> Shawn Lin (9):
> mmc: dw_mmc: add runtime PM callback
> mmc: dw_mmc-rockchip: add runtime PM support
> mmc: core: expose the capability of gpio card detect
> mmc: dw_mmc-rockchip: disable biu clk if possible
> mmc: dw_mmc-k3: deploay runtime PM facilities
> mmc: dw_mmc-exynos: deploay runtime PM facilities
> mmc: dw_mmc-pci: deploay runtime PM facilities
> mmc: dw_mmc-pltfm: deploay runtime PM facilities
Maybe s/deploay/deploy
Best Regards,
Jaehoon Chung
> mmc: dw_mmc: remove system PM callback
>
> drivers/mmc/core/slot-gpio.c | 8 +++++
> drivers/mmc/host/dw_mmc-exynos.c | 21 ++++++-----
> drivers/mmc/host/dw_mmc-k3.c | 37 +++++++++----------
> drivers/mmc/host/dw_mmc-pci.c | 25 ++++++++-----
> drivers/mmc/host/dw_mmc-pltfm.c | 26 ++++++++------
> drivers/mmc/host/dw_mmc-rockchip.c | 73 ++++++++++++++++++++++++++++++++++++--
> drivers/mmc/host/dw_mmc.c | 24 ++++++-------
> drivers/mmc/host/dw_mmc.h | 6 ++--
> include/linux/mmc/slot-gpio.h | 1 +
> 9 files changed, 152 insertions(+), 69 deletions(-)
>
^ permalink raw reply
* Re: [PATCH] ARM: dts: imx50: imx50-esdhc use imx53-esdhc
From: Alexander Kurz @ 2016-10-09 21:09 UTC (permalink / raw)
To: Vladimir Zapolskiy
Cc: Shawn Guo, Sascha Hauer, Fabio Estevam, Lucas Stach, linux-mmc
In-Reply-To: <5eae97cb-7a15-9bcf-f732-6141f99f0990@mleia.com>
On Sun, 9 Oct 2016, Vladimir Zapolskiy wrote:
> Hi Alexander,
>
> On 09.10.2016 18:34, Alexander Kurz wrote:
> > According to the reference manuals, both imx50/imx53 SOC seem to share
> > the same eSDHC controller, especially the section on "Multi-block Read"
> > mentioned in commit 361b8482026c ("mmc: sdhci-esdhc-imx: fix multiblock
> > reads on i.MX53") is identical for both SOC.
> > Hence, let imx50 use imx53-esdhc.
> >
> > Signed-off-by: Alexander Kurz <akurz@blala.de>
> > ---
> > arch/arm/boot/dts/imx50.dtsi | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/imx50.dtsi b/arch/arm/boot/dts/imx50.dtsi
> > index e245713..769eac7 100644
> > --- a/arch/arm/boot/dts/imx50.dtsi
> > +++ b/arch/arm/boot/dts/imx50.dtsi
> > @@ -99,7 +99,7 @@
> > ranges;
> >
> > esdhc1: esdhc@50004000 {
> > - compatible = "fsl,imx50-esdhc";
> > + compatible = "fsl,imx50-esdhc", "fsl,imx53-esdhc";
>
> Instead of this change please add "fsl,imx50-esdhc" compatible to a list found in
> drivers/mmc/host/sdhci-esdhc-imx.c
For me it looks like a typical usecase to be handled via DT.
>From the perspective of the reference manuals it looks like both SOC
feature exactly the same sdhc hardware.
Extra benefit: projects sharing the linux DT set (e.g. barebox) will
immediately profit from this change.
>
> Regarding DTS change if the controllers are the same I think it may
> have sense to add "fsl,imx50-esdhc" compatible to imx53.dtsi definitions,
> but not the other way round.
Is there some requirement for ascending numbers?
I guess, i.MX50 just got the smaller number (compared to i.MX51/53) for
marketing reasons (consumer-e-book reader vs "serious" industrial
applications). Also the mx50 is a younger design compared to the mx51.
The mx50 is an SOC for e-book readers and other battery powered gadgets
(e.g. battery powered wireless hard disk) - it never got the attention
like the industrial SOC series (mx51/53).
>
> --
> With best wishes,
> Vladimir
>
^ permalink raw reply
* Re: [PATCH] ARM: dts: imx50: imx50-esdhc use imx53-esdhc
From: Vladimir Zapolskiy @ 2016-10-09 20:25 UTC (permalink / raw)
To: Alexander Kurz, Shawn Guo
Cc: Sascha Hauer, Fabio Estevam, Lucas Stach, linux-mmc
In-Reply-To: <1476030890-18274-1-git-send-email-akurz@blala.de>
Hi Alexander,
On 09.10.2016 18:34, Alexander Kurz wrote:
> According to the reference manuals, both imx50/imx53 SOC seem to share
> the same eSDHC controller, especially the section on "Multi-block Read"
> mentioned in commit 361b8482026c ("mmc: sdhci-esdhc-imx: fix multiblock
> reads on i.MX53") is identical for both SOC.
> Hence, let imx50 use imx53-esdhc.
>
> Signed-off-by: Alexander Kurz <akurz@blala.de>
> ---
> arch/arm/boot/dts/imx50.dtsi | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/imx50.dtsi b/arch/arm/boot/dts/imx50.dtsi
> index e245713..769eac7 100644
> --- a/arch/arm/boot/dts/imx50.dtsi
> +++ b/arch/arm/boot/dts/imx50.dtsi
> @@ -99,7 +99,7 @@
> ranges;
>
> esdhc1: esdhc@50004000 {
> - compatible = "fsl,imx50-esdhc";
> + compatible = "fsl,imx50-esdhc", "fsl,imx53-esdhc";
Instead of this change please add "fsl,imx50-esdhc" compatible to a list found in
drivers/mmc/host/sdhci-esdhc-imx.c
Regarding DTS change if the controllers are the same I think it may
have sense to add "fsl,imx50-esdhc" compatible to imx53.dtsi definitions,
but not the other way round.
--
With best wishes,
Vladimir
^ permalink raw reply
* [PATCH] ARM: dts: imx50: imx50-esdhc use imx53-esdhc
From: Alexander Kurz @ 2016-10-09 16:34 UTC (permalink / raw)
To: Shawn Guo
Cc: Sascha Hauer, Fabio Estevam, Lucas Stach, linux-mmc,
Alexander Kurz
According to the reference manuals, both imx50/imx53 SOC seem to share
the same eSDHC controller, especially the section on "Multi-block Read"
mentioned in commit 361b8482026c ("mmc: sdhci-esdhc-imx: fix multiblock
reads on i.MX53") is identical for both SOC.
Hence, let imx50 use imx53-esdhc.
Signed-off-by: Alexander Kurz <akurz@blala.de>
---
arch/arm/boot/dts/imx50.dtsi | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/imx50.dtsi b/arch/arm/boot/dts/imx50.dtsi
index e245713..769eac7 100644
--- a/arch/arm/boot/dts/imx50.dtsi
+++ b/arch/arm/boot/dts/imx50.dtsi
@@ -99,7 +99,7 @@
ranges;
esdhc1: esdhc@50004000 {
- compatible = "fsl,imx50-esdhc";
+ compatible = "fsl,imx50-esdhc", "fsl,imx53-esdhc";
reg = <0x50004000 0x4000>;
interrupts = <1>;
clocks = <&clks IMX5_CLK_ESDHC1_IPG_GATE>,
@@ -111,7 +111,7 @@
};
esdhc2: esdhc@50008000 {
- compatible = "fsl,imx50-esdhc";
+ compatible = "fsl,imx50-esdhc", "fsl,imx53-esdhc";
reg = <0x50008000 0x4000>;
interrupts = <2>;
clocks = <&clks IMX5_CLK_ESDHC2_IPG_GATE>,
@@ -160,7 +160,7 @@
};
esdhc3: esdhc@50020000 {
- compatible = "fsl,imx50-esdhc";
+ compatible = "fsl,imx50-esdhc", "fsl,imx53-esdhc";
reg = <0x50020000 0x4000>;
interrupts = <3>;
clocks = <&clks IMX5_CLK_ESDHC3_IPG_GATE>,
@@ -172,7 +172,7 @@
};
esdhc4: esdhc@50024000 {
- compatible = "fsl,imx50-esdhc";
+ compatible = "fsl,imx50-esdhc", "fsl,imx53-esdhc";
reg = <0x50024000 0x4000>;
interrupts = <4>;
clocks = <&clks IMX5_CLK_ESDHC4_IPG_GATE>,
--
2.1.4
^ permalink raw reply related
* [RESEND PATCH 8/9] mmc: dw_mmc-pltfm: deploay runtime PM facilities
From: Shawn Lin @ 2016-10-09 14:02 UTC (permalink / raw)
To: Jaehoon Chung, Ulf Hansson
Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Shawn Lin,
linux-mmc-u79uwXL29TY76Z2rM5mHXA, Doug Anderson
In-Reply-To: <1476021721-923-1-git-send-email-shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Let's migrate it to use runtime PM and remove the system
PM callback from this driver. With this patch, it could
handle system PM properly and could also use runtime PM
if we enable it.
Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
---
drivers/mmc/host/dw_mmc-pltfm.c | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c
index c0bb0c7..214e593 100644
--- a/drivers/mmc/host/dw_mmc-pltfm.c
+++ b/drivers/mmc/host/dw_mmc-pltfm.c
@@ -16,6 +16,7 @@
#include <linux/io.h>
#include <linux/irq.h>
#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
#include <linux/slab.h>
#include <linux/mmc/host.h>
#include <linux/mmc/mmc.h>
@@ -57,26 +58,29 @@ int dw_mci_pltfm_register(struct platform_device *pdev,
}
EXPORT_SYMBOL_GPL(dw_mci_pltfm_register);
-#ifdef CONFIG_PM_SLEEP
-/*
- * TODO: we should probably disable the clock to the card in the suspend path.
- */
-static int dw_mci_pltfm_suspend(struct device *dev)
+#ifdef CONFIG_PM
+static int dw_mci_pltfm_runtime_suspend(struct device *dev)
{
struct dw_mci *host = dev_get_drvdata(dev);
- return dw_mci_suspend(host);
+ return dw_mci_runtime_suspend(host);
}
-static int dw_mci_pltfm_resume(struct device *dev)
+static int dw_mci_pltfm_runtime_resume(struct device *dev)
{
struct dw_mci *host = dev_get_drvdata(dev);
- return dw_mci_resume(host);
+ return dw_mci_runtime_resume(host);
}
-#endif /* CONFIG_PM_SLEEP */
-
-SIMPLE_DEV_PM_OPS(dw_mci_pltfm_pmops, dw_mci_pltfm_suspend, dw_mci_pltfm_resume);
+#endif /* CONFIG_PM */
+
+const struct dev_pm_ops dw_mci_pltfm_pmops = {
+ SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+ pm_runtime_force_resume)
+ SET_RUNTIME_PM_OPS(dw_mci_pltfm_runtime_suspend,
+ dw_mci_pltfm_runtime_resume,
+ NULL)
+};
EXPORT_SYMBOL_GPL(dw_mci_pltfm_pmops);
static const struct of_device_id dw_mci_pltfm_match[] = {
--
2.3.7
^ permalink raw reply related
* [RESEND PATCH 9/9] mmc: dw_mmc: remove system PM callback
From: Shawn Lin @ 2016-10-09 14:02 UTC (permalink / raw)
To: Jaehoon Chung, Ulf Hansson
Cc: linux-mmc, Doug Anderson, linux-rockchip, Shawn Lin
In-Reply-To: <1476021721-923-1-git-send-email-shawn.lin@rock-chips.com>
Now there are no variant drivers using dw_mci_suspend
and dw_mci_resume, so let's remove it.
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---
drivers/mmc/host/dw_mmc.c | 44 ++++++++------------------------------------
drivers/mmc/host/dw_mmc.h | 2 --
2 files changed, 8 insertions(+), 38 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index c5ef263..46a14c5 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -3267,26 +3267,24 @@ EXPORT_SYMBOL(dw_mci_remove);
#ifdef CONFIG_PM
-/*
- * TODO: we should probably disable the clock to the card in the suspend path.
- */
-int dw_mci_suspend(struct dw_mci *host)
+int dw_mci_runtime_suspend(struct dw_mci *host)
{
if (host->use_dma && host->dma_ops->exit)
host->dma_ops->exit(host);
+ clk_disable_unprepare(host->ciu_clk);
+
return 0;
}
-EXPORT_SYMBOL(dw_mci_suspend);
+EXPORT_SYMBOL(dw_mci_runtime_suspend);
-int dw_mci_resume(struct dw_mci *host)
+int dw_mci_runtime_resume(struct dw_mci *host)
{
- int i, ret;
+ int i, ret = 0;
- if (!dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS)) {
- ret = -ENODEV;
+ ret = clk_prepare_enable(host->ciu_clk);
+ if (ret)
return ret;
- }
if (host->use_dma && host->dma_ops->init)
host->dma_ops->init(host);
@@ -3323,32 +3321,6 @@ int dw_mci_resume(struct dw_mci *host)
return 0;
}
-EXPORT_SYMBOL(dw_mci_resume);
-
-int dw_mci_runtime_suspend(struct dw_mci *host)
-{
- int err = 0;
-
- err = dw_mci_suspend(host);
- if (err)
- return err;
-
- clk_disable_unprepare(host->ciu_clk);
-
- return err;
-}
-EXPORT_SYMBOL(dw_mci_runtime_suspend);
-
-int dw_mci_runtime_resume(struct dw_mci *host)
-{
- int ret = 0;
-
- ret = clk_prepare_enable(host->ciu_clk);
- if (ret)
- return ret;
-
- return dw_mci_resume(host);
-}
EXPORT_SYMBOL(dw_mci_runtime_resume);
#endif /* CONFIG_PM */
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index baa7261..b642dbb 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -235,8 +235,6 @@
extern int dw_mci_probe(struct dw_mci *host);
extern void dw_mci_remove(struct dw_mci *host);
#ifdef CONFIG_PM
-extern int dw_mci_suspend(struct dw_mci *host);
-extern int dw_mci_resume(struct dw_mci *host);
extern int dw_mci_runtime_suspend(struct dw_mci *host);
extern int dw_mci_runtime_resume(struct dw_mci *host);
#endif
--
2.3.7
^ permalink raw reply related
* [RESEND PATCH 7/9] mmc: dw_mmc-pci: deploay runtime PM facilities
From: Shawn Lin @ 2016-10-09 14:01 UTC (permalink / raw)
To: Jaehoon Chung, Ulf Hansson
Cc: linux-mmc, Doug Anderson, linux-rockchip, Shawn Lin
In-Reply-To: <1476021721-923-1-git-send-email-shawn.lin@rock-chips.com>
Let's migrate it to use runtime PM and remove the system
PM callback from this driver. With this patch, it could
handle system PM properly and could also use runtime PM
if we enable it.
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---
drivers/mmc/host/dw_mmc-pci.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc-pci.c b/drivers/mmc/host/dw_mmc-pci.c
index 4c69fbd..61789b1 100644
--- a/drivers/mmc/host/dw_mmc-pci.c
+++ b/drivers/mmc/host/dw_mmc-pci.c
@@ -14,6 +14,7 @@
#include <linux/io.h>
#include <linux/irq.h>
#include <linux/pci.h>
+#include <linux/pm_runtime.h>
#include <linux/slab.h>
#include <linux/mmc/host.h>
#include <linux/mmc/mmc.h>
@@ -79,25 +80,31 @@ static void dw_mci_pci_remove(struct pci_dev *pdev)
dw_mci_remove(host);
}
-#ifdef CONFIG_PM_SLEEP
-static int dw_mci_pci_suspend(struct device *dev)
+#ifdef CONFIG_PM
+static int dw_mci_pci_runtime_suspend(struct device *dev)
{
struct pci_dev *pdev = to_pci_dev(dev);
struct dw_mci *host = pci_get_drvdata(pdev);
- return dw_mci_suspend(host);
+ return dw_mci_runtime_suspend(host);
}
-static int dw_mci_pci_resume(struct device *dev)
+static int dw_mci_pci_runtime_resume(struct device *dev)
{
struct pci_dev *pdev = to_pci_dev(dev);
struct dw_mci *host = pci_get_drvdata(pdev);
- return dw_mci_resume(host);
+ return dw_mci_runtime_resume(host);
}
-#endif /* CONFIG_PM_SLEEP */
-
-static SIMPLE_DEV_PM_OPS(dw_mci_pci_pmops, dw_mci_pci_suspend, dw_mci_pci_resume);
+#endif /* CONFIG_PM */
+
+static const struct dev_pm_ops dw_mci_pci_dev_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+ pm_runtime_force_resume)
+ SET_RUNTIME_PM_OPS(dw_mci_pci_runtime_suspend,
+ dw_mci_pci_runtime_resume,
+ NULL)
+};
static const struct pci_device_id dw_mci_pci_id[] = {
{ PCI_DEVICE(SYNOPSYS_DW_MCI_VENDOR_ID, SYNOPSYS_DW_MCI_DEVICE_ID) },
@@ -111,7 +118,7 @@ static struct pci_driver dw_mci_pci_driver = {
.probe = dw_mci_pci_probe,
.remove = dw_mci_pci_remove,
.driver = {
- .pm = &dw_mci_pci_pmops
+ .pm = &dw_mci_pci_dev_pm_ops,
},
};
--
2.3.7
^ permalink raw reply related
* [RESEND PATCH 6/9] mmc: dw_mmc-exynos: deploay runtime PM facilities
From: Shawn Lin @ 2016-10-09 14:01 UTC (permalink / raw)
To: Jaehoon Chung, Ulf Hansson
Cc: linux-mmc, Doug Anderson, linux-rockchip, Shawn Lin
In-Reply-To: <1476021721-923-1-git-send-email-shawn.lin@rock-chips.com>
Let's migrate it to use runtime PM and remove the system
PM callback from this driver. With this patch, it could
handle system PM properly and could also use runtime PM
if we enable it.
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---
drivers/mmc/host/dw_mmc-exynos.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index 7ab3d74..189fd38 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -17,6 +17,7 @@
#include <linux/mmc/mmc.h>
#include <linux/of.h>
#include <linux/of_gpio.h>
+#include <linux/pm_runtime.h>
#include <linux/slab.h>
#include "dw_mmc.h"
@@ -161,20 +162,20 @@ static void dw_mci_exynos_set_clksel_timing(struct dw_mci *host, u32 timing)
set_bit(DW_MMC_CARD_NO_USE_HOLD, &host->cur_slot->flags);
}
-#ifdef CONFIG_PM_SLEEP
-static int dw_mci_exynos_suspend(struct device *dev)
+#ifdef CONFIG_PM
+static int dw_mci_exynos_runtime_suspend(struct device *dev)
{
struct dw_mci *host = dev_get_drvdata(dev);
- return dw_mci_suspend(host);
+ return dw_mci_runtime_suspend(host);
}
-static int dw_mci_exynos_resume(struct device *dev)
+static int dw_mci_exynos_runtime_resume(struct device *dev)
{
struct dw_mci *host = dev_get_drvdata(dev);
dw_mci_exynos_config_smu(host);
- return dw_mci_resume(host);
+ return dw_mci_runtime_resume(host);
}
/**
@@ -211,10 +212,8 @@ static int dw_mci_exynos_resume_noirq(struct device *dev)
return 0;
}
#else
-#define dw_mci_exynos_suspend NULL
-#define dw_mci_exynos_resume NULL
#define dw_mci_exynos_resume_noirq NULL
-#endif /* CONFIG_PM_SLEEP */
+#endif /* CONFIG_PM */
static void dw_mci_exynos_config_hs400(struct dw_mci *host, u32 timing)
{
@@ -531,7 +530,11 @@ static int dw_mci_exynos_probe(struct platform_device *pdev)
}
static const struct dev_pm_ops dw_mci_exynos_pmops = {
- SET_SYSTEM_SLEEP_PM_OPS(dw_mci_exynos_suspend, dw_mci_exynos_resume)
+ SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+ pm_runtime_force_resume)
+ SET_RUNTIME_PM_OPS(dw_mci_exynos_runtime_suspend,
+ dw_mci_exynos_runtime_resume,
+ NULL)
.resume_noirq = dw_mci_exynos_resume_noirq,
.thaw_noirq = dw_mci_exynos_resume_noirq,
.restore_noirq = dw_mci_exynos_resume_noirq,
--
2.3.7
^ permalink raw reply related
* [RESEND PATCH 5/9] mmc: dw_mmc-k3: deploay runtime PM facilities
From: Shawn Lin @ 2016-10-09 14:01 UTC (permalink / raw)
To: Jaehoon Chung, Ulf Hansson
Cc: linux-mmc, Doug Anderson, linux-rockchip, Shawn Lin
In-Reply-To: <1476021506-869-1-git-send-email-shawn.lin@rock-chips.com>
Let's migrate it to use runtime PM and remove the system
PM callback from this driver. With this patch, it could
handle system PM properly and could also use runtime PM
if we enable it.
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---
drivers/mmc/host/dw_mmc-k3.c | 37 ++++++++++++++++---------------------
1 file changed, 16 insertions(+), 21 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc-k3.c b/drivers/mmc/host/dw_mmc-k3.c
index 6247894..99b859d 100644
--- a/drivers/mmc/host/dw_mmc-k3.c
+++ b/drivers/mmc/host/dw_mmc-k3.c
@@ -15,6 +15,7 @@
#include <linux/module.h>
#include <linux/of_address.h>
#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
#include <linux/regmap.h>
#include <linux/regulator/consumer.h>
@@ -162,35 +163,29 @@ static int dw_mci_k3_probe(struct platform_device *pdev)
return dw_mci_pltfm_register(pdev, drv_data);
}
-#ifdef CONFIG_PM_SLEEP
-static int dw_mci_k3_suspend(struct device *dev)
+#ifdef CONFIG_PM
+static int dw_mci_k3_runtime_suspend(struct device *dev)
{
struct dw_mci *host = dev_get_drvdata(dev);
- int ret;
-
- ret = dw_mci_suspend(host);
- if (!ret)
- clk_disable_unprepare(host->ciu_clk);
- return ret;
+ return dw_mci_runtime_suspend(host);
}
-static int dw_mci_k3_resume(struct device *dev)
+static int dw_mci_k3_runtime_resume(struct device *dev)
{
struct dw_mci *host = dev_get_drvdata(dev);
- int ret;
- ret = clk_prepare_enable(host->ciu_clk);
- if (ret) {
- dev_err(host->dev, "failed to enable ciu clock\n");
- return ret;
- }
-
- return dw_mci_resume(host);
+ return dw_mci_runtime_resume(host);
}
-#endif /* CONFIG_PM_SLEEP */
-
-static SIMPLE_DEV_PM_OPS(dw_mci_k3_pmops, dw_mci_k3_suspend, dw_mci_k3_resume);
+#endif /* CONFIG_PM */
+
+static const struct dev_pm_ops dw_mci_k3_dev_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+ pm_runtime_force_resume)
+ SET_RUNTIME_PM_OPS(dw_mci_k3_runtime_suspend,
+ dw_mci_k3_runtime_resume,
+ NULL)
+};
static struct platform_driver dw_mci_k3_pltfm_driver = {
.probe = dw_mci_k3_probe,
@@ -198,7 +193,7 @@ static struct platform_driver dw_mci_k3_pltfm_driver = {
.driver = {
.name = "dwmmc_k3",
.of_match_table = dw_mci_k3_match,
- .pm = &dw_mci_k3_pmops,
+ .pm = &dw_mci_k3_dev_pm_ops,
},
};
--
2.3.7
^ permalink raw reply related
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