[LEDE-DEV] [PATCH v2] ipq806x: add ap.dk01.1 board support

Roman Yeryomin roman at advem.lv
Mon Oct 23 10:44:15 PDT 2017


On 2017-10-16 23:30, Christian Lamparter wrote:
> On Monday, October 16, 2017 10:10:52 PM CEST Roman Yeryomin wrote:
>> On 2017-10-16 20:32, Christian Lamparter wrote:
>> > Added John, maybe he has more comments.
> (This time with the right mail address - sorry)
> 
>> >> Changes:
>> >> - add partitions
>> >> - enable wifi and ethernet
>> >> - set max cpu speed to 710MHz
>> >> - set memory size to 256MB
>> >> - add image generation
>> >> - extract pre-cal data from ART partition
>> >>
>> >> Wirespeed NAT can be achieved with spreading rx interrupts over
>> >> different
>> >> cores. Wifi needs love -- too slow. Could be that latest ath10k helps,
>> >> didn't test yet.
>> > That "Wifi needs love" stinks of board-2.bin issues. And we had to deal
>> > with this before:
>> > <http://lists.infradead.org/pipermail/ath10k/2016-November/008763.html>
>> >
>> > Verify that you have the correct (and up to date) board-2.bin for the
>> > board.
>> > please add the board's board-2.bin to the ipq-wifi package.

I don't think it needs some different board data.
bmi id is 17 for the board (taken from ART partition) which exists in 
board-2.bin
Also now, after fixing CPU speed (again), I'm getting 550Mbps which is 
close to max for open air tests.

>> I was going to deal with wifi related issues later.
> Please don't defere the wireless issue. Fix it now.

Hm? Don't understand this.
There are a lot boards added with something not working. Double 
standards?
And in this case it was actually working.

> In the mail, Michal Kazior who was working for Qualcomm Atheros at the 
> time
> stated quite clearly:
> 
> "Please don't do that. Your basically pushing bogus data as board data
> to the device. I'm a ltittle surprised firmware didn't notice. Anyway,
> expect the device to misbehave (crash, hang, regulatory violations)
> with this "board.bin"."

I don't feed it with board.bin, you refer to irrelevant context.

> and also:
> 
> "board-2 is a key-value store of actual board files. Some devices,

Exactly, that's why board-2.bin just works.

> notably qca61x4 hw3+ and qca4019 need distinct board files to be
> uploaded. Otherwise they fail in various ways."
> 
>> >>
>> >> +define Device/AP-DK01.1-C1
>> >> +	PROFILES += $$(DEVICE_NAME)
>> >> +	DEVICE_TITLE := QCA AP-DK01.1-C1
>> >> +	BOARD_NAME := ap-dk01.1-c1
>> >> +	DEVICE_DTS := qcom-ipq4019-ap.dk01.1-c1
>> >    This "qcom-ipq4019-ap.dk01.1-c1" is important later on.
>> >
>> >> +	KERNEL_LOADADDR := 0x80208000
>> >> +	KERNEL_INSTALL := 1
>> >> +	KERNEL_SIZE := 4096k
>> >> +	IMAGE_SIZE := 26624k
>> >> +	FILESYSTEMS := squashfs
>> >> +	$(call Device/FitImage)
>> > Any reason why this include is not at the top of the define?
>> 
>> This is kernel image generation, right before full image.
>> Actually I don't understand why it's put on top.
> But you did notice that all other devices definitions have it on top as 
> well?
> Are you really sure, that you really want to trigger people with OCD to 
> do
> make a "cleanup" patch later? You could avoid this altogether right 
> now.

I don't understand this.
I would rather clean this up now and move all the other calls lower. So 
they would fit at least some logic.

