* Re: [PATCH 2/5] mmc: zx: Initial support for ZX mmc controller
From: Jun Nie @ 2016-10-25 8:29 UTC (permalink / raw)
To: Shawn Lin
Cc: Shawn Guo, xie.baoyou, Ulf Hansson, Jaehoon Chung, Jason Liu,
linux-mmc
In-Reply-To: <0c1d8185-618b-8e5d-e072-af305cd2ff78@rock-chips.com>
2016-10-24 20:29 GMT+08:00 Shawn Lin <shawn.lin@rock-chips.com>:
> 在 2016/10/24 17:11, Jun Nie 写道:
>>
>> This platform driver adds initial support for the DW host controller
>> found on ZTE SoCs.
>>
>> It has been tested on ZX296718 EVB board currently. More support on
>> timing tuning will be added when hardware is available.
>>
>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>> ---
>> drivers/mmc/host/Kconfig | 9 ++
>> drivers/mmc/host/Makefile | 1 +
>> drivers/mmc/host/dw_mmc-zx.c | 230
>> +++++++++++++++++++++++++++++++++++++++++++
>> drivers/mmc/host/dw_mmc-zx.h | 23 +++++
>> 4 files changed, 263 insertions(+)
>> create mode 100644 drivers/mmc/host/dw_mmc-zx.c
>> create mode 100644 drivers/mmc/host/dw_mmc-zx.h
>>
>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>> index 5274f50..2b3202c 100644
>> --- a/drivers/mmc/host/Kconfig
>> +++ b/drivers/mmc/host/Kconfig
>> @@ -662,6 +662,15 @@ config MMC_DW_ROCKCHIP
>> Synopsys DesignWare Memory Card Interface driver. Select this
>> option
>> for platforms based on RK3066, RK3188 and RK3288 SoC's.
>>
>> +config MMC_DW_ZX
>> + tristate "ZTE specific extensions for Synopsys DW Memory Card
>> Interface"
>> + depends on MMC_DW
>> + select MMC_DW_PLTFM
>> + help
>> + This selects support for ZTE SoC specific extensions to the
>> + Synopsys DesignWare Memory Card Interface driver. Select this
>> option
>> + for platforms based on ZX296718 SoC's.
>> +
>> config MMC_SH_MMCIF
>> tristate "SuperH Internal MMCIF support"
>> depends on HAS_DMA
>> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
>> index e2bdaaf..9766143 100644
>> --- a/drivers/mmc/host/Makefile
>> +++ b/drivers/mmc/host/Makefile
>> @@ -48,6 +48,7 @@ obj-$(CONFIG_MMC_DW_EXYNOS) += dw_mmc-exynos.o
>> obj-$(CONFIG_MMC_DW_K3) += dw_mmc-k3.o
>> obj-$(CONFIG_MMC_DW_PCI) += dw_mmc-pci.o
>> obj-$(CONFIG_MMC_DW_ROCKCHIP) += dw_mmc-rockchip.o
>> +obj-$(CONFIG_MMC_DW_ZX) += dw_mmc-zx.o
>> obj-$(CONFIG_MMC_SH_MMCIF) += sh_mmcif.o
>> obj-$(CONFIG_MMC_JZ4740) += jz4740_mmc.o
>> obj-$(CONFIG_MMC_VUB300) += vub300.o
>> diff --git a/drivers/mmc/host/dw_mmc-zx.c b/drivers/mmc/host/dw_mmc-zx.c
>> new file mode 100644
>> index 0000000..d0e4ef2
>> --- /dev/null
>> +++ b/drivers/mmc/host/dw_mmc-zx.c
>> @@ -0,0 +1,230 @@
>> +/*
>> + * ZX Specific Extensions for Synopsys DW Multimedia Card Interface
>> driver
>> + *
>> + * Copyright (C) 2016, Linaro Ltd.
>> + * Copyright (C) 2016, ZTE Corp.
>> + *
>> + * 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; either version 2 of the License, or
>> + * (at your option) any later version.
>> + */
>> +
>> +#include <linux/platform_device.h>
>> +#include <linux/clk.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/mmc/dw_mmc.h>
>> +#include <linux/mmc/host.h>
>> +#include <linux/mmc/mmc.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>
>
> #include <linux/platform_device.h>
> to keep alphabetical order
Will do.
>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/regmap.h>
>> +#include <linux/slab.h>
>> +
>> +#include "dw_mmc.h"
>> +#include "dw_mmc-pltfm.h"
>> +#include "dw_mmc-zx.h"
>> +
>> +#define ZX_DLL_LOCKED BIT(2)
>> +
>> +struct dw_mci_zx_priv_data {
>> + struct regmap *sysc_base;
>> +};
>> +
>> +static int dw_mci_zx_emmc_set_delay(struct dw_mci *host, unsigned int
>> delay,
>> + unsigned int clk_flag)
>
>
> I don't know if this is expected/legit way to control the delay/phase
> by adding code here. It looks more general to me that we should use
> clk_{set,get}_phase API.
The phase control register reside in a system controller, not a
standard clock controller. The register control data sampling phase
and bypass mode etc, but no clk rate/mux. So it will take more code
and ugly logic to expose a clk_* API based on such register.
>
>
>> +{
>> + struct dw_mci_zx_priv_data *priv = host->priv;
>> + struct regmap *sysc_base = priv->sysc_base;
>> + unsigned int clksel;
>> + unsigned int loop = 1000;
>> + int ret;
>> +
>> + ret = regmap_write(sysc_base, LB_AON_EMMC_CFG_REG0,
>> + PARA_DLL_START_POINT(4) |
>> PARA_DLL_LOCK_NUM(4));
>> + if (ret)
>> + return ret;
>> +
>> + ret = regmap_read(sysc_base, LB_AON_EMMC_CFG_REG1, &clksel);
>> + if (ret)
>> + return ret;
>> +
>> + if (clk_flag) {
>> + clksel &= ~(CLK_SAMP_DELAY(0x7F));
>> + clksel |= (delay << 8);
>> + } else {
>> + clksel &= ~(READ_DQS_DELAY(0x7F));
>> + clksel |= delay;
>> + }
>> +
>> + regmap_write(sysc_base, LB_AON_EMMC_CFG_REG1, clksel);
>> + regmap_write(sysc_base, LB_AON_EMMC_CFG_REG0,
>> + PARA_DLL_START_POINT(4) | PARA_DLL_LOCK_NUM(4) |
>> + DLL_REG_SET);
>> +
>> + do {
>> + ret = regmap_read(sysc_base, LB_AON_EMMC_CFG_REG2,
>> &clksel);
>> + if (ret)
>> + return ret;
>> +
>> + } while (--loop && !(clksel & ZX_DLL_LOCKED));
>> +
>> + if (!loop) {
>> + dev_err(host->dev, "Error: %s dll lock fail\n", __func__);
>> + return -EIO;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int dw_mci_zx_emmc_execute_tuning(struct dw_mci_slot *slot, u32
>> opcode)
>> +{
>> + struct dw_mci *host = slot->host;
>> + struct mmc_host *mmc = slot->mmc;
>> + int len, start = 0, end = 0, delay, best = 0;
>> + int ret = 0;
>> +
>> + for (delay = 1 ; delay < 128; delay++) {
>> + ret = dw_mci_zx_emmc_set_delay(host, delay, 1);
>> + if (ret)
>> + return ret;
>> +
>> + if (mmc_send_tuning(mmc, opcode, NULL)) {
>> + if (start >= 0) {
>> + end = delay - 1;
>> + /* check and update longest good range */
>> + if ((end - start) > len) {
>> + best = (start + end) >> 1;
>> + len = end - start;
>> + }
>> + }
>> + start = -1;
>> + end = 0;
>> + continue;
>> + }
>> + if (start < 0)
>> + start = delay;
>> + }
>> +
>> + if (start >= 0) {
>> + end = delay - 1;
>> + if ((end - start) > len) {
>> + best = (start + end) >> 1;
>> + len = end - start;
>> + }
>> + }
>> + if (best < 0)
>> + return -EIO;
>> +
>> + dev_info(host->dev, "%s best range: start %d end %d\n", __func__,
>> + start, end);
>> + dw_mci_zx_emmc_set_delay(host, best, 1);
>> + return 0;
>> +}
>> +
>> +static int dw_mci_zx_prepare_hs400_tuning(struct dw_mci *host,
>> + struct mmc_ios *ios)
>> +{
>> + int ret;
>> +
>> + /* config phase shift 90 */
>> + ret = dw_mci_zx_emmc_set_delay(host, 32, 0);
>> + if (ret < 0)
>> + return -EIO;
>> +
>> + return 0;
>> +}
>> +
>> +static int dw_mci_zx_execute_tuning(struct dw_mci_slot *slot, u32 opcode)
>> +{
>> + struct dw_mci *host = slot->host;
>> +
>> + if (host->verid == 0x290a) /* emmc */
>> + return dw_mci_zx_emmc_execute_tuning(slot, opcode);
>> +
>> + return 0;
>> +}
>> +
>> +static int dw_mci_zx_parse_dt(struct dw_mci *host)
>> +{
>> + struct device_node *np = host->dev->of_node;
>> + struct device_node *node;
>> + struct dw_mci_zx_priv_data *priv;
>> + struct regmap *sysc_base;
>> + int ret;
>> +
>> + node = of_parse_phandle(np, "zte,aon-syscon", 0);
>> + if (node) {
>> + sysc_base = syscon_node_to_regmap(node);
>> + of_node_put(node);
>> +
>> + if (IS_ERR(sysc_base)) {
>> + ret = PTR_ERR(sysc_base);
>> + if (ret != -EPROBE_DEFER)
>> + dev_err(host->dev, "Can't get syscon:
>> %d\n",
>> + ret);
>> + return ret;
>> + }
>> + }
>> +
>> + priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
>> + if (!priv)
>> + return -ENOMEM;
>> + priv->sysc_base = sysc_base;
>> + host->priv = priv;
>> +
>> + return 0;
>> +}
>> +
>> +static unsigned long zx_dwmmc_caps[3] = {
>> + MMC_CAP_HW_RESET,
>> + 0,
>> + 0,
>> +};
>> +
>
>
> just a thought, maybe you could add the cap-mmc-hw-reset for your
> emmc node on the DT as
>
> 1) you could save some bits here
> 2) Moreover, it's related to the board design since it isn't
> guaranteed that the hw_rest pin won't be used for other purpose..
>
You are right.
>
>> +static const struct dw_mci_drv_data zx_drv_data = {
>> + .caps = zx_dwmmc_caps,
>> + .execute_tuning = dw_mci_zx_execute_tuning,
>> + .prepare_hs400_tuning = dw_mci_zx_prepare_hs400_tuning,
>> + .parse_dt = dw_mci_zx_parse_dt,
>> +};
>> +
>> +static const struct of_device_id dw_mci_zx_match[] = {
>> + { .compatible = "zte,dw-mshc", .data = &zx_drv_data},
>> +};
>> +MODULE_DEVICE_TABLE(of, dw_mci_zx_match);
>> +
>> +static int dw_mci_zx_probe(struct platform_device *pdev)
>> +{
>> + const struct dw_mci_drv_data *drv_data;
>> + const struct of_device_id *match;
>> +
>> + match = of_match_node(dw_mci_zx_match, pdev->dev.of_node);
>> + drv_data = match->data;
>> +
>> + return dw_mci_pltfm_register(pdev, drv_data);
>> +}
>> +
>> +static const struct dev_pm_ops dw_mci_zx_dev_pm_ops = {
>> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
>> + pm_runtime_force_resume)
>> + SET_RUNTIME_PM_OPS(dw_mci_runtime_suspend,
>> + dw_mci_runtime_resume,
>> + NULL)
>> +};
>> +
>> +static struct platform_driver dw_mci_zx_pltfm_driver = {
>> + .probe = dw_mci_zx_probe,
>> + .remove = dw_mci_pltfm_remove,
>> + .driver = {
>> + .name = "dwmmc_zx",
>> + .of_match_table = dw_mci_zx_match,
>> + .pm = &dw_mci_zx_dev_pm_ops,
>> + },
>> +};
>> +
>> +module_platform_driver(dw_mci_zx_pltfm_driver);
>> +
>> +MODULE_DESCRIPTION("ZTE emmc/sd driver");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/mmc/host/dw_mmc-zx.h b/drivers/mmc/host/dw_mmc-zx.h
>> new file mode 100644
>> index 0000000..b1aac52
>> --- /dev/null
>> +++ b/drivers/mmc/host/dw_mmc-zx.h
>> @@ -0,0 +1,23 @@
>> +#ifndef _DW_MMC_ZX_H_
>> +#define _DW_MMC_ZX_H_
>> +
>> +/* dll reg offset*/
>> +#define LB_AON_EMMC_CFG_REG0 0x1B0
>> +#define LB_AON_EMMC_CFG_REG1 0x1B4
>> +#define LB_AON_EMMC_CFG_REG2 0x1B8
>> +
>> +/* LB_AON_EMMC_CFG_REG0 register defines */
>> +#define PARA_DLL_START_POINT(x) (((x) & 0xFF) << 0)
>> +#define DLL_REG_SET BIT(8)
>> +#define PARA_DLL_LOCK_NUM(x) (((x) & 7) << 16)
>> +#define PARA_PHASE_DET_SEL(x) (((x) & 7) << 20)
>> +#define PARA_DLL_BYPASS_MODE BIT(23)
>> +#define PARA_HALF_CLK_MODE BIT(24)
>> +
>> +/* LB_AON_EMMC_CFG_REG1 register defines */
>> +#define READ_DQS_DELAY(x) (((x) & 0x7F) << 0)
>> +#define READ_DQS_BYPASS_MODE BIT(7)
>> +#define CLK_SAMP_DELAY(x) (((x) & 0x7F) << 8)
>> +#define CLK_SAMP_BYPASS_MODE BIT(15)
>> +
>> +#endif /* _DW_MMC_ZX_H_ */
>>
>
>
> --
> Best Regards
> Shawn Lin
>
^ permalink raw reply
* Re: [PATCH 3/5] Documentation: synopsys-dw-mshc: add binding for fifo quirks
From: Jun Nie @ 2016-10-25 8:33 UTC (permalink / raw)
To: Jaehoon Chung
Cc: Shawn Lin, Shawn Guo, xie.baoyou, Ulf Hansson, Jason Liu,
linux-mmc
In-Reply-To: <5c0758f5-420c-55e5-86e3-a9dfef73b52e@samsung.com>
2016-10-25 16:09 GMT+08:00 Jaehoon Chung <jh80.chung@samsung.com>:
> On 10/25/2016 01:17 PM, Shawn Lin wrote:
>> On 2016/10/25 10:16, Jaehoon Chung wrote:
>>> On 10/24/2016 09:19 PM, Shawn Lin wrote:
>>>> On 2016/10/24 17:11, Jun Nie wrote:
>>>>> Add fifo-addr-override property and fifo-watermark-quirk property to
>>>>> synopsys-dw-mshc bindings. It is intended to provide workarounds to
>>>>> support more SoCs that break current assumption.
>>>>>
>>>>> See Documentation/devicetree/bindings/reset/reset.txt for details.
>>>>>
>>>>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>>>>> ---
>>>>> Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt | 13 +++++++++++++
>>>>> 1 file changed, 13 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>>>>> index 4e00e85..eb64921 100644
>>>>> --- a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>>>>> +++ b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
>>>>> @@ -76,6 +76,17 @@ Optional properties:
>>>>>
>>>>> * broken-cd: as documented in mmc core bindings.
>>>>>
>>>>> +* fifo-addr-override: Override fifo address with value provided by DT. The FIFO
>>>>> + reg offset of version 0x210A break current assumption that 0x100 (version < 0x240A)
>>>>> + and 0x200(version >= 0x240A) in some implementation. So this property serves as
>>>>> + workaround.
>>>>
>>>> Can we hardcode this to the code by checking the host version?
>>>
>>> I think it should be not workaround..According to TRM, Address is equal or greater than 0x100.
>>> It means address can be 0x200, right?
>>> If you needs to overwrite the DATA register offset for your target, just can add the property for this.
>>>
>>
>> I can't follow yours here as I don't have 210A TRM. Do you mean the TRM
>> for 210A say: "Address is equal or greater than 0x100" ?
>
> Upper version than IP 2.40a is used the offset 0x100 as other purpose.
> For preventing to use wrong register. we put to check host->verid.
>
> Yes, can set the address to any value 0x100 or greater.
> If some soc needs to use 0x200, why not?
>
> But it's not true that FIFO reg offset of version 2.10a break.
> I have checked 2.10a TRM..It's DATA offset >= 0x100.
>
> Best Regards,
> Jaehoon Chung
OK, will change property name to "fifo-addr" to set fifo reg address
via property in next version. I do not have 2.10a TRM and had assumed
current code is following the TRM.
Jun
>
>>
>>>>
>>>>> +
>>>>> +* fifo-watermark-quirk: Data done irq is expected if data length is less than
>>>>> + watermark in PIO mode. But fifo watermark is requested to be aligned with data
>>>>> + length in some SoC so that TX/RX irq can be generated with data done irq. Add the
>>>>> + watermark quirk to mark this requirement and force fifo watermark setting
>>>>> + accordingly.
>>>>
>>>> I would like to know if this limitation is *really* related to some
>>>> Socs or the version of 210A dw_mmc?
>>>>
>>>>
>>>>> +
>>>>> * vmmc-supply: The phandle to the regulator to use for vmmc. If this is
>>>>> specified we'll defer probe until we can find this regulator.
>>>>>
>>>>> @@ -103,6 +114,8 @@ board specific portions as listed below.
>>>>> interrupts = <0 75 0>;
>>>>> #address-cells = <1>;
>>>>> #size-cells = <0>;
>>>>> + fifo-addr-override = <0x200>;
>>>>> + fifo-watermark-quirk;
>>>>> };
>>>>>
>>>>> [board specific internal DMA resources]
>>>>>
>>>>
>>>>
>>>
>>>
>>>
>>>
>>
>>
>
^ permalink raw reply
* Re: [PATCH 4/4] mmc: core: Don't use ->card_busy() and CMD13 in combination when polling
From: Ulf Hansson @ 2016-10-25 8:44 UTC (permalink / raw)
To: Linus Walleij
Cc: linux-mmc@vger.kernel.org, Jaehoon Chung, Adrian Hunter,
Chaotian Jing
In-Reply-To: <CACRpkdbASwAgV2kTzVAMTHwC5fsGuEsJuYXq5n5VKgXmUqMfgA@mail.gmail.com>
On 24 October 2016 at 09:49, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Thu, Oct 20, 2016 at 10:19 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
>> When polling for busy after sending a MMC_SWITCH command, both the optional
>> ->card_busy() callback and CMD13 are being used in conjunction.
>>
>> This doesn't make sense. Instead it's more reasonable to rely solely on the
>> ->card_busy() callback when it exists. Let's change that and instead use
>> the CMD13 as a fall-back. In this way we avoid sending CMD13, unless it's
>> really needed.
>>
>> Within this context, let's also take the opportunity to make some
>> additional clean-ups and clarifications to the related code.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>
> Tried these four patches but it doesn't make my root mount before
> 20 minutes on the Dragonboard, but I don't know if that was the
> intention even? Just cleanup? Sorry if I got it wrong...
Apologize if this was confusing, these changes was not intended to fix
your reported problem. They are clean-ups and the last change improves
the polling loop for switch commands a bit.
I will post a patch for the reported regression asap and will keep you
on cc. Appreciate if you could help in testing!
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH 4/4] mmc: core: Don't use ->card_busy() and CMD13 in combination when polling
From: Ulf Hansson @ 2016-10-25 8:45 UTC (permalink / raw)
To: Adrian Hunter; +Cc: linux-mmc, Jaehoon Chung, Linus Walleij, Chaotian Jing
In-Reply-To: <e9e44d6e-f2a6-13f0-4938-3fa1757b40e8@intel.com>
On 21 October 2016 at 11:19, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 20/10/16 11:19, Ulf Hansson wrote:
>> When polling for busy after sending a MMC_SWITCH command, both the optional
>> ->card_busy() callback and CMD13 are being used in conjunction.
>>
>> This doesn't make sense. Instead it's more reasonable to rely solely on the
>> ->card_busy() callback when it exists. Let's change that and instead use
>> the CMD13 as a fall-back. In this way we avoid sending CMD13, unless it's
>> really needed.
>>
>> Within this context, let's also take the opportunity to make some
>> additional clean-ups and clarifications to the related code.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>> ---
>> drivers/mmc/core/mmc_ops.c | 30 ++++++++++++++----------------
>> 1 file changed, 14 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
>> index a84a880..481bbdb 100644
>> --- a/drivers/mmc/core/mmc_ops.c
>> +++ b/drivers/mmc/core/mmc_ops.c
>> @@ -495,34 +495,32 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
>> timeout = jiffies + msecs_to_jiffies(timeout_ms) + 1;
>> do {
>> /*
>> - * Due to the possibility of being preempted after
>> - * sending the status command, check the expiration
>> - * time first.
>> + * Due to the possibility of being preempted while polling,
>> + * check the expiration time first.
>> */
>> expired = time_after(jiffies, timeout);
>> - if (send_status) {
>> +
>> + if (host->ops->card_busy) {
>> + busy = host->ops->card_busy(host);
>
> I didn't really have time to look at these patches, sorry :-(. But this
> loop looks like it could use a cond_resched()
Yes, something like that is definitely needed! Although, I suggest we
do that improvement on top of this change, if you are fine with that
approach?
The reason is that I am also pondering over, whether it could make
sense to poll with a dynamically increased interval. At least when
using ->card_busy().
Let's say by starting at 1 us interval, then at each poll attempt we
double the interval time. We would then rather use a combination of
udelay(), usleep_range() and msleep() to accomplish what you propose.
Does that make sense to you?
Kind regards
Uffe
^ permalink raw reply
* Re: [GIT PULL] Update dw-mmc controller
From: Ulf Hansson @ 2016-10-25 8:48 UTC (permalink / raw)
To: Jaehoon Chung; +Cc: linux-mmc@vger.kernel.org, Shawn Lin
In-Reply-To: <010f8179-56e4-88d2-1a6f-1c477a1d3f4a@samsung.com>
On 24 October 2016 at 00:52, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Dear Ulf,
>
> Could you pull these patches into your mmc-next?
>
> The following changes since commit 2a688788ba1dd1dc905c24bf5b617aefdff4a68b:
>
> mmc: core: Don't use ->card_busy() and CMD13 in combination when polling (2016-10-21 10:18:07 +0200)
>
> are available in the git repository at:
>
> https://github.com/jh80chung/dw-mmc.git for-ulf
>
> for you to fetch changes up to 0a9bd53611e862d2a5e2f68513b2677b13b7b379:
>
> mmc: dw_mmc: remove system PM callback (2016-10-24 07:13:07 +0900)
>
> ----------------------------------------------------------------
> 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: disable biu clk if possible
> mmc: dw_mmc-k3: deploy runtime PM facilities
> mmc: dw_mmc-exynos: deploy runtime PM facilities
> mmc: dw_mmc-pci: deploy runtime PM facilities
> mmc: dw_mmc-pltfm: deploy runtime PM facilities
> mmc: dw_mmc: remove system PM callback
>
> drivers/mmc/core/slot-gpio.c | 8 +++++++
> drivers/mmc/host/dw_mmc-exynos.c | 24 +++++++++-----------
> drivers/mmc/host/dw_mmc-k3.c | 39 ++++++++------------------------
> drivers/mmc/host/dw_mmc-pci.c | 29 ++++++++----------------
> drivers/mmc/host/dw_mmc-pltfm.c | 28 +++++++----------------
> drivers/mmc/host/dw_mmc-rockchip.c | 42 +++++++++++++++++++++++++++++++---
> drivers/mmc/host/dw_mmc.c | 46 +++++++++++++++++++++++++-------------
> drivers/mmc/host/dw_mmc.h | 6 ++---
> include/linux/mmc/slot-gpio.h | 1 +
> 9 files changed, 117 insertions(+), 106 deletions(-)
>
> Best Regards,
> Jaehoon Chung
Thanks, pulled into my next branch.
I noticed that you probably forgot to add your sob-tag to the changes,
so I did that. Pleas tell me if that wasn't okay.
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH] mmc: dw_mmc-pltfm: fix the potential NULL pointer dereference
From: Ulf Hansson @ 2016-10-25 8:48 UTC (permalink / raw)
To: Jaehoon Chung; +Cc: linux-mmc, Shawn Lin, # 4.0+, Greg Kroah-Hartman
In-Reply-To: <20161021105757.22712-1-jh80.chung@samsung.com>
On 21 October 2016 at 12:57, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> platform_get_resource can be returned the NULL pointer.
> Then regs->start should be referred to NULL Pointer.
> devm_ioremap_resource() checks whether res is NULL or not.
>
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
Thanks, applied for fixes abd I also added a stable tag.
Kind regards
Uffe
> ---
> drivers/mmc/host/dw_mmc-pltfm.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c
> index b486fef..1236d49b 100644
> --- a/drivers/mmc/host/dw_mmc-pltfm.c
> +++ b/drivers/mmc/host/dw_mmc-pltfm.c
> @@ -47,12 +47,13 @@ int dw_mci_pltfm_register(struct platform_device *pdev,
> host->pdata = pdev->dev.platform_data;
>
> regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - /* Get registers' physical base address */
> - host->phy_regs = regs->start;
> host->regs = devm_ioremap_resource(&pdev->dev, regs);
> if (IS_ERR(host->regs))
> return PTR_ERR(host->regs);
>
> + /* Get registers' physical base address */
> + host->phy_regs = regs->start;
> +
> platform_set_drvdata(pdev, host);
> return dw_mci_probe(host);
> }
> --
> 2.10.1
>
^ permalink raw reply
* [PATCH 1/3 v2] mmc: mmci: clean up header defines
From: Linus Walleij @ 2016-10-25 9:06 UTC (permalink / raw)
To: Ulf Hansson, linux-mmc, linux-arm-kernel, Srinivas Kandagatla
Cc: Russell King, Linus Walleij
There was some confusion in the CPSM (Command Path State Machine)
and DPSM (Data Path State Machine) regarding the naming of the
registers, clarify the meaning of this acronym so the naming is
understandable, and consistently use BIT() to define these fields.
Consequently name the register bit defines MCI_[C|D]PSM_* and
adjust the driver as well.
Include new definitions for a few bits found in a patch from
Srinivas Kandagatla.
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Use more consequent register naming.
---
drivers/mmc/host/mmci.c | 16 ++++++------
drivers/mmc/host/mmci.h | 69 +++++++++++++++++++++++++++----------------------
2 files changed, 46 insertions(+), 39 deletions(-)
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index df990bb8c873..6a8ea9c633d4 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -137,7 +137,7 @@ static struct variant_data variant_u300 = {
.clkreg_enable = MCI_ST_U300_HWFCEN,
.clkreg_8bit_bus_enable = MCI_ST_8BIT_BUS,
.datalength_bits = 16,
- .datactrl_mask_sdio = MCI_ST_DPSM_SDIOEN,
+ .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN,
.st_sdio = true,
.pwrreg_powerup = MCI_PWR_ON,
.f_max = 100000000,
@@ -152,7 +152,7 @@ static struct variant_data variant_nomadik = {
.clkreg = MCI_CLK_ENABLE,
.clkreg_8bit_bus_enable = MCI_ST_8BIT_BUS,
.datalength_bits = 24,
- .datactrl_mask_sdio = MCI_ST_DPSM_SDIOEN,
+ .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN,
.st_sdio = true,
.st_clkdiv = true,
.pwrreg_powerup = MCI_PWR_ON,
@@ -170,7 +170,7 @@ static struct variant_data variant_ux500 = {
.clkreg_8bit_bus_enable = MCI_ST_8BIT_BUS,
.clkreg_neg_edge_enable = MCI_ST_UX500_NEG_EDGE,
.datalength_bits = 24,
- .datactrl_mask_sdio = MCI_ST_DPSM_SDIOEN,
+ .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN,
.st_sdio = true,
.st_clkdiv = true,
.pwrreg_powerup = MCI_PWR_ON,
@@ -188,9 +188,9 @@ static struct variant_data variant_ux500v2 = {
.clkreg_enable = MCI_ST_UX500_HWFCEN,
.clkreg_8bit_bus_enable = MCI_ST_8BIT_BUS,
.clkreg_neg_edge_enable = MCI_ST_UX500_NEG_EDGE,
- .datactrl_mask_ddrmode = MCI_ST_DPSM_DDRMODE,
+ .datactrl_mask_ddrmode = MCI_DPSM_ST_DDRMODE,
.datalength_bits = 24,
- .datactrl_mask_sdio = MCI_ST_DPSM_SDIOEN,
+ .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN,
.st_sdio = true,
.st_clkdiv = true,
.blksz_datactrl16 = true,
@@ -210,7 +210,7 @@ static struct variant_data variant_qcom = {
MCI_QCOM_CLK_SELECT_IN_FBCLK,
.clkreg_8bit_bus_enable = MCI_QCOM_CLK_WIDEBUS_8,
.datactrl_mask_ddrmode = MCI_QCOM_CLK_SELECT_IN_DDR_MODE,
- .data_cmd_enable = MCI_QCOM_CSPM_DATCMD,
+ .data_cmd_enable = MCI_CPSM_QCOM_DATCMD,
.blksz_datactrl4 = true,
.datalength_bits = 24,
.pwrreg_powerup = MCI_PWR_UP,
@@ -295,7 +295,7 @@ static void mmci_write_pwrreg(struct mmci_host *host, u32 pwr)
static void mmci_write_datactrlreg(struct mmci_host *host, u32 datactrl)
{
/* Keep ST Micro busy mode if enabled */
- datactrl |= host->datactrl_reg & MCI_ST_DPSM_BUSYMODE;
+ datactrl |= host->datactrl_reg & MCI_DPSM_ST_BUSYMODE;
if (host->datactrl_reg != datactrl) {
host->datactrl_reg = datactrl;
@@ -1614,7 +1614,7 @@ static int mmci_probe(struct amba_device *dev,
if (variant->busy_detect) {
mmci_ops.card_busy = mmci_card_busy;
- mmci_write_datactrlreg(host, MCI_ST_DPSM_BUSYMODE);
+ mmci_write_datactrlreg(host, MCI_DPSM_ST_BUSYMODE);
mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY;
mmc->max_busy_timeout = 0;
}
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index a1f5e4f49e2a..7cabf270050b 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -51,25 +51,27 @@
#define MCI_QCOM_CLK_SELECT_IN_DDR_MODE (BIT(14) | BIT(15))
#define MMCIARGUMENT 0x008
-#define MMCICOMMAND 0x00c
-#define MCI_CPSM_RESPONSE (1 << 6)
-#define MCI_CPSM_LONGRSP (1 << 7)
-#define MCI_CPSM_INTERRUPT (1 << 8)
-#define MCI_CPSM_PENDING (1 << 9)
-#define MCI_CPSM_ENABLE (1 << 10)
-/* Argument flag extenstions in the ST Micro versions */
-#define MCI_ST_SDIO_SUSP (1 << 11)
-#define MCI_ST_ENCMD_COMPL (1 << 12)
-#define MCI_ST_NIEN (1 << 13)
-#define MCI_ST_CE_ATACMD (1 << 14)
-/* Modified on Qualcomm Integrations */
-#define MCI_QCOM_CSPM_DATCMD BIT(12)
-#define MCI_QCOM_CSPM_MCIABORT BIT(13)
-#define MCI_QCOM_CSPM_CCSENABLE BIT(14)
-#define MCI_QCOM_CSPM_CCSDISABLE BIT(15)
-#define MCI_QCOM_CSPM_AUTO_CMD19 BIT(16)
-#define MCI_QCOM_CSPM_AUTO_CMD21 BIT(21)
+/* The command register controls the Command Path State Machine (CPSM) */
+#define MMCICOMMAND 0x00c
+#define MCI_CPSM_RESPONSE BIT(6)
+#define MCI_CPSM_LONGRSP BIT(7)
+#define MCI_CPSM_INTERRUPT BIT(8)
+#define MCI_CPSM_PENDING BIT(9)
+#define MCI_CPSM_ENABLE BIT(10)
+/* Command register flag extenstions in the ST Micro versions */
+#define MCI_CPSM_ST_SDIO_SUSP BIT(11)
+#define MCI_CPSM_ST_ENCMD_COMPL BIT(12)
+#define MCI_CPSM_ST_NIEN BIT(13)
+#define MCI_CPSM_ST_CE_ATACMD BIT(14)
+/* Command register flag extensions in the Qualcomm versions */
+#define MCI_CPSM_QCOM_PROGENA BIT(11)
+#define MCI_CPSM_QCOM_DATCMD BIT(12)
+#define MCI_CPSM_QCOM_MCIABORT BIT(13)
+#define MCI_CPSM_QCOM_CCSENABLE BIT(14)
+#define MCI_CPSM_QCOM_CCSDISABLE BIT(15)
+#define MCI_CPSM_QCOM_AUTO_CMD19 BIT(16)
+#define MCI_CPSM_QCOM_AUTO_CMD21 BIT(21)
#define MMCIRESPCMD 0x010
#define MMCIRESPONSE0 0x014
@@ -78,22 +80,27 @@
#define MMCIRESPONSE3 0x020
#define MMCIDATATIMER 0x024
#define MMCIDATALENGTH 0x028
+
+/* The data control register controls the Data Path State Machine (DPSM) */
#define MMCIDATACTRL 0x02c
-#define MCI_DPSM_ENABLE (1 << 0)
-#define MCI_DPSM_DIRECTION (1 << 1)
-#define MCI_DPSM_MODE (1 << 2)
-#define MCI_DPSM_DMAENABLE (1 << 3)
-#define MCI_DPSM_BLOCKSIZE (1 << 4)
+#define MCI_DPSM_ENABLE BIT(0)
+#define MCI_DPSM_DIRECTION BIT(1)
+#define MCI_DPSM_MODE BIT(2)
+#define MCI_DPSM_DMAENABLE BIT(3)
+#define MCI_DPSM_BLOCKSIZE BIT(4)
/* Control register extensions in the ST Micro U300 and Ux500 versions */
-#define MCI_ST_DPSM_RWSTART (1 << 8)
-#define MCI_ST_DPSM_RWSTOP (1 << 9)
-#define MCI_ST_DPSM_RWMOD (1 << 10)
-#define MCI_ST_DPSM_SDIOEN (1 << 11)
+#define MCI_DPSM_ST_RWSTART BIT(8)
+#define MCI_DPSM_ST_RWSTOP BIT(9)
+#define MCI_DPSM_ST_RWMOD BIT(10)
+#define MCI_DPSM_ST_SDIOEN BIT(11)
/* Control register extensions in the ST Micro Ux500 versions */
-#define MCI_ST_DPSM_DMAREQCTL (1 << 12)
-#define MCI_ST_DPSM_DBOOTMODEEN (1 << 13)
-#define MCI_ST_DPSM_BUSYMODE (1 << 14)
-#define MCI_ST_DPSM_DDRMODE (1 << 15)
+#define MCI_DPSM_ST_DMAREQCTL BIT(12)
+#define MCI_DPSM_ST_DBOOTMODEEN BIT(13)
+#define MCI_DPSM_ST_BUSYMODE BIT(14)
+#define MCI_DPSM_ST_DDRMODE BIT(15)
+/* Control register extensions in the Qualcomm versions */
+#define MCI_DPSM_QCOM_DATA_PEND BIT(17)
+#define MCI_DPSM_QCOM_RX_DATA_PEND BIT(20)
#define MMCIDATACNT 0x030
#define MMCISTATUS 0x034
--
2.7.4
^ permalink raw reply related
* [PATCH 2/3 v2] mmc: mmci: refactor ST Micro busy detection
From: Linus Walleij @ 2016-10-25 9:06 UTC (permalink / raw)
To: Ulf Hansson, linux-mmc, linux-arm-kernel, Srinivas Kandagatla
Cc: Russell King, Linus Walleij
In-Reply-To: <1477386367-18514-1-git-send-email-linus.walleij@linaro.org>
The ST Micro-specific busy detection was made after the assumption
that only this variant supports busy detection. So when doing busy
detection, the host immediately tries to use some ST-specific
register bits.
Since the qualcomm variant also supports some busy detection
schemes, encapsulate the variant flags better in the variant struct
and prepare to add more variants by just providing some bitmasks
to the logic.
Put the entire busy detection logic within an if()-clause in the
mmci_cmd_irq() function so the code is only executed when busy
detection is enabled, and so that it is kept in (almost) one
place, and add comments describing what is going on so the
code can be understood.
Tested on the Ux500 by introducing some prints in the busy
detection path and noticing how the IRQ is enabled, used and
disabled successfully.
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Rebase on new register naming.
---
drivers/mmc/host/mmci.c | 113 +++++++++++++++++++++++++++++++++++-------------
drivers/mmc/host/mmci.h | 2 +-
2 files changed, 85 insertions(+), 30 deletions(-)
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 6a8ea9c633d4..7f68fa7a961e 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -71,7 +71,12 @@ static unsigned int fmax = 515633;
* @f_max: maximum clk frequency supported by the controller.
* @signal_direction: input/out direction of bus signals can be indicated
* @pwrreg_clkgate: MMCIPOWER register must be used to gate the clock
- * @busy_detect: true if busy detection on dat0 is supported
+ * @busy_detect: true if the variant supports busy detection on DAT0.
+ * @busy_dpsm_flag: bitmask enabling busy detection in the DPSM
+ * @busy_detect_flag: bitmask identifying the bit in the MMCISTATUS register
+ * indicating that the card is busy
+ * @busy_detect_mask: bitmask identifying the bit in the MMCIMASK0 to mask for
+ * getting busy end detection interrupts
* @pwrreg_nopower: bits in MMCIPOWER don't controls ext. power supply
* @explicit_mclk_control: enable explicit mclk control in driver.
* @qcom_fifo: enables qcom specific fifo pio read logic.
@@ -98,6 +103,9 @@ struct variant_data {
bool signal_direction;
bool pwrreg_clkgate;
bool busy_detect;
+ u32 busy_dpsm_flag;
+ u32 busy_detect_flag;
+ u32 busy_detect_mask;
bool pwrreg_nopower;
bool explicit_mclk_control;
bool qcom_fifo;
@@ -178,6 +186,9 @@ static struct variant_data variant_ux500 = {
.signal_direction = true,
.pwrreg_clkgate = true,
.busy_detect = true,
+ .busy_dpsm_flag = MCI_DPSM_ST_BUSYMODE,
+ .busy_detect_flag = MCI_ST_CARDBUSY,
+ .busy_detect_mask = MCI_ST_BUSYENDMASK,
.pwrreg_nopower = true,
};
@@ -199,6 +210,9 @@ static struct variant_data variant_ux500v2 = {
.signal_direction = true,
.pwrreg_clkgate = true,
.busy_detect = true,
+ .busy_dpsm_flag = MCI_DPSM_ST_BUSYMODE,
+ .busy_detect_flag = MCI_ST_CARDBUSY,
+ .busy_detect_mask = MCI_ST_BUSYENDMASK,
.pwrreg_nopower = true,
};
@@ -220,6 +234,7 @@ static struct variant_data variant_qcom = {
.qcom_dml = true,
};
+/* Busy detection for the ST Micro variant */
static int mmci_card_busy(struct mmc_host *mmc)
{
struct mmci_host *host = mmc_priv(mmc);
@@ -227,7 +242,7 @@ static int mmci_card_busy(struct mmc_host *mmc)
int busy = 0;
spin_lock_irqsave(&host->lock, flags);
- if (readl(host->base + MMCISTATUS) & MCI_ST_CARDBUSY)
+ if (readl(host->base + MMCISTATUS) & host->variant->busy_detect_flag)
busy = 1;
spin_unlock_irqrestore(&host->lock, flags);
@@ -294,8 +309,8 @@ static void mmci_write_pwrreg(struct mmci_host *host, u32 pwr)
*/
static void mmci_write_datactrlreg(struct mmci_host *host, u32 datactrl)
{
- /* Keep ST Micro busy mode if enabled */
- datactrl |= host->datactrl_reg & MCI_DPSM_ST_BUSYMODE;
+ /* Keep busy mode in DPSM if enabled */
+ datactrl |= host->datactrl_reg & host->variant->busy_dpsm_flag;
if (host->datactrl_reg != datactrl) {
host->datactrl_reg = datactrl;
@@ -973,37 +988,66 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
unsigned int status)
{
void __iomem *base = host->base;
- bool sbc, busy_resp;
+ bool sbc;
if (!cmd)
return;
sbc = (cmd == host->mrq->sbc);
- busy_resp = host->variant->busy_detect && (cmd->flags & MMC_RSP_BUSY);
- if (!((status|host->busy_status) & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT|
- MCI_CMDSENT|MCI_CMDRESPEND)))
+ /*
+ * We need to be one of these interrupts to be considered worth
+ * handling. Note that we tag on any latent IRQs postponed
+ * due to waiting for busy status.
+ */
+ if (!((status|host->busy_status) &
+ (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT|MCI_CMDSENT|MCI_CMDRESPEND)))
return;
- /* Check if we need to wait for busy completion. */
- if (host->busy_status && (status & MCI_ST_CARDBUSY))
- return;
+ /*
+ * ST Micro variant: handle busy detection.
+ */
+ if (host->variant->busy_detect) {
+ bool busy_resp = !!(cmd->flags & MMC_RSP_BUSY);
- /* Enable busy completion if needed and supported. */
- if (!host->busy_status && busy_resp &&
- !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) &&
- (readl(base + MMCISTATUS) & MCI_ST_CARDBUSY)) {
- writel(readl(base + MMCIMASK0) | MCI_ST_BUSYEND,
- base + MMCIMASK0);
- host->busy_status = status & (MCI_CMDSENT|MCI_CMDRESPEND);
- return;
- }
+ /* We are busy with a command, return */
+ if (host->busy_status &&
+ (status & host->variant->busy_detect_flag))
+ return;
+
+ /*
+ * We were not busy, but we now got a busy response on
+ * something that was not an error, and we double-check
+ * that the special busy status bit is still set before
+ * proceeding.
+ */
+ if (!host->busy_status && busy_resp &&
+ !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) &&
+ (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) {
+ /* Unmask the busy IRQ */
+ writel(readl(base + MMCIMASK0) |
+ host->variant->busy_detect_mask,
+ base + MMCIMASK0);
+ /*
+ * Now cache the last response status code (until
+ * the busy bit goes low), and return.
+ */
+ host->busy_status =
+ status & (MCI_CMDSENT|MCI_CMDRESPEND);
+ return;
+ }
- /* At busy completion, mask the IRQ and complete the request. */
- if (host->busy_status) {
- writel(readl(base + MMCIMASK0) & ~MCI_ST_BUSYEND,
- base + MMCIMASK0);
- host->busy_status = 0;
+ /*
+ * At this point we are not busy with a command, we have
+ * not recieved a new busy request, mask the busy IRQ and
+ * fall through to process the IRQ.
+ */
+ if (host->busy_status) {
+ writel(readl(base + MMCIMASK0) &
+ ~host->variant->busy_detect_mask,
+ base + MMCIMASK0);
+ host->busy_status = 0;
+ }
}
host->cmd = NULL;
@@ -1257,9 +1301,11 @@ static irqreturn_t mmci_irq(int irq, void *dev_id)
mmci_data_irq(host, host->data, status);
}
- /* Don't poll for busy completion in irq context. */
- if (host->busy_status)
- status &= ~MCI_ST_CARDBUSY;
+ /*
+ * Don't poll for busy completion in irq context.
+ */
+ if (host->variant->busy_detect && host->busy_status)
+ status &= ~host->variant->busy_detect_flag;
ret = 1;
} while (status);
@@ -1612,9 +1658,18 @@ static int mmci_probe(struct amba_device *dev,
/* We support these capabilities. */
mmc->caps |= MMC_CAP_CMD23;
+ /*
+ * Enable busy detection.
+ */
if (variant->busy_detect) {
mmci_ops.card_busy = mmci_card_busy;
- mmci_write_datactrlreg(host, MCI_DPSM_ST_BUSYMODE);
+ /*
+ * Not all variants have a flag to enable busy detection
+ * in the DPSM, but if they do, set it here.
+ */
+ if (variant->busy_dpsm_flag)
+ mmci_write_datactrlreg(host,
+ host->variant->busy_dpsm_flag);
mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY;
mmc->max_busy_timeout = 0;
}
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 7cabf270050b..56322c6afba4 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -174,7 +174,7 @@
/* Extended status bits for the ST Micro variants */
#define MCI_ST_SDIOITMASK (1 << 22)
#define MCI_ST_CEATAENDMASK (1 << 23)
-#define MCI_ST_BUSYEND (1 << 24)
+#define MCI_ST_BUSYENDMASK (1 << 24)
#define MMCIMASK1 0x040
#define MMCIFIFOCNT 0x048
--
2.7.4
^ permalink raw reply related
* [PATCH 3/3 v2] RFC: mmc: mmci: add qcom specific program end support
From: Linus Walleij @ 2016-10-25 9:06 UTC (permalink / raw)
To: Ulf Hansson, linux-mmc, linux-arm-kernel, Srinivas Kandagatla
Cc: Russell King, Linus Walleij
In-Reply-To: <1477386367-18514-1-git-send-email-linus.walleij@linaro.org>
From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
This patch adds support to programend interrupt which is very
specific to QCOM integration. This interrupt is use as busy signal
when a command forces the card to enter into programming state
like CMD6 writing to ext_csd registers.
Hopefully, this also fixes the __mmc_switch timeout issue reproted
with latest versions of the eMMC used on DB600c board.
This patch is based on a WIP patch from Srinivas Kandagatla and
augmented by Linus Walleij for another approach.
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Rebase on new register naming.
Srinivas: please test to see if this fixes your problems. Sadly
it does *NOT* solve my APQ8060 issues, but it would be nice if
the common code path works for the busy detection on your
DB600c.
---
drivers/mmc/host/mmci.c | 21 +++++++++++++++++++--
drivers/mmc/host/mmci.h | 6 ++++++
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 7f68fa7a961e..580ca89d33dc 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -72,7 +72,10 @@ static unsigned int fmax = 515633;
* @signal_direction: input/out direction of bus signals can be indicated
* @pwrreg_clkgate: MMCIPOWER register must be used to gate the clock
* @busy_detect: true if the variant supports busy detection on DAT0.
- * @busy_dpsm_flag: bitmask enabling busy detection in the DPSM
+ * @busy_cpsm_flag: bitmask enabling busy detection in the CPSM (command
+ * path state machine)
+ * @busy_dpsm_flag: bitmask enabling busy detection in the DPSM (data path
+ * state machine)
* @busy_detect_flag: bitmask identifying the bit in the MMCISTATUS register
* indicating that the card is busy
* @busy_detect_mask: bitmask identifying the bit in the MMCIMASK0 to mask for
@@ -103,6 +106,7 @@ struct variant_data {
bool signal_direction;
bool pwrreg_clkgate;
bool busy_detect;
+ u32 busy_cpsm_flag;
u32 busy_dpsm_flag;
u32 busy_detect_flag;
u32 busy_detect_mask;
@@ -229,6 +233,10 @@ static struct variant_data variant_qcom = {
.datalength_bits = 24,
.pwrreg_powerup = MCI_PWR_UP,
.f_max = 208000000,
+ .busy_detect = true,
+ .busy_cpsm_flag = MCI_CPSM_QCOM_PROGENA,
+ .busy_detect_flag = MCI_QCOM_PROGDONE,
+ .busy_detect_mask = MCI_QCOM_PROGDONEMASK,
.explicit_mclk_control = true,
.qcom_fifo = true,
.qcom_dml = true,
@@ -903,6 +911,15 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c)
if (/*interrupt*/0)
c |= MCI_CPSM_INTERRUPT;
+ /*
+ * Enable the program end interrupt for specific commands
+ * used for busy detection.
+ */
+ if (host->variant->busy_detect &&
+ (cmd->flags & MMC_RSP_R1B) == MMC_RSP_R1B) {
+ c |= host->variant->busy_cpsm_flag;
+ }
+
if (mmc_cmd_type(cmd) == MMC_CMD_ADTC)
c |= host->variant->data_cmd_enable;
@@ -1005,7 +1022,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd,
return;
/*
- * ST Micro variant: handle busy detection.
+ * ST Micro and Qualcomm variants: handle busy detection.
*/
if (host->variant->busy_detect) {
bool busy_resp = !!(cmd->flags & MMC_RSP_BUSY);
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 56322c6afba4..355259b0a24b 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -130,6 +130,8 @@
#define MCI_ST_SDIOIT (1 << 22)
#define MCI_ST_CEATAEND (1 << 23)
#define MCI_ST_CARDBUSY (1 << 24)
+/* Extended status bits for the QCOM variants */
+#define MCI_QCOM_PROGDONE (1 << 23)
#define MMCICLEAR 0x038
#define MCI_CMDCRCFAILCLR (1 << 0)
@@ -147,6 +149,8 @@
#define MCI_ST_SDIOITC (1 << 22)
#define MCI_ST_CEATAENDC (1 << 23)
#define MCI_ST_BUSYENDC (1 << 24)
+/* Extended status bits for the QCOM variants */
+#define MCI_QCOM_PROGDONECLR (1 << 23)
#define MMCIMASK0 0x03c
#define MCI_CMDCRCFAILMASK (1 << 0)
@@ -175,6 +179,8 @@
#define MCI_ST_SDIOITMASK (1 << 22)
#define MCI_ST_CEATAENDMASK (1 << 23)
#define MCI_ST_BUSYENDMASK (1 << 24)
+/* Extended status bits for the Qualcomm variants */
+#define MCI_QCOM_PROGDONEMASK (1 << 23)
#define MMCIMASK1 0x040
#define MMCIFIFOCNT 0x048
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] mmc: Hynix: add QUIRK_NOTIFY_POWEROFF_ON_SLEEP
From: Ulf Hansson @ 2016-10-25 10:03 UTC (permalink / raw)
To: Thierry Escande; +Cc: linux-mmc
In-Reply-To: <1475504388-31304-1-git-send-email-thierry.escande@collabora.com>
On 3 October 2016 at 16:19, Thierry Escande
<thierry.escande@collabora.com> wrote:
> From: zhaojohn <john.zhao@intel.com>
>
> Hynix eMMC devices sometimes take 50% longer to resume from sleep.
> Based on a recommendation from Hynix, send a Power-Off Notification
> before going to S3 to restore a resume time consistently within spec.
Could you also share what mmc controller and SoC you get this results from?
More precisely, are you using MMC_CAP_WAIT_WHILE_BUSY?
>
> Signed-off-by: zhaojohn <john.zhao@intel.com>
> Signed-off-by: Arindam Roy <arindam.roy@intel.com>
> Tested-by: Freddy Paul <freddy.paul@intel.com>
> Reviewed-by: Icarus W Sparry <icarus.w.sparry@intel.com>
> Reviewed-by: Marc Herbert <marc.herbert@intel.com>
> Reviewed-by: Eric Caruso <ejcaruso@chromium.org>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> ---
> drivers/mmc/card/block.c | 8 ++++++++
> drivers/mmc/core/mmc.c | 8 +++++++-
> include/linux/mmc/card.h | 2 ++
> 3 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index 2206d44..cbc2d97 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -2572,6 +2572,14 @@ static const struct mmc_fixup blk_fixups[] =
> MMC_FIXUP("V10016", CID_MANFID_KINGSTON, CID_OEMID_ANY, add_quirk_mmc,
> MMC_QUIRK_TRIM_BROKEN),
>
> + /*
> + * Hynix eMMC devices sometimes take 50% longer to resume from sleep.
> + * Based on a recommendation from Hynix, send a Power-Off Notification
> + * before going to S3 to restore a resume time consistently within spec.
> + */
> + MMC_FIXUP(CID_NAME_ANY, CID_MANFID_HYNIX, CID_OEMID_ANY, add_quirk_mmc,
> + MMC_QUIRK_NOTIFY_POWEROFF_ON_SLEEP),
> +
> END_FIXUP
> };
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index f2d185c..46a4562 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1925,8 +1925,14 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
> if (mmc_can_poweroff_notify(host->card) &&
> ((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend))
> err = mmc_poweroff_notify(host->card, notify_type);
> - else if (mmc_can_sleep(host->card))
> + else if (mmc_can_sleep(host->card)) {
> + if (host->card->quirks & MMC_QUIRK_NOTIFY_POWEROFF_ON_SLEEP) {
> + err = mmc_poweroff_notify(host->card, notify_type);
> + if (err)
> + goto out;
> + }
So, I am curious to know from a power management point of view; how
does the card behave comparing the sleep and power off notification
command?
Is the card in a low power state after the power off notification has
been received? If so, did you manage to do some measurement for that
or perhaps the data-sheet tells about this? It would be interesting to
know if there were any differences between sleep and power off
notification in this regards.
[...]
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH v7 03/11] arm64: dts: r8a7795: salvator-x: Enable UHS-I SDR-104
From: Geert Uytterhoeven @ 2016-10-25 10:48 UTC (permalink / raw)
To: Simon Horman
Cc: Wolfram Sang, Ulf Hansson, Magnus Damm, Linux MMC List,
Linux-Renesas, linux-arm-kernel@lists.infradead.org
In-Reply-To: <1473764228-24768-4-git-send-email-horms+renesas@verge.net.au>
On Tue, Sep 13, 2016 at 12:57 PM, Simon Horman
<horms+renesas@verge.net.au> wrote:
> Add the sd-uhs-sdr104 property to SDHI0 and SDHI1.
SDHI0 and SDHI3?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v7 11/11] arm64: dts: r8a7796: salvator-x: Enable UHS-I SDR-104
From: Geert Uytterhoeven @ 2016-10-25 10:49 UTC (permalink / raw)
To: Simon Horman
Cc: Wolfram Sang, Ulf Hansson, Magnus Damm, Linux MMC List,
Linux-Renesas, linux-arm-kernel@lists.infradead.org
In-Reply-To: <1473764228-24768-12-git-send-email-horms+renesas@verge.net.au>
On Tue, Sep 13, 2016 at 12:57 PM, Simon Horman
<horms+renesas@verge.net.au> wrote:
> Add the sd-uhs-sdr104 property to SDHI0 and SDHI1.
SDHI0 and SDHI3.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 4/4] mmc: core: Don't use ->card_busy() and CMD13 in combination when polling
From: Adrian Hunter @ 2016-10-25 10:58 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, Jaehoon Chung, Linus Walleij, Chaotian Jing
In-Reply-To: <CAPDyKFoMzDvjOvZmuNp7eKz3rx1-sSzhXO2oLAcVj9MHBfZdkQ@mail.gmail.com>
On 25/10/16 11:45, Ulf Hansson wrote:
> On 21 October 2016 at 11:19, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> On 20/10/16 11:19, Ulf Hansson wrote:
>>> When polling for busy after sending a MMC_SWITCH command, both the optional
>>> ->card_busy() callback and CMD13 are being used in conjunction.
>>>
>>> This doesn't make sense. Instead it's more reasonable to rely solely on the
>>> ->card_busy() callback when it exists. Let's change that and instead use
>>> the CMD13 as a fall-back. In this way we avoid sending CMD13, unless it's
>>> really needed.
>>>
>>> Within this context, let's also take the opportunity to make some
>>> additional clean-ups and clarifications to the related code.
>>>
>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>> ---
>>> drivers/mmc/core/mmc_ops.c | 30 ++++++++++++++----------------
>>> 1 file changed, 14 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
>>> index a84a880..481bbdb 100644
>>> --- a/drivers/mmc/core/mmc_ops.c
>>> +++ b/drivers/mmc/core/mmc_ops.c
>>> @@ -495,34 +495,32 @@ static int mmc_poll_for_busy(struct mmc_card *card, unsigned int timeout_ms,
>>> timeout = jiffies + msecs_to_jiffies(timeout_ms) + 1;
>>> do {
>>> /*
>>> - * Due to the possibility of being preempted after
>>> - * sending the status command, check the expiration
>>> - * time first.
>>> + * Due to the possibility of being preempted while polling,
>>> + * check the expiration time first.
>>> */
>>> expired = time_after(jiffies, timeout);
>>> - if (send_status) {
>>> +
>>> + if (host->ops->card_busy) {
>>> + busy = host->ops->card_busy(host);
>>
>> I didn't really have time to look at these patches, sorry :-(. But this
>> loop looks like it could use a cond_resched()
>
> Yes, something like that is definitely needed! Although, I suggest we
> do that improvement on top of this change, if you are fine with that
> approach?
Sure
>
> The reason is that I am also pondering over, whether it could make
> sense to poll with a dynamically increased interval. At least when
> using ->card_busy().
> Let's say by starting at 1 us interval, then at each poll attempt we
> double the interval time. We would then rather use a combination of
> udelay(), usleep_range() and msleep() to accomplish what you propose.
> Does that make sense to you?
That potentially doubles the operation time. It might be nicer to limit the
worst case e.g. sleep 1/8th of the total time spent looping
s64 sleep_us;
ktime_t start_time = ktime_get();
do {
...
sleep_us = ktime_us_delta(ktime_get(), start_time) >> 3;
sleep_us = sleep_us ? : 1;
...
} while(busy);
^ permalink raw reply
* Re: [RFC v2 2/2] mmc: sdhci: Ignore capability register when it comes to speeds and use DT binding instead when sdhci-cap-speed-modes-broken is set.
From: Adrian Hunter @ 2016-10-25 11:30 UTC (permalink / raw)
To: Zach Brown
Cc: ulf.hansson-QSEj5FYQhm4dnm+yROfE0A,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-mmc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161024154820.GA16144@zach-desktop>
On 24/10/16 18:48, Zach Brown wrote:
> On Mon, Oct 24, 2016 at 10:34:46AM +0300, Adrian Hunter wrote:
>> On 22/10/16 00:35, Zach Brown wrote:
>>> When the sdhci-cap-speed-modes-broken DT property is set, the driver
>>> will ignore the bits of the capability registers that correspond to
>>> speed modes.
>>>
>>> Signed-off-by: Zach Brown <zach.brown-acOepvfBmUk@public.gmane.org>
>>> ---
>>> drivers/mmc/host/sdhci.c | 10 ++++++++++
>>> 1 file changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index 1e25b01..59c62d3 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -22,6 +22,7 @@
>>> #include <linux/scatterlist.h>
>>> #include <linux/regulator/consumer.h>
>>> #include <linux/pm_runtime.h>
>>> +#include <linux/of.h>
>>>
>>> #include <linux/leds.h>
>>>
>>> @@ -3013,10 +3014,19 @@ void __sdhci_read_caps(struct sdhci_host *host, u16 *ver, u32 *caps, u32 *caps1)
>>>
>>> host->caps = caps ? *caps : sdhci_readl(host, SDHCI_CAPABILITIES);
>>>
>>> + if (of_property_read_bool(mmc_dev(host->mmc)->of_node,
>>> + "sdhci-cap-speed-modes-broken"))
>>
>> It rather begs the question: if you are going to do something sdhci
>> specific, why not just read the whole of the caps register from DT?
>>
>
> Throwing out the whole of the caps register seems like overkill. Also
What about 2 values: one for the 64-bit caps register and one for a 64-bit
mask of bits to override. That way you can select which bits to override
and what to override them to.
> there are some things set by the caps that are not available in the DT.
> For example, SDHCI_CAN_64BIT is set by the cap register and is used in
> sdhci_setup_host to set host->flags SDHCI_USE_64_BIT_DMA.
Not sure what you mean.
--
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
* [RFC 0/2] mmc: sdhci: Fix sdhci caps register bits with corrections provided by dt
From: Zach Brown @ 2016-10-25 19:58 UTC (permalink / raw)
To: ulf.hansson-QSEj5FYQhm4dnm+yROfE0A
Cc: adrian.hunter-ral2JQCrhuEAvxtiuMwx3w,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-mmc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, zach.brown-acOepvfBmUk
For various reasons the sdhci caps register can be incorrect. This patch set
introduces a general way to correct the bits when they are read to accurately
reflect the capabilties of the controller/board combo.
The first patch creates sdhci-caps and sdhci-caps-mask dt properties that
combined represent the correction to the sdhci caps register.
The second patch uses the new dt properties to correct the caps from the
register as they read during __sdhci_read_caps.
Zach Brown (2):
mmc: sdhci: dt: Add device tree properties sdhci-caps and sdhci-caps-mask
mmc: sdhci: Use sdhci-caps-mask and sdhci-caps to change the caps read
during __sdhci_read_caps
Documentation/devicetree/bindings/mmc/mmc.txt | 7 +++++++
drivers/mmc/host/sdhci.c | 24 ++++++++++++++++++++++--
2 files changed, 29 insertions(+), 2 deletions(-)
--
2.7.4
--
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
* [RFC 1/2] mmc: sdhci: dt: Add device tree properties sdhci-caps and sdhci-caps-mask
From: Zach Brown @ 2016-10-25 19:58 UTC (permalink / raw)
To: ulf.hansson
Cc: adrian.hunter, robh+dt, mark.rutland, linux-mmc, devicetree,
linux-kernel, zach.brown
In-Reply-To: <1477425538-3315-1-git-send-email-zach.brown@ni.com>
On some systems the sdhci capabilty registers are incorrect for one
reason or another.
The sdhci-caps-mask property specifies which bits in the registers
are incorrect and should be turned off before using sdhci-caps to turn
on bits.
The sdhci-caps property specifies which bits should be turned on.
Signed-off-by: Zach Brown <zach.brown@ni.com>
---
Documentation/devicetree/bindings/mmc/mmc.txt | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
index 8a37782..1415aa0 100644
--- a/Documentation/devicetree/bindings/mmc/mmc.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc.txt
@@ -52,6 +52,13 @@ Optional properties:
- no-sdio: controller is limited to send sdio cmd during initialization
- no-sd: controller is limited to send sd cmd during initialization
- no-mmc: controller is limited to send mmc cmd during initialization
+- sdhci-caps-mask: The sdhci capabilities registers are incorrect. This 64bit
+ property corresponds to the bits in the sdhci capabilty registers. If the bit
+ is on in the mask then the bit is incorrect in the registers and should be
+ turned off.
+- sdhci-caps: The sdhci capabilities registers are incorrect. This 64bit
+ property corresponds to the bits in the sdhci capability registers. If the
+ bit is on in the property then the bit should be on in the reigsters.
*NOTE* on CD and WP polarity. To use common for all SD/MMC host controllers line
polarity properties, we have to fix the meaning of the "normal" and "inverted"
--
2.7.4
^ permalink raw reply related
* [RFC 2/2] mmc: sdhci: Use sdhci-caps-mask and sdhci-caps to change the caps read during __sdhci_read_caps
From: Zach Brown @ 2016-10-25 19:58 UTC (permalink / raw)
To: ulf.hansson
Cc: adrian.hunter, robh+dt, mark.rutland, linux-mmc, devicetree,
linux-kernel, zach.brown
In-Reply-To: <1477425538-3315-1-git-send-email-zach.brown@ni.com>
The sdhci capabilities registers can be incorrect. The sdhci-caps-mask
and sdhci-caps dt properties specify which bits of the registers are
incorrect and what their values should be. This patch makes the sdhci
driver use those properties to correct the caps during
__sdhci_read_caps.
During __sdhci_read_caps
Use the sdhci-caps-mask property to turn off the incorrect bits of the
sdhci registers after reading them.
Use the sdhci-caps to turn on bits after using sdhci-caps-mask to turn
off the incorrect ones.
Signed-off-by: Zach Brown <zach.brown@ni.com>
---
drivers/mmc/host/sdhci.c | 24 ++++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 1e25b01..d5feae4 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -22,6 +22,7 @@
#include <linux/scatterlist.h>
#include <linux/regulator/consumer.h>
#include <linux/pm_runtime.h>
+#include <linux/of.h>
#include <linux/leds.h>
@@ -2991,6 +2992,8 @@ static int sdhci_set_dma_mask(struct sdhci_host *host)
void __sdhci_read_caps(struct sdhci_host *host, u16 *ver, u32 *caps, u32 *caps1)
{
u16 v;
+ u64 dt_caps_mask = 0;
+ u64 dt_caps = 0;
if (host->read_caps)
return;
@@ -3005,18 +3008,35 @@ void __sdhci_read_caps(struct sdhci_host *host, u16 *ver, u32 *caps, u32 *caps1)
sdhci_do_reset(host, SDHCI_RESET_ALL);
+ of_property_read_u64(mmc_dev(host->mmc)->of_node,
+ "sdhci-caps-mask", &dt_caps_mask);
+ of_property_read_u64(mmc_dev(host->mmc)->of_node,
+ "sdhci-caps", &dt_caps);
+
v = ver ? *ver : sdhci_readw(host, SDHCI_HOST_VERSION);
host->version = (v & SDHCI_SPEC_VER_MASK) >> SDHCI_SPEC_VER_SHIFT;
if (host->quirks & SDHCI_QUIRK_MISSING_CAPS)
return;
- host->caps = caps ? *caps : sdhci_readl(host, SDHCI_CAPABILITIES);
+ if (caps)
+ host->caps = *caps;
+ else {
+ host->caps = sdhci_readl(host, SDHCI_CAPABILITIES);
+ host->caps &= ~lower_32_bits(dt_caps_mask);
+ host->caps |= lower_32_bits(dt_caps);
+ }
if (host->version < SDHCI_SPEC_300)
return;
- host->caps1 = caps1 ? *caps1 : sdhci_readl(host, SDHCI_CAPABILITIES_1);
+ if (caps1)
+ host->caps1 = *caps1;
+ else {
+ host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
+ host->caps1 &= ~upper_32_bits(dt_caps_mask);
+ host->caps1 |= upper_32_bits(dt_caps);
+ }
}
EXPORT_SYMBOL_GPL(__sdhci_read_caps);
--
2.7.4
^ permalink raw reply related
* [PATCH] mmc: s3cmci: Use DMA slave map rather than exported DMA filter
From: Sylwester Nawrocki @ 2016-10-26 9:08 UTC (permalink / raw)
To: ulf.hansson, linux-samsung-soc
Cc: sam.van.den.berge, linux-mmc, Sylwester Nawrocki
Support for DMA slave map has been added to the s3c24xx-dma
controller in commit 34681d84a0f7cc22ded1413dc79eef8a2f23d9c3
"dmaengine: s3c24xx: Add dma_slave_map for s3c2440 devices"
This patch converts the s3cmci driver to also use it, so we can
eventually get rid of the exported filter function once all
DMA related clients are updated.
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
drivers/mmc/host/s3cmci.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/mmc/host/s3cmci.c b/drivers/mmc/host/s3cmci.c
index c531dee..c109e57 100644
--- a/drivers/mmc/host/s3cmci.c
+++ b/drivers/mmc/host/s3cmci.c
@@ -28,7 +28,6 @@
#include <mach/dma.h>
#include <mach/gpio-samsung.h>
-#include <linux/platform_data/dma-s3c24xx.h>
#include <linux/platform_data/mmc-s3cmci.h>
#include "s3cmci.h"
@@ -1685,13 +1684,7 @@ static int s3cmci_probe(struct platform_device *pdev)
/* depending on the dma state, get a dma channel to use. */
if (s3cmci_host_usedma(host)) {
- dma_cap_mask_t mask;
-
- dma_cap_zero(mask);
- dma_cap_set(DMA_SLAVE, mask);
-
- host->dma = dma_request_slave_channel_compat(mask,
- s3c24xx_dma_filter, (void *)DMACH_SDI, &pdev->dev, "rx-tx");
+ host->dma = dma_request_slave_channel(&pdev->dev, "rx-tx");
if (!host->dma) {
dev_err(&pdev->dev, "cannot get DMA channel.\n");
ret = -EBUSY;
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] mmc: s3cmci: Use DMA slave map rather than exported DMA filter
From: Arnd Bergmann @ 2016-10-26 9:21 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: ulf.hansson, linux-samsung-soc, sam.van.den.berge, linux-mmc
In-Reply-To: <1477472907-5932-1-git-send-email-s.nawrocki@samsung.com>
On Wednesday, October 26, 2016 11:08:27 AM CEST Sylwester Nawrocki wrote:
> @@ -1685,13 +1684,7 @@ static int s3cmci_probe(struct platform_device *pdev)
> /* depending on the dma state, get a dma channel to use. */
>
> if (s3cmci_host_usedma(host)) {
> - dma_cap_mask_t mask;
> -
> - dma_cap_zero(mask);
> - dma_cap_set(DMA_SLAVE, mask);
> -
> - host->dma = dma_request_slave_channel_compat(mask,
> - s3c24xx_dma_filter, (void *)DMACH_SDI, &pdev->dev, "rx-tx");
> + host->dma = dma_request_slave_channel(&pdev->dev, "rx-tx");
> if (!host->dma) {
> dev_err(&pdev->dev, "cannot get DMA channel.\n");
> ret = -EBUSY;
Can you convert it to use dma_request_chan()+PTR_ERR_OR_ZERO() instead?
Otherwise looks good, thanks for looking into this!
Arnd
^ permalink raw reply
* Re: [PATCH] mmc: s3cmci: Use DMA slave map rather than exported DMA filter
From: Sylwester Nawrocki @ 2016-10-26 12:49 UTC (permalink / raw)
To: Arnd Bergmann
Cc: ulf.hansson, linux-samsung-soc, sam.van.den.berge, linux-mmc
In-Reply-To: <4067826.3L3geWCA41@wuerfel>
On 10/26/2016 11:21 AM, Arnd Bergmann wrote:
> On Wednesday, October 26, 2016 11:08:27 AM CEST Sylwester Nawrocki wrote:
>> > @@ -1685,13 +1684,7 @@ static int s3cmci_probe(struct platform_device *pdev)
>> > /* depending on the dma state, get a dma channel to use. */
>> >
>> > if (s3cmci_host_usedma(host)) {
>> > - dma_cap_mask_t mask;
>> > -
>> > - dma_cap_zero(mask);
>> > - dma_cap_set(DMA_SLAVE, mask);
>> > -
>> > - host->dma = dma_request_slave_channel_compat(mask,
>> > - s3c24xx_dma_filter, (void *)DMACH_SDI, &pdev->dev, "rx-tx");
>> > + host->dma = dma_request_slave_channel(&pdev->dev, "rx-tx");
>> > if (!host->dma) {
>> > dev_err(&pdev->dev, "cannot get DMA channel.\n");
>> > ret = -EBUSY;
>
> Can you convert it to use dma_request_chan()+PTR_ERR_OR_ZERO() instead?
Thanks for your review. dma_request_chan() seems to be returning
either valid pointer or ERR_PTR(), so it would be sufficient
to use just PTR_ERR(host->dma) on error path?
^ permalink raw reply
* Re: [PATCH] mmc: s3cmci: Use DMA slave map rather than exported DMA filter
From: Arnd Bergmann @ 2016-10-26 13:31 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: ulf.hansson, linux-samsung-soc, sam.van.den.berge, linux-mmc
In-Reply-To: <11c41f92-5591-dd45-816f-6740da76b91f@samsung.com>
On Wednesday, October 26, 2016 2:49:44 PM CEST Sylwester Nawrocki wrote:
> On 10/26/2016 11:21 AM, Arnd Bergmann wrote:
> > On Wednesday, October 26, 2016 11:08:27 AM CEST Sylwester Nawrocki wrote:
> >> > @@ -1685,13 +1684,7 @@ static int s3cmci_probe(struct platform_device *pdev)
> >> > /* depending on the dma state, get a dma channel to use. */
> >> >
> >> > if (s3cmci_host_usedma(host)) {
> >> > - dma_cap_mask_t mask;
> >> > -
> >> > - dma_cap_zero(mask);
> >> > - dma_cap_set(DMA_SLAVE, mask);
> >> > -
> >> > - host->dma = dma_request_slave_channel_compat(mask,
> >> > - s3c24xx_dma_filter, (void *)DMACH_SDI, &pdev->dev, "rx-tx");
> >> > + host->dma = dma_request_slave_channel(&pdev->dev, "rx-tx");
> >> > if (!host->dma) {
> >> > dev_err(&pdev->dev, "cannot get DMA channel.\n");
> >> > ret = -EBUSY;
> >
> > Can you convert it to use dma_request_chan()+PTR_ERR_OR_ZERO() instead?
>
> Thanks for your review. dma_request_chan() seems to be returning
> either valid pointer or ERR_PTR(), so it would be sufficient
> to use just PTR_ERR(host->dma) on error path?
Correct. You can use either
if (IS_ERR(host->dma) {
ret = PTR_ERR(host->dma);
goto probe_free_gpio_wp;
}
or
ret = PTR_ERR_OR_ZERO(host->dma);
if (ret)
goto probe_free_gpio_wp;
The two do exactly the same, I just find the second one slightly more
intuitive, and I've seen a couple of spurious warnings caused by
the first (that won't happen here).
Arnd
^ permalink raw reply
* [PATCH -next] mmc: sdhci-msm: Fix error return code in sdhci_msm_probe()
From: Wei Yongjun @ 2016-10-26 15:04 UTC (permalink / raw)
To: Adrian Hunter, Ulf Hansson, Georgi Djakov; +Cc: Wei Yongjun, linux-mmc
From: Wei Yongjun <weiyongjun1@huawei.com>
Fix to return a negative error code from the platform_get_irq_byname()
error handling case instead of 0, as done elsewhere in this function.
Fixes: ad81d3871004 ("mmc: sdhci-msm: Add support for UHS cards")
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
drivers/mmc/host/sdhci-msm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 795f16f..b78d72f 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -649,6 +649,7 @@ static int sdhci_msm_probe(struct platform_device *pdev)
if (msm_host->pwr_irq < 0) {
dev_err(&pdev->dev, "Get pwr_irq failed (%d)\n",
msm_host->pwr_irq);
+ ret = msm_host->pwr_irq;
goto clk_disable;
}
^ permalink raw reply related
* [PATCH v2] mmc: s3cmci: Use DMA slave map rather than exported DMA filter
From: Sylwester Nawrocki @ 2016-10-26 16:30 UTC (permalink / raw)
To: ulf.hansson, linux-samsung-soc
Cc: sam.van.den.berge, arnd, linux-mmc, b.zolnierkie,
Sylwester Nawrocki
Support for DMA slave map has been added to the s3c24xx-dma
controller in commit 34681d84a0f7cc22ded1413dc79eef8a2f23d9c3
"dmaengine: s3c24xx: Add dma_slave_map for s3c2440 devices"
This patch converts the s3cmci driver to also use it, so we can
eventually get rid of the exported filter function once all
related DMA clients are updated.
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
Changes since v1:
- dma_request_chan() used instead of dma_request_slave_channel()
for better error handling.
drivers/mmc/host/s3cmci.c | 15 ++++-----------
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/drivers/mmc/host/s3cmci.c b/drivers/mmc/host/s3cmci.c
index c531dee..932a4b1 100644
--- a/drivers/mmc/host/s3cmci.c
+++ b/drivers/mmc/host/s3cmci.c
@@ -28,7 +28,6 @@
#include <mach/dma.h>
#include <mach/gpio-samsung.h>
-#include <linux/platform_data/dma-s3c24xx.h>
#include <linux/platform_data/mmc-s3cmci.h>
#include "s3cmci.h"
@@ -1682,19 +1681,13 @@ static int s3cmci_probe(struct platform_device *pdev)
gpio_direction_input(host->pdata->gpio_wprotect);
}
- /* depending on the dma state, get a dma channel to use. */
+ /* Depending on the dma state, get a DMA channel to use. */
if (s3cmci_host_usedma(host)) {
- dma_cap_mask_t mask;
-
- dma_cap_zero(mask);
- dma_cap_set(DMA_SLAVE, mask);
-
- host->dma = dma_request_slave_channel_compat(mask,
- s3c24xx_dma_filter, (void *)DMACH_SDI, &pdev->dev, "rx-tx");
- if (!host->dma) {
+ host->dma = dma_request_chan(&pdev->dev, "rx-tx");
+ ret = PTR_ERR_OR_ZERO(host->dma);
+ if (ret) {
dev_err(&pdev->dev, "cannot get DMA channel.\n");
- ret = -EBUSY;
goto probe_free_gpio_wp;
}
}
--
1.9.1
^ permalink raw reply related
* Re: [v12, 5/8] soc: fsl: add GUTS driver for QorIQ platforms
From: Scott Wood @ 2016-10-26 17:06 UTC (permalink / raw)
To: Yangbo Lu, linux-mmc, ulf.hansson, Arnd Bergmann
Cc: linuxppc-dev, devicetree, linux-arm-kernel, linux-kernel,
linux-clk, linux-i2c, iommu, netdev, Mark Rutland, Rob Herring,
Russell King, Jochen Friedrich, Joerg Roedel, Claudiu Manoil,
Bhupesh Sharma, Qiang Zhao, Kumar Gala, Santosh Shilimkar, Leo Li,
Xiaobo Xie, Minghuan Lian
In-Reply-To: <1474441040-11946-6-git-send-email-yangbo.lu@nxp.com>
On Wed, 2016-09-21 at 14:57 +0800, Yangbo Lu wrote:
> diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig
> new file mode 100644
> index 0000000..b99764c
> --- /dev/null
> +++ b/drivers/soc/fsl/Kconfig
> @@ -0,0 +1,19 @@
> +#
> +# Freescale SOC drivers
> +#
> +
> +source "drivers/soc/fsl/qe/Kconfig"
> +
> +config FSL_GUTS
> + bool "Freescale QorIQ GUTS driver"
> + select SOC_BUS
> + help
> + The global utilities block controls power management, I/O device
> + enabling, power-onreset(POR) configuration monitoring, alternate
> + function selection for multiplexed signals,and clock control.
> + This driver is to manage and access global utilities block.
> + Initially only reading SVR and registering soc device are
> supported.
> + Other guts accesses, such as reading RCW, should eventually be
> moved
> + into this driver as well.
> +
> + If you want GUTS driver support, you should say Y here.
This is user-enablable without dependencies, which means it will break some
randconfigs. If this is to be enabled via select then remove the text after
"bool".
> +/* SoC die attribute definition for QorIQ platform */
> +static const struct fsl_soc_die_attr fsl_soc_die[] = {
> +#ifdef CONFIG_PPC
> + /*
> + * Power Architecture-based SoCs T Series
> + */
> +
> + /* Die: T4240, SoC: T4240/T4160/T4080 */
> + { .die = "T4240",
> + .svr = 0x82400000,
> + .mask = 0xfff00000,
> + },
> + /* Die: T1040, SoC: T1040/T1020/T1042/T1022 */
> + { .die = "T1040",
> + .svr = 0x85200000,
> + .mask = 0xfff00000,
> + },
> + /* Die: T2080, SoC: T2080/T2081 */
> + { .die = "T2080",
> + .svr = 0x85300000,
> + .mask = 0xfff00000,
> + },
> + /* Die: T1024, SoC: T1024/T1014/T1023/T1013 */
> + { .die = "T1024",
> + .svr = 0x85400000,
> + .mask = 0xfff00000,
> + },
> +#endif /* CONFIG_PPC */
> +#if defined(CONFIG_ARCH_MXC) || defined(CONFIG_ARCH_LAYERSCAPE)
Will this driver ever be probed on MXC? Why do we need these ifdefs at all?
> + /*
> + * ARM-based SoCs LS Series
> + */
> +
> + /* Die: LS1043A, SoC: LS1043A/LS1023A */
> + { .die = "LS1043A",
> + .svr = 0x87920000,
> + .mask = 0xffff0000,
> + },
> + /* Die: LS2080A, SoC: LS2080A/LS2040A/LS2085A */
> + { .die = "LS2080A",
> + .svr = 0x87010000,
> + .mask = 0xff3f0000,
> + },
> + /* Die: LS1088A, SoC: LS1088A/LS1048A/LS1084A/LS1044A */
> + { .die = "LS1088A",
> + .svr = 0x87030000,
> + .mask = 0xff3f0000,
> + },
> + /* Die: LS1012A, SoC: LS1012A */
> + { .die = "LS1012A",
> + .svr = 0x87040000,
> + .mask = 0xffff0000,
> + },
> + /* Die: LS1046A, SoC: LS1046A/LS1026A */
> + { .die = "LS1046A",
> + .svr = 0x87070000,
> + .mask = 0xffff0000,
> + },
> + /* Die: LS2088A, SoC: LS2088A/LS2048A/LS2084A/LS2044A */
> + { .die = "LS2088A",
> + .svr = 0x87090000,
> + .mask = 0xff3f0000,
> + },
> + /* Die: LS1021A, SoC: LS1021A/LS1020A/LS1022A
> + * Note: Put this die at the end in cause of incorrect
> identification
> + */
> + { .die = "LS1021A",
> + .svr = 0x87000000,
> + .mask = 0xfff00000,
> + },
> +#endif /* CONFIG_ARCH_MXC || CONFIG_ARCH_LAYERSCAPE */
Instead of relying on ordering, add more bits to the mask so that there's no
overlap. I think 0xfff70000 would work.
> +out:
> + kfree(soc_dev_attr.machine);
> + kfree(soc_dev_attr.family);
> + kfree(soc_dev_attr.soc_id);
> + kfree(soc_dev_attr.revision);
> + iounmap(guts->regs);
> +out_free:
> + kfree(guts);
> + return ret;
> +}
Please use devm.
-Scott
^ permalink raw reply
* Re: [PATCH v2] mmc: s3cmci: Use DMA slave map rather than exported DMA filter
From: Arnd Bergmann @ 2016-10-26 20:10 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: ulf.hansson, linux-samsung-soc, sam.van.den.berge, linux-mmc,
b.zolnierkie
In-Reply-To: <1477499453-22549-1-git-send-email-s.nawrocki@samsung.com>
On Wednesday, October 26, 2016 6:30:53 PM CEST Sylwester Nawrocki wrote:
> Support for DMA slave map has been added to the s3c24xx-dma
> controller in commit 34681d84a0f7cc22ded1413dc79eef8a2f23d9c3
> "dmaengine: s3c24xx: Add dma_slave_map for s3c2440 devices"
> This patch converts the s3cmci driver to also use it, so we can
> eventually get rid of the exported filter function once all
> related DMA clients are updated.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
^ 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