Re: [edk2-devel] [edk2-platforms: PATCH 1/1] Platform/RPi3: Accept "ethernet" or "ethernet0" aliases in device tree
Just adding a comment for one item, that isn't directly relevant to what
direction we should take with this patch, but that clarifies where we
got our .dtb's from.
On 2019.07.23 12:00, Michael Brown wrote:
On 23/07/2019 11:34, Leif Lindholm wrote:
On Fri, Jul 19, 2019 at 06:29:07PM +0100, Michael Brown wrote:
Older device trees tend to use the alias "ethernet". Newer device
trees tend to use "ethernet0" since older versions of U-Boot would
skip aliases that do not include an index number. See, for example,
Linux kernel commit 10b6c0c ("ARM: dts: bcm2835: add index to the
ethernet alias") and U-Boot commit f8e57c6 ("fdt_support: Fixup
'ethernet' aliases not ending in digits").
Accept either "ethernet" or "ethernet0" as being the alias to the node
for which the MAC address should be updated.
Why is this patch useful?
I understand the problem, but we include the .dtb from our own
edk2-non-osi tree. And it seems that device tree already provides an
alias to support both:
https://github.com/tianocore/edk2-non-osi/blob/master/Platform/RaspberryPi/RPi3/DeviceTree/bcm2710-rpi-3-b.dts
Would it be more useful to warn loudly if "ethernet" isn't present?
(And just *replace* ethernet[0] with ethernet below.)
The device tree can also be provided by the VC4 firmware, e.g. with a
config.txt containing e.g.
device_tree_address=0x1
device_tree_end=0x2
device_tree=bcm2710-rpi-3-b-plus.dtb
It is also possible to omit the explicit "device_tree=..." and have just
device_tree_address=0x1
device_tree_end=0x2
This configuration will cause the VC4 firmware to automatically load the
appropriate device tree file. I have tested that this allows me to boot
from a 3B or 3B+ using a single SD card image that contains all of the
.dtb files from
https://github.com/raspberrypi/firmware/tree/master/boot
These "official" raspberrypi-firmware .dtb files are built from the
stock Linux kernel .dts files, and contain only "ethernet0".
Note that the .dtb's we use for this platform (in edk2-non-osi) were
actually picked up from the URL above, then decompiled to produce a .dts
which was then edited to fix a couple issues, as per:
https://github.com/tianocore/edk2-non-osi/blob/master/Platform/RaspberryPi/RPi3/DeviceTree/bcm2710-rpi-3-b-plus.dts#L6-L15
Most notably, unless you change
compatible = "brcm,bcm2708-usb";
to
compatible = "brcm,bcm2835-usb";
in the usb@7e98 section you will find your USB keyboard may not work
during a Linux install (Tested with both the Ubuntu 18.04 and Debian 10
installers).
Now, I am also aware that the Raspberry Pi Foundation has updated its
official .dtb's for the Pi 3 with the release of the Pi 4, and I have
some plans to send a patch to also update our Device Tree accordingly at
some stage. For the record, I've been testing these updated official
versions last week and confirmed that the keyboard issue was still present.
Regards,
/Pete
To address your specific questions:
> Why is this patch useful?
It allows us to boot using the "official" .dtb files without any
unexpected behaviour.
This in turn allows us to boot with the VC4 firmware automatically
selecting the appropriate .dtb file for the hardware. This is extremely
useful when trying to provide a single SD card image that will work on a
wide range of hardware.
> Would it be more useful to warn loudly if "ethernet" isn't present?
> (And just *replace* ethernet[0] with ethernet below.)
This would also solve the problem. I would find it mildly surprising
behaviour if I were debugging around it, since the "official" .dtb files
have moved in the other direction (going from "ethernet" to
"ethernet0"), but I would not expect it to break anything.
Another option would be to have UpdateMacAddress() ensure that both
"ethernet" and "ethernet0" aliases exist.
I am happy to work up a new version of the patch: just let me know which
option you prefer.
Many thanks,
Michael
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#44245): https://edk2.groups.io/g/devel/message/44245
Mute This Topic: https://groups.io/mt/32529756/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2-platforms: PATCH 1/1] Platform/RPi3: Accept "ethernet" or "ethernet0" aliases in device tree
On Tue, Jul 23, 2019 at 12:00:04PM +0100, Michael Brown wrote: > > Why is this patch useful? > > I understand the problem, but we include the .dtb from our own > > edk2-non-osi tree. And it seems that device tree already provides an > > alias to support both: > > https://github.com/tianocore/edk2-non-osi/blob/master/Platform/RaspberryPi/RPi3/DeviceTree/bcm2710-rpi-3-b.dts > > > > Would it be more useful to warn loudly if "ethernet" isn't present? > > (And just *replace* ethernet[0] with ethernet below.) > > The device tree can also be provided by the VC4 firmware, e.g. with a > config.txt containing e.g. > > device_tree_address=0x1 > device_tree_end=0x2 > device_tree=bcm2710-rpi-3-b-plus.dtb Ah, right. > It is also possible to omit the explicit "device_tree=..." and have just > > device_tree_address=0x1 > device_tree_end=0x2 > > This configuration will cause the VC4 firmware to automatically load the > appropriate device tree file. I have tested that this allows me to boot > from a 3B or 3B+ using a single SD card image that contains all of the .dtb > files from > > https://github.com/raspberrypi/firmware/tree/master/boot > > These "official" raspberrypi-firmware .dtb files are built from the stock > Linux kernel .dts files, and contain only "ethernet0". *facepalm* > To address your specific questions: > > > Why is this patch useful? > > It allows us to boot using the "official" .dtb files without any unexpected > behaviour. > > This in turn allows us to boot with the VC4 firmware automatically selecting > the appropriate .dtb file for the hardware. This is extremely useful when > trying to provide a single SD card image that will work on a wide range of > hardware. Yeah, understood. I just have a poor understanding of the esoteric boot architecture of the Pis. > > Would it be more useful to warn loudly if "ethernet" isn't present? > > (And just *replace* ethernet[0] with ethernet below.) > > This would also solve the problem. I would find it mildly surprising > behaviour if I were debugging around it, since the "official" .dtb files > have moved in the other direction (going from "ethernet" to "ethernet0"), > but I would not expect it to break anything. > > Another option would be to have UpdateMacAddress() ensure that both > "ethernet" and "ethernet0" aliases exist. Yeah, that would probably be my preferred option. > I am happy to work up a new version of the patch: just let me know which > option you prefer. As mentioned. And thank you for the explanation. Best Regards, Leif -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44244): https://edk2.groups.io/g/devel/message/44244 Mute This Topic: https://groups.io/mt/32529756/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2-platforms: PATCH 1/1] Platform/RPi3: Accept "ethernet" or "ethernet0" aliases in device tree
On 23/07/2019 11:34, Leif Lindholm wrote:
On Fri, Jul 19, 2019 at 06:29:07PM +0100, Michael Brown wrote:
Older device trees tend to use the alias "ethernet". Newer device
trees tend to use "ethernet0" since older versions of U-Boot would
skip aliases that do not include an index number. See, for example,
Linux kernel commit 10b6c0c ("ARM: dts: bcm2835: add index to the
ethernet alias") and U-Boot commit f8e57c6 ("fdt_support: Fixup
'ethernet' aliases not ending in digits").
Accept either "ethernet" or "ethernet0" as being the alias to the node
for which the MAC address should be updated.
Why is this patch useful?
I understand the problem, but we include the .dtb from our own
edk2-non-osi tree. And it seems that device tree already provides an
alias to support both:
https://github.com/tianocore/edk2-non-osi/blob/master/Platform/RaspberryPi/RPi3/DeviceTree/bcm2710-rpi-3-b.dts
Would it be more useful to warn loudly if "ethernet" isn't present?
(And just *replace* ethernet[0] with ethernet below.)
The device tree can also be provided by the VC4 firmware, e.g. with a
config.txt containing e.g.
device_tree_address=0x1
device_tree_end=0x2
device_tree=bcm2710-rpi-3-b-plus.dtb
It is also possible to omit the explicit "device_tree=..." and have just
device_tree_address=0x1
device_tree_end=0x2
This configuration will cause the VC4 firmware to automatically load the
appropriate device tree file. I have tested that this allows me to boot
from a 3B or 3B+ using a single SD card image that contains all of the
.dtb files from
https://github.com/raspberrypi/firmware/tree/master/boot
These "official" raspberrypi-firmware .dtb files are built from the
stock Linux kernel .dts files, and contain only "ethernet0".
To address your specific questions:
> Why is this patch useful?
It allows us to boot using the "official" .dtb files without any
unexpected behaviour.
This in turn allows us to boot with the VC4 firmware automatically
selecting the appropriate .dtb file for the hardware. This is extremely
useful when trying to provide a single SD card image that will work on a
wide range of hardware.
> Would it be more useful to warn loudly if "ethernet" isn't present?
> (And just *replace* ethernet[0] with ethernet below.)
This would also solve the problem. I would find it mildly surprising
behaviour if I were debugging around it, since the "official" .dtb files
have moved in the other direction (going from "ethernet" to
"ethernet0"), but I would not expect it to break anything.
Another option would be to have UpdateMacAddress() ensure that both
"ethernet" and "ethernet0" aliases exist.
I am happy to work up a new version of the patch: just let me know which
option you prefer.
Many thanks,
Michael
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#44243): https://edk2.groups.io/g/devel/message/44243
Mute This Topic: https://groups.io/mt/32529756/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2-platforms: PATCH 1/1] Platform/RPi3: Accept "ethernet" or "ethernet0" aliases in device tree
On Fri, Jul 19, 2019 at 06:29:07PM +0100, Michael Brown wrote:
> Older device trees tend to use the alias "ethernet". Newer device
> trees tend to use "ethernet0" since older versions of U-Boot would
> skip aliases that do not include an index number. See, for example,
> Linux kernel commit 10b6c0c ("ARM: dts: bcm2835: add index to the
> ethernet alias") and U-Boot commit f8e57c6 ("fdt_support: Fixup
> 'ethernet' aliases not ending in digits").
>
> Accept either "ethernet" or "ethernet0" as being the alias to the node
> for which the MAC address should be updated.
Why is this patch useful?
I understand the problem, but we include the .dtb from our own
edk2-non-osi tree. And it seems that device tree already provides an
alias to support both:
https://github.com/tianocore/edk2-non-osi/blob/master/Platform/RaspberryPi/RPi3/DeviceTree/bcm2710-rpi-3-b.dts
Would it be more useful to warn loudly if "ethernet" isn't present?
(And just *replace* ethernet[0] with ethernet below.)
Regards,
Leif
> Signed-off-by: Michael Brown
> ---
> Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c
> b/Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c
> index 83446e3e45..e80bc2c391 100644
> --- a/Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c
> +++ b/Platform/RaspberryPi/RPi3/Drivers/FdtDxe/FdtDxe.c
> @@ -35,11 +35,14 @@ UpdateMacAddress (
>UINT8 MacAddress[6];
>
>//
> - // Locate the node that the 'ethernet' alias refers to
> + // Locate the node that the 'ethernet[0]' alias refers to
>//
>Node = fdt_path_offset (mFdtImage, "ethernet");
>if (Node < 0) {
> -DEBUG ((DEBUG_ERROR, "%a: failed to locate 'ethernet' alias\n",
> __FUNCTION__));
> +Node = fdt_path_offset (mFdtImage, "ethernet0");
> + }
> + if (Node < 0) {
> +DEBUG ((DEBUG_ERROR, "%a: failed to locate 'ethernet[0]' alias\n",
> __FUNCTION__));
> return;
>}
>
> --
> 2.20.1
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#44242): https://edk2.groups.io/g/devel/message/44242
Mute This Topic: https://groups.io/mt/32529756/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