> [...]
> 
>> >> +@@ -20,6 +20,11 @@
>> >> + 	model = "Qualcomm Technologies, Inc. IPQ4019/AP-DK01.1";
>> >> + 	compatible = "qcom,ipq4019";
>> > I think this should be "qcom,ipq4019-ap.dk01.1-c1", "qcom,ipq4019".
>> > The device-tree folks stick to their rules in the
>> > usage-model.txt / 2.2 Platform Identification
>> > <https://www.kernel.org/doc/Documentation/devicetree/usage-model.txt>
>> 
>> I didn't change that. And will not in v3, since I'm going to patch
>> qcom-ipq4019-ap.dk01.1-c1.dts
> This has to be done! Again the usage-model.txt clearly states that:
> "The 'compatible' property contains a sorted list of strings starting
> with the *exact name of the machine*, followed by an optional list of
> boards it is compatible with sorted from most compatible to least."
> (Read the rest of 2.2 Platform Identification as well)

I think you forgot what you wrote yourself.
This is dtsi, for dts, the one which should be changed (as you noted 
yourself), I agree makes sense, but not dtsi.

> Also, Mathias Kresin has plans to use the first "compatible" entry
> as an unique identifier for the userspace scripts. (And remove
> ipq806x current custom board identifier script that relies on the
> model)

That's good, but I cannot rely on plans and have to add custom board 
identifier.
Once that's introduced he can change every user, as it's always done 
with many other different things.
Currently 01_preinit_do_ipq806x.sh overrides this:

> part of this work has already been merged see:
> https://github.com/lede-project/source/blob/master/package/base-files/files/lib/preinit/02_sysinfo
> 
> Since the "qcom,ipq4019" compatible is already used by the 
> AP-DK04.1-C1,
> this will cause a conflict with your "AP-DK01.1-C1"... And then the
> userspace won't know "which is which".

Sure, but again, not for dtsi

Actually it appeared that the board I have is AP-DK01.2-C1
All ref boards differ either with flash configuration (spi/nand/emmc) or 
SoC (4019/4029/4018/4028).
And actually all of them have same RAM size (so this is going to be in 
dtsi).

>> >> + 	clocks {
>> >> +                 xo: xo {
>> >> +                         compatible = "fixed-clock";
>> >> diff --git
>> >> a/target/linux/ipq806x/patches-4.9/864-02-dts-ipq4019-ap.dk01.1-fix-max-cpu-speed.patch
>> >> b/target/linux/ipq806x/patches-4.9/864-02-dts-ipq4019-ap.dk01.1-fix-max-cpu-speed.patch
>> >> new file mode 100644
>> >> index 0000000..e9540f4
>> >> --- /dev/null
>> >> +++
>> >> b/target/linux/ipq806x/patches-4.9/864-02-dts-ipq4019-ap.dk01.1-fix-max-cpu-speed.patch
>> >> @@ -0,0 +1,15 @@
>> >> +--- a/arch/arm/boot/dts/qcom-ipq4019-ap.dk01.1.dtsi
>> >> ++++ b/arch/arm/boot/dts/qcom-ipq4019-ap.dk01.1.dtsi
>> >> +@@ -135,3 +135,12 @@
>> >> + 		};
>> >> + 	};
>> >> + };
>> >> ++
>> >> ++&cpu0_opp_table {
>> >> ++	/delete-node/ opp at 666000000;
>> >> ++
>> >> ++	opp at 710000000 {
>> >> ++		opp-hz = /bits/ 64 <710000000>;
>> >> ++		clock-latency-ns = <256000>;
>> >> ++	};
>> >> ++};
>> > This looks rather familiar. Like exactly a 1:1 copy from the FB4040's
>> > dts
>> > right here:
>> > https://github.com/lede-project/source/blob/master/target/linux/ipq806x/files-4.9/arch/arm/boot/dts/qcom-ipq4019-fritz4040.dts#L286
>> > Is this true?
>> >
>> > There's a reason to stick with the 666MHz rate though. You should check

I don't think there is. All ipq40xx CPUs are 716.8MHz at max.
The supported frequencies are: 48, 200, 500 and 716.8 MHz

>> > if the device produces messages like:
>> > [    1.399981] cpufreq: cpufreq_online: CPU0: Running at unlisted
>> > freq: 666000 KHz
>> > [    1.400256] cpufreq: cpufreq_online: CPU0: Unlisted initial
>> > frequency changed to: 710000 KHz
>> > (From what I know, the SBL actually sets it to 666MHz)
>> >
>> > But there's more. If you look at qualcomm's page, it's says that the
>> > CPU Clock Speed is 717 MHz: <https://www.qualcomm.com/products/ipq4028>
>> >
>> > Since you are working for a OEM/ODM. You could please ask what is the
>> > right MHz table for these devices? Unfortunately, Qualcomm never got
>> > around to fix this upstream and without an official statement from them
>> > it's very difficult to push stuff like this upstream.

