All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 1/1] ptp4l: new package
@ 2017-05-09  9:42 Petr Kulhavy
  2017-05-09 11:51 ` Thomas Petazzoni
  2017-05-09 15:10 ` Danomi Manchego
  0 siblings, 2 replies; 5+ messages in thread
From: Petr Kulhavy @ 2017-05-09  9:42 UTC (permalink / raw)
  To: buildroot

Add the Linux PTP Project package, aka ptp4l.
http://linuxptp.sourceforge.net/

The sysV and systemd init scripts start the daemon in automatic mode on eth0.

Signed-off-by: Petr Kulhavy <brain@jikos.cz>
---
 package/Config.in           |  1 +
 package/ptp4l/Config.in     | 13 +++++++++++++
 package/ptp4l/S65ptp4l      | 29 +++++++++++++++++++++++++++++
 package/ptp4l/ptp4l.hash    |  2 ++
 package/ptp4l/ptp4l.mk      | 37 +++++++++++++++++++++++++++++++++++++
 package/ptp4l/ptp4l.service | 10 ++++++++++
 6 files changed, 92 insertions(+)
 create mode 100644 package/ptp4l/Config.in
 create mode 100755 package/ptp4l/S65ptp4l
 create mode 100644 package/ptp4l/ptp4l.hash
 create mode 100644 package/ptp4l/ptp4l.mk
 create mode 100644 package/ptp4l/ptp4l.service

diff --git a/package/Config.in b/package/Config.in
index d57813c..a59da25 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -1660,6 +1660,7 @@ endif
 	source "package/proxychains-ng/Config.in"
 	source "package/ptpd/Config.in"
 	source "package/ptpd2/Config.in"
+	source "package/ptp4l/Config.in"
 	source "package/pure-ftpd/Config.in"
 	source "package/putty/Config.in"
 	source "package/quagga/Config.in"
