Re: [PATCH v3) posix-timers: make it configurable

[Nicolas Pitre]

On Thu, 15 Sep 2016, John Stultz wrote:

> On Thu, Sep 15, 2016 at 11:37 AM, Nicolas Pitre
> <nicolas.pitre@xxxxxxxxxx> wrote:
> > On Thu, 15 Sep 2016, John Stultz wrote:
> >
> >> On Thu, Sep 15, 2016 at 11:28 AM, Nicolas Pitre
> >> <nicolas.pitre@xxxxxxxxxx> wrote:
> >> > On Thu, 15 Sep 2016, John Stultz wrote:
> >> >
> >> >> > diff --git a/kernel/time/Kconfig b/kernel/time/Kconfig
> >> >> > index 62824f2fe4..62504a2c9f 100644
> >> >> > --- a/kernel/time/Kconfig
> >> >> > +++ b/kernel/time/Kconfig
> >> >> > @@ -195,3 +195,21 @@ config HIGH_RES_TIMERS
> >> >> >
> >> >> >  endmenu
> >> >> >  endif
> >> >> > +
> >> >> > +config POSIX_TIMERS
> >> >> > +       bool "Posix Clocks & timers" if EMBEDDED
> >> >> > +       default y
> >> >> > +       help
> >> >> > +         This includes native support for POSIX timers to the kernel.
> >> >> > +         Most embedded systems may have no use for them and therefore they
> >> >> > +         can be configured out to reduce the size of the kernel image.
> >> >> > +
> >> >> > +         When this option is disabled, the following syscalls won't be
> >> >> > +         available: timer_create, timer_gettime: timer_getoverrun,
> >> >> > +         timer_settime, timer_delete, clock_adjtime. Furthermore, the
> >> >> > +         clock_settime, clock_gettime, clock_getres and clock_nanosleep
> >> >> > +         syscalls will be limited to CLOCK_REALTIME and CLOCK_MONOTONIC
> >> >> > +         only.
> >> >> > +
> >> >> > +         If unsure say y.
> >> >> >
> >> >>
> >> >> One thought.. Should this go under:
> >> >>     Configure standard kernel features (expert users)
> >> >> rather then a top level item under  General Setup ?
> >> >
> >> > Hmmm... probably yes.
> >> >
> >> > Do you need that I repost the patch?
> >>
> >> I can see about moving it..
> >
> > OK.
> >
> > Making it conditional on EXPERT rather than EMBEDDED would also be more
> > inline with the other similar options there.
> 
> Ack.
> 
> Although I'm also seeing some Kconfig noise when I disable it:
> 
> warning: (SFC && TILE_NET && AMD_XGBE && BFIN_MAC_USE_HWSTAMP &&
> TIGON3 && BNX2X && LIQUIDIO && FEC && E1000E && IGB && IXGBE && I40E
> && FM10K && MLX4_EN && MLX5_CORE_EN && RAVB && SXGBE_ETH && STMMAC_ETH
> && TI_CPTS && PTP_1588_CLOCK_GIANFAR && PTP_1588_CLOCK_IXP46X &&
> DP83640_PHY && PTP_1588_CLOCK_PCH) selects PTP_1588_CLOCK which has
> unmet direct dependencies (NET && POSIX_TIMERS)
> warning: (SFC && TILE_NET && AMD_XGBE && BFIN_MAC_USE_HWSTAMP &&
> TIGON3 && BNX2X && LIQUIDIO && FEC && E1000E && IGB && IXGBE && I40E
> && FM10K && MLX4_EN && MLX5_CORE_EN && RAVB && SXGBE_ETH && STMMAC_ETH
> && TI_CPTS && PTP_1588_CLOCK_GIANFAR && PTP_1588_CLOCK_IXP46X &&
> DP83640_PHY && PTP_1588_CLOCK_PCH) selects PTP_1588_CLOCK which has
> unmet direct dependencies (NET && POSIX_TIMERS)
> 
> Not sure if this is just expected given we can't do reverse dependency
> checking on select...
> 
> Maybe PTP_1588_CLOCK needs to select POSIX_TIMERS instead of just depend on it?

That would forcefully pull in a whole bunch of code that one wanted out 
by explicitly not enabling CONFIG_POSIX_TIMERS.  And the current depends 
statement forces out a bunch of ethernet drivers.

Maybe there ought to be fewer of those hard dependencies such as net 
drivers with ptp, and ptp with POSIX timers.

What about the following patch that would take care of the later?

diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
index 00e6098e9a..ee3de3421f 100644
--- a/drivers/ptp/Kconfig
+++ b/drivers/ptp/Kconfig
@@ -6,7 +6,7 @@ menu "PTP clock support"
 
 config PTP_1588_CLOCK
 	tristate "PTP clock support"
-	depends on NET && POSIX_TIMERS
+	depends on NET
 	select PPS
 	select NET_PTP_CLASSIFY
 	help
diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 2e481b9e8e..873addff63 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -208,7 +208,8 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
 		goto no_slot;
 	}
 
-	ptp->clock.ops = ptp_clock_ops;
+	if (IS_ENABLED(CONFIG_POSIX_TIMERS))
+		ptp->clock.ops = ptp_clock_ops;
 	ptp->clock.release = delete_ptp_clock;
 	ptp->info = info;
 	ptp->devid = MKDEV(major, index);
@@ -245,10 +246,12 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
 	}
 
 	/* Create a posix clock. */
-	err = posix_clock_register(&ptp->clock, ptp->devid);
-	if (err) {
-		pr_err("failed to create posix clock\n");
-		goto no_clock;
+	if (IS_ENABLED(CONFIG_POSIX_TIMERS)) {
+		err = posix_clock_register(&ptp->clock, ptp->devid);
+		if (err) {
+			pr_err("failed to create posix clock\n");
+			goto no_clock;
+		}
 	}
 
 	return ptp;
@@ -281,7 +284,8 @@ int ptp_clock_unregister(struct ptp_clock *ptp)
 	ptp_cleanup_sysfs(ptp);
 	device_destroy(ptp_class, ptp->devid);
 
-	posix_clock_unregister(&ptp->clock);
+	if (IS_ENABLED(CONFIG_POSIX_TIMERS))
+		posix_clock_unregister(&ptp->clock);
 	return 0;
 }
 EXPORT_SYMBOL(ptp_clock_unregister);



> 
> thanks
> -john
>