>-----Original Message----- >From: U-Boot <u-boot-boun...@lists.denx.de> On Behalf Of Pragnesh Patel >Sent: 05 May 2020 21:25 >To: Jagan Teki <ja...@amarulasolutions.com>; Heinrich Schuchardt ><xypron.g...@gmx.de>; Bin Meng <bmeng...@gmail.com> >Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Atish Patra ><atish.pa...@wdc.com>; Palmer Dabbelt <palmerdabb...@google.com>; Paul >Walmsley <paul.walms...@sifive.com>; Troy Benjegerdes ><troy.benjeger...@sifive.com>; Anup Patel <anup.pa...@wdc.com>; Sagar >Kadam <sagar.ka...@sifive.com>; Rick Chen <r...@andestech.com>; Peng >Fan <peng....@nxp.com>; Lukasz Majewski <lu...@denx.de>; Simon >Goldschmidt <simon.k.r.goldschm...@gmail.com>; Simon Glass ><s...@chromium.org>; Markus Klotzbuecher ><markus.klotzbuec...@kistler.com>; Baruch Siach <bar...@tkos.co.il>; Joel >Johnson <mrj...@lixil.net>; Anatolij Gustschin <ag...@denx.de>; AKASHI >Takahiro <takahiro.aka...@linaro.org>; Marek Behún <marek.be...@nic.cz> >Subject: RE: [PATCH v7 04/22] lib: Makefile: build crc7.c when >CONFIG_MMC_SPI > > > >>-----Original Message----- >>From: U-Boot <u-boot-boun...@lists.denx.de> On Behalf Of Pragnesh Patel >>Sent: 04 May 2020 11:15 >>To: Jagan Teki <ja...@amarulasolutions.com>; Heinrich Schuchardt >><xypron.g...@gmx.de>; Bin Meng <bmeng...@gmail.com> >>Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Atish Patra >><atish.pa...@wdc.com>; Palmer Dabbelt <palmerdabb...@google.com>; >Paul >>Walmsley <paul.walms...@sifive.com>; Troy Benjegerdes >><troy.benjeger...@sifive.com>; Anup Patel <anup.pa...@wdc.com>; Sagar >>Kadam <sagar.ka...@sifive.com>; Rick Chen <r...@andestech.com>; Peng >>Fan <peng....@nxp.com>; Lukasz Majewski <lu...@denx.de>; Simon >>Goldschmidt <simon.k.r.goldschm...@gmail.com>; Simon Glass >><s...@chromium.org>; Markus Klotzbuecher >><markus.klotzbuec...@kistler.com>; Baruch Siach <bar...@tkos.co.il>; >>Joel Johnson <mrj...@lixil.net>; Anatolij Gustschin <ag...@denx.de>; >>AKASHI Takahiro <takahiro.aka...@linaro.org>; Marek Behún >><marek.be...@nic.cz> >>Subject: RE: [PATCH v7 04/22] lib: Makefile: build crc7.c when >>CONFIG_MMC_SPI >> >>>-----Original Message----- >>>From: Jagan Teki <ja...@amarulasolutions.com> >>>Sent: 02 May 2020 21:04 >>>To: Heinrich Schuchardt <xypron.g...@gmx.de>; Pragnesh Patel >>><pragnesh.pa...@sifive.com>; Bin Meng <bmeng...@gmail.com> >>>Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Atish Patra >>><atish.pa...@wdc.com>; Palmer Dabbelt <palmerdabb...@google.com>; >>Paul >>>Walmsley <paul.walms...@sifive.com>; Troy Benjegerdes >>><troy.benjeger...@sifive.com>; Anup Patel <anup.pa...@wdc.com>; Sagar >>>Kadam <sagar.ka...@sifive.com>; Rick Chen <r...@andestech.com>; Peng >>>Fan <peng....@nxp.com>; Lukasz Majewski <lu...@denx.de>; Simon >>>Goldschmidt <simon.k.r.goldschm...@gmail.com>; Simon Glass >>><s...@chromium.org>; Markus Klotzbuecher >>><markus.klotzbuec...@kistler.com>; Baruch Siach <bar...@tkos.co.il>; >>>Joel Johnson <mrj...@lixil.net>; Anatolij Gustschin <ag...@denx.de>; >>>AKASHI Takahiro <takahiro.aka...@linaro.org>; Marek Behún >>><marek.be...@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.g...@gmx.de> >>>wrote: >>>> >>>> Am May 2, 2020 11:47:10 AM UTC schrieb Bin Meng >>><bmeng...@gmail.com>: >>>> >Hi Heinrich, >>>> > >>>> >On Sat, May 2, 2020 at 6:30 PM Heinrich Schuchardt >>>> ><xypron.g...@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.pa...@sifive.com> >>>> >> > Reviewed-by: Jagan Teki <ja...@amarulasolutions.com> >>>> >> > Reviewed-by: Bin Meng <bmeng...@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.