On 7/23/19 4:47 PM, Saeed Mahameed wrote:

On Mon, 2019-07-22 at 14:40 -0700, Shannon Nelson wrote:

quoted

The ionic device has a small set of PCI registers, including a
device control and data space, and a large set of message
commands.

Signed-off-by: Shannon Nelson <redacted>
---
  drivers/net/ethernet/pensando/ionic/Makefile  |    2 +-
  drivers/net/ethernet/pensando/ionic/ionic.h   |   20 +
  .../net/ethernet/pensando/ionic/ionic_bus.h   |    1 +
  .../ethernet/pensando/ionic/ionic_bus_pci.c   |  140 +-
  .../ethernet/pensando/ionic/ionic_debugfs.c   |   67 +
  .../ethernet/pensando/ionic/ionic_debugfs.h   |   28 +
  .../net/ethernet/pensando/ionic/ionic_dev.c   |  132 +
  .../net/ethernet/pensando/ionic/ionic_dev.h   |  144 +
  .../net/ethernet/pensando/ionic/ionic_if.h    | 2552
+++++++++++++++++
  .../net/ethernet/pensando/ionic/ionic_main.c  |  296 ++
  .../net/ethernet/pensando/ionic/ionic_regs.h  |  133 +
  11 files changed, 3512 insertions(+), 3 deletions(-)
  create mode 100644
drivers/net/ethernet/pensando/ionic/ionic_debugfs.c
  create mode 100644
drivers/net/ethernet/pensando/ionic/ionic_debugfs.h
  create mode 100644 drivers/net/ethernet/pensando/ionic/ionic_dev.c
  create mode 100644 drivers/net/ethernet/pensando/ionic/ionic_dev.h
  create mode 100644 drivers/net/ethernet/pensando/ionic/ionic_if.h
  create mode 100644 drivers/net/ethernet/pensando/ionic/ionic_regs.h

[...]
  

quoted

  static void ionic_remove(struct pci_dev *pdev)
  {
  	struct ionic *ionic = pci_get_drvdata(pdev);
  
-	devm_kfree(&pdev->dev, ionic);
+	if (ionic) {

nit, in case you are doing another re-spin  maybe early return here:
if (!ionic)
      return;
//do stuff

Sure

quoted

+		ionic_reset(ionic);
+		ionic_dev_teardown(ionic);
+		ionic_unmap_bars(ionic);
+		pci_release_regions(pdev);
+		pci_clear_master(pdev);
+		pci_disable_sriov(pdev);
+		pci_disable_device(pdev);
+		ionic_debugfs_del_dev(ionic);
+		mutex_destroy(&ionic->dev_cmd_lock);
+
+		devm_kfree(&pdev->dev, ionic);
+	}
  }

[...]

quoted

  
+
+/* Devcmd Interface */
+u8 ionic_dev_cmd_status(struct ionic_dev *idev)
+{
+	return ioread8(&idev->dev_cmd_regs->comp.comp.status);
+}
+
+bool ionic_dev_cmd_done(struct ionic_dev *idev)
+{
+	return ioread32(&idev->dev_cmd_regs->done) & DEV_CMD_DONE;
+}
+
+void ionic_dev_cmd_comp(struct ionic_dev *idev, union dev_cmd_comp
*comp)
+{
+	memcpy_fromio(comp, &idev->dev_cmd_regs->comp, sizeof(*comp));
+}
+
+void ionic_dev_cmd_go(struct ionic_dev *idev, union dev_cmd *cmd)
+{
+	memcpy_toio(&idev->dev_cmd_regs->cmd, cmd, sizeof(*cmd));
+	iowrite32(0, &idev->dev_cmd_regs->done);
+	iowrite32(1, &idev->dev_cmd_regs->doorbell);
+}
+
+/* Device commands */
+void ionic_dev_cmd_identify(struct ionic_dev *idev, u8 ver)
+{
+	union dev_cmd cmd = {
+		.identify.opcode = CMD_OPCODE_IDENTIFY,
+		.identify.ver = ver,
+	};
+
+	ionic_dev_cmd_go(idev, &cmd);
+}
+
+void ionic_dev_cmd_init(struct ionic_dev *idev)
+{
+	union dev_cmd cmd = {
+		.init.opcode = CMD_OPCODE_INIT,
+		.init.type = 0,
+	};
+
+	ionic_dev_cmd_go(idev, &cmd);
+}
+
+void ionic_dev_cmd_reset(struct ionic_dev *idev)
+{
+	union dev_cmd cmd = {
+		.reset.opcode = CMD_OPCODE_RESET,
+	};
+
+	ionic_dev_cmd_go(idev, &cmd);
+}

[...]

quoted

+int ionic_dev_cmd_wait(struct ionic *ionic, unsigned long
max_seconds)
+{
+	struct ionic_dev *idev = &ionic->idev;
+	unsigned long max_wait, start_time, duration;
+	int opcode;
+	int done;
+	int err;
+
+	WARN_ON(in_interrupt());
+
+	/* Wait for dev cmd to complete, retrying if we get EAGAIN,
+	 * but don't wait any longer than max_seconds.
+	 */
+	max_wait = jiffies + (max_seconds * HZ);
+try_again:
+	start_time = jiffies;
+	do {
+		done = ionic_dev_cmd_done(idev);

READ_ONCE required here ? to read from coherent memory modified
by the device and read by the driver ?

Good idea, I'll add that in.

quoted

+		if (done)
+			break;
+		msleep(20);
+	} while (!done && time_before(jiffies, max_wait));

so your command interface is busy polling based, i am relating here to
Dave's comment regarding async command completion, is it possible to
have interrupt (MSIX?) based command completion in this hw ?

As I wrote elsewhere, this is only the low-level dev_cmd that does 
polling; the adminq does a wait that is completed by an MSI-x handler.

sln