| Message ID | 20200502100628.24809-5-pragnesh.patel@sifive.com |
|---|---|
| State | New |
| Headers | show |
| Series | RISC-V SiFive FU540 support SPL | expand |
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 and move that line inside lib/Makefile so that is does not depend on CONFIG_SPL_BUILD. Best regards Heinrich > obj-$(CONFIG_$(SPL_TPL_)HASH_SUPPORT) += crc16.o > obj-y += net_utils.o > endif >
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. > > and move that line inside lib/Makefile so that is does not depend on > CONFIG_SPL_BUILD. > Regards, Bin
On Sat, May 2, 2020 at 6:07 PM Pragnesh Patel <pragnesh.patel at sifive.com> 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(+) > Tested-by: Bin Meng <bmeng.cn at gmail.com>
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. Best regards Heinrich > >> >> and move that line inside lib/Makefile so that is does not depend on >> CONFIG_SPL_BUILD. >> > >Regards, >Bin
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.
Hi Heinrich, >-----Original Message----- >From: Heinrich Schuchardt <xypron.glpk at gmx.de> >Sent: 02 May 2020 18:29 >To: Bin Meng <bmeng.cn at gmail.com> >Cc: Pragnesh Patel <pragnesh.patel at sifive.com>; U-Boot Mailing List <u- >boot at lists.denx.de>; Jagan Teki <jagan at amarulasolutions.com>; 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 > >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. I think this is already discussed in v3. https://patchwork.ozlabs.org/project/uboot/patch/20200124055026.30787-4-pragnesh.patel at sifive.com/ > >Best regards > >Heinrich > >> >>> >>> and move that line inside lib/Makefile so that is does not depend on >>> CONFIG_SPL_BUILD. >>> >> >>Regards, >>Bin
>-----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 ? > >Jagan.
>-----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. > >> >>Jagan.
>-----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.
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 obj-$(CONFIG_$(SPL_TPL_)HASH_SUPPORT) += crc16.o obj-y += net_utils.o endif