Messages in this thread | | | Subject | Re: [EXT] Re: [PATCH] bus: fsl-mc: Add ACPI support for fsl-mc | From | Robin Murphy <> | Date | Fri, 31 Jan 2020 12:48:48 +0000 |
| |
On 2020-01-31 12:28 pm, Jon Nettleton wrote: > On Fri, Jan 31, 2020 at 1:02 PM Ard Biesheuvel > <ard.biesheuvel@linaro.org> wrote: >> >> On Fri, 31 Jan 2020 at 12:06, Marc Zyngier <maz@kernel.org> wrote: >>> >>> On 2020-01-31 10:35, Makarand Pawagi wrote: >>>>> -----Original Message----- >>>>> From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> >>>>> Sent: Tuesday, January 28, 2020 4:39 PM >>>>> To: Makarand Pawagi <makarand.pawagi@nxp.com> >>>>> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm- >>>>> kernel@lists.infradead.org; linux-acpi@vger.kernel.org; >>>>> linux@armlinux.org.uk; >>>>> jon@solid-run.com; Cristi Sovaiala <cristian.sovaiala@nxp.com>; >>>>> Laurentiu >>>>> Tudor <laurentiu.tudor@nxp.com>; Ioana Ciornei >>>>> <ioana.ciornei@nxp.com>; >>>>> Varun Sethi <V.Sethi@nxp.com>; Calvin Johnson >>>>> <calvin.johnson@nxp.com>; >>>>> Pankaj Bansal <pankaj.bansal@nxp.com>; guohanjun@huawei.com; >>>>> sudeep.holla@arm.com; rjw@rjwysocki.net; lenb@kernel.org; >>>>> stuyoder@gmail.com; tglx@linutronix.de; jason@lakedaemon.net; >>>>> maz@kernel.org; shameerali.kolothum.thodi@huawei.com; will@kernel.org; >>>>> robin.murphy@arm.com; nleeder@codeaurora.org >>>>> Subject: [EXT] Re: [PATCH] bus: fsl-mc: Add ACPI support for fsl-mc >>>>> >>>>> Caution: EXT Email >>>>> >>>>> On Tue, Jan 28, 2020 at 01:38:45PM +0530, Makarand Pawagi wrote: >>>>>> ACPI support is added in the fsl-mc driver. Driver will parse MC DSDT >>>>>> table to extract memory and other resorces. >>>>>> >>>>>> Interrupt (GIC ITS) information will be extracted from MADT table by >>>>>> drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c. >>>>>> >>>>>> IORT table will be parsed to configure DMA. >>>>>> >>>>>> Signed-off-by: Makarand Pawagi <makarand.pawagi@nxp.com> >>>>>> --- >>>>>> drivers/acpi/arm64/iort.c | 53 +++++++++++++++++++++ >>>>>> drivers/bus/fsl-mc/dprc-driver.c | 3 +- >>>>>> drivers/bus/fsl-mc/fsl-mc-bus.c | 48 +++++++++++++------ >>>>>> drivers/bus/fsl-mc/fsl-mc-msi.c | 10 +++- >>>>>> drivers/bus/fsl-mc/fsl-mc-private.h | 4 +- >>>>>> drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c | 71 >>>>> ++++++++++++++++++++++++++++- >>>>>> include/linux/acpi_iort.h | 5 ++ >>>>>> 7 files changed, 174 insertions(+), 20 deletions(-) >>>>>> >>>>>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c >>>>>> index 33f7198..beb9cd5 100644 >>>>>> --- a/drivers/acpi/arm64/iort.c >>>>>> +++ b/drivers/acpi/arm64/iort.c >>>>>> @@ -15,6 +15,7 @@ >>>>>> #include <linux/kernel.h> >>>>>> #include <linux/list.h> >>>>>> #include <linux/pci.h> >>>>>> +#include <linux/fsl/mc.h> >>>>>> #include <linux/platform_device.h> >>>>>> #include <linux/slab.h> >>>>>> >>>>>> @@ -622,6 +623,29 @@ static int iort_dev_find_its_id(struct device >>>>>> *dev, u32 req_id, } >>>>>> >>>>>> /** >>>>>> + * iort_get_fsl_mc_device_domain() - Find MSI domain related to a >>>>>> +device >>>>>> + * @dev: The device. >>>>>> + * @mc_icid: ICID for the fsl_mc device. >>>>>> + * >>>>>> + * Returns: the MSI domain for this device, NULL otherwise */ struct >>>>>> +irq_domain *iort_get_fsl_mc_device_domain(struct device *dev, >>>>>> + u32 mc_icid) { >>>>>> + struct fwnode_handle *handle; >>>>>> + int its_id; >>>>>> + >>>>>> + if (iort_dev_find_its_id(dev, mc_icid, 0, &its_id)) >>>>>> + return NULL; >>>>>> + >>>>>> + handle = iort_find_domain_token(its_id); >>>>>> + if (!handle) >>>>>> + return NULL; >>>>>> + >>>>>> + return irq_find_matching_fwnode(handle, DOMAIN_BUS_FSL_MC_MSI); >>>>>> +} >>>>> >>>>> NAK >>>>> >>>>> I am not willing to take platform specific code in the generic IORT >>>>> layer. >>>>> >>>>> ACPI on ARM64 works on platforms that comply with SBSA/SBBR >>>>> guidelines: >>>>> >>>>> >>>>> https://developer.arm.com/architectures/platform-design/server-systems >>>>> >>>>> Deviating from those requires butchering ACPI specifications (ie IORT) >>>>> and >>>>> related kernel code which goes totally against what ACPI is meant for >>>>> on ARM64 >>>>> systems, so there is no upstream pathway for this code I am afraid. >>>>> >>>> Reason of adding this platform specific function in the generic IORT >>>> layer is >>>> That iort_get_device_domain() only deals with PCI bus >>>> (DOMAIN_BUS_PCI_MSI). >>>> >>>> fsl-mc objects when probed, need to find irq_domain which is associated >>>> with >>>> the fsl-mc bus (DOMAIN_BUS_FSL_MC_MSI). It will not be possible to do >>>> that >>>> if we do not add this function because there are no other suitable APIs >>>> exported >>>> by IORT layer to do the job. >>> >>> I think we all understood the patch. What both Lorenzo and myself are >>> saying is >>> that we do not want non-PCI support in IORT. >>> >> >> IORT supports platform devices (aka named components) as well, and >> there is some support for platform MSIs in the GIC layer. >> >> So it may be possible to hide your exotic bus from the OS entirely, >> and make the firmware instantiate a DSDT with device objects and >> associated IORT nodes that describe whatever lives on that bus as >> named components. >> >> That way, you will not have to change the OS at all, so your hardware >> will not only be supported in linux v5.7+, it will also be supported >> by OSes that commercial distro vendors are shipping today. *That* is >> the whole point of using ACPI. >> >> If you are going to bother and modify the OS, you lose this advantage, >> and ACPI gives you no benefit over DT at all. > > You beat me to it, but thanks for the clarification Ard. No where in > the SBSA spec that I have read does it state that only PCIe devices > are supported by the SMMU. It uses PCIe devices as an example, but > the SMMU section is very generic in term and only says "devices". > > I feel the SBSA omission of SerDes best practices is an oversight in > the standard and something that probably needs to be revisited. > Forcing high speed networking interfaces to be hung off a bus just for > the sake of having a "standard" PCIe interface seems like a step > backward in this regard. I would much rather have the Spec include a > common standard that could be exposed in a consistent manner. But > this is a conversation for a different place.
Just to clarify further, it's not about serdes or high-speed networking per se - describing a fixed-function network adapter as a named component is entirely within scope. The issue is when the hardware is merely a pool of accelerator components that can be dynamically configured at runtime into something that looks like one or more 'virtual' network adapters - there is no standard interface for *that* for SBSA to consider.
Robin.
> > I will work with NXP and find a better way to implement this. > > -Jon >
| |