On Thursday, November 01, 2012 09:19:59 AM Yinghai Lu wrote: > On Thu, Nov 1, 2012 at 7:35 AM, Rafael J. Wysocki <rjw@xxxxxxx> wrote: > >> > @@ -1544,8 +1553,13 @@ static acpi_status acpi_bus_check_add(ac > >> > */ > >> > device = NULL; > >> > acpi_bus_get_device(handle, &device); > >> > - if (ops->acpi_op_add && !device) > >> > + if (ops->acpi_op_add && !device) { > >> > acpi_add_single_object(&device, handle, type, sta, ops); > >> > + /* Is the device a known good platform device? */ > >> > + if (device > >> > + && !acpi_match_device_ids(device, acpi_platform_device_ids)) > >> > + acpi_create_platform_device(device); > >> > + } > >> > >> That is ugly! any reason for not using acpi_driver for them. > > > > Yes, a couple of reasons. First off, there are existing platform drivers for > > these things already and there's no reason for creating a "glue" layer between > > those drivers and struct acpi_device objects. Second, we're going to get rid > > of acpi_driver things entirely going forward (the existing ones will be replaced > > by platform drivers or included into the ACPI core). > > that should be glue between acpi_device and platform_device. > > how are you going to handle removing path ? aka when acpi_device get > trimed, how those platform get deleted? This is a valid point. Roughly, the idea is to walk the dev's list of physical nodes and call .remove() for each of them in acpi_bus_remove(). Or to do something equivalent to that. However, we're not going to add any IDs of removable devices to acpi_platform_device_ids[] for now, so we simply don't need to modify that code path just yet (we'll modify it when we're about to add such devices to that table). > >> there is not reason to treat those platform_device different from pci > >> root device and other pnp device. > > > > Well, I agree. :-) > > > > So those things you're talking about we'll be platform devices too in the > > future. > > > >> something like attached patch. .add of acpi_driver ops could call > >> acpi_create_platform_device. > > > > Been there, decided to do it differently this time. > > > > So you are going to replace acpi_device/acpi_driver with > platform_device/platform_driver ? Not exactly. Let me start from the big picture, though. :-) First off, we need to unify the handling of devices by the ACPI subsystem regardless of whether they are on a specific bus, like PCI, or they are busless "platform" devices. Currently, if a device is on a specific bus *and* there is a device node in the ACPI namespace corresponding to it, there will be two objects based on struct device for it eventually, the "physical node", like struct pci_dev, and the "ACPI node" represented by struct acpi_device. They are associated with each other through the code in drivers/acpi/glue.c. In turn, if the device is busless and not discoverable natively, we only create the "ACPI node" struct acpi_device thing for it. Those ACPI nodes are then *sometimes* bind to drivers (represented by struct acpi_driver). The fact that the busless devices are *sometimes* handled by binding drivers directly to struct acpi_device while the other devices are handled through glue.c confuses things substantially and causes problems to happen right now (for example, acpi_driver drivers sometimes attempt to bind to things that have other native drivers and should really be handled by them). Furthermore, the situation will only get worse over time if we don't do anything about that, because we're going to see more and more devices that won't be discoverable natively and will have corresponding nodes in the ACPI namespace and we're going to see more buses whose devices will have such nodes. Moreover, for many of those devices there are native drivers present in the kernel tree already, because they will be based on IP blocks used in the current hardware (for example, we may see ARM-based systems based on exactly the same hardware with ACPI BIOSes and without them). That applies to busless devices as well as to devices on specific buses. Now, the problem is how the unification is going to be done and I honestly don't think we have much *choice* here. Namely, for PCI (and other devices discoverable natively) we pretty much have to do the glue.c thing (or something equivalent), because we need to match what we've discovered natively against the information from the ACPI tables in the BIOS. This means that for busless devices we need to create "physical" nodes as well, so that all of them are handled by drivers binding to the "physical" node rather than to struct acpi_device. This also will allow us to reuse the existing drivers with minimum modifications (well, hopefully). Pretty much every ACPI developer I have discussed that with so far, including people like Matthew Garrett and Zhang Rui who have been working on ACPI for years, seems to agree that this is the way to go. Thus, in the long run, acpi_driver drivers will be replaced by something else (platform drivers seem to be a natural choice in many cases), but struct acpi_device objects won't go away, at least not entirely. My long haul plan is to drop the "dev" component of struct acpi_device and rename it to something like struct acpi_dev_node, to clarify its meaning. I'm quite confident that this is doable. The part I'm most concerned about is the interactions with user space, because we need to pay attention not to break the existing user space interfaces (that are actually used). That said, I'm not expecting this to be a one-shot transition. Rather, this is going to take time, like several kernel releases, and I'm sure there are details we need to be very careful about. That's one of the reasons why acpi_platform_device_ids[] is there: so that we can carefully carry out the unification step by step. > Did you ever try to start to the work to see if it is doable? aka do > you have draft version to do that? Unfortunately, I don't have anything I could share with you at the moment. 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