Re: [Xen-devel] [PATCHv6] 08/28] build: convert HAS_IOPORTS use to Kconfig

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 03.12.15 at 01:39, <cardoe@xxxxxxxxxx> wrote:
> On 11/30/15 9:01 AM, Jan Beulich wrote:
>>>>> On 30.11.15 at 15:56, <JBeulich@xxxxxxxx> wrote:
>>>>>> On 24.11.15 at 18:51, <cardoe@xxxxxxxxxx> wrote:
>>>> --- a/xen/drivers/char/Kconfig
>>>> +++ b/xen/drivers/char/Kconfig
>>>> @@ -3,3 +3,8 @@ config UART_NS16550
>>>>    bool "16550-series UART support"
>>>>    help
>>>>      This selects the 16550-series UART support. For most systems, say Y.
>>>> +
>>>> +# Select HAS_IOPORTS if 16550 has I/O ports
>>>> +config HAS_IOPORTS
>>>> +  bool
>>>> +  depends on UART_NS16550
>>>
>>> Even if only used in ns16550.c, this is a generic setting and hence
>>> belongs in common/Kconfig and should be selected by the arches
>>> as needed.
>> 
>> I see it is being selected, but the "depends" here is bogus: When
>> there is no prompt and no default, there should also not be any
>> dependency.
> 
> The dependency just means it can't be enabled unless the dependency is
> met.

I don't think that's true. All you get when "select"-ing it without
also having selected the dependency is a warning. I've frequently
seen this on Linux - people just don't seem to look at those
warnings (and I admit they're not always easy to spot, since they
might scroll off before one even has a chance to look).

> I believe its valid for the case when the config system starts and
> the user disables UART_NS16550, then HAS_IOPORTS would need to be
> disabled. Without the dependency then this wouldn't be turned off when
> UART_NS16550 is turned off. There are a number of examples of this in
> the Linux kernel's usage of kconfig.

Question is whether these are good examples, which I doubt. In fact
I recall discussions to that effect. Forward dependencies make sense
when controlling user selectable options (i.e. such with visible prompts).
Reverse dependencies should be used when dealing with automatic
selections (i.e. options without visible prompts).

But regardless of these abstract considerations the dependency you
introduce above is (as said) wrong anyway: HAS_IOPORTS is a
property or the architecture, not something that results from enabling
UART_NS16550. You simply got misled by ns16550.c being the only
file using that symbol.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel