Re: [PATCH net] enetc: Advance the taprio base time in the future

[Vladimir Oltean <vladimir.oltean@nxp.com>]

On Tue, Nov 24, 2020 at 09:58:12AM -0800, Jakub Kicinski wrote:
> > This is the right way for calculation. For the ENETC,  hardware also
> > do the same calculation before send to Operation State Machine.
> > For some TSN IP, like Felix and DesignWare TSN in RT1170 and IMX8MP
> > require the basetime limite the range not less than the current time
> > 8 cycles, software may do calculation before setting to the
> > hardware.
> > Actually, I do suggest this calculation to sch_taprio.c, but I found
> > same calculation only for the TXTIME by taprio_get_start_time().
> > Which means:
> > If (currenttime < basetime)
> >        Admin_basetime = basetime;
> > Else
> >        Admin_basetime =  basetime + (n+1)* cycletime;
> > N is the minimal value which make Admin_basetime is larger than the
> > currenttime.
> >
> > User space never to get the current time. Just set a value as offset
> > OR future time user want.
> > For example: set basetime = 1000000ns, means he want time align to
> > 1000000ns, and on the other device, also set the basetime =
> > 1000000ns, then the two devices are aligned cycle.
> > If user want all the devices start at 11.24.2020 11:00 then set
> > basetime = 1606273200.0 s.
> >
> > > - the sja1105 offload does it via future_base_time()
> > > - the ocelot/felix offload does it via vsc9959_new_base_time()
> > >
> > > As for the obvious question: doesn't the hardware just "do the right thing"
> > > if passed a time in the past? I've tested and it doesn't look like it. I cannot
> >
> > So hardware already do calculation same way.
>
> So the patch is unnecessary? Or correct? Not sure what you're saying..

He's not saying the patch is unnecessary. What the enetc driver
currently does for the case where the base_time is zero is bogus anyway.

What Po is saying is that calling future_base_time() should not be
needed. Instead, he is suggesting we could program directly the
admin_conf->base_time into the hardware, which will do the right thing
as long as the driver doesn't mangle it in various ways, such as replace
the base_time with the current time.

And what I said in the commit message is that I've been there before and
there were some still apparent issues with the schedule's phase. I had
some issues at the application layer as well. In the meantime I sorted
those out, and after re-applying the simple kernel change and giving the
system some thorough testing, it looks like Po is right.