Re: [PATCH] tester: Limit simultaneous QEMU jobs to 1

2021-08-31 Thread Chris Johns
On 31/8/21 3:20 pm, Sebastian Huber wrote:
> On 30/08/2021 20:32, Kinsey Moore wrote:
>> On 8/30/2021 12:12, Sebastian Huber wrote:
>>> On 24/08/2021 20:45, Kinsey Moore wrote:
 diff --git a/tester/rtems/testing/bsps/a53_ilp32_qemu.ini
 b/tester/rtems/testing/bsps/a53_ilp32_qemu.ini
 index 3beba06..581c59c 100644
 --- a/tester/rtems/testing/bsps/a53_ilp32_qemu.ini
 +++ b/tester/rtems/testing/bsps/a53_ilp32_qemu.ini
 @@ -36,3 +36,4 @@ bsp   = a53_ilp32_qemu
   arch  = aarch64
   tester    = %{_rtscripts}/qemu.cfg
   bsp_qemu_opts = %{qemu_opts_base} -serial mon:stdio -machine
 virt,gic-version=3 -cpu cortex-a53 -m 4096
 +jobs  = 1
>>>
>>> Does this overwrite the command line option or is this a default value?
>>>
>> When this is set in the tester configuration, the command line switch has no
>> effect but it can be overridden in the user-config.
> 
> Overruling the command line option is not that great. I have a vastly 
> different
> test run duration with --jobs=1 vs. --jobs=48 with more or less the same test
> results. 

What does more or less mean?

I appreciate the efforts Kinsey has gone to looking into why we have this
happening and I also believe we need to keep pushing towards repeatable result.
If limiting to 1 gives us repeatable results on qemu then I prefer this over
tainted test results with intermittent tags.

> I think this option should be split into a "force-jobs" and
> "default-jobs" option.

I am sorry I do not understand these options?

The command line is ignored because and the value is fixed on purpose and I am
not seeing a reason to change this.

When specified in a config it is a physical limit. A user being able to change
the number of TFTP jobs on the command line does not make sense.

This tool's focus is testing on hardware and I see that as more important. And
as I have said before if we have problematic tests maybe the test or the tool
generating the results needs to be investigated.

I see this issue as something specific to the design of qemu and a few of our
tests. I can guess at some of the reasons qemu does this but also being able to
have the tick timer's clock be sync'ed with the CPU clock is important in some
types of simulation, ie our case and these problematic test. We are a real-time
operating system so needing this to be right to closer in simulation does not
seem unreasonable.

This discussion send a clear message, tier 1 archs and BSPs are very important
to this project.

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

Re: [PATCH 0/5] Preliminary Exception Manager Work

2021-08-27 Thread Chris Johns
On 24/8/21 10:34 pm, Kinsey Moore wrote:
> On 8/23/2021 21:41, Chris Johns wrote:
>> On 24/8/21 9:50 am, Kinsey Moore wrote:
>>> This patch set contains the result of the Exception Manager work I
>>> proposed a while back to manage handling of machine exceptions along
>>> with a general feature for mapping exceptions to POSIX signals without
>>> delving into the CPU Port-specific details. This is a pretty basic
>>> initial implementation, but it can easily be expanded with mutators for
>>> the CPU_Exception_frame for additional capabilities. Also included is a
>>> test that demonstrates usage of the Exception Manager and exception to
>>> signal mapping functionality.
>> Could you please provide a link to the previous discussion?
> 
> Most of the discussion occurred here:
> 
> https://lists.rtems.org/pipermail/devel/2021-April/066597.html
> 
> I sent out my consolidated thoughts here:
> 
> https://lists.rtems.org/pipermail/devel/2021-April/066902.html
> 
>> I have some concerns on how this interface and libdebugger are to work?
> 
> Theoretically, libdebugger should be able to be implemented on top of this 
> with
> some additional CPU_Exception_frame and machine state manipulators. I think I
> captured a reasonable set of generic exceptions for this purpose.

My brief review had me wondering about the switch statement to handle the paths
taken for each type of exception. For example a break point was in one group and
the data abort in another however libdebugger needs everything. Is this 
possible?

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


Re: [PATCH] aarch64: Add tests that are failing intermittently

2021-08-27 Thread Chris Johns
On 27/8/21 9:36 am, Kinsey Moore wrote:
> On 8/20/2021 22:06, Chris Johns wrote:
>> On 21/8/21 2:38 am, Kinsey Moore wrote:
>>> On 8/19/2021 18:03, Chris Johns wrote:
>>> My comment in that regard was that other system
>>> loading (or multiple simultaneous test runs) can also cause the same problem
>>> and so
>>> this is only a partial solution. Barring a fix for RTEMS or QEMU for these 
>>> load-
>>> dependent and sporadic failures, this at least still needs to be documented
>>> in some
>>> form.
>> Yes and the failures should highlight an issue on the host that needs to be
>> looked into.
> 
> Since I'm working on SMP and I've had some of those tests failing sporadically
> as well, I took a dive into smpschededf01.exe on AArch64 and the issue that
> particular test seems to be encountering is a mismatch between the busy wait
> delay using rtems_test_busy_cpu_usage() and the number of kernel ticks that 
> have
> been experienced. My hypothesis is that QEMU is prone to dumping a pile of 
> timer
> ticks into the virtual CPU all at once to catch up to wall time after 
> returning
> from a context switch on the host OS. This would support the observation that
> failures are sporadic and increase under system load.  I instrumented the code
> and can see that the loop in rtems_test_busy_cpu_usage() isn't running
> substantially between these tick interrupts if at all.

Oh that would confuse things.

> I guess my next step is seeing if QEMU has an option to run its timers closer 
> to
> the illusion of metal instead of being based on the wall clock.

QEMU would need to handle instruction or a CPU timer to manage this.

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

Re: [PATCH v2] tester: Add MicroBlaze KCU105 QEMU BSP

2021-08-25 Thread Chris Johns
Hi Alex,

OK to push and thanks

Chris

On 26/8/21 12:44 pm, Alex White wrote:
> ---
>  tester/rtems/testing/bsps/kcu105_qemu.ini | 38 +++
>  tester/rtems/testing/qemu.cfg |  2 +-
>  2 files changed, 39 insertions(+), 1 deletion(-)
>  create mode 100644 tester/rtems/testing/bsps/kcu105_qemu.ini
> 
> diff --git a/tester/rtems/testing/bsps/kcu105_qemu.ini 
> b/tester/rtems/testing/bsps/kcu105_qemu.ini
> new file mode 100644
> index 000..f61a3e3
> --- /dev/null
> +++ b/tester/rtems/testing/bsps/kcu105_qemu.ini
> @@ -0,0 +1,38 @@
> +#
> +# RTEMS Tools Project (http://www.rtems.org/)
> +# Copyright 2021 On-Line Applications Research Corporation (OAR).
> +# All rights reserved.
> +#
> +# This file is part of the RTEMS Tools package in 'rtems-tools'.
> +#
> +# 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 MicroBlaze KCU105 QEMU BSP.
> +#
> +[kcu105_qemu]
> +bsp   = kcu105_qemu
> +arch  = microblazeel
> +tester= %{_rtscripts}/qemu.cfg
> +bsp_qemu_opts = %{qemu_opts_base} -M microblaze-fdt-plnx -m 256 -serial 
> mon:stdio -display none
> diff --git a/tester/rtems/testing/qemu.cfg b/tester/rtems/testing/qemu.cfg
> index efaef8d..ad14cdb 100644
> --- a/tester/rtems/testing/qemu.cfg
> +++ b/tester/rtems/testing/qemu.cfg
> @@ -82,4 +82,4 @@
>  #
>  # Executable
>  #
> -%execute %{qemu_cmd} %{qemu_opts} -kernel %{test_executable}
> +%execute %{qemu_cmd} %{qemu_opts} %{bsp_qemu_extra_opts} -kernel 
> %{test_executable}
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: [PATCH rtems v1 2/2] bsps/zynqmp: Added I2C support for ZynqMP

2021-08-24 Thread Chris Johns
On 25/8/21 12:05 am, Stephen Clark wrote:
> This approach was also used in bsps/arm/xilinx-zynq/include/bsp/i2c.h. I kept 
> it specifically for consistency; I assumed it was the standard approach, but 
> your response makes me think it's not.

I am also not sure and why I asked :)

> Is there a case to be made for breaking the register functions in both i2c.h 
> files out into their own c files?

I do not know. There can be a tendency to place things into line functions that
do not need to be. It saves work by not need to touch the build system. If the
code is being moved should it be moved to a C file? This is the reason I raised
this. I am happy to follow what ever Joel thinks.

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


Re: [PATCH] tester: Add MicroBlaze KCU105 QEMU BSP

2021-08-24 Thread Chris Johns
On 25/8/21 7:46 am, Alex White wrote:
> On Mon, Aug 23, 2021 at 9:29 PM Chris Johns  wrote:
>>
>> Hi,
>>
>> Could you please explain this file?
>>
>> Where is the source?
>>
>> Why would we allow a binay blob into the tester like this?
> 
> Hi Chris,
> 
> Running MicroBlaze tests on QEMU requires an appropriate DTB file to pass to
> QEMU via the "-hw-dtb" flag. There does not appear to be a way to pass the
> device tree source to QEMU.
> 
> This particular DTB comes from Xilinx as part of their KCU105 BSP.

The file you posted has no attribution and that means we cannot accept it. There
is also no source and so the binary blob is not acceptable. Anyway there are
better solutions to solve this.

> It seems most
> sensible to store it next to the tester configuration similar to the other
> device tree files in that directory (psim-device-tree, etc.).

In my view the device tree files are not the same.

> I realize that this is a binary file and the other device tree files in
> tester/rtems/testing/bsps are not, but it seems that this needs to live
> alongside the BSP configuration so that it can be referenced by 
> kcu105_qemu.ini.

I do not agree.

>> This seems specific to a set up or a BSP and not the tester. I am not
>> comfortable with this approach. Have alternative approaches have you 
>> considered?
> 
> I am not aware of another way that would ensure consistency with the tester
> configuration. 

The tester is designed to handle this. The user focused INI files:

https://docs.rtems.org/branches/master/user/testing/configuration.html#bsp-and-user-configuration

You provide for your site are imported here:

https://git.rtems.org/rtems-tools/tree/tester/rt/config.py#n468

The key/value pairs are entered into the macro defaults. This means you can add
`%{bsp_qemu_extra_opts}` to the execute command in `qemu.ini` and then you can
create a suitable site specific INI file to match your BSP.

Where you then hold the DTB and the DTS becomes a separate and wider discussion.

> We could require that a user obtain the DTB from elsewhere and
> copy it to the correct directory, but that seems a lot more involved than just
> including the needed file and would be more difficult to keep consistent with
> any future changes that might be needed.

Yes it is more involved but how would the user of the BSP you are doing the work
for handle that same situation for their custom BSP? They cannot post a closed
DTB for their project to this list in the hope we commit it and I would not
suggest to them they create a branch in their repo to hold it. We have 2 roles,
one maintaining RTEMS and BSPs and another developing a suitable eco-system our
users can extend without touching the built tools we provide.

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


Re: [PATCH 0/5] Preliminary Exception Manager Work

2021-08-23 Thread Chris Johns
On 24/8/21 9:50 am, Kinsey Moore wrote:
> This patch set contains the result of the Exception Manager work I
> proposed a while back to manage handling of machine exceptions along
> with a general feature for mapping exceptions to POSIX signals without
> delving into the CPU Port-specific details. This is a pretty basic
> initial implementation, but it can easily be expanded with mutators for
> the CPU_Exception_frame for additional capabilities. Also included is a
> test that demonstrates usage of the Exception Manager and exception to
> signal mapping functionality.

Could you please provide a link to the previous discussion?

I have some concerns on how this interface and libdebugger are to work?

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


Re: [PATCH rtems v1 2/2] bsps/zynqmp: Added I2C support for ZynqMP

2021-08-23 Thread Chris Johns
On 24/8/21 8:24 am, Stephen Clark wrote:
> Added I2C drivers for ZynqMP and updated build system accordingly.
> ---
>  bsps/aarch64/xilinx-zynqmp/include/bsp.h  |  4 ++
>  bsps/aarch64/xilinx-zynqmp/include/bsp/i2c.h  | 63 +++
>  bsps/aarch64/xilinx-zynqmp/include/bsp/irq.h  |  2 +
>  bsps/aarch64/xilinx-zynqmp/start/bspstart.c   | 10 +++
>  spec/build/bsps/aarch64/xilinx-zynqmp/grp.yml |  4 ++
>  .../bsps/aarch64/xilinx-zynqmp/grp_zu3eg.yml  |  2 +
>  .../aarch64/xilinx-zynqmp/objcadencei2c.yml   | 19 ++
>  .../bsps/aarch64/xilinx-zynqmp/optclki2c0.yml | 19 ++
>  .../bsps/aarch64/xilinx-zynqmp/optclki2c1.yml | 19 ++
>  9 files changed, 142 insertions(+)
>  create mode 100644 bsps/aarch64/xilinx-zynqmp/include/bsp/i2c.h
>  create mode 100644 spec/build/bsps/aarch64/xilinx-zynqmp/objcadencei2c.yml
>  create mode 100644 spec/build/bsps/aarch64/xilinx-zynqmp/optclki2c0.yml
>  create mode 100644 spec/build/bsps/aarch64/xilinx-zynqmp/optclki2c1.yml
> 
> diff --git a/bsps/aarch64/xilinx-zynqmp/include/bsp.h 
> b/bsps/aarch64/xilinx-zynqmp/include/bsp.h
> index 83f2e2f4e4..6d49b9ad2a 100644
> --- a/bsps/aarch64/xilinx-zynqmp/include/bsp.h
> +++ b/bsps/aarch64/xilinx-zynqmp/include/bsp.h
> @@ -70,6 +70,10 @@ BSP_START_TEXT_SECTION void 
> zynqmp_setup_mmu_and_cache(void);
>  
>  void zynqmp_debug_console_flush(void);
>  
> +uint32_t zynqmp_clock_i2c0(void);
> +
> +uint32_t zynqmp_clock_i2c1(void);
> +
>  #ifdef __cplusplus
>  }
>  #endif /* __cplusplus */
> diff --git a/bsps/aarch64/xilinx-zynqmp/include/bsp/i2c.h 
> b/bsps/aarch64/xilinx-zynqmp/include/bsp/i2c.h
> new file mode 100644
> index 00..e09747d414
> --- /dev/null
> +++ b/bsps/aarch64/xilinx-zynqmp/include/bsp/i2c.h
> @@ -0,0 +1,63 @@
> +/*
> + * SPDX-License-Identifier: BSD-2-Clause
> + *
> + * Copyright (C) 2021 On-Line Applications Research (OAR)
> + *
> + * 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 OWNER 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.
> + */
> +
> +#ifndef LIBBSP_ARM_XILINX_ZYNQ_I2C_H
> +#define LIBBSP_ARM_XILINX_ZYNQ_I2C_H
> +
> +#include 
> +#include 
> +#include 
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif /* __cplusplus */
> +
> +static inline int zynqmp_register_i2c_0(void)
> +{
> +  return i2c_bus_register_cadence(
> +"/dev/i2c-0",
> +0x00FF02,
> +zynqmp_clock_i2c0(),
> +ZYNQMP_IRQ_I2C_0
> +  );
> +}
> +
> +static inline int zynqmp_register_i2c_1(void)
> +{
> +  return i2c_bus_register_cadence(
> +"/dev/i2c-1",
> +0x00FF03,
> +zynqmp_clock_i2c1(),
> +ZYNQMP_IRQ_I2C_1
> +  );

I know these are currently inlined but I do not know why they are. It is the
only BSP that does this. Should they be moved to a .c file seem they are being
touched?

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


Re: [PATCH] tester: Add MicroBlaze KCU105 QEMU BSP

2021-08-23 Thread Chris Johns
Hi,

Could you please explain this file?

Where is the source?

Why would we allow a binay blob into the tester like this?

This seems specific to a set up or a BSP and not the tester. I am not
comfortable with this approach. Have alternative approaches have you considered?

Chris

On 24/8/21 4:46 am, Alex White wrote:
> ---
>  tester/rtems/testing/bsps/kcu105.dtb  | Bin 0 -> 15256 bytes
>  tester/rtems/testing/bsps/kcu105_qemu.ini |  38 ++
>  2 files changed, 38 insertions(+)
>  create mode 100644 tester/rtems/testing/bsps/kcu105.dtb
>  create mode 100644 tester/rtems/testing/bsps/kcu105_qemu.ini
> 
> diff --git a/tester/rtems/testing/bsps/kcu105.dtb 
> b/tester/rtems/testing/bsps/kcu105.dtb
> new file mode 100644
> index 
> ..998e8c03b74d8d9ddc82b5f6fa53242415abbd84
> GIT binary patch
> literal 15256
>

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


Re: [PATCH] aarch64: Add tests that are failing intermittently

2021-08-20 Thread Chris Johns
On 21/8/21 2:38 am, Kinsey Moore wrote:
> On 8/19/2021 18:03, Chris Johns wrote:
>> On 20/8/21 4:55 am, Kinsey Moore wrote:
>>> On 8/19/2021 13:32, Gedare Bloom wrote:
>>>> On Thu, Aug 19, 2021 at 11:43 AM Kinsey Moore  
>>>> wrote:
>>>>> I've seen these failures on my local system, in our CI, and on a build
>>>>> server that I sometimes
>>>>> use for development/testing so if it's a configuration issue we're being
>>>>> pretty consistent about
>>>>> misconfiguration across some pretty different environments (docker,
>>>>> bare-metal, VM, different
>>>>> OSs, different QEMU versions). I've seen enough of the spintrcritical
>>>>> tests fail sporadically on
>>>>> QEMU to lump them all into this category. These are also tests that I
>>>>> have seen behave badly
>>>>> on ARMv7 QEMU on my local system (which doesn't rule out
>>>>> misconfiguration, but it's another
>>>>> data point).
>>>>>
>>>> Yes, for example, it may be a matter of qemu process counts spawned by
>>>> rtems-test, and the order in which tests get invoked could be a cause
>>>> for which ones don't work. I could easily see this happening, since
>>>> each test runtime will be fairly consistent, so you'll often see the
>>>> same tests running concurrently with each other. But, if you change
>>>> the order (e.g., by adding new tests), then we may see a new set of
>>>> sporadically failing testcases, will we just add those, or do we need
>>>> to re-examine this indetermine set periodically? Who will maintain
>>>> this list? That's kind of the root of my concern here.
>>> I understand your concern about maintenance of the failure list and I don't
>>> have a good answer for you. I imagine going forward it would be a 
>>> combination
>>> of the current stake-holders for a given BSP and anyone who watches the
>>> automated build output from Joel's runs for these kinds of issues.
>>>
>>> On the other hand if we don't mark those tests, people will get fatigued
>>> looking at the spurious failures and assume any new ones just fall into the
>>> same category as others. At that point is it even worth running the
>>> automated tests for that platform?
>>>
>>>>> As far as your worry about marking these indeterminate, they're only
>>>>> being marked as such for
>>>>> QEMU BSPs. The ZynqMP hardware BSP doesn't have these testing carve-outs
>>>>> and runs all these tests flawlessly.
>> Great, this is important.
>>
>>>>> These failures become much more common when there is otherwise load on
>>>>> the system and a
>>>>> lot of them disappear when you limit the tester to a single QEMU
>>>>> instance at a time.
>>>>>
>>>> I'm wondering if we should sacrifice testing speed for
>>>> coverage/quality. If throttling rtems-test leads to more reliable test
>>>> results, then it may be a better option than basically ignoring a
>>>> swath of our testsuite.
>>> That would certainly mitigate some of the failures, but you'd also have to
>>> guarantee nothing else is running on the system which could cause the same
>>> problem. I know at least some of the current automated runs operate on a
>>> shared system which can and does often have other intensive processes
>>> running on it. There are also the tests that are sporadic on QEMU even
>>> without additional load.
>> What is it in these tests when combined with qemu that causes the tests to 
>> fail?
>> Is there some relation to a real clock, some shared host resource or a bug in
>> qemu? I am concerned a simulator can vary like this based on the host's load 
>> and
>> it makes me wonder how people use it on machines to host a number VMs.
> I experienced very similar results on an ARMv7 BSP (not Zynq) and assumed 
> that this
> was a known/accepted problem with QEMU when the same issues popped up on
> AArch64.

I think we have just ignored issue. I know I have ignored it because of the
rabbit hole it is.

> My local system under no other load produces these failures for the
> Zynq A9 QEMU
> BSP:
> 
>     "failed": [
>     "spcpucounter01.exe",
>     "psxtimes01.exe",
>     "sp69.exe",
>     "psx12.exe",
>     "minimum.exe",
>     &

Re: [PATCH] gdb: prefere python3 if it is installed

2021-08-20 Thread Chris Johns
On 21/8/21 4:08 am, o...@c-mauderer.de wrote:
> Started to test both (MSYS2 and Cygwin) and I start to suspect that our manual
> needs some minor updates. I'm taking notes ...

Oh awesome, I am sooo grateful.

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


Re: [PATCH] gdb: prefere python3 if it is installed

2021-08-20 Thread Chris Johns
On 20/8/21 5:42 pm, Christian MAUDERER wrote:
> Am 20.08.21 um 09:02 schrieb Chris Johns:
>> On 20/8/21 4:48 pm, Christian MAUDERER wrote:
>>> Hello,
>>>
>>> Am 20.08.21 um 03:49 schrieb Chris Johns:
>>>> On 20/8/21 3:16 am, Joel Sherrill wrote:
>>>>> On Thu, Aug 19, 2021 at 11:51 AM Gedare Bloom  wrote:
>>>>>>
>>>>>> I have no problem with this. I think it is sensible to look for
>>>>>> python3 before python2. At some point we'll have to stop looking for
>>>>>> python2 :)
>>>>>
>>>>> That is further in the future than I would have thought based on the
>>>>> CentOS project changes. I still see user organizations with no plans
>>>>> to move off CentOS 7. It will receive maintenance updates through
>>>>> 2024-06-30.
>>>>>
>>>>> But on CentOS 7, I can load a Python 3.6, newer GCC, etc. so it isn't
>>>>> that bad. I had to test something on a true 32-bit distribution this week
>>>>> and even CentOS 7 (i386) wasn't as painful as I expected to setup.
>>>>
>>>> There will come a time when a change made cannot be easily tested by us on
>>>> python2 and that will in effect end our support. We are not there yet.
>>>
>>> STOP.
>>
>> Please do not do this again.
>>
>> That discussion took a wrong direction. We discussed deprecating python2 a
>>> few times and I know that we will not do it before the big long living
>>> distributions drop it. I can live with that and I don't want to re-start 
>>> this
>>> discussion with this patch.
>>
>> If Joel feels the need to make this statement he can and he has more than 
>> earned
>> the right to do so. His work and those he supports is important.
> 
> Sorry, I didn't want to offend you, Joel or Gedare.
> 
> I had that with discussions before that the original idea (here: giving 
> priority
> to python3 over python2 when building gdb) started to be buried by a similar 
> but
> slightly different one which has been already discussed multiple times (here:
> deprecating python2). I wanted to avoid that.
> 
> I should have worded that different. I'm sorry if I have offended you, Joel or
> Gedare or anyone else.

Thanks and none taken. I am clear on the focus of your posts so it is ok. :)

> 
>>
>>>
>>>>
>>>>>> On Thu, Aug 19, 2021 at 3:24 AM Christian MAUDERER
>>>>>>  wrote:
>>>>>>>
>>>>>>> PS: I had the problem on the 5 branch of RTEMS source builder. I think
>>>>>>> we should apply a patch to both: master and the 5 branch.
>>>>>>>
>>>>>>> Am 19.08.21 um 10:34 schrieb Christian Mauderer:
>>>>>>>> More and more systems stop shipping python2. So we should start to
>>>>>>>> prefer python3 over python2. For building gdb it is not only necessary
>>>>>>>> to have a python binary installed, but also the matching python-devel
>>>>>>>> packet. On a lot of hosts that will now be more often python3-devel
>>>>>>>> and not python2-devel.
>>>>>>>> ---
>>>>>>>>
>>>>>>>> Note: Please see that patch more as an suggestion / base for
>>>>>>>> discussion. I'm open to better solutions.
>>>>>>>>
>>>>>>>> My problem that triggered this patch was a build of a toolchain on a
>>>>>>>> github CI/CD system that has been originally set up by one of our
>>>>>>>> users (see [1] for the log). It seems that on the "macos-latest"
>>>>>>>> machines a python2 is installed but no python2 headers are found.
>>>>
>>>> I have just built the latest RSB master GDB on a fully updated MacOS (Big 
>>>> Sur):
>>>>
>>>> config: tools/rtems-gdb-10.cfg
>>>> package: arm-rtems6-gdb-7ab567f-x86_64-apple-darwin20.1.0-1
>>>> building: arm-rtems6-gdb-7ab567f-x86_64-apple-darwin20.1.0-1
>>>> sizes: arm-rtems6-gdb-7ab567f-x86_64-apple-darwin20.1.0-1: 629.086MB
>>>> (installed:
>>>> 19.586MB)
>>>> cleaning: arm-rtems6-gdb-7ab567f-x86_64-apple-darwin20.1.0-1
>>>> reporting: tools/rtems-gdb-10.cfg ->
>>>> arm-rtems6-gdb-7ab567f-x86_64-apple-darwin20.1.0-1.txt
>>>> reporting: tools/rtems-gdb-10.cfg ->
>>>> arm-r

Re: [PATCH] gdb: prefere python3 if it is installed

