diff mbox

[1/2] ptp_clock: allow for it to be optional

Message ID alpine.LFD.2.20.1609190942230.9311@knanqh.ubzr
State Superseded, archived
Delegated to: David Miller
Headers show

Commit Message

Nicolas Pitre Sept. 19, 2016, 2:10 p.m. UTC 
On Mon, 19 Sep 2016, Jiri Benc wrote:

> On Sun, 18 Sep 2016 23:51:09 -0400, Nicolas Pitre wrote:
> > 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.
> 
> With this patch applied, the user is free to set a NIC driver as built
> in and PTP_1588_CLOCK as a module, right? If so, that would lead to
> non-functional PTP without any warning due to the use of IS_REACHABLE.
> That doesn't sound right. Could easily cause hours of headache to
> someone.
> 
> Or is this handled somehow?

I don't see how to remove the ability to select m for PTP_1588_CLOCK 
based on (PTP_1588_CLOCK_SELECTED = y).

What about this on top then:

Comments

Jiri Benc Sept. 19, 2016, 5:04 p.m. UTC | #1 
On Mon, 19 Sep 2016 10:10:21 -0400 (EDT), Nicolas Pitre wrote:
> --- a/include/linux/ptp_clock_kernel.h
> +++ b/include/linux/ptp_clock_kernel.h
> @@ -207,7 +207,16 @@ int ptp_find_pin(struct ptp_clock *ptp,
>  #else
>  static inline struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
>  						   struct device *parent)
> -{ return NULL; }
> +{
> +	if (IS_MODULE(CONFIG_PTP_1588_CLOCK)) {
> +		pr_warn("%s is built-in while PTP clock subsystem is modular, "
> +			"PTP clock ignored\n", KBUILD_MODNAME);
> +	} else {
> +		pr_warn("ignoring PTP clock from %s as PTP clock subsystem "
> +			"is configured out\n", KBUILD_MODNAME);
> +	}
> +	return NULL;
> +}

I think the else part is not needed. If PTP is disabled, it is
disabled, nobody should be surprised by that. Looks good otherwise.

Thanks,

 Jiri
Josh Triplett Sept. 19, 2016, 5:31 p.m. UTC | #2 
On Mon, Sep 19, 2016 at 07:04:15PM +0200, Jiri Benc wrote:
> On Mon, 19 Sep 2016 10:10:21 -0400 (EDT), Nicolas Pitre wrote:
> > --- a/include/linux/ptp_clock_kernel.h
> > +++ b/include/linux/ptp_clock_kernel.h
> > @@ -207,7 +207,16 @@ int ptp_find_pin(struct ptp_clock *ptp,
> >  #else
> >  static inline struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
> >  						   struct device *parent)
> > -{ return NULL; }
> > +{
> > +	if (IS_MODULE(CONFIG_PTP_1588_CLOCK)) {
> > +		pr_warn("%s is built-in while PTP clock subsystem is modular, "
> > +			"PTP clock ignored\n", KBUILD_MODNAME);
> > +	} else {
> > +		pr_warn("ignoring PTP clock from %s as PTP clock subsystem "
> > +			"is configured out\n", KBUILD_MODNAME);
> > +	}
> > +	return NULL;
> > +}
> 
> I think the else part is not needed. If PTP is disabled, it is
> disabled, nobody should be surprised by that. Looks good otherwise.

This works, and it should compile away to nothing in the normal case.
But it does seem unfortunate that this can't happen at build time via
Kconfig.  CCing linux-kbuild in case someone has an idea for how to fix
this.
Nicolas Pitre Sept. 19, 2016, 6:09 p.m. UTC | #3 
On Mon, 19 Sep 2016, Josh Triplett wrote:

> But it does seem unfortunate that this can't happen at build time via
> Kconfig.  CCing linux-kbuild in case someone has an idea for how to fix
> this.

I hoped something like this could work:

config FOO
	prompt "Blah-blah"
	tristate if (BAR != y)
	bool if (BAR = y)
	default BAR

This way FOO could be y, m or n when BAR is m or n, otherwise FOO could 
be y or n only.

But that didn't work.


Nicolas
diff mbox

Patch

diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index 4c29eb8e53..74079b2fcf 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -207,7 +207,16 @@  int ptp_find_pin(struct ptp_clock *ptp,
 #else
 static inline struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
 						   struct device *parent)
-{ return NULL; }
+{
+	if (IS_MODULE(CONFIG_PTP_1588_CLOCK)) {
+		pr_warn("%s is built-in while PTP clock subsystem is modular, "
+			"PTP clock ignored\n", KBUILD_MODNAME);
+	} else {
+		pr_warn("ignoring PTP clock from %s as PTP clock subsystem "
+			"is configured out\n", KBUILD_MODNAME);
+	}
+	return NULL;
+}
 static inline int ptp_clock_unregister(struct ptp_clock *ptp)
 { return 0; }
 static inline void ptp_clock_event(struct ptp_clock *ptp,