Re: [PATCH v7 1/4] mmc: omap_hsmmc: Enable SDIO IRQ.
* Andreas Fenkart [140311 02:45]: > Hi, > > 2014-03-05 17:33 GMT+01:00 Tony Lindgren : > > * Andreas Fenkart [140305 00:30]: > >> Hi, > >> > >> 2014-02-27 22:33 GMT+01:00 Tony Lindgren : > >> > > >> > The wake-irq is needed for omaps with wake-up path and also > >> > when doing GPIO remuxing. So the wake-up handling is pretty > >> > much the same for both cases. > >> I added this comment to the patch, since I was puzzled that you need > >> a wake_irq even whithout remuxing. > > > > Yeah we need it because omap_hsmmc is already doing runtime PM, > > so there's nothing stopping from shutting it down. And there's > > really no need to block runtime PM for it as it's working. > > > Also added this comment, since it really clarifies the issue. OK thanks. > FYI, just tried the patch without pin remuxing on am335x and > it actually works. Read: I'm always using the sdio pinmux never > reconfiguring the pin as a GPIO, but still get GPIO interrupts. > Yes, it fails if I comment out the pm_runtime_resume. > > So it's not that the am335x suddenly has a swakeup line, > but looks like an implementation detail of the am335x GPIO > block. Oh OK, that's good to know. I think 2420 behaved that way too for the serial wake-up. What also may affect things is if that GPIO line is in the first GPIO bank, for the other banks remuxing may still be needed for deeper idle states. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 1/4] mmc: omap_hsmmc: Enable SDIO IRQ.
Hi, 2014-03-05 17:33 GMT+01:00 Tony Lindgren : > * Andreas Fenkart [140305 00:30]: >> Hi, >> >> 2014-02-27 22:33 GMT+01:00 Tony Lindgren : >> > >> > The wake-irq is needed for omaps with wake-up path and also >> > when doing GPIO remuxing. So the wake-up handling is pretty >> > much the same for both cases. >> I added this comment to the patch, since I was puzzled that you need >> a wake_irq even whithout remuxing. > > Yeah we need it because omap_hsmmc is already doing runtime PM, > so there's nothing stopping from shutting it down. And there's > really no need to block runtime PM for it as it's working. > Also added this comment, since it really clarifies the issue. FYI, just tried the patch without pin remuxing on am335x and it actually works. Read: I'm always using the sdio pinmux never reconfiguring the pin as a GPIO, but still get GPIO interrupts. Yes, it fails if I comment out the pm_runtime_resume. So it's not that the am335x suddenly has a swakeup line, but looks like an implementation detail of the am335x GPIO block. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 1/4] mmc: omap_hsmmc: Enable SDIO IRQ.
* Balaji T K [140307 07:15]: > On Wednesday 05 March 2014 02:00 PM, Andreas Fenkart wrote: > >2014-02-28 18:04 GMT+01:00 Balaji T K : > >> > >>I tried testing this patch series on am335x, I see throughput in the > >>range of KBs. Will give another try with Tony's version. > >KB sounds really bad, even with polling I have > 5MBytes/s. > > > >Could you pls paste > ># cat /sys/kernel/debug/mmc0/regs > > > Hi, > > Find below debug reg dump > > mmc0: > sdio irqenabled > pinmux config sdio > ctx_loss: 1 > > regs: > CON:0x0600 > PSTATE: 0x01f7 > HCTL: 0x0d06 > SYSCTL: 0x000d0087 > IE: 0x0100 > ISE:0x0100 > CAPA: 0x06e10080 Is this with wl12xx? If so, wl12xx driver is missing calls to sdio_claim_irq() and sdio_release_irq() so it probably won't behave properly with the SDIO interrupt until those are added. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 1/4] mmc: omap_hsmmc: Enable SDIO IRQ.
On Wednesday 05 March 2014 02:00 PM, Andreas Fenkart wrote: Hi, 2014-02-28 18:04 GMT+01:00 Balaji T K : On Tuesday 25 February 2014 06:07 PM, Andreas Fenkart wrote: For now, only support SDIO interrupt if we are booted with DT. This is because some platforms need special quirks. And we don't want to add new legacy mux platform init code callbacks any longer as we are moving to DT based booting anyways. Signed-off-by: Andreas Fenkart diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index a5a38cc..bd3bb0c 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -1705,7 +1757,18 @@ static void omap_hsmmc_debugfs(struct mmc_host *mmc) #endif #ifdef CONFIG_OF -static u16 omap4_reg_offset = 0x100; +struct of_data { + u16 offset; + int flags; +}; + Hi Andreas, struct of_data declaration needs to be moved out of #ifdef CONFIG_OF for !CONFIG_OF build. should be fixed with new version I tried testing this patch series on am335x, I see throughput in the range of KBs. Will give another try with Tony's version. KB sounds really bad, even with polling I have > 5MBytes/s. Could you pls paste # cat /sys/kernel/debug/mmc0/regs Hi, Find below debug reg dump mmc0: sdio irqenabled pinmux config sdio ctx_loss: 1 regs: CON:0x0600 PSTATE: 0x01f7 HCTL: 0x0d06 SYSCTL: 0x000d0087 IE: 0x0100 ISE:0x0100 CAPA: 0x06e10080 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 1/4] mmc: omap_hsmmc: Enable SDIO IRQ.
* Andreas Fenkart [140305 00:30]: > Hi, > > 2014-02-27 22:33 GMT+01:00 Tony Lindgren : > > > > Thanks for updating this, I finally got around to spend some > > time with it again. I've folded in your fixes and quirk support > > into my earlier patch from [0] as that had a better changelog > > describing the earlier work. > thanks, always struggled with that > > > And I've also now made the SDIO support to depend on properly > > configured wake-up irq from device tree as otherwise wake from > > idle states won't work properly. I've also cleaned up the the > > wake-up irq initialization a bit. > Looks much better now. > > Mind that the sdio irq is level triggered. So we have to disable the > IRQ in the handler otherwise we enter an infinite loop. Oh OK, I was trying to get rid of all the extra flags. But it seem we may still need the wake_irq enabled status bit then. > > The wake-irq is needed for omaps with wake-up path and also > > when doing GPIO remuxing. So the wake-up handling is pretty > > much the same for both cases. > I added this comment to the patch, since I was puzzled that you need > a wake_irq even whithout remuxing. Yeah we need it because omap_hsmmc is already doing runtime PM, so there's nothing stopping from shutting it down. And there's really no need to block runtime PM for it as it's working. > > I've kept your Signed-off-by, can you please check if the patch > > below works for you with the second patch I'll post shortly? > > Will send out another patch soon. Left your Signed-off as well, not > in the sense of your agreement, but didn't dare to remove it because > of your contributions. Yeah thanks will try to test it today :) Regards, Tony > > [0] https://www.mail-archive.com/linux-mmc@vger.kernel.org/msg22290.html -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 1/4] mmc: omap_hsmmc: Enable SDIO IRQ.
Hi, 2014-02-28 18:04 GMT+01:00 Balaji T K : > On Tuesday 25 February 2014 06:07 PM, Andreas Fenkart wrote: >> >> For now, only support SDIO interrupt if we are booted with >> DT. This is because some platforms need special quirks. And >> we don't want to add new legacy mux platform init code >> callbacks any longer as we are moving to DT based booting >> anyways. >> >> Signed-off-by: Andreas Fenkart >> >> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c >> index a5a38cc..bd3bb0c 100644 >> --- a/drivers/mmc/host/omap_hsmmc.c >> +++ b/drivers/mmc/host/omap_hsmmc.c > > > > > >> @@ -1705,7 +1757,18 @@ static void omap_hsmmc_debugfs(struct mmc_host >> *mmc) >> #endif >> >> #ifdef CONFIG_OF >> -static u16 omap4_reg_offset = 0x100; >> +struct of_data { >> + u16 offset; >> + int flags; >> +}; >> + > > Hi Andreas, > > struct of_data declaration needs to be moved out of #ifdef CONFIG_OF > for !CONFIG_OF build. should be fixed with new version > > I tried testing this patch series on am335x, I see throughput in the > range of KBs. Will give another try with Tony's version. KB sounds really bad, even with polling I have > 5MBytes/s. Could you pls paste # cat /sys/kernel/debug/mmc0/regs Thanks for testing. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 1/4] mmc: omap_hsmmc: Enable SDIO IRQ.
Hi, 2014-02-27 22:33 GMT+01:00 Tony Lindgren : > > Thanks for updating this, I finally got around to spend some > time with it again. I've folded in your fixes and quirk support > into my earlier patch from [0] as that had a better changelog > describing the earlier work. thanks, always struggled with that > And I've also now made the SDIO support to depend on properly > configured wake-up irq from device tree as otherwise wake from > idle states won't work properly. I've also cleaned up the the > wake-up irq initialization a bit. Looks much better now. Mind that the sdio irq is level triggered. So we have to disable the IRQ in the handler otherwise we enter an infinite loop. > The wake-irq is needed for omaps with wake-up path and also > when doing GPIO remuxing. So the wake-up handling is pretty > much the same for both cases. I added this comment to the patch, since I was puzzled that you need a wake_irq even whithout remuxing. > > I've kept your Signed-off-by, can you please check if the patch > below works for you with the second patch I'll post shortly? Will send out another patch soon. Left your Signed-off as well, not in the sense of your agreement, but didn't dare to remove it because of your contributions. > > Regards, > > Tony > > [0] https://www.mail-archive.com/linux-mmc@vger.kernel.org/msg22290.html > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 1/4] mmc: omap_hsmmc: Enable SDIO IRQ.
On Tuesday 25 February 2014 06:07 PM, Andreas Fenkart wrote: For now, only support SDIO interrupt if we are booted with DT. This is because some platforms need special quirks. And we don't want to add new legacy mux platform init code callbacks any longer as we are moving to DT based booting anyways. Signed-off-by: Andreas Fenkart diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index a5a38cc..bd3bb0c 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -1705,7 +1757,18 @@ static void omap_hsmmc_debugfs(struct mmc_host *mmc) #endif #ifdef CONFIG_OF -static u16 omap4_reg_offset = 0x100; +struct of_data { + u16 offset; + int flags; +}; + Hi Andreas, struct of_data declaration needs to be moved out of #ifdef CONFIG_OF for !CONFIG_OF build. I tried testing this patch series on am335x, I see throughput in the range of KBs. Will give another try with Tony's version. Thanks and Regards, Balaji T K +static struct of_data omap4_data = { + .offset = 0x100, +}; +static struct of_data am33xx_data = { + .offset = 0x100, + .flags = OMAP_HSMMC_SWAKEUP_MISSING, +}; static const struct of_device_id omap_mmc_of_match[] = { { -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 1/4] mmc: omap_hsmmc: Enable SDIO IRQ.
* Andreas Fenkart [140225 04:46]: > For now, only support SDIO interrupt if we are booted with > DT. This is because some platforms need special quirks. And > we don't want to add new legacy mux platform init code > callbacks any longer as we are moving to DT based booting > anyways. Thanks for updating this, I finally got around to spend some time with it again. I've folded in your fixes and quirk support into my earlier patch from [0] as that had a better changelog describing the earlier work. And I've also now made the SDIO support to depend on properly configured wake-up irq from device tree as otherwise wake from idle states won't work properly. I've also cleaned up the the wake-up irq initialization a bit. The wake-irq is needed for omaps with wake-up path and also when doing GPIO remuxing. So the wake-up handling is pretty much the same for both cases. I've kept your Signed-off-by, can you please check if the patch below works for you with the second patch I'll post shortly? Regards, Tony [0] https://www.mail-archive.com/linux-mmc@vger.kernel.org/msg22290.html 8< -- From: Tony Lindgren Date: Tue, 25 Feb 2014 13:37:43 +0100 Subject: [PATCH] mmc: omap_hsmmc: Enable SDIO interrupt 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 mwifiex connected to MMC3 with mwifiex 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 , Steve Sakoman and Andreas Fenkart with the SDIO IRQ handing improved following how sdhci.c is doing it. For now, only support SDIO interrupt if we are booted with a separate wake-irq configued via device tree. This is because omaps need the wake-irq for idle states, and some omaps need special quirks. And we don't want to add new legacy mux platform init code callbacks any longer as we are moving to DT based booting anyways. To use it, you need to specify the wake-irq using the interrupts-extended property. [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 Cc: Balaji T K Signed-off-by: Andreas Fenkart Signed-off-by: Tony Lindgren diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index dbd32ad..e945060 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -36,6 +37,7 @@ #include #include #include +#include #include #include #include @@ -104,6 +106,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) @@ -178,6 +181,7 @@ struct omap_hsmmc_host { u32 sysctl; u32 capa; int irq; + int wake_irq; int use_dma, dma_ch; struct dma_chan *tx_chan; struct dma_chan *rx_chan; @@ -188,6 +192,9 @@ struct omap_hsmmc_host { int reqs_blocked; int use_reg; int req_in_progress; + int flags; +#define HSMMC_SDIO_IRQ_ENABLED (1 << 0)/* SDIO irq enabled */ +#define HSMMC_SWAKEUP_QUIRK(1 << 1) struct omap_hsmmc_next next_data; struct omap_mmc_platform_data *pdata; }; @@ -468,27 +475,40 @@ 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)