Thanks Daniel a lot for your help and the time you took to review my patch! :)
I think some aspects are still unclear to me, so I am asking here to get a
better understanding.
On Mon, Nov 04, 2024 at 10:32:27PM +0000, Daniel Golle wrote:
> Hi Enrico,
>
> the patch generally looks very good.
> Few things:
> - It would be better to use NVMEM in device tree to assign MAC
> addresses.
> - You could use 'fitblk' in order to have the bootloader validate the
> whole image before booting and prevent bootloops in case of corrupted
> squashfs (eg. due to loss of power during sysupgrade).
> - Maybe split this patch into three, so it becomes easier to read:
> 1/3 Add device to U-Boot
> 2/3 Add device to uboot-envtools
> 3/3 Add device to mediatek target
>
> See comments inline below illustrating the first two points.
>
> On Sun, Nov 03, 2024 at 05:30:34PM +0100, Enrico Mioso wrote:
> > [...]
> > --- /dev/null
> > +++ b/target/linux/mediatek/dts/mt7981b-gatonetworks-gdsp.dts
> > @@ -0,0 +1,377 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> > +
> > +/dts-v1/;
> > +#include "mt7981.dtsi"
> > +
> > +/ {
> > + model = "GatoNetworks GDSP";
> > + compatible = "gatonetworks,gdsp", "mediatek,mt7981";
> > +
> > + aliases {
> > + serial0 = &uart0;
> > + label-mac-device = &wifi;
> > + led-boot = &sg1;
> > + led-failsafe = &sg1;
> > + led-running = &sg1;
> > + led-upgrade = &sg1;
> > + };
> > +
> > + chosen {
> > + stdout-path = "serial0:115200n8";
> > + bootargs = "console=ttyS0,115200n1
> > earlycon=uart8250,mmio32,0x11002000";
>
> rootdisk = <&firmware>;
>
> > + };
> > [...]
> > + partition@180000 {
>
> firmware: partition@180000 {
... applied and testing now the FIT related changes. And very happy about the
advantages it brings!
However, I will probably need to stay compatible with mtk "standard" bootloader
- do you think I will break compatibility by this change?
In case I do break it, may i ask for some guidance on how to produce images
like the default one I inherited from mediatek target and produced with this
patch?
>
> > + label = "firmware";
> > + reg = <0x180000 0x1E80000>;
> > + };
> > [...]
>
> > + gatonetworks,gdsp)
> > + local factory_dev="/dev/mtd$(find_mtd_index "Factory")"
> > + label_mac=$(get_mac_binary $factory_dev 0x4)
>
> Why not use NVMEM and mac-base to read and assign the MAC address?
I am actually doing it in the devicetree and it seems to work properly. Or am I
missing something?
I just wanted the label mac property to show up in the board.json, - is it
needed or may I leave it off?
I think I need to understand the deep meaning of this in the OpenWrt system.
>
> > [...]
> > ---
> > a/target/linux/mediatek/filogic/base-files/etc/hotplug.d/ieee80211/11_fix_wifi_mac
> > +++
> > b/target/linux/mediatek/filogic/base-files/etc/hotplug.d/ieee80211/11_fix_wifi_mac
> > @@ -89,6 +89,11 @@ case "$board" in
> > addr=$(mtd_get_mac_binary "Odm" 0x81)
> > [ "$PHYNBR" = "1" ] && macaddr_add $addr 3 >
> > /sys${DEVPATH}/macaddress
> > ;;
> > + gatonetworks,gdsp)
> > + addr=$(get_mac_label)
> > + [ "$PHYNBR" = "0" ] && echo "$addr" > /sys${DEVPATH}/macaddress
> > + [ "$PHYNBR" = "1" ] && macaddr_setbit_la $(macaddr_add $addr 1)
> > > /sys${DEVPATH}/macaddress
>
> Same here, use NVMEM and mac-base if possible.
Done, I will do what the "stock" firmware does here: where label mac is 2.4 GHZ
Wi-Fi, and 5 GHZ Wi-Fi has mac + 1.
I was wrongly assigning the same MAC to all of the two interfaces and took the
wrong decision here. Fixing it.
Howeve, what is not clear to me, is: what about the "locally-administered" (la)
bit ? Do I need to set it here or not? In that case I guess I can not do so via
devicetree. For the moment being, I simply removed this part, relying only on
devictree to increment the MAC adress.
>
> > --- a/target/linux/mediatek/filogic/base-files/lib/upgrade/platform.sh
> > +++ b/target/linux/mediatek/filogic/base-files/lib/upgrade/platform.sh
> > @@ -111,6 +111,7 @@ platform_do_upgrade() {
> > ;;
> > cudy,re3000-v1|\
> > cudy,wr3000-v1|\
> > + gatonetworks,gdsp|\
>
> Move the device into group of devices uses fitblk.
Done, thanks :)
>
> > [...]
> > +define Device/gatonetworks_gdsp
> > + DEVICE_VENDOR := GatoNetworks
> > + DEVICE_MODEL := gdsp
> > + DEVICE_DTS := mt7981b-gatonetworks-gdsp
> > + DEVICE_DTS_DIR := ../dts
> > + IMAGES := sysupgrade.bin
> > + IMAGE_SIZE := 32768k
> > + DEVICE_PACKAGES := kmod-mt7915e kmod-mt7981-firmware mt7981-wo-firmware \
> > + kmod-usb-net-qmi-wwan uqmi kmod-usb3 kmod-usb-serial-option \
> > + -kmod-phy-aquantia
>
> Add 'fitblk' package.
> Set KERNEL and KERNEL_INITRAMFS like BPi-R3.
>
After fixing up things, I will also split my patch.
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
