* Re: [PATCH 1/2] dt-bindings: mmc: add DT binding for S3C24XX MMC/SD/SDIO controller
From: Jaehoon Chung @ 2016-12-02 0:48 UTC (permalink / raw)
To: Sergio Prado, ulf.hansson, robh+dt, mark.rutland, linux-mmc,
devicetree, linux-kernel, ben-linux, linux-arm-kernel
In-Reply-To: <1480637665-6325-2-git-send-email-sergio.prado@e-labworks.com>
On 12/02/2016 09:14 AM, Sergio Prado wrote:
> Adds the device tree bindings description for Samsung S3C24XX
> MMC/SD/SDIO controller, used as a connectivity interface with external
> MMC, SD and SDIO storage mediums.
>
> Signed-off-by: Sergio Prado <sergio.prado@e-labworks.com>
> ---
> .../devicetree/bindings/mmc/samsung,s3cmci.txt | 38 ++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mmc/samsung,s3cmci.txt
>
> diff --git a/Documentation/devicetree/bindings/mmc/samsung,s3cmci.txt b/Documentation/devicetree/bindings/mmc/samsung,s3cmci.txt
> new file mode 100644
> index 000000000000..3f044076e69a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/samsung,s3cmci.txt
> @@ -0,0 +1,38 @@
> +* Samsung's S3C24XX MMC/SD/SDIO controller device tree bindings
> +
> +Samsung's S3C24XX MMC/SD/SDIO controller is used as a connectivity interface
> +with external MMC, SD and SDIO storage mediums.
> +
> +This file documents differences between the core mmc properties described by
> +mmc.txt and the properties used by the Samsung S3C24XX MMC/SD/SDIO controller
> +implementation.
> +
> +Required SoC Specific Properties:
> +- compatible: should be one of the following
> + - "samsung,s3c2410-sdi": for controllers compatible with s3c2410
> + - "samsung,s3c2412-sdi": for controllers compatible with s3c2412
> + - "samsung,s3c2440-sdi": for controllers compatible with s3c2440
> +- clocks: Should reference the controller clock
> +- clock-names: Should contain "sdi"
> +
> +Required Board Specific Properties:
> +- pinctrl-0: Should specify pin control groups used for this controller.
> +- pinctrl-names: Should contain only one value - "default".
I'm not sure but i think this description doesn't need at here.
> +
> +Example:
> + sdi: sdi@5a000000 {
I think it needs to use "mmc0: sdi@5a000000" instead of "sdi: sdi@5a000000"
Because mmc is more clear than sdi.
> + compatible = "samsung,s3c2440-sdi";
> + pinctrl-names = "default";
> + pinctrl-0 = <&sdi_pins>;
> + reg = <0x5a000000 0x100000>;
> + interrupts = <0 0 21 3>;
> + clocks = <&clocks PCLK_SDI>;
> + clock-names = "sdi";
> + bus-width = <4>;
> + cd-gpios = <&gpg 8 GPIO_ACTIVE_LOW>;
> + wp-gpios = <&gph 8 GPIO_ACTIVE_LOW>;
> + };
> +
> + Note: This example shows both SoC specific and board specific properties
> + in a single device node. The properties can be actually be separated
> + into SoC specific node and board specific node.
>
^ permalink raw reply
* [PATCH 2/2] mmc: host: s3cmci: allow probing from device tree
From: Sergio Prado @ 2016-12-02 0:14 UTC (permalink / raw)
To: ulf.hansson-QSEj5FYQhm4dnm+yROfE0A,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-mmc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
ben-linux-elnMNo+KYs3YtjvyW6yDsg,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Sergio Prado
In-Reply-To: <1480637665-6325-1-git-send-email-sergio.prado-1e4yhPs3/ABSwrhanM7KvQ@public.gmane.org>
Allows configuring Samsung S3C24XX MMC/SD/SDIO controller using a device
tree.
Signed-off-by: Sergio Prado <sergio.prado-1e4yhPs3/ABSwrhanM7KvQ@public.gmane.org>
---
drivers/mmc/host/s3cmci.c | 155 +++++++++++++++++++++++++++++++++++++++-------
1 file changed, 131 insertions(+), 24 deletions(-)
diff --git a/drivers/mmc/host/s3cmci.c b/drivers/mmc/host/s3cmci.c
index 932a4b1fed33..bfeb90e8ffee 100644
--- a/drivers/mmc/host/s3cmci.c
+++ b/drivers/mmc/host/s3cmci.c
@@ -23,6 +23,9 @@
#include <linux/gpio.h>
#include <linux/irq.h>
#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
#include <plat/gpio-cfg.h>
#include <mach/dma.h>
@@ -127,6 +130,22 @@ enum dbg_channels {
dbg_conf = (1 << 8),
};
+struct s3cmci_drv_data {
+ int is2440;
+};
+
+static const struct s3cmci_drv_data s3c2410_s3cmci_drv_data = {
+ .is2440 = 0,
+};
+
+static const struct s3cmci_drv_data s3c2412_s3cmci_drv_data = {
+ .is2440 = 1,
+};
+
+static const struct s3cmci_drv_data s3c2440_s3cmci_drv_data = {
+ .is2440 = 1,
+};
+
static const int dbgmap_err = dbg_fail;
static const int dbgmap_info = dbg_info | dbg_conf;
static const int dbgmap_debug = dbg_err | dbg_debug;
@@ -1241,8 +1260,9 @@ static void s3cmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
case MMC_POWER_ON:
case MMC_POWER_UP:
/* Configure GPE5...GPE10 pins in SD mode */
- s3c_gpio_cfgall_range(S3C2410_GPE(5), 6, S3C_GPIO_SFN(2),
- S3C_GPIO_PULL_NONE);
+ if (!host->pdev->dev.of_node)
+ s3c_gpio_cfgall_range(S3C2410_GPE(5), 6, S3C_GPIO_SFN(2),
+ S3C_GPIO_PULL_NONE);
if (host->pdata->set_power)
host->pdata->set_power(ios->power_mode, ios->vdd);
@@ -1254,7 +1274,8 @@ static void s3cmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
case MMC_POWER_OFF:
default:
- gpio_direction_output(S3C2410_GPE(5), 0);
+ if (!host->pdev->dev.of_node)
+ gpio_direction_output(S3C2410_GPE(5), 0);
if (host->is2440)
mci_con |= S3C2440_SDICON_SDRESET;
@@ -1544,21 +1565,12 @@ static inline void s3cmci_debugfs_remove(struct s3cmci_host *host) { }
#endif /* CONFIG_DEBUG_FS */
-static int s3cmci_probe(struct platform_device *pdev)
+static int s3cmci_probe_pdata(struct s3cmci_host *host)
{
- struct s3cmci_host *host;
- struct mmc_host *mmc;
- int ret;
- int is2440;
- int i;
+ struct platform_device *pdev = host->pdev;
+ int i, ret;
- is2440 = platform_get_device_id(pdev)->driver_data;
-
- mmc = mmc_alloc_host(sizeof(struct s3cmci_host), &pdev->dev);
- if (!mmc) {
- ret = -ENOMEM;
- goto probe_out;
- }
+ host->is2440 = platform_get_device_id(pdev)->driver_data;
for (i = S3C2410_GPE(5); i <= S3C2410_GPE(10); i++) {
ret = gpio_request(i, dev_name(&pdev->dev));
@@ -1568,14 +1580,90 @@ static int s3cmci_probe(struct platform_device *pdev)
for (i--; i >= S3C2410_GPE(5); i--)
gpio_free(i);
- goto probe_free_host;
+ return ret;
}
}
+ return 0;
+}
+
+static int s3cmci_probe_dt(struct s3cmci_host *host)
+{
+ struct platform_device *pdev = host->pdev;
+ struct s3c24xx_mci_pdata *pdata;
+ const struct s3cmci_drv_data *drvdata;
+ struct mmc_host *mmc = host->mmc;
+ int gpio, ret;
+
+ drvdata = of_device_get_match_data(&pdev->dev);
+ if (!drvdata)
+ return -ENODEV;
+
+ host->is2440 = drvdata->is2440;
+
+ ret = mmc_of_parse(mmc);
+ if (ret)
+ return ret;
+
+ pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return -ENOMEM;
+
+ pdata->ocr_avail = mmc->ocr_avail;
+
+ if (mmc->caps2 & MMC_CAP2_NO_WRITE_PROTECT)
+ pdata->no_wprotect = 1;
+
+ if (mmc->caps & MMC_CAP_NEEDS_POLL)
+ pdata->no_detect = 1;
+
+ if (mmc->caps2 & MMC_CAP2_RO_ACTIVE_HIGH)
+ pdata->wprotect_invert = 1;
+
+ if (mmc->caps2 & MMC_CAP2_CD_ACTIVE_HIGH)
+ pdata->detect_invert = 1;
+
+ gpio = of_get_named_gpio(pdev->dev.of_node, "cd-gpios", 0);
+ if (gpio_is_valid(gpio)) {
+ pdata->gpio_detect = gpio;
+ gpio_free(gpio);
+ }
+
+ gpio = of_get_named_gpio(pdev->dev.of_node, "wp-gpios", 0);
+ if (gpio_is_valid(gpio)) {
+ pdata->gpio_wprotect = gpio;
+ gpio_free(gpio);
+ }
+
+ pdev->dev.platform_data = pdata;
+
+ return 0;
+}
+
+static int s3cmci_probe(struct platform_device *pdev)
+{
+ struct s3cmci_host *host;
+ struct mmc_host *mmc;
+ int ret;
+ int i;
+
+ mmc = mmc_alloc_host(sizeof(struct s3cmci_host), &pdev->dev);
+ if (!mmc) {
+ ret = -ENOMEM;
+ goto probe_out;
+ }
+
host = mmc_priv(mmc);
host->mmc = mmc;
host->pdev = pdev;
- host->is2440 = is2440;
+
+ if (pdev->dev.of_node)
+ ret = s3cmci_probe_dt(host);
+ else
+ ret = s3cmci_probe_pdata(host);
+
+ if (ret)
+ goto probe_free_host;
host->pdata = pdev->dev.platform_data;
if (!host->pdata) {
@@ -1586,7 +1674,7 @@ static int s3cmci_probe(struct platform_device *pdev)
spin_lock_init(&host->complete_lock);
tasklet_init(&host->pio_tasklet, pio_tasklet, (unsigned long) host);
- if (is2440) {
+ if (host->is2440) {
host->sdiimsk = S3C2440_SDIIMSK;
host->sdidata = S3C2440_SDIDATA;
host->clk_div = 1;
@@ -1789,8 +1877,9 @@ static int s3cmci_probe(struct platform_device *pdev)
release_mem_region(host->mem->start, resource_size(host->mem));
probe_free_gpio:
- for (i = S3C2410_GPE(5); i <= S3C2410_GPE(10); i++)
- gpio_free(i);
+ if (!pdev->dev.of_node)
+ for (i = S3C2410_GPE(5); i <= S3C2410_GPE(10); i++)
+ gpio_free(i);
probe_free_host:
mmc_free_host(mmc);
@@ -1837,9 +1926,9 @@ static int s3cmci_remove(struct platform_device *pdev)
if (!pd->no_detect)
gpio_free(pd->gpio_detect);
- for (i = S3C2410_GPE(5); i <= S3C2410_GPE(10); i++)
- gpio_free(i);
-
+ if (!pdev->dev.of_node)
+ for (i = S3C2410_GPE(5); i <= S3C2410_GPE(10); i++)
+ gpio_free(i);
iounmap(host->base);
release_mem_region(host->mem->start, resource_size(host->mem));
@@ -1848,6 +1937,23 @@ static int s3cmci_remove(struct platform_device *pdev)
return 0;
}
+static const struct of_device_id s3cmci_dt_match[] = {
+ {
+ .compatible = "samsung,s3c2410-sdi",
+ .data = &s3c2410_s3cmci_drv_data,
+ },
+ {
+ .compatible = "samsung,s3c2412-sdi",
+ .data = &s3c2412_s3cmci_drv_data,
+ },
+ {
+ .compatible = "samsung,s3c2440-sdi",
+ .data = &s3c2440_s3cmci_drv_data,
+ },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, sdhci_s3c_dt_match);
+
static const struct platform_device_id s3cmci_driver_ids[] = {
{
.name = "s3c2410-sdi",
@@ -1867,6 +1973,7 @@ static int s3cmci_remove(struct platform_device *pdev)
static struct platform_driver s3cmci_driver = {
.driver = {
.name = "s3c-sdi",
+ .of_match_table = s3cmci_dt_match,
},
.id_table = s3cmci_driver_ids,
.probe = s3cmci_probe,
--
1.9.1
--
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 related
* [PATCH 1/2] dt-bindings: mmc: add DT binding for S3C24XX MMC/SD/SDIO controller
From: Sergio Prado @ 2016-12-02 0:14 UTC (permalink / raw)
To: ulf.hansson, robh+dt, mark.rutland, linux-mmc, devicetree,
linux-kernel, ben-linux, linux-arm-kernel
Cc: Sergio Prado
In-Reply-To: <1480637665-6325-1-git-send-email-sergio.prado@e-labworks.com>
Adds the device tree bindings description for Samsung S3C24XX
MMC/SD/SDIO controller, used as a connectivity interface with external
MMC, SD and SDIO storage mediums.
Signed-off-by: Sergio Prado <sergio.prado@e-labworks.com>
---
.../devicetree/bindings/mmc/samsung,s3cmci.txt | 38 ++++++++++++++++++++++
1 file changed, 38 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mmc/samsung,s3cmci.txt
diff --git a/Documentation/devicetree/bindings/mmc/samsung,s3cmci.txt b/Documentation/devicetree/bindings/mmc/samsung,s3cmci.txt
new file mode 100644
index 000000000000..3f044076e69a
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/samsung,s3cmci.txt
@@ -0,0 +1,38 @@
+* Samsung's S3C24XX MMC/SD/SDIO controller device tree bindings
+
+Samsung's S3C24XX MMC/SD/SDIO controller is used as a connectivity interface
+with external MMC, SD and SDIO storage mediums.
+
+This file documents differences between the core mmc properties described by
+mmc.txt and the properties used by the Samsung S3C24XX MMC/SD/SDIO controller
+implementation.
+
+Required SoC Specific Properties:
+- compatible: should be one of the following
+ - "samsung,s3c2410-sdi": for controllers compatible with s3c2410
+ - "samsung,s3c2412-sdi": for controllers compatible with s3c2412
+ - "samsung,s3c2440-sdi": for controllers compatible with s3c2440
+- clocks: Should reference the controller clock
+- clock-names: Should contain "sdi"
+
+Required Board Specific Properties:
+- pinctrl-0: Should specify pin control groups used for this controller.
+- pinctrl-names: Should contain only one value - "default".
+
+Example:
+ sdi: sdi@5a000000 {
+ compatible = "samsung,s3c2440-sdi";
+ pinctrl-names = "default";
+ pinctrl-0 = <&sdi_pins>;
+ reg = <0x5a000000 0x100000>;
+ interrupts = <0 0 21 3>;
+ clocks = <&clocks PCLK_SDI>;
+ clock-names = "sdi";
+ bus-width = <4>;
+ cd-gpios = <&gpg 8 GPIO_ACTIVE_LOW>;
+ wp-gpios = <&gph 8 GPIO_ACTIVE_LOW>;
+ };
+
+ Note: This example shows both SoC specific and board specific properties
+ in a single device node. The properties can be actually be separated
+ into SoC specific node and board specific node.
--
1.9.1
^ permalink raw reply related
* [PATCH 0/2] mmc: host: s3cmci: add device tree support
From: Sergio Prado @ 2016-12-02 0:14 UTC (permalink / raw)
To: ulf.hansson, robh+dt, mark.rutland, linux-mmc, devicetree,
linux-kernel, ben-linux, linux-arm-kernel
Cc: Sergio Prado
This series adds support for configuring Samsung's S3C24XX MMC/SD/SDIO
controller via device tree.
Tested on FriendlyARM mini2440, based on s3c2440 SoC.
Sergio Prado (2):
dt-bindings: mmc: add DT binding for S3C24XX MMC/SD/SDIO controller
mmc: host: s3cmci: allow probing from device tree
.../devicetree/bindings/mmc/samsung,s3cmci.txt | 38 +++++
drivers/mmc/host/s3cmci.c | 155 +++++++++++++++++----
2 files changed, 169 insertions(+), 24 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mmc/samsung,s3cmci.txt
--
1.9.1
^ permalink raw reply
* [PATCH] mmc: sdhci-acpi: support 80860F14 UID 2 SDIO bus
From: Daniel Drake @ 2016-12-01 20:33 UTC (permalink / raw)
To: adrian.hunter, ulf.hansson; +Cc: linux-mmc, linux
Add an entry for the SDIO bus in the ECS EF20 cherry trail laptop:
Device (SDHB) {
Name (_ADR, 0x00110000)
Name (_HID, "80860F14" /* Intel Baytrail SDIO/MMC Host Controller */)
Name (_CID, "PNP0D40" /* SDA Standard Compliant SD Host Controller */)
Name (_DDN, "Intel(R) SDIO Controller - 80862295")
Name (_UID, 0x02)
Name (_HRV, One)
A SDHB device with the same _HID and _UID can also be found on other
cherry trail products like Chuwi Hi10.
Signed-off-by: Daniel Drake <drake@endlessm.com>
---
drivers/mmc/host/sdhci-acpi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
index 81d4dc0..160f695 100644
--- a/drivers/mmc/host/sdhci-acpi.c
+++ b/drivers/mmc/host/sdhci-acpi.c
@@ -328,6 +328,7 @@ static const struct sdhci_acpi_uid_slot sdhci_acpi_uids[] = {
{ "80865ACC", NULL, &sdhci_acpi_slot_int_emmc },
{ "80865AD0", NULL, &sdhci_acpi_slot_int_sdio },
{ "80860F14" , "1" , &sdhci_acpi_slot_int_emmc },
+ { "80860F14" , "2" , &sdhci_acpi_slot_int_sdio },
{ "80860F14" , "3" , &sdhci_acpi_slot_int_sd },
{ "80860F16" , NULL, &sdhci_acpi_slot_int_sd },
{ "INT33BB" , "2" , &sdhci_acpi_slot_int_sdio },
--
2.9.3
^ permalink raw reply related
* Re: [PATCH v2] mmc: sdhci-msm: Add sdhci_reset() implementation
From: Georgi Djakov @ 2016-12-01 18:08 UTC (permalink / raw)
To: Ritesh Harjani
Cc: adrian.hunter, ulf.hansson, linux-mmc, linux-kernel,
linux-arm-msm
In-Reply-To: <27e95224-ff32-954d-443a-2310c1225ca0@codeaurora.org>
On 11/29/2016 05:40 AM, Ritesh Harjani wrote:
> Hi Georgi,
>
> On 11/28/2016 11:09 PM, Georgi Djakov wrote:
>> On apq8016, apq8084 and apq8074 platforms, writing to the software
>> reset register triggers the "power irq". We need to ack and handle
>> the irq, otherwise the following message appears:
>>
>> mmc0: Reset 0x1 never completed.
>> sdhci: =========== REGISTER DUMP (mmc0)===========
>> sdhci: Sys addr: 0x00000000 | Version: 0x00002e02
>> sdhci: Blk size: 0x00004000 | Blk cnt: 0x00000000
>> sdhci: Argument: 0x00000000 | Trn mode: 0x00000000
>> sdhci: Present: 0x01f80000 | Host ctl: 0x00000000
>> sdhci: Power: 0x00000000 | Blk gap: 0x00000000
>> sdhci: Wake-up: 0x00000000 | Clock: 0x00000003
>> sdhci: Timeout: 0x00000000 | Int stat: 0x00000000
>> sdhci: Int enab: 0x00000000 | Sig enab: 0x00000000
>> sdhci: AC12 err: 0x00000000 | Slot int: 0x00000000
>> sdhci: Caps: 0x322dc8b2 | Caps_1: 0x00008007
>> sdhci: Cmd: 0x00000000 | Max curr: 0x00000000
>> sdhci: Host ctl2: 0x00000000
>> sdhci: ADMA Err: 0x00000000 | ADMA Ptr: 0x0000000000000000
>> sdhci: ===========================================
>>
>> Fix it by implementing the custom sdhci_reset() function,
>> which performs the software reset and then handles the irq.
>>
>> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
>> ---
>>
>> Changes since v1: (https://lkml.org/lkml/2016/11/22/411)
>> * Perform the software reset by just writing to the SDHCI_SOFTWARE_RESET
>> register and then check for the irq.
> I am still not sure about this change. Below change will trigger the
> IRQ, once this call is completed (say on a single core system) -since
> this is called while holding spin_lock_irqsave.
>
> But you will be end up calling sdhci_msm_voltage_switch twice,
> which to me does not seems correct.
> 1. 1st time you are directly calling it in sdhci_msm_reset.
> 2. 2nd time will be called from within pwr_irq to handle the same case.
Yes, that would be the behavior. This function actually is not doing
anything much than to check irq status and ack it. Testing the patch
on a few different platforms does not show any side effects, however i
could not be 100% sure as i have limited information about the hardware.
Thanks,
Georgi
>
> Please let me know your thoughts on this ?
>
> Regards
> Ritesh
>
>>
>> drivers/mmc/host/sdhci-msm.c | 29 ++++++++++++++++++++++++++++-
>> 1 file changed, 28 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index 32879b845b75..157ae07f9309 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -1019,6 +1019,33 @@ static void sdhci_msm_set_clock(struct
>> sdhci_host *host, unsigned int clock)
>> __sdhci_msm_set_clock(host, clock);
>> }
>>
>> +void sdhci_msm_reset(struct sdhci_host *host, u8 mask)
>> +{
>> + unsigned long timeout = 100;
>> +
>> + sdhci_writeb(host, mask, SDHCI_SOFTWARE_RESET);
>> +
>> + if (mask & SDHCI_RESET_ALL) {
>> + host->clock = 0;
>> +
>> + /*
>> + * SDHCI_RESET_ALL triggers the PWR IRQ and we need
>> + * to handle it here.
>> + */
>> + sdhci_msm_voltage_switch(host);
>> + }
>> +
>> + while (sdhci_readb(host, SDHCI_SOFTWARE_RESET) & mask) {
>> + if (timeout == 0) {
>> + pr_err("%s: Reset 0x%x never completed.\n",
>> + mmc_hostname(host->mmc), (int)mask);
>> + return;
>> + }
>> + timeout--;
>> + mdelay(1);
>> + }
>> +}
>> +
>> static const struct of_device_id sdhci_msm_dt_match[] = {
>> { .compatible = "qcom,sdhci-msm-v4" },
>> {},
>> @@ -1028,7 +1055,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,
>> + .reset = sdhci_msm_reset,
>> .set_clock = sdhci_msm_set_clock,
>> .get_min_clock = sdhci_msm_get_min_clock,
>> .get_max_clock = sdhci_msm_get_max_clock,
>> --
>> 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
* [RFC] mmc: tmio: use SDIO master interrupt bit only when allowed
From: Wolfram Sang @ 2016-12-01 15:08 UTC (permalink / raw)
To: linux-mmc; +Cc: linux-renesas-soc, Yasushi SHOJI, Wolfram Sang
The master bit to enable SDIO interrupts can only be accessed if
SCLKDIVEN bit allows that. However, the core uses the SDIO enable
callback at times when SCLKDIVEN forbids the change. This leads to
"timeout waiting for SD bus idle" messages.
We now activate the master bit in probe once if SDIO is supported. IRQ
en-/disabling will be done now by the individual IRQ enablement bits
only.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
Compile tested only, due to no testcase.
drivers/mmc/host/tmio_mmc_pio.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 700567603107a0..476c4e1094a014 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -140,12 +140,10 @@ static void tmio_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
host->sdio_irq_mask = TMIO_SDIO_MASK_ALL &
~TMIO_SDIO_STAT_IOIRQ;
- sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0001);
sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask);
} else if (!enable && host->sdio_irq_enabled) {
host->sdio_irq_mask = TMIO_SDIO_MASK_ALL;
sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask);
- sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0000);
host->sdio_irq_enabled = false;
pm_runtime_mark_last_busy(mmc_dev(mmc));
@@ -1137,7 +1135,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
if (pdata->flags & TMIO_MMC_SDIO_IRQ) {
_host->sdio_irq_mask = TMIO_SDIO_MASK_ALL;
sd_ctrl_write16(_host, CTL_SDIO_IRQ_MASK, _host->sdio_irq_mask);
- sd_ctrl_write16(_host, CTL_TRANSACTION_CTL, 0x0000);
+ sd_ctrl_write16(_host, CTL_TRANSACTION_CTL, 0x0001);
}
spin_lock_init(&_host->lock);
@@ -1185,6 +1183,9 @@ void tmio_mmc_host_remove(struct tmio_mmc_host *host)
struct platform_device *pdev = host->pdev;
struct mmc_host *mmc = host->mmc;
+ if (host->pdata->flags & TMIO_MMC_SDIO_IRQ)
+ sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0000);
+
if (!host->native_hotplug)
pm_runtime_get_sync(&pdev->dev);
--
2.10.2
^ permalink raw reply related
* Re: [PATCH RFC 0/5] mmc: sdhci: Tidy tuning
From: Ulf Hansson @ 2016-12-01 14:57 UTC (permalink / raw)
To: Adrian Hunter; +Cc: linux-mmc
In-Reply-To: <1480601071-8881-1-git-send-email-adrian.hunter@intel.com>
On 1 December 2016 at 15:04, Adrian Hunter <adrian.hunter@intel.com> wrote:
> Hi
>
> Here are some tidy-ups wrt SDHCI tuning. Un-tested so far.
Nice. Thanks!
>
> Any other suggestions? Otherwise I will test and submit the whole series.
As an additional step, could you try to make use of mmc_send_tuning(),
instead of sending the command internally from sdhci_send_tuning()?
Kind regards
Uffe
^ permalink raw reply
* Re: mmc: tmio: why enable/disable SDIO irq on every transaction with IOMOD?
From: Wolfram Sang @ 2016-12-01 14:49 UTC (permalink / raw)
To: Yasushi SHOJI
Cc: ulf.hansson, linux-mmc, wsa+renesas, horms, linux-renesas-soc
In-Reply-To: <87y3zzvmth.wl@dns1.atmark-techno.com>
[-- Attachment #1: Type: text/plain, Size: 405 bytes --]
Hi,
> Is Arnd Hannemann, the author, around these days by any chance?
According to git history, he is sending a patch occasionally, but all
his SH involvement was from 2010. I don't know him.
> We don't have it yet. A question we have is that where should we
> enable / disable the IOMOD?
I think unconditionally in probe. I will send an RFC with some
explanations in a minute.
Thanks,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH RFC 5/5] mmc: sdhci: Tidy tuning loop
From: Adrian Hunter @ 2016-12-01 14:04 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc
In-Reply-To: <1480601071-8881-1-git-send-email-adrian.hunter@intel.com>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
drivers/mmc/host/sdhci.c | 81 +++++++++++++++++++++++++-----------------------
1 file changed, 43 insertions(+), 38 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index e9ff7d005851..1ac08e1a6211 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2055,11 +2055,47 @@ static void sdhci_send_tuning(struct sdhci_host *host, u32 opcode,
spin_lock_irqsave(&host->lock, flags);
}
+static void __sdhci_execute_tuning(struct sdhci_host *host, u32 opcode,
+ unsigned long flags)
+{
+ int i;
+
+ /*
+ * Issue opcode repeatedly till Execute Tuning is set to 0 or the number
+ * of loops reaches 40 times.
+ */
+ for (i = 0; i < MAX_TUNING_LOOP; i++) {
+ u16 ctrl;
+
+ sdhci_send_tuning(host, opcode, flags);
+
+ if (!host->tuning_done) {
+ pr_info("%s: Tuning timeout, falling back to fixed sampling clock\n",
+ mmc_hostname(host->mmc));
+ sdhci_abort_tuning(host, opcode, flags);
+ return;
+ }
+
+ ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+ if (!(ctrl & SDHCI_CTRL_EXEC_TUNING)) {
+ if (ctrl & SDHCI_CTRL_TUNED_CLK)
+ return; /* Success! */
+ break;
+ }
+
+ /* eMMC spec does not require a delay between tuning cycles */
+ if (opcode == MMC_SEND_TUNING_BLOCK)
+ mdelay(1);
+ }
+
+ pr_info("%s: Tuning failed, falling back to fixed sampling clock\n",
+ mmc_hostname(host->mmc));
+ sdhci_reset_tuning(host);
+}
+
static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
{
struct sdhci_host *host = mmc_priv(mmc);
- u16 ctrl;
- int tuning_loop_counter = MAX_TUNING_LOOP;
int err = 0;
unsigned long flags;
unsigned int tuning_count = 0;
@@ -2110,50 +2146,19 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
if (host->ops->platform_execute_tuning) {
spin_unlock_irqrestore(&host->lock, flags);
- err = host->ops->platform_execute_tuning(host, opcode);
- return err;
+ return host->ops->platform_execute_tuning(host, opcode);
}
- sdhci_start_tuning(host);
-
- /*
- * Issue CMD19 repeatedly till Execute Tuning is set to 0 or the number
- * of loops reaches 40 times.
- */
- do {
- if (tuning_loop_counter-- == 0)
- break;
-
- sdhci_send_tuning(host, opcode, flags);
-
- if (!host->tuning_done) {
- pr_info(DRIVER_NAME ": Timeout waiting for Buffer Read Ready interrupt during tuning procedure, falling back to fixed sampling clock\n");
- sdhci_abort_tuning(host, opcode, flags);
- goto out;
- }
-
- ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
-
- /* eMMC spec does not require a delay between tuning cycles */
- if (opcode == MMC_SEND_TUNING_BLOCK)
- mdelay(1);
- } while (ctrl & SDHCI_CTRL_EXEC_TUNING);
+ host->mmc->retune_period = tuning_count;
- /*
- * The Host Driver has exhausted the maximum number of loops allowed,
- * so use fixed sampling frequency.
- */
- if (tuning_loop_counter < 0)
- sdhci_reset_tuning(host);
+ sdhci_start_tuning(host);
- if (tuning_loop_counter < 0 || !(ctrl & SDHCI_CTRL_TUNED_CLK))
- pr_info(DRIVER_NAME ": Tuning procedure failed, falling back to fixed sampling clock\n");
-out:
- host->mmc->retune_period = tuning_count;
+ __sdhci_execute_tuning(host, opcode, flags);
sdhci_end_tuning(host);
out_unlock:
spin_unlock_irqrestore(&host->lock, flags);
+
return err;
}
--
1.9.1
^ permalink raw reply related
* [PATCH RFC 4/5] mmc: sdhci: Simplify tuning block size logic
From: Adrian Hunter @ 2016-12-01 14:04 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc
In-Reply-To: <1480601071-8881-1-git-send-email-adrian.hunter@intel.com>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
drivers/mmc/host/sdhci.c | 16 +++++-----------
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index fa89870a690b..e9ff7d005851 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2024,17 +2024,11 @@ static void sdhci_send_tuning(struct sdhci_host *host, u32 opcode,
* block to the Host Controller. So we set the block size
* to 64 here.
*/
- if (cmd.opcode == MMC_SEND_TUNING_BLOCK_HS200) {
- if (mmc->ios.bus_width == MMC_BUS_WIDTH_8)
- sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 128),
- SDHCI_BLOCK_SIZE);
- else if (mmc->ios.bus_width == MMC_BUS_WIDTH_4)
- sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 64),
- SDHCI_BLOCK_SIZE);
- } else {
- sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 64),
- SDHCI_BLOCK_SIZE);
- }
+ if (cmd.opcode == MMC_SEND_TUNING_BLOCK_HS200 &&
+ mmc->ios.bus_width == MMC_BUS_WIDTH_8)
+ sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 128), SDHCI_BLOCK_SIZE);
+ else
+ sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 64), SDHCI_BLOCK_SIZE);
/*
* The tuning block is sent by the card to the host controller.
--
1.9.1
^ permalink raw reply related
* [PATCH RFC 1/5] mmc: mmc: Introduce mmc_abort_tuning()
From: Adrian Hunter @ 2016-12-01 14:04 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc
In-Reply-To: <1480601071-8881-1-git-send-email-adrian.hunter@intel.com>
If a tuning command times out, the card could still be processing it, which
will cause problems for recovery. The eMMC specification says that CMD12
can be used to stop CMD21, so add a function that does that.
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
drivers/mmc/core/mmc_ops.c | 25 +++++++++++++++++++++++++
include/linux/mmc/core.h | 1 +
2 files changed, 26 insertions(+)
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 9b2617cfff67..c0cfaaeb7637 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -688,6 +688,31 @@ int mmc_send_tuning(struct mmc_host *host, u32 opcode, int *cmd_error)
}
EXPORT_SYMBOL_GPL(mmc_send_tuning);
+int mmc_abort_tuning(struct mmc_host *host, u32 opcode)
+{
+ struct mmc_command cmd = {0};
+
+ /*
+ * eMMC specification specifies that CMD12 can be used to stop a tuning
+ * command, but SD specification does not, so do nothing unless it is
+ * eMMC.
+ */
+ if (opcode != MMC_SEND_TUNING_BLOCK_HS200)
+ return 0;
+
+ cmd.opcode = MMC_STOP_TRANSMISSION;
+ cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_AC;
+
+ /*
+ * For drivers that override R1 to R1b, set an arbitrary timeout based
+ * on the tuning timeout i.e. 150ms.
+ */
+ cmd.busy_timeout = 150;
+
+ return mmc_wait_for_cmd(host, &cmd, 0);
+}
+EXPORT_SYMBOL_GPL(mmc_abort_tuning);
+
static int
mmc_send_bus_test(struct mmc_card *card, struct mmc_host *host, u8 opcode,
u8 len)
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index 0ce928b3ce90..e33cc748dcfe 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -176,6 +176,7 @@ extern int mmc_wait_for_app_cmd(struct mmc_host *, struct mmc_card *,
extern void mmc_start_bkops(struct mmc_card *card, bool from_exception);
extern int mmc_switch(struct mmc_card *, u8, u8, u8, unsigned int);
extern int mmc_send_tuning(struct mmc_host *host, u32 opcode, int *cmd_error);
+extern int mmc_abort_tuning(struct mmc_host *host, u32 opcode);
extern int mmc_get_ext_csd(struct mmc_card *card, u8 **new_ext_csd);
#define MMC_ERASE_ARG 0x00000000
--
1.9.1
^ permalink raw reply related
* [PATCH RFC 3/5] mmc: sdhci: Factor out tuning helper functions
From: Adrian Hunter @ 2016-12-01 14:04 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc
In-Reply-To: <1480601071-8881-1-git-send-email-adrian.hunter@intel.com>
Factor out some functions to tidy up the code in sdhci_execute_tuning.
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
drivers/mmc/host/sdhci.c | 209 ++++++++++++++++++++++++++---------------------
1 file changed, 117 insertions(+), 92 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index b841d0a57af1..fa89870a690b 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1952,6 +1952,115 @@ static int sdhci_prepare_hs400_tuning(struct mmc_host *mmc, struct mmc_ios *ios)
return 0;
}
+static void sdhci_start_tuning(struct sdhci_host *host)
+{
+ u16 ctrl;
+
+ ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+ ctrl |= SDHCI_CTRL_EXEC_TUNING;
+ if (host->quirks2 & SDHCI_QUIRK2_TUNING_WORK_AROUND)
+ ctrl |= SDHCI_CTRL_TUNED_CLK;
+ sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
+
+ /*
+ * As per the Host Controller spec v3.00, tuning command
+ * generates Buffer Read Ready interrupt, so enable that.
+ *
+ * Note: The spec clearly says that when tuning sequence
+ * is being performed, the controller does not generate
+ * interrupts other than Buffer Read Ready interrupt. But
+ * to make sure we don't hit a controller bug, we _only_
+ * enable Buffer Read Ready interrupt here.
+ */
+ sdhci_writel(host, SDHCI_INT_DATA_AVAIL, SDHCI_INT_ENABLE);
+ sdhci_writel(host, SDHCI_INT_DATA_AVAIL, SDHCI_SIGNAL_ENABLE);
+}
+
+static void sdhci_end_tuning(struct sdhci_host *host)
+{
+ sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
+ sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
+}
+
+static void sdhci_reset_tuning(struct sdhci_host *host)
+{
+ u16 ctrl;
+
+ ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+ ctrl &= ~SDHCI_CTRL_TUNED_CLK;
+ ctrl &= ~SDHCI_CTRL_EXEC_TUNING;
+ sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
+}
+
+static void sdhci_abort_tuning(struct sdhci_host *host, u32 opcode,
+ unsigned long flags)
+{
+ sdhci_reset_tuning(host);
+
+ sdhci_do_reset(host, SDHCI_RESET_CMD);
+ sdhci_do_reset(host, SDHCI_RESET_DATA);
+
+ sdhci_end_tuning(host);
+
+ spin_unlock_irqrestore(&host->lock, flags);
+ mmc_abort_tuning(host->mmc, opcode);
+ spin_lock_irqsave(&host->lock, flags);
+}
+
+static void sdhci_send_tuning(struct sdhci_host *host, u32 opcode,
+ unsigned long flags)
+{
+ struct mmc_host *mmc = host->mmc;
+ struct mmc_command cmd = {0};
+ struct mmc_request mrq = {NULL};
+
+ cmd.opcode = opcode;
+ cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
+ cmd.mrq = &mrq;
+
+ mrq.cmd = &cmd;
+ /*
+ * In response to CMD19, the card sends 64 bytes of tuning
+ * block to the Host Controller. So we set the block size
+ * to 64 here.
+ */
+ if (cmd.opcode == MMC_SEND_TUNING_BLOCK_HS200) {
+ if (mmc->ios.bus_width == MMC_BUS_WIDTH_8)
+ sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 128),
+ SDHCI_BLOCK_SIZE);
+ else if (mmc->ios.bus_width == MMC_BUS_WIDTH_4)
+ sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 64),
+ SDHCI_BLOCK_SIZE);
+ } else {
+ sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 64),
+ SDHCI_BLOCK_SIZE);
+ }
+
+ /*
+ * The tuning block is sent by the card to the host controller.
+ * So we set the TRNS_READ bit in the Transfer Mode register.
+ * This also takes care of setting DMA Enable and Multi Block
+ * Select in the same register to 0.
+ */
+ sdhci_writew(host, SDHCI_TRNS_READ, SDHCI_TRANSFER_MODE);
+
+ sdhci_send_command(host, &cmd);
+
+ host->cmd = NULL;
+
+ sdhci_del_timer(host, &mrq);
+
+ host->tuning_done = 0;
+
+ spin_unlock_irqrestore(&host->lock, flags);
+
+ /* Wait for Buffer Read Ready interrupt */
+ wait_event_timeout(host->buf_ready_int, (host->tuning_done == 1),
+ msecs_to_jiffies(50));
+
+ spin_lock_irqsave(&host->lock, flags);
+}
+
static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
{
struct sdhci_host *host = mmc_priv(mmc);
@@ -2011,105 +2120,24 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
return err;
}
- ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
- ctrl |= SDHCI_CTRL_EXEC_TUNING;
- if (host->quirks2 & SDHCI_QUIRK2_TUNING_WORK_AROUND)
- ctrl |= SDHCI_CTRL_TUNED_CLK;
- sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
-
- /*
- * As per the Host Controller spec v3.00, tuning command
- * generates Buffer Read Ready interrupt, so enable that.
- *
- * Note: The spec clearly says that when tuning sequence
- * is being performed, the controller does not generate
- * interrupts other than Buffer Read Ready interrupt. But
- * to make sure we don't hit a controller bug, we _only_
- * enable Buffer Read Ready interrupt here.
- */
- sdhci_writel(host, SDHCI_INT_DATA_AVAIL, SDHCI_INT_ENABLE);
- sdhci_writel(host, SDHCI_INT_DATA_AVAIL, SDHCI_SIGNAL_ENABLE);
+ sdhci_start_tuning(host);
/*
* Issue CMD19 repeatedly till Execute Tuning is set to 0 or the number
* of loops reaches 40 times.
*/
do {
- struct mmc_command cmd = {0};
- struct mmc_request mrq = {NULL};
-
- cmd.opcode = opcode;
- cmd.arg = 0;
- cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
- cmd.retries = 0;
- cmd.data = NULL;
- cmd.mrq = &mrq;
- cmd.error = 0;
-
if (tuning_loop_counter-- == 0)
break;
- mrq.cmd = &cmd;
-
- /*
- * In response to CMD19, the card sends 64 bytes of tuning
- * block to the Host Controller. So we set the block size
- * to 64 here.
- */
- if (cmd.opcode == MMC_SEND_TUNING_BLOCK_HS200) {
- if (mmc->ios.bus_width == MMC_BUS_WIDTH_8)
- sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 128),
- SDHCI_BLOCK_SIZE);
- else if (mmc->ios.bus_width == MMC_BUS_WIDTH_4)
- sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 64),
- SDHCI_BLOCK_SIZE);
- } else {
- sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 64),
- SDHCI_BLOCK_SIZE);
- }
-
- /*
- * The tuning block is sent by the card to the host controller.
- * So we set the TRNS_READ bit in the Transfer Mode register.
- * This also takes care of setting DMA Enable and Multi Block
- * Select in the same register to 0.
- */
- sdhci_writew(host, SDHCI_TRNS_READ, SDHCI_TRANSFER_MODE);
-
- sdhci_send_command(host, &cmd);
-
- host->cmd = NULL;
- sdhci_del_timer(host, &mrq);
-
- spin_unlock_irqrestore(&host->lock, flags);
- /* Wait for Buffer Read Ready interrupt */
- wait_event_timeout(host->buf_ready_int,
- (host->tuning_done == 1),
- msecs_to_jiffies(50));
- spin_lock_irqsave(&host->lock, flags);
+ sdhci_send_tuning(host, opcode, flags);
if (!host->tuning_done) {
pr_info(DRIVER_NAME ": Timeout waiting for Buffer Read Ready interrupt during tuning procedure, falling back to fixed sampling clock\n");
- ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
- ctrl &= ~SDHCI_CTRL_TUNED_CLK;
- ctrl &= ~SDHCI_CTRL_EXEC_TUNING;
- sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
-
- sdhci_do_reset(host, SDHCI_RESET_CMD);
- sdhci_do_reset(host, SDHCI_RESET_DATA);
-
- sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
- sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
-
- spin_unlock_irqrestore(&host->lock, flags);
- mmc_abort_tuning(mmc, opcode);
- spin_lock_irqsave(&host->lock, flags);
-
+ sdhci_abort_tuning(host, opcode, flags);
goto out;
}
- host->tuning_done = 0;
-
ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
/* eMMC spec does not require a delay between tuning cycles */
@@ -2121,18 +2149,15 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
* The Host Driver has exhausted the maximum number of loops allowed,
* so use fixed sampling frequency.
*/
- if (tuning_loop_counter < 0) {
- ctrl &= ~SDHCI_CTRL_TUNED_CLK;
- ctrl &= ~SDHCI_CTRL_EXEC_TUNING;
- sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
- }
- if (!(ctrl & SDHCI_CTRL_TUNED_CLK))
+ if (tuning_loop_counter < 0)
+ sdhci_reset_tuning(host);
+
+ if (tuning_loop_counter < 0 || !(ctrl & SDHCI_CTRL_TUNED_CLK))
pr_info(DRIVER_NAME ": Tuning procedure failed, falling back to fixed sampling clock\n");
out:
host->mmc->retune_period = tuning_count;
- sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
- sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
+ sdhci_end_tuning(host);
out_unlock:
spin_unlock_irqrestore(&host->lock, flags);
return err;
--
1.9.1
^ permalink raw reply related
* [PATCH RFC 2/5] mmc: sdhci: Use mmc_abort_tuning()
From: Adrian Hunter @ 2016-12-01 14:04 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc
In-Reply-To: <1480601071-8881-1-git-send-email-adrian.hunter@intel.com>
Use mmc_abort_tuning() instead of open-coding the stop command.
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
drivers/mmc/host/sdhci.c | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index a23887799f43..b841d0a57af1 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2098,20 +2098,11 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
sdhci_do_reset(host, SDHCI_RESET_CMD);
sdhci_do_reset(host, SDHCI_RESET_DATA);
- if (cmd.opcode != MMC_SEND_TUNING_BLOCK_HS200)
- goto out;
-
sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
spin_unlock_irqrestore(&host->lock, flags);
-
- memset(&cmd, 0, sizeof(cmd));
- cmd.opcode = MMC_STOP_TRANSMISSION;
- cmd.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC;
- cmd.busy_timeout = 50;
- mmc_wait_for_cmd(mmc, &cmd, 0);
-
+ mmc_abort_tuning(mmc, opcode);
spin_lock_irqsave(&host->lock, flags);
goto out;
--
1.9.1
^ permalink raw reply related
* [PATCH RFC 0/5] mmc: sdhci: Tidy tuning
From: Adrian Hunter @ 2016-12-01 14:04 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc
Hi
Here are some tidy-ups wrt SDHCI tuning. Un-tested so far.
Any other suggestions? Otherwise I will test and submit the whole series.
Regards
Adrian
^ permalink raw reply
* Re: [PATCH 1/1] mmc: mmc: Relax checking for switch errors after HS200 switch
From: Ulf Hansson @ 2016-12-01 14:06 UTC (permalink / raw)
To: Adrian Hunter
Cc: linux-mmc, Jaehoon Chung, Chaotian Jing, Stephen Boyd,
Michael Walle, Yong Mao, Shawn Lin
In-Reply-To: <dd764c3d-6303-2d26-4201-d491957dc980@intel.com>
On 1 December 2016 at 14:40, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 01/12/16 12:18, Ulf Hansson wrote:
>> On 24 November 2016 at 13:02, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>> The JEDEC specification indicates CMD13 can be used after a HS200 switch
>>> to check for errors. However in practice some boards experience CRC errors
>>> in the CMD13 response. Consequently, for HS200, CRC errors are not a
>>> reliable way to know the switch failed. If there really is a problem, we
>>> would expect tuning will fail and the result ends up the same. So change
>>> the error condition to ignore CRC errors in that case.
>>
>> I agree, this seems like a good idea.
>>
>> However...
>>
>>>
>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>>> ---
>>> drivers/mmc/core/mmc.c | 15 ++++++++++++++-
>>> 1 file changed, 14 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>> index 3268fcd3378d..34d30e2a09ff 100644
>>> --- a/drivers/mmc/core/mmc.c
>>> +++ b/drivers/mmc/core/mmc.c
>>> @@ -1223,7 +1223,12 @@ int mmc_hs400_to_hs200(struct mmc_card *card)
>>> mmc_set_timing(host, MMC_TIMING_MMC_HS200);
>>>
>>> err = mmc_switch_status(card);
>>> - if (err)
>>> + /*
>>> + * For HS200, CRC errors are not a reliable way to know the switch
>>> + * failed. If there really is a problem, we would expect tuning will
>>> + * fail and the result ends up the same.
>>> + */
>>> + if (err && err != -EILSEQ)
>>
>> I would rather change mmc_switch_status() to take a new parameter,
>> which tells it about ignoring CRC errors.
>>
>> The reason is simply that I think these changes should probably apply
>> to HS400 as well, and then it just gets lots of duplicated code for
>> the same error check.
>
> In the HS200 case, we have not yet done tuning and are not yet at the HS200 frequency - so there are reasons to ignore CRC errors. As well as the fact that there is hardware that is known to have that weakness.
>
> The HS400 case is a little different because we have already done tuning (in HS200 mode) by that time. After the final switch, both the card and host controller have reached their final operating point, and then CMD13 is sent - so CRC errors would be unexpected.
>
> The switches on the way from HS200 to HS400 are either done:
> - in HS200 mode after tuning is done and at HS200 frequency
> - in a mode that does not require tuning
> So again in those cases, CRC errors would be unexpected.
Agree.
>
> Which does not mean I am against relaxing those cases too, but the rational is different, and I have no strong opinion on it - other than the obvious: that returning a fatal error when the hardware can in fact work, is counterproductive.
Okay, so I suggest we make the changes for HS200 first, then we can
look into changes for HS400 as a step on top.
Anyway, I would still prefer to make mmc_switch_status() take another
parameter, unless you insist on the current proposal.
>
>>
>>> goto out_err;
>>>
>>> mmc_set_bus_speed(card);
>>> @@ -1387,6 +1392,14 @@ static int mmc_select_hs200(struct mmc_card *card)
>>>
>>> err = mmc_switch_status(card);
>>> /*
>>> + * For HS200, CRC errors are not a reliable way to know the
>>> + * switch failed. If there really is a problem, we would expect
>>> + * tuning will fail and the result ends up the same.
>>> + */
>>> + if (err == -EILSEQ)
>>> + err = 0;
>>> +
>>> + /*
>>> * mmc_select_timing() assumes timing has not changed if
>>> * it is a switch error.
>>> */
>>> --
>>> 1.9.1
>>>
>>
>> Moreover, and somewhat related to this change:
>>
>> Do we want to change mmc_switch_status() to be able to tell
>> mmc_send_status() about the number of command retries? Currently it
>> defaults to MMC_CMD_RETRIES, but I guess we only want to send *one*
>> CMD13?
>
> Yeah, although there is always the possibility that the retries overcome sporadic CRC errors, which would otherwise be fatal.
I was thinking that the card could have cleared the SWITCH STATUS
information when CRC errors gets detected. But I guess that depends on
*how* the CRC error is triggered, perhaps as you say, then it's better
to retry than not.
Okay, so let's keep using MMC_CMD_RETRIES for this case.
>
> I tend to think this patch should still be separate because it's rational is different.
> However we could consider something like this:
>
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index c0cfaaeb7637..d61a955b69fc 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -54,7 +54,7 @@
> 0xff, 0x77, 0x77, 0xff, 0x77, 0xbb, 0xdd, 0xee,
> };
>
> -int mmc_send_status(struct mmc_card *card, u32 *status)
> +int __mmc_send_status(struct mmc_card *card, u32 *status, int retries)
> {
> int err;
> struct mmc_command cmd = {0};
> @@ -67,7 +67,7 @@ int mmc_send_status(struct mmc_card *card, u32 *status)
> cmd.arg = card->rca << 16;
> cmd.flags = MMC_RSP_SPI_R2 | MMC_RSP_R1 | MMC_CMD_AC;
>
> - err = mmc_wait_for_cmd(card->host, &cmd, MMC_CMD_RETRIES);
> + err = mmc_wait_for_cmd(card->host, &cmd, retries);
> if (err)
> return err;
>
> @@ -80,6 +80,11 @@ int mmc_send_status(struct mmc_card *card, u32 *status)
> return 0;
> }
>
> +int mmc_send_status(struct mmc_card *card, u32 *status)
> +{
> + return __mmc_send_status(card, status, MMC_CMD_RETRIES);
> +}
> +
> static int _mmc_select_card(struct mmc_host *host, struct mmc_card *card)
> {
> struct mmc_command cmd = {0};
> @@ -453,7 +458,9 @@ int mmc_switch_status(struct mmc_card *card)
> u32 status;
> int err;
>
> - err = mmc_send_status(card, &status);
> + err = __mmc_send_status(card, &status, 0);
> + if (err == -EILSEQ)
> + return 0;
> if (err)
> return err;
>
>
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH v6 1/5] mmc: dt-bindings: add ZTE ZX296718 MMC bindings
From: Rob Herring @ 2016-12-01 13:45 UTC (permalink / raw)
To: Jun Nie
Cc: Shawn Guo, xie.baoyou, Mark Rutland, Ulf Hansson, Jaehoon Chung,
Jason Liu, chen.chaokai, lai.binz, linux-mmc,
devicetree@vger.kernel.org
In-Reply-To: <CABymUCPRuNnMmJ9W6YRD3rG8hdMay2QxNAUXkNK2YXBLR7Q9VA@mail.gmail.com>
On Wed, Nov 30, 2016 at 7:34 PM, Jun Nie <jun.nie@linaro.org> wrote:
> 2016-11-24 10:17 GMT+08:00 Jun Nie <jun.nie@linaro.org>:
>> 2016-11-18 14:29 GMT+08:00 Jun Nie <jun.nie@linaro.org>:
>>> Document the device-tree binding of ZTE MMC host on
>>> ZX296718 SoC.
>>>
>>> Signed-off-by: Jun Nie <jun.nie@linaro.org>
>>> ---
>>> .../devicetree/bindings/mmc/zx-dw-mshc.txt | 35 ++++++++++++++++++++++
>>> 1 file changed, 35 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>> new file mode 100644
>>> index 0000000..c175c4b
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>>> @@ -0,0 +1,35 @@
>>> +* ZTE specific extensions to the Synopsys Designware Mobile Storage
>>> + Host Controller
>>> +
>>> +The Synopsys designware mobile storage host controller is used to interface
>>> +a SoC with storage medium such as eMMC or SD/MMC cards. This file documents
>>> +differences between the core Synopsys dw mshc controller properties described
>>> +by synopsys-dw-mshc.txt and the properties used by the ZTE specific
>>> +extensions to the Synopsys Designware Mobile Storage Host Controller.
>>> +
>>> +Required Properties:
>>> +
>>> +* compatible: should be
>>> + - "zte,zx296718-dw-mshc": for ZX SoCs
>>> +
>>> +Example:
>>> +
>>> + mmc1: mmc@1110000 {
>>> + compatible = "zte,zx296718-dw-mshc";
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> + reg = <0x01110000 0x1000>;
>>> + interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
>>> + fifo-depth = <32>;
>>> + data-addr = <0x200>;
>>> + fifo-watermark-aligned;
>>> + bus-width = <4>;
>>> + clock-frequency = <50000000>;
>>> + clocks = <&topcrm SD0_AHB>, <&topcrm SD0_WCLK>;
>>> + clock-names = "biu", "ciu";
>>> + num-slots = <1>;
>>> + max-frequency = <50000000>;
>>> + cap-sdio-irq;
>>> + cap-sd-highspeed;
>>> + status = "disabled";
>>> + };
>>> --
>>> 1.9.1
>>>
>>
>> Hi Rob & Mark,
>>
>> Could you help review and act this patch if you think it is OK? Thank you!
>>
>> Jun
>
> Hi Rob & Mark,
>
> Could you help comment or act on this patch? Thank you!
Resend the patch to the DT list so patchwork will pick it up if you
want it reviewed.
Rob
^ permalink raw reply
* Re: [PATCH 1/1] mmc: mmc: Relax checking for switch errors after HS200 switch
From: Adrian Hunter @ 2016-12-01 13:40 UTC (permalink / raw)
To: Ulf Hansson
Cc: linux-mmc, Jaehoon Chung, Chaotian Jing, Stephen Boyd,
Michael Walle, Yong Mao, Shawn Lin
In-Reply-To: <CAPDyKFpxHVYc2wYc-yV3YLa=NKg1k1aktKhzCKfaZ2oaA_QLSQ@mail.gmail.com>
On 01/12/16 12:18, Ulf Hansson wrote:
> On 24 November 2016 at 13:02, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> The JEDEC specification indicates CMD13 can be used after a HS200 switch
>> to check for errors. However in practice some boards experience CRC errors
>> in the CMD13 response. Consequently, for HS200, CRC errors are not a
>> reliable way to know the switch failed. If there really is a problem, we
>> would expect tuning will fail and the result ends up the same. So change
>> the error condition to ignore CRC errors in that case.
>
> I agree, this seems like a good idea.
>
> However...
>
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>> drivers/mmc/core/mmc.c | 15 ++++++++++++++-
>> 1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 3268fcd3378d..34d30e2a09ff 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -1223,7 +1223,12 @@ int mmc_hs400_to_hs200(struct mmc_card *card)
>> mmc_set_timing(host, MMC_TIMING_MMC_HS200);
>>
>> err = mmc_switch_status(card);
>> - if (err)
>> + /*
>> + * For HS200, CRC errors are not a reliable way to know the switch
>> + * failed. If there really is a problem, we would expect tuning will
>> + * fail and the result ends up the same.
>> + */
>> + if (err && err != -EILSEQ)
>
> I would rather change mmc_switch_status() to take a new parameter,
> which tells it about ignoring CRC errors.
>
> The reason is simply that I think these changes should probably apply
> to HS400 as well, and then it just gets lots of duplicated code for
> the same error check.
In the HS200 case, we have not yet done tuning and are not yet at the HS200 frequency - so there are reasons to ignore CRC errors. As well as the fact that there is hardware that is known to have that weakness.
The HS400 case is a little different because we have already done tuning (in HS200 mode) by that time. After the final switch, both the card and host controller have reached their final operating point, and then CMD13 is sent - so CRC errors would be unexpected.
The switches on the way from HS200 to HS400 are either done:
- in HS200 mode after tuning is done and at HS200 frequency
- in a mode that does not require tuning
So again in those cases, CRC errors would be unexpected.
Which does not mean I am against relaxing those cases too, but the rational is different, and I have no strong opinion on it - other than the obvious: that returning a fatal error when the hardware can in fact work, is counterproductive.
>
>> goto out_err;
>>
>> mmc_set_bus_speed(card);
>> @@ -1387,6 +1392,14 @@ static int mmc_select_hs200(struct mmc_card *card)
>>
>> err = mmc_switch_status(card);
>> /*
>> + * For HS200, CRC errors are not a reliable way to know the
>> + * switch failed. If there really is a problem, we would expect
>> + * tuning will fail and the result ends up the same.
>> + */
>> + if (err == -EILSEQ)
>> + err = 0;
>> +
>> + /*
>> * mmc_select_timing() assumes timing has not changed if
>> * it is a switch error.
>> */
>> --
>> 1.9.1
>>
>
> Moreover, and somewhat related to this change:
>
> Do we want to change mmc_switch_status() to be able to tell
> mmc_send_status() about the number of command retries? Currently it
> defaults to MMC_CMD_RETRIES, but I guess we only want to send *one*
> CMD13?
Yeah, although there is always the possibility that the retries overcome sporadic CRC errors, which would otherwise be fatal.
I tend to think this patch should still be separate because it's rational is different.
However we could consider something like this:
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index c0cfaaeb7637..d61a955b69fc 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -54,7 +54,7 @@
0xff, 0x77, 0x77, 0xff, 0x77, 0xbb, 0xdd, 0xee,
};
-int mmc_send_status(struct mmc_card *card, u32 *status)
+int __mmc_send_status(struct mmc_card *card, u32 *status, int retries)
{
int err;
struct mmc_command cmd = {0};
@@ -67,7 +67,7 @@ int mmc_send_status(struct mmc_card *card, u32 *status)
cmd.arg = card->rca << 16;
cmd.flags = MMC_RSP_SPI_R2 | MMC_RSP_R1 | MMC_CMD_AC;
- err = mmc_wait_for_cmd(card->host, &cmd, MMC_CMD_RETRIES);
+ err = mmc_wait_for_cmd(card->host, &cmd, retries);
if (err)
return err;
@@ -80,6 +80,11 @@ int mmc_send_status(struct mmc_card *card, u32 *status)
return 0;
}
+int mmc_send_status(struct mmc_card *card, u32 *status)
+{
+ return __mmc_send_status(card, status, MMC_CMD_RETRIES);
+}
+
static int _mmc_select_card(struct mmc_host *host, struct mmc_card *card)
{
struct mmc_command cmd = {0};
@@ -453,7 +458,9 @@ int mmc_switch_status(struct mmc_card *card)
u32 status;
int err;
- err = mmc_send_status(card, &status);
+ err = __mmc_send_status(card, &status, 0);
+ if (err == -EILSEQ)
+ return 0;
if (err)
return err;
^ permalink raw reply related
* Re: mmc: tmio: why enable/disable SDIO irq on every transaction with IOMOD?
From: Yasushi SHOJI @ 2016-12-01 12:46 UTC (permalink / raw)
To: wsa-dev; +Cc: ulf.hansson, linux-mmc, wsa+renesas, horms, linux-renesas-soc
In-Reply-To: <20161201104622.GA1541@katana>
Hi Wolfram,
On Thu, 01 Dec 2016 19:46:22 +0900,
Wolfram Sang wrote:
>
> > > So my question is that "What is the reason behind to disable IRQ with
> > > SDIO_MODE? Is there any situation which masking with SDIO_INFO1_MASK
> > > is not enough?
>
> The code was introduced with 845ecd20239c28 ("mmc: tmio_mmc: implement
> SDIO IRQ support") which was in 2010. I don't have that old datasheets
> to check if the SCLKDIVEN restriction was already present in the SDHI
> cores which were available back then.
Is Arnd Hannemann, the author, around these days by any chance?
Or does anyone on the list have a shareable old datasheet? The one we
have is the one for R-Mobile A1 and has CONFIDENTIAL on it.
> My assumption is that it was not, or it was overlooked. So, it might be
> just for completeness that not only the individual IRQs have been
> disabled but also the big master switch (IOMOD) was turned off.
>
> My further assumption is that it is very likely good enough to disable
> the individual IRQs. If we can't guarantee the conditions to set IOMOD,
> it seems okay to me to just leave it.
Yes that's what we think and what we see with our test code, which is
just the original without IOMOD removed.
> Do you have a patch which works for you?
We don't have it yet. A question we have is that where should we
enable / disable the IOMOD?
Do we have to keep it disabled while the controller is in non-SDIO?
The spec says that we don't get SDIO irq while in non-SDIO mode. But
should we trust?
Or, should we disable it when we switch back to non-SDIO mode and
enable when we detect SDIO?
Please enlighten us this area. We'll submit the proper fix once we
know how we should do it.
Thanks,
--
yashi
^ permalink raw reply
* [PATCH 15/39] Annotate hardware config module parameters in drivers/mmc/host/
From: David Howells @ 2016-12-01 12:31 UTC (permalink / raw)
To: linux-kernel
Cc: gnomes, Ulf Hansson, minyard, linux-mmc, dhowells,
linux-security-module, keyrings, Pierre Ossman
In-Reply-To: <148059537897.31612.9461043954611464597.stgit@warthog.procyon.org.uk>
When the kernel is running in secure boot mode, we lock down the kernel to
prevent userspace from modifying the running kernel image. Whilst this
includes prohibiting access to things like /dev/mem, it must also prevent
access by means of configuring driver modules in such a way as to cause a
device to access or modify the kernel image.
To this end, annotate module_param* statements that refer to hardware
configuration and indicate for future reference what type of parameter they
specify. The parameter parser in the core sees this information and can
skip such parameters with an error message if the kernel is locked down.
The module initialisation then runs as normal, but just sees whatever the
default values for those parameters is.
Note that we do still need to do the module initialisation because some
drivers have viable defaults set in case parameters aren't specified and
some drivers support automatic configuration (e.g. PNP or PCI) in addition
to manually coded parameters.
This patch annotates drivers in drivers/mmc/host/.
Suggested-by: One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Pierre Ossman <pierre@ossman.eu>
cc: Ulf Hansson <ulf.hansson@linaro.org>
cc: linux-mmc@vger.kernel.org
---
drivers/mmc/host/wbsd.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/host/wbsd.c b/drivers/mmc/host/wbsd.c
index c3fd16d997ca..76c7f643fab5 100644
--- a/drivers/mmc/host/wbsd.c
+++ b/drivers/mmc/host/wbsd.c
@@ -1995,11 +1995,11 @@ static void __exit wbsd_drv_exit(void)
module_init(wbsd_drv_init);
module_exit(wbsd_drv_exit);
#ifdef CONFIG_PNP
-module_param_named(nopnp, param_nopnp, uint, 0444);
+module_param_hw_named(nopnp, param_nopnp, uint, other, 0444);
#endif
-module_param_named(io, param_io, uint, 0444);
-module_param_named(irq, param_irq, uint, 0444);
-module_param_named(dma, param_dma, int, 0444);
+module_param_hw_named(io, param_io, uint, ioport, 0444);
+module_param_hw_named(irq, param_irq, uint, irq, 0444);
+module_param_hw_named(dma, param_dma, int, dma, 0444);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Pierre Ossman <pierre@ossman.eu>");
^ permalink raw reply related
* Re: arasan,sdhci.txt "compatibility" DT binding
From: Mason @ 2016-12-01 12:29 UTC (permalink / raw)
To: linux-mmc
Cc: Mark Rutland, Ulf Hansson, Rameshwar Sahu, Suman Tripathi,
Heiko Stuebner, Shawn Lin, Adrian Hunter, Jisheng Zhang,
Russell King, Anton Vorontsov, Michal Simek, Soren Brinkmann,
Linus Walleij, P L Sai Krishna, Zach Brown, Sebastian Frias,
Arnd Bergmann, Suneel Garapati, Linux ARM, Michal Simek,
Douglas Anderson, Maxime Ripard, Xiaobo Xie
In-Reply-To: <f64ea0f8-31be-c5b5-3a43-a30abd8aec30@rock-chips.com>
On 01/12/2016 05:09, Shawn Lin wrote:
> On 2016/11/28 23:44, Mason wrote:
>
>> Shawn Lin, could you take a look below and tell me exactly
>> which IP core(s) Rockchip is using in its SoCs?
>
> From the Host Controller version register (0xfe)
> bit[7:0]: 0x2 : specification version number is 3.00
> bit[15:8]: 0x10: Vendor version number is 1.0
>
> Command Queueing version register (0x200)
> bit[11:8]: 0x5 eMMC Major version number
> bit[7:4]: 0x1 eMMC manor version number
> bit[3:0]: 0x0 eMMC version suffix
>
> User guide "eMMC 5.1/SD3.0/SDIO3.0 Host Controller"
> Revision number: 1.14
> Released on Dec. 2014
Wow! Yet another HW revision I wasn't aware of :-)
For the record, I think the 0x200 register was introduced fairly
recently, as it's not documented in any of the user guides I have
access to.
To summarize the situation, Arasan has made (at least) the
following versions of the HW block:
SD_2.0_SDIO_2.0__MMC_3.31_Host_Controller
SD_3.0_SDIO_3.0_eMMC_4.4__Host_Controller
SD_3.0_SDIO_3.0_eMMC_4.41_Host_Controller
SD_3.0_SDIO_3.0_eMMC_4.5__Host_Controller
SD_3.0_SDIO_3.0_eMMC_4.51_Host_Controller
SD_3.0_SDIO_3.0_eMMC_5.1__Host Controller
SD_4.1_SDIO_4.1_eMMC_4.51_Host_Controller
SD_4.1_SDIO_4.1_eMMC_5.1__Host_Controller
Xilinx = "arasan,sdhci-8.9a" compat string
SD2.0 / SDIO2.0 / MMC3.31 (in Zynq)
SD3.0 / SDIO3.0 / eMMC4.51 (in ZynqMP)
Vendor version 0x89 (Zynq, from 8.9a) and 0x10 (ZynqMP)
Sigma = no compat string yet
SD3.0 / SDIO3.0 / eMMC4.4 (in SMP87xx)
Vendor version 0x99 (not related to document revision)
APM = "arasan,sdhci-4.9a"
SD3.0 / SDIO3.0 / eMMC4.41
Vendor version unknown
Rockchip = "arasan,sdhci-5.1"
SD3.0 / SDIO3.0 / eMMC 5.1
Vendor version 0x10
Conclusion, it doesn't look like the "Vendor version" field
contains dependable information, considering the duplicate
0x10 in different HW revisions.
Regards.
^ permalink raw reply
* Re: mmc: tmio: why enable/disable SDIO irq on every transaction with IOMOD?
From: Wolfram Sang @ 2016-12-01 10:46 UTC (permalink / raw)
To: Ulf Hansson
Cc: Yasushi SHOJI, linux-mmc@vger.kernel.org, Wolfram Sang,
Simon Horman, linux-renesas-soc
In-Reply-To: <CAPDyKFop3a6eHhOZq1E=BdFiHzmpu4uEx0F79nyBVCOu6z6usw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1069 bytes --]
Hi,
thanks for the report and thanks to Ulf for forwarding. Adding
renesas-soc list, too.
> > So my question is that "What is the reason behind to disable IRQ with
> > SDIO_MODE? Is there any situation which masking with SDIO_INFO1_MASK
> > is not enough?
This code predates the time since I took over maintainership of the
driver, so I had to dig in git history.
The code was introduced with 845ecd20239c28 ("mmc: tmio_mmc: implement
SDIO IRQ support") which was in 2010. I don't have that old datasheets
to check if the SCLKDIVEN restriction was already present in the SDHI
cores which were available back then.
My assumption is that it was not, or it was overlooked. So, it might be
just for completeness that not only the individual IRQs have been
disabled but also the big master switch (IOMOD) was turned off.
My further assumption is that it is very likely good enough to disable
the individual IRQs. If we can't guarantee the conditions to set IOMOD,
it seems okay to me to just leave it.
Do you have a patch which works for you?
Regards,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 1/1] mmc: mmc: Relax checking for switch errors after HS200 switch
From: Ulf Hansson @ 2016-12-01 10:18 UTC (permalink / raw)
To: Adrian Hunter
Cc: linux-mmc, Jaehoon Chung, Chaotian Jing, Stephen Boyd,
Michael Walle, Yong Mao, Shawn Lin
In-Reply-To: <1479988969-1747-2-git-send-email-adrian.hunter@intel.com>
On 24 November 2016 at 13:02, Adrian Hunter <adrian.hunter@intel.com> wrote:
> The JEDEC specification indicates CMD13 can be used after a HS200 switch
> to check for errors. However in practice some boards experience CRC errors
> in the CMD13 response. Consequently, for HS200, CRC errors are not a
> reliable way to know the switch failed. If there really is a problem, we
> would expect tuning will fail and the result ends up the same. So change
> the error condition to ignore CRC errors in that case.
I agree, this seems like a good idea.
However...
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> drivers/mmc/core/mmc.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 3268fcd3378d..34d30e2a09ff 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1223,7 +1223,12 @@ int mmc_hs400_to_hs200(struct mmc_card *card)
> mmc_set_timing(host, MMC_TIMING_MMC_HS200);
>
> err = mmc_switch_status(card);
> - if (err)
> + /*
> + * For HS200, CRC errors are not a reliable way to know the switch
> + * failed. If there really is a problem, we would expect tuning will
> + * fail and the result ends up the same.
> + */
> + if (err && err != -EILSEQ)
I would rather change mmc_switch_status() to take a new parameter,
which tells it about ignoring CRC errors.
The reason is simply that I think these changes should probably apply
to HS400 as well, and then it just gets lots of duplicated code for
the same error check.
> goto out_err;
>
> mmc_set_bus_speed(card);
> @@ -1387,6 +1392,14 @@ static int mmc_select_hs200(struct mmc_card *card)
>
> err = mmc_switch_status(card);
> /*
> + * For HS200, CRC errors are not a reliable way to know the
> + * switch failed. If there really is a problem, we would expect
> + * tuning will fail and the result ends up the same.
> + */
> + if (err == -EILSEQ)
> + err = 0;
> +
> + /*
> * mmc_select_timing() assumes timing has not changed if
> * it is a switch error.
> */
> --
> 1.9.1
>
Moreover, and somewhat related to this change:
Do we want to change mmc_switch_status() to be able to tell
mmc_send_status() about the number of command retries? Currently it
defaults to MMC_CMD_RETRIES, but I guess we only want to send *one*
CMD13?
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH 1/4] mmc: mediatek: Fix CMD6 timeout issue
From: Ulf Hansson @ 2016-12-01 9:51 UTC (permalink / raw)
To: Yong Mao
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
YH Huang, Nicolas Boichat, Mathias Nyman, srv_heupstream,
Catalin Marinas, Will Deacon, Douglas Anderson,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Chunfeng Yun, Rob Herring, Geert Uytterhoeven,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Philipp Zabel, Matthias Brugger,
linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Eddie Huang,
Chaotian Jing <chaot>
In-Reply-To: <1478585341-6749-2-git-send-email-yong.mao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
On 8 November 2016 at 07:08, Yong Mao <yong.mao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> From: yong mao <yong.mao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>
> When initializing EMMC, after switch to HS400,
> it will issue CMD6 to change ext_csd, if first CMD6 got CRC
> error, the repeat CMD6 may get timeout, that's
> because SDCBSY was cleared by msdc_reset_hw()
Sorry for the delay!
We have recently been re-working the sequence for how to deal more
properly with CMD6 in the mmc core.
The changes done so far should mostly concern switches to HS and HS
DDR, but I think you should run a re-test to make sure you still hit
the same problems.
>
> Signed-off-by: Yong Mao <yong.mao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Chaotian Jing <chaotian.jing-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
> drivers/mmc/host/mtk-sd.c | 77 ++++++++++++++++++++++++++++++---------------
> 1 file changed, 51 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
> index 84e9afc..b29683b 100644
> --- a/drivers/mmc/host/mtk-sd.c
> +++ b/drivers/mmc/host/mtk-sd.c
> @@ -826,6 +826,15 @@ static bool msdc_cmd_done(struct msdc_host *host, int events,
> return true;
> }
>
> +static int msdc_card_busy(struct mmc_host *mmc)
> +{
> + struct msdc_host *host = mmc_priv(mmc);
> + u32 status = readl(host->base + MSDC_PS);
> +
> + /* check if data0 is low */
> + return !(status & BIT(16));
> +}
> +
> /* It is the core layer's responsibility to ensure card status
> * is correct before issue a request. but host design do below
> * checks recommended.
Hmm. Why?
I think you should rely on the mmc core to invoke the ->card_busy()
ops instead. The core knows better when it's needed.
Perhaps that may also resolve some of these issues for you!?
> @@ -835,10 +844,20 @@ static inline bool msdc_cmd_is_ready(struct msdc_host *host,
> {
> /* The max busy time we can endure is 20ms */
> unsigned long tmo = jiffies + msecs_to_jiffies(20);
> + u32 count = 0;
> +
> + if (in_interrupt()) {
> + while ((readl(host->base + SDC_STS) & SDC_STS_CMDBUSY) &&
> + (count < 1000)) {
> + udelay(1);
> + count++;
This seems like a bad idea, "busy-wait" in irq context is never a good idea.
> + }
> + } else {
> + while ((readl(host->base + SDC_STS) & SDC_STS_CMDBUSY) &&
> + time_before(jiffies, tmo))
> + cpu_relax();
> + }
>
> - while ((readl(host->base + SDC_STS) & SDC_STS_CMDBUSY) &&
> - time_before(jiffies, tmo))
> - cpu_relax();
> if (readl(host->base + SDC_STS) & SDC_STS_CMDBUSY) {
> dev_err(host->dev, "CMD bus busy detected\n");
> host->error |= REQ_CMD_BUSY;
> @@ -846,17 +865,35 @@ static inline bool msdc_cmd_is_ready(struct msdc_host *host,
> return false;
> }
>
> - if (mmc_resp_type(cmd) == MMC_RSP_R1B || cmd->data) {
> - tmo = jiffies + msecs_to_jiffies(20);
> - /* R1B or with data, should check SDCBUSY */
> - while ((readl(host->base + SDC_STS) & SDC_STS_SDCBUSY) &&
> - time_before(jiffies, tmo))
> - cpu_relax();
> - if (readl(host->base + SDC_STS) & SDC_STS_SDCBUSY) {
> - dev_err(host->dev, "Controller busy detected\n");
> - host->error |= REQ_CMD_BUSY;
> - msdc_cmd_done(host, MSDC_INT_CMDTMO, mrq, cmd);
> - return false;
> + if (cmd->opcode != MMC_SEND_STATUS) {
> + count = 0;
> + /* Consider that CMD6 crc error before card was init done,
> + * mmc_retune() will return directly as host->card is null.
> + * and CMD6 will retry 3 times, must ensure card is in transfer
> + * state when retry.
> + */
> + tmo = jiffies + msecs_to_jiffies(60 * 1000);
> + while (1) {
> + if (msdc_card_busy(host->mmc)) {
> + if (in_interrupt()) {
> + udelay(1);
> + count++;
> + } else {
> + msleep_interruptible(10);
> + }
> + } else {
> + break;
> + }
> + /* Timeout if the device never
> + * leaves the program state.
> + */
> + if (count > 1000 || time_after(jiffies, tmo)) {
> + pr_err("%s: Card stuck in programming state! %s\n",
> + mmc_hostname(host->mmc), __func__);
> + host->error |= REQ_CMD_BUSY;
> + msdc_cmd_done(host, MSDC_INT_CMDTMO, mrq, cmd);
> + return false;
> + }
This hole new code is a hack, that shouldn't be needed in the host driver.
I think we need to investigate and fix the issue in the mmc core
layer, to make this work for your host driver. That instead of doing a
work around in the host.
> }
> }
> return true;
> @@ -1070,18 +1107,6 @@ static int msdc_ops_switch_volt(struct mmc_host *mmc, struct mmc_ios *ios)
> return ret;
> }
>
> -static int msdc_card_busy(struct mmc_host *mmc)
> -{
> - struct msdc_host *host = mmc_priv(mmc);
> - u32 status = readl(host->base + MSDC_PS);
> -
> - /* check if any pin between dat[0:3] is low */
> - if (((status >> 16) & 0xf) != 0xf)
> - return 1;
> -
> - return 0;
> -}
> -
> static void msdc_request_timeout(struct work_struct *work)
> {
> struct msdc_host *host = container_of(work, struct msdc_host,
> --
> 1.7.9.5
>
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH 1/1] mmc: mmc: Relax checking for switch errors after HS200 switch
From: Jaehoon Chung @ 2016-12-01 9:18 UTC (permalink / raw)
To: Adrian Hunter, Ulf Hansson
Cc: linux-mmc, Chaotian Jing, Stephen Boyd, Michael Walle, Yong Mao,
Shawn Lin
In-Reply-To: <49e45a44-64ff-3898-ef44-aaef0f103ca5@intel.com>
Hi Adrian,
On 12/01/2016 05:15 PM, Adrian Hunter wrote:
> On 24/11/16 14:02, Adrian Hunter wrote:
>> The JEDEC specification indicates CMD13 can be used after a HS200 switch
>> to check for errors. However in practice some boards experience CRC errors
>> in the CMD13 response. Consequently, for HS200, CRC errors are not a
>> reliable way to know the switch failed. If there really is a problem, we
>> would expect tuning will fail and the result ends up the same. So change
>> the error condition to ignore CRC errors in that case.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>
> Any comments on this?
I agreed your opinion..CMD13 can't know whether switch is failed or not with CRC error.
It might just know whether HS200 is working fine or not with CRC error.
If CRC error is occurred, then user can knows when transfer some data.
Then i think it's more easier to debug than now..does it make sense?
Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
Best Regards,
Jaehoon Chung
>
>
>> ---
>> drivers/mmc/core/mmc.c | 15 ++++++++++++++-
>> 1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 3268fcd3378d..34d30e2a09ff 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -1223,7 +1223,12 @@ int mmc_hs400_to_hs200(struct mmc_card *card)
>> mmc_set_timing(host, MMC_TIMING_MMC_HS200);
>>
>> err = mmc_switch_status(card);
>> - if (err)
>> + /*
>> + * For HS200, CRC errors are not a reliable way to know the switch
>> + * failed. If there really is a problem, we would expect tuning will
>> + * fail and the result ends up the same.
>> + */
>> + if (err && err != -EILSEQ)
>> goto out_err;
>>
>> mmc_set_bus_speed(card);
>> @@ -1387,6 +1392,14 @@ static int mmc_select_hs200(struct mmc_card *card)
>>
>> err = mmc_switch_status(card);
>> /*
>> + * For HS200, CRC errors are not a reliable way to know the
>> + * switch failed. If there really is a problem, we would expect
>> + * tuning will fail and the result ends up the same.
>> + */
>> + if (err == -EILSEQ)
>> + err = 0;
>> +
>> + /*
>> * mmc_select_timing() assumes timing has not changed if
>> * it is a switch error.
>> */
>>
>
>
>
>
^ 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