On Thu, Sep 15, 2016 at 12:31 PM, Nicolas Pitre
<nicolas.pi...@linaro.org> wrote:
> On Thu, 15 Sep 2016, John Stultz wrote:
>
>> On Thu, Sep 15, 2016 at 11:37 AM, Nicolas Pitre
>> <nicolas.pi...@linaro.org> wrote:
>> > On Thu, 15 Sep 2016, John Stultz wrote:
>> >
>> >> On Thu, Sep 15, 2016 at 11:28 AM, Nicolas Pitre
>> >> <nicolas.pi...@linaro.org> 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);
This doesn't look too bad.
Richard: Your thoughts?
thanks
-john
