RE: [EXT] Re: [PATCH v2 12/12] bus: fsl-mc: Add ACPI support for fsl-mc | linux-arm-kernel

quoted


-----Original Message-----
From: Lorenzo Pieralisi <redacted>
Sent: Wednesday, July 15, 2020 3:37 PM
To: Makarand Pawagi <redacted>
Cc: Diana Madalina Craciun (OSS) <redacted>; Laurentiu
Tudor [off-list ref]; linux-arm-kernel@lists.infradead.org;
iommu@lists.linux-foundation.org; linux-acpi@vger.kernel.org;
devicetree@vger.kernel.org; linux-pci@vger.kernel.org; Rob Herring
[off-list ref]; Rafael J. Wysocki [off-list ref]; Joerg Roedel
[off-list ref]; Hanjun Guo [off-list ref]; Bjorn Helgaas
[off-list ref]; Sudeep Holla [off-list ref]; Robin
Murphy [off-list ref]; Catalin Marinas
[off-list ref]; Will Deacon [off-list ref]; Marc Zyngier
[off-list ref]
Subject: Re: [EXT] Re: [PATCH v2 12/12] bus: fsl-mc: Add ACPI support for fsl-mc

Caution: EXT Email

On Thu, Jul 09, 2020 at 10:52:52AM +0000, Makarand Pawagi wrote:

[...]

fsl_mc_bus_probe(struct platform_device *pdev)
     struct fsl_mc_io *mc_io = NULL;
     int container_id;
     phys_addr_t mc_portal_phys_addr;
-   u32 mc_portal_size;
-   struct resource res;
+   u32 mc_portal_size, mc_stream_id;
+   struct resource *plat_res;
+
+   if (!iommu_present(&fsl_mc_bus_type))
+           return -EPROBE_DEFER;

     mc = devm_kzalloc(&pdev->dev, sizeof(*mc), GFP_KERNEL);
     if (!mc)
@@ -874,19 +887,33 @@ static int fsl_mc_bus_probe(struct
platform_device *pdev)

     platform_set_drvdata(pdev, mc);

+   plat_res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+   mc->fsl_mc_regs = devm_ioremap_resource(&pdev->dev,
plat_res);
+   if (IS_ERR(mc->fsl_mc_regs))
+           return PTR_ERR(mc->fsl_mc_regs);
+
+   if (IS_ENABLED(CONFIG_ACPI) && !dev_of_node(&pdev->dev)) {
+           mc_stream_id = readl(mc->fsl_mc_regs + FSL_MC_FAPR);
+           /*
+            * HW ORs the PL and BMT bit, places the result in bit 15 of
+            * the StreamID and ORs in the ICID. Calculate it accordingly.
+            */
+           mc_stream_id = (mc_stream_id & 0xffff) |
+                           ((mc_stream_id & (MC_FAPR_PL | MC_FAPR_BMT)) ?
+                                   0x4000 : 0);
+           error = acpi_dma_configure_id(&pdev->dev,
DEV_DMA_COHERENT,
+                                         &mc_stream_id);
+           if (error)
+                   dev_warn(&pdev->dev, "failed to configure dma: %d.\n",
+                            error);
+   }
+
     /*
      * Get physical address of MC portal for the root DPRC:
      */
-   error = of_address_to_resource(pdev->dev.of_node, 0, &res);
-   if (error < 0) {
-           dev_err(&pdev->dev,
-                   "of_address_to_resource() failed for %pOF\n",
-                   pdev->dev.of_node);
-           return error;
-   }
-
-   mc_portal_phys_addr = res.start;
-   mc_portal_size = resource_size(&res);
+   plat_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+   mc_portal_phys_addr = plat_res->start;
+   mc_portal_size = resource_size(plat_res);
     error = fsl_create_mc_io(&pdev->dev, mc_portal_phys_addr,
                              mc_portal_size, NULL,

FSL_MC_IO_ATOMIC_CONTEXT_PORTAL, &mc_io); @@ -903,11
+930,13
@@ static int fsl_mc_bus_probe(struct
platform_device *pdev)
     dev_info(&pdev->dev, "MC firmware version: %u.%u.%u\n",
              mc_version.major, mc_version.minor,
mc_version.revision);

-   error = get_mc_addr_translation_ranges(&pdev->dev,
-                                          &mc->translation_ranges,
-                                          &mc->num_translation_ranges);
-   if (error < 0)
-           goto error_cleanup_mc_io;
+   if (dev_of_node(&pdev->dev)) {
+           error = get_mc_addr_translation_ranges(&pdev->dev,
+                                           &mc->translation_ranges,
+                                           &mc->num_translation_ranges);
+           if (error < 0)
+                   goto error_cleanup_mc_io;
+   }

     error = dprc_get_container_id(mc_io, 0, &container_id);
     if (error < 0) {
@@ -934,6 +963,7 @@ static int fsl_mc_bus_probe(struct
platform_device
*pdev)
             goto error_cleanup_mc_io;

     mc->root_mc_bus_dev = mc_bus_dev;
+   mc_bus_dev->dev.fwnode = pdev->dev.fwnode;
Makarand, this looks a bit weird. Is there really a reason for it?
Can you clarify please so that we can reach a conclusion on this matter ?
Laurentiu, can you clarify what exactly is the doubt here? Are
you asking about
fwnode assignment from pdev to mc_bus_dev?
Yes. I remember that a while ago I tested without this fwnode
assignment and didn't encounter any issues. Maybe we can just drop it?
Did you tested with PHY changes? Because this is needed for MAC
driver,
where it needs the mc bus node.

Maybe it worth a comment or maybe have it in a different patch?
Since this change is needed for ACPI case and this is ACPI support
case, I feel we should have this change in this patch only instead of
separate patch.
Anyway - you need to seek feedback from Marc on whether patches
11 and 12 are OK from an irqchip perspective, it is possible we can take the rest
of the series independently if everyone agrees but I don't necessarily see a
reason for that.

Long story short: you need Marc's ACK on [11-12], it is your code.
Hi Marc, can you please review/ack this patch?

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

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help