* Re: [PATCH V7 12/25] mmc: block: Disable Command Queue while RPMB is used
From: Ritesh Harjani @ 2016-11-28 4:46 UTC (permalink / raw)
To: Adrian Hunter
Cc: Ulf Hansson, linux-mmc, Alex Lemberg, Mateusz Nowak,
Yuliy Izrailov, Jaehoon Chung, Dong Aisheng, Das Asutosh,
Zhangfei Gao, Dorfman Konstantin, David Griego, Sahitya Tummala,
Venu Byravarasu, Linus Walleij
In-Reply-To: <1480068442-5169-13-git-send-email-adrian.hunter@intel.com>
On 11/25/2016 3:37 PM, Adrian Hunter wrote:
> RPMB does not allow Command Queue commands. Disable and re-enable the
> Command Queue when switching.
>
> Note that the driver only switches partitions when the queue is empty.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Minor comment.
Reviewed-by: Harjani Ritesh <riteshh@codeaurora.org>
Regards
Ritesh
> ---
> drivers/mmc/card/block.c | 46 ++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 38 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index f22df69823cc..157d1b3d58d6 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -746,10 +746,41 @@ static int mmc_blk_compat_ioctl(struct block_device *bdev, fmode_t mode,
> #endif
> };
>
> +static int mmc_blk_part_switch_pre(struct mmc_card *card,
> + unsigned int part_type)
> +{
> + int ret;
int ret = 0;
> +
> + if (part_type == EXT_CSD_PART_CONFIG_ACC_RPMB) {
> + if (card->ext_csd.cmdq_en) {
> + ret = mmc_cmdq_disable(card);
> + if (ret)
> + return ret;
> + }
> + mmc_retune_pause(card->host);
> + }
> +
> + return 0;
Here return ret;
> +}
> +
> +static int mmc_blk_part_switch_post(struct mmc_card *card,
> + unsigned int part_type)
> +{
> + int ret = 0;
> +
> + if (part_type == EXT_CSD_PART_CONFIG_ACC_RPMB) {
> + mmc_retune_unpause(card->host);
> + if (card->reenable_cmdq && !card->ext_csd.cmdq_en)
> + ret = mmc_cmdq_enable(card);
> + }
> +
> + return ret;
> +}
> +
> static inline int mmc_blk_part_switch(struct mmc_card *card,
> struct mmc_blk_data *md)
> {
> - int ret;
> + int ret = 0;
> struct mmc_blk_data *main_md = dev_get_drvdata(&card->dev);
>
> if (main_md->part_curr == md->part_type)
> @@ -758,8 +789,9 @@ static inline int mmc_blk_part_switch(struct mmc_card *card,
> if (mmc_card_mmc(card)) {
> u8 part_config = card->ext_csd.part_config;
>
> - if (md->part_type == EXT_CSD_PART_CONFIG_ACC_RPMB)
> - mmc_retune_pause(card->host);
> + ret = mmc_blk_part_switch_pre(card, md->part_type);
> + if (ret)
> + return ret;
>
> part_config &= ~EXT_CSD_PART_CONFIG_ACC_MASK;
> part_config |= md->part_type;
> @@ -768,19 +800,17 @@ static inline int mmc_blk_part_switch(struct mmc_card *card,
> EXT_CSD_PART_CONFIG, part_config,
> card->ext_csd.part_time);
> if (ret) {
> - if (md->part_type == EXT_CSD_PART_CONFIG_ACC_RPMB)
> - mmc_retune_unpause(card->host);
> + mmc_blk_part_switch_post(card, md->part_type);
> return ret;
> }
>
> card->ext_csd.part_config = part_config;
>
> - if (main_md->part_curr == EXT_CSD_PART_CONFIG_ACC_RPMB)
> - mmc_retune_unpause(card->host);
> + ret = mmc_blk_part_switch_post(card, main_md->part_curr);
> }
>
> main_md->part_curr = md->part_type;
> - return 0;
> + return ret;
> }
>
> static u32 mmc_sd_num_wr_blocks(struct mmc_card *card)
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH V7 11/25] mmc: mmc_test: Disable Command Queue while mmc_test is used
From: Ritesh Harjani @ 2016-11-28 4:40 UTC (permalink / raw)
To: Adrian Hunter
Cc: Ulf Hansson, linux-mmc, Alex Lemberg, Mateusz Nowak,
Yuliy Izrailov, Jaehoon Chung, Dong Aisheng, Das Asutosh,
Zhangfei Gao, Dorfman Konstantin, David Griego, Sahitya Tummala,
Venu Byravarasu, Linus Walleij
In-Reply-To: <1480068442-5169-12-git-send-email-adrian.hunter@intel.com>
On 11/25/2016 3:37 PM, Adrian Hunter wrote:
> Normal read and write commands may not be used while the command queue is
> enabled. Disable the Command Queue when mmc_test is probed and re-enable it
> when it is removed.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Looks good!
Maybe we can later add SW CMDQ test cases to mmc_test framework.
Reviewed-by: Harjani Ritesh <riteshh@codeaurora.org>
Regards
Ritesh
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH V7 10/25] mmc: mmc: Add functions to enable / disable the Command Queue
From: Ritesh Harjani @ 2016-11-28 4:36 UTC (permalink / raw)
To: Adrian Hunter
Cc: Ulf Hansson, linux-mmc, Alex Lemberg, Mateusz Nowak,
Yuliy Izrailov, Jaehoon Chung, Dong Aisheng, Das Asutosh,
Zhangfei Gao, Dorfman Konstantin, David Griego, Sahitya Tummala,
Venu Byravarasu, Linus Walleij
In-Reply-To: <1480068442-5169-11-git-send-email-adrian.hunter@intel.com>
On 11/25/2016 3:37 PM, Adrian Hunter wrote:
> Add helper functions to enable or disable the Command Queue.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> Documentation/mmc/mmc-dev-attrs.txt | 1 +
> drivers/mmc/core/mmc.c | 2 ++
> drivers/mmc/core/mmc_ops.c | 27 +++++++++++++++++++++++++++
> include/linux/mmc/card.h | 1 +
> include/linux/mmc/core.h | 2 ++
> 5 files changed, 33 insertions(+)
>
> diff --git a/Documentation/mmc/mmc-dev-attrs.txt b/Documentation/mmc/mmc-dev-attrs.txt
> index 404a0e9e92b0..dcd1252877fb 100644
> --- a/Documentation/mmc/mmc-dev-attrs.txt
> +++ b/Documentation/mmc/mmc-dev-attrs.txt
> @@ -30,6 +30,7 @@ All attributes are read-only.
> rel_sectors Reliable write sector count
> ocr Operation Conditions Register
> dsr Driver Stage Register
> + cmdq_en Command Queue enabled: 1 => enabled, 0 => not enabled
>
> Note on Erase Size and Preferred Erase Size:
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 6e9830997eef..d6a30bbd399d 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -770,6 +770,7 @@ static int mmc_compare_ext_csds(struct mmc_card *card, unsigned bus_width)
> MMC_DEV_ATTR(raw_rpmb_size_mult, "%#x\n", card->ext_csd.raw_rpmb_size_mult);
> MMC_DEV_ATTR(rel_sectors, "%#x\n", card->ext_csd.rel_sectors);
> MMC_DEV_ATTR(ocr, "%08x\n", card->ocr);
> +MMC_DEV_ATTR(cmdq_en, "%d\n", card->ext_csd.cmdq_en);
>
> static ssize_t mmc_fwrev_show(struct device *dev,
> struct device_attribute *attr,
> @@ -823,6 +824,7 @@ static ssize_t mmc_dsr_show(struct device *dev,
> &dev_attr_rel_sectors.attr,
> &dev_attr_ocr.attr,
> &dev_attr_dsr.attr,
> + &dev_attr_cmdq_en.attr,
> NULL,
> };
> ATTRIBUTE_GROUPS(mmc_std);
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index 9b2617cfff67..92a1de9b4981 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -824,3 +824,30 @@ int mmc_can_ext_csd(struct mmc_card *card)
> {
> return (card && card->csd.mmca_vsn > CSD_SPEC_VER_3);
> }
> +
> +int mmc_cmdq_switch(struct mmc_card *card, int enable)
> +{
> + int err;
> +
> + if (!card->ext_csd.cmdq_support)
> + return -EOPNOTSUPP;
> +
> + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_CMDQ_MODE_EN,
> + enable, card->ext_csd.generic_cmd6_time);
> + if (!err)
> + card->ext_csd.cmdq_en = enable;
> +
> + return err;
> +}
> +
> +int mmc_cmdq_enable(struct mmc_card *card)
> +{
> + return mmc_cmdq_switch(card, EXT_CSD_CMDQ_MODE_ENABLED);
EXT_CSD_CMDQ_MODE_ENABLED is defined as a BIT(0), but getting used here
as a value of 1. EXT_CSD_CMDQ_MODE_ENABLED seems redundant anyways.
Do you think we can remove it and pass 1 directly?
> +}
> +EXPORT_SYMBOL_GPL(mmc_cmdq_enable);
> +
> +int mmc_cmdq_disable(struct mmc_card *card)
> +{
> + return mmc_cmdq_switch(card, 0);
> +}
> +EXPORT_SYMBOL_GPL(mmc_cmdq_disable);
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index 95d69d498296..2d9c24f4e88e 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -89,6 +89,7 @@ struct mmc_ext_csd {
> unsigned int boot_ro_lock; /* ro lock support */
> bool boot_ro_lockable;
> bool ffu_capable; /* Firmware upgrade support */
> + bool cmdq_en; /* Command Queue enabled */
> bool cmdq_support; /* Command Queue supported */
> unsigned int cmdq_depth; /* Command Queue depth */
> #define MMC_FIRMWARE_LEN 8
> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
> index 0ce928b3ce90..d045b06fc7ea 100644
> --- a/include/linux/mmc/core.h
> +++ b/include/linux/mmc/core.h
> @@ -177,6 +177,8 @@ extern int mmc_wait_for_app_cmd(struct mmc_host *, struct mmc_card *,
> extern int mmc_switch(struct mmc_card *, u8, u8, u8, unsigned int);
> extern int mmc_send_tuning(struct mmc_host *host, u32 opcode, int *cmd_error);
> extern int mmc_get_ext_csd(struct mmc_card *card, u8 **new_ext_csd);
> +extern int mmc_cmdq_enable(struct mmc_card *card);
> +extern int mmc_cmdq_disable(struct mmc_card *card);
>
> #define MMC_ERASE_ARG 0x00000000
> #define MMC_SECURE_ERASE_ARG 0x80000000
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH V7 09/25] mmc: mmc: Add Command Queue definitions
From: Ritesh Harjani @ 2016-11-28 4:29 UTC (permalink / raw)
To: Adrian Hunter
Cc: Ulf Hansson, linux-mmc, Alex Lemberg, Mateusz Nowak,
Yuliy Izrailov, Jaehoon Chung, Dong Aisheng, Das Asutosh,
Zhangfei Gao, Dorfman Konstantin, David Griego, Sahitya Tummala,
Venu Byravarasu, Linus Walleij
In-Reply-To: <1480068442-5169-10-git-send-email-adrian.hunter@intel.com>
On 11/25/2016 3:37 PM, Adrian Hunter wrote:
> Add definitions relating to Command Queuing.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> drivers/mmc/core/mmc.c | 17 +++++++++++++++++
> include/linux/mmc/card.h | 2 ++
> include/linux/mmc/mmc.h | 17 +++++++++++++++++
> 3 files changed, 36 insertions(+)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 3268fcd3378d..6e9830997eef 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -618,6 +618,23 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8 *ext_csd)
> (ext_csd[EXT_CSD_SUPPORTED_MODE] & 0x1) &&
> !(ext_csd[EXT_CSD_FW_CONFIG] & 0x1);
> }
> +
> + /* eMMC v5.1 or later */
> + if (card->ext_csd.rev >= 8) {
> + card->ext_csd.cmdq_support = ext_csd[EXT_CSD_CMDQ_SUPPORT] &
> + EXT_CSD_CMDQ_SUPPORTED;
> + card->ext_csd.cmdq_depth = (ext_csd[EXT_CSD_CMDQ_DEPTH] &
> + EXT_CSD_CMDQ_DEPTH_MASK) + 1;
> + if (card->ext_csd.cmdq_depth <= 2) {
> + card->ext_csd.cmdq_support = false;
> + card->ext_csd.cmdq_depth = 0;
> + }
Could you please explain, why for cmdq_depth <=2, we are disabling
cmdq_support ? Maybe we can add a comment there.
> + if (card->ext_csd.cmdq_support) {
> + pr_debug("%s: Command Queue supported depth %u\n",
> + mmc_hostname(card->host),
> + card->ext_csd.cmdq_depth);
> + }
> + }
> out:
> return err;
> }
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index e49a3ff9d0e0..95d69d498296 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -89,6 +89,8 @@ struct mmc_ext_csd {
> unsigned int boot_ro_lock; /* ro lock support */
> bool boot_ro_lockable;
> bool ffu_capable; /* Firmware upgrade support */
> + bool cmdq_support; /* Command Queue supported */
> + unsigned int cmdq_depth; /* Command Queue depth */
> #define MMC_FIRMWARE_LEN 8
> u8 fwrev[MMC_FIRMWARE_LEN]; /* FW version */
> u8 raw_exception_status; /* 54 */
> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
> index c376209c70ef..672730acc705 100644
> --- a/include/linux/mmc/mmc.h
> +++ b/include/linux/mmc/mmc.h
> @@ -84,6 +84,13 @@
> #define MMC_APP_CMD 55 /* ac [31:16] RCA R1 */
> #define MMC_GEN_CMD 56 /* adtc [0] RD/WR R1 */
>
> + /* class 11 */
> +#define MMC_QUE_TASK_PARAMS 44 /* ac [20:16] task id R1 */
> +#define MMC_QUE_TASK_ADDR 45 /* ac [31:0] data addr R1 */
> +#define MMC_EXECUTE_READ_TASK 46 /* adtc [20:16] task id R1 */
> +#define MMC_EXECUTE_WRITE_TASK 47 /* adtc [20:16] task id R1 */
> +#define MMC_CMDQ_TASK_MGMT 48 /* ac [20:16] task id R1b */
> +
> static inline bool mmc_op_multi(u32 opcode)
> {
> return opcode == MMC_WRITE_MULTIPLE_BLOCK ||
> @@ -272,6 +279,7 @@ struct _mmc_csd {
> * EXT_CSD fields
> */
>
> +#define EXT_CSD_CMDQ_MODE_EN 15 /* R/W */
> #define EXT_CSD_FLUSH_CACHE 32 /* W */
> #define EXT_CSD_CACHE_CTRL 33 /* R/W */
> #define EXT_CSD_POWER_OFF_NOTIFICATION 34 /* R/W */
> @@ -331,6 +339,8 @@ struct _mmc_csd {
> #define EXT_CSD_CACHE_SIZE 249 /* RO, 4 bytes */
> #define EXT_CSD_PWR_CL_DDR_200_360 253 /* RO */
> #define EXT_CSD_FIRMWARE_VERSION 254 /* RO, 8 bytes */
> +#define EXT_CSD_CMDQ_DEPTH 307 /* RO */
> +#define EXT_CSD_CMDQ_SUPPORT 308 /* RO */
> #define EXT_CSD_SUPPORTED_MODE 493 /* RO */
> #define EXT_CSD_TAG_UNIT_SIZE 498 /* RO */
> #define EXT_CSD_DATA_TAG_SUPPORT 499 /* RO */
> @@ -438,6 +448,13 @@ struct _mmc_csd {
> #define EXT_CSD_MANUAL_BKOPS_MASK 0x01
>
> /*
> + * Command Queue
> + */
> +#define EXT_CSD_CMDQ_MODE_ENABLED BIT(0)
Is there a need for both EXT_CSD_CMDQ_MODE_ENABLED and
EXT_CSD_CMDQ_MODE_SUPPORTED. Arent they doing same thing?
> +#define EXT_CSD_CMDQ_DEPTH_MASK GENMASK(4, 0)
> +#define EXT_CSD_CMDQ_SUPPORTED BIT(0)
Ditto
> +
> +/*
> * MMC_SWITCH access modes
> */
>
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH V7 08/25] mmc: queue: Allocate queue of size qdepth
From: Ritesh Harjani @ 2016-11-28 4:22 UTC (permalink / raw)
To: Adrian Hunter
Cc: Ulf Hansson, linux-mmc, Alex Lemberg, Mateusz Nowak,
Yuliy Izrailov, Jaehoon Chung, Dong Aisheng, Das Asutosh,
Zhangfei Gao, Dorfman Konstantin, David Griego, Sahitya Tummala,
Venu Byravarasu, Linus Walleij
In-Reply-To: <1480068442-5169-9-git-send-email-adrian.hunter@intel.com>
On 11/25/2016 3:37 PM, Adrian Hunter wrote:
> Now that the queue resources are allocated according to the size of the
> queue, it is possible to allocate the queue to be an arbitrary size.
>
> A side-effect is that deallocation of 'packed' resources must be done
> before deallocation of the queue.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Looks good!
Reviewed-by: Harjani Ritesh <riteshh@codeaurora.org>
Regards
Ritesh
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH V7 07/25] mmc: queue: Use queue depth to allocate and free
From: Ritesh Harjani @ 2016-11-28 4:21 UTC (permalink / raw)
To: Adrian Hunter
Cc: Ulf Hansson, linux-mmc, Alex Lemberg, Mateusz Nowak,
Yuliy Izrailov, Jaehoon Chung, Dong Aisheng, Das Asutosh,
Zhangfei Gao, Dorfman Konstantin, David Griego, Sahitya Tummala,
Venu Byravarasu, Linus Walleij
In-Reply-To: <1480068442-5169-8-git-send-email-adrian.hunter@intel.com>
On 11/25/2016 3:37 PM, Adrian Hunter wrote:
> Instead of allocating resources for 2 slots in the queue, allow for an
> arbitrary number.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Looks good!
Reviewed-by: Harjani Ritesh <riteshh@codeaurora.org>
Regards
Ritesh
> ---
> drivers/mmc/card/queue.c | 103 +++++++++++++++++++++--------------------------
> 1 file changed, 46 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
> index 60fa095adb14..1ea007f51ec9 100644
> --- a/drivers/mmc/card/queue.c
> +++ b/drivers/mmc/card/queue.c
> @@ -189,86 +189,75 @@ static void mmc_queue_setup_discard(struct request_queue *q,
> static bool mmc_queue_alloc_bounce_bufs(struct mmc_queue *mq,
> unsigned int bouncesz)
> {
> - struct mmc_queue_req *mqrq_cur = mq->mqrq_cur;
> - struct mmc_queue_req *mqrq_prev = mq->mqrq_prev;
> -
> - mqrq_cur->bounce_buf = kmalloc(bouncesz, GFP_KERNEL);
> - if (!mqrq_cur->bounce_buf) {
> - pr_warn("%s: unable to allocate bounce cur buffer\n",
> - mmc_card_name(mq->card));
> - return false;
> - }
> + int i;
>
> - mqrq_prev->bounce_buf = kmalloc(bouncesz, GFP_KERNEL);
> - if (!mqrq_prev->bounce_buf) {
> - pr_warn("%s: unable to allocate bounce prev buffer\n",
> - mmc_card_name(mq->card));
> - kfree(mqrq_cur->bounce_buf);
> - mqrq_cur->bounce_buf = NULL;
> - return false;
> + for (i = 0; i < mq->qdepth; i++) {
> + mq->mqrq[i].bounce_buf = kmalloc(bouncesz, GFP_KERNEL);
> + if (!mq->mqrq[i].bounce_buf)
> + goto out_err;
> }
>
> return true;
> +
> +out_err:
> + while (--i >= 0) {
> + kfree(mq->mqrq[i].bounce_buf);
> + mq->mqrq[i].bounce_buf = NULL;
> + }
> + pr_warn("%s: unable to allocate bounce buffers\n",
> + mmc_card_name(mq->card));
> + return false;
> }
>
> static int mmc_queue_alloc_bounce_sgs(struct mmc_queue *mq,
> unsigned int bouncesz)
> {
> - struct mmc_queue_req *mqrq_cur = mq->mqrq_cur;
> - struct mmc_queue_req *mqrq_prev = mq->mqrq_prev;
> - int ret;
> -
> - mqrq_cur->sg = mmc_alloc_sg(1, &ret);
> - if (ret)
> - return ret;
> + int i, ret;
>
> - mqrq_cur->bounce_sg = mmc_alloc_sg(bouncesz / 512, &ret);
> - if (ret)
> - return ret;
> -
> - mqrq_prev->sg = mmc_alloc_sg(1, &ret);
> - if (ret)
> - return ret;
> + for (i = 0; i < mq->qdepth; i++) {
> + mq->mqrq[i].sg = mmc_alloc_sg(1, &ret);
> + if (ret)
> + return ret;
>
> - mqrq_prev->bounce_sg = mmc_alloc_sg(bouncesz / 512, &ret);
> + mq->mqrq[i].bounce_sg = mmc_alloc_sg(bouncesz / 512, &ret);
> + if (ret)
> + return ret;
> + }
>
> - return ret;
> + return 0;
> }
>
> static int mmc_queue_alloc_sgs(struct mmc_queue *mq, int max_segs)
> {
> - struct mmc_queue_req *mqrq_cur = mq->mqrq_cur;
> - struct mmc_queue_req *mqrq_prev = mq->mqrq_prev;
> - int ret;
> + int i, ret;
>
> - mqrq_cur->sg = mmc_alloc_sg(max_segs, &ret);
> - if (ret)
> - return ret;
> + for (i = 0; i < mq->qdepth; i++) {
> + mq->mqrq[i].sg = mmc_alloc_sg(max_segs, &ret);
> + if (ret)
> + return ret;
> + }
>
> - mqrq_prev->sg = mmc_alloc_sg(max_segs, &ret);
> + return 0;
> +}
>
> - return ret;
> +static void mmc_queue_req_free_bufs(struct mmc_queue_req *mqrq)
> +{
> + kfree(mqrq->bounce_sg);
> + mqrq->bounce_sg = NULL;
> +
> + kfree(mqrq->sg);
> + mqrq->sg = NULL;
> +
> + kfree(mqrq->bounce_buf);
> + mqrq->bounce_buf = NULL;
> }
>
> static void mmc_queue_reqs_free_bufs(struct mmc_queue *mq)
> {
> - struct mmc_queue_req *mqrq_cur = mq->mqrq_cur;
> - struct mmc_queue_req *mqrq_prev = mq->mqrq_prev;
> -
> - kfree(mqrq_cur->bounce_sg);
> - mqrq_cur->bounce_sg = NULL;
> - kfree(mqrq_prev->bounce_sg);
> - mqrq_prev->bounce_sg = NULL;
> -
> - kfree(mqrq_cur->sg);
> - mqrq_cur->sg = NULL;
> - kfree(mqrq_cur->bounce_buf);
> - mqrq_cur->bounce_buf = NULL;
> -
> - kfree(mqrq_prev->sg);
> - mqrq_prev->sg = NULL;
> - kfree(mqrq_prev->bounce_buf);
> - mqrq_prev->bounce_buf = NULL;
> + int i;
> +
> + for (i = 0; i < mq->qdepth; i++)
> + mmc_queue_req_free_bufs(&mq->mqrq[i]);
> }
>
> /**
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH V7 06/25] mmc: queue: Introduce queue depth
From: Ritesh Harjani @ 2016-11-28 4:19 UTC (permalink / raw)
To: Adrian Hunter
Cc: Ulf Hansson, linux-mmc, Alex Lemberg, Mateusz Nowak,
Yuliy Izrailov, Jaehoon Chung, Dong Aisheng, Das Asutosh,
Zhangfei Gao, Dorfman Konstantin, David Griego, Sahitya Tummala,
Venu Byravarasu, Linus Walleij
In-Reply-To: <1480068442-5169-7-git-send-email-adrian.hunter@intel.com>
On 11/25/2016 3:37 PM, Adrian Hunter wrote:
> Add a mmc_queue member to record the size of the queue, which currently
> supports 2 requests on-the-go at a time.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> drivers/mmc/card/block.c | 3 +++
> drivers/mmc/card/queue.c | 1 +
> drivers/mmc/card/queue.h | 1 +
> 3 files changed, 5 insertions(+)
>
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index f8e51640596e..47835b78872f 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -1439,6 +1439,9 @@ static int mmc_packed_init(struct mmc_queue *mq, struct mmc_card *card)
> struct mmc_queue_req *mqrq_prev = &mq->mqrq[1];
> int ret = 0;
>
> + /* Queue depth is only ever 2 with packed commands */
> + if (mq->qdepth != 2)
> + return -EINVAL;
I think you are referring here that with SWMCDQ, packed commands wont be
used. Instead of qdepth do you think we should check cmdq_en ?
Also maybe we shouldn't even call mmc_packed_init if cmdq_en is true?
>
> mqrq_cur->packed = kzalloc(sizeof(struct mmc_packed), GFP_KERNEL);
> if (!mqrq_cur->packed) {
> diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
> index cbe92c9cfda1..60fa095adb14 100644
> --- a/drivers/mmc/card/queue.c
> +++ b/drivers/mmc/card/queue.c
> @@ -296,6 +296,7 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card,
> if (!mq->queue)
> return -ENOMEM;
>
> + mq->qdepth = 2;
> mq->mqrq_cur = &mq->mqrq[0];
> mq->mqrq_prev = &mq->mqrq[1];
> mq->queue->queuedata = mq;
> diff --git a/drivers/mmc/card/queue.h b/drivers/mmc/card/queue.h
> index 0e8133c626c9..8a0a45e5650d 100644
> --- a/drivers/mmc/card/queue.h
> +++ b/drivers/mmc/card/queue.h
> @@ -64,6 +64,7 @@ struct mmc_queue {
> struct mmc_queue_req mqrq[2];
> struct mmc_queue_req *mqrq_cur;
> struct mmc_queue_req *mqrq_prev;
> + int qdepth;
> };
>
> extern int mmc_init_queue(struct mmc_queue *, struct mmc_card *, spinlock_t *,
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH V7 05/25] mmc: queue: Factor out mmc_queue_reqs_free_bufs()
From: Ritesh Harjani @ 2016-11-28 3:50 UTC (permalink / raw)
To: Adrian Hunter
Cc: Ulf Hansson, linux-mmc, Alex Lemberg, Mateusz Nowak,
Yuliy Izrailov, Jaehoon Chung, Dong Aisheng, Das Asutosh,
Zhangfei Gao, Dorfman Konstantin, David Griego, Sahitya Tummala,
Venu Byravarasu, Linus Walleij
In-Reply-To: <1480068442-5169-6-git-send-email-adrian.hunter@intel.com>
On 11/25/2016 3:37 PM, Adrian Hunter wrote:
> In preparation for supporting a queue of requests, factor out
> mmc_queue_reqs_free_bufs().
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Looks good!
Reviewed-by: Harjani Ritesh <riteshh@codeaurora.org>
Regards
Ritesh
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH V7 04/25] mmc: queue: Factor out mmc_queue_alloc_sgs()
From: Ritesh Harjani @ 2016-11-28 3:49 UTC (permalink / raw)
To: Adrian Hunter
Cc: Ulf Hansson, linux-mmc, Alex Lemberg, Mateusz Nowak,
Yuliy Izrailov, Jaehoon Chung, Dong Aisheng, Das Asutosh,
Zhangfei Gao, Dorfman Konstantin, David Griego, Sahitya Tummala,
Venu Byravarasu, Linus Walleij
In-Reply-To: <1480068442-5169-5-git-send-email-adrian.hunter@intel.com>
On 11/25/2016 3:37 PM, Adrian Hunter wrote:
> In preparation for supporting a queue of requests, factor out
> mmc_queue_alloc_sgs().
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Looks good!
Reviewed-by: Harjani Ritesh <riteshh@codeaurora.org>
Regards
Ritesh
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH V7 03/25] mmc: queue: Factor out mmc_queue_alloc_bounce_sgs()
From: Ritesh Harjani @ 2016-11-28 3:48 UTC (permalink / raw)
To: Adrian Hunter
Cc: Ulf Hansson, linux-mmc, Alex Lemberg, Mateusz Nowak,
Yuliy Izrailov, Jaehoon Chung, Dong Aisheng, Das Asutosh,
Zhangfei Gao, Dorfman Konstantin, David Griego, Sahitya Tummala,
Venu Byravarasu, Linus Walleij
In-Reply-To: <1480068442-5169-4-git-send-email-adrian.hunter@intel.com>
On 11/25/2016 3:37 PM, Adrian Hunter wrote:
> In preparation for supporting a queue of requests, factor out
> mmc_queue_alloc_bounce_sgs().
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Looks good!
Reviewed-by: Harjani Ritesh <riteshh@codeaurora.org>
Regards
Ritesh
> ---
> drivers/mmc/card/queue.c | 44 ++++++++++++++++++++++++++++----------------
> 1 file changed, 28 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
> index ea8b01f76d55..3756303b4bbc 100644
> --- a/drivers/mmc/card/queue.c
> +++ b/drivers/mmc/card/queue.c
> @@ -211,6 +211,30 @@ static bool mmc_queue_alloc_bounce_bufs(struct mmc_queue *mq,
> return true;
> }
>
> +static int mmc_queue_alloc_bounce_sgs(struct mmc_queue *mq,
> + unsigned int bouncesz)
> +{
> + struct mmc_queue_req *mqrq_cur = mq->mqrq_cur;
> + struct mmc_queue_req *mqrq_prev = mq->mqrq_prev;
> + int ret;
> +
> + mqrq_cur->sg = mmc_alloc_sg(1, &ret);
> + if (ret)
> + return ret;
> +
> + mqrq_cur->bounce_sg = mmc_alloc_sg(bouncesz / 512, &ret);
> + if (ret)
> + return ret;
> +
> + mqrq_prev->sg = mmc_alloc_sg(1, &ret);
> + if (ret)
> + return ret;
> +
> + mqrq_prev->bounce_sg = mmc_alloc_sg(bouncesz / 512, &ret);
> +
> + return ret;
> +}
> +
> /**
> * mmc_init_queue - initialise a queue structure.
> * @mq: mmc queue
> @@ -225,6 +249,7 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card,
> {
> struct mmc_host *host = card->host;
> u64 limit = BLK_BOUNCE_HIGH;
> + bool bounce = false;
> int ret;
> struct mmc_queue_req *mqrq_cur = &mq->mqrq[0];
> struct mmc_queue_req *mqrq_prev = &mq->mqrq[1];
> @@ -267,28 +292,15 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card,
> blk_queue_max_segments(mq->queue, bouncesz / 512);
> blk_queue_max_segment_size(mq->queue, bouncesz);
>
> - mqrq_cur->sg = mmc_alloc_sg(1, &ret);
> - if (ret)
> - goto cleanup_queue;
> -
> - mqrq_cur->bounce_sg =
> - mmc_alloc_sg(bouncesz / 512, &ret);
> - if (ret)
> - goto cleanup_queue;
> -
> - mqrq_prev->sg = mmc_alloc_sg(1, &ret);
> - if (ret)
> - goto cleanup_queue;
> -
> - mqrq_prev->bounce_sg =
> - mmc_alloc_sg(bouncesz / 512, &ret);
> + ret = mmc_queue_alloc_bounce_sgs(mq, bouncesz);
> if (ret)
> goto cleanup_queue;
> + bounce = true;
> }
> }
> #endif
>
> - if (!mqrq_cur->bounce_buf && !mqrq_prev->bounce_buf) {
> + if (!bounce) {
> blk_queue_bounce_limit(mq->queue, limit);
> blk_queue_max_hw_sectors(mq->queue,
> min(host->max_blk_count, host->max_req_size / 512));
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH V7 02/25] mmc: queue: Factor out mmc_queue_alloc_bounce_bufs()
From: Ritesh Harjani @ 2016-11-28 3:36 UTC (permalink / raw)
To: Adrian Hunter, Ulf Hansson
Cc: linux-mmc, Alex Lemberg, Mateusz Nowak, Yuliy Izrailov,
Jaehoon Chung, Dong Aisheng, Das Asutosh, Zhangfei Gao,
Dorfman Konstantin, David Griego, Sahitya Tummala,
Venu Byravarasu, Linus Walleij, Harjani, Ritesh
In-Reply-To: <1480068442-5169-3-git-send-email-adrian.hunter@intel.com>
On 11/25/2016 3:36 PM, Adrian Hunter wrote:
> In preparation for supporting a queue of requests, factor out
> mmc_queue_alloc_bounce_bufs().
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Looks good!
Reviewed-by: Harjani Ritesh <riteshh@codeaurora.org>
Regards
Ritesh
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH V7 01/25] mmc: queue: Fix queue thread wake-up
From: Ritesh Harjani @ 2016-11-28 3:32 UTC (permalink / raw)
To: Adrian Hunter
Cc: Ulf Hansson, linux-mmc, Alex Lemberg, Mateusz Nowak,
Yuliy Izrailov, Jaehoon Chung, Dong Aisheng, Das Asutosh,
Zhangfei Gao, Dorfman Konstantin, David Griego, Sahitya Tummala,
Venu Byravarasu, Linus Walleij
In-Reply-To: <1480068442-5169-2-git-send-email-adrian.hunter@intel.com>
On 11/25/2016 3:36 PM, Adrian Hunter wrote:
> The only time the driver sleeps expecting to be woken upon the arrival of
> a new request, is when the dispatch queue is empty. The only time that it
> is known whether the dispatch queue is empty is after NULL is returned
> from blk_fetch_request() while under the queue lock.
>
> Recognizing those facts, simplify the synchronization between the queue
> thread and the request function. A couple of flags tell the request
> function what to do, and the queue lock and barriers associated with
> wake-ups ensure synchronization.
>
> The result is simpler and allows the removal of the context_info lock.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Looks good!!
Reviewed-by: Harjani Ritesh <riteshh@codeaurora.org>
Regards
Ritesh
> ---
> drivers/mmc/card/block.c | 7 -------
> drivers/mmc/card/queue.c | 35 +++++++++++++++++++++--------------
> drivers/mmc/card/queue.h | 1 +
> drivers/mmc/core/core.c | 6 ------
> include/linux/mmc/host.h | 2 --
> 5 files changed, 22 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index 6618126fcb9f..f8e51640596e 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -2193,8 +2193,6 @@ int mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req)
> int ret;
> struct mmc_blk_data *md = mq->blkdata;
> struct mmc_card *card = md->queue.card;
> - struct mmc_host *host = card->host;
> - unsigned long flags;
> bool req_is_special = mmc_req_is_special(req);
>
> if (req && !mq->mqrq_prev->req)
> @@ -2227,11 +2225,6 @@ int mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req)
> mmc_blk_issue_rw_rq(mq, NULL);
> ret = mmc_blk_issue_flush(mq, req);
> } else {
> - if (!req && host->areq) {
> - spin_lock_irqsave(&host->context_info.lock, flags);
> - host->context_info.is_waiting_last_req = true;
> - spin_unlock_irqrestore(&host->context_info.lock, flags);
> - }
> ret = mmc_blk_issue_rw_rq(mq, req);
> }
>
> diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
> index 3f6a2463ab30..c4ac4b8a1a98 100644
> --- a/drivers/mmc/card/queue.c
> +++ b/drivers/mmc/card/queue.c
> @@ -53,6 +53,7 @@ static int mmc_queue_thread(void *d)
> {
> struct mmc_queue *mq = d;
> struct request_queue *q = mq->queue;
> + struct mmc_context_info *cntx = &mq->card->host->context_info;
>
> current->flags |= PF_MEMALLOC;
>
> @@ -63,6 +64,19 @@ static int mmc_queue_thread(void *d)
> spin_lock_irq(q->queue_lock);
> set_current_state(TASK_INTERRUPTIBLE);
> req = blk_fetch_request(q);
> + mq->asleep = false;
> + cntx->is_waiting_last_req = false;
> + cntx->is_new_req = false;
> + if (!req) {
> + /*
> + * Dispatch queue is empty so set flags for
> + * mmc_request_fn() to wake us up.
> + */
> + if (mq->mqrq_prev->req)
> + cntx->is_waiting_last_req = true;
> + else
> + mq->asleep = true;
> + }
> mq->mqrq_cur->req = req;
> spin_unlock_irq(q->queue_lock);
>
> @@ -115,7 +129,6 @@ static void mmc_request_fn(struct request_queue *q)
> {
> struct mmc_queue *mq = q->queuedata;
> struct request *req;
> - unsigned long flags;
> struct mmc_context_info *cntx;
>
> if (!mq) {
> @@ -127,19 +140,13 @@ static void mmc_request_fn(struct request_queue *q)
> }
>
> cntx = &mq->card->host->context_info;
> - if (!mq->mqrq_cur->req && mq->mqrq_prev->req) {
> - /*
> - * New MMC request arrived when MMC thread may be
> - * blocked on the previous request to be complete
> - * with no current request fetched
> - */
> - spin_lock_irqsave(&cntx->lock, flags);
> - if (cntx->is_waiting_last_req) {
> - cntx->is_new_req = true;
> - wake_up_interruptible(&cntx->wait);
> - }
> - spin_unlock_irqrestore(&cntx->lock, flags);
> - } else if (!mq->mqrq_cur->req && !mq->mqrq_prev->req)
> +
> + if (cntx->is_waiting_last_req) {
> + cntx->is_new_req = true;
> + wake_up_interruptible(&cntx->wait);
> + }
> +
> + if (mq->asleep)
> wake_up_process(mq->thread);
> }
>
> diff --git a/drivers/mmc/card/queue.h b/drivers/mmc/card/queue.h
> index 334c9306070f..0e8133c626c9 100644
> --- a/drivers/mmc/card/queue.h
> +++ b/drivers/mmc/card/queue.h
> @@ -58,6 +58,7 @@ struct mmc_queue {
> unsigned int flags;
> #define MMC_QUEUE_SUSPENDED (1 << 0)
> #define MMC_QUEUE_NEW_REQUEST (1 << 1)
> + bool asleep;
> struct mmc_blk_data *blkdata;
> struct request_queue *queue;
> struct mmc_queue_req mqrq[2];
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index f39397f7c8dc..dc1f27ee50b8 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -504,18 +504,14 @@ static enum mmc_blk_status mmc_wait_for_data_req_done(struct mmc_host *host,
> struct mmc_command *cmd;
> struct mmc_context_info *context_info = &host->context_info;
> enum mmc_blk_status status;
> - unsigned long flags;
>
> while (1) {
> wait_event_interruptible(context_info->wait,
> (context_info->is_done_rcv ||
> context_info->is_new_req));
> - spin_lock_irqsave(&context_info->lock, flags);
> context_info->is_waiting_last_req = false;
> - spin_unlock_irqrestore(&context_info->lock, flags);
> if (context_info->is_done_rcv) {
> context_info->is_done_rcv = false;
> - context_info->is_new_req = false;
> cmd = mrq->cmd;
>
> if (!cmd->error || !cmd->retries ||
> @@ -534,7 +530,6 @@ static enum mmc_blk_status mmc_wait_for_data_req_done(struct mmc_host *host,
> continue; /* wait for done/new event again */
> }
> } else if (context_info->is_new_req) {
> - context_info->is_new_req = false;
> if (!next_req)
> return MMC_BLK_NEW_REQUEST;
> }
> @@ -3016,7 +3011,6 @@ void mmc_unregister_pm_notifier(struct mmc_host *host)
> */
> void mmc_init_context_info(struct mmc_host *host)
> {
> - spin_lock_init(&host->context_info.lock);
> host->context_info.is_new_req = false;
> host->context_info.is_done_rcv = false;
> host->context_info.is_waiting_last_req = false;
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 2a6418d0c343..bcf6d252ec67 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -197,14 +197,12 @@ struct mmc_slot {
> * @is_new_req wake up reason was new request
> * @is_waiting_last_req mmc context waiting for single running request
> * @wait wait queue
> - * @lock lock to protect data fields
> */
> struct mmc_context_info {
> bool is_done_rcv;
> bool is_new_req;
> bool is_waiting_last_req;
> wait_queue_head_t wait;
> - spinlock_t lock;
> };
>
> struct regulator;
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH 3/3] mmc: dw_mmc: display the clock message only one time when card is polling
From: Jaehoon Chung @ 2016-11-28 1:42 UTC (permalink / raw)
To: linux-mmc; +Cc: ulf.hansson, shawn.lin
In-Reply-To: <20161124110442.22058-3-jh80.chung@samsung.com>
On 11/24/2016 08:04 PM, Jaehoon Chung wrote:
> When card is polling (broken-cd), there is a spamming messge related to
> clock.
> After applied this patch, display the message only one time at boot
> time. It's enough to check which clock values is used.
> Also prevent to display the spamming message.
>
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
Applied on my dwmmc repository.
Best Regards,
Jaehoon Chung
> ---
> drivers/mmc/host/dw_mmc.c | 13 ++++++++++++-
> drivers/mmc/host/dw_mmc.h | 1 +
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index d508225..dd7e95d 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1176,13 +1176,24 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>
> div = (host->bus_hz != clock) ? DIV_ROUND_UP(div, 2) : 0;
>
> - if (clock != slot->__clk_old || force_clkinit)
> + if ((clock != slot->__clk_old &&
> + !test_bit(DW_MMC_CARD_NEEDS_POLL, &slot->flags)) ||
> + force_clkinit) {
> dev_info(&slot->mmc->class_dev,
> "Bus speed (slot %d) = %dHz (slot req %dHz, actual %dHZ div = %d)\n",
> slot->id, host->bus_hz, clock,
> div ? ((host->bus_hz / div) >> 1) :
> host->bus_hz, div);
>
> + /*
> + * If card is polling, display the message only
> + * one time at boot time.
> + */
> + if (slot->mmc->caps & MMC_CAP_NEEDS_POLL &&
> + slot->mmc->f_min == clock)
> + set_bit(DW_MMC_CARD_NEEDS_POLL, &slot->flags);
> + }
> +
> /* disable clock */
> mci_writel(host, CLKENA, 0);
> mci_writel(host, CLKSRC, 0);
> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
> index 4a6ae750..c594658 100644
> --- a/drivers/mmc/host/dw_mmc.h
> +++ b/drivers/mmc/host/dw_mmc.h
> @@ -272,6 +272,7 @@ struct dw_mci_slot {
> #define DW_MMC_CARD_NEED_INIT 1
> #define DW_MMC_CARD_NO_LOW_PWR 2
> #define DW_MMC_CARD_NO_USE_HOLD 3
> +#define DW_MMC_CARD_NEEDS_POLL 4
> int id;
> int sdio_id;
> };
>
^ permalink raw reply
* Re: [PATCH 2/3] mmc: dw_mmc: add the debug message for polling and non-removable
From: Jaehoon Chung @ 2016-11-28 1:42 UTC (permalink / raw)
To: linux-mmc; +Cc: ulf.hansson, shawn.lin
In-Reply-To: <20161124110442.22058-2-jh80.chung@samsung.com>
On 11/24/2016 08:04 PM, Jaehoon Chung wrote:
> If card is polling or non-removable, display the more exact message.
> It's helpful to debug which detecting scheme is using.
>
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
Applied on my dwmmc repository.
Best Regards,
Jaehoon Chung
> ---
> drivers/mmc/host/dw_mmc.c | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index c6cc618..d508225 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1525,9 +1525,23 @@ static int dw_mci_get_cd(struct mmc_host *mmc)
> int gpio_cd = mmc_gpio_get_cd(mmc);
>
> /* Use platform get_cd function, else try onboard card detect */
> - if ((mmc->caps & MMC_CAP_NEEDS_POLL) || !mmc_card_is_removable(mmc))
> + if (((mmc->caps & MMC_CAP_NEEDS_POLL)
> + || !mmc_card_is_removable(mmc))) {
> present = 1;
> - else if (gpio_cd >= 0)
> +
> + if (!test_bit(DW_MMC_CARD_PRESENT, &slot->flags)) {
> + if (mmc->caps & MMC_CAP_NEEDS_POLL) {
> + dev_info(&mmc->class_dev,
> + "card is polling.\n");
> + } else {
> + dev_info(&mmc->class_dev,
> + "card is non-removable.\n");
> + }
> + set_bit(DW_MMC_CARD_PRESENT, &slot->flags);
> + }
> +
> + return present;
> + } else if (gpio_cd >= 0)
> present = gpio_cd;
> else
> present = (mci_readl(slot->host, CDETECT) & (1 << slot->id))
>
^ permalink raw reply
* Re: [PATCH 1/3] mmc: dw_mmc: check the "present" variable before checking flags
From: Jaehoon Chung @ 2016-11-28 1:42 UTC (permalink / raw)
To: linux-mmc; +Cc: ulf.hansson, shawn.lin
In-Reply-To: <20161124110442.22058-1-jh80.chung@samsung.com>
On 11/24/2016 08:04 PM, Jaehoon Chung wrote:
> Before checking flags, it has to check "present" variable.
> Otherwise, flags should be cleared everytime.
>
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
Applied on my dwmmc repository.
Best Regards,
Jaehoon Chung
> ---
> drivers/mmc/host/dw_mmc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index d400afc..c6cc618 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1536,7 +1536,8 @@ static int dw_mci_get_cd(struct mmc_host *mmc)
> spin_lock_bh(&host->lock);
> if (present && !test_and_set_bit(DW_MMC_CARD_PRESENT, &slot->flags))
> dev_dbg(&mmc->class_dev, "card is present\n");
> - else if (!test_and_clear_bit(DW_MMC_CARD_PRESENT, &slot->flags))
> + else if (!present &&
> + !test_and_clear_bit(DW_MMC_CARD_PRESENT, &slot->flags))
> dev_dbg(&mmc->class_dev, "card is not present\n");
> spin_unlock_bh(&host->lock);
>
>
^ permalink raw reply
* Re: [PATCH v3] mmc: block: delete packed command support
From: Alex Lemberg @ 2016-11-27 14:25 UTC (permalink / raw)
To: Ulf Hansson, Linus Walleij
Cc: linux-mmc@vger.kernel.org, Chunyan Zhang, Baolin Wang,
Namjae Jeon, Maya Erez
In-Reply-To: <CAPDyKFoQOb19xq2zLZuAHp_JCcpyeo5MvtvFnNjq8hRcPZcTxA@mail.gmail.com>
Hi Ulf and Linus,
I understand your concerns about the current packed commands
support and the problem of transition to blk_mq.
But as I mentioned in previous email thread, the packed commands
feature is still in use on eMMC4.5 or eMMC5.1 devices on hosts
without SW/HW CMDQ support.
Taking this feature out meaning stop supporting one of important
performance features of eMMC devices.
Looking forward this feature is replaced with a CMDQ, but should we
kill it for those who doesn’t have a CMDQ?
Thanks,
Alex
On 11/25/16, 3:30 PM, "linux-mmc-owner@vger.kernel.org on behalf of Ulf Hansson" <linux-mmc-owner@vger.kernel.org on behalf of ulf.hansson@linaro.org> wrote:
>On 25 November 2016 at 10:35, Linus Walleij <linus.walleij@linaro.org> wrote:
>> I've had it with this code now.
>>
>> The packed command support is a complex hurdle in the MMC/SD block
>> layer, around 500+ lines of code which was introduced in 2013 in
>> commits
>>
>> ce39f9d17c14 ("mmc: support packed write command for eMMC4.5 devices")
>> abd9ac144947 ("mmc: add packed command feature of eMMC4.5")
>>
>> ...and since then it has been rotting. The original author of the
>> code has disappeared from the community and the mail address is
>> bouncing.
>>
>> For the code to be exercised the host must flag that it supports
>> packed commands, so in mmc_blk_prep_packed_list() which is called for
>> every single request, the following construction appears:
>>
>> u8 max_packed_rw = 0;
>>
>> if ((rq_data_dir(cur) == WRITE) &&
>> mmc_host_packed_wr(card->host))
>> max_packed_rw = card->ext_csd.max_packed_writes;
>>
>> if (max_packed_rw == 0)
>> goto no_packed;
>>
>> This has the following logical deductions:
>>
>> - Only WRITE commands can really be packed, so the solution is
>> only half-done: we support packed WRITE but not packed READ.
>> The packed command support has not been finalized by supporting
>> reads in three years!
>>
>> - mmc_host_packed_wr() is just a static inline that checks
>> host->caps2 & MMC_CAP2_PACKED_WR. The problem with this is
>> that NO upstream host sets this capability flag! No driver
>> in the kernel is using it, and we can't test it. Packed
>> command may be supported in out-of-tree code, but I doubt
>> it. I doubt that the code is even working anymore due to
>> other refactorings in the MMC block layer, who would
>> notice if patches affecting it broke packed commands?
>> No one.
>>
>> - There is no Device Tree binding or code to mark a host as
>> supporting packed read or write commands, just this flag
>> in caps2, so for sure there are not any DT systems using
>> it either.
>>
>> It has other problems as well: mmc_blk_prep_packed_list() is
>> speculatively picking requests out of the request queue with
>> blk_fetch_request() making the MMC/SD stack harder to convert
>> to the multiqueue block layer. By this we get rid of an
>> obstacle.
>>
>> The way I see it this is just cruft littering the MMC/SD
>> stack.
>>
>> Cc: Namjae Jeon <namjae.jeon@samsung.com>
>> Cc: Maya Erez <qca_merez@qca.qualcomm.com>
>> Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>
>Thanks, applied for next!
>
>Kind regards
>Uffe
>
^ permalink raw reply
* Xmas Offer
From: Mrs Julie Leach @ 2016-11-26 2:20 UTC (permalink / raw)
To: Recipients
You are a recipient to Mrs Julie Leach Donation of $3 million USD. Contact ( julieleach93@gmail.com ) for claims.
^ permalink raw reply
* Re: [PATCH v2] mmc: block: delete packed command support
From: Nicolas Pitre @ 2016-11-25 20:38 UTC (permalink / raw)
To: Linus Walleij
Cc: linux-mmc, Ulf Hansson, Chunyan Zhang, Baolin Wang, Namjae Jeon,
Maya Erez
In-Reply-To: <9ZmUc0DeYSJWc9ZmVc6SWm@videotron.ca>
On Wed, 23 Nov 2016, Linus Walleij wrote:
> I've had it with this code now.
>
> The packed command support is a complex hurdle in the MMC/SD block
> layer, around 500+ lines of code which was introduced in 2013 in
> commits
>
> ce39f9d17c14 ("mmc: support packed write command for eMMC4.5 devices")
> abd9ac144947 ("mmc: add packed command feature of eMMC4.5")
>
> ...and since then it has been rotting. The original author of the
> code has disappeared from the community and the mail address is
> bouncing.
Just rip it out.
And if ever this comes back then please make sure it is configurable
from kconfig. Not only would that help keeping tiny Linux systems tiny,
but it would also helps identifying the code actually responsible for
that feature.
Nicolas
^ permalink raw reply
* Re: [PATCH V7 06/25] mmc: queue: Introduce queue depth
From: Adrian Hunter @ 2016-11-25 17:20 UTC (permalink / raw)
To: Linus Walleij
Cc: Ulf Hansson, linux-mmc, Alex Lemberg, Mateusz Nowak,
Yuliy Izrailov, Jaehoon Chung, Dong Aisheng, Das Asutosh,
Zhangfei Gao, Dorfman Konstantin, David Griego, Sahitya Tummala,
Harjani Ritesh, Venu Byravarasu
In-Reply-To: <CACRpkdaQL3UM5DK9nwiQEcR_R19qWqRwt+KpOJNFR1HKEQ5bkA@mail.gmail.com>
On 25/11/2016 4:43 p.m., Linus Walleij wrote:
> On Fri, Nov 25, 2016 at 11:07 AM, Adrian Hunter <adrian.hunter@intel.com> wrote:
>
>> Add a mmc_queue member to record the size of the queue, which currently
>> supports 2 requests on-the-go at a time.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> (...)
>> + /* Queue depth is only ever 2 with packed commands */
>
> That's a weird comment.
> It doesn't have anything with packed commands to do does it?
No it doesn't. Packed commands and command queue are mutually exclusive,
but the packed commands implementation expects there to be 2
mmc_queue_req's. Adding a check was just for the code to protect itself
from oops.
> In that case it was just deleted.
Yes, I will re-base.
>
> Are you referring to the asynchronous issuing pipeline thing
> (cur & next)?
Yes.
^ permalink raw reply
* Re: [PATCH V7 25/25] mmc: sdhci-acpi: Enable Software Command Queuing for some Intel controllers
From: Linus Walleij @ 2016-11-25 15:15 UTC (permalink / raw)
To: Adrian Hunter
Cc: Ulf Hansson, linux-mmc, Alex Lemberg, Mateusz Nowak,
Yuliy Izrailov, Jaehoon Chung, Dong Aisheng, Das Asutosh,
Zhangfei Gao, Dorfman Konstantin, David Griego, Sahitya Tummala,
Harjani Ritesh, Venu Byravarasu
In-Reply-To: <1480068442-5169-26-git-send-email-adrian.hunter@intel.com>
On Fri, Nov 25, 2016 at 11:07 AM, Adrian Hunter <adrian.hunter@intel.com> wrote:
> Set MMC_CAP_SWCMDQ for Intel BYT and related eMMC host controllers.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> drivers/mmc/host/sdhci-acpi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
> index 81d4dc034793..95fc4de05c54 100644
> --- a/drivers/mmc/host/sdhci-acpi.c
> +++ b/drivers/mmc/host/sdhci-acpi.c
> @@ -274,7 +274,7 @@ static int sdhci_acpi_sd_probe_slot(struct platform_device *pdev,
> static const struct sdhci_acpi_slot sdhci_acpi_slot_int_emmc = {
> .chip = &sdhci_acpi_chip_int,
> .caps = MMC_CAP_8_BIT_DATA | MMC_CAP_NONREMOVABLE |
> - MMC_CAP_HW_RESET | MMC_CAP_1_8V_DDR |
> + MMC_CAP_HW_RESET | MMC_CAP_1_8V_DDR | MMC_CAP_SWCMDQ |
Actually I don't see why SOFTWARE command queueing would need a cap flag
in the host at all?
Isn't the whole point with it that if it is available, we don't need any special
hardware support to use it with any host?
So why not just enable it if the card supports it in that case, why flag
it in the host at all?
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH V7 20/25] mmc: queue: Share mmc request array between partitions
From: Linus Walleij @ 2016-11-25 15:01 UTC (permalink / raw)
To: Adrian Hunter
Cc: Ulf Hansson, linux-mmc, Alex Lemberg, Mateusz Nowak,
Yuliy Izrailov, Jaehoon Chung, Dong Aisheng, Das Asutosh,
Zhangfei Gao, Dorfman Konstantin, David Griego, Sahitya Tummala,
Harjani Ritesh, Venu Byravarasu
In-Reply-To: <1480068442-5169-21-git-send-email-adrian.hunter@intel.com>
On Fri, Nov 25, 2016 at 11:07 AM, Adrian Hunter <adrian.hunter@intel.com> wrote:
> eMMC can have multiple internal partitions that are represented as separate
> disks / queues. However the card has only 1 command queue which must be
> empty when switching partitions. Consequently the array of mmc requests
> that are queued can be shared between partitions saving memory.
>
> Keep a pointer to the mmc request queue on the card, and use that instead
> of allocating a new one for each partition. Use a reference count to keep
> track of when to free it.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
This is a good refactoring no matter how we proceed with command
queueuing. Some comments.
> @@ -1480,6 +1480,9 @@ static int mmc_packed_init(struct mmc_queue *mq, struct mmc_card *card)
> if (mq->qdepth != 2)
> return -EINVAL;
>
> + if (mqrq_cur->packed)
> + goto out;
Well packed command is gone so this goes away.
> +++ b/drivers/mmc/card/queue.c
> @@ -200,10 +200,17 @@ static struct mmc_queue_req *mmc_queue_alloc_mqrqs(struct mmc_queue *mq,
> struct mmc_queue_req *mqrq;
> int i;
>
> + if (mq->card->mqrq) {
> + mq->card->mqrq_ref_cnt += 1;
> + return mq->card->mqrq;
> + }
> +
> mqrq = kcalloc(qdepth, sizeof(*mqrq), GFP_KERNEL);
> if (mqrq) {
> for (i = 0; i < mq->qdepth; i++)
> mqrq[i].task_id = i;
> + mq->card->mqrq = mqrq;
> + mq->card->mqrq_ref_cnt = 1;
> }
OK
> + if (mq->card->mqrq_ref_cnt > 1)
> + return !!mq->mqrq[0].bounce_buf;
Hm that seems inseparable from the other changes.
Decrease of refcount seems correct.
> + struct mmc_queue_req *mqrq; /* Shared queue structure */
> + int mqrq_ref_cnt; /* Shared queue ref. count */
I'm not smart enough to see if we're always increasing/decreasing
this under a lock or otherwise exclusive context, or if it would be
better to use an atomic type for counting, like kref does?
Well maybe the whole thing could use kref I dunno.
I guess it should be an unsigned int atleast.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH V7 18/25] mmc: block: Pass mqrq to mmc_blk_prep_packed_list()
From: Linus Walleij @ 2016-11-25 14:53 UTC (permalink / raw)
To: Adrian Hunter
Cc: Ulf Hansson, linux-mmc, Alex Lemberg, Mateusz Nowak,
Yuliy Izrailov, Jaehoon Chung, Dong Aisheng, Das Asutosh,
Zhangfei Gao, Dorfman Konstantin, David Griego, Sahitya Tummala,
Harjani Ritesh, Venu Byravarasu
In-Reply-To: <1480068442-5169-19-git-send-email-adrian.hunter@intel.com>
On Fri, Nov 25, 2016 at 11:07 AM, Adrian Hunter <adrian.hunter@intel.com> wrote:
> A subsequent patch will remove 'mq->mqrq_cur'. Prepare for that by
> passing mqrq to mmc_blk_prep_packed_list().
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
This code path is deleted upstream.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH V7 17/25] mmc: block: Use local var for mqrq_cur
From: Linus Walleij @ 2016-11-25 14:52 UTC (permalink / raw)
To: Adrian Hunter
Cc: Ulf Hansson, linux-mmc, Alex Lemberg, Mateusz Nowak,
Yuliy Izrailov, Jaehoon Chung, Dong Aisheng, Das Asutosh,
Zhangfei Gao, Dorfman Konstantin, David Griego, Sahitya Tummala,
Harjani Ritesh, Venu Byravarasu
In-Reply-To: <1480068442-5169-18-git-send-email-adrian.hunter@intel.com>
On Fri, Nov 25, 2016 at 11:07 AM, Adrian Hunter <adrian.hunter@intel.com> wrote:
> A subsequent patch will remove 'mq->mqrq_cur'. Prepare for that by
> assigning it to a local variable.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH V7 16/25] mmc: block: Fix 4K native sector check
From: Linus Walleij @ 2016-11-25 14:51 UTC (permalink / raw)
To: Adrian Hunter
Cc: Ulf Hansson, linux-mmc, Alex Lemberg, Mateusz Nowak,
Yuliy Izrailov, Jaehoon Chung, Dong Aisheng, Das Asutosh,
Zhangfei Gao, Dorfman Konstantin, David Griego, Sahitya Tummala,
Harjani Ritesh, Venu Byravarasu
In-Reply-To: <1480068442-5169-17-git-send-email-adrian.hunter@intel.com>
On Fri, Nov 25, 2016 at 11:07 AM, Adrian Hunter <adrian.hunter@intel.com> wrote:
> The 4K native sector check does not allow for the 'do' loop nor the
> variables used after the 'cmd_abort' label.
>
> 'brq' and 'req' get reassigned in the 'do' loop, so the check must not
> assume what their values are. After the 'cmd_abort' label, 'mq_rq' and
> 'req' are used, but 'rqc' must be NULL otherwise it can be started again.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Looks correct, and is clearly a sign of what we all know:
mmc_blk_issue_rw_rq() is hopelessly convoluted and needs to
be refactored into something we can read.
Shouldn't this patch just be moved to the front of the patch queue
and merged as a fix? AFAICT it's a plain bug.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH V7 06/25] mmc: queue: Introduce queue depth
From: Linus Walleij @ 2016-11-25 14:43 UTC (permalink / raw)
To: Adrian Hunter
Cc: Ulf Hansson, linux-mmc, Alex Lemberg, Mateusz Nowak,
Yuliy Izrailov, Jaehoon Chung, Dong Aisheng, Das Asutosh,
Zhangfei Gao, Dorfman Konstantin, David Griego, Sahitya Tummala,
Harjani Ritesh, Venu Byravarasu
In-Reply-To: <1480068442-5169-7-git-send-email-adrian.hunter@intel.com>
On Fri, Nov 25, 2016 at 11:07 AM, Adrian Hunter <adrian.hunter@intel.com> wrote:
> Add a mmc_queue member to record the size of the queue, which currently
> supports 2 requests on-the-go at a time.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
(...)
> + /* Queue depth is only ever 2 with packed commands */
That's a weird comment.
It doesn't have anything with packed commands to do does it?
In that case it was just deleted.
Are you referring to the asynchronous issuing pipeline thing
(cur & next)?
Yours,
Linus Walleij
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox