Re: [PATCH v2 1/2] mmc: core: Proper signal voltage switch

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Ulf,

2012/9/25 Ulf Hansson <ulf.hansson@xxxxxxxxxx>:
> On 25 September 2012 09:03, Johan Rudholm <johan.rudholm@xxxxxxxxxxxxxx> wrote:
>> @@ -1243,8 +1244,29 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, bool cmd11
>>         host->ios.signal_voltage = signal_voltage;
>>
>>         if (host->ops->start_signal_voltage_switch) {
>> +               u32 clock = host->ios.clock;
>> +
>>                 mmc_host_clk_hold(host);
>> +               host->ios.clock = 0;
>> +               mmc_set_ios(host);
>>                 err = host->ops->start_signal_voltage_switch(host, &host->ios);
>> +
>> +               /* Hold clock for at least 5 ms according to spec */
>
> This also potentially affects eMMC. Is that really what you want?
> According to commit msg you will only fixup something for SD and SDIO.

No, this should only happen when the switch is preceeded by CMD11. I
will address this, thanks.

> By the way, it seems like some fixes for drivers/mmc/core/sdio.c
> should be included in this patch as well? Or do you intend to send
> those changes as a separate patch?

I will address the SDIO changes in a separate patch. I was a bit
hesitant about how to do this, since we have no SDIO card (or host)
that supports UHS. However, I'll compose a patch set and submit it and
ask for help to verify it.

>> +               mmc_delay(5);
>> +               host->ios.clock = clock;
>> +               mmc_set_ios(host);
>> +
>> +               /* Wait for at least 1 ms until we check if card is ready */
>> +               mmc_delay(1);
>> +
>> +               /* Check busy */
>> +               if (cmd11 && host->ops->card_busy &&
>> +                                               host->ops->card_busy(host)) {
>> +                       host->ios.signal_voltage = old_voltage;
>
> Why bother about restoring signal voltage state? Could that not be
> handled from upper layer?

I guess, but I thought it nicer if the function restores the signal
voltage to the original state before returning failure. Also, if we do
it from the upper layers it would mean doing it at several places and
not only one. I'd prefer to keep it here, but I'll move it if you feel
strongly about it?

Kind regards, Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux