All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Williams <dan.j.williams@intel.com>
To: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Greg KH <gregkh@linuxfoundation.org>,
	Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
	alsa-devel@alsa-project.org, Kiran Patil <kiran.patil@intel.com>,
	linux-rdma <linux-rdma@vger.kernel.org>,
	Shiraz Saleem <shiraz.saleem@intel.com>,
	Martin Habets <mhabets@solarflare.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Ranjani Sridharan <ranjani.sridharan@linux.intel.com>,
	Fred Oh <fred.oh@linux.intel.com>,
	Mark Brown <broonie@kernel.org>, Jason Gunthorpe <jgg@nvidia.com>,
	Dave Ertman <david.m.ertman@intel.com>,
	Jakub Kicinski <kuba@kernel.org>, Netdev <netdev@vger.kernel.org>,
	Leon Romanovsky <leonro@nvidia.com>,
	David Miller <davem@davemloft.net>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Parav Pandit <parav@mellanox.com>
Subject: Re: [resend/standalone PATCH v4] Add auxiliary bus support
Date: Thu, 17 Dec 2020 18:39:55 -0800	[thread overview]
Message-ID: <CAPcyv4h-jg0dxKZ89yYnHsTEDj7jLWDBhBVTgEC77tLLsz92pw@mail.gmail.com> (raw)
In-Reply-To: <20201217211937.GA3177478@piout.net>

On Thu, Dec 17, 2020 at 1:20 PM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
>
> Hello,
>
> On 05/12/2020 16:51:36+0100, Greg KH wrote:
> > > To me, the documentation was written, and reviewed, more from the
> > > perspective of "why not open code a custom bus instead". So I can see
> > > after the fact how that is a bit too much theory and justification and
> > > not enough practical application. Before the fact though this was a
> > > bold mechanism to propose and it was not clear that everyone was
> > > grokking the "why" and the tradeoffs.
> >
> > Understood, I guess I read this from the "of course you should do this,
> > now how do I use it?" point of view.  Which still needs to be addressed
> > I feel.
> >
> > > I also think it was a bit early to identify consistent design patterns
> > > across the implementations and codify those. I expect this to evolve
> > > convenience macros just like other parts of the driver-core gained
> > > over time. Now that it is in though, another pass through the
> > > documentation to pull in more examples seems warranted.
> >
> > A real, working, example would be great to have, so that people can know
> > how to use this.  Trying to dig through the sound or IB patches to view
> > how it is being used is not a trivial thing to do, which is why
> > reviewing this took so much work.  Having a simple example test module,
> > that creates a number of devices on a bus, ideally tied into the ktest
> > framework, would be great.  I'll attach below a .c file that I used for
> > some basic local testing to verify some of this working, but it does not
> > implement a aux bus driver, which needs to be also tested.
> >
>
> There is something I don't get from the documentation and it is what is
> this introducing that couldn't already be done using platform drivers
> and platform devices?

There is room for documentation improvement here. I realize reading it
back now that much of the justification for "why not platform bus?"
happened on the list, but only a small mention made it into the
document. It turns out that platform-bus has some special integrations
and hacks with platform-firmware implementations. For example, the
ACPI companion magic and specific platform firmware integrations in
platform_match(). It's also an awkward bus name to use because these
devices do not belong to the platform. The platform bus is for devices
that do not have an enumeration mechanism besides board files or
firmware descriptions.

So while many of the auxiliary device use cases might be able to be
squeezed into a platform-bus scheme it further overloads what is
already a wide responsibility.

In comparison, the auxiliary-bus is tailored to the "sub-function of a
parent device/driver" use case. It lets the host driver be the root of
a namespace of sub-functionality in a standard template way.

> We already have a bunch of drivers in tree that have to share a state
> and register other drivers from other subsystems for the same device.
> How is the auxiliary bus different?

