[OpenWrt-Devel,4/8] mvebu: image: keep global DTS_DIR intact
diff mbox series

Message ID 20200131154620.25773-5-tomek_n@o2.pl
State Superseded
Headers show
Series
  • mvebu: Second round of clean-ups
Related show

Commit Message

Tomasz Maciej Nowak Jan. 31, 2020, 3:46 p.m. UTC
Don't rewrite global DTS_DIR, instead, use proper variable for
specifying devices dts directory.

Signed-off-by: Tomasz Maciej Nowak <tomek_n@o2.pl>
---
 target/linux/mvebu/image/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Adrian Schmutzler Jan. 31, 2020, 4:02 p.m. UTC | #1
Hi,

> -----Original Message-----
> From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org] On
> Behalf Of Tomasz Maciej Nowak
> Sent: Freitag, 31. Januar 2020 16:46
> To: openwrt-devel@lists.openwrt.org
> Subject: [OpenWrt-Devel] [PATCH 4/8] mvebu: image: keep global DTS_DIR intact
> 
> Don't rewrite global DTS_DIR, instead, use proper variable for
> specifying devices dts directory.

Have you build-tested this? DEVICE_DTS_DIR and DTS_DIR might behave differently
when it comes to includes in DTS files.
That's why I couldn't replace SUNXI_DTS_DIR with DEVICE_DTS_DIR when touching
this some months ago.

Best

Adrian

> 
> Signed-off-by: Tomasz Maciej Nowak <tomek_n@o2.pl>
> ---
>  target/linux/mvebu/image/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/linux/mvebu/image/Makefile
> b/target/linux/mvebu/image/Makefile
> index d9e4b1acce..ae4d3b9594 100644
> --- a/target/linux/mvebu/image/Makefile
> +++ b/target/linux/mvebu/image/Makefile
> @@ -91,7 +91,7 @@ endef
> 
>  define Device/Default-arm64
>    BOOT_SCRIPT := generic-arm64
> -  DTS_DIR := $(DTS_DIR)/marvell
> +  DEVICE_DTS_DIR := $(DTS_DIR)/marvell
>    IMAGES := sdcard.img.gz
>    IMAGE/sdcard.img.gz := boot-scr | boot-img-ext4 | sdcard-img-ext4 | gzip |
> append-metadata
>    KERNEL_NAME := Image
> --
> 2.25.0
> 
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Tomasz Maciej Nowak Jan. 31, 2020, 6:39 p.m. UTC | #2
W dniu 31.01.2020 o 17:02, Adrian Schmutzler pisze:
> Hi,
> 
>> -----Original Message-----
>> From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org] On
>> Behalf Of Tomasz Maciej Nowak
>> Sent: Freitag, 31. Januar 2020 16:46
>> To: openwrt-devel@lists.openwrt.org
>> Subject: [OpenWrt-Devel] [PATCH 4/8] mvebu: image: keep global DTS_DIR intact
>>
>> Don't rewrite global DTS_DIR, instead, use proper variable for
>> specifying devices dts directory.
> 
> Have you build-tested this?

Yes, images built fine before sending and from glance, they looked fine. What I
did not test was Image Builder. Will do that and send a feedback.

> DEVICE_DTS_DIR and DTS_DIR might behave differently
> when it comes to includes in DTS files.
> That's why I couldn't replace SUNXI_DTS_DIR with DEVICE_DTS_DIR when touching
> this some months ago.

The issue might stem from DEVICE_VARS defined inside the profile, which might
be exported too late? I'm not so good at Makefile syntax so maybe others can
comment on that.

> 
> Best
> 
> Adrian

Regards

> 
>>
>> Signed-off-by: Tomasz Maciej Nowak <tomek_n@o2.pl>
>> ---
>>  target/linux/mvebu/image/Makefile | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target/linux/mvebu/image/Makefile
>> b/target/linux/mvebu/image/Makefile
>> index d9e4b1acce..ae4d3b9594 100644
>> --- a/target/linux/mvebu/image/Makefile
>> +++ b/target/linux/mvebu/image/Makefile
>> @@ -91,7 +91,7 @@ endef
>>
>>  define Device/Default-arm64
>>    BOOT_SCRIPT := generic-arm64
>> -  DTS_DIR := $(DTS_DIR)/marvell
>> +  DEVICE_DTS_DIR := $(DTS_DIR)/marvell
>>    IMAGES := sdcard.img.gz
>>    IMAGE/sdcard.img.gz := boot-scr | boot-img-ext4 | sdcard-img-ext4 | gzip |
>> append-metadata
>>    KERNEL_NAME := Image
>> --
>> 2.25.0
>>
>>
>> _______________________________________________
>> openwrt-devel mailing list
>> openwrt-devel@lists.openwrt.org
>> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
>
Adrian Schmutzler Jan. 31, 2020, 6:47 p.m. UTC | #3
Hi,

> -----Original Message-----
> From: Tomasz Maciej Nowak [mailto:tomek_n@o2.pl]
> Sent: Freitag, 31. Januar 2020 19:39
> To: Adrian Schmutzler <mail@adrianschmutzler.de>; openwrt-
> devel@lists.openwrt.org
> Subject: Re: [OpenWrt-Devel] [PATCH 4/8] mvebu: image: keep global DTS_DIR
> intact
> 
> W dniu 31.01.2020 o 17:02, Adrian Schmutzler pisze:
> > Hi,
> >
> >> -----Original Message-----
> >> From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org] On
> >> Behalf Of Tomasz Maciej Nowak
> >> Sent: Freitag, 31. Januar 2020 16:46
> >> To: openwrt-devel@lists.openwrt.org
> >> Subject: [OpenWrt-Devel] [PATCH 4/8] mvebu: image: keep global DTS_DIR
> intact
> >>
> >> Don't rewrite global DTS_DIR, instead, use proper variable for
> >> specifying devices dts directory.
> >
> > Have you build-tested this?
> 
> Yes, images built fine before sending and from glance, they looked fine. What I
> did not test was Image Builder. Will do that and send a feedback.

On a second look, if you introduce DEVICE_DTS_DIR for Default-arm64, we should also introduce it to Device/Default:

DEVICE_DTS_DIR := $(DTS_DIR)

Otherwise, we would have part of the target relying on the device-specific variable and part on the global one, and I do not think that's desirable. With the change, the whole target will use DEVICE_DTS_DIR.

> 
> > DEVICE_DTS_DIR and DTS_DIR might behave differently
> > when it comes to includes in DTS files.
> > That's why I couldn't replace SUNXI_DTS_DIR with DEVICE_DTS_DIR when
> touching
> > this some months ago.
> 
> The issue might stem from DEVICE_VARS defined inside the profile, which might
> be exported too late? I'm not so good at Makefile syntax so maybe others can
> comment on that.

For sunxi, no. There the problem is that DEVICE_DTS_DIR is present in conditions in image.mk (or one of those files), and the device setup in sunxi itself assumes that subdirs are part of DEVICE_DTS. But this effectively might be a separate issue, as it is connected to the setup in sunxi.
It's not easy to fix that in a proper way.

Best

Adrian

