All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3) posix-timers: make it configurable
@ 2016-09-15  3:47 Nicolas Pitre
  2016-09-15 17:48 ` John Stultz
                   ` (2 more replies)
  0 siblings, 3 replies; 25+ messages in thread
From: Nicolas Pitre @ 2016-09-15  3:47 UTC (permalink / raw)
  To: Thomas Gleixner, John Stultz; +Cc: Josh Triplett, Richard Cochran, linux-kernel

Many embedded systems typically don't need them.  This removes about
22KB from the kernel binary size on ARM when configured out.

Corresponding syscalls are routed to a stub logging the attempt to
use those syscalls which should be enough of a clue if they were
disabled without proper consideration. They are: timer_create,
timer_gettime: timer_getoverrun, timer_settime, timer_delete,
clock_adjtime.

The clock_settime, clock_gettime, clock_getres and clock_nanosleep syscalls
are replaced by simple wrappers compatible with CLOCK_REALTIME,
CLOCK_MONOTONIC and CLOCK_BOOTTIME only.

Signed-off-by: Nicolas Pitre <nico@linaro.org>
---

Changes from v2:

- Fix compilation with CONFIG_COMPAT=y due to missing 
  clock_nanosleep_restart(), found by kbuild test robot.

Changes from RFC/v1:

- Stubbed-out functions moved to static inlines.
- The timer signal handling code is now removed.
- The list of removed syscalls is explicitly documented.
- The clock_settime, clock_gettime, clock_getres and clock_nanosleep 
  syscalls are minimally preserved as this required very little code.

I'm now able to boot a copy of Fedora 21 with this patch and 
CONFIG_POSIX_TIMERS=n with no apparent issues.

diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
index ee3de3421f..00e6098e9a 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
+	depends on NET && POSIX_TIMERS
 	select PPS
 	select NET_PTP_CLASSIFY
 	help
diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h
index 62d44c1760..2288c5c557 100644
--- a/include/linux/posix-timers.h
+++ b/include/linux/posix-timers.h
@@ -118,6 +118,8 @@ struct k_clock {
 extern struct k_clock clock_posix_cpu;
 extern struct k_clock clock_posix_dynamic;
 
+#ifdef CONFIG_POSIX_TIMERS
+
 void posix_timers_register_clock(const clockid_t clock_id, struct k_clock *new_clock);
 
 /* function to call to trigger timer event */
@@ -131,8 +133,30 @@ void posix_cpu_timers_exit_group(struct task_struct *task);
 void set_process_cpu_timer(struct task_struct *task, unsigned int clock_idx,
 			   cputime_t *newval, cputime_t *oldval);
 
-long clock_nanosleep_restart(struct restart_block *restart_block);
-
 void update_rlimit_cpu(struct task_struct *task, unsigned long rlim_new);
 
+#else
+
+#include <linux/random.h>
+
+static inline void posix_timers_register_clock(const clockid_t clock_id,
+					       struct k_clock *new_clock) {}
+static inline int posix_timer_event(struct k_itimer *timr, int si_private)
+{ return 0; }
+static inline void run_posix_cpu_timers(struct task_struct *task) {}
+static inline void posix_cpu_timers_exit(struct task_struct *task)
+{
+	add_device_randomness((const void*) &task->se.sum_exec_runtime,
+			      sizeof(unsigned long long));
+}
+static inline void posix_cpu_timers_exit_group(struct task_struct *task) {}
+static inline void set_process_cpu_timer(struct task_struct *task,
+		unsigned int clock_idx, cputime_t *newval, cputime_t *oldval) {}
+static inline void update_rlimit_cpu(struct task_struct *task,
+				     unsigned long rlim_new) {}
+
+#endif
+
+long clock_nanosleep_restart(struct restart_block *restart_block);
+
 #endif
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 54182d52a0..39a1d6d3f5 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2924,8 +2924,13 @@ static inline void exit_thread(struct task_struct *tsk)
 extern void exit_files(struct task_struct *);
 extern void __cleanup_sighand(struct sighand_struct *);
 
+#ifdef CONFIG_POSIX_TIMERS
 extern void exit_itimers(struct signal_struct *);
 extern void flush_itimer_signals(void);
+#else
+static inline void exit_itimers(struct signal_struct *s) {}
+static inline void flush_itimer_signals(void) {}
+#endif
 
 extern void do_group_exit(int);
 
@@ -3382,7 +3387,12 @@ static __always_inline bool need_resched(void)
  * Thread group CPU time accounting.
  */
 void thread_group_cputime(struct task_struct *tsk, struct task_cputime *times);
+#ifdef CONFIG_POSIX_TIMERS
 void thread_group_cputimer(struct task_struct *tsk, struct task_cputime *times);
+#else
+static inline void thread_group_cputimer(struct task_struct *tsk,
+					 struct task_cputime *times) {}
+#endif
 
 /*
  * Reevaluate whether the task has signals pending delivery.
diff --git a/kernel/signal.c b/kernel/signal.c
index af21afc00d..ea75065e29 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -427,6 +427,7 @@ void flush_signals(struct task_struct *t)
 	spin_unlock_irqrestore(&t->sighand->siglock, flags);
 }
 
+#ifdef CONFIG_POSIX_TIMERS
 static void __flush_itimer_signals(struct sigpending *pending)
 {
 	sigset_t signal, retain;
@@ -460,6 +461,7 @@ void flush_itimer_signals(void)
 	__flush_itimer_signals(&tsk->signal->shared_pending);
 	spin_unlock_irqrestore(&tsk->sighand->siglock, flags);
 }
+#endif
 
 void ignore_signals(struct task_struct *t)
 {
@@ -611,6 +613,7 @@ int dequeue_signal(struct task_struct *tsk, sigset_t *mask, siginfo_t *info)
 		 */
 		current->jobctl |= JOBCTL_STOP_DEQUEUED;
 	}
+#ifdef CONFIG_POSIX_TIMERS
 	if ((info->si_code & __SI_MASK) == __SI_TIMER && info->si_sys_private) {
 		/*
 		 * Release the siglock to ensure proper locking order
@@ -622,6 +625,7 @@ int dequeue_signal(struct task_struct *tsk, sigset_t *mask, siginfo_t *info)
 		do_schedule_next_timer(info);
 		spin_lock(&tsk->sighand->siglock);
 	}
+#endif
 	return signr;
 }
 
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.
+
diff --git a/kernel/time/Makefile b/kernel/time/Makefile
index 49eca0beed..fc26c308f5 100644
--- a/kernel/time/Makefile
+++ b/kernel/time/Makefile
@@ -1,6 +1,12 @@
-obj-y += time.o timer.o hrtimer.o itimer.o posix-timers.o posix-cpu-timers.o
+obj-y += time.o timer.o hrtimer.o itimer.o
 obj-y += timekeeping.o ntp.o clocksource.o jiffies.o timer_list.o
-obj-y += timeconv.o timecounter.o posix-clock.o alarmtimer.o
+obj-y += timeconv.o timecounter.o alarmtimer.o
+
+ifeq ($(CONFIG_POSIX_TIMERS),y)
+ obj-y += posix-timers.o posix-cpu-timers.o posix-clock.o
+else
+ obj-y += posix-stubs.o
+endif
 
 obj-$(CONFIG_GENERIC_CLOCKEVENTS)		+= clockevents.o tick-common.o
 ifeq ($(CONFIG_GENERIC_CLOCKEVENTS_BROADCAST),y)
diff --git a/kernel/time/posix-stubs.c b/kernel/time/posix-stubs.c
new file mode 100644
index 0000000000..fe857bd4a0
--- /dev/null
+++ b/kernel/time/posix-stubs.c
@@ -0,0 +1,118 @@
+/*
+ * Dummy stubs used when CONFIG_POSIX_TIMERS=n
+ *
+ * Created by:  Nicolas Pitre, July 2016
+ * Copyright:   (C) 2016 Linaro Limited
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/linkage.h>
+#include <linux/kernel.h>
+#include <linux/sched.h>
+#include <linux/errno.h>
+#include <linux/syscalls.h>
+#include <linux/ktime.h>
+#include <linux/timekeeping.h>
+#include <linux/posix-timers.h>
+
+asmlinkage long sys_ni_posix_timers(void)
+{
+	pr_err_once("process %d (%s) attempted a POSIX timer syscall "
+		    "while CONFIG_POSIX_TIMERS is not set\n",
+		    current->pid, current->comm);
+	return -ENOSYS;
+}
+
+#define SYS_NI(name)  SYSCALL_ALIAS(sys_##name, sys_ni_posix_timers)
+
+SYS_NI(timer_create);
+SYS_NI(timer_gettime);
+SYS_NI(timer_getoverrun);
+SYS_NI(timer_settime);
+SYS_NI(timer_delete);
+SYS_NI(clock_adjtime);
+
+/*
+ * We preserve minimal support for CLOCK_REALTIME and CLOCK_MONOTONIC
+ * as it is easy to remain compatible with little code. CLOCK_BOOTTIME
+ * is also included for convenience as at least systemd uses it.
+ */
+
+SYSCALL_DEFINE2(clock_settime, const clockid_t, which_clock,
+		const struct timespec __user *, tp)
+{
+	struct timespec new_tp;
+
+	if (which_clock != CLOCK_REALTIME)
+		return -EINVAL;
+	if (copy_from_user(&new_tp, tp, sizeof (*tp)))
+		return -EFAULT;
+	return do_sys_settimeofday(&new_tp, NULL);
+}
+
+SYSCALL_DEFINE2(clock_gettime, const clockid_t, which_clock,
+		struct timespec __user *,tp)
+{
+	struct timespec kernel_tp;
+
+	switch (which_clock) {
+	case CLOCK_REALTIME: ktime_get_real_ts(&kernel_tp); break;
+	case CLOCK_MONOTONIC: ktime_get_ts(&kernel_tp); break;
+	case CLOCK_BOOTTIME: get_monotonic_boottime(&kernel_tp); break;
+	default: return -EINVAL;
+	}
+	if (copy_to_user(tp, &kernel_tp, sizeof (kernel_tp)))
+		return -EFAULT;
+	return 0;
+}
+
+SYSCALL_DEFINE2(clock_getres, const clockid_t, which_clock, struct timespec __user *, tp)
+{
+	struct timespec rtn_tp = {
+		.tv_sec = 0,
+		.tv_nsec = hrtimer_resolution,
+	};
+
+	switch (which_clock) {
+	case CLOCK_REALTIME:
+	case CLOCK_MONOTONIC:
+	case CLOCK_BOOTTIME:
+		if (copy_to_user(tp, &rtn_tp, sizeof(rtn_tp)))
+			return -EFAULT;
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+SYSCALL_DEFINE4(clock_nanosleep, const clockid_t, which_clock, int, flags,
+		const struct timespec __user *, rqtp,
+		struct timespec __user *, rmtp)
+{
+	struct timespec t;
+
+	switch (which_clock) {
+	case CLOCK_REALTIME:
+	case CLOCK_MONOTONIC:
+	case CLOCK_BOOTTIME:
+		if (copy_from_user(&t, rqtp, sizeof (struct timespec)))
+			return -EFAULT;
+		if (!timespec_valid(&t))
+			return -EINVAL;
+		return hrtimer_nanosleep(&t, rmtp, flags & TIMER_ABSTIME ?
+					 HRTIMER_MODE_ABS : HRTIMER_MODE_REL,
+					 which_clock);
+	default:
+		return -EINVAL;
+	}
+}
+
+#ifdef CONFIG_COMPAT
+long clock_nanosleep_restart(struct restart_block *restart_block)
+{
+	return hrtimer_nanosleep_restart(restart_block);
+}
+#endif

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH v3) posix-timers: make it configurable
  2016-09-15  3:47 [PATCH v3) posix-timers: make it configurable Nicolas Pitre
@ 2016-09-15 17:48 ` John Stultz
  2016-09-15 17:56   ` Nicolas Pitre
  2016-09-15 18:13 ` John Stultz
  2016-09-16 14:31 ` kbuild test robot
  2 siblings, 1 reply; 25+ messages in thread
From: John Stultz @ 2016-09-15 17:48 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Thomas Gleixner, Josh Triplett, Richard Cochran, lkml

On Wed, Sep 14, 2016 at 8:47 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> Many embedded systems typically don't need them.  This removes about
> 22KB from the kernel binary size on ARM when configured out.
>
> Corresponding syscalls are routed to a stub logging the attempt to
> use those syscalls which should be enough of a clue if they were
> disabled without proper consideration. They are: timer_create,
> timer_gettime: timer_getoverrun, timer_settime, timer_delete,
> clock_adjtime.
>
> The clock_settime, clock_gettime, clock_getres and clock_nanosleep syscalls
> are replaced by simple wrappers compatible with CLOCK_REALTIME,
> CLOCK_MONOTONIC and CLOCK_BOOTTIME only.

Thanks for re-sending. I'm happier that you're keeping some basic
functionality here. No real objections at this point.

I need to apply it for testing and take a closer look, but its on my
to-queue list. However, with an upcoming conference, as being a little
later in the cycle I can't promise it will make it for the 4.9 merge
window. But we'll see.

thanks!
-john

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3) posix-timers: make it configurable
  2016-09-15 17:48 ` John Stultz
@ 2016-09-15 17:56   ` Nicolas Pitre
  0 siblings, 0 replies; 25+ messages in thread
From: Nicolas Pitre @ 2016-09-15 17:56 UTC (permalink / raw)
  To: John Stultz; +Cc: Thomas Gleixner, Josh Triplett, Richard Cochran, lkml

On Thu, 15 Sep 2016, John Stultz wrote:

> On Wed, Sep 14, 2016 at 8:47 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > Many embedded systems typically don't need them.  This removes about
> > 22KB from the kernel binary size on ARM when configured out.
> >
> > Corresponding syscalls are routed to a stub logging the attempt to
> > use those syscalls which should be enough of a clue if they were
> > disabled without proper consideration. They are: timer_create,
> > timer_gettime: timer_getoverrun, timer_settime, timer_delete,
> > clock_adjtime.
> >
> > The clock_settime, clock_gettime, clock_getres and clock_nanosleep syscalls
> > are replaced by simple wrappers compatible with CLOCK_REALTIME,
> > CLOCK_MONOTONIC and CLOCK_BOOTTIME only.
> 
> Thanks for re-sending. I'm happier that you're keeping some basic
> functionality here. No real objections at this point.
> 
> I need to apply it for testing and take a closer look, but its on my
> to-queue list. However, with an upcoming conference, as being a little
> later in the cycle I can't promise it will make it for the 4.9 merge
> window. But we'll see.

Thanks.  I'll buy you a beer in 2 weeks.  :-)


Nicolas

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3) posix-timers: make it configurable
  2016-09-15  3:47 [PATCH v3) posix-timers: make it configurable Nicolas Pitre
  2016-09-15 17:48 ` John Stultz
@ 2016-09-15 18:13 ` John Stultz
  2016-09-15 18:28   ` Nicolas Pitre
  2016-09-16 14:31 ` kbuild test robot
  2 siblings, 1 reply; 25+ messages in thread
