Re: [PATCH net-next v4 1/2] net: add support for Cavium PTP coprocessor

2017-12-08 Thread Philippe Ombredanne 
Dear Aleksey, Dear Radoslaw,

On Fri, Dec 8, 2017 at 11:34 AM, Aleksey Makarov
 wrote:
> From: Radoslaw Biernacki 
>
> This patch adds support for the Precision Time Protocol
> Clocks and Timestamping hardware found on Cavium ThunderX
> processors.
>
> Signed-off-by: Radoslaw Biernacki 
> Signed-off-by: Aleksey Makarov 
[]
> --- /dev/null
> +++ b/drivers/net/ethernet/cavium/common/cavium_ptp.c
> @@ -0,0 +1,334 @@
> +/*
> + * cavium_ptp.c - PTP 1588 clock on Cavium hardware
> + *
> + * Copyright (c) 2003-2015, 2017 Cavium, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License.


Have you considered using the new SPDX ids instead of this fine legalese? e.g.:

// SPDX-License-Identifier: GPL-2.0

This is much shorter and neater (unless you are a legalese lover of
course!)... Check also Thomas doc patches and Linus comments on why he
prefers the C++ comment style for these.

And even better, what about this more compact form? I am a big fan of the
C++ style comments for these (and so is Linus):

// SPDX-License-Identifier: GPL-2.0
// Copyright (c) 2003-2015, 2017 Cavium, Inc.
// cavium_ptp.c - PTP 1588 clock on Cavium hardware


> + *
> + * This file may also be available under a different license from Cavium.
> + * Contact Cavium, Inc. for more information
> + */

I am not so sure that the kernel source tree is the right place for
commercial advertisement... I mean, this is a fine statement to put on
your company web site, but who reads this code: you and I do we
care seriously about this? Anyone who uses your hardware would likely
have some other kind of arrangements with your company anyway, making
this sentence moot in all cases. Therefore I tend to think this is
just a noisy distraction from your otherwise fine code contributions:
It does not come out to me as kernically-correct ;)

So can you consider removing this fine marketing statement from this
and other Cavium past, present and future contributions? That would be
much appreciated! (and while you are at it, using SPDX ids throughout
would give you good karma extra points)

PS: Now, if this "different license" of yours is a fine BSD or MIT,
you could use instead this SPDX shorthand to the same effect without
turning the kernel code into a billboard:

// SPDX-License-Identifier: (GPL-2.0 OR MIT)

-- 
Cordially
Philippe Ombredanne, your ad-sensitive licensing scruffy

[PATCH net-next v4 1/2] net: add support for Cavium PTP coprocessor

2017-12-08 Thread Aleksey Makarov 
From: Radoslaw Biernacki 

This patch adds support for the Precision Time Protocol
Clocks and Timestamping hardware found on Cavium ThunderX
processors.

Signed-off-by: Radoslaw Biernacki 
Signed-off-by: Aleksey Makarov 
---
 drivers/net/ethernet/cavium/Kconfig |  12 +
 drivers/net/ethernet/cavium/Makefile|   1 +
 drivers/net/ethernet/cavium/common/Makefile |   1 +
 drivers/net/ethernet/cavium/common/cavium_ptp.c | 334 
 drivers/net/ethernet/cavium/common/cavium_ptp.h |  78 ++
 5 files changed, 426 insertions(+)
 create mode 100644 drivers/net/ethernet/cavium/common/Makefile
 create mode 100644 drivers/net/ethernet/cavium/common/cavium_ptp.c
 create mode 100644 drivers/net/ethernet/cavium/common/cavium_ptp.h

diff --git a/drivers/net/ethernet/cavium/Kconfig 
b/drivers/net/ethernet/cavium/Kconfig
index 63be75eb34d2..2380e9834007 100644
--- a/drivers/net/ethernet/cavium/Kconfig
+++ b/drivers/net/ethernet/cavium/Kconfig
@@ -50,6 +50,18 @@ config   THUNDER_NIC_RGX
  This driver supports configuring XCV block of RGX interface
  present on CN81XX chip.
 
+config CAVIUM_PTP
+   tristate "Cavium PTP coprocessor as PTP clock"
+   depends on 64BIT
+   select PTP_1588_CLOCK
+   default y
+   ---help---
+ This driver adds support for the Precision Time Protocol Clocks and
+ Timestamping coprocessor (PTP) found on Cavium processors.
+ PTP provides timestamping mechanism that is suitable for use in IEEE 
1588
+ Precision Time Protocol or other purposes.  Timestamps can be used in
+ BGX, TNS, GTI, and NIC blocks.
+
 config LIQUIDIO
