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

From: Arun.Ramadoss
Date: Tue Nov 29 2022 - 23:32:55 EST


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
> >