From: John Stultz @ 2016-09-15 18:13 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Thomas Gleixner, Josh Triplett, Richard Cochran, lkml

> 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 ?

thanks
-john

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3) posix-timers: make it configurable
  2016-09-15 18:13 ` John Stultz
@ 2016-09-15 18:28   ` Nicolas Pitre
  2016-09-15 18:35     ` John Stultz
  0 siblings, 1 reply; 25+ messages in thread
From: Nicolas Pitre @ 2016-09-15 18:28 UTC (permalink / raw)
  To: John Stultz; +Cc: Thomas Gleixner, Josh Triplett, Richard Cochran, lkml

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?


Nicolas

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3) posix-timers: make it configurable
  2016-09-15 18:28   ` Nicolas Pitre
@ 2016-09-15 18:35     ` John Stultz
  2016-09-15 18:37       ` Nicolas Pitre
  0 siblings, 1 reply; 25+ messages in thread
From: John Stultz @ 2016-09-15 18:35 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Thomas Gleixner, Josh Triplett, Richard Cochran, lkml

On Thu, Sep 15, 2016 at 11:28 AM, Nicolas Pitre
<nicolas.pitre@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..

thanks
-john

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3) posix-timers: make it configurable
  2016-09-15 18:35     ` John Stultz
@ 2016-09-15 18:37       ` Nicolas Pitre
  2016-09-15 18:46         ` John Stultz
  0 siblings, 1 reply; 25+ messages in thread
From: Nicolas Pitre @ 2016-09-15 18:37 UTC (permalink / raw)
  To: John Stultz; +Cc: Thomas Gleixner, Josh Triplett, Richard Cochran, lkml

On Thu, 15 Sep 2016, John Stultz wrote:

> On Thu, Sep 15, 2016 at 11:28 AM, Nicolas Pitre
> <nicolas.pitre@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.


Nicolas

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3) posix-timers: make it configurable
  2016-09-15 18:37       ` Nicolas Pitre
@ 2016-09-15 18:46         ` John Stultz
  2016-09-15 19:31           ` Nicolas Pitre
  0 siblings, 1 reply; 25+ messages in thread
From: John Stultz @ 2016-09-15 18:46 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Thomas Gleixner, Josh Triplett, Richard Cochran, lkml

On Thu, Sep 15, 2016 at 11:37 AM, Nicolas Pitre
<nicolas.pitre@linaro.org> wrote:
> On Thu, 15 Sep 2016, John Stultz wrote:
>
>> On Thu, Sep 15, 2016 at 11:28 AM, Nicolas Pitre
>> <nicolas.pitre@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?

thanks
-john

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3) posix-timers: make it configurable
  2016-09-15 18:46         ` John Stultz
@ 2016-09-15 19:31           ` Nicolas Pitre
  2016-09-15 19:58             ` John Stultz
  0 siblings, 1 reply; 25+ messages in thread
From: Nicolas Pitre @ 2016-09-15 19:31 UTC (permalink / raw)
  To: John Stultz; +Cc: Thomas Gleixner, Josh Triplett, Richard Cochran, lkml

On Thu, 15 Sep 2016, John Stultz wrote:

> On Thu, Sep 15, 2016 at 11:37 AM, Nicolas Pitre
> <nicolas.pitre@linaro.org> wrote:
> > On Thu, 15 Sep 2016, John Stultz wrote:
> >
> >> On Thu, Sep 15, 2016 at 11:28 AM, Nicolas Pitre
> >> <nicolas.pitre@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);



> 
> thanks
> -john
> 

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH v3) posix-timers: make it configurable
  2016-09-15 19:31           ` Nicolas Pitre
@ 2016-09-15 19:58             ` John Stultz
  2016-09-15 21:07               ` Richard Cochran
  2016-09-15 21:23               ` Richard Cochran
  0 siblings, 2 replies; 25+ messages in thread
From: John Stultz @ 2016-09-15 19:58 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Thomas Gleixner, Josh Triplett, Richard Cochran, lkml

On Thu, Sep 15, 2016 at 12:31 PM, Nicolas Pitre
<nicolas.pitre@linaro.org> wrote:
> On Thu, 15 Sep 2016, John Stultz wrote:
>
>> On Thu, Sep 15, 2016 at 11:37 AM, Nicolas Pitre
>> <nicolas.pitre@linaro.org> wrote:
>> > On Thu, 15 Sep 2016, John Stultz wrote:
>> >
>> >> On Thu, Sep 15, 2016 at 11:28 AM, Nicolas Pitre
>> >> <nicolas.pitre@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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3) posix-timers: make it configurable
  2016-09-15 19:58             ` John Stultz
@ 2016-09-15 21:07               ` Richard Cochran
  2016-09-15 21:15                 ` Josh Triplett
  2016-09-15 21:23               ` Richard Cochran
  1 sibling, 1 reply; 25+ messages in thread
From: Richard Cochran @ 2016-09-15 21:07 UTC (permalink / raw)
  To: John Stultz; +Cc: Nicolas Pitre, Thomas Gleixner, Josh Triplett, lkml

On Thu, Sep 15, 2016 at 12:58:22PM -0700, John Stultz wrote:
> This doesn't look too bad.

I disagree.  It looks ugly.  If tinification means sprinkling more and
more of these conditionals all over the place, then it is going to be
a tough sell.
 
> Richard: Your thoughts?

We decided to have MAC drivers select PTP for a reason.  It is a
feature that people generally want to have.  If those drivers bring in
22 KB timer support, then too bad.  As I said before, timers are the
last thing I would remove.

Maybe there is a way to make this work in Kconfig land without messing
with the code?

Thanks,
Richard

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3) posix-timers: make it configurable
  2016-09-15 21:07               ` Richard Cochran
@ 2016-09-15 21:15                 ` Josh Triplett
  2016-09-15 21:35                   ` Nicolas Pitre
  0 siblings, 1 reply; 25+ messages in thread
From: Josh Triplett @ 2016-09-15 21:15 UTC (permalink / raw)
  To: Richard Cochran; +Cc: John Stultz, Nicolas Pitre, Thomas Gleixner, lkml

On Thu, Sep 15, 2016 at 11:07:24PM +0200, Richard Cochran wrote:
> On Thu, Sep 15, 2016 at 12:58:22PM -0700, John Stultz wrote:
> > This doesn't look too bad.
> 
> I disagree.  It looks ugly.  If tinification means sprinkling more and
> more of these conditionals all over the place, then it is going to be
> a tough sell.

Looking at this particular patch, it does seem a bit much for the
ability to have PTP without timers.  That doesn't seem like a very
likely combination.  Handling that in Kconfig seems fine, unless there's
a concrete use case for that combination.

