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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxx>

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@xxxxxxxxxxx>
Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@xxxxxxxxxxx>
Reviewed-by: Jeffrey Hugo <quic_jhugo@xxxxxxxxxxx>
Reviewed-by: Carl Vanderlip <quic_carlv@xxxxxxxxxxx>
Reviewed-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@xxxxxxxxxxx>
Signed-off-by: Jeffrey Hugo <quic_jhugo@xxxxxxxxxxx>
Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@xxxxxxxxxxxxxxx>

@@ -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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux