| Message ID | ff5a9ea519a5c72dd917369e6409674b9d4a340f.1436702899.git.yann.morin.1998@free.fr |
|---|---|
| State | Changes Requested |
| Headers | show |
Hi Yann, On Sun, Jul 12, 2015 at 02:11:24PM +0200, Yann E. MORIN wrote: > Currently, this is triggering the error message: > make randconfig > make source > > Limit the checks that enforce a DTS is set and at most one DTB is > appended to when we are actually building, like is done for the > configuration-file variables. > > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> [snip] > -ifeq ($(BR2_LINUX_KERNEL_DTS_SUPPORT)$(KERNEL_DTS_NAME),y) > +ifeq ($(BR_BUILDING)$(BR2_LINUX_KERNEL_DTS_SUPPORT)$(KERNEL_DTS_NAME),yy) yy means that exactly two of these must be 'y'. Is that intended? baruch
On 07/12/15 22:03, Baruch Siach wrote: > Hi Yann, > > On Sun, Jul 12, 2015 at 02:11:24PM +0200, Yann E. MORIN wrote: >> Currently, this is triggering the error message: >> make randconfig >> make source >> >> Limit the checks that enforce a DTS is set and at most one DTB is >> appended to when we are actually building, like is done for the >> configuration-file variables. >> >> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > > [snip] > >> -ifeq ($(BR2_LINUX_KERNEL_DTS_SUPPORT)$(KERNEL_DTS_NAME),y) >> +ifeq ($(BR_BUILDING)$(BR2_LINUX_KERNEL_DTS_SUPPORT)$(KERNEL_DTS_NAME),yy) > > yy means that exactly two of these must be 'y'. Is that intended? Yes it is, because KERNEL_DTS_NAME is a string, not a boolean, so it essentially checks that it is empty and DTS_SUPPORT is selected. But admittedly it's not very transparent, it would make sense to add delimiters in-between at least. Regards, Arnout
Hi Arnout, On Mon, Jul 13, 2015 at 01:44:31AM +0200, Arnout Vandecappelle wrote: > On 07/12/15 22:03, Baruch Siach wrote: > > On Sun, Jul 12, 2015 at 02:11:24PM +0200, Yann E. MORIN wrote: > >> Currently, this is triggering the error message: > >> make randconfig > >> make source > >> > >> Limit the checks that enforce a DTS is set and at most one DTB is > >> appended to when we are actually building, like is done for the > >> configuration-file variables. > >> > >> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > > > > [snip] > > > >> -ifeq ($(BR2_LINUX_KERNEL_DTS_SUPPORT)$(KERNEL_DTS_NAME),y) > >> +ifeq ($(BR_BUILDING)$(BR2_LINUX_KERNEL_DTS_SUPPORT)$(KERNEL_DTS_NAME),yy) > > > > yy means that exactly two of these must be 'y'. Is that intended? > > Yes it is, because KERNEL_DTS_NAME is a string, not a boolean, so it > essentially checks that it is empty and DTS_SUPPORT is selected. But admittedly > it's not very transparent, it would make sense to add delimiters in-between at > least. Thanks for the explanation. A comment would be helpful here, I guess. Shouldn't we protect the code against someone naming a DTS just 'y'? baruch
Baruch, All, On 2015-07-13 06:37 +0300, Baruch Siach spake thusly: > On Mon, Jul 13, 2015 at 01:44:31AM +0200, Arnout Vandecappelle wrote: > > On 07/12/15 22:03, Baruch Siach wrote: > > > On Sun, Jul 12, 2015 at 02:11:24PM +0200, Yann E. MORIN wrote: > > >> Currently, this is triggering the error message: > > >> make randconfig > > >> make source > > >> > > >> Limit the checks that enforce a DTS is set and at most one DTB is > > >> appended to when we are actually building, like is done for the > > >> configuration-file variables. > > >> > > >> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > > > > > > [snip] > > > > > >> -ifeq ($(BR2_LINUX_KERNEL_DTS_SUPPORT)$(KERNEL_DTS_NAME),y) > > >> +ifeq ($(BR_BUILDING)$(BR2_LINUX_KERNEL_DTS_SUPPORT)$(KERNEL_DTS_NAME),yy) > > > > > > yy means that exactly two of these must be 'y'. Is that intended? > > > > Yes it is, because KERNEL_DTS_NAME is a string, not a boolean, so it > > essentially checks that it is empty and DTS_SUPPORT is selected. But admittedly > > it's not very transparent, it would make sense to add delimiters in-between at > > least. > > Thanks for the explanation. A comment would be helpful here, I guess. > > Shouldn't we protect the code against someone naming a DTS just 'y'? Yeah, we already discussed that with Thomas (live, we're in the same place for a few days), and we've come to the conclusion that this construct is indeed too tricky, and that we need a simpler code. Your comments are now confirming this. ;-) I'll rework the series shortly. Thanks! :-) Regards, Yann E. MORIN.
diff --git a/linux/linux.mk b/linux/linux.mk index eca1450..b2c9481 100644 --- a/linux/linux.mk +++ b/linux/linux.mk @@ -88,12 +88,12 @@ else ifeq ($(BR2_LINUX_KERNEL_USE_CUSTOM_DTS),y) KERNEL_DTS_NAME = $(basename $(filter %.dts,$(notdir $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_DTS_PATH))))) endif -ifeq ($(BR2_LINUX_KERNEL_DTS_SUPPORT)$(KERNEL_DTS_NAME),y) +ifeq ($(BR_BUILDING)$(BR2_LINUX_KERNEL_DTS_SUPPORT)$(KERNEL_DTS_NAME),yy) $(error No kernel device tree source specified, check your \ BR2_LINUX_KERNEL_USE_INTREE_DTS / BR2_LINUX_KERNEL_USE_CUSTOM_DTS settings) endif -ifeq ($(BR2_LINUX_KERNEL_APPENDED_DTB),y) +ifeq ($(BR_BUILDING)$(BR2_LINUX_KERNEL_APPENDED_DTB),yy) ifneq ($(words $(KERNEL_DTS_NAME)),1) $(error Kernel with appended device tree needs exactly one DTS source. \ Check BR2_LINUX_KERNEL_INTREE_DTS_NAME or BR2_LINUX_KERNEL_CUSTOM_DTS_PATH.)
Currently, this is triggering the error message: make randconfig make source Limit the checks that enforce a DTS is set and at most one DTB is appended to when we are actually building, like is done for the configuration-file variables. Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> --- linux/linux.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)