Re: [RFC] mmc: core: add the capability for broken voltage

[Adrian Hunter <adrian.hunter@intel.com>]

On 18/01/12 10:13, Kyungmin Park wrote:
> On 1/18/12, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> On 18/01/12 01:58, Kyungmin Park wrote:
>>> On 1/17/12, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>> On 17/01/12 11:53, Kyungmin Park wrote:
>>>>> On 1/17/12, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>>>> On 17/01/12 11:05, Kyungmin Park wrote:
>>>>>>> On 1/17/12, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>>>>>> On 16/01/12 10:49, Jaehoon Chung wrote:
>>>>>>>>> This patch is added the MMC_CAP2_BROKEN_VOLTAGE.
>>>>>>>>>
>>>>>>>>> if the voltage didn't satisfy between min_uV and max_uV,
>>>>>>>>
>>>>>>>> Why is the fixed voltage not in the acceptable range for the card?
>>>>>>>> Doesn't that risk breaking the card?
>>>>>>> Hi Adrian,
>>>>>>>
>>>>>>> I don't know, they uses the not supported voltage. but it's worked
>>>>>>> properly for long time.
>>>>>>
>>>>>> I wonder if there is a generic problem here that this fix hides.
>>>>>> It would be nice to have an explanation.  Do you know:
>>>>>> 	- what is the fixed voltage?
>>>>> Now 2.8V is supplied by gpio so make it fixed regulator
>>>>>> 	- is it supplying VCC or VCCQ?
>>>>> VDD in our schematic.
>>>>
>>>> Is it the NAND core voltage or the I/O voltage?
>>> In case of eMMC v4.41, it uses the same voltage, VDD is used for MMC
>>> I/O block and controller and another VDDF is used for flash.
>>
>> It seems to me you just need to override the value of mmc->ocr_avail_mmc
>> calculated in sdhci_add_host() in sdhci.c to reflect the fact that the only
>> voltage available is 2.8V.
> 
> current sdhci ocr_avail is 0x300080 and wanted ocr is 0x10000 so it
> doesn't support it.
> 
> [    0.984754] sdhci_add_host[2888] ocr_avail 0x300080
> [    0.989909] sdhci_add_host[2889] host->ocr_avail_mmc 0x10000

Yes because sdhci_add_host() ANDs the values i.e.

	if (host->ocr_avail_mmc)
		mmc->ocr_avail_mmc &= host->ocr_avail_mmc;

You will have to add a new quirk or callback e.g.

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 8d66706..b8a04e3 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2980,6 +2980,12 @@ int sdhci_add_host(struct sdhci_host *host)
                host->vmmc = NULL;
        }

+       if (host->ops->fixup_host) {
+               ret = host->ops->fixup_host(host);
+               if (ret)
+                       goto regput;
+       }
+
        sdhci_init(host, 0);

 #ifdef CONFIG_MMC_DEBUG
@@ -3012,6 +3018,9 @@ int sdhci_add_host(struct sdhci_host *host)

        return 0;

+regput:
+       if (host->vmmc)
+               regulator_put(host->vmmc);
 #ifdef SDHCI_USE_LEDS_CLASS
 reset:
        sdhci_reset(host, SDHCI_RESET_ALL);


Then implement ->fixup_host() to make the change.

Note that this is for I/O voltage or single voltage supply.  If there is a
need to control 2 separate voltages (e.g. VDD, VDDF or VCCQ, VCC as JEDEC
calls them) it is more challenging because mmc core does not support it.

