[PATCH 2/2] ipq40xx: add support for eero Cento (J010001)

[Connor Northway]

Hi,

Thanks for the thorough review and sorry for the delay in getting back
to it!

I just wanted to double check regarding the mac address concern before I
send out another patch.

(CC Christian, who replied with the same question on the original email)

>> +    eero,cento)
>> +        wan_mac=$(mtd_get_mac_ascii 0:IDENTITY mac)
>> +        lan_mac=$(macaddr_add "$(mtd_get_mac_ascii 0:IDENTITY mac)" 1)
> This should be replaced with NVMEM cells instead.

>> +    eero,cento)
>> +        caldata_extract "0:ART" 0x1000 0x2f20
>> +        ath10k_patch_mac $(macaddr_add $(mtd_get_mac_ascii 0:IDENTITY
>> mac) 3)
> Use NVMEM cells for both caldata and MAC setting.

>> +    eero,cento)
>> +        caldata_extract "0:ART" 0x5000 0x2f20
>> +        ath10k_patch_mac $(macaddr_add $(mtd_get_mac_ascii 0:IDENTITY
>> mac) 4)
> Use NVMEM cells for both caldata and MAC setting.

The pre-calibration bin from the 0:ART partiti
on has an
incorrect/generic mac address, which needs to be patched to the correct
one. If I understand correctly, providing the cal bin with an NVMEM cell
means you also need to provide the mac address with an NVMEM cell at the
same time - you can't patch it later.

To get the mac address into an NVMEM cell you'd need a constant memory
location, and this router only stores its mac address in an ASCII
key-value area, so I can't guarantee a specific offset.

The mtd_get_mac_ascii function has no problem extracting the value.

Let me know if I'm misunderstanding any of this or missing something.
I would definitely prefer to have it all contained to the device tree.

>> +    memory {
>> +        device_type = "memory";
>> +        reg = <0x80000000 0x20000000>;
>
> Is this required at all, bootloader should patch it in.

Nope. Removed it and it works fine. I think it was residual from my
early tests.

> Replace "ok" with "okay", "ok" has been deprec
ated for years.
> Same applies for all nodes.

Will do.

> What is kmod-mmc required for?
> MMC/SDHCI support is compiled in on this target already.

Don't actually need it, you're right. Tested without and it works fine.

> You are including $(call Device/UbiFit) which already has the same
> recipe for sysupgrade.bin

Thanks, will remove.