On Wed, Jun 14, 2023 at 08:12:13AM -0700, Bjorn Andersson wrote: > On Wed, May 31, 2023 at 11:34:54AM +0200, Stephan Gerhold wrote: > > Reserved memory can be either looked up using the generic function > > of_address_to_resource() or using the special of_reserved_mem_lookup(). > > The latter has the advantage that it ensures that the referenced memory > > region was really reserved and is not e.g. status = "disabled". > > > > of_reserved_mem also supports allocating reserved memory dynamically at > > boot time. This works only when using of_reserved_mem_lookup() since > > there won't be a fixed address in the device tree. > > > > Switch the code to use of_reserved_mem_lookup(), similar to > > qcom_q6v5_wcss.c which is using it already. There is no functional > > difference for static reserved memory allocations. > > > > While at it this also adds two missing of_node_put() calls in > > qcom_q6v5_pas.c. > > > > Signed-off-by: Stephan Gerhold <stephan@xxxxxxxxxxx> > > --- > > See e.g. [1] for an example of dynamically allocated reserved memory. > > (This patch does *not* depend on [1] and is useful without as well...) > > > > NOTE: Changes in qcom_q6v5_adsp.c and qcom_q6v5_pas.c are untested, > > I only checked qcom_q6v5_mss.c and qcom_wcnss.c on MSM8916/DB410c. > > The code changes are pretty similar for all of those though. > > > > [1]: https://lore.kernel.org/linux-arm-msm/20230510-dt-resv-bottom-up-v1-5-3bf68873dbed@xxxxxxxxxxx/ > > --- > > drivers/remoteproc/qcom_q6v5_adsp.c | 22 ++++++++-------- > > drivers/remoteproc/qcom_q6v5_mss.c | 35 +++++++++++++++---------- > > drivers/remoteproc/qcom_q6v5_pas.c | 51 ++++++++++++++++++++----------------- > > drivers/remoteproc/qcom_wcnss.c | 24 ++++++++--------- > > 4 files changed, 69 insertions(+), 63 deletions(-) > > > > diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c > > index 6777a3bd6226..948b3d00a564 100644 > > --- a/drivers/remoteproc/qcom_q6v5_adsp.c > > +++ b/drivers/remoteproc/qcom_q6v5_adsp.c > > @@ -14,8 +14,8 @@ > > #include <linux/kernel.h> > > #include <linux/mfd/syscon.h> > > #include <linux/module.h> > > -#include <linux/of_address.h> > > #include <linux/of_device.h> > > +#include <linux/of_reserved_mem.h> > > #include <linux/platform_device.h> > > #include <linux/pm_domain.h> > > #include <linux/pm_runtime.h> > > @@ -637,28 +637,26 @@ static int adsp_init_mmio(struct qcom_adsp *adsp, > > > > static int adsp_alloc_memory_region(struct qcom_adsp *adsp) > > { > > + struct reserved_mem *rmem = NULL; > > struct device_node *node; > > - struct resource r; > > - int ret; > > > > node = of_parse_phandle(adsp->dev->of_node, "memory-region", 0); > > + if (node) > > + rmem = of_reserved_mem_lookup(node); > > + of_node_put(node); > > + > > if (!node) { > > - dev_err(adsp->dev, "no memory-region specified\n"); > > + dev_err(adsp->dev, "unable to resolve memory-region\n"); > > return -EINVAL; > > } > > > > - ret = of_address_to_resource(node, 0, &r); > > - of_node_put(node); > > - if (ret) > > - return ret; > > - > > - adsp->mem_phys = adsp->mem_reloc = r.start; > > - adsp->mem_size = resource_size(&r); > > Aren't you missing a check for !rmem here? (The others has it) > Indeed, thanks for spotting this! Will send a v2. Stephan
