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

Fri May 8 07:59:11 CEST 2020

>-----Original Message-----
>From: U-Boot <u-boot-bounces at lists.denx.de> On Behalf Of Pragnesh Patel
>Sent: 05 May 2020 21:25
>To: Jagan Teki <jagan at amarulasolutions.com>; Heinrich Schuchardt
><xypron.glpk at gmx.de>; Bin Meng <bmeng.cn at gmail.com>
>Cc: U-Boot Mailing List <u-boot at lists.denx.de>; Atish Patra
><atish.patra at wdc.com>; Palmer Dabbelt <palmerdabbelt at google.com>; Paul
>Walmsley <paul.walmsley at sifive.com>; Troy Benjegerdes
><troy.benjegerdes at sifive.com>; Anup Patel <anup.patel at wdc.com>; Sagar
>Kadam <sagar.kadam at sifive.com>; Rick Chen <rick at andestech.com>; Peng
>Fan <peng.fan at nxp.com>; Lukasz Majewski <lukma at denx.de>; Simon
>Goldschmidt <simon.k.r.goldschmidt at gmail.com>; Simon Glass
><sjg at chromium.org>; Markus Klotzbuecher
><markus.klotzbuecher at kistler.com>; Baruch Siach <baruch at tkos.co.il>; Joel
>Johnson <mrjoel at lixil.net>; Anatolij Gustschin <agust at denx.de>; AKASHI
>Takahiro <takahiro.akashi at linaro.org>; Marek Behún <marek.behun at 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 at lists.denx.de> On Behalf Of Pragnesh Patel
>>Sent: 04 May 2020 11:15
>>To: Jagan Teki <jagan at amarulasolutions.com>; Heinrich Schuchardt
>><xypron.glpk at gmx.de>; Bin Meng <bmeng.cn at gmail.com>
>>Cc: U-Boot Mailing List <u-boot at lists.denx.de>; Atish Patra
>><atish.patra at wdc.com>; Palmer Dabbelt <palmerdabbelt at google.com>;
>Paul
>>Walmsley <paul.walmsley at sifive.com>; Troy Benjegerdes
>><troy.benjegerdes at sifive.com>; Anup Patel <anup.patel at wdc.com>; Sagar
>>Kadam <sagar.kadam at sifive.com>; Rick Chen <rick at andestech.com>; Peng
>>Fan <peng.fan at nxp.com>; Lukasz Majewski <lukma at denx.de>; Simon
>>Goldschmidt <simon.k.r.goldschmidt at gmail.com>; Simon Glass
>><sjg at chromium.org>; Markus Klotzbuecher
>><markus.klotzbuecher at kistler.com>; Baruch Siach <baruch at tkos.co.il>;
>>Joel Johnson <mrjoel at lixil.net>; Anatolij Gustschin <agust at denx.de>;
>>AKASHI Takahiro <takahiro.akashi at linaro.org>; Marek Behún
>><marek.behun at nic.cz>
>>Subject: RE: [PATCH v7 04/22] lib: Makefile: build crc7.c when
>>CONFIG_MMC_SPI
>>
>>>-----Original Message-----
>>>From: Jagan Teki <jagan at amarulasolutions.com>
>>>Sent: 02 May 2020 21:04
>>>To: Heinrich Schuchardt <xypron.glpk at gmx.de>; Pragnesh Patel
>>><pragnesh.patel at sifive.com>; Bin Meng <bmeng.cn at gmail.com>
>>>Cc: U-Boot Mailing List <u-boot at lists.denx.de>; Atish Patra
>>><atish.patra at wdc.com>; Palmer Dabbelt <palmerdabbelt at google.com>;
>>Paul
>>>Walmsley <paul.walmsley at sifive.com>; Troy Benjegerdes
>>><troy.benjegerdes at sifive.com>; Anup Patel <anup.patel at wdc.com>; Sagar
>>>Kadam <sagar.kadam at sifive.com>; Rick Chen <rick at andestech.com>; Peng
>>>Fan <peng.fan at nxp.com>; Lukasz Majewski <lukma at denx.de>; Simon
>>>Goldschmidt <simon.k.r.goldschmidt at gmail.com>; Simon Glass
>>><sjg at chromium.org>; Markus Klotzbuecher
>>><markus.klotzbuecher at kistler.com>; Baruch Siach <baruch at tkos.co.il>;
>>>Joel Johnson <mrjoel at lixil.net>; Anatolij Gustschin <agust at denx.de>;
>>>AKASHI Takahiro <takahiro.akashi at linaro.org>; Marek Behún
>>><marek.behun at 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 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 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.