[PATCH 0/4] net: phy: adin: implement support for 1588 start-of-packet indication

[PATCH 0/4] net: phy: adin: implement support for 1588 start-of-packet indication

[Alexandru Ardelean]

The ADIN1300 & ADIN1200 PHYs support detection of IEEE 1588 time stamp
packets. This mechanism can be used to signal the MAC via a pulse-signal
when the PHY detects such a packet.
This reduces the time when the MAC can check these packets and can improve
the accuracy of the PTP algorithm.

The PHYs support configurable delays for when the signaling happens to the
MAC. These delays would typically get adjusted using a userspace phytool to
identify the best value for the setup. That values can then be added in the
system configuration via device-tree or ACPI and read as an array of 3
elements.

For the RX delays, the units are in MII clock cycles, while for TX delays
the units are in 8 nano-second intervals.

The indication of either RX or TX must use one of 4 pins from the device:
LED_0, LINK_ST, GP_CLK and INT_N.

The driver will make sure that TX SOP & RX SOP will not use the same pin.

Alexandru Ardelean (4):
  net: phy: adin: const-ify static data
  net: phy: adin: rename struct adin_hw_stat -> adin_map
  net: phy: adin: implement support for 1588 start-of-packet indication
  dt-bindings: net: adin: document 1588 TX/RX SOP bindings

 .../devicetree/bindings/net/adi,adin.yaml     |  60 +++++
 drivers/net/phy/adin.c                        | 227 +++++++++++++++++-
 2 files changed, 274 insertions(+), 13 deletions(-)

-- 
2.20.1

[PATCH 1/4] net: phy: adin: const-ify static data

[Alexandru Ardelean]

Some bits of static data should have been made const from the start.
This change adds the const qualifier where appropriate.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/net/phy/adin.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index cf5a391c93e6..1dca3e883df4 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -145,7 +145,7 @@ struct adin_clause45_mmd_map {
 	u16 adin_regnum;
 };
 
