Messages in this thread Patches in this message | | | Subject | Re: [PATCH] bus: fsl-mc: Add ACPI support for fsl-mc | From | Diana Craciun OSS <> | Date | Tue, 18 Feb 2020 17:24:02 +0200 |
| |
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@lists.infradead.org > 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
| |