- Josh Triplett

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3) posix-timers: make it configurable
  2016-09-15 19:58             ` John Stultz
  2016-09-15 21:07               ` Richard Cochran
@ 2016-09-15 21:23               ` Richard Cochran
  1 sibling, 0 replies; 25+ messages in thread
From: Richard Cochran @ 2016-09-15 21:23 UTC (permalink / raw)
  To: John Stultz; +Cc: Nicolas Pitre, Thomas Gleixner, Josh Triplett, lkml

On Thu, Sep 15, 2016 at 12:58:22PM -0700, John Stultz wrote:
> This doesn't look too bad.

Actually, it is worse than ugly.  It is a disaster.  With this change,
registration will succeed and MAC drivers will happily report the PTP
devices in 'ethtool -T', but there won't be anything behind them.

IMHO, it would be better in this case to stub out the whole
subsystem.  That would save you ptp_clock.o, ptp_chardev.o, and
ptp_sysfs.o.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3) posix-timers: make it configurable
  2016-09-15 21:15                 ` Josh Triplett
@ 2016-09-15 21:35                   ` Nicolas Pitre
  2016-09-15 21:56                     ` Josh Triplett
  0 siblings, 1 reply; 25+ messages in thread
From: Nicolas Pitre @ 2016-09-15 21:35 UTC (permalink / raw)
  To: Josh Triplett; +Cc: Richard Cochran, John Stultz, Thomas Gleixner, lkml

On Thu, 15 Sep 2016, Josh Triplett wrote:

> On Thu, Sep 15, 2016 at 11:07:24PM +0200, Richard Cochran wrote:
> > On Thu, Sep 15, 2016 at 12:58:22PM -0700, John Stultz wrote:
> > > This doesn't look too bad.
> > 
> > I disagree.  It looks ugly.  If tinification means sprinkling more and
> > more of these conditionals all over the place, then it is going to be
> > a tough sell.
> 
> Looking at this particular patch, it does seem a bit much for the
> ability to have PTP without timers.  That doesn't seem like a very
> likely combination.  Handling that in Kconfig seems fine, unless there's
> a concrete use case for that combination.

I doubt there is.  This is more for randconfig purposes or the like.

I suspect there is more of a case for having net drivers _without_ ptp 
support.  This could be implemented with a ptp_clock_register() stub 
returning NULL when ptp is not configured.  I didn't look at most 
drivers but at least broadcom/tg3.c seems to be fine with such an 
approach.

Alternatively, all those ethernet drivers currently selecting 
PTP_1588_CLOCK could be banned from the kernel config when POSIX_TIMERS 
is not selected.

What do people prefer?


Nicolas

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3) posix-timers: make it configurable
  2016-09-15 21:35                   ` Nicolas Pitre
@ 2016-09-15 21:56                     ` Josh Triplett
  2016-09-16  7:24                       ` Richard Cochran
  0 siblings, 1 reply; 25+ messages in thread
From: Josh Triplett @ 2016-09-15 21:56 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Richard Cochran, John Stultz, Thomas Gleixner, lkml

On Thu, Sep 15, 2016 at 05:35:28PM -0400, Nicolas Pitre wrote:
> On Thu, 15 Sep 2016, Josh Triplett wrote:
> 
> > On Thu, Sep 15, 2016 at 11:07:24PM +0200, Richard Cochran wrote:
> > > On Thu, Sep 15, 2016 at 12:58:22PM -0700, John Stultz wrote:
> > > > This doesn't look too bad.
> > > 
> > > I disagree.  It looks ugly.  If tinification means sprinkling more and
> > > more of these conditionals all over the place, then it is going to be
> > > a tough sell.
> > 
> > Looking at this particular patch, it does seem a bit much for the
> > ability to have PTP without timers.  That doesn't seem like a very
> > likely combination.  Handling that in Kconfig seems fine, unless there's
> > a concrete use case for that combination.
> 
> I doubt there is.  This is more for randconfig purposes or the like.
> 
> I suspect there is more of a case for having net drivers _without_ ptp 
> support.  This could be implemented with a ptp_clock_register() stub 
> returning NULL when ptp is not configured.  I didn't look at most 
> drivers but at least broadcom/tg3.c seems to be fine with such an 
> approach.
> 
> Alternatively, all those ethernet drivers currently selecting 
> PTP_1588_CLOCK could be banned from the kernel config when POSIX_TIMERS 
> is not selected.
> 
> What do people prefer?

If the stubs prove as simple as you suggest above (a static inline
returning NULL), that sounds ideal.  If this would require a non-trivial
amount of stub code, then preventing those drivers from building without
POSIX_TIMERS seems preferable to that.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3) posix-timers: make it configurable
  2016-09-15 21:56                     ` Josh Triplett
@ 2016-09-16  7:24                       ` Richard Cochran
  2016-09-17  2:57                         ` Nicolas Pitre
  0 siblings, 1 reply; 25+ messages in thread
From: Richard Cochran @ 2016-09-16  7:24 UTC (permalink / raw)
  To: Josh Triplett; +Cc: Nicolas Pitre, John Stultz, Thomas Gleixner, lkml

On Thu, Sep 15, 2016 at 02:56:49PM -0700, Josh Triplett wrote:
> > I suspect there is more of a case for having net drivers _without_ ptp 
> > support.  This could be implemented with a ptp_clock_register() stub 
> > returning NULL when ptp is not configured.  I didn't look at most 
> > drivers but at least broadcom/tg3.c seems to be fine with such an 
> > approach.

(I wouldn't be suprised if some drivers fail to deal with NULL
gracefully, but those can always be fixed ;)

> > Alternatively, all those ethernet drivers currently selecting 
> > PTP_1588_CLOCK could be banned from the kernel config when POSIX_TIMERS 
> > is not selected.
> > 
> > What do people prefer?
> 
> If the stubs prove as simple as you suggest above (a static inline
> returning NULL), that sounds ideal.  If this would require a non-trivial
> amount of stub code, then preventing those drivers from building without
> POSIX_TIMERS seems preferable to that.

I agree that stubs are the better solution.  There are only five
functions that deal with 'struct ptp_clock' at all.  The stubs could
be:

static inline
struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
				     struct device *parent) { return NULL; }

static inline
int ptp_clock_unregister(struct ptp_clock *ptp) { return -1; }

static inline
void ptp_clock_event(struct ptp_clock *ptp, struct ptp_clock_event *event) { }

static inline
int ptp_clock_index(struct ptp_clock *ptp) { return -1; }

static inline
int ptp_find_pin(struct ptp_clock *ptp,
		 enum ptp_pin_function func, unsigned int chan) { return -1; }

Thanks,
Richard

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3) posix-timers: make it configurable
  2016-09-15  3:47 [PATCH v3) posix-timers: make it configurable Nicolas Pitre
  2016-09-15 17:48 ` John Stultz
  2016-09-15 18:13 ` John Stultz
@ 2016-09-16 14:31 ` kbuild test robot
  2 siblings, 0 replies; 25+ messages in thread
From: kbuild test robot @ 2016-09-16 14:31 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: kbuild-all, Thomas Gleixner, John Stultz, Josh Triplett,
	Richard Cochran, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1349 bytes --]

Hi Nicolas,

[auto build test WARNING on linus/master]
[also build test WARNING on v4.8-rc6 next-20160916]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]

url:    https://github.com/0day-ci/linux/commits/Nicolas-Pitre/PATCH-v3-posix-timers-make-it-configurable/20160915-122808
config: x86_64-randconfig-a0-09161200 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

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)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27367 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3) posix-timers: make it configurable
  2016-09-16  7:24                       ` Richard Cochran
@ 2016-09-17  2:57                         ` Nicolas Pitre
  2016-09-18 14:35                           ` Richard Cochran
  0 siblings, 1 reply; 25+ messages in thread
From: Nicolas Pitre @ 2016-09-17  2:57 UTC (permalink / raw)
  To: Richard Cochran; +Cc: Josh Triplett, John Stultz, Thomas Gleixner, lkml

On Fri, 16 Sep 2016, Richard Cochran wrote:

> On Thu, Sep 15, 2016 at 02:56:49PM -0700, Josh Triplett wrote:
> > > I suspect there is more of a case for having net drivers _without_ ptp 
> > > support.  This could be implemented with a ptp_clock_register() stub 
> > > returning NULL when ptp is not configured.  I didn't look at most 
> > > drivers but at least broadcom/tg3.c seems to be fine with such an 
> > > approach.
> 
> (I wouldn't be suprised if some drivers fail to deal with NULL
> gracefully, but those can always be fixed ;)

I've looked at all of them and I think they should be fine with the 
minor changes I've included below.  Tested with allyesconfig and 
CONFIG_POSIX_TIMERS=n.

----- >8
Subject: [PATCH] ptp_clock: allow for it to be optional

In order to break the hard dependency between the PTP clock subsystem and
ethernet drivers capable of being clock providers, this patch provides
simple PTP stub functions to allow linkage of those drivers into the
kernel even when the PTP subsystem is configured out.

And to make it possible for PTP to be configured out, the select statement
in the Kconfig entry for those ethernet drivers is changed from selecting
PTP_1588_CLOCK to PTP_1588_CLOCK_DEFAULT whose purpose is to indicate the
default Kconfig value for the PTP subsystem.

This way the PTP subsystem may have Kconfig dependencies of its own, such
as POSIX_TIMERS, without making those ethernet drivers unavailable if
POSIX timers are cconfigured out. And when support for POSIX timers is
selected again then PTP clock support will also be selected accordingly.

Drivers must be ready to accept NULL from ptp_clock_register().
The pch_gbe driver is a bit special as it relies on extra code in
drivers/ptp/ptp_pch.c. Therefore we let the make process descend into
drivers/ptp/ even if PTP_1588_CLOCK is unselected.

Signed-off-by: Nicolas Pitre <nico@linaro.org>

diff --git a/drivers/Makefile b/drivers/Makefile
index 53abb4a5f7..8a538d0856 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -105,7 +105,7 @@ obj-$(CONFIG_INPUT)		+= input/
 obj-$(CONFIG_RTC_LIB)		+= rtc/
 obj-y				+= i2c/ media/
 obj-$(CONFIG_PPS)		+= pps/
-obj-$(CONFIG_PTP_1588_CLOCK)	+= ptp/
+obj-y				+= ptp/
 obj-$(CONFIG_W1)		+= w1/
 obj-y				+= power/
 obj-$(CONFIG_HWMON)		+= hwmon/
diff --git a/drivers/net/ethernet/adi/Kconfig b/drivers/net/ethernet/adi/Kconfig
index 6b94ba6103..eea414337c 100644
--- a/drivers/net/ethernet/adi/Kconfig
+++ b/drivers/net/ethernet/adi/Kconfig
@@ -55,10 +55,14 @@ config BFIN_RX_DESC_NUM
 	---help---
 	  Set the number of buffer packets used in driver.
 
