Re: [PATCH v2 net-next 1/1] net: dsa: hellcreek: Add TAPRIO offloading support

[Kurt Kanzenbach <kurt@linutronix.de>]

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

On Tue Jan 19 2021, Jakub Kicinski wrote:
> On Sat, 16 Jan 2021 13:49:22 +0100 Kurt Kanzenbach wrote:
>> +	if (base_time_ns - current_ns < (s64)8 * NSEC_PER_SEC)
>> +		return true;
>> +
>> +	return false;
>
> nit:
> 	return base_time_ns - current_ns < (s64)8 * NSEC_PER_SEC;

Sure.

>> +	/* Schedule periodic schedule check */
>> +	schedule_delayed_work(&hellcreek_port->schedule_work,
>> +			      HELLCREEK_SCHEDULE_PERIOD);
>
> Why schedule this work every 2 seconds rather than scheduling it
> $start_time - 8 sec + epsilon?

The two seconds are taken from the programming guide. That's why I used
it.

The PTP frequency starts to matter for large deltas. In theory the
rescheduling period can be increased [1]. Should I adjust it? 

>
>> +static bool hellcreek_validate_schedule(struct hellcreek *hellcreek,
>> +					struct tc_taprio_qopt_offload *schedule)
>> +{
>> +	/* Does this hellcreek version support Qbv in hardware? */
>> +	if (!hellcreek->pdata->qbv_support)
>> +		return false;
>> +
>> +	/* cycle time can only be 32bit */
>> +	if (schedule->cycle_time > (u32)-1)
>> +		return false;
>> +
>> +	/* cycle time extension is not supported */
>> +	if (schedule->cycle_time_extension)
>> +		return false;
>
> What's the story with entries[i].command? I see most drivers validate
> the command is what they expect.
>

Good catch! I'll add the validation.

Thanks,
Kurt

[1] - https://lkml.kernel.org/netdev/20200901144755.jd2wnmweywwvkwvl@skbuf/

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