On Wed, Jan 18, 2012 at 5:56 PM, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
> On 18/01/12 10:13, Kyungmin Park wrote:
>> On 1/18/12, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
>>> On 18/01/12 01:58, Kyungmin Park wrote:
>>>> On 1/17/12, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
>>>>> On 17/01/12 11:53, Kyungmin Park wrote:
>>>>>> On 1/17/12, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
>>>>>>> On 17/01/12 11:05, Kyungmin Park wrote:
>>>>>>>> On 1/17/12, Adrian Hunter <adrian.hunter@xxxxxxxxx> 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;
Right, so it's meaningless set the host->ocr_avail_mmc. basically
sdhci doesn't support this voltage.
>
> You will have to add a new quirk or callback e.g.
That's reason add new quirk. MMC_CAP2_BROKEN_VOLTAGE.
>
> 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.
New H/W has different voltage for eMMC v4.5. but it's covered by PMIC.
It can be switches on/off by gpio then PMIC turn on/off each LDOs for
eMMC properly.
Thank you,
Kyungmin Park
>
>>
>> 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@xxxxxxxxxxx>
>>>>>>>>>> Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
>>>>>>>>>> ---
>>>>>>>>>> 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@xxxxxxxxxxxxxxx
>>>>>>>>>> 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@xxxxxxxxxxxxxxx
>>>>>>>>> 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@xxxxxxxxxxxxxxx
>>>>>>> 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@xxxxxxxxxxxxxxx
>>>>> 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@xxxxxxxxxxxxxxx
>>> 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@xxxxxxxxxxxxxxx
> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