+config BFIN_MAC_HAS_HWSTAMP
+	def_tristate BFIN_MAC
+	depends on BF518
+	select PTP_1588_CLOCK_DEFAULT
+
 config BFIN_MAC_USE_HWSTAMP
 	bool "Use IEEE 1588 hwstamp"
-	depends on BFIN_MAC && BF518
-	select PTP_1588_CLOCK
+	depends on BFIN_MAC_HAS_HWSTAMP && PTP_1588_CLOCK
 	default y
 	---help---
 	  To support the IEEE 1588 Precision Time Protocol (PTP), select y here
diff --git a/drivers/net/ethernet/amd/Kconfig b/drivers/net/ethernet/amd/Kconfig
index 0038709fd3..c2aaa5f949 100644
--- a/drivers/net/ethernet/amd/Kconfig
+++ b/drivers/net/ethernet/amd/Kconfig
@@ -177,7 +177,7 @@ config AMD_XGBE
 	depends on ARM64 || COMPILE_TEST
 	select BITREVERSE
 	select CRC32
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_DEFAULT
 	---help---
 	  This driver supports the AMD 10GbE Ethernet device found on an
 	  AMD SoC.
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-main.c b/drivers/net/ethernet/amd/xgbe/xgbe-main.c
index 3eee3201b5..4aeeb018b6 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-main.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-main.c
@@ -773,7 +773,8 @@ static int xgbe_probe(struct platform_device *pdev)
 		goto err_wq;
 	}
 
-	xgbe_ptp_register(pdata);
+	if (IS_REACHABLE(CONFIG_PTP_1588_CLOCK))
+		xgbe_ptp_register(pdata);
 
 	xgbe_debugfs_init(pdata);
 
@@ -812,7 +813,8 @@ static int xgbe_remove(struct platform_device *pdev)
 
 	xgbe_debugfs_exit(pdata);
 
-	xgbe_ptp_unregister(pdata);
+	if (IS_REACHABLE(CONFIG_PTP_1588_CLOCK))
+		xgbe_ptp_unregister(pdata);
 
 	flush_workqueue(pdata->an_workqueue);
 	destroy_workqueue(pdata->an_workqueue);
diff --git a/drivers/net/ethernet/broadcom/Kconfig b/drivers/net/ethernet/broadcom/Kconfig
index bd8c80c0b7..9c53786a5c 100644
--- a/drivers/net/ethernet/broadcom/Kconfig
+++ b/drivers/net/ethernet/broadcom/Kconfig
@@ -110,7 +110,7 @@ config TIGON3
 	depends on PCI
 	select PHYLIB
 	select HWMON
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_DEFAULT
 	---help---
 	  This driver supports Broadcom Tigon3 based gigabit Ethernet cards.
 
@@ -120,7 +120,7 @@ config TIGON3
 config BNX2X
 	tristate "Broadcom NetXtremeII 10Gb support"
 	depends on PCI
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_DEFAULT
 	select FW_LOADER
 	select ZLIB_INFLATE
 	select LIBCRC32C
diff --git a/drivers/net/ethernet/cavium/Kconfig b/drivers/net/ethernet/cavium/Kconfig
index e1b78b5003..ee08b27e4f 100644
--- a/drivers/net/ethernet/cavium/Kconfig
+++ b/drivers/net/ethernet/cavium/Kconfig
@@ -53,7 +53,7 @@ config	THUNDER_NIC_RGX
 config LIQUIDIO
 	tristate "Cavium LiquidIO support"
 	depends on 64BIT
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_DEFAULT
 	select FW_LOADER
 	select LIBCRC32C
 	---help---
diff --git a/drivers/net/ethernet/freescale/Kconfig b/drivers/net/ethernet/freescale/Kconfig
index d1ca45fbb1..3b46e167e6 100644
--- a/drivers/net/ethernet/freescale/Kconfig
+++ b/drivers/net/ethernet/freescale/Kconfig
@@ -25,7 +25,7 @@ config FEC
 		   ARCH_MXC || SOC_IMX28)
 	default ARCH_MXC || SOC_IMX28 if ARM
 	select PHYLIB
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_DEFAULT
 	---help---
 	  Say Y here if you want to use the built-in 10/100 Fast ethernet
 	  controller on some Motorola ColdFire and Freescale i.MX processors.
diff --git a/drivers/net/ethernet/intel/Kconfig b/drivers/net/ethernet/intel/Kconfig
index c0e17433f6..d41b455e5f 100644
--- a/drivers/net/ethernet/intel/Kconfig
+++ b/drivers/net/ethernet/intel/Kconfig
@@ -58,7 +58,7 @@ config E1000E
 	tristate "Intel(R) PRO/1000 PCI-Express Gigabit Ethernet support"
 	depends on PCI && (!SPARC32 || BROKEN)
 	select CRC32
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_DEFAULT
 	---help---
 	  This driver supports the PCI-Express Intel(R) PRO/1000 gigabit
 	  ethernet family of adapters. For PCI or PCI-X e1000 adapters,
@@ -83,7 +83,7 @@ config E1000E_HWTS
 config IGB
 	tristate "Intel(R) 82575/82576 PCI-Express Gigabit Ethernet support"
 	depends on PCI
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_DEFAULT
 	select I2C
 	select I2C_ALGOBIT
 	---help---
@@ -156,7 +156,7 @@ config IXGBE
 	tristate "Intel(R) 10GbE PCI Express adapters support"
 	depends on PCI
 	select MDIO
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_DEFAULT
 	---help---
 	  This driver supports Intel(R) 10GbE PCI Express family of
 	  adapters.  For more information on how to identify your adapter, go
@@ -213,7 +213,7 @@ config IXGBEVF
 
 config I40E
 	tristate "Intel(R) Ethernet Controller XL710 Family support"
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_DEFAULT
 	depends on PCI
 	---help---
 	  This driver supports Intel(R) Ethernet Controller XL710 Family of
@@ -264,7 +264,7 @@ config FM10K
 	tristate "Intel(R) FM10000 Ethernet Switch Host Interface Support"
 	default n
 	depends on PCI_MSI
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_DEFAULT
 	---help---
 	  This driver supports Intel(R) FM10000 Ethernet Switch Host
 	  Interface.  For more information on how to identify your adapter,
diff --git a/drivers/net/ethernet/intel/e1000e/ptp.c b/drivers/net/ethernet/intel/e1000e/ptp.c
index 2e1b17ad52..ad03763e00 100644
--- a/drivers/net/ethernet/intel/e1000e/ptp.c
+++ b/drivers/net/ethernet/intel/e1000e/ptp.c
@@ -334,7 +334,7 @@ void e1000e_ptp_init(struct e1000_adapter *adapter)
 	if (IS_ERR(adapter->ptp_clock)) {
 		adapter->ptp_clock = NULL;
 		e_err("ptp_clock_register failed\n");
-	} else {
+	} else if (adapter->ptp_clock) {
 		e_info("registered PHC clock\n");
 	}
 }
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
index ed39cbad24..f1feceab75 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
@@ -669,7 +669,7 @@ void i40e_ptp_init(struct i40e_pf *pf)
 		pf->ptp_clock = NULL;
 		dev_err(&pf->pdev->dev, "%s: ptp_clock_register failed\n",
 			__func__);
-	} else {
+	} else if (pf->ptp_clock) {
 		struct timespec64 ts;
 		u32 regval;
 
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index 66dfa2085c..1dd14e166d 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -1159,7 +1159,7 @@ void igb_ptp_init(struct igb_adapter *adapter)
 	if (IS_ERR(adapter->ptp_clock)) {
 		adapter->ptp_clock = NULL;
 		dev_err(&adapter->pdev->dev, "ptp_clock_register failed\n");
-	} else {
+	} else if (adapter->ptp_clock) {
 		dev_info(&adapter->pdev->dev, "added PHC on %s\n",
 			 adapter->netdev->name);
 		adapter->ptp_flags |= IGB_PTP_ENABLED;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
index e5431bfe33..a92277683a 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
@@ -1254,7 +1254,7 @@ static long ixgbe_ptp_create_clock(struct ixgbe_adapter *adapter)
 		adapter->ptp_clock = NULL;
 		e_dev_err("ptp_clock_register failed\n");
 		return err;
-	} else
+	} else if (adapter->ptp_clock)
 		e_dev_info("registered PHC device on %s\n", netdev->name);
 
 	/* set default timestamp mode to disabled here. We do this in
diff --git a/drivers/net/ethernet/mellanox/mlx4/Kconfig b/drivers/net/ethernet/mellanox/mlx4/Kconfig
index 5098e7f219..bc37746046 100644
--- a/drivers/net/ethernet/mellanox/mlx4/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlx4/Kconfig
@@ -7,7 +7,7 @@ config MLX4_EN
 	depends on MAY_USE_DEVLINK
 	depends on PCI
 	select MLX4_CORE
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_DEFAULT
 	---help---
 	  This driver supports Mellanox Technologies ConnectX Ethernet
 	  devices.
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_clock.c b/drivers/net/ethernet/mellanox/mlx4/en_clock.c
index 1494997c4f..08fc5fc56d 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_clock.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_clock.c
@@ -298,7 +298,7 @@ void mlx4_en_init_timestamp(struct mlx4_en_dev *mdev)
 	if (IS_ERR(mdev->ptp_clock)) {
 		mdev->ptp_clock = NULL;
 		mlx4_err(mdev, "ptp_clock_register failed\n");
-	} else {
+	} else if (mdev->ptp_clock) {
 		mlx4_info(mdev, "registered PHC clock\n");
 	}
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
index aae46884bf..14e06b6cea 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
@@ -14,7 +14,7 @@ config MLX5_CORE
 config MLX5_CORE_EN
 	bool "Mellanox Technologies ConnectX-4 Ethernet support"
 	depends on NETDEVICES && ETHERNET && PCI && MLX5_CORE
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_DEFAULT
 	default n
 	---help---
 	  Ethernet support in Mellanox Technologies ConnectX-4 NIC.
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_clock.c b/drivers/net/ethernet/mellanox/mlx5/core/en_clock.c
index 847a8f3ac2..13dc388667 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_clock.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_clock.c
@@ -273,7 +273,7 @@ void mlx5e_timestamp_init(struct mlx5e_priv *priv)
 
 	tstamp->ptp = ptp_clock_register(&tstamp->ptp_info,
 					 &priv->mdev->pdev->dev);
-	if (IS_ERR_OR_NULL(tstamp->ptp)) {
+	if (IS_ERR(tstamp->ptp)) {
 		mlx5_core_warn(priv->mdev, "ptp_clock_register failed %ld\n",
 			       PTR_ERR(tstamp->ptp));
 		tstamp->ptp = NULL;
diff --git a/drivers/net/ethernet/renesas/Kconfig b/drivers/net/ethernet/renesas/Kconfig
index 4f132cf177..ab2dec643f 100644
--- a/drivers/net/ethernet/renesas/Kconfig
+++ b/drivers/net/ethernet/renesas/Kconfig
@@ -37,7 +37,7 @@ config RAVB
 	select MII
 	select MDIO_BITBANG
 	select PHYLIB
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_DEFAULT
 	help
 	  Renesas Ethernet AVB device driver.
 	  This driver supports the following SoCs:
diff --git a/drivers/net/ethernet/samsung/Kconfig b/drivers/net/ethernet/samsung/Kconfig
index 2360d81507..14fb8185d2 100644
--- a/drivers/net/ethernet/samsung/Kconfig
+++ b/drivers/net/ethernet/samsung/Kconfig
@@ -21,7 +21,7 @@ config SXGBE_ETH
 	depends on HAS_IOMEM && HAS_DMA
 	select PHYLIB
 	select CRC32
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_DEFAULT
 	---help---
 	  This is the driver for the SXGBE 10G Ethernet IP block found on
 	  Samsung platforms.
diff --git a/drivers/net/ethernet/sfc/Kconfig b/drivers/net/ethernet/sfc/Kconfig
index 4dd92b7b80..623731ce00 100644
--- a/drivers/net/ethernet/sfc/Kconfig
+++ b/drivers/net/ethernet/sfc/Kconfig
@@ -5,7 +5,7 @@ config SFC
 	select CRC32
 	select I2C
 	select I2C_ALGOBIT
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_DEFAULT
 	---help---
 	  This driver supports 10/40-gigabit Ethernet cards based on
 	  the Solarflare SFC4000, SFC9000-family and SFC9100-family
diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index dd204d9704..77a5364f7a 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c
@@ -1269,13 +1269,13 @@ int efx_ptp_probe(struct efx_nic *efx, struct efx_channel *channel)
 		if (IS_ERR(ptp->phc_clock)) {
 			rc = PTR_ERR(ptp->phc_clock);
 			goto fail3;
-		}
-
-		INIT_WORK(&ptp->pps_work, efx_ptp_pps_worker);
-		ptp->pps_workwq = create_singlethread_workqueue("sfc_pps");
-		if (!ptp->pps_workwq) {
-			rc = -ENOMEM;
-			goto fail4;
+		} else if (ptp->phc_clock) {
+			INIT_WORK(&ptp->pps_work, efx_ptp_pps_worker);
+			ptp->pps_workwq = create_singlethread_workqueue("sfc_pps");
+			if (!ptp->pps_workwq) {
+				rc = -ENOMEM;
+				goto fail4;
+			}
 		}
 	}
 	ptp->nic_ts_enabled = false;
diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index 8f06a6621a..4bafe20d5f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -4,7 +4,7 @@ config STMMAC_ETH
 	select MII
 	select PHYLIB
 	select CRC32
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_DEFAULT
 	select RESET_CONTROLLER
 	---help---
 	  This is the driver for the Ethernet IPs are built around a
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
index 170a18b612..6e3b82972c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
@@ -187,7 +187,7 @@ int stmmac_ptp_register(struct stmmac_priv *priv)
 	if (IS_ERR(priv->ptp_clock)) {
 		priv->ptp_clock = NULL;
 		pr_err("ptp_clock_register() failed on %s\n", priv->dev->name);
-	} else
+	} else if (priv->ptp_clock)
 		pr_debug("Added PTP HW clock successfully on %s\n",
 			 priv->dev->name);
 
diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
index 9904d740d5..c7ae90858f 100644
--- a/drivers/net/ethernet/ti/Kconfig
+++ b/drivers/net/ethernet/ti/Kconfig
@@ -76,7 +76,7 @@ config TI_CPSW
 config TI_CPTS
 	bool "TI Common Platform Time Sync (CPTS) Support"
 	depends on TI_CPSW
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_DEFAULT
 	---help---
 	  This driver supports the Common Platform Time Sync unit of
 	  the CPSW Ethernet Switch. The unit can time stamp PTP UDP/IPv4
diff --git a/drivers/net/ethernet/tile/Kconfig b/drivers/net/ethernet/tile/Kconfig
index f59a6c2653..d21cd822c3 100644
--- a/drivers/net/ethernet/tile/Kconfig
+++ b/drivers/net/ethernet/tile/Kconfig
@@ -9,7 +9,7 @@ config TILE_NET
 	select CRC32
 	select TILE_GXIO_MPIPE if TILEGX
 	select HIGH_RES_TIMERS if TILEGX
-	select PTP_1588_CLOCK if TILEGX
+	select PTP_1588_CLOCK_DEFAULT if TILEGX
 	---help---
 	  This is a standard Linux network device driver for the
 	  on-chip Tilera Gigabit Ethernet and XAUI interfaces.
diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
index 00e6098e9a..c36c018f0f 100644
--- a/drivers/ptp/Kconfig
+++ b/drivers/ptp/Kconfig
@@ -4,8 +4,12 @@
 
 menu "PTP clock support"
 
+config PTP_1588_CLOCK_DEFAULT
+	tristate
+
 config PTP_1588_CLOCK
 	tristate "PTP clock support"
+	default PTP_1588_CLOCK_DEFAULT
 	depends on NET && POSIX_TIMERS
 	select PPS
 	select NET_PTP_CLASSIFY
@@ -28,7 +32,7 @@ config PTP_1588_CLOCK
 config PTP_1588_CLOCK_GIANFAR
 	tristate "Freescale eTSEC as PTP clock"
 	depends on GIANFAR
-	select PTP_1588_CLOCK
+	depends on PTP_1588_CLOCK
 	default y
 	help
 	  This driver adds support for using the eTSEC as a PTP
@@ -42,7 +46,7 @@ config PTP_1588_CLOCK_GIANFAR
 config PTP_1588_CLOCK_IXP46X
 	tristate "Intel IXP46x as PTP clock"
 	depends on IXP4XX_ETH
-	select PTP_1588_CLOCK
+	depends on PTP_1588_CLOCK
 	default y
 	help
 	  This driver adds support for using the IXP46X as a PTP
@@ -60,7 +64,7 @@ config DP83640_PHY
 	tristate "Driver for the National Semiconductor DP83640 PHYTER"
 	depends on NETWORK_PHY_TIMESTAMPING
 	depends on PHYLIB
-	select PTP_1588_CLOCK
+	depends on PTP_1588_CLOCK
 	---help---
 	  Supports the DP83640 PHYTER with IEEE 1588 features.
 
@@ -76,7 +80,7 @@ config PTP_1588_CLOCK_PCH
 	tristate "Intel PCH EG20T as PTP clock"
 	depends on X86_32 || COMPILE_TEST
 	depends on HAS_IOMEM && NET
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_DEFAULT
 	help
 	  This driver adds support for using the PCH EG20T as a PTP
 	  clock. The hardware supports time stamping of PTP packets
diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index 6b15e16814..4ce8bf74be 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -122,24 +122,6 @@ struct ptp_clock_info {
 
 struct ptp_clock;
 
-/**
- * ptp_clock_register() - register a PTP hardware clock driver
- *
- * @info:   Structure describing the new clock.
- * @parent: Pointer to the parent device of the new clock.
- */
-
-extern struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
-					    struct device *parent);
-
-/**
- * ptp_clock_unregister() - unregister a PTP hardware clock driver
- *
- * @ptp:  The clock to remove from service.
- */
-
-extern int ptp_clock_unregister(struct ptp_clock *ptp);
-
 
 enum ptp_clock_events {
 	PTP_CLOCK_ALARM,
@@ -166,6 +148,26 @@ struct ptp_clock_event {
 	};
 };
 
+#if IS_REACHABLE(CONFIG_PTP_1588_CLOCK)
+
+/**
+ * ptp_clock_register() - register a PTP hardware clock driver
+ *
+ * @info:   Structure describing the new clock.
+ * @parent: Pointer to the parent device of the new clock.
+ */
+
+extern struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
+					    struct device *parent);
+
+/**
+ * ptp_clock_unregister() - unregister a PTP hardware clock driver
+ *
+ * @ptp:  The clock to remove from service.
+ */
+
+extern int ptp_clock_unregister(struct ptp_clock *ptp);
+
 /**
  * ptp_clock_event() - notify the PTP layer about an event
  *
@@ -197,4 +199,20 @@ extern int ptp_clock_index(struct ptp_clock *ptp);
 int ptp_find_pin(struct ptp_clock *ptp,
 		 enum ptp_pin_function func, unsigned int chan);
 
+#else
+static inline struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
+						   struct device *parent)
+{ return NULL; }
+static inline int ptp_clock_unregister(struct ptp_clock *ptp)
+{ return 0; }
+static inline void ptp_clock_event(struct ptp_clock *ptp,
+				   struct ptp_clock_event *event)
+{ (void)event; }
+static inline int ptp_clock_index(struct ptp_clock *ptp)
+{ return -1; }
+static inline int ptp_find_pin(struct ptp_clock *ptp,
+			       enum ptp_pin_function func, unsigned int chan)
+{ return -1; }
+#endif
+
 #endif

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH v3) posix-timers: make it configurable
  2016-09-17  2:57                         ` Nicolas Pitre
@ 2016-09-18 14:35                           ` Richard Cochran
  2016-09-18 16:54                             ` Nicolas Pitre
  0 siblings, 1 reply; 25+ messages in thread
From: Richard Cochran @ 2016-09-18 14:35 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Josh Triplett, John Stultz, Thomas Gleixner, lkml

On Fri, Sep 16, 2016 at 10:57:58PM -0400, Nicolas Pitre wrote:
> Subject: [PATCH] ptp_clock: allow for it to be optional
> 
> In order to break the hard dependency between the PTP clock subsystem and
> ethernet drivers capable of being clock providers, this patch provides
> simple PTP stub functions to allow linkage of those drivers into the
> kernel even when the PTP subsystem is configured out.
> 
> And to make it possible for PTP to be configured out, the select statement
> in the Kconfig entry for those ethernet drivers is changed from selecting
> PTP_1588_CLOCK to PTP_1588_CLOCK_DEFAULT whose purpose is to indicate the
> default Kconfig value for the PTP subsystem.
> 
> This way the PTP subsystem may have Kconfig dependencies of its own, such
> as POSIX_TIMERS, without making those ethernet drivers unavailable if
> POSIX timers are cconfigured out. And when support for POSIX timers is
> selected again then PTP clock support will also be selected accordingly.
> 
> Drivers must be ready to accept NULL from ptp_clock_register().
> The pch_gbe driver is a bit special as it relies on extra code in
> drivers/ptp/ptp_pch.c. Therefore we let the make process descend into
> drivers/ptp/ even if PTP_1588_CLOCK is unselected.

Thanks for the detailed description.

> diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
> index 00e6098e9a..c36c018f0f 100644
> --- a/drivers/ptp/Kconfig
> +++ b/drivers/ptp/Kconfig
> @@ -4,8 +4,12 @@
>  
>  menu "PTP clock support"
>  
> +config PTP_1588_CLOCK_DEFAULT
> +	tristate

I see what this option is doing, but I wonder about the name
"DEFAULT".  In what sense is this a default?

> +/**
> + * ptp_clock_register() - register a PTP hardware clock driver
> + *
> + * @info:   Structure describing the new clock.
> + * @parent: Pointer to the parent device of the new clock.

Here we should finally explain the return value for authors of new
drivers.  Something like this:

 * Returns a valid pointer on success or PTR_ERR on failure.  If PHC
 * support is missing at the configuration level, this function
 * returns NULL, and drivers are expected to gracefully handle that
 * case separately.

> + */
> +
> +extern struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
> +					    struct device *parent);