> 
> Thank you,
> Kyungmin Park
>>
>>>>
>>>>>> 	- what is the contents of the OCR register?
>>>>> In sdhci_set_power(). it returns SDHCI_POWER_180, SDHCI_POWER_300, and
>>>>> SDHCI_POWER_330.
>>>>> In probe time, it return the SDHCI_POWER_330, and but fixed regulator
>>>>> returns the 2.8V so it calls regulator_set_voltage. and return -EINVAL
>>>>> since it's fixed regulator.
>>>>>
>>>>> Of course we can make workaround, set fixed regulator voltage as 3.3V.
>>>>> Even though actual voltage is 2.8V. so it's ambiguous.
>>>>>
>>>>> that's reason introduces the new quirk.
>>>>>
>>>>> Thank you,
>>>>> Kyungmin Park
>>>>>>
>>>>>>> Galaxy S2 also uses the same voltage as ours.
>>>>>>>
>>>>>>> I also think the modify the regulator framework to support voltage
>>>>>>> change at fixed regulator. but it's not good idea and doesn't match
>>>>>>> the regulator concept. so modify the sdhci codes to support our
>>>>>>> boards.
>>>>>>>
>>>>>>> Thank you,
>>>>>>> Kyungmin Park
>>>>>>>>
>>>>>>>>> try to change the voltage in core.c.
>>>>>>>>> When change the voltage, maybe use the regulator_set_voltage().
>>>>>>>>>
>>>>>>>>> In regulator_set_voltage(), check the below condition.
>>>>>>>>>
>>>>>>>>> 	/* sanity check */
>>>>>>>>> 	if (!rdev->desc->ops->set_voltage &&
>>>>>>>>> 	    !rdev->desc->ops->set_voltage_sel) {
>>>>>>>>> 		ret = -EINVAL;
>>>>>>>>> 		goto out;
>>>>>>>>> 	}
>>>>>>>>>
>>>>>>>>> If Some-board should use the fixed-regulator, always return -EINVAL.
>>>>>>>>> Then, eMMC didn't initialize always.
>>>>>>>>>
>>>>>>>>> So if use the fixed-regulator or etc, we need to add the
>>>>>>>>> MMC_CAP2_BROKEN_VOLTAGE.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>>>>>>>>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>>>>>>>>> ---
>>>>>>>>>  drivers/mmc/core/core.c  |    4 ++++
>>>>>>>>>  include/linux/mmc/host.h |    1 +
>>>>>>>>>  2 files changed, 5 insertions(+), 0 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>>>>>>>> index bec0bf2..6848789 100644
>>>>>>>>> --- a/drivers/mmc/core/core.c
>>>>>>>>> +++ b/drivers/mmc/core/core.c
>>>>>>>>> @@ -1121,6 +1121,10 @@ int mmc_regulator_set_ocr(struct mmc_host
>>>>>>>>> *mmc,
>>>>>>>>>  		 * might not allow this operation
>>>>>>>>>  		 */
>>>>>>>>>  		voltage = regulator_get_voltage(supply);
>>>>>>>>> +
>>>>>>>>> +		if (mmc->caps2 & MMC_CAP2_BROKEN_VOLTAGE)
>>>>>>>>> +			min_uV = max_uV = voltage;
>>>>>>>>> +
>>>>>>>>>  		if (voltage < 0)
>>>>>>>>>  			result = voltage;
>>>>>>>>>  		else if (voltage < min_uV || voltage > max_uV)
>>>>>>>>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>>>>>>>>> index dd13e05..5659aee 100644
>>>>>>>>> --- a/include/linux/mmc/host.h
>>>>>>>>> +++ b/include/linux/mmc/host.h
>>>>>>>>> @@ -257,6 +257,7 @@ struct mmc_host {
>>>>>>>>>  #define MMC_CAP2_HS200_1_2V_SDR	(1 << 6)        /* can support */
>>>>>>>>>  #define MMC_CAP2_HS200		(MMC_CAP2_HS200_1_8V_SDR | \
>>>>>>>>>  				 MMC_CAP2_HS200_1_2V_SDR)
>>>>>>>>> +#define MMC_CAP2_BROKEN_VOLTAGE	(1 << 7)	/* Use the broken voltage
>>>>>>>>> */
>>>>>>>>>
>>>>>>>>>  	mmc_pm_flag_t		pm_caps;	/* supported pm features */
>>>>>>>>>  	unsigned int        power_notify_type;
>>>>>>>>> --
>>>>>>>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc"
>>>>>>>>> in
>>>>>>>>> the body of a message to majordomo@vger.kernel.org
>>>>>>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>>>>>
>>>>>>>> --
>>>>>>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc"
>>>>>>>> in
>>>>>>>> the body of a message to majordomo@vger.kernel.org
>>>>>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>> --
>>>>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>>>>> the body of a message to majordomo@vger.kernel.org
>>>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>>>
>>>>>
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>