* [PATCH v4 0/3] Fixes for SDIO interrupts for dw_mmc @ 2014-12-02 20:49 Doug Anderson 2014-12-02 20:49 ` [PATCH v4 1/3] mmc: core: Support the optional init_card() callback for MMC and SD Doug Anderson 0 siblings, 1 reply; 4+ messages in thread From: Doug Anderson @ 2014-12-02 20:49 UTC (permalink / raw) To: Jaehoon Chung, Seungwon Jeon, Ulf Hansson Cc: Alim Akhtar, Sonny Rao, Andrew Bresticker, Heiko Stuebner, Doug Anderson, linux, wsa, tony, linux-kernel, s.hauer, linux-mmc, chris, gsoutade, joe, axel.lin, linux-omap, linux-arm-kernel Bing Zhao at Marvell found a problem with dw_mmc where interrupts weren't firing sometimes. He tracked it down to a read-modify-write problem with the INTMASK. These three patches fix the problem. Note: I've picked up a > 1-year old series here to make another attempt at landing it upstream. These patches have been in shipping Chromebooks for the last year. There are essentially no changes from the last v3 other than a rebase. The first patch extends the "init_card()" mechanism of MMC core to actually be called for all card types, not just SDIO. That could be applied any time and should fix at least one longstanding bug (untested). The second patch is a cleanup patch to use init_card() to move things around a bit so we don't need to handle SDIO cards in such a strange place. On earlier versions of this patch Seungwon brought up a few points which I have _not_ addressed. See <https://patchwork.kernel.org/patch/3049071/>. Other than talk of cards with out of band interrupts maybe being able to gate their clocks, he wanted to use MMC_QUIRK_BROKEN_CLK_GATING. I didn't do that because of the ordering of init_card() and when the quirks are set. Some users of init_card() like pandora_wl1251_init_card() rely on it being called very early in the process. pandora_wl1251_init_card() hardcodes a vendor and device and thus need to be called super early. On the other hand the code that adds quirks _reads_ the vendor and device. It can't possibly move before init_card(). If folks are willing to take an additional host op of init_card_late() I can certainly go that way, though. The third patch is (I think) reviewed and ready to go assuming the other two land. Changes in v3: - Add fixup to pandora_wl1251_init_card(). Changes in v2: - mmc core change new for this version. - Fixed "|" to "&". - intmask_lock renamed to irq_lock Doug Anderson (3): mmc: core: Support the optional init_card() callback for MMC and SD mmc: dw_mmc: Cleanup disable of low power mode w/ SDIO interrupts mmc: dw_mmc: Protect read-modify-write of INTMASK with a lock arch/arm/mach-omap2/board-omap3pandora.c | 14 +++--- drivers/mmc/core/mmc.c | 6 +++ drivers/mmc/core/sd.c | 7 ++- drivers/mmc/host/dw_mmc.c | 80 +++++++++++++++++++------------- drivers/mmc/host/dw_mmc.h | 1 + include/linux/mmc/dw_mmc.h | 6 +++ 6 files changed, 74 insertions(+), 40 deletions(-) -- 2.2.0.rc0.207.ga3a616c ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v4 1/3] mmc: core: Support the optional init_card() callback for MMC and SD 2014-12-02 20:49 [PATCH v4 0/3] Fixes for SDIO interrupts for dw_mmc Doug Anderson @ 2014-12-02 20:49 ` Doug Anderson 2014-12-02 23:15 ` Jaehoon Chung 0 siblings, 1 reply; 4+ messages in thread From: Doug Anderson @ 2014-12-02 20:49 UTC (permalink / raw) To: Jaehoon Chung, Seungwon Jeon, Ulf Hansson Cc: Alim Akhtar, Sonny Rao, Andrew Bresticker, Heiko Stuebner, Doug Anderson, tony, linux, chris, gsoutade, axel.lin, s.hauer, wsa, joe, linux-arm-kernel, linux-omap, linux-kernel, linux-mmc In (3fcb027 ARM: MXC: mxcmmc: work around a bug in the SDHC busy line handling) the optional init_card() callback was added. According to the original change it was "for now only called from mmc_sdio_init_card()". This callback really ought to be called from the SD and MMC init functions as well. One current user of this callback (mxcmci_init_card) will not work as expected if you insert an SDIO card, then eject it and put a normal SD card in. Specifically the normal SD card will not get to run with 4-bit data. I'd like to use the init_card() callback to handle a similar quirk on dw_mmc when using SDIO Interrupts (the "low power" feature of the card needs to be disabled), so that will add a second user of the function. As part of this change fixup the one place that relied on the callback only happening for SDIO cards. Signed-off-by: Doug Anderson <dianders@chromium.org> Reviewed-by: Grant Grundler <grundler@chromium.org> --- Changes in v3: - Add fixup to pandora_wl1251_init_card(). Changes in v2: - mmc core change new for this version. arch/arm/mach-omap2/board-omap3pandora.c | 14 ++++++++------ drivers/mmc/core/mmc.c | 6 ++++++ drivers/mmc/core/sd.c | 7 ++++++- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/arch/arm/mach-omap2/board-omap3pandora.c b/arch/arm/mach-omap2/board-omap3pandora.c index 7f17087..969e100 100644 --- a/arch/arm/mach-omap2/board-omap3pandora.c +++ b/arch/arm/mach-omap2/board-omap3pandora.c @@ -254,12 +254,14 @@ static void pandora_wl1251_init_card(struct mmc_card *card) * We have TI wl1251 attached to MMC3. Pass this information to * SDIO core because it can't be probed by normal methods. */ - card->quirks |= MMC_QUIRK_NONSTD_SDIO; - card->cccr.wide_bus = 1; - card->cis.vendor = 0x104c; - card->cis.device = 0x9066; - card->cis.blksize = 512; - card->cis.max_dtr = 20000000; + if (card->type == MMC_TYPE_SDIO || card->type == MMC_TYPE_SD_COMBO) { + card->quirks |= MMC_QUIRK_NONSTD_SDIO; + card->cccr.wide_bus = 1; + card->cis.vendor = 0x104c; + card->cis.device = 0x9066; + card->cis.blksize = 512; + card->cis.max_dtr = 20000000; + } } static struct omap2_hsmmc_info omap3pandora_mmc[] = { diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index 02ad792..4a21d66 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -1297,6 +1297,12 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, } /* + * Call the optional HC's init_card function to handle quirks. + */ + if (host->ops->init_card) + host->ops->init_card(host, card); + + /* * For native busses: set card RCA and quit open drain mode. */ if (!mmc_host_is_spi(host)) { diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index d90a6de..29fccdc 100644 --- a/drivers/mmc/core/sd.c +++ b/drivers/mmc/core/sd.c @@ -933,6 +933,12 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr, } /* + * Call the optional HC's init_card function to handle quirks. + */ + if (host->ops->init_card) + host->ops->init_card(host, card); + + /* * For native busses: get card RCA and quit open drain mode. */ if (!mmc_host_is_spi(host)) { @@ -1271,4 +1277,3 @@ err: return err; } - -- 2.2.0.rc0.207.ga3a616c ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v4 1/3] mmc: core: Support the optional init_card() callback for MMC and SD 2014-12-02 20:49 ` [PATCH v4 1/3] mmc: core: Support the optional init_card() callback for MMC and SD Doug Anderson @ 2014-12-02 23:15 ` Jaehoon Chung 2014-12-02 23:43 ` Doug Anderson 0 siblings, 1 reply; 4+ messages in thread From: Jaehoon Chung @ 2014-12-02 23:15 UTC (permalink / raw) To: Doug Anderson, Seungwon Jeon, Ulf Hansson Cc: Alim Akhtar, Sonny Rao, Andrew Bresticker, Heiko Stuebner, tony, linux, chris, gsoutade, axel.lin, s.hauer, wsa, joe, linux-arm-kernel, linux-omap, linux-kernel, linux-mmc Hi Doug. I think good that this patch is separated to two patches. (board file and codes relevant to mmc.) Best Regards, Jaehoon Chung On 12/03/2014 05:49 AM, Doug Anderson wrote: > In (3fcb027 ARM: MXC: mxcmmc: work around a bug in the SDHC busy line > handling) the optional init_card() callback was added. According to > the original change it was "for now only called from > mmc_sdio_init_card()". > > This callback really ought to be called from the SD and MMC init > functions as well. One current user of this callback > (mxcmci_init_card) will not work as expected if you insert an SDIO > card, then eject it and put a normal SD card in. Specifically the > normal SD card will not get to run with 4-bit data. > > I'd like to use the init_card() callback to handle a similar quirk on > dw_mmc when using SDIO Interrupts (the "low power" feature of the card > needs to be disabled), so that will add a second user of the function. > > As part of this change fixup the one place that relied on the callback > only happening for SDIO cards. > > Signed-off-by: Doug Anderson <dianders@chromium.org> > Reviewed-by: Grant Grundler <grundler@chromium.org> > --- > Changes in v3: > - Add fixup to pandora_wl1251_init_card(). > > Changes in v2: > - mmc core change new for this version. > > arch/arm/mach-omap2/board-omap3pandora.c | 14 ++++++++------ > drivers/mmc/core/mmc.c | 6 ++++++ > drivers/mmc/core/sd.c | 7 ++++++- > 3 files changed, 20 insertions(+), 7 deletions(-) > > diff --git a/arch/arm/mach-omap2/board-omap3pandora.c b/arch/arm/mach-omap2/board-omap3pandora.c > index 7f17087..969e100 100644 > --- a/arch/arm/mach-omap2/board-omap3pandora.c > +++ b/arch/arm/mach-omap2/board-omap3pandora.c > @@ -254,12 +254,14 @@ static void pandora_wl1251_init_card(struct mmc_card *card) > * We have TI wl1251 attached to MMC3. Pass this information to > * SDIO core because it can't be probed by normal methods. > */ > - card->quirks |= MMC_QUIRK_NONSTD_SDIO; > - card->cccr.wide_bus = 1; > - card->cis.vendor = 0x104c; > - card->cis.device = 0x9066; > - card->cis.blksize = 512; > - card->cis.max_dtr = 20000000; > + if (card->type == MMC_TYPE_SDIO || card->type == MMC_TYPE_SD_COMBO) { > + card->quirks |= MMC_QUIRK_NONSTD_SDIO; > + card->cccr.wide_bus = 1; > + card->cis.vendor = 0x104c; > + card->cis.device = 0x9066; > + card->cis.blksize = 512; > + card->cis.max_dtr = 20000000; > + } > } > > static struct omap2_hsmmc_info omap3pandora_mmc[] = { > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c > index 02ad792..4a21d66 100644 > --- a/drivers/mmc/core/mmc.c > +++ b/drivers/mmc/core/mmc.c > @@ -1297,6 +1297,12 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr, > } > > /* > + * Call the optional HC's init_card function to handle quirks. > + */ > + if (host->ops->init_card) > + host->ops->init_card(host, card); > + > + /* > * For native busses: set card RCA and quit open drain mode. > */ > if (!mmc_host_is_spi(host)) { > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c > index d90a6de..29fccdc 100644 > --- a/drivers/mmc/core/sd.c > +++ b/drivers/mmc/core/sd.c > @@ -933,6 +933,12 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr, > } > > /* > + * Call the optional HC's init_card function to handle quirks. > + */ > + if (host->ops->init_card) > + host->ops->init_card(host, card); > + > + /* > * For native busses: get card RCA and quit open drain mode. > */ > if (!mmc_host_is_spi(host)) { > @@ -1271,4 +1277,3 @@ err: > > return err; > } > - > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v4 1/3] mmc: core: Support the optional init_card() callback for MMC and SD 2014-12-02 23:15 ` Jaehoon Chung @ 2014-12-02 23:43 ` Doug Anderson 0 siblings, 0 replies; 4+ messages in thread From: Doug Anderson @ 2014-12-02 23:43 UTC (permalink / raw) To: Jaehoon Chung Cc: Seungwon Jeon, Ulf Hansson, Alim Akhtar, Sonny Rao, Andrew Bresticker, Heiko Stuebner, Tony Lindgren, Russell King, Chris Ball, gsoutade, Axel Lin, Sascha Hauer, Wolfram Sang, Joe Perches, linux-arm-kernel@lists.infradead.org, linux-omap, linux-kernel@vger.kernel.org, linux-mmc@vger.kernel.org Jaehoon, On Tue, Dec 2, 2014 at 3:15 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote: > Hi Doug. > > I think good that this patch is separated to two patches. > (board file and codes relevant to mmc.) Yes, good idea. I've spun v5 with this. -Doug ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-12-02 23:43 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-12-02 20:49 [PATCH v4 0/3] Fixes for SDIO interrupts for dw_mmc Doug Anderson 2014-12-02 20:49 ` [PATCH v4 1/3] mmc: core: Support the optional init_card() callback for MMC and SD Doug Anderson 2014-12-02 23:15 ` Jaehoon Chung 2014-12-02 23:43 ` Doug Anderson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).