* [RFC 1/3] mmc: host: tmio: refactor calls to sdio irq
2017-01-05 11:43 [RFC 0/3] mmc: host: tmio: sdio irq improvements Wolfram Sang
@ 2017-01-05 11:43 ` Wolfram Sang
2017-01-10 8:05 ` Simon Horman
2017-01-05 11:43 ` [RFC 2/3] mmc: host: tmio: SDIO_STATUS_QUIRK is rather SDIO_STATUS_SETBITS Wolfram Sang
2017-01-05 11:43 ` [RFC 3/3] mmc: tmio: discard obsolete SDIO irqs before enabling irqs Wolfram Sang
2 siblings, 1 reply; 9+ messages in thread
From: Wolfram Sang @ 2017-01-05 11:43 UTC (permalink / raw)
To: linux-mmc; +Cc: linux-renesas-soc, Ulf Hansson, Wolfram Sang
tmio_mmc_sdio_irq() is not used as a seperate irq handler anymore, so we
can make it similar to the other irq helper functions, namely:
* only give the host as argument function which is what it really needs
* prefix function name with __
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/mmc/host/tmio_mmc_pio.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 3ca97f3c307975..60dc43b4574117 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -709,9 +709,8 @@ static bool __tmio_mmc_sdcard_irq(struct tmio_mmc_host *host,
return false;
}
-static void tmio_mmc_sdio_irq(int irq, void *devid)
+static void __tmio_mmc_sdio_irq(struct tmio_mmc_host *host)
{
- struct tmio_mmc_host *host = devid;
struct mmc_host *mmc = host->mmc;
struct tmio_mmc_data *pdata = host->pdata;
unsigned int ireg, status;
@@ -752,7 +751,7 @@ irqreturn_t tmio_mmc_irq(int irq, void *devid)
if (__tmio_mmc_sdcard_irq(host, ireg, status))
return IRQ_HANDLED;
- tmio_mmc_sdio_irq(irq, devid);
+ __tmio_mmc_sdio_irq(host);
return IRQ_HANDLED;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [RFC 1/3] mmc: host: tmio: refactor calls to sdio irq
2017-01-05 11:43 ` [RFC 1/3] mmc: host: tmio: refactor calls to sdio irq Wolfram Sang
@ 2017-01-10 8:05 ` Simon Horman
0 siblings, 0 replies; 9+ messages in thread
From: Simon Horman @ 2017-01-10 8:05 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-mmc, linux-renesas-soc, Ulf Hansson
On Thu, Jan 05, 2017 at 12:43:24PM +0100, Wolfram Sang wrote:
> tmio_mmc_sdio_irq() is not used as a seperate irq handler anymore, so we
> can make it similar to the other irq helper functions, namely:
>
> * only give the host as argument function which is what it really needs
> * prefix function name with __
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
I like that this avoids implicitly casting to and from void *.
Acked-by: Simon Horman <horms+renesas@verge.net.au>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [RFC 2/3] mmc: host: tmio: SDIO_STATUS_QUIRK is rather SDIO_STATUS_SETBITS
2017-01-05 11:43 [RFC 0/3] mmc: host: tmio: sdio irq improvements Wolfram Sang
2017-01-05 11:43 ` [RFC 1/3] mmc: host: tmio: refactor calls to sdio irq Wolfram Sang
@ 2017-01-05 11:43 ` Wolfram Sang
2017-01-10 8:07 ` Simon Horman
2017-01-05 11:43 ` [RFC 3/3] mmc: tmio: discard obsolete SDIO irqs before enabling irqs Wolfram Sang
2 siblings, 1 reply; 9+ messages in thread
From: Wolfram Sang @ 2017-01-05 11:43 UTC (permalink / raw)
To: linux-mmc; +Cc: linux-renesas-soc, Ulf Hansson, Wolfram Sang
QUIRK sounds like there is something wrong, but actually there are just
some bits which need to be 1. Rename it to be more clear.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/mmc/host/sh_mobile_sdhi.c | 6 ++----
drivers/mmc/host/tmio_mmc_pio.c | 2 +-
include/linux/mfd/tmio.h | 2 +-
3 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 59db14b4827c33..1600fefc2c3e2c 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -641,10 +641,8 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
*/
mmc_data->flags |= TMIO_MMC_HAVE_CMD12_CTRL;
- /*
- * All SDHI need SDIO_INFO1 reserved bit
- */
- mmc_data->flags |= TMIO_MMC_SDIO_STATUS_QUIRK;
+ /* All SDHI have reserved bits */
+ mmc_data->flags |= TMIO_MMC_SDIO_STATUS_SETBITS;
ret = tmio_mmc_host_probe(host, mmc_data);
if (ret < 0)
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 60dc43b4574117..d6033620a45c12 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -723,7 +723,7 @@ static void __tmio_mmc_sdio_irq(struct tmio_mmc_host *host)
ireg = status & TMIO_SDIO_MASK_ALL & ~host->sdio_irq_mask;
sdio_status = status & ~TMIO_SDIO_MASK_ALL;
- if (pdata->flags & TMIO_MMC_SDIO_STATUS_QUIRK)
+ if (pdata->flags & TMIO_MMC_SDIO_STATUS_SETBITS)
sdio_status |= 6;
sd_ctrl_write16(host, CTL_SDIO_STATUS, sdio_status);
diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
index fba44abd05ba1a..5a05a34cc1a292 100644
--- a/include/linux/mfd/tmio.h
+++ b/include/linux/mfd/tmio.h
@@ -97,7 +97,7 @@
/*
* Some controllers needs to set 1 on SDIO status reserved bits
*/
-#define TMIO_MMC_SDIO_STATUS_QUIRK (1 << 8)
+#define TMIO_MMC_SDIO_STATUS_SETBITS (1 << 8)
/*
* Some controllers have a 32-bit wide data port register
--
2.11.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [RFC 2/3] mmc: host: tmio: SDIO_STATUS_QUIRK is rather SDIO_STATUS_SETBITS
2017-01-05 11:43 ` [RFC 2/3] mmc: host: tmio: SDIO_STATUS_QUIRK is rather SDIO_STATUS_SETBITS Wolfram Sang
@ 2017-01-10 8:07 ` Simon Horman
2017-01-19 19:40 ` Wolfram Sang
0 siblings, 1 reply; 9+ messages in thread
From: Simon Horman @ 2017-01-10 8:07 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-mmc, linux-renesas-soc, Ulf Hansson
On Thu, Jan 05, 2017 at 12:43:25PM +0100, Wolfram Sang wrote:
> QUIRK sounds like there is something wrong, but actually there are just
> some bits which need to be 1. Rename it to be more clear.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> drivers/mmc/host/sh_mobile_sdhi.c | 6 ++----
> drivers/mmc/host/tmio_mmc_pio.c | 2 +-
> include/linux/mfd/tmio.h | 2 +-
> 3 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> index 59db14b4827c33..1600fefc2c3e2c 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -641,10 +641,8 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
> */
> mmc_data->flags |= TMIO_MMC_HAVE_CMD12_CTRL;
>
> - /*
> - * All SDHI need SDIO_INFO1 reserved bit
> - */
> - mmc_data->flags |= TMIO_MMC_SDIO_STATUS_QUIRK;
> + /* All SDHI have reserved bits */
> + mmc_data->flags |= TMIO_MMC_SDIO_STATUS_SETBITS;
>
> ret = tmio_mmc_host_probe(host, mmc_data);
> if (ret < 0)
> diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
> index 60dc43b4574117..d6033620a45c12 100644
> --- a/drivers/mmc/host/tmio_mmc_pio.c
> +++ b/drivers/mmc/host/tmio_mmc_pio.c
> @@ -723,7 +723,7 @@ static void __tmio_mmc_sdio_irq(struct tmio_mmc_host *host)
> ireg = status & TMIO_SDIO_MASK_ALL & ~host->sdio_irq_mask;
>
> sdio_status = status & ~TMIO_SDIO_MASK_ALL;
> - if (pdata->flags & TMIO_MMC_SDIO_STATUS_QUIRK)
> + if (pdata->flags & TMIO_MMC_SDIO_STATUS_SETBITS)
> sdio_status |= 6;
>
> sd_ctrl_write16(host, CTL_SDIO_STATUS, sdio_status);
> diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
> index fba44abd05ba1a..5a05a34cc1a292 100644
> --- a/include/linux/mfd/tmio.h
> +++ b/include/linux/mfd/tmio.h
> @@ -97,7 +97,7 @@
> /*
> * Some controllers needs to set 1 on SDIO status reserved bits
> */
Is the following update to the above appropriate?
In particular, is it some or all?
* Controllers need to set 1 on SDIO status reserved bits
> -#define TMIO_MMC_SDIO_STATUS_QUIRK (1 << 8)
> +#define TMIO_MMC_SDIO_STATUS_SETBITS (1 << 8)
>
> /*
> * Some controllers have a 32-bit wide data port register
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC 2/3] mmc: host: tmio: SDIO_STATUS_QUIRK is rather SDIO_STATUS_SETBITS
2017-01-10 8:07 ` Simon Horman
@ 2017-01-19 19:40 ` Wolfram Sang
0 siblings, 0 replies; 9+ messages in thread
From: Wolfram Sang @ 2017-01-19 19:40 UTC (permalink / raw)
To: Simon Horman; +Cc: Wolfram Sang, linux-mmc, linux-renesas-soc, Ulf Hansson
> > --- a/include/linux/mfd/tmio.h
> > +++ b/include/linux/mfd/tmio.h
> > @@ -97,7 +97,7 @@
> > /*
> > * Some controllers needs to set 1 on SDIO status reserved bits
> > */
>
> Is the following update to the above appropriate?
> In particular, is it some or all?
Some.
> * Controllers need to set 1 on SDIO status reserved bits
I updated this and the other comment to be more precise. Thanks!
^ permalink raw reply [flat|nested] 9+ messages in thread
* [RFC 3/3] mmc: tmio: discard obsolete SDIO irqs before enabling irqs
2017-01-05 11:43 [RFC 0/3] mmc: host: tmio: sdio irq improvements Wolfram Sang
2017-01-05 11:43 ` [RFC 1/3] mmc: host: tmio: refactor calls to sdio irq Wolfram Sang
2017-01-05 11:43 ` [RFC 2/3] mmc: host: tmio: SDIO_STATUS_QUIRK is rather SDIO_STATUS_SETBITS Wolfram Sang
@ 2017-01-05 11:43 ` Wolfram Sang
2017-01-10 8:09 ` Simon Horman
2 siblings, 1 reply; 9+ messages in thread
From: Wolfram Sang @ 2017-01-05 11:43 UTC (permalink / raw)
To: linux-mmc; +Cc: linux-renesas-soc, Ulf Hansson, Wolfram Sang
Before enabling SDIO irqs, clear the status bit, so we discard old and
stale interrupts. Needed to get two wireless cards working.
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/mmc/host/tmio_mmc_pio.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index d6033620a45c12..ebe3e12f0083dd 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -134,12 +134,21 @@ static void tmio_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
struct tmio_mmc_host *host = mmc_priv(mmc);
if (enable && !host->sdio_irq_enabled) {
+ u16 sdio_status;
+
/* Keep device active while SDIO irq is enabled */
pm_runtime_get_sync(mmc_dev(mmc));
- host->sdio_irq_enabled = true;
+ host->sdio_irq_enabled = true;
host->sdio_irq_mask = TMIO_SDIO_MASK_ALL &
~TMIO_SDIO_STAT_IOIRQ;
+
+ /* Clear obsolete interrupts before enabling */
+ sdio_status = sd_ctrl_read16(host, CTL_SDIO_STATUS) & ~TMIO_SDIO_MASK_ALL;
+ if (host->pdata->flags & TMIO_MMC_SDIO_STATUS_SETBITS)
+ sdio_status |= 6;
+ sd_ctrl_write16(host, CTL_SDIO_STATUS, sdio_status);
+
sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask);
} else if (!enable && host->sdio_irq_enabled) {
host->sdio_irq_mask = TMIO_SDIO_MASK_ALL;
--
2.11.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [RFC 3/3] mmc: tmio: discard obsolete SDIO irqs before enabling irqs
2017-01-05 11:43 ` [RFC 3/3] mmc: tmio: discard obsolete SDIO irqs before enabling irqs Wolfram Sang
@ 2017-01-10 8:09 ` Simon Horman
2017-01-19 19:42 ` Wolfram Sang
0 siblings, 1 reply; 9+ messages in thread
From: Simon Horman @ 2017-01-10 8:09 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-mmc, linux-renesas-soc, Ulf Hansson
On Thu, Jan 05, 2017 at 12:43:26PM +0100, Wolfram Sang wrote:
> Before enabling SDIO irqs, clear the status bit, so we discard old and
> stale interrupts. Needed to get two wireless cards working.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> drivers/mmc/host/tmio_mmc_pio.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
> index d6033620a45c12..ebe3e12f0083dd 100644
> --- a/drivers/mmc/host/tmio_mmc_pio.c
> +++ b/drivers/mmc/host/tmio_mmc_pio.c
> @@ -134,12 +134,21 @@ static void tmio_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
> struct tmio_mmc_host *host = mmc_priv(mmc);
>
> if (enable && !host->sdio_irq_enabled) {
> + u16 sdio_status;
> +
> /* Keep device active while SDIO irq is enabled */
> pm_runtime_get_sync(mmc_dev(mmc));
> - host->sdio_irq_enabled = true;
>
> + host->sdio_irq_enabled = true;
> host->sdio_irq_mask = TMIO_SDIO_MASK_ALL &
> ~TMIO_SDIO_STAT_IOIRQ;
> +
> + /* Clear obsolete interrupts before enabling */
> + sdio_status = sd_ctrl_read16(host, CTL_SDIO_STATUS) & ~TMIO_SDIO_MASK_ALL;
> + if (host->pdata->flags & TMIO_MMC_SDIO_STATUS_SETBITS)
> + sdio_status |= 6;
Perhaps a #define would be an improvement over "6".
> + sd_ctrl_write16(host, CTL_SDIO_STATUS, sdio_status);
> +
> sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask);
> } else if (!enable && host->sdio_irq_enabled) {
> host->sdio_irq_mask = TMIO_SDIO_MASK_ALL;
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread