Hi Torsten,
On 2026-09-25T10:54:32, Torsten Duwe <[email protected]> wrote:
> test: add dma-ranges UT for dynamically created nodes
>
> When devices were found using PCI(e) autoprobing, there is usually a
> connecting bus node as parent, and the device node itself, both
> created dynamically without a corresponding device tree node. If the
> device needs to do inbound PCI transfers, DM looks up the DMA mapping
> ("dma-ranges") in the parent's DT node; but in this case there is
> none. Test whether DM is able to find the property on the grandparent
> node.
>
> Signed-off-by: Torsten Duwe <[email protected]>
>
> arch/sandbox/dts/test.dts | 9 +++++++++
> test/dm/ofnode.c | 4 ++--
> test/dm/phys2bus.c | 32 ++++++++++++++++++++++++++++++++
> 3 files changed, 43 insertions(+), 2 deletions(-)
> When devices were found using PCI(e) autoprobing, there is usually a
> connecting bus node as parent, and the device node itself, both
> created dynamically without a corresponding device tree node.
I read this as your discovering something in previous testing, but I
think you mean 'When devices are found'
> diff --git a/test/dm/phys2bus.c b/test/dm/phys2bus.c
> @@ -31,6 +31,38 @@ static int dm_test_phys_to_bus(struct unit_test_state *uts)
> +#if CONFIG_IS_ENABLED(OF_REAL)
> + if (uts->of_live) {
> + /* DMA-ranges are to be found on the grandparent bus node,
> + * DT nodes neither here nor at parent.
> + */
> + struct udevice *pdev;
> + ofnode psave, nsave;
Thanks for the test. I'd rather see this as a separate DM_TEST (e.g.
dm_test_phys_to_bus_dyn) - the test name then describes what it does,
and the of_live guard becomes a single ut_skip_on() at the top instead
of wrapping the whole body. It also avoids the #if in the middle of a
function.
> diff --git a/test/dm/phys2bus.c b/test/dm/phys2bus.c
> @@ -31,6 +31,38 @@ static int dm_test_phys_to_bus(struct unit_test_state *uts)
> + /* pretend the devices were discovered dynamically and have no
> DT node */
> + psave = pdev->node_;
> + pdev->node_.np = NULL;
Please don't reach into dev->node_ directly - the struct comment says
'do not access outside driver model'. Use dev_ofnode() to save and
dev_set_ofnode(dev, ofnode_null()) to clear; that also stops the test
from silently assuming the live-tree representation of ofnode.
> diff --git a/test/dm/phys2bus.c b/test/dm/phys2bus.c
> @@ -31,6 +31,38 @@ static int dm_test_phys_to_bus(struct unit_test_state *uts)
> + /* DMA-ranges are to be found on the grandparent bus node,
> + * DT nodes neither here nor at parent.
> + */
Multi-line block comment should use kernel style with the opening /*
on its own line.
> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> @@ -664,6 +664,15 @@
> + subnode@8 {
> + compatible = "denx,u-boot-fdt-test";
> + };
Stray extra spaces before 'compatible' - the rest of the file uses a
single tab of extra indent.
Regards,
Simon