There's also custom subsystem buses that do this. Why not other
alternatives? They didn't capture the simultaneous mindshare of RDMA,
SOF, and NETDEV developers. Personally my plans for using
auxiliary-bus do not map cleanly to anything else in the tree. I want
to use it for attaching an NPEM driver (Native PCIE Enclosure
Management) to any PCI device driver that opts-in, but it would be
overkill to go create an "npem" bus for this.

WARNING: multiple messages have this Message-ID (diff)
From: Dan Williams <dan.j.williams@intel.com>
To: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: alsa-devel@alsa-project.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Kiran Patil <kiran.patil@intel.com>,
	David Miller <davem@davemloft.net>,
	linux-rdma <linux-rdma@vger.kernel.org>,
	Greg KH <gregkh@linuxfoundation.org>,
	Martin Habets <mhabets@solarflare.com>,
	Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Fred Oh <fred.oh@linux.intel.com>,
	Mark Brown <broonie@kernel.org>,
	Ranjani Sridharan <ranjani.sridharan@linux.intel.com>,
	Jason Gunthorpe <jgg@nvidia.com>,
	Jakub Kicinski <kuba@kernel.org>,
	Dave Ertman <david.m.ertman@intel.com>,
	Shiraz Saleem <shiraz.saleem@intel.com>,
	Netdev <netdev@vger.kernel.org>,
	Leon Romanovsky <leonro@nvidia.com>,
	Parav Pandit <parav@mellanox.com>
Subject: Re: [resend/standalone PATCH v4] Add auxiliary bus support
Date: Thu, 17 Dec 2020 18:39:55 -0800	[thread overview]
Message-ID: <CAPcyv4h-jg0dxKZ89yYnHsTEDj7jLWDBhBVTgEC77tLLsz92pw@mail.gmail.com> (raw)
In-Reply-To: <20201217211937.GA3177478@piout.net>

On Thu, Dec 17, 2020 at 1:20 PM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
>
> Hello,
>
> On 05/12/2020 16:51:36+0100, Greg KH wrote:
> > > To me, the documentation was written, and reviewed, more from the
> > > perspective of "why not open code a custom bus instead". So I can see
> > > after the fact how that is a bit too much theory and justification and
> > > not enough practical application. Before the fact though this was a
> > > bold mechanism to propose and it was not clear that everyone was
> > > grokking the "why" and the tradeoffs.
> >
> > Understood, I guess I read this from the "of course you should do this,
> > now how do I use it?" point of view.  Which still needs to be addressed
> > I feel.
> >
> > > I also think it was a bit early to identify consistent design patterns
> > > across the implementations and codify those. I expect this to evolve
> > > convenience macros just like other parts of the driver-core gained
> > > over time. Now that it is in though, another pass through the
> > > documentation to pull in more examples seems warranted.
> >
> > A real, working, example would be great to have, so that people can know
> > how to use this.  Trying to dig through the sound or IB patches to view
> > how it is being used is not a trivial thing to do, which is why
> > reviewing this took so much work.  Having a simple example test module,
> > that creates a number of devices on a bus, ideally tied into the ktest
> > framework, would be great.  I'll attach below a .c file that I used for
> > some basic local testing to verify some of this working, but it does not
> > implement a aux bus driver, which needs to be also tested.
> >
>
> There is something I don't get from the documentation and it is what is
> this introducing that couldn't already be done using platform drivers
> and platform devices?

There is room for documentation improvement here. I realize reading it
back now that much of the justification for "why not platform bus?"
happened on the list, but only a small mention made it into the
document. It turns out that platform-bus has some special integrations
and hacks with platform-firmware implementations. For example, the
ACPI companion magic and specific platform firmware integrations in
platform_match(). It's also an awkward bus name to use because these
devices do not belong to the platform. The platform bus is for devices
that do not have an enumeration mechanism besides board files or
firmware descriptions.

So while many of the auxiliary device use cases might be able to be
squeezed into a platform-bus scheme it further overloads what is
already a wide responsibility.

In comparison, the auxiliary-bus is tailored to the "sub-function of a
parent device/driver" use case. It lets the host driver be the root of
a namespace of sub-functionality in a standard template way.

