From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net [217.70.183.200]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 976DE2C694; Mon, 15 Jan 2024 10:06:39 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=bootlin.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b="RfikiN2L" Received: by mail.gandi.net (Postfix) with ESMTPSA id 84DC72000B; Mon, 15 Jan 2024 10:06:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=gm1; t=1705313197; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=2sURSIiRwE+VMw4pqfsGC2UVhFnlMv9fsKlnWFF3DNM=; b=RfikiN2LGt9usO6Kr2immj7tGr4jK+B73EUyXm5DpAJY2tcHu0kTW6gdS3NKOXOnAYahTN jxcMl0kTA7QOAKNYHdoL8Rn+xAROzgs7NueK6Cvk/ur0dPi0PtVYPBo4x9uyjGawo+/sS8 FuL0K/EDJzRjU8g3FnxzhjdEYPFRtKBHsiorAFrybGqWsXTsDi4HLwXUt0FnUDpK8fA65C O/Ofp1bCZtQEXgOAgIop0bEr0BAsAiFeyMDtpcAMsYj2+L44yeTmlzsUSzzR16i5vLjDhM bKuYavKnHHtIXCp1AzmNNR/1wdzI4YVxfMjo2PsfHxGHKhRIR+o+AfHE6qkG7A== Date: Mon, 15 Jan 2024 11:06:35 +0100 From: Maxime Chevallier To: Horatiu Vultur Cc: , , , , , , , , , , , Subject: Re: [PATCH net 1/2] net: micrel: Fix PTP frame parsing for lan8814 Message-ID: <20240115110635.60b1884c@device-28.home> In-Reply-To: <20240113131521.1051921-2-horatiu.vultur@microchip.com> References: <20240113131521.1051921-1-horatiu.vultur@microchip.com> <20240113131521.1051921-2-horatiu.vultur@microchip.com> Organization: Bootlin X-Mailer: Claws Mail 4.2.0 (GTK 3.24.39; x86_64-redhat-linux-gnu) Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-GND-Sasl: maxime.chevallier@bootlin.com Hello Horatiu, On Sat, 13 Jan 2024 14:15:20 +0100 Horatiu Vultur wrote: > The HW has the capability to check each frame if it is a PTP frame, > which domain it is, which ptp frame type it is, different ip address in > the frame. And if one of these checks fail then the frame is not > timestamp. Most of these checks were disabled except checking the field > minorVersionPTP inside the PTP header. Meaning that once a partner sends > a frame compliant to 8021AS which has minorVersionPTP set to 1, then the > frame was not timestamp because the HW expected by default a value of 0 > in minorVersionPTP. This is exactly the same issue as on lan8841. > Fix this issue by removing this check so the userspace can decide on this. > > Fixes: ece19502834d ("net: phy: micrel: 1588 support for LAN8814 phy") > Signed-off-by: Horatiu Vultur > --- > drivers/net/phy/micrel.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c > index bf4053431dcb3..1752eaeadc42e 100644 > --- a/drivers/net/phy/micrel.c > +++ b/drivers/net/phy/micrel.c > @@ -120,6 +120,9 @@ > */ > #define LAN8814_1PPM_FORMAT 17179 > > +#define PTP_RX_VERSION 0x0248 > +#define PTP_TX_VERSION 0x0288 > + > #define PTP_RX_MOD 0x024F > #define PTP_RX_MOD_BAD_UDPV4_CHKSUM_FORCE_FCS_DIS_ BIT(3) > #define PTP_RX_TIMESTAMP_EN 0x024D > @@ -3150,6 +3153,10 @@ static void lan8814_ptp_init(struct phy_device *phydev) > lanphy_write_page_reg(phydev, 5, PTP_TX_PARSE_IP_ADDR_EN, 0); > lanphy_write_page_reg(phydev, 5, PTP_RX_PARSE_IP_ADDR_EN, 0); > > + /* Disable checking for minorVersionPTP field */ > + lanphy_write_page_reg(phydev, 5, PTP_RX_VERSION, 0xff00); > + lanphy_write_page_reg(phydev, 5, PTP_TX_VERSION, 0xff00); Small nit: This looks a bit like magic values, from the datasheet I understand the upper byte is the max supported version and the lower byte is the min supported version, maybe this could be wrapped in macros ? > + > skb_queue_head_init(&ptp_priv->tx_queue); > skb_queue_head_init(&ptp_priv->rx_queue); > INIT_LIST_HEAD(&ptp_priv->rx_ts_list); Besides that, Reviewed-by: Maxime Chevallier Thanks, Maxime