Hello
This patchset was inspired questions from 李想 of Allwinner and incorporates as
well suggestions from Hans related to spin locks.
For example have not all attributes of the shared host object been protected
by the spin lock, this has been changed.
Also some register names and reset behavior have been fixed now.
And a minor cleanup happened at the clock frequecy setting, since there has
been a piece of left over code which has been there because it was in the Android
driver and we just adapted it because we didn't have proper documentation about
the timing of the module.
Looking forward to your feedback
Cheers David
---
David Lanzendörfer (4):
mmc: sunxi: Lock fix
mmc: sunxi: Correcting SDXC_HARDWARE_RESET bit
mmc: sunxi: Reset behavior fix
mmc: sunxi: Removing unused code
drivers/mmc/host/sunxi-mmc.c | 37 +++++++++++++++++++++++++------------
1 file changed, 25 insertions(+), 12 deletions(-)
--
Signature
1) Adding a comment in order to clarify the choice of the locks within
sunxi_mmc_handle_manual_stop
2) As 李想 has pointed out the wait_dma variable was not accessed within the spin lock
block in sunxi_mmc_request and so (even if it should never happend) it would have
theoretically been possible that some other function would access the variable
at the same time as the function.
This has been changed now and the function is using local variables outside the lock
and copys the value over during the lock phase.
Signed-off-by: David Lanzendörfer <[email protected]>
Reported-by: 李想 <[email protected]>
---
drivers/mmc/host/sunxi-mmc.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index 15cb8b7..25ba321 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -570,6 +570,15 @@ static irqreturn_t sunxi_mmc_handle_manual_stop(int irq, void *dev_id)
}
dev_err(mmc_dev(host->mmc), "data error, sending stop command\n");
+
+ /*
+ * We will never have more than one outstanding request,
+ * and we do not complete the request until after
+ * we've cleared host->manual_stop_mrq so we do not need to
+ * spin lock this function.
+ * Additionally we have wait states within this function
+ * so having it in a lock is a very bad idea.
+ */
sunxi_mmc_send_manual_stop(host, mrq);
spin_lock_irqsave(&host->lock, iflags);
@@ -766,6 +775,7 @@ static void sunxi_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
unsigned long iflags;
u32 imask = SDXC_INTERRUPT_ERROR_BIT;
u32 cmd_val = SDXC_START | (cmd->opcode & 0x3f);
+ bool wait_dma = host->wait_dma;
int ret;
/* Check for set_ios errors (should never happen) */
@@ -816,7 +826,7 @@ static void sunxi_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
if (cmd->data->flags & MMC_DATA_WRITE)
cmd_val |= SDXC_WRITE;
else
- host->wait_dma = true;
+ wait_dma = true;
} else {
imask |= SDXC_COMMAND_DONE;
}
@@ -850,6 +860,7 @@ static void sunxi_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
}
host->mrq = mrq;
+ host->wait_dma = wait_dma;
mmc_writel(host, REG_IMASK, host->sdio_imask | imask);
mmc_writel(host, REG_CARG, cmd->arg);
mmc_writel(host, REG_CMDR, cmd_val);
When there is only one DES available the DMA performs a FIFO reset and waits until the reinitialization has been completed.
Disabling the SDXC_IDMAC_DES0_DIC bit prevents the DMA from sending an interrupt after it has finished this reinitialization.
The flags SDXC_IDMAC_DES0_FD and SDXC_IDMAC_DES0_LD are both required in order to use the controller with more than one DES.
Signed-off-by: David Lanzendörfer <[email protected]>
Reported-by: 李想 <[email protected]>
---
drivers/mmc/host/sunxi-mmc.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index 50bd3d2..5331c88 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -310,7 +310,17 @@ static void sunxi_mmc_init_idma_des(struct sunxi_mmc_host *host,
}
pdes[0].config |= SDXC_IDMAC_DES0_FD;
- pdes[i - 1].config = SDXC_IDMAC_DES0_OWN | SDXC_IDMAC_DES0_LD;
+
+ /*
+ * When there is only one DES available the DMA performs a FIFO reset and waits
+ * until the reinitialization has been completed.
+ * Disabling the SDXC_IDMAC_DES0_DIC bit prevents the DMA from sending an interrupt
+ * after it has finished this reinitialization.
+ */
+ pdes[i - 1].config = SDXC_IDMAC_DES0_OWN;
+ pdes[i - 1].config |= SDXC_IDMAC_DES0_FD;
+ pdes[i - 1].config |= SDXC_IDMAC_DES0_LD;
+ pdes[i - 1].config &= ~SDXC_IDMAC_DES0_DIC;
/*
* Avoid the io-store starting the idmac hitting io-mem before the
Removing a relict from reverse engineering of the Android driver code in
sunxi_mmc_clk_set_rate.
Signed-off-by: David Lanzendörfer <[email protected]>
Reported-by: 李想 <[email protected]>
---
drivers/mmc/host/sunxi-mmc.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index 5331c88..f73a569 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -635,7 +635,7 @@ static int sunxi_mmc_oclk_onoff(struct sunxi_mmc_host *host, u32 oclk_en)
static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host,
struct mmc_ios *ios)
{
- u32 rate, oclk_dly, rval, sclk_dly, src_clk;
+ u32 rate, oclk_dly, rval, sclk_dly;
int ret;
rate = clk_round_rate(host->clk_mmc, ios->clock);
@@ -680,14 +680,6 @@ static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host,
sclk_dly = 4;
}
- src_clk = clk_get_rate(clk_get_parent(host->clk_mmc));
- if (src_clk >= 300000000 && src_clk <= 400000000) {
- if (oclk_dly)
- oclk_dly--;
- if (sclk_dly)
- sclk_dly--;
- }
-
clk_sunxi_mmc_phase_control(host->clk_mmc, sclk_dly, oclk_dly);
return sunxi_mmc_oclk_onoff(host, 1);
Fixing the register name in sunxi_mmc_reset_host since the SDXC_HARDWARE_RESET bit
is actually located within REG_GCTRL and not REG_CMDR as it was pointed out by Allwinner.
Signed-off-by: David Lanzendörfer <[email protected]>
Reported-by: 李想 <[email protected]>
---
drivers/mmc/host/sunxi-mmc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index 25ba321..50bd3d2 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -252,7 +252,7 @@ static int sunxi_mmc_reset_host(struct sunxi_mmc_host *host)
unsigned long expire = jiffies + msecs_to_jiffies(250);
u32 rval;
- mmc_writel(host, REG_CMDR, SDXC_HARDWARE_RESET);
+ mmc_writel(host, REG_GCTRL, SDXC_HARDWARE_RESET);
do {
rval = mmc_readl(host, REG_GCTRL);
} while (time_before(jiffies, expire) && (rval & SDXC_HARDWARE_RESET));
Hi,
On 16-12-14 01:37, David Lanzendörfer wrote:
> When there is only one DES available the DMA performs a FIFO reset and waits until the reinitialization has been completed.
> Disabling the SDXC_IDMAC_DES0_DIC bit prevents the DMA from sending an interrupt after it has finished this reinitialization.
>
> The flags SDXC_IDMAC_DES0_FD and SDXC_IDMAC_DES0_LD are both required in order to use the controller with more than one DES.
>
> Signed-off-by: David Lanzendörfer <[email protected]>
> Reported-by: 李想 <[email protected]>
> ---
> drivers/mmc/host/sunxi-mmc.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
> index 50bd3d2..5331c88 100644
> --- a/drivers/mmc/host/sunxi-mmc.c
> +++ b/drivers/mmc/host/sunxi-mmc.c
> @@ -310,7 +310,17 @@ static void sunxi_mmc_init_idma_des(struct sunxi_mmc_host *host,
> }
>
> pdes[0].config |= SDXC_IDMAC_DES0_FD;
> - pdes[i - 1].config = SDXC_IDMAC_DES0_OWN | SDXC_IDMAC_DES0_LD;
> +
> + /*
> + * When there is only one DES available the DMA performs a FIFO reset and waits
> + * until the reinitialization has been completed.
> + * Disabling the SDXC_IDMAC_DES0_DIC bit prevents the DMA from sending an interrupt
> + * after it has finished this reinitialization.
> + */
> + pdes[i - 1].config = SDXC_IDMAC_DES0_OWN;
> + pdes[i - 1].config |= SDXC_IDMAC_DES0_FD;
> + pdes[i - 1].config |= SDXC_IDMAC_DES0_LD;
> + pdes[i - 1].config &= ~SDXC_IDMAC_DES0_DIC;
>
> /*
> * Avoid the io-store starting the idmac hitting io-mem before the
This is wrong. For one you are starting with an assignment, so you could just as well
set all the flags you want with a single line / assignment. Besides that you're setting
the FD flag, which should only be set for the first descriptor of a transfer, so for
multi descriptor transfers this is wrong.
Please see the patch which I send you which gets this right.
Regards,
Hans
Hello folks
Is anyone of you at the Chaos Communication Congress?
I'd like to organize a table for sunxi-linux-project
All the best
David
Hi,
On 28-12-14 20:32, David Lanzendörfer wrote:
> Hello folks
> Is anyone of you at the Chaos Communication Congress?
> I'd like to organize a table for sunxi-linux-project
I'm not attending 31c3, so count me out.
Regards,
Hans