Re: [PATCH 19/41] bsps/irq: Implement new directives for GICv2/3

2021-07-13 Thread Kinsey Moore


On 7/13/2021 00:16, Sebastian Huber wrote:

On 13/07/2021 04:46, Kinsey Moore wrote:

index a1ba5e9112..6f5d4015e4 100644
--- a/bsps/shared/dev/irq/arm-gicv2.c
+++ b/bsps/shared/dev/irq/arm-gicv2.c
@@ -1,5 +1,5 @@
  /*
- * Copyright (c) 2013, 2019 embedded brains GmbH.  All rights 
reserved.
+ * Copyright (c) 2013, 2021 embedded brains GmbH.  All rights 
reserved.

   *
   *  embedded brains GmbH
   *  Dornierstr. 4
@@ -69,6 +69,28 @@ rtems_status_code bsp_interrupt_get_attributes(
    rtems_interrupt_attributes *attributes
  )
  {
+  attributes->is_maskable = true;
+  attributes->maybe_enable = true;
+
+  if ( vector <= ARM_GIC_IRQ_SGI_LAST ) {
+    attributes->always_enabled = true;


As far as I'm aware, SGIs can be enabled or disabled using 
GICD_ISENABLER0 just like


PPI or SPI interrupts for both GICv2 and GICv3. Section 3.1.2 of the 
GICv2 architecture


spec (IHI0048B) references this, though I have seen implementations 
where certain SGI


and PPI interrupts are hard-wired enabled or disabled and that state 
can't be changed


(which is also covered in this section).


Ok, on Qemu and the i.MX7D the SGI are always enabled. I would keep 
the attributes like this until we have a system which is different. I 
will remove the check in bsp_interrupt_vector_enable/disable(). So, in 
the worst case, the attributes are wrong.
I only mention it because I've seen it on ZynqMP hardware. Interrupt 
enable bits for interrupts 0-24 are locked with 0-7 permanently enabled 
and 8-24 permanently disabled. I think the QEMU GICv3 driver allows all 
SGIs to be enabled or disabled. I tried to get more information about 
whether those bits can be unlocked, but nothing has been forthcoming: 
https://forums.xilinx.com/t5/Processor-System-Design-and-AXI/Zynq-Ultrascale-MPSoC-SGI-and-PPI-enable/td-p/1212370





+    attributes->can_enable = true;
+    attributes->can_cause = true;
+    attributes->can_cause_on = true;
+    attributes->cleared_by_acknowledge = true;
+  } else {
+    attributes->can_disable = true;
+    attributes->can_cause = true;
+    attributes->can_clear = true;
+
+    if ( vector > ARM_GIC_IRQ_PPI_LAST ) {
+  /* SPI */
+  attributes->can_enable = true;
+  attributes->can_get_affinity = true;
+  attributes->can_set_affinity = true;
+    }
+  }
+
    return RTEMS_SUCCESSFUL;
  }
@@ -77,16 +99,25 @@ rtems_status_code bsp_interrupt_is_pending(
    bool   *pending
  )
  {
- bsp_interrupt_assert(bsp_interrupt_is_valid_vector(vector));
-  bsp_interrupt_assert(pending != NULL);
-  *pending = false;
-  return RTEMS_UNSATISFIED;
+  volatile gic_dist *dist = ARM_GIC_DIST;
+
+  *pending = gic_id_is_pending(dist, vector);
+  return RTEMS_SUCCESSFUL;
  }
  rtems_status_code bsp_interrupt_cause(rtems_vector_number vector)
  {
bsp_interrupt_assert(bsp_interrupt_is_valid_vector(vector));
-  return RTEMS_UNSATISFIED;
+
+  if (vector <= ARM_GIC_IRQ_SGI_LAST) {
+    arm_gic_trigger_sgi(vector, 1U << _SMP_Get_current_processor());
+  } else {
+    volatile gic_dist *dist = ARM_GIC_DIST;
+
+    gic_id_set_pending(dist, vector);
+  }
+
+  return RTEMS_SUCCESSFUL;
  }
  #if defined(RTEMS_SMP)
