On Wed, Apr 20, 2022 at 11:17:21AM +0900, AKASHI Takahiro wrote:
> Hi Tom,
> 
> On Tue, Apr 19, 2022 at 08:12:07AM -0400, Tom Rini wrote:
> > On Tue, Apr 19, 2022 at 01:11:23PM +0900, AKASHI Takahiro wrote:
> > > On Mon, Apr 18, 2022 at 11:09:38PM -0400, Tom Rini wrote:
> > > > On Tue, Apr 19, 2022 at 10:01:54AM +0900, AKASHI Takahiro wrote:
> > > > 
> > > > > Some defconfig enables CMD_PART even if none of any partition table
> > > > > types (CONFIG_*_PARTITION) are enabled.
> > > > > This will lead to the size growth in SPL/TPL code since disk/part.c
> > > > > will be compiled in any way.
> > > > > We will change disk/Kconfig later so that CONFIG_PARTITIONS is only
> > > > > enabled when, at least, one of CONFIG_*_PARTITION is enabled.
> > > > > 
> > > > > To make the build work (in particular, "part" command) correctly,
> > > > > a few functions should be defined as void functions in case of
> > > > > !CONFIG_PARTITIONS.
> > > > > 
> > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > > 
> > > > I guess I wonder why we don't just make CMD_PART depend on PARTITIONS
> > > > now and thus correct the few (single?) board that has this enabled
> > > > without underlying partition code by removing the can't be functional
> > > > cmd.
> > > 
> > > Well, that is partially what I did in my RFC and I thought
> > > that you declined to accept my change.
> > > Did I misunderstand you?
> > 
> > Yes, I wasn't clear, sorry for the confusion.  Just this part of the
> > series should be replaced with making CMD_PART depend on PARTITIONS and
> > if there really is a use case for 'part' without PARTITION support
> > (rather than it being an unintentionally enabled feature) we can deal
> > with it then.  The rest of the series looks good to me and I'll let
> > Heinrich comment on the EFI specific parts.
> 
> I do understand what you expect here, but, what I call, "nullified
> function" technique is already used in several places.
> For instance, take blk_get_device_part_str() function which has
> a nullified version of definition in include/part.h.
> It is used without explicit dependency on CONFIG_PARTITIONS at:
>         cmd/zfs.c
>         cmd/disk.c
>         cmd/reiser.c
>         cmd/fat.c
>         env/ext4.c
>         env/fat.c
> 
> So I would like to propose to create another patch that you expect (and
> what I did in RFC) instead of removing this patch because the latter
> has no negative effect.
> 
> If you agree, I will post it as a separate patch.

OK, lets go with that then, thanks!

-- 
Tom