> 
> >
> > Best
> >
> > Adrian
> 
> Regards
> 
> >
> >>
> >> Signed-off-by: Tomasz Maciej Nowak <tomek_n@o2.pl>
> >> ---
> >>  target/linux/mvebu/image/Makefile | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/target/linux/mvebu/image/Makefile
> >> b/target/linux/mvebu/image/Makefile
> >> index d9e4b1acce..ae4d3b9594 100644
> >> --- a/target/linux/mvebu/image/Makefile
> >> +++ b/target/linux/mvebu/image/Makefile
> >> @@ -91,7 +91,7 @@ endef
> >>
> >>  define Device/Default-arm64
> >>    BOOT_SCRIPT := generic-arm64
> >> -  DTS_DIR := $(DTS_DIR)/marvell
> >> +  DEVICE_DTS_DIR := $(DTS_DIR)/marvell
> >>    IMAGES := sdcard.img.gz
> >>    IMAGE/sdcard.img.gz := boot-scr | boot-img-ext4 | sdcard-img-ext4 | gzip |
> >> append-metadata
> >>    KERNEL_NAME := Image
> >> --
> >> 2.25.0
> >>
> >>
> >> _______________________________________________
> >> openwrt-devel mailing list
> >> openwrt-devel@lists.openwrt.org
> >> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
> >
> 
> 
> --
> TMN
Tomasz Maciej Nowak Jan. 31, 2020, 7:07 p.m. UTC | #4
W dniu 31.01.2020 o 19:47, Adrian Schmutzler pisze:
> Hi,
> 
>> -----Original Message-----
>> From: Tomasz Maciej Nowak [mailto:tomek_n@o2.pl]
>> Sent: Freitag, 31. Januar 2020 19:39
>> To: Adrian Schmutzler <mail@adrianschmutzler.de>; openwrt-
>> devel@lists.openwrt.org
>> Subject: Re: [OpenWrt-Devel] [PATCH 4/8] mvebu: image: keep global DTS_DIR
>> intact
>>
>> W dniu 31.01.2020 o 17:02, Adrian Schmutzler pisze:
>>> Hi,
>>>
>>>> -----Original Message-----
>>>> From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org] On
>>>> Behalf Of Tomasz Maciej Nowak
>>>> Sent: Freitag, 31. Januar 2020 16:46
>>>> To: openwrt-devel@lists.openwrt.org
>>>> Subject: [OpenWrt-Devel] [PATCH 4/8] mvebu: image: keep global DTS_DIR
>> intact
>>>>
>>>> Don't rewrite global DTS_DIR, instead, use proper variable for
>>>> specifying devices dts directory.
>>>
>>> Have you build-tested this?
>>
>> Yes, images built fine before sending and from glance, they looked fine. What I
>> did not test was Image Builder. Will do that and send a feedback.
> 
> On a second look, if you introduce DEVICE_DTS_DIR for Default-arm64, we should also introduce it to Device/Default:
> 
> DEVICE_DTS_DIR := $(DTS_DIR)
> 
> Otherwise, we would have part of the target relying on the device-specific variable and part on the global one, and I do not think that's desirable. With the change, the whole target will use DEVICE_DTS_DIR.

Ok, will send this as part of v2.

Regards

> 
>>
>>> DEVICE_DTS_DIR and DTS_DIR might behave differently
>>> when it comes to includes in DTS files.
>>> That's why I couldn't replace SUNXI_DTS_DIR with DEVICE_DTS_DIR when
>> touching
>>> this some months ago.
>>
>> The issue might stem from DEVICE_VARS defined inside the profile, which might
>> be exported too late? I'm not so good at Makefile syntax so maybe others can
>> comment on that.
> 
> For sunxi, no. There the problem is that DEVICE_DTS_DIR is present in conditions in image.mk (or one of those files), and the device setup in sunxi itself assumes that subdirs are part of DEVICE_DTS. But this effectively might be a separate issue, as it is connected to the setup in sunxi.
> It's not easy to fix that in a proper way.
> 
> Best
> 
> Adrian
> 
>>
>>>
>>> Best
>>>
>>> Adrian
>>
>> Regards
>>
>>>
>>>>
>>>> Signed-off-by: Tomasz Maciej Nowak <tomek_n@o2.pl>
>>>> ---
>>>>  target/linux/mvebu/image/Makefile | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/target/linux/mvebu/image/Makefile
>>>> b/target/linux/mvebu/image/Makefile
>>>> index d9e4b1acce..ae4d3b9594 100644
>>>> --- a/target/linux/mvebu/image/Makefile
>>>> +++ b/target/linux/mvebu/image/Makefile
>>>> @@ -91,7 +91,7 @@ endef
>>>>
>>>>  define Device/Default-arm64
>>>>    BOOT_SCRIPT := generic-arm64
>>>> -  DTS_DIR := $(DTS_DIR)/marvell
>>>> +  DEVICE_DTS_DIR := $(DTS_DIR)/marvell
>>>>    IMAGES := sdcard.img.gz
>>>>    IMAGE/sdcard.img.gz := boot-scr | boot-img-ext4 | sdcard-img-ext4 | gzip |
>>>> append-metadata
>>>>    KERNEL_NAME := Image
>>>> --
>>>> 2.25.0
>>>>
>>>>
>>>> _______________________________________________
>>>> openwrt-devel mailing list
>>>> openwrt-devel@lists.openwrt.org
>>>> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
>>>
>>
>>
>> --
>> TMN
>
Adrian Schmutzler Jan. 31, 2020, 7:12 p.m. UTC | #5
> Ok, will send this as part of v2.

