[PATCH 00/13] mmc: bcm2835: more cleanups

[PATCH 00/13] mmc: bcm2835: more cleanups

[Gerd Hoffmann]

  Hi folks,

Continuing bcm283x mmc driver cleanup.  Changes:

 * switch to threaded irq handler.
 * handle timeouts using delayed work instead of timers.

With these changes most of the driver code runs in
thread context, which in turn allows to remove the
work queue and the tasklet.  Also some wait loops can
be simplified because almost all driver code is allowed
to sleep now.

Code is also available here:
  git://git.kraxel.org/linux bcm2837-sdhost-cleanup

Baseline for the patch series is this:
  git://git.kraxel.org/linux bcm2837-wifi

Tested on raspberry pi 2b only so far.
PIO mode only, didn't try to enable DMA.

Lots of baby patches for now, to make review
and (if needed) bisecting easier.  Will all be
squashed into the bcm283x mmc driver patch later.

cheers,
  Gerd

Gerd Hoffmann (13):
  mmc: bcm2835: add bcm2835_read_wait_sdcmd
  mmc: bcm2835: use bcm2835_read_wait_sdcmd in bcm2835_send_command
  mmc: bcm2835: add bcm2835_threaded_irq
  mmc: bcm2835: add bcm2835_check_data_error
  mmc: bcm2835: call bcm2835_block_irq from irqthread
  mmc: bcm2835: add bcm2835_check_cmd_error
  mmc: bcm2835: call bcm2835_busy_irq from irqthread
  mmc: bcm2835: split bcm2835_data_irq
  mmc: bcm2835: switch locking to mutex
  mmc: bcm2835: work queue is dead code now, zap
  mmc: bcm2835: kill tasklet
  mmc: bcm2835: move timeout to thread context
  mmc: bcm2835: use bcm2835_read_wait_sdcmd in bcm2835_finish_command

 drivers/mmc/host/bcm2835.c | 448 ++++++++++++++++++++-------------------------
 1 file changed, 197 insertions(+), 251 deletions(-)

-- 
1.8.3.1

[PATCH 01/13] mmc: bcm2835: add bcm2835_read_wait_sdcmd

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/mmc/host/bcm2835.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/mmc/host/bcm2835.c b/drivers/mmc/host/bcm2835.c
index 9744517..ceee4cf 100644
--- a/drivers/mmc/host/bcm2835.c
+++ b/drivers/mmc/host/bcm2835.c
@@ -609,6 +609,36 @@ static void bcm2835_prepare_data(struct bcm2835_host *host,
 	writel(data->blocks, host->ioaddr + SDHBLC);
 }
 
+static u32 bcm2835_read_wait_sdcmd(struct bcm2835_host *host, u32 timeout,
+				   bool check_fail)
+{
+	struct device *dev = &host->pdev->dev;
+	unsigned long start = jiffies;
+	unsigned long fastpoll = start + usecs_to_jiffies(10);
+	unsigned long end = start + msecs_to_jiffies(timeout);
+	u32 value;
+
+	for (;;) {
+		value = readl(host->ioaddr + SDCMD);
+		if (!(value & SDCMD_NEW_FLAG))
+			break;
+		if (check_fail && (value & SDCMD_FAIL_FLAG))
+			break;
+		if (time_after(jiffies, end)) {
+			dev_err(dev, "%s: timeout (%d us)\n",
+				__func__, timeout);
+			break;
+		}
+
+		/* if it takes longer reduce poll interval */
+		if (time_after(jiffies, fastpoll))
+			udelay(10);
+		else
+			cpu_relax();
+	}
+	return value;
+}
+
 bool bcm2835_send_command(struct bcm2835_host *host,
 			  struct mmc_command *cmd)
 {
-- 
1.8.3.1

Re: [PATCH 01/13] mmc: bcm2835: add bcm2835_read_wait_sdcmd

[Florian Fainelli]

On 01/26/2017 03:37 PM, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Few things with your submission:

- if you send more than one patch, a cover letter is a welcome to
explain your changes

- try to provide commit messages for things that are not obvious (in
fact, most maintainers actually request commit messages even for trivial
things)

- have not we switched to the iproc SDHCI driver, or is this for the
other SDIO connected peripherals here?

Thanks
-- 
Florian

Re: [PATCH 01/13] mmc: bcm2835: add bcm2835_read_wait_sdcmd

[Gerd Hoffmann]

On Do, 2017-01-26 at 15:51 -0800, Florian Fainelli wrote:
> On 01/26/2017 03:37 PM, Gerd Hoffmann wrote:
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> 
> Few things with your submission:
> 
> - if you send more than one patch, a cover letter is a welcome to
> explain your changes

There actually is a cover letter.  The git-send-email "cccmd =
scripts/get_maintainer.pl" hook doesn't work for the cover letter though
because it isn't a patch.  I've hand-picked some people who
touched/reviewed the driver before to the git-send-email command line,
but everybody else got the patches only.

Anyone has a good solution for this btw?

> - try to provide commit messages for things that are not obvious (in
> fact, most maintainers actually request commit messages even for trivial
> things)

The driver is out of tree still, these are cleanups Ulf Hansson asked
for last time the driver was submitted.

Full set of patches:
  https://www.kraxel.org/cgit/linux/log/?h=bcm2837-sdhost-cleanup

Plan for upstream merge is to actually squash all the incremental
cleanups, but for now I kept them as separate changes.  I think this
makes it easier to spot mistakes and it'll also help in case there are
regressions b/c you can bisect things.

> - have not we switched to the iproc SDHCI driver, or is this for the
> other SDIO connected peripherals here?

The 2835 has two SD controllers: The Arasan SDHCI controller that we
currently use (iproc drives that one), and a custom SD controller.

This is a driver for the custom one.  Has better performance than the
sdhci controller (according to Eric Anholt).  For the rpi3 switching to
this driver has the additional advantage that we can use the iproc
driver driver to handle the wifi.

cheers,
  Gerd

Re: [PATCH 01/13] mmc: bcm2835: add bcm2835_read_wait_sdcmd

[Florian Fainelli]

On 01/27/2017 12:04 AM, Gerd Hoffmann wrote:
> On Do, 2017-01-26 at 15:51 -0800, Florian Fainelli wrote:
>> On 01/26/2017 03:37 PM, Gerd Hoffmann wrote:
>>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>>
>> Few things with your submission:
>>
>> - if you send more than one patch, a cover letter is a welcome to
>> explain your changes
> 
> There actually is a cover letter.  The git-send-email "cccmd =
> scripts/get_maintainer.pl" hook doesn't work for the cover letter though
> because it isn't a patch.  I've hand-picked some people who
> touched/reviewed the driver before to the git-send-email command line,
> but everybody else got the patches only.
> 
> Anyone has a good solution for this btw?

Do a dry run against all patches, collect the cclist in a file, and have
a script that outputs one line, something like this:

./scripts/get_maintainer.pl patches/*.patch > cclist

cclist.sh:

#!/bin/sh
cat $(dirname $0)/cclist

git send-email --cc-cmd=cclist.sh patches/*.patch

> 
>> - try to provide commit messages for things that are not obvious (in
>> fact, most maintainers actually request commit messages even for trivial
>> things)
> 
> The driver is out of tree still, these are cleanups Ulf Hansson asked
> for last time the driver was submitted.
> 
> Full set of patches:
>   https://www.kraxel.org/cgit/linux/log/?h=bcm2837-sdhost-cleanup
> 
> Plan for upstream merge is to actually squash all the incremental
> cleanups, but for now I kept them as separate changes.  I think this
> makes it easier to spot mistakes and it'll also help in case there are
> regressions b/c you can bisect things.
> 
>> - have not we switched to the iproc SDHCI driver, or is this for the
>> other SDIO connected peripherals here?
> 
> The 2835 has two SD controllers: The Arasan SDHCI controller that we
> currently use (iproc drives that one), and a custom SD controller.
> 
> This is a driver for the custom one.  Has better performance than the
> sdhci controller (according to Eric Anholt).  For the rpi3 switching to
> this driver has the additional advantage that we can use the iproc
> driver driver to handle the wifi.

OK
-- 
Florian

Re: [PATCH 01/13] mmc: bcm2835: add bcm2835_read_wait_sdcmd

[Jeremy McNicoll]

> On Jan 27, 2017, at 10:23 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> 
> On 01/27/2017 12:04 AM, Gerd Hoffmann wrote:
>> On Do, 2017-01-26 at 15:51 -0800, Florian Fainelli wrote:
>>> On 01/26/2017 03:37 PM, Gerd Hoffmann wrote:
>>>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>>> 
>>> Few things with your submission:
>>> 
>>> - if you send more than one patch, a cover letter is a welcome to
>>> explain your changes
>> 
>> There actually is a cover letter.  The git-send-email "cccmd =
>> scripts/get_maintainer.pl" hook doesn't work for the cover letter though
>> because it isn't a patch.  I've hand-picked some people who
>> touched/reviewed the driver before to the git-send-email command line,
>> but everybody else got the patches only.
>> 
>> Anyone has a good solution for this btw?
> 
> Do a dry run against all patches, collect the cclist in a file, and have
> a script that outputs one line, something like this:
> 
> ./scripts/get_maintainer.pl patches/*.patch > cclist
> 
> cclist.sh:
> 
> #!/bin/sh
> cat $(dirname $0)/cclist
> 
> git send-email --cc-cmd=cclist.sh patches/*.patch


How is the cover letter being created?  

$ git send-email -10 —-compose  

drops you into your editor to write the cover letter / lead in. 

What about this?

$ git format-patch -o /tmp/blah/ --cover-letter --subject-prefix="[V2]" HEAD~10

Your first patch 0000.patch will be your cover letter to fill in. 

Add this to your ~/.gitconfig
   [sendmail] 
       ….
       confirm = always

$ git config --global sendemail.confirm always


As well there is a ‘—-dry-run’ option to git send-email. 

HTH

Its always good to cut twice and measure once ;-)

-jeremy



> 
>> 
>>> - try to provide commit messages for things that are not obvious (in
>>> fact, most maintainers actually request commit messages even for trivial
>>> things)
>> 
>> The driver is out of tree still, these are cleanups Ulf Hansson asked
>> for last time the driver was submitted.
>> 
>> Full set of patches:
>>  https://www.kraxel.org/cgit/linux/log/?h=bcm2837-sdhost-cleanup
>> 
>> Plan for upstream merge is to actually squash all the incremental
>> cleanups, but for now I kept them as separate changes.  I think this
>> makes it easier to spot mistakes and it'll also help in case there are
>> regressions b/c you can bisect things.
>> 
>>> - have not we switched to the iproc SDHCI driver, or is this for the
>>> other SDIO connected peripherals here?
>> 
>> The 2835 has two SD controllers: The Arasan SDHCI controller that we
>> currently use (iproc drives that one), and a custom SD controller.
>> 
>> This is a driver for the custom one.  Has better performance than the
>> sdhci controller (according to Eric Anholt).  For the rpi3 switching to
>> this driver has the additional advantage that we can use the iproc
>> driver driver to handle the wifi.
> 
> OK
> -- 
> Florian
> 
> _______________________________________________
> linux-rpi-kernel mailing list
> linux-rpi-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rpi-kernel


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 01/13] mmc: bcm2835: add bcm2835_read_wait_sdcmd

[Shawn Lin]

On 2017/1/27 7:37, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  drivers/mmc/host/bcm2835.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
> diff --git a/drivers/mmc/host/bcm2835.c b/drivers/mmc/host/bcm2835.c
> index 9744517..ceee4cf 100644
> --- a/drivers/mmc/host/bcm2835.c
> +++ b/drivers/mmc/host/bcm2835.c
> @@ -609,6 +609,36 @@ static void bcm2835_prepare_data(struct bcm2835_host *host,
>  	writel(data->blocks, host->ioaddr + SDHBLC);
>  }
>
> +static u32 bcm2835_read_wait_sdcmd(struct bcm2835_host *host, u32 timeout,
> +				   bool check_fail)
> +{
> +	struct device *dev = &host->pdev->dev;
> +	unsigned long start = jiffies;
> +	unsigned long fastpoll = start + usecs_to_jiffies(10);
> +	unsigned long end = start + msecs_to_jiffies(timeout);
> +	u32 value;
> +
> +	for (;;) {
> +		value = readl(host->ioaddr + SDCMD);
> +		if (!(value & SDCMD_NEW_FLAG))
> +			break;
> +		if (check_fail && (value & SDCMD_FAIL_FLAG))
> +			break;
> +		if (time_after(jiffies, end)) {
> +			dev_err(dev, "%s: timeout (%d us)\n",
> +				__func__, timeout);
> +			break;
> +		}
> +
> +		/* if it takes longer reduce poll interval */
> +		if (time_after(jiffies, fastpoll))
> +			udelay(10);
> +		else
> +			cpu_relax();
> +	}

