On 03/13/2014 05:37 PM, Seungwon Jeon wrote:
> On Thu, March 13, 2014, Jaehoon Chung wrote:
>> Dear, Seungwon.
>>
>> On 03/10/2014 08:59 PM, Seungwon Jeon wrote:
>>> On Mon, March 10, 2014, Jaehoon Chung wrote:
>>>> On 03/07/2014 11:36 PM, Seungwon Jeon wrote:
>>>>> Device types which are supported by both host and device
>>>>> can be identified when EXT_CSD is read. There is no need to
>>>>> check host's capability anymore.
>>>>>
>>>>> Signed-off-by: Seungwon Jeon <tgih.jun@xxxxxxxxxxx>
>>>>> ---
>>>>> Changes in v2:
>>>>> Just rebased with latest one.
>>>>>
>>>>> drivers/mmc/core/mmc.c | 77 ++++++++++++++++++++++++++-------------------
>>>>> include/linux/mmc/card.h | 6 ++-
>>>>> include/linux/mmc/host.h | 6 ---
>>>>> include/linux/mmc/mmc.h | 12 +++++--
>>>>> 4 files changed, 56 insertions(+), 45 deletions(-)
>>>>>
>>>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>>>> index db9655f..0abece0 100644
>>>>> --- a/drivers/mmc/core/mmc.c
>>>>> +++ b/drivers/mmc/core/mmc.c
>>>>> @@ -243,28 +243,46 @@ static void mmc_select_card_type(struct mmc_card *card)
>>>>> u8 card_type = card->ext_csd.raw_card_type & EXT_CSD_CARD_TYPE_MASK;
>>>>> u32 caps = host->caps, caps2 = host->caps2;
>>>>> unsigned int hs_max_dtr = 0;
>>>>> + unsigned int avail_type = 0;
>>>>>
>>>>> - if (card_type & EXT_CSD_CARD_TYPE_26)
>>>>> + if (caps & MMC_CAP_MMC_HIGHSPEED &&
>>>>> + card_type & EXT_CSD_CARD_TYPE_HS_26) {
>>>>> hs_max_dtr = MMC_HIGH_26_MAX_DTR;
>>>>> + avail_type |= EXT_CSD_CARD_TYPE_HS_26;
>>>>> + }
>>>>>
>>>>> if (caps & MMC_CAP_MMC_HIGHSPEED &&
>>>>> - card_type & EXT_CSD_CARD_TYPE_52)
>>>>> + card_type & EXT_CSD_CARD_TYPE_HS_52) {
>>>>> hs_max_dtr = MMC_HIGH_52_MAX_DTR;
>>>>> + avail_type |= EXT_CSD_CARD_TYPE_HS_52;
>>>>> + }
>>>> Can it bind with "caps & MMC_CAP_MMC_HIGHSPEED"?
>>> Yes, 'nested if' may be possible here.
>>> I just think current style is more harmonious though.
>> Don't mind. it's ok. :)
>>
>>>
>>>> if (caps & MMC_CAP_MMC_HIGH_SPEED) {
>>>> if (card_type & EXT_CSD_CARD_TYPE_HS_26) {
>>>> ...
>>>> }
>>>> if (card_type & EXT_CSD_CARD_TYPE_HS_52) {
>>>> ...
>>>> }
>>>> }
>>>>>
>>>>> - if ((caps & MMC_CAP_1_8V_DDR &&
>>>>> - card_type & EXT_CSD_CARD_TYPE_DDR_1_8V) ||
>>>>> - (caps & MMC_CAP_1_2V_DDR &&
>>>>> - card_type & EXT_CSD_CARD_TYPE_DDR_1_2V))
>>>>> + if (caps & MMC_CAP_1_8V_DDR &&
>>>>> + card_type & EXT_CSD_CARD_TYPE_DDR_1_8V) {
>>>>> hs_max_dtr = MMC_HIGH_DDR_MAX_DTR;
>>>>> + avail_type |= EXT_CSD_CARD_TYPE_DDR_1_8V;
>>>>> + }
>>>>>
>>>>> - if ((caps2 & MMC_CAP2_HS200_1_8V_SDR &&
>>>>> - card_type & EXT_CSD_CARD_TYPE_SDR_1_8V) ||
>>>>> - (caps2 & MMC_CAP2_HS200_1_2V_SDR &&
>>>>> - card_type & EXT_CSD_CARD_TYPE_SDR_1_2V))
>>>>> + if (caps & MMC_CAP_1_2V_DDR &&
>>>>> + card_type & EXT_CSD_CARD_TYPE_DDR_1_2V) {
>>>>> + hs_max_dtr = MMC_HIGH_DDR_MAX_DTR;
>>>>> + avail_type |= EXT_CSD_CARD_TYPE_DDR_1_2V;
>>>>> + }
>>>>> +
>>>>> + if (caps2 & MMC_CAP2_HS200_1_8V_SDR &&
>>>>> + card_type & EXT_CSD_CARD_TYPE_HS200_1_8V) {
>>>>> hs_max_dtr = MMC_HS200_MAX_DTR;
>>>>> + avail_type |= EXT_CSD_CARD_TYPE_HS200_1_8V;
>>>>> + }
>>>>> +
>>>>> + if (caps2 & MMC_CAP2_HS200_1_2V_SDR &&
>>>>> + card_type & EXT_CSD_CARD_TYPE_HS200_1_2V) {
>>>>> + hs_max_dtr = MMC_HS200_MAX_DTR;
>>>>> + avail_type |= EXT_CSD_CARD_TYPE_HS200_1_2V;
>>>>> + }
>>>>>
>>>>> card->ext_csd.hs_max_dtr = hs_max_dtr;
>>>>> - card->ext_csd.card_type = card_type;
>>>>> + card->mmc_avail_type = avail_type;
>>>>> }
>>>>>
>>>>> /*
>>>>> @@ -708,6 +726,11 @@ static struct device_type mmc_type = {
>>>>> .groups = mmc_attr_groups,
>>>>> };
>>>>>
>>>>> +static inline unsigned int mmc_snoop_ddr(struct mmc_card *card)
>>>>> +{
>>>>> + return card->mmc_avail_type & EXT_CSD_CARD_TYPE_DDR_52;
>>>>> +}
>>>>> +
>>>>> /*
>>>>> * Select the PowerClass for the current bus width
>>>>> * If power class is defined for 4/8 bit bus in the
>>>>> @@ -808,12 +831,10 @@ static int mmc_select_hs200(struct mmc_card *card)
>>>>>
>>>>> host = card->host;
>>>>>
>>>>> - if (card->ext_csd.card_type & EXT_CSD_CARD_TYPE_SDR_1_2V &&
>>>>> - host->caps2 & MMC_CAP2_HS200_1_2V_SDR)
>>>>> + if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS200_1_2V)
>>>>> err = __mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_120);
>>>>>
>>>>> - if (err && card->ext_csd.card_type & EXT_CSD_CARD_TYPE_SDR_1_8V &&
>>>>> - host->caps2 & MMC_CAP2_HS200_1_8V_SDR)
>>>>> + if (err && card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS200_1_8V)
>>>>> err = __mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_180);
>>>>>
>>>>> /* If fails try again during next card power cycle */
>>>>> @@ -1072,10 +1093,9 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>>>>> */
>>>>> if (card->ext_csd.hs_max_dtr != 0) {
>>>>> err = 0;
>>>>> - if (card->ext_csd.hs_max_dtr > 52000000 &&
>>>>> - host->caps2 & MMC_CAP2_HS200)
>>>> MMC_CAP2_HS200 need no more, doesn't?
>>> If you mean to remove 'MMC_CAP2_HS200' definition, it is currently used in sdhci.c
>> I have also checked that it is used in sdhci.c.
>> but I think that capability like MMC_CAP2_HS200 was defined to use for core, not controller.
>> It can be changed other quirks instead of MMC_CAP2_HS200 into sdhci.c
>> how about?
> Hmm.
> I feel like current way is not bad in sdhci.c, but if you have an idea, it would be different patch.
Sure..Then when your patch-set is merged, i will send the other patch.
Best Regards,
Jaehoon Chung
>
> Thanks,
> Seungwon Jeon
>
> --
> 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
