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

[Roger Quadros <rogerq@kernel.org>]

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?

> 
> 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?
> 
> Regards,
> SImon

cheers,
-roger