tristate "Cavium LiquidIO support"
depends on 64BIT
diff --git a/drivers/net/ethernet/cavium/Makefile 
b/drivers/net/ethernet/cavium/Makefile
index 872da9f7c31a..946bba84e81d 100644
--- a/drivers/net/ethernet/cavium/Makefile
+++ b/drivers/net/ethernet/cavium/Makefile
@@ -1,6 +1,7 @@
 #
 # Makefile for the Cavium ethernet device drivers.
 #
+obj-$(CONFIG_NET_VENDOR_CAVIUM) += common/
 obj-$(CONFIG_NET_VENDOR_CAVIUM) += thunder/
 obj-$(CONFIG_NET_VENDOR_CAVIUM) += liquidio/
 obj-$(CONFIG_NET_VENDOR_CAVIUM) += octeon/
diff --git a/drivers/net/ethernet/cavium/common/Makefile 
b/drivers/net/ethernet/cavium/common/Makefile
new file mode 100644
index ..dd8561b8060b
--- /dev/null
+++ b/drivers/net/ethernet/cavium/common/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_CAVIUM_PTP) += cavium_ptp.o
diff --git a/drivers/net/ethernet/cavium/common/cavium_ptp.c 
b/drivers/net/ethernet/cavium/common/cavium_ptp.c
new file mode 100644
index ..f4c738db27fd
--- /dev/null
+++ b/drivers/net/ethernet/cavium/common/cavium_ptp.c
@@ -0,0 +1,334 @@
+/*
+ * cavium_ptp.c - PTP 1588 clock on Cavium hardware
+ *
+ * Copyright (c) 2003-2015, 2017 Cavium, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License.
+ *
+ * This file may also be available under a different license from Cavium.
+ * Contact Cavium, Inc. for more information
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include "cavium_ptp.h"
+
+#define DRV_NAME   "Cavium PTP Driver"
+
+#define PCI_DEVICE_ID_CAVIUM_PTP   0xA00C
+#define PCI_DEVICE_ID_CAVIUM_RST   0xA00E
+
+#define PCI_PTP_BAR_NO 0
+#define PCI_RST_BAR_NO 0
+
+#define PTP_CLOCK_CFG  0xF00ULL
+#define  PTP_CLOCK_CFG_PTP_EN  BIT(0)
+#define PTP_CLOCK_LO   0xF08ULL
+#define PTP_CLOCK_HI   0xF10ULL
+#define PTP_CLOCK_COMP 0xF18ULL
+
+#define RST_BOOT   0x1600ULL
+#define CLOCK_BASE_RATE5000ULL
+
+static u64 ptp_cavium_clock_get(void)
+{
+   struct pci_dev *pdev;
+   void __iomem *base;
+   u64 ret = CLOCK_BASE_RATE * 16;
+
+   pdev = pci_get_device(PCI_VENDOR_ID_CAVIUM,
+ PCI_DEVICE_ID_CAVIUM_RST, NULL);
+   if (!pdev)
+   goto error;
+
+   base = pci_ioremap_bar(pdev, PCI_RST_BAR_NO);
+   if (!base)
+   goto error_put_pdev;
+
+   ret = CLOCK_BASE_RATE * ((readq(base + RST_BOOT) >> 33) & 0x3f);
+
+   iounmap(base);
+
+error_put_pdev:
+   pci_dev_put(pdev);
+
+error:
+   return ret;
+}
+
+struct cavium_ptp *cavium_ptp_get(void)
+{
+   struct cavium_ptp *ptp;
+   struct pci_dev *pdev;
+
+   pdev = pci_get_device(PCI_VENDOR_ID_CAVIUM,
+ PCI_DEVICE_ID_CAVIUM_PTP, NULL);
+   if (!pdev)
+   return ERR_PTR(-ENODEV);
+
+   ptp = pci_get_drvdata(pdev);
+   if (!ptp) {
+   pci_dev_put(pdev);
+   ptp = ERR_PTR(-EPROBE_DEFER);
+   }
+
+   return ptp;
+}
+EXPORT_SYMBOL(cavium_ptp_get);
+
+void cavium_ptp_put(struct cavium_ptp *ptp)
+{
+   pci_dev_put(ptp->pdev);
+}