[BUG] Fix drivers/memory O= build

Tue Apr 10 08:01:11 PDT 2018

2018-04-10 1:05 GMT+09:00 Dave Gerlach <d-gerlach at ti.com>:
> Hi,
> On 04/07/2018 08:38 AM, Russell King - ARM Linux wrote:
>> On Sat, Apr 07, 2018 at 11:55:22AM +0900, Masahiro Yamada wrote:
>>> 2018-04-07 3:55 GMT+09:00 Russell King - ARM Linux <linux at armlinux.org.uk>:
>>>> On Fri, Apr 06, 2018 at 11:45:01AM -0700, Santosh Shilimkar wrote:
>>>>> On 4/6/2018 9:46 AM, Tony Lindgren wrote:
>>>>>> * Russell King - ARM Linux <linux at armlinux.org.uk> [180406 03:27]:
>>>>>>> When building a kernel with split object directories, the following
>>>>>>> failure occurs:
>>>>>>>
>>>>>>> .../drivers/memory/emif-asm-offsets.c:1:0: fatal error: can't open drivers/memory/emif-asm-offsets.s for writing: No such file or directory
>>>>>>> compilation terminated.
>>>>>>> drivers/memory/Makefile.asm-offsets:2: recipe for target 'drivers/memory/emif-asm-offsets.s' failed
>>>>>>> make[2]: *** [drivers/memory/emif-asm-offsets.s] Error 1
>>>>>>> .../Makefile:1060: recipe for target 'arch/arm/mach-omap2' failed
>>>>>>> make[1]: *** [arch/arm/mach-omap2] Error 2
>>>>>>>
>>>>>>> This prevents any kernel being built that includes decending into
>>>>>>> arch/arm/mach-omap2 with split object directories.
>>>>>>>
>>>>>>> This is a regression.
>>>>>>
>>>>>> Dave, I presume you're looking into this already?
>>>>>>
>>>>> Fix is already posted by 'Anders Roxell <anders.roxell at linaro.org>'
>>>>> You are copied on the email Tony.
>>>>
>>>> Arnd also has a fix for it as well, which moves emif-asm-offsets.s
>>>> into include/generated, as that will always exist prior to these
>>>> rules being invoked.  Arnd's passing it through his randconfig
>>>> builder before publishing it.
>>>
>>>
>>> I do not like this idea.
>>>
>>> Not sure exactly what Arnd is trying to do,
>>> but the same file should not be touched from multiple points
>>> in case Kbuild descends into mach-omap2/ and memory/ at the same time.
>>>
>>>
>>> I recommend to put copies in each directory:
>>>
>>> arch/arm/mach-omap2/emif-asm-offsets.h
>>> drivers/memory/ti-emif-asm-offsets.h
>>>
>>>
>>> If you want me to send a patch,
>>> I will give it a try.
>>
>> Or we try and find another way around this.
>>
>> The original idea was to start throwing platform specific (iow, stuff in
>> arch/arm/mach-omap2/) includes into arch/arm/kernel/asm-offsets.c.  I
>> objected to that, and suggested that the generic infrastructure for doing
>> the asm-offsets thing should be used to generate an architecture specific
>> header.
>>
>> Then it turns out that files in both arch/arm/mach-omap2 and drivers/memory
>> need these constants, and TI proposed the existing method.
>>
>> Obviously, all the right people reviewed it and approved it, which is why
>> it's now in mainline, but unfortunately it doesn't actually work, and
>> breaks my kernel builds!
>>
>
> A solution that I see is to allow pm-asm-offsets to just include the macros that
> would be defined by emif-asm-offsets in the header generated by pm-asm-offsets.
> This way there is no need to introduce the arch -> drivers build dependency that
> is causing our issue, each driver generates its own header with everything it
> needs included. I'll move the actual macro definitions for emif-asm-offsets.c
> into a header and then by including them in both emif-asm-offsets and
> pm-asm-offsets we get all macro definitions in both places with them still being
> defined only in one location.
>
> I've inlined the changes below. I will send a proper split up series if it's
> considered acceptable.
>
> Regards,
> Dave
>
> 8< ----------------------
> From 7350c7caa773d7235ddb21013bd039a16cb6d156 Mon Sep 17 00:00:00 2001
> From: Dave Gerlach <d-gerlach at ti.com>
> Date: Mon, 9 Apr 2018 10:56:59 -0500
> Subject: [PATCH] RFC: Fix build when using split object directories
>
> The sleep33xx and sleep43xx files should not depend on a header file
> generated in drivers/memory. Remove this dependency and instead generate
> the macros needed as part of pm-asm-offsets.c to avoid complicating the
> build.
>
> Signed-off-by: Dave Gerlach <d-gerlach at ti.com>
> ---
>  arch/arm/mach-omap2/Makefile         |  7 +---
>  arch/arm/mach-omap2/pm-asm-offsets.c |  3 ++
>  arch/arm/mach-omap2/sleep33xx.S      |  1 -
>  arch/arm/mach-omap2/sleep43xx.S      |  1 -
>  drivers/memory/emif-asm-offsets.c    | 72 +---------------------------------
>  include/linux/ti-emif-sram.h         | 75 ++++++++++++++++++++++++++++++++++++
>  6 files changed, 81 insertions(+), 78 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
> index 4603c30fef73..1257810cdb80 100644
> --- a/arch/arm/mach-omap2/Makefile
> +++ b/arch/arm/mach-omap2/Makefile
> @@ -243,8 +243,5 @@ arch/arm/mach-omap2/pm-asm-offsets.s:
> arch/arm/mach-omap2/pm-asm-offsets.c
>  include/generated/ti-pm-asm-offsets.h: arch/arm/mach-omap2/pm-asm-offsets.s FORCE
>         $(call filechk,offsets,__TI_PM_ASM_OFFSETS_H__)
>
> -# For rule to generate ti-emif-asm-offsets.h dependency
> -include drivers/memory/Makefile.asm-offsets
> -
> -arch/arm/mach-omap2/sleep33xx.o: include/generated/ti-pm-asm-offsets.h
> include/generated/ti-emif-asm-offsets.h
> -arch/arm/mach-omap2/sleep43xx.o: include/generated/ti-pm-asm-offsets.h
> include/generated/ti-emif-asm-offsets.h
> +arch/arm/mach-omap2/sleep33xx.o: include/generated/ti-pm-asm-offsets.h
> +arch/arm/mach-omap2/sleep43xx.o: include/generated/ti-pm-asm-offsets.h

Nit:
You are touching these two lines.
You can also describe it like this:

$(obj)/sleep33xx.o $(obj)/sleep43xx.o: include/generated/ti-pm-asm-offsets.h

I prefer putting the local header, ti-pm-asm-offsets.h,
into arch/arm/mach-omap2/, but that is a different issue.

So, I am fine with this.

Reviewed-by: Masahiro Yamada <yamada.masahiro at socionext.com>

-- 
Best Regards
Masahiro Yamada