diff --git a/package/ptp4l/Config.in b/package/ptp4l/Config.in
new file mode 100644
index 0000000..a070545
--- /dev/null
+++ b/package/ptp4l/Config.in
@@ -0,0 +1,13 @@
+config BR2_PACKAGE_PTP4L
+	bool "ptp4l / Linux PTP"
+	help
+	  The Linux PTP Project is the Precision Time Protocol implementation
+	  according to IEEE standard 1588 for Linux.
+
+	  The dual design goals are to provide a robust implementation of the
+	  standard and to use the most relevant and modern Application
+	  Programming Interfaces (API) offered by the Linux kernel. Supporting
+	  legacy APIs and other platforms is not a goal.
+
+	  http://linuxptp.sourceforge.net/
+
diff --git a/package/ptp4l/S65ptp4l b/package/ptp4l/S65ptp4l
new file mode 100755
index 0000000..6912658
--- /dev/null
+++ b/package/ptp4l/S65ptp4l
@@ -0,0 +1,29 @@
+#!/bin/sh
+#
+# Start ptp4l
+#
+
+case "$1" in
+  start)
+	printf "Starting ptp4l: "
+	start-stop-daemon -S -b -q -x /usr/sbin/ptp4l -- -A -i eth0
+	if [ $? != 0 ]; then
+		echo "FAILED"
+		exit 1
+	else
+		echo "OK"
+	fi
+	;;
+  stop)
+	printf "Stopping ptp4l: "
+	start-stop-daemon -K -q -x /usr/sbin/ptp4l
+	echo "OK"
+	;;
+  restart|reload)
+	;;
+  *)
+	echo "Usage: $0 {start|stop|restart}"
+	exit 1
+esac
+
+exit $?
diff --git a/package/ptp4l/ptp4l.hash b/package/ptp4l/ptp4l.hash
new file mode 100644
index 0000000..1ac9443
--- /dev/null
+++ b/package/ptp4l/ptp4l.hash
@@ -0,0 +1,2 @@
+# Locally computed:
+sha256	fa8e00f6ec73cefa7bb313dce7f60dfe5eb9e2bde3353594e9ac18edc93e5165  linuxptp-1.8.tgz
diff --git a/package/ptp4l/ptp4l.mk b/package/ptp4l/ptp4l.mk
new file mode 100644
index 0000000..49d083d
--- /dev/null
+++ b/package/ptp4l/ptp4l.mk
@@ -0,0 +1,37 @@
+################################################################################
+#
+# ptp4l
+#
+################################################################################
+
+PTP4L_VERSION = 1.8
+PTP4L_SOURCE = linuxptp-$(PTP4L_VERSION).tgz
+PTP4L_SITE = http://sourceforge.net/projects/linuxptp/files/v$(PTP4L_VERSION)
+PTP4L_LICENSE = GPLv2
+PTP4L_LICENSE_FILES = COPYING
+PTP4L_CFLAGS = $(TARGET_CFLAGS)
+
+
+define PTP4L_BUILD_CMDS
+	$(TARGET_MAKE_ENV) $(MAKE) KBUILD_OUTPUT=$(TARGET_DIR) CC=$(TARGET_CC) EXTRA_CFLAGS="$(PTP4L_CFLAGS)" -C $(@D) all
+endef
+
+define PTP4L_INSTALL_TARGET_CMDS
+	$(TARGET_MAKE_ENV) $(MAKE) prefix=$(TARGET_DIR)/usr $(TARGET_CONFIGURE_OPTS) -C $(@D) install
+endef
+
+define PTP4L_INSTALL_INIT_SYSV
+	$(INSTALL) -m 755 -D $(@D)/package/ptp4l/S65ptp4l \
+		$(TARGET_DIR)/etc/init.d/S65ptp4l
+endef
+
+define PTP4L_INSTALL_INIT_SYSTEMD
+	$(INSTALL) -D -m 644 $(PTP4L_PKGDIR)/ptp4l.service \
+		$(TARGET_DIR)/usr/lib/systemd/system/ptp4l.service
+	mkdir -p $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants
+	ln -sf ../../../../usr/lib/systemd/system/ptp4l.service \
+		$(TARGET_DIR)/etc/systemd/system/multi-user.target.wants/ptp4l.service
+endef
+
+$(eval $(generic-package))
+
diff --git a/package/ptp4l/ptp4l.service b/package/ptp4l/ptp4l.service
new file mode 100644
index 0000000..ec6bbc5
--- /dev/null
+++ b/package/ptp4l/ptp4l.service
@@ -0,0 +1,10 @@
+[Unit]
+Description=Precision Time Protocol daemon
+After=syslog.target network.target
+
+[Service]
+ExecStart=/usr/sbin/ptp4l -A -i eth0
+Restart=always
+
+[Install]
+WantedBy=multi-user.target
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [Buildroot] [PATCH 1/1] ptp4l: new package
  2017-05-09  9:42 [Buildroot] [PATCH 1/1] ptp4l: new package Petr Kulhavy
