Re: [PATCH v1 4/8] net: dsa: hellcreek: Add support for hardware timestamping

On Mon, Jul 13, 2020 at 12:57:34PM +0200, Kurt Kanzenbach wrote:

Hi Vladimir,

On Mon Jul 13 2020, Vladimir Oltean wrote:

quoted

quoted

+/* Get a pointer to the PTP header in this skb */
+static u8 *parse_ptp_header(struct sk_buff *skb, unsigned int type)

Maybe this and the function from mv88e6xxx could share the same
implementation somehow.

Actually both functions are identical. Should it be moved to the ptp
core, maybe? Then, all drivers could use that. I guess we should also
define a PTP offset for the reserved field which is accessed in
hellcreek_get_reserved_field() just with 16 instead of a proper macro
constant.

I support re-factoring the code that parses the PTP header.  Last time
I looked, each driver needed slightly different fields, and I didn't
see an easy way to accommodate them all.

quoted

I would like to get some clarification on whether "SKBTX_IN_PROGRESS"
should be set in shtx->tx_flags or not. On one hand, it's asking for
trouble, on the other hand, it's kind of required for proper compliance
to API pre-SO_TIMESTAMPING...

Hm. We actually oriented our code on the mv88e6xxx time stamping code base.

Where in mv88e6xxx does the driver set SKBTX_IN_PROGRESS?

I don't think it makes sense for DSA drivers to set this bit, as it
serves no purpose in the DSA context.

Thanks,
Richard