Philippe Gerum wrote:
> On Thu, 2010-08-26 at 11:38 +0200, Jan Kiszka wrote:
>> Philippe Gerum wrote:
>>> On Wed, 2010-08-25 at 12:25 +0200, Jan Kiszka wrote:
>>>> Philippe Gerum wrote:
>>>>> 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.
>>>> Don't find your example.
>>> I just did:
>>>  find . -name 'Kconfig*' -print |xargs grep SYSCALL_WRAPPERS
>> Ah, now I see.
>>
>>>>  But maybe you should have a look at
>>>> [HAVE_]USER_RETURN_NOTIFIER (and maybe I should push [HAVE_]IPIPE_HOSTRT
>>>> into arch/Kconfig).
>>> And select it conditionally on IPIPE in arch/x86/Kconfig? why not.
>> I can change this if you want us to, but I think it would be better to
>> have the generic dependency on IPIPE in a generic Kconfig - not every
>> arch version.
> 
> What I mean is:
> 
> diff --git a/arch/Kconfig b/arch/Kconfig
> index acda512..1b87a53 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -151,4 +151,7 @@ config HAVE_MIXED_BREAKPOINTS_REGS
>  config HAVE_USER_RETURN_NOTIFIER
>       bool
>  
> +config HAVE_IPIPE_HOSTRT
> +       bool
> +
>  source "kernel/gcov/Kconfig"
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index dcb0593..96ab0f4 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -58,6 +58,7 @@ config X86
>       select ANON_INODES
>       select HAVE_ARCH_KMEMCHECK
>       select HAVE_USER_RETURN_NOTIFIER
> +     select HAVE_IPIPE_HOSTRT if IPIPE
>  
>  config INSTRUCTION_DECODER
>       def_bool (KPROBES || PERF_EVENTS)
> 

I fully understood what you meant, and to end this discussion I already
implemented it. We will just have to make sure that every arch properly
adds the required "if IPIPE" suffix.

Jan


-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

_______________________________________________
Adeos-main mailing list
[email protected]
https://mail.gna.org/listinfo/adeos-main

Reply via email to