Thanks,
Richard

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3) posix-timers: make it configurable
  2016-09-18 14:35                           ` Richard Cochran
@ 2016-09-18 16:54                             ` Nicolas Pitre
  2016-09-18 18:20                               ` Richard Cochran
  0 siblings, 1 reply; 25+ messages in thread
From: Nicolas Pitre @ 2016-09-18 16:54 UTC (permalink / raw)
  To: Richard Cochran; +Cc: Josh Triplett, John Stultz, Thomas Gleixner, lkml

On Sun, 18 Sep 2016, Richard Cochran wrote:

> On Fri, Sep 16, 2016 at 10:57:58PM -0400, Nicolas Pitre wrote:
> > Subject: [PATCH] ptp_clock: allow for it to be optional
> > 
> > In order to break the hard dependency between the PTP clock subsystem and
> > ethernet drivers capable of being clock providers, this patch provides
> > simple PTP stub functions to allow linkage of those drivers into the
> > kernel even when the PTP subsystem is configured out.
> > 
> > And to make it possible for PTP to be configured out, the select statement
> > in the Kconfig entry for those ethernet drivers is changed from selecting
> > PTP_1588_CLOCK to PTP_1588_CLOCK_DEFAULT whose purpose is to indicate the
> > default Kconfig value for the PTP subsystem.
> > 
> > This way the PTP subsystem may have Kconfig dependencies of its own, such
> > as POSIX_TIMERS, without making those ethernet drivers unavailable if
> > POSIX timers are cconfigured out. And when support for POSIX timers is
> > selected again then PTP clock support will also be selected accordingly.
> > 
> > Drivers must be ready to accept NULL from ptp_clock_register().
> > The pch_gbe driver is a bit special as it relies on extra code in
> > drivers/ptp/ptp_pch.c. Therefore we let the make process descend into
> > drivers/ptp/ even if PTP_1588_CLOCK is unselected.
> 
> Thanks for the detailed description.
> 
> > diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
> > index 00e6098e9a..c36c018f0f 100644
> > --- a/drivers/ptp/Kconfig
> > +++ b/drivers/ptp/Kconfig
> > @@ -4,8 +4,12 @@
> >  
> >  menu "PTP clock support"
> >  
> > +config PTP_1588_CLOCK_DEFAULT
> > +	tristate
> 
> I see what this option is doing, but I wonder about the name
> "DEFAULT".  In what sense is this a default?

It provides the default answer for when the PTP_1588_CLOCK config option 
is available.  I could have called it HAVE_PTP_1588_CLOCK or 
PTP_1588_CLOCK_WANTED but none of the alternatives struck me as any 
better.  Any idea?

> > +/**
> > + * ptp_clock_register() - register a PTP hardware clock driver
> > + *
> > + * @info:   Structure describing the new clock.
> > + * @parent: Pointer to the parent device of the new clock.
> 
> Here we should finally explain the return value for authors of new
> drivers.  Something like this:
> 
>  * Returns a valid pointer on success or PTR_ERR on failure.  If PHC
>  * support is missing at the configuration level, this function
>  * returns NULL, and drivers are expected to gracefully handle that
>  * case separately.

Good point.


Nicolas

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3) posix-timers: make it configurable
  2016-09-18 16:54                             ` Nicolas Pitre
@ 2016-09-18 18:20                               ` Richard Cochran
  2016-09-18 18:49                                 ` Nicolas Pitre
  0 siblings, 1 reply; 25+ messages in thread
From: Richard Cochran @ 2016-09-18 18:20 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Josh Triplett, John Stultz, Thomas Gleixner, lkml

On Sun, Sep 18, 2016 at 12:54:58PM -0400, Nicolas Pitre wrote:
> > > +config PTP_1588_CLOCK_DEFAULT
> > > +	tristate
> > 
> > I see what this option is doing, but I wonder about the name
> > "DEFAULT".  In what sense is this a default?
> 
> It provides the default answer for when the PTP_1588_CLOCK config option 
> is available.  I could have called it HAVE_PTP_1588_CLOCK or 
> PTP_1588_CLOCK_WANTED but none of the alternatives struck me as any 
> better.  Any idea?

How about:  PTP_1588_CLOCK_SELECTED ?

