[PATCH net v4 1/2] imx7s/imx7d has the ptp interrupt newly added as well.

[PATCH net v4 1/2] imx7s/imx7d has the ptp interrupt newly added as well.

[Troy Kisky]

For imx7, "int0" is the interrupt for queue 0 and ENET_MII
"int1" is for queue 1
"int2" is for queue 2

For imx6sx, "int0" handles all 3 queues and ENET_MII

And of course, the "pps" interrupt is for the PTP_CLOCK_PPS interrupts
This will help document what each interrupt does.

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

---
v2: replaced empty names with "int0","int1", or "int2"

reordered imx7 interrupts so that "int0", for queue 0, is first.

v3: renamed "ptp" interrupt to "pps", added binding documentation
for interrupt-names.

v4: add blank, ie s/"int0","pps"/"int0", "pps"/ as suggested by Andrew Lunn
---
 Documentation/devicetree/bindings/net/fsl-fec.txt | 13 +++++++++++++
 arch/arm/boot/dts/imx6qdl.dtsi                    |  1 +
 arch/arm/boot/dts/imx6sx.dtsi                     |  2 ++
 arch/arm/boot/dts/imx6ul.dtsi                     |  2 ++
 arch/arm/boot/dts/imx7d.dtsi                      |  6 ++++--
 arch/arm/boot/dts/imx7s.dtsi                      |  6 ++++--
 6 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt
index 6f55bdd52f8a..f0dc94409107 100644
--- a/Documentation/devicetree/bindings/net/fsl-fec.txt
+++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
@@ -34,6 +34,19 @@ Optional properties:
 - fsl,err006687-workaround-present: If present indicates that the system has
   the hardware workaround for ERR006687 applied and does not need a software
   workaround.
