Re: [PATCH 2/4] sdio: mediatek: Support sdio feature

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Mon, Jan 23, 2017 at 7:24 PM, Wei-Ning Huang <wnhuang@xxxxxxxxxx> wrote:
> On Tue, Nov 8, 2016 at 2:08 PM, Yong Mao <yong.mao@xxxxxxxxxxxx> wrote:
>> From: yong mao <yong.mao@xxxxxxxxxxxx>
>>
>> 1. Add irqlock to protect accessing the shared register
>> 2. Modify the implementation of msdc_card_busy due to SDIO
>> 3. Implement enable_sdio_irq
>> 4. Add msdc_recheck_sdio_irq mechanism to make sure all
>>    interrupts can be processed immediately
>>
>> Signed-off-by: Yong Mao <yong.mao@xxxxxxxxxxxx>
>> Signed-off-by: Chaotian Jing <chaotian.jing@xxxxxxxxxxxx>
>> ---
>>  drivers/mmc/host/mtk-sd.c |  167 ++++++++++++++++++++++++++++++++++-----------
>>  1 file changed, 129 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
>> index b29683b..37edf30 100644
>> --- a/drivers/mmc/host/mtk-sd.c
>> +++ b/drivers/mmc/host/mtk-sd.c
>> @@ -117,6 +117,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  */
>>
>> @@ -304,6 +305,7 @@ struct msdc_host {
>>         int cmd_rsp;
>>
>>         spinlock_t lock;
>> +       spinlock_t irqlock;     /* sdio irq lock */
>>         struct mmc_request *mrq;
>>         struct mmc_command *cmd;
>>         struct mmc_data *data;
>> @@ -322,12 +324,14 @@ struct msdc_host {
>>         struct pinctrl_state *pins_uhs;
>>         struct delayed_work req_timeout;
>>         int irq;                /* host interrupt */
>> +       bool irq_thread_alive;
>>
>>         struct clk *src_clk;    /* msdc source clock */
>>         struct clk *h_clk;      /* msdc h_clk */
>>         u32 mclk;               /* mmc subsystem clock frequency */
>>         u32 src_clk_freq;       /* source clock frequency */
>>         u32 sclk;               /* SD/MS bus clock frequency */
>> +       bool clock_on;
>>         unsigned char timing;
>>         bool vqmmc_enabled;
>>         u32 hs400_ds_delay;
>> @@ -387,6 +391,7 @@ static void msdc_reset_hw(struct msdc_host *host)
>>
>>  static void msdc_cmd_next(struct msdc_host *host,
>>                 struct mmc_request *mrq, struct mmc_command *cmd);
>> +static void msdc_recheck_sdio_irq(struct msdc_host *host);
>>
>>  static const u32 cmd_ints_mask = MSDC_INTEN_CMDRDY | MSDC_INTEN_RSPCRCERR |
>>                         MSDC_INTEN_CMDTMO | MSDC_INTEN_ACMDRDY |
>> @@ -513,6 +518,7 @@ static void msdc_gate_clock(struct msdc_host *host)
>>  {
>>         clk_disable_unprepare(host->src_clk);
>>         clk_disable_unprepare(host->h_clk);
>> +       host->clock_on = false;
>>  }
>>
>>  static void msdc_ungate_clock(struct msdc_host *host)
>> @@ -521,6 +527,7 @@ static void msdc_ungate_clock(struct msdc_host *host)
>>         clk_prepare_enable(host->src_clk);
>>         while (!(readl(host->base + MSDC_CFG) & MSDC_CFG_CKSTB))
>>                 cpu_relax();
>> +       host->clock_on = true;
>>  }
>>
>>  static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz)
>> @@ -529,6 +536,7 @@ static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz)
>>         u32 flags;
>>         u32 div;
>>         u32 sclk;
>> +       unsigned long irq_flags;
>>
>>         if (!hz) {
>>                 dev_dbg(host->dev, "set mclk to 0\n");
>> @@ -537,8 +545,11 @@ static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz)
>>                 return;
>>         }
>>
>> +       spin_lock_irqsave(&host->irqlock, irq_flags);
>>         flags = readl(host->base + MSDC_INTEN);
>>         sdr_clr_bits(host->base + MSDC_INTEN, flags);
>> +       spin_unlock_irqrestore(&host->irqlock, irq_flags);
>> +
>>         sdr_clr_bits(host->base + MSDC_CFG, MSDC_CFG_HS400_CK_MODE);
>>         if (timing == MMC_TIMING_UHS_DDR50 ||
>>             timing == MMC_TIMING_MMC_DDR52 ||
>> @@ -588,7 +599,10 @@ static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz)
>>         host->timing = timing;
>>         /* need because clk changed. */
>>         msdc_set_timeout(host, host->timeout_ns, host->timeout_clks);
>> +
>> +       spin_lock_irqsave(&host->irqlock, irq_flags);
>>         sdr_set_bits(host->base + MSDC_INTEN, flags);
>> +       spin_unlock_irqrestore(&host->irqlock, irq_flags);
>>
>>         /*
>>          * mmc_select_hs400() will drop to 50Mhz and High speed mode,
>> @@ -690,6 +704,7 @@ static inline u32 msdc_cmd_prepare_raw_cmd(struct msdc_host *host,
>>  static void msdc_start_data(struct msdc_host *host, struct mmc_request *mrq,
>>                             struct mmc_command *cmd, struct mmc_data *data)
>>  {
>> +       unsigned long flags;
>>         bool read;
>>
>>         WARN_ON(host->data);
>> @@ -698,8 +713,12 @@ static void msdc_start_data(struct msdc_host *host, struct mmc_request *mrq,
>>
>>         mod_delayed_work(system_wq, &host->req_timeout, DAT_TIMEOUT);
>>         msdc_dma_setup(host, &host->dma, data);
>> +
>> +       spin_lock_irqsave(&host->irqlock, flags);
>>         sdr_set_bits(host->base + MSDC_INTEN, data_ints_mask);
>>         sdr_set_field(host->base + MSDC_DMA_CTRL, MSDC_DMA_CTRL_START, 1);
>> +       spin_unlock_irqrestore(&host->irqlock, flags);
>> +
>>         dev_dbg(host->dev, "DMA start\n");
>>         dev_dbg(host->dev, "%s: cmd=%d DMA data: %d blocks; read=%d\n",
>>                         __func__, cmd->opcode, data->blocks, read);
>> @@ -756,6 +775,7 @@ static void msdc_request_done(struct msdc_host *host, struct mmc_request *mrq)
>>         if (mrq->data)
>>                 msdc_unprepare_data(host, mrq);
>>         mmc_request_done(host->mmc, mrq);
>> +       msdc_recheck_sdio_irq(host);
>>  }
>>
>>  /* returns true if command is fully handled; returns false otherwise */
>> @@ -779,15 +799,17 @@ static bool msdc_cmd_done(struct msdc_host *host, int events,
>>                                         | MSDC_INT_CMDTMO)))
>>                 return done;
>>
>> -       spin_lock_irqsave(&host->lock, flags);
>>         done = !host->cmd;
>> +       spin_lock_irqsave(&host->lock, flags);
>>         host->cmd = NULL;
>>         spin_unlock_irqrestore(&host->lock, flags);
>>
>>         if (done)
>>                 return true;
>>
>> +       spin_lock_irqsave(&host->irqlock, flags);
>>         sdr_clr_bits(host->base + MSDC_INTEN, cmd_ints_mask);
>> +       spin_unlock_irqrestore(&host->irqlock, flags);
>>
>>         if (cmd->flags & MMC_RSP_PRESENT) {
>>                 if (cmd->flags & MMC_RSP_136) {
>> @@ -902,6 +924,7 @@ static inline bool msdc_cmd_is_ready(struct msdc_host *host,
>>  static void msdc_start_command(struct msdc_host *host,
>>                 struct mmc_request *mrq, struct mmc_command *cmd)
>>  {
>> +       unsigned long flags;
>>         u32 rawcmd;
>>
>>         WARN_ON(host->cmd);
>> @@ -920,7 +943,10 @@ static void msdc_start_command(struct msdc_host *host,
>>         rawcmd = msdc_cmd_prepare_raw_cmd(host, mrq, cmd);
>>         mod_delayed_work(system_wq, &host->req_timeout, DAT_TIMEOUT);
>>
>> +       spin_lock_irqsave(&host->irqlock, flags);
>>         sdr_set_bits(host->base + MSDC_INTEN, cmd_ints_mask);
>> +       spin_unlock_irqrestore(&host->irqlock, flags);
>> +
>>         writel(cmd->arg, host->base + SDC_ARG);
>>         writel(rawcmd, host->base + SDC_CMD);
>>  }
>> @@ -1013,8 +1039,8 @@ static bool msdc_data_xfer_done(struct msdc_host *host, u32 events,
>>              | MSDC_INT_DMA_BDCSERR | MSDC_INT_DMA_GPDCSERR
>>              | MSDC_INT_DMA_PROTECT);
>>
>> -       spin_lock_irqsave(&host->lock, flags);
>>         done = !host->data;
>> +       spin_lock_irqsave(&host->lock, flags);
>>         if (check_data)
>>                 host->data = NULL;
>>         spin_unlock_irqrestore(&host->lock, flags);
>> @@ -1029,7 +1055,11 @@ static bool msdc_data_xfer_done(struct msdc_host *host, u32 events,
>>                                 1);
>>                 while (readl(host->base + MSDC_DMA_CFG) & MSDC_DMA_CFG_STS)
>>                         cpu_relax();
>> +
>> +               spin_lock_irqsave(&host->irqlock, flags);
>>                 sdr_clr_bits(host->base + MSDC_INTEN, data_ints_mask);
>> +               spin_unlock_irqrestore(&host->irqlock, flags);
>> +
>>                 dev_dbg(host->dev, "DMA stop\n");
>>
>>                 if ((events & MSDC_INT_XFER_COMPL) && (!stop || !stop->error)) {
>> @@ -1134,44 +1164,47 @@ static void msdc_request_timeout(struct work_struct *work)
>>
>>  static irqreturn_t msdc_irq(int irq, void *dev_id)
>>  {
>> +       unsigned long flags;
>>         struct msdc_host *host = (struct msdc_host *) dev_id;
>> +       struct mmc_request *mrq;
>> +       struct mmc_command *cmd;
>> +       struct mmc_data *data;
>> +       u32 events, event_mask;
>> +
>> +       spin_lock_irqsave(&host->irqlock, flags);
>> +       events = readl(host->base + MSDC_INT);
>> +       event_mask = readl(host->base + MSDC_INTEN);
>> +       /* clear interrupts */
>> +       writel(events & event_mask, host->base + MSDC_INT);
>> +
>> +       mrq = host->mrq;
>> +       cmd = host->cmd;
>> +       data = host->data;
>> +       spin_unlock_irqrestore(&host->irqlock, flags);
>> +
>> +       if ((events & event_mask) & MSDC_INT_SDIOIRQ) {
>> +               mmc_signal_sdio_irq(host->mmc);
>> +               if (!mrq)
>> +                       return IRQ_HANDLED;
>> +       }
>>
>> -       while (true) {
>> -               unsigned long flags;
>> -               struct mmc_request *mrq;
>> -               struct mmc_command *cmd;
>> -               struct mmc_data *data;
>> -               u32 events, event_mask;
>> -
>> -               spin_lock_irqsave(&host->lock, flags);
>> -               events = readl(host->base + MSDC_INT);
>> -               event_mask = readl(host->base + MSDC_INTEN);
>> -               /* clear interrupts */
>> -               writel(events & event_mask, host->base + MSDC_INT);
>> -
>> -               mrq = host->mrq;
>> -               cmd = host->cmd;
>> -               data = host->data;
>> -               spin_unlock_irqrestore(&host->lock, flags);
>> -
>> -               if (!(events & event_mask))
>> -                       break;
>> +       if (!(events & (event_mask & ~MSDC_INT_SDIOIRQ)))
>> +               return IRQ_HANDLED;
>>
>> -               if (!mrq) {
>> -                       dev_err(host->dev,
>> -                               "%s: MRQ=NULL; events=%08X; event_mask=%08X\n",
>> -                               __func__, events, event_mask);
>> -                       WARN_ON(1);
>> -                       break;
>> -               }
>> +       if (!mrq) {
>> +               dev_err(host->dev,
>> +                       "%s: MRQ=NULL; events=%08X; event_mask=%08X\n",
>> +                       __func__, events, event_mask);
>> +               WARN_ON(1);
>> +               return IRQ_HANDLED;
>> +       }
>>
>> -               dev_dbg(host->dev, "%s: events=%08X\n", __func__, events);
>> +       dev_dbg(host->dev, "%s: events=%08X\n", __func__, events);
>>
>> -               if (cmd)
>> -                       msdc_cmd_done(host, events, mrq, cmd);
>> -               else if (data)
>> -                       msdc_data_xfer_done(host, events, mrq, data);
>> -       }
>> +       if (cmd)
>> +               msdc_cmd_done(host, events, mrq, cmd);
>> +       else if (data)
>> +               msdc_data_xfer_done(host, events, mrq, data);
>>
>>         return IRQ_HANDLED;
>>  }
>> @@ -1179,6 +1212,7 @@ static irqreturn_t msdc_irq(int irq, void *dev_id)
>>  static void msdc_init_hw(struct msdc_host *host)
>>  {
>>         u32 val;
>> +       unsigned long flags;
>>
>>         /* Configure to MMC/SD mode, clock free running */
>>         sdr_set_bits(host->base + MSDC_CFG, MSDC_CFG_MODE | MSDC_CFG_CKPDN);
>> @@ -1190,9 +1224,11 @@ static void msdc_init_hw(struct msdc_host *host)
>>         sdr_clr_bits(host->base + MSDC_PS, MSDC_PS_CDEN);
>>
>>         /* Disable and clear all interrupts */
>> +       spin_lock_irqsave(&host->irqlock, flags);
>>         writel(0, host->base + MSDC_INTEN);
>>         val = readl(host->base + MSDC_INT);
>>         writel(val, host->base + MSDC_INT);
>> +       spin_unlock_irqrestore(&host->irqlock, flags);
>>
>>         writel(0, host->base + MSDC_PAD_TUNE);
>>         writel(0, host->base + MSDC_IOCON);
>> @@ -1207,9 +1243,11 @@ static void msdc_init_hw(struct msdc_host *host)
>>          */
>>         sdr_set_bits(host->base + SDC_CFG, SDC_CFG_SDIO);
>>
>> -       /* disable detect SDIO device interrupt function */
>> -       sdr_clr_bits(host->base + SDC_CFG, SDC_CFG_SDIOIDE);
>> -
>> +       if (host->mmc->caps & MMC_CAP_SDIO_IRQ)
>> +               sdr_set_bits(host->base + SDC_CFG, SDC_CFG_SDIOIDE);
>> +       else
>> +               /* disable detect SDIO device interrupt function */
>> +               sdr_clr_bits(host->base + SDC_CFG, SDC_CFG_SDIOIDE);
>>         /* Configure to default data timeout */
>>         sdr_set_field(host->base + SDC_CFG, SDC_CFG_DTOC, 3);
>>
>> @@ -1221,11 +1259,15 @@ static void msdc_init_hw(struct msdc_host *host)
>>  static void msdc_deinit_hw(struct msdc_host *host)
>>  {
>>         u32 val;
>> +       unsigned long flags;
>> +
>>         /* Disable and clear all interrupts */
>> +       spin_lock_irqsave(&host->irqlock, flags);
>>         writel(0, host->base + MSDC_INTEN);
>>
>>         val = readl(host->base + MSDC_INT);
>>         writel(val, host->base + MSDC_INT);
>> +       spin_unlock_irqrestore(&host->irqlock, flags);
>>  }
>>
>>  /* init gpd and bd list in msdc_drv_probe */
>> @@ -1493,6 +1535,52 @@ static void msdc_hw_reset(struct mmc_host *mmc)
>>         sdr_clr_bits(host->base + EMMC_IOCON, 1);
>>  }
>>
>> +/**
>> + * msdc_recheck_sdio_irq - recheck whether the SDIO IRQ is lost
>> + * @host: The host to check.
>> + *
>> + * 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_ps;
>> +
>> +       if (host->clock_on &&
>> +           (host->mmc->caps & MMC_CAP_SDIO_IRQ) &&
>> +           host->irq_thread_alive) {
>> +               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))) {
>> +                       mmc_signal_sdio_irq(host->mmc);
>> +               }
>> +       }
>> +}
>> +
>> +static void msdc_enable_sdio_irq(struct mmc_host *mmc, int enable)
>> +{
>> +       unsigned long flags;
>> +       struct msdc_host *host = mmc_priv(mmc);
>> +
>> +       host->irq_thread_alive = true;
>> +       if (enable) {
>> +               pm_runtime_get_sync(host->dev);\
> This cause problems in kernel >= 3.19. Since pm_runtime_get_sync calls
> __might_sleep, and you are not suppose to sleep in a IRQ handler.
Ignore above, apparently I don't know what I'm talking about ...
It's actually because the sdio_irq_thread set task state to
TASK_INTERRUPTABLE before calling enable_sdio_irq:

    set_current_state(TASK_INTERRUPTIBLE);
    if (host->caps & MMC_CAP_SDIO_IRQ)
      host->ops->enable_sdio_irq(host, 1);
    if (!kthread_should_stop())
      schedule_timeout(period);
    set_current_state(TASK_RUNNING);

so we are not suppose to call __might_sleep in the enable_sdio_irq callback.
>
> 2017-01-20T00:32:49.855603-08:00 WARNING kernel: [   11.068860] do not
> call blocking ops when !TASK_RUNNING; state=1 set at
> [<ffffffc0007a350c>] sdio_irq_thread+0x11c/0x1dc
> ...
> 2017-01-20T00:32:49.856042-08:00 EMERG kernel: [   12.136156] Call
> trace:
> 2017-01-20T00:32:49.856044-08:00 WARNING kernel: [   12.138584]
> [<ffffffc000249d84>] __might_sleep+0x64/0x90
> 2017-01-20T00:32:49.856047-08:00 WARNING kernel: [   12.143855]
> [<ffffffc000630f54>] __pm_runtime_resume+0x40/0x9c
> 2017-01-20T00:32:49.856049-08:00 WARNING kernel: [   12.149644]
> [<ffffffc0007ae994>] msdc_enable_sdio_irq+0x44/0xd0
> 2017-01-20T00:32:49.856051-08:00 WARNING kernel: [   12.155516]
> [<ffffffc0007a3544>] sdio_irq_thread+0x154/0x1dc
> 2017-01-20T00:32:49.856053-08:00 WARNING kernel: [   12.161131]
> [<ffffffc00023f30c>] kthread+0x10c/0x114
> 2017-01-20T00:32:49.856055-08:00 WARNING kernel: [   12.166056]
> [<ffffffc000203dd0>] ret_from_fork+0x10/0x40
> 2017-01-20T00:32:49.856057-08:00 WARNING kernel: [   12.171368] sched:
> RT throttling activated for rt_rq ffffffc0fff5a5c0 (cpu 1)
> 2017-01-20T00:32:49.856059-08:00 WARNING kernel: [   12.171368]
> potential CPU hogs:
> 2017-01-20T00:32:49.856061-08:00 WARNING kernel: [   12.171368]
>  ksdioirqd/mmc2 (1772)
>
> dw_mmc also enables autosuspend but it didn't call any pm_runtime_get*
> function in it's enable_sdio_irq function, I don't really know how it
> worked around this.
> Ulf, any suggestions on how we can do this?
>
> Wei-Ning
>
>> +               msdc_recheck_sdio_irq(host);
>> +
>> +               spin_lock_irqsave(&host->irqlock, flags);
>> +               sdr_set_bits(host->base + SDC_CFG, SDC_CFG_SDIOIDE);
>> +               sdr_set_bits(host->base + MSDC_INTEN, MSDC_INTEN_SDIOIRQ);
>> +               spin_unlock_irqrestore(&host->irqlock, flags);
>> +       } else {
>> +               spin_lock_irqsave(&host->irqlock, flags);
>> +               sdr_clr_bits(host->base + MSDC_INTEN, MSDC_INTEN_SDIOIRQ);
>> +               spin_unlock_irqrestore(&host->irqlock, flags);
>> +       }
>> +}
>> +
>>  static struct mmc_host_ops mt_msdc_ops = {
>>         .post_req = msdc_post_req,
>>         .pre_req = msdc_pre_req,
>> @@ -1504,6 +1592,7 @@ static void msdc_hw_reset(struct mmc_host *mmc)
>>         .execute_tuning = msdc_execute_tuning,
>>         .prepare_hs400_tuning = msdc_prepare_hs400_tuning,
>>         .hw_reset = msdc_hw_reset,
>> +       .enable_sdio_irq = msdc_enable_sdio_irq,
>>  };
>>
>>  static int msdc_drv_probe(struct platform_device *pdev)
>> @@ -1600,6 +1689,7 @@ static int msdc_drv_probe(struct platform_device *pdev)
>>         mmc_dev(mmc)->dma_mask = &host->dma_mask;
>>
>>         host->timeout_clks = 3 * 1048576;
>> +       host->irq_thread_alive = false;
>>         host->dma.gpd = dma_alloc_coherent(&pdev->dev,
>>                                 2 * sizeof(struct mt_gpdma_desc),
>>                                 &host->dma.gpd_addr, GFP_KERNEL);
>> @@ -1613,6 +1703,7 @@ static int msdc_drv_probe(struct platform_device *pdev)
>>         msdc_init_gpd_bd(host, &host->dma);
>>         INIT_DELAYED_WORK(&host->req_timeout, msdc_request_timeout);
>>         spin_lock_init(&host->lock);
>> +       spin_lock_init(&host->irqlock);
>>
>>         platform_set_drvdata(pdev, mmc);
>>         msdc_ungate_clock(host);
>> --
>> 1.7.9.5
>>
>
>
>
> --
> Wei-Ning Huang, 黃偉寧 | Software Engineer, Google Inc., Taiwan |
> wnhuang@xxxxxxxxxx | Cell: +886 910-380678



-- 
Wei-Ning Huang, 黃偉寧 | Software Engineer, Google Inc., Taiwan |
wnhuang@xxxxxxxxxx | Cell: +886 910-380678
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux