| Message ID | 20230321124005.7014-2-harini.katakam@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Macb PTP minor updates | expand |
| Context | Check | Description |
|---|---|---|
| netdev/series_format | success | Posting correctly formatted |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 39 this patch: 39 |
| netdev/cc_maintainers | success | CCed 8 of 8 maintainers |
| netdev/build_clang | success | Errors and warnings before: 18 this patch: 18 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/deprecated_api | success | None detected |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 39 this patch: 39 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 43 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
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);
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 --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); }