[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2195bcc4-e5e5-44de-93c7-667da1409de3@arm.com>
Date: Mon, 11 Mar 2024 12:21:27 +0000
From: Robin Murphy <robin.murphy@....com>
To: Howard Yen <howardyen@...gle.com>, gregkh@...uxfoundation.org,
rafael@...nel.org, mathias.nyman@...el.com, hch@....de,
m.szyprowski@...sung.com, andriy.shevchenko@...ux.intel.com,
petr.tesarik.ext@...wei.com, broonie@...nel.org, james@...iv.tech,
james.clark@....com, masahiroy@...nel.org
Cc: linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
iommu@...ts.linux.dev
Subject: Re: [PATCH v5 2/2] usb: host: xhci-plat: add support for multi memory
regions
On 2024-03-11 9:49 am, Howard Yen wrote:
> The reason why it needs multiple regions is that in my system there is
> an always-on subsystem which includes a small size memory, and several
> functions need to run and occupy the memory from the small memory if
> they need to run on the always-on subsystem. These functions must
> allocate the memory from the small memory region, so that they can get
> benefit from the always-on subsystem. So the small memory is split for
> multiple functions which are satisfied with their generic use cases.
> But in specific use cases, like USB3 devices which support the stream
> trasnsfer or multiple devices connect to the host, they required more
> memory than their pre-allocated memory region, so I tried to propose
> this patch to give it the ability to get the memory from the other
> larger memory to solve the issue.
Once again this still fails to make sense - The USB controller has a
special always-on pool from which it "must allocate", yet it's fine if
it also allocates from elsewhere? How on Earth is that supposed to work?
As I said before, if it's actually the case that only certain specific
allocations (based on driver-level knowledge) must come from the special
pool, then this is not something which can realistically be abstracted
by the generic dma-coherent API as it stands (and if so, do the
non-special allocations even need a dedicated second reserved region, or
is that in fact just a massive hack around the dma_coherent_mem design
intentionally not falling back to the regular allocator?)
Thanks,
Robin.
> Signed-off-by: Howard Yen <howardyen@...gle.com>
> ---
> drivers/usb/host/xhci-plat.c | 19 ++++++++++++++++++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 3d071b875308..7892d3eb26d2 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -14,6 +14,7 @@
> #include <linux/pci.h>
> #include <linux/of.h>
> #include <linux/of_device.h>
> +#include <linux/of_reserved_mem.h>
> #include <linux/platform_device.h>
> #include <linux/usb/phy.h>
> #include <linux/slab.h>
> @@ -149,7 +150,7 @@ int xhci_plat_probe(struct platform_device *pdev, struct device *sysdev, const s
> struct xhci_hcd *xhci;
> struct resource *res;
> struct usb_hcd *hcd, *usb3_hcd;
> - int ret;
> + int i, count, ret;
> int irq;
> struct xhci_plat_priv *priv = NULL;
> bool of_match;
> @@ -194,6 +195,19 @@ int xhci_plat_probe(struct platform_device *pdev, struct device *sysdev, const s
>
> xhci->allow_single_roothub = 1;
>
> + count = of_property_count_u32_elems(sysdev->of_node, "memory-region");
> +
> + for (i = 0; i < count; i++) {
> + ret = of_reserved_mem_device_init_by_idx(sysdev, sysdev->of_node, i);
> + if (ret) {
> + dev_err(sysdev, "Could not get reserved memory\n");
> + if (i > 0)
> + of_reserved_mem_device_release(sysdev);
> +
> + return ret;
> + }
> + }
> +
> /*
> * Not all platforms have clks so it is not an error if the
> * clock do not exist.
> @@ -431,6 +445,9 @@ void xhci_plat_remove(struct platform_device *dev)
> clk_disable_unprepare(clk);
> clk_disable_unprepare(reg_clk);
> reset_control_assert(xhci->reset);
> +
> + of_reserved_mem_device_release(hcd->self.sysdev);
> +
> usb_put_hcd(hcd);
>
> pm_runtime_disable(&dev->dev);
Powered by blists - more mailing lists