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

[Andrei.Pistirica]

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

> > +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)
> >  			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.

[...]

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