Re: libbsd kernel namespace generation

2021-07-23 Thread Chris Johns
On 23/7/21 5:00 pm, Sebastian Huber wrote:
> Hello Chris,
> 
> On 22/07/2021 10:44, Chris Johns wrote:
>> Hello,
>>
>> Libbsd uses the pre-processor to map all the kernel calls into a libbsd 
>> kernel
>> name space by prepending _bsd_ to each symbol. The script ...
>>
>> https://git.rtems.org/rtems-libbsd/tree/create-kernel-namespace.sh?h=6-freebsd-12
>>
>> ... generates the list and the result is pushed into the repo. The symbols 
>> need
>> to be regenerated when new sources are added into the `freebsd` tree.
>>
>> The script has a few issues:
>>
>> 1. Objdump does not work on FreeBSD for different archs.
>>
>> 2. Binutils is being removed from FreeBSD base.
> 
> what would be the alternative?

The ones we build. It is not a big change to the script but one that needs to be
amde. We cannot expect the host OS to provide a suitable objdump.

>> 3. A number of BSPs need to be built to cover all the possible symbols
>>
>> I would like to document the list of BSPs a generate needs to cover. I 
>> propose:
>>
>>   arm/xilinx_zynq_a9_qemu
>>   aarch64/xilinx_versal_lp64_qemu
>>   i386/pc686
>>   powerpc/mvme2307
>>   sparc/erc32
> 
> Basically if you import code you just have to build a BSP which covers the
> imported code. Then you use ...

There are other complexities that only a handful of people in the world would
know or understand. For example you build a zynq BSP or any 32bit BSP and import
code that conditionally defines 64bit functionality then you may miss some
symbols. We have cases of this, well I am seeing them adding VFS.

>> Also the documentation says to use `git add -p` to add the changes. How does 
>> an
>> interactive add help?
> 
> git add -p
> 
> to add only the changes relevant to the imported (or removed or changed) code.

OK and this is good advice so I will add something to the doco to state this is 
why.

A baseline list of BSPs is also needed to regenerate the file. We cannot add for
ever because some symbols disappear over time.

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

Re: [PATCH] build: Add "family/" prefix to BSP familiy enable

2021-07-22 Thread Chris Johns


> On 22 Jul 2021, at 6:53 pm, Sebastian Huber 
>  wrote:
>> On 22/07/2021 10:47, Chris Johns wrote:
>>> On 22/7/21 6:37 pm, Sebastian Huber wrote:
>>> On 22/07/2021 10:33, Chris Johns wrote:
>>>>>> and so the arch part is not
>>>>>> really needed. My concern is this type code ...
>>>>>> 
>>>>>> https://git.rtems.org/rtems_waf/tree/rtems.py#n758
>>>>>> 
>>>>>> that breaks. Is this an issue? I think a single `/` in a BSP or family is
>>>>>> cleaner.
>>>>> Why is this an issue? This BSP family stuff is local to the RTEMS build 
>>>>> system.
>>>> Currently. It is about the symmetry of the naming and how it would look 
>>>> from
>>>> outside. Nothing more.
>>> You mean that maybe someone wants to build an application or library for a 
>>> BSP
>>> family? I guess this is currently not supported, but you could do this with 
>>> the
>>> "bsps/powerpc/motorola_powerpc" approach.
>> Yes it could happen and this is where the symmetry and the existing code
>> matters. For example with `bsps/motorola_powerpc` the code can be easily or
>> cleanly extended by looking for `bsps` as an arch and knowing that is a
>> `family`. Otherwise the error is `invalid arch/bsp` and then you need check 
>> two
>> lengths etc etc.
> 
> If you want to add this feature, then you have to update the code anyway. You 
> have to figure out which BSPs belong to a family for example. 

Yeap. It is a detail but I am looking ahead. 

Chris

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


Re: [PATCH] build: Add "family/" prefix to BSP familiy enable

2021-07-22 Thread Chris Johns
On 22/7/21 6:37 pm, Sebastian Huber wrote:
> On 22/07/2021 10:33, Chris Johns wrote:
>>>> and so the arch part is not
>>>> really needed. My concern is this type code ...
>>>>
>>>> https://git.rtems.org/rtems_waf/tree/rtems.py#n758
>>>>
>>>> that breaks. Is this an issue? I think a single `/` in a BSP or family is
>>>> cleaner.
>>> Why is this an issue? This BSP family stuff is local to the RTEMS build 
>>> system.
>> Currently. It is about the symmetry of the naming and how it would look from
>> outside. Nothing more.
> 
> You mean that maybe someone wants to build an application or library for a BSP
> family? I guess this is currently not supported, but you could do this with 
> the
> "bsps/powerpc/motorola_powerpc" approach.

Yes it could happen and this is where the symmetry and the existing code
matters. For example with `bsps/motorola_powerpc` the code can be easily or
cleanly extended by looking for `bsps` as an arch and knowing that is a
`family`. Otherwise the error is `invalid arch/bsp` and then you need check two
lengths etc etc.

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


libbsd kernel namespace generation

2021-07-22 Thread Chris Johns
Hello,

Libbsd uses the pre-processor to map all the kernel calls into a libbsd kernel
name space by prepending _bsd_ to each symbol. The script ...

https://git.rtems.org/rtems-libbsd/tree/create-kernel-namespace.sh?h=6-freebsd-12

... generates the list and the result is pushed into the repo. The symbols need
to be regenerated when new sources are added into the `freebsd` tree.

The script has a few issues:

1. Objdump does not work on FreeBSD for different archs.

2. Binutils is being removed from FreeBSD base.

3. A number of BSPs need to be built to cover all the possible symbols

I would like to document the list of BSPs a generate needs to cover. I propose:

 arm/xilinx_zynq_a9_qemu
 aarch64/xilinx_versal_lp64_qemu
 i386/pc686
 powerpc/mvme2307
 sparc/erc32

Also the documentation says to use `git add -p` to add the changes. How does an
interactive add help?

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


Re: [PATCH] build: Add "family/" prefix to BSP familiy enable

2021-07-22 Thread Chris Johns