> We already have a bunch of drivers in tree that have to share a state
> and register other drivers from other subsystems for the same device.
> How is the auxiliary bus different?

There's also custom subsystem buses that do this. Why not other
alternatives? They didn't capture the simultaneous mindshare of RDMA,
SOF, and NETDEV developers. Personally my plans for using
auxiliary-bus do not map cleanly to anything else in the tree. I want
to use it for attaching an NPEM driver (Native PCIE Enclosure
Management) to any PCI device driver that opts-in, but it would be
overkill to go create an "npem" bus for this.

  reply	other threads:[~2020-12-18  2:40 UTC|newest]

Thread overview: 114+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-03  0:54 [resend/standalone PATCH v4] Add auxiliary bus support Dan Williams
2020-12-03  0:54 ` Dan Williams
2020-12-03 15:06 ` Greg KH
2020-12-03 15:06   ` Greg KH
2020-12-04  2:33   ` Jason Gunthorpe
2020-12-04  2:33     ` Jason Gunthorpe
2020-12-04  3:37     ` Dan Williams
2020-12-04  3:37       ` Dan Williams
2020-12-03 15:07 ` Greg KH
2020-12-03 15:07   ` Greg KH
2020-12-03 15:55   ` Leon Romanovsky
2020-12-03 15:55     ` Leon Romanovsky
2020-12-04 11:42 ` Greg KH
2020-12-04 11:42   ` Greg KH
2020-12-04 11:43   ` [PATCH 1/3] driver core: auxiliary bus: move slab.h from include file Greg KH
2020-12-04 11:43     ` Greg KH
2020-12-04 11:44     ` [PATCH 2/3] driver core: auxiliary bus: make remove function return void Greg KH
2020-12-04 11:44       ` Greg KH
2020-12-04 11:44       ` [PATCH 3/3] driver core: auxiliary bus: minor coding style tweaks Greg KH
2020-12-04 11:44         ` Greg KH
2020-12-04 11:48         ` Greg KH
2020-12-04 11:48           ` Greg KH
2020-12-04 11:49       ` [PATCH v2 " Greg KH
2020-12-04 11:49         ` Greg KH
2020-12-04 12:32   ` [resend/standalone PATCH v4] Add auxiliary bus support Leon Romanovsky
2020-12-04 12:32     ` Leon Romanovsky
2020-12-04 12:43     ` Parav Pandit
2020-12-04 12:43       ` Parav Pandit
2020-12-04 12:59     ` Greg KH
2020-12-04 12:59       ` Greg KH
2020-12-04 17:10       ` Ranjani Sridharan
2020-12-04 17:10         ` Ranjani Sridharan
2020-12-05  9:02         ` Greg KH
2020-12-05  9:02           ` Greg KH
2020-12-04 16:41   ` Dan Williams
2020-12-04 16:41     ` Dan Williams
2020-12-05 15:51     ` Greg KH
2020-12-05 15:51       ` Greg KH
2020-12-17 21:19       ` Alexandre Belloni
2020-12-17 21:19         ` Alexandre Belloni
2020-12-18  2:39         ` Dan Williams [this message]
2020-12-18  2:39           ` Dan Williams
2020-12-18 14:20           ` Mark Brown
2020-12-18 14:20             ` Mark Brown
2020-12-18  7:10         ` Greg KH
2020-12-18  7:10           ` Greg KH
2020-12-18 13:17           ` Mark Brown
2020-12-18 13:17             ` Mark Brown
2020-12-18 13:46             ` Lee Jones
2020-12-18 13:46               ` Lee Jones
2020-12-18 14:08             ` Jason Gunthorpe
2020-12-18 14:08               ` Jason Gunthorpe
2020-12-18 15:52               ` Mark Brown
2020-12-18 15:52                 ` Mark Brown
2020-12-18 16:28                 ` Jason Gunthorpe
2020-12-18 16:28                   ` Jason Gunthorpe
2020-12-18 17:15                   ` Alexandre Belloni
2020-12-18 17:15                     ` Alexandre Belloni
2020-12-18 18:03                   ` Mark Brown
2020-12-18 18:03                     ` Mark Brown
2020-12-18 18:41                     ` Jason Gunthorpe
2020-12-18 18:41                       ` Jason Gunthorpe
2020-12-18 19:09                       ` Lee Jones
2020-12-18 19:09                         ` Lee Jones
2020-12-18 20:14                         ` Jason Gunthorpe
2020-12-18 20:14                           ` Jason Gunthorpe
2020-12-18 20:32                       ` Mark Brown
2020-12-18 20:32                         ` Mark Brown
2020-12-18 20:58                         ` Jason Gunthorpe
2020-12-18 20:58                           ` Jason Gunthorpe
2020-12-18 21:16                           ` Alexandre Belloni
2020-12-18 21:16                             ` Alexandre Belloni
2020-12-18 22:36                             ` Dan Williams
2020-12-18 22:36                               ` Dan Williams
2020-12-18 23:36                             ` Jason Gunthorpe
2020-12-18 23:36                               ` Jason Gunthorpe
2020-12-19  0:22                               ` Alexandre Belloni
2020-12-19  0:22                                 ` Alexandre Belloni
2020-12-21 18:51                           ` Mark Brown
2020-12-21 18:51                             ` Mark Brown
2021-01-04 18:08                             ` Jason Gunthorpe
2021-01-04 18:08                               ` Jason Gunthorpe
2021-01-04 21:19                               ` Mark Brown
2021-01-04 21:19                                 ` Mark Brown
2021-01-05  0:13                                 ` Jason Gunthorpe
2021-01-05  0:13                                   ` Jason Gunthorpe
2021-01-05  0:51                                   ` Dan Williams
2021-01-05  0:51                                     ` Dan Williams
2021-01-05  1:53                                     ` Jason Gunthorpe
2021-01-05  1:53                                       ` Jason Gunthorpe
2021-01-05  3:12                                       ` Dan Williams
2021-01-05  3:12                                         ` Dan Williams
2021-01-05 12:49                                         ` Jason Gunthorpe
2021-01-05 12:49                                           ` Jason Gunthorpe
2021-01-05 13:42                                   ` Mark Brown
2021-01-05 13:42                                     ` Mark Brown
2021-01-05 14:36                                     ` Jason Gunthorpe
2021-01-05 14:36                                       ` Jason Gunthorpe
2021-01-05 15:47                                       ` Mark Brown
2021-01-05 15:47                                         ` Mark Brown
2020-12-04 12:35 ` Greg KH
2020-12-04 12:35   ` Greg KH
2020-12-04 12:54   ` Leon Romanovsky
2020-12-04 12:54     ` Leon Romanovsky
2020-12-04 16:25     ` Jakub Kicinski
2020-12-04 16:25       ` Jakub Kicinski
2020-12-04 17:57       ` Saeed Mahameed
2020-12-04 17:57         ` Saeed Mahameed
2020-12-04 18:05   ` Ranjani Sridharan
2020-12-04 18:05     ` Ranjani Sridharan
2020-12-06  0:24 ` David Ahern
2020-12-06  0:24   ` David Ahern
2020-12-06  0:32   ` Dan Williams
2020-12-06  0:32     ` Dan Williams

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=CAPcyv4h-jg0dxKZ89yYnHsTEDj7jLWDBhBVTgEC77tLLsz92pw@mail.gmail.com \
    --to=dan.j.williams@intel.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=davem@davemloft.net \
    --cc=david.m.ertman@intel.com \
    --cc=fred.oh@linux.intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jgg@nvidia.com \
    --cc=kiran.patil@intel.com \
    --cc=kuba@kernel.org \
    --cc=leonro@nvidia.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=mhabets@solarflare.com \
    --cc=netdev@vger.kernel.org \
    --cc=parav@mellanox.com \
    --cc=pierre-louis.bossart@linux.intel.com \
    --cc=ranjani.sridharan@linux.intel.com \
    --cc=shiraz.saleem@intel.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.