On Tue, Apr 28, 2020 at 11:21 AM Geert Uytterhoeven <ge...@linux-m68k.org> wrote: > On Tue, Apr 28, 2020 at 11:14 AM Clay McClure <c...@daemons.net> wrote: > > Commit d1cbfd771ce8 ("ptp_clock: Allow for it to be optional") changed > > all PTP-capable Ethernet drivers from `select PTP_1588_CLOCK` to `imply > > PTP_1588_CLOCK`, "in order to break the hard dependency between the PTP > > clock subsystem and ethernet drivers capable of being clock providers." > > As a result it is possible to build PTP-capable Ethernet drivers without > > the PTP subsystem by deselecting PTP_1588_CLOCK. Drivers are required to > > handle the missing dependency gracefully. > > > > Some PTP-capable Ethernet drivers (e.g., TI_CPSW) factor their PTP code > > out into separate drivers (e.g., TI_CPTS_MOD). The above commit also > > changed these PTP-specific drivers to `imply PTP_1588_CLOCK`, making it > > possible to build them without the PTP subsystem. But as Grygorii > > Strashko noted in [1]: > > > > On Wed, Apr 22, 2020 at 02:16:11PM +0300, Grygorii Strashko wrote: > > > > > Another question is that CPTS completely nonfunctional in this case and > > > it was never expected that somebody will even try to use/run such > > > configuration (except for random build purposes). > > > > In my view, enabling a PTP-specific driver without the PTP subsystem is > > a configuration error made possible by the above commit. Kconfig should > > not allow users to create a configuration with missing dependencies that > > results in "completely nonfunctional" drivers. > > > > I audited all network drivers that call ptp_clock_register() and found > > six that look like PTP-specific drivers that are likely nonfunctional > > without PTP_1588_CLOCK: > > > > NET_DSA_MV88E6XXX_PTP > > NET_DSA_SJA1105_PTP > > MACB_USE_HWSTAMP > > CAVIUM_PTP > > TI_CPTS_MOD > > PTP_1588_CLOCK_IXP46X > > > > Note how they all reference PTP or timestamping in their name; this is a > > clue that they depend on PTP_1588_CLOCK. > > > > Change these drivers back [2] to `select PTP_1588_CLOCK`. Note that this > > requires also selecting POSIX_TIMERS, a transitive dependency of > > PTP_1588_CLOCK. > > If these drivers have a hard dependency on PTP_1588_CLOCK, IMHO they > should depend on PTP_1588_CLOCK, not select PTP_1588_CLOCK.
Agreed. Note that for drivers that only optionally use the PTP_1588_CLOCK support, we probably want 'depends on PTP_1588_CLOCK || !PTP_1588_CLOCK' (or the syntax replacing it eventually), to avoid the case where a built-in driver fails to use a modular ptp implementation. Arnd