From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753865AbcL3MK2 (ORCPT ); Fri, 30 Dec 2016 07:10:28 -0500 Received: from szxga01-in.huawei.com ([58.251.152.64]:65250 "EHLO szxga01-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752411AbcL3MK0 (ORCPT ); Fri, 30 Dec 2016 07:10:26 -0500 Subject: Re: [PATCH v5 09/14] ACPI: platform: setup MSI domain for ACPI based platform device To: "Rafael J. Wysocki" References: <1482384922-21507-1-git-send-email-guohanjun@huawei.com> <1482384922-21507-10-git-send-email-guohanjun@huawei.com> <585E24FB.9050805@huawei.com> <586072E9.3060609@huawei.com> CC: Marc Zyngier , "Rafael J. Wysocki" , Lorenzo Pieralisi , "ACPI Devel Maling List" , "linux-arm-kernel@lists.infradead.org" , Linux Kernel Mailing List , Thomas Gleixner , Greg KH , Tomasz Nowicki , Ma Jun , Kefeng Wang , "Agustin Vega-Frias" , Sinan Kaya , Charles Garcia-Tobin , , , , Jon Masters , Hanjun Guo From: Hanjun Guo Message-ID: <58663C04.9000603@huawei.com> Date: Fri, 30 Dec 2016 18:50:44 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <586072E9.3060609@huawei.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.177.17.188] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A090206.586647D5.0062,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2013-06-18 04:22:30, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: f8855f9507e54b1850588ecd17119606 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >>> #include >>> #include >>> +#include >>> #include >>> >>> #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