On Thu, Oct 01, 2020 at 05:20:35PM +0000, Ertman, David M wrote: > > -----Original Message----- > > From: Leon Romanovsky <leon@xxxxxxxxxx> > > Sent: Thursday, October 1, 2020 1:32 AM > > To: Ertman, David M <david.m.ertman@xxxxxxxxx> > > Cc: linux-rdma@xxxxxxxxxxxxxxx > > Subject: Re: [PATCH 1/6] Add ancillary bus support > > > > On Wed, Sep 30, 2020 at 10:05:29PM -0700, Dave Ertman wrote: > > > Add support for the Ancillary Bus, ancillary_device and ancillary_driver. > > > It enables drivers to create an ancillary_device and bind an > > > ancillary_driver to it. > > > > > > The bus supports probe/remove shutdown and suspend/resume callbacks. > > > Each ancillary_device has a unique string based id; driver binds to > > > an ancillary_device based on this id through the bus. > > > > > > Co-developed-by: Kiran Patil <kiran.patil@xxxxxxxxx> > > > Signed-off-by: Kiran Patil <kiran.patil@xxxxxxxxx> > > > Co-developed-by: Ranjani Sridharan <ranjani.sridharan@xxxxxxxxxxxxxxx> > > > Signed-off-by: Ranjani Sridharan <ranjani.sridharan@xxxxxxxxxxxxxxx> > > > Co-developed-by: Fred Oh <fred.oh@xxxxxxxxxxxxxxx> > > > Signed-off-by: Fred Oh <fred.oh@xxxxxxxxxxxxxxx> > > > Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx> > > > Reviewed-by: Shiraz Saleem <shiraz.saleem@xxxxxxxxx> > > > Reviewed-by: Parav Pandit <parav@xxxxxxxxxxxx> > > > Reviewed-by: Dan Williams <dan.j.williams@xxxxxxxxx> > > > Signed-off-by: Dave Ertman <david.m.ertman@xxxxxxxxx> > > > --- > > > Documentation/driver-api/ancillary_bus.rst | 230 > > +++++++++++++++++++++ > > > Documentation/driver-api/index.rst | 1 + > > > drivers/bus/Kconfig | 3 + > > > drivers/bus/Makefile | 3 + > > > drivers/bus/ancillary.c | 191 +++++++++++++++++ > > > include/linux/ancillary_bus.h | 58 ++++++ > > > include/linux/mod_devicetable.h | 8 + > > > scripts/mod/devicetable-offsets.c | 3 + > > > scripts/mod/file2alias.c | 8 + > > > 9 files changed, 505 insertions(+) > > > create mode 100644 Documentation/driver-api/ancillary_bus.rst > > > create mode 100644 drivers/bus/ancillary.c > > > create mode 100644 include/linux/ancillary_bus.h > > > > > > diff --git a/Documentation/driver-api/ancillary_bus.rst > > b/Documentation/driver-api/ancillary_bus.rst > > > new file mode 100644 > > > index 000000000000..0a11979aa927 > > > --- /dev/null > > > +++ b/Documentation/driver-api/ancillary_bus.rst > > > @@ -0,0 +1,230 @@ > > > +.. SPDX-License-Identifier: GPL-2.0-only > > > + > > > +============= > > > +Ancillary Bus > > > +============= > > > + > > > +In some subsystems, the functionality of the core device > > (PCI/ACPI/other) is > > > +too complex for a single device to be managed as a monolithic block or a > > part of > > > +the functionality needs to be exposed to a different subsystem. Splitting > > the > > > +functionality into smaller orthogonal devices would make it easier to > > manage > > > +data, power management and domain-specific interaction with the > > hardware. A key > > > +requirement for such a split is that there is no dependency on a physical > > bus, > > > +device, register accesses or regmap support. These individual devices split > > from > > > +the core cannot live on the platform bus as they are not physical devices > > that > > > +are controlled by DT/ACPI. The same argument applies for not using MFD > > in this > > > +scenario as MFD relies on individual function devices being physical > > devices > > > +that are DT enumerated. > > > + > > > +An example for this kind of requirement is the audio subsystem where a > > single > > > +IP is handling multiple entities such as HDMI, Soundwire, local devices > > such as > > > +mics/speakers etc. The split for the core's functionality can be arbitrary or > > > +be defined by the DSP firmware topology and include hooks for > > test/debug. This > > > +allows for the audio core device to be minimal and focused on hardware- > > specific > > > +control and communication. > > > + > > > +The ancillary bus is intended to be minimal, generic and avoid domain- > > specific > > > +assumptions. Each ancillary_device represents a part of its parent > > > +functionality. The generic behavior can be extended and specialized as > > needed > > > +by encapsulating an ancillary_device within other domain-specific > > structures and > > > +the use of .ops callbacks. Devices on the ancillary bus do not share any > > > +structures and the use of a communication channel with the parent is > > > +domain-specific. > > > + > > > +When Should the Ancillary Bus Be Used > > > +===================================== > > > + > > > +The ancillary bus is to be used when a driver and one or more kernel > > modules, > > > +who share a common header file with the driver, need a mechanism to > > connect and > > > +provide access to a shared object allocated by the ancillary_device's > > > +registering driver. The registering driver for the ancillary_device(s) and > > the > > > +kernel module(s) registering ancillary_drivers can be from the same > > subsystem, > > > +or from multiple subsystems. > > > + > > > +The emphasis here is on a common generic interface that keeps > > subsystem > > > +customization out of the bus infrastructure. > > > + > > > +One example could be a multi-port PCI network device that is rdma- > > capable and > > > +needs to export this functionality and attach to an rdma driver in another > > > +subsystem. The PCI driver will allocate and register an ancillary_device for > > > +each physical function on the NIC. The rdma driver will register an > > > +ancillary_driver that will be matched with and probed for each of these > > > +ancillary_devices. This will give the rdma driver access to the shared > > data/ops > > > +in the PCI drivers shared object to establish a connection with the PCI > > driver. > > > + > > > +Another use case is for the a PCI device to be split out into multiple sub > > > +functions. For each sub function an ancillary_device will be created. A PCI > > > +sub function driver will bind to such devices that will create its own one or > > > +more class devices. A PCI sub function ancillary device will likely be > > > +contained in a struct with additional attributes such as user defined sub > > > +function number and optional attributes such as resources and a link to > > the > > > +parent device. These attributes could be used by systemd/udev; and > > hence should > > > +be initialized before a driver binds to an ancillary_device. > > > + > > > +Ancillary Device > > > +================ > > > + > > > +An ancillary_device is created and registered to represent a part of its > > parent > > > +device's functionality. It is given a name that, combined with the > > registering > > > +drivers KBUILD_MODNAME, creates a match_name that is used for driver > > binding, > > > +and an id that combined with the match_name provide a unique name to > > register > > > +with the bus subsystem. > > > + > > > +Registering an ancillary_device is a two-step process. First you must call > > > +ancillary_device_initialize(), which will check several aspects of the > > > +ancillary_device struct and perform a device_initialize(). After this step > > > +completes, any error state must have a call to put_device() in its > > resolution > > > +path. The second step in registering an ancillary_device is to perform a > > call > > > +to ancillary_device_add(), which will set the name of the device and add > > the > > > +device to the bus. > > > + > > > +To unregister an ancillary_device, just a call to > > ancillary_device_unregister() > > > +is used. This will perform both a device_del() and a put_device(). > > > > Why did you chose ancillary_device_initialize() and not > > ancillary_device_register() to be paired with ancillary_device_unregister()? > > > > Thanks > > We originally had a single call to ancillary_device_register() that paired with > unregister, but there was an ask to separate the register into an initialize and > add to make the error condition unwind more compartimentalized. It is correct thing to separate, but I would expect: ancillary_device_register() ancillary_device_add() vs. ancillary_device_unregister() It is not a big deal, just curious. The much more big deal is that I'm required to create 1-to-1 mapping between device and driver, and I can't connect all my different modules to one xxx_core.pf.y device in N-to-1 mapping. "N" represents different protocols (IB, ETH, SCSI) and "1" is one PCI core. Thanks > > -DaveE