Re: [PATCH] Remove remaining references of CONFIG_GENERIC_TIME

Re: [PATCH] Remove remaining references of CONFIG_GENERIC_TIME

[Geert Uytterhoeven]

On Mon, Aug 1, 2011 at 12:14, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Aug 01, 2011 at 12:09:02PM +0200, Geert Uytterhoeven wrote:
>> On Mon, Aug 1, 2011 at 10:36, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > On Sat, Jul 30, 2011 at 06:14:38PM +0200, Zoltan Devai wrote:
>> >> Commit 592913ecb87a9e06f98ddb55b298f1a66bf94c6b has killed off any
>> >> use of this config option long ago.
>> >
>> > I don't see the point of this - we were free of GENERIC_TIME on ARM
>> > shortly after it was originally killed off.  The problem is you can't
>> > stop people introducing new uses of this - because it existed once and
>> > there's nothing which errors out on its presence, people are going to
>> > continue submitting patches with it in.  And it's going to continue
>> > being missed at the review stage.
>> >
>> > I've a similar problem with folk on ARM including mach/gpio.h as their
>> > sole gpio header file rather than linux/gpio.h - I've been trying for
>> > the last 1-2 years to educate people to use linux/ in preference.  You
>> > can't do it, and I'm still just about the only one who picks up on that.
>> > (SoC maintainers don't care.)  They will end up caring when I push a
>> > change during the next merge window though, so I'll eventually stop
>> > mach/gpio.h being included.  (Instead, it'll be asm/gpio.h).
>> >
>> > GENERIC_TIME though... I don't think you'll ever stop new uses of it
>> > creeping in unless you can arrange for something to error out.
>>
>> Doesn't kconf error out when trying to select a non-existent symbol?
>
> Nope.

You're right. So that's a bug.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] Remove remaining references of CONFIG_GENERIC_TIME

[Uwe Kleine-König]

On Mon, Aug 01, 2011 at 01:27:52PM +0200, Geert Uytterhoeven wrote:
> On Mon, Aug 1, 2011 at 12:14, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Mon, Aug 01, 2011 at 12:09:02PM +0200, Geert Uytterhoeven wrote:
> >> Doesn't kconf error out when trying to select a non-existent symbol?
> >
> > Nope.
> 
> You're right. So that's a bug.
Not sure, this might have been done on purpose to make cross-tree series
easier!?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH] Remove remaining references of CONFIG_GENERIC_TIME

[Arnaud Lacombe]

Hi,

