RE: [PATCH net-next v2] ethernet/intel: fix PTP_1588_CLOCK dependencies

-----Original Message-----
From: Arnd Bergmann <arnd@kernel.org>
Sent: Monday, August 02, 2021 1:32 PM
To: Keller, Jacob E <jacob.e.keller@intel.com>
Cc: Richard Cochran <richardcochran@gmail.com>; Nicolas Pitre
[off-list ref]; Brandeburg, Jesse [off-list ref];
Nguyen, Anthony L [off-list ref]; David S. Miller
[off-list ref]; Jakub Kicinski [off-list ref]; Arnd Bergmann
[off-list ref]; Kurt Kanzenbach [off-list ref]; Saleem, Shiraz
[off-list ref]; Ertman, David M [off-list ref];
intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-
kernel@vger.kernel.org
Subject: Re: [PATCH net-next v2] ethernet/intel: fix PTP_1588_CLOCK
dependencies

On Mon, Aug 2, 2021 at 9:54 PM Keller, Jacob E [off-list ref]
wrote:

quoted

So go back to "select"?

It looks like Arnd proposed in the thread a solution that did a sort of
"please enable this" but still let you disable it.

An alternative (unfortunately per-driver...) solution was to setup the
drivers so that they gracefully fall back to disabling PTP if the PTP
core support is not reachable.. but that obviously requires that drivers
do the right thing, and at least Intel drivers have not tested this
properly.

I'm definitely in favor of removing "implies" entirely. The semantics
are unclear, and the fact that it doesn't handle the case of "i'm
builtin, so my implies can't be modules"...

I don't really like the syntax of the double "depends on A || !A".. I'd
prefer if we had some keyword for this, since it would be more obvious
and not run against the standard logic (A || !A is a tautology!)

I think the main reason we don't have a keyword for it is that nobody
so far has come up with an English word that expresses what it is
supposed to mean.

Right. I don't have a great example that's a single word either.

You can do something like it for a particular symbol though, such as

config MAY_USE_PTP_1588_CLOCK
       def_tristate PTP_1588_CLOCK || !PTP_1588_CLOCK

 config E1000E
        tristate "Intel(R) PRO/1000 PCI-Express Gigabit Ethernet support"
        depends on PCI && (!SPARC32 || BROKEN)
+       depends on MAY_USE_PTP_1588_CLOCK
        select CRC32
-       imply PTP_1588_CLOCK

What about "integrates"?

Or.. what if we just changed "implies" to also include the dependencies automatically? i.e. "implies PTP_1588_CLOCK" also means the depends trick which ensures that you can't have it as module if this is built-in.

I.e. we still get the nice "this will turn on automatically in the menu if you enable this" and we enforce that you can't have it as a module since it would be a dependency if it's on"?

          Arnd