Re: [PATCH v2 7/8] ARM: stm32mp: dk2: add barebox SD-Card update handler

From: Ahmad Fatoum <a.fatoum@pengutronix.de>
To: Sascha Hauer <s.hauer@pengutronix.de>
Cc: barebox@lists.infradead.org
Subject: Re: [PATCH v2 7/8] ARM: stm32mp: dk2: add barebox SD-Card update handler
Date: Mon, 28 Oct 2019 18:32:57 +0100	[thread overview]
Message-ID: <a2649862-2249-c8f4-6d02-5e38e22208df@pengutronix.de> (raw)
In-Reply-To: <20191028113157.oem65ah2fkaoh42s@pengutronix.de>

On 10/28/19 12:31 PM, Sascha Hauer wrote:
> On Mon, Oct 28, 2019 at 12:18:31AM +0100, Ahmad Fatoum wrote:
>> Now with the SD/MMC controller supported, lets add a bbu handler, so we
>> can use it to update the second stage boot loader partition.
>>
>> While doing this, I noticed that making use of the bus-width = <4>
>> property in the device tree now makes mmc usage fail when reading the
>> environment:
>>
>>   ERROR: error SDMMC_STA_DCRCFAIL (0x81042) for cmd 18
>>   ERROR: stm32_sdmmc2_end_cmd: error SDMMC_STA_CTIMEOUT (0x4) for cmd 12
>>   WARNING: stm32_sdmmc2_send_cmd: cmd 12 failed, retrying ...
>>   ERROR: stm32_sdmmc2_end_cmd: error SDMMC_STA_CTIMEOUT (0x4) for cmd 12
>>   WARNING: stm32_sdmmc2_send_cmd: cmd 12 failed, retrying ...
>>   ERROR: stm32_sdmmc2_end_cmd: error SDMMC_STA_CTIMEOUT (0x4) for cmd 12
>>   WARNING: stm32_sdmmc2_send_cmd: cmd 12 failed, retrying ...
>>   ERROR: stm32_sdmmc2_end_cmd: error SDMMC_STA_CTIMEOUT (0x4) for cmd 12
> 
> Could you make this an extra patch?

Can do.

> 
>>
>> We'll want to fix this eventually, but for now force the bus width to 1
>> and print a notice to the console that we've done so. This is not
>> enough, however because it then fails at loading the kernel from MMC:
>>
>>   ERROR: Time out on waiting for SDMMC_STA. cmd 18
>>   ERROR: stm32_sdmmc2_end_cmd: error SDMMC_STA_CTIMEOUT (0x804) for cmd 12
>>   WARNING: stm32_sdmmc2_send_cmd: cmd 18 failed, retrying ...
>>
>> Fixing the max frequency at 208 MHz fixes this. So do that and print a
>> notice for it as well.
>>
>> @@ -627,11 +627,22 @@ static int stm32_sdmmc2_probe(struct amba_device *adev,
>>  	if (IS_ERR(priv->reset_ctl))
>>  		priv->reset_ctl = NULL;
>>  
>> +	mci_of_parse(&priv->mci);
>> +
>>  	mci->f_min = 400000;
>> -	/* f_max is taken from kernel v5.3 variant_stm32_sdmmc */
>> -	mci->f_max = 208000000;
>>  	mci->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195;
>>  
>> +	if (mci->f_max != 208000000) {
>> +		/* f_max is taken from kernel v5.3 variant_stm32_sdmmc */
>> +		dev_notice(dev, "Fixing max-frequency to 208 MHz due to driver limitation\n");
>> +		mci->f_max = 208000000;
>> +	}
> 
> f_max previously was hardcoded to 208MHz. It still is now, so this can't
> fix anything.
> Looking at the device tree we have max-frequency = <120000000>, so you
> probably mean that it doesn't work with the 120MHz specified here,
> right? If it doesn't work with a lower frequency it really looks fishy.

Exactly.

> 
> Could you have another look what is going on here?

I intend to, but not sure if it'll be before the cut-off for v2019.12.0.
I'd like for that release to have full STM32MP support.

I'll send a v2. 

> 
> Sascha
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox