[U-Boot] [PATCH v3] SPL: add support to boot from a partition type

[Alexander Graf]

On 31/01/2017 09:41, Dalon Westergreen wrote:
> From: Dalon Westergreen <dalon.westergreen at intel.com>
>
> the socfpga bootrom supports mmc booting from either a raw image
> starting at 0x0, or from a partition of type 0xa2.  This patch
> adds support for locating the boot image in the first type 0xa2
> partition found.
>
> Assigned a partition number of -1 will cause a search for a
> partition of type CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION_TYPE
> and use it to find the u-boot image
>
> Signed-off-by: Dalon Westergreen <dalon.westergreen at intel.com>

For reviewing, it makes life much easier if the change log is also 
present in the individual patches of a patch set. In this case - since 
it's only one patch - only in the patch set.

The best way to do that in git is to just write a

--

line after your Signed-Off-By line and put the change log there. When 
the patch then gets applied with git am later, git will drop everything 
below the -- line. However, during git rebase and git commit --amend, 
the -- line does not have any effect, so you can easily keep the change 
log inside the patch itself.

> ---
>  common/spl/Kconfig   | 17 +++++++++++++++++
>  common/spl/spl_mmc.c | 13 +++++++++++++
>  disk/part_dos.c      |  1 +
>  include/part.h       |  3 +++
>  4 files changed, 34 insertions(+)
>
> diff --git a/common/spl/Kconfig b/common/spl/Kconfig
> index b2ba492..37e002f 100644
> --- a/common/spl/Kconfig
> +++ b/common/spl/Kconfig
> @@ -96,6 +96,23 @@ config SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR
>  	  Address on the MMC to load U-Boot from, when the MMC is being used
>  	  in raw mode. Units: MMC sectors (1 sector = 512 bytes).
>
> +config SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION_TYPE
> +	bool "MMC raw mode: by partition type"
> +	depends on SPL && DOS_PARTITION
> +	default y if ARCH_SOCFPGA
> +	help
> +	  Use partition type for specifying U-Boot partition on MMC/SD in
> +	  raw mode. U-Boot will be loaded from the first partition of this
> +	  type to be found.
> +
> +config SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION_TYPE
> +	hex "Partition Type on the MMC to load U-Boot from"
> +	depends on SPL && SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION_TYPE
> +	default 0xa2
> +	help
> +	  Partition Type on the MMC to load U-Boot from, when the MMC is being
> +	  used in raw mode.
> +
>  config TPL
>  	bool
>  	depends on SPL && SUPPORT_TPL
> diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
> index 0cd355c..cce9584 100644
> --- a/common/spl/spl_mmc.c
> +++ b/common/spl/spl_mmc.c
> @@ -157,6 +157,19 @@ static int mmc_load_image_raw_partition(struct spl_image_info *spl_image,
>  	disk_partition_t info;
>  	int err;
>
> +#ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION_TYPE
> +	if ( partition == -1 ) {

This means you need to explicitly define 
CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION to -1 for every board that 
wants to use this feature, right? The only caller of this function I 
could find is:

                 err = mmc_load_image_raw_partition(spl_image, mmc,
                         CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION);

which means you're now making those two features mutually exclusive. Why 
not drop the whole partition == -1 check? In that case you would first 
search for a partition type, if that's not available fall back to a 
static partition number.

> +		/* Only support MBR so DOS_ENTRY_NUMBERS */
> +		for (partition = 1; partition <= DOS_ENTRY_NUMBERS; partition++) {
> +			err = part_get_info(mmc_get_blk_desc(mmc), partition, &info);
> +			if(err)
> +				continue;
> +			if(info.sys_ind == CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION_TYPE)
> +				break;
> +		}
> +	}
> +#endif
> +
>  	err = part_get_info(mmc_get_blk_desc(mmc), partition, &info);
>  	if (err) {
>  #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT

below here is the fallback case when 
CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION is not set. Since that is not 
a Kconfig option you can not depend on it inside Kconfig.

Ideally, you would convert CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION to 
Kconfig as well so you can depend on it :). Sorry for making you dig so 
deeply.

The rest looks very good to me :). Thanks a lot!

Alex

> diff --git a/disk/part_dos.c b/disk/part_dos.c
> index c77d881..7ede15e 100644
> --- a/disk/part_dos.c
> +++ b/disk/part_dos.c
> @@ -217,6 +217,7 @@ static int part_get_info_extended(struct blk_desc *dev_desc,
>  #if CONFIG_IS_ENABLED(PARTITION_UUIDS)
>  			sprintf(info->uuid, "%08x-%02x", disksig, part_num);
>  #endif
> +			info->sys_ind = pt->sys_ind;
>  			return 0;
>  		}
>
> diff --git a/include/part.h b/include/part.h
> index 9d0e20d..b6d1b33 100644
> --- a/include/part.h
> +++ b/include/part.h
> @@ -59,6 +59,9 @@ typedef struct disk_partition {
>  #ifdef CONFIG_PARTITION_TYPE_GUID
>  	char	type_guid[37];	/* type GUID as string, if exists	*/
>  #endif
> +#ifdef CONFIG_DOS_PARTITION
> +	uchar	sys_ind;	/* partition type 			*/
> +#endif
>  } disk_partition_t;
>
>  /* Misc _get_dev functions */
>