Re: [u-boot][PATCH v2 2/4] scripts: Makefile.spl: Enable memory drivers to be built for SPL

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 2529 bytes --]

On Tue, Oct 25, 2022 at 05:35:29PM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Thu, 20 Oct 2022 at 06:29, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Oct 20, 2022 at 03:23:42PM +0300, Roger Quadros wrote:
> > > Hi Tom,
> > >
> > > On 19/10/2022 15:54, Tom Rini wrote:
> > > > On Wed, Oct 19, 2022 at 11:17:35AM +0300, Roger Quadros wrote:
> > > >>
> > > >>
> > > >> On 18/10/2022 20:40, Tom Rini wrote:
> > > >>> On Thu, Oct 06, 2022 at 04:23:58PM +0300, Roger Quadros wrote:
> > > >>>> We will need ti-gpmc driver for SPL. Allow memory drivers
> > > >>>> do be built for SPL.
> > > >>>>
> > > >>>> Signed-off-by: Roger Quadros <rogerq@kernel.org>
> > > >>>> ---
> > > >>>>  scripts/Makefile.spl | 1 +
> > > >>>>  1 file changed, 1 insertion(+)
> > > >>>>
> > > >>>> diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
> > > >>>> index 3bafeb4fe9..110076b22f 100644
> > > >>>> --- a/scripts/Makefile.spl
> > > >>>> +++ b/scripts/Makefile.spl
> > > >>>> @@ -114,6 +114,7 @@ libs-$(CONFIG_PARTITIONS) += disk/
> > > >>>>  endif
> > > >>>>
> > > >>>>  libs-y += drivers/
> > > >>>> +libs-y += drivers/memory/
> > > >>>>  libs-$(CONFIG_SPL_USB_GADGET) += drivers/usb/dwc3/
> > > >>>>  libs-$(CONFIG_SPL_USB_GADGET) += drivers/usb/cdns3/
> > > >>>>  libs-y += dts/
> > > >>>
> > > >>> This ends up being the wrong approach as it then pulls in
> > > >>> drivers/memory/stm32-fmc2-ebi.o on all of those platforms, in SPL, which
> > > >>> is not what's intended. We need an SPL_MEMORY symbol and then gate the
> > > >>> directory on that.
> > > >>>
> > >
> > > I have a question about how CONFIG_SPL_MEMORY works together with CONFIG_MEMORY.
> > >
> > > Do we use CONFIG_SPL_MEMORY only to gate the drivers/memory directory inclusion?
> > > Then continue to use CONFIG_MEMORY and others to enable/disable driver
> > > build for both non-SPL and SPL case?
> > >
> > > So drivers/memory/Makefile remains as it is?
> >
> > Well, for consistency code should use IS_ENABLED(MEMORY) which will be
> > true for CONFIG_MEMORY or CONFIG_SPL_MEMORY.
> 
> nit: CONFIG_IS_ENABLED(MEMORY)
> 
> Perhaps we can look at my Kconfig series so it can become
> CONFIG(MEMORY) and we can drop all the SPL_TPL_ stuff in Makefiles?

I think that was the part I had some reservations about, but we should
re-visit that idea as I wasn't disagreeing (I believe) with making the
logic easier, but I think that we need to re-evaluate how we manage the
SPL/TPL/full U-Boot idea.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]