The following patches fix a couple of issues which prevented Venice2
boards from booting via eMMC and SD card reliably. Note that this
includes disabling UHS support since SDR50 and above require a
Tegra-specific tuning procedure which is not supported yet (and still
seems to have issues even in downstream kernels).
Tested on Tegra124-based Venice2 and Norrin boards.
Andrew Bresticker (4):
mmc: tegra: disable UHS modes
mmc: tegra: fix reporting of base clock frequency
mmc: sdhci: defer probing on regulator_get_optional() failures
ARM: tegra: fix Venice2 VQMMC regulators
arch/arm/boot/dts/tegra124-venice2.dts | 3 ++-
drivers/mmc/host/sdhci-tegra.c | 40 +++++++++++++++++++++++++---------
drivers/mmc/host/sdhci.c | 19 +++++++++++++---
3 files changed, 48 insertions(+), 14 deletions(-)
--
1.9.1.423.g4596e3a
Tegra SDHCI controllers, by default, report a base clock frequency
of 208Mhz in SDHCI_CAPABILTIES which may or may not be equal to the
actual base clock frequency. While this can be overridden by setting
BASE_CLK_FREQ in VENDOR_CLOCK_CTRL on Tegra30 and later SoCs, just
set SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN and supply a get_max_clock()
callback to get the actual rate of the base clock.
Signed-off-by: Andrew Bresticker <[email protected]>
---
drivers/mmc/host/sdhci-tegra.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 3cadd9c..c3f92d9 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -165,13 +165,15 @@ static const struct sdhci_ops tegra_sdhci_ops = {
.write_l = tegra_sdhci_writel,
.platform_bus_width = tegra_sdhci_buswidth,
.platform_reset_exit = tegra_sdhci_reset_exit,
+ .get_max_clock = sdhci_pltfm_clk_get_max_clock,
};
static const struct sdhci_pltfm_data sdhci_tegra20_pdata = {
.quirks = SDHCI_QUIRK_BROKEN_TIMEOUT_VAL |
SDHCI_QUIRK_SINGLE_POWER_WRITE |
SDHCI_QUIRK_NO_HISPD_BIT |
- SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC,
+ SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC |
+ SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
.ops = &tegra_sdhci_ops,
};
@@ -186,7 +188,8 @@ static const struct sdhci_pltfm_data sdhci_tegra30_pdata = {
SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
SDHCI_QUIRK_SINGLE_POWER_WRITE |
SDHCI_QUIRK_NO_HISPD_BIT |
- SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC,
+ SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC |
+ SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
.ops = &tegra_sdhci_ops,
};
@@ -202,7 +205,8 @@ static const struct sdhci_pltfm_data sdhci_tegra114_pdata = {
SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
SDHCI_QUIRK_SINGLE_POWER_WRITE |
SDHCI_QUIRK_NO_HISPD_BIT |
- SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC,
+ SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC |
+ SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
.ops = &tegra_sdhci_ops,
};
--
1.9.1.423.g4596e3a
Program TEGRA_SDHCI_VENDOR_MISC_CTRL so that UHS modes aren't advertised
in SDHCI_CAPABILITIES_1. While the Tegra SDHCI controller does support
these modes, they require Tegra-specific tuning and calibration routines
which the driver does not support yet.
Signed-off-by: Andrew Bresticker <[email protected]>
---
drivers/mmc/host/sdhci-tegra.c | 30 +++++++++++++++++++++++-------
1 file changed, 23 insertions(+), 7 deletions(-)
diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index a835898..3cadd9c 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -32,11 +32,17 @@
/* Tegra SDHOST controller vendor register definitions */
#define SDHCI_TEGRA_VENDOR_MISC_CTRL 0x120
+#define SDHCI_MISC_CTRL_ENABLE_SDR104 0x8
+#define SDHCI_MISC_CTRL_ENABLE_SDR50 0x10
#define SDHCI_MISC_CTRL_ENABLE_SDHCI_SPEC_300 0x20
+#define SDHCI_MISC_CTRL_ENABLE_DDR50 0x200
#define NVQUIRK_FORCE_SDHCI_SPEC_200 BIT(0)
#define NVQUIRK_ENABLE_BLOCK_GAP_DET BIT(1)
#define NVQUIRK_ENABLE_SDHCI_SPEC_300 BIT(2)
+#define NVQUIRK_DISABLE_SDR50 BIT(3)
+#define NVQUIRK_DISABLE_SDR104 BIT(4)
+#define NVQUIRK_DISABLE_DDR50 BIT(5)
struct sdhci_tegra_soc_data {
const struct sdhci_pltfm_data *pdata;
@@ -113,18 +119,23 @@ static void tegra_sdhci_reset_exit(struct sdhci_host *host, u8 mask)
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct sdhci_tegra *tegra_host = pltfm_host->priv;
const struct sdhci_tegra_soc_data *soc_data = tegra_host->soc_data;
+ u32 misc_ctrl;
if (!(mask & SDHCI_RESET_ALL))
return;
+ misc_ctrl = sdhci_readw(host, SDHCI_TEGRA_VENDOR_MISC_CTRL);
/* Erratum: Enable SDHCI spec v3.00 support */
- if (soc_data->nvquirks & NVQUIRK_ENABLE_SDHCI_SPEC_300) {
- u32 misc_ctrl;
-
- misc_ctrl = sdhci_readb(host, SDHCI_TEGRA_VENDOR_MISC_CTRL);
+ if (soc_data->nvquirks & NVQUIRK_ENABLE_SDHCI_SPEC_300)
misc_ctrl |= SDHCI_MISC_CTRL_ENABLE_SDHCI_SPEC_300;
- sdhci_writeb(host, misc_ctrl, SDHCI_TEGRA_VENDOR_MISC_CTRL);
- }
+ /* Don't advertise UHS modes which aren't supported yet */
+ if (soc_data->nvquirks & NVQUIRK_DISABLE_SDR50)
+ misc_ctrl &= ~SDHCI_MISC_CTRL_ENABLE_SDR50;
+ if (soc_data->nvquirks & NVQUIRK_DISABLE_DDR50)
+ misc_ctrl &= ~SDHCI_MISC_CTRL_ENABLE_DDR50;
+ if (soc_data->nvquirks & NVQUIRK_DISABLE_SDR104)
+ misc_ctrl &= ~SDHCI_MISC_CTRL_ENABLE_SDR104;
+ sdhci_writew(host, misc_ctrl, SDHCI_TEGRA_VENDOR_MISC_CTRL);
}
static int tegra_sdhci_buswidth(struct sdhci_host *host, int bus_width)
@@ -181,7 +192,9 @@ static const struct sdhci_pltfm_data sdhci_tegra30_pdata = {
static struct sdhci_tegra_soc_data soc_data_tegra30 = {
.pdata = &sdhci_tegra30_pdata,
- .nvquirks = NVQUIRK_ENABLE_SDHCI_SPEC_300,
+ .nvquirks = NVQUIRK_ENABLE_SDHCI_SPEC_300 |
+ NVQUIRK_DISABLE_SDR50 |
+ NVQUIRK_DISABLE_SDR104,
};
static const struct sdhci_pltfm_data sdhci_tegra114_pdata = {
@@ -195,6 +208,9 @@ static const struct sdhci_pltfm_data sdhci_tegra114_pdata = {
static struct sdhci_tegra_soc_data soc_data_tegra114 = {
.pdata = &sdhci_tegra114_pdata,
+ .nvquirks = NVQUIRK_DISABLE_SDR50 |
+ NVQUIRK_DISABLE_DDR50 |
+ NVQUIRK_DISABLE_SDR104,
};
static const struct of_device_id sdhci_tegra_dt_match[] = {
--
1.9.1.423.g4596e3a
VDDIO_SDMMC3 is the VQMMC supply, not the VMMC supply, for the SD
slot. Add 1.8V_VDDIO as the VQMMC supply for the eMMC.
Signed-off-by: Andrew Bresticker <[email protected]>
---
arch/arm/boot/dts/tegra124-venice2.dts | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/tegra124-venice2.dts b/arch/arm/boot/dts/tegra124-venice2.dts
index c17283c..53061ef 100644
--- a/arch/arm/boot/dts/tegra124-venice2.dts
+++ b/arch/arm/boot/dts/tegra124-venice2.dts
@@ -933,12 +933,13 @@
power-gpios = <&gpio TEGRA_GPIO(R, 0) GPIO_ACTIVE_HIGH>;
status = "okay";
bus-width = <4>;
- vmmc-supply = <&vddio_sdmmc3>;
+ vqmmc-supply = <&vddio_sdmmc3>;
};
sdhci@0,700b0600 {
status = "okay";
bus-width = <8>;
+ vqmmc-supply = <&vddio_1v8>;
};
ahub@0,70300000 {
--
1.9.1.423.g4596e3a
If regulator_get_optional() returns EPROBE_DEFER, it indicates
that the regulator may show up later (e.g. the DT property is
present but the corresponding regulator may not have probed).
Instead of continuing without the regulator, return EPROBE_DEFER
from sdhci_add_host(). Also, fix regulator leaks in the error
paths in sdhci_add_host().
Signed-off-by: Andrew Bresticker <[email protected]>
---
drivers/mmc/host/sdhci.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 9a79fc4..e87c5d3 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2978,7 +2978,9 @@ int sdhci_add_host(struct sdhci_host *host)
/* If vqmmc regulator and no 1.8V signalling, then there's no UHS */
host->vqmmc = regulator_get_optional(mmc_dev(mmc), "vqmmc");
if (IS_ERR_OR_NULL(host->vqmmc)) {
- if (PTR_ERR(host->vqmmc) < 0) {
+ if (PTR_ERR(host->vqmmc) == -EPROBE_DEFER) {
+ return PTR_ERR(host->vqmmc);
+ } else if (PTR_ERR(host->vqmmc) < 0) {
pr_info("%s: no vqmmc regulator found\n",
mmc_hostname(mmc));
host->vqmmc = NULL;
@@ -2993,6 +2995,7 @@ int sdhci_add_host(struct sdhci_host *host)
if (ret) {
pr_warn("%s: Failed to enable vqmmc regulator: %d\n",
mmc_hostname(mmc), ret);
+ regulator_put(host->vqmmc);
host->vqmmc = NULL;
}
}
@@ -3056,7 +3059,10 @@ int sdhci_add_host(struct sdhci_host *host)
host->vmmc = regulator_get_optional(mmc_dev(mmc), "vmmc");
if (IS_ERR_OR_NULL(host->vmmc)) {
- if (PTR_ERR(host->vmmc) < 0) {
+ if (PTR_ERR(host->vmmc) == -EPROBE_DEFER) {
+ ret = PTR_ERR(host->vmmc);
+ goto put_vqmmc;
+ } else if (PTR_ERR(host->vmmc) < 0) {
pr_info("%s: no vmmc regulator found\n",
mmc_hostname(mmc));
host->vmmc = NULL;
@@ -3150,7 +3156,8 @@ int sdhci_add_host(struct sdhci_host *host)
if (mmc->ocr_avail == 0) {
pr_err("%s: Hardware doesn't report any "
"support voltages.\n", mmc_hostname(mmc));
- return -ENODEV;
+ ret = -ENODEV;
+ goto put_vmmc;
}
spin_lock_init(&host->lock);
@@ -3280,6 +3287,12 @@ reset:
untasklet:
tasklet_kill(&host->card_tasklet);
tasklet_kill(&host->finish_tasklet);
+put_vmmc:
+ if (host->vmmc)
+ regulator_put(host->vmmc);
+put_vqmmc:
+ if (host->vqmmc)
+ regulator_put(host->vqmmc);
return ret;
}
--
1.9.1.423.g4596e3a
On Tue, Apr 15, 2014 at 10:42 AM, Andrew Bresticker
<[email protected]> wrote:
> If regulator_get_optional() returns EPROBE_DEFER, it indicates
> that the regulator may show up later (e.g. the DT property is
> present but the corresponding regulator may not have probed).
> Instead of continuing without the regulator, return EPROBE_DEFER
> from sdhci_add_host(). Also, fix regulator leaks in the error
> paths in sdhci_add_host().
I tried this series and while it seems to fix some MMC errors I was
seeing on a regular basis, I had to revert this particular patch which
is causing the following oops at boot on my Venice2 (with 3.15-rc1):
[ 2.007916] sdhci-tegra 700b0400.sdhci: Got CD GPIO #170.
[ 2.008197] Unable to handle kernel NULL pointer dereference at
virtual address 0000003c
[ 2.008202] pgd = c0004000
[ 2.008208] [0000003c] *pgd=00000000
[ 2.008216] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
[ 2.008222] Modules linked in:
[ 2.008230] CPU: 1 PID: 79 Comm: irq/362-700b040 Not tainted
3.15.0-rc1-00030-g20b0e68aa3b9 #1411
[ 2.008235] task: e61ea680 ti: e5862000 task.ti: e5862000
[ 2.008248] PC is at mmc_gpio_cd_irqt+0xc/0x3c
[ 2.008258] LR is at irq_thread_fn+0x1c/0x40
[ 2.008265] pc : [<c048d740>] lr : [<c006ae70>] psr: 60000113
[ 2.008265] sp : e5863f18 ip : 00000000 fp : c006ae54
[ 2.008268] r10: c091e4a4 r9 : e5845340 r8 : e60e31c0
[ 2.008272] r7 : 00000001 r6 : 00000000 r5 : e60e31c0 r4 : e5844800
[ 2.008277] r3 : 00000000 r2 : 00000000 r1 : e5844800 r0 : 0000016a
[ 2.008283] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM
Segment kernel
[ 2.008289] Control: 10c5387d Table: 8000406a DAC: 00000015
[ 2.008293] Process irq/362-700b040 (pid: 79, stack limit = 0xe5862240)
[ 2.008298] Stack: (0xe5863f18 to 0xe5864000)
[ 2.008304] 3f00:
e5845340 c006ae70
[ 2.008312] 3f20: e5845360 e5862000 00000000 c006b190 c006b058
e5862030 00000000 c006af9c
[ 2.008319] 3f40: 00000000 e5845300 00000000 e5845340 c006b058
00000000 00000000 00000000
[ 2.008326] 3f60: 00000000 c0040ea0 5a5a5a5a 00000000 5a5a5a5a
e5845340 00000000 00000000
[ 2.008333] 3f80: e5863f80 e5863f80 00000000 00000000 e5863f90
e5863f90 e5863fac e5845300
[ 2.008340] 3fa0: c0040dc8 00000000 00000000 c000e638 00000000
00000000 00000000 00000000
[ 2.008346] 3fc0: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
[ 2.008353] 3fe0: 00000000 00000000 00000000 00000000 00000013
00000000 00000000 00000000
[ 2.008369] [<c048d740>] (mmc_gpio_cd_irqt) from [<c006ae70>]
(irq_thread_fn+0x1c/0x40)
[ 2.008382] [<c006ae70>] (irq_thread_fn) from [<c006b190>]
(irq_thread+0x138/0x170)
[ 2.008393] [<c006b190>] (irq_thread) from [<c0040ea0>] (kthread+0xd8/0xf0)
[ 2.008404] [<c0040ea0>] (kthread) from [<c000e638>]
(ret_from_fork+0x14/0x3c)
[ 2.008413] Code: eaffffea e92d4010 e1a04001 e591318c (e593303c)
[ 2.008420] ---[ end trace 4e1b9273d6367aac ]---
[ 2.008442] Unable to handle kernel paging request at virtual
address ffffffec
[ 2.008444] pgd = c0004000
[ 2.008456] [ffffffec] *pgd=a77be821, *pte=00000000, *ppte=00000000
[ 2.008461] Internal error: Oops: 17 [#2] PREEMPT SMP ARM
[ 2.008466] Modules linked in:
[ 2.008472] CPU: 1 PID: 79 Comm: irq/362-700b040 Tainted: G D
3.15.0-rc1-00030-g20b0e68aa3b9 #1411
[ 2.008477] task: e61ea680 ti: e5862000 task.ti: e5862000
[ 2.008482] PC is at kthread_data+0x4/0xc
[ 2.008489] LR is at irq_thread_dtor+0x28/0xbc
[ 2.008495] pc : [<c00413d0>] lr : [<c006afc4>] psr: 20000113
[ 2.008495] sp : e5863ca8 ip : e5863f84 fp : e61ea680
[ 2.008498] r10: c048d744 r9 : 00000000 r8 : e5862010
[ 2.008502] r7 : e61ea680 r6 : c0982970 r5 : 00000000 r4 : e61ea680
[ 2.008505] r3 : 00000000 r2 : e5863ca8 r1 : e5863f38 r0 : e61ea680
[ 2.008511] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
[ 2.008515] Control: 10c5387d Table: 8000406a DAC: 00000015
[ 2.008520] Process irq/362-700b040 (pid: 79, stack limit = 0xe5862240)
[ 2.008523] Stack: (0xe5863ca8 to 0xe5864000)
[ 2.008530] 3ca0: c006af9c e61eab6c 00000000
c003e560 00000039 0000000b
[ 2.008539] 3cc0: e5863ed0 e5862000 0000000b c002781c e5863ce0
00000001 c0980ac4 c0669b70
[ 2.008546] 3ce0: c07e19d0 c0927710 00000002 c0922fc0 e5863ed0
e5862000 0000000b 00000001
[ 2.008552] 3d00: c048d742 c048d744 c0980ac4 c0011d08 e5862240
0000000b 60000113 c07dd458
[ 2.008559] 3d20: e5862010 00000000 00000000 00000008 65862010
66666661 20616566 64323965
[ 2.008567] 3d40: 30313034 61316520 30303430 35652031 31333139
28206338 33393565 63333033
[ 2.008574] 3d60: c0002029 c0669b70 c08416c8 0000003c 00000005
00000000 e5863ed0 0000003c
[ 2.008581] 3d80: e61ea680 c091e4a4 c006ae54 c0669484 00000005
c001b988 25e6c000 e604bfc0
[ 2.008588] 3da0: c091e478 e604aa00 00000001 e677fc00 00000530
c0056094 5a5a5a5a a55a5a5a
[ 2.008595] 3dc0: 3a393731 5a003436 5a5a5a5a 5a5a5a5a 0183681b
00000000 ffff8bfa e604aa00
[ 2.008602] 3de0: c091ef7c c0913c00 c0913c00 e604aa08 00000001
00000002 c091ecfc e677e4a8
[ 2.008609] 3e00: c091e470 00000000 00000000 00000005 c001bc24
c0923c20 0000003c e5863ed0
[ 2.008616] 3e20: e5845340 c091e4a4 c006ae54 c0008594 e677e4a8
00000000 00000000 00000020
[ 2.008623] 3e40: 00000000 00000002 0183681b 00000000 00000001
00000000 ffff8bfa c091e478
[ 2.008631] 3e60: e604aa00 e677fc00 ffff8b6e c0056f1c e5863ea4
c091ef7c 0000077d 00000000
[ 2.008638] 3e80: 77ad1fda 00000000 00000000 c09180c0 c00099f0
ffffffff 00000000 c0044cd4
[ 2.008645] 3ea0: e6084000 00000002 c0980910 00000000 00000000
c0044f60 c048d740 60000113
[ 2.008652] 3ec0: ffffffff e5863f04 e60e31c0 c0012418 0000016a
e5844800 00000000 00000000
[ 2.008659] 3ee0: e5844800 e60e31c0 00000000 00000001 e60e31c0
e5845340 c091e4a4 c006ae54
[ 2.008666] 3f00: 00000000 e5863f18 c006ae70 c048d740 60000113
ffffffff e5845340 c006ae70
[ 2.008673] 3f20: e5845360 e5862000 00000000 c006b190 c006b058
e5862030 00000000 c006af9c
[ 2.008680] 3f40: 00000000 e5845300 00000000 e5845340 c006b058
00000000 00000000 00000000
[ 2.008687] 3f60: 00000000 c0040ea0 5a5a5a5a 00000000 5a5a5a5a
e5845340 00000000 00000000
[ 2.008695] 3f80: e5863f80 e5863f80 00000001 00010001 e5863f90
e5863f90 e5863fac e5845300
[ 2.008701] 3fa0: c0040dc8 00000000 00000000 c000e638 00000000
00000000 00000000 00000000
[ 2.008707] 3fc0: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
[ 2.008714] 3fe0: 00000000 00000000 00000000 00000000 00000013
00000000 00000000 00000000
[ 2.008725] [<c00413d0>] (kthread_data) from [<c006afc4>]
(irq_thread_dtor+0x28/0xbc)
[ 2.008740] [<c006afc4>] (irq_thread_dtor) from [<c003e560>]
(task_work_run+0xb4/0xe4)
[ 2.008754] [<c003e560>] (task_work_run) from [<c002781c>]
(do_exit+0x2b8/0x8cc)
[ 2.008768] [<c002781c>] (do_exit) from [<c0011d08>] (die+0x400/0x418)
[ 2.008783] [<c0011d08>] (die) from [<c0669484>]
(__do_kernel_fault.part.11+0x64/0x74)
[ 2.008795] [<c0669484>] (__do_kernel_fault.part.11) from
[<c001b988>] (do_page_fault+0x1c8/0x3d0)
[ 2.008804] [<c001b988>] (do_page_fault) from [<c0008594>]
(do_DataAbort+0x38/0x98)
[ 2.008814] [<c0008594>] (do_DataAbort) from [<c0012418>]
(__dabt_svc+0x38/0x60)
[ 2.008818] Exception stack(0xe5863ed0 to 0xe5863f18)
[ 2.008824] 3ec0: 0000016a
e5844800 00000000 00000000
[ 2.008831] 3ee0: e5844800 e60e31c0 00000000 00000001 e60e31c0
e5845340 c091e4a4 c006ae54
[ 2.008838] 3f00: 00000000 e5863f18 c006ae70 c048d740 60000113 ffffffff
[ 2.008851] [<c0012418>] (__dabt_svc) from [<c048d740>]
(mmc_gpio_cd_irqt+0xc/0x3c)
[ 2.008864] [<c048d740>] (mmc_gpio_cd_irqt) from [<c006ae70>]
(irq_thread_fn+0x1c/0x40)
[ 2.008876] [<c006ae70>] (irq_thread_fn) from [<c006b190>]
(irq_thread+0x138/0x170)
[ 2.008885] [<c006b190>] (irq_thread) from [<c0040ea0>] (kthread+0xd8/0xf0)
[ 2.008895] [<c0040ea0>] (kthread) from [<c000e638>]
(ret_from_fork+0x14/0x3c)
[ 2.008903] Code: e513001c e7e00150 e12fff1e e5903374 (e5130014)
[ 2.008907] ---[ end trace 4e1b9273d6367aad ]---
[ 2.008910] Fixing recursive fault but reboot is needed!
On 04/14/2014 07:42 PM, Andrew Bresticker wrote:
> Tegra SDHCI controllers, by default, report a base clock frequency
> of 208Mhz in SDHCI_CAPABILTIES which may or may not be equal to the
> actual base clock frequency.
Some explanation of why this "may or may not be equal to the actual base
clock frequency" would be nice.
Presumably, it's because the clock frequency is supplied by the clock
controller module, and configuring that happens externally to the SD
controller, so the SD HW has no knowledge of the actual frequency, and
hence simply reports a hard-coded maximum possible clock frequency?
> While this can be overridden by setting
> BASE_CLK_FREQ in VENDOR_CLOCK_CTRL on Tegra30 and later SoCs, just
> set SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN and supply a get_max_clock()
> callback to get the actual rate of the base clock.
It's not clear to me from the function name that
sdhci_pltfm_clk_get_max_clock() simply calls clk_get() on the actual
clock. It might be nice to mention that in the commit description.
On 04/14/2014 07:42 PM, Andrew Bresticker wrote:
> If regulator_get_optional() returns EPROBE_DEFER, it indicates
> that the regulator may show up later (e.g. the DT property is
> present but the corresponding regulator may not have probed).
> Instead of continuing without the regulator, return EPROBE_DEFER
> from sdhci_add_host(). Also, fix regulator leaks in the error
> paths in sdhci_add_host().
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> if (IS_ERR_OR_NULL(host->vqmmc)) {
This is a pre-existing condition, but ick: Why doesn't this test either
IS_ERR() /or/ == NULL, but not both. On error, the regulator API should
either return and error-point or return NULL, so that client code
shouldn't need to check for both.
> +put_vmmc:
> + if (host->vmmc)
> + regulator_put(host->vmmc);
> +put_vqmmc:
> + if (host->vqmmc)
> + regulator_put(host->vqmmc);
If IS_ERR_OR_NULL() really is required above, it should be used here
too. More likely, I hope you need to replace if (host->vmmc) with if
(!IS_ERR(host->vmmc)).
I wonder if fixing this would help solve the crashes that Alex saw?
On 04/14/2014 07:42 PM, Andrew Bresticker wrote:
> VDDIO_SDMMC3 is the VQMMC supply, not the VMMC supply, for the SD
> slot. Add 1.8V_VDDIO as the VQMMC supply for the eMMC.
Is there documentation re: what vmmc-supply and vqmmc-supply actually
refer to? I looked a long while ago and couldn't find any, and didn't
get an answer when I asked.
Neither
Documentation/devicetree/bindings/mmc/{mmc.txt,nvidia,tegra20-sdhci.txt}
seem to say:-(
On Mon, Apr 14, 2014 at 6:42 PM, Andrew Bresticker
<[email protected]> wrote:
> If regulator_get_optional() returns EPROBE_DEFER, it indicates
> that the regulator may show up later (e.g. the DT property is
> present but the corresponding regulator may not have probed).
> Instead of continuing without the regulator, return EPROBE_DEFER
> from sdhci_add_host(). Also, fix regulator leaks in the error
> paths in sdhci_add_host().
>
> Signed-off-by: Andrew Bresticker <[email protected]>
> ---
> drivers/mmc/host/sdhci.c | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
This appears to be an improvement on Mike Looijmans patch:
https://lkml.org/lkml/2014/4/7/34
The regulator_put() calls are appropriate but I wonder if we should
take this a step farther. Ulf is sure to point out that
mmc_regulator_get_supply() can be used to get regulators (though it
does stuff the pointers in host->mmc->supply.vmmc/vqmmc instead of
host->vmmc/vqmmc). However, that function doesn't put back the
reference to vmmc if the request for vqmmc returns EPROBE_DEFER. If
it did, it believe it could be used here to simplify the error
handling as all the regulator checks would happen up front. What do
you think?
-Tim
Hi Stephen,
On Tue, Apr 15 2014, Stephen Warren wrote:
> Is there documentation re: what vmmc-supply and vqmmc-supply actually
> refer to? I looked a long while ago and couldn't find any, and didn't
> get an answer when I asked.
>
> Neither
> Documentation/devicetree/bindings/mmc/{mmc.txt,nvidia,tegra20-sdhci.txt}
> seem to say:-(
MMC cards use the same voltage for the VDD pin on the card and to pull
up the signal lines. UHS SDHCI switches to using 1.8V vqmmc (signaling
voltage) and 3.3V vmmc (card supply voltage).
So the idea is to supply a vqmmc-supply if your board supports
ultra-high-speed modes on an SD 3.0 host, and a vmmc-supply if your
card power is controlled by a regulator. I agree that this ought to
be in the bindings somewhere.
Thanks,
- Chris.
--
Chris Ball <http://printf.net/>
On Tue, Apr 15, 2014 at 11:25 AM, Stephen Warren <[email protected]> wrote:
> On 04/14/2014 07:42 PM, Andrew Bresticker wrote:
>> Tegra SDHCI controllers, by default, report a base clock frequency
>> of 208Mhz in SDHCI_CAPABILTIES which may or may not be equal to the
>> actual base clock frequency.
>
> Some explanation of why this "may or may not be equal to the actual base
> clock frequency" would be nice.
>
> Presumably, it's because the clock frequency is supplied by the clock
> controller module, and configuring that happens externally to the SD
> controller, so the SD HW has no knowledge of the actual frequency, and
> hence simply reports a hard-coded maximum possible clock frequency?
Correct. I'll fix up the commit message.
>
>> While this can be overridden by setting
>> BASE_CLK_FREQ in VENDOR_CLOCK_CTRL on Tegra30 and later SoCs, just
>> set SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN and supply a get_max_clock()
>> callback to get the actual rate of the base clock.
>
> It's not clear to me from the function name that
> sdhci_pltfm_clk_get_max_clock() simply calls clk_get() on the actual
> clock. It might be nice to mention that in the commit description.
On Tue, Apr 15, 2014 at 12:03 PM, Tim Kryger <[email protected]> wrote:
> On Mon, Apr 14, 2014 at 6:42 PM, Andrew Bresticker
> <[email protected]> wrote:
>> If regulator_get_optional() returns EPROBE_DEFER, it indicates
>> that the regulator may show up later (e.g. the DT property is
>> present but the corresponding regulator may not have probed).
>> Instead of continuing without the regulator, return EPROBE_DEFER
>> from sdhci_add_host(). Also, fix regulator leaks in the error
>> paths in sdhci_add_host().
>>
>> Signed-off-by: Andrew Bresticker <[email protected]>
>> ---
>> drivers/mmc/host/sdhci.c | 19 ++++++++++++++++---
>> 1 file changed, 16 insertions(+), 3 deletions(-)
>
> This appears to be an improvement on Mike Looijmans patch:
>
> https://lkml.org/lkml/2014/4/7/34
>
> The regulator_put() calls are appropriate but I wonder if we should
> take this a step farther. Ulf is sure to point out that
> mmc_regulator_get_supply() can be used to get regulators (though it
> does stuff the pointers in host->mmc->supply.vmmc/vqmmc instead of
> host->vmmc/vqmmc). However, that function doesn't put back the
> reference to vmmc if the request for vqmmc returns EPROBE_DEFER. If
> it did, it believe it could be used here to simplify the error
> handling as all the regulator checks would happen up front. What do
> you think?
Seems reasonable. The put()s aren't necessary with
mmc_regulator_get_supply() since it uses the devm_* API, however it
doesn't return an error in case of EPROBE_DEFER when getting vqmmc,
which is what we want in this case.
>
> -Tim
On Tue, Apr 15, 2014 at 11:28 AM, Stephen Warren <[email protected]> wrote:
> On 04/14/2014 07:42 PM, Andrew Bresticker wrote:
>> If regulator_get_optional() returns EPROBE_DEFER, it indicates
>> that the regulator may show up later (e.g. the DT property is
>> present but the corresponding regulator may not have probed).
>> Instead of continuing without the regulator, return EPROBE_DEFER
>> from sdhci_add_host(). Also, fix regulator leaks in the error
>> paths in sdhci_add_host().
>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>
>> if (IS_ERR_OR_NULL(host->vqmmc)) {
>
> This is a pre-existing condition, but ick: Why doesn't this test either
> IS_ERR() /or/ == NULL, but not both. On error, the regulator API should
> either return and error-point or return NULL, so that client code
> shouldn't need to check for both.
Unfortunately, this is necessary. regulator_get() returns NULL if
!CONFIG_REGULATOR and we do not want to call
regulator_is_supported_voltage() in this case since it will always
return false, causing sdhci_add_host() to disable UHS modes.
Alternatively, we could put the regulator_is_supported_voltage() check
wihin an #ifdef CONFIG_REGULATOR, as is done when checking supported
vmmc voltages below.
>
>> +put_vmmc:
>> + if (host->vmmc)
>> + regulator_put(host->vmmc);
>> +put_vqmmc:
>> + if (host->vqmmc)
>> + regulator_put(host->vqmmc);
>
> If IS_ERR_OR_NULL() really is required above, it should be used here
> too. More likely, I hope you need to replace if (host->vmmc) with if
> (!IS_ERR(host->vmmc)).
host->vmmc and host->vqmmc are set to NULL in the case of IS_ERR(), so
the IS_ERR() check here isn't necessary.
>
> I wonder if fixing this would help solve the crashes that Alex saw?
I believe the crash Alex is seeing is due to a race between tearing
down the sdhci driver on probe deferral and the card-detect IRQ.
Looking at it now.
On Tue, Apr 15, 2014 at 12:59:37PM -0700, Andrew Bresticker wrote:
> I believe the crash Alex is seeing is due to a race between tearing
> down the sdhci driver on probe deferral and the card-detect IRQ.
> Looking at it now.
You probably want to check out my massive sdhci patch set posted a
while back.
http://www.spinics.net/lists/linux-mmc/msg25031.html
I'll re-post it again soon (probably next week as I'm rather busy),
updated to v3.15-rc1.
--
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
On Mon, Apr 14, 2014 at 11:06 PM, Alexandre Courbot <[email protected]> wrote:
> On Tue, Apr 15, 2014 at 10:42 AM, Andrew Bresticker
> <[email protected]> wrote:
>> If regulator_get_optional() returns EPROBE_DEFER, it indicates
>> that the regulator may show up later (e.g. the DT property is
>> present but the corresponding regulator may not have probed).
>> Instead of continuing without the regulator, return EPROBE_DEFER
>> from sdhci_add_host(). Also, fix regulator leaks in the error
>> paths in sdhci_add_host().
>
> I tried this series and while it seems to fix some MMC errors I was
> seeing on a regular basis, I had to revert this particular patch which
> is causing the following oops at boot on my Venice2 (with 3.15-rc1):
>
> [ 2.007916] sdhci-tegra 700b0400.sdhci: Got CD GPIO #170.
> [ 2.008197] Unable to handle kernel NULL pointer dereference at
> virtual address 0000003c
> [ 2.008202] pgd = c0004000
> [ 2.008208] [0000003c] *pgd=00000000
> [ 2.008216] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
> [ 2.008222] Modules linked in:
> [ 2.008230] CPU: 1 PID: 79 Comm: irq/362-700b040 Not tainted
> 3.15.0-rc1-00030-g20b0e68aa3b9 #1411
> [ 2.008235] task: e61ea680 ti: e5862000 task.ti: e5862000
> [ 2.008248] PC is at mmc_gpio_cd_irqt+0xc/0x3c
> [ 2.008258] LR is at irq_thread_fn+0x1c/0x40
> [ 2.008265] pc : [<c048d740>] lr : [<c006ae70>] psr: 60000113
> [ 2.008265] sp : e5863f18 ip : 00000000 fp : c006ae54
> [ 2.008268] r10: c091e4a4 r9 : e5845340 r8 : e60e31c0
> [ 2.008272] r7 : 00000001 r6 : 00000000 r5 : e60e31c0 r4 : e5844800
> [ 2.008277] r3 : 00000000 r2 : 00000000 r1 : e5844800 r0 : 0000016a
> [ 2.008283] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM
> Segment kernel
> [ 2.008289] Control: 10c5387d Table: 8000406a DAC: 00000015
> [ 2.008293] Process irq/362-700b040 (pid: 79, stack limit = 0xe5862240)
> [ 2.008298] Stack: (0xe5863f18 to 0xe5864000)
> [ 2.008304] 3f00:
> e5845340 c006ae70
> [ 2.008312] 3f20: e5845360 e5862000 00000000 c006b190 c006b058
> e5862030 00000000 c006af9c
> [ 2.008319] 3f40: 00000000 e5845300 00000000 e5845340 c006b058
> 00000000 00000000 00000000
> [ 2.008326] 3f60: 00000000 c0040ea0 5a5a5a5a 00000000 5a5a5a5a
> e5845340 00000000 00000000
> [ 2.008333] 3f80: e5863f80 e5863f80 00000000 00000000 e5863f90
> e5863f90 e5863fac e5845300
> [ 2.008340] 3fa0: c0040dc8 00000000 00000000 c000e638 00000000
> 00000000 00000000 00000000
> [ 2.008346] 3fc0: 00000000 00000000 00000000 00000000 00000000
> 00000000 00000000 00000000
> [ 2.008353] 3fe0: 00000000 00000000 00000000 00000000 00000013
> 00000000 00000000 00000000
> [ 2.008369] [<c048d740>] (mmc_gpio_cd_irqt) from [<c006ae70>]
> (irq_thread_fn+0x1c/0x40)
> [ 2.008382] [<c006ae70>] (irq_thread_fn) from [<c006b190>]
> (irq_thread+0x138/0x170)
> [ 2.008393] [<c006b190>] (irq_thread) from [<c0040ea0>] (kthread+0xd8/0xf0)
> [ 2.008404] [<c0040ea0>] (kthread) from [<c000e638>]
> (ret_from_fork+0x14/0x3c)
> [ 2.008413] Code: eaffffea e92d4010 e1a04001 e591318c (e593303c)
> [ 2.008420] ---[ end trace 4e1b9273d6367aac ]---
This is due to the card-detect IRQ firing (which is requested/enabled
by mmc_of_parse()) before sdhci_add_host(), so mmc->ops will be NULL.
It looks like many MMC hosts have this bug. I suspect we weren't
hitting this before because the bootloader had configured the CD and
power GPIOs for the SD slot, and, when probe was deferred, they got
released as GPIOs. When we re-probed, they would get re-enabled as
GPIOs and the value on the line may have changed. Perhaps the CD IRQ
should not get requested by mmc_of_parse() and the host should be
responsible for requesting the IRQ later when it's ready. Thoughts?
+Adrian, who it looks like has been working on the CD/WP GPIO stuff.
> [ 2.008442] Unable to handle kernel paging request at virtual
> address ffffffec
> [ 2.008444] pgd = c0004000
> [ 2.008456] [ffffffec] *pgd=a77be821, *pte=00000000, *ppte=00000000
> [ 2.008461] Internal error: Oops: 17 [#2] PREEMPT SMP ARM
> [ 2.008466] Modules linked in:
> [ 2.008472] CPU: 1 PID: 79 Comm: irq/362-700b040 Tainted: G D
> 3.15.0-rc1-00030-g20b0e68aa3b9 #1411
> [ 2.008477] task: e61ea680 ti: e5862000 task.ti: e5862000
> [ 2.008482] PC is at kthread_data+0x4/0xc
> [ 2.008489] LR is at irq_thread_dtor+0x28/0xbc
> [ 2.008495] pc : [<c00413d0>] lr : [<c006afc4>] psr: 20000113
> [ 2.008495] sp : e5863ca8 ip : e5863f84 fp : e61ea680
> [ 2.008498] r10: c048d744 r9 : 00000000 r8 : e5862010
> [ 2.008502] r7 : e61ea680 r6 : c0982970 r5 : 00000000 r4 : e61ea680
> [ 2.008505] r3 : 00000000 r2 : e5863ca8 r1 : e5863f38 r0 : e61ea680
> [ 2.008511] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> [ 2.008515] Control: 10c5387d Table: 8000406a DAC: 00000015
> [ 2.008520] Process irq/362-700b040 (pid: 79, stack limit = 0xe5862240)
> [ 2.008523] Stack: (0xe5863ca8 to 0xe5864000)
> [ 2.008530] 3ca0: c006af9c e61eab6c 00000000
> c003e560 00000039 0000000b
> [ 2.008539] 3cc0: e5863ed0 e5862000 0000000b c002781c e5863ce0
> 00000001 c0980ac4 c0669b70
> [ 2.008546] 3ce0: c07e19d0 c0927710 00000002 c0922fc0 e5863ed0
> e5862000 0000000b 00000001
> [ 2.008552] 3d00: c048d742 c048d744 c0980ac4 c0011d08 e5862240
> 0000000b 60000113 c07dd458
> [ 2.008559] 3d20: e5862010 00000000 00000000 00000008 65862010
> 66666661 20616566 64323965
> [ 2.008567] 3d40: 30313034 61316520 30303430 35652031 31333139
> 28206338 33393565 63333033
> [ 2.008574] 3d60: c0002029 c0669b70 c08416c8 0000003c 00000005
> 00000000 e5863ed0 0000003c
> [ 2.008581] 3d80: e61ea680 c091e4a4 c006ae54 c0669484 00000005
> c001b988 25e6c000 e604bfc0
> [ 2.008588] 3da0: c091e478 e604aa00 00000001 e677fc00 00000530
> c0056094 5a5a5a5a a55a5a5a
> [ 2.008595] 3dc0: 3a393731 5a003436 5a5a5a5a 5a5a5a5a 0183681b
> 00000000 ffff8bfa e604aa00
> [ 2.008602] 3de0: c091ef7c c0913c00 c0913c00 e604aa08 00000001
> 00000002 c091ecfc e677e4a8
> [ 2.008609] 3e00: c091e470 00000000 00000000 00000005 c001bc24
> c0923c20 0000003c e5863ed0
> [ 2.008616] 3e20: e5845340 c091e4a4 c006ae54 c0008594 e677e4a8
> 00000000 00000000 00000020
> [ 2.008623] 3e40: 00000000 00000002 0183681b 00000000 00000001
> 00000000 ffff8bfa c091e478
> [ 2.008631] 3e60: e604aa00 e677fc00 ffff8b6e c0056f1c e5863ea4
> c091ef7c 0000077d 00000000
> [ 2.008638] 3e80: 77ad1fda 00000000 00000000 c09180c0 c00099f0
> ffffffff 00000000 c0044cd4
> [ 2.008645] 3ea0: e6084000 00000002 c0980910 00000000 00000000
> c0044f60 c048d740 60000113
> [ 2.008652] 3ec0: ffffffff e5863f04 e60e31c0 c0012418 0000016a
> e5844800 00000000 00000000
> [ 2.008659] 3ee0: e5844800 e60e31c0 00000000 00000001 e60e31c0
> e5845340 c091e4a4 c006ae54
> [ 2.008666] 3f00: 00000000 e5863f18 c006ae70 c048d740 60000113
> ffffffff e5845340 c006ae70
> [ 2.008673] 3f20: e5845360 e5862000 00000000 c006b190 c006b058
> e5862030 00000000 c006af9c
> [ 2.008680] 3f40: 00000000 e5845300 00000000 e5845340 c006b058
> 00000000 00000000 00000000
> [ 2.008687] 3f60: 00000000 c0040ea0 5a5a5a5a 00000000 5a5a5a5a
> e5845340 00000000 00000000
> [ 2.008695] 3f80: e5863f80 e5863f80 00000001 00010001 e5863f90
> e5863f90 e5863fac e5845300
> [ 2.008701] 3fa0: c0040dc8 00000000 00000000 c000e638 00000000
> 00000000 00000000 00000000
> [ 2.008707] 3fc0: 00000000 00000000 00000000 00000000 00000000
> 00000000 00000000 00000000
> [ 2.008714] 3fe0: 00000000 00000000 00000000 00000000 00000013
> 00000000 00000000 00000000
> [ 2.008725] [<c00413d0>] (kthread_data) from [<c006afc4>]
> (irq_thread_dtor+0x28/0xbc)
> [ 2.008740] [<c006afc4>] (irq_thread_dtor) from [<c003e560>]
> (task_work_run+0xb4/0xe4)
> [ 2.008754] [<c003e560>] (task_work_run) from [<c002781c>]
> (do_exit+0x2b8/0x8cc)
> [ 2.008768] [<c002781c>] (do_exit) from [<c0011d08>] (die+0x400/0x418)
> [ 2.008783] [<c0011d08>] (die) from [<c0669484>]
> (__do_kernel_fault.part.11+0x64/0x74)
> [ 2.008795] [<c0669484>] (__do_kernel_fault.part.11) from
> [<c001b988>] (do_page_fault+0x1c8/0x3d0)
> [ 2.008804] [<c001b988>] (do_page_fault) from [<c0008594>]
> (do_DataAbort+0x38/0x98)
> [ 2.008814] [<c0008594>] (do_DataAbort) from [<c0012418>]
> (__dabt_svc+0x38/0x60)
> [ 2.008818] Exception stack(0xe5863ed0 to 0xe5863f18)
> [ 2.008824] 3ec0: 0000016a
> e5844800 00000000 00000000
> [ 2.008831] 3ee0: e5844800 e60e31c0 00000000 00000001 e60e31c0
> e5845340 c091e4a4 c006ae54
> [ 2.008838] 3f00: 00000000 e5863f18 c006ae70 c048d740 60000113 ffffffff
> [ 2.008851] [<c0012418>] (__dabt_svc) from [<c048d740>]
> (mmc_gpio_cd_irqt+0xc/0x3c)
> [ 2.008864] [<c048d740>] (mmc_gpio_cd_irqt) from [<c006ae70>]
> (irq_thread_fn+0x1c/0x40)
> [ 2.008876] [<c006ae70>] (irq_thread_fn) from [<c006b190>]
> (irq_thread+0x138/0x170)
> [ 2.008885] [<c006b190>] (irq_thread) from [<c0040ea0>] (kthread+0xd8/0xf0)
> [ 2.008895] [<c0040ea0>] (kthread) from [<c000e638>]
> (ret_from_fork+0x14/0x3c)
> [ 2.008903] Code: e513001c e7e00150 e12fff1e e5903374 (e5130014)
> [ 2.008907] ---[ end trace 4e1b9273d6367aad ]---
> [ 2.008910] Fixing recursive fault but reboot is needed!
> sdhci@0,700b0600 {
> status = "okay";
> bus-width = <8>;
> + vqmmc-supply = <&vddio_1v8>;
It turns out this bit isn't needed as the initialization failures are
instead due to +3V3_RUN toggling, which Stephen has fixed:
http://patchwork.ozlabs.org/patch/339386/
On 04/15/2014 06:29 PM, Andrew Bresticker wrote:
>> sdhci@0,700b0600 {
>> status = "okay";
>> bus-width = <8>;
>> + vqmmc-supply = <&vddio_1v8>;
>
> It turns out this bit isn't needed as the initialization failures are
> instead due to +3V3_RUN toggling, which Stephen has fixed:
> http://patchwork.ozlabs.org/patch/339386/
Indeed, if I apply patches 1, 2, and 4 from this series, revert the hunk
above, apply my Venice2 regulator patch, and finally apply your AS3722
regulator GPIO patch, then everything works fine for me:-)
On Wed, Apr 16, 2014 at 9:19 AM, Stephen Warren <[email protected]> wrote:
> On 04/15/2014 06:29 PM, Andrew Bresticker wrote:
>>> sdhci@0,700b0600 {
>>> status = "okay";
>>> bus-width = <8>;
>>> + vqmmc-supply = <&vddio_1v8>;
>>
>> It turns out this bit isn't needed as the initialization failures are
>> instead due to +3V3_RUN toggling, which Stephen has fixed:
>> http://patchwork.ozlabs.org/patch/339386/
>
> Indeed, if I apply patches 1, 2, and 4 from this series, revert the hunk
> above, apply my Venice2 regulator patch, and finally apply your AS3722
> regulator GPIO patch, then everything works fine for me:-)
Great, I'll fix up and resend without patch 3 as it looks like that
will need some more work.
Program TEGRA_SDHCI_VENDOR_MISC_CTRL so that UHS modes aren't advertised
in SDHCI_CAPABILITIES_1. While the Tegra SDHCI controller does support
these modes, they require Tegra-specific tuning and calibration routines
which the driver does not support yet.
Signed-off-by: Andrew Bresticker <[email protected]>
---
No changes from v1
---
drivers/mmc/host/sdhci-tegra.c | 30 +++++++++++++++++++++++-------
1 file changed, 23 insertions(+), 7 deletions(-)
diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index a835898..3cadd9c 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -32,11 +32,17 @@
/* Tegra SDHOST controller vendor register definitions */
#define SDHCI_TEGRA_VENDOR_MISC_CTRL 0x120
+#define SDHCI_MISC_CTRL_ENABLE_SDR104 0x8
+#define SDHCI_MISC_CTRL_ENABLE_SDR50 0x10
#define SDHCI_MISC_CTRL_ENABLE_SDHCI_SPEC_300 0x20
+#define SDHCI_MISC_CTRL_ENABLE_DDR50 0x200
#define NVQUIRK_FORCE_SDHCI_SPEC_200 BIT(0)
#define NVQUIRK_ENABLE_BLOCK_GAP_DET BIT(1)
#define NVQUIRK_ENABLE_SDHCI_SPEC_300 BIT(2)
+#define NVQUIRK_DISABLE_SDR50 BIT(3)
+#define NVQUIRK_DISABLE_SDR104 BIT(4)
+#define NVQUIRK_DISABLE_DDR50 BIT(5)
struct sdhci_tegra_soc_data {
const struct sdhci_pltfm_data *pdata;
@@ -113,18 +119,23 @@ static void tegra_sdhci_reset_exit(struct sdhci_host *host, u8 mask)
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct sdhci_tegra *tegra_host = pltfm_host->priv;
const struct sdhci_tegra_soc_data *soc_data = tegra_host->soc_data;
+ u32 misc_ctrl;
if (!(mask & SDHCI_RESET_ALL))
return;
+ misc_ctrl = sdhci_readw(host, SDHCI_TEGRA_VENDOR_MISC_CTRL);
/* Erratum: Enable SDHCI spec v3.00 support */
- if (soc_data->nvquirks & NVQUIRK_ENABLE_SDHCI_SPEC_300) {
- u32 misc_ctrl;
-
- misc_ctrl = sdhci_readb(host, SDHCI_TEGRA_VENDOR_MISC_CTRL);
+ if (soc_data->nvquirks & NVQUIRK_ENABLE_SDHCI_SPEC_300)
misc_ctrl |= SDHCI_MISC_CTRL_ENABLE_SDHCI_SPEC_300;
- sdhci_writeb(host, misc_ctrl, SDHCI_TEGRA_VENDOR_MISC_CTRL);
- }
+ /* Don't advertise UHS modes which aren't supported yet */
+ if (soc_data->nvquirks & NVQUIRK_DISABLE_SDR50)
+ misc_ctrl &= ~SDHCI_MISC_CTRL_ENABLE_SDR50;
+ if (soc_data->nvquirks & NVQUIRK_DISABLE_DDR50)
+ misc_ctrl &= ~SDHCI_MISC_CTRL_ENABLE_DDR50;
+ if (soc_data->nvquirks & NVQUIRK_DISABLE_SDR104)
+ misc_ctrl &= ~SDHCI_MISC_CTRL_ENABLE_SDR104;
+ sdhci_writew(host, misc_ctrl, SDHCI_TEGRA_VENDOR_MISC_CTRL);
}
static int tegra_sdhci_buswidth(struct sdhci_host *host, int bus_width)
@@ -181,7 +192,9 @@ static const struct sdhci_pltfm_data sdhci_tegra30_pdata = {
static struct sdhci_tegra_soc_data soc_data_tegra30 = {
.pdata = &sdhci_tegra30_pdata,
- .nvquirks = NVQUIRK_ENABLE_SDHCI_SPEC_300,
+ .nvquirks = NVQUIRK_ENABLE_SDHCI_SPEC_300 |
+ NVQUIRK_DISABLE_SDR50 |
+ NVQUIRK_DISABLE_SDR104,
};
static const struct sdhci_pltfm_data sdhci_tegra114_pdata = {
@@ -195,6 +208,9 @@ static const struct sdhci_pltfm_data sdhci_tegra114_pdata = {
static struct sdhci_tegra_soc_data soc_data_tegra114 = {
.pdata = &sdhci_tegra114_pdata,
+ .nvquirks = NVQUIRK_DISABLE_SDR50 |
+ NVQUIRK_DISABLE_DDR50 |
+ NVQUIRK_DISABLE_SDR104,
};
static const struct of_device_id sdhci_tegra_dt_match[] = {
--
1.9.1.423.g4596e3a
Tegra SDHCI controllers, by default, report a base clock frequency
of 208Mhz in SDHCI_CAPABILTIES which may or may not be equal to the
actual base clock frequency. This is because the clock rate is
configured by the clock controller, which is external to the SD/MMC
controller. Since the SD/MMC controller has no knowledge of how this
clock is configured, it will simply report the maximum frequency.
While the reported value can be overridden by setting BASE_CLK_FREQ in
VENDOR_CLOCK_CTRL on Tegra30 and later SoCs, just set CAP_CLOCK_BASE_BROKEN
and supply sdhci_pltfm_clk_get_max_clock(), which simply does a
clk_get_rate(), as the get_max_clock() callback.
Signed-off-by: Andrew Bresticker <[email protected]>
---
Changes from v1:
- fixed up commit message per Stephen's suggestions
---
drivers/mmc/host/sdhci-tegra.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 3cadd9c..c3f92d9 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -165,13 +165,15 @@ static const struct sdhci_ops tegra_sdhci_ops = {
.write_l = tegra_sdhci_writel,
.platform_bus_width = tegra_sdhci_buswidth,
.platform_reset_exit = tegra_sdhci_reset_exit,
+ .get_max_clock = sdhci_pltfm_clk_get_max_clock,
};
static const struct sdhci_pltfm_data sdhci_tegra20_pdata = {
.quirks = SDHCI_QUIRK_BROKEN_TIMEOUT_VAL |
SDHCI_QUIRK_SINGLE_POWER_WRITE |
SDHCI_QUIRK_NO_HISPD_BIT |
- SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC,
+ SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC |
+ SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
.ops = &tegra_sdhci_ops,
};
@@ -186,7 +188,8 @@ static const struct sdhci_pltfm_data sdhci_tegra30_pdata = {
SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
SDHCI_QUIRK_SINGLE_POWER_WRITE |
SDHCI_QUIRK_NO_HISPD_BIT |
- SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC,
+ SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC |
+ SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
.ops = &tegra_sdhci_ops,
};
@@ -202,7 +205,8 @@ static const struct sdhci_pltfm_data sdhci_tegra114_pdata = {
SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
SDHCI_QUIRK_SINGLE_POWER_WRITE |
SDHCI_QUIRK_NO_HISPD_BIT |
- SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC,
+ SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC |
+ SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
.ops = &tegra_sdhci_ops,
};
--
1.9.1.423.g4596e3a
VDDIO_SDMMC3 is the VQMMC (I/O) supply, not the VMMC (core) supply,
for the SD slot on Venice2.
Signed-off-by: Andrew Bresticker <[email protected]>
---
Changes from v1:
- removed vqmmc supply for eMMC
---
arch/arm/boot/dts/tegra124-venice2.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/tegra124-venice2.dts b/arch/arm/boot/dts/tegra124-venice2.dts
index c17283c..91b5916 100644
--- a/arch/arm/boot/dts/tegra124-venice2.dts
+++ b/arch/arm/boot/dts/tegra124-venice2.dts
@@ -933,7 +933,7 @@
power-gpios = <&gpio TEGRA_GPIO(R, 0) GPIO_ACTIVE_HIGH>;
status = "okay";
bus-width = <4>;
- vmmc-supply = <&vddio_sdmmc3>;
+ vqmmc-supply = <&vddio_sdmmc3>;
};
sdhci@0,700b0600 {
--
1.9.1.423.g4596e3a
The following patches fix a couple of issues which prevented Venice2
boards from booting via eMMC and SD card reliably. Note that this
includes disabling UHS support since SDR50 and above require a
Tegra-specific tuning procedure which is not supported yet (and still
seems to have issues even in downstream kernels).
This depends on the following patches which fix an issue where the
eMMC's power supply gets toggled, causing eMMC failures:
http://patchwork.ozlabs.org/patch/339386/
http://patchwork.ozlabs.org/patch/339710/
Tested on Tegra124-based Venice2 and Norrin boards.
Andrew Bresticker (3):
mmc: tegra: disable UHS modes
mmc: tegra: fix reporting of base clock frequency
ARM: tegra: fix Venice2 SD card VQMMC supply
arch/arm/boot/dts/tegra124-venice2.dts | 2 +-
drivers/mmc/host/sdhci-tegra.c | 40 +++++++++++++++++++++++++---------
2 files changed, 31 insertions(+), 11 deletions(-)
--
1.9.1.423.g4596e3a
On 04/16/2014 05:08 PM, Andrew Bresticker wrote:
> VDDIO_SDMMC3 is the VQMMC (I/O) supply, not the VMMC (core) supply,
> for the SD slot on Venice2.
I've applied this (one patch) to Tegra's for-3.16/dt branch.