diff mbox series

[net-next,v2,1/2] net: macb: Enable PTP unicast

Message ID 20230321124005.7014-2-harini.katakam@amd.com
State New
Headers
Series Macb PTP minor updates |

Commit Message

Harini Katakam March 21, 2023, 12:40 p.m. UTC 
  From: Harini Katakam <harini.katakam@xilinx.com>

Enable transmission and reception of PTP unicast packets by
updating PTP unicast config bit and setting current HW mac
address as allowed address in PTP unicast filter registers.

Signed-off-by: Harini Katakam <harini.katakam@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
---
v2:
Handle operation using a single write as suggested by Cladiu

 drivers/net/ethernet/cadence/macb.h      |  4 ++++
 drivers/net/ethernet/cadence/macb_main.c | 15 +++++++++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

Comments

Jakub Kicinski March 23, 2023, 5:24 a.m. UTC | #1 
On Tue, 21 Mar 2023 18:10:04 +0530 Harini Katakam wrote:
> +#ifdef CONFIG_MACB_USE_HWSTAMP
> +	if (gem_has_ptp(bp)) {
> +		gem_writel(bp, RXPTPUNI, bottom);
> +		gem_writel(bp, TXPTPUNI, bottom);
> +	}
> +#endif

You can use the same IS_ENABLED() trick here as you used in the if ()
below, to avoid the direct preprocessor use. In fact why not just
add the IS_ENABLED(CONFIG_MACB_USE_HWSTAMP) to the condition inside
gem_has_ptp() ? It looks like all callers want that extra check.

> +	if (IS_ENABLED(CONFIG_MACB_USE_HWSTAMP) && gem_has_ptp(bp))
> +		ctrl |= MACB_BIT(PTPUNI);
Harini Katakam March 23, 2023, 7:01 a.m. UTC | #2 
Hi Jakub,

> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Thursday, March 23, 2023 10:54 AM
> To: Katakam, Harini <harini.katakam@amd.com>
> Cc: nicolas.ferre@microchip.com; davem@davemloft.net;
> richardcochran@gmail.com; claudiu.beznea@microchip.com;
> andrei.pistirica@microchip.com; edumazet@google.com;
> pabeni@redhat.com; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> Simek, Michal <michal.simek@amd.com>; harinikatakamlinux@gmail.com
> Subject: Re: [PATCH net-next v2 1/2] net: macb: Enable PTP unicast
> 
> On Tue, 21 Mar 2023 18:10:04 +0530 Harini Katakam wrote:
> > +#ifdef CONFIG_MACB_USE_HWSTAMP
> > +	if (gem_has_ptp(bp)) {
> > +		gem_writel(bp, RXPTPUNI, bottom);
> > +		gem_writel(bp, TXPTPUNI, bottom);
> > +	}
> > +#endif
> 
> You can use the same IS_ENABLED() trick here as you used in the if ()
> below, to avoid the direct preprocessor use. In fact why not just
> add the IS_ENABLED(CONFIG_MACB_USE_HWSTAMP) to the condition inside
> gem_has_ptp() ? It looks like all callers want that extra check.

Thanks for the suggestion.
I believe gem_has_ptp was originally defined for checking device
capability (Atmel/SiFive/AMD) and CONFIG_MACB_USE_HWSTAMP for the
kernel selection but I agree that there is currently no usecase for a 
scenario where gem_has_ptp is TRUE and MACB_USE_HWSTAMP is false. If
hear are no objections, I'll include this check inside gem_has_ptp and send v3.

Regards,
Harini
diff mbox series

Patch

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 9c410f93a103..1aa578c1ca4a 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -95,6 +95,8 @@ 
 #define GEM_SA4B		0x00A0 /* Specific4 Bottom */
 #define GEM_SA4T		0x00A4 /* Specific4 Top */
 #define GEM_WOL			0x00b8 /* Wake on LAN */
+#define GEM_RXPTPUNI		0x00D4 /* PTP RX Unicast address */
+#define GEM_TXPTPUNI		0x00D8 /* PTP TX Unicast address */
 #define GEM_EFTSH		0x00e8 /* PTP Event Frame Transmitted Seconds Register 47:32 */
 #define GEM_EFRSH		0x00ec /* PTP Event Frame Received Seconds Register 47:32 */
 #define GEM_PEFTSH		0x00f0 /* PTP Peer Event Frame Transmitted Seconds Register 47:32 */
@@ -245,6 +247,8 @@ 
 #define MACB_TZQ_OFFSET		12 /* Transmit zero quantum pause frame */
 #define MACB_TZQ_SIZE		1
 #define MACB_SRTSM_OFFSET	15 /* Store Receive Timestamp to Memory */
+#define MACB_PTPUNI_OFFSET	20 /* PTP Unicast packet enable */
+#define MACB_PTPUNI_SIZE	1
 #define MACB_OSSMODE_OFFSET	24 /* Enable One Step Synchro Mode */
 #define MACB_OSSMODE_SIZE	1
 #define MACB_MIIONRGMII_OFFSET	28 /* MII Usage on RGMII Interface */
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 51c9fd6f68a4..4c2c82573399 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -288,6 +288,13 @@  static void macb_set_hwaddr(struct macb *bp)
 	top = cpu_to_le16(*((u16 *)(bp->dev->dev_addr + 4)));
 	macb_or_gem_writel(bp, SA1T, top);
 
+#ifdef CONFIG_MACB_USE_HWSTAMP
+	if (gem_has_ptp(bp)) {
+		gem_writel(bp, RXPTPUNI, bottom);
+		gem_writel(bp, TXPTPUNI, bottom);
+	}
+#endif
+
 	/* Clear unused address register sets */
 	macb_or_gem_writel(bp, SA2B, 0);
 	macb_or_gem_writel(bp, SA2T, 0);
@@ -721,8 +728,12 @@  static void macb_mac_link_up(struct phylink_config *config,
 
 	spin_unlock_irqrestore(&bp->lock, flags);
 
-	/* Enable Rx and Tx */
-	macb_writel(bp, NCR, macb_readl(bp, NCR) | MACB_BIT(RE) | MACB_BIT(TE));
+	/* Enable Rx and Tx; Enable PTP unicast */
+	ctrl = macb_readl(bp, NCR);
+	if (IS_ENABLED(CONFIG_MACB_USE_HWSTAMP) && gem_has_ptp(bp))
+		ctrl |= MACB_BIT(PTPUNI);
+
+	macb_writel(bp, NCR, ctrl | MACB_BIT(RE) | MACB_BIT(TE));
 
 	netif_tx_wake_all_queues(ndev);
 }