public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: guohanjun@huawei•com (Hanjun Guo)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v5 09/14] ACPI: platform: setup MSI domain for ACPI based platform device
Date: Sat, 24 Dec 2016 15:34:19 +0800	[thread overview]
Message-ID: <585E24FB.9050805@huawei.com> (raw)
In-Reply-To: <CAJZ5v0g4AoU-g4UWZFxLgQ_yfTC7KSBnZ_Ve945d+i58e=GuDQ@mail.gmail.com>

Hi Rafael,

Thank you for your comments, when I was demoing your suggestion,
I got a little bit confusions, please see my comments below.

On 2016/12/22 20:57, Rafael J. Wysocki wrote:
> On Thu, Dec 22, 2016 at 6:35 AM, Hanjun Guo <guohanjun@huawei•com> wrote:
>> From: Hanjun Guo <hanjun.guo@linaro•org>
>>
>> With the platform msi domain created, we can set up the msi domain
>> for a platform device when it's probed.
>>
>> In order to do that, we need to get the domain that the platform
>> device connecting to, so the iort_get_platform_device_domain() is
>> introduced to retrieve the domain from iort.
>>
>> After the domain is retrieved, we need a proper way to set the
>> domain to paltform device, as some platform devices such as an
>> irqchip needs the msi irqdomain to be the interrupt parent domain,
>> we need to get irqdomain before platform device is probed but after
>> the platform device is allocated, so introduce a callback (pre_add_cb)
>> in pdevinfo to prepare firmware related information which is needed
>> for device probe, then set the msi domain in that callback.
>>
>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro•org>
>> Cc: Marc Zyngier <marc.zyngier@arm•com>
>> Cc: Rafael J. Wysocki <rjw@rjwysocki•net>
>> Cc: Greg KH <gregkh@linuxfoundation•org>
>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm•com>
>> Cc: Thomas Gleixner <tglx@linutronix•de>
>> ---
>>  drivers/acpi/acpi_platform.c    | 11 +++++++++++
>>  drivers/acpi/arm64/iort.c       | 43 +++++++++++++++++++++++++++++++++++++++++
>>  drivers/base/platform.c         |  3 +++
>>  include/linux/acpi_iort.h       |  3 +++
>>  include/linux/platform_device.h |  3 +++
>>  5 files changed, 63 insertions(+)
>>
>> diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c
>> index b4c1a6a..5d8d61b4 100644
>> --- a/drivers/acpi/acpi_platform.c
>> +++ b/drivers/acpi/acpi_platform.c
>> @@ -12,6 +12,7 @@
>>   */
>>
>>  #include <linux/acpi.h>
>> +#include <linux/acpi_iort.h>
>>  #include <linux/device.h>
>>  #include <linux/err.h>
>>  #include <linux/kernel.h>
>> @@ -48,6 +49,15 @@ static void acpi_platform_fill_resource(struct acpi_device *adev,
>>  }
>>
>>  /**
>> + * acpi_platform_pre_add_cb - callback before platform device is added, to
>> + * prepare firmware related information which is needed for device probe
>> + */
>> +static void acpi_platform_pre_add_cb(struct device *dev)
>> +{
>> +       acpi_configure_pmsi_domain(dev);
>> +}
>> +
>> +/**
>>   * acpi_create_platform_device - Create platform device for ACPI device node
>>   * @adev: ACPI device node to create a platform device for.
>>   * @properties: Optional collection of build-in properties.
>> @@ -109,6 +119,7 @@ struct platform_device *acpi_create_platform_device(struct acpi_device *adev,
>>         pdevinfo.num_res = count;
>>         pdevinfo.fwnode = acpi_fwnode_handle(adev);
>>         pdevinfo.properties = properties;
>> +       pdevinfo.pre_add_cb = acpi_platform_pre_add_cb;
> Why don't you point that directly to acpi_configure_pmsi_domain()?  It
> doesn't look like the wrapper is necessary at all.

I was thinking that we can add something more in the future
if we need to extend the function of the callback, I can just
use acpi_configure_pmsi_domain() here.

>
> And I'm not sure why the new callback is necessary ->

I was demoing your suggestion but...

>
>>         if (acpi_dma_supported(adev))
>>                 pdevinfo.dma_mask = DMA_BIT_MASK(32);
>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
>> index bc68d93..6b72fcb 100644
>> --- a/drivers/acpi/arm64/iort.c
>> +++ b/drivers/acpi/arm64/iort.c
>> @@ -527,6 +527,49 @@ struct irq_domain *iort_get_device_domain(struct device *dev, u32 req_id)
>>         return irq_find_matching_fwnode(handle, DOMAIN_BUS_PCI_MSI);
>>  }
>>
>> +/**
>> + * iort_get_platform_device_domain() - Find MSI domain related to a
>> + * platform device
>> + * @dev: the dev pointer associated with the platform device
>> + *
>> + * Returns: the MSI domain for this device, NULL otherwise
>> + */
>> +static struct irq_domain *iort_get_platform_device_domain(struct device *dev)
>> +{
>> +       struct acpi_iort_node *node, *msi_parent;
>> +       struct fwnode_handle *iort_fwnode;
>> +       struct acpi_iort_its_group *its;
>> +
>> +       /* find its associated iort node */
>> +       node = iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
>> +                             iort_match_node_callback, dev);
>> +       if (!node)
>> +               return NULL;
>> +
>> +       /* then find its msi parent node */
>> +       msi_parent = iort_node_get_id(node, NULL, IORT_MSI_TYPE, 0);
>> +       if (!msi_parent)
>> +               return NULL;
>> +
>> +       /* Move to ITS specific data */
>> +       its = (struct acpi_iort_its_group *)msi_parent->node_data;
>> +
>> +       iort_fwnode = iort_find_domain_token(its->identifiers[0]);
>> +       if (!iort_fwnode)
>> +               return NULL;
>> +
>> +       return irq_find_matching_fwnode(iort_fwnode, DOMAIN_BUS_PLATFORM_MSI);
>> +}
>> +
>> +void acpi_configure_pmsi_domain(struct device *dev)
>> +{
>> +       struct irq_domain *msi_domain;
>> +
>> +       msi_domain = iort_get_platform_device_domain(dev);
>> +       if (msi_domain)
>> +               dev_set_msi_domain(dev, msi_domain);
>> +}
>> +
>>  static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
>>  {
>>         u32 *rid = data;
>> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
>> index c4af003..3e68f31 100644
>> --- a/drivers/base/platform.c
>> +++ b/drivers/base/platform.c
>> @@ -537,6 +537,9 @@ struct platform_device *platform_device_register_full(
>>                         goto err;
>>         }
>>
>> +       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);
+
        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)?

