diff mbox series

[06/11] mmc: core: Prepare mmc_send_cxd_data() to be re-used for additional cmds

Message ID 20210504161222.101536-7-ulf.hansson@linaro.org
State New
Headers show
Series Initital support for new power/perf features for SD cards | expand

Commit Message

Ulf Hansson May 4, 2021, 4:12 p.m. UTC 
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(-)

-- 
2.25.1

Comments

Linus Walleij May 6, 2021, 12:56 p.m. UTC | #1 
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
Linus Walleij May 6, 2021, 1 p.m. UTC | #2 
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
Shawn Lin May 7, 2021, 1:53 a.m. UTC | #3 
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);

>
Ulf Hansson May 7, 2021, 7:30 a.m. UTC | #4 
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 mbox series

Patch

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);