|
|
|
|
This patch makes pci-xgene-msi driver ACPI-aware and provides
MSI capability for X-Gene v1 PCIe controllers in ACPI boot mode.
Signed-off-by: Duc Dang < [hidden email]>
---
drivers/pci/host/pci-xgene-msi.c | 35 ++++++++++++++++++++++++++++++++---
1 file changed, 32 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/host/pci-xgene-msi.c b/drivers/pci/host/pci-xgene-msi.c
index a6456b5..466aa93 100644
--- a/drivers/pci/host/pci-xgene-msi.c
+++ b/drivers/pci/host/pci-xgene-msi.c
@@ -24,6 +24,7 @@
#include <linux/pci.h>
#include <linux/platform_device.h>
#include <linux/of_pci.h>
+#include <linux/acpi.h>
#define MSI_IR0 0x000000
#define MSI_INT0 0x800000
@@ -39,7 +40,7 @@ struct xgene_msi_group {
};
struct xgene_msi {
- struct device_node *node;
+ struct fwnode_handle *fwnode;
struct irq_domain *inner_domain;
struct irq_domain *msi_domain;
u64 msi_addr;
@@ -249,6 +250,13 @@ static const struct irq_domain_ops msi_domain_ops = {
.free = xgene_irq_domain_free,
};
+#ifdef CONFIG_ACPI
+static struct fwnode_handle *xgene_msi_get_fwnode(struct device *dev)
+{
+ return xgene_msi_ctrl.fwnode;
+}
+#endif
+
static int xgene_allocate_domains(struct xgene_msi *msi)
{
msi->inner_domain = irq_domain_add_linear(NULL, NR_MSI_VEC,
@@ -256,7 +264,7 @@ static int xgene_allocate_domains(struct xgene_msi *msi)
if (!msi->inner_domain)
return -ENOMEM;
- msi->msi_domain = pci_msi_create_irq_domain(of_node_to_fwnode(msi->node),
+ msi->msi_domain = pci_msi_create_irq_domain(msi->fwnode,
&xgene_msi_domain_info,
msi->inner_domain);
@@ -265,6 +273,9 @@ static int xgene_allocate_domains(struct xgene_msi *msi)
return -ENOMEM;
}
+#ifdef CONFIG_ACPI
+ pci_msi_register_fwnode_provider(&xgene_msi_get_fwnode);
+#endif
return 0;
}
@@ -473,6 +484,13 @@ static const struct of_device_id xgene_msi_match_table[] = {
{},
};
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id xgene_msi_acpi_ids[] = {
+ {"APMC0D0E", 0},
+ { },
+};
+#endif
+
static int xgene_msi_probe(struct platform_device *pdev)
{
struct resource *res;
@@ -494,7 +512,17 @@ static int xgene_msi_probe(struct platform_device *pdev)
goto error;
}
xgene_msi->msi_addr = res->start;
- xgene_msi->node = pdev->dev.of_node;
+
+ xgene_msi->fwnode = of_node_to_fwnode(pdev->dev.of_node);
+ if (!xgene_msi->fwnode) {
+ xgene_msi->fwnode = irq_domain_alloc_fwnode(NULL);
+ if (!xgene_msi->fwnode) {
+ dev_err(&pdev->dev, "Failed to create fwnode\n");
+ rc = ENOMEM;
+ goto error;
+ }
+ }
+
xgene_msi->num_cpus = num_possible_cpus();
rc = xgene_msi_init_allocator(xgene_msi);
@@ -571,6 +599,7 @@ static struct platform_driver xgene_msi_driver = {
.driver = {
.name = "xgene-msi",
.of_match_table = xgene_msi_match_table,
+ .acpi_match_table = ACPI_PTR(xgene_msi_acpi_ids),
},
.probe = xgene_msi_probe,
.remove = xgene_msi_remove,
--
1.9.1
|
|
|
On 10/02/16 01:56, Duc Dang wrote:
> This patch makes pci-xgene-msi driver ACPI-aware and provides
> MSI capability for X-Gene v1 PCIe controllers in ACPI boot mode.
>
> Signed-off-by: Duc Dang < [hidden email]>
Another nail on the ACPI "standardization" coffin, I suppose... Oh well.
Acked-by: Marc Zyngier < [hidden email]>
M.
--
Jazz is not dead. It just smells funny...
|
|
|
On Wed, Feb 10, 2016 at 9:08 AM, Marc Zyngier < [hidden email]> wrote:
> On 10/02/16 01:56, Duc Dang wrote:
>> This patch makes pci-xgene-msi driver ACPI-aware and provides
>> MSI capability for X-Gene v1 PCIe controllers in ACPI boot mode.
>>
>> Signed-off-by: Duc Dang < [hidden email]>
>
> Another nail on the ACPI "standardization" coffin, I suppose... Oh well.
>
> Acked-by: Marc Zyngier < [hidden email]>
Thanks, Marc.
Regards,
Duc Dang.
>
> M.
> --
> Jazz is not dead. It just smells funny...
|
|
|
On Tue, Feb 9, 2016 at 5:56 PM, Duc Dang < [hidden email]> wrote:
> This patch makes pci-xgene-msi driver ACPI-aware and provides
> MSI capability for X-Gene v1 PCIe controllers in ACPI boot mode.
Hi Bjorn,
Are you planning to take this patch into your tree?
Regards,
Duc Dang.
>
> Signed-off-by: Duc Dang < [hidden email]>
> ---
> drivers/pci/host/pci-xgene-msi.c | 35 ++++++++++++++++++++++++++++++++---
> 1 file changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/host/pci-xgene-msi.c b/drivers/pci/host/pci-xgene-msi.c
> index a6456b5..466aa93 100644
> --- a/drivers/pci/host/pci-xgene-msi.c
> +++ b/drivers/pci/host/pci-xgene-msi.c
> @@ -24,6 +24,7 @@
> #include <linux/pci.h>
> #include <linux/platform_device.h>
> #include <linux/of_pci.h>
> +#include <linux/acpi.h>
>
> #define MSI_IR0 0x000000
> #define MSI_INT0 0x800000
> @@ -39,7 +40,7 @@ struct xgene_msi_group {
> };
>
> struct xgene_msi {
> - struct device_node *node;
> + struct fwnode_handle *fwnode;
> struct irq_domain *inner_domain;
> struct irq_domain *msi_domain;
> u64 msi_addr;
> @@ -249,6 +250,13 @@ static const struct irq_domain_ops msi_domain_ops = {
> .free = xgene_irq_domain_free,
> };
>
> +#ifdef CONFIG_ACPI
> +static struct fwnode_handle *xgene_msi_get_fwnode(struct device *dev)
> +{
> + return xgene_msi_ctrl.fwnode;
> +}
> +#endif
> +
> static int xgene_allocate_domains(struct xgene_msi *msi)
> {
> msi->inner_domain = irq_domain_add_linear(NULL, NR_MSI_VEC,
> @@ -256,7 +264,7 @@ static int xgene_allocate_domains(struct xgene_msi *msi)
> if (!msi->inner_domain)
> return -ENOMEM;
>
> - msi->msi_domain = pci_msi_create_irq_domain(of_node_to_fwnode(msi->node),
> + msi->msi_domain = pci_msi_create_irq_domain(msi->fwnode,
> &xgene_msi_domain_info,
> msi->inner_domain);
>
> @@ -265,6 +273,9 @@ static int xgene_allocate_domains(struct xgene_msi *msi)
> return -ENOMEM;
> }
>
> +#ifdef CONFIG_ACPI
> + pci_msi_register_fwnode_provider(&xgene_msi_get_fwnode);
> +#endif
> return 0;
> }
>
> @@ -473,6 +484,13 @@ static const struct of_device_id xgene_msi_match_table[] = {
> {},
> };
>
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id xgene_msi_acpi_ids[] = {
> + {"APMC0D0E", 0},
> + { },
> +};
> +#endif
> +
> static int xgene_msi_probe(struct platform_device *pdev)
> {
> struct resource *res;
> @@ -494,7 +512,17 @@ static int xgene_msi_probe(struct platform_device *pdev)
> goto error;
> }
> xgene_msi->msi_addr = res->start;
> - xgene_msi->node = pdev->dev.of_node;
> +
> + xgene_msi->fwnode = of_node_to_fwnode(pdev->dev.of_node);
> + if (!xgene_msi->fwnode) {
> + xgene_msi->fwnode = irq_domain_alloc_fwnode(NULL);
> + if (!xgene_msi->fwnode) {
> + dev_err(&pdev->dev, "Failed to create fwnode\n");
> + rc = ENOMEM;
> + goto error;
> + }
> + }
> +
> xgene_msi->num_cpus = num_possible_cpus();
>
> rc = xgene_msi_init_allocator(xgene_msi);
> @@ -571,6 +599,7 @@ static struct platform_driver xgene_msi_driver = {
> .driver = {
> .name = "xgene-msi",
> .of_match_table = xgene_msi_match_table,
> + .acpi_match_table = ACPI_PTR(xgene_msi_acpi_ids),
> },
> .probe = xgene_msi_probe,
> .remove = xgene_msi_remove,
> --
> 1.9.1
>
|
|
|
On Sat, Feb 20, 2016 at 1:47 PM, Duc Dang < [hidden email]> wrote:
> On Tue, Feb 9, 2016 at 5:56 PM, Duc Dang < [hidden email]> wrote:
>> This patch makes pci-xgene-msi driver ACPI-aware and provides
>> MSI capability for X-Gene v1 PCIe controllers in ACPI boot mode.
>
> Hi Bjorn,
>
> Are you planning to take this patch into your tree?
Sorry, I haven't had a chance to look at this yet. I had a sick
daughter last week, so I'm even farther behind than usual.
When I get to it, I'll be asking questions like:
- Does this correspond to some analogous x86 code?
- Is there something unique about arm64 or X-Gene that means it
needs special code?
I don't really understand the arm64/ACPI strategy yet, so I'll be
looking first for generic solutions that work across x86/ia64/arm64,
and justifications for the inevitable exceptions.
>> Signed-off-by: Duc Dang < [hidden email]>
>> ---
>> drivers/pci/host/pci-xgene-msi.c | 35 ++++++++++++++++++++++++++++++++---
>> 1 file changed, 32 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/pci/host/pci-xgene-msi.c b/drivers/pci/host/pci-xgene-msi.c
>> index a6456b5..466aa93 100644
>> --- a/drivers/pci/host/pci-xgene-msi.c
>> +++ b/drivers/pci/host/pci-xgene-msi.c
>> @@ -24,6 +24,7 @@
>> #include <linux/pci.h>
>> #include <linux/platform_device.h>
>> #include <linux/of_pci.h>
>> +#include <linux/acpi.h>
>>
>> #define MSI_IR0 0x000000
>> #define MSI_INT0 0x800000
>> @@ -39,7 +40,7 @@ struct xgene_msi_group {
>> };
>>
>> struct xgene_msi {
>> - struct device_node *node;
>> + struct fwnode_handle *fwnode;
>> struct irq_domain *inner_domain;
>> struct irq_domain *msi_domain;
>> u64 msi_addr;
>> @@ -249,6 +250,13 @@ static const struct irq_domain_ops msi_domain_ops = {
>> .free = xgene_irq_domain_free,
>> };
>>
>> +#ifdef CONFIG_ACPI
>> +static struct fwnode_handle *xgene_msi_get_fwnode(struct device *dev)
>> +{
>> + return xgene_msi_ctrl.fwnode;
>> +}
>> +#endif
>> +
>> static int xgene_allocate_domains(struct xgene_msi *msi)
>> {
>> msi->inner_domain = irq_domain_add_linear(NULL, NR_MSI_VEC,
>> @@ -256,7 +264,7 @@ static int xgene_allocate_domains(struct xgene_msi *msi)
>> if (!msi->inner_domain)
>> return -ENOMEM;
>>
>> - msi->msi_domain = pci_msi_create_irq_domain(of_node_to_fwnode(msi->node),
>> + msi->msi_domain = pci_msi_create_irq_domain(msi->fwnode,
>> &xgene_msi_domain_info,
>> msi->inner_domain);
>>
>> @@ -265,6 +273,9 @@ static int xgene_allocate_domains(struct xgene_msi *msi)
>> return -ENOMEM;
>> }
>>
>> +#ifdef CONFIG_ACPI
>> + pci_msi_register_fwnode_provider(&xgene_msi_get_fwnode);
>> +#endif
>> return 0;
>> }
>>
>> @@ -473,6 +484,13 @@ static const struct of_device_id xgene_msi_match_table[] = {
>> {},
>> };
>>
>> +#ifdef CONFIG_ACPI
>> +static const struct acpi_device_id xgene_msi_acpi_ids[] = {
>> + {"APMC0D0E", 0},
>> + { },
>> +};
>> +#endif
>> +
>> static int xgene_msi_probe(struct platform_device *pdev)
>> {
>> struct resource *res;
>> @@ -494,7 +512,17 @@ static int xgene_msi_probe(struct platform_device *pdev)
>> goto error;
>> }
>> xgene_msi->msi_addr = res->start;
>> - xgene_msi->node = pdev->dev.of_node;
>> +
>> + xgene_msi->fwnode = of_node_to_fwnode(pdev->dev.of_node);
>> + if (!xgene_msi->fwnode) {
>> + xgene_msi->fwnode = irq_domain_alloc_fwnode(NULL);
>> + if (!xgene_msi->fwnode) {
>> + dev_err(&pdev->dev, "Failed to create fwnode\n");
>> + rc = ENOMEM;
>> + goto error;
>> + }
>> + }
>> +
>> xgene_msi->num_cpus = num_possible_cpus();
>>
>> rc = xgene_msi_init_allocator(xgene_msi);
>> @@ -571,6 +599,7 @@ static struct platform_driver xgene_msi_driver = {
>> .driver = {
>> .name = "xgene-msi",
>> .of_match_table = xgene_msi_match_table,
>> + .acpi_match_table = ACPI_PTR(xgene_msi_acpi_ids),
>> },
>> .probe = xgene_msi_probe,
>> .remove = xgene_msi_remove,
>> --
>> 1.9.1
>>
|
|
|
On Tue, 2016-02-09 at 17:56 -0800, Duc Dang wrote:
> This patch makes pci-xgene-msi driver ACPI-aware and provides
> MSI capability for X-Gene v1 PCIe controllers in ACPI boot mode.
>
> Signed-off-by: Duc Dang < [hidden email]>
> ---
> Â drivers/pci/host/pci-xgene-msi.c | 35 ++++++++++++++++++++++++++++++++---
> Â 1 file changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/host/pci-xgene-msi.c b/drivers/pci/host/pci-xgene-msi.c
> index a6456b5..466aa93 100644
> --- a/drivers/pci/host/pci-xgene-msi.c
> +++ b/drivers/pci/host/pci-xgene-msi.c
> @@ -24,6 +24,7 @@
> Â #include <linux/pci.h>
> Â #include <linux/platform_device.h>
> Â #include <linux/of_pci.h>
> +#include <linux/acpi.h>
> Â
> Â #define MSI_IR0 0x000000
> Â #define MSI_INT0 0x800000
> @@ -39,7 +40,7 @@ struct xgene_msi_group {
> Â };
> Â
> Â struct xgene_msi {
> - struct device_node *node;
> + struct fwnode_handle *fwnode;
> Â struct irq_domain *inner_domain;
> Â struct irq_domain *msi_domain;
> Â u64 msi_addr;
> @@ -249,6 +250,13 @@ static const struct irq_domain_ops msi_domain_ops = {
>  .free   = xgene_irq_domain_free,
> Â };
> Â
> +#ifdef CONFIG_ACPI
> +static struct fwnode_handle *xgene_msi_get_fwnode(struct device *dev)
> +{
> + return xgene_msi_ctrl.fwnode;
> +}
> +#endif
> +
> Â static int xgene_allocate_domains(struct xgene_msi *msi)
> Â {
> Â msi->inner_domain = irq_domain_add_linear(NULL, NR_MSI_VEC,
> @@ -256,7 +264,7 @@ static int xgene_allocate_domains(struct xgene_msi *msi)
> Â if (!msi->inner_domain)
> Â return -ENOMEM;
> Â
> - msi->msi_domain = pci_msi_create_irq_domain(of_node_to_fwnode(msi->node),
> + msi->msi_domain = pci_msi_create_irq_domain(msi->fwnode,
> Â Â Â Â Â &xgene_msi_domain_info,
> Â Â Â Â Â msi->inner_domain);
This doesn't work for me (ACPI probing on Mustang) unless I change this
to be pci_msi_create_default_irq_domain(). The problem seems to be that
the MSI probe happens after the PCIe RC is probed so there is no MSI domain
at the time the PCIe root is initialized by ACPI.
> Â
> @@ -265,6 +273,9 @@ static int xgene_allocate_domains(struct xgene_msi *msi)
> Â return -ENOMEM;
> Â }
> Â
> +#ifdef CONFIG_ACPI
> + pci_msi_register_fwnode_provider(&xgene_msi_get_fwnode);
> +#endif
> Â return 0;
> Â }
> Â
> @@ -473,6 +484,13 @@ static const struct of_device_id xgene_msi_match_table[] = {
> Â {},
> Â };
> Â
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id xgene_msi_acpi_ids[] = {
> + {"APMC0D0E", 0},
> + { },
> +};
> +#endif
> +
> Â static int xgene_msi_probe(struct platform_device *pdev)
> Â {
> Â struct resource *res;
> @@ -494,7 +512,17 @@ static int xgene_msi_probe(struct platform_device *pdev)
> Â goto error;
> Â }
> Â xgene_msi->msi_addr = res->start;
> - xgene_msi->node = pdev->dev.of_node;
> +
> + xgene_msi->fwnode = of_node_to_fwnode(pdev->dev.of_node);
> + if (!xgene_msi->fwnode) {
> + xgene_msi->fwnode = irq_domain_alloc_fwnode(NULL);
> + if (!xgene_msi->fwnode) {
> + dev_err(&pdev->dev, "Failed to create fwnode\n");
> + rc = ENOMEM;
> + goto error;
> + }
> + }
> +
> Â xgene_msi->num_cpus = num_possible_cpus();
> Â
> Â rc = xgene_msi_init_allocator(xgene_msi);
> @@ -571,6 +599,7 @@ static struct platform_driver xgene_msi_driver = {
> Â .driver = {
> Â .name = "xgene-msi",
> Â .of_match_table = xgene_msi_match_table,
> + .acpi_match_table = ACPI_PTR(xgene_msi_acpi_ids),
> Â },
> Â .probe = xgene_msi_probe,
> Â .remove = xgene_msi_remove,
|
|
|
On 24/02/16 16:09, Mark Salter wrote:
> On Tue, 2016-02-09 at 17:56 -0800, Duc Dang wrote:
>> This patch makes pci-xgene-msi driver ACPI-aware and provides
>> MSI capability for X-Gene v1 PCIe controllers in ACPI boot mode.
>>
>> Signed-off-by: Duc Dang < [hidden email]>
>> ---
>> drivers/pci/host/pci-xgene-msi.c | 35 ++++++++++++++++++++++++++++++++---
>> 1 file changed, 32 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/pci/host/pci-xgene-msi.c b/drivers/pci/host/pci-xgene-msi.c
>> index a6456b5..466aa93 100644
>> --- a/drivers/pci/host/pci-xgene-msi.c
>> +++ b/drivers/pci/host/pci-xgene-msi.c
>> @@ -24,6 +24,7 @@
>> #include <linux/pci.h>
>> #include <linux/platform_device.h>
>> #include <linux/of_pci.h>
>> +#include <linux/acpi.h>
>>
>> #define MSI_IR0 0x000000
>> #define MSI_INT0 0x800000
>> @@ -39,7 +40,7 @@ struct xgene_msi_group {
>> };
>>
>> struct xgene_msi {
>> - struct device_node *node;
>> + struct fwnode_handle *fwnode;
>> struct irq_domain *inner_domain;
>> struct irq_domain *msi_domain;
>> u64 msi_addr;
>> @@ -249,6 +250,13 @@ static const struct irq_domain_ops msi_domain_ops = {
>> .free = xgene_irq_domain_free,
>> };
>>
>> +#ifdef CONFIG_ACPI
>> +static struct fwnode_handle *xgene_msi_get_fwnode(struct device *dev)
>> +{
>> + return xgene_msi_ctrl.fwnode;
>> +}
>> +#endif
>> +
>> static int xgene_allocate_domains(struct xgene_msi *msi)
>> {
>> msi->inner_domain = irq_domain_add_linear(NULL, NR_MSI_VEC,
>> @@ -256,7 +264,7 @@ static int xgene_allocate_domains(struct xgene_msi *msi)
>> if (!msi->inner_domain)
>> return -ENOMEM;
>>
>> - msi->msi_domain = pci_msi_create_irq_domain(of_node_to_fwnode(msi->node),
>> + msi->msi_domain = pci_msi_create_irq_domain(msi->fwnode,
>> &xgene_msi_domain_info,
>> msi->inner_domain);
>
> This doesn't work for me (ACPI probing on Mustang) unless I change this
> to be pci_msi_create_default_irq_domain(). The problem seems to be that
> the MSI probe happens after the PCIe RC is probed so there is no MSI domain
> at the time the PCIe root is initialized by ACPI.
pci_msi_create_default_irq_domain is the wrong thing do use, specially
if you have multiple MSI controllers in the system. I certainly wouldn't
want to see it being used on arm64.
This is the usual dependency hell. You try moving the probing earlier,
but that may break something else in the process.
M.
--
Jazz is not dead. It just smells funny...
|
|
|
On Wed, Feb 24, 2016 at 8:16 AM, Marc Zyngier < [hidden email]> wrote:
> On 24/02/16 16:09, Mark Salter wrote:
>> On Tue, 2016-02-09 at 17:56 -0800, Duc Dang wrote:
>>> This patch makes pci-xgene-msi driver ACPI-aware and provides
>>> MSI capability for X-Gene v1 PCIe controllers in ACPI boot mode.
>>>
>>> Signed-off-by: Duc Dang < [hidden email]>
>>> ---
>>> drivers/pci/host/pci-xgene-msi.c | 35 ++++++++++++++++++++++++++++++++---
>>> 1 file changed, 32 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/pci/host/pci-xgene-msi.c b/drivers/pci/host/pci-xgene-msi.c
>>> index a6456b5..466aa93 100644
>>> --- a/drivers/pci/host/pci-xgene-msi.c
>>> +++ b/drivers/pci/host/pci-xgene-msi.c
>>> @@ -24,6 +24,7 @@
>>> #include <linux/pci.h>
>>> #include <linux/platform_device.h>
>>> #include <linux/of_pci.h>
>>> +#include <linux/acpi.h>
>>>
>>> #define MSI_IR0 0x000000
>>> #define MSI_INT0 0x800000
>>> @@ -39,7 +40,7 @@ struct xgene_msi_group {
>>> };
>>>
>>> struct xgene_msi {
>>> - struct device_node *node;
>>> + struct fwnode_handle *fwnode;
>>> struct irq_domain *inner_domain;
>>> struct irq_domain *msi_domain;
>>> u64 msi_addr;
>>> @@ -249,6 +250,13 @@ static const struct irq_domain_ops msi_domain_ops = {
>>> .free = xgene_irq_domain_free,
>>> };
>>>
>>> +#ifdef CONFIG_ACPI
>>> +static struct fwnode_handle *xgene_msi_get_fwnode(struct device *dev)
>>> +{
>>> + return xgene_msi_ctrl.fwnode;
>>> +}
>>> +#endif
>>> +
>>> static int xgene_allocate_domains(struct xgene_msi *msi)
>>> {
>>> msi->inner_domain = irq_domain_add_linear(NULL, NR_MSI_VEC,
>>> @@ -256,7 +264,7 @@ static int xgene_allocate_domains(struct xgene_msi *msi)
>>> if (!msi->inner_domain)
>>> return -ENOMEM;
>>>
>>> - msi->msi_domain = pci_msi_create_irq_domain(of_node_to_fwnode(msi->node),
>>> + msi->msi_domain = pci_msi_create_irq_domain(msi->fwnode,
>>> &xgene_msi_domain_info,
>>> msi->inner_domain);
>>
>> This doesn't work for me (ACPI probing on Mustang) unless I change this
>> to be pci_msi_create_default_irq_domain(). The problem seems to be that
>> the MSI probe happens after the PCIe RC is probed so there is no MSI domain
>> at the time the PCIe root is initialized by ACPI.
>
> pci_msi_create_default_irq_domain is the wrong thing do use, specially
> if you have multiple MSI controllers in the system. I certainly wouldn't
> want to see it being used on arm64.
>
> This is the usual dependency hell. You try moving the probing earlier,
> but that may break something else in the process.
Hi Mark and Marc,
I have modified Tianocore firmware to have MSI node declared before
PCIe node in Dsdt table. With this modification, the MSI driver will
be loaded before PCIe driver and MSI domain is available at the time
PCIe root is initialized.
So we will need a UEFI firmware update to work with this patch.
Regards,
Duc Dang.
>
> M.
> --
> Jazz is not dead. It just smells funny...
|
|
|
On Wed, Feb 24, 2016 at 02:28:10PM -0800, Duc Dang wrote:
> On Wed, Feb 24, 2016 at 8:16 AM, Marc Zyngier < [hidden email]> wrote:
> > On 24/02/16 16:09, Mark Salter wrote:
> >> On Tue, 2016-02-09 at 17:56 -0800, Duc Dang wrote:
> >>> This patch makes pci-xgene-msi driver ACPI-aware and provides
> >>> MSI capability for X-Gene v1 PCIe controllers in ACPI boot mode.
> >>>
> >>> Signed-off-by: Duc Dang < [hidden email]>
> >>> ---
> >>> drivers/pci/host/pci-xgene-msi.c | 35 ++++++++++++++++++++++++++++++++---
> >>> 1 file changed, 32 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/drivers/pci/host/pci-xgene-msi.c b/drivers/pci/host/pci-xgene-msi.c
> >>> index a6456b5..466aa93 100644
> >>> --- a/drivers/pci/host/pci-xgene-msi.c
> >>> +++ b/drivers/pci/host/pci-xgene-msi.c
> >>> @@ -24,6 +24,7 @@
> >>> #include <linux/pci.h>
> >>> #include <linux/platform_device.h>
> >>> #include <linux/of_pci.h>
> >>> +#include <linux/acpi.h>
> >>>
> >>> #define MSI_IR0 0x000000
> >>> #define MSI_INT0 0x800000
> >>> @@ -39,7 +40,7 @@ struct xgene_msi_group {
> >>> };
> >>>
> >>> struct xgene_msi {
> >>> - struct device_node *node;
> >>> + struct fwnode_handle *fwnode;
> >>> struct irq_domain *inner_domain;
> >>> struct irq_domain *msi_domain;
> >>> u64 msi_addr;
> >>> @@ -249,6 +250,13 @@ static const struct irq_domain_ops msi_domain_ops = {
> >>> .free = xgene_irq_domain_free,
> >>> };
> >>>
> >>> +#ifdef CONFIG_ACPI
> >>> +static struct fwnode_handle *xgene_msi_get_fwnode(struct device *dev)
> >>> +{
> >>> + return xgene_msi_ctrl.fwnode;
> >>> +}
> >>> +#endif
> >>> +
> >>> static int xgene_allocate_domains(struct xgene_msi *msi)
> >>> {
> >>> msi->inner_domain = irq_domain_add_linear(NULL, NR_MSI_VEC,
> >>> @@ -256,7 +264,7 @@ static int xgene_allocate_domains(struct xgene_msi *msi)
> >>> if (!msi->inner_domain)
> >>> return -ENOMEM;
> >>>
> >>> - msi->msi_domain = pci_msi_create_irq_domain(of_node_to_fwnode(msi->node),
> >>> + msi->msi_domain = pci_msi_create_irq_domain(msi->fwnode,
> >>> &xgene_msi_domain_info,
> >>> msi->inner_domain);
> >>
> >> This doesn't work for me (ACPI probing on Mustang) unless I change this
> >> to be pci_msi_create_default_irq_domain(). The problem seems to be that
> >> the MSI probe happens after the PCIe RC is probed so there is no MSI domain
> >> at the time the PCIe root is initialized by ACPI.
> >
> > pci_msi_create_default_irq_domain is the wrong thing do use, specially
> > if you have multiple MSI controllers in the system. I certainly wouldn't
> > want to see it being used on arm64.
> >
> > This is the usual dependency hell. You try moving the probing earlier,
> > but that may break something else in the process.
>
> Hi Mark and Marc,
>
> I have modified Tianocore firmware to have MSI node declared before
> PCIe node in Dsdt table. With this modification, the MSI driver will
> be loaded before PCIe driver and MSI domain is available at the time
> PCIe root is initialized.
I am totally against this. We should not hack ACPI tables to make
the kernel work (on top of that with unwritten ordering rules that may
well require changes as kernel evolves), we should have a standard set
of bindings that people use to describe HW in the DSDT and the kernel(s)
has to cope with that. If there is a dependency problem in the description
we may solve it at bindings level, but I absolutely do not want to rely
on DSDT nodes ordering for things to work, that's fragile, no shortcuts
please.
Thanks,
Lorenzo
|
|
|
On Mon, Feb 22, 2016 at 8:03 AM, Bjorn Helgaas < [hidden email]> wrote:
>
> On Sat, Feb 20, 2016 at 1:47 PM, Duc Dang < [hidden email]> wrote:
> > On Tue, Feb 9, 2016 at 5:56 PM, Duc Dang < [hidden email]> wrote:
> >> This patch makes pci-xgene-msi driver ACPI-aware and provides
> >> MSI capability for X-Gene v1 PCIe controllers in ACPI boot mode.
> >
> > Hi Bjorn,
> >
> > Are you planning to take this patch into your tree?
>
> Sorry, I haven't had a chance to look at this yet. I had a sick
> daughter last week, so I'm even farther behind than usual.
>
Hi Bjorn,
I hope your daughter is doing well.
> When I get to it, I'll be asking questions like:
>
> - Does this correspond to some analogous x86 code?
I am not aware of any x86 code similar to this.
> - Is there something unique about arm64 or X-Gene that means it
> needs special code?
The MSI controller on X-Gene v1 is a separate IP block. pci-xgene-msi
is a separate X-Gene v1 specific driver that provides MSI capability
for X-Gene v1 PCIe controllers. We already have device-tree version of
this driver. So the purpose of this patch is to make the driver can be
discovered and loaded when booting with ACPI mode.
Regards,
Duc Dang.
>
> I don't really understand the arm64/ACPI strategy yet, so I'll be
> looking first for generic solutions that work across x86/ia64/arm64,
> and justifications for the inevitable exceptions.
>
> >> Signed-off-by: Duc Dang < [hidden email]>
> >> ---
> >> drivers/pci/host/pci-xgene-msi.c | 35 ++++++++++++++++++++++++++++++++---
> >> 1 file changed, 32 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/pci/host/pci-xgene-msi.c b/drivers/pci/host/pci-xgene-msi.c
> >> index a6456b5..466aa93 100644
> >> --- a/drivers/pci/host/pci-xgene-msi.c
> >> +++ b/drivers/pci/host/pci-xgene-msi.c
> >> @@ -24,6 +24,7 @@
> >> #include <linux/pci.h>
> >> #include <linux/platform_device.h>
> >> #include <linux/of_pci.h>
> >> +#include <linux/acpi.h>
> >>
> >> #define MSI_IR0 0x000000
> >> #define MSI_INT0 0x800000
> >> @@ -39,7 +40,7 @@ struct xgene_msi_group {
> >> };
> >>
> >> struct xgene_msi {
> >> - struct device_node *node;
> >> + struct fwnode_handle *fwnode;
> >> struct irq_domain *inner_domain;
> >> struct irq_domain *msi_domain;
> >> u64 msi_addr;
> >> @@ -249,6 +250,13 @@ static const struct irq_domain_ops msi_domain_ops = {
> >> .free = xgene_irq_domain_free,
> >> };
> >>
> >> +#ifdef CONFIG_ACPI
> >> +static struct fwnode_handle *xgene_msi_get_fwnode(struct device *dev)
> >> +{
> >> + return xgene_msi_ctrl.fwnode;
> >> +}
> >> +#endif
> >> +
> >> static int xgene_allocate_domains(struct xgene_msi *msi)
> >> {
> >> msi->inner_domain = irq_domain_add_linear(NULL, NR_MSI_VEC,
> >> @@ -256,7 +264,7 @@ static int xgene_allocate_domains(struct xgene_msi *msi)
> >> if (!msi->inner_domain)
> >> return -ENOMEM;
> >>
> >> - msi->msi_domain = pci_msi_create_irq_domain(of_node_to_fwnode(msi->node),
> >> + msi->msi_domain = pci_msi_create_irq_domain(msi->fwnode,
> >> &xgene_msi_domain_info,
> >> msi->inner_domain);
> >>
> >> @@ -265,6 +273,9 @@ static int xgene_allocate_domains(struct xgene_msi *msi)
> >> return -ENOMEM;
> >> }
> >>
> >> +#ifdef CONFIG_ACPI
> >> + pci_msi_register_fwnode_provider(&xgene_msi_get_fwnode);
> >> +#endif
> >> return 0;
> >> }
> >>
> >> @@ -473,6 +484,13 @@ static const struct of_device_id xgene_msi_match_table[] = {
> >> {},
> >> };
> >>
> >> +#ifdef CONFIG_ACPI
> >> +static const struct acpi_device_id xgene_msi_acpi_ids[] = {
> >> + {"APMC0D0E", 0},
> >> + { },
> >> +};
> >> +#endif
> >> +
> >> static int xgene_msi_probe(struct platform_device *pdev)
> >> {
> >> struct resource *res;
> >> @@ -494,7 +512,17 @@ static int xgene_msi_probe(struct platform_device *pdev)
> >> goto error;
> >> }
> >> xgene_msi->msi_addr = res->start;
> >> - xgene_msi->node = pdev->dev.of_node;
> >> +
> >> + xgene_msi->fwnode = of_node_to_fwnode(pdev->dev.of_node);
> >> + if (!xgene_msi->fwnode) {
> >> + xgene_msi->fwnode = irq_domain_alloc_fwnode(NULL);
> >> + if (!xgene_msi->fwnode) {
> >> + dev_err(&pdev->dev, "Failed to create fwnode\n");
> >> + rc = ENOMEM;
> >> + goto error;
> >> + }
> >> + }
> >> +
> >> xgene_msi->num_cpus = num_possible_cpus();
> >>
> >> rc = xgene_msi_init_allocator(xgene_msi);
> >> @@ -571,6 +599,7 @@ static struct platform_driver xgene_msi_driver = {
> >> .driver = {
> >> .name = "xgene-msi",
> >> .of_match_table = xgene_msi_match_table,
> >> + .acpi_match_table = ACPI_PTR(xgene_msi_acpi_ids),
> >> },
> >> .probe = xgene_msi_probe,
> >> .remove = xgene_msi_remove,
> >> --
> >> 1.9.1
> >>
|
|
|
On Thu, Feb 25, 2016 at 9:38 AM, Lorenzo Pieralisi
< [hidden email]> wrote:
> On Wed, Feb 24, 2016 at 02:28:10PM -0800, Duc Dang wrote:
>> On Wed, Feb 24, 2016 at 8:16 AM, Marc Zyngier < [hidden email]> wrote:
>> > On 24/02/16 16:09, Mark Salter wrote:
>> >> On Tue, 2016-02-09 at 17:56 -0800, Duc Dang wrote:
>> >>> This patch makes pci-xgene-msi driver ACPI-aware and provides
>> >>> MSI capability for X-Gene v1 PCIe controllers in ACPI boot mode.
>> >>>
>> >>> Signed-off-by: Duc Dang < [hidden email]>
>> >>> ---
>> >>> drivers/pci/host/pci-xgene-msi.c | 35 ++++++++++++++++++++++++++++++++---
>> >>> 1 file changed, 32 insertions(+), 3 deletions(-)
>> >>>
>> >>> diff --git a/drivers/pci/host/pci-xgene-msi.c b/drivers/pci/host/pci-xgene-msi.c
>> >>> index a6456b5..466aa93 100644
>> >>> --- a/drivers/pci/host/pci-xgene-msi.c
>> >>> +++ b/drivers/pci/host/pci-xgene-msi.c
>> >>> @@ -24,6 +24,7 @@
>> >>> #include <linux/pci.h>
>> >>> #include <linux/platform_device.h>
>> >>> #include <linux/of_pci.h>
>> >>> +#include <linux/acpi.h>
>> >>>
>> >>> #define MSI_IR0 0x000000
>> >>> #define MSI_INT0 0x800000
>> >>> @@ -39,7 +40,7 @@ struct xgene_msi_group {
>> >>> };
>> >>>
>> >>> struct xgene_msi {
>> >>> - struct device_node *node;
>> >>> + struct fwnode_handle *fwnode;
>> >>> struct irq_domain *inner_domain;
>> >>> struct irq_domain *msi_domain;
>> >>> u64 msi_addr;
>> >>> @@ -249,6 +250,13 @@ static const struct irq_domain_ops msi_domain_ops = {
>> >>> .free = xgene_irq_domain_free,
>> >>> };
>> >>>
>> >>> +#ifdef CONFIG_ACPI
>> >>> +static struct fwnode_handle *xgene_msi_get_fwnode(struct device *dev)
>> >>> +{
>> >>> + return xgene_msi_ctrl.fwnode;
>> >>> +}
>> >>> +#endif
>> >>> +
>> >>> static int xgene_allocate_domains(struct xgene_msi *msi)
>> >>> {
>> >>> msi->inner_domain = irq_domain_add_linear(NULL, NR_MSI_VEC,
>> >>> @@ -256,7 +264,7 @@ static int xgene_allocate_domains(struct xgene_msi *msi)
>> >>> if (!msi->inner_domain)
>> >>> return -ENOMEM;
>> >>>
>> >>> - msi->msi_domain = pci_msi_create_irq_domain(of_node_to_fwnode(msi->node),
>> >>> + msi->msi_domain = pci_msi_create_irq_domain(msi->fwnode,
>> >>> &xgene_msi_domain_info,
>> >>> msi->inner_domain);
>> >>
>> >> This doesn't work for me (ACPI probing on Mustang) unless I change this
>> >> to be pci_msi_create_default_irq_domain(). The problem seems to be that
>> >> the MSI probe happens after the PCIe RC is probed so there is no MSI domain
>> >> at the time the PCIe root is initialized by ACPI.
>> >
>> > pci_msi_create_default_irq_domain is the wrong thing do use, specially
>> > if you have multiple MSI controllers in the system. I certainly wouldn't
>> > want to see it being used on arm64.
>> >
>> > This is the usual dependency hell. You try moving the probing earlier,
>> > but that may break something else in the process.
>>
>> Hi Mark and Marc,
>>
>> I have modified Tianocore firmware to have MSI node declared before
>> PCIe node in Dsdt table. With this modification, the MSI driver will
>> be loaded before PCIe driver and MSI domain is available at the time
>> PCIe root is initialized.
>
> I am totally against this. We should not hack ACPI tables to make
> the kernel work (on top of that with unwritten ordering rules that may
> well require changes as kernel evolves), we should have a standard set
> of bindings that people use to describe HW in the DSDT and the kernel(s)
> has to cope with that. If there is a dependency problem in the description
> we may solve it at bindings level, but I absolutely do not want to rely
> on DSDT nodes ordering for things to work, that's fragile, no shortcuts
> please.
Hi Lorenzo, Bjorn,
(Refresh this thread on this merge cycle)
I tried to use _DEP method to describe the dependency of PCIe node to
MSI node but it turns out that PCIe driver does not check for the
dependency (only EC (ec.c) and I2C core (i2c-core.c) use
acpi_walk_dep_device_list to check for dependency before loading
dependent drivers)
Do you have other suggestion/example about how to ensure strict
ordering on driver loading in ACPI boot mode without changing the
order of ACPI nodes in DSDT?
Regards,
Duc Dang.
>
> Thanks,
> Lorenzo
|
|
|
Hi Duc,
On Wed, May 25, 2016 at 04:13:35PM -0700, Duc Dang wrote:
> On Thu, Feb 25, 2016 at 9:38 AM, Lorenzo Pieralisi
> < [hidden email]> wrote:
> > On Wed, Feb 24, 2016 at 02:28:10PM -0800, Duc Dang wrote:
> >> On Wed, Feb 24, 2016 at 8:16 AM, Marc Zyngier < [hidden email]> wrote:
> >> > On 24/02/16 16:09, Mark Salter wrote:
> >> >> On Tue, 2016-02-09 at 17:56 -0800, Duc Dang wrote:
> >> >>> This patch makes pci-xgene-msi driver ACPI-aware and provides
> >> >>> MSI capability for X-Gene v1 PCIe controllers in ACPI boot mode.
> >> >>>
> >> >>> Signed-off-by: Duc Dang < [hidden email]>
> >> >>> ---
> >> >>> drivers/pci/host/pci-xgene-msi.c | 35 ++++++++++++++++++++++++++++++++---
> >> >>> 1 file changed, 32 insertions(+), 3 deletions(-)
> >> >>>
> >> >>> diff --git a/drivers/pci/host/pci-xgene-msi.c b/drivers/pci/host/pci-xgene-msi.c
> >> >>> index a6456b5..466aa93 100644
> >> >>> --- a/drivers/pci/host/pci-xgene-msi.c
> >> >>> +++ b/drivers/pci/host/pci-xgene-msi.c
> >> >>> @@ -24,6 +24,7 @@
> >> >>> #include <linux/pci.h>
> >> >>> #include <linux/platform_device.h>
> >> >>> #include <linux/of_pci.h>
> >> >>> +#include <linux/acpi.h>
> >> >>>
> >> >>> #define MSI_IR0 0x000000
> >> >>> #define MSI_INT0 0x800000
> >> >>> @@ -39,7 +40,7 @@ struct xgene_msi_group {
> >> >>> };
> >> >>>
> >> >>> struct xgene_msi {
> >> >>> - struct device_node *node;
> >> >>> + struct fwnode_handle *fwnode;
> >> >>> struct irq_domain *inner_domain;
> >> >>> struct irq_domain *msi_domain;
> >> >>> u64 msi_addr;
> >> >>> @@ -249,6 +250,13 @@ static const struct irq_domain_ops msi_domain_ops = {
> >> >>> .free = xgene_irq_domain_free,
> >> >>> };
> >> >>>
> >> >>> +#ifdef CONFIG_ACPI
> >> >>> +static struct fwnode_handle *xgene_msi_get_fwnode(struct device *dev)
> >> >>> +{
> >> >>> + return xgene_msi_ctrl.fwnode;
> >> >>> +}
> >> >>> +#endif
> >> >>> +
> >> >>> static int xgene_allocate_domains(struct xgene_msi *msi)
> >> >>> {
> >> >>> msi->inner_domain = irq_domain_add_linear(NULL, NR_MSI_VEC,
> >> >>> @@ -256,7 +264,7 @@ static int xgene_allocate_domains(struct xgene_msi *msi)
> >> >>> if (!msi->inner_domain)
> >> >>> return -ENOMEM;
> >> >>>
> >> >>> - msi->msi_domain = pci_msi_create_irq_domain(of_node_to_fwnode(msi->node),
> >> >>> + msi->msi_domain = pci_msi_create_irq_domain(msi->fwnode,
> >> >>> &xgene_msi_domain_info,
> >> >>> msi->inner_domain);
> >> >>
> >> >> This doesn't work for me (ACPI probing on Mustang) unless I change this
> >> >> to be pci_msi_create_default_irq_domain(). The problem seems to be that
> >> >> the MSI probe happens after the PCIe RC is probed so there is no MSI domain
> >> >> at the time the PCIe root is initialized by ACPI.
> >> >
> >> > pci_msi_create_default_irq_domain is the wrong thing do use, specially
> >> > if you have multiple MSI controllers in the system. I certainly wouldn't
> >> > want to see it being used on arm64.
> >> >
> >> > This is the usual dependency hell. You try moving the probing earlier,
> >> > but that may break something else in the process.
> >>
> >> Hi Mark and Marc,
> >>
> >> I have modified Tianocore firmware to have MSI node declared before
> >> PCIe node in Dsdt table. With this modification, the MSI driver will
> >> be loaded before PCIe driver and MSI domain is available at the time
> >> PCIe root is initialized.
> >
> > I am totally against this. We should not hack ACPI tables to make
> > the kernel work (on top of that with unwritten ordering rules that may
> > well require changes as kernel evolves), we should have a standard set
> > of bindings that people use to describe HW in the DSDT and the kernel(s)
> > has to cope with that. If there is a dependency problem in the description
> > we may solve it at bindings level, but I absolutely do not want to rely
> > on DSDT nodes ordering for things to work, that's fragile, no shortcuts
> > please.
>
> Hi Lorenzo, Bjorn,
>
> (Refresh this thread on this merge cycle)
>
> I tried to use _DEP method to describe the dependency of PCIe node to
> MSI node but it turns out that PCIe driver does not check for the
> dependency (only EC (ec.c) and I2C core (i2c-core.c) use
> acpi_walk_dep_device_list to check for dependency before loading
> dependent drivers)
>
> Do you have other suggestion/example about how to ensure strict
> ordering on driver loading in ACPI boot mode without changing the
> order of ACPI nodes in DSDT?
Before doing anything else, I would like to ask you to report the code
paths leading to failures (what fails actually ?) please, and I want
to understand what "this does not work" means.
In particular:
- Why using pci_msi_create_default_irq_domain() works
- Why, if you change the DSDT nodes ordering, things work (please add
some debugging to MSI allocation functions to detect the code paths
leading to correct MSI allocation)
- What's the difference wrt the DT probe path
When we have a detailed view of MSI allocation code paths and failures
we can see what we can do to fix it in ACPI.
Thanks,
Lorenzo
|
|
Locked
