| Message ID | 20240226000259.2752893-12-sergey.kambalin@auriga.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Raspberry Pi 4B machine | expand |
On 26/2/24 01:02, Sergey Kambalin wrote: > This commit adds RPi4B device tree modifications: > - disable pcie, rng200, thermal sensor and genet devices > (they're going to be re-enabled in the following commits) > - create additional memory region in device tree > if RAM amount exceeds VC base address. This patch really looks like a respin of https://lore.kernel.org/qemu-devel/20201018205551.1537927-4-f4bug@amsat.org/ Keeping previous author S-o-b or adding a "based on work ..." is usually welcomed. > Signed-off-by: Sergey Kambalin <sergey.kambalin@auriga.com> > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/arm/raspi.c | 5 +-- > hw/arm/raspi4b.c | 62 +++++++++++++++++++++++++++++++++ > include/hw/arm/raspi_platform.h | 4 +++ > 3 files changed, 67 insertions(+), 4 deletions(-) > > diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c > index 8b1a046912..a7a662f40d 100644 > --- a/hw/arm/raspi.c > +++ b/hw/arm/raspi.c > @@ -37,9 +37,6 @@ OBJECT_DECLARE_SIMPLE_TYPE(RaspiMachineState, RASPI_MACHINE) > #define FIRMWARE_ADDR_3 0x80000 /* Pi 3 loads kernel.img here by default */ > #define SPINTABLE_ADDR 0xd8 /* Pi 3 bootloader spintable */ > > -/* Registered machine type (matches RPi Foundation bootloader and U-Boot) */ > -#define MACH_TYPE_BCM2708 3138 > - > struct RaspiMachineState { > /*< private >*/ > RaspiBaseMachineState parent_obj; > @@ -75,7 +72,7 @@ static const struct { > [PROCESSOR_ID_BCM2838] = {TYPE_BCM2838, BCM283X_NCPUS}, > }; > > -static uint64_t board_ram_size(uint32_t board_rev) > +uint64_t board_ram_size(uint32_t board_rev) > { > assert(FIELD_EX32(board_rev, REV_CODE, STYLE)); /* Only new style */ > return 256 * MiB << FIELD_EX32(board_rev, REV_CODE, MEMORY_SIZE); > diff --git a/hw/arm/raspi4b.c b/hw/arm/raspi4b.c > index 36a4593928..49dec6e53a 100644 > --- a/hw/arm/raspi4b.c > +++ b/hw/arm/raspi4b.c > @@ -21,6 +21,7 @@ > #include "hw/arm/boot.h" > #include "qom/object.h" > #include "hw/arm/bcm2838.h" > +#include <libfdt.h> > > #define TYPE_RASPI4B_MACHINE MACHINE_TYPE_NAME("raspi4b-2g") > OBJECT_DECLARE_SIMPLE_TYPE(Raspi4bMachineState, RASPI4B_MACHINE) > @@ -30,6 +31,66 @@ struct Raspi4bMachineState { > BCM2838State soc; > }; > > +/* > + * Add second memory region if board RAM amount exceeds VC base address > + * (see https://datasheets.raspberrypi.com/bcm2711/bcm2711-peripherals.pdf > + * 1.2 Address Map) > + */ > +static int raspi_add_memory_node(void *fdt, hwaddr mem_base, hwaddr mem_len) > +{ > + int ret; > + uint32_t acells, scells; > + char *nodename = g_strdup_printf("/memory@%" PRIx64, mem_base); > + > + acells = qemu_fdt_getprop_cell(fdt, "/", "#address-cells", > + NULL, &error_fatal); > + scells = qemu_fdt_getprop_cell(fdt, "/", "#size-cells", > + NULL, &error_fatal); > + if (acells == 0 || scells == 0) { > + fprintf(stderr, "dtb file invalid (#address-cells or #size-cells 0)\n"); > + ret = -1; > + } else { > + qemu_fdt_add_subnode(fdt, nodename); > + qemu_fdt_setprop_string(fdt, nodename, "device_type", "memory"); > + ret = qemu_fdt_setprop_sized_cells(fdt, nodename, "reg", > + acells, mem_base, > + scells, mem_len); > + } > + > + g_free(nodename); > + return ret; > +} > + > +static void raspi4_modify_dtb(const struct arm_boot_info *info, void *fdt) > +{ > + uint64_t ram_size; > + > + /* Temporarily disable following devices until they are implemented */ > + const char *nodes_to_remove[] = { > + "brcm,bcm2711-pcie", > + "brcm,bcm2711-rng200", > + "brcm,bcm2711-thermal", > + "brcm,bcm2711-genet-v5", > + }; > + > + for (int i = 0; i < ARRAY_SIZE(nodes_to_remove); i++) { > + const char *dev_str = nodes_to_remove[i]; > + > + int offset = fdt_node_offset_by_compatible(fdt, -1, dev_str); > + if (offset >= 0) { > + if (!fdt_nop_node(fdt, offset)) { Peter, I remember a discussion where you wre not keen on altering DTB for non-Virt machines. Since these devices are all implemented at the end of the series, why not add the devices then the raspi4 board at the end, so this patch is not even required? Regards, Phil. > + warn_report("bcm2711 dtc: %s has been disabled!", dev_str); > + } > + } > + } > + > + ram_size = board_ram_size(info->board_id); > + > + if (info->ram_size > UPPER_RAM_BASE) { > + raspi_add_memory_node(fdt, UPPER_RAM_BASE, ram_size - UPPER_RAM_BASE); > + } > +}
On Mon, 26 Feb 2024 at 13:35, Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > > On 26/2/24 01:02, Sergey Kambalin wrote: > > +static void raspi4_modify_dtb(const struct arm_boot_info *info, void *fdt) > > +{ > > + uint64_t ram_size; > > + > > + /* Temporarily disable following devices until they are implemented */ > > + const char *nodes_to_remove[] = { > > + "brcm,bcm2711-pcie", > > + "brcm,bcm2711-rng200", > > + "brcm,bcm2711-thermal", > > + "brcm,bcm2711-genet-v5", > > + }; > > + > > + for (int i = 0; i < ARRAY_SIZE(nodes_to_remove); i++) { > > + const char *dev_str = nodes_to_remove[i]; > > + > > + int offset = fdt_node_offset_by_compatible(fdt, -1, dev_str); > > + if (offset >= 0) { > > + if (!fdt_nop_node(fdt, offset)) { > > Peter, I remember a discussion where you wre not keen on altering DTB > for non-Virt machines. > > Since these devices are all implemented at the end of the series, why > not add the devices then the raspi4 board at the end, so this patch is > not even required? I'm not super-keen on it, but as you say it goes away once all the devices are implemented, so I'm not too worried. Doing it this way around would let us take the first 11 patches in the series into git now (they've all been reviewed), which gives us (I think) a functional raspi4 with some missing devices, which seems useful in the interim until the rest of the series gets reviewed and committed. -- PMM
On 26/2/24 14:39, Peter Maydell wrote: > On Mon, 26 Feb 2024 at 13:35, Philippe Mathieu-Daudé <philmd@linaro.org> wrote: >> >> On 26/2/24 01:02, Sergey Kambalin wrote: >>> +static void raspi4_modify_dtb(const struct arm_boot_info *info, void *fdt) >>> +{ >>> + uint64_t ram_size; >>> + >>> + /* Temporarily disable following devices until they are implemented */ >>> + const char *nodes_to_remove[] = { >>> + "brcm,bcm2711-pcie", >>> + "brcm,bcm2711-rng200", >>> + "brcm,bcm2711-thermal", >>> + "brcm,bcm2711-genet-v5", >>> + }; >>> + >>> + for (int i = 0; i < ARRAY_SIZE(nodes_to_remove); i++) { >>> + const char *dev_str = nodes_to_remove[i]; >>> + >>> + int offset = fdt_node_offset_by_compatible(fdt, -1, dev_str); >>> + if (offset >= 0) { >>> + if (!fdt_nop_node(fdt, offset)) { >> >> Peter, I remember a discussion where you wre not keen on altering DTB >> for non-Virt machines. >> >> Since these devices are all implemented at the end of the series, why >> not add the devices then the raspi4 board at the end, so this patch is >> not even required? > > I'm not super-keen on it, but as you say it goes away once all > the devices are implemented, so I'm not too worried. > > Doing it this way around would let us take the first 11 patches > in the series into git now (they've all been reviewed), which > gives us (I think) a functional raspi4 with some missing devices, > which seems useful in the interim until the rest of the series > gets reviewed and committed. Fine by me! Sergey, don't we also need patch #39 (Add missed BCM2835 properties) to have a happy Linux boot? Patch #17 "Implement BCM2838 thermal sensor" could also go in but it doesn't apply cleanly on top of 1-12); maybe Sergey can send a series of "patches already reviewed" on top so they get in for v9, postponing pcie/network for after release. Regards, Phil.
On Mon, 26 Feb 2024 at 16:06, Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > > On 26/2/24 14:39, Peter Maydell wrote: > > On Mon, 26 Feb 2024 at 13:35, Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > >> > >> On 26/2/24 01:02, Sergey Kambalin wrote: > >>> +static void raspi4_modify_dtb(const struct arm_boot_info *info, void *fdt) > >>> +{ > >>> + uint64_t ram_size; > >>> + > >>> + /* Temporarily disable following devices until they are implemented */ > >>> + const char *nodes_to_remove[] = { > >>> + "brcm,bcm2711-pcie", > >>> + "brcm,bcm2711-rng200", > >>> + "brcm,bcm2711-thermal", > >>> + "brcm,bcm2711-genet-v5", > >>> + }; > >>> + > >>> + for (int i = 0; i < ARRAY_SIZE(nodes_to_remove); i++) { > >>> + const char *dev_str = nodes_to_remove[i]; > >>> + > >>> + int offset = fdt_node_offset_by_compatible(fdt, -1, dev_str); > >>> + if (offset >= 0) { > >>> + if (!fdt_nop_node(fdt, offset)) { > >> > >> Peter, I remember a discussion where you wre not keen on altering DTB > >> for non-Virt machines. > >> > >> Since these devices are all implemented at the end of the series, why > >> not add the devices then the raspi4 board at the end, so this patch is > >> not even required? > > > > I'm not super-keen on it, but as you say it goes away once all > > the devices are implemented, so I'm not too worried. > > > > Doing it this way around would let us take the first 11 patches > > in the series into git now (they've all been reviewed), which > > gives us (I think) a functional raspi4 with some missing devices, > > which seems useful in the interim until the rest of the series > > gets reviewed and committed. > > Fine by me! Sergey, don't we also need patch #39 (Add missed BCM2835 > properties) to have a happy Linux boot? > > Patch #17 "Implement BCM2838 thermal sensor" could also go in but it > doesn't apply cleanly on top of 1-12); maybe Sergey can send a series > of "patches already reviewed" on top so they get in for v9, postponing > pcie/network for after release. I'll put together a pullreq tomorrow (see my other email for details of which patches plus the necessary changes to the avocado tests). Sergey -- I suggest you wait til that gets upstream, and then rebase on that. -- PMM
Hi Peter and Philippe! Thank you for the review and feedback! OK, I'll fix PCIE-relarted comments and the overlapping issue BR, Sergey Kambalin Software Developer, Auriga Inc.
diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c index 8b1a046912..a7a662f40d 100644 --- a/hw/arm/raspi.c +++ b/hw/arm/raspi.c @@ -37,9 +37,6 @@ OBJECT_DECLARE_SIMPLE_TYPE(RaspiMachineState, RASPI_MACHINE) #define FIRMWARE_ADDR_3 0x80000 /* Pi 3 loads kernel.img here by default */ #define SPINTABLE_ADDR 0xd8 /* Pi 3 bootloader spintable */ -/* Registered machine type (matches RPi Foundation bootloader and U-Boot) */ -#define MACH_TYPE_BCM2708 3138 - struct RaspiMachineState { /*< private >*/ RaspiBaseMachineState parent_obj; @@ -75,7 +72,7 @@ static const struct { [PROCESSOR_ID_BCM2838] = {TYPE_BCM2838, BCM283X_NCPUS}, }; -static uint64_t board_ram_size(uint32_t board_rev) +uint64_t board_ram_size(uint32_t board_rev) { assert(FIELD_EX32(board_rev, REV_CODE, STYLE)); /* Only new style */ return 256 * MiB << FIELD_EX32(board_rev, REV_CODE, MEMORY_SIZE); diff --git a/hw/arm/raspi4b.c b/hw/arm/raspi4b.c index 36a4593928..49dec6e53a 100644 --- a/hw/arm/raspi4b.c +++ b/hw/arm/raspi4b.c @@ -21,6 +21,7 @@ #include "hw/arm/boot.h" #include "qom/object.h" #include "hw/arm/bcm2838.h" +#include <libfdt.h> #define TYPE_RASPI4B_MACHINE MACHINE_TYPE_NAME("raspi4b-2g") OBJECT_DECLARE_SIMPLE_TYPE(Raspi4bMachineState, RASPI4B_MACHINE) @@ -30,6 +31,66 @@ struct Raspi4bMachineState { BCM2838State soc; }; +/* + * Add second memory region if board RAM amount exceeds VC base address + * (see https://datasheets.raspberrypi.com/bcm2711/bcm2711-peripherals.pdf + * 1.2 Address Map) + */ +static int raspi_add_memory_node(void *fdt, hwaddr mem_base, hwaddr mem_len) +{ + int ret; + uint32_t acells, scells; + char *nodename = g_strdup_printf("/memory@%" PRIx64, mem_base); + + acells = qemu_fdt_getprop_cell(fdt, "/", "#address-cells", + NULL, &error_fatal); + scells = qemu_fdt_getprop_cell(fdt, "/", "#size-cells", + NULL, &error_fatal); + if (acells == 0 || scells == 0) { + fprintf(stderr, "dtb file invalid (#address-cells or #size-cells 0)\n"); + ret = -1; + } else { + qemu_fdt_add_subnode(fdt, nodename); + qemu_fdt_setprop_string(fdt, nodename, "device_type", "memory"); + ret = qemu_fdt_setprop_sized_cells(fdt, nodename, "reg", + acells, mem_base, + scells, mem_len); + } + + g_free(nodename); + return ret; +} + +static void raspi4_modify_dtb(const struct arm_boot_info *info, void *fdt) +{ + uint64_t ram_size; + + /* Temporarily disable following devices until they are implemented */ + const char *nodes_to_remove[] = { + "brcm,bcm2711-pcie", + "brcm,bcm2711-rng200", + "brcm,bcm2711-thermal", + "brcm,bcm2711-genet-v5", + }; + + for (int i = 0; i < ARRAY_SIZE(nodes_to_remove); i++) { + const char *dev_str = nodes_to_remove[i]; + + int offset = fdt_node_offset_by_compatible(fdt, -1, dev_str); + if (offset >= 0) { + if (!fdt_nop_node(fdt, offset)) { + warn_report("bcm2711 dtc: %s has been disabled!", dev_str); + } + } + } + + ram_size = board_ram_size(info->board_id); + + if (info->ram_size > UPPER_RAM_BASE) { + raspi_add_memory_node(fdt, UPPER_RAM_BASE, ram_size - UPPER_RAM_BASE); + } +} + static void raspi4b_machine_init(MachineState *machine) { Raspi4bMachineState *s = RASPI4B_MACHINE(machine); @@ -37,6 +98,7 @@ static void raspi4b_machine_init(MachineState *machine) RaspiBaseMachineClass *mc = RASPI_BASE_MACHINE_GET_CLASS(machine); BCM2838State *soc = &s->soc; + s_base->binfo.modify_dtb = raspi4_modify_dtb; s_base->binfo.board_id = mc->board_rev; object_initialize_child(OBJECT(machine), "soc", soc, diff --git a/include/hw/arm/raspi_platform.h b/include/hw/arm/raspi_platform.h index 45003e2425..0db146e592 100644 --- a/include/hw/arm/raspi_platform.h +++ b/include/hw/arm/raspi_platform.h @@ -31,6 +31,9 @@ #include "hw/boards.h" #include "hw/arm/boot.h" +/* Registered machine type (matches RPi Foundation bootloader and U-Boot) */ +#define MACH_TYPE_BCM2708 3138 + #define TYPE_RASPI_BASE_MACHINE MACHINE_TYPE_NAME("raspi-base") OBJECT_DECLARE_TYPE(RaspiBaseMachineState, RaspiBaseMachineClass, RASPI_BASE_MACHINE) @@ -59,6 +62,7 @@ void raspi_base_machine_init(MachineState *machine, void raspi_machine_class_common_init(MachineClass *mc, uint32_t board_rev); +uint64_t board_ram_size(uint32_t board_rev); #define MSYNC_OFFSET 0x0000 /* Multicore Sync Block */ #define CCPT_OFFSET 0x1000 /* Compact Camera Port 2 TX */