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

Christian Lamparter chunkeey at gmail.com
Mon Oct 16 13:30:59 PDT 2017


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 was going to deal with wifi related issues later.
Please don't defere the wireless issue. Fix it now.

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

and also:

"board-2 is a key-value store of actual board files. Some devices,
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.

[...]

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

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)

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

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

> >> 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.
> 
> I have no information about those.
> I think there should be also NSS region...
Do you know, if the IPQ40XX actually has a NSS?

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.

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

> > (please move this to the qcom-ipq4019-ap.dk01.1-c1.dts.)
> >> ++
> >> + 	clocks {
> >> +                 xo: xo {
> >> +                         compatible = "fixed-clock";
> > 
> > 
> > _______________________________________________
> > Lede-dev mailing list
> > Lede-dev at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/lede-dev
> 
> Why do I receive emails two times?
> One with this signature ^^^ and another without?

Probably because, I hit "reply all"? But this is a mailing-list!
If you are not familiar with the pro-and-cons, you can read more
about this duplicated mails topic right here:
<http://david.woodhou.se/reply-to-list.html>.

Regards,
Christian



More information about the Lede-dev mailing list