Re: [RESEND PATCH 1/2] net: macb: Enable PTP unicast

[<Claudiu.Beznea@microchip.com>]

On 16.08.2022 14:54, Harini Katakam wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> 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>
> ---
> Added check for gem_has_ptp as per Claudiu's comments.
> 
>  drivers/net/ethernet/cadence/macb.h      |  4 ++++
>  drivers/net/ethernet/cadence/macb_main.c | 13 ++++++++++++-
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> 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 494fe961a49d..4699699a1593 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -287,6 +287,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);
> @@ -720,7 +727,11 @@ static void macb_mac_link_up(struct phylink_config *config,
> 
>         spin_unlock_irqrestore(&bp->lock, flags);
> 
> -       /* Enable Rx and Tx */
> +       /* Enable Rx and Tx; Enable PTP unicast */
> +#ifdef CONFIG_MACB_USE_HWSTAMP
> +       if (gem_has_ptp(bp))
> +               macb_writel(bp, NCR, macb_readl(bp, NCR) | MACB_BIT(PTPUNI));
> +#endif
>         macb_writel(bp, NCR, macb_readl(bp, NCR) | MACB_BIT(RE) | MACB_BIT(TE));

Can you use a single read and single write here? something like:

	/* 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));

Thank you,
Claudiu Beznea

> 
>         netif_tx_wake_all_queues(ndev);
> --
> 2.17.1
>