> -----Original Message-----
> From: Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>
> Sent: Saturday, April 18, 2020 5:51 AM
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@xxxxxxxxx>
> Cc: davem@xxxxxxxxxxxxx; 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;
> leonro@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 1/9] Implementation of Virtual Bus
>
> On Fri, Apr 17, 2020 at 10:10:26AM -0700, Jeff Kirsher wrote:
> > @@ -0,0 +1,53 @@
> > +/* 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 {
> > + struct device dev;
> > + const char *name;
>
> struct device already has a name, why do you need another one?
The name in dev is the base name appended with the id to make sure each device
has unique name. The name in vdev is the abbreviated one (without the id) which
will be used in the matching function, so that a driver can claim to support
<name> and will be matched with all <name>.<id> devices for all id's.
This is similar logic to platform_device's name field.
>
> > + void (*release)(struct virtbus_device *);
>
> A bus should have the release function, not the actual device itself. A
> device should not need function pointers.
>
The bus does have a release function, but it is a wrapper to call the release defined by
the device. This is where the KO registering the virtbus_device is expected to clean up
the resources allocated for this device (e.g. free memory, etc). Having the virtual_bus_release
call a release callback in the virtual_device allows for extra cleanup from the originating KO
if necessary.
The memory model of virtual bus is for the originating KO to manage the lifespan of the
memory for the virtual_device. The virtual_bus expects the KO defining the virtbus_device
have the memory allocated before registering a virtbus_device and to clean up that memory
when the release is called.
The platform_device also has function pointers in it, by including a MFD object, but the
platform_bus is managing the memory for the platform_bus_object that contains the
platform_device which it why it using a generic kref_put to free memory.
> > + int id;
>
> Shouldn't that be a specific type, like u64 or something?
Changed to be a u32 to match struct device.id.
>
> thanks,
>
> greg k-h
-DaveE