@ 2017-05-09 11:51 ` Thomas Petazzoni
  2017-05-09 12:31   ` Petr Kulhavy
  2017-05-09 15:10 ` Danomi Manchego
  1 sibling, 1 reply; 5+ messages in thread
From: Thomas Petazzoni @ 2017-05-09 11:51 UTC (permalink / raw)
  To: buildroot

Hello Petr,

Thanks for this contribution! Looks mostly good, just a few comments,
see below.

On Tue,  9 May 2017 11:42:24 +0200, Petr Kulhavy wrote:

>  package/Config.in           |  1 +
>  package/ptp4l/Config.in     | 13 +++++++++++++
>  package/ptp4l/S65ptp4l      | 29 +++++++++++++++++++++++++++++
>  package/ptp4l/ptp4l.hash    |  2 ++
>  package/ptp4l/ptp4l.mk      | 37 +++++++++++++++++++++++++++++++++++++
>  package/ptp4l/ptp4l.service | 10 ++++++++++

Could you add yourself to the DEVELOPERS file for this package?

> diff --git a/package/ptp4l/Config.in b/package/ptp4l/Config.in
> new file mode 100644
> index 0000000..a070545
> --- /dev/null
> +++ b/package/ptp4l/Config.in
> @@ -0,0 +1,13 @@
> +config BR2_PACKAGE_PTP4L
> +	bool "ptp4l / Linux PTP"

Just:

	bool "ptp4l"

> +	help
> +	  The Linux PTP Project is the Precision Time Protocol implementation
> +	  according to IEEE standard 1588 for Linux.
> +
> +	  The dual design goals are to provide a robust implementation of the
> +	  standard and to use the most relevant and modern Application
> +	  Programming Interfaces (API) offered by the Linux kernel. Supporting
> +	  legacy APIs and other platforms is not a goal.
> +
> +	  http://linuxptp.sourceforge.net/
> +

Unneeded empty new line.

> +case "$1" in
> +  start)
> +	printf "Starting ptp4l: "
> +	start-stop-daemon -S -b -q -x /usr/sbin/ptp4l -- -A -i eth0
> +	if [ $? != 0 ]; then
> +		echo "FAILED"
> +		exit 1
> +	else
> +		echo "OK"
> +	fi

Use:

	[ $? = 0 ] && echo "OK" || echo "FAIL"

Also, please specify a PID file using:

	-p /var/run/ptp4l.pid

> +  stop)
> +	printf "Stopping ptp4l: "
> +	start-stop-daemon -K -q -x /usr/sbin/ptp4l

And use the PID file here as well.

> +  restart|reload)
> +	;;

Nothing?

> +PTP4L_VERSION = 1.8
> +PTP4L_SOURCE = linuxptp-$(PTP4L_VERSION).tgz
> +PTP4L_SITE = http://sourceforge.net/projects/linuxptp/files/v$(PTP4L_VERSION)
> +PTP4L_LICENSE = GPLv2

You should use SPDX license code, so instead of GPLv2 it should be
GPL-2.0.

However, after checking the actual tarball, it seems like the license
is GPL-2.0+ (i.e GPLv2 or later).

> +PTP4L_LICENSE_FILES = COPYING
> +PTP4L_CFLAGS = $(TARGET_CFLAGS)

Why is this variable useful?

> +
> +

One too many empty new line.

> +define PTP4L_BUILD_CMDS
> +	$(TARGET_MAKE_ENV) $(MAKE) KBUILD_OUTPUT=$(TARGET_DIR) CC=$(TARGET_CC) EXTRA_CFLAGS="$(PTP4L_CFLAGS)" -C $(@D) all

Just use TARGET_CFLAGS instead of PTP4L_CFLAGS.

Also, split this line that is a bit too long.

What is KBUILD_OUTPUT used for ?

> +endef
> +
> +define PTP4L_INSTALL_TARGET_CMDS
> +	$(TARGET_MAKE_ENV) $(MAKE) prefix=$(TARGET_DIR)/usr $(TARGET_CONFIGURE_OPTS) -C $(@D) install

Please use instead:

	prefix=/usr DESTDIR=$(TARGET_DIR)

Why are you passing TARGET_CONFIGURE_OPTS at install time and not build
time ?

> +endef
> +
> +define PTP4L_INSTALL_INIT_SYSV
> +	$(INSTALL) -m 755 -D $(@D)/package/ptp4l/S65ptp4l \
> +		$(TARGET_DIR)/etc/init.d/S65ptp4l
> +endef
> +
> +define PTP4L_INSTALL_INIT_SYSTEMD
> +	$(INSTALL) -D -m 644 $(PTP4L_PKGDIR)/ptp4l.service \
> +		$(TARGET_DIR)/usr/lib/systemd/system/ptp4l.service
> +	mkdir -p $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants
> +	ln -sf ../../../../usr/lib/systemd/system/ptp4l.service \
> +		$(TARGET_DIR)/etc/systemd/system/multi-user.target.wants/ptp4l.service
> +endef
> +
> +$(eval $(generic-package))
> +

Useless empty new line.

Final questions/comments:

 - Why is your package named ptp4l and not linuxptp like the upstream
   tarball/project ?

 - Make sure to run support/scripts/check-package (to verify that there
   are no coding style issues in your package) and
   support/scripts/test-pkg (to make sure your package builds fine with
   all toolchains).

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [Buildroot] [PATCH 1/1] ptp4l: new package
  2017-05-09 11:51 ` Thomas Petazzoni
