diff mbox series

[v10,01/12] net: mana: Add support for auxiliary device

Message ID 1667502990-2559-2-git-send-email-longli@linuxonhyperv.com (mailing list archive)
State Awaiting Upstream
Delegated to: Netdev Maintainers
Headers show
Series Introduce Microsoft Azure Network Adapter (MANA) RDMA driver | expand

Checks

Context Check Description
netdev/tree_selection success Guessing tree name failed - patch did not apply

Commit Message

Long Li Nov. 3, 2022, 7:16 p.m. UTC
From: Long Li <longli@microsoft.com>

In preparation for supporting MANA RDMA driver, add support for auxiliary
device in the Ethernet driver. The RDMA device is modeled as an auxiliary
device to the Ethernet device.

Reviewed-by: Dexuan Cui <decui@microsoft.com>
Signed-off-by: Long Li <longli@microsoft.com>
Acked-by: Haiyang Zhang <haiyangz@microsoft.com>
---
Change log:
v3: define mana_adev_idx_alloc and mana_adev_idx_free as static
v7: fix a bug that may assign a negative value to adev->id

 drivers/net/ethernet/microsoft/Kconfig        |  1 +
 drivers/net/ethernet/microsoft/mana/gdma.h    |  2 +
 .../ethernet/microsoft/mana/mana_auxiliary.h  | 10 +++
 drivers/net/ethernet/microsoft/mana/mana_en.c | 83 ++++++++++++++++++-
 4 files changed, 95 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/ethernet/microsoft/mana/mana_auxiliary.h

Comments

Leon Romanovsky Nov. 8, 2022, 7:18 p.m. UTC | #1
On Thu, Nov 03, 2022 at 12:16:19PM -0700, longli@linuxonhyperv.com wrote:
> From: Long Li <longli@microsoft.com>
> 
> In preparation for supporting MANA RDMA driver, add support for auxiliary
> device in the Ethernet driver. The RDMA device is modeled as an auxiliary
> device to the Ethernet device.
> 
> Reviewed-by: Dexuan Cui <decui@microsoft.com>
> Signed-off-by: Long Li <longli@microsoft.com>
> Acked-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---
> Change log:
> v3: define mana_adev_idx_alloc and mana_adev_idx_free as static
> v7: fix a bug that may assign a negative value to adev->id

<...>

>  int mana_probe(struct gdma_dev *gd, bool resuming)
>  {
>  	struct gdma_context *gc = gd->gdma_context;
> @@ -2173,6 +2249,8 @@ int mana_probe(struct gdma_dev *gd, bool resuming)
>  				break;
>  		}
>  	}
> +
> +	err = add_adev(gd);
>  out:
>  	if (err)
>  		mana_remove(gd, false);
> @@ -2189,6 +2267,10 @@ void mana_remove(struct gdma_dev *gd, bool suspending)
>  	int err;
>  	int i;
>  
> +	/* adev currently doesn't support suspending, always remove it */
> +	if (gd->adev)

This condition is always true, isn't it?

> +		remove_adev(gd);
> +

Thanks
Long Li Nov. 8, 2022, 9:33 p.m. UTC | #2
> > int mana_probe(struct gdma_dev *gd, bool resuming)
> >  				break;
> >  		}
> >  	}
> > +
> > +	err = add_adev(gd);
> >  out:
> >  	if (err)
> >  		mana_remove(gd, false);
> > @@ -2189,6 +2267,10 @@ void mana_remove(struct gdma_dev *gd, bool
> suspending)
> >  	int err;
> >  	int i;
> >
> > +	/* adev currently doesn't support suspending, always remove it */
> > +	if (gd->adev)
> 
> This condition is always true, isn't it?

I think the check is necessary. mana_probe() will call mana_remove() if it fails to
add this adev to gd. If this is the case, we can't call remove_adev().

Thanks,
Long
Leon Romanovsky Nov. 9, 2022, 6:20 a.m. UTC | #3
On Tue, Nov 08, 2022 at 09:33:21PM +0000, Long Li wrote:
> 
> > > int mana_probe(struct gdma_dev *gd, bool resuming)
> > >  				break;
> > >  		}
> > >  	}
> > > +
> > > +	err = add_adev(gd);
> > >  out:
> > >  	if (err)
> > >  		mana_remove(gd, false);
> > > @@ -2189,6 +2267,10 @@ void mana_remove(struct gdma_dev *gd, bool
> > suspending)
> > >  	int err;
> > >  	int i;
> > >
> > > +	/* adev currently doesn't support suspending, always remove it */
> > > +	if (gd->adev)
> > 
> > This condition is always true, isn't it?
> 
> I think the check is necessary. mana_probe() will call mana_remove() if it fails to
> add this adev to gd. If this is the case, we can't call remove_adev().

I'm sad to hear that. It is so anti-pattern to hide error unwind in one global function.
But ok, it is already there, so let's take this series as is.

Thanks

> 
> Thanks,
> Long
diff mbox series

Patch

