Re: [PATCH v4 01/10] Add auxiliary bus support
On Tue, Nov 17, 2020 at 01:04:56PM -0800, Dan Williams wrote: > On Mon, Nov 16, 2020 at 11:02 PM Greg KH wrote: > > > > On Tue, Nov 17, 2020 at 07:30:00AM +0200, Leon Romanovsky wrote: > > > On Fri, Nov 13, 2020 at 08:18:50AM -0800, Dave Ertman wrote: > > > > Add support for the Auxiliary Bus, auxiliary_device and > > > > auxiliary_driver. > > > > It enables drivers to create an auxiliary_device and bind an > > > > auxiliary_driver to it. > > > > > > > > The bus supports probe/remove shutdown and suspend/resume callbacks. > > > > Each auxiliary_device has a unique string based id; driver binds to > > > > an auxiliary_device based on this id through the bus. > > > > > > > > Co-developed-by: Kiran Patil > > > > Signed-off-by: Kiran Patil > > > > Co-developed-by: Ranjani Sridharan > > > > Signed-off-by: Ranjani Sridharan > > > > Co-developed-by: Fred Oh > > > > Signed-off-by: Fred Oh > > > > Co-developed-by: Leon Romanovsky > > > > Signed-off-by: Leon Romanovsky > > > > Reviewed-by: Pierre-Louis Bossart > > > > Reviewed-by: Shiraz Saleem > > > > Reviewed-by: Parav Pandit > > > > Reviewed-by: Dan Williams > > > > Signed-off-by: Dave Ertman > > > > --- > > > > > > Greg, > > > > > > This horse was beaten to death, can we please progress with this patch? > > > Create special topic branch or ack so I'll prepare this branch. > > > > > > We are in -rc4 now and we (Mellanox) can't hold our submissions anymore. > > > My mlx5_core probe patches [1] were too intrusive and they are ready to > > > be merged, Parav's patches got positive review as well [2] and will be > > > taken next. > > > > > > We delayed and have in our internal queues the patches for VDPA, eswitch > > > and followup for mlx5_core probe rework, but trapped due to this AUX bus > > > patch. > > > > There are no deadlines for kernel patches here, sorry. Give me some > > time to properly review this, core kernel changes should not be rushed. > > > > Also, if you really want to blame someone for the delay, look at the > > patch submitters not the reviewers, as they are the ones that took a > > very long time with this over the lifecycle of this patchset, not me. I > > have provided many "instant" reviews of this patchset, and then months > > went by between updates from them. > > Please stop this finger pointing. It was already noted that the team, > out of abundance of caution / deference to the process, decided not to > push the patches while I was out on family leave. It's cruel to hold > that against them, and if anyone is to blame it's me for not > clarifying it was ok to proceed while I was out. I'm not blaming anyone, I'm just getting pissed when people are insisting that I do "quick reviews" for this patchset, which has been happening by different people since the very beginning of this whole feature, so I am trying to explain where others should be pointing their frustration at instead of me if they really want to do such a thing (hint, they shouldn't, but I wasn't explicit about that, sorry). Combine this with the long delays between my reviews and a new patchset submission, and on my end it's an extremely frustrating situation, which frankly, makes me want to review this thing even less and less as I know it's not going to be a fun or easy time when I do so. Everyone needs to remember that there are no deadlines here, and the people involved all have other things to work on at the same time, and that there are a lot of different subsystems and moving parts all involved. So someone is going to get grumpy about it, and right now, it seems to be me. I know I need to review this, and complaining that I haven't done so within 3 days of sending an updated patch set is not helping anyone. I'm going to try to carve out some time this week to review this properly. Hopefully there's no other major security "scares" popping up like there was the past few weeks to divert me from this... thanks, greg k-h
Re: [PATCH v4 01/10] Add auxiliary bus support
On Mon, Nov 16, 2020 at 11:02 PM Greg KH wrote: > > On Tue, Nov 17, 2020 at 07:30:00AM +0200, Leon Romanovsky wrote: > > On Fri, Nov 13, 2020 at 08:18:50AM -0800, Dave Ertman wrote: > > > Add support for the Auxiliary Bus, auxiliary_device and auxiliary_driver. > > > It enables drivers to create an auxiliary_device and bind an > > > auxiliary_driver to it. > > > > > > The bus supports probe/remove shutdown and suspend/resume callbacks. > > > Each auxiliary_device has a unique string based id; driver binds to > > > an auxiliary_device based on this id through the bus. > > > > > > Co-developed-by: Kiran Patil > > > Signed-off-by: Kiran Patil > > > Co-developed-by: Ranjani Sridharan > > > Signed-off-by: Ranjani Sridharan > > > Co-developed-by: Fred Oh > > > Signed-off-by: Fred Oh > > > Co-developed-by: Leon Romanovsky > > > Signed-off-by: Leon Romanovsky > > > Reviewed-by: Pierre-Louis Bossart > > > Reviewed-by: Shiraz Saleem > > > Reviewed-by: Parav Pandit > > > Reviewed-by: Dan Williams > > > Signed-off-by: Dave Ertman > > > --- > > > > Greg, > > > > This horse was beaten to death, can we please progress with this patch? > > Create special topic branch or ack so I'll prepare this branch. > > > > We are in -rc4 now and we (Mellanox) can't hold our submissions anymore. > > My mlx5_core probe patches [1] were too intrusive and they are ready to > > be merged, Parav's patches got positive review as well [2] and will be > > taken next. > > > > We delayed and have in our internal queues the patches for VDPA, eswitch > > and followup for mlx5_core probe rework, but trapped due to this AUX bus > > patch. > > There are no deadlines for kernel patches here, sorry. Give me some > time to properly review this, core kernel changes should not be rushed. > > Also, if you really want to blame someone for the delay, look at the > patch submitters not the reviewers, as they are the ones that took a > very long time with this over the lifecycle of this patchset, not me. I > have provided many "instant" reviews of this patchset, and then months > went by between updates from them. Please stop this finger pointing. It was already noted that the team, out of abundance of caution / deference to the process, decided not to push the patches while I was out on family leave. It's cruel to hold that against them, and if anyone is to blame it's me for not clarifying it was ok to proceed while I was out.
Re: [PATCH v4 01/10] Add auxiliary bus support
On Fri, Nov 13, 2020 at 08:18:50AM -0800, Dave Ertman wrote: > Add support for the Auxiliary Bus, auxiliary_device and auxiliary_driver. > It enables drivers to create an auxiliary_device and bind an > auxiliary_driver to it. > > The bus supports probe/remove shutdown and suspend/resume callbacks. > Each auxiliary_device has a unique string based id; driver binds to > an auxiliary_device based on this id through the bus. Reviewed-by: Martin Habets > Co-developed-by: Kiran Patil > Signed-off-by: Kiran Patil > Co-developed-by: Ranjani Sridharan > Signed-off-by: Ranjani Sridharan > Co-developed-by: Fred Oh > Signed-off-by: Fred Oh > Co-developed-by: Leon Romanovsky > Signed-off-by: Leon Romanovsky > Reviewed-by: Pierre-Louis Bossart > Reviewed-by: Shiraz Saleem > Reviewed-by: Parav Pandit > Reviewed-by: Dan Williams > Signed-off-by: Dave Ertman > --- > Documentation/driver-api/auxiliary_bus.rst | 234 ++ > Documentation/driver-api/index.rst | 1 + > drivers/base/Kconfig | 3 + > drivers/base/Makefile | 1 + > drivers/base/auxiliary.c | 268 + > include/linux/auxiliary_bus.h | 78 ++ > include/linux/mod_devicetable.h| 8 + > scripts/mod/devicetable-offsets.c | 3 + > scripts/mod/file2alias.c | 8 + > 9 files changed, 604 insertions(+) > create mode 100644 Documentation/driver-api/auxiliary_bus.rst > create mode 100644 drivers/base/auxiliary.c > create mode 100644 include/linux/auxiliary_bus.h > > diff --git a/Documentation/driver-api/auxiliary_bus.rst > b/Documentation/driver-api/auxiliary_bus.rst > new file mode 100644 > index ..5dd7804631ef > --- /dev/null > +++ b/Documentation/driver-api/auxiliary_bus.rst > @@ -0,0 +1,234 @@ > +.. SPDX-License-Identifier: GPL-2.0-only > + > += > +Auxiliary Bus > += > + > +In some subsystems, the functionality of the core device (PCI/ACPI/other) is > +too complex for a single device to be managed by a monolithic driver > +(e.g. Sound Open Firmware), multiple devices might implement a common > +intersection of functionality (e.g. NICs + RDMA), or a driver may want to > +export an interface for another subsystem to drive (e.g. SIOV Physical > Function > +export Virtual Function management). A split of the functinoality into > child- > +devices representing sub-domains of functionality makes it possible to > +compartmentalize, layer, and distribute domain-specific concerns via a Linux > +device-driver model. > + > +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. > + > +Each auxiliary_device represents a part of its parent functionality. The > +generic behavior can be extended and specialized as needed by encapsulating > an > +auxiliary_device within other domain-specific structures and the use of .ops > +callbacks. Devices on the auxiliary bus do not share any structures and the > use > +of a communication channel with the parent is domain-specific. > + > +Note that ops are intended as a way to augment instance behavior within a > class > +of auxiliary devices, it is not the mechanism for exporting common > +infrastructure from the parent. Consider EXPORT_SYMBOL_NS() to convey > +infrastructure from the parent module to the auxiliary module(s). > + > + > +When Should the Auxiliary Bus Be Used > += > + > +The auxiliary 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 auxiliary_device's > +registering driver. The registering driver for the auxiliary_device(s) and > the > +kernel module(s) registering auxiliary_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 is a PCI network device that is RDMA-capable and exports a child > +device to be driven by an auxiliary_driver in the RDMA subsystem. The PCI > +driver allocates and registers an auxiliary_device for each physical > +function on the NIC. The RDMA driver registers an auxiliary_driver that > claims > +each of these auxiliary_devices. This conveys data/ops published by the > parent > +PCI device/driver to the RDMA auxiliary_driver. > + > +Another use case is for the PCI device to be split out into multiple sub > +functions. For each sub
Re: [PATCH v4 01/10] Add auxiliary bus support
On Tue, Nov 17, 2020 at 03:57:24PM +0200, Leon Romanovsky wrote: > On Tue, Nov 17, 2020 at 01:48:08PM +, Mark Brown wrote: > > On Tue, Nov 17, 2020 at 07:30:00AM +0200, Leon Romanovsky wrote: > > > On Fri, Nov 13, 2020 at 08:18:50AM -0800, Dave Ertman wrote: > > > > > > Add support for the Auxiliary Bus, auxiliary_device and > > > > auxiliary_driver. > > > > It enables drivers to create an auxiliary_device and bind an > > > > auxiliary_driver to it. > > > > > This horse was beaten to death, can we please progress with this patch? > > > Create special topic branch or ack so I'll prepare this branch. > > > > It's been about 2 working days since the patch was last posted. > > There is no code changes between v3 and v4 except docs improvements. > The v3 was posted almost 3-4 weeks ago. But everything else that came in since then needs to be reviewed first, right? It's a fifo queue, no jumping the line. And, to be frank, if people complain, that's a sure way for it to get dumped to the end of the line, as you know. thanks, greg k-h
Re: [PATCH v4 01/10] Add auxiliary bus support
On Tue, Nov 17, 2020 at 01:48:08PM +, Mark Brown wrote: > On Tue, Nov 17, 2020 at 07:30:00AM +0200, Leon Romanovsky wrote: > > On Fri, Nov 13, 2020 at 08:18:50AM -0800, Dave Ertman wrote: > > > > Add support for the Auxiliary Bus, auxiliary_device and auxiliary_driver. > > > It enables drivers to create an auxiliary_device and bind an > > > auxiliary_driver to it. > > > This horse was beaten to death, can we please progress with this patch? > > Create special topic branch or ack so I'll prepare this branch. > > It's been about 2 working days since the patch was last posted. There is no code changes between v3 and v4 except docs improvements. The v3 was posted almost 3-4 weeks ago. Thanks
Re: [PATCH v4 01/10] Add auxiliary bus support
On Tue, Nov 17, 2020 at 07:30:00AM +0200, Leon Romanovsky wrote: > On Fri, Nov 13, 2020 at 08:18:50AM -0800, Dave Ertman wrote: > > Add support for the Auxiliary Bus, auxiliary_device and auxiliary_driver. > > It enables drivers to create an auxiliary_device and bind an > > auxiliary_driver to it. > This horse was beaten to death, can we please progress with this patch? > Create special topic branch or ack so I'll prepare this branch. It's been about 2 working days since the patch was last posted. signature.asc Description: PGP signature
Re: [PATCH v4 01/10] Add auxiliary bus support
On Tue, Nov 17, 2020 at 08:02:53AM +0100, Greg KH wrote: > On Tue, Nov 17, 2020 at 07:30:00AM +0200, Leon Romanovsky wrote: > > On Fri, Nov 13, 2020 at 08:18:50AM -0800, Dave Ertman wrote: > > > Add support for the Auxiliary Bus, auxiliary_device and auxiliary_driver. > > > It enables drivers to create an auxiliary_device and bind an > > > auxiliary_driver to it. > > > > > > The bus supports probe/remove shutdown and suspend/resume callbacks. > > > Each auxiliary_device has a unique string based id; driver binds to > > > an auxiliary_device based on this id through the bus. > > > > > > Co-developed-by: Kiran Patil > > > Signed-off-by: Kiran Patil > > > Co-developed-by: Ranjani Sridharan > > > Signed-off-by: Ranjani Sridharan > > > Co-developed-by: Fred Oh > > > Signed-off-by: Fred Oh > > > Co-developed-by: Leon Romanovsky > > > Signed-off-by: Leon Romanovsky > > > Reviewed-by: Pierre-Louis Bossart > > > Reviewed-by: Shiraz Saleem > > > Reviewed-by: Parav Pandit > > > Reviewed-by: Dan Williams > > > Signed-off-by: Dave Ertman > > > --- > > > > Greg, > > > > This horse was beaten to death, can we please progress with this patch? > > Create special topic branch or ack so I'll prepare this branch. > > > > We are in -rc4 now and we (Mellanox) can't hold our submissions anymore. > > My mlx5_core probe patches [1] were too intrusive and they are ready to > > be merged, Parav's patches got positive review as well [2] and will be > > taken next. > > > > We delayed and have in our internal queues the patches for VDPA, eswitch > > and followup for mlx5_core probe rework, but trapped due to this AUX bus > > patch. > > There are no deadlines for kernel patches here, sorry. Give me some > time to properly review this, core kernel changes should not be rushed. And here comes the difference between our views, from my POV it is not core kernel change that must to be done perfectly from the beginning, but change that will need to be improved/extended over time once more users will come. > > Also, if you really want to blame someone for the delay, look at the > patch submitters, not the reviewers, as they are the ones that took a > very long time with this over the lifecycle of this patchset, not me. I > have provided many "instant" reviews of this patchset, and then months > went by between updates from them. I'm not blaming anyone and especially you. It is just unfair that I found myself in the middle of this disaster while care enough to remind about the series. Thanks > > greg k-h
Re: [PATCH v4 01/10] Add auxiliary bus support
On Tue, Nov 17, 2020 at 07:30:00AM +0200, Leon Romanovsky wrote: > On Fri, Nov 13, 2020 at 08:18:50AM -0800, Dave Ertman wrote: > > Add support for the Auxiliary Bus, auxiliary_device and auxiliary_driver. > > It enables drivers to create an auxiliary_device and bind an > > auxiliary_driver to it. > > > > The bus supports probe/remove shutdown and suspend/resume callbacks. > > Each auxiliary_device has a unique string based id; driver binds to > > an auxiliary_device based on this id through the bus. > > > > Co-developed-by: Kiran Patil > > Signed-off-by: Kiran Patil > > Co-developed-by: Ranjani Sridharan > > Signed-off-by: Ranjani Sridharan > > Co-developed-by: Fred Oh > > Signed-off-by: Fred Oh > > Co-developed-by: Leon Romanovsky > > Signed-off-by: Leon Romanovsky > > Reviewed-by: Pierre-Louis Bossart > > Reviewed-by: Shiraz Saleem > > Reviewed-by: Parav Pandit > > Reviewed-by: Dan Williams > > Signed-off-by: Dave Ertman > > --- > > Greg, > > This horse was beaten to death, can we please progress with this patch? > Create special topic branch or ack so I'll prepare this branch. > > We are in -rc4 now and we (Mellanox) can't hold our submissions anymore. > My mlx5_core probe patches [1] were too intrusive and they are ready to > be merged, Parav's patches got positive review as well [2] and will be > taken next. > > We delayed and have in our internal queues the patches for VDPA, eswitch > and followup for mlx5_core probe rework, but trapped due to this AUX bus > patch. There are no deadlines for kernel patches here, sorry. Give me some time to properly review this, core kernel changes should not be rushed. Also, if you really want to blame someone for the delay, look at the patch submitters, not the reviewers, as they are the ones that took a very long time with this over the lifecycle of this patchset, not me. I have provided many "instant" reviews of this patchset, and then months went by between updates from them. greg k-h
Re: [PATCH v4 01/10] Add auxiliary bus support
On Fri, Nov 13, 2020 at 08:18:50AM -0800, Dave Ertman wrote: > Add support for the Auxiliary Bus, auxiliary_device and auxiliary_driver. > It enables drivers to create an auxiliary_device and bind an > auxiliary_driver to it. > > The bus supports probe/remove shutdown and suspend/resume callbacks. > Each auxiliary_device has a unique string based id; driver binds to > an auxiliary_device based on this id through the bus. > > Co-developed-by: Kiran Patil > Signed-off-by: Kiran Patil > Co-developed-by: Ranjani Sridharan > Signed-off-by: Ranjani Sridharan > Co-developed-by: Fred Oh > Signed-off-by: Fred Oh > Co-developed-by: Leon Romanovsky > Signed-off-by: Leon Romanovsky > Reviewed-by: Pierre-Louis Bossart > Reviewed-by: Shiraz Saleem > Reviewed-by: Parav Pandit > Reviewed-by: Dan Williams > Signed-off-by: Dave Ertman > --- Greg, This horse was beaten to death, can we please progress with this patch? Create special topic branch or ack so I'll prepare this branch. We are in -rc4 now and we (Mellanox) can't hold our submissions anymore. My mlx5_core probe patches [1] were too intrusive and they are ready to be merged, Parav's patches got positive review as well [2] and will be taken next. We delayed and have in our internal queues the patches for VDPA, eswitch and followup for mlx5_core probe rework, but trapped due to this AUX bus patch. Thanks [1] https://lore.kernel.org/linux-rdma/20201101201542.2027568-1-l...@kernel.org/ [2] https://lore.kernel.org/linux-rdma/by5pr12mb43229f23c101afbcd2971534dc...@by5pr12mb4322.namprd12.prod.outlook.com/T/#md25b9a2feb4c60d3fef5d57da41db559af1062d8