Use readl_poll_timeout intead of open-coding them..

> +	return value;
> +}
> +
>  bool bcm2835_send_command(struct bcm2835_host *host,
>  			  struct mmc_command *cmd)
>  {
>


-- 
Best Regards
Shawn Lin

Re: [PATCH 01/13] mmc: bcm2835: add bcm2835_read_wait_sdcmd

[Gerd Hoffmann]

[-- Attachment #1: Type: text/plain, Size: 652 bytes --]

  Hi,

> > +	for (;;) {
> > +		value = readl(host->ioaddr + SDCMD);
> > +		if (!(value & SDCMD_NEW_FLAG))
> > +			break;
> > +		if (check_fail && (value & SDCMD_FAIL_FLAG))
> > +			break;
> > +		if (time_after(jiffies, end)) {
> > +			dev_err(dev, "%s: timeout (%d us)\n",
> > +				__func__, timeout);
> > +			break;
> > +		}
> > +
> > +		/* if it takes longer reduce poll interval */
> > +		if (time_after(jiffies, fastpoll))
> > +			udelay(10);
> > +		else
> > +			cpu_relax();
> > +	}
> 
> Use readl_poll_timeout intead of open-coding them..

Cool.  Didn't know this exists.  Incremental fixup attached.

thanks,
  Gerd


[-- Attachment #2: Type: text/x-patch, Size: 2245 bytes --]

From de60d73849d83b0ddf4ee8bd6a7e175c69157a90 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Fri, 27 Jan 2017 11:25:54 +0100
Subject: [PATCH] mmc: bcm2835: use readl_poll_timeout in
 bcm2835_read_wait_sdcmd

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/mmc/host/bcm2835.c | 37 ++++++++++++++++---------------------
 1 file changed, 16 insertions(+), 21 deletions(-)

diff --git a/drivers/mmc/host/bcm2835.c b/drivers/mmc/host/bcm2835.c
index 64a2334..e8a3f6e 100644
--- a/drivers/mmc/host/bcm2835.c
+++ b/drivers/mmc/host/bcm2835.c
@@ -31,6 +31,7 @@
 #include <linux/err.h>
 #include <linux/highmem.h>
 #include <linux/io.h>
+#include <linux/iopoll.h>
 #include <linux/module.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
@@ -609,33 +610,27 @@ static void bcm2835_prepare_data(struct bcm2835_host *host,
 	writel(data->blocks, host->ioaddr + SDHBLC);
 }
 
-static u32 bcm2835_read_wait_sdcmd(struct bcm2835_host *host, u32 timeout,
+static u32 bcm2835_read_wait_sdcmd(struct bcm2835_host *host, u32 max_ms,
 				   bool check_fail)
 {
 	struct device *dev = &host->pdev->dev;
-	unsigned long start = jiffies;
-	unsigned long fastpoll = start + usecs_to_jiffies(10);
-	unsigned long end = start + msecs_to_jiffies(timeout);
 	u32 value;
+	int ret;
 
-	for (;;) {
-		value = readl(host->ioaddr + SDCMD);
-		if (!(value & SDCMD_NEW_FLAG))
-			break;
-		if (check_fail && (value & SDCMD_FAIL_FLAG))
-			break;
-		if (time_after(jiffies, end)) {
-			dev_err(dev, "%s: timeout (%d us)\n",
-				__func__, timeout);
-			break;
-		}
+	ret = readl_poll_timeout(host->ioaddr + SDCMD, value,
+				 (!(value & SDCMD_NEW_FLAG)) ||
+				 (check_fail && (value & SDCMD_FAIL_FLAG)),
+				 1, 10);
+	if (ret == -ETIMEDOUT)
+		/* if it takes a while make poll interval bigger */
+		ret = readl_poll_timeout(host->ioaddr + SDCMD, value,
+					 (!(value & SDCMD_NEW_FLAG)) ||
+					 (check_fail && (value & SDCMD_FAIL_FLAG)),
+					 10, max_ms * 1000);
+	if (ret == -ETIMEDOUT)
+		dev_err(dev, "%s: timeout (%d ms)\n",
+			__func__, max_ms);
 
-		/* if it takes longer reduce poll interval */
-		if (time_after(jiffies, fastpoll))
-			udelay(10);
-		else
-			cpu_relax();
-	}
 	return value;
 }
 
-- 
1.8.3.1


[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 02/13] mmc: bcm2835: use bcm2835_read_wait_sdcmd in bcm2835_send_command

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/mmc/host/bcm2835.c | 28 +++++++---------------------
 1 file changed, 7 insertions(+), 21 deletions(-)

diff --git a/drivers/mmc/host/bcm2835.c b/drivers/mmc/host/bcm2835.c
index ceee4cf..ffb9140 100644
--- a/drivers/mmc/host/bcm2835.c
+++ b/drivers/mmc/host/bcm2835.c
@@ -645,7 +645,6 @@ bool bcm2835_send_command(struct bcm2835_host *host,
 	struct device *dev = &host->pdev->dev;
 	u32 sdcmd, sdhsts;
 	unsigned long timeout;
-	int delay;
 
 	WARN_ON(host->cmd);
 
@@ -660,26 +659,13 @@ bool bcm2835_send_command(struct bcm2835_host *host,
 			cmd->opcode, cmd->arg, cmd->flags);
 	}
 
-	/* Wait max 100 ms */
-	timeout = 10000;
-
-	while (readl(host->ioaddr + SDCMD) & SDCMD_NEW_FLAG) {
-		if (timeout == 0) {
-			dev_err(dev, "previous command never completed.\n");
-			bcm2835_dumpregs(host);
-			cmd->error = -EILSEQ;
-			tasklet_schedule(&host->finish_tasklet);
-			return false;
-		}
-		timeout--;
-		udelay(10);
-	}
-
-	delay = (10000 - timeout) / 100;
-	if (delay > host->max_delay) {
-		host->max_delay = delay;
-		dev_warn(dev, "controller hung for %d ms\n",
-			 host->max_delay);
+	sdcmd = bcm2835_read_wait_sdcmd(host, 100, false);
+	if (sdcmd & SDCMD_NEW_FLAG) {
+		dev_err(dev, "previous command never completed.\n");
+		bcm2835_dumpregs(host);
+		cmd->error = -EILSEQ;
+		tasklet_schedule(&host->finish_tasklet);
+		return false;
 	}
 
 	timeout = jiffies;
-- 
1.8.3.1

[PATCH 03/13] mmc: bcm2835: add bcm2835_threaded_irq

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/mmc/host/bcm2835.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/bcm2835.c b/drivers/mmc/host/bcm2835.c
index ffb9140..6f9fb12 100644
--- a/drivers/mmc/host/bcm2835.c
+++ b/drivers/mmc/host/bcm2835.c
@@ -1105,6 +1105,11 @@ static irqreturn_t bcm2835_irq(int irq, void *dev_id)
 	return result;
 }
 
+static irqreturn_t bcm2835_threaded_irq(int irq, void *dev_id)
+{
+	return IRQ_HANDLED;
+}
+
 void bcm2835_set_clock(struct bcm2835_host *host, unsigned int clock)
 {
 	int div = 0; /* Initialized for compiler warning */
@@ -1405,8 +1410,9 @@ int bcm2835_add_host(struct bcm2835_host *host)
 
 	bcm2835_reset_internal(host);
 
-	ret = request_irq(host->irq, bcm2835_irq, 0 /*IRQF_SHARED*/,
-			  mmc_hostname(mmc), host);
+	ret = request_threaded_irq(host->irq, bcm2835_irq,
+				   bcm2835_threaded_irq,
+				   0, mmc_hostname(mmc), host);
 	if (ret) {
 		dev_err(dev, "failed to request IRQ %d: %d\n",
 			host->irq, ret);
-- 
1.8.3.1

[PATCH 04/13] mmc: bcm2835: add bcm2835_check_data_error

[Gerd Hoffmann]

Factor out common code.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/mmc/host/bcm2835.c | 30 ++++++++++++------------------
 1 file changed, 12 insertions(+), 18 deletions(-)

diff --git a/drivers/mmc/host/bcm2835.c b/drivers/mmc/host/bcm2835.c
index 6f9fb12..d25b85a 100644
--- a/drivers/mmc/host/bcm2835.c
+++ b/drivers/mmc/host/bcm2835.c
@@ -951,6 +951,16 @@ static void bcm2835_timeout(unsigned long data)
 	spin_unlock_irqrestore(&host->lock, flags);
 }
 
+static void bcm2835_check_data_error(struct bcm2835_host *host, u32 intmask)
+{
+	if (!host->data)
+		return;
+	if (intmask & (SDHSTS_CRC16_ERROR | SDHSTS_FIFO_ERROR))
+		host->data->error = -EILSEQ;
+	if (intmask & SDHSTS_REW_TIME_OUT)
+		host->data->error = -ETIMEDOUT;
+}
+
 static void bcm2835_busy_irq(struct bcm2835_host *host, u32 intmask)
 {
 	struct device *dev = &host->pdev->dev;
@@ -1007,15 +1017,7 @@ static void bcm2835_data_irq(struct bcm2835_host *host, u32 intmask)
 	if (!host->data)
 		return;
 
-	if (intmask & (SDHSTS_CRC16_ERROR |
-		       SDHSTS_FIFO_ERROR |
-		       SDHSTS_REW_TIME_OUT)) {
-		if (intmask & (SDHSTS_CRC16_ERROR |
-			       SDHSTS_FIFO_ERROR))
-			host->data->error = -EILSEQ;
-		else
-			host->data->error = -ETIMEDOUT;
-	}
+	bcm2835_check_data_error(host, intmask);
 
 	if (host->data->error) {
 		bcm2835_finish_data(host);
@@ -1043,15 +1045,7 @@ static void bcm2835_block_irq(struct bcm2835_host *host, u32 intmask)
 		return;
 	}
 
-	if (intmask & (SDHSTS_CRC16_ERROR |
-		       SDHSTS_FIFO_ERROR |
-		       SDHSTS_REW_TIME_OUT)) {
-		if (intmask & (SDHSTS_CRC16_ERROR |
-			       SDHSTS_FIFO_ERROR))
-			host->data->error = -EILSEQ;
-		else
-			host->data->error = -ETIMEDOUT;
-	}
+	bcm2835_check_data_error(host, intmask);
 
 	if (!host->dma_desc) {
 		WARN_ON(!host->blocks);
-- 
1.8.3.1

Re: [PATCH 04/13] mmc: bcm2835: add bcm2835_check_data_error

[Shawn Lin]

On 2017/1/27 7:37, Gerd Hoffmann wrote:
> Factor out common code.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  drivers/mmc/host/bcm2835.c | 30 ++++++++++++------------------
>  1 file changed, 12 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/mmc/host/bcm2835.c b/drivers/mmc/host/bcm2835.c
> index 6f9fb12..d25b85a 100644
> --- a/drivers/mmc/host/bcm2835.c
> +++ b/drivers/mmc/host/bcm2835.c
> @@ -951,6 +951,16 @@ static void bcm2835_timeout(unsigned long data)
>  	spin_unlock_irqrestore(&host->lock, flags);
>  }
>
> +static void bcm2835_check_data_error(struct bcm2835_host *host, u32 intmask)
> +{
> +	if (!host->data)
> +		return;
> +	if (intmask & (SDHSTS_CRC16_ERROR | SDHSTS_FIFO_ERROR))
> +		host->data->error = -EILSEQ;
> +	if (intmask & SDHSTS_REW_TIME_OUT)
> +		host->data->error = -ETIMEDOUT;
> +}
> +
>  static void bcm2835_busy_irq(struct bcm2835_host *host, u32 intmask)
>  {
>  	struct device *dev = &host->pdev->dev;
> @@ -1007,15 +1017,7 @@ static void bcm2835_data_irq(struct bcm2835_host *host, u32 intmask)
>  	if (!host->data)
>  		return;
>

remove this check, !host->data, as well.

> -	if (intmask & (SDHSTS_CRC16_ERROR |
> -		       SDHSTS_FIFO_ERROR |
> -		       SDHSTS_REW_TIME_OUT)) {
> -		if (intmask & (SDHSTS_CRC16_ERROR |
> -			       SDHSTS_FIFO_ERROR))
> -			host->data->error = -EILSEQ;
> -		else
> -			host->data->error = -ETIMEDOUT;
> -	}
> +	bcm2835_check_data_error(host, intmask);
>
>  	if (host->data->error) {
>  		bcm2835_finish_data(host);
> @@ -1043,15 +1045,7 @@ static void bcm2835_block_irq(struct bcm2835_host *host, u32 intmask)
>  		return;
>  	}
>
> -	if (intmask & (SDHSTS_CRC16_ERROR |
> -		       SDHSTS_FIFO_ERROR |
> -		       SDHSTS_REW_TIME_OUT)) {
> -		if (intmask & (SDHSTS_CRC16_ERROR |
> -			       SDHSTS_FIFO_ERROR))
> -			host->data->error = -EILSEQ;
> -		else
> -			host->data->error = -ETIMEDOUT;
> -	}
> +	bcm2835_check_data_error(host, intmask);
>
>  	if (!host->dma_desc) {
>  		WARN_ON(!host->blocks);
>


-- 
Best Regards
Shawn Lin

Re: [PATCH 04/13] mmc: bcm2835: add bcm2835_check_data_error

[Gerd Hoffmann]

  Hi,

> > @@ -1007,15 +1017,7 @@ static void bcm2835_data_irq(struct bcm2835_host *host, u32 intmask)
> >  	if (!host->data)
> >  		return;
> >
> 
> remove this check, !host->data, as well.

There is a comment in the code saying it is needed, here is more
context:

static void bcm2835_data_irq(struct bcm2835_host *host, u32 intmask)
{
	/* There are no dedicated data/space available interrupt
	 * status bits, so it is necessary to use the single shared
	 * data/space available FIFO status bits. It is therefore not
	 * an error to get here when there is no data transfer in
	 * progress.
	 */
	if (!host->data)
		return;

	bcm2835_check_data_error(host, intmask);

cheers,
  Gerd

[PATCH 05/13] mmc: bcm2835: call bcm2835_block_irq from irqthread

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/mmc/host/bcm2835.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/bcm2835.c b/drivers/mmc/host/bcm2835.c
index d25b85a..8270c76 100644
--- a/drivers/mmc/host/bcm2835.c
+++ b/drivers/mmc/host/bcm2835.c
@@ -168,6 +168,9 @@ struct bcm2835_host {
 	bool			use_busy:1;	/* Wait for busy interrupt */
 	bool			use_sbc:1;	/* Send CMD23 */
 
+	/* for threaded irq handler */
+	bool                    irq_block;
+
 	/* DMA part */
 	struct dma_chan		*dma_chan_rx;
 	struct dma_chan		*dma_chan_tx;
@@ -1035,18 +1038,13 @@ static void bcm2835_data_irq(struct bcm2835_host *host, u32 intmask)
 	}
 }
 
-static void bcm2835_block_irq(struct bcm2835_host *host, u32 intmask)
+static void bcm2835_block_irq(struct bcm2835_host *host)
 {
-	if (!host->data) {
-		dev_err(&host->pdev->dev,
-			"got block interrupt 0x%08x even though no data operation was in progress.\n",
-			(unsigned int)intmask);
+	if (WARN_ON(!host->data)) {
 		bcm2835_dumpregs(host);
 		return;
 	}
 
-	bcm2835_check_data_error(host, intmask);
-
 	if (!host->dma_desc) {
 		WARN_ON(!host->blocks);
 		if (host->data->error || (--host->blocks == 0))
@@ -1075,8 +1073,9 @@ static irqreturn_t bcm2835_irq(int irq, void *dev_id)
 	       host->ioaddr + SDHSTS);
 
 	if (intmask & SDHSTS_BLOCK_IRPT) {
-		bcm2835_block_irq(host, intmask);
-		result = IRQ_HANDLED;
+		bcm2835_check_data_error(host, intmask);
+		host->irq_block = true;
+		result = IRQ_WAKE_THREAD;
 	}
 
 	if (intmask & SDHSTS_BUSY_IRPT) {
@@ -1101,6 +1100,20 @@ static irqreturn_t bcm2835_irq(int irq, void *dev_id)
 
 static irqreturn_t bcm2835_threaded_irq(int irq, void *dev_id)
 {
+	struct bcm2835_host *host = dev_id;
+	unsigned long flags;
+	bool block;
+
+	spin_lock_irqsave(&host->lock, flags);
+
+	block = host->irq_block;
+	host->irq_block = false;
+
+	if (block)
+		bcm2835_block_irq(host);
+
+	spin_unlock_irqrestore(&host->lock, flags);
+
 	return IRQ_HANDLED;
 }
 
-- 
1.8.3.1

[PATCH 06/13] mmc: bcm2835: add bcm2835_check_cmd_error

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/mmc/host/bcm2835.c | 58 ++++++++++++++++++++++++++++------------------
 1 file changed, 35 insertions(+), 23 deletions(-)

diff --git a/drivers/mmc/host/bcm2835.c b/drivers/mmc/host/bcm2835.c
index 8270c76..33eaebd 100644
--- a/drivers/mmc/host/bcm2835.c
+++ b/drivers/mmc/host/bcm2835.c
@@ -954,6 +954,38 @@ static void bcm2835_timeout(unsigned long data)
 	spin_unlock_irqrestore(&host->lock, flags);
 }
 
+static bool bcm2835_check_cmd_error(struct bcm2835_host *host, u32 intmask)
+{
+	struct device *dev = &host->pdev->dev;
+
+	if (!(intmask & SDHSTS_ERROR_MASK))
+		return false;
+
+	if (!host->cmd)
+		return true;
+
+	dev_err(dev, "sdhost_busy_irq: intmask %x, data %p\n",
+		intmask, host->mrq->data);
+	if (intmask & SDHSTS_CRC7_ERROR) {
+		host->cmd->error = -EILSEQ;
+	} else if (intmask & (SDHSTS_CRC16_ERROR |
+			      SDHSTS_FIFO_ERROR)) {
+		if (host->mrq->data)
+			host->mrq->data->error = -EILSEQ;
+		else
+			host->cmd->error = -EILSEQ;
+	} else if (intmask & SDHSTS_REW_TIME_OUT) {
+		if (host->mrq->data)
+			host->mrq->data->error = -ETIMEDOUT;
+		else
+			host->cmd->error = -ETIMEDOUT;
+	} else if (intmask & SDHSTS_CMD_TIME_OUT) {
+		host->cmd->error = -ETIMEDOUT;
+	}
+	bcm2835_dumpregs(host);
+	return true;
+}
+
 static void bcm2835_check_data_error(struct bcm2835_host *host, u32 intmask)
 {
 	if (!host->data)
@@ -983,30 +1015,10 @@ static void bcm2835_busy_irq(struct bcm2835_host *host, u32 intmask)
 	}
 	host->use_busy = false;
 
-	if (intmask & SDHSTS_ERROR_MASK) {
-		dev_err(dev, "sdhost_busy_irq: intmask %x, data %p\n",
-			intmask, host->mrq->data);
-		if (intmask & SDHSTS_CRC7_ERROR) {
-			host->cmd->error = -EILSEQ;
-		} else if (intmask & (SDHSTS_CRC16_ERROR |
-				    SDHSTS_FIFO_ERROR)) {
-			if (host->mrq->data)
-				host->mrq->data->error = -EILSEQ;
-			else
-				host->cmd->error = -EILSEQ;
-		} else if (intmask & SDHSTS_REW_TIME_OUT) {
-			if (host->mrq->data)
-				host->mrq->data->error = -ETIMEDOUT;
-			else
-				host->cmd->error = -ETIMEDOUT;
-		} else if (intmask & SDHSTS_CMD_TIME_OUT) {
-			host->cmd->error = -ETIMEDOUT;
-		}
+	if (bcm2835_check_cmd_error(host, intmask))
+		return;
 
-		bcm2835_dumpregs(host);
-	} else {
-		bcm2835_finish_command(host, NULL);
-	}
+	bcm2835_finish_command(host, NULL);
 }
 
 static void bcm2835_data_irq(struct bcm2835_host *host, u32 intmask)
-- 
1.8.3.1

[PATCH 07/13] mmc: bcm2835: call bcm2835_busy_irq from irqthread

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/mmc/host/bcm2835.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/mmc/host/bcm2835.c b/drivers/mmc/host/bcm2835.c
index 33eaebd..814812b 100644
--- a/drivers/mmc/host/bcm2835.c
+++ b/drivers/mmc/host/bcm2835.c
@@ -170,6 +170,7 @@ struct bcm2835_host {
 
 	/* for threaded irq handler */
 	bool                    irq_block;
+	bool                    irq_busy;
 
 	/* DMA part */
 	struct dma_chan		*dma_chan_rx;
@@ -996,28 +997,19 @@ static void bcm2835_check_data_error(struct bcm2835_host *host, u32 intmask)
 		host->data->error = -ETIMEDOUT;
 }
 
-static void bcm2835_busy_irq(struct bcm2835_host *host, u32 intmask)
+static void bcm2835_busy_irq(struct bcm2835_host *host)
 {
-	struct device *dev = &host->pdev->dev;
-
-	if (!host->cmd) {
-		dev_err(dev, "got command busy interrupt 0x%08x even though no command operation was in progress.\n",
-			(unsigned int)intmask);
+	if (WARN_ON(!host->cmd)) {
 		bcm2835_dumpregs(host);
 		return;
 	}
 
-	if (!host->use_busy) {
-		dev_err(dev, "got command busy interrupt 0x%08x even though not expecting one.\n",
-			(unsigned int)intmask);
+	if (WARN_ON(!host->use_busy)) {
 		bcm2835_dumpregs(host);
 		return;
 	}
 	host->use_busy = false;
 
-	if (bcm2835_check_cmd_error(host, intmask))
-		return;
-
 	bcm2835_finish_command(host, NULL);
 }
 
@@ -1091,8 +1083,12 @@ static irqreturn_t bcm2835_irq(int irq, void *dev_id)
 	}
 
 	if (intmask & SDHSTS_BUSY_IRPT) {
-		bcm2835_busy_irq(host, intmask);
-		result = IRQ_HANDLED;
+		if (!bcm2835_check_cmd_error(host, intmask)) {
+			host->irq_busy = true;
+			result = IRQ_WAKE_THREAD;
+		} else {
+			result = IRQ_HANDLED;
+		}
 	}
 
 	/* There is no true data interrupt status bit, so it is
@@ -1114,15 +1110,19 @@ static irqreturn_t bcm2835_threaded_irq(int irq, void *dev_id)
 {
 	struct bcm2835_host *host = dev_id;
 	unsigned long flags;
-	bool block;
+	bool block, busy;
 
 	spin_lock_irqsave(&host->lock, flags);
 
 	block = host->irq_block;
+	busy  = host->irq_busy;
 	host->irq_block = false;
+	host->irq_busy  = false;
 
 	if (block)
 		bcm2835_block_irq(host);
+	if (busy)
+		bcm2835_busy_irq(host);
 
 	spin_unlock_irqrestore(&host->lock, flags);
 
-- 
1.8.3.1

[PATCH 08/13] mmc: bcm2835: split bcm2835_data_irq

[Gerd Hoffmann]

pio transfers stay in irq handler.
handle finished xfers moved to threaded handler.
most bcm2835 code runs in thread context now.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/mmc/host/bcm2835.c | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/bcm2835.c b/drivers/mmc/host/bcm2835.c
index 814812b..2bea4e0 100644
--- a/drivers/mmc/host/bcm2835.c
+++ b/drivers/mmc/host/bcm2835.c
@@ -171,6 +171,7 @@ struct bcm2835_host {
 	/* for threaded irq handler */
 	bool                    irq_block;
 	bool                    irq_busy;
+	bool                    irq_data;
 
 	/* DMA part */
 	struct dma_chan		*dma_chan_rx;
@@ -1025,10 +1026,10 @@ static void bcm2835_data_irq(struct bcm2835_host *host, u32 intmask)
 		return;
 
 	bcm2835_check_data_error(host, intmask);
+	if (host->data->error)
+		goto finished;
 
-	if (host->data->error) {
-		bcm2835_finish_data(host);
-	} else if (host->data->flags & MMC_DATA_WRITE) {
+	if (host->data->flags & MMC_DATA_WRITE) {
 		/* Use the block interrupt for writes after the first block */
 		host->hcfg &= ~(SDHCFG_DATA_IRPT_EN);
 		host->hcfg |= SDHCFG_BLOCK_IRPT_EN;
@@ -1038,8 +1039,22 @@ static void bcm2835_data_irq(struct bcm2835_host *host, u32 intmask)
 		bcm2835_transfer_pio(host);
 		host->blocks--;
 		if ((host->blocks == 0) || host->data->error)
-			bcm2835_finish_data(host);
+			goto finished;
 	}
+	return;
+
+finished:
+	host->hcfg &= ~(SDHCFG_DATA_IRPT_EN | SDHCFG_BLOCK_IRPT_EN);
+	writel(host->hcfg, host->ioaddr + SDHCFG);
+	return;
+}
+
+static void bcm2835_data_threaded_irq(struct bcm2835_host *host)
+{
+	if (!host->data)
+		return;
+	if ((host->blocks == 0) || host->data->error)
+		bcm2835_finish_data(host);
 }
 
 static void bcm2835_block_irq(struct bcm2835_host *host)
@@ -1098,7 +1113,8 @@ static irqreturn_t bcm2835_irq(int irq, void *dev_id)
 	if ((intmask & SDHSTS_DATA_FLAG) &&
 	    (host->hcfg & SDHCFG_DATA_IRPT_EN)) {
 		bcm2835_data_irq(host, intmask);
-		result = IRQ_HANDLED;
+		host->irq_data = true;
+		result = IRQ_WAKE_THREAD;
 	}
 
 	spin_unlock(&host->lock);
@@ -1110,19 +1126,23 @@ static irqreturn_t bcm2835_threaded_irq(int irq, void *dev_id)
 {
 	struct bcm2835_host *host = dev_id;
 	unsigned long flags;
-	bool block, busy;
+	bool block, busy, data;
 
 	spin_lock_irqsave(&host->lock, flags);
 
 	block = host->irq_block;
 	busy  = host->irq_busy;
+	data  = host->irq_data;
 	host->irq_block = false;
 	host->irq_busy  = false;
+	host->irq_data  = false;
 
 	if (block)
 		bcm2835_block_irq(host);
 	if (busy)
 		bcm2835_busy_irq(host);
+	if (data)
+		bcm2835_data_threaded_irq(host);
 
 	spin_unlock_irqrestore(&host->lock, flags);
 
-- 
1.8.3.1

[PATCH 09/13] mmc: bcm2835: switch locking to mutex

[Gerd Hoffmann]

so we can sleep now in most bcm2835 code.
especially bcm2835_finish_command can always sleep.

FIXME: timeout timer seems to run in irq context.
FIXME: tasklet seems to run in irq context too.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/mmc/host/bcm2835.c | 74 +++++++++++++++++++---------------------------
 1 file changed, 31 insertions(+), 43 deletions(-)

diff --git a/drivers/mmc/host/bcm2835.c b/drivers/mmc/host/bcm2835.c
index 2bea4e0..5c588d9 100644
--- a/drivers/mmc/host/bcm2835.c
+++ b/drivers/mmc/host/bcm2835.c
@@ -136,6 +136,7 @@
 
 struct bcm2835_host {
 	spinlock_t		lock;
+	struct mutex            mutex;
 
 	void __iomem		*ioaddr;
 	u32			phys_addr;
@@ -278,8 +279,7 @@ static void bcm2835_reset(struct mmc_host *mmc)
 	bcm2835_reset_internal(host);
 }
 
-static void bcm2835_finish_command(struct bcm2835_host *host,
-				   unsigned long *irq_flags);
+static void bcm2835_finish_command(struct bcm2835_host *host);
 
 static void bcm2835_wait_transfer_complete(struct bcm2835_host *host)
 {
@@ -325,9 +325,8 @@ static void bcm2835_dma_complete(void *param)
 {
 	struct bcm2835_host *host = param;
 	struct mmc_data *data = host->data;
-	unsigned long flags;
 
-	spin_lock_irqsave(&host->lock, flags);
+	mutex_lock(&host->mutex);
 
 	if (host->dma_chan) {
 		dma_unmap_sg(host->dma_chan->device->dev,
@@ -357,7 +356,7 @@ static void bcm2835_dma_complete(void *param)
 
 	bcm2835_finish_data(host);
 
-	spin_unlock_irqrestore(&host->lock, flags);
+	mutex_unlock(&host->mutex);
 }
 
 static void bcm2835_transfer_block_pio(struct bcm2835_host *host,
@@ -744,7 +743,7 @@ static void bcm2835_transfer_complete(struct bcm2835_host *host)
 		if (bcm2835_send_command(host, host->mrq->stop)) {
 			/* No busy, so poll for completion */
 			if (!host->use_busy)
-				bcm2835_finish_command(host, NULL);
+				bcm2835_finish_command(host);
 		}
 	} else {
 		bcm2835_wait_transfer_complete(host);
@@ -782,11 +781,7 @@ static void bcm2835_finish_data(struct bcm2835_host *host)
 	}
 }
 
-/* If irq_flags is valid, the caller is in a thread context and is
- * allowed to sleep
- */
-static void bcm2835_finish_command(struct bcm2835_host *host,
-				   unsigned long *irq_flags)
+static void bcm2835_finish_command(struct bcm2835_host *host)
 {
 	struct device *dev = &host->pdev->dev;
 	struct mmc_command *cmd = host->cmd;
@@ -837,18 +832,10 @@ static void bcm2835_finish_command(struct bcm2835_host *host,
 	if (!retries) {
 		unsigned long wait_max;
 
-		if (!irq_flags) {
-			/* Schedule the work */
-			schedule_work(&host->cmd_wait_wq);
-			return;
-		}
-
 		/* Wait max 100 ms */
 		wait_max = jiffies + msecs_to_jiffies(100);
 		while (time_before(jiffies, wait_max)) {
-			spin_unlock_irqrestore(&host->lock, *irq_flags);
 			usleep_range(1, 10);
-			spin_lock_irqsave(&host->lock, *irq_flags);
 			sdcmd = readl(host->ioaddr + SDCMD);
 			if (!(sdcmd & SDCMD_NEW_FLAG) ||
 			    (sdcmd & SDCMD_FAIL_FLAG))
@@ -913,7 +900,7 @@ static void bcm2835_finish_command(struct bcm2835_host *host,
 				bcm2835_start_dma(host);
 
 			if (!host->use_busy)
-				bcm2835_finish_command(host, NULL);
+				bcm2835_finish_command(host);
 		}
 	} else if (cmd == host->mrq->stop) {
 		/* Finished CMD12 */
@@ -932,9 +919,8 @@ static void bcm2835_timeout(unsigned long data)
 {
 	struct bcm2835_host *host = (struct bcm2835_host *)data;
 	struct device *dev = &host->pdev->dev;
-	unsigned long flags;
 
-	spin_lock_irqsave(&host->lock, flags);
+	mutex_lock(&host->mutex);
 
 	if (host->mrq) {
 		dev_err(dev, "timeout waiting for hardware interrupt.\n");
@@ -953,7 +939,8 @@ static void bcm2835_timeout(unsigned long data)
 			tasklet_schedule(&host->finish_tasklet);
 		}
 	}
-	spin_unlock_irqrestore(&host->lock, flags);
+
+	mutex_unlock(&host->mutex);
 }
 
 static bool bcm2835_check_cmd_error(struct bcm2835_host *host, u32 intmask)
@@ -1011,7 +998,7 @@ static void bcm2835_busy_irq(struct bcm2835_host *host)
 	}
 	host->use_busy = false;
 
-	bcm2835_finish_command(host, NULL);
+	bcm2835_finish_command(host);
 }
 
 static void bcm2835_data_irq(struct bcm2835_host *host, u32 intmask)
@@ -1137,6 +1124,10 @@ static irqreturn_t bcm2835_threaded_irq(int irq, void *dev_id)
 	host->irq_busy  = false;
 	host->irq_data  = false;
 
+	spin_unlock_irqrestore(&host->lock, flags);
+
+	mutex_lock(&host->mutex);
+
 	if (block)
 		bcm2835_block_irq(host);
 	if (busy)
@@ -1144,7 +1135,7 @@ static irqreturn_t bcm2835_threaded_irq(int irq, void *dev_id)
 	if (data)
 		bcm2835_data_threaded_irq(host);
 
-	spin_unlock_irqrestore(&host->lock, flags);
+	mutex_unlock(&host->mutex);
 
 	return IRQ_HANDLED;
 }
@@ -1215,7 +1206,6 @@ static void bcm2835_request(struct mmc_host *mmc,
 {
 	struct bcm2835_host *host = mmc_priv(mmc);
 	struct device *dev = &host->pdev->dev;
-	unsigned long flags;
 	u32 edm, fsm;
 
 	/* Reset the error statuses in case this is a retry */
@@ -1240,7 +1230,7 @@ static void bcm2835_request(struct mmc_host *mmc,
 	    (mrq->data->blocks > host->pio_limit))
 		bcm2835_prepare_dma(host, mrq->data);
 
-	spin_lock_irqsave(&host->lock, flags);
+	mutex_lock(&host->mutex);
 
 	WARN_ON(host->mrq);
 	host->mrq = mrq;
@@ -1256,7 +1246,7 @@ static void bcm2835_request(struct mmc_host *mmc,
 		bcm2835_dumpregs(host);
 		mrq->cmd->error = -EILSEQ;
 		tasklet_schedule(&host->finish_tasklet);
-		spin_unlock_irqrestore(&host->lock, flags);
+		mutex_unlock(&host->mutex);
 		return;
 	}
 
@@ -1264,7 +1254,7 @@ static void bcm2835_request(struct mmc_host *mmc,
 	if (host->use_sbc) {
 		if (bcm2835_send_command(host, mrq->sbc)) {
 			if (!host->use_busy)
-				bcm2835_finish_command(host, &flags);
+				bcm2835_finish_command(host);
 		}
 	} else if (bcm2835_send_command(host, mrq->cmd)) {
 		if (host->data && host->dma_desc) {
@@ -1273,18 +1263,17 @@ static void bcm2835_request(struct mmc_host *mmc,
 		}
 
 		if (!host->use_busy)
-			bcm2835_finish_command(host, &flags);
+			bcm2835_finish_command(host);
 	}
 
-	spin_unlock_irqrestore(&host->lock, flags);
+	mutex_unlock(&host->mutex);
 }
 
 static void bcm2835_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 {
 	struct bcm2835_host *host = mmc_priv(mmc);
-	unsigned long flags;
 
-	spin_lock_irqsave(&host->lock, flags);
+	mutex_lock(&host->mutex);
 
 	if (!ios->clock || ios->clock != host->clock) {
 		bcm2835_set_clock(host, ios->clock);
@@ -1303,7 +1292,7 @@ static void bcm2835_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 
 	writel(host->hcfg, host->ioaddr + SDHCFG);
 
-	spin_unlock_irqrestore(&host->lock, flags);
+	mutex_unlock(&host->mutex);
 }
 
 static struct mmc_host_ops bcm2835_ops = {
@@ -1315,39 +1304,37 @@ static void bcm2835_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 static void bcm2835_cmd_wait_work(struct work_struct *work)
 {
 	struct bcm2835_host *host;
-	unsigned long flags;
 
 	host = container_of(work, struct bcm2835_host, cmd_wait_wq);
 
-	spin_lock_irqsave(&host->lock, flags);
+	mutex_lock(&host->mutex);
 
 	/* If this tasklet gets rescheduled while running, it will
 	 * be run again afterwards but without any active request.
 	 */
 	if (!host->mrq) {
-		spin_unlock_irqrestore(&host->lock, flags);
+		mutex_unlock(&host->mutex);
 		return;
 	}
 
-	bcm2835_finish_command(host, &flags);
+	bcm2835_finish_command(host);
 
-	spin_unlock_irqrestore(&host->lock, flags);
+	mutex_unlock(&host->mutex);
 }
 
 static void bcm2835_tasklet_finish(unsigned long param)
 {
 	struct bcm2835_host *host = (struct bcm2835_host *)param;
-	unsigned long flags;
 	struct mmc_request *mrq;
 	struct dma_chan *terminate_chan = NULL;
 
-	spin_lock_irqsave(&host->lock, flags);
+	mutex_lock(&host->mutex);
 
 	/* If this tasklet gets rescheduled while running, it will
 	 * be run again afterwards but without any active request.
 	 */
 	if (!host->mrq) {
-		spin_unlock_irqrestore(&host->lock, flags);
+		mutex_unlock(&host->mutex);
 		return;
 	}
 
@@ -1363,7 +1350,7 @@ static void bcm2835_tasklet_finish(unsigned long param)
 	terminate_chan = host->dma_chan;
 	host->dma_chan = NULL;
 
-	spin_unlock_irqrestore(&host->lock, flags);
+	mutex_unlock(&host->mutex);
 
 	if (terminate_chan) {
 		int err = dmaengine_terminate_all(terminate_chan);
@@ -1400,6 +1387,7 @@ int bcm2835_add_host(struct bcm2835_host *host)
 		     MMC_CAP_CMD23;
 
 	spin_lock_init(&host->lock);
+	mutex_init(&host->mutex);
 
 	if (IS_ERR_OR_NULL(host->dma_chan_tx) ||
 	    IS_ERR_OR_NULL(host->dma_chan_rx)) {
-- 
1.8.3.1

[PATCH 10/13] mmc: bcm2835: work queue is dead code now, zap

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/mmc/host/bcm2835.c | 24 ------------------------
 1 file changed, 24 deletions(-)

diff --git a/drivers/mmc/host/bcm2835.c b/drivers/mmc/host/bcm2835.c
index 5c588d9..6a1fef3 100644
--- a/drivers/mmc/host/bcm2835.c
+++ b/drivers/mmc/host/bcm2835.c
@@ -148,7 +148,6 @@ struct bcm2835_host {
 	int			clock;		/* Current clock speed */
 	unsigned int		max_clk;	/* Max possible freq */
 	struct tasklet_struct	finish_tasklet;	/* Tasklet structures */
-	struct work_struct	cmd_wait_wq;	/* Workqueue function */
 	struct timer_list	timer;		/* Timer for timeouts */
 	struct sg_mapping_iter	sg_miter;	/* SG state for PIO */
 	unsigned int		blocks;		/* remaining PIO blocks */
@@ -1301,27 +1300,6 @@ static void bcm2835_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	.hw_reset = bcm2835_reset,
 };
 
-static void bcm2835_cmd_wait_work(struct work_struct *work)
-{
-	struct bcm2835_host *host;
-
-	host = container_of(work, struct bcm2835_host, cmd_wait_wq);
-
-	mutex_lock(&host->mutex);
-
-	/* If this tasklet gets rescheduled while running, it will
-	 * be run again afterwards but without any active request.
-	 */
-	if (!host->mrq) {
-		mutex_unlock(&host->mutex);
-		return;
-	}
-
-	bcm2835_finish_command(host);
-
-	mutex_unlock(&host->mutex);
-}
-
 static void bcm2835_tasklet_finish(unsigned long param)
 {
 	struct bcm2835_host *host = (struct bcm2835_host *)param;
@@ -1427,8 +1405,6 @@ int bcm2835_add_host(struct bcm2835_host *host)
 	tasklet_init(&host->finish_tasklet,
 		     bcm2835_tasklet_finish, (unsigned long)host);
 
-	INIT_WORK(&host->cmd_wait_wq, bcm2835_cmd_wait_work);
-
 	setup_timer(&host->timer, bcm2835_timeout,
 		    (unsigned long)host);
 
-- 
1.8.3.1

[PATCH 11/13] mmc: bcm2835: kill tasklet

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/mmc/host/bcm2835.c | 53 ++++++++++++----------------------------------
 1 file changed, 14 insertions(+), 39 deletions(-)

diff --git a/drivers/mmc/host/bcm2835.c b/drivers/mmc/host/bcm2835.c
index 6a1fef3..fc2d02f 100644
--- a/drivers/mmc/host/bcm2835.c
+++ b/drivers/mmc/host/bcm2835.c
@@ -147,7 +147,6 @@ struct bcm2835_host {
 	u32			pio_timeout;	/* In jiffies */
 	int			clock;		/* Current clock speed */
 	unsigned int		max_clk;	/* Max possible freq */
-	struct tasklet_struct	finish_tasklet;	/* Tasklet structures */
 	struct timer_list	timer;		/* Timer for timeouts */
 	struct sg_mapping_iter	sg_miter;	/* SG state for PIO */
 	unsigned int		blocks;		/* remaining PIO blocks */
@@ -274,11 +273,12 @@ static void bcm2835_reset(struct mmc_host *mmc)
 
 	if (host->dma_chan)
 		dmaengine_terminate_sync(host->dma_chan);
-	tasklet_kill(&host->finish_tasklet);
 	bcm2835_reset_internal(host);
 }
 
+static void bcm2835_finish_data(struct bcm2835_host *host);
 static void bcm2835_finish_command(struct bcm2835_host *host);
+static void bcm2835_finish_request(struct bcm2835_host *host);
 
 static void bcm2835_wait_transfer_complete(struct bcm2835_host *host)
 {
@@ -318,8 +318,6 @@ static void bcm2835_wait_transfer_complete(struct bcm2835_host *host)
 	}
 }
 
-static void bcm2835_finish_data(struct bcm2835_host *host);
-
 static void bcm2835_dma_complete(void *param)
 {
 	struct bcm2835_host *host = param;
@@ -667,7 +665,7 @@ bool bcm2835_send_command(struct bcm2835_host *host,
 		dev_err(dev, "previous command never completed.\n");
 		bcm2835_dumpregs(host);
 		cmd->error = -EILSEQ;
-		tasklet_schedule(&host->finish_tasklet);
+		bcm2835_finish_request(host);
 		return false;
 	}
 
@@ -688,7 +686,7 @@ bool bcm2835_send_command(struct bcm2835_host *host,
 	if ((cmd->flags & MMC_RSP_136) && (cmd->flags & MMC_RSP_BUSY)) {
 		dev_err(dev, "unsupported response type!\n");
 		cmd->error = -EINVAL;
-		tasklet_schedule(&host->finish_tasklet);
+		bcm2835_finish_request(host);
 		return false;
 	}
 
@@ -746,7 +744,7 @@ static void bcm2835_transfer_complete(struct bcm2835_host *host)
 		}
 	} else {
 		bcm2835_wait_transfer_complete(host);
-		tasklet_schedule(&host->finish_tasklet);
+		bcm2835_finish_request(host);
 	}
 }
 
@@ -847,7 +845,7 @@ static void bcm2835_finish_command(struct bcm2835_host *host)
 		dev_err(dev, "command never completed.\n");
 		bcm2835_dumpregs(host);
 		host->cmd->error = -EIO;
-		tasklet_schedule(&host->finish_tasklet);
+		bcm2835_finish_request(host);
 		return;
 	} else if (sdcmd & SDCMD_FAIL_FLAG) {
 		u32 sdhsts = readl(host->ioaddr + SDHSTS);
@@ -865,7 +863,7 @@ static void bcm2835_finish_command(struct bcm2835_host *host)
 				bcm2835_dumpregs(host);
 				host->cmd->error = -EILSEQ;
 			}
-			tasklet_schedule(&host->finish_tasklet);
+			bcm2835_finish_request(host);
 			return;
 		}
 	}
@@ -903,12 +901,12 @@ static void bcm2835_finish_command(struct bcm2835_host *host)
 		}
 	} else if (cmd == host->mrq->stop) {
 		/* Finished CMD12 */
-		tasklet_schedule(&host->finish_tasklet);
+		bcm2835_finish_request(host);
 	} else {
 		/* Processed actual command. */
 		host->cmd = NULL;
 		if (!host->data)
-			tasklet_schedule(&host->finish_tasklet);
+			bcm2835_finish_request(host);
 		else if (host->data_complete)
 			bcm2835_transfer_complete(host);
 	}
@@ -935,7 +933,7 @@ static void bcm2835_timeout(unsigned long data)
 				host->mrq->cmd->error = -ETIMEDOUT;
 
 			dev_dbg(dev, "timeout_timer tasklet_schedule\n");
-			tasklet_schedule(&host->finish_tasklet);
+			bcm2835_finish_request(host);
 		}
 	}
 
@@ -1244,7 +1242,7 @@ static void bcm2835_request(struct mmc_host *mmc,
 			edm);
 		bcm2835_dumpregs(host);
 		mrq->cmd->error = -EILSEQ;
-		tasklet_schedule(&host->finish_tasklet);
+		bcm2835_finish_request(host);
 		mutex_unlock(&host->mutex);
 		return;
 	}
@@ -1300,21 +1298,10 @@ static void bcm2835_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	.hw_reset = bcm2835_reset,
 };
 
-static void bcm2835_tasklet_finish(unsigned long param)
+static void bcm2835_finish_request(struct bcm2835_host *host)
 {
-	struct bcm2835_host *host = (struct bcm2835_host *)param;
-	struct mmc_request *mrq;
 	struct dma_chan *terminate_chan = NULL;
-
-	mutex_lock(&host->mutex);
-
-	/* If this tasklet gets rescheduled while running, it will
-	 * be run again afterwards but without any active request.
-	 */
-	if (!host->mrq) {
-		mutex_unlock(&host->mutex);
-		return;
-	}
+	struct mmc_request *mrq;
 
 	del_timer(&host->timer);
 
@@ -1328,8 +1315,6 @@ static void bcm2835_tasklet_finish(unsigned long param)
 	terminate_chan = host->dma_chan;
 	host->dma_chan = NULL;
 
-	mutex_unlock(&host->mutex);
-
 	if (terminate_chan) {
 		int err = dmaengine_terminate_all(terminate_chan);
 
@@ -1402,9 +1387,6 @@ int bcm2835_add_host(struct bcm2835_host *host)
 	/* report supported voltage ranges */
 	mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
 
-	tasklet_init(&host->finish_tasklet,
-		     bcm2835_tasklet_finish, (unsigned long)host);
-
 	setup_timer(&host->timer, bcm2835_timeout,
 		    (unsigned long)host);
 
@@ -1419,7 +1401,7 @@ int bcm2835_add_host(struct bcm2835_host *host)
 	if (ret) {
 		dev_err(dev, "failed to request IRQ %d: %d\n",
 			host->irq, ret);
-		goto untasklet;
+		return ret;
 	}
 
 	mmc_add_host(mmc);
@@ -1432,11 +1414,6 @@ int bcm2835_add_host(struct bcm2835_host *host)
 		 pio_limit_string);
 
 	return 0;
-
-untasklet:
-	tasklet_kill(&host->finish_tasklet);
-
-	return ret;
 }
 
 static int bcm2835_probe(struct platform_device *pdev)
@@ -1548,8 +1525,6 @@ static int bcm2835_remove(struct platform_device *pdev)
 
 	del_timer_sync(&host->timer);
 
-	tasklet_kill(&host->finish_tasklet);
-
 	mmc_free_host(host->mmc);
 	platform_set_drvdata(pdev, NULL);
 
-- 
1.8.3.1

[PATCH 12/13] mmc: bcm2835: move timeout to thread context

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/mmc/host/bcm2835.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/mmc/host/bcm2835.c b/drivers/mmc/host/bcm2835.c
index fc2d02f..82caddc 100644
--- a/drivers/mmc/host/bcm2835.c
+++ b/drivers/mmc/host/bcm2835.c
@@ -147,7 +147,7 @@ struct bcm2835_host {
 	u32			pio_timeout;	/* In jiffies */
 	int			clock;		/* Current clock speed */
 	unsigned int		max_clk;	/* Max possible freq */
-	struct timer_list	timer;		/* Timer for timeouts */
+	struct delayed_work     timeout_work;   /* Timer for timeouts */
 	struct sg_mapping_iter	sg_miter;	/* SG state for PIO */
 	unsigned int		blocks;		/* remaining PIO blocks */
 	int			irq;		/* Device IRQ */
@@ -669,12 +669,11 @@ bool bcm2835_send_command(struct bcm2835_host *host,
 		return false;
 	}
 
-	timeout = jiffies;
 	if (!cmd->data && cmd->busy_timeout > 9000)
-		timeout += DIV_ROUND_UP(cmd->busy_timeout, 1000) * HZ + HZ;
+		timeout = DIV_ROUND_UP(cmd->busy_timeout, 1000) * HZ + HZ;
 	else
-		timeout += 10 * HZ;
-	mod_timer(&host->timer, timeout);
+		timeout = 10 * HZ;
+	schedule_delayed_work(&host->timeout_work, timeout);
 
 	host->cmd = cmd;
 
@@ -912,9 +911,11 @@ static void bcm2835_finish_command(struct bcm2835_host *host)
 	}
 }
 
-static void bcm2835_timeout(unsigned long data)
+static void bcm2835_timeout(struct work_struct *work)
 {
-	struct bcm2835_host *host = (struct bcm2835_host *)data;
+	struct delayed_work *d = to_delayed_work(work);
+	struct bcm2835_host *host =
+		container_of(d, struct bcm2835_host, timeout_work);
 	struct device *dev = &host->pdev->dev;
 
 	mutex_lock(&host->mutex);
@@ -1303,7 +1304,7 @@ static void bcm2835_finish_request(struct bcm2835_host *host)
 	struct dma_chan *terminate_chan = NULL;
 	struct mmc_request *mrq;
 
-	del_timer(&host->timer);
+	cancel_delayed_work(&host->timeout_work);
 
 	mrq = host->mrq;
 
@@ -1387,8 +1388,7 @@ int bcm2835_add_host(struct bcm2835_host *host)
 	/* report supported voltage ranges */
 	mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
 
-	setup_timer(&host->timer, bcm2835_timeout,
-		    (unsigned long)host);
+	INIT_DELAYED_WORK(&host->timeout_work, bcm2835_timeout);
 
 	/* Set interrupt enables */
 	host->hcfg = SDHCFG_BUSY_IRPT_EN;
@@ -1523,7 +1523,7 @@ static int bcm2835_remove(struct platform_device *pdev)
 
 	free_irq(host->irq, host);
 
-	del_timer_sync(&host->timer);
+	cancel_delayed_work_sync(&host->timeout_work);
 
 	mmc_free_host(host->mmc);
 	platform_set_drvdata(pdev, NULL);
-- 
1.8.3.1

Re: [PATCH 12/13] mmc: bcm2835: move timeout to thread context

[Shawn Lin]

On 2017/1/27 7:37, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  drivers/mmc/host/bcm2835.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/mmc/host/bcm2835.c b/drivers/mmc/host/bcm2835.c
> index fc2d02f..82caddc 100644
> --- a/drivers/mmc/host/bcm2835.c
> +++ b/drivers/mmc/host/bcm2835.c
> @@ -147,7 +147,7 @@ struct bcm2835_host {
>  	u32			pio_timeout;	/* In jiffies */
>  	int			clock;		/* Current clock speed */
>  	unsigned int		max_clk;	/* Max possible freq */
> -	struct timer_list	timer;		/* Timer for timeouts */
> +	struct delayed_work     timeout_work;   /* Timer for timeouts */
>  	struct sg_mapping_iter	sg_miter;	/* SG state for PIO */
>  	unsigned int		blocks;		/* remaining PIO blocks */
>  	int			irq;		/* Device IRQ */
> @@ -669,12 +669,11 @@ bool bcm2835_send_command(struct bcm2835_host *host,
>  		return false;
>  	}
>
> -	timeout = jiffies;
>  	if (!cmd->data && cmd->busy_timeout > 9000)
> -		timeout += DIV_ROUND_UP(cmd->busy_timeout, 1000) * HZ + HZ;
> +		timeout = DIV_ROUND_UP(cmd->busy_timeout, 1000) * HZ + HZ;

You should elaborate more to help folks understand
what you want to do here...


>  	else
> -		timeout += 10 * HZ;
> -	mod_timer(&host->timer, timeout);
> +		timeout = 10 * HZ;
> +	schedule_delayed_work(&host->timeout_work, timeout);
>
>  	host->cmd = cmd;
>
> @@ -912,9 +911,11 @@ static void bcm2835_finish_command(struct bcm2835_host *host)
>  	}
>  }
>
> -static void bcm2835_timeout(unsigned long data)
> +static void bcm2835_timeout(struct work_struct *work)
>  {
> -	struct bcm2835_host *host = (struct bcm2835_host *)data;
> +	struct delayed_work *d = to_delayed_work(work);
> +	struct bcm2835_host *host =
> +		container_of(d, struct bcm2835_host, timeout_work);
>  	struct device *dev = &host->pdev->dev;
>
>  	mutex_lock(&host->mutex);
> @@ -1303,7 +1304,7 @@ static void bcm2835_finish_request(struct bcm2835_host *host)
>  	struct dma_chan *terminate_chan = NULL;
>  	struct mmc_request *mrq;
>
> -	del_timer(&host->timer);
> +	cancel_delayed_work(&host->timeout_work);
>
>  	mrq = host->mrq;
>
> @@ -1387,8 +1388,7 @@ int bcm2835_add_host(struct bcm2835_host *host)
>  	/* report supported voltage ranges */
>  	mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
>
> -	setup_timer(&host->timer, bcm2835_timeout,
> -		    (unsigned long)host);
> +	INIT_DELAYED_WORK(&host->timeout_work, bcm2835_timeout);
>
>  	/* Set interrupt enables */
>  	host->hcfg = SDHCFG_BUSY_IRPT_EN;
> @@ -1523,7 +1523,7 @@ static int bcm2835_remove(struct platform_device *pdev)
>
>  	free_irq(host->irq, host);
>
> -	del_timer_sync(&host->timer);
> +	cancel_delayed_work_sync(&host->timeout_work);
>
>  	mmc_free_host(host->mmc);
>  	platform_set_drvdata(pdev, NULL);
>


-- 
Best Regards
Shawn Lin

[PATCH 13/13] mmc: bcm2835: use bcm2835_read_wait_sdcmd in bcm2835_finish_command

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/mmc/host/bcm2835.c | 56 +---------------------------------------------
 1 file changed, 1 insertion(+), 55 deletions(-)

diff --git a/drivers/mmc/host/bcm2835.c b/drivers/mmc/host/bcm2835.c
index 82caddc..64a2334 100644
--- a/drivers/mmc/host/bcm2835.c
+++ b/drivers/mmc/host/bcm2835.c
@@ -152,7 +152,6 @@ struct bcm2835_host {
 	unsigned int		blocks;		/* remaining PIO blocks */
 	int			irq;		/* Device IRQ */
 
-	u32			cmd_quick_poll_retries;
 	u32			ns_per_fifo_word;
 
 	/* cached registers */
@@ -782,62 +781,10 @@ static void bcm2835_finish_command(struct bcm2835_host *host)
 	struct device *dev = &host->pdev->dev;
 	struct mmc_command *cmd = host->cmd;
 	u32 sdcmd;
-	u32 retries;
 
 	dev_dbg(dev, "finish_command(%x)\n", readl(host->ioaddr + SDCMD));
 
-	/* Poll quickly at first */
-
-	retries = host->cmd_quick_poll_retries;
-	if (!retries) {
-		/* Work out how many polls take 1us by timing 10us */
-		struct timeval start, now;
-		int us_diff;
-
-		retries = 1;
-		do {
-			int i;
-
-			retries *= 2;
-
-			do_gettimeofday(&start);
-
-			for (i = 0; i < retries; i++) {
-				cpu_relax();
-				sdcmd = readl(host->ioaddr + SDCMD);
-			}
-
-			do_gettimeofday(&now);
-			us_diff = (now.tv_sec - start.tv_sec) * 1000000 +
-				(now.tv_usec - start.tv_usec);
-		} while (us_diff < 10);
-
-		host->cmd_quick_poll_retries =
-			((retries * us_diff + 9) * CMD_DALLY_US) / 10 + 1;
-		retries = 1; /* We've already waited long enough this time */
-	}
-
-	retries = host->cmd_quick_poll_retries;
-	for (sdcmd = readl(host->ioaddr + SDCMD);
-	     (sdcmd & SDCMD_NEW_FLAG) && !(sdcmd & SDCMD_FAIL_FLAG) && retries;
-	     retries--) {
-		cpu_relax();
-		sdcmd = readl(host->ioaddr + SDCMD);
-	}
-
-	if (!retries) {
-		unsigned long wait_max;
-
-		/* Wait max 100 ms */
-		wait_max = jiffies + msecs_to_jiffies(100);
-		while (time_before(jiffies, wait_max)) {
-			usleep_range(1, 10);
-			sdcmd = readl(host->ioaddr + SDCMD);
-			if (!(sdcmd & SDCMD_NEW_FLAG) ||
-			    (sdcmd & SDCMD_FAIL_FLAG))
-				break;
-		}
-	}
+	sdcmd = bcm2835_read_wait_sdcmd(host, 100, true);
 
 	/* Check for errors */
 	if (sdcmd & SDCMD_NEW_FLAG) {
@@ -1435,7 +1382,6 @@ static int bcm2835_probe(struct platform_device *pdev)
 	host = mmc_priv(mmc);
 	host->mmc = mmc;
 	host->pdev = pdev;
-	host->cmd_quick_poll_retries = 0;
 	host->pio_timeout = msecs_to_jiffies(500);
 	host->pio_limit = 1;
 	host->max_delay = 1; /* Warn if over 1ms */
-- 
1.8.3.1

Re: [PATCH 00/13] mmc: bcm2835: more cleanups

[Ulf Hansson]

On 27 January 2017 at 00:37, Gerd Hoffmann <kraxel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>   Hi folks,
>
> Continuing bcm283x mmc driver cleanup.  Changes:
>
>  * switch to threaded irq handler.
>  * handle timeouts using delayed work instead of timers.
>
> With these changes most of the driver code runs in
> thread context, which in turn allows to remove the
> work queue and the tasklet.  Also some wait loops can
> be simplified because almost all driver code is allowed
> to sleep now.
>
> Code is also available here:
>   git://git.kraxel.org/linux bcm2837-sdhost-cleanup
>
> Baseline for the patch series is this:
>   git://git.kraxel.org/linux bcm2837-wifi
>
> Tested on raspberry pi 2b only so far.
> PIO mode only, didn't try to enable DMA.
>
> Lots of baby patches for now, to make review
> and (if needed) bisecting easier.  Will all be
> squashed into the bcm283x mmc driver patch later.
>
> cheers,
>   Gerd
>
> Gerd Hoffmann (13):
>   mmc: bcm2835: add bcm2835_read_wait_sdcmd
>   mmc: bcm2835: use bcm2835_read_wait_sdcmd in bcm2835_send_command
>   mmc: bcm2835: add bcm2835_threaded_irq
>   mmc: bcm2835: add bcm2835_check_data_error
>   mmc: bcm2835: call bcm2835_block_irq from irqthread
>   mmc: bcm2835: add bcm2835_check_cmd_error
>   mmc: bcm2835: call bcm2835_busy_irq from irqthread
>   mmc: bcm2835: split bcm2835_data_irq
>   mmc: bcm2835: switch locking to mutex
>   mmc: bcm2835: work queue is dead code now, zap
>   mmc: bcm2835: kill tasklet
>   mmc: bcm2835: move timeout to thread context
>   mmc: bcm2835: use bcm2835_read_wait_sdcmd in bcm2835_finish_command
>
>  drivers/mmc/host/bcm2835.c | 448 ++++++++++++++++++++-------------------------
>  1 file changed, 197 insertions(+), 251 deletions(-)
>
> --
> 1.8.3.1
>

So, I think these kind of incremental changes on top of an old posted
series are just confusing. Please just create a new version of the
original series - and add a nice history of what changes have been
made for each step.

Moreover, I know I have asked before why sdhci-iproc can be extended
for this controller. However, from the responses to some of the
changes in this series it sounds like the controllers are related? Or
are the controllers completely independent designed HW? Please clarify
this once more.

Kind regards
Uffe

Re: [PATCH 00/13] mmc: bcm2835: more cleanups

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 2630 bytes --]

Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> writes:

> On 27 January 2017 at 00:37, Gerd Hoffmann <kraxel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>>   Hi folks,
>>
>> Continuing bcm283x mmc driver cleanup.  Changes:
>>
>>  * switch to threaded irq handler.
>>  * handle timeouts using delayed work instead of timers.
>>
>> With these changes most of the driver code runs in
>> thread context, which in turn allows to remove the
>> work queue and the tasklet.  Also some wait loops can
>> be simplified because almost all driver code is allowed
>> to sleep now.
>>
>> Code is also available here:
>>   git://git.kraxel.org/linux bcm2837-sdhost-cleanup
>>
>> Baseline for the patch series is this:
>>   git://git.kraxel.org/linux bcm2837-wifi
>>
>> Tested on raspberry pi 2b only so far.
>> PIO mode only, didn't try to enable DMA.
>>
>> Lots of baby patches for now, to make review
>> and (if needed) bisecting easier.  Will all be
>> squashed into the bcm283x mmc driver patch later.
>>
>> cheers,
>>   Gerd
>>
>> Gerd Hoffmann (13):
>>   mmc: bcm2835: add bcm2835_read_wait_sdcmd
>>   mmc: bcm2835: use bcm2835_read_wait_sdcmd in bcm2835_send_command
>>   mmc: bcm2835: add bcm2835_threaded_irq
>>   mmc: bcm2835: add bcm2835_check_data_error
>>   mmc: bcm2835: call bcm2835_block_irq from irqthread
>>   mmc: bcm2835: add bcm2835_check_cmd_error
>>   mmc: bcm2835: call bcm2835_busy_irq from irqthread
>>   mmc: bcm2835: split bcm2835_data_irq
>>   mmc: bcm2835: switch locking to mutex
>>   mmc: bcm2835: work queue is dead code now, zap
>>   mmc: bcm2835: kill tasklet
>>   mmc: bcm2835: move timeout to thread context
>>   mmc: bcm2835: use bcm2835_read_wait_sdcmd in bcm2835_finish_command
>>
>>  drivers/mmc/host/bcm2835.c | 448 ++++++++++++++++++++-------------------------
>>  1 file changed, 197 insertions(+), 251 deletions(-)
>>
>> --
>> 1.8.3.1
>>
>
> So, I think these kind of incremental changes on top of an old posted
> series are just confusing. Please just create a new version of the
> original series - and add a nice history of what changes have been
> made for each step.
>
> Moreover, I know I have asked before why sdhci-iproc can be extended
> for this controller. However, from the responses to some of the
> changes in this series it sounds like the controllers are related? Or
> are the controllers completely independent designed HW? Please clarify
> this once more.

There are two completely separate, unrelated controllers.  We also have
two devices we need to drive.  Thus we need to pinmux one controller to
one device, and the other controller to the other device.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 206 bytes --]

_______________________________________________
linux-rpi-kernel mailing list
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
http://lists.infradead.org/mailman/listinfo/linux-rpi-kernel