Coding style fixup and rebase of v3, and resubmit of the Kconfig patch
that got dropped from v2. No other changes.
Thanks for your continued attention and reviews!
George
The module was initializing completions whenever it was going to wait on
them, and not when the completion was allocated. This is incorrect
according to the completion docs:
Calling init_completion() on the same completion object twice is
most likely a bug [...]
Re-initialization is also unnecessary because the module never uses
complete_all(). Fix this by only ever initializing the completion a
single time, and log if the completions are not consumed as intended
(this is not a fatal problem, but should not go unnoticed).
Signed-off-by: George Hilliard <[email protected]>
---
v2: rewrite of v1
v3: Remove BUG_ON() calls
v4: Indent style fixup
drivers/staging/mt7621-mmc/sd.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c
index e346167754bd..ed63bd3ba6cc 100644
--- a/drivers/staging/mt7621-mmc/sd.c
+++ b/drivers/staging/mt7621-mmc/sd.c
@@ -466,7 +466,11 @@ static unsigned int msdc_command_start(struct msdc_host *host,
host->cmd = cmd;
host->cmd_rsp = resp;
- init_completion(&host->cmd_done);
+ // The completion should have been consumed by the previous command
+ // response handler, because the mmc requests should be serialized
+ if(completion_done(&host->cmd_done))
+ dev_err(mmc_dev(host->mmc),
+ "previous command was not handled\n");
sdr_set_bits(host->base + MSDC_INTEN, wints);
sdc_send_cmd(rawcmd, cmd->arg);
@@ -488,7 +492,6 @@ static unsigned int msdc_command_resp(struct msdc_host *host,
MSDC_INT_ACMD19_DONE;
BUG_ON(in_interrupt());
- //init_completion(&host->cmd_done);
//sdr_set_bits(host->base + MSDC_INTEN, wints);
spin_unlock(&host->lock);
@@ -670,7 +673,13 @@ static int msdc_do_request(struct mmc_host *mmc, struct mmc_request *mrq)
//msdc_clr_fifo(host); /* no need */
msdc_dma_on(); /* enable DMA mode first!! */
- init_completion(&host->xfer_done);
+
+ // The completion should have been consumed by the previous
+ // xfer response handler, because the mmc requests should be
+ // serialized
+ if(completion_done(&host->cmd_done))
+ dev_err(mmc_dev(host->mmc),
+ "previous transfer was not handled\n");
/* start the command first*/
if (msdc_command_start(host, cmd, CMD_TIMEOUT) != 0)
@@ -696,7 +705,6 @@ static int msdc_do_request(struct mmc_host *mmc, struct mmc_request *mrq)
/* for read, the data coming too fast, then CRC error
* start DMA no business with CRC.
*/
- //init_completion(&host->xfer_done);
msdc_dma_start(host);
spin_unlock(&host->lock);
@@ -1687,6 +1695,8 @@ static int msdc_drv_probe(struct platform_device *pdev)
}
msdc_init_gpd_bd(host, &host->dma);
+ init_completion(&host->cmd_done);
+ init_completion(&host->xfer_done);
INIT_DELAYED_WORK(&host->card_delaywork, msdc_tasklet_card);
spin_lock_init(&host->lock);
msdc_init_hw(host);
--
2.21.0
These values are not referred to anywhere else in the kernel. Card
detect is controlled by the device tree property "mediatek,cd-poll",
and there is no driver support for eMMC whatsoever.
Signed-off-by: George Hilliard <[email protected]>
---
v2: Rewrite of v1
v3: [Not present]
v4: Resubmit of v2
drivers/staging/mt7621-mmc/Kconfig | 8 --------
1 file changed, 8 deletions(-)
diff --git a/drivers/staging/mt7621-mmc/Kconfig b/drivers/staging/mt7621-mmc/Kconfig
index 1eb79cd6e22f..01f231dd8511 100644
--- a/drivers/staging/mt7621-mmc/Kconfig
+++ b/drivers/staging/mt7621-mmc/Kconfig
@@ -6,11 +6,3 @@ config MTK_AEE_KDUMP
bool "MTK AEE KDUMP"
depends on MTK_MMC
-config MTK_MMC_CD_POLL
- bool "Card Detect with Polling"
- depends on MTK_MMC
-
-config MTK_MMC_EMMC_8BIT
- bool "eMMC 8-bit support"
- depends on MTK_MMC && RALINK_MT7628
-
--
2.21.0
On Tue, Mar 26, 2019 at 09:21:39AM -0600, George Hilliard wrote:
> The module was initializing completions whenever it was going to wait on
> them, and not when the completion was allocated. This is incorrect
> according to the completion docs:
>
> Calling init_completion() on the same completion object twice is
> most likely a bug [...]
>
> Re-initialization is also unnecessary because the module never uses
> complete_all(). Fix this by only ever initializing the completion a
> single time, and log if the completions are not consumed as intended
> (this is not a fatal problem, but should not go unnoticed).
>
> Signed-off-by: George Hilliard <[email protected]>
> ---
> v2: rewrite of v1
> v3: Remove BUG_ON() calls
> v4: Indent style fixup
>
> drivers/staging/mt7621-mmc/sd.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c
> index e346167754bd..ed63bd3ba6cc 100644
> --- a/drivers/staging/mt7621-mmc/sd.c
> +++ b/drivers/staging/mt7621-mmc/sd.c
> @@ -466,7 +466,11 @@ static unsigned int msdc_command_start(struct msdc_host *host,
> host->cmd = cmd;
> host->cmd_rsp = resp;
>
> - init_completion(&host->cmd_done);
> + // The completion should have been consumed by the previous command
> + // response handler, because the mmc requests should be serialized
> + if(completion_done(&host->cmd_done))
Did you run your patch through checkpatch.pl? It should have reported
an error here :(
On Tue, Mar 26, 2019 at 6:46 PM Greg Kroah-Hartman
<[email protected]> wrote:
>
> On Tue, Mar 26, 2019 at 09:21:39AM -0600, George Hilliard wrote:
> > - init_completion(&host->cmd_done);
> > + // The completion should have been consumed by the previous command
> > + // response handler, because the mmc requests should be serialized
> > + if(completion_done(&host->cmd_done))
>
> Did you run your patch through checkpatch.pl? It should have reported
> an error here :(
No, I didn't. My mistake, sorry. Will fix up and resend again.
George