[OpenWrt-Devel] [PATCH v2] ipq40xx: Add support for IPQ4019 ap-dk07.1-c1
Ram Chandra Jangir
rjangir at codeaurora.org
Fri Sep 14 11:16:05 EDT 2018
On 9/12/2018 4:52 AM, Christian Lamparter wrote:
> 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+
> mode
> l%3A%3A~IPQ40*]]%3C%2Fq%3E&po=%3FCPU1+model%3DCPU1%0D%0A%3FCPU1+clock+
> spee
> d>
>
> 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.htm
> l>
>
> 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.
If you can reproduce the same issue on DK07, please revert back. Since
DK03 is not upstreamed, I would like to defer this discussion until then.
>
>>> 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=f9cc
> c8f6
> 46
>> 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/boar
> d/qc om/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/boar
> d/qc om/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-ena
> ble-
> 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)
OK
>
>>> ++
>>> ++ 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/jede
> c%2C
> spi-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
> /ser
> ial-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).
>
I have conveyed to DK01 team and they will look in to it and will update the ticket.
There is no single sysupgrade image for nor & nand. For nor board, it is openwrt-ipq40xx-qcom_ap-dk07.1-c1-squashfs-sysupgrade.bin wheres for nand board it is openwrt-ipq40xx-qcom_ap-dk07.1-c1-squashfs-nand-sysupgrade.bin.
Thanks,
Ram
> (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
>
_______________________________________________
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