Re: [PATCH] bus: fsl-mc: Add ACPI support for fsl-mc

From: Diana Craciun OSS
Date: Tue Feb 18 2020 - 10:24:13 EST

Next message: Peter Ujfalusi: "Re: [PATCH] ASoC: ti: Allocate dais dynamically for TDM and audio graph card"
Previous message: Jonathan NeuschÃfer: "[PATCH] scripts/spelling.txt: Add syfs/sysfs pattern"
In reply to: Pankaj Bansal (OSS): "RE: [PATCH] bus: fsl-mc: Add ACPI support for fsl-mc"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Lorenzo,

On 2/18/2020 4:46 PM, Lorenzo Pieralisi wrote:

On Tue, Feb 18, 2020 at 12:48:39PM +0000, Pankaj Bansal (OSS) wrote:

[...]

In DT case, we create the domain DOMAIN_BUS_FSL_MC_MSI for MC bus and

it's children.

And then when MC child device is created, we search the "msi-parent"

property from the MC

DT node and get the ITS associated with MC bus. Then we search

DOMAIN_BUS_FSL_MC_MSI

on that ITS. Once we find the domain, we can call msi_domain_alloc_irqs for

that domain.

This is exactly what we tried to do initially with ACPI. But the searching

DOMAIN_BUS_FSL_MC_MSI

associated to an ITS, is something that is part of drivers/acpi/arm64/iort.c.
(similar to DOMAIN_BUS_PLATFORM_MSI and DOMAIN_BUS_PCI_MSI)

Can you have a look at mbigen driver (drivers/irqchip/irq-mbigen.c) to see if
it helps you?

mbigen is an irq converter to convert device's wired interrupts into MSI
(connecting to ITS), which will alloc a bunch of MSIs from ITS platform MSI
domain at the setup.

Unfortunately this is not the same case as ours. As I see Hisilicon IORT table
Is using single id mapping with named components.

https://github.com/tianocore/edk2-platforms/blob/master/Silicon/Hisilicon/Hi1616/D05AcpiTables/D05Iort.asl#L300

while we are not:

https://source.codeaurora.org/external/qoriq/qoriq-components/edk2-platforms/tree/Platform/NXP/LX2160aRdbPkg/AcpiTables/Iort.aslc?h=LX2160_UEFI_ACPI_EAR1#n290

This is because as I said, we are trying to represent a bus in IORT
via named components and not individual devices connected to that bus.

I had a thorough look into this and strictly speaking there is no
*mapping* requirement at all, all you need to know is what ITS the FSL
MC bus is mapping MSIs to. Which brings me to the next question (which
is orthogonal to how to model FSL MC in IORT, that has to be discussed
but I want to have a full picture in mind first).

When you probe the FSL MC as a platform device, the ACPI core,
through IORT (if you add the 1:1 mapping as an array of single
mappings) already link the platform device to ITS platform
device MSI domain (acpi_configure_pmsi_domain()).

The associated fwnode is the *same* (IIUC) as for the
DOMAIN_BUS_FSL_MC_MSI and ITS DOMAIN_BUS_NEXUS, so in practice
you don't need IORT code to retrieve the DOMAIN_BUS_FSL_MC_MSI
domain, the fwnode is the same as the one in the FSL MC platform
device IRQ domain->fwnode pointer and you can use it to
retrieve the DOMAIN_BUS_FSL_MC_MSI domain through it.

Is my reading correct ?

Thank you very much for your effort! Really appreciated! Yes, the understanding is correct. I have prototyped this idea for DT, see below [1].
So, I get the fwnode from the platform device domain (because they are the same with the devices underneath the MC-BUS bridge) and use the fwnode to retrieve the MC-BUS domain.

Overall, DOMAIN_BUS_FSL_MC_MSI is just an MSI layer to override the
provide the MSI domain ->prepare hook (ie to stash the MC device id), no
more (ie its_fsl_mc_msi_prepare()).

That's it for the MSI layer - I need to figure out whether we *want* to
extend IORT (and/or ACPI) to defined bindings for "additional busses",
what I write above is a summary of my understanding, I have not made my
mind up yet.

As for the IOMMU code, it seems like the only thing needed is
extending named components configuration to child devices,
hierarchically.

Laurentiu used a similar approach for DMA configuration (again prototyped for DT). [2]
It involves wiring up a custom .dma_configure for our devices as anyway, it made little sense to pretend that these devices are platform devices and trick the DT or ACPI layers into that. As a nice side effect, this will allow to get rid of our existing hooks in the DT generic code.

As Marc already mentioned, IOMMU and IRQ code must be separate for
future postings but first we need to find a suitable answer to
the problem at hand.

Lorenzo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[1] MSI configuration