-static struct adin_clause45_mmd_map adin_clause45_mmd_map[] = {
+static const struct adin_clause45_mmd_map adin_clause45_mmd_map[] = {
 	{ MDIO_MMD_PCS,	MDIO_PCS_EEE_ABLE,	ADIN1300_EEE_CAP_REG },
 	{ MDIO_MMD_AN,	MDIO_AN_EEE_LPABLE,	ADIN1300_EEE_LPABLE_REG },
 	{ MDIO_MMD_AN,	MDIO_AN_EEE_ADV,	ADIN1300_EEE_ADV_REG },
@@ -159,7 +159,7 @@ struct adin_hw_stat {
 	u16 reg2;
 };
 
-static struct adin_hw_stat adin_hw_stats[] = {
+static const struct adin_hw_stat adin_hw_stats[] = {
 	{ "total_frames_checked_count",		0x940A, 0x940B }, /* hi + lo */
 	{ "length_error_frames_count",		0x940C },
 	{ "alignment_error_frames_count",	0x940D },
@@ -456,7 +456,7 @@ static int adin_phy_config_intr(struct phy_device *phydev)
 static int adin_cl45_to_adin_reg(struct phy_device *phydev, int devad,
 				 u16 cl45_regnum)
 {
-	struct adin_clause45_mmd_map *m;
+	const struct adin_clause45_mmd_map *m;
 	int i;
 
 	if (devad == MDIO_MMD_VEND1)
@@ -650,7 +650,7 @@ static void adin_get_strings(struct phy_device *phydev, u8 *data)
 }
 
 static int adin_read_mmd_stat_regs(struct phy_device *phydev,
-				   struct adin_hw_stat *stat,
+				   const struct adin_hw_stat *stat,
 				   u32 *val)
 {
 	int ret;
@@ -676,7 +676,7 @@ static int adin_read_mmd_stat_regs(struct phy_device *phydev,
 
 static u64 adin_get_stat(struct phy_device *phydev, int i)
 {
-	struct adin_hw_stat *stat = &adin_hw_stats[i];
+	const struct adin_hw_stat *stat = &adin_hw_stats[i];
 	struct adin_priv *priv = phydev->priv;
 	u32 val;
 	int ret;
-- 
2.20.1

[PATCH 2/4] net: phy: adin: rename struct adin_hw_stat -> adin_map

[Alexandru Ardelean]

The structure format will be re-used in an upcoming change. This change
renames to have a more generic name.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/net/phy/adin.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index 1dca3e883df4..11ffeaa665a1 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -153,13 +153,13 @@ static const struct adin_clause45_mmd_map adin_clause45_mmd_map[] = {
 	{ MDIO_MMD_PCS, MDIO_PCS_EEE_WK_ERR,	ADIN1300_LPI_WAKE_ERR_CNT_REG },
 };
 
-struct adin_hw_stat {
+struct adin_map {
 	const char *string;
-	u16 reg1;
-	u16 reg2;
+	u16 val1;
+	u16 val2;
 };
 
-static const struct adin_hw_stat adin_hw_stats[] = {
+static const struct adin_map adin_hw_stats[] = {
 	{ "total_frames_checked_count",		0x940A, 0x940B }, /* hi + lo */
 	{ "length_error_frames_count",		0x940C },
 	{ "alignment_error_frames_count",	0x940D },
@@ -650,21 +650,21 @@ static void adin_get_strings(struct phy_device *phydev, u8 *data)
 }
 
 static int adin_read_mmd_stat_regs(struct phy_device *phydev,
-				   const struct adin_hw_stat *stat,
+				   const struct adin_map *stat,
 				   u32 *val)
 {
 	int ret;
 
-	ret = phy_read_mmd(phydev, MDIO_MMD_VEND1, stat->reg1);
+	ret = phy_read_mmd(phydev, MDIO_MMD_VEND1, stat->val1);
 	if (ret < 0)
 		return ret;
 
 	*val = (ret & 0xffff);
 
-	if (stat->reg2 == 0)
+	if (stat->val2 == 0)
 		return 0;
 
-	ret = phy_read_mmd(phydev, MDIO_MMD_VEND1, stat->reg2);
+	ret = phy_read_mmd(phydev, MDIO_MMD_VEND1, stat->val2);
 	if (ret < 0)
 		return ret;
 
@@ -676,17 +676,17 @@ static int adin_read_mmd_stat_regs(struct phy_device *phydev,
 
 static u64 adin_get_stat(struct phy_device *phydev, int i)
 {
-	const struct adin_hw_stat *stat = &adin_hw_stats[i];
+	const struct adin_map *stat = &adin_hw_stats[i];
 	struct adin_priv *priv = phydev->priv;
 	u32 val;
 	int ret;
 
-	if (stat->reg1 > 0x1f) {
+	if (stat->val1 > 0x1f) {
 		ret = adin_read_mmd_stat_regs(phydev, stat, &val);
 		if (ret < 0)
 			return (u64)(~0);
 	} else {
-		ret = phy_read(phydev, stat->reg1);
+		ret = phy_read(phydev, stat->val1);
 		if (ret < 0)
 			return (u64)(~0);
 		val = (ret & 0xffff);
-- 
2.20.1

[PATCH 3/4] net: phy: adin: implement support for 1588 start-of-packet indication

[Alexandru Ardelean]

The ADIN1300 & ADIN1200 PHYs support detection of IEEE 1588 time stamp
packets. This mechanism can be used to signal the MAC via a pulse-signal
when the PHY detects such a packet.
This reduces the time when the MAC can check these packets and can improve
the accuracy of the PTP algorithm.

The PHYs support configurable delays for when the signaling happens to the
MAC. These delays would typically get adjusted using a userspace phytool to
identify the best value for the setup. That values can then be added in the
system configuration via device-tree or ACPI and read as an array of 3
elements.

For the RX delays, the units are in MII clock cycles, while for TX delays
the units are in 8 nano-second intervals.

The indication of either RX or TX must use one of 4 pins from the device:
LED_0, LINK_ST, GP_CLK and INT_N.

The driver will make sure that TX SOP & RX SOP will not use the same pin.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/net/phy/adin.c | 201 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 201 insertions(+)

diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index 11ffeaa665a1..4247fe5d813a 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -69,6 +69,42 @@
 #define ADIN1300_CLOCK_STOP_REG			0x9400
 #define ADIN1300_LPI_WAKE_ERR_CNT_REG		0xa000
 
+#define ADIN1300_SOP_CTRL			0x9428
+#define   ADIN1300_SOP_N_8_CYCM_1		GENMASK(6, 4)
+#define   ADIN1300_SOP_NCYC_EN			BIT(3)
+#define   ADIN1300_SOP_SFD_EN			BIT(2)
+#define   ADIN1300_SOP_RX_EN			BIT(1)
+#define   ADIN1300_SOP_TX_EN			BIT(0)
+
+enum {
+	SOP_DELAY_10,
+	SOP_DELAY_100,
+	SOP_DELAY_1000,
+	__SOP_DELAY_MAX,
+};
+
+#define ADIN1300_SOP_RX_DELAY			0x9429
+#define   ADIN1300_SOP_RX_10_DEL_NCYC_MSK	GENMASK(15, 11)
+#define   ADIN1300_SOP_RX_10_DEL_NCYC_SEL(x)	\
+		FIELD_PREP(ADIN1300_SOP_RX_10_DEL_NCYC_MSK, x)
+#define   ADIN1300_SOP_RX_100_DEL_NCYC_MSK	GENMASK(10, 6)
+#define   ADIN1300_SOP_RX_100_DEL_NCYC_SEL(x)	\
+		FIELD_PREP(ADIN1300_SOP_RX_100_DEL_NCYC_MSK, x)
+#define   ADIN1300_SOP_RX_1000_DEL_NCYC_MSK	GENMASK(5, 0)
+#define   ADIN1300_SOP_RX_1000_DEL_NCYC_SEL(x)	\
+		FIELD_PREP(ADIN1300_SOP_RX_1000_DEL_NCYC_MSK, x)
+
+#define ADIN1300_SOP_TX_DELAY			0x942a
+#define   ADIN1300_SOP_TX_10_DEL_N_8_NS_MSK	GENMASK(12, 8)
+#define   ADIN1300_SOP_TX_10_DEL_N_8_NS_SEL(x)	\
+		FIELD_PREP(ADIN1300_SOP_TX_10_DEL_N_8_NS_MSK, x)
+#define   ADIN1300_SOP_TX_100_DEL_N_8_NS_MSK	GENMASK(7, 4)
+#define   ADIN1300_SOP_TX_100_DEL_N_8_NS_SEL(x)	\
+		FIELD_PREP(ADIN1300_SOP_TX_100_DEL_N_8_NS_MSK, x)
+#define   ADIN1300_SOP_TX_1000_DEL_N_8_NS_MSK	GENMASK(3, 0)
+#define   ADIN1300_SOP_TX_1000_DEL_N_8_NS_SEL(x)\
+		FIELD_PREP(ADIN1300_SOP_TX_1000_DEL_N_8_NS_MSK, x)
+
 #define ADIN1300_GE_SOFT_RESET_REG		0xff0c
 #define   ADIN1300_GE_SOFT_RESET		BIT(0)
 
@@ -104,6 +140,21 @@
 #define ADIN1300_RMII_20_BITS			0x0004
 #define ADIN1300_RMII_24_BITS			0x0005
 
+#define ADIN1300_GE_IO_GP_CLK_OR_CNTRL		0xff3d
+#define ADIN1300_GE_IO_GP_OUT_OR_CNTRL		0xff3e
+#define ADIN1300_GE_IO_INT_N_OR_CNTRL		0xff3f
+#define ADIN1300_GE_IO_LED_A_OR_CNTRL		0xff41
+
+/* Common definitions for ADIN1300_GE_IO_* regs */
+enum ge_io_fn {
+	GE_IO_FN_DFLT,
+	GE_IO_FN_LINK_STATUS,
+	GE_IO_FN_TX_SOP_IND,
+	GE_IO_FN_RX_SOP_IND,
+	GE_IO_FN_CRS,
+	GE_IO_FN_COL,
+};
+
 /**
  * struct adin_cfg_reg_map - map a config value to aregister value
  * @cfg		value in device configuration
@@ -172,6 +223,13 @@ static const struct adin_map adin_hw_stats[] = {
 	{ "false_carrier_events_count",		0x9414 },
 };
 
+static const struct adin_map adin_ge_io_pins[] = {
+	{ "gp_clk",	ADIN1300_GE_IO_GP_CLK_OR_CNTRL,	GENMASK(2, 0) },
+	{ "link_st",	ADIN1300_GE_IO_GP_OUT_OR_CNTRL,	GENMASK(2, 0) },
+	{ "int_n",	ADIN1300_GE_IO_INT_N_OR_CNTRL,	GENMASK(2, 0) },
+	{ "led_0",	ADIN1300_GE_IO_LED_A_OR_CNTRL,	GENMASK(3, 0) },
+};
+
 /**
  * struct adin_priv - ADIN PHY driver private data
  * stats		statistic counters for the PHY
@@ -381,6 +439,145 @@ static int adin_set_edpd(struct phy_device *phydev, u16 tx_interval)
 			  val);
 }
 
+static int adin_set_pin_function(struct phy_device *phydev,
+				 const char *prop_name,
+				 enum ge_io_fn ge_io_fn,
+				 int *selected_pin)
+{
+	struct device *dev = &phydev->mdio.dev;
+	const struct adin_map *p;
+	const char *pin_name;
+	int i, rc;
+
+	rc = device_property_read_string(dev, prop_name, &pin_name);
+	if (rc) {
+		phydev_err(phydev, "Could not get property '%s', error: %d\n",
+			   prop_name, rc);
+		return rc;
+	}
+
+	p = NULL;
+	for (i = 0; i < ARRAY_SIZE(adin_ge_io_pins); i++) {
+		if (strcmp(adin_ge_io_pins[i].string, pin_name) == 0) {
+			if (selected_pin)
+				*selected_pin = i;
+			p = &adin_ge_io_pins[i];
+			break;
+		}
+	}
+
+	if (!p) {
+		phydev_err(phydev, "Invalid pin name %s\n", pin_name);
+		return -EINVAL;
+	}
+
+	return phy_modify_mmd(phydev, MDIO_MMD_VEND1, p->val1, p->val2,
+			      ge_io_fn);
+}
+
+static int adin_config_1588_sop_rx(struct phy_device *phydev, int *rx_sop_pin)
+{
+	struct device *dev = &phydev->mdio.dev;
+	u8 values[__SOP_DELAY_MAX];
+	u16 val;
+	int rc;
+
+	rc = device_property_read_u8_array(dev,
+					   "adi,1588-rx-sop-delays-cycles",
+					   values, ARRAY_SIZE(values));
+	if (rc)
+		return 0;
+
+	rc = adin_set_pin_function(phydev, "adi,1588-rx-sop-pin-name",
+				   GE_IO_FN_RX_SOP_IND, rx_sop_pin);
+	if (rc)
+		return rc;
+
+	val = ADIN1300_SOP_RX_10_DEL_NCYC_SEL(values[SOP_DELAY_10]) |
+	      ADIN1300_SOP_RX_100_DEL_NCYC_SEL(values[SOP_DELAY_100]) |
+	      ADIN1300_SOP_RX_1000_DEL_NCYC_SEL(values[SOP_DELAY_1000]);
+
+	rc = phy_write_mmd(phydev, MDIO_MMD_VEND1, ADIN1300_SOP_RX_DELAY, val);
+	if (rc < 0) {
+		adin_set_pin_function(phydev, "adi,1588-rx-sop-pin-name",
+				      GE_IO_FN_DFLT, NULL);
+		return rc;
+	}
+
+	return 1;
+}
+
+static int adin_config_1588_sop_tx(struct phy_device *phydev, int rx_sop_pin)
+{
+	struct device *dev = &phydev->mdio.dev;
+	u8 values[__SOP_DELAY_MAX];
+	int tx_sop_pin = -1;
+	u16 val;
+	int i, rc;
+
+	rc = device_property_read_u8_array(dev,
+					   "adi,1588-tx-sop-delays-ns",
+					   values, ARRAY_SIZE(values));
+	if (rc)
+		return 0;
+
+	for (i = 0; i < __SOP_DELAY_MAX; i++) {
+		if (values[i] % 8 != 0) {
+			phydev_err(phydev,
+				   "SOP TX delays must be multiples of 8\n");
+			return -EINVAL;
+		}
+	}
+
+	val = ADIN1300_SOP_TX_10_DEL_N_8_NS_SEL(values[SOP_DELAY_10] / 8) |
+	      ADIN1300_SOP_TX_100_DEL_N_8_NS_SEL(values[SOP_DELAY_100] / 8) |
+	      ADIN1300_SOP_TX_1000_DEL_N_8_NS_SEL(values[SOP_DELAY_1000] / 8);
+
+	rc = adin_set_pin_function(phydev, "adi,1588-tx-sop-pin-name",
+				   GE_IO_FN_TX_SOP_IND, &tx_sop_pin);
+	if (rc)
+		return rc;
+
+	if (rx_sop_pin == tx_sop_pin) {
+		phydev_err(phydev,
+			   "TX SOP can't use the same pin as RX SOP\n");
+		return -EFAULT;
+	}
+
+	rc = phy_write_mmd(phydev, MDIO_MMD_VEND1, ADIN1300_SOP_TX_DELAY, val);
+	if (rc < 0) {
+		adin_set_pin_function(phydev, "adi,1588-tx-sop-pin-name",
+				      GE_IO_FN_DFLT, NULL);
+		return rc;
+	}
+
+	return 1;
+}
+
+static int adin_config_1588_sop(struct phy_device *phydev)
+{
+	int rx_sop_pin = -1;
+	u16 val = 0;
+	int rc;
+
+	rc = adin_config_1588_sop_rx(phydev, &rx_sop_pin);
+	if (rc < 0)
+		return rc;
+	if (rc == 1)
+		val |= ADIN1300_SOP_RX_EN;
+
+	rc = adin_config_1588_sop_tx(phydev, rx_sop_pin);
+	if (rc < 0)
+		return rc;
+	if (rc == 1)
+		val |= ADIN1300_SOP_TX_EN;
+
+	if (val)
+		val |= ADIN1300_SOP_SFD_EN;
+
+	return phy_write_mmd(phydev, MDIO_MMD_VEND1, ADIN1300_SOP_CTRL, val);
+}
+
 static int adin_get_tunable(struct phy_device *phydev,
 			    struct ethtool_tunable *tuna, void *data)
 {
@@ -429,6 +626,10 @@ static int adin_config_init(struct phy_device *phydev)
 	if (rc < 0)
 		return rc;
 
+	rc = adin_config_1588_sop(phydev);
+	if (rc < 0)
+		return rc;
+
 	phydev_dbg(phydev, "PHY is using mode '%s'\n",
 		   phy_modes(phydev->interface));
 
-- 
2.20.1

[PATCH 4/4] dt-bindings: net: adin: document 1588 TX/RX SOP bindings

[Alexandru Ardelean]

This change documents the device-tree bindings for the TX/RX indication of
IEEE 1588 packets.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 .../devicetree/bindings/net/adi,adin.yaml     | 60 +++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/adi,adin.yaml b/Documentation/devicetree/bindings/net/adi,adin.yaml
index d95cc691a65f..eb56f35309e0 100644
--- a/Documentation/devicetree/bindings/net/adi,adin.yaml
+++ b/Documentation/devicetree/bindings/net/adi,adin.yaml
@@ -36,6 +36,60 @@ properties:
     enum: [ 4, 8, 12, 16, 20, 24 ]
     default: 8
 
+  adi,1588-rx-sop-delays-cycles:
+    allOf:
+      - $ref: /schemas/types.yaml#definitions/uint8-array
+      - items:
+          - minItems: 3
+            maxItems: 3
+    description: |
+      Enables Start Packet detection (SOP) for received IEEE 1588 time stamp
+      controls, and configures the number of cycles (of the MII RX_CLK clock)
+      to delay the indication of RX SOP frames for 10/100/1000 BASE-T links.
+      The first element (in the array) configures the delay for 10BASE-T,
+      the second for 100BASE-T, and the third for 1000BASE-T.
+
+  adi,1588-rx-sop-pin-name:
+    description: |
+      This option must be used in together with 'adi,1588-rx-sop-delays-cycles'
+      to specify which physical pin should be used to signal the MAC that
+      the PHY is currently processing an IEEE 1588 timestamp control packet.
+      The driver will report an error if the value of this property is the
+      same as 'adi,1588-tx-sop-pin-name'
+    enum:
+      - gp_clk
+      - link_st
+      - int_n
+      - led_0
+
+  adi,1588-tx-sop-delays-ns:
+    allOf:
+      - $ref: /schemas/types.yaml#definitions/uint8-array
+      - items:
+          - minItems: 3
+            maxItems: 3
+    description: |
+      Enables Start Packet detection (SOP) for IEEE 1588 time stamp controls,
+      and configures the number of nano-seconds to delay the indication of
+      TX frames for 10/100/1000 BASE-T links.
+      The first element (in the array) configures the delay for 10BASE-T,
+      the second for 100BASE-T, and the third for 1000BASE-T.
+      The delays must be multiples of 8 ns (i.e. 8, 16, 24, etc).
+
+  adi,1588-tx-sop-pin-name:
+    description: |
+      This option must be used in together with 'adi,1588-tx-sop-delays-ns'
+      to specify which physical pin should be used to signal the MAC that
+      the PHY is currently processing an IEEE 1588 timestamp control packet
+      on the TX path.
+      The driver will report an error if the value of this property is the
+      same as 'adi,1588-rx-sop-pin-name'
+    enum:
+      - gp_clk
+      - link_st
+      - int_n
+      - led_0
+
 examples:
   - |
     ethernet {
@@ -62,5 +116,11 @@ examples:
             reg = <1>;
 
             adi,fifo-depth-bits = <16>;
+
+            adi,1588-rx-sop-delays-cycles = [ 00 00 00 ];
+            adi,1588-rx-sop-pin-name = "int_n";
+
+            adi,1588-tx-sop-delays-ns = [ 00 08 10 ];
+            adi,1588-tx-sop-pin-name = "led_0";
         };
     };
-- 
2.20.1

Re: [PATCH 1/4] net: phy: adin: const-ify static data

[Andrew Lunn]

On Thu, Jan 16, 2020 at 11:14:51AM +0200, Alexandru Ardelean wrote:
> Some bits of static data should have been made const from the start.
> This change adds the const qualifier where appropriate.
> 
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH 2/4] net: phy: adin: rename struct adin_hw_stat -> adin_map

[Andrew Lunn]

On Thu, Jan 16, 2020 at 11:14:52AM +0200, Alexandru Ardelean wrote:
> The structure format will be re-used in an upcoming change. This change
> renames to have a more generic name.

NACK.

Defining a new structure does not cost you anything. And you get type
checking, so if you were to pass a adin_map adin_ge_io_pins to a stats
function, the compiler will warn.

	  Andrew

Re: [PATCH 4/4] dt-bindings: net: adin: document 1588 TX/RX SOP bindings

[Andrew Lunn]

> +  adi,1588-rx-sop-delays-cycles:
> +    allOf:
> +      - $ref: /schemas/types.yaml#definitions/uint8-array
> +      - items:
> +          - minItems: 3
> +            maxItems: 3
> +    description: |
> +      Enables Start Packet detection (SOP) for received IEEE 1588 time stamp
> +      controls, and configures the number of cycles (of the MII RX_CLK clock)
> +      to delay the indication of RX SOP frames for 10/100/1000 BASE-T links.
> +      The first element (in the array) configures the delay for 10BASE-T,
> +      the second for 100BASE-T, and the third for 1000BASE-T.

Do you know the clock frequency? It would be much better to express
this in ns, as with adi,1588-tx-sop-delays-ns.

> @@ -62,5 +116,11 @@ examples:
>              reg = <1>;
>  
>              adi,fifo-depth-bits = <16>;
> +
> +            adi,1588-rx-sop-delays-cycles = [ 00 00 00 ];
> +            adi,1588-rx-sop-pin-name = "int_n";
> +
> +            adi,1588-tx-sop-delays-ns = [ 00 08 10 ];

10 is not a multiple of 8!

   Andrew

Re: [PATCH 3/4] net: phy: adin: implement support for 1588 start-of-packet indication

[Andrew Lunn]

On Thu, Jan 16, 2020 at 11:14:53AM +0200, Alexandru Ardelean wrote:
> The ADIN1300 & ADIN1200 PHYs support detection of IEEE 1588 time stamp
> packets. This mechanism can be used to signal the MAC via a pulse-signal
> when the PHY detects such a packet.

Do you have patches for a MAC driver? I want to see how this connects
together.

	Thanks
		Andrew	

Re: [PATCH 2/4] net: phy: adin: rename struct adin_hw_stat -> adin_map

[Ardelean, Alexandru]

On Thu, 2020-01-16 at 14:38 +0100, Andrew Lunn wrote:
> [External]
> 
> On Thu, Jan 16, 2020 at 11:14:52AM +0200, Alexandru Ardelean wrote:
> > The structure format will be re-used in an upcoming change. This change
> > renames to have a more generic name.
> 
> NACK.
> 
> Defining a new structure does not cost you anything. And you get type
> checking, so if you were to pass a adin_map adin_ge_io_pins to a stats
> function, the compiler will warn.
> 

Ack.
Will re-spin.

> 	  Andrew

Re: [PATCH 3/4] net: phy: adin: implement support for 1588 start-of-packet indication

[Ardelean, Alexandru]

On Thu, 2020-01-16 at 14:55 +0100, Andrew Lunn wrote:
> [External]
> 
> On Thu, Jan 16, 2020 at 11:14:53AM +0200, Alexandru Ardelean wrote:
> > The ADIN1300 & ADIN1200 PHYs support detection of IEEE 1588 time stamp
> > packets. This mechanism can be used to signal the MAC via a pulse-
> > signal
> > when the PHY detects such a packet.
> 
> Do you have patches for a MAC driver? I want to see how this connects
> together.

Nope.

I admit that on the MAC side, I'm not yet familiar how this is integrated.
I'd need to study this more in-depth.

> 
> 	Thanks
> 		Andrew	

Re: [PATCH 4/4] dt-bindings: net: adin: document 1588 TX/RX SOP bindings

[Ardelean, Alexandru]

On Thu, 2020-01-16 at 14:43 +0100, Andrew Lunn wrote:
> [External]
> 
> > +  adi,1588-rx-sop-delays-cycles:
> > +    allOf:
> > +      - $ref: /schemas/types.yaml#definitions/uint8-array
> > +      - items:
> > +          - minItems: 3
> > +            maxItems: 3
> > +    description: |
> > +      Enables Start Packet detection (SOP) for received IEEE 1588 time
> > stamp
> > +      controls, and configures the number of cycles (of the MII RX_CLK
> > clock)
> > +      to delay the indication of RX SOP frames for 10/100/1000 BASE-T
> > links.
> > +      The first element (in the array) configures the delay for
> > 10BASE-T,
> > +      the second for 100BASE-T, and the third for 1000BASE-T.
> 
> Do you know the clock frequency? It would be much better to express
> this in ns, as with adi,1588-tx-sop-delays-ns.

Yep.
We know the clock frequency here.
I'll take a look about converting this.

> 
> > @@ -62,5 +116,11 @@ examples:
> >              reg = <1>;
> >  
> >              adi,fifo-depth-bits = <16>;
> > +
> > +            adi,1588-rx-sop-delays-cycles = [ 00 00 00 ];
> > +            adi,1588-rx-sop-pin-name = "int_n";
> > +
> > +            adi,1588-tx-sop-delays-ns = [ 00 08 10 ];
> 
> 10 is not a multiple of 8!

My bad here.
I should point-out somewhere that these are hex-values.
It's kind of implied via the DT uint8 array type.

> 
>    Andrew

Re: [PATCH 3/4] net: phy: adin: implement support for 1588 start-of-packet indication

[Andrew Lunn]

On Thu, Jan 16, 2020 at 01:58:55PM +0000, Ardelean, Alexandru wrote:
> On Thu, 2020-01-16 at 14:55 +0100, Andrew Lunn wrote:
> > [External]
> > 
> > On Thu, Jan 16, 2020 at 11:14:53AM +0200, Alexandru Ardelean wrote:
> > > The ADIN1300 & ADIN1200 PHYs support detection of IEEE 1588 time stamp
> > > packets. This mechanism can be used to signal the MAC via a pulse-
> > > signal
> > > when the PHY detects such a packet.
> > 
> > Do you have patches for a MAC driver? I want to see how this connects
> > together.
> 
> Nope.
> 
> I admit that on the MAC side, I'm not yet familiar how this is integrated.
> I'd need to study this more in-depth.

O.K.

Then i suggest you post patch #1 as a single patch. And then work on
the MAC side, and post both MAC and PHY as a complete and tested
patchset.

Thanks
	Andrew

Re: [PATCH 3/4] net: phy: adin: implement support for 1588 start-of-packet indication

[Ardelean, Alexandru]

On Thu, 2020-01-16 at 15:02 +0100, Andrew Lunn wrote:
> [External]
> 
> On Thu, Jan 16, 2020 at 01:58:55PM +0000, Ardelean, Alexandru wrote:
> > On Thu, 2020-01-16 at 14:55 +0100, Andrew Lunn wrote:
> > > [External]
> > > 
> > > On Thu, Jan 16, 2020 at 11:14:53AM +0200, Alexandru Ardelean wrote:
> > > > The ADIN1300 & ADIN1200 PHYs support detection of IEEE 1588 time
> > > > stamp
> > > > packets. This mechanism can be used to signal the MAC via a pulse-
> > > > signal
> > > > when the PHY detects such a packet.
> > > 
> > > Do you have patches for a MAC driver? I want to see how this connects
> > > together.
> > 
> > Nope.
> > 
> > I admit that on the MAC side, I'm not yet familiar how this is
> > integrated.
> > I'd need to study this more in-depth.
> 
> O.K.
> 
> Then i suggest you post patch #1 as a single patch. And then work on
> the MAC side, and post both MAC and PHY as a complete and tested
> patchset.
> 

Ack
Thanks

> Thanks
> 	Andrew

Re: [PATCH 4/4] dt-bindings: net: adin: document 1588 TX/RX SOP bindings

[Rob Herring]

On Thu, Jan 16, 2020 at 11:14:54AM +0200, Alexandru Ardelean wrote:
> This change documents the device-tree bindings for the TX/RX indication of
> IEEE 1588 packets.
> 
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
>  .../devicetree/bindings/net/adi,adin.yaml     | 60 +++++++++++++++++++
>  1 file changed, 60 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/adi,adin.yaml b/Documentation/devicetree/bindings/net/adi,adin.yaml
> index d95cc691a65f..eb56f35309e0 100644
> --- a/Documentation/devicetree/bindings/net/adi,adin.yaml
> +++ b/Documentation/devicetree/bindings/net/adi,adin.yaml
> @@ -36,6 +36,60 @@ properties:
>      enum: [ 4, 8, 12, 16, 20, 24 ]
>      default: 8
>  
> +  adi,1588-rx-sop-delays-cycles:
> +    allOf:
> +      - $ref: /schemas/types.yaml#definitions/uint8-array
> +      - items:
> +          - minItems: 3
> +            maxItems: 3

You can split up the description into constraints something like this 
(and minItems/maxItems becomes implied):

items:
  - description: delay for 10BASE-T
  - description: delay for 100BASE-T
  - description: delay for 1000BASE-T

> +    description: |
> +      Enables Start Packet detection (SOP) for received IEEE 1588 time stamp
> +      controls, and configures the number of cycles (of the MII RX_CLK clock)
> +      to delay the indication of RX SOP frames for 10/100/1000 BASE-T links.
> +      The first element (in the array) configures the delay for 10BASE-T,
> +      the second for 100BASE-T, and the third for 1000BASE-T.
> +
> +  adi,1588-rx-sop-pin-name:
> +    description: |
> +      This option must be used in together with 'adi,1588-rx-sop-delays-cycles'
> +      to specify which physical pin should be used to signal the MAC that
> +      the PHY is currently processing an IEEE 1588 timestamp control packet.
> +      The driver will report an error if the value of this property is the
> +      same as 'adi,1588-tx-sop-pin-name'
> +    enum:
> +      - gp_clk
> +      - link_st
> +      - int_n
> +      - led_0
> +
> +  adi,1588-tx-sop-delays-ns:
> +    allOf:
> +      - $ref: /schemas/types.yaml#definitions/uint8-array
> +      - items:
> +          - minItems: 3
> +            maxItems: 3

This should be:

      - minItems: 3
        maxItems: 3
        items:
          multipleOf: 8

> +    description: |
> +      Enables Start Packet detection (SOP) for IEEE 1588 time stamp controls,
> +      and configures the number of nano-seconds to delay the indication of
> +      TX frames for 10/100/1000 BASE-T links.
> +      The first element (in the array) configures the delay for 10BASE-T,
> +      the second for 100BASE-T, and the third for 1000BASE-T.
> +      The delays must be multiples of 8 ns (i.e. 8, 16, 24, etc).
> +
> +  adi,1588-tx-sop-pin-name:
> +    description: |
> +      This option must be used in together with 'adi,1588-tx-sop-delays-ns'
> +      to specify which physical pin should be used to signal the MAC that
> +      the PHY is currently processing an IEEE 1588 timestamp control packet
> +      on the TX path.
> +      The driver will report an error if the value of this property is the
> +      same as 'adi,1588-rx-sop-pin-name'
> +    enum:
> +      - gp_clk
> +      - link_st
> +      - int_n
> +      - led_0
> +
>  examples:
>    - |
>      ethernet {
> @@ -62,5 +116,11 @@ examples:
>              reg = <1>;
>  
>              adi,fifo-depth-bits = <16>;
> +
> +            adi,1588-rx-sop-delays-cycles = [ 00 00 00 ];
> +            adi,1588-rx-sop-pin-name = "int_n";
> +
> +            adi,1588-tx-sop-delays-ns = [ 00 08 10 ];
> +            adi,1588-tx-sop-pin-name = "led_0";
>          };
>      };
> -- 
> 2.20.1
>