[U-Boot] [PATCH 12/22] mmc: Enable signal voltage to be selected from mmc core

[Jean-Jacques Hiblot]

On 15/05/2017 05:28, Simon Glass wrote:
> Hi Jen-Jacques,
>
> On 12 May 2017 at 12:16, Jean-Jacques Hiblot <jjhiblot at ti.com> wrote:
>> From: Kishon Vijay Abraham I <kishon at ti.com>
>>
>> Add a new function *mmc_set_signal_voltage* in mmc core
>> which can be used during mmc initialization to select the
>> signal voltage. Platform driver should use the set_ios
>> callback function to select the signal voltage.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon at ti.com>
>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot at ti.com>
>> ---
>>   drivers/mmc/mmc.c | 15 +++++++++++++++
>>   include/mmc.h     |  5 +++++
>>   2 files changed, 20 insertions(+)
>>
>> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>> index 2ae6f1c..10af81d 100644
>> --- a/drivers/mmc/mmc.c
>> +++ b/drivers/mmc/mmc.c
>> @@ -29,6 +29,7 @@ static const unsigned int sd_au_size[] = {
>>          SZ_4M / 512,    SZ_8M / 512,            (SZ_8M + SZ_4M) / 512,
>>          SZ_16M / 512,   (SZ_16M + SZ_8M) / 512, SZ_32M / 512,   SZ_64M / 512,
>>   };
>> +static int mmc_set_signal_voltage(struct mmc *mmc, uint signal_voltage);
>>
>>   #if CONFIG_IS_ENABLED(MMC_TINY)
>>   static struct mmc mmc_static;
>> @@ -1247,6 +1248,12 @@ struct mode_width_tuning {
>>          uint widths;
>>   };
>>
>> +static int mmc_set_signal_voltage(struct mmc *mmc, uint signal_voltage)
>> +{
>> +       mmc->signal_voltage = signal_voltage;
>> +       return mmc_set_ios(mmc);
>> +}
>> +
>>   static const struct mode_width_tuning sd_modes_by_pref[] = {
>>          {
>>                  .mode = SD_HS,
>> @@ -1935,6 +1942,14 @@ int mmc_start_init(struct mmc *mmc)
>>                  return err;
>>   #endif
>>          mmc->ddr_mode = 0;
>> +
>> +       /* First try to set 3.3V. If it fails set to 1.8V */
>> +       err = mmc_set_signal_voltage(mmc, MMC_SIGNAL_VOLTAGE_330);
>> +       if (err != 0)
>> +               err = mmc_set_signal_voltage(mmc, MMC_SIGNAL_VOLTAGE_180);
>> +       if (err != 0)
>> +               printf("failed to set signal voltage\n");
> Return error?
Maybe we should. I don't know.
Setting signal voltage is optional (at least for most modes), some 
platforms don't support it. So I wouldn't exit on this kind of error. 
Those 2 calls to mmc_set_signal_voltage() are here merely to turn on 
regulators before doing the card init. If setting voltage is required 
and fails, the init process won't go very far.

>> +
>>          mmc_set_bus_width(mmc, 1);
>>          mmc_set_clock(mmc, 1);
>>
>> diff --git a/include/mmc.h b/include/mmc.h
>> index 9f20eb4..89cb26c 100644
>> --- a/include/mmc.h
>> +++ b/include/mmc.h
>> @@ -266,6 +266,10 @@
>>   #define ENHNCD_SUPPORT         (0x2)
>>   #define PART_ENH_ATTRIB                (0x1f)
>>
>> +#define MMC_SIGNAL_VOLTAGE_330 1
>> +#define MMC_SIGNAL_VOLTAGE_180 2
>> +#define MMC_SIGNAL_VOLTAGE_120 3
>> +
>>   /* Maximum block size for MMC */
>>   #define MMC_MAX_BLOCK_LEN      512
>>
>> @@ -452,6 +456,7 @@ struct mmc {
>>          int high_capacity;
>>          uint bus_width;
>>          uint clock;
>> +       uint signal_voltage;
> Comment. What does this value mean? What units is it? Millivolts or
> something else?
it's more an enum than a value with a physical meaning (like mV). I'll 
change this in the next version to be a real enum.
>
>>          uint card_caps;
>>          uint ocr;
>>          uint dsr;
>> --
>> 1.9.1
>>
> Regards,
> Simon
>