linux-kernel - Re: [PATCH] Revert "mmc: sdhci_am654: Add sdhci_am654_start_signal_voltage

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <4baab328-f78d-43d0-b67a-d31865a8dbb3@solid-run.com>
Date: Mon, 27 Jan 2025 20:08:55 +0000
From: Josua Mayer <josua@...id-run.com>
To: Adrian Hunter <adrian.hunter@...el.com>, Ulf Hansson
	<ulf.hansson@...aro.org>, Judith Mendez <jm@...com>
CC: "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Rabeeh Khoury
	<rabeeh@...id-run.com>, Jon Nettleton <jon@...id-run.com>,
	"stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: Re: [PATCH] Revert "mmc: sdhci_am654: Add
 sdhci_am654_start_signal_voltage_switch"

Am 27.01.25 um 15:03 schrieb Adrian Hunter:
> On 27/01/25 15:35, Josua Mayer wrote:
>> This reverts commit 941a7abd4666912b84ab209396fdb54b0dae685d.
>>
>> This commit uses presence of device-tree properties vmmc-supply and
>> vqmmc-supply for deciding whether to enable a quirk affecting timing of
>> clock and data.
>> The intention was to address issues observed with eMMC and SD on AM62
>> platforms.
>>
>> This new quirk is however also enabled for AM64 breaking microSD access
>> on the SolidRun HimmingBoard-T which is supported in-tree since v6.11,
>> causing a regression. During boot microSD initialization now fails with
>> the error below:
>>
>> [    2.008520] mmc1: SDHCI controller on fa00000.mmc [fa00000.mmc] using ADMA 64-bit
>> [    2.115348] mmc1: error -110 whilst initialising SD card
>>
>> The heuristics for enabling the quirk are clearly not correct as they
>> break at least one but potentially many existing boards.
>>
>> Revert the change and restore original behaviour until a more
>> appropriate method of selecting the quirk is derived.
>>
>> Fixes: <941a7abd4666> ("mtd: spi-nor: core: replace dummy buswidth from addr to data")
> Angle brackets < > should not be present, 
Adrian, Greg, thank you for pointing this out, I will resolve it with v2.
> and the commit
> description is incorrect.

Now I see why checkpatch had complained before adding <>,
Thank you!

>
>> Closes: https://lore.kernel.org/linux-mmc/a70fc9fc-186f-4165-a652-3de50733763a@solid-run.com/
>> Cc: stable@...r.kernel.org # 6.13
> With a Fixes tag, better to leave out "# 6.13"
Okay.
>
>> Signed-off-by: Josua Mayer <josua@...id-run.com>
> In the absence of an alternative, we have to revert, so:
I do not understand the quirk sufficiently to suggest an alternative :(.
>
> Acked-by: Adrian Hunter <adrian.hunter@...el.com>
>
>> ---
>>  drivers/mmc/host/sdhci_am654.c | 30 ------------------------------
>>  1 file changed, 30 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
>> index b73f673db92bbc042392995e715815e15ace6005..f75c31815ab00d17b5757063521f56ba5643babe 100644
>> --- a/drivers/mmc/host/sdhci_am654.c
>> +++ b/drivers/mmc/host/sdhci_am654.c
>> @@ -155,7 +155,6 @@ 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 {
>> @@ -357,29 +356,6 @@ 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);
>> @@ -868,11 +844,6 @@ 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;
>> @@ -969,7 +940,6 @@ 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: ffd294d346d185b70e28b1a28abe367bbfe53c04
>> change-id: 20250127-am654-mmc-regression-ed289f8967c2
>>
>> Best regards,