It looks like it was mostly a limitation introduced by hw instability 
(both IC and boards) in early stages.
Mass production chips (at least should) run at full speed.

>> Hmm...
> Could you please ask Qualcomm?
> (Also to consider: there are IPQ40XX devices with and without an 
> heat-sink.
> It could be that devices w/o a heat-sink have to have lower clocks 
> prevent
> overheating.)

Reference design boards don't have them and they run at full speed.

>> >> diff --git
>> >> a/target/linux/ipq806x/patches-4.9/864-04-dts-ipq4019-ap.dk01.1-enable-wifi-and-ethernet.patch
>> >> b/target/linux/ipq806x/patches-4.9/864-04-dts-ipq4019-ap.dk01.1-enable-wifi-and-ethernet.patch
>> >> new file mode 100644
>> >> index 0000000..cc90475
>> >> --- /dev/null
>> >> +++
>> >> b/target/linux/ipq806x/patches-4.9/864-04-dts-ipq4019-ap.dk01.1-enable-wifi-and-ethernet.patch
>> >> @@ -0,0 +1,87 @@
>> >> +--- a/arch/arm/boot/dts/qcom-ipq4019-ap.dk01.1.dtsi
>> >> ++++ b/arch/arm/boot/dts/qcom-ipq4019-ap.dk01.1.dtsi
>> >> +@@ -15,6 +15,7 @@
>> >> +  */
>> >> +
>> >> + #include "qcom-ipq4019.dtsi"
>> >> ++#include <dt-bindings/soc/qcom,tcsr.h>
>> > please move this to the qcom-ipq4019-ap.dk01.1-c1.dts.
>> >
>> >> +
>> >> + / {
>> >> + 	model = "Qualcomm Technologies, Inc. IPQ4019/AP-DK01.1";
>> >> +@@ -25,6 +26,27 @@
>> >> + 		reg = <0x80000000 0x10000000>;
>> >> + 	};
>> >> +
>> >> ++	reserved-memory {
>> >> ++		#address-cells = <0x1>;
>> >> ++		#size-cells = <0x1>;
>> >> ++		ranges;
>> >> ++
>> >> ++		rsvd1 at 87000000 {
>> >> ++			reg = <0x87000000 0x0500000>;
>> >> ++			no-map;
>> >> ++		};
>> >> ++
>> >> ++		wifi_dump at 87500000 {
>> >> ++			reg = <0x87500000 0x600000>;
>> >> ++			no-map;
>> >> ++		};
>> >> ++
>> >> ++		rsvd2 at 87B00000 {
>> >> ++			reg = <0x87b00000 0x500000>;
>> >> ++			no-map;
>> >> ++		};
>> >> ++	};
>> >
>> > The wifi_dump area is not needed by the ath10k driver.
>> > Furthermore, the you could probably get away with reserving
>> > less memory. From what I know, you only need to reserve space
>> > for the "QSEE environment", which is located between
>> > 0x87e00000 and 0x88000000.

wifi_dump == cnss_debug (not sure what this is)
I wouldn't be so sure that it's not needed at all.

QSEE == TZ
And seems your information is incorrect.
Will introduce full map in v3.

>> I have no information about those.
>> I think there should be also NSS region...
> Do you know, if the IPQ40XX actually has a NSS?

I believe it has. Just called differently.

> Qualcomm's IPQ40[1|2][89] pages don't mention anything about a NSS.
> And there isn't any special NSS driver in the GPL dumps I have seen.

There is HWNAT block which I believe is the same.
Looks like they call it edma now, looks like it's same engine with 
different interface.

> From what I know these Network Subsystem (NSS) are only a thing for
> the IPQ8064, IPQ8065 and the upcoming IPQ8074 tier.

I could be wrong though. Still waiting for info.


Regards,
Roman



More information about the Lede-dev mailing list