| Message ID | 1527590335-171787-1-git-send-email-xiaqing17@hisilicon.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
+ Shawn Lin, Evgeniy Didin, Doug Andersson On 29 May 2018 at 12:38, Qing Xia <xiaqing17@hisilicon.com> wrote: > From: x00270170 <xiaqing17@hisilicon.com> > > Card write threshold control is supposed to be set since controller > version 2.80a for data write in HS400 mode and data read in > HS200/HS400/SDR104 mode. However the current code returns without > configuring it in the case of data writing in HS400 mode. > Meanwhile the patch fixes that the current code goes to > 'disable' when doing data reading in HS400 mode. > > Signed-off-by: Qing Xia <xiaqing17@hisilicon.com> This looks good to me. However, it seems like Jaehoon has been busy, no response yet. I have looped in a few more people to see if they thinks this makes sense. Kind regards Uffe > --- > drivers/mmc/host/dw_mmc.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c > index 29a1afa..3ee8f57 100644 > --- a/drivers/mmc/host/dw_mmc.c > +++ b/drivers/mmc/host/dw_mmc.c > @@ -1065,8 +1065,8 @@ static void dw_mci_ctrl_thld(struct dw_mci *host, struct mmc_data *data) > * It's used when HS400 mode is enabled. > */ > if (data->flags & MMC_DATA_WRITE && > - !(host->timing != MMC_TIMING_MMC_HS400)) > - return; > + host->timing != MMC_TIMING_MMC_HS400) > + goto disable; > > if (data->flags & MMC_DATA_WRITE) > enable = SDMMC_CARD_WR_THR_EN; > @@ -1074,7 +1074,8 @@ static void dw_mci_ctrl_thld(struct dw_mci *host, struct mmc_data *data) > enable = SDMMC_CARD_RD_THR_EN; > > if (host->timing != MMC_TIMING_MMC_HS200 && > - host->timing != MMC_TIMING_UHS_SDR104) > + host->timing != MMC_TIMING_UHS_SDR104 && > + host->timing != MMC_TIMING_MMC_HS400) > goto disable; > > blksz_depth = blksz / (1 << host->data_shift); > -- > 2.8.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2018/6/11 20:20, Ulf Hansson wrote: > + Shawn Lin, Evgeniy Didin, Doug Andersson > > On 29 May 2018 at 12:38, Qing Xia <xiaqing17@hisilicon.com> wrote: >> From: x00270170 <xiaqing17@hisilicon.com> >> >> Card write threshold control is supposed to be set since controller >> version 2.80a for data write in HS400 mode and data read in >> HS200/HS400/SDR104 mode. However the current code returns without >> configuring it in the case of data writing in HS400 mode. >> Meanwhile the patch fixes that the current code goes to >> 'disable' when doing data reading in HS400 mode. >> I'm more or less unable to review this, as I don't have 2.80a databook, nor a such platform to verify it. :( >> Signed-off-by: Qing Xia <xiaqing17@hisilicon.com> > > This looks good to me. However, it seems like Jaehoon has been busy, > no response yet. > > I have looped in a few more people to see if they thinks this makes sense. > > Kind regards > Uffe > >> --- >> drivers/mmc/host/dw_mmc.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c >> index 29a1afa..3ee8f57 100644 >> --- a/drivers/mmc/host/dw_mmc.c >> +++ b/drivers/mmc/host/dw_mmc.c >> @@ -1065,8 +1065,8 @@ static void dw_mci_ctrl_thld(struct dw_mci *host, struct mmc_data *data) >> * It's used when HS400 mode is enabled. >> */ >> if (data->flags & MMC_DATA_WRITE && >> - !(host->timing != MMC_TIMING_MMC_HS400)) >> - return; >> + host->timing != MMC_TIMING_MMC_HS400) >> + goto disable; >> >> if (data->flags & MMC_DATA_WRITE) >> enable = SDMMC_CARD_WR_THR_EN; >> @@ -1074,7 +1074,8 @@ static void dw_mci_ctrl_thld(struct dw_mci *host, struct mmc_data *data) >> enable = SDMMC_CARD_RD_THR_EN; >> >> if (host->timing != MMC_TIMING_MMC_HS200 && >> - host->timing != MMC_TIMING_UHS_SDR104) >> + host->timing != MMC_TIMING_UHS_SDR104 && >> + host->timing != MMC_TIMING_MMC_HS400) >> goto disable; >> >> blksz_depth = blksz / (1 << host->data_shift); >> -- >> 2.8.1 >> > > >
On 11 June 2018 at 14:20, Ulf Hansson <ulf.hansson@linaro.org> wrote: > + Shawn Lin, Evgeniy Didin, Doug Andersson > > On 29 May 2018 at 12:38, Qing Xia <xiaqing17@hisilicon.com> wrote: >> From: x00270170 <xiaqing17@hisilicon.com> >> >> Card write threshold control is supposed to be set since controller >> version 2.80a for data write in HS400 mode and data read in >> HS200/HS400/SDR104 mode. However the current code returns without >> configuring it in the case of data writing in HS400 mode. >> Meanwhile the patch fixes that the current code goes to >> 'disable' when doing data reading in HS400 mode. >> >> Signed-off-by: Qing Xia <xiaqing17@hisilicon.com> > > This looks good to me. However, it seems like Jaehoon has been busy, > no response yet. > > I have looped in a few more people to see if they thinks this makes sense. It sounds this should be a fix for regression (also confirmed off-list). Could you please resend and by adding a fixes tag? In the meantime, I have applied this for my next branch, as to get it tested. I can move it later to fixes if needed. [...] Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Mon, Jun 11, 2018 at 7:17 AM, Shawn Lin <shawn.lin@rock-chips.com> wrote: > On 2018/6/11 20:20, Ulf Hansson wrote: >> >> + Shawn Lin, Evgeniy Didin, Doug Andersson >> >> On 29 May 2018 at 12:38, Qing Xia <xiaqing17@hisilicon.com> wrote: >>> >>> From: x00270170 <xiaqing17@hisilicon.com> >>> >>> Card write threshold control is supposed to be set since controller >>> version 2.80a for data write in HS400 mode and data read in >>> HS200/HS400/SDR104 mode. However the current code returns without >>> configuring it in the case of data writing in HS400 mode. >>> Meanwhile the patch fixes that the current code goes to >>> 'disable' when doing data reading in HS400 mode. >>> > > I'm more or less unable to review this, as I don't have 2.80a databook, > nor a such platform to verify it. :( Sorry for not responding earlier. I didn't have a lot of context here so reviewing it never made it to the top of my queue. ...but quickly checking what I can... I'm in almost the same boat as Shawn. I don't have any HS400 hardware using dw_mmc available to me, but I do seem to have the 2.80a databook hanging around and it agrees that we need to enable the card write threshold for HS400 writes. That also matches the comments, so the patch seems right to me. Probably Jaehoon would make a better reviewer since he submitted the original code and also has more familiarity with HS400 on dw_mmc. I don't think I have enough context to give a full Reviewed-by, but in the very least I can confirm that the patch seems sane. -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c index 29a1afa..3ee8f57 100644 --- a/drivers/mmc/host/dw_mmc.c +++ b/drivers/mmc/host/dw_mmc.c @@ -1065,8 +1065,8 @@ static void dw_mci_ctrl_thld(struct dw_mci *host, struct mmc_data *data) * It's used when HS400 mode is enabled. */ if (data->flags & MMC_DATA_WRITE && - !(host->timing != MMC_TIMING_MMC_HS400)) - return; + host->timing != MMC_TIMING_MMC_HS400) + goto disable; if (data->flags & MMC_DATA_WRITE) enable = SDMMC_CARD_WR_THR_EN; @@ -1074,7 +1074,8 @@ static void dw_mci_ctrl_thld(struct dw_mci *host, struct mmc_data *data) enable = SDMMC_CARD_RD_THR_EN; if (host->timing != MMC_TIMING_MMC_HS200 && - host->timing != MMC_TIMING_UHS_SDR104) + host->timing != MMC_TIMING_UHS_SDR104 && + host->timing != MMC_TIMING_MMC_HS400) goto disable; blksz_depth = blksz / (1 << host->data_shift);