* [PATCH] mmc: mediatek: fix SDIO irq issue @ 2020-01-21 7:20 Yong Mao 2020-01-21 7:20 ` Yong Mao 0 siblings, 1 reply; 6+ messages in thread From: Yong Mao @ 2020-01-21 7:20 UTC (permalink / raw) To: Ulf Hansson Cc: Chaotian Jing, Matthias Brugger, linux-mmc, linux-arm-kernel, linux-mediatek, linux-kernel, srv_heupstream yong mao (1): mmc: mediatek: fix SDIO irq issue drivers/mmc/host/mtk-sd.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) -- 2.24.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] mmc: mediatek: fix SDIO irq issue 2020-01-21 7:20 [PATCH] mmc: mediatek: fix SDIO irq issue Yong Mao @ 2020-01-21 7:20 ` Yong Mao 2020-01-21 7:38 ` Hsin-Yi Wang 2020-02-12 15:29 ` Ulf Hansson 0 siblings, 2 replies; 6+ messages in thread From: Yong Mao @ 2020-01-21 7:20 UTC (permalink / raw) To: Ulf Hansson Cc: Chaotian Jing, Matthias Brugger, linux-mmc, linux-arm-kernel, linux-mediatek, linux-kernel, srv_heupstream, yong mao From: yong mao <yong.mao@mediatek.com> Host controller may lost interrupt in some specail case. Add SDIO irq recheck mechanism to make sure all interrupts can be processed immediately. Signed-off-by: Yong Mao <yong.mao@mediatek.com> --- drivers/mmc/host/mtk-sd.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c index 7726dcf..18a1b86 100644 --- a/drivers/mmc/host/mtk-sd.c +++ b/drivers/mmc/host/mtk-sd.c @@ -128,6 +128,7 @@ #define MSDC_PS_CDSTS (0x1 << 1) /* R */ #define MSDC_PS_CDDEBOUNCE (0xf << 12) /* RW */ #define MSDC_PS_DAT (0xff << 16) /* R */ +#define MSDC_PS_DATA1 (0x1 << 17) /* R */ #define MSDC_PS_CMD (0x1 << 24) /* R */ #define MSDC_PS_WP (0x1 << 31) /* R */ @@ -361,6 +362,7 @@ struct msdc_save_para { struct mtk_mmc_compatible { u8 clk_div_bits; + bool recheck_sdio_irq; bool hs400_tune; /* only used for MT8173 */ u32 pad_tune_reg; bool async_fifo; @@ -436,6 +438,7 @@ struct msdc_host { static const struct mtk_mmc_compatible mt8135_compat = { .clk_div_bits = 8, + .recheck_sdio_irq = false, .hs400_tune = false, .pad_tune_reg = MSDC_PAD_TUNE, .async_fifo = false, @@ -448,6 +451,7 @@ struct msdc_host { static const struct mtk_mmc_compatible mt8173_compat = { .clk_div_bits = 8, + .recheck_sdio_irq = true, .hs400_tune = true, .pad_tune_reg = MSDC_PAD_TUNE, .async_fifo = false, @@ -460,6 +464,7 @@ struct msdc_host { static const struct mtk_mmc_compatible mt8183_compat = { .clk_div_bits = 12, + .recheck_sdio_irq = false, .hs400_tune = false, .pad_tune_reg = MSDC_PAD_TUNE0, .async_fifo = true, @@ -472,6 +477,7 @@ struct msdc_host { static const struct mtk_mmc_compatible mt2701_compat = { .clk_div_bits = 12, + .recheck_sdio_irq = false, .hs400_tune = false, .pad_tune_reg = MSDC_PAD_TUNE0, .async_fifo = true, @@ -484,6 +490,7 @@ struct msdc_host { static const struct mtk_mmc_compatible mt2712_compat = { .clk_div_bits = 12, + .recheck_sdio_irq = false, .hs400_tune = false, .pad_tune_reg = MSDC_PAD_TUNE0, .async_fifo = true, @@ -496,6 +503,7 @@ struct msdc_host { static const struct mtk_mmc_compatible mt7622_compat = { .clk_div_bits = 12, + .recheck_sdio_irq = false, .hs400_tune = false, .pad_tune_reg = MSDC_PAD_TUNE0, .async_fifo = true, @@ -508,6 +516,7 @@ struct msdc_host { static const struct mtk_mmc_compatible mt8516_compat = { .clk_div_bits = 12, + .recheck_sdio_irq = false, .hs400_tune = false, .pad_tune_reg = MSDC_PAD_TUNE0, .async_fifo = true, @@ -518,6 +527,7 @@ struct msdc_host { static const struct mtk_mmc_compatible mt7620_compat = { .clk_div_bits = 8, + .recheck_sdio_irq = false, .hs400_tune = false, .pad_tune_reg = MSDC_PAD_TUNE, .async_fifo = false, @@ -1007,6 +1017,30 @@ static int msdc_auto_cmd_done(struct msdc_host *host, int events, return cmd->error; } +/** + * msdc_recheck_sdio_irq - recheck whether the SDIO irq is lost + * + * Host controller may lost interrupt in some special case. + * Add SDIO irq recheck mechanism to make sure all interrupts + * can be processed immediately + * + */ +static void msdc_recheck_sdio_irq(struct msdc_host *host) +{ + u32 reg_int, reg_inten, reg_ps; + + if ((host->mmc->caps & MMC_CAP_SDIO_IRQ)) { + reg_inten = readl(host->base + MSDC_INTEN); + if (reg_inten & MSDC_INTEN_SDIOIRQ) { + reg_int = readl(host->base + MSDC_INT); + reg_ps = readl(host->base + MSDC_PS); + if (!((reg_int & MSDC_INT_SDIOIRQ) || + (reg_ps & MSDC_PS_DATA1))) + sdio_signal_irq(host->mmc); + } + } +} + static void msdc_track_cmd_data(struct msdc_host *host, struct mmc_command *cmd, struct mmc_data *data) { @@ -1035,6 +1069,8 @@ static void msdc_request_done(struct msdc_host *host, struct mmc_request *mrq) if (host->error) msdc_reset_hw(host); mmc_request_done(host->mmc, mrq); + if (host->dev_comp->recheck_sdio_irq) + msdc_recheck_sdio_irq(host); } /* returns true if command is fully handled; returns false otherwise */ @@ -1393,6 +1429,8 @@ static void __msdc_enable_sdio_irq(struct msdc_host *host, int enb) if (enb) { sdr_set_bits(host->base + MSDC_INTEN, MSDC_INTEN_SDIOIRQ); sdr_set_bits(host->base + SDC_CFG, SDC_CFG_SDIOIDE); + if (host->dev_comp->recheck_sdio_irq) + msdc_recheck_sdio_irq(host); } else { sdr_clr_bits(host->base + MSDC_INTEN, MSDC_INTEN_SDIOIRQ); sdr_clr_bits(host->base + SDC_CFG, SDC_CFG_SDIOIDE); -- 1.9.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mmc: mediatek: fix SDIO irq issue 2020-01-21 7:20 ` Yong Mao @ 2020-01-21 7:38 ` Hsin-Yi Wang 2020-02-10 4:24 ` Hsin-Yi Wang 2020-02-12 15:29 ` Ulf Hansson 1 sibling, 1 reply; 6+ messages in thread From: Hsin-Yi Wang @ 2020-01-21 7:38 UTC (permalink / raw) To: Yong Mao Cc: Ulf Hansson, Chaotian Jing, Matthias Brugger, linux-mmc, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, linux-mediatek, lkml, srv_heupstream On Tue, Jan 21, 2020 at 7:20 AM Yong Mao <yong.mao@mediatek.com> wrote: > > From: yong mao <yong.mao@mediatek.com> > > Host controller may lost interrupt in some specail case. > Add SDIO irq recheck mechanism to make sure all interrupts > can be processed immediately. > > Signed-off-by: Yong Mao <yong.mao@mediatek.com> > --- Thanks, mt8173 need this patch for cap-sdio-irq to work. Tested-by: Hsin-Yi Wang <hsinyi@chromium.org> > drivers/mmc/host/mtk-sd.c | 38 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 38 insertions(+) > > diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c > index 7726dcf..18a1b86 100644 > --- a/drivers/mmc/host/mtk-sd.c > +++ b/drivers/mmc/host/mtk-sd.c > @@ -128,6 +128,7 @@ > #define MSDC_PS_CDSTS (0x1 << 1) /* R */ > #define MSDC_PS_CDDEBOUNCE (0xf << 12) /* RW */ > #define MSDC_PS_DAT (0xff << 16) /* R */ > +#define MSDC_PS_DATA1 (0x1 << 17) /* R */ > #define MSDC_PS_CMD (0x1 << 24) /* R */ > #define MSDC_PS_WP (0x1 << 31) /* R */ > > @@ -361,6 +362,7 @@ struct msdc_save_para { > > struct mtk_mmc_compatible { > u8 clk_div_bits; > + bool recheck_sdio_irq; > bool hs400_tune; /* only used for MT8173 */ > u32 pad_tune_reg; > bool async_fifo; > @@ -436,6 +438,7 @@ struct msdc_host { > > static const struct mtk_mmc_compatible mt8135_compat = { > .clk_div_bits = 8, > + .recheck_sdio_irq = false, > .hs400_tune = false, > .pad_tune_reg = MSDC_PAD_TUNE, > .async_fifo = false, > @@ -448,6 +451,7 @@ struct msdc_host { > > static const struct mtk_mmc_compatible mt8173_compat = { > .clk_div_bits = 8, > + .recheck_sdio_irq = true, > .hs400_tune = true, > .pad_tune_reg = MSDC_PAD_TUNE, > .async_fifo = false, > @@ -460,6 +464,7 @@ struct msdc_host { > > static const struct mtk_mmc_compatible mt8183_compat = { > .clk_div_bits = 12, > + .recheck_sdio_irq = false, > .hs400_tune = false, > .pad_tune_reg = MSDC_PAD_TUNE0, > .async_fifo = true, > @@ -472,6 +477,7 @@ struct msdc_host { > > static const struct mtk_mmc_compatible mt2701_compat = { > .clk_div_bits = 12, > + .recheck_sdio_irq = false, > .hs400_tune = false, > .pad_tune_reg = MSDC_PAD_TUNE0, > .async_fifo = true, > @@ -484,6 +490,7 @@ struct msdc_host { > > static const struct mtk_mmc_compatible mt2712_compat = { > .clk_div_bits = 12, > + .recheck_sdio_irq = false, > .hs400_tune = false, > .pad_tune_reg = MSDC_PAD_TUNE0, > .async_fifo = true, > @@ -496,6 +503,7 @@ struct msdc_host { > > static const struct mtk_mmc_compatible mt7622_compat = { > .clk_div_bits = 12, > + .recheck_sdio_irq = false, > .hs400_tune = false, > .pad_tune_reg = MSDC_PAD_TUNE0, > .async_fifo = true, > @@ -508,6 +516,7 @@ struct msdc_host { > > static const struct mtk_mmc_compatible mt8516_compat = { > .clk_div_bits = 12, > + .recheck_sdio_irq = false, > .hs400_tune = false, > .pad_tune_reg = MSDC_PAD_TUNE0, > .async_fifo = true, > @@ -518,6 +527,7 @@ struct msdc_host { > > static const struct mtk_mmc_compatible mt7620_compat = { > .clk_div_bits = 8, > + .recheck_sdio_irq = false, > .hs400_tune = false, > .pad_tune_reg = MSDC_PAD_TUNE, > .async_fifo = false, > @@ -1007,6 +1017,30 @@ static int msdc_auto_cmd_done(struct msdc_host *host, int events, > return cmd->error; > } > > +/** > + * msdc_recheck_sdio_irq - recheck whether the SDIO irq is lost > + * > + * Host controller may lost interrupt in some special case. > + * Add SDIO irq recheck mechanism to make sure all interrupts > + * can be processed immediately > + * > + */ > +static void msdc_recheck_sdio_irq(struct msdc_host *host) > +{ > + u32 reg_int, reg_inten, reg_ps; > + > + if ((host->mmc->caps & MMC_CAP_SDIO_IRQ)) { > + reg_inten = readl(host->base + MSDC_INTEN); > + if (reg_inten & MSDC_INTEN_SDIOIRQ) { > + reg_int = readl(host->base + MSDC_INT); > + reg_ps = readl(host->base + MSDC_PS); > + if (!((reg_int & MSDC_INT_SDIOIRQ) || > + (reg_ps & MSDC_PS_DATA1))) > + sdio_signal_irq(host->mmc); > + } > + } > +} > + > static void msdc_track_cmd_data(struct msdc_host *host, > struct mmc_command *cmd, struct mmc_data *data) > { > @@ -1035,6 +1069,8 @@ static void msdc_request_done(struct msdc_host *host, struct mmc_request *mrq) > if (host->error) > msdc_reset_hw(host); > mmc_request_done(host->mmc, mrq); > + if (host->dev_comp->recheck_sdio_irq) > + msdc_recheck_sdio_irq(host); > } > > /* returns true if command is fully handled; returns false otherwise */ > @@ -1393,6 +1429,8 @@ static void __msdc_enable_sdio_irq(struct msdc_host *host, int enb) > if (enb) { > sdr_set_bits(host->base + MSDC_INTEN, MSDC_INTEN_SDIOIRQ); > sdr_set_bits(host->base + SDC_CFG, SDC_CFG_SDIOIDE); > + if (host->dev_comp->recheck_sdio_irq) > + msdc_recheck_sdio_irq(host); > } else { > sdr_clr_bits(host->base + MSDC_INTEN, MSDC_INTEN_SDIOIRQ); > sdr_clr_bits(host->base + SDC_CFG, SDC_CFG_SDIOIDE); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mmc: mediatek: fix SDIO irq issue 2020-01-21 7:38 ` Hsin-Yi Wang @ 2020-02-10 4:24 ` Hsin-Yi Wang 0 siblings, 0 replies; 6+ messages in thread From: Hsin-Yi Wang @ 2020-02-10 4:24 UTC (permalink / raw) To: Yong Mao Cc: Ulf Hansson, Chaotian Jing, Matthias Brugger, linux-mmc, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, linux-mediatek, lkml, srv_heupstream On Tue, Jan 21, 2020 at 3:38 PM Hsin-Yi Wang <hsinyi@chromium.org> wrote: > > On Tue, Jan 21, 2020 at 7:20 AM Yong Mao <yong.mao@mediatek.com> wrote: > > > > From: yong mao <yong.mao@mediatek.com> > > > > Host controller may lost interrupt in some specail case. > > Add SDIO irq recheck mechanism to make sure all interrupts > > can be processed immediately. > > > > Signed-off-by: Yong Mao <yong.mao@mediatek.com> > > --- > > Thanks, mt8173 need this patch for cap-sdio-irq to work. > > Tested-by: Hsin-Yi Wang <hsinyi@chromium.org> > > Gentle ping Thanks. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mmc: mediatek: fix SDIO irq issue 2020-01-21 7:20 ` Yong Mao 2020-01-21 7:38 ` Hsin-Yi Wang @ 2020-02-12 15:29 ` Ulf Hansson 2020-02-26 9:50 ` yong.mao 1 sibling, 1 reply; 6+ messages in thread From: Ulf Hansson @ 2020-02-12 15:29 UTC (permalink / raw) To: Yong Mao Cc: Chaotian Jing, Matthias Brugger, linux-mmc, Linux ARM, moderated list:ARM/Mediatek SoC support, Linux Kernel Mailing List, srv_heupstream On Tue, 21 Jan 2020 at 08:21, Yong Mao <yong.mao@mediatek.com> wrote: > > From: yong mao <yong.mao@mediatek.com> > > Host controller may lost interrupt in some specail case. Please explain a bit more about the special cases. When and how often does it happen? > Add SDIO irq recheck mechanism to make sure all interrupts > can be processed immediately. > > Signed-off-by: Yong Mao <yong.mao@mediatek.com> > --- > drivers/mmc/host/mtk-sd.c | 38 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 38 insertions(+) > > diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c > index 7726dcf..18a1b86 100644 > --- a/drivers/mmc/host/mtk-sd.c > +++ b/drivers/mmc/host/mtk-sd.c > @@ -128,6 +128,7 @@ > #define MSDC_PS_CDSTS (0x1 << 1) /* R */ > #define MSDC_PS_CDDEBOUNCE (0xf << 12) /* RW */ > #define MSDC_PS_DAT (0xff << 16) /* R */ > +#define MSDC_PS_DATA1 (0x1 << 17) /* R */ > #define MSDC_PS_CMD (0x1 << 24) /* R */ > #define MSDC_PS_WP (0x1 << 31) /* R */ > > @@ -361,6 +362,7 @@ struct msdc_save_para { > > struct mtk_mmc_compatible { > u8 clk_div_bits; > + bool recheck_sdio_irq; > bool hs400_tune; /* only used for MT8173 */ > u32 pad_tune_reg; > bool async_fifo; > @@ -436,6 +438,7 @@ struct msdc_host { > > static const struct mtk_mmc_compatible mt8135_compat = { > .clk_div_bits = 8, > + .recheck_sdio_irq = false, > .hs400_tune = false, > .pad_tune_reg = MSDC_PAD_TUNE, > .async_fifo = false, > @@ -448,6 +451,7 @@ struct msdc_host { > > static const struct mtk_mmc_compatible mt8173_compat = { > .clk_div_bits = 8, > + .recheck_sdio_irq = true, > .hs400_tune = true, > .pad_tune_reg = MSDC_PAD_TUNE, > .async_fifo = false, > @@ -460,6 +464,7 @@ struct msdc_host { > > static const struct mtk_mmc_compatible mt8183_compat = { > .clk_div_bits = 12, > + .recheck_sdio_irq = false, > .hs400_tune = false, > .pad_tune_reg = MSDC_PAD_TUNE0, > .async_fifo = true, > @@ -472,6 +477,7 @@ struct msdc_host { > > static const struct mtk_mmc_compatible mt2701_compat = { > .clk_div_bits = 12, > + .recheck_sdio_irq = false, > .hs400_tune = false, > .pad_tune_reg = MSDC_PAD_TUNE0, > .async_fifo = true, > @@ -484,6 +490,7 @@ struct msdc_host { > > static const struct mtk_mmc_compatible mt2712_compat = { > .clk_div_bits = 12, > + .recheck_sdio_irq = false, > .hs400_tune = false, > .pad_tune_reg = MSDC_PAD_TUNE0, > .async_fifo = true, > @@ -496,6 +503,7 @@ struct msdc_host { > > static const struct mtk_mmc_compatible mt7622_compat = { > .clk_div_bits = 12, > + .recheck_sdio_irq = false, > .hs400_tune = false, > .pad_tune_reg = MSDC_PAD_TUNE0, > .async_fifo = true, > @@ -508,6 +516,7 @@ struct msdc_host { > > static const struct mtk_mmc_compatible mt8516_compat = { > .clk_div_bits = 12, > + .recheck_sdio_irq = false, > .hs400_tune = false, > .pad_tune_reg = MSDC_PAD_TUNE0, > .async_fifo = true, > @@ -518,6 +527,7 @@ struct msdc_host { > > static const struct mtk_mmc_compatible mt7620_compat = { > .clk_div_bits = 8, > + .recheck_sdio_irq = false, > .hs400_tune = false, > .pad_tune_reg = MSDC_PAD_TUNE, > .async_fifo = false, > @@ -1007,6 +1017,30 @@ static int msdc_auto_cmd_done(struct msdc_host *host, int events, > return cmd->error; > } > > +/** > + * msdc_recheck_sdio_irq - recheck whether the SDIO irq is lost > + * > + * Host controller may lost interrupt in some special case. > + * Add SDIO irq recheck mechanism to make sure all interrupts > + * can be processed immediately > + * > + */ > +static void msdc_recheck_sdio_irq(struct msdc_host *host) > +{ > + u32 reg_int, reg_inten, reg_ps; > + > + if ((host->mmc->caps & MMC_CAP_SDIO_IRQ)) { > + reg_inten = readl(host->base + MSDC_INTEN); > + if (reg_inten & MSDC_INTEN_SDIOIRQ) { > + reg_int = readl(host->base + MSDC_INT); > + reg_ps = readl(host->base + MSDC_PS); > + if (!((reg_int & MSDC_INT_SDIOIRQ) || > + (reg_ps & MSDC_PS_DATA1))) This looks a bit unnecessary complicated and there are more parentheses than needed. I am also wondering about the logic. This looks like you want to signal an SDIO IRQ when both MSDC_INT_SDIOIRQ and MSDC_PS_DATA1 are cleared. Is that really correct? Moreover, this means that you will be polling the registers for each every request you complete. This sounds quite inefficient and I wonder if it can be done more seldom, perhaps via a timer event instead. And, what if there is no request for a while, then this means the re-check doesn't gets to run. Could that be a problem? > + sdio_signal_irq(host->mmc); Before calling sdio_signal_irq(), the SDIO IRQ needs to be temporarily disabled. In other words, looks like you should be calling __msdc_enable_sdio_irq(0) from here as well. > + } > + } > +} > + > static void msdc_track_cmd_data(struct msdc_host *host, > struct mmc_command *cmd, struct mmc_data *data) > { > @@ -1035,6 +1069,8 @@ static void msdc_request_done(struct msdc_host *host, struct mmc_request *mrq) > if (host->error) > msdc_reset_hw(host); > mmc_request_done(host->mmc, mrq); > + if (host->dev_comp->recheck_sdio_irq) > + msdc_recheck_sdio_irq(host); > } > > /* returns true if command is fully handled; returns false otherwise */ > @@ -1393,6 +1429,8 @@ static void __msdc_enable_sdio_irq(struct msdc_host *host, int enb) > if (enb) { > sdr_set_bits(host->base + MSDC_INTEN, MSDC_INTEN_SDIOIRQ); > sdr_set_bits(host->base + SDC_CFG, SDC_CFG_SDIOIDE); > + if (host->dev_comp->recheck_sdio_irq) > + msdc_recheck_sdio_irq(host); > } else { > sdr_clr_bits(host->base + MSDC_INTEN, MSDC_INTEN_SDIOIRQ); > sdr_clr_bits(host->base + SDC_CFG, SDC_CFG_SDIOIDE); > -- > 1.9.1 Kind regards Uffe ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mmc: mediatek: fix SDIO irq issue 2020-02-12 15:29 ` Ulf Hansson @ 2020-02-26 9:50 ` yong.mao 0 siblings, 0 replies; 6+ messages in thread From: yong.mao @ 2020-02-26 9:50 UTC (permalink / raw) To: Ulf Hansson Cc: Chaotian Jing, Matthias Brugger, linux-mmc, Linux ARM, moderated list:ARM/Mediatek SoC support, Linux Kernel Mailing List, srv_heupstream On Wed, 2020-02-12 at 16:29 +0100, Ulf Hansson wrote: > On Tue, 21 Jan 2020 at 08:21, Yong Mao <yong.mao@mediatek.com> wrote: > > > > From: yong mao <yong.mao@mediatek.com> > > > > Host controller may lost interrupt in some specail case. > > Please explain a bit more about the special cases. When and how often > does it happen? SDIO irq is not triggered by low level, but by falling edge in our previous IC. This mechanism only have one chance to catch if a SDIO irq comes within the multiple block transmission. This SDIO irq may easily lost, because falling edge appears only once within 2 clock after data transmission is completed. > > > Add SDIO irq recheck mechanism to make sure all interrupts > > can be processed immediately. > > > > Signed-off-by: Yong Mao <yong.mao@mediatek.com> > > --- > > drivers/mmc/host/mtk-sd.c | 38 ++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 38 insertions(+) > > > > diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c > > index 7726dcf..18a1b86 100644 > > --- a/drivers/mmc/host/mtk-sd.c > > +++ b/drivers/mmc/host/mtk-sd.c > > @@ -128,6 +128,7 @@ > > #define MSDC_PS_CDSTS (0x1 << 1) /* R */ > > #define MSDC_PS_CDDEBOUNCE (0xf << 12) /* RW */ > > #define MSDC_PS_DAT (0xff << 16) /* R */ > > +#define MSDC_PS_DATA1 (0x1 << 17) /* R */ > > #define MSDC_PS_CMD (0x1 << 24) /* R */ > > #define MSDC_PS_WP (0x1 << 31) /* R */ > > > > @@ -361,6 +362,7 @@ struct msdc_save_para { > > > > struct mtk_mmc_compatible { > > u8 clk_div_bits; > > + bool recheck_sdio_irq; > > bool hs400_tune; /* only used for MT8173 */ > > u32 pad_tune_reg; > > bool async_fifo; > > @@ -436,6 +438,7 @@ struct msdc_host { > > > > static const struct mtk_mmc_compatible mt8135_compat = { > > .clk_div_bits = 8, > > + .recheck_sdio_irq = false, > > .hs400_tune = false, > > .pad_tune_reg = MSDC_PAD_TUNE, > > .async_fifo = false, > > @@ -448,6 +451,7 @@ struct msdc_host { > > > > static const struct mtk_mmc_compatible mt8173_compat = { > > .clk_div_bits = 8, > > + .recheck_sdio_irq = true, > > .hs400_tune = true, > > .pad_tune_reg = MSDC_PAD_TUNE, > > .async_fifo = false, > > @@ -460,6 +464,7 @@ struct msdc_host { > > > > static const struct mtk_mmc_compatible mt8183_compat = { > > .clk_div_bits = 12, > > + .recheck_sdio_irq = false, > > .hs400_tune = false, > > .pad_tune_reg = MSDC_PAD_TUNE0, > > .async_fifo = true, > > @@ -472,6 +477,7 @@ struct msdc_host { > > > > static const struct mtk_mmc_compatible mt2701_compat = { > > .clk_div_bits = 12, > > + .recheck_sdio_irq = false, > > .hs400_tune = false, > > .pad_tune_reg = MSDC_PAD_TUNE0, > > .async_fifo = true, > > @@ -484,6 +490,7 @@ struct msdc_host { > > > > static const struct mtk_mmc_compatible mt2712_compat = { > > .clk_div_bits = 12, > > + .recheck_sdio_irq = false, > > .hs400_tune = false, > > .pad_tune_reg = MSDC_PAD_TUNE0, > > .async_fifo = true, > > @@ -496,6 +503,7 @@ struct msdc_host { > > > > static const struct mtk_mmc_compatible mt7622_compat = { > > .clk_div_bits = 12, > > + .recheck_sdio_irq = false, > > .hs400_tune = false, > > .pad_tune_reg = MSDC_PAD_TUNE0, > > .async_fifo = true, > > @@ -508,6 +516,7 @@ struct msdc_host { > > > > static const struct mtk_mmc_compatible mt8516_compat = { > > .clk_div_bits = 12, > > + .recheck_sdio_irq = false, > > .hs400_tune = false, > > .pad_tune_reg = MSDC_PAD_TUNE0, > > .async_fifo = true, > > @@ -518,6 +527,7 @@ struct msdc_host { > > > > static const struct mtk_mmc_compatible mt7620_compat = { > > .clk_div_bits = 8, > > + .recheck_sdio_irq = false, > > .hs400_tune = false, > > .pad_tune_reg = MSDC_PAD_TUNE, > > .async_fifo = false, > > @@ -1007,6 +1017,30 @@ static int msdc_auto_cmd_done(struct msdc_host *host, int events, > > return cmd->error; > > } > > > > +/** > > + * msdc_recheck_sdio_irq - recheck whether the SDIO irq is lost > > + * > > + * Host controller may lost interrupt in some special case. > > + * Add SDIO irq recheck mechanism to make sure all interrupts > > + * can be processed immediately > > + * > > + */ > > +static void msdc_recheck_sdio_irq(struct msdc_host *host) > > +{ > > + u32 reg_int, reg_inten, reg_ps; > > + > > + if ((host->mmc->caps & MMC_CAP_SDIO_IRQ)) { > > + reg_inten = readl(host->base + MSDC_INTEN); > > + if (reg_inten & MSDC_INTEN_SDIOIRQ) { > > + reg_int = readl(host->base + MSDC_INT); > > + reg_ps = readl(host->base + MSDC_PS); > > + if (!((reg_int & MSDC_INT_SDIOIRQ) || > > + (reg_ps & MSDC_PS_DATA1))) > > This looks a bit unnecessary complicated and there are more > parentheses than needed. Thanks. We will remove unnecessary parentheses in the next version. > > I am also wondering about the logic. This looks like you want to > signal an SDIO IRQ when both MSDC_INT_SDIOIRQ and MSDC_PS_DATA1 are > cleared. Is that really correct? Yes. This can make sure every SW SDIO irq is really lost by HW. And also make sure it is not fake irq. > > Moreover, this means that you will be polling the registers for each > every request you complete. This sounds quite inefficient and I wonder > if it can be done more seldom, Yes. You are right. Re-check will be invoked often. But registers access does not cost more time. > perhaps via a timer event instead. This timer will be alive until the SDIO card is removed. It is very similar with polling mechanism in sdio_irq_thread. Because this patch is only for previous IC(Our new IC does not have this issue), we don't want make too many changes in mtk-sd.c. If there is some other simple solution, we will update in the next version. > And, > what if there is no request for a while, then this means the re-check > doesn't gets to run. Could that be a problem? The SDIO irq in this case can be catch by HW correctly. > > > + sdio_signal_irq(host->mmc); > > Before calling sdio_signal_irq(), the SDIO IRQ needs to be temporarily > disabled. In other words, looks like you should be calling > __msdc_enable_sdio_irq(0) from here as well. Yes. You are right. We will update it in next version. > > > + } > > + } > > +} > > + > > static void msdc_track_cmd_data(struct msdc_host *host, > > struct mmc_command *cmd, struct mmc_data *data) > > { > > @@ -1035,6 +1069,8 @@ static void msdc_request_done(struct msdc_host *host, struct mmc_request *mrq) > > if (host->error) > > msdc_reset_hw(host); > > mmc_request_done(host->mmc, mrq); > > + if (host->dev_comp->recheck_sdio_irq) > > + msdc_recheck_sdio_irq(host); > > } > > > > /* returns true if command is fully handled; returns false otherwise */ > > @@ -1393,6 +1429,8 @@ static void __msdc_enable_sdio_irq(struct msdc_host *host, int enb) > > if (enb) { > > sdr_set_bits(host->base + MSDC_INTEN, MSDC_INTEN_SDIOIRQ); > > sdr_set_bits(host->base + SDC_CFG, SDC_CFG_SDIOIDE); > > + if (host->dev_comp->recheck_sdio_irq) > > + msdc_recheck_sdio_irq(host); > > } else { > > sdr_clr_bits(host->base + MSDC_INTEN, MSDC_INTEN_SDIOIRQ); > > sdr_clr_bits(host->base + SDC_CFG, SDC_CFG_SDIOIDE); > > -- > > 1.9.1 > > Kind regards > Uffe ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-02-26 9:51 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-01-21 7:20 [PATCH] mmc: mediatek: fix SDIO irq issue Yong Mao 2020-01-21 7:20 ` Yong Mao 2020-01-21 7:38 ` Hsin-Yi Wang 2020-02-10 4:24 ` Hsin-Yi Wang 2020-02-12 15:29 ` Ulf Hansson 2020-02-26 9:50 ` yong.mao
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).