[net-next,v2,1/9] Implementation of Virtual Bus

Commit Message

Kirsher, Jeffrey T April 21, 2020, 8:02 a.m. UTC

From: Dave Ertman <[e-mail]>

This is the initial implementation of the Virtual Bus,
virtbus_device and virtbus_driver.  The virtual bus is
a software based bus intended to support registering
virtbus_devices and virtbus_drivers and provide matching
between them and probing of the registered drivers.

The bus will support probe/remove shutdown and
suspend/resume callbacks.

Kconfig and Makefile alterations are included

Signed-off-by: Dave Ertman <[e-mail]>
Signed-off-by: Kiran Patil <[e-mail]>
Reviewed-by: Pierre-Louis Bossart <[e-mail]>
Tested-by: Andrew Bowers <[e-mail]>
Signed-off-by: Jeff Kirsher <[e-mail]>
---
 Documentation/driver-api/virtual_bus.rst |  62 +++++
 drivers/bus/Kconfig                      |  10 +
 drivers/bus/Makefile                     |   2 +
 drivers/bus/virtual_bus.c                | 279 +++++++++++++++++++++++
 include/linux/mod_devicetable.h          |   8 +
 include/linux/virtual_bus.h              |  53 +++++
 scripts/mod/devicetable-offsets.c        |   3 +
 scripts/mod/file2alias.c                 |   7 +
 8 files changed, 424 insertions(+)
 create mode 100644 Documentation/driver-api/virtual_bus.rst
 create mode 100644 drivers/bus/virtual_bus.c
 create mode 100644 include/linux/virtual_bus.h

Comments

Greg KH April 21, 2020, 8:37 a.m. UTC | #1

On Tue, Apr 21, 2020 at 01:02:27AM -0700, Jeff Kirsher wrote:
> --- /dev/null
> +++ b/Documentation/driver-api/virtual_bus.rst
> @@ -0,0 +1,62 @@
> +===============================
> +Virtual Bus Devices and Drivers
> +===============================
> +
> +See <linux/virtual_bus.h> for the models for virtbus_device and virtbus_driver.
> +This bus is meant to be a lightweight software based bus to attach generic
> +devices and drivers to so that a chunk of data can be passed between them.
> +
> +One use case example is an RDMA driver needing to connect with several
> +different types of PCI LAN devices to be able to request resources from
> +them (queue sets).  Each LAN driver that supports RDMA will register a
> +virtbus_device on the virtual bus for each physical function.  The RDMA
> +driver will register as a virtbus_driver on the virtual bus to be
> +matched up with multiple virtbus_devices and receive a pointer to a
> +struct containing the callbacks that the PCI LAN drivers support for
> +registering with them.
> +
> +Sections in this document:
> +        Virtbus devices
> +        Virtbus drivers
> +        Device Enumeration
> +        Device naming and driver binding
> +        Virtual Bus API entry points
> +
> +Virtbus devices
> +~~~~~~~~~~~~~~~
> +Virtbus_devices support the minimal device functionality.  Devices will
> +accept a name, and then, when added to the virtual bus, an automatically
> +generated index is concatenated onto it for the virtbus_device->name.
> +
> +Virtbus drivers
> +~~~~~~~~~~~~~~~
> +Virtbus drivers register with the virtual bus to be matched with virtbus
> +devices.  They expect to be registered with a probe and remove callback,
> +and also support shutdown, suspend, and resume callbacks.  They otherwise
> +follow the standard driver behavior of having discovery and enumeration
> +handled in the bus infrastructure.
> +
> +Virtbus drivers register themselves with the API entry point
> +virtbus_register_driver and unregister with virtbus_unregister_driver.
> +
> +Device Enumeration
> +~~~~~~~~~~~~~~~~~~
> +Enumeration is handled automatically by the bus infrastructure via the
> +ida_simple methods.
> +
> +Device naming and driver binding
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +The virtbus_device.dev.name is the canonical name for the device. It is
> +built from two other parts:
> +
> +        - virtbus_device.name (also used for matching).
> +        - virtbus_device.id (generated automatically from ida_simple calls)
> +
> +Virtbus device IDs are always in "<name>.<instance>" format.  Instances are
> +automatically selected through an ida_simple_get so are positive integers.
> +Name is taken from the device name field.
> +
> +Driver IDs are simple <name>.
> +
> +Need to extract the name from the Virtual Device compare to name of the
> +driver.

Why is this document even needed?

I understand the goal of documenting how to use this and such, but the
above document does none of that.  The last sentance here doesn't even
make sense to me.

How about tieing it into the kerneldoc functions that you have created
in the .c file, to create something that actually is useful?  As it is,
the above text doesn't describe anything to me that I could actually
use, did it help someone who wants to use this api that you know of?

Bad documentation is worse than no documentation for things like this...

greg k-h

Greg KH April 21, 2020, 8:47 a.m. UTC | #2

On Tue, Apr 21, 2020 at 01:02:27AM -0700, Jeff Kirsher wrote:
> +struct virtbus_driver {
> +	int (*probe)(struct virtbus_device *);
> +	int (*remove)(struct virtbus_device *);
> +	void (*shutdown)(struct virtbus_device *);
> +	int (*suspend)(struct virtbus_device *, pm_message_t);
> +	int (*resume)(struct virtbus_device *);
> +	struct device_driver driver;
> +	const struct virtbus_dev_id *id_table;
> +};

You create this type of driver, but then never use it in your
implementations that happen to create virtbus devices.  So does that
imply that you do not need virbus_drivers at all?  Why add all of this
code that never gets used?

Or perhaps you should be creating a virbus driver?  I can't tell, but as
it is, this series is not ready to be merged.

greg k-h

Kirsher, Jeffrey T April 21, 2020, 8:50 a.m. UTC | #3

