Re: [PATCH 1/5] disk: Add an option for partitions in SPL

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

Hi Simon,

On Fri, Mar 11, 2022 at 12:10:01PM -0700, Simon Glass wrote:
> In some cases we do not want to enable partition support in SPL. Add an
> option to allow this.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  disk/Kconfig               | 24 ++++++++++++++++++++----
>  disk/Makefile              |  6 +++---
>  drivers/block/blk-uclass.c |  2 +-
>  3 files changed, 24 insertions(+), 8 deletions(-)
> 
> diff --git a/disk/Kconfig b/disk/Kconfig
> index cee16a80bc2..13700322e97 100644
> --- a/disk/Kconfig
> +++ b/disk/Kconfig
> @@ -4,10 +4,6 @@ menu "Partition Types"
>  config PARTITIONS
>  	bool "Enable Partition Labels (disklabels) support"
>  	default y
> -	select SPL_SPRINTF if SPL
> -	select TPL_SPRINTF if TPL
> -	select SPL_STRTO if SPL
> -	select TPL_STRTO if TPL
>  	help
>  	  Partition Labels (disklabels) Supported:
>  	  Zero or more of the following:
> @@ -23,6 +19,26 @@ config PARTITIONS
>  	  you must configure support for at least one non-MTD partition type
>  	  as well.
>  
> +config SPL_PARTITIONS
> +	bool "Enable Partition Labels (disklabels) support in SPL"
> +	default y if PARTITIONS
> +	select SPL_SPRINTF
> +	select SPL_STRTO
> +	help
> +	  Enable this for base partition support in SPL. The required
> +	  partition table types shold be enabled separately. This add a
> +	  small amount of size to SPL, typically 500 bytes.

Despite of you aim here, this option doesn't work.
As I mentioned in
  https://lists.denx.de/pipermail/u-boot/2022-April/481258.html
phycore-rk3288_defconfig, for instance, compiles disk/part.o
(and disk/disk-uclass.o) in SPL image.

Basically, I think that CONFIG_(SPL_)PARTITIONS should not be
user-selectable. Instead, it should be implicitly selected
by any of CONFIG_(SPL_)XXX_PARTITION's.

-Takahiro Akashi

> +
> +config TPL_PARTITIONS
> +	bool "Enable Partition Labels (disklabels) support in TPL"
> +	default y if PARTITIONS
> +	select TPL_SPRINTF
> +	select TPL_STRTO
> +	help
> +	  Enable this for base partition support in SPL. The required
> +	  partition table types shold be enabled separately. This add a
> +	  small amount of size to SPL, typically 500 bytes.
> +
>  config MAC_PARTITION
>  	bool "Enable Apple's MacOS partition table"
>  	depends on PARTITIONS
> diff --git a/disk/Makefile b/disk/Makefile
> index ec37b74f5f4..ffd7b07f867 100644
> --- a/disk/Makefile
> +++ b/disk/Makefile
> @@ -5,9 +5,9 @@
>  
>  #ccflags-y += -DET_DEBUG -DDEBUG
>  
> -obj-$(CONFIG_PARTITIONS)	+= part.o
> -ifdef CONFIG_$(SPL_)BLK
> -obj-$(CONFIG_PARTITIONS) 	+= disk-uclass.o
> +obj-$(CONFIG_$(SPL_TPL_)PARTITIONS)	+= part.o
> +ifdef CONFIG_$(SPL_TPL_)BLK
> +obj-$(CONFIG_$(SPL_TPL_)PARTITIONS)	+= disk-uclass.o
>  endif
>  obj-$(CONFIG_$(SPL_)MAC_PARTITION)   += part_mac.o
>  obj-$(CONFIG_$(SPL_)DOS_PARTITION)   += part_dos.o
> diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
> index 58dc74e71f1..bcd18ed38b2 100644
> --- a/drivers/block/blk-uclass.c
> +++ b/drivers/block/blk-uclass.c
> @@ -712,7 +712,7 @@ int blk_unbind_all(int if_type)
>  
>  static int blk_post_probe(struct udevice *dev)
>  {
> -	if (IS_ENABLED(CONFIG_PARTITIONS) &&
> +	if (CONFIG_IS_ENABLED(PARTITIONS) &&
>  	    IS_ENABLED(CONFIG_HAVE_BLOCK_DEVICE)) {
>  		struct blk_desc *desc = dev_get_uclass_plat(dev);
>  
> -- 
> 2.35.1.723.g4982287a31-goog
>