Hi Lorenzo
Many thanks for your review.
在 2016/10/22 0:08, Lorenzo Pieralisi 写道:
On Fri, Oct 21, 2016 at 02:12:44PM +0800, Dongdong Liu wrote:
[...]
+static int hisi_pcie_init(struct pci_config_window *cfg)
+{
+ int ret;
+ struct acpi_device *adev = to_acpi_device(cfg->parent);
Why is this expected to be struct acpi_device?
I use this acpi device(Device (PCI2)) to get it's child acpi device(Device (RES2)), then
obtain rc base addresses from PNP0C02 as subdevice of PNP0A03.
The procedure to get the value of cfg->parent is as the below.
arch/arm64/kernel/pci.c
pci_acpi_scan_root(struct acpi_pci_root *root)
--->pci_acpi_setup_ecam_mapping(root, ri);
--->pci_ecam_create(&root->device->dev, &cfgres, bus_res, ecam_ops);
--->cfg->parent = dev
;
PCIe DSDT table defines as below.
Device (PCI2)
{
Name (_HID, "PNP0A08") // PCI Express Root Bridge
Name (_CID, "PNP0A03") // Compatible PCI Root Bridge
......
Device (RES2)
{
Name (_HID, "HISI0081") // HiSi PCIe RC config base address
Name (_CID, "PNP0C02") // Motherboard reserved resource
Name (_CRS, ResourceTemplate (){
Memory32Fixed (ReadWrite, 0xa00a0000 , 0x10000)
})
......
}
Shouldn't it be a "physical" device whose companion is an ACPI device object?
Sorry, I don't understand. What do you mean ?
I think Rafael meant the physical node (that in this case is the pci
bridge device) that is associated with the acpi device (the struct
pci_config_window.parent pointer points at the PNP0A03/PNP0A08 acpi
device object though, not the RES2 acpi device that's what you need).
Have a look at:
Documentation/acpi/enumeration.txt
Documentation/acpi/namespace.txt
Thanks for explaining this.
It should be a "physical" device. but I have not saw about
ACPI_COMPANION_SET() of this device.
The PNP0A08 acpi_device (that is what is pointed at by
struct acpi_device = to_acpi_device(pci_config_window.parent)
is the respective pci bridge device companion.
arch/arm64/kernel/pci.c (pcibios_root_bridge_prepare()).
Now for something completely different :), your RES2 pseudo-device.
IIUC platform device (physical node) is created by the ACPI enumeration
code for your RES2 pseudo-device and its resources are already
initialized (by parsing its _CRS) in acpi_create_platform_device().
This is not right. About RES2 pseudo-device,it's _CID is PNP0C02,
this will attach acpi_pnp_attach() (drivers/acpi/acpi_pnp.c) .
acpi_pnp_attach() return 1;ret = acpi_scan_attach_handler() will return 1.
So this will not call acpi_default_enumeration(),
this also will not call acpi_create_platform_device().
drivers/acpi/scan.c
static void acpi_bus_attach(struct acpi_device *device)
{
......
ret = acpi_scan_attach_handler(device);
if (ret < 0)
return;
device->flags.match_driver = true;
if (!ret) {
ret = device_attach(&device->dev);
if (ret < 0)
return;
if (!ret && device->pnp.type.platform_id)
acpi_default_enumeration(device);
}
......
}
Dongdong
Thanks
This means that by the time you match the PNP0A03/PNP0A08 child with an
acpi_device with _HID == "HISI0081", you can get its associated
physical node and get its resources IOMEM through the physical
node (ie platform device and related resources) instead of re-parsing
the _CRS if I am not mistaken (and that's an IF because I did not
test it).
As above said this does not re-parse the _CRS for RES2 pseudo-device.
Dongdong
Thanks
Regardless, I am not entirely sure there are kernel drivers/control
paths already using this mechanism to handle child devices (keeping in
mind that RES2 is not a real device at all, it is there to represent
PNP0A03 sub-address space for PCI quirks), so it would be good to get
Rafael's agreement on this approach to prevent abusing the current ACPI
platform devices glue code assumptions.
It would also be good to agree on the whole approach soundness.
Thanks,
Lorenzo
.
--
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
