of: Common "memory-region" parsing
Subsystems (7)
Rob Herring (Arm)
maintainercover of: Common "memory-region" parsing
While there's a common function to parse "memory-region" properties for DMA pool regions, there's not anything for driver private regions. As a result, drivers have resorted to parsing "memory-region" properties themselves repeating the same pattern over and over. To fix this, this series adds 2 functions to handle those cases:of_reserved_mem_region_to_resource() and of_reserved_mem_region_count().
I've converted the whole tree, but just including remoteproc here as it has the most cases. I intend to apply the first 2 patches for 6.15 so the driver conversions can be applied for 6.16.
A git tree with all the drivers converted is here[1].
[1] git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git dt/memory-region
Rob Herring (Arm) (3):
of: reserved_mem: Add functions to parse "memory-region"
of: Simplify of_dma_set_restricted_buffer() to use
of_for_each_phandle()
remoteproc: Use of_reserved_mem_region_* functions for "memory-region"
drivers/of/device.c | 34 ++++------ drivers/of/of_reserved_mem.c | 77 +++++++++++++++++++++++ drivers/remoteproc/imx_dsp_rproc.c | 44 +++++-------- drivers/remoteproc/imx_rproc.c | 65 ++++++++----------- drivers/remoteproc/qcom_q6v5_adsp.c | 24 +++---- drivers/remoteproc/qcom_q6v5_mss.c | 60 ++++++------------ drivers/remoteproc/qcom_q6v5_pas.c | 69 ++++++++------------ drivers/remoteproc/qcom_q6v5_wcss.c | 25 +++----- drivers/remoteproc/qcom_wcnss.c | 23 +++---- drivers/remoteproc/rcar_rproc.c | 36 +++++------ drivers/remoteproc/st_remoteproc.c | 39 +++++------- drivers/remoteproc/stm32_rproc.c | 42 ++++++------- drivers/remoteproc/ti_k3_dsp_remoteproc.c | 28 ++++----- drivers/remoteproc/ti_k3_m4_remoteproc.c | 28 ++++----- drivers/remoteproc/ti_k3_r5_remoteproc.c | 28 ++++----- drivers/remoteproc/xlnx_r5_remoteproc.c | 49 +++++---------- include/linux/of_reserved_mem.h | 26 ++++++++ 17 files changed, 329 insertions(+), 368 deletions(-)
--
2.47.2
Rob Herring (Arm)
maintainerPATCH 1/3 of: reserved_mem: Add functions to parse "memory-region"
Drivers with "memory-region" properties currently have to do their own parsing of "memory-region" properties. The result is all the drivers have similar patterns of a call to parse "memory-region" and then get the region's address and size. As this is a standard property, it should have common functions for drivers to use. Add new functions to count the number of regions and retrieve the region's address as a resource.
--- drivers/of/of_reserved_mem.c | 77 +++++++++++++++++++++++++++++++++ include/linux/of_reserved_mem.h | 26 +++++++++++ 2 files changed, 103 insertions(+)
drivers/of/of_reserved_mem.c +77 -0
@@ -12,6 +12,7 @@#define pr_fmt (fmt) "OF: reserved mem: " fmt#include <linux/err.h> +#include <linux/ioport.h> #include <linux/libfdt.h> #include <linux/of.h> #include <linux/of_fdt.h>
@@ -740,3 +741,79 @@ struct reserved_mem *of_reserved_mem_lookup(struct device_node *np)return NULL ; }EXPORT_SYMBOL_GPL (of_reserved_mem_lookup); + +/** +* of_reserved_mem_region_to_resource() - Get a reserved memory region as a resource +* @np: node containing 'memory-region' property +* @idx: index of 'memory-region' property to lookup +* @res: Pointer to a struct resource to fill in with reserved region +* +* This function allows drivers to lookup a node's 'memory-region' property +* entries by index and return a struct resource for the entry. +* +* Returns 0 on success with @res filled in. Returns -ENODEV if 'memory-region' +* is missing or unavailable, -EINVAL for any other error. +*/ +int of_reserved_mem_region_to_resource (const struct device_node *np,unsigned int idx,struct resource *res) +{ +struct reserved_mem *rmem; + +if (!np) +return -EINVAL ; + + struct device_node __free(device_node) *target = of_parse_phandle(np, "memory-region", idx); +if (!target || !of_device_is_available (target)) +return -ENODEV ; + + rmem =of_reserved_mem_lookup (target); +if (!rmem) +return -EINVAL ; + +resource_set_range (res, rmem->base, rmem->size); + res->name = rmem->name; +return 0 ; +} +EXPORT_SYMBOL_GPL (of_reserved_mem_region_to_resource); + +/** +* of_reserved_mem_region_to_resource_byname() - Get a reserved memory region as a resource +* @np: node containing 'memory-region' property +* @name: name of 'memory-region' property entry to lookup +* @res: Pointer to a struct resource to fill in with reserved region +* +* This function allows drivers to lookup a node's 'memory-region' property +* entries by name and return a struct resource for the entry. +* +* Returns 0 on success with @res filled in. Returns -ENODEV if 'memory-region' +* is missing or unavailable, -EINVAL for any other error. +*/ +int of_reserved_mem_region_to_resource_byname (const struct device_node *np,const char *name,struct resource *res) +{ +int idx; + +if (!name) +return -EINVAL ; + + idx =of_property_match_string (np,"memory-region-names" , name); +if (idx <0 ) +return idx; + +return of_reserved_mem_region_to_resource (np, idx, res); +} +EXPORT_SYMBOL_GPL (of_reserved_mem_region_to_resource_byname); + +/** +* of_reserved_mem_region_count() - Return the number of 'memory-region' entries +* @np: node containing 'memory-region' property +* +* This function allows drivers to retrieve the number of entries for a node's +* 'memory-region' property. +* +* Returns the number of entries on success, or negative error code on a +* malformed property. +*/ +int of_reserved_mem_region_count (const struct device_node *np) +{ +return of_count_phandle_with_args (np,"memory-region" ,NULL ); +} +EXPORT_SYMBOL_GPL (of_reserved_mem_region_count);
include/linux/of_reserved_mem.h +26 -0
@@ -7,6 +7,7 @@struct of_phandle_args ;struct reserved_mem_ops ; +struct resource ;struct reserved_mem {const char *name;
@@ -39,6 +40,12 @@ int of_reserved_mem_device_init_by_name(struct device *dev,void of_reserved_mem_device_release (struct device *dev);struct reserved_mem *of_reserved_mem_lookup (struct device_node *np); +int of_reserved_mem_region_to_resource (const struct device_node *np, +unsigned int idx,struct resource *res); +int of_reserved_mem_region_to_resource_byname (const struct device_node *np, +const char *name,struct resource *res); +int of_reserved_mem_region_count (const struct device_node *np); +#else #define RESERVEDMEM_OF_DECLARE (name, compat, init) \
@@ -63,6 +70,25 @@ static inline struct reserved_mem *of_reserved_mem_lookup(struct device_node *np {return NULL ; } + +static inline int of_reserved_mem_region_to_resource (const struct device_node *np, +unsigned int idx, +struct resource *res) +{ +return -ENOSYS ; +} + +static inline int of_reserved_mem_region_to_resource_byname (const struct device_node *np, +const char *name, +struct resource *res) +{ +return -ENOSYS ; +} + +static inline int of_reserved_mem_region_count (const struct device_node *np) +{ +return 0 ; +}#endif /**
Signature · 1 line
2.47.2
Re: [PATCH 1/3] of: reserved_mem: Add functions to parse "memory-region"
Quoted 9 lines · Rob Herring (Arm)
Drivers with "memory-region" properties currently have to do their own parsing of "memory-region" properties. The result is all the drivers have similar patterns of a call to parse "memory-region" and then get the region's address and size. As this is a standard property, it should have common functions for drivers to use. Add new functions to count the number of regions and retrieve the region's address as a resource.
Signed-off-by Rob Herring (Arm) <[email protected]>
Re: [PATCH 1/3] of: reserved_mem: Add functions to parse "memory-region"
In reply to Rob Herring (Arm)Quoted 33 lines · On Mon, 17 Mar 2025 18:24:21 -0500 "Rob Herring (Arm)" <[email protected]> wrote:
Drivers with "memory-region" properties currently have to do their own parsing of "memory-region" properties. The result is all the drivers have similar patterns of a call to parse "memory-region" and then get the region's address and size. As this is a standard property, it should have common functions for drivers to use. Add new functions to count the number of regions and retrieve the region's address as a resource.
Signed-off-by Rob Herring (Arm) <[email protected]>--- drivers/of/of_reserved_mem.c | 77 +++++++++++++++++++++++++++++++++ include/linux/of_reserved_mem.h | 26 +++++++++++ 2 files changed, 103 insertions(+)drivers/of/of_reserved_mem.c +5
-0@@ -12,6 +12,7 @@#define pr_fmt (fmt) "OF: reserved mem: " fmt#include <linux/err.h> +#include <linux/ioport.h> #include <linux/libfdt.h> #include <linux/of.h> #include <linux/of_fdt.h> @@ -740,3 +741,79 @@ struct reserved_mem *of_reserved_mem_lookup(struct device_node *np)return NULL ; }EXPORT_SYMBOL_GPL (of_reserved_mem_lookup); + +/** +* of_reserved_mem_region_to_resource() - Get a reserved memory region as a resource +* @np: node containing 'memory-region' property
drivers/of/of_reserved_mem.c struct reserved_mem *of_reserved_mem_lookup(struct device_node *np) · line 757
Quoted 10 lines
drivers/of/of_reserved_mem.c +10
-0+* @idx: index of 'memory-region' property to lookup +* @res: Pointer to a struct resource to fill in with reserved region +* +* This function allows drivers to lookup a node's 'memory-region' property +* entries by index and return a struct resource for the entry. +* +* Returns 0 on success with @res filled in. Returns -ENODEV if 'memory-region' +* is missing or unavailable, -EINVAL for any other error. +*/ +int of_reserved_mem_region_to_resource (const struct device_node *np,unsigned int idx,struct resource *res)
Maybe wrap?
Quoted 1 line
drivers/of/of_reserved_mem.c +1
-0+{
Rob Herring (Arm)
maintainerPATCH 2/3 of: Simplify of_dma_set_restricted_buffer() to use of_for_each_phandle()
In reply to Rob Herring (Arm)Simplify of_dma_set_restricted_buffer() by using of_property_present() and of_for_each_phandle() iterator.
--- drivers/of/device.c | 34 +++++++++++++--------------------- 1 file changed, 13 insertions(+), 21 deletions(-)
drivers/of/device.c +13 -21
@@ -35,44 +35,36 @@ EXPORT_SYMBOL(of_match_device);static void of_dma_set_restricted_buffer (struct device *dev,struct device_node *np) { -struct device_node *node, *of_node = dev->of_node; -int count, i; +struct device_node *of_node = dev->of_node; +struct of_phandle_iterator it; +int rc, i =0 ;if (!IS_ENABLED (CONFIG_DMA_RESTRICTED_POOL ))return ; - count =of_property_count_elems_of_size (of_node,"memory-region" , -sizeof (u32));/* * If dev->of_node doesn't exist or doesn't contain memory-region, try * the OF node having DMA configuration. */ -if (count <=0 ) { +if (!of_property_present (of_node,"memory-region" )) of_node = np; - count =of_property_count_elems_of_size ( - of_node,"memory-region" ,sizeof (u32)); - } -for (i =0 ; i < count; i++) { - node =of_parse_phandle (of_node,"memory-region" , i); +of_for_each_phandle (&it, rc, of_node,"memory-region" ,NULL ,0 ) {/* * There might be multiple memory regions, but only one * restricted-dma-pool region is allowed. */ -if (of_device_is_compatible (node,"restricted-dma-pool" ) && -of_device_is_available (node)) { -of_node_put (node); -break ; +if (of_device_is_compatible (it.node,"restricted-dma-pool" ) && +of_device_is_available (it.node)) { +if (!of_reserved_mem_device_init_by_idx (dev, of_node, i)) { +of_node_put (it.node); +return ; + } } -of_node_put (node); + i++; } -/* -* Attempt to initialize a restricted-dma-pool region if one was found. -* Note that count can hold a negative error code. -*/ -if (i < count &&of_reserved_mem_device_init_by_idx (dev, of_node, i)) -dev_warn (dev,"failed to initialise \"restricted-dma-pool\" memory node\n" ); +dev_warn (dev,"failed to initialise \"restricted-dma-pool\" memory node\n" ); }/**
Signature · 1 line
2.47.2
Re: [PATCH 2/3] of: Simplify of_dma_set_restricted_buffer() to use of_for_each_phandle()
Hi,
Quoted 58 lines · Rob Herring (Arm)
Simplify
of_dma_set_restricted_buffer()by usingof_property_present()andof_for_each_phandle()iterator.Signed-off-by Rob Herring (Arm) <[email protected]>--- drivers/of/device.c | 34 +++++++++++++--------------------- 1 file changed, 13 insertions(+), 21 deletions(-)drivers/of/device.c +11
-15@@ -35,44 +35,36 @@ EXPORT_SYMBOL(of_match_device);static void of_dma_set_restricted_buffer (struct device *dev,struct device_node *np) { -struct device_node *node, *of_node = dev->of_node; -int count, i; +struct device_node *of_node = dev->of_node; +struct of_phandle_iterator it; +int rc, i =0 ;if (!IS_ENABLED (CONFIG_DMA_RESTRICTED_POOL ))return ; - count =of_property_count_elems_of_size (of_node,"memory-region" , -sizeof (u32));/* * If dev->of_node doesn't exist or doesn't contain memory-region, try * the OF node having DMA configuration. */ -if (count <=0 ) { +if (!of_property_present (of_node,"memory-region" )) of_node = np; - count =of_property_count_elems_of_size ( - of_node,"memory-region" ,sizeof (u32)); - } -for (i =0 ; i < count; i++) { - node =of_parse_phandle (of_node,"memory-region" , i); +of_for_each_phandle (&it, rc, of_node,"memory-region" ,NULL ,0 ) {/* * There might be multiple memory regions, but only one * restricted-dma-pool region is allowed. */ -if (of_device_is_compatible (node,"restricted-dma-pool" ) && -of_device_is_available (node)) { -of_node_put (node); -break ; +if (of_device_is_compatible (it.node,"restricted-dma-pool" ) && +of_device_is_available (it.node)) { +if (!of_reserved_mem_device_init_by_idx (dev, of_node, i)) { +of_node_put (it.node); +return ; + } } -of_node_put (node);
drivers/of/device.c EXPORT_SYMBOL(of_match_device); · line 67
Quoted 10 lines
drivers/of/device.c +2
-6+ i++; } -/* -* Attempt to initialize a restricted-dma-pool region if one was found. -* Note that count can hold a negative error code. -*/ -if (i < count &&of_reserved_mem_device_init_by_idx (dev, of_node, i)) -dev_warn (dev,"failed to initialise \"restricted-dma-pool\" memory node\n" ); +dev_warn (dev,"failed to initialise \"restricted-dma-pool\" memory node\n" );
This changes the behavior. Before this patch, it was:
if a restricted dma pool was found, but initializing it failed, print a warning.
Whereas now it has become:
print a warning unless a restricted dma pool was found and successfully initialized.
This change causes the kernel to print out the warning for devices that don't even do DMA:
simple-pm-bus soc: failed to initialise "restricted-dma-pool" memory node simple-pm-bus 10006000.syscon: failed to initialise "restricted-dma-pool" memory node
mtk-tphy soc:t-phy@11c80000: failed to initialise "restricted-dma-pool" memory node
mtk-tphy soc:t-phy@11ca0000: failed to initialise "restricted-dma-pool" memory node
mediatek-mipi-tx 11cc0000.dsi-phy: failed to initialise "restricted-dma-pool" memory node
mediatek-mipi-tx 11cc0000.dsi-phy: can't get nvmem_cell_get, ignore it clk-mt8186-apmixed 1000c000.syscon: failed to initialise "restricted-dma-pool" memory node
clk-mt8186-topck 10000000.syscon: failed to initialise "restricted-dma-pool" memory node
clk-mt8186-infra-ao 10001000.syscon: failed to initialise "restricted-dma-pool" memory node
clk-mt8186-cam 1a000000.clock-controller: failed to initialise "restricted-dma-pool" memory node
clk-mt8186-cam 1a04f000.clock-controller: failed to initialise "restricted-dma-pool" memory node
clk-mt8186-cam 1a06f000.clock-controller: failed to initialise "restricted-dma-pool" memory node
clk-mt8186-img 15020000.clock-controller: failed to initialise "restricted-dma-pool" memory node
clk-mt8186-img 15820000.clock-controller: failed to initialise "restricted-dma-pool" memory node
clk-mt8186-imp_iic_wrap 11017000.clock-controller: failed to initialise "restricted-dma-pool" memory node
clk-mt8186-ipe 1c000000.clock-controller: failed to initialise "restricted-dma-pool" memory node
clk-mt8186-mcu c53a000.syscon: failed to initialise "restricted-dma-pool" memory node
clk-mt8186-mdp 1b000000.clock-controller: failed to initialise "restricted-dma-pool" memory node
clk-mt8186-mfg 13000000.clock-controller: failed to initialise "restricted-dma-pool" memory node
clk-mt8186-vdec 1602f000.clock-controller: failed to initialise "restricted-dma-pool" memory node
clk-mt8186-venc 17000000.clock-controller: failed to initialise "restricted-dma-pool" memory node
clk-mt8186-wpe 14020000.clock-controller: failed to initialise "restricted-dma-pool" memory node
mt-pmic-pwrap 1000d000.pwrap: failed to initialise "restricted-dma-pool" memory node
platform 1000d000.pwrap:pmic: failed to initialise "restricted-dma-pool" memory node
mtk-svs 1100bc00.svs: failed to initialise "restricted-dma-pool" memory node
ChenYu
Quoted 7 lines
drivers/of/device.c +0
-1}/** -- 2.47.2
Rob Herring
maintainerRe: [PATCH 2/3] of: Simplify of_dma_set_restricted_buffer() to use of_for_each_phandle()
Quoted 78 lines · Chen-Yu Tsai
Hi,
Quoted 58 lines · Rob Herring (Arm)
Simplify
of_dma_set_restricted_buffer()by usingof_property_present()andof_for_each_phandle()iterator.Signed-off-by Rob Herring (Arm) <[email protected]>--- drivers/of/device.c | 34 +++++++++++++--------------------- 1 file changed, 13 insertions(+), 21 deletions(-)drivers/of/device.c +11
-15@@ -35,44 +35,36 @@ EXPORT_SYMBOL(of_match_device);static void of_dma_set_restricted_buffer (struct device *dev,struct device_node *np) { -struct device_node *node, *of_node = dev->of_node; -int count, i; +struct device_node *of_node = dev->of_node; +struct of_phandle_iterator it; +int rc, i =0 ;if (!IS_ENABLED (CONFIG_DMA_RESTRICTED_POOL ))return ; - count =of_property_count_elems_of_size (of_node,"memory-region" , -sizeof (u32));/* * If dev->of_node doesn't exist or doesn't contain memory-region, try * the OF node having DMA configuration. */ -if (count <=0 ) { +if (!of_property_present (of_node,"memory-region" )) of_node = np; - count =of_property_count_elems_of_size ( - of_node,"memory-region" ,sizeof (u32)); - } -for (i =0 ; i < count; i++) { - node =of_parse_phandle (of_node,"memory-region" , i); +of_for_each_phandle (&it, rc, of_node,"memory-region" ,NULL ,0 ) {/* * There might be multiple memory regions, but only one * restricted-dma-pool region is allowed. */ -if (of_device_is_compatible (node,"restricted-dma-pool" ) && -of_device_is_available (node)) { -of_node_put (node); -break ; +if (of_device_is_compatible (it.node,"restricted-dma-pool" ) && +of_device_is_available (it.node)) { +if (!of_reserved_mem_device_init_by_idx (dev, of_node, i)) { +of_node_put (it.node); +return ; + } } -of_node_put (node);drivers/of/device.c EXPORT_SYMBOL(of_match_device); · line 67
Quoted 10 lines
drivers/of/device.c +2
-6+ i++; } -/* -* Attempt to initialize a restricted-dma-pool region if one was found. -* Note that count can hold a negative error code. -*/ -if (i < count &&of_reserved_mem_device_init_by_idx (dev, of_node, i)) -dev_warn (dev,"failed to initialise \"restricted-dma-pool\" memory node\n" ); +dev_warn (dev,"failed to initialise \"restricted-dma-pool\" memory node\n" );This changes the behavior. Before this patch, it was:
if a restricted dma pool was found, but initializing it failed, print a warning.
Quoted 7 lines
Whereas now it has become:
print a warning unless a restricted dma pool was found and successfully initialized.
This change causes the kernel to print out the warning for devices that don't even do DMA:
Thanks. I fixed it up to only warn if i is non-zero.
Rob
Re: [PATCH 2/3] of: Simplify of_dma_set_restricted_buffer() to use of_for_each_phandle()
Quoted 82 lines · Rob Herring
Quoted 80 lines · Chen-Yu Tsai
Hi,
Quoted 58 lines · Rob Herring (Arm)
Simplify
of_dma_set_restricted_buffer()by usingof_property_present()andof_for_each_phandle()iterator.Signed-off-by Rob Herring (Arm) <[email protected]>--- drivers/of/device.c | 34 +++++++++++++--------------------- 1 file changed, 13 insertions(+), 21 deletions(-)drivers/of/device.c +11
-15@@ -35,44 +35,36 @@ EXPORT_SYMBOL(of_match_device);static void of_dma_set_restricted_buffer (struct device *dev,struct device_node *np) { -struct device_node *node, *of_node = dev->of_node; -int count, i; +struct device_node *of_node = dev->of_node; +struct of_phandle_iterator it; +int rc, i =0 ;if (!IS_ENABLED (CONFIG_DMA_RESTRICTED_POOL ))return ; - count =of_property_count_elems_of_size (of_node,"memory-region" , -sizeof (u32));/* * If dev->of_node doesn't exist or doesn't contain memory-region, try * the OF node having DMA configuration. */ -if (count <=0 ) { +if (!of_property_present (of_node,"memory-region" )) of_node = np; - count =of_property_count_elems_of_size ( - of_node,"memory-region" ,sizeof (u32)); - } -for (i =0 ; i < count; i++) { - node =of_parse_phandle (of_node,"memory-region" , i); +of_for_each_phandle (&it, rc, of_node,"memory-region" ,NULL ,0 ) {/* * There might be multiple memory regions, but only one * restricted-dma-pool region is allowed. */ -if (of_device_is_compatible (node,"restricted-dma-pool" ) && -of_device_is_available (node)) { -of_node_put (node); -break ; +if (of_device_is_compatible (it.node,"restricted-dma-pool" ) && +of_device_is_available (it.node)) { +if (!of_reserved_mem_device_init_by_idx (dev, of_node, i)) { +of_node_put (it.node); +return ; + } } -of_node_put (node);drivers/of/device.c EXPORT_SYMBOL(of_match_device); · line 67
Quoted 10 lines
drivers/of/device.c +2
-6+ i++; } -/* -* Attempt to initialize a restricted-dma-pool region if one was found. -* Note that count can hold a negative error code. -*/ -if (i < count &&of_reserved_mem_device_init_by_idx (dev, of_node, i)) -dev_warn (dev,"failed to initialise \"restricted-dma-pool\" memory node\n" ); +dev_warn (dev,"failed to initialise \"restricted-dma-pool\" memory node\n" );This changes the behavior. Before this patch, it was:
if a restricted dma pool was found, but initializing it failed, print a warning.
Whereas now it has become:
Quoted 7 lines
Quoted 5 lines
print a warning unless a restricted dma pool was found and successfully initialized.
This change causes the kernel to print out the warning for devices that don't even do DMA:
Thanks. I fixed it up to only warn if i is non-zero.
Not sure if that matches the old behavior though? A node could have memory-regions for shared dma pools but not restricted dma pools, and i would be non-zero.
IMO the warning should be in the "else" branch of
if (!of_reserved_mem_device_init_by_idx(dev, of_node, i))
ChenYu
Rob Herring (Arm)
maintainerPATCH 3/3 remoteproc: Use of_reserved_mem_region_* functions for "memory-region"
In reply to Rob Herring (Arm)Use the newly added of_reserved_mem_region_to_resource() and of_reserved_mem_region_count() functions to handle "memory-region" properties.
The error handling is a bit different in some cases. Often "memory-region" is optional, so failed lookup is not an error. But then an error in of_reserved_mem_lookup() is treated as an error. However, that distinction is not really important. Either the region is available and usable or it is not. So now, it is just of_reserved_mem_region_to_resource() which is checked for an error.
---
For v6.16
drivers/remoteproc/imx_dsp_rproc.c | 44 ++++++--------- drivers/remoteproc/imx_rproc.c | 65 ++++++++------------- drivers/remoteproc/qcom_q6v5_adsp.c | 24 +++----- drivers/remoteproc/qcom_q6v5_mss.c | 60 +++++++------------- drivers/remoteproc/qcom_q6v5_pas.c | 69 ++++++++--------------- drivers/remoteproc/qcom_q6v5_wcss.c | 25 ++++---- drivers/remoteproc/qcom_wcnss.c | 23 +++----- drivers/remoteproc/rcar_rproc.c | 36 +++++------- drivers/remoteproc/st_remoteproc.c | 39 ++++++------- drivers/remoteproc/stm32_rproc.c | 42 ++++++-------- drivers/remoteproc/ti_k3_dsp_remoteproc.c | 28 ++++----- drivers/remoteproc/ti_k3_m4_remoteproc.c | 28 ++++----- drivers/remoteproc/ti_k3_r5_remoteproc.c | 28 ++++----- drivers/remoteproc/xlnx_r5_remoteproc.c | 49 ++++++---------- 14 files changed, 213 insertions(+), 347 deletions(-)
drivers/remoteproc/imx_dsp_rproc.c +17 -27
@@ -595,11 +595,9 @@ static int imx_dsp_rproc_add_carveout(struct imx_dsp_rproc *priv)struct rproc *rproc = priv->rproc;struct device *dev = rproc->dev.parent;struct device_node *np = dev->of_node; -struct of_phandle_iterator it;struct rproc_mem_entry *mem; -struct reserved_mem *rmem;void __iomem *cpu_addr; -int a; +int a, i =0 ;u64 da;/* Remap required addresses */
@@ -630,45 +628,37 @@ static int imx_dsp_rproc_add_carveout(struct imx_dsp_rproc *priv)rproc_add_carveout (rproc, mem); } -of_phandle_iterator_init (&it, np,"memory-region" ,NULL ,0 ); -while (of_phandle_iterator_next (&it) ==0 ) { +while (1 ) { +int err; +struct resource res; + + err =of_reserved_mem_region_to_resource (np, i++, &res); +if (err) +return 0 ; +/* * Ignore the first memory region which will be used vdev buffer. * No need to do extra handlings, rproc_add_virtio_dev will handle it. */ -if (!strcmp (it.node->name,"vdev0buffer" )) +if (!strcmp (res.name,"vdev0buffer" ))continue ; - rmem =of_reserved_mem_lookup (it.node); -if (!rmem) { -of_node_put (it.node); -dev_err (dev,"unable to acquire memory-region\n" ); +if (imx_dsp_rproc_sys_to_da (priv, res.start,resource_size (&res), &da))return -EINVAL ; - } -if (imx_dsp_rproc_sys_to_da (priv, rmem->base, rmem->size, &da)) { -of_node_put (it.node); -return -EINVAL ; - } - - cpu_addr =devm_ioremap_wc (dev, rmem->base, rmem->size); + cpu_addr =devm_ioremap_resource_wc (dev, &res);if (!cpu_addr) { -of_node_put (it.node); -dev_err (dev,"failed to map memory %p\n" , &rmem->base); +dev_err (dev,"failed to map memory %pR\n" , &res);return -ENOMEM ; }/* Register memory region */ - mem =rproc_mem_entry_init (dev, (void __force *)cpu_addr, (dma_addr_t )rmem->base, - rmem->size, da,NULL ,NULL , it.node->name); - -if (mem) { -rproc_coredump_add_segment (rproc, da, rmem->size); - }else { -of_node_put (it.node); + mem =rproc_mem_entry_init (dev, (void __force *)cpu_addr, (dma_addr_t )res.start, +resource_size (&res), da,NULL ,NULL , res.name); +if (!mem)return -ENOMEM ; - } +rproc_coredump_add_segment (rproc, da,resource_size (&res));rproc_add_carveout (rproc, mem); }
drivers/remoteproc/imx_rproc.c +25 -40
118 lines not on this page · raw message
drivers/remoteproc/qcom_q6v5_adsp.c +9 -15
@@ -625,26 +625,20 @@ 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; - - node =of_parse_phandle (adsp->dev->of_node,"memory-region" ,0 ); -if (node) - rmem =of_reserved_mem_lookup (node); -of_node_put (node); +int ret; +struct resource res; -if (!rmem) { + ret =of_reserved_mem_region_to_resource (adsp->dev->of_node,0 , &res); +if (!ret) {dev_err (adsp->dev,"unable to resolve memory-region\n" ); -return -EINVAL ; +return ret; } - adsp->mem_phys = adsp->mem_reloc = rmem->base; - adsp->mem_size = rmem->size; - adsp->mem_region =devm_ioremap_wc (adsp->dev, - adsp->mem_phys, adsp->mem_size); + adsp->mem_phys = adsp->mem_reloc = res.start; + adsp->mem_size =resource_size (&res); + adsp->mem_region =devm_ioremap_resource_wc (adsp->dev, &res);if (!adsp->mem_region) { -dev_err (adsp->dev,"unable to map memory region: %pa+%zx\n" , - &rmem->base, adsp->mem_size); +dev_err (adsp->dev,"unable to map memory region: %pR\n" , &res);return -EBUSY ; }
drivers/remoteproc/qcom_q6v5_mss.c +19 -41
98 lines not on this page · raw message
drivers/remoteproc/qcom_q6v5_pas.c +25 -44
110 lines not on this page · raw message
drivers/remoteproc/qcom_q6v5_wcss.c +10 -15
@@ -873,27 +873,22 @@ static int q6v5_wcss_init_mmio(struct q6v5_wcss *wcss,static int q6v5_alloc_memory_region (struct q6v5_wcss *wcss) { -struct reserved_mem *rmem =NULL ; -struct device_node *node;struct device *dev = wcss->dev; +struct resource res; +int ret; - node =of_parse_phandle (dev->of_node,"memory-region" ,0 ); -if (node) - rmem =of_reserved_mem_lookup (node); -of_node_put (node); - -if (!rmem) { + ret =of_reserved_mem_region_to_resource (dev->of_node,0 , &res); +if (ret) {dev_err (dev,"unable to acquire memory-region\n" ); -return -EINVAL ; +return ret; } - wcss->mem_phys = rmem->base; - wcss->mem_reloc = rmem->base; - wcss->mem_size = rmem->size; - wcss->mem_region =devm_ioremap_wc (dev, wcss->mem_phys, wcss->mem_size); + wcss->mem_phys = res.start; + wcss->mem_reloc = res.start; + wcss->mem_size =resource_size (&res); + wcss->mem_region =devm_ioremap_resource_wc (dev, &res);if (!wcss->mem_region) { -dev_err (dev,"unable to map memory region: %pa+%pa\n" , - &rmem->base, &rmem->size); +dev_err (dev,"unable to map memory region: %pR\n" , &res);return -EBUSY ; }
drivers/remoteproc/qcom_wcnss.c +9 -14
@@ -506,25 +506,20 @@ static int wcnss_request_irq(struct qcom_wcnss *wcnss,static int wcnss_alloc_memory_region (struct qcom_wcnss *wcnss) { -struct reserved_mem *rmem =NULL ; -struct device_node *node; - - node =of_parse_phandle (wcnss->dev->of_node,"memory-region" ,0 ); -if (node) - rmem =of_reserved_mem_lookup (node); -of_node_put (node); +struct resource res; +int ret; -if (!rmem) { + ret =of_reserved_mem_region_to_resource (wcnss->dev->of_node,0 , &res); +if (ret) {dev_err (wcnss->dev,"unable to resolve memory-region\n" ); -return -EINVAL ; +return ret; } - wcnss->mem_phys = wcnss->mem_reloc = rmem->base; - wcnss->mem_size = rmem->size; - wcnss->mem_region =devm_ioremap_wc (wcnss->dev, wcnss->mem_phys, wcnss->mem_size); + wcnss->mem_phys = wcnss->mem_reloc = res.start; + wcnss->mem_size =resource_size (&res); + wcnss->mem_region =devm_ioremap_resource_wc (wcnss->dev, &res);if (!wcnss->mem_region) { -dev_err (wcnss->dev,"unable to map memory region: %pa+%zx\n" , - &rmem->base, wcnss->mem_size); +dev_err (wcnss->dev,"unable to map memory region: %pR\n" , &res);return -EBUSY ; }
drivers/remoteproc/rcar_rproc.c +14 -22
@@ -52,41 +52,33 @@ static int rcar_rproc_prepare(struct rproc *rproc) {struct device *dev = rproc->dev.parent;struct device_node *np = dev->of_node; -struct of_phandle_iterator it;struct rproc_mem_entry *mem; -struct reserved_mem *rmem; +int i =0 ;u32 da;/* Register associated reserved memory regions */ -of_phandle_iterator_init (&it, np,"memory-region" ,NULL ,0 ); -while (of_phandle_iterator_next (&it) ==0 ) { - - rmem =of_reserved_mem_lookup (it.node); -if (!rmem) { -of_node_put (it.node); -dev_err (&rproc->dev, -"unable to acquire memory-region\n" ); -return -EINVAL ; - } +while (1 ) { +struct resource res; +int ret; + + ret =of_reserved_mem_region_to_resource (np, i++, &res); +if (ret) +return 0 ; -if (rmem->base >U32_MAX ) { -of_node_put (it.node); +if (res.start >U32_MAX )return -EINVAL ; - }/* No need to translate pa to da, R-Car use same map */ - da = rmem->base; + da = res.start; mem =rproc_mem_entry_init (dev,NULL , - rmem->base, - rmem->size, da, + res.start, +resource_size (&res), da, rcar_rproc_mem_alloc, rcar_rproc_mem_release, - it.node->name); + res.name); -if (!mem) { -of_node_put (it.node); +if (!mem)return -ENOMEM ; - }rproc_add_carveout (rproc, mem); }
drivers/remoteproc/st_remoteproc.c +17 -22
@@ -120,40 +120,35 @@ static int st_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw)struct device *dev = rproc->dev.parent;struct device_node *np = dev->of_node;struct rproc_mem_entry *mem; -struct reserved_mem *rmem; -struct of_phandle_iterator it; -int index =0 ; - -of_phandle_iterator_init (&it, np,"memory-region" ,NULL ,0 ); -while (of_phandle_iterator_next (&it) ==0 ) { - rmem =of_reserved_mem_lookup (it.node); -if (!rmem) { -of_node_put (it.node); -dev_err (dev,"unable to acquire memory-region\n" ); -return -EINVAL ; - } +int index =0 , mr =0 ; + +while (1 ) { +struct resource res; +int ret; + + ret =of_reserved_mem_region_to_resource (np, mr++, &res); +if (ret) +return 0 ;/* No need to map vdev buffer */ -if (strcmp (it.node->name,"vdev0buffer" )) { +if (strcmp (res.name,"vdev0buffer" )) {/* Register memory region */ mem =rproc_mem_entry_init (dev,NULL , - (dma_addr_t )rmem->base, - rmem->size, rmem->base, + (dma_addr_t )res.start, +resource_size (&res), res.start, st_rproc_mem_alloc, st_rproc_mem_release, - it.node->name); + res.name); }else {/* Register reserved memory for vdev buffer allocation */ mem =rproc_of_resm_mem_entry_init (dev, index, - rmem->size, - rmem->base, - it.node->name); +resource_size (&res), + res.start, + res.name); } -if (!mem) { -of_node_put (it.node); +if (!mem)return -ENOMEM ; - }rproc_add_carveout (rproc, mem); index++;
drivers/remoteproc/stm32_rproc.c +18 -24
70 lines not on this page · raw message
drivers/remoteproc/ti_k3_dsp_remoteproc.c +11 -17
@@ -535,13 +535,10 @@ static int k3_dsp_reserved_mem_init(struct k3_dsp_rproc *kproc) {struct device *dev = kproc->dev;struct device_node *np = dev->of_node; -struct device_node *rmem_np; -struct reserved_mem *rmem;int num_rmems;int ret, i; - num_rmems =of_property_count_elems_of_size (np,"memory-region" , -sizeof (phandle)); + num_rmems =of_reserved_mem_region_count (np);if (num_rmems <0 ) {dev_err (dev,"device does not reserved memory regions (%pe)\n" ,ERR_PTR (num_rmems));
@@ -571,23 +568,20 @@ static int k3_dsp_reserved_mem_init(struct k3_dsp_rproc *kproc)/* use remaining reserved memory regions for static carveouts */ for (i =0 ; i < num_rmems; i++) { - rmem_np =of_parse_phandle (np,"memory-region" , i +1 ); -if (!rmem_np) -return -EINVAL ; +struct resource res; - rmem =of_reserved_mem_lookup (rmem_np); -of_node_put (rmem_np); -if (!rmem) -return -EINVAL ; + ret =of_reserved_mem_region_to_resource (np, i +1 , &res); +if (ret) +return ret; - kproc->rmem[i].bus_addr = rmem->base; + kproc->rmem[i].bus_addr = res.start;/* 64-bit address regions currently not supported */ - kproc->rmem[i].dev_addr = (u32 )rmem->base; - kproc->rmem[i].size = rmem->size; - kproc->rmem[i].cpu_addr =devm_ioremap_wc (dev, rmem->base, rmem->size); + kproc->rmem[i].dev_addr = (u32 )res.start; + kproc->rmem[i].size =resource_size (&res); + kproc->rmem[i].cpu_addr =devm_ioremap_resource_wc (dev, &res);if (!kproc->rmem[i].cpu_addr) { -dev_err (dev,"failed to map reserved memory#%d at %pa of size %pa\n" , - i +1 , &rmem->base, &rmem->size); +dev_err (dev,"failed to map reserved memory#%d at %pR\n" , + i +1 , &res);return -ENOMEM ; }
drivers/remoteproc/ti_k3_m4_remoteproc.c +11 -17
@@ -393,13 +393,10 @@ static int k3_m4_reserved_mem_init(struct k3_m4_rproc *kproc) {struct device *dev = kproc->dev;struct device_node *np = dev->of_node; -struct device_node *rmem_np; -struct reserved_mem *rmem;int num_rmems;int ret, i; - num_rmems =of_property_count_elems_of_size (np,"memory-region" , -sizeof (phandle)); + num_rmems =of_reserved_mem_region_count (np);if (num_rmems <0 ) {dev_err (dev,"device does not reserved memory regions (%d)\n" , num_rmems);
@@ -428,23 +425,20 @@ static int k3_m4_reserved_mem_init(struct k3_m4_rproc *kproc)/* use remaining reserved memory regions for static carveouts */ for (i =0 ; i < num_rmems; i++) { - rmem_np =of_parse_phandle (np,"memory-region" , i +1 ); -if (!rmem_np) -return -EINVAL ; +struct resource res; - rmem =of_reserved_mem_lookup (rmem_np); -of_node_put (rmem_np); -if (!rmem) -return -EINVAL ; + ret =of_reserved_mem_region_to_resource (np, i +1 , &res); +if (ret) +return ret; - kproc->rmem[i].bus_addr = rmem->base; + kproc->rmem[i].bus_addr = res.start;/* 64-bit address regions currently not supported */ - kproc->rmem[i].dev_addr = (u32 )rmem->base; - kproc->rmem[i].size = rmem->size; - kproc->rmem[i].cpu_addr =devm_ioremap_wc (dev, rmem->base, rmem->size); + kproc->rmem[i].dev_addr = (u32 )res.start; + kproc->rmem[i].size =resource_size (&res); + kproc->rmem[i].cpu_addr =devm_ioremap_resource_wc (dev, &res);if (!kproc->rmem[i].cpu_addr) { -dev_err (dev,"failed to map reserved memory#%d at %pa of size %pa\n" , - i +1 , &rmem->base, &rmem->size); +dev_err (dev,"failed to map reserved memory#%d at %pR\n" , + i +1 , &res);return -ENOMEM ; }
drivers/remoteproc/ti_k3_r5_remoteproc.c +11 -17
@@ -966,13 +966,10 @@ static int k3_r5_reserved_mem_init(struct k3_r5_rproc *kproc) {struct device *dev = kproc->dev;struct device_node *np =dev_of_node (dev); -struct device_node *rmem_np; -struct reserved_mem *rmem;int num_rmems;int ret, i; - num_rmems =of_property_count_elems_of_size (np,"memory-region" , -sizeof (phandle)); + num_rmems =of_reserved_mem_region_count (np);if (num_rmems <=0 ) {dev_err (dev,"device does not have reserved memory regions, ret = %d\n" , num_rmems);
@@ -1003,16 +1000,13 @@ static int k3_r5_reserved_mem_init(struct k3_r5_rproc *kproc)/* use remaining reserved memory regions for static carveouts */ for (i =0 ; i < num_rmems; i++) { - rmem_np =of_parse_phandle (np,"memory-region" , i +1 ); -if (!rmem_np) -return -EINVAL ; +struct resource res; - rmem =of_reserved_mem_lookup (rmem_np); -of_node_put (rmem_np); -if (!rmem) -return -EINVAL ; + ret =of_reserved_mem_region_to_resource (np, i +1 , &res); +if (ret) +return ret; - kproc->rmem[i].bus_addr = rmem->base; + kproc->rmem[i].bus_addr = res.start;/* * R5Fs do not have an MMU, but have a Region Address Translator * (RAT) module that provides a fixed entry translation between
@@ -1023,12 +1017,12 @@ static int k3_r5_reserved_mem_init(struct k3_r5_rproc *kproc)* addresses/supported memory regions are restricted to 32-bit * bus addresses, and are identical */ - kproc->rmem[i].dev_addr = (u32 )rmem->base; - kproc->rmem[i].size = rmem->size; - kproc->rmem[i].cpu_addr =devm_ioremap_wc (dev, rmem->base, rmem->size); + kproc->rmem[i].dev_addr = (u32 )res.start; + kproc->rmem[i].size =resource_size (&res); + kproc->rmem[i].cpu_addr =devm_ioremap_resource_wc (dev, &res);if (!kproc->rmem[i].cpu_addr) { -dev_err (dev,"failed to map reserved memory#%d at %pa of size %pa\n" , - i +1 , &rmem->base, &rmem->size); +dev_err (dev,"failed to map reserved memory#%d at %pR\n" , + i +1 , &res);return -ENOMEM ; }
drivers/remoteproc/xlnx_r5_remoteproc.c +17 -32
87 lines not on this page · raw message
--
2.47.2
Re: [PATCH 3/3] remoteproc: Use of_reserved_mem_region_* functions for "memory-region"
Quoted 5 lines · Rob Herring (Arm)
Use the newly added
of_reserved_mem_region_to_resource()andof_reserved_mem_region_count()functions to handle "memory-region" properties.
Quoted 8 lines
The error handling is a bit different in some cases. Often "memory-region" is optional, so failed lookup is not an error. But then an error in
of_reserved_mem_lookup()is treated as an error. However, that distinction is not really important. Either the region is available and usable or it is not. So now, it is justof_reserved_mem_region_to_resource()which is checked for an error.Signed-off-by Rob Herring (Arm) <[email protected]>
For IMX part:
Re: [PATCH 3/3] remoteproc: Use of_reserved_mem_region_* functions for "memory-region"
In reply to Rob Herring (Arm)Hi,
Quoted 15 lines · Rob Herring (Arm)
Use the newly added
of_reserved_mem_region_to_resource()andof_reserved_mem_region_count()functions to handle "memory-region" properties.The error handling is a bit different in some cases. Often "memory-region" is optional, so failed lookup is not an error. But then an error in
of_reserved_mem_lookup()is treated as an error. However, that distinction is not really important. Either the region is available and usable or it is not. So now, it is justof_reserved_mem_region_to_resource()which is checked for an error.Signed-off-by Rob Herring (Arm) <[email protected]>---
For v6.16
Quoted 16 lines
drivers/remoteproc/imx_dsp_rproc.c | 44 ++++++--------- drivers/remoteproc/imx_rproc.c | 65 ++++++++------------- drivers/remoteproc/qcom_q6v5_adsp.c | 24 +++----- drivers/remoteproc/qcom_q6v5_mss.c | 60 +++++++------------- drivers/remoteproc/qcom_q6v5_pas.c | 69 ++++++++--------------- drivers/remoteproc/qcom_q6v5_wcss.c | 25 ++++---- drivers/remoteproc/qcom_wcnss.c | 23 +++----- drivers/remoteproc/rcar_rproc.c | 36 +++++------- drivers/remoteproc/st_remoteproc.c | 39 ++++++------- drivers/remoteproc/stm32_rproc.c | 42 ++++++-------- drivers/remoteproc/ti_k3_dsp_remoteproc.c | 28 ++++----- drivers/remoteproc/ti_k3_m4_remoteproc.c | 28 ++++----- drivers/remoteproc/ti_k3_r5_remoteproc.c | 28 ++++----- drivers/remoteproc/xlnx_r5_remoteproc.c | 49 ++++++---------- 14 files changed, 213 insertions(+), 347 deletions(-)
<snip>
I get:
ERROR: modpost: "devm_ioremap_resource_wc" [drivers/remoteproc/qcom_q6v5_adsp.ko] undefined!
ERROR: modpost: "devm_ioremap_resource_wc" [drivers/remoteproc/qcom_q6v5_pas.ko] undefined!
ERROR: modpost: "devm_ioremap_resource_wc" [drivers/remoteproc/qcom_wcnss_pil.ko] undefined!
ERROR: modpost: "devm_ioremap_resource_wc" [drivers/remoteproc/ti_k3_dsp_remoteproc.ko] undefined!
ERROR: modpost: "devm_ioremap_resource_wc" [drivers/remoteproc/ti_k3_m4_remoteproc.ko] undefined!
ERROR: modpost: "devm_ioremap_resource_wc" [drivers/remoteproc/ti_k3_r5_remoteproc.ko] undefined!
when building with arm64 defconfig. I guess a proper:EXPORT_SYMBOL(devm_ioremap_resource_wc)
is missing.
Neil
Re: [Linux-stm32] [PATCH 3/3] remoteproc: Use of_reserved_mem_region_* functions for "memory-region"
In reply to Rob Herring (Arm)Hello Rob,
Quoted 733 lines · Rob Herring (Arm)
Use the newly added
of_reserved_mem_region_to_resource()andof_reserved_mem_region_count()functions to handle "memory-region" properties.The error handling is a bit different in some cases. Often "memory-region" is optional, so failed lookup is not an error. But then an error in
of_reserved_mem_lookup()is treated as an error. However, that distinction is not really important. Either the region is available and usable or it is not. So now, it is justof_reserved_mem_region_to_resource()which is checked for an error.Signed-off-by Rob Herring (Arm) <[email protected]>---
For v6.16drivers/remoteproc/imx_dsp_rproc.c | 44 ++++++--------- drivers/remoteproc/imx_rproc.c | 65 ++++++++------------- drivers/remoteproc/qcom_q6v5_adsp.c | 24 +++----- drivers/remoteproc/qcom_q6v5_mss.c | 60 +++++++------------- drivers/remoteproc/qcom_q6v5_pas.c | 69 ++++++++--------------- drivers/remoteproc/qcom_q6v5_wcss.c | 25 ++++---- drivers/remoteproc/qcom_wcnss.c | 23 +++----- drivers/remoteproc/rcar_rproc.c | 36 +++++------- drivers/remoteproc/st_remoteproc.c | 39 ++++++------- drivers/remoteproc/stm32_rproc.c | 42 ++++++-------- drivers/remoteproc/ti_k3_dsp_remoteproc.c | 28 ++++----- drivers/remoteproc/ti_k3_m4_remoteproc.c | 28 ++++----- drivers/remoteproc/ti_k3_r5_remoteproc.c | 28 ++++----- drivers/remoteproc/xlnx_r5_remoteproc.c | 49 ++++++---------- 14 files changed, 213 insertions(+), 347 deletions(-)drivers/remoteproc/imx_dsp_rproc.c +17
-27@@ -595,11 +595,9 @@ static int imx_dsp_rproc_add_carveout(struct imx_dsp_rproc *priv)struct rproc *rproc = priv->rproc;struct device *dev = rproc->dev.parent;struct device_node *np = dev->of_node; -struct of_phandle_iterator it;struct rproc_mem_entry *mem; -struct reserved_mem *rmem;void __iomem *cpu_addr; -int a; +int a, i =0 ;u64 da;/* Remap required addresses */ @@ -630,45 +628,37 @@ static int imx_dsp_rproc_add_carveout(struct imx_dsp_rproc *priv)rproc_add_carveout (rproc, mem); } -of_phandle_iterator_init (&it, np,"memory-region" ,NULL ,0 ); -while (of_phandle_iterator_next (&it) ==0 ) { +while (1 ) { +int err; +struct resource res; + + err =of_reserved_mem_region_to_resource (np, i++, &res); +if (err) +return 0 ; +/* * Ignore the first memory region which will be used vdev buffer. * No need to do extra handlings, rproc_add_virtio_dev will handle it. */ -if (!strcmp (it.node->name,"vdev0buffer" )) +if (!strcmp (res.name,"vdev0buffer" ))continue ; - rmem =of_reserved_mem_lookup (it.node); -if (!rmem) { -of_node_put (it.node); -dev_err (dev,"unable to acquire memory-region\n" ); +if (imx_dsp_rproc_sys_to_da (priv, res.start,resource_size (&res), &da))return -EINVAL ; - } -if (imx_dsp_rproc_sys_to_da (priv, rmem->base, rmem->size, &da)) { -of_node_put (it.node); -return -EINVAL ; - } - - cpu_addr =devm_ioremap_wc (dev, rmem->base, rmem->size); + cpu_addr =devm_ioremap_resource_wc (dev, &res);if (!cpu_addr) { -of_node_put (it.node); -dev_err (dev,"failed to map memory %p\n" , &rmem->base); +dev_err (dev,"failed to map memory %pR\n" , &res);return -ENOMEM ; }/* Register memory region */ - mem =rproc_mem_entry_init (dev, (void __force *)cpu_addr, (dma_addr_t )rmem->base, - rmem->size, da,NULL ,NULL , it.node->name); - -if (mem) { -rproc_coredump_add_segment (rproc, da, rmem->size); - }else { -of_node_put (it.node); + mem =rproc_mem_entry_init (dev, (void __force *)cpu_addr, (dma_addr_t )res.start, +resource_size (&res), da,NULL ,NULL , res.name); +if (!mem)return -ENOMEM ; - } +rproc_coredump_add_segment (rproc, da,resource_size (&res));rproc_add_carveout (rproc, mem); }drivers/remoteproc/imx_rproc.c +25
-40@@ -549,46 +549,41 @@ static int imx_rproc_prepare(struct rproc *rproc) {struct imx_rproc *priv = rproc->priv;struct device_node *np = priv->dev->of_node; -struct of_phandle_iterator it;struct rproc_mem_entry *mem; -struct reserved_mem *rmem; +int i =0 ;u32 da;/* Register associated reserved memory regions */ -of_phandle_iterator_init (&it, np,"memory-region" ,NULL ,0 ); -while (of_phandle_iterator_next (&it) ==0 ) { +while (1 ) { +int err; +struct resource res; + + err =of_reserved_mem_region_to_resource (np, i++, &res); +if (err) +return 0 ; +/* * Ignore the first memory region which will be used vdev buffer. * No need to do extra handlings, rproc_add_virtio_dev will handle it. */ -if (!strcmp (it.node->name,"vdev0buffer" )) +if (!strcmp (res.name,"vdev0buffer" ))continue ; -if (!strcmp (it.node->name,"rsc-table" )) +if (!strcmp (res.name,"rsc-table" ))continue ; - rmem =of_reserved_mem_lookup (it.node); -if (!rmem) { -of_node_put (it.node); -dev_err (priv->dev,"unable to acquire memory-region\n" ); -return -EINVAL ; - } -/* No need to translate pa to da, i.MX use same map */ - da = rmem->base; + da = res.start;/* Register memory region */ - mem =rproc_mem_entry_init (priv->dev,NULL , (dma_addr_t )rmem->base, rmem->size, da, + mem =rproc_mem_entry_init (priv->dev,NULL , (dma_addr_t )res.start,resource_size (&res), da, imx_rproc_mem_alloc, imx_rproc_mem_release, - it.node->name); + res.name); -if (mem) { -rproc_coredump_add_segment (rproc, da, rmem->size); - }else { -of_node_put (it.node); +if (!mem)return -ENOMEM ; - } +rproc_coredump_add_segment (rproc, da,resource_size (&res));rproc_add_carveout (rproc, mem); }@@ -723,47 +718,37 @@ static int imx_rproc_addr_init(struct imx_rproc *priv, }/* memory-region is optional property */ - nph =of_count_phandle_with_args (np,"memory-region" ,NULL ); + nph =of_reserved_mem_region_count (np);if (nph <=0 )return 0 ;/* remap optional addresses */ for (a =0 ; a < nph; a++) { -struct device_node *node;struct resource res; - node =of_parse_phandle (np,"memory-region" , a); -if (!node) -continue ; -/* Not map vdevbuffer, vdevring region */ -if (!strncmp (node->name,"vdev" ,strlen ("vdev" ))) { -of_node_put (node); -continue ; - } - err =of_address_to_resource (node,0 , &res); + err =of_reserved_mem_region_to_resource (np, a, &res);if (err) {dev_err (dev,"unable to resolve memory region\n" ); -of_node_put (node);return err; } -if (b >=IMX_RPROC_MEM_MAX ) { -of_node_put (node); +/* Not map vdevbuffer, vdevring region */ +if (!strncmp (res.name,"vdev" ,strlen ("vdev" ))) +continue ; + +if (b >=IMX_RPROC_MEM_MAX )break ; - }/* Not use resource version, because we might share region */ - priv->mem[b].cpu_addr =devm_ioremap_wc (&pdev->dev, res.start,resource_size (&res)); + priv->mem[b].cpu_addr =devm_ioremap_resource_wc (&pdev->dev, &res);if (!priv->mem[b].cpu_addr) {dev_err (dev,"failed to remap %pr\n" , &res); -of_node_put (node);return -ENOMEM ; } priv->mem[b].sys_addr = res.start; priv->mem[b].size =resource_size (&res); -if (!strcmp (node->name,"rsc-table" )) +if (!strcmp (res.name,"rsc-table" )) priv->rsc_table = priv->mem[b].cpu_addr; -of_node_put (node); b++; }drivers/remoteproc/qcom_q6v5_adsp.c +9
-15@@ -625,26 +625,20 @@ 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; - - node =of_parse_phandle (adsp->dev->of_node,"memory-region" ,0 ); -if (node) - rmem =of_reserved_mem_lookup (node); -of_node_put (node); +int ret; +struct resource res; -if (!rmem) { + ret =of_reserved_mem_region_to_resource (adsp->dev->of_node,0 , &res); +if (!ret) {dev_err (adsp->dev,"unable to resolve memory-region\n" ); -return -EINVAL ; +return ret; } - adsp->mem_phys = adsp->mem_reloc = rmem->base; - adsp->mem_size = rmem->size; - adsp->mem_region =devm_ioremap_wc (adsp->dev, - adsp->mem_phys, adsp->mem_size); + adsp->mem_phys = adsp->mem_reloc = res.start; + adsp->mem_size =resource_size (&res); + adsp->mem_region =devm_ioremap_resource_wc (adsp->dev, &res);if (!adsp->mem_region) { -dev_err (adsp->dev,"unable to map memory region: %pa+%zx\n" , - &rmem->base, adsp->mem_size); +dev_err (adsp->dev,"unable to map memory region: %pR\n" , &res);return -EBUSY ; }drivers/remoteproc/qcom_q6v5_mss.c +19
-41@@ -1881,8 +1881,8 @@ static int q6v5_init_reset(struct q6v5 *qproc)static int q6v5_alloc_memory_region (struct q6v5 *qproc) {struct device_node *child; -struct reserved_mem *rmem; -struct device_node *node; +struct resource res; +int ret;/* * In the absence of mba/mpss sub-child, extract the mba and mpss @@ -1890,71 +1890,49 @@ static int q6v5_alloc_memory_region(struct q6v5 *qproc)*/ child =of_get_child_by_name (qproc->dev->of_node,"mba" );if (!child) { - node =of_parse_phandle (qproc->dev->of_node, -"memory-region" ,0 ); + ret =of_reserved_mem_region_to_resource (qproc->dev->of_node,0 , &res); }else { - node =of_parse_phandle (child,"memory-region" ,0 ); + ret =of_reserved_mem_region_to_resource (child,0 , &res);of_node_put (child); } -if (!node) { -dev_err (qproc->dev,"no mba memory-region specified\n" ); -return -EINVAL ; - } - - rmem =of_reserved_mem_lookup (node); -of_node_put (node); -if (!rmem) { +if (ret) {dev_err (qproc->dev,"unable to resolve mba region\n" ); -return -EINVAL ; +return ret; } - qproc->mba_phys = rmem->base; - qproc->mba_size = rmem->size; + qproc->mba_phys = res.start; + qproc->mba_size =resource_size (&res);if (!child) { - node =of_parse_phandle (qproc->dev->of_node, -"memory-region" ,1 ); + ret =of_reserved_mem_region_to_resource (qproc->dev->of_node,1 , &res); }else { child =of_get_child_by_name (qproc->dev->of_node,"mpss" ); - node =of_parse_phandle (child,"memory-region" ,0 ); + ret =of_reserved_mem_region_to_resource (child,0 , &res);of_node_put (child); } -if (!node) { -dev_err (qproc->dev,"no mpss memory-region specified\n" ); -return -EINVAL ; - } - - rmem =of_reserved_mem_lookup (node); -of_node_put (node); -if (!rmem) { +if (ret) {dev_err (qproc->dev,"unable to resolve mpss region\n" ); -return -EINVAL ; +return ret; } - qproc->mpss_phys = qproc->mpss_reloc = rmem->base; - qproc->mpss_size = rmem->size; + qproc->mpss_phys = qproc->mpss_reloc = res.start; + qproc->mpss_size =resource_size (&res);if (!child) { - node =of_parse_phandle (qproc->dev->of_node,"memory-region" ,2 ); + ret =of_reserved_mem_region_to_resource (qproc->dev->of_node,2 , &res); }else { child =of_get_child_by_name (qproc->dev->of_node,"metadata" ); - node =of_parse_phandle (child,"memory-region" ,0 ); + ret =of_reserved_mem_region_to_resource (child,0 , &res);of_node_put (child); } -if (!node) +if (ret)return 0 ; - rmem =of_reserved_mem_lookup (node); -if (!rmem) { -dev_err (qproc->dev,"unable to resolve metadata region\n" ); -return -EINVAL ; - } - - qproc->mdata_phys = rmem->base; - qproc->mdata_size = rmem->size; + qproc->mdata_phys = res.start; + qproc->mdata_size =resource_size (&res);return 0 ; }drivers/remoteproc/qcom_q6v5_pas.c +25
-44@@ -546,53 +546,37 @@ static void adsp_pds_detach(struct qcom_adsp *adsp, struct device **pds,static int adsp_alloc_memory_region (struct qcom_adsp *adsp) { -struct reserved_mem *rmem; -struct device_node *node; - - node =of_parse_phandle (adsp->dev->of_node,"memory-region" ,0 ); -if (!node) { -dev_err (adsp->dev,"no memory-region specified\n" ); -return -EINVAL ; - } +struct resource res; +int ret; - rmem =of_reserved_mem_lookup (node); -of_node_put (node); -if (!rmem) { + ret =of_reserved_mem_region_to_resource (adsp->dev->of_node,0 , &res); +if (ret) {dev_err (adsp->dev,"unable to resolve memory-region\n" ); -return -EINVAL ; +return ret; } - adsp->mem_phys = adsp->mem_reloc = rmem->base; - adsp->mem_size = rmem->size; - adsp->mem_region =devm_ioremap_wc (adsp->dev, adsp->mem_phys, adsp->mem_size); + adsp->mem_phys = adsp->mem_reloc = res.start; + adsp->mem_size =resource_size (&res); + adsp->mem_region =devm_ioremap_resource_wc (adsp->dev, &res);if (!adsp->mem_region) { -dev_err (adsp->dev,"unable to map memory region: %pa+%zx\n" , - &rmem->base, adsp->mem_size); +dev_err (adsp->dev,"unable to map memory region: %pR\n" , &res);return -EBUSY ; }if (!adsp->dtb_pas_id)return 0 ; - node =of_parse_phandle (adsp->dev->of_node,"memory-region" ,1 ); -if (!node) { -dev_err (adsp->dev,"no dtb memory-region specified\n" ); -return -EINVAL ; - } - - rmem =of_reserved_mem_lookup (node); -of_node_put (node); -if (!rmem) { + ret =of_reserved_mem_region_to_resource (adsp->dev->of_node,1 , &res); +if (ret) {dev_err (adsp->dev,"unable to resolve dtb memory-region\n" ); -return -EINVAL ; +return ret; } - adsp->dtb_mem_phys = adsp->dtb_mem_reloc = rmem->base; - adsp->dtb_mem_size = rmem->size; - adsp->dtb_mem_region =devm_ioremap_wc (adsp->dev, adsp->dtb_mem_phys, adsp->dtb_mem_size); + adsp->dtb_mem_phys = adsp->dtb_mem_reloc = res.start; + adsp->dtb_mem_size =resource_size (&res); + adsp->dtb_mem_region =devm_ioremap_resource_wc (adsp->dev, &res);if (!adsp->dtb_mem_region) { -dev_err (adsp->dev,"unable to map dtb memory region: %pa+%zx\n" , - &rmem->base, adsp->dtb_mem_size); +dev_err (adsp->dev,"unable to map dtb memory region: %pR\n" , &res);return -EBUSY ; }@@ -602,7 +586,6 @@ static int adsp_alloc_memory_region(struct qcom_adsp *adsp)static int adsp_assign_memory_region (struct qcom_adsp *adsp) {struct qcom_scm_vmperm perm[MAX_ASSIGN_COUNT ]; -struct device_node *node;unsigned int perm_size;int offset;int ret;@@ -611,17 +594,15 @@ static int adsp_assign_memory_region(struct qcom_adsp *adsp)return 0 ;for (offset =0 ; offset < adsp->region_assign_count; ++offset) { -struct reserved_mem *rmem =NULL ; - - node =of_parse_phandle (adsp->dev->of_node,"memory-region" , - adsp->region_assign_idx + offset); -if (node) - rmem =of_reserved_mem_lookup (node); -of_node_put (node); -if (!rmem) { +struct resource res; + + ret =of_reserved_mem_region_to_resource (adsp->dev->of_node, + adsp->region_assign_idx + offset, + &res); +if (ret) {dev_err (adsp->dev,"unable to resolve shareable memory-region index %d\n" , offset); -return -EINVAL ; +return ret; }if (adsp->region_assign_shared) {@@ -636,8 +617,8 @@ static int adsp_assign_memory_region(struct qcom_adsp *adsp) perm_size =1 ; } - adsp->region_assign_phys[offset] = rmem->base; - adsp->region_assign_size[offset] = rmem->size; + adsp->region_assign_phys[offset] = res.start; + adsp->region_assign_size[offset] =resource_size (&res); adsp->region_assign_owners[offset] =BIT (QCOM_SCM_VMID_HLOS ); ret = qcom_scm_assign_mem(adsp->region_assign_phys[offset],drivers/remoteproc/qcom_q6v5_wcss.c +10
-15@@ -873,27 +873,22 @@ static int q6v5_wcss_init_mmio(struct q6v5_wcss *wcss,static int q6v5_alloc_memory_region (struct q6v5_wcss *wcss) { -struct reserved_mem *rmem =NULL ; -struct device_node *node;struct device *dev = wcss->dev; +struct resource res; +int ret; - node =of_parse_phandle (dev->of_node,"memory-region" ,0 ); -if (node) - rmem =of_reserved_mem_lookup (node); -of_node_put (node); - -if (!rmem) { + ret =of_reserved_mem_region_to_resource (dev->of_node,0 , &res); +if (ret) {dev_err (dev,"unable to acquire memory-region\n" ); -return -EINVAL ; +return ret; } - wcss->mem_phys = rmem->base; - wcss->mem_reloc = rmem->base; - wcss->mem_size = rmem->size; - wcss->mem_region =devm_ioremap_wc (dev, wcss->mem_phys, wcss->mem_size); + wcss->mem_phys = res.start; + wcss->mem_reloc = res.start; + wcss->mem_size =resource_size (&res); + wcss->mem_region =devm_ioremap_resource_wc (dev, &res);if (!wcss->mem_region) { -dev_err (dev,"unable to map memory region: %pa+%pa\n" , - &rmem->base, &rmem->size); +dev_err (dev,"unable to map memory region: %pR\n" , &res);return -EBUSY ; }drivers/remoteproc/qcom_wcnss.c +9
-14@@ -506,25 +506,20 @@ static int wcnss_request_irq(struct qcom_wcnss *wcnss,static int wcnss_alloc_memory_region (struct qcom_wcnss *wcnss) { -struct reserved_mem *rmem =NULL ; -struct device_node *node; - - node =of_parse_phandle (wcnss->dev->of_node,"memory-region" ,0 ); -if (node) - rmem =of_reserved_mem_lookup (node); -of_node_put (node); +struct resource res; +int ret; -if (!rmem) { + ret =of_reserved_mem_region_to_resource (wcnss->dev->of_node,0 , &res); +if (ret) {dev_err (wcnss->dev,"unable to resolve memory-region\n" ); -return -EINVAL ; +return ret; } - wcnss->mem_phys = wcnss->mem_reloc = rmem->base; - wcnss->mem_size = rmem->size; - wcnss->mem_region =devm_ioremap_wc (wcnss->dev, wcnss->mem_phys, wcnss->mem_size); + wcnss->mem_phys = wcnss->mem_reloc = res.start; + wcnss->mem_size =resource_size (&res); + wcnss->mem_region =devm_ioremap_resource_wc (wcnss->dev, &res);if (!wcnss->mem_region) { -dev_err (wcnss->dev,"unable to map memory region: %pa+%zx\n" , - &rmem->base, wcnss->mem_size); +dev_err (wcnss->dev,"unable to map memory region: %pR\n" , &res);return -EBUSY ; }drivers/remoteproc/rcar_rproc.c +14
-22@@ -52,41 +52,33 @@ static int rcar_rproc_prepare(struct rproc *rproc) {struct device *dev = rproc->dev.parent;struct device_node *np = dev->of_node; -struct of_phandle_iterator it;struct rproc_mem_entry *mem; -struct reserved_mem *rmem; +int i =0 ;u32 da;/* Register associated reserved memory regions */ -of_phandle_iterator_init (&it, np,"memory-region" ,NULL ,0 ); -while (of_phandle_iterator_next (&it) ==0 ) { - - rmem =of_reserved_mem_lookup (it.node); -if (!rmem) { -of_node_put (it.node); -dev_err (&rproc->dev, -"unable to acquire memory-region\n" ); -return -EINVAL ; - } +while (1 ) { +struct resource res; +int ret; + + ret =of_reserved_mem_region_to_resource (np, i++, &res); +if (ret) +return 0 ; -if (rmem->base >U32_MAX ) { -of_node_put (it.node); +if (res.start >U32_MAX )return -EINVAL ; - }/* No need to translate pa to da, R-Car use same map */ - da = rmem->base; + da = res.start; mem =rproc_mem_entry_init (dev,NULL , - rmem->base, - rmem->size, da, + res.start, +resource_size (&res), da, rcar_rproc_mem_alloc, rcar_rproc_mem_release, - it.node->name); + res.name); -if (!mem) { -of_node_put (it.node); +if (!mem)return -ENOMEM ; - }rproc_add_carveout (rproc, mem); }drivers/remoteproc/st_remoteproc.c +17
-22@@ -120,40 +120,35 @@ static int st_rproc_parse_fw(struct rproc *rproc, const struct firmware *fw)struct device *dev = rproc->dev.parent;struct device_node *np = dev->of_node;struct rproc_mem_entry *mem; -struct reserved_mem *rmem; -struct of_phandle_iterator it; -int index =0 ; - -of_phandle_iterator_init (&it, np,"memory-region" ,NULL ,0 ); -while (of_phandle_iterator_next (&it) ==0 ) { - rmem =of_reserved_mem_lookup (it.node); -if (!rmem) { -of_node_put (it.node); -dev_err (dev,"unable to acquire memory-region\n" ); -return -EINVAL ; - } +int index =0 , mr =0 ; + +while (1 ) { +struct resource res; +int ret; + + ret =of_reserved_mem_region_to_resource (np, mr++, &res); +if (ret) +return 0 ;/* No need to map vdev buffer */ -if (strcmp (it.node->name,"vdev0buffer" )) { +if (strcmp (res.name,"vdev0buffer" )) {/* Register memory region */ mem =rproc_mem_entry_init (dev,NULL , - (dma_addr_t )rmem->base, - rmem->size, rmem->base, + (dma_addr_t )res.start, +resource_size (&res), res.start, st_rproc_mem_alloc, st_rproc_mem_release, - it.node->name); + res.name); }else {/* Register reserved memory for vdev buffer allocation */ mem =rproc_of_resm_mem_entry_init (dev, index, - rmem->size, - rmem->base, - it.node->name); +resource_size (&res), + res.start, + res.name); } -if (!mem) { -of_node_put (it.node); +if (!mem)return -ENOMEM ; - }rproc_add_carveout (rproc, mem); index++;drivers/remoteproc/stm32_rproc.c +8
-13@@ -213,52 +213,46 @@ static int stm32_rproc_prepare(struct rproc *rproc) {struct device *dev = rproc->dev.parent;struct device_node *np = dev->of_node; -struct of_phandle_iterator it;struct rproc_mem_entry *mem; -struct reserved_mem *rmem;u64 da; -int index =0 ; +int index =0 , mr =0 ;/* Register associated reserved memory regions */ -of_phandle_iterator_init (&it, np,"memory-region" ,NULL ,0 ); -while (of_phandle_iterator_next (&it) ==0 ) { - rmem =of_reserved_mem_lookup (it.node); -if (!rmem) { -of_node_put (it.node); -dev_err (dev,"unable to acquire memory-region\n" ); -return -EINVAL ; - } +while (1 ) { +struct resource res; +int ret; + + ret =of_reserved_mem_region_to_resource (np, mr++, &res); +if (ret) +return 0 ; -if (stm32_rproc_pa_to_da (rproc, rmem->base, &da) <0 ) { -of_node_put (it.node);
drivers/remoteproc/stm32_rproc.c static int stm32_rproc_prepare(struct rproc *rproc) · line 235
Quoted 10 lines
drivers/remoteproc/stm32_rproc.c +3
-3-dev_err (dev,"memory region not valid %pa\n" , - &rmem->base); +if (stm32_rproc_pa_to_da (rproc, res.start, &da) <0 ) { +dev_err (dev,"memory region not valid %pR\n" , &res);return -EINVAL ; }/* No need to map vdev buffer */ -if (strcmp (it.node->name,"vdev0buffer" )) { +if (strcmp (res.name,"vdev0buffer" )) {
I tested your patches
The update introduces a regression here. The strcmp function never returns 0. Indeed, it.node->name stores the memory region label "vdev0buffer," while res.name stores the memory region name "vdev0buffer@10042000."
Several remoteproc drivers may face the same issue as they embed similar code.
Regards,
Arnaud
Quoted 297 lines
drivers/remoteproc/stm32_rproc.c +7
-8/* Register memory region */ mem =rproc_mem_entry_init (dev,NULL , - (dma_addr_t )rmem->base, - rmem->size, da, + (dma_addr_t )res.start, +resource_size (&res), da, stm32_rproc_mem_alloc, stm32_rproc_mem_release, - it.node->name); + res.name);if (mem)rproc_coredump_add_segment (rproc, da, - rmem->size); +resource_size (&res)); }else {/* Register reserved memory for vdev buffer alloc */ mem =rproc_of_resm_mem_entry_init (dev, index, - rmem->size, - rmem->base, - it.node->name); +resource_size (&res), + res.start, + res.name); }if (!mem) { -of_node_put (it.node);return -ENOMEM ; }drivers/remoteproc/ti_k3_dsp_remoteproc.c +11
-17@@ -535,13 +535,10 @@ static int k3_dsp_reserved_mem_init(struct k3_dsp_rproc *kproc) {struct device *dev = kproc->dev;struct device_node *np = dev->of_node; -struct device_node *rmem_np; -struct reserved_mem *rmem;int num_rmems;int ret, i; - num_rmems =of_property_count_elems_of_size (np,"memory-region" , -sizeof (phandle)); + num_rmems =of_reserved_mem_region_count (np);if (num_rmems <0 ) {dev_err (dev,"device does not reserved memory regions (%pe)\n" ,ERR_PTR (num_rmems));@@ -571,23 +568,20 @@ static int k3_dsp_reserved_mem_init(struct k3_dsp_rproc *kproc)/* use remaining reserved memory regions for static carveouts */ for (i =0 ; i < num_rmems; i++) { - rmem_np =of_parse_phandle (np,"memory-region" , i +1 ); -if (!rmem_np) -return -EINVAL ; +struct resource res; - rmem =of_reserved_mem_lookup (rmem_np); -of_node_put (rmem_np); -if (!rmem) -return -EINVAL ; + ret =of_reserved_mem_region_to_resource (np, i +1 , &res); +if (ret) +return ret; - kproc->rmem[i].bus_addr = rmem->base; + kproc->rmem[i].bus_addr = res.start;/* 64-bit address regions currently not supported */ - kproc->rmem[i].dev_addr = (u32 )rmem->base; - kproc->rmem[i].size = rmem->size; - kproc->rmem[i].cpu_addr =devm_ioremap_wc (dev, rmem->base, rmem->size); + kproc->rmem[i].dev_addr = (u32 )res.start; + kproc->rmem[i].size =resource_size (&res); + kproc->rmem[i].cpu_addr =devm_ioremap_resource_wc (dev, &res);if (!kproc->rmem[i].cpu_addr) { -dev_err (dev,"failed to map reserved memory#%d at %pa of size %pa\n" , - i +1 , &rmem->base, &rmem->size); +dev_err (dev,"failed to map reserved memory#%d at %pR\n" , + i +1 , &res);return -ENOMEM ; }drivers/remoteproc/ti_k3_m4_remoteproc.c +11
-17@@ -393,13 +393,10 @@ static int k3_m4_reserved_mem_init(struct k3_m4_rproc *kproc) {struct device *dev = kproc->dev;struct device_node *np = dev->of_node; -struct device_node *rmem_np; -struct reserved_mem *rmem;int num_rmems;int ret, i; - num_rmems =of_property_count_elems_of_size (np,"memory-region" , -sizeof (phandle)); + num_rmems =of_reserved_mem_region_count (np);if (num_rmems <0 ) {dev_err (dev,"device does not reserved memory regions (%d)\n" , num_rmems);@@ -428,23 +425,20 @@ static int k3_m4_reserved_mem_init(struct k3_m4_rproc *kproc)/* use remaining reserved memory regions for static carveouts */ for (i =0 ; i < num_rmems; i++) { - rmem_np =of_parse_phandle (np,"memory-region" , i +1 ); -if (!rmem_np) -return -EINVAL ; +struct resource res; - rmem =of_reserved_mem_lookup (rmem_np); -of_node_put (rmem_np); -if (!rmem) -return -EINVAL ; + ret =of_reserved_mem_region_to_resource (np, i +1 , &res); +if (ret) +return ret; - kproc->rmem[i].bus_addr = rmem->base; + kproc->rmem[i].bus_addr = res.start;/* 64-bit address regions currently not supported */ - kproc->rmem[i].dev_addr = (u32 )rmem->base; - kproc->rmem[i].size = rmem->size; - kproc->rmem[i].cpu_addr =devm_ioremap_wc (dev, rmem->base, rmem->size); + kproc->rmem[i].dev_addr = (u32 )res.start; + kproc->rmem[i].size =resource_size (&res); + kproc->rmem[i].cpu_addr =devm_ioremap_resource_wc (dev, &res);if (!kproc->rmem[i].cpu_addr) { -dev_err (dev,"failed to map reserved memory#%d at %pa of size %pa\n" , - i +1 , &rmem->base, &rmem->size); +dev_err (dev,"failed to map reserved memory#%d at %pR\n" , + i +1 , &res);return -ENOMEM ; }drivers/remoteproc/ti_k3_r5_remoteproc.c +11
-17@@ -966,13 +966,10 @@ static int k3_r5_reserved_mem_init(struct k3_r5_rproc *kproc) {struct device *dev = kproc->dev;struct device_node *np =dev_of_node (dev); -struct device_node *rmem_np; -struct reserved_mem *rmem;int num_rmems;int ret, i; - num_rmems =of_property_count_elems_of_size (np,"memory-region" , -sizeof (phandle)); + num_rmems =of_reserved_mem_region_count (np);if (num_rmems <=0 ) {dev_err (dev,"device does not have reserved memory regions, ret = %d\n" , num_rmems);@@ -1003,16 +1000,13 @@ static int k3_r5_reserved_mem_init(struct k3_r5_rproc *kproc)/* use remaining reserved memory regions for static carveouts */ for (i =0 ; i < num_rmems; i++) { - rmem_np =of_parse_phandle (np,"memory-region" , i +1 ); -if (!rmem_np) -return -EINVAL ; +struct resource res; - rmem =of_reserved_mem_lookup (rmem_np); -of_node_put (rmem_np); -if (!rmem) -return -EINVAL ; + ret =of_reserved_mem_region_to_resource (np, i +1 , &res); +if (ret) +return ret; - kproc->rmem[i].bus_addr = rmem->base; + kproc->rmem[i].bus_addr = res.start;/* * R5Fs do not have an MMU, but have a Region Address Translator * (RAT) module that provides a fixed entry translation between @@ -1023,12 +1017,12 @@ static int k3_r5_reserved_mem_init(struct k3_r5_rproc *kproc)* addresses/supported memory regions are restricted to 32-bit * bus addresses, and are identical */ - kproc->rmem[i].dev_addr = (u32 )rmem->base; - kproc->rmem[i].size = rmem->size; - kproc->rmem[i].cpu_addr =devm_ioremap_wc (dev, rmem->base, rmem->size); + kproc->rmem[i].dev_addr = (u32 )res.start; + kproc->rmem[i].size =resource_size (&res); + kproc->rmem[i].cpu_addr =devm_ioremap_resource_wc (dev, &res);if (!kproc->rmem[i].cpu_addr) { -dev_err (dev,"failed to map reserved memory#%d at %pa of size %pa\n" , - i +1 , &rmem->base, &rmem->size); +dev_err (dev,"failed to map reserved memory#%d at %pR\n" , + i +1 , &res);return -ENOMEM ; }drivers/remoteproc/xlnx_r5_remoteproc.c +17
-33@@ -460,49 +460,42 @@ static int add_mem_regions_carveout(struct rproc *rproc) {struct rproc_mem_entry *rproc_mem;struct zynqmp_r5_core *r5_core; -struct of_phandle_iterator it; -struct reserved_mem *rmem;int i =0 ; r5_core = rproc->priv;/* Register associated reserved memory regions */ -of_phandle_iterator_init (&it, r5_core->np,"memory-region" ,NULL ,0 ); +while (1 ) { +int err; +struct resource res; -while (of_phandle_iterator_next (&it) ==0 ) { - rmem =of_reserved_mem_lookup (it.node); -if (!rmem) { -of_node_put (it.node); -dev_err (&rproc->dev,"unable to acquire memory-region\n" ); -return -EINVAL ; - } + err =of_reserved_mem_region_to_resource (r5_core->np, i++, &res); +if (err) +return 0 ; -if (!strcmp (it.node->name,"vdev0buffer" )) { +if (!strcmp (res.name,"vdev0buffer" )) {/* Init reserved memory for vdev buffer */ rproc_mem =rproc_of_resm_mem_entry_init (&rproc->dev, i, - rmem->size, - rmem->base, - it.node->name); +resource_size (&res), + res.start, + res.name); }else {/* Register associated reserved memory regions */ rproc_mem =rproc_mem_entry_init (&rproc->dev,NULL , - (dma_addr_t )rmem->base, - rmem->size, rmem->base, + (dma_addr_t )res.start, +resource_size (&res), res.start, zynqmp_r5_mem_region_map, zynqmp_r5_mem_region_unmap, - it.node->name); + res.name); } -if (!rproc_mem) { -of_node_put (it.node); +if (!rproc_mem)return -ENOMEM ; - }rproc_add_carveout (rproc, rproc_mem); -rproc_coredump_add_segment (rproc, rmem->base, rmem->size); +rproc_coredump_add_segment (rproc, res.start,resource_size (&res)); -dev_dbg (&rproc->dev,"reserved mem carveout %s addr=%llx, size=0x%llx" , - it.node->name, rmem->base, rmem->size); +dev_dbg (&rproc->dev,"reserved mem carveout %pR\n" , &res); i++; }@@ -776,7 +769,6 @@ static int zynqmp_r5_get_rsc_table_va(struct zynqmp_r5_core *r5_core)struct device *dev = r5_core->dev;struct rsc_tbl_data *rsc_data_va;struct resource res_mem; -struct device_node *np;int ret;/* @@ -786,14 +778,7 @@ static int zynqmp_r5_get_rsc_table_va(struct zynqmp_r5_core *r5_core)* contains that data structure which holds resource table address, size * and some magic number to validate correct resource table entry. */ - np =of_parse_phandle (r5_core->np,"memory-region" ,0 ); -if (!np) { -dev_err (dev,"failed to get memory region dev node\n" ); -return -EINVAL ; - } - - ret =of_address_to_resource (np,0 , &res_mem); -of_node_put (np); + ret =of_reserved_mem_region_to_resource (r5_core->np,0 , &res_mem);if (ret) {dev_err (dev,"failed to get memory-region resource addr\n" );return -EINVAL ; --2.47.2
_______________________________________________
Linux-stm32 mailing list
[email protected]
https://st-md-mailman.stormreply.com/mailman/listinfo/linux-stm32
Rob Herring
maintainerRe: [Linux-stm32] [PATCH 3/3] remoteproc: Use of_reserved_mem_region_* functions for "memory-region"
Quoted 19 lines · Arnaud POULIQUEN
Hello Rob,
Quoted 15 lines · Rob Herring (Arm)
Use the newly added
of_reserved_mem_region_to_resource()andof_reserved_mem_region_count()functions to handle "memory-region" properties.The error handling is a bit different in some cases. Often "memory-region" is optional, so failed lookup is not an error. But then an error in
of_reserved_mem_lookup()is treated as an error. However, that distinction is not really important. Either the region is available and usable or it is not. So now, it is justof_reserved_mem_region_to_resource()which is checked for an error.Signed-off-by Rob Herring (Arm) <[email protected]>---
For v6.16
[...]
Quoted 41 lines
Quoted 41 lines
drivers/remoteproc/stm32_rproc.c +10
-15@@ -213,52 +213,46 @@ static int stm32_rproc_prepare(struct rproc *rproc) {struct device *dev = rproc->dev.parent;struct device_node *np = dev->of_node; -struct of_phandle_iterator it;struct rproc_mem_entry *mem; -struct reserved_mem *rmem;u64 da; -int index =0 ; +int index =0 , mr =0 ;/* Register associated reserved memory regions */ -of_phandle_iterator_init (&it, np,"memory-region" ,NULL ,0 ); -while (of_phandle_iterator_next (&it) ==0 ) { - rmem =of_reserved_mem_lookup (it.node); -if (!rmem) { -of_node_put (it.node); -dev_err (dev,"unable to acquire memory-region\n" ); -return -EINVAL ; - } +while (1 ) { +struct resource res; +int ret; + + ret =of_reserved_mem_region_to_resource (np, mr++, &res); +if (ret) +return 0 ; -if (stm32_rproc_pa_to_da (rproc, rmem->base, &da) <0 ) { -of_node_put (it.node); -dev_err (dev,"memory region not valid %pa\n" , - &rmem->base); +if (stm32_rproc_pa_to_da (rproc, res.start, &da) <0 ) { +dev_err (dev,"memory region not valid %pR\n" , &res);return -EINVAL ; }
Quoted 5 lines
drivers/remoteproc/stm32_rproc.c static int stm32_rproc_prepare(struct rproc *rproc) · line 235
Quoted 3 lines
drivers/remoteproc/stm32_rproc.c +1
-1/* No need to map vdev buffer */ -if (strcmp (it.node->name,"vdev0buffer" )) { +if (strcmp (res.name,"vdev0buffer" )) {I tested your patches
Thank you.
Quoted 5 lines
The update introduces a regression here. The strcmp function never returns 0. Indeed,
it.node->namestores the memory region label "vdev0buffer," while res.name stores the memory region name "vdev0buffer@10042000."Several remoteproc drivers may face the same issue as they embed similar code.
Indeed. I confused myself because node 'name' is without the unit-address, but this is using the full name. I've replaced the strcmp's with strstarts() to address this. I've updated my branch with the changes.
Rob
Re: [Linux-stm32] [PATCH 3/3] remoteproc: Use of_reserved_mem_region_* functions for "memory-region"
Quoted 77 lines · Rob Herring
Quoted 19 lines · Arnaud POULIQUEN
Hello Rob,
Quoted 15 lines · Rob Herring (Arm)
Use the newly added
of_reserved_mem_region_to_resource()andof_reserved_mem_region_count()functions to handle "memory-region" properties.The error handling is a bit different in some cases. Often "memory-region" is optional, so failed lookup is not an error. But then an error in
of_reserved_mem_lookup()is treated as an error. However, that distinction is not really important. Either the region is available and usable or it is not. So now, it is justof_reserved_mem_region_to_resource()which is checked for an error.Signed-off-by Rob Herring (Arm) <[email protected]>---
For v6.16[...]
Quoted 41 lines
Quoted 41 lines
drivers/remoteproc/stm32_rproc.c +10
-15@@ -213,52 +213,46 @@ static int stm32_rproc_prepare(struct rproc *rproc) {struct device *dev = rproc->dev.parent;struct device_node *np = dev->of_node; -struct of_phandle_iterator it;struct rproc_mem_entry *mem; -struct reserved_mem *rmem;u64 da; -int index =0 ; +int index =0 , mr =0 ;/* Register associated reserved memory regions */ -of_phandle_iterator_init (&it, np,"memory-region" ,NULL ,0 ); -while (of_phandle_iterator_next (&it) ==0 ) { - rmem =of_reserved_mem_lookup (it.node); -if (!rmem) { -of_node_put (it.node); -dev_err (dev,"unable to acquire memory-region\n" ); -return -EINVAL ; - } +while (1 ) { +struct resource res; +int ret; + + ret =of_reserved_mem_region_to_resource (np, mr++, &res); +if (ret) +return 0 ; -if (stm32_rproc_pa_to_da (rproc, rmem->base, &da) <0 ) { -of_node_put (it.node); -dev_err (dev,"memory region not valid %pa\n" , - &rmem->base); +if (stm32_rproc_pa_to_da (rproc, res.start, &da) <0 ) { +dev_err (dev,"memory region not valid %pR\n" , &res);return -EINVAL ; }Quoted 5 lines
drivers/remoteproc/stm32_rproc.c static int stm32_rproc_prepare(struct rproc *rproc) · line 235
Quoted 3 lines
drivers/remoteproc/stm32_rproc.c +1
-1/* No need to map vdev buffer */ -if (strcmp (it.node->name,"vdev0buffer" )) { +if (strcmp (res.name,"vdev0buffer" )) {I tested your patches
Thank you.
Quoted 4 lines
The update introduces a regression here. The strcmp function never returns 0. Indeed,
it.node->namestores the memory region label "vdev0buffer," while res.name stores the memory region name "vdev0buffer@10042000."
Quoted 6 lines
Quoted 1 line
Several remoteproc drivers may face the same issue as they embed similar code.
Indeed. I confused myself because node 'name' is without the unit-address, but this is using the full name. I've replaced the strcmp's with
strstarts()to address this. I've updated my branch with the changes.
This is not enough as the remoteproc core function rproc_find_carveout_by_name() also compares the memory names. With the following additional fix, it is working on my STM32MP15-DK board.
@@ -309,11 +309,11 @@ rproc_find_carveout_by_name(struct rproc *rproc, const char *name, ...)
vsnprintf(_name, sizeof(_name), name, args); va_end(args);
list_for_each_entry(carveout, &rproc->carveouts, node) {
/* Compare carveout and requested names */
- if (!strcmp(carveout->name, _name)) {
+ if (strstarts(carveout->name, _name)) {
mem = carveout;
break;
}
}
I just wonder if would not be more suitable to address this using the "memory-region-names" field.
The drawback is that we would break compatibility with legacy boards...
I let Mathieu and Bjorn review and comment
Else with the fix in rproc_find_carveout_by_name(),
-for the stm32_rproc:
- for the st_remoteproc
Thanks,
Arnaud
Quoted 2 lines
Rob
Re: [Linux-stm32] [PATCH 3/3] remoteproc: Use of_reserved_mem_region_* functions for "memory-region"
Quoted 105 lines · Arnaud POULIQUEN
Quoted 77 lines · Rob Herring
Quoted 19 lines · Arnaud POULIQUEN
Hello Rob,
Quoted 15 lines · Rob Herring (Arm)
Use the newly added
of_reserved_mem_region_to_resource()andof_reserved_mem_region_count()functions to handle "memory-region" properties.The error handling is a bit different in some cases. Often "memory-region" is optional, so failed lookup is not an error. But then an error in
of_reserved_mem_lookup()is treated as an error. However, that distinction is not really important. Either the region is available and usable or it is not. So now, it is justof_reserved_mem_region_to_resource()which is checked for an error.Signed-off-by Rob Herring (Arm) <[email protected]>---
For v6.16[...]
Quoted 41 lines
Quoted 41 lines
drivers/remoteproc/stm32_rproc.c +10
-15@@ -213,52 +213,46 @@ static int stm32_rproc_prepare(struct rproc *rproc) {struct device *dev = rproc->dev.parent;struct device_node *np = dev->of_node; -struct of_phandle_iterator it;struct rproc_mem_entry *mem; -struct reserved_mem *rmem;u64 da; -int index =0 ; +int index =0 , mr =0 ;/* Register associated reserved memory regions */ -of_phandle_iterator_init (&it, np,"memory-region" ,NULL ,0 ); -while (of_phandle_iterator_next (&it) ==0 ) { - rmem =of_reserved_mem_lookup (it.node); -if (!rmem) { -of_node_put (it.node); -dev_err (dev,"unable to acquire memory-region\n" ); -return -EINVAL ; - } +while (1 ) { +struct resource res; +int ret; + + ret =of_reserved_mem_region_to_resource (np, mr++, &res); +if (ret) +return 0 ; -if (stm32_rproc_pa_to_da (rproc, rmem->base, &da) <0 ) { -of_node_put (it.node); -dev_err (dev,"memory region not valid %pa\n" , - &rmem->base); +if (stm32_rproc_pa_to_da (rproc, res.start, &da) <0 ) { +dev_err (dev,"memory region not valid %pR\n" , &res);return -EINVAL ; }Quoted 5 lines
drivers/remoteproc/stm32_rproc.c static int stm32_rproc_prepare(struct rproc *rproc) · line 235
Quoted 3 lines
drivers/remoteproc/stm32_rproc.c +1
-1/* No need to map vdev buffer */ -if (strcmp (it.node->name,"vdev0buffer" )) { +if (strcmp (res.name,"vdev0buffer" )) {I tested your patches
Thank you.
Quoted 4 lines
The update introduces a regression here. The strcmp function never returns 0. Indeed,
it.node->namestores the memory region label "vdev0buffer," while res.name stores the memory region name "vdev0buffer@10042000."Quoted 6 lines
Quoted 1 line
Several remoteproc drivers may face the same issue as they embed similar code.
Indeed. I confused myself because node 'name' is without the unit-address, but this is using the full name. I've replaced the strcmp's with
strstarts()to address this. I've updated my branch with the changes.This is not enough as the remoteproc core function
rproc_find_carveout_by_name()also compares the memory names. With the following additional fix, it is working on my STM32MP15-DK board.(quoted diff) +1
-1@@ -309,11 +309,11 @@ rproc_find_carveout_by_name(struct rproc *rproc, const char *name, ...) vsnprintf(_name, sizeof(_name), name, args); va_end(args); list_for_each_entry(carveout, &rproc->carveouts, node) { /* Compare carveout and requested names */ - if (!strcmp(carveout->name, _name)) { + if (strstarts(carveout->name, _name)) { mem = carveout; break; } }
Quoted 4 lines
I just wonder if would not be more suitable to address this using the "memory-region-names" field.
The drawback is that we would break compatibility with legacy boards...
Errata:
The drawback is that we would break compatibility with legacy DTs...
Quoted 23 lines
I let Mathieu and Bjorn review and comment
Else with the fix in
rproc_find_carveout_by_name(),-for the
stm32_rproc:Reviewed-by Arnaud Pouliquen <[email protected]>Tested-by Arnaud Pouliquen <[email protected]>- for the
st_remoteprocReviewed-by Arnaud Pouliquen <[email protected]>Thanks,
ArnaudQuoted 2 lines
Rob
_______________________________________________
Linux-stm32 mailing list
[email protected]
https://st-md-mailman.stormreply.com/mailman/listinfo/linux-stm32
Rob Herring
maintainerRe: [Linux-stm32] [PATCH 3/3] remoteproc: Use of_reserved_mem_region_* functions for "memory-region"
In reply to Arnaud POULIQUENQuoted 106 lines · Arnaud POULIQUEN
Quoted 77 lines · Rob Herring
Quoted 19 lines · Arnaud POULIQUEN
Hello Rob,
Quoted 15 lines · Rob Herring (Arm)
Use the newly added
of_reserved_mem_region_to_resource()andof_reserved_mem_region_count()functions to handle "memory-region" properties.The error handling is a bit different in some cases. Often "memory-region" is optional, so failed lookup is not an error. But then an error in
of_reserved_mem_lookup()is treated as an error. However, that distinction is not really important. Either the region is available and usable or it is not. So now, it is justof_reserved_mem_region_to_resource()which is checked for an error.Signed-off-by Rob Herring (Arm) <[email protected]>---
For v6.16[...]
Quoted 41 lines
Quoted 41 lines
drivers/remoteproc/stm32_rproc.c +10
-15@@ -213,52 +213,46 @@ static int stm32_rproc_prepare(struct rproc *rproc) {struct device *dev = rproc->dev.parent;struct device_node *np = dev->of_node; -struct of_phandle_iterator it;struct rproc_mem_entry *mem; -struct reserved_mem *rmem;u64 da; -int index =0 ; +int index =0 , mr =0 ;/* Register associated reserved memory regions */ -of_phandle_iterator_init (&it, np,"memory-region" ,NULL ,0 ); -while (of_phandle_iterator_next (&it) ==0 ) { - rmem =of_reserved_mem_lookup (it.node); -if (!rmem) { -of_node_put (it.node); -dev_err (dev,"unable to acquire memory-region\n" ); -return -EINVAL ; - } +while (1 ) { +struct resource res; +int ret; + + ret =of_reserved_mem_region_to_resource (np, mr++, &res); +if (ret) +return 0 ; -if (stm32_rproc_pa_to_da (rproc, rmem->base, &da) <0 ) { -of_node_put (it.node); -dev_err (dev,"memory region not valid %pa\n" , - &rmem->base); +if (stm32_rproc_pa_to_da (rproc, res.start, &da) <0 ) { +dev_err (dev,"memory region not valid %pR\n" , &res);return -EINVAL ; }Quoted 5 lines
drivers/remoteproc/stm32_rproc.c static int stm32_rproc_prepare(struct rproc *rproc) · line 235
Quoted 3 lines
drivers/remoteproc/stm32_rproc.c +1
-1/* No need to map vdev buffer */ -if (strcmp (it.node->name,"vdev0buffer" )) { +if (strcmp (res.name,"vdev0buffer" )) {I tested your patches
Thank you.
Quoted 4 lines
The update introduces a regression here. The strcmp function never returns 0. Indeed,
it.node->namestores the memory region label "vdev0buffer," while res.name stores the memory region name "vdev0buffer@10042000."Quoted 6 lines
Quoted 1 line
Several remoteproc drivers may face the same issue as they embed similar code.
Indeed. I confused myself because node 'name' is without the unit-address, but this is using the full name. I've replaced the strcmp's with
strstarts()to address this. I've updated my branch with the changes.This is not enough as the remoteproc core function
rproc_find_carveout_by_name()also compares the memory names. With the following additional fix, it is working on my STM32MP15-DK board.(quoted diff) +1
-1@@ -309,11 +309,11 @@ rproc_find_carveout_by_name(struct rproc *rproc, const char *name, ...) vsnprintf(_name, sizeof(_name), name, args); va_end(args); list_for_each_entry(carveout, &rproc->carveouts, node) { /* Compare carveout and requested names */ - if (!strcmp(carveout->name, _name)) { + if (strstarts(carveout->name, _name)) { mem = carveout; break; } }
Quoted 2 lines
I just wonder if would not be more suitable to address this using the "memory-region-names" field.
That would be better as you shouldn't really care what a provider node name is where-as "memory-region-names" is meaningful to the driver.
Quoted 2 lines
The drawback is that we would break compatibility with legacy boards...
So not an option.
I think I'll have to fix this within the reserved mem code storing the name or do something like the diff below. I'd like to avoid the former. Using the original device_node.name is also problematic because I want to get rid of it. We redundantly store the node name with and without the unit-address. There's a lot of places like this one where we hand out the pointer with no lifetime.
drivers/remoteproc/stm32_rproc.c +1 -1
@@ -239,7 +239,7 @@ static int stm32_rproc_prepare(struct rproc *rproc)resource_size (&res), da, stm32_rproc_mem_alloc, stm32_rproc_mem_release, - res.name); + "%.*s",
strchrnul(res.name, '@') - res.name, res.name);
if (mem)
rproc_coredump_add_segment(rproc, da,
@@ -249,7 +249,7 @@ static int stm32_rproc_prepare(struct rproc *rproc)
mem = rproc_of_resm_mem_entry_init(dev, index,
resource_size(&res), res.start,
- res.name);
+ "vdev0buffer");
}
if (!mem) {Re: [Linux-stm32] [PATCH 3/3] remoteproc: Use of_reserved_mem_region_* functions for "memory-region"
Quoted 142 lines · Rob Herring
Quoted 106 lines · Arnaud POULIQUEN
Quoted 77 lines · Rob Herring
Quoted 19 lines · Arnaud POULIQUEN
Hello Rob,
Quoted 15 lines · Rob Herring (Arm)
Use the newly added
of_reserved_mem_region_to_resource()andof_reserved_mem_region_count()functions to handle "memory-region" properties.The error handling is a bit different in some cases. Often "memory-region" is optional, so failed lookup is not an error. But then an error in
of_reserved_mem_lookup()is treated as an error. However, that distinction is not really important. Either the region is available and usable or it is not. So now, it is justof_reserved_mem_region_to_resource()which is checked for an error.Signed-off-by Rob Herring (Arm) <[email protected]>---
For v6.16[...]
Quoted 41 lines
Quoted 41 lines
drivers/remoteproc/stm32_rproc.c +10
-15@@ -213,52 +213,46 @@ static int stm32_rproc_prepare(struct rproc *rproc) {struct device *dev = rproc->dev.parent;struct device_node *np = dev->of_node; -struct of_phandle_iterator it;struct rproc_mem_entry *mem; -struct reserved_mem *rmem;u64 da; -int index =0 ; +int index =0 , mr =0 ;/* Register associated reserved memory regions */ -of_phandle_iterator_init (&it, np,"memory-region" ,NULL ,0 ); -while (of_phandle_iterator_next (&it) ==0 ) { - rmem =of_reserved_mem_lookup (it.node); -if (!rmem) { -of_node_put (it.node); -dev_err (dev,"unable to acquire memory-region\n" ); -return -EINVAL ; - } +while (1 ) { +struct resource res; +int ret; + + ret =of_reserved_mem_region_to_resource (np, mr++, &res); +if (ret) +return 0 ; -if (stm32_rproc_pa_to_da (rproc, rmem->base, &da) <0 ) { -of_node_put (it.node); -dev_err (dev,"memory region not valid %pa\n" , - &rmem->base); +if (stm32_rproc_pa_to_da (rproc, res.start, &da) <0 ) { +dev_err (dev,"memory region not valid %pR\n" , &res);return -EINVAL ; }Quoted 5 lines
drivers/remoteproc/stm32_rproc.c static int stm32_rproc_prepare(struct rproc *rproc) · line 235
Quoted 3 lines
drivers/remoteproc/stm32_rproc.c +1
-1/* No need to map vdev buffer */ -if (strcmp (it.node->name,"vdev0buffer" )) { +if (strcmp (res.name,"vdev0buffer" )) {I tested your patches
Thank you.
Quoted 4 lines
The update introduces a regression here. The strcmp function never returns 0. Indeed,
it.node->namestores the memory region label "vdev0buffer," while res.name stores the memory region name "vdev0buffer@10042000."Quoted 6 lines
Quoted 1 line
Several remoteproc drivers may face the same issue as they embed similar code.
Indeed. I confused myself because node 'name' is without the unit-address, but this is using the full name. I've replaced the strcmp's with
strstarts()to address this. I've updated my branch with the changes.This is not enough as the remoteproc core function
rproc_find_carveout_by_name()also compares the memory names. With the following additional fix, it is working on my STM32MP15-DK board.(quoted diff) +1
-1@@ -309,11 +309,11 @@ rproc_find_carveout_by_name(struct rproc *rproc, const char *name, ...) vsnprintf(_name, sizeof(_name), name, args); va_end(args); list_for_each_entry(carveout, &rproc->carveouts, node) { /* Compare carveout and requested names */ - if (!strcmp(carveout->name, _name)) { + if (strstarts(carveout->name, _name)) { mem = carveout; break; } }Quoted 2 lines
I just wonder if would not be more suitable to address this using the "memory-region-names" field.
That would be better as you shouldn't really care what a provider node name is where-as "memory-region-names" is meaningful to the driver.
Quoted 2 lines
The drawback is that we would break compatibility with legacy boards...
So not an option.
I think I'll have to fix this within the reserved mem code storing the name or do something like the diff below. I'd like to avoid the former. Using the original device_node.name is also problematic because I want to get rid of it. We redundantly store the node name with and without the unit-address. There's a lot of places like this one where we hand out the pointer with no lifetime.
drivers/remoteproc/stm32_rproc.c +1
-1@@ -239,7 +239,7 @@ static int stm32_rproc_prepare(struct rproc *rproc)resource_size (&res), da, stm32_rproc_mem_alloc, stm32_rproc_mem_release, - res.name); +"%.*s" , strchrnul(res.name, '@') - res.name, res.name);if (mem)rproc_coredump_add_segment(rproc, da,
drivers/remoteproc/stm32_rproc.c static int stm32_rproc_prepare(struct rproc *rproc) · line 255
Quoted 8 lines
drivers/remoteproc/stm32_rproc.c +1
-1@@ -249,7 +249,7 @@ static int stm32_rproc_prepare(struct rproc *rproc) mem =rproc_of_resm_mem_entry_init (dev, index,resource_size (&res), res.start, - res.name); +"vdev0buffer" ); }if (!mem) {
That's work on my side.
Could we have an OF helper to retrieve the name from the full name?
Thanks,
Arnaud
Rob Herring
maintainerRe: [Linux-stm32] [PATCH 3/3] remoteproc: Use of_reserved_mem_region_* functions for "memory-region"
Quoted 153 lines · Arnaud POULIQUEN
Quoted 149 lines · Rob Herring
Quoted 106 lines · Arnaud POULIQUEN
Quoted 77 lines · Rob Herring
Quoted 19 lines · Arnaud POULIQUEN
Hello Rob,
Quoted 15 lines · Rob Herring (Arm)
Use the newly added
of_reserved_mem_region_to_resource()andof_reserved_mem_region_count()functions to handle "memory-region" properties.The error handling is a bit different in some cases. Often "memory-region" is optional, so failed lookup is not an error. But then an error in
of_reserved_mem_lookup()is treated as an error. However, that distinction is not really important. Either the region is available and usable or it is not. So now, it is justof_reserved_mem_region_to_resource()which is checked for an error.Signed-off-by Rob Herring (Arm) <[email protected]>---
For v6.16[...]
Quoted 41 lines
Quoted 41 lines
drivers/remoteproc/stm32_rproc.c +10
-15@@ -213,52 +213,46 @@ static int stm32_rproc_prepare(struct rproc *rproc) {struct device *dev = rproc->dev.parent;struct device_node *np = dev->of_node; -struct of_phandle_iterator it;struct rproc_mem_entry *mem; -struct reserved_mem *rmem;u64 da; -int index =0 ; +int index =0 , mr =0 ;/* Register associated reserved memory regions */ -of_phandle_iterator_init (&it, np,"memory-region" ,NULL ,0 ); -while (of_phandle_iterator_next (&it) ==0 ) { - rmem =of_reserved_mem_lookup (it.node); -if (!rmem) { -of_node_put (it.node); -dev_err (dev,"unable to acquire memory-region\n" ); -return -EINVAL ; - } +while (1 ) { +struct resource res; +int ret; + + ret =of_reserved_mem_region_to_resource (np, mr++, &res); +if (ret) +return 0 ; -if (stm32_rproc_pa_to_da (rproc, rmem->base, &da) <0 ) { -of_node_put (it.node); -dev_err (dev,"memory region not valid %pa\n" , - &rmem->base); +if (stm32_rproc_pa_to_da (rproc, res.start, &da) <0 ) { +dev_err (dev,"memory region not valid %pR\n" , &res);return -EINVAL ; }Quoted 5 lines
drivers/remoteproc/stm32_rproc.c static int stm32_rproc_prepare(struct rproc *rproc) · line 235
Quoted 3 lines
drivers/remoteproc/stm32_rproc.c +1
-1/* No need to map vdev buffer */ -if (strcmp (it.node->name,"vdev0buffer" )) { +if (strcmp (res.name,"vdev0buffer" )) {I tested your patches
Thank you.
Quoted 4 lines
The update introduces a regression here. The strcmp function never returns 0. Indeed,
it.node->namestores the memory region label "vdev0buffer," while res.name stores the memory region name "vdev0buffer@10042000."Quoted 6 lines
Quoted 1 line
Several remoteproc drivers may face the same issue as they embed similar code.
Indeed. I confused myself because node 'name' is without the unit-address, but this is using the full name. I've replaced the strcmp's with
strstarts()to address this. I've updated my branch with the changes.This is not enough as the remoteproc core function
rproc_find_carveout_by_name()also compares the memory names. With the following additional fix, it is working on my STM32MP15-DK board.(quoted diff) +1
-1@@ -309,11 +309,11 @@ rproc_find_carveout_by_name(struct rproc *rproc, const char *name, ...) vsnprintf(_name, sizeof(_name), name, args); va_end(args); list_for_each_entry(carveout, &rproc->carveouts, node) { /* Compare carveout and requested names */ - if (!strcmp(carveout->name, _name)) { + if (strstarts(carveout->name, _name)) { mem = carveout; break; } }Quoted 2 lines
I just wonder if would not be more suitable to address this using the "memory-region-names" field.
That would be better as you shouldn't really care what a provider node name is where-as "memory-region-names" is meaningful to the driver.
Quoted 2 lines
The drawback is that we would break compatibility with legacy boards...
So not an option.
I think I'll have to fix this within the reserved mem code storing the name or do something like the diff below. I'd like to avoid the former. Using the original device_node.name is also problematic because I want to get rid of it. We redundantly store the node name with and without the unit-address. There's a lot of places like this one where we hand out the pointer with no lifetime.
drivers/remoteproc/stm32_rproc.c +1
-1@@ -239,7 +239,7 @@ static int stm32_rproc_prepare(struct rproc *rproc)resource_size (&res), da, stm32_rproc_mem_alloc, stm32_rproc_mem_release, - res.name); +"%.*s" , strchrnul(res.name, '@') - res.name, res.name);if (mem)rproc_coredump_add_segment(rproc, da,
drivers/remoteproc/stm32_rproc.c +1
-1@@ -249,7 +249,7 @@ static int stm32_rproc_prepare(struct rproc *rproc) mem =rproc_of_resm_mem_entry_init (dev, index,resource_size (&res), res.start, - res.name); +"vdev0buffer" ); }
Quoted 5 lines
drivers/remoteproc/stm32_rproc.c static int stm32_rproc_prepare(struct rproc *rproc) · line 255
Quoted 1 line
drivers/remoteproc/stm32_rproc.c +0
-0if (!mem) {That's work on my side.
Could we have an OF helper to retrieve the name from the full name?
That would be: sprintf(buf, "%pOFn", node);
The problem here is we don't have the device_node pointer. The only way I see to make the above prettier is perhaps a define.
Rob