[RFC PATCH net-next v3 1/2] macb: Add 1588 support in Cadence GEM.
Andrei.Pistirica at microchip.com
Andrei.Pistirica at microchip.com
Mon Dec 12 02:22:43 PST 2016
> -----Original Message-----
> From: Rafal Ozieblo [mailto:rafalo at cadence.com]
> Sent: Friday, December 09, 2016 11:20 AM
> To: Andrei Pistirica - M16132; richardcochran at gmail.com
> Cc: netdev at vger.kernel.org; linux-kernel at vger.kernel.org; linux-arm-
> kernel at lists.infradead.org; davem at davemloft.net;
> nicolas.ferre at atmel.com; harinikatakamlinux at gmail.com;
> harini.katakam at xilinx.com; punnaia at xilinx.com; michals at xilinx.com;
> anirudh at xilinx.com; boris.brezillon at free-electrons.com;
> alexandre.belloni at free-electrons.com; tbultel at pixelsurmer.com
> Subject: RE: [RFC PATCH net-next v3 1/2] macb: Add 1588 support in Cadence
> GEM.
>
> -----Original Message-----
> > From: Andrei.Pistirica at microchip.com
> > [mailto:Andrei.Pistirica at microchip.com]
> > Sent: 8 grudnia 2016 15:42
> > To: richardcochran at gmail.com
> > Cc: netdev at vger.kernel.org; linux-kernel at vger.kernel.org;
> > linux-arm-kernel at lists.infradead.org; davem at davemloft.net;
> > nicolas.ferre at atmel.com; harinikatakamlinux at gmail.com;
> > harini.katakam at xilinx.com; punnaia at xilinx.com; michals at xilinx.com;
> > anirudh at xilinx.com; boris.brezillon at free-electrons.com;
> > alexandre.belloni at free-electrons.com; tbultel at pixelsurmer.com; Rafal
> > Ozieblo
> > Subject: RE: [RFC PATCH net-next v3 1/2] macb: Add 1588 support in
> Cadence GEM.
> >
> >
> >
> > > -----Original Message-----
> > > From: Richard Cochran [mailto:richardcochran at gmail.com]
> > > Sent: Wednesday, December 07, 2016 11:04 PM
> > > To: Andrei Pistirica - M16132
> > > Cc: netdev at vger.kernel.org; linux-kernel at vger.kernel.org; linux-arm-
> > > kernel at lists.infradead.org; davem at davemloft.net;
> > > nicolas.ferre at atmel.com; harinikatakamlinux at gmail.com;
> > > harini.katakam at xilinx.com; punnaia at xilinx.com; michals at xilinx.com;
> > > anirudh at xilinx.com; boris.brezillon at free-electrons.com;
> > > alexandre.belloni at free-electrons.com; tbultel at pixelsurmer.com;
> > > rafalo at cadence.com
> > > Subject: Re: [RFC PATCH net-next v3 1/2] macb: Add 1588 support in
> > > Cadence GEM.
> > >
> > > On Wed, Dec 07, 2016 at 08:39:09PM +0100, Richard Cochran wrote:
> > > > > +static s32 gem_ptp_max_adj(unsigned int f_nom) {
> > > > > + u64 adj;
> > > > > +
> > > > > + /* The 48 bits of seconds for the GEM overflows every:
> > > > > + * 2^48/(365.25 * 24 * 60 *60) =~ 8 925 512 years (~= 9 mil years),
> > > > > + * thus the maximum adjust frequency must not overflow
> > > > > + CNS
> > > register:
> > > > > + *
> > > > > + * addend = 10^9/nominal_freq
> > > > > + * adj_max = +/- addend*ppb_max/10^9
> > > > > + * max_ppb = (2^8-1)*nominal_freq-10^9
> > > > > + */
> > > > > + adj = f_nom;
> > > > > + adj *= 0xffff;
> > > > > + adj -= 1000000000ULL;
> > > >
> > > > What is this computation, and how does it relate to the comment?
> >
> > I considered the following simple equation: increment value at nominal
> frequency (which is 10^9/nominal frequency nsecs) + the maximum drift
> value (nsecs) <= maximum increment value at nominal frequency (which is
> 8bit:0xffff).
> > If maximum drift is written as function of nominal frequency and
> maximum ppb, then the equation above yields that the maximum ppb is:
> (2^8 - 1) *nominal_frequency - 10^9. The equation is also simplified by the
> fact that the drift is written as ppm + 16bit_fractions and the increment
> value is written as nsec + 16bit_fractions.
> >
> > Rafal said that this value is hardcoded: 0x64E6, while Harini said:
> 250000000.
>
> To clarify a little bit. In my reference code this value (0x64E6) was taken
> from our legacy code. It was used for testing only. I know it should be
> change to something more accurate. This is the reason why I asked how did
> you count it (250000000). According to our calculations this value depends
> on actual set period (incr_ns and incr_sub_ns) and min and max value we
> can set. The calculation were a little bit intricate, so we decided to leave it
> as it was.
>
> >
> > I need to dig into this...
> >
> > >
> > > I am not sure what you meant, but it sounds like you are on the wrong
> track.
> > > Let me explain...
> >
> > Thanks.
> >
> > >
> > > The max_adj has nothing at all to do with the width of the time register.
> > > Rather, it should reflect the maximum possible change in the tuning
> word.
> > >
> > > For example, with a nominal 8 ns period, the tuning word is 0x80000.
> > > Looking at running the clock more slowly, the slowest possible word
> > > is 0x00001, meaning a difference of 0x7FFFF. This implies an
> > > adjustment of
> > > 0x7FFFF/0x80000 or 999998092 ppb. Running more quickly, we can
> > > already have 0x100000, twice as fast, or just under 2 billion ppb.
> > >
> > > You should consider the extreme cases to determine the most limited
> > > (smallest) max_adj value:
> > >
> > > Case 1 - high frequency
> > > ~~~~~~~~~~~~~~~~~~~~~~~
> > >
> > > With a nominal 1 ns period, we have the nominal tuning word 0x10000.
> > > The smallest is 0x1 for a difference of 0xFFFF. This corresponds to
> > > an adjustment of 0xFFFF/0x10000 = .9999847412109375 or 999984741 ppb.
> > >
> > > Case 2 - low frequency
> > > ~~~~~~~~~~~~~~~~~~~~~~
> > >
> > > With a nominal 255 ns period, the nominal word is 0xFF0000, the
> > > largest 0xFFFFFF, and the difference is 0xFFFF. This corresponds to
> > > and adjustment of 0xFFFF/0xFF0000 = .0039215087890625 or 3921508 ppb.
> > >
> > > Since 3921508 ppb is a huge adjustment, you can simply use that as a
> > > safe maximum, ignoring the actual input clock.
> > >
> > > Thanks,
> > > Richard
> > >
> > >
> >
> > Regards,
> > Andrei
> >
>
> Best regards,
> Rafal Ozieblo | Firmware System Engineer,
> phone nbr.: +48 32 5085469
> www.cadence.com
Hi Guys,
Based on Richard's input, this is what I want to do for our platforms:
struct macb_ptp_info {
void (*ptp_init)(struct net_device *ndev);
void (*ptp_remove)(struct net_device *ndev);
+ s32 (*get_ptp_max_adj)(void);
unsigned int (*get_tsu_rate)(struct macb *bp);
int (*get_ts_info)(struct net_device *dev,
struct ethtool_ts_info *info);
int (*get_hwtst)(struct net_device *netdev,
struct ifreq *ifr);
int (*set_hwtst)(struct net_device *netdev,
struct ifreq *ifr, int cmd);
};
+static s32 gem_get_ptp_max_adj(void)
+{
+ return 3921508;
+}
static struct macb_ptp_info gem_ptp_info = {
.ptp_init = gem_ptp_init,
.ptp_remove = gem_ptp_remove,
+ .get_ptp_max_adj = gem_get_ptp_max_adj,
.get_tsu_rate = gem_get_tsu_rate,
.get_ts_info = gem_get_ts_info,
.get_hwtst = gem_get_hwtst,
.set_hwtst = gem_set_hwtst,
};
[...]
void gem_ptp_init(struct net_device *ndev)
{
[...]
/* nominal frequency and maximum adjustment in ppb */
bp->tsu_rate = bp->ptp_info->get_tsu_rate(bp);
+ bp->ptp_caps.max_adj = bp->ptp_info->get_ptp_max_adj();
[...]
}
Richard, are you agree with this?
Harini, you can fill the callback with the value for your platform. Tell me if you are ok with function's signature.
Regards,
Andrei
More information about the linux-arm-kernel
mailing list