Re: [RFC Patch net-next v2 7/8] net: dsa: microchip: add the transmission tstamp logic

[Arun.Ramadoss]

Hi Vladimir,
Thanks for the detailed explanation of race condition.

On Tue, 2022-11-22 at 00:51 +0200, Vladimir Oltean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> Hi Arun,
> 
> On Mon, Nov 21, 2022 at 09:11:49PM +0530, Arun Ramadoss wrote:
> > This patch adds the routines for transmission of ptp packets. When
> > the
> > ptp packets(sync, pdelay_req, pdelay_rsp) to be transmitted, the
> > skb is
> > copied to global skb through port_txtstamp ioctl.
> > After the packet is transmitted, ISR is triggered. The time at
> > which
> > packet transmitted is recorded to separate register available for
> > each
> > message. This value is reconstructed to absolute time and posted to
> > the
> > user application through skb complete.
> 
> "skb complete" is not a thing. "socket error queue" is.

I will update as socket error queue.

> 
> > 
> > 
> diff --git a/drivers/net/dsa/microchip/ksz9477_ptp.c
> b/drivers/net/dsa/microchip/ksz9477_ptp.c
> index c646689cb71e..bc3f0283859a 100644
> --- a/drivers/net/dsa/microchip/ksz9477_ptp.c
> +++ b/drivers/net/dsa/microchip/ksz9477_ptp.c
> @@ -1106,6 +1106,11 @@ irqreturn_t ksz9477_ptp_port_interrupt(struct
> ksz_device *dev, int port)
>         if (ret)
>                 return IRQ_NONE;
> 
> +       /* Clear interrupt(s) (W1C) */
> +       ret = ksz_write16(dev, addr, data);
> +       if (ret)
> +               return IRQ_NONE;
> +
>         if (data & PTP_PORT_XDELAY_REQ_INT) {
>                 /* Timestamp for Pdelay_Req / Delay_Req */
>                 struct ksz_device_ptp_shared *ptp_shared = &dev-
> >ptp_shared;
> @@ -1128,11 +1133,6 @@ irqreturn_t ksz9477_ptp_port_interrupt(struct
> ksz_device *dev, int port)
>                 complete(&prt->tstamp_completion);
>         }
> 
> -       /* Clear interrupt(s) (W1C) */
> -       ret = ksz_write16(dev, addr, data);
> -       if (ret)
> -               return IRQ_NONE;
> -
>         return IRQ_HANDLED;
>  }
> 
> 
> About the only difference seems to be that ACK-ing the interrupt is
> done
> at the end of ksz_ptp_irq_thread_fn(), while complete(&port-
> >tstamp_msg_comp)
> is called from ksz_ptp_msg_thread_fn() - which is called by
> handle_nested_irq()
> IIUC.

I will clear the interrupts before calling the handle_nested_irq().

> 
> > 
> > +/*