Re: [PATCH net-next 5/8] net: dsa: microchip: Add KSZ8463 tail tag handling

[Bastien Curutchet <bastien.curutchet@bootlin.com>]

Hi Simon,

On 1/20/26 6:17 PM, Simon Horman wrote:
> On Thu, Jan 15, 2026 at 04:57:04PM +0100, Bastien Curutchet (Schneider Electric) wrote:
>> KSZ8463 uses the KSZ9893 DSA TAG driver. However, the KSZ8463 doesn't
>> use the tail tag to convey timestamps to the host as KSZ9893 does. It
>> uses the reserved fields in the PTP header instead.
>>
>> Add a KSZ8463-specifig DSA_TAG driver to handle KSZ8463 timestamps.
>> There is no information in the tail tag to distinguish PTP packets from
>> others so use the ptp_classify_raw() helper to find the PTP packets and
>> extract the timestamp from their PTP headers.
>>
>> Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
> 
> ...
> 
>> diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
>> index 9170a0148cc43b4213ec4bd8e81d338589671f23..635679402f8a96b29536a91988346a8825bae976 100644
>> --- a/net/dsa/tag_ksz.c
>> +++ b/net/dsa/tag_ksz.c
>> @@ -16,6 +16,7 @@
>>   #define KSZ9477_NAME "ksz9477"
>>   #define KSZ9893_NAME "ksz9893"
>>   #define LAN937X_NAME "lan937x"
>> +#define KSZ8463_NAME "ksz8463"
>>   
>>   /* Typically only one byte is used for tail tag. */
>>   #define KSZ_PTP_TAG_LEN			4
>> @@ -383,6 +384,108 @@ static const struct dsa_device_ops ksz9893_netdev_ops = {
>>   DSA_TAG_DRIVER(ksz9893_netdev_ops);
>>   MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ9893, KSZ9893_NAME);
>>   
>> +#define KSZ8463_TAIL_TAG_PRIO		GENMASK(4, 3)
>> +#define KSZ8463_TAIL_TAG_EG_PORT_M	GENMASK(2, 0)
>> +
>> +static void ksz8463_xmit_timestamp(struct dsa_port *dp, struct sk_buff *skb)
>> +{
>> +	struct ksz_tagger_private *priv;
>> +	struct ptp_header *ptp_hdr;
>> +	unsigned int ptp_type;
>> +	u32 tstamp_raw = 0;
>> +	s64 correction;
>> +
>> +	priv = ksz_tagger_private(dp->ds);
>> +
>> +	if (!test_bit(KSZ_HWTS_EN, &priv->state))
>> +		return;
>> +
>> +	if (!KSZ_SKB_CB(skb)->update_correction)
>> +		return;
>> +
>> +	ptp_type = KSZ_SKB_CB(skb)->ptp_type;
>> +	ptp_hdr = ptp_parse_header(skb, ptp_type);
>> +	if (!ptp_hdr)
>> +		return;
>> +
>> +	correction = (s64)get_unaligned_be64(&ptp_hdr->correction);
>> +
>> +	if (correction < 0) {
>> +		struct timespec64 ts;
>> +
>> +		ts = ns_to_timespec64(-correction >> 16);
>> +		tstamp_raw = ((ts.tv_sec & 3) << 30) | ts.tv_nsec;
>> +
>> +		ptp_hdr->reserved2 = tstamp_raw;
> 
> I think that you need to assign a be32 rather than a u32 to reserved2.
> 

Indeed. By testing this back, I realize that this is only needed by the 
one-step mode. This series only adds support for two-step so I'll just 
remove this part.


Best regards,
Bastien