On Mon, Aug 1, 2011 at 7:27 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Mon, Aug 1, 2011 at 12:14, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>> On Mon, Aug 01, 2011 at 12:09:02PM +0200, Geert Uytterhoeven wrote:
>>> On Mon, Aug 1, 2011 at 10:36, Russell King - ARM Linux
>>> <linux@arm.linux.org.uk> wrote:
>>> > On Sat, Jul 30, 2011 at 06:14:38PM +0200, Zoltan Devai wrote:
>>> >> Commit 592913ecb87a9e06f98ddb55b298f1a66bf94c6b has killed off any
>>> >> use of this config option long ago.
>>> >
>>> > I don't see the point of this - we were free of GENERIC_TIME on ARM
>>> > shortly after it was originally killed off.  The problem is you can't
>>> > stop people introducing new uses of this - because it existed once and
>>> > there's nothing which errors out on its presence, people are going to
>>> > continue submitting patches with it in.  And it's going to continue
>>> > being missed at the review stage.
>>> >
>>> > I've a similar problem with folk on ARM including mach/gpio.h as their
>>> > sole gpio header file rather than linux/gpio.h - I've been trying for
>>> > the last 1-2 years to educate people to use linux/ in preference.  You
>>> > can't do it, and I'm still just about the only one who picks up on that.
>>> > (SoC maintainers don't care.)  They will end up caring when I push a
>>> > change during the next merge window though, so I'll eventually stop
>>> > mach/gpio.h being included.  (Instead, it'll be asm/gpio.h).
>>> >
>>> > GENERIC_TIME though... I don't think you'll ever stop new uses of it
>>> > creeping in unless you can arrange for something to error out.
>>>
>>> Doesn't kconf error out when trying to select a non-existent symbol?
>>
>> Nope.
>
> You're right. So that's a bug.
>
depends on what you are trying to achieve and what the problem is.

Internally kconfig will create a dummy symbol when it encounter a
missing symbol so that arch/arm/Kconfig can reference a symbol which
will be fully defined later on. I do not think you want to forward
decl all the symbol which can be used. That'd be a mess. That said, we
can come with a form of symbol deprecation that would error-out when
used.

 - Arnaud

Re: [PATCH] Remove remaining references of CONFIG_GENERIC_TIME

[Kyle Moffett]

On Mon, Aug 1, 2011 at 11:04, Arnaud Lacombe <lacombar@gmail.com> wrote:
> Hi,
>
> On Mon, Aug 1, 2011 at 7:27 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>> On Mon, Aug 1, 2011 at 12:14, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>>> On Mon, Aug 01, 2011 at 12:09:02PM +0200, Geert Uytterhoeven wrote:
>>>> On Mon, Aug 1, 2011 at 10:36, Russell King - ARM Linux
>>>> <linux@arm.linux.org.uk> wrote:
>>>> > On Sat, Jul 30, 2011 at 06:14:38PM +0200, Zoltan Devai wrote:
>>>> >> Commit 592913ecb87a9e06f98ddb55b298f1a66bf94c6b has killed off any
>>>> >> use of this config option long ago.
>>>> >
>>>> > I don't see the point of this - we were free of GENERIC_TIME on ARM
>>>> > shortly after it was originally killed off.  The problem is you can't
>>>> > stop people introducing new uses of this - because it existed once and
>>>> > there's nothing which errors out on its presence, people are going to
>>>> > continue submitting patches with it in.  And it's going to continue
>>>> > being missed at the review stage.
>>>> >
>>>> > I've a similar problem with folk on ARM including mach/gpio.h as their
>>>> > sole gpio header file rather than linux/gpio.h - I've been trying for
>>>> > the last 1-2 years to educate people to use linux/ in preference.  You
>>>> > can't do it, and I'm still just about the only one who picks up on that.
>>>> > (SoC maintainers don't care.)  They will end up caring when I push a
>>>> > change during the next merge window though, so I'll eventually stop
>>>> > mach/gpio.h being included.  (Instead, it'll be asm/gpio.h).
>>>> >
>>>> > GENERIC_TIME though... I don't think you'll ever stop new uses of it
>>>> > creeping in unless you can arrange for something to error out.
>>>>
>>>> Doesn't kconf error out when trying to select a non-existent symbol?
>>>
>>> Nope.
>>
>> You're right. So that's a bug.
>>
> depends on what you are trying to achieve and what the problem is.
>
> Internally kconfig will create a dummy symbol when it encounter a
> missing symbol so that arch/arm/Kconfig can reference a symbol which
> will be fully defined later on. I do not think you want to forward
> decl all the symbol which can be used. That'd be a mess. That said, we
> can come with a form of symbol deprecation that would error-out when
> used.

Would it be possible instead to make Kconfig go through all the symbols
after everything is processed and identify any remaining "dummy symbols"
which were not actually declared anywhere?

Right now if you typo a "select" statement you get no warnings that you
are selecting something that does not exist, which is probably a cause
of many kinds of errors beyond this particular one.

Cheers,
Kyle Moffett

Re: [PATCH] Remove remaining references of CONFIG_GENERIC_TIME

[Arnaud Lacombe]

Hi,

On Mon, Aug 1, 2011 at 11:15 AM, Kyle Moffett <kyle@moffetthome.net> wrote:
> On Mon, Aug 1, 2011 at 11:04, Arnaud Lacombe <lacombar@gmail.com> wrote:
>> Hi,
>>
>> On Mon, Aug 1, 2011 at 7:27 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>> On Mon, Aug 1, 2011 at 12:14, Russell King - ARM Linux
>>>>>[...]
>>>>>
>>>>> Doesn't kconf error out when trying to select a non-existent symbol?
>>>>
>>>> Nope.
>>>
>>> You're right. So that's a bug.
>>>
>> depends on what you are trying to achieve and what the problem is.
>>
>> Internally kconfig will create a dummy symbol when it encounter a
>> missing symbol so that arch/arm/Kconfig can reference a symbol which
>> will be fully defined later on. I do not think you want to forward
>> decl all the symbol which can be used. That'd be a mess. That said, we
>> can come with a form of symbol deprecation that would error-out when
>> used.
>
> Would it be possible instead to make Kconfig go through all the symbols
> after everything is processed and identify any remaining "dummy symbols"
> which were not actually declared anywhere?
>
this is software, everything is possible.

> Right now if you typo a "select" statement you get no warnings that you
> are selecting something that does not exist, which is probably a cause
> of many kinds of errors beyond this particular one.
>
d'oh! ... I'm not sure we want that. Dummy symbol are heavily used
internally, a trivial implementation[0] triggered:

% make REGENERATE_PARSERS=y alldefconfig 2>&1 | grep 'defined without
type' | wc -l
817

Moreover, this approach is deemed to fail. The current symbol
namespace is tied to an arch, so whenever you do:

arch/arm/Kconfig:
config FOO
    bool

config BAZ
    bool

drivers/cpufreq/Kconfig
config BAR
    depends on ARM && FOO
    select BAZ

You will end up triggering the warning for every ARCH != ARM...

 - Arnaud

[0]: or rather a fix as we currently only do the check for symbol linked to menu

Re: [PATCH] Remove remaining references of CONFIG_GENERIC_TIME

[Nicolas Pitre]

On Mon, 1 Aug 2011, Arnaud Lacombe wrote:

> Moreover, this approach is deemed to fail. The current symbol
> namespace is tied to an arch, so whenever you do:
> 
> arch/arm/Kconfig:
> config FOO
>     bool
> 
> config BAZ
>     bool
> 
> drivers/cpufreq/Kconfig
> config BAR
>     depends on ARM && FOO
>     select BAZ
> 
> You will end up triggering the warning for every ARCH != ARM...

You can keep a state for those symbols with regard to their level of 
reference.  Surely if ARM isn't true, BAZ is not "actively" referenced 
in the end.


Nicolas

Re: [PATCH] Remove remaining references of CONFIG_GENERIC_TIME

[Arnaud Lacombe]

Hi,

On Mon, Aug 1, 2011 at 12:40 PM, Nicolas Pitre <nico@fluxnic.net> wrote:
> On Mon, 1 Aug 2011, Arnaud Lacombe wrote:
>
>> Moreover, this approach is deemed to fail. The current symbol
>> namespace is tied to an arch, so whenever you do:
>>
>> arch/arm/Kconfig:
>> config FOO
>>     bool
>>
>> config BAZ
>>     bool
>>
>> drivers/cpufreq/Kconfig
>> config BAR
>>     depends on ARM && FOO
>>     select BAZ
>>
>> You will end up triggering the warning for every ARCH != ARM...
>
> You can keep a state for those symbols with regard to their level of
> reference.  Surely if ARM isn't true, BAZ is not "actively" referenced
> in the end.
>
he :) we enter in compilers optimizations. As much as I agree with
you, that might not be trivial to implement given the internal,
ad-hoc, structure of kconfig. All of that would be useless if we had a
single global namespace.

 - Arnaud

Re: [PATCH] Remove remaining references of CONFIG_GENERIC_TIME

[Kyle Moffett]

On Mon, Aug 1, 2011 at 12:08, Arnaud Lacombe <lacombar@gmail.com> wrote:
> On Mon, Aug 1, 2011 at 11:15 AM, Kyle Moffett <kyle@moffetthome.net> wrote:
>> On Mon, Aug 1, 2011 at 11:04, Arnaud Lacombe <lacombar@gmail.com> wrote:
>>> On Mon, Aug 1, 2011 at 7:27 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>>> On Mon, Aug 1, 2011 at 12:14, Russell King - ARM Linux
>>>>>>[...]
>>>>>>
>>>>>> Doesn't kconf error out when trying to select a non-existent symbol?
>>>>>
>>>>> Nope.
>>>>
>>>> You're right. So that's a bug.
>>>>
>>> depends on what you are trying to achieve and what the problem is.
>>>
>>> Internally kconfig will create a dummy symbol when it encounter a
>>> missing symbol so that arch/arm/Kconfig can reference a symbol which
>>> will be fully defined later on. I do not think you want to forward
>>> decl all the symbol which can be used. That'd be a mess. That said, we
>>> can come with a form of symbol deprecation that would error-out when
>>> used.
>>
>> Would it be possible instead to make Kconfig go through all the symbols
>> after everything is processed and identify any remaining "dummy symbols"
>> which were not actually declared anywhere?
>>
> this is software, everything is possible.
>
>> Right now if you typo a "select" statement you get no warnings that you
>> are selecting something that does not exist, which is probably a cause
>> of many kinds of errors beyond this particular one.
>>
> d'oh! ... I'm not sure we want that. Dummy symbol are heavily used
> internally, a trivial implementation[0] triggered:
>
> % make REGENERATE_PARSERS=y alldefconfig 2>&1 | grep 'defined without
> type' | wc -l
> 817
>
> Moreover, this approach is deemed to fail. The current symbol
> namespace is tied to an arch, so whenever you do:
>
> arch/arm/Kconfig:
> config FOO
>    bool
>
> config BAZ
>    bool
>
> drivers/cpufreq/Kconfig
> config BAR
>    depends on ARM && FOO
>    select BAZ
>
> You will end up triggering the warning for every ARCH != ARM...

Perhaps that's an argument for building a single Kconfig namespace?
At the very least, most of the architecture-generic warnings could be
worked around with trivial predeclarations in an "arch/Kconfig" file:

config ARM
        bool
[...]
config X86
        bool

There's probably technical issues that I'm missing that would make
that inordinately painful, but I can't see them right now... any ideas?

Cheers,
Kyle Moffett

Re: [PATCH] Remove remaining references of CONFIG_GENERIC_TIME

[Arnaud Lacombe]

Hi,

On Mon, Aug 1, 2011 at 12:48 PM, Kyle Moffett <kyle@moffetthome.net> wrote:
> On Mon, Aug 1, 2011 at 12:08, Arnaud Lacombe <lacombar@gmail.com> wrote:
>> On Mon, Aug 1, 2011 at 11:15 AM, Kyle Moffett <kyle@moffetthome.net> wrote:
>>> On Mon, Aug 1, 2011 at 11:04, Arnaud Lacombe <lacombar@gmail.com> wrote:
>>>> On Mon, Aug 1, 2011 at 7:27 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>>>> On Mon, Aug 1, 2011 at 12:14, Russell King - ARM Linux
>>>>>>>[...]
>>>>>>>
>>>>>>> Doesn't kconf error out when trying to select a non-existent symbol?
>>>>>>
>>>>>> Nope.
>>>>>
>>>>> You're right. So that's a bug.
>>>>>
>>>> depends on what you are trying to achieve and what the problem is.
>>>>
>>>> Internally kconfig will create a dummy symbol when it encounter a
>>>> missing symbol so that arch/arm/Kconfig can reference a symbol which
>>>> will be fully defined later on. I do not think you want to forward
>>>> decl all the symbol which can be used. That'd be a mess. That said, we
>>>> can come with a form of symbol deprecation that would error-out when
>>>> used.
>>>
>>> Would it be possible instead to make Kconfig go through all the symbols
>>> after everything is processed and identify any remaining "dummy symbols"
>>> which were not actually declared anywhere?
>>>
>> this is software, everything is possible.
>>
>>> Right now if you typo a "select" statement you get no warnings that you
>>> are selecting something that does not exist, which is probably a cause
>>> of many kinds of errors beyond this particular one.
>>>
>> d'oh! ... I'm not sure we want that. Dummy symbol are heavily used
>> internally, a trivial implementation[0] triggered:
>>
>> % make REGENERATE_PARSERS=y alldefconfig 2>&1 | grep 'defined without
>> type' | wc -l
>> 817
>>
>> Moreover, this approach is deemed to fail. The current symbol
>> namespace is tied to an arch, so whenever you do:
>>
>> arch/arm/Kconfig:
>> config FOO
>>    bool
>>
>> config BAZ
>>    bool
>>
>> drivers/cpufreq/Kconfig
>> config BAR
>>    depends on ARM && FOO
>>    select BAZ
>>
>> You will end up triggering the warning for every ARCH != ARM...
>
> Perhaps that's an argument for building a single Kconfig namespace?
I'm working on that, but this is a real mess :)

> At the very least, most of the architecture-generic warnings could be
> worked around with trivial predeclarations in an "arch/Kconfig" file:
>
> config ARM
>        bool
> [...]
> config X86
>        bool
>
> There's probably technical issues that I'm missing that would make
> that inordinately painful, but I can't see them right now... any ideas?
>
not I directly forsee.

 - Arnaud

> Cheers,
> Kyle Moffett
>

Re: [PATCH] Remove remaining references of CONFIG_GENERIC_TIME

[Arnaud Lacombe]

Hi,

On Mon, Aug 1, 2011 at 12:08 PM, Arnaud Lacombe <lacombar@gmail.com> wrote:
> Hi,
>
> On Mon, Aug 1, 2011 at 11:15 AM, Kyle Moffett <kyle@moffetthome.net> wrote:
>> On Mon, Aug 1, 2011 at 11:04, Arnaud Lacombe <lacombar@gmail.com> wrote:
>>> Hi,
>>>
>>> On Mon, Aug 1, 2011 at 7:27 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>>> On Mon, Aug 1, 2011 at 12:14, Russell King - ARM Linux
>>>>>>[...]
>>>>>>
>>>>>> Doesn't kconf error out when trying to select a non-existent symbol?
>>>>>
>>>>> Nope.
>>>>
>>>> You're right. So that's a bug.
>>>>
>>> depends on what you are trying to achieve and what the problem is.
>>>
>>> Internally kconfig will create a dummy symbol when it encounter a
>>> missing symbol so that arch/arm/Kconfig can reference a symbol which
>>> will be fully defined later on. I do not think you want to forward
>>> decl all the symbol which can be used. That'd be a mess. That said, we
>>> can come with a form of symbol deprecation that would error-out when
>>> used.
>>
>> Would it be possible instead to make Kconfig go through all the symbols
>> after everything is processed and identify any remaining "dummy symbols"
>> which were not actually declared anywhere?
>>
> this is software, everything is possible.
>
>> Right now if you typo a "select" statement you get no warnings that you
>> are selecting something that does not exist, which is probably a cause
>> of many kinds of errors beyond this particular one.
>>
> d'oh! ... I'm not sure we want that. Dummy symbol are heavily used
> internally, a trivial implementation[0] triggered:
>
> % make REGENERATE_PARSERS=y alldefconfig 2>&1 | grep 'defined without
> type' | wc -l
> 817
>
> Moreover, this approach is deemed to fail. The current symbol
> namespace is tied to an arch, so whenever you do:
>
> arch/arm/Kconfig:
> config FOO
>    bool
>
> config BAZ
>    bool
>
> drivers/cpufreq/Kconfig
> config BAR
>    depends on ARM && FOO
>    select BAZ
>
> You will end up triggering the warning for every ARCH != ARM...
>
Ok, I restricted the tagging to selected symbol, and we definitively
have this pattern occurring. It shall be fixed when/if we ends up
having a global configuration namespace.

I think I can work a patch.

 - Arnaud