From: Greg KH <gregkh@linuxfoundation.org>
To: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: davem@davemloft.net, Dave Ertman <david.m.ertman@intel.com>,
netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
nhorman@redhat.com, sassmann@redhat.com, jgg@ziepe.ca,
parav@mellanox.com, Kiran Patil <kiran.patil@intel.com>
Subject: Re: [PATCH v3 01/20] virtual-bus: Implementation of Virtual Bus
Date: Tue, 10 Dec 2019 16:20:06 +0100 [thread overview]
Message-ID: <20191210152006.GA4053085@kroah.com> (raw)
In-Reply-To: <20191209224935.1780117-2-jeffrey.t.kirsher@intel.com>
On Mon, Dec 09, 2019 at 02:49:16PM -0800, Jeff Kirsher wrote:
> From: Dave Ertman <david.m.ertman@intel.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 primary purpose of the virtual bus is to provide
> matching services to allow the use of a container_of
> to get access to a piece of desired data. This will
> allow two separate kernel objects to match up and
> start communication.
What? That's not the job of a virtual bus, a virtual bus is there to
put devices on it and hook them up to drivers.
What do you mean by "two separate kernel objects"? What are the objects
here?
>
> 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@intel.com>
> Signed-off-by: Kiran Patil <kiran.patil@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
> Documentation/driver-api/virtual_bus.rst | 76 +++++
> drivers/bus/Kconfig | 12 +
> drivers/bus/Makefile | 1 +
> drivers/bus/virtual_bus.c | 295 ++++++++++++++++++
> include/linux/mod_devicetable.h | 8 +
> include/linux/virtual_bus.h | 45 +++
> scripts/mod/devicetable-offsets.c | 3 +
> scripts/mod/file2alias.c | 8 +
> .../virtual_bus/virtual_bus_dev/Makefile | 7 +
> .../virtual_bus_dev/virtual_bus_dev.c | 60 ++++
> .../virtual_bus/virtual_bus_drv/Makefile | 7 +
> .../virtual_bus_drv/virtual_bus_drv.c | 115 +++++++
> 12 files changed, 637 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
> create mode 100644 tools/testing/selftests/virtual_bus/virtual_bus_dev/Makefile
> create mode 100644 tools/testing/selftests/virtual_bus/virtual_bus_dev/virtual_bus_dev.c
> create mode 100644 tools/testing/selftests/virtual_bus/virtual_bus_drv/Makefile
> create mode 100644 tools/testing/selftests/virtual_bus/virtual_bus_drv/virtual_bus_drv.c
>
> diff --git a/Documentation/driver-api/virtual_bus.rst b/Documentation/driver-api/virtual_bus.rst
> new file mode 100644
> index 000000000000..db8c34fcafe8
> --- /dev/null
> +++ b/Documentation/driver-api/virtual_bus.rst
> @@ -0,0 +1,76 @@
> +===============================
> +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 are lightweight objects that support the minimal device
> +functionality. Devices will accept a name, and then an automatically
> +generated index is concatenated onto it for the virtbus_device->name.
> +
> +The virtbus_driver and virtbus_device creators need to both have access
> +to a predefined virtbus_device_object struct that will look like the
> +following:
> + struct virtbus_object {
> + struct virtbus_device vdev;
> + struct foo_type foo;
> + }
> +
> +Then when the virtbus_driver's probe is called with the virtbus_device
> +as a parameter, it can do a container_of on the virtbus_device to get
> +to the struct foo_type foo that it cares about.
That's just how the driver model works, I'm not quite sure what you are
trying to say here. What is this section for?
> +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_drv_reg
> +and unregister with virtbus_drv_unreg.
> +
> +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)
> +
> +This allows for multiple virtbus_devices with the same name, which will all
> +be matched to the same virtbus_driver. Driver binding is performed by the
> +driver core, invoking driver probe() after finding a match between device and driver.
> +
> +Virtual Bus API entry points
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +int virtbus_dev_register(struct virtbus_device *vdev)
> +void virtbus_dev_unregister(struct virtbus_device *vdev)
> +int virtbus_drv_register(struct virtbus_driver *vdrv, struct module *owner)
> +void virtbus_drv_unregister(struct virtbus_driver *vdrv)
I don't understand this documentation at all. Can't you put what you
need in the code itself and have it be auto-generated? This is really
vague stuff that I can't find useful. And if I don't understand it,
well... :)
> diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
> index 50200d1c06ea..770519d16d43 100644
> --- a/drivers/bus/Kconfig
> +++ b/drivers/bus/Kconfig
> @@ -203,4 +203,16 @@ config DA8XX_MSTPRI
>
> source "drivers/bus/fsl-mc/Kconfig"
>
> +config VIRTUAL_BUS
> + tristate "lightweight Virtual Bus"
Why "lightweight"? Where's the heavy one? :)
> + depends on PM
Why does it depend on PM?
> + help
> + Provides a software bus for virtbus_devices to be added to it
> + and virtbus_drivers to be registered on it. Will create a match
> + between the driver and device, then call the driver's probe with
> + the virtbus_device's struct.
> + 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 1320bcf9fa9d..6721c77dc71b 100644
> --- a/drivers/bus/Makefile
> +++ b/drivers/bus/Makefile
> @@ -34,3 +34,4 @@ obj-$(CONFIG_UNIPHIER_SYSTEM_BUS) += uniphier-system-bus.o
> obj-$(CONFIG_VEXPRESS_CONFIG) += vexpress-config.o
>
> obj-$(CONFIG_DA8XX_MSTPRI) += da8xx-mstpri.o
> +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..6bc986659e4b
> --- /dev/null
> +++ b/drivers/bus/virtual_bus.c
> @@ -0,0 +1,295 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * virtual_bus.c - lightweight software based bus for virtual devices
> + *
> + * Copyright (c) 2019-20 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("Lightweight Virtual Bus");
> +MODULE_AUTHOR("David Ertman <david.m.ertman@intel.com>");
> +MODULE_AUTHOR("Kiran Patil <kiran.patil@intel.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)) {
> + vdev->matched_element = id;
> + return id;
> + }
> + id++;
> + }
> + return NULL;
> +}
> +
> +#define to_virtbus_dev(x) (container_of((x), struct virtbus_device, dev))
> +#define to_virtbus_drv(x) (container_of((x), struct virtbus_driver, \
> + driver))
> +
> +/**
> + * virtbus_match - bind virtbus device to virtbus driver
> + * @dev: device
> + * @drv: driver
> + *
> + * Virtbus device IDs are always in "<name>.<instance>" format.
> + * Instances are automatically selected through an ida_simple_get so
> + * are positive integers. Names are 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 kerneldoc for static functions?
This naming scheme is great to have documented somewhere, why am I just
finding it here? See, your .rst file was useless :(
Make the documentation be generated from this .c file please.
> + */
> +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);
> +
> + if (vdrv->id_table)
> + return virtbus_match_id(vdrv->id_table, vdev) != NULL;
> +
> + return !strcmp(vdev->name, drv->name);
Wait, why would a driver not have an id table? You are matching two
different ways here, only one documented above?
> +}
> +
> +/**
> + * virtbus_probe - call probe of the virtbus_drv
> + * @dev: device struct
> + */
> +static int virtbus_probe(struct device *dev)
> +{
> + if (dev->driver->probe)
> + return dev->driver->probe(dev);
How can we not have a probe? Test for that at registration.
> +
> + return 0;
> +}
> +
> +static int virtbus_remove(struct device *dev)
> +{
> + if (dev->driver->remove)
> + return dev->driver->remove(dev);
Same as above.
> +
> + return 0;
> +}
> +
> +static void virtbus_shutdown(struct device *dev)
> +{
> + if (dev->driver->shutdown)
> + dev->driver->shutdown(dev);
Same as above
> +}
> +
> +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_dev_register - add a virtual bus device
> + * @vdev: virtual bus device to add
> + */
> +int virtbus_dev_register(struct virtbus_device *vdev)
> +{
> + int ret;
> +
> + device_initialize(&vdev->dev);
> +
> + vdev->dev.bus = &virtual_bus_type;
> + /* All device IDs are automatically allocated */
> + ret = ida_simple_get(&virtbus_dev_ida, 0, 0, GFP_KERNEL);
> + if (ret < 0)
> + return ret;
Ugh, you just lost the reference count logic for your call to
device_initialize() above, right? Or did you document the heck out of
this saying that if you call virtbus_dev_register, you MUST call
put_device() to properly clean things up.
Nope, you did not :(
> +
> + vdev->id = ret;
> + dev_set_name(&vdev->dev, "%s.%d", vdev->name, vdev->id);
> +
> + 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:
> + /* Error adding virtual device */
We know that, document things that are not obvious :)
> + put_device(&vdev->dev);
> + ida_simple_remove(&virtbus_dev_ida, vdev->id);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(virtbus_dev_register);
> +
> +/**
> + * virtbus_dev_unregister - remove a virtual bus device
> + * vdev: virtual bus device we are removing
> + */
> +void virtbus_dev_unregister(struct virtbus_device *vdev)
> +{
> + put_device(&vdev->dev);
> +}
> +EXPORT_SYMBOL_GPL(virtbus_dev_unregister);
> +
> +struct virtbus_object {
> + struct virtbus_device vdev;
> + char name[];
Why an empty array? are you sure?
> +};
> +
> +/**
> + * virtbus_dev_release - Destroy a virtbus device
> + * @vdev: virtual device to release
> + *
> + * Note that the vdev->data which is separately allocated needs to be
> + * separately freed on it own.
> + */
> +static void virtbus_dev_release(struct device *dev)
> +{
> + struct virtbus_object *vo = container_of(dev, struct virtbus_object,
> + vdev.dev);
> +
> + ida_simple_remove(&virtbus_dev_ida, vo->vdev.id);
> + kfree(vo);
> +}
> +
> +static int virtbus_drv_probe(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 attatch to PM Domain : %d\n", ret);
> + return ret;
> + }
> +
> + if (vdrv->probe) {
> + ret = vdrv->probe(vdev);
> + if (ret)
> + dev_pm_domain_detach(_dev, true);
> + }
> +
> + return ret;
> +}
> +
> +static int virtbus_drv_remove(struct device *_dev)
> +{
> + struct virtbus_driver *vdrv = to_virtbus_drv(_dev->driver);
> + struct virtbus_device *vdev = to_virtbus_dev(_dev);
> + int ret = 0;
> +
> + if (vdrv->remove)
> + ret = vdrv->remove(vdev);
> +
> + dev_pm_domain_detach(_dev, true);
> +
> + return ret;
> +}
> +
> +static void virtbus_drv_shutdown(struct device *_dev)
> +{
> + struct virtbus_driver *vdrv = to_virtbus_drv(_dev->driver);
> + struct virtbus_device *vdev = to_virtbus_dev(_dev);
> +
> + if (vdrv->shutdown)
> + vdrv->shutdown(vdev);
> +}
> +
> +static int virtbus_drv_suspend(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_drv_resume(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_drv_register - register a driver for virtual bus devices
> + * @vdrv: virtbus_driver structure
> + * @owner: owning module/driver
> + */
> +int __virtbus_drv_register(struct virtbus_driver *vdrv, struct module *owner)
> +{
> + vdrv->driver.owner = owner;
> + vdrv->driver.bus = &virtual_bus_type;
> + vdrv->driver.probe = virtbus_drv_probe;
> + vdrv->driver.remove = virtbus_drv_remove;
> + vdrv->driver.shutdown = virtbus_drv_shutdown;
> + vdrv->driver.suspend = virtbus_drv_suspend;
> + vdrv->driver.resume = virtbus_drv_resume;
> +
> + return driver_register(&vdrv->driver);
> +}
> +EXPORT_SYMBOL_GPL(__virtbus_drv_register);
> +
> +/**
> + * virtbus_drv_unregister - unregister a driver for virtual bus devices
> + * @drv: virtbus_driver structure
> + */
> +void virtbus_drv_unregister(struct virtbus_driver *vdrv)
> +{
> + driver_unregister(&vdrv->driver);
> +}
> +EXPORT_SYMBOL_GPL(virtbus_drv_unregister);
> +
> +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 5714fd35a83c..e19214f84c98 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -821,4 +821,12 @@ struct wmi_device_id {
> const void *context;
> };
>
> +#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..fe4725b1e6b1
> --- /dev/null
> +++ b/include/linux/virtual_bus.h
> @@ -0,0 +1,45 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * virtual_bus.h - lightweight software bus
> + *
> + * Copyright (c) 2019-20 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 {
> + const char *name;
> + int id;
> + const struct virtbus_dev_id *matched_element;
> + struct device dev;
Put dev at the front might align things a bit better in memory. Not a
big issue...
> +};
> +
> +/* If the driver uses a id_table to match with virtbus_devices, then the
> + * memory for the table is expected to remain allocated for the duration
> + * of the pairing between driver and device. The pointer for the matching
> + * element will be copied to the dev_id field of the virtbus_device.
Again, id_table is optional? Huh????
> + */
> +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;
> +};
> +
> +int virtbus_dev_register(struct virtbus_device *vdev);
> +void virtbus_dev_unregister(struct virtbus_device *vdev);
> +int __virtbus_drv_register(struct virtbus_driver *vdrv, struct module *owner);
> +void virtbus_drv_unregister(struct virtbus_driver *vdrv);
> +
> +#define virtbus_drv_register(vdrv) \
> + __virtbus_drv_register(vdrv, THIS_MODULE)
> +
> +#endif /* _VIRTUAL_BUS_H_ */
> diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c
> index 054405b90ba4..9a6099bf90c8 100644
> --- a/scripts/mod/devicetable-offsets.c
> +++ b/scripts/mod/devicetable-offsets.c
> @@ -231,5 +231,8 @@ int main(void)
> DEVID(wmi_device_id);
> DEVID_FIELD(wmi_device_id, guid_string);
>
> + 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 c91eba751804..713fdfe010b0 100644
> --- a/scripts/mod/file2alias.c
> +++ b/scripts/mod/file2alias.c
> @@ -1335,6 +1335,13 @@ static int do_wmi_entry(const char *filename, void *symval, char *alias)
> 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;
> +}
> +
> /* Does namelen bytes of name exactly match the symbol? */
> static bool sym_is(const char *name, unsigned namelen, const char *symbol)
> {
> @@ -1407,6 +1414,7 @@ static const struct devtable devtable[] = {
> {"typec", SIZE_typec_device_id, do_typec_entry},
> {"tee", SIZE_tee_client_device_id, do_tee_entry},
> {"wmi", SIZE_wmi_device_id, do_wmi_entry},
> + {"virtbus", SIZE_virtbus_dev_id, do_virtbus_entry},
> };
>
> /* Create MODULE_ALIAS() statements.
> diff --git a/tools/testing/selftests/virtual_bus/virtual_bus_dev/Makefile b/tools/testing/selftests/virtual_bus/virtual_bus_dev/Makefile
> new file mode 100644
> index 000000000000..ddd5088eb26b
> --- /dev/null
> +++ b/tools/testing/selftests/virtual_bus/virtual_bus_dev/Makefile
> @@ -0,0 +1,7 @@
> +obj-m += virtual_bus_dev.o
> +
> +all:
> + make -C /lib/modules/$(shell uname -r)/build M=$(PWD) modules
> +
> +clean:
> + make -C /lib/modules/$(shell uname -r)/build M=$(PWD) clean
> diff --git a/tools/testing/selftests/virtual_bus/virtual_bus_dev/virtual_bus_dev.c b/tools/testing/selftests/virtual_bus/virtual_bus_dev/virtual_bus_dev.c
> new file mode 100644
> index 000000000000..966e882452d2
> --- /dev/null
> +++ b/tools/testing/selftests/virtual_bus/virtual_bus_dev/virtual_bus_dev.c
> @@ -0,0 +1,60 @@
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/module.h>
> +#include <linux/virtual_bus.h>
> +#include <linux/string.h>
> +#include <linux/slab.h>
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Dave Ertman");
> +MODULE_DESCRIPTION("Test to create a device on virtual bus");
> +MODULE_VERSION("1.0");
> +
> +struct virtbus_data {
> + int numb;
> +};
> +
> +struct virtbus_object {
> + struct virtbus_device vdev;
> + struct virtbus_data vd;
> +};
> +
> +static struct virtbus_object vo = {
> + .vdev = {
> + .name = "virtual_bus_dev",
> + },
> + .vd = {
> + .numb = 789,
> + },
> +};
> +
> +static int __init test_dev_init(void)
> +{
> + int ret = 0;
> +
> + printk(KERN_INFO "Loading Virtual Bus Test Device\n");
> +
> + printk(KERN_ERR "Virtbus Device values:\n\t%s\n\t%d\n", vo.vdev.name,
> + vo.vd.numb);
> +
> + ret = virtbus_dev_register(&vo.vdev);
> + if (ret) {
> + printk(KERN_ERR "FAILED TO ADD VIRTBUS DEVICE %d\n", ret);
> + return ret;
> + }
> +
> + printk(KERN_INFO "Virtual Device created\n");
> + return ret;
> +}
> +
> +static void __exit test_dev_exit(void)
> +{
> + printk(KERN_INFO "Exiting Virtual Bus Test Device");
> +
> + virtbus_dev_unregister(&vo.vdev);
> +
> + printk(KERN_INFO "Virtual Bus Test Device removed\n");
> +}
> +
> +module_init(test_dev_init);
> +module_exit(test_dev_exit);
> diff --git a/tools/testing/selftests/virtual_bus/virtual_bus_drv/Makefile b/tools/testing/selftests/virtual_bus/virtual_bus_drv/Makefile
> new file mode 100644
> index 000000000000..a4b7467f7878
> --- /dev/null
> +++ b/tools/testing/selftests/virtual_bus/virtual_bus_drv/Makefile
> @@ -0,0 +1,7 @@
> +obj-m += virtual_bus_drv.o
> +
> +all:
> + make -C /lib/modules/$(shell uname -r)/build M=$(PWD) modules
> +
> +clean:
> + make -C /lib/modules/$(shell uname -r)/build M=$(PWD) clean
Add all the test stuff as a follow-on patch, not all in one big mess
here.
> diff --git a/tools/testing/selftests/virtual_bus/virtual_bus_drv/virtual_bus_drv.c b/tools/testing/selftests/virtual_bus/virtual_bus_drv/virtual_bus_drv.c
> new file mode 100644
> index 000000000000..13e2e3d686ab
> --- /dev/null
> +++ b/tools/testing/selftests/virtual_bus/virtual_bus_drv/virtual_bus_drv.c
> @@ -0,0 +1,115 @@
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/module.h>
> +#include <linux/virtual_bus.h>
> +#include <linux/string.h>
> +#include <linux/slab.h>
> +#include <linux/mod_devicetable.h>
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Dave Ertman");
> +MODULE_DESCRIPTION("Test to register a driver on virtual bus");
> +MODULE_VERSION("1.0");
MODULE_VERSION? What is this, the 1990's? :(
> +
> +struct virtbus_data {
> + int numb;
> +};
> +
> +struct virtbus_object {
> + struct virtbus_device vdev;
> + struct virtbus_data vd;
> +};
> +
> +static int td_probe(struct virtbus_device *vdev)
> +{
> + struct virtbus_object *vo;
> +
> + printk(KERN_ERR "VIRTBUS DRIVER PROBED\n");
> +
> + vo = container_of(vdev, struct virtbus_object, vdev);
> +
> + if (vdev->matched_element) {
> + printk(KERN_ERR "DEV_ID->DATA 0x%08x\n",
> + (uint)vdev->matched_element->driver_data);
> + if (vdev->matched_element->driver_data == 1)
> + printk(KERN_ERR "NUMB: %d\n",
> + vo->vd.numb);
> + } else {
> + printk(KERN_ERR "MATCHED_ELEMENT->DATA is NULL\n");
> + }
> +
> + return 0;
> +}
> +
> +static int td_remove(struct virtbus_device *vdev)
> +{
> + printk(KERN_ERR "VIRTBUS DRIVER REMOVED\n");
> + return 0;
> +}
> +
> +static void td_shutdown(struct virtbus_device *vdev)
> +{
> + printk(KERN_ERR "VIRTBUS DRIVER SHUTDOWN\n");
> +}
> +
> +static const struct virtbus_dev_id vdev_id_table[] = {
> +
> + {
> + .name = "NOT THE NAME",
> + .driver_data = 0x00000000,
> + },
> + {
> + .name = "virtual_bus_dev",
> + .driver_data = 0x00000001,
> + },
> + {
> + .name = "ice_rdma",
> + .driver_data = 0x00000002,
> + },
> + {
> + .name = "YET AGAIN NOT NAME",
> + .driver_data = 0x00000003,
> + },
> +};
> +
> +static struct virtbus_driver vdrv = {
> + .probe = td_probe,
> + .remove = td_remove,
> + .shutdown = td_shutdown,
> + .driver = {
> + .name = "virtual_bus_dev",
> + },
> +};
> +
> +static int __init test_drv_init(void)
> +{
> + int ret;
> +
> + printk(KERN_INFO "Registering Virtual Bus Test Driver\n");
> +
> + /* To do a simple match, leave the id_table as NULL */
> + vdrv.id_table = &vdev_id_table[0];
> +
> + printk(KERN_ERR "name of 0 is %s\n", vdrv.id_table->name);
> +
> + ret = virtbus_drv_register(&vdrv);
> +
> + if (!ret)
> + printk(KERN_INFO "Virtual Driver registered\n");
> + else
> + printk(KERN_INFO "Virtual Driver FAILED!!\n");
> +
> + return ret;
> +}
> +
> +static void __exit test_drv_exit(void)
> +{
> + printk(KERN_INFO "Exiting Virtual Bus Test Driver");
> +
> + virtbus_drv_unregister(&vdrv);
> +
> + printk(KERN_INFO "Virtual Bus Test Driver removed\n");
> +}
> +
> +module_init(test_drv_init);
> +module_exit(test_drv_exit);
Odd test, what is it supposed to do? If you use kunit, does that make
it easier to test things out?
thanks,
greg k-h
next prev parent reply other threads:[~2019-12-10 15:20 UTC|newest]
Thread overview: 62+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-09 22:49 [net-next v3 00/20][pull request] Intel Wired LAN Driver Updates 2019-12-09 Jeff Kirsher
2019-12-09 22:49 ` [PATCH v3 01/20] virtual-bus: Implementation of Virtual Bus Jeff Kirsher
2019-12-10 6:49 ` Leon Romanovsky
2019-12-10 17:51 ` Jason Gunthorpe
2019-12-10 15:20 ` Greg KH [this message]
2019-12-10 18:22 ` Jason Gunthorpe
2019-12-09 22:49 ` [PATCH v3 02/20] ice: Initialize and register a virtual bus to provide RDMA Jeff Kirsher
2019-12-10 15:32 ` Greg KH
2019-12-23 19:06 ` Allan, Bruce W
2019-12-09 22:49 ` [PATCH v3 03/20] ice: Implement peer communications Jeff Kirsher
2019-12-09 22:49 ` [PATCH v3 04/20] i40e: Register a virtbus device to provide RDMA Jeff Kirsher
2019-12-10 15:33 ` Greg KH
2019-12-10 15:39 ` Greg KH
2019-12-13 23:08 ` Saleem, Shiraz
2019-12-14 8:37 ` Greg KH
2019-12-18 18:57 ` Saleem, Shiraz
2019-12-18 19:20 ` Jason Gunthorpe
2020-01-02 16:01 ` Saleem, Shiraz
2019-12-19 8:46 ` 'Greg KH'
2019-12-16 3:48 ` Parav Pandit
2019-12-16 7:15 ` Greg KH
2019-12-16 8:36 ` Parav Pandit
2019-12-16 8:58 ` Greg KH
2019-12-16 9:17 ` Parav Pandit
2019-12-09 22:49 ` [PATCH v3 05/20] RDMA/irdma: Add driver framework definitions Jeff Kirsher
2019-12-10 19:04 ` Jason Gunthorpe
2019-12-11 6:07 ` Leon Romanovsky
2019-12-12 1:40 ` Saleem, Shiraz
2019-12-12 8:39 ` Leon Romanovsky
2019-12-12 9:12 ` gregkh
2019-12-17 21:00 ` Jason Gunthorpe
2019-12-21 0:00 ` Keller, Jacob E
2019-12-09 22:49 ` [PATCH v3 06/20] RDMA/irdma: Implement device initialization definitions Jeff Kirsher
2019-12-09 22:49 ` [PATCH v3 07/20] RDMA/irdma: Implement HW Admin Queue OPs Jeff Kirsher
2019-12-09 22:49 ` [PATCH v3 08/20] RDMA/irdma: Add HMC backing store setup functions Jeff Kirsher
2019-12-09 22:49 ` [PATCH v3 09/20] RDMA/irdma: Add privileged UDA queue implementation Jeff Kirsher
2019-12-09 22:49 ` [PATCH v3 10/20] RDMA/irdma: Add QoS definitions Jeff Kirsher
2019-12-09 22:49 ` [PATCH v3 11/20] RDMA/irdma: Add connection manager Jeff Kirsher
2019-12-09 22:49 ` [PATCH v3 12/20] RDMA/irdma: Add PBLE resource manager Jeff Kirsher
2019-12-09 22:49 ` [PATCH v3 13/20] RDMA/irdma: Implement device supported verb APIs Jeff Kirsher
2019-12-09 22:49 ` [PATCH v3 14/20] RDMA/irdma: Add RoCEv2 UD OP support Jeff Kirsher
2019-12-09 22:49 ` [PATCH v3 15/20] RDMA/irdma: Add user/kernel shared libraries Jeff Kirsher
2019-12-09 22:49 ` [PATCH v3 16/20] RDMA/irdma: Add miscellaneous utility definitions Jeff Kirsher
2019-12-09 22:49 ` [PATCH v3 17/20] RDMA/irdma: Add dynamic tracing for CM Jeff Kirsher
2019-12-09 22:49 ` [PATCH v3 18/20] RDMA/irdma: Add ABI definitions Jeff Kirsher
2019-12-09 22:49 ` [PATCH v3 19/20] RDMA: Add irdma Kconfig/Makefile and remove i40iw Jeff Kirsher
2019-12-11 20:02 ` Jason Gunthorpe
2019-12-13 23:06 ` Saleem, Shiraz
2019-12-17 21:04 ` Jason Gunthorpe
2020-01-02 16:00 ` Saleem, Shiraz
2020-01-02 17:04 ` Jason Gunthorpe
2020-01-02 17:50 ` Saleem, Shiraz
2020-01-02 18:06 ` Jason Gunthorpe
2019-12-09 22:49 ` [PATCH v3 20/20] RDMA/irdma: Update MAINTAINERS file Jeff Kirsher
2019-12-10 7:33 ` [net-next v3 00/20][pull request] Intel Wired LAN Driver Updates 2019-12-09 Greg KH
2019-12-10 17:22 ` Jason Gunthorpe
2019-12-10 18:06 ` Jeff Kirsher
2019-12-10 18:25 ` Jason Gunthorpe
2019-12-10 18:41 ` Jeff Kirsher
2019-12-10 19:11 ` Jason Gunthorpe
2019-12-10 19:23 ` Jeff Kirsher
2019-12-10 19:44 ` Jason Gunthorpe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191210152006.GA4053085@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=davem@davemloft.net \
--cc=david.m.ertman@intel.com \
--cc=jeffrey.t.kirsher@intel.com \
--cc=jgg@ziepe.ca \
--cc=kiran.patil@intel.com \
--cc=linux-rdma@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=nhorman@redhat.com \
--cc=parav@mellanox.com \
--cc=sassmann@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.