[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d3222aeb-4920-41a2-a8d7-2551115ab776@intel.com>
Date: Wed, 14 May 2025 12:10:11 +0300
From: Adrian Hunter <adrian.hunter@...el.com>
To: Judith Mendez <jm@...com>, Ulf Hansson <ulf.hansson@...aro.org>
CC: Josua Mayer <josua@...id-run.com>, <linux-mmc@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, Nishanth Menon <nm@...com>, Francesco Dolcini
<francesco@...cini.it>, Hiago De Franco <hiagofranco@...il.com>, Moteen Shah
<m-shah@...com>
Subject: Re: [PATCH v5] mmc: sdhci_am654: Add SDHCI_QUIRK2_SUPPRESS_V1P8_ENA
quirk to am62 compatible
On 14/05/2025 03:25, Judith Mendez wrote:
> Add a new struct for platform data for the ti,am62-sdhci compatible to
> apply additional quirks, namely "SDHCI_QUIRK2_SUPPRESS_V1P8_ENA", to
> host controllers with am62 compatible.
Could add:
Note, the fix was originally introduced by commit 941a7abd4666
("mmc: sdhci_am654: Add sdhci_am654_start_signal_voltage_switch") but was
found to be applied too broadly and had to be reverted.
>
> This fixes MMC init failures seen across am62x boards.
>
> Fixes: ac5a41b472b4 ("Revert "mmc: sdhci_am654: Add sdhci_am654_start_signal_voltage_switch"")
> Fixes: 941a7abd4666 ("mmc: sdhci_am654: Add sdhci_am654_start_signal_voltage_switch")
> Cc: stable@...r.kernel.org
> Suggested-by: Nishanth Menon <nm@...com>
> Signed-off-by: Judith Mendez <jm@...com>
Minor cosmetic comments, otherwise:
Acked-by: Adrian Hunter <adrian.hunter@...el.com>
> ---
> Changes since v4:
> - Drop support for quirk in sdhci.c
> - Add quirk member to driver data struct that shall be per compatible quirks
>
> Tested on am62x SK, am64x EVM and am64x SK. Tested-bys are welcome
>
> Link to v4:
> https://lore.kernel.org/linux-mmc/20250424180036.1541568-1-jm@ti.com/
> Link to v3 RESEND:
> https://lore.kernel.org/linux-mmc/8678d284-db12-451a-b789-2b75f9932f9f@ti.com
> Link to v2:
> https://lore.kernel.org/linux-mmc/20250417182652.3521104-1-jm@ti.com/
> Link to v1:
> https://lore.kernel.org/linux-mmc/20250407222702.2199047-1-jm@ti.com/
> ---
> drivers/mmc/host/sdhci_am654.c | 41 +++++++++++++++++++++++++++++++++-
> 1 file changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
> index f75c31815ab0..c2bea228a67a 100644
> --- a/drivers/mmc/host/sdhci_am654.c
> +++ b/drivers/mmc/host/sdhci_am654.c
> @@ -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 {
> @@ -166,6 +167,7 @@ struct window {
> struct sdhci_am654_driver_data {
> const struct sdhci_pltfm_data *pdata;
> u32 flags;
> + u32 quirks;
> #define IOMUX_PRESENT (1 << 0)
> #define FREQSEL_2_BIT (1 << 1)
> #define STRBSEL_4_BIT (1 << 2)
> @@ -356,6 +358,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);
> @@ -650,6 +675,18 @@ static const struct sdhci_am654_driver_data sdhci_j721e_4bit_drvdata = {
> .flags = IOMUX_PRESENT,
> };
>
> +static const struct sdhci_pltfm_data sdhci_am62_4bit_pdata = {
> + .ops = &sdhci_j721e_4bit_ops,
> + .quirks = SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12,
> + .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
> +};
sdhci_am62_4bit_pdata is the same as sdhci_j721e_4bit_pdata
> +
> +static const struct sdhci_am654_driver_data sdhci_am62_4bit_drvdata = {
> + .pdata = &sdhci_am62_4bit_pdata,
> + .flags = IOMUX_PRESENT,
> + .quirks = SDHCI_AM654_QUIRK_SUPPRESS_V1P8_ENA,
> +};
> +
> static const struct soc_device_attribute sdhci_am654_devices[] = {
> { .family = "AM65X",
> .revision = "SR1.0",
> @@ -872,7 +909,7 @@ static const struct of_device_id sdhci_am654_of_match[] = {
> },
> {
> .compatible = "ti,am62-sdhci",
> - .data = &sdhci_j721e_4bit_drvdata,
> + .data = &sdhci_am62_4bit_drvdata,
> },
> { /* sentinel */ }
> };
> @@ -906,6 +943,7 @@ static int sdhci_am654_probe(struct platform_device *pdev)
> pltfm_host = sdhci_priv(host);
> sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
> sdhci_am654->flags = drvdata->flags;
> + sdhci_am654->quirks = drvdata->quirks;
>
> clk_xin = devm_clk_get(dev, "clk_xin");
> if (IS_ERR(clk_xin)) {
> @@ -940,6 +978,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: aa94665adc28f3fdc3de2979ac1e98bae961d6ca
Powered by blists - more mailing lists