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

[Keller, Jacob E]

> -----Original Message-----
> From: Arnd Bergmann <arnd@xxxxxxxxxx>
> Sent: Monday, August 02, 2021 1:32 PM
> To: Keller, Jacob E <jacob.e.keller@xxxxxxxxx>
> Cc: Richard Cochran <richardcochran@xxxxxxxxx>; Nicolas Pitre
> <nicolas.pitre@xxxxxxxxxx>; Brandeburg, Jesse <jesse.brandeburg@xxxxxxxxx>;
> Nguyen, Anthony L <anthony.l.nguyen@xxxxxxxxx>; David S. Miller
> <davem@xxxxxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; Arnd Bergmann
> <arnd@xxxxxxxx>; Kurt Kanzenbach <kurt@xxxxxxxxxxxxx>; Saleem, Shiraz
> <shiraz.saleem@xxxxxxxxx>; Ertman, David M <david.m.ertman@xxxxxxxxx>;
> intel-wired-lan@xxxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> 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 <jacob.e.keller@xxxxxxxxx>
> wrote:
> >
> > 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