On 22/7/21 4:44 pm, Sebastian Huber wrote:
> On 22/07/2021 08:37, Chris Johns wrote:
>> On 22/7/21 4:20 pm, Sebastian Huber wrote:
>>> On 22/07/2021 02:39, Chris Johns wrote:
>>>> On 22/7/21 5:22 am, Sebastian Huber wrote:
>>>>> BSP family and BSP variant names may be equal.  This prefix avoids
>>>>> ambiguity in the enabled-by expressions.
>>>>> ---
>>>>>    wscript | 2 +-
>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/wscript b/wscript
>>>>> index f27dba6831..b7a0412150 100755
>>>>> --- a/wscript
>>>>> +++ b/wscript
>>>>> @@ -1394,7 +1394,7 @@ def configure_variant(conf, cp, bsp_map, path_list,
>>>>> top_group, variant):
>>>>>    conf.env["ENABLE"] = [
>>>>>    get_compiler(conf, cp, variant),
>>>>>    arch,
>>>>> -    arch_family,
>>>>> +    "family/" + arch_family,
>>>>  "bsps/" + arch_family,
>>>>
>>>> ... as discussed in the other thread? If you are happy with the change as 
>>>> shown
>>>> please push.
>>>
>>> Yes, this is good and matches with our directory layout. I checked it in 
>>> with
>>> this change.
>>>
>>> We could also merge the default-by-family and default-by-variant lists with 
>>> this
>>> approach,
>>
>> I am not sure. My initial reaction was "yes" but how would different settings
>> for a BSP and a family be handled? I am assuming a BSP variant setting is 
>> able
>> to override a family setting. Is that possible if they are merged?
> 
> Yes, a BSP variant would have higher priority, this is enforced by the search
> order:
> 
>     for default in self.data["default-by-variant"]:
>     if OptionItem._is_variant(default["variants"], variant):
>     value = default["value"]
>     break
>     for default in self.data["default-by-family"]:
>     if OptionItem._is_variant(default["families"], family):
>     value = default["value"]
>     break

Then I am fine with the merging back to a single default-by-variant.

>>> for example:
>>>
>>> diff --git a/spec/build/bsps/optconsolebaud.yml
>>> b/spec/build/bsps/optconsolebaud.yml
>>> index 4b0869beca..0233fdd61b 100644
>>> --- a/spec/build/bsps/optconsolebaud.yml
>>> +++ b/spec/build/bsps/optconsolebaud.yml
>>> @@ -6,13 +6,10 @@ build-type: option
>>>   copyrights:
>>>   - Copyright (C) 2020 embedded brains GmbH (http://www.embedded-brains.de)
>>>   default: 115200
>>> -default-by-family:
>>> -- value: 9600
>>> -  families:
>>> -  - powerpc/motorola_powerpc
>>>   default-by-variant:
>>>   - value: 9600
>>>     variants:
>>> +  - bsps/powerpc/motorola_powerpc
>>
>> Oh I think my patch piece may have been wrong. This has two `/` and so three
>> components. We _must_ have unique family names 
> 
> The BSP family names are just names in an architecture directory, so this rule
> would be not enforced by the directory layout.

Sure and that fine.

>> and so the arch part is not
>> really needed. My concern is this type code ...
>>
>> https://git.rtems.org/rtems_waf/tree/rtems.py#n758
>>
>> that breaks. Is this an issue? I think a single `/` in a BSP or family is
>> cleaner.
> 
> Why is this an issue? This BSP family stuff is local to the RTEMS build 
> system.

Currently. It is about the symmetry of the naming and how it would look from
outside. Nothing more.

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

Re: [PATCH 20/41] sparc/irq: Implement new interrupt directives

2021-07-22 Thread Chris Johns
On 22/7/21 5:08 am, Sebastian Huber wrote:
> On 21/07/2021 21:04, Gedare Bloom wrote:
>> On Wed, Jul 21, 2021 at 12:31 PM Sebastian Huber
>>   wrote:
>>> On 21/07/2021 20:28, Gedare Bloom wrote:
 Why not throw an error here instead? In production, you wouldn't want
 this code...
>>> The main issue is the bad chip design. If we don't have this code, we
>>> can't test the extended interrupts. In production, you want tested code.
>>>
>> ok, thanks. My comments are all pretty minor, except for the
>> terminology issues of "cause" but that wording already exists. post
>> the v2 series, but I probably won't review it and you can check it in
>> if no one complains. It's up to you if you want to work a different
>> wording than "cause" -- I prefer "raise"
> 
> Thanks a lot for the review.
> 
> Joel, what is your opinion with respect to "cause" vs. "raise"?
> 

I think `raise`.

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

Re: [PATCH] build: Add "family/" prefix to BSP familiy enable

2021-07-22 Thread Chris Johns
On 22/7/21 4:20 pm, Sebastian Huber wrote:
> On 22/07/2021 02:39, Chris Johns wrote:
>> On 22/7/21 5:22 am, Sebastian Huber wrote:
>>> BSP family and BSP variant names may be equal.  This prefix avoids
>>> ambiguity in the enabled-by expressions.
>>> ---
>>>   wscript | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/wscript b/wscript
>>> index f27dba6831..b7a0412150 100755
>>> --- a/wscript
>>> +++ b/wscript
>>> @@ -1394,7 +1394,7 @@ def configure_variant(conf, cp, bsp_map, path_list,
>>> top_group, variant):
>>>   conf.env["ENABLE"] = [
>>>   get_compiler(conf, cp, variant),
>>>   arch,
>>> -    arch_family,
>>> +    "family/" + arch_family,
>>     "bsps/" + arch_family,
>>
>> ... as discussed in the other thread? If you are happy with the change as 
>> shown
>> please push.
> 
> Yes, this is good and matches with our directory layout. I checked it in with
> this change.
> 
> We could also merge the default-by-family and default-by-variant lists with 
> this
> approach, 

I am not sure. My initial reaction was "yes" but how would different settings
for a BSP and a family be handled? I am assuming a BSP variant setting is able
to override a family setting. Is that possible if they are merged?

> for example:
> 
> diff --git a/spec/build/bsps/optconsolebaud.yml
> b/spec/build/bsps/optconsolebaud.yml
> index 4b0869beca..0233fdd61b 100644
> --- a/spec/build/bsps/optconsolebaud.yml
> +++ b/spec/build/bsps/optconsolebaud.yml
> @@ -6,13 +6,10 @@ build-type: option
>  copyrights:
>  - Copyright (C) 2020 embedded brains GmbH (http://www.embedded-brains.de)
>  default: 115200
> -default-by-family:
> -- value: 9600
> -  families:
> -  - powerpc/motorola_powerpc
>  default-by-variant:
>  - value: 9600
>    variants:
> +  - bsps/powerpc/motorola_powerpc

Oh I think my patch piece may have been wrong. This has two `/` and so three
components. We _must_ have unique family names and so the arch part is not
really needed. My concern is this type code ...

https://git.rtems.org/rtems_waf/tree/rtems.py#n758

that breaks. Is this an issue? I think a single `/` in a BSP or family is 
cleaner.

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

Re: [PATCH] build: Add "family/" prefix to BSP familiy enable

2021-07-21 Thread Chris Johns
On 22/7/21 5:22 am, Sebastian Huber wrote:
> BSP family and BSP variant names may be equal.  This prefix avoids
> ambiguity in the enabled-by expressions.
> ---
>  wscript | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/wscript b/wscript
> index f27dba6831..b7a0412150 100755
> --- a/wscript
> +++ b/wscript
> @@ -1394,7 +1394,7 @@ def configure_variant(conf, cp, bsp_map, path_list, 
> top_group, variant):
>  conf.env["ENABLE"] = [
>  get_compiler(conf, cp, variant),
>  arch,
> -arch_family,
> +"family/" + arch_family,

   "bsps/" + arch_family,

... as discussed in the other thread? If you are happy with the change as shown
please push.

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


Re: [PATCH] linker_set.h: Add alignof implementation for when not C11 or C++11

2021-07-21 Thread Chris Johns
On 22/7/21 7:55 am, Joel Sherrill wrote:
> On Wed, Jul 21, 2021, 4:31 PM Gedare Bloom  > wrote:
> 
> This seems alright to me. At least, it should get some complaints
> quickly if it doesn't work :)
> 
> The old version should have gotten complaints but didn't. Probably indicates
> people are not being careful about specifying the language version they want 
> to
> use and just taking the GCC default which periodically changes. 

Is this a problem on 5-freebsd-12?

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


Re: [PATCH] build: Add the BSP family to the enable set

2021-07-21 Thread Chris Johns
On 22/7/21 8:55 am, Joel Sherrill wrote:
> On Wed, Jul 21, 2021, 5:49 PM Chris Johns  <mailto:chr...@rtems.org>> wrote:
> 
> On 22/7/21 8:35 am, Joel Sherrill wrote:
> > On Wed, Jul 21, 2021 at 2:10 PM Sebastian Huber
> >  <mailto:sebastian.hu...@embedded-brains.de>> wrote:
> >>
> >> On 21/07/2021 21:05, Gedare Bloom wrote:
> >>>>> The problem is that one BSP which supports SMP "riscv/griscv" is
> identical to
> >>>>> the family "riscv/griscv" which contains BSPs which do not support 
> SMP
> >>>>> ("riscv/grv32i" and riscv/grv32im").
> >>>> Hmm, tricky. Can the BSPs that do not support SMP disable SMP in the 
> BSP
> >>>> specific config, ie override/specialise in the BSP?
> >>>>
> >>> Or, can we avoid having duplication between BSP names and family 
> names?
> >>
> >> Yes, good idea. We could use a "family/" prefix for example
> >> ("family//").
> 
> Great idea.
> 
> > Ideally we would never have a family and BSP variant have the same name.
> > But that rule is violated multiple times now.
> 
> Yeap.
> 
> > I am not sure where your triple is intended to be used but we have used
> > the pattern arch/BSP which is really / as the
> > full name of BSPs.
> >
> > If we want to step that further, we could do something similar to the
> > GNU target triple where the middle component is a rarely used vendor.
> > //.
> 
> I think we are to late for this because the spec file format follows what 
> I did
> in the eco-system and I would prefer we maintained a single unified way 
> of doing
> this than changing it.
> 
> > In recent history, there was an issue of BSP variant names needing to
> > be unique across all architectures. This may also need to apply to bsp
> > family names.
> 
> What about "bsps/". This means you have:
> 
> powerpc/mvme2307
> bsps/motorola_powerpc
> 
> Is the first line is our BSP formal naming pattern and the second is how
> families are named, that should be ok.

It is as you state. In a formal sense it is:

BSP: /
BSP FAMILY:  bsps

This format and approach does not break existing eco-system code that is set up
to parse a single `/` into two parts, the arch and bsp. A check of the arch
component for `bsps` can raise an error, ie unknown arch, or be extended to
handle a BSP family.

> Rules for uniqueness apply.

Yes.

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


Re: [PATCH] bsp: Remove fatal from exit(0). Add extended heap error output

2021-07-21 Thread Chris Johns
On 22/7/21 8:45 am, Joel Sherrill wrote:
> On Wed, Jul 21, 2021 at 2:04 AM Chris Johns  wrote:
>>
>> On 21/7/21 3:37 pm, Sebastian Huber wrote:
>>> Hello Chris,
>>>
>>> thanks, this is a nice improvement.
>>
>> It helps find the corruption with a watch point.
>>
>>> On 21/07/2021 07:17, chr...@rtems.org wrote:
>>>> --- a/bsps/shared/start/bspfatal-default.c
>>>> +++ b/bsps/shared/start/bspfatal-default.c
>>>> @@ -22,15 +22,24 @@ void bsp_fatal_extension(
>>>>   {
>>>> #if BSP_VERBOSE_FATAL_EXTENSION
>>>>   Thread_Control *executing;
>>>> +const char* TYPE = "*** FATAL ***\n";
>>>> +const char* type = "fatal";
>>>> +
>>>> +if ( source == RTEMS_FATAL_SOURCE_EXIT ) {
>>>> +  TYPE = "";
>>>
>>> It would be good to still have a unique pattern which starts the fatal 
>>> extension
>>> message.  The "***" is also used for the test begin/end marker. What about 
>>> "***
>>> SYSTEM TERMINATED ***"?
>>
>> I considered this but I was concerned about the difference not being clear.
>> Maybe I overstepped. The patch gives us:
>>
>> exit source: 5 (RTEMS_FATAL_SOURCE_EXIT)
>> exit code: 0 (0x)
>> RTEMS version: 6.0.0.87609bacd323eef1f3f4d6620ce42e7e5309ad10
>> RTEMS tools: 10.3.1 20210409 (RTEMS 6, RSB
>> 4e6dc6431435821a534da6307e72ecbd7e42b82a, Newlib 0c0f3df)
>> executing thread ID: 0x08a010001
>> executing thread name: UI1
>>
>> and with the banner text:
>>
>> *** EXIT ***
>> exit source: 5 (RTEMS_FATAL_SOURCE_EXIT)
>> exit code: 0 (0x)
>> RTEMS version: 6.0.0.87609bacd323eef1f3f4d6620ce42e7e5309ad10
>> RTEMS tools: 10.3.1 20210409 (RTEMS 6, RSB
>> 4e6dc6431435821a534da6307e72ecbd7e42b82a, Newlib 0c0f3df)
>> executing thread ID: 0x08a010001
>> executing thread name: UI1
>>
>> It is easy to bend any direction here so maybe we wait for Joel to comment
>> before I make a v2 patch.
> 
> This still looks scary when it is a normal exit(0) which is a big
> issue with the current output. If exit source == 5 and exit code == 0,
> can those two lines be a friendly, feel good message instead of something
> you have to understand to know it was a normal exit.
> 
> When you run a test, will we end up with something like this for a normal
> exit?
> 
> *** END OF TEST ...
> *** EXIT ***
> 
> And just "*** EXIT ***" when there is no END OF TEST message?

The end of test and the fatal message are no related so I am not sure how you
could do this. You only the values passed to the fatal error extension handler.

> How about something as simple as NORMAL or ABNORMAL EXIT in your
> new line? And if normal, drop the exit source and code lines.
> 
> I still have flashbacks to the first time I saw this output every time I
> show someone a test running. It took me a few minutes to realize
> it was OK. And it still often takes a second glance to say "all ok."
> If it can be easier to see "good," that would be a user facing improvement IMO

Yes it trips me up often as I search for the code to see if it is a real fatal
error and I also miss some real errors. How about:

RTEMS Shutdown
RTEMS version: 6.0.0.87609bacd323eef1f3f4d6620ce42e7e5309ad10
RTEMS tools: 10.3.1 20210409 (RTEMS 6, RSB
4e6dc6431435821a534da6307e72ecbd7e42b82a, Newlib 0c0f3df)
executing thread ID: 0x08a010001
executing thread name: UI1

and if `exit(1)` is called:

*** EXIT ERROR ***
exit code: 1 (0x0001)
RTEMS version: 6.0.0.87609bacd323eef1f3f4d6620ce42e7e5309ad10
RTEMS tools: 10.3.1 20210409 (RTEMS 6, RSB
4e6dc6431435821a534da6307e72ecbd7e42b82a, Newlib 0c0f3df)
executing thread ID: 0x08a010001
executing thread name: UI1

?

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


Re: [PATCH] build: Add the BSP family to the enable set

2021-07-21 Thread Chris Johns
On 22/7/21 8:35 am, Joel Sherrill wrote:
> On Wed, Jul 21, 2021 at 2:10 PM Sebastian Huber
>  wrote:
>>
>> On 21/07/2021 21:05, Gedare Bloom wrote:
> The problem is that one BSP which supports SMP "riscv/griscv" is 
> identical to
> the family "riscv/griscv" which contains BSPs which do not support SMP
> ("riscv/grv32i" and riscv/grv32im").
 Hmm, tricky. Can the BSPs that do not support SMP disable SMP in the BSP
 specific config, ie override/specialise in the BSP?

>>> Or, can we avoid having duplication between BSP names and family names?
>>
>> Yes, good idea. We could use a "family/" prefix for example
>> ("family//").

Great idea.

> Ideally we would never have a family and BSP variant have the same name.
> But that rule is violated multiple times now.

Yeap.

> I am not sure where your triple is intended to be used but we have used
> the pattern arch/BSP which is really / as the
> full name of BSPs.
> 
> If we want to step that further, we could do something similar to the
> GNU target triple where the middle component is a rarely used vendor.
> //.

I think we are to late for this because the spec file format follows what I did
in the eco-system and I would prefer we maintained a single unified way of doing
this than changing it.

> In recent history, there was an issue of BSP variant names needing to
> be unique across all architectures. This may also need to apply to bsp
> family names.

What about "bsps/". This means you have:

powerpc/mvme2307
bsps/motorola_powerpc

It is simple and clean.

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


Re: [PATCH rtems] bsps/imxrt: Add attribution in file headers

2021-07-21 Thread Chris Johns
On 21/7/21 5:34 pm, Christian MAUDERER wrote:
> Hello Chris,
> 
> Am 21.07.21 um 09:22 schrieb Chris Johns:
>> On 21/7/21 5:05 pm, Christian MAUDERER wrote:
>>> Hello,
>>>
>>> I don't object to clear rules. At the moment it's a bit of a mix.
>>
>> Yes I understand and I think the line you posted in the patch is fine as is, 
>> it
>> just needs to be separate from the license text because it could taint it 
>> and we
>> need to avoid that.
>>
>>> Some examples
>>> of what I have found (mainly in PowerPC):
>>>
>>> Above the copyright line:
>>>
>>> https://git.rtems.org/rtems/tree/bsps/powerpc/ss555/start/vectors.S
>>> https://git.rtems.org/rtems/tree/bsps/powerpc/ss555/start/vectors_init.c
>>> https://git.rtems.org/rtems/tree/bsps/powerpc/ss555/start/irq.c
>>> https://git.rtems.org/rtems/tree/bsps/powerpc/include/libcpu/irq.h
>>
>> Yes there are some legacy comments. I would not touch those. We are more
>> informed on how we handle these things these days.
>>
>>> Mixed in the header somewhere:
>>>
>>> https://git.rtems.org/rtems/tree/bsps/powerpc/ss555/start/bspstart.c
>>>
>>> Am 21.07.21 um 07:51 schrieb Sebastian Huber:
>>>> On 21/07/2021 02:46, Chris Johns wrote:
>>>>> On 21/7/21 6:47 am, Gedare Bloom wrote:
>>>>>> This seems fine to me. We don't have any standard way to document this
>>>>>> kind of attribution. However, we have had individual authors add their
>>>>>> names below their company's copyright. It might make sense to put the
>>>>>> sponsorship part beneath the copyright of the sponsored
>>>>>> company/person?
>>>>>
>>>>> Make sense. The copyright block should be limited to just copyrights and
>>>>> sponsor
>>>>> acknowledgements should in a separate block, maybe after the license 
>>>>> block.
>>>>
>>>> Yes, I also think it should be outside the license comment block.
>>>>
>>>>>
>>>>>> Not a big deal, but it might help for some kind of
>>>>>> consistent guidance if we do this more in the future.
>>>>>
>>>>> I think we need some guidelines. I do not agree with URL links, email
>>>>> addresses,
>>>>> phone numbers or street addreses appearing in the source. I also think a
>>>>> sponsor
>>>>> acknowledgement is never updated or changed even if a company changes 
>>>>> name.
>>>>> What
>>>>> I am not sure about is the areas of the source we allow this to happen 
>>>>> in, ie
>>>>> score ...?
>>>>
>>>> We could add some text here:
>>>>
>>>> https://docs.rtems.org/branches/master/eng/coding-file-hdr.html
>>>>
>>>
>>> Without discussing it with Onto: An alternative to attribution in the code 
>>> could
>>> be an attribution page in the manuals where we can collect such notes. That
>>> would avoid scattering them throughout the code. It would also make it 
>>> easier to
>>> (for example) attribute to supporters that maybe don't fund coding but other
>>> activities like infrastructure.
>>
>> Sorry, collecting in a manual is a step to far and I would not support it. 
>> Some
>> companies have tight marketing requirements and trademarks for placement of
>> their company details in manuals or printed material and we need to keep well
>> clear of this. There are sponsors of RTEMS who require nothing is said, ever.
> 
> Yes. Most companies don't want to be mentioned. I was a bit surprised that 
> Onto
> asked for it ;-)

I am OK with this.

>> There is a legal aspect to this that I am not sure about yet. What happens 
>> if a
>> company complains about the addition?
> 
> I would only add companies that ask for it. I think most paid projects have 
> some
> kind of contract where that could be clarified right from the beginning.

You know this but I do not. We need a simple way to handle this.

> For
> sponsors without a project, it could be done with some kind of "yes, I want to
> be added" field on a donation page. I assume that's how other projects handle
> that kind of stuff.

Yes we need a donate page but that is a separate topic.

> 
> One example for a project with a big supporters list:
> 
> https://metabrainz.org/supporters
> 
> The just added a "I would like the donation to be anonymous" to the donate 
> page:
> 
> https://metabrainz.org/donate
> 

Yes.

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


Re: [PATCH rtems] bsps/imxrt: Add attribution in file headers

2021-07-21 Thread Chris Johns
On 21/7/21 5:05 pm, Christian MAUDERER wrote:
> Hello,
> 
> I don't object to clear rules. At the moment it's a bit of a mix. 

Yes I understand and I think the line you posted in the patch is fine as is, it
just needs to be separate from the license text because it could taint it and we
need to avoid that.

> Some examples
> of what I have found (mainly in PowerPC):
> 
> Above the copyright line:
> 
> https://git.rtems.org/rtems/tree/bsps/powerpc/ss555/start/vectors.S
> https://git.rtems.org/rtems/tree/bsps/powerpc/ss555/start/vectors_init.c
> https://git.rtems.org/rtems/tree/bsps/powerpc/ss555/start/irq.c
> https://git.rtems.org/rtems/tree/bsps/powerpc/include/libcpu/irq.h

Yes there are some legacy comments. I would not touch those. We are more
informed on how we handle these things these days.

> Mixed in the header somewhere:
> 
> https://git.rtems.org/rtems/tree/bsps/powerpc/ss555/start/bspstart.c
> 
> Am 21.07.21 um 07:51 schrieb Sebastian Huber:
>> On 21/07/2021 02:46, Chris Johns wrote:
>>> On 21/7/21 6:47 am, Gedare Bloom wrote:
>>>> This seems fine to me. We don't have any standard way to document this
>>>> kind of attribution. However, we have had individual authors add their
>>>> names below their company's copyright. It might make sense to put the
>>>> sponsorship part beneath the copyright of the sponsored
>>>> company/person?
>>>
>>> Make sense. The copyright block should be limited to just copyrights and 
>>> sponsor
>>> acknowledgements should in a separate block, maybe after the license block.
>>
>> Yes, I also think it should be outside the license comment block.
>>
>>>
>>>> Not a big deal, but it might help for some kind of
>>>> consistent guidance if we do this more in the future.
>>>
>>> I think we need some guidelines. I do not agree with URL links, email 
>>> addresses,
>>> phone numbers or street addreses appearing in the source. I also think a 
>>> sponsor
>>> acknowledgement is never updated or changed even if a company changes name. 
>>> What
>>> I am not sure about is the areas of the source we allow this to happen in, 
>>> ie
>>> score ...?
>>
>> We could add some text here:
>>
>> https://docs.rtems.org/branches/master/eng/coding-file-hdr.html
>>
> 
> Without discussing it with Onto: An alternative to attribution in the code 
> could
> be an attribution page in the manuals where we can collect such notes. That
> would avoid scattering them throughout the code. It would also make it easier 
> to
> (for example) attribute to supporters that maybe don't fund coding but other
> activities like infrastructure.

Sorry, collecting in a manual is a step to far and I would not support it. Some
companies have tight marketing requirements and trademarks for placement of
their company details in manuals or printed material and we need to keep well
clear of this. There are sponsors of RTEMS who require nothing is said, ever.

There is a legal aspect to this that I am not sure about yet. What happens if a
company complains about the addition?

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


Re: [PATCH] bsp: Remove fatal from exit(0). Add extended heap error output

2021-07-21 Thread Chris Johns
On 21/7/21 3:37 pm, Sebastian Huber wrote:
> Hello Chris,
> 
> thanks, this is a nice improvement.

It helps find the corruption with a watch point.

> On 21/07/2021 07:17, chr...@rtems.org wrote:
>> --- a/bsps/shared/start/bspfatal-default.c
>> +++ b/bsps/shared/start/bspfatal-default.c
>> @@ -22,15 +22,24 @@ void bsp_fatal_extension(
>>   {
>>     #if BSP_VERBOSE_FATAL_EXTENSION
>>   Thread_Control *executing;
>> +    const char* TYPE = "*** FATAL ***\n";
>> +    const char* type = "fatal";
>> +
>> +    if ( source == RTEMS_FATAL_SOURCE_EXIT ) {
>> +  TYPE = "";
> 
> It would be good to still have a unique pattern which starts the fatal 
> extension
> message.  The "***" is also used for the test begin/end marker. What about 
> "***
> SYSTEM TERMINATED ***"?

I considered this but I was concerned about the difference not being clear.
Maybe I overstepped. The patch gives us:

exit source: 5 (RTEMS_FATAL_SOURCE_EXIT)
exit code: 0 (0x)
RTEMS version: 6.0.0.87609bacd323eef1f3f4d6620ce42e7e5309ad10
RTEMS tools: 10.3.1 20210409 (RTEMS 6, RSB
4e6dc6431435821a534da6307e72ecbd7e42b82a, Newlib 0c0f3df)
executing thread ID: 0x08a010001
executing thread name: UI1

and with the banner text:

*** EXIT ***
exit source: 5 (RTEMS_FATAL_SOURCE_EXIT)
exit code: 0 (0x)
RTEMS version: 6.0.0.87609bacd323eef1f3f4d6620ce42e7e5309ad10
RTEMS tools: 10.3.1 20210409 (RTEMS 6, RSB
4e6dc6431435821a534da6307e72ecbd7e42b82a, Newlib 0c0f3df)
executing thread ID: 0x08a010001
executing thread name: UI1

It is easy to bend any direction here so maybe we wait for Joel to comment
before I make a v2 patch.

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

Re: [PATCH rtems] bsps/imxrt: Add attribution in file headers

2021-07-21 Thread Chris Johns
On 21/7/21 3:51 pm, Sebastian Huber wrote:
> On 21/07/2021 02:46, Chris Johns wrote:
>> I think we need some guidelines. I do not agree with URL links, email 
>> addresses,
>> phone numbers or street addreses appearing in the source. I also think a 
>> sponsor
>> acknowledgement is never updated or changed even if a company changes name. 
>> What
>> I am not sure about is the areas of the source we allow this to happen in, ie
>> score ...?
> 
> We could add some text here:
> 
> https://docs.rtems.org/branches/master/eng/coding-file-hdr.html
> 

Yes. I will consider this and post an initial approach we can discuss in detail.

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


Re: [PATCH v2 1/5] rtems-utils.h: Create ostream_guard

2021-07-20 Thread Chris Johns
Close, some minor formatting issues 

On 21/7/21 1:06 am, Ryan Long wrote:
> ---
>  rtemstoolkit/rtems-utils.h | 18 ++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/rtemstoolkit/rtems-utils.h b/rtemstoolkit/rtems-utils.h
> index 4ce9c68..c4a262e 100644
> --- a/rtemstoolkit/rtems-utils.h
> +++ b/rtemstoolkit/rtems-utils.h
> @@ -47,6 +47,24 @@ namespace rtems
> boolreal = false,
> size_t  line_length = 16,
> uint32_toffset = 0);
> +
> +/*
> + * Save and restore the output stream's settings.
> + */
> +struct ostream_guard {
> +  std::ostream&   o;
> +  std::ios_base::fmtflags flags;
> +
> +  ostream_guard ( std::ostream& o_ ) : o( o_ ), flags( o_.flags() )

The syntax I use and posted is:

   ostream_guard (std::ostream& o_)
 : o (o_),
   flags (o_.flags ())
   {
   }

First the spaces are what the toolkit uses, there are no extra spaces but the
initialiser list is the important piece so I will explain why it is the way it 
is.

The way I have written the code lets you make changes to a struct (or class)
without impacting a single line. This struct is pretty simple but more complex
structs can have many initialisers and they need to be in order in the
initialiser list and if you add or move members the order needs to change. Being
able to visually inspect the struct and the list is important. Considers these
constructors 

https://git.rtems.org/rtems-tools/tree/rtemstoolkit/rld-symbols.cpp#n72

> +  {
> +  }
> +
> +  ~ostream_guard ()
> +  {
> +  o.flags( flags );

2 spaces please.

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


Re: [PATCH] build: Add the BSP family to the enable set

2021-07-20 Thread Chris Johns


On 20/7/21 8:25 pm, Sebastian Huber wrote:
> On 15/07/2021 08:19, Sebastian Huber wrote:
>> This makes it possible to use the BSP family in expressions of the enabled-by
>> attribute.
>> ---
>>   wscript | 7 ++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/wscript b/wscript
>> index 487951fdba..1331ae149e 100755
>> --- a/wscript
>> +++ b/wscript
>> @@ -1387,7 +1387,12 @@ def configure_variant(conf, cp, bsp_map, path_list,
>> top_group, variant):
>>     # For the enabled-by evaluation we have to use the base BSP defined 
>> by
>> the
>>   # build specification and not the BSP name provided by the user.
>> -    conf.env["ENABLE"] = [get_compiler(conf, cp, variant), arch, arch_bsp]
>> +    conf.env["ENABLE"] = [
>> +    get_compiler(conf, cp, variant),
>> +    arch,
>> +    arch_family,
>> +    arch_bsp,
>> +    ]
>>     conf.env["TOP"] = conf.path.abspath()
>>   conf.env["TOPGROUP"] = top_group
> 
> This actually broke some riscv BSPs which do not support SMP. We have
> 
> cat spec/build/cpukit/optsmp.yml
> SPDX-License-Identifier: CC-BY-SA-4.0 OR BSD-2-Clause
> actions:
> - get-boolean: null
> - env-enable: null
> - define-condition: null
> build-type: option
> copyrights:
> - Copyright (C) 2020 embedded brains GmbH (http://www.embedded-brains.de)
> default: false
> default-by-family: []
> default-by-variant: []
> description: |
>   Enable the Symmetric Multiprocessing (SMP) support
> enabled-by:
> - arm/altcycv_devkit
> - arm/fvp_cortex_r52
> - arm/imx7
> - arm/raspberrypi2
> - arm/realview_pbx_a9_qemu
> - arm/xilinx_zynq_a9_qemu
> - arm/xilinx_zynqmp_ultra96
> - arm/xilinx_zynq_zc702
> - arm/xilinx_zynq_zc706
> - arm/xilinx_zynq_zedboard
> - powerpc/qoriq_e500
> - powerpc/qoriq_e6500_32
> - powerpc/qoriq_e6500_64
> - riscv/griscv
> - riscv/grv32imac
> - riscv/grv32imafdc
> - riscv/rv32iac
> - riscv/rv32imac
> - riscv/rv32imafc
> - riscv/rv32imafd
> - riscv/rv32imafdc
> - riscv/rv64imac
> - riscv/rv64imac_medany
> - riscv/rv64imafd
> - riscv/rv64imafdc
> - riscv/rv64imafdc_medany
> - riscv/rv64imafd_medany
> - sparc/erc32
> - sparc/gr712rc
> - sparc/gr740
> - sparc/leon3
> links: []
> name: RTEMS_SMP
> type: build
> 
> The problem is that one BSP which supports SMP "riscv/griscv" is identical to
> the family "riscv/griscv" which contains BSPs which do not support SMP
> ("riscv/grv32i" and riscv/grv32im").

Hmm, tricky. Can the BSPs that do not support SMP disable SMP in the BSP
specific config, ie override/specialise in the BSP?

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

Re: [PATCH rtems] bsps/imxrt: Add attribution in file headers

2021-07-20 Thread Chris Johns
On 21/7/21 6:47 am, Gedare Bloom wrote:
> This seems fine to me. We don't have any standard way to document this
> kind of attribution. However, we have had individual authors add their
> names below their company's copyright. It might make sense to put the
> sponsorship part beneath the copyright of the sponsored
> company/person? 

Make sense. The copyright block should be limited to just copyrights and sponsor
acknowledgements should in a separate block, maybe after the license block.

> Not a big deal, but it might help for some kind of
> consistent guidance if we do this more in the future.

I think we need some guidelines. I do not agree with URL links, email addresses,
phone numbers or street addreses appearing in the source. I also think a sponsor
acknowledgement is never updated or changed even if a company changes name. What
I am not sure about is the areas of the source we allow this to happen in, ie
score ...?

Chris

> 
> On Tue, Jul 20, 2021 at 8:20 AM Christian Mauderer
>  wrote:
>>
>> Onto Innovations Incorporated originally sponsored the development of
>> this BSP. This patch adds the attribution for it.
>>
>> The patch also fixes an old license header in bspstarthooks.c that was
>> accidentally copied into that file.
>> ---
>>  bsps/arm/imxrt/console/console.c  |  1 +
>>  bsps/arm/imxrt/dts/imxrt1050-evkb.dts |  2 ++
>>  bsps/arm/imxrt/i2c/imxrt-lpi2c.c  |  1 +
>>  bsps/arm/imxrt/include/bsp.h  |  1 +
>>  bsps/arm/imxrt/include/bsp/flash-headers.h|  1 +
>>  bsps/arm/imxrt/include/bsp/irq.h  |  1 +
>>  bsps/arm/imxrt/include/fsl_device_registers.h |  1 +
>>  .../imxrt/include/imxrt/imxrt1050-pinfunc.h   |  1 +
>>  bsps/arm/imxrt/include/imxrt/imxrt1050.dtsi   |  2 ++
>>  bsps/arm/imxrt/include/imxrt/lpspi.h  |  1 +
>>  bsps/arm/imxrt/include/imxrt/memory.h |  1 +
>>  bsps/arm/imxrt/include/imxrt/mpu-config.h |  1 +
>>  bsps/arm/imxrt/spi/imxrt-lpspi.c  |  1 +
>>  bsps/arm/imxrt/start/bspstart.c   |  1 +
>>  bsps/arm/imxrt/start/bspstarthooks.c  | 32 +--
>>  bsps/arm/imxrt/start/clock-arm-pll-config.c   |  1 +
>>  bsps/arm/imxrt/start/flash-boot-data.c|  1 +
>>  bsps/arm/imxrt/start/flash-flexspi-config.c   |  1 +
>>  bsps/arm/imxrt/start/flash-ivt.c  |  1 +
>>  bsps/arm/imxrt/start/imxrt-ffec-init.c|  1 +
>>  bsps/arm/imxrt/start/mpu-config.c |  1 +
>>  21 files changed, 45 insertions(+), 9 deletions(-)
>>
>> diff --git a/bsps/arm/imxrt/console/console.c 
>> b/bsps/arm/imxrt/console/console.c
>> index 05320f2c4c..ab278b9d22 100644
>> --- a/bsps/arm/imxrt/console/console.c
>> +++ b/bsps/arm/imxrt/console/console.c
>> @@ -1,6 +1,7 @@
>>  /* SPDX-License-Identifier: BSD-2-Clause */
>>
>>  /*
>> + * i.MX RT port sponsored by Onto Innovation Incorporated
>>   * Copyright (C) 2020 embedded brains GmbH (http://www.embedded-brains.de)
>>   *
>>   * Redistribution and use in source and binary forms, with or without
>> diff --git a/bsps/arm/imxrt/dts/imxrt1050-evkb.dts 
>> b/bsps/arm/imxrt/dts/imxrt1050-evkb.dts
>> index 9310371428..bf24762d86 100644
>> --- a/bsps/arm/imxrt/dts/imxrt1050-evkb.dts
>> +++ b/bsps/arm/imxrt/dts/imxrt1050-evkb.dts
>> @@ -1,7 +1,9 @@
>>  /* SPDX-License-Identifier: BSD-2-Clause */
>>
>>  /*
>> + * i.MX RT port sponsored by Onto Innovation Incorporated
>>   * Copyright (C) 2020 embedded brains GmbH (http://www.embedded-brains.de)
>> + *
>>   * Redistribution and use in source and binary forms, with or without
>>   * modification, are permitted provided that the following conditions
>>   * are met:
>> diff --git a/bsps/arm/imxrt/i2c/imxrt-lpi2c.c 
>> b/bsps/arm/imxrt/i2c/imxrt-lpi2c.c
>> index 783c6e18e6..0873499a24 100644
>> --- a/bsps/arm/imxrt/i2c/imxrt-lpi2c.c
>> +++ b/bsps/arm/imxrt/i2c/imxrt-lpi2c.c
>> @@ -1,6 +1,7 @@
>>  /* SPDX-License-Identifier: BSD-2-Clause */
>>
>>  /*
>> + * i.MX RT port sponsored by Onto Innovation Incorporated
>>   * Copyright (C) 2020 embedded brains GmbH (http://www.embedded-brains.de)
>>   *
>>   * Redistribution and use in source and binary forms, with or without
>> diff --git a/bsps/arm/imxrt/include/bsp.h b/bsps/arm/imxrt/include/bsp.h
>> index f93c28aee0..78fa6a42e1 100644
>> --- a/bsps/arm/imxrt/include/bsp.h
>> +++ b/bsps/arm/imxrt/include/bsp.h
>> @@ -7,6 +7,7 @@
>>   */
>>
>>  /*
>> + * i.MX RT port sponsored by Onto Innovation Incorporated
>>   * Copyright (C) 2020 embedded brains GmbH (http://www.embedded-brains.de)
>>   *
>>   * Redistribution and use in source and binary forms, with or without
>> diff --git a/bsps/arm/imxrt/include/bsp/flash-headers.h 
>> b/bsps/arm/imxrt/include/bsp/flash-headers.h
>> index 6575b1c21b..a50ae68d58 100644
>> --- a/bsps/arm/imxrt/include/bsp/flash-headers.h
>> +++ b/bsps/arm/imxrt/include/bsp/flash-headers.h
>> @@ -9,6 +9,7 @@
>>   */
>>
>>  /*
>> + * i.MX RT port sponsored by Onto Innovation Incorporated
>>   * Copyright (C) 

Re: [PATCH 1/4] rtems-exeinfo.cpp: Restore ostream format

2021-07-19 Thread Chris Johns
On 20/7/21 3:13 am, Ryan Long wrote:
> CID 1503006: Not restoring ostream format
> CID 1503007: Not restoring ostream format
> 
> Used a variable to store the format of the ostream before any changes,
> and copied what was originally there back into the stream before
> returning from the function.
> 
> Closes #4469
> ---
>  linkers/rtems-exeinfo.cpp | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/linkers/rtems-exeinfo.cpp b/linkers/rtems-exeinfo.cpp
> index 6e92206..bc1ad45 100644
> --- a/linkers/rtems-exeinfo.cpp
> +++ b/linkers/rtems-exeinfo.cpp
> @@ -31,6 +31,7 @@
>  #endif
>  
>  #include 
> +#include 
>  #include 
>  
>  #include 
> @@ -366,6 +367,8 @@ namespace rld
> */
>  
>rld::strings all_flags;
> +  std::ofstream oldState;

I suggest adding the following guard to rtems-utils.h and then use that. A guard
is safe in exceptions ...

/*
 * Save and restore the output stream's settings.
 */
struct ostream_guard {
std::ostream& o;
std::ios_base::fmtflags flags;
ostream_guard (std::ostream& o_)
  : o(o_),
flags(o_.flags ()) {
}
~ostream_guard () {
o.flags (flags);
}
};

> +  oldState.copyfmt( std::cout );

Note, there is no camelcase in the code so can we please not start now. Covoar
is the only place it exists and that is a contained place. :)

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


Re: [PATCH] AddressToLineMapper.h: Remove pointer to temporary string

2021-07-18 Thread Chris Johns
Hi Alex,

This looks good and can be pushed. Thank you.

Chris

On 17/7/21 4:03 am, Alex White wrote:
> CID 1505281: Pointer to local outside scope
> 
> Closes #4473
> ---
>  tester/covoar/AddressToLineMapper.cc | 11 ---
>  tester/covoar/AddressToLineMapper.h  | 21 -
>  2 files changed, 24 insertions(+), 8 deletions(-)
> 
> diff --git a/tester/covoar/AddressToLineMapper.cc 
> b/tester/covoar/AddressToLineMapper.cc
> index c305e3b..838b156 100644
> --- a/tester/covoar/AddressToLineMapper.cc
> +++ b/tester/covoar/AddressToLineMapper.cc
> @@ -19,9 +19,13 @@ namespace Coverage {
>  return is_end_sequence;
>}
>  
> -  const std::string& SourceLine::path() const
> +  const std::string SourceLine::path() const
>{
> -return path_;
> +if (!path_) {
> +  return "unknown";
> +} else {
> +  return *path_;
> +}
>}
>  
>int SourceLine::line() const
> @@ -31,7 +35,8 @@ namespace Coverage {
>  
>void AddressLineRange::addSourceLine(const rld::dwarf::address& address)
>{
> -auto insertResult = sourcePaths.insert(address.path());
> +auto insertResult = sourcePaths.insert(
> +  std::make_shared(address.path()));
>  
>  sourceLines.emplace_back(
>SourceLine (
> diff --git a/tester/covoar/AddressToLineMapper.h 
> b/tester/covoar/AddressToLineMapper.h
> index 88bf475..308925a 100644
> --- a/tester/covoar/AddressToLineMapper.h
> +++ b/tester/covoar/AddressToLineMapper.h
> @@ -8,6 +8,7 @@
>  #define __ADDRESS_TO_LINE_MAPPER_H__
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -26,7 +27,7 @@ namespace Coverage {
>  
>  SourceLine()
>  : address(0),
> -  path_("unknown"),
> +  path_(nullptr),
>line_num(-1),
>is_end_sequence(true)
>  {
> @@ -34,7 +35,7 @@ namespace Coverage {
>  
>  SourceLine(
>uint64_t addr,
> -  const std::string& src,
> +  const std::shared_ptr& src,
>int line,
>bool end_sequence
>  ) : address(addr),
> @@ -64,7 +65,7 @@ namespace Coverage {
>   *
>   *  @return Returns the source file path of this address
>   */
> -const std::string& path() const;
> +const std::string path() const;
>  
>  /*!
>   *  This method gets the source line number of this address.
> @@ -84,7 +85,7 @@ namespace Coverage {
>   *  An iterator pointing to the location in the set that contains the
>   *  source file path of the address.
>   */
> -const std::string& path_;
> +const std::shared_ptr path_;
>  
>  /*!
>   *  The source line number of the address.
> @@ -100,7 +101,17 @@ namespace Coverage {
>  
>typedef std::vector SourceLines;
>  
> -  typedef std::set SourcePaths;
> +  /* This allows comparison of strings owned by shared_ptrs. */
> +  struct SharedStringCmp {
> +bool operator()(
> +  const std::shared_ptr& lhs,
> +  const std::shared_ptr& rhs
> +) const {
> +  return *lhs < *rhs;
> +}
> +  };
> +
> +  typedef std::set, SharedStringCmp> 
> SourcePaths;
>  
>/*! @class AddressLineRange
> *
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: [PATCH v2] setbuilder minor list-host option

2021-07-16 Thread Chris Johns
Pushed and thanks

Chris

On 16/7/21 10:15 pm, Robin Mueller wrote:
> This adds a way to print the host triplet
> Can be useful for cross-compiling toolchains
> ---
> Change from v1 to v2: Removed "Displaying" in printout on first line.
> 
>  source-builder/sb/setbuilder.py | 14 +-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/source-builder/sb/setbuilder.py b/source-builder/sb/setbuilder.py
> index c8c8fee..63392fe 100644
> --- a/source-builder/sb/setbuilder.py
> +++ b/source-builder/sb/setbuilder.py
> @@ -673,6 +673,16 @@ def list_bset_cfg_files(opts, configs):
>  return True
>  return False
>  
> +def list_host(opts):
> +if opts.get_arg('--list-host'):
> +print('Host operating system information:')
> +print('Operating system: %s' % macro_expand(opts.defaults, '%{_os}'))
> +print('Number of processors: %s' % macro_expand(opts.defaults, 
> '%{_ncpus}'))
> +print('Build architecture: %s' % macro_expand(opts.defaults, 
> '%{_host_arch}'))
> +print('Host triplet: %s' % macro_expand(opts.defaults, '%{_host}'))
> +return True
> +return False
> +
>  def run():
>  import sys
>  ec = 0
> @@ -683,6 +693,7 @@ def run():
>  '--list-bsets':'List available build sets',
>  '--list-configs':  'List available configuration files.',
>  '--list-deps': 'List the dependent files.',
> +'--list-host': 'List host information and the host 
> triplet.',
>  '--bset-tar-file': 'Create a build set tar file',
>  '--pkg-tar-files': 'Create package tar files',
>  '--no-report': 'Do not create a package report.',
> @@ -720,7 +731,8 @@ def run():
>  deps = []
>  else:
>  deps = None
> -if not list_bset_cfg_files(opts, configs):
> +
> +if not list_bset_cfg_files(opts, configs) and not list_host(opts):
>  prefix = macro_expand(opts.defaults, '%{_prefix}')
>  if opts.canadian_cross():
>  opts.disable_install()
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: [PATCH] cpukit: occured -> occurred

2021-07-16 Thread Chris Johns
OK to push.
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: [PATCH 2/2] This adds a way to print the host triplet

2021-07-15 Thread Chris Johns
On 15/7/21 9:49 pm, Robin Müller wrote:
> I think this patch was forgotten so I'm just pushing it up again :-)

It was and sorry about that.

Minor change below ... no need to say "Displaying" as we know that. :)

if opts.get_arg('--list-host'):
print('Host operating system information: ')
print('Operating system: %s' % macro_expand(opts.defaults, '%{_os}'))
print('Number of processors: %s' % macro_expand(opts.defaults, '%{_ncpus}'))
print('Build architecture: %s' % macro_expand(opts.defaults, 
'%{_host_arch}'))
print('Host triplet: %s' % macro_expand(opts.defaults, '%{_host}'))
return True

A patch with that change is OK to be pushed if I do not get to it.

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

Re: [PATCH v3] bsps: Move optfdt* files to shared parent directory

2021-07-15 Thread Chris Johns
The patch need to add `default-by-family: []` to the places `default-by-variant`
are.

Chris

On 16/7/21 4:00 am, Joel Sherrill wrote:
> I was going to do a sweep of all BSPs and then push but had an issue
> with the patch. If something happened with the mail Pranav, please just
> compress it and send it to me privately as an attachment. If it needs
> rebasing, please take care of that.
> 
> [joel@devel rtems]$ git am /tmp/x
> Applying: bsps: Move optfdt* files to shared parent directory
> error: patch failed: spec/build/bsps/arm/beagle/optfdtcpyro.yml:1
> error: spec/build/bsps/arm/beagle/optfdtcpyro.yml: patch does not apply
> error: patch failed: spec/build/bsps/arm/beagle/optfdtmxsz.yml:1
> error: spec/build/bsps/arm/beagle/optfdtmxsz.yml: patch does not apply
> error: patch failed: spec/build/bsps/arm/beagle/optfdtro.yml:1
> error: spec/build/bsps/arm/beagle/optfdtro.yml: patch does not apply
> error: patch failed: spec/build/bsps/arm/beagle/optfdtuboot.yml:1
> error: spec/build/bsps/arm/beagle/optfdtuboot.yml: patch does not apply
> error: patch failed: spec/build/bsps/arm/imx/optfdtcpyro.yml:1
> error: spec/build/bsps/arm/imx/optfdtcpyro.yml: patch does not apply
> error: patch failed: spec/build/bsps/arm/imx/optfdtmxsz.yml:1
> error: spec/build/bsps/arm/imx/optfdtmxsz.yml: patch does not apply
> error: patch failed: spec/build/bsps/arm/imx/optfdtro.yml:1
> error: spec/build/bsps/arm/imx/optfdtro.yml: patch does not apply
> error: patch failed: spec/build/bsps/arm/imx/optfdtuboot.yml:1
> error: spec/build/bsps/arm/imx/optfdtuboot.yml: patch does not apply
> error: patch failed: spec/build/bsps/powerpc/qoriq/optfdtmxsz.yml:1
> error: spec/build/bsps/powerpc/qoriq/optfdtmxsz.yml: patch does not apply
> error: patch failed: spec/build/bsps/powerpc/qoriq/optfdtro.yml:1
> error: spec/build/bsps/powerpc/qoriq/optfdtro.yml: patch does not apply
> error: patch failed: spec/build/bsps/riscv/riscv/optfdtcpyro.yml:1
> error: spec/build/bsps/riscv/riscv/optfdtcpyro.yml: patch does not apply
> error: patch failed: spec/build/bsps/riscv/riscv/optfdtmxsz.yml:1
> error: spec/build/bsps/riscv/riscv/optfdtmxsz.yml: patch does not apply
> error: patch failed: spec/build/bsps/riscv/riscv/optfdtro.yml:1
> error: spec/build/bsps/riscv/riscv/optfdtro.yml: patch does not apply
> error: patch failed: spec/build/bsps/riscv/riscv/optfdtuboot.yml:1
> error: spec/build/bsps/riscv/riscv/optfdtuboot.yml: patch does not apply
> Patch failed at 0001 bsps: Move optfdt* files to shared parent directory
> hint: Use 'git am --show-current-patch' to see the failed patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
> 
> On Thu, Jul 15, 2021 at 10:13 AM Gedare Bloom  wrote:
>>
>> This patch looks fine to me if someone can pick it up. It does touch a
>> lot of build BSPs though.
>>
>> On Fri, Jul 9, 2021 at 12:16 AM Pranav Dangi  wrote:
>>>
>>> ping. (Apart from moving the files in a shared directory, a part of this 
>>> patch also gets the RTEMS master branch to work on all the Pi's except 4.)
>>>
>>> On Fri, 2 Jul 2021, 09:35 Pranav Dangi,  wrote:

 yes, I've built all of them

 On Thu, Jul 1, 2021 at 10:01 PM Gedare Bloom  wrote:
>
> Did you build all affected BSPs?
>
> On Thu, Jul 1, 2021 at 4:05 AM pranav  wrote:
>>
>> ---
>>  .../arm/altera-cyclone-v/bspalteracyclonev.yml   |  8 
>>  spec/build/bsps/arm/beagle/grp.yml   |  8 
>>  spec/build/bsps/arm/beagle/optfdtcpyro.yml   | 15 ---
>>  spec/build/bsps/arm/beagle/optfdtmxsz.yml| 16 
>>  spec/build/bsps/arm/beagle/optfdtro.yml  | 15 ---
>>  spec/build/bsps/arm/beagle/optfdtuboot.yml   | 15 ---
>>  spec/build/bsps/arm/imx/bspimx.yml   |  8 
>>  spec/build/bsps/arm/imx/optfdtcpyro.yml  | 15 ---
>>  spec/build/bsps/arm/imx/optfdtmxsz.yml   | 16 
>>  spec/build/bsps/arm/imx/optfdtro.yml | 15 ---
>>  spec/build/bsps/arm/imx/optfdtuboot.yml  | 15 ---
>>  spec/build/bsps/arm/raspberrypi/grp.yml  |  8 
>>  .../{arm/altera-cyclone-v => }/optfdtcpyro.yml   |  0
>>  .../{arm/altera-cyclone-v => }/optfdtmxsz.yml|  0
>>  .../bsps/{arm/altera-cyclone-v => }/optfdtro.yml |  0
>>  .../{arm/altera-cyclone-v => }/optfdtuboot.yml   |  0
>>  spec/build/bsps/powerpc/qoriq/grp.yml|  4 ++--
>>  spec/build/bsps/powerpc/qoriq/optfdtmxsz.yml | 16 
>>  spec/build/bsps/powerpc/qoriq/optfdtro.yml   | 15 ---
>>  spec/build/bsps/riscv/riscv/grp.yml  |  8 
>>  spec/build/bsps/riscv/riscv/optfdtcpyro.yml  | 15 ---
>>  

Re: GSoC - Code Formatting and Style Checking for RTEMS score

2021-07-15 Thread Chris Johns
Hi,

I will bring the discussion to the devel list and I hope the comments are in
line with the purpose of the project. Please correct what I say if it is not.

The pre-commit script that is in github review is a good base and I believe it
solves that problem for those on Linux. It is a great start and it is nice to 
see.

The work needs to mature and progress and that means a few things. I was
approached by Joel about where this would live in rtems-tools. As the script
stands it is not suitable for rtems-tool because the format is specific to the
score code in rtems.git.

I think a pre-commit script needs to live in the repo it is for in a spot a user
can copy to the hooks directory of their repo. For example `git-hooks`.

A git hooks script for rtems.git needs to run on all the supported hosts or we
may result in patches being left on the floor. If a contributor's host OS is not
supported and the patch is rejected for formatting reasons picked up by the
check the contributor may just walk away.

The script should use `/usr/bin/env python3` and it needs to check for a range
of `clang-format` instances. FreeBSD has a package that installs the format
command as `clang-format10` for LLVM 10 and `clang-format11` for version 11.

A contributor being able to run the script may depend on the host having a
package or packages installed. Given this is for RTEMS development that is fine.
The script needs to handle any set up errors nicely if something is missing. I
prefer we avoid the "experts" approach to error management.

There is possible future work adding a command to rtems-tools. Ida and I
discussed this on discord and we decided a command called `rtems-style` would be
nice. It would be nice if that command checked, ie `--mode=check`, a style as
well as `--mode=reformat` to automatically reformat a source file.

A command for rtems-tools has to support python2 and python3 and it has to
handle errors in suitable manner. Printing uncaught exception is not OK. It
should also be self contained and not depend on any pip python packages.

If an `rtems-style` command is created and installed when rtems-tool is
installed the pre-commit git script could be made to use it and so the style is
held in a single location.

Finally an rtems-style command could be extended to support python
(--lang=python) or other styles for other code formats. This is inline with our
other eco-system interfaces we provide.

I hope this helps.

Chris

On 16/7/21 5:13 am, Ida Delphine wrote:
> I will check on this
> 
> On Thu, Jul 15, 2021 at 3:39 PM Gedare Bloom  > wrote:
> 
> On Thu, Jul 15, 2021 at 5:19 AM Ida Delphine  > wrote:
> >
> > Hello everyone,
> > From the discussion on discord it looks like clang-format cannot be
> installed on MacOS and FreeBSD. Is there any alternative or way to have it
> on these operating systems?
> >
> What are the challenges with them? Can clang-format be built from
> source on those hosts?
> 
> > On Wed, 5 May 2021, 10:21 pm Ida Delphine,  > wrote:
> >>
> >> Hello everyone,
> >>
> >> Regarding this project (https://devel.rtems.org/ticket/3860
> ) I went with clang-format as we all
> agreed. I have tested it on some "score" files and it made some changes
> which I don't think are very much in line with the RTEMS coding style.
> However, it wasn't really clear if we will chage the RTEMS coding style or
> try to make changes to clang-format to fit the style.
> >> Please will love to know the best option.
> >>
> >> Cheers,
> >> Ida.
> >
> > ___
> > devel mailing list
> > devel@rtems.org 
> > http://lists.rtems.org/mailman/listinfo/devel
> 
> 
> 
> ___
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: [PATCH] covoar: Fix errors building on FreeBSD and clang

2021-07-15 Thread Chris Johns
On 16/7/21 12:22 pm, Alex White wrote:
> 
> On Thu, Jul 15, 2021 at 1:20 AM Chris Johns  wrote:
>>
>> On 15/7/21 12:45 pm, Alex White wrote:
>> > On Tue, Jul 6, 2021 at 7:55 PM Chris Johns  wrote:
>> >>
>> >> On 3/7/21 5:56 am, Alex White wrote:
>> >> > On Wed, Jun 30, 2021 at 11:40 PM  wrote:
>> >> >>
>> >> >> From: Chris Johns 
>> >> >>
>> >> >> - The member variable `path_` cannot be a reference and initialised to
>> >> >>   a const char* type input. To do so would require there is a 
>> >> >> temporary with
>> >> >>   an unspecified life time.
>> >> >> ---
>> >> >>  tester/covoar/AddressToLineMapper.h | 2 +-
>> >> >>  tester/covoar/Target_aarch64.h      | 2 +-
>> >> >>  2 files changed, 2 insertions(+), 2 deletions(-)
>> >> >>
>> >> >> diff --git a/tester/covoar/AddressToLineMapper.h
>> >> > b/tester/covoar/AddressToLineMapper.h
>> >> >> index 88bf475..c78aef7 100644
>> >> >> --- a/tester/covoar/AddressToLineMapper.h
>> >> >> +++ b/tester/covoar/AddressToLineMapper.h
>> >> >> @@ -84,7 +84,7 @@ namespace Coverage {
>> >> >>       *  An iterator pointing to the location in the set that contains 
>> >> >> the
>> >> >>       *  source file path of the address.
>> >> >>       */
>> >> >> -    const std::string& path_;
>> >> >> +    const std::string path_;
>> >> >
>> >> > Ryan alerted me about this issue a couple weeks back. This patch would
> fix the
>> >> > problem, but it would lead to a second copy of every file path string 
>> >> > being
>> >> > stored in memory. This would also take away the usefulness of the set 
>> >> > of file
>> >> > path strings maintained by the AddressLineRange class.
>> >> >
>> >> > Instead, I propose we change the empty SourceLine constructor to take a
> `const
>> >> > std::string&`. This would allow the addition of something like this to
> the top
>> >> > of AddressToLineMapper::getSource():
>> >> > const std::string unknown = "unknown";
>> >> > const SourceLine default_sourceline = SourceLine(unknown);
>> >> >
>> >> > That should eliminate the issue and preserve the memory conservation
> efforts of
>> >> > the original design.
>> >>
>> >> Yes it would but for some reason, that I cannot put my finger on, it 
>> >> seems like
>> >> a breach of boundaries between classes.
>> >>
>> >> How much data are we talking about? Are you able to see the memory foot 
>> >> print
>> >> with the strings being contained vs what you have now?
>> >
>> > Sorry for the late reply.
>> >
>> > What I have now yields a peak memory usage for covoar when run on all ARM 
>> > tests
>> > of 219 MB.
>> > Changing `SourceLine::path_` to plain `std::string` results in an increase 
>> > to
>> > 523 MB.
>> >
>> > So it is a significant increase.
>>
>> Yes and thank you for the test results. This makes sharing a worth while 
>> exercise.
>>
>> >> If the figures show the strings need to be shared to avoid a memory blow 
>> >> out
>> >> would a std::shared_ptr be something to look at where all
>> >> references are a shared pointer? A shared pointer means any changes do 
>> >> not flow
>> >> from one class to other.
>> >
>> > I don't think that `std::shared_ptr` would help here. Wouldn't that handle 
>> > the
>> > case of an unknown path the same way as a raw pointer solution? Wouldn't we
>> > still have to check the return value of `SourceLine::path()` to make sure 
>> > that
>> > it is not null?
>> > The only other way I see to make it work would be to store some "unknown" 
>> > string
>> > object and hand pointers to it to all `SourceLine` objects with unknown 
>> > paths.
>> > But that seems mostly equivalent to what I propose in my original reply to 
>> > this
>> > patch.
>>
>> I have not checked all the detail but a raw pointer will always be fragile
>> compared to a shared_ptr. You may get it working but there is 

Re: [PATCH] build: Add the BSP family to the enable set

2021-07-15 Thread Chris Johns
That is a nice change.

OK to push

Chris

On 15/7/21 4:19 pm, Sebastian Huber wrote:
> This makes it possible to use the BSP family in expressions of the enabled-by
> attribute.
> ---
>  wscript | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/wscript b/wscript
> index 487951fdba..1331ae149e 100755
> --- a/wscript
> +++ b/wscript
> @@ -1387,7 +1387,12 @@ def configure_variant(conf, cp, bsp_map, path_list, 
> top_group, variant):
>  
>  # For the enabled-by evaluation we have to use the base BSP defined by 
> the
>  # build specification and not the BSP name provided by the user.
> -conf.env["ENABLE"] = [get_compiler(conf, cp, variant), arch, arch_bsp]
> +conf.env["ENABLE"] = [
> +get_compiler(conf, cp, variant),
> +arch,
> +arch_family,
> +arch_bsp,
> +]
>  
>  conf.env["TOP"] = conf.path.abspath()
>  conf.env["TOPGROUP"] = top_group
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: [PATCH] covoar: Fix errors building on FreeBSD and clang

2021-07-15 Thread Chris Johns
On 15/7/21 12:45 pm, Alex White wrote:
> On Tue, Jul 6, 2021 at 7:55 PM Chris Johns  wrote:
>>
>> On 3/7/21 5:56 am, Alex White wrote:
>> > On Wed, Jun 30, 2021 at 11:40 PM  wrote:
>> >>
>> >> From: Chris Johns 
>> >>
>> >> - The member variable `path_` cannot be a reference and initialised to
>> >>   a const char* type input. To do so would require there is a temporary 
>> >> with
>> >>   an unspecified life time.
>> >> ---
>> >>  tester/covoar/AddressToLineMapper.h | 2 +-
>> >>  tester/covoar/Target_aarch64.h      | 2 +-
>> >>  2 files changed, 2 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/tester/covoar/AddressToLineMapper.h
>> > b/tester/covoar/AddressToLineMapper.h
>> >> index 88bf475..c78aef7 100644
>> >> --- a/tester/covoar/AddressToLineMapper.h
>> >> +++ b/tester/covoar/AddressToLineMapper.h
>> >> @@ -84,7 +84,7 @@ namespace Coverage {
>> >>       *  An iterator pointing to the location in the set that contains the
>> >>       *  source file path of the address.
>> >>       */
>> >> -    const std::string& path_;
>> >> +    const std::string path_;
>> >
>> > Ryan alerted me about this issue a couple weeks back. This patch would fix 
>> > the
>> > problem, but it would lead to a second copy of every file path string being
>> > stored in memory. This would also take away the usefulness of the set of 
>> > file
>> > path strings maintained by the AddressLineRange class.
>> >
>> > Instead, I propose we change the empty SourceLine constructor to take a 
>> > `const
>> > std::string&`. This would allow the addition of something like this to the 
>> > top
>> > of AddressToLineMapper::getSource():
>> > const std::string unknown = "unknown";
>> > const SourceLine default_sourceline = SourceLine(unknown);
>> >
>> > That should eliminate the issue and preserve the memory conservation 
>> > efforts of
>> > the original design.
>>
>> Yes it would but for some reason, that I cannot put my finger on, it seems 
>> like
>> a breach of boundaries between classes.
>>
>> How much data are we talking about? Are you able to see the memory foot print
>> with the strings being contained vs what you have now?
> 
> Sorry for the late reply.
> 
> What I have now yields a peak memory usage for covoar when run on all ARM 
> tests
> of 219 MB.
> Changing `SourceLine::path_` to plain `std::string` results in an increase to
> 523 MB.
> 
> So it is a significant increase.

Yes and thank you for the test results. This makes sharing a worth while 
exercise.

>> If the figures show the strings need to be shared to avoid a memory blow out
>> would a std::shared_ptr be something to look at where all
>> references are a shared pointer? A shared pointer means any changes do not 
>> flow
>> from one class to other.
> 
> I don't think that `std::shared_ptr` would help here. Wouldn't that handle the
> case of an unknown path the same way as a raw pointer solution? Wouldn't we
> still have to check the return value of `SourceLine::path()` to make sure that
> it is not null?
> The only other way I see to make it work would be to store some "unknown" 
> string
> object and hand pointers to it to all `SourceLine` objects with unknown paths.
> But that seems mostly equivalent to what I propose in my original reply to 
> this
> patch.

I have not checked all the detail but a raw pointer will always be fragile
compared to a shared_ptr. You may get it working but there is nothing making
sure any references are still pointing to valid memory. Given the difference in
sizes you found there is a lot of sharing and so a lot of references.

> Now that I think about it, maybe making `SourceLine::_path` into an
> `std::string*` and doing a `nullptr` check in the `SourceLine::path()` would 
> be
> most elegant?

All I see are potential leaks, maybe not now but may be after the next person
makes changes.

> That way we don't have to do any `nullptr` checks outside of the `SourceLine`
> implementation. We could just have `SourceLine::path()` return a plain old
> `std::string` and do something like this:
> 
> if (_path == nullptr) {
>     return "unknown";
> else {
>     return *_path;
> }

A std::shared_ptr has the bool operator to check if the pointer is the nullptr
so it can store a nullptr. Why not use:

 if (!_path) {
 return static_const_unknown_string;
 } else {
 return *(_path.get());
 }

?

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


Re: [PATCH] build: Prefer variant default value over family

2021-07-15 Thread Chris Johns
Yes this is good thing to do and thanks.

Please push.

Chris

On 15/7/21 4:07 pm, Sebastian Huber wrote:
> Update #4468.
> ---
>  wscript | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/wscript b/wscript
> index 1206c4d882..bba0c1502a 100755
> --- a/wscript
> +++ b/wscript
> @@ -694,14 +694,14 @@ class OptionItem(Item):
>  
>  def default_value(self, variant, family):
>  value = self.data["default"]
> -for default in self.data["default-by-family"]:
> -if OptionItem._is_variant(default["families"], family):
> -value = default["value"]
> -break
>  for default in self.data["default-by-variant"]:
>  if OptionItem._is_variant(default["variants"], variant):
>  value = default["value"]
>  break
> +for default in self.data["default-by-family"]:
> +if OptionItem._is_variant(default["families"], family):
> +value = default["value"]
> +break
>  if value is None:
>  return value
>  if isinstance(value, list):
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: [PATCH rtems-docs] user: Update ZynqMP network configuration

2021-07-14 Thread Chris Johns
OK

On 15/7/21 5:56 am, Kinsey Moore wrote:
> Network configuration is now automatic and requires no user
> configuration.
> ---
>  user/bsps/aarch64/xilinx-zynqmp.rst | 11 +++
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/user/bsps/aarch64/xilinx-zynqmp.rst 
> b/user/bsps/aarch64/xilinx-zynqmp.rst
> index 71b6842..90270db 100644
> --- a/user/bsps/aarch64/xilinx-zynqmp.rst
> +++ b/user/bsps/aarch64/xilinx-zynqmp.rst
> @@ -59,14 +59,9 @@ Network Configuration
>  -
>  
>  When used with LibBSD, these BSP variants support networking via the four
> -Cadence GEM instances present on all ZynqMP hardware variants. These are 
> enabled
> -using config.inc in LibBSD by setting any of the following constants to 1:
> -NET_CFG_ZYNQMP_USE_CGEM0 = 1
> -NET_CFG_ZYNQMP_USE_CGEM1 = 1
> -NET_CFG_ZYNQMP_USE_CGEM2 = 1
> -NET_CFG_ZYNQMP_USE_CGEM3 = 1
> -
> -Most ZynqMP dev boards use CGEM3. None of the interfaces are enabled by 
> default.
> +Cadence GEM instances present on all ZynqMP hardware variants. All interfaces
> +are enabled by default, but only interfaces with operational MII busses will 
> be
> +recognized and usable in RTEMS. Most ZynqMP dev boards use CGEM3.
>  
>  Running Executables on QEMU
>  ---
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


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

2021-07-14 Thread Chris Johns
On 14/7/21 5:01 pm, Sebastian Huber wrote:
> On 14/07/2021 08:51, chr...@rtems.org wrote:
>> @@ -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
> 
> During the discussion of the build items, we agreed that all attributes should
> be explicit in the items. So, a "default-by-family: []" should be added to all
> BSP items.

Can I add `default-by-family: []` before `default-by-variant` in the spec files?

It is easier to batch edit.

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

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

2021-07-14 Thread Chris Johns
On 14/7/21 5:24 pm, Sebastian Huber wrote:
> On 14/07/2021 09:21, Chris Johns wrote:
>> Is there a nice way to catch any missing fields and report them? The waf 
>> conf is
>> not present and I am not sure if we catch any standard python exception and
>> report them by waf?
> 
> The current approach is to check the user input (e.g. config.ini) and report
> configuration errors. For errors in the build specification which is supposed 
> to
> be maintained by experts you get exceptions.

I prefer a soft landing. It helps bring those who are learning to be experts
become experts.

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


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

2021-07-14 Thread Chris Johns
On 14/7/21 5:01 pm, Sebastian Huber wrote:
> On 14/07/2021 08:51, chr...@rtems.org wrote:
>> @@ -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
> 
> During the discussion of the build items, we agreed that all attributes should
> be explicit in the items. So, a "default-by-family: []" should be added to all
> BSP items.

OK I will generate a v3 patch.

Is there a nice way to catch any missing fields and report them? The waf conf is
not present and I am not sure if we catch any standard python exception and
report them by waf?

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

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

2021-07-14 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
>  

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: [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 rtems-libbsd v2 1/2] freebsd/if_cgem: Fail probe for unterminated MII

2021-07-11 Thread Chris Johns
On 10/7/21 12:53 am, Kinsey Moore wrote:
> When the MII bus is unterminated on unused interfaces, it results in PHY
> read timeouts which manifest as spurious PHYs during the attach call.
> Detect these timeouts during the probe so the device can be ignored.
> ---
>  freebsd/sys/dev/cadence/if_cgem.c | 18 ++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/freebsd/sys/dev/cadence/if_cgem.c 
> b/freebsd/sys/dev/cadence/if_cgem.c
> index 34df7ac7..51e0bd6d 100644
> --- a/freebsd/sys/dev/cadence/if_cgem.c
> +++ b/freebsd/sys/dev/cadence/if_cgem.c
> @@ -1955,6 +1955,24 @@ cgem_probe(device_t dev)
>   return (ENXIO);
>  #endif /* __rtems__ */
>  
> + struct cgem_softc *sc = device_get_softc(dev);
> + int val, rid = 0;
> +
> + /* Check for PHY read timeouts which indicate an unterminated MII bus */
> + sc->mem_res = bus_alloc_resource_any(dev, SYS_RES_MEMORY, ,
> +  RF_ACTIVE);
> +
> + val = cgem_miibus_readreg(dev, 0, MII_BMSR);
> + if (val == -1) {
> + bus_release_resource(dev, SYS_RES_MEMORY, ,
> +  sc->mem_res);
> + sc->mem_res = NULL;
> + return (ENXIO);
> + }
> + bus_release_resource(dev, SYS_RES_MEMORY, ,
> +  sc->mem_res);
> + sc->mem_res = NULL;

Should this change be in an "#else /* __rtems__ */" section until merged 
upstream?

Otherwise this change is looking good.

Chris

> +
>   device_set_desc(dev, "Cadence CGEM Gigabit Ethernet Interface");
>   return (0);
>  }
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: Throughput/Goodput analysis on RTEMS

2021-07-11 Thread Chris Johns
On 10/7/21 1:28 pm, Vijay Kumar Banerjee wrote:
> On Fri, Jul 9, 2021 at 8:25 PM Chris Johns  wrote:
>>
>> On 2/7/21 4:40 am, Vijay Kumar Banerjee wrote:
>>> I'm planning to do a throughput analysis on the RTEMS network stacks
>>> and I'm looking for some suggestions on the tools/applications for
>>> that if anyone has done something like that before.
>>
>> This is a great project.
>>
>>> If such application has not been used with RTEMS, then I might be
>>> willing to port it to RTEMS or write one from scratch as a net-demo
>>> maybe. Any resource/advice for that is welcome and much appreciated.
>>
>> Throughput is one parameter when dealing with networking and it is important 
>> in
>> some applications while latency is another parameter that can be critical.
>> Consider a voice switch with operators using Push To Talk (PTT) buttons to
>> activate a radio's transmitter, an enable packet needs to be delivered within
>> 10msec max in all cases.
>>
> This is an interesting point. Thanks for mentioning latency.

In RTEMS and in an RTOS in general I also consider the latency for separate
socket paths to separate readers and/or concurrent readers and writers as
something important. A lot of stacks maintain a single giant lock for the stack
and that serialises the processing through the stack. Libbsd does not have this
limitation with it's fine grain locking. A protocol such as PTP benefits from
deterministic handling of packets.

>> I would look at removing the networking fabric from the analysis because it 
>> is
>> subjective and can be effected by the NIC hardware, PHY configuration plus 
>> any
>> externally connected equipment.
>>
> I'm approaching this by running different network stacks over the same
> hardware. I have a uC5282 that runs legacy networking and I have
> ported the driver to libbsd nexus device (dhcp doesn't work yet) and
> I'm able to see some difference in the round trip time over loopback
> with different stacks.

Will your analysis look at architectures that support SMP and have various cache
configurations? On a Zynq libbsd creates over 4000 locks and so there may be a
performance hit on smaller single core systems compared with multi-core
architectures that can run concurrent threads in the stack at the same time.

I think the 5282 may bias the results in a particular way and other archs will
bias in a different way. I do not see this as wrong or a problem, rather it is
something that needs to be accounted for so readers of the analysis do not get
the wrong impression.

>> In terms of network stack performance for RTEMS you need to consider the
>> protocol, buffering used, size of the buffer pool, transmit and receive 
>> paths,
>> they will have different characteristics, and target's memory effects such as
>> caches. On top of this is filtering, ie packet filtering, and the types of 
>> access.
>>
> Thanks for these interesting attributes to consider. I have done
> preliminary analysis over ICMP with packets of same size over
> different network stacks on the same board.
> 
>> With libbsd there are a number of sysctl settings that effect the 
>> performance.
>> How will you manage these?
>>
> I'm not sure. I didn't know about the possible performance difference
> based on sysctl settings. This would be relevant for any user and I
> would like to explore it. Could you please point to some resources or
> examples?

There are a lot of posts on the net on this topic (search: "freebsd tune network
sysctl") and the values can be technical but they do make a difference for
different data workflows. There is no single set of values what work universally
and for libbsd this is OK because I encourage users to explore the settings and
see what works for them. There can be a memory used vs performance trade off. We
support the sysctl API and there is a sysctl command for the shell.

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


Re: Throughput/Goodput analysis on RTEMS

2021-07-09 Thread Chris Johns
On 2/7/21 4:40 am, Vijay Kumar Banerjee wrote:
> I'm planning to do a throughput analysis on the RTEMS network stacks
> and I'm looking for some suggestions on the tools/applications for
> that if anyone has done something like that before.

This is a great project.

> If such application has not been used with RTEMS, then I might be
> willing to port it to RTEMS or write one from scratch as a net-demo
> maybe. Any resource/advice for that is welcome and much appreciated.

Throughput is one parameter when dealing with networking and it is important in
some applications while latency is another parameter that can be critical.
Consider a voice switch with operators using Push To Talk (PTT) buttons to
activate a radio's transmitter, an enable packet needs to be delivered within
10msec max in all cases.

I would look at removing the networking fabric from the analysis because it is
subjective and can be effected by the NIC hardware, PHY configuration plus any
externally connected equipment.

In terms of network stack performance for RTEMS you need to consider the
protocol, buffering used, size of the buffer pool, transmit and receive paths,
they will have different characteristics, and target's memory effects such as
caches. On top of this is filtering, ie packet filtering, and the types of 
access.

With libbsd there are a number of sysctl settings that effect the performance.
How will you manage these?

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


Re: [PATCH rtems-libbsd] rtemsbsd: Use a separate header for test devices

2021-07-09 Thread Chris Johns
On 9/7/21 3:39 pm, Sebastian Huber wrote:
> On 08/07/2021 03:11, Kinsey Moore wrote:
>> I wonder why this never came up with Zynq or QorIQ. Maybe no one wanted to 
>> run
>> network tests on the alternate interfaces because dev boards with those
>> interfaces configured didn't exist? It's possible that the ukphy driver could
>> be improved and this entire problem just goes away or we ban that driver from
>> the default configuration for multi-interface BSPs and the problem goes away.
> 
> The QorIQ BSP uses a device tree.
> 
> The chips will get more and more complex. Managing this complexity with hand
> written C preprocessor defines is a dead end from my point of view. Device 
> trees
> allow you to provide a generic BSP which is initialized for a particular board
> using the device tree. Compared to other operating systems, the device tree
> support and generic device enumeration in RTEMS is a bit under developed. See
> for example:
> 
> https://docs.zephyrproject.org/latest/guides/dts/index.html#dt-guide
> 

I agree and this is well said. I am concerned an ad-hock implementation leaves
us with fragmented and hard to maintain support at the BSP level.

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


Re: [PATCH rtems-libbsd 1/3] dev/mii: Handle PHY read timeouts

2021-07-08 Thread Chris Johns
On 9/7/21 3:28 pm, Sebastian Huber wrote:
> On 09/07/2021 03:14, Kinsey Moore wrote:
>> PHY read timeouts return 0x and bypass the current bad/no PHY
>> checks. This adds a check specifically for that read timeout to avoid
>> probing PHYs that don't exist.
>> ---
>>   freebsd/sys/dev/mii/mii.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/freebsd/sys/dev/mii/mii.c b/freebsd/sys/dev/mii/mii.c
>> index d0428f24..8f087cab 100644
>> --- a/freebsd/sys/dev/mii/mii.c
>> +++ b/freebsd/sys/dev/mii/mii.c
>> @@ -474,6 +474,7 @@ mii_attach(device_t dev, device_t *miibus, if_t ifp,
>>    */
>>   bmsr = MIIBUS_READREG(dev, ma.mii_phyno, MII_BMSR);
>>   if (bmsr == 0 || bmsr == 0x ||
>> +    bmsr == 0x ||
> 
> Could you please fix the driver so that it returns 0 or 0x in case of
> timeouts? This is the general MII module of FreeBSD. If you think there is a 
> bug
> in this code, then please report it to FreeBSD first.

We should resolve the issue in the probe and not the attach call. Attach errors
are real errors, probe errors or failures are niche to the specifics of the
hardware.

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

Re: [PATCH rtems-libbsd] rtemsbsd: Use a separate header for test devices

2021-07-08 Thread Chris Johns
On 9/7/21 7:32 am, Kinsey Moore wrote:
> On 7/8/2021 10:36, Kinsey Moore wrote:
>> On 7/8/2021 02:48, Chris Johns wrote:
>>> On 8/7/21 11:11 am, Kinsey Moore wrote:
>>>> On 7/7/2021 19:28, Chris Johns wrote:
>>>>> We can:
>>>>> 1. Add FDT support. This is something I would like to avoid as it adds an
>>>>> extra
>>>>> layer of dependency and it complicates backwards compatibility for 
>>>>> existing
>>>>> users. I however wonder if this is just me and it is something that will 
>>>>> be
>>>>> needed more and more and cannot be avoided.
>>>> It's not just you, I (and Joel) lean this direction as well. Having the 
>>>> option
>>>> of FDT would be nice and would certainly cover this case, but making it a 
>>>> hard
>>>> requirement seems excessive.
>>> If we could contain a suitable default FDT in the BSP that achieves the 
>>> outcome
>>> then I would be OK.
>>>
>>>>> 2. Add a weak call that is RTEMS and BSP specific to return a probe 
>>>>> state. The
>>>>> default state could be enabled and the tests provide something from the
>>>>> network
>>>>> config. Not a great solution but it is simple and cheap to implement. The
>>>>> driver
>>>>> already has a weak call to set the reference clock.
>>>> This patch (since omitted above) strikes a similar compromise but has the
>>>> downside of the default application configuration not having network at 
>>>> all.
>>>> The
>>>> downside of this suggestion (option #2) is the addition of a second method 
>>>> to
>>>> accomplish the same goal: define a new hook vs redefine or add to the 
>>>> existing
>>>> definitions in nexus-devices.h. My initial patch addressed both, but added 
>>>> what
>>>> appeared to be dead options to RTEMS and has since been reverted. The other
>>>> current/alternate patch addresses both but leaks test configuration state 
>>>> into
>>>> the application by installing the test network configuration.
>>> I do not see them as being the same thing, yes they achieve the same result
>>> however via different interfaces. Your patch creates a top level "user
>>> interface", ie at the publicly exposed nexus bus interface while a weak 
>>> driver
>>> interface is a private agreement between the BSP and this driver plus users 
>>> can
>>> provide their own implementation to further change the mix without needing 
>>> to
>>> configure or touch any released code. I am concerned the pattern of defining
>>> mixes in the nexus header is adopted by others on a look and copy basis and 
>>> we
>>> get more and more options that never get compiled and rot.
>>>
>>> A 2.1 alternative could be adding a global define such as 
>>>
>>>   RTEMS_BSD_CONFIG_NEXUS_BUS_DISABLE_DEFAULT_DRIVERS
>>>
>>> that disables all the provided definitions and the user can then provide 
>>> their
>>> own? Maybe the network config could be extended to allow such defines?
>>>
>>> A global disable is needed so the BSD config header I posted before could be
>>> used without nexus defines clashing.
>>>
>>>>> 3. Try and detect a PHY in the probe? I am not sure if that is easy or
>>>>> hard. If
>>>>> read works maybe that is something that may be suitable.
>>>> That's what is currently happening.
>>> Are you sure? I see the probe as passing and the device is consider 
>>> available
>>> however there is no PHY and that creates the errors and that seems 
>>> reasonable.
>>> The UKPHY driver is designed to inspect all PHY addresses however needing 
>>> 200
>>> reties does seem excessive.
>>>
>>> I was thinking about a simpler scan of the MII bus for a PHY in the probe 
>>> and if
>>> none found disable return -1 from the probe.
>> Sorry, I misunderstood what you were saying. You're correct that a PHY
>> detection does not occur in the CGEM probe, but that wouldn't improve the
>> current situation since PHYs are being detected on all available slots. If
>> that weren't happening there wouldn't be PHY read/write errors, either.
>>>> Unfortunately, the ukphy driver is there to
>>>> detect braindead PHYs (I have a board which needs it, but could probably 
>>>

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

2021-07-08 Thread Chris Johns
On 9/7/21 3:06 am, Alex White wrote:
> On Wed, Jul 7, 2021 at 6:55 PM Chris Johns  wrote:
>>
>> On 7/7/21 11:37 pm, Ryan Long wrote:
>> > I'll get those pointers changed to references, and remove the whitespace 
>> > changes. Is there a particular reason to not use '\n' instead of std::endl?
>>
>> Awesome and thanks.
>>
>> > I read that std::endl is slower since it's flushing the buffer each time 
>> > that it is used.
>>
>> The std::endl is platform independent so language implementers can match it 
>> to
>> the platform the code is being built for. It is similar to os.linesep in 
>> python
>> and why that should be used there.
> 
> Chris,
> 
> I thought this, too, until Ryan forced me to look into it further. Thanks, 
> Ryan :)
> 
> According to various sources, '\n' gets translated to the current platform's
> line separator as long as the C++ file stream is opened in text mode. See, for
> example, https://stackoverflow.com/a/213977 
> <https://stackoverflow.com/a/213977>.

Why would you use std::endl in a binary stream? Is the report a binary format?

> So |std::endl|​ would indeed likely be slower AND unnecessary to achieve
> platform independence.

Some platforms require '\r\n' for an end of line. How does this approach handle
that?

Have you measured the performance savings this provides?

I prefer you do not add special code for that sort of stuff and I prefer we
avoid micro-optimisations over following the standards based ways of handling
these things.

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

Re: [PATCH rtems-libbsd] rtemsbsd: Use a separate header for test devices

2021-07-08 Thread Chris Johns
On 8/7/21 11:11 am, Kinsey Moore wrote:
> On 7/7/2021 19:28, Chris Johns wrote:
>> On 7/7/21 11:26 pm, Kinsey Moore wrote:
>>> On 7/6/2021 21:20, Chris Johns wrote:
>>>> On 7/7/21 12:03 pm, Kinsey Moore wrote:
>>>>> On 7/6/2021 20:57, Chris Johns wrote:
>>>>>> On 7/7/21 11:05 am, Kinsey Moore wrote:
>>>>>>> The need for the difference on ZynqMP is that there are 4 different CGEM
>>>>>>> interfaces of which dev boards primarily make use of CGEM3.
>>>>>> RTEMS_BSD_DRIVER_XILINX_ZYNQMP_CGEM0(ZYNQMP_IRQ_ETHERNET_0);
>>>>>> RTEMS_BSD_DRIVER_XILINX_ZYNQMP_CGEM1(ZYNQMP_IRQ_ETHERNET_1);
>>>>>> RTEMS_BSD_DRIVER_XILINX_ZYNQMP_CGEM2(ZYNQMP_IRQ_ETHERNET_2);
>>>>>> RTEMS_BSD_DRIVER_XILINX_ZYNQMP_CGEM3(ZYNQMP_IRQ_ETHERNET_3);
>>>>>>
>>>>>> ?
>>>>> Yes, this does technically work
>>>> Hmm, I suggest this is what we should support as a default.
>>>>
>>>>> if you can read the shell output past the log
>>>>> spam. The other interfaces trying and failing to come up throw a 
>>>>> gargantuan
>>>>> amount of messages to the console.
>>>> Why do these interfaces fail to initialise? What are the errors?
>>>>
>>>> The removed probe check is based around FDT and so I suspect is the reason
>>>> Sebastian suggested FDT support. Needing FDT support would break existing 
>>>> Zynq
>>>> users.
>>> The devices themselves are detected just fine and are likely operational. 
>>> The
>>> MII busses are probably unterminated or pulled high/low which yields a ukphy
>>> detection on every available slot and constant PHY read/write timeouts. A 
>>> small
>>> sample from enabling both CGEM2 and CGEM3 on this custom board:
>>>
>>> ukphy13:  none, 10baseT, 10baseT-FDX, 100baseTX, 100baseTX-FDX, 100baseT4,
>>> 1000baseSX, 1000baseSX-FDX, 1000baseT, 1000baseT-master, 1000baseT-FDX,
>>> 1000baseT-FDX-master, auto
>>> cgem3: phy read timeout: 0
>>> ukphy14:  PHY 14 on miibus0
>>> cgem3: phy write timeout: 0
>>> cgem3: phy read timeout: 0
>>> cgem3: phy read timeout: 0
>>> cgem3: phy read timeout: 0
>>> cgem3: phy read timeout: 0
>>> cgem3: phy read timeout: 0
>>> cgem3: phy read timeout: 0
>>>
>>> ...
>>>
>>> cgem3: phy read timeout: 0
>> That is noisy and painful.
>>
>>> This type of output reached ukphy31 before I was able to run the shutdown
>>> command with 50+ PHY read/write errors and more continuing afterward. I 
>>> imagine
>>> it's worse if I enable all 4 interfaces.
>> The output is from the cgem driver which is good because it is localised. I
>> agree we need something to control what is enabled and I prefer we avoid 
>> special
>> build options, I will explain why I think this later. We can:
>>
>> 1. Add FDT support. This is something I would like to avoid as it adds an 
>> extra
>> layer of dependency and it complicates backwards compatibility for existing
>> users. I however wonder if this is just me and it is something that will be
>> needed more and more and cannot be avoided.
> 
> It's not just you, I (and Joel) lean this direction as well. Having the option
> of FDT would be nice and would certainly cover this case, but making it a hard
> requirement seems excessive.

If we could contain a suitable default FDT in the BSP that achieves the outcome
then I would be OK.

>> 2. Add a weak call that is RTEMS and BSP specific to return a probe state. 
>> The
>> default state could be enabled and the tests provide something from the 
>> network
>> config. Not a great solution but it is simple and cheap to implement. The 
>> driver
>> already has a weak call to set the reference clock.
> This patch (since omitted above) strikes a similar compromise but has the
> downside of the default application configuration not having network at all. 
> The
> downside of this suggestion (option #2) is the addition of a second method to
> accomplish the same goal: define a new hook vs redefine or add to the existing
> definitions in nexus-devices.h. My initial patch addressed both, but added 
> what
> appeared to be dead options to RTEMS and has since been reverted. The other
> current/alternate patch addresses both but leaks test configuration state into
> the application by installing the test network configuration.

I do not see them as being the same thing, yes they achieve the same 

Re: [PATCH rtems-libbsd] rtemsbsd: Use a separate header for test devices

2021-07-07 Thread Chris Johns
On 7/7/21 11:26 pm, Kinsey Moore wrote:
> On 7/6/2021 21:20, Chris Johns wrote:
>> On 7/7/21 12:03 pm, Kinsey Moore wrote:
>>> On 7/6/2021 20:57, Chris Johns wrote:
>>>> On 7/7/21 11:05 am, Kinsey Moore wrote:
>>>>> The need for the difference on ZynqMP is that there are 4 different CGEM
>>>>> interfaces of which dev boards primarily make use of CGEM3.
>>>> RTEMS_BSD_DRIVER_XILINX_ZYNQMP_CGEM0(ZYNQMP_IRQ_ETHERNET_0);
>>>> RTEMS_BSD_DRIVER_XILINX_ZYNQMP_CGEM1(ZYNQMP_IRQ_ETHERNET_1);
>>>> RTEMS_BSD_DRIVER_XILINX_ZYNQMP_CGEM2(ZYNQMP_IRQ_ETHERNET_2);
>>>> RTEMS_BSD_DRIVER_XILINX_ZYNQMP_CGEM3(ZYNQMP_IRQ_ETHERNET_3);
>>>>
>>>> ?
>>> Yes, this does technically work
>> Hmm, I suggest this is what we should support as a default.
>>
>>> if you can read the shell output past the log
>>> spam. The other interfaces trying and failing to come up throw a gargantuan
>>> amount of messages to the console.
>> Why do these interfaces fail to initialise? What are the errors?
>>
>> The removed probe check is based around FDT and so I suspect is the reason
>> Sebastian suggested FDT support. Needing FDT support would break existing 
>> Zynq
>> users.
> 
> The devices themselves are detected just fine and are likely operational. The
> MII busses are probably unterminated or pulled high/low which yields a ukphy
> detection on every available slot and constant PHY read/write timeouts. A 
> small
> sample from enabling both CGEM2 and CGEM3 on this custom board:
> 
> ukphy13:  none, 10baseT, 10baseT-FDX, 100baseTX, 100baseTX-FDX, 100baseT4,
> 1000baseSX, 1000baseSX-FDX, 1000baseT, 1000baseT-master, 1000baseT-FDX,
> 1000baseT-FDX-master, auto
> cgem3: phy read timeout: 0
> ukphy14:  PHY 14 on miibus0
> cgem3: phy write timeout: 0
> cgem3: phy read timeout: 0
> cgem3: phy read timeout: 0
> cgem3: phy read timeout: 0
> cgem3: phy read timeout: 0
> cgem3: phy read timeout: 0
> cgem3: phy read timeout: 0
> 
> ...
> 
> cgem3: phy read timeout: 0

That is noisy and painful.

> This type of output reached ukphy31 before I was able to run the shutdown
> command with 50+ PHY read/write errors and more continuing afterward. I 
> imagine
> it's worse if I enable all 4 interfaces.

The output is from the cgem driver which is good because it is localised. I
agree we need something to control what is enabled and I prefer we avoid special
build options, I will explain why I think this later. We can:

1. Add FDT support. This is something I would like to avoid as it adds an extra
layer of dependency and it complicates backwards compatibility for existing
users. I however wonder if this is just me and it is something that will be
needed more and more and cannot be avoided.

2. Add a weak call that is RTEMS and BSP specific to return a probe state. The
default state could be enabled and the tests provide something from the network
config. Not a great solution but it is simple and cheap to implement. The driver
already has a weak call to set the reference clock.

3. Try and detect a PHY in the probe? I am not sure if that is easy or hard. If
read works maybe that is something that may be suitable.


I would prefer we avoid special build options for BSPs. It is easy for us as
developers to make these changes and build a specific RTEMS plus we can be a
little narrow in our focus when delivering something for a client in the
defaults we select. Specific builds of a BSP to configure options becomes hard
for users where they have a few Zynq or ZynqMP designs all running RTEMS and
each with a different mix of network interfaces. This creates a complex set of
build options for common application code plus more configuration items to 
control.

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

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

2021-07-07 Thread Chris Johns
On 7/7/21 11:37 pm, Ryan Long wrote:
> I'll get those pointers changed to references, and remove the whitespace 
> changes. Is there a particular reason to not use '\n' instead of std::endl? 

Awesome and thanks.

> I read that std::endl is slower since it's flushing the buffer each time that 
> it is used.

The std::endl is platform independent so language implementers can match it to
the platform the code is being built for. It is similar to os.linesep in python
and why that should be used there.

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


Re: [PATCH rtems-libbsd] rtemsbsd: Use a separate header for test devices

2021-07-06 Thread Chris Johns
On 7/7/21 12:03 pm, Kinsey Moore wrote:
> On 7/6/2021 20:57, Chris Johns wrote:
>> On 7/7/21 11:05 am, Kinsey Moore wrote:
>>> The need for the difference on ZynqMP is that there are 4 different CGEM
>>> interfaces of which dev boards primarily make use of CGEM3.
>> RTEMS_BSD_DRIVER_XILINX_ZYNQMP_CGEM0(ZYNQMP_IRQ_ETHERNET_0);
>> RTEMS_BSD_DRIVER_XILINX_ZYNQMP_CGEM1(ZYNQMP_IRQ_ETHERNET_1);
>> RTEMS_BSD_DRIVER_XILINX_ZYNQMP_CGEM2(ZYNQMP_IRQ_ETHERNET_2);
>> RTEMS_BSD_DRIVER_XILINX_ZYNQMP_CGEM3(ZYNQMP_IRQ_ETHERNET_3);
>>
>> ?
> 
> Yes, this does technically work

Hmm, I suggest this is what we should support as a default.

> if you can read the shell output past the log
> spam. The other interfaces trying and failing to come up throw a gargantuan
> amount of messages to the console.

Why do these interfaces fail to initialise? What are the errors?

The removed probe check is based around FDT and so I suspect is the reason
Sebastian suggested FDT support. Needing FDT support would break existing Zynq
users.

Maybe we need something more in the probe conditional for RTEMS to see if clocks
or other related pieces of the hardware are set up? There maybe a relationship
between the FSBL and the errors you are seeing.

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


Re: [PATCH rtems-libbsd] rtemsbsd: Use a separate header for test devices

2021-07-06 Thread Chris Johns
On 7/7/21 11:05 am, Kinsey Moore wrote:
> The need for the difference on ZynqMP is that there are 4 different CGEM
> interfaces of which dev boards primarily make use of CGEM3. 

RTEMS_BSD_DRIVER_XILINX_ZYNQMP_CGEM0(ZYNQMP_IRQ_ETHERNET_0);
RTEMS_BSD_DRIVER_XILINX_ZYNQMP_CGEM1(ZYNQMP_IRQ_ETHERNET_1);
RTEMS_BSD_DRIVER_XILINX_ZYNQMP_CGEM2(ZYNQMP_IRQ_ETHERNET_2);
RTEMS_BSD_DRIVER_XILINX_ZYNQMP_CGEM3(ZYNQMP_IRQ_ETHERNET_3);

?

> I have a custom
> board in hand that uses CGEM2 which I can't run the network tests on without
> modifying nexus-devices.h manually.

NET_CFG_INTERFACE_0 "cgem2"

> Zynq7000 boards that use CGEM1 instead of
> CGEM0 could make use of the same mechanism for testing.

NET_CFG_INTERFACE_0 "cgem1"

FYI I am not sure how I will solve testing the MRMAC on the Versal chip. It may
require something extra and new to be done. What that is I do not know as the
MRMAC and the Versal presents a new level of complication.

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


Re: [PATCH] covoar: Fix errors building on FreeBSD and clang

2021-07-06 Thread Chris Johns
On 3/7/21 5:56 am, Alex White wrote:
> On Wed, Jun 30, 2021 at 11:40 PM  wrote:
>>
>> From: Chris Johns 
>>
>> - The member variable `path_` cannot be a reference and initialised to
>>   a const char* type input. To do so would require there is a temporary with
>>   an unspecified life time.
>> ---
>>  tester/covoar/AddressToLineMapper.h | 2 +-
>>  tester/covoar/Target_aarch64.h      | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tester/covoar/AddressToLineMapper.h
> b/tester/covoar/AddressToLineMapper.h
>> index 88bf475..c78aef7 100644
>> --- a/tester/covoar/AddressToLineMapper.h
>> +++ b/tester/covoar/AddressToLineMapper.h
>> @@ -84,7 +84,7 @@ namespace Coverage {
>>       *  An iterator pointing to the location in the set that contains the
>>       *  source file path of the address.
>>       */
>> -    const std::string& path_;
>> +    const std::string path_;
> 
> Ryan alerted me about this issue a couple weeks back. This patch would fix the
> problem, but it would lead to a second copy of every file path string being
> stored in memory. This would also take away the usefulness of the set of file
> path strings maintained by the AddressLineRange class.
> 
> Instead, I propose we change the empty SourceLine constructor to take a `const
> std::string&`. This would allow the addition of something like this to the top
> of AddressToLineMapper::getSource():
> const std::string unknown = "unknown";
> const SourceLine default_sourceline = SourceLine(unknown);
> 
> That should eliminate the issue and preserve the memory conservation efforts 
> of
> the original design.

Yes it would but for some reason, that I cannot put my finger on, it seems like
a breach of boundaries between classes.

How much data are we talking about? Are you able to see the memory foot print
with the strings being contained vs what you have now?

If the figures show the strings need to be shared to avoid a memory blow out
would a std::shared_ptr be something to look at where all
references are a shared pointer? A shared pointer means any changes do not flow
from one class to other.

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


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

2021-07-06 Thread Chris Johns
On 7/7/21 1:53 am, Ryan Long wrote:
> Made formatting consistent and converted C code to C++.
> ---
>  tester/covoar/ReportsBase.cc |  525 +--
>  tester/covoar/ReportsBase.h  |  156 +++---
>  tester/covoar/ReportsHtml.cc | 1183 
> +-
>  tester/covoar/ReportsHtml.h  |  141 ++---
>  tester/covoar/ReportsText.cc |  348 ++---
>  tester/covoar/ReportsText.h  |   75 ++-
>  6 files changed, 1041 insertions(+), 1387 deletions(-)
> 
> diff --git a/tester/covoar/ReportsBase.cc b/tester/covoar/ReportsBase.cc
> index 328980d..3041df2 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,10 +23,12 @@
>  
>  namespace Coverage {
>  
> -ReportsBase::ReportsBase( time_t timestamp, std::string symbolSetName ):
> -  reportExtension_m(""),
> -  symbolSetName_m(symbolSetName),
> -  timestamp_m( timestamp )
> +ReportsBase::ReportsBase(
> +  time_t timestamp,
> +  const std::string& symbolSetName
> +): reportExtension_m( "" ),
> +   symbolSetName_m( symbolSetName ),
> +   timestamp_m(  timestamp  )
>  {
>  }
>  
> @@ -31,14 +36,14 @@ ReportsBase::~ReportsBase()
>  {
>  }
>  
> -FILE* ReportsBase::OpenFile(
> -  const char* const fileName,
> -  const char* const symbolSetName
> +std::ofstream* ReportsBase::OpenFile(

A raw pointer anywhere is a possible source of a leak and with exceptions it is
hard to track all possible paths.

> +  const std::string& fileName,
> +  const std::string& symbolSetName

Would having ...

 std::ostream& aFile

as a referenced arguement work?

>  )
>  {
> -  int  sc;
> -  FILE*aFile;
> -  std::string  file;
> +  intsc;
> +  std::ofstream* aFile = new std::ofstream;

Please consider std::shared_ptr<> or std::unique_ptr<> for pointers or just
avoid using them which I find simpler.

> +  std::stringfile;
>  
>std::string symbolSetOutputDirectory;
>rld::path::path_join(
> @@ -51,136 +56,110 @@ FILE* ReportsBase::OpenFile(
>  #ifdef _WIN32
>sc = _mkdir( symbolSetOutputDirectory );
>  #else
> -  sc = mkdir( symbolSetOutputDirectory.c_str(),0755 );
> +  sc = mkdir( symbolSetOutputDirectory.c_str(), 0755 );
>  #endif
> -  if ( (sc == -1) && (errno != EEXIST) ) {
> -fprintf(
> -  stderr,
> -  "Unable to create output directory %s\n",
> -  symbolSetOutputDirectory.c_str()
> -);
> +  if ( ( sc == -1 ) && ( errno != EEXIST ) ) {
> +std::cerr << "Unable to create output directory "
> +  << symbolSetOutputDirectory << "\n";

Please do not use "\n", please use std::endl.

Why not throw an exception?

>  return NULL;

NULL should be nullptr in C++ but if this becomes 'void' it can be removed.

>}
>  
>file = symbolSetOutputDirectory;
> -  rld::path::path_join(file, fileName, file);
> +  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 << "\n";
>}
> +
>return aFile;
>  }
>  
> -void ReportsBase::WriteIndex(
> -  const char* const fileName
> -)
> +void ReportsBase::WriteIndex( const std::string& fileName )
>  {
>  }
>  
> -FILE* ReportsBase::OpenAnnotatedFile(
> -  const char* const fileName
> -)
> +std::ofstream* ReportsBase::OpenAnnotatedFile( const std::string& fileName )

.. and here ..

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

... and here ...

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

Re: [PATCH rtems-libbsd] rtemsbsd: Use a separate header for test devices

2021-07-06 Thread Chris Johns
On 7/7/21 5:46 am, Kinsey Moore wrote:
> This is an alternate patch to solve the issue of test-related information 
> being included in an installed application-targeted header.

Why is the ZYNQMP different to the zync BSP? I have been running the tests on
the zynq for years and the CGEM hard IP is also almost the same.

As I said in the other thread I have lost the reason for needing this special
case for the ZYNQMP.

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


Re: [PATCH] waf: Ensure network-config.h gets installed

2021-07-06 Thread Chris Johns
On 6/7/21 11:52 pm, Kinsey Moore wrote:
> On 7/6/2021 04:45, Chris Johns wrote:
>> On 3/7/21 11:16 am, Kinsey Moore wrote:
>>> network-config.h is now required for application compilation when using
>>> nexus-devices.h. This makes sure that it gets installed to resolve build
>>> errors.
>> I think it is a mistake to install this file. It is an internal file 
>> generated
>> by config.inc to ease creating the tests.
>>
>> I am concerned the internals of how tests are built will leaking to 
>> applications
>> and this creates a new set interfaces.
>>
>> I need to look at Peter's question that created this request. The preferred 
>> way
>> of doing this is to use rc.conf or craft your set of calls using BSd 
>> interfaces.
>>
>> Chris
> 
> I had that thought as well which leads me to the question: Is nexus-devices.h
> meant for test configuration or for application usage?

It is but indirectly used. See rtemsbsd/include/machine/rtems-bsd-config.h. All
I do in an application is:

/*
 * Configure LibBSD.
 */
#define RTEMS_BSD_CONFIG_DOMAIN_PAGE_MBUFS_SIZE (32 * 1024 * 1024)
#define RTEMS_BSD_CONFIG_BSP_CONFIG
#define RTEMS_BSD_CONFIG_INIT

#include 

int start(int iface)
{
  rtems_bsd_initialize();
  FILE* rc_conf = fopen("/etc/rc.conf", "w");
  fprintf(rc_conf, "hostname=\"test\"\n");
  fprintf(rc_conf, "ifconfig_cgem%d=\"DHCP rxcsum txcsum\"\n", iface);
  fclose(rc_conf);
  return rtems_bsd_run_etc_rc_conf(30, true);
}

> It is very clearly being used by applications as a place to store default
> configurations that should work for most hardware and I'm not sure we can (or
> should) change that.

You can provide special cases in your own code, for example in the section
above. his may be needed for a PC BSP where the card in the PCI bus is not one
listed.

> It would likely also be painful to change and raise the
> barrier to entry. On the other hand, recent conversations on this subject have
> pointed me toward nexus-devices.h being primarily for test usage while
> applications should provide their own set of definitions similar to 
> nexus-devices.h

The including of network-config.h is a mistake. A grep shows the
network-config.h is only included by the tests.

> Maybe it would be better to have a test-specific include that goes into all
> tests and supplements nexus-devices.h. This would force things to be left out 
> of
> the default-application-configuration-style nexus-devices.h to accomodate test
> needs, but would still provide a good starter configuration for applications.

I am not sure. What is in nexus-devices.h you need network-config.h for? I have
lost track of the problem you are wanting to solve?

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


Re: Is rtems/bsd/test/network-config.h supposed to be installed?

2021-07-06 Thread Chris Johns
> On 3 Jul 2021, at 5:14 am, Peter Dufault  wrote:
> 
> I updated my libbsd today and an application is failing to build because it 
> can't find the include file "rtems/bsd/test/network-config.h".  It was added 
> yesterday to "rtemsbsd/include/bsp/nexus-devices.h".  "nexus-devices.h" the 
> only file outside of the testsuite directory that includes 
> "network-config.h", and that network-config.h file is in .gitignore.  Or am I 
> missing a build step?

I think including it in a user facing header is a mistake. It is a tool to 
handle the tests that are internal to libbsd.

Chris

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

Re: [PATCH] waf: Ensure network-config.h gets installed

2021-07-06 Thread Chris Johns
On 3/7/21 11:16 am, Kinsey Moore wrote:
> network-config.h is now required for application compilation when using
> nexus-devices.h. This makes sure that it gets installed to resolve build
> errors.

I think it is a mistake to install this file. It is an internal file generated
by config.inc to ease creating the tests.

I am concerned the internals of how tests are built will leaking to applications
and this creates a new set interfaces.

I need to look at Peter's question that created this request. The preferred way
of doing this is to use rc.conf or craft your set of calls using BSd interfaces.

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


Re: [PATCH rtems-libbsd] rtembsd: Use config.inc to control ZynqMP ethernet

2021-06-30 Thread Chris Johns
Looks good and thanks.

Chris

On 1/7/21 6:11 am, Kinsey Moore wrote:
> This alters the selection of the 4 Cadence GEM interfaces on the Zynq
> Ultrascale+ MPSoC BSP to be provided by config.inc instead of being
> provided by options in the RTEMS BSP itself since those options appear
> to be dead code when not used in conjunction with LibBSD.
> ---
>  rtemsbsd/include/bsp/nexus-devices.h | 9 +
>  testsuite/include/rtems/bsd/test/network-config.h.in | 8 
>  waf_libbsd.py| 4 +++-
>  3 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/rtemsbsd/include/bsp/nexus-devices.h 
> b/rtemsbsd/include/bsp/nexus-devices.h
> index 5c1bab42..cbb3f48b 100644
> --- a/rtemsbsd/include/bsp/nexus-devices.h
> +++ b/rtemsbsd/include/bsp/nexus-devices.h
> @@ -38,6 +38,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  
> @@ -115,16 +116,16 @@ RTEMS_BSD_DRIVER_XILINX_ZYNQMP_SLCR;
>   * CGEM3 is used for LibBSD because all Zynq Ultrascale+ MPSoC dev boards 
> treat
>   * the highest-mapped CGEM as the primary interface.
>   */
> -#if BSP_XILINX_ZYNQMP_USE_CGEM0
> +#if NET_CFG_ZYNQMP_USE_CGEM0 == '1'
>  RTEMS_BSD_DRIVER_XILINX_ZYNQMP_CGEM0(ZYNQMP_IRQ_ETHERNET_0);
>  #endif
> -#if BSP_XILINX_ZYNQMP_USE_CGEM1
> +#if NET_CFG_ZYNQMP_USE_CGEM1 == '1'
>  RTEMS_BSD_DRIVER_XILINX_ZYNQMP_CGEM1(ZYNQMP_IRQ_ETHERNET_1);
>  #endif
> -#if BSP_XILINX_ZYNQMP_USE_CGEM2
> +#if NET_CFG_ZYNQMP_USE_CGEM2 == '1'
>  RTEMS_BSD_DRIVER_XILINX_ZYNQMP_CGEM2(ZYNQMP_IRQ_ETHERNET_2);
>  #endif
> -#if BSP_XILINX_ZYNQMP_USE_CGEM3
> +#if NET_CFG_ZYNQMP_USE_CGEM3 == '1'
>  RTEMS_BSD_DRIVER_XILINX_ZYNQMP_CGEM3(ZYNQMP_IRQ_ETHERNET_3);
>  #endif
>  RTEMS_BSD_DRIVER_E1000PHY;
> diff --git a/testsuite/include/rtems/bsd/test/network-config.h.in 
> b/testsuite/include/rtems/bsd/test/network-config.h.in
> index 39bb5388..da316e15 100755
> --- a/testsuite/include/rtems/bsd/test/network-config.h.in
> +++ b/testsuite/include/rtems/bsd/test/network-config.h.in
> @@ -64,4 +64,12 @@
>  
>  #define NET_CFG_GATEWAY_IP "@NET_CFG_GATEWAY_IP@"
>  
> +#define NET_CFG_ZYNQMP_USE_CGEM0 '@NET_CFG_ZYNQMP_USE_CGEM0@'
> +
> +#define NET_CFG_ZYNQMP_USE_CGEM1 '@NET_CFG_ZYNQMP_USE_CGEM1@'
> +
> +#define NET_CFG_ZYNQMP_USE_CGEM2 '@NET_CFG_ZYNQMP_USE_CGEM2@'
> +
> +#define NET_CFG_ZYNQMP_USE_CGEM3 '@NET_CFG_ZYNQMP_USE_CGEM3@'
> +
>  #endif /* _RTEMS_BSD_TEST_NETWORK_CONFIG_H_ */
> diff --git a/waf_libbsd.py b/waf_libbsd.py
> index e7222a03..bb4182e3 100644
> --- a/waf_libbsd.py
> +++ b/waf_libbsd.py
> @@ -289,7 +289,9 @@ class Builder(builder.ModuleManager):
>(bld.env.NET_CONFIG))
>  tags = [
>  'NET_CFG_INTERFACE_0', 'NET_CFG_SELF_IP', 'NET_CFG_NETMASK',
> -'NET_CFG_PEER_IP', 'NET_CFG_GATEWAY_IP'
> +'NET_CFG_PEER_IP', 'NET_CFG_GATEWAY_IP',
> +'NET_CFG_ZYNQMP_USE_CGEM0', 'NET_CFG_ZYNQMP_USE_CGEM1',
> +'NET_CFG_ZYNQMP_USE_CGEM2', 'NET_CFG_ZYNQMP_USE_CGEM3'
>  ]
>  try:
>  net_cfg_lines = open(bld.env.NET_CONFIG).readlines()
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: [PATCH] defaults.mc: Remove any checks for objdump and objcopy

2021-06-30 Thread Chris Johns
On 1/7/21 12:31 am, Joel Sherrill wrote:
> On Wed, Jun 30, 2021 at 3:28 AM  wrote:
>>
>> From: Chris Johns 
>>
>> - FreeBSD is removing any dependence on binutils and release 13
>>   has remove objdump. This is fine as we build our own version.
> 
> removed. :)

Thanks.

> 
>> ---
>>  source-builder/defaults.mc | 5 ++---
>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/source-builder/defaults.mc b/source-builder/defaults.mc
>> index 8ed7003..98775e8 100644
>> --- a/source-builder/defaults.mc
>> +++ b/source-builder/defaults.mc
>> @@ -184,13 +184,12 @@ __mkdir: exe, required, '/bin/mkdir'
>>  __mkdir_p:   exe, none, '/bin/mkdir -p'
>>  __mv:exe, required, '/bin/mv'
>>  __nm:exe, required, '/usr/bin/nm'
>> -__objcopy:   exe, optional, '/usr/bin/objcopy'
>> -__objdump:   exe, optional, '/usr/bin/objdump'
>> +__objcopy:   exe, none, '/usr/bin/objcopy'
>> +__objdump:   exe, none, '/usr/bin/objdump'
> 
> Looks like you are also removing objcopy.
> 
> Why would we have ever needed the native versions of either?

We should not. It is hang over from the very start of the RSB when I translated
across the spec file defaults and that is focused on native build.

> And what's the alternative on FreeBSD if someone did need that
> functionality?

The maintainer doing the work put in the 13 release notes you use readelf and/or
llvm-objdump. I should point out this is removed from base so you can still
install binutils as a package using pkg.

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


Re: [PATCH] bsps/zynqmp: Allow any or all CGEMs to be enabled

2021-06-29 Thread Chris Johns
On 30/6/21 2:19 am, Kinsey Moore wrote:
> On 6/29/2021 11:10, Gedare Bloom wrote:
>> On Tue, Jun 29, 2021 at 6:34 AM Kinsey Moore  
>> wrote:
>>> I suppose this could have been configured in libbsd (possibly
>>> config.inc) instead if that's what you're getting at. The overall goal
>>> is to be able to run the tests that use the network on any one of the
>>> board variants that this BSP supports. My takeaway from the earlier
>>> conversation on the list is that this was the preferred method of
>>> switching the ethernet interface for those tests.
>>>
>> I acked this patch, and then I also dug into the cgem code. It seems
>> we should leave the decision of which GEM to use to the application.
>> This patch and way of doing that accomplishes the configuration by the
>> application as a BSP option. However, there's no code in the BSP that
>> actually gets controlled by the option. So, I think we should probably
>> find a better way to allow the application to select which GEM it
>> uses, and to provide the glue in the BSP to allow all the possible
>> GEMs to be available.
> 
> This patch was primarily targeted at the tests themselves. The application can
> always provide its own nexus definitions instead of using the ones baked into
> libbsd for testing (this was brought up in the initial thread where I asked
> about how this was typically done). Unfortunately, this is hard to do with the
> tests because the tests are the application and can't be changed for those
> overrides/alternatives.

The tests should be handled by config.inc as you said. If that support is not
good enough then we should address that.

The issue adding this type of support in BSPs is a possible dependence and that
means we can never change it. If we add things for down stream packages be it
libbsd or anything else we can never change it because we have to assume there
will always be a version of something somewhere that depends on it, eg bisect.

I suggest this be removed.

I know there are other BSP that have similar defines and we should address as
time permits.

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


Re: [PATCH] bsps/zynqmp: Allow any or all CGEMs to be enabled

2021-06-29 Thread Chris Johns
On 30/6/21 1:21 am, Gedare Bloom wrote:
> On Tue, Jun 29, 2021 at 7:50 AM Sebastian Huber
>  wrote:
>>
>> On 29/06/2021 15:48, Kinsey Moore wrote:
>>> On 6/9/2021 00:27, Sebastian Huber wrote:
 On 08/06/2021 22:18, Kinsey Moore wrote:
> Provide the options necessary to enable any combination of CGEM ethernet
> interfaces in LibBSD. The default is still CGEM3, so this should
> continue to operate as expected on typical Zynq Ultrascale+ MPSoC
> development hardware.

 An alternative to this configuration approach would be to enable the
 device tree support for this BSP.

>>> Sorry, I missed this earlier due to travel. Device tree support in
>>> libbsd for if_cgem.c is currently #ifdef'd out and has been since it was
>>> introduced. I had assumed that was the case for other FDT support as
>>> well based on the comments in that initial commit.
>>
>> The device tree support is disabled since the original Zynq BSP was
>> added before RTEMS had support for device trees in general. Nobody had
>> the time/budget/need to change the BSP to support device trees so far.
>>
> I'm not sure anyone has the time/budget yet either. FDT support is
> challenging for RTEMS because it requires a closer interaction between
> a bootloader and RTEMS, which raises legal/license compliance
> concerns.

Or RTEMS itself if linked into a single executable.

> Although I agree that FDT is a great solution, I'm not
> certain about the scope (or creep) it implies in terms of the
> relationship between RTEMS and Bootloaders/firmware, or whether we
> need to produce our own "mini bootloader" that deals with FDT support
> early in the RTEMS startup sequence.

I agree. RTEMS use to be a single binary image that contained everything and as
a configuration controlled item it was simple to manage. FDT and then boot
loaders that support FDT brings into RTEMS a range of new issues. Some users
completely ban GPL and uboot is GPL and then some FDT is licensed GPL. Licenses
aside users need to manage and configuration control 3 separate pieces. These
pieces all need to line up and that presents challengers to our project.

> Right now, the design eludes me, but I'm certain that requiring users
> to U-Boot for FDT support is not the best solution for RTEMS.

Yeap, it should be "a" solution and users should be to select what they want.

And finally from a developer point of view I understand uboot and FDT is an easy
and attractive solution but we as developers do not have to live with the flow
on issues these additions crate. As an open source project we need to consider
both positions.

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


Re: [PATCH] bsps/zynqmp: Allow any or all CGEMs to be enabled

2021-06-28 Thread Chris Johns
Hi,

I am just catching up and missed this one until I saw the patch was pushed. I am
sorry but I am confused by this patch.

Could someone please explain this reason for this addition to the BSP and
approach being taken? Is there something in the BSP that requires this
information be provided here?

Chris

On 9/6/21 6:18 am, Kinsey Moore wrote:
> Provide the options necessary to enable any combination of CGEM ethernet
> interfaces in LibBSD. The default is still CGEM3, so this should
> continue to operate as expected on typical Zynq Ultrascale+ MPSoC
> development hardware.
> ---
>  spec/build/bsps/aarch64/xilinx-zynqmp/grp.yml|  8 
>  .../bsps/aarch64/xilinx-zynqmp/optcgem0.yml  | 16 
>  .../bsps/aarch64/xilinx-zynqmp/optcgem1.yml  | 16 
>  .../bsps/aarch64/xilinx-zynqmp/optcgem2.yml  | 16 
>  .../bsps/aarch64/xilinx-zynqmp/optcgem3.yml  | 16 
>  5 files changed, 72 insertions(+)
>  create mode 100644 spec/build/bsps/aarch64/xilinx-zynqmp/optcgem0.yml
>  create mode 100644 spec/build/bsps/aarch64/xilinx-zynqmp/optcgem1.yml
>  create mode 100644 spec/build/bsps/aarch64/xilinx-zynqmp/optcgem2.yml
>  create mode 100644 spec/build/bsps/aarch64/xilinx-zynqmp/optcgem3.yml
> 
> diff --git a/spec/build/bsps/aarch64/xilinx-zynqmp/grp.yml 
> b/spec/build/bsps/aarch64/xilinx-zynqmp/grp.yml
> index 16e2b8a7e9..1b6b756912 100644
> --- a/spec/build/bsps/aarch64/xilinx-zynqmp/grp.yml
> +++ b/spec/build/bsps/aarch64/xilinx-zynqmp/grp.yml
> @@ -27,6 +27,14 @@ links:
>uid: optramori
>  - role: build-dependency
>uid: optclkuart
> +- role: build-dependency
> +  uid: optcgem0
> +- role: build-dependency
> +  uid: optcgem1
> +- role: build-dependency
> +  uid: optcgem2
> +- role: build-dependency
> +  uid: optcgem3
>  - role: build-dependency
>uid: ../../optconminor
>  - role: build-dependency
> diff --git a/spec/build/bsps/aarch64/xilinx-zynqmp/optcgem0.yml 
> b/spec/build/bsps/aarch64/xilinx-zynqmp/optcgem0.yml
> new file mode 100644
> index 00..fc878fda60
> --- /dev/null
> +++ b/spec/build/bsps/aarch64/xilinx-zynqmp/optcgem0.yml
> @@ -0,0 +1,16 @@
> +SPDX-License-Identifier: CC-BY-SA-4.0 OR BSD-2-Clause
> +actions:
> +- get-boolean: null
> +- env-enable: null
> +- define-condition: null
> +build-type: option
> +copyrights:
> +- Copyright (C) 2021 On-Line Applications Research
> +default: false
> +default-by-variant: []
> +description: |
> +  Enable support for CGEM0
> +enabled-by: true
> +links: []
> +name: BSP_XILINX_ZYNQMP_USE_CGEM0
> +type: build
> diff --git a/spec/build/bsps/aarch64/xilinx-zynqmp/optcgem1.yml 
> b/spec/build/bsps/aarch64/xilinx-zynqmp/optcgem1.yml
> new file mode 100644
> index 00..6d5096bbde
> --- /dev/null
> +++ b/spec/build/bsps/aarch64/xilinx-zynqmp/optcgem1.yml
> @@ -0,0 +1,16 @@
> +SPDX-License-Identifier: CC-BY-SA-4.0 OR BSD-2-Clause
> +actions:
> +- get-boolean: null
> +- env-enable: null
> +- define-condition: null
> +build-type: option
> +copyrights:
> +- Copyright (C) 2021 On-Line Applications Research
> +default: false
> +default-by-variant: []
> +description: |
> +  Enable support for CGEM1
> +enabled-by: true
> +links: []
> +name: BSP_XILINX_ZYNQMP_USE_CGEM1
> +type: build
> diff --git a/spec/build/bsps/aarch64/xilinx-zynqmp/optcgem2.yml 
> b/spec/build/bsps/aarch64/xilinx-zynqmp/optcgem2.yml
> new file mode 100644
> index 00..a8aca3ebbd
> --- /dev/null
> +++ b/spec/build/bsps/aarch64/xilinx-zynqmp/optcgem2.yml
> @@ -0,0 +1,16 @@
> +SPDX-License-Identifier: CC-BY-SA-4.0 OR BSD-2-Clause
> +actions:
> +- get-boolean: null
> +- env-enable: null
> +- define-condition: null
> +build-type: option
> +copyrights:
> +- Copyright (C) 2021 On-Line Applications Research
> +default: false
> +default-by-variant: []
> +description: |
> +  Enable support for CGEM2
> +enabled-by: true
> +links: []
> +name: BSP_XILINX_ZYNQMP_USE_CGEM2
> +type: build
> diff --git a/spec/build/bsps/aarch64/xilinx-zynqmp/optcgem3.yml 
> b/spec/build/bsps/aarch64/xilinx-zynqmp/optcgem3.yml
> new file mode 100644
> index 00..8275ad3440
> --- /dev/null
> +++ b/spec/build/bsps/aarch64/xilinx-zynqmp/optcgem3.yml
> @@ -0,0 +1,16 @@
> +SPDX-License-Identifier: CC-BY-SA-4.0 OR BSD-2-Clause
> +actions:
> +- get-boolean: null
> +- env-enable: null
> +- define-condition: null
> +build-type: option
> +copyrights:
> +- Copyright (C) 2021 On-Line Applications Research
> +default: true
> +default-by-variant: []
> +description: |
> +  Enable support for CGEM3
> +enabled-by: true
> +links: []
> +name: BSP_XILINX_ZYNQMP_USE_CGEM3
> +type: build
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: [PATCH] covoar/Explanations.cc: Remove unnecessary newline strip

2021-06-25 Thread Chris Johns
OK

On 25/6/21 1:40 pm, Alex White wrote:
> This corrects an issue where the starting line numbers of explanations
> had their last digit removed. This was caused by code left over for
> handling C file IO where the newline was included in the buffer.
> ---
>  tester/covoar/Explanations.cc | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/tester/covoar/Explanations.cc b/tester/covoar/Explanations.cc
> index 4d177eb..91e65f2 100644
> --- a/tester/covoar/Explanations.cc
> +++ b/tester/covoar/Explanations.cc
> @@ -55,7 +55,6 @@ namespace Coverage {
>  if (explain.fail()) {
>return;
>  }
> -inputBuffer[ strlen(inputBuffer) - 1] = '\0';
>  line++;
>} while ( inputBuffer[0] == '\0' );
>  
> @@ -80,7 +79,6 @@ namespace Coverage {
>   << "out of sync at the classification";
>  throw rld::error( what, "Explanations::load" );
>}
> -  inputBuffer[ strlen(inputBuffer) - 1] = '\0';
>e.classification = inputBuffer;
>line++;
>  
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: [PATCH v2] covoar/ Explanations.cc: Handle newline at end of file

2021-06-25 Thread Chris Johns
OK

On 25/6/21 1:37 pm, Alex White wrote:
> ---
>  tester/covoar/Explanations.cc | 24 +++-
>  1 file changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/tester/covoar/Explanations.cc b/tester/covoar/Explanations.cc
> index 1449fb2..4d177eb 100644
> --- a/tester/covoar/Explanations.cc
> +++ b/tester/covoar/Explanations.cc
> @@ -85,26 +85,24 @@ namespace Coverage {
>line++;
>  
>// Get the explanation
> -  while (1) {
> -explain.getline( inputBuffer, MAX_LINE_LENGTH );
> -// fprintf( stderr, "%d - %s\n", line, inputBuffer );
> -if (explain.fail()) {
> -  std::ostringstream what;
> -  what << "line " << line
> -   << "out of sync at the explanation";
> -  throw rld::error( what, "Explanations::load" );
> -}
> -inputBuffer[ strlen(inputBuffer) - 1] = '\0';
> +  for (std::string input_line; std::getline( explain, input_line ); ) {
>  line++;
>  
> -const char delimiter[4] = "+++";
> -if (!strncmp( inputBuffer, delimiter, 3 )) {
> +const std::string delimiter = "+++";
> +if (input_line.compare( delimiter ) == 0) {
>break;
>  }
>  // XXX only taking last line.  Needs to be a vector
> -e.explanation.push_back( inputBuffer );
> +e.explanation.push_back( input_line );
>}
>  
> +if (explain.fail()) {
> +  std::ostringstream what;
> +  what << "line " << line
> +   << "out of sync at the explanation";
> +  throw rld::error( what, "Explanations::load" );
> +}
> +
>// Add this to the set of Explanations
>set[ e.startingPoint ] = e;
>  }
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: BBB hello does not run...

2021-06-23 Thread Chris Johns
Hi Christian,

Thanks for the reply.

My uboot SD card does not have any FDT blobs so this is the reason for the
failure. We seem to now have a dependency on an FDT. I have played a bit and
found the cause of the error:

https://git.rtems.org/rtems/tree/bsps/shared/ofw/ofw.c#n86

I have no problem with the FDT being used in RTEMS but I do have some concerns
about how we handle things. A hard error here is a bit abrupt and maybe just not
having the drivers installed and available would be a better solution. An app
will fail when the driver does not exist. On the other hand a hard error is fine
if we packaged the FDT blob in the executable, something we currently do not do.

I think we should consider the single executable approach to handling FDT blobs,
that is embedding the blob into RTEMS. This is not something we can reach in a
simple single step plus we have the licensing issue but I feel we can make it
happen.

The first step I am looking at is the building of the FDT source with includes.
A command in the rtems-tool that is installed would be a good first step. The
FreeBSD script is a good base but it is all a bit awkward to get to and use.

Chris

On 23/6/21 5:10 pm, Christian Mauderer wrote:
> Hello Chris,
> 
> there is no new requirement that I know of. The driver should parse the same 
> FDT
> fields that have been parsed by libbsd earlier. It only want's to avoid the
> double initialization that had been done by RTEMS and libbsd.
> 
> But there is a simple method how we can find out whether it's FDT related:
> Please try the dtb that I normally use:
> 
>     https://nc.c-mauderer.de/index.php/s/JntMtLrE2GpH2Xf
> 
> That FDT is build from the FreeBSD sources from that revision:
> 
> 
> https://github.com/freebsd/freebsd-src/tree/19a6ceb89dbacf74697d493e48c388767126d418
> 
> 
> Note: I don't think that using that FDT is a solution. It's just to find out
> whether it's the problem. A solution would be to find a method how we can
> distribute matching FDTs with various licenses.
> 
> Best regards
> 
> Christian
> 
> On 23/06/2021 07:24, Chris Johns wrote:
>> I have bisect'ed the failure and this is the commit that is the problem:
>>
>> https://git.rtems.org/rtems/commit/?id=56074644a733ecc984722da2a1b61736275270c0
>>
>> Hm Is there a new requirement on the needed FDT for a 
>> beagleboneblack.
>>
>> Chris
>>
>>
>> On 23/6/21 2:52 pm, Chris Johns wrote:
>>> Hi,
>>>
>>> hello.exe is not running. Any hints?
>>>
>>> master at ...
>>>
>>> https://git.rtems.org/rtems/commit/?id=b47dbbc5f7c8518634c7c5fccd57d78c65444f2d
>>>
>>> config.ini:
>>> [DEFAULT]
>>> BUILD_TESTS = False
>>> RTEMS_DEBUG = True
>>> RTEMS_POSIX_API = True
>>>
>>> [arm/beagleboneblack]
>>> BUILD_TESTS = True
>>>
>>> output:
>>>
>>> RTEMS Beagleboard: am335x-based
>>>  ARM Debug: 0x4b141000
>>>
>>> *** FATAL ***
>>> fatal source: 1 (INTERNAL_ERROR_RTEMS_API)
>>> fatal code: 22 (0x0016)
>>> RTEMS version: 6.0.0.4977dc74c60e75b90fe8dd72e4dedafd55d70c73
>>> RTEMS tools: 10.3.1 20210409 (RTEMS 6, RSB
>>> 4e6dc6431435821a534da6307e72ecbd7e42b82a, Newlib 0c0f3df)
>>> executing thread ID: 0x089010001
>>> executing thread name: IDLE
>>>
>>> Chris
>>> ___
>>> devel mailing list
>>> devel@rtems.org
>>> http://lists.rtems.org/mailman/listinfo/devel
>>>
>>
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: rtems-docs: Update eclipse/rtems.rst?

2021-06-23 Thread Chris Johns
On 23/6/21 5:13 am, Raymond Garza wrote:
> While going through the docs to update everything for RTEMS 6, I’ve noticed 
> one
> particular file to be problematic. Before I do anything to it, I’d like to
> consult the mailing list.
> 
> In the rtems-docs repository, the eclipse -> rtems.rst file describes
> instructions to build a BSP in Eclipse. For reference, the resulting html page
> can be found at https://docs.rtems.org/branches/master/eclipse/rtems.html
> 
> 
> Now, with the introduction of RTEMS 6, all autoconf/make build instructions 
> are
> no longer supported. Waf is used instead, and Eclipse offers no support for 
> Waf
> builds. So, the majority of this article needs to be either replaced with
> instructions to hack a Waf build into Eclipse, or be deleted. Before I do
> anything, I’d like some opinions on the matter.

It would be nice to have some documentation on Eclipse however it is something
that takes some effort to maintain.

I say remove it and if someone wants to do this again there is a copy in the
repo history.

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


Re: BBB hello does not run...

2021-06-22 Thread Chris Johns
I have bisect'ed the failure and this is the commit that is the problem:

https://git.rtems.org/rtems/commit/?id=56074644a733ecc984722da2a1b61736275270c0

Hm Is there a new requirement on the needed FDT for a beagleboneblack.

Chris


On 23/6/21 2:52 pm, Chris Johns wrote:
> Hi,
> 
> hello.exe is not running. Any hints?
> 
> master at ...
> 
> https://git.rtems.org/rtems/commit/?id=b47dbbc5f7c8518634c7c5fccd57d78c65444f2d
> 
> config.ini:
> [DEFAULT]
> BUILD_TESTS = False
> RTEMS_DEBUG = True
> RTEMS_POSIX_API = True
> 
> [arm/beagleboneblack]
> BUILD_TESTS = True
> 
> output:
> 
> RTEMS Beagleboard: am335x-based
> ARM Debug: 0x4b141000
> 
> *** FATAL ***
> fatal source: 1 (INTERNAL_ERROR_RTEMS_API)
> fatal code: 22 (0x0016)
> RTEMS version: 6.0.0.4977dc74c60e75b90fe8dd72e4dedafd55d70c73
> RTEMS tools: 10.3.1 20210409 (RTEMS 6, RSB
> 4e6dc6431435821a534da6307e72ecbd7e42b82a, Newlib 0c0f3df)
> executing thread ID: 0x089010001
> executing thread name: IDLE
> 
> Chris
> ___
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


BBB hello does not run...

2021-06-22 Thread Chris Johns
Hi,

hello.exe is not running. Any hints?

master at ...

https://git.rtems.org/rtems/commit/?id=b47dbbc5f7c8518634c7c5fccd57d78c65444f2d

config.ini:
[DEFAULT]
BUILD_TESTS = False
RTEMS_DEBUG = True
RTEMS_POSIX_API = True

[arm/beagleboneblack]
BUILD_TESTS = True

output:

RTEMS Beagleboard: am335x-based
ARM Debug: 0x4b141000

*** FATAL ***
fatal source: 1 (INTERNAL_ERROR_RTEMS_API)
fatal code: 22 (0x0016)
RTEMS version: 6.0.0.4977dc74c60e75b90fe8dd72e4dedafd55d70c73
RTEMS tools: 10.3.1 20210409 (RTEMS 6, RSB
4e6dc6431435821a534da6307e72ecbd7e42b82a, Newlib 0c0f3df)
executing thread ID: 0x089010001
executing thread name: IDLE

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


Re: [PATCH 0/3] Simplify generic interrupt controller support

2021-06-18 Thread Chris Johns
On 18/6/21 5:20 pm, Sebastian Huber wrote:
> This patch set simplifies the generic interrupt controller support a bit to
> prepare for more complex follow up changes.

This change touches a large number of BSPs in an important area.

I spent some time this year fixing a 10+ year old IRQ bug related to these
defines in the MVME2700 BSP that was not easy to see and did not appear to be an
issue when reviewing the code and the changes to it. As a result I am wary of
changes in this area without suitable testing.

Could you please provide a list of the BSPs effected by this change?

Can you please indicate which BSPs you have tested the change on and if the
tests are on simulation or real hardware?

I do not expect us to have complete coverage however I do think we need a plan
that documents what we have tested, what we think is covered by other test
results and what is not tested. I also think we will all need to provide test
results on hardware we have.

How do we stage the changes so we can perform the testing?

Should we test all the changes as a set and bisect if we have an issue? It may
be faster if there are no issues.

What test or tests to we consider have to pass for the changes to be accepted?

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


Re: [PATCH] covoar/ Explanations.cc: Handle newline at end of file

2021-06-18 Thread Chris Johns
On 19/6/21 7:00 am, Alex White wrote:
> ---
>  tester/covoar/Explanations.cc | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tester/covoar/Explanations.cc b/tester/covoar/Explanations.cc
> index 1449fb2..64cad5c 100644
> --- a/tester/covoar/Explanations.cc
> +++ b/tester/covoar/Explanations.cc
> @@ -87,6 +87,9 @@ namespace Coverage {
>// Get the explanation
>while (1) {
>  explain.getline( inputBuffer, MAX_LINE_LENGTH );
> +if (explain.eof()) {
> +  break;
> +}
>  // fprintf( stderr, "%d - %s\n", line, inputBuffer );
>  if (explain.fail()) {
>std::ostringstream what;

Maybe the std::getline example with a C++11 range base loop would be a better
solution?

https://en.cppreference.com/w/cpp/string/basic_string/getline

:)

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


Re: New API directives to enable/disable interrupt vectors

2021-06-17 Thread Chris Johns
On 18/6/21 1:28 am, Gedare Bloom wrote:
> On Wed, Jun 16, 2021 at 11:40 PM Sebastian Huber
>  wrote:
>>
>> On 16/06/2021 20:36, Gedare Bloom wrote:
>>> Looks like the existing irq-extension.h uses 'vector', so
>>> rtems_interrupt_disable_vector() is a possibility, or else
>>> rtems_interrupt_controller_disable_vector() is more wordy but if we
>>> want to treat 'interrupt controller' as its own category of API
>>> separate from 'interrupt'.
>>
>> Yes, mixing "vector" into the name a good idea.  What about
>> rtems_interrupt_vector_enable() and rtems_interrupt_vector_disable()?
>>
>> I am not that fond of using "interrupt controller" since I don't want to
>> change the existing API and it would result in very long directive names.
>>
> I'm fine with the rtems_interrupt_vector_* to distinguish between the
> score/cpu level support, and the bsp/irq controllers. We already use
> this for example as "bsp_interrupt_vector_enable()". I think it will
> be great to lift this "bsp" interface up to the "rtems" level.

I have no objection to any of this but it raised my interest in what is the
whole rtems_interrupt_* API and how consistent it is ...

I have managed to filter the cpukit headers [1] to get:

rtems_interrupt_catch
rtems_interrupt_cause
rtems_interrupt_clear
rtems_interrupt_disable
rtems_interrupt_enable
rtems_interrupt_flash
rtems_interrupt_is_in_progress
rtems_interrupt_level_body

rtems_interrupt_local_disable
rtems_interrupt_local_enable

rtems_interrupt_handler_install
rtems_interrupt_handler_iterate
rtems_interrupt_handler_remove

rtems_interrupt_get_affinity
rtems_interrupt_set_affinity

rtems_interrupt_lock_acquire
rtems_interrupt_lock_acquire_isr
rtems_interrupt_lock_destroy
rtems_interrupt_lock_initialize
rtems_interrupt_lock_interrupt_disable
rtems_interrupt_lock_release
rtems_interrupt_lock_release_isr

rtems_interrupt_server_action_prepend
rtems_interrupt_server_create
rtems_interrupt_server_delete
rtems_interrupt_server_entry_destroy
rtems_interrupt_server_entry_initialize
rtems_interrupt_server_entry_move
rtems_interrupt_server_entry_submit
rtems_interrupt_server_handler_install
rtems_interrupt_server_handler_iterate
rtems_interrupt_server_handler_remove
rtems_interrupt_server_initialize
rtems_interrupt_server_move
rtems_interrupt_server_request_destroy
rtems_interrupt_server_request_initialize
rtems_interrupt_server_request_set_vector
rtems_interrupt_server_request_submit
rtems_interrupt_server_resume
rtems_interrupt_server_set_affinity
rtems_interrupt_server_suspend

The list does not capture things like rtems_interrupt_mask which is a typedef.

My concern is:

rtems_interrupt_disable
rtems_interrupt_enable

rtems_interrupt_local_disable
rtems_interrupt_local_enable

and then we add:

rtems_interrupt_vector_disable
rtems_interrupt_vector_enable

How do each of these enable/disable pairs interact with the other parts of the 
API?

This also seems to pull back to Sebastian's initial comment about the
enable/disable being occupied and I wonder if that concern highlights the need
for some cleaning in the API? It is does it may take a release or 2 to migrate
users but it may be worth it in the long run if the API is made clearer.

I am not across enough of the detail to comment specifically about this and I am
happy to see what others think and follow their lead.

Chris

[1] egrep -r 'rtems_interrupt_.*\(' cpukit/include | sed -e 's/.*
\(rtems_interrupt_.*\).*/\1/' | sed -e 's/(.*//' | grep '^rtems' | sort | uniq
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: [PATCH] covoar/CoverageMapBase: Use reference for iteration in validAddress

2021-06-17 Thread Chris Johns
Ok

On 18/6/21 7:13 am, Alex White wrote:
> ---
>  tester/covoar/CoverageMapBase.cc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tester/covoar/CoverageMapBase.cc 
> b/tester/covoar/CoverageMapBase.cc
> index 6ca5cf7..f0b5890 100644
> --- a/tester/covoar/CoverageMapBase.cc
> +++ b/tester/covoar/CoverageMapBase.cc
> @@ -121,7 +121,7 @@ namespace Coverage {
>  
>bool CoverageMapBase::validAddress( const uint32_t address ) const
>{
> -for ( auto r : Ranges )
> +for ( auto& r : Ranges )
>if (r.inside( address ))
>  return true;
>  return false;
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: [PATCH v2] covoar: Store address-to-line info outside of DWARF

2021-06-17 Thread Chris Johns
On 17/6/21 1:01 pm, Alex White wrote:
> So is this ok to push now that it is known to have an insignificant 
> performance
> impact or is a different approach still warranted?

OK to push.

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


Re: [rtems-tools commit] rtems-bsp-builder: Change to waf build system

2021-06-15 Thread Chris Johns
On 15/6/21 4:50 pm, Sebastian Huber wrote:
> On 15/06/2021 08:46, Chris Johns wrote:
>> If this is pushed through and the task it left for the "demand" queue it is
>> actually being placed on my queue because I am the one releasing RTEMS and I
>> think having these sorts of things resolved at release time is not great 
>> because
>> there is never enough testing.
> 
> I don't think this would have happened. 

Thanks and I hope it does not.

> I usually fix things which I broke (in
> this case the removal of the old build system). I just don't like it to waste 
> my
> time on things which have no future and this is definitely the old build 
> system.

I also see no future holding onto the old build system and agree it is a waste
of time. I am not blocking the progress just bringing into focus what we have to
sort to out and what happens when it is not done. Also I do not have a workable
solution for building a kernel with the RSB and I would love to have the time to
work on one.

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


Re: [rtems-tools commit] rtems-bsp-builder: Change to waf build system

2021-06-15 Thread Chris Johns
On 15/6/21 4:28 pm, Sebastian Huber wrote:
> On 15/06/2021 07:50, Chris Johns wrote:
>> On 15/6/21 3:44 pm, Sebastian Huber wrote:
>>> On 15/06/2021 06:03, Chris Johns wrote:
>>>> On 8/6/21 10:23 pm, Sebastian Huber wrote:
>>>>> On 08/06/2021 14:08, Joel Sherrill wrote:
>>>>>> Is the kernel rsb recipe fixed yet? That was a blocker.
>>>>> Is this really a blocker? Can't this be done on demand when someone wants 
>>>>> to
>>>>> update them?
>>>> This depends on our shared view of supporting an eco-system.
>>>>
>>>> Support was added before we moved the build system to waf to build the
>>>> kernel as
>>>> part of a vertical software stack and this I consider important.
>>> I didn't say the contrary. What I don't agree with is the work package 
>>> ordering
>>> and dependency chain. The time I wasted on keeping the build system zombie 
>>> more
>>> or less alive I could have spent on updating the RSB.
>> No doubt. I am responding to your comment that this task can get done on 
>> demand.
>> How does the ecosystem view of this fit into your "demand" driven view?
> 
> On demand means if someone wants update the RSB, the files are converted to 
> the
> new build system. The files using the old build system still work since they 
> use
> commits which have the old build system included. At least this is how I think
> the RSB works.

The ecosystem is a backwards compatible means to do something in RTEMS. Users
can depend on the interface provided to work. What and how is for us to solve as
long we maintain the interface. We are currently providing command line
interfaces and the RSB is a central part. Vertical software stacks also form an
important part for the same reasons and we should be moving to this being a way
users of releases work. As a result I see the RSB being able to build a kernel
as important to us being able to make a release. My hope is this falls under the
description of "demand".

If this is pushed through and the task it left for the "demand" queue it is
actually being placed on my queue because I am the one releasing RTEMS and I
think having these sorts of things resolved at release time is not great because
there is never enough testing.

I would rather we discuss and understand these things up front rather than 
later.

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


Re: [rtems-tools commit] rtems-bsp-builder: Change to waf build system

2021-06-14 Thread Chris Johns
On 15/6/21 3:44 pm, Sebastian Huber wrote:
> On 15/06/2021 06:03, Chris Johns wrote:
>> On 8/6/21 10:23 pm, Sebastian Huber wrote:
>>> On 08/06/2021 14:08, Joel Sherrill wrote:
>>>> Is the kernel rsb recipe fixed yet? That was a blocker.
>>> Is this really a blocker? Can't this be done on demand when someone wants to
>>> update them?
>> This depends on our shared view of supporting an eco-system.
>>
>> Support was added before we moved the build system to waf to build the 
>> kernel as
>> part of a vertical software stack and this I consider important.
> 
> I didn't say the contrary. What I don't agree with is the work package 
> ordering
> and dependency chain. The time I wasted on keeping the build system zombie 
> more
> or less alive I could have spent on updating the RSB.

No doubt. I am responding to your comment that this task can get done on demand.
How does the ecosystem view of this fit into your "demand" driven view?

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


Re: [rtems-tools commit] rtems-bsp-builder: Change to waf build system

2021-06-14 Thread Chris Johns
On 8/6/21 10:23 pm, Sebastian Huber wrote:
> On 08/06/2021 14:08, Joel Sherrill wrote:
>> Is the kernel rsb recipe fixed yet? That was a blocker.
> 
> Is this really a blocker? Can't this be done on demand when someone wants to
> update them?

This depends on our shared view of supporting an eco-system.

Support was added before we moved the build system to waf to build the kernel as
part of a vertical software stack and this I consider important.

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


Re: [PATCH rtems 2/2] bsps/imxrt: Simplify linkcmds and make it flexible

2021-06-09 Thread Chris Johns
On 9/6/21 5:05 pm, Christian Mauderer wrote:
> On 09/06/2021 01:52, Chris Johns wrote:
>> On 8/6/21 8:26 pm, Sebastian Huber wrote:
>>> On 08/06/2021 05:07, Chris Johns wrote:
>>>> On 7/6/21 6:40 pm, Christian Mauderer wrote:> I think the Options don't 
>>>> need
>>>> documentation changes because the options in the
>>>>> waf based build system are now documented directly in the yaml files and
>>>>> printed
>>>>> if you generate the default config.
>>>> Hmm I am not sure I agree with the premise the YAML is the documentation. 
>>>> The
>>>> user manual exists for this purpose and user wanting to explore RTEMS would
>>>> look
>>>> there rather than cloning the repo, running commands etc.
>>>>
>>>> I accept the current defaults etc work flow is a good place to start for 
>>>> the
>>>> waf
>>>> conversion project but we need to do a lot more to make accessing the YAML
>>>> easier. The wscript contains the only rtems.git repo means to read the 
>>>> YAML and
>>>> the pickled data and that limits how we can change and evolve this.
>>>
>>> We could use rtems-central to generate sections for the user manual from the
>>> build specification. This would be similar to the generation of the 
>>> glossaries,
>>> application configuration option documentation, and the directive 
>>> documentation.
>>
>> Of course we could use it and there are a number of other possible ways we do
>> this as well without that repo. The rtems-central repo and work flows have 
>> clear
>> and defined boundary we have put in place for very good reasons. A move that 
>> way
>> would be considered creep.
>>
>> Chris
> 
> Hello Chris,
> 
> Gedare mentioned that he is OK with the one-line-command in the manual that
> shows the user how he gets the options (see
> https://lists.rtems.org/pipermail/devel/2021-June/067627.html). Is that OK for
> you too? If yes, that would be my preferred solution and you can ignore the
> remaining part of the mail.

Yes this is fine. I am happy with something until we can automate.

> If not: I'm OK to automate or at least semi-automate the process of generating
> that part of the manual because I think it's an improvement. I'm not OK with a
> manual step because it increases the maintenance effort and the time I have to
> invest on _every_ commit that is vaguely similar to this one. That would mean
> more frustration for me and less time that I have for other stuff.
> 
> I can accept that you don't want that kind of automation in rtems-central. 
> What
> would be your preferred solution and location for that kind of automation?

Honestly I am not sure. Nothing is a perfect fit and it is an exercise in the
less of the evils. I would like to see the python support used in `wscript`
being made available as a python module in rtems.git but Sebastian says there is
support in rtems-central but that is still not an option. Until I have time to
look more I do not know.

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


Re: [PATCH v2 2/2] i386: Remove unneeded include header files

2021-06-09 Thread Chris Johns
On 8/6/21 11:52 pm, jan.som...@dlr.de wrote:
> I would also like to backport this to 5-freebsd-12 to solve this issue there 
> as well.
> I created a corresponding ticket: https://devel.rtems.org/ticket/4452#ticket

OK for 5.

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


Re: [PATCH rtems 2/2] bsps/imxrt: Simplify linkcmds and make it flexible

2021-06-08 Thread Chris Johns
On 8/6/21 8:26 pm, Sebastian Huber wrote:
> On 08/06/2021 05:07, Chris Johns wrote:
>> On 7/6/21 6:40 pm, Christian Mauderer wrote:> I think the Options don't need
>> documentation changes because the options in the
>>> waf based build system are now documented directly in the yaml files and 
>>> printed
>>> if you generate the default config.
>> Hmm I am not sure I agree with the premise the YAML is the documentation. The
>> user manual exists for this purpose and user wanting to explore RTEMS would 
>> look
>> there rather than cloning the repo, running commands etc.
>>
>> I accept the current defaults etc work flow is a good place to start for the 
>> waf
>> conversion project but we need to do a lot more to make accessing the YAML
>> easier. The wscript contains the only rtems.git repo means to read the YAML 
>> and
>> the pickled data and that limits how we can change and evolve this.
> 
> We could use rtems-central to generate sections for the user manual from the
> build specification. This would be similar to the generation of the 
> glossaries,
> application configuration option documentation, and the directive 
> documentation.

Of course we could use it and there are a number of other possible ways we do
this as well without that repo. The rtems-central repo and work flows have clear
and defined boundary we have put in place for very good reasons. A move that way
would be considered creep.

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


Re: [PATCH rtems 2/2] bsps/imxrt: Simplify linkcmds and make it flexible

2021-06-08 Thread Chris Johns
On 8/6/21 7:08 pm, Christian Mauderer wrote:
> Hello Chris,
> 
> On 08/06/2021 05:07, Chris Johns wrote:
>> On 7/6/21 6:40 pm, Christian Mauderer wrote:> I think the Options don't need
>> documentation changes because the options in the
>>> waf based build system are now documented directly in the yaml files and 
>>> printed
>>> if you generate the default config.
>>
>> Hmm I am not sure I agree with the premise the YAML is the documentation. The
>> user manual exists for this purpose and user wanting to explore RTEMS would 
>> look
>> there rather than cloning the repo, running commands etc.
> 
> The alternative would be to duplicate documentation of the BSP options in the
> user manual. And (manual) duplication is always a bad idea if you ask me. Or
> move the description from the yaml to the documentation entirely which means
> that you need to pick the correct version of the documentation for the 
> sources.
> It also means that you have to look up the meaning of each option in the right
> version of the manual instead of in the default config.

At the moment users need to work too hard to find it. Maybe manual changes can
be added if someone wants to added them until a better solution is provided. I
think it is better that blocking changes until it is.
> I think the only reasonable option (with the current structure) would be to
> somehow automatically generate that documentation part. For example by 
> creating
> the default BSP config.ini for each BSP and adding it to the user manual. But
> that has to be an automatic process. Otherwise it will be out of date right 
> from
> the beginning.

 or something else. I don't think we need to solve the problem here and now
as we do not have enough support in rtems.git to handle it.

I do not want to depend on rtems-central at this point in time. It needs to
mature, it needs to exist for a while and be supported before we can integrate 
it.

>> I accept the current defaults etc work flow is a good place to start for the 
>> waf
>> conversion project but we need to do a lot more to make accessing the YAML
>> easier. The wscript contains the only rtems.git repo means to read the YAML 
>> and
>> the pickled data and that limits how we can change and evolve this.
>>
> 
> We could merge documentation and code again. From my point of view that would
> make it simpler to add documentation for new features right when they are
> implemented and there would be always the right version for a source commit. I
> don't really know the reason why it was split up in 2015 / 2016 so we would 
> have
> to look that up before discussing further into that direction. Additionally it
> would need discussion how and where (for example) network stacks would be
> documented.

It was split because it did not work as it did not handle the whole project so
please lets not be distracted revisiting old problems.

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


Re: [PATCH rtems 2/2] bsps/imxrt: Simplify linkcmds and make it flexible

2021-06-07 Thread Chris Johns
On 7/6/21 6:40 pm, Christian Mauderer wrote:> I think the Options don't need
documentation changes because the options in the
> waf based build system are now documented directly in the yaml files and 
> printed
> if you generate the default config.

Hmm I am not sure I agree with the premise the YAML is the documentation. The
user manual exists for this purpose and user wanting to explore RTEMS would look
there rather than cloning the repo, running commands etc.

I accept the current defaults etc work flow is a good place to start for the waf
conversion project but we need to do a lot more to make accessing the YAML
easier. The wscript contains the only rtems.git repo means to read the YAML and
the pickled data and that limits how we can change and evolve this.

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


Re: [PATCH v2 2/2] i386: Remove unneeded include header files

2021-06-07 Thread Chris Johns
I agree this is the way to solve this issue.

Chris

On 8/6/21 3:09 am, Gedare Bloom wrote:
> This looks like a much better solution :) If no one complains by
> Thursday go ahead and push it.
> 
> On Mon, Jun 7, 2021 at 2:53 AM Jan Sommer  wrote:
>>
>> ---
>>  .../sys/i386/include/machine/intr_machdep.h   |6 -
>>  rtemsbsd/include/x86/bus.h| 1109 
>>  rtemsbsd/include/x86/specialreg.h | 1143 -
>>  3 files changed, 2258 deletions(-)
>>  delete mode 100644 freebsd/sys/i386/include/machine/intr_machdep.h
>>  delete mode 100644 rtemsbsd/include/x86/bus.h
>>  delete mode 100644 rtemsbsd/include/x86/specialreg.h
>>
>> diff --git a/freebsd/sys/i386/include/machine/intr_machdep.h 
>> b/freebsd/sys/i386/include/machine/intr_machdep.h
>> deleted file mode 100644
>> index a0b28387..
>> --- a/freebsd/sys/i386/include/machine/intr_machdep.h
>> +++ /dev/null
>> @@ -1,6 +0,0 @@
>> -/*-
>> - * This file is in the public domain.
>> - */
>> -/* $FreeBSD$ */
>> -
>> -#include 
>> diff --git a/rtemsbsd/include/x86/bus.h b/rtemsbsd/include/x86/bus.h
>> deleted file mode 100644
>> index 2427ae51..
>> --- a/rtemsbsd/include/x86/bus.h
>> +++ /dev/null
>> @@ -1,1109 +0,0 @@
>> -/*-
>> - * SPDX-License-Identifier: BSD-3-Clause AND BSD-2-Clause-NetBSDE
>> - *
>> - * Copyright (c) KATO Takenori, 1999.
>> - *
>> - * All rights reserved.  Unpublished rights reserved under the copyright
>> - * laws of Japan.
>> - *
>> - * Redistribution and use in source and binary forms, with or without
>> - * modification, are permitted provided that the following conditions
>> - * are met:
>> - *
>> - * 1. Redistributions of source code must retain the above copyright
>> - *notice, this list of conditions and the following disclaimer as
>> - *the first lines of this file unmodified.
>> - * 2. Redistributions in binary form must reproduce the above copyright
>> - *notice, this list of conditions and the following disclaimer in the
>> - *documentation and/or other materials provided with the distribution.
>> - * 3. The name of the author may not be used to endorse or promote products
>> - *derived from this software without specific prior written permission.
>> - *
>> - * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
>> - * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
>> - * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
>> - * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
>> - * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
>> - * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
>> - * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
>> - * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>> - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
>> - * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>> - *
>> - * $FreeBSD$
>> - */
>> -
>> -/* $NetBSD: bus.h,v 1.12 1997/10/01 08:25:15 fvdl Exp $*/
>> -
>> -/*-
>> - * Copyright (c) 1996, 1997 The NetBSD Foundation, Inc.
>> - * All rights reserved.
>> - *
>> - * This code is derived from software contributed to The NetBSD Foundation
>> - * by Jason R. Thorpe of the Numerical Aerospace Simulation Facility,
>> - * NASA Ames Research Center.
>> - *
>> - * Redistribution and use in source and binary forms, with or without
>> - * modification, are permitted provided that the following conditions
>> - * are met:
>> - * 1. Redistributions of source code must retain the above copyright
>> - *notice, this list of conditions and the following disclaimer.
>> - * 2. Redistributions in binary form must reproduce the above copyright
>> - *notice, this list of conditions and the following disclaimer in the
>> - *documentation and/or other materials provided with the distribution.
>> - *
>> - * THIS SOFTWARE IS PROVIDED BY THE NETBSD FOUNDATION, INC. AND CONTRIBUTORS
>> - * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT 
>> LIMITED
>> - * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A 
>> PARTICULAR
>> - * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL THE FOUNDATION OR CONTRIBUTORS
>> - * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
>> - * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
>> - * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
>> - * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
>> - * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
>> - * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF 
>> THE
>> - * POSSIBILITY OF SUCH DAMAGE.
>> - */
>> -
>> -/*-
>> - * Copyright (c) 1996 Charles M. Hannum.  All rights reserved.
>> - * Copyright (c) 1996 Christopher G. Demetriou.  All rights 

Re: Selection of ethernet peripheral by application

2021-06-07 Thread Chris Johns


On 7/6/21 10:05 pm, Kinsey Moore wrote:
> On 6/6/2021 17:42, Chris Johns wrote:
>> On 4/6/21 11:26 pm, Joel Sherrill wrote:
>>> On Fri, Jun 4, 2021 at 7:59 AM Kinsey Moore >> <mailto:kinsey.mo...@oarcorp.com>> wrote:
>>>
>>>  On 6/4/2021 02:32, Christian MAUDERER wrote:
>>>  > Am 02.06.21 um 20:37 schrieb Kinsey Moore:
>>>  >> Hello,
>>>  >>
>>>  >>  From what I’ve seen of the various BSPs supported by LibBSD that
>>>  >> have multiple ethernet peripherals,
>>>  >>
>>>  >> only one tends to be chosen and supported. I’ve encountered a
>>>  >> situation where the majority of platform
>>>  >>
>>>  >> examples (Zynq Ultrascale+ MPSoC dev boards) use CGEM3 as the
>>>  >> only/primary ethernet interface and I
>>>  >>
>>>  >> need to have the option of selecting a different peripheral as the
>>>  >> primary interface.
>>>  >>
>>>  >> Unfortunately, the current configuration of LibBSD/RTEMS does not
>>>  >> allow for easy switching among the
>>>  >>
>>>  >> available interfaces. The easy one-off option is to just create a 
>>> BSP
>>>  >> variant with a little bit of extra logic
>>>  >>
>>>  >> to select the correct interface in LibBSD, but this problem seems
>>>  >> more generally applicable than just
>>>  >>
>>>  >> ethernet and just this one BSP. Is the best alternative to configure
>>>  >> all available devices in
>>>  >>
>>>  >> nexus-devices.h and let LibBSD figure it out?
>>>  >>
>>>  >> Thanks,
>>>  >> Kinsey
>>>  >>
>>>  >
>>>  > If the problem is with "nexus-devices.h": You can always just add
>>>  > further devices or a completely own set of devices in your
>>>  > application. For example it should be no problem to use
>>>  >
>>>  > 
>>>  > SYSINIT_MODULE_REFERENCE(wlan_ratectl_none);
>>>  > SYSINIT_MODULE_REFERENCE(wlan_sta);
>>>  > SYSINIT_MODULE_REFERENCE(wlan_amrr);
>>>  > SYSINIT_MODULE_REFERENCE(wlan_wep);
>>>  > SYSINIT_MODULE_REFERENCE(wlan_tkip);
>>>  > SYSINIT_MODULE_REFERENCE(wlan_ccmp);
>>>  > SYSINIT_DRIVER_REFERENCE(rtwn_usb, uhub);
>>>  > SYSINIT_REFERENCE(rtwn_rtl8188eufw);
>>>  >
>>>  > #define RTEMS_BSD_CONFIG_BSP_CONFIG
>>>  > #define RTEMS_BSD_CONFIG_TERMIOS_KQUEUE_AND_POLL
>>>  > #define RTEMS_BSD_CONFIG_INIT
>>>  >
>>>  > #include 
>>>  > 
>>>  >
>>>  > in your application to add WLAN support. You could also remove the
>>>  > RTEMS_BSD_CONFIG_BSP_CONFIG (which has the effect that 
>>> nexus-devices.h
>>>  > is not included in the rtems-bsd-config.h) and define a different set
>>>  > of modules for your application.
>>>  >
>>>  > Best regards
>>>  >
>>>  > Christian
>>>
>>>  I guess this is a misunderstanding on my part. It might still be nice 
>>> to
>>>  have something switchable for testing purposes, but I see that it can 
>>> be
>>>  altered relatively easily on the application side of things.
>>>
>>>
>>> I think what you want is like what some of the BSPs do which have a
>>> second ifdef inside the LIBBSP conditional. This is an example from
>>> the QORIQ. Perhaps an option in the BSP?
>> Is the Ultrascale's interface different to the Zync, ie same driver?
> It's the same driver after some upgrades went in to LibBSD to pull in the
> changes from 13 with some additional modifications.

Excellent. The Versal has the same MAC in the hard IP as the Ultra. The Versal
also has a MRMAC and MCDMA glue for the PL but that is a different topic.

>>> https://git.rtems.org/rtems-libbsd/tree/rtemsbsd/include/bsp/nexus-devices.h#n212
>>> <https://git.rtems.org/rtems-libbsd/tree/rtemsbsd/include/bsp/nexus-devices.h#n212>
>>>
>>>
>>> Would this qualify as a BSP variant?
>> If the hardware exists then I suggest adding both interfaces. This does not
>> effect how or why they are configured (see the config INI file). I would 
>> prefer
>> we do not add more variants for hardware that is present, ie 2 NICs.
> Every ultrascale+ chip has all 4 ethernet interfaces. What varies is where the
> PHY(s) are attached.

I suggest you add the MACs and some PHY drivers the eval boards use and let the
stack sort it out. The unknown PHY may work in some cases.

>>> If you can probe for each, you could configure both but I suspect that's
>>> likely undesirable for many applications. A configure option might be
>>> the cleanest thing. Probably not worth a BSP variant.
>> If this is for testing then I suggest you extend or look at the INI file
>> configure option. You should be able to select the interface to use for 
>> testing.
> 
> I'll add an option for testing purposes for use in config.ini.

Great and thanks.

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

Re: [PATCH v2] covoar: Store address-to-line info outside of DWARF

2021-06-07 Thread Chris Johns
On 8/6/21 1:44 am, Joel Sherrill wrote:
> 
> 
> On Mon, Jun 7, 2021 at 7:00 AM Alex White  <mailto:alex.wh...@oarcorp.com>> wrote:
> 
> 
> On Wed, Jun 2, 2021 at 7:37 PM Chris Johns  <mailto:chr...@rtems.org>> wrote:
> >
> > Looking good with a single comment below ...
> >
> > On 3/6/21 6:08 am, Alex White wrote:
> > > This adds the AddressToLineMapper class and supporting classes to
> > > assume responsibility of tracking address-to-line information.
> > >
> > > This allows the DWARF library to properly cleanup all of its resources
> > > and leads to significant memory savings.
> > >
> > > Closes #4383
> > > ---
> > >  rtemstoolkit/rld-dwarf.cpp           |   5 +
> > >  rtemstoolkit/rld-dwarf.h             |   5 +
> > >  tester/covoar/AddressToLineMapper.cc | 104 +
> > >  tester/covoar/AddressToLineMapper.h  | 211 
> +++
> > >  tester/covoar/ExecutableInfo.cc      |  73 +
> > >  tester/covoar/ExecutableInfo.h       |  11 +-
> > >  tester/covoar/wscript                |   1 +
> > >  7 files changed, 368 insertions(+), 42 deletions(-)
> > >  create mode 100644 tester/covoar/AddressToLineMapper.cc
> > >  create mode 100644 tester/covoar/AddressToLineMapper.h
> > >
> > > diff --git a/rtemstoolkit/rld-dwarf.cpp b/rtemstoolkit/rld-dwarf.cpp
> > > index 2fce0e4..1eae50c 100644
> > > --- a/rtemstoolkit/rld-dwarf.cpp
> > > +++ b/rtemstoolkit/rld-dwarf.cpp
> > > @@ -1893,6 +1893,11 @@ namespace rld
> > >        return false;
> > >      }
> > >
> > > +    const addresses& compilation_unit::get_addresses () const
> > > +    {
> > > +      return addr_lines_;
> > > +    }
> > > +
> > >      functions&
> > >      compilation_unit::get_functions ()
> > >      {
> > > diff --git a/rtemstoolkit/rld-dwarf.h b/rtemstoolkit/rld-dwarf.h
> > > index 1210813..eadb50d 100644
> > > --- a/rtemstoolkit/rld-dwarf.h
> > > +++ b/rtemstoolkit/rld-dwarf.h
> > > @@ -707,6 +707,11 @@ namespace rld
> > >         */
> > >        unsigned int pc_high () const;
> > >
> > > +      /**
> > > +       * The addresses associated with this compilation unit.
> > > +       */
> > > +      const addresses& get_addresses () const;
> > > +
> > >        /**
> > >         * Get the source and line for an address. If the address does
> not match
> > >         * false is returned the file is set to 'unknown' and the line 
> is
> set to
> > > diff --git a/tester/covoar/AddressToLineMapper.cc
> b/tester/covoar/AddressToLineMapper.cc
> > > new file mode 100644
> > > index 000..c305e3b
> > > --- /dev/null
> > > +++ b/tester/covoar/AddressToLineMapper.cc
> > > @@ -0,0 +1,104 @@
> > > +/*! @file AddressToLineMapper.cc
> > > + *  @brief AddressToLineMapper Implementation
> > > + *
> > > + *  This file contains the implementation of the functionality
> > > + *  of the AddressToLineMapper class.
> > > + */
> > > +
> > > +#include "AddressToLineMapper.h"
> > > +
> > > +namespace Coverage {
> > > +
> > > +  uint64_t SourceLine::location() const
> > > +  {
> > > +    return address;
> > > +  }
> > > +
> > > +  bool SourceLine::is_an_end_sequence() const
> > > +  {
> > > +    return is_end_sequence;
> > > +  }
> > > +
> > > +  const std::string& SourceLine::path() const
> > > +  {
> > > +    return path_;
> > > +  }
> > > +
> > > +  int SourceLine::line() const
> > > +  {
> > > +    return line_num;
> > > +  }
> > > +
> > > +  void AddressLineRange::addSourceLine(const rld::dwarf::address& 
> address)
> > > +  {
> > > +    auto insertResult = sourcePaths.insert(address.path());
> > > +
> > > +    sourceLines.emplace_back(
> > > +      SourceLine (
> &g

Re: Selection of ethernet peripheral by application

2021-06-06 Thread Chris Johns
On 4/6/21 11:26 pm, Joel Sherrill wrote:
> On Fri, Jun 4, 2021 at 7:59 AM Kinsey Moore  > wrote:
> 
> On 6/4/2021 02:32, Christian MAUDERER wrote:
> > Am 02.06.21 um 20:37 schrieb Kinsey Moore:
> >> Hello,
> >>
> >>  From what I’ve seen of the various BSPs supported by LibBSD that
> >> have multiple ethernet peripherals,
> >>
> >> only one tends to be chosen and supported. I’ve encountered a
> >> situation where the majority of platform
> >>
> >> examples (Zynq Ultrascale+ MPSoC dev boards) use CGEM3 as the
> >> only/primary ethernet interface and I
> >>
> >> need to have the option of selecting a different peripheral as the
> >> primary interface.
> >>
> >> Unfortunately, the current configuration of LibBSD/RTEMS does not
> >> allow for easy switching among the
> >>
> >> available interfaces. The easy one-off option is to just create a BSP
> >> variant with a little bit of extra logic
> >>
> >> to select the correct interface in LibBSD, but this problem seems
> >> more generally applicable than just
> >>
> >> ethernet and just this one BSP. Is the best alternative to configure
> >> all available devices in
> >>
> >> nexus-devices.h and let LibBSD figure it out?
> >>
> >> Thanks,
> >> Kinsey
> >>
> >
> > If the problem is with "nexus-devices.h": You can always just add
> > further devices or a completely own set of devices in your
> > application. For example it should be no problem to use
> >
> > 
> > SYSINIT_MODULE_REFERENCE(wlan_ratectl_none);
> > SYSINIT_MODULE_REFERENCE(wlan_sta);
> > SYSINIT_MODULE_REFERENCE(wlan_amrr);
> > SYSINIT_MODULE_REFERENCE(wlan_wep);
> > SYSINIT_MODULE_REFERENCE(wlan_tkip);
> > SYSINIT_MODULE_REFERENCE(wlan_ccmp);
> > SYSINIT_DRIVER_REFERENCE(rtwn_usb, uhub);
> > SYSINIT_REFERENCE(rtwn_rtl8188eufw);
> >
> > #define RTEMS_BSD_CONFIG_BSP_CONFIG
> > #define RTEMS_BSD_CONFIG_TERMIOS_KQUEUE_AND_POLL
> > #define RTEMS_BSD_CONFIG_INIT
> >
> > #include 
> > 
> >
> > in your application to add WLAN support. You could also remove the
> > RTEMS_BSD_CONFIG_BSP_CONFIG (which has the effect that nexus-devices.h
> > is not included in the rtems-bsd-config.h) and define a different set
> > of modules for your application.
> >
> > Best regards
> >
> > Christian
> 
> I guess this is a misunderstanding on my part. It might still be nice to
> have something switchable for testing purposes, but I see that it can be
> altered relatively easily on the application side of things.
> 
> 
> I think what you want is like what some of the BSPs do which have a 
> second ifdef inside the LIBBSP conditional. This is an example from
> the QORIQ. Perhaps an option in the BSP?

Is the Ultrascale's interface different to the Zync, ie same driver?

> https://git.rtems.org/rtems-libbsd/tree/rtemsbsd/include/bsp/nexus-devices.h#n212
>  
> 
> 
> Would this qualify as a BSP variant? 

If the hardware exists then I suggest adding both interfaces. This does not
effect how or why they are configured (see the config INI file). I would prefer
we do not add more variants for hardware that is present, ie 2 NICs.

> If you can probe for each, you could configure both but I suspect that's
> likely undesirable for many applications. A configure option might be
> the cleanest thing. Probably not worth a BSP variant.

If this is for testing then I suggest you extend or look at the INI file
configure option. You should be able to select the interface to use for testing.

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

Re: [PATCH v1] Explanations.cc: Convert to c++ file handling

2021-06-04 Thread Chris Johns
Ok

On 5/6/21 4:17 am, Ryan Long wrote:
> ---
>  tester/covoar/Explanations.cc | 45 
> ---
>  1 file changed, 17 insertions(+), 28 deletions(-)
> 
> diff --git a/tester/covoar/Explanations.cc b/tester/covoar/Explanations.cc
> index d94cd2e..1449fb2 100644
> --- a/tester/covoar/Explanations.cc
> +++ b/tester/covoar/Explanations.cc
> @@ -10,6 +10,8 @@
>  #include 
>  #include 
>  
> +#include 
> +
>  #include 
>  
>  #include "Explanations.h"
> @@ -30,15 +32,14 @@ namespace Coverage {
>)
>{
>  #define MAX_LINE_LENGTH 512
> -FILE   *explain;
> -char*cStatus;
> -Explanation  e;
> -int  line = 1;
> +std::ifstream  explain;
> +Explanatione;
> +intline = 1;
>  
>  if (!explanations)
>return;
>  
> -explain = ::fopen( explanations, "r" );
> +explain.open( explanations );
>  if (!explain) {
>std::ostringstream what;
>what << "Unable to open " << explanations;
> @@ -50,9 +51,9 @@ namespace Coverage {
>// skip blank lines between
>do {
>  inputBuffer[0] = '\0';
> -cStatus = fgets( inputBuffer, MAX_LINE_LENGTH, explain );
> -if (cStatus == NULL) {
> -  goto done;
> +explain.getline( inputBuffer, MAX_LINE_LENGTH );
> +if (explain.fail()) {
> +  return;
>  }
>  inputBuffer[ strlen(inputBuffer) - 1] = '\0';
>  line++;
> @@ -64,7 +65,6 @@ namespace Coverage {
>  what << "line " << line
>   << "contains a duplicate explanation ("
>   << inputBuffer << ")";
> -fclose( explain );
>  throw rld::error( what, "Explanations::load" );
>}
>  
> @@ -73,12 +73,11 @@ namespace Coverage {
>e.found = false;
>  
>// Get the classification
> -  cStatus = fgets( inputBuffer, MAX_LINE_LENGTH, explain );
> -  if (cStatus == NULL) {
> +  explain.getline( inputBuffer, MAX_LINE_LENGTH );
> +  if (explain.fail()) {
>  std::ostringstream what;
>  what << "line " << line
>   << "out of sync at the classification";
> -fclose( explain );
>  throw rld::error( what, "Explanations::load" );
>}
>inputBuffer[ strlen(inputBuffer) - 1] = '\0';
> @@ -87,13 +86,12 @@ namespace Coverage {
>  
>// Get the explanation
>while (1) {
> -cStatus = fgets( inputBuffer, MAX_LINE_LENGTH, explain );
> +explain.getline( inputBuffer, MAX_LINE_LENGTH );
>  // fprintf( stderr, "%d - %s\n", line, inputBuffer );
> -if (cStatus == NULL) {
> +if (explain.fail()) {
>std::ostringstream what;
>what << "line " << line
> << "out of sync at the explanation";
> -  fclose( explain );
>throw rld::error( what, "Explanations::load" );
>  }
>  inputBuffer[ strlen(inputBuffer) - 1] = '\0';
> @@ -110,10 +108,6 @@ namespace Coverage {
>// Add this to the set of Explanations
>set[ e.startingPoint ] = e;
>  }
> -done:
> -;
> -
> -fclose( explain );
>  
>  #undef MAX_LINE_LENGTH
>}
> @@ -137,14 +131,14 @@ done:
>  const char* const fileName
>)
>{
> -FILE* notFoundFile;
> +std::ofstream notFoundFile;
>  bool  notFoundOccurred = false;
>  
>  if (!fileName)
>return;
>  
> -notFoundFile = fopen( fileName, "w" );
> -if (notFoundFile == nullptr) {
> +notFoundFile.open( fileName );
> +if (!notFoundFile) {
>std::ostringstream what;
>what << "Unable to open " << fileName
> << " out of sync at the explanation";
> @@ -159,14 +153,9 @@ done:
>  
>if (!e.found) {
>  notFoundOccurred = true;
> -fprintf(
> -  notFoundFile,
> -  "%s\n",
> -  e.startingPoint.c_str()
> -);
> +notFoundFile << e.startingPoint << std::endl;
>}
>  }
> -fclose( notFoundFile );
>  
>  if (!notFoundOccurred) {
>if (!unlink( fileName )) {
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: [PATCH v1 1/6] Explanations.cc: Fix resource leaks

2021-06-04 Thread Chris Johns
On 5/6/21 2:47 am, Ryan Long wrote:
> We'll submit the C++ conversions as separate patches since they are larger. 
> That patch will be submitted shortly.

Excellent and thanks.

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


Re: [PATCH] cpukit/libblock/src: Add better memory handling in flashdisk.c

2021-06-04 Thread Chris Johns
On 5/6/21 8:11 am, Gedare Bloom wrote:
> On Fri, Jun 4, 2021 at 1:17 PM Joel Sherrill  wrote:
>>
>> On the surface, this looks OK to me. But I remember looking at this one
>> and wondering if there was any cleanup required because of the various
>> subroutine calls as you work down through this method.
>>
>> Did you look into each of the subroutines called and make sure they
>> didn't do further allocations?
>>
> We hadn't. Now, we took a closer look at this also, and see some more
> problems that need to be addressed. The fd variable is a loop
> iterator, so we can't just free() it. Should this function cleanup
> everything that was attempted and not to return with a partial
> initialization in case of a failure? Or, should it allow the partial
> initialization of some of the fd's, and just cleanup the last one that
> fails (due to out of memory)?

I think there is one that pops up on a regular basis where you cannot delete
things because they are registered with something that has not unregister.

I am not sure this is the case. It might pay to check before pushing.

> 
> 
>> --joel
>>
>> On Fri, Jun 4, 2021 at 1:27 PM Harrison Edward Gerber  
>> wrote:
>>>
>>> See also CID 1439298
>>>
>>> Closes #3570
>>> ---
>>>  cpukit/libblock/src/flashdisk.c | 16 
>>>  1 file changed, 16 insertions(+)
>>>
>>> diff --git a/cpukit/libblock/src/flashdisk.c 
>>> b/cpukit/libblock/src/flashdisk.c
>>> index 91f99e0d52..4de6ecd807 100644
>>> --- a/cpukit/libblock/src/flashdisk.c
>>> +++ b/cpukit/libblock/src/flashdisk.c
>>> @@ -2486,17 +2486,29 @@ rtems_fdisk_initialize (rtems_device_major_number 
>>> major,
>>>   */
>>>  fd->copy_buffer = malloc (c->block_size);
>>>  if (!fd->copy_buffer)
>>> +{
>>> +  free(fd);

Missing space to be consistent? More of these below.

Chris

>>>return RTEMS_NO_MEMORY;
>>> +}
>>>
>>>  fd->blocks = calloc (blocks, sizeof (rtems_fdisk_block_ctl));
>>>  if (!fd->blocks)
>>> +{
>>> +  free(fd->copy_buffer);
>>> +  free(fd);
>>>return RTEMS_NO_MEMORY;
>>> +}
>>>
>>>  fd->block_count = blocks;
>>>
>>>  fd->devices = calloc (c->device_count, sizeof 
>>> (rtems_fdisk_device_ctl));
>>>  if (!fd->devices)
>>> +{
>>> +  free(fd->blocks);
>>> +  free(fd->copy_buffer);
>>> +  free(fd);
>>>return RTEMS_NO_MEMORY;
>>> +}
>>>
>>>  rtems_mutex_init (>lock, "Flash Disk");
>>>
>>> @@ -2508,6 +2520,7 @@ rtems_fdisk_initialize (rtems_device_major_number 
>>> major,
>>>free (fd->copy_buffer);
>>>free (fd->blocks);
>>>free (fd->devices);
>>> +  free(fd);
>>>rtems_fdisk_error ("disk create phy failed");
>>>return sc;
>>>  }
>>> @@ -2529,6 +2542,7 @@ rtems_fdisk_initialize (rtems_device_major_number 
>>> major,
>>>  free (fd->copy_buffer);
>>>  free (fd->blocks);
>>>  free (fd->devices);
>>> +free(fd);
>>>  return RTEMS_NO_MEMORY;
>>>}
>>>
>>> @@ -2564,6 +2578,7 @@ rtems_fdisk_initialize (rtems_device_major_number 
>>> major,
>>>free (fd->copy_buffer);
>>>free (fd->blocks);
>>>free (fd->devices);
>>> +  free(fd);
>>>rtems_fdisk_error ("recovery of disk failed: %s (%d)",
>>>   strerror (ret), ret);
>>>return ret;
>>> @@ -2577,6 +2592,7 @@ rtems_fdisk_initialize (rtems_device_major_number 
>>> major,
>>>free (fd->copy_buffer);
>>>free (fd->blocks);
>>>free (fd->devices);
>>> +  free(fd);
>>>rtems_fdisk_error ("compacting of disk failed: %s (%d)",
>>>   strerror (ret), ret);
>>>return ret;
>>> --
>>> 2.25.1
>>>
>>> ___
>>> devel mailing list
>>> devel@rtems.org
>>> http://lists.rtems.org/mailman/listinfo/devel
>>
>> ___
>> devel mailing list
>> devel@rtems.org
>> http://lists.rtems.org/mailman/listinfo/devel
> ___
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: [PATCH v2] covoar: Store address-to-line info outside of DWARF

2021-06-02 Thread Chris Johns
Looking good with a single comment below ...

On 3/6/21 6:08 am, Alex White wrote:
> This adds the AddressToLineMapper class and supporting classes to
> assume responsibility of tracking address-to-line information.
> 
> This allows the DWARF library to properly cleanup all of its resources
> and leads to significant memory savings.
> 
> Closes #4383
> ---
>  rtemstoolkit/rld-dwarf.cpp   |   5 +
>  rtemstoolkit/rld-dwarf.h |   5 +
>  tester/covoar/AddressToLineMapper.cc | 104 +
>  tester/covoar/AddressToLineMapper.h  | 211 +++
>  tester/covoar/ExecutableInfo.cc  |  73 +
>  tester/covoar/ExecutableInfo.h   |  11 +-
>  tester/covoar/wscript|   1 +
>  7 files changed, 368 insertions(+), 42 deletions(-)
>  create mode 100644 tester/covoar/AddressToLineMapper.cc
>  create mode 100644 tester/covoar/AddressToLineMapper.h
> 
> diff --git a/rtemstoolkit/rld-dwarf.cpp b/rtemstoolkit/rld-dwarf.cpp
> index 2fce0e4..1eae50c 100644
> --- a/rtemstoolkit/rld-dwarf.cpp
> +++ b/rtemstoolkit/rld-dwarf.cpp
> @@ -1893,6 +1893,11 @@ namespace rld
>return false;
>  }
>  
> +const addresses& compilation_unit::get_addresses () const
> +{
> +  return addr_lines_;
> +}
> +
>  functions&
>  compilation_unit::get_functions ()
>  {
> diff --git a/rtemstoolkit/rld-dwarf.h b/rtemstoolkit/rld-dwarf.h
> index 1210813..eadb50d 100644
> --- a/rtemstoolkit/rld-dwarf.h
> +++ b/rtemstoolkit/rld-dwarf.h
> @@ -707,6 +707,11 @@ namespace rld
> */
>unsigned int pc_high () const;
>  
> +  /**
> +   * The addresses associated with this compilation unit.
> +   */
> +  const addresses& get_addresses () const;
> +
>/**
> * Get the source and line for an address. If the address does not 
> match
> * false is returned the file is set to 'unknown' and the line is set 
> to
> diff --git a/tester/covoar/AddressToLineMapper.cc 
> b/tester/covoar/AddressToLineMapper.cc
> new file mode 100644
> index 000..c305e3b
> --- /dev/null
> +++ b/tester/covoar/AddressToLineMapper.cc
> @@ -0,0 +1,104 @@
> +/*! @file AddressToLineMapper.cc
> + *  @brief AddressToLineMapper Implementation
> + *
> + *  This file contains the implementation of the functionality
> + *  of the AddressToLineMapper class.
> + */
> +
> +#include "AddressToLineMapper.h"
> +
> +namespace Coverage {
> +
> +  uint64_t SourceLine::location() const
> +  {
> +return address;
> +  }
> +
> +  bool SourceLine::is_an_end_sequence() const
> +  {
> +return is_end_sequence;
> +  }
> +
> +  const std::string& SourceLine::path() const
> +  {
> +return path_;
> +  }
> +
> +  int SourceLine::line() const
> +  {
> +return line_num;
> +  }
> +
> +  void AddressLineRange::addSourceLine(const rld::dwarf::address& address)
> +  {
> +auto insertResult = sourcePaths.insert(address.path());
> +
> +sourceLines.emplace_back(
> +  SourceLine (
> +address.location(),
> +*insertResult.first,
> +address.line(),
> +address.is_an_end_sequence()
> +  )
> +);
> +  }
> +
> +  const SourceLine& AddressLineRange::getSourceLine(uint32_t address) const
> +  {
> +if (address < lowAddress || address > highAddress) {
> +  throw SourceNotFoundError(std::to_string(address));
> +}
> +
> +const SourceLine* last_line = nullptr;
> +for (const auto  : sourceLines) {
> +  if (address <= line.location())
> +  {
> +if (address == line.location())
> +  last_line = 
> +break;
> +  }
> +  last_line = 
> +}
> +
> +if (last_line == nullptr) {
> +  throw SourceNotFoundError(std::to_string(address));
> +}
> +
> +return *last_line;
> +  }

How often is this function being called? If it is a hot spot are there other
ways to search for the address? I suppose it depends on the number of lines in
the vector.

If the sourcesLines vector was sorted can this loop be replaced with some form
of std::upper_bound? That is ...

 auto last_line =
std::upper_bound(sourceLines.begin(),
 sourceLines.end(),
 address,
 [](const SourceLine& sl) {
   return address < sl.location();
 });

[ not sure if the code I posted would work and if I got the lamda right :) ]

Just wondering.

Chris

> +
> +  void AddressToLineMapper::getSource(
> +uint32_t address,
> +std::string& sourceFile,
> +int& sourceLine
> +  ) const {
> +const SourceLine default_sourceline = SourceLine();
> +const SourceLine* match = _sourceline;
> +
> +for (const auto  : addressLineRanges) {
> +  try {
> +const SourceLine& potential_match = range.getSourceLine(address);
> +
> +if (match->is_an_end_sequence() || 
> !potential_match.is_an_end_sequence()) {
> +  match = _match;
> +}
> +  } catch (const 

Re: Selection of ethernet peripheral by application

2021-06-02 Thread Chris Johns
On 3/6/21 4:37 am, Kinsey Moore wrote:
> Hello,
> 
> From what I’ve seen of the various BSPs supported by LibBSD that have multiple
> ethernet peripherals,
> 
> only one tends to be chosen and supported. I’ve encountered a situation where
> the majority of platform
> 
> examples (Zynq Ultrascale+ MPSoC dev boards) use CGEM3 as the only/primary
> ethernet interface and I
> 
> need to have the option of selecting a different peripheral as the primary
> interface.
> 
>  
> 
> Unfortunately, the current configuration of LibBSD/RTEMS does not allow for 
> easy
> switching among the
> 
> available interfaces. The easy one-off option is to just create a BSP variant
> with a little bit of extra logic
> 
> to select the correct interface in LibBSD, but this problem seems more 
> generally
> applicable than just
> 
> ethernet and just this one BSP. Is the best alternative to configure all
> available devices in
> 
> nexus-devices.h and let LibBSD figure it out?

I am not sure what the issue is. I also do not know what is "the correct
interface" means.

How many interfaces do you need to support and what are they?

If a device like the Ultrascale as 2 hard IP GEM MACs maybe LibBSD for that BSP
should enable both by default. Maybe the Zynq and Versal (when added) should be
the same. If a user adds another MAC in the PL, for example a Tri-Core LogicCore
block (and adds a driver) then there may be 3 interfaces present but that would
not be enabled by default. The same goes for the PC with the ability to plug in
NIC boards.

What happens after probing and the drivers are present is for the application to
manage via the stack's various interfaces. The rc.conf support I added uses
commands such as ifconfig to access those interfaces and provide support for
initialisation and set up. There are other ways that could be done.

I wonder if there is a misunderstanding and it could be on my part. LibBSD does
not ship with THE defined set of interfaces. It has a minimal set to allow
testing and maybe that is wrong. A lot of the testing framework is just that a
testing framework and if you wish to leverage that support in applications then
all the best because that support can and may change to suite the testing needs
of LibBSD and there is no requirement to make it backwards compatible for
application that depend on it.

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


Re: [PATCH v1 0/2] [libbsd] Install correct machine include headers

2021-06-01 Thread Chris Johns
On 2/6/21 4:20 am, Christian Mauderer wrote:
> On 01/06/2021 19:24, Gedare Bloom wrote:
>> On Mon, May 10, 2021 at 11:26 AM Jan Sommer  wrote:
>>>
>>> Hello,
>>>
>>> This is a follow-up on this discussion regarding the installed header files
>>> in libbsd: https://lists.rtems.org/pipermail/devel/2021-April/066211.html
>>>
>>> The current situation is, that for example for all machines the bus.h is
>>> installed from
>>> within the rtemsbsd directory
>>> (https://git.rtems.org/rtems-libbsd/tree/rtemsbsd/include/machine/bus.h).
>>>
>>> According to the file docu it originates from the amd64 version of this 
>>> file.
>>> It also has the following section in it which we ran into when compiling
>>> Chris' ptpd2 archive:
>>>
>>> #ifdef __i386__
>>>    #error "your include paths are wrong"
>>> #endif
>>>
>>> This patchset does the following:
>>> - Add the target dependent machine include directory to 'header-paths' in
>>> libbsd.py
>>> - Import (mostly) '_bus.h', 'bus.h' and 'cpufunc.h' for the targets from
>>> freebsd-org
>>> - Remove those header files from rtemsbsd directory
>>>
>>> As a result the matching versions for machine dependent header files are now
>>> installed
>>> for each BSP.
>>> Would this be an acceptable solution?
>>>
>>> So far I compiled some BSPs for i386, arm, aarch64, powerpc, riscv, sparc 
>>> and
>>> sparc64 to check that they still compile after the changes.
>>> Are there any other architectures which should be included?
> 
> I think the best starting point to find out the minimum supported platforms is
> the nexus-devices.h. At least the officially supported BSPs should have an 
> entry
> there. That is:
> 
> #if defined(LIBBSP_ARM_REALVIEW_PBX_A9_BSP_H)
> #elif defined(LIBBSP_ARM_BEAGLE_BSP_H)
> #elif defined(LIBBSP_ARM_LPC32XX_BSP_H)
> #elif defined(LIBBSP_M68K_GENMCF548X_BSP_H)
> #elif defined(LIBBSP_ARM_XILINX_ZYNQ_BSP_H)
> #elif defined(LIBBSP_AARCH64_XILINX_ZYNQMP_BSP_H)
> #elif defined(LIBBSP_ARM_ATSAM_BSP_H)
> #elif defined(LIBBSP_ARM_ALTERA_CYCLONE_V_BSP_H)
> #elif defined(LIBBSP_ARM_IMX_BSP_H)
> #elif defined(LIBBSP_ARM_IMXRT_BSP_H)
> #elif defined(LIBBSP_ARM_LPC24XX_BSP_H)
> #elif defined(LIBBSP_ARM_STM32H7_BSP_H)
> #elif defined(LIBBSP_I386_PC386_BSP_H)
> #elif defined(LIBBSP_POWERPC_QORIQ_BSP_H)
> #elif defined(LIBBSP_POWERPC_TQM8XX_BSP_H)
> #elif defined(LIBBSP_POWERPC_MOTOROLA_POWERPC_BSP_H)
> 
> So I think you should have a look at m68k too.
> 
> Did you try to run it on any of the platforms?
> 
>>>
>>> I ran into one problem regarding the compilation of
>>> rtemsbsd/sys/dev/dw_mmc/dw_mmc.c:105
>>>
 return (bus_space_read_4(0, sc->bushandle, off));
>>>
>>> The first parameter creates an error for riscv (I think because 
>>> dereferencing
>>> a NULL pointer).
>>> Are there any suggestion how to solve it (I am not familiar with the bus
>>> space and there is a lot of macro magic going on)?
>>>
>> It looks like this will be a problem for any architecture. so should
>> the function that calls bus_space_write_4(0 ...)
>>   The macro trail goes...
>>
>> #define    __bs_rs(sz, t, h, o)  
>>   \
>>     (*(t)->__bs_opname(r,sz))((t)->bs_cookie, h, o)
>>
>> #define    bus_space_read_4(t, h, o)   __bs_rs(4,(t),(h),(o))
>>
>> so t is dereferenced. It appears to be an error in the API usage by
>> the dw_mmc.c code to not specify a valid bus space.
> 
> dw_mmc is only relevant for very few platforms. So in theory it could be 
> limited
> to these. But most likely that won't really solve the problem.
> 
> The right answer is that dw_mmc doesn't use the API like intended (like Gedare
> said). That could be a problem for more drivers that use stuff from bus.h in
> rtemsbsd. They never had to use that API correctly. Even worse: Some of the
> __rtems__ hacks in freebsd could have that problem too.
> 
> I think for this patch set we either need to have a thorough look at these
> locations or run (not only build) it on a number of platforms, especially the
> ones with special drivers in rtemsbsd or with heavily adapted drivers.

The only arch that supports a tag we current have is the x86 and that is for IO
vs mem space. I posted some patches earlier this year I need to revisit for the
powerpc (mvme2700). If possible we prefer no tags and a single flat address
space that is cache coherent. In the x86 tags cannot be avoided and supported
drivers handle the tags correctly.

Be-careful reviewing FreeBSD's bus space implementations. For example the
PowerPC support uses tables of calls and we do not need that overhead.

The issue here is using this driver on the x86 platform. Until that happens I am
fine with the tag being 0. The API requires something even if it is not used.

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

Re: [PATCH v1 4/4] rtl-allocator.c: Put dereferences after nullcheck

2021-05-28 Thread Chris Johns
Ok

On 29/5/21 7:11 am, Ryan Long wrote:
> CID 1444139: Dereference null return value in rtems_rtl_alloc_hook().
> 
> Closes #4333
> ---
>  cpukit/libdl/rtl-allocator.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/cpukit/libdl/rtl-allocator.c b/cpukit/libdl/rtl-allocator.c
> index 647c0c8..861754e 100644
> --- a/cpukit/libdl/rtl-allocator.c
> +++ b/cpukit/libdl/rtl-allocator.c
> @@ -162,8 +162,11 @@ rtems_rtl_allocator
>  rtems_rtl_alloc_hook (rtems_rtl_allocator handler)
>  {
>rtems_rtl_data* rtl = rtems_rtl_lock ();
> -  rtems_rtl_allocator previous = rtl->allocator.allocator;
> -  rtl->allocator.allocator = handler;
> +  rtems_rtl_allocator previous = NULL;
> +  if (rtl != NULL) {
> +previous = rtl->allocator.allocator;
> +rtl->allocator.allocator = handler;
> +  }
>rtems_rtl_unlock ();
>return previous;
>  }
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: [PATCH v1 1/6] Explanations.cc: Fix resource leaks

2021-05-28 Thread Chris Johns
On 29/5/21 8:08 am, Ryan Long wrote:
> CID 1399608: Resource leak in load().
> CID 1399611: Resource leak in load().
> 
> Closes #4417
> ---
>  tester/covoar/Explanations.cc | 18 +++---
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/tester/covoar/Explanations.cc b/tester/covoar/Explanations.cc
> index 12bd809..d94cd2e 100644
> --- a/tester/covoar/Explanations.cc
> +++ b/tester/covoar/Explanations.cc
> @@ -32,7 +32,7 @@ namespace Coverage {
>  #define MAX_LINE_LENGTH 512
>  FILE   *explain;
>  char*cStatus;
> -Explanation *e;
> +Explanation  e;
>  int  line = 1;
>  
>  if (!explanations)
> @@ -46,7 +46,6 @@ namespace Coverage {
>  }
>  
>  while ( 1 ) {
> -  e = new Explanation;
>// Read the starting line of this explanation and
>// skip blank lines between
>do {
> @@ -65,12 +64,13 @@ namespace Coverage {
>  what << "line " << line
>   << "contains a duplicate explanation ("
>   << inputBuffer << ")";
> +fclose( explain );
>  throw rld::error( what, "Explanations::load" );

If you used a std::ostream type object it would be automatically closed when
that object destructs via any exit path.

While this patch fixes the bug(s) the mixing of C here is generating these
issues and effort cleaning that up will reward you.

Chris

>}
>  
>// Add the starting line and file
> -  e->startingPoint = std::string(inputBuffer);
> -  e->found = false;
> +  e.startingPoint = std::string(inputBuffer);
> +  e.found = false;
>  
>// Get the classification
>cStatus = fgets( inputBuffer, MAX_LINE_LENGTH, explain );
> @@ -78,10 +78,11 @@ namespace Coverage {
>  std::ostringstream what;
>  what << "line " << line
>   << "out of sync at the classification";
> +fclose( explain );
>  throw rld::error( what, "Explanations::load" );
>}
>inputBuffer[ strlen(inputBuffer) - 1] = '\0';
> -  e->classification = inputBuffer;
> +  e.classification = inputBuffer;
>line++;
>  
>// Get the explanation
> @@ -92,6 +93,7 @@ namespace Coverage {
>std::ostringstream what;
>what << "line " << line
> << "out of sync at the explanation";
> +  fclose( explain );
>throw rld::error( what, "Explanations::load" );
>  }
>  inputBuffer[ strlen(inputBuffer) - 1] = '\0';
> @@ -102,15 +104,17 @@ namespace Coverage {
>break;
>  }
>  // XXX only taking last line.  Needs to be a vector
> -e->explanation.push_back( inputBuffer );
> +e.explanation.push_back( inputBuffer );
>}
>  
>// Add this to the set of Explanations
> -  set[ e->startingPoint ] = *e;
> +  set[ e.startingPoint ] = e;
>  }
>  done:
>  ;
>  
> +fclose( explain );
> +
>  #undef MAX_LINE_LENGTH
>}
>  
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: [PATCH] rtems-binutils-head.cfg, rtems-gdb-head.cfg: Bump to 3b2bef8

2021-05-27 Thread Chris Johns
On 28/5/21 6:01 am, Joel Sherrill wrote:
> On Wed, May 26, 2021 at 2:29 AM Chris Johns  <mailto:chr...@rtems.org>> wrote:
> 
> I am ok with this going in but 
> 
> I am becoming concerned we are building a debt around MacOS and Windows 
> in the
> tools that we need to resolve. Our ability to release when we would like 
> to
> depends on the state of the tools and the state of the tools for all 
> archs. :)
> 
> 
> I don't disagree. This bump was actually necessary because gdb wouldn't
> build on CentOS 7. It had a bug and didn't add -std=c99 and was using 
> C99 code.

If this position was taken for Windows or some archs would we have moved to GDB
head like we have? Does Linux by itself define a stable build suitable for a
change? I do not think it does. If this move fixed Linux but broke Windows is
making the change valid?

I suggest it is in the interest of everyone to see the tools fixed on all hosts.
I am not sure how we stage changes like this so they can be tested as requiring
each developer to have access to all hosts is not feasible.

I have a real concern that fixing some of these issues may require regressing
tool changes and with projects like pre-qual there may be a negative impact.

> Crossing threads, the mailer series of modifications by Alex was to support my
> need to send reports from Windows. The addition of the various smtp options
> and using gitconfig for email options. My intent was to turn on some Windows
> testing (Mingw and Cygwin) but I couldn't get test reports to build@. I'm not 
> tackling MacOS but was trying to get reports on the others which is the first
> step.

This is a great addition and a good solution. It however is an after the fact
change and I am wondering how we pre-test changes?

> I know that's not apparent from the changes themselves but that was the
> overall goal.

I knew it was and understood this.

> Also, I think the mailer changes were already in either RSB or rtems-tools 
> and needed to be synced. If something was missed when the patches were
> reviewed and committed in the first repo, that can be addressed in a follow up
> patch.
> 
> I'm trying to increase my testing from 3 hosts to 5 and need reports from them
> all to show up on build@.

Thank you for your efforts. They make a huge difference in the quality we can
produce.

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

Re: [PATCH rtems v2] cpukit: Add description of release version numbers

2021-05-27 Thread Chris Johns
Ok and thanks. :)

On 27/5/21 6:56 pm, Christian Mauderer wrote:
> The release version in the git sources doesn't change. Add a note why
> that is the case.
> ---
> v2: Integrate suggestions from Chris Johns.
> 
>  cpukit/include/rtems/version.h | 21 +
>  1 file changed, 21 insertions(+)
> 
> diff --git a/cpukit/include/rtems/version.h b/cpukit/include/rtems/version.h
> index 87d5e1492c..cdd8905735 100644
> --- a/cpukit/include/rtems/version.h
> +++ b/cpukit/include/rtems/version.h
> @@ -32,6 +32,27 @@ extern "C" {
>   * @brief The Version API provides functions to return the version or parts 
> of
>   * the version of RTEMS you are using.
>   *
> + * A branch in the version control system will always fall back to a
> + * NOT-RELEASED version number with a minor number of 0. Only the release
> + * archives have a VERSION file with a final release number. That means for
> + * example that the 5 development branch will still show a version 5.0.0 even
> + * after the 5.1 release.
> + *
> + * The reason for that are the following:
> + *
> + * 1. All pre-release tests are performed with a specific git hash. A 
> committed
> + * VERSION file would need to be changed and committed afterwards for 
> releasing
> + * with the required release version causing the released version to have a
> + * different git hash and the test results couldn't be linked to the released
> + * version.
> + *
> + * 2. Users deploying RTEMS would need to commit a local change to a 
> committed
> + * VERSION file and that would clash with the project changes. Deployment can
> + * use the project repos directly.
> + *
> + * 3. The VERSION file management and generation is the responsibility of the
> + * release manager and the release process.
> + *
>   * @{
>   */
>  
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: RFS and 2038

2021-05-26 Thread Chris Johns
On 1/5/21 6:19 am, Joel Sherrill wrote:
> Hi
> 
> Ryan has been working to add support for the new (not obsolete) nanosecond
> granularity variants of utime. In changing the utime_h file system handler to
> utimens_h and propagating the changes, Ryan tripped across this.
> 
> rtems/rfs/rtems-rfs-inode.h:typedef uint32_t rtems_rfs_time;
> 
> Our first inclination was to change this to time_t but I remembered that 
> time_t
> was 64-bits and that changing this could ripple to on-media structures. 
> 
> Ignoring that there is no sub-second granularity on the utime family of
> timestamps for the RFS (and other filesystems), how can this year 2038 problem
> be addressed?
> 
> I am pretty sure a ticket is needed but I wanted to discuss this and 
> understand
> the scope.
> 
> We may also have similar issues in other file systems that we didn't spot.

Did this get resolved?

Chris
  (still under his rock)
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH] tester: Expand special case for minimum.exe

2021-05-26 Thread Chris Johns
On 27/5/21 2:04 pm, Kinsey Moore wrote:
> On 5/26/2021 19:22, Chris Johns wrote:
>> On 27/5/21 12:06 pm, Joel Sherrill wrote:
>>> On Wed, May 26, 2021, 7:03 PM Chris Johns >> <mailto:chr...@rtems.org>> wrote:
>>>
>>>  On 26/5/21 1:52 am, Kinsey Moore wrote:
>>>  > The minimum.exe test case is expected to fail as an "invalid" test in
>>>  > the tester since it is completely stripped down and does not output 
>>> the
>>>  > normal test header and footer. When fatal error detection support was
>>>  > added, this caught minimum.exe and started flagging it as "fatal"
>>>  > instead of "invalid". The special-case detection of minimum.exe only
>>>  > matched on "invalid" results and not "fatal" results and so began
>>>  > flagging minimum.exe as an actual failure.>
>>>  > This change adds the special-case handling to the "fatal" test state
>>>  > handling.
>>>
>>>  Is this the right solution?
>>>
>>>  Is minimum.exe suppose to run and not fail? It would seem easy to make 
>>> a
>>>  minimum.exe with nothing in it, ie minimal, that seems to pass. It 
>>> would
>>> make
>>>  great marketing material.
>>>
>>>  What happens if minimum fails? I feel minimum needs to be able to run
>>> and not
>>>  fail to be a valid minimum.
>>>
>>>
>>> It is an empty thread body that doesn't print. I suppose we could add
>>> rtems_shutdown_executive(0) if that helps
>> What if the work to make it small removes something that is needed? Is 
>> minimum
>> suppose to be run and if it is how do we know it was successfull?
>>
>> My point is about the purpose of minimum. If we can never tell a run failed
>> should it be run? If we cannot tell then excluding it as a test to run for 
>> all
>> BSPs may be a simpler option that this change.
> 
> hello.exe already provides a minimum test case for something that can be
> verified to be functional, so I'd lean toward excluding minimum.exe from test
> runs of all BSPs. If minimum.exe is kept in, it needs its expected test state 
> to
> be set to fatal-error by default for all BSPs. Unfortunately, fatal-error is
> currently bundled in as a more generic failure when being reported.

Or minimum.exe is created as a no-run executable? See the various API link only
tests for an example.

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

Re: RTEMS source builder: can't find RTEMS include files but then can?

2021-05-26 Thread Chris Johns
On 30/4/21 1:15 am, dufa...@hda.com wrote:
> 
> 
>> On Apr 30, 2021, at 05:03 , dufa...@hda.com wrote:
>>
>> Can I override what to use as "cmake" in RSB to specify "cmake3"?
>>
>>> On Apr 30, 2021, at 04:41 , Peter Dufault  wrote:
>>>
>>> I verified my environment is squeaky-clean with a simple path and no 
>>> environment variables set that affect RTEMS or "make".
>>
> 
> I need help figuring out how to setup rtems-source-builder, SOEM, and cmake.  
> I got it to "work" but it's mis-configured.  At first it is not finding 
> header files (is it looking at the Linux headers?), then it installs SOEM in 
> the wrong place, etc.  I can get the build and install to "work" by 
> re-running commands in a shell script - the second time a command is run it 
> behaves differently.
> 
> - *Notes*
> -- This started when I updated SOEM and needed to change to "cmake3".
> -- After the following convoluted process the testing I've done on the 
> results are OK.
> 
> - To simplify my path I make it simple except I add a directory with only a 
> "cmake" bash script: "cmake3 $*" in order to pick up cmake3.

Maybe the --macros argument can help. Create a file `xyz.txt` containing ...

__cmake: exe, optional, 'cmake3'

then try running with `--macros=xyz.txt` If the recipe for the build using cmake
uses `%{__cmake}` then this may work. I have not tested any of this. :)


Chris

> 
> - The first time I run "sb-set-builder" the SOEM build fails because it can't 
> find . This is just after it compiled a C file that includes 
> .  I'm not sure where it is looking for the headers, I'd expect it 
> would fail earlier if it was trying to use Linux headers, it's all the way to 
> the last file.
> 
> - I then run the do-build script sb-set-builder created and it builds SOEM 
> *but it installs it in the wrong place*.
> -- The do-build script is in 
> "${GITDIR}/rtems/rtems-source-builder/rtems/build/soem-powerpc-rtems6-1/do-build"
> -- "do-build" installs SOEM in in 
> "${GITDIR}/rtems-source-builder/rtems/build/tmp/soem-powerpc-rtems6-1-root-dufault/opt/flatland/opt/rtems-6"
>  instead of "/opt/flatland/opt/rtems-6".
> 
> - I then go to the generated directory 
> "${GITDIR}/rtems/rtems-source-builder/rtems/build/soem-powerpc-rtems6-1/build-xc"
>  and type "make install" The make succeeds, and *it installs SOEM in the 
> correct place* and SOEM is "successfully" built and installed.
> 
> - The last nit is that when I try to build the SOEM examples I need to export 
> LDFLAGS="-lrtemsdefaultconfig -lm" to get the SOEM examples to configure. 
> *Note* this was an issue before I switched to cmake3, this one is not new.
> 
> 
> I hope this gives someone enough clues to suggest what must be done to set 
> source builder up correctly.  For reference here's the script.
> 
> #!/bin/sh
> set -e
> # Script to get SOEM to build and install "properly".  I just need to be 
> insistent.
> 
> HERE=$(pwd)
> # Clean up environment  rsb-overrides only has a cmake shell script that is 
> "cmake $*".
> export PATH=${HOME}/bin/rsb-overrides:/usr/bin:/usr/sbin:/bin:/sbin
> unset LD_IBRARY_PATH
> 
> export RTEMS_GIT=/home/dufault/development/rtems
> export RTEMS_VER=6
> export RTEMS_SB=${RTEMS_GIT}/rtems-source-builder
> export RTEMS_SB_BUILD=${RTEMS_SB}/rtems/build
> export RTEMS_TOOLS=/opt/flatland/opt/rtems-${RTEMS_VER}
> export RTEMS_ARCH=powerpc
> export RTEMS_BSP=beatnik
> # "soem-rtems" fetches waf-2.0.4.
> SOEM_WAF=waf-2.0.4
> 
> SOEM_TIDY_UP="\
> ${RTEMS_TOOLS}/share/rtems/rsb/soem-${RTEMS_ARCH}-rtems${RTEMS_VER}-1.txt 
> \
> ${RTEMS_TOOLS}/share/rtems/rsb/soem-${RTEMS_ARCH}-rtems${RTEMS_VER}-1.xml 
> \
> 
> ${RTEMS_TOOLS}/${RTEMS_ARCH}-rtems${RTEMS_VER}/${RTEMS_BSP}/lib/share/soem \
> 
> ${RTEMS_TOOLS}/${RTEMS_ARCH}-rtems${RTEMS_VER}/${RTEMS_BSP}/lib/include/soem"
> 
> 
> # We have this setup: ${RTEMS_GIT}
> #|-- soem-rtems
> #|-- rtems
> #|-- rtems-source-builder
> 
> # First tidy-up so we always start in the same state.
> rm -rf ${RTEMS_SB_BUILD} ${SOEM_TIDY_UP}
> cd ${RTEMS_GIT}/soem-rtems
> if [ ! -e ${SOEM_WAF} ]; then
> wget -P . https://waf.io/${SOEM_WAF}
> chmod u+x ${SOEM_WAF}
> git submodule init
> git submodule update
> else
> ./${SOEM_WAF} distclean
> fi
> 
> cd ${RTEMS_SB}/rtems
> 
> RTEMS_SB_DEBUG="--no-clean --jobs=1"
> 
> # Now configure to build SOEM.
> # This will fail with:
> # 
> ${RTEMS_SB_BUILD}soem-${RTEMS_ARCH}-rtems${RTEMS_VER}-1/soem/oshw/rtems/nicdrv.c:37:10:
> #  fatal error: net/bpf.h: No such file or directory
> 
> if ${RTEMS_SB}/source-builder/sb-set-builder \
> --log=log_${RTEMS_ARCH}_soem \
> --prefix=${RTEMS_TOOLS} \
> --with-tools=${RTEMS_TOOLS} \
> --host=${RTEMS_ARCH}-rtems${RTEMS_VER} \
> --with-rtems-bsp=${RTEMS_BSP} \
> ${RTEMS_SB_DEBUG} \
> net/soem ; \
> then
> echo 2>&1 "*"
> echo 2>&1 

Re: [PATCH v4] sb: Merge mailer changes from rtems-tools

2021-05-26 Thread Chris Johns
On 12/5/21 3:19 am, Alex White wrote:
> This adds the improved mailer.py script from rtems-tools.
> 
> Closes #4388
> ---
>  source-builder/sb/mailer.py | 194 ++--
>  source-builder/sb/options.py|  26 -
>  source-builder/sb/setbuilder.py |   2 +
>  3 files changed, 189 insertions(+), 33 deletions(-)
> 
> diff --git a/source-builder/sb/mailer.py b/source-builder/sb/mailer.py
> index ff25df5..aafe6d6 100644
> --- a/source-builder/sb/mailer.py
> +++ b/source-builder/sb/mailer.py
> @@ -1,21 +1,33 @@
>  #
>  # RTEMS Tools Project (http://www.rtems.org/)
> -# Copyright 2013 Chris Johns (chr...@rtems.org)
> +# Copyright 2013-2016 Chris Johns (chr...@rtems.org)
> +# Copyright (C) 2021 On-Line Applications Research Corporation (OAR)
>  # All rights reserved.
>  #
>  # This file is part of the RTEMS Tools package in 'rtems-tools'.
>  #
> -# Permission to use, copy, modify, and/or distribute this software for any
> -# purpose with or without fee is hereby granted, provided that the above
> -# copyright notice and this permission notice appear in all copies.
> +# Redistribution and use in source and binary forms, with or without
> +# modification, are permitted provided that the following conditions are met:
> +#
> +# 1. Redistributions of source code must retain the above copyright notice,
> +# this list of conditions and the following disclaimer.
> +#
> +# 2. Redistributions in binary form must reproduce the above copyright 
> notice,
> +# this list of conditions and the following disclaimer in the documentation
> +# and/or other materials provided with the distribution.
> +#
> +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> +# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
> +# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> +# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> +# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> +# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> +# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> +# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> +# POSSIBILITY OF SUCH DAMAGE.
>  #
> -# THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> -# WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> -# MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> -# ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> -# WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> -# ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> -# OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>  
>  #
>  # Manage emailing results or reports.
> @@ -28,18 +40,72 @@ import smtplib
>  import socket
>  
>  from . import error
> +from . import execute
>  from . import options
>  from . import path
>  
> +_options = {
> +'--mail' : 'Send email report or results.',
> +'--use-gitconfig': 'Use mail configuration from git config.',
> +'--mail-to'  : 'Email address to send the email to.',
> +'--mail-from': 'Email address the report is from.',
> +'--smtp-host': 'SMTP host to send via.',
> +'--smtp-port': 'SMTP port to send via.',
> +'--smtp-user': 'User for SMTP authentication.',
> +'--smtp-password': 'Password for SMTP authentication.'
> +}
> +
>  def append_options(opts):
> -opts['--mail'] = 'Send email report or results.'
> -opts['--smtp-host'] = 'SMTP host to send via.'
> -opts['--mail-to'] = 'Email address to send the email too.'
> -opts['--mail-from'] = 'Email address the report is from.'
> +for o in _options:
> +opts[o] = _options[o]
> +
> +def add_arguments(argsp):
> +argsp.add_argument('--mail', help = _options['--mail'], action = 
> 'store_true')
> +argsp.add_argument('--use-gitconfig', help = 
> _options['--use-gitconfig'], action = 'store_true')
> +no_add = ['--mail', '--use-gitconfig']
> +for o in [opt for opt in list(_options) if opt not in no_add]:
> +argsp.add_argument(o, help = _options[o], type = str)
>  
>  class mail:
>  def __init__(self, opts):
>  self.opts = opts
> +self.gitconfig_lines = None
> +if opts.find_arg('--use-gitconfig') is not None:
> +# Read the output of `git config --lis

Re: AW: question about posix timer expiration

2021-05-26 Thread Chris Johns
On 21/5/21 9:44 pm, gabriel.moy...@dlr.de wrote:
> Did you compile ptpd with kqueue only? I’ve tried with posix timer, which are
> implemented in rtems, but it seems that the handler with siginfo_t is not 
> called
> correctly. I’ve managed to get it working using a workaround but I’ll continue
> with kqueue, thx!.

Yes I did build with kqueue. I added the kqueue support to ptpdv2 because libbsd
does not support signal type events. I found this out the hard why when getting
ptpdv2 to run. I had posix timers running but libbsd threads did not wake. I
chased this down to code like ...

https://git.rtems.org/rtems-libbsd/tree/rtemsbsd/rtems/rtems-kernel-muteximpl.c#n45

The state set does not include signals so the kernel does not wake the thread.

I discussed this with Sebastian at the time and agreed supporting signals in
libbsd was a lot of work and open a range of possible issues so it was simpler
to add kqueue support to ptpdv2.

I also agree with Sebastian that kqueue is a better interface than select etc
and we should encourage its use. I think ptpdv2 benefits from using kqueue over
select and posix timers.

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

Re: [PATCH] tester: Expand special case for minimum.exe

2021-05-26 Thread Chris Johns
On 27/5/21 12:06 pm, Joel Sherrill wrote:
> On Wed, May 26, 2021, 7:03 PM Chris Johns  <mailto:chr...@rtems.org>> wrote:
> 
> On 26/5/21 1:52 am, Kinsey Moore wrote:
> > The minimum.exe test case is expected to fail as an "invalid" test in
> > the tester since it is completely stripped down and does not output the
> > normal test header and footer. When fatal error detection support was
> > added, this caught minimum.exe and started flagging it as "fatal"
> > instead of "invalid". The special-case detection of minimum.exe only
> > matched on "invalid" results and not "fatal" results and so began
> > flagging minimum.exe as an actual failure.>
> > This change adds the special-case handling to the "fatal" test state
> > handling.
> 
> Is this the right solution?
> 
> Is minimum.exe suppose to run and not fail? It would seem easy to make a
> minimum.exe with nothing in it, ie minimal, that seems to pass. It would 
> make
> great marketing material.
> 
> What happens if minimum fails? I feel minimum needs to be able to run and 
> not
> fail to be a valid minimum.
> 
> 
> It is an empty thread body that doesn't print. I suppose we could add
> rtems_shutdown_executive(0) if that helps

What if the work to make it small removes something that is needed? Is minimum
suppose to be run and if it is how do we know it was successfull?

My point is about the purpose of minimum. If we can never tell a run failed
should it be run? If we cannot tell then excluding it as a test to run for all
BSPs may be a simpler option that this change.

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


  1   2   3   4   5   6   7   8   9   10   >