* Re: [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Ulf Hansson @ 2016-11-24 9:05 UTC (permalink / raw)
To: Gregory CLEMENT, Rob Herring
Cc: Ziji Hu, Adrian Hunter, linux-mmc@vger.kernel.org, Jason Cooper,
Andrew Lunn, Sebastian Hesselbarth, devicetree@vger.kernel.org,
Thomas Petazzoni, linux-arm-kernel@lists.infradead.org,
Jack(SH) Zhu, Jimmy Xu, Jisheng Zhang, Nadav Haklai, Ryan Gao,
Doug Jones, Shiwu Zhang, Victor Gu, Wei(SOCP) Liu
In-Reply-To: <87d1hno2d7.fsf@free-electrons.com>
On 22 November 2016 at 18:23, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:
> Hi Rob,
>
> On jeu., nov. 10 2016, Ziji Hu <huziji@marvell.com> wrote:
>
> [...]
>
>>>> +
>>>> +- reg:
>>>> + * For "marvell,xenon-sdhci", one register area for Xenon IP.
>>>> +
>>>> + * For "marvell,armada-3700-sdhci", two register areas.
>>>> + The first one for Xenon IP register. The second one for the Armada 3700 SOC
>>>> + PHY PAD Voltage Control register.
>>>> + Please follow the examples with compatible "marvell,armada-3700-sdhci"
>>>> + in below.
>>>> + Please also check property marvell,pad-type in below.
>>>> +
>>>> +Optional Properties:
>>>> +- marvell,xenon-slotno:
>>>
>>> Multiple slots should be represented as child nodes IMO. I think some
>>> other bindings already do this.
>>>
>>
>> All the slots are entirely independent.
>> I prefer to consider it as multiple independent SDHCs placed in
>> a single IP, instead of that a IP contains multiple child slots.
>
> It was indeed what I tried to show in my answer for the 1st version:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2016-October/461860.html
>
> Maybe you missed it.
>
> You also mentioned other bindings using child nodes, but for this one
> we have one controller with only one set of register with multiple slots
> (Atmel is an example). Here each slot have it own set of register.
>
> Actually giving the fact that each slot is controlled by a different set
> of register I wonder why the hardware can't also deduce the slot number
> from the address register. For me it looks like an hardware bug but we
> have to deal with it.
>
> Do you still think we needchild node here?
Using child-nodes for slots like what's done in the atmel case, is
currently broken. I would recommend to avoid using child-nodes for
slots, if possible.
To give you some more background, currently the mmc core treats child
nodes as embedded non-removable cards or SDIO funcs. However, we can
change to make child-nodes also allowed to describe slots, but it
requires a specific compatible for "slots" and of course then we also
need to update the DT parsing of the child-nodes in the mmc core.
Documentation/devicetree/bindings/mmc/mmc.txt
Documentation/devicetree/bindings/mmc/mmc-card.txt
>
>>
>> It is unlike the implementation which put multiple slots behind PCIe EP interface. sdhci-pci.c will handle each slot init one by one.
>> If Xenon SDHC slots are represented as child nodes, there should also be a main entry in Xenon driver to init each child node one by one.
>> In my very own opinion, it is inconvenient and unnecessary.
>
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH v2] mmc: block: delete packed command support
From: kbuild test robot @ 2016-11-24 6:35 UTC (permalink / raw)
Cc: kbuild-all, linux-mmc, Ulf Hansson, Chunyan Zhang, Baolin Wang,
Linus Walleij, Namjae Jeon, Maya Erez
In-Reply-To: <1479916086-26641-1-git-send-email-linus.walleij@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 2975 bytes --]
Hi Linus,
[auto build test WARNING on ulf.hansson-mmc/next]
[cannot apply to linus/master linux/master v4.9-rc6 next-20161123]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Linus-Walleij/mmc-block-delete-packed-command-support/20161124-095329
base: git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git next
config: x86_64-randconfig-s0-11241041 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
drivers/mmc/card/block.c: In function 'mmc_blk_issue_rw_rq':
>> drivers/mmc/card/block.c:1604: warning: unused variable 'reqs'
vim +/reqs +1604 drivers/mmc/card/block.c
ecf8b5d0a Subhash Jadavani 2012-06-07 1588 ret = blk_end_request(req, 0, blocks << 9);
67716327e Adrian Hunter 2011-08-29 1589 }
67716327e Adrian Hunter 2011-08-29 1590 }
67716327e Adrian Hunter 2011-08-29 1591 return ret;
67716327e Adrian Hunter 2011-08-29 1592 }
67716327e Adrian Hunter 2011-08-29 1593
ee8a43a51 Per Forlin 2011-07-01 1594 static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
54d49d776 Per Forlin 2011-07-01 1595 {
3e1319bfe Linus Walleij 2016-11-18 1596 struct mmc_blk_data *md = mq->blkdata;
54d49d776 Per Forlin 2011-07-01 1597 struct mmc_card *card = md->queue.card;
54d49d776 Per Forlin 2011-07-01 1598 struct mmc_blk_request *brq = &mq->mqrq_cur->brq;
b8360a494 Adrian Hunter 2015-05-07 1599 int ret = 1, disable_multi = 0, retry = 0, type, retune_retry_done = 0;
d78d4a8ad Per Forlin 2011-07-01 1600 enum mmc_blk_status status;
ee8a43a51 Per Forlin 2011-07-01 1601 struct mmc_queue_req *mq_rq;
a5075eb94 Saugata Das 2012-05-17 1602 struct request *req = rqc;
ee8a43a51 Per Forlin 2011-07-01 1603 struct mmc_async_req *areq;
ce39f9d17 Seungwon Jeon 2013-02-06 @1604 u8 reqs = 0;
ee8a43a51 Per Forlin 2011-07-01 1605
ee8a43a51 Per Forlin 2011-07-01 1606 if (!rqc && !mq->mqrq_prev->req)
ee8a43a51 Per Forlin 2011-07-01 1607 return 0;
54d49d776 Per Forlin 2011-07-01 1608
54d49d776 Per Forlin 2011-07-01 1609 do {
ee8a43a51 Per Forlin 2011-07-01 1610 if (rqc) {
a5075eb94 Saugata Das 2012-05-17 1611 /*
a5075eb94 Saugata Das 2012-05-17 1612 * When 4KB native sector is enabled, only 8 blocks
:::::: The code at line 1604 was first introduced by commit
:::::: ce39f9d17c14e56ea6772aa84393e6e0cc8499c4 mmc: support packed write command for eMMC4.5 devices
:::::: TO: Seungwon Jeon <tgih.jun@samsung.com>
:::::: CC: Chris Ball <cjb@laptop.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31381 bytes --]
^ permalink raw reply
* Re: [PATCH 2/5] smd: Make packet size a constant
From: Bjorn Andersson @ 2016-11-24 6:14 UTC (permalink / raw)
To: Jeremy McNicoll
Cc: linux-arm-msm, linux-soc, devicetree, linux-mmc, andy.gross,
sboyd, robh, arnd, riteshh
In-Reply-To: <1479863388-23678-3-git-send-email-jeremymc@redhat.com>
On Tue 22 Nov 17:09 PST 2016, Jeremy McNicoll wrote:
> Use a macro to define the maximum size of a RPM message.
>
No thanks.
> Signed-off-by: Jeremy McNicoll <jeremymc@redhat.com>
> ---
> drivers/soc/qcom/smd-rpm.c | 2 +-
> include/linux/soc/qcom/smd.h | 7 +++++++
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/qcom/smd-rpm.c b/drivers/soc/qcom/smd-rpm.c
> index 6609d7e..b5a2836 100644
> --- a/drivers/soc/qcom/smd-rpm.c
> +++ b/drivers/soc/qcom/smd-rpm.c
> @@ -114,7 +114,7 @@ int qcom_rpm_smd_write(struct qcom_smd_rpm *rpm,
> size_t size = sizeof(*pkt) + count;
>
> /* SMD packets to the RPM may not exceed 256 bytes */
> - if (WARN_ON(size >= 256))
> + if (WARN_ON(size >= SMD_RPM_MAX_SIZE))
> return -EINVAL;
The only thing you do is to change "oh, the max packet size is 256
bytes" to "hmm, i wonder what SMD_RPM_MAX_SIZE is and if the comment is
still valid".
>
> pkt = kmalloc(size, GFP_KERNEL);
> diff --git a/include/linux/soc/qcom/smd.h b/include/linux/soc/qcom/smd.h
> index f148e0f..8039015 100644
> --- a/include/linux/soc/qcom/smd.h
> +++ b/include/linux/soc/qcom/smd.h
> @@ -4,6 +4,13 @@
> #include <linux/device.h>
> #include <linux/mod_devicetable.h>
>
> +
> +/*
> + * SMD packets to the RPM may not exceed 256 bytes
> + */
> +#define SMD_RPM_MAX_SIZE 256
> +
And this has nothing to do with SMD, it's a limitation of RPM.
Regards,
Bjorn
^ permalink raw reply
* Re: [PATCH v2] mmc: block: delete packed command support
From: Jaehoon Chung @ 2016-11-24 5:24 UTC (permalink / raw)
To: Linus Walleij, linux-mmc, Ulf Hansson
Cc: Chunyan Zhang, Baolin Wang, Namjae Jeon, Maya Erez
In-Reply-To: <1479916086-26641-1-git-send-email-linus.walleij@linaro.org>
Hi Linus,
On 11/24/2016 12:48 AM, 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.
>
> 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>
Did i send the acked-by tag? :)
I just agreed some this patch's points.
When i become assured, i will send the acked-by tag. is this way right?
Best Regards,
Jaehoon Chung
> ---
> ChangeLog v1->v2:
> - Rebased on Ulf's next branch
> - Added Jaehoon's ACK
> ---
> drivers/mmc/card/block.c | 482 ++---------------------------------------------
> drivers/mmc/card/queue.c | 53 +-----
> drivers/mmc/card/queue.h | 19 --
> include/linux/mmc/host.h | 5 -
> 4 files changed, 20 insertions(+), 539 deletions(-)
>
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index 6618126fcb9f..3d0f1e05a425 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -66,9 +66,6 @@ MODULE_ALIAS("mmc:block");
>
> #define mmc_req_rel_wr(req) ((req->cmd_flags & REQ_FUA) && \
> (rq_data_dir(req) == WRITE))
> -#define PACKED_CMD_VER 0x01
> -#define PACKED_CMD_WR 0x02
> -
> static DEFINE_MUTEX(block_mutex);
>
> /*
> @@ -102,7 +99,6 @@ struct mmc_blk_data {
> unsigned int flags;
> #define MMC_BLK_CMD23 (1 << 0) /* Can do SET_BLOCK_COUNT for multiblock */
> #define MMC_BLK_REL_WR (1 << 1) /* MMC Reliable write support */
> -#define MMC_BLK_PACKED_CMD (1 << 2) /* MMC packed command support */
>
> unsigned int usage;
> unsigned int read_only;
> @@ -126,12 +122,6 @@ struct mmc_blk_data {
>
> static DEFINE_MUTEX(open_lock);
>
> -enum {
> - MMC_PACKED_NR_IDX = -1,
> - MMC_PACKED_NR_ZERO,
> - MMC_PACKED_NR_SINGLE,
> -};
> -
> module_param(perdev_minors, int, 0444);
> MODULE_PARM_DESC(perdev_minors, "Minors numbers to allocate per device");
>
> @@ -139,17 +129,6 @@ static inline int mmc_blk_part_switch(struct mmc_card *card,
> struct mmc_blk_data *md);
> static int get_card_status(struct mmc_card *card, u32 *status, int retries);
>
> -static inline void mmc_blk_clear_packed(struct mmc_queue_req *mqrq)
> -{
> - struct mmc_packed *packed = mqrq->packed;
> -
> - mqrq->cmd_type = MMC_PACKED_NONE;
> - packed->nr_entries = MMC_PACKED_NR_ZERO;
> - packed->idx_failure = MMC_PACKED_NR_IDX;
> - packed->retries = 0;
> - packed->blocks = 0;
> -}
> -
> static struct mmc_blk_data *mmc_blk_get(struct gendisk *disk)
> {
> struct mmc_blk_data *md;
> @@ -1420,111 +1399,12 @@ static enum mmc_blk_status mmc_blk_err_check(struct mmc_card *card,
> if (!brq->data.bytes_xfered)
> return MMC_BLK_RETRY;
>
> - if (mmc_packed_cmd(mq_mrq->cmd_type)) {
> - if (unlikely(brq->data.blocks << 9 != brq->data.bytes_xfered))
> - return MMC_BLK_PARTIAL;
> - else
> - return MMC_BLK_SUCCESS;
> - }
> -
> if (blk_rq_bytes(req) != brq->data.bytes_xfered)
> return MMC_BLK_PARTIAL;
>
> return MMC_BLK_SUCCESS;
> }
>
> -static int mmc_packed_init(struct mmc_queue *mq, struct mmc_card *card)
> -{
> - struct mmc_queue_req *mqrq_cur = &mq->mqrq[0];
> - struct mmc_queue_req *mqrq_prev = &mq->mqrq[1];
> - int ret = 0;
> -
> -
> - mqrq_cur->packed = kzalloc(sizeof(struct mmc_packed), GFP_KERNEL);
> - if (!mqrq_cur->packed) {
> - pr_warn("%s: unable to allocate packed cmd for mqrq_cur\n",
> - mmc_card_name(card));
> - ret = -ENOMEM;
> - goto out;
> - }
> -
> - mqrq_prev->packed = kzalloc(sizeof(struct mmc_packed), GFP_KERNEL);
> - if (!mqrq_prev->packed) {
> - pr_warn("%s: unable to allocate packed cmd for mqrq_prev\n",
> - mmc_card_name(card));
> - kfree(mqrq_cur->packed);
> - mqrq_cur->packed = NULL;
> - ret = -ENOMEM;
> - goto out;
> - }
> -
> - INIT_LIST_HEAD(&mqrq_cur->packed->list);
> - INIT_LIST_HEAD(&mqrq_prev->packed->list);
> -
> -out:
> - return ret;
> -}
> -
> -static void mmc_packed_clean(struct mmc_queue *mq)
> -{
> - struct mmc_queue_req *mqrq_cur = &mq->mqrq[0];
> - struct mmc_queue_req *mqrq_prev = &mq->mqrq[1];
> -
> - kfree(mqrq_cur->packed);
> - mqrq_cur->packed = NULL;
> - kfree(mqrq_prev->packed);
> - mqrq_prev->packed = NULL;
> -}
> -
> -static enum mmc_blk_status mmc_blk_packed_err_check(struct mmc_card *card,
> - struct mmc_async_req *areq)
> -{
> - struct mmc_queue_req *mq_rq = container_of(areq, struct mmc_queue_req,
> - mmc_active);
> - struct request *req = mq_rq->req;
> - struct mmc_packed *packed = mq_rq->packed;
> - enum mmc_blk_status status, check;
> - int err;
> - u8 *ext_csd;
> -
> - packed->retries--;
> - check = mmc_blk_err_check(card, areq);
> - err = get_card_status(card, &status, 0);
> - if (err) {
> - pr_err("%s: error %d sending status command\n",
> - req->rq_disk->disk_name, err);
> - return MMC_BLK_ABORT;
> - }
> -
> - if (status & R1_EXCEPTION_EVENT) {
> - err = mmc_get_ext_csd(card, &ext_csd);
> - if (err) {
> - pr_err("%s: error %d sending ext_csd\n",
> - req->rq_disk->disk_name, err);
> - return MMC_BLK_ABORT;
> - }
> -
> - if ((ext_csd[EXT_CSD_EXP_EVENTS_STATUS] &
> - EXT_CSD_PACKED_FAILURE) &&
> - (ext_csd[EXT_CSD_PACKED_CMD_STATUS] &
> - EXT_CSD_PACKED_GENERIC_ERROR)) {
> - if (ext_csd[EXT_CSD_PACKED_CMD_STATUS] &
> - EXT_CSD_PACKED_INDEXED_ERROR) {
> - packed->idx_failure =
> - ext_csd[EXT_CSD_PACKED_FAILURE_INDEX] - 1;
> - check = MMC_BLK_PARTIAL;
> - }
> - pr_err("%s: packed cmd failed, nr %u, sectors %u, "
> - "failure index: %d\n",
> - req->rq_disk->disk_name, packed->nr_entries,
> - packed->blocks, packed->idx_failure);
> - }
> - kfree(ext_csd);
> - }
> -
> - return check;
> -}
> -
> static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
> struct mmc_card *card,
> int disable_multi,
> @@ -1685,222 +1565,6 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
> mmc_queue_bounce_pre(mqrq);
> }
>
> -static inline u8 mmc_calc_packed_hdr_segs(struct request_queue *q,
> - struct mmc_card *card)
> -{
> - unsigned int hdr_sz = mmc_large_sector(card) ? 4096 : 512;
> - unsigned int max_seg_sz = queue_max_segment_size(q);
> - unsigned int len, nr_segs = 0;
> -
> - do {
> - len = min(hdr_sz, max_seg_sz);
> - hdr_sz -= len;
> - nr_segs++;
> - } while (hdr_sz);
> -
> - return nr_segs;
> -}
> -
> -static u8 mmc_blk_prep_packed_list(struct mmc_queue *mq, struct request *req)
> -{
> - struct request_queue *q = mq->queue;
> - struct mmc_card *card = mq->card;
> - struct request *cur = req, *next = NULL;
> - struct mmc_blk_data *md = mq->blkdata;
> - struct mmc_queue_req *mqrq = mq->mqrq_cur;
> - bool en_rel_wr = card->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN;
> - unsigned int req_sectors = 0, phys_segments = 0;
> - unsigned int max_blk_count, max_phys_segs;
> - bool put_back = true;
> - u8 max_packed_rw = 0;
> - u8 reqs = 0;
> -
> - /*
> - * We don't need to check packed for any further
> - * operation of packed stuff as we set MMC_PACKED_NONE
> - * and return zero for reqs if geting null packed. Also
> - * we clean the flag of MMC_BLK_PACKED_CMD to avoid doing
> - * it again when removing blk req.
> - */
> - if (!mqrq->packed) {
> - md->flags &= (~MMC_BLK_PACKED_CMD);
> - goto no_packed;
> - }
> -
> - if (!(md->flags & MMC_BLK_PACKED_CMD))
> - goto no_packed;
> -
> - 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;
> -
> - if (mmc_req_rel_wr(cur) &&
> - (md->flags & MMC_BLK_REL_WR) && !en_rel_wr)
> - goto no_packed;
> -
> - if (mmc_large_sector(card) &&
> - !IS_ALIGNED(blk_rq_sectors(cur), 8))
> - goto no_packed;
> -
> - mmc_blk_clear_packed(mqrq);
> -
> - max_blk_count = min(card->host->max_blk_count,
> - card->host->max_req_size >> 9);
> - if (unlikely(max_blk_count > 0xffff))
> - max_blk_count = 0xffff;
> -
> - max_phys_segs = queue_max_segments(q);
> - req_sectors += blk_rq_sectors(cur);
> - phys_segments += cur->nr_phys_segments;
> -
> - if (rq_data_dir(cur) == WRITE) {
> - req_sectors += mmc_large_sector(card) ? 8 : 1;
> - phys_segments += mmc_calc_packed_hdr_segs(q, card);
> - }
> -
> - do {
> - if (reqs >= max_packed_rw - 1) {
> - put_back = false;
> - break;
> - }
> -
> - spin_lock_irq(q->queue_lock);
> - next = blk_fetch_request(q);
> - spin_unlock_irq(q->queue_lock);
> - if (!next) {
> - put_back = false;
> - break;
> - }
> -
> - if (mmc_large_sector(card) &&
> - !IS_ALIGNED(blk_rq_sectors(next), 8))
> - break;
> -
> - if (mmc_req_is_special(next))
> - break;
> -
> - if (rq_data_dir(cur) != rq_data_dir(next))
> - break;
> -
> - if (mmc_req_rel_wr(next) &&
> - (md->flags & MMC_BLK_REL_WR) && !en_rel_wr)
> - break;
> -
> - req_sectors += blk_rq_sectors(next);
> - if (req_sectors > max_blk_count)
> - break;
> -
> - phys_segments += next->nr_phys_segments;
> - if (phys_segments > max_phys_segs)
> - break;
> -
> - list_add_tail(&next->queuelist, &mqrq->packed->list);
> - cur = next;
> - reqs++;
> - } while (1);
> -
> - if (put_back) {
> - spin_lock_irq(q->queue_lock);
> - blk_requeue_request(q, next);
> - spin_unlock_irq(q->queue_lock);
> - }
> -
> - if (reqs > 0) {
> - list_add(&req->queuelist, &mqrq->packed->list);
> - mqrq->packed->nr_entries = ++reqs;
> - mqrq->packed->retries = reqs;
> - return reqs;
> - }
> -
> -no_packed:
> - mqrq->cmd_type = MMC_PACKED_NONE;
> - return 0;
> -}
> -
> -static void mmc_blk_packed_hdr_wrq_prep(struct mmc_queue_req *mqrq,
> - struct mmc_card *card,
> - struct mmc_queue *mq)
> -{
> - struct mmc_blk_request *brq = &mqrq->brq;
> - struct request *req = mqrq->req;
> - struct request *prq;
> - struct mmc_blk_data *md = mq->blkdata;
> - struct mmc_packed *packed = mqrq->packed;
> - bool do_rel_wr, do_data_tag;
> - __le32 *packed_cmd_hdr;
> - u8 hdr_blocks;
> - u8 i = 1;
> -
> - mqrq->cmd_type = MMC_PACKED_WRITE;
> - packed->blocks = 0;
> - packed->idx_failure = MMC_PACKED_NR_IDX;
> -
> - packed_cmd_hdr = packed->cmd_hdr;
> - memset(packed_cmd_hdr, 0, sizeof(packed->cmd_hdr));
> - packed_cmd_hdr[0] = cpu_to_le32((packed->nr_entries << 16) |
> - (PACKED_CMD_WR << 8) | PACKED_CMD_VER);
> - hdr_blocks = mmc_large_sector(card) ? 8 : 1;
> -
> - /*
> - * Argument for each entry of packed group
> - */
> - list_for_each_entry(prq, &packed->list, queuelist) {
> - do_rel_wr = mmc_req_rel_wr(prq) && (md->flags & MMC_BLK_REL_WR);
> - do_data_tag = (card->ext_csd.data_tag_unit_size) &&
> - (prq->cmd_flags & REQ_META) &&
> - (rq_data_dir(prq) == WRITE) &&
> - blk_rq_bytes(prq) >= card->ext_csd.data_tag_unit_size;
> - /* Argument of CMD23 */
> - packed_cmd_hdr[(i * 2)] = cpu_to_le32(
> - (do_rel_wr ? MMC_CMD23_ARG_REL_WR : 0) |
> - (do_data_tag ? MMC_CMD23_ARG_TAG_REQ : 0) |
> - blk_rq_sectors(prq));
> - /* Argument of CMD18 or CMD25 */
> - packed_cmd_hdr[((i * 2)) + 1] = cpu_to_le32(
> - mmc_card_blockaddr(card) ?
> - blk_rq_pos(prq) : blk_rq_pos(prq) << 9);
> - packed->blocks += blk_rq_sectors(prq);
> - i++;
> - }
> -
> - memset(brq, 0, sizeof(struct mmc_blk_request));
> - brq->mrq.cmd = &brq->cmd;
> - brq->mrq.data = &brq->data;
> - brq->mrq.sbc = &brq->sbc;
> - brq->mrq.stop = &brq->stop;
> -
> - brq->sbc.opcode = MMC_SET_BLOCK_COUNT;
> - brq->sbc.arg = MMC_CMD23_ARG_PACKED | (packed->blocks + hdr_blocks);
> - brq->sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
> -
> - brq->cmd.opcode = MMC_WRITE_MULTIPLE_BLOCK;
> - brq->cmd.arg = blk_rq_pos(req);
> - if (!mmc_card_blockaddr(card))
> - brq->cmd.arg <<= 9;
> - brq->cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
> -
> - brq->data.blksz = 512;
> - brq->data.blocks = packed->blocks + hdr_blocks;
> - brq->data.flags = MMC_DATA_WRITE;
> -
> - brq->stop.opcode = MMC_STOP_TRANSMISSION;
> - brq->stop.arg = 0;
> - brq->stop.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC;
> -
> - mmc_set_data_timeout(&brq->data, card);
> -
> - brq->data.sg = mqrq->sg;
> - brq->data.sg_len = mmc_queue_map_sg(mq, mqrq);
> -
> - mqrq->mmc_active.mrq = &brq->mrq;
> - mqrq->mmc_active.err_check = mmc_blk_packed_err_check;
> -
> - mmc_queue_bounce_pre(mqrq);
> -}
> -
> static int mmc_blk_cmd_err(struct mmc_blk_data *md, struct mmc_card *card,
> struct mmc_blk_request *brq, struct request *req,
> int ret)
> @@ -1923,79 +1587,10 @@ static int mmc_blk_cmd_err(struct mmc_blk_data *md, struct mmc_card *card,
> if (blocks != (u32)-1) {
> ret = blk_end_request(req, 0, blocks << 9);
> }
> - } else {
> - if (!mmc_packed_cmd(mq_rq->cmd_type))
> - ret = blk_end_request(req, 0, brq->data.bytes_xfered);
> - }
> - return ret;
> -}
> -
> -static int mmc_blk_end_packed_req(struct mmc_queue_req *mq_rq)
> -{
> - struct request *prq;
> - struct mmc_packed *packed = mq_rq->packed;
> - int idx = packed->idx_failure, i = 0;
> - int ret = 0;
> -
> - while (!list_empty(&packed->list)) {
> - prq = list_entry_rq(packed->list.next);
> - if (idx == i) {
> - /* retry from error index */
> - packed->nr_entries -= idx;
> - mq_rq->req = prq;
> - ret = 1;
> -
> - if (packed->nr_entries == MMC_PACKED_NR_SINGLE) {
> - list_del_init(&prq->queuelist);
> - mmc_blk_clear_packed(mq_rq);
> - }
> - return ret;
> - }
> - list_del_init(&prq->queuelist);
> - blk_end_request(prq, 0, blk_rq_bytes(prq));
> - i++;
> }
> -
> - mmc_blk_clear_packed(mq_rq);
> return ret;
> }
>
> -static void mmc_blk_abort_packed_req(struct mmc_queue_req *mq_rq)
> -{
> - struct request *prq;
> - struct mmc_packed *packed = mq_rq->packed;
> -
> - while (!list_empty(&packed->list)) {
> - prq = list_entry_rq(packed->list.next);
> - list_del_init(&prq->queuelist);
> - blk_end_request(prq, -EIO, blk_rq_bytes(prq));
> - }
> -
> - mmc_blk_clear_packed(mq_rq);
> -}
> -
> -static void mmc_blk_revert_packed_req(struct mmc_queue *mq,
> - struct mmc_queue_req *mq_rq)
> -{
> - struct request *prq;
> - struct request_queue *q = mq->queue;
> - struct mmc_packed *packed = mq_rq->packed;
> -
> - while (!list_empty(&packed->list)) {
> - prq = list_entry_rq(packed->list.prev);
> - if (prq->queuelist.prev != &packed->list) {
> - list_del_init(&prq->queuelist);
> - spin_lock_irq(q->queue_lock);
> - blk_requeue_request(mq->queue, prq);
> - spin_unlock_irq(q->queue_lock);
> - } else {
> - list_del_init(&prq->queuelist);
> - }
> - }
> -
> - mmc_blk_clear_packed(mq_rq);
> -}
> -
> static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
> {
> struct mmc_blk_data *md = mq->blkdata;
> @@ -2006,15 +1601,11 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
> struct mmc_queue_req *mq_rq;
> struct request *req = rqc;
> struct mmc_async_req *areq;
> - const u8 packed_nr = 2;
> u8 reqs = 0;
>
> if (!rqc && !mq->mqrq_prev->req)
> return 0;
>
> - if (rqc)
> - reqs = mmc_blk_prep_packed_list(mq, rqc);
> -
> do {
> if (rqc) {
> /*
> @@ -2029,11 +1620,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
> goto cmd_abort;
> }
>
> - if (reqs >= packed_nr)
> - mmc_blk_packed_hdr_wrq_prep(mq->mqrq_cur,
> - card, mq);
> - else
> - mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
> + mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
> areq = &mq->mqrq_cur->mmc_active;
> } else
> areq = NULL;
> @@ -2058,13 +1645,8 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
> */
> mmc_blk_reset_success(md, type);
>
> - if (mmc_packed_cmd(mq_rq->cmd_type)) {
> - ret = mmc_blk_end_packed_req(mq_rq);
> - break;
> - } else {
> - ret = blk_end_request(req, 0,
> - brq->data.bytes_xfered);
> - }
> + ret = blk_end_request(req, 0,
> + brq->data.bytes_xfered);
>
> /*
> * If the blk_end_request function returns non-zero even
> @@ -2101,8 +1683,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
> err = mmc_blk_reset(md, card->host, type);
> if (!err)
> break;
> - if (err == -ENODEV ||
> - mmc_packed_cmd(mq_rq->cmd_type))
> + if (err == -ENODEV)
> goto cmd_abort;
> /* Fall through */
> }
> @@ -2133,23 +1714,14 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
> }
>
> if (ret) {
> - if (mmc_packed_cmd(mq_rq->cmd_type)) {
> - if (!mq_rq->packed->retries)
> - goto cmd_abort;
> - mmc_blk_packed_hdr_wrq_prep(mq_rq, card, mq);
> - mmc_start_req(card->host,
> - &mq_rq->mmc_active, NULL);
> - } else {
> -
> - /*
> - * In case of a incomplete request
> - * prepare it again and resend.
> - */
> - mmc_blk_rw_rq_prep(mq_rq, card,
> - disable_multi, mq);
> - mmc_start_req(card->host,
> - &mq_rq->mmc_active, NULL);
> - }
> + /*
> + * In case of a incomplete request
> + * prepare it again and resend.
> + */
> + mmc_blk_rw_rq_prep(mq_rq, card,
> + disable_multi, mq);
> + mmc_start_req(card->host,
> + &mq_rq->mmc_active, NULL);
> mq_rq->brq.retune_retry_done = retune_retry_done;
> }
> } while (ret);
> @@ -2157,15 +1729,11 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
> return 1;
>
> cmd_abort:
> - if (mmc_packed_cmd(mq_rq->cmd_type)) {
> - mmc_blk_abort_packed_req(mq_rq);
> - } else {
> - if (mmc_card_removed(card))
> - req->cmd_flags |= REQ_QUIET;
> - while (ret)
> - ret = blk_end_request(req, -EIO,
> - blk_rq_cur_bytes(req));
> - }
> + if (mmc_card_removed(card))
> + req->cmd_flags |= REQ_QUIET;
> + while (ret)
> + ret = blk_end_request(req, -EIO,
> + blk_rq_cur_bytes(req));
>
> start_new_req:
> if (rqc) {
> @@ -2173,12 +1741,6 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
> rqc->cmd_flags |= REQ_QUIET;
> blk_end_request_all(rqc, -EIO);
> } else {
> - /*
> - * If current request is packed, it needs to put back.
> - */
> - if (mmc_packed_cmd(mq->mqrq_cur->cmd_type))
> - mmc_blk_revert_packed_req(mq, mq->mqrq_cur);
> -
> mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
> mmc_start_req(card->host,
> &mq->mqrq_cur->mmc_active, NULL);
> @@ -2361,14 +1923,6 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
> blk_queue_write_cache(md->queue.queue, true, true);
> }
>
> - if (mmc_card_mmc(card) &&
> - (area_type == MMC_BLK_DATA_AREA_MAIN) &&
> - (md->flags & MMC_BLK_CMD23) &&
> - card->ext_csd.packed_event_en) {
> - if (!mmc_packed_init(&md->queue, card))
> - md->flags |= MMC_BLK_PACKED_CMD;
> - }
> -
> return md;
>
> err_putdisk:
> @@ -2472,8 +2026,6 @@ static void mmc_blk_remove_req(struct mmc_blk_data *md)
> */
> card = md->queue.card;
> mmc_cleanup_queue(&md->queue);
> - if (md->flags & MMC_BLK_PACKED_CMD)
> - mmc_packed_clean(&md->queue);
> if (md->disk->flags & GENHD_FL_UP) {
> device_remove_file(disk_to_dev(md->disk), &md->force_ro);
> if ((md->area_type & MMC_BLK_DATA_AREA_BOOT) &&
> diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
> index 3f6a2463ab30..7dacf2744fbd 100644
> --- a/drivers/mmc/card/queue.c
> +++ b/drivers/mmc/card/queue.c
> @@ -406,41 +406,6 @@ void mmc_queue_resume(struct mmc_queue *mq)
> }
> }
>
> -static unsigned int mmc_queue_packed_map_sg(struct mmc_queue *mq,
> - struct mmc_packed *packed,
> - struct scatterlist *sg,
> - enum mmc_packed_type cmd_type)
> -{
> - struct scatterlist *__sg = sg;
> - unsigned int sg_len = 0;
> - struct request *req;
> -
> - if (mmc_packed_wr(cmd_type)) {
> - unsigned int hdr_sz = mmc_large_sector(mq->card) ? 4096 : 512;
> - unsigned int max_seg_sz = queue_max_segment_size(mq->queue);
> - unsigned int len, remain, offset = 0;
> - u8 *buf = (u8 *)packed->cmd_hdr;
> -
> - remain = hdr_sz;
> - do {
> - len = min(remain, max_seg_sz);
> - sg_set_buf(__sg, buf + offset, len);
> - offset += len;
> - remain -= len;
> - sg_unmark_end(__sg++);
> - sg_len++;
> - } while (remain);
> - }
> -
> - list_for_each_entry(req, &packed->list, queuelist) {
> - sg_len += blk_rq_map_sg(mq->queue, req, __sg);
> - __sg = sg + (sg_len - 1);
> - sg_unmark_end(__sg++);
> - }
> - sg_mark_end(sg + (sg_len - 1));
> - return sg_len;
> -}
> -
> /*
> * Prepare the sg list(s) to be handed of to the host driver
> */
> @@ -449,26 +414,14 @@ unsigned int mmc_queue_map_sg(struct mmc_queue *mq, struct mmc_queue_req *mqrq)
> unsigned int sg_len;
> size_t buflen;
> struct scatterlist *sg;
> - enum mmc_packed_type cmd_type;
> int i;
>
> - cmd_type = mqrq->cmd_type;
> -
> - if (!mqrq->bounce_buf) {
> - if (mmc_packed_cmd(cmd_type))
> - return mmc_queue_packed_map_sg(mq, mqrq->packed,
> - mqrq->sg, cmd_type);
> - else
> - return blk_rq_map_sg(mq->queue, mqrq->req, mqrq->sg);
> - }
> + if (!mqrq->bounce_buf)
> + return blk_rq_map_sg(mq->queue, mqrq->req, mqrq->sg);
>
> BUG_ON(!mqrq->bounce_sg);
>
> - if (mmc_packed_cmd(cmd_type))
> - sg_len = mmc_queue_packed_map_sg(mq, mqrq->packed,
> - mqrq->bounce_sg, cmd_type);
> - else
> - sg_len = blk_rq_map_sg(mq->queue, mqrq->req, mqrq->bounce_sg);
> + sg_len = blk_rq_map_sg(mq->queue, mqrq->req, mqrq->bounce_sg);
>
> mqrq->bounce_sg_len = sg_len;
>
> diff --git a/drivers/mmc/card/queue.h b/drivers/mmc/card/queue.h
> index 334c9306070f..47f5532b5776 100644
> --- a/drivers/mmc/card/queue.h
> +++ b/drivers/mmc/card/queue.h
> @@ -22,23 +22,6 @@ struct mmc_blk_request {
> int retune_retry_done;
> };
>
> -enum mmc_packed_type {
> - MMC_PACKED_NONE = 0,
> - MMC_PACKED_WRITE,
> -};
> -
> -#define mmc_packed_cmd(type) ((type) != MMC_PACKED_NONE)
> -#define mmc_packed_wr(type) ((type) == MMC_PACKED_WRITE)
> -
> -struct mmc_packed {
> - struct list_head list;
> - __le32 cmd_hdr[1024];
> - unsigned int blocks;
> - u8 nr_entries;
> - u8 retries;
> - s16 idx_failure;
> -};
> -
> struct mmc_queue_req {
> struct request *req;
> struct mmc_blk_request brq;
> @@ -47,8 +30,6 @@ struct mmc_queue_req {
> struct scatterlist *bounce_sg;
> unsigned int bounce_sg_len;
> struct mmc_async_req mmc_active;
> - enum mmc_packed_type cmd_type;
> - struct mmc_packed *packed;
> };
>
> struct mmc_queue {
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 2a6418d0c343..2ce32fefb41c 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -494,11 +494,6 @@ static inline int mmc_host_uhs(struct mmc_host *host)
> MMC_CAP_UHS_DDR50);
> }
>
> -static inline int mmc_host_packed_wr(struct mmc_host *host)
> -{
> - return host->caps2 & MMC_CAP2_PACKED_WR;
> -}
> -
> static inline int mmc_card_hs(struct mmc_card *card)
> {
> return card->host->ios.timing == MMC_TIMING_SD_HS ||
>
^ permalink raw reply
* Re: [PATCH v6 3/5] Documentation: synopsys-dw-mshc: add binding for fifo quirks
From: Jun Nie @ 2016-11-24 2:19 UTC (permalink / raw)
To: Shawn Guo, xie.baoyou, Rob Herring, mark.rutland
Cc: Ulf Hansson, Jaehoon Chung, Jason Liu, chen.chaokai, lai.binz,
linux-mmc, Jun Nie, devicetree
In-Reply-To: <1479450555-19047-4-git-send-email-jun.nie@linaro.org>
2016-11-18 14:29 GMT+08:00 Jun Nie <jun.nie@linaro.org>:
> Add fifo-addr property and fifo-watermark-quirk property to
> synopsys-dw-mshc bindings. It is intended to provide more
> dt interface to support SoCs specific configuration.
>
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
> Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
> index 4e00e85..8bf2e41 100644
> --- a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
> +++ b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt
> @@ -76,6 +76,17 @@ Optional properties:
>
> * broken-cd: as documented in mmc core bindings.
>
> +* data-addr: Override fifo address with value provided by DT. The default FIFO reg
> + offset is assumed as 0x100 (version < 0x240A) and 0x200(version >= 0x240A) by
> + driver. If the controller does not follow this rule, please use this property
> + to set fifo address in device tree.
> +
> +* fifo-watermark-aligned: Data done irq is expected if data length is less than
> + watermark in PIO mode. But fifo watermark is requested to be aligned with data
> + length in some SoC so that TX/RX irq can be generated with data done irq. Add this
> + watermark quirk to mark this requirement and force fifo watermark setting
> + accordingly.
> +
> * vmmc-supply: The phandle to the regulator to use for vmmc. If this is
> specified we'll defer probe until we can find this regulator.
>
> @@ -103,6 +114,8 @@ board specific portions as listed below.
> interrupts = <0 75 0>;
> #address-cells = <1>;
> #size-cells = <0>;
> + data-addr = <0x200>;
> + fifo-watermark-aligned;
> };
>
> [board specific internal DMA resources]
> --
> 1.9.1
>
Hi Rob & Mark,
Could you help review and act this patch if you think it is OK? Thank you!
Jun
^ permalink raw reply
* Re: [PATCH v6 1/5] mmc: dt-bindings: add ZTE ZX296718 MMC bindings
From: Jun Nie @ 2016-11-24 2:17 UTC (permalink / raw)
To: Shawn Guo, xie.baoyou, Rob Herring, mark.rutland
Cc: Ulf Hansson, Jaehoon Chung, Jason Liu, chen.chaokai, lai.binz,
linux-mmc, Jun Nie, devicetree
In-Reply-To: <1479450555-19047-2-git-send-email-jun.nie@linaro.org>
2016-11-18 14:29 GMT+08:00 Jun Nie <jun.nie@linaro.org>:
> Document the device-tree binding of ZTE MMC host on
> ZX296718 SoC.
>
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
> .../devicetree/bindings/mmc/zx-dw-mshc.txt | 35 ++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>
> diff --git a/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
> new file mode 100644
> index 0000000..c175c4b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
> @@ -0,0 +1,35 @@
> +* ZTE specific extensions to the Synopsys Designware Mobile Storage
> + Host Controller
> +
> +The Synopsys designware mobile storage host controller is used to interface
> +a SoC with storage medium such as eMMC or SD/MMC cards. This file documents
> +differences between the core Synopsys dw mshc controller properties described
> +by synopsys-dw-mshc.txt and the properties used by the ZTE specific
> +extensions to the Synopsys Designware Mobile Storage Host Controller.
> +
> +Required Properties:
> +
> +* compatible: should be
> + - "zte,zx296718-dw-mshc": for ZX SoCs
> +
> +Example:
> +
> + mmc1: mmc@1110000 {
> + compatible = "zte,zx296718-dw-mshc";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x01110000 0x1000>;
> + interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
> + fifo-depth = <32>;
> + data-addr = <0x200>;
> + fifo-watermark-aligned;
> + bus-width = <4>;
> + clock-frequency = <50000000>;
> + clocks = <&topcrm SD0_AHB>, <&topcrm SD0_WCLK>;
> + clock-names = "biu", "ciu";
> + num-slots = <1>;
> + max-frequency = <50000000>;
> + cap-sdio-irq;
> + cap-sd-highspeed;
> + status = "disabled";
> + };
> --
> 1.9.1
>
Hi Rob & Mark,
Could you help review and act this patch if you think it is OK? Thank you!
Jun
^ permalink raw reply
* Re: [PATCH v9 02/16] clk: qcom: Move all sdcc rcgs to use clk_rcg2_floor_ops
From: Stephen Boyd @ 2016-11-23 19:00 UTC (permalink / raw)
To: Ritesh Harjani
Cc: ulf.hansson, linux-mmc, adrian.hunter, andy.gross, shawn.lin,
devicetree, linux-clk, david.brown, linux-arm-msm, georgi.djakov,
alex.lemberg, mateusz.nowak, Yuliy.Izrailov, asutoshd,
david.griego, stummala, venkatg, rnayak, pramod.gurav, jeremymc
In-Reply-To: <1479710246-26676-3-git-send-email-riteshh@codeaurora.org>
On 11/21, Ritesh Harjani wrote:
> From: Rajendra Nayak <rnayak@codeaurora.org>
>
> The sdcc driver for msm8996/msm8916/msm8974/msm8994 and apq8084
> expects a clk_set_rate() on the sdcc rcg clk to set
> a floor value of supported clk rate closest to the requested
> rate, by looking up the frequency table.
> So move all the sdcc rcgs on all these platforms to use the
> newly introduced clk_rcg2_floor_ops
>
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
> Signed-off-by: Jeremy McNicoll <jeremymc@redhat.com>
> ---
Applied to clk-next
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH v9 01/16] clk: qcom: Add rcg ops to return floor value closest to the requested rate
From: Stephen Boyd @ 2016-11-23 19:00 UTC (permalink / raw)
To: Ritesh Harjani
Cc: ulf.hansson-QSEj5FYQhm4dnm+yROfE0A,
linux-mmc-u79uwXL29TY76Z2rM5mHXA,
adrian.hunter-ral2JQCrhuEAvxtiuMwx3w,
andy.gross-QSEj5FYQhm4dnm+yROfE0A,
shawn.lin-TNX95d0MmH7DzftRWevZcw,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
david.brown-QSEj5FYQhm4dnm+yROfE0A,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
georgi.djakov-QSEj5FYQhm4dnm+yROfE0A,
alex.lemberg-XdAiOPVOjttBDgjK7y7TUQ,
mateusz.nowak-ral2JQCrhuEAvxtiuMwx3w,
Yuliy.Izrailov-XdAiOPVOjttBDgjK7y7TUQ,
asutoshd-sgV2jX0FEOL9JmXXK+q4OQ,
david.griego-QSEj5FYQhm4dnm+yROfE0A,
stummala-sgV2jX0FEOL9JmXXK+q4OQ, venkatg-sgV2jX0FEOL9JmXXK+q4OQ,
rnayak-sgV2jX0FEOL9JmXXK+q4OQ,
pramod.gurav-QSEj5FYQhm4dnm+yROfE0A,
jeremymc-H+wXaHxf7aLQT0dZR+AlfA
In-Reply-To: <1479710246-26676-2-git-send-email-riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On 11/21, Ritesh Harjani wrote:
> From: Rajendra Nayak <rnayak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>
> The default behaviour with clk_rcg2_ops is for the
> clk_round_rate()/clk_set_rate() to return/set a ceil clock
> rate closest to the requested rate by looking up the corresponding
> frequency table.
> However, we do have some instances (mainly sdcc on various platforms)
> of clients expecting a clk_set_rate() to set a floor value instead.
> Add a new clk_rcg2_floor_ops to handle this for such specific
> rcg instances
>
> Signed-off-by: Rajendra Nayak <rnayak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Signed-off-by: Ritesh Harjani <riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
Applied to clk-next
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v2] mmc: block: delete packed command support
From: Linus Walleij @ 2016-11-23 15:48 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson
Cc: Chunyan Zhang, Baolin Wang, Linus Walleij, Namjae Jeon, Maya Erez
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>
---
ChangeLog v1->v2:
- Rebased on Ulf's next branch
- Added Jaehoon's ACK
---
drivers/mmc/card/block.c | 482 ++---------------------------------------------
drivers/mmc/card/queue.c | 53 +-----
drivers/mmc/card/queue.h | 19 --
include/linux/mmc/host.h | 5 -
4 files changed, 20 insertions(+), 539 deletions(-)
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 6618126fcb9f..3d0f1e05a425 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -66,9 +66,6 @@ MODULE_ALIAS("mmc:block");
#define mmc_req_rel_wr(req) ((req->cmd_flags & REQ_FUA) && \
(rq_data_dir(req) == WRITE))
-#define PACKED_CMD_VER 0x01
-#define PACKED_CMD_WR 0x02
-
static DEFINE_MUTEX(block_mutex);
/*
@@ -102,7 +99,6 @@ struct mmc_blk_data {
unsigned int flags;
#define MMC_BLK_CMD23 (1 << 0) /* Can do SET_BLOCK_COUNT for multiblock */
#define MMC_BLK_REL_WR (1 << 1) /* MMC Reliable write support */
-#define MMC_BLK_PACKED_CMD (1 << 2) /* MMC packed command support */
unsigned int usage;
unsigned int read_only;
@@ -126,12 +122,6 @@ struct mmc_blk_data {
static DEFINE_MUTEX(open_lock);
-enum {
- MMC_PACKED_NR_IDX = -1,
- MMC_PACKED_NR_ZERO,
- MMC_PACKED_NR_SINGLE,
-};
-
module_param(perdev_minors, int, 0444);
MODULE_PARM_DESC(perdev_minors, "Minors numbers to allocate per device");
@@ -139,17 +129,6 @@ static inline int mmc_blk_part_switch(struct mmc_card *card,
struct mmc_blk_data *md);
static int get_card_status(struct mmc_card *card, u32 *status, int retries);
-static inline void mmc_blk_clear_packed(struct mmc_queue_req *mqrq)
-{
- struct mmc_packed *packed = mqrq->packed;
-
- mqrq->cmd_type = MMC_PACKED_NONE;
- packed->nr_entries = MMC_PACKED_NR_ZERO;
- packed->idx_failure = MMC_PACKED_NR_IDX;
- packed->retries = 0;
- packed->blocks = 0;
-}
-
static struct mmc_blk_data *mmc_blk_get(struct gendisk *disk)
{
struct mmc_blk_data *md;
@@ -1420,111 +1399,12 @@ static enum mmc_blk_status mmc_blk_err_check(struct mmc_card *card,
if (!brq->data.bytes_xfered)
return MMC_BLK_RETRY;
- if (mmc_packed_cmd(mq_mrq->cmd_type)) {
- if (unlikely(brq->data.blocks << 9 != brq->data.bytes_xfered))
- return MMC_BLK_PARTIAL;
- else
- return MMC_BLK_SUCCESS;
- }
-
if (blk_rq_bytes(req) != brq->data.bytes_xfered)
return MMC_BLK_PARTIAL;
return MMC_BLK_SUCCESS;
}
-static int mmc_packed_init(struct mmc_queue *mq, struct mmc_card *card)
-{
- struct mmc_queue_req *mqrq_cur = &mq->mqrq[0];
- struct mmc_queue_req *mqrq_prev = &mq->mqrq[1];
- int ret = 0;
-
-
- mqrq_cur->packed = kzalloc(sizeof(struct mmc_packed), GFP_KERNEL);
- if (!mqrq_cur->packed) {
- pr_warn("%s: unable to allocate packed cmd for mqrq_cur\n",
- mmc_card_name(card));
- ret = -ENOMEM;
- goto out;
- }
-
- mqrq_prev->packed = kzalloc(sizeof(struct mmc_packed), GFP_KERNEL);
- if (!mqrq_prev->packed) {
- pr_warn("%s: unable to allocate packed cmd for mqrq_prev\n",
- mmc_card_name(card));
- kfree(mqrq_cur->packed);
- mqrq_cur->packed = NULL;
- ret = -ENOMEM;
- goto out;
- }
-
- INIT_LIST_HEAD(&mqrq_cur->packed->list);
- INIT_LIST_HEAD(&mqrq_prev->packed->list);
-
-out:
- return ret;
-}
-
-static void mmc_packed_clean(struct mmc_queue *mq)
-{
- struct mmc_queue_req *mqrq_cur = &mq->mqrq[0];
- struct mmc_queue_req *mqrq_prev = &mq->mqrq[1];
-
- kfree(mqrq_cur->packed);
- mqrq_cur->packed = NULL;
- kfree(mqrq_prev->packed);
- mqrq_prev->packed = NULL;
-}
-
-static enum mmc_blk_status mmc_blk_packed_err_check(struct mmc_card *card,
- struct mmc_async_req *areq)
-{
- struct mmc_queue_req *mq_rq = container_of(areq, struct mmc_queue_req,
- mmc_active);
- struct request *req = mq_rq->req;
- struct mmc_packed *packed = mq_rq->packed;
- enum mmc_blk_status status, check;
- int err;
- u8 *ext_csd;
-
- packed->retries--;
- check = mmc_blk_err_check(card, areq);
- err = get_card_status(card, &status, 0);
- if (err) {
- pr_err("%s: error %d sending status command\n",
- req->rq_disk->disk_name, err);
- return MMC_BLK_ABORT;
- }
-
- if (status & R1_EXCEPTION_EVENT) {
- err = mmc_get_ext_csd(card, &ext_csd);
- if (err) {
- pr_err("%s: error %d sending ext_csd\n",
- req->rq_disk->disk_name, err);
- return MMC_BLK_ABORT;
- }
-
- if ((ext_csd[EXT_CSD_EXP_EVENTS_STATUS] &
- EXT_CSD_PACKED_FAILURE) &&
- (ext_csd[EXT_CSD_PACKED_CMD_STATUS] &
- EXT_CSD_PACKED_GENERIC_ERROR)) {
- if (ext_csd[EXT_CSD_PACKED_CMD_STATUS] &
- EXT_CSD_PACKED_INDEXED_ERROR) {
- packed->idx_failure =
- ext_csd[EXT_CSD_PACKED_FAILURE_INDEX] - 1;
- check = MMC_BLK_PARTIAL;
- }
- pr_err("%s: packed cmd failed, nr %u, sectors %u, "
- "failure index: %d\n",
- req->rq_disk->disk_name, packed->nr_entries,
- packed->blocks, packed->idx_failure);
- }
- kfree(ext_csd);
- }
-
- return check;
-}
-
static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
struct mmc_card *card,
int disable_multi,
@@ -1685,222 +1565,6 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
mmc_queue_bounce_pre(mqrq);
}
-static inline u8 mmc_calc_packed_hdr_segs(struct request_queue *q,
- struct mmc_card *card)
-{
- unsigned int hdr_sz = mmc_large_sector(card) ? 4096 : 512;
- unsigned int max_seg_sz = queue_max_segment_size(q);
- unsigned int len, nr_segs = 0;
-
- do {
- len = min(hdr_sz, max_seg_sz);
- hdr_sz -= len;
- nr_segs++;
- } while (hdr_sz);
-
- return nr_segs;
-}
-
-static u8 mmc_blk_prep_packed_list(struct mmc_queue *mq, struct request *req)
-{
- struct request_queue *q = mq->queue;
- struct mmc_card *card = mq->card;
- struct request *cur = req, *next = NULL;
- struct mmc_blk_data *md = mq->blkdata;
- struct mmc_queue_req *mqrq = mq->mqrq_cur;
- bool en_rel_wr = card->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN;
- unsigned int req_sectors = 0, phys_segments = 0;
- unsigned int max_blk_count, max_phys_segs;
- bool put_back = true;
- u8 max_packed_rw = 0;
- u8 reqs = 0;
-
- /*
- * We don't need to check packed for any further
- * operation of packed stuff as we set MMC_PACKED_NONE
- * and return zero for reqs if geting null packed. Also
- * we clean the flag of MMC_BLK_PACKED_CMD to avoid doing
- * it again when removing blk req.
- */
- if (!mqrq->packed) {
- md->flags &= (~MMC_BLK_PACKED_CMD);
- goto no_packed;
- }
-
- if (!(md->flags & MMC_BLK_PACKED_CMD))
- goto no_packed;
-
- 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;
-
- if (mmc_req_rel_wr(cur) &&
- (md->flags & MMC_BLK_REL_WR) && !en_rel_wr)
- goto no_packed;
-
- if (mmc_large_sector(card) &&
- !IS_ALIGNED(blk_rq_sectors(cur), 8))
- goto no_packed;
-
- mmc_blk_clear_packed(mqrq);
-
- max_blk_count = min(card->host->max_blk_count,
- card->host->max_req_size >> 9);
- if (unlikely(max_blk_count > 0xffff))
- max_blk_count = 0xffff;
-
- max_phys_segs = queue_max_segments(q);
- req_sectors += blk_rq_sectors(cur);
- phys_segments += cur->nr_phys_segments;
-
- if (rq_data_dir(cur) == WRITE) {
- req_sectors += mmc_large_sector(card) ? 8 : 1;
- phys_segments += mmc_calc_packed_hdr_segs(q, card);
- }
-
- do {
- if (reqs >= max_packed_rw - 1) {
- put_back = false;
- break;
- }
-
- spin_lock_irq(q->queue_lock);
- next = blk_fetch_request(q);
- spin_unlock_irq(q->queue_lock);
- if (!next) {
- put_back = false;
- break;
- }
-
- if (mmc_large_sector(card) &&
- !IS_ALIGNED(blk_rq_sectors(next), 8))
- break;
-
- if (mmc_req_is_special(next))
- break;
-
- if (rq_data_dir(cur) != rq_data_dir(next))
- break;
-
- if (mmc_req_rel_wr(next) &&
- (md->flags & MMC_BLK_REL_WR) && !en_rel_wr)
- break;
-
- req_sectors += blk_rq_sectors(next);
- if (req_sectors > max_blk_count)
- break;
-
- phys_segments += next->nr_phys_segments;
- if (phys_segments > max_phys_segs)
- break;
-
- list_add_tail(&next->queuelist, &mqrq->packed->list);
- cur = next;
- reqs++;
- } while (1);
-
- if (put_back) {
- spin_lock_irq(q->queue_lock);
- blk_requeue_request(q, next);
- spin_unlock_irq(q->queue_lock);
- }
-
- if (reqs > 0) {
- list_add(&req->queuelist, &mqrq->packed->list);
- mqrq->packed->nr_entries = ++reqs;
- mqrq->packed->retries = reqs;
- return reqs;
- }
-
-no_packed:
- mqrq->cmd_type = MMC_PACKED_NONE;
- return 0;
-}
-
-static void mmc_blk_packed_hdr_wrq_prep(struct mmc_queue_req *mqrq,
- struct mmc_card *card,
- struct mmc_queue *mq)
-{
- struct mmc_blk_request *brq = &mqrq->brq;
- struct request *req = mqrq->req;
- struct request *prq;
- struct mmc_blk_data *md = mq->blkdata;
- struct mmc_packed *packed = mqrq->packed;
- bool do_rel_wr, do_data_tag;
- __le32 *packed_cmd_hdr;
- u8 hdr_blocks;
- u8 i = 1;
-
- mqrq->cmd_type = MMC_PACKED_WRITE;
- packed->blocks = 0;
- packed->idx_failure = MMC_PACKED_NR_IDX;
-
- packed_cmd_hdr = packed->cmd_hdr;
- memset(packed_cmd_hdr, 0, sizeof(packed->cmd_hdr));
- packed_cmd_hdr[0] = cpu_to_le32((packed->nr_entries << 16) |
- (PACKED_CMD_WR << 8) | PACKED_CMD_VER);
- hdr_blocks = mmc_large_sector(card) ? 8 : 1;
-
- /*
- * Argument for each entry of packed group
- */
- list_for_each_entry(prq, &packed->list, queuelist) {
- do_rel_wr = mmc_req_rel_wr(prq) && (md->flags & MMC_BLK_REL_WR);
- do_data_tag = (card->ext_csd.data_tag_unit_size) &&
- (prq->cmd_flags & REQ_META) &&
- (rq_data_dir(prq) == WRITE) &&
- blk_rq_bytes(prq) >= card->ext_csd.data_tag_unit_size;
- /* Argument of CMD23 */
- packed_cmd_hdr[(i * 2)] = cpu_to_le32(
- (do_rel_wr ? MMC_CMD23_ARG_REL_WR : 0) |
- (do_data_tag ? MMC_CMD23_ARG_TAG_REQ : 0) |
- blk_rq_sectors(prq));
- /* Argument of CMD18 or CMD25 */
- packed_cmd_hdr[((i * 2)) + 1] = cpu_to_le32(
- mmc_card_blockaddr(card) ?
- blk_rq_pos(prq) : blk_rq_pos(prq) << 9);
- packed->blocks += blk_rq_sectors(prq);
- i++;
- }
-
- memset(brq, 0, sizeof(struct mmc_blk_request));
- brq->mrq.cmd = &brq->cmd;
- brq->mrq.data = &brq->data;
- brq->mrq.sbc = &brq->sbc;
- brq->mrq.stop = &brq->stop;
-
- brq->sbc.opcode = MMC_SET_BLOCK_COUNT;
- brq->sbc.arg = MMC_CMD23_ARG_PACKED | (packed->blocks + hdr_blocks);
- brq->sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
-
- brq->cmd.opcode = MMC_WRITE_MULTIPLE_BLOCK;
- brq->cmd.arg = blk_rq_pos(req);
- if (!mmc_card_blockaddr(card))
- brq->cmd.arg <<= 9;
- brq->cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
-
- brq->data.blksz = 512;
- brq->data.blocks = packed->blocks + hdr_blocks;
- brq->data.flags = MMC_DATA_WRITE;
-
- brq->stop.opcode = MMC_STOP_TRANSMISSION;
- brq->stop.arg = 0;
- brq->stop.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC;
-
- mmc_set_data_timeout(&brq->data, card);
-
- brq->data.sg = mqrq->sg;
- brq->data.sg_len = mmc_queue_map_sg(mq, mqrq);
-
- mqrq->mmc_active.mrq = &brq->mrq;
- mqrq->mmc_active.err_check = mmc_blk_packed_err_check;
-
- mmc_queue_bounce_pre(mqrq);
-}
-
static int mmc_blk_cmd_err(struct mmc_blk_data *md, struct mmc_card *card,
struct mmc_blk_request *brq, struct request *req,
int ret)
@@ -1923,79 +1587,10 @@ static int mmc_blk_cmd_err(struct mmc_blk_data *md, struct mmc_card *card,
if (blocks != (u32)-1) {
ret = blk_end_request(req, 0, blocks << 9);
}
- } else {
- if (!mmc_packed_cmd(mq_rq->cmd_type))
- ret = blk_end_request(req, 0, brq->data.bytes_xfered);
- }
- return ret;
-}
-
-static int mmc_blk_end_packed_req(struct mmc_queue_req *mq_rq)
-{
- struct request *prq;
- struct mmc_packed *packed = mq_rq->packed;
- int idx = packed->idx_failure, i = 0;
- int ret = 0;
-
- while (!list_empty(&packed->list)) {
- prq = list_entry_rq(packed->list.next);
- if (idx == i) {
- /* retry from error index */
- packed->nr_entries -= idx;
- mq_rq->req = prq;
- ret = 1;
-
- if (packed->nr_entries == MMC_PACKED_NR_SINGLE) {
- list_del_init(&prq->queuelist);
- mmc_blk_clear_packed(mq_rq);
- }
- return ret;
- }
- list_del_init(&prq->queuelist);
- blk_end_request(prq, 0, blk_rq_bytes(prq));
- i++;
}
-
- mmc_blk_clear_packed(mq_rq);
return ret;
}
-static void mmc_blk_abort_packed_req(struct mmc_queue_req *mq_rq)
-{
- struct request *prq;
- struct mmc_packed *packed = mq_rq->packed;
-
- while (!list_empty(&packed->list)) {
- prq = list_entry_rq(packed->list.next);
- list_del_init(&prq->queuelist);
- blk_end_request(prq, -EIO, blk_rq_bytes(prq));
- }
-
- mmc_blk_clear_packed(mq_rq);
-}
-
-static void mmc_blk_revert_packed_req(struct mmc_queue *mq,
- struct mmc_queue_req *mq_rq)
-{
- struct request *prq;
- struct request_queue *q = mq->queue;
- struct mmc_packed *packed = mq_rq->packed;
-
- while (!list_empty(&packed->list)) {
- prq = list_entry_rq(packed->list.prev);
- if (prq->queuelist.prev != &packed->list) {
- list_del_init(&prq->queuelist);
- spin_lock_irq(q->queue_lock);
- blk_requeue_request(mq->queue, prq);
- spin_unlock_irq(q->queue_lock);
- } else {
- list_del_init(&prq->queuelist);
- }
- }
-
- mmc_blk_clear_packed(mq_rq);
-}
-
static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
{
struct mmc_blk_data *md = mq->blkdata;
@@ -2006,15 +1601,11 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
struct mmc_queue_req *mq_rq;
struct request *req = rqc;
struct mmc_async_req *areq;
- const u8 packed_nr = 2;
u8 reqs = 0;
if (!rqc && !mq->mqrq_prev->req)
return 0;
- if (rqc)
- reqs = mmc_blk_prep_packed_list(mq, rqc);
-
do {
if (rqc) {
/*
@@ -2029,11 +1620,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
goto cmd_abort;
}
- if (reqs >= packed_nr)
- mmc_blk_packed_hdr_wrq_prep(mq->mqrq_cur,
- card, mq);
- else
- mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
+ mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
areq = &mq->mqrq_cur->mmc_active;
} else
areq = NULL;
@@ -2058,13 +1645,8 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
*/
mmc_blk_reset_success(md, type);
- if (mmc_packed_cmd(mq_rq->cmd_type)) {
- ret = mmc_blk_end_packed_req(mq_rq);
- break;
- } else {
- ret = blk_end_request(req, 0,
- brq->data.bytes_xfered);
- }
+ ret = blk_end_request(req, 0,
+ brq->data.bytes_xfered);
/*
* If the blk_end_request function returns non-zero even
@@ -2101,8 +1683,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
err = mmc_blk_reset(md, card->host, type);
if (!err)
break;
- if (err == -ENODEV ||
- mmc_packed_cmd(mq_rq->cmd_type))
+ if (err == -ENODEV)
goto cmd_abort;
/* Fall through */
}
@@ -2133,23 +1714,14 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
}
if (ret) {
- if (mmc_packed_cmd(mq_rq->cmd_type)) {
- if (!mq_rq->packed->retries)
- goto cmd_abort;
- mmc_blk_packed_hdr_wrq_prep(mq_rq, card, mq);
- mmc_start_req(card->host,
- &mq_rq->mmc_active, NULL);
- } else {
-
- /*
- * In case of a incomplete request
- * prepare it again and resend.
- */
- mmc_blk_rw_rq_prep(mq_rq, card,
- disable_multi, mq);
- mmc_start_req(card->host,
- &mq_rq->mmc_active, NULL);
- }
+ /*
+ * In case of a incomplete request
+ * prepare it again and resend.
+ */
+ mmc_blk_rw_rq_prep(mq_rq, card,
+ disable_multi, mq);
+ mmc_start_req(card->host,
+ &mq_rq->mmc_active, NULL);
mq_rq->brq.retune_retry_done = retune_retry_done;
}
} while (ret);
@@ -2157,15 +1729,11 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
return 1;
cmd_abort:
- if (mmc_packed_cmd(mq_rq->cmd_type)) {
- mmc_blk_abort_packed_req(mq_rq);
- } else {
- if (mmc_card_removed(card))
- req->cmd_flags |= REQ_QUIET;
- while (ret)
- ret = blk_end_request(req, -EIO,
- blk_rq_cur_bytes(req));
- }
+ if (mmc_card_removed(card))
+ req->cmd_flags |= REQ_QUIET;
+ while (ret)
+ ret = blk_end_request(req, -EIO,
+ blk_rq_cur_bytes(req));
start_new_req:
if (rqc) {
@@ -2173,12 +1741,6 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
rqc->cmd_flags |= REQ_QUIET;
blk_end_request_all(rqc, -EIO);
} else {
- /*
- * If current request is packed, it needs to put back.
- */
- if (mmc_packed_cmd(mq->mqrq_cur->cmd_type))
- mmc_blk_revert_packed_req(mq, mq->mqrq_cur);
-
mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
mmc_start_req(card->host,
&mq->mqrq_cur->mmc_active, NULL);
@@ -2361,14 +1923,6 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
blk_queue_write_cache(md->queue.queue, true, true);
}
- if (mmc_card_mmc(card) &&
- (area_type == MMC_BLK_DATA_AREA_MAIN) &&
- (md->flags & MMC_BLK_CMD23) &&
- card->ext_csd.packed_event_en) {
- if (!mmc_packed_init(&md->queue, card))
- md->flags |= MMC_BLK_PACKED_CMD;
- }
-
return md;
err_putdisk:
@@ -2472,8 +2026,6 @@ static void mmc_blk_remove_req(struct mmc_blk_data *md)
*/
card = md->queue.card;
mmc_cleanup_queue(&md->queue);
- if (md->flags & MMC_BLK_PACKED_CMD)
- mmc_packed_clean(&md->queue);
if (md->disk->flags & GENHD_FL_UP) {
device_remove_file(disk_to_dev(md->disk), &md->force_ro);
if ((md->area_type & MMC_BLK_DATA_AREA_BOOT) &&
diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index 3f6a2463ab30..7dacf2744fbd 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -406,41 +406,6 @@ void mmc_queue_resume(struct mmc_queue *mq)
}
}
-static unsigned int mmc_queue_packed_map_sg(struct mmc_queue *mq,
- struct mmc_packed *packed,
- struct scatterlist *sg,
- enum mmc_packed_type cmd_type)
-{
- struct scatterlist *__sg = sg;
- unsigned int sg_len = 0;
- struct request *req;
-
- if (mmc_packed_wr(cmd_type)) {
- unsigned int hdr_sz = mmc_large_sector(mq->card) ? 4096 : 512;
- unsigned int max_seg_sz = queue_max_segment_size(mq->queue);
- unsigned int len, remain, offset = 0;
- u8 *buf = (u8 *)packed->cmd_hdr;
-
- remain = hdr_sz;
- do {
- len = min(remain, max_seg_sz);
- sg_set_buf(__sg, buf + offset, len);
- offset += len;
- remain -= len;
- sg_unmark_end(__sg++);
- sg_len++;
- } while (remain);
- }
-
- list_for_each_entry(req, &packed->list, queuelist) {
- sg_len += blk_rq_map_sg(mq->queue, req, __sg);
- __sg = sg + (sg_len - 1);
- sg_unmark_end(__sg++);
- }
- sg_mark_end(sg + (sg_len - 1));
- return sg_len;
-}
-
/*
* Prepare the sg list(s) to be handed of to the host driver
*/
@@ -449,26 +414,14 @@ unsigned int mmc_queue_map_sg(struct mmc_queue *mq, struct mmc_queue_req *mqrq)
unsigned int sg_len;
size_t buflen;
struct scatterlist *sg;
- enum mmc_packed_type cmd_type;
int i;
- cmd_type = mqrq->cmd_type;
-
- if (!mqrq->bounce_buf) {
- if (mmc_packed_cmd(cmd_type))
- return mmc_queue_packed_map_sg(mq, mqrq->packed,
- mqrq->sg, cmd_type);
- else
- return blk_rq_map_sg(mq->queue, mqrq->req, mqrq->sg);
- }
+ if (!mqrq->bounce_buf)
+ return blk_rq_map_sg(mq->queue, mqrq->req, mqrq->sg);
BUG_ON(!mqrq->bounce_sg);
- if (mmc_packed_cmd(cmd_type))
- sg_len = mmc_queue_packed_map_sg(mq, mqrq->packed,
- mqrq->bounce_sg, cmd_type);
- else
- sg_len = blk_rq_map_sg(mq->queue, mqrq->req, mqrq->bounce_sg);
+ sg_len = blk_rq_map_sg(mq->queue, mqrq->req, mqrq->bounce_sg);
mqrq->bounce_sg_len = sg_len;
diff --git a/drivers/mmc/card/queue.h b/drivers/mmc/card/queue.h
index 334c9306070f..47f5532b5776 100644
--- a/drivers/mmc/card/queue.h
+++ b/drivers/mmc/card/queue.h
@@ -22,23 +22,6 @@ struct mmc_blk_request {
int retune_retry_done;
};
-enum mmc_packed_type {
- MMC_PACKED_NONE = 0,
- MMC_PACKED_WRITE,
-};
-
-#define mmc_packed_cmd(type) ((type) != MMC_PACKED_NONE)
-#define mmc_packed_wr(type) ((type) == MMC_PACKED_WRITE)
-
-struct mmc_packed {
- struct list_head list;
- __le32 cmd_hdr[1024];
- unsigned int blocks;
- u8 nr_entries;
- u8 retries;
- s16 idx_failure;
-};
-
struct mmc_queue_req {
struct request *req;
struct mmc_blk_request brq;
@@ -47,8 +30,6 @@ struct mmc_queue_req {
struct scatterlist *bounce_sg;
unsigned int bounce_sg_len;
struct mmc_async_req mmc_active;
- enum mmc_packed_type cmd_type;
- struct mmc_packed *packed;
};
struct mmc_queue {
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 2a6418d0c343..2ce32fefb41c 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -494,11 +494,6 @@ static inline int mmc_host_uhs(struct mmc_host *host)
MMC_CAP_UHS_DDR50);
}
-static inline int mmc_host_packed_wr(struct mmc_host *host)
-{
- return host->caps2 & MMC_CAP2_PACKED_WR;
-}
-
static inline int mmc_card_hs(struct mmc_card *card)
{
return card->host->ios.timing == MMC_TIMING_SD_HS ||
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] mmc: delete is_first_req parameter from pre-request callback
From: Ulf Hansson @ 2016-11-23 13:02 UTC (permalink / raw)
To: Linus Walleij; +Cc: linux-mmc@vger.kernel.org, Chunyan Zhang, Baolin Wang
In-Reply-To: <1479895344-16818-1-git-send-email-linus.walleij@linaro.org>
On 23 November 2016 at 11:02, Linus Walleij <linus.walleij@linaro.org> wrote:
> The void (*pre_req) callback in the struct mmc_host_ops vtable
> is passing an argument "is_first_req" indicating whether this is
> the first request or not.
>
> None of the in-kernel users use this parameter: instead, since
> they all just do variants of dma_map* they use the DMA cookie
> to indicate whether a pre* callback has already been done for
> a request when they decide how to handle it.
>
> Delete the parameter from the callback and all users, as it is
> just pointless cruft.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Thanks, applied for next!
Kind regards
Uffe
> ---
> drivers/mmc/core/core.c | 11 ++++-------
> drivers/mmc/host/dw_mmc.c | 3 +--
> drivers/mmc/host/jz4740_mmc.c | 3 +--
> drivers/mmc/host/mmci.c | 3 +--
> drivers/mmc/host/mtk-sd.c | 3 +--
> drivers/mmc/host/omap_hsmmc.c | 3 +--
> drivers/mmc/host/rtsx_pci_sdmmc.c | 3 +--
> drivers/mmc/host/sdhci.c | 3 +--
> include/linux/mmc/host.h | 3 +--
> 9 files changed, 12 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 67e262315a50..faeea26efba5 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -611,18 +611,15 @@ EXPORT_SYMBOL(mmc_is_req_done);
> * mmc_pre_req - Prepare for a new request
> * @host: MMC host to prepare command
> * @mrq: MMC request to prepare for
> - * @is_first_req: true if there is no previous started request
> - * that may run in parellel to this call, otherwise false
> *
> * mmc_pre_req() is called in prior to mmc_start_req() to let
> * host prepare for the new request. Preparation of a request may be
> * performed while another request is running on the host.
> */
> -static void mmc_pre_req(struct mmc_host *host, struct mmc_request *mrq,
> - bool is_first_req)
> +static void mmc_pre_req(struct mmc_host *host, struct mmc_request *mrq)
> {
> if (host->ops->pre_req)
> - host->ops->pre_req(host, mrq, is_first_req);
> + host->ops->pre_req(host, mrq);
> }
>
> /**
> @@ -667,7 +664,7 @@ struct mmc_async_req *mmc_start_req(struct mmc_host *host,
>
> /* Prepare a new request */
> if (areq)
> - mmc_pre_req(host, areq->mrq, !host->areq);
> + mmc_pre_req(host, areq->mrq);
>
> if (host->areq) {
> status = mmc_wait_for_data_req_done(host, host->areq->mrq, areq);
> @@ -696,7 +693,7 @@ struct mmc_async_req *mmc_start_req(struct mmc_host *host,
>
> /* prepare the request again */
> if (areq)
> - mmc_pre_req(host, areq->mrq, !host->areq);
> + mmc_pre_req(host, areq->mrq);
> }
> }
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 4fcbc4012ed0..969acbf0740c 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -883,8 +883,7 @@ static int dw_mci_pre_dma_transfer(struct dw_mci *host,
> }
>
> static void dw_mci_pre_req(struct mmc_host *mmc,
> - struct mmc_request *mrq,
> - bool is_first_req)
> + struct mmc_request *mrq)
> {
> struct dw_mci_slot *slot = mmc_priv(mmc);
> struct mmc_data *data = mrq->data;
> diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c
> index 684087db170b..819ad32964fc 100644
> --- a/drivers/mmc/host/jz4740_mmc.c
> +++ b/drivers/mmc/host/jz4740_mmc.c
> @@ -320,8 +320,7 @@ static int jz4740_mmc_start_dma_transfer(struct jz4740_mmc_host *host,
> }
>
> static void jz4740_mmc_pre_request(struct mmc_host *mmc,
> - struct mmc_request *mrq,
> - bool is_first_req)
> + struct mmc_request *mrq)
> {
> struct jz4740_mmc_host *host = mmc_priv(mmc);
> struct mmc_data *data = mrq->data;
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index df990bb8c873..1ddbb64e385a 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -684,8 +684,7 @@ static void mmci_get_next_data(struct mmci_host *host, struct mmc_data *data)
> next->dma_chan = NULL;
> }
>
> -static void mmci_pre_request(struct mmc_host *mmc, struct mmc_request *mrq,
> - bool is_first_req)
> +static void mmci_pre_request(struct mmc_host *mmc, struct mmc_request *mrq)
> {
> struct mmci_host *host = mmc_priv(mmc);
> struct mmc_data *data = mrq->data;
> diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
> index 84e9afcb5c09..db185441966d 100644
> --- a/drivers/mmc/host/mtk-sd.c
> +++ b/drivers/mmc/host/mtk-sd.c
> @@ -927,8 +927,7 @@ static void msdc_ops_request(struct mmc_host *mmc, struct mmc_request *mrq)
> msdc_start_command(host, mrq, mrq->cmd);
> }
>
> -static void msdc_pre_req(struct mmc_host *mmc, struct mmc_request *mrq,
> - bool is_first_req)
> +static void msdc_pre_req(struct mmc_host *mmc, struct mmc_request *mrq)
> {
> struct msdc_host *host = mmc_priv(mmc);
> struct mmc_data *data = mrq->data;
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 5f2f24a7360d..ad11c4cc12ed 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -1565,8 +1565,7 @@ static void omap_hsmmc_post_req(struct mmc_host *mmc, struct mmc_request *mrq,
> }
> }
>
> -static void omap_hsmmc_pre_req(struct mmc_host *mmc, struct mmc_request *mrq,
> - bool is_first_req)
> +static void omap_hsmmc_pre_req(struct mmc_host *mmc, struct mmc_request *mrq)
> {
> struct omap_hsmmc_host *host = mmc_priv(mmc);
>
> diff --git a/drivers/mmc/host/rtsx_pci_sdmmc.c b/drivers/mmc/host/rtsx_pci_sdmmc.c
> index 3ccaa1415f33..ecb99a8d2fa2 100644
> --- a/drivers/mmc/host/rtsx_pci_sdmmc.c
> +++ b/drivers/mmc/host/rtsx_pci_sdmmc.c
> @@ -190,8 +190,7 @@ static int sd_pre_dma_transfer(struct realtek_pci_sdmmc *host,
> return using_cookie;
> }
>
> -static void sdmmc_pre_req(struct mmc_host *mmc, struct mmc_request *mrq,
> - bool is_first_req)
> +static void sdmmc_pre_req(struct mmc_host *mmc, struct mmc_request *mrq)
> {
> struct realtek_pci_sdmmc *host = mmc_priv(mmc);
> struct mmc_data *data = mrq->data;
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 48055666c655..eefe5ed73822 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2190,8 +2190,7 @@ static void sdhci_post_req(struct mmc_host *mmc, struct mmc_request *mrq,
> data->host_cookie = COOKIE_UNMAPPED;
> }
>
> -static void sdhci_pre_req(struct mmc_host *mmc, struct mmc_request *mrq,
> - bool is_first_req)
> +static void sdhci_pre_req(struct mmc_host *mmc, struct mmc_request *mrq)
> {
> struct sdhci_host *host = mmc_priv(mmc);
>
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 5310f94be0ab..fdd13e0b8ec8 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -93,8 +93,7 @@ struct mmc_host_ops {
> */
> void (*post_req)(struct mmc_host *host, struct mmc_request *req,
> int err);
> - void (*pre_req)(struct mmc_host *host, struct mmc_request *req,
> - bool is_first_req);
> + void (*pre_req)(struct mmc_host *host, struct mmc_request *req);
> void (*request)(struct mmc_host *host, struct mmc_request *req);
>
> /*
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH v2 0/7] mmc: core: Re-work CMD13 polling method for CMD6 for mmc
From: Ulf Hansson @ 2016-11-23 12:50 UTC (permalink / raw)
To: linux-mmc@vger.kernel.org, Ulf Hansson
Cc: Jaehoon Chung, Adrian Hunter, Linus Walleij, Chaotian Jing,
Stephen Boyd, Michael Walle, Yong Mao, Shawn Lin
In-Reply-To: <CAPDyKFp8PwbZeCYkp6eoAv1zKEcdJ6wFtozXWErn_NFPferJCw@mail.gmail.com>
On 22 November 2016 at 22:01, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 22 November 2016 at 21:56, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> Changes in v2:
>>
>> - From discussions around the difficulties for how to support CMD13
>> polling for HS200, HS400 HS400ES, I decided to drop those patches for
>> now. It's is particularly due to the need for tuning, after a speed mode
>> switch, that makes it hard to rely on CMD13 polling for these cases.
>> Perhaps we can allow CMD13 polling when switching to the intermediate
>> speed modes, but let's deal with that outside of this series.
>>
>> - Folded in a new patch, which checks the SWITCH_ERROR bit in each
>> CMD13 response when polling with CMD13. Patch v2 4/7.
>>
>> - No other changes made to the rest of the series.
>>
>>
>>
>> Several changes has been made for how and when to use CMD13 as a
>> polling-method, to find out when the mmc cards stops signaling busy after
>> sending a CMD6. This particularly concerns the cases when switching to new bus
>> speed modes, such as HS (high-speed), HS DDR, HS200, HS400 and HS400ES.
>>
>> Currently CMD13 polling is *not* allowed for these cases, but this was recently
>> changed - and which did cause regressions for card detection for some
>> platforms.
>>
>> A simple fix was made to solve these regressions, simply by using a default
>> CMD6 generic timeout of 500ms, as to avoid using the fall-back timeout in
>> __mmc_switch() of 10min. That greatly improve the situation and one could
>> consider the regressions as "solved".
>>
>> However, this simple fix is still causing unnecessary long card initialization
>> times, especially for those mmc hosts that doesn't support HW busy detection
>> (using MMC_CAP_WAIT_WHILE_BUSY and/or implements the ->card_busy() host ops).
>>
>> This because we wait a fixed amount of time (CMD6 generic timeout) to make sure
>> the card is done signaling busy, while in practice this happens a lot sooner.
>>
>> A couple of tests for HS and HS DDR eMMC cards, shows the card only need a few
>> ms to complete the bus speed mode changes, thus it's far less than the CMD6
>> generic timeout.
>>
>> To improve this behaviour and shorten the card initialization time, we need to
>> allow using CMD13 as polling method to find out when the card stops signaling
>> busy. Although, enabling CMD13 polling may also introduce other issues as
>> according to the JEDEC spec, it's not the recommended method. Especially it
>> mentions that CRC errors may be triggered when sending a CMD13 command during a
>> bus timing change.
>>
>> To deal with these issues, we need to change from ignoring those CRC errors but
>> instead continue to treat the card as being busy and continue to poll with
>> CMD13. Perhaps this behaviour is one of reasons to why the earlier CMD13 polling
>> method didn't work out well.
>>
>> This series requires extensive testing, please help with that!
>>
>> I have currently tested it with HS and HS DDR eMMC cards, and for combinations
>> of an mmc hosts (mmci) supporting HW busy detection and not (local hacks in
>> mmci.c).
>>
>>
>> Ulf Hansson (9):
>> mmc: core: Retry instead of ignore at CRC errors when polling for busy
>> mmc: core: Remove redundant __mmc_send_status()
>> mmc: core: Rename ignore_crc to retry_crc_err to reflect its purpose
>> mmc: core: Enable __mmc_switch() to change bus speed timing for the
>> host
>> mmc: core: Allow CMD13 polling when switching to HS mode for mmc
>> mmc: core: Update CMD13 polling policy when switch to HS DDR mode
>> mmc: core: Allow CMD13 polling when switch to HS200 mode
>> mmc: core: Allow CMD13 polling when switch to HS400 mode
>> mmc: core: Allow CMD13 polling when switch to HS400ES mode
>>
>> drivers/mmc/core/core.c | 2 +-
>> drivers/mmc/core/mmc.c | 156 ++++++++++++++-------------------------------
>> drivers/mmc/core/mmc_ops.c | 45 +++++++------
>> drivers/mmc/core/mmc_ops.h | 4 +-
>> 4 files changed, 77 insertions(+), 130 deletions(-)
>
> Oops, forgot to clean up the cover letter. Please ignore the summary
> of changes above. It's the below changes that are the v2 series.
>
> Apologize for the inconvenience!
>
> Kind regards
> Uffe
>
>>
>> --
>> 1.9.1
>>
>>
>>
>>
>>
>>
>> *** BLURB HERE ***
>>
>> Ulf Hansson (7):
>> mmc: core: Retry instead of ignore at CRC errors when polling for busy
>> mmc: core: Remove redundant __mmc_send_status()
>> mmc: core: Rename ignore_crc to retry_crc_err to reflect its purpose
>> mmc: core: Check SWITCH_ERROR bit from each CMD13 response when
>> polling
>> mmc: core: Enable __mmc_switch() to change bus speed timing for the
>> host
>> mmc: core: Allow CMD13 polling when switching to HS mode for mmc
>> mmc: core: Update CMD13 polling policy when switch to HS DDR mode
>>
>> drivers/mmc/core/core.c | 2 +-
>> drivers/mmc/core/mmc.c | 53 +++++++++++++++++++++++-----------------------
>> drivers/mmc/core/mmc_ops.c | 51 ++++++++++++++++++++++++++------------------
>> drivers/mmc/core/mmc_ops.h | 4 ++--
>> 4 files changed, 60 insertions(+), 50 deletions(-)
>>
>> --
>> 1.9.1
>>
I have applied this for next, let's see what the kernelci reports.
More tests is still warmly appreciated, and I can still add
tested/reviewed-by tags.
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH 0/2] mmc: sdhci-pci: Add GLK support and misc
From: Ulf Hansson @ 2016-11-23 12:49 UTC (permalink / raw)
To: Adrian Hunter; +Cc: linux-mmc, David E. Box
In-Reply-To: <1479805418-19603-1-git-send-email-adrian.hunter@intel.com>
On 22 November 2016 at 10:03, Adrian Hunter <adrian.hunter@intel.com> wrote:
> Hi
>
> Here are a couple of small patches.
>
>
> Adrian Hunter (1):
> mmc: sdhci-pci: Add support for Intel GLK
>
> David E. Box (1):
> mmc: sdhci-pci: Allow deferred probe for sd card detect gpio
>
> drivers/mmc/host/sdhci-pci-core.c | 42 +++++++++++++++++++++++++++++++++------
> drivers/mmc/host/sdhci-pci.h | 3 +++
> 2 files changed, 39 insertions(+), 6 deletions(-)
>
Thanks, applied for next!
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH v9 00/16] mmc: sdhci-msm: Add clk-rates, DDR, HS400 support
From: Ulf Hansson @ 2016-11-23 12:49 UTC (permalink / raw)
To: Ritesh Harjani
Cc: Stephen Boyd, Andy Gross, linux-mmc, Adrian Hunter, Shawn Lin,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-clk,
David Brown,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Georgi Djakov, Alex Lemberg, Mateusz Nowak, Yuliy Izrailov,
Asutosh Das, David Griego, Sahitya Tummala, Venkat Gopalakrishnan,
Rajendra Nayak
In-Reply-To: <6dd874b4-8f60-471b-d1e7-089b4b035ad2-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On 23 November 2016 at 01:05, Ritesh Harjani <riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
>
>
> On 11/22/2016 4:41 AM, Stephen Boyd wrote:
>>
>> On 11/21, Ritesh Harjani wrote:
>>>
>>>
>>>
>>> On 11/21/2016 3:36 PM, Ulf Hansson wrote:
>>>>
>>>> On 21 November 2016 at 07:37, Ritesh Harjani <riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>>> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> This is v9 version of the patch series which adds support for MSM8996.
>>>>> Adds HS400 driver support as well.
>>>>> These are tested on internal msm8996 & db410c HW.
>>>>>
>>>>> The patch series is ready. Do we think we can apply these
>>>>> patches for next now?
>>>>
>>>>
>>>> I guess the DTS changes can be picked up by Andy, so they can go via
>>>> arm-soc?
>>>
>>> Yes.
>>>
>>>>
>>>> Then, does the mmc changes depend on the clock changes? If so, I can
>>>> pick them as well, but then I need an ack from Stephen....
>>>
>>> Ideal and preferable, would be that clk & mmc changes go in
>>> together. But either ways should be fine.
>>>
>>
>> There's only a runtime dependency where the clk rates will be
>> wrong if clk tree isn't merged. I'd rather just apply the clk
>> ones directly to clk tree and let all three trees come together
>> in linux-next and work.
>
>
> Ok great! So can we queue this patch series to next?
Yes. I have picked up all the mmc patches, including the DT
documentation patch. They are queued for next!
The rest is for Andy and Stephen to pick.
Thanks and kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] mmc: sd: Meet alignment requirements for raw_ssr DMA
From: Ulf Hansson @ 2016-11-23 12:36 UTC (permalink / raw)
To: Paul Burton; +Cc: linux-mmc
In-Reply-To: <3676563.EjIzX52DqA@np-p-burton>
[...]
>> > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
>> > index 73c762a..f6f40a1 100644
>> > --- a/drivers/mmc/core/sd.c
>> > +++ b/drivers/mmc/core/sd.c
>> > @@ -223,6 +223,7 @@ static int mmc_decode_scr(struct mmc_card *card)
>> >
>> > static int mmc_read_ssr(struct mmc_card *card)
>> > {
>> >
>> > unsigned int au, es, et, eo;
>> >
>> > + u32 *raw_ssr;
>> >
>> > int i;
>> >
>> > if (!(card->csd.cmdclass & CCC_APP_SPEC)) {
>> >
>> > @@ -231,14 +232,21 @@ static int mmc_read_ssr(struct mmc_card *card)
>> >
>> > return 0;
>> >
>> > }
>> >
>> > - if (mmc_app_sd_status(card, card->raw_ssr)) {
>>
>> So the struct mmc_card, which contains "u32 raw_ssr[16]", is already
>> allocated via using a kzalloc().
>> As kzalloc() returns a physically contiguous allocated memory, using
>> DMA for reading the data into the memory pointed to by card->raw_ssr
>> should be safe.
>>
>> Unless I am missing something, of course.
>
> Sadly you are. This is incorrect because the raw_ssr field of struct mmc_card
> may not begin or end at a cache-line boundary. On systems where mapping a
> buffer for DMA requires cache invalidation we may therefore lose data if it
> shares a cache line with some other data. In this case let's say we have
Yes, of course - you are right! Apologize for my ignorance!
> something like this:
>
> 0x1000: u32 raw_scr[2];
> 0x1008: u32 raw_ssr[16];
>
> Now let's say we have 16 byte cache lines and we try to map raw_ssr for DMA.
> If the architecture requires that we invalidate the memory being mapped in
> caches (in order to avoid any writebacks clobbering the DMA'd data) then we
> have a problem because our choice is either to:
>
> - Writeback the line that overlaps with raw_scr, then invalidate it along with
> the rest of the cache lines covering raw_ssr. This won't do because there's
> nothing stopping us from pulling the line back into the cache whilst the DMA
> occurs on some systems - say we access raw_scr whilst the DMA to raw_ssr
> happens or the CPU just speculatively prefetches something. The DMA API
> cannot detect the former & would have no way to deal with it if it could,
> and on systems where the latter is possible we have no choice but to
> invalidate the cache lines again after the DMA is complete. What would we do
> with the first line then? We can't write it back to preserve raw_scr because
> it includes some potentially stale copy of the start of raw_ssr, which we
> would then clobber the DMA'd data with.
>
> - Or, we just invalidate the line which overlaps with raw_scr. We can't do
> this because we might throw away some part of raw_scr.
>
> So we have no choice but to align the buffers used for DMA at a cache line
> boundary. This is what the paragraph in Documentation/DMA-API.txt is about &
> the MMC code currently fails to meet the requirements it sets out. Allocating
> the buffer to be DMA'd to with kmalloc will meet that alignment requirement.
> One possible alternative would be to ensure that the raw_ssr field of struct
> mmc_card is always aligned at an ARCH_DMA_MINALIGN boundary within the struct,
> with nothing after it until another ARCH_DMA_MINALIGN boundary. That seems
> like it'd be a bit messy though, and probably easy for someone to break.
>
> Of course when a system has DMA that is coherent with caches there's generally
> no need to worry about any of this, but users of the DMA API generally can't
> rely on that, again as Documentation/DMA-API.txt says. Any buffers mapped for
> DMA should be ensured to be cache-line aligned at minimum giving the
> architecture code backing the DMA API a chance to do the right thing for the
> given system.
Thanks a lot for providing this detailed description to the problem.
It really helped to refresh my mind for how this works!
>
> Thanks,
> Paul
>
>>
>> > + raw_ssr = kmalloc(sizeof(card->raw_ssr), GFP_KERNEL);
>> > + if (!raw_ssr)
>> > + return -ENOMEM;
>> > +
Creating this bounce buffer does of course comes with a small cost.
Maybe we could neglect its impact!?
However we could also make the card->raw_ssr be cacheline aligned, via
using the __cacheline_aligned attribute for it.
Also, this isn't the only case when the mmc core creates bounce
buffers while reading card registers during the card initialization.
Perhaps a better method is to use the __cacheline_aligned for these
buffers that needs to be DMA-able (requests which is
MMC_DATA_READ|WRITE)). Of course that change would affect/increase the
number of bytes allocated for each card struct (due to padding), but
since this is just a single struct being allocated per card, this
shouldn't be an issue.
What do you think?
>> > + if (mmc_app_sd_status(card, raw_ssr)) {
>> >
>> > pr_warn("%s: problem reading SD Status register\n",
>> >
>> > mmc_hostname(card->host));
>> >
>> > + kfree(raw_ssr);
>> >
>> > return 0;
>> >
>> > }
>> >
>> > for (i = 0; i < 16; i++)
>> >
>> > - card->raw_ssr[i] = be32_to_cpu(card->raw_ssr[i]);
>> > + card->raw_ssr[i] = be32_to_cpu(raw_ssr[i]);
>> > +
>> > + kfree(raw_ssr);
>> >
>> > /*
>> >
>> > * UNSTUFF_BITS only works with four u32s so we have to offset the
>> >
>> > --
>> > 2.10.2
>>
Kind regards
Uffe
^ permalink raw reply
* Re: [GIT PULL] DWMMC controller for next
From: Jaehoon Chung @ 2016-11-23 12:31 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc@vger.kernel.org
In-Reply-To: <ed2ab8e4-6209-49d4-8ef3-76db0a0342bf@samsung.com>
On 11/23/2016 08:50 PM, Jaehoon Chung wrote:
> On 11/23/2016 08:47 PM, Ulf Hansson wrote:
>> On 22 November 2016 at 17:13, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>> On 22 November 2016 at 10:59, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>>> Dear Ulf,
>>>>
>>>> Could you pull these patches on your next branch?
>>>> If there is a problem, let me know, plz.
>>>>
>>>> The following changes since commit e2081b37d910c5e6c6925d9b4d0a7a6283f84ec5:
>>>>
>>>> Merge branch 'fixes' into next (2016-11-21 11:09:18 +0100)
>>>>
>>>> are available in the git repository at:
>>>>
>>>> https://github.com/jh80chung/dw-mmc.git for-ulf
>>>>
>>>> for you to fetch changes up to e25fd245b557482a8e0f7ab87841085f30706f3a:
>>>>
>>>> Documentation: synopsys-dw-mshc: remove the unused properties (2016-11-22 10:34:05 +0900)
>>>>
>>>> ----------------------------------------------------------------
>>>> Colin Ian King (1):
>>>> mmc: dw_mmc: fix spelling mistake in dev_dbg message
>>>>
>>>> Jaehoon Chung (9):
>>>> mmc: dw_mmc: display the real register value on debugfs
>>>> mmc: dw_mmc: fix the debug message for checking card's present
>>>> mmc: dw_mmc: change the DW_MCI_FREQ_MIN from 400K to 100K
>>>> mmc: dw_mmc: use the hold register when send stop command
>>>> mmc: dw_mmc: call the dw_mci_prep_stop_abort() by default
>>>> mmc: dw_mmc: use the cookie's enum values for post/pre_req()
>>>> mmc: dw_mmc: remove the unnecessary mmc_data structure
>>>> mmc: dw_mmc: The "clock-freq-min-max" property was deprecated
>>>> Documentation: synopsys-dw-mshc: remove the unused properties
>>>>
>>>> Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt | 8 ++------
>>>> drivers/mmc/host/dw_mmc.c | 92 ++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------
>>>> include/linux/mmc/dw_mmc.h | 6 ++++++
>>>> 3 files changed, 52 insertions(+), 54 deletions(-)
>>>>
>>>> Best Regards,
>>>> Jaehoon Chung
>>>
>>> Dear Jaehoon, thanks for the pull request. I have add queued it all up
>>> for my next branch!
>>>
>>>
>>> Kind regards
>>> Uffe
>>
>> Jaehoon,
>>
>> It seems like some of these patches breaks Exynos 5250-arndale. Can
>> you please have a look and see what you think?
>> https://kernelci.org/boot/all/job/ulfh/kernel/v4.9-rc5-78-g52eb5d4531e4/
>
> I will check and reply. Thanks for noticing.
Ulf,
I'm not sure but if more stable than now..i think that needs to check NULL pointer.
What do you want how i do? Send patch or revert commit?
I didn't have the 5250-arndale..so i can't test..How about the below?
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 881ca3e..7a0cb97 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -167,12 +167,14 @@ static int dw_mci_regs_show(struct seq_file *s, void *v)
{
struct dw_mci *host = s->private;
- seq_printf(s, "STATUS:\t0x%08x\n", mci_readl(host, STATUS));
- seq_printf(s, "RINTSTS:\t0x%08x\n", mci_readl(host, RINTSTS));
- seq_printf(s, "CMD:\t0x%08x\n", mci_readl(host, CMD));
- seq_printf(s, "CTRL:\t0x%08x\n", mci_readl(host, CTRL));
- seq_printf(s, "INTMASK:\t0x%08x\n", mci_readl(host, INTMASK));
- seq_printf(s, "CLKENA:\t0x%08x\n", mci_readl(host, CLKENA));
+ if (host->regs) {
+ seq_printf(s, "STATUS:\t0x%08x\n", mci_readl(host, STATUS));
+ seq_printf(s, "RINTSTS:\t0x%08x\n", mci_readl(host, RINTSTS));
+ seq_printf(s, "CMD:\t0x%08x\n", mci_readl(host, CMD));
+ seq_printf(s, "CTRL:\t0x%08x\n", mci_readl(host, CTRL));
+ seq_printf(s, "INTMASK:\t0x%08x\n", mci_readl(host, INTMASK));
+ seq_printf(s, "CLKENA:\t0x%08x\n", mci_readl(host, CLKENA));
+ }
return 0;
}
Best Regards,
Jaehoon Chung
>
> Beset Regards,
> Jaehoon Chung
>
>>
>> Kind regards
>> Uffe
>> --
>> 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
>>
>>
>>
>
> --
> 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
>
>
>
^ permalink raw reply related
* Re: [GIT PULL] DWMMC controller for next
From: Jaehoon Chung @ 2016-11-23 11:50 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc@vger.kernel.org
In-Reply-To: <CAPDyKFokGM+-6hjGpgJRJOVwANX43gEww4p17LCxvD=e0Qbj=g@mail.gmail.com>
On 11/23/2016 08:47 PM, Ulf Hansson wrote:
> On 22 November 2016 at 17:13, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> On 22 November 2016 at 10:59, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>> Dear Ulf,
>>>
>>> Could you pull these patches on your next branch?
>>> If there is a problem, let me know, plz.
>>>
>>> The following changes since commit e2081b37d910c5e6c6925d9b4d0a7a6283f84ec5:
>>>
>>> Merge branch 'fixes' into next (2016-11-21 11:09:18 +0100)
>>>
>>> are available in the git repository at:
>>>
>>> https://github.com/jh80chung/dw-mmc.git for-ulf
>>>
>>> for you to fetch changes up to e25fd245b557482a8e0f7ab87841085f30706f3a:
>>>
>>> Documentation: synopsys-dw-mshc: remove the unused properties (2016-11-22 10:34:05 +0900)
>>>
>>> ----------------------------------------------------------------
>>> Colin Ian King (1):
>>> mmc: dw_mmc: fix spelling mistake in dev_dbg message
>>>
>>> Jaehoon Chung (9):
>>> mmc: dw_mmc: display the real register value on debugfs
>>> mmc: dw_mmc: fix the debug message for checking card's present
>>> mmc: dw_mmc: change the DW_MCI_FREQ_MIN from 400K to 100K
>>> mmc: dw_mmc: use the hold register when send stop command
>>> mmc: dw_mmc: call the dw_mci_prep_stop_abort() by default
>>> mmc: dw_mmc: use the cookie's enum values for post/pre_req()
>>> mmc: dw_mmc: remove the unnecessary mmc_data structure
>>> mmc: dw_mmc: The "clock-freq-min-max" property was deprecated
>>> Documentation: synopsys-dw-mshc: remove the unused properties
>>>
>>> Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt | 8 ++------
>>> drivers/mmc/host/dw_mmc.c | 92 ++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------
>>> include/linux/mmc/dw_mmc.h | 6 ++++++
>>> 3 files changed, 52 insertions(+), 54 deletions(-)
>>>
>>> Best Regards,
>>> Jaehoon Chung
>>
>> Dear Jaehoon, thanks for the pull request. I have add queued it all up
>> for my next branch!
>>
>>
>> Kind regards
>> Uffe
>
> Jaehoon,
>
> It seems like some of these patches breaks Exynos 5250-arndale. Can
> you please have a look and see what you think?
> https://kernelci.org/boot/all/job/ulfh/kernel/v4.9-rc5-78-g52eb5d4531e4/
I will check and reply. Thanks for noticing.
Beset Regards,
Jaehoon Chung
>
> Kind regards
> Uffe
> --
> 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
>
>
>
^ permalink raw reply
* Re: [GIT PULL] DWMMC controller for next
From: Ulf Hansson @ 2016-11-23 11:47 UTC (permalink / raw)
To: Jaehoon Chung; +Cc: linux-mmc@vger.kernel.org
In-Reply-To: <CAPDyKFo+W12sV-U8r44ySerWHi2tAwEEnGNMd97iUN8NLV4duA@mail.gmail.com>
On 22 November 2016 at 17:13, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 22 November 2016 at 10:59, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> Dear Ulf,
>>
>> Could you pull these patches on your next branch?
>> If there is a problem, let me know, plz.
>>
>> The following changes since commit e2081b37d910c5e6c6925d9b4d0a7a6283f84ec5:
>>
>> Merge branch 'fixes' into next (2016-11-21 11:09:18 +0100)
>>
>> are available in the git repository at:
>>
>> https://github.com/jh80chung/dw-mmc.git for-ulf
>>
>> for you to fetch changes up to e25fd245b557482a8e0f7ab87841085f30706f3a:
>>
>> Documentation: synopsys-dw-mshc: remove the unused properties (2016-11-22 10:34:05 +0900)
>>
>> ----------------------------------------------------------------
>> Colin Ian King (1):
>> mmc: dw_mmc: fix spelling mistake in dev_dbg message
>>
>> Jaehoon Chung (9):
>> mmc: dw_mmc: display the real register value on debugfs
>> mmc: dw_mmc: fix the debug message for checking card's present
>> mmc: dw_mmc: change the DW_MCI_FREQ_MIN from 400K to 100K
>> mmc: dw_mmc: use the hold register when send stop command
>> mmc: dw_mmc: call the dw_mci_prep_stop_abort() by default
>> mmc: dw_mmc: use the cookie's enum values for post/pre_req()
>> mmc: dw_mmc: remove the unnecessary mmc_data structure
>> mmc: dw_mmc: The "clock-freq-min-max" property was deprecated
>> Documentation: synopsys-dw-mshc: remove the unused properties
>>
>> Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt | 8 ++------
>> drivers/mmc/host/dw_mmc.c | 92 ++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------
>> include/linux/mmc/dw_mmc.h | 6 ++++++
>> 3 files changed, 52 insertions(+), 54 deletions(-)
>>
>> Best Regards,
>> Jaehoon Chung
>
> Dear Jaehoon, thanks for the pull request. I have add queued it all up
> for my next branch!
>
>
> Kind regards
> Uffe
Jaehoon,
It seems like some of these patches breaks Exynos 5250-arndale. Can
you please have a look and see what you think?
https://kernelci.org/boot/all/job/ulfh/kernel/v4.9-rc5-78-g52eb5d4531e4/
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH] mmc: delete is_first_req parameter from pre-request callback
From: Jaehoon Chung @ 2016-11-23 10:18 UTC (permalink / raw)
To: Linus Walleij, linux-mmc, Ulf Hansson; +Cc: Chunyan Zhang, Baolin Wang
In-Reply-To: <1479895344-16818-1-git-send-email-linus.walleij@linaro.org>
Hi Linus,
On 11/23/2016 07:02 PM, Linus Walleij wrote:
> The void (*pre_req) callback in the struct mmc_host_ops vtable
> is passing an argument "is_first_req" indicating whether this is
> the first request or not.
>
> None of the in-kernel users use this parameter: instead, since
> they all just do variants of dma_map* they use the DMA cookie
> to indicate whether a pre* callback has already been done for
> a request when they decide how to handle it.
>
> Delete the parameter from the callback and all users, as it is
> just pointless cruft.
Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
Best Regards,
Jaehoon Chung
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> drivers/mmc/core/core.c | 11 ++++-------
> drivers/mmc/host/dw_mmc.c | 3 +--
> drivers/mmc/host/jz4740_mmc.c | 3 +--
> drivers/mmc/host/mmci.c | 3 +--
> drivers/mmc/host/mtk-sd.c | 3 +--
> drivers/mmc/host/omap_hsmmc.c | 3 +--
> drivers/mmc/host/rtsx_pci_sdmmc.c | 3 +--
> drivers/mmc/host/sdhci.c | 3 +--
> include/linux/mmc/host.h | 3 +--
> 9 files changed, 12 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 67e262315a50..faeea26efba5 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -611,18 +611,15 @@ EXPORT_SYMBOL(mmc_is_req_done);
> * mmc_pre_req - Prepare for a new request
> * @host: MMC host to prepare command
> * @mrq: MMC request to prepare for
> - * @is_first_req: true if there is no previous started request
> - * that may run in parellel to this call, otherwise false
> *
> * mmc_pre_req() is called in prior to mmc_start_req() to let
> * host prepare for the new request. Preparation of a request may be
> * performed while another request is running on the host.
> */
> -static void mmc_pre_req(struct mmc_host *host, struct mmc_request *mrq,
> - bool is_first_req)
> +static void mmc_pre_req(struct mmc_host *host, struct mmc_request *mrq)
> {
> if (host->ops->pre_req)
> - host->ops->pre_req(host, mrq, is_first_req);
> + host->ops->pre_req(host, mrq);
> }
>
> /**
> @@ -667,7 +664,7 @@ struct mmc_async_req *mmc_start_req(struct mmc_host *host,
>
> /* Prepare a new request */
> if (areq)
> - mmc_pre_req(host, areq->mrq, !host->areq);
> + mmc_pre_req(host, areq->mrq);
>
> if (host->areq) {
> status = mmc_wait_for_data_req_done(host, host->areq->mrq, areq);
> @@ -696,7 +693,7 @@ struct mmc_async_req *mmc_start_req(struct mmc_host *host,
>
> /* prepare the request again */
> if (areq)
> - mmc_pre_req(host, areq->mrq, !host->areq);
> + mmc_pre_req(host, areq->mrq);
> }
> }
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 4fcbc4012ed0..969acbf0740c 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -883,8 +883,7 @@ static int dw_mci_pre_dma_transfer(struct dw_mci *host,
> }
>
> static void dw_mci_pre_req(struct mmc_host *mmc,
> - struct mmc_request *mrq,
> - bool is_first_req)
> + struct mmc_request *mrq)
> {
> struct dw_mci_slot *slot = mmc_priv(mmc);
> struct mmc_data *data = mrq->data;
> diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c
> index 684087db170b..819ad32964fc 100644
> --- a/drivers/mmc/host/jz4740_mmc.c
> +++ b/drivers/mmc/host/jz4740_mmc.c
> @@ -320,8 +320,7 @@ static int jz4740_mmc_start_dma_transfer(struct jz4740_mmc_host *host,
> }
>
> static void jz4740_mmc_pre_request(struct mmc_host *mmc,
> - struct mmc_request *mrq,
> - bool is_first_req)
> + struct mmc_request *mrq)
> {
> struct jz4740_mmc_host *host = mmc_priv(mmc);
> struct mmc_data *data = mrq->data;
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index df990bb8c873..1ddbb64e385a 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -684,8 +684,7 @@ static void mmci_get_next_data(struct mmci_host *host, struct mmc_data *data)
> next->dma_chan = NULL;
> }
>
> -static void mmci_pre_request(struct mmc_host *mmc, struct mmc_request *mrq,
> - bool is_first_req)
> +static void mmci_pre_request(struct mmc_host *mmc, struct mmc_request *mrq)
> {
> struct mmci_host *host = mmc_priv(mmc);
> struct mmc_data *data = mrq->data;
> diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
> index 84e9afcb5c09..db185441966d 100644
> --- a/drivers/mmc/host/mtk-sd.c
> +++ b/drivers/mmc/host/mtk-sd.c
> @@ -927,8 +927,7 @@ static void msdc_ops_request(struct mmc_host *mmc, struct mmc_request *mrq)
> msdc_start_command(host, mrq, mrq->cmd);
> }
>
> -static void msdc_pre_req(struct mmc_host *mmc, struct mmc_request *mrq,
> - bool is_first_req)
> +static void msdc_pre_req(struct mmc_host *mmc, struct mmc_request *mrq)
> {
> struct msdc_host *host = mmc_priv(mmc);
> struct mmc_data *data = mrq->data;
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 5f2f24a7360d..ad11c4cc12ed 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -1565,8 +1565,7 @@ static void omap_hsmmc_post_req(struct mmc_host *mmc, struct mmc_request *mrq,
> }
> }
>
> -static void omap_hsmmc_pre_req(struct mmc_host *mmc, struct mmc_request *mrq,
> - bool is_first_req)
> +static void omap_hsmmc_pre_req(struct mmc_host *mmc, struct mmc_request *mrq)
> {
> struct omap_hsmmc_host *host = mmc_priv(mmc);
>
> diff --git a/drivers/mmc/host/rtsx_pci_sdmmc.c b/drivers/mmc/host/rtsx_pci_sdmmc.c
> index 3ccaa1415f33..ecb99a8d2fa2 100644
> --- a/drivers/mmc/host/rtsx_pci_sdmmc.c
> +++ b/drivers/mmc/host/rtsx_pci_sdmmc.c
> @@ -190,8 +190,7 @@ static int sd_pre_dma_transfer(struct realtek_pci_sdmmc *host,
> return using_cookie;
> }
>
> -static void sdmmc_pre_req(struct mmc_host *mmc, struct mmc_request *mrq,
> - bool is_first_req)
> +static void sdmmc_pre_req(struct mmc_host *mmc, struct mmc_request *mrq)
> {
> struct realtek_pci_sdmmc *host = mmc_priv(mmc);
> struct mmc_data *data = mrq->data;
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 48055666c655..eefe5ed73822 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2190,8 +2190,7 @@ static void sdhci_post_req(struct mmc_host *mmc, struct mmc_request *mrq,
> data->host_cookie = COOKIE_UNMAPPED;
> }
>
> -static void sdhci_pre_req(struct mmc_host *mmc, struct mmc_request *mrq,
> - bool is_first_req)
> +static void sdhci_pre_req(struct mmc_host *mmc, struct mmc_request *mrq)
> {
> struct sdhci_host *host = mmc_priv(mmc);
>
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 5310f94be0ab..fdd13e0b8ec8 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -93,8 +93,7 @@ struct mmc_host_ops {
> */
> void (*post_req)(struct mmc_host *host, struct mmc_request *req,
> int err);
> - void (*pre_req)(struct mmc_host *host, struct mmc_request *req,
> - bool is_first_req);
> + void (*pre_req)(struct mmc_host *host, struct mmc_request *req);
> void (*request)(struct mmc_host *host, struct mmc_request *req);
>
> /*
>
^ permalink raw reply
* [PATCH] mmc: delete is_first_req parameter from pre-request callback
From: Linus Walleij @ 2016-11-23 10:02 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson; +Cc: Chunyan Zhang, Baolin Wang, Linus Walleij
The void (*pre_req) callback in the struct mmc_host_ops vtable
is passing an argument "is_first_req" indicating whether this is
the first request or not.
None of the in-kernel users use this parameter: instead, since
they all just do variants of dma_map* they use the DMA cookie
to indicate whether a pre* callback has already been done for
a request when they decide how to handle it.
Delete the parameter from the callback and all users, as it is
just pointless cruft.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/mmc/core/core.c | 11 ++++-------
drivers/mmc/host/dw_mmc.c | 3 +--
drivers/mmc/host/jz4740_mmc.c | 3 +--
drivers/mmc/host/mmci.c | 3 +--
drivers/mmc/host/mtk-sd.c | 3 +--
drivers/mmc/host/omap_hsmmc.c | 3 +--
drivers/mmc/host/rtsx_pci_sdmmc.c | 3 +--
drivers/mmc/host/sdhci.c | 3 +--
include/linux/mmc/host.h | 3 +--
9 files changed, 12 insertions(+), 23 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 67e262315a50..faeea26efba5 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -611,18 +611,15 @@ EXPORT_SYMBOL(mmc_is_req_done);
* mmc_pre_req - Prepare for a new request
* @host: MMC host to prepare command
* @mrq: MMC request to prepare for
- * @is_first_req: true if there is no previous started request
- * that may run in parellel to this call, otherwise false
*
* mmc_pre_req() is called in prior to mmc_start_req() to let
* host prepare for the new request. Preparation of a request may be
* performed while another request is running on the host.
*/
-static void mmc_pre_req(struct mmc_host *host, struct mmc_request *mrq,
- bool is_first_req)
+static void mmc_pre_req(struct mmc_host *host, struct mmc_request *mrq)
{
if (host->ops->pre_req)
- host->ops->pre_req(host, mrq, is_first_req);
+ host->ops->pre_req(host, mrq);
}
/**
@@ -667,7 +664,7 @@ struct mmc_async_req *mmc_start_req(struct mmc_host *host,
/* Prepare a new request */
if (areq)
- mmc_pre_req(host, areq->mrq, !host->areq);
+ mmc_pre_req(host, areq->mrq);
if (host->areq) {
status = mmc_wait_for_data_req_done(host, host->areq->mrq, areq);
@@ -696,7 +693,7 @@ struct mmc_async_req *mmc_start_req(struct mmc_host *host,
/* prepare the request again */
if (areq)
- mmc_pre_req(host, areq->mrq, !host->areq);
+ mmc_pre_req(host, areq->mrq);
}
}
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 4fcbc4012ed0..969acbf0740c 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -883,8 +883,7 @@ static int dw_mci_pre_dma_transfer(struct dw_mci *host,
}
static void dw_mci_pre_req(struct mmc_host *mmc,
- struct mmc_request *mrq,
- bool is_first_req)
+ struct mmc_request *mrq)
{
struct dw_mci_slot *slot = mmc_priv(mmc);
struct mmc_data *data = mrq->data;
diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c
index 684087db170b..819ad32964fc 100644
--- a/drivers/mmc/host/jz4740_mmc.c
+++ b/drivers/mmc/host/jz4740_mmc.c
@@ -320,8 +320,7 @@ static int jz4740_mmc_start_dma_transfer(struct jz4740_mmc_host *host,
}
static void jz4740_mmc_pre_request(struct mmc_host *mmc,
- struct mmc_request *mrq,
- bool is_first_req)
+ struct mmc_request *mrq)
{
struct jz4740_mmc_host *host = mmc_priv(mmc);
struct mmc_data *data = mrq->data;
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index df990bb8c873..1ddbb64e385a 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -684,8 +684,7 @@ static void mmci_get_next_data(struct mmci_host *host, struct mmc_data *data)
next->dma_chan = NULL;
}
-static void mmci_pre_request(struct mmc_host *mmc, struct mmc_request *mrq,
- bool is_first_req)
+static void mmci_pre_request(struct mmc_host *mmc, struct mmc_request *mrq)
{
struct mmci_host *host = mmc_priv(mmc);
struct mmc_data *data = mrq->data;
diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
index 84e9afcb5c09..db185441966d 100644
--- a/drivers/mmc/host/mtk-sd.c
+++ b/drivers/mmc/host/mtk-sd.c
@@ -927,8 +927,7 @@ static void msdc_ops_request(struct mmc_host *mmc, struct mmc_request *mrq)
msdc_start_command(host, mrq, mrq->cmd);
}
-static void msdc_pre_req(struct mmc_host *mmc, struct mmc_request *mrq,
- bool is_first_req)
+static void msdc_pre_req(struct mmc_host *mmc, struct mmc_request *mrq)
{
struct msdc_host *host = mmc_priv(mmc);
struct mmc_data *data = mrq->data;
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 5f2f24a7360d..ad11c4cc12ed 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1565,8 +1565,7 @@ static void omap_hsmmc_post_req(struct mmc_host *mmc, struct mmc_request *mrq,
}
}
-static void omap_hsmmc_pre_req(struct mmc_host *mmc, struct mmc_request *mrq,
- bool is_first_req)
+static void omap_hsmmc_pre_req(struct mmc_host *mmc, struct mmc_request *mrq)
{
struct omap_hsmmc_host *host = mmc_priv(mmc);
diff --git a/drivers/mmc/host/rtsx_pci_sdmmc.c b/drivers/mmc/host/rtsx_pci_sdmmc.c
index 3ccaa1415f33..ecb99a8d2fa2 100644
--- a/drivers/mmc/host/rtsx_pci_sdmmc.c
+++ b/drivers/mmc/host/rtsx_pci_sdmmc.c
@@ -190,8 +190,7 @@ static int sd_pre_dma_transfer(struct realtek_pci_sdmmc *host,
return using_cookie;
}
-static void sdmmc_pre_req(struct mmc_host *mmc, struct mmc_request *mrq,
- bool is_first_req)
+static void sdmmc_pre_req(struct mmc_host *mmc, struct mmc_request *mrq)
{
struct realtek_pci_sdmmc *host = mmc_priv(mmc);
struct mmc_data *data = mrq->data;
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 48055666c655..eefe5ed73822 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2190,8 +2190,7 @@ static void sdhci_post_req(struct mmc_host *mmc, struct mmc_request *mrq,
data->host_cookie = COOKIE_UNMAPPED;
}
-static void sdhci_pre_req(struct mmc_host *mmc, struct mmc_request *mrq,
- bool is_first_req)
+static void sdhci_pre_req(struct mmc_host *mmc, struct mmc_request *mrq)
{
struct sdhci_host *host = mmc_priv(mmc);
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 5310f94be0ab..fdd13e0b8ec8 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -93,8 +93,7 @@ struct mmc_host_ops {
*/
void (*post_req)(struct mmc_host *host, struct mmc_request *req,
int err);
- void (*pre_req)(struct mmc_host *host, struct mmc_request *req,
- bool is_first_req);
+ void (*pre_req)(struct mmc_host *host, struct mmc_request *req);
void (*request)(struct mmc_host *host, struct mmc_request *req);
/*
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v2 0/7] mmc: core: Re-work CMD13 polling method for CMD6 for mmc
From: Adrian Hunter @ 2016-11-23 9:28 UTC (permalink / raw)
To: Ulf Hansson, linux-mmc
Cc: Jaehoon Chung, Linus Walleij, Chaotian Jing, Stephen Boyd,
Michael Walle, Yong Mao, Shawn Lin
In-Reply-To: <1479848173-20881-1-git-send-email-ulf.hansson@linaro.org>
On 22/11/16 22:56, Ulf Hansson wrote:
> Changes in v2:
>
> - From discussions around the difficulties for how to support CMD13
> polling for HS200, HS400 HS400ES, I decided to drop those patches for
> now. It's is particularly due to the need for tuning, after a speed mode
> switch, that makes it hard to rely on CMD13 polling for these cases.
> Perhaps we can allow CMD13 polling when switching to the intermediate
> speed modes, but let's deal with that outside of this series.
>
> - Folded in a new patch, which checks the SWITCH_ERROR bit in each
> CMD13 response when polling with CMD13. Patch v2 4/7.
>
> - No other changes made to the rest of the series.
>
For the whole series:
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
^ permalink raw reply
* [PATCH] mmc: dw_mmc: add missing codes for runtime resume
From: Joonyoung Shim @ 2016-11-23 9:33 UTC (permalink / raw)
To: linux-mmc; +Cc: jh80.chung, ulf.hansson, shawn.lin, jy0922.shim
The commit 64997de4fd17 ("mmc: dw_mmc: remove system PM callback") is
missing to call dw_mci_ctrl_reset(). This adds to call
dw_mci_ctrl_reset() and to handle error of clocks.
Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
drivers/mmc/host/dw_mmc.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index d426fa41bcce..25e21a20ee04 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -3303,6 +3303,17 @@ int dw_mci_runtime_resume(struct device *dev)
if (ret)
return ret;
+ if (!dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS)) {
+ clk_disable_unprepare(host->ciu_clk);
+
+ if (host->cur_slot &&
+ (mmc_can_gpio_cd(host->cur_slot->mmc) ||
+ !mmc_card_is_removable(host->cur_slot->mmc)))
+ clk_disable_unprepare(host->biu_clk);
+
+ return -ENODEV;
+ }
+
if (host->use_dma && host->dma_ops->init)
host->dma_ops->init(host);
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] mmc: block: delete packed command support
From: Jaehoon Chung @ 2016-11-23 9:40 UTC (permalink / raw)
To: Ulf Hansson, Arnd Bergmann
Cc: Alex Lemberg, Linus Walleij, linux-mmc@vger.kernel.org,
Chunyan Zhang, Baolin Wang, Namjae Jeon, Maya Erez, Luca Porzio
In-Reply-To: <CAPDyKFppA8P=KNPNp1D+9gTxTM4MN7eBJ+rdG4JzSrPRdYqYFQ@mail.gmail.com>
On 11/23/2016 01:06 AM, Ulf Hansson wrote:
> [...]
>
>>>
>>> Regarding "performance", are you merely thinking about increased
>>> throughput? With packed command we decrease the communication overhead
>>> with the card so less commands becomes sent/received.
>>>
>>> Or, did you also observed an improved behaviour of the card from a
>>> garbage collect point of view? In other words also a decreased latency
>>> when the device is becoming more and more used?
>>>
>>> Finally, did you compare the packed command, towards using the
>>> asynchronous request mechanisms (using the ->pre|post_req() mmc host
>>> ops)?
>>
>> The main point of command packing is that the device can be smarter
>> about garbage collection as well as combine sub-page sized writes.
>
> Ideally, yes! But I am not sure that is the case. Vendors would have to confirm.
>
> By following the evolution of development of the eMMC spec one could
> make some guesses. Let me try it :-).
>
> In the eMMC 4.5 spec, "data tag", "context ID" and "packed command"
> was added. It seems to me that "data tag" and "context ID" was
> intended to help the device to be smarter about garbage collection,
> while "packed command" is about reducing overhead.
>
> The below is quoted from the packed command section in the spec:
>
> "Read and write commands can be packed in groups of commands (either
> all read or all write) that transfer the data for all commands in the
> group in one transfer on the bus, to reduce overheads."
>
>>
>> The communication overhead is nearly irrelevant in comparison,
>> and we would probably not have done anything for that.
>
> I think we did.
>
> And that's particularly why I am interested to see a fresh comparison
> against the async request transfer mechanism.
>
>>
>>>>> As far as I can say from reviewing the mobile (Android)
>>>>> platforms kernel source (from different vendors),
>>>>> many of them are enabling Packed Commands.
>>>>
>>>> Actually, some shipping Samsung devices with eMMC4.5 might be used packed command.
>>>> (For Android/Tizen OS and ARTIK boards)
>>>
>>> Thanks for sharing this information!
>>>
>>> It seems like we need to run another round of performance
>>> measurements, as to get some fresh number of the benefit of packed
>>> command.
>>> I would really appreciate if you could help out with that.
>>
>> As far as I'm concerned, there are already two conclusions and
>> I don't think those measurements would help much:
>>
>> - It's a problem that none of our upstream drivers support this
>> feature, and we really want them to do so, at least after the
>> blk_mq change.
>>
>> - Linus' analysis is still valid: there is no regression in removing
>> it from the traditional blk code today, anyone who has a private
>> driver that uses it can simply revert the removal in their next
>> private product release.
It looks like makes sense..I agreed this..
>>
>> If removing it helps us enable blk_mq support more easily, then
>> I think we can take out the packed command handling, but we have
>> to be prepared to put it back later on top of blk_mq.
>>
>
> Thanks for the summary. This do seems like a nice approach.
>
> Let's see what other people thinks about this.
>
> Kind regards
> Uffe
> --
> 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
>
>
>
^ permalink raw reply
* [PATCH] mmc: dw_mmc: exynos: fix to call suspend callback
From: Joonyoung Shim @ 2016-11-23 9:36 UTC (permalink / raw)
To: linux-mmc; +Cc: jh80.chung, ulf.hansson, jy0922.shim
The dw_mmc-exynos should be RPM_ACTIVE on probe() to call suspend
callback of runtime PM in pm_runtime_force_suspend() during first system
suspend. Also call pm_runtime_get_noresume() on probe() because it
doesn't call suspend/resume callback by runtime PM now.
Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
drivers/mmc/host/dw_mmc-exynos.c | 28 ++++++++++++++++++++++++++--
1 file changed, 26 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index e2439a108873..e1335289316c 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -516,10 +516,34 @@ static int dw_mci_exynos_probe(struct platform_device *pdev)
{
const struct dw_mci_drv_data *drv_data;
const struct of_device_id *match;
+ int ret;
match = of_match_node(dw_mci_exynos_match, pdev->dev.of_node);
drv_data = match->data;
- return dw_mci_pltfm_register(pdev, drv_data);
+
+ pm_runtime_get_noresume(&pdev->dev);
+ pm_runtime_set_active(&pdev->dev);
+ pm_runtime_enable(&pdev->dev);
+
+ ret = dw_mci_pltfm_register(pdev, drv_data);
+ if (ret) {
+ pm_runtime_disable(&pdev->dev);
+ pm_runtime_set_suspended(&pdev->dev);
+ pm_runtime_put_noidle(&pdev->dev);
+
+ return ret;
+ }
+
+ return 0;
+}
+
+static int dw_mci_exynos_remove(struct platform_device *pdev)
+{
+ pm_runtime_disable(&pdev->dev);
+ pm_runtime_set_suspended(&pdev->dev);
+ pm_runtime_put_noidle(&pdev->dev);
+
+ return dw_mci_pltfm_remove(pdev);
}
static const struct dev_pm_ops dw_mci_exynos_pmops = {
@@ -535,7 +559,7 @@ static int dw_mci_exynos_probe(struct platform_device *pdev)
static struct platform_driver dw_mci_exynos_pltfm_driver = {
.probe = dw_mci_exynos_probe,
- .remove = dw_mci_pltfm_remove,
+ .remove = dw_mci_exynos_remove,
.driver = {
.name = "dwmmc_exynos",
.of_match_table = dw_mci_exynos_match,
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] mmc: block: delete packed command support
From: Jaehoon Chung @ 2016-11-23 9:34 UTC (permalink / raw)
To: Linus Walleij, Ulf Hansson
Cc: Alex Lemberg, Arnd Bergmann, linux-mmc@vger.kernel.org,
Chunyan Zhang, Baolin Wang, Namjae Jeon, Maya Erez, Luca Porzio
In-Reply-To: <CACRpkdZ_aaq5hA=LG5k0OM52Sx-ca8ntdpohsi8j47VH60ucNw@mail.gmail.com>
Hi Linus,
On 11/22/2016 09:49 PM, Linus Walleij wrote:
> On Tue, Nov 22, 2016 at 9:49 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> On 22 November 2016 at 04:53, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>
>>>> Correct, in general there is no value in using packed for Read.
>>>> But I can’t say this for all existing flash management solution.
>>>> The eMMC spec allows to use it for Read as well.
>>>
>>> As i know, when packed command had implemented, early eMMC had the firmware problem
>>> for Packed Read operation. but so I can't say Packed Read doesn't have the benefit for performance.
>>> But Packed Write command can see the benefit for performance.
>>
>> Regarding "performance", are you merely thinking about increased
>> throughput? With packed command we decrease the communication overhead
>> with the card so less commands becomes sent/received.
>>
>> Or, did you also observed an improved behaviour of the card from a
>> garbage collect point of view? In other words also a decreased latency
>> when the device is becoming more and more used?
>>
>> Finally, did you compare the packed command, towards using the
>> asynchronous request mechanisms (using the ->pre|post_req() mmc host
>> ops)?
>
> Packed write (and read, if we had implemented it) can only happen when
> we get a number of requests to different areas of the card.
>
> If they are to consecutive sectors (such as with a dd-test) it will not
> improve performance, as that case is already optimized by the block
> layer by front-merging of the requests into large chunks, I guess?
>
> Indeed commit ce39f9d17c14 mentions improvement on lmdd but not
> how it was invoked. I suspect it was invoked with random writes...
Well, I don't know those improvements was right or not..
>
> It is important to do everything we can to improve random small writes
> though, and it seems packed command can do that.
Agreed...but i have checked with latest kernel and older version(v3.18).
I saw the random write performance with small chunk size was increased with v3.18, not v4.9-rc5.
I don't know what happens.
>
> (...)
>>> Actually, some shipping Samsung devices with eMMC4.5 might be used packed command.
>>> (For Android/Tizen OS and ARTIK boards)
>>
>> Thanks for sharing this information!
>>
>> It seems like we need to run another round of performance
>> measurements, as to get some fresh number of the benefit of packed
>> command.
>> I would really appreciate if you could help out with that.
>
> I would add: do it on the upstream kernel and submit the stuff needed
> to make it work for you out-of-the box.
>
> This is on Exynos I guess?
>
> Can we have this flag set for the Exynos host controller, and/or
> proper DT bindings to mark it as packed command enabled?
Actually, I can add the property or flags..but i think it's meaningless..
If you ask me my opinion, i don't want to add the enabling packed command at this time.
Because i didn't see the increasing performance dynamically at this time.
But i can say..when we had applied the packed command (2~3years ago.), there were some benefit to use this command.
Yes..my comments might be confused..
but i just mentioned about some Samsung guys are using packed command for their devices with eMMC4.5.
I'm not sure which kernel version they used..maybe older version.
>
> Hell I don't even know if this is a feature that needs anything special
> from the host controller. Does it? Should we rather just enable it for all
> host controllers if the card supports packed command? I'm lost.
>
I'm also feeling likes you...Now I'm checking about performance with all exynos devices that i have.
Best Regards,
Jaehoon Chung
> Yours,
> Linus Walleij
> --
> 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
>
>
>
^ permalink raw reply
* Re: [PATCH v2 0/7] mmc: core: Re-work CMD13 polling method for CMD6 for mmc
From: Linus Walleij @ 2016-11-23 8:55 UTC (permalink / raw)
To: Ulf Hansson
Cc: linux-mmc@vger.kernel.org, Jaehoon Chung, Adrian Hunter,
Chaotian Jing, Stephen Boyd, Michael Walle, Yong Mao, Shawn Lin
In-Reply-To: <1479848173-20881-1-git-send-email-ulf.hansson@linaro.org>
On Tue, Nov 22, 2016 at 9:56 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> Changes in v2:
>
> - From discussions around the difficulties for how to support CMD13
> polling for HS200, HS400 HS400ES, I decided to drop those patches for
> now. It's is particularly due to the need for tuning, after a speed mode
> switch, that makes it hard to rely on CMD13 polling for these cases.
> Perhaps we can allow CMD13 polling when switching to the intermediate
> speed modes, but let's deal with that outside of this series.
ALso v2 works like a charm on APQ8060 Dragonboard:
Tested-by: Linus Walleij <linus.walleij@linaro.org>
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