| Message ID | 20250317232426.952188-3-robh@kernel.org (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | of: Common "memory-region" parsing | expand |
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: 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
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"); } /**
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(-)