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.
[1]: https://lkml.org/lkml/2020/4/22/1056
[2]: NET_DSA_SJA1105_PTP had never declared any type of dependency on
PTP_1588_CLOCK (`imply` or otherwise); adding it here seems appropriate.
Cc: Arnd Bergmann <[email protected]>
Cc: Richard Cochran <[email protected]>
Cc: Nicolas Pitre <[email protected]>
Cc: Grygorii Strashko <[email protected]>
Fixes: d1cbfd771ce8 ("ptp_clock: Allow for it to be optional")
Signed-off-by: Clay McClure <[email protected]>
---
drivers/net/dsa/mv88e6xxx/Kconfig | 3 ++-
drivers/net/dsa/sja1105/Kconfig | 2 ++
drivers/net/ethernet/cadence/Kconfig | 3 ++-
drivers/net/ethernet/cavium/Kconfig | 3 ++-
drivers/net/ethernet/ti/Kconfig | 2 +-
drivers/net/ethernet/xscale/Kconfig | 3 ++-
6 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/Kconfig b/drivers/net/dsa/mv88e6xxx/Kconfig
index 6435020d690d..6e8250599eb1 100644
--- a/drivers/net/dsa/mv88e6xxx/Kconfig
+++ b/drivers/net/dsa/mv88e6xxx/Kconfig
@@ -25,7 +25,8 @@ config NET_DSA_MV88E6XXX_PTP
default n
depends on NET_DSA_MV88E6XXX_GLOBAL2
imply NETWORK_PHY_TIMESTAMPING
- imply PTP_1588_CLOCK
+ select PTP_1588_CLOCK
+ select POSIX_TIMERS
help
Say Y to enable PTP hardware timestamping on Marvell 88E6xxx switch
chips that support it.
diff --git a/drivers/net/dsa/sja1105/Kconfig b/drivers/net/dsa/sja1105/Kconfig
index 0fe1ae173aa1..84349b6c8c44 100644
--- a/drivers/net/dsa/sja1105/Kconfig
+++ b/drivers/net/dsa/sja1105/Kconfig
@@ -20,6 +20,8 @@ tristate "NXP SJA1105 Ethernet switch family support"
config NET_DSA_SJA1105_PTP
bool "Support for the PTP clock on the NXP SJA1105 Ethernet switch"
depends on NET_DSA_SJA1105
+ select PTP_1588_CLOCK
+ select POSIX_TIMERS
help
This enables support for timestamping and PTP clock manipulations in
the SJA1105 DSA driver.
diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig
index 53b50c24d9c9..bc792c334903 100644
--- a/drivers/net/ethernet/cadence/Kconfig
+++ b/drivers/net/ethernet/cadence/Kconfig
@@ -36,7 +36,8 @@ config MACB_USE_HWSTAMP
bool "Use IEEE 1588 hwstamp"
depends on MACB
default y
- imply PTP_1588_CLOCK
+ select PTP_1588_CLOCK
+ select POSIX_TIMERS
---help---
Enable IEEE 1588 Precision Time Protocol (PTP) support for MACB.
diff --git a/drivers/net/ethernet/cavium/Kconfig b/drivers/net/ethernet/cavium/Kconfig
index 6a700d34019e..bae58c488792 100644
--- a/drivers/net/ethernet/cavium/Kconfig
+++ b/drivers/net/ethernet/cavium/Kconfig
@@ -54,7 +54,8 @@ config THUNDER_NIC_RGX
config CAVIUM_PTP
tristate "Cavium PTP coprocessor as PTP clock"
depends on 64BIT && PCI
- imply PTP_1588_CLOCK
+ select PTP_1588_CLOCK
+ select POSIX_TIMERS
---help---
This driver adds support for the Precision Time Protocol Clocks and
Timestamping coprocessor (PTP) found on Cavium processors.
diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
index 89cec778cf2d..1177953790c5 100644
--- a/drivers/net/ethernet/ti/Kconfig
+++ b/drivers/net/ethernet/ti/Kconfig
@@ -92,7 +92,7 @@ config TI_CPTS_MOD
depends on TI_CPTS
default y if TI_CPSW=y || TI_KEYSTONE_NETCP=y || TI_CPSW_SWITCHDEV=y
select NET_PTP_CLASSIFY
- imply PTP_1588_CLOCK
+ select PTP_1588_CLOCK
default m
config TI_K3_AM65_CPSW_NUSS
diff --git a/drivers/net/ethernet/xscale/Kconfig b/drivers/net/ethernet/xscale/Kconfig
index 98aa7b8ddb06..82d4ec3c6398 100644
--- a/drivers/net/ethernet/xscale/Kconfig
+++ b/drivers/net/ethernet/xscale/Kconfig
@@ -30,7 +30,8 @@ config IXP4XX_ETH
config PTP_1588_CLOCK_IXP46X
tristate "Intel IXP46x as PTP clock"
depends on IXP4XX_ETH
- depends on PTP_1588_CLOCK
+ select PTP_1588_CLOCK
+ select POSIX_TIMERS
default y
help
This driver adds support for using the IXP46X as a PTP
--
2.20.1
Hi Clay,
Thanks for your patch!
On Tue, Apr 28, 2020 at 11:14 AM Clay McClure <[email protected]> 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.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Tue, Apr 28, 2020 at 11:21 AM Geert Uytterhoeven
<[email protected]> wrote:
> On Tue, Apr 28, 2020 at 11:14 AM Clay McClure <[email protected]> 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
On Tue, Apr 28, 2020 at 06:07:45PM +0200, Arnd Bergmann wrote:
> On Tue, Apr 28, 2020 at 11:21 AM Geert Uytterhoeven
> <[email protected]> wrote:
> > On Tue, Apr 28, 2020 at 11:14 AM Clay McClure <[email protected]> wrote:
> > > 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.
Thanks for reviewing the patch. I'll post v2 using `depends on` shortly.
> 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.
I see some drivers are starting to do just that, e.g.:
commit 96c34151d157 ("net/mlx5: Kconfig: convert imply usage to weak dependency")
I can post a patch this weekend converting the rest of the drivers.
--
Clay
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() but merely
`imply PTP_1588_CLOCK` and found five 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
Note how these symbols all reference PTP or timestamping in their name;
this is a clue that they depend on PTP_1588_CLOCK.
Change them from `imply PTP_1588_CLOCK` [2] to `depends on PTP_1588_CLOCK`.
I'm not using `select PTP_1588_CLOCK` here because PTP_1588_CLOCK has
its own dependencies, which `select` would not transitively apply.
Additionally, remove the `select NET_PTP_CLASSIFY` from CPTS_TI_MOD;
PTP_1588_CLOCK already selects that.
[1]: https://lore.kernel.org/lkml/[email protected]/
[2]: NET_DSA_SJA1105_PTP had never declared any type of dependency on
PTP_1588_CLOCK (`imply` or otherwise); adding a `depends on PTP_1588_CLOCK`
here seems appropriate.
Cc: Arnd Bergmann <[email protected]>
Cc: Richard Cochran <[email protected]>
Cc: Nicolas Pitre <[email protected]>
Cc: Grygorii Strashko <[email protected]>
Cc: Geert Uytterhoeven <[email protected]>
Fixes: d1cbfd771ce8 ("ptp_clock: Allow for it to be optional")
Signed-off-by: Clay McClure <[email protected]>
---
drivers/net/dsa/mv88e6xxx/Kconfig | 2 +-
drivers/net/dsa/sja1105/Kconfig | 1 +
drivers/net/ethernet/cadence/Kconfig | 2 +-
drivers/net/ethernet/cavium/Kconfig | 2 +-
drivers/net/ethernet/ti/Kconfig | 3 +--
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/Kconfig b/drivers/net/dsa/mv88e6xxx/Kconfig
index 6435020d690d..51185e4d7d15 100644
--- a/drivers/net/dsa/mv88e6xxx/Kconfig
+++ b/drivers/net/dsa/mv88e6xxx/Kconfig
@@ -24,8 +24,8 @@ config NET_DSA_MV88E6XXX_PTP
bool "PTP support for Marvell 88E6xxx"
default n
depends on NET_DSA_MV88E6XXX_GLOBAL2
+ depends on PTP_1588_CLOCK
imply NETWORK_PHY_TIMESTAMPING
- imply PTP_1588_CLOCK
help
Say Y to enable PTP hardware timestamping on Marvell 88E6xxx switch
chips that support it.
diff --git a/drivers/net/dsa/sja1105/Kconfig b/drivers/net/dsa/sja1105/Kconfig
index 0fe1ae173aa1..68c3086af9af 100644
--- a/drivers/net/dsa/sja1105/Kconfig
+++ b/drivers/net/dsa/sja1105/Kconfig
@@ -20,6 +20,7 @@ tristate "NXP SJA1105 Ethernet switch family support"
config NET_DSA_SJA1105_PTP
bool "Support for the PTP clock on the NXP SJA1105 Ethernet switch"
depends on NET_DSA_SJA1105
+ depends on PTP_1588_CLOCK
help
This enables support for timestamping and PTP clock manipulations in
the SJA1105 DSA driver.
diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig
index 53b50c24d9c9..2c4c12b03502 100644
--- a/drivers/net/ethernet/cadence/Kconfig
+++ b/drivers/net/ethernet/cadence/Kconfig
@@ -35,8 +35,8 @@ config MACB
config MACB_USE_HWSTAMP
bool "Use IEEE 1588 hwstamp"
depends on MACB
+ depends on PTP_1588_CLOCK
default y
- imply PTP_1588_CLOCK
---help---
Enable IEEE 1588 Precision Time Protocol (PTP) support for MACB.
diff --git a/drivers/net/ethernet/cavium/Kconfig b/drivers/net/ethernet/cavium/Kconfig
index 6a700d34019e..4520e7ee00fe 100644
--- a/drivers/net/ethernet/cavium/Kconfig
+++ b/drivers/net/ethernet/cavium/Kconfig
@@ -54,7 +54,7 @@ config THUNDER_NIC_RGX
config CAVIUM_PTP
tristate "Cavium PTP coprocessor as PTP clock"
depends on 64BIT && PCI
- imply PTP_1588_CLOCK
+ depends on PTP_1588_CLOCK
---help---
This driver adds support for the Precision Time Protocol Clocks and
Timestamping coprocessor (PTP) found on Cavium processors.
diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
index 89cec778cf2d..8e348780efb6 100644
--- a/drivers/net/ethernet/ti/Kconfig
+++ b/drivers/net/ethernet/ti/Kconfig
@@ -90,9 +90,8 @@ config TI_CPTS
config TI_CPTS_MOD
tristate
depends on TI_CPTS
+ depends on PTP_1588_CLOCK
default y if TI_CPSW=y || TI_KEYSTONE_NETCP=y || TI_CPSW_SWITCHDEV=y
- select NET_PTP_CLASSIFY
- imply PTP_1588_CLOCK
default m
config TI_K3_AM65_CPSW_NUSS
--
2.20.1
From: Clay McClure <[email protected]>
Date: Wed, 29 Apr 2020 00:59:00 -0700
> 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.
...
> Note how these symbols all reference PTP or timestamping in their name;
> this is a clue that they depend on PTP_1588_CLOCK.
>
> Change them from `imply PTP_1588_CLOCK` [2] to `depends on PTP_1588_CLOCK`.
> I'm not using `select PTP_1588_CLOCK` here because PTP_1588_CLOCK has
> its own dependencies, which `select` would not transitively apply.
>
> Additionally, remove the `select NET_PTP_CLASSIFY` from CPTS_TI_MOD;
> PTP_1588_CLOCK already selects that.
...
Applied, thanks.