> -----Original Message-----
> From: Greg KH <[e-mail]>
> Sent: Tuesday, April 21, 2020 01:38
> To: Kirsher, Jeffrey T <[e-mail]>
> Cc: [e-mail]; Ertman, David M <[e-mail]>;
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail];
> [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; Patil,
> Kiran <[e-mail]>; Bowers, AndrewX
> <[e-mail]>
> Subject: Re: [net-next v2 1/9] Implementation of Virtual Bus
> 
> On Tue, Apr 21, 2020 at 01:02:27AM -0700, Jeff Kirsher wrote:
> > --- /dev/null
> > +++ b/Documentation/driver-api/virtual_bus.rst
> > @@ -0,0 +1,62 @@
> > +===============================
> > +Virtual Bus Devices and Drivers
> > +===============================
> > +
> > +See <linux/virtual_bus.h> for the models for virtbus_device and
> virtbus_driver.
> > +This bus is meant to be a lightweight software based bus to attach
> > +generic devices and drivers to so that a chunk of data can be passed
> between them.
> > +
> > +One use case example is an RDMA driver needing to connect with
> > +several different types of PCI LAN devices to be able to request
> > +resources from them (queue sets).  Each LAN driver that supports RDMA
> > +will register a virtbus_device on the virtual bus for each physical
> > +function.  The RDMA driver will register as a virtbus_driver on the
> > +virtual bus to be matched up with multiple virtbus_devices and
> > +receive a pointer to a struct containing the callbacks that the PCI
> > +LAN drivers support for registering with them.
> > +
> > +Sections in this document:
> > +        Virtbus devices
> > +        Virtbus drivers
> > +        Device Enumeration
> > +        Device naming and driver binding
> > +        Virtual Bus API entry points
> > +
> > +Virtbus devices
> > +~~~~~~~~~~~~~~~
> > +Virtbus_devices support the minimal device functionality.  Devices
> > +will accept a name, and then, when added to the virtual bus, an
> > +automatically generated index is concatenated onto it for the
> virtbus_device->name.
> > +
> > +Virtbus drivers
> > +~~~~~~~~~~~~~~~
> > +Virtbus drivers register with the virtual bus to be matched with
> > +virtbus devices.  They expect to be registered with a probe and
> > +remove callback, and also support shutdown, suspend, and resume
> > +callbacks.  They otherwise follow the standard driver behavior of
> > +having discovery and enumeration handled in the bus infrastructure.
> > +
> > +Virtbus drivers register themselves with the API entry point
> > +virtbus_register_driver and unregister with virtbus_unregister_driver.
> > +
> > +Device Enumeration
> > +~~~~~~~~~~~~~~~~~~
> > +Enumeration is handled automatically by the bus infrastructure via
> > +the ida_simple methods.
> > +
> > +Device naming and driver binding
> > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > +The virtbus_device.dev.name is the canonical name for the device. It
> > +is built from two other parts:
> > +
> > +        - virtbus_device.name (also used for matching).
> > +        - virtbus_device.id (generated automatically from ida_simple
> > + calls)
> > +
> > +Virtbus device IDs are always in "<name>.<instance>" format.
> > +Instances are automatically selected through an ida_simple_get so are
> positive integers.
> > +Name is taken from the device name field.
> > +
> > +Driver IDs are simple <name>.
> > +
> > +Need to extract the name from the Virtual Device compare to name of
> > +the driver.
> 
> Why is this document even needed?
> 
> I understand the goal of documenting how to use this and such, but the above
> document does none of that.  The last sentance here doesn't even make sense
> to me.
> 
> How about tieing it into the kerneldoc functions that you have created in the .c
> file, to create something that actually is useful?  As it is, the above text doesn't
> describe anything to me that I could actually use, did it help someone who
> wants to use this api that you know of?
[Kirsher, Jeffrey T] 

Thank you for the feedback, I will work with David to fix the documentation so
that it is useful to you.
 
> Bad documentation is worse than no documentation for things like this...
> 
> greg k-h

Leon Romanovsky April 21, 2020, 9:08 a.m. UTC | #4

On Tue, Apr 21, 2020 at 01:02:27AM -0700, Jeff Kirsher wrote:
> From: Dave Ertman <[e-mail]>
>
> This is the initial implementation of the Virtual Bus,
> virtbus_device and virtbus_driver.  The virtual bus is
> a software based bus intended to support registering
> virtbus_devices and virtbus_drivers and provide matching
> between them and probing of the registered drivers.
>
> The bus will support probe/remove shutdown and
> suspend/resume callbacks.
>
> Kconfig and Makefile alterations are included
>
> Signed-off-by: Dave Ertman <[e-mail]>
> Signed-off-by: Kiran Patil <[e-mail]>
> Reviewed-by: Pierre-Louis Bossart <[e-mail]>
> Tested-by: Andrew Bowers <[e-mail]>
> Signed-off-by: Jeff Kirsher <[e-mail]>
> ---
>  Documentation/driver-api/virtual_bus.rst |  62 +++++
>  drivers/bus/Kconfig                      |  10 +
>  drivers/bus/Makefile                     |   2 +
>  drivers/bus/virtual_bus.c                | 279 +++++++++++++++++++++++
>  include/linux/mod_devicetable.h          |   8 +
>  include/linux/virtual_bus.h              |  53 +++++
>  scripts/mod/devicetable-offsets.c        |   3 +
>  scripts/mod/file2alias.c                 |   7 +
>  8 files changed, 424 insertions(+)
>  create mode 100644 Documentation/driver-api/virtual_bus.rst
>  create mode 100644 drivers/bus/virtual_bus.c
>  create mode 100644 include/linux/virtual_bus.h
>
> diff --git a/Documentation/driver-api/virtual_bus.rst b/Documentation/driver-api/virtual_bus.rst
> new file mode 100644
> index 000000000000..a79db0e9231e
> --- /dev/null
> +++ b/Documentation/driver-api/virtual_bus.rst
> @@ -0,0 +1,62 @@
> +===============================
> +Virtual Bus Devices and Drivers
> +===============================
> +
> +See <linux/virtual_bus.h> for the models for virtbus_device and virtbus_driver.
> +This bus is meant to be a lightweight software based bus to attach generic
> +devices and drivers to so that a chunk of data can be passed between them.
> +
> +One use case example is an RDMA driver needing to connect with several
> +different types of PCI LAN devices to be able to request resources from
> +them (queue sets).  Each LAN driver that supports RDMA will register a
> +virtbus_device on the virtual bus for each physical function.  The RDMA
> +driver will register as a virtbus_driver on the virtual bus to be
> +matched up with multiple virtbus_devices and receive a pointer to a
> +struct containing the callbacks that the PCI LAN drivers support for
> +registering with them.
> +
> +Sections in this document:
> +        Virtbus devices
> +        Virtbus drivers
> +        Device Enumeration
> +        Device naming and driver binding
> +        Virtual Bus API entry points
> +
> +Virtbus devices
> +~~~~~~~~~~~~~~~
> +Virtbus_devices support the minimal device functionality.  Devices will
> +accept a name, and then, when added to the virtual bus, an automatically
> +generated index is concatenated onto it for the virtbus_device->name.
> +
> +Virtbus drivers
> +~~~~~~~~~~~~~~~
> +Virtbus drivers register with the virtual bus to be matched with virtbus
> +devices.  They expect to be registered with a probe and remove callback,
> +and also support shutdown, suspend, and resume callbacks.  They otherwise
> +follow the standard driver behavior of having discovery and enumeration
> +handled in the bus infrastructure.
> +
> +Virtbus drivers register themselves with the API entry point
> +virtbus_register_driver and unregister with virtbus_unregister_driver.
> +
> +Device Enumeration
> +~~~~~~~~~~~~~~~~~~
> +Enumeration is handled automatically by the bus infrastructure via the
> +ida_simple methods.
> +
> +Device naming and driver binding
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +The virtbus_device.dev.name is the canonical name for the device. It is
> +built from two other parts:
> +
> +        - virtbus_device.name (also used for matching).
> +        - virtbus_device.id (generated automatically from ida_simple calls)
> +
> +Virtbus device IDs are always in "<name>.<instance>" format.  Instances are
> +automatically selected through an ida_simple_get so are positive integers.
> +Name is taken from the device name field.
> +
> +Driver IDs are simple <name>.
> +
> +Need to extract the name from the Virtual Device compare to name of the
> +driver.
> diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
> index 6d4e4497b59b..00553c78510c 100644
> --- a/drivers/bus/Kconfig
> +++ b/drivers/bus/Kconfig
> @@ -203,4 +203,14 @@ config DA8XX_MSTPRI
>  source "drivers/bus/fsl-mc/Kconfig"
>  source "drivers/bus/mhi/Kconfig"
>
> +config VIRTUAL_BUS
> +       tristate "Software based Virtual Bus"
> +       help
> +         Provides a software bus for virtbus_devices to be added to it
> +         and virtbus_drivers to be registered on it.  It matches driver
> +         and device based on id and calls the driver's probe routine.
> +         One example is the irdma driver needing to connect with various
> +         PCI LAN drivers to request resources (queues) to be able to perform
> +         its function.
> +
>  endmenu
> diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
> index 05f32cd694a4..d30828a4768c 100644
> --- a/drivers/bus/Makefile
> +++ b/drivers/bus/Makefile
> @@ -37,3 +37,5 @@ obj-$(CONFIG_DA8XX_MSTPRI)	+= da8xx-mstpri.o
>
>  # MHI
>  obj-$(CONFIG_MHI_BUS)		+= mhi/
> +
> +obj-$(CONFIG_VIRTUAL_BUS)	+= virtual_bus.o
> diff --git a/drivers/bus/virtual_bus.c b/drivers/bus/virtual_bus.c
> new file mode 100644
> index 000000000000..f5e66d110385
> --- /dev/null
> +++ b/drivers/bus/virtual_bus.c
> @@ -0,0 +1,279 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * virtual_bus.c - lightweight software based bus for virtual devices
> + *
> + * Copyright (c) 2019-2020 Intel Corporation
> + *
> + * Please see Documentation/driver-api/virtual_bus.rst for
> + * more information
> + */
> +
> +#include <linux/string.h>
> +#include <linux/virtual_bus.h>
> +#include <linux/of_irq.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/pm_domain.h>
> +#include <linux/acpi.h>
> +#include <linux/device.h>
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Virtual Bus");
> +MODULE_AUTHOR("David Ertman <[e-mail]>");
> +MODULE_AUTHOR("Kiran Patil <[e-mail]>");
> +
> +static DEFINE_IDA(virtbus_dev_ida);
> +
> +static const
> +struct virtbus_dev_id *virtbus_match_id(const struct virtbus_dev_id *id,
> +					struct virtbus_device *vdev)
> +{
> +	while (id->name[0]) {
> +		if (!strcmp(vdev->name, id->name))
> +			return id;
> +		id++;
> +	}
> +	return NULL;
> +}
> +
> +static int virtbus_match(struct device *dev, struct device_driver *drv)
> +{
> +	struct virtbus_driver *vdrv = to_virtbus_drv(drv);
> +	struct virtbus_device *vdev = to_virtbus_dev(dev);
> +
> +	return virtbus_match_id(vdrv->id_table, vdev) != NULL;
> +}
> +
> +static int virtbus_probe(struct device *dev)
> +{
> +	return dev->driver->probe(dev);
> +}
> +
> +static int virtbus_remove(struct device *dev)
> +{
> +	return dev->driver->remove(dev);
> +}
> +
> +static void virtbus_shutdown(struct device *dev)
> +{
> +	dev->driver->shutdown(dev);
> +}
> +
> +static int virtbus_suspend(struct device *dev, pm_message_t state)
> +{
> +	if (dev->driver->suspend)
> +		return dev->driver->suspend(dev, state);
> +
> +	return 0;
> +}
> +
> +static int virtbus_resume(struct device *dev)
> +{
> +	if (dev->driver->resume)
> +		return dev->driver->resume(dev);
> +
> +	return 0;

The common practice is to write it differently.
static int virtbus_resume(struct device *dev)
{
	if (!dev->driver->resume)
		return 0;

	return dev->driver->resume(dev);
}

and you are not consistent in this patch, sometimes you write
these functions in your format, sometimes in my format.

Thanks

Greg KH April 21, 2020, 9:30 a.m. UTC | #5

On Tue, Apr 21, 2020 at 08:50:47AM +0000, Kirsher, Jeffrey T wrote:
> > -----Original Message-----
> > From: Greg KH <[e-mail]>
> > Sent: Tuesday, April 21, 2020 01:38
> > To: Kirsher, Jeffrey T <[e-mail]>
> > Cc: [e-mail]; Ertman, David M <[e-mail]>;
> > [e-mail]; [e-mail]; [e-mail];
> > [e-mail]; [e-mail]; [e-mail];
> > [e-mail]; [e-mail];
> > [e-mail]; [e-mail];
> > [e-mail]; [e-mail]; [e-mail];
> > [e-mail]; [e-mail]; [e-mail];
> > [e-mail]; [e-mail]; Patil,
> > Kiran <[e-mail]>; Bowers, AndrewX
> > <[e-mail]>
> > Subject: Re: [net-next v2 1/9] Implementation of Virtual Bus
> > 
> > On Tue, Apr 21, 2020 at 01:02:27AM -0700, Jeff Kirsher wrote:
> > > --- /dev/null
> > > +++ b/Documentation/driver-api/virtual_bus.rst
> > > @@ -0,0 +1,62 @@
> > > +===============================
> > > +Virtual Bus Devices and Drivers
> > > +===============================
> > > +
> > > +See <linux/virtual_bus.h> for the models for virtbus_device and
> > virtbus_driver.
> > > +This bus is meant to be a lightweight software based bus to attach
> > > +generic devices and drivers to so that a chunk of data can be passed
> > between them.
> > > +
> > > +One use case example is an RDMA driver needing to connect with
> > > +several different types of PCI LAN devices to be able to request
> > > +resources from them (queue sets).  Each LAN driver that supports RDMA
> > > +will register a virtbus_device on the virtual bus for each physical
> > > +function.  The RDMA driver will register as a virtbus_driver on the
> > > +virtual bus to be matched up with multiple virtbus_devices and
> > > +receive a pointer to a struct containing the callbacks that the PCI
> > > +LAN drivers support for registering with them.
> > > +
> > > +Sections in this document:
> > > +        Virtbus devices
> > > +        Virtbus drivers
> > > +        Device Enumeration
> > > +        Device naming and driver binding
> > > +        Virtual Bus API entry points
> > > +
> > > +Virtbus devices
> > > +~~~~~~~~~~~~~~~
> > > +Virtbus_devices support the minimal device functionality.  Devices
> > > +will accept a name, and then, when added to the virtual bus, an
> > > +automatically generated index is concatenated onto it for the
> > virtbus_device->name.
> > > +
> > > +Virtbus drivers
> > > +~~~~~~~~~~~~~~~
> > > +Virtbus drivers register with the virtual bus to be matched with
> > > +virtbus devices.  They expect to be registered with a probe and
> > > +remove callback, and also support shutdown, suspend, and resume
> > > +callbacks.  They otherwise follow the standard driver behavior of
> > > +having discovery and enumeration handled in the bus infrastructure.
> > > +
> > > +Virtbus drivers register themselves with the API entry point
> > > +virtbus_register_driver and unregister with virtbus_unregister_driver.
> > > +
> > > +Device Enumeration
> > > +~~~~~~~~~~~~~~~~~~
> > > +Enumeration is handled automatically by the bus infrastructure via
> > > +the ida_simple methods.
> > > +
> > > +Device naming and driver binding
> > > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > +The virtbus_device.dev.name is the canonical name for the device. It
> > > +is built from two other parts:
> > > +
> > > +        - virtbus_device.name (also used for matching).
> > > +        - virtbus_device.id (generated automatically from ida_simple
> > > + calls)
> > > +
> > > +Virtbus device IDs are always in "<name>.<instance>" format.
> > > +Instances are automatically selected through an ida_simple_get so are
> > positive integers.
> > > +Name is taken from the device name field.
> > > +
> > > +Driver IDs are simple <name>.
> > > +
> > > +Need to extract the name from the Virtual Device compare to name of
> > > +the driver.
> > 
> > Why is this document even needed?
> > 
> > I understand the goal of documenting how to use this and such, but the above
> > document does none of that.  The last sentance here doesn't even make sense
> > to me.
> > 
> > How about tieing it into the kerneldoc functions that you have created in the .c
> > file, to create something that actually is useful?  As it is, the above text doesn't
> > describe anything to me that I could actually use, did it help someone who
> > wants to use this api that you know of?
> [Kirsher, Jeffrey T] 
> 
> Thank you for the feedback, I will work with David to fix the documentation so
> that it is useful to you.

Try making it useful to the person having to actually use this (i.e.
who ever needs to integrate the virtual bus code into their drivers) as
that is the proper audience for this, right?

greg k-h

Jason Gunthorpe April 21, 2020, 12:08 p.m. UTC | #6

On Tue, Apr 21, 2020 at 01:02:27AM -0700, Jeff Kirsher wrote:
> +/**
> + * virtbus_release_device - Destroy a virtbus device
> + * @_dev: device to release
> + */
> +static void virtbus_release_device(struct device *_dev)
> +{
> +	struct virtbus_device *vdev = to_virtbus_dev(_dev);
> +	int ida = vdev->id;
> +
> +	vdev->release(vdev);
> +	ida_simple_remove(&virtbus_dev_ida, ida);
> +}
> +
> +/**
> + * virtbus_register_device - add a virtual bus device
> + * @vdev: virtual bus device to add
> + */
> +int virtbus_register_device(struct virtbus_device *vdev)
> +{
> +	int ret;
> +
> +	if (!vdev->release) {
> +		dev_err(&vdev->dev, "virtbus_device MUST have a .release callback that does something.\n");
> +		return -EINVAL;
> +	}
> +	
> +	/* Don't return on error here before the device_initialize.
> +	 * All error paths out of this function must perform a
> +	 * put_device(), unless the release callback does not exist,
> +	 * so that the .release() callback is called, and thus have
> +	 * to occur after the device_initialize.
> +	 */
> +	device_initialize(&vdev->dev);
> +
> +	vdev->dev.bus = &virtual_bus_type;
> +	vdev->dev.release = virtbus_release_device;
> +
> +	/* All device IDs are automatically allocated */
> +	ret = ida_simple_get(&virtbus_dev_ida, 0, 0, GFP_KERNEL);
> +
> +	if (ret < 0) {
> +		dev_err(&vdev->dev, "get IDA idx for virtbus device failed!\n");
> +		goto device_pre_err;

This still has the problem I described, why are you resending without
fixing?

Jason

Ranjani Sridharan April 21, 2020, 3:58 p.m. UTC | #7

On Tue, 2020-04-21 at 01:02 -0700, Jeff Kirsher wrote:
> From: Dave Ertman <[e-mail]>
> 
> This is the initial implementation of the Virtual Bus,
> virtbus_device and virtbus_driver.  The virtual bus is
> a software based bus intended to support registering
> virtbus_devices and virtbus_drivers and provide matching
> between them and probing of the registered drivers.
> 
> The bus will support probe/remove shutdown and
> suspend/resume callbacks.
> 
> Kconfig and Makefile alterations are included
> 
> Signed-off-by: Dave Ertman <[e-mail]>
> Signed-off-by: Kiran Patil <[e-mail]>
> Reviewed-by: Pierre-Louis Bossart <
> [e-mail]>
> Tested-by: Andrew Bowers <[e-mail]>
> Signed-off-by: Jeff Kirsher <[e-mail]>
> ---
>  Documentation/driver-api/virtual_bus.rst |  62 +++++
>  drivers/bus/Kconfig                      |  10 +
>  drivers/bus/Makefile                     |   2 +
>  drivers/bus/virtual_bus.c                | 279
> +++++++++++++++++++++++
>  include/linux/mod_devicetable.h          |   8 +
>  include/linux/virtual_bus.h              |  53 +++++
>  scripts/mod/devicetable-offsets.c        |   3 +
>  scripts/mod/file2alias.c                 |   7 +
>  8 files changed, 424 insertions(+)
>  create mode 100644 Documentation/driver-api/virtual_bus.rst
>  create mode 100644 drivers/bus/virtual_bus.c
>  create mode 100644 include/linux/virtual_bus.h
> 
> diff --git a/Documentation/driver-api/virtual_bus.rst
> b/Documentation/driver-api/virtual_bus.rst
> new file mode 100644
> index 000000000000..a79db0e9231e
> --- /dev/null
> +++ b/Documentation/driver-api/virtual_bus.rst
> @@ -0,0 +1,62 @@
> +===============================
> +Virtual Bus Devices and Drivers
> +===============================
> +
> +See <linux/virtual_bus.h> for the models for virtbus_device and
> virtbus_driver.
> +This bus is meant to be a lightweight software based bus to attach
> generic
> +devices and drivers to so that a chunk of data can be passed between
> them.
> +
> +One use case example is an RDMA driver needing to connect with
> several
> +different types of PCI LAN devices to be able to request resources
> from
> +them (queue sets).  Each LAN driver that supports RDMA will register
> a
> +virtbus_device on the virtual bus for each physical function.  The
> RDMA
> +driver will register as a virtbus_driver on the virtual bus to be
> +matched up with multiple virtbus_devices and receive a pointer to a
> +struct containing the callbacks that the PCI LAN drivers support for
> +registering with them.
> +
> +Sections in this document:
> +        Virtbus devices
> +        Virtbus drivers
> +        Device Enumeration
> +        Device naming and driver binding
> +        Virtual Bus API entry points
> +
> +Virtbus devices
> +~~~~~~~~~~~~~~~
> +Virtbus_devices support the minimal device functionality.  Devices
> will
> +accept a name, and then, when added to the virtual bus, an
> automatically
> +generated index is concatenated onto it for the virtbus_device-
> >name.
> +
> +Virtbus drivers
> +~~~~~~~~~~~~~~~
> +Virtbus drivers register with the virtual bus to be matched with
> virtbus
> +devices.  They expect to be registered with a probe and remove
> callback,
> +and also support shutdown, suspend, and resume callbacks.  They
> otherwise
> +follow the standard driver behavior of having discovery and
> enumeration
> +handled in the bus infrastructure.
> +
> +Virtbus drivers register themselves with the API entry point
> +virtbus_register_driver and unregister with
> virtbus_unregister_driver.
> +
> +Device Enumeration
> +~~~~~~~~~~~~~~~~~~
> +Enumeration is handled automatically by the bus infrastructure via
> the
> +ida_simple methods.
> +
> +Device naming and driver binding
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +The virtbus_device.dev.name is the canonical name for the device. It
> is
> +built from two other parts:
> +
> +        - virtbus_device.name (also used for matching).
> +        - virtbus_device.id (generated automatically from ida_simple
> calls)
> +
> +Virtbus device IDs are always in "<name>.<instance>"
> format.  Instances are
> +automatically selected through an ida_simple_get so are positive
> integers.
> +Name is taken from the device name field.
> +
> +Driver IDs are simple <name>.
> +
> +Need to extract the name from the Virtual Device compare to name of
> the
> +driver.
> diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
> index 6d4e4497b59b..00553c78510c 100644
> --- a/drivers/bus/Kconfig
> +++ b/drivers/bus/Kconfig
> @@ -203,4 +203,14 @@ config DA8XX_MSTPRI
>  source "drivers/bus/fsl-mc/Kconfig"
>  source "drivers/bus/mhi/Kconfig"
>  
> +config VIRTUAL_BUS
> +       tristate "Software based Virtual Bus"
> +       help
> +         Provides a software bus for virtbus_devices to be added to
> it
> +         and virtbus_drivers to be registered on it.  It matches
> driver
> +         and device based on id and calls the driver's probe
> routine.
> +         One example is the irdma driver needing to connect with
> various
> +         PCI LAN drivers to request resources (queues) to be able to
> perform
> +         its function.
> +
>  endmenu
> diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
> index 05f32cd694a4..d30828a4768c 100644
> --- a/drivers/bus/Makefile
> +++ b/drivers/bus/Makefile
> @@ -37,3 +37,5 @@ obj-$(CONFIG_DA8XX_MSTPRI)	+= da8xx-mstpri.o
>  
>  # MHI
>  obj-$(CONFIG_MHI_BUS)		+= mhi/
> +
> +obj-$(CONFIG_VIRTUAL_BUS)	+= virtual_bus.o
> diff --git a/drivers/bus/virtual_bus.c b/drivers/bus/virtual_bus.c
> new file mode 100644
> index 000000000000..f5e66d110385
> --- /dev/null
> +++ b/drivers/bus/virtual_bus.c
> @@ -0,0 +1,279 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * virtual_bus.c - lightweight software based bus for virtual
> devices
> + *
> + * Copyright (c) 2019-2020 Intel Corporation
> + *
> + * Please see Documentation/driver-api/virtual_bus.rst for
> + * more information
> + */
> +
> +#include <linux/string.h>
> +#include <linux/virtual_bus.h>
> +#include <linux/of_irq.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/pm_domain.h>
> +#include <linux/acpi.h>
> +#include <linux/device.h>
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Virtual Bus");
> +MODULE_AUTHOR("David Ertman <[e-mail]>");
> +MODULE_AUTHOR("Kiran Patil <[e-mail]>");
> +
> +static DEFINE_IDA(virtbus_dev_ida);
> +
> +static const
> +struct virtbus_dev_id *virtbus_match_id(const struct virtbus_dev_id
> *id,
> +					struct virtbus_device *vdev)
> +{
> +	while (id->name[0]) {
> +		if (!strcmp(vdev->name, id->name))
> +			return id;
> +		id++;
> +	}
> +	return NULL;
> +}
> +
> +static int virtbus_match(struct device *dev, struct device_driver
> *drv)
> +{
> +	struct virtbus_driver *vdrv = to_virtbus_drv(drv);
> +	struct virtbus_device *vdev = to_virtbus_dev(dev);
> +
> +	return virtbus_match_id(vdrv->id_table, vdev) != NULL;
> +}
> +
> +static int virtbus_probe(struct device *dev)
> +{
> +	return dev->driver->probe(dev);
> +}
> +
> +static int virtbus_remove(struct device *dev)
> +{
> +	return dev->driver->remove(dev);
> +}
> +
> +static void virtbus_shutdown(struct device *dev)
> +{
> +	dev->driver->shutdown(dev);
> +}
> +
> +static int virtbus_suspend(struct device *dev, pm_message_t state)
> +{
> +	if (dev->driver->suspend)
> +		return dev->driver->suspend(dev, state);
> +
> +	return 0;
> +}
> +
> +static int virtbus_resume(struct device *dev)
> +{
> +	if (dev->driver->resume)
> +		return dev->driver->resume(dev);
> +
> +	return 0;
> +}
> +
> +struct bus_type virtual_bus_type = {
> +	.name = "virtbus",
> +	.match = virtbus_match,
> +	.probe = virtbus_probe,
> +	.remove = virtbus_remove,
> +	.shutdown = virtbus_shutdown,
> +	.suspend = virtbus_suspend,
> +	.resume = virtbus_resume,
> +};
> +
> +/**
> + * virtbus_release_device - Destroy a virtbus device
> + * @_dev: device to release
> + */
> +static void virtbus_release_device(struct device *_dev)
> +{
> +	struct virtbus_device *vdev = to_virtbus_dev(_dev);
> +	int ida = vdev->id;
> +
> +	vdev->release(vdev);
> +	ida_simple_remove(&virtbus_dev_ida, ida);
> +}
> +
> +/**
> + * virtbus_register_device - add a virtual bus device
> + * @vdev: virtual bus device to add
> + */
> +int virtbus_register_device(struct virtbus_device *vdev)
> +{
> +	int ret;
> +
> +	if (!vdev->release) {
> +		dev_err(&vdev->dev, "virtbus_device MUST have a
> .release callback that does something.\n");
> +		return -EINVAL;
> +	}
> +	
> +	/* Don't return on error here before the device_initialize.
> +	 * All error paths out of this function must perform a
> +	 * put_device(), unless the release callback does not exist,
> +	 * so that the .release() callback is called, and thus have
> +	 * to occur after the device_initialize.
> +	 */
> +	device_initialize(&vdev->dev);
> +
> +	vdev->dev.bus = &virtual_bus_type;
> +	vdev->dev.release = virtbus_release_device;
> +
> +	/* All device IDs are automatically allocated */
> +	ret = ida_simple_get(&virtbus_dev_ida, 0, 0, GFP_KERNEL);
> +
> +	if (ret < 0) {
> +		dev_err(&vdev->dev, "get IDA idx for virtbus device
> failed!\n");
> +		goto device_pre_err;
> +	}
> +
> +	vdev->id = ret;
> +
> +	ret = dev_set_name(&vdev->dev, "%s.%d", vdev->name, vdev->id);
> +	if (ret) {
> +		dev_err(&vdev->dev, "dev_set_name failed for
> device\n");
> +		goto device_add_err;
> +	}
> +
> +	dev_dbg(&vdev->dev, "Registering virtbus device '%s'\n",
> +		dev_name(&vdev->dev));
> +
> +	ret = device_add(&vdev->dev);
> +	if (ret)
> +		goto device_add_err;
> +
> +	return 0;
> +
> +device_add_err:
> +	ida_simple_remove(&virtbus_dev_ida, vdev->id);
> +
> +device_pre_err:
> +	dev_err(&vdev->dev, "Add device to virtbus failed!: %d\n",
> ret);
> +	put_device(&vdev->dev);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(virtbus_register_device);
> +
> +/**
> + * virtbus_unregister_device - remove a virtual bus device
> + * @vdev: virtual bus device we are removing
> + */
> +void virtbus_unregister_device(struct virtbus_device *vdev)
> +{
> +	device_unregister(&vdev->dev);
> +}
> +EXPORT_SYMBOL_GPL(virtbus_unregister_device);
> +
> +static int virtbus_probe_driver(struct device *_dev)
> +{
> +	struct virtbus_driver *vdrv = to_virtbus_drv(_dev->driver);
> +	struct virtbus_device *vdev = to_virtbus_dev(_dev);
> +	int ret;
> +
> +	ret = dev_pm_domain_attach(_dev, true);
> +	if (ret) {
> +		dev_warn(_dev, "Failed to attach to PM Domain : %d\n",
> ret);
> +		return ret;
> +	}
> +
> +	ret = vdrv->probe(vdev);
> +	if (ret) {
> +		dev_err(&vdev->dev, "Probe returned error\n");
> +		dev_pm_domain_detach(_dev, true);
> +	}
> +
> +	return ret;
> +}
> +
> +static int virtbus_remove_driver(struct device *_dev)
> +{
> +	struct virtbus_driver *vdrv = to_virtbus_drv(_dev->driver);
> +	struct virtbus_device *vdev = to_virtbus_dev(_dev);
> +	int ret = 0;
> +
> +	ret = vdrv->remove(vdev);
> +	dev_pm_domain_detach(_dev, true);
> +
> +	return ret;
> +}
> +
> +static void virtbus_shutdown_driver(struct device *_dev)
> +{
> +	struct virtbus_driver *vdrv = to_virtbus_drv(_dev->driver);
> +	struct virtbus_device *vdev = to_virtbus_dev(_dev);
> +
> +	vdrv->shutdown(vdev);
> +}
> +
> +static int virtbus_suspend_driver(struct device *_dev, pm_message_t
> state)
> +{
> +	struct virtbus_driver *vdrv = to_virtbus_drv(_dev->driver);
> +	struct virtbus_device *vdev = to_virtbus_dev(_dev);
> +
> +	if (vdrv->suspend)
> +		return vdrv->suspend(vdev, state);
> +
> +	return 0;
> +}
> +
> +static int virtbus_resume_driver(struct device *_dev)
> +{
> +	struct virtbus_driver *vdrv = to_virtbus_drv(_dev->driver);
> +	struct virtbus_device *vdev = to_virtbus_dev(_dev);
> +
> +	if (vdrv->resume)
> +		return vdrv->resume(vdev);
> +
> +	return 0;
> +}
> +
> +/**
> + * __virtbus_register_driver - register a driver for virtual bus
> devices
> + * @vdrv: virtbus_driver structure
> + * @owner: owning module/driver
> + */
> +int __virtbus_register_driver(struct virtbus_driver *vdrv, struct
> module *owner)
> +{
> +	if (!vdrv->probe || !vdrv->remove || !vdrv->shutdown || !vdrv-
> >id_table)
> +		return -EINVAL;
> +
> +	vdrv->driver.owner = owner;
> +	vdrv->driver.bus = &virtual_bus_type;
> +	vdrv->driver.probe = virtbus_probe_driver;
> +	vdrv->driver.remove = virtbus_remove_driver;
> +	vdrv->driver.shutdown = virtbus_shutdown_driver;
> +	vdrv->driver.suspend = virtbus_suspend_driver;
> +	vdrv->driver.resume = virtbus_resume_driver;
> +
> +	return driver_register(&vdrv->driver);
> +}
> +EXPORT_SYMBOL_GPL(__virtbus_register_driver);
> +
> +/**
> + * virtbus_unregister_driver - unregister a driver for virtual bus
> devices
> + * @vdrv: virtbus_driver structure
> + */
> +void virtbus_unregister_driver(struct virtbus_driver *vdrv)
> +{
> +	driver_unregister(&vdrv->driver);
> +}
> +EXPORT_SYMBOL_GPL(virtbus_unregister_driver);
> +
> +static int __init virtual_bus_init(void)
> +{
> +	return bus_register(&virtual_bus_type);
> +}
> +
> +static void __exit virtual_bus_exit(void)
> +{
> +	bus_unregister(&virtual_bus_type);
> +	ida_destroy(&virtbus_dev_ida);
> +}
> +
> +module_init(virtual_bus_init);
> +module_exit(virtual_bus_exit);
> diff --git a/include/linux/mod_devicetable.h
> b/include/linux/mod_devicetable.h
> index 4c2ddd0941a7..60bcfe75fb94 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -832,4 +832,12 @@ struct mhi_device_id {
>  	kernel_ulong_t driver_data;
>  };
>  
> +#define VIRTBUS_NAME_SIZE 20
> +#define VIRTBUS_MODULE_PREFIX "virtbus:"
> +
> +struct virtbus_dev_id {
> +	char name[VIRTBUS_NAME_SIZE];
> +	kernel_ulong_t driver_data;
> +};
> +
>  #endif /* LINUX_MOD_DEVICETABLE_H */
> diff --git a/include/linux/virtual_bus.h
> b/include/linux/virtual_bus.h
> new file mode 100644
> index 000000000000..5c7b38db1b47
> --- /dev/null
> +++ b/include/linux/virtual_bus.h
> @@ -0,0 +1,53 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * virtual_bus.h - lightweight software bus
> + *
> + * Copyright (c) 2019-2020 Intel Corporation
> + *
> + * Please see Documentation/driver-api/virtual_bus.rst for more
> information
> + */
> +
> +#ifndef _VIRTUAL_BUS_H_
> +#define _VIRTUAL_BUS_H_
> +
> +#include <linux/device.h>
> +
> +struct virtbus_device {
> +	struct device dev;
> +	const char *name;
> +	void (*release)(struct virtbus_device *);
> +	u32 id;
> +};
> +
> +struct virtbus_driver {
> +	int (*probe)(struct virtbus_device *);
> +	int (*remove)(struct virtbus_device *);
> +	void (*shutdown)(struct virtbus_device *);
> +	int (*suspend)(struct virtbus_device *, pm_message_t);
> +	int (*resume)(struct virtbus_device *);
> +	struct device_driver driver;
> +	const struct virtbus_dev_id *id_table;

Sorry my feedback for v1 was a bit late but I'd still like to know if
we need the callbacks both here and in bus_type.

I'm seeing the following message when I experimented with the
virtualbus implementation in the audio driver. Removing them from the
bus_type gets rid of this message.
"Driver 'sof-ipc-test-virtbus-drv' needs updating - please use bus_type
methods"

Thanks,
Ranjani

Ertman, David M April 21, 2020, 11:27 p.m. UTC | #8

> -----Original Message-----
> From: Leon Romanovsky <[e-mail]>
> Sent: Tuesday, April 21, 2020 2:08 AM
> To: Kirsher, Jeffrey T <[e-mail]>
> Cc: [e-mail]; [e-mail]; Ertman, David M
> <[e-mail]>; [e-mail]; linux-
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail];
> Patil, Kiran <[e-mail]>; Bowers, AndrewX
> <[e-mail]>
> Subject: Re: [net-next v2 1/9] Implementation of Virtual Bus
> 
> On Tue, Apr 21, 2020 at 01:02:27AM -0700, Jeff Kirsher wrote:
> > From: Dave Ertman <[e-mail]>
> >
> > This is the initial implementation of the Virtual Bus,
> > virtbus_device and virtbus_driver.  The virtual bus is
> > a software based bus intended to support registering
> > virtbus_devices and virtbus_drivers and provide matching
> > between them and probing of the registered drivers.
> >
> > The bus will support probe/remove shutdown and
> > suspend/resume callbacks.
> >
> > Kconfig and Makefile alterations are included
> >
> > Signed-off-by: Dave Ertman <[e-mail]>
> > Signed-off-by: Kiran Patil <[e-mail]>
> > Reviewed-by: Pierre-Louis Bossart <[e-mail]>
> > Tested-by: Andrew Bowers <[e-mail]>
> > Signed-off-by: Jeff Kirsher <[e-mail]>
> > ---
> >  Documentation/driver-api/virtual_bus.rst |  62 +++++
> >  drivers/bus/Kconfig                      |  10 +
> >  drivers/bus/Makefile                     |   2 +
> >  drivers/bus/virtual_bus.c                | 279 +++++++++++++++++++++++
> >  include/linux/mod_devicetable.h          |   8 +
> >  include/linux/virtual_bus.h              |  53 +++++
> >  scripts/mod/devicetable-offsets.c        |   3 +
> >  scripts/mod/file2alias.c                 |   7 +
> >  8 files changed, 424 insertions(+)
> >  create mode 100644 Documentation/driver-api/virtual_bus.rst
> >  create mode 100644 drivers/bus/virtual_bus.c
> >  create mode 100644 include/linux/virtual_bus.h
> >
> > diff --git a/Documentation/driver-api/virtual_bus.rst
> b/Documentation/driver-api/virtual_bus.rst
> > new file mode 100644
> > index 000000000000..a79db0e9231e
> > --- /dev/null
> > +++ b/Documentation/driver-api/virtual_bus.rst
> > @@ -0,0 +1,62 @@
> > +===============================
> > +Virtual Bus Devices and Drivers
> > +===============================
> > +
> > +See <linux/virtual_bus.h> for the models for virtbus_device and
> virtbus_driver.
> > +This bus is meant to be a lightweight software based bus to attach generic
> > +devices and drivers to so that a chunk of data can be passed between
> them.
> > +
> > +One use case example is an RDMA driver needing to connect with several
> > +different types of PCI LAN devices to be able to request resources from
> > +them (queue sets).  Each LAN driver that supports RDMA will register a
> > +virtbus_device on the virtual bus for each physical function.  The RDMA
> > +driver will register as a virtbus_driver on the virtual bus to be
> > +matched up with multiple virtbus_devices and receive a pointer to a
> > +struct containing the callbacks that the PCI LAN drivers support for
> > +registering with them.
> > +
> > +Sections in this document:
> > +        Virtbus devices
> > +        Virtbus drivers
> > +        Device Enumeration
> > +        Device naming and driver binding
> > +        Virtual Bus API entry points
> > +
> > +Virtbus devices
> > +~~~~~~~~~~~~~~~
> > +Virtbus_devices support the minimal device functionality.  Devices will
> > +accept a name, and then, when added to the virtual bus, an automatically
> > +generated index is concatenated onto it for the virtbus_device->name.
> > +
> > +Virtbus drivers
> > +~~~~~~~~~~~~~~~
> > +Virtbus drivers register with the virtual bus to be matched with virtbus
> > +devices.  They expect to be registered with a probe and remove callback,
> > +and also support shutdown, suspend, and resume callbacks.  They
> otherwise
> > +follow the standard driver behavior of having discovery and enumeration
> > +handled in the bus infrastructure.
> > +
> > +Virtbus drivers register themselves with the API entry point
> > +virtbus_register_driver and unregister with virtbus_unregister_driver.
> > +
> > +Device Enumeration
> > +~~~~~~~~~~~~~~~~~~
> > +Enumeration is handled automatically by the bus infrastructure via the
> > +ida_simple methods.
> > +
> > +Device naming and driver binding
> > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > +The virtbus_device.dev.name is the canonical name for the device. It is
> > +built from two other parts:
> > +
> > +        - virtbus_device.name (also used for matching).
> > +        - virtbus_device.id (generated automatically from ida_simple calls)
> > +
> > +Virtbus device IDs are always in "<name>.<instance>" format.  Instances
> are
> > +automatically selected through an ida_simple_get so are positive integers.
> > +Name is taken from the device name field.
> > +
> > +Driver IDs are simple <name>.
> > +
> > +Need to extract the name from the Virtual Device compare to name of
> the
> > +driver.
> > diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
> > index 6d4e4497b59b..00553c78510c 100644
> > --- a/drivers/bus/Kconfig
> > +++ b/drivers/bus/Kconfig
> > @@ -203,4 +203,14 @@ config DA8XX_MSTPRI
> >  source "drivers/bus/fsl-mc/Kconfig"
> >  source "drivers/bus/mhi/Kconfig"
> >
> > +config VIRTUAL_BUS
> > +       tristate "Software based Virtual Bus"
> > +       help
> > +         Provides a software bus for virtbus_devices to be added to it
> > +         and virtbus_drivers to be registered on it.  It matches driver
> > +         and device based on id and calls the driver's probe routine.
> > +         One example is the irdma driver needing to connect with various
> > +         PCI LAN drivers to request resources (queues) to be able to perform
> > +         its function.
> > +
> >  endmenu
> > diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
> > index 05f32cd694a4..d30828a4768c 100644
> > --- a/drivers/bus/Makefile
> > +++ b/drivers/bus/Makefile
> > @@ -37,3 +37,5 @@ obj-$(CONFIG_DA8XX_MSTPRI)	+= da8xx-mstpri.o
> >
> >  # MHI
> >  obj-$(CONFIG_MHI_BUS)		+= mhi/
> > +
> > +obj-$(CONFIG_VIRTUAL_BUS)	+= virtual_bus.o
> > diff --git a/drivers/bus/virtual_bus.c b/drivers/bus/virtual_bus.c
> > new file mode 100644
> > index 000000000000..f5e66d110385
> > --- /dev/null
> > +++ b/drivers/bus/virtual_bus.c
> > @@ -0,0 +1,279 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * virtual_bus.c - lightweight software based bus for virtual devices
> > + *
> > + * Copyright (c) 2019-2020 Intel Corporation
> > + *
> > + * Please see Documentation/driver-api/virtual_bus.rst for
> > + * more information
> > + */
> > +
> > +#include <linux/string.h>
> > +#include <linux/virtual_bus.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/module.h>
> > +#include <linux/init.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/pm_domain.h>
> > +#include <linux/acpi.h>
> > +#include <linux/device.h>
> > +
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_DESCRIPTION("Virtual Bus");
> > +MODULE_AUTHOR("David Ertman <[e-mail]>");
> > +MODULE_AUTHOR("Kiran Patil <[e-mail]>");
> > +
> > +static DEFINE_IDA(virtbus_dev_ida);
> > +
> > +static const
> > +struct virtbus_dev_id *virtbus_match_id(const struct virtbus_dev_id *id,
> > +					struct virtbus_device *vdev)
> > +{
> > +	while (id->name[0]) {
> > +		if (!strcmp(vdev->name, id->name))
> > +			return id;
> > +		id++;
> > +	}
> > +	return NULL;
> > +}
> > +
> > +static int virtbus_match(struct device *dev, struct device_driver *drv)
> > +{
> > +	struct virtbus_driver *vdrv = to_virtbus_drv(drv);
> > +	struct virtbus_device *vdev = to_virtbus_dev(dev);
> > +
> > +	return virtbus_match_id(vdrv->id_table, vdev) != NULL;
> > +}
> > +
> > +static int virtbus_probe(struct device *dev)
> > +{
> > +	return dev->driver->probe(dev);
> > +}
> > +
> > +static int virtbus_remove(struct device *dev)
> > +{
> > +	return dev->driver->remove(dev);
> > +}
> > +
> > +static void virtbus_shutdown(struct device *dev)
> > +{
> > +	dev->driver->shutdown(dev);
> > +}
> > +
> > +static int virtbus_suspend(struct device *dev, pm_message_t state)
> > +{
> > +	if (dev->driver->suspend)
> > +		return dev->driver->suspend(dev, state);
> > +
> > +	return 0;
> > +}
> > +
> > +static int virtbus_resume(struct device *dev)
> > +{
> > +	if (dev->driver->resume)
> > +		return dev->driver->resume(dev);
> > +
> > +	return 0;
> 
> The common practice is to write it differently.
> static int virtbus_resume(struct device *dev)
> {
> 	if (!dev->driver->resume)
> 		return 0;
> 
> 	return dev->driver->resume(dev);
> }
> 
> and you are not consistent in this patch, sometimes you write
> these functions in your format, sometimes in my format.
> 
> Thanks

Changed functions to match common practice

-DaveE

Ertman, David M April 21, 2020, 11:27 p.m. UTC | #9

Rewrote the Documentation with more of an idea to helping someone
doing an implementation - please let me know if it is better / worse, needs some
tweaking, or just axe it all together.

Thanks for all of the feedback!!

-DaveE

> -----Original Message-----
> From: Greg KH <[e-mail]>
> Sent: Tuesday, April 21, 2020 2:30 AM
> To: Kirsher, Jeffrey T <[e-mail]>
> Cc: [e-mail]; Ertman, David M <[e-mail]>;
> [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail];
> [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; pierre-
> [e-mail]; Patil, Kiran <[e-mail]>; Bowers,
> AndrewX <[e-mail]>
> Subject: Re: [net-next v2 1/9] Implementation of Virtual Bus
> 
> On Tue, Apr 21, 2020 at 08:50:47AM +0000, Kirsher, Jeffrey T wrote:
> > > -----Original Message-----
> > > From: Greg KH <[e-mail]>
> > > Sent: Tuesday, April 21, 2020 01:38
> > > To: Kirsher, Jeffrey T <[e-mail]>
> > > Cc: [e-mail]; Ertman, David M
> <[e-mail]>;
> > > [e-mail]; [e-mail];
> [e-mail];
> > > [e-mail]; [e-mail]; [e-mail];
> > > [e-mail]; [e-mail];
> > > [e-mail]; [e-mail];
> > > [e-mail]; [e-mail];
> [e-mail];
> > > [e-mail]; [e-mail]; [e-mail];
> > > [e-mail]; [e-mail];
> Patil,
> > > Kiran <[e-mail]>; Bowers, AndrewX
> > > <[e-mail]>
> > > Subject: Re: [net-next v2 1/9] Implementation of Virtual Bus
> > >
> > > On Tue, Apr 21, 2020 at 01:02:27AM -0700, Jeff Kirsher wrote:
> > > > --- /dev/null
> > > > +++ b/Documentation/driver-api/virtual_bus.rst
> > > > @@ -0,0 +1,62 @@
> > > > +===============================
> > > > +Virtual Bus Devices and Drivers
> > > > +===============================
> > > > +
> > > > +See <linux/virtual_bus.h> for the models for virtbus_device and
> > > virtbus_driver.
> > > > +This bus is meant to be a lightweight software based bus to attach
> > > > +generic devices and drivers to so that a chunk of data can be passed
> > > between them.
> > > > +
> > > > +One use case example is an RDMA driver needing to connect with
> > > > +several different types of PCI LAN devices to be able to request
> > > > +resources from them (queue sets).  Each LAN driver that supports
> RDMA
> > > > +will register a virtbus_device on the virtual bus for each physical
> > > > +function.  The RDMA driver will register as a virtbus_driver on the
> > > > +virtual bus to be matched up with multiple virtbus_devices and
> > > > +receive a pointer to a struct containing the callbacks that the PCI
> > > > +LAN drivers support for registering with them.
> > > > +
> > > > +Sections in this document:
> > > > +        Virtbus devices
> > > > +        Virtbus drivers
> > > > +        Device Enumeration
> > > > +        Device naming and driver binding
> > > > +        Virtual Bus API entry points
> > > > +
> > > > +Virtbus devices
> > > > +~~~~~~~~~~~~~~~
> > > > +Virtbus_devices support the minimal device functionality.  Devices
> > > > +will accept a name, and then, when added to the virtual bus, an
> > > > +automatically generated index is concatenated onto it for the
> > > virtbus_device->name.
> > > > +
> > > > +Virtbus drivers
> > > > +~~~~~~~~~~~~~~~
> > > > +Virtbus drivers register with the virtual bus to be matched with
> > > > +virtbus devices.  They expect to be registered with a probe and
> > > > +remove callback, and also support shutdown, suspend, and resume
> > > > +callbacks.  They otherwise follow the standard driver behavior of
> > > > +having discovery and enumeration handled in the bus infrastructure.
> > > > +
> > > > +Virtbus drivers register themselves with the API entry point
> > > > +virtbus_register_driver and unregister with virtbus_unregister_driver.
> > > > +
> > > > +Device Enumeration
> > > > +~~~~~~~~~~~~~~~~~~
> > > > +Enumeration is handled automatically by the bus infrastructure via
> > > > +the ida_simple methods.
> > > > +
> > > > +Device naming and driver binding
> > > > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > +The virtbus_device.dev.name is the canonical name for the device. It
> > > > +is built from two other parts:
> > > > +
> > > > +        - virtbus_device.name (also used for matching).
> > > > +        - virtbus_device.id (generated automatically from ida_simple
> > > > + calls)
> > > > +
> > > > +Virtbus device IDs are always in "<name>.<instance>" format.
> > > > +Instances are automatically selected through an ida_simple_get so are
> > > positive integers.
> > > > +Name is taken from the device name field.
> > > > +
> > > > +Driver IDs are simple <name>.
> > > > +
> > > > +Need to extract the name from the Virtual Device compare to name of
> > > > +the driver.
> > >
> > > Why is this document even needed?
> > >
> > > I understand the goal of documenting how to use this and such, but the
> above
> > > document does none of that.  The last sentance here doesn't even make
> sense
> > > to me.
> > >
> > > How about tieing it into the kerneldoc functions that you have created in
> the .c
> > > file, to create something that actually is useful?  As it is, the above text
> doesn't
> > > describe anything to me that I could actually use, did it help someone
> who
> > > wants to use this api that you know of?
> > [Kirsher, Jeffrey T]
> >
> > Thank you for the feedback, I will work with David to fix the documentation
> so
> > that it is useful to you.
> 
> Try making it useful to the person having to actually use this (i.e.
> who ever needs to integrate the virtual bus code into their drivers) as
> that is the proper audience for this, right?
> 
> greg k-h

Ertman, David M April 21, 2020, 11:27 p.m. UTC | #10

> -----Original Message-----
> From: Jason Gunthorpe <[e-mail]>
> Sent: Tuesday, April 21, 2020 5:09 AM
> To: Kirsher, Jeffrey T <[e-mail]>
> Cc: [e-mail]; [e-mail]; Ertman, David M
> <[e-mail]>; [e-mail]; linux-
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail];
> [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; pierre-
> [e-mail]; Patil, Kiran <[e-mail]>; Bowers,
> AndrewX <[e-mail]>
> Subject: Re: [net-next v2 1/9] Implementation of Virtual Bus
> 
> On Tue, Apr 21, 2020 at 01:02:27AM -0700, Jeff Kirsher wrote:
> > +/**
> > + * virtbus_release_device - Destroy a virtbus device
> > + * @_dev: device to release
> > + */
> > +static void virtbus_release_device(struct device *_dev)
> > +{
> > +	struct virtbus_device *vdev = to_virtbus_dev(_dev);
> > +	int ida = vdev->id;
> > +
> > +	vdev->release(vdev);
> > +	ida_simple_remove(&virtbus_dev_ida, ida);
> > +}
> > +
> > +/**
> > + * virtbus_register_device - add a virtual bus device
> > + * @vdev: virtual bus device to add
> > + */
> > +int virtbus_register_device(struct virtbus_device *vdev)
> > +{
> > +	int ret;
> > +
> > +	if (!vdev->release) {
> > +		dev_err(&vdev->dev, "virtbus_device MUST have a .release
> callback that does something.\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* Don't return on error here before the device_initialize.
> > +	 * All error paths out of this function must perform a
> > +	 * put_device(), unless the release callback does not exist,
> > +	 * so that the .release() callback is called, and thus have
> > +	 * to occur after the device_initialize.
> > +	 */
> > +	device_initialize(&vdev->dev);
> > +
> > +	vdev->dev.bus = &virtual_bus_type;
> > +	vdev->dev.release = virtbus_release_device;
> > +
> > +	/* All device IDs are automatically allocated */
> > +	ret = ida_simple_get(&virtbus_dev_ida, 0, 0, GFP_KERNEL);
> > +
> > +	if (ret < 0) {
> > +		dev_err(&vdev->dev, "get IDA idx for virtbus device
> failed!\n");
> > +		goto device_pre_err;
> 
> This still has the problem I described, why are you resending without
> fixing?
> 
> Jason

Fix should be in the next revision - sorry for the confusion.

-DaveE

Patch

diff --git a/Documentation/driver-api/virtual_bus.rst b/Documentation/driver-api/virtual_bus.rst
new file mode 100644
index 000000000000..a79db0e9231e
--- /dev/null
+++ b/Documentation/driver-api/virtual_bus.rst
@@ -0,0 +1,62 @@ 
+===============================
+Virtual Bus Devices and Drivers
+===============================
+
+See <linux/virtual_bus.h> for the models for virtbus_device and virtbus_driver.
+This bus is meant to be a lightweight software based bus to attach generic
+devices and drivers to so that a chunk of data can be passed between them.
+
+One use case example is an RDMA driver needing to connect with several
+different types of PCI LAN devices to be able to request resources from
+them (queue sets).  Each LAN driver that supports RDMA will register a
+virtbus_device on the virtual bus for each physical function.  The RDMA
+driver will register as a virtbus_driver on the virtual bus to be
+matched up with multiple virtbus_devices and receive a pointer to a
+struct containing the callbacks that the PCI LAN drivers support for
+registering with them.
+
+Sections in this document:
+        Virtbus devices
+        Virtbus drivers
+        Device Enumeration
+        Device naming and driver binding
+        Virtual Bus API entry points
+
+Virtbus devices
+~~~~~~~~~~~~~~~
+Virtbus_devices support the minimal device functionality.  Devices will
+accept a name, and then, when added to the virtual bus, an automatically
+generated index is concatenated onto it for the virtbus_device->name.
+
+Virtbus drivers
+~~~~~~~~~~~~~~~
+Virtbus drivers register with the virtual bus to be matched with virtbus
+devices.  They expect to be registered with a probe and remove callback,
+and also support shutdown, suspend, and resume callbacks.  They otherwise
+follow the standard driver behavior of having discovery and enumeration
+handled in the bus infrastructure.
+
+Virtbus drivers register themselves with the API entry point
+virtbus_register_driver and unregister with virtbus_unregister_driver.
+
+Device Enumeration
+~~~~~~~~~~~~~~~~~~
+Enumeration is handled automatically by the bus infrastructure via the
+ida_simple methods.
+
+Device naming and driver binding
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+The virtbus_device.dev.name is the canonical name for the device. It is
+built from two other parts:
+
+        - virtbus_device.name (also used for matching).
+        - virtbus_device.id (generated automatically from ida_simple calls)
+
+Virtbus device IDs are always in "<name>.<instance>" format.  Instances are
+automatically selected through an ida_simple_get so are positive integers.
+Name is taken from the device name field.
+
+Driver IDs are simple <name>.
+
+Need to extract the name from the Virtual Device compare to name of the
+driver.
diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
index 6d4e4497b59b..00553c78510c 100644
--- a/drivers/bus/Kconfig
+++ b/drivers/bus/Kconfig
@@ -203,4 +203,14 @@  config DA8XX_MSTPRI
 source "drivers/bus/fsl-mc/Kconfig"
 source "drivers/bus/mhi/Kconfig"
 
+config VIRTUAL_BUS
+       tristate "Software based Virtual Bus"
+       help
+         Provides a software bus for virtbus_devices to be added to it
+         and virtbus_drivers to be registered on it.  It matches driver
+         and device based on id and calls the driver's probe routine.
+         One example is the irdma driver needing to connect with various
+         PCI LAN drivers to request resources (queues) to be able to perform
+         its function.
+
 endmenu
diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
index 05f32cd694a4..d30828a4768c 100644
--- a/drivers/bus/Makefile
+++ b/drivers/bus/Makefile
@@ -37,3 +37,5 @@  obj-$(CONFIG_DA8XX_MSTPRI)	+= da8xx-mstpri.o
 
 # MHI
 obj-$(CONFIG_MHI_BUS)		+= mhi/
+
+obj-$(CONFIG_VIRTUAL_BUS)	+= virtual_bus.o
diff --git a/drivers/bus/virtual_bus.c b/drivers/bus/virtual_bus.c
new file mode 100644
index 000000000000..f5e66d110385
--- /dev/null
+++ b/drivers/bus/virtual_bus.c
@@ -0,0 +1,279 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * virtual_bus.c - lightweight software based bus for virtual devices
+ *
+ * Copyright (c) 2019-2020 Intel Corporation
+ *
+ * Please see Documentation/driver-api/virtual_bus.rst for
+ * more information
+ */
+
+#include <linux/string.h>
+#include <linux/virtual_bus.h>
+#include <linux/of_irq.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/pm_runtime.h>
+#include <linux/pm_domain.h>
+#include <linux/acpi.h>
+#include <linux/device.h>
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Virtual Bus");
+MODULE_AUTHOR("David Ertman <[e-mail]>");
+MODULE_AUTHOR("Kiran Patil <[e-mail]>");
+
+static DEFINE_IDA(virtbus_dev_ida);
+
+static const
+struct virtbus_dev_id *virtbus_match_id(const struct virtbus_dev_id *id,
+					struct virtbus_device *vdev)
+{
+	while (id->name[0]) {
+		if (!strcmp(vdev->name, id->name))
+			return id;
+		id++;
+	}
+	return NULL;
+}
+
+static int virtbus_match(struct device *dev, struct device_driver *drv)
+{
+	struct virtbus_driver *vdrv = to_virtbus_drv(drv);
+	struct virtbus_device *vdev = to_virtbus_dev(dev);
+
+	return virtbus_match_id(vdrv->id_table, vdev) != NULL;
+}
+
+static int virtbus_probe(struct device *dev)
+{
+	return dev->driver->probe(dev);
+}
+
+static int virtbus_remove(struct device *dev)
+{
+	return dev->driver->remove(dev);
+}
+
+static void virtbus_shutdown(struct device *dev)
+{
+	dev->driver->shutdown(dev);
+}
+
+static int virtbus_suspend(struct device *dev, pm_message_t state)
+{
+	if (dev->driver->suspend)
+		return dev->driver->suspend(dev, state);
+
+	return 0;
+}
+
+static int virtbus_resume(struct device *dev)
+{
+	if (dev->driver->resume)
+		return dev->driver->resume(dev);
+
+	return 0;
+}
+
+struct bus_type virtual_bus_type = {
+	.name = "virtbus",
+	.match = virtbus_match,
+	.probe = virtbus_probe,
+	.remove = virtbus_remove,
+	.shutdown = virtbus_shutdown,
+	.suspend = virtbus_suspend,
+	.resume = virtbus_resume,
+};
+
+/**
+ * virtbus_release_device - Destroy a virtbus device
+ * @_dev: device to release
+ */
+static void virtbus_release_device(struct device *_dev)
+{
+	struct virtbus_device *vdev = to_virtbus_dev(_dev);
+	int ida = vdev->id;
+
+	vdev->release(vdev);
+	ida_simple_remove(&virtbus_dev_ida, ida);
+}
+
+/**
+ * virtbus_register_device - add a virtual bus device
+ * @vdev: virtual bus device to add
+ */
+int virtbus_register_device(struct virtbus_device *vdev)
+{
+	int ret;
+
+	if (!vdev->release) {
+		dev_err(&vdev->dev, "virtbus_device MUST have a .release callback that does something.\n");
+		return -EINVAL;
+	}
+	
+	/* Don't return on error here before the device_initialize.
+	 * All error paths out of this function must perform a
+	 * put_device(), unless the release callback does not exist,
+	 * so that the .release() callback is called, and thus have
+	 * to occur after the device_initialize.
+	 */
+	device_initialize(&vdev->dev);
+
+	vdev->dev.bus = &virtual_bus_type;
+	vdev->dev.release = virtbus_release_device;
+
+	/* All device IDs are automatically allocated */
+	ret = ida_simple_get(&virtbus_dev_ida, 0, 0, GFP_KERNEL);
+
+	if (ret < 0) {
+		dev_err(&vdev->dev, "get IDA idx for virtbus device failed!\n");
+		goto device_pre_err;
+	}
+
+	vdev->id = ret;
+
+	ret = dev_set_name(&vdev->dev, "%s.%d", vdev->name, vdev->id);
+	if (ret) {
+		dev_err(&vdev->dev, "dev_set_name failed for device\n");
+		goto device_add_err;
+	}
+
+	dev_dbg(&vdev->dev, "Registering virtbus device '%s'\n",
+		dev_name(&vdev->dev));
+
+	ret = device_add(&vdev->dev);
+	if (ret)
+		goto device_add_err;
+
+	return 0;
+
+device_add_err:
+	ida_simple_remove(&virtbus_dev_ida, vdev->id);
+
+device_pre_err:
+	dev_err(&vdev->dev, "Add device to virtbus failed!: %d\n", ret);
+	put_device(&vdev->dev);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(virtbus_register_device);
+
+/**
+ * virtbus_unregister_device - remove a virtual bus device
+ * @vdev: virtual bus device we are removing
+ */
+void virtbus_unregister_device(struct virtbus_device *vdev)
+{
+	device_unregister(&vdev->dev);
+}
+EXPORT_SYMBOL_GPL(virtbus_unregister_device);
+
+static int virtbus_probe_driver(struct device *_dev)
+{
+	struct virtbus_driver *vdrv = to_virtbus_drv(_dev->driver);
+	struct virtbus_device *vdev = to_virtbus_dev(_dev);
+	int ret;
+
+	ret = dev_pm_domain_attach(_dev, true);
+	if (ret) {
+		dev_warn(_dev, "Failed to attach to PM Domain : %d\n", ret);
+		return ret;
+	}
+
+	ret = vdrv->probe(vdev);
+	if (ret) {
+		dev_err(&vdev->dev, "Probe returned error\n");
+		dev_pm_domain_detach(_dev, true);
+	}
+
+	return ret;
+}
+
+static int virtbus_remove_driver(struct device *_dev)
+{
+	struct virtbus_driver *vdrv = to_virtbus_drv(_dev->driver);
+	struct virtbus_device *vdev = to_virtbus_dev(_dev);
+	int ret = 0;
+
+	ret = vdrv->remove(vdev);
+	dev_pm_domain_detach(_dev, true);
+
+	return ret;
+}
+
+static void virtbus_shutdown_driver(struct device *_dev)
+{
+	struct virtbus_driver *vdrv = to_virtbus_drv(_dev->driver);
+	struct virtbus_device *vdev = to_virtbus_dev(_dev);
+
+	vdrv->shutdown(vdev);
+}
+
+static int virtbus_suspend_driver(struct device *_dev, pm_message_t state)
+{
+	struct virtbus_driver *vdrv = to_virtbus_drv(_dev->driver);
+	struct virtbus_device *vdev = to_virtbus_dev(_dev);
+
+	if (vdrv->suspend)
+		return vdrv->suspend(vdev, state);
+
+	return 0;
+}
+
+static int virtbus_resume_driver(struct device *_dev)
+{
+	struct virtbus_driver *vdrv = to_virtbus_drv(_dev->driver);
+	struct virtbus_device *vdev = to_virtbus_dev(_dev);
+
+	if (vdrv->resume)
+		return vdrv->resume(vdev);
+
+	return 0;
+}
+
+/**
+ * __virtbus_register_driver - register a driver for virtual bus devices
+ * @vdrv: virtbus_driver structure
+ * @owner: owning module/driver
+ */
+int __virtbus_register_driver(struct virtbus_driver *vdrv, struct module *owner)
+{
+	if (!vdrv->probe || !vdrv->remove || !vdrv->shutdown || !vdrv->id_table)
+		return -EINVAL;
+
+	vdrv->driver.owner = owner;
+	vdrv->driver.bus = &virtual_bus_type;
+	vdrv->driver.probe = virtbus_probe_driver;
+	vdrv->driver.remove = virtbus_remove_driver;
+	vdrv->driver.shutdown = virtbus_shutdown_driver;
+	vdrv->driver.suspend = virtbus_suspend_driver;
+	vdrv->driver.resume = virtbus_resume_driver;
+
+	return driver_register(&vdrv->driver);
+}
+EXPORT_SYMBOL_GPL(__virtbus_register_driver);
+
+/**
+ * virtbus_unregister_driver - unregister a driver for virtual bus devices
+ * @vdrv: virtbus_driver structure
+ */
+void virtbus_unregister_driver(struct virtbus_driver *vdrv)
+{
+	driver_unregister(&vdrv->driver);
+}
+EXPORT_SYMBOL_GPL(virtbus_unregister_driver);
+
+static int __init virtual_bus_init(void)
+{
+	return bus_register(&virtual_bus_type);
+}
+
+static void __exit virtual_bus_exit(void)
+{
+	bus_unregister(&virtual_bus_type);
+	ida_destroy(&virtbus_dev_ida);
+}
+
+module_init(virtual_bus_init);
+module_exit(virtual_bus_exit);
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 4c2ddd0941a7..60bcfe75fb94 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -832,4 +832,12 @@  struct mhi_device_id {
 	kernel_ulong_t driver_data;
 };
 
+#define VIRTBUS_NAME_SIZE 20
+#define VIRTBUS_MODULE_PREFIX "virtbus:"
+
+struct virtbus_dev_id {
+	char name[VIRTBUS_NAME_SIZE];
+	kernel_ulong_t driver_data;
+};
+
 #endif /* LINUX_MOD_DEVICETABLE_H */
diff --git a/include/linux/virtual_bus.h b/include/linux/virtual_bus.h
new file mode 100644
index 000000000000..5c7b38db1b47
--- /dev/null
+++ b/include/linux/virtual_bus.h
@@ -0,0 +1,53 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * virtual_bus.h - lightweight software bus
+ *
+ * Copyright (c) 2019-2020 Intel Corporation
+ *
+ * Please see Documentation/driver-api/virtual_bus.rst for more information
+ */
+
+#ifndef _VIRTUAL_BUS_H_
+#define _VIRTUAL_BUS_H_
+
+#include <linux/device.h>
+
+struct virtbus_device {
+	struct device dev;
+	const char *name;
+	void (*release)(struct virtbus_device *);
+	u32 id;
+};
+
+struct virtbus_driver {
+	int (*probe)(struct virtbus_device *);
+	int (*remove)(struct virtbus_device *);
+	void (*shutdown)(struct virtbus_device *);
+	int (*suspend)(struct virtbus_device *, pm_message_t);
+	int (*resume)(struct virtbus_device *);
+	struct device_driver driver;
+	const struct virtbus_dev_id *id_table;
+};
+
+static inline
+struct virtbus_device *to_virtbus_dev(struct device *dev)
+{
+	return container_of(dev, struct virtbus_device, dev);
+}
+
+static inline
+struct virtbus_driver *to_virtbus_drv(struct device_driver *drv)
+{
+	return container_of(drv, struct virtbus_driver, driver);
+}
+
+int virtbus_register_device(struct virtbus_device *vdev);
+void virtbus_unregister_device(struct virtbus_device *vdev);
+int
+__virtbus_register_driver(struct virtbus_driver *vdrv, struct module *owner);
+void virtbus_unregister_driver(struct virtbus_driver *vdrv);
+
+#define virtbus_register_driver(vdrv) \
+	__virtbus_register_driver(vdrv, THIS_MODULE)
+
+#endif /* _VIRTUAL_BUS_H_ */
diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c
index 010be8ba2116..0c8e0e3a7c84 100644
--- a/scripts/mod/devicetable-offsets.c
+++ b/scripts/mod/devicetable-offsets.c
@@ -241,5 +241,8 @@  int main(void)
 	DEVID(mhi_device_id);
 	DEVID_FIELD(mhi_device_id, chan);
 
+	DEVID(virtbus_dev_id);
+	DEVID_FIELD(virtbus_dev_id, name);
+
 	return 0;
 }
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 02d5d79da284..7d78fa3fba34 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -1358,7 +1358,13 @@  static int do_mhi_entry(const char *filename, void *symval, char *alias)
 {
 	DEF_FIELD_ADDR(symval, mhi_device_id, chan);
 	sprintf(alias, MHI_DEVICE_MODALIAS_FMT, *chan);
+	return 1;
+}
 
+static int do_virtbus_entry(const char *filename, void *symval, char *alias)
+{
+	DEF_FIELD_ADDR(symval, virtbus_dev_id, name);
+	sprintf(alias, VIRTBUS_MODULE_PREFIX "%s", *name);
 	return 1;
 }
 
@@ -1436,6 +1442,7 @@  static const struct devtable devtable[] = {
 	{"tee", SIZE_tee_client_device_id, do_tee_entry},
 	{"wmi", SIZE_wmi_device_id, do_wmi_entry},
 	{"mhi", SIZE_mhi_device_id, do_mhi_entry},
+	{"virtbus", SIZE_virtbus_dev_id, do_virtbus_entry},
 };
 
 /* Create MODULE_ALIAS() statements.