mmc: sdhci_am654: Add sdhci_am654_start_signal_voltage_switch
Subsystems (3)
Judith Mendez
maintainerPATCH v2 1/1 mmc: sdhci_am654: Add sdhci_am654_start_signal_voltage_switch
The sdhci_start_signal_voltage_switch function sets V1P8_SIGNAL_ENA by default after switching to 1v8 signaling. V1P8_SIGNAL_ENA determines whether to launch cmd/data on neg edge or pos edge of clock.
Due to some eMMC and SD failures seen across am62x platform, do not set V1P8_SIGNAL_ENA by default, only enable the bit for devices that require this bit in order to switch to 1v8 voltage for uhs modes.
---
Changes since v1:
- Invert quirk logic
- Simplify sdhci_am654_start_signal_voltage_switch() and call sdhci_start_signal_voltage_switch() when the quirk does not apply
- Simply logic when detecting when quirk should be applied
--- drivers/mmc/host/sdhci_am654.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)
drivers/mmc/host/sdhci_am654.c +30 -0
@@ -155,6 +155,7 @@ struct sdhci_am654_data {u32 tuning_loop;#define SDHCI_AM654_QUIRK_FORCE_CDTEST BIT(0) +#define SDHCI_AM654_QUIRK_SUPPRESS_V1P8_ENA BIT(1) };struct window {
@@ -356,6 +357,29 @@ static void sdhci_j721e_4bit_set_clock(struct sdhci_host *host,sdhci_set_clock (host, clock); } +static int sdhci_am654_start_signal_voltage_switch (struct mmc_host *mmc,struct mmc_ios *ios) +{ +struct sdhci_host *host =mmc_priv (mmc); +struct sdhci_pltfm_host *pltfm_host =sdhci_priv (host); +struct sdhci_am654_data *sdhci_am654 =sdhci_pltfm_priv (pltfm_host); +int ret; + +if ((sdhci_am654->quirks &SDHCI_AM654_QUIRK_SUPPRESS_V1P8_ENA ) && + ios->signal_voltage ==MMC_SIGNAL_VOLTAGE_180 ) { +if (!IS_ERR (mmc->supply.vqmmc)) { + ret =mmc_regulator_set_vqmmc (mmc, ios); +if (ret <0 ) { +pr_err ("%s: Switching to 1.8V signalling voltage failed,\n" , +mmc_hostname (mmc)); +return -EIO ; + } + } +return 0 ; + } + +return sdhci_start_signal_voltage_switch (mmc, ios); +} +static u8 sdhci_am654_write_power_on (struct sdhci_host *host,u8 val,int reg) {writeb (val, host->ioaddr + reg);
@@ -844,6 +868,11 @@ static int sdhci_am654_get_of_property(struct platform_device *pdev,if (device_property_read_bool (dev,"ti,fails-without-test-cd" )) sdhci_am654->quirks |=SDHCI_AM654_QUIRK_FORCE_CDTEST ; +/* Suppress v1p8 ena for eMMC and SD with vqmmc supply */ +if (!!of_parse_phandle (dev->of_node,"vmmc-supply" ,0 ) == + !!of_parse_phandle (dev->of_node,"vqmmc-supply" ,0 )) + sdhci_am654->quirks |=SDHCI_AM654_QUIRK_SUPPRESS_V1P8_ENA ; +sdhci_get_of_property (pdev);return 0 ;
@@ -940,6 +969,7 @@ static int sdhci_am654_probe(struct platform_device *pdev) goto err_pltfm_free; } + host->mmc_host_ops.start_signal_voltage_switch = sdhci_am654_start_signal_voltage_switch; host->mmc_host_ops.execute_tuning = sdhci_am654_execute_tuning;pm_runtime_get_noresume (dev);
base-commit: cf6444ba528f043398b112ac36e041a4d8685cb1
Signature · 1 line
2.46.0
Quoted 11 lines · Judith Mendez
The
sdhci_start_signal_voltage_switchfunction setsV1P8_SIGNAL_ENAby default after switching to 1v8 signaling.V1P8_SIGNAL_ENAdetermines whether to launch cmd/data on neg edge or pos edge of clock.Due to some eMMC and SD failures seen across am62x platform, do not set
V1P8_SIGNAL_ENAby default, only enable the bit for devices that require this bit in order to switch to 1v8 voltage for uhs modes.Signed-off-by Judith Mendez <[email protected]>
Quoted 74 lines
---
Changes since v1:
- Invert quirk logic
- Simplifysdhci_am654_start_signal_voltage_switch()and callsdhci_start_signal_voltage_switch()when the quirk does not apply
- Simply logic when detecting when quirk should be applied--- drivers/mmc/host/sdhci_am654.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)drivers/mmc/host/sdhci_am654.c +30
-0@@ -155,6 +155,7 @@ struct sdhci_am654_data {u32 tuning_loop;#define SDHCI_AM654_QUIRK_FORCE_CDTEST BIT(0) +#define SDHCI_AM654_QUIRK_SUPPRESS_V1P8_ENA BIT(1) };struct window {@@ -356,6 +357,29 @@ static void sdhci_j721e_4bit_set_clock(struct sdhci_host *host,sdhci_set_clock (host, clock); } +static int sdhci_am654_start_signal_voltage_switch (struct mmc_host *mmc,struct mmc_ios *ios) +{ +struct sdhci_host *host =mmc_priv (mmc); +struct sdhci_pltfm_host *pltfm_host =sdhci_priv (host); +struct sdhci_am654_data *sdhci_am654 =sdhci_pltfm_priv (pltfm_host); +int ret; + +if ((sdhci_am654->quirks &SDHCI_AM654_QUIRK_SUPPRESS_V1P8_ENA ) && + ios->signal_voltage ==MMC_SIGNAL_VOLTAGE_180 ) { +if (!IS_ERR (mmc->supply.vqmmc)) { + ret =mmc_regulator_set_vqmmc (mmc, ios); +if (ret <0 ) { +pr_err ("%s: Switching to 1.8V signalling voltage failed,\n" , +mmc_hostname (mmc)); +return -EIO ; + } + } +return 0 ; + } + +return sdhci_start_signal_voltage_switch (mmc, ios); +} +static u8 sdhci_am654_write_power_on (struct sdhci_host *host,u8 val,int reg) {writeb (val, host->ioaddr + reg);@@ -844,6 +868,11 @@ static int sdhci_am654_get_of_property(struct platform_device *pdev,if (device_property_read_bool (dev,"ti,fails-without-test-cd" )) sdhci_am654->quirks |=SDHCI_AM654_QUIRK_FORCE_CDTEST ; +/* Suppress v1p8 ena for eMMC and SD with vqmmc supply */ +if (!!of_parse_phandle (dev->of_node,"vmmc-supply" ,0 ) == + !!of_parse_phandle (dev->of_node,"vqmmc-supply" ,0 )) + sdhci_am654->quirks |=SDHCI_AM654_QUIRK_SUPPRESS_V1P8_ENA ; +sdhci_get_of_property (pdev);return 0 ;@@ -940,6 +969,7 @@ static int sdhci_am654_probe(struct platform_device *pdev) goto err_pltfm_free; } + host->mmc_host_ops.start_signal_voltage_switch = sdhci_am654_start_signal_voltage_switch; host->mmc_host_ops.execute_tuning = sdhci_am654_execute_tuning;pm_runtime_get_noresume (dev);base-commit:
cf6444ba528f043398b112ac36e041a4d8685cb1
Quoted 6 lines · Judith Mendez
The
sdhci_start_signal_voltage_switchfunction setsV1P8_SIGNAL_ENAby default after switching to 1v8 signaling.V1P8_SIGNAL_ENAdetermines whether to launch cmd/data on neg edge or pos edge of clock.
Quoted 6 lines
Due to some eMMC and SD failures seen across am62x platform, do not set
V1P8_SIGNAL_ENAby default, only enable the bit for devices that require this bit in order to switch to 1v8 voltage for uhs modes.Signed-off-by Judith Mendez <[email protected]>
Applied for next, thanks!
Kind regards
Uffe
Quoted 77 lines
---
Changes since v1:
- Invert quirk logic
- Simplifysdhci_am654_start_signal_voltage_switch()and callsdhci_start_signal_voltage_switch()when the quirk does not apply
- Simply logic when detecting when quirk should be applied--- drivers/mmc/host/sdhci_am654.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)drivers/mmc/host/sdhci_am654.c +30
-0@@ -155,6 +155,7 @@ struct sdhci_am654_data {u32 tuning_loop;#define SDHCI_AM654_QUIRK_FORCE_CDTEST BIT(0) +#define SDHCI_AM654_QUIRK_SUPPRESS_V1P8_ENA BIT(1) };struct window {@@ -356,6 +357,29 @@ static void sdhci_j721e_4bit_set_clock(struct sdhci_host *host,sdhci_set_clock (host, clock); } +static int sdhci_am654_start_signal_voltage_switch (struct mmc_host *mmc,struct mmc_ios *ios) +{ +struct sdhci_host *host =mmc_priv (mmc); +struct sdhci_pltfm_host *pltfm_host =sdhci_priv (host); +struct sdhci_am654_data *sdhci_am654 =sdhci_pltfm_priv (pltfm_host); +int ret; + +if ((sdhci_am654->quirks &SDHCI_AM654_QUIRK_SUPPRESS_V1P8_ENA ) && + ios->signal_voltage ==MMC_SIGNAL_VOLTAGE_180 ) { +if (!IS_ERR (mmc->supply.vqmmc)) { + ret =mmc_regulator_set_vqmmc (mmc, ios); +if (ret <0 ) { +pr_err ("%s: Switching to 1.8V signalling voltage failed,\n" , +mmc_hostname (mmc)); +return -EIO ; + } + } +return 0 ; + } + +return sdhci_start_signal_voltage_switch (mmc, ios); +} +static u8 sdhci_am654_write_power_on (struct sdhci_host *host,u8 val,int reg) {writeb (val, host->ioaddr + reg);@@ -844,6 +868,11 @@ static int sdhci_am654_get_of_property(struct platform_device *pdev,if (device_property_read_bool (dev,"ti,fails-without-test-cd" )) sdhci_am654->quirks |=SDHCI_AM654_QUIRK_FORCE_CDTEST ; +/* Suppress v1p8 ena for eMMC and SD with vqmmc supply */ +if (!!of_parse_phandle (dev->of_node,"vmmc-supply" ,0 ) == + !!of_parse_phandle (dev->of_node,"vqmmc-supply" ,0 )) + sdhci_am654->quirks |=SDHCI_AM654_QUIRK_SUPPRESS_V1P8_ENA ; +sdhci_get_of_property (pdev);return 0 ;@@ -940,6 +969,7 @@ static int sdhci_am654_probe(struct platform_device *pdev) goto err_pltfm_free; } + host->mmc_host_ops.start_signal_voltage_switch = sdhci_am654_start_signal_voltage_switch; host->mmc_host_ops.execute_tuning = sdhci_am654_execute_tuning;pm_runtime_get_noresume (dev);base-commit:
cf6444ba528f043398b112ac36e041a4d8685cb1--
2.46.0
Hi Judith,
Am 13.09.24 um 20:54 schrieb Judith Mendez:
Quoted 9 lines
The
sdhci_start_signal_voltage_switchfunction setsV1P8_SIGNAL_ENAby default after switching to 1v8 signaling.V1P8_SIGNAL_ENAdetermines whether to launch cmd/data on neg edge or pos edge of clock.Due to some eMMC and SD failures seen across am62x platform, do not set
V1P8_SIGNAL_ENAby default, only enable the bit for devices that require this bit in order to switch to 1v8 voltage for uhs modes.
I have just tested this patch on downstream TI Linux fork tagged 10.01.10 (v6.6.58) and it breaks sd-card detection on SolidRun AM642 HummingBoard-T supported in-tree: arch/arm64/boot/dts/ti/k3-am642-hummingboard-t.dts since v6.11.
The board fails during boot from sdcard:
mmc1: error -110 whilst initialising SD card
Hence I suspect something is not okay with the heuristics enabling this quirk. Kindly note I have not tested it with pure 6.13-rc1 yet.
My downstream workaround in dts is to link a fixed regulator vmmc-supply, but the upstream dts specifies neither vmmc- nor vqmmc supplies.
Quoted 16 lines
Signed-off-by Judith Mendez <[email protected]>---
Changes since v1:
- Invert quirk logic
- Simplifysdhci_am654_start_signal_voltage_switch()and callsdhci_start_signal_voltage_switch()when the quirk does not apply
- Simply logic when detecting when quirk should be applied--- drivers/mmc/host/sdhci_am654.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)drivers/mmc/host/sdhci_am654.c +0
-0
cut
drivers/mmc/host/sdhci_am654.c static int sdhci_am654_get_of_property(struct platform_device *pdev, · line 875
Quoted 9 lines
drivers/mmc/host/sdhci_am654.c +5
-0@@ -844,6 +868,11 @@ static int sdhci_am654_get_of_property(struct platform_device *pdev,if (device_property_read_bool (dev,"ti,fails-without-test-cd" )) sdhci_am654->quirks |=SDHCI_AM654_QUIRK_FORCE_CDTEST ; +/* Suppress v1p8 ena for eMMC and SD with vqmmc supply */ +if (!!of_parse_phandle (dev->of_node,"vmmc-supply" ,0 ) == + !!of_parse_phandle (dev->of_node,"vqmmc-supply" ,0 )) + sdhci_am654->quirks |=SDHCI_AM654_QUIRK_SUPPRESS_V1P8_ENA ; +
Was it intentional to apply the quirk both when neither, and when both supplies are specified?
Quoted 13 lines
drivers/mmc/host/sdhci_am654.c +1
-0sdhci_get_of_property (pdev);return 0 ;@@ -940,6 +969,7 @@ static int sdhci_am654_probe(struct platform_device *pdev) goto err_pltfm_free; } + host->mmc_host_ops.start_signal_voltage_switch = sdhci_am654_start_signal_voltage_switch; host->mmc_host_ops.execute_tuning = sdhci_am654_execute_tuning;pm_runtime_get_noresume (dev);base-commit:
cf6444ba528f043398b112ac36e041a4d8685cb1
Am 19.01.25 um 21:15 schrieb Josua Mayer:
Quoted 16 lines
Hi Judith,
Am 13.09.24 um 20:54 schrieb Judith Mendez:
Quoted 9 lines
The
sdhci_start_signal_voltage_switchfunction setsV1P8_SIGNAL_ENAby default after switching to 1v8 signaling.V1P8_SIGNAL_ENAdetermines whether to launch cmd/data on neg edge or pos edge of clock.Due to some eMMC and SD failures seen across am62x platform, do not set
V1P8_SIGNAL_ENAby default, only enable the bit for devices that require this bit in order to switch to 1v8 voltage for uhs modes.I have just tested this patch on downstream TI Linux fork tagged 10.01.10 (v6.6.58) and it breaks sd-card detection on SolidRun AM642 HummingBoard-T supported in-tree:
arch/arm64/boot/dts/ti/k3-am642-hummingboard-t.dtssince v6.11.
Quoted 8 lines
The board fails during boot from sdcard:
mmc1: error -110 whilst initialising SD cardHence I suspect something is not okay with the heuristics enabling this quirk. Kindly note I have not tested it with pure 6.13-rc1 yet.
My downstream workaround in dts is to link a fixed regulator vmmc-supply, but the upstream dts specifies neither vmmc- nor vqmmc supplies.
I just tested v6.13.0 release and it has a regression failing microSD:
[ 1.960216] mmc1: CQHCI version 5.10
[ 2.008520] mmc1: SDHCI controller on fa00000.mmc [fa00000.mmc] using ADMA 64-bit [ 2.115348] mmc1: error -110 whilst initialising SD card
After reverting this patch original functionality is restored:
[ 1.952241] mmc1: CQHCI version 5.10
[ 2.003283] mmc1: SDHCI controller on fa00000.mmc [fa00000.mmc] using ADMA 64-bit [ 2.100210] mmc1: new UHS-I speed SDR104 SDHC card at address aaaa [ 2.107387] mmcblk1: mmc1:aaaa SC16G 14.8 GiB
[ 2.126668] mmcblk1: p1 p2
Quoted 41 lines
Quoted 16 lines
Signed-off-by Judith Mendez <[email protected]>---
Changes since v1:
- Invert quirk logic
- Simplifysdhci_am654_start_signal_voltage_switch()and callsdhci_start_signal_voltage_switch()when the quirk does not apply
- Simply logic when detecting when quirk should be applied--- drivers/mmc/host/sdhci_am654.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)drivers/mmc/host/sdhci_am654.c +0
-0cut
drivers/mmc/host/sdhci_am654.c static int sdhci_am654_get_of_property(struct platform_device *pdev, · line 875
Quoted 9 lines
drivers/mmc/host/sdhci_am654.c +5
-0@@ -844,6 +868,11 @@ static int sdhci_am654_get_of_property(struct platform_device *pdev,if (device_property_read_bool (dev,"ti,fails-without-test-cd" )) sdhci_am654->quirks |=SDHCI_AM654_QUIRK_FORCE_CDTEST ; +/* Suppress v1p8 ena for eMMC and SD with vqmmc supply */ +if (!!of_parse_phandle (dev->of_node,"vmmc-supply" ,0 ) == + !!of_parse_phandle (dev->of_node,"vqmmc-supply" ,0 )) + sdhci_am654->quirks |=SDHCI_AM654_QUIRK_SUPPRESS_V1P8_ENA ; +Was it intentional to apply the quirk both when neither, and when both supplies are specified?
Quoted 13 lines
drivers/mmc/host/sdhci_am654.c +1
-0sdhci_get_of_property (pdev);return 0 ;@@ -940,6 +969,7 @@ static int sdhci_am654_probe(struct platform_device *pdev) goto err_pltfm_free; } + host->mmc_host_ops.start_signal_voltage_switch = sdhci_am654_start_signal_voltage_switch; host->mmc_host_ops.execute_tuning = sdhci_am654_execute_tuning;pm_runtime_get_noresume (dev);base-commit:
cf6444ba528f043398b112ac36e041a4d8685cb1