2021-08-20 Thread Chris Johns
On 20/8/21 4:48 pm, Christian MAUDERER wrote:
> Hello,
> 
> Am 20.08.21 um 03:49 schrieb Chris Johns:
>> On 20/8/21 3:16 am, Joel Sherrill wrote:
>>> On Thu, Aug 19, 2021 at 11:51 AM Gedare Bloom  wrote:
>>>>
>>>> I have no problem with this. I think it is sensible to look for
>>>> python3 before python2. At some point we'll have to stop looking for
>>>> python2 :)
>>>
>>> That is further in the future than I would have thought based on the
>>> CentOS project changes. I still see user organizations with no plans
>>> to move off CentOS 7. It will receive maintenance updates through
>>> 2024-06-30.
>>>
>>> But on CentOS 7, I can load a Python 3.6, newer GCC, etc. so it isn't
>>> that bad. I had to test something on a true 32-bit distribution this week
>>> and even CentOS 7 (i386) wasn't as painful as I expected to setup.
>>
>> There will come a time when a change made cannot be easily tested by us on
>> python2 and that will in effect end our support. We are not there yet.
> 
> STOP. 

Please do not do this again.

That discussion took a wrong direction. We discussed deprecating python2 a
> few times and I know that we will not do it before the big long living
> distributions drop it. I can live with that and I don't want to re-start this
> discussion with this patch.

If Joel feels the need to make this statement he can and he has more than earned
the right to do so. His work and those he supports is important.

> 
>>
>>>> On Thu, Aug 19, 2021 at 3:24 AM Christian MAUDERER
>>>>  wrote:
>>>>>
>>>>> PS: I had the problem on the 5 branch of RTEMS source builder. I think
>>>>> we should apply a patch to both: master and the 5 branch.
>>>>>
>>>>> Am 19.08.21 um 10:34 schrieb Christian Mauderer:
>>>>>> More and more systems stop shipping python2. So we should start to
>>>>>> prefer python3 over python2. For building gdb it is not only necessary
>>>>>> to have a python binary installed, but also the matching python-devel
>>>>>> packet. On a lot of hosts that will now be more often python3-devel
>>>>>> and not python2-devel.
>>>>>> ---
>>>>>>
>>>>>> Note: Please see that patch more as an suggestion / base for
>>>>>> discussion. I'm open to better solutions.
>>>>>>
>>>>>> My problem that triggered this patch was a build of a toolchain on a
>>>>>> github CI/CD system that has been originally set up by one of our
>>>>>> users (see [1] for the log). It seems that on the "macos-latest"
>>>>>> machines a python2 is installed but no python2 headers are found.
>>
>> I have just built the latest RSB master GDB on a fully updated MacOS (Big 
>> Sur):
>>
>> config: tools/rtems-gdb-10.cfg
>> package: arm-rtems6-gdb-7ab567f-x86_64-apple-darwin20.1.0-1
>> building: arm-rtems6-gdb-7ab567f-x86_64-apple-darwin20.1.0-1
>> sizes: arm-rtems6-gdb-7ab567f-x86_64-apple-darwin20.1.0-1: 629.086MB 
>> (installed:
>> 19.586MB)
>> cleaning: arm-rtems6-gdb-7ab567f-x86_64-apple-darwin20.1.0-1
>> reporting: tools/rtems-gdb-10.cfg ->
>> arm-rtems6-gdb-7ab567f-x86_64-apple-darwin20.1.0-1.txt
>> reporting: tools/rtems-gdb-10.cfg ->
>> arm-rtems6-gdb-7ab567f-x86_64-apple-darwin20.1.0-1.xml
>>
>> I support the latest MacOS with Xcode and have a dedicated Mac machine that
>> tracks the current RSB. It had wedged itself and I have cleared that so it is
>> now building the latest.
> 
> So do I understand that correct: This is a "clean" Mac with no additional 
> python
> installs or similar and it worked? What python versions are available on this
> this machine?

It is working for me and I have not looked into it any more.

> If I did understand you correctly, it seems that github has made some odd
> choices about their default config for the macos-latest machines. They seem to
> have python2 but no devel headers.

You will need to raise that with them.

>>>>>> Homebrew - which could be used earlier on MacOS to install the
>>>>>> necessary headers - dropped the python@2 packet.
>>
>> Homebrew and MacPorts are a personal choice and I am fine with users heading
>> down this path however it is beyond this project's scope to support those
>> frameworks. I have posted the reasons in the past and the MacPorts 
>> maintainers
>> are aware of those reasons.
> 
> No problem. I'm not a Ma

Re: [PATCH v4 0/2] Add MicroBlaze tools to RSB

2021-08-19 Thread Chris Johns
Hi Alex,

These patches are OK to push.

One minor point, the script sha512-base64 in the RSB creates a base64 encoded
hash and that makes them smaller. I prefer we move to this more and more.

Chris

On 19/8/21 1:34 pm, Alex White wrote:
> Hi,
> 
> This patch set now depends on Joel's newlib hash bump and should be merged 
> after
> it.
> 
> v4:
>   - Rename to rtems-xilinx-gcc-10-newlib-head.cfg
>   - Simplify rtems-xilinx-gcc-10-newlib-head.cfg with include
> 
> v3:
>   - Follow naming convention for Xilinx .cfg files
>   - Simplify rtems-xilinx-binutils-2.36.cfg with rtems-binutils-2.36.cfg 
> include
> 
> v2:
>   - Add Xilinx QEMU version to .cfg filename
>   - Exclude GDB configuration with unneeded Xilinx patches
> 
> This patch set adds the MicroBlaze tools to rtems-source-builder.
> 
> Thanks,
> 
> Alex
> 
> Alex White (2):
>   rsb: Add MicroBlaze tools
>   rsb: Add Xilinx QEMU
> 
>  bare/config/devel/qemu-xilinx-v2020.2-1.cfg   | 23 ++
>  bare/config/devel/qemu-xilinx.bset| 24 ++
>  rtems/config/6/rtems-microblaze.bset  | 23 +-
>  .../tools/rtems-xilinx-binutils-2.36.cfg  | 40 
>  .../tools/rtems-xilinx-gcc-10-newlib-head.cfg | 46 +++
>  5 files changed, 155 insertions(+), 1 deletion(-)
>  create mode 100644 bare/config/devel/qemu-xilinx-v2020.2-1.cfg
>  create mode 100644 bare/config/devel/qemu-xilinx.bset
>  create mode 100644 rtems/config/tools/rtems-xilinx-binutils-2.36.cfg
>  create mode 100644 rtems/config/tools/rtems-xilinx-gcc-10-newlib-head.cfg
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH] gdb: prefere python3 if it is installed

2021-08-19 Thread Chris Johns
On 20/8/21 3:16 am, Joel Sherrill wrote:
> On Thu, Aug 19, 2021 at 11:51 AM Gedare Bloom  wrote:
>>
>> I have no problem with this. I think it is sensible to look for
>> python3 before python2. At some point we'll have to stop looking for
>> python2 :)
> 
> That is further in the future than I would have thought based on the
> CentOS project changes. I still see user organizations with no plans
> to move off CentOS 7. It will receive maintenance updates through
> 2024-06-30.
> 
> But on CentOS 7, I can load a Python 3.6, newer GCC, etc. so it isn't
> that bad. I had to test something on a true 32-bit distribution this week
> and even CentOS 7 (i386) wasn't as painful as I expected to setup.

There will come a time when a change made cannot be easily tested by us on
python2 and that will in effect end our support. We are not there yet.

>> On Thu, Aug 19, 2021 at 3:24 AM Christian MAUDERER
>>  wrote:
>>>
>>> PS: I had the problem on the 5 branch of RTEMS source builder. I think
>>> we should apply a patch to both: master and the 5 branch.
>>>
>>> Am 19.08.21 um 10:34 schrieb Christian Mauderer:
 More and more systems stop shipping python2. So we should start to
 prefer python3 over python2. For building gdb it is not only necessary
 to have a python binary installed, but also the matching python-devel
 packet. On a lot of hosts that will now be more often python3-devel
 and not python2-devel.
 ---

 Note: Please see that patch more as an suggestion / base for
 discussion. I'm open to better solutions.

 My problem that triggered this patch was a build of a toolchain on a
 github CI/CD system that has been originally set up by one of our
 users (see [1] for the log). It seems that on the "macos-latest"
 machines a python2 is installed but no python2 headers are found.

I have just built the latest RSB master GDB on a fully updated MacOS (Big Sur):

config: tools/rtems-gdb-10.cfg
package: arm-rtems6-gdb-7ab567f-x86_64-apple-darwin20.1.0-1
building: arm-rtems6-gdb-7ab567f-x86_64-apple-darwin20.1.0-1
sizes: arm-rtems6-gdb-7ab567f-x86_64-apple-darwin20.1.0-1: 629.086MB (installed:
19.586MB)
cleaning: arm-rtems6-gdb-7ab567f-x86_64-apple-darwin20.1.0-1
reporting: tools/rtems-gdb-10.cfg ->
arm-rtems6-gdb-7ab567f-x86_64-apple-darwin20.1.0-1.txt
reporting: tools/rtems-gdb-10.cfg ->
arm-rtems6-gdb-7ab567f-x86_64-apple-darwin20.1.0-1.xml

I support the latest MacOS with Xcode and have a dedicated Mac machine that
tracks the current RSB. It had wedged itself and I have cleared that so it is
now building the latest.

 Homebrew - which could be used earlier on MacOS to install the
 necessary headers - dropped the python@2 packet. 

Homebrew and MacPorts are a personal choice and I am fine with users heading
down this path however it is beyond this project's scope to support those
frameworks. I have posted the reasons in the past and the MacPorts maintainers
are aware of those reasons.

 So it seems that on a
 modern MacOS there is no possibility to get python2 headers.

As I have just built GDB this does not appear to be true. I would prefer we do
not overreach until we all agree there is a problem. If there is an issue I
would raise the problem in Apple'd developer bug system. I have raised a number
of issues over the years and to Apple's credit they have dealt with them all.

 If
 python2 is still installed on the machine (for whatever reason), it is
 not possible to successfully use RTEMS source builder to build a gdb.
 If python3 is preferred instead, that should solve the problem.

The change seems sensible.

 Note that at the moment I only tried it on my OpenSUSE-Linux machine.
 For that I made sure that I have python2 and python3 installed but no
 python-devel (which is python2 on OpenSUSE). Earlier I know that I
 needed python-devel to build gdb. With this patch, I can build with
 only python3-devel.

 I'll try to add that patch to the CI too to see whether it works on
 MacOS. But I'm a bit unsure what other problems this patch could
 trigger and therefore I want to start a discussion early.

 Best regards

 Christian

 [1] https://github.com/grisp/grisp2-rtems-toolchain/runs/3362717606

I am looking forward to their hardware being shipped.

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


Re: Using libbsd dhcpcd options

2021-08-19 Thread Chris Johns
On 20/8/21 1:45 am, junkes wrote:
> Hallo Peter,
> I do not know if this is best practice but I run the following code:
> 
> static void
> default_network_dhcpcd(void)
> {
>     static const char default_cfg[] = "clientid test client\n";
>     rtems_status_code sc;
>     int fd;
>     int rv;
>     ssize_t n;
>     struct stat statbuf;
> 
>     if (ENOENT == stat("/etc/dhcpcd.conf", )) {
>     fd = open("/etc/dhcpcd.conf", O_CREAT | O_WRONLY,
>   S_IRWXU | S_IRWXG | S_IRWXO);
>     assert(fd >= 0);
> 
>     n = write(fd, default_cfg, sizeof(default_cfg) - 1);
>     assert(n == (ssize_t) sizeof(default_cfg) - 1);
> 
>     static const char fhi_cfg[] =
>     "nodhcp6\n"
>     "ipv4only\n"
>     "option ntp_servers\n"
>     "option rtems_cmdline\n"
>     "option tftp_server_name\n"
>     "option bootfile_name\n"
>     "define 129 string rtems_cmdline\n"
>     "timeout 0";
> 
>     n = write(fd, fhi_cfg, sizeof(fhi_cfg) - 1);
>     assert(n == (ssize_t) sizeof(fhi_cfg) - 1);
> 
>     rv = close(fd);
>     assert(rv == 0);
>     }
> 
>     sc = rtems_dhcpcd_start(NULL);
>     assert(sc == RTEMS_SUCCESSFUL);
> }
Hienz, this is a good solution and using a file best solution.

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

Re: [PATCH] aarch64: Add tests that are failing intermittently

2021-08-19 Thread Chris Johns
On 20/8/21 4:55 am, Kinsey Moore wrote:
> On 8/19/2021 13:32, Gedare Bloom wrote:
>> On Thu, Aug 19, 2021 at 11:43 AM Kinsey Moore  
>> wrote:
>>> I've seen these failures on my local system, in our CI, and on a build
>>> server that I sometimes
>>> use for development/testing so if it's a configuration issue we're being
>>> pretty consistent about
>>> misconfiguration across some pretty different environments (docker,
>>> bare-metal, VM, different
>>> OSs, different QEMU versions). I've seen enough of the spintrcritical
>>> tests fail sporadically on
>>> QEMU to lump them all into this category. These are also tests that I
>>> have seen behave badly
>>> on ARMv7 QEMU on my local system (which doesn't rule out
>>> misconfiguration, but it's another
>>> data point).
>>>
>> Yes, for example, it may be a matter of qemu process counts spawned by
>> rtems-test, and the order in which tests get invoked could be a cause
>> for which ones don't work. I could easily see this happening, since
>> each test runtime will be fairly consistent, so you'll often see the
>> same tests running concurrently with each other. But, if you change
>> the order (e.g., by adding new tests), then we may see a new set of
>> sporadically failing testcases, will we just add those, or do we need
>> to re-examine this indetermine set periodically? Who will maintain
>> this list? That's kind of the root of my concern here.
> I understand your concern about maintenance of the failure list and I don't
> have a good answer for you. I imagine going forward it would be a combination
> of the current stake-holders for a given BSP and anyone who watches the
> automated build output from Joel's runs for these kinds of issues.
> 
> On the other hand if we don't mark those tests, people will get fatigued
> looking at the spurious failures and assume any new ones just fall into the
> same category as others. At that point is it even worth running the
> automated tests for that platform?
> 
>>
>>> As far as your worry about marking these indeterminate, they're only
>>> being marked as such for
>>> QEMU BSPs. The ZynqMP hardware BSP doesn't have these testing carve-outs
>>> and runs all these tests flawlessly.

Great, this is important.

>>> These failures become much more common when there is otherwise load on
>>> the system and a
>>> lot of them disappear when you limit the tester to a single QEMU
>>> instance at a time.
>>>
>> I'm wondering if we should sacrifice testing speed for
>> coverage/quality. If throttling rtems-test leads to more reliable test
>> results, then it may be a better option than basically ignoring a
>> swath of our testsuite.
> That would certainly mitigate some of the failures, but you'd also have to
> guarantee nothing else is running on the system which could cause the same
> problem. I know at least some of the current automated runs operate on a
> shared system which can and does often have other intensive processes
> running on it. There are also the tests that are sporadic on QEMU even
> without additional load.

What is it in these tests when combined with qemu that causes the tests to fail?
Is there some relation to a real clock, some shared host resource or a bug in
qemu? I am concerned a simulator can vary like this based on the host's load and
it makes me wonder how people use it on machines to host a number VMs.

I feel with this volume of tests being tagged this way we should have a better
understanding of the problem and so a means to track or not track how to resolve
it. As Gedare has kindly stated once pushed this change disappears into a dark
corner and we have no means to track it.

The other solution is to set `jobs` to `1` in this BSP's tester config, again
something Gedare has raised. It means we get better or even valid results. What
is more important, valid results or running the testsuite as fast as possible?

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


Re: [PATCH] aarch64: Add tests that are failing intermittently

2021-08-19 Thread Chris Johns
On 20/8/21 2:58 am, Gedare Bloom wrote:
> I know I OK'd looking at the versal, but on second thought, I'd rather
> leave the xilinx-versal/tstqemu.yml alone until the BSP is finished,
> so revert that part of your patch. Sorry about that.

Agreed, please leave the Versal as is.

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


Re: [PATCH rtems] arm/xilinx: Fix zynq-uart interrupt receive

2021-08-19 Thread Chris Johns
On 19/8/21 10:45 pm, Kinsey Moore wrote:
> On 8/18/2021 18:02, Chris Johns wrote:
>> On 19/8/21 5:49 am, Kinsey Moore wrote:
>>> On 8/18/2021 13:20, Chris Johns wrote:
>>>> On 19/8/21 3:41 am, Kinsey Moore wrote:
>>>>> This is functional on the ZynqMP board I currently have setup for testing
>>>>> and on
>>>>> ZynqMP QEMU except for the data corruption/loss caused by the removal of 
>>>>> the
>>>>> post-baud-set null write.
>>>> Thanks for the testing.
>>>>
>>>> I am not sure if you are saying both the ZyncMP hardware and qemu need the
>>>> write
>>>> or just qemu. The write may work but it does not make sense because at some
>>>> point the software will print a character and achive the same thing?
>>> QEMU works fine either way. The ZynqMP hardware is what has issues without 
>>> the
>>> null write.
>> OK
>>
>>> The problem is that it's picky about which characters will actually fix the 
>>> data
>>> loss/corruption.
>>> I've seen that both space and null will do it while letters and numbers 
>>> won't.
>>> Null was chosen specifically because it's not printable.
>> It shows up on Zynq consoles I have running a a junk character.
>>
>>> Without this null print, I see garbage
>>> output and it eventually starts printing text correctly.
>> How many characters? It smells like a clock is hunting. I suggest you ask 
>> Joel
>> to raise this with Xilinx to see if there is any known errata.
> The number varies depending on the text being output. It continues spewing bad
> characters until
> it encounters a character that resets whatever internal mechanism is causing
> this. I'll see if we can get some information from Xilinx.

Interesting. A nul character means a single level on the line and that implies a
signal filtering issue, ie a low freq signal.

>> Another suggestion is downloading the Xilinx SDK and looking over the bare 
>> metal
>> console support to see what they do there.
> The sticking point here seems to be baud rate manipulation and the TX/RX reset
> that must occur.
> The bare metal driver has code for this, but it is never called as far as I 
> can
> tell. I still haven't found
> the startup sequence that initializes the UART, but I'll take another look.

Ok and thanks.

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


Re: [PATCH rtems] arm/xilinx: Fix zynq-uart interrupt receive

2021-08-18 Thread Chris Johns
On 19/8/21 5:49 am, Kinsey Moore wrote:
> On 8/18/2021 13:20, Chris Johns wrote:
>> On 19/8/21 3:41 am, Kinsey Moore wrote:
>>> This is functional on the ZynqMP board I currently have setup for testing 
>>> and on
>>> ZynqMP QEMU except for the data corruption/loss caused by the removal of the
>>> post-baud-set null write.
>> Thanks for the testing.
>>
>> I am not sure if you are saying both the ZyncMP hardware and qemu need the 
>> write
>> or just qemu. The write may work but it does not make sense because at some
>> point the software will print a character and achive the same thing?
> QEMU works fine either way. The ZynqMP hardware is what has issues without the
> null write.

OK

> The problem is that it's picky about which characters will actually fix the 
> data
> loss/corruption.
> I've seen that both space and null will do it while letters and numbers won't.
> Null was chosen specifically because it's not printable. 

It shows up on Zynq consoles I have running a a junk character.

> Without this null print, I see garbage
> output and it eventually starts printing text correctly.

How many characters? It smells like a clock is hunting. I suggest you ask Joel
to raise this with Xilinx to see if there is any known errata.

Another suggestion is downloading the Xilinx SDK and looking over the bare metal
console support to see what they do there.

The Versal VCK190 eval board has a ZynqMP in the corner and it runs a standard
FSBL etc and I do not see any dirty characters on its UART console when it
reboots. We need to be sure this is not specific to the board you are using.

So I am sorry, I am not yet convinced we the issue is in the ZynqMP.