Thanks,
Richard

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3) posix-timers: make it configurable
  2016-09-18 18:20                               ` Richard Cochran
@ 2016-09-18 18:49                                 ` Nicolas Pitre
  2016-09-18 20:22                                   ` Richard Cochran
  0 siblings, 1 reply; 25+ messages in thread
From: Nicolas Pitre @ 2016-09-18 18:49 UTC (permalink / raw)
  To: Richard Cochran; +Cc: Josh Triplett, John Stultz, Thomas Gleixner, lkml

On Sun, 18 Sep 2016, Richard Cochran wrote:

> On Sun, Sep 18, 2016 at 12:54:58PM -0400, Nicolas Pitre wrote:
> > > > +config PTP_1588_CLOCK_DEFAULT
> > > > +	tristate
> > > 
> > > I see what this option is doing, but I wonder about the name
> > > "DEFAULT".  In what sense is this a default?
> > 
> > It provides the default answer for when the PTP_1588_CLOCK config option 
> > is available.  I could have called it HAVE_PTP_1588_CLOCK or 
> > PTP_1588_CLOCK_WANTED but none of the alternatives struck me as any 
> > better.  Any idea?
> 
> How about:  PTP_1588_CLOCK_SELECTED ?

It sounds just as good to me.

Who should merge this patch?

----- >8
Subject: [PATCH v2] ptp_clock: allow for it to be optional

In order to break the hard dependency between the PTP clock subsystem and
ethernet drivers capable of being clock providers, this patch provides
simple PTP stub functions to allow linkage of those drivers into the
kernel even when the PTP subsystem is configured out.

And to make it possible for PTP to be configured out, the select statement
in the Kconfig entry for those ethernet drivers is changed from selecting
PTP_1588_CLOCK to PTP_1588_CLOCK_SELECTED whose purpose is to indicate the
default Kconfig value for the PTP subsystem.

This way the PTP subsystem may have Kconfig dependencies of its own, such
as POSIX_TIMERS, without making those ethernet drivers unavailable if
POSIX timers are cconfigured out. And when support for POSIX timers is
selected again then PTP clock support will also be selected accordingly.

Drivers must be ready to accept NULL from ptp_clock_register().
The pch_gbe driver is a bit special as it relies on extra code in
drivers/ptp/ptp_pch.c. Therefore we let the make process descend into
drivers/ptp/ even if PTP_1588_CLOCK is unselected.

Signed-off-by: Nicolas Pitre <nico@linaro.org>
---

Changes from v1:


- Documented ptp_clock_register() returning NULL in the code
- s/PTP_1588_CLOCK_DEFAULT/PTP_1588_CLOCK_SELECTED/

diff --git a/drivers/Makefile b/drivers/Makefile
index 53abb4a5f7..8a538d0856 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -105,7 +105,7 @@ obj-$(CONFIG_INPUT)		+= input/
 obj-$(CONFIG_RTC_LIB)		+= rtc/
 obj-y				+= i2c/ media/
 obj-$(CONFIG_PPS)		+= pps/
-obj-$(CONFIG_PTP_1588_CLOCK)	+= ptp/
+obj-y				+= ptp/
 obj-$(CONFIG_W1)		+= w1/
 obj-y				+= power/
 obj-$(CONFIG_HWMON)		+= hwmon/
diff --git a/drivers/net/ethernet/adi/Kconfig b/drivers/net/ethernet/adi/Kconfig
index 6b94ba6103..67094a9cfe 100644
--- a/drivers/net/ethernet/adi/Kconfig
+++ b/drivers/net/ethernet/adi/Kconfig
@@ -55,10 +55,14 @@ config BFIN_RX_DESC_NUM
 	---help---
 	  Set the number of buffer packets used in driver.
 
+config BFIN_MAC_HAS_HWSTAMP
+	def_tristate BFIN_MAC
+	depends on BF518
+	select PTP_1588_CLOCK_SELECTED
+
 config BFIN_MAC_USE_HWSTAMP
 	bool "Use IEEE 1588 hwstamp"
-	depends on BFIN_MAC && BF518
-	select PTP_1588_CLOCK
+	depends on BFIN_MAC_HAS_HWSTAMP && PTP_1588_CLOCK
 	default y
 	---help---
 	  To support the IEEE 1588 Precision Time Protocol (PTP), select y here
diff --git a/drivers/net/ethernet/amd/Kconfig b/drivers/net/ethernet/amd/Kconfig
index 0038709fd3..327e71a554 100644
--- a/drivers/net/ethernet/amd/Kconfig
+++ b/drivers/net/ethernet/amd/Kconfig
@@ -177,7 +177,7 @@ config AMD_XGBE
 	depends on ARM64 || COMPILE_TEST
 	select BITREVERSE
 	select CRC32
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_SELECTED
 	---help---
 	  This driver supports the AMD 10GbE Ethernet device found on an
 	  AMD SoC.
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-main.c b/drivers/net/ethernet/amd/xgbe/xgbe-main.c
index 3eee3201b5..4aeeb018b6 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-main.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-main.c
@@ -773,7 +773,8 @@ static int xgbe_probe(struct platform_device *pdev)
 		goto err_wq;
 	}
 
-	xgbe_ptp_register(pdata);
+	if (IS_REACHABLE(CONFIG_PTP_1588_CLOCK))
+		xgbe_ptp_register(pdata);
 
 	xgbe_debugfs_init(pdata);
 
@@ -812,7 +813,8 @@ static int xgbe_remove(struct platform_device *pdev)
 
 	xgbe_debugfs_exit(pdata);
 
-	xgbe_ptp_unregister(pdata);
+	if (IS_REACHABLE(CONFIG_PTP_1588_CLOCK))
+		xgbe_ptp_unregister(pdata);
 
 	flush_workqueue(pdata->an_workqueue);
 	destroy_workqueue(pdata->an_workqueue);
diff --git a/drivers/net/ethernet/broadcom/Kconfig b/drivers/net/ethernet/broadcom/Kconfig
index bd8c80c0b7..3db7eca92c 100644
--- a/drivers/net/ethernet/broadcom/Kconfig
+++ b/drivers/net/ethernet/broadcom/Kconfig
@@ -110,7 +110,7 @@ config TIGON3
 	depends on PCI
 	select PHYLIB
 	select HWMON
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_SELECTED
 	---help---
 	  This driver supports Broadcom Tigon3 based gigabit Ethernet cards.
 
@@ -120,7 +120,7 @@ config TIGON3
 config BNX2X
 	tristate "Broadcom NetXtremeII 10Gb support"
 	depends on PCI
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_SELECTED
 	select FW_LOADER
 	select ZLIB_INFLATE
 	select LIBCRC32C
diff --git a/drivers/net/ethernet/cavium/Kconfig b/drivers/net/ethernet/cavium/Kconfig
index e1b78b5003..5dd86cf683 100644
--- a/drivers/net/ethernet/cavium/Kconfig
+++ b/drivers/net/ethernet/cavium/Kconfig
@@ -53,7 +53,7 @@ config	THUNDER_NIC_RGX
 config LIQUIDIO
 	tristate "Cavium LiquidIO support"
 	depends on 64BIT
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_SELECTED
 	select FW_LOADER
 	select LIBCRC32C
 	---help---
diff --git a/drivers/net/ethernet/freescale/Kconfig b/drivers/net/ethernet/freescale/Kconfig
index d1ca45fbb1..775dbfeb7b 100644
--- a/drivers/net/ethernet/freescale/Kconfig
+++ b/drivers/net/ethernet/freescale/Kconfig
@@ -25,7 +25,7 @@ config FEC
 		   ARCH_MXC || SOC_IMX28)
 	default ARCH_MXC || SOC_IMX28 if ARM
 	select PHYLIB
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_SELECTED
 	---help---
 	  Say Y here if you want to use the built-in 10/100 Fast ethernet
 	  controller on some Motorola ColdFire and Freescale i.MX processors.
diff --git a/drivers/net/ethernet/intel/Kconfig b/drivers/net/ethernet/intel/Kconfig
index c0e17433f6..8bbfb43f09 100644
--- a/drivers/net/ethernet/intel/Kconfig
+++ b/drivers/net/ethernet/intel/Kconfig
@@ -58,7 +58,7 @@ config E1000E
 	tristate "Intel(R) PRO/1000 PCI-Express Gigabit Ethernet support"
 	depends on PCI && (!SPARC32 || BROKEN)
 	select CRC32
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_SELECTED
 	---help---
 	  This driver supports the PCI-Express Intel(R) PRO/1000 gigabit
 	  ethernet family of adapters. For PCI or PCI-X e1000 adapters,
@@ -83,7 +83,7 @@ config E1000E_HWTS
 config IGB
 	tristate "Intel(R) 82575/82576 PCI-Express Gigabit Ethernet support"
 	depends on PCI
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_SELECTED
 	select I2C
 	select I2C_ALGOBIT
 	---help---
@@ -156,7 +156,7 @@ config IXGBE
 	tristate "Intel(R) 10GbE PCI Express adapters support"
 	depends on PCI
 	select MDIO
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_SELECTED
 	---help---
 	  This driver supports Intel(R) 10GbE PCI Express family of
 	  adapters.  For more information on how to identify your adapter, go
@@ -213,7 +213,7 @@ config IXGBEVF
 
 config I40E
 	tristate "Intel(R) Ethernet Controller XL710 Family support"
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_SELECTED
 	depends on PCI
 	---help---
 	  This driver supports Intel(R) Ethernet Controller XL710 Family of
@@ -264,7 +264,7 @@ config FM10K
 	tristate "Intel(R) FM10000 Ethernet Switch Host Interface Support"
 	default n
 	depends on PCI_MSI
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_SELECTED
 	---help---
 	  This driver supports Intel(R) FM10000 Ethernet Switch Host
 	  Interface.  For more information on how to identify your adapter,
diff --git a/drivers/net/ethernet/intel/e1000e/ptp.c b/drivers/net/ethernet/intel/e1000e/ptp.c
index 2e1b17ad52..ad03763e00 100644
--- a/drivers/net/ethernet/intel/e1000e/ptp.c
+++ b/drivers/net/ethernet/intel/e1000e/ptp.c
@@ -334,7 +334,7 @@ void e1000e_ptp_init(struct e1000_adapter *adapter)
 	if (IS_ERR(adapter->ptp_clock)) {
 		adapter->ptp_clock = NULL;
 		e_err("ptp_clock_register failed\n");
-	} else {
+	} else if (adapter->ptp_clock) {
 		e_info("registered PHC clock\n");
 	}
 }
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
index ed39cbad24..f1feceab75 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
@@ -669,7 +669,7 @@ void i40e_ptp_init(struct i40e_pf *pf)
 		pf->ptp_clock = NULL;
 		dev_err(&pf->pdev->dev, "%s: ptp_clock_register failed\n",
 			__func__);
-	} else {
+	} else if (pf->ptp_clock) {
 		struct timespec64 ts;
 		u32 regval;
 
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index 66dfa2085c..1dd14e166d 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -1159,7 +1159,7 @@ void igb_ptp_init(struct igb_adapter *adapter)
 	if (IS_ERR(adapter->ptp_clock)) {
 		adapter->ptp_clock = NULL;
 		dev_err(&adapter->pdev->dev, "ptp_clock_register failed\n");
-	} else {
+	} else if (adapter->ptp_clock) {
 		dev_info(&adapter->pdev->dev, "added PHC on %s\n",
 			 adapter->netdev->name);
 		adapter->ptp_flags |= IGB_PTP_ENABLED;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
index e5431bfe33..a92277683a 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
@@ -1254,7 +1254,7 @@ static long ixgbe_ptp_create_clock(struct ixgbe_adapter *adapter)
 		adapter->ptp_clock = NULL;
 		e_dev_err("ptp_clock_register failed\n");
 		return err;
-	} else
+	} else if (adapter->ptp_clock)
 		e_dev_info("registered PHC device on %s\n", netdev->name);
 
 	/* set default timestamp mode to disabled here. We do this in
diff --git a/drivers/net/ethernet/mellanox/mlx4/Kconfig b/drivers/net/ethernet/mellanox/mlx4/Kconfig
index 5098e7f219..b2998bc5ab 100644
--- a/drivers/net/ethernet/mellanox/mlx4/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlx4/Kconfig
@@ -7,7 +7,7 @@ config MLX4_EN
 	depends on MAY_USE_DEVLINK
 	depends on PCI
 	select MLX4_CORE
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_SELECTED
 	---help---
 	  This driver supports Mellanox Technologies ConnectX Ethernet
 	  devices.
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_clock.c b/drivers/net/ethernet/mellanox/mlx4/en_clock.c
index 1494997c4f..08fc5fc56d 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_clock.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_clock.c
@@ -298,7 +298,7 @@ void mlx4_en_init_timestamp(struct mlx4_en_dev *mdev)
 	if (IS_ERR(mdev->ptp_clock)) {
 		mdev->ptp_clock = NULL;
 		mlx4_err(mdev, "ptp_clock_register failed\n");
-	} else {
+	} else if (mdev->ptp_clock) {
 		mlx4_info(mdev, "registered PHC clock\n");
 	}
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
index aae46884bf..0d679346dc 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
@@ -14,7 +14,7 @@ config MLX5_CORE
 config MLX5_CORE_EN
 	bool "Mellanox Technologies ConnectX-4 Ethernet support"
 	depends on NETDEVICES && ETHERNET && PCI && MLX5_CORE
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_SELECTED
 	default n
 	---help---
 	  Ethernet support in Mellanox Technologies ConnectX-4 NIC.
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_clock.c b/drivers/net/ethernet/mellanox/mlx5/core/en_clock.c
index 847a8f3ac2..13dc388667 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_clock.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_clock.c
@@ -273,7 +273,7 @@ void mlx5e_timestamp_init(struct mlx5e_priv *priv)
 
 	tstamp->ptp = ptp_clock_register(&tstamp->ptp_info,
 					 &priv->mdev->pdev->dev);
-	if (IS_ERR_OR_NULL(tstamp->ptp)) {
+	if (IS_ERR(tstamp->ptp)) {
 		mlx5_core_warn(priv->mdev, "ptp_clock_register failed %ld\n",
 			       PTR_ERR(tstamp->ptp));
 		tstamp->ptp = NULL;
diff --git a/drivers/net/ethernet/renesas/Kconfig b/drivers/net/ethernet/renesas/Kconfig
index 4f132cf177..6862a9c1b6 100644
--- a/drivers/net/ethernet/renesas/Kconfig
+++ b/drivers/net/ethernet/renesas/Kconfig
@@ -37,7 +37,7 @@ config RAVB
 	select MII
 	select MDIO_BITBANG
 	select PHYLIB
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_SELECTED
 	help
 	  Renesas Ethernet AVB device driver.
 	  This driver supports the following SoCs:
diff --git a/drivers/net/ethernet/samsung/Kconfig b/drivers/net/ethernet/samsung/Kconfig
index 2360d81507..121b7e4426 100644
--- a/drivers/net/ethernet/samsung/Kconfig
+++ b/drivers/net/ethernet/samsung/Kconfig
@@ -21,7 +21,7 @@ config SXGBE_ETH
 	depends on HAS_IOMEM && HAS_DMA
 	select PHYLIB
 	select CRC32
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_SELECTED
 	---help---
 	  This is the driver for the SXGBE 10G Ethernet IP block found on
 	  Samsung platforms.
diff --git a/drivers/net/ethernet/sfc/Kconfig b/drivers/net/ethernet/sfc/Kconfig
index 4dd92b7b80..472152bd72 100644
--- a/drivers/net/ethernet/sfc/Kconfig
+++ b/drivers/net/ethernet/sfc/Kconfig
@@ -5,7 +5,7 @@ config SFC
 	select CRC32
 	select I2C
 	select I2C_ALGOBIT
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_SELECTED
 	---help---
 	  This driver supports 10/40-gigabit Ethernet cards based on
 	  the Solarflare SFC4000, SFC9000-family and SFC9100-family
diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index dd204d9704..77a5364f7a 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c
@@ -1269,13 +1269,13 @@ int efx_ptp_probe(struct efx_nic *efx, struct efx_channel *channel)
 		if (IS_ERR(ptp->phc_clock)) {
 			rc = PTR_ERR(ptp->phc_clock);
 			goto fail3;
-		}
-
-		INIT_WORK(&ptp->pps_work, efx_ptp_pps_worker);
-		ptp->pps_workwq = create_singlethread_workqueue("sfc_pps");
-		if (!ptp->pps_workwq) {
-			rc = -ENOMEM;
-			goto fail4;
+		} else if (ptp->phc_clock) {
+			INIT_WORK(&ptp->pps_work, efx_ptp_pps_worker);
+			ptp->pps_workwq = create_singlethread_workqueue("sfc_pps");
+			if (!ptp->pps_workwq) {
+				rc = -ENOMEM;
+				goto fail4;
+			}
 		}
 	}
 	ptp->nic_ts_enabled = false;
diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index 8f06a6621a..578e5a15cb 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -4,7 +4,7 @@ config STMMAC_ETH
 	select MII
 	select PHYLIB
 	select CRC32
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_SELECTED
 	select RESET_CONTROLLER
 	---help---
 	  This is the driver for the Ethernet IPs are built around a
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
index 170a18b612..6e3b82972c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
@@ -187,7 +187,7 @@ int stmmac_ptp_register(struct stmmac_priv *priv)
 	if (IS_ERR(priv->ptp_clock)) {
 		priv->ptp_clock = NULL;
 		pr_err("ptp_clock_register() failed on %s\n", priv->dev->name);
-	} else
+	} else if (priv->ptp_clock)
 		pr_debug("Added PTP HW clock successfully on %s\n",
 			 priv->dev->name);
 
diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
index 9904d740d5..bc895114c9 100644
--- a/drivers/net/ethernet/ti/Kconfig
+++ b/drivers/net/ethernet/ti/Kconfig
@@ -76,7 +76,7 @@ config TI_CPSW
 config TI_CPTS
 	bool "TI Common Platform Time Sync (CPTS) Support"
 	depends on TI_CPSW
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_SELECTED
 	---help---
 	  This driver supports the Common Platform Time Sync unit of
 	  the CPSW Ethernet Switch. The unit can time stamp PTP UDP/IPv4
diff --git a/drivers/net/ethernet/tile/Kconfig b/drivers/net/ethernet/tile/Kconfig
index f59a6c2653..b6ba43c78c 100644
--- a/drivers/net/ethernet/tile/Kconfig
+++ b/drivers/net/ethernet/tile/Kconfig
@@ -9,7 +9,7 @@ config TILE_NET
 	select CRC32
 	select TILE_GXIO_MPIPE if TILEGX
 	select HIGH_RES_TIMERS if TILEGX
-	select PTP_1588_CLOCK if TILEGX
+	select PTP_1588_CLOCK_SELECTED if TILEGX
 	---help---
 	  This is a standard Linux network device driver for the
 	  on-chip Tilera Gigabit Ethernet and XAUI interfaces.
diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
index ee3de3421f..f34b3748c0 100644
--- a/drivers/ptp/Kconfig
+++ b/drivers/ptp/Kconfig
@@ -4,8 +4,12 @@
 
 menu "PTP clock support"
 
+config PTP_1588_CLOCK_SELECTED
+	tristate
+
 config PTP_1588_CLOCK
 	tristate "PTP clock support"
+	default PTP_1588_CLOCK_SELECTED
 	depends on NET
 	select PPS
 	select NET_PTP_CLASSIFY
@@ -28,7 +32,7 @@ config PTP_1588_CLOCK
 config PTP_1588_CLOCK_GIANFAR
 	tristate "Freescale eTSEC as PTP clock"
 	depends on GIANFAR
-	select PTP_1588_CLOCK
+	depends on PTP_1588_CLOCK
 	default y
 	help
 	  This driver adds support for using the eTSEC as a PTP
@@ -42,7 +46,7 @@ config PTP_1588_CLOCK_GIANFAR
 config PTP_1588_CLOCK_IXP46X
 	tristate "Intel IXP46x as PTP clock"
 	depends on IXP4XX_ETH
-	select PTP_1588_CLOCK
+	depends on PTP_1588_CLOCK
 	default y
 	help
 	  This driver adds support for using the IXP46X as a PTP
@@ -60,7 +64,7 @@ config DP83640_PHY
 	tristate "Driver for the National Semiconductor DP83640 PHYTER"
 	depends on NETWORK_PHY_TIMESTAMPING
 	depends on PHYLIB
-	select PTP_1588_CLOCK
+	depends on PTP_1588_CLOCK
 	---help---
 	  Supports the DP83640 PHYTER with IEEE 1588 features.
 
@@ -76,7 +80,7 @@ config PTP_1588_CLOCK_PCH
 	tristate "Intel PCH EG20T as PTP clock"
 	depends on X86_32 || COMPILE_TEST
 	depends on HAS_IOMEM && NET
-	select PTP_1588_CLOCK
+	select PTP_1588_CLOCK_SELECTED
 	help
 	  This driver adds support for using the PCH EG20T as a PTP
 	  clock. The hardware supports time stamping of PTP packets
diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index 6b15e16814..4c29eb8e53 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -122,24 +122,6 @@ struct ptp_clock_info {
 
 struct ptp_clock;
 
-/**
- * ptp_clock_register() - register a PTP hardware clock driver
- *
- * @info:   Structure describing the new clock.
- * @parent: Pointer to the parent device of the new clock.
- */
-
-extern struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
-					    struct device *parent);
-
-/**
- * ptp_clock_unregister() - unregister a PTP hardware clock driver
- *
- * @ptp:  The clock to remove from service.
- */
-
-extern int ptp_clock_unregister(struct ptp_clock *ptp);
-
 
 enum ptp_clock_events {
 	PTP_CLOCK_ALARM,
@@ -166,6 +148,31 @@ struct ptp_clock_event {
 	};
 };
 
+#if IS_REACHABLE(CONFIG_PTP_1588_CLOCK)
+
+/**
+ * ptp_clock_register() - register a PTP hardware clock driver
+ *
+ * @info:   Structure describing the new clock.
+ * @parent: Pointer to the parent device of the new clock.
+ *
+ * Returns a valid pointer on success or PTR_ERR on failure.  If PHC
+ * support is missing at the configuration level, this function
+ * returns NULL, and drivers are expected to gracefully handle that
+ * case separately.
+ */
+
+extern struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
+					    struct device *parent);
+
+/**
+ * ptp_clock_unregister() - unregister a PTP hardware clock driver
+ *
+ * @ptp:  The clock to remove from service.
+ */
+
+extern int ptp_clock_unregister(struct ptp_clock *ptp);
+
 /**
  * ptp_clock_event() - notify the PTP layer about an event
  *
@@ -197,4 +204,20 @@ extern int ptp_clock_index(struct ptp_clock *ptp);
 int ptp_find_pin(struct ptp_clock *ptp,
 		 enum ptp_pin_function func, unsigned int chan);
 
+#else
+static inline struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
+						   struct device *parent)
+{ return NULL; }
+static inline int ptp_clock_unregister(struct ptp_clock *ptp)
+{ return 0; }
+static inline void ptp_clock_event(struct ptp_clock *ptp,
+				   struct ptp_clock_event *event)
+{ (void)event; }
+static inline int ptp_clock_index(struct ptp_clock *ptp)
+{ return -1; }
+static inline int ptp_find_pin(struct ptp_clock *ptp,
+			       enum ptp_pin_function func, unsigned int chan)
+{ return -1; }
+#endif
+
 #endif

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH v3) posix-timers: make it configurable
  2016-09-18 18:49                                 ` Nicolas Pitre
