On 8 January 2013 01:56, Kevin Liu <keyuan.liu@xxxxxxxxx> wrote:
> 2013/1/8 Ulf Hansson <ulf.hansson@xxxxxxxxxx>:
>> On 7 January 2013 16:10, Johan Rudholm <johan.rudholm@xxxxxxxxxxxxxx> wrote:
>>> Allow callers to access the start_signal_voltage_switch host_ops
>>> member without going through any cmd11 logic. This is mostly a
>>> preparation for the following signal voltage switch patch.
>>>
>>> Signed-off-by: Johan Rudholm <johan.rudholm@xxxxxxxxxxxxxx>
>>> ---
>>> drivers/mmc/core/core.c | 31 +++++++++++++++++++------------
>>> drivers/mmc/core/core.h | 4 ++--
>>> drivers/mmc/core/mmc.c | 8 ++++----
>>> drivers/mmc/core/sd.c | 2 +-
>>> drivers/mmc/core/sdio.c | 3 +--
>>> 5 files changed, 27 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>> index 285f064..90f8e11 100644
>>> --- a/drivers/mmc/core/core.c
>>> +++ b/drivers/mmc/core/core.c
>>> @@ -1219,7 +1219,22 @@ u32 mmc_select_voltage(struct mmc_host *host, u32 ocr)
>>> return ocr;
>>> }
>>>
>>> -int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, bool cmd11)
>>> +int __mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage)
>>> +{
>>> + int err = 0;
>>> +
>>> + host->ios.signal_voltage = signal_voltage;
>>> + if (host->ops->start_signal_voltage_switch) {
>>> + mmc_host_clk_hold(host);
>>> + err = host->ops->start_signal_voltage_switch(host, &host->ios);
>>
>> Would it not make sense to restore the host->ios.signal_voltage value
>> if this goes wrong?
>>
>
> Why need to transfer ios to the voltage switch function which only
> need the voltage value.
> Why not just transfer the voltage value to the voltage switch
> function? If return ok then set host->ios.signal_voltage =
> signal_voltage otherwise do nothing.
> I did this in my previous patch.
I like the idea!
Although, if the "host->ops->start_signal_voltage_switch" function
pointer is missing, we should fall back to set the new voltage value
anyway I think.
>>> + mmc_host_clk_release(host);
>>> + }
>>> +
>>> + return err;
>>> +
>>> +}
>>> +
>>> +int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage)
>>> {
>>> struct mmc_command cmd = {0};
>>> int err = 0;
>>> @@ -1230,7 +1245,7 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, bool cmd11
>>> * Send CMD11 only if the request is to switch the card to
>>> * 1.8V signalling.
>>> */
>>> - if ((signal_voltage != MMC_SIGNAL_VOLTAGE_330) && cmd11) {
>>> + if (signal_voltage != MMC_SIGNAL_VOLTAGE_330) {
>>> cmd.opcode = SD_SWITCH_VOLTAGE;
>>> cmd.arg = 0;
>>> cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
>>> @@ -1243,15 +1258,7 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, bool cmd11
>>> return -EIO;
>>> }
>>>
>>> - host->ios.signal_voltage = signal_voltage;
>>> -
>>> - if (host->ops->start_signal_voltage_switch) {
>>> - mmc_host_clk_hold(host);
>>> - err = host->ops->start_signal_voltage_switch(host, &host->ios);
>>> - mmc_host_clk_release(host);
>>> - }
>>> -
>>> - return err;
>>> + return __mmc_set_signal_voltage(host, signal_voltage);
>>> }
>>>
>>> /*
>>> @@ -1314,7 +1321,7 @@ static void mmc_power_up(struct mmc_host *host)
>>> mmc_set_ios(host);
>>>
>>> /* Set signal voltage to 3.3V */
>>> - mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330, false);
>>> + __mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330);
>>>
>>> /*
>>> * This delay should be sufficient to allow the power supply
>>> diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
>>> index 4a90067..f2439d5 100644
>>> --- a/drivers/mmc/core/core.h
>>> +++ b/drivers/mmc/core/core.h
>>> @@ -40,8 +40,8 @@ void mmc_set_ungated(struct mmc_host *host);
>>> void mmc_set_bus_mode(struct mmc_host *host, unsigned int mode);
>>> void mmc_set_bus_width(struct mmc_host *host, unsigned int width);
>>> u32 mmc_select_voltage(struct mmc_host *host, u32 ocr);
>>> -int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage,
>>> - bool cmd11);
>>> +int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage);
>>> +int __mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage);
>>> void mmc_set_timing(struct mmc_host *host, unsigned int timing);
>>> void mmc_set_driver_type(struct mmc_host *host, unsigned int drv_type);
>>> void mmc_power_off(struct mmc_host *host);
>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>> index e6e3911..bd86771 100644
>>> --- a/drivers/mmc/core/mmc.c
>>> +++ b/drivers/mmc/core/mmc.c
>>> @@ -769,11 +769,11 @@ static int mmc_select_hs200(struct mmc_card *card)
>>>
>>> if (card->ext_csd.card_type & EXT_CSD_CARD_TYPE_SDR_1_2V &&
>>> host->caps2 & MMC_CAP2_HS200_1_2V_SDR)
>>> - err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_120, 0);
>>> + err = __mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_120);
>>>
>>> if (err && card->ext_csd.card_type & EXT_CSD_CARD_TYPE_SDR_1_8V &&
>>> host->caps2 & MMC_CAP2_HS200_1_8V_SDR)
>>> - err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_180, 0);
>>> + err = __mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_180);
>>>
>>> /* If fails try again during next card power cycle */
>>> if (err)
>>> @@ -1221,8 +1221,8 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>>> * WARNING: eMMC rules are NOT the same as SD DDR
>>> */
>>> if (ddr == MMC_1_2V_DDR_MODE) {
>>> - err = mmc_set_signal_voltage(host,
>>> - MMC_SIGNAL_VOLTAGE_120, 0);
>>> + err = __mmc_set_signal_voltage(host,
>>> + MMC_SIGNAL_VOLTAGE_120);
>>> if (err)
>>> goto err;
>>> }
>>> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
>>> index 74972c2..0cc1b26 100644
>>> --- a/drivers/mmc/core/sd.c
>>> +++ b/drivers/mmc/core/sd.c
>>> @@ -759,7 +759,7 @@ try_again:
>>> */
>>> if (!mmc_host_is_spi(host) && rocr &&
>>> ((*rocr & 0x41000000) == 0x41000000)) {
>>> - err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_180, true);
>>> + err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_180);
>>> if (err) {
>>> ocr &= ~SD_OCR_S18R;
>>> goto try_again;
>>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
>>> index 2273ce6..0c84e0f 100644
>>> --- a/drivers/mmc/core/sdio.c
>>> +++ b/drivers/mmc/core/sdio.c
>>> @@ -650,8 +650,7 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
>>> (MMC_CAP_UHS_SDR12 | MMC_CAP_UHS_SDR25 |
>>> MMC_CAP_UHS_SDR50 | MMC_CAP_UHS_SDR104 |
>>> MMC_CAP_UHS_DDR50))) {
>>> - err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_180,
>>> - true);
>>> + err = mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_180);
>>> if (err) {
>>> ocr &= ~R4_18V_PRESENT;
>>> host->ocr &= ~R4_18V_PRESENT;
>>> --
>>> 1.7.10
>>>
>>
>> Kind regards
>> Ulf Hansson
--
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
