From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755582AbcEXKQH (ORCPT ); Tue, 24 May 2016 06:16:07 -0400 Received: from mailout1.samsung.com ([203.254.224.24]:37878 "EHLO mailout1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752951AbcEXKQE (ORCPT ); Tue, 24 May 2016 06:16:04 -0400 MIME-version: 1.0 Content-type: text/plain; charset=utf-8 X-AuditID: cbfee690-f79056d000001327-a1-574429dfde5a Content-transfer-encoding: 8BIT Message-id: <574429DF.1000501@samsung.com> Date: Tue, 24 May 2016 19:15:59 +0900 From: Jaehoon Chung User-Agent: Mozilla/5.0 (X11; Linux i686; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 To: Shawn Lin , Ulf Hansson Cc: Adrian Hunter , Michal Simek , soren.brinkmann@xilinx.com, Rob Herring , linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org, Doug Anderson , Brian Norris , Heiko Stuebner , linux-rockchip@lists.infradead.org, devicetree@vger.kernel.org Subject: Re: [PATCH v5 3/5] mmc: core: implement enhanced strobe support References: <1463976789-29618-1-git-send-email-shawn.lin@rock-chips.com> <1463976835-29780-1-git-send-email-shawn.lin@rock-chips.com> <574425A3.6090303@samsung.com> <1f67fa2e-9a14-819d-8d75-3b35be819f0d@rock-chips.com> In-reply-to: <1f67fa2e-9a14-819d-8d75-3b35be819f0d@rock-chips.com> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrAIsWRmVeSWpSXmKPExsWyRsSkRPe+pku4weWJyhYnn6xhs9j08T2r xfwj51gtzi47yGbx/9FrVovLu+awWRz5389o8enBf2aLdy8jLFr3HmG3uPNkPavFzR2b2S2O rw134PWY3XCRxWPxnpdMHptWdbJ53Lm2h81j85J6j7+z9rN4bL82j9nj8yY5j72ff7MEcEZx 2aSk5mSWpRbp2yVwZRxduZ+x4HZcxf1/u1kaGFvduxg5OSQETCQ23PrHCGGLSVy4t56ti5GL Q0hgBaPEzukP2GGKPp/axQqRWMooMf/cd1aQBK+AoMSPyfdYuhg5OJgF5CWOXMqGMNUlpkzJ hSh/wChx7vlrRohyLYmryzuZQWpYBFQlGj7Lg4TZBHQktn87zgRiiwqESTxYtxdsuoiAn8TW HVvB7mEWaGOWOPRzAdgcYQEPiVsdfVD3bGGSmDvpHQtIglPASWLDnAYmkISEwEwOiQsHW5hB EiwCAhLfJh8CO1RCQFZi0wFmiMckJQ6uuMEygVFsFpJ3ZiG8MwvhnQWMzKsYRVMLkguKk9KL TPSKE3OLS/PS9ZLzczcxAiP69L9nE3Yw3jtgfYhRgINRiYc3IN85XIg1say4MvcQoynQDROZ pUST84FpI68k3tDYzMjC1MTU2Mjc0kxJnPe11M9gIYH0xJLU7NTUgtSi+KLSnNTiQ4xMHJxS DYyZ3VxpKzYwfL1be/6CbY2g7mZvvvRI9Wd/NzTeWCl6vkmv0OHvWUuj/bP+P124/9zWmTY7 m9yiNrsdYmY+cZJxwhNJ2U/sBt2VW2WkU5KCqh+aFLi3VevZ7M0UTSiQKjaJuLC7snp6w08V TfufQrFfz0de/rVTMn6TbJuHtOK0mh/XykKXTlBiKc5INNRiLipOBAD/X5GT4wIAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprLKsWRmVeSWpSXmKPExsVy+t9jAd37mi7hBldbJSxOPlnDZrHp43tW i/lHzrFanF12kM3i/6PXrBaXd81hszjyv5/R4tOD/8wW715GWLTuPcJucefJelaLmzs2s1sc XxvuwOsxu+Eii8fiPS+ZPDat6mTzuHNtD5vH5iX1Hn9n7Wfx2H5tHrPH501yHns//2YJ4Ixq YLTJSE1MSS1SSM1Lzk/JzEu3VfIOjneONzUzMNQ1tLQwV1LIS8xNtVVy8QnQdcvMATpbSaEs MacUKBSQWFyspG+HaUJoiJuuBUxjhK5vSBBcj5EBGkhYw5hxdOV+xoLbcRX3/+1maWBsde9i 5OSQEDCR+HxqFyuELSZx4d56ti5GLg4hgaWMEvPPfQdL8AoISvyYfI+li5GDg1lAXuLIpWwI U11iypRciPIHjBLnnr9mhCjXkri6vJMZpIZFQFWi4bM8SJhNQEdi+7fjTCC2qECYxIN1e8Gm iwj4SWzdsRVsLbNAG7PEoZ8LwOYIC3hI3OroY4VYsIVJYu6kdywgCU4BJ4kNcxqYJjAKzEJy 3iyE82YhnLeAkXkVo0RqQXJBcVJ6rlFearlecWJucWleul5yfu4mRnDSeCa9g/HwLvdDjAIc jEo8vAH5zuFCrIllxZW5hxglOJiVRHiLNVzChXhTEiurUovy44tKc1KLDzGaAv03kVlKNDkf mNDySuINjU3MjCyNzA0tjIzNlcR5H/9fFyYkkJ5YkpqdmlqQWgTTx8TBKdXAOO9tnoRWuNfM TbPfJbpauki+5tvp6lPYJMY1r/zuxptx5hVXXp9Ruftx9W6rmLT8SOv00mTGuAheoaX3iyaF BNfxXsv1rrSZa3tq/Z2at1sK90dI24sFBiY7i9689O1y66UVTs2Okw+L6x4KatNMeqUe/mq7 zwee/bcqpX4cWX/Pc/5V265qJZbijERDLeai4kQA+4ApbTADAAA= DLP-Filter: Pass X-MTR: 20000000000000000@CPGS X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/24/2016 07:11 PM, Shawn Lin wrote: > 在 2016/5/24 17:57, Jaehoon Chung 写道: >> On 05/23/2016 01:13 PM, Shawn Lin wrote: >>> Controllers use data strobe line to latch data from devices >>> under hs400 mode, but not for cmd line. So since emmc 5.1, JEDEC >>> introduces enhanced strobe mode for latching cmd response from >>> emmc devices to host controllers. This new feature is optional, >>> so it depends both on device's cap and host's cap to decide >>> whether to use it or not. >>> >>> Signed-off-by: Shawn Lin >>> --- >>> >>> Changes in v5: None >>> Changes in v4: None >>> Changes in v3: None >>> Changes in v2: None >>> >>> drivers/mmc/core/bus.c | 3 +- >>> drivers/mmc/core/core.c | 9 ++++++ >>> drivers/mmc/core/mmc.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++-- >>> include/linux/mmc/card.h | 1 + >>> include/linux/mmc/host.h | 11 +++++++ >>> include/linux/mmc/mmc.h | 3 ++ >>> 6 files changed, 108 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c >>> index 4bc48f1..c64266f 100644 >>> --- a/drivers/mmc/core/bus.c >>> +++ b/drivers/mmc/core/bus.c >>> @@ -332,12 +332,13 @@ int mmc_add_card(struct mmc_card *card) >>> mmc_card_ddr52(card) ? "DDR " : "", >>> type); >>> } else { >>> - pr_info("%s: new %s%s%s%s%s card at address %04x\n", >>> + pr_info("%s: new %s%s%s%s%s%s card at address %04x\n", >>> mmc_hostname(card->host), >>> mmc_card_uhs(card) ? "ultra high speed " : >>> (mmc_card_hs(card) ? "high speed " : ""), >>> mmc_card_hs400(card) ? "HS400 " : >>> (mmc_card_hs200(card) ? "HS200 " : ""), >>> + mmc_card_hs400es(card) ? "Enhanced strobe " : "", >>> mmc_card_ddr52(card) ? "DDR " : "", >>> uhs_bus_speed_mode, type, card->rca); >>> } >>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c >>> index 99275e4..b0c1f07 100644 >>> --- a/drivers/mmc/core/core.c >>> +++ b/drivers/mmc/core/core.c >>> @@ -1127,6 +1127,15 @@ void mmc_set_initial_state(struct mmc_host *host) >>> host->ios.bus_width = MMC_BUS_WIDTH_1; >>> host->ios.timing = MMC_TIMING_LEGACY; >>> host->ios.drv_type = 0; >>> + host->ios.enhanced_strobe = false; >>> + >>> + /* >>> + * Make sure we are in non-enhanced strobe mode before we >>> + * actually enable it in ext_csd. >>> + */ >>> + if ((host->caps2 & MMC_CAP2_HS400_ES) && >>> + host->ops->hs400_enhanced_strobe) >>> + host->ops->hs400_enhanced_strobe(host, &host->ios); >> >> >> I wonder what is enhanced_strobe's purpose. >> When i saw your patches, this is used to check whether timing is strobe or not in sdhci-of-arasan, right? > > > Also we use it for debugfs. > >> If ios.enhanced_strobe is used only this purpose, is it possible to use the below? >> >> host->ops->hs400_enhanced_strobe(host, false/true); ? >> >> This callback is also called only two locations..init (false) and switch hs400es (true). > > It was suggested by Adrian. I'm ok with both of them. I don't mind what using.. :) It's your preference. > >> >>> >>> mmc_set_ios(host); >>> } >>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c >>> index f99c47e..9dc91ba 100644 >>> --- a/drivers/mmc/core/mmc.c >>> +++ b/drivers/mmc/core/mmc.c >>> @@ -235,6 +235,11 @@ static void mmc_select_card_type(struct mmc_card *card) >>> avail_type |= EXT_CSD_CARD_TYPE_HS400_1_2V; >>> } >>> >>> + if ((caps2 & MMC_CAP2_HS400_ES) && >>> + card->ext_csd.strobe_support && >>> + (avail_type & EXT_CSD_CARD_TYPE_HS400)) >>> + avail_type |= EXT_CSD_CARD_TYPE_HS400ES; >>> + >>> card->ext_csd.hs_max_dtr = hs_max_dtr; >>> card->ext_csd.hs200_max_dtr = hs200_max_dtr; >>> card->mmc_avail_type = avail_type; >>> @@ -383,6 +388,7 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd) >>> mmc_card_set_blockaddr(card); >>> } >>> >>> + card->ext_csd.strobe_support = ext_csd[EXT_CSD_STROBE_SUPPORT]; >>> card->ext_csd.raw_card_type = ext_csd[EXT_CSD_CARD_TYPE]; >>> mmc_select_card_type(card); >>> >>> @@ -1216,6 +1222,78 @@ out_err: >>> return err; >>> } >>> >>> +static int mmc_select_hs400es(struct mmc_card *card) >>> +{ >>> + struct mmc_host *host = card->host; >>> + int err = 0; >>> + u8 val; >>> + >>> + if (!(host->caps & MMC_CAP_8_BIT_DATA)) { >>> + err = -ENOTSUPP; >>> + goto out_err; >>> + } >>> + >>> + err = mmc_select_bus_width(card); >>> + if (IS_ERR_VALUE(err)) >>> + goto out_err; >>> + >>> + /* Switch card to HS mode */ >>> + err = mmc_select_hs(card); >>> + if (err) { >>> + pr_err("%s: switch to high-speed failed, err:%d\n", >>> + mmc_hostname(host), err); >>> + goto out_err; >>> + } >>> + >>> + err = mmc_switch_status(card); >>> + if (err) >>> + goto out_err; >>> + >>> + /* Switch card to DDR with strobe bit */ >>> + val = EXT_CSD_DDR_BUS_WIDTH_8 | EXT_CSD_BUS_WIDTH_STROBE; >>> + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, >>> + EXT_CSD_BUS_WIDTH, >>> + val, >>> + card->ext_csd.generic_cmd6_time); >>> + if (err) { >>> + pr_err("%s: switch to bus width for hs400es failed, err:%d\n", >>> + mmc_hostname(host), err); >>> + goto out_err; >>> + } >>> + >>> + /* Switch card to HS400 */ >>> + val = EXT_CSD_TIMING_HS400 | >>> + card->drive_strength << EXT_CSD_DRV_STR_SHIFT; >>> + err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, >>> + EXT_CSD_HS_TIMING, val, >>> + card->ext_csd.generic_cmd6_time, >>> + true, false, true); >>> + if (err) { >>> + pr_err("%s: switch to hs400es failed, err:%d\n", >>> + mmc_hostname(host), err); >>> + goto out_err; >>> + } >>> + >>> + /* Set host controller to HS400 timing and frequency */ >>> + mmc_set_timing(host, MMC_TIMING_MMC_HS400); >>> + >>> + /* Controller enable enhanced strobe function */ >>> + host->ios.enhanced_strobe = true; >>> + if (host->ops->hs400_enhanced_strobe) >>> + host->ops->hs400_enhanced_strobe(host, &host->ios); >>> + >>> + err = mmc_switch_status(card); >>> + if (err) >>> + goto out_err; >>> + >>> + return 0; >>> + >>> +out_err: >> >> If ios.ehanced_strobe is really needs, you need to set from true to false at here? > > I don't think so. If failing to run into HS400es, I'd not expect it > fallback to HS400, and why not let mmc core retring it. We currently > don't have speed-mode fallback mechanism, right? Ok. I have tested with my exynos board. It's working fine. Tested-by: Jaehoon Chung Reviewed-by: Jaehoon Chung Best Regards, Jaehoon Chung > >> >> Best Regards, >> Jaehoon Chung >> >>> + pr_err("%s: %s failed, error %d\n", mmc_hostname(card->host), >>> + __func__, err); >>> + return err; >>> +} >>> + >>> static void mmc_select_driver_type(struct mmc_card *card) >>> { >>> int card_drv_type, drive_strength, drv_type; >>> @@ -1297,7 +1375,7 @@ err: >>> } >>> >>> /* >>> - * Activate High Speed or HS200 mode if supported. >>> + * Activate High Speed, HS200 or HS400ES mode if supported. >>> */ >>> static int mmc_select_timing(struct mmc_card *card) >>> { >>> @@ -1306,7 +1384,9 @@ static int mmc_select_timing(struct mmc_card *card) >>> if (!mmc_can_ext_csd(card)) >>> goto bus_speed; >>> >>> - if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS200) >>> + if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS400ES) >>> + err = mmc_select_hs400es(card); >>> + else if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS200) >>> err = mmc_select_hs200(card); >>> else if (card->mmc_avail_type & EXT_CSD_CARD_TYPE_HS) >>> err = mmc_select_hs(card); >>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h >>> index eb0151b..22defc2 100644 >>> --- a/include/linux/mmc/card.h >>> +++ b/include/linux/mmc/card.h >>> @@ -95,6 +95,7 @@ struct mmc_ext_csd { >>> u8 raw_partition_support; /* 160 */ >>> u8 raw_rpmb_size_mult; /* 168 */ >>> u8 raw_erased_mem_count; /* 181 */ >>> + u8 strobe_support; /* 184 */ >>> u8 raw_ext_csd_structure; /* 194 */ >>> u8 raw_card_type; /* 196 */ >>> u8 raw_driver_strength; /* 197 */ >>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h >>> index 2a06fb0..0eabb72 100644 >>> --- a/include/linux/mmc/host.h >>> +++ b/include/linux/mmc/host.h >>> @@ -19,6 +19,7 @@ >>> >>> #include >>> #include >>> +#include >>> #include >>> >>> struct mmc_ios { >>> @@ -77,6 +78,8 @@ struct mmc_ios { >>> #define MMC_SET_DRIVER_TYPE_A 1 >>> #define MMC_SET_DRIVER_TYPE_C 2 >>> #define MMC_SET_DRIVER_TYPE_D 3 >>> + >>> + bool enhanced_strobe; /* hs400es selection */ >>> }; >>> >>> struct mmc_host_ops { >>> @@ -143,6 +146,9 @@ struct mmc_host_ops { >>> >>> /* Prepare HS400 target operating frequency depending host driver */ >>> int (*prepare_hs400_tuning)(struct mmc_host *host, struct mmc_ios *ios); >>> + /* Prepare enhanced strobe depending host driver */ >>> + void (*hs400_enhanced_strobe)(struct mmc_host *host, >>> + struct mmc_ios *ios); >>> int (*select_drive_strength)(struct mmc_card *card, >>> unsigned int max_dtr, int host_drv, >>> int card_drv, int *drv_type); >>> @@ -513,6 +519,11 @@ static inline bool mmc_card_hs400(struct mmc_card *card) >>> return card->host->ios.timing == MMC_TIMING_MMC_HS400; >>> } >>> >>> +static inline bool mmc_card_hs400es(struct mmc_card *card) >>> +{ >>> + return card->host->ios.enhanced_strobe; >>> +} >>> + >>> void mmc_retune_timer_stop(struct mmc_host *host); >>> >>> static inline void mmc_retune_needed(struct mmc_host *host) >>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h >>> index 15f2c4a..c376209 100644 >>> --- a/include/linux/mmc/mmc.h >>> +++ b/include/linux/mmc/mmc.h >>> @@ -297,6 +297,7 @@ struct _mmc_csd { >>> #define EXT_CSD_PART_CONFIG 179 /* R/W */ >>> #define EXT_CSD_ERASED_MEM_CONT 181 /* RO */ >>> #define EXT_CSD_BUS_WIDTH 183 /* R/W */ >>> +#define EXT_CSD_STROBE_SUPPORT 184 /* RO */ >>> #define EXT_CSD_HS_TIMING 185 /* R/W */ >>> #define EXT_CSD_POWER_CLASS 187 /* R/W */ >>> #define EXT_CSD_REV 192 /* RO */ >>> @@ -380,12 +381,14 @@ struct _mmc_csd { >>> #define EXT_CSD_CARD_TYPE_HS400_1_2V (1<<7) /* Card can run at 200MHz DDR, 1.2V */ >>> #define EXT_CSD_CARD_TYPE_HS400 (EXT_CSD_CARD_TYPE_HS400_1_8V | \ >>> EXT_CSD_CARD_TYPE_HS400_1_2V) >>> +#define EXT_CSD_CARD_TYPE_HS400ES (1<<8) /* Card can run at HS400ES */ >>> >>> #define EXT_CSD_BUS_WIDTH_1 0 /* Card is in 1 bit mode */ >>> #define EXT_CSD_BUS_WIDTH_4 1 /* Card is in 4 bit mode */ >>> #define EXT_CSD_BUS_WIDTH_8 2 /* Card is in 8 bit mode */ >>> #define EXT_CSD_DDR_BUS_WIDTH_4 5 /* Card is in 4 bit DDR mode */ >>> #define EXT_CSD_DDR_BUS_WIDTH_8 6 /* Card is in 8 bit DDR mode */ >>> +#define EXT_CSD_BUS_WIDTH_STROBE BIT(7) /* Enhanced strobe mode */ >>> >>> #define EXT_CSD_TIMING_BC 0 /* Backwards compatility */ >>> #define EXT_CSD_TIMING_HS 1 /* High speed */ >>> >> >> >> >> > >