All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Keller, Jacob E" <jacob.e.keller@intel.com>
To: Richard Cochran <richardcochran@gmail.com>,
	Arnd Bergmann <arnd@kernel.org>
Cc: Nicolas Pitre <nicolas.pitre@linaro.org>,
	"Brandeburg, Jesse" <jesse.brandeburg@intel.com>,
	"Nguyen, Anthony L" <anthony.l.nguyen@intel.com>,
	"David S. Miller" <davem@davemloft.net>,
	"Jakub Kicinski" <kuba@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
	Kurt Kanzenbach <kurt@linutronix.de>,
	"Saleem, Shiraz" <shiraz.saleem@intel.com>,
	"Ertman, David M" <david.m.ertman@intel.com>,
	"intel-wired-lan@lists.osuosl.org"
	<intel-wired-lan@lists.osuosl.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net-next v2] ethernet/intel: fix PTP_1588_CLOCK dependencies
Date: Mon, 2 Aug 2021 19:54:20 +0000	[thread overview]
Message-ID: <bd631e36-1701-b120-a9b0-8825d14cc694@intel.com> (raw)
In-Reply-To: <20210802164907.GA9832@hoboy.vegasvil.org>

On 8/2/2021 9:49 AM, Richard Cochran wrote:
> On Mon, Aug 02, 2021 at 04:59:33PM +0200, Arnd Bergmann wrote:
> 
>> This is a recurring problem in many drivers, and we have discussed
>> it several times befores, without reaching a consensus. I'm providing
>> a link to the previous email thread for reference, which discusses
>> some related problems, though I can't find what reasons there were
>> against the approach with the extra Kconfig dependency.
> 
> Quoting myself in the thread from 12 Nov 2020:
> 
>    This whole "implies" thing turned out to be a colossal PITA.
> 
>    I regret the fact that it got merged.  It wasn't my idea.
> 
> This whole thing came about because Nicolas Pitre wanted to make PHC
> core support into a module and also to be able to remove dynamic posix
> clock support for tinification.  It has proved to be a never ending
> source of confusion.
> 
> Let's restore the core functionality and remove "implies" for good.
> 
> Thanks,
> Richard
> 

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!)

Thanks,
Jake


WARNING: multiple messages have this Message-ID (diff)
From: Keller, Jacob E <jacob.e.keller@intel.com>
To: intel-wired-lan@osuosl.org
Subject: [Intel-wired-lan] [PATCH net-next v2] ethernet/intel: fix PTP_1588_CLOCK dependencies
Date: Mon, 2 Aug 2021 19:54:20 +0000	[thread overview]
Message-ID: <bd631e36-1701-b120-a9b0-8825d14cc694@intel.com> (raw)
In-Reply-To: <20210802164907.GA9832@hoboy.vegasvil.org>

On 8/2/2021 9:49 AM, Richard Cochran wrote:
> On Mon, Aug 02, 2021 at 04:59:33PM +0200, Arnd Bergmann wrote:
> 
>> This is a recurring problem in many drivers, and we have discussed
>> it several times befores, without reaching a consensus. I'm providing
>> a link to the previous email thread for reference, which discusses
>> some related problems, though I can't find what reasons there were
>> against the approach with the extra Kconfig dependency.
> 
> Quoting myself in the thread from 12 Nov 2020:
> 
>    This whole "implies" thing turned out to be a colossal PITA.
> 
>    I regret the fact that it got merged.  It wasn't my idea.
> 
> This whole thing came about because Nicolas Pitre wanted to make PHC
> core support into a module and also to be able to remove dynamic posix
> clock support for tinification.  It has proved to be a never ending
> source of confusion.
> 
> Let's restore the core functionality and remove "implies" for good.
> 
> Thanks,
> Richard
> 

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!)

