[PATCH] mmc: Add mmc_force_detect_change_begin / _end functions
Hans de Goede
hdegoede at redhat.com
Mon Aug 22 08:30:01 PDT 2016
Hi,
On 22-08-16 15:39, Ulf Hansson wrote:
> On 6 August 2016 at 15:05, Hans de Goede <hdegoede at redhat.com> wrote:
>> Some sdio devices have a multiple stage bring-up process. Specifically
>> the esp8089 (for which an out of tree driver is available) loads firmware
>> on the first call to its sdio-drivers' probe function and then resets
>> the device causing it to reboot from its RAM with the new firmware.
>>
>> When this sdio device reboots it comes back up in 1 bit 400 KHz mode
>> again, and we need to walk through the whole ios negatiation and sdio setup
>> again.
>>
>> There are 2 problems with this:
>>
>> 1) Typically these devices are soldered onto some (ARM) tablet / SBC
>> PCB and as such are described in devicetree as "non-removable", which
>> causes the mmc-core to scan them only once and not poll for the device
>> dropping of the bus. Normally this is the right thing todo but in the
>> eso8089 example we need the mmc-core to notice the module has disconnected
>> (since it is now in 1 bit mode again it will not talk to the host in 4 bit
>> mode). This can be worked around by using "broken-cd" in devicetree
>> instead of "non-removable", but that is not a proper fix since the device
>> really is non-removable.
>>
>> 2) When the mmc-core detects the device has disconnected it will poweroff
>> the device, causing the RAM loaded firmware to be lost. This can be worked
>> around in devicetree by using regulator-always-on (and avoiding the use of
>> mmc-pwrseq), but again that is more of a hack then a proper fix.
>
> Agree, and thanks for the detailed description of the problem!
>
>>
>> This commmit fixes 1) by adding a mmc_force_detect_change function which
>> will cause scanning for device removal / insertion until a new device is
>> detected. 2) Is fixed by a keep_power flag to the mmc_force_detect_change
>> function which when set causes the mmc-core to keep the power to the device
>> on during the rescan.
>
> Hmm, in the first scan-attempt there will be an SDIO card properly
> detected, right!?
Correct.
> The problem occurs when the SDIO func driver gets probed and when it
> loads the new firmware
Correct.
> which may happen later or even not at all as
> it also depends if the SDIO func driver has been registered.
The driver never loading is not a scenario we need to care about here,
because then the device does not need to be re-initialized :)
> I am also thinking about the system PM suspend/resume case. In cases
> where the SDIO card actually becomes powered off/on in suspend/resume.
> In this case, I assume the SDIO card will be re-detected through
> mmc_sdio_resume(), although the same sequence as during the SDIO func
> driver ->probe() must be repeated (loading firmware etc).
Right, ideally we would be able to put the device in a low power mode
but keep it powered, but that may not always be possible.
> To me it seems like we need a new SDIO func API, to re-initialize the
> SDIO card without powering off it first. I guess in the end it will be
> quite similar to what you propose, but my point is that I rather make
> this a specific SDIO API instead of trying to hook it into the rescan
> sequence.
Ah yes that would be more elegant, so basically we would reset
the ios to its defaults, call mmc_set_ios(host) and then
call mmc_sdio_init_card() with the card as oldcard as if it
is a resume ?
This would also mean the driver's probe function does not end up
getting called twice which certainly is better :)
I'm not sure I'm familiar enough with the mmc subsys to come
up with a good implementation of this though, nor when I'll have
the time to look into this.
Any chance you could make a RFC patch ? I can then test it on
hardware which actually needs this and bug-fix the code until
it actually works :)
Regards,
Hans
>> Cc: Icenowy Zheng <icenowy at aosc.xyz>
>> Cc: Maxime Ripard <maxime.ripard at free-electrons.com>
>> Cc: Chen-Yu Tsai <wens at csie.org>
>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>> ---
>> drivers/mmc/core/core.c | 47 ++++++++++++++++++++++++++++++++++++++++++-----
>> include/linux/mmc/host.h | 4 ++++
>> 2 files changed, 46 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index 8b4dfd4..d850cde 100644
>> --- a/drivers/mmc/core/core.c
>> +++ b/drivers/mmc/core/core.c
>> @@ -1716,8 +1716,11 @@ int mmc_select_drive_strength(struct mmc_card *card, unsigned int max_dtr,
>> */
>> void mmc_power_up(struct mmc_host *host, u32 ocr)
>> {
>> - if (host->ios.power_mode == MMC_POWER_ON)
>> + if (host->ios.power_mode == MMC_POWER_ON) {
>> + if (host->ios.clock == 0)
>> + goto set_clock;
>> return;
>> + }
>>
>> mmc_pwrseq_pre_power_on(host);
>>
>> @@ -1742,6 +1745,7 @@ void mmc_power_up(struct mmc_host *host, u32 ocr)
>>
>> mmc_pwrseq_post_power_on(host);
>>
>> +set_clock:
>> host->ios.clock = host->f_init;
>>
>> host->ios.power_mode = MMC_POWER_ON;
>> @@ -1759,6 +1763,11 @@ void mmc_power_off(struct mmc_host *host)
>> if (host->ios.power_mode == MMC_POWER_OFF)
>> return;
>>
>> + if (host->rescan_keep_power) {
>> + mmc_set_clock(host, 0);
>> + return;
>> + }
>> +
>> mmc_pwrseq_power_off(host);
>>
>> host->ios.clock = 0;
>> @@ -1907,6 +1916,27 @@ void mmc_detect_change(struct mmc_host *host, unsigned long delay)
>> }
>> EXPORT_SYMBOL(mmc_detect_change);
>>
>> +/**
>> + * mmc_force_detect_change - force rescanning of a MMC socket even if
>> + * it is non-removable
>> + * @host: host to rescan
>> + * @delay: optional delay to wait before detection (jiffies)
>> + * @keep_power: if set do not turn of vdd / call pwrseq_off during rescan
>> + *
>> + * MMC drivers which need non-removable sdio devices to be rescanned
>> + * (e.g. because the device reboots its fw after a firmware upload),
>> + * can call this to force scanning the MMC socket for changes, even
>> + * if it is non-removable.
>> + */
>> +void mmc_force_detect_change(struct mmc_host *host, unsigned long delay,
>> + bool keep_power)
>> +{
>> + host->rescan_force = 1;
>> + host->rescan_keep_power = keep_power;
>> + _mmc_detect_change(host, delay, false);
>> +}
>> +EXPORT_SYMBOL(mmc_force_detect_change);
>> +
>> void mmc_init_erase(struct mmc_card *card)
>> {
>> unsigned int sz;
>> @@ -2584,7 +2614,8 @@ void mmc_rescan(struct work_struct *work)
>> return;
>>
>> /* If there is a non-removable card registered, only scan once */
>> - if (!mmc_card_is_removable(host) && host->rescan_entered)
>> + if (!mmc_card_is_removable(host) && host->rescan_entered &&
>> + !host->rescan_force)
>> return;
>> host->rescan_entered = 1;
>>
>> @@ -2601,7 +2632,8 @@ void mmc_rescan(struct work_struct *work)
>> * if there is a _removable_ card registered, check whether it is
>> * still present
>> */
>> - if (host->bus_ops && !host->bus_dead && mmc_card_is_removable(host))
>> + if (host->bus_ops && !host->bus_dead &&
>> + (mmc_card_is_removable(host) || host->rescan_force))
>> host->bus_ops->detect(host);
>>
>> host->detect_change = 0;
>> @@ -2634,15 +2666,20 @@ void mmc_rescan(struct work_struct *work)
>> }
>>
>> for (i = 0; i < ARRAY_SIZE(freqs); i++) {
>> - if (!mmc_rescan_try_freq(host, max(freqs[i], host->f_min)))
>> + if (!mmc_rescan_try_freq(host, max(freqs[i], host->f_min))) {
>> + if (host->rescan_force) {
>> + host->rescan_force = 0;
>> + host->rescan_keep_power = 0;
>> + }
>> break;
>> + }
>> if (freqs[i] <= host->f_min)
>> break;
>> }
>> mmc_release_host(host);
>>
>> out:
>> - if (host->caps & MMC_CAP_NEEDS_POLL)
>> + if ((host->caps & MMC_CAP_NEEDS_POLL) || host->rescan_force)
>> mmc_schedule_delayed_work(&host->detect, HZ);
>> }
>>
>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>> index 45cde8c..e9b3115 100644
>> --- a/include/linux/mmc/host.h
>> +++ b/include/linux/mmc/host.h
>> @@ -333,6 +333,8 @@ struct mmc_host {
>>
>> int rescan_disable; /* disable card detection */
>> int rescan_entered; /* used with nonremovable devices */
>> + int rescan_force; /* force rescan of (nonremovable) devices */
>> + int rescan_keep_power; /* Do not power off card */
>>
>> int need_retune; /* re-tuning is needed */
>> int hold_retune; /* hold off re-tuning */
>> @@ -408,6 +410,8 @@ int mmc_power_save_host(struct mmc_host *host);
>> int mmc_power_restore_host(struct mmc_host *host);
>>
>> void mmc_detect_change(struct mmc_host *, unsigned long delay);
>> +void mmc_force_detect_change(struct mmc_host *host, unsigned long delay,
>> + bool keep_power);
>> void mmc_request_done(struct mmc_host *, struct mmc_request *);
>>
>> static inline void mmc_signal_sdio_irq(struct mmc_host *host)
>> --
>> 2.7.4
>>
>
> Kind regards
> Uffe
>
More information about the linux-arm-kernel
mailing list