On Tue, 24 Nov 2020 21:00:04 +0000 Vladimir Oltean wrote: > 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.
Thanks for explaining!