Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: "Keller, Jacob E" <jacob.e.keller@intel.com>
Cc: Richard Cochran <richardcochran@gmail.com>,
	Nicolas Pitre <nico@fluxnic.net>,
	"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>,
	Yangbo Lu <yangbo.lu@nxp.com>
Subject: Re: [PATCH net-next v2] ethernet/intel: fix PTP_1588_CLOCK dependencies
Date: Wed, 4 Aug 2021 13:18:48 +0200	[thread overview]
Message-ID: <CAK8P3a2=pLt9iwE1xw5yhk=DA_i5HLsf5q94GQ2BCG48rS45jQ@mail.gmail.com> (raw)
In-Reply-To: <CAK8P3a06enZOf=XyZ+zcAwBczv41UuCTz+=0FMf2gBz1_cOnZQ@mail.gmail.com>

On Tue, Aug 3, 2021 at 8:27 PM Arnd Bergmann <arnd@kernel.org> wrote:
>
> On Tue, Aug 3, 2021 at 7:19 PM Keller, Jacob E <jacob.e.keller@intel.com> wrote:
> > > On Tue, Aug 3, 2021 at 6:14 PM Richard Cochran <richardcochran@gmail.com> wrote:
>
> > There is an alternative solution to fixing the imply keyword:
> >
> > Make the drivers use it properly by *actually* conditionally enabling the feature only when IS_REACHABLE, i.e. fix ice so that it uses IS_REACHABLE instead of IS_ENABLED, and so that its stub implementation in ice_ptp.h actually just silently does nothing but returns 0 to tell the rest of the driver things are fine.
>
> I would consider IS_REACHABLE() part of the problem, not the solution, it makes
> things magically build, but then surprises users at runtime when they do not get
> the intended behavior.

Case in point: two patches from Yangbo Lu that call into the ptp core
from built-in
network code look like they could never have worked with CONFIG_PTP_1588_CLOCK,
but did not cause a link failure because of the IS_REACHABLE() check, see
commit d7c088265588 ("net: socket: support hardware timestamp conversion to
PHC bound") and c156174a6707 ("ethtool: add a new command for getting PHC
virtual clocks").

I found that by testing my patch that turns the IS_REACHABLE() back into
IS_ENABLED() and got

aarch64-linux-ld: net/socket.o: in function `__sock_recv_timestamp':
socket.c:(.text+0x1f20): undefined reference to `ptp_convert_timestamp'
aarch64-linux-ld: net/ethtool/common.o: in function `ethtool_get_phc_vclocks':
common.c:(.text+0x35c): undefined reference to `ptp_get_vclocks_index'

I added a workaround to my patch now to keep it working as before, but this
needs to be fixed properly. The easiest way would be to no longer support
modular PTP at all as Richard would like to do anyway, but I have not
attempted other fixes.

       Arnd

  parent reply	other threads:[~2021-08-04 11:19 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-02 14:59 Arnd Bergmann
2021-08-02 16:49 ` Richard Cochran
2021-08-02 19:54   ` Keller, Jacob E
2021-08-02 20:32     ` Arnd Bergmann
2021-08-02 20:46       ` Keller, Jacob E
2021-08-02 20:59         ` Arnd Bergmann
2021-08-02 21:09           ` Keller, Jacob E
2021-08-02 21:10           ` Keller, Jacob E
2021-08-02 21:22           ` Nicolas Pitre
2021-08-03 20:29       ` Arnd Bergmann
2021-08-02 23:09     ` Richard Cochran
2021-08-02 23:45       ` Keller, Jacob E
2021-08-03  0:03         ` Richard Cochran
2021-08-03  6:59       ` Arnd Bergmann
2021-08-03 15:55         ` Richard Cochran
2021-08-03 16:14           ` Richard Cochran
2021-08-03 17:00             ` Arnd Bergmann
2021-08-03 17:18               ` Keller, Jacob E
2021-08-03 18:27                 ` Arnd Bergmann
2021-08-03 23:25                   ` Keller, Jacob E
2021-08-04 11:18                   ` Arnd Bergmann [this message]
2021-08-03 20:54               ` Richard Cochran
2021-08-04 20:53                 ` 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='CAK8P3a2=pLt9iwE1xw5yhk=DA_i5HLsf5q94GQ2BCG48rS45jQ@mail.gmail.com' \
    --to=arnd@arndb.de \
    --cc=anthony.l.nguyen@intel.com \
    --cc=davem@davemloft.net \
    --cc=david.m.ertman@intel.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=jacob.e.keller@intel.com \
    --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=nico@fluxnic.net \
    --cc=richardcochran@gmail.com \
    --cc=shiraz.saleem@intel.com \
    --cc=yangbo.lu@nxp.com \
    --subject='Re: [PATCH net-next v2] ethernet/intel: fix PTP_1588_CLOCK dependencies' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).