Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
Feedback-ID: i56a14606:Fastmail
User-Agent: Cyrus-JMAP/3.9.0-alpha0-745-g95dd7bea33-fm-20230905.001-g95dd7bea
Mime-Version: 1.0
Message-Id: <b234530c-88fe-4a2a-993c-f1733fe4d0c1@app.fastmail.com>
In-Reply-To: <ZQISGujwlH00B8KJ@fjasle.eu>
References: <20230913113801.1901152-1-arnd@kernel.org>
 <ZQISGujwlH00B8KJ@fjasle.eu>
Date:   Wed, 13 Sep 2023 21:55:36 +0200
From:   "Arnd Bergmann" <arnd@arndb.de>
To:     "Nicolas Schier" <nicolas@fjasle.eu>,
        "Arnd Bergmann" <arnd@kernel.org>
Cc:     "Masahiro Yamada" <masahiroy@kernel.org>,
        "Jonathan Corbet" <corbet@lwn.net>,
        "Sakari Ailus" <sakari.ailus@iki.fi>,
        "Javier Martinez Canillas" <javierm@redhat.com>,
        "Nathan Chancellor" <nathan@kernel.org>,
        "Nick Desaulniers" <ndesaulniers@google.com>,
        linux-kbuild@vger.kernel.org, linux-doc@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Documentation: kbuild: explain handling optional dependencies
Content-Type: text/plain
Precedence: bulk

On Wed, Sep 13, 2023, at 21:48, Nicolas Schier wrote:
> On Wed, Sep 13, 2023 at 01:37:52PM +0200 Arnd Bergmann wrote:
>
>>  Documentation/kbuild/kconfig-language.rst | 26 +++++++++++++++++++++++
>>  1 file changed, 26 insertions(+)
>> 
>> diff --git a/Documentation/kbuild/kconfig-language.rst b/Documentation/kbuild/kconfig-language.rst
>> index 858ed5d80defe..89dea587a469a 100644
>> --- a/Documentation/kbuild/kconfig-language.rst
>> +++ b/Documentation/kbuild/kconfig-language.rst
>> @@ -573,6 +573,32 @@ above, leading to:
>>  	bool "Support for foo hardware"
>>  	depends on ARCH_FOO_VENDOR || COMPILE_TEST
>>  
>> +Optional dependencies
>> +~~~~~~~~~~~~~~~~~~~~~
>> +
>> +Some drivers are able to optionally use a feature from another module
>> +or build cleanly with that module disabled, but cause a link failure
>> +when trying to use that loadable module from a built-in driver.
>> +
>> +The most common way to express this optional dependency in Kconfig logic
>> +uses the slighly counterintuitive
>
> slighly -> slightly

Fixed, thanks

> For better RST compliance: could you explicitly start the code block e.g. by
> appending '::' as in "... counterintuitive::"?

Ok, done.

>> +
>> +  config FOO
>> +	bool "Support for foo hardware"
>> +	depends on BAR || !BAR
>
> are you sure that this is enough?  While testing, I needed to explicitly use
> =y|=n:
>
>     depends on BAR=y || BAR=n
>
> to prevent FOO to be selectable iff BAR=m.

I see my problem, I made a different mistake here. Your version
is correct for a 'bool' symbol as I had here, but the intention
of this was to make it work for tristate symbols, which are the
interesting case. I've fixed it up this way now, hope it now makes
sense to you:

--- a/Documentation/kbuild/kconfig-language.rst
+++ b/Documentation/kbuild/kconfig-language.rst
@@ -581,19 +581,19 @@ or build cleanly with that module disabled, but cause a link failure
 when trying to use that loadable module from a built-in driver.
 
 The most common way to express this optional dependency in Kconfig logic
-uses the slighly counterintuitive
+uses the slightly counterintuitive::
 
   config FOO
-       bool "Support for foo hardware"
+       tristate "Support for foo hardware"
        depends on BAR || !BAR
 
 This means that there is either a dependency on BAR that disallows
 the combination of FOO=y with BAR=m, or BAR is completely disabled.
 For a more formalized approach if there are multiple drivers that have
-the same dependency, a helper symbol can be used, like
+the same dependency, a helper symbol can be used, like::
 
   config FOO
-       bool "Support for foo hardware"
+       tristate "Support for foo hardware"
        depends on BAR_OPTIONAL
 
   config BAR_OPTIONAL

>> +This means that there is either a dependency on BAR that disallows
>> +the combination of FOO=y with BAR=m, or BAR is completely disabled.
>
> For me, this sentence is hard to parse (but I am not a native speaker); what
> about something like this:
>
> This means that FOO can only be enabled, iff BAR is either built-in or
> completely disabled.  If BAR is built as a module, FOO cannot be enabled.

That would describe the version you suggested, but that's a
different issue. Let me know if you still think it needs
clarification after fixing the example.

      Arnd