@@ -95,15 +126,27 @@ rtems_status_code bsp_interrupt_cause_on(
    uint32_t    cpu_index
  )
  {
- bsp_interrupt_assert(bsp_interrupt_is_valid_vector(vector));
-  return RTEMS_UNSATISFIED;
+  if (vector >= 16) {
+    return RTEMS_UNSATISFIED;
+  }
+
+  arm_gic_trigger_sgi(vector, 1U << cpu_index);
+  return RTEMS_SUCCESSFUL;
  }
  #endif
  rtems_status_code bsp_interrupt_clear(rtems_vector_number vector)
  {
+  volatile gic_dist *dist = ARM_GIC_DIST;
+
bsp_interrupt_assert(bsp_interrupt_is_valid_vector(vector));
-  return RTEMS_UNSATISFIED;
+
+  if (vector <= ARM_GIC_IRQ_SGI_LAST) {
+    return RTEMS_UNSATISFIED;
+  }
+
+  gic_id_clear_pending(dist, vector);
+  return RTEMS_SUCCESSFUL;
  }
  rtems_status_code bsp_interrupt_vector_is_enabled(
@@ -113,8 +156,16 @@ rtems_status_code bsp_interrupt_vector_is_enabled(
  {
bsp_interrupt_assert(bsp_interrupt_is_valid_vector(vector));
    bsp_interrupt_assert(enabled != NULL);
-  *enabled = false;
-  return RTEMS_UNSATISFIED;
+
+  if (vector <= ARM_GIC_IRQ_SGI_LAST) {
+    *enabled = true;
+  } else {
+    volatile gic_dist *dist = ARM_GIC_DIST;
+
+    *enabled = gic_id_is_enabled(dist, vector);
+  }
+
+  return RTEMS_SUCCESSFUL;
  }
  rtems_status_code bsp_interrupt_vector_enable(rtems_vector_number 
vector)
@@ -133,6 +184,11 @@ rtems_status_code 
bsp_interrupt_vector_disable(rtems_vector_number vector)

bsp_interrupt_assert(bsp_interrupt_is_valid_vector(vector));
+  if (vector <= ARM_GIC_IRQ_SGI_LAST) {
+    /* SGI cannot be disabled */
+    return RTEMS_UNSATISFIED;
+  }
+


I will remove this check here.


    gic_id_disable(dist, vector);
    return RTEMS_SUCCESSFUL;
  }
@@ -207,8 +263,8 @@ BSP_START_TEXT_SECTION void 
arm_gic_irq_initialize_secondary_cpu(void)

    dist->icdigr[0] = 0x;
  #endif
-  /* Initialize Peripheral Private Interrupts (

Re: [PATCH 19/41] bsps/irq: Implement new directives for GICv2/3

2021-07-13 Thread Chris Johns
On 13/7/21 10:55 pm, Kinsey Moore wrote:
> On 7/13/2021 00:16, Sebastian Huber wrote:
>> On 13/07/2021 04:46, Kinsey Moore wrote:
 index a1ba5e9112..6f5d4015e4 100644
 --- a/bsps/shared/dev/irq/arm-gicv2.c
 +++ b/bsps/shared/dev/irq/arm-gicv2.c
 @@ -1,5 +1,5 @@
   /*
 - * Copyright (c) 2013, 2019 embedded brains GmbH.  All rights reserved.
 + * Copyright (c) 2013, 2021 embedded brains GmbH.  All rights reserved.
    *
    *  embedded brains GmbH
    *  Dornierstr. 4
 @@ -69,6 +69,28 @@ rtems_status_code bsp_interrupt_get_attributes(
     rtems_interrupt_attributes *attributes
   )
   {
 +  attributes->is_maskable = true;
 +  attributes->maybe_enable = true;
 +
 +  if ( vector <= ARM_GIC_IRQ_SGI_LAST ) {
 +    attributes->always_enabled = true;
>>>
>>> As far as I'm aware, SGIs can be enabled or disabled using GICD_ISENABLER0
>>> just like
>>>
>>> PPI or SPI interrupts for both GICv2 and GICv3. Section 3.1.2 of the GICv2
>>> architecture
>>>
>>> spec (IHI0048B) references this, though I have seen implementations where
>>> certain SGI
>>>
>>> and PPI interrupts are hard-wired enabled or disabled and that state can't 
>>> be
>>> changed
>>>
>>> (which is also covered in this section).
>>
>> Ok, on Qemu and the i.MX7D the SGI are always enabled. I would keep the
>> attributes like this until we have a system which is different. 

Should a comment be added that says this?

>> I will remove
>> the check in bsp_interrupt_vector_enable/disable(). So, in the worst case, 
>> the
>> attributes are wrong.
> I only mention it because I've seen it on ZynqMP hardware. Interrupt enable 
> bits
> for interrupts 0-24 are locked with 0-7 permanently enabled and 8-24 
> permanently
> disabled. I think the QEMU GICv3 driver allows all SGIs to be enabled or
> disabled. I tried to get more information about whether those bits can be
> unlocked, but nothing has been forthcoming:
> https://forums.xilinx.com/t5/Processor-System-Design-and-AXI/Zynq-Ultrascale-MPSoC-SGI-and-PPI-enable/td-p/1212370

I do not know about the GIC but I know with the ARM debug hardware the
implementers can wrap the IP in different ways and that means something that is
user controllable may be fixed in other implementation or brought out to
external pins in another.

Chris
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH v1] Reports: Convert to C++

2021-07-13 Thread Chris Johns
Hi Ryan,

Looks great and thank you for making the changes. I have a minor nit below and
with that change the patch to OK to push. There is no need for a fruther review.

Thanks
Chris

On 12/7/21 11:42 pm, Ryan Long wrote:
> ---
>  tester/covoar/ReportsBase.cc |  296 ++--
>  tester/covoar/ReportsBase.h  |  118 +++--
>  tester/covoar/ReportsHtml.cc | 1074 
> +-
>  tester/covoar/ReportsHtml.h  |   94 ++--
>  tester/covoar/ReportsText.cc |  261 +-
>  tester/covoar/ReportsText.h  |   34 +-
>  6 files changed, 838 insertions(+), 1039 deletions(-)
> 
> diff --git a/tester/covoar/ReportsBase.cc b/tester/covoar/ReportsBase.cc
> index 328980d..7fd3422 100644
> --- a/tester/covoar/ReportsBase.cc
> +++ b/tester/covoar/ReportsBase.cc
> @@ -4,6 +4,9 @@
>  #include 
>  #include 
>  
> +#include 
> +#include 
> +
>  #include "ReportsBase.h"
>  #include "app_common.h"
>  #include "CoverageRanges.h"
> @@ -20,7 +23,7 @@
>  
>  namespace Coverage {
>  
> -ReportsBase::ReportsBase( time_t timestamp, std::string symbolSetName ):
> +ReportsBase::ReportsBase( time_t timestamp, const std::string& symbolSetName 
> ):
>reportExtension_m(""),
>symbolSetName_m(symbolSetName),
>timestamp_m( timestamp )
> @@ -31,13 +34,13 @@ ReportsBase::~ReportsBase()
>  {
>  }
>  
> -FILE* ReportsBase::OpenFile(
> -  const char* const fileName,
> -  const char* const symbolSetName
> +void ReportsBase::OpenFile(
> +  const std::string& fileName,
> +  const std::string& symbolSetName,
> +  std::ofstream& aFile
>  )
>  {
>int  sc;
> -  FILE*aFile;
>std::string  file;
>  
>std::string symbolSetOutputDirectory;
> @@ -54,120 +57,131 @@ FILE* ReportsBase::OpenFile(
>sc = mkdir( symbolSetOutputDirectory.c_str(),0755 );
>  #endif
>if ( (sc == -1) && (errno != EEXIST) ) {
> -fprintf(
> -  stderr,
> -  "Unable to create output directory %s\n",
> -  symbolSetOutputDirectory.c_str()
> +throw rld::error(
> +  "Unable to create output directory",
> +  "ReportsBase::OpenFile"
>  );
> -return NULL;
> +return;
>}
>  
>file = symbolSetOutputDirectory;
>rld::path::path_join(file, fileName, file);
>  
>// Open the file.
> -  aFile = fopen( file.c_str(), "w" );
> -  if ( !aFile ) {
> -fprintf( stderr, "Unable to open %s\n", file.c_str() );
> +  aFile.open( file );
> +  if ( !aFile.is_open() ) {
> +std::cerr << "Unable to open " << file << std::endl;
>}
> -  return aFile;
> +  return;

Not needed.

>  }
>  
>  void ReportsBase::WriteIndex(
> -  const char* const fileName
> +  const std::string& fileName
>  )
>  {
>  }
>  
> -FILE* ReportsBase::OpenAnnotatedFile(
> -  const char* const fileName
> +void ReportsBase::OpenAnnotatedFile(
> +  const std::string& fileName,
> +  std::ofstream& aFile
>  )
>  {
> -  return OpenFile(fileName, symbolSetName_m.c_str());
> +  OpenFile(fileName, symbolSetName_m, aFile);
> +  return;

Same.

>  }
>  
> -FILE* ReportsBase::OpenBranchFile(
> -  const char* const fileName,
> -  bool  hasBranches
> +void ReportsBase::OpenBranchFile(
> +  const std::string& fileName,
> +  bool   hasBranches,
> +  std::ofstream& aFile
>  )
>  {
> -  return OpenFile(fileName, symbolSetName_m.c_str());
> +  OpenFile(fileName, symbolSetName_m, aFile);
> +  return;
>  }
>  
> -FILE* ReportsBase::OpenCoverageFile(
> -  const char* const fileName
> +void ReportsBase::OpenCoverageFile(
> +  const std::string& fileName,
> +  std::ofstream& aFile
>  )
>  {
> -  return OpenFile(fileName, symbolSetName_m.c_str());
> +  OpenFile(fileName, symbolSetName_m, aFile);
> +  return;

same

>  }
>  
> -FILE* ReportsBase::OpenNoRangeFile(
> -  const char* const fileName
> +void ReportsBase::OpenNoRangeFile(
> +  const std::string& fileName,
> +  std::ofstream& aFile
>  )
>  {
> -  return OpenFile(fileName, symbolSetName_m.c_str());
> +  OpenFile(fileName, symbolSetName_m, aFile);
> +  return;

etc

>  }
>  
>  
> -FILE* ReportsBase::OpenSizeFile(
> -  const char* const fileName
> +void ReportsBase::OpenSizeFile(
> +  const std::string& fileName,
> +  std::ofstream& aFile
>  )
>  {
> -  return OpenFile(fileName, symbolSetName_m.c_str());
> +  OpenFile(fileName, symbolSetName_m, aFile);
> +  return;
>  }
>  
> -FILE* ReportsBase::OpenSymbolSummaryFile(
> -  const char* const fileName
> +void ReportsBase::OpenSymbolSummaryFile(
> +  const std::string& fileName,
> +  std::ofstream& aFile
>  )
>  {
> -  return OpenFile(fileName, symbolSetName_m.c_str());
> +  OpenFile(fileName, symbolSetName_m, aFile);
> +  return;
>  }
>  
>  void ReportsBase::CloseFile(
> -  FILE*  aFile
> +  std::ofstream& aFile
>  )
>  {
> -  fclose( aFile );
> +  aFile.close();
>  }
>  
>  void ReportsBase::CloseAnnotatedFile(
> -  FILE*  aFile
> +  std::ofstream& aFile
>  )
>  {
>CloseFile( aFile );
>  }
>  
>  void ReportsBase::CloseBranchFile(
> -  FILE*  aFile,
> -  bool   hasBranches
> 

Re: typical RTEMS system HW?

2021-07-13 Thread Shiro


> On Jul 9, 2021, at 11:40 AM, Joel Sherrill  wrote:
> 
> On Thu, Jul 8, 2021 at 3:11 AM Shiro  wrote:
>> 
>> Hello,
>> 
>> I’m new to RTEMS but very much like its features.  I’m wonder what HW 
>> (MCU/MPU and memory size) RTEMS is typical used on.  Anyone care to share a 
>> bit of details?
>> 
> 
> Well I held back hoping more people on the using side would answer.
> This is my perspective from the long-term developer view.
> 
> Most users are on 32-bit platforms. There have been 64-bit ports for a
> long time but most people aren't using them.
> 
> There are a lot of users from the space community who tend to be on
> hardened hardware. This SPARC LEON family is probably the most
> commonly used there. In the early days of the LEON predecessor
> (ERC32), I would say 1-4 MB was quite common for RAM. Other CPU
> architectures have been used in RTEMS space-based systems like
> PowerPC, Coldfire, MIPS, and ARM.
> 
> There are also a lot of users in the EPICS (Experimental Physics and
> Industrial Control System) community which is commonly used in high
> energy physics, astronomy, and other big science equipment. I would
> tend to put a lot of those users on VME and CompactPCI boards with the
> PowerPC and x86 being common. They may still use some of their old
> VME m68040 boards or at least we haven't dropped support for them.
> These mostly tend to have 4-16 MB RAM.
> 
> There are also a lot of commercial users and currently I think a lot
> of those are on ARM platforms (STM, Xilinx, NXP, etc) with RISC-V
> coming on strong. There are users on other architectures but I think
> the bulk are there now. PowerPC users seem to be realizing they are on
> the tail end of the lifecycle and need to look forward to a
> transition.
> 
> RTEMS has been ported to 16-bit CPUs but there are none currently in the tree.
> 
> You can certainly have an RTEMS application which fits into a small
> amount of Flash but as you add capabilities, you tend to pick up code
> and data space requirements. The earliest RTEMS systems would be big
> if they had 1-4 MB total Flash and RAM but that's considered small
> these days in general. I recall one m68k based system that has 96K
> total code and data space. The legacy network stack could easily run
> on 1 MB RAM. Now libbsd generally has more features and takes more
> memory. It may be able to be trimmed down but no one has invested the
> time. lwIP would be a better alternative in those cases.
> 
> If you need a lot of features, the footprint naturally grows. If you
> are on a small target board, some tests won't fit. For example, some
> of the file system tests assume a 1MB RAM disk. If you don't have the
> space, you just don't.
> 
> I still think that a large RTEMS system is on the smallest end of
> Linux systems.
> 
> Not sure this gives specific answers but that's my impression.
> 
> —joel


Wow, what an awesome reply.  Thank you for the detailed response, I appreciate 
your time.  This helps me quite a bit.  My take away is that RTEMS is not going 
to be a comfortable fit for Cortex M3/M4 MCUs with 64k to 256k RAM (although it 
can be shoehorned in with some work).

This helps me partition the MCU/MPU space by RTOS.

Thanks again,
Shiro


___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

[PATCH] build: Use BSP family for options

2021-07-13 Thread Sebastian Huber
Close #4468.
---
 wscript | 51 ---
 1 file changed, 28 insertions(+), 23 deletions(-)

diff --git a/wscript b/wscript
index 6626fafb74..357e8918df 100755
--- a/wscript
+++ b/wscript
@@ -203,11 +203,11 @@ class Item(object):
 def get_enabled_by(self):
 return self.data["enabled-by"]
 
-def defaults(self, enable, variant):
+def defaults(self, enable, variant, family):
 if _is_enabled(enable, self.get_enabled_by()):
 for p in self.links():
-p.defaults(enable, variant)
-self.do_defaults(variant)
+p.defaults(enable, variant, family)
+self.do_defaults(variant, family)
 
 def configure(self, conf, cic):
 if _is_enabled(conf.env.ENABLE, self.get_enabled_by()):
@@ -223,7 +223,7 @@ class Item(object):
 p.build(bld, bic)
 self.do_build(bld, bic)
 
-def do_defaults(self, variant):
+def do_defaults(self, variant, family):
 return
 
 def prepare_configure(self, conf, cic):
@@ -592,9 +592,6 @@ class BSPItem(Item):
 arch_bsps = bsps.setdefault(data["arch"].strip(), {})
 arch_bsps[data["bsp"].strip()] = self
 
-def prepare_configure(self, conf, cic):
-conf.env.BSP_FAMILY = self.data["family"]
-
 def prepare_build(self, bld, bic):
 return BuildItemContext(
 bic.includes + bld.env.BSP_INCLUDES.split(), [], [], []
@@ -689,16 +686,18 @@ class OptionItem(Item):
 super(OptionItem, self).__init__(uid, data)
 
 @staticmethod
-def _is_variant(variants, variant):
+def _is_variant(variants, variant, family):
 for pattern in variants:
 if re.match(pattern + "$", variant):
 return True
+if re.match(pattern + "$", family):
+return True
 return False
 
-def default_value(self, variant):
+def default_value(self, variant, family):
 value = self.data["default"]
 for default in self.data["default-by-variant"]:
-if OptionItem._is_variant(default["variants"], variant):
+if OptionItem._is_variant(default["variants"], variant, family):
 value = default["value"]
 break
 if value is None:
@@ -709,8 +708,8 @@ class OptionItem(Item):
 return value
 return self.data["format"].format(value)
 
-def do_defaults(self, variant):
-value = self.default_value(variant)
+def do_defaults(self, variant, family):
+value = self.default_value(variant, family)
 if value is None:
 return
 description = self.data["description"]
@@ -917,7 +916,7 @@ class OptionItem(Item):
 value = cic.cp.getboolean(conf.variant, name)
 cic.add_option(name)
 except configparser.NoOptionError:
-value = self.default_value(conf.env.ARCH_BSP)
+value = self.default_value(conf.env.ARCH_BSP, conf.env.ARCH_FAMILY)
 except ValueError as ve:
 conf.fatal(
 "Invalid value for configuration option {}: {}".format(name, 
ve)
@@ -933,7 +932,7 @@ class OptionItem(Item):
 value = cic.cp.get(conf.variant, name)
 cic.add_option(name)
 except configparser.NoOptionError:
-value = self.default_value(conf.env.ARCH_BSP)
+value = self.default_value(conf.env.ARCH_BSP, conf.env.ARCH_FAMILY)
 if value is None:
 return value
 try:
@@ -952,7 +951,7 @@ class OptionItem(Item):
 cic.add_option(name)
 value = no_unicode(value)
 except configparser.NoOptionError:
-value = self.default_value(conf.env.ARCH_BSP)
+value = self.default_value(conf.env.ARCH_BSP, conf.env.ARCH_FAMILY)
 return value
 
 def _script(self, conf, cic, value, arg):
@@ -1365,11 +1364,20 @@ def configure_variant(conf, cp, bsp_map, path_list, 
top_group, variant):
 conf.setenv(variant)
 arch, bsp_name = variant.split("/")
 bsp_base = bsp_map.get(bsp_name, bsp_name)
-arch_bsp = arch + "/" + bsp_base
 
+try:
+bsp_item = bsps[arch][bsp_base]
+except KeyError:
+conf.fatal("No such base BSP: '{}'".format(variant))
+
+bsp_family = self.data["family"]
+arch_bsp = arch + "/" + bsp_base
+arch_family = arch + "/" + bsp_family
 conf.env["ARCH"] = arch
 conf.env["ARCH_BSP"] = arch_bsp
+conf.env["ARCH_FAMILY"] = arch_family
 conf.env["BSP_BASE"] = bsp_base
+conf.env["BSP_FAMILY"] = bsp_family
 conf.env["BSP_NAME"] = bsp_name
 conf.env["DEST_OS"] = "rtems"
 
@@ -1384,11 +1392,6 @@ def configure_variant(conf, cp, bsp_map, path_list, 
top_group, variant):
 cic = ConfigItemContext(cp, path_list)
 
 items[conf.env.TOPGROUP].configure(conf, cic)
-
-try:
-bsp_item = bsps[arch][bsp_base]
-except KeyError:
-conf.fatal("No

Re: [PATCH] build: Use BSP family for options

2021-07-13 Thread Chris Johns



On 14/7/21 4:20 pm, Sebastian Huber wrote:
> Close #4468.
> ---
>  wscript | 51 ---
>  1 file changed, 28 insertions(+), 23 deletions(-)
> 
> diff --git a/wscript b/wscript
> index 6626fafb74..357e8918df 100755
> --- a/wscript
> +++ b/wscript
> @@ -203,11 +203,11 @@ class Item(object):
>  def get_enabled_by(self):
>  return self.data["enabled-by"]
>  
> -def defaults(self, enable, variant):
> +def defaults(self, enable, variant, family):
>  if _is_enabled(enable, self.get_enabled_by()):
>  for p in self.links():
> -p.defaults(enable, variant)
> -self.do_defaults(variant)
> +p.defaults(enable, variant, family)
> +self.do_defaults(variant, family)
>  
>  def configure(self, conf, cic):
>  if _is_enabled(conf.env.ENABLE, self.get_enabled_by()):
> @@ -223,7 +223,7 @@ class Item(object):
>  p.build(bld, bic)
>  self.do_build(bld, bic)
>  
> -def do_defaults(self, variant):
> +def do_defaults(self, variant, family):
>  return
>  
>  def prepare_configure(self, conf, cic):
> @@ -592,9 +592,6 @@ class BSPItem(Item):
>  arch_bsps = bsps.setdefault(data["arch"].strip(), {})
>  arch_bsps[data["bsp"].strip()] = self
>  
> -def prepare_configure(self, conf, cic):
> -conf.env.BSP_FAMILY = self.data["family"]
> -
>  def prepare_build(self, bld, bic):
>  return BuildItemContext(
>  bic.includes + bld.env.BSP_INCLUDES.split(), [], [], []
> @@ -689,16 +686,18 @@ class OptionItem(Item):
>  super(OptionItem, self).__init__(uid, data)
>  
>  @staticmethod
> -def _is_variant(variants, variant):
> +def _is_variant(variants, variant, family):
>  for pattern in variants:
>  if re.match(pattern + "$", variant):
>  return True
> +if re.match(pattern + "$", family):
> +return True
>  return False
>  
> -def default_value(self, variant):
> +def default_value(self, variant, family):
>  value = self.data["default"]
>  for default in self.data["default-by-variant"]:
> -if OptionItem._is_variant(default["variants"], variant):
> +if OptionItem._is_variant(default["variants"], variant, family):

I would prefer we keep the families and the variants separate. It is cleaner.

I have basically the same patch but optional handling `default-by-family`. :)

I have discussed this on the ticket.

Chris

>  value = default["value"]
>  break
>  if value is None:
> @@ -709,8 +708,8 @@ class OptionItem(Item):
>  return value
>  return self.data["format"].format(value)
>  
> -def do_defaults(self, variant):
> -value = self.default_value(variant)
> +def do_defaults(self, variant, family):
> +value = self.default_value(variant, family)
>  if value is None:
>  return
>  description = self.data["description"]
> @@ -917,7 +916,7 @@ class OptionItem(Item):
>  value = cic.cp.getboolean(conf.variant, name)
>  cic.add_option(name)
>  except configparser.NoOptionError:
> -value = self.default_value(conf.env.ARCH_BSP)
> +value = self.default_value(conf.env.ARCH_BSP, 
> conf.env.ARCH_FAMILY)
>  except ValueError as ve:
>  conf.fatal(
>  "Invalid value for configuration option {}: {}".format(name, 
> ve)
> @@ -933,7 +932,7 @@ class OptionItem(Item):
>  value = cic.cp.get(conf.variant, name)
>  cic.add_option(name)
>  except configparser.NoOptionError:
> -value = self.default_value(conf.env.ARCH_BSP)
> +value = self.default_value(conf.env.ARCH_BSP, 
> conf.env.ARCH_FAMILY)
>  if value is None:
>  return value
>  try:
> @@ -952,7 +951,7 @@ class OptionItem(Item):
>  cic.add_option(name)
>  value = no_unicode(value)
>  except configparser.NoOptionError:
> -value = self.default_value(conf.env.ARCH_BSP)
> +value = self.default_value(conf.env.ARCH_BSP, 
> conf.env.ARCH_FAMILY)
>  return value
>  
>  def _script(self, conf, cic, value, arg):
> @@ -1365,11 +1364,20 @@ def configure_variant(conf, cp, bsp_map, path_list, 
> top_group, variant):
>  conf.setenv(variant)
>  arch, bsp_name = variant.split("/")
>  bsp_base = bsp_map.get(bsp_name, bsp_name)
> -arch_bsp = arch + "/" + bsp_base
>  
> +try:
> +bsp_item = bsps[arch][bsp_base]
> +except KeyError:
> +conf.fatal("No such base BSP: '{}'".format(variant))
> +
> +bsp_family = self.data["family"]
> +arch_bsp = arch + "/" + bsp_base
> +arch_family = arch + "/" + bsp_family
>  conf.env["ARCH"] = arch
>  conf.env["ARCH_BSP

[PATCH v2] build: Use BSP family for options

2021-07-13 Thread chrisj
From: Chris Johns 

- Optionally add support for 'default-by-family' to allow
  option to be set by a family and so all related BSPs

Close #4468
---
 wscript | 36 +++-
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/wscript b/wscript
index 6626fafb74..61253b4a0b 100755
--- a/wscript
+++ b/wscript
@@ -592,9 +592,6 @@ class BSPItem(Item):
 arch_bsps = bsps.setdefault(data["arch"].strip(), {})
 arch_bsps[data["bsp"].strip()] = self
 
-def prepare_configure(self, conf, cic):
-conf.env.BSP_FAMILY = self.data["family"]
-
 def prepare_build(self, bld, bic):
 return BuildItemContext(
 bic.includes + bld.env.BSP_INCLUDES.split(), [], [], []
@@ -695,12 +692,18 @@ class OptionItem(Item):
 return True
 return False
 
-def default_value(self, variant):
+def default_value(self, variant, family):
 value = self.data["default"]
 for default in self.data["default-by-variant"]:
 if OptionItem._is_variant(default["variants"], variant):
 value = default["value"]
 break
+if 'default-by-family' in self.data:
+for default in self.data["default-by-family"]:
+if 'families' in default:
+if OptionItem._is_variant(default["families"], family):
+value = default["value"]
+break
 if value is None:
 return value
 if isinstance(value, list):
@@ -709,8 +712,8 @@ class OptionItem(Item):
 return value
 return self.data["format"].format(value)
 
-def do_defaults(self, variant):
-value = self.default_value(variant)
+def do_defaults(self, variant, family):
+value = self.default_value(variant, family)
 if value is None:
 return
 description = self.data["description"]
@@ -917,7 +920,7 @@ class OptionItem(Item):
 value = cic.cp.getboolean(conf.variant, name)
 cic.add_option(name)
 except configparser.NoOptionError:
-value = self.default_value(conf.env.ARCH_BSP)
+value = self.default_value(conf.env.ARCH_BSP, conf.env.ARCH_FAMILY)
 except ValueError as ve:
 conf.fatal(
 "Invalid value for configuration option {}: {}".format(name, 
ve)
@@ -933,7 +936,7 @@ class OptionItem(Item):
 value = cic.cp.get(conf.variant, name)
 cic.add_option(name)
 except configparser.NoOptionError:
-value = self.default_value(conf.env.ARCH_BSP)
+value = self.default_value(conf.env.ARCH_BSP, conf.env.ARCH_FAMILY)
 if value is None:
 return value
 try:
@@ -952,7 +955,7 @@ class OptionItem(Item):
 cic.add_option(name)
 value = no_unicode(value)
 except configparser.NoOptionError:
-value = self.default_value(conf.env.ARCH_BSP)
+value = self.default_value(conf.env.ARCH_BSP, conf.env.ARCH_FAMILY)
 return value
 
 def _script(self, conf, cic, value, arg):
@@ -1365,12 +1368,23 @@ def configure_variant(conf, cp, bsp_map, path_list, 
top_group, variant):
 conf.setenv(variant)
 arch, bsp_name = variant.split("/")
 bsp_base = bsp_map.get(bsp_name, bsp_name)
+
+try:
+bsp_item = bsps[arch][bsp_base]
+except KeyError:
+conf.fatal("No such base BSP: '{}'".format(variant))
+
+family = bsp_item.data['family']
+
 arch_bsp = arch + "/" + bsp_base
+arch_family = arch + "/" + family
 
 conf.env["ARCH"] = arch
 conf.env["ARCH_BSP"] = arch_bsp
+conf.env["ARCH_FAMILY"] = arch_family
 conf.env["BSP_BASE"] = bsp_base
 conf.env["BSP_NAME"] = bsp_name
+conf.env["BSP_FAMILY"] = family
 conf.env["DEST_OS"] = "rtems"
 
 # For the enabled-by evaluation we have to use the base BSP defined by the
@@ -1385,10 +1399,6 @@ def configure_variant(conf, cp, bsp_map, path_list, 
top_group, variant):
 
 items[conf.env.TOPGROUP].configure(conf, cic)
 
-try:
-bsp_item = bsps[arch][bsp_base]
-except KeyError:
-conf.fatal("No such base BSP: '{}'".format(variant))
 bsp_item.configure(conf, cic)
 
 options = set([o[0].upper() for o in cp.items(variant)])
-- 
2.24.1

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel