Hi, 2013/11/11 Michael Trimarchi <michael@xxxxxxxxxxxxxxxxxxxx>: > Hi > > On Mon, Nov 11, 2013 at 7:52 PM, <afenkart@xxxxxxxxx> wrote: >> From: Andreas Fenkart <afenkart@xxxxxxxxx> >> >> Add SDIO IRQ entries to debugfs entry. Note that PSTATE shows current >> state of data lines, incl. SDIO IRQ pending >> >> Signed-off-by: Andreas Fenkart <afenkart@xxxxxxxxx> >> --- >> drivers/mmc/host/omap_hsmmc.c | 22 +++++++++++++++++++++- >> 1 file changed, 21 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c >> index e880b44..34e2ee0 100644 >> --- a/drivers/mmc/host/omap_hsmmc.c >> +++ b/drivers/mmc/host/omap_hsmmc.c >> @@ -81,6 +81,7 @@ static void apply_clk_hack(struct device *dev) >> #define OMAP_HSMMC_RSP54 0x0118 >> #define OMAP_HSMMC_RSP76 0x011C >> #define OMAP_HSMMC_DATA 0x0120 >> +#define OMAP_HSMMC_PSTATE 0x0124 >> #define OMAP_HSMMC_HCTL 0x0128 >> #define OMAP_HSMMC_SYSCTL 0x012C >> #define OMAP_HSMMC_STAT 0x0130 >> @@ -1800,6 +1801,23 @@ 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); >> + unsigned long flags; >> + bool suspended; >> + >> + spin_lock_irqsave(&host->irq_lock, flags); > > Why do you need a spin_lock_irqsave just to print out the status? > >> + seq_puts(s, "\n"); >> + seq_printf(s, "sdio irq\t%s\n", ((host->flags & HSMMC_SDIO_IRQ_ENABLED) >> + ? "enabled" : "disabled")); initially, because the flags field could be changed by an irq. It's atomic and since the code path doesn't change, it's safe to remove the spinlock, thx >> + suspended = host->dev->power.runtime_status != RPM_ACTIVE; >> + if (host->flags & HSMMC_SWAKEUP_QUIRK) { >> + seq_printf(s, "pinmux config\t%s\n", (suspended ? >> + "gpio" : "sdio")); >> + if (suspended) >> + seq_printf(s, "sdio irq pin\t%s\n", >> + gpio_get_value(mmc_slot(host).gpio_cirq) ? >> + "high" : "low"); This is more of an issue, the pinmux could have changed underneath, if we resumed meanwile. Reading the GPIO value read will not reflect the pin on the package. It has nothing to do with the spinlock though so will remove that. . >> + } >> + spin_unlock_irqrestore(&host->irq_lock, flags); >> > > Michael > >> if (host->suspended) { >> seq_printf(s, "host suspended, can't read registers\n"); >> @@ -1807,9 +1825,11 @@ static int omap_hsmmc_regs_show(struct seq_file *s, void *data) >> } >> >> pm_runtime_get_sync(host->dev); >> - >> + seq_puts(s, "\nregs:\n"); >> seq_printf(s, "CON:\t\t0x%08x\n", >> OMAP_HSMMC_READ(host->base, CON)); >> + seq_printf(s, "PSTATE:\t\t0x%08x\n", >> + OMAP_HSMMC_READ(host->base, PSTATE)); >> seq_printf(s, "HCTL:\t\t0x%08x\n", >> OMAP_HSMMC_READ(host->base, HCTL)); >> seq_printf(s, "SYSCTL:\t\t0x%08x\n", >> -- >> 1.7.10.4 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-omap" in >> the body of a message to majordomo@xxxxxxxxxxxxxxx >> More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html