[OpenWrt-Devel] [PATCH 3/3] ath79: Extend GL.iNet AR750S support to NAND file system

[Jeff Kletsky]

On 5/15/19 2:00 AM, Petr Štetiar wrote:

> Jeff Kletsky <lede at allycomm.com> [2019-05-14 15:39:56]:

[...]

A question and an answer as I wrap up the punch-down list on this

>> +comma_to_underscore() {
>> +	echo "${1%%,*}_${1#*,}"
>> +}
>> +
>> [...]
> I think, that it would be better to add support for DT compatible based
> board_name format directly into nand_do_platform_check, so it could be reused
> by other platforms as well.

I originally had applied this in `nand_do_platform_check()`,
but when I checked for potential adverse impact on other boards
I found that there was an unfortunate exception with the
pistachio marduck board, which passes `img,pistachio-marduk`
to nand_do_platform_check(), preventing a "blind" change
in that function[1]. "img" is apparently from "imgtec.com".

While `target/linux/pistachio/image/Makefile` uses

     IMAGE/sysupgrade.tar := sysupgrade-tar

it also defines a custom board name

     define Device/marduk
       BOARD_NAME := img,pistachio-marduk
       DEVICE_DTS := img/pistachio_marduk

apparently resulting in the comma being in the tar directory

     sysupgrade-img,pistachio-marduk/
     sysupgrade-img,pistachio-marduk/root
     sysupgrade-img,pistachio-marduk/CONTROL
     sysupgrade-img,pistachio-marduk/kernel

While this likely could be fixed by unifying the board name
and tar directory name with current convention, I'm hesitant
to change a device build that I can't test, especially around
flashing and upgrade.


As a side note, the pistachio platform is a "snowflake"
in other ways, including what looks like a custom SPI-NAND
framework originally on Linux 4.9. `git log -1 52c17bff3c`


The remainder of the boards that I found that call `nand_do_platform_check()`
do so with a comma-free board_name or with an explicit, comma-free string.
Surprisingly few make this check; ath79, imx6, ar71xx, layerscape



For clarity, orthogonality, and maintainability, I was considering
handling this with two, distinct commits:

(1) Add the "automatic" first-comma-to-underscore transformation to
     package/base-files/files/lib/upgrade/nand.sh:nand_do_platform_check()

(2) Add a specific exception to (1) for `img,pistachio-marduk`


Is this a reasonable approach?





[...]

>> diff --git a/target/linux/ath79/image/nand.mk b/target/linux/ath79/image/nand.mk
>> index e69de29bb2..7db5f51c98 100644
>> --- a/target/linux/ath79/image/nand.mk
>> +++ b/target/linux/ath79/image/nand.mk
>> @@ -0,0 +1,15 @@
>> +define Device/glinet_gl-ar750s-nand
>> +  ATH_SOC := qca9563
>> +  DEVICE_TITLE := GL.iNet GL-AR750S
>> +  DEVICE_PACKAGES := kmod-usb2 kmod-ath10k-ct ath10k-firmware-qca9887-ct kmod-i2c-gpio
>> +  KERNEL_SIZE := 2048k
>> +  BLOCKSIZE := 128k
>> +  PAGESIZE := 2048
>> +  VID_HDR_OFFSET := 2048
>> +  IMAGES += factory.img
>> +  IMAGE/sysupgrade.bin := sysupgrade-tar | append-metadata
>> +  IMAGE/factory.img := append-kernel | pad-to $$$$(KERNEL_SIZE) | append-ubi
>> +  SUPPORTED_DEVICES += gl-ar750s
> I'm really not sure about this. Do we've enough checks in place, that we won't
> allow sysupgrade from NOR?
>

The ath79 builds, prior to this proposed patch set, use `glinet,gl-ar750s`,
so are not impacted. After this patch set there will be a `-nor` and `-nand`
suffix to clearly differentiate between the two variants.

The `gl-ar750s` board name comes from ar71xx builds for this board,
either from OpenWrt or GL.iNet (based on 18.06.1) source[2].

GL.iNet only provides the NAND-based variant through their download site[3],
from what I can tell. GL.iNet and their U-Boot differentiate NOR-based
with a .bin extension, NAND-based with a .img or .tar extension.

 From target/linux/ar71xx/base-files/lib/upgrade/platform.sh

     331                 [ "$magic" != "2705" ] && {
     332                         echo "Invalid image type."
     333                         return 1
     334                 }
     335
     336                 return 0
     337                 ;;

"2705" is keying off U-Boot's

     #define IH_MAGIC	0x27051956	/* Image Magic Number		*/

GL.iNet provides similar checks for their boards in their source
including detection if `gl_board_is_nand()` [4].

I have confirmed that this works as expected on OpenWrt snapshots
as well as from a NOR-based build from the GL.iNet sources.



Surprisingly, there is no first-order image check for the ath79 platform.
This would be a project unto itself, based on looking at the ar71xx code.




Jeff


---

[1] http://lists.infradead.org/pipermail/openwrt-devel/2019-May/017045.html

[2] https://github.com/gl-inet/openwrt
  
[3] https://dl.gl-inet.com/firmware/ar750s/

[4] https://github.com/gl-inet/openwrt/blob/develop/target/linux/ar71xx/base-files/lib/upgrade/platform.sh#L215





_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel