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

Tue Sep 22 12:56:16 EDT 2020

W dniu 18.09.2020 o 20:47, Christian Lamparter pisze:
> 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).

Yeah, this is one of the features making UBI so good. Anyway there's enough space
so let's keep the OEM firmware.

> 
>> 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.

Yes, I renamed it to accommodate OpenWrt features (that was before I shifted to use
kernel1+ubi_rootfs1).

> 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.

That was my guess too, except the dummy partition is almost empty, with small amount of garbage.
Seems to be a failed update, since on first (active) set of kernel+ubi_rootfs I had 1.1-16r
firmware version and on the second one 1.1-18. Reading from scripts the second set is always
updated first.

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

ACK

> 
> 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)

I'm (very much) fine with proposed solution. I like the volumes renaming which I didn't
consider previously. The instructions worked, except when restoring ubi_rootfs1 there was not
enough PEBs. I'm thinking about also removing the "dummy" volume in installation instruction,
since it's not used by OEM firmware for anything (that solved missing PEBs). Also I would
change the order of restoring volumes from:
 ubimkvol /dev/ubi0 -S 264 -N rootfs_data
 ubimkvol /dev/ubi0 -S 34 -N kernel1
 ubimkvol /dev/ubi0 -S 320 -N ubi_rootfs1
to
 ubimkvol /dev/ubi0 -S 34 -N kernel1
 ubimkvol /dev/ubi0 -S 320 -N ubi_rootfs1
 ubimkvol /dev/ubi0 -S 264 -N rootfs_data
since the starting point is beginning of flash, no the middle.

> 
>>
>> 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)

I skimmed too fast on the changes on Your first proposal and got confused, overall the changes
were in good direction. There was a mistake in
'target/linux/ipq40xx/base-files/lib/upgrade/platform.sh' (it,s still retained in Your second
proposal) and I didn't see the difference (brainfart). Also small mistake in the installation
description in point 10. needed changes:
 ubirmvol /dev/ubi0 -N ubi_rootfs1
 ubirmvol /dev/ubi0 -N kernel1
to
 ubirmvol /dev/ubi0 -N ubi_rootfs
 ubirmvol /dev/ubi0 -N kernel
or only
 ubirmvol /dev/ubi0 -N ubi_rootfs
which You already did.

> 
>>
>> 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?

As written above, not always. The culprit is probably failed update but I can't say that for
sure, since I bought the devices on aftermarket.

> 
>>>
>>> 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.

Works with Brian's patch applied.

> 
> Cheers,
> Christian
> 

Regards
-- 
TMN