Re: [Adeos-main] [PATCH 1/2] ipipe: Pass NTP-corrected time information from Linux to higher domains

[Philippe Gerum <rpm@xenomai.org>]

On Wed, 2010-08-25 at 11:19 +0200, Jan Kiszka wrote:
> Philippe Gerum wrote:
> > On Wed, 2010-08-25 at 10:58 +0200, Jan Kiszka wrote:
> >> Philippe Gerum wrote:
> >>> On Wed, 2010-08-25 at 10:50 +0200, Jan Kiszka wrote:
> >>>> Philippe Gerum wrote:
> >>>>> On Fri, 2010-07-02 at 13:50 +0200, Wolfgang Mauerer wrote:
> >>>>>
> >>>>> <snip>
> >>>>>
> >>>>>> diff --git a/include/linux/ipipe_tickdev.h b/include/linux/ipipe_tickdev.h
> >>>>>> index 4a1cb1b..86f13e0 100644
> >>>>>> --- a/include/linux/ipipe_tickdev.h
> >>>>>> +++ b/include/linux/ipipe_tickdev.h
> >>>>>> @@ -25,6 +25,7 @@
> >>>>>>  #if defined(CONFIG_IPIPE) && defined(CONFIG_GENERIC_CLOCKEVENTS)
> >>>>> Since we should have CONFIG_HAVE_IPIPE_HOSTRT by now, let's use it.
> >>>> Don't get yet how this fits here.
> >>> arch-dep would define CONFIG_HAVE_IPIPE_HOSTRT [if IPIPE]
> >>>
> >> Still don't see the relation to the line you cited above.
> >>
> > 
> > That is because you chose to have CONFIG_IPIPE_HOSTRT and
> > CONFIG_HAVE_IPIPE_HOSTRT. I would have only defined the latter, the way
> > you define the former. I'm looking for the hostrt support to be compiled
> > in if CONFIG_HAVE_IPIPE_HOSTRT is available from the arch-dep section,
> > so we don't need CONFIG_IPIPE_HOSTRT. Generic bits may depend on HAVE_*
> > as well.
> 
> First of all, the code you cited _above_ is not changed by our patches,
> so the context still puzzles me (but maybe you are referring to some
> other place in fact).

Patch v2 says:

diff --git a/kernel/ipipe/Kconfig b/kernel/ipipe/Kconfig
index de5e6a3..bc7a00c 100644
--- a/kernel/ipipe/Kconfig
+++ b/kernel/ipipe/Kconfig
@@ -33,3 +33,10 @@ config IPIPE_UNMASKED_CONTEXT_SWITCH
        bool
        depends on IPIPE
        default n
+
+config HAVE_IPIPE_HOSTRT
+       bool
+
+config IPIPE_HOSTRT
+       def_bool y
+       depends on HAVE_IPIPE_HOSTRT && IPIPE

So what's your point?

> 
> Second, CONFIG_HAVE_IPIPE_HOSTRT is designed to be set independently of
> CONFIG_IPIPE - it's a static arch feature like all the other
> CONFIG_HAVE_* in arch/*/Kconfig. So it takes a second, generically
> defined CONFIG switch if the generic support also depends on
> CONFIG_IPIPE like in this case.

Which does not make any sense. We don't want to make this selectable at
all. Mainline has CONFIG_HAVE_SYSCALL_WRAPPERS for instance, and you
won't find any CONFIG_SYSCALL_WRAPPERS, because it makes no sense not to
use them when the architecture _have_ them. It goes exactly the same way
with hostrt.

>  That's a kernel convention we follow. If
> you want us to do it I-pipe-specific, no problem, I just want to have
> this pointed out.

This is no ipipe-specific. It's just sensible.

> 
> Jan
> 

-- 
Philippe.