Re: [PATCH 02/12] mmc: sd: add support for signal voltage switch procedure

Philip Rakity
2011-03-03T22:51:02Z

Anath,

Is it sufficient to check the CAPS for voltage etc ? Now that we have a regulator framework do we need to also look at that ?

Philip

On Mar 3, 2011, at 1:08 PM, <subhashj@codeaurora.org> <subhashj@codeaurora.org> wrote:

>
>> -----Original Message-----
>> From: Nath, Arindam [mailto:Arindam.Nath@amd.com]
>> Sent: Thursday, March 03, 2011 7:05 PM
>> To: subhashj@codeaurora.org; cjb@laptop.org
>> Cc: linux-mmc@vger.kernel.org; Su, Henry; Lu, Aaron;
>> anath.amd@gmail.com
>> Subject: RE: [PATCH 02/12] mmc: sd: add support for signal voltage
>> switch procedure
>>
>> Hi Subhash,
>>
>>> -----Original Message-----
>>> From: subhashj@codeaurora.org [mailto:subhashj@codeaurora.org]
>>> Sent: Thursday, March 03, 2011 6:46 PM
>>> To: Nath, Arindam; cjb@laptop.org
>>> Cc: linux-mmc@vger.kernel.org; Su, Henry; Lu, Aaron;
>>> anath.amd@gmail.com
>>> Subject: RE: [PATCH 02/12] mmc: sd: add support for signal voltage
>>> switch procedure
>>>
>>>
>>>
>>>> -----Original Message-----
>>>> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
>>>> owner@vger.kernel.org] On Behalf Of Nath, Arindam
>>>> Sent: Wednesday, March 02, 2011 1:36 PM
>>>> To: subhashj@codeaurora.org; cjb@laptop.org
>>>> Cc: linux-mmc@vger.kernel.org; Su, Henry; Lu, Aaron;
>>>> anath.amd@gmail.com
>>>> Subject: RE: [PATCH 02/12] mmc: sd: add support for signal voltage
>>>> switch procedure
>>>>
>>>> Hi Subhash,
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: subhashj@codeaurora.org [mailto:subhashj@codeaurora.org]
>>>>> Sent: Tuesday, March 01, 2011 9:33 PM
>>>>> To: Nath, Arindam; cjb@laptop.org
>>>>> Cc: linux-mmc@vger.kernel.org; Su, Henry; Lu, Aaron;
>>>>> anath.amd@gmail.com
>>>>> Subject: RE: [PATCH 02/12] mmc: sd: add support for signal
>> voltage
>>>>> switch procedure
>>>>>
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
>>>>>> owner@vger.kernel.org] On Behalf Of Arindam Nath
>>>>>> Sent: Tuesday, February 15, 2011 3:05 PM
>>>>>> To: cjb@laptop.org
>>>>>> Cc: linux-mmc@vger.kernel.org; henry.su@amd.com;
>>> aaron.lu@amd.com;
>>>>>> anath.amd@gmail.com; Arindam Nath
>>>>>> Subject: [PATCH 02/12] mmc: sd: add support for signal voltage
>>>> switch
>>>>>> procedure
>>>>>>
>>>>>> Host Controller v3.00 adds another Capabilities register. We
>> can
>>>>>> directly read this register without any version checking since
>>> the
>>>>>> new register bits will be considred as reserved in older
>>>> controllers,
>>>>>> and will be read as 0. Apart from other things, this new
>> register
>>>>>> indicates whether the Host Controller supports SDR50, SDR104,
>> and
>>>>>> DDR50 UHS-I modes. So depending on the host support, we set
>>>>>> the corresponding MMC_CAP_* flags. One more new register. Host
>>>>> Control2
>>>>>> is added in v3.00, which is used during Signal Voltage Switch
>>>>> prcedure
>>>>>> described below.
>>>>>>
>>>>>> Since as per v3.00 spec, UHS-I supported hosts should set S18R
>> to
>>>> 1,
>>>>>> we set S18R (bit 24) of OCR before sending ACMD41. We also need
>>> to
>>>>> set
>>>>>> XPC (bit 28) of OCR in case the host can supply >150mA. This
>>>> support
>>>>> is
>>>>>> indicated by the Maximum Current Capabilities register of the
>>> Host
>>>>>> Controller.
>>>>>>
>>>>>> If the response of ACMD41 has both CCS and S18A set, we start
>> the
>>>>>> signal voltage switch procedure, which if successfull, will
>>> switch
>>>>>> the card from 3.3V signalling to 1.8V signalling. Signal
>> voltage
>>>>>> switch procedure adds support for a new command CMD11 in the
>>>> Physical
>>>>>> Layer Spec v3.01. As part of this procedure, we need to set
>> 1.8V
>>>>>> Signalling
>>>>>> Enable (bit 3) of Host Control2 register, which if remains set
>>>> after
>>>>>> 5ms,
>>>>>> means the switch to 1.8V signalling is successfull. Otherwise,
>> we
>>>>> clear
>>>>>> bit 24 of OCR and retry the initialization sequence.
>>>>>>
>>>>>> Signed-off-by: Arindam Nath <arindam.nath@amd.com>
>>>>>> ---
>>>>>> drivers/mmc/core/sd.c | 37 ++++++++++-
>>>>>> drivers/mmc/core/sd_ops.c | 25 ++++++++
>>>>>> drivers/mmc/core/sd_ops.h | 1 +
>>>>>> drivers/mmc/host/sdhci.c | 147
>>>>>> +++++++++++++++++++++++++++++++++++++++++----
>>>>>> drivers/mmc/host/sdhci.h | 18 +++++-
>>>>>> include/linux/mmc/host.h | 8 +++
>>>>>> include/linux/mmc/sd.h | 1 +
>>>>>> 7 files changed, 219 insertions(+), 18 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
>>>>>> index b3f8a3c..e968d5c 100644
>>>>>> --- a/drivers/mmc/core/sd.c
>>>>>> +++ b/drivers/mmc/core/sd.c
>>>>>> @@ -408,6 +408,7 @@ struct device_type sd_type = {
>>>>>> int mmc_sd_get_cid(struct mmc_host *host, u32 ocr, u32 *cid)
>>>>>> {
>>>>>> int err;
>>>>>> + u32 rocr;
>>>>>>
>>>>>> /*
>>>>>> * Since we're changing the OCR value, we seem to
>>>>>> @@ -427,10 +428,25 @@ int mmc_sd_get_cid(struct mmc_host *host,
>>> u32
>>>>>> ocr, u32 *cid)
>>>>>> if (!err)
>>>>>> ocr |= 1 << 30;
>>>>>>
>>>>>> - err = mmc_send_app_op_cond(host, ocr, NULL);
>>>>>> + /* If the host can supply more than 150mA, XPC should be
>> set
>>> to
>>>>>> 1. */
>>>>>> + if (host->caps & (MMC_CAP_SET_XPC_330 | MMC_CAP_SET_XPC_300
>> |
>>>>>> + MMC_CAP_SET_XPC_180))
>>>>>> + ocr |= 1 << 28;
>>>>>> +
>>>>>
>>>>> Why do you need MMC_CAP_SET_XPC_300 here? MMC_CAP_SET_XPC_330 &
>>>>> MMC_CAP_SET_XPC_180 is not enough?
>>>>
>>>> According to the Host Controller Spec v3.00, section 2.2.26:
>>>>
>>>> "If a Host System can afford more than 150mA, Host Driver set XPC
>> to
>>>> 1."
>>>>
>>>> Now since >150mA can be set for 1.8V, 3.0V and 3.3V, the above
>>>> condition checks for all these cases.
>>>>
>>>
>>> 3.0v and 3.3v is part of High voltage range supported by card (2.7v-
>>> 3.3v).
>>> Now if host supports any voltage from this range with 150ma driver
>>> strength
>>> then I think only config say MMC_CAP_SET_XPC_HV (high voltage) should
>>> be set
>>> by
>>> host and then only one check required here. Does it make sense?
>>
>> You have a valid point here. But I kept the configs *XPC_300 and
>> *XPC_330 separate so that in future if someone wants to base his/her
>> code on either of the flags being set, but not both, they can easily do
>> so. If you still think it is a good idea to have a common *CAP for both
>> of them, please let me know. I will modify the code accordingly.
>>
>
> Ok. it's fine. you keep them as 2 separate caps.
>
> Thanks,
> Subhash
>
>> Thanks,
>> Arindam
>
>
> --
> 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