netdev - Re: [PATCH 3/3] net: stmmac: Support coarse mode through ioctl

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] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 26 May 2020 20:55:09 -0700
From:   Richard Cochran <richardcochran@...il.com>
To:     Olivier Dautricourt <olivier.dautricourt@...lia.com>
Cc:     Giuseppe Cavallaro <peppe.cavallaro@...com>,
        Alexandre Torgue <alexandre.torgue@...com>,
        Jose Abreu <joabreu@...opsys.com>,
        "David S . Miller" <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH 3/3] net: stmmac: Support coarse mode through ioctl

On Thu, May 14, 2020 at 12:28:08PM +0200, Olivier Dautricourt wrote:
> The required time adjustment is written in the Timestamp Update registers
> while the Sub-second increment register is programmed with the period
> of the clock, which is the precision of our correction.

I don't see in this patch where the "required time adjustment is
written in the Timestamp Update registers".

What am I missing?

Thanks,
Richard


> 
> The fine adjutment mode is always the default behavior of the driver.
> One should use the HWTSAMP_FLAG_ADJ_COARSE flag while calling
> SIOCGHWTSTAMP ioctl to enable coarse mode for stmmac driver.
> 
> Signed-off-by: Olivier Dautricourt <olivier.dautricourt@...lia.com>
> ---
>  .../net/ethernet/stmicro/stmmac/stmmac_main.c   | 17 +++++++++++++----
>  .../net/ethernet/stmicro/stmmac/stmmac_ptp.c    |  3 +++
>  2 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index c39fafe69b12..f46503b086f4 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -541,9 +541,12 @@ static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
>  	netdev_dbg(priv->dev, "%s config flags:0x%x, tx_type:0x%x, rx_filter:0x%x\n",
>  		   __func__, config.flags, config.tx_type, config.rx_filter);
>  
> -	/* reserved for future extensions */
> -	if (config.flags)
> -		return -EINVAL;
> +	if (config.flags != HWTSTAMP_FLAGS_ADJ_COARSE) {
> +		/* Defaulting to fine adjustment for compatibility */
> +		netdev_dbg(priv->dev, "%s defaulting to fine adjustment mode\n",
> +			   __func__);
> +		config.flags = HWTSTAMP_FLAGS_ADJ_FINE;
> +	}
>  
>  	if (config.tx_type != HWTSTAMP_TX_OFF &&
>  	    config.tx_type != HWTSTAMP_TX_ON)
> @@ -689,10 +692,16 @@ static int stmmac_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
>  		stmmac_set_hw_tstamping(priv, priv->ptpaddr, 0);
>  	else {
>  		stmmac_get_hw_tstamping(priv, priv->ptpaddr, &value);
> -		value |= (PTP_TCR_TSENA | PTP_TCR_TSCFUPDT | PTP_TCR_TSCTRLSSR |
> +		value |= (PTP_TCR_TSENA |  PTP_TCR_TSCTRLSSR |
>  			 tstamp_all | ptp_v2 | ptp_over_ethernet |
>  			 ptp_over_ipv6_udp | ptp_over_ipv4_udp | ts_event_en |
>  			 ts_master_en | snap_type_sel);
> +
> +		if (config.flags == HWTSTAMP_FLAGS_ADJ_FINE)
> +			value |= PTP_TCR_TSCFUPDT;
> +		else
> +			value &= ~PTP_TCR_TSCFUPDT;
> +
>  		stmmac_set_hw_tstamping(priv, priv->ptpaddr, value);
>  
>  		/* program Sub Second Increment reg */
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> index 920f0f3ebbca..7fb318441015 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> @@ -27,6 +27,9 @@ static int stmmac_adjust_freq(struct ptp_clock_info *ptp, s32 ppb)
>  	int neg_adj = 0;
>  	u64 adj;
>  
> +	if (priv->tstamp_config.flags != HWTSTAMP_FLAGS_ADJ_FINE)
> +		return -EPERM;
> +
>  	if (ppb < 0) {
>  		neg_adj = 1;
>  		ppb = -ppb;
> -- 
> 2.17.1
>