Re: Raspberry Pi 4 support on 32 bit earmv7hf GENERIC kernel

[Nick Hudson <nick.hudson%gmx.co.uk@localhost>]

> On 21 Mar 2026, at 17:02, Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost> wrote:
> 
> 


> 
>>> diff --git a/sys/arch/arm/arm32/bus_dma.c b/sys/arch/arm/arm32/bus_dma.c
>>> index c41fb1e1a065..a6d693a4dae5 100644
>>> --- a/sys/arch/arm/arm32/bus_dma.c
>>> +++ b/sys/arch/arm/arm32/bus_dma.c
>>> @@ -160,7 +160,7 @@ _bus_dma_paddr_inrange(struct arm32_dma_range *ranges, int nranges,
>>> 
>>>   for (i = 0, dr = ranges; i < nranges; i++, dr++) {
>>>   if (pa >= dr->dr_sysbase &&
>>> -    pa < dr->dr_sysbase + dr->dr_len)
>>> +    pa - dr->dr_sysbase < dr->dr_len)
>>>   return dr;
>>>   }
>>> 
>>> @@ -180,11 +180,11 @@ _bus_dma_busaddr_to_paddr(bus_dma_tag_t t, bus_addr_t curaddr)
>>>   return curaddr;
>>> 
>>>   for (i = 0, dr = t->_ranges; i < t->_nranges; i++, dr++) {
>>> - if (dr->dr_busbase <= curaddr
>>> -    && curaddr < dr->dr_busbase + dr->dr_len)
>>> + if (curaddr >= dr->dr_busbase &&
>>> +    curaddr - dr->dr_busbase < dr->dr_len)
>>>   return curaddr - dr->dr_busbase + dr->dr_sysbase;
>>>   }
>>> - panic("%s: curaddr %#" PRIxBUSADDR "not in range", __func__, curaddr);
>>> + panic("%s: curaddr %#" PRIxBUSADDR " not in range", __func__, curaddr);
>>>  }
>>> 
>> 
>> Why do you think this is needed?
> 
> See GitHub commit logs:
> https://github.com/tsutsui/netbsd-src/commit/db488da3abb090fc0fecf89d97a55a30e1ca7ffe
> ---
> arm32/bus_dma: fix interger overflow on 32 bit kernels
> 
> On Raspberry Pi 4B with BCM2711, dr_busbase is 0xc0000000 and
> dr_len is 0x40000000, so "dr->dr_busbase + dr->dr_len" causes
> wraparound.
> 
> Fixes panic on Raspberry Pi 4B during attaching vcmbox0:
>> panic: _bus_dma_busaddr_to_paddr: curaddr 0xc5b6c000not in range

OK


