Re: [PATCH net-next 2/8] net/fungible: Add service module for Fungible drivers

On Thu, Dec 30, 2021 at 9:28 AM Andrew Lunn [off-list ref] wrote:

quoted

+/* Wait for the CSTS.RDY bit to match @enabled. */
+static int fun_wait_ready(struct fun_dev *fdev, bool enabled)
+{
+     unsigned int cap_to = NVME_CAP_TIMEOUT(fdev->cap_reg);
+     unsigned long timeout = ((cap_to + 1) * HZ / 2) + jiffies;
+     u32 bit = enabled ? NVME_CSTS_RDY : 0;

Reverse Christmas tree, since this is a network driver.

The longer line in the middle depends on the previous line, I'd need to
remove the initializers to sort these by length.

Please also consider using include/linux/iopoll.h. The signal handling
might make that not possible, but signal handling in driver code is in
itself very unusual.

This initialization is based on NVMe, hence the use of NVMe registers,
and this function is based on nvme_wait_ready(). The check sequence
including signal handling comes from there.

iopoll is possible with the signal check removed, though I see I'd need a
shorter delay than the 100ms used here and it doesn't check for reads of
all 1s, which happen occasionally. My preference though would be to keep
this close to the NVMe version. Let me know.

quoted

+
+     do {
+             u32 csts = readl(fdev->bar + NVME_REG_CSTS);
+
+             if (csts == ~0) {
+                     dev_err(fdev->dev, "CSTS register read %#x\n", csts);
+                     return -EIO;
+             }
+
+             if ((csts & NVME_CSTS_RDY) == bit)
+                     return 0;
+
+             msleep(100);
+             if (fatal_signal_pending(current))
+                     return -EINTR;
+     } while (time_is_after_eq_jiffies(timeout));
+
+     dev_err(fdev->dev,
+             "Timed out waiting for device to indicate RDY %u; aborting %s\n",
+             enabled, enabled ? "initialization" : "reset");
+     return -ETIMEDOUT;
+}

  Andrew