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

[Richard Cochran]

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 

> 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

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

[Richard Cochran]

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 

> 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

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

[Nicolas Pitre]

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

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

[Nicolas Pitre]

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

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

[Richard Cochran]

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

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

[Richard Cochran]

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

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

[Nicolas Pitre]

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

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

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

[Nicolas Pitre]

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

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 

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

[Richard Cochran]

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

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

[Richard Cochran]

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

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

[Nicolas Pitre]

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

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

[Nicolas Pitre]

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

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

[Richard Cochran]

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

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

[Richard Cochran]

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

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

[Nicolas Pitre]

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 

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 

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

[Nicolas Pitre]

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 

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 

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

[kbuild test robot]

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= (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 infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

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

[kbuild test robot]

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= (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 infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

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

[Richard Cochran]

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

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

[Richard Cochran]

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

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

[Josh Triplett]

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.

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

[Josh Triplett]

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.

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

[Nicolas Pitre]

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

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

[Nicolas Pitre]

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

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

[Richard Cochran]

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

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

[Richard Cochran]

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

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

[Josh Triplett]

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

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

[Josh Triplett]

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

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

[Richard Cochran]

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

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

[Richard Cochran]

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

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

[John Stultz]

On Thu, Sep 15, 2016 at 12:31 PM, Nicolas Pitre
 wrote:
> On Thu, 15 Sep 2016, John Stultz wrote:
>
>> On Thu, Sep 15, 2016 at 11:37 AM, Nicolas Pitre
>>  wrote:
>> > On Thu, 15 Sep 2016, John Stultz wrote:
>> >
>> >> On Thu, Sep 15, 2016 at 11:28 AM, Nicolas Pitre
>> >>  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(>clock, ptp->devid);
> -   if (err) {
> -   pr_err("failed to create posix clock\n");
> -   goto no_clock;
> +   if (IS_ENABLED(CONFIG_POSIX_TIMERS)) {
> +   err = 

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

[John Stultz]

On Thu, Sep 15, 2016 at 12:31 PM, Nicolas Pitre
 wrote:
> On Thu, 15 Sep 2016, John Stultz wrote:
>
>> On Thu, Sep 15, 2016 at 11:37 AM, Nicolas Pitre
>>  wrote:
>> > On Thu, 15 Sep 2016, John Stultz wrote:
>> >
>> >> On Thu, Sep 15, 2016 at 11:28 AM, Nicolas Pitre
>> >>  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(>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(>clock, ptp->devid);
> +   if (err) {
> +   

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

[Nicolas Pitre]

On Thu, 15 Sep 2016, John Stultz wrote:

> On Thu, Sep 15, 2016 at 11:37 AM, Nicolas Pitre
>  wrote:
> > On Thu, 15 Sep 2016, John Stultz wrote:
> >
> >> On Thu, Sep 15, 2016 at 11:28 AM, Nicolas Pitre
> >>  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(>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(>clock, ptp->devid);
+   if (err) {
+   pr_err("failed to create posix clock\n");
+   goto no_clock;
+   }
}
 
return ptp;
@@ -281,7 +284,8 @@ int 

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

[Nicolas Pitre]

On Thu, 15 Sep 2016, John Stultz wrote:

> On Thu, Sep 15, 2016 at 11:37 AM, Nicolas Pitre
>  wrote:
> > On Thu, 15 Sep 2016, John Stultz wrote:
> >
> >> On Thu, Sep 15, 2016 at 11:28 AM, Nicolas Pitre
> >>  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(>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(>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)

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

[John Stultz]

On Thu, Sep 15, 2016 at 11:37 AM, Nicolas Pitre
 wrote:
> On Thu, 15 Sep 2016, John Stultz wrote:
>
>> On Thu, Sep 15, 2016 at 11:28 AM, Nicolas Pitre
>>  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

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

[John Stultz]

On Thu, Sep 15, 2016 at 11:37 AM, Nicolas Pitre
 wrote:
> On Thu, 15 Sep 2016, John Stultz wrote:
>
>> On Thu, Sep 15, 2016 at 11:28 AM, Nicolas Pitre
>>  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

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

[Nicolas Pitre]

On Thu, 15 Sep 2016, John Stultz wrote:

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

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

[Nicolas Pitre]

On Thu, 15 Sep 2016, John Stultz wrote:

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

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

[John Stultz]

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

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

[John Stultz]

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

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

[Nicolas Pitre]

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

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

[Nicolas Pitre]

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

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

[John Stultz]

> 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

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

[John Stultz]

> 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

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

[Nicolas Pitre]

On Thu, 15 Sep 2016, John Stultz wrote:

> On Wed, Sep 14, 2016 at 8:47 PM, Nicolas Pitre  
> 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

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

[Nicolas Pitre]

On Thu, 15 Sep 2016, John Stultz wrote:

> On Wed, Sep 14, 2016 at 8:47 PM, Nicolas Pitre  
> 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

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

[John Stultz]

On Wed, Sep 14, 2016 at 8:47 PM, Nicolas Pitre  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

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

[John Stultz]

On Wed, Sep 14, 2016 at 8:47 PM, Nicolas Pitre  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