[OpenWrt-Devel] [PATCH v2] ipq40xx: Add support for IPQ4019 ap-dk07.1-c1
Christian Lamparter
chunkeey at gmail.com
Tue Sep 11 19:22:01 EDT 2018
On Monday, September 10, 2018 5:50:53 PM CEST Ram Chandra Jangir wrote:
> On Friday, September 07, 2018 8:36 PM CEST Christian Lamparter wrote:
> > On Friday, September 7, 2018 2:10:30 PM CEST Ram Chandra Jangir wrote:
>
> > 3) WDOG test
> > 4) cpu frequency scaling
> >Hey, that's great. Glad to hear that it is working.
> >So, do you know of the upstream status of the cpu frequency scaling fix
> that OpenWrt carries:
> ><https://patchwork.codeaurora.org/patch/473589/>
>
> >Because it would be a real shame if that wasn't fixed upstream too, right?
>
> yes, I just saw your above patch and as mentioned it is appearing on Compex
> WPJ428 and AVM FritzBox!4040 boards.
> Currently I don't have these boards to test locally. I may help if you can
> try to reproduce this problem on any of DK01/DK04 or DK07 board.
Hah, this is good one. The Compex WPJ428 is a 3rd party dev board and
Compex sells it with the all-important "Based on QCA Reference Design AP-DK03"
bullet point: <http://shop.compex.com.sg/wpj428.html>. So the question
becomes: How's this AP-DK03 different from the AP-DK01 or AP-DK04? And
was this "710MHz CPU Clock" a change made by Compex or Qualcomm?
I bet it has more to do with Qualcomm: Because there are more alternatives,
just in case Qualcomm has ran out of the AP-DK03 reference design boards.
A list of more potential boards is available over at wikidevi:
<https://wikidevi.com/w/index.php?title=Special%3AAsk&q=%3Cq%3E[[CPU1+model%3A%3A~IPQ40*]]%3C%2Fq%3E&po=%3FCPU1+model%3DCPU1%0D%0A%3FCPU1+clock+speed>
Currently, there are 7 listed boards @ 710 MHz. But there are also
other odd frequencies: 700 MHz, 600 MHz, 666 MHz, 632 MHz, 638 MHz?...
What is going on?
And cpufreq-dt is wondering as well: It complains about the following on boot:
|[ 1.399981] cpufreq: cpufreq_online: CPU0: Running at unlisted freq: 666000 KHz
or
> [ 1.275858] cpufreq: cpufreq_online: CPU0: Running at unlisted freq: 632000 KHz
<https://lists.openwrt.org/pipermail/openwrt-devel/2018-May/012126.html>
So, there must be a way to source some of the contenders/offenders.
And after so much time has passed since the discovery, I think it
would be high-time for Qualcomm to look what has caused this confusing
CPU speed mess and offer a proper fix to improve upon the current
081-clk-fix-apss-cpu-overclocking.patch way.
> > Signed-off-by: Ram Chandra Jangir <rjangir at codeaurora.org>
> >I do have a few comments down below.
>
> > --- a/target/linux/ipq40xx/image/Makefile
> > +++ b/target/linux/ipq40xx/image/Makefile
> > @@ -207,6 +207,19 @@ define Device/qcom_ap-dk04.1-c1 endef
> > TARGET_DEVICES += qcom_ap-dk04.1-c1
> >
> > +define Device/qcom_ap-dk07.1-c1
> > + $(call Device/FitImage)
> > + $(call Device/UbiFit)
> > + BOARD_NAME := ap-dk07.1-c1
> > This is a new device. So, why do you need a legacy BOARD_NAME for it?
> legacy ? Normally we refer the device name as AP-DK07.1-C1 internally too.
I think the BOARD_NAME moniker should be left alone. Currently, it serves
as a way to support the early/previous installations (For example for people
who had the ipq806x-target based FritzBox!4040 image).
But new devices don't need it. You should be fine without it.
> > + DEVICE_DTS := qcom-ipq4019-ap.dk07.1-c1
> > + KERNEL_INSTALL := 1
> > + KERNEL_SIZE := 4096k
> > + BLOCKSIZE := 128k
> > + PAGESIZE := 2048
> > + DEVICE_TITLE := QCA AP-DK07.1-C1
> > +endef
> > +TARGET_DEVICES += qcom_ap-dk07.1-c1
> >
> > What's wrong with calling the device "qcom_ipq4019-ap.dk07.1-c1"?
> I am fine to have the complete compatible string as device name
> "qcom_ipq4019-ap-dk07.1-c1" but not ok to have dts name as device name
> "qcom_ipq4019-ap.dk07.1-c1".
> Let John also comment on this. But this may need renaming of existing
> dk01 and dk04 device name too.
I don't understand why exactly this would be a problem, though.
Can you please explain in detail why you are "not ok to have dts
name as device name" there? Is it something about the "ipq4019",
or is it about the extra "."?
> > By the way:
> > Also, John is preparing to drop ath10k:
> >
> <https://git.openwrt.org/?p=openwrt/staging/blogic.git;a=commit;h=f9ccc8f646
> 70e3e15b160cffb19f1ef1a8519e97>
> >
> > | author John Crispin <john at phrozen.org>
> > | Wed, 5 Sep 2018 14:51:44 +0200 (14:51 +0200)
> > | committer John Crispin <john at phrozen.org>
> > | Thu, 6 Sep 2018 19:00:21 +0200 (19:00 +0200)
> > | commit f9ccc8f64670e3e15b160cffb19f1ef1a8519e97
> > |
> > |treewide: drop ath10k support
> > |
> > |people should use ath10k-ct instead
> > |
> > |Signed-off-by: John Crispin <john at phrozen.org>
> >
> > So please make sure that the Wi-Fis are indeed working with ath10k-ct.
>
> I think john's change is independent from AP-DK07.1-C1 board support and
> both changes can be merged independently.
> I will test with ath10k-ct too once available.
Depends. If your patch goes through before ath10k-ct is the new default, then
"Sure, you are fine". If the "switch to ath10k-ct" is faster: you have been
notifed and please verify it's working.
> > +From: Sricharan R <sricharan at codeaurora.org>
> > +Date: Fri, 25 May 2018 11:41:17 +0530
> > +Subject: [PATCH] ARM: dts: ipq4019: Add ipq4019-ap.dk07.1 common data
> > +
> > +Add the common data for all dk07 based boards.
> > +
> > +Reviewed-by: Abhishek Sahu <absahu at codeaurora.org>
> > +Signed-off-by: Sricharan R <sricharan at codeaurora.org>
> > +Signed-off-by: Andy Gross <andy.gross at linaro.org>
> > +---
> > + arch/arm/boot/dts/qcom-ipq4019-ap.dk07.1.dtsi | 75
> > ++++++++++++++++++++++++++++
> > + 1 file changed, 75 insertions(+)
> > + create mode 100644 arch/arm/boot/dts/qcom-ipq4019-ap.dk07.1.dtsi
> > +
> > +diff --git a/arch/arm/boot/dts/qcom-ipq4019-ap.dk07.1.dtsi
> > +b/arch/arm/boot/dts/qcom-ipq4019-ap.dk07.1.dtsi
> > +new file mode 100644
> > +index 0000000..9f1a5a66
> > +--- /dev/null
> > ++++ b/arch/arm/boot/dts/qcom-ipq4019-ap.dk07.1.dtsi
>
> >I guess you'll need to push those changes upstream.
> It looks like you missed to read my commit message where I have clearly
> mentioned that
> "The ap-dk07.1-c1 board is already available in upstream linux v4.17."
> Please see below patches for your reference:
> https://patchwork.kernel.org/patch/10426423/
> https://patchwork.kernel.org/patch/10426401/
And exactly for these files have some comments and questions... So, I have
added inline comments in order to have the necessary context and noted that
these should be fixed in linux upstream as well. Ok? Great!
as for:
> Please feel free to send your reviews changes to upstream linux, I will
> check with my internal team too on incorporating these reviews/changes.
This "ipq40xx: Add support for IPQ4019 ap-dk07.1-c1" mail is yours, isn't it?
And this is upstream openwrt, right? Is upstream openwrt somehow not
as good or important as upstream linux? So, why is there an extra step
involved here? Obviously, you are doing this as an Qualcomm employee and not
on your free time, since you are adding a image for a reference board that
can't be purchased/aquired like the mass-produced customer products that are
"based" on them.
And from my point of view: Reference boards should be held to a higher
standard, since many devs and hobbiest will use them as the "reference".
So, I really like them to be spotless and free of errors, oddities and
something that can cause misunderstandings. Because any "funny stuff"
there has a higher chance to turn up again and again and again and again.
With that out of the way: you have my previous mails and you have my
permission to go all out and forward it to the correct destination, since
you get paid for this work. Now, I tried doing this myself in the past -
you can find my patches on linux-msm-arm. - But I have to say that I was
mostly met with denial or silence.
So, you have to understand: It has come to this, that this way/method of
bugging you here has become more efficient than the alternatives. If
there is a better way that will cause less friction and actually lead to
faster and better results: please let us know.
> > +@@ -0,0 +1,75 @@
> > ++// SPDX-License-Identifier: GPL-2.0
> > ++// Copyright (c) 2018, The Linux Foundation. All rights reserved.
> > ++
> > ++#include "qcom-ipq4019.dtsi"
> > ++#include <dt-bindings/input/input.h> #include
> > ++<dt-bindings/gpio/gpio.h>
> > I noticed that you included input.h put none of these reference boards
> actually define any gpio-keys.
> >
> > So, what's the reason here? Are these DTS actually for real "boards" or
> are they just for show? (Also related: why is there no system/power LED
> > defined?)
> This patch is just a copy of already submitted patch in upstream kernel
> https://patchwork.kernel.org/patch/10426423/ to ensure that once we
> Upgrade to linux v4.17 or higher, then it can be dropped smoothly.
> Please feel free to send your reviews changes to upstream linux, I will
> check with my internal team too on incorporating these reviews/changes.
Yes, please forward it there.
Thanks.
> > ++/ {
> > ++ model = "Qualcomm Technologies, Inc. IPQ4019/AP-DK07.1";
> > ++
> > ++ memory {
> > ++ device_type = "memory";
> > ++ reg = <0x80000000 0x20000000>; /* 512MB */
> > ++ };
> > ++
> > ++ aliases {
> > ++ serial0 = &blsp1_uart1;
> > ++ serial1 = &blsp1_uart2;
(The alias node and chosen node was removed later in a separate
patch.)
and serial1 was never enabled. Can it be removed or not?
> > ++ };
> > ++ [...]
> > ++
> > ++ soc {
> > ++ pinctrl at 1000000 {
> > [...]
> > ++
> > ++ nand_pins: nand-pins {
> > ++ pins = "gpio53", "gpio55", "gpio56",
> > ++ "gpio57", "gpio58", "gpio59",
> > ++ "gpio60", "gpio62", "gpio63",
> > ++ "gpio64", "gpio65", "gpio66",
> > ++ "gpio67", "gpio68", "gpio69";
> > ++ function = "qpic";
> > ++ };
> > ++ };
> > bias-pull-ups and bias-pull-downs are missing here.
> > Also what happend to gpio52? The "qcom-ipq4019-ap.dk04.1.dtsi" still
> > defines it and from what I can tell, there seems to be only one valid way to
> > wire up a nand chip with the SoC.
>
> nand pins pull-up/pull-down will be taken care by uboot.
Do you have the u-boot code somewhere? The copy we got:
<https://github.com/riptidewave93/meraki-uboot/blob/mr33-20170427/board/qcom/ipq40xx_cdp/ipq40xx_board_param.h#L186>
specifies that gpio52 (that is missing above) is somehow needed
for MACH_TYPE_IPQ40XX_AP_DK07_1_C1 device there.
<https://github.com/riptidewave93/meraki-uboot/blob/mr33-20170427/board/qcom/ipq40xx_cdp/ipq40xx_board_param.h#L1158>
I don't necessarily trust u-boot there, since we had issues with it
in the past with the Cisco Meraki MR33.
> I have not
> added this in kernel yet, i will test and add one by one remaining
> patches/features too.
That's good to hear.
> > ++
> > ++ dma at 7884000 {
> > ++ status = "ok";
> > ++ };
> > [...]
> > +diff --git a/arch/arm/boot/dts/qcom-ipq4019-ap.dk07.1-c1.dts
> > +b/arch/arm/boot/dts/qcom-ipq4019-ap.dk07.1-c1.dts
> > +new file mode 100644
> > +index 0000000..8c7ef65
> > +--- /dev/null
> > ++++ b/arch/arm/boot/dts/qcom-ipq4019-ap.dk07.1-c1.dts
>
> > Same as with the .dtsi: please push those changes upstream.
> This is copy of upstream linux patch, please see my above comments.
>
As with the qcom-ipq4019-ap.dk07.1.dtsi: See my comment above. Thanks.
> > +@@ -0,0 +1,64 @@
> > ++// SPDX-License-Identifier: GPL-2.0
> > ++// Copyright (c) 2018, The Linux Foundation. All rights reserved.
> > ++
> > ++#include "qcom-ipq4019-ap.dk07.1.dtsi"
> > ++
> > ++/ {
> > ++ model = "Qualcomm Technologies, Inc. IPQ4019/AP-DK07.1-C1";
> > ++ compatible = "qcom,ipq4019-ap-dk07.1-c1";
> > Make that:
>
> > compatible = "qcom,ipq4019-ap-dk07.1-c1", "qcom,ipq4019";
> As I am saying this is copy of upstream patch, please refer below patch
> 905-dts-qcom-ipq4019-ap.dk07.1-enable-nodes.patch
> which already does the similar.
Similar?
Let's look at 905-dts-qcom-ipq4019-ap.dk07.1-enable-nodes.patch from
your previous mail:
|+++ b/target/linux/ipq40xx/patches-4.14/905-dts-qcom-ipq4019-ap.dk07.1-enable-nodes.patch
|@@ -0,0 +1,79 @@
|+--- a/arch/arm/boot/dts/qcom-ipq4019-ap.dk07.1-c1.dts
|++++ b/arch/arm/boot/dts/qcom-ipq4019-ap.dk07.1-c1.dts
|+@@ -5,7 +5,7 @@
|+
|+ / {
|+ model = "Qualcomm Technologies, Inc. IPQ4019/AP-DK07.1-C1";
|+- compatible = "qcom,ipq4019-ap-dk07.1-c1";
|++ compatible = "qcom,ap-dk07.1-c1", "qcom,ipq4019";
It changed the compatible from the upstream "qcom,ipq4019-ap-dk07.1-c1" to
"qcom,ap-dk07.1-c1" for no reason. Why exactly was this change made?
(The change to add the "qcom,ipq4019" is fine though: +1.
And I understand that it can dropped for the next LTS/v4.19 kernel)
> > ++
> > ++ soc {
> > ++ pci at 40000000 {
> > ++ status = "ok";
> > ++ perst-gpio = <&tlmm 38 0x1>;
> > ++ };
> > The phandle's 3'rd parameter should be GPIO_ACTIVE_LOW and not 0x1.
Great, I'll be looking forward to having this changed.
> > ++
> > ++ spi at 78b6000 {
> > [...]
> > ++
> > ++ m25p80 at 0 {
> > ++ #address-cells = <1>;
> > ++ #size-cells = <1>;
> > ++ reg = <0>;
> > ++ compatible = "n25q128a11";
> > I think "jedec,spi-nor" is missing here. It must be included for any SPI
> NOR flash that supports the JEDE READ ID Opcode.
> This is copy of upstream linux patch, please see my above comments.
>
In case you are not aware: That "jedec,spi-nor" is a "must" requirement of
the jedec,spi-nor.txt binding file in upstream:
<https://www.kernel.org/doc/Documentation/devicetree/bindings/mtd/jedec%2Cspi-nor.txt>
| Must also include "jedec,spi-nor" for any SPI NOR flash that can
| be identified by the JEDEC READ ID opcode (0x9F).
The n25q128a series supports that JEDEC READ ID opcode
as per Table 16: Command Set:
<http://www.micron.com/~/media/documents/products/data-sheet/nor-flash/serial-nor/n25q/n25q_128mb_3v_65nm.pdf>
> > +--- a/arch/arm/boot/dts/qcom-ipq4019-ap.dk07.1.dtsi
> > ++++ b/arch/arm/boot/dts/qcom-ipq4019-ap.dk07.1.dtsi
> > +@@ -67,9 +71,49 @@
> > + };
> > +
> > + qpic-nand at 79b0000 {
> > ++ compatible = "qcom,ipq4019-nand", "qcom,msm-nand";
> > + pinctrl-0 = <&nand_pins>;
> > + pinctrl-names = "default";
> > + status = "ok";
> > ++
> > ++ nand at 0 {
> > ++ compatible = "qcom,nandcs";
> > ++ reg = <0>;
> > ++ #address-cells = <1>;
> > ++ #size-cells = <1>;
> > ++
> > ++ nand-ecc-strength = <4>;
> > ++ nand-ecc-step-size = <512>;
> > ++ nand-bus-width = <8>;
> > Is u-boot filling in the partitions here?
> Yes, u-boot will fill the SMEM partitions details.
OK.
(there's also an smem-partition parser upstream.
I think it's located in drivers/soc/qcom/.
But if u-boot does its job: this is fine.)
> > ++ };
> > ++ };
> > ++
> > ++ mdio at 90000 {
> > ++ status = "okay";
> > ++ };
> > ++
> > ++ ess-switch at c000000 {
> > ++ status = "okay";
> > ++ switch_mac_mode = <0x0>; /* mac mode for
> RGMII RMII */
> > The comment (RGMII/RMII mode) is either wrong or the switch_mac_mode value
> is misleading.
No answer?
Is the RGMII RMII comment wrong? Or is the 0x0 ( = PSGMII) value bad?
> [...]
>
> > OT: A user reported a kernel panic with the IPQ4019 ap-dk01.1-c1 on the
> forum: <https://forum.openwrt.org/t/ipq4018-dk01-1-kernel-panic/20060>
> > I wonder if this affects the ap-dk04.1-c1 and ap-dk07.1-c1 as well.
> > Can you please comment on that.
> We can take this issue offline, Still I am not done with sysupgrade,
> envtools and other features support/testing yet.
> I will look in to and will check what is wrong during sysupgrade and will
> udate in the actual forum:
> <https://forum.openwrt.org/t/ipq4018-dk01-1-kernel-panic/20060> .
I think the combined nand-and-nor sysupgrade.bin will be a real pain
to deal with the existing sysupgrade logic. It might be easier to make two
separate "devices". One will be the nand-only version while the other will
get the nor-only versions of the ipq4019-ap-dk07.1-c1.dts (and of course
each a different compatible or model). This should make it easy to hook the
two versions into the existing sysupgrade code and it would make the
reference board a "good example" of "how to do it"(tm).
(The only sore spot will be the switching between the nand or nor version.
But this could be done by flashing the initial [nand|nor]-only version
with u-boot. From what I can tell, the nor / nand switching feature is only
ever relevant for development and prototype boards. And thankfully, the
mass-produced consumer boards will just stick to one of the options
for the full production run or at the very atleast a major revision.)
Regards,
Christian
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
More information about the openwrt-devel
mailing list