Simplify of_dma_set_restricted_buffer() by using of_property_present()
and of_for_each_phandle() iterator.
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
drivers/of/device.c | 34 +++++++++++++---------------------
1 file changed, 13 insertions(+), 21 deletions(-)
diff --git a/drivers/of/device.c b/drivers/of/device.c
index edf3be197265..bb4a47d58249 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -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");
}
/**
--
2.47.2Hi,
On Tue, Mar 18, 2025 at 7:29 AM Rob Herring (Arm) <robh@kernel.org> wrote:
>
> Simplify of_dma_set_restricted_buffer() by using of_property_present()
> and of_for_each_phandle() iterator.
>
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> ---
> drivers/of/device.c | 34 +++++++++++++---------------------
> 1 file changed, 13 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/of/device.c b/drivers/of/device.c
> index edf3be197265..bb4a47d58249 100644
> --- a/drivers/of/device.c
> +++ b/drivers/of/device.c
> @@ -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");
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
> }
>
> /**
> --
> 2.47.2
>
>
On Wed, Mar 26, 2025 at 1:44 AM Chen-Yu Tsai <wens@kernel.org> wrote:
>
> Hi,
>
> On Tue, Mar 18, 2025 at 7:29 AM Rob Herring (Arm) <robh@kernel.org> wrote:
> >
> > Simplify of_dma_set_restricted_buffer() by using of_property_present()
> > and of_for_each_phandle() iterator.
> >
> > Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> > ---
> > drivers/of/device.c | 34 +++++++++++++---------------------
> > 1 file changed, 13 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/of/device.c b/drivers/of/device.c
> > index edf3be197265..bb4a47d58249 100644
> > --- a/drivers/of/device.c
> > +++ b/drivers/of/device.c
> > @@ -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");
>
> 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:
Thanks. I fixed it up to only warn if i is non-zero.
Rob
On Thu, Mar 27, 2025 at 2:53 AM Rob Herring <robh@kernel.org> wrote:
>
> On Wed, Mar 26, 2025 at 1:44 AM Chen-Yu Tsai <wens@kernel.org> wrote:
> >
> > Hi,
> >
> > On Tue, Mar 18, 2025 at 7:29 AM Rob Herring (Arm) <robh@kernel.org> wrote:
> > >
> > > Simplify of_dma_set_restricted_buffer() by using of_property_present()
> > > and of_for_each_phandle() iterator.
> > >
> > > Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> > > ---
> > > drivers/of/device.c | 34 +++++++++++++---------------------
> > > 1 file changed, 13 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/drivers/of/device.c b/drivers/of/device.c
> > > index edf3be197265..bb4a47d58249 100644
> > > --- a/drivers/of/device.c
> > > +++ b/drivers/of/device.c
> > > @@ -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");
> >
> > 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:
>
> 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
© 2016 - 2025 Red Hat, Inc.