[lwip 2/2] Move and rename sys_arch_data_sync_barier()

[Pavel Pisa]

Hello Sebastian,

On Friday 24 of March 2023 11:21:57 Sebastian Huber wrote:
> Hello Pavel,
>
> On 18.03.23 01:04, Pavel Pisa wrote:
> > 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.
>
> ok, I will keep the sys_arch_data_sync_barier(). What I don't understand
> is why this stuff is under #ifndef __rtems__ (NOT defined __rtems__).

this is more complex, I need to refresh my memory. We have been using
uLAN OMK fork of LwIP code code in system less, RTEMS and FreeRTOS
setup.

On simple setups, as is FreeRTOS, the sys_arch_protect()
and sys_arch_unprotect() enables and disables interrupts

https://gitlab.com/pikron/sw-base/lwip/-/blob/omk-devel/ports/os/freertos/arch/sys_arch.h#L103

The same change has been made done for RTEMS by bulk change

https://gitlab.com/pikron/sw-base/lwip/-/blob/omk-devel/ports/os/rtems/arch/sys_arch.h#L103

But solution with IRQ disable only could not work on SMP system.

https://git.rtems.org/rtems-lwip/tree/uLan/ports/os/rtems/arch/sys_arch.c#n371

There is patch by Kinsey Moore which correct code to be safe for SMP
and switches rtems_interrupt_disable() to 

pval = _Thread_Dispatch_disable();
+  rtems_recursive_mutex_lock( &sys_arch_lock );

https://git.rtems.org/rtems-lwip/commit/uLan/ports/os/rtems/arch/sys_arch.c?id=c3b8ce71c4897be9984e555d8f1042e273edaa35

and later to

rtems_recursive_mutex_lock( &sys_arch_lock );

which are correct if the interrupt is never called directly into LwIP
driver but only through worker thread wakeup.

So in actual header file, the part which is under  #ifndef __rtems__
should read as use interrupt disable on generic system
but on RTEMS define declare functions prototypes and actual
implementation is in os/rtems/arch/sys_arch.c and for case of
use of the drivers over worker thread is correct.

But if I remember well then actual protect mechanism
should be used minimally if the driver uses thread
model..

So generally, the part in #ifndef __rtems__ in header file
can be moved away, the decalartions without functions body
are enough.

We need to resolve license update in the files and get
rid of uLAN directory and I hope that it can be cleaned
up. I would like to test code on TMS570LS3137 when I find
time and on TMS570LC43 one day...

Best wishes,

Pavel