[U-Boot] [PATCH] spl: Allow cache drivers to be used in SPL

[Ley Foon Tan]

On Thu, Nov 28, 2019 at 7:38 PM Simon Goldschmidt
<simon.k.r.goldschmidt at gmail.com> wrote:
>
> On Thu, Nov 28, 2019 at 1:59 AM Ley Foon Tan <lftan.linux at gmail.com> wrote:
> >
> > On Thu, Nov 28, 2019 at 4:33 AM Simon Goldschmidt
> > <simon.k.r.goldschmidt at gmail.com> wrote:
> > >
> > > Ley, Tom,
> > >
> > > Am 26.11.2019 um 10:26 schrieb Ley Foon Tan:
> > > > On Fri, Nov 8, 2019 at 10:53 AM Ley Foon Tan <ley.foon.tan at intel.com> wrote:
> > > >>
> > > >> Add an option for building cache drivers in SPL.
> > >
> > > Ley:
> > >
> > > What's the actual problem here? Can you further describe your change?
> > > Why do you need to change drivers/cache/Makefile? That seems to only
> > > make sense if you enable the new SPL_CACHE without (non-SPL) CACHE.
> > >
> > > However, the series this was pulled out from adds a new cache driver and
> > > makes it select CACHE, not SPL_CACHE, so how would this be activated anyway?
> > >
> > > Maybe it would be better to always dive down into drivers/cache/ if
> > > CACHE is y?
> > Hi Simon
> >
> > The existing drivers/cache is only build when compile for Uboot
> > proper, but not SPL build.
> > So, this patch mainly is to allow drivers/cache to compile in SPL build.
> > User can enable CONFIG_SPL_CACHE if they need to include drivers/cache
> > in SPL, eg: Agilex platform.
>
> But you might not have CACHE enabled since you can enable SPL_CACHE
> without CACHE, see my suggestions below.
>
> >
> > Regards
> > Ley Foon
> >
> > >
> > > Tom:
> > >
> > > As drivers/cache is rather new and initiated via the socfpga tree, would
> > > you be OK for us to take this via the socfpga/next tree once it's sorted
> > > out?
> > >
> > > Regards,
> > > Simon
> > >
> > > >>
> > > >> Signed-off-by: Ley Foon Tan <ley.foon.tan at intel.com>
> > > >> ---
> > > >>   common/spl/Kconfig     | 5 +++++
> > > >>   drivers/Makefile       | 1 +
> > > >>   drivers/cache/Makefile | 2 +-
> > > >>   3 files changed, 7 insertions(+), 1 deletion(-)
> > > >>
> > > >> diff --git a/common/spl/Kconfig b/common/spl/Kconfig
> > > >> index c661809923..6e095c33e1 100644
> > > >> --- a/common/spl/Kconfig
> > > >> +++ b/common/spl/Kconfig
> > > >> @@ -714,6 +714,11 @@ config SPL_UBI
> > > >>            README.ubispl for more info.
> > > >>
> > > >>   if SPL_DM
> > > >> +config SPL_CACHE
> > > >> +       bool "Support cache drivers in SPL"
>
> I think this needs to depend on CACHE.
Okay.
>
> > > >> +       help
> > > >> +         Enable support for cache drivers in SPL.
> > > >> +
> > > >>   config SPL_DM_SPI
> > > >>          bool "Support SPI DM drivers in SPL"
> > > >>          help
> > > >> diff --git a/drivers/Makefile b/drivers/Makefile
> > > >> index 0befeddfcb..0e42d006b9 100644
> > > >> --- a/drivers/Makefile
> > > >> +++ b/drivers/Makefile
> > > >> @@ -31,6 +31,7 @@ ifndef CONFIG_TPL_BUILD
> > > >>   ifdef CONFIG_SPL_BUILD
> > > >>
> > > >>   obj-$(CONFIG_SPL_BOOTCOUNT_LIMIT) += bootcount/
> > > >> +obj-$(CONFIG_SPL_CACHE) += cache/
>
> Can you move this up to the common part and make it:
Noted.

>
> obj-$(CONFIG_$(SPL_TPL_)CACHE) += cache/
>
> > > >>   obj-$(CONFIG_SPL_CPU_SUPPORT) += cpu/
> > > >>   obj-$(CONFIG_SPL_CRYPTO_SUPPORT) += crypto/
> > > >>   obj-$(CONFIG_SPL_MPC8XXX_INIT_DDR_SUPPORT) += ddr/fsl/
> > > >> diff --git a/drivers/cache/Makefile b/drivers/cache/Makefile
> > > >> index 4a6458c602..c1f766cfca 100644
> > > >> --- a/drivers/cache/Makefile
> > > >> +++ b/drivers/cache/Makefile
> > > >> @@ -1,5 +1,5 @@
> > > >>
> > > >> -obj-$(CONFIG_CACHE) += cache-uclass.o
> > > >> +obj-$(CONFIG_$(SPL_)CACHE) += cache-uclass.o
>
> This should then probably be $(SPL_TPL), too?
>
> Although we don't have CONFIG_TPL_CACHE, yet, it might be
> better like that to prevent including cache drivers in TPL?
Okay.

Regards
Ley Foon
>
> > > >>   obj-$(CONFIG_SANDBOX) += sandbox_cache.o
> > > >>   obj-$(CONFIG_L2X0_CACHE) += cache-l2x0.o
> > > >>   obj-$(CONFIG_V5L2_CACHE) += cache-v5l2.o
> > > >> --
> > > >> 2.19.0
> > > > Hi Tom
> > > >
> > > > Any comment on this patch?
> > > >
> > > > Regards
> > > >
> > > > Ley Foon
> > > >
> > >