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.
