-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
mx4300 bootargs patch #2
Conversation
22e29f3
to
21c917a
Compare
find-rootblock = "ubi.mtd="; | ||
bootargs-append = ""; | ||
bootargs-match-key-1 = "ubi.mtd=22,2048"; | ||
bootargs-sel-1 = "rootfstype=squashfs ubi.mtd=22,4096 ubi.block=0,0 root=/dev/ubiblock0_0 rootwait ro"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think replacing the entire bootargs command is not a good idea.
It will overwrite any changes you made manually.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can change the match-key to the full boot command passed by the vanilla u-boot config. If the user changed the bootargs in u-boot, then it won't match, and thus replacement won't happen.
In my thought, there are two cases.
- The user doesn't modify the bootargs in u-boot.
- The user changed the bootargs manually.
In case 1, I think a full replacement is appropriate. In case 2, we can assume the user knows what they are doing. We just leave the boot command as it is.
4328963
to
943ecb4
Compare
018877d
to
d42bf52
Compare
943ecb4
to
5a5dd45
Compare
ee6aa76
to
bf70df2
Compare
Rebased and
|
d89569f
to
ed9f006
Compare
I think this commit now satisfies OpenWrt's commit guideline. Let me know if there are any other problems. |
ed9f006
to
3cae3ad
Compare
Thanks for your changes. In the case of
|
I did try that with bootargs-append before, and I recall that the OS will try to attach both ubi. |
I can confirm that the OS will try to attach both. ` ` |
So we should only keep the ubi.mtd that we want to attach in the command line. |
Thanks for the logs.
|
I think the sole purpose of this patch is only to help people smoothly convert their devices from stock firmware to OpenWrt without using a serial adapter. If a simple solution can achieve the goal, I prefer not to do more complex things such as parsing or regexp in the kernel because they are more error-prone. Also, that patch requires hard coding part of the bootargs into the device tree. It parses the command line from u-boot and appends the partition number to the command line hardcoded in the device tree. I think that patch is worse than the current find-and-replace design since the current design allows users to modify the bootargs without recompiling the kernel. In addition, the app2_data and app2 partitions are unused on MX4300. I think some users will like to put the rootfs_data on them and attach them at boot time, which requires modifying the bootargs and is incompatible with that patch. This is just my thoughts. Please let me know what you think. |
Another thing is code reusability. The current design can do more things than fixing the ubi offset, which I think can be helpful for adding support for other devices in the future. For example, it can remove an unwanted argument in the middle of the cmdline. |
3cae3ad
to
bed40f9
Compare
@@ -7,6 +7,19 @@ | |||
/ { | |||
model = "Linksys MX4300"; | |||
compatible = "linksys,mx4300", "qcom,ipq8074"; | |||
|
|||
chosen { | |||
bootargs-append = ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not used now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed
1d99eca
to
28a031f
Compare
@@ -7,6 +7,17 @@ | |||
/ { | |||
model = "Linksys MX4300"; | |||
compatible = "linksys,mx4300", "qcom,ipq8074"; | |||
|
|||
chosen { | |||
// Replace the bootargs from u-boot with find and replace |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect comment in dts. Should be: /* text */
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am going to change it. Although I found // is legit too.
https://devicetree-specification.readthedocs.io/en/v0.3/source-language.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment is fixed and improved.
// Replace the bootargs from u-boot with find and replace | ||
bootargs-find-1 = "init=/sbin/init rootfstype=squashfs ubi.mtd=22,2048 ubi.block=0,0 root=/dev/ubiblock0_0 rootwait ro"; | ||
bootargs-exact-match-1 = "y"; | ||
bootargs-replace-1 = "init=/sbin/init rootfstype=squashfs ubi.mtd=22,4096 ubi.block=0,0 root=/dev/ubiblock0_0 rootwait ro"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to replace only ubi.mtd
in the current implementation?
bootargs-find-1 = "ubi.mtd=22,2048";
bootargs-exact-match-1 = "y";
bootargs-replace-1 = "ubi.mtd=22,4096";
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is possible. But do we need to consider the case that a user modifies the bootargs?
Without using the full command for matching, we may accidentally replace a user's modified bootargs. For example, ... ubi.mtd=22,2048 ubi.mtd=24,4096 ...
suppose this is what the user deliberately wants to do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's why I think the replacement should happen only if the kernel command line is identical to the default one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I think that replacing just the ubi.mtd
parameter is sufficient. There is no perfect solution in this case. If someone adds something to bootargs
but forgets to change the size, the current solution will not work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, now it only replaces the ubi.mtd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
git format-patch
is used to generate patches that are pushed to the Linux kernel, etc.
Some people push such patches directly to OpenWrt: openwrt@ca44690
But you can edit it before adding it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the current version acceptable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no summary of changes in your patch: openwrt@ca44690#diff-653f70c37ed4096e337f0b8bf08b92e60f9a5455d7bca753030bfbeaeadc29a7R17-R18
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After some trail and error, I finally figured out quilt refresh --diffstat
will put the diffstat into the patch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now the summary is included in the patch.
28a031f
to
62f25b2
Compare
75a987c
to
9cc8ee8
Compare
62f25b2
to
52feff1
Compare
52feff1
to
c57d30d
Compare
dfd6644
to
b963796
Compare
Copied the usage of the hack into the commit message. Now it looks good to me. |
b963796
to
e4ddf15
Compare
9775623
to
124fda1
Compare
Add kernel command line replacement hack to qualcommax. Now we can find and replace arguments in the kernel command line by setting bootargs-find-1, bootargs-replace-1, bootargs-exact-match-1 and bootargs-find-2, bootargs-replace-2, bootargs-exact-match-2 under the chosen node in the device tree. This hack replaces the first occurence of bootargs-find-X with bootargs-replace-X. When bootargs-exact-match-X is set to "y", then the replacement happens only if the kernel command line is identical to bootargs-find-X. Signed-off-by: Qiyuan Zhang <[email protected]>
124fda1
to
8c179fe
Compare
QSDK NSS builds utilize skbuff recycling for better handling of memory. On a Dynalink DL-WRX36 (pbuf script should be set to 'auto') a significant drop in memory usage was observed as well consistent sustained RX/TX speeds. BEFORE: echo 3 >! /proc/sys/vm/drop_caches free -m total used free shared buff/cache available Mem: 867 338 547 90 101 528 Swap: 0 0 0 AFTER: total used free shared buff/cache available Mem: 867 242 594 1 81 624 Swap: 0 0 0 NOTE: For 512MB platforms, users need to test with the following scenarios, as the patch `999-233-ath11k-Disable-rx_header-tlv-for-2K-SKB.patch` is really only testable on platforms with 512M or less RAM. 1.) Explicitly setting 'ATH11K_MEM_PROFILE_512M' on and see if system crashes on boot. 2.) Explicitly setting 'ATH11K_MEM_PROFILE_1G' 3.) Remove patches 999-233-ath11k-Disable-rx_header-tlv-for-2K-SKB.patch 999-311-ath11k-configure-nss-thread-priority-during-pdev_ini.patch And re-test with testuser7#1 and testuser7#2 It was incorrectly assumed that setting a 512M for 1G platforms would save memory, instead it needs to be explicitly set to know proper memory regions, otherwise it would cause fw crash. ath11k_nss: fix typo in 512M memory profile ath11k_nss: remove SFE patch 718-e-mac80211-Deliver-the-frame-to-driver-tx-ops-directly It is not relevant to NSS builds and only meant for SFE. ath11k_nss: remove unecessary patches Color collision should be left on by default, as it's a primary feature of 802.11AX. ath11k_nss: fix spacing ath11k_nss: Remove unnecessary TKIP bloat Remove TKIP patches that are not being used as 99% of folks are running modern encryption (AES-CCMP,SAE,etc). ath11k_nss: parameterize DP_RXDMA_REFILL_RING_SIZE memory profile ath11k_nss: Remove SFE related code Cleanup SFE (shortcut fe) related code as we're not using it on NSS ath11k_nss: idr, ampdu, and skb headroom check optimizations ath11k_nss: get valid last_rate for rx_bitrate from cpu stats ath11k_nss: Fix BCCA counter for EMA Currently BCCA counter is updated to FW via csa counter offs and beacon with new countdown is updated for every beacon tx completion event. For EMA, all EMA beacons are updated in one shot, and counter update for every tx event will mess up the actual sequence of countdown sent over the air. Allow FW to update the countdown till 1 and finalize the color change. ath11k_nss: Fix compile for TRACE feature
A patch that can search the original command line and replace it if it matches the key.