netdev - Re: [PATCH net-next v3 5/9] net: phy: micrel: add HW timestamp configuration reporting

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]

Message-ID: <5a1cab05-bc20-43ae-b5e7-3fd22b7feed6@linux.dev>
Date: Wed, 19 Nov 2025 15:10:27 +0000
From: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
To: Kory Maincent <kory.maincent@...tlin.com>
Cc: Andrew Lunn <andrew+netdev@...n.ch>,
 Florian Fainelli <florian.fainelli@...adcom.com>,
 Russell King <linux@...linux.org.uk>, Heiner Kallweit
 <hkallweit1@...il.com>, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Andrei Botila <andrei.botila@....nxp.com>,
 Richard Cochran <richardcochran@...il.com>, Andrew Lunn <andrew@...n.ch>,
 Simon Horman <horms@...nel.org>, Vladimir Oltean <vladimir.oltean@....com>,
 Jacob Keller <jacob.e.keller@...el.com>,
 bcm-kernel-feedback-list@...adcom.com, netdev@...r.kernel.org
Subject: Re: [PATCH net-next v3 5/9] net: phy: micrel: add HW timestamp
 configuration reporting

On 19/11/2025 14:44, Kory Maincent wrote:
> On Wed, 19 Nov 2025 12:47:21 +0000
> Vadim Fedorenko <vadim.fedorenko@...ux.dev> wrote:
> 
>> The driver stores HW timestamping configuration and can technically
>> report it. Add callback to do it.
>>
>> Signed-off-by: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
>> ---
>>   drivers/net/phy/micrel.c | 27 +++++++++++++++++++++++++++
>>   1 file changed, 27 insertions(+)
>>
>> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
>> index 05de68b9f719..b149d539aec3 100644
>> --- a/drivers/net/phy/micrel.c
>> +++ b/drivers/net/phy/micrel.c
>> @@ -3147,6 +3147,18 @@ static void lan8814_flush_fifo(struct phy_device
>> *phydev, bool egress) lanphy_read_page_reg(phydev, LAN8814_PAGE_PORT_REGS,
>> PTP_TSU_INT_STS); }
>>   
>> +static int lan8814_hwtstamp_get(struct mii_timestamper *mii_ts,
>> +				struct kernel_hwtstamp_config *config)
>> +{
>> +	struct kszphy_ptp_priv *ptp_priv =
>> +			  container_of(mii_ts, struct kszphy_ptp_priv,
>> mii_ts); +
>> +	config->tx_type = ptp_priv->hwts_tx_type;
>> +	config->rx_filter = ptp_priv->rx_filter;
>> +
>> +	return 0;
>> +}
>> +
>>   static int lan8814_hwtstamp_set(struct mii_timestamper *mii_ts,
>>   				struct kernel_hwtstamp_config *config,
>>   				struct netlink_ext_ack *extack)
>> @@ -4390,6 +4402,7 @@ static void lan8814_ptp_init(struct phy_device *phydev)
>>   	ptp_priv->mii_ts.rxtstamp = lan8814_rxtstamp;
>>   	ptp_priv->mii_ts.txtstamp = lan8814_txtstamp;
>>   	ptp_priv->mii_ts.hwtstamp_set = lan8814_hwtstamp_set;
>> +	ptp_priv->mii_ts.hwtstamp_get = lan8814_hwtstamp_get;
>>   	ptp_priv->mii_ts.ts_info  = lan8814_ts_info;
>>   
>>   	phydev->mii_ts = &ptp_priv->mii_ts;
>> @@ -5042,6 +5055,19 @@ static void lan8841_ptp_enable_processing(struct
>> kszphy_ptp_priv *ptp_priv, #define LAN8841_PTP_TX_TIMESTAMP_EN
>> 443 #define LAN8841_PTP_TX_MOD			445
>>   
>> +static int lan8841_hwtstamp_get(struct mii_timestamper *mii_ts,
>> +				struct kernel_hwtstamp_config *config)
>> +{
>> +	struct kszphy_ptp_priv *ptp_priv;
>> +
>> +	ptp_priv = container_of(mii_ts, struct kszphy_ptp_priv, mii_ts);
>> +
>> +	config->tx_type = ptp_priv->hwts_tx_type;
>> +	config->rx_filter = ptp_priv->rx_filter;
> 
> Something related to this patch, it seems there is an issue in the set
> callback:
> https://elixir.bootlin.com/linux/v6.18-rc6/source/drivers/net/phy/micrel.c#L3056
> 
> The priv->rx_filter and priv->hwts_tx_type are set before the switch condition.
> The hwtstamp_get can then report something that is not supported.
> Also HWTSTAMP_TX_ONESTEP_P2P is not managed by the driver but not returning
> -ERANGE either so if we set this config the hwtstamp_get will report something
> wrong as not supported.
> I think you will need to add a new patch here to fix the hwtstamp_set ops.

I agree, that there is a problem in the flow, but such change is out of
scope of this patch set. I'm going to provide some logic improvements on
per-driver basis as follow up work.

> Maybe we should update net_hwtstamp_validate to check on the capabilities
> reported by ts_info but that is a bigger change.

In this case we have to introduce validation callback and implement it
in drivers. Some drivers do downgrade filter values if provided value is
not in the list of what was provided in ethtool::ts_info. And we have to
keep this logic as otherwise it may be considered as API breakage.

> 
>> +
>> +	return 0;
>> +}
>> +
>>   static int lan8841_hwtstamp_set(struct mii_timestamper *mii_ts,
>>   				struct kernel_hwtstamp_config *config,
>>   				struct netlink_ext_ack *extack)
>> @@ -5925,6 +5951,7 @@ static int lan8841_probe(struct phy_device *phydev)
>>   	ptp_priv->mii_ts.rxtstamp = lan8841_rxtstamp;
>>   	ptp_priv->mii_ts.txtstamp = lan8814_txtstamp;
>>   	ptp_priv->mii_ts.hwtstamp_set = lan8841_hwtstamp_set;
>> +	ptp_priv->mii_ts.hwtstamp_get = lan8841_hwtstamp_get;
>>   	ptp_priv->mii_ts.ts_info = lan8841_ts_info;
>>   
>>   	phydev->mii_ts = &ptp_priv->mii_ts;
> 
> 
>