Re: [RFC v2 PATCH 06/10] net: ti: prueth: Adds HW timestamping support for PTP using PRU-ICSS IEP module
From: Simon Horman <horms@kernel.org>
Date: 2025-01-31 10:34:01
Also in:
linux-arm-kernel, linux-devicetree, linux-omap, lkml
On Fri, Jan 24, 2025 at 07:10:52PM +0530, Basharath Hussain Khaja wrote:
From: Roger Quadros <redacted> PRU-ICSS IEP module, which is capable of timestamping RX and TX packets at HW level, is used for time synchronization by PTP4L. This change includes interaction between firmware and user space application (ptp4l) with required packet timestamps. The driver initializes the PRU firmware with appropriate mode and configuration flags. Firmware updates local registers with the flags set by driver and uses for further operation. RX SOF timestamp comes along with packet and firmware will rise interrupt with TX SOF timestamp after pushing the packet on to the wire. IEP driver is available in upstream and we are reusing for hardware configuration for ICSSM as well. On top of that we have extended it with the changes for AM57xx SoC. Extended ethtool for reading HW timestamping capability of the PRU interfaces. Currently ordinary clock (OC) configuration has been validated with Linux ptp4l. Signed-off-by: Roger Quadros <redacted> Signed-off-by: Andrew F. Davis <redacted> Signed-off-by: Parvathi Pudi <parvathi@couthit.com> Signed-off-by: Basharath Hussain Khaja <redacted>
...
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/ti/icssm/icssm_prueth.c b/drivers/net/ethernet/ti/icssm/icssm_prueth.c
...
quoted hunk ↗ jump to hunk
@@ -682,9 +899,22 @@ int icssm_emac_rx_packet(struct prueth_emac *emac, u16 *bd_rd_ptr, src_addr += actual_pkt_len; } + if (pkt_info->timestamp) { + src_addr = (void *)roundup((uintptr_t)src_addr, + ICSS_BLOCK_SIZE);
Can PTR_ALIGN() be used here?
+ dst_addr = &ts;
+ memcpy(dst_addr, src_addr, sizeof(ts));
+ }
+
if (!pkt_info->sv_frame) {
skb_put(skb, actual_pkt_len);
+ if (icssm_prueth_ptp_rx_ts_is_enabled(emac) &&
+ pkt_info->timestamp) {
+ ssh = skb_hwtstamps(skb);
+ memset(ssh, 0, sizeof(*ssh));
+ ssh->hwtstamp = ns_to_ktime(ts);
+ }
/* send packet up the stack */
skb->protocol = eth_type_trans(skb, ndev);
local_bh_disable();
The code preceding the hunk below is:
static int icssm_emac_request_irqs(struct prueth_emac *emac)
{
struct net_device *ndev = emac->ndev;
int ret;
ret = request_threaded_irq(emac->rx_irq, NULL, icssm_emac_rx_thread,
IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
ndev->name, ndev);
if (ret) {
netdev_err(ndev, "unable to request RX IRQ\n");
return ret;
}
quoted hunk ↗ jump to hunk
@@ -855,9 +1085,64 @@ static int icssm_emac_request_irqs(struct prueth_emac *emac) return ret; } + if (emac->emac_ptp_tx_irq) { + ret = request_threaded_irq(emac->emac_ptp_tx_irq, + icssm_prueth_ptp_tx_irq_handle, + icssm_prueth_ptp_tx_irq_work, + IRQF_TRIGGER_HIGH | IRQF_ONESHOT, + ndev->name, ndev); + if (ret) { + netdev_err(ndev, "unable to request PTP TX IRQ\n"); + free_irq(emac->rx_irq, ndev); + free_irq(emac->tx_irq, ndev);
This seems somewhat asymmetric. This function does request emac->rx_irq but not emac->tx_irq. So I don't think it is appropriate to free emac->tx_irq here. Also, I would suggest using a goto label for unwind here.
+ } + } + return ret; }
...