Re: [PATCH net-next 2/8] igc: remove I226 Qbv BaseTime restriction

[Kurt Kanzenbach <kurt@linutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 2173 bytes --]

On Mon Dec 05 2022, Tony Nguyen wrote:
> From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
>
> Remove the Qbv BaseTime restriction for I226 so that the BaseTime can be
> scheduled to the future time. A new register bit of Tx Qav Control
> (Bit-7: FutScdDis) was introduced to allow I226 scheduling future time as
> Qbv BaseTime and not having the Tx hang timeout issue.
>
> Besides, according to datasheet section 7.5.2.9.3.3, FutScdDis bit has to
> be configured first before the cycle time and base time.
>
> Indeed the FutScdDis bit is only active on re-configuration, thus we have
> to set the BASET_L to zero and then only set it to the desired value.
>
> Please also note that the Qbv configuration flow is moved around based on
> the Qbv programming guideline that is documented in the latest datasheet.
>
> Co-developed-by: Tan Tee Min <tee.min.tan@linux.intel.com>
> Signed-off-by: Tan Tee Min <tee.min.tan@linux.intel.com>
> Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
> Tested-by: Naama Meir <naamax.meir@linux.intel.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>

[snip]

> @@ -5852,8 +5853,10 @@ static bool validate_schedule(struct igc_adapter *adapter,
>  	 * in the future, it will hold all the packets until that
>  	 * time, causing a lot of TX Hangs, so to avoid that, we
>  	 * reject schedules that would start in the future.
> +	 * Note: Limitation above is no longer in i226.
>  	 */
> -	if (!is_base_time_past(qopt->base_time, &now))
> +	if (!is_base_time_past(qopt->base_time, &now) &&
> +	    igc_is_device_id_i225(hw))
>  		return false;

Nothing against this patch per se, but you should lift the base time
restriction for i225 as well. Even if it's hardware limitation, the
driver should deal with that e.g., using a timer, workqueue, ... The
TAPRIO interface allows the user to set an arbitrary base time, which
can and most likely will be in the future. IMHO the driver should handle
that. For instance, the hellcreek TSN switch has a similar limitation
(base time can only be applied up to 8 seconds in the future) and I've
worked around it in the driver.

Thanks,
Kurt

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]