2014-12-16 14:10:59

by David Lanzendörfer

[permalink] [raw]
Subject: [PATCH 0/4] mmc: sunxi: General fixup (v2)

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 (3):
mmc: sunxi: Lock fix
mmc: sunxi: Correcting SDXC_HARDWARE_RESET bit
mmc: sunxi: Removing unused code

Hans de Goede (1):
mmc: sunxi: Fix setup of last descriptor of dma transfer


drivers/mmc/host/sunxi-mmc.c | 29 +++++++++++++++++------------
1 file changed, 17 insertions(+), 12 deletions(-)

--
Signature


2014-12-16 14:11:06

by David Lanzendörfer

[permalink] [raw]
Subject: [PATCH 1/4] mmc: sunxi: Fix setup of last descriptor of dma transfer

From: Hans de Goede <[email protected]>

The last descriptor might be the first descriptor as well, so use masking to
add the LD (last descriptor) bit and drop the DIC (disable interrupt on
completion) bit rather then hard assignment as hard assigment will override
the FD (first descriptor) bit if there is only 1 descriptor.

Also set the ER (end of ring) bit and clear buf_addr_ptr2 on the last
descriptor, like the android kernel code does.

Signed-off-by: Hans de Goede <[email protected]>
Signed-off-by: David Lanzendörfer <[email protected]>
Reported-by: 李想 <[email protected]>
---
drivers/mmc/host/sunxi-mmc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index 15cb8b7..1fe54a8 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -310,7 +310,9 @@ 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;
+ pdes[i - 1].config |= SDXC_IDMAC_DES0_LD | SDXC_IDMAC_DES0_ER;
+ pdes[i - 1].config &= ~SDXC_IDMAC_DES0_DIC;
+ pdes[i - 1].buf_addr_ptr2 = 0;

/*
* Avoid the io-store starting the idmac hitting io-mem before the

2014-12-16 14:11:09

by David Lanzendörfer

[permalink] [raw]
Subject: [PATCH 2/4] mmc: sunxi: Lock fix

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 1fe54a8..67e680c 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -572,6 +572,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);
@@ -768,6 +777,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) */
@@ -818,7 +828,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;
}
@@ -852,6 +862,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);

2014-12-16 14:11:20

by David Lanzendörfer

[permalink] [raw]
Subject: [PATCH 4/4] mmc: sunxi: Removing unused code

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 695fe85..6af0a28 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -627,7 +627,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);
@@ -672,14 +672,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);

2014-12-16 14:11:15

by David Lanzendörfer

[permalink] [raw]
Subject: [PATCH 3/4] mmc: sunxi: Correcting SDXC_HARDWARE_RESET bit

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 67e680c..695fe85 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));

2014-12-16 14:13:43

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH 0/4] mmc: sunxi: General fixup (v2)

Hi,

On 16-12-14 15:10, David Lanzendörfer wrote:
> 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

Thanks.

Series looks good and is:

Acked-by: Hans de Goede <[email protected]>

Regards,

Hans

2014-12-19 12:32:41

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH 0/4] mmc: sunxi: General fixup (v2)

On 16 December 2014 at 15:10, David Lanzendörfer
<[email protected]> wrote:
> 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

Thanks! Queued for 3.20.

Kind regards
Uffe

>
> ---
>
> David Lanzendörfer (3):
> mmc: sunxi: Lock fix
> mmc: sunxi: Correcting SDXC_HARDWARE_RESET bit
> mmc: sunxi: Removing unused code
>
> Hans de Goede (1):
> mmc: sunxi: Fix setup of last descriptor of dma transfer
>
>
> drivers/mmc/host/sunxi-mmc.c | 29 +++++++++++++++++------------
> 1 file changed, 17 insertions(+), 12 deletions(-)
>
> --
> Signature