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

From: Andy Duan <hidden>
Date: 2017-10-09 02:55:40

From: Troy Kisky <redacted> Sent: Tuesday, October 03, 2017 8:05 AM

To: shawn.guo@linaro.org; Andy Duan <redacted>;
netdev@vger.kernel.org; davem@davemloft.net
Cc: Fabio Estevam <redacted>; lznuaa@gmail.com; Troy Kisky
[off-list ref]
Subject: [PATCH net v1 2/2] net: fec: Let fec_ptp have its own interrupt
routine

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

---
If this patch is taken before the corresponding dtb file patch than time
stamping will be momentarily broken.
---

The two patches for PPS seems

quoted hunk ↗ jump to hunk

drivers/net/ethernet/freescale/fec.h      |  1 -
drivers/net/ethernet/freescale/fec_main.c | 15 +++---
drivers/net/ethernet/freescale/fec_ptp.c  | 77 ++++++++++++++++++--------
-----
3 files changed, 52 insertions(+), 41 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h

b/drivers/net/ethernet/freescale/fec.h
index ede1876a9a19..782509041102 100644

--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h

@@ -587,7 +587,6 @@ 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..80fe04165ba0 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,7 @@ fec_probe(struct platform_device *pdev)

	struct device_node *np = pdev->dev.of_node, *phy_node;
	int num_tx_qs;
	int num_rx_qs;
+	int ptp_irq, j;

	fec_enet_get_queue_num(pdev, &num_tx_qs, &num_rx_qs);

@@ -3472,20 +3469,24 @@ fec_probe(struct platform_device *pdev)

	if (ret)
		goto failed_init;

-	for (i = 0; i < FEC_IRQ_NUM; i++) {
-		irq = platform_get_irq(pdev, i);
+	ptp_irq = platform_get_irq_byname(pdev, "ptp");

Here, interrupt-names property is required properties, it should update binding doc.
Also need to handle the return value checking.

+	i = j = 0;
+	while (i < FEC_IRQ_NUM) {
+		irq = platform_get_irq(pdev, j++);
		if (irq < 0) {
			if (i)
				break;
			ret = irq;
			goto failed_irq;
		}
+		if (irq == ptp_irq)
+			continue;
		ret = devm_request_irq(&pdev->dev, irq, fec_enet_interrupt,
				       0, pdev->name, ndev);
		if (ret)
			goto failed_irq;

-		fep->irq[i] = irq;
+		fep->irq[i++] = irq;

I think it is not necessary to split pps interrupt handler.
To support pps interrupt handler,  just change the maximum interrupt number FEC_IRQ_NUM to 4 that minimize the code change.

quoted hunk ↗ jump to hunk

	}

	init_completion(&fep->mdio_done);

diff --git a/drivers/net/ethernet/freescale/fec_ptp.c

b/drivers/net/ethernet/freescale/fec_ptp.c
index 6ebad3fac81d..20e01a3ca453 100644

--- a/drivers/net/ethernet/freescale/fec_ptp.c
+++ b/drivers/net/ethernet/freescale/fec_ptp.c

@@ -549,6 +549,38 @@ 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_ptp_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-

quoted

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

@@ -562,6 +594,8 @@ void fec_ptp_init(struct platform_device *pdev)  {

	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 +621,16 @@
void fec_ptp_init(struct platform_device *pdev)

	INIT_DELAYED_WORK(&fep->time_keep, fec_time_keep);

+	irq = platform_get_irq_byname(pdev, "ptp");
+	if (irq < 0)
+		return;

If dts don't set interrupt name, here break ptp function. Suppose PPS don't break ptp.

+	ret = devm_request_irq(&pdev->dev, irq, fec_ptp_interrupt,
+			       0, pdev->name, ndev);
+	if (ret < 0) {
+		dev_warn(&pdev->dev, "request for ptp irq failed(%d)\n",
ret);
+		return;

Ditto...

quoted hunk ↗ jump to hunk

+	}
+
	fep->ptp_clock = ptp_clock_register(&fep->ptp_caps, &pdev->dev);
	if (IS_ERR(fep->ptp_clock)) {
		fep->ptp_clock = NULL;

@@ -605,36 +649,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-

quoted

reload_period) & fep->cc.mask;

-
-		event.type = PTP_CLOCK_PPS;
-		ptp_clock_event(fep->ptp_clock, &event);
-		return 1;
-	}
-
-	return 0;
-}
--
2.11.0

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help