@ 2016-09-18 20:22                                   ` Richard Cochran
  2016-09-18 20:30                                     ` Nicolas Pitre
  0 siblings, 1 reply; 25+ messages in thread
From: Richard Cochran @ 2016-09-18 20:22 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Josh Triplett, John Stultz, Thomas Gleixner, lkml

On Sun, Sep 18, 2016 at 02:49:08PM -0400, Nicolas Pitre wrote:
> Who should merge this patch?

John, I guess.  Or do we need acks from the driver maintainers?  In
that case, please post to netdev, and then davem can merge it.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3) posix-timers: make it configurable
  2016-09-18 20:22                                   ` Richard Cochran
@ 2016-09-18 20:30                                     ` Nicolas Pitre
  2016-09-18 21:11                                       ` Richard Cochran
  0 siblings, 1 reply; 25+ messages in thread
From: Nicolas Pitre @ 2016-09-18 20:30 UTC (permalink / raw)
  To: Richard Cochran; +Cc: Josh Triplett, John Stultz, Thomas Gleixner, lkml

On Sun, 18 Sep 2016, Richard Cochran wrote:

> On Sun, Sep 18, 2016 at 02:49:08PM -0400, Nicolas Pitre wrote:
> > Who should merge this patch?
> 
> John, I guess.  Or do we need acks from the driver maintainers?  In
> that case, please post to netdev, and then davem can merge it.

I don't think the driver changes are significant enough to warrant a ACK 
from individual driver maintainers.

Yours would be welcome though.  ;-)

Then I'll resend the whole thing to John as a series and CC netdev.

What do you think?


Nicolas

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3) posix-timers: make it configurable
  2016-09-18 20:30                                     ` Nicolas Pitre
@ 2016-09-18 21:11                                       ` Richard Cochran
  0 siblings, 0 replies; 25+ messages in thread
From: Richard Cochran @ 2016-09-18 21:11 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Josh Triplett, John Stultz, Thomas Gleixner, lkml

On Sun, Sep 18, 2016 at 04:30:21PM -0400, Nicolas Pitre wrote:
> I don't think the driver changes are significant enough to warrant a ACK 
> from individual driver maintainers.
> 
> Yours would be welcome though.  ;-)

Acked-by: Richard Cochran <richardcochran@gmail.com>

> Then I'll resend the whole thing to John as a series and CC netdev.
> 
> What do you think?

Sounds ok to me.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2016-09-18 21:11 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-15  3:47 [PATCH v3) posix-timers: make it configurable Nicolas Pitre
2016-09-15 17:48 ` John Stultz
2016-09-15 17:56   ` Nicolas Pitre
2016-09-15 18:13 ` John Stultz
2016-09-15 18:28   ` Nicolas Pitre
2016-09-15 18:35     ` John Stultz
2016-09-15 18:37       ` Nicolas Pitre
2016-09-15 18:46         ` John Stultz
2016-09-15 19:31           ` Nicolas Pitre
2016-09-15 19:58             ` John Stultz
2016-09-15 21:07               ` Richard Cochran
2016-09-15 21:15                 ` Josh Triplett
2016-09-15 21:35                   ` Nicolas Pitre
2016-09-15 21:56                     ` Josh Triplett
2016-09-16  7:24                       ` Richard Cochran
2016-09-17  2:57                         ` Nicolas Pitre
2016-09-18 14:35                           ` Richard Cochran
2016-09-18 16:54                             ` Nicolas Pitre
2016-09-18 18:20                               ` Richard Cochran
2016-09-18 18:49                                 ` Nicolas Pitre
2016-09-18 20:22                                   ` Richard Cochran
2016-09-18 20:30                                     ` Nicolas Pitre
2016-09-18 21:11                                       ` Richard Cochran
2016-09-15 21:23               ` Richard Cochran
2016-09-16 14:31 ` kbuild test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.