Michael Neuhauser wrote:
On Mon, 2005-03-21 at 22:05, Jan Kiszka wrote:

Michael Neuhauser schrieb:

On Mon, 2005-03-21 at 17:54, Jan Kiszka wrote:


rtai_oldnames.h in magma-CVS is broken. This patch fixes it for x86, other architectures may require a check as well.

With this patch applied, RTnet at least loads on top of magma now.

Jan
[...]
-- rtai_oldnames.h      5 Jan 2005 16:48:31 -0000       1.4
+++ rtai_oldnames.h     21 Mar 2005 16:36:29 -0000
@@ -37,9 +37,9 @@
#define IFLAG                        RTAI_IFLAG
#define hard_cli()                   rtai_cli()
#define hard_sti()                   rtai_sti()
-#define hard_save_flags_and_cli(x)   rtai_local_irq_save(x)
-#define hard_restore_flags(x)        rtai_local_irq_restore(x)
-#define hard_save_flags(x)           rtai_local_irq_flags(x)
+#define hard_save_flags_and_cli(x)   rtai_save_flags_and_cli(x)
+#define hard_restore_flags(x)        rtai_restore_flags(x)
+#define hard_save_flags(x)           rtai_save_flags(x)
#define hard_cpu_id                  adeos_processor_id

#endif /* __KERNEL__ */


Hm, there is definitely something wrong here (rtai_local_irq_*() are not
defined for i386). But I'm not sure if your patch is correct either.


It lets RTnet compile, isn't this enough? ;)


It's a start :-)


Seriously:


What is expected from the hard_*() macros? (I'm not so familiar with
"historical" RTAI but would like to get it right for ARM.) If they
should disable the CPU's interrupt enabled flag (as their name suggests)
then it should be

#define hard_cli()                   rtai_hw_cli()
#define hard_sti()                   rtai_hw_sti()
#define hard_save_flags_and_cli()    rtai_hw_save_flags_and_cli()
...

I think hard_* were originally (rthal) intended to touch the hardware directly. With adeos, it's sufficient (I think it's actually the only correct way) to stall the pipeline in front of RTAI.


If you know what you are doing it is perfectly OK to hard disable IRQs
(at least I see it that way).


In fact, that's ok for the use case RTnet.


Your patch is only correct if the hard_*() macros are expected to
prevent interrupts to be delivered to RTAI, but nothing more, i.e. you
can't use them to protect time-critical hardware operations (if only the
RTAI pipeline is stalled, interrupts are still handled by Adeos, they
are just not dispatched to RTAI).

So, let's walk the #define chain of x86-vesuvio (assuming it is correct):


Wait a minute, are we talking about vesuvio or magma/vulcano now?


hard_cli() -> rtai_cli() -> adeos_stall_pipeline_from(&rtai_domain)

That's what I did to fiddle out the hopefully correct meaning for my patch.


The define chain is not the problem. The problem seems to me, that it
seems unclear what the hard_*() macros should provide:
      * protection against IRQ-delivery to RTAI
or
      * protection against IRQs happening at all
I don't expect you to answer this, but maybe someone with a big picture
view (Paolo, are you listening?) might shed some light on it.

Well I'm, not when out of office though :-).

Philippe as said something already.

So I think you are right in saying that one should distinguish between cli/sti used either for accessing the hardware or for fast atomic stuff. In the first case there _must_ be rtai_hw_something or, since adeos is there anyhow, adeso_hw_something directly.

In the fast atomic case the fix of Jan is OK. So it is up to him to know what he's using the oldnames for.

In any case the patch is due as the name rtai_local_something are no more in 3.2, so their use will crash the simple making. In fact in 3.2 I reversed to the naming I liked since DOS times, as local_irq_save means just saving the irq state flag not clearing it too. So I prefer to let it for Linux magicians and use a naming that says it long and clear and thus not hide what's behind to poor people like me.

Paolo.


Reply via email to