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; 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@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