Re: [PATCH 2/3] spi / ACPI: add ACPI enumeration support

["Rafael J. Wysocki" <rjw@sisk.pl>]

On Saturday, November 03, 2012 09:59:28 PM Rafael J. Wysocki wrote:
> On Saturday, November 03, 2012 10:13:10 PM Mika Westerberg wrote:
> > On Sat, Nov 03, 2012 at 01:42:02PM -0600, Bjorn Helgaas wrote:
> > > On Sat, Nov 3, 2012 at 1:46 AM, Mika Westerberg
> > > <mika.westerberg@linux.intel.com> wrote:
> > > > ACPI 5 introduced SPISerialBus resource that allows us to enumerate and
> > > > configure the SPI slave devices behind the SPI controller. This patch adds
> > > > support for this to the SPI core.
[...]
> > And if the ACPI core parses the _CRS, how does it pass all the resources to
> > the drivers?
> 
> Pretty much the same way the $subject patch does.
> 
> Instead of parsing the entire subtree below an SPI controller and trying
> acpi_spi_add_device() for each device node in there, it could call
> acpi_spi_add_device() whenever it finds a device of type
> ACPI_RESOURCE_TYPE_SERIAL_BUS/ACPI_RESOURCE_SERIAL_TYPE_SPI.
> The only problem is how to pass "master" to it.
> 
> So Bjorn, do you have any idea how we could pass the "master" pointer from the
> ACPI core to acpi_spi_add_device() in a sensible way?
> 
> An alternative might be to store the information obtained from _CRS in
> struct acpi_device objects created by the ACPI core while parsing the
> namespace.  We do that already for things like _PRW, so we might as well do it
> for _CRS.  Then, the SPI core could just walk the subtree of the device hierarchy
> below the SPI controller's acpi_device to extract that information.
> Maybe that's the way to go?

The general idea is to move the _CRS parsing routine from acpi_platform.c
to scan.c and make it attach resource objects to struct acpi_device.

I'm thinking about adding a list head to struct acpi_device pointing to a
list of entries like:

struct resource_list_entry {
	struct list_head node;
	struct resource *resources;
	unsigned int count;
};

where "resources" is an array of resources (e.g. interrupts) in the given
entry and count is the size of that array.

That list would contain common resources like ACPI_RESOURCE_TYPE_FIXED_MEMORY32,
ACPI_RESOURCE_TYPE_IRQ, ACPI_RESOURCE_TYPE_ADDRESS32, ACPI_RESOURCE_TYPE_EXTENDED_IRQ.
I think adding it would allow us to make acpi_create_platform_device(),
acpi_spi_add_resources() and acpi_i2c_add_resources() more straightforward (and
remove some code duplication between the last two routines).

In addition to that, I'd add an entry containing serial bus information, if
applicable, for the given struct acpi_device, something like:

union acpi_resource_serial_bus {
	struct acpi_resource_common_serialbus;
	struct acpi_resource_spi_serialbus;
	struct acpi_resource_i2c_serialbus;
	struct acpi_resource_uart_serialbus;
};

struct acpi_device {
	...
	union acpi_resource_serial_bus *serial;
	...
};

Then, things like acpi_spi_add_resources() and acpi_i2c_add_resources() would
be able to use struct acpi_device objects directly without running any AML.

That could be done on top of the current series and I'm going to prepare a patch
for that in the next few days if I find some time between the LCE sessions.

Thanks,
Rafael


--- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.