Re: [PATCH] irqchip/stm32: fix return value of stm32_exti_h_set_affinity
Hi Marc Just a gentleman ping about this patch. I verified, you could always apply this patch on linux master branch. Regards Ludo Le 6/12/20 à 9:29 AM, Ludovic Barre a écrit : exti hardware point of view, there is no specific action on set_affinity. So the affinity must be forwarded to parent if there is a descendent irqchips, otherwise just return IRQ_SET_MASK_OK_DONE. Signed-off-by: Ludovic Barre --- drivers/irqchip/irq-stm32-exti.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/irqchip/irq-stm32-exti.c b/drivers/irqchip/irq-stm32-exti.c index faa8482c8246..1a0a60ee7140 100644 --- a/drivers/irqchip/irq-stm32-exti.c +++ b/drivers/irqchip/irq-stm32-exti.c @@ -555,7 +555,7 @@ static int stm32_exti_h_set_affinity(struct irq_data *d, if (d->parent_data->chip) return irq_chip_set_affinity_parent(d, dest, force); - return -EINVAL; + return IRQ_SET_MASK_OK_DONE; } static int __maybe_unused stm32_exti_h_suspend(void)
[PATCH] irqchip/stm32: fix return value of stm32_exti_h_set_affinity
exti hardware point of view, there is no specific action on set_affinity. So the affinity must be forwarded to parent if there is a descendent irqchips, otherwise just return IRQ_SET_MASK_OK_DONE. Signed-off-by: Ludovic Barre --- drivers/irqchip/irq-stm32-exti.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/irqchip/irq-stm32-exti.c b/drivers/irqchip/irq-stm32-exti.c index faa8482c8246..1a0a60ee7140 100644 --- a/drivers/irqchip/irq-stm32-exti.c +++ b/drivers/irqchip/irq-stm32-exti.c @@ -555,7 +555,7 @@ static int stm32_exti_h_set_affinity(struct irq_data *d, if (d->parent_data->chip) return irq_chip_set_affinity_parent(d, dest, force); - return -EINVAL; + return IRQ_SET_MASK_OK_DONE; } static int __maybe_unused stm32_exti_h_suspend(void) -- 2.17.1
[PATCH] mmc: mmci: add sdio datactrl mask for sdmmc revisions
This patch adds datactrl_mask_sdio for sdmmc revisions. sdmmc revisions used same bit of previous ST variant. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index a69d6a0c2e15..b5a41a7ce165 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -267,6 +267,7 @@ static struct variant_data variant_stm32_sdmmc = { .datalength_bits= 25, .datactrl_blocksz = 14, .datactrl_any_blocksz = true, + .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN, .stm32_idmabsize_mask = GENMASK(12, 5), .busy_timeout = true, .busy_detect= true, @@ -292,6 +293,7 @@ static struct variant_data variant_stm32_sdmmcv2 = { .datalength_bits= 25, .datactrl_blocksz = 14, .datactrl_any_blocksz = true, + .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN, .stm32_idmabsize_mask = GENMASK(16, 5), .dma_lli= true, .busy_timeout = true, -- 2.17.1
[PATCH 1/2] mmc: mmci_sdmmc: fix DMA API warning overlapping mappings
Turning on CONFIG_DMA_API_DEBUG_SG results in the following warning: WARNING: CPU: 1 PID: 20 at kernel/dma/debug.c:500 add_dma_entry+0x16c/0x17c DMA-API: exceeded 7 overlapping mappings of cacheline 0x031d2645 Modules linked in: CPU: 1 PID: 20 Comm: kworker/1:1 Not tainted 5.5.0-rc2-00021-gdeda30999c2b-dirty #49 Hardware name: STM32 (Device Tree Support) Workqueue: events_freezable mmc_rescan [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (dump_stack+0xc0/0xd4) [] (dump_stack) from [] (__warn+0xd0/0xf8) [] (__warn) from [] (warn_slowpath_fmt+0x94/0xb8) [] (warn_slowpath_fmt) from [] (add_dma_entry+0x16c/0x17c) [] (add_dma_entry) from [] (debug_dma_map_sg+0xe4/0x3d4) [] (debug_dma_map_sg) from [] (sdmmc_idma_prep_data+0x94/0xf8) [] (sdmmc_idma_prep_data) from [] (mmci_prep_data+0x2c/0xb0) [] (mmci_prep_data) from [] (mmci_start_data+0x134/0x2f0) [] (mmci_start_data) from [] (mmci_request+0xe8/0x154) [] (mmci_request) from [] (mmc_start_request+0x94/0xbc) DMA api debug brings to light leaking dma-mappings, dma_map_sg and dma_unmap_sg are not correctly balanced. If a request is prepared, the dma_map/unmap are done in asynchronous call pre_req (prep_data) and post_req (unprep_data). In this case the dma-mapping is right balanced. But if the request was not prepared, the data->host_cookie is define to zero and the dma_map/unmap must be done in the request. The dma_map is called by mmci_dma_start (prep_data), but there is no dma_unmap in this case. This patch adds dma_unmap_sg when the dma is finalized and the data cookie is zero (request not prepared). Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci_stm32_sdmmc.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c index 14f99d8aa3f0..2965b1c062e1 100644 --- a/drivers/mmc/host/mmci_stm32_sdmmc.c +++ b/drivers/mmc/host/mmci_stm32_sdmmc.c @@ -188,6 +188,9 @@ static int sdmmc_idma_start(struct mmci_host *host, unsigned int *datactrl) static void sdmmc_idma_finalize(struct mmci_host *host, struct mmc_data *data) { writel_relaxed(0, host->base + MMCI_STM32_IDMACTRLR); + + if (!data->host_cookie) + sdmmc_idma_unprep_data(host, data, 0); } static void mmci_sdmmc_set_clkreg(struct mmci_host *host, unsigned int desired) -- 2.17.1
[PATCH 2/2] mmc: mmci_sdmmc: fix DMA API warning max segment size
Turning on CONFIG_DMA_API_DEBUG_SG results in the following warning: WARNING: CPU: 1 PID: 85 at kernel/dma/debug.c:1302 debug_dma_map_sg+0x2a0/0x3cc mmci-pl18x 58005000.sdmmc: DMA-API: mapping sg segment longer than device claims to support [len=126976] [max=65536] dma api debug checks and compares the segment size to dma_get_max_seg_size (dev->dma_parms->max_segment_size), the sdmmc variant has an internal DMA and should define its max_segment_size constraint to avoid this warning. This Patch defines the dev->dma_parms->max_segment_size with the constraint already set for mmc core (host->mmc->max_seg_size). Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci_stm32_sdmmc.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c index 2965b1c062e1..51db30acf4dc 100644 --- a/drivers/mmc/host/mmci_stm32_sdmmc.c +++ b/drivers/mmc/host/mmci_stm32_sdmmc.c @@ -119,20 +119,19 @@ static void sdmmc_idma_unprep_data(struct mmci_host *host, static int sdmmc_idma_setup(struct mmci_host *host) { struct sdmmc_idma *idma; + struct device *dev = mmc_dev(host->mmc); - idma = devm_kzalloc(mmc_dev(host->mmc), sizeof(*idma), GFP_KERNEL); + idma = devm_kzalloc(dev, sizeof(*idma), GFP_KERNEL); if (!idma) return -ENOMEM; host->dma_priv = idma; if (host->variant->dma_lli) { - idma->sg_cpu = dmam_alloc_coherent(mmc_dev(host->mmc), - SDMMC_LLI_BUF_LEN, + idma->sg_cpu = dmam_alloc_coherent(dev, SDMMC_LLI_BUF_LEN, &idma->sg_dma, GFP_KERNEL); if (!idma->sg_cpu) { - dev_err(mmc_dev(host->mmc), - "Failed to alloc IDMA descriptor\n"); + dev_err(dev, "Failed to alloc IDMA descriptor\n"); return -ENOMEM; } host->mmc->max_segs = SDMMC_LLI_BUF_LEN / @@ -143,7 +142,7 @@ static int sdmmc_idma_setup(struct mmci_host *host) host->mmc->max_seg_size = host->mmc->max_req_size; } - return 0; + return dma_set_max_seg_size(dev, host->mmc->max_seg_size); } static int sdmmc_idma_start(struct mmci_host *host, unsigned int *datactrl) -- 2.17.1
[PATCH 0/2] mmc: mmci_sdmmc: fix dma api warnings
This patch series fixes warnings see with DMA_API_DEBUG_SG=y Ludovic Barre (2): mmc: mmci_sdmmc: fix DMA API warning overlapping mappings mmc: mmci_sdmmc: fix DMA API warning max segment size drivers/mmc/host/mmci_stm32_sdmmc.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) -- 2.17.1
[PATCH 0/2] mmc: add unstuck function if host is in deadlock state
From: Ludovic Barre As discussed in this thread: https://patchwork.kernel.org/patch/10786421/ After a request, the host could be in deadlock state, and waiting for a specific action to unstuck the hardware block before resending a new command. This series adds mmc_hw_unstuck callback (structure mmc_host_ops) before resending a new command (call in mmc_blk_mq_rw_recovery, mmc_wait_for_req_done). Ludovic Barre (2): mmc: add unstuck function if host is in deadlock state mmc: mmci: add unstuck feature drivers/mmc/core/block.c | 11 +++ drivers/mmc/core/core.c | 35 +-- drivers/mmc/host/mmci.c | 23 +-- include/linux/mmc/core.h | 1 + include/linux/mmc/host.h | 7 +++ 5 files changed, 73 insertions(+), 4 deletions(-) -- 2.17.1
[PATCH 2/2] mmc: mmci: add unstuck feature
From: Ludovic Barre On busy_timeout feature if busy is too long on R1B command a datatimeout occurs and a specific actions is needed to clear the DPSM bit: -reset the controller to clear the DPSM bit. -restore registers: clk, pwr, datactrl. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 23 +-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 40e72c30ea84..dafba4e0afc5 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -1320,7 +1320,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, cmd->error = -EILSEQ; } else if (host->variant->busy_timeout && busy_resp && status & MCI_DATATIMEOUT) { - cmd->error = -ETIMEDOUT; + cmd->error = -EDEADLK; } else { cmd->resp[0] = readl(base + MMCIRESPONSE0); cmd->resp[1] = readl(base + MMCIRESPONSE1); @@ -1332,7 +1332,6 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, if (host->data) { /* Terminate the DMA transfer */ mmci_dma_error(host); - mmci_stop_data(host); if (host->variant->cmdreg_stop && cmd->error) { mmci_stop_command(host); @@ -1787,6 +1786,25 @@ static int mmci_sig_volt_switch(struct mmc_host *mmc, struct mmc_ios *ios) return ret; } +static void mmci_hw_unstuck(struct mmc_host *mmc) +{ + struct mmci_host *host = mmc_priv(mmc); + unsigned long flags; + + if (host->rst) { + reset_control_assert(host->rst); + udelay(2); + reset_control_deassert(host->rst); + } + + spin_lock_irqsave(&host->lock, flags); + writel(host->clk_reg, host->base + MMCICLOCK); + writel(host->pwr_reg, host->base + MMCIPOWER); + writel(MCI_IRQENABLE | host->variant->start_err, + host->base + MMCIMASK0); + spin_unlock_irqrestore(&host->lock, flags); +} + static struct mmc_host_ops mmci_ops = { .request= mmci_request, .pre_req= mmci_pre_request, @@ -1795,6 +1813,7 @@ static struct mmc_host_ops mmci_ops = { .get_ro = mmc_gpio_get_ro, .get_cd = mmci_get_cd, .start_signal_voltage_switch = mmci_sig_volt_switch, + .hw_unstuck = mmci_hw_unstuck, }; static int mmci_of_parse(struct device_node *np, struct mmc_host *mmc) -- 2.17.1
[PATCH 1/2] mmc: add unstuck function if host is in deadlock state
From: Ludovic Barre After a request a host may be in deadlock state, and wait a specific action to unstuck the hardware block before re-sending a new command. This patch adds an optional callback mmc_hw_unstuck which allows the host to unstuck the controller. In order to avoid a critical context, this callback must be called when the request is completed. Depending the mmc request, the completion function is defined by mrq->done and could be in block.c or core.c. mmc_hw_unstuck is called if the host returns an cmd/sbc/stop/data DEADLK error. Signed-off-by: Ludovic Barre --- drivers/mmc/core/block.c | 11 +++ drivers/mmc/core/core.c | 35 +-- include/linux/mmc/core.h | 1 + include/linux/mmc/host.h | 7 +++ 4 files changed, 52 insertions(+), 2 deletions(-) diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index 2c71a434c915..2f723e2f5fde 100644 --- a/drivers/mmc/core/block.c +++ b/drivers/mmc/core/block.c @@ -1799,6 +1799,17 @@ static void mmc_blk_mq_rw_recovery(struct mmc_queue *mq, struct request *req) u32 blocks; int err; + /* +* if the host return a deadlock, it needs to be unstuck +* before to send a new command. +*/ + if (brq->sbc.error == -EDEADLK || brq->cmd.error == -EDEADLK || + brq->stop.error == -EDEADLK || brq->data.error == -EDEADLK) { + pr_err("%s: host is in bad state, must be unstuck\n", + req->rq_disk->disk_name); + mmc_hw_unstuck(card->host); + } + /* * Some errors the host driver might not have seen. Set the number of * bytes transferred to zero in that case. diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 221127324709..43fe59a7403b 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -397,6 +397,7 @@ static int __mmc_start_req(struct mmc_host *host, struct mmc_request *mrq) void mmc_wait_for_req_done(struct mmc_host *host, struct mmc_request *mrq) { struct mmc_command *cmd; + int sbc_err, stop_err, data_err; while (1) { wait_for_completion(&mrq->completion); @@ -420,8 +421,24 @@ void mmc_wait_for_req_done(struct mmc_host *host, struct mmc_request *mrq) mmc_hostname(host), __func__); } } - if (!cmd->error || !cmd->retries || - mmc_card_removed(host->card)) + + sbc_err = mrq->sbc ? mrq->sbc->error : 0; + stop_err = mrq->stop ? mrq->stop->error : 0; + data_err = mrq->data ? mrq->data->error : 0; + + /* +* if the host return a deadlock, it needs to be unstuck +* before to send a new command. +*/ + if (cmd->error == -EDEADLK || sbc_err == -EDEADLK || + stop_err == -EDEADLK || data_err == -EDEADLK) { + pr_debug("%s: host is in bad state, must be unstuck\n", +mmc_hostname(host)); + mmc_hw_unstuck(host); + } + + if ((!cmd->error && !sbc_err && !stop_err && !data_err) || + !cmd->retries || mmc_card_removed(host->card)) break; mmc_retune_recheck(host); @@ -430,6 +447,12 @@ void mmc_wait_for_req_done(struct mmc_host *host, struct mmc_request *mrq) mmc_hostname(host), cmd->opcode, cmd->error); cmd->retries--; cmd->error = 0; + if (mrq->sbc) + mrq->sbc->error = 0; + if (mrq->stop) + mrq->stop->error = 0; + if (mrq->data) + mrq->data->error = 0; __mmc_start_request(host, mrq); } @@ -2161,6 +2184,14 @@ int mmc_sw_reset(struct mmc_host *host) } EXPORT_SYMBOL(mmc_sw_reset); +void mmc_hw_unstuck(struct mmc_host *host) +{ + if (!host->ops->hw_unstuck) + return; + host->ops->hw_unstuck(host); +} +EXPORT_SYMBOL(mmc_hw_unstuck); + static int mmc_rescan_try_freq(struct mmc_host *host, unsigned freq) { host->f_init = freq; diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h index b7ba8810a3b5..eb10b8194073 100644 --- a/include/linux/mmc/core.h +++ b/include/linux/mmc/core.h @@ -173,6 +173,7 @@ void mmc_wait_for_req(struct mmc_host *host, struct mmc_request *mrq); int mmc_wait_for_cmd(struct mmc_host *host, struct mmc_command *cmd, int retries); +void mmc_hw_unstuck(struct mmc_host *host); int mmc_hw_reset(struct mmc_host *host); int mmc_sw_reset(struct mmc_host *host); void m
[PATCH V7 3/3] mmc: mmci: sdmmc: add busy_complete callback
From: Ludovic Barre This patch adds a specific busy_complete callback for sdmmc variant. sdmmc has 2 status flags: -busyd0: This is a hardware status flag (inverted value of d0 line). it does not generate an interrupt. -busyd0end: This indicates only end of busy following a CMD response. On busy to Not busy changes, an interrupt is generated (if unmask) and BUSYD0END status flag is set. Status flag is cleared by writing corresponding interrupt clear bit in MMCICLEAR. The legacy busy completion has no dedicated interrupt for the end of busy, so it's must monitor step by step the busy progression. On sdmmc variant, this procedure is not needed, it's just need to wait the busyd0end interrupt. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 4 +++ drivers/mmc/host/mmci.h | 1 + drivers/mmc/host/mmci_stm32_sdmmc.c | 42 + 3 files changed, 47 insertions(+) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 5e53f9b6d82a..40e72c30ea84 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -262,6 +262,10 @@ static struct variant_data variant_stm32_sdmmc = { .datalength_bits= 25, .datactrl_blocksz = 14, .stm32_idmabsize_mask = GENMASK(12, 5), + .busy_timeout = true, + .busy_detect= true, + .busy_detect_flag = MCI_STM32_BUSYD0, + .busy_detect_mask = MCI_STM32_BUSYD0ENDMASK, .init = sdmmc_variant_init, }; diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h index 2a0b98f98c36..158e1231aa23 100644 --- a/drivers/mmc/host/mmci.h +++ b/drivers/mmc/host/mmci.h @@ -164,6 +164,7 @@ #define MCI_ST_CARDBUSY(1 << 24) /* Extended status bits for the STM32 variants */ #define MCI_STM32_BUSYD0 BIT(20) +#define MCI_STM32_BUSYD0ENDBIT(21) #define MMCICLEAR 0x038 #define MCI_CMDCRCFAILCLR (1 << 0) diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c index 8e83ae6920ae..1de855d29ad4 100644 --- a/drivers/mmc/host/mmci_stm32_sdmmc.c +++ b/drivers/mmc/host/mmci_stm32_sdmmc.c @@ -282,6 +282,47 @@ static u32 sdmmc_get_dctrl_cfg(struct mmci_host *host) return datactrl; } +static bool sdmmc_busy_complete(struct mmci_host *host, u32 status, u32 err_msk) +{ + void __iomem *base = host->base; + u32 busy_d0, busy_d0end, mask, sdmmc_status; + + mask = readl_relaxed(base + MMCIMASK0); + sdmmc_status = readl_relaxed(base + MMCISTATUS); + busy_d0end = sdmmc_status & MCI_STM32_BUSYD0END; + busy_d0 = sdmmc_status & MCI_STM32_BUSYD0; + + /* complete if there is an error or busy_d0end */ + if ((status & err_msk) || busy_d0end) + goto complete; + + /* +* On response the busy signaling is reflected in the BUSYD0 flag. +* if busy_d0 is in-progress we must activate busyd0end interrupt +* to wait this completion. Else this request has no busy step. +*/ + if (busy_d0) { + if (!host->busy_status) { + writel_relaxed(mask | host->variant->busy_detect_mask, + base + MMCIMASK0); + host->busy_status = status & + (MCI_CMDSENT | MCI_CMDRESPEND); + } + return false; + } + +complete: + if (host->busy_status) { + writel_relaxed(mask & ~host->variant->busy_detect_mask, + base + MMCIMASK0); + writel_relaxed(host->variant->busy_detect_mask, + base + MMCICLEAR); + host->busy_status = 0; + } + + return true; +} + static struct mmci_host_ops sdmmc_variant_ops = { .validate_data = sdmmc_idma_validate_data, .prep_data = sdmmc_idma_prep_data, @@ -292,6 +333,7 @@ static struct mmci_host_ops sdmmc_variant_ops = { .dma_finalize = sdmmc_idma_finalize, .set_clkreg = mmci_sdmmc_set_clkreg, .set_pwrreg = mmci_sdmmc_set_pwrreg, + .busy_complete = sdmmc_busy_complete, }; void sdmmc_variant_init(struct mmci_host *host) -- 2.17.1
[PATCH V7 2/3] mmc: mmci: add busy_complete callback
From: Ludovic Barre This patch adds busy_completion callback at mmci_host_ops to allow to define a specific busy completion by variant. The legacy code corresponding to busy completion used by ux500 variants is moved to ux500_busy_complete function. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 134 +--- drivers/mmc/host/mmci.h | 1 + 2 files changed, 73 insertions(+), 62 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index ed0b40287dea..5e53f9b6d82a 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -44,6 +44,7 @@ #define DRIVER_NAME "mmci-pl18x" static void mmci_variant_init(struct mmci_host *host); +static void ux500_variant_init(struct mmci_host *host); static void ux500v2_variant_init(struct mmci_host *host); static unsigned int fmax = 515633; @@ -184,7 +185,7 @@ static struct variant_data variant_ux500 = { .irq_pio_mask = MCI_IRQ_PIO_MASK, .start_err = MCI_STARTBITERR, .opendrain = MCI_OD, - .init = mmci_variant_init, + .init = ux500_variant_init, }; static struct variant_data variant_ux500v2 = { @@ -610,6 +611,67 @@ static u32 ux500v2_get_dctrl_cfg(struct mmci_host *host) return MCI_DPSM_ENABLE | (host->data->blksz << 16); } +static bool ux500_busy_complete(struct mmci_host *host, u32 status, u32 err_msk) +{ + void __iomem *base = host->base; + + /* +* Before unmasking for the busy end IRQ, confirm that the +* command was sent successfully. To keep track of having a +* command in-progress, waiting for busy signaling to end, +* store the status in host->busy_status. +* +* Note that, the card may need a couple of clock cycles before +* it starts signaling busy on DAT0, hence re-read the +* MMCISTATUS register here, to allow the busy bit to be set. +* Potentially we may even need to poll the register for a +* while, to allow it to be set, but tests indicates that it +* isn't needed. +*/ + if (!host->busy_status && !(status & err_msk) && + (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) { + writel(readl(base + MMCIMASK0) | + host->variant->busy_detect_mask, + base + MMCIMASK0); + + host->busy_status = status & (MCI_CMDSENT | MCI_CMDRESPEND); + return false; + } + + /* +* If there is a command in-progress that has been successfully +* sent, then bail out if busy status is set and wait for the +* busy end IRQ. +* +* Note that, the HW triggers an IRQ on both edges while +* monitoring DAT0 for busy completion, but there is only one +* status bit in MMCISTATUS for the busy state. Therefore +* both the start and the end interrupts needs to be cleared, +* one after the other. So, clear the busy start IRQ here. +*/ + if (host->busy_status && + (status & host->variant->busy_detect_flag)) { + writel(host->variant->busy_detect_mask, base + MMCICLEAR); + return false; + } + + /* +* If there is a command in-progress that has been successfully +* sent and the busy bit isn't set, it means we have received +* the busy end IRQ. Clear and mask the IRQ, then continue to +* process the command. +*/ + if (host->busy_status) { + writel(host->variant->busy_detect_mask, base + MMCICLEAR); + + writel(readl(base + MMCIMASK0) & + ~host->variant->busy_detect_mask, base + MMCIMASK0); + host->busy_status = 0; + } + + return true; +} + /* * All the DMA operation mode stuff goes inside this ifdef. * This assumes that you have a generic DMA device interface, @@ -953,9 +1015,16 @@ static void mmci_variant_init(struct mmci_host *host) host->ops = &mmci_variant_ops; } +static void ux500_variant_init(struct mmci_host *host) +{ + host->ops = &mmci_variant_ops; + host->ops->busy_complete = ux500_busy_complete; +} + static void ux500v2_variant_init(struct mmci_host *host) { host->ops = &mmci_variant_ops; + host->ops->busy_complete = ux500_busy_complete; host->ops->get_datactrl_cfg = ux500v2_get_dctrl_cfg; } @@ -1235,68 +1304,9 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, return; /* Handle busy detection on DAT0 if the variant supports it. */ - if (busy_resp && host->variant->busy_detect) { - - /* -* Be
[PATCH V7 0/3] mmc: mmci: add busy detect for stm32 sdmmc variant
From: Ludovic Barre This patch series adds busy detect for stm32 sdmmc variant. Some adaptations are required: -On sdmmc the data timer is started on data transfert and busy state, so we must add hardware busy timeout support. -Add busy_complete callback at mmci_host_ops to allow to define a specific busy completion by variant. -Add sdmmc busy_complete callback. V7: -Patch 1/3: rephrasing like proposed (thx ulf) -If busy timeout is undefined => increase to 10s -Keep busy_detect. -Patch 3/3: rephrasing comment header -Avoid twice read of status register -Avoid writing in MMCIMASK0 & MMCICLEAR if not modified V6: -mmci_start_command: set datatimer only on rsp_busy flag (remove host->mrq->data). -move max_busy_timeout in set_ios callback. -typo fix: err_msk, clks on one lines. V5: -Replaces !cmd->data to !host->mrq->data to avoid overwrite of datatimer register by the first command (cmd23, without data) of SBC request. V4: -Re-work with busy_complete callback -In series, move "mmc: mmci: add hardware busy timeout feature" in first to simplify busy_complete prototype with err_msk parameter. V3: -rebase on latest mmc next -replace re-read by status parameter. V2: -mmci_cmd_irq cleanup in separate patch. -simplify the busy_detect_flag exclude -replace sdmmc specific comment in "mmc: mmci: avoid fake busy polling in mmci_irq" to focus on common behavior Ludovic Barre (3): mmc: mmci: add hardware busy timeout feature mmc: mmci: add busy_complete callback mmc: mmci: sdmmc: add busy_complete callback drivers/mmc/host/mmci.c | 178 +--- drivers/mmc/host/mmci.h | 5 + drivers/mmc/host/mmci_stm32_sdmmc.c | 42 +++ 3 files changed, 159 insertions(+), 66 deletions(-) -- 2.17.1
[PATCH V7 1/3] mmc: mmci: add hardware busy timeout feature
From: Ludovic Barre In the stm32_sdmmc variant, the datatimer is active not only during data transfers with the DPSM, but also while waiting for the busyend IRQs from commands having the MMC_RSP_BUSY flag set. This leads to an incorrect IRQ being raised to signal MCI_DATATIMEOUT error, which simply breaks the behaviour. Address this by updating the datatimer value before sending a command having the MMC_RSP_BUSY flag set. To inform the mmc core about the maximum supported busy timeout, which also depends on the current clock rate, set ->max_busy_timeout (in ms). Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 42 - drivers/mmc/host/mmci.h | 3 +++ 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index e14003e52058..ed0b40287dea 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -1075,6 +1075,7 @@ static void mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c) { void __iomem *base = host->base; + unsigned long long clks; dev_dbg(mmc_dev(host->mmc), "op %02x arg %08x flags %08x\n", cmd->opcode, cmd->arg, cmd->flags); @@ -1097,6 +1098,16 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c) else c |= host->variant->cmdreg_srsp; } + + if (host->variant->busy_timeout && cmd->flags & MMC_RSP_BUSY) { + if (!cmd->busy_timeout) + cmd->busy_timeout = 10 * MSEC_PER_SEC; + + clks = (unsigned long long)cmd->busy_timeout * host->cclk; + do_div(clks, MSEC_PER_SEC); + writel_relaxed(clks, host->base + MMCIDATATIMER); + } + if (/*interrupt*/0) c |= MCI_CPSM_INTERRUPT; @@ -1201,6 +1212,7 @@ static void mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, unsigned int status) { + u32 err_msk = MCI_CMDCRCFAIL | MCI_CMDTIMEOUT; void __iomem *base = host->base; bool sbc, busy_resp; @@ -1215,8 +1227,11 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, * handling. Note that we tag on any latent IRQs postponed * due to waiting for busy status. */ - if (!((status|host->busy_status) & - (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT|MCI_CMDSENT|MCI_CMDRESPEND))) + if (host->variant->busy_timeout && busy_resp) + err_msk |= MCI_DATATIMEOUT; + + if (!((status | host->busy_status) & + (err_msk | MCI_CMDSENT | MCI_CMDRESPEND))) return; /* Handle busy detection on DAT0 if the variant supports it. */ @@ -1235,8 +1250,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, * while, to allow it to be set, but tests indicates that it * isn't needed. */ - if (!host->busy_status && - !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) && + if (!host->busy_status && !(status & err_msk) && (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) { writel(readl(base + MMCIMASK0) | @@ -1290,6 +1304,9 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, cmd->error = -ETIMEDOUT; } else if (status & MCI_CMDCRCFAIL && cmd->flags & MMC_RSP_CRC) { cmd->error = -EILSEQ; + } else if (host->variant->busy_timeout && busy_resp && + status & MCI_DATATIMEOUT) { + cmd->error = -ETIMEDOUT; } else { cmd->resp[0] = readl(base + MMCIRESPONSE0); cmd->resp[1] = readl(base + MMCIRESPONSE1); @@ -1583,6 +1600,20 @@ static void mmci_request(struct mmc_host *mmc, struct mmc_request *mrq) spin_unlock_irqrestore(&host->lock, flags); } +static void mmci_set_max_busy_timeout(struct mmc_host *mmc) +{ + struct mmci_host *host = mmc_priv(mmc); + u32 max_busy_timeout = 0; + + if (!host->variant->busy_detect) + return; + + if (host->variant->busy_timeout && mmc->actual_clock) + max_busy_timeout = ~0UL / (mmc->actual_clock / MSEC_PER_SEC); + + mmc->max_busy_timeout = max_busy_timeout; +} + static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) { struct mmci_host *host = mmc_priv(mmc); @@ -1687,6 +1718,8 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) else mmci_set_clkreg(host, ios->clock); + mmci_set_max_busy_timeout(mmc); + if (host->ops &&a
Re: [PATCH V6 3/3] mmc: mmci: sdmmc: add busy_complete callback
hi Ulf Le 10/4/19 à 9:31 AM, Ulf Hansson a écrit : On Thu, 5 Sep 2019 at 14:22, Ludovic Barre wrote: From: Ludovic Barre This patch adds a specific busy_complete callback for sdmmc variant. sdmmc has 2 status flags: -busyd0: This is a hardware status flag (inverted value of d0 line). it does not generate an interrupt. -busyd0end: This indicates only end of busy following a CMD response. On busy to Not busy changes, an interrupt is generated (if unmask) and BUSYD0END status flag is set. Status flag is cleared by writing corresponding interrupt clear bit in MMCICLEAR. The legacy busy completion monitors step by step the busy progression start/in-progress/end. On sdmmc variant, the monitoring of busy steps is difficult and not adapted (the software can miss a step and locks the monitoring), the sdmmc has just need to wait the busyd0end bit without monitoring all the changes. To me it's a bit of the opposite as you describe it above. The legacy variants suffers from a somewhat broken HW that generates also a "busystart" IRQ. For the stm32_sdmmc variant, it's more clean/correct as only a busyend IRQ is raised. Maybe you can rephrase the above a bit to make that more clear somehow. Yes, I will rephrase. The legacy busy completion has no dedicated interrupt for the end of busy, so it's must monitor step by step the busy progression. On sdmmc variant, this procedure is not needed, it's just need to wait the busyd0end status. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 3 +++ drivers/mmc/host/mmci.h | 1 + drivers/mmc/host/mmci_stm32_sdmmc.c | 38 + 3 files changed, 42 insertions(+) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index e20164f4354d..a666d826dbbd 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -260,6 +260,9 @@ static struct variant_data variant_stm32_sdmmc = { .datalength_bits= 25, .datactrl_blocksz = 14, .stm32_idmabsize_mask = GENMASK(12, 5), + .busy_timeout = true, + .busy_detect_flag = MCI_STM32_BUSYD0, + .busy_detect_mask = MCI_STM32_BUSYD0ENDMASK, .init = sdmmc_variant_init, }; diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h index 733f9a035b06..841c5281beb5 100644 --- a/drivers/mmc/host/mmci.h +++ b/drivers/mmc/host/mmci.h @@ -164,6 +164,7 @@ #define MCI_ST_CARDBUSY(1 << 24) /* Extended status bits for the STM32 variants */ #define MCI_STM32_BUSYD0 BIT(20) +#define MCI_STM32_BUSYD0ENDBIT(21) #define MMCICLEAR 0x038 #define MCI_CMDCRCFAILCLR (1 << 0) diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c index 8e83ae6920ae..bb5499cc9e81 100644 --- a/drivers/mmc/host/mmci_stm32_sdmmc.c +++ b/drivers/mmc/host/mmci_stm32_sdmmc.c @@ -282,6 +282,43 @@ static u32 sdmmc_get_dctrl_cfg(struct mmci_host *host) return datactrl; } +bool sdmmc_busy_complete(struct mmci_host *host, u32 status, u32 err_msk) +{ + void __iomem *base = host->base; + u32 busy_d0, busy_d0end, mask; + + mask = readl_relaxed(base + MMCIMASK0); + busy_d0end = readl_relaxed(base + MMCISTATUS) & MCI_STM32_BUSYD0END; + busy_d0 = readl_relaxed(base + MMCISTATUS) & MCI_STM32_BUSYD0; I have found some potential optimizations, but I leave it to you to decide what to do with my comments. *) You could avoid to read registers upfront, if that be skipped because of checking a known error condition. For example: "if (!host->busy_status && !(status & err_msk))" - would tell if it's even worth considering to unmask the busyend IRQ. Yes, it's a possibility, but I prefer to keep reading the bits busy_doend and busy_d0. This is not the most optimized way, but it is easier to understand the completion's reason (based on hardware bit). On the other hand, I would be independent of any change about status or busy_status. **) Reading MMCISTATUS twice in row seems a bit silly, why not read it once and store its value in a local variable that you operate upon instead. yes, I will store MMCISTATUS in local variable (thx). + + /* complete if there is an error or busy_d0end */ + if ((status & err_msk) || busy_d0end) + goto complete; From here, you may end up writing to MMCIMASK0 and MMCICLEAR, even if you didn't unmask the busyend IRQ in first place. I will add this condition into complete: if (host->busy_status) { ... } return true; } + + /* +* On response the busy signaling is reflected in the BUSYD0 flag. +* if busy_d0 is in-progress we must activate busyd0end interrupt +* to wait this completion. Else
Re: [PATCH V6 1/3] mmc: mmci: add hardware busy timeout feature
hi Ulf Le 10/4/19 à 8:20 AM, Ulf Hansson a écrit : On Fri, 4 Oct 2019 at 08:12, Ulf Hansson wrote: On Thu, 5 Sep 2019 at 14:21, Ludovic Barre wrote: From: Ludovic Barre In some variants, the data timer starts and decrements when the DPSM enters in Wait_R or Busy state (while data transfer or MMC_RSP_BUSY), and generates a data timeout error if the counter reach 0. -Define max_busy_timeout (in ms) according to clock. -Set data timer register if the command has rsp_busy flag. If busy_timeout is not defined by framework, the busy length after Data Burst is defined as 1 second (refer: 4.6.2.2 Write of sd specification part1 v6-0). How about re-phrasing this as below: - In the stm32_sdmmc variant, the datatimer is active not only during data transfers with the DPSM, but also while waiting for the busyend IRQs from commands having the MMC_RSP_BUSY flag set. This leads to an incorrect IRQ being raised to signal MCI_DATATIMEOUT error, which simply breaks the behaviour. Address this by updating the datatimer value before sending a command having the MMC_RSP_BUSY flag set. To inform the mmc core about the maximum supported busy timeout, which also depends on the current clock rate, set ->max_busy_timeout (in ms). Thanks for the re-phrasing. - Regarding the busy_timeout, the core should really assign it a value for all commands having the RSP_BUSY flag set. However, I realize the core needs to be improved to cover all these cases - and I am looking at that, but not there yet. I would also suggest to use a greater value than 1s, as that seems a bit low for the "undefined" case. Perhaps use the max_busy_timeout, which would be nice a simple or 10s, which I think is used by some other drivers. OK, I will set 10s, the max_busy_timeout could be very long for small frequencies (example, 25Mhz => 171s). -Add MCI_DATATIMEOUT error management in mmci_cmd_irq. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 42 - drivers/mmc/host/mmci.h | 3 +++ 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index c37e70dbe250..c30319255dc2 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -1075,6 +1075,7 @@ static void mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c) { void __iomem *base = host->base; + unsigned long long clks; dev_dbg(mmc_dev(host->mmc), "op %02x arg %08x flags %08x\n", cmd->opcode, cmd->arg, cmd->flags); @@ -1097,6 +1098,16 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c) else c |= host->variant->cmdreg_srsp; } + + if (host->variant->busy_timeout && cmd->flags & MMC_RSP_BUSY) { + if (!cmd->busy_timeout) + cmd->busy_timeout = 1000; + + clks = (unsigned long long)cmd->busy_timeout * host->cclk; + do_div(clks, MSEC_PER_SEC); + writel_relaxed(clks, host->base + MMCIDATATIMER); + } + if (/*interrupt*/0) c |= MCI_CPSM_INTERRUPT; @@ -1201,6 +1212,7 @@ static void mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, unsigned int status) { + u32 err_msk = MCI_CMDCRCFAIL | MCI_CMDTIMEOUT; void __iomem *base = host->base; bool sbc, busy_resp; @@ -1215,8 +1227,11 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, * handling. Note that we tag on any latent IRQs postponed * due to waiting for busy status. */ - if (!((status|host->busy_status) & - (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT|MCI_CMDSENT|MCI_CMDRESPEND))) + if (host->variant->busy_timeout && busy_resp) + err_msk |= MCI_DATATIMEOUT; + + if (!((status | host->busy_status) & + (err_msk | MCI_CMDSENT | MCI_CMDRESPEND))) return; /* Handle busy detection on DAT0 if the variant supports it. */ @@ -1235,8 +1250,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, * while, to allow it to be set, but tests indicates that it * isn't needed. */ - if (!host->busy_status && - !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) && + if (!host->busy_status && !(status & err_msk) && (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) { writel(readl(base + MMCIMASK0) | @@ -1290,6 +1304,9 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, cmd->error = -ETIMEDOUT; } else if (status & MCI_CMDCR
Re: [PATCH V6 0/3] mmc: mmci: add busy detect for stm32 sdmmc variant
hi Ulf Just a "gentleman ping" about this series and https://lkml.org/lkml/2019/9/4/747 Regards Ludo Le 9/5/19 à 2:21 PM, Ludovic Barre a écrit : From: Ludovic Barre This patch series adds busy detect for stm32 sdmmc variant. Some adaptations are required: -On sdmmc the data timer is started on data transfert and busy state, so we must add hardware busy timeout support. -Add busy_complete callback at mmci_host_ops to allow to define a specific busy completion by variant. -Add sdmmc busy_complete callback. V6: -mmci_start_command: set datatimer only on rsp_busy flag (remove host->mrq->data). -move max_busy_timeout in set_ios callback. -typo fix: err_msk, clks on one lines. V5: -Replaces !cmd->data to !host->mrq->data to avoid overwrite of datatimer register by the first command (cmd23, without data) of SBC request. V4: -Re-work with busy_complete callback -In series, move "mmc: mmci: add hardware busy timeout feature" in first to simplify busy_complete prototype with err_msk parameter. V3: -rebase on latest mmc next -replace re-read by status parameter. V2: -mmci_cmd_irq cleanup in separate patch. -simplify the busy_detect_flag exclude -replace sdmmc specific comment in "mmc: mmci: avoid fake busy polling in mmci_irq" to focus on common behavior Ludovic Barre (3): mmc: mmci: add hardware busy timeout feature mmc: mmci: add busy_complete callback mmc: mmci: sdmmc: add busy_complete callback drivers/mmc/host/mmci.c | 183 +--- drivers/mmc/host/mmci.h | 7 +- drivers/mmc/host/mmci_stm32_sdmmc.c | 38 ++ 3 files changed, 156 insertions(+), 72 deletions(-)
[PATCH V6 3/3] mmc: mmci: sdmmc: add busy_complete callback
From: Ludovic Barre This patch adds a specific busy_complete callback for sdmmc variant. sdmmc has 2 status flags: -busyd0: This is a hardware status flag (inverted value of d0 line). it does not generate an interrupt. -busyd0end: This indicates only end of busy following a CMD response. On busy to Not busy changes, an interrupt is generated (if unmask) and BUSYD0END status flag is set. Status flag is cleared by writing corresponding interrupt clear bit in MMCICLEAR. The legacy busy completion monitors step by step the busy progression start/in-progress/end. On sdmmc variant, the monitoring of busy steps is difficult and not adapted (the software can miss a step and locks the monitoring), the sdmmc has just need to wait the busyd0end bit without monitoring all the changes. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 3 +++ drivers/mmc/host/mmci.h | 1 + drivers/mmc/host/mmci_stm32_sdmmc.c | 38 + 3 files changed, 42 insertions(+) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index e20164f4354d..a666d826dbbd 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -260,6 +260,9 @@ static struct variant_data variant_stm32_sdmmc = { .datalength_bits= 25, .datactrl_blocksz = 14, .stm32_idmabsize_mask = GENMASK(12, 5), + .busy_timeout = true, + .busy_detect_flag = MCI_STM32_BUSYD0, + .busy_detect_mask = MCI_STM32_BUSYD0ENDMASK, .init = sdmmc_variant_init, }; diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h index 733f9a035b06..841c5281beb5 100644 --- a/drivers/mmc/host/mmci.h +++ b/drivers/mmc/host/mmci.h @@ -164,6 +164,7 @@ #define MCI_ST_CARDBUSY(1 << 24) /* Extended status bits for the STM32 variants */ #define MCI_STM32_BUSYD0 BIT(20) +#define MCI_STM32_BUSYD0ENDBIT(21) #define MMCICLEAR 0x038 #define MCI_CMDCRCFAILCLR (1 << 0) diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c index 8e83ae6920ae..bb5499cc9e81 100644 --- a/drivers/mmc/host/mmci_stm32_sdmmc.c +++ b/drivers/mmc/host/mmci_stm32_sdmmc.c @@ -282,6 +282,43 @@ static u32 sdmmc_get_dctrl_cfg(struct mmci_host *host) return datactrl; } +bool sdmmc_busy_complete(struct mmci_host *host, u32 status, u32 err_msk) +{ + void __iomem *base = host->base; + u32 busy_d0, busy_d0end, mask; + + mask = readl_relaxed(base + MMCIMASK0); + busy_d0end = readl_relaxed(base + MMCISTATUS) & MCI_STM32_BUSYD0END; + busy_d0 = readl_relaxed(base + MMCISTATUS) & MCI_STM32_BUSYD0; + + /* complete if there is an error or busy_d0end */ + if ((status & err_msk) || busy_d0end) + goto complete; + + /* +* On response the busy signaling is reflected in the BUSYD0 flag. +* if busy_d0 is in-progress we must activate busyd0end interrupt +* to wait this completion. Else this request has no busy step. +*/ + if (busy_d0) { + if (!host->busy_status) { + writel_relaxed(mask | host->variant->busy_detect_mask, + base + MMCIMASK0); + host->busy_status = status & + (MCI_CMDSENT | MCI_CMDRESPEND); + } + return false; + } + +complete: + writel_relaxed(mask & ~host->variant->busy_detect_mask, + base + MMCIMASK0); + writel_relaxed(host->variant->busy_detect_mask, base + MMCICLEAR); + host->busy_status = 0; + + return true; +} + static struct mmci_host_ops sdmmc_variant_ops = { .validate_data = sdmmc_idma_validate_data, .prep_data = sdmmc_idma_prep_data, @@ -292,6 +329,7 @@ static struct mmci_host_ops sdmmc_variant_ops = { .dma_finalize = sdmmc_idma_finalize, .set_clkreg = mmci_sdmmc_set_clkreg, .set_pwrreg = mmci_sdmmc_set_pwrreg, + .busy_complete = sdmmc_busy_complete, }; void sdmmc_variant_init(struct mmci_host *host) -- 2.17.1
[PATCH V6 2/3] mmc: mmci: add busy_complete callback
From: Ludovic Barre This patch adds busy_completion callback at mmci_host_ops to allow to define a specific busy completion by variant. The legacy code corresponding to busy completion used by ux500 variants is moved to ux500_busy_complete function. The busy_detect boolean property is replaced by busy_complete callback definition. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 142 +--- drivers/mmc/host/mmci.h | 3 +- 2 files changed, 76 insertions(+), 69 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index c30319255dc2..e20164f4354d 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -44,6 +44,7 @@ #define DRIVER_NAME "mmci-pl18x" static void mmci_variant_init(struct mmci_host *host); +static void ux500_variant_init(struct mmci_host *host); static void ux500v2_variant_init(struct mmci_host *host); static unsigned int fmax = 515633; @@ -175,7 +176,6 @@ static struct variant_data variant_ux500 = { .f_max = 1, .signal_direction = true, .pwrreg_clkgate = true, - .busy_detect= true, .busy_dpsm_flag = MCI_DPSM_ST_BUSYMODE, .busy_detect_flag = MCI_ST_CARDBUSY, .busy_detect_mask = MCI_ST_BUSYENDMASK, @@ -184,7 +184,7 @@ static struct variant_data variant_ux500 = { .irq_pio_mask = MCI_IRQ_PIO_MASK, .start_err = MCI_STARTBITERR, .opendrain = MCI_OD, - .init = mmci_variant_init, + .init = ux500_variant_init, }; static struct variant_data variant_ux500v2 = { @@ -208,7 +208,6 @@ static struct variant_data variant_ux500v2 = { .f_max = 1, .signal_direction = true, .pwrreg_clkgate = true, - .busy_detect= true, .busy_dpsm_flag = MCI_DPSM_ST_BUSYMODE, .busy_detect_flag = MCI_ST_CARDBUSY, .busy_detect_mask = MCI_ST_BUSYENDMASK, @@ -610,6 +609,67 @@ static u32 ux500v2_get_dctrl_cfg(struct mmci_host *host) return MCI_DPSM_ENABLE | (host->data->blksz << 16); } +static bool ux500_busy_complete(struct mmci_host *host, u32 status, u32 err_msk) +{ + void __iomem *base = host->base; + + /* +* Before unmasking for the busy end IRQ, confirm that the +* command was sent successfully. To keep track of having a +* command in-progress, waiting for busy signaling to end, +* store the status in host->busy_status. +* +* Note that, the card may need a couple of clock cycles before +* it starts signaling busy on DAT0, hence re-read the +* MMCISTATUS register here, to allow the busy bit to be set. +* Potentially we may even need to poll the register for a +* while, to allow it to be set, but tests indicates that it +* isn't needed. +*/ + if (!host->busy_status && !(status & err_msk) && + (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) { + writel(readl(base + MMCIMASK0) | + host->variant->busy_detect_mask, + base + MMCIMASK0); + + host->busy_status = status & (MCI_CMDSENT | MCI_CMDRESPEND); + return false; + } + + /* +* If there is a command in-progress that has been successfully +* sent, then bail out if busy status is set and wait for the +* busy end IRQ. +* +* Note that, the HW triggers an IRQ on both edges while +* monitoring DAT0 for busy completion, but there is only one +* status bit in MMCISTATUS for the busy state. Therefore +* both the start and the end interrupts needs to be cleared, +* one after the other. So, clear the busy start IRQ here. +*/ + if (host->busy_status && + (status & host->variant->busy_detect_flag)) { + writel(host->variant->busy_detect_mask, base + MMCICLEAR); + return false; + } + + /* +* If there is a command in-progress that has been successfully +* sent and the busy bit isn't set, it means we have received +* the busy end IRQ. Clear and mask the IRQ, then continue to +* process the command. +*/ + if (host->busy_status) { + writel(host->variant->busy_detect_mask, base + MMCICLEAR); + + writel(readl(base + MMCIMASK0) & + ~host->variant->busy_detect_mask, base + MMCIMASK0); + host->busy_status = 0; + } + + return true; +} + /* * All the DMA operation mode stuff goes inside this ifdef. * This assumes that you have a gen
[PATCH V6 0/3] mmc: mmci: add busy detect for stm32 sdmmc variant
From: Ludovic Barre This patch series adds busy detect for stm32 sdmmc variant. Some adaptations are required: -On sdmmc the data timer is started on data transfert and busy state, so we must add hardware busy timeout support. -Add busy_complete callback at mmci_host_ops to allow to define a specific busy completion by variant. -Add sdmmc busy_complete callback. V6: -mmci_start_command: set datatimer only on rsp_busy flag (remove host->mrq->data). -move max_busy_timeout in set_ios callback. -typo fix: err_msk, clks on one lines. V5: -Replaces !cmd->data to !host->mrq->data to avoid overwrite of datatimer register by the first command (cmd23, without data) of SBC request. V4: -Re-work with busy_complete callback -In series, move "mmc: mmci: add hardware busy timeout feature" in first to simplify busy_complete prototype with err_msk parameter. V3: -rebase on latest mmc next -replace re-read by status parameter. V2: -mmci_cmd_irq cleanup in separate patch. -simplify the busy_detect_flag exclude -replace sdmmc specific comment in "mmc: mmci: avoid fake busy polling in mmci_irq" to focus on common behavior Ludovic Barre (3): mmc: mmci: add hardware busy timeout feature mmc: mmci: add busy_complete callback mmc: mmci: sdmmc: add busy_complete callback drivers/mmc/host/mmci.c | 183 +--- drivers/mmc/host/mmci.h | 7 +- drivers/mmc/host/mmci_stm32_sdmmc.c | 38 ++ 3 files changed, 156 insertions(+), 72 deletions(-) -- 2.17.1
[PATCH V6 1/3] mmc: mmci: add hardware busy timeout feature
From: Ludovic Barre In some variants, the data timer starts and decrements when the DPSM enters in Wait_R or Busy state (while data transfer or MMC_RSP_BUSY), and generates a data timeout error if the counter reach 0. -Define max_busy_timeout (in ms) according to clock. -Set data timer register if the command has rsp_busy flag. If busy_timeout is not defined by framework, the busy length after Data Burst is defined as 1 second (refer: 4.6.2.2 Write of sd specification part1 v6-0). -Add MCI_DATATIMEOUT error management in mmci_cmd_irq. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 42 - drivers/mmc/host/mmci.h | 3 +++ 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index c37e70dbe250..c30319255dc2 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -1075,6 +1075,7 @@ static void mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c) { void __iomem *base = host->base; + unsigned long long clks; dev_dbg(mmc_dev(host->mmc), "op %02x arg %08x flags %08x\n", cmd->opcode, cmd->arg, cmd->flags); @@ -1097,6 +1098,16 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c) else c |= host->variant->cmdreg_srsp; } + + if (host->variant->busy_timeout && cmd->flags & MMC_RSP_BUSY) { + if (!cmd->busy_timeout) + cmd->busy_timeout = 1000; + + clks = (unsigned long long)cmd->busy_timeout * host->cclk; + do_div(clks, MSEC_PER_SEC); + writel_relaxed(clks, host->base + MMCIDATATIMER); + } + if (/*interrupt*/0) c |= MCI_CPSM_INTERRUPT; @@ -1201,6 +1212,7 @@ static void mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, unsigned int status) { + u32 err_msk = MCI_CMDCRCFAIL | MCI_CMDTIMEOUT; void __iomem *base = host->base; bool sbc, busy_resp; @@ -1215,8 +1227,11 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, * handling. Note that we tag on any latent IRQs postponed * due to waiting for busy status. */ - if (!((status|host->busy_status) & - (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT|MCI_CMDSENT|MCI_CMDRESPEND))) + if (host->variant->busy_timeout && busy_resp) + err_msk |= MCI_DATATIMEOUT; + + if (!((status | host->busy_status) & + (err_msk | MCI_CMDSENT | MCI_CMDRESPEND))) return; /* Handle busy detection on DAT0 if the variant supports it. */ @@ -1235,8 +1250,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, * while, to allow it to be set, but tests indicates that it * isn't needed. */ - if (!host->busy_status && - !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) && + if (!host->busy_status && !(status & err_msk) && (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) { writel(readl(base + MMCIMASK0) | @@ -1290,6 +1304,9 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, cmd->error = -ETIMEDOUT; } else if (status & MCI_CMDCRCFAIL && cmd->flags & MMC_RSP_CRC) { cmd->error = -EILSEQ; + } else if (host->variant->busy_timeout && busy_resp && + status & MCI_DATATIMEOUT) { + cmd->error = -ETIMEDOUT; } else { cmd->resp[0] = readl(base + MMCIRESPONSE0); cmd->resp[1] = readl(base + MMCIRESPONSE1); @@ -1583,6 +1600,20 @@ static void mmci_request(struct mmc_host *mmc, struct mmc_request *mrq) spin_unlock_irqrestore(&host->lock, flags); } +static void mmci_set_max_busy_timeout(struct mmc_host *mmc) +{ + struct mmci_host *host = mmc_priv(mmc); + u32 max_busy_timeout = 0; + + if (!host->variant->busy_detect) + return; + + if (host->variant->busy_timeout && mmc->actual_clock) + max_busy_timeout = ~0UL / (mmc->actual_clock / MSEC_PER_SEC); + + mmc->max_busy_timeout = max_busy_timeout; +} + static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) { struct mmci_host *host = mmc_priv(mmc); @@ -1687,6 +1718,8 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) else mmci_set_clkreg(host, ios->clock); + mmci_set_max_busy_timeout(mmc); + if (host->ops && host->ops->set_pwrreg)
Re: [PATCH V5 1/3] mmc: mmci: add hardware busy timeout feature
hi Ulf On 8/26/19 1:39 PM, Ulf Hansson wrote: On Tue, 13 Aug 2019 at 12:00, Ludovic Barre wrote: From: Ludovic Barre In some variants, the data timer starts and decrements when the DPSM enters in Wait_R or Busy state (while data transfer or MMC_RSP_BUSY), and generates a data timeout error if the counter reach 0. I don't quite follow here, sorry. Can you please try to elaborate on the use case(s) more exactly? For example, what happens when a data transfer has just finished (for example when MCI_DATAEND has been received) and we are going to send a CMD12 to stop it? In this case the CMD12 has the MMC_RSP_BUSY flag set. example with cmd25 (write multi block) mmci_request - mmci_start_data set MMCIDATATIMER, MMCIDATALENGTH, MMCIMASK0 - mmci_start_command: set MMCIARGUMENT, MMCICOMMAND (cmd25) mmci_irq: - irq MCI_CMDRESPEND - irq MCI_DATAEND - send cmd12 => mmci_start_command(host->stop_abort or data->stop) these cmds have flag rsp_busy and no data associate host->cmd = cmd (host->stop_abort or data->stop) for next irq mmci_irq: - irq MCI_CMDRESPEND - irq BUSYD0END - mmci_request_end Another example is the CMD5, which has no data with it. -Define max_busy_timeout (in ms) according to clock. -Set data timer register if the command has rsp_busy flag. If busy_timeout is not defined by framework, the busy length after Data Burst is defined as 1 second (refer: 4.6.2.2 Write of sd specification part1 v6-0). One second is not sufficient for all operations, like ERASE for example. However, I understand that you want to pick some value, as a safety. I guess that's fine. I am thinking that if the command has the MMC_RSP_BUSY flag set, the core should really provide a busy timeout for it. That said, maybe the host driver should splat a WARN in case there is not busy timeout specified. Today, I just see a busy_timeout not defined on write request. On erase request, the timeout is defined in function mmc_do_erase. In core, there are several paths to done a write request, and I not be sure to fix all. For safety, I preferred fix with the max value of write request. -Add MCI_DATATIMEOUT error management in mmci_cmd_irq. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 37 - drivers/mmc/host/mmci.h | 3 +++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index c37e70dbe250..c50586540765 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -1075,6 +1075,7 @@ static void mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c) { void __iomem *base = host->base; + unsigned long long clks = 0; dev_dbg(mmc_dev(host->mmc), "op %02x arg %08x flags %08x\n", cmd->opcode, cmd->arg, cmd->flags); @@ -1097,6 +1098,19 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c) else c |= host->variant->cmdreg_srsp; } + + if (host->variant->busy_timeout && !host->mrq->data) { Suppose this is a CMD12 command, having the MMC_RSP_BUSY flag set. The command would then be sent to stop the transmission and then host->mrq->data would also be set. Sorry, it's a mistake introduce by v5. I would keep the clear of datatimer when is not needed (no data & no rsp busy, see below). But on cmd23 (set_block_count) with datactrl_first variant property the datatimer should be protected. To simplify and fix the code, I will remove the clear of datatimer when there is no data & no rsp busy. - if (host->variant->busy_timeout && !host->mrq->data) { + if (host->variant->busy_timeout && !cmd->flags & MMC_RSP_BUSY) { + + writel_relaxed(clks, host->base + MMCIDATATIMER); + } If I recall earlier what you stated about the new sdmmc variant, the CMD12 is needed to exit the DPSM. Hence don't you need to re-program a new value for the MMCIDATATIMER register for this scenario? + if (cmd->flags & MMC_RSP_BUSY) { + if (!cmd->busy_timeout) + cmd->busy_timeout = 1000; + + clks = (unsigned long long)cmd->busy_timeout; + clks *= host->cclk; Any problems with putting the above on one line? No, it was just to not exceed 80 characters. + do_div(clks, MSEC_PER_SEC); + } + writel_relaxed(clks, host->base + MMCIDATATIMER); This is writing zero to MMCIDATATIMER in case the MMC_RSP_BUSY isn't set, is that on purpose? It was to clear the datatimer when the command has no data & no rsp_busy. This allowed to look if the datatimer was used and no
[PATCH V5 0/3] mmc: mmci: add busy detect for stm32 sdmmc variant
From: Ludovic Barre This patch series adds busy detect for stm32 sdmmc variant. Some adaptations are required: -On sdmmc the data timer is started on data transfert and busy state, so we must add hardware busy timeout support. -Add busy_complete callback at mmci_host_ops to allow to define a specific busy completion by variant. -Add sdmmc busy_complete calback. V5: -Replaces !cmd->data to !host->mrq->data to avoid overwrite of datatimer register by the first command (cmd23, without data) of SBC request. V4: -Re-work with busy_complete callback -In series, move "mmc: mmci: add hardware busy timeout feature" in first to simplify busy_complete prototype with err_msk parameter. V3: -rebase on latest mmc next -replace re-read by status parameter. V2: -mmci_cmd_irq cleanup in separate patch. -simplify the busy_detect_flag exclude -replace sdmmc specific comment in "mmc: mmci: avoid fake busy polling in mmci_irq" to focus on common behavior Ludovic Barre (3): mmc: mmci: add hardware busy timeout feature mmc: mmci: add busy_complete callback mmc: mmci: sdmmc: add busy_complete callback drivers/mmc/host/mmci.c | 178 +--- drivers/mmc/host/mmci.h | 7 +- drivers/mmc/host/mmci_stm32_sdmmc.c | 38 ++ 3 files changed, 151 insertions(+), 72 deletions(-) -- 2.17.1
[PATCH V5 2/3] mmc: mmci: add busy_complete callback
From: Ludovic Barre This patch adds busy_completion callback at mmci_host_ops to allow to define a specific busy completion by variant. The legacy code corresponding to busy completion used by ux500 variants is moved to ux500_busy_complete function. The busy_detect boolean property is replaced by busy_complete callback definition. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 140 +--- drivers/mmc/host/mmci.h | 3 +- 2 files changed, 75 insertions(+), 68 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index c50586540765..9eac3f482119 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -44,6 +44,7 @@ #define DRIVER_NAME "mmci-pl18x" static void mmci_variant_init(struct mmci_host *host); +static void ux500_variant_init(struct mmci_host *host); static void ux500v2_variant_init(struct mmci_host *host); static unsigned int fmax = 515633; @@ -175,7 +176,6 @@ static struct variant_data variant_ux500 = { .f_max = 1, .signal_direction = true, .pwrreg_clkgate = true, - .busy_detect= true, .busy_dpsm_flag = MCI_DPSM_ST_BUSYMODE, .busy_detect_flag = MCI_ST_CARDBUSY, .busy_detect_mask = MCI_ST_BUSYENDMASK, @@ -184,7 +184,7 @@ static struct variant_data variant_ux500 = { .irq_pio_mask = MCI_IRQ_PIO_MASK, .start_err = MCI_STARTBITERR, .opendrain = MCI_OD, - .init = mmci_variant_init, + .init = ux500_variant_init, }; static struct variant_data variant_ux500v2 = { @@ -208,7 +208,6 @@ static struct variant_data variant_ux500v2 = { .f_max = 1, .signal_direction = true, .pwrreg_clkgate = true, - .busy_detect= true, .busy_dpsm_flag = MCI_DPSM_ST_BUSYMODE, .busy_detect_flag = MCI_ST_CARDBUSY, .busy_detect_mask = MCI_ST_BUSYENDMASK, @@ -610,6 +609,67 @@ static u32 ux500v2_get_dctrl_cfg(struct mmci_host *host) return MCI_DPSM_ENABLE | (host->data->blksz << 16); } +static bool ux500_busy_complete(struct mmci_host *host, u32 status, u32 err_msk) +{ + void __iomem *base = host->base; + + /* +* Before unmasking for the busy end IRQ, confirm that the +* command was sent successfully. To keep track of having a +* command in-progress, waiting for busy signaling to end, +* store the status in host->busy_status. +* +* Note that, the card may need a couple of clock cycles before +* it starts signaling busy on DAT0, hence re-read the +* MMCISTATUS register here, to allow the busy bit to be set. +* Potentially we may even need to poll the register for a +* while, to allow it to be set, but tests indicates that it +* isn't needed. +*/ + if (!host->busy_status && !(status & err_msk) && + (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) { + writel(readl(base + MMCIMASK0) | + host->variant->busy_detect_mask, + base + MMCIMASK0); + + host->busy_status = status & (MCI_CMDSENT | MCI_CMDRESPEND); + return false; + } + + /* +* If there is a command in-progress that has been successfully +* sent, then bail out if busy status is set and wait for the +* busy end IRQ. +* +* Note that, the HW triggers an IRQ on both edges while +* monitoring DAT0 for busy completion, but there is only one +* status bit in MMCISTATUS for the busy state. Therefore +* both the start and the end interrupts needs to be cleared, +* one after the other. So, clear the busy start IRQ here. +*/ + if (host->busy_status && + (status & host->variant->busy_detect_flag)) { + writel(host->variant->busy_detect_mask, base + MMCICLEAR); + return false; + } + + /* +* If there is a command in-progress that has been successfully +* sent and the busy bit isn't set, it means we have received +* the busy end IRQ. Clear and mask the IRQ, then continue to +* process the command. +*/ + if (host->busy_status) { + writel(host->variant->busy_detect_mask, base + MMCICLEAR); + + writel(readl(base + MMCIMASK0) & + ~host->variant->busy_detect_mask, base + MMCIMASK0); + host->busy_status = 0; + } + + return true; +} + /* * All the DMA operation mode stuff goes inside this ifdef. * This assumes that you have a gen
[PATCH V5 1/3] mmc: mmci: add hardware busy timeout feature
From: Ludovic Barre In some variants, the data timer starts and decrements when the DPSM enters in Wait_R or Busy state (while data transfer or MMC_RSP_BUSY), and generates a data timeout error if the counter reach 0. -Define max_busy_timeout (in ms) according to clock. -Set data timer register if the command has rsp_busy flag. If busy_timeout is not defined by framework, the busy length after Data Burst is defined as 1 second (refer: 4.6.2.2 Write of sd specification part1 v6-0). -Add MCI_DATATIMEOUT error management in mmci_cmd_irq. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 37 - drivers/mmc/host/mmci.h | 3 +++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index c37e70dbe250..c50586540765 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -1075,6 +1075,7 @@ static void mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c) { void __iomem *base = host->base; + unsigned long long clks = 0; dev_dbg(mmc_dev(host->mmc), "op %02x arg %08x flags %08x\n", cmd->opcode, cmd->arg, cmd->flags); @@ -1097,6 +1098,19 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c) else c |= host->variant->cmdreg_srsp; } + + if (host->variant->busy_timeout && !host->mrq->data) { + if (cmd->flags & MMC_RSP_BUSY) { + if (!cmd->busy_timeout) + cmd->busy_timeout = 1000; + + clks = (unsigned long long)cmd->busy_timeout; + clks *= host->cclk; + do_div(clks, MSEC_PER_SEC); + } + writel_relaxed(clks, host->base + MMCIDATATIMER); + } + if (/*interrupt*/0) c |= MCI_CPSM_INTERRUPT; @@ -1203,6 +1217,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, { void __iomem *base = host->base; bool sbc, busy_resp; + u32 err_msk; if (!cmd) return; @@ -1215,8 +1230,12 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, * handling. Note that we tag on any latent IRQs postponed * due to waiting for busy status. */ - if (!((status|host->busy_status) & - (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT|MCI_CMDSENT|MCI_CMDRESPEND))) + err_msk = MCI_CMDCRCFAIL | MCI_CMDTIMEOUT; + if (host->variant->busy_timeout && busy_resp) + err_msk |= MCI_DATATIMEOUT; + + if (!((status | host->busy_status) & + (err_msk | MCI_CMDSENT | MCI_CMDRESPEND))) return; /* Handle busy detection on DAT0 if the variant supports it. */ @@ -1235,8 +1254,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, * while, to allow it to be set, but tests indicates that it * isn't needed. */ - if (!host->busy_status && - !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) && + if (!host->busy_status && !(status & err_msk) && (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) { writel(readl(base + MMCIMASK0) | @@ -1290,6 +1308,9 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, cmd->error = -ETIMEDOUT; } else if (status & MCI_CMDCRCFAIL && cmd->flags & MMC_RSP_CRC) { cmd->error = -EILSEQ; + } else if (host->variant->busy_timeout && busy_resp && + status & MCI_DATATIMEOUT) { + cmd->error = -ETIMEDOUT; } else { cmd->resp[0] = readl(base + MMCIRESPONSE0); cmd->resp[1] = readl(base + MMCIRESPONSE1); @@ -1948,6 +1969,8 @@ static int mmci_probe(struct amba_device *dev, * Enable busy detection. */ if (variant->busy_detect) { + u32 max_busy_timeout = 0; + mmci_ops.card_busy = mmci_card_busy; /* * Not all variants have a flag to enable busy detection @@ -1957,7 +1980,11 @@ static int mmci_probe(struct amba_device *dev, mmci_write_datactrlreg(host, host->variant->busy_dpsm_flag); mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY; - mmc->max_busy_timeout = 0; + + if (variant->busy_timeout) + max_busy_timeout = ~0UL / (mmc->f_max / MSEC_PER_SEC); + + mmc->max_busy_timeout = max_busy_timeout; }
[PATCH V5 3/3] mmc: mmci: sdmmc: add busy_complete callback
From: Ludovic Barre This patch adds a specific busy_complete callback for sdmmc variant. sdmmc has 2 status flags: -busyd0: This is a hardware status flag (inverted value of d0 line). it does not generate an interrupt. -busyd0end: This indicates only end of busy following a CMD response. On busy to Not busy changes, an interrupt is generated (if unmask) and BUSYD0END status flag is set. Status flag is cleared by writing corresponding interrupt clear bit in MMCICLEAR. The legacy busy completion monitors step by step the busy progression start/in-progress/end. On sdmmc variant, the monitoring of busy steps is difficult and not adapted (the software can miss a step and locks the monitoring), the sdmmc has just need to wait the busyd0end bit without monitoring all the changes. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 3 +++ drivers/mmc/host/mmci.h | 1 + drivers/mmc/host/mmci_stm32_sdmmc.c | 38 + 3 files changed, 42 insertions(+) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 9eac3f482119..9bec82d2dbf7 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -260,6 +260,9 @@ static struct variant_data variant_stm32_sdmmc = { .datalength_bits= 25, .datactrl_blocksz = 14, .stm32_idmabsize_mask = GENMASK(12, 5), + .busy_timeout = true, + .busy_detect_flag = MCI_STM32_BUSYD0, + .busy_detect_mask = MCI_STM32_BUSYD0ENDMASK, .init = sdmmc_variant_init, }; diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h index 733f9a035b06..841c5281beb5 100644 --- a/drivers/mmc/host/mmci.h +++ b/drivers/mmc/host/mmci.h @@ -164,6 +164,7 @@ #define MCI_ST_CARDBUSY(1 << 24) /* Extended status bits for the STM32 variants */ #define MCI_STM32_BUSYD0 BIT(20) +#define MCI_STM32_BUSYD0ENDBIT(21) #define MMCICLEAR 0x038 #define MCI_CMDCRCFAILCLR (1 << 0) diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c index 8e83ae6920ae..bb5499cc9e81 100644 --- a/drivers/mmc/host/mmci_stm32_sdmmc.c +++ b/drivers/mmc/host/mmci_stm32_sdmmc.c @@ -282,6 +282,43 @@ static u32 sdmmc_get_dctrl_cfg(struct mmci_host *host) return datactrl; } +bool sdmmc_busy_complete(struct mmci_host *host, u32 status, u32 err_msk) +{ + void __iomem *base = host->base; + u32 busy_d0, busy_d0end, mask; + + mask = readl_relaxed(base + MMCIMASK0); + busy_d0end = readl_relaxed(base + MMCISTATUS) & MCI_STM32_BUSYD0END; + busy_d0 = readl_relaxed(base + MMCISTATUS) & MCI_STM32_BUSYD0; + + /* complete if there is an error or busy_d0end */ + if ((status & err_msk) || busy_d0end) + goto complete; + + /* +* On response the busy signaling is reflected in the BUSYD0 flag. +* if busy_d0 is in-progress we must activate busyd0end interrupt +* to wait this completion. Else this request has no busy step. +*/ + if (busy_d0) { + if (!host->busy_status) { + writel_relaxed(mask | host->variant->busy_detect_mask, + base + MMCIMASK0); + host->busy_status = status & + (MCI_CMDSENT | MCI_CMDRESPEND); + } + return false; + } + +complete: + writel_relaxed(mask & ~host->variant->busy_detect_mask, + base + MMCIMASK0); + writel_relaxed(host->variant->busy_detect_mask, base + MMCICLEAR); + host->busy_status = 0; + + return true; +} + static struct mmci_host_ops sdmmc_variant_ops = { .validate_data = sdmmc_idma_validate_data, .prep_data = sdmmc_idma_prep_data, @@ -292,6 +329,7 @@ static struct mmci_host_ops sdmmc_variant_ops = { .dma_finalize = sdmmc_idma_finalize, .set_clkreg = mmci_sdmmc_set_clkreg, .set_pwrreg = mmci_sdmmc_set_pwrreg, + .busy_complete = sdmmc_busy_complete, }; void sdmmc_variant_init(struct mmci_host *host) -- 2.17.1
[PATCH V4 0/3] mmc: mmci: add busy detect for stm32 sdmmc variant
From: Ludovic Barre This patch series adds busy detect for stm32 sdmmc variant. Some adaptations are required: -On sdmmc the data timer is started on data transfert and busy state, so we must add hardware busy timeout support. -Add busy_complete callback at mmci_host_ops to allow to define a specific busy completion by variant. -Add sdmmc busy_complete calback. V4: -Re-work with busy_complete callback -In series, move "mmc: mmci: add hardware busy timeout feature" in first to simplify busy_complete prototype with err_msk parameter. V3: -rebase on latest mmc next -replace re-read by status parameter. V2: -mmci_cmd_irq cleanup in separate patch. -simplify the busy_detect_flag exclude -replace sdmmc specific comment in "mmc: mmci: avoid fake busy polling in mmci_irq" to focus on common behavior Ludovic Barre (3): mmc: mmci: add hardware busy timeout feature mmc: mmci: add busy_complete callback mmc: mmci: sdmmc: add busy_complete callback drivers/mmc/host/mmci.c | 178 +--- drivers/mmc/host/mmci.h | 7 +- drivers/mmc/host/mmci_stm32_sdmmc.c | 38 ++ 3 files changed, 151 insertions(+), 72 deletions(-) -- 2.17.1
[PATCH V4 2/3] mmc: mmci: add busy_complete callback
From: Ludovic Barre This patch adds busy_completion callback at mmci_host_ops to allow to define a specific busy completion by variant. The legacy code corresponding to busy completion used by ux500 variants is moved to ux500_busy_complete function. The busy_detect boolean property is replaced by busy_complete callback definition. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 140 +--- drivers/mmc/host/mmci.h | 3 +- 2 files changed, 75 insertions(+), 68 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index e79c9148af84..17948615d4a5 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -47,6 +47,7 @@ #define DRIVER_NAME "mmci-pl18x" static void mmci_variant_init(struct mmci_host *host); +static void ux500_variant_init(struct mmci_host *host); static void ux500v2_variant_init(struct mmci_host *host); static unsigned int fmax = 515633; @@ -178,7 +179,6 @@ static struct variant_data variant_ux500 = { .f_max = 1, .signal_direction = true, .pwrreg_clkgate = true, - .busy_detect= true, .busy_dpsm_flag = MCI_DPSM_ST_BUSYMODE, .busy_detect_flag = MCI_ST_CARDBUSY, .busy_detect_mask = MCI_ST_BUSYENDMASK, @@ -187,7 +187,7 @@ static struct variant_data variant_ux500 = { .irq_pio_mask = MCI_IRQ_PIO_MASK, .start_err = MCI_STARTBITERR, .opendrain = MCI_OD, - .init = mmci_variant_init, + .init = ux500_variant_init, }; static struct variant_data variant_ux500v2 = { @@ -211,7 +211,6 @@ static struct variant_data variant_ux500v2 = { .f_max = 1, .signal_direction = true, .pwrreg_clkgate = true, - .busy_detect= true, .busy_dpsm_flag = MCI_DPSM_ST_BUSYMODE, .busy_detect_flag = MCI_ST_CARDBUSY, .busy_detect_mask = MCI_ST_BUSYENDMASK, @@ -613,6 +612,67 @@ static u32 ux500v2_get_dctrl_cfg(struct mmci_host *host) return MCI_DPSM_ENABLE | (host->data->blksz << 16); } +static bool ux500_busy_complete(struct mmci_host *host, u32 status, u32 err_msk) +{ + void __iomem *base = host->base; + + /* +* Before unmasking for the busy end IRQ, confirm that the +* command was sent successfully. To keep track of having a +* command in-progress, waiting for busy signaling to end, +* store the status in host->busy_status. +* +* Note that, the card may need a couple of clock cycles before +* it starts signaling busy on DAT0, hence re-read the +* MMCISTATUS register here, to allow the busy bit to be set. +* Potentially we may even need to poll the register for a +* while, to allow it to be set, but tests indicates that it +* isn't needed. +*/ + if (!host->busy_status && !(status & err_msk) && + (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) { + writel(readl(base + MMCIMASK0) | + host->variant->busy_detect_mask, + base + MMCIMASK0); + + host->busy_status = status & (MCI_CMDSENT | MCI_CMDRESPEND); + return false; + } + + /* +* If there is a command in-progress that has been successfully +* sent, then bail out if busy status is set and wait for the +* busy end IRQ. +* +* Note that, the HW triggers an IRQ on both edges while +* monitoring DAT0 for busy completion, but there is only one +* status bit in MMCISTATUS for the busy state. Therefore +* both the start and the end interrupts needs to be cleared, +* one after the other. So, clear the busy start IRQ here. +*/ + if (host->busy_status && + (status & host->variant->busy_detect_flag)) { + writel(host->variant->busy_detect_mask, base + MMCICLEAR); + return false; + } + + /* +* If there is a command in-progress that has been successfully +* sent and the busy bit isn't set, it means we have received +* the busy end IRQ. Clear and mask the IRQ, then continue to +* process the command. +*/ + if (host->busy_status) { + writel(host->variant->busy_detect_mask, base + MMCICLEAR); + + writel(readl(base + MMCIMASK0) & + ~host->variant->busy_detect_mask, base + MMCIMASK0); + host->busy_status = 0; + } + + return true; +} + /* * All the DMA operation mode stuff goes inside this ifdef. * This assumes that you have a gen
[PATCH V4 1/3] mmc: mmci: add hardware busy timeout feature
From: Ludovic Barre In some variants, the data timer starts and decrements when the DPSM enters in Wait_R or Busy state (while data transfer or MMC_RSP_BUSY), and generates a data timeout error if the counter reach 0. -Define max_busy_timeout (in ms) according to clock. -Set data timer register if the command has rsp_busy flag. If busy_timeout is not defined by framework, the busy length after Data Burst is defined as 1 second (refer: 4.6.2.2 Write of sd specification part1 v6-0). -Add MCI_DATATIMEOUT error management in mmci_cmd_irq. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 37 - drivers/mmc/host/mmci.h | 3 +++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 4c35e7609c89..e79c9148af84 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -1078,6 +1078,7 @@ static void mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c) { void __iomem *base = host->base; + unsigned long long clks = 0; dev_dbg(mmc_dev(host->mmc), "op %02x arg %08x flags %08x\n", cmd->opcode, cmd->arg, cmd->flags); @@ -1100,6 +1101,19 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c) else c |= host->variant->cmdreg_srsp; } + + if (host->variant->busy_timeout && !cmd->data) { + if (cmd->flags & MMC_RSP_BUSY) { + if (!cmd->busy_timeout) + cmd->busy_timeout = 1000; + + clks = (unsigned long long)cmd->busy_timeout; + clks *= host->cclk; + do_div(clks, MSEC_PER_SEC); + } + writel_relaxed(clks, host->base + MMCIDATATIMER); + } + if (/*interrupt*/0) c |= MCI_CPSM_INTERRUPT; @@ -1206,6 +1220,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, { void __iomem *base = host->base; bool sbc, busy_resp; + u32 err_msk; if (!cmd) return; @@ -1218,8 +1233,12 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, * handling. Note that we tag on any latent IRQs postponed * due to waiting for busy status. */ - if (!((status|host->busy_status) & - (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT|MCI_CMDSENT|MCI_CMDRESPEND))) + err_msk = MCI_CMDCRCFAIL | MCI_CMDTIMEOUT; + if (host->variant->busy_timeout && busy_resp) + err_msk |= MCI_DATATIMEOUT; + + if (!((status | host->busy_status) & + (err_msk | MCI_CMDSENT | MCI_CMDRESPEND))) return; /* Handle busy detection on DAT0 if the variant supports it. */ @@ -1238,8 +1257,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, * while, to allow it to be set, but tests indicates that it * isn't needed. */ - if (!host->busy_status && - !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) && + if (!host->busy_status && !(status & err_msk) && (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) { writel(readl(base + MMCIMASK0) | @@ -1293,6 +1311,9 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, cmd->error = -ETIMEDOUT; } else if (status & MCI_CMDCRCFAIL && cmd->flags & MMC_RSP_CRC) { cmd->error = -EILSEQ; + } else if (host->variant->busy_timeout && busy_resp && + status & MCI_DATATIMEOUT) { + cmd->error = -ETIMEDOUT; } else { cmd->resp[0] = readl(base + MMCIRESPONSE0); cmd->resp[1] = readl(base + MMCIRESPONSE1); @@ -1951,6 +1972,8 @@ static int mmci_probe(struct amba_device *dev, * Enable busy detection. */ if (variant->busy_detect) { + u32 max_busy_timeout = 0; + mmci_ops.card_busy = mmci_card_busy; /* * Not all variants have a flag to enable busy detection @@ -1960,7 +1983,11 @@ static int mmci_probe(struct amba_device *dev, mmci_write_datactrlreg(host, host->variant->busy_dpsm_flag); mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY; - mmc->max_busy_timeout = 0; + + if (variant->busy_timeout) + max_busy_timeout = ~0UL / (mmc->f_max / MSEC_PER_SEC); + + mmc->max_busy_timeout = max_busy_timeout; }
[PATCH V4 3/3] mmc: mmci: sdmmc: add busy_complete callback
From: Ludovic Barre This patch adds a specific busy_complete callback for sdmmc variant. sdmmc has 2 status flags: -busyd0: This is a hardware status flag (inverted value of d0 line). it does not generate an interrupt. -busyd0end: This indicates only end of busy following a CMD response. On busy to Not busy changes, an interrupt is generated (if unmask) and BUSYD0END status flag is set. Status flag is cleared by writing corresponding interrupt clear bit in MMCICLEAR. The legacy busy completion monitors step by step the busy progression start/in-progress/end. On sdmmc variant, the monitoring of busy steps is difficult and not adapted (the software can miss a step and locks the monitoring), the sdmmc has just need to wait the busyd0end bit without monitoring all the changes. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 3 +++ drivers/mmc/host/mmci.h | 1 + drivers/mmc/host/mmci_stm32_sdmmc.c | 38 + 3 files changed, 42 insertions(+) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 17948615d4a5..2751415d0fd1 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -263,6 +263,9 @@ static struct variant_data variant_stm32_sdmmc = { .datalength_bits= 25, .datactrl_blocksz = 14, .stm32_idmabsize_mask = GENMASK(12, 5), + .busy_timeout = true, + .busy_detect_flag = MCI_STM32_BUSYD0, + .busy_detect_mask = MCI_STM32_BUSYD0ENDMASK, .init = sdmmc_variant_init, }; diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h index 03eb21f1c258..64ae7720477c 100644 --- a/drivers/mmc/host/mmci.h +++ b/drivers/mmc/host/mmci.h @@ -167,6 +167,7 @@ #define MCI_ST_CARDBUSY(1 << 24) /* Extended status bits for the STM32 variants */ #define MCI_STM32_BUSYD0 BIT(20) +#define MCI_STM32_BUSYD0ENDBIT(21) #define MMCICLEAR 0x038 #define MCI_CMDCRCFAILCLR (1 << 0) diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c index 8e83ae6920ae..bb5499cc9e81 100644 --- a/drivers/mmc/host/mmci_stm32_sdmmc.c +++ b/drivers/mmc/host/mmci_stm32_sdmmc.c @@ -282,6 +282,43 @@ static u32 sdmmc_get_dctrl_cfg(struct mmci_host *host) return datactrl; } +bool sdmmc_busy_complete(struct mmci_host *host, u32 status, u32 err_msk) +{ + void __iomem *base = host->base; + u32 busy_d0, busy_d0end, mask; + + mask = readl_relaxed(base + MMCIMASK0); + busy_d0end = readl_relaxed(base + MMCISTATUS) & MCI_STM32_BUSYD0END; + busy_d0 = readl_relaxed(base + MMCISTATUS) & MCI_STM32_BUSYD0; + + /* complete if there is an error or busy_d0end */ + if ((status & err_msk) || busy_d0end) + goto complete; + + /* +* On response the busy signaling is reflected in the BUSYD0 flag. +* if busy_d0 is in-progress we must activate busyd0end interrupt +* to wait this completion. Else this request has no busy step. +*/ + if (busy_d0) { + if (!host->busy_status) { + writel_relaxed(mask | host->variant->busy_detect_mask, + base + MMCIMASK0); + host->busy_status = status & + (MCI_CMDSENT | MCI_CMDRESPEND); + } + return false; + } + +complete: + writel_relaxed(mask & ~host->variant->busy_detect_mask, + base + MMCIMASK0); + writel_relaxed(host->variant->busy_detect_mask, base + MMCICLEAR); + host->busy_status = 0; + + return true; +} + static struct mmci_host_ops sdmmc_variant_ops = { .validate_data = sdmmc_idma_validate_data, .prep_data = sdmmc_idma_prep_data, @@ -292,6 +329,7 @@ static struct mmci_host_ops sdmmc_variant_ops = { .dma_finalize = sdmmc_idma_finalize, .set_clkreg = mmci_sdmmc_set_clkreg, .set_pwrreg = mmci_sdmmc_set_pwrreg, + .busy_complete = sdmmc_busy_complete, }; void sdmmc_variant_init(struct mmci_host *host) -- 2.17.1
Re: [Linux-stm32] [PATCH V3 1/3] mmc: mmci: fix read status for busy detect
hi Ulf On 7/26/19 11:41 AM, Ludovic BARRE wrote: hi Ulf Thanks to your "Clarify comments ..." commit, like is closes I resumed upstream of this series. On 7/15/19 6:31 PM, Ulf Hansson wrote: On Mon, 3 Jun 2019 at 17:55, Ludovic Barre wrote: From: Ludovic Barre "busy_detect_flag" is used to read & clear busy value of mmci status. "busy_detect_mask" is used to manage busy irq of mmci mask. So to read mmci status the busy_detect_flag must be take account. if the variant does not support busy detect feature the flag is null and there is no impact. By reading the changelog, it doesn't tell me the purpose of this change. When going forward, please work harder on your changelogs. Make sure to answer the questions, *why* is this change needed, *what/how* does the change do. Ok, I will explain the differences with the legacy and the needs of sdmmc variant about busy detection. Not need to re-read the status register in mmci_cmd_irq, the status parameter can be used. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 356833a..5b5cc45 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -1240,7 +1240,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, */ if (!host->busy_status && !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) && - (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) { + (status & host->variant->busy_detect_flag)) { I suggested you to do this change through some of my earlier comments, however I think it should be made as a stand alone change. Anyway, when looking at the details in your series, I decided to try to help out a bit, so I have prepared a couple of related patches for cleaning up and clarifying the busy detection code/comments in mmci. I have incorporated the above change, so let me post them asap. /* Clear the busy start IRQ */ writel(host->variant->busy_detect_mask, @@ -1517,7 +1517,8 @@ static irqreturn_t mmci_irq(int irq, void *dev_id) * to make sure that both start and end interrupts are always * cleared one after the other. */ - status &= readl(host->base + MMCIMASK0); + status &= readl(host->base + MMCIMASK0) | + host->variant->busy_detect_flag; As I told earlier in the review, this looks wrong to me. It means that you will add the bit for the ->busy_detect_flag to the status field we have just read from the MMCISTATUS register. That means the busy status may be set when it shouldn't. if (host->variant->busy_detect) writel(status & ~host->variant->busy_detect_mask, host->base + MMCICLEAR); -- 2.7.4 By looking at the other changes in the series, I assume @subject patch is intended to prepare for the other changes on top. But it's not really clear. Anyway, in that regards, the below is my observations of what seems to be important part, when supporting busy detection for the stm32 sdmmc variant (except the timeout things in patch2, which I intend to comment separately on). I figured, these are the involved register bits/masks: MMCISTATUS: MCI_STM32_BUSYD0 BIT(20) MCI_STM32_BUSYD0END BIT(21) MMCIMASK0: MCI_STM32_BUSYD0ENDMASK BIT(21) it's exact: MCI_STM32_BUSYD0 BIT(20): This is a hardware status flag only (inverted value of d0 line), it does not generate an interrupt, and so no mask bit. MCI_STM32_BUSYD0ENDMASK BIT(21): This indicates only end of busy following a CMD response. On busy to Not busy changes, an interrupt is generated (if unmask) and BUSYD0END status flag is set. status flag is cleared by writing corresponding interrupt clear bit in MMCICLEAR. For the legacy ST variant, there is only one register bit in MMCISTATUS that is used for indicating busy (MCI_ST_CARDBUSY BIT(24)). There is no dedicated busy-end bit for the busy-end IRQ, which I believe is the reason to why the current code also is bit messy. yes It seems like the stm32 sdmmc variant have a separate status bit for the busy-end IRQ, correct? yes If I understand correctly by looking at patch3, you don't use the dedicated busy-end status bit (MCI_STM32_BUSYD0END), right? Then why not? like your are clarify in previous series, the busy detection is done in 3 steps. if I use: .busy_detect_flag = MCI_STM32_BUSYD0ENDMASK, .busy_detect_mask = MCI_STM32_BUSYD0ENDMASK, the sdmmc request will be not correctly completed, because the third step can't be happen. chronologies: step1: when busyd0end
Re: [PATCH V3 1/3] mmc: mmci: fix read status for busy detect
hi Ulf Thanks to your "Clarify comments ..." commit, like is closes I resumed upstream of this series. On 7/15/19 6:31 PM, Ulf Hansson wrote: On Mon, 3 Jun 2019 at 17:55, Ludovic Barre wrote: From: Ludovic Barre "busy_detect_flag" is used to read & clear busy value of mmci status. "busy_detect_mask" is used to manage busy irq of mmci mask. So to read mmci status the busy_detect_flag must be take account. if the variant does not support busy detect feature the flag is null and there is no impact. By reading the changelog, it doesn't tell me the purpose of this change. When going forward, please work harder on your changelogs. Make sure to answer the questions, *why* is this change needed, *what/how* does the change do. Ok, I will explain the differences with the legacy and the needs of sdmmc variant about busy detection. Not need to re-read the status register in mmci_cmd_irq, the status parameter can be used. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 356833a..5b5cc45 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -1240,7 +1240,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, */ if (!host->busy_status && !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) && - (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) { + (status & host->variant->busy_detect_flag)) { I suggested you to do this change through some of my earlier comments, however I think it should be made as a stand alone change. Anyway, when looking at the details in your series, I decided to try to help out a bit, so I have prepared a couple of related patches for cleaning up and clarifying the busy detection code/comments in mmci. I have incorporated the above change, so let me post them asap. /* Clear the busy start IRQ */ writel(host->variant->busy_detect_mask, @@ -1517,7 +1517,8 @@ static irqreturn_t mmci_irq(int irq, void *dev_id) * to make sure that both start and end interrupts are always * cleared one after the other. */ - status &= readl(host->base + MMCIMASK0); + status &= readl(host->base + MMCIMASK0) | + host->variant->busy_detect_flag; As I told earlier in the review, this looks wrong to me. It means that you will add the bit for the ->busy_detect_flag to the status field we have just read from the MMCISTATUS register. That means the busy status may be set when it shouldn't. if (host->variant->busy_detect) writel(status & ~host->variant->busy_detect_mask, host->base + MMCICLEAR); -- 2.7.4 By looking at the other changes in the series, I assume @subject patch is intended to prepare for the other changes on top. But it's not really clear. Anyway, in that regards, the below is my observations of what seems to be important part, when supporting busy detection for the stm32 sdmmc variant (except the timeout things in patch2, which I intend to comment separately on). I figured, these are the involved register bits/masks: MMCISTATUS: MCI_STM32_BUSYD0 BIT(20) MCI_STM32_BUSYD0END BIT(21) MMCIMASK0: MCI_STM32_BUSYD0ENDMASK BIT(21) it's exact: MCI_STM32_BUSYD0 BIT(20): This is a hardware status flag only (inverted value of d0 line), it does not generate an interrupt, and so no mask bit. MCI_STM32_BUSYD0ENDMASK BIT(21): This indicates only end of busy following a CMD response. On busy to Not busy changes, an interrupt is generated (if unmask) and BUSYD0END status flag is set. status flag is cleared by writing corresponding interrupt clear bit in MMCICLEAR. For the legacy ST variant, there is only one register bit in MMCISTATUS that is used for indicating busy (MCI_ST_CARDBUSY BIT(24)). There is no dedicated busy-end bit for the busy-end IRQ, which I believe is the reason to why the current code also is bit messy. yes It seems like the stm32 sdmmc variant have a separate status bit for the busy-end IRQ, correct? yes If I understand correctly by looking at patch3, you don't use the dedicated busy-end status bit (MCI_STM32_BUSYD0END), right? Then why not? like your are clarify in previous series, the busy detection is done in 3 steps. if I use: .busy_detect_flag = MCI_STM32_BUSYD0ENDMASK, .busy_detect_mask = MCI_STM32_BUSYD0ENDMASK, the sdmmc request will be not correctly completed, because the third step can't be happen. chronologies: step1: when busyd0end change to 1 => busyd0end interrupt is unmasked =&g
RE: [PATCH V3 0/3] mmc: mmci: add busy detect for stm32 sdmmc variant
Hi Ulf like scheduled, I send you a "gentleman ping" about this series. Regards, Ludo De : Ulf Hansson Envoyé : jeudi 20 juin 2019 15:50 À : Ludovic BARRE Cc : Rob Herring; Srinivas Kandagatla; Maxime Coquelin; Alexandre TORGUE; Linux ARM; Linux Kernel Mailing List; DTML; linux-...@vger.kernel.org; linux-st...@st-md-mailman.stormreply.com Objet : Re: [PATCH V3 0/3] mmc: mmci: add busy detect for stm32 sdmmc variant Hi Ludovic, On Thu, 13 Jun 2019 at 15:13, Ulf Hansson wrote: > > On Thu, 13 Jun 2019 at 15:02, Ludovic BARRE wrote: > > > > hi Ulf > > > > Just a "gentleman ping" about this series. > > I know you are busy, it's just to be sure you do not forget me :-) > > Thanks! I started briefly to review, but got distracted again. I will > come to it, but it just seems to take more time than it should, my > apologies. Alright, so I planned to review this this week - but failed. I have been overwhelmed with work lately (as usual when vacation is getting closer). I need to gently request to come back to this as of week 28, when I will give this the highest prio. Again apologize for the delays! Kind regards Uffe > > Br > Uffe > > > > > Regards > > Ludo > > > > On 6/3/19 5:55 PM, Ludovic Barre wrote: > > > From: Ludovic Barre > > > > > > This patch series adds busy detect for stm32 sdmmc variant. > > > Some adaptations are required: > > > -Clear busy status bit if busy_detect_flag and busy_detect_mask are > > > different. > > > -Add hardware busy timeout with MMCIDATATIMER register. > > > > > > V3: > > > -rebase on latest mmc next > > > -replace re-read by status parameter. > > > > > > V2: > > > -mmci_cmd_irq cleanup in separate patch. > > > -simplify the busy_detect_flag exclude > > > -replace sdmmc specific comment in > > > "mmc: mmci: avoid fake busy polling in mmci_irq" > > > to focus on common behavior > > > > > > Ludovic Barre (3): > > >mmc: mmci: fix read status for busy detect > > >mmc: mmci: add hardware busy timeout feature > > >mmc: mmci: add busy detect for stm32 sdmmc variant > > > > > > drivers/mmc/host/mmci.c | 49 > > > + > > > drivers/mmc/host/mmci.h | 3 +++ > > > 2 files changed, 44 insertions(+), 8 deletions(-) > > >
[PATCH] ARM: dts: stm32: activate dma for qspi on stm32mp157
From: Ludovic Barre This patch activates dma for qspi on stm32mp157. Signed-off-by: Ludovic Barre --- arch/arm/boot/dts/stm32mp157c.dtsi | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi b/arch/arm/boot/dts/stm32mp157c.dtsi index 2afeee6..205ea1d 100644 --- a/arch/arm/boot/dts/stm32mp157c.dtsi +++ b/arch/arm/boot/dts/stm32mp157c.dtsi @@ -1074,6 +1074,9 @@ reg = <0x58003000 0x1000>, <0x7000 0x1000>; reg-names = "qspi", "qspi_mm"; interrupts = ; + dmas = <&mdma1 22 0x10 0x12 0x0 0x0>, + <&mdma1 22 0x10 0x18 0x0 0x0>; + dma-names = "tx", "rx"; clocks = <&rcc QSPI_K>; resets = <&rcc QSPI_R>; status = "disabled"; -- 2.7.4
[PATCH] spi: stm32-qspi: remove signal sensitive on completion
From: Ludovic Barre On umount step a sigkill signal is set (without user specific action), due to sigkill signal the completion will be interrupted and the data transfer can't be finished if a sync is needed. Signed-off-by: Ludovic Barre --- drivers/spi/spi-stm32-qspi.c | 10 +++--- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/drivers/spi/spi-stm32-qspi.c b/drivers/spi/spi-stm32-qspi.c index 5dbb6a8..655e4af 100644 --- a/drivers/spi/spi-stm32-qspi.c +++ b/drivers/spi/spi-stm32-qspi.c @@ -245,12 +245,8 @@ static int stm32_qspi_tx_dma(struct stm32_qspi *qspi, writel_relaxed(cr | CR_DMAEN, qspi->io_base + QSPI_CR); t_out = sgt.nents * STM32_COMP_TIMEOUT_MS; - if (!wait_for_completion_interruptible_timeout(&qspi->dma_completion, - msecs_to_jiffies(t_out))) - err = -ETIMEDOUT; - - if (dma_async_is_tx_complete(dma_ch, cookie, -NULL, NULL) != DMA_COMPLETE) + if (!wait_for_completion_timeout(&qspi->dma_completion, +msecs_to_jiffies(t_out))) err = -ETIMEDOUT; if (err) @@ -304,7 +300,7 @@ static int stm32_qspi_wait_cmd(struct stm32_qspi *qspi, cr = readl_relaxed(qspi->io_base + QSPI_CR); writel_relaxed(cr | CR_TCIE | CR_TEIE, qspi->io_base + QSPI_CR); - if (!wait_for_completion_interruptible_timeout(&qspi->data_completion, + if (!wait_for_completion_timeout(&qspi->data_completion, msecs_to_jiffies(STM32_COMP_TIMEOUT_MS))) { err = -ETIMEDOUT; } else { -- 2.7.4
[PATCH] dt-bindings: spi: stm32-qspi: add dma properties
From: Ludovic Barre This patch adds description of dma properties (optional). Signed-off-by: Ludovic Barre --- Documentation/devicetree/bindings/spi/spi-stm32-qspi.txt | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/spi/spi-stm32-qspi.txt b/Documentation/devicetree/bindings/spi/spi-stm32-qspi.txt index adeeb63..bfc038b 100644 --- a/Documentation/devicetree/bindings/spi/spi-stm32-qspi.txt +++ b/Documentation/devicetree/bindings/spi/spi-stm32-qspi.txt @@ -19,8 +19,11 @@ Required properties: - reg: chip-Select number (QSPI controller may connect 2 flashes) - spi-max-frequency: max frequency of spi bus -Optional property: +Optional properties: - spi-rx-bus-width: see ./spi-bus.txt for the description +- dmas: DMA specifiers for tx and rx dma. See the DMA client binding, +Documentation/devicetree/bindings/dma/dma.txt. +- dma-names: DMA request names should include "tx" and "rx" if present. Example: -- 2.7.4
Re: [PATCH V3 0/3] mmc: mmci: add busy detect for stm32 sdmmc variant
hi Ulf Just a "gentleman ping" about this series. I know you are busy, it's just to be sure you do not forget me :-) Regards Ludo On 6/3/19 5:55 PM, Ludovic Barre wrote: From: Ludovic Barre This patch series adds busy detect for stm32 sdmmc variant. Some adaptations are required: -Clear busy status bit if busy_detect_flag and busy_detect_mask are different. -Add hardware busy timeout with MMCIDATATIMER register. V3: -rebase on latest mmc next -replace re-read by status parameter. V2: -mmci_cmd_irq cleanup in separate patch. -simplify the busy_detect_flag exclude -replace sdmmc specific comment in "mmc: mmci: avoid fake busy polling in mmci_irq" to focus on common behavior Ludovic Barre (3): mmc: mmci: fix read status for busy detect mmc: mmci: add hardware busy timeout feature mmc: mmci: add busy detect for stm32 sdmmc variant drivers/mmc/host/mmci.c | 49 + drivers/mmc/host/mmci.h | 3 +++ 2 files changed, 44 insertions(+), 8 deletions(-)
[PATCH V3 0/3] mmc: mmci: add busy detect for stm32 sdmmc variant
From: Ludovic Barre This patch series adds busy detect for stm32 sdmmc variant. Some adaptations are required: -Clear busy status bit if busy_detect_flag and busy_detect_mask are different. -Add hardware busy timeout with MMCIDATATIMER register. V3: -rebase on latest mmc next -replace re-read by status parameter. V2: -mmci_cmd_irq cleanup in separate patch. -simplify the busy_detect_flag exclude -replace sdmmc specific comment in "mmc: mmci: avoid fake busy polling in mmci_irq" to focus on common behavior Ludovic Barre (3): mmc: mmci: fix read status for busy detect mmc: mmci: add hardware busy timeout feature mmc: mmci: add busy detect for stm32 sdmmc variant drivers/mmc/host/mmci.c | 49 + drivers/mmc/host/mmci.h | 3 +++ 2 files changed, 44 insertions(+), 8 deletions(-) -- 2.7.4
[PATCH V3 1/3] mmc: mmci: fix read status for busy detect
From: Ludovic Barre "busy_detect_flag" is used to read & clear busy value of mmci status. "busy_detect_mask" is used to manage busy irq of mmci mask. So to read mmci status the busy_detect_flag must be take account. if the variant does not support busy detect feature the flag is null and there is no impact. Not need to re-read the status register in mmci_cmd_irq, the status parameter can be used. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 356833a..5b5cc45 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -1240,7 +1240,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, */ if (!host->busy_status && !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) && - (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) { + (status & host->variant->busy_detect_flag)) { /* Clear the busy start IRQ */ writel(host->variant->busy_detect_mask, @@ -1517,7 +1517,8 @@ static irqreturn_t mmci_irq(int irq, void *dev_id) * to make sure that both start and end interrupts are always * cleared one after the other. */ - status &= readl(host->base + MMCIMASK0); + status &= readl(host->base + MMCIMASK0) | + host->variant->busy_detect_flag; if (host->variant->busy_detect) writel(status & ~host->variant->busy_detect_mask, host->base + MMCICLEAR); -- 2.7.4
[PATCH V3 2/3] mmc: mmci: add hardware busy timeout feature
From: Ludovic Barre In some variants, the data timer is enabled when the DPSM is in busy state (while data transfer or MMC_RSP_BUSY), and could generate a data timeout error if the counter reach 0. -Define max_busy_timeout (in ms) according to clock. -Set data timer register if the command has rsp_busy flag. If busy_timeout is not defined by framework, the busy length after Data Burst is defined as 1 second (refer: 4.6.2.2 Write of sd specification part1 v6-0). -Add MCI_DATATIMEOUT error management in mmci_cmd_irq. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 40 ++-- drivers/mmc/host/mmci.h | 2 ++ 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 5b5cc45..5a8b232 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -1078,6 +1078,7 @@ static void mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c) { void __iomem *base = host->base; + unsigned long long clks = 0; dev_dbg(mmc_dev(host->mmc), "op %02x arg %08x flags %08x\n", cmd->opcode, cmd->arg, cmd->flags); @@ -1100,6 +1101,19 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c) else c |= host->variant->cmdreg_srsp; } + + if (host->variant->busy_timeout && !cmd->data) { + if (cmd->flags & MMC_RSP_BUSY) { + if (!cmd->busy_timeout) + cmd->busy_timeout = 1000; + + clks = (unsigned long long)cmd->busy_timeout; + clks *= host->cclk; + do_div(clks, MSEC_PER_SEC); + } + writel_relaxed(clks, host->base + MMCIDATATIMER); + } + if (/*interrupt*/0) c |= MCI_CPSM_INTERRUPT; @@ -1206,6 +1220,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, { void __iomem *base = host->base; bool sbc, busy_resp; + u32 err_msk; if (!cmd) return; @@ -1218,8 +1233,12 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, * handling. Note that we tag on any latent IRQs postponed * due to waiting for busy status. */ - if (!((status|host->busy_status) & - (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT|MCI_CMDSENT|MCI_CMDRESPEND))) + err_msk = MCI_CMDCRCFAIL | MCI_CMDTIMEOUT; + if (host->variant->busy_timeout && busy_resp) + err_msk |= MCI_DATATIMEOUT; + + if (!((status | host->busy_status) & + (err_msk | MCI_CMDSENT | MCI_CMDRESPEND))) return; /* @@ -1228,7 +1247,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, if (busy_resp && host->variant->busy_detect) { /* We are busy with a command, return */ - if (host->busy_status && + if (host->busy_status && !(status & (err_msk)) && (status & host->variant->busy_detect_flag)) return; @@ -1238,8 +1257,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, * that the special busy status bit is still set before * proceeding. */ - if (!host->busy_status && - !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) && + if (!host->busy_status && !(status & (err_msk)) && (status & host->variant->busy_detect_flag)) { /* Clear the busy start IRQ */ @@ -1282,6 +1300,9 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, cmd->error = -ETIMEDOUT; } else if (status & MCI_CMDCRCFAIL && cmd->flags & MMC_RSP_CRC) { cmd->error = -EILSEQ; + } else if (host->variant->busy_timeout && busy_resp && + status & MCI_DATATIMEOUT) { + cmd->error = -ETIMEDOUT; } else { cmd->resp[0] = readl(base + MMCIRESPONSE0); cmd->resp[1] = readl(base + MMCIRESPONSE1); @@ -1543,6 +1564,7 @@ static irqreturn_t mmci_irq(int irq, void *dev_id) status &= ~host->variant->busy_detect_flag; ret = 1; + } while (status); spin_unlock(&host->lock); @@ -1947,6 +1969,8 @@ static int mmci_probe(struct amba_device *dev, * Enable busy detection. */ if (variant->busy_detect) { + u32 max_busy_timeout = 0; + mmci_ops.card_busy = mmci_card_busy; /*
[PATCH V3 3/3] mmc: mmci: add busy detect for stm32 sdmmc variant
From: Ludovic Barre This patch enables busy detection for stm32 sdmmc which requires to set data timer to define the busy timeout. sdmmc has 2 flags: -busyd0: inverted value of d0 line. -busyd0end which indicates only end of busy following a cmd response. Only one interrupt on busyd0end. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 4 drivers/mmc/host/mmci.h | 1 + 2 files changed, 5 insertions(+) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 5a8b232..6210f1f 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -264,6 +264,10 @@ static struct variant_data variant_stm32_sdmmc = { .datalength_bits= 25, .datactrl_blocksz = 14, .stm32_idmabsize_mask = GENMASK(12, 5), + .busy_detect= true, + .busy_timeout = true, + .busy_detect_flag = MCI_STM32_BUSYD0, + .busy_detect_mask = MCI_STM32_BUSYD0ENDMASK, .init = sdmmc_variant_init, }; diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h index b43a958..ac19de8 100644 --- a/drivers/mmc/host/mmci.h +++ b/drivers/mmc/host/mmci.h @@ -167,6 +167,7 @@ #define MCI_ST_CARDBUSY(1 << 24) /* Extended status bits for the STM32 variants */ #define MCI_STM32_BUSYD0 BIT(20) +#define MCI_STM32_BUSYD0ENDBIT(21) #define MMCICLEAR 0x038 #define MCI_CMDCRCFAILCLR (1 << 0) -- 2.7.4
Re: [PATCH V2 3/5] mmc: mmci: fix clear of busy detect status
hi Ulf On 5/27/19 8:17 PM, Ulf Hansson wrote: On Fri, 26 Apr 2019 at 09:46, Ludovic Barre wrote: From: Ludovic Barre The "busy_detect_flag" is used to read/clear busy value of mmci status. The "busy_detect_mask" is used to manage busy irq of mmci mask. For sdmmc variant, the 2 properties have not the same offset. To clear the busyd0 status bit, we must add busy detect flag, the mmci mask is not enough. Signed-off-by: Ludovic Barre Ludovic, again, apologies for the delay. --- drivers/mmc/host/mmci.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index a040f54..3cd52e8 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -1517,7 +1517,8 @@ static irqreturn_t mmci_irq(int irq, void *dev_id) * to make sure that both start and end interrupts are always * cleared one after the other. */ - status &= readl(host->base + MMCIMASK0); + status &= readl(host->base + MMCIMASK0) | + host->variant->busy_detect_flag; I think this is not entirely correct, because it would mean we check for busy even if we haven't unmasked the busy IRQ via the variant->busy_detect_mask. if the variant is busy_detect false: => no problem because the busy_detect_flag or busy_detect_mask is not defined. if variant is busy_detect true: the busy handle is split in 3 steps (see mmci_cmd_irq): step 1: detection of busy line => unmasked the busy irq end step 2: in busy wait => ignore cmd irq while current busy flag is enabled. step 3: end of busy => clear and mask busy irq To detect the first step (see mmci_cmd_irq: which unmasks the busy irq) we need to know the current busy state. Actually, the status register is re-read in mmci_cmd_irq, why not used the status read in mmci_irq and in parameter ? Regards, Ludo I suggest to store a new bool in the host (call it "busy_detect_unmasked" or whatever makes sense to you), to track whether we have unmasked the busy IRQ or not. Then take this flag into account, before ORing the value of host->variant->busy_detect_flag, according to above. if (host->variant->busy_detect) writel(status & ~host->variant->busy_detect_mask, host->base + MMCICLEAR); -- 2.7.4 Kind regards Uffe
Re: [PATCH V2 0/5] mmc: mmci: add busy detect for stm32 sdmmc variant
On 5/21/19 9:56 AM, Ulf Hansson wrote: On Tue, 21 May 2019 at 09:38, Ludovic BARRE wrote: hi Ulf Just a "gentleman ping" about the rest of series. "mmc: mmci: add busy detect for stm32 sdmmc variant" Thanks! It's been a busy period and I am currently traveling. My plan is to look at in detail beginning of next week when get back home. I hope that's okay. yes, I understand, it's just to not forget me :-) Kind regards Uffe Regards Ludo On 5/3/19 3:29 PM, Ulf Hansson wrote: On Tue, 30 Apr 2019 at 14:06, Ludovic BARRE wrote: On 4/30/19 1:13 PM, Ulf Hansson wrote: On Fri, 26 Apr 2019 at 09:46, Ludovic Barre wrote: From: Ludovic Barre This patch series adds busy detect for stm32 sdmmc variant. Some adaptations are required: -Avoid to check and poll busy status when is not expected. -Clear busy status bit if busy_detect_flag and busy_detect_mask are different. -Add hardware busy timeout with MMCIDATATIMER register. V2: -mmci_cmd_irq cleanup in separate patch. -simplify the busy_detect_flag exclude -replace sdmmc specific comment in "mmc: mmci: avoid fake busy polling in mmci_irq" to focus on common behavior Ludovic Barre (5): mmc: mmci: cleanup mmci_cmd_irq for busy detect feature mmc: mmci: avoid fake busy polling in mmci_irq mmc: mmci: fix clear of busy detect status mmc: mmci: add hardware busy timeout feature mmc: mmci: add busy detect for stm32 sdmmc variant drivers/mmc/host/mmci.c | 61 ++--- drivers/mmc/host/mmci.h | 3 +++ 2 files changed, 51 insertions(+), 13 deletions(-) -- 2.7.4 Ludovic, just wanted to let you know that I am reviewing and testing this series. However, while running some tests on Ux500 for validating the busy detection code, even without your series applied, I encounter some odd behaviors. I am looking into the problem to understand better and will let you know as soon as I have some more data to share. Oops, don't hesitate to share your status, if I could help. Thanks! Good and bad news here, then. I now understand what is going on - and there is certainly room for improvements here, but more importantly the actual mmci busy detection works as expected. When it comes to improvements, the main issue I have found is how we treat DATA WRITES. In many cases we simply don't use the HW busy detection at all, but instead rely on the mmc core to send CMD13 in a loop to poll. Well, then if the polling would have consisted of a couple of CMD13s that wouldn't be an issue, but my observations is rather that the numbers of CMD13 sent to poll is in the range or hundreds/thousands - per each WRITE request! I am going to send a patch (or two) that improves the behavior. It might even involve changing parts in core layer, not sure how the end result will look like yet. In any case, I have applied patch 1 and patch2 for next, as the tests turned out well at my side. I also took the liberty of updating some of the comments/changelogs, please have look and tell if there is something you want to change. I will continue with the rest of series next week. Kind regards Uffe
Re: [PATCH V2 0/5] mmc: mmci: add busy detect for stm32 sdmmc variant
hi Ulf Just a "gentleman ping" about the rest of series. "mmc: mmci: add busy detect for stm32 sdmmc variant" Regards Ludo On 5/3/19 3:29 PM, Ulf Hansson wrote: On Tue, 30 Apr 2019 at 14:06, Ludovic BARRE wrote: On 4/30/19 1:13 PM, Ulf Hansson wrote: On Fri, 26 Apr 2019 at 09:46, Ludovic Barre wrote: From: Ludovic Barre This patch series adds busy detect for stm32 sdmmc variant. Some adaptations are required: -Avoid to check and poll busy status when is not expected. -Clear busy status bit if busy_detect_flag and busy_detect_mask are different. -Add hardware busy timeout with MMCIDATATIMER register. V2: -mmci_cmd_irq cleanup in separate patch. -simplify the busy_detect_flag exclude -replace sdmmc specific comment in "mmc: mmci: avoid fake busy polling in mmci_irq" to focus on common behavior Ludovic Barre (5): mmc: mmci: cleanup mmci_cmd_irq for busy detect feature mmc: mmci: avoid fake busy polling in mmci_irq mmc: mmci: fix clear of busy detect status mmc: mmci: add hardware busy timeout feature mmc: mmci: add busy detect for stm32 sdmmc variant drivers/mmc/host/mmci.c | 61 ++--- drivers/mmc/host/mmci.h | 3 +++ 2 files changed, 51 insertions(+), 13 deletions(-) -- 2.7.4 Ludovic, just wanted to let you know that I am reviewing and testing this series. However, while running some tests on Ux500 for validating the busy detection code, even without your series applied, I encounter some odd behaviors. I am looking into the problem to understand better and will let you know as soon as I have some more data to share. Oops, don't hesitate to share your status, if I could help. Thanks! Good and bad news here, then. I now understand what is going on - and there is certainly room for improvements here, but more importantly the actual mmci busy detection works as expected. When it comes to improvements, the main issue I have found is how we treat DATA WRITES. In many cases we simply don't use the HW busy detection at all, but instead rely on the mmc core to send CMD13 in a loop to poll. Well, then if the polling would have consisted of a couple of CMD13s that wouldn't be an issue, but my observations is rather that the numbers of CMD13 sent to poll is in the range or hundreds/thousands - per each WRITE request! I am going to send a patch (or two) that improves the behavior. It might even involve changing parts in core layer, not sure how the end result will look like yet. In any case, I have applied patch 1 and patch2 for next, as the tests turned out well at my side. I also took the liberty of updating some of the comments/changelogs, please have look and tell if there is something you want to change. I will continue with the rest of series next week. Kind regards Uffe
[PATCH] mtd: spi-nor: stm32: remove the driver as it was replaced by spi-stm32-qspi.c
From: Ludovic Barre There's a new driver using the SPI memory interface of the SPI framework at spi/spi-stm32-qspi.c, which can be used together with m25p80.c to replace the functionality of this SPI NOR driver. The "new" driver uses the same dt properties and not affects the legacy compatibility. Signed-off-by: Ludovic Barre --- .../devicetree/bindings/mtd/stm32-quadspi.txt | 43 -- drivers/mtd/spi-nor/Kconfig| 7 - drivers/mtd/spi-nor/Makefile | 1 - drivers/mtd/spi-nor/stm32-quadspi.c| 720 - 4 files changed, 771 deletions(-) delete mode 100644 Documentation/devicetree/bindings/mtd/stm32-quadspi.txt delete mode 100644 drivers/mtd/spi-nor/stm32-quadspi.c diff --git a/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt b/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt deleted file mode 100644 index ddd18c1..000 --- a/Documentation/devicetree/bindings/mtd/stm32-quadspi.txt +++ /dev/null @@ -1,43 +0,0 @@ -* STMicroelectronics Quad Serial Peripheral Interface(QuadSPI) - -Required properties: -- compatible: should be "st,stm32f469-qspi" -- reg: the first contains the register location and length. - the second contains the memory mapping address and length -- reg-names: should contain the reg names "qspi" "qspi_mm" -- interrupts: should contain the interrupt for the device -- clocks: the phandle of the clock needed by the QSPI controller -- A pinctrl must be defined to set pins in mode of operation for QSPI transfer - -Optional properties: -- resets: must contain the phandle to the reset controller. - -A spi flash must be a child of the nor_flash node and could have some -properties. Also see jedec,spi-nor.txt. - -Required properties: -- reg: chip-Select number (QSPI controller may connect 2 nor flashes) -- spi-max-frequency: max frequency of spi bus - -Optional property: -- spi-rx-bus-width: see ../spi/spi-bus.txt for the description - -Example: - -qspi: spi@a0001000 { - compatible = "st,stm32f469-qspi"; - reg = <0xa0001000 0x1000>, <0x9000 0x1000>; - reg-names = "qspi", "qspi_mm"; - interrupts = <91>; - resets = <&rcc STM32F4_AHB3_RESET(QSPI)>; - clocks = <&rcc 0 STM32F4_AHB3_CLOCK(QSPI)>; - pinctrl-names = "default"; - pinctrl-0 = <&pinctrl_qspi0>; - - flash@0 { - reg = <0>; - spi-rx-bus-width = <4>; - spi-max-frequency = <10800>; - ... - }; -}; diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig index dab9866..54b5da0 100644 --- a/drivers/mtd/spi-nor/Kconfig +++ b/drivers/mtd/spi-nor/Kconfig @@ -104,11 +104,4 @@ config SPI_INTEL_SPI_PLATFORM To compile this driver as a module, choose M here: the module will be called intel-spi-platform. -config SPI_STM32_QUADSPI - tristate "STM32 Quad SPI controller" - depends on ARCH_STM32 || COMPILE_TEST - help - This enables support for the STM32 Quad SPI controller. - We only connect the NOR to this controller. - endif # MTD_SPI_NOR diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile index 189a15c..9c5ed03 100644 --- a/drivers/mtd/spi-nor/Makefile +++ b/drivers/mtd/spi-nor/Makefile @@ -8,4 +8,3 @@ obj-$(CONFIG_SPI_NXP_SPIFI) += nxp-spifi.o obj-$(CONFIG_SPI_INTEL_SPI)+= intel-spi.o obj-$(CONFIG_SPI_INTEL_SPI_PCI)+= intel-spi-pci.o obj-$(CONFIG_SPI_INTEL_SPI_PLATFORM) += intel-spi-platform.o -obj-$(CONFIG_SPI_STM32_QUADSPI)+= stm32-quadspi.o diff --git a/drivers/mtd/spi-nor/stm32-quadspi.c b/drivers/mtd/spi-nor/stm32-quadspi.c deleted file mode 100644 index 13e9fc9..000 --- a/drivers/mtd/spi-nor/stm32-quadspi.c +++ /dev/null @@ -1,720 +0,0 @@ -/* - * Driver for stm32 quadspi controller - * - * Copyright (C) 2017, STMicroelectronics - All Rights Reserved - * Author(s): Ludovic Barre author . - * - * License terms: GPL V2.0. - * - * This program is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License version 2 as published by - * the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more - * details. - * - * You should have received a copy of the GNU General Public License along with - * This program. If not, see <http://www.gnu.org/licenses/>. - */ -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#define QUADSPI_CR 0x00 -#define CR_EN
Re: [PATCH V2 0/5] mmc: mmci: add busy detect for stm32 sdmmc variant
hi Ulf On 5/3/19 3:29 PM, Ulf Hansson wrote: On Tue, 30 Apr 2019 at 14:06, Ludovic BARRE wrote: On 4/30/19 1:13 PM, Ulf Hansson wrote: On Fri, 26 Apr 2019 at 09:46, Ludovic Barre wrote: From: Ludovic Barre This patch series adds busy detect for stm32 sdmmc variant. Some adaptations are required: -Avoid to check and poll busy status when is not expected. -Clear busy status bit if busy_detect_flag and busy_detect_mask are different. -Add hardware busy timeout with MMCIDATATIMER register. V2: -mmci_cmd_irq cleanup in separate patch. -simplify the busy_detect_flag exclude -replace sdmmc specific comment in "mmc: mmci: avoid fake busy polling in mmci_irq" to focus on common behavior Ludovic Barre (5): mmc: mmci: cleanup mmci_cmd_irq for busy detect feature mmc: mmci: avoid fake busy polling in mmci_irq mmc: mmci: fix clear of busy detect status mmc: mmci: add hardware busy timeout feature mmc: mmci: add busy detect for stm32 sdmmc variant drivers/mmc/host/mmci.c | 61 ++--- drivers/mmc/host/mmci.h | 3 +++ 2 files changed, 51 insertions(+), 13 deletions(-) -- 2.7.4 Ludovic, just wanted to let you know that I am reviewing and testing this series. However, while running some tests on Ux500 for validating the busy detection code, even without your series applied, I encounter some odd behaviors. I am looking into the problem to understand better and will let you know as soon as I have some more data to share. Oops, don't hesitate to share your status, if I could help. Thanks! Good and bad news here, then. I now understand what is going on - and there is certainly room for improvements here, but more importantly the actual mmci busy detection works as expected. When it comes to improvements, the main issue I have found is how we treat DATA WRITES. In many cases we simply don't use the HW busy detection at all, but instead rely on the mmc core to send CMD13 in a loop to poll. Well, then if the polling would have consisted of a couple of CMD13s that wouldn't be an issue, but my observations is rather that the numbers of CMD13 sent to poll is in the range or hundreds/thousands - per each WRITE request! I am going to send a patch (or two) that improves the behavior. It might even involve changing parts in core layer, not sure how the end result will look like yet. yes, these will improve the drivers without hardware busy completion. great In any case, I have applied patch 1 and patch2 for next, as the tests turned out well at my side. I also took the liberty of updating some of the comments/changelogs, please have look and tell if there is something you want to change. I will continue with the rest of series next week. thanks, and good week-end. Kind regards Uffe
[PATCH V3] watchdog: stm32: add dynamic prescaler support
From: Ludovic Barre This patch allows to define the max prescaler by compatible. To set a large range of timeout, the prescaler should be set dynamically (from the timeout request) to improve the resolution in order to have a timeout close to the expected value. Signed-off-by: Ludovic Barre --- drivers/watchdog/stm32_iwdg.c | 84 --- 1 file changed, 47 insertions(+), 37 deletions(-) diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c index 6f7c2bc..d569a36 100644 --- a/drivers/watchdog/stm32_iwdg.c +++ b/drivers/watchdog/stm32_iwdg.c @@ -34,36 +34,44 @@ #define KR_KEY_EWA 0x /* write access enable */ #define KR_KEY_DWA 0x /* write access disable */ -/* IWDG_PR register bit values */ -#define PR_4 0x00 /* prescaler set to 4 */ -#define PR_8 0x01 /* prescaler set to 8 */ -#define PR_16 0x02 /* prescaler set to 16 */ -#define PR_32 0x03 /* prescaler set to 32 */ -#define PR_64 0x04 /* prescaler set to 64 */ -#define PR_128 0x05 /* prescaler set to 128 */ -#define PR_256 0x06 /* prescaler set to 256 */ +/* IWDG_PR register */ +#define PR_SHIFT 2 +#define PR_MIN BIT(PR_SHIFT) /* IWDG_RLR register values */ -#define RLR_MIN0x07C /* min value supported by reload register */ -#define RLR_MAX0xFFF /* max value supported by reload register */ +#define RLR_MIN0x2 /* min value recommended */ +#define RLR_MAXGENMASK(11, 0) /* max value of reload register */ /* IWDG_SR register bit mask */ -#define FLAG_PVU BIT(0) /* Watchdog prescaler value update */ -#define FLAG_RVU BIT(1) /* Watchdog counter reload value update */ +#define SR_PVU BIT(0) /* Watchdog prescaler value update */ +#define SR_RVU BIT(1) /* Watchdog counter reload value update */ /* set timeout to 10 us */ #define TIMEOUT_US 10 #define SLEEP_US 1000 -#define HAS_PCLK true +struct stm32_iwdg_data { + bool has_pclk; + u32 max_prescaler; +}; + +static const struct stm32_iwdg_data stm32_iwdg_data = { + .has_pclk = false, + .max_prescaler = 256, +}; + +static const struct stm32_iwdg_data stm32mp1_iwdg_data = { + .has_pclk = true, + .max_prescaler = 1024, +}; struct stm32_iwdg { struct watchdog_device wdd; + const struct stm32_iwdg_data *data; void __iomem*regs; struct clk *clk_lsi; struct clk *clk_pclk; unsigned intrate; - boolhas_pclk; }; static inline u32 reg_read(void __iomem *base, u32 reg) @@ -79,31 +87,35 @@ static inline void reg_write(void __iomem *base, u32 reg, u32 val) static int stm32_iwdg_start(struct watchdog_device *wdd) { struct stm32_iwdg *wdt = watchdog_get_drvdata(wdd); - u32 val = FLAG_PVU | FLAG_RVU; - u32 reload; + u32 tout, presc, iwdg_rlr, iwdg_pr, iwdg_sr; int ret; dev_dbg(wdd->parent, "%s\n", __func__); - /* prescaler fixed to 256 */ - reload = clamp_t(unsigned int, ((wdd->timeout * wdt->rate) / 256) - 1, -RLR_MIN, RLR_MAX); + tout = clamp_t(unsigned int, wdd->timeout, + wdd->min_timeout, wdd->max_hw_heartbeat_ms / 1000); + + presc = DIV_ROUND_UP(tout * wdt->rate, RLR_MAX + 1); + + /* The prescaler is align on power of 2 and start at 2 ^ PR_SHIFT. */ + presc = roundup_pow_of_two(presc); + iwdg_pr = presc <= 1 << PR_SHIFT ? 0 : ilog2(presc) - PR_SHIFT; + iwdg_rlr = ((tout * wdt->rate) / presc) - 1; /* enable write access */ reg_write(wdt->regs, IWDG_KR, KR_KEY_EWA); /* set prescaler & reload registers */ - reg_write(wdt->regs, IWDG_PR, PR_256); /* prescaler fix to 256 */ - reg_write(wdt->regs, IWDG_RLR, reload); + reg_write(wdt->regs, IWDG_PR, iwdg_pr); + reg_write(wdt->regs, IWDG_RLR, iwdg_rlr); reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE); /* wait for the registers to be updated (max 100ms) */ - ret = readl_relaxed_poll_timeout(wdt->regs + IWDG_SR, val, -!(val & (FLAG_PVU | FLAG_RVU)), + ret = readl_relaxed_poll_timeout(wdt->regs + IWDG_SR, iwdg_sr, +!(iwdg_sr & (SR_PVU | SR_RVU)), SLEEP_US, TIMEOUT_US); if (ret) { - dev_err(wdd->parent, - "Fail to set prescaler or reload registers\n"); + dev_err(wdd->parent, "Fail to set prescaler, reload regs\n"); return ret; } @@ -156,7 +168,7 @@ static int stm32_iwdg_clk_init(struct platform_device *pdev,
Re: [PATCH V2 1/3] watchdog: stm32: update to devm_watchdog_register_device
hi Guenter On 5/2/19 10:21 PM, Guenter Roeck wrote: On Thu, May 02, 2019 at 04:08:44PM +0200, Ludovic Barre wrote: From: Ludovic Barre This patch updates to devm_watchdog_register_device interface Not that easy. See below. A more complete solution is at https://patchwork.kernel.org/patch/10894355 I have a total of three patches for this driver pending for the next kernel release. Maybe it would make sense to (re-) start this series from there after the next commit window closes. I used the repository defined in MAINTAINERS file git://www.linux-watchdog.org/linux-watchdog.git but there is no next branch. Today, I see your kernel.org repository https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git/ And I see your next branch, so I will use it. Regards, Ludo Guenter Signed-off-by: Ludovic Barre --- drivers/watchdog/stm32_iwdg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c index e00e3b3..e191bd8 100644 --- a/drivers/watchdog/stm32_iwdg.c +++ b/drivers/watchdog/stm32_iwdg.c @@ -243,7 +243,7 @@ static int stm32_iwdg_probe(struct platform_device *pdev) dev_warn(&pdev->dev, "unable to set timeout value, using default\n"); - ret = watchdog_register_device(wdd); + ret = devm_watchdog_register_device(&pdev->dev, wdd); if (ret) { dev_err(&pdev->dev, "failed to register watchdog device\n"); goto err; @@ -263,7 +263,6 @@ static int stm32_iwdg_remove(struct platform_device *pdev) { struct stm32_iwdg *wdt = platform_get_drvdata(pdev); - watchdog_unregister_device(&wdt->wdd); clk_disable_unprepare(wdt->clk_lsi); clk_disable_unprepare(wdt->clk_pclk); This disables the clock while the watchdog is still registered and running. That is not a good idea. -- 2.7.4
[PATCH V2 2/3] watchdog: stm32: update return values recommended by watchdog kernel api
From: Ludovic Barre This patch updates return values on watchdog-kernel-api.txt: return 0 on succes, -EINVAL for "parameter out of range" and -EIO for "could not write value to the watchdog". Signed-off-by: Ludovic Barre --- drivers/watchdog/stm32_iwdg.c | 21 + 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c index e191bd8..f19a6d4 100644 --- a/drivers/watchdog/stm32_iwdg.c +++ b/drivers/watchdog/stm32_iwdg.c @@ -81,7 +81,6 @@ static int stm32_iwdg_start(struct watchdog_device *wdd) struct stm32_iwdg *wdt = watchdog_get_drvdata(wdd); u32 val = FLAG_PVU | FLAG_RVU; u32 reload; - int ret; dev_dbg(wdd->parent, "%s\n", __func__); @@ -98,13 +97,11 @@ static int stm32_iwdg_start(struct watchdog_device *wdd) reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE); /* wait for the registers to be updated (max 100ms) */ - ret = readl_relaxed_poll_timeout(wdt->regs + IWDG_SR, val, -!(val & (FLAG_PVU | FLAG_RVU)), -SLEEP_US, TIMEOUT_US); - if (ret) { - dev_err(wdd->parent, - "Fail to set prescaler or reload registers\n"); - return ret; + if (readl_relaxed_poll_timeout(wdt->regs + IWDG_SR, val, + !(val & (FLAG_PVU | FLAG_RVU)), + SLEEP_US, TIMEOUT_US)) { + dev_err(wdd->parent, "Fail to set prescaler, reload regs\n"); + return -EIO; } /* reload watchdog */ @@ -128,8 +125,16 @@ static int stm32_iwdg_ping(struct watchdog_device *wdd) static int stm32_iwdg_set_timeout(struct watchdog_device *wdd, unsigned int timeout) { + unsigned int tout = clamp(timeout, wdd->min_timeout, + wdd->max_hw_heartbeat_ms / 1000); + dev_dbg(wdd->parent, "%s timeout: %d sec\n", __func__, timeout); + if (tout != timeout) { + dev_err(wdd->parent, "parameter out of range\n"); + return -EINVAL; + } + wdd->timeout = timeout; if (watchdog_active(wdd)) -- 2.7.4
[PATCH V2 0/3] watchdog: stm32: add dynamic prescaler support
From: Ludovic Barre This patch series updates stm32 watchdog driver on: -use devm_watchdog_register_device -Guenter's recommendation about return value: set_timeout return 0 on succes, -EINVAL for "parameter out of range" and -EIO for "could not write value to the watchdog". Set of reload and prescaler registers are stay in start function, because the stm32 watchdog must be enabled to write these registers. -adds dynamic prescaler support Ludovic Barre (3): watchdog: stm32: update to devm_watchdog_register_device watchdog: stm32: update return values recommended by watchdog kernel api watchdog: stm32: add dynamic prescaler support drivers/watchdog/stm32_iwdg.c | 96 --- 1 file changed, 54 insertions(+), 42 deletions(-) -- 2.7.4
[PATCH V2 3/3] watchdog: stm32: add dynamic prescaler support
From: Ludovic Barre This patch allows to define the max prescaler by compatible. To set a large range of timeout, the prescaler should be set dynamically (from the timeout request) to improve the resolution in order to have a timeout close to the expected value. Signed-off-by: Ludovic Barre --- drivers/watchdog/stm32_iwdg.c | 76 --- 1 file changed, 42 insertions(+), 34 deletions(-) diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c index f19a6d4..0c765d4 100644 --- a/drivers/watchdog/stm32_iwdg.c +++ b/drivers/watchdog/stm32_iwdg.c @@ -34,18 +34,12 @@ #define KR_KEY_EWA 0x /* write access enable */ #define KR_KEY_DWA 0x /* write access disable */ -/* IWDG_PR register bit values */ -#define PR_4 0x00 /* prescaler set to 4 */ -#define PR_8 0x01 /* prescaler set to 8 */ -#define PR_16 0x02 /* prescaler set to 16 */ -#define PR_32 0x03 /* prescaler set to 32 */ -#define PR_64 0x04 /* prescaler set to 64 */ -#define PR_128 0x05 /* prescaler set to 128 */ -#define PR_256 0x06 /* prescaler set to 256 */ +#define PR_SHIFT 2 +#define PR_MIN BIT(PR_SHIFT) /* IWDG_RLR register values */ -#define RLR_MIN0x07C /* min value supported by reload register */ -#define RLR_MAX0xFFF /* max value supported by reload register */ +#define RLR_MIN0x2 /* min value recommended */ +#define RLR_MAXGENMASK(11, 0) /* max value of reload register */ /* IWDG_SR register bit mask */ #define FLAG_PVU BIT(0) /* Watchdog prescaler value update */ @@ -55,15 +49,28 @@ #define TIMEOUT_US 10 #define SLEEP_US 1000 -#define HAS_PCLK true +struct stm32_iwdg_data { + bool has_pclk; + u32 max_prescaler; +}; + +static const struct stm32_iwdg_data stm32_iwdg_data = { + .has_pclk = false, + .max_prescaler = 256, +}; + +static const struct stm32_iwdg_data stm32mp1_iwdg_data = { + .has_pclk = true, + .max_prescaler = 1024, +}; struct stm32_iwdg { struct watchdog_device wdd; + const struct stm32_iwdg_data *data; void __iomem*regs; struct clk *clk_lsi; struct clk *clk_pclk; unsigned intrate; - boolhas_pclk; }; static inline u32 reg_read(void __iomem *base, u32 reg) @@ -79,26 +86,28 @@ static inline void reg_write(void __iomem *base, u32 reg, u32 val) static int stm32_iwdg_start(struct watchdog_device *wdd) { struct stm32_iwdg *wdt = watchdog_get_drvdata(wdd); - u32 val = FLAG_PVU | FLAG_RVU; - u32 reload; + u32 presc, iwdg_rlr, iwdg_pr, iwdg_sr; dev_dbg(wdd->parent, "%s\n", __func__); - /* prescaler fixed to 256 */ - reload = clamp_t(unsigned int, ((wdd->timeout * wdt->rate) / 256) - 1, -RLR_MIN, RLR_MAX); + presc = DIV_ROUND_UP(wdd->timeout * wdt->rate, RLR_MAX + 1); + + /* The prescaler is align on power of 2 and start at 2 ^ PR_SHIFT. */ + presc = roundup_pow_of_two(presc); + iwdg_pr = presc <= 1 << PR_SHIFT ? 0 : ilog2(presc) - PR_SHIFT; + iwdg_rlr = ((wdd->timeout * wdt->rate) / presc) - 1; + /* enable watchdog */ + reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE); /* enable write access */ reg_write(wdt->regs, IWDG_KR, KR_KEY_EWA); - /* set prescaler & reload registers */ - reg_write(wdt->regs, IWDG_PR, PR_256); /* prescaler fix to 256 */ - reg_write(wdt->regs, IWDG_RLR, reload); - reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE); + reg_write(wdt->regs, IWDG_PR, iwdg_pr); + reg_write(wdt->regs, IWDG_RLR, iwdg_rlr); /* wait for the registers to be updated (max 100ms) */ - if (readl_relaxed_poll_timeout(wdt->regs + IWDG_SR, val, - !(val & (FLAG_PVU | FLAG_RVU)), + if (readl_relaxed_poll_timeout(wdt->regs + IWDG_SR, iwdg_sr, + !(iwdg_sr & (FLAG_PVU | FLAG_RVU)), SLEEP_US, TIMEOUT_US)) { dev_err(wdd->parent, "Fail to set prescaler, reload regs\n"); return -EIO; @@ -155,7 +164,7 @@ static int stm32_iwdg_clk_init(struct platform_device *pdev, } /* optional peripheral clock */ - if (wdt->has_pclk) { + if (wdt->data->has_pclk) { wdt->clk_pclk = devm_clk_get(&pdev->dev, "pclk"); if (IS_ERR(wdt->clk_pclk)) { dev_err(&pdev->dev, "Unable to get pclk clock\n"); @@ -196,8 +205,8 @@ static const struct watchdog_ops stm32_iwdg_ops = { }; static const struct of_device
[PATCH V2 1/3] watchdog: stm32: update to devm_watchdog_register_device
From: Ludovic Barre This patch updates to devm_watchdog_register_device interface Signed-off-by: Ludovic Barre --- drivers/watchdog/stm32_iwdg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c index e00e3b3..e191bd8 100644 --- a/drivers/watchdog/stm32_iwdg.c +++ b/drivers/watchdog/stm32_iwdg.c @@ -243,7 +243,7 @@ static int stm32_iwdg_probe(struct platform_device *pdev) dev_warn(&pdev->dev, "unable to set timeout value, using default\n"); - ret = watchdog_register_device(wdd); + ret = devm_watchdog_register_device(&pdev->dev, wdd); if (ret) { dev_err(&pdev->dev, "failed to register watchdog device\n"); goto err; @@ -263,7 +263,6 @@ static int stm32_iwdg_remove(struct platform_device *pdev) { struct stm32_iwdg *wdt = platform_get_drvdata(pdev); - watchdog_unregister_device(&wdt->wdd); clk_disable_unprepare(wdt->clk_lsi); clk_disable_unprepare(wdt->clk_pclk); -- 2.7.4
Re: [PATCH V2 0/5] mmc: mmci: add busy detect for stm32 sdmmc variant
On 4/30/19 1:13 PM, Ulf Hansson wrote: On Fri, 26 Apr 2019 at 09:46, Ludovic Barre wrote: From: Ludovic Barre This patch series adds busy detect for stm32 sdmmc variant. Some adaptations are required: -Avoid to check and poll busy status when is not expected. -Clear busy status bit if busy_detect_flag and busy_detect_mask are different. -Add hardware busy timeout with MMCIDATATIMER register. V2: -mmci_cmd_irq cleanup in separate patch. -simplify the busy_detect_flag exclude -replace sdmmc specific comment in "mmc: mmci: avoid fake busy polling in mmci_irq" to focus on common behavior Ludovic Barre (5): mmc: mmci: cleanup mmci_cmd_irq for busy detect feature mmc: mmci: avoid fake busy polling in mmci_irq mmc: mmci: fix clear of busy detect status mmc: mmci: add hardware busy timeout feature mmc: mmci: add busy detect for stm32 sdmmc variant drivers/mmc/host/mmci.c | 61 ++--- drivers/mmc/host/mmci.h | 3 +++ 2 files changed, 51 insertions(+), 13 deletions(-) -- 2.7.4 Ludovic, just wanted to let you know that I am reviewing and testing this series. However, while running some tests on Ux500 for validating the busy detection code, even without your series applied, I encounter some odd behaviors. I am looking into the problem to understand better and will let you know as soon as I have some more data to share. Oops, don't hesitate to share your status, if I could help. Kind regards Uffe
Re: [PATCH] watchdog: stm32: add dynamic prescaler support
Hi Guenter On 4/26/19 4:16 PM, Guenter Roeck wrote: On Fri, Apr 26, 2019 at 03:41:15PM +0200, Ludovic Barre wrote: From: Ludovic Barre This patch allows to define the max prescaler by compatible. To set a large range of timeout, the prescaler should be set dynamically (from the timeout request) to improve the resolution in order to have a timeout close to the expected value. Signed-off-by: Ludovic Barre --- drivers/watchdog/stm32_iwdg.c | 70 +-- 1 file changed, 41 insertions(+), 29 deletions(-) diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c index e00e3b3..91d0a89 100644 --- a/drivers/watchdog/stm32_iwdg.c +++ b/drivers/watchdog/stm32_iwdg.c @@ -34,18 +34,10 @@ #define KR_KEY_EWA0x /* write access enable */ #define KR_KEY_DWA0x /* write access disable */ -/* IWDG_PR register bit values */ -#define PR_4 0x00 /* prescaler set to 4 */ -#define PR_8 0x01 /* prescaler set to 8 */ -#define PR_16 0x02 /* prescaler set to 16 */ -#define PR_32 0x03 /* prescaler set to 32 */ -#define PR_64 0x04 /* prescaler set to 64 */ -#define PR_128 0x05 /* prescaler set to 128 */ -#define PR_256 0x06 /* prescaler set to 256 */ +#define PR_SHIFT 2 /* IWDG_RLR register values */ -#define RLR_MIN0x07C /* min value supported by reload register */ -#define RLR_MAX0xFFF /* max value supported by reload register */ +#define RLR_MAXGENMASK(11, 0) /* max value of reload register */ /* IWDG_SR register bit mask */ #define FLAG_PVU BIT(0) /* Watchdog prescaler value update */ @@ -55,15 +47,28 @@ #define TIMEOUT_US10 #define SLEEP_US 1000 -#define HAS_PCLK true +struct stm32_iwdg_data { + bool has_pclk; + u32 max_prescaler; +}; + +static const struct stm32_iwdg_data stm32_iwdg_data = { + .has_pclk = false, + .max_prescaler = 256, +}; + +static const struct stm32_iwdg_data stm32mp1_iwdg_data = { + .has_pclk = true, + .max_prescaler = 1024, +}; struct stm32_iwdg { struct watchdog_device wdd; + const struct stm32_iwdg_data *data; void __iomem*regs; struct clk *clk_lsi; struct clk *clk_pclk; unsigned intrate; - boolhas_pclk; }; static inline u32 reg_read(void __iomem *base, u32 reg) @@ -80,21 +85,30 @@ static int stm32_iwdg_start(struct watchdog_device *wdd) { struct stm32_iwdg *wdt = watchdog_get_drvdata(wdd); u32 val = FLAG_PVU | FLAG_RVU; - u32 reload; + u32 timeout, presc, iwdg_rlr, iwdg_pr; int ret; dev_dbg(wdd->parent, "%s\n", __func__); - /* prescaler fixed to 256 */ - reload = clamp_t(unsigned int, ((wdd->timeout * wdt->rate) / 256) - 1, -RLR_MIN, RLR_MAX); + timeout = clamp_t(unsigned int, wdd->timeout, + wdd->min_timeout, wdd->max_hw_heartbeat_ms / 1000); + + if (timeout != wdd->timeout) + dev_warn(wdd->parent, "timeout skrinked to %d\n", timeout); + Valid values for timeout should be set in the set_timeout function, not here. As such, there is no need for this warning. More specifically, if the selected timeout is between min_timeout and max_hw_heartbeat_ms, and the hardware can not meet the exact requested value, the set_timeout function should adjust wdd->timeout value accordingly. Ok, so I will adjust the timeout (with prescaler and reload look-up) value in set_timeout function. thanks Example: The requested timeout is 55 seconds, but the hardware can only support either 32 or 64 seconds. The set_timeout function should then set wdd->timeout to either 32 or 64. Furthermore, this is a valid condition. For example, the timeout could be set for one hour (or day), and the maximum heartbeat could be one minute. In that situation, the watchdog core handles heartbeat/ping requests. Again, this does not warrant a warning. On top of all that, if the hardware can not set a minimum timeout of 1 second, min_timeout should not be set to 1 second. It should be set to the actual minimum supportable value if that value is larger than 1 second. + presc = DIV_ROUND_UP(timeout * wdt->rate, RLR_MAX + 1); + + /* The prescaler is align on power of 2 and start at 2 ^ PR_SHIFT. */ + presc = roundup_pow_of_two(presc); + iwdg_pr = presc <= 1 << PR_SHIFT ? 0 : ilog2(presc) - PR_SHIFT; + iwdg_rlr = ((timeout * wdt->rate) / presc) - 1; /* enable write access */ reg_write(wdt->regs, IWDG_KR, KR_KEY_EWA); /* set prescaler & reload registers */ - reg_write(wdt->regs, IWDG_PR, PR_256); /* prescaler fix to 256 */ - reg_write(wdt->regs, IWDG_RLR, reload
[PATCH] watchdog: stm32: add dynamic prescaler support
From: Ludovic Barre This patch allows to define the max prescaler by compatible. To set a large range of timeout, the prescaler should be set dynamically (from the timeout request) to improve the resolution in order to have a timeout close to the expected value. Signed-off-by: Ludovic Barre --- drivers/watchdog/stm32_iwdg.c | 70 +-- 1 file changed, 41 insertions(+), 29 deletions(-) diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c index e00e3b3..91d0a89 100644 --- a/drivers/watchdog/stm32_iwdg.c +++ b/drivers/watchdog/stm32_iwdg.c @@ -34,18 +34,10 @@ #define KR_KEY_EWA 0x /* write access enable */ #define KR_KEY_DWA 0x /* write access disable */ -/* IWDG_PR register bit values */ -#define PR_4 0x00 /* prescaler set to 4 */ -#define PR_8 0x01 /* prescaler set to 8 */ -#define PR_16 0x02 /* prescaler set to 16 */ -#define PR_32 0x03 /* prescaler set to 32 */ -#define PR_64 0x04 /* prescaler set to 64 */ -#define PR_128 0x05 /* prescaler set to 128 */ -#define PR_256 0x06 /* prescaler set to 256 */ +#define PR_SHIFT 2 /* IWDG_RLR register values */ -#define RLR_MIN0x07C /* min value supported by reload register */ -#define RLR_MAX0xFFF /* max value supported by reload register */ +#define RLR_MAXGENMASK(11, 0) /* max value of reload register */ /* IWDG_SR register bit mask */ #define FLAG_PVU BIT(0) /* Watchdog prescaler value update */ @@ -55,15 +47,28 @@ #define TIMEOUT_US 10 #define SLEEP_US 1000 -#define HAS_PCLK true +struct stm32_iwdg_data { + bool has_pclk; + u32 max_prescaler; +}; + +static const struct stm32_iwdg_data stm32_iwdg_data = { + .has_pclk = false, + .max_prescaler = 256, +}; + +static const struct stm32_iwdg_data stm32mp1_iwdg_data = { + .has_pclk = true, + .max_prescaler = 1024, +}; struct stm32_iwdg { struct watchdog_device wdd; + const struct stm32_iwdg_data *data; void __iomem*regs; struct clk *clk_lsi; struct clk *clk_pclk; unsigned intrate; - boolhas_pclk; }; static inline u32 reg_read(void __iomem *base, u32 reg) @@ -80,21 +85,30 @@ static int stm32_iwdg_start(struct watchdog_device *wdd) { struct stm32_iwdg *wdt = watchdog_get_drvdata(wdd); u32 val = FLAG_PVU | FLAG_RVU; - u32 reload; + u32 timeout, presc, iwdg_rlr, iwdg_pr; int ret; dev_dbg(wdd->parent, "%s\n", __func__); - /* prescaler fixed to 256 */ - reload = clamp_t(unsigned int, ((wdd->timeout * wdt->rate) / 256) - 1, -RLR_MIN, RLR_MAX); + timeout = clamp_t(unsigned int, wdd->timeout, + wdd->min_timeout, wdd->max_hw_heartbeat_ms / 1000); + + if (timeout != wdd->timeout) + dev_warn(wdd->parent, "timeout skrinked to %d\n", timeout); + + presc = DIV_ROUND_UP(timeout * wdt->rate, RLR_MAX + 1); + + /* The prescaler is align on power of 2 and start at 2 ^ PR_SHIFT. */ + presc = roundup_pow_of_two(presc); + iwdg_pr = presc <= 1 << PR_SHIFT ? 0 : ilog2(presc) - PR_SHIFT; + iwdg_rlr = ((timeout * wdt->rate) / presc) - 1; /* enable write access */ reg_write(wdt->regs, IWDG_KR, KR_KEY_EWA); /* set prescaler & reload registers */ - reg_write(wdt->regs, IWDG_PR, PR_256); /* prescaler fix to 256 */ - reg_write(wdt->regs, IWDG_RLR, reload); + reg_write(wdt->regs, IWDG_PR, iwdg_pr); + reg_write(wdt->regs, IWDG_RLR, iwdg_rlr); reg_write(wdt->regs, IWDG_KR, KR_KEY_ENABLE); /* wait for the registers to be updated (max 100ms) */ @@ -150,7 +164,7 @@ static int stm32_iwdg_clk_init(struct platform_device *pdev, } /* optional peripheral clock */ - if (wdt->has_pclk) { + if (wdt->data->has_pclk) { wdt->clk_pclk = devm_clk_get(&pdev->dev, "pclk"); if (IS_ERR(wdt->clk_pclk)) { dev_err(&pdev->dev, "Unable to get pclk clock\n"); @@ -191,8 +205,8 @@ static const struct watchdog_ops stm32_iwdg_ops = { }; static const struct of_device_id stm32_iwdg_of_match[] = { - { .compatible = "st,stm32-iwdg", .data = (void *)!HAS_PCLK }, - { .compatible = "st,stm32mp1-iwdg", .data = (void *)HAS_PCLK }, + { .compatible = "st,stm32-iwdg", .data = &stm32_iwdg_data }, + { .compatible = "st,stm32mp1-iwdg", .data = &stm32mp1_iwdg_data }, { /* end node */ } }; MODULE_DEVICE_TABLE(of, stm32_iwdg_of_match); @@ -200,20 +214,17 @@ MODULE_
[PATCH V2 3/5] mmc: mmci: fix clear of busy detect status
From: Ludovic Barre The "busy_detect_flag" is used to read/clear busy value of mmci status. The "busy_detect_mask" is used to manage busy irq of mmci mask. For sdmmc variant, the 2 properties have not the same offset. To clear the busyd0 status bit, we must add busy detect flag, the mmci mask is not enough. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index a040f54..3cd52e8 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -1517,7 +1517,8 @@ static irqreturn_t mmci_irq(int irq, void *dev_id) * to make sure that both start and end interrupts are always * cleared one after the other. */ - status &= readl(host->base + MMCIMASK0); + status &= readl(host->base + MMCIMASK0) | + host->variant->busy_detect_flag; if (host->variant->busy_detect) writel(status & ~host->variant->busy_detect_mask, host->base + MMCICLEAR); -- 2.7.4
[PATCH V2 1/5] mmc: mmci: cleanup mmci_cmd_irq for busy detect feature
From: Ludovic Barre This patch cleans mmci_cmd_irq function for busy detect feature. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 9e9596a..049f8e3 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -1205,12 +1205,13 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, unsigned int status) { void __iomem *base = host->base; - bool sbc; + bool sbc, busy_resp; if (!cmd) return; sbc = (cmd == host->mrq->sbc); + busy_resp = !!(cmd->flags & MMC_RSP_BUSY); /* * We need to be one of these interrupts to be considered worth @@ -1224,8 +1225,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, /* * ST Micro variant: handle busy detection. */ - if (host->variant->busy_detect) { - bool busy_resp = !!(cmd->flags & MMC_RSP_BUSY); + if (busy_resp && host->variant->busy_detect) { /* We are busy with a command, return */ if (host->busy_status && @@ -1238,7 +1238,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, * that the special busy status bit is still set before * proceeding. */ - if (!host->busy_status && busy_resp && + if (!host->busy_status && !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) && (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) { -- 2.7.4
[PATCH V2 5/5] mmc: mmci: add busy detect for stm32 sdmmc variant
From: Ludovic Barre This patch enables busy detection for stm32 sdmmc which requires to set data timer to define the busy timeout. sdmmc has 2 flags: -busyd0: inverted value of d0 line. -busyd0end which indicates only end of busy following a cmd response. Only one interrupt on busyd0end. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 4 drivers/mmc/host/mmci.h | 1 + 2 files changed, 5 insertions(+) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 8dcb980..02a3cdd 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -264,6 +264,10 @@ static struct variant_data variant_stm32_sdmmc = { .datalength_bits= 25, .datactrl_blocksz = 14, .stm32_idmabsize_mask = GENMASK(12, 5), + .busy_detect= true, + .busy_timeout = true, + .busy_detect_flag = MCI_STM32_BUSYD0, + .busy_detect_mask = MCI_STM32_BUSYD0ENDMASK, .init = sdmmc_variant_init, }; diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h index b43a958..ac19de8 100644 --- a/drivers/mmc/host/mmci.h +++ b/drivers/mmc/host/mmci.h @@ -167,6 +167,7 @@ #define MCI_ST_CARDBUSY(1 << 24) /* Extended status bits for the STM32 variants */ #define MCI_STM32_BUSYD0 BIT(20) +#define MCI_STM32_BUSYD0ENDBIT(21) #define MMCICLEAR 0x038 #define MCI_CMDCRCFAILCLR (1 << 0) -- 2.7.4
[PATCH V2 2/5] mmc: mmci: avoid fake busy polling in mmci_irq
From: Ludovic Barre mmci_irq function loops until the status is totally cleared. However the busy_detect_flag could occurred even if no busy response is expected and that busy d0 line is low (like in cmd11: voltage switch). Like busy_detect_flag is handled into mmci_cmd_irq this flag can be always excluded. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 049f8e3..a040f54 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -1535,9 +1535,10 @@ static irqreturn_t mmci_irq(int irq, void *dev_id) } /* -* Don't poll for busy completion in irq context. +* Busy_detect_flag has been handled by mmci_cmd_irq, +* it can be excluded to avoid to poll on it */ - if (host->variant->busy_detect && host->busy_status) + if (host->variant->busy_detect_flag) status &= ~host->variant->busy_detect_flag; ret = 1; -- 2.7.4
[PATCH V2 0/5] mmc: mmci: add busy detect for stm32 sdmmc variant
From: Ludovic Barre This patch series adds busy detect for stm32 sdmmc variant. Some adaptations are required: -Avoid to check and poll busy status when is not expected. -Clear busy status bit if busy_detect_flag and busy_detect_mask are different. -Add hardware busy timeout with MMCIDATATIMER register. V2: -mmci_cmd_irq cleanup in separate patch. -simplify the busy_detect_flag exclude -replace sdmmc specific comment in "mmc: mmci: avoid fake busy polling in mmci_irq" to focus on common behavior Ludovic Barre (5): mmc: mmci: cleanup mmci_cmd_irq for busy detect feature mmc: mmci: avoid fake busy polling in mmci_irq mmc: mmci: fix clear of busy detect status mmc: mmci: add hardware busy timeout feature mmc: mmci: add busy detect for stm32 sdmmc variant drivers/mmc/host/mmci.c | 61 ++--- drivers/mmc/host/mmci.h | 3 +++ 2 files changed, 51 insertions(+), 13 deletions(-) -- 2.7.4
[PATCH V2 4/5] mmc: mmci: add hardware busy timeout feature
From: Ludovic Barre In some variants, the data timer is enabled when the DPSM is in busy state (while data transfert or MMC_RSP_BUSY), and could generate a data timeout error if the counter reach 0. -Define max_busy_timeout (in ms) according to clock. -Set data timer register if the command has rsp_busy flag. If busy_timeout is not defined by framework, the busy length after Data Burst is defined as 1 second (refer: 4.6.2.2 Write of sd specification part1 v6-0). -Add MCI_DATATIMEOUT error management in mmci_cmd_irq. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 43 --- drivers/mmc/host/mmci.h | 2 ++ 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 3cd52e8..8dcb980 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -1078,6 +1078,7 @@ static void mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c) { void __iomem *base = host->base; + unsigned long long clks = 0; dev_dbg(mmc_dev(host->mmc), "op %02x arg %08x flags %08x\n", cmd->opcode, cmd->arg, cmd->flags); @@ -1100,6 +1101,19 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c) else c |= host->variant->cmdreg_srsp; } + + if (host->variant->busy_timeout && !cmd->data) { + if (cmd->flags & MMC_RSP_BUSY) { + if (!cmd->busy_timeout) + cmd->busy_timeout = 1000; + + clks = (unsigned long long)cmd->busy_timeout; + clks *= host->cclk; + do_div(clks, MSEC_PER_SEC); + } + writel_relaxed(clks, host->base + MMCIDATATIMER); + } + if (/*interrupt*/0) c |= MCI_CPSM_INTERRUPT; @@ -1206,6 +1220,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, { void __iomem *base = host->base; bool sbc, busy_resp; + u32 err_msk; if (!cmd) return; @@ -1218,8 +1233,12 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, * handling. Note that we tag on any latent IRQs postponed * due to waiting for busy status. */ - if (!((status|host->busy_status) & - (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT|MCI_CMDSENT|MCI_CMDRESPEND))) + err_msk = MCI_CMDCRCFAIL | MCI_CMDTIMEOUT; + if (host->variant->busy_timeout && busy_resp) + err_msk |= MCI_DATATIMEOUT; + + if (!((status | host->busy_status) & + (err_msk | MCI_CMDSENT | MCI_CMDRESPEND))) return; /* @@ -1228,7 +1247,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, if (busy_resp && host->variant->busy_detect) { /* We are busy with a command, return */ - if (host->busy_status && + if (host->busy_status && !(status & (err_msk)) && (status & host->variant->busy_detect_flag)) return; @@ -1238,9 +1257,9 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, * that the special busy status bit is still set before * proceeding. */ - if (!host->busy_status && - !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) && - (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) { + if (!host->busy_status && !(status & (err_msk)) && + (readl(base + MMCISTATUS) & +host->variant->busy_detect_flag)) { /* Clear the busy start IRQ */ writel(host->variant->busy_detect_mask, @@ -1282,6 +1301,9 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, cmd->error = -ETIMEDOUT; } else if (status & MCI_CMDCRCFAIL && cmd->flags & MMC_RSP_CRC) { cmd->error = -EILSEQ; + } else if (host->variant->busy_timeout && busy_resp && + status & MCI_DATATIMEOUT) { + cmd->error = -ETIMEDOUT; } else { cmd->resp[0] = readl(base + MMCIRESPONSE0); cmd->resp[1] = readl(base + MMCIRESPONSE1); @@ -1543,6 +1565,7 @@ static irqreturn_t mmci_irq(int irq, void *dev_id) status &= ~host->variant->busy_detect_flag; ret = 1; + } while (status); spin_unlock(&host->lock); @@ -1947,6 +1970,8 @@ static int mmci_probe(struct amba_device *d
Re: [PATCH 1/4] mmc: mmci: avoid fake busy polling
On 4/25/19 12:08 PM, Ulf Hansson wrote: On Thu, 25 Apr 2019 at 11:22, Ludovic BARRE wrote: hi Ulf On 4/23/19 3:39 PM, Ulf Hansson wrote: On Tue, 5 Mar 2019 at 17:10, Ludovic Barre wrote: From: Ludovic Barre The busy status bit could occurred even if no busy response is expected (example cmd11). On sdmmc variant, the busy_detect_flag reflects inverted value of d0 state, it's sampled at the end of a CMD response and a second time 2 clk cycles after the CMD response. To avoid a fake busy, the busy status could be checked and polled only if the command has RSP_BUSY flag. I would appreciate a better explanation of what this patch really changes. The above is giving some background to the behavior of sdmmc variant, but at this point that variant doesn't even have the ->variant->busy_detect flag set. Yes, I will try to explain more and focus on common behavior. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 19 +-- 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 387ff14..4901b73 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -1220,12 +1220,13 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, unsigned int status) { void __iomem *base = host->base; - bool sbc; + bool sbc, busy_resp; if (!cmd) return; sbc = (cmd == host->mrq->sbc); + busy_resp = !!(cmd->flags & MMC_RSP_BUSY); /* * We need to be one of these interrupts to be considered worth @@ -1239,8 +1240,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, /* * ST Micro variant: handle busy detection. */ - if (host->variant->busy_detect) { - bool busy_resp = !!(cmd->flags & MMC_RSP_BUSY); + if (busy_resp && host->variant->busy_detect) { /* We are busy with a command, return */ if (host->busy_status && @@ -1253,7 +1253,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, * that the special busy status bit is still set before * proceeding. */ - if (!host->busy_status && busy_resp && + if (!host->busy_status && !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) && (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) { All the changes above makes perfect sense to me, but looks more like a cleanup of the code, rather than actually changing the behavior. yes, few changing (this just avoid to enter in "if (host->variant->busy_detect)") at each time. I could move this part in cleanup patch (before this patch) Sounds good to me! @@ -1508,6 +1508,7 @@ static irqreturn_t mmci_irq(int irq, void *dev_id) { struct mmci_host *host = dev_id; u32 status; + bool busy_resp; int ret = 0; spin_lock(&host->lock); @@ -1550,9 +1551,15 @@ static irqreturn_t mmci_irq(int irq, void *dev_id) } /* -* Don't poll for busy completion in irq context. +* Don't poll for: +* -busy completion in irq context. +* -no busy response expected. */ - if (host->variant->busy_detect && host->busy_status) + busy_resp = host->cmd ? + !!(host->cmd->flags & MMC_RSP_BUSY) : false; This doesn't make sense to me, but I may be missing something. host->busy_status is being updated by mmci_cmd_irq() and only when MMC_RSP_BUSY is set for the command in flight. In other words, checking for MMC_RSP_BUSY here as well is redundant. No? In mmci_irq the "do while" loops until the status is totally cleared. Today (for variant with busy_detect option), the status busy_detect_flag is excluded only while busy_status period (command with MMC_RSP_BUSY and while busy line is low => "busy_status=1") On SDMMC variant I noticed that busy_detect_flag status could be enabled even if the command is not MMC_RSP_BUSY, for example sdmmc variant stay in loop while cmd11 voltage switch. Right, I see. So I wish extend host->variant->busy_detect_flag exclusion for all commands which is not a MMC_RSP_BUSY. I suppose that other variants could have the same behavior, and else there is no impact, normally. I am guessing this is because the variant->busy_dpsm_flag has been set in the datactrl register, which is needed for mmci_card_busy(). That said, I am kind of wondering if we ever should need repeat the while loop if 'status' contains the bit for hos
Re: [PATCH 1/4] mmc: mmci: avoid fake busy polling
hi Ulf On 4/23/19 3:39 PM, Ulf Hansson wrote: On Tue, 5 Mar 2019 at 17:10, Ludovic Barre wrote: From: Ludovic Barre The busy status bit could occurred even if no busy response is expected (example cmd11). On sdmmc variant, the busy_detect_flag reflects inverted value of d0 state, it's sampled at the end of a CMD response and a second time 2 clk cycles after the CMD response. To avoid a fake busy, the busy status could be checked and polled only if the command has RSP_BUSY flag. I would appreciate a better explanation of what this patch really changes. The above is giving some background to the behavior of sdmmc variant, but at this point that variant doesn't even have the ->variant->busy_detect flag set. Yes, I will try to explain more and focus on common behavior. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 19 +-- 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 387ff14..4901b73 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -1220,12 +1220,13 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, unsigned int status) { void __iomem *base = host->base; - bool sbc; + bool sbc, busy_resp; if (!cmd) return; sbc = (cmd == host->mrq->sbc); + busy_resp = !!(cmd->flags & MMC_RSP_BUSY); /* * We need to be one of these interrupts to be considered worth @@ -1239,8 +1240,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, /* * ST Micro variant: handle busy detection. */ - if (host->variant->busy_detect) { - bool busy_resp = !!(cmd->flags & MMC_RSP_BUSY); + if (busy_resp && host->variant->busy_detect) { /* We are busy with a command, return */ if (host->busy_status && @@ -1253,7 +1253,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, * that the special busy status bit is still set before * proceeding. */ - if (!host->busy_status && busy_resp && + if (!host->busy_status && !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) && (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) { All the changes above makes perfect sense to me, but looks more like a cleanup of the code, rather than actually changing the behavior. yes, few changing (this just avoid to enter in "if (host->variant->busy_detect)") at each time. I could move this part in cleanup patch (before this patch) @@ -1508,6 +1508,7 @@ static irqreturn_t mmci_irq(int irq, void *dev_id) { struct mmci_host *host = dev_id; u32 status; + bool busy_resp; int ret = 0; spin_lock(&host->lock); @@ -1550,9 +1551,15 @@ static irqreturn_t mmci_irq(int irq, void *dev_id) } /* -* Don't poll for busy completion in irq context. +* Don't poll for: +* -busy completion in irq context. +* -no busy response expected. */ - if (host->variant->busy_detect && host->busy_status) + busy_resp = host->cmd ? + !!(host->cmd->flags & MMC_RSP_BUSY) : false; This doesn't make sense to me, but I may be missing something. host->busy_status is being updated by mmci_cmd_irq() and only when MMC_RSP_BUSY is set for the command in flight. In other words, checking for MMC_RSP_BUSY here as well is redundant. No? In mmci_irq the "do while" loops until the status is totally cleared. Today (for variant with busy_detect option), the status busy_detect_flag is excluded only while busy_status period (command with MMC_RSP_BUSY and while busy line is low => "busy_status=1") On SDMMC variant I noticed that busy_detect_flag status could be enabled even if the command is not MMC_RSP_BUSY, for example sdmmc variant stay in loop while cmd11 voltage switch. So I wish extend host->variant->busy_detect_flag exclusion for all commands which is not a MMC_RSP_BUSY. I suppose that other variants could have the same behavior, and else there is no impact, normally. + + if (host->variant->busy_detect && + (!busy_resp || host->busy_status)) status &= ~host->variant->busy_detect_flag; ret = 1; -- 2.7.4 Kind regards Uffe
Re: [PATCH] spi: stm32-qspi: manage the get_irq error case
On 4/24/19 5:19 PM, Fabien Dessenne wrote: During probe, check the "get_irq" error value. Signed-off-by: Fabien Dessenne Acked-by: Ludovic Barre --- drivers/spi/spi-stm32-qspi.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/spi/spi-stm32-qspi.c b/drivers/spi/spi-stm32-qspi.c index 11a89aa..42f8e3c 100644 --- a/drivers/spi/spi-stm32-qspi.c +++ b/drivers/spi/spi-stm32-qspi.c @@ -574,6 +574,12 @@ static int stm32_qspi_probe(struct platform_device *pdev) } irq = platform_get_irq(pdev, 0); + if (irq < 0) { + if (irq != -EPROBE_DEFER) + dev_err(dev, "IRQ error missing or invalid\n"); + return irq; + } + ret = devm_request_irq(dev, irq, stm32_qspi_irq, 0, dev_name(dev), qspi); if (ret) {
Re: [PATCH 0/4] mmc: mmci: add busy detect for stm32 sdmmc variant
hi Ulf On 4/11/19 3:29 PM, Ulf Hansson wrote: On Thu, 11 Apr 2019 at 14:37, Ludovic BARRE wrote: Hi Ulf Just a gentleman ping about this series. I sent this series at same time of dt_mode (no dependence between both). Thanks for pinging. It's been a busy period, with travels etc. I am just catching up on everything and I will soon come back to this. No problem, I saw lot of power group presentations at linaro connect :-) I haven't tried to apply this on top of the other recent queued changes for mmci, but perhaps you can check if a rebase is needed? I try on the last mmc next, rebase, build and stm32mp157 test OK Especially since I plan to run this on my ux500 board. Kind regards Uffe BR Ludo On 3/5/19 5:10 PM, Ludovic Barre wrote: From: Ludovic Barre This patch series adds busy detect for stm32 sdmmc variant. Some adaptations are required: -Avoid to check and poll busy status when is not expected. -Clear busy status bit if busy_detect_flag and busy_detect_mask are different. -Add hardware busy timeout with MMCIDATATIMER register. Ludovic Barre (4): mmc: mmci: avoid fake busy polling mmc: mmci: fix clear of busy detect status mmc: mmci: add hardware busy timeout feature mmc: mmci: add busy detect for stm32 sdmmc variant drivers/mmc/host/mmci.c | 67 +++-- drivers/mmc/host/mmci.h | 3 +++ 2 files changed, 57 insertions(+), 13 deletions(-)
Re: [PATCH 0/4] mmc: mmci: add busy detect for stm32 sdmmc variant
Hi Ulf Just a gentleman ping about this series. I sent this series at same time of dt_mode (no dependence between both). BR Ludo On 3/5/19 5:10 PM, Ludovic Barre wrote: From: Ludovic Barre This patch series adds busy detect for stm32 sdmmc variant. Some adaptations are required: -Avoid to check and poll busy status when is not expected. -Clear busy status bit if busy_detect_flag and busy_detect_mask are different. -Add hardware busy timeout with MMCIDATATIMER register. Ludovic Barre (4): mmc: mmci: avoid fake busy polling mmc: mmci: fix clear of busy detect status mmc: mmci: add hardware busy timeout feature mmc: mmci: add busy detect for stm32 sdmmc variant drivers/mmc/host/mmci.c | 67 +++-- drivers/mmc/host/mmci.h | 3 +++ 2 files changed, 57 insertions(+), 13 deletions(-)
Re: [PATCH V4 4/5] mmc: mmci: stm32: define get_dctrl_cfg
On 3/27/19 11:54 AM, Ulf Hansson wrote: On Wed, 27 Mar 2019 at 10:05, Ludovic Barre wrote: From: Ludovic Barre This patch defines get_dctrl_cfg callback for sdmmc variant. sdmmc variant has specific stm32 transfer modes. sdmmc data transfer mode selection could be: -Block data transfer ending on block count. -SDIO multibyte data transfer. -MMC Stream data transfer (not used). -Block data transfer ending with STOP_TRANSMISSION command. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.h | 5 + drivers/mmc/host/mmci_stm32_sdmmc.c | 18 ++ 2 files changed, 23 insertions(+) diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h index 35c91d0..82e9f94 100644 --- a/drivers/mmc/host/mmci.h +++ b/drivers/mmc/host/mmci.h @@ -131,6 +131,11 @@ /* Control register extensions in the Qualcomm versions */ #define MCI_DPSM_QCOM_DATA_PENDBIT(17) #define MCI_DPSM_QCOM_RX_DATA_PEND BIT(20) +/* Control register extensions in STM32 versions */ +#define MCI_DPSM_STM32_MODE_BLOCK (0 << 2) +#define MCI_DPSM_STM32_MODE_SDIO (1 << 2) +#define MCI_DPSM_STM32_MODE_STREAM (2 << 2) +#define MCI_DPSM_STM32_MODE_BLOCK_STOP (3 << 2) #define MMCIDATACNT0x030 #define MMCISTATUS 0x034 diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c index cfbfc6f..8e83ae6 100644 --- a/drivers/mmc/host/mmci_stm32_sdmmc.c +++ b/drivers/mmc/host/mmci_stm32_sdmmc.c @@ -265,10 +265,28 @@ static void mmci_sdmmc_set_pwrreg(struct mmci_host *host, unsigned int pwr) } } +static u32 sdmmc_get_dctrl_cfg(struct mmci_host *host) +{ + u32 datactrl; + + datactrl = mmci_dctrl_blksz(host); + + if (host->mmc->card && mmc_card_sdio(host->mmc->card) && + host->data->blocks == 1) + datactrl |= MCI_DPSM_STM32_MODE_SDIO; + else if (host->data->stop && !host->mrq->sbc) + datactrl |= MCI_DPSM_STM32_MODE_BLOCK_STOP; Just a question here. Does this mean that the controller sends the stop command automatically when a transfer has finished successfully? The controller not sends the stop command, this bit is just an information for the Data State Machine to know how the data transfer finish. Regards, Ludo + else + datactrl |= MCI_DPSM_STM32_MODE_BLOCK; + + return datactrl; +} + static struct mmci_host_ops sdmmc_variant_ops = { .validate_data = sdmmc_idma_validate_data, .prep_data = sdmmc_idma_prep_data, .unprep_data = sdmmc_idma_unprep_data, + .get_datactrl_cfg = sdmmc_get_dctrl_cfg, .dma_setup = sdmmc_idma_setup, .dma_start = sdmmc_idma_start, .dma_finalize = sdmmc_idma_finalize, -- 2.7.4 Kind regards Uffe
[PATCH V4 2/5] mmc: mmci: define get_dctrl_cfg for legacy variant
From: Ludovic Barre This patch defines get_dctrl_cfg callback for legacy variants whatever DMA_ENGINE configuration. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 30 -- 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 9e6a2c1..a818511 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -46,11 +46,8 @@ #define DRIVER_NAME "mmci-pl18x" -#ifdef CONFIG_DMA_ENGINE static void mmci_variant_init(struct mmci_host *host); -#else -static inline void mmci_variant_init(struct mmci_host *host) {} -#endif +static void ux500v2_variant_init(struct mmci_host *host); static unsigned int fmax = 515633; @@ -231,7 +228,7 @@ static struct variant_data variant_ux500v2 = { .irq_pio_mask = MCI_IRQ_PIO_MASK, .start_err = MCI_STARTBITERR, .opendrain = MCI_OD, - .init = mmci_variant_init, + .init = ux500v2_variant_init, }; static struct variant_data variant_stm32 = { @@ -617,6 +614,16 @@ static void mmci_init_sg(struct mmci_host *host, struct mmc_data *data) sg_miter_start(&host->sg_miter, data->sg, data->sg_len, flags); } +static u32 mmci_get_dctrl_cfg(struct mmci_host *host) +{ + return MCI_DPSM_ENABLE | mmci_dctrl_blksz(host); +} + +static u32 ux500v2_get_dctrl_cfg(struct mmci_host *host) +{ + return MCI_DPSM_ENABLE | (host->data->blksz << 16); +} + /* * All the DMA operation mode stuff goes inside this ifdef. * This assumes that you have a generic DMA device interface, @@ -941,6 +948,7 @@ void mmci_dmae_unprep_data(struct mmci_host *host, static struct mmci_host_ops mmci_variant_ops = { .prep_data = mmci_dmae_prep_data, .unprep_data = mmci_dmae_unprep_data, + .get_datactrl_cfg = mmci_get_dctrl_cfg, .get_next_data = mmci_dmae_get_next_data, .dma_setup = mmci_dmae_setup, .dma_release = mmci_dmae_release, @@ -948,12 +956,22 @@ static struct mmci_host_ops mmci_variant_ops = { .dma_finalize = mmci_dmae_finalize, .dma_error = mmci_dmae_error, }; +#else +static struct mmci_host_ops mmci_variant_ops = { + .get_datactrl_cfg = mmci_get_dctrl_cfg, +} +#endif void mmci_variant_init(struct mmci_host *host) { host->ops = &mmci_variant_ops; } -#endif + +void ux500v2_variant_init(struct mmci_host *host) +{ + host->ops = &mmci_variant_ops; + host->ops->get_datactrl_cfg = ux500v2_get_dctrl_cfg; +} static void mmci_pre_request(struct mmc_host *mmc, struct mmc_request *mrq) { -- 2.7.4
[PATCH V4 3/5] mmc: mmci: qcom: define get_dctrl_cfg
From: Ludovic Barre This patch defines get_dctrl_cfg callback for qcom variant. qcom variant has a specific block size definition. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci_qcom_dml.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/mmc/host/mmci_qcom_dml.c b/drivers/mmc/host/mmci_qcom_dml.c index ccc1b18..3af396b 100644 --- a/drivers/mmc/host/mmci_qcom_dml.c +++ b/drivers/mmc/host/mmci_qcom_dml.c @@ -188,9 +188,15 @@ static int qcom_dma_setup(struct mmci_host *host) return 0; } +static u32 qcom_get_dctrl_cfg(struct mmci_host *host) +{ + return MCI_DPSM_ENABLE | (host->data->blksz << 4); +} + static struct mmci_host_ops qcom_variant_ops = { .prep_data = mmci_dmae_prep_data, .unprep_data = mmci_dmae_unprep_data, + .get_datactrl_cfg = qcom_get_dctrl_cfg, .get_next_data = mmci_dmae_get_next_data, .dma_setup = qcom_dma_setup, .dma_release = mmci_dmae_release, -- 2.7.4
[PATCH V4 4/5] mmc: mmci: stm32: define get_dctrl_cfg
From: Ludovic Barre This patch defines get_dctrl_cfg callback for sdmmc variant. sdmmc variant has specific stm32 transfer modes. sdmmc data transfer mode selection could be: -Block data transfer ending on block count. -SDIO multibyte data transfer. -MMC Stream data transfer (not used). -Block data transfer ending with STOP_TRANSMISSION command. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.h | 5 + drivers/mmc/host/mmci_stm32_sdmmc.c | 18 ++ 2 files changed, 23 insertions(+) diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h index 35c91d0..82e9f94 100644 --- a/drivers/mmc/host/mmci.h +++ b/drivers/mmc/host/mmci.h @@ -131,6 +131,11 @@ /* Control register extensions in the Qualcomm versions */ #define MCI_DPSM_QCOM_DATA_PENDBIT(17) #define MCI_DPSM_QCOM_RX_DATA_PEND BIT(20) +/* Control register extensions in STM32 versions */ +#define MCI_DPSM_STM32_MODE_BLOCK (0 << 2) +#define MCI_DPSM_STM32_MODE_SDIO (1 << 2) +#define MCI_DPSM_STM32_MODE_STREAM (2 << 2) +#define MCI_DPSM_STM32_MODE_BLOCK_STOP (3 << 2) #define MMCIDATACNT0x030 #define MMCISTATUS 0x034 diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c index cfbfc6f..8e83ae6 100644 --- a/drivers/mmc/host/mmci_stm32_sdmmc.c +++ b/drivers/mmc/host/mmci_stm32_sdmmc.c @@ -265,10 +265,28 @@ static void mmci_sdmmc_set_pwrreg(struct mmci_host *host, unsigned int pwr) } } +static u32 sdmmc_get_dctrl_cfg(struct mmci_host *host) +{ + u32 datactrl; + + datactrl = mmci_dctrl_blksz(host); + + if (host->mmc->card && mmc_card_sdio(host->mmc->card) && + host->data->blocks == 1) + datactrl |= MCI_DPSM_STM32_MODE_SDIO; + else if (host->data->stop && !host->mrq->sbc) + datactrl |= MCI_DPSM_STM32_MODE_BLOCK_STOP; + else + datactrl |= MCI_DPSM_STM32_MODE_BLOCK; + + return datactrl; +} + static struct mmci_host_ops sdmmc_variant_ops = { .validate_data = sdmmc_idma_validate_data, .prep_data = sdmmc_idma_prep_data, .unprep_data = sdmmc_idma_unprep_data, + .get_datactrl_cfg = sdmmc_get_dctrl_cfg, .dma_setup = sdmmc_idma_setup, .dma_start = sdmmc_idma_start, .dma_finalize = sdmmc_idma_finalize, -- 2.7.4
[PATCH V4 1/5] mmc: mmci: add get_datactrl_cfg callback and helper functions
From: Ludovic Barre This patch adds get_datactrl_cfg callback in mmci_host_ops to allow to get datactrl configuration specific at variant. Common helper function is defined and could be call by variant. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h index 6bde28c..35c91d0 100644 --- a/drivers/mmc/host/mmci.h +++ b/drivers/mmc/host/mmci.h @@ -362,6 +362,7 @@ struct mmci_host_ops { bool next); void (*unprep_data)(struct mmci_host *host, struct mmc_data *data, int err); + u32 (*get_datactrl_cfg)(struct mmci_host *host); void (*get_next_data)(struct mmci_host *host, struct mmc_data *data); int (*dma_setup)(struct mmci_host *host); void (*dma_release)(struct mmci_host *host); @@ -429,6 +430,11 @@ struct mmci_host { void mmci_write_clkreg(struct mmci_host *host, u32 clk); void mmci_write_pwrreg(struct mmci_host *host, u32 pwr); +static inline u32 mmci_dctrl_blksz(struct mmci_host *host) +{ + return (ffs(host->data->blksz) - 1) << 4; +} + #ifdef CONFIG_DMA_ENGINE int mmci_dmae_prep_data(struct mmci_host *host, struct mmc_data *data, bool next); -- 2.7.4
[PATCH V4 5/5] mmc: mmci: replace blksz_datactrlXX by get_datactrl_cfg callback
From: Ludovic Barre This patch allows to get datactrl configuration specific at variant. This introduce more flexibility on datactlr value. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 26 ++ drivers/mmc/host/mmci.h | 7 --- 2 files changed, 2 insertions(+), 31 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index a818511..5b2cf77 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -60,7 +60,6 @@ static struct variant_data variant_arm = { .cmdreg_srsp= MCI_CPSM_RESPONSE, .datalength_bits= 16, .datactrl_blocksz = 11, - .datactrl_dpsm_enable = MCI_DPSM_ENABLE, .pwrreg_powerup = MCI_PWR_UP, .f_max = 1, .reversed_irq_handling = true, @@ -80,7 +79,6 @@ static struct variant_data variant_arm_extended_fifo = { .cmdreg_srsp= MCI_CPSM_RESPONSE, .datalength_bits= 16, .datactrl_blocksz = 11, - .datactrl_dpsm_enable = MCI_DPSM_ENABLE, .pwrreg_powerup = MCI_PWR_UP, .f_max = 1, .mmcimask1 = true, @@ -100,7 +98,6 @@ static struct variant_data variant_arm_extended_fifo_hwfc = { .cmdreg_srsp= MCI_CPSM_RESPONSE, .datalength_bits= 16, .datactrl_blocksz = 11, - .datactrl_dpsm_enable = MCI_DPSM_ENABLE, .pwrreg_powerup = MCI_PWR_UP, .f_max = 1, .mmcimask1 = true, @@ -121,7 +118,6 @@ static struct variant_data variant_u300 = { .cmdreg_srsp= MCI_CPSM_RESPONSE, .datalength_bits= 16, .datactrl_blocksz = 11, - .datactrl_dpsm_enable = MCI_DPSM_ENABLE, .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN, .st_sdio= true, .pwrreg_powerup = MCI_PWR_ON, @@ -147,7 +143,6 @@ static struct variant_data variant_nomadik = { .cmdreg_srsp= MCI_CPSM_RESPONSE, .datalength_bits= 24, .datactrl_blocksz = 11, - .datactrl_dpsm_enable = MCI_DPSM_ENABLE, .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN, .st_sdio= true, .st_clkdiv = true, @@ -176,7 +171,6 @@ static struct variant_data variant_ux500 = { .cmdreg_srsp= MCI_CPSM_RESPONSE, .datalength_bits= 24, .datactrl_blocksz = 11, - .datactrl_dpsm_enable = MCI_DPSM_ENABLE, .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN, .st_sdio= true, .st_clkdiv = true, @@ -210,11 +204,9 @@ static struct variant_data variant_ux500v2 = { .datactrl_mask_ddrmode = MCI_DPSM_ST_DDRMODE, .datalength_bits= 24, .datactrl_blocksz = 11, - .datactrl_dpsm_enable = MCI_DPSM_ENABLE, .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN, .st_sdio= true, .st_clkdiv = true, - .blksz_datactrl16 = true, .pwrreg_powerup = MCI_PWR_ON, .f_max = 1, .signal_direction = true, @@ -245,7 +237,6 @@ static struct variant_data variant_stm32 = { .irq_pio_mask = MCI_IRQ_PIO_MASK, .datalength_bits= 24, .datactrl_blocksz = 11, - .datactrl_dpsm_enable = MCI_DPSM_ENABLE, .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN, .st_sdio= true, .st_clkdiv = true, @@ -289,10 +280,8 @@ static struct variant_data variant_qcom = { .cmdreg_srsp_crc= MCI_CPSM_RESPONSE, .cmdreg_srsp= MCI_CPSM_RESPONSE, .data_cmd_enable= MCI_CPSM_QCOM_DATCMD, - .blksz_datactrl4= true, .datalength_bits= 24, .datactrl_blocksz = 11, - .datactrl_dpsm_enable = MCI_DPSM_ENABLE, .pwrreg_powerup = MCI_PWR_UP, .f_max = 20800, .explicit_mclk_control = true, @@ -1007,7 +996,6 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data) unsigned int datactrl, timeout, irqmask; unsigned long long clks; void __iomem *base; - int blksz_bits; dev_dbg(mmc_dev(host->mmc), "blksz %04x blks %04x flags %08x\n", data->blksz, data->blocks, data->flags); @@ -1025,18 +1013,8 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data) writel(timeout, base + MMCIDATATIMER); writel(host->size, base + MMCIDATALENGTH); - blksz_bits = ffs(data->blksz) - 1; - BUG_ON(1 << blksz_bits != data->blksz); - - if (variant->blksz_datactrl16) - datactrl
[PATCH V4 0/5] mmc: mmci: add get_datactrl_cfg callback
From: Ludovic Barre This patch series adds get_datactrl_cfg callback in mmci_host_ops to allow to get datactrl configuration specific at variant. change V4: -keep mmci and ux500v2 variant init in the c file. change V3: -keep the common functions in mmci_start_data. define function used by some variants like an helper (example mmci_dctrl_blks used by mmci and sdmmc). -To be consistent rename callback for ux500v2 change V2: -This V2 has been rebased on "mmc: mmci: Cleanup some variant related code" series -add helpers functions to define default datactrl value, each variant could use these helpers to define datactrl and adds their specific bits. -use static in qcom and stm32 -regroup mmci_(ux500v2)variant_init in header file to avoid checkpatch warning: "WARNING: externs should be avoided in .c files" -remove unused variant properties: "datactrl_dpsm_enable" "blksz_datactrl16" "blksz_datactrl4" Ludovic Barre (5): mmc: mmci: add get_datactrl_cfg callback and helper functions mmc: mmci: define get_dctrl_cfg for legacy variant mmc: mmci: qcom: define get_dctrl_cfg mmc: mmci: stm32: define get_dctrl_cfg mmc: mmci: replace blksz_datactrlXX by get_datactrl_cfg callback drivers/mmc/host/mmci.c | 56 + drivers/mmc/host/mmci.h | 18 +++- drivers/mmc/host/mmci_qcom_dml.c| 6 drivers/mmc/host/mmci_stm32_sdmmc.c | 18 4 files changed, 61 insertions(+), 37 deletions(-) -- 2.7.4
Re: [PATCH V3 2/5] mmc: mmci: define get_dctrl_cfg for legacy variant
On 3/26/19 6:46 PM, Ulf Hansson wrote: On Tue, 26 Mar 2019 at 10:11, Ludovic Barre wrote: From: Ludovic Barre This patch defines get_dctrl_cfg callback for legacy variants whatever DMA_ENGINE configuration. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 31 +++ drivers/mmc/host/mmci.h | 3 +++ 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 9e6a2c1..4041e36 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -46,12 +46,6 @@ #define DRIVER_NAME "mmci-pl18x" -#ifdef CONFIG_DMA_ENGINE -static void mmci_variant_init(struct mmci_host *host); -#else -static inline void mmci_variant_init(struct mmci_host *host) {} -#endif Looks like you are moving the declaration to the header file. I would rather keep it here as there is no point in sharing to another c-file (at least not yet). The same applies for the new ux500v2 init function. no problem, I will resend a V4 to keep mmci and ux500v2 variant init in the c file. Other than that, this looks good to me! thanks Kind regards Uffe
[PATCH V3 3/5] mmc: mmci: qcom: define get_dctrl_cfg
From: Ludovic Barre This patch defines get_dctrl_cfg callback for qcom variant. qcom variant has a specific block size definition. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci_qcom_dml.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/mmc/host/mmci_qcom_dml.c b/drivers/mmc/host/mmci_qcom_dml.c index ccc1b18..3af396b 100644 --- a/drivers/mmc/host/mmci_qcom_dml.c +++ b/drivers/mmc/host/mmci_qcom_dml.c @@ -188,9 +188,15 @@ static int qcom_dma_setup(struct mmci_host *host) return 0; } +static u32 qcom_get_dctrl_cfg(struct mmci_host *host) +{ + return MCI_DPSM_ENABLE | (host->data->blksz << 4); +} + static struct mmci_host_ops qcom_variant_ops = { .prep_data = mmci_dmae_prep_data, .unprep_data = mmci_dmae_unprep_data, + .get_datactrl_cfg = qcom_get_dctrl_cfg, .get_next_data = mmci_dmae_get_next_data, .dma_setup = qcom_dma_setup, .dma_release = mmci_dmae_release, -- 2.7.4
[PATCH V3 5/5] mmc: mmci: replace blksz_datactrlXX by get_datactrl_cfg callback
From: Ludovic Barre This patch allows to get datactrl configuration specific at variant. This introduce more flexibility on datactlr value. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 26 ++ drivers/mmc/host/mmci.h | 7 --- 2 files changed, 2 insertions(+), 31 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 4041e36..cd3a65c 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -57,7 +57,6 @@ static struct variant_data variant_arm = { .cmdreg_srsp= MCI_CPSM_RESPONSE, .datalength_bits= 16, .datactrl_blocksz = 11, - .datactrl_dpsm_enable = MCI_DPSM_ENABLE, .pwrreg_powerup = MCI_PWR_UP, .f_max = 1, .reversed_irq_handling = true, @@ -77,7 +76,6 @@ static struct variant_data variant_arm_extended_fifo = { .cmdreg_srsp= MCI_CPSM_RESPONSE, .datalength_bits= 16, .datactrl_blocksz = 11, - .datactrl_dpsm_enable = MCI_DPSM_ENABLE, .pwrreg_powerup = MCI_PWR_UP, .f_max = 1, .mmcimask1 = true, @@ -97,7 +95,6 @@ static struct variant_data variant_arm_extended_fifo_hwfc = { .cmdreg_srsp= MCI_CPSM_RESPONSE, .datalength_bits= 16, .datactrl_blocksz = 11, - .datactrl_dpsm_enable = MCI_DPSM_ENABLE, .pwrreg_powerup = MCI_PWR_UP, .f_max = 1, .mmcimask1 = true, @@ -118,7 +115,6 @@ static struct variant_data variant_u300 = { .cmdreg_srsp= MCI_CPSM_RESPONSE, .datalength_bits= 16, .datactrl_blocksz = 11, - .datactrl_dpsm_enable = MCI_DPSM_ENABLE, .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN, .st_sdio= true, .pwrreg_powerup = MCI_PWR_ON, @@ -144,7 +140,6 @@ static struct variant_data variant_nomadik = { .cmdreg_srsp= MCI_CPSM_RESPONSE, .datalength_bits= 24, .datactrl_blocksz = 11, - .datactrl_dpsm_enable = MCI_DPSM_ENABLE, .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN, .st_sdio= true, .st_clkdiv = true, @@ -173,7 +168,6 @@ static struct variant_data variant_ux500 = { .cmdreg_srsp= MCI_CPSM_RESPONSE, .datalength_bits= 24, .datactrl_blocksz = 11, - .datactrl_dpsm_enable = MCI_DPSM_ENABLE, .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN, .st_sdio= true, .st_clkdiv = true, @@ -207,11 +201,9 @@ static struct variant_data variant_ux500v2 = { .datactrl_mask_ddrmode = MCI_DPSM_ST_DDRMODE, .datalength_bits= 24, .datactrl_blocksz = 11, - .datactrl_dpsm_enable = MCI_DPSM_ENABLE, .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN, .st_sdio= true, .st_clkdiv = true, - .blksz_datactrl16 = true, .pwrreg_powerup = MCI_PWR_ON, .f_max = 1, .signal_direction = true, @@ -242,7 +234,6 @@ static struct variant_data variant_stm32 = { .irq_pio_mask = MCI_IRQ_PIO_MASK, .datalength_bits= 24, .datactrl_blocksz = 11, - .datactrl_dpsm_enable = MCI_DPSM_ENABLE, .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN, .st_sdio= true, .st_clkdiv = true, @@ -286,10 +277,8 @@ static struct variant_data variant_qcom = { .cmdreg_srsp_crc= MCI_CPSM_RESPONSE, .cmdreg_srsp= MCI_CPSM_RESPONSE, .data_cmd_enable= MCI_CPSM_QCOM_DATCMD, - .blksz_datactrl4= true, .datalength_bits= 24, .datactrl_blocksz = 11, - .datactrl_dpsm_enable = MCI_DPSM_ENABLE, .pwrreg_powerup = MCI_PWR_UP, .f_max = 20800, .explicit_mclk_control = true, @@ -1004,7 +993,6 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data) unsigned int datactrl, timeout, irqmask; unsigned long long clks; void __iomem *base; - int blksz_bits; dev_dbg(mmc_dev(host->mmc), "blksz %04x blks %04x flags %08x\n", data->blksz, data->blocks, data->flags); @@ -1022,18 +1010,8 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data) writel(timeout, base + MMCIDATATIMER); writel(host->size, base + MMCIDATALENGTH); - blksz_bits = ffs(data->blksz) - 1; - BUG_ON(1 << blksz_bits != data->blksz); - - if (variant->blksz_datactrl16) - datactrl
[PATCH V3 2/5] mmc: mmci: define get_dctrl_cfg for legacy variant
From: Ludovic Barre This patch defines get_dctrl_cfg callback for legacy variants whatever DMA_ENGINE configuration. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 31 +++ drivers/mmc/host/mmci.h | 3 +++ 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 9e6a2c1..4041e36 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -46,12 +46,6 @@ #define DRIVER_NAME "mmci-pl18x" -#ifdef CONFIG_DMA_ENGINE -static void mmci_variant_init(struct mmci_host *host); -#else -static inline void mmci_variant_init(struct mmci_host *host) {} -#endif - static unsigned int fmax = 515633; static struct variant_data variant_arm = { @@ -231,7 +225,7 @@ static struct variant_data variant_ux500v2 = { .irq_pio_mask = MCI_IRQ_PIO_MASK, .start_err = MCI_STARTBITERR, .opendrain = MCI_OD, - .init = mmci_variant_init, + .init = ux500v2_variant_init, }; static struct variant_data variant_stm32 = { @@ -617,6 +611,16 @@ static void mmci_init_sg(struct mmci_host *host, struct mmc_data *data) sg_miter_start(&host->sg_miter, data->sg, data->sg_len, flags); } +static u32 mmci_get_dctrl_cfg(struct mmci_host *host) +{ + return MCI_DPSM_ENABLE | mmci_dctrl_blksz(host); +} + +static u32 ux500v2_get_dctrl_cfg(struct mmci_host *host) +{ + return MCI_DPSM_ENABLE | (host->data->blksz << 16); +} + /* * All the DMA operation mode stuff goes inside this ifdef. * This assumes that you have a generic DMA device interface, @@ -941,6 +945,7 @@ void mmci_dmae_unprep_data(struct mmci_host *host, static struct mmci_host_ops mmci_variant_ops = { .prep_data = mmci_dmae_prep_data, .unprep_data = mmci_dmae_unprep_data, + .get_datactrl_cfg = mmci_get_dctrl_cfg, .get_next_data = mmci_dmae_get_next_data, .dma_setup = mmci_dmae_setup, .dma_release = mmci_dmae_release, @@ -948,12 +953,22 @@ static struct mmci_host_ops mmci_variant_ops = { .dma_finalize = mmci_dmae_finalize, .dma_error = mmci_dmae_error, }; +#else +static struct mmci_host_ops mmci_variant_ops = { + .get_datactrl_cfg = mmci_get_dctrl_cfg, +} +#endif void mmci_variant_init(struct mmci_host *host) { host->ops = &mmci_variant_ops; } -#endif + +void ux500v2_variant_init(struct mmci_host *host) +{ + host->ops = &mmci_variant_ops; + host->ops->get_datactrl_cfg = ux500v2_get_dctrl_cfg; +} static void mmci_pre_request(struct mmc_host *mmc, struct mmc_request *mrq) { diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h index 35c91d0..c614ea7 100644 --- a/drivers/mmc/host/mmci.h +++ b/drivers/mmc/host/mmci.h @@ -448,6 +448,9 @@ void mmci_dmae_finalize(struct mmci_host *host, struct mmc_data *data); void mmci_dmae_error(struct mmci_host *host); #endif +void mmci_variant_init(struct mmci_host *host); +void ux500v2_variant_init(struct mmci_host *host); + #ifdef CONFIG_MMC_QCOM_DML void qcom_variant_init(struct mmci_host *host); #else -- 2.7.4
[PATCH V3 4/5] mmc: mmci: stm32: define get_dctrl_cfg
From: Ludovic Barre This patch defines get_dctrl_cfg callback for sdmmc variant. sdmmc variant has specific stm32 transfer modes. sdmmc data transfer mode selection could be: -Block data transfer ending on block count. -SDIO multibyte data transfer. -MMC Stream data transfer (not used). -Block data transfer ending with STOP_TRANSMISSION command. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.h | 5 + drivers/mmc/host/mmci_stm32_sdmmc.c | 18 ++ 2 files changed, 23 insertions(+) diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h index c614ea7..fd24fdb 100644 --- a/drivers/mmc/host/mmci.h +++ b/drivers/mmc/host/mmci.h @@ -131,6 +131,11 @@ /* Control register extensions in the Qualcomm versions */ #define MCI_DPSM_QCOM_DATA_PENDBIT(17) #define MCI_DPSM_QCOM_RX_DATA_PEND BIT(20) +/* Control register extensions in STM32 versions */ +#define MCI_DPSM_STM32_MODE_BLOCK (0 << 2) +#define MCI_DPSM_STM32_MODE_SDIO (1 << 2) +#define MCI_DPSM_STM32_MODE_STREAM (2 << 2) +#define MCI_DPSM_STM32_MODE_BLOCK_STOP (3 << 2) #define MMCIDATACNT0x030 #define MMCISTATUS 0x034 diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c index cfbfc6f..8e83ae6 100644 --- a/drivers/mmc/host/mmci_stm32_sdmmc.c +++ b/drivers/mmc/host/mmci_stm32_sdmmc.c @@ -265,10 +265,28 @@ static void mmci_sdmmc_set_pwrreg(struct mmci_host *host, unsigned int pwr) } } +static u32 sdmmc_get_dctrl_cfg(struct mmci_host *host) +{ + u32 datactrl; + + datactrl = mmci_dctrl_blksz(host); + + if (host->mmc->card && mmc_card_sdio(host->mmc->card) && + host->data->blocks == 1) + datactrl |= MCI_DPSM_STM32_MODE_SDIO; + else if (host->data->stop && !host->mrq->sbc) + datactrl |= MCI_DPSM_STM32_MODE_BLOCK_STOP; + else + datactrl |= MCI_DPSM_STM32_MODE_BLOCK; + + return datactrl; +} + static struct mmci_host_ops sdmmc_variant_ops = { .validate_data = sdmmc_idma_validate_data, .prep_data = sdmmc_idma_prep_data, .unprep_data = sdmmc_idma_unprep_data, + .get_datactrl_cfg = sdmmc_get_dctrl_cfg, .dma_setup = sdmmc_idma_setup, .dma_start = sdmmc_idma_start, .dma_finalize = sdmmc_idma_finalize, -- 2.7.4
[PATCH V3 0/5] mmc: mmci: add get_datactrl_cfg callback
From: Ludovic Barre This patch series adds get_datactrl_cfg callback in mmci_host_ops to allow to get datactrl configuration specific at variant. change V3: -keep the common functions in mmci_start_data. define function used by some variants like an helper (example mmci_dctrl_blks used by mmci and sdmmc). -To be consistent rename callback for ux500v2 change V2: -This V2 has been rebased on "mmc: mmci: Cleanup some variant related code" series -add helpers functions to define default datactrl value, each variant could use these helpers to define datactrl and adds their specific bits. -use static in qcom and stm32 -regroup mmci_(ux500v2)variant_init in header file to avoid checkpatch warning: "WARNING: externs should be avoided in .c files" -remove unused variant properties: "datactrl_dpsm_enable" "blksz_datactrl16" "blksz_datactrl4" Ludovic Barre (5): mmc: mmci: add get_datactrl_cfg callback and helper functions mmc: mmci: define get_dctrl_cfg for legacy variant mmc: mmci: qcom: define get_dctrl_cfg mmc: mmci: stm32: define get_dctrl_cfg mmc: mmci: replace blksz_datactrlXX by get_datactrl_cfg callback drivers/mmc/host/mmci.c | 57 - drivers/mmc/host/mmci.h | 21 +- drivers/mmc/host/mmci_qcom_dml.c| 6 drivers/mmc/host/mmci_stm32_sdmmc.c | 18 4 files changed, 63 insertions(+), 39 deletions(-) -- 2.7.4
[PATCH V3 1/5] mmc: mmci: add get_datactrl_cfg callback and helper functions
From: Ludovic Barre This patch adds get_datactrl_cfg callback in mmci_host_ops to allow to get datactrl configuration specific at variant. Common helper function is defined and could be call by variant. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h index 6bde28c..35c91d0 100644 --- a/drivers/mmc/host/mmci.h +++ b/drivers/mmc/host/mmci.h @@ -362,6 +362,7 @@ struct mmci_host_ops { bool next); void (*unprep_data)(struct mmci_host *host, struct mmc_data *data, int err); + u32 (*get_datactrl_cfg)(struct mmci_host *host); void (*get_next_data)(struct mmci_host *host, struct mmc_data *data); int (*dma_setup)(struct mmci_host *host); void (*dma_release)(struct mmci_host *host); @@ -429,6 +430,11 @@ struct mmci_host { void mmci_write_clkreg(struct mmci_host *host, u32 clk); void mmci_write_pwrreg(struct mmci_host *host, u32 pwr); +static inline u32 mmci_dctrl_blksz(struct mmci_host *host) +{ + return (ffs(host->data->blksz) - 1) << 4; +} + #ifdef CONFIG_DMA_ENGINE int mmci_dmae_prep_data(struct mmci_host *host, struct mmc_data *data, bool next); -- 2.7.4
[PATCH V2 2/2] spi: stm32-qspi: add dma support
From: Ludovic Barre This patch adds the dma support for the stm32-qspi hardware. The memory buffer constraints (lowmem, vmalloc, kmap) are taken into account by framework. In read mode, the memory map is preferred vs dma (due to better throughput). If the dma transfer fails the buffer is sent by polling. Signed-off-by: Ludovic Barre --- drivers/spi/spi-stm32-qspi.c | 136 ++- 1 file changed, 135 insertions(+), 1 deletion(-) diff --git a/drivers/spi/spi-stm32-qspi.c b/drivers/spi/spi-stm32-qspi.c index 9875139..11a89aa 100644 --- a/drivers/spi/spi-stm32-qspi.c +++ b/drivers/spi/spi-stm32-qspi.c @@ -5,6 +5,8 @@ */ #include #include +#include +#include #include #include #include @@ -84,6 +86,7 @@ #define STM32_FIFO_TIMEOUT_US 3 #define STM32_BUSY_TIMEOUT_US 10 #define STM32_ABT_TIMEOUT_US 10 +#define STM32_COMP_TIMEOUT_MS 1000 struct stm32_qspi_flash { struct stm32_qspi *qspi; @@ -94,6 +97,7 @@ struct stm32_qspi_flash { struct stm32_qspi { struct device *dev; struct spi_controller *ctrl; + phys_addr_t phys_base; void __iomem *io_base; void __iomem *mm_base; resource_size_t mm_size; @@ -103,6 +107,10 @@ struct stm32_qspi { struct completion data_completion; u32 fmode; + struct dma_chan *dma_chtx; + struct dma_chan *dma_chrx; + struct completion dma_completion; + u32 cr_reg; u32 dcr_reg; @@ -181,6 +189,81 @@ static int stm32_qspi_tx_mm(struct stm32_qspi *qspi, return 0; } +static void stm32_qspi_dma_callback(void *arg) +{ + struct completion *dma_completion = arg; + + complete(dma_completion); +} + +static int stm32_qspi_tx_dma(struct stm32_qspi *qspi, +const struct spi_mem_op *op) +{ + struct dma_async_tx_descriptor *desc; + enum dma_transfer_direction dma_dir; + struct dma_chan *dma_ch; + struct sg_table sgt; + dma_cookie_t cookie; + u32 cr, t_out; + int err; + + if (op->data.dir == SPI_MEM_DATA_IN) { + dma_dir = DMA_DEV_TO_MEM; + dma_ch = qspi->dma_chrx; + } else { + dma_dir = DMA_MEM_TO_DEV; + dma_ch = qspi->dma_chtx; + } + + /* +* spi_map_buf return -EINVAL if the buffer is not DMA-able +* (DMA-able: in vmalloc | kmap | virt_addr_valid) +*/ + err = spi_controller_dma_map_mem_op_data(qspi->ctrl, op, &sgt); + if (err) + return err; + + desc = dmaengine_prep_slave_sg(dma_ch, sgt.sgl, sgt.nents, + dma_dir, DMA_PREP_INTERRUPT); + if (!desc) { + err = -ENOMEM; + goto out_unmap; + } + + cr = readl_relaxed(qspi->io_base + QSPI_CR); + + reinit_completion(&qspi->dma_completion); + desc->callback = stm32_qspi_dma_callback; + desc->callback_param = &qspi->dma_completion; + cookie = dmaengine_submit(desc); + err = dma_submit_error(cookie); + if (err) + goto out; + + dma_async_issue_pending(dma_ch); + + writel_relaxed(cr | CR_DMAEN, qspi->io_base + QSPI_CR); + + t_out = sgt.nents * STM32_COMP_TIMEOUT_MS; + if (!wait_for_completion_interruptible_timeout(&qspi->dma_completion, + msecs_to_jiffies(t_out))) + err = -ETIMEDOUT; + + if (dma_async_is_tx_complete(dma_ch, cookie, +NULL, NULL) != DMA_COMPLETE) + err = -ETIMEDOUT; + + if (err) + dmaengine_terminate_all(dma_ch); + +out: + writel_relaxed(cr & ~CR_DMAEN, qspi->io_base + QSPI_CR); +out_unmap: + spi_controller_dma_unmap_mem_op_data(qspi->ctrl, op, &sgt); + + return err; +} + static int stm32_qspi_tx(struct stm32_qspi *qspi, const struct spi_mem_op *op) { if (!op->data.nbytes) @@ -188,6 +271,10 @@ static int stm32_qspi_tx(struct stm32_qspi *qspi, const struct spi_mem_op *op) if (qspi->fmode == CCR_FMODE_MM) return stm32_qspi_tx_mm(qspi, op); + else if ((op->data.dir == SPI_MEM_DATA_IN && qspi->dma_chrx) || +(op->data.dir == SPI_MEM_DATA_OUT && qspi->dma_chtx)) + if (!stm32_qspi_tx_dma(qspi, op)) + return 0; return stm32_qspi_tx_poll(qspi, op); } @@ -218,7 +305,7 @@ static int stm32_qspi_wait_cmd(struct stm32_qspi *qspi, writel_relaxed(cr | CR_TCIE | CR_TEIE, qspi->io_base + QSPI_CR); if (!wait_for_completion_interruptible_timeout(&qspi->data_completion, - msecs_to_jiffies(1000))) { + msecs_to_jiffies(STM32_COMP_TIMEOUT_MS))) { err =
[PATCH V2 1/2] spi: stm32-qspi: add spi_master_put in release function
From: Ludovic Barre This patch adds spi_master_put in release function to drop the controller's refcount. Signed-off-by: Ludovic Barre --- drivers/spi/spi-stm32-qspi.c | 46 +++- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/drivers/spi/spi-stm32-qspi.c b/drivers/spi/spi-stm32-qspi.c index 7879a52..9875139 100644 --- a/drivers/spi/spi-stm32-qspi.c +++ b/drivers/spi/spi-stm32-qspi.c @@ -93,6 +93,7 @@ struct stm32_qspi_flash { struct stm32_qspi { struct device *dev; + struct spi_controller *ctrl; void __iomem *io_base; void __iomem *mm_base; resource_size_t mm_size; @@ -400,6 +401,7 @@ static void stm32_qspi_release(struct stm32_qspi *qspi) writel_relaxed(0, qspi->io_base + QSPI_CR); mutex_destroy(&qspi->lock); clk_disable_unprepare(qspi->clk); + spi_master_put(qspi->ctrl); } static int stm32_qspi_probe(struct platform_device *pdev) @@ -416,43 +418,54 @@ static int stm32_qspi_probe(struct platform_device *pdev) return -ENOMEM; qspi = spi_controller_get_devdata(ctrl); + qspi->ctrl = ctrl; res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qspi"); qspi->io_base = devm_ioremap_resource(dev, res); - if (IS_ERR(qspi->io_base)) - return PTR_ERR(qspi->io_base); + if (IS_ERR(qspi->io_base)) { + ret = PTR_ERR(qspi->io_base); + goto err; + } res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qspi_mm"); qspi->mm_base = devm_ioremap_resource(dev, res); - if (IS_ERR(qspi->mm_base)) - return PTR_ERR(qspi->mm_base); + if (IS_ERR(qspi->mm_base)) { + ret = PTR_ERR(qspi->mm_base); + goto err; + } qspi->mm_size = resource_size(res); - if (qspi->mm_size > STM32_QSPI_MAX_MMAP_SZ) - return -EINVAL; + if (qspi->mm_size > STM32_QSPI_MAX_MMAP_SZ) { + ret = -EINVAL; + goto err; + } irq = platform_get_irq(pdev, 0); ret = devm_request_irq(dev, irq, stm32_qspi_irq, 0, dev_name(dev), qspi); if (ret) { dev_err(dev, "failed to request irq\n"); - return ret; + goto err; } init_completion(&qspi->data_completion); qspi->clk = devm_clk_get(dev, NULL); - if (IS_ERR(qspi->clk)) - return PTR_ERR(qspi->clk); + if (IS_ERR(qspi->clk)) { + ret = PTR_ERR(qspi->clk); + goto err; + } qspi->clk_rate = clk_get_rate(qspi->clk); - if (!qspi->clk_rate) - return -EINVAL; + if (!qspi->clk_rate) { + ret = -EINVAL; + goto err; + } ret = clk_prepare_enable(qspi->clk); if (ret) { dev_err(dev, "can not enable the clock\n"); - return ret; + goto err; } rstc = devm_reset_control_get_exclusive(dev, NULL); @@ -475,14 +488,11 @@ static int stm32_qspi_probe(struct platform_device *pdev) ctrl->dev.of_node = dev->of_node; ret = devm_spi_register_master(dev, ctrl); - if (ret) - goto err_spi_register; - - return 0; + if (!ret) + return 0; -err_spi_register: +err: stm32_qspi_release(qspi); - return ret; } -- 2.7.4
[PATCH V2 0/2] spi: stm32-qspi: add dma support
From: Ludovic Barre This patch series adds dma support for the stm32-qspi. In read mode, the memory map is preferred vs dma (due to better throughput). If the dma transfer fails the buffer is sent by polling. V2: -fixe build error in patch 1/2 (move qspi->phys_base in patch 2) Ludovic Barre (2): spi: stm32-qspi: add spi_master_put in release function spi: stm32-qspi: add dma support drivers/spi/spi-stm32-qspi.c | 182 ++- 1 file changed, 163 insertions(+), 19 deletions(-) -- 2.7.4
Re: [PATCH 1/2] spi: stm32-qspi: add spi_master_put in release function
hi Mark On 3/25/19 5:03 PM, Mark Brown wrote: On Fri, Mar 22, 2019 at 03:35:53PM +0100, Ludovic Barre wrote: From: Ludovic Barre This patch adds spi_master_put in release function to drop the controller's refcount. I'm getting build errors with this: CC drivers/spi/spi-stm32-qspi.o drivers/spi/spi-stm32-qspi.c: In function ‘stm32_qspi_probe’: drivers/spi/spi-stm32-qspi.c:430:8: error: ‘struct stm32_qspi’ has no member named ‘phys_base’; did you mean ‘io_base’? qspi->phys_base = res->start; ^ io_base I was sure I built each patch, but the fact is there an error :-( I resend a serie with this line in the second patch Regards Ludo
[PATCH 2/2] spi: stm32-qspi: add dma support
From: Ludovic Barre This patch adds the dma support for the stm32-qspi hardware. The memory buffer constraints (lowmem, vmalloc, kmap) are taken into account by framework. In read mode, the memory map is preferred vs dma (due to better throughput). If the dma transfer fails the buffer is sent by polling. Signed-off-by: Ludovic Barre --- drivers/spi/spi-stm32-qspi.c | 134 ++- 1 file changed, 133 insertions(+), 1 deletion(-) diff --git a/drivers/spi/spi-stm32-qspi.c b/drivers/spi/spi-stm32-qspi.c index 983584d..11a89aa 100644 --- a/drivers/spi/spi-stm32-qspi.c +++ b/drivers/spi/spi-stm32-qspi.c @@ -5,6 +5,8 @@ */ #include #include +#include +#include #include #include #include @@ -84,6 +86,7 @@ #define STM32_FIFO_TIMEOUT_US 3 #define STM32_BUSY_TIMEOUT_US 10 #define STM32_ABT_TIMEOUT_US 10 +#define STM32_COMP_TIMEOUT_MS 1000 struct stm32_qspi_flash { struct stm32_qspi *qspi; @@ -94,6 +97,7 @@ struct stm32_qspi_flash { struct stm32_qspi { struct device *dev; struct spi_controller *ctrl; + phys_addr_t phys_base; void __iomem *io_base; void __iomem *mm_base; resource_size_t mm_size; @@ -103,6 +107,10 @@ struct stm32_qspi { struct completion data_completion; u32 fmode; + struct dma_chan *dma_chtx; + struct dma_chan *dma_chrx; + struct completion dma_completion; + u32 cr_reg; u32 dcr_reg; @@ -181,6 +189,81 @@ static int stm32_qspi_tx_mm(struct stm32_qspi *qspi, return 0; } +static void stm32_qspi_dma_callback(void *arg) +{ + struct completion *dma_completion = arg; + + complete(dma_completion); +} + +static int stm32_qspi_tx_dma(struct stm32_qspi *qspi, +const struct spi_mem_op *op) +{ + struct dma_async_tx_descriptor *desc; + enum dma_transfer_direction dma_dir; + struct dma_chan *dma_ch; + struct sg_table sgt; + dma_cookie_t cookie; + u32 cr, t_out; + int err; + + if (op->data.dir == SPI_MEM_DATA_IN) { + dma_dir = DMA_DEV_TO_MEM; + dma_ch = qspi->dma_chrx; + } else { + dma_dir = DMA_MEM_TO_DEV; + dma_ch = qspi->dma_chtx; + } + + /* +* spi_map_buf return -EINVAL if the buffer is not DMA-able +* (DMA-able: in vmalloc | kmap | virt_addr_valid) +*/ + err = spi_controller_dma_map_mem_op_data(qspi->ctrl, op, &sgt); + if (err) + return err; + + desc = dmaengine_prep_slave_sg(dma_ch, sgt.sgl, sgt.nents, + dma_dir, DMA_PREP_INTERRUPT); + if (!desc) { + err = -ENOMEM; + goto out_unmap; + } + + cr = readl_relaxed(qspi->io_base + QSPI_CR); + + reinit_completion(&qspi->dma_completion); + desc->callback = stm32_qspi_dma_callback; + desc->callback_param = &qspi->dma_completion; + cookie = dmaengine_submit(desc); + err = dma_submit_error(cookie); + if (err) + goto out; + + dma_async_issue_pending(dma_ch); + + writel_relaxed(cr | CR_DMAEN, qspi->io_base + QSPI_CR); + + t_out = sgt.nents * STM32_COMP_TIMEOUT_MS; + if (!wait_for_completion_interruptible_timeout(&qspi->dma_completion, + msecs_to_jiffies(t_out))) + err = -ETIMEDOUT; + + if (dma_async_is_tx_complete(dma_ch, cookie, +NULL, NULL) != DMA_COMPLETE) + err = -ETIMEDOUT; + + if (err) + dmaengine_terminate_all(dma_ch); + +out: + writel_relaxed(cr & ~CR_DMAEN, qspi->io_base + QSPI_CR); +out_unmap: + spi_controller_dma_unmap_mem_op_data(qspi->ctrl, op, &sgt); + + return err; +} + static int stm32_qspi_tx(struct stm32_qspi *qspi, const struct spi_mem_op *op) { if (!op->data.nbytes) @@ -188,6 +271,10 @@ static int stm32_qspi_tx(struct stm32_qspi *qspi, const struct spi_mem_op *op) if (qspi->fmode == CCR_FMODE_MM) return stm32_qspi_tx_mm(qspi, op); + else if ((op->data.dir == SPI_MEM_DATA_IN && qspi->dma_chrx) || +(op->data.dir == SPI_MEM_DATA_OUT && qspi->dma_chtx)) + if (!stm32_qspi_tx_dma(qspi, op)) + return 0; return stm32_qspi_tx_poll(qspi, op); } @@ -218,7 +305,7 @@ static int stm32_qspi_wait_cmd(struct stm32_qspi *qspi, writel_relaxed(cr | CR_TCIE | CR_TEIE, qspi->io_base + QSPI_CR); if (!wait_for_completion_interruptible_timeout(&qspi->data_completion, - msecs_to_jiffies(1000))) { + msecs_to_jiffies(STM32_COMP_TIMEOUT_MS))) { err =
[PATCH 1/2] spi: stm32-qspi: add spi_master_put in release function
From: Ludovic Barre This patch adds spi_master_put in release function to drop the controller's refcount. Signed-off-by: Ludovic Barre --- drivers/spi/spi-stm32-qspi.c | 48 +++- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/drivers/spi/spi-stm32-qspi.c b/drivers/spi/spi-stm32-qspi.c index 7879a52..983584d 100644 --- a/drivers/spi/spi-stm32-qspi.c +++ b/drivers/spi/spi-stm32-qspi.c @@ -93,6 +93,7 @@ struct stm32_qspi_flash { struct stm32_qspi { struct device *dev; + struct spi_controller *ctrl; void __iomem *io_base; void __iomem *mm_base; resource_size_t mm_size; @@ -400,6 +401,7 @@ static void stm32_qspi_release(struct stm32_qspi *qspi) writel_relaxed(0, qspi->io_base + QSPI_CR); mutex_destroy(&qspi->lock); clk_disable_unprepare(qspi->clk); + spi_master_put(qspi->ctrl); } static int stm32_qspi_probe(struct platform_device *pdev) @@ -416,43 +418,56 @@ static int stm32_qspi_probe(struct platform_device *pdev) return -ENOMEM; qspi = spi_controller_get_devdata(ctrl); + qspi->ctrl = ctrl; res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qspi"); qspi->io_base = devm_ioremap_resource(dev, res); - if (IS_ERR(qspi->io_base)) - return PTR_ERR(qspi->io_base); + if (IS_ERR(qspi->io_base)) { + ret = PTR_ERR(qspi->io_base); + goto err; + } + + qspi->phys_base = res->start; res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qspi_mm"); qspi->mm_base = devm_ioremap_resource(dev, res); - if (IS_ERR(qspi->mm_base)) - return PTR_ERR(qspi->mm_base); + if (IS_ERR(qspi->mm_base)) { + ret = PTR_ERR(qspi->mm_base); + goto err; + } qspi->mm_size = resource_size(res); - if (qspi->mm_size > STM32_QSPI_MAX_MMAP_SZ) - return -EINVAL; + if (qspi->mm_size > STM32_QSPI_MAX_MMAP_SZ) { + ret = -EINVAL; + goto err; + } irq = platform_get_irq(pdev, 0); ret = devm_request_irq(dev, irq, stm32_qspi_irq, 0, dev_name(dev), qspi); if (ret) { dev_err(dev, "failed to request irq\n"); - return ret; + goto err; } init_completion(&qspi->data_completion); qspi->clk = devm_clk_get(dev, NULL); - if (IS_ERR(qspi->clk)) - return PTR_ERR(qspi->clk); + if (IS_ERR(qspi->clk)) { + ret = PTR_ERR(qspi->clk); + goto err; + } qspi->clk_rate = clk_get_rate(qspi->clk); - if (!qspi->clk_rate) - return -EINVAL; + if (!qspi->clk_rate) { + ret = -EINVAL; + goto err; + } ret = clk_prepare_enable(qspi->clk); if (ret) { dev_err(dev, "can not enable the clock\n"); - return ret; + goto err; } rstc = devm_reset_control_get_exclusive(dev, NULL); @@ -475,14 +490,11 @@ static int stm32_qspi_probe(struct platform_device *pdev) ctrl->dev.of_node = dev->of_node; ret = devm_spi_register_master(dev, ctrl); - if (ret) - goto err_spi_register; - - return 0; + if (!ret) + return 0; -err_spi_register: +err: stm32_qspi_release(qspi); - return ret; } -- 2.7.4
[PATCH 0/2] spi: stm32-qspi: add dma support
From: Ludovic Barre This patch series adds dma support for the stm32-qspi. In read mode, the memory map is preferred vs dma (due to better throughput). If the dma transfer fails the buffer is sent by polling. Ludovic Barre (2): spi: stm32-qspi: add spi_master_put in release function spi: stm32-qspi: add dma support drivers/spi/spi-stm32-qspi.c | 182 ++- 1 file changed, 163 insertions(+), 19 deletions(-) -- 2.7.4
[PATCH 1/2] ARM: dts: stm32: add pinctrl sleep config for qspi on stm32mp157c-ev1
From: Ludovic Barre This patch adds pinctrl sleep config for qspi on stm32mp157c-ev1 Signed-off-by: Ludovic Barre --- arch/arm/boot/dts/stm32mp157-pinctrl.dtsi | 26 ++ arch/arm/boot/dts/stm32mp157c-ev1.dts | 3 ++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi b/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi index 9ec4694..5520f65 100644 --- a/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi +++ b/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi @@ -289,6 +289,12 @@ }; }; + qspi_clk_sleep_pins_a: qspi-clk-sleep-0 { + pins { + pinmux = ; /* QSPI_CLK */ + }; + }; + qspi_bk1_pins_a: qspi-bk1-0 { pins1 { pinmux = , /* QSPI_BK1_IO0 */ @@ -307,6 +313,16 @@ }; }; + qspi_bk1_sleep_pins_a: qspi-bk1-sleep-0 { + pins { + pinmux = , /* QSPI_BK1_IO0 */ +, /* QSPI_BK1_IO1 */ +, /* QSPI_BK1_IO2 */ +, /* QSPI_BK1_IO3 */ +; /* QSPI_BK1_NCS */ + }; + }; + qspi_bk2_pins_a: qspi-bk2-0 { pins1 { pinmux = , /* QSPI_BK2_IO0 */ @@ -325,6 +341,16 @@ }; }; + qspi_bk2_sleep_pins_a: qspi-bk2-sleep-0 { + pins { + pinmux = , /* QSPI_BK2_IO0 */ +, /* QSPI_BK2_IO1 */ +, /* QSPI_BK2_IO2 */ +, /* QSPI_BK2_IO3 */ +; /* QSPI_BK2_NCS */ + }; + }; + uart4_pins_a: uart4-0 { pins1 { pinmux = ; /* UART4_TX */ diff --git a/arch/arm/boot/dts/stm32mp157c-ev1.dts b/arch/arm/boot/dts/stm32mp157c-ev1.dts index b6aca40..7fef155 100644 --- a/arch/arm/boot/dts/stm32mp157c-ev1.dts +++ b/arch/arm/boot/dts/stm32mp157c-ev1.dts @@ -131,8 +131,9 @@ }; &qspi { - pinctrl-names = "default"; + pinctrl-names = "default", "sleep"; pinctrl-0 = <&qspi_clk_pins_a &qspi_bk1_pins_a &qspi_bk2_pins_a>; + pinctrl-1 = <&qspi_clk_sleep_pins_a &qspi_bk1_sleep_pins_a &qspi_bk2_sleep_pins_a>; reg = <0x58003000 0x1000>, <0x7000 0x400>; #address-cells = <1>; #size-cells = <0>; -- 2.7.4
[PATCH 0/2] ARM: dts: stm32: qspi update for stm32mp157c-ev1
From: Ludovic Barre This patch series add sleep pins configuration needed to suspend support and add jedec compatible for 2 nor flash of stm32mp157c-ev1. Ludovic Barre (2): ARM: dts: stm32: add pinctrl sleep config for qspi on stm32mp157c-ev1 ARM: dts: stm32: add jedec compatible for nor flash on stm32mp157c-ev1 arch/arm/boot/dts/stm32mp157-pinctrl.dtsi | 26 ++ arch/arm/boot/dts/stm32mp157c-ev1.dts | 5 - 2 files changed, 30 insertions(+), 1 deletion(-) -- 2.7.4
[PATCH 2/2] ARM: dts: stm32: add jedec compatible for nor flash on stm32mp157c-ev1
From: Ludovic Barre This patch adds jedec compatible for spi-nor flash on stm32mp157c-ev1 (needed with new spi-mem interface). Signed-off-by: Ludovic Barre --- arch/arm/boot/dts/stm32mp157c-ev1.dts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm/boot/dts/stm32mp157c-ev1.dts b/arch/arm/boot/dts/stm32mp157c-ev1.dts index 7fef155..cae88d9 100644 --- a/arch/arm/boot/dts/stm32mp157c-ev1.dts +++ b/arch/arm/boot/dts/stm32mp157c-ev1.dts @@ -140,6 +140,7 @@ status = "okay"; flash0: mx66l51235l@0 { + compatible = "jedec,spi-nor"; reg = <0>; spi-rx-bus-width = <4>; spi-max-frequency = <10800>; @@ -148,6 +149,7 @@ }; flash1: mx66l51235l@1 { + compatible = "jedec,spi-nor"; reg = <1>; spi-rx-bus-width = <4>; spi-max-frequency = <10800>; -- 2.7.4
[PATCH 2/2] spi: spi-mem: stm32-qspi: add suspend/resume support
From: Ludovic Barre This patch adds suspend and resume support for spi-stm32-qspi drivers. Signed-off-by: Ludovic Barre --- drivers/spi/spi-stm32-qspi.c | 39 +++ 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/drivers/spi/spi-stm32-qspi.c b/drivers/spi/spi-stm32-qspi.c index 7354f9d..3e8ca10 100644 --- a/drivers/spi/spi-stm32-qspi.c +++ b/drivers/spi/spi-stm32-qspi.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -101,6 +102,9 @@ struct stm32_qspi { struct completion data_completion; u32 fmode; + u32 cr_reg; + u32 dcr_reg; + /* * to protect device configuration, could be different between * 2 flash access (bk1, bk2) @@ -355,7 +359,7 @@ static int stm32_qspi_setup(struct spi_device *spi) struct spi_controller *ctrl = spi->master; struct stm32_qspi *qspi = spi_controller_get_devdata(ctrl); struct stm32_qspi_flash *flash; - u32 cr, presc; + u32 presc; if (ctrl->busy) return -EBUSY; @@ -371,11 +375,12 @@ static int stm32_qspi_setup(struct spi_device *spi) flash->presc = presc; mutex_lock(&qspi->lock); - cr = FIELD_PREP(CR_FTHRES_MASK, 3) | CR_SSHIFT | CR_EN; - writel_relaxed(cr, qspi->io_base + QSPI_CR); + qspi->cr_reg = FIELD_PREP(CR_FTHRES_MASK, 3) | CR_SSHIFT | CR_EN; + writel_relaxed(qspi->cr_reg, qspi->io_base + QSPI_CR); /* set dcr fsize to max address */ - writel_relaxed(DCR_FSIZE_MASK, qspi->io_base + QSPI_DCR); + qspi->dcr_reg = DCR_FSIZE_MASK; + writel_relaxed(qspi->dcr_reg, qspi->io_base + QSPI_DCR); mutex_unlock(&qspi->lock); return 0; @@ -489,6 +494,31 @@ static int stm32_qspi_remove(struct platform_device *pdev) return 0; } +static int __maybe_unused stm32_qspi_suspend(struct device *dev) +{ + struct stm32_qspi *qspi = dev_get_drvdata(dev); + + clk_disable_unprepare(qspi->clk); + pinctrl_pm_select_sleep_state(dev); + + return 0; +} + +static int __maybe_unused stm32_qspi_resume(struct device *dev) +{ + struct stm32_qspi *qspi = dev_get_drvdata(dev); + + pinctrl_pm_select_default_state(dev); + clk_prepare_enable(qspi->clk); + + writel_relaxed(qspi->cr_reg, qspi->io_base + QSPI_CR); + writel_relaxed(qspi->dcr_reg, qspi->io_base + QSPI_DCR); + + return 0; +} + +SIMPLE_DEV_PM_OPS(stm32_qspi_pm_ops, stm32_qspi_suspend, stm32_qspi_resume); + static const struct of_device_id stm32_qspi_match[] = { {.compatible = "st,stm32f469-qspi"}, {} @@ -501,6 +531,7 @@ static struct platform_driver stm32_qspi_driver = { .driver = { .name = "stm32-qspi", .of_match_table = stm32_qspi_match, + .pm = &stm32_qspi_pm_ops, }, }; module_platform_driver(stm32_qspi_driver); -- 2.7.4
[PATCH 0/2] spi: spi-mem: stm32-qspi: add suspend support and fix
From: Ludovic Barre This patch series adds suspend support and fix a nor memory corruption due to timeout counter issue. Ludovic Barre (2): spi: spi-mem: stm32-qspi: avoid memory corruption at low frequency spi: spi-mem: stm32-qspi: add suspend/resume support drivers/spi/spi-stm32-qspi.c | 41 +++-- 1 file changed, 35 insertions(+), 6 deletions(-) -- 2.7.4
[PATCH 1/2] spi: spi-mem: stm32-qspi: avoid memory corruption at low frequency
From: Ludovic Barre This patch solves a memory corruption seen at 8 MHz. To avoid such issue, timeout counter is disabled. Signed-off-by: Ludovic Barre --- drivers/spi/spi-stm32-qspi.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/spi/spi-stm32-qspi.c b/drivers/spi/spi-stm32-qspi.c index 3b2a9a6..7354f9d 100644 --- a/drivers/spi/spi-stm32-qspi.c +++ b/drivers/spi/spi-stm32-qspi.c @@ -76,7 +76,6 @@ #define QSPI_PSMAR 0x28 #define QSPI_PIR 0x2c #define QSPI_LPTR 0x30 -#define LPTR_DFT_TIMEOUT 0x10 #define STM32_QSPI_MAX_MMAP_SZ SZ_256M #define STM32_QSPI_MAX_NORCHIP 2 @@ -372,8 +371,7 @@ static int stm32_qspi_setup(struct spi_device *spi) flash->presc = presc; mutex_lock(&qspi->lock); - writel_relaxed(LPTR_DFT_TIMEOUT, qspi->io_base + QSPI_LPTR); - cr = FIELD_PREP(CR_FTHRES_MASK, 3) | CR_TCEN | CR_SSHIFT | CR_EN; + cr = FIELD_PREP(CR_FTHRES_MASK, 3) | CR_SSHIFT | CR_EN; writel_relaxed(cr, qspi->io_base + QSPI_CR); /* set dcr fsize to max address */ -- 2.7.4
Re: [PATCH V2 6/6] mmc: mmci: replace blksz_datactrlXX by get_datactrl_cfg callback
hi Russell, Ulf On 3/7/19 5:46 PM, Russell King - ARM Linux admin wrote: On Thu, Mar 07, 2019 at 05:39:02PM +0100, Ludovic Barre wrote: - if (data->flags & MMC_DATA_READ) - datactrl |= MCI_DPSM_DIRECTION; Given that this is currently an invariant between all, it doesn't make sense to have a separate public function and combine it into the get_datactrl_cfg() implementations. You may as well leave it in place here, after you call get_datactrl_cfg(). + datactrl = host->ops->get_datactrl_cfg(host); Otherwise, I don't see a problem with this, although it would be nice to avoid the overhead of so many public functions, which could be done by adding them as inline functions in mmci.h To combine your comments (above and https://lkml.org/lkml/2019/3/6/318). I could regroup mmci_dctrl_dir & mmci_dctrl_ddr & mmci_dctrl_sdio in a common function mmci_dctrl_common and call by: -Each get_datactrl_cfg variant -Or in mmci_start_data What do you prefer ? Regards, Ludo
[PATCH V2 4/6] mmc: mmci: qcom: define get_dctrl_cfg
From: Ludovic Barre This patch defines get_dctrl_cfg callback for qcom variant. qcom variant has a specific block size definition. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci_qcom_dml.c | 13 + 1 file changed, 13 insertions(+) diff --git a/drivers/mmc/host/mmci_qcom_dml.c b/drivers/mmc/host/mmci_qcom_dml.c index ccc1b18..f437ad1 100644 --- a/drivers/mmc/host/mmci_qcom_dml.c +++ b/drivers/mmc/host/mmci_qcom_dml.c @@ -188,9 +188,22 @@ static int qcom_dma_setup(struct mmci_host *host) return 0; } +static u32 qcom_get_dctrl_cfg(struct mmci_host *host) +{ + u32 datactrl; + + datactrl = MCI_DPSM_ENABLE; + datactrl |= (host->data->blksz << 4); + datactrl |= mmci_dctrl_dir(host); + datactrl |= mmci_dctrl_sdio(host) | mmci_dctrl_ddr(host); + + return datactrl; +} + static struct mmci_host_ops qcom_variant_ops = { .prep_data = mmci_dmae_prep_data, .unprep_data = mmci_dmae_unprep_data, + .get_datactrl_cfg = qcom_get_dctrl_cfg, .get_next_data = mmci_dmae_get_next_data, .dma_setup = qcom_dma_setup, .dma_release = mmci_dmae_release, -- 2.7.4
[PATCH V2 3/6] mmc: mmci: define get_dctrl_cfg for legacy variant
From: Ludovic Barre This patch defines get_dctrl_cfg callback for legacy variants whatever DMA_ENGINE configuration. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 44 drivers/mmc/host/mmci.h | 3 +++ 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 28b76c5..52f9dbf 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -46,12 +46,6 @@ #define DRIVER_NAME "mmci-pl18x" -#ifdef CONFIG_DMA_ENGINE -static void mmci_variant_init(struct mmci_host *host); -#else -static inline void mmci_variant_init(struct mmci_host *host) {} -#endif - static unsigned int fmax = 515633; static struct variant_data variant_arm = { @@ -231,7 +225,7 @@ static struct variant_data variant_ux500v2 = { .irq_pio_mask = MCI_IRQ_PIO_MASK, .start_err = MCI_STARTBITERR, .opendrain = MCI_OD, - .init = mmci_variant_init, + .init = mmci_ux500v2_variant_init, }; static struct variant_data variant_stm32 = { @@ -617,6 +611,29 @@ static void mmci_init_sg(struct mmci_host *host, struct mmc_data *data) sg_miter_start(&host->sg_miter, data->sg, data->sg_len, flags); } +u32 mmci_get_dctrl_cfg(struct mmci_host *host) +{ + u32 datactrl; + + datactrl = MCI_DPSM_ENABLE; + datactrl |= mmci_dctrl_blksz(host) | mmci_dctrl_dir(host); + datactrl |= mmci_dctrl_sdio(host) | mmci_dctrl_ddr(host); + + return datactrl; +} + +u32 mmci_ux500v2_get_dctrl_cfg(struct mmci_host *host) +{ + u32 datactrl; + + datactrl = MCI_DPSM_ENABLE; + datactrl |= (host->data->blksz << 16); + datactrl |= mmci_dctrl_dir(host); + datactrl |= mmci_dctrl_sdio(host) | mmci_dctrl_ddr(host); + + return datactrl; +} + /* * All the DMA operation mode stuff goes inside this ifdef. * This assumes that you have a generic DMA device interface, @@ -941,6 +958,7 @@ void mmci_dmae_unprep_data(struct mmci_host *host, static struct mmci_host_ops mmci_variant_ops = { .prep_data = mmci_dmae_prep_data, .unprep_data = mmci_dmae_unprep_data, + .get_datactrl_cfg = mmci_get_dctrl_cfg, .get_next_data = mmci_dmae_get_next_data, .dma_setup = mmci_dmae_setup, .dma_release = mmci_dmae_release, @@ -948,12 +966,22 @@ static struct mmci_host_ops mmci_variant_ops = { .dma_finalize = mmci_dmae_finalize, .dma_error = mmci_dmae_error, }; +#else +static struct mmci_host_ops mmci_variant_ops = { + .get_datactrl_cfg = mmci_get_dctrl_cfg, +} +#endif void mmci_variant_init(struct mmci_host *host) { host->ops = &mmci_variant_ops; } -#endif + +void mmci_ux500v2_variant_init(struct mmci_host *host) +{ + host->ops = &mmci_variant_ops; + host->ops->get_datactrl_cfg = mmci_ux500v2_get_dctrl_cfg; +} static void mmci_pre_request(struct mmc_host *mmc, struct mmc_request *mrq) { diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h index 32ae41d..bfc6e54 100644 --- a/drivers/mmc/host/mmci.h +++ b/drivers/mmc/host/mmci.h @@ -448,6 +448,9 @@ void mmci_dmae_finalize(struct mmci_host *host, struct mmc_data *data); void mmci_dmae_error(struct mmci_host *host); #endif +void mmci_variant_init(struct mmci_host *host); +void mmci_ux500v2_variant_init(struct mmci_host *host); + #ifdef CONFIG_MMC_QCOM_DML void qcom_variant_init(struct mmci_host *host); #else -- 2.7.4
[PATCH V2 2/6] mmc: mmci: add helper functions to define datactrl value for variants
From: Ludovic Barre This patch adds default helper functions to define datactrl value. Each variant could use these helpers to define datactrl and adds their specific field. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 25 + drivers/mmc/host/mmci.h | 5 + 2 files changed, 30 insertions(+) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 9e6a2c1..28b76c5 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -983,6 +983,31 @@ static void mmci_post_request(struct mmc_host *mmc, struct mmc_request *mrq, mmci_unprep_data(host, data, err); } +u32 mmci_dctrl_blksz(struct mmci_host *host) +{ + return (ffs(host->data->blksz) - 1) << 4; +} + +u32 mmci_dctrl_dir(struct mmci_host *host) +{ + return host->data->flags & MMC_DATA_READ ? MCI_DPSM_DIRECTION : 0; +} + +u32 mmci_dctrl_ddr(struct mmci_host *host) +{ + if (host->mmc->ios.timing == MMC_TIMING_UHS_DDR50 || + host->mmc->ios.timing == MMC_TIMING_MMC_DDR52) + return host->variant->datactrl_mask_ddrmode; + return 0; +} + +u32 mmci_dctrl_sdio(struct mmci_host *host) +{ + if (host->mmc->card && mmc_card_sdio(host->mmc->card)) + return host->variant->datactrl_mask_sdio; + return 0; +} + static void mmci_start_data(struct mmci_host *host, struct mmc_data *data) { struct variant_data *variant = host->variant; diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h index a59049b..32ae41d 100644 --- a/drivers/mmc/host/mmci.h +++ b/drivers/mmc/host/mmci.h @@ -430,6 +430,11 @@ struct mmci_host { void mmci_write_clkreg(struct mmci_host *host, u32 clk); void mmci_write_pwrreg(struct mmci_host *host, u32 pwr); +u32 mmci_dctrl_blksz(struct mmci_host *host); +u32 mmci_dctrl_dir(struct mmci_host *host); +u32 mmci_dctrl_ddr(struct mmci_host *host); +u32 mmci_dctrl_sdio(struct mmci_host *host); + #ifdef CONFIG_DMA_ENGINE int mmci_dmae_prep_data(struct mmci_host *host, struct mmc_data *data, bool next); -- 2.7.4
[PATCH V2 6/6] mmc: mmci: replace blksz_datactrlXX by get_datactrl_cfg callback
From: Ludovic Barre This patch allows to get datactrl configuration specific at variant. This introduce more flexibility on datactlr value. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.c | 31 +-- drivers/mmc/host/mmci.h | 7 --- 2 files changed, 1 insertion(+), 37 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 52f9dbf..dcee78f 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -57,7 +57,6 @@ static struct variant_data variant_arm = { .cmdreg_srsp= MCI_CPSM_RESPONSE, .datalength_bits= 16, .datactrl_blocksz = 11, - .datactrl_dpsm_enable = MCI_DPSM_ENABLE, .pwrreg_powerup = MCI_PWR_UP, .f_max = 1, .reversed_irq_handling = true, @@ -77,7 +76,6 @@ static struct variant_data variant_arm_extended_fifo = { .cmdreg_srsp= MCI_CPSM_RESPONSE, .datalength_bits= 16, .datactrl_blocksz = 11, - .datactrl_dpsm_enable = MCI_DPSM_ENABLE, .pwrreg_powerup = MCI_PWR_UP, .f_max = 1, .mmcimask1 = true, @@ -97,7 +95,6 @@ static struct variant_data variant_arm_extended_fifo_hwfc = { .cmdreg_srsp= MCI_CPSM_RESPONSE, .datalength_bits= 16, .datactrl_blocksz = 11, - .datactrl_dpsm_enable = MCI_DPSM_ENABLE, .pwrreg_powerup = MCI_PWR_UP, .f_max = 1, .mmcimask1 = true, @@ -118,7 +115,6 @@ static struct variant_data variant_u300 = { .cmdreg_srsp= MCI_CPSM_RESPONSE, .datalength_bits= 16, .datactrl_blocksz = 11, - .datactrl_dpsm_enable = MCI_DPSM_ENABLE, .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN, .st_sdio= true, .pwrreg_powerup = MCI_PWR_ON, @@ -144,7 +140,6 @@ static struct variant_data variant_nomadik = { .cmdreg_srsp= MCI_CPSM_RESPONSE, .datalength_bits= 24, .datactrl_blocksz = 11, - .datactrl_dpsm_enable = MCI_DPSM_ENABLE, .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN, .st_sdio= true, .st_clkdiv = true, @@ -173,7 +168,6 @@ static struct variant_data variant_ux500 = { .cmdreg_srsp= MCI_CPSM_RESPONSE, .datalength_bits= 24, .datactrl_blocksz = 11, - .datactrl_dpsm_enable = MCI_DPSM_ENABLE, .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN, .st_sdio= true, .st_clkdiv = true, @@ -207,11 +201,9 @@ static struct variant_data variant_ux500v2 = { .datactrl_mask_ddrmode = MCI_DPSM_ST_DDRMODE, .datalength_bits= 24, .datactrl_blocksz = 11, - .datactrl_dpsm_enable = MCI_DPSM_ENABLE, .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN, .st_sdio= true, .st_clkdiv = true, - .blksz_datactrl16 = true, .pwrreg_powerup = MCI_PWR_ON, .f_max = 1, .signal_direction = true, @@ -242,7 +234,6 @@ static struct variant_data variant_stm32 = { .irq_pio_mask = MCI_IRQ_PIO_MASK, .datalength_bits= 24, .datactrl_blocksz = 11, - .datactrl_dpsm_enable = MCI_DPSM_ENABLE, .datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN, .st_sdio= true, .st_clkdiv = true, @@ -286,10 +277,8 @@ static struct variant_data variant_qcom = { .cmdreg_srsp_crc= MCI_CPSM_RESPONSE, .cmdreg_srsp= MCI_CPSM_RESPONSE, .data_cmd_enable= MCI_CPSM_QCOM_DATCMD, - .blksz_datactrl4= true, .datalength_bits= 24, .datactrl_blocksz = 11, - .datactrl_dpsm_enable = MCI_DPSM_ENABLE, .pwrreg_powerup = MCI_PWR_UP, .f_max = 20800, .explicit_mclk_control = true, @@ -1042,7 +1031,6 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data) unsigned int datactrl, timeout, irqmask; unsigned long long clks; void __iomem *base; - int blksz_bits; dev_dbg(mmc_dev(host->mmc), "blksz %04x blks %04x flags %08x\n", data->blksz, data->blocks, data->flags); @@ -1060,24 +1048,11 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data) writel(timeout, base + MMCIDATATIMER); writel(host->size, base + MMCIDATALENGTH); - blksz_bits = ffs(data->blksz) - 1; - BUG_ON(1 << blksz_bits != data->blksz); - - if (variant->blksz_datactrl16) -
[PATCH V2 5/6] mmc: mmci: stm32: define get_dctrl_cfg
From: Ludovic Barre This patch defines get_dctrl_cfg callback for sdmmc variant. sdmmc variant has specific stm32 transfer modes. sdmmc data transfer mode selection could be: -Block data transfer ending on block count. -SDIO multibyte data transfer. -MMC Stream data transfer (not used). -Block data transfer ending with STOP_TRANSMISSION command. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.h | 5 + drivers/mmc/host/mmci_stm32_sdmmc.c | 19 +++ 2 files changed, 24 insertions(+) diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h index bfc6e54..4d33c4f 100644 --- a/drivers/mmc/host/mmci.h +++ b/drivers/mmc/host/mmci.h @@ -131,6 +131,11 @@ /* Control register extensions in the Qualcomm versions */ #define MCI_DPSM_QCOM_DATA_PENDBIT(17) #define MCI_DPSM_QCOM_RX_DATA_PEND BIT(20) +/* Control register extensions in STM32 versions */ +#define MCI_DPSM_STM32_MODE_BLOCK (0 << 2) +#define MCI_DPSM_STM32_MODE_SDIO (1 << 2) +#define MCI_DPSM_STM32_MODE_STREAM (2 << 2) +#define MCI_DPSM_STM32_MODE_BLOCK_STOP (3 << 2) #define MMCIDATACNT0x030 #define MMCISTATUS 0x034 diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c index cfbfc6f..4c710e1 100644 --- a/drivers/mmc/host/mmci_stm32_sdmmc.c +++ b/drivers/mmc/host/mmci_stm32_sdmmc.c @@ -265,10 +265,29 @@ static void mmci_sdmmc_set_pwrreg(struct mmci_host *host, unsigned int pwr) } } +static u32 sdmmc_get_dctrl_cfg(struct mmci_host *host) +{ + u32 datactrl; + + datactrl = mmci_dctrl_blksz(host) | mmci_dctrl_dir(host); + datactrl |= mmci_dctrl_sdio(host) | mmci_dctrl_ddr(host); + + if (host->mmc->card && mmc_card_sdio(host->mmc->card) && + host->data->blocks == 1) + datactrl |= MCI_DPSM_STM32_MODE_SDIO; + else if (host->data->stop && !host->mrq->sbc) + datactrl |= MCI_DPSM_STM32_MODE_BLOCK_STOP; + else + datactrl |= MCI_DPSM_STM32_MODE_BLOCK; + + return datactrl; +} + static struct mmci_host_ops sdmmc_variant_ops = { .validate_data = sdmmc_idma_validate_data, .prep_data = sdmmc_idma_prep_data, .unprep_data = sdmmc_idma_unprep_data, + .get_datactrl_cfg = sdmmc_get_dctrl_cfg, .dma_setup = sdmmc_idma_setup, .dma_start = sdmmc_idma_start, .dma_finalize = sdmmc_idma_finalize, -- 2.7.4
[PATCH V2 0/6] mmc: mmci: add get_datactrl_cfg callback
From: Ludovic Barre This patch series adds get_datactrl_cfg callback in mmci_host_ops to allow to get datactrl configuration specific at variant. change V2: -This V2 has been rebased on "mmc: mmci: Cleanup some variant related code" series -add helpers functions to define default datactrl value, each variant could use these helpers to define datactrl and adds their specific bits. -use static in qcom and stm32 -regroup mmci_(ux500v2)variant_init in header file to avoid checkpatch warning: "WARNING: externs should be avoided in .c files" -remove unused variant properties: "datactrl_dpsm_enable" "blksz_datactrl16" "blksz_datactrl4" Ludovic Barre (6): mmc: mmci: add get_datactrl_cfg callback mmc: mmci: add helper functions to define datactrl value for variants mmc: mmci: define get_dctrl_cfg for legacy variant mmc: mmci: qcom: define get_dctrl_cfg mmc: mmci: stm32: define get_dctrl_cfg mmc: mmci: replace blksz_datactrlXX by get_datactrl_cfg callback drivers/mmc/host/mmci.c | 100 ++-- drivers/mmc/host/mmci.h | 21 +--- drivers/mmc/host/mmci_qcom_dml.c| 13 + drivers/mmc/host/mmci_stm32_sdmmc.c | 19 +++ 4 files changed, 108 insertions(+), 45 deletions(-) -- 2.7.4
[PATCH V2 1/6] mmc: mmci: add get_datactrl_cfg callback
From: Ludovic Barre This patch adds get_datactrl_cfg callback in mmci_host_ops to allow to get datactrl configuration specific at variant. Signed-off-by: Ludovic Barre --- drivers/mmc/host/mmci.h | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h index 6bde28c..a59049b 100644 --- a/drivers/mmc/host/mmci.h +++ b/drivers/mmc/host/mmci.h @@ -362,6 +362,7 @@ struct mmci_host_ops { bool next); void (*unprep_data)(struct mmci_host *host, struct mmc_data *data, int err); + u32 (*get_datactrl_cfg)(struct mmci_host *host); void (*get_next_data)(struct mmci_host *host, struct mmc_data *data); int (*dma_setup)(struct mmci_host *host); void (*dma_release)(struct mmci_host *host); -- 2.7.4