| Message ID | 20190913191147.2323-1-stewart.hildebrand@dornerworks.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/arm: platform: additional Raspberry Pi compatible string | expand |
Hi, On 9/13/19 8:11 PM, Stewart Hildebrand wrote: > Upstream Linux kernel will use "brcm,bcm2711" as the compatible string > for Raspberry Pi 4 [1]. Add this string to our platform compatible list > for compatibility with the upstream kernel. This raises a few questions: 1) Why such discrepancies in naming? 2) Is the patch [1] merged? If so, which version? 3) Both upstream and non-upstream seem to have the compatible "raspberrypi,4-model-b", so would it make sense to check that instead? > > [1] https://patchwork.kernel.org/patch/11092621/ > > Signed-off-by: Stewart Hildebrand <stewart.hildebrand@dornerworks.com> > --- > xen/arch/arm/platforms/brcm-raspberry-pi.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/xen/arch/arm/platforms/brcm-raspberry-pi.c b/xen/arch/arm/platforms/brcm-raspberry-pi.c > index e22d2b3184..a95a3db83f 100644 > --- a/xen/arch/arm/platforms/brcm-raspberry-pi.c > +++ b/xen/arch/arm/platforms/brcm-raspberry-pi.c > @@ -21,6 +21,7 @@ > > static const char *const brcm_bcm2838_dt_compat[] __initconst = > { > + "brcm,bcm2711", If a new compatible is added, then you likely need to rename the different structure within this file. > "brcm,bcm2838", > NULL > }; > Cheers,
On Friday, September 13, 2019 5:42 PM, Julien Grall <julien.grall@arm.com> wrote: >Hi, > >On 9/13/19 8:11 PM, Stewart Hildebrand wrote: >> Upstream Linux kernel will use "brcm,bcm2711" as the compatible string >> for Raspberry Pi 4 [1]. Add this string to our platform compatible list >> for compatibility with the upstream kernel. > >This raises a few questions: > 1) Why such discrepancies in naming? Traditionally the raspberry pi tree has used the bcm2708/9/10 naming convention, and upstream bcm2835/6/7. It seems they've switched it up for the RPi4. In the RFC version of the series intended for upstream, they indeed had it as 2838 [2], but after that it changed to 2711 [3] [4]. The SoC name in documentation is BCM2711 [5] [6]. > 2) Is the patch [1] merged? If so, which version? No. > 3) Both upstream and non-upstream seem to have the compatible >"raspberrypi,4-model-b", so would it make sense to check that instead? "raspberrypi,4-model-b" describes a board, and "brcm,bcm2xxx" describes a SoC. It is feasible that there will be more boards based on this SoC, in which case we'd have to add those hypothetical new boards separately. If we list both "brcm,bcm2711" and "brcm,bcm2838", then it seems to me we'd have a better chance at matching future boards with this particular SoC, while also matching both upstream and non-upstream trees. >> static const char *const brcm_bcm2838_dt_compat[] __initconst = >> { >> + "brcm,bcm2711", > >If a new compatible is added, then you likely need to rename the >different structure within this file. Good point. Since the documentation uses the BCM2711 convention, it would make sense to me to rename it reflecting this, even when the list contains both brcm,bcm2711 and brcm,bcm2838. And perhaps also add a comment by the brcm,bcm2838 entry to explain the oddity. static const char *const brcm_bcm2711_dt_compat[] __initconst = { "brcm,bcm2711", /* * While the name of the SoC is BCM2711, some variants of Linux * have also used the brcm,bcm2838 naming convention. We consider * either compatible string to be a valid match for the BCM2711 SoC. */ "brcm,bcm2838", The bcm2838 naming convention statement ironically refers to the raspberry pi tree [7]. [2] RFC https://patchwork.kernel.org/cover/11048215/#22767107 [3] v1 https://patchwork.kernel.org/cover/11051653/ [4] v2 https://patchwork.kernel.org/cover/11092641/ [5] https://www.raspberrypi.org/documentation/hardware/raspberrypi/bcm2711/README.md [6] https://www.raspberrypi.org/documentation/hardware/raspberrypi/bcm2711/rpi_DATA_2711_1p0_preliminary.pdf [7] https://github.com/raspberrypi/linux/blob/rpi-4.19.y/arch/arm/boot/dts/bcm2711-rpi-4-b.dts#L8
Hi Stewart, On 9/14/19 2:22 AM, Stewart Hildebrand wrote: > On Friday, September 13, 2019 5:42 PM, Julien Grall <julien.grall@arm.com> wrote: >> Hi, >> >> On 9/13/19 8:11 PM, Stewart Hildebrand wrote: >>> Upstream Linux kernel will use "brcm,bcm2711" as the compatible string >>> for Raspberry Pi 4 [1]. Add this string to our platform compatible list >>> for compatibility with the upstream kernel. >> >> This raises a few questions: >> 1) Why such discrepancies in naming? > > Traditionally the raspberry pi tree has used the bcm2708/9/10 naming convention, and upstream bcm2835/6/7. It seems they've switched it up for the RPi4. In the RFC version of the series intended for upstream, they indeed had it as 2838 [2], but after that it changed to 2711 [3] [4]. > > The SoC name in documentation is BCM2711 [5] [6]. > >> 2) Is the patch [1] merged? If so, which version? > > No. I would rather wait until the patch is merged in Linux before adding the compatible. > >> 3) Both upstream and non-upstream seem to have the compatible >> "raspberrypi,4-model-b", so would it make sense to check that instead? > > "raspberrypi,4-model-b" describes a board, and "brcm,bcm2xxx" describes a SoC. It is feasible that there will be more boards based on this SoC, in which case we'd have to add those hypothetical new boards separately. If we list both "brcm,bcm2711" and "brcm,bcm2838", then it seems to me we'd have a better chance at matching future boards with this particular SoC, while also matching both upstream and non-upstream trees. To be honest, if I knew the compatible would be different between upstream and non-upstream, I would have NAcked the previous patch. A Device-Tree is meant to describe the platform in an agnostic-kernel way. In the ideal world, the device-tree is part shipped with the firmware. So here, you would have to use a different device-tree depending on whether you are using upstream or non-upstream kernel. From Xen PoV, we should really only used stable bindings (i.e accepted by the kernel community). Anything non-stable are at risk to change in the future. Anyway, the RPI community should definitely sort out this mess... This also raise the question on what's going to happen for blacklist device? Are they still going to contain "bcm2835"? > >>> static const char *const brcm_bcm2838_dt_compat[] __initconst = >>> { >>> + "brcm,bcm2711", >> >> If a new compatible is added, then you likely need to rename the >> different structure within this file. > > Good point. Since the documentation uses the BCM2711 convention, it would make sense to me to rename it reflecting this, even when the list contains both brcm,bcm2711 and brcm,bcm2838. And perhaps also add a comment by the brcm,bcm2838 entry to explain the oddity. I would prefer rpi4_dt_compat as this file is only meant to cover rpi4. The comment explaining why brcm,brcm2838 is present is good as it hints this is not an upstream thing. > > static const char *const brcm_bcm2711_dt_compat[] __initconst = > { > "brcm,bcm2711", > /* > * While the name of the SoC is BCM2711, some variants of Linux > * have also used the brcm,bcm2838 naming convention. We consider > * either compatible string to be a valid match for the BCM2711 SoC. > */ > "brcm,bcm2838", > > The bcm2838 naming convention statement ironically refers to the raspberry pi tree [7]. > > [2] RFC https://patchwork.kernel.org/cover/11048215/#22767107 > [3] v1 https://patchwork.kernel.org/cover/11051653/ > [4] v2 https://patchwork.kernel.org/cover/11092641/ > [5] https://www.raspberrypi.org/documentation/hardware/raspberrypi/bcm2711/README.md > [6] https://www.raspberrypi.org/documentation/hardware/raspberrypi/bcm2711/rpi_DATA_2711_1p0_preliminary.pdf > [7] https://github.com/raspberrypi/linux/blob/rpi-4.19.y/arch/arm/boot/dts/bcm2711-rpi-4-b.dts#L8 > Cheers,
On Tuesday, September 17, 2019 7:05 AM, Julien Grall <julien.grall@arm.com> wrote: >Hi Stewart, > >On 9/14/19 2:22 AM, Stewart Hildebrand wrote: >> On Friday, September 13, 2019 5:42 PM, Julien Grall <julien.grall@arm.com> wrote: >>> 2) Is the patch [1] merged? If so, which version? >> >> No. > >I would rather wait until the patch is merged in Linux before adding the >compatible. The downstream kernel has changed their compatible to "brcm,bcm2711" [8], so "brcm,bcm2838" is obsolete now. While the upstream series has not been merged yet [9], the lack of "brcm,bcm2711" in our compatible list is preventing us from booting the downstream kernel. I will send a v2 of the patch. >This also raise the question on what's going to happen for blacklist >device? Are they still going to contain "bcm2835"? The peripherals/devices still have the same "brcm,bcm2835" prefix. No change. [8] https://github.com/raspberrypi/linux/commit/53fdd7b8c8cb9c87190caab4fd459f89e1b4a7f8 [9] v3 https://patchwork.kernel.org/cover/11165395/
diff --git a/xen/arch/arm/platforms/brcm-raspberry-pi.c b/xen/arch/arm/platforms/brcm-raspberry-pi.c index e22d2b3184..a95a3db83f 100644 --- a/xen/arch/arm/platforms/brcm-raspberry-pi.c +++ b/xen/arch/arm/platforms/brcm-raspberry-pi.c @@ -21,6 +21,7 @@ static const char *const brcm_bcm2838_dt_compat[] __initconst = { + "brcm,bcm2711", "brcm,bcm2838", NULL };
Upstream Linux kernel will use "brcm,bcm2711" as the compatible string for Raspberry Pi 4 [1]. Add this string to our platform compatible list for compatibility with the upstream kernel. [1] https://patchwork.kernel.org/patch/11092621/ Signed-off-by: Stewart Hildebrand <stewart.hildebrand@dornerworks.com> --- xen/arch/arm/platforms/brcm-raspberry-pi.c | 1 + 1 file changed, 1 insertion(+)