[PATCH v7 04/22] lib: Makefile: build crc7.c when CONFIG_MMC_SPI

[Jagan Teki]

On Sat, May 2, 2020 at 6:45 PM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> Am May 2, 2020 11:47:10 AM UTC schrieb Bin Meng <bmeng.cn at gmail.com>:
> >Hi Heinrich,
> >
> >On Sat, May 2, 2020 at 6:30 PM Heinrich Schuchardt <xypron.glpk at gmx.de>
> >wrote:
> >>
> >> On 5/2/20 12:06 PM, Pragnesh Patel wrote:
> >> > When build U-Boot SPL, meet an issue of undefined reference to
> >> > 'crc7' for drivers/mmc/mmc_spi.c, so let's compile crc7.c when
> >> > CONFIG_MMC_SPI selected.
> >> >
> >> > Signed-off-by: Pragnesh Patel <pragnesh.patel at sifive.com>
> >> > Reviewed-by: Jagan Teki <jagan at amarulasolutions.com>
> >> > Reviewed-by: Bin Meng <bmeng.cn at gmail.com>
> >> > ---
> >> >  common/spl/Kconfig  | 6 ++++++
> >> >  drivers/mmc/Kconfig | 1 +
> >> >  lib/Makefile        | 1 +
> >> >  3 files changed, 8 insertions(+)
> >> >
> >> > diff --git a/common/spl/Kconfig b/common/spl/Kconfig
> >> > index ef5bf66696..d1f0e6bc4c 100644
> >> > --- a/common/spl/Kconfig
> >> > +++ b/common/spl/Kconfig
> >> > @@ -401,6 +401,12 @@ config SPL_CRC32_SUPPORT
> >> >         for detected accidental image corruption. For secure
> >applications you
> >> >         should consider SHA1 or SHA256.
> >> >
> >> > +config SPL_CRC7_SUPPORT
> >> > +     bool "Support CRC7"
> >> > +     help
> >> > +       Enable CRC7 hashing for drivers which are using in SPL.
> >> > +       This is a 32-bit checksum value that can be used to verify
> >images.
> >> > +
> >> >  config SPL_MD5_SUPPORT
> >> >       bool "Support MD5"
> >> >       depends on SPL_FIT
> >> > diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> >> > index 8f0df568b9..139599072a 100644
> >> > --- a/drivers/mmc/Kconfig
> >> > +++ b/drivers/mmc/Kconfig
> >> > @@ -49,6 +49,7 @@ if MMC
> >> >  config MMC_SPI
> >> >       bool "Support for SPI-based MMC controller"
> >> >       depends on DM_MMC && DM_SPI
> >> > +     select SPL_CRC7_SUPPORT if SPL
> >> >       help
> >> >         This selects SPI-based MMC controllers.
> >> >         If you have an MMC controller on a SPI bus, say Y here.
> >> > diff --git a/lib/Makefile b/lib/Makefile
> >> > index c6f862b0c2..fcd934857f 100644
> >> > --- a/lib/Makefile
> >> > +++ b/lib/Makefile
> >> > @@ -80,6 +80,7 @@ endif
> >> >
> >> >  ifdef CONFIG_SPL_BUILD
> >> >  obj-$(CONFIG_SPL_YMODEM_SUPPORT) += crc16.o
> >> > +obj-$(CONFIG_SPL_CRC7_SUPPORT) += crc7.o
> >>
> >> crc7.o is not needed in main U-Boot if MMC_SPI is not selected.
> >>
> >> So instead of adding a new configuration variable simply correct the
> >> existing line in lib/Makefile
> >>
> >> -obj-y += crc7.o
> >> +obj-$(CONFIG_MMC_SPI) += crc7.o
> >
> >This looks incorrect to me. CRC7 can be useful for other drivers too.
> >It should not depend on CONFIG_MMC_SPI.
>
> Using this argument you would always compile everything. Compiling files that are not used is simply a waste of CPU time.
>
> Following your argument. you could also always compile crc7.c for SPL and hope the compiler will eliminate it.
>
> Either way we do not need a new config symbol.

This is one of the reasons I just suggested that the mark 'depends on
MMC_SPI' at the very initial patch.

Marking config SPL_CRC7_SUPPORT which depends on MMC_SPI would work for me.

Jagan.