[PATCH v2 1/2] ipq40xx: add support for Luma Home WRTQ-329ACN

[Christian Lamparter]

On 2020-09-15 16:50, Tomasz Maciej Nowak wrote:
> Hi Christian, thanks for looking at this.
> 
> W dniu 13.09.2020 o 00:04, Christian Lamparter pisze:
>> On 2020-08-30 13:28, Tomasz Maciej Nowak wrote:
>>> Luma Home WRTQ-329ACN, also known as Luma WiFi System, is a dual-band
>>> wireless access point.
>>>
>>> Specification
>>> SoC: Qualcomm Atheros IPQ4018
>>> RAM: 256 MB DDR3
>>> Flash: 2 MB SPI NOR
>>>          128 MB SPI NAND
[...]

>> So, because you install from the initramfs and introduced a custom run
>> command with a custom "kernel1" ubi volume... It might be possible to
>> cut the extra parts in the sysupgrade and bootargs and unify it with
>> the generic nand_do_upgrade() crowd.
>>
>> I've incorperated what maybe could be done in this patch on my
>> staging tree: [0]
>>
>> (Note the changes in step 9 (kernel1->kernel), step 10 (ubirmvol ubi_rootfs1 & kernel1) and the "step 2 in
>> back to OEM".
>>
>> The idea is that we use OpenWrt's existing automount for ubi
>> and "rootfs" volumes.
>>
>> (In theory this could make the "back to OEM" even easier.
>> Because if it works the way I think it does, we could drop
>> the ubirmvol of ubi_rootfs1 and kernel1 in step 10 of my version.
>> Next, it should be possible to drop the ubimkvol of
>> kernel1 and ubi_rootfs1 step 2 in the "Reverting to OEM" section.
>> This is because the original rootfs1 and kernel1 still exist and
>> are left untouched - no idea if this works out or not.)
> 
> Personally I would like to use whole NAND for OpenWrt, that would simplify everything a lot.
> Unfortunately some users might be dissatisfied with how things are done in OpenWrt and wish
> to revert to stock firmware. I'll explain in points what are the current issues with this
> device. My knowledge is based on what I could dig out from firmware (QSDK) without access to
> source modifications.

I don't think there's a need to do that. On old (and new) devices that only have
8MiB/16MiB of flash space, it's impossible to keep the vendor firmware around.
There's no obligation to do that. From the looks of it, though. I think it's more
of a hassle to fully remove the vendor than to keep it around and praise this as
a feature. So either way, both are fine :-) . If somebody wants to use the full
NAND, she/he could just delete the remaining "kernel1, ubi_rootfs[1]" volumes
and resize the rootfs_data. (This can be done in place).

> 1. The devices have different flash layouts, some come with
> mtd8: 08000000 00020000 "rootfs"
> mtd9: 0041e000 0001f000 "kernel"
> mtd10: 026c0000 0001f000 "ubi_rootfs"
> mtd11: 01ff8000 0001f000 "rootfs_data"
> mtd12: 0041e000 0001f000 "kernel1"
> mtd13: 026c0000 0001f000 "ubi_rootfs1"
> 
> and some
> 
> mtd8: 08000000 00020000 "rootfs"
> mtd9: 003a2000 0001f000 "kernel"
> mtd10: 02283000 0001f000 "ubi_rootfs"
> mtd11: 0062d000 0001f000 "dummy"
> mtd12: 01ff8000 0001f000 "rootfs_data"
> mtd13: 0041e000 0001f000 "kernel1"
> mtd14: 026c0000 0001f000 "ubi_rootfs1"
> 
> this reflects the bootargs in dts.

Ok, that 128 MiB "rootfs" looks to be the "ubi" partition. My best "this could be"
guess is that the "dummy" and the different sizes of the kernel vs kernel1 partition
stems from a update. The dummy would be a temporary filesystem for the stuff that
was worth keeping... But I don't know.

I just like to see the use of the established "kernel" and "rootfs" volumes whenever
possible.

I did the necessary changes in this version:
<https://git.openwrt.org/?p=openwrt/staging/chunkeey.git;a=commit;h=b30fedc765871e7738257eae305674b0069bbd96>

This will repurpose the "kernel" partition (by overwriting it)
and ditch "ubi_rootfs". The big mtd partition "rootfs" is renamed to
"ubi" (this is because OpenWrt will auto-attach the volume - handled
by 490-ubi-auto-attach-mtd-device-named-ubi-or-data-on-boot.patch)

and the squashfs root is placed in "rootfs". (OpenWrt does
automatically create a ubiblock for volumes called rootfs -
done by 491-ubi-auto-create-ubiblock-device-for-rootfs.patch @Daniel)

What's your opinion? Do you want it in a different way? Or are you
fine with the changes? (I would need to know if this works or not
though)

> 
> 2. As illustrated above, the device comes with two sets of kernel+ubi_rootfs and shared
> overlay "rootfs_data". The U-Boot with 'bootipq' command boots the first kernel+ubi_rootfs
> pair. There is some kind of fallback mechanism to issue boot from the second pair, but I was
> unable to trigger it (didn't try hammer approach by deleting first kernel+ubi_rootfs set).
> 
> 3. On each reboot the 'bootipq' command rewrites some of U-Boot environments variables to
> default state. I don't know every one of them and that makes this not a stable environment,
> which could overwrite our changes. Also this increases flash wear (it's NOR, I know, but still).
> That's why I decided to introduce custom 'bootcmd'.
Custom "bootcmd" is fine.

> 
> 4. The stock firmware likes to fiddle with U-Boot environment from userspace. I could miss
> some case of it and users who reverted to stock firmware (which would need custom 'bootcmd'
> if we went with Your changes) could end up with OpenWrt booted after reboot (in best case)
> or in U-Boot shell. Explaining what to do, would include non-trivial tasks for average user.
> This is why I decided to write OpenWrt firmware to kernel1+ubi_rootfs1 set.
Hm? Did I make a mistake somewhere? I thought that I kept the custom bootcmd. I only changed
the "kernel1" to "kernel" and let the kernel use auto-attach the "ubi"-volume and boot from
the "rootfs" volume. (Note: sysupgrade in the initramfs also installs into "kernel"
and "rootfs". These are the defaults for CI_KERNPART and CI_ROOTPART). Did you try this with
your existing setup? (You would need to start with a newly created initramfs image to test
this)

> 
> 5. What we have stored in flash is all we've got in terms of OEM firmware. There is no
> downloadable image (I know of) we could use to flash the device with, in case no backup is
> made and all partitions are wiped.
> Given that, I decided to go for reverting to OEM setup, as close as possible, so self-heal
> implemented in stock firmware won't be confused.
I updated the "back to stock" instruction. These are from the POV of a newly bought device.
I assume that the content of ubi_rootfs1 matches ubi_rootfs, likewise "kernel1" would match
"kernel". Is this not true?

>>
>> Note2: You don't mention it in your commit. But I'm seeing that you are
>> using GPIO OPEN_DRAIN for the bitbanged i2c. So, don't you also need
>> Brain's pinctrl patch that wires up OPEN_DRAIN for this? [1]
> 
> I went with suggestion in [3], but even setting to GPIO_ACTIVE_HIGH is enough. I tested
> this with and without Brian's patch. Should I send v3 without GPIO_OPEN_DRAIN?
> 
Brain's patch is upstream I think.
The Open Drain is coming from the I2C spec itself. From what I remember it's
important for multi-master and if slaves employ Clock stretching. I would keep it,
but please test it now before it causes problems later.

Cheers,
Christian