On 8/26/19 9:36 PM, Jakub Kicinski wrote:
On Mon, 26 Aug 2019 14:33:24 -0700, Shannon Nelson wrote:
The port management commands apply to the physical port
associated with the PCI device, which might be shared among
several logical interfaces.

Signed-off-by: Shannon Nelson <snel...@pensando.io>
[...]

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h 
b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
index 30a5206bba4e..5b83f21af18a 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
@@ -122,6 +122,10 @@ struct ionic_dev {
        struct ionic_intr __iomem *intr_ctrl;
        u64 __iomem *intr_status;
+ u32 port_info_sz;
+       struct ionic_port_info *port_info;
+       dma_addr_t port_info_pa;
+
        struct ionic_devinfo dev_info;
  };
@@ -140,4 +144,15 @@ void ionic_dev_cmd_identify(struct ionic_dev *idev, u8 ver);
  void ionic_dev_cmd_init(struct ionic_dev *idev);
  void ionic_dev_cmd_reset(struct ionic_dev *idev);
+void ionic_dev_cmd_port_identify(struct ionic_dev *idev);
+void ionic_dev_cmd_port_init(struct ionic_dev *idev);
+void ionic_dev_cmd_port_reset(struct ionic_dev *idev);
+void ionic_dev_cmd_port_state(struct ionic_dev *idev, u8 state);
+void ionic_dev_cmd_port_speed(struct ionic_dev *idev, u32 speed);
+void ionic_dev_cmd_port_mtu(struct ionic_dev *idev, u32 mtu);
+void ionic_dev_cmd_port_autoneg(struct ionic_dev *idev, u8 an_enable);
+void ionic_dev_cmd_port_fec(struct ionic_dev *idev, u8 fec_type);
+void ionic_dev_cmd_port_pause(struct ionic_dev *idev, u8 pause_type);
+void ionic_dev_cmd_port_loopback(struct ionic_dev *idev, u8 loopback_mode);
I don't think you call most of these functions in this patch.

No, but most get used in the ethtool code added a few patches later.  The port_mtu probably won't get used, so I can pull that out.  The port_loopback will get used when I add a loopback test, but I can pull that out for now until that test is added.


  #endif /* _IONIC_DEV_H_ */
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c 
b/drivers/net/ethernet/pensando/ionic/ionic_main.c
index f52eb6c50358..47928f184230 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
@@ -309,6 +309,92 @@ int ionic_reset(struct ionic *ionic)
        return err;
  }
+int ionic_port_identify(struct ionic *ionic)
+{
+       struct ionic_identity *ident = &ionic->ident;
+       struct ionic_dev *idev = &ionic->idev;
+       size_t sz;
+       int err;
+
+       mutex_lock(&ionic->dev_cmd_lock);
+
+       ionic_dev_cmd_port_identify(idev);
+       err = ionic_dev_cmd_wait(ionic, devcmd_timeout);
+       if (!err) {
+               sz = min(sizeof(ident->port), sizeof(idev->dev_cmd_regs->data));
+               memcpy_fromio(&ident->port, &idev->dev_cmd_regs->data, sz);
+       }
+
+       mutex_unlock(&ionic->dev_cmd_lock);
+
+       return err;
+}
+
+int ionic_port_init(struct ionic *ionic)
+{
+       struct ionic_identity *ident = &ionic->ident;
+       struct ionic_dev *idev = &ionic->idev;
+       size_t sz;
+       int err;
+
+       if (idev->port_info)
+               return 0;
+
+       idev->port_info_sz = ALIGN(sizeof(*idev->port_info), PAGE_SIZE);
+       idev->port_info = dma_alloc_coherent(ionic->dev, idev->port_info_sz,
+                                            &idev->port_info_pa,
+                                            GFP_KERNEL);
+       if (!idev->port_info) {
+               dev_err(ionic->dev, "Failed to allocate port info, aborting\n");
+               return -ENOMEM;
+       }
+
+       sz = min(sizeof(ident->port.config), sizeof(idev->dev_cmd_regs->data));
+
+       mutex_lock(&ionic->dev_cmd_lock);
+
+       memcpy_toio(&idev->dev_cmd_regs->data, &ident->port.config, sz);
+       ionic_dev_cmd_port_init(idev);
+       err = ionic_dev_cmd_wait(ionic, devcmd_timeout);
+
+       ionic_dev_cmd_port_state(&ionic->idev, IONIC_PORT_ADMIN_STATE_UP);
+       (void)ionic_dev_cmd_wait(ionic, devcmd_timeout);
+
+       mutex_unlock(&ionic->dev_cmd_lock);
+       if (err) {
+               dev_err(ionic->dev, "Failed to init port\n");
The lifetime of port_info seems a little strange. Why is it left in
place even if the command failed? Doesn't this leak memory?

+               return err;
+       }
+
+       return 0;
return err; work for both paths

+}
+
+int ionic_port_reset(struct ionic *ionic)
+{
+       struct ionic_dev *idev = &ionic->idev;
+       int err;
+
+       if (!idev->port_info)
+               return 0;
+
+       mutex_lock(&ionic->dev_cmd_lock);
+       ionic_dev_cmd_port_reset(idev);
+       err = ionic_dev_cmd_wait(ionic, devcmd_timeout);
+       mutex_unlock(&ionic->dev_cmd_lock);
+       if (err) {
+               dev_err(ionic->dev, "Failed to reset port\n");
+               return err;
Again, memory leak if command fails? (nothing frees port_info)

+       }
+
+       dma_free_coherent(ionic->dev, idev->port_info_sz,
+                         idev->port_info, idev->port_info_pa);
+
+       idev->port_info = NULL;
+       idev->port_info_pa = 0;
+
+       return err;
Well, with current code err can only be 0 at this point.

I'll revisit these bits.

sln


Reply via email to