diff --git a/drivers/net/ethernet/microsoft/Kconfig b/drivers/net/ethernet/microsoft/Kconfig
index fe4e7a7d9c0b..090e6b983243 100644
--- a/drivers/net/ethernet/microsoft/Kconfig
+++ b/drivers/net/ethernet/microsoft/Kconfig
@@ -19,6 +19,7 @@  config MICROSOFT_MANA
 	tristate "Microsoft Azure Network Adapter (MANA) support"
 	depends on PCI_MSI && X86_64
 	depends on PCI_HYPERV
+	select AUXILIARY_BUS
 	help
 	  This driver supports Microsoft Azure Network Adapter (MANA).
 	  So far, the driver is only supported on X86_64.
diff --git a/drivers/net/ethernet/microsoft/mana/gdma.h b/drivers/net/ethernet/microsoft/mana/gdma.h
index 4a6efe6ada08..f321a2616d03 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma.h
+++ b/drivers/net/ethernet/microsoft/mana/gdma.h
@@ -204,6 +204,8 @@  struct gdma_dev {
 
 	/* GDMA driver specific pointer */
 	void *driver_data;
+
+	struct auxiliary_device *adev;
 };
 
 #define MINIMUM_SUPPORTED_PAGE_SIZE PAGE_SIZE
diff --git a/drivers/net/ethernet/microsoft/mana/mana_auxiliary.h b/drivers/net/ethernet/microsoft/mana/mana_auxiliary.h
new file mode 100644
index 000000000000..373d59756846
--- /dev/null
+++ b/drivers/net/ethernet/microsoft/mana/mana_auxiliary.h
@@ -0,0 +1,10 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* Copyright (c) 2022, Microsoft Corporation. */
+
+#include "mana.h"
+#include <linux/auxiliary_bus.h>
+
+struct mana_adev {
+	struct auxiliary_device adev;
+	struct gdma_dev *mdev;
+};
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index 9259a74eca40..8751e475d1ba 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -13,6 +13,19 @@ 
 #include <net/ip6_checksum.h>
 
 #include "mana.h"
+#include "mana_auxiliary.h"
+
+static DEFINE_IDA(mana_adev_ida);
+
+static int mana_adev_idx_alloc(void)
+{
+	return ida_alloc(&mana_adev_ida, GFP_KERNEL);
+}
+
+static void mana_adev_idx_free(int idx)
+{
+	ida_free(&mana_adev_ida, idx);
+}
 
 /* Microsoft Azure Network Adapter (MANA) functions */
 
@@ -2106,6 +2119,69 @@  static int mana_probe_port(struct mana_context *ac, int port_idx,
 	return err;
 }
 
+static void adev_release(struct device *dev)
+{
+	struct mana_adev *madev = container_of(dev, struct mana_adev, adev.dev);
+
+	kfree(madev);
+}
+
+static void remove_adev(struct gdma_dev *gd)
+{
+	struct auxiliary_device *adev = gd->adev;
+	int id = adev->id;
+
+	auxiliary_device_delete(adev);
+	auxiliary_device_uninit(adev);
+
+	mana_adev_idx_free(id);
+	gd->adev = NULL;
+}
+
+static int add_adev(struct gdma_dev *gd)
+{
+	struct auxiliary_device *adev;
+	struct mana_adev *madev;
+	int ret;
+
+	madev = kzalloc(sizeof(*madev), GFP_KERNEL);
+	if (!madev)
+		return -ENOMEM;
+
+	adev = &madev->adev;
+	ret = mana_adev_idx_alloc();
+	if (ret < 0)
+		goto idx_fail;
+	adev->id = ret;
+
+	adev->name = "rdma";
+	adev->dev.parent = gd->gdma_context->dev;
+	adev->dev.release = adev_release;
+	madev->mdev = gd;
+
+	ret = auxiliary_device_init(adev);
+	if (ret)
+		goto init_fail;
+
+	ret = auxiliary_device_add(adev);
+	if (ret)
+		goto add_fail;
+
+	gd->adev = adev;
+	return 0;
+
+add_fail:
+	auxiliary_device_uninit(adev);
+
+init_fail:
+	mana_adev_idx_free(adev->id);
+
+idx_fail:
+	kfree(madev);
+
+	return ret;
+}
+
 int mana_probe(struct gdma_dev *gd, bool resuming)
 {
 	struct gdma_context *gc = gd->gdma_context;
@@ -2173,6 +2249,8 @@  int mana_probe(struct gdma_dev *gd, bool resuming)
 				break;
 		}
 	}
+
+	err = add_adev(gd);
 out:
 	if (err)
 		mana_remove(gd, false);
@@ -2189,6 +2267,10 @@  void mana_remove(struct gdma_dev *gd, bool suspending)
 	int err;
 	int i;
 
+	/* adev currently doesn't support suspending, always remove it */
+	if (gd->adev)
+		remove_adev(gd);
+
 	for (i = 0; i < ac->num_ports; i++) {
 		ndev = ac->ports[i];
 		if (!ndev) {
@@ -2221,7 +2303,6 @@  void mana_remove(struct gdma_dev *gd, bool suspending)
 	}
 
 	mana_destroy_eq(ac);
-
 out:
 	mana_gd_deregister_device(gd);