>>> One nit below.
>>>>> +    while (ctx->tx_queued < 32 && len > 0) {
>>> Is 32 the size of the TX FIFO? It would be nice to see that as a sizeof() 
>>> or a
>>> #define for better context.
>> The Versal is 32 bytes and the Zynq is 64 bytes. The Versal is based on the 
>> Arm
>> IP r1p5-00rel1 and the Zynq and ZynqMP is based on Cadence IP. We either 
>> accept
>> the lesser size on the Zynq and ZyncMP or the zynq_uart struct will need
>> specialised info from each variant of device sharing this code.
> I'm fine using 32 bytes across the board(s).
>> It looks like the UARTs are not fully compatible. The Versal's FIFO trigger
>> level reg is:
>>
>> https://www.xilinx.com/html_docs/registers/am012/am012-versal-register-reference.html#uart___fifo_level.html
>>
>>
>> so it needs the timeout. I will add back the timeout support and see what
>> happens on the Zynq vs the Versal.
> 
> Sounds good.

I was mistaken, the Versal does not share the same driver so the change is
needed. Gedare has added a new serial driver for the Versal.

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

Re: [PATCH rtems] arm/xilinx: Fix zynq-uart interrupt receive

2021-08-18 Thread Chris Johns
On 19/8/21 3:41 am, Kinsey Moore wrote:
> This is functional on the ZynqMP board I currently have setup for testing and 
> on
> ZynqMP QEMU except for the data corruption/loss caused by the removal of the
> post-baud-set null write.

Thanks for the testing.

I am not sure if you are saying both the ZyncMP hardware and qemu need the write
or just qemu. The write may work but it does not make sense because at some
point the software will print a character and achive the same thing?

> Unrelated to this patch, I just realized that zynq_uart_set_attributes needs a
> call to zynq_uart_reset_tx_flush before adjusting any registers to avoid data
> loss there.

Ah OK, I will add this.

> One nit below.
> >> +    while (ctx->tx_queued < 32 && len > 0) {
> Is 32 the size of the TX FIFO? It would be nice to see that as a sizeof() or a
> #define for better context.

The Versal is 32 bytes and the Zynq is 64 bytes. The Versal is based on the Arm
IP r1p5-00rel1 and the Zynq and ZynqMP is based on Cadence IP. We either accept
the lesser size on the Zynq and ZyncMP or the zynq_uart struct will need
specialised info from each variant of device sharing this code.

It looks like the UARTs are not fully compatible. The Versal's FIFO trigger
level reg is:

https://www.xilinx.com/html_docs/registers/am012/am012-versal-register-reference.html#uart___fifo_level.html

so it needs the timeout. I will add back the timeout support and see what
happens on the Zynq vs the Versal.

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

Re: [PATCH rtems] arm/xilinx: Fix zynq-uart interrupt receive

2021-08-18 Thread Chris Johns
> On 18 Aug 2021, at 4:41 pm, jan.som...@dlr.de wrote:
> 
> Is this patch also something to backport to RTEMS5 or is this problem only 
> specific to the current version?

I have not considered doing this as 5 works on the Zynq. If termios is the same 
we may look at this. 

And Versal is not handling bursts of data so I am looking into this. 

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

Re: [PATCH rtems] arm/xilinx: Fix zynq-uart interrupt receive

2021-08-17 Thread Chris Johns
On 17/8/21 11:21 pm, Joel Sherrill wrote:
> I'm only asking where and how did this get tested? BSPs? Qemu/HW? Input?

I listed the testing in the covering email ...

https://lists.rtems.org/pipermail/devel/2021-August/068938.html

> Long overdue. I just hope it stays fixed.

I agree.

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


Re: Xilinx Zynq console rx not working

2021-08-16 Thread Chris Johns
On 17/8/21 8:59 am, Chris Johns wrote:
> On 16/8/21 11:03 pm, Kinsey Moore wrote:
>> On 8/16/2021 04:45, Chris Johns wrote:
>>> On 16/8/21 6:38 pm, Chris Johns wrote:
>>>> I have taken a closer look at the driver. I am receiving RX interrupts and 
>>>> the
>>>> characters are being queued however the receive FIFO trigger interrupt is 
>>>> only
>>>> raised when the FIFO reaches the set threshold of half the FIFO size. I 
>>>> suspect
>>>> there is an assumption the RX timeout will fire but it is not.
>>>>
>>> Doing this is questionable 
>>>
>>> https://git.rtems.org/rtems/tree/bsps/shared/dev/serial/zynq-uart.c#n222
>>>
>>> You cannot send a character when touching the attributes. Where is this 
>>> hardware
>>> bug documented by Xilinx?
>> The attributes are done being touched and the TX/RX enable flags are set 
>> again
>> before sending the character. I was seeing the same behavior on Zynq QEMU 
>> even
>> with this code removed.
>>
>> I couldn't find the hardware bug documented anywhere, but out of the 3 ZynqMP
>> boards I have one requires this consistently.
> 
> I suggest we use chip maker errata when documenting hardware bugs as it could
> turn out to be a bug in a our code.
> 
> This smells to me like a set up problem.
> 
>>>
>>> My application does this ...
>>>
>>>  if (tcgetattr(fileno(stdout), ) < 0)
>>>  error_message();
>>>  cfsetispeed (, B115200);
>>>  cfsetospeed (, B115200);
>>>  if (tcsetattr (fileno(stdout), TCSADRAIN, ) < 0)
>>>  error_message();
>>>  if (tcgetattr(fileno(stdin), ) < 0)
>>>  error_message();
>>>  cfsetispeed (, B115200);
>>>  cfsetospeed (, B115200);
>>>  if (tcsetattr (fileno(stdin), TCSADRAIN, ) < 0)
>>>  error_message();
>>>
>>> and this kills the receive interrupts.
>>
>> Does removing the null character kick restore functionality for you in this 
>> case?
> 
> No it did not.

I have read the Zynq-7000 TRM and the Versal ACAP TRM and there is a difference
in the wording around the RX timeout. It seems the RX timeout has evolved in the
Versal to a timer that can interrupt if data has entered the FIFO then no more
within the timeout period and the FIFO trigger level has not been reached. The
timer on the Zynq does not do this. It is something that can handle inter-gap
checks for those serial protocols that require this and there a few of them.
They delimit frames using time domain signalling. To make it work on a zynq-7000
I think you need to receive the first character (rtrig=1) then in the interrupt
handler raise the FIFO trigger level and prime the timer to achieve the Versal
timer functionality. I cannot be bothered doing this.

All we have is a console and that is a pretty basic use case for a UART. As a
result I have lowered the RX trigger level to 1 and disabled the RX timer. I
have also added support to send a FIFO full of data.

The solution is not as optimal as it could be with the Versal UART. If this
becomes a problem and someone wants a lower interrupt count we can look
specialising the driver based on the variant of the UART or implement the timer
protocol I outlined above.

I will create a patch and test on qemu and the Versal then post.

Chris

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

Re: Xilinx Zynq console rx not working

2021-08-16 Thread Chris Johns
On 16/8/21 11:03 pm, Kinsey Moore wrote:
> On 8/16/2021 04:45, Chris Johns wrote:
>> On 16/8/21 6:38 pm, Chris Johns wrote:
>>> I have taken a closer look at the driver. I am receiving RX interrupts and 
>>> the
>>> characters are being queued however the receive FIFO trigger interrupt is 
>>> only
>>> raised when the FIFO reaches the set threshold of half the FIFO size. I 
>>> suspect
>>> there is an assumption the RX timeout will fire but it is not.
>>>
>> Doing this is questionable 
>>
>> https://git.rtems.org/rtems/tree/bsps/shared/dev/serial/zynq-uart.c#n222
>>
>> You cannot send a character when touching the attributes. Where is this 
>> hardware
>> bug documented by Xilinx?
> The attributes are done being touched and the TX/RX enable flags are set again
> before sending the character. I was seeing the same behavior on Zynq QEMU even
> with this code removed.
> 
> I couldn't find the hardware bug documented anywhere, but out of the 3 ZynqMP
> boards I have one requires this consistently.

I suggest we use chip maker errata when documenting hardware bugs as it could
turn out to be a bug in a our code.

This smells to me like a set up problem.

>>
>> My application does this ...
>>
>>  if (tcgetattr(fileno(stdout), ) < 0)
>>  error_message();
>>  cfsetispeed (, B115200);
>>  cfsetospeed (, B115200);
>>  if (tcsetattr (fileno(stdout), TCSADRAIN, ) < 0)
>>  error_message();
>>  if (tcgetattr(fileno(stdin), ) < 0)
>>  error_message();
>>  cfsetispeed (, B115200);
>>  cfsetospeed (, B115200);
>>  if (tcsetattr (fileno(stdin), TCSADRAIN, ) < 0)
>>  error_message();
>>
>> and this kills the receive interrupts.
> 
> Does removing the null character kick restore functionality for you in this 
> case?

No it did not.

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

Re: RTEMS Tools on 5 release can't be build with modern LLVM

2021-08-16 Thread Chris Johns
Looks good and thank you for sorting this out.

Chris

On 17/8/21 1:17 am, Christian MAUDERER wrote:
> Am 16.08.21 um 17:12 schrieb Joel Sherrill:
>> On Mon, Aug 16, 2021 at 10:08 AM Christian MAUDERER
>>  wrote:
>>>
>>> Hello Joel,
>>>
>>> Am 16.08.21 um 17:03 schrieb Joel Sherrill:
 On Mon, Aug 16, 2021 at 9:49 AM Christian MAUDERER
  wrote:
>
> Hello,
>
> I noted that I can't build the RTEMS 5 toolchain if I have a modern LLVM
> installed using the RTEMS source builder. Therefore I would like to
> backport the patch 37ad446d9dce3 ([1]) to the 5 branch. Is that OK? If
> someone acknowledges it, I would do the following steps without further
> review (but of course with testing):
>
> 1. Create a ticket for the problem.
> 2. In rtems-tools: Cherry pick the patch 37ad446d9dce3 to the 5 branch.
> 3. In rtems-source-builder: Backport the patch 91932d0a0 [2] to the 5
> branch (or rather re-create it because it will be a different commit and
> a different hash).

 I'm ok with this. I remember doing a patch for the opposite direction.
 I couldn't
 build on an older gcc because it assumed a different language version as
 default. gcc 4.8 as on CentOS 7 assumes C++03. Newer versions assume C++11
 as the base. Similarly newer gcc's seem to default to C11.
>>>
>>> Thanks. I'll wait a bit more before adding the patches in case someone
>>> else want's to object.
>>
>>
>> Good plan.
>>

 I posted a few weeks ago that users should really be careful to always use
 -std and consciously pick what they want.
>>>
>>> In this case, the version of the standard depends quite a lot on the
>>> LLVM headers. LLVM seems to love using new features.
>>
>> Then this is probably not the first time we will face this.
> 
> Definitively not. It is only a backport of a patch so I had that problem 
> already
> on another system. I only missed to add it to the 5 branch back then.
> 
> Best regards
> 
> Christian
> 
>>
>> --joel
>>
>>> Best regards
>>>
>>> Christian
>>>

 --joel

>
> Best regards
>
> Christian
>
> [1]
> https://git.rtems.org/rtems-tools/commit/?id=37ad446d9dce3438d6d32e1caf56d3fdccdd2ad0
>
>
> [2]
> https://git.rtems.org/rtems-source-builder/commit/?id=91932d0a0c86400366f9c75a62123b6488ff458a
>
> -- 
> 
> embedded brains GmbH
> Herr Christian MAUDERER
> Dornierstr. 4
> 82178 Puchheim
> Germany
> email: christian.maude...@embedded-brains.de
> phone: +49-89-18 94 741 - 18
> fax:   +49-89-18 94 741 - 08
>
> Registergericht: Amtsgericht München
> Registernummer: HRB 157899
> Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
> Unsere Datenschutzerklärung finden Sie hier:
> https://embedded-brains.de/datenschutzerklaerung/
> ___
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
>>>
>>> -- 
>>> 
>>> embedded brains GmbH
>>> Herr Christian MAUDERER
>>> Dornierstr. 4
>>> 82178 Puchheim
>>> Germany
>>> email: christian.maude...@embedded-brains.de
>>> phone: +49-89-18 94 741 - 18
>>> fax:   +49-89-18 94 741 - 08
>>>
>>> Registergericht: Amtsgericht München
>>> Registernummer: HRB 157899
>>> Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
>>> Unsere Datenschutzerklärung finden Sie hier:
>>> https://embedded-brains.de/datenschutzerklaerung/
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: Xilinx Zynq console rx not working

2021-08-16 Thread Chris Johns
On 16/8/21 6:38 pm, Chris Johns wrote:
> On 14/8/21 12:40 am, Kinsey Moore wrote:
>> On 8/13/2021 03:05, Chris Johns wrote:
>>> Hi,
>>>
>>> I am bring up a major app on RTEMS 6 and LibBSD on custom Zynq hardware and 
>>> I am
>>> not getting any receive interrupts from the console UART. The same hardware
>>> works with RTEMS 4.11 and RTEMS 5.
>>>
>>> I also do not see any console input with qemu and I thought this was 
>>> something
>>> in qemu but I am now not so sure.
>>>
>>> Anyone else seeing this issue?
>>>
>>> There has been a lot of playing with the Zynq UART driver recently.
>>
>> I just went through and backed out all those changes you mentioned back to 
>> when
>> the polled-only driver was split out and I still don't have functionality on
>> Zynq under QEMU. Testing using fileio.exe, things are partially functional 
>> until
>> I revert the change that enabled set_attributes. After the commit that 
>> enables
>> set_attributes for the zynq UART, fileio produces output waiting for user 
>> input
>> and will accept the first character to open the menu, but accepts no further
>> input. Before that commit, the fileio test doesn't produce the expected 
>> output.
>>
>> A full bisect would probably provide more accurate results, but this is what 
>> I
>> could do quickly.
>>
>> Note that ZynqMP fileio works on QEMU with current code and is fully
>> operational. It is almost certainly the exact same emulated peripheral in 
>> QEMU
>> and the hardware is supposed to be very close if not identical as well.
> 
> I have taken a closer look at the driver. I am receiving RX interrupts and the
> characters are being queued however the receive FIFO trigger interrupt is only
> raised when the FIFO reaches the set threshold of half the FIFO size. I 
> suspect
> there is an assumption the RX timeout will fire but it is not.
> 

Doing this is questionable 

https://git.rtems.org/rtems/tree/bsps/shared/dev/serial/zynq-uart.c#n222

You cannot send a character when touching the attributes. Where is this hardware
bug documented by Xilinx?

My application does this ...

if (tcgetattr(fileno(stdout), ) < 0)
error_message();
cfsetispeed (, B115200);
cfsetospeed (, B115200);
if (tcsetattr (fileno(stdout), TCSADRAIN, ) < 0)
error_message();
if (tcgetattr(fileno(stdin), ) < 0)
error_message();
cfsetispeed (, B115200);
cfsetospeed (, B115200);
if (tcsetattr (fileno(stdin), TCSADRAIN, ) < 0)
error_message();

and this kills the receive interrupts.

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


Re: Xilinx Zynq console rx not working

2021-08-16 Thread Chris Johns
On 14/8/21 12:40 am, Kinsey Moore wrote:
> On 8/13/2021 03:05, Chris Johns wrote:
>> Hi,
>>
>> I am bring up a major app on RTEMS 6 and LibBSD on custom Zynq hardware and 
>> I am
>> not getting any receive interrupts from the console UART. The same hardware
>> works with RTEMS 4.11 and RTEMS 5.
>>
>> I also do not see any console input with qemu and I thought this was 
>> something
>> in qemu but I am now not so sure.
>>
>> Anyone else seeing this issue?
>>
>> There has been a lot of playing with the Zynq UART driver recently.
> 
> I just went through and backed out all those changes you mentioned back to 
> when
> the polled-only driver was split out and I still don't have functionality on
> Zynq under QEMU. Testing using fileio.exe, things are partially functional 
> until
> I revert the change that enabled set_attributes. After the commit that enables
> set_attributes for the zynq UART, fileio produces output waiting for user 
> input
> and will accept the first character to open the menu, but accepts no further
> input. Before that commit, the fileio test doesn't produce the expected 
> output.
> 
> A full bisect would probably provide more accurate results, but this is what I
> could do quickly.
> 
> Note that ZynqMP fileio works on QEMU with current code and is fully
> operational. It is almost certainly the exact same emulated peripheral in QEMU
> and the hardware is supposed to be very close if not identical as well.

I have taken a closer look at the driver. I am receiving RX interrupts and the
characters are being queued however the receive FIFO trigger interrupt is only
raised when the FIFO reaches the set threshold of half the FIFO size. I suspect
there is an assumption the RX timeout will fire but it is not.

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


Xilinx Zynq console rx not working

2021-08-13 Thread Chris Johns
Hi,

I am bring up a major app on RTEMS 6 and LibBSD on custom Zynq hardware and I am
not getting any receive interrupts from the console UART. The same hardware
works with RTEMS 4.11 and RTEMS 5.

I also do not see any console input with qemu and I thought this was something
in qemu but I am now not so sure.

Anyone else seeing this issue?

There has been a lot of playing with the Zynq UART driver recently.

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


Re: [PATCH rtems-libbsd] ppp: Fix transmitting data

2021-08-13 Thread Chris Johns
On 13/8/21 5:22 pm, Christian MAUDERER wrote:
> Hello Chris,
> 
> Am 13.08.21 um 04:46 schrieb Chris Johns:
>> On 12/8/21 9:42 pm, Christian Mauderer wrote:
>>> The pppstart expected that a driver write would somehow magically
>>> process all data passed to the write function. Because ppp disables all
>>> buffering that originally has been in termios, that assumption is not
>>> true for all but polled drivers.
>>>
>>> With this patch, the pppstart now gets and processes the feedback that
>>> is returned from the driver via rtems_termios_dequeue_characters.
>>>
>>> Fixes #4493
>>> ---
>>>   rtemsbsd/sys/net/if_ppp.c    | 11 ++-
>>>   rtemsbsd/sys/net/if_pppvar.h |  1 +
>>>   rtemsbsd/sys/net/ppp_tty.c   | 32 
>>>   3 files changed, 35 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/rtemsbsd/sys/net/if_ppp.c b/rtemsbsd/sys/net/if_ppp.c
>>> index 709f13e04..e134dc760 100644
>>> --- a/rtemsbsd/sys/net/if_ppp.c
>>> +++ b/rtemsbsd/sys/net/if_ppp.c
>>> @@ -313,11 +313,12 @@ static rtems_task ppp_txdaemon(rtems_task_argument 
>>> arg)
>>>     frag=0;
>>>       /* initialize output values */
>>> -  sc->sc_outfcs    = PPP_INITFCS;
>>> -  sc->sc_outbuf    = (u_char *)0;
>>> -  sc->sc_outlen    = (short   )0;
>>> -  sc->sc_outoff    = (short   )0;
>>> -  sc->sc_outfcslen = (short   )0;
>>> +  sc->sc_outfcs    = PPP_INITFCS;
>>> +  sc->sc_outbuf    = (u_char *)0;
>>> +  sc->sc_outlen    = (short   )0;
>>> +  sc->sc_outoff    = (short   )0;
>>> +  sc->sc_outoff_update = false;
>>> +  sc->sc_outfcslen = (short   )0;
>>
>> This is not the FreeBSD clang-format. Once my changes are pushed you can use 
>> the
>> style to reformat this code.
> 
> I touched only a small part of the file and therefore I adapted to the style 
> in
> that file (also it's a bit messy).

Ah ok and yes we are sort of all over the place.

> If we want the code in the ppp parts
> reformatted, I would suggest an extra patch that applies the coding style to 
> the
> complete file. What do you think?

I have only just learnt about this requirement and I think it is a good one. I
am no expert on what is the FreeBSd format so I can only highlight what I have
observed after reformatting my code.

I think it would be good to have a single pass to clean this up but it is
painful because it breaks blame. So I do not know and happy to see what everyone
else thinks.

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

Re: [PATCH rtems-libbsd] ppp: Fix transmitting data

2021-08-12 Thread Chris Johns
On 12/8/21 9:42 pm, Christian Mauderer wrote:
> The pppstart expected that a driver write would somehow magically
> process all data passed to the write function. Because ppp disables all
> buffering that originally has been in termios, that assumption is not
> true for all but polled drivers.
> 
> With this patch, the pppstart now gets and processes the feedback that
> is returned from the driver via rtems_termios_dequeue_characters.
> 
> Fixes #4493
> ---
>  rtemsbsd/sys/net/if_ppp.c| 11 ++-
>  rtemsbsd/sys/net/if_pppvar.h |  1 +
>  rtemsbsd/sys/net/ppp_tty.c   | 32 
>  3 files changed, 35 insertions(+), 9 deletions(-)
> 
> diff --git a/rtemsbsd/sys/net/if_ppp.c b/rtemsbsd/sys/net/if_ppp.c
> index 709f13e04..e134dc760 100644
> --- a/rtemsbsd/sys/net/if_ppp.c
> +++ b/rtemsbsd/sys/net/if_ppp.c
> @@ -313,11 +313,12 @@ static rtems_task ppp_txdaemon(rtems_task_argument arg)
>frag=0;
>  
>/* initialize output values */
> -  sc->sc_outfcs= PPP_INITFCS;
> -  sc->sc_outbuf= (u_char *)0;
> -  sc->sc_outlen= (short   )0;
> -  sc->sc_outoff= (short   )0;
> -  sc->sc_outfcslen = (short   )0;
> +  sc->sc_outfcs= PPP_INITFCS;
> +  sc->sc_outbuf= (u_char *)0;
> +  sc->sc_outlen= (short   )0;
> +  sc->sc_outoff= (short   )0;
> +  sc->sc_outoff_update = false;
> +  sc->sc_outfcslen = (short   )0;

This is not the FreeBSD clang-format. Once my changes are pushed you can use the
style to reformat this code.

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


Re: [PATCH v2] score: Replace priority prepend it with an enum

2021-08-12 Thread Chris Johns
+1

On 13/8/21 3:43 am, Gedare Bloom wrote:
> looks good to me.
> 
> On Thu, Aug 12, 2021 at 8:26 AM Sebastian Huber
>  wrote:
>>
>> Use the new Priority_Group_order enum instead of a boolean to indicated if a
>> priority should be inserted as the first or last node into its priority 
>> group.
>> This makes the code more expressive.  It is also a bit more efficient since a
>> branch in _Scheduler_Node_set_priority() is avoided and a simple bitwise or
>> operation can be used.
>> ---
>> v2:
>>
>> Rename Priority_Flags in Priority_Group_order.
>>
>>  cpukit/include/rtems/posix/muteximpl.h|  2 +-
>>  cpukit/include/rtems/score/coremuteximpl.h|  2 +-
>>  cpukit/include/rtems/score/priorityimpl.h | 44 ++-
>>  cpukit/include/rtems/score/schedulerimpl.h| 12 -
>>  .../include/rtems/score/schedulernodeimpl.h   | 30 ++---
>>  cpukit/include/rtems/score/threadimpl.h   | 20 +
>>  cpukit/posix/src/pthreadsetschedparam.c   |  2 +-
>>  cpukit/posix/src/pthreadsetschedprio.c|  2 +-
>>  cpukit/rtems/src/tasksetpriority.c|  2 +-
>>  cpukit/score/src/scheduleredfreleasejob.c |  2 +-
>>  cpukit/score/src/threadchangepriority.c   | 39 +---
>>  cpukit/score/src/threadqops.c | 18 
>>  cpukit/score/src/threadrestart.c  |  4 +-
>>  testsuites/smptests/smpscheduler03/test.c | 33 +++---
>>  14 files changed, 126 insertions(+), 86 deletions(-)
>>
>> diff --git a/cpukit/include/rtems/posix/muteximpl.h 
>> b/cpukit/include/rtems/posix/muteximpl.h
>> index 435b43634d..5d20bc1ef6 100644
>> --- a/cpukit/include/rtems/posix/muteximpl.h
>> +++ b/cpukit/include/rtems/posix/muteximpl.h
>> @@ -273,7 +273,7 @@ RTEMS_INLINE_ROUTINE void _POSIX_Mutex_Set_priority(
>>owner,
>>_mutex->Priority_ceiling,
>>priority_ceiling,
>> -  false,
>> +  PRIORITY_GROUP_LAST,
>>queue_context
>>  );
>>  _Thread_Wait_release( owner, queue_context );
>> diff --git a/cpukit/include/rtems/score/coremuteximpl.h 
>> b/cpukit/include/rtems/score/coremuteximpl.h
>> index cbc1e720fb..426c4c5a95 100644
>> --- a/cpukit/include/rtems/score/coremuteximpl.h
>> +++ b/cpukit/include/rtems/score/coremuteximpl.h
>> @@ -375,7 +375,7 @@ RTEMS_INLINE_ROUTINE void 
>> _CORE_ceiling_mutex_Set_priority(
>>owner,
>>_mutex->Priority_ceiling,
>>priority_ceiling,
>> -  false,
>> +  PRIORITY_GROUP_LAST,
>>queue_context
>>  );
>>  _Thread_Wait_release( owner, queue_context );
>> diff --git a/cpukit/include/rtems/score/priorityimpl.h 
>> b/cpukit/include/rtems/score/priorityimpl.h
>> index 7a14ec97b8..2895a0c4a5 100644
>> --- a/cpukit/include/rtems/score/priorityimpl.h
>> +++ b/cpukit/include/rtems/score/priorityimpl.h
>> @@ -37,6 +37,29 @@ extern "C" {
>>   * @{
>>   */
>>
>> + /**
>> +  * @brief The priority group order determines if a priority node is 
>> inserted
>> +  *   as the first or last node into its priority group.
>> +  *
>> +  * The values of the enumerators matter.  The least significant bit of a
>> +  * ::Priority_Control value is not used for the actual priority of a node.
>> +  * During insertion the least significant bit is used to determine the
>> +  * ordering within a priority group based on the enumerator values.
>> +  */
>> +typedef enum {
>> +  /**
>> +   * @brief Priority group first option requests that the priority node is
>> +   *   inserted as the first node into its priority group.
>> +   */
>> +  PRIORITY_GROUP_FIRST = 0,
>> +
>> +  /**
>> +   * @brief Priority group last option requests that the priority node is
>> +   *   inserted as the last node into its priority group.
>> +   */
>> +  PRIORITY_GROUP_LAST = 1
>> +} Priority_Group_order;
>> +
>>  /**
>>   * @brief Initializes the priority actions empty.
>>   *
>> @@ -465,7 +488,7 @@ typedef void ( *Priority_Add_handler )(
>>
>>  typedef void ( *Priority_Change_handler )(
>>Priority_Aggregation *aggregation,
>> -  bool  prepend_it,
>> +  Priority_Group_order  group_order,
>>Priority_Actions *actions,
>>void *arg
>>  );
>> @@ -482,19 +505,19 @@ typedef void ( *Priority_Remove_handler )(
>>   * This method does nothing.
>>   *
>>   * @param aggregation Is ignored by the method.
>> - * @param prepend_it Is ignored by the method.
>> + * @param group_order Is ignored by the method.
>>   * @param actions Is ignored by the method.
>>   * @param arg Is ignored by the method.
>>   */
>>  RTEMS_INLINE_ROUTINE void _Priority_Change_nothing(
>>Priority_Aggregation *aggregation,
>> -  bool  prepend_it,
>> +  Priority_Group_order  group_order,
>>Priority_Actions *actions,
>>void *arg
>>  )
>>  {
>>(void) aggregation;
>> -  (void) prepend_it;
>> +  (void) group_order;
>>(void) actions;
>>(void) arg;
>>  }
>> @@ -547,7 +570,7 @@ RTEMS_INLINE_ROUTINE void 

Re: [PATCH 0/3] Add ostream_guard patches

2021-08-11 Thread Chris Johns
OK to push.

Thanks
Chris

On 12/8/21 7:09 am, Ryan Long wrote:
> Hi,
> 
> In this series of patches, I just added ostream_guards where there were
> "Restore ostream format" errors. This won't make the error go away, but
> we're going to start ignoring those errors and just placing
> ostream_guards as was done here.
> 
> Thanks,
> Ryan
> 
> Ryan Long (3):
>   GcovData.cc: Add ostream_guard
>   GcovFunctionData.cc: Add ostream_guard
>   ReportsText.cc: Add ostream_guard
> 
>  tester/covoar/GcovData.cc | 3 +++
>  tester/covoar/GcovFunctionData.cc | 3 +++
>  tester/covoar/ReportsText.cc  | 2 ++
>  3 files changed, 8 insertions(+)
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: [PATCH 1/2] score: Replace priority prepend it with flags

2021-08-11 Thread Chris Johns
On 11/8/21 3:21 pm, Sebastian Huber wrote:
> On 10/08/2021 16:50, Sebastian Huber wrote:
>> On 10/08/2021 16:46, Gedare Bloom wrote:
>>> This is a good cleanup. The naming seems a bit off to me, but it's all
>>> internal so we can always adjust it later. (I think it should be
>>> singular "Priority_Flag", but really it's not just a flag, it's
>>> something like the "Priority_Discipline" -- I can't think what is the
>>> right word however for how you decide to break ties.) So you can just
>>> leave it be for now and ignore my rambling. :)
>>
>> Thanks for the review. Maybe we have more flags in the future. If not we can
>> still rename it after some time.
> 
> Actually I am not sure if we really need more flags. What about:

This is nicer.

> /**
>  * @brief The priority group flags indicate if the priority should be appended
>  *   or prepended to its priority group.
>  */
> typedef enum {
>   /**
>* @brief The priority group prepend flag indicates that the priority should
>*   be prepended to its priority group.

Could this be more direct 

* @brief The Priority group prepend option will prepended the priority
to the priority group.

?

I am not sure `indicates` is the write word.

>    */
>   PRIORITY_GROUP_PREPEND = 0,
> 
>   /**
>    * @brief The priority group append flag indicates that the priority should
>    *   be appended to its priority group.
>    */
>   PRIORITY_GROUP_APPEND = 1
> } Priority_Group;

I am not a fan of adding things like _FLAG because it only describes some of
what the option is so why just have that bit when there could be so much more?
;) For example _FLAG could also be _FLAG_ENUM or _FLAG_DEFINE etc.

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

Re: [PATCH rtems-libbsd v3 1/3] rtemsbsd/bus: Add PCI support to the nexus bus

2021-08-09 Thread Chris Johns
On 10/8/21 1:43 am, Gedare Bloom wrote:
> On Sun, Aug 8, 2021 at 7:22 PM Chris Johns  wrote:
>>
>> - Add PCI IO region support
>>
>> - Add support map buffers to PCI address space
>>
>> - Add BSP conditional IO space support. Some PC implementations
>>   have PCI IO space mapped differently to memory space and this needs
>>   to be reflected in the busspace.
>>
>> - Include bsp.h to pick per BSP configuration.
>>
>> Closes #4245
>> ---
>>  rtemsbsd/include/machine/bus.h| 370 ++
>>  rtemsbsd/rtems/rtems-kernel-bus-dma.c |   5 +-
>>  rtemsbsd/rtems/rtems-kernel-nexus.c   |  52 +++-
>>  3 files changed, 312 insertions(+), 115 deletions(-)
>>
>> diff --git a/rtemsbsd/include/machine/bus.h b/rtemsbsd/include/machine/bus.h
>> index 2f0e7ad6..999f5d45 100644
>> --- a/rtemsbsd/include/machine/bus.h
>> +++ b/rtemsbsd/include/machine/bus.h
>> @@ -6,9 +6,13 @@
>>   * @brief TODO.
>>   *
>>   * File origin from FreeBSD 'sys/amd64/include/bus.h'.
>> + *
>> + * Conditionally supports PCI IO regions (IO Ports).
>>   */
>>
>>  /*-
>> + * Copyright (c) 2021 Chris Johns.  All rights reserved.
>> + *
>>   * Copyright (c) 2009, 2015 embedded brains GmbH.  All rights reserved.
>>   *
>>   *  embedded brains GmbH
>> @@ -25,7 +29,7 @@
>>   * 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.
>> @@ -34,7 +38,7 @@
>>   *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.
>> @@ -123,9 +127,46 @@
>>  #endif
>>
>>  #ifdef __i386__
>> -  #error "your include paths are wrong"
>> +  #error "x86 has its own bus.h; check your include paths are correct"
>>  #endif
>>
>> +#include 
>> +
>> +/*
>> + * BSP PCI Support
>> + *
>> + * The RTEMS Nexus bus support can optionaly support PC PCI spaces that
> optionally
> 
>> + * mapped to BSP speciic address spaces. Add the following define to
> specific
> 
>> + * the BSP header file to enable this support:
>> + *
>> + *  #define BSP_HAS_PC_PCI
> 
> Is there an rtems.git patch to add this to some BSP?

Yes that is to come once we have this sorted or it just keeps changing.

> 
> I might suggest BSP_HAS_PCI_IO_PORTS
> 

I think using PC PCI is better because the PCI originally came from the PC and
it defines the IO space and the memory space so the define means the bus space
will support the IO _and_ the memory spaces. The default is what exists and that
is a flat 1:1 mapping.

>> + *
>> + * If enabled a BSP must the following IO region calls:
> must support?
> 
>> + *
>> + * inb  : read 8 bits
>> + * outb : write 8 bits
>> + * inw  : read 16 bits
>> + * outw : write 16 bits
>> + * inl  : read 32 bits
>> + * outl : write 32 bits
>> + *
>> + * The BSP needs to provide the DRAM address space offset
>> + * PCI_DRAM_OFFSET. This is the base address of the DRAM as seen by a
>> + * PCI master.
>> + *
>> + * i386 BSPs have a special bus.h file and do not use this file.
>> + */
>> +
>> +#ifdef BSP_HAS_PC_PCI
>> +
>> +/*
>> + * Values for the bus space tag, not to be used directly by MI code.
>> + */
>> +#defineBSP_BUS_SPACE_IO0   /* space is i/o space */
>> +#defineBSP_BUS_SPACE_MEM   1   /* space is mem space */
>> +
>> +#endif /* BSP_HAS_PC_PCI */
>> +
>>  /*
>>   * Bus address alignment.
>>   */
>> @@ -144,6 +185,7 @@
>>  /*
>>   * Bus access.
>>   */
>> +#define BUS_SPACE_INVALID_DATA (~0)
> Please use 0U here. I get the undefined behavior (UB) willies when I
> see bit-twiddling on signed integer types.

Sure but I was just keeping the code as close to FB as I could ...

https://github.com/freebsd/freebsd

Re: [PATCH v2] rtems-utils: Change data type definition

2021-08-09 Thread Chris Johns
On 9/8/21 10:48 pm, Ryan Long wrote:
> Without the global namespace prefix, it results in the following error
> 
> ../linkers/rtems-exeinfo.cpp: In member function ‘void 
> rld::exeinfo::image::output_compilation_unit(bool, bool)’:
> ../linkers/rtems-exeinfo.cpp:370:14: error: ‘rld::rtems::utils’ has not been 
> declared
>rtems::utils::ostream_guard old_state( std::cout );
>   ^
> ../linkers/rtems-exeinfo.cpp:370:35: error: expected ‘;’ before ‘old_state’
>rtems::utils::ostream_guard old_state( std::cout );
> 
> Alex and I determined that the compiler is thinking that ostream_guard is 
> defined in the rld::rtems namespace. This was the only way we figured out how 
> to get it to compile without the typedef at the top of the file.

Ah OK. This makes sense and I think it is better to have the global prefix there
so the full path is known.

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

Re: [PATCH rtems-libbsd v3 2/3] rtemsbsd: Add interface support routines

2021-08-09 Thread Chris Johns
On 10/8/21 1:50 am, Gedare Bloom wrote:
> On Sun, Aug 8, 2021 at 7:22 PM Chris Johns  wrote:
>>
>> - Add the ability to check if an interface is up
>> ---
>>  libbsd.py  |   1 +
>>  rtemsbsd/include/rtems/bsd/iface.h |  62 
>>  rtemsbsd/rtems/rtems-bsd-iface.c   | 146 +
>>  3 files changed, 209 insertions(+)
>>  create mode 100644 rtemsbsd/include/rtems/bsd/iface.h
>>  create mode 100644 rtemsbsd/rtems/rtems-bsd-iface.c
>>
>> diff --git a/libbsd.py b/libbsd.py
>> index 09a1fbc4..6e09a07c 100644
>> --- a/libbsd.py
>> +++ b/libbsd.py
>> @@ -253,6 +253,7 @@ class rtems(builder.Module):
>>  'rtems/rtems-bsd-cxx.cc',
>>  'rtems/rtems-bsd-get-ethernet-addr.c',
>>  'rtems/rtems-bsd-ifconfig.c',
>> +'rtems/rtems-bsd-iface.c',
>>  'rtems/rtems-bsd-ifconfig-lo0.c',
>>  'rtems/rtems-bsd-init-dhcp.c',
>>  'rtems/rtems-bsd-rc-conf-net.c',
>> diff --git a/rtemsbsd/include/rtems/bsd/iface.h 
>> b/rtemsbsd/include/rtems/bsd/iface.h
>> new file mode 100644
>> index ..9e583b8a
>> --- /dev/null
>> +++ b/rtemsbsd/include/rtems/bsd/iface.h
>> @@ -0,0 +1,62 @@
>> +/**
>> + * @file
>> + *
>> + * @ingroup rtems_bsd_rtems
>> + *
>> + * @brief This file provide a high level interface to simple interface
>> + * support calls.
>> + */
>> +
>> +/*
>> + * Copyright (c) 2021. Chris Johns . All rights reserved.
>> + *
>> + * 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 AUTHOR 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 AUTHOR 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.
>> + */
>> +
>> +#ifndef _RTEMS_BSD_IFACE_H_
>> +#define _RTEMS_BSD_IFACE_H_
>> +
>> +#include 
>> +
>> +#include 
>> +
>> +#include 
>> +#include 
>> +#include 
>> +
>> +struct rtems_bsd_iface {
>> +   char name[IFNAMSIZ];
>> +   int unit;
>> +   struct in_addr address;
>> +   size_t hw_len;
>> +   uint8_t hw_address[16];
>> +   struct ifreq ifr;
>> +   int linkstate;
>> +};
>> +
>> +/*
>> + * These calls return 0 is successful and -1 and errno set on error.
>> + */
>> +int rtems_bsd_iface_get(const char *name, struct rtems_bsd_iface *iface);
>> +int rtems_bsd_iface_link_state(const char *name, bool *state);
>> +
>> +#endif
>> diff --git a/rtemsbsd/rtems/rtems-bsd-iface.c 
>> b/rtemsbsd/rtems/rtems-bsd-iface.c
>> new file mode 100644
>> index ..4cf25a9b
>> --- /dev/null
>> +++ b/rtemsbsd/rtems/rtems-bsd-iface.c
>> @@ -0,0 +1,146 @@
>> +/**
>> + * @file
>> + *
>> + * @ingroup rtems_bsd_rtems
>> + *
>> + * @brief Interface support routines
>> + */
>> +
>> +/*
>> + * Copyright (c) 2021. Chris Johns   All rights reserved.
>> + *
>> + * 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 bin

Re: [PATCH rtems-libbsd v2 0/5] Beagle BSP: USB Template Driver for CDC Ethernet

2021-08-09 Thread Chris Johns



On 9/8/21 5:14 pm, Chris Johns wrote:
> On 9/8/21 5:02 pm, Christian MAUDERER wrote:
>> Hello Chris,
>>
>> Am 09.08.21 um 02:34 schrieb Chris Johns:
>>> On 1/8/21 9:27 pm, Christian Mauderer wrote:
>>>> Hello Husni,
>>>>
>>>> just tested that and it works fine. With a simple curl I can reach about 
>>>> 8.5
>>>> MByte/s sending to the beagle and 19.5 MByte/s receiving from it.
>>>>
>>>> Please take a look at the points that Chris Johns asked. As soon as these 
>>>> are
>>>> addressed, I think the patches could be merged.
>>>
>>> Can these patches please wait until my patches are merged? I have not seen 
>>> any
>>> issues and given the size I am waiting a while.
>>>
>>> I ask because the symbol changes in the namespace ripple through the 150K 
>>> lines
>>> of patches I have posted for review and it is a pain to rebase.
>>>
>>> Chris
>>
>> About when do you plan to merge your patches?
> 
> My plan is this week. I have some testing planned.
> 
>> Although not entirely necessary, I think it would be great if we could merge 
>> the
>> patches before end of GSoC so that Husni can add to his final report that 
>> code
>> has been merged.
> 
> Yes it would be nice. I think the changes looks fine so there is issue with 
> them
> going in.

That should read "no issue"

Chris

> 
>> If that doesn't fit your time plan, I'll add them to my branch where I 
>> collect
>> to-be-merged patches so that I at least don't forget them.
> 
> Yeap. If I am slipping I will let you know.
> 
> And thank you, it is appreciated.
> 
> Chris
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: [PATCH rtems-libbsd v2 0/5] Beagle BSP: USB Template Driver for CDC Ethernet

2021-08-09 Thread Chris Johns
On 9/8/21 5:02 pm, Christian MAUDERER wrote:
> Hello Chris,
> 
> Am 09.08.21 um 02:34 schrieb Chris Johns:
>> On 1/8/21 9:27 pm, Christian Mauderer wrote:
>>> Hello Husni,
>>>
>>> just tested that and it works fine. With a simple curl I can reach about 8.5
>>> MByte/s sending to the beagle and 19.5 MByte/s receiving from it.
>>>
>>> Please take a look at the points that Chris Johns asked. As soon as these 
>>> are
>>> addressed, I think the patches could be merged.
>>
>> Can these patches please wait until my patches are merged? I have not seen 
>> any
>> issues and given the size I am waiting a while.
>>
>> I ask because the symbol changes in the namespace ripple through the 150K 
>> lines
>> of patches I have posted for review and it is a pain to rebase.
>>
>> Chris
> 
> About when do you plan to merge your patches?

My plan is this week. I have some testing planned.

> Although not entirely necessary, I think it would be great if we could merge 
> the
> patches before end of GSoC so that Husni can add to his final report that code
> has been merged.

Yes it would be nice. I think the changes looks fine so there is issue with them
going in.

> If that doesn't fit your time plan, I'll add them to my branch where I collect
> to-be-merged patches so that I at least don't forget them.

Yeap. If I am slipping I will let you know.

And thank you, it is appreciated.

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


[PATCH rtems-libbsd v3 3/3] testsuite: Wait for the link to be UP

2021-08-08 Thread Chris Johns
- Wait for a slow PHY to bring the link UP. If the IP address is
  static the test can start before the link is up and the test
  fails.

- Make 2 tests wait. Others will need to be added.
---
 .../include/rtems/bsd/test/default-init.h | 29 +++
 .../rtems/bsd/test/default-network-init.h | 28 ++
 testsuite/nfs01/test_main.c   |  1 +
 testsuite/ping01/test_main.c  |  1 +
 4 files changed, 59 insertions(+)

diff --git a/testsuite/include/rtems/bsd/test/default-init.h 
b/testsuite/include/rtems/bsd/test/default-init.h
index f8ea3acd..ea502f94 100644
--- a/testsuite/include/rtems/bsd/test/default-init.h
+++ b/testsuite/include/rtems/bsd/test/default-init.h
@@ -9,11 +9,31 @@
 #include 
 #include 
 #include 
+#include 
+
 #include 
 #include 
 #include 
 #include 
 
+static void default_wait_for_link_up( const char *name )
+{
+  size_t seconds = 0;
+  while ( true ) {
+bool link_active = false;
+assert(rtems_bsd_iface_link_state( name, _active ) == 0);
+if (link_active) {
+  return;
+}
+sleep( 1 );
+++seconds;
+if (seconds > 10) {
+  printf("error: %s: no active link\n", name);
+  assert(seconds < 10);
+}
+  }
+}
+
 static void default_set_self_prio( rtems_task_priority prio )
 {
   rtems_status_code sc;
@@ -68,6 +88,15 @@ rtems_task Init(
   sc = rtems_task_wake_after( 2 );
   assert(sc == RTEMS_SUCCESSFUL);
 
+#if defined(TEST_WAIT_FOR_LINK)
+  /*
+   * Per test option to wait for the network interface. If the address
+   * is static the PHY may take a while to connect and bring the
+   * interface online.
+   */
+  default_wait_for_link_up( TEST_WAIT_FOR_LINK );
+#endif
+
   test_main();
   /* should not return */
 
diff --git a/testsuite/include/rtems/bsd/test/default-network-init.h 
b/testsuite/include/rtems/bsd/test/default-network-init.h
index ce1fc015..ba995910 100644
--- a/testsuite/include/rtems/bsd/test/default-network-init.h
+++ b/testsuite/include/rtems/bsd/test/default-network-init.h
@@ -40,6 +40,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -175,6 +176,25 @@ default_network_on_exit(int exit_code, void *arg)
}
 }
 
+static void
+default_wait_for_link_up( const char *name )
+{
+   size_t seconds = 0;
+   while ( true ) {
+   bool link_active = false;
+   assert(rtems_bsd_iface_link_state( name, _active ) == 0);
+   if (link_active) {
+   return;
+   }
+   sleep( 1 );
+   ++seconds;
+   if (seconds > 10) {
+   printf("error: %s: no active link\n", name);
+   assert(seconds < 10);
+   }
+   }
+}
+
 static void
 Init(rtems_task_argument arg)
 {
@@ -238,6 +258,14 @@ Init(rtems_task_argument arg)
 #endif
default_network_dhcpcd();
 
+#if defined(TEST_WAIT_FOR_LINK)
+   /*
+* Per test option to wait for the network interface. If the address
+* is static the PHY may take a while to connect and bring the
+* interface online.
+*/
+   default_wait_for_link_up( TEST_WAIT_FOR_LINK );
+#endif
test_main();
 
assert(0);
diff --git a/testsuite/nfs01/test_main.c b/testsuite/nfs01/test_main.c
index 2312040a..170cd484 100644
--- a/testsuite/nfs01/test_main.c
+++ b/testsuite/nfs01/test_main.c
@@ -46,6 +46,7 @@
 #include 
 
 #define TEST_NAME "LIBBSD NFS 1"
+#define TEST_WAIT_FOR_LINK NET_CFG_INTERFACE_0
 #define TEST_STATE_USER_INPUT 1
 
 static void
diff --git a/testsuite/ping01/test_main.c b/testsuite/ping01/test_main.c
index 5702cee2..8b9a42ce 100644
--- a/testsuite/ping01/test_main.c
+++ b/testsuite/ping01/test_main.c
@@ -46,6 +46,7 @@
 #include 
 
 #define TEST_NAME "LIBBSD PING 1"
+#define TEST_WAIT_FOR_LINK NET_CFG_INTERFACE_0
 
 static void
 test_ping(void)
-- 
2.24.1

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


[PATCH rtems-libbsd v3 1/3] rtemsbsd/bus: Add PCI support to the nexus bus

2021-08-08 Thread Chris Johns
- Add PCI IO region support

- Add support map buffers to PCI address space

- Add BSP conditional IO space support. Some PC implementations
  have PCI IO space mapped differently to memory space and this needs
  to be reflected in the busspace.

- Include bsp.h to pick per BSP configuration.

Closes #4245
---
 rtemsbsd/include/machine/bus.h| 370 ++
 rtemsbsd/rtems/rtems-kernel-bus-dma.c |   5 +-
 rtemsbsd/rtems/rtems-kernel-nexus.c   |  52 +++-
 3 files changed, 312 insertions(+), 115 deletions(-)

diff --git a/rtemsbsd/include/machine/bus.h b/rtemsbsd/include/machine/bus.h
index 2f0e7ad6..999f5d45 100644
--- a/rtemsbsd/include/machine/bus.h
+++ b/rtemsbsd/include/machine/bus.h
@@ -6,9 +6,13 @@
  * @brief TODO.
  *
  * File origin from FreeBSD 'sys/amd64/include/bus.h'.
+ *
+ * Conditionally supports PCI IO regions (IO Ports).
  */
 
 /*-
+ * Copyright (c) 2021 Chris Johns.  All rights reserved.
+ *
  * Copyright (c) 2009, 2015 embedded brains GmbH.  All rights reserved.
  *
  *  embedded brains GmbH
@@ -25,7 +29,7 @@
  * 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.
@@ -34,7 +38,7 @@
  *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.
@@ -123,9 +127,46 @@
 #endif
 
 #ifdef __i386__
-  #error "your include paths are wrong"
+  #error "x86 has its own bus.h; check your include paths are correct"
 #endif
 
+#include 
+
+/*
+ * BSP PCI Support
+ *
+ * The RTEMS Nexus bus support can optionaly support PC PCI spaces that
+ * mapped to BSP speciic address spaces. Add the following define to
+ * the BSP header file to enable this support:
+ *
+ *  #define BSP_HAS_PC_PCI
+ *
+ * If enabled a BSP must the following IO region calls:
+ *
+ * inb  : read 8 bits
+ * outb : write 8 bits
+ * inw  : read 16 bits
+ * outw : write 16 bits
+ * inl  : read 32 bits
+ * outl : write 32 bits
+ *
+ * The BSP needs to provide the DRAM address space offset
+ * PCI_DRAM_OFFSET. This is the base address of the DRAM as seen by a
+ * PCI master.
+ *
+ * i386 BSPs have a special bus.h file and do not use this file.
+ */
+
+#ifdef BSP_HAS_PC_PCI
+
+/*
+ * Values for the bus space tag, not to be used directly by MI code.
+ */
+#defineBSP_BUS_SPACE_IO0   /* space is i/o space */
+#defineBSP_BUS_SPACE_MEM   1   /* space is mem space */
+
+#endif /* BSP_HAS_PC_PCI */
+
 /*
  * Bus address alignment.
  */
@@ -144,6 +185,7 @@
 /*
  * Bus access.
  */
+#define BUS_SPACE_INVALID_DATA (~0)
 #define BUS_SPACE_UNRESTRICTED (~0U)
 
 /*
@@ -222,6 +264,102 @@ bus_space_barrier(bus_space_tag_t bst __unused, 
bus_space_handle_t bsh, bus_size
/* Do nothing */
 }
 
+/*
+ * BSP Bus Space Map Support
+ *
+ * Provide as C macros in the BSP header (bsp.h) the following:
+ *
+ *  RTEMS_BSP_READ_1
+ *  RTEMS_BSP_READ_2
+ *  RTEMS_BSP_READ_4
+ *  RTEMS_BSP_READ_8
+ *  RTEMS_BSP_WRITE_1
+ *  RTEMS_BSP_WRITE_2
+ *  RTEMS_BSP_WRITE_4
+ *  RTEMS_BSP_WRITE_8
+ */
+
+static __inline uint8_t
+bsp_bus_space_read_1(const uint8_t __volatile *bsp)
+{
+#if defined(RTEMS_BSP_READ_1)
+   return RTEMS_BSP_READ_1(bsp);
+#else
+   return (*bsp);
+#endif
+}
+
+static __inline uint16_t
+bsp_bus_space_read_2(const uint16_t __volatile *bsp)
+{
+#if defined(RTEMS_BSP_READ_2)
+   return RTEMS_BSP_READ_2(bsp);
+#else
+   return (*bsp);
+#endif
+}
+
+static __inline uint32_t
+bsp_bus_space_read_4(const uint32_t __volatile *bsp)
+{
+#if defined(RTEMS_BSP_READ_4)
+   return RTEMS_BSP_READ_4(bsp);
+#else
+   return (*bsp);
+#endif
+}
+
+static __inline uint64_t
+bsp_bus_space_read_8(const uint64_t __volatile *bsp)
+{
+#if defined(RTEMS_BSP_READ_8)
+   return RTEMS_BSP_READ_8(bsp);
+#else
+   return (*bsp);
+#endif
+}
+
+static __inline void
+bsp_bus_space_write_1(uint8_t __volatile *bsp, uint8_t val)
+{
+#if defined(RTEMS_BSP_WRITE_1)
+   RTEMS_BSP_WRITE_1(bsp, val);
+#else
+   *bsp = val;
+#endif
+}
+
+static __inline void
+bsp_bus_space_write_2(uint16_t __volatile *bsp, uint16_t val)
+{
+#if defined(RTEMS_BSP_WRITE_2)
+   RTEMS_BSP_WRITE_2(bsp, val);
+#else
+   *bsp = val;
+#endif
+}
+
+static __inline void
+bsp_bus_space_write_4(uint32_t __volatile *bsp, uint32_t val)
+{
+#if defined(RTEMS_BSP_WRITE_4)
+   RTEMS_BSP_WRITE_4(bsp, val);

[PATCH rtems-libbsd v3 2/3] rtemsbsd: Add interface support routines

2021-08-08 Thread Chris Johns
- Add the ability to check if an interface is up
---
 libbsd.py  |   1 +
 rtemsbsd/include/rtems/bsd/iface.h |  62 
 rtemsbsd/rtems/rtems-bsd-iface.c   | 146 +
 3 files changed, 209 insertions(+)
 create mode 100644 rtemsbsd/include/rtems/bsd/iface.h
 create mode 100644 rtemsbsd/rtems/rtems-bsd-iface.c

diff --git a/libbsd.py b/libbsd.py
index 09a1fbc4..6e09a07c 100644
--- a/libbsd.py
+++ b/libbsd.py
@@ -253,6 +253,7 @@ class rtems(builder.Module):
 'rtems/rtems-bsd-cxx.cc',
 'rtems/rtems-bsd-get-ethernet-addr.c',
 'rtems/rtems-bsd-ifconfig.c',
+'rtems/rtems-bsd-iface.c',
 'rtems/rtems-bsd-ifconfig-lo0.c',
 'rtems/rtems-bsd-init-dhcp.c',
 'rtems/rtems-bsd-rc-conf-net.c',
diff --git a/rtemsbsd/include/rtems/bsd/iface.h 
b/rtemsbsd/include/rtems/bsd/iface.h
new file mode 100644
index ..9e583b8a
--- /dev/null
+++ b/rtemsbsd/include/rtems/bsd/iface.h
@@ -0,0 +1,62 @@
+/**
+ * @file
+ *
+ * @ingroup rtems_bsd_rtems
+ *
+ * @brief This file provide a high level interface to simple interface
+ * support calls.
+ */
+
+/*
+ * Copyright (c) 2021. Chris Johns . All rights reserved.
+ *
+ * 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 AUTHOR 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 AUTHOR 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.
+ */
+
+#ifndef _RTEMS_BSD_IFACE_H_
+#define _RTEMS_BSD_IFACE_H_
+
+#include 
+
+#include 
+
+#include 
+#include 
+#include 
+
+struct rtems_bsd_iface {
+   char name[IFNAMSIZ];
+   int unit;
+   struct in_addr address;
+   size_t hw_len;
+   uint8_t hw_address[16];
+   struct ifreq ifr;
+   int linkstate;
+};
+
+/*
+ * These calls return 0 is successful and -1 and errno set on error.
+ */
+int rtems_bsd_iface_get(const char *name, struct rtems_bsd_iface *iface);
+int rtems_bsd_iface_link_state(const char *name, bool *state);
+
+#endif
diff --git a/rtemsbsd/rtems/rtems-bsd-iface.c b/rtemsbsd/rtems/rtems-bsd-iface.c
new file mode 100644
index ..4cf25a9b
--- /dev/null
+++ b/rtemsbsd/rtems/rtems-bsd-iface.c
@@ -0,0 +1,146 @@
+/**
+ * @file
+ *
+ * @ingroup rtems_bsd_rtems
+ *
+ * @brief Interface support routines
+ */
+
+/*
+ * Copyright (c) 2021. Chris Johns   All rights reserved.
+ *
+ * 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 AUTHOR 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 AUTHOR 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.
+ */
+
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#include 
+
+#if !defined(RTEMS_BSD_IFACE_TRACE)
+#define RTEMS_BSD_IFACE_TRACE 0
+#endif
+
+int

LibBSD - bus space support for PC PCI devices

2021-08-08 Thread Chris Johns
Hello,

These patches address 2 issues, PC PCI suport for non-Intel targets
and tests with static IP addresses faiing.

- PC PCI

BSPs like MVME2700 (MVME2307) have PC PCI devices with separate IO and
memory bus spaces mapped into separate address spaces. This requires
the BSP provides suitable remapping support. The PCI devices report relative
offsets and not absolute mappings. The support added is only enabled if
the BSP does this otherwise there is no change to existing BSPs and the
direct mappings the bus space support provides.

The bus space support has been cleaned up and a number of issues in some
of the support functions fixed. They must have not been used.

- Tests Waiting for Link UP

Tests that use the network test configured interface and have a static IP 
address did not wait for the link to be UP before starting the test. A DHCP 
client configuration would and simulators like QEMU did not simulate the 
time a PHY takes to come on line.

Add support to check if an interface has a valid and UP media link and
have tests wait for this to happen. I have added the support to 2 tests
and I am sure there other so please add the support when you find one.

Thanks
Chris


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


Re: [PATCH rtems-libbsd v2 0/5] Beagle BSP: USB Template Driver for CDC Ethernet

2021-08-08 Thread Chris Johns
On 9/8/21 6:20 am, Christian Mauderer wrote:
> I think all open questions should be answered or did I miss one from your 
> side?
> If not I would like to push the patches.

As asked in another thread can they please wait?

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


Re: [PATCH rtems-libbsd v2 0/5] Beagle BSP: USB Template Driver for CDC Ethernet

2021-08-08 Thread Chris Johns
On 8/8/21 6:06 pm, Christian Mauderer wrote:
> Hello Husni,
> 
> On 07/08/2021 21:56, Ahamed Husni wrote:
>> Hi Christian,
>>
>> Are there any issues I should address in this patch set?
> 
> I think the big open question is the one from Chris:
> 
> https://lists.rtems.org/pipermail/devel/2021-July/068634.html
> 
> You mentioned that you filtered the ones for the imported code. That's OK for
> most symbols if they are not relevant for the part where you work on. But 
> Chris
> copied a whole block of "extern struct usb_temp_device_desc *". It's not clear
> for me whether you filtered some of them or whether they haven't been 
> generated.
> 
> If they are generated: Please keep them. Basically keep everything that is in
> one of the files that you touch, except if you have a good reason why the 
> symbol
> should _not_ be added.
> 
> If they are not generated: Please clearly say that.
> 

The issue is the merging against my changes and as I have stated in the other
thread I have replaced the change script because it on worked on Linux plus the
sort order was something I could not match on other host platforms.

My changes address the sort order, tools used and I added the few symbols that
must have been missed over a period of time. That happens. I hope the updated
procedure is simpler and safer to use on any host platform we support.

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


Re: [PATCH rtems-libbsd v2 0/5] Beagle BSP: USB Template Driver for CDC Ethernet

2021-08-08 Thread Chris Johns
On 1/8/21 9:27 pm, Christian Mauderer wrote:
> Hello Husni,
> 
> just tested that and it works fine. With a simple curl I can reach about 8.5
> MByte/s sending to the beagle and 19.5 MByte/s receiving from it.
> 
> Please take a look at the points that Chris Johns asked. As soon as these are
> addressed, I think the patches could be merged.

Can these patches please wait until my patches are merged? I have not seen any
issues and given the size I am waiting a while.

I ask because the symbol changes in the namespace ripple through the 150K lines
of patches I have posted for review and it is a pain to rebase.

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


Re: [PATCH rtems-libbsd v2 5/5] create-kernel-namespace for USB Template driver

2021-08-08 Thread Chris Johns


On 2/8/21 4:24 am, Ahamed Husni wrote:
> Hello Chris,
> 
> On Thu, Jul 29, 2021 at 5:41 AM Chris Johns  <mailto:chr...@rtems.org>> wrote:
> 
> On 28/7/21 9:56 pm, Husni Faiz wrote:
> > Signed-off-by: Husni Faiz  <mailto:ahamedhusn...@gmail.com>>
> > ---
> >  rtemsbsd/include/machine/rtems-bsd-kernel-namespace.h | 5 +
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/rtemsbsd/include/machine/rtems-bsd-kernel-namespace.h
> b/rtemsbsd/include/machine/rtems-bsd-kernel-namespace.h
> > index 97cdb625..ae56ad9c 100644
> > --- a/rtemsbsd/include/machine/rtems-bsd-kernel-namespace.h
> > +++ b/rtemsbsd/include/machine/rtems-bsd-kernel-namespace.h
> > @@ -5279,6 +5279,7 @@
> >  #define      usbd_do_request_proc _bsd_usbd_do_request_proc
> >  #define      usbd_dummy_timeout _bsd_usbd_dummy_timeout
> >  #define      usb_debug _bsd_usb_debug
> > +#define      usb_decode_str_desc _bsd_usb_decode_str_desc
> >  #define      usbd_enum_is_locked _bsd_usbd_enum_is_locked
> >  #define      usbd_enum_lock _bsd_usbd_enum_lock
> >  #define      usbd_enum_lock_sig _bsd_usbd_enum_lock_sig
> > @@ -5515,8 +5516,12 @@
> >  #define      usb_suspend_resume _bsd_usb_suspend_resume
> >  #define      usb_temp_get_desc_p _bsd_usb_temp_get_desc_p
> >  #define      usb_template _bsd_usb_template
> > +#define      usb_template_cdce _bsd_usb_template_cdce
> > +#define      usb_temp_setup _bsd_usb_temp_setup
> >  #define      usb_temp_setup_by_index_p _bsd_usb_temp_setup_by_index_p
> > +#define      usb_temp_sysctl _bsd_usb_temp_sysctl
> >  #define      usb_temp_unload _bsd_usb_temp_unload
> > +#define      usb_temp_unsetup _bsd_usb_temp_unsetup
> >  #define      usb_temp_unsetup_p _bsd_usb_temp_unsetup_p
> >  #define      usb_test_quirk _bsd_usb_test_quirk
> >  #define      usb_test_quirk_p _bsd_usb_test_quirk_p
> 
> How were these additions done? 
> 
>  
> I used the create-kernel-namespace.sh script which auto genarates these 
> definitions.
> That generated more definitions than I have added here. I only filtered out 
> the
> definitions for the codes I imported.

Thanks and yes it is wise to do this.

I am going to ask for these changes to be queued behind my posted patches for
6-freebsd-12 where I have removed the create-kernel-namespace.sh and replaced it
with a new python script.

> I ask because I see these externs in the template code ...
> 
> 
> extern struct usb_temp_device_desc usb_template_audio;
> extern struct usb_temp_device_desc usb_template_cdce;
> extern struct usb_temp_device_desc usb_template_kbd;
> extern struct usb_temp_device_desc usb_template_modem;
> extern struct usb_temp_device_desc usb_template_mouse;
> extern struct usb_temp_device_desc usb_template_msc;
> extern struct usb_temp_device_desc usb_template_mtp;
> extern struct usb_temp_device_desc usb_template_phone;
> extern struct usb_temp_device_desc usb_template_serialnet;
> extern struct usb_temp_device_desc usb_template_midi;
> extern struct usb_temp_device_desc usb_template_multi;
> extern struct usb_temp_device_desc usb_template_cdceem;
> 
> 
> These externs are for all the usb templates supported by freebsd.
> I only have imported the usb_template_cdce. Other templates are
> not imported yet. Shall I exclude them?

All good. I was more interested in the update process than the USB side of 
things :)

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

Re: [PATCH v2] rtems-utils: Change data type definition

2021-08-05 Thread Chris Johns
On 6/8/21 6:43 am, Ryan Long wrote:
> Remove typedef of ostream_guard and change datatype of ostream_guards to
> have the namespace in the variable declarations.
> ---
>  linkers/rtems-exeinfo.cpp| 6 ++
>  tester/covoar/CoverageMapBase.cc | 4 +---
>  tester/covoar/ReportsHtml.cc | 4 +---
>  tester/covoar/ReportsText.cc | 4 +---
>  4 files changed, 5 insertions(+), 13 deletions(-)
> 
> diff --git a/linkers/rtems-exeinfo.cpp b/linkers/rtems-exeinfo.cpp
> index c9bf5b6..caae168 100644
> --- a/linkers/rtems-exeinfo.cpp
> +++ b/linkers/rtems-exeinfo.cpp
> @@ -53,8 +53,6 @@
>  #define kill(p,s) raise(s)
>  #endif
>  
> -typedef rtems::utils::ostream_guard ostream_guard;
> -
>  namespace rld
>  {
>namespace exeinfo
> @@ -369,7 +367,7 @@ namespace rld
> */
>  
>rld::strings all_flags;
> -  ostream_guard old_state( std::cout );
> +  ::rtems::utils::ostream_guard old_state( std::cout );
>  
>size_t source_max = 0;
>  
> @@ -636,7 +634,7 @@ namespace rld
>  
>  void image::output_tls ()
>  {
> -  ostream_guard old_state( std::cout );
> +  ::rtems::utils::ostream_guard old_state( std::cout );

Why the global namespace prefix, ie `::` at the start? I have only seen this
when referencing a global symbol that is not in a namespace, ie a C call.

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


Re: [PATCH v1 1/5] rtems-utils.h: Remove ostream_guard

2021-08-04 Thread Chris Johns
On 5/8/21 9:13 am, Gedare Bloom wrote:
> On Wed, Aug 4, 2021 at 4:47 PM Chris Johns  wrote:
>>
>> On 5/8/21 1:54 am, Ryan Long wrote:
>>> ostream_guard did not fix the "Not restoring ostream format" Coverity
>>> issues as hoped, so there's no reason to have it.
>>
>> Can you capture the missing pieces here?
>>
> Also, just because it didn't fix coverity, doesn't mean it's a bad
> thing to do. It seemed reasonable to me to save/restore the format so
> it is unmodified across calls.

Yes, we should drive the design and not coverity.

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


Re: [PATCH v1 1/5] rtems-utils.h: Remove ostream_guard

2021-08-04 Thread Chris Johns
On 5/8/21 1:54 am, Ryan Long wrote:
> ostream_guard did not fix the "Not restoring ostream format" Coverity
> issues as hoped, so there's no reason to have it.

Can you capture the missing pieces here?

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


Re: [PATCH v1 0/5] Fix ostream format errors

2021-08-04 Thread Chris Johns
On 5/8/21 1:54 am, Ryan Long wrote:
> 
> The last "Restore ostream format" patch set didn't fix the Coverity issues.
> We verified that resetiosflags() will get rid of the issues complaints about
> std::left and std::right changing the stream's format, I noticed that it
> didn't complain if std::setfill(' ') was used after changing the setfill value
> to something else.
> 
> I don't know if Coverity will recognize storing the precision value and
> restoring it as a fix for the setprecision() calls, but the only other
> option is to just set the precision value to 6 (the default value).
> 

This patch is wrong. Please do not reset. You need to restore.

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


Re: [PATCH rtems-libbsd 0/5] RTEMS LibBSD Documentation

2021-08-04 Thread Chris Johns
On 5/8/21 2:22 am, Christian Mauderer wrote:
> On 04/08/2021 18:09, Gedare Bloom wrote:
>> On Wed, Aug 4, 2021 at 9:05 AM Christian MAUDERER
>>  wrote:
>>> Am 04.08.21 um 16:55 schrieb Gedare Bloom:
 On Wed, Aug 4, 2021 at 4:18 AM Christian MAUDERER
  wrote:
 My preference would be to leave the legacy doc where it is,
>>>
>>> Just a comment for that point: I know that the doc has been moved around
>>> a bit. But I think we should try to get all similar options onto the
>>> same "level". Otherwise if a user searches for "How to do networking
>>> with RTEMS" and he finds https://docs.rtems.org/ the only manual with
>>> "Networking" is the legacy stack. If it is on the same page (level,
>>> hirarchie, ...) like the headline "libbsd Networking and other cool
>>> stuff" or "lwIP", a user instantly can see that there is more than one
>>> option.
>>>
>>
>> That's a good point, but I want to keep the legacy stack separate from
>> the rest of the documentation to make it simpler to deprecate/obsolete
>> it. I don't see value in moving it, just to kill it in the next
>> release. AFAIK, we will strongly discourage anyone from using it in
>> rtems-6, and I'd like to kill it off moving forward once we feel
>> confident that lwIP is feasible for us to maintain. Your point about
>> marketing is well-taken though.
> 
> OK. I didn't expect that we are that far that we already plan to (maybe) 
> remove
> it in the next release. In that case I agree: It's not worth the effort to 
> move it.
> 

Should something be added to the legacy manual indicating it's status?

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


Re: [PATCH rtems-libbsd 0/5] RTEMS LibBSD Documentation

2021-08-04 Thread Chris Johns


On 4/8/21 6:34 pm, Christian MAUDERER wrote:
> Hello Chris,
> 
> Am 04.08.21 um 09:28 schrieb Chris Johns:
>> On 3/8/21 5:00 pm, Christian MAUDERER wrote:
>>> Hello,
>>>
>>> Am 03.08.21 um 04:07 schrieb Chris Johns:
>>>> On 3/8/21 3:24 am, Sebastian Huber wrote:
>>>>> On 02/08/2021 18:37, Vijay Kumar Banerjee wrote:
>>>>>> I think there should be a high-level user manual subsection for
>>>>>> networking that describes how the selection of the network stack
>>>>>> works. We can then add another subsection about lwip since legacy
>>>>>> already has one, and libbsd is getting added now.
>>>>>
>>>>> This sounds like a good approach.
>>>>>
>>>>
>>>> I agree.
>>>>
>>>> Chris
>>>
>>> Just so that we are all on the same page:
>>>
>>> That would be a number of new subsections in
>>> https://docs.rtems.org/branches/master/user/index.html:
>>>
>>> - 15. Selecting a Network Stack
>>> - 16. Add-on: libbsd Stack
>>> - 17. Add-on: lwIP
>>> - 18. Add-on: Legacy Network Stack
>>>
>>> Or was it meant to be only a section
>>
>> I think a single section so all things networking are grouped.
> 
> That will result in either a deep nesting for the chapters or in very hard to
> organize information because two levels are used up already.
> 
> For example the current legacy network manual has a
> 
>   5.3.1. Additional include files
> 
> If we make one "networking" group, that would be
> 
>   15. Networking
>   15.3 Legacy Network Stack
>   15.3.5 Using Networking in an RTEMS application
>   15.3.5.3 Initialization
>   15.3.5.3.1 Additional include files
> 
> And please also note: I think we should see libbsd more as an Add-on package 
> and
> less as a pure network stack. 

Yes this is true and if your top level is LibBSD you do not see networking and
then networking becomes confusing if spread out in pieces. What would LibBSD and
Legacy Networking mean if you are new RTEMS? Is Legacy networking some form of
old embedded realtime protocol we support? :)

Maybe Networking is about the history, features and options available in each
package (Selection) and the LibBSD part is a link to the networking section of
that package? This would mean the lwIP and Legacy network stack is still under
Networking.

Does "Additional includes" etc need to be a numbered section? Why not just make
a heading in the block without it being a section?

> It provides a lot more than just networking. It
> has USB, it can add HDMI (on beagle for example), it includes an OpenSSL
> implementation, ...

Yes and if you look at the top level you could see something like:

 9 Networking
10 USB
11 Graphics
12 FreeBSD On RTEMS

The user manual has clear top level sections and I would like it to stay this
way. But yes I understand it is hard to get the breakdown right and it may take
a couple more loops until we have something that is clear.

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

Re: [PATCH rtems-libbsd 0/5] RTEMS LibBSD Documentation

2021-08-04 Thread Chris Johns
On 3/8/21 5:00 pm, Christian MAUDERER wrote:
> Hello,
> 
> Am 03.08.21 um 04:07 schrieb Chris Johns:
>> On 3/8/21 3:24 am, Sebastian Huber wrote:
>>> On 02/08/2021 18:37, Vijay Kumar Banerjee wrote:
>>>> I think there should be a high-level user manual subsection for
>>>> networking that describes how the selection of the network stack
>>>> works. We can then add another subsection about lwip since legacy
>>>> already has one, and libbsd is getting added now.
>>>
>>> This sounds like a good approach.
>>>
>>
>> I agree.
>>
>> Chris
> 
> Just so that we are all on the same page:
> 
> That would be a number of new subsections in
> https://docs.rtems.org/branches/master/user/index.html:
> 
> - 15. Selecting a Network Stack
> - 16. Add-on: libbsd Stack
> - 17. Add-on: lwIP
> - 18. Add-on: Legacy Network Stack
> 
> Or was it meant to be only a section

I think a single section so all things networking are grouped.

Chris

> 
> - 15. Selecting a Network Stack
> in https://docs.rtems.org/branches/master/user/index.html,
> - a new manual "libbsd" in the top level https://docs.rtems.org/,
> - a new manual "lwip" in the top level https://docs.rtems.org/
> 
> By the way: I think we maybe should keep in mind that there can be more add-on
> packages added some-when in the future that are not network stacks. We could 
> for
> example add documentation for a CAN library, a display library or a weather
> control station. So if we need a top level name for all packages, we maybe
> should call it "add-on packages" instead of "network libraries".
> 
> Best regards
> 
> Christian
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: [PATCH v2 00/13] Remove app_common

2021-08-02 Thread Chris Johns
OK to push.

Chris

On 3/8/21 6:44 am, Ryan Long wrote:
> Hi,
> 
> In this iteration I:
> 
> - Removed the typedef in ObjdumpProcessor.h
> - Changed targetInfo to be a shared pointer
> - Got rid of some of the get & set methods for targetInfo_m
> - Added lastState_m to initialization list in ReportsHtml.cc
> 
> Thanks,
> Ryan
> 
> Ryan Long (13):
>   Remove AllExplanations global variable
>   ReportsHtml.cc: Initialize lastState_m
>   Remove objdumpProcessor global variable
>   Remove Verbose global variable
>   Remove dynamicLibrary global variable
>   Remove projectName global variable
>   Remove outputDirectory global variable
>   Remove input buffer global variables
>   Remove SymbolsToAnalyze global variable
>   Remove BranchInfoAvailable global variable
>   Remove TargetInfo global variable
>   app_common: Remove functions and macros
>   Remove app_common and all references to it
> 
>  tester/covoar/CoverageReaderBase.cc |   5 ++
>  tester/covoar/CoverageReaderBase.h  |  15 
>  tester/covoar/CoverageReaderQEMU.cc |   7 +-
>  tester/covoar/CoverageReaderTSIM.cc |   5 +-
>  tester/covoar/DesiredSymbols.cc |  28 +++---
>  tester/covoar/DesiredSymbols.h  |  27 --
>  tester/covoar/ExecutableInfo.cc |  23 ++---
>  tester/covoar/ExecutableInfo.h  |  17 +++-
>  tester/covoar/Explanations.cc   |  21 ++---
>  tester/covoar/GcovData.cc   |   7 +-
>  tester/covoar/GcovData.h|  12 ++-
>  tester/covoar/GcovFunctionData.cc   |  15 ++--
>  tester/covoar/GcovFunctionData.h|   6 +-
>  tester/covoar/Makefile  |   6 +-
>  tester/covoar/ObjdumpProcessor.cc   |  68 ++-
>  tester/covoar/ObjdumpProcessor.h| 113 +++-
>  tester/covoar/ReportsBase.cc| 166 
> +++-
>  tester/covoar/ReportsBase.h |  69 +--
>  tester/covoar/ReportsHtml.cc|  64 +-
>  tester/covoar/ReportsHtml.h |  10 ++-
>  tester/covoar/ReportsText.cc|  28 --
>  tester/covoar/ReportsText.h |  10 ++-
>  tester/covoar/SymbolTable.cc|   1 -
>  tester/covoar/TraceConverter.cc |  39 ++---
>  tester/covoar/TraceReaderBase.h |   7 +-
>  tester/covoar/TraceReaderLogQEMU.cc |  25 +-
>  tester/covoar/TraceReaderLogQEMU.h  |   6 +-
>  tester/covoar/TraceWriterBase.h |   9 +-
>  tester/covoar/TraceWriterQEMU.cc|  12 +--
>  tester/covoar/TraceWriterQEMU.h |   4 +-
>  tester/covoar/app_common.cc | 118 -
>  tester/covoar/app_common.h  |  34 
>  tester/covoar/coverage_converter.cc |   1 -
>  tester/covoar/covmerge.cc   |   8 +-
>  tester/covoar/covoar.cc | 125 ++-
>  tester/covoar/wscript   |   3 +-
>  36 files changed, 659 insertions(+), 455 deletions(-)
>  delete mode 100644 tester/covoar/app_common.cc
>  delete mode 100644 tester/covoar/app_common.h
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: [PATCH rtems-libbsd 0/5] RTEMS LibBSD Documentation

2021-08-02 Thread Chris Johns
On 3/8/21 3:24 am, Sebastian Huber wrote:
> On 02/08/2021 18:37, Vijay Kumar Banerjee wrote:
>> I think there should be a high-level user manual subsection for
>> networking that describes how the selection of the network stack
>> works. We can then add another subsection about lwip since legacy
>> already has one, and libbsd is getting added now.
> 
> This sounds like a good approach.
> 

I agree.

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


Re: [PATCH rtems-libbsd 0/5] RTEMS LibBSD Documentation

2021-08-02 Thread Chris Johns
On 2/8/21 4:58 pm, Christian MAUDERER wrote:
> Hello Husni,
> 
> thanks for the patches. I'm sure that this will start a discussion about the
> right place for that documentation. libbsd documentation is a long overdue 
> topic
> that has been neglected by all of us (including myself) so I think it's good 
> to
> start that discussion.

I suggest the user manual.

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


Re: [PATCH rtems-libbsd 1/7] rtemsbsd: Catch timeout overflows

2021-08-02 Thread Chris Johns
On 29/7/21 8:44 am, Chris Johns wrote:
> On 28/7/21 4:44 pm, Sebastian Huber wrote:
>> On 28/07/2021 08:43, Chris Johns wrote:
>>>> Why don't we use the FreeBSD implementation one-to-one:
>>>>
>>>> freebsd-org/sys/kern/kern_clock.c
>>>>
>>> I am fixing the code that exists. I have no idea why kern_clock.c was not 
>>> ported
>>> in the first place. I will not be porting the clock code at this point in 
>>> time,
>>> that is out of scope for my current work.
>>
>> What I meant was to copy the tvtohz() from kern_clock.c to this file.
> 
> Ah ok, sure I can take a look.

The only difference is some extra checks and if there is value in having those
checks they should be added to the cpukit.

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


Re: [PATCH v1 09/13] Remove SymbolsToAnalyze global variable

2021-08-01 Thread Chris Johns


On 2/8/21 1:20 pm, Joel Sherrill wrote:
> 
> 
> On Sun, Aug 1, 2021, 10:11 PM Chris Johns  <mailto:chr...@rtems.org>> wrote:
> 
> On 2/8/21 9:23 am, Ryan Long wrote:
> > - Removed SymbolsToAnalyze from app_common and replaced it with the
> >   symbolsToAnalyze_m member variable in DesiredSymbols, GcovData, and
> >   ObjdumpProcessor
> > - Added a parameter to constructors to initialize symbolsToAnalyze_m
> > - Moved the definition of objdumpLine_t out of ObjdumpProcessor to make
> >   it work with DesiredSymbols
> > ---
> >  tester/covoar/DesiredSymbols.cc   | 12 +++--
> >  tester/covoar/DesiredSymbols.h    | 17 +--
> >  tester/covoar/ExecutableInfo.cc   | 14 +++---
> >  tester/covoar/ExecutableInfo.h    | 14 +-
> >  tester/covoar/GcovData.cc         |  7 +--
> >  tester/covoar/GcovData.h          | 12 -
> >  tester/covoar/GcovFunctionData.cc | 14 --
> >  tester/covoar/GcovFunctionData.h  |  6 ++-
> >  tester/covoar/ObjdumpProcessor.cc | 22 +
> >  tester/covoar/ObjdumpProcessor.h  | 90 
> +++
> >  tester/covoar/ReportsBase.cc      | 98
> +--
> >  tester/covoar/ReportsBase.h       | 33 +
> >  tester/covoar/ReportsHtml.cc      | 16 ---
> >  tester/covoar/ReportsHtml.h       | 11 +++--
> >  tester/covoar/ReportsText.cc      |  8 ++--
> >  tester/covoar/ReportsText.h       |  3 +-
> >  tester/covoar/TraceConverter.cc   | 17 +--
> >  tester/covoar/app_common.cc       |  1 -
> >  tester/covoar/app_common.h        |  1 -
> >  tester/covoar/covoar.cc           | 40 +---
> >  20 files changed, 271 insertions(+), 165 deletions(-)
> >
> > diff --git a/tester/covoar/DesiredSymbols.cc 
> b/tester/covoar/DesiredSymbols.cc
> > index 5d5e637..d76c5af 100644
> > --- a/tester/covoar/DesiredSymbols.cc
> > +++ b/tester/covoar/DesiredSymbols.cc
> > @@ -25,7 +25,6 @@
> >  #include "DesiredSymbols.h"
> >  #include "app_common.h"
> >  #include "CoverageMap.h"
> > -#include "ObjdumpProcessor.h"
> > 
> >  namespace Coverage {
> > 
> > @@ -116,10 +115,10 @@ namespace Coverage {
> >      }
> >    }
> > 
> > -  void DesiredSymbols::preprocess( void )
> > +  void DesiredSymbols::preprocess( const DesiredSymbols& 
> symbolsToAnalyze )
> >    {
> >      // Look at each symbol.
> > -    for (auto& s : SymbolsToAnalyze->set) {
> > +    for (auto& s : symbolsToAnalyze.set) {
> >        // If the unified coverage map does not exist, the symbol was
> >        // never referenced by any executable.  Just skip it.
> >        CoverageMapBase* theCoverageMap = s.second.unifiedCoverageMap;
> > @@ -441,10 +440,13 @@ namespace Coverage {
> >        return [ symbolName ];
> >    }
> > 
> > -  void DesiredSymbols::findSourceForUncovered( bool verbose )
> > +  void DesiredSymbols::findSourceForUncovered(
> > +    bool                  verbose,
> > +    const DesiredSymbols& symbolsToAnalyze
> > +  )
> >    {
> >      // Process uncovered ranges and/or branches for each symbol.
> > -    for (auto& d : SymbolsToAnalyze->set) {
> > +    for (auto& d : symbolsToAnalyze.set) {
> >        // First the unexecuted ranges, ...
> >        CoverageRanges* theRanges = d.second.uncoveredRanges;
> >        if (theRanges != nullptr) {
> > diff --git a/tester/covoar/DesiredSymbols.h 
> b/tester/covoar/DesiredSymbols.h
> > index 791b4ea..70bc39b 100644
> > --- a/tester/covoar/DesiredSymbols.h
> > +++ b/tester/covoar/DesiredSymbols.h
> > @@ -19,6 +19,11 @@
> > 
> >  namespace Coverage {
> > 
> > +  class ObjdumpProcessor;
> > +  struct objdumpLine;
> > +  using objdumpLine_t = objdumpLine;
> > +  class ExecutableInfo;
> > +
> > 
> >    /*!
> >     *
> > @@ -139,7 +144,7 @@ namespace Coverage {
> >      /*!
> >       *  This member contains the disassembly associated with a symbol.
> >       */
> > -    std::list instructions;
> > +    std::list instructions;
> > 
> >      /*!
> >       *  This me

Re: [PATCH v1 11/13] Remove TargetInfo global variable

2021-08-01 Thread Chris Johns
On 2/8/21 9:23 am, Ryan Long wrote:
> - Remove TargetInfo from app_common
> - Created the targetInfo_m  member variable in CoverageReaderBase,
>   TraceWriterBase, and ObjdumpProcessor
> - Made functions to set the value of targetInfo_m
> ---
>  tester/covoar/CoverageReaderBase.cc |  5 +
>  tester/covoar/CoverageReaderBase.h  | 12 
>  tester/covoar/CoverageReaderQEMU.cc |  4 ++--
>  tester/covoar/ObjdumpProcessor.cc   | 29 ++---
>  tester/covoar/ObjdumpProcessor.h| 20 +++-
>  tester/covoar/TraceConverter.cc |  6 --
>  tester/covoar/TraceWriterBase.cc|  5 +
>  tester/covoar/TraceWriterBase.h | 12 
>  tester/covoar/TraceWriterQEMU.cc|  4 ++--
>  tester/covoar/app_common.cc |  1 -
>  tester/covoar/app_common.h  |  1 -
>  tester/covoar/covmerge.cc   |  7 ++-
>  tester/covoar/covoar.cc |  8 ++--
>  13 files changed, 91 insertions(+), 23 deletions(-)
> 
> diff --git a/tester/covoar/CoverageReaderBase.cc 
> b/tester/covoar/CoverageReaderBase.cc
> index e226964..06732a0 100644
> --- a/tester/covoar/CoverageReaderBase.cc
> +++ b/tester/covoar/CoverageReaderBase.cc
> @@ -21,4 +21,9 @@ namespace Coverage {
>{
>  return branchInfoAvailable_m;
>}
> +
> +  void CoverageReaderBase::setTargetInfo( Target::TargetBase* targetInfo )
> +  {
> +targetInfo_m = targetInfo;
> +  }
>  }
> diff --git a/tester/covoar/CoverageReaderBase.h 
> b/tester/covoar/CoverageReaderBase.h
> index ba909e6..6c8cadf 100644
> --- a/tester/covoar/CoverageReaderBase.h
> +++ b/tester/covoar/CoverageReaderBase.h
> @@ -49,9 +49,21 @@ namespace Coverage {
>bool getBranchInfoAvailable() const;
>  
>/*!
> +   *  This method sets the targetInfo_m variable
> +   *
> +   *  @param[in] targetInfo the targetInfo to use
> +   */
> +  void setTargetInfo( Target::TargetBase* targetInfo );
> +
> +  /*!
> * This member variable tells whether the branch info is available.
> */
>bool branchInfoAvailable_m = false;
> +
> +  /*!
> +   * This member variable points to the target's info
> +   */
> +  Target::TargetBase* targetInfo_m = nullptr;
>};
>  
>  }
> diff --git a/tester/covoar/CoverageReaderQEMU.cc 
> b/tester/covoar/CoverageReaderQEMU.cc
> index 802d862..a3d9d02 100644
> --- a/tester/covoar/CoverageReaderQEMU.cc
> +++ b/tester/covoar/CoverageReaderQEMU.cc
> @@ -51,8 +51,8 @@ namespace Coverage {
>  uint8_t notTaken;
>  uint8_t branchInfo;
>  
> -taken  = TargetInfo->qemuTakenBit();
> -notTaken   = TargetInfo->qemuNotTakenBit();
> +taken  = targetInfo_m->qemuTakenBit();
> +notTaken   = targetInfo_m->qemuNotTakenBit();
>  branchInfo = taken | notTaken;
>  
>  //
> diff --git a/tester/covoar/ObjdumpProcessor.cc 
> b/tester/covoar/ObjdumpProcessor.cc
> index f590ece..5e1fb13 100644
> --- a/tester/covoar/ObjdumpProcessor.cc
> +++ b/tester/covoar/ObjdumpProcessor.cc
> @@ -124,8 +124,10 @@ namespace Coverage {
>}
>  
>ObjdumpProcessor::ObjdumpProcessor(
> -DesiredSymbols& symbolsToAnalyze
> -  ): symbolsToAnalyze_m( symbolsToAnalyze )
> +DesiredSymbols& symbolsToAnalyze,
> +Target::TargetBase* targetInfo
> +  ): symbolsToAnalyze_m( symbolsToAnalyze ),
> + targetInfo_m( targetInfo )
>{
>}
>  
> @@ -191,7 +193,7 @@ namespace Coverage {
>  const char *instruction
>)
>{
> -if ( !TargetInfo ) {
> +if ( !targetInfo_m ) {
>fprintf(
>  stderr,
>  "ERROR: ObjdumpProcessor::IsBranch - unknown architecture\n"
> @@ -200,14 +202,14 @@ namespace Coverage {
>return false;
>  }
>  
> -return TargetInfo->isBranch( instruction );
> +return targetInfo_m->isBranch( instruction );
>}
>  
>bool ObjdumpProcessor::isBranchLine(
>  const char* const line
>)
>{
> -if ( !TargetInfo ) {
> +if ( !targetInfo_m ) {
>fprintf(
>  stderr,
>  "ERROR: ObjdumpProcessor::isBranchLine - unknown architecture\n"
> @@ -216,7 +218,7 @@ namespace Coverage {
>return false;
>  }
>  
> -return  TargetInfo->isBranchLine( line );
> +return  targetInfo_m->isBranchLine( line );
>}
>  
>bool ObjdumpProcessor::isNop(
> @@ -224,7 +226,7 @@ namespace Coverage {
>  int&  size
>)
>{
> -if ( !TargetInfo ){
> +if ( !targetInfo_m ){
>fprintf(
>  stderr,
>  "ERROR: ObjdumpProcessor::isNop - unknown architecture\n"
> @@ -233,7 +235,7 @@ namespace Coverage {
>return false;
>  }
>  
> -return TargetInfo->isNopLine( line, size );
> +return targetInfo_m->isNopLine( line, size );
>}
>  
>void ObjdumpProcessor::getFile(
> @@ -243,12 +245,12 @@ namespace Coverage {
>  )
>{
>  rld::process::statusstatus;
> -rld::process::arg_container args = { TargetInfo->getObjdump(),
> +rld::process::arg_container args = { 

Re: [PATCH v1 09/13] Remove SymbolsToAnalyze global variable

2021-08-01 Thread Chris Johns
On 2/8/21 9:23 am, Ryan Long wrote:
> - Removed SymbolsToAnalyze from app_common and replaced it with the
>   symbolsToAnalyze_m member variable in DesiredSymbols, GcovData, and
>   ObjdumpProcessor
> - Added a parameter to constructors to initialize symbolsToAnalyze_m
> - Moved the definition of objdumpLine_t out of ObjdumpProcessor to make
>   it work with DesiredSymbols
> ---
>  tester/covoar/DesiredSymbols.cc   | 12 +++--
>  tester/covoar/DesiredSymbols.h| 17 +--
>  tester/covoar/ExecutableInfo.cc   | 14 +++---
>  tester/covoar/ExecutableInfo.h| 14 +-
>  tester/covoar/GcovData.cc |  7 +--
>  tester/covoar/GcovData.h  | 12 -
>  tester/covoar/GcovFunctionData.cc | 14 --
>  tester/covoar/GcovFunctionData.h  |  6 ++-
>  tester/covoar/ObjdumpProcessor.cc | 22 +
>  tester/covoar/ObjdumpProcessor.h  | 90 +++
>  tester/covoar/ReportsBase.cc  | 98 
> +--
>  tester/covoar/ReportsBase.h   | 33 +
>  tester/covoar/ReportsHtml.cc  | 16 ---
>  tester/covoar/ReportsHtml.h   | 11 +++--
>  tester/covoar/ReportsText.cc  |  8 ++--
>  tester/covoar/ReportsText.h   |  3 +-
>  tester/covoar/TraceConverter.cc   | 17 +--
>  tester/covoar/app_common.cc   |  1 -
>  tester/covoar/app_common.h|  1 -
>  tester/covoar/covoar.cc   | 40 +---
>  20 files changed, 271 insertions(+), 165 deletions(-)
> 
> diff --git a/tester/covoar/DesiredSymbols.cc b/tester/covoar/DesiredSymbols.cc
> index 5d5e637..d76c5af 100644
> --- a/tester/covoar/DesiredSymbols.cc
> +++ b/tester/covoar/DesiredSymbols.cc
> @@ -25,7 +25,6 @@
>  #include "DesiredSymbols.h"
>  #include "app_common.h"
>  #include "CoverageMap.h"
> -#include "ObjdumpProcessor.h"
>  
>  namespace Coverage {
>  
> @@ -116,10 +115,10 @@ namespace Coverage {
>  }
>}
>  
> -  void DesiredSymbols::preprocess( void )
> +  void DesiredSymbols::preprocess( const DesiredSymbols& symbolsToAnalyze )
>{
>  // Look at each symbol.
> -for (auto& s : SymbolsToAnalyze->set) {
> +for (auto& s : symbolsToAnalyze.set) {
>// If the unified coverage map does not exist, the symbol was
>// never referenced by any executable.  Just skip it.
>CoverageMapBase* theCoverageMap = s.second.unifiedCoverageMap;
> @@ -441,10 +440,13 @@ namespace Coverage {
>return [ symbolName ];
>}
>  
> -  void DesiredSymbols::findSourceForUncovered( bool verbose )
> +  void DesiredSymbols::findSourceForUncovered(
> +bool  verbose,
> +const DesiredSymbols& symbolsToAnalyze
> +  )
>{
>  // Process uncovered ranges and/or branches for each symbol.
> -for (auto& d : SymbolsToAnalyze->set) {
> +for (auto& d : symbolsToAnalyze.set) {
>// First the unexecuted ranges, ...
>CoverageRanges* theRanges = d.second.uncoveredRanges;
>if (theRanges != nullptr) {
> diff --git a/tester/covoar/DesiredSymbols.h b/tester/covoar/DesiredSymbols.h
> index 791b4ea..70bc39b 100644
> --- a/tester/covoar/DesiredSymbols.h
> +++ b/tester/covoar/DesiredSymbols.h
> @@ -19,6 +19,11 @@
>  
>  namespace Coverage {
>  
> +  class ObjdumpProcessor;
> +  struct objdumpLine;
> +  using objdumpLine_t = objdumpLine;
> +  class ExecutableInfo;
> +
>  
>/*!
> *
> @@ -139,7 +144,7 @@ namespace Coverage {
>  /*!
>   *  This member contains the disassembly associated with a symbol.
>   */
> -std::list instructions;
> +std::list instructions;
>  
>  /*!
>   *  This member contains the executable that was used to
> @@ -263,8 +268,12 @@ namespace Coverage {
>   *  uncovered ranges or branches.
>   *
>   *  @param[in] verbose specifies whether to be verbose with output
> + *  @param[in] symbolsToAnalyze the symbols to be analyzed
>   */
> -void findSourceForUncovered( bool verbose );
> +void findSourceForUncovered(
> +  bool verbose,
> +  const DesiredSymbols& symbolsToAnalyze
> +);
>  
>  /*!
>   *  This method returns the total number of branches always taken
> @@ -398,8 +407,10 @@ namespace Coverage {
>  /*!
>   *  This method preprocesses each symbol's coverage map to mark nop
>   *  and branch information.
> + *
> + *  @param[in] symbolsToAnalyze the symbols to be analyzed
>   */
> -void preprocess( void );
> +void preprocess( const DesiredSymbols& symbolsToAnalyze );
>  
>private:
>  
> diff --git a/tester/covoar/ExecutableInfo.cc b/tester/covoar/ExecutableInfo.cc
> index 9c3031e..328f970 100644
> --- a/tester/covoar/ExecutableInfo.cc
> +++ b/tester/covoar/ExecutableInfo.cc
> @@ -10,9 +10,9 @@
>  #include 
>  
>  #include "ExecutableInfo.h"
> +#include "ObjdumpProcessor.h"
>  #include "app_common.h"
>  #include "CoverageMap.h"
> -#include "DesiredSymbols.h"
>  #include "SymbolTable.h"
>  
>  namespace Coverage {
> @@ -20,9 +20,11 @@ namespace 

Re: [PATCH] rtems: Generate

2021-07-31 Thread Chris Johns
On 30/7/21 4:52 pm, Sebastian Huber wrote:
> On 30/07/2021 00:54, Chris Johns wrote:
>> On 29/7/21 10:59 pm, Sebastian Huber wrote:
>>> On 29/07/2021 14:51, Sebastian Huber wrote:
>>>> Change license to BSD-2-Clause according to file histories and
>>>> documentation re-licensing agreement.
>>>>
>>>> Place the group into the I/O Manager group.  Add all source files to the
>>>> group.
>>>>
>>>> Update #3899.
>>>> Update #3993.
>>>
>>> It would be easy to add a chapter for this to the Classic API Guide (for 
>>> example
>>> "Kernel Character I/O Support"):
>>>
>>> https://ftp.rtems.org/pub/rtems/people/sebh/c-user.pdf
>>>
>>
>> Looks good.
>>
>> I think something should be said about the relationship of this interface to 
>> the
>> standard console and termios streams. Specifically they can use the same BSP
>> device and the output can become mixed if both are being used at the same 
>> time.
>>
>> Should something be said about which calls can be used during interrupts? 
>> Also
>> concurrent calls on SMP system are not serialised.
>>
> I updated the proposal:
> 
> https://lists.rtems.org/pipermail/devel/2021-July/068657.html
> 

Looks good. Thanks.

OK from me.

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

Re: doubt about 'bool is_executing'

2021-07-29 Thread Chris Johns
On 29/7/21 11:07 pm, Sebastian Huber wrote:
> On 29/07/2021 15:01, Gedare Bloom wrote:
> It would be quite bad for interoperability if the bool type is not defined by
> the ABI.  For example:
> 
> https://developer.arm.com/documentation/ihi0055/b/

https://yarchive.net/comp/linux/bool.html

Bool has been a sore point, even further back to the Alpha days.

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

Re: [PATCH] rtems-utils: Change data type definition

2021-07-29 Thread Chris Johns
Why not use the namespace explicitly and avoid this shortening? Namespaces are
there to help.

With C++ I have a simple rule which is always use the namespace path where
possible. There are a few cases, for example in a class, a function or local
block where using an alias helps the formatting otherwise I avoid doing this.

I have found shortening like this can create churn in more complex cases where
the number of cases grows and the potential for clashes rises requiring some
pieces to change and that results in churn. In this case I need to remember the
local `ostream_guard` is actually the standard one and not something special
rather than just knowing this by reading it and in the more complex cases I need
to remember more and more mappings to understand the code.

Chris

On 29/7/21 11:34 pm, Ryan Long wrote:
> Change typedef for ostream_guard to using.
> ---
>  linkers/rtems-exeinfo.cpp| 2 +-
>  tester/covoar/CoverageMapBase.cc | 2 +-
>  tester/covoar/ReportsHtml.cc | 2 +-
>  tester/covoar/ReportsText.cc | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/linkers/rtems-exeinfo.cpp b/linkers/rtems-exeinfo.cpp
> index c9bf5b6..faa98a2 100644
> --- a/linkers/rtems-exeinfo.cpp
> +++ b/linkers/rtems-exeinfo.cpp
> @@ -53,7 +53,7 @@
>  #define kill(p,s) raise(s)
>  #endif
>  
> -typedef rtems::utils::ostream_guard ostream_guard;
> +using ostream_guard = rtems::utils::ostream_guard;
>  
>  namespace rld
>  {
> diff --git a/tester/covoar/CoverageMapBase.cc 
> b/tester/covoar/CoverageMapBase.cc
> index 4de9307..9966f73 100644
> --- a/tester/covoar/CoverageMapBase.cc
> +++ b/tester/covoar/CoverageMapBase.cc
> @@ -16,7 +16,7 @@
>  
>  #include "CoverageMapBase.h"
>  
> -typedef rtems::utils::ostream_guard ostream_guard;
> +using ostream_guard = rtems::utils::ostream_guard;
>  
>  namespace Coverage {
>  
> diff --git a/tester/covoar/ReportsHtml.cc b/tester/covoar/ReportsHtml.cc
> index e276732..d68efd4 100644
> --- a/tester/covoar/ReportsHtml.cc
> +++ b/tester/covoar/ReportsHtml.cc
> @@ -14,7 +14,7 @@
>  #include "DesiredSymbols.h"
>  #include "ObjdumpProcessor.h"
>  
> -typedef rtems::utils::ostream_guard ostream_guard;
> +using ostream_guard = rtems::utils::ostream_guard;
>  
>  #if 0
>  #define TABLE_HEADER_CLASS \
> diff --git a/tester/covoar/ReportsText.cc b/tester/covoar/ReportsText.cc
> index c4a5dbc..aba7e51 100644
> --- a/tester/covoar/ReportsText.cc
> +++ b/tester/covoar/ReportsText.cc
> @@ -12,7 +12,7 @@
>  
>  #include 
>  
> -typedef rtems::utils::ostream_guard ostream_guard;
> +using ostream_guard = rtems::utils::ostream_guard;
>  
>  namespace Coverage {
>  
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: [PATCH] rtems: Generate

2021-07-29 Thread Chris Johns
On 29/7/21 10:59 pm, Sebastian Huber wrote:
> On 29/07/2021 14:51, Sebastian Huber wrote:
>> Change license to BSD-2-Clause according to file histories and
>> documentation re-licensing agreement.
>>
>> Place the group into the I/O Manager group.  Add all source files to the
>> group.
>>
>> Update #3899.
>> Update #3993.
> 
> It would be easy to add a chapter for this to the Classic API Guide (for 
> example
> "Kernel Character I/O Support"):
> 
> https://ftp.rtems.org/pub/rtems/people/sebh/c-user.pdf
> 

Looks good.

I think something should be said about the relationship of this interface to the
standard console and termios streams. Specifically they can use the same BSP
device and the output can become mixed if both are being used at the same time.

Should something be said about which calls can be used during interrupts? Also
concurrent calls on SMP system are not serialised.

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

Re: [PATCH rtems-libbsd v2 2/5] usb_template: Configure template driver only for CDC Ethernet

2021-07-29 Thread Chris Johns
On 29/7/21 4:20 pm, Christian MAUDERER wrote:
> Hello Chris,
> 
> Am 29.07.21 um 02:07 schrieb Chris Johns:
>> On 29/7/21 1:10 am, Ahamed Husni wrote:
>>>
>>>
>>> On Wed, 28 Jul 2021, 17:27 Husni Faiz, >> <mailto:ahamedhusn...@gmail.com>> wrote:
>>>
>>>  Add the conditional macro to prevent the driver from referencing
>>>  the templates which are not imported yet.
>>>
>>>  Include functions which adds the hw.usb.template sysctl variable.
>>>
>>>  Signed-off-by: Husni Faiz >>  <mailto:ahamedhusn...@gmail.com>>
>>>  ---
>>>   freebsd/sys/dev/usb/template/usb_template.c | 4 
>>>   freebsd/sys/dev/usb/usb_device.c            | 8 ++--
>>>   2 files changed, 10 insertions(+), 2 deletions(-)
>>>
>>>  diff --git a/freebsd/sys/dev/usb/template/usb_template.c
>>>  b/freebsd/sys/dev/usb/template/usb_template.c
>>>  index 0650da15..76567e85 100644
>>>  --- a/freebsd/sys/dev/usb/template/usb_template.c
>>>  +++ b/freebsd/sys/dev/usb/template/usb_template.c
>>>  @@ -1433,12 +1433,15 @@ usb_temp_setup_by_index(struct usb_device 
>>> *udev,
>>>  uint16_t index)
>>>          usb_error_t err;
>>>
>>>          switch (index) {
>>>  +#ifndef __rtems__
>>>          case USB_TEMP_MSC:
>>>                  err = usb_temp_setup(udev, _template_msc);
>>>                  break;
>>>  +#endif /* __rtems__ */
>>>          case USB_TEMP_CDCE:
>>>                  err = usb_temp_setup(udev, _template_cdce);
>>>                  break;
>>>  +#ifndef __rtems__
>>>          case USB_TEMP_MTP:
>>>                  err = usb_temp_setup(udev, _template_mtp);
>>>                  break;
>>>  @@ -1469,6 +1472,7 @@ usb_temp_setup_by_index(struct usb_device *udev,
>>>  uint16_t index)
>>>          case USB_TEMP_CDCEEM:
>>>                  err = usb_temp_setup(udev, _template_cdceem);
>>>                  break;
>>>  +#endif /* __rtems__ */
>>>          default:
>>>                  return (USB_ERR_INVAL);
>>>          }
>>>  diff --git a/freebsd/sys/dev/usb/usb_device.c
>>> b/freebsd/sys/dev/usb/usb_device.c
>>>  index ee240949..619cae5a 100644
>>>  --- a/freebsd/sys/dev/usb/usb_device.c
>>>  +++ b/freebsd/sys/dev/usb/usb_device.c
>>>  @@ -28,6 +28,10 @@
>>>    * SUCH DAMAGE.
>>>    */
>>>
>>>  +#ifdef __rtems__
>>>  +#include 
>>>  +#endif
>>>  +
>>>   #ifdef USB_GLOBAL_INCLUDE_FILE
>>>   #include USB_GLOBAL_INCLUDE_FILE
>>>   #else
>>>  @@ -123,7 +127,7 @@ int usb_template = USB_TEMPLATE;
>>>   int    usb_template = -1;
>>>   #endif
>>>
>>>  -#ifndef __rtems__
>>>  +#if !defined(__rtems__) || defined(RTEMS_BSD_MODULE_DEV_USB_TEMPLATE)
>>
>> Is doing this supported by the freebsd-to-rtems.py script?
> 
> I sure hope that it is. I know that I used similar constructions in the past 
> and
> it seems that I'm not the only one, because I'm sure that I haven't touched 
> some
> of these files:

Hmm I thought this was being handled but it seems it is not.

> (Hope that get's through the mail well enoguth that it is readable. If not: 
> Just
> grep for "defined(__rtems__)" yourself.)
> 
> I don't think that our script touches the "#ifdef(__rtems__)" except maybe in
> some special header cases, does it? I thought that's basically how the rebase
> works: Just copying all that stuff unchanged to freebsd-org and then do a 
> rebase
> with all changes. The guards are more or less to make it simpler to see what's
> been added by RTEMS.

Yes.

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

Re: [PATCH 4/4] score: Move per-CPU jobs support

2021-07-29 Thread Chris Johns
On 29/7/21 5:19 pm, Sebastian Huber wrote:
> On 29/07/2021 02:44, Chris Johns wrote:
>>> +void _Per_CPU_Add_job( Per_CPU_Control *cpu, Per_CPU_Job *job )
>>> +{
>>> +  ISR_lock_Context lock_context;
>>> +
>>> +  _Atomic_Store_ulong( >done, 0, ATOMIC_ORDER_RELAXED );
>>> +  _Assert( job->next == NULL );
>> Given the dispatch does not check the handler is adding ...
>>
>>   _Assert( job->context->handler != NULL );
>>
>> worth doing ?
> 
> Yes, good idea. I added this:
> 
> void _Per_CPU_Add_job( Per_CPU_Control *cpu, Per_CPU_Job *job )
> {
>   ISR_lock_Context lock_context;
> 
>   _Assert( job->context != NULL && job->context->handler != NULL );
> 
>   _Atomic_Store_ulong( >done, 0, ATOMIC_ORDER_RELAXED );
>   _Assert( job->next == NULL );
> 

Nice.

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

Re: [PATCH 4/4] score: Move per-CPU jobs support

2021-07-28 Thread Chris Johns



On 28/7/21 9:18 pm, Sebastian Huber wrote:
> Add percpujobs.c to contain the per-CPU jobs implementation.
> ---
>  cpukit/Makefile.am|   1 +
>  cpukit/score/src/percpujobs.c | 124 ++
>  cpukit/score/src/smpmulticastaction.c |  89 +-
>  spec/build/cpukit/objsmp.yml  |   4 +-
>  4 files changed, 130 insertions(+), 88 deletions(-)
>  create mode 100644 cpukit/score/src/percpujobs.c
> 
> diff --git a/cpukit/Makefile.am b/cpukit/Makefile.am
> index c83167668d..5673c4e8fb 100644
> --- a/cpukit/Makefile.am
> +++ b/cpukit/Makefile.am
> @@ -1150,6 +1150,7 @@ endif
>  
>  if HAS_SMP
>  
> +librtemscpu_a_SOURCES += score/src/percpujobs.c
>  librtemscpu_a_SOURCES += score/src/percpustatewait.c
>  librtemscpu_a_SOURCES += score/src/profilingsmplock.c
>  librtemscpu_a_SOURCES += score/src/schedulerdefaultpinunpin.c
> diff --git a/cpukit/score/src/percpujobs.c b/cpukit/score/src/percpujobs.c
> new file mode 100644
> index 00..4ce96dc738
> --- /dev/null
> +++ b/cpukit/score/src/percpujobs.c
> @@ -0,0 +1,124 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +
> +/**
> + * @file
> + *
> + * @ingroup RTEMSScorePerCPU
> + *
> + * @brief This source file contains the implementation of _Per_CPU_Add_job(),
> + *   _Per_CPU_Perform_jobs(), and _Per_CPU_Wait_for_job().
> + */
> +
> +/*
> + * Copyright (C) 2019 embedded brains GmbH
> + *
> + * 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 OWNER 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.
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include "config.h"
> +#endif
> +
> +#include 
> +#include 
> +
> +#define _Per_CPU_Jobs_ISR_disable_and_acquire( cpu, lock_context ) \
> +  _ISR_lock_ISR_disable_and_acquire( &( cpu )->Jobs.Lock, lock_context )
> +
> +#define _Per_CPU_Jobs_release_and_ISR_enable( cpu, lock_context ) \
> +  _ISR_lock_Release_and_ISR_enable( &( cpu )->Jobs.Lock, lock_context )
> +
> +void _Per_CPU_Perform_jobs( Per_CPU_Control *cpu )
> +{
> +  ISR_lock_Context  lock_context;
> +  Per_CPU_Job  *job;
> +
> +  _Per_CPU_Jobs_ISR_disable_and_acquire( cpu, _context );
> +  job = cpu->Jobs.head;
> +  cpu->Jobs.head = NULL;
> +  _Per_CPU_Jobs_release_and_ISR_enable( cpu, _context );
> +
> +  while ( job != NULL ) {
> +const Per_CPU_Job_context *context;
> +Per_CPU_Job   *next;
> +
> +context = job->context;
> +next = job->next;
> +( *context->handler )( context->arg );
> +_Atomic_Store_ulong( >done, PER_CPU_JOB_DONE, ATOMIC_ORDER_RELEASE 
> );
> +
> +job = next;
> +  }
> +}
> +
> +void _Per_CPU_Add_job( Per_CPU_Control *cpu, Per_CPU_Job *job )
> +{
> +  ISR_lock_Context lock_context;
> +
> +  _Atomic_Store_ulong( >done, 0, ATOMIC_ORDER_RELAXED );
> +  _Assert( job->next == NULL );

Given the dispatch does not check the handler is adding ...

 _Assert( job->context->handler != NULL );

worth doing ?

> +
> +  _Per_CPU_Jobs_ISR_disable_and_acquire( cpu, _context );
> +
> +  if ( cpu->Jobs.head == NULL ) {
> +cpu->Jobs.head = job;
> +  } else {
> +*cpu->Jobs.tail = job;
> +  }
> +
> +  cpu->Jobs.tail = >next;
> +
> +  _Per_CPU_Jobs_release_and_ISR_enable( cpu, _context );
> +}
> +
> +void _Per_CPU_Wait_for_job(
> +  const Per_CPU_Control *cpu,
> +  const Per_CPU_Job *job
> +)
> +{
> +  while (
> +_Atomic_Load_ulong( >done, ATOMIC_ORDER_ACQUIRE )
> +  != PER_CPU_JOB_DONE
> +  ) {
> +Per_CPU_Control *cpu_self;
> +
> +switch ( _Per_CPU_Get_state( cpu ) ) {
> +  case PER_CPU_STATE_INITIAL:
> +  case PER_CPU_STATE_READY_TO_START_MULTITASKING:
> +  case PER_CPU_STATE_UP:
> +/*
> + * Calling this function with the current processor is intentional.  
> We
> + * have 

Re: [PATCH rtems-libbsd v2 5/5] create-kernel-namespace for USB Template driver

2021-07-28 Thread Chris Johns
On 28/7/21 9:56 pm, Husni Faiz wrote:
> Signed-off-by: Husni Faiz 
> ---
>  rtemsbsd/include/machine/rtems-bsd-kernel-namespace.h | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/rtemsbsd/include/machine/rtems-bsd-kernel-namespace.h 
> b/rtemsbsd/include/machine/rtems-bsd-kernel-namespace.h
> index 97cdb625..ae56ad9c 100644
> --- a/rtemsbsd/include/machine/rtems-bsd-kernel-namespace.h
> +++ b/rtemsbsd/include/machine/rtems-bsd-kernel-namespace.h
> @@ -5279,6 +5279,7 @@
>  #define  usbd_do_request_proc _bsd_usbd_do_request_proc
>  #define  usbd_dummy_timeout _bsd_usbd_dummy_timeout
>  #define  usb_debug _bsd_usb_debug
> +#define  usb_decode_str_desc _bsd_usb_decode_str_desc
>  #define  usbd_enum_is_locked _bsd_usbd_enum_is_locked
>  #define  usbd_enum_lock _bsd_usbd_enum_lock
>  #define  usbd_enum_lock_sig _bsd_usbd_enum_lock_sig
> @@ -5515,8 +5516,12 @@
>  #define  usb_suspend_resume _bsd_usb_suspend_resume
>  #define  usb_temp_get_desc_p _bsd_usb_temp_get_desc_p
>  #define  usb_template _bsd_usb_template
> +#define  usb_template_cdce _bsd_usb_template_cdce
> +#define  usb_temp_setup _bsd_usb_temp_setup
>  #define  usb_temp_setup_by_index_p _bsd_usb_temp_setup_by_index_p
> +#define  usb_temp_sysctl _bsd_usb_temp_sysctl
>  #define  usb_temp_unload _bsd_usb_temp_unload
> +#define  usb_temp_unsetup _bsd_usb_temp_unsetup
>  #define  usb_temp_unsetup_p _bsd_usb_temp_unsetup_p
>  #define  usb_test_quirk _bsd_usb_test_quirk
>  #define  usb_test_quirk_p _bsd_usb_test_quirk_p

How were these additions done?

I ask because I see these externs in the template code ...

extern struct usb_temp_device_desc usb_template_audio;
extern struct usb_temp_device_desc usb_template_cdce;
extern struct usb_temp_device_desc usb_template_kbd;
extern struct usb_temp_device_desc usb_template_modem;
extern struct usb_temp_device_desc usb_template_mouse;
extern struct usb_temp_device_desc usb_template_msc;
extern struct usb_temp_device_desc usb_template_mtp;
extern struct usb_temp_device_desc usb_template_phone;
extern struct usb_temp_device_desc usb_template_serialnet;
extern struct usb_temp_device_desc usb_template_midi;
extern struct usb_temp_device_desc usb_template_multi;
extern struct usb_temp_device_desc usb_template_cdceem;

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


Re: [PATCH rtems-libbsd v2 4/5] libbsd.py: add usb template files

2021-07-28 Thread Chris Johns
On 28/7/21 9:56 pm, Husni Faiz wrote:
> add the imported template files for CDC Ethernet
> register the dev_usb_template module
> enable usb template in default buildset
> 
> Signed-off-by: Husni Faiz 
> ---
>  buildset/default.ini |  1 +
>  buildset/minimal.ini |  3 ++-
>  libbsd.py| 25 +
>  3 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/buildset/default.ini b/buildset/default.ini
> index 472c535d..117bacc3 100644
> --- a/buildset/default.ini
> +++ b/buildset/default.ini
> @@ -34,6 +34,7 @@ dev_usb_net = on
>  dev_usb_quirk = on
>  dev_usb_serial = on
>  dev_usb_storage = on
> +dev_usb_template = on
>  dev_usb_wlan = off
>  dev_wlan_rtwn = off
>  iic = on
> diff --git a/buildset/minimal.ini b/buildset/minimal.ini
> index 03dd96bb..060b1930 100644
> --- a/buildset/minimal.ini
> +++ b/buildset/minimal.ini
> @@ -19,5 +19,6 @@ extends = default.ini
>  
>  [modules]
>  crypto_openssl = off
> +dev_usb_template = off
>  netinet6 = off
> -usr_bin_openssl = off
> +usr_bin_openssl = off
> \ No newline at end of file

What happened here?

Chris

> diff --git a/libbsd.py b/libbsd.py
> index c9151901..049ec7f8 100644
> --- a/libbsd.py
> +++ b/libbsd.py
> @@ -1162,6 +1162,30 @@ class dev_usb_storage(builder.Module):
>  mm.generator['source']()
>  )
>  
> +#
> +# USB Template
> +#
> +class dev_usb_template(builder.Module):
> +
> +def __init__(self, manager):
> +super(dev_usb_template, self).__init__(manager, type(self).__name__)
> +
> +def generate(self):
> +mm = self.manager
> +self.addDependency('dev_usb')
> +self.addKernelSpaceHeaderFiles(
> +[
> +'sys/dev/usb/template/usb_template.h',
> +]
> +)
> +self.addKernelSpaceSourceFiles(
> +[
> +'sys/dev/usb/template/usb_template.c',
> +'sys/dev/usb/template/usb_template_cdce.c',
> +],
> +mm.generator['source']()
> +)
> +
>  #
>  # BBB USB
>  #
> @@ -5480,6 +5504,7 @@ def load(mm):
>  
>  mm.addModule(cam(mm))
>  mm.addModule(dev_usb_storage(mm))
> +mm.addModule(dev_usb_template(mm))
>  mm.addModule(dev_usb_controller_bbb(mm))
>  
>  mm.addModule(net(mm))
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: [PATCH rtems-libbsd v2 2/5] usb_template: Configure template driver only for CDC Ethernet

2021-07-28 Thread Chris Johns
On 29/7/21 1:10 am, Ahamed Husni wrote:
> 
> 
> On Wed, 28 Jul 2021, 17:27 Husni Faiz,  > wrote:
> 
> Add the conditional macro to prevent the driver from referencing
> the templates which are not imported yet.
> 
> Include functions which adds the hw.usb.template sysctl variable.
> 
> Signed-off-by: Husni Faiz  >
> ---
>  freebsd/sys/dev/usb/template/usb_template.c | 4 
>  freebsd/sys/dev/usb/usb_device.c            | 8 ++--
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/freebsd/sys/dev/usb/template/usb_template.c
> b/freebsd/sys/dev/usb/template/usb_template.c
> index 0650da15..76567e85 100644
> --- a/freebsd/sys/dev/usb/template/usb_template.c
> +++ b/freebsd/sys/dev/usb/template/usb_template.c
> @@ -1433,12 +1433,15 @@ usb_temp_setup_by_index(struct usb_device *udev,
> uint16_t index)
>         usb_error_t err;
> 
>         switch (index) {
> +#ifndef __rtems__
>         case USB_TEMP_MSC:
>                 err = usb_temp_setup(udev, _template_msc);
>                 break;
> +#endif /* __rtems__ */
>         case USB_TEMP_CDCE:
>                 err = usb_temp_setup(udev, _template_cdce);
>                 break;
> +#ifndef __rtems__
>         case USB_TEMP_MTP:
>                 err = usb_temp_setup(udev, _template_mtp);
>                 break;
> @@ -1469,6 +1472,7 @@ usb_temp_setup_by_index(struct usb_device *udev,
> uint16_t index)
>         case USB_TEMP_CDCEEM:
>                 err = usb_temp_setup(udev, _template_cdceem);
>                 break;
> +#endif /* __rtems__ */
>         default:
>                 return (USB_ERR_INVAL);
>         }
> diff --git a/freebsd/sys/dev/usb/usb_device.c 
> b/freebsd/sys/dev/usb/usb_device.c
> index ee240949..619cae5a 100644
> --- a/freebsd/sys/dev/usb/usb_device.c
> +++ b/freebsd/sys/dev/usb/usb_device.c
> @@ -28,6 +28,10 @@
>   * SUCH DAMAGE.
>   */
> 
> +#ifdef __rtems__
> +#include 
> +#endif
> +
>  #ifdef USB_GLOBAL_INCLUDE_FILE
>  #include USB_GLOBAL_INCLUDE_FILE
>  #else
> @@ -123,7 +127,7 @@ int usb_template = USB_TEMPLATE;
>  int    usb_template = -1;
>  #endif
> 
> -#ifndef __rtems__
> +#if !defined(__rtems__) || defined(RTEMS_BSD_MODULE_DEV_USB_TEMPLATE)

Is doing this supported by the freebsd-to-rtems.py script?

Why is the RTEMS_BSD_MODULE_DEV_USB_TEMPLATE needed?

>  SYSCTL_PROC(_hw_usb, OID_AUTO, template,
>      CTLTYPE_INT | CTLFLAG_RWTUN | CTLFLAG_MPSAFE,
>      NULL, 0, sysctl_hw_usb_template,
> @@ -255,7 +259,7 @@ sysctl_hw_usb_template(SYSCTL_HANDLER_ARGS)
> 
>         return (0);
>  }
> -#endif /* __rtems__ */
> +#endif /* !__rtmes__ || RTEMS_BSD_MODULE_DEV_USB_TEMPLATE */
> 
> Ah, a typo here (rtmes). Should I send a new set of patches?

Yes it needs to be correct for the script to work but I would check the
freebsd-to-rtems.py script works with these guard comments.

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

Re: [PATCH rtems-libbsd v3 1/4] Implement portable kernel symbol namespace tool

2021-07-28 Thread Chris Johns
On 29/7/21 12:27 am, Gedare Bloom wrote:
> On Wed, Jul 28, 2021 at 12:50 AM Chris Johns  wrote:
>>
>> On 28/7/21 4:38 pm, Sebastian Huber wrote:
>>> On 28/07/2021 04:53, chr...@rtems.org wrote:
>>>> +for sym in self.output['symbols']:
>>>> +out += ['#define %s_bsd_%s' % (sym, sym)]
>>>
>>> If you add a tab after the #define, then the output is in line with the 
>>> FreeBSD
>>> style and you have a lot less changes in the generated file compared to the
>>> previous script.
>>
>> You have lost me on the FreeBSD style so I am moving on.
>>
> I think the suggestion is to use
> out += ['#define\t%s_bsd_%s' % (sym, sym)]

Yes. I was not aware there was a coding standard at work in rtemsbsd. I add this
to the list.

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


Re: [PATCH rtems-libbsd 3/7] sys/kern: Add lockmgr support

2021-07-28 Thread Chris Johns
On 29/7/21 9:42 am, Gedare Bloom wrote:
> On Wed, Jul 28, 2021 at 5:05 PM Chris Johns  wrote:
>> On 29/7/21 12:03 am, Gedare Bloom wrote:
>>> On Tue, Jul 27, 2021 at 2:59 AM  wrote:
>>>> From: Chris Johns 
>>>> +static int
>>>> +lockmgr_upgrade(struct lock *lk, u_int flags, struct lock_object *ilk,
>>>> +   const char *file, int line)
>>>> +{
>>>> +  uintptr_t x, v;
>>>> +  int error = 0;
>>>> +  LOCK_LOG_LOCK("XUPGRADE", >lock_object, 0, 0, file, line);
>>>> +  lockmgr_trace("xupgrade", 'I', lk);
>>>> +  v = lk->lk_lock;
>>>> +  x = v & ~LK_SHARE;
>>>> +  atomic_store_rel_ptr(>lk_lock, x);
>>>
>>> I think this function is to upgrade a shared (waiters/spinner) lock to
>>> non-shared (spinner) lock? I'm not confident about correctness here.
>>> It looks to me like there can be a race condition if multiple waiters
>>> attempt to upgrade in parallel. It works by a preemption after the
>>> first waiter loads `v = lk->lk_lock` and then another waiter loads
>>> `v`. Actually, there doesn't seem to be anything that prevents two
>>> separate calls to upgrade the lock by different waiters in sequence
>>> either. I could be wrong (of course).
>>
>> We do not support shared locks and we do not share specific paths through 
>> locks
>> so while the code may request a shared lock it is not shared.
>>
> ok that is helpful.
> 
>> You could be correct and I welcome you reviewing the code in the FB kernel 
>> this
>> is based on.
>>
> I took a quick look.

Thanks

> the FB kernel uses spinning and
> atomic_fcmpset_ptr instead, which I think exactly addresses this
> problem.

Is this something we should do or is this something we should not be worried 
about?

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


Re: [PATCH rtems-libbsd 3/7] sys/kern: Add lockmgr support

2021-07-28 Thread Chris Johns
On 29/7/21 12:03 am, Gedare Bloom wrote:
> On Tue, Jul 27, 2021 at 2:59 AM  wrote:
>> From: Chris Johns 
>>
>>  struct lock_object {
>> -#ifndef __rtems__
>> const   char *lo_name;  /* Individual lock name. */
>> u_int   lo_flags;
>> u_int   lo_data;/* General class specific data. */
>> +#ifndef __rtems__
>> struct  witness *lo_witness;/* Data for witness. */
>> -#else /* __rtems__ */
>> -   unsigned int lo_flags;
>> +#endif /* __rtems__ */
>> +#ifdef __rtems__
> Can use
> #else /* __rtems__ */
> instead of endif+ifdef

I will see how this ends up after looking at the fields Sebastian wants removed.

>> +static void
>> +lock_lockmgr(struct lock_object *lock, uintptr_t how)
>> +{
>> +  panic("lockmgr locks do not support sleep interlocking");
>> +}
>> +
>> +static uintptr_t
>> +unlock_lockmgr(struct lock_object *lock)
>> +{
>> +  panic("lockmgr locks do not support sleep interlocking");
>> +}
>> +
> Should there be different messages for the two cases here?

Sure. Either case means something is being used by new ported code and these
calls need to be looked into.

> 
>> +static struct thread *
>> +lockmgr_xholder(const struct lock *lk)
>> +{
>> +  uintptr_t x;
>> +  x = lk->lk_lock;
>> +  if ((x & LK_SHARE))
>> +return NULL;
>> +  return rtems_bsd_get_thread(lk->lock_object.lo_mtx.queue.Queue.owner);
>> +}
>> +
>> +static void
>> +lockmgr_exit(u_int flags, struct lock_object *ilk, int wakeup_swapper)
> any good reason to use ilk instead of lk as elsewhere?

None. I copy the decl from FreeBSD ...

freebsd/sys/kern/kern_lock.c:lockmgr_exit(u_int flags, struct lock_object *ilk,
int wakeup_swapper)

>> +static int
>> +lockmgr_slock_hard(struct lock *lk, u_int flags, struct lock_object *ilk,
>> +  const char *file, int line)
>> +{
>> +  uintptr_t x;
>> +  int error = 0;
>> +  lockmgr_trace("slock", 'I', lk);
>> +  rtems_bsd_mutex_lock(>lock_object);
>> +  x = lk->lk_lock;
>> +  atomic_store_rel_ptr(>lk_lock, x + LK_ONE_SHARER);
>> +  if (rtems_bsd_mutex_recursed(>lock_object))
>> +lk->lk_recurse++;
>> +  else
>> +lk->lk_recurse = 0;
>> +  lockmgr_trace("slock", 'O', lk);
>> +  LOCK_LOG_LOCK("SLOCK", >lock_object, 0, lk->lk_recurse, file, line);
>> +  lockmgr_exit(flags, ilk, 0);
>> +  return error;
> this is always 0 (successful)?

Yeap.

>> +static int
>> +lockmgr_upgrade(struct lock *lk, u_int flags, struct lock_object *ilk,
>> +   const char *file, int line)
>> +{
>> +  uintptr_t x, v;
>> +  int error = 0;
>> +  LOCK_LOG_LOCK("XUPGRADE", >lock_object, 0, 0, file, line);
>> +  lockmgr_trace("xupgrade", 'I', lk);
>> +  v = lk->lk_lock;
>> +  x = v & ~LK_SHARE;
>> +  atomic_store_rel_ptr(>lk_lock, x);
> 
> I think this function is to upgrade a shared (waiters/spinner) lock to
> non-shared (spinner) lock? I'm not confident about correctness here.
> It looks to me like there can be a race condition if multiple waiters
> attempt to upgrade in parallel. It works by a preemption after the
> first waiter loads `v = lk->lk_lock` and then another waiter loads
> `v`. Actually, there doesn't seem to be anything that prevents two
> separate calls to upgrade the lock by different waiters in sequence
> either. I could be wrong (of course).

We do not support shared locks and we do not share specific paths through locks
so while the code may request a shared lock it is not shared.

You could be correct and I welcome you reviewing the code in the FB kernel this
is based on.

>> +int
>> +lockstatus(const struct lock *lk)
>> +{
>> +  uintptr_t v, x;
>> +  int ret;
>> +
>> +  ret = LK_SHARED;
>> +  x = lk->lk_lock;
>> +
>> +  if ((x & LK_SHARE) == 0) {
>> +v = rtems_bsd_mutex_owned(RTEMS_DECONST(struct lock_object*, 
>> >lock_object));
>> +if (v)
>> +  ret = LK_EXCLUSIVE;
>> +else
>> +  ret = LK_EXCLOTHER;
>> +  } else if (x == LK_UNLOCKED) {
>> +ret = 0;
>> +  }
> 
> there's no else {} here, is it possible that (x & LK_SHARE) != 0, and
> (x != LK_UNLOCKED)? If so, ret may be returned without initialization?
> 
> Maybe just initialize ret = 0.

Yes and thanks.

Chris

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


Re: [PATCH rtems-libbsd 1/7] rtemsbsd: Catch timeout overflows

2021-07-28 Thread Chris Johns
On 28/7/21 4:44 pm, Sebastian Huber wrote:
> On 28/07/2021 08:43, Chris Johns wrote:
>>> Why don't we use the FreeBSD implementation one-to-one:
>>>
>>> freebsd-org/sys/kern/kern_clock.c
>>>
>> I am fixing the code that exists. I have no idea why kern_clock.c was not 
>> ported
>> in the first place. I will not be porting the clock code at this point in 
>> time,
>> that is out of scope for my current work.
> 
> What I meant was to copy the tvtohz() from kern_clock.c to this file.

Ah ok, sure I can take a look.

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


Re: [PATCH rtems-libbsd 5/7] kern: Add a proc0

2021-07-28 Thread Chris Johns
On 29/7/21 12:22 am, Gedare Bloom wrote:
> On Tue, Jul 27, 2021 at 2:59 AM  wrote:
>> From: Chris Johns 
>> @@ -135,6 +171,8 @@ rtems_bsd_initialize(void)
>> sbt_tickthreshold = bttosbt(bt_tickthreshold);
>> maxid_maxcpus = (int) rtems_scheduler_get_processor_maximum();
>>
>> +   maxproc = 16;
> Why not use 1? Is this the smallest value that can be put here?

It is used here:

freebsd/sys/kern/kern_resource.c:   uihashtbl = hashinit(maxproc / 16,
M_UIDINFO, );

so anything less than 16 crashes. One of those things where you cater for all
cases or you work around how it is implemented and keep as much code as possible
as close to upstream as possible.

>> @@ -331,6 +336,8 @@ kthread_add(void (*func)(void *), void *arg, struct proc 
>> *p, struct thread **new
>> va_list ap;
>>
>> va_start(ap, fmt);
>> +   /* the cast here is a hack but passing a proc as a thread struct is 
>> just wrong and I
>> +* have no idea why it is like this */
> Does this point need discussion?

I do not know, I only added the comment.

Chris

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


Re: [PATCH rtems-libbsd 3/7] sys/kern: Add lockmgr support

2021-07-28 Thread Chris Johns
On 28/7/21 4:23 pm, Sebastian Huber wrote:
> On 27/07/2021 10:58, chr...@rtems.org wrote:
>> From: Chris Johns 
>>
>> - See `man lockmgr`
>>
>> - Implement the lock_object and move the RTEMS mutex to that object
>>
>> - Add debug support to track the locks with gdb
>>
>> Update #4475
>> ---
>>   freebsd/sys/sys/_lock.h   |  10 +-
>>   freebsd/sys/sys/_lockmgr.h    |   6 +
>>   freebsd/sys/sys/_mutex.h  |   5 -
>>   freebsd/sys/sys/_rwlock.h |   5 -
>>   freebsd/sys/sys/_sx.h |   5 -
>>   rtemsbsd/include/machine/_kernel_lock.h   |   2 +-
>>   rtemsbsd/include/machine/rtems-bsd-mutex.h    |   8 +-
>>   .../include/machine/rtems-bsd-muteximpl.h |  87 ++-
>>   rtemsbsd/include/machine/rtems-bsd-thread.h   |   1 +
>>   rtemsbsd/rtems/rtems-kernel-epoch.c   |   2 +-
>>   rtemsbsd/rtems/rtems-kernel-jail.c    |  10 +-
>>   rtemsbsd/rtems/rtems-kernel-lockmgr.c | 578 ++
>>   rtemsbsd/rtems/rtems-kernel-mutex.c   |  21 +-
>>   rtemsbsd/rtems/rtems-kernel-muteximpl.c   |   5 +-
>>   rtemsbsd/rtems/rtems-kernel-rwlock.c  |  28 +-
>>   rtemsbsd/rtems/rtems-kernel-sx.c  |  34 +-
>>   16 files changed, 724 insertions(+), 83 deletions(-)
>>   create mode 100644 rtemsbsd/rtems/rtems-kernel-lockmgr.c
>>
>> diff --git a/freebsd/sys/sys/_lock.h b/freebsd/sys/sys/_lock.h
>> index ae10254c..9e3388d5 100644
>> --- a/freebsd/sys/sys/_lock.h
>> +++ b/freebsd/sys/sys/_lock.h
>> @@ -32,15 +32,19 @@
>>     #ifndef _SYS__LOCK_H_
>>   #define    _SYS__LOCK_H_
>> +#ifdef __rtems__
>> +#include 
>> +#endif /* __rtems__ */
>>     struct lock_object {
>> -#ifndef __rtems__
>>   const    char *lo_name;    /* Individual lock name. */
>>   u_int    lo_flags;
>>   u_int    lo_data;    /* General class specific data. */
>> +#ifndef __rtems__
>>   struct    witness *lo_witness;    /* Data for witness. */
>> -#else /* __rtems__ */
>> -    unsigned int lo_flags;
>> +#endif /* __rtems__ */
>> +#ifdef __rtems__
>> +    rtems_bsd_mutex lo_mtx;
>>   #endif /* __rtems__ */
>>   };
> 
> This change increases the size by 40% from 20 bytes to 28 bytes. 

Thanks for pointing this out. I suspect I added them back early and have not
looked at it again.

> Why do we need the lo_name? 

I suspect this could be conditional. It is useful when debugging the locks so
being able to get the name in the trace is a good thing.

> The thread queues have already a name. 

Does this mean referencing the internals of the RTEMS mutex to get the name?

The references to this field are only a few and in rtemsbsd so it looks like
this could change.

> For what do we need lo_data?

This field is used in various places for different type of locks so it needs to
stay.

> 
> [...]
>> diff --git a/rtemsbsd/include/machine/rtems-bsd-muteximpl.h
>> b/rtemsbsd/include/machine/rtems-bsd-muteximpl.h
>> index b362e524..2e180b97 100644
>> --- a/rtemsbsd/include/machine/rtems-bsd-muteximpl.h
>> +++ b/rtemsbsd/include/machine/rtems-bsd-muteximpl.h
>> @@ -50,6 +50,8 @@
>>     #include 
>>   +#include 
>> +#include 
>>   #include 
>>   #include 
>>   @@ -60,17 +62,48 @@ extern "C" {
>>   #define    RTEMS_BSD_MUTEX_TQ_OPERATIONS \
>>   &_Thread_queue_Operations_priority_inherit
>>   +#if RTEMS_DEBUG
>> +/*
>> + * Resource tracking. In GDB you can:
>> + *
>> + * define mutex-owned
>> + *  set $m = $arg0
>> + *  set $c = 0
>> + *  while $m != 0
>> + *   set $c = $c + 1
>> + *   if $m->queue.Queue.owner != 0
>> + *    printf "%08x %-40s\n", $m->queue.Queue.owner, $m->queue.Queue.name
>> + *   end
>> + *   set $m = $m->mutex_list.tqe_next
>> + *  end
>> + *  printf "Total: %d\n", $c
>> + * end
>> + *
>> + * (gdb) mutex-owned _bsd_bsd_mutexlist->tqh_first
>> + */
>> +extern TAILQ_HEAD(bsd_mutex_list, rtems_bsd_mutex) bsd_mutexlist;
>> +extern rtems_mutex bsd_mutexlist_lock;
>> +#endif /* RTEMS_DEBUG */
> 
> Global symbols should be prefixed with rtems_bsd or _bsd, but not bsd.
> 
> [...]
>> diff --git a/rtemsbsd/rtems/rtems-kernel-lockmgr.c
>> b/rtemsbsd/rtems/rtems-kernel-lockmgr.c
>> new file mode 100644
>> index ..36e7a82f
>> --- /dev/null
>> +++ b/rtemsbsd/rtems/rtems-kernel-lockmgr.c
>> @@ -0,0 +1,578 @@
>> +/**
>> 

Re: [PATCH rtems-libbsd 7/7] kern: Add kernel trace support (KTR)

2021-07-28 Thread Chris Johns
On 28/7/21 4:57 pm, Sebastian Huber wrote:
> On 28/07/2021 08:53, Chris Johns wrote:
>> On 28/7/21 4:27 pm, Sebastian Huber wrote:
>>> On 27/07/2021 10:58,chr...@rtems.org  wrote:
>>>> diff --git a/rtemsbsd/rtems/rtems-kernel-thread.c
>>>> b/rtemsbsd/rtems/rtems-kernel-thread.c
>>>> index 3e1e44b9..49ec6df7 100644
>>>> --- a/rtemsbsd/rtems/rtems-kernel-thread.c
>>>> +++ b/rtemsbsd/rtems/rtems-kernel-thread.c
>>>> @@ -260,6 +260,9 @@ rtems_bsd_thread_start(struct thread **td_ptr, void
>>>> (*func)(void *), void *arg,
>>>>    BSD_ASSERT(td != NULL);
>>>>      _Thread_Set_name(thread, name);
>>>> +#ifdef KTR
>>>> +    strlcpy(td->td_name, name, sizeof(td->td_name));
>>>> +#endif
>>> Do we really need this copy and the td_name member?
>>>
>> I found it useful with the KTR output to have the thread. KTR is a
>> developer/development aid and a good addition to libbsd.
> 
> We already have a name in the Thread_Control:
> 
> the_thread->Join_queue.Queue.name;
> 
> See also _Thread_Get_name().

The `td_name` field is only present when the KTR is defined. I simply reused the
decl present in `struct thread` rather than optimise what could be done and
possibly create further issues. If the name field was present all the time I
would agree a pointer to the TCB would be preferred but that adds a pointer to
the thread struct which is currently not there.

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

Re: [PATCH rtems-libbsd 7/7] kern: Add kernel trace support (KTR)

2021-07-28 Thread Chris Johns
On 28/7/21 4:27 pm, Sebastian Huber wrote:
> On 27/07/2021 10:58, chr...@rtems.org wrote:
>> diff --git a/rtemsbsd/rtems/rtems-kernel-thread.c
>> b/rtemsbsd/rtems/rtems-kernel-thread.c
>> index 3e1e44b9..49ec6df7 100644
>> --- a/rtemsbsd/rtems/rtems-kernel-thread.c
>> +++ b/rtemsbsd/rtems/rtems-kernel-thread.c
>> @@ -260,6 +260,9 @@ rtems_bsd_thread_start(struct thread **td_ptr, void
>> (*func)(void *), void *arg,
>>   BSD_ASSERT(td != NULL);
>>     _Thread_Set_name(thread, name);
>> +#ifdef KTR
>> +    strlcpy(td->td_name, name, sizeof(td->td_name));
>> +#endif
> 
> Do we really need this copy and the td_name member?
> 

I found it useful with the KTR output to have the thread. KTR is a
developer/development aid and a good addition to libbsd.

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

Re: [PATCH rtems-libbsd v3 1/4] Implement portable kernel symbol namespace tool

2021-07-28 Thread Chris Johns
On 28/7/21 4:38 pm, Sebastian Huber wrote:
> On 28/07/2021 04:53, chr...@rtems.org wrote:
>> +    for sym in self.output['symbols']:
>> +    out += ['#define %s_bsd_%s' % (sym, sym)]
> 
> If you add a tab after the #define, then the output is in line with the 
> FreeBSD
> style and you have a lot less changes in the generated file compared to the
> previous script.

You have lost me on the FreeBSD style so I am moving on.

In regards to diff'ing the file the sort order was something bespoke to Linux's
sort and so all the defines moved about anyway. I check all the symbols present
before are present in the generated file. I also checked the executables from
before and after regeneration matched.

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

Re: [PATCH rtems-libbsd 1/7] rtemsbsd: Catch timeout overflows

2021-07-28 Thread Chris Johns
On 28/7/21 4:03 pm, Sebastian Huber wrote:
> On 28/07/2021 00:53, Chris Johns wrote:
>> On 28/7/21 7:38 am, Gedare Bloom wrote:
>>> On Tue, Jul 27, 2021 at 2:59 AM  wrote:
>>>> From: Chris Johns
>>>>
>>>> Update #4475
>>> This change could probably use its own ticket.
>> Without the change the RPC client fails to connect in a reliable way because 
>> the
>> connection may time out for no real reason. A connection that is stable 
>> appears
>> fragile. I used this ticket because the change was needed for NFS to work.
>>
>> https://github.com/freebsd/freebsd-src/blob/main/sys/rpc/clnt_dg.c#L388
>>
>>>> ---
>>>>   rtemsbsd/rtems/rtems-kernel-timesupport.c | 8 +++-
>>>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/rtemsbsd/rtems/rtems-kernel-timesupport.c
>>>> b/rtemsbsd/rtems/rtems-kernel-timesupport.c
>>>> index ef14d1fa..5d290d66 100644
>>>> --- a/rtemsbsd/rtems/rtems-kernel-timesupport.c
>>>> +++ b/rtemsbsd/rtems/rtems-kernel-timesupport.c
>>>> @@ -35,6 +35,7 @@
>>>>
>>>>   #include 
>>>>
>>>> +#include 
>>>>   #include 
>>>>
>>>>   #include 
>>>> @@ -46,9 +47,14 @@ int
>>>>   tvtohz(struct timeval *tv)
>>>>   {
>>>>     struct timespec ts;
>>>> +  uint32_t ticks;
>>>>
>>>>     ts.tv_sec = tv->tv_sec;
>>>>     ts.tv_nsec = tv->tv_usec * 1000;
>>>>
>>>> -  return (int) _Timespec_To_ticks(  );
>>>> +  ticks = _Timespec_To_ticks(  );
>>>> +  if (ticks > INT_MAX)
>>>> +    ticks = INT_MAX;
>>>> +
>>> This changes the behavior to saturating in the overflow case, which is
>>> at least well-defined, but is it the best thing to do?  (I have no
>>> idea.)
>> I do not have a better suggestion? The user is providing a timeval that 
>> exceeds
>> the size of the return value so providing the max value at least the gets you
>> closer to the actual value than any other value?
> 
> Why don't we use the FreeBSD implementation one-to-one:
> 
> freebsd-org/sys/kern/kern_clock.c
> 

I am fixing the code that exists. I have no idea why kern_clock.c was not ported
in the first place. I will not be porting the clock code at this point in time,
that is out of scope for my current work.

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

Re: [PATCH rtems-libbsd v3 1/4] Implement portable kernel symbol namespace tool

2021-07-28 Thread Chris Johns
On 28/7/21 4:34 pm, Sebastian Huber wrote:
> On 28/07/2021 04:53, chr...@rtems.org wrote:
>> -#define    zy7_slcr_postload_pl _bsd_zy7_slcr_postload_pl
>> +#define AES_CBC_MAC_Final _bsd_AES_CBC_MAC_Final
> 
> The FreeBSD coding style has a tab after the #define. This is why it was 
> where.
> 

Does it make a difference given the file is generated and our code?

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

Re: [PATCH rtems-libbsd v3 2/4] waf: Fix clashing symbols in the user land symbols

2021-07-28 Thread Chris Johns
On 28/7/21 4:29 pm, Sebastian Huber wrote:
> On 28/07/2021 04:53, chr...@rtems.org wrote:
>> From: Chris Johns
>>
>> Update #4475
>> ---
>>   libbsd.py | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libbsd.py b/libbsd.py
>> index 09a1fbc4..2318e470 100644
>> --- a/libbsd.py
>> +++ b/libbsd.py
>> @@ -5019,7 +5019,7 @@ class dhcpcd(builder.Module):
>>   'dhcpcd/compat/pselect.c',
>>   'dhcpcd/crypt/hmac_md5.c',
>>   ],
>> -    mm.generator['source']('-D__FreeBSD__ -DTHERE_IS_NO_FORK
>> -DMASTER_ONLY -DINET')
>> +    mm.generator['source']('-D__FreeBSD__ -DTHERE_IS_NO_FORK
>> -DMASTER_ONLY -DINET -Dhmac_md5=_dhcpcd_hmac_md5')
> 
> Can't this be added to:
> 
> dhcpcd/namespace.h

I tried but it did not compile and I do not know the code or what can be done
about this.

I am sorry but I have no time left to look into what is wrong. For someone who
knows this code I am sure it can be fixed after this is pushed and this define
removed.

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

NSFv4: LibBSD VFS and FS

2021-07-27 Thread Chris Johns
Hello,

Ticket #4475 documents the addition of the FreeBSD NFS client to LibBSD.

https://devel.rtems.org/ticket/4475

The change is large because the NFS client uses FreeBSD's VFS and this in turn
requires LibBSD use FreeBSD's file descriptor and file support. Breaking this
patch down into section that supports bisect would be too much work.

Given the size I have pushed the changes to me personal repo:

https://git.rtems.org/chrisj/rtems-libbsd.git/log/?h=4475-vfs

All branches are presented here.

4475-regen-symbols
--

Posted as:
 https://lists.rtems.org/pipermail/devel/2021-July/068571.html

- This change adds a new kernel symbol generation tool that has a consistent
sort across all supported platforms. The script uses the sort command and
FreeBSD and Linux did not agree.

- The namespace kernel header is sorted and committed without addition.

- The namespace kernel header is updated with the everything builtset.

- Building the Versal uncovered clashing function names in WPA and dhcpcd code.

- The new kernel symbol tool exposed duplicate symbols in sys/netinet.

4475-lockmgr-proc0-ktr
--

Posted as:
 https://lists.rtems.org/pipermail/devel/2021-July/068544.html

- Prevent a timer overflow if the timeval is out of range for the platform int.
This change is needed to avoid an indeterminate NFS RPC client connection due to
an invalid timeout value.

-  Add lockmgr support to locks. The VFS and file system code uses the lockmgr
interface to control locks. The existing lock code was refactored to expose a
lock object that all types of locks use. If RTEMS is built with RTEMS_DEBUG the
locks are held on a list that can be walked with a debugger to aid debugging.

- A proc0 is added. This is the FreeBSD initial process from which all FreeBSD
processes are created from. RTEMS is a single process OS and proc0 is the only
process. The process is required to hold the file descriptors and files the
process has open. It also holds the credentials. The VFS and file system code
all depend on these being present and valid. Only defaults are supported. Adding
a proc0 cleans up a lot of the code that checks for the process a thread belongs
too.

- Kernel Trace or KTR is now supported. You can add a waf configure option to
turn KTR on. KTR is useful with the VFS and file systems.

4475-inmport-vfs-fs
---

The direct import of the FreeBSD code without changes.

4475-vfs


This is large patch that contains:

- VFS changes for libbsd
- File descriptor and file changes for libbsd with tracing
- LibIO integration changes with tracing
- System call (syscall) interface with tracing

See `waf --help` for the tracing options

File Descriptors and Files
==

LibIO file descriptors (fd) map to a FreeBSd fds and from that to a FreeBSD file
object or vnodes. The code around this is made more complicated because of the
of the limited fields available in a LibIO iop to hold the data needed. The
LibIO iop can hold the fd and/or vnode pointers for a vnode or a parent vnode.

The select call requires special treatment to handle the different bitmaps
because the user provided bitmap is based on the LibIO fds and not the FB fds.

The kqueue iop holds the knote data kqueue uses.

A lot of functionality has been moved into rtemsbsd and related sources.

Range locking is not supported.

VFS
===

The VFS support means the syscall interfaces can be centralised into rtembsd and
a number of places direct libio access was implemented have be reverted to the
FB code. For example sockets and pipes.

The VFS paging and and VM is not supported. The bio is direct to the devices.
The VFS does start its worker threads to handle flushing and the other related
activities it runs.

File System
---

The pseudo file system provides a VFS file we can use to create a root file
system that is mounted in the RTEMS file system. FreeBSD file systems are
mounted onto this root file system.

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


Re: [PATCH rtems-libbsd v2 1/4] Implement portable kernel symbol namespace tool

2021-07-27 Thread Chris Johns
Please ignore v2, I forgot to the rebase the fixes from the review. V3 will be
posted soon.

On 28/7/21 11:06 am, chr...@rtems.org wrote:
> From: Chris Johns 
> 
> - The script's use of sort proved to not be portable
> 
> - No need to check the commits as symbols are only added
> 
> - Regenerated kernel header to reset the sort order
> 
> Update #4475

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


Re: [PATCH rtems-libbsd.nfs 1/4] Implement portable kernel symbol namespace tool

2021-07-27 Thread Chris Johns
Thanks for the review ...

On 28/7/21 7:16 am, Gedare Bloom wrote:
> On Tue, Jul 27, 2021 at 2:09 AM  wrote:
>> +environment `PATH` variable or you can specify the top level path as an 
>> argument:
>> +```
>> +$ ./rtems-kern-symbols --tools=/opt/work/rtems/6
>> +```
> 
> This is inconsistent with other ecosystem support, use --rtems-tools instead?

Yes, changed for v2

Chris

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


Re: [PATCH 1/3] rsb: Store patches in separate directories

2021-07-27 Thread Chris Johns
On 28/7/21 12:13 am, Alex White wrote:
> On Tue, Jul 27, 2021 at 12:46 AM Chris Johns  wrote:
>>
>> On 27/7/21 3:39 pm, Sebastian Huber wrote:
>> > On 27/07/2021 05:30, Alex White wrote:
>> >> This prevents patches with the same name from conflicting and causing
>> >> the build to fail.
>> >
>> > Why can't the patches have a unique file name?
>>
>> Yeap and if specific to Xilinx and their repos it should be labelled in a 
>> manner
>> that highlights that. Further you cannot create a release without unique file
>> names because we have a single source directory 
>>
>> https://ftp.rtems.org/pub/rtems/releases/5/5.1/sources/
>>
>> And I see no reason to change this.
> 
> I just realized that even if I separate these patches by build name, the 
> problem
> would remain that patches for the same build item with the same name (however
> improbable they are) would conflict.
> 
> It looks like it would be better for me to create an RTEMS ticket to hold 
> these
> patches and rename them to work around this issue. I'll do that.

Alex,

Could you please explain what the issue is rather than presenting us with
solutions we then reject? I have no idea what the problem is you are attempting
to solve.

Please do not create a ticket until there is general agreement this is the best
solution.

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


Re: [PATCH rtems-libbsd 1/7] rtemsbsd: Catch timeout overflows

2021-07-27 Thread Chris Johns
On 28/7/21 7:38 am, Gedare Bloom wrote:
> On Tue, Jul 27, 2021 at 2:59 AM  wrote:
>>
>> From: Chris Johns 
>>
>> Update #4475
> This change could probably use its own ticket.

Without the change the RPC client fails to connect in a reliable way because the
connection may time out for no real reason. A connection that is stable appears
fragile. I used this ticket because the change was needed for NFS to work.

https://github.com/freebsd/freebsd-src/blob/main/sys/rpc/clnt_dg.c#L388

>> ---
>>  rtemsbsd/rtems/rtems-kernel-timesupport.c | 8 +++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/rtemsbsd/rtems/rtems-kernel-timesupport.c 
>> b/rtemsbsd/rtems/rtems-kernel-timesupport.c
>> index ef14d1fa..5d290d66 100644
>> --- a/rtemsbsd/rtems/rtems-kernel-timesupport.c
>> +++ b/rtemsbsd/rtems/rtems-kernel-timesupport.c
>> @@ -35,6 +35,7 @@
>>
>>  #include 
>>
>> +#include 
>>  #include 
>>
>>  #include 
>> @@ -46,9 +47,14 @@ int
>>  tvtohz(struct timeval *tv)
>>  {
>>struct timespec ts;
>> +  uint32_t ticks;
>>
>>ts.tv_sec = tv->tv_sec;
>>ts.tv_nsec = tv->tv_usec * 1000;
>>
>> -  return (int) _Timespec_To_ticks(  );
>> +  ticks = _Timespec_To_ticks(  );
>> +  if (ticks > INT_MAX)
>> +ticks = INT_MAX;
>> +
> This changes the behavior to saturating in the overflow case, which is
> at least well-defined, but is it the best thing to do?  (I have no
> idea.)

I do not have a better suggestion? The user is providing a timeval that exceeds
the size of the return value so providing the max value at least the gets you
closer to the actual value than any other value?

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


Re: [PATCH 1/3] rsb: Store patches in separate directories

2021-07-26 Thread Chris Johns
On 27/7/21 3:39 pm, Sebastian Huber wrote:
> On 27/07/2021 05:30, Alex White wrote:
>> This prevents patches with the same name from conflicting and causing
>> the build to fail.
> 
> Why can't the patches have a unique file name?

Yeap and if specific to Xilinx and their repos it should be labelled in a manner
that highlights that. Further you cannot create a release without unique file
names because we have a single source directory 

https://ftp.rtems.org/pub/rtems/releases/5/5.1/sources/

And I see no reason to change this.

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


Re: ttcp only on 6-freebsd-12 and not on master of libbsd

2021-07-26 Thread Chris Johns
Hi

Thanks for all this and digging into the 6-freebsd-12 and master branches.

I think it is clear from what has been stated and what I have experienced we
need to look at how we contributing to libbsd. I am struggling with a few things
that are creating issues for me. I am making large changes and for that I am
carefully checking things however I am finding when I check back at the
6-freebsd-12 branch there are issues.

1. I do not think the `everything.ini` module is being used to check patches.
This must happen.

2. Symbol namespace is not easy to maintain and things have been missed. The
script is specific to Linux and cannot be updated on FreeBSD. The sort command
is different and objdump does not work on archs other than the hosts.

I am pretty close to posting the first set of patches for 6-freebsd-12. This is
all the changes up to the NFSv4 file systems. It includes the major re-factor of
freebsd/sys/sys and freebsd/sys/kern. I am to look at master after.

I have written a new tool to manage the kernel namespace symbols with the hope
of making checking and updating easier on all hosts.

In relation to rtems-waf, the branches can be on different commits if it makes
sense. I will take a closer look at that as well.

Chris

On 25/7/21 7:52 am, Kinsey Moore wrote:
> Christian's handy script correctly identified that the CGEM patches for ZynqMP
> (and all related changes) have not gone into rtems-libbsd master. This is due 
> to
> those initial patches being primarily a backport of existing FreeBSD 13 
> patches
> with some modifications that rendered them very unlike the original patches 
> that
> went into FreeBSD 13. We had decided to hold off on merging the CGEM patches 
> to
> master and I think it was pending the NFSv4 work that Chris is doing. Most of
> the attendant patches to that work can be cherry-picked in with little to no
> effort, but the core patch is going to require a rewrite since much of what it
> required is already present in FreeBSD 13.
> 
> 
> Kinsey
> 
> On 7/24/2021 15:11, Christian Mauderer wrote:
>> Hello Joel,
>>
>> On 24/07/2021 22:02, Joel Sherrill wrote:
>>>
>>>
>>> On Sat, Jul 24, 2021, 1:55 PM Christian Mauderer >> > wrote:
>>>
>>>     Hello Joel,
>>>
>>>     I wrote a short script to find different commits based on author and
>>>     subject (see attached python-script). It seems that there are quite
>>>     some
>>>     differences (see attached output).
>>>
>>>
>>> Quick work!
>>>
>>
>> Yes, a quick and _dirty_ hacked together script ;-)
>>
>> Most likely that script is not fit for inclusion into our repos. It's 
>> python3,
>> it's using an external lib and I haven't take care that it matches a coding
>> style. The last one is a small problem.
>>
>>>
>>>     The big blog of HDMI on Beagle patches that is only on master is caused
>>>     by me. These have been developed by Vijay while I was a mentor for his
>>>     project. Would be a question whether that's a feature that should be on
>>>     the stable branch too or not. Some commits will be most likely only on
>>>     one branch (for example 816a2f912f where Chris updated rtems_waf).
>>>
>>>
>>> Anything 5 is stable. I would personally say it's ok to merge Vijay's work 
>>> to
>>> 6-freebsd-12.
>>
>> Sorry, I used the wrong term: For me the *-freebsd-12 is something that I
>> would recommend for projects that want something solid. So I would also see
>> 6-freebsd-12 as a mostly stable version (also it's under active development).
>>
>>> The biggest consideration for me is not negatively impacting Chris sorting
>>> out merging NFSv4 and all that comes along with that. But this should be
>>> independent.
>>
>> Good point. Maybe we should wait a bit. I think cherry-picking patches 
>> between
>> the branches can be done after that too.
>>
>> Best regards
>>
>> Christian
>>
>>>
>>> --joel
>>>
>>>
>>>     Best regards
>>>
>>>     Christian
>>>
>>>     On 24/07/2021 18:40, Joel Sherrill wrote:
>>>  >
>>>  >
>>>  > On Sat, Jul 24, 2021, 11:30 AM Christian Mauderer
>>>     mailto:o...@c-mauderer.de>
>>>  > >> wrote:
>>>  >
>>>  >     Hello Joel,
>>>  >
>>>  >     On 24/07/2021 18:18, Joel Sherrill wrote:
>>>  >      >
>>>  >      >
>>>  >      > On Sat, Jul 24, 2021, 10:19 AM Christian Mauderer
>>>  >     mailto:o...@c-mauderer.de>
>>>     >
>>>  >      > 
>>>     >>  >      >
>>>  >      >     Hello,
>>>  >      >
>>>  >      >     in a discussion with Husni I noted that the patches adding
>>>  >     ttcp are
>>>  >      >     only
>>>  >      >     on the 6-freebsd-12 branch of libbsd. Is there a
>>>     reason that
>>>  >     they are
>>>  >      >     not on master too?
>>>  

Re: [PATCH 1/3] rsb: Store patches in separate directories

2021-07-26 Thread Chris Johns
Hi Ryan,

Why is this needed?

Does this change effect all builds and does a user end up with copies of the
same patches?

Have you built a full release with this change? It will be required for me to
consider accepting any part of these patches.

Chris

On 27/7/21 1:30 pm, Alex White wrote:
> This prevents patches with the same name from conflicting and causing
> the build to fail.
> ---
>  source-builder/defaults.mc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/source-builder/defaults.mc b/source-builder/defaults.mc
> index 98775e8..d24d969 100644
> --- a/source-builder/defaults.mc
> +++ b/source-builder/defaults.mc
> @@ -101,7 +101,7 @@ _topdir: dir, required, '%{_cwd}'
>  _configdir:  dir, optional, 
> '%{_topdir}/config:%{_sbdir}/config:%{_sbtop}/bare/config'
>  _tardir: dir, optional, '%{_topdir}/tar'
>  _sourcedir:  dir, optional, '%{_topdir}/sources'
> -_patchdir:   dir, optional, 
> '%{_topdir}/patches:%{_sbdir}/patches'
> +_patchdir:   dir, optional, 
> '%{_topdir}/patches/%{buildname}:%{_sbdir}/patches'
>  _builddir:   dir, optional, '%{_topdir}/build/%{buildname}'
>  _buildcxcdir:dir, optional, '%{_topdir}/build/%{buildname}-cxc'
>  _buildxcdir: dir, optional, '%{_topdir}/build/%{buildname}-xc'
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


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


<    7   8   9   10   11   12   13   14   15   16   >