@ 2017-05-09 12:31   ` Petr Kulhavy
  0 siblings, 0 replies; 5+ messages in thread
From: Petr Kulhavy @ 2017-05-09 12:31 UTC (permalink / raw)
  To: buildroot

Hi Thomas,

thank you for your review, I will do the corrections as per your 
suggestions and submit a new version of the patch.

Regarding the name you are correctly pointing out that the official web 
is called "Linux PTP Package".
However there are several other PTP implementations for Linux (ptpd, 
ptpd2) and in this context I find the name a bit too generic.
So I named the package after the "ptp4l" daemon, which is also the name 
used by few others to refer this package (e.g. Redhat or some 
conferences about PTP).
I'm however a bit undecided which name is better...


On 09/05/17 13:51, Thomas Petazzoni wrote:
> On Tue,  9 May 2017 11:42:24 +0200, Petr Kulhavy wrote:
>
>> +PTP4L_LICENSE_FILES = COPYING
>> +PTP4L_CFLAGS = $(TARGET_CFLAGS)
> Why is this variable useful?
>
> What is KBUILD_OUTPUT used for ?
>
>> +endef
>> +
>> +define PTP4L_INSTALL_TARGET_CMDS
>> +	$(TARGET_MAKE_ENV) $(MAKE) prefix=$(TARGET_DIR)/usr $(TARGET_CONFIGURE_OPTS) -C $(@D) install
> Please use instead:
>
> 	prefix=/usr DESTDIR=$(TARGET_DIR)
>
> Why are you passing TARGET_CONFIGURE_OPTS at install time and not build
> time ?
This is due to the unfortunate way the package's makefile is build:
The makefile calls the script incdefs.sh to detect some features and set 
the corresponding -Dxxxx flags in CFLAGS .
Unless KBUILD_OUTPUT is set the script searches in /usr/include, i.e. on 
host machine.

The makefile constructs its own CFLAGS based on the above and the 
environment CFLAGS are expected to be in EXTRA_CFLAGS.
So if CFLAGS are set on the make's command-line make doesn't call the 
incdefs.sh script and the detection and possibly the compilation fails.

That's why I couldn't use the TARGET_CONFIGURE_OPTS for the build phase.
However I left it as standard for the install phase.

I'm not sure if what I did is 100% clean, but it seems to work :-)

Petr

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [Buildroot] [PATCH 1/1] ptp4l: new package
  2017-05-09  9:42 [Buildroot] [PATCH 1/1] ptp4l: new package Petr Kulhavy
  2017-05-09 11:51 ` Thomas Petazzoni
@ 2017-05-09 15:10 ` Danomi Manchego
  2017-05-09 15:26   ` Petr Kulhavy
  1 sibling, 1 reply; 5+ messages in thread
From: Danomi Manchego @ 2017-05-09 15:10 UTC (permalink / raw)
  To: buildroot

Petr,

On Tue, May 9, 2017 at 5:42 AM, Petr Kulhavy <brain@jikos.cz> wrote:
<snip>
> +PTP4L_CFLAGS = $(TARGET_CFLAGS)
> +
> +
> +define PTP4L_BUILD_CMDS
> +       $(TARGET_MAKE_ENV) $(MAKE) KBUILD_OUTPUT=$(TARGET_DIR) CC=$(TARGET_CC) EXTRA_CFLAGS="$(PTP4L_CFLAGS)" -C $(@D) all
> +endef

Perhaps you should add EXTRA_LDFLAGS="$(TARGET_LDFLAGS)" as well.
(In addition to eliminating PTP4L_CFLAGS and using
EXTRA_CFLAGS="$(TARGET_CFLAGS)" instead,)

FWIW: changing the name to linuxptp: +1

Danomi -


> +
> +define PTP4L_INSTALL_TARGET_CMDS
> +       $(TARGET_MAKE_ENV) $(MAKE) prefix=$(TARGET_DIR)/usr $(TARGET_CONFIGURE_OPTS) -C $(@D) install
> +endef
> +
> +define PTP4L_INSTALL_INIT_SYSV
> +       $(INSTALL) -m 755 -D $(@D)/package/ptp4l/S65ptp4l \
> +               $(TARGET_DIR)/etc/init.d/S65ptp4l
> +endef
> +
> +define PTP4L_INSTALL_INIT_SYSTEMD
> +       $(INSTALL) -D -m 644 $(PTP4L_PKGDIR)/ptp4l.service \
> +               $(TARGET_DIR)/usr/lib/systemd/system/ptp4l.service
> +       mkdir -p $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants
> +       ln -sf ../../../../usr/lib/systemd/system/ptp4l.service \
> +               $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants/ptp4l.service
> +endef
> +
> +$(eval $(generic-package))
> +
> diff --git a/package/ptp4l/ptp4l.service b/package/ptp4l/ptp4l.service
> new file mode 100644
> index 0000000..ec6bbc5
> --- /dev/null
> +++ b/package/ptp4l/ptp4l.service
> @@ -0,0 +1,10 @@
> +[Unit]
> +Description=Precision Time Protocol daemon
> +After=syslog.target network.target
> +
> +[Service]
> +ExecStart=/usr/sbin/ptp4l -A -i eth0
> +Restart=always
> +
> +[Install]
> +WantedBy=multi-user.target
> --
> 2.7.4
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [Buildroot] [PATCH 1/1] ptp4l: new package
  2017-05-09 15:10 ` Danomi Manchego
@ 2017-05-09 15:26   ` Petr Kulhavy
  0 siblings, 0 replies; 5+ messages in thread