I will mark 3/8 as rejected.

I'd merge 1, 2 and 4 tomorrow or on Sunday, so if you want to wait for additional input on 5-8 you can also only send the former as smaller v2 patchset.

Best

Adrian

> 
> Regards
> 
> >
> >>
> >>> DEVICE_DTS_DIR and DTS_DIR might behave differently
> >>> when it comes to includes in DTS files.
> >>> That's why I couldn't replace SUNXI_DTS_DIR with DEVICE_DTS_DIR when
> >> touching
> >>> this some months ago.
> >>
> >> The issue might stem from DEVICE_VARS defined inside the profile, which
> might
> >> be exported too late? I'm not so good at Makefile syntax so maybe others can
> >> comment on that.
> >
> > For sunxi, no. There the problem is that DEVICE_DTS_DIR is present in
> conditions in image.mk (or one of those files), and the device setup in sunxi itself
> assumes that subdirs are part of DEVICE_DTS. But this effectively might be a
> separate issue, as it is connected to the setup in sunxi.
> > It's not easy to fix that in a proper way.
> >
> > Best
> >
> > Adrian
> >
> >>
> >>>
> >>> Best
> >>>
> >>> Adrian
> >>
> >> Regards
> >>
> >>>
> >>>>
> >>>> Signed-off-by: Tomasz Maciej Nowak <tomek_n@o2.pl>
> >>>> ---
> >>>>  target/linux/mvebu/image/Makefile | 2 +-
> >>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/target/linux/mvebu/image/Makefile
> >>>> b/target/linux/mvebu/image/Makefile
> >>>> index d9e4b1acce..ae4d3b9594 100644
> >>>> --- a/target/linux/mvebu/image/Makefile
> >>>> +++ b/target/linux/mvebu/image/Makefile
> >>>> @@ -91,7 +91,7 @@ endef
> >>>>
> >>>>  define Device/Default-arm64
> >>>>    BOOT_SCRIPT := generic-arm64
> >>>> -  DTS_DIR := $(DTS_DIR)/marvell
> >>>> +  DEVICE_DTS_DIR := $(DTS_DIR)/marvell
> >>>>    IMAGES := sdcard.img.gz
> >>>>    IMAGE/sdcard.img.gz := boot-scr | boot-img-ext4 | sdcard-img-ext4 | gzip
> |
> >>>> append-metadata
> >>>>    KERNEL_NAME := Image
> >>>> --
> >>>> 2.25.0
> >>>>
> >>>>
> >>>> _______________________________________________
> >>>> openwrt-devel mailing list
> >>>> openwrt-devel@lists.openwrt.org
> >>>> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
> >>>
> >>
> >>
> >> --
> >> TMN
> >
> 
> 
> --
> TMN

Patch
diff mbox series

diff --git a/target/linux/mvebu/image/Makefile b/target/linux/mvebu/image/Makefile
index d9e4b1acce..ae4d3b9594 100644
--- a/target/linux/mvebu/image/Makefile
+++ b/target/linux/mvebu/image/Makefile
@@ -91,7 +91,7 @@  endef
 
 define Device/Default-arm64
   BOOT_SCRIPT := generic-arm64
-  DTS_DIR := $(DTS_DIR)/marvell
+  DEVICE_DTS_DIR := $(DTS_DIR)/marvell
   IMAGES := sdcard.img.gz
   IMAGE/sdcard.img.gz := boot-scr | boot-img-ext4 | sdcard-img-ext4 | gzip | append-metadata
   KERNEL_NAME := Image