On Mon, 2019-07-22 at 14:40 -0700, Shannon Nelson wrote: > 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 <snel...@pensando.io> > --- > 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 >
[...] > 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 > + 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); > + } > } > [...] > > + > +/* 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); > +} [...] > +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 ? > + 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 ? > + duration = jiffies - start_time; > + > + opcode = idev->dev_cmd_regs->cmd.cmd.opcode; > + dev_dbg(ionic->dev, "DEVCMD %s (%d) done=%d took %ld secs (%ld > jiffies)\n", > + ionic_opcode_to_str(opcode), opcode, > + done, duration / HZ, duration); > + > + if (!done && !time_before(jiffies, max_wait)) { > + dev_warn(ionic->dev, "DEVCMD %s (%d) timeout after %ld > secs\n", > + ionic_opcode_to_str(opcode), opcode, > max_seconds); > + return -ETIMEDOUT; > + } > + > + err = ionic_dev_cmd_status(&ionic->idev); > + if (err) { > + if (err == IONIC_RC_EAGAIN && !time_after(jiffies, > max_wait)) { > + dev_err(ionic->dev, "DEV_CMD %s (%d) error, %s > (%d) retrying...\n", > + ionic_opcode_to_str(opcode), opcode, > + ionic_error_to_str(err), err); > + > + msleep(1000); > + iowrite32(0, &idev->dev_cmd_regs->done); > + iowrite32(1, &idev->dev_cmd_regs->doorbell); > + goto try_again; > + } > + > + dev_err(ionic->dev, "DEV_CMD %s (%d) error, %s (%d) > failed\n", > + ionic_opcode_to_str(opcode), opcode, > + ionic_error_to_str(err), err); > + > + return ionic_error_to_errno(err); > + } > + > + return 0; > +} > + > [...]