[PATCH v2 1/2] accel/qaic: Add support for periodic timesync

[Jeffrey Hugo]

On 10/22/2023 5:06 AM, Stanislaw Gruszka wrote:
> On Mon, Oct 16, 2023 at 11:01:13AM -0600, Jeffrey Hugo wrote:
>> From: Ajit Pal Singh <quic_ajitpals at quicinc.com>
>>
>> Device and Host have a time synchronization mechanism that happens once
>> during boot when device is in SBL mode. After that, in mission-mode there
>> is no timesync. In an experiment after continuous operation, device time
>> drifted w.r.t. host by approximately 3 seconds per day. This drift leads
>> to mismatch in timestamp of device and Host logs. To correct this
>> implement periodic timesync in driver. This timesync is carried out via
>> QAIC_TIMESYNC_PERIODIC MHI channel.
>>
>> Signed-off-by: Ajit Pal Singh <quic_ajitpals at quicinc.com>
>> Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy at quicinc.com>
>> Reviewed-by: Jeffrey Hugo <quic_jhugo at quicinc.com>
>> Reviewed-by: Carl Vanderlip <quic_carlv at quicinc.com>
>> Reviewed-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy at quicinc.com>
>> Signed-off-by: Jeffrey Hugo <quic_jhugo at quicinc.com>
> Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka at linux.intel.com>
> 
>> @@ -586,8 +587,16 @@ static int __init qaic_init(void)
>>   		goto free_pci;
>>   	}
>>   
>> +	ret = qaic_timesync_init();
>> +	if (ret) {
>> +		pr_debug("qaic: qaic_timesync_init failed %d\n", ret);
>> +		goto free_mhi;
> I would print at error level here. Or if timesync is optional do not error exit.

Good point.  timesync is optional so will not do error exit.

> 
>> +#ifdef readq
>> +static u64 read_qtimer(const volatile void __iomem *addr)
>> +{
>> +	return readq(addr);
>> +}
>> +#else
>> +static u64 read_qtimer(const volatile void __iomem *addr)
>> +{
>> +	u64 low, high;
>> +
>> +	low = readl(addr);
>> +	high = readl(addr + sizeof(u32));
>> +	return low | (high << 32);
>> +}
> If that's only for compile on 32-bit PowerPC, I think would be better
> to limit supported architectures on Kconfig.

The issue was flagged on 32-bit PowerPC, but I concluded from a code 
review that the issue exists on any 32-bit architecture.  Given that 
this is an add-on card I'd prefer to support as many architectures as 
possible.

-Jeff