linux-kernel - Re: [RFC v2 PATCH 06/10] net: ti: prueth: Adds HW timestamping support for PTP using PRU-ICSS IEP module

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1980208448.516541.1738758271732.JavaMail.zimbra@couthit.local>
Date: Wed, 5 Feb 2025 17:54:31 +0530 (IST)
From: Basharath Hussain Khaja <basharath@...thit.com>
To: horms <horms@...nel.org>
Cc: basharath <basharath@...thit.com>, danishanwar <danishanwar@...com>, 
	rogerq <rogerq@...nel.org>, andrew+netdev <andrew+netdev@...n.ch>, 
	davem <davem@...emloft.net>, edumazet <edumazet@...gle.com>, 
	kuba <kuba@...nel.org>, pabeni <pabeni@...hat.com>, 
	Rob Herring <robh@...nel.org>, krzk+dt <krzk+dt@...nel.org>, 
	conor+dt <conor+dt@...nel.org>, nm <nm@...com>, 
	ssantosh <ssantosh@...nel.org>, tony <tony@...mide.com>, 
	richardcochran <richardcochran@...il.com>, 
	parvathi <parvathi@...thit.com>, schnelle <schnelle@...ux.ibm.com>, 
	rdunlap <rdunlap@...radead.org>, diogo ivo <diogo.ivo@...mens.com>, 
	m-karicheri2 <m-karicheri2@...com>, 
	jacob e keller <jacob.e.keller@...el.com>, 
	m-malladi <m-malladi@...com>, 
	javier carrasco cruz <javier.carrasco.cruz@...il.com>, 
	afd <afd@...com>, s-anna <s-anna@...com>, 
	linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>, 
	netdev <netdev@...r.kernel.org>, 
	devicetree <devicetree@...r.kernel.org>, 
	linux-kernel <linux-kernel@...r.kernel.org>, 
	linux-omap <linux-omap@...r.kernel.org>, 
	pratheesh <pratheesh@...com>, prajith <prajith@...com>, 
	vigneshr <vigneshr@...com>, praneeth <praneeth@...com>, 
	srk <srk@...com>, rogerq <rogerq@...com>, 
	krishna <krishna@...thit.com>, pmohan <pmohan@...thit.com>, 
	mohan <mohan@...thit.com>
Subject: Re: [RFC v2 PATCH 06/10] net: ti: prueth: Adds HW timestamping
 support for PTP using PRU-ICSS IEP module


> On Fri, Jan 24, 2025 at 07:10:52PM +0530, Basharath Hussain Khaja wrote:
>> From: Roger Quadros <rogerq@...com>
>> 
>> 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 <rogerq@...com>
>> Signed-off-by: Andrew F. Davis <afd@...com>
>> Signed-off-by: Parvathi Pudi <parvathi@...thit.com>
>> Signed-off-by: Basharath Hussain Khaja <basharath@...thit.com>
> 
> ...
> 
>> diff --git a/drivers/net/ethernet/ti/icssm/icssm_prueth.c
>> b/drivers/net/ethernet/ti/icssm/icssm_prueth.c
> 
> ...
> 
>> @@ -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?
> 

We are making sure Both roundup() and PTR_ALIGN() will point to 
the same address location. We will change this in the next version.

>> +		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;
>	}
> 
>> @@ -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.
> 

Sure. We will clean this in the next version.

>> +		}
>> +	}
>> +
>>  	return ret;
>>  }
>>  
> 
> ...

Thanks & Best Regards,
Basharath