On Tuesday, November 06, 2012 11:28:37 PM Rafael J. Wysocki wrote: > On Tuesday, November 06, 2012 01:35:56 PM Bjorn Helgaas wrote: > > On Tue, Nov 6, 2012 at 6:43 AM, Rafael J. Wysocki <rjw@xxxxxxx> wrote: > > > On Monday, November 05, 2012 09:54:42 AM Bjorn Helgaas wrote: > > >> On Mon, Nov 5, 2012 at 3:31 AM, Rafael J. Wysocki <rjw@xxxxxxx> wrote: > > >> > 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@xxxxxxxxxxxxxxx> 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. > > >> > > >> This is exactly what PNPACPI already does. For every Device node in > > >> the namespace, pnpacpi/rsparser.c parses _CRS and builds a list of > > >> struct resources that correspond to it. If you put this functionality > > >> in acpi/scan.c, should we get rid of PNPACPI? > > > > > > Quite likely. At least this part of it, if you want the core to parse > > > resources. > > > > > > That said, I actually tried to abstract out resource parsing in a more generic > > > fashion on the basis of our new platform device support code, but quite frankly > > > I wasn't able to. > > > > > > The problem is that struct resource is too simple to be useful for representing > > > all of the information that can be encoded in ACPI resources. As a result, some > > > information have to be stored directly in things like struct pnp_dev, struct > > > platform_device etc. and if we want to represent it generically, the only way > > > to do that seems to be using the ACPICA resource types as defined in acrestyp.h. > > > > Really? I didn't have that impression. It might be that the new GPIO > > and SERIAL_BUS stuff makes it too complicated for a struct resource, > > but prior to that, I don't think it was. It's true that there are > > many different formats (IO, FIXED_IO, MEMORY24, MEMORY32, etc.), but > > only the core needs to be aware of the encoding of all those formats. > > As far as a *driver* is concerned, there are only IRQ, DMA, IO, and > > MEM resources, and those fit easily in a struct resource. > > However, for example expanding ACPI_RESOURCE_TYPE_EXTENDED_IRQ into an > array of struct resource objects before anyone actually needs it seems a > bit wasteful to me. Let alone registering GSI for the interrupts while > we're parsing those resources. > > > I want to expand on what I said before about _CRS being the domain of > > the core, not drivers. > > Well, I see a difference between _evaluating_ _CRS and _parsing_ its > output. In particular, I don't see a reason to do those two things in > one operation. In fact, I see reasons to do otherwise. :-) > > > The core must evaluate _CRS to know where > > devices are and avoid conflicts when assigning resources. The core > > must evaluate _PRS and _SRS when making resource assignments. It > > doesn't make sense for drivers to be using _PRS and _SRS; they don't > > have a global view of resources, and any assignment they make would > > likely cause conflicts with other devices. I think it makes sense to > > consolidate all _CRS, _PRS, and _SRS management in the core rather > > than splitting it up. This is exactly analogous to PCI BAR > > management, and we don't intend drivers to read and write BARs > > directly. > > OK, but then we need to pass the information obtained from _CRS > (presumably after some adjustments through _SRS) to drivers, or rather to > things like the SPI core, I2C core etc. so that they can create device > objects for drivers to bind to and quite frankly I don't see why not to use > ACPI resources for that. Nevertheless, the routines for parsing those resources should belong to the ACPI core, mostly to avoid code duplication. Thanks, Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html