ath79: add Embedded Wireless Balin Platform

[Christian Lamparter]

Hello,

I have some comments below.

On 30/11/2021 17:06, Catrinel Catrinescu wrote:
> Add the Embedded Wireless "Balin" platform, it is in ar71xx too
>   SoC: QCA AR9344 or AR9350
>   RAM: DDR2-RAM 64MBytes
>   Flash: SPI-NOR 16MBytes
>   WLAN: 2 x 2 MIMO 2.4 & 5 GHz IEEE802.11 a/b/g/n
>   Ethernet: 3 x 10/100 Mb/s
>   USB: 1 x USB2.0 Host/Device bootstrap-pin at power-up
>   PCI-Express: 1 x lane PCIe 1.2
>   UART: 1 x Normal, 1 x High-Speed
>   JTAG: 1 x EJTAG
>   GPIO: 10 x Input/Output multiplexed
> 
> The module comes already with the current vanilla OpenWrt firmware.
> To update, use "sysupgrade" image directly in vendor firmware.

I'm guessing here. But the vanilla OpenWrt firmware that your device
ships with is still based on the ar71xx target?

If so, could you please add in your commit message something about
that "sysupgrade -n --force <image>" will be needed for the first
sysupgrade to the ath79 release? (with '-n = restores device back to
"factory settings"')

Alternatively, it is possible to do this without the flags as well.
This requires migration scripts for led, net, etc... As well as
adding the previous EW-BALIN boardname to the SUPPORTED_DEVICES
variable in the generic.mk portion of this patch. This makes it
possible to get the "sysupgrade <image>" upgrade procedure path.

>[..]
> +
> +&spi {
> +	status = "okay";
> +
> +	flash at 0 {
> +		compatible = "jedec,spi-nor";
> +		reg = <0>;
> +		spi-max-frequency = <50000000>;
> +
> +		partitions {
> +			compatible = "fixed-partitions";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +
> +			uboot: partition at 0 {
is that uboot: label used somewhere?

> +				label = "u-boot";
> +				reg = <0x000000 0x040000>;
> +				read-only;
> +			};
> +
> +			partition at 40000 {
> +				label = "u-boot-env";
> +				reg = <0x040000 0x010000>;
> +			};
> +
> +			partition at 50000 {
> +				compatible = "denx,uimage";
> +				label = "firmware";
> +				reg = <0x050000 0xfa0000>;
> +			};
> +
> +			art: partition at ff0000 {
> +				label = "art";
> +				reg = <0xff0000 0x010000>;
> +				read-only;

you can add the mac-address (+calibration) as nvmem cells right here.
There's no need to have this as a separate entity (i.e.: &art { ... })
below.

(the art: label could be redundant as well)

> +			};
> +		};
> +	};
> +};
> +/*
> +&pcie {
> +	status = "okay";
> +
> +	ath9k: wifi at 0,0 {
> +		compatible = "pci168c,0030";
> +		reg = <0x0000 0 0 0 0>;
> +		nvmem-cells = <&macaddr_art_c>;
> +		nvmem-cell-names = "mac-address";
> +		qca,no-eeprom;

hmm, this is commented out. So, the pcie will not be working without making
changes. Maybe it would be advantageous to move the comment block at least
inside the node around the ath9k part? (So the mini pcie slot works with the
provided images.) ... Or remove the comment block completely so that the
ath9k minipcie (which I assume comes with the eval board) works out of the box?

about "working out of the box":

The "qca,no-eeprom" usually goes along with a change to
target/linux/ath79/generic/base-files/etc/hotplug.d/firmware/10-ath9k-eeprom.
The initialization will fail otherwise. This is because no eeprom is provided
to the driver. But instead of adding an entry to 10-ath9k-eeprom, you can now
specify a "calibration" nvmem cell and drop the "qca,no-eeprom" property.

For an example: the WNDR3700 series and TP-Link Archer C7(v1+v2 with ath10k)
have been updated to use the nvmem-cells.

(of course, this is a bit tricky. Because this both nvmem and qca,no-eeprom
are detrimental if an ath9k minipcie card is connected that is not the one
that came with the eval board.)

> +		#gpio-cells = <2>;
> +		gpio-controller;
> +	};
> +};
> +*/
> +&builtin_switch {
> +
> +	/delete-property/qca,phy4-mii-enable;
> +};
> +
> +&eth1 {
> +	status = "okay";
> +
> +	nvmem-cells = <&macaddr_art_1002>;
> +	nvmem-cell-names = "mac-address";
> +	mac-address-increment-byte = <3>;
> +	mac-address-increment = <0x40>;
this has been converted to nvmem. Great!

> +
> +	gmac-config {
> +		device = <&gmac>;
> +		switch-phy-swap = <0>;
> +		switch-only-mode = <1>;
> +	};
> +};
> +
> +&wmac {
> +	status = "okay";
> +
> +	mtd-cal-data = <&art 0x1000>;

the ath9k ahb devices do also work with the nvmem-cells
"calibration" cells in this configuration.
	
> +};
> +

from here ...
> +&art {
> +	compatible = "nvmem-cells";
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +	macaddr_art_1002: macaddr at 1002 {
> +		reg = <0x1002 0x6>;
> +	};
to here: This all can be moved into the art node above.

Regards,
Christian