1. Patchew
  2. Xen
  3. View series

[Xen-devel] [PATCH] xen/arm: platform: additional Raspberry Pi compatible string

Stewart Hildebrand posted 1 patch 4 years, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/xen tags/patchew/20190913191147.2323-1-stewart.hildebrand@dornerworks.com
xen/arch/arm/platforms/brcm-raspberry-pi.c | 1 +
1 file changed, 1 insertion(+)
[Xen-devel] [PATCH] xen/arm: platform: additional Raspberry Pi compatible string
Posted by Stewart Hildebrand 4 years, 7 months ago 
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(+)

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
 };
-- 
2.23.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: platform: additional Raspberry Pi compatible string
Posted by Julien Grall 4 years, 7 months ago 
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,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: platform: additional Raspberry Pi compatible string
Posted by Stewart Hildebrand 4 years, 7 months ago 
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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: platform: additional Raspberry Pi compatible string
Posted by Julien Grall 4 years, 7 months ago 
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,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: platform: additional Raspberry Pi compatible string
Posted by Stewart Hildebrand 4 years, 6 months ago 
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/
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.