Message-ID: <DM6PR11MB28416D46B4F4AE93EDE421D8DDD50@DM6PR11MB2841.namprd11.prod.outlook.com>
Date:   Tue, 21 Apr 2020 23:27:09 +0000
From:   "Ertman, David M" <david.m.ertman@...el.com>
To:     Leon Romanovsky <leonro@...lanox.com>,
        "Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        "nhorman@...hat.com" <nhorman@...hat.com>,
        "sassmann@...hat.com" <sassmann@...hat.com>,
        "jgg@...pe.ca" <jgg@...pe.ca>,
        "parav@...lanox.com" <parav@...lanox.com>,
        "galpress@...zon.com" <galpress@...zon.com>,
        "selvin.xavier@...adcom.com" <selvin.xavier@...adcom.com>,
        "sriharsha.basavapatna@...adcom.com" 
        <sriharsha.basavapatna@...adcom.com>,
        "benve@...co.com" <benve@...co.com>,
        "bharat@...lsio.com" <bharat@...lsio.com>,
        "xavier.huwei@...wei.com" <xavier.huwei@...wei.com>,
        "yishaih@...lanox.com" <yishaih@...lanox.com>,
        "mkalderon@...vell.com" <mkalderon@...vell.com>,
        "aditr@...are.com" <aditr@...are.com>,
        "ranjani.sridharan@...ux.intel.com" 
        <ranjani.sridharan@...ux.intel.com>,
        "pierre-louis.bossart@...ux.intel.com" 
        <pierre-louis.bossart@...ux.intel.com>,
        "Patil, Kiran" <kiran.patil@...el.com>,
        "Bowers, AndrewX" <andrewx.bowers@...el.com>
Subject: RE: [net-next v2 1/9] Implementation of Virtual Bus

> -----Original Message-----
> From: Leon Romanovsky <leonro@...lanox.com>
> Sent: Tuesday, April 21, 2020 2:08 AM
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@...el.com>
> Cc: davem@...emloft.net; gregkh@...uxfoundation.org; Ertman, David M
> <david.m.ertman@...el.com>; netdev@...r.kernel.org; linux-
> rdma@...r.kernel.org; nhorman@...hat.com; sassmann@...hat.com;
> jgg@...pe.ca; parav@...lanox.com; galpress@...zon.com;
> selvin.xavier@...adcom.com; sriharsha.basavapatna@...adcom.com;
> benve@...co.com; bharat@...lsio.com; xavier.huwei@...wei.com;
> yishaih@...lanox.com; mkalderon@...vell.com; aditr@...are.com;
> ranjani.sridharan@...ux.intel.com; pierre-louis.bossart@...ux.intel.com;
> Patil, Kiran <kiran.patil@...el.com>; Bowers, AndrewX
> <andrewx.bowers@...el.com>
> 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 <david.m.ertman@...el.com>
> >
> > 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 <david.m.ertman@...el.com>
> > Signed-off-by: Kiran Patil <kiran.patil@...el.com>
> > Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
> > Tested-by: Andrew Bowers <andrewx.bowers@...el.com>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@...el.com>
> > ---
> >  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 <david.m.ertman@...el.com>");
> > +MODULE_AUTHOR("Kiran Patil <kiran.patil@...el.com>");
> > +
> > +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

Powered by blists - more mailing lists