[LEDE-DEV] [PATCH 2/2] ipq806x: Add support for ipq40xx subtarget
Christian Lamparter
chunkeey at googlemail.com
Wed Apr 19 08:22:42 PDT 2017
Hello Ram,
On Monday, April 17, 2017 11:13:11 PM CEST Ram Chandra Jangir wrote:
> > diff --git a/target/linux/ipq806x/Makefile
> > b/target/linux/ipq806x/Makefile index 5a5551c..b5b36e1 100644
> > --- a/target/linux/ipq806x/Makefile
> > +++ b/target/linux/ipq806x/Makefile
> > @@ -21,7 +21,7 @@ DEFAULT_PACKAGES += \
> > kmod-ata-core kmod-ata-ahci kmod-ata-ahci-platform \
> > kmod-usb-core kmod-usb-ohci kmod-usb2 kmod-usb-ledtrig-usbport \
> > kmod-usb3 kmod-usb-dwc3-of-simple kmod-usb-phy-qcom-dwc3 \
> > - kmod-ath10k wpad-mini \
> > + kmod-ath10k wpad-mini kmod-switch-ar40xx kmod-ipq40xx-edma \
>
> >>the switch and edma driver are already part of the default image.
> >>Why do you want to have them as separate packages?
> [Ram]: It is good to have enabled them as module instead of inbuilt,
> Theoretically ar40xx switch depends on swconfig module which is
> enabled as kmod only.
I fail to see why this would be good idea. Can you please explain in !detail!
why having this as an external module would be advantageous. As for why not:
The kmod-swconfig module does not set SWCONFIG_LEDS. This will break the LAN
LED on the Fritz!Box 4040.
> > diff --git
> > a/target/linux/ipq806x/patches-4.9/852-ipq4019-pinctrl-Updated-various
> > -Pin-definitions.patch
> > b/target/linux/ipq806x/patches-4.9/852-ipq4019-pinctrl-Updated-various
> > -Pin-definitions.patch
> > new file mode 100644
> > index 0000000..4267d47
> > --- /dev/null
> > +++ b/target/linux/ipq806x/patches-4.9/852-ipq4019-pinctrl-Updated-var
> > +++ ious-Pin-definitions.patch
> > @@ -0,0 +1,1332 @@
> > +From fc6cf61517b8b4ab4678659936fc7572f699d6e7 Mon Sep 17 00:00:00
> > +2001
> > +From: Ram Chandra Jangir <rjangir at codeaurora.org>
> > +Date: Tue, 28 Mar 2017 14:00:00 +0530
> > +Subject: [PATCH] ipq4019: pinctrl: Updated various Pin definitions
> > +
> > +Populate default values for various GPIO functions
> > +
> > +Signed-off-by: Ram Chandra Jangir <rjangir at codeaurora.org>
> >Please send this upstream to linux-gpio at vger.kernel.org.
> >I'm saying this because it has become nearly impossible to merge this stuff
> into the kernel without the right domain in the email-address.
> [Ram]: it will be upstreamed soon.
There were a bunch of comments for the original submission to linux-gpio.
Please check them out: <https://patchwork.kernel.org/patch/7662241/>
before you submit this again. Because from what I can tell, the original
issues in v2 did not get addressed in this version.
(i.e.: owner parameter. see Bjorn's comment about the platform driver.)
> > +@@ -122,11 +123,60 @@ static int ipq40xx_mdio_write(struct mii_bus *bus,
> int mii_id, int regnum,
> > + return 0;
> > + }
> > +
> > ++static int ipq40xx_phy_reset(struct platform_device *pdev) {
> > ++ struct device_node *mdio_node;
> > ++ int phy_reset_gpio_number;
> > ++ int ret;
> > ++
> > ++ mdio_node = of_find_node_by_name(NULL, "mdio");
> > ++ if (!mdio_node) {
> > ++ dev_err(&pdev->dev, "Could not find mdio node\n");
> > ++ return -ENOENT;
> > ++ }
> > ++
> > ++ ret = of_get_named_gpio(mdio_node, "phy-reset-gpio", 0);
> > ++ if (ret < 0) {
> > ++ dev_err(&pdev->dev, "Could not find phy-reset-gpio\n");
> > ++ return ret;
> >This could break existing configurations. Please add a check that would
> skip the error in case the "phy-reset-gpio" property isn't available.
> [Ram]: This is required to reset via gpio, will skip if this property is not
> available/defined in next patch version.
One thing, can you ask around what would happen if the PHY is resetted on a
device powered by PoE? I know that this is an issue with a Meraki MR18
(AR71XX with AR8035) when it is powered by PoE Alternative B. In this
case, the MR18 will simply fail to boot, because once the PHY reset GPIO
is asserted it just restarts due to the "sudden" loss of power.
> > diff --git
> > a/target/linux/ipq806x/patches-4.9/858-arm-dts-Add-ess-switch-device-f
> > or-IPQ4019.patch
> > b/target/linux/ipq806x/patches-4.9/858-arm-dts-Add-ess-switch-device-f
> > or-IPQ4019.patch
> > new file mode 100644
> > index 0000000..4598451
> > --- /dev/null
> > +++ b/target/linux/ipq806x/patches-4.9/858-arm-dts-Add-ess-switch-devi
> > +++ ce-for-IPQ4019.patch
> > @@ -0,0 +1,486 @@
> > +From 4842020af3b39ce8c7c9a92de106d8fffd92b7c0 Mon Sep 17 00:00:00
> > +2001
> > +From: Ram Chandra Jangir <rjangir at codeaurora.org>
> > +Date: Tue, 28 Mar 2017 14:00:00 +0530
> > +Subject: [PATCH] arm: dts: Add ess switch device for IPQ4019
> > +
> > +- Update ipq4019 dts nodes for mdio interface, ess-switch
> > + and edma driver.
> > +- Update dt documentation for qca-ess ethernet subsystem
> > +
> > +Signed-off-by: xiaofeis <xiaofeis at codeaurora.org>
> > +Signed-off-by: Ram Chandra Jangir <rjangir at codeaurora.org>
> > +---
> > +diff --git a/Documentation/devicetree/bindings/net/qca-ess.txt
> > +b/Documentation/devicetree/bindings/net/qca-ess.txt
> > +new file mode 100644
> > +index 0000000..c192774
> > +--- /dev/null
> > ++++ b/Documentation/devicetree/bindings/net/qca-ess.txt
> > +@@ -0,0 +1,107 @@
> > ++QCA Ethernet Subsystem
> > ++----------------------
> > ++
> > ++This driver adds support for the Ethernet subsystem
> > ++
> > ++1. QCA Ethernet DMA
> > ++----------------------
> > ++
> > ++Required properties:
> > ++ - compatible = "qcom,ess-edma";
> > ++
> > ++Optional properties:
> > ++
> > ++Example:
> > ++ edma at c080000 {
> > ++ compatible = "qcom,ess-edma";
> > ++ reg = <0xc080000 0x8000>;
> > ++ qcom,page-mode = <0>;
> > ++ qcom,rx_head_buf_size = <1540>;
> rx-head-buf-size?
> > ++ qcom,port_id_wan = <0x5>;
> >port_id_wan? port-id-wan ?
> [Ram]: This is port id for wan port, example: here for dakota, port 5 is the
> WAN port.
No, check again. the device-tree description is out of date. Your code looks
for "qcom,rx-head-buf-size" and not "qcom,rx_head_buf_size". The same is true
for the other properties.
> > ++2. QCA Ethernet Switch
> > ++----------------------
> > ++
> > ++Required properties:
> > ++ - compatible = "qcom,ess-switch";
> > ++
> > ++Optional properties:
> > ++
> > ++Example:
> > ++
> > ++ ess-switch at c000000 {
> > ++ compatible = "qcom,ess-switch";
> > ++ reg = <0xc000000 0x80000>; /* 512KB */
> > ++ switch_access_mode = "local bus";
> >switch-access-mode?
> [Ram]: there are some boards which uses different switch, we use mdio bus so
> here to identify whether to use mdio bus or not.
Same problem. Please update the description.
> > ++ resets = <&gcc ESS_RESET>;
> > ++ switch_cpu_bmp = <0x1>; /* cpu port bitmap */
> > ++ switch_lan_bmp = <0x1e>; /* lan port bitmap */
> > ++ switch_wan_bmp = <0x20>; /* wan port bitmap */
> >switch_cpu_bmp, switch_lan_bmp, switch_wan_bmp can be removed.
> >Userspace should take care of setting up the VLAN.
> [Ram]: Userspace can always change the VLANs, but these are default values ,
> It is not good to declare these default values as macros inside driver code
> instead it is good to add them in dts file, it will also help to
> configure the different default values based on the different boards.
I say: It's not a good idea to dublicate what userspace is doing later.
See: target/linux/ipq806x/base-files/etc/board.d/02_network:
+ ucidef_set_interfaces_lan_wan "eth1" "eth0"
+ ucidef_add_switch "switch0" \
+ "0t at eth1" "1:lan" "2:lan" "3:lan" "4:lan"
So what exactly is the point of doing this twice then? Please keep the
switch and ethernet configuration in userspace. Or alternatively
convert essedma and ar40xx to use the Distributed Switch Architecture.
> > ++ };
> > ++
> > ++3. QCA Ethernet PHY mode
> > ++-------------------------
> > ++
> > ++Required properties:
> > ++ - compatible = "qcom,ess-psgmii";
> > ++
> > ++Optional properties:
> > ++
> > ++Example:
> > ++
> > ++ ess-psgmii at 98000 {
> > ++ compatible = "qcom,ess-psgmii";
> > ++ reg = <0x98000 0x800>; /* 2k */
> > ++ psgmii_access_mode = "local bus";
> >psgmii-access-mode?
> [Ram]: same as switch-access-mode.
Please replace _ with -
> > ++ compatible = "qcom,ipq4019-mdio";
> > ++ reg = <0x90000 0x64>;
> > ++ };
> > ++
> > +diff --git a/arch/arm/boot/dts/qcom-ipq4019-ap.dk04.1.dtsi
> > +b/arch/arm/boot/dts/qcom-ipq4019-ap.dk04.1.dtsi
> > +index b68fc1a..1aee3b1 100644
> > +--- a/arch/arm/boot/dts/qcom-ipq4019-ap.dk04.1.dtsi
> > ++++ b/arch/arm/boot/dts/qcom-ipq4019-ap.dk04.1.dtsi
> > +@@ -161,5 +182,56 @@
> > + watchdog at b017000 {
> > + status = "ok";
> > + };
> > ++
> > ++ ess-switch at c000000 {
> > ++ status = "ok";
> > ++ switch_mac_mode = <0x0>; /* mac mode for RGMII RMII */
> > ++ switch_initvlas = <
> > ++ 0x0007c 0x54 /* PORT0_STATUS */
> > ++ >;
> > ++ led_source at 0 {
> > ++ led = <0x3>; /*led number */
> > ++ source = <0x1>; /*source id 1 */
> > ++ mode = "normal"; /*on,off,blink,normal */
> > ++ speed = "all"; /*10M,100M,1000M,all */
> > ++ freq = "auto"; /*2Hz,4Hz,8Hz,auto*/
> > ++ };
> [...]
> >Just a heads up: I had to add a "gpio-controller" to the ar40xx driver.
> >This was necessary, because the AVM FritzBox 4040 uses one of the switch
> LEDs to enable the power of both USB ports.
> >(Off-topic: Yes, the usb power will "blink" for a few seconds during boot
> :-D )
Also, where does this led_source node get parsed?
> > +diff --git a/arch/arm/boot/dts/qcom-ipq4019.dtsi
> > +b/arch/arm/boot/dts/qcom-ipq4019.dtsi
> > +index 7013c85..9793611 100644
> > +--- a/arch/arm/boot/dts/qcom-ipq4019.dtsi
> > ++++ b/arch/arm/boot/dts/qcom-ipq4019.dtsi
> > +@@ -325,43 +325,47 @@
> > +
> > + mdio at 90000 {
> > + #address-cells = <1>;
> > +- #size-cells = <0>;
> > ++ #size-cells = <1>;
> #size-cells = <0>;
> >you don't have any size cells in the subnodes.
> [Ram]: Yes, you are right. It seems 0 is better. will add it in next patch.
Don't forget to update the device tree binding documentation as well.
> > +@@ -378,57 +384,58 @@
> > + compatible = "qcom,ess-edma";
> > + reg = <0xc080000 0x8000>;
> > + qcom,page-mode = <0>;
> > +- qcom,rx_head_buf_size = <1540>;
> > +- qcom,mdio_supported;
> > +- qcom,poll_required = <1>;
> > +- qcom,num_gmac = <2>;
> > +- interrupts = <0 65 IRQ_TYPE_EDGE_RISING
> [...]
> > +- 0 255 IRQ_TYPE_EDGE_RISING>;
> > ++ qcom,rx-head-buf-size = <1540>;
> > ++ qcom,num-gmac = <2>;
> >Technically, the essedma has only one. See the VLAN comment above.
> [Ram]: There are two groups in dakota boards, one is LAN group and second
> is WAN group. num-gmac tells number of groups to be created, which is
> currently set to 2.
VLAN configuration is done by userspace. The secondary (tertiary, ...) MACs
are just a virtual MACs for VLAN and nothing else. The kernel already does VLAN
for you. Please, do not waste resources for this. The essedma driver allocates
a full rx receive ring for these virtual MACs too and this is problem for 128 MiB
RAM devices like the Asus RT-AC58U.
> > + local-mac-address = [00 00 00 00 00 00];
> > +- vlan_tag = <1 0x1f>;
> > ++ qcom,phy-mdio-addr = <4>;
> > ++ qcom,poll-required = <1>;
> > ++ qcom,poll-required-dynamic = <1>;
> > ++ qcom,forced-speed = <1000>;
> > ++ qcom,forced-duplex = <1>;
> > ++ vlan-tag = <2 0x20>;
> >Ideally, fixed-link should be used instead of custom "force-speed".
> [Ram]: I think both should be same, just different way to define it.
Well, you could try to mark them (qcom,forced-speed, qcom,forced-duplex)
as deprecated. Altough, Rob or Mark will probably tell you to remove
those, since the essedma node isn't present in the upstream kernel.
> >The custom VLAN stuff ( phy-mdio-addr, vlan_tag) should be removed.
> >The userspace will deal with setting up the switch.
> [Ram]: User can setup the switch but here are some default values
> populated. Vlan tag for WAN group is 2, for LAN group is 1. This is
> pre-determined and populated with default values here. This can be
> changed from user space too. phy-mdio-addr is for enabling WAN link
> detection.
VLAN configuration is performed by userspace. Of course, if you want,
you can do some of this with DSA as well.
> > +--
> > +2.7.2
> > diff --git a/target/linux/ipq806x/profiles/00-default.mk
> > b/target/linux/ipq806x/profiles/00-default.mk
> > index 9baa24f..c4b58a2 100644
> > --- a/target/linux/ipq806x/profiles/00-default.mk
> > +++ b/target/linux/ipq806x/profiles/00-default.mk
> > @@ -1,7 +1,8 @@
> > define Profile/Default
> > NAME:=Default Profile
> > PRIORITY:=1
> > - PACKAGES:=ath10k-firmware-qca99x0 ath10k-firmware-qca988x
> > ath10k-firmware-qca9984
> > + PACKAGES:=ath10k-firmware-qca99x0 ath10k-firmware-qca988x
> ath10k-firmware-qca9984 \
> > + ath10k-firmware-qca4019
> > Currently, ath10k-firmware-qca4019 gets added automatically by the ipq-wifi
> > board package: <https://github.com/lede-project/source/blob/master/package/firmware/ipq-wifi/Makefile>
> >
> > As this is supposed to be the reference board. the data should be part of
> > ath10k-firmware's board-2.bin. However, other platforms aren't that lucky.
> >
> >This issue has been reported to ath10k-devel and linux-wireless:
> ><https://www.mail-archive.com/ath10k@lists.infradead.org/msg06154.html>
> >
> >And it would be nice to have a official solution for this.
> [Ram]: I did not use ipq-wifi during validation. Currently ath10k needs calibration
> data which are stored in ART partition. We cannot have common board-2.bin for all
> boards as calibration data will differ per board.
Please read Kalle Valo's reply in the thread linked above:
https://www.mail-archive.com/ath10k@lists.infradead.org/msg06563.html
"I haven't followed the discussion very closely, so I might be way off,
but for laptop SMBIOS implementations Waldemar added a variant field to
board-2.bin so that we can have multiple images for the same subsystem
id. Could it help here also?"
The ath10k has already introduced a way to make this work. I would suggest
you bring this to the attention of your managers and let them work this out
together with the ODM that decided to go for different PAs/RF Layouts.
The provided board-2.bin (from ath10k-firmware repository) simply does not
work for most of the released customer devices: Asus RT-AC58U, Zyxel NBG6617,
Meraki MR33, Netgear Orbi and the TP-Link Deco (M5?).
Again: There needs to be a working solution for all devices... and not just
the reference boards.
Regards,
Christian
More information about the Lede-dev
mailing list