Re: [RSB 1/3] 6/7: Update Newlib

[Sebastian Huber]

On 13/07/2022 11:24, Sebastian Huber wrote:

This makes the --enable-newlib-reent-thread-local (_REENT_THREAD_LOCAL_STORAGE)
Newlib configuration option available.


Any comments with respect to using the Newlib thread-local storage 
configuration option for arm, i386, microblaze, nios2, powerpc, riscv, 
and sparc?


I was a bit of work to add this Newlib option (about 50 patches).

--
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

[PATCH] score: Use PTHREAD_CANCELED for _Thread_Cancel()

[Sebastian Huber]

The rtems_task_delete() directive is basically just a combined pthread_cancel()
and pthread_join().  In addition, it removes the PTHREAD_DETACHED state.  The
exit value returned by pthread_join() of threads cancelled by
rtems_task_delete() should reflect this by getting a PTHREAD_CANCELED value and
instead of NULL.

Close #4680.
---
 cpukit/include/rtems/score/threadimpl.h |  5 +
 cpukit/posix/src/cancel.c   |  2 +-
 cpukit/score/src/threadrestart.c| 10 +-
 testsuites/psxtests/psx08/init.c|  2 +-
 4 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/cpukit/include/rtems/score/threadimpl.h 
b/cpukit/include/rtems/score/threadimpl.h
index e6e77b195c..638815237f 100644
--- a/cpukit/include/rtems/score/threadimpl.h
+++ b/cpukit/include/rtems/score/threadimpl.h
@@ -429,14 +429,11 @@ typedef enum {
 
  * @param[in, out] life_states_to_clear is the set of thread life states to
  *   clear for the thread to cancel.
-
- * @param exit_value is the exit value for the thread to cancel.
  */
 Thread_Cancel_state _Thread_Cancel(
   Thread_Control   *the_thread,
   Thread_Control   *executing,
-  Thread_Life_state life_states_to_clear,
-  void *exit_value
+  Thread_Life_state life_states_to_clear
 );
 
 /**
diff --git a/cpukit/posix/src/cancel.c b/cpukit/posix/src/cancel.c
index 0fb2199f0a..1ccfe75b0b 100644
--- a/cpukit/posix/src/cancel.c
+++ b/cpukit/posix/src/cancel.c
@@ -75,7 +75,7 @@ int pthread_cancel( pthread_t thread )
   } else {
 _Thread_Dispatch_disable_with_CPU( cpu_self, &lock_context );
 _ISR_lock_ISR_enable( &lock_context );
-(void) _Thread_Cancel( the_thread, executing, 0, PTHREAD_CANCELED );
+(void) _Thread_Cancel( the_thread, executing, 0 );
 _Thread_Dispatch_enable( cpu_self );
   }
   return 0;
diff --git a/cpukit/score/src/threadrestart.c b/cpukit/score/src/threadrestart.c
index e1911beb2b..584cfa5d8b 100644
--- a/cpukit/score/src/threadrestart.c
+++ b/cpukit/score/src/threadrestart.c
@@ -55,6 +55,8 @@
 #include 
 #include 
 
+#include 
+
 #define THREAD_JOIN_TQ_OPERATIONS &_Thread_queue_Operations_priority_inherit
 
 static void _Thread_Life_action_handler(
@@ -433,8 +435,7 @@ static void _Thread_Try_life_change_request(
 Thread_Cancel_state _Thread_Cancel(
   Thread_Control   *the_thread,
   Thread_Control   *executing,
-  Thread_Life_state life_states_to_clear,
-  void *exit_value
+  Thread_Life_state life_states_to_clear
 )
 {
   ISR_lock_Context  lock_context;
@@ -444,7 +445,7 @@ Thread_Cancel_state _Thread_Cancel(
 
   _Thread_State_acquire( the_thread, &lock_context );
 
-  _Thread_Set_exit_value( the_thread, exit_value );
+  _Thread_Set_exit_value( the_thread, PTHREAD_CANCELED );
   previous = _Thread_Change_life_locked(
 the_thread,
 life_states_to_clear,
@@ -476,8 +477,7 @@ Status_Control _Thread_Close(
   );
   _ISR_lock_ISR_enable( &queue_context->Lock_context.Lock_context );
 
-  cancel_state =
-_Thread_Cancel( the_thread, executing, THREAD_LIFE_DETACHED, NULL );
+  cancel_state = _Thread_Cancel( the_thread, executing, THREAD_LIFE_DETACHED );
 
   if ( cancel_state == THREAD_CANCEL_DONE ) {
 _Thread_Dispatch_enable( cpu_self );
diff --git a/testsuites/psxtests/psx08/init.c b/testsuites/psxtests/psx08/init.c
index 1caf9aa537..56124f4e23 100644
--- a/testsuites/psxtests/psx08/init.c
+++ b/testsuites/psxtests/psx08/init.c
@@ -144,7 +144,7 @@ static void test_delete_deadlock( void )
   value = NULL;
   eno = pthread_join( ctx.protected_join, &value );
   rtems_test_assert( eno == 0 );
-  rtems_test_assert( value == NULL );
+  rtems_test_assert( value == PTHREAD_CANCELED );
 
   rtems_test_assert( ctx.delete_status == RTEMS_INCORRECT_STATE );
 }
-- 
2.35.3

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

Re: [PATCH] cpukit/dev/can: Added CAN support

[oss]

Hello Prashanth,

first question: You also worked on a driver for beagle DCAN. Did you 
already adapt that driver to this API? If yes, it would be usefull to 
post that as a patch too so that the direction and the method how it 
will be used is more clear.


Note that some of my comments might are a bit critical because you add a 
general API. I would have a lot less problems if it would be only a 
driver specific API. If you add a general API, it has to fit the needs 
of not only your current driver but most future drivers too. Changing an 
API later is allways tricky.


Am 15.07.22 um 20:11 schrieb Prashanth S:

---
  cpukit/dev/can/can-queue.c| 112 +++
  cpukit/dev/can/can.c  | 480 ++
  cpukit/include/dev/can/can.h  | 115 +++
  spec/build/cpukit/librtemscpu.yml |   5 +
  4 files changed, 712 insertions(+)
  create mode 100644 cpukit/dev/can/can-queue.c
  create mode 100644 cpukit/dev/can/can.c
  create mode 100644 cpukit/include/dev/can/can.h

diff --git a/cpukit/dev/can/can-queue.c b/cpukit/dev/can/can-queue.c
new file mode 100644
index 00..1cebed2ca4
--- /dev/null
+++ b/cpukit/dev/can/can-queue.c
@@ -0,0 +1,112 @@
+/* SPDX-License-Identifier: BSD-2-Clause */
+
+/**
+ * @file
+ *
+ * @ingroup CANBus
+ *
+ * @brief Controller Area Network (CAN) Bus Implementation
+ *
+ */
+
+/*
+ * Copyright (C) 2022
+ *
+ * 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.
+ */
+
+#include 
+#include 
+
+#include 
+
+#include 
+#include 
+#include 
+
+rtems_status_code can_create_rx_buffers(struct can_bus *bus)
+{
+  static int init = 0;
+
+  return rtems_message_queue_create(rtems_build_name('c', 'a', 'n', '0' + 
init++), CAN_RX_BUF_COUNT, sizeof(struct can_msg),
+RTEMS_FIFO | RTEMS_LOCAL, 
&bus->rx_queue_id);


Please make sure to stic to the RTEMS style and use a 80 character line 
length.


You create a name with a "'0' + init++": Are you sure that a maximum of 
10 buffers are created? Otherwise you will get names like "can:", "can;" 
and - sooner or later: "can\xFF", "can\x00", ...


In the header you somewhere had a "destroy" function. Do you need some 
way to de-initialize buffers only used by that bus?



+}
+
+rtems_status_code can_create_tx_buffers(struct can_bus *bus)
+{
+  if ((bus->tx_fifo.pbuf = (struct can_msg *)malloc(CAN_TX_BUF_COUNT * 
sizeof(struct can_msg))) == NULL) {


You seem to like writing a lot of code in one line. From my point of 
view that makes code harder to read. If you write it into multiple lines 
instead, someone else doesn't have to think that much about which 
bracket closes where. For example in this case the following would be 
simpler to read:



bus->tx_fifo.pbuf = (struct can_msg *)malloc(CAN_TX_BUF_COUNT * 
sizeof(struct can_msg));

if (bus->tx_fifo.pbuf == NULL) {
   
}



+printf("malloc failed\n");


Prints are nice for debugging but can be really annoying in 
applications. Think about some kind of "debug_print" or similar that you 
can enable / disable with a macro at the start of the file. You can also 
use that to add a prefix or function name to all prints. If you search a 
problem, a line like "mallof failed" is meaningless if you are not 
currently working on specially this code. A line like 
"can_create_tx_buffers: malloc failed" is a lot more usefull. There are 
preprocessor macros like __FUNCTION__ that you can use for something 
like that.



+return RTEMS_NO_MEMORY;
+  }
+
+  bus->tx_fifo.head = bus->tx_fifo.tail = 0;


Same here: This is hard to read. I would suggest to split that into two 
lines. Alternatively you might want to think about just using a memset 
to have a clean start for a structure that y

[Feedback needed] Proposal for ADC API

[Duc Doan]

Hello,

I would like to propose an ADC API that aims to create an interface for
reading analog value conveniently with blocking/non-blocking styles and
support custom transfer function for each pin. The API depends on the
new GPIO API. 

Here are some features of the ADC API:
- Blocking/non-blocking read
- Interrupt
- Assign a transfer function to a pin, so that the result of a read()
function may already be converted to a desired output
- Setting some common options: resolution, data alignment

The changes I needed to make to the GPIO API:
- Add members to rtems_gpio struct: adc_handlers, adc_tf (only if
option BSP_ENABLE_ADC is defined 1)

I implemented ADC for STM32F4 using this API. The current state of
STM32F4 ADC:
- Using single channel conversion, regular group
- Pins that belong to multiple ADCs use ADC1
- Non-blocking read uses EOCS interrupt
- Supports up to 3 user-defined ISR, one for each ADC

Example API calls:

//
// Get GPIO objects and configure pin in ANALOG mode
rtems_gpio_get(POT_VPIN, &pot);
rtems_gpio_init(pot);
rtems_gpio_set_pin_mode(pot, RTEMS_GPIO_PINMODE_ANALOG);
rtems_gpio_set_pull(pot, RTEMS_GPIO_NOPULL);

// Setting resolution
rtems_adc_set_resolution(pot, 10);

// Blocking read
rtems_adc_read_raw(pot, &pot_value);

// Non-blocking read
rtems_adc_start_read_nb(pot);
rtems_adc_read_raw_nb(pot, &pot_value);

// Reading with transfer function y = 6x + 5
uint32_t params[] = {6, 5};
rtems_adc_assign_tf(pot, linear1, params);
double tf_result;
rtems_adc_read(pot, &tf_result);
...
double linear1(void *params, uint32_t raw) {
uint32_t *p = (uint32_t *) params;
return ((double) raw) * p[0] + p[1];
}
...
/**/

Sample applications can be found in the RTEMS_ADC_* folders in this
repo: https://github.com/dtbpkmte/GSoC-2022-RTEMS-Sample-Apps

Please have a look at these files:
- Modified GPIO API:
https://github.com/dtbpkmte/GSoC-2022-RTEMS/blob/adc-api/bsps/include/bsp/gpio2.h
https://github.com/dtbpkmte/GSoC-2022-RTEMS/blob/adc-api/bsps/shared/dev/gpio/gpio.c
- ADC API:
https://github.com/dtbpkmte/GSoC-2022-RTEMS/blob/adc-api/bsps/include/bsp/adc.h
https://github.com/dtbpkmte/GSoC-2022-RTEMS/tree/adc-api/bsps/shared/dev/adc
- STM32F4 GPIO:
https://github.com/dtbpkmte/GSoC-2022-RTEMS/blob/adc-api/bsps/arm/stm32f4/include/bsp/stm32f4_gpio.h
https://github.com/dtbpkmte/GSoC-2022-RTEMS/blob/adc-api/bsps/arm/stm32f4/gpio/gpio.c
- STM32F4 ADC:
https://github.com/dtbpkmte/GSoC-2022-RTEMS/blob/adc-api/bsps/arm/stm32f4/include/bsp/stm32f4_adc.h
https://github.com/dtbpkmte/GSoC-2022-RTEMS/blob/adc-api/bsps/arm/stm32f4/adc/adc.c

The code still has bugs and I am fixing/writing test applications and
adding documentation. I would appreciate all feedback. Thank you very
much.

Best,

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

Re: [Feedback needed] Proposal for ADC API

[Sebastian Huber]

Hello,

we should also consider to use existing APIs and implementations for 
stuff we do not yet have in RTEMS, for example:


https://docs.zephyrproject.org/latest/hardware/peripherals/adc.html

The Zephyr project uses device trees to statically configure device drivers:

https://docs.zephyrproject.org/latest/build/dts/index.html

This is quite interesting for lower end targets.

--
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

Possible tls bug on arm / microblaze

[Sam Price]

On microblaze i was getting a

ESR: Unaligned data access exception

I narrowed this down to this "ti->offset;" being uninitialized in the
following two bsps

https://git.rtems.org/rtems/tree/cpukit/score/cpu/microblaze/__tls_get_addr.c
Also this file
cpukit/score/cpu/arm/__tls_get_addr.c

I tried recompiling with this on the microblaze build with offset being
renamed to offseta.
This only caused a compilation error in the __tls_get_addr function.
So I think this variable is being used without being initialized or ever
changed.
Is it safe to just remove the offset variable?

-- 
Thank you,

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

Re: [PATCH] cpukit/dev/can: Added CAN support

[Prashanth S]

Hi Christian,

This is to reply to review comments.

> first question: You also worked on a driver for beagle DCAN. Did you
> already adapt that driver to this API? If yes, it would be usefull to
> post that as a patch too so that the direction and the method how it
> will be used is more clear.
Yes, Waiting for License confirmation.

> Note that some of my comments might are a bit critical because you add a
> general API. I would have a lot less problems if it would be only a
> driver specific API. If you add a general API, it has to fit the needs
> of not only your current driver but most future drivers too. Changing an
> API later is allways tricky.
Ok

> Please make sure to stic to the RTEMS style and use a 80 character line
> length.
OK

> You create a name with a "'0' + init++": Are you sure that a maximum of
> 10 buffers are created? Otherwise you will get names like "can:", "can;"
> and - sooner or later: "can\xFF", "can\x00", ...
Assuming we have less than 10 CAN controllers, I will update this to limit
to 10 queues.

> In the header you somewhere had a "destroy" function. Do you need some
> way to de-initialize buffers only used by that bus?
Yes, the can_bus structure holds data related only to a specific bus.

>> +}
>> +
>> +rtems_status_code can_create_tx_buffers(struct can_bus *bus)
>> +{
>> +  if ((bus->tx_fifo.pbuf = (struct can_msg *)malloc(CAN_TX_BUF_COUNT *
sizeof(struct can_msg))) == NULL) {

> You seem to like writing a lot of code in one line. From my point of
> view that makes code harder to read. If you write it into multiple lines
> instead, someone else doesn't have to think that much about which
> bracket closes where. For example in this case the following would be
> simpler to read:
I will make changes to limit line length to 80.

> 
> bus->tx_fifo.pbuf = (struct can_msg *)malloc(CAN_TX_BUF_COUNT *
> sizeof(struct can_msg));
> if (bus->tx_fifo.pbuf == NULL) {
>
> }
> 
>
> +printf("malloc failed\n");
>
> Prints are nice for debugging but can be really annoying in
> applications. Think about some kind of "debug_print" or similar that you
> can enable / disable with a macro at the start of the file. You can also
> use that to add a prefix or function name to all prints. If you search a
> problem, a line like "mallof failed" is meaningless if you are not
> currently working on specially this code. A line like
> "can_create_tx_buffers: malloc failed" is a lot more usefull. There are
> preprocessor macros like __FUNCTION__ that you can use for something
> like that.
Ok, I will update the prints with the function name in it.

> +return RTEMS_NO_MEMORY;
> +  }
> +
> +  bus->tx_fifo.head = bus->tx_fifo.tail = 0;
>
> Same here: This is hard to read. I would suggest to split that into two
> lines. Alternatively you might want to think about just using a memset
> to have a clean start for a structure that you initialize.
Ok, memset is done in the can_bus_init or calloc is used in
can_bus_alloc_and_init.
Removing bus->tx_fifo.head = bus->tx_fifo.tail = 0;

> +  bus->tx_fifo.empty_count = CAN_TX_BUF_COUNT;
> +
> +  return 0;
> +}
> +
> +bool can_tx_buf_isempty(struct can_bus *bus)
> +{
> +  if (bus->tx_fifo.empty_count <= 0) {
> +/* tx_fifo.empty_count does not go below zero, incase if it goes
update to zero */
> +bus->tx_fifo.empty_count = 0;

empty_count is an unsigned type and therefore can never be smaller than
0. This line is not necessary at all.
Ok, removing it.

> +
> +return false;
> +  }
> +
> +  return true;
> +}
> +
> +struct can_msg *can_tx_get_data_buf(struct can_bus *bus)
> +{
> +  struct can_msg *msg = NULL;
> +
> +  if (bus->tx_fifo.empty_count == CAN_TX_BUF_COUNT) {
> +printf("can_tx_get_next_data_buf All buffers are empty\n");
> +return NULL;
> +  }
> +
> +  msg = &bus->tx_fifo.pbuf[bus->tx_fifo.tail];
> +  bus->tx_fifo.empty_count++;
> +  bus->tx_fifo.tail = (bus->tx_fifo.tail + 1) % CAN_TX_BUF_COUNT;

> You want to return the pointer to the message (msg) to the caller,
> right? In that case you must not marke it as empty here. Otherwise
> someone else could already use that memory again while the application
> is still processing it.
Here the application does not use this (driver buffer) buffer, we copy the
message from this buffer (driver buffer)
to the application passed buffer.

> Looks like a helper function that is only used in this file? Please make
> it  static and remove it from the header. Same is true for "take_sem" and
> similar below.
Functions in can-queue.c are used in can.c, Moving them to can-queue.h or
can.h header file.
I will add static to take_sem and give_sem.

> Is an interrupt lock really necessary? Wouldn't a mutex or similar work
> too to protect the relevant data? An interrupt lock is something that
> should be used really carefull. You tell the system: Stop everything
> else. This here is the most important task.
>
> That means that for example a time critical data acquisition process can
> be delayed 

Re: [PATCH] cpukit/dev/can: Added CAN support

[Christian Mauderer]

Hello Prashanth,

Am 19.07.22 um 15:09 schrieb Prashanth S:

Hi Christian,

This is to reply to review comments.

first question: You also worked on a driver for beagle DCAN. Did you 
already adapt that driver to this API? If yes, it would be usefull to 
post that as a patch too so that the direction and the method how it 
will be used is more clear.

Yes, Waiting for License confirmation.


If you didn't have to agree to some really odd license, you can post it 
as a patch on the list. Make sure to make it _very_ clear that you are 
not sure about the license and for what reason. Things like licensing 
should be discussed with the community in public so that the discussion 
is archived together with the list.




Note that some of my comments might are a bit critical because you add a 
general API. I would have a lot less problems if it would be only a 
driver specific API. If you add a general API, it has to fit the needs 
of not only your current driver but most future drivers too. Changing an 
API later is allways tricky.

Ok

Please make sure to stic to the RTEMS style and use a 80 character line 
length.

OK

You create a name with a "'0' + init++": Are you sure that a maximum of 
10 buffers are created? Otherwise you will get names like "can:", "can;" 
and - sooner or later: "can\xFF", "can\x00", ...
Assuming we have less than 10 CAN controllers, I will update this to 
limit to 10 queues.


In the header you somewhere had a "destroy" function. Do you need some 
way to de-initialize buffers only used by that bus?

Yes, the can_bus structure holds data related only to a specific bus.



In that case the static variable will make trouble. You have a static 
counter. If I register the bus, deregister it and register it again, the 
counter will have odd values.



+}
+
+rtems_status_code can_create_tx_buffers(struct can_bus *bus)
+{
+  if ((bus->tx_fifo.pbuf = (struct can_msg *)malloc(CAN_TX_BUF_COUNT * 
sizeof(struct can_msg))) == NULL) {


You seem to like writing a lot of code in one line. From my point of 
view that makes code harder to read. If you write it into multiple lines 
instead, someone else doesn't have to think that much about which 
bracket closes where. For example in this case the following would be 
simpler to read:

I will make changes to limit line length to 80.



bus->tx_fifo.pbuf = (struct can_msg *)malloc(CAN_TX_BUF_COUNT * 
sizeof(struct can_msg));

if (bus->tx_fifo.pbuf == NULL) {
    
}


+    printf("malloc failed\n");

Prints are nice for debugging but can be really annoying in 
applications. Think about some kind of "debug_print" or similar that you 
can enable / disable with a macro at the start of the file. You can also 
use that to add a prefix or function name to all prints. If you search a 
problem, a line like "mallof failed" is meaningless if you are not 
currently working on specially this code. A line like 
"can_create_tx_buffers: malloc failed" is a lot more usefull. There are 
preprocessor macros like __FUNCTION__ that you can use for something 
like that.

Ok, I will update the prints with the function name in it.



Better: Use some macro to automate that and enable or disable all debug 
messages at once. Also it's not the nicest solution, it's not uncommon 
to have something like that. Examples are:


cpukit/libdrvmgr/drvmgr.c:44:#define DBG(x...) printk(x)

cpukit/include/rtems/posix/aio_misc.h:116:#define AIO_printf(_x) printf(_x)

cpukit/libfs/src/ftpfs/ftpfs.c:61:  #define DEBUG_PRINTF(...) 
printf(__VA_ARGS__)


bsps/mips/malta/pci/pci.c:49:  #define JPRINTK(fmt, ...) printk("%s: " 
fmt, __FUNCTION__, ##__VA_ARGS__)



+    return RTEMS_NO_MEMORY;
+  }
+
+  bus->tx_fifo.head = bus->tx_fifo.tail = 0;

Same here: This is hard to read. I would suggest to split that into two 
lines. Alternatively you might want to think about just using a memset 
to have a clean start for a structure that you initialize.
Ok, memset is done in the can_bus_init or calloc is used in 
can_bus_alloc_and_init.

Removing bus->tx_fifo.head = bus->tx_fifo.tail = 0;


+  bus->tx_fifo.empty_count = CAN_TX_BUF_COUNT;
+
+  return 0;
+}
+
+bool can_tx_buf_isempty(struct can_bus *bus)
+{
+  if (bus->tx_fifo.empty_count <= 0) {
+    /* tx_fifo.empty_count does not go below zero, incase if it goes update to 
zero */
+    bus->tx_fifo.empty_count = 0;


empty_count is an unsigned type and therefore can never be smaller than
0. This line is not necessary at all.
Ok, removing it.


+
+    return false;
+  }
+
+  return true;
+}
+
+struct can_msg *can_tx_get_data_buf(struct can_bus *bus)
+{
+  struct can_msg *msg = NULL;
+
+  if (bus->tx_fifo.empty_count == CAN_TX_BUF_COUNT) {
+    printf("can_tx_get_next_data_buf All buffers are empty\n");
+    return NULL;
+  }
+
+  msg = &bus->tx_fifo.pbuf[bus->tx_fifo.tail];
+  bus->tx_fifo.empty_count++;
+  bus->tx_fifo.tail = (bus->tx_fifo.tail + 1) % CAN_TX_BUF_COUNT;


You want to return the pointer to the message (msg) to the caller, 
right?

Re: Time to promote lwip git repo

[Gedare Bloom]

On Wed, Jul 13, 2022 at 11:27 PM Christian MAUDERER
 wrote:
>
> Am 13.07.22 um 04:51 schrieb Chris Johns:
> > On 13/7/2022 10:08 am, Joel Sherrill wrote:
> >> Vijay and Kinsey have made great progress in addressing the issues that 
> >> were
> >> raised about the lwip tcpip stack that needed to be addressed before it 
> >> became
> >> an official top level repository. Thanks to both of them.
> >
> > Well done. Great effort.
> >
> >> I think it's time to promote the repo. Hopefully just a couple of core
> >> developers agreeing is sufficient to get this moving.
> >
> > I support this happening. It is great to see this networking option for 
> > small
> > devices become available.
>
> I haven't tested the lwip repository yet, but I agree that a stack for
> small targets is great. So I would support that too.
>
I'm glad to see this happening and support the promotion.

> Christian
> --
> 
> embedded brains GmbH
> Herr Christian MAUDERER
> Dornierstr. 4
> 82178 Puchheim
> Germany
> email:  christian.maude...@embedded-brains.de
> phone:  +49-89-18 94 741 - 18
> mobile: +49-176-152 206 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
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: Possible tls bug on arm / microblaze

[Sebastian Huber]

On 19/07/2022 14:51, Sam Price wrote:

On microblaze i was getting a

ESR: Unaligned data access exception

I narrowed this down to this "ti->offset;" being uninitialized in the 
following two bsps


https://git.rtems.org/rtems/tree/cpukit/score/cpu/microblaze/__tls_get_addr.c 


Also this file
cpukit/score/cpu/arm/__tls_get_addr.c

I tried recompiling with this on the microblaze build with offset being 
renamed to offseta.

This only caused a compilation error in the __tls_get_addr function.
So I think this variable is being used without being initialized or ever 
changed.

Is it safe to just remove the offset variable?


No, it is not safe to remove the offset member. The offset member value 
is provided by the caller of __tls_get_addr(). You have to check that 
TLS_Index definition matches the GCC ABI. Do the sptls* tests run on 
your system?


--
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

[PATCH] aarch64: Use page table level 0

[Kinsey Moore]

This alters the AArch64 page table generation and mapping code and MMU
configuration to use page table level 0 in addition to levels 1, 2, and
3. This allows the mapping of up to 48 bits of memory space and is the
maximum that can be mapped without relying on additional processor
extensions. Mappings are restricted based on the number of physical
address bits that the CPU supports.
---
 bsps/aarch64/include/bsp/aarch64-mmu.h| 58 +++
 bsps/aarch64/shared/mmu/vmsav8-64.c   |  7 ++-
 .../aarch64/include/libcpu/mmu-vmsav8-64.h|  1 -
 3 files changed, 53 insertions(+), 13 deletions(-)

diff --git a/bsps/aarch64/include/bsp/aarch64-mmu.h 
b/bsps/aarch64/include/bsp/aarch64-mmu.h
index 35fc79d73a..c881d5e825 100644
--- a/bsps/aarch64/include/bsp/aarch64-mmu.h
+++ b/bsps/aarch64/include/bsp/aarch64-mmu.h
@@ -48,6 +48,9 @@
 extern "C" {
 #endif /* __cplusplus */
 
+/* AArch64 uses levels 0, 1, 2, and 3 */
+#define MMU_MAX_SUBTABLE_PAGE_BITS ( 3 * MMU_BITS_PER_LEVEL + MMU_PAGE_BITS )
+
 typedef struct {
   uintptr_t begin;
   uintptr_t end;
@@ -216,15 +219,15 @@ aarch64_mmu_get_sub_table(
 
 BSP_START_TEXT_SECTION static inline rtems_status_code aarch64_mmu_map_block(
   uint64_t *page_table,
-  uintptr_t root_address,
-  uintptr_t addr,
+  uint64_t root_address,
+  uint64_t addr,
   uint64_t size,
-  uint32_t level,
+  int8_t level,
   uint64_t flags
 )
 {
   uint32_t shift = ( 2 - level ) * MMU_BITS_PER_LEVEL + MMU_PAGE_BITS;
-  uintptr_t granularity = 1 << shift;
+  uint64_t granularity = 1LLU << shift;
   uint64_t page_flag = 0;
 
   if ( level == 2 ) {
@@ -233,7 +236,7 @@ BSP_START_TEXT_SECTION static inline rtems_status_code 
aarch64_mmu_map_block(
 
   while ( size > 0 ) {
 uintptr_t index = aarch64_mmu_get_index( root_address, addr, shift );
-uintptr_t block_bottom = RTEMS_ALIGN_DOWN( addr, granularity );
+uint64_t block_bottom = RTEMS_ALIGN_DOWN( addr, granularity );
 uint64_t chunk_size = granularity;
 
 /* check for perfect block match */
@@ -270,7 +273,7 @@ BSP_START_TEXT_SECTION static inline rtems_status_code 
aarch64_mmu_map_block(
 }
 
 /* Deal with any subtable modification  */
-uintptr_t new_root_address = root_address + index * granularity;
+uint64_t new_root_address = root_address + index * granularity;
 uint64_t *sub_table = NULL;
 rtems_status_code sc;
 
@@ -311,6 +314,33 @@ BSP_START_DATA_SECTION extern const 
aarch64_mmu_config_entry
 BSP_START_DATA_SECTION extern const size_t
   aarch64_mmu_config_table_size;
 
+/* Get the maximum number of bits supported by this hardware */
+BSP_START_TEXT_SECTION static inline uint64_t
+aarch64_mmu_get_cpu_pa_bits( void )
+{
+  uint64_t id_reg = _AArch64_Read_id_aa64mmfr0_el1();
+
+  switch ( AARCH64_ID_AA64MMFR0_EL1_PARANGE_GET( id_reg ) ) {
+  case 0:
+ return 32;
+  case 1:
+ return 36;
+  case 2:
+ return 40;
+  case 3:
+ return 42;
+  case 4:
+ return 44;
+  case 5:
+ return 48;
+  case 6:
+ return 52;
+  default:
+ return 48;
+  }
+  return 48;
+}
+
 BSP_START_TEXT_SECTION static inline void
 aarch64_mmu_set_translation_table_entries(
   uint64_t *ttb,
@@ -320,14 +350,19 @@ aarch64_mmu_set_translation_table_entries(
   /* Force alignemnt to 4k page size */
   uintptr_t begin = RTEMS_ALIGN_DOWN( config->begin, MMU_PAGE_SIZE );
   uintptr_t end = RTEMS_ALIGN_UP( config->end, MMU_PAGE_SIZE );
+  uint64_t max_mappable = 1LLU << aarch64_mmu_get_cpu_pa_bits();
   rtems_status_code sc;
 
+  if ( begin >= max_mappable || end > max_mappable ) {
+rtems_fatal_error_occurred( RTEMS_INVALID_ADDRESS );
+  }
+
   sc = aarch64_mmu_map_block(
 ttb,
 0x0,
 begin,
 end - begin,
-0,
+-1,
 config->flags
   );
 
@@ -347,7 +382,7 @@ BSP_START_TEXT_SECTION static inline void 
aarch64_mmu_setup_translation_table(
   aarch64_mmu_page_table_set_blocks(
 ttb,
 (uintptr_t) NULL,
-MMU_TOP_LEVEL_PAGE_BITS,
+MMU_MAX_SUBTABLE_PAGE_BITS,
 0
   );
 
@@ -390,10 +425,11 @@ aarch64_mmu_disable( void )
 BSP_START_TEXT_SECTION static inline void aarch64_mmu_setup( void )
 {
   /* Set TCR */
-  /* 128GB/36 bits mappable (64-0x1c) */
+  /* 256TB/48 bits mappable (64-0x10) */
   _AArch64_Write_tcr_el1(
-AARCH64_TCR_EL1_T0SZ( 0x1c ) | AARCH64_TCR_EL1_IRGN0( 0x1 ) |
-AARCH64_TCR_EL1_ORGN0( 0x1 ) | AARCH64_TCR_EL1_SH0( 0x3 ) | 
AARCH64_TCR_EL1_TG0( 0x0 )
+AARCH64_TCR_EL1_T0SZ( 0x10 ) | AARCH64_TCR_EL1_IRGN0( 0x1 ) |
+AARCH64_TCR_EL1_ORGN0( 0x1 ) | AARCH64_TCR_EL1_SH0( 0x3 ) |
+AARCH64_TCR_EL1_TG0( 0x0 ) | AARCH64_TCR_EL1_IPS( 0x5ULL )
   );
 
   /* Set MAIR */
diff --git a/bsps/aarch64/shared/mmu/vmsav8-64.c 
b/bsps/aarch64/shared/mmu/vmsav8-64.c
index 9caa91c414..190a05f7d5 100644
--- a/bsps/aarch64/shared/mmu/vmsav8-64.c
+++ b/bsps/aarch64/shared/mmu/vmsav8-64.c
@@ -47,6 +47,11 @@ rtems_status_code aarch64_mmu_map(
 )
 {
   rtems_status_code sc;
+  uint64_t max_mappable = 1LLU << a

Re: Possible tls bug on arm / microblaze

[Sam Price]

At one point I believe a company tested the sptls tests, unsure if they did
this on qemu or hardware.

I'll try running those tests on a recent hash, and back when the company
ran those tests.

Where would the ti->offset member be updated / initialized at?
Only the arm, and microblaze bsps have this in them.

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

Re: Possible tls bug on arm / microblaze

[Sebastian Huber]

On 19/07/2022 20:10, Sam Price wrote:


Where would the ti->offset member be updated / initialized at?


The caller of __tls_get_addr() has to do this. On ARM, this is function 
is normally not used.


--
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

[PATCH rtems-docs] zynqmp: Add commentary about lwIP usage

[Kinsey Moore]

---
 user/bsps/aarch64/xilinx-zynqmp.rst | 9 +
 1 file changed, 9 insertions(+)

diff --git a/user/bsps/aarch64/xilinx-zynqmp.rst 
b/user/bsps/aarch64/xilinx-zynqmp.rst
index 78bff12..58954d6 100644
--- a/user/bsps/aarch64/xilinx-zynqmp.rst
+++ b/user/bsps/aarch64/xilinx-zynqmp.rst
@@ -244,6 +244,15 @@ Cadence GEM instances present on all ZynqMP hardware 
variants. All interfaces
 are enabled by default, but only interfaces with operational MII busses will be
 recognized and usable in RTEMS. Most ZynqMP dev boards use CGEM3.
 
+When used with lwIP from the rtems-lwip integration repository, these BSP
+variants support networking via the 0th and any one other of the Cadence GEM
+instances simultaneously. This is a limitation of the Xilinx driver,
+specifically in code referring directly to XPAR_XEMACPS_0_BASEADDR.
+
+The interfaces will not come up by default under lwIP and must be configured
+manually. There are examples of this in the start_networking() implementation
+in netstart.c as used by the network tests.
+
 Running Executables on QEMU
 ---
 
-- 
2.30.2

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

Re: [PATCH] aarch64: Use page table level 0

[Chris Johns]

On 20/7/2022 2:58 am, Kinsey Moore wrote:
> This alters the AArch64 page table generation and mapping code and MMU
> configuration to use page table level 0 in addition to levels 1, 2, and
> 3. This allows the mapping of up to 48 bits of memory space and is the
> maximum that can be mapped without relying on additional processor
> extensions. Mappings are restricted based on the number of physical
> address bits that the CPU supports.

OK to push.

I have tested this with 8G of memory mapped to the Versal's DDRCM0_region0_mem
and DDRCM0_region1_mem address spaces.

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

Re: [PATCH rtems-docs] zynqmp: Add commentary about lwIP usage

[Chris Johns]

On 20/7/2022 5:42 am, Kinsey Moore wrote:
> ---
>  user/bsps/aarch64/xilinx-zynqmp.rst | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/user/bsps/aarch64/xilinx-zynqmp.rst 
> b/user/bsps/aarch64/xilinx-zynqmp.rst
> index 78bff12..58954d6 100644
> --- a/user/bsps/aarch64/xilinx-zynqmp.rst
> +++ b/user/bsps/aarch64/xilinx-zynqmp.rst
> @@ -244,6 +244,15 @@ Cadence GEM instances present on all ZynqMP hardware 
> variants. All interfaces
>  are enabled by default, but only interfaces with operational MII busses will 
> be
>  recognized and usable in RTEMS. Most ZynqMP dev boards use CGEM3.
>  
> +When used with lwIP from the rtems-lwip integration repository, these BSP
> +variants support networking via the 0th and any one other of the Cadence GEM
> +instances simultaneously. 

Is there a simpler way to word this? English may not be a user's first language.

 When used with lwIP from the rtems-lwip integration repository, these BSP
 variants support networking using the Cadence GEM interfaces labelled
 ``CGEM0`` to ``CGEM3``. Multiple interfaces can be used at the same time.

?

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

Re: Time to promote lwip git repo

[Chris Johns]

Thanks to everyone for this effort.

Vijay, if you need a hand doing this please ping me on discord.

Chris

On 20/7/2022 12:40 am, Gedare Bloom wrote:
> On Wed, Jul 13, 2022 at 11:27 PM Christian MAUDERER
>  wrote:
>>
>> Am 13.07.22 um 04:51 schrieb Chris Johns:
>>> On 13/7/2022 10:08 am, Joel Sherrill wrote:
 Vijay and Kinsey have made great progress in addressing the issues that 
 were
 raised about the lwip tcpip stack that needed to be addressed before it 
 became
 an official top level repository. Thanks to both of them.
>>>
>>> Well done. Great effort.
>>>
 I think it's time to promote the repo. Hopefully just a couple of core
 developers agreeing is sufficient to get this moving.
>>>
>>> I support this happening. It is great to see this networking option for 
>>> small
>>> devices become available.
>>
>> I haven't tested the lwip repository yet, but I agree that a stack for
>> small targets is great. So I would support that too.
>>
> I'm glad to see this happening and support the promotion.
> 
>> Christian
>> --
>> 
>> embedded brains GmbH
>> Herr Christian MAUDERER
>> Dornierstr. 4
>> 82178 Puchheim
>> Germany
>> email:  christian.maude...@embedded-brains.de
>> phone:  +49-89-18 94 741 - 18
>> mobile: +49-176-152 206 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
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: Time to promote lwip git repo

[Vijay Kumar Banerjee]

Thank you everyone for supporting the move!

On Tue, Jul 19, 2022 at 3:24 PM Chris Johns  wrote:
>
> Thanks to everyone for this effort.
>
> Vijay, if you need a hand doing this please ping me on discord.
I have moved it to the top level and added the hooks using the doc I
wrote last time:
https://docs.rtems.org/branches/master/eng/vc-authors.html#migrate-a-personal-repository-to-top-level



>
> Chris
>
> On 20/7/2022 12:40 am, Gedare Bloom wrote:
> > On Wed, Jul 13, 2022 at 11:27 PM Christian MAUDERER
> >  wrote:
> >>
> >> Am 13.07.22 um 04:51 schrieb Chris Johns:
> >>> On 13/7/2022 10:08 am, Joel Sherrill wrote:
>  Vijay and Kinsey have made great progress in addressing the issues that 
>  were
>  raised about the lwip tcpip stack that needed to be addressed before it 
>  became
>  an official top level repository. Thanks to both of them.
> >>>
> >>> Well done. Great effort.
> >>>
>  I think it's time to promote the repo. Hopefully just a couple of core
>  developers agreeing is sufficient to get this moving.
> >>>
> >>> I support this happening. It is great to see this networking option for 
> >>> small
> >>> devices become available.
> >>
> >> I haven't tested the lwip repository yet, but I agree that a stack for
> >> small targets is great. So I would support that too.
> >>
> > I'm glad to see this happening and support the promotion.
> >
> >> Christian
> >> --
> >> 
> >> embedded brains GmbH
> >> Herr Christian MAUDERER
> >> Dornierstr. 4
> >> 82178 Puchheim
> >> Germany
> >> email:  christian.maude...@embedded-brains.de
> >> phone:  +49-89-18 94 741 - 18
> >> mobile: +49-176-152 206 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
> ___
> 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: [RSB 1/3] 6/7: Update Newlib

[Chris Johns]

On 19/7/2022 5:18 pm, Sebastian Huber wrote:
> On 13/07/2022 11:24, Sebastian Huber wrote:
>> This makes the --enable-newlib-reent-thread-local 
>> (_REENT_THREAD_LOCAL_STORAGE)
>> Newlib configuration option available.
> 
> Any comments with respect to using the Newlib thread-local storage 
> configuration
> option for arm, i386, microblaze, nios2, powerpc, riscv, and sparc?

Do these archs have working TLS support? It seems Microblaze has issues which
our tests are not picking up. It would be good to know why our tests are not
picking up the reported problem.

Which archs in the list have you run the testsuite on?

Has any of newlib's tests been run? Can we run newlib tests?

Did the switch to TLS exceptions end up on 6? I cannot see anything in the RSB
commit history mentioning it.

Are these TLS allocations in newlib based on a single use of a newlib call that
brings in the reent stuff? How does that work?

Does this change effect libdl users? TLS has not been implemented in libdl and
the current reent model still works for those users dependent on libdl

> I was a bit of work to add this Newlib option (about 50 patches).

I am sure.

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

Re: [PATCH] aarch64: Use page table level 0

[Sebastian Huber]

On 19/07/2022 18:58, Kinsey Moore wrote:

+  if ( begin >= max_mappable || end > max_mappable ) {
+rtems_fatal_error_occurred( RTEMS_INVALID_ADDRESS );
+  }


Such a fatal error is not really helpful, since you cannot get the error 
location from the fatal source/code pair. I would add a new code for 
bsp_fatal() and use this function. Alternatively, we could add a new 
fatal error source which uses the link register or the source code 
file/line for generic fatal errors.


--
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