Thanks,
Jake


  reply	other threads:[~2021-08-02 19:54 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-02 14:59 [PATCH net-next v2] ethernet/intel: fix PTP_1588_CLOCK dependencies Arnd Bergmann
2021-08-02 14:59 ` [Intel-wired-lan] " Arnd Bergmann
2021-08-02 16:49 ` Richard Cochran
2021-08-02 16:49   ` [Intel-wired-lan] " Richard Cochran
2021-08-02 19:54   ` Keller, Jacob E [this message]
2021-08-02 19:54     ` Keller, Jacob E
2021-08-02 20:32     ` Arnd Bergmann
2021-08-02 20:32       ` [Intel-wired-lan] " Arnd Bergmann
2021-08-02 20:46       ` Keller, Jacob E
2021-08-02 20:46         ` [Intel-wired-lan] " Keller, Jacob E
2021-08-02 20:59         ` Arnd Bergmann
2021-08-02 20:59           ` [Intel-wired-lan] " Arnd Bergmann
2021-08-02 21:09           ` Keller, Jacob E
2021-08-02 21:09             ` [Intel-wired-lan] " Keller, Jacob E
2021-08-02 21:10           ` Keller, Jacob E
2021-08-02 21:10             ` [Intel-wired-lan] " Keller, Jacob E
2021-08-02 21:22           ` Nicolas Pitre
2021-08-02 21:22             ` [Intel-wired-lan] " Nicolas Pitre
2021-08-03 20:29       ` Arnd Bergmann
2021-08-03 20:29         ` [Intel-wired-lan] " Arnd Bergmann
2021-08-02 23:09     ` Richard Cochran
2021-08-02 23:09       ` [Intel-wired-lan] " Richard Cochran
2021-08-02 23:45       ` Keller, Jacob E
2021-08-02 23:45         ` [Intel-wired-lan] " Keller, Jacob E
2021-08-03  0:03         ` Richard Cochran
2021-08-03  0:03           ` [Intel-wired-lan] " Richard Cochran
2021-08-03  6:59       ` Arnd Bergmann
2021-08-03  6:59         ` [Intel-wired-lan] " Arnd Bergmann
2021-08-03 15:55         ` Richard Cochran
2021-08-03 15:55           ` [Intel-wired-lan] " Richard Cochran
2021-08-03 16:14           ` Richard Cochran
2021-08-03 16:14             ` [Intel-wired-lan] " Richard Cochran
2021-08-03 17:00             ` Arnd Bergmann
2021-08-03 17:00               ` [Intel-wired-lan] " Arnd Bergmann
2021-08-03 17:18               ` Keller, Jacob E
2021-08-03 17:18                 ` [Intel-wired-lan] " Keller, Jacob E
2021-08-03 18:27                 ` Arnd Bergmann
2021-08-03 18:27                   ` [Intel-wired-lan] " Arnd Bergmann
2021-08-03 23:25                   ` Keller, Jacob E
2021-08-03 23:25                     ` [Intel-wired-lan] " Keller, Jacob E
2021-08-04 11:18                   ` Arnd Bergmann
2021-08-04 11:18                     ` [Intel-wired-lan] " Arnd Bergmann
2021-08-03 20:54               ` Richard Cochran
2021-08-03 20:54                 ` [Intel-wired-lan] " Richard Cochran
2021-08-04 20:53                 ` Keller, Jacob E
2021-08-04 20:53                   ` [Intel-wired-lan] " Keller, Jacob E

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bd631e36-1701-b120-a9b0-8825d14cc694@intel.com \
    --to=jacob.e.keller@intel.com \
    --cc=anthony.l.nguyen@intel.com \
    --cc=arnd@arndb.de \
    --cc=arnd@kernel.org \
    --cc=davem@davemloft.net \
    --cc=david.m.ertman@intel.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=jesse.brandeburg@intel.com \
    --cc=kuba@kernel.org \
    --cc=kurt@linutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=nicolas.pitre@linaro.org \
    --cc=richardcochran@gmail.com \
    --cc=shiraz.saleem@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.