> ---
> 
>>> --- a/sys/arch/arm/broadcom/bcm283x_platform.c
>>> +++ b/sys/arch/arm/broadcom/bcm283x_platform.c
>>> @@ -74,6 +74,8 @@ __KERNEL_RCSID(0, "$NetBSD: bcm283x_platform.c,v 1.54 2026/01/08 00:51:20 christ
>>> 
>>>  #include <libfdt.h>
>>> 
>>> +#include <dev/ofw/openfirm.h>
>>> +
>>>  #include <arm/broadcom/bcm2835reg.h>
>>>  #include <arm/broadcom/bcm2835var.h>
>>>  #include <arm/broadcom/bcm283x_platform.h>
>>> @@ -856,12 +858,42 @@ bcm2711_bootparams(void)
>>>  }
>>> 
>>>  #if defined(MULTIPROCESSOR)
>>> +static bool
>>> +bcm283x_is_2711(void)
>>> +{
>>> + static const struct device_compatible_entry compat_data[] = {
>>> + { .compat = "brcm,bcm2711" },
>>> + DEVICE_COMPAT_EOL
>>> + };
>>> + const int root = OF_finddevice("/");
>>> +
>>> + if (root < 0) {
>>> + return false;
>>> + }
>>> +
>>> + if (of_compatible_match(root, compat_data) > 0) {
>>> + return true;
>>> + }
>>> +
>>> + return false;
>>> +}
>>> +
>>>  static int
>>>  cpu_enable_bcm2836(int phandle)
>>>  {
>>> - bus_space_tag_t iot = &bcm2836_bs_tag;
>>> - bus_space_handle_t ioh = BCM2836_ARM_LOCAL_VBASE;
>>> + bus_space_tag_t iot;
>>> + bus_space_handle_t ioh;
>>>   uint64_t mpidr;
>>> + bool is2711;
>>> +
>>> + is2711 = bcm283x_is_2711();
>>> + if (is2711) {
>>> + iot = &bcm2711_bs_tag;
>>> + ioh = BCM2711_ARM_LOCAL_VBASE;
>>> + } else {
>>> + iot = &bcm2836_bs_tag;
>>> + ioh = BCM2836_ARM_LOCAL_VBASE;
>>> + }
>>> 
>>>   fdtbus_get_reg64(phandle, 0, &mpidr, NULL);
>>> 
>> 
>> 
>> This seems completely wrong.
>> 
>> Perhaps the DTS need patching with
>> 
>> bcm2711.dtsi:           enable-method = "brcm,bcm2836-smp"; // for ARM 32-bit
>> 
>> s/brcm,bcm2836-smp/brcm,bcm2711-smp/
> 
> I have no idea if we should patch DTS for maintainability.

Well there is a way to fix upstream DTS (it can and does have bugs) without
changing sys/external/gpl2/dts/dist

See sys/arch/arm/dts


> 
>>> diff --git a/sys/dev/fdt/fdtbus.c b/sys/dev/fdt/fdtbus.c
>>> index e557b0c6479a..fc244ef1e88a 100644
>>> --- a/sys/dev/fdt/fdtbus.c
>>> +++ b/sys/dev/fdt/fdtbus.c
>>> @@ -90,6 +90,7 @@ static void fdt_post_attach(struct fdt_node *);
>>>  static const struct device_compatible_entry compat_data[] = {
>>>   { .compat = "simple-bus" },
>>>   { .compat = "simple-pm-bus" },
>>> + { .compat = "simple_bus" },
>>>   DEVICE_COMPAT_EOL
>>>  };
>> 
>> Are you using vendor DTS? Please don't.
> 
> I'm not sure what "vendor DTS" means, but I'm using armv7.img.gz
> from NetBSD 11.0_RC2 and only the kernel is built on -current tree.
> (I have not tried -current image)

Where did you get your dtb?

~/netbsd/nbcvs/src % grep -r simple_bus sys/external/gpl2/dts/dist
~/netbsd/nbcvs/src %

> 
>>> diff --git a/sys/dev/ic/bcmgenet.c b/sys/dev/ic/bcmgenet.c
>>> index 36c0cbfe1358..0705db0e948c 100644
>>> --- a/sys/dev/ic/bcmgenet.c
>>> +++ b/sys/dev/ic/bcmgenet.c
>>> @@ -189,7 +189,7 @@ genet_setup_txdesc(struct genet_softc *sc, int index, int flags,
>>>   status = flags | __SHIFTIN(len, GENET_TX_DESC_STATUS_BUFLEN);
>>> 
>>>   WR4(sc, GENET_TX_DESC_ADDRESS_LO(index), (uint32_t)paddr);
>>> - WR4(sc, GENET_TX_DESC_ADDRESS_HI(index), (uint32_t)(paddr >> 32));
>>> + WR4(sc, GENET_TX_DESC_ADDRESS_HI(index), (uint32_t)((uint64_t)paddr >> 32));
>>>   WR4(sc, GENET_TX_DESC_STATUS(index), status);
>>>  }
>>> 
>>> @@ -259,7 +259,7 @@ genet_setup_rxdesc(struct genet_softc *sc, int index,
>>>      bus_addr_t paddr, bus_size_t len)
>>>  {
>>>   WR4(sc, GENET_RX_DESC_ADDRESS_LO(index), (uint32_t)paddr);
>>> - WR4(sc, GENET_RX_DESC_ADDRESS_HI(index), (uint32_t)(paddr >> 32));
>>> + WR4(sc, GENET_RX_DESC_ADDRESS_HI(index), (uint32_t)((uint64_t)paddr >> 32));
>>>  }
>>> 
>>>  static int
>>> 
>> 
>> 
>> Just use BUS_ADDR_{LO,HI}32
> 
> I see.
> (Note several other drivers like sys/dev/ic/dwc_eqos.c also use
> "(uint32_t)((uint64_t)paddr >> 32)" formats)

They’re equally fugly and should use BUS_ADDR_{LO,HI}32


Nick