RE: [PATCH net-next] macb: Common code to enable ptp support for SAMA5Dx platforms.

Subject: Re: [PATCH net-next] macb: Common code to enable ptp support
for SAMA5Dx platforms.

Le 18/01/2017 à 09:57, Andrei Pistirica a écrit :

quoted

This patch does the following:
- add GEM-PTP interface
- registers and bitfields for TSU are named according to SAMA5Dx data
sheet
- PTP support based on platform capability

The $subject will certainly never match reality, sadly "enable ptp support
for SAMA5Dx platforms". So, you'd better change it.
(no "." at the end BTW).

I will change it to: " Common code to enable ptp support for MACB/GEM"

quoted

+2518,7 @@ static void macb_configure_caps(struct macb *bp,
 		dcfg = gem_readl(bp, DCFG2);
 		if ((dcfg & (GEM_BIT(RX_PKT_BUFF) |

GEM_BIT(TX_PKT_BUFF))) == 0)

quoted

 			bp->caps |= MACB_CAPS_FIFO_MODE;
+

Nitpicking, just because other issue exists: this white line doesn't belong to
the patch.

Ok I'll remove it. I missed it because checkpatch didn't report any warning.

[...]

quoted

 #define MACB_CAPS_GIGABIT_MODE_AVAILABLE	0x20000000
 #define MACB_CAPS_SG_DISABLED			0x40000000
 #define MACB_CAPS_MACB_IS_GEM			0x80000000
+#define MACB_CAPS_GEM_HAS_PTP			0x00000020

No, this mask already exists a couple of lines above:
#define MACB_CAPS_JUMBO        0x00000020

That leads to a NACK, sorry (I didn't spotted earlier, BTW).

Yes... you are right... sorry.

[...] 

Otherwise, I'm okay with the rest.

I suggest to people that will keep the ball rolling on this topic to take
advantage of the chunks of code that Andrei developed with the help of
Richard and the best practices discussed. I think particularly, if it makes
sense with HW, about:
- gem_ptp_do_[rt]xstamp(bp, skb) dereference scheme
- gem_ptp_adjfine() rationale
- gem_get_ptp_peer() if needed

Just mind that in case of an implementation with buffer rings and irqs
a different mechanism have to be used.

Regards,
Andrei

Regards,
--
Nicolas Ferre