Âdrivers/bus/fsl-mc/fsl-mc-msi.c | 11 +++++++++--
Â1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/bus/fsl-mc/fsl-mc-msi.c b/drivers/bus/fsl-mc/fsl-mc-msi.c
index 8b9c66d7c4ff..674f5a60109b 100644
--- a/drivers/bus/fsl-mc/fsl-mc-msi.c
+++ b/drivers/bus/fsl-mc/fsl-mc-msi.c
@@ -182,16 +182,23 @@ int fsl_mc_find_msi_domain(struct device *mc_platform_dev,
Â{
ÂÂÂÂ struct irq_domain *msi_domain;
ÂÂÂÂ struct device_node *mc_of_node = mc_platform_dev->of_node;
+ÂÂÂ struct fwnode_handle *fwnode;

-ÂÂÂ msi_domain = of_msi_get_domain(mc_platform_dev, mc_of_node,
-ÂÂÂ ÂÂÂ ÂÂÂ ÂÂÂ ÂÂÂÂÂÂ DOMAIN_BUS_FSL_MC_MSI);
+ÂÂÂ msi_domain = dev_get_msi_domain(mc_platform_dev);
ÂÂÂÂ if (!msi_domain) {
ÂÂÂÂ ÂÂÂ pr_err("Unable to find fsl-mc MSI domain for %pOF\n",
ÂÂÂÂ ÂÂÂ ÂÂÂÂÂÂ mc_of_node);

ÂÂÂÂ ÂÂÂ return -ENOENT;
ÂÂÂÂ }
+ÂÂÂ fwnode = msi_domain->fwnode;
+ÂÂÂ msi_domain = irq_find_matching_fwnode(fwnode, DOMAIN_BUS_FSL_MC_MSI);
+ÂÂÂ if (!msi_domain) {
+ÂÂÂ ÂÂÂ pr_err("Unable to find fsl-mc MSI domain for %pOF\n",
+ÂÂÂ ÂÂÂ ÂÂÂÂÂ mc_of_node);

+ÂÂÂ ÂÂÂ return -ENOENT;
+ÂÂÂ }
ÂÂÂÂ *mc_msi_domain = msi_domain;
ÂÂÂÂ return 0;
Â}
--
2.17.1

[2] DMA configuration

Âdrivers/bus/fsl-mc/fsl-mc-bus.c | 42 ++++++++++++++++++++++++++++++++-
Â1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
index f9bc9c384ab5..5c6021a13612 100644
--- a/drivers/bus/fsl-mc/fsl-mc-bus.c
+++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
@@ -132,11 +132,51 @@ static int fsl_mc_bus_uevent(struct device *dev, struct kobj_uevent_env *env)
Âstatic int fsl_mc_dma_configure(struct device *dev)
Â{
ÂÂÂÂ struct device *dma_dev = dev;
+ÂÂÂ struct iommu_fwspec *fwspec;
+ÂÂÂ const struct iommu_ops *iommu_ops;
+ÂÂÂ struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
+ÂÂÂ int ret;
+ÂÂÂ u32 icid;

ÂÂÂÂ while (dev_is_fsl_mc(dma_dev))
ÂÂÂÂ ÂÂÂ dma_dev = dma_dev->parent;

-ÂÂÂ return of_dma_configure(dev, dma_dev->of_node, 0);
+ÂÂÂ fwspec = dev_iommu_fwspec_get(dma_dev);
+ÂÂÂ if (!fwspec) {
+ÂÂÂ ÂÂÂ dev_err(dev, "%s: null fwspec\n", __func__);
+ÂÂÂ ÂÂÂ return -ENODEV;
+ÂÂÂ }
+ÂÂÂ iommu_ops = iommu_ops_from_fwnode(fwspec->iommu_fwnode);
+ÂÂÂ if (!iommu_ops) {
+ÂÂÂ ÂÂÂ dev_err(dev, "%s: null iommu ops\n", __func__);
+ÂÂÂ ÂÂÂ return -ENODEV;
+ÂÂÂ }
+
+ÂÂÂ ret = iommu_fwspec_init(dev, fwspec->iommu_fwnode, iommu_ops);
+ÂÂÂ if (ret) {
+ÂÂÂ ÂÂÂ dev_err(dev, "%s: iommu_fwspec_init failed with %d\n", __func__, ret);
+ÂÂÂ ÂÂÂ return ret;
+ÂÂÂ }
+
+ÂÂÂ icid = mc_dev->icid;
+ÂÂÂ ret = iommu_fwspec_add_ids(dev, &icid, 1);
+ÂÂÂ if (ret) {
+ÂÂÂ ÂÂÂ dev_err(dev, "%s: iommu_fwspec_add_ids failed with %d\n", __func__, ret);
+ÂÂÂ ÂÂÂ return ret;
+ÂÂÂ }
+
+ÂÂÂ if (!device_iommu_mapped(dev)) {
+ÂÂÂ ÂÂÂ ret = iommu_probe_device(dev);
+ÂÂÂ ÂÂÂ if (ret) {
+ÂÂÂ ÂÂÂ ÂÂÂ dev_err(dev, "%s: iommu_fwspec_add_ids failed with %d\n", __func__, ret);
+ÂÂÂ ÂÂÂ ÂÂÂ return ret;
+ÂÂÂ ÂÂÂ }
+ÂÂÂ }
+
+ÂÂÂ arch_setup_dma_ops(dev, 0, *dma_dev->dma_mask + 1,
+ÂÂÂ ÂÂÂ ÂÂÂ ÂÂÂ iommu_ops, true);
+
+ÂÂÂ return 0;
Â}

Âstatic ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
--
2.17.1

Regards,
Diana

Next message: Peter Ujfalusi: "Re: [PATCH] ASoC: ti: Allocate dais dynamically for TDM and audio graph card"
Previous message: Jonathan NeuschÃfer: "[PATCH] scripts/spelling.txt: Add syfs/sysfs pattern"
In reply to: Pankaj Bansal (OSS): "RE: [PATCH] bus: fsl-mc: Add ACPI support for fsl-mc"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]