On Mon, 2024-08-05 at 10:31 +0100, Julien Grall wrote:
Hi Oleksii,
On 24/07/2024 16:31, Oleksii Kurochko wrote:
From: Shawn Anastasio <sanasta...@raptorengineering.com>
Move Arm's bootfdt.c to xen/common so that it can be used by other
device tree architectures like PPC and RISCV.
Suggested-by: Julien Grall <jul...@xen.org>
Signed-off-by: Shawn Anastasio <sanasta...@raptorengineering.com>
Acked-by: Julien Grall <jul...@xen.org>
On Matrix you asked me to review this patch again. This wasn't
obvious
given you kept my ack. If you think a review is needed, then please
either drop the ack or explain why you keep it and ask if it is fine.
Also, I tend to list in the changes where this was acked. In this
case,
you said I acked v4.
Anyway, before confirming my ack, I would like to ask some
clarification.
Signed-off-by: Oleksii Kurochko <oleksii.kuroc...@gmail.com>
---
Changes in V7:
- Nothing changed. Only rebase.
---
Changes in V6:
- update the version of the patch to v6.
---
Changes in V5:
- add xen/include/xen/bootfdt.h to MAINTAINERS file.
I don't see any change in MAINTAINERS within this patch. Did you
happen
to copy/paste all the changes made in the series?
This change should be mentioned in this patch. It is part of the
previous patch (
https://lore.kernel.org/xen-devel/102f8b60c55cdf2db5890b9fb1c2fb66e61c4a67.1721834549.git.oleksii.kuroc...@gmail.com/
)
In fact the only change related to this patch doesn't seem to be
listed.
[...]
+#ifndef CONFIG_STATIC_SHM
+static inline int process_shm_node(const void *fdt, int node,
+ uint32_t address_cells,
uint32_t size_cells)
+{
+ printk("CONFIG_STATIC_SHM must be enabled for parsing static
shared"
+ " memory nodes\n");
+ return -EINVAL;
+}
+#endif
I see you duplicated the stub from arch/arm/include/static-shmem.h.
But
the one in static-shmem.h will now be unreachable. I think it needs
to
be removed.
Overlooked that. Originally I added that to make Xen RISC-V and PPC
build happy as early_scan_node() code uses process_shm_node():
static int __init early_scan_node(const void *fdt,
int node, const char *name, int
depth,
u32 address_cells, u32 size_cells,
void *data)
{
...
else if ( depth == 2 && device_tree_node_compatible(fdt, node,
"xen,domain") )
rc = process_domain_node(fdt, node, name, address_cells,
size_cells);
else if ( depth <= 3 && device_tree_node_compatible(fdt, node,
"xen,domain-shared-memory-v1") )
rc = process_shm_node(fdt, node, address_cells, size_cells);
if ( rc < 0 )
printk("fdt: node `%s': parsing failed\n", name);
return rc;
}
Instead of introducing stub for process_shm_node() when
CONFIG_STATIC_SHM I think it would be better to add "#ifdef
CONFIG_STATIC_SHM" to early_scan_node():
static int __init early_scan_node(const void *fdt,
int node, const char *name, int
depth,
u32 address_cells, u32 size_cells,
void *data)
{
...
else if ( depth == 2 && device_tree_node_compatible(fdt, node,
"xen,domain") )
rc = process_domain_node(fdt, node, name, address_cells,
size_cells);
#ifdef CONFIG_STATIC_SHM
else if ( depth <= 3 && device_tree_node_compatible(fdt, node,
"xen,domain-shared-memory-v1") )
rc = process_shm_node(fdt, node, address_cells, size_cells);
#endif