-----Original Message----- From: U-Boot u-boot-bounces@lists.denx.de On Behalf Of Pragnesh Patel Sent: 05 May 2020 21:25 To: Jagan Teki jagan@amarulasolutions.com; Heinrich Schuchardt xypron.glpk@gmx.de; Bin Meng bmeng.cn@gmail.com Cc: U-Boot Mailing List u-boot@lists.denx.de; Atish Patra atish.patra@wdc.com; Palmer Dabbelt palmerdabbelt@google.com; Paul Walmsley paul.walmsley@sifive.com; Troy Benjegerdes troy.benjegerdes@sifive.com; Anup Patel anup.patel@wdc.com; Sagar Kadam sagar.kadam@sifive.com; Rick Chen rick@andestech.com; Peng Fan peng.fan@nxp.com; Lukasz Majewski lukma@denx.de; Simon Goldschmidt simon.k.r.goldschmidt@gmail.com; Simon Glass sjg@chromium.org; Markus Klotzbuecher markus.klotzbuecher@kistler.com; Baruch Siach baruch@tkos.co.il; Joel Johnson mrjoel@lixil.net; Anatolij Gustschin agust@denx.de; AKASHI Takahiro takahiro.akashi@linaro.org; Marek Behún marek.behun@nic.cz Subject: RE: [PATCH v7 04/22] lib: Makefile: build crc7.c when CONFIG_MMC_SPI
-----Original Message----- From: U-Boot u-boot-bounces@lists.denx.de On Behalf Of Pragnesh Patel Sent: 04 May 2020 11:15 To: Jagan Teki jagan@amarulasolutions.com; Heinrich Schuchardt xypron.glpk@gmx.de; Bin Meng bmeng.cn@gmail.com Cc: U-Boot Mailing List u-boot@lists.denx.de; Atish Patra atish.patra@wdc.com; Palmer Dabbelt palmerdabbelt@google.com;
Paul
Walmsley paul.walmsley@sifive.com; Troy Benjegerdes troy.benjegerdes@sifive.com; Anup Patel anup.patel@wdc.com; Sagar Kadam sagar.kadam@sifive.com; Rick Chen rick@andestech.com; Peng Fan peng.fan@nxp.com; Lukasz Majewski lukma@denx.de; Simon Goldschmidt simon.k.r.goldschmidt@gmail.com; Simon Glass sjg@chromium.org; Markus Klotzbuecher markus.klotzbuecher@kistler.com; Baruch Siach baruch@tkos.co.il; Joel Johnson mrjoel@lixil.net; Anatolij Gustschin agust@denx.de; AKASHI Takahiro takahiro.akashi@linaro.org; Marek Behún marek.behun@nic.cz Subject: RE: [PATCH v7 04/22] lib: Makefile: build crc7.c when CONFIG_MMC_SPI
-----Original Message----- From: Jagan Teki jagan@amarulasolutions.com Sent: 02 May 2020 21:04 To: Heinrich Schuchardt xypron.glpk@gmx.de; Pragnesh Patel pragnesh.patel@sifive.com; Bin Meng bmeng.cn@gmail.com Cc: U-Boot Mailing List u-boot@lists.denx.de; Atish Patra atish.patra@wdc.com; Palmer Dabbelt palmerdabbelt@google.com;
Paul
Walmsley paul.walmsley@sifive.com; Troy Benjegerdes troy.benjegerdes@sifive.com; Anup Patel anup.patel@wdc.com; Sagar Kadam sagar.kadam@sifive.com; Rick Chen rick@andestech.com; Peng Fan peng.fan@nxp.com; Lukasz Majewski lukma@denx.de; Simon Goldschmidt simon.k.r.goldschmidt@gmail.com; Simon Glass sjg@chromium.org; Markus Klotzbuecher markus.klotzbuecher@kistler.com; Baruch Siach baruch@tkos.co.il; Joel Johnson mrjoel@lixil.net; Anatolij Gustschin agust@denx.de; AKASHI Takahiro takahiro.akashi@linaro.org; Marek Behún marek.behun@nic.cz Subject: Re: [PATCH v7 04/22] lib: Makefile: build crc7.c when CONFIG_MMC_SPI
[External Email] Do not click links or attachments unless you recognize the sender and know the content is safe
On Sat, May 2, 2020 at 6:45 PM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am May 2, 2020 11:47:10 AM UTC schrieb Bin Meng
Hi Heinrich,
On Sat, May 2, 2020 at 6:30 PM Heinrich Schuchardt xypron.glpk@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@sifive.com > Reviewed-by: Jagan Teki jagan@amarulasolutions.com > Reviewed-by: Bin Meng bmeng.cn@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 Teki I think MMC_SPI should be depend on or select SPL_CRC7_SUPPORT. SPL_CRC7_SUPPORT is not depend on MMC_SPI.
@Heinrich Schuchardt Compiling crc7.o every time is not a good idea. I have 1 suggestion, Can we make 1 Kconfig like
+config CRC7_SUPPORT
- bool "Support CRC7"
- help
Enable CRC7 hashing for drivers.This is a 32-bit checksum value that can be used to verify images.Which is common for U-Boot proper and U-Boot SPL and compile crc7
- obj-$(CONFIG_CRC7_SUPPORT) += crc7.o
Any suggestions are welcome ?
Any comment, I am planning to send v8.
I am going with Heinrich's suggestion and remove SPL Kconfig in v8.
Jagan.