> -----Original Message-----
> From: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> Sent: Tuesday, April 21, 2020 2:08 AM
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@xxxxxxxxx>
> Cc: davem@xxxxxxxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx; Ertman, David M
> <david.m.ertman@xxxxxxxxx>; netdev@xxxxxxxxxxxxxxx; linux-
> rdma@xxxxxxxxxxxxxxx; nhorman@xxxxxxxxxx; sassmann@xxxxxxxxxx;
> jgg@xxxxxxxx; parav@xxxxxxxxxxxx; galpress@xxxxxxxxxx;
> selvin.xavier@xxxxxxxxxxxx; sriharsha.basavapatna@xxxxxxxxxxxx;
> benve@xxxxxxxxx; bharat@xxxxxxxxxxx; xavier.huwei@xxxxxxxxxx;
> yishaih@xxxxxxxxxxxx; mkalderon@xxxxxxxxxxx; aditr@xxxxxxxxxx;
> ranjani.sridharan@xxxxxxxxxxxxxxx; pierre-louis.bossart@xxxxxxxxxxxxxxx;
> Patil, Kiran <kiran.patil@xxxxxxxxx>; Bowers, AndrewX
> <andrewx.bowers@xxxxxxxxx>
> 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@xxxxxxxxx>
> >
> > 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@xxxxxxxxx>
> > Signed-off-by: Kiran Patil <kiran.patil@xxxxxxxxx>
> > Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
> > Tested-by: Andrew Bowers <andrewx.bowers@xxxxxxxxx>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@xxxxxxxxx>
> > ---
> > 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@xxxxxxxxx>");
> > +MODULE_AUTHOR("Kiran Patil <kiran.patil@xxxxxxxxx>");
> > +
> > +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
