Re: [Patch net-next v1 02/12] net: dsa: microchip: ptp: Initial hardware time stamping support

[Arun.Ramadoss]

Hi Pavan,
Thanks for the comment.

On Tue, 2022-11-29 at 14:19 +0530, Pavan Chebbi wrote:
> On Mon, Nov 28, 2022 at 4:03 PM Arun Ramadoss
> <arun.ramadoss@xxxxxxxxxxxxx> wrote:
> > 
> > From: Christian Eggers <ceggers@xxxxxxx>
> > 
> > This patch adds the routine for get_ts_info, hwstamp_get, set. This
> > enables
> > the PTP support towards userspace applications such as linuxptp.
> > Tx timestamping can be enabled per port and Rx timestamping enabled
> > globally.
> > 
> > Signed-off-by: Christian Eggers <ceggers@xxxxxxx>
> > Co-developed-by: Arun Ramadoss <arun.ramadoss@xxxxxxxxxxxxx>
> > Signed-off-by: Arun Ramadoss <arun.ramadoss@xxxxxxxxxxxxx>
> > 
> > ---
> > RFC v2 -> Patch v1
> > - moved tagger set and get function to separate patch
> > - Removed unnecessary comments
> > ---
> >  drivers/net/dsa/microchip/ksz_common.c |  2 +
> >  drivers/net/dsa/microchip/ksz_common.h |  4 ++
> >  drivers/net/dsa/microchip/ksz_ptp.c    | 77
> > +++++++++++++++++++++++++-
> >  drivers/net/dsa/microchip/ksz_ptp.h    | 14 +++++
> >  4 files changed, 95 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/dsa/microchip/ksz_common.c
> > b/drivers/net/dsa/microchip/ksz_common.c
> > index 2d09cd141db6..7b85b258270c 100644
> > --- a/drivers/net/dsa/microchip/ksz_common.c
> > +++ b/drivers/net/dsa/microchip/ksz_common.c
> > @@ -2873,6 +2873,8 @@ static const struct dsa_switch_ops
> > ksz_switch_ops = {
> >         .port_change_mtu        = ksz_change_mtu,
> >         .port_max_mtu           = ksz_max_mtu,
> >         .get_ts_info            = ksz_get_ts_info,
> > +       .port_hwtstamp_get      = ksz_hwtstamp_get,
> > +       .port_hwtstamp_set      = ksz_hwtstamp_set,
> >  };
> > 
> >  struct ksz_device *ksz_switch_alloc(struct device *base, void
> > *priv)
> > diff --git a/drivers/net/dsa/microchip/ksz_common.h
> > b/drivers/net/dsa/microchip/ksz_common.h
> > index 5a6bfd42c6f9..cd20f39a565f 100644
> > --- a/drivers/net/dsa/microchip/ksz_common.h
> > +++ b/drivers/net/dsa/microchip/ksz_common.h
> > @@ -103,6 +103,10 @@ struct ksz_port {
> >         struct ksz_device *ksz_dev;
> >         struct ksz_irq pirq;
> >         u8 num;
> > +#if IS_ENABLED(CONFIG_NET_DSA_MICROCHIP_KSZ_PTP)
> > +       u8 hwts_tx_en;
> > +       bool hwts_rx_en;
> 
> I see that the hwts_rx_en gets removed in the later patch. Instead
> you
> could add rx filters support only later when you have the final code
> in place.

In RFC v2, this patch 2 and next patch 3 are in single patch. There are
two simple reasons, I had splitted them two. One is to have logical
commit and ease of code review. Patch 2 will add the hwtstamp_set and
get dsa hooks and patch 3 to mechanism to coordinate with the tag_ksz.c
to add additional 4 bytes in tail tag when PTP is enabled based on
tagger_data.
Second reason to split is to have patch authorship. I had used the
Christian Eggers patch and extending support for LAN937x. In the
initial series, it has dsa_port->priv variable to store the
ptp_shared_data. Now priv variable is removed from dsa_port so I used
tagger_data based on sja1105 implementation. As it is different from
intial patch series, moved to separate patch. 

> 
> > +#endif
> >  };
> > 
> >  
> > 
> > 
> > 
> >  #endif
> > --
> > 2.36.1
> >