+ -interrupt-names:  names of the interrupts listed in interrupts property in
+  the same order. The defaults if not specified are
+  __Number of interrupts__   __Default__
+	1			"int0"
+	2			"int0", "pps"
+	3			"int0", "int1", "int2"
+	4			"int0", "int1", "int2", "pps"
+  The order may be changed as long as they correspond to the interrupts
+  property. Currently, only i.mx7 uses "int1" and "int2". They correspond to
+  tx/rx queues 1 and 2. "int0" will be used for queue 0 and ENET_MII interrupts.
+  For imx6sx, "int0" handles all 3 queues and ENET_MII. "pps" is for the pulse
+  per second interrupt associated with 1588 precision time protocol(PTP).
+
 
 Optional subnodes:
 - mdio : specifies the mdio bus in the FEC, used as a container for phy nodes
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 8884b4a3cafb..f97dee04b9be 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -1017,6 +1017,7 @@
 			fec: ethernet@02188000 {
 				compatible = "fsl,imx6q-fec";
 				reg = <0x02188000 0x4000>;
+				interrupt-names = "int0", "pps";
 				interrupts-extended =
 					<&intc 0 118 IRQ_TYPE_LEVEL_HIGH>,
 					<&intc 0 119 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index 6c7eb54be9e2..7f3773b88c47 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -861,6 +861,7 @@
 			fec1: ethernet@02188000 {
 				compatible = "fsl,imx6sx-fec", "fsl,imx6q-fec";
 				reg = <0x02188000 0x4000>;
+				interrupt-names = "int0", "pps";
 				interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>,
 					     <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6SX_CLK_ENET>,
@@ -970,6 +971,7 @@
 			fec2: ethernet@021b4000 {
 				compatible = "fsl,imx6sx-fec", "fsl,imx6q-fec";
 				reg = <0x021b4000 0x4000>;
+				interrupt-names = "int0", "pps";
 				interrupts = <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,
 					     <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6SX_CLK_ENET>,
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index f11a241a340d..5ac00ff959b2 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -476,6 +476,7 @@
 			fec2: ethernet@020b4000 {
 				compatible = "fsl,imx6ul-fec", "fsl,imx6q-fec";
 				reg = <0x020b4000 0x4000>;
+				interrupt-names = "int0", "pps";
 				interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>,
 					     <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6UL_CLK_ENET>,
@@ -775,6 +776,7 @@
 			fec1: ethernet@02188000 {
 				compatible = "fsl,imx6ul-fec", "fsl,imx6q-fec";
 				reg = <0x02188000 0x4000>;
+				interrupt-names = "int0", "pps";
 				interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>,
 					     <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6UL_CLK_ENET>,
diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
index 4d308d17f040..8b9c8c0695df 100644
--- a/arch/arm/boot/dts/imx7d.dtsi
+++ b/arch/arm/boot/dts/imx7d.dtsi
@@ -114,9 +114,11 @@
 	fec2: ethernet@30bf0000 {
 		compatible = "fsl,imx7d-fec", "fsl,imx6sx-fec";
 		reg = <0x30bf0000 0x10000>;
-		interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>,
+		interrupt-names = "int0", "int1", "int2", "pps";
+		interrupts = <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,
+			<GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>,
 			<GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>,
-			<GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>;
+			<GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&clks IMX7D_ENET_AXI_ROOT_CLK>,
 			<&clks IMX7D_ENET_AXI_ROOT_CLK>,
 			<&clks IMX7D_ENET2_TIME_ROOT_CLK>,
diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 82ad26e766eb..966b97fdc394 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -1007,9 +1007,11 @@
 			fec1: ethernet@30be0000 {
 				compatible = "fsl,imx7d-fec", "fsl,imx6sx-fec";
 				reg = <0x30be0000 0x10000>;
-				interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>,
+				interrupt-names = "int0", "int1", "int2", "pps";
+				interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>,
+					<GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>,
 					<GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>,
-					<GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
+					<GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX7D_ENET_AXI_ROOT_CLK>,
 					<&clks IMX7D_ENET_AXI_ROOT_CLK>,
 					<&clks IMX7D_ENET1_TIME_ROOT_CLK>,
-- 
2.11.0

[PATCH net v4 2/2] net: fec: Let fec_ptp have its own interrupt routine

[Troy Kisky]

This is better for code locality and should slightly
speed up normal interrupts.

This also allows PPS clock output to start working for
i.mx7. This is because i.mx7 was already using the limit
of 3 interrupts, and needed another.

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

---

v2: made this change independent of any devicetree change
so that old dtbs continue to work.

Continue to register ptp clock if interrupt is not found.

v3: renamed "ptp" interrupt to "pps" interrupt

v4: no change
---
 drivers/net/ethernet/freescale/fec.h      |  3 +-
 drivers/net/ethernet/freescale/fec_main.c | 25 ++++++----
 drivers/net/ethernet/freescale/fec_ptp.c  | 82 ++++++++++++++++++-------------
 3 files changed, 65 insertions(+), 45 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index ede1876a9a19..be56ac1f1ac4 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -582,12 +582,11 @@ struct fec_enet_private {
 	u64 ethtool_stats[0];
 };
 
-void fec_ptp_init(struct platform_device *pdev);
+void fec_ptp_init(struct platform_device *pdev, int irq_index);
 void fec_ptp_stop(struct platform_device *pdev);
 void fec_ptp_start_cyclecounter(struct net_device *ndev);
 int fec_ptp_set(struct net_device *ndev, struct ifreq *ifr);
 int fec_ptp_get(struct net_device *ndev, struct ifreq *ifr);
-uint fec_ptp_check_pps_event(struct fec_enet_private *fep);
 
 /****************************************************************************/
 #endif /* FEC_H */
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 3dc2d771a222..18fc136817c9 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1602,10 +1602,6 @@ fec_enet_interrupt(int irq, void *dev_id)
 		ret = IRQ_HANDLED;
 		complete(&fep->mdio_done);
 	}
-
-	if (fep->ptp_clock)
-		if (fec_ptp_check_pps_event(fep))
-			ret = IRQ_HANDLED;
 	return ret;
 }
 
@@ -3325,6 +3321,8 @@ fec_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node, *phy_node;
 	int num_tx_qs;
 	int num_rx_qs;
+	char irq_name[8];
+	int irq_cnt;
 
 	fec_enet_get_queue_num(pdev, &num_tx_qs, &num_rx_qs);
 
@@ -3465,18 +3463,27 @@ fec_probe(struct platform_device *pdev)
 	if (ret)
 		goto failed_reset;
 
+	irq_cnt = platform_irq_count(pdev);
+	if (irq_cnt > FEC_IRQ_NUM)
+		irq_cnt = FEC_IRQ_NUM;	/* last for pps */
+	else if (irq_cnt == 2)
+		irq_cnt = 1;	/* last for pps */
+	else if (irq_cnt <= 0)
+		irq_cnt = 1;	/* Let the for loop fail */
+
 	if (fep->bufdesc_ex)
-		fec_ptp_init(pdev);
+		fec_ptp_init(pdev, irq_cnt);
 
 	ret = fec_enet_init(ndev);
 	if (ret)
 		goto failed_init;
 
-	for (i = 0; i < FEC_IRQ_NUM; i++) {
-		irq = platform_get_irq(pdev, i);
+	for (i = 0; i < irq_cnt; i++) {
+		sprintf(irq_name, "int%d", i);
+		irq = platform_get_irq_byname(pdev, irq_name);
+		if (irq < 0)
+			irq = platform_get_irq(pdev, i);
 		if (irq < 0) {
-			if (i)
-				break;
 			ret = irq;
 			goto failed_irq;
 		}
diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
index 6ebad3fac81d..0f59b0ea1001 100644
--- a/drivers/net/ethernet/freescale/fec_ptp.c
+++ b/drivers/net/ethernet/freescale/fec_ptp.c
@@ -549,6 +549,37 @@ static void fec_time_keep(struct work_struct *work)
 	schedule_delayed_work(&fep->time_keep, HZ);
 }
 
+/* This function checks the pps event and reloads the timer compare counter. */
+static irqreturn_t fec_pps_interrupt(int irq, void *dev_id)
+{
+	struct net_device *ndev = dev_id;
+	struct fec_enet_private *fep = netdev_priv(ndev);
+	u32 val;
+	u8 channel = fep->pps_channel;
+	struct ptp_clock_event event;
+
+	val = readl(fep->hwp + FEC_TCSR(channel));
+	if (val & FEC_T_TF_MASK) {
+		/* Write the next next compare(not the next according the spec)
+		 * value to the register
+		 */
+		writel(fep->next_counter, fep->hwp + FEC_TCCR(channel));
+		do {
+			writel(val, fep->hwp + FEC_TCSR(channel));
+		} while (readl(fep->hwp + FEC_TCSR(channel)) & FEC_T_TF_MASK);
+
+		/* Update the counter; */
+		fep->next_counter = (fep->next_counter + fep->reload_period) &
+				fep->cc.mask;
+
+		event.type = PTP_CLOCK_PPS;
+		ptp_clock_event(fep->ptp_clock, &event);
+		return IRQ_HANDLED;
+	}
+
+	return IRQ_NONE;
+}
+
 /**
  * fec_ptp_init
  * @ndev: The FEC network adapter
@@ -558,10 +589,12 @@ static void fec_time_keep(struct work_struct *work)
  * cyclecounter init routine and exits.
  */
 
-void fec_ptp_init(struct platform_device *pdev)
+void fec_ptp_init(struct platform_device *pdev, int irq_index)
 {
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct fec_enet_private *fep = netdev_priv(ndev);
+	int irq;
+	int ret;
 
 	fep->ptp_caps.owner = THIS_MODULE;
 	snprintf(fep->ptp_caps.name, 16, "fec ptp");
@@ -587,6 +620,20 @@ void fec_ptp_init(struct platform_device *pdev)
 
 	INIT_DELAYED_WORK(&fep->time_keep, fec_time_keep);
 
+	irq = platform_get_irq_byname(pdev, "pps");
+	if (irq < 0)
+		irq = platform_get_irq(pdev, irq_index);
+	/* Failure to get an irq is not fatal,
+	 * only the PTP_CLOCK_PPS clock events should stop
+	 */
+	if (irq >= 0) {
+		ret = devm_request_irq(&pdev->dev, irq, fec_pps_interrupt,
+				       0, pdev->name, ndev);
+		if (ret < 0)
+			dev_warn(&pdev->dev, "request for ptp irq failed(%d)\n",
+				 ret);
+	}
+
 	fep->ptp_clock = ptp_clock_register(&fep->ptp_caps, &pdev->dev);
 	if (IS_ERR(fep->ptp_clock)) {
 		fep->ptp_clock = NULL;
@@ -605,36 +652,3 @@ void fec_ptp_stop(struct platform_device *pdev)
 	if (fep->ptp_clock)
 		ptp_clock_unregister(fep->ptp_clock);
 }
-
-/**
- * fec_ptp_check_pps_event
- * @fep: the fec_enet_private structure handle
- *
- * This function check the pps event and reload the timer compare counter.
- */
-uint fec_ptp_check_pps_event(struct fec_enet_private *fep)
-{
-	u32 val;
-	u8 channel = fep->pps_channel;
-	struct ptp_clock_event event;
-
-	val = readl(fep->hwp + FEC_TCSR(channel));
-	if (val & FEC_T_TF_MASK) {
-		/* Write the next next compare(not the next according the spec)
-		 * value to the register
-		 */
-		writel(fep->next_counter, fep->hwp + FEC_TCCR(channel));
-		do {
-			writel(val, fep->hwp + FEC_TCSR(channel));
-		} while (readl(fep->hwp + FEC_TCSR(channel)) & FEC_T_TF_MASK);
-
-		/* Update the counter; */
-		fep->next_counter = (fep->next_counter + fep->reload_period) & fep->cc.mask;
-
-		event.type = PTP_CLOCK_PPS;
-		ptp_clock_event(fep->ptp_clock, &event);
-		return 1;
-	}
-
-	return 0;
-}
-- 
2.11.0

RE: [PATCH net v4 2/2] net: fec: Let fec_ptp have its own interrupt routine

[Andy Duan]

From: Troy Kisky <troy.kisky@boundarydevices.com> Sent: Wednesday, November 01, 2017 4:17 AM
>This is better for code locality and should slightly speed up normal interrupts.
>
>This also allows PPS clock output to start working for i.mx7. This is because
>i.mx7 was already using the limit of 3 interrupts, and needed another.
>
>Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
>
>---
>
>v2: made this change independent of any devicetree change so that old dtbs
>continue to work.
>
>Continue to register ptp clock if interrupt is not found.
>
>v3: renamed "ptp" interrupt to "pps" interrupt
>
>v4: no change
>---
> drivers/net/ethernet/freescale/fec.h      |  3 +-
> drivers/net/ethernet/freescale/fec_main.c | 25 ++++++----
>drivers/net/ethernet/freescale/fec_ptp.c  | 82 ++++++++++++++++++--------
>-----
> 3 files changed, 65 insertions(+), 45 deletions(-)
>
>diff --git a/drivers/net/ethernet/freescale/fec.h
>b/drivers/net/ethernet/freescale/fec.h
>index ede1876a9a19..be56ac1f1ac4 100644
>--- a/drivers/net/ethernet/freescale/fec.h
>+++ b/drivers/net/ethernet/freescale/fec.h
>@@ -582,12 +582,11 @@ struct fec_enet_private {
> 	u64 ethtool_stats[0];
> };
>
>-void fec_ptp_init(struct platform_device *pdev);
>+void fec_ptp_init(struct platform_device *pdev, int irq_index);

Seems change irq_index to irq_idx much better.

> void fec_ptp_stop(struct platform_device *pdev);  void
>fec_ptp_start_cyclecounter(struct net_device *ndev);  int fec_ptp_set(struct
>net_device *ndev, struct ifreq *ifr);  int fec_ptp_get(struct net_device *ndev,
>struct ifreq *ifr); -uint fec_ptp_check_pps_event(struct fec_enet_private
>*fep);
>
>
>/**********************************************************
>******************/
> #endif /* FEC_H */
>diff --git a/drivers/net/ethernet/freescale/fec_main.c
>b/drivers/net/ethernet/freescale/fec_main.c
>index 3dc2d771a222..18fc136817c9 100644
>--- a/drivers/net/ethernet/freescale/fec_main.c
>+++ b/drivers/net/ethernet/freescale/fec_main.c
>@@ -1602,10 +1602,6 @@ fec_enet_interrupt(int irq, void *dev_id)
> 		ret = IRQ_HANDLED;
> 		complete(&fep->mdio_done);
> 	}
>-
>-	if (fep->ptp_clock)
>-		if (fec_ptp_check_pps_event(fep))
>-			ret = IRQ_HANDLED;
> 	return ret;
> }
>
>@@ -3325,6 +3321,8 @@ fec_probe(struct platform_device *pdev)
> 	struct device_node *np = pdev->dev.of_node, *phy_node;
> 	int num_tx_qs;
> 	int num_rx_qs;
>+	char irq_name[8];
>+	int irq_cnt;
>
> 	fec_enet_get_queue_num(pdev, &num_tx_qs, &num_rx_qs);
>
>@@ -3465,18 +3463,27 @@ fec_probe(struct platform_device *pdev)
> 	if (ret)
> 		goto failed_reset;
>
>+	irq_cnt = platform_irq_count(pdev);
>+	if (irq_cnt > FEC_IRQ_NUM)
>+		irq_cnt = FEC_IRQ_NUM;	/* last for pps */
>+	else if (irq_cnt == 2)
>+		irq_cnt = 1;	/* last for pps */
>+	else if (irq_cnt <= 0)
>+		irq_cnt = 1;	/* Let the for loop fail */
>+

Do some parse on probe function seems uncomfortable...can you encapsulate these code into one api ?

Others seems fine to me.
Thanks.

> 	if (fep->bufdesc_ex)
>-		fec_ptp_init(pdev);
>+		fec_ptp_init(pdev, irq_cnt);
>
> 	ret = fec_enet_init(ndev);
> 	if (ret)
> 		goto failed_init;
>
>-	for (i = 0; i < FEC_IRQ_NUM; i++) {
>-		irq = platform_get_irq(pdev, i);
>+	for (i = 0; i < irq_cnt; i++) {
>+		sprintf(irq_name, "int%d", i);
>+		irq = platform_get_irq_byname(pdev, irq_name);
>+		if (irq < 0)
>+			irq = platform_get_irq(pdev, i);
> 		if (irq < 0) {
>-			if (i)
>-				break;
> 			ret = irq;
> 			goto failed_irq;
> 		}
>diff --git a/drivers/net/ethernet/freescale/fec_ptp.c
>b/drivers/net/ethernet/freescale/fec_ptp.c
>index 6ebad3fac81d..0f59b0ea1001 100644
>--- a/drivers/net/ethernet/freescale/fec_ptp.c
>+++ b/drivers/net/ethernet/freescale/fec_ptp.c
>@@ -549,6 +549,37 @@ static void fec_time_keep(struct work_struct *work)
> 	schedule_delayed_work(&fep->time_keep, HZ);  }
>
>+/* This function checks the pps event and reloads the timer compare
>+counter. */ static irqreturn_t fec_pps_interrupt(int irq, void *dev_id)
>+{
>+	struct net_device *ndev = dev_id;
>+	struct fec_enet_private *fep = netdev_priv(ndev);
>+	u32 val;
>+	u8 channel = fep->pps_channel;
>+	struct ptp_clock_event event;
>+
>+	val = readl(fep->hwp + FEC_TCSR(channel));
>+	if (val & FEC_T_TF_MASK) {
>+		/* Write the next next compare(not the next according the
>spec)
>+		 * value to the register
>+		 */
>+		writel(fep->next_counter, fep->hwp + FEC_TCCR(channel));
>+		do {
>+			writel(val, fep->hwp + FEC_TCSR(channel));
>+		} while (readl(fep->hwp + FEC_TCSR(channel)) &
>FEC_T_TF_MASK);
>+
>+		/* Update the counter; */
>+		fep->next_counter = (fep->next_counter + fep-
>>reload_period) &
>+				fep->cc.mask;
>+
>+		event.type = PTP_CLOCK_PPS;
>+		ptp_clock_event(fep->ptp_clock, &event);
>+		return IRQ_HANDLED;
>+	}
>+
>+	return IRQ_NONE;
>+}
>+
> /**
>  * fec_ptp_init
>  * @ndev: The FEC network adapter
>@@ -558,10 +589,12 @@ static void fec_time_keep(struct work_struct *work)
>  * cyclecounter init routine and exits.
>  */
>
>-void fec_ptp_init(struct platform_device *pdev)
>+void fec_ptp_init(struct platform_device *pdev, int irq_index)
> {
> 	struct net_device *ndev = platform_get_drvdata(pdev);
> 	struct fec_enet_private *fep = netdev_priv(ndev);
>+	int irq;
>+	int ret;
>
> 	fep->ptp_caps.owner = THIS_MODULE;
> 	snprintf(fep->ptp_caps.name, 16, "fec ptp"); @@ -587,6 +620,20 @@
>void fec_ptp_init(struct platform_device *pdev)
>
> 	INIT_DELAYED_WORK(&fep->time_keep, fec_time_keep);
>
>+	irq = platform_get_irq_byname(pdev, "pps");
>+	if (irq < 0)
>+		irq = platform_get_irq(pdev, irq_index);
>+	/* Failure to get an irq is not fatal,
>+	 * only the PTP_CLOCK_PPS clock events should stop
>+	 */
>+	if (irq >= 0) {
>+		ret = devm_request_irq(&pdev->dev, irq, fec_pps_interrupt,
>+				       0, pdev->name, ndev);
>+		if (ret < 0)
>+			dev_warn(&pdev->dev, "request for ptp irq
>failed(%d)\n",
>+				 ret);
>+	}
>+
> 	fep->ptp_clock = ptp_clock_register(&fep->ptp_caps, &pdev->dev);
> 	if (IS_ERR(fep->ptp_clock)) {
> 		fep->ptp_clock = NULL;
>@@ -605,36 +652,3 @@ void fec_ptp_stop(struct platform_device *pdev)
> 	if (fep->ptp_clock)
> 		ptp_clock_unregister(fep->ptp_clock);
> }
>-
>-/**
>- * fec_ptp_check_pps_event
>- * @fep: the fec_enet_private structure handle
>- *
>- * This function check the pps event and reload the timer compare counter.
>- */
>-uint fec_ptp_check_pps_event(struct fec_enet_private *fep) -{
>-	u32 val;
>-	u8 channel = fep->pps_channel;
>-	struct ptp_clock_event event;
>-
>-	val = readl(fep->hwp + FEC_TCSR(channel));
>-	if (val & FEC_T_TF_MASK) {
>-		/* Write the next next compare(not the next according the
>spec)
>-		 * value to the register
>-		 */
>-		writel(fep->next_counter, fep->hwp + FEC_TCCR(channel));
>-		do {
>-			writel(val, fep->hwp + FEC_TCSR(channel));
>-		} while (readl(fep->hwp + FEC_TCSR(channel)) &
>FEC_T_TF_MASK);
>-
>-		/* Update the counter; */
>-		fep->next_counter = (fep->next_counter + fep-
>>reload_period) & fep->cc.mask;
>-
>-		event.type = PTP_CLOCK_PPS;
>-		ptp_clock_event(fep->ptp_clock, &event);
>-		return 1;
>-	}
>-
>-	return 0;
>-}
>--
>2.11.0

Re: [PATCH net v4 2/2] net: fec: Let fec_ptp have its own interrupt routine

[Troy Kisky]

On 10/31/2017 7:43 PM, Andy Duan wrote:
> From: Troy Kisky <troy.kisky@boundarydevices.com> Sent: Wednesday, November 01, 2017 4:17 AM
>> This is better for code locality and should slightly speed up normal interrupts.
>>
>> This also allows PPS clock output to start working for i.mx7. This is because
>> i.mx7 was already using the limit of 3 interrupts, and needed another.
>>
>> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
>>
>> ---
>>
>> v2: made this change independent of any devicetree change so that old dtbs
>> continue to work.
>>
>> Continue to register ptp clock if interrupt is not found.
>>
>> v3: renamed "ptp" interrupt to "pps" interrupt
>>
>> v4: no change
>> ---
>> drivers/net/ethernet/freescale/fec.h      |  3 +-
>> drivers/net/ethernet/freescale/fec_main.c | 25 ++++++----
>> drivers/net/ethernet/freescale/fec_ptp.c  | 82 ++++++++++++++++++--------
>> -----
>> 3 files changed, 65 insertions(+), 45 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/freescale/fec.h
>> b/drivers/net/ethernet/freescale/fec.h
>> index ede1876a9a19..be56ac1f1ac4 100644
>> --- a/drivers/net/ethernet/freescale/fec.h
>> +++ b/drivers/net/ethernet/freescale/fec.h
>> @@ -582,12 +582,11 @@ struct fec_enet_private {
>> 	u64 ethtool_stats[0];
>> };
>>
>> -void fec_ptp_init(struct platform_device *pdev);
>> +void fec_ptp_init(struct platform_device *pdev, int irq_index);
> 
> Seems change irq_index to irq_idx much better.


no problem

>> @@ -3465,18 +3463,27 @@ fec_probe(struct platform_device *pdev)
>> 	if (ret)
>> 		goto failed_reset;
>>
>> +	irq_cnt = platform_irq_count(pdev);
>> +	if (irq_cnt > FEC_IRQ_NUM)
>> +		irq_cnt = FEC_IRQ_NUM;	/* last for pps */
>> +	else if (irq_cnt == 2)
>> +		irq_cnt = 1;	/* last for pps */
>> +	else if (irq_cnt <= 0)
>> +		irq_cnt = 1;	/* Let the for loop fail */
>> +
> 
> Do some parse on probe function seems uncomfortable...can you encapsulate these code into one api ?
> 


Do you mean something like
	irq_cnt = limit_irq(pdev);


int limit_irq(struct platform_device *pdev)
{
	int irq_cnt = platform_irq_count(pdev);

	if (irq_cnt > FEC_IRQ_NUM)
		irq_cnt = FEC_IRQ_NUM;	/* last for pps */
	else if (irq_cnt == 2)
		irq_cnt = 1;	/* last for pps */
	else if (irq_cnt <= 0)
		irq_cnt = 1;	/* At least 1 irq is needed */
	return irq_cnt;
}
___________
Can you give some code to your idea? I don't think I understand.


	
> Others seems fine to me.
> Thanks.
> 

RE: [PATCH net v4 2/2] net: fec: Let fec_ptp have its own interrupt routine

[Andy Duan]

From: Troy Kisky <troy.kisky@boundarydevices.com>  Sent: Thursday, November 02, 2017 1:36 AM
>To: Andy Duan <fugang.duan@nxp.com>; shawn.guo@linaro.org;
>netdev@vger.kernel.org; davem@davemloft.net
>Cc: Fabio Estevam <fabio.estevam@nxp.com>; lznuaa@gmail.com;
>andrew@lunn.ch
>Subject: Re: [PATCH net v4 2/2] net: fec: Let fec_ptp have its own interrupt
>routine
>
>On 10/31/2017 7:43 PM, Andy Duan wrote:
>> From: Troy Kisky <troy.kisky@boundarydevices.com> Sent: Wednesday,
>> November 01, 2017 4:17 AM
>>> This is better for code locality and should slightly speed up normal
>interrupts.
>>>
>>> This also allows PPS clock output to start working for i.mx7. This is
>>> because
>>> i.mx7 was already using the limit of 3 interrupts, and needed another.
>>>
>>> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
>>>
>>> ---
>>>
>>> v2: made this change independent of any devicetree change so that old
>>> dtbs continue to work.
>>>
>>> Continue to register ptp clock if interrupt is not found.
>>>
>>> v3: renamed "ptp" interrupt to "pps" interrupt
>>>
>>> v4: no change
>>> ---
>>> drivers/net/ethernet/freescale/fec.h      |  3 +-
>>> drivers/net/ethernet/freescale/fec_main.c | 25 ++++++----
>>> drivers/net/ethernet/freescale/fec_ptp.c  | 82
>>> ++++++++++++++++++--------
>>> -----
>>> 3 files changed, 65 insertions(+), 45 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/freescale/fec.h
>>> b/drivers/net/ethernet/freescale/fec.h
>>> index ede1876a9a19..be56ac1f1ac4 100644
>>> --- a/drivers/net/ethernet/freescale/fec.h
>>> +++ b/drivers/net/ethernet/freescale/fec.h
>>> @@ -582,12 +582,11 @@ struct fec_enet_private {
>>> 	u64 ethtool_stats[0];
>>> };
>>>
>>> -void fec_ptp_init(struct platform_device *pdev);
>>> +void fec_ptp_init(struct platform_device *pdev, int irq_index);
>>
>> Seems change irq_index to irq_idx much better.
>
>
>no problem
>
>>> @@ -3465,18 +3463,27 @@ fec_probe(struct platform_device *pdev)
>>> 	if (ret)
>>> 		goto failed_reset;
>>>
>>> +	irq_cnt = platform_irq_count(pdev);
>>> +	if (irq_cnt > FEC_IRQ_NUM)
>>> +		irq_cnt = FEC_IRQ_NUM;	/* last for pps */
>>> +	else if (irq_cnt == 2)
>>> +		irq_cnt = 1;	/* last for pps */
>>> +	else if (irq_cnt <= 0)
>>> +		irq_cnt = 1;	/* Let the for loop fail */
>>> +
>>
>> Do some parse on probe function seems uncomfortable...can you
>encapsulate these code into one api ?
>>
>
>
>Do you mean something like
>	irq_cnt = limit_irq(pdev);
>
>
>int limit_irq(struct platform_device *pdev) {

It is better: fec_enet_get_irq_cnt()

>	int irq_cnt = platform_irq_count(pdev);
>
>	if (irq_cnt > FEC_IRQ_NUM)
>		irq_cnt = FEC_IRQ_NUM;	/* last for pps */
>	else if (irq_cnt == 2)
>		irq_cnt = 1;	/* last for pps */
>	else if (irq_cnt <= 0)
>		irq_cnt = 1;	/* At least 1 irq is needed */
>	return irq_cnt;
>}
>___________
>Can you give some code to your idea? I don't think I understand.

Others seems fine for me.

Re: [PATCH net v4 1/2] imx7s/imx7d has the ptp interrupt newly added as well.

[Shawn Guo]

On Wed, Nov 1, 2017 at 4:16 AM, Troy Kisky
<troy.kisky@boundarydevices.com> wrote:
> For imx7, "int0" is the interrupt for queue 0 and ENET_MII
> "int1" is for queue 1
> "int2" is for queue 2
>
> For imx6sx, "int0" handles all 3 queues and ENET_MII
>
> And of course, the "pps" interrupt is for the PTP_CLOCK_PPS interrupts
> This will help document what each interrupt does.
>
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

I had updated my email address in MAINTAINERS for years.  Please use that one.

> v4: add blank, ie s/"int0","pps"/"int0", "pps"/ as suggested by Andrew Lunn
> ---
>  Documentation/devicetree/bindings/net/fsl-fec.txt | 13 +++++++++++++

Please have bindings go with net driver instead of DTS changes.

Shawn

>  arch/arm/boot/dts/imx6qdl.dtsi                    |  1 +
>  arch/arm/boot/dts/imx6sx.dtsi                     |  2 ++
>  arch/arm/boot/dts/imx6ul.dtsi                     |  2 ++
>  arch/arm/boot/dts/imx7d.dtsi                      |  6 ++++--
>  arch/arm/boot/dts/imx7s.dtsi                      |  6 ++++--
>  6 files changed, 26 insertions(+), 4 deletions(-)