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

Tom Rini
2022-10-26T13:52:37Z

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

On Wed, Oct 26, 2022 at 01:41:12PM +0300, Roger Quadros wrote:
> Tom,
>
> On 26/10/2022 02:35, 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)
>
> CONFIG_IS_ENABLED* is used in code but not by any Makefile.
> My question was on what basis do we gate drivers/memory inclusion in build from Makefile.
>
> Can CONFIG_MEMORY and CONFIG_SPL_MEMORY co-exist or that should never happen?

What we have for now in your patch is fine and we can tweak slightly if
needed if other cases arise.

--
Tom

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