Re: [resend/standalone PATCH v4] Add auxiliary bus support

From: Mark Brown <broonie@kernel.org>
To: Jason Gunthorpe <jgg@nvidia.com>
Cc: Greg KH <gregkh@linuxfoundation.org>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	Dan Williams <dan.j.williams@intel.com>,
	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>,
	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>,
	lee.jones@linaro.org
Subject: Re: [resend/standalone PATCH v4] Add auxiliary bus support
Date: Tue, 5 Jan 2021 13:42:56 +0000	[thread overview]
Message-ID: <20210105134256.GA4487@sirena.org.uk> (raw)
In-Reply-To: <20210105001341.GL552508@nvidia.com>

[-- Attachment #1: Type: text/plain, Size: 2634 bytes --]

On Mon, Jan 04, 2021 at 08:13:41PM -0400, Jason Gunthorpe wrote:
> On Mon, Jan 04, 2021 at 09:19:30PM +0000, Mark Brown wrote:

> > Like I keep saying the same thing applies to all non-enumerable buses -
> > exactly the same considerations exist for all the other buses like I2C
> > (including the ACPI naming issue you mention below), and for that matter
> > with enumerable buses which can have firmware info.

> And most busses do already have their own bus type. ACPI, I2C, PCI,
> etc. It is just a few that have been squished into platform, notably
> OF.

You're missing the point there.  I2C is enumerated by firmware in
exactly the same way as the platform bus is, it's not discoverable from
the hardware (and similarly for a bunch of other buses).  If we were to
say that we need separate device types for platform devices enumerated
using firmware then by analogy we should do the same for devices on
these other buses that happen to be enumerated by firmware.

I'm not convinced this is actually the design that's being pushed by
Greg here, to the extent anything is being actively pushed.

> But now it begs the question, why not push harder to make 'struct
> device' the generic universal access point and add some resource_get()
> API along these lines so even a platform_device * isn't needed?

We still need a struct device of some kind so the discussions about
exactly which bus type one is supposed to use in which circumstances
remain given that you're not supposed to have raw struct devices.

> Then the path seems much clearer, add a multi-bus-type device_driver
> that has a probe(struct device *) and uses the 'universal api_get()'
> style interface to find the generic 'resources'.

That's exactly how things like mixed I2C/SPI devices work at present,
given that they can usually use regmap to hide register I/O.

> int gpiod_count(struct device *dev, const char *con_id)
> {
> 	int count = -ENOENT;

> 	if (IS_ENABLED(CONFIG_OF) && dev && dev->of_node)
> 		count = of_gpio_get_count(dev, con_id);
> 	else if (IS_ENABLED(CONFIG_ACPI) && dev && ACPI_HANDLE(dev))
> 		count = acpi_gpio_count(dev, con_id);
> 
> 	if (count < 0)
> 		count = platform_gpio_count(dev, con_id);

> With an actual bus specific virtual function:

>     return dev->bus->gpio_count(dev);

That won't work, you might have a mix of enumeration types for a given
bus type in a single system so you'd need to do this per device.  It's
also not clear to me that it is useful to spread things out like this,
it makes it more hassle to add new APIs since you have to change core
code.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]