| Message ID | 20210504161222.101536-7-ulf.hansson@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Initital support for new power/perf features for SD cards | expand |
On Tue, May 4, 2021 at 6:12 PM Ulf Hansson <ulf.hansson@linaro.org> wrote: > The function mmc_send_cxd_data() sends a data read command of ADTC type and > prepares to receive an R1 response. To make it even more re-usable, let's > extend it with another in-parameter for the command argument. While at it, > let's also rename the function to mmc_send_adtc_data() as it better > describes its purpose. > > Note that, this change doesn't add any new users of the function. Instead > that is done from subsequent changes. > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On Tue, May 4, 2021 at 6:12 PM Ulf Hansson <ulf.hansson@linaro.org> wrote: > * NOTE: void *buf, caller for the buf is required to use DMA-capable > * buffer or on-stack buffer (with some overhead in callee). > */ > -static int > -mmc_send_cxd_data(struct mmc_card *card, struct mmc_host *host, > - u32 opcode, void *buf, unsigned len) > +int mmc_send_adtc_data(struct mmc_card *card, struct mmc_host *host, u32 opcode, > + u32 args, void *buf, unsigned len) Just a note here (the change is good) When applying please add some kerneldoc above mmc_send_adtc_data() and expand the ADTC acronym and add some info explaining what it is maybe a small protocol ref or so, so readers of the code get an intuitive feeling for what this function does and what ADTC is. Yours, Linus Walleijq
On 2021/5/5 0:12, Ulf Hansson wrote: > The function mmc_send_cxd_data() sends a data read command of ADTC type and > prepares to receive an R1 response. To make it even more re-usable, let's > extend it with another in-parameter for the command argument. While at it, > let's also rename the function to mmc_send_adtc_data() as it better > describes its purpose. > > Note that, this change doesn't add any new users of the function. Instead > that is done from subsequent changes. Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com> > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > --- > drivers/mmc/core/mmc_ops.c | 11 +++++------ > drivers/mmc/core/mmc_ops.h | 2 ++ > 2 files changed, 7 insertions(+), 6 deletions(-) > > diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c > index 653627fe02a3..b1da8f1950ee 100644 > --- a/drivers/mmc/core/mmc_ops.c > +++ b/drivers/mmc/core/mmc_ops.c > @@ -252,9 +252,8 @@ mmc_send_cxd_native(struct mmc_host *host, u32 arg, u32 *cxd, int opcode) > * NOTE: void *buf, caller for the buf is required to use DMA-capable > * buffer or on-stack buffer (with some overhead in callee). > */ > -static int > -mmc_send_cxd_data(struct mmc_card *card, struct mmc_host *host, > - u32 opcode, void *buf, unsigned len) > +int mmc_send_adtc_data(struct mmc_card *card, struct mmc_host *host, u32 opcode, > + u32 args, void *buf, unsigned len) > { > struct mmc_request mrq = {}; > struct mmc_command cmd = {}; > @@ -265,7 +264,7 @@ mmc_send_cxd_data(struct mmc_card *card, struct mmc_host *host, > mrq.data = &data; > > cmd.opcode = opcode; > - cmd.arg = 0; > + cmd.arg = args; > > /* NOTE HACK: the MMC_RSP_SPI_R1 is always correct here, but we > * rely on callers to never use this with "native" calls for reading > @@ -311,7 +310,7 @@ static int mmc_spi_send_cxd(struct mmc_host *host, u32 *cxd, u32 opcode) > if (!cxd_tmp) > return -ENOMEM; > > - ret = mmc_send_cxd_data(NULL, host, opcode, cxd_tmp, 16); > + ret = mmc_send_adtc_data(NULL, host, opcode, 0, cxd_tmp, 16); > if (ret) > goto err; > > @@ -359,7 +358,7 @@ int mmc_get_ext_csd(struct mmc_card *card, u8 **new_ext_csd) > if (!ext_csd) > return -ENOMEM; > > - err = mmc_send_cxd_data(card, card->host, MMC_SEND_EXT_CSD, ext_csd, > + err = mmc_send_adtc_data(card, card->host, MMC_SEND_EXT_CSD, 0, ext_csd, > 512); > if (err) > kfree(ext_csd); > diff --git a/drivers/mmc/core/mmc_ops.h b/drivers/mmc/core/mmc_ops.h > index aca66c128804..2b1d730e56bf 100644 > --- a/drivers/mmc/core/mmc_ops.h > +++ b/drivers/mmc/core/mmc_ops.h > @@ -26,6 +26,8 @@ int mmc_set_dsr(struct mmc_host *host); > int mmc_go_idle(struct mmc_host *host); > int mmc_send_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr); > int mmc_set_relative_addr(struct mmc_card *card); > +int mmc_send_adtc_data(struct mmc_card *card, struct mmc_host *host, u32 opcode, > + u32 args, void *buf, unsigned len); > int mmc_send_csd(struct mmc_card *card, u32 *csd); > int __mmc_send_status(struct mmc_card *card, u32 *status, unsigned int retries); > int mmc_send_status(struct mmc_card *card, u32 *status); >
On Thu, 6 May 2021 at 15:00, Linus Walleij <linus.walleij@linaro.org> wrote: > > On Tue, May 4, 2021 at 6:12 PM Ulf Hansson <ulf.hansson@linaro.org> wrote: > > > * NOTE: void *buf, caller for the buf is required to use DMA-capable > > * buffer or on-stack buffer (with some overhead in callee). > > */ > > -static int > > -mmc_send_cxd_data(struct mmc_card *card, struct mmc_host *host, > > - u32 opcode, void *buf, unsigned len) > > +int mmc_send_adtc_data(struct mmc_card *card, struct mmc_host *host, u32 opcode, > > + u32 args, void *buf, unsigned len) > > Just a note here (the change is good) > > When applying please add some kerneldoc above mmc_send_adtc_data() > and expand the ADTC acronym and add some info explaining what it > is maybe a small protocol ref or so, so readers of the code get an > intuitive feeling for what this function does and what ADTC is. Thanks for the suggestion and the reviews, I look into this! Kind regards Uffe
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c index 653627fe02a3..b1da8f1950ee 100644 --- a/drivers/mmc/core/mmc_ops.c +++ b/drivers/mmc/core/mmc_ops.c @@ -252,9 +252,8 @@ mmc_send_cxd_native(struct mmc_host *host, u32 arg, u32 *cxd, int opcode) * NOTE: void *buf, caller for the buf is required to use DMA-capable * buffer or on-stack buffer (with some overhead in callee). */ -static int -mmc_send_cxd_data(struct mmc_card *card, struct mmc_host *host, - u32 opcode, void *buf, unsigned len) +int mmc_send_adtc_data(struct mmc_card *card, struct mmc_host *host, u32 opcode, + u32 args, void *buf, unsigned len) { struct mmc_request mrq = {}; struct mmc_command cmd = {}; @@ -265,7 +264,7 @@ mmc_send_cxd_data(struct mmc_card *card, struct mmc_host *host, mrq.data = &data; cmd.opcode = opcode; - cmd.arg = 0; + cmd.arg = args; /* NOTE HACK: the MMC_RSP_SPI_R1 is always correct here, but we * rely on callers to never use this with "native" calls for reading @@ -311,7 +310,7 @@ static int mmc_spi_send_cxd(struct mmc_host *host, u32 *cxd, u32 opcode) if (!cxd_tmp) return -ENOMEM; - ret = mmc_send_cxd_data(NULL, host, opcode, cxd_tmp, 16); + ret = mmc_send_adtc_data(NULL, host, opcode, 0, cxd_tmp, 16); if (ret) goto err; @@ -359,7 +358,7 @@ int mmc_get_ext_csd(struct mmc_card *card, u8 **new_ext_csd) if (!ext_csd) return -ENOMEM; - err = mmc_send_cxd_data(card, card->host, MMC_SEND_EXT_CSD, ext_csd, + err = mmc_send_adtc_data(card, card->host, MMC_SEND_EXT_CSD, 0, ext_csd, 512); if (err) kfree(ext_csd); diff --git a/drivers/mmc/core/mmc_ops.h b/drivers/mmc/core/mmc_ops.h index aca66c128804..2b1d730e56bf 100644 --- a/drivers/mmc/core/mmc_ops.h +++ b/drivers/mmc/core/mmc_ops.h @@ -26,6 +26,8 @@ int mmc_set_dsr(struct mmc_host *host); int mmc_go_idle(struct mmc_host *host); int mmc_send_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr); int mmc_set_relative_addr(struct mmc_card *card); +int mmc_send_adtc_data(struct mmc_card *card, struct mmc_host *host, u32 opcode, + u32 args, void *buf, unsigned len); int mmc_send_csd(struct mmc_card *card, u32 *csd); int __mmc_send_status(struct mmc_card *card, u32 *status, unsigned int retries); int mmc_send_status(struct mmc_card *card, u32 *status);
The function mmc_send_cxd_data() sends a data read command of ADTC type and prepares to receive an R1 response. To make it even more re-usable, let's extend it with another in-parameter for the command argument. While at it, let's also rename the function to mmc_send_adtc_data() as it better describes its purpose. Note that, this change doesn't add any new users of the function. Instead that is done from subsequent changes. Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> --- drivers/mmc/core/mmc_ops.c | 11 +++++------ drivers/mmc/core/mmc_ops.h | 2 ++ 2 files changed, 7 insertions(+), 6 deletions(-)