Re: [PATCH v5 09/14] ACPI: platform: setup MSI domain for ACPI based platform device

From: Hanjun Guo
Date: Fri Dec 30 2016 - 07:10:31 EST


Hi Rafael,

On 2016/12/26 9:31, Hanjun Guo wrote:
[cut]
>
> + if (pdevinfo->pre_add_cb)
> + pdevinfo->pre_add_cb(&pdev->dev);
> +
>>>> -> because it looks like this might be done in acpi_platform_notify()
>>>> for platform devices.
>>> It works and I just simply add the code below:
>>>
>>> diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
>>> index f8d6564..e0cd649 100644
>>> --- a/drivers/acpi/glue.c
>>> +++ b/drivers/acpi/glue.c
>>> @@ -13,6 +13,7 @@
>>> #include <linux/slab.h>
>>> #include <linux/rwsem.h>
>>> #include <linux/acpi.h>
>>> +#include <linux/acpi_iort.h>
>>> #include <linux/dma-mapping.h>
>>>
>>> #include "internal.h"
>>> @@ -315,6 +316,8 @@ static int acpi_platform_notify(struct device *dev)
>>> if (!adev)
>>> goto out;
>>>
>>> + acpi_configure_pmsi_domain(dev);
>>> +
>> But that should apply to platform devices only I suppose?
> Yes, it's only for the platform device.
>
>>> if (type && type->setup)
>>> type->setup(dev);
>>> else if (adev->handler && adev->handler->bind)
>>>
>>> Do you suggesting to configure the msi domain in this way?
>>> or add the function in the type->setup() callback (which needs
>>> to introduce a new acpi bus type)?
>> A type->setup() would be somewhat cleaner I think, but then it's more
>> code. Whichever works better I guess. :-)
> Agree, I will demo the type->setup() way and send out the patch for review,
> also I find one minor issue for the IORT code, will update that also for next
> version.

Just demo the code and find out it's seems to cut the feet to the type->setup() code,
because we need a match function (it's ok) and a find_companion() (we don't need that
and make the code worse because we will call the find_companion callback which it not needed
for platform devices:

diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c
index 96983c9..654021d9b 100644
--- a/drivers/acpi/acpi_platform.c
+++ b/drivers/acpi/acpi_platform.c
@@ -138,3 +138,31 @@ struct platform_device *acpi_create_platform_device(struct acpi_device *adev,
return pdev;
}
EXPORT_SYMBOL_GPL(acpi_create_platform_device);
+
+static bool platform_acpi_bus_match(struct device *dev)
+{
+ return dev->bus == &platform_bus_type;
+}
+
+static struct acpi_device *platform_acpi_bus_find_companion(struct device *dev)
+{
+ /* demo code, do nothing here */
+ return NULL;
+}
+
+static void platform_acpi_setup(struct device *dev)
+{
+ acpi_configure_pmsi_domain(dev);
+}
+
+static struct acpi_bus_type acpi_platform_bus = {
+ .name = "Platform",
+ .match = platform_acpi_bus_match,
+ .find_companion = platform_acpi_bus_find_companion,
+ .setup = platform_acpi_setup,
+};
+
+int acpi_platform_bus_register(void)
+{
+ return register_acpi_bus_type(&acpi_platform_bus);
+}
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 95855cb..0a0a639 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -1199,6 +1199,7 @@ static int __init acpi_init(void)
}

pci_mmcfg_late_init();
+ acpi_platform_bus_register();
acpi_iort_init();
acpi_scan_init();
acpi_ec_init();
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 809b536..1d05f92 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -597,6 +597,8 @@ extern bool acpi_driver_match_device(struct device *dev,

struct platform_device *acpi_create_platform_device(struct acpi_device *,
struct property_entry *);
+int acpi_platform_bus_register(void);
+
#define ACPI_PTR(_ptr) (_ptr)

static inline void acpi_device_set_enumerated(struct acpi_device *adev)


So how about just add the code as below?

diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
index 11e63dd..37a8dfe 100644
--- a/drivers/acpi/glue.c
+++ b/drivers/acpi/glue.c
@@ -316,7 +316,8 @@ static int acpi_platform_notify(struct device *dev)
if (!adev)
goto out;

+ if (dev->bus == &platform_bus_type)
+ acpi_configure_pmsi_domain(dev);

if (type && type->setup)
type->setup(dev);

Thanks
Hanjun