patches to enable SDIO interrupt support for omap_hsmmc for v3.13
Subsystems (2)
cover patches to enable SDIO interrupt support for omap_hsmmc for v3.13
Hi all,
Here are some patches to enable the SDIO interrupt support for omap_hsmmc. Note that this series does not do the remux, but relies on working io chain wake-up events. So omaps that do require remuxing for wake-up events need more work, but let's do this first.
Regards,
Tony
---
Tony Lindgren (2):
mmc: omap_hsmmc: Fix context save and restore for DT
mmc: omap_hsmmc: Enable SDIO interrupt
drivers/mmc/host/omap_hsmmc.c | 116 +++++++++++++++++++++++++++-------------- 1 file changed, 75 insertions(+), 41 deletions(-)
Signature · 1 line
Signature
PATCH 1/2 mmc: omap_hsmmc: Fix context save and restore for DT
We want to get rid of the omap specific platform init code callbacks as they don't play nice with device tree.
Let's convert the context loss check to be based on a register state detection instead.
--- drivers/mmc/host/omap_hsmmc.c | 41 ++++++++--------------------------------- 1 file changed, 8 insertions(+), 33 deletions(-)
drivers/mmc/host/omap_hsmmc.c +8 -33
@@ -171,19 +171,19 @@ struct omap_hsmmc_host {unsigned char bus_mode;unsigned char power_mode;int suspended; +u32 hctl; +u32 capa;int irq;int use_dma, dma_ch;struct dma_chan *tx_chan;struct dma_chan *rx_chan;int slot_id;int response_busy; -int context_loss;int protect_card;int reqs_blocked;int use_reg;int req_in_progress;struct omap_hsmmc_next next_data; -struct omap_mmc_platform_data *pdata; };
@@ -597,25 +597,16 @@ static void omap_hsmmc_set_bus_mode(struct omap_hsmmc_host *host)static int omap_hsmmc_context_restore (struct omap_hsmmc_host *host) {struct mmc_ios *ios = &host->mmc->ios; -struct omap_mmc_platform_data *pdata = host->pdata; -int context_loss =0 ;u32 hctl, capa;unsigned long timeout; -if (pdata->get_context_loss_count) { - context_loss = pdata->get_context_loss_count (host->dev); -if (context_loss <0 ) -return 1 ; - } - -dev_dbg (mmc_dev (host->mmc),"context was %slost\n" , - context_loss == host->context_loss ?"not " :"" ); -if (host->context_loss == context_loss) -return 1 ; -if (!OMAP_HSMMC_READ (host->base,SYSSTATUS ) &RESETDONE )return 1 ; +if (host->hctl ==OMAP_HSMMC_READ (host->base,HCTL ) && + host->capa ==OMAP_HSMMC_READ (host->base,CAPA )) +return 0 ; +if (host->pdata->controller_flags &OMAP_HSMMC_SUPPORTS_DUAL_VOLT ) {if (host->power_mode !=MMC_POWER_OFF && (1 << ios->vdd) <=MMC_VDD_23_24 )
@@ -655,8 +646,6 @@ static int omap_hsmmc_context_restore(struct omap_hsmmc_host *host)omap_hsmmc_set_bus_mode (host); out: - host->context_loss = context_loss; -dev_dbg (mmc_dev (host->mmc),"context is restored\n" );return 0 ; }
@@ -666,15 +655,8 @@ out:*/ static void omap_hsmmc_context_save (struct omap_hsmmc_host *host) { -struct omap_mmc_platform_data *pdata = host->pdata; -int context_loss; - -if (pdata->get_context_loss_count) { - context_loss = pdata->get_context_loss_count (host->dev); -if (context_loss <0 ) -return ; - host->context_loss = context_loss; - } + host->hctl =OMAP_HSMMC_READ (host->base,HCTL ); + host->capa =OMAP_HSMMC_READ (host->base,CAPA ); }#else
@@ -1635,13 +1617,6 @@ static int omap_hsmmc_regs_show(struct seq_file *s, void *data) {struct mmc_host *mmc = s->private;struct omap_hsmmc_host *host =mmc_priv (mmc); -int context_loss =0 ; - -if (host->pdata->get_context_loss_count) - context_loss = host->pdata->get_context_loss_count (host->dev); - -seq_printf (s,"mmc%d:\n ctx_loss:\t%d:%d\n\nregs:\n" , - mmc->index, host->context_loss, context_loss);if (host->suspended) {seq_printf (s,"host suspended, can't read registers\n" );
PATCH 2/2 mmc: omap_hsmmc: Enable SDIO interrupt
In reply to Tony LindgrenThere have been various patches floating around for enabling the SDIO IRQ for hsmmc, but none of them ever got merged.
Probably the reason for not merging the SDIO interrupt patches has been the lack of wake-up path for SDIO on some omaps that has also needed remuxing the SDIO DAT1 line to a GPIO making the patches complex.
This patch adds the minimal SDIO IRQ support to hsmmc for omaps that do have the wake-up path. For those omaps, the DAT1 line need to have the wake-up enable bit set, and the wake-up interrupt is the same as for the MMC controller.
This patch has been tested on am3730 es1.2 with wl12xx connected to MMC2 with wl12xx waking to Ethernet traffic from off-idle mode. Note that for omaps that do not have the SDIO wake-up path, this patch will not work for idle modes and further patches for remuxing DAT1 to GPIO are needed.
Based on earlier patches [1][2] by David Vrabel <[email protected]>, Steve Sakoman <[email protected]> and Andreas Fenkart <[email protected]> with the SDIO IRQ handing improved following how sdhci.c is doing it.
[1] http://www.sakoman.com/cgi-bin/gitweb.cgi?p=linux.git;a=commitdiff_plain;h=010810d22f6f49ac03da4ba384969432e0320453
[2] http://comments.gmane.org/gmane.linux.kernel.mmc/20446
--- drivers/mmc/host/omap_hsmmc.c | 75 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 67 insertions(+), 8 deletions(-)
drivers/mmc/host/omap_hsmmc.c +67 -8
@@ -103,6 +103,7 @@#define TC_EN (1 << 1)#define BWR_EN (1 << 4)#define BRR_EN (1 << 5) +#define CIRQ_EN (1 << 8)#define ERR_EN (1 << 15)#define CTO_EN (1 << 16)#define CCRC_EN (1 << 17)
@@ -183,6 +184,11 @@ struct omap_hsmmc_host {int reqs_blocked;int use_reg;int req_in_progress; + +int flags; +#define HSMMC_RUNTIME_SUSPENDED (1 << 0)/* Runtime suspended */ +#define HSMMC_SDIO_IRQ_ENABLED (1 << 1)/* SDIO irq enabled */ +struct omap_hsmmc_next next_data;struct omap_mmc_platform_data *pdata; };
@@ -463,27 +469,34 @@ static void omap_hsmmc_stop_clock(struct omap_hsmmc_host *host)static void omap_hsmmc_enable_irq (struct omap_hsmmc_host *host,struct mmc_command *cmd) { -unsigned int irq_mask; +u32 irq_mask =INT_EN_MASK ; +unsigned long flags;if (host->use_dma) - irq_mask =INT_EN_MASK & ~(BRR_EN |BWR_EN ); -else - irq_mask =INT_EN_MASK ; + irq_mask &= ~(BRR_EN |BWR_EN );/* Disable timeout for erases */ if (cmd->opcode ==MMC_ERASE ) irq_mask &= ~DTO_EN ; +spin_lock_irqsave (&host->irq_lock, flags);OMAP_HSMMC_WRITE (host->base,STAT ,STAT_CLEAR );OMAP_HSMMC_WRITE (host->base,ISE , irq_mask); +if (host->flags &HSMMC_SDIO_IRQ_ENABLED ) + irq_mask |=CIRQ_EN ;OMAP_HSMMC_WRITE (host->base,IE , irq_mask); +spin_unlock_irqrestore (&host->irq_lock, flags); }static void omap_hsmmc_disable_irq (struct omap_hsmmc_host *host) { +unsigned long flags; + +spin_lock_irqsave (&host->irq_lock, flags);OMAP_HSMMC_WRITE (host->base,ISE ,0 );OMAP_HSMMC_WRITE (host->base,IE ,0 );OMAP_HSMMC_WRITE (host->base,STAT ,STAT_CLEAR ); +spin_unlock_irqrestore (&host->irq_lock, flags); }/* Calculate divisor for the given clock frequency */
@@ -1040,8 +1053,12 @@ static irqreturn_t omap_hsmmc_irq(int irq, void *dev_id)int status; status =OMAP_HSMMC_READ (host->base,STAT ); -while (status &INT_EN_MASK && host->req_in_progress) { -omap_hsmmc_do_irq (host, status); +while (status & (INT_EN_MASK |CIRQ_EN )) { +if (host->req_in_progress) +omap_hsmmc_do_irq (host, status); + +if (status &CIRQ_EN ) +mmc_signal_sdio_irq (host->mmc);/* Flush posted write */ status =OMAP_HSMMC_READ (host->base,STAT );
@@ -1556,6 +1573,43 @@ static void omap_hsmmc_init_card(struct mmc_host *mmc, struct mmc_card *card)mmc_slot (host).init_card (card); } +static void omap_hsmmc_enable_sdio_irq_nolock (struct omap_hsmmc_host *host, +int enable) +{ +u32 irq_mask; + +if (enable) + host->flags |=HSMMC_SDIO_IRQ_ENABLED ; +else + host->flags &= ~HSMMC_SDIO_IRQ_ENABLED ; + +/* SDIO IRQ will be enabled as appropriate in runtime resume */ +if (host->flags &HSMMC_RUNTIME_SUSPENDED ) + goto out; + + irq_mask =OMAP_HSMMC_READ (host->base,IE ); +if (enable) + irq_mask |=CIRQ_EN ; +else + irq_mask &= ~CIRQ_EN ; +OMAP_HSMMC_WRITE (host->base,IE , irq_mask); +if (!host->req_in_progress) +OMAP_HSMMC_WRITE (host->base,ISE , irq_mask); + +out: +OMAP_HSMMC_READ (host->base,IE ); +} + +static void omap_hsmmc_enable_sdio_irq (struct mmc_host *mmc,int enable) +{ +struct omap_hsmmc_host *host =mmc_priv (mmc); +unsigned long flags; + +spin_lock_irqsave (&host->irq_lock, flags); +omap_hsmmc_enable_sdio_irq_nolock (host, enable); +spin_unlock_irqrestore (&host->irq_lock, flags); +} +static void omap_hsmmc_conf_bus_power (struct omap_hsmmc_host *host) {u32 hctl, capa, value;
@@ -1608,7 +1662,7 @@ static const struct mmc_host_ops omap_hsmmc_ops = { .get_cd = omap_hsmmc_get_cd, .get_ro = omap_hsmmc_get_ro, .init_card = omap_hsmmc_init_card, -/* NYET -- enable_sdio_irq */ + .enable_sdio_irq = omap_hsmmc_enable_sdio_irq, };#ifdef CONFIG_DEBUG_FS
@@ -1874,7 +1928,8 @@ static int omap_hsmmc_probe(struct platform_device *pdev) mmc->max_seg_size = mmc->max_req_size; mmc->caps |=MMC_CAP_MMC_HIGHSPEED |MMC_CAP_SD_HIGHSPEED | -MMC_CAP_WAIT_WHILE_BUSY |MMC_CAP_ERASE ; +MMC_CAP_WAIT_WHILE_BUSY |MMC_CAP_ERASE | +MMC_CAP_SDIO_IRQ ; mmc->caps |=mmc_slot (host).caps;if (mmc->caps &MMC_CAP_8_BIT_DATA )
@@ -2172,6 +2227,7 @@ static int omap_hsmmc_runtime_suspend(struct device *dev) host =platform_get_drvdata (to_platform_device (dev));omap_hsmmc_context_save (host); + host->flags |=HSMMC_RUNTIME_SUSPENDED ;dev_dbg (dev,"disabled\n" );return 0 ;
@@ -2183,6 +2239,9 @@ static int omap_hsmmc_runtime_resume(struct device *dev) host =platform_get_drvdata (to_platform_device (dev));omap_hsmmc_context_restore (host); + host->flags &= ~HSMMC_RUNTIME_SUSPENDED ; +if ((host->flags &HSMMC_SDIO_IRQ_ENABLED )) +omap_hsmmc_enable_sdio_irq_nolock (host, true);dev_dbg (dev,"enabled\n" );return 0 ;
PATCH 2/2 mmc: omap_hsmmc: Enable SDIO interrupt
2013/9/26 Tony Lindgren <[email protected]>:
Quoted 85 lines
There have been various patches floating around for enabling the SDIO IRQ for hsmmc, but none of them ever got merged.
Probably the reason for not merging the SDIO interrupt patches has been the lack of wake-up path for SDIO on some omaps that has also needed remuxing the SDIO DAT1 line to a GPIO making the patches complex.
This patch adds the minimal SDIO IRQ support to hsmmc for omaps that do have the wake-up path. For those omaps, the DAT1 line need to have the wake-up enable bit set, and the wake-up interrupt is the same as for the MMC controller.
This patch has been tested on am3730 es1.2 with wl12xx connected to MMC2 with wl12xx waking to Ethernet traffic from off-idle mode. Note that for omaps that do not have the SDIO wake-up path, this patch will not work for idle modes and further patches for remuxing DAT1 to GPIO are needed.
Based on earlier patches [1][2] by David Vrabel <[email protected]>, Steve Sakoman <[email protected]> and Andreas Fenkart <[email protected]> with the SDIO IRQ handing improved following how
sdhci.cis doing it.[1] http://www.sakoman.com/cgi-bin/gitweb.cgi?p=linux.git;a=commitdiff_plain;h=010810d22f6f49ac03da4ba384969432e0320453
[2] http://comments.gmane.org/gmane.linux.kernel.mmc/20446Cc Andreas Fenkart <[email protected]>Cc Balaji T K <[email protected]>Signed-off-by Tony Lindgren <[email protected]>--- drivers/mmc/host/omap_hsmmc.c | 75 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 67 insertions(+), 8 deletions(-)drivers/mmc/host/omap_hsmmc.c +13
-4@@ -103,6 +103,7 @@#define TC_EN (1 << 1)#define BWR_EN (1 << 4)#define BRR_EN (1 << 5) +#define CIRQ_EN (1 << 8)#define ERR_EN (1 << 15)#define CTO_EN (1 << 16)#define CCRC_EN (1 << 17)@@ -183,6 +184,11 @@ struct omap_hsmmc_host {int reqs_blocked;int use_reg;int req_in_progress; + +int flags; +#define HSMMC_RUNTIME_SUSPENDED (1 << 0)/* Runtime suspended */ +#define HSMMC_SDIO_IRQ_ENABLED (1 << 1)/* SDIO irq enabled */ +struct omap_hsmmc_next next_data;struct omap_mmc_platform_data *pdata; };@@ -463,27 +469,34 @@ static void omap_hsmmc_stop_clock(struct omap_hsmmc_host *host)static void omap_hsmmc_enable_irq (struct omap_hsmmc_host *host,struct mmc_command *cmd) { -unsigned int irq_mask; +u32 irq_mask =INT_EN_MASK ; +unsigned long flags;if (host->use_dma) - irq_mask =INT_EN_MASK & ~(BRR_EN |BWR_EN ); -else - irq_mask =INT_EN_MASK ; + irq_mask &= ~(BRR_EN |BWR_EN );/* Disable timeout for erases */ if (cmd->opcode ==MMC_ERASE ) irq_mask &= ~DTO_EN ; +spin_lock_irqsave (&host->irq_lock, flags);OMAP_HSMMC_WRITE (host->base,STAT ,STAT_CLEAR );OMAP_HSMMC_WRITE (host->base,ISE , irq_mask); +if (host->flags &HSMMC_SDIO_IRQ_ENABLED ) + irq_mask |=CIRQ_EN ;OMAP_HSMMC_WRITE (host->base,IE , irq_mask); +spin_unlock_irqrestore (&host->irq_lock, flags);
drivers/mmc/host/omap_hsmmc.c static void omap_hsmmc_stop_clock(struct omap_hsmmc_host *host) · line 498
Quoted 10 lines
drivers/mmc/host/omap_hsmmc.c +3
-0}static void omap_hsmmc_disable_irq (struct omap_hsmmc_host *host) { +unsigned long flags; + +spin_lock_irqsave (&host->irq_lock, flags);OMAP_HSMMC_WRITE (host->base,ISE ,0 );OMAP_HSMMC_WRITE (host->base,IE ,0 );OMAP_HSMMC_WRITE (host->base,STAT ,STAT_CLEAR );
This is wrong. SDIO IRQ must not be disabled upon host initiated transfer. see omap_hsmmc_request_done
Quoted 26 lines
drivers/mmc/host/omap_hsmmc.c +10
-2+spin_unlock_irqrestore (&host->irq_lock, flags); }/* Calculate divisor for the given clock frequency */ @@ -1040,8 +1053,12 @@ static irqreturn_t omap_hsmmc_irq(int irq, void *dev_id)int status; status =OMAP_HSMMC_READ (host->base,STAT ); -while (status &INT_EN_MASK && host->req_in_progress) { -omap_hsmmc_do_irq (host, status); +while (status & (INT_EN_MASK |CIRQ_EN )) { +if (host->req_in_progress) +omap_hsmmc_do_irq (host, status); + +if (status &CIRQ_EN ) +mmc_signal_sdio_irq (host->mmc);/* Flush posted write */ status =OMAP_HSMMC_READ (host->base,STAT );@@ -1556,6 +1573,43 @@ static void omap_hsmmc_init_card(struct mmc_host *mmc, struct mmc_card *card)mmc_slot (host).init_card (card); } +static void omap_hsmmc_enable_sdio_irq_nolock (struct omap_hsmmc_host *host, +int enable) +{
drivers/mmc/host/omap_hsmmc.c static void omap_hsmmc_init_card(struct mmc_host *mmc, struct mmc_card *card) · line 1588
Quoted 10 lines
drivers/mmc/host/omap_hsmmc.c +10
-0+u32 irq_mask; + +if (enable) + host->flags |=HSMMC_SDIO_IRQ_ENABLED ; +else + host->flags &= ~HSMMC_SDIO_IRQ_ENABLED ; + +/* SDIO IRQ will be enabled as appropriate in runtime resume */ +if (host->flags &HSMMC_RUNTIME_SUSPENDED ) + goto out;
Not sure here, will the module still come out of suspend even with SDIO IRQ disabled?
Otherwise nak, sdio irq must enabled independent of pm runtime state. In the worst case need pm_runtime_get().
Quoted 28 lines
drivers/mmc/host/omap_hsmmc.c +24
-0+ + irq_mask =OMAP_HSMMC_READ (host->base,IE ); +if (enable) + irq_mask |=CIRQ_EN ; +else + irq_mask &= ~CIRQ_EN ; +OMAP_HSMMC_WRITE (host->base,IE , irq_mask); +if (!host->req_in_progress) +OMAP_HSMMC_WRITE (host->base,ISE , irq_mask); + +out: +OMAP_HSMMC_READ (host->base,IE ); +} + +static void omap_hsmmc_enable_sdio_irq (struct mmc_host *mmc,int enable) +{ +struct omap_hsmmc_host *host =mmc_priv (mmc); +unsigned long flags; + +spin_lock_irqsave (&host->irq_lock, flags); +omap_hsmmc_enable_sdio_irq_nolock (host, enable); +spin_unlock_irqrestore (&host->irq_lock, flags); +} +static void omap_hsmmc_conf_bus_power (struct omap_hsmmc_host *host) {u32 hctl, capa, value;
drivers/mmc/host/omap_hsmmc.c static const struct mmc_host_ops omap_hsmmc_ops = { · line 1665
Quoted 5 lines
drivers/mmc/host/omap_hsmmc.c +1
-1@@ -1608,7 +1662,7 @@ static const struct mmc_host_ops omap_hsmmc_ops = { .get_cd = omap_hsmmc_get_cd, .get_ro = omap_hsmmc_get_ro, .init_card = omap_hsmmc_init_card, -/* NYET -- enable_sdio_irq */ + .enable_sdio_irq = omap_hsmmc_enable_sdio_irq,
What about am335x, if this patch goes in, it will break that platform. quirk flag via device tree?
Quoted 32 lines
drivers/mmc/host/omap_hsmmc.c +6
-1};#ifdef CONFIG_DEBUG_FS @@ -1874,7 +1928,8 @@ static int omap_hsmmc_probe(struct platform_device *pdev) mmc->max_seg_size = mmc->max_req_size; mmc->caps |=MMC_CAP_MMC_HIGHSPEED |MMC_CAP_SD_HIGHSPEED | -MMC_CAP_WAIT_WHILE_BUSY |MMC_CAP_ERASE ; +MMC_CAP_WAIT_WHILE_BUSY |MMC_CAP_ERASE | +MMC_CAP_SDIO_IRQ ; mmc->caps |=mmc_slot (host).caps;if (mmc->caps &MMC_CAP_8_BIT_DATA )@@ -2172,6 +2227,7 @@ static int omap_hsmmc_runtime_suspend(struct device *dev) host =platform_get_drvdata (to_platform_device (dev));omap_hsmmc_context_save (host); + host->flags |=HSMMC_RUNTIME_SUSPENDED ;dev_dbg (dev,"disabled\n" );return 0 ;@@ -2183,6 +2239,9 @@ static int omap_hsmmc_runtime_resume(struct device *dev) host =platform_get_drvdata (to_platform_device (dev));omap_hsmmc_context_restore (host); + host->flags &= ~HSMMC_RUNTIME_SUSPENDED ; +if ((host->flags &HSMMC_SDIO_IRQ_ENABLED )) +omap_hsmmc_enable_sdio_irq_nolock (host, true);dev_dbg (dev,"enabled\n" );return 0 ;
PATCH 2/2 mmc: omap_hsmmc: Enable SDIO interrupt
Hi,
Quoted 28 lines · Andreas Fenkart
2013/9/26 Tony Lindgren <[email protected]>:
Quoted 27 lines
drivers/mmc/host/omap_hsmmc.c +7
-4@@ -463,27 +469,34 @@ static void omap_hsmmc_stop_clock(struct omap_hsmmc_host *host)static void omap_hsmmc_enable_irq (struct omap_hsmmc_host *host,struct mmc_command *cmd) { -unsigned int irq_mask; +u32 irq_mask =INT_EN_MASK ; +unsigned long flags;if (host->use_dma) - irq_mask =INT_EN_MASK & ~(BRR_EN |BWR_EN ); -else - irq_mask =INT_EN_MASK ; + irq_mask &= ~(BRR_EN |BWR_EN );/* Disable timeout for erases */ if (cmd->opcode ==MMC_ERASE ) irq_mask &= ~DTO_EN ; +spin_lock_irqsave (&host->irq_lock, flags);OMAP_HSMMC_WRITE (host->base,STAT ,STAT_CLEAR );OMAP_HSMMC_WRITE (host->base,ISE , irq_mask); +if (host->flags &HSMMC_SDIO_IRQ_ENABLED ) + irq_mask |=CIRQ_EN ;OMAP_HSMMC_WRITE (host->base,IE , irq_mask); +spin_unlock_irqrestore (&host->irq_lock, flags); }
Quoted 11 lines
drivers/mmc/host/omap_hsmmc.c static void omap_hsmmc_stop_clock(struct omap_hsmmc_host *host) · line 498
Quoted 8 lines
drivers/mmc/host/omap_hsmmc.c +3
-0static void omap_hsmmc_disable_irq (struct omap_hsmmc_host *host) { +unsigned long flags; + +spin_lock_irqsave (&host->irq_lock, flags);OMAP_HSMMC_WRITE (host->base,ISE ,0 );OMAP_HSMMC_WRITE (host->base,IE ,0 );OMAP_HSMMC_WRITE (host->base,STAT ,STAT_CLEAR );This is wrong. SDIO IRQ must not be disabled upon host initiated transfer. see
omap_hsmmc_request_done
Hmm I don't quite follow you here, are you saying that omap_hsmmc_request_done() should call this conditionally?
Or maybe even do a patch on top of this based on what you discovered with your earlier patches?
I know we don't have the remuxing support yet, but that has dependencies to the pending pinctrl wake-up handling in general, and pinctrl state grouping. So let's remove those dependencies for now and get the basics going first before moving on to the remuxing part :)
Quoted 33 lines
Quoted 33 lines
drivers/mmc/host/omap_hsmmc.c +17
-2+spin_unlock_irqrestore (&host->irq_lock, flags); }/* Calculate divisor for the given clock frequency */ @@ -1040,8 +1053,12 @@ static irqreturn_t omap_hsmmc_irq(int irq, void *dev_id)int status; status =OMAP_HSMMC_READ (host->base,STAT ); -while (status &INT_EN_MASK && host->req_in_progress) { -omap_hsmmc_do_irq (host, status); +while (status & (INT_EN_MASK |CIRQ_EN )) { +if (host->req_in_progress) +omap_hsmmc_do_irq (host, status); + +if (status &CIRQ_EN ) +mmc_signal_sdio_irq (host->mmc);/* Flush posted write */ status =OMAP_HSMMC_READ (host->base,STAT );@@ -1556,6 +1573,43 @@ static void omap_hsmmc_init_card(struct mmc_host *mmc, struct mmc_card *card)mmc_slot (host).init_card (card); } +static void omap_hsmmc_enable_sdio_irq_nolock (struct omap_hsmmc_host *host, +int enable) +{ +u32 irq_mask; + +if (enable) + host->flags |=HSMMC_SDIO_IRQ_ENABLED ; +else + host->flags &= ~HSMMC_SDIO_IRQ_ENABLED ; +
Quoted 8 lines
drivers/mmc/host/omap_hsmmc.c static void omap_hsmmc_init_card(struct mmc_host *mmc, struct mmc_card *card) · line 1588
Quoted 3 lines
drivers/mmc/host/omap_hsmmc.c +3
-0+/* SDIO IRQ will be enabled as appropriate in runtime resume */ +if (host->flags &HSMMC_RUNTIME_SUSPENDED ) + goto out;Not sure here, will the module still come out of suspend even with SDIO IRQ disabled?
Otherwise nak, sdio irq must enabled independent of pm runtime state. In the worst case needpm_runtime_get().
Well this handling is similar to what's done in sdhci.c and seems to work for me for off-idle on 3730. In that case the whole MMC block is powered off and the wake up follows the dedicated io chain path. For the am33xx off-idle case, the wake-up path would be remuxed to the GPIO in the first GPIO bank that's always on, so again the whole MMC block is off.
Maybe try to test this with your case with some additional patches?
Quoted 4 lines
Quoted 4 lines
drivers/mmc/host/omap_hsmmc.c +0
-0static void omap_hsmmc_conf_bus_power (struct omap_hsmmc_host *host) {u32 hctl, capa, value;@@ -1608,7 +1662,7 @@ static const struct mmc_host_ops omap_hsmmc_ops = {
Quoted 8 lines
drivers/mmc/host/omap_hsmmc.c static const struct mmc_host_ops omap_hsmmc_ops = { · line 1665
Quoted 5 lines
drivers/mmc/host/omap_hsmmc.c +1
-1.get_cd = omap_hsmmc_get_cd, .get_ro = omap_hsmmc_get_ro, .init_card = omap_hsmmc_init_card, -/* NYET -- enable_sdio_irq */ + .enable_sdio_irq = omap_hsmmc_enable_sdio_irq,What about am335x, if this patch goes in, it will break that platform. quirk flag via device tree?
I think it should still work, except the wake-up won't work for the off-idle case unless the SDIO interrupt is remux to the GPIO input?
If there are other issues then yes, we can use the compatible flags.
Thanks for the review and comments,
Tony
PATCH 2/2 mmc: omap_hsmmc: Enable SDIO interrupt
Quoted 43 lines · Tony Lindgren
Hi,
Quoted 28 lines · Andreas Fenkart
2013/9/26 Tony Lindgren <[email protected]>:
Quoted 27 lines
drivers/mmc/host/omap_hsmmc.c +7
-4@@ -463,27 +469,34 @@ static void omap_hsmmc_stop_clock(struct omap_hsmmc_host *host)static void omap_hsmmc_enable_irq (struct omap_hsmmc_host *host,struct mmc_command *cmd) { -unsigned int irq_mask; +u32 irq_mask =INT_EN_MASK ; +unsigned long flags;if (host->use_dma) - irq_mask =INT_EN_MASK & ~(BRR_EN |BWR_EN ); -else - irq_mask =INT_EN_MASK ; + irq_mask &= ~(BRR_EN |BWR_EN );/* Disable timeout for erases */ if (cmd->opcode ==MMC_ERASE ) irq_mask &= ~DTO_EN ; +spin_lock_irqsave (&host->irq_lock, flags);OMAP_HSMMC_WRITE (host->base,STAT ,STAT_CLEAR );OMAP_HSMMC_WRITE (host->base,ISE , irq_mask); +if (host->flags &HSMMC_SDIO_IRQ_ENABLED ) + irq_mask |=CIRQ_EN ;OMAP_HSMMC_WRITE (host->base,IE , irq_mask); +spin_unlock_irqrestore (&host->irq_lock, flags); }Quoted 11 lines
drivers/mmc/host/omap_hsmmc.c static void omap_hsmmc_stop_clock(struct omap_hsmmc_host *host) · line 498
Quoted 8 lines
drivers/mmc/host/omap_hsmmc.c +3
-0static void omap_hsmmc_disable_irq (struct omap_hsmmc_host *host) { +unsigned long flags; + +spin_lock_irqsave (&host->irq_lock, flags);OMAP_HSMMC_WRITE (host->base,ISE ,0 );OMAP_HSMMC_WRITE (host->base,IE ,0 );OMAP_HSMMC_WRITE (host->base,STAT ,STAT_CLEAR );This is wrong. SDIO IRQ must not be disabled upon host initiated transfer. see
omap_hsmmc_request_done
Quoted 5 lines
Hmm I don't quite follow you here, are you saying that
omap_hsmmc_request_done()should call this conditionally?Or maybe even do a patch on top of this based on what you discovered with your earlier patches?
Hi Tony,
Resetting CIRQ_ENABLE in ISE and IE, clears the (latched) pending CIRQ_EN if set in STAT register and also disables SDIO interrupt during interrupt period (outside of command/data transaction).
+ u32 irq_mask = 0;
+ unsigned long flags;
+
+ spin_lock_irqsave(&host->irq_lock, flags);
+
+ /* no transfer running, need to signal cirq if */
+ if (host->sdio_irq_en)
+ irq_mask |= CIRQ_EN;
+
+ OMAP_HSMMC_WRITE(host->base, ISE, irq_mask);
+ OMAP_HSMMC_WRITE(host->base, IE, irq_mask);
I will try testing these patches with above code change and let you know
Re: [PATCH 2/2] mmc: omap_hsmmc: Enable SDIO interrupt
In reply to Tony Lindgren2013/9/26 Tony Lindgren <[email protected]>:
Quoted 42 lines
Hi,
Quoted 39 lines · Andreas Fenkart
2013/9/26 Tony Lindgren <[email protected]>:
Quoted 25 lines
drivers/mmc/host/omap_hsmmc.c +7
-4@@ -463,27 +469,34 @@ static void omap_hsmmc_stop_clock(struct omap_hsmmc_host *host)static void omap_hsmmc_enable_irq (struct omap_hsmmc_host *host,struct mmc_command *cmd) { -unsigned int irq_mask; +u32 irq_mask =INT_EN_MASK ; +unsigned long flags;if (host->use_dma) - irq_mask =INT_EN_MASK & ~(BRR_EN |BWR_EN ); -else - irq_mask =INT_EN_MASK ; + irq_mask &= ~(BRR_EN |BWR_EN );/* Disable timeout for erases */ if (cmd->opcode ==MMC_ERASE ) irq_mask &= ~DTO_EN ; +spin_lock_irqsave (&host->irq_lock, flags);OMAP_HSMMC_WRITE (host->base,STAT ,STAT_CLEAR );OMAP_HSMMC_WRITE (host->base,ISE , irq_mask); +if (host->flags &HSMMC_SDIO_IRQ_ENABLED ) + irq_mask |=CIRQ_EN ;OMAP_HSMMC_WRITE (host->base,IE , irq_mask); +spin_unlock_irqrestore (&host->irq_lock, flags);drivers/mmc/host/omap_hsmmc.c static void omap_hsmmc_stop_clock(struct omap_hsmmc_host *host) · line 498
Quoted 10 lines
drivers/mmc/host/omap_hsmmc.c +3
-0}static void omap_hsmmc_disable_irq (struct omap_hsmmc_host *host) { +unsigned long flags; + +spin_lock_irqsave (&host->irq_lock, flags);OMAP_HSMMC_WRITE (host->base,ISE ,0 );OMAP_HSMMC_WRITE (host->base,IE ,0 );OMAP_HSMMC_WRITE (host->base,STAT ,STAT_CLEAR );This is wrong. SDIO IRQ must not be disabled upon host initiated transfer. see
omap_hsmmc_request_done
It's what Balaji already pointed out.
+ /* no transfer running, need to signal cirq if */
+ if (host->sdio_irq_en)
+ irq_mask |= CIRQ_EN;
omap_hsmmc_request_done calls omap_hsmmc_disable_irq
imagine this case:
1. host issues transfer from host->card (e.g. get RSSI)
2. at end of this transfer, SDIO IRQ are being masked
3. card wants to reply, raises an SDIO IRQ
4. nothing happens, since in 2. SDIO IRQ are masked
I verified this with my 88W8787 card. It is a problem
Quoted 43 lines
Quoted 34 lines
Quoted 34 lines
drivers/mmc/host/omap_hsmmc.c +17
-2+spin_unlock_irqrestore (&host->irq_lock, flags); }/* Calculate divisor for the given clock frequency */ @@ -1040,8 +1053,12 @@ static irqreturn_t omap_hsmmc_irq(int irq, void *dev_id)int status; status =OMAP_HSMMC_READ (host->base,STAT ); -while (status &INT_EN_MASK && host->req_in_progress) { -omap_hsmmc_do_irq (host, status); +while (status & (INT_EN_MASK |CIRQ_EN )) { +if (host->req_in_progress) +omap_hsmmc_do_irq (host, status); + +if (status &CIRQ_EN ) +mmc_signal_sdio_irq (host->mmc);/* Flush posted write */ status =OMAP_HSMMC_READ (host->base,STAT );@@ -1556,6 +1573,43 @@ static void omap_hsmmc_init_card(struct mmc_host *mmc, struct mmc_card *card)mmc_slot (host).init_card (card); } +static void omap_hsmmc_enable_sdio_irq_nolock (struct omap_hsmmc_host *host, +int enable) +{ +u32 irq_mask; + +if (enable) + host->flags |=HSMMC_SDIO_IRQ_ENABLED ; +else + host->flags &= ~HSMMC_SDIO_IRQ_ENABLED ; +Quoted 8 lines
drivers/mmc/host/omap_hsmmc.c static void omap_hsmmc_init_card(struct mmc_host *mmc, struct mmc_card *card) · line 1588
Quoted 3 lines
drivers/mmc/host/omap_hsmmc.c +3
-0+/* SDIO IRQ will be enabled as appropriate in runtime resume */ +if (host->flags &HSMMC_RUNTIME_SUSPENDED ) + goto out;Not sure here, will the module still come out of suspend even with SDIO IRQ disabled?
Otherwise nak, sdio irq must enabled independent of pm runtime state. In the worst case needpm_runtime_get().
Quoted 6 lines
Well this handling is similar to what's done in
sdhci.cand seems to work for me for off-idle on 3730. In that case the whole MMC block is powered off and the wake up follows the dedicated io chain path. For the am33xx off-idle case, the wake-up path would be remuxed to the GPIO in the first GPIO bank that's always on, so again the whole MMC block is off.
Something like this but shorter as a comment:
/*
* Must not touch registers while fclk if off, otherwise SIGBUS.
* Card will still wake up upon SDIO IRQ, and we'll complete
* the request in pm_runtime_resume
*/
Another interpretation of your code could be:
/* Sorry I'm having Siesta right now, try later */
Quoted 17 lines
Maybe try to test this with your case with some additional patches?
Quoted 4 lines
Quoted 4 lines
drivers/mmc/host/omap_hsmmc.c +0
-0static void omap_hsmmc_conf_bus_power (struct omap_hsmmc_host *host) {u32 hctl, capa, value;@@ -1608,7 +1662,7 @@ static const struct mmc_host_ops omap_hsmmc_ops = {Quoted 8 lines
drivers/mmc/host/omap_hsmmc.c static const struct mmc_host_ops omap_hsmmc_ops = { · line 1665
Quoted 5 lines
drivers/mmc/host/omap_hsmmc.c +1
-1.get_cd = omap_hsmmc_get_cd, .get_ro = omap_hsmmc_get_ro, .init_card = omap_hsmmc_init_card, -/* NYET -- enable_sdio_irq */ + .enable_sdio_irq = omap_hsmmc_enable_sdio_irq,What about am335x, if this patch goes in, it will break that platform. quirk flag via device tree?
Quoted 6 lines
I think it should still work, except the wake-up won't work for the off-idle case unless the SDIO interrupt is remux to the GPIO input?
If there are other issues then yes, we can use the compatible flags.
Compatible section you mean, that sounds to good to me. But do we know platform have the deficiency?
There used to be an if statement like this in one of your previous patches.
+ if (match) {
+ mmc->caps |= MMC_CAP_SDIO_IRQ;
+ if (of_find_property(host->dev->of_node,
+ "ti,quirk-swakeup-missing", NULL)) {
+ /* no wakeup from deeper power states, use polling */
+ mmc->caps &= ~MMC_CAP_SDIO_IRQ;
+ }
+ }
Either or is needed, otherwise existing platforms will break, and will have to apply patches on top of yours to reenable polling (e.g. beaglebone)
Quoted 5 lines
Thanks for the review and comments,
Tony
Re: [PATCH 2/2] mmc: omap_hsmmc: Enable SDIO interrupt
Quoted 10 lines · Andreas Fenkart
It's what Balaji already pointed out.
(quoted diff) +3
-0+ /* no transfer running, need to signal cirq if */ + if (host->sdio_irq_en) + irq_mask |= CIRQ_EN;
omap_hsmmc_request_donecallsomap_hsmmc_disable_irq
Quoted 7 lines
imagine this case:
1. host issues transfer fromhost->card(e.g. get RSSI)
2. at end of this transfer, SDIO IRQ are being masked
3. card wants to reply, raises an SDIO IRQ
4. nothing happens, since in 2. SDIO IRQ are maskedI verified this with my 88W8787 card. It is a problem
OK thanks makes sense.
Quoted 11 lines
Something like this but shorter as a comment:
/*
* Must not touch registers while fclk if off, otherwise SIGBUS.
* Card will still wake up upon SDIO IRQ, and we'll complete
* the request inpm_runtime_resume
*/Another interpretation of your code could be:
/* Sorry I'm having Siesta right now, try later */
Well the clocking issue should be already handled by the runtime PM, otherwise we have a bug somewhere. I'm almost certain that no special handling is needed for register access beyond runtime PM, but it would probably be best if you could test it.
Quoted 15 lines
Compatible section you mean, that sounds to good to me. But do we know platform have the deficiency?
There used to be an if statement like this in one of your previous patches.
(quoted diff) +8
-0+ if (match) { + mmc->caps |= MMC_CAP_SDIO_IRQ; + if (of_find_property(host->dev->of_node, + "ti,quirk-swakeup-missing", NULL)) { + /* no wakeup from deeper power states, use polling */ + mmc->caps &= ~MMC_CAP_SDIO_IRQ; + } + }
Quoted 2 lines
Either or is needed, otherwise existing platforms will break, and will have to apply patches on top of yours to reenable polling (e.g. beaglebone)
Yes, let's merge the necessary tweaks to this patch before applying. The SDIO interrupt should work during runtime for all omaps, so maybe we should just ensure that runtime PM keeps those omaps from going to deeper idle modes for now.
Regards,
Tony