On 10/27/2023 3:23 PM, Ulf Hansson wrote:
[...]
+{
+ int err = 0;
+ struct mmc_card *card = host->card;
+
+ mmc_set_bus_width(host, host->cached_ios.bus_width);
+ mmc_set_timing(host, host->cached_ios.timing);
+ if (host->cached_ios.enhanced_strobe) {
+ host->ios.enhanced_strobe = true;
+ if (host->ops->hs400_enhanced_strobe)
+ host->ops->hs400_enhanced_strobe(host, &host->ios);
+ }
+ mmc_set_clock(host, host->cached_ios.clock);
+ mmc_set_bus_mode(host, host->cached_ios.bus_mode);
+
Rather than re-using the above APIs and the ->set_ios() callback in
the host, I believe it would be better to add a new host ops to manage
all of the above at once instead. Something along the lines of the
below, would then replace all of the above.
host->ops->restore_ios(host, &host->cached_ios)
memcpy(&host->ios, &host->cached_ios, sizeof(host->ios));
Would that make sense to you too?
I didn't get this completely. Do you mean that we should implement a new
restore_ios callback (e.g. sdhci_restore_ios) similar to sdhci_set_ios
and removing all the redundant code from sdhci_set_ios which should
achieve the behaviour same as calling all the above mmc_set_* API's ?
Correct. Would it not simply the things in the driver too?
+ if (!mmc_card_hs400es(card) &&
+ (mmc_card_hs200(card) || mmc_card_hs400(card))) {
+ err = mmc_execute_tuning(card);
+ if (err) {
+ pr_err("%s: %s: Tuning failed (%d)\n",
+ mmc_hostname(host), __func__, err);
There is already a print being done in mmc_execute_tuning() at
failure. So, let's drop the above print.
Sure will take care in V4.
+ goto out;
+ }
+ }
+
+ err = mmc_test_awake_ext_csd(host);
Again, I don't get why this is needed, so let's discuss this more.
This is just a safety check added because ext_csd has some W/E_P or
W/C_P registers which gets reset if any HW reset happens to the card.
So this will check for those cases if any other vendor is doing reset as
part of suspend and compare a subset of those W/E_P and W/C_P registers
and if they are changed then we will bail out of this partial init
feature and go for full initialization.
We are also fine with removing this function but just added for the
above mentioned case.
In that case, I would rather remove it as I think it's superfluous.
More precisely, I would expect that we fail to wake up the card with a
CMD5 (get an error response for the CMD) if there has been a HW reset
somewhere done before.
Another reason to *not* read the ext_csd would be to further improve
the resume time, as reading it takes time too. I would be curious to
know how much though. :-)
[...]
Kind regards
Uffe
Hi ulf,
Sorry for the delay but we are seeing some stability issues when testing
this feature with HS400 cards which I am debugging and may take some
time and will come back.
Note: This feature is working perfectly fine with HS400ES cards.
