Re: [lwip 2/2] Move and rename sys_arch_data_sync_barier()
Hello Kinsey and Sebastian,
On Thursday 09 of March 2023 14:46:28 Kinsey Moore wrote:
> Normally with rtems-lwip I would complain that this doesn't follow the
> convention of using #ifdef __rtems__ to modify files from upstream sources
> (each root directory except rtemslwip has an upstream source), but the uLan
> authors have given permission to fully integrate this code into the
> rtemslwip directory where possible (the TI code will go into its own
> directory). This patch looks good.
driver has no upstream, it has been designed for RTEMS and then
used even on FreeRTOS and it except for headers from Ti mainly
used for MDIO, it has been designed from scratch at CTU.
We have originally started and tried to fix Ti HalCoGen
code to work with Ti provided FreeRTOS and it newer worked
reliably. Then we have thrown away all that stuff and started
from scratch with some inspiration from similar BSD drivers.
I have call with Premek and we both agree on relicense to RTEMS
BSD variant. I can prepare commit when I have some while (I am
loaded by lectures, students support now). Next step is to move
code to
rtemslwip/tms570
As for
+static inline void
+tms570_data_sync_barier(void)
+{
+#ifdef __arm__
+ _ARM_Data_synchronization_barrier();
+#endif
+}
it is OK but may it be not ideal, because more drivers could require
write buffers and instructions memory access order synchronization.
We have added that definition to ports/os/rtems/arch/sys_arch.h
#ifndef __rtems__
static inline sys_prot_t
sys_arch_protect(void)
{
sys_prot_t pval;
rtems_interrupt_disable(pval);
return pval;
}
static inline void
sys_arch_unprotect(sys_prot_t pval)
{
rtems_interrupt_enable(pval);
}
static inline void
sys_arch_data_sync_barier(void){
_ARM_Data_synchronization_barrier();
}
#else
as the generic solution when to allow same drivers to be used
with different architectures. There can be more drivers requiring
such protection.
In the fact tms570_emac is similar or equivalent to some other
ETHERNET NICs, I do not remember which one at the moment, I would
need to search. But I think that I have seen same/similar registers
organization on other targets.
If I prepare changes of license, should I sent patches
only to the mailing list or should I try to create branch
and try to push it to rtems-lwip or fork some GitHub
matching repo?
...
It seems that my SSH key for pp...@dispatch.rtems.org has
expired?
Best wishes,
Pavel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH 1/2] dev/flash: Add API for Flash devices
Some comments below. However, I think it will be best to first re-send
just the flashdev.h file so that we can iterate on the API before you
put too much more effort into implementing it.
On Wed, Mar 15, 2023 at 8:56 PM wrote:
>
> From: Aaron Nyholm
>
> Updates #4896
I don't see a ticket with this number.
> ---
> cpukit/dev/flash/flashdev.c | 353
> cpukit/include/dev/flash/flashdev.h | 95
> spec/build/cpukit/librtemscpu.yml | 4 +
> 3 files changed, 452 insertions(+)
> create mode 100644 cpukit/dev/flash/flashdev.c
> create mode 100644 cpukit/include/dev/flash/flashdev.h
>
> diff --git a/cpukit/dev/flash/flashdev.c b/cpukit/dev/flash/flashdev.c
> new file mode 100644
> index 00..a37d0e6c3d
> --- /dev/null
> +++ b/cpukit/dev/flash/flashdev.c
> @@ -0,0 +1,353 @@
> +/*
> + * Copyright (C) 2023, 2023 Aaron Nyholm
> + *
> + * 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
> +
> +
only 1 blank horizontal line at a time
> +#include
> +
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +
> +int flashdev_do_init(
> + flashdev *flash,
> + void (*destroy)(flashdev *flash)
> +);
> +
> +void flashdev_destroy_and_free(flashdev *flash);
> +
> +void flashdev_destroy(flashdev *flash);
> +
Global function declarations should be in header files. If these are
only needed here, declare static.
> +static void flashdev_obtain(flashdev *flash)
{
linebreak {
> + rtems_recursive_mutex_lock(>mutex);
> +}
> +
> +static void flashdev_release(flashdev *flash) {
> + rtems_recursive_mutex_unlock(>mutex);
> +}
> +
> +static ssize_t flashdev_read(
> + rtems_libio_t *iop,
> + void *buffer,
> + size_t count
> + )
delete leading spaces
> +{
> + flashdev *flash = IMFS_generic_get_context_by_iop(iop);
> +
> + // Check reading from valid region
no // comments
> + off_t new_offset = iop->offset + count;
> +
> + if (iop->data1 != NULL && new_offset >
> ((flashdev_region*)iop->data1)->length) {
> +rtems_set_errno_and_return_minus_one(EINVAL);
> + }
> +
> + // Read flash
> + off_t addr;
decls at start of function
> + if (iop->data1 == NULL) {
> +addr = iop->offset;
> + } else {
> +addr = (iop->offset + ((flashdev_region*)iop->data1)->base);
> + }
> + flashdev_obtain(flash);
> + int status = (*flash->read)(flash->driver, addr, count, buffer);
> + flashdev_release(flash);
> +
> + // Update offset and return
> + if (status == 0) {
> +iop->offset = new_offset;
> +return count;
> + } else {
> +rtems_set_errno_and_return_minus_one(-status);
why -status. error conditions of this driver framework are not defined.
> + }
> +}
> +
> +static ssize_t flashdev_write(
> + rtems_libio_t *iop,
> + const void *buffer,
> + size_t count
> + )
> +{
> + flashdev *flash = IMFS_generic_get_context_by_iop(iop);
> +
> + // Check writing to valid region
> + off_t new_offset = iop->offset + count;
> +
> + if (iop->data1 != NULL && new_offset >
> ((flashdev_region*)iop->data1)->length) {
> +rtems_set_errno_and_return_minus_one(EINVAL);
> + }
> +
> + // Write to flash
> + off_t addr;
> + if (iop->data1 == NULL) {
> +addr = iop->offset;
> + } else {
> +addr = (iop->offset + ((flashdev_region*)iop->data1)->base);
> + }
this code identical to above, consider a helper function anytime you
find yourself copy-pasting code.
> + flashdev_obtain(flash);
> + int status = (*flash->write)(flash->driver, addr, count, buffer);
> + flashdev_release(flash);
> +
> + // Update offset and return
> + if (status == 0) {
> +iop->offset = new_offset;
>
Re: Flash Device API
On Fri, Mar 17, 2023 at 4:24 PM Gedare Bloom wrote: > Thank you Aaron for contributing this. I have a few comments below, > and I'll also give some comments on your code shortly. > > On Fri, Mar 17, 2023 at 1:34 AM Sebastian Huber > wrote: > > > > > > > > On 16.03.23 23:07, Chris Johns wrote: > > > On 16/3/2023 6:13 pm, Sebastian Huber wrote: > > >> Hello Aaron, > > >> > > >> this API seems to be RTEMS-specific. Maybe we should simply pick up > an existing > > >> solution which is in more wide spread use, for example: > > >> > > >> https://docs.zephyrproject.org/latest/hardware/peripherals/flash.html > > > > > > That interface seems Zepher specific and looks to me like a series of > calls that > > > appear reasonable as a list to cover. > > > > The Zephyr API has drivers for a couple of chips: > > > > https://github.com/zephyrproject-rtos/zephyr/tree/main/drivers/flash > > > That code looks very zephyr-oriented. I think it would be a challenge > to go first toward porting this API, if it requires substantial > modifications of the upstream drivers to also use them, then it is > more of a maintenance burden than the probable benefits of the code > reuse. As a starting point, I do see a bespoke RTEMS implementation as > a suitable place to begin. But, it would be good to understand the > alternative flash driver frameworks that are out there, and what may > be pros/cons of adopting them. > > > > The patch Aaron has posted is a driver. I > > > prefer a driver like we have for I2C, SPI etc because the of support > it brings. > > > > The I2C and SPI APIs are ported from Linux, so not RTEMS-specific. > > > > > > > > The initial set of ioctl commands is small to start with. If you feel > we should > > > offer more I suggest they get added once this is merged. > > > > > > Is the change OK? > > > > I am not opposed to commit this API, however, I don't think it is a step > > forward. It is yet another RTEMS-specific API. It has no related > > documentation patches. The API has no high level user (for example > > JAFFS2). The API has no implementation. It has no tests. It has no > > driver framework (ONFI, JEDEC). It has no example driver. It has no > > Doxygen documentation. The coding style of the new file has nothing to > > do with the RTEMS coding style. > > > > I think the provided code appears to be a good starting point. I > would request that you might add Doxygen something aligned with what > we have for the i2c or spi would be good. > https://docs.rtems.org/doxygen/branches/master/group__I2C.html > > In addition to the shell command addition, can you create/extend a > testsuite for exercising the API? > > I will address the code directly also, but these high-level additions > will greatly improve the submission of a new API. Unfortunately, my > work on code formatting is not quite complete enough to rely on it for > fixing any style issues. I'll do what I can to guide you through that. > Aaron, Just in case you haven't already found it, the document you should be using for style and header documentation is here: https://docs.rtems.org/branches/master/eng/coding-conventions.html Kinsey ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: Flash Device API
Thank you Aaron for contributing this. I have a few comments below, and I'll also give some comments on your code shortly. On Fri, Mar 17, 2023 at 1:34 AM Sebastian Huber wrote: > > > > On 16.03.23 23:07, Chris Johns wrote: > > On 16/3/2023 6:13 pm, Sebastian Huber wrote: > >> Hello Aaron, > >> > >> this API seems to be RTEMS-specific. Maybe we should simply pick up an > >> existing > >> solution which is in more wide spread use, for example: > >> > >> https://docs.zephyrproject.org/latest/hardware/peripherals/flash.html > > > > That interface seems Zepher specific and looks to me like a series of calls > > that > > appear reasonable as a list to cover. > > The Zephyr API has drivers for a couple of chips: > > https://github.com/zephyrproject-rtos/zephyr/tree/main/drivers/flash > That code looks very zephyr-oriented. I think it would be a challenge to go first toward porting this API, if it requires substantial modifications of the upstream drivers to also use them, then it is more of a maintenance burden than the probable benefits of the code reuse. As a starting point, I do see a bespoke RTEMS implementation as a suitable place to begin. But, it would be good to understand the alternative flash driver frameworks that are out there, and what may be pros/cons of adopting them. > > The patch Aaron has posted is a driver. I > > prefer a driver like we have for I2C, SPI etc because the of support it > > brings. > > The I2C and SPI APIs are ported from Linux, so not RTEMS-specific. > > > > > The initial set of ioctl commands is small to start with. If you feel we > > should > > offer more I suggest they get added once this is merged. > > > > Is the change OK? > > I am not opposed to commit this API, however, I don't think it is a step > forward. It is yet another RTEMS-specific API. It has no related > documentation patches. The API has no high level user (for example > JAFFS2). The API has no implementation. It has no tests. It has no > driver framework (ONFI, JEDEC). It has no example driver. It has no > Doxygen documentation. The coding style of the new file has nothing to > do with the RTEMS coding style. > I think the provided code appears to be a good starting point. I would request that you might add Doxygen something aligned with what we have for the i2c or spi would be good. https://docs.rtems.org/doxygen/branches/master/group__I2C.html In addition to the shell command addition, can you create/extend a testsuite for exercising the API? I will address the code directly also, but these high-level additions will greatly improve the submission of a new API. Unfortunately, my work on code formatting is not quite complete enough to rely on it for fixing any style issues. I'll do what I can to guide you through that. Gedare ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH 1/3] bsps/riscv: Make SMP start more robust
Hello Alan, On 16.03.23 22:33, Alan Cudmore wrote: I applied these three patches after my patches and ran them on my K210 board and simulator. I have a set of 12 tests including benchmarks, SMP01, SMP08, ticker, etc. Everything ran OK. Is there anything in particular I can try to test them like setting the maximum CPUs to 1? (K210 is a dual core) the check for the configured processor maximum was already in the code, however, in C code. So without compiler optimization there was a risk that stack memory is still used. The other patches aim to improve the support for RISCV_BOOT_HARTID != 0. -- embedded brains GmbH Herr Sebastian HUBER Dornierstr. 4 82178 Puchheim Germany email: sebastian.hu...@embedded-brains.de phone: +49-89-18 94 741 - 16 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: [PATCH 1/3] bsps/riscv: Make SMP start more robust
> On Thu, 2023-03-16 at 09:59 +0100, Sebastian Huber wrote:
>
> In SMP configurations, check that we run on a configured
> processor. If not,
> then there is not much what can be done since we do not have a stack
> available
> for this processor. Just loop forever in this case. Do this in
> assemlby to
> ensure that no stack memory is used.
I have tested it on the Microchip PolarFire SoC Icicle Kit and working
fine.
Regards
Padmarao
> ---
> bsps/riscv/riscv/start/bspsmp.c | 5 +
> bsps/riscv/shared/start/start.S | 16 ++--
> 2 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/bsps/riscv/riscv/start/bspsmp.c
> b/bsps/riscv/riscv/start/bspsmp.c
> index 91f4f7b96a..ce5792f5b8 100644
> --- a/bsps/riscv/riscv/start/bspsmp.c
> +++ b/bsps/riscv/riscv/start/bspsmp.c
> @@ -36,10 +36,7 @@ void
> bsp_start_on_secondary_processor(Per_CPU_Control *cpu_self)
>
>cpu_index_self = _Per_CPU_Get_index(cpu_self);
>
> - if (
> -cpu_index_self < rtems_configuration_get_maximum_processors()
> - && _SMP_Should_start_processor(cpu_index_self)
> - ) {
> + if (_SMP_Should_start_processor(cpu_index_self)) {
> set_csr(mie, MIP_MSIP | MIP_MEIP);
> _SMP_Start_multitasking_on_secondary_processor(cpu_self);
>} else {
> diff --git a/bsps/riscv/shared/start/start.S
> b/bsps/riscv/shared/start/start.S
> index 34e1839ca1..42e4348cd0 100644
> --- a/bsps/riscv/shared/start/start.S
> +++ b/bsps/riscv/shared/start/start.S
> @@ -66,8 +66,17 @@ SYM(_start):
> LADDR sp, _ISR_Stack_area_begin
> LADDR t2, _ISR_Stack_size
> csrrs0, mhartid
> - li t3, RISCV_BOOT_HARTID
> - sub s0, s0, t3
> + li t3, RISCV_BOOT_HARTID
> + sub s0, s0, t3
> +
> + /*
> +* Check that this is a configured processor. If not, then
> there is
> +* not much what can be done since we do not have a stack
> available for
> +* this processor. Just loop forever in this case.
> +*/
> + LREGt3, _SMP_Processor_configured_maximum
> + bgeus0, t3, .Lwfi
> +
> LADDR t0, _Per_CPU_Information
> sllit1, s0, PER_CPU_CONTROL_SIZE_LOG2
> add s1, t0, t1
> @@ -100,6 +109,9 @@ SYM(_start):
> tailboot_card
>
> #ifdef RTEMS_SMP
> +.Lwfi:
> + wfi
> + j .Lwfi
>
> .Lstart_on_secondary_processor:
>
> --
> 2.35.3
>
> ___
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH v2 0/3] bsps/riscv: Add kendrytek210 riscv BSP variant
I have tested it on the renode.io simulator and working fine. Regards Padmarao > On Wed, 2023-03-15 at 22:04 -0400, Alan Cudmore wrote: > > Version 2 patch updates: Separated the device tree source and encoded > device tree blob into a separate patch, added the license text to > k210.h, eliminated whitespace warnings, and eliminated dead code in > the conditional compilation structure for the core_get_frequency > function in start/bspstart.c. > > This patch set adds the riscv/kendrytek210 BSP variant to support the > Kendryte K210 Dual Core RISC-V SoC. The BSP runs on the renode.io > simulator, the Sipeed MAiX BiT and MAiXDuino boards, and would likely > run on other boards. RTEMS binaries can be flashed to the boards > using > the kflash python utility available through the pip command. > Currently > the BSP supports the console UART which is shared with the > frdme310arty, > an interrupt controller, and timer. The included device tree source > just covers a minimal set of peripherals. The device tree can be > expanded as additional device support is addded. > Manufacturer, board links, and other information can be found in > ticket #4876. > > Documentation that describes how to build and run the BSP on the > boards and simulator has been prepared and will be submitted after > the bsp > is merged. > > The full testsuite has not been run on this BSP, but I run a > subset of the of testsuite on the renode.io robot test framework. > > Alan Cudmore (3): > bsps/riscv: add device tree source and device tree blob header for > k210 bsp variant > bsps/riscv: add riscv/kendrytek210 BSP variant source changes > spec: add riscv kendrytek210 variant build options > > bsps/riscv/riscv/config/kendrytek210.cfg | 9 + > bsps/riscv/riscv/console/console-config.c | 10 +- > bsps/riscv/riscv/console/fe310-uart.c | 2 +- > bsps/riscv/riscv/dts/kendryte-k210.dts| 216 > bsps/riscv/riscv/include/bsp.h| 4 + > bsps/riscv/riscv/include/bsp/k210.h | 105 ++ > .../riscv/include/bsp/kendryte-k210-dtb.h | 315 > ++ > bsps/riscv/riscv/include/bsp/riscv.h | 4 + > bsps/riscv/riscv/start/bspstart.c | 43 +++ > spec/build/bsps/optdtb.yml| 4 +- > spec/build/bsps/optdtbheaderpath.yml | 2 + > spec/build/bsps/optfdtuboot.yml | 3 + > spec/build/bsps/riscv/optramsize.yml | 2 + > spec/build/bsps/riscv/riscv/abi.yml | 1 + > .../bsps/riscv/riscv/bspkendrtyek210.yml | 19 ++ > spec/build/bsps/riscv/riscv/grp.yml | 4 + > spec/build/bsps/riscv/riscv/obj.yml | 1 + > .../bsps/riscv/riscv/optkendrytek210.yml | 18 + > spec/build/bsps/riscv/riscv/optns16550max.yml | 4 +- > spec/build/bsps/riscv/riscv/optsifiveuart.yml | 20 ++ > spec/build/cpukit/optsmp.yml | 1 + > 21 files changed, 779 insertions(+), 8 deletions(-) > create mode 100644 bsps/riscv/riscv/config/kendrytek210.cfg > create mode 100644 bsps/riscv/riscv/dts/kendryte-k210.dts > create mode 100644 bsps/riscv/riscv/include/bsp/k210.h > create mode 100644 bsps/riscv/riscv/include/bsp/kendryte-k210-dtb.h > create mode 100644 spec/build/bsps/riscv/riscv/bspkendrtyek210.yml > create mode 100644 spec/build/bsps/riscv/riscv/optkendrytek210.yml > create mode 100644 spec/build/bsps/riscv/riscv/optsifiveuart.yml > > -- > 2.25.1 > > ___ > devel mailing list > devel@rtems.org > http://lists.rtems.org/mailman/listinfo/devel ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: Flash Device API
On 16.03.23 23:07, Chris Johns wrote: On 16/3/2023 6:13 pm, Sebastian Huber wrote: Hello Aaron, this API seems to be RTEMS-specific. Maybe we should simply pick up an existing solution which is in more wide spread use, for example: https://docs.zephyrproject.org/latest/hardware/peripherals/flash.html That interface seems Zepher specific and looks to me like a series of calls that appear reasonable as a list to cover. The Zephyr API has drivers for a couple of chips: https://github.com/zephyrproject-rtos/zephyr/tree/main/drivers/flash The patch Aaron has posted is a driver. I prefer a driver like we have for I2C, SPI etc because the of support it brings. The I2C and SPI APIs are ported from Linux, so not RTEMS-specific. The initial set of ioctl commands is small to start with. If you feel we should offer more I suggest they get added once this is merged. Is the change OK? I am not opposed to commit this API, however, I don't think it is a step forward. It is yet another RTEMS-specific API. It has no related documentation patches. The API has no high level user (for example JAFFS2). The API has no implementation. It has no tests. It has no driver framework (ONFI, JEDEC). It has no example driver. It has no Doxygen documentation. The coding style of the new file has nothing to do with the RTEMS coding style. -- embedded brains GmbH Herr Sebastian HUBER Dornierstr. 4 82178 Puchheim Germany email: sebastian.hu...@embedded-brains.de phone: +49-89-18 94 741 - 16 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