From: Petr Kulhavy @ 2017-05-09 15:26 UTC (permalink / raw)
  To: buildroot

Thanks, Danomi!

Petr

On 09/05/17 17:10, Danomi Manchego wrote:
> Petr,
>
> On Tue, May 9, 2017 at 5:42 AM, Petr Kulhavy <brain@jikos.cz> wrote:
> <snip>
>> +PTP4L_CFLAGS = $(TARGET_CFLAGS)
>> +
>> +
>> +define PTP4L_BUILD_CMDS
>> +       $(TARGET_MAKE_ENV) $(MAKE) KBUILD_OUTPUT=$(TARGET_DIR) CC=$(TARGET_CC) EXTRA_CFLAGS="$(PTP4L_CFLAGS)" -C $(@D) all
>> +endef
> Perhaps you should add EXTRA_LDFLAGS="$(TARGET_LDFLAGS)" as well.
> (In addition to eliminating PTP4L_CFLAGS and using
> EXTRA_CFLAGS="$(TARGET_CFLAGS)" instead,)
>
> FWIW: changing the name to linuxptp: +1
>
> Danomi -
>
>
>> +
>> +define PTP4L_INSTALL_TARGET_CMDS
>> +       $(TARGET_MAKE_ENV) $(MAKE) prefix=$(TARGET_DIR)/usr $(TARGET_CONFIGURE_OPTS) -C $(@D) install
>> +endef
>> +
>> +define PTP4L_INSTALL_INIT_SYSV
>> +       $(INSTALL) -m 755 -D $(@D)/package/ptp4l/S65ptp4l \
>> +               $(TARGET_DIR)/etc/init.d/S65ptp4l
>> +endef
>> +
>> +define PTP4L_INSTALL_INIT_SYSTEMD
>> +       $(INSTALL) -D -m 644 $(PTP4L_PKGDIR)/ptp4l.service \
>> +               $(TARGET_DIR)/usr/lib/systemd/system/ptp4l.service
>> +       mkdir -p $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants
>> +       ln -sf ../../../../usr/lib/systemd/system/ptp4l.service \
>> +               $(TARGET_DIR)/etc/systemd/system/multi-user.target.wants/ptp4l.service
>> +endef
>> +
>> +$(eval $(generic-package))
>> +
>> diff --git a/package/ptp4l/ptp4l.service b/package/ptp4l/ptp4l.service
>> new file mode 100644
>> index 0000000..ec6bbc5
>> --- /dev/null
>> +++ b/package/ptp4l/ptp4l.service
>> @@ -0,0 +1,10 @@
>> +[Unit]
>> +Description=Precision Time Protocol daemon
>> +After=syslog.target network.target
>> +
>> +[Service]
>> +ExecStart=/usr/sbin/ptp4l -A -i eth0
>> +Restart=always
>> +
>> +[Install]
>> +WantedBy=multi-user.target
>> --
>> 2.7.4
>>
>> _______________________________________________
>> buildroot mailing list
>> buildroot at busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-05-09 15:26 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-09  9:42 [Buildroot] [PATCH 1/1] ptp4l: new package Petr Kulhavy
2017-05-09 11:51 ` Thomas Petazzoni
2017-05-09 12:31   ` Petr Kulhavy
2017-05-09 15:10 ` Danomi Manchego
2017-05-09 15:26   ` Petr Kulhavy

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