Thanks
Hanjun

  reply	other threads:[~2016-12-24  7:34 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-22  5:35 [PATCH v5 00/14] ACPI platform MSI support and its example mbigen Hanjun Guo
2016-12-22  5:35 ` [PATCH v5 01/14] ACPI: ARM64: IORT: minor cleanup for iort_match_node_callback() Hanjun Guo
2016-12-30  8:56   ` Xinwei Kong
2016-12-22  5:35 ` [PATCH v5 02/14] irqchip: gic-v3-its: keep the head file include in alphabetic order Hanjun Guo
2016-12-30  8:56   ` Xinwei Kong
2016-12-22  5:35 ` [PATCH v5 03/14] ACPI: ARM64: IORT: add missing comment for iort_dev_find_its_id() Hanjun Guo
2016-12-30  8:56   ` Xinwei Kong
2016-12-22  5:35 ` [PATCH v5 04/14] irqchip: gicv3-its: platform-msi: refactor its_pmsi_prepare() Hanjun Guo
2016-12-30  8:57   ` Xinwei Kong
2016-12-22  5:35 ` [PATCH v5 05/14] ACPI: platform-msi: retrieve dev id from IORT Hanjun Guo
2016-12-30  8:57   ` Xinwei Kong
2016-12-22  5:35 ` [PATCH v5 06/14] irqchip: gicv3-its: platform-msi: refactor its_pmsi_init() to prepare for ACPI Hanjun Guo
2016-12-30  8:57   ` Xinwei Kong
2016-12-22  5:35 ` [PATCH v5 07/14] irqchip: gicv3-its: platform-msi: scan MADT to create platform msi domain Hanjun Guo
2016-12-30  8:57   ` Xinwei Kong
2016-12-22  5:35 ` [PATCH v5 08/14] ACPI: ARM64: IORT: rework iort_node_get_id() Hanjun Guo
2016-12-30  9:00   ` Xinwei Kong
2016-12-22  5:35 ` [PATCH v5 09/14] ACPI: platform: setup MSI domain for ACPI based platform device Hanjun Guo
2016-12-22 12:57   ` Rafael J. Wysocki
2016-12-24  7:34     ` Hanjun Guo [this message]
2016-12-26  0:31       ` Rafael J. Wysocki
2016-12-26  1:31         ` Hanjun Guo
2016-12-29 14:44           ` Sinan Kaya
2016-12-30 10:40             ` Hanjun Guo
2016-12-30 10:50           ` Hanjun Guo
2016-12-31 20:45             ` Rafael J. Wysocki
2017-01-02 12:02               ` Hanjun Guo
2016-12-30  8:58   ` Xinwei Kong
2016-12-22  5:35 ` [PATCH v5 10/14] ACPI: ARM64: IORT: rework iort_node_get_id() for NC->SMMU->ITS case Hanjun Guo
2016-12-30  8:58   ` Xinwei Kong
2016-12-22  5:35 ` [PATCH v5 11/14] msi: platform: make platform_msi_create_device_domain() ACPI aware Hanjun Guo
2016-12-30  8:58   ` Xinwei Kong
2016-12-22  5:35 ` [PATCH v5 12/14] irqchip: mbigen: drop module owner Hanjun Guo
2016-12-26  8:59   ` majun (Euler7)
2016-12-30  8:58   ` Xinwei Kong
2016-12-22  5:35 ` [PATCH v5 13/14] irqchip: mbigen: introduce mbigen_of_create_domain() Hanjun Guo
2016-12-26  8:59   ` majun (Euler7)
2016-12-30  8:59   ` Xinwei Kong
2016-12-22  5:35 ` [PATCH v5 14/14] irqchip: mbigen: Add ACPI support Hanjun Guo
2016-12-26  9:00   ` majun (Euler7)
2016-12-30  8:59   ` Xinwei Kong
2016-12-26  8:57 ` [PATCH v5 00/14] ACPI platform MSI support and its example mbigen majun (Euler7)
2016-12-26  9:26   ` majun (Euler7)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=585E24FB.9050805@huawei.com \
    --to=guohanjun@huawei$(echo .)com \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox