[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a402ea5d2badda79cf205e790d3eb967f2cb7084.camel@mellanox.com>
Date: Tue, 23 Jul 2019 23:47:43 +0000
From: Saeed Mahameed <saeedm@...lanox.com>
To: "snelson@...sando.io" <snelson@...sando.io>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"davem@...emloft.net" <davem@...emloft.net>
Subject: Re: [PATCH v4 net-next 02/19] ionic: Add hardware init and device
commands
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 <snelson@...sando.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;
> +}
> +
>
[...]
Powered by blists - more mailing lists