[U-Boot] [PATCH] mmc: add wrappers for MMC block_{read, write, erase}

[Jaehoon Chung]

Hi, Steve.

On 05/29/2014 07:15 AM, Steve Rae wrote:
> Each wrapper function:
> - switches to the specified physical partition, then
> - performs the original function, and then
> - switches back to the original physical partition
> where the physical partition (aka HW partition) is
>   0=User, 1=Boot1, 2=Boot2, etc.
> 
> Signed-off-by: Steve Rae <srae at broadcom.com>
> ---
> based on a discussion: http://lists.denx.de/pipermail/u-boot/2014-April/178171.html
> The original calling code is (for example):
>   mmc->block_dev.block_read(dev_num, start, blkcnt, buffer)
> Therefore, these wrappers use the following naming convention:
>   mmc_block_read_hwpart(dev_num, part_num, start, blkcnt, buffer)
> "hwpart" comes from: Stephen Warren <swarren at nvidia.com>
> 
>  drivers/mmc/Makefile     |  1 +
>  drivers/mmc/mmc_hwpart.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/mmc.h            | 10 +++++++
>  3 files changed, 86 insertions(+)
>  create mode 100644 drivers/mmc/mmc_hwpart.c
> 
> diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
> index 4c6ab9e..04f87f9 100644
> --- a/drivers/mmc/Makefile
> +++ b/drivers/mmc/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_FSL_ESDHC) += fsl_esdhc.o
>  obj-$(CONFIG_FTSDC010) += ftsdc010_mci.o
>  obj-$(CONFIG_FTSDC021) += ftsdc021_sdhci.o
>  obj-$(CONFIG_GENERIC_MMC) += mmc.o
> +obj-$(CONFIG_GENERIC_MMC) += mmc_hwpart.o
>  obj-$(CONFIG_GENERIC_ATMEL_MCI) += gen_atmel_mci.o
>  obj-$(CONFIG_MMC_SPI) += mmc_spi.o
>  obj-$(CONFIG_ARM_PL180_MMCI) += arm_pl180_mmci.o
> diff --git a/drivers/mmc/mmc_hwpart.c b/drivers/mmc/mmc_hwpart.c
> new file mode 100644
> index 0000000..1c29f8f
> --- /dev/null
> +++ b/drivers/mmc/mmc_hwpart.c
> @@ -0,0 +1,75 @@
> +/*
> + * Copyright 2014 Broadcom Corporation.
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#include <mmc.h>
> +
> +static int switch_part(struct mmc *mmc,
> +		       int dev,
> +		       unsigned int chk_part_num,
> +		       unsigned int part_num)
> +{
> +	if (!mmc)
> +		return -1;
> +
> +	if (mmc->part_num != chk_part_num) {
> +		if (mmc_switch_part(dev, part_num)) {
> +			printf("MMC partition switch to %d failed [dev=%d]\n",
> +			       part_num, dev);
> +			return -1;
> +		}
> +	}
> +	return 0;
> +}
> +
> +unsigned long mmc_block_read_hwpart(int dev,
> +				    unsigned int part_num,
> +				    lbaint_t start,
> +				    lbaint_t blkcnt,
> +				    void *buffer)
> +{
> +	unsigned long rc = 0;
> +	struct mmc *mmc = find_mmc_device(dev);
> +
> +	if (switch_part(mmc, dev, part_num, part_num))
> +		return 0;
return 0 is right?

> +	rc = mmc->block_dev.block_read(dev, start, blkcnt, buffer);
> +	switch_part(mmc, dev, part_num, mmc->part_num);

Didn't need to check whether switched partition or not? And if block_read is failed?

> +
> +	return rc;
> +}
> +
> +unsigned long mmc_block_write_hwpart(int dev,
> +				     unsigned int part_num,
> +				     lbaint_t start,
> +				     lbaint_t blkcnt,
> +				     const void *buffer)
> +{
> +	unsigned long rc = 0;
> +	struct mmc *mmc = find_mmc_device(dev);
> +
> +	if (switch_part(mmc, dev, part_num, part_num))
> +		return 0;

ditto..

> +	rc = mmc->block_dev.block_write(dev, start, blkcnt, buffer);
> +	switch_part(mmc, dev, part_num, mmc->part_num);
> +
> +	return rc;
> +}
> +
> +unsigned long mmc_block_erase_hwpart(int dev,
> +				     unsigned int part_num,
> +				     lbaint_t start,
> +				     lbaint_t blkcnt)
> +{
> +	unsigned long rc = -1;
Why did you assign to "-1"?

> +	struct mmc *mmc = find_mmc_device(dev);
> +
> +	if (switch_part(mmc, dev, part_num, part_num))
> +		return -1;
At here, return -1?

Best Regards,
Jaehoon Chung
> +	rc = mmc->block_dev.block_erase(dev, start, blkcnt);
> +	switch_part(mmc, dev, part_num, mmc->part_num);
> +
> +	return rc;
> +}
> diff --git a/include/mmc.h b/include/mmc.h
> index a3a100b..4871c08 100644
> --- a/include/mmc.h
> +++ b/include/mmc.h
> @@ -347,6 +347,16 @@ int mmc_rpmb_read(struct mmc *mmc, void *addr, unsigned short blk,
>  		  unsigned short cnt, unsigned char *key);
>  int mmc_rpmb_write(struct mmc *mmc, void *addr, unsigned short blk,
>  		   unsigned short cnt, unsigned char *key);
> +/* Functions to read/write/erase from the specified HW partition */
> +unsigned long mmc_block_read_hwpart(int dev, unsigned int part_num,
> +				    lbaint_t start, lbaint_t blkcnt,
> +				    void *buffer);
> +unsigned long mmc_block_write_hwpart(int dev, unsigned int part_num,
> +				     lbaint_t start, lbaint_t blkcnt,
> +				     const void *buffer);
> +
> +unsigned long mmc_block_erase_hwpart(int dev, unsigned int part_num,
> +				     lbaint_t start, lbaint_t blkcnt);
>  /**
>   * Start device initialization and return immediately; it does not block on
>   * polling OCR (operation condition register) status.  Then you should call
>