Re: [PATCH v3 01/10] Add auxiliary bus support
On Sat, Nov 14, 2020 at 12:21:39AM +0100, Greg KH wrote: > On Fri, Nov 13, 2020 at 04:07:57PM +, Ertman, David M wrote: > > > -Original Message- > > > From: Greg KH > > > Sent: Friday, November 13, 2020 7:50 AM > > > To: Ertman, David M > > > Cc: alsa-de...@alsa-project.org; ti...@suse.de; broo...@kernel.org; linux- > > > r...@vger.kernel.org; j...@nvidia.com; dledf...@redhat.com; > > > net...@vger.kernel.org; da...@davemloft.net; k...@kernel.org; > > > ranjani.sridha...@linux.intel.com; pierre-louis.boss...@linux.intel.com; > > > fred...@linux.intel.com; pa...@mellanox.com; Saleem, Shiraz > > > ; Williams, Dan J ; > > > Patil, Kiran ; linux-kernel@vger.kernel.org; > > > leo...@nvidia.com > > > Subject: Re: [PATCH v3 01/10] Add auxiliary bus support > > > > > > On Thu, Oct 22, 2020 at 05:33:29PM -0700, 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 > > > > --- > > > > > > Is this really the "latest" version of this patch submission? > > > > > > I see a number of comments on it already, have you sent out a newer one, > > > or is this the same one that the mlx5 driver conversion was done on top > > > of? > > > > > > thanks, > > > > > > greg k-h > > > > V3 is the latest sent so far. There was a suggestion that v3 might be > > merged and > > the documentation changes could be in a follow up patch. I have those > > changes done > > and ready though, so no reason not to merge them in and do a resend. > > > > Please expect v4 in just a little while. > > Thank you, follow-up patches aren't usually a good idea :) The changes were in documentation area that will be changed anyway after dust will settle and we all see real users and more or less stable in-kernel API. Thanks
Re: [PATCH v3 01/10] Add auxiliary bus support
On Fri, Nov 13, 2020 at 04:07:57PM +, Ertman, David M wrote: > > -Original Message- > > From: Greg KH > > Sent: Friday, November 13, 2020 7:50 AM > > To: Ertman, David M > > Cc: alsa-de...@alsa-project.org; ti...@suse.de; broo...@kernel.org; linux- > > r...@vger.kernel.org; j...@nvidia.com; dledf...@redhat.com; > > net...@vger.kernel.org; da...@davemloft.net; k...@kernel.org; > > ranjani.sridha...@linux.intel.com; pierre-louis.boss...@linux.intel.com; > > fred...@linux.intel.com; pa...@mellanox.com; Saleem, Shiraz > > ; Williams, Dan J ; > > Patil, Kiran ; linux-kernel@vger.kernel.org; > > leo...@nvidia.com > > Subject: Re: [PATCH v3 01/10] Add auxiliary bus support > > > > On Thu, Oct 22, 2020 at 05:33:29PM -0700, 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 > > > --- > > > > Is this really the "latest" version of this patch submission? > > > > I see a number of comments on it already, have you sent out a newer one, > > or is this the same one that the mlx5 driver conversion was done on top > > of? > > > > thanks, > > > > greg k-h > > V3 is the latest sent so far. There was a suggestion that v3 might be merged > and > the documentation changes could be in a follow up patch. I have those > changes done > and ready though, so no reason not to merge them in and do a resend. > > Please expect v4 in just a little while. Thank you, follow-up patches aren't usually a good idea :)
RE: [PATCH v3 01/10] Add auxiliary bus support
> -Original Message- > From: Greg KH > Sent: Friday, November 13, 2020 7:50 AM > To: Ertman, David M > Cc: alsa-de...@alsa-project.org; ti...@suse.de; broo...@kernel.org; linux- > r...@vger.kernel.org; j...@nvidia.com; dledf...@redhat.com; > net...@vger.kernel.org; da...@davemloft.net; k...@kernel.org; > ranjani.sridha...@linux.intel.com; pierre-louis.boss...@linux.intel.com; > fred...@linux.intel.com; pa...@mellanox.com; Saleem, Shiraz > ; Williams, Dan J ; > Patil, Kiran ; linux-kernel@vger.kernel.org; > leo...@nvidia.com > Subject: Re: [PATCH v3 01/10] Add auxiliary bus support > > On Thu, Oct 22, 2020 at 05:33:29PM -0700, 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 > > --- > > Is this really the "latest" version of this patch submission? > > I see a number of comments on it already, have you sent out a newer one, > or is this the same one that the mlx5 driver conversion was done on top > of? > > thanks, > > greg k-h V3 is the latest sent so far. There was a suggestion that v3 might be merged and the documentation changes could be in a follow up patch. I have those changes done and ready though, so no reason not to merge them in and do a resend. Please expect v4 in just a little while. Thanks, -DaveE
Re: [PATCH v3 01/10] Add auxiliary bus support
On Thu, Oct 22, 2020 at 05:33:29PM -0700, 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 > --- Is this really the "latest" version of this patch submission? I see a number of comments on it already, have you sent out a newer one, or is this the same one that the mlx5 driver conversion was done on top of? thanks, greg k-h
Re: [PATCH v3 01/10] Add auxiliary bus support
On Fri, Nov 06, 2020 at 07:35:37PM +, Mark Brown wrote: > On Thu, Nov 05, 2020 at 08:37:14PM +, Parav Pandit wrote: > > > > > This example describes the mlx5 PCI subfunction use case. > > > > I didn't follow your question about 'explicit example'. > > > > What part is missing to identify it as explicit example? > > > > Specifically listing "mlx5" so if someone reading this document thinks to > > > themselves "hey mlx5 sounds like my use case" they can go grep for that. > > > Ah, I see. > > "mlx5" is not listed explicitly, because it is not included in this > > patchset. > > In various previous discussions in this thread, mlx5 subfunction use case > > is described that justifies the existence of the bus. > > I will be happy to update this documentation once mlx5 subfunction will be > > part of kernel so that grep actually shows valid output. > > (waiting to post them as it uses auxiliary bus :-)). > > For ease of review if there's a new version it might be as well to just > reference it anyway, hopefully the mlx5 code will be merged fairly > quickly once the bus itself is merged. It's probably easier all round > than adding the reference later, it seems more likely that mlx5 will get > merged than that it'll fall by the wayside. Another use-case for this patch-set is going to be the habanalabs driver. The GAUDI ASIC is a PCI H/W accelerator for deep-learning which also exposes network ports.We are going to use this auxiliary-bus feature to separate our monolithic driver into several parts that will reside in different subsystems and communicate between them through the bus. Thanks, Oded
Re: [PATCH v3 01/10] Add auxiliary bus support
On Thu, Nov 05, 2020 at 08:37:14PM +, Parav Pandit wrote: > > > This example describes the mlx5 PCI subfunction use case. > > > I didn't follow your question about 'explicit example'. > > > What part is missing to identify it as explicit example? > > Specifically listing "mlx5" so if someone reading this document thinks to > > themselves "hey mlx5 sounds like my use case" they can go grep for that. > Ah, I see. > "mlx5" is not listed explicitly, because it is not included in this patchset. > In various previous discussions in this thread, mlx5 subfunction use case is > described that justifies the existence of the bus. > I will be happy to update this documentation once mlx5 subfunction will be > part of kernel so that grep actually shows valid output. > (waiting to post them as it uses auxiliary bus :-)). For ease of review if there's a new version it might be as well to just reference it anyway, hopefully the mlx5 code will be merged fairly quickly once the bus itself is merged. It's probably easier all round than adding the reference later, it seems more likely that mlx5 will get merged than that it'll fall by the wayside. signature.asc Description: PGP signature
RE: [PATCH v3 01/10] Add auxiliary bus support
> -Original Message- > From: Dan Williams > Sent: Thursday, November 5, 2020 2:00 PM > To: Ertman, David M > Cc: alsa-de...@alsa-project.org; Takashi Iwai ; Mark Brown > ; linux-rdma ; Jason > Gunthorpe ; Doug Ledford ; > Netdev ; David Miller ; > Jakub Kicinski ; Greg KH ; > Ranjani Sridharan ; Pierre-Louis Bossart > ; Fred Oh ; > Parav Pandit ; Saleem, Shiraz > ; Patil, Kiran ; Linux > Kernel Mailing List ; Leon Romanovsky > > Subject: Re: [PATCH v3 01/10] Add auxiliary bus support > > On Thu, Nov 5, 2020 at 11:28 AM Ertman, David M > wrote: > [..] > > > > 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. > > > > > > Should there be any guidance here on when to use ops and when to just > > > export functions from parent driver to child. EXPORT_SYMBOL_NS() > seems > > > a perfect fit for publishing shared routines between parent and child. > > > > > > > I would leave this up the driver writers to determine what is best for them. > > I think there is a pathological case that can be avoided with a > statement like the following: > > "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)." > > As for your other dispositions of the feedback, looks good to me. I will add this in. -DaveE
Re: [PATCH v3 01/10] Add auxiliary bus support
On Thu, Nov 5, 2020 at 11:28 AM Ertman, David M wrote: [..] > > > 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. > > > > Should there be any guidance here on when to use ops and when to just > > export functions from parent driver to child. EXPORT_SYMBOL_NS() seems > > a perfect fit for publishing shared routines between parent and child. > > > > I would leave this up the driver writers to determine what is best for them. I think there is a pathological case that can be avoided with a statement like the following: "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)." As for your other dispositions of the feedback, looks good to me.
RE: [PATCH v3 01/10] Add auxiliary bus support
> -Original Message- > From: Leon Romanovsky > Sent: Thursday, November 5, 2020 11:35 AM > To: Ertman, David M > Cc: Williams, Dan J ; alsa-de...@alsa-project.org; > Takashi Iwai ; Mark Brown ; linux- > rdma ; Jason Gunthorpe ; > Doug Ledford ; Netdev ; > David Miller ; Jakub Kicinski ; > Greg KH ; Ranjani Sridharan > ; Pierre-Louis Bossart louis.boss...@linux.intel.com>; Fred Oh ; Parav > Pandit ; Saleem, Shiraz ; > Patil, Kiran ; Linux Kernel Mailing List ker...@vger.kernel.org> > Subject: Re: [PATCH v3 01/10] Add auxiliary bus support > > On Thu, Nov 05, 2020 at 07:27:56PM +, Ertman, David M wrote: > > > -Original Message- > > > From: Dan Williams > > > Sent: Thursday, November 5, 2020 1:19 AM > > > To: Ertman, David M > > > Cc: alsa-de...@alsa-project.org; Takashi Iwai ; Mark > Brown > > > ; linux-rdma ; Jason > > > Gunthorpe ; Doug Ledford ; > > > Netdev ; David Miller > ; > > > Jakub Kicinski ; Greg KH > ; > > > Ranjani Sridharan ; Pierre-Louis > Bossart > > > ; Fred Oh > ; > > > Parav Pandit ; Saleem, Shiraz > > > ; Patil, Kiran ; Linux > > > Kernel Mailing List ; Leon Romanovsky > > > > > > Subject: Re: [PATCH v3 01/10] Add auxiliary bus support > > > > > > Some doc fixups, and minor code feedback. Otherwise looks good to me. > > > > > > On Thu, Oct 22, 2020 at 5:35 PM Dave Ertman > > > > wrote: > > <...> > > > > > Again, thanks for the review Dan. Changes will be in next release (v4) once > I give > > stake-holders a little time to respond. > > Everything here can go as a Fixes, the review comments are valuable and > need > to be fixed, but they don't change anything dramatically that prevent from > merging v3. > This works for me - I have the changes saved into an add-on patch that I haven't squashed into the main patch yet. > Thanks > > > > > -DaveE
RE: [PATCH v3 01/10] Add auxiliary bus support
> From: Dan Williams > Sent: Friday, November 6, 2020 1:56 AM > > On Thu, Nov 5, 2020 at 11:40 AM Parav Pandit wrote: > > > > > > > > > From: Ertman, David M > > > Sent: Friday, November 6, 2020 12:58 AM > > > Subject: RE: [PATCH v3 01/10] Add auxiliary bus support > > > > > > > -Original Message- > > > > From: Dan Williams > > > > Sent: Thursday, November 5, 2020 1:19 AM > > > > > > > > [..] > > > > > + > > > > > +Another use case is for the PCI device to be split out into > > > > > +multiple sub functions. For each sub function an > > > > > +auxiliary_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 auxiliary 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 auxiliary_device. > > > > > > > > This does not read like an explicit example like the previous 2. > > > > Did you have something specific in mind? > > > > > > > > > > This was added by request of Parav. > > > > > This example describes the mlx5 PCI subfunction use case. > > I didn't follow your question about 'explicit example'. > > What part is missing to identify it as explicit example? > > Specifically listing "mlx5" so if someone reading this document thinks to > themselves "hey mlx5 sounds like my use case" they can go grep for that. Ah, I see. "mlx5" is not listed explicitly, because it is not included in this patchset. In various previous discussions in this thread, mlx5 subfunction use case is described that justifies the existence of the bus. I will be happy to update this documentation once mlx5 subfunction will be part of kernel so that grep actually shows valid output. (waiting to post them as it uses auxiliary bus :-)).
Re: [PATCH v3 01/10] Add auxiliary bus support
On Thu, Nov 5, 2020 at 11:30 AM Leon Romanovsky wrote:
>
> On Thu, Nov 05, 2020 at 09:12:51AM -0800, Dan Williams wrote:
> > On Thu, Nov 5, 2020 at 1:47 AM Leon Romanovsky wrote:
> > >
> > > On Thu, Nov 05, 2020 at 01:19:09AM -0800, Dan Williams wrote:
> > > > Some doc fixups, and minor code feedback. Otherwise looks good to me.
> > > >
> > > > On Thu, Oct 22, 2020 at 5:35 PM Dave Ertman
> > > > wrote:
> > > > >
> > >
> > > <...>
> > >
> > > > >
> > > > > +config AUXILIARY_BUS
> > > > > + bool
> > > >
> > > > tristate? Unless you need non-exported symbols, might as well let this
> > > > be a module.
> > >
> > > I asked it to be "bool", because bus as a module is an invitation for
> > > a disaster. For example if I compile-in mlx5 which is based on this bus,
> > > and won't add auxiliary_bus as a module to initramfs, the system won't
> > > boot.
> >
> > Something is broken if module dependencies don't arrange for
> > auxiliary_bus.ko to be added to the initramfs automatically, but yes,
> > it is another degree of freedom for something to go wrong if you build
> > the initramfs by hand.
>
> And this is something that I would like to avoid for now.
Fair enough.
>
> >
> > >
> > > <...>
> > >
> > > >
> > > > Per above SPDX is v2 only, so...
> > >
> > > Isn't it default for the Linux kernel?
> >
> > SPDX eliminated the need to guess a default, and MODULE_LICENSE("GPL")
> > implies the "or later" language. The only default assumption is that
> > the license is GPL v2 compatible, those possibilities are myriad, but
> > v2-only is the first preference.
>
> I mean that plain GPL == GPL v2 in the kernel.
You are right, I was wrong.
Re: [PATCH v3 01/10] Add auxiliary bus support
On Thu, Nov 5, 2020 at 11:40 AM Parav Pandit wrote: > > > > > From: Ertman, David M > > Sent: Friday, November 6, 2020 12:58 AM > > Subject: RE: [PATCH v3 01/10] Add auxiliary bus support > > > > > -Original Message- > > > From: Dan Williams > > > Sent: Thursday, November 5, 2020 1:19 AM > > > > > [..] > > > > + > > > > +Another use case is for the PCI device to be split out into > > > > +multiple sub functions. For each sub function an auxiliary_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 auxiliary 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 auxiliary_device. > > > > > > This does not read like an explicit example like the previous 2. Did > > > you have something specific in mind? > > > > > > > This was added by request of Parav. > > > This example describes the mlx5 PCI subfunction use case. > I didn't follow your question about 'explicit example'. > What part is missing to identify it as explicit example? Specifically listing "mlx5" so if someone reading this document thinks to themselves "hey mlx5 sounds like my use case" they can go grep for that.
Re: [PATCH v3 01/10] Add auxiliary bus support
On Thu, Nov 5, 2020 at 12:21 PM Greg KH wrote:
>
> On Thu, Nov 05, 2020 at 09:12:51AM -0800, Dan Williams wrote:
> > > >
> > > > Per above SPDX is v2 only, so...
> > >
> > > Isn't it default for the Linux kernel?
> >
> > SPDX eliminated the need to guess a default, and MODULE_LICENSE("GPL")
> > implies the "or later" language. The only default assumption is that
> > the license is GPL v2 compatible, those possibilities are myriad, but
> > v2-only is the first preference.
>
> No, MODULE_LICENSE("GPL") does not imply "or later" at all. Please see
> include/linux/module.h, it means "GPL version 2".
>
Oh, I did, and stopped before getting to:
"only/or later" distinction is completely irrelevant and does neither
*replace the proper license identifiers in the corresponding source file
...sorry for the noise.
Re: [PATCH v3 01/10] Add auxiliary bus support
On Thu, Nov 05, 2020 at 09:12:51AM -0800, Dan Williams wrote:
> > >
> > > Per above SPDX is v2 only, so...
> >
> > Isn't it default for the Linux kernel?
>
> SPDX eliminated the need to guess a default, and MODULE_LICENSE("GPL")
> implies the "or later" language. The only default assumption is that
> the license is GPL v2 compatible, those possibilities are myriad, but
> v2-only is the first preference.
No, MODULE_LICENSE("GPL") does not imply "or later" at all. Please see
include/linux/module.h, it means "GPL version 2".
thanks,
greg k-h
RE: [PATCH v3 01/10] Add auxiliary bus support
> From: Ertman, David M > Sent: Friday, November 6, 2020 12:58 AM > Subject: RE: [PATCH v3 01/10] Add auxiliary bus support > > > -Original Message- > > From: Dan Williams > > Sent: Thursday, November 5, 2020 1:19 AM > > [..] > > > + > > > +Another use case is for the PCI device to be split out into > > > +multiple sub functions. For each sub function an auxiliary_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 auxiliary 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 auxiliary_device. > > > > This does not read like an explicit example like the previous 2. Did > > you have something specific in mind? > > > > This was added by request of Parav. > This example describes the mlx5 PCI subfunction use case. I didn't follow your question about 'explicit example'. What part is missing to identify it as explicit example?
Re: [PATCH v3 01/10] Add auxiliary bus support
On Thu, Nov 05, 2020 at 01:32:40PM -0600, Pierre-Louis Bossart wrote:
>
> > > > +module_init(auxiliary_bus_init);
> > > > +module_exit(auxiliary_bus_exit);
> > > > +
> > > > +MODULE_LICENSE("GPL");
> > >
> > > Per above SPDX is v2 only, so...
> > >
> > > MODULE_LICENSE("GPL v2");
> > >
> >
> > added v2.
>
> "GPL v2" is the same as "GPL" here, it does not have any additional meaning.
>
> https://www.kernel.org/doc/html/latest/process/license-rules.html
>
> “GPL” Module is licensed under GPL version 2. This does not express any
> distinction between GPL-2.0-only or GPL-2.0-or-later. The exact license
> information can only be determined via the license information in the
> corresponding source files.
+1,
https://lore.kernel.org/lkml/20201105193009.GA5475@unreal
>
> “GPL v2” Same as “GPL”. It exists for historic reasons.
>
Re: [PATCH v3 01/10] Add auxiliary bus support
On Thu, Nov 05, 2020 at 07:27:56PM +, Ertman, David M wrote: > > -Original Message- > > From: Dan Williams > > Sent: Thursday, November 5, 2020 1:19 AM > > To: Ertman, David M > > Cc: alsa-de...@alsa-project.org; Takashi Iwai ; Mark Brown > > ; linux-rdma ; Jason > > Gunthorpe ; Doug Ledford ; > > Netdev ; David Miller ; > > Jakub Kicinski ; Greg KH ; > > Ranjani Sridharan ; Pierre-Louis Bossart > > ; Fred Oh ; > > Parav Pandit ; Saleem, Shiraz > > ; Patil, Kiran ; Linux > > Kernel Mailing List ; Leon Romanovsky > > > > Subject: Re: [PATCH v3 01/10] Add auxiliary bus support > > > > Some doc fixups, and minor code feedback. Otherwise looks good to me. > > > > On Thu, Oct 22, 2020 at 5:35 PM Dave Ertman > > wrote: <...> > > Again, thanks for the review Dan. Changes will be in next release (v4) once > I give > stake-holders a little time to respond. Everything here can go as a Fixes, the review comments are valuable and need to be fixed, but they don't change anything dramatically that prevent from merging v3. Thanks > > -DaveE
Re: [PATCH v3 01/10] Add auxiliary bus support
+module_init(auxiliary_bus_init);
+module_exit(auxiliary_bus_exit);
+
+MODULE_LICENSE("GPL");
Per above SPDX is v2 only, so...
MODULE_LICENSE("GPL v2");
added v2.
"GPL v2" is the same as "GPL" here, it does not have any additional meaning.
https://www.kernel.org/doc/html/latest/process/license-rules.html
“GPL” Module is licensed under GPL version 2. This does not express any
distinction between GPL-2.0-only or GPL-2.0-or-later. The exact license
information can only be determined via the license information in the
corresponding source files.
“GPL v2”Same as “GPL”. It exists for historic reasons.
Re: [PATCH v3 01/10] Add auxiliary bus support
On Thu, Nov 05, 2020 at 09:12:51AM -0800, Dan Williams wrote:
> On Thu, Nov 5, 2020 at 1:47 AM Leon Romanovsky wrote:
> >
> > On Thu, Nov 05, 2020 at 01:19:09AM -0800, Dan Williams wrote:
> > > Some doc fixups, and minor code feedback. Otherwise looks good to me.
> > >
> > > On Thu, Oct 22, 2020 at 5:35 PM Dave Ertman
> > > wrote:
> > > >
> >
> > <...>
> >
> > > >
> > > > +config AUXILIARY_BUS
> > > > + bool
> > >
> > > tristate? Unless you need non-exported symbols, might as well let this
> > > be a module.
> >
> > I asked it to be "bool", because bus as a module is an invitation for
> > a disaster. For example if I compile-in mlx5 which is based on this bus,
> > and won't add auxiliary_bus as a module to initramfs, the system won't boot.
>
> Something is broken if module dependencies don't arrange for
> auxiliary_bus.ko to be added to the initramfs automatically, but yes,
> it is another degree of freedom for something to go wrong if you build
> the initramfs by hand.
And this is something that I would like to avoid for now.
>
> >
> > <...>
> >
> > >
> > > Per above SPDX is v2 only, so...
> >
> > Isn't it default for the Linux kernel?
>
> SPDX eliminated the need to guess a default, and MODULE_LICENSE("GPL")
> implies the "or later" language. The only default assumption is that
> the license is GPL v2 compatible, those possibilities are myriad, but
> v2-only is the first preference.
I mean that plain GPL == GPL v2 in the kernel.
Thanks
RE: [PATCH v3 01/10] Add auxiliary bus support
> -Original Message- > From: Dan Williams > Sent: Thursday, November 5, 2020 1:19 AM > To: Ertman, David M > Cc: alsa-de...@alsa-project.org; Takashi Iwai ; Mark Brown > ; linux-rdma ; Jason > Gunthorpe ; Doug Ledford ; > Netdev ; David Miller ; > Jakub Kicinski ; Greg KH ; > Ranjani Sridharan ; Pierre-Louis Bossart > ; Fred Oh ; > Parav Pandit ; Saleem, Shiraz > ; Patil, Kiran ; Linux > Kernel Mailing List ; Leon Romanovsky > > Subject: Re: [PATCH v3 01/10] Add auxiliary bus support > > Some doc fixups, and minor code feedback. Otherwise looks good to me. > > On Thu, Oct 22, 2020 at 5:35 PM 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 > > --- > > Documentation/driver-api/auxiliary_bus.rst | 228 ++ > > Documentation/driver-api/index.rst | 1 + > > drivers/base/Kconfig | 3 + > > drivers/base/Makefile | 1 + > > drivers/base/auxiliary.c | 267 + > > 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, 597 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 ..500f29692c81 > > --- /dev/null > > +++ b/Documentation/driver-api/auxiliary_bus.rst > > @@ -0,0 +1,228 @@ > > +.. 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 as a monolithic block or a > part of > > +the functionality needs to be exposed to a different subsystem. > > I think there are three identified use cases for this now, so call out > those examples for others to compare if aux bus is a fit for their use > case. > > "In some subsystems, the functionality of the core device > (PCI/ACPI/other) is too complex to be handled 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)." > Additional example added. Thanks for the review Dan! > > + Splitting the > > +functionality into smaller orthogonal devices would make it easier to > manage > > +data, power management and domain-specific interaction with the > hardware. > > Passive voice and I don't understand what is meant by the word "orthogonal" > > "A split of the functionality 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" > verbiage changed. > > 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. > > These las
Re: [PATCH v3 01/10] Add auxiliary bus support
On Thu, Nov 5, 2020 at 1:47 AM Leon Romanovsky wrote:
>
> On Thu, Nov 05, 2020 at 01:19:09AM -0800, Dan Williams wrote:
> > Some doc fixups, and minor code feedback. Otherwise looks good to me.
> >
> > On Thu, Oct 22, 2020 at 5:35 PM Dave Ertman
> > wrote:
> > >
>
> <...>
>
> > >
> > > +config AUXILIARY_BUS
> > > + bool
> >
> > tristate? Unless you need non-exported symbols, might as well let this
> > be a module.
>
> I asked it to be "bool", because bus as a module is an invitation for
> a disaster. For example if I compile-in mlx5 which is based on this bus,
> and won't add auxiliary_bus as a module to initramfs, the system won't boot.
Something is broken if module dependencies don't arrange for
auxiliary_bus.ko to be added to the initramfs automatically, but yes,
it is another degree of freedom for something to go wrong if you build
the initramfs by hand.
>
> <...>
>
> >
> > Per above SPDX is v2 only, so...
>
> Isn't it default for the Linux kernel?
SPDX eliminated the need to guess a default, and MODULE_LICENSE("GPL")
implies the "or later" language. The only default assumption is that
the license is GPL v2 compatible, those possibilities are myriad, but
v2-only is the first preference.
Re: [PATCH v3 01/10] Add auxiliary bus support
On Thu, Nov 05, 2020 at 01:19:09AM -0800, Dan Williams wrote:
> Some doc fixups, and minor code feedback. Otherwise looks good to me.
>
> On Thu, Oct 22, 2020 at 5:35 PM Dave Ertman wrote:
> >
<...>
> >
> > +config AUXILIARY_BUS
> > + bool
>
> tristate? Unless you need non-exported symbols, might as well let this
> be a module.
I asked it to be "bool", because bus as a module is an invitation for
a disaster. For example if I compile-in mlx5 which is based on this bus,
and won't add auxiliary_bus as a module to initramfs, the system won't boot.
<...>
>
> Per above SPDX is v2 only, so...
Isn't it default for the Linux kernel?
>
> MODULE_LICENSE("GPL v2");
Thanks
Re: [PATCH v3 01/10] Add auxiliary bus support
Some doc fixups, and minor code feedback. Otherwise looks good to me. On Thu, Oct 22, 2020 at 5:35 PM 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 > --- > Documentation/driver-api/auxiliary_bus.rst | 228 ++ > Documentation/driver-api/index.rst | 1 + > drivers/base/Kconfig | 3 + > drivers/base/Makefile | 1 + > drivers/base/auxiliary.c | 267 + > 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, 597 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 ..500f29692c81 > --- /dev/null > +++ b/Documentation/driver-api/auxiliary_bus.rst > @@ -0,0 +1,228 @@ > +.. 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 as a monolithic block or a > part of > +the functionality needs to be exposed to a different subsystem. I think there are three identified use cases for this now, so call out those examples for others to compare if aux bus is a fit for their use case. "In some subsystems, the functionality of the core device (PCI/ACPI/other) is too complex to be handled 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)." > + Splitting the > +functionality into smaller orthogonal devices would make it easier to manage > +data, power management and domain-specific interaction with the hardware. Passive voice and I don't understand what is meant by the word "orthogonal" "A split of the functionality 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" > 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. These last few sentences are confusing the justification of the existence of auxiliary bus, not the explanation of when why and how to use it. The "When Should the Auxiliary Bus Be Used" should mention where Auxiliary bus fits relative to the platform-bus and MFD, perhaps in an explicit "When not to use Auxiliary Bus" section to direct people to platform-bus and MFD when those are a better fit. > + > +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 auxiliary bus is intended to be minimal, generic and avoid > domain-specific > +assumptions. As this file will be sitting in Documentation/ it will be too late to "intend" it either does and or it doesn't. This again feels more like justification for existence that should already be in the changelog, it can go from the Documentation. > Each auxiliary_device represents a part of its parent > +functionality. The generic behavior can be extended and specialized
