| Message ID | 20210907151204.118861-3-huobean@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Two nimor eMMc changes | expand |
On Tue, 7 Sept 2021 at 17:12, Bean Huo <huobean@gmail.com> wrote: > > From: Bean Huo <beanhuo@micron.com> > > In case CQE is enabled, the timeout value of data transmission is always > set to be maximum in sdhci_cqe_enable(), so, calculating its timeout value > is obviously superfluous. Change to return directly, which can save some CPU > cycle time. > > Signed-off-by: Bean Huo <beanhuo@micron.com> > --- > drivers/mmc/core/core.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c > index 240c5af793dc..0b571a3bc988 100644 > --- a/drivers/mmc/core/core.c > +++ b/drivers/mmc/core/core.c > @@ -649,6 +649,7 @@ EXPORT_SYMBOL(mmc_wait_for_cmd); > void mmc_set_data_timeout(struct mmc_data *data, const struct mmc_card *card) > { > unsigned int mult; > + struct mmc_host *host = card->host; > > /* > * SDIO cards only define an upper 1 s limit on access. > @@ -659,6 +660,13 @@ void mmc_set_data_timeout(struct mmc_data *data, const struct mmc_card *card) > return; > } > > + /* > + * In case CQE is enabled, the timeout will be set a maximum timeout in > + * sdhci_cqe_enable(), so, no need to go through the below algorithm. > + */ > + if (host->cqe_enabled) I don't think this is a good idea. For example, host->cqe_enabled is set for the hsq case well. > + return; > + > /* > * SD cards use a 100 multiplier rather than 10 > */ Kind regards Uffe
On Tue, 2021-09-14 at 10:13 +0200, Ulf Hansson wrote: > > } > > + /* > > + * In case CQE is enabled, the timeout will be set a > > maximum timeout in > > + * sdhci_cqe_enable(), so, no need to go through the below > > algorithm. > > + */ > > + if (host->cqe_enabled) > > > I don't think this is a good idea. For example, host->cqe_enabled is > > set for the hsq case well. Uffe, My apologies for this, I forgot to check hsq, hsq will call sdhci_send_command() as well. How about changing it to this? diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 240c5af793dc..7235e398ef93 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -649,6 +649,7 @@ EXPORT_SYMBOL(mmc_wait_for_cmd); void mmc_set_data_timeout(struct mmc_data *data, const struct mmc_card *card) { unsigned int mult; + struct mmc_host *host = card->host; /* * SDIO cards only define an upper 1 s limit on access. @@ -659,6 +660,13 @@ void mmc_set_data_timeout(struct mmc_data *data, const struct mmc_card *card) return; } + /* + * For the CQE use case, the data transfer timeout will be set a maximum + * timeout value in HW timer in function sdhci_cqe_enable(), so, no need + * to go through the below algorithm. + */ + if (host->cqe_enabled && !host->hsq_enabled) + return; /* * SD cards use a 100 multiplier rather than 10 */ I have another timeout change associated with data transfer as well, if this change is acceptible, I will submit it with that together. Kind regards, Bean
On Wed, 15 Sept 2021 at 11:54, Bean Huo <huobean@gmail.com> wrote: > > On Tue, 2021-09-14 at 10:13 +0200, Ulf Hansson wrote: > > > } > > > + /* > > > + * In case CQE is enabled, the timeout will be set a > > > maximum timeout in > > > + * sdhci_cqe_enable(), so, no need to go through the below > > > algorithm. > > > + */ > > > + if (host->cqe_enabled) > > > > > > I don't think this is a good idea. For example, host->cqe_enabled is > > > > set for the hsq case well. > > Uffe, > > My apologies for this, I forgot to check hsq, hsq will call > sdhci_send_command() as well. > > > How about changing it to this? > > > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c > index 240c5af793dc..7235e398ef93 100644 > --- a/drivers/mmc/core/core.c > +++ b/drivers/mmc/core/core.c > @@ -649,6 +649,7 @@ EXPORT_SYMBOL(mmc_wait_for_cmd); > void mmc_set_data_timeout(struct mmc_data *data, const struct mmc_card > *card) > { > unsigned int mult; > + struct mmc_host *host = card->host; > > /* > * SDIO cards only define an upper 1 s limit on access. > @@ -659,6 +660,13 @@ void mmc_set_data_timeout(struct mmc_data *data, > const struct mmc_card *card) > return; > } > > + /* > + * For the CQE use case, the data transfer timeout will be set > a maximum > + * timeout value in HW timer in function sdhci_cqe_enable(), > so, no need > + * to go through the below algorithm. > + */ > + if (host->cqe_enabled && !host->hsq_enabled) > + return; Are you really sure the timeout isn't used (or could make sense to be used for new cases)? For example, we also have mtk-sd, which doesn't make use of sdhci_cqe_enable(). > /* > * SD cards use a 100 multiplier rather than 10 > */ > > I have another timeout change associated with data transfer as well, if > this change is acceptible, I will submit it with that together. > > Kind regards, > Bean > Kind regards Uffe
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 240c5af793dc..0b571a3bc988 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -649,6 +649,7 @@ EXPORT_SYMBOL(mmc_wait_for_cmd); void mmc_set_data_timeout(struct mmc_data *data, const struct mmc_card *card) { unsigned int mult; + struct mmc_host *host = card->host; /* * SDIO cards only define an upper 1 s limit on access. @@ -659,6 +660,13 @@ void mmc_set_data_timeout(struct mmc_data *data, const struct mmc_card *card) return; } + /* + * In case CQE is enabled, the timeout will be set a maximum timeout in + * sdhci_cqe_enable(), so, no need to go through the below algorithm. + */ + if (host->cqe_enabled) + return; + /* * SD cards use a 100 multiplier rather than 10 */