Skip to content
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

Device Tree Overlays Loadable at Run-Time #62

Closed
greenbreakfast opened this issue Jul 17, 2023 · 10 comments
Closed

Device Tree Overlays Loadable at Run-Time #62

greenbreakfast opened this issue Jul 17, 2023 · 10 comments
Assignees

Comments

@greenbreakfast
Copy link
Contributor

Context

openwrt-22.03

Motivation

By being able to dynamically add hardware support to any running firmware just by installing packages, this opens the door for easier support of more use cases for the Omega2.

With this, users can avoid:

  • Having to build fully custom firmware to configure specific hardware functionality
  • Having to maintain multiple DTS files

With the current static device tree setup, if a user wants to make a change in the device tree, they have to build a completely custom firmware using the build system. Not only does this take time to first figure out and then compile, but it also limits the number of people who are able to make these types of hardware configurations changes on the Omega2.

Description

Add support for runtime loading of device tree overlays to the Omega2 openwrt-22.03 firmware

The end-result: when device tree overlay definition files are placed in a specific location in the filesystem, on the next boot, the overlay should be loaded.

Specifics

  • Add patches required to support DT overlay to buildsystem wrapper
  • Create a package that can be installed with opkg to enable software SPI - as a demo

Alternatives Considered

This is the best path forward.
Dynamic device tree overlays offer flexibility for all users. This also gives us a chance to create packages around some of the most common hardware functionality requests that device tree changes.

Much better than statically having to define DTS files and recompile firmware from scratch.

Additional Resources

See this post by @plan44 on the Onion Community: https://community.onion.io/topic/4993/beta-firmware-update-nodejs-v16-19-openwrt-22-03-3-pwm-and-more/4

Reference to existing IPK package file?

Implemented previously by @plan44
See patches 251 to 253 in https://github.com/plan44/plan44-feed/tree/main/p44b-onion/p44build/global-patches

@jempatel
Copy link
Collaborator

  • Flashed firmware
  • After boot, triggered opkg update
  • Checked available packages using opkg list | grep onion. It listed onion-dt-overlay-sw-spi
  • Installed onion-dt-overlay-sw-spi and reboot
  • Wait for the complete boot and check the console log If the device tree is loaded or not. It shows a log message as seen in the screenshot - applying device-tree overlay 'sw-spi' -
  • After login, check the status of dtbo in configfs in the screenshot, cat /sys/kernel/config/device-tree/overlays/sw-spi/status

screenshot_from_2023-09-13_10-34-10
screenshot_from_2023-09-13_10-34-35

@greenbreakfast
Copy link
Contributor Author

great work @jempatel 😃
closing this issue

@kishangondaliya
Copy link

Build fails if package "onion-dt-overlay-sw-spi" is enabled to include in the build. Is this expected?

@kishangondaliya
Copy link

Also, is below issue related to this package? I don't think so but still confirming.

[ 0.355272] spi-mt7621 10000b00.spi: sys_freq: 193333333
[ 0.384812] ------------[ cut here ]------------
[ 0.389526] WARNING: CPU: 0 PID: 1 at drivers/mtd/spi-nor/core.c:2863 spi_nor_init+0x17c/0x184
[ 0.398363] enabling reset hack; may not recover from unexpected reboots
[ 0.405180] Modules linked in:
[ 0.408287] CPU: 0 PID: 1 Comm: swapper Not tainted 5.15.150 #0
[ 0.414310] Stack : 00000000 00000000 80c23944 80870000 806c0000 80613540 80c3b280 806b9e03
[ 0.422843] 808733b4 00000001 809c8c7c 80061a48 8060cc5c 00000001 80c23900 b28cf387
[ 0.431370] 00000000 00000000 80613540 80c23798 ffffefff 00000000 00000000 ffffffea
[ 0.439897] 00000000 80c237a4 00000049 806c0298 80870000 00000009 00000000 80378cf4
[ 0.448411] 00000009 809c8c7c 80650000 80e34400 00000018 8033b394 00000000 80870000
[ 0.456933] ...
[ 0.459422] Call Trace:
[ 0.461909] [<8000702c>] show_stack+0x28/0xf0
[ 0.466364] [<80026228>] __warn+0x9c/0x124
[ 0.470547] [<8002633c>] warn_slowpath_fmt+0x8c/0xac
[ 0.475595] [<80378cf4>] spi_nor_init+0x17c/0x184
[ 0.480387] [<80379588>] spi_nor_scan+0x7a4/0xba0
[ 0.485171] [<8037a2c8>] spi_nor_probe+0x94/0x310
[ 0.489963] [<8034c8ac>] really_probe.part.0+0xac/0x354
[ 0.495279] [<8034cde0>] driver_probe_device+0x4c/0x154
[ 0.500599] [<8034d438>] __device_attach_driver+0xd0/0x15c
[ 0.506176] [<8034a524>] bus_for_each_drv+0x70/0xb0
[ 0.511171] [<8034d15c>] __device_attach+0xe0/0x180
[ 0.516130] [<8034b7f8>] bus_probe_device+0xa0/0xbc
[ 0.521096] [<80347f9c>] device_add+0x3e4/0x8e0
[ 0.525709] [<80384bd4>] __spi_add_device+0x90/0x178
[ 0.530773] [<80384d1c>] spi_add_device+0x60/0x9c
[ 0.535556] [<803850e8>] of_register_spi_device+0x21c/0x364
[ 0.541229] [<80385808>] spi_register_controller+0x5d8/0x7dc
[ 0.546981] [<803882d8>] mt7621_spi_probe+0x168/0x220
[ 0.552130] [<8034f18c>] platform_probe+0x50/0xa4
[ 0.556923] [<8034c8ac>] really_probe.part.0+0xac/0x354
[ 0.562242] [<8034cde0>] driver_probe_device+0x4c/0x154
[ 0.567553] [<8034d570>] __driver_attach+0xac/0x1ac
[ 0.572518] [<8034a478>] bus_for_each_dev+0x68/0xa4
[ 0.577482] [<8034bacc>] bus_add_driver+0x150/0x238
[ 0.582450] [<8034deb4>] driver_register+0x98/0x154
[ 0.587409] [<80000638>] do_one_initcall+0x50/0x1b4
[ 0.592377] [<8074a0e0>] kernel_init_freeable+0x1d0/0x26c
[ 0.597875] [<80579600>] kernel_init+0x20/0x108
[ 0.602511] [<80002478>] ret_from_kernel_thread+0x14/0x1c
[ 0.608005]
[ 0.609558] ---[ end trace 542a901b67c5a14f ]---
[ 0.614747] spi-nor spi0.0: w25q256 (32768 Kbytes)
[ 0.619722] 4 fixed-partitions partitions found on MTD device spi0.0
[ 0.626287] OF: Bad cell count for /palmbus@10000000/spi@b00/flash@0/partitions
[ 0.633792] OF: Bad cell count for /palmbus@10000000/spi@b00/flash@0/partitions
[ 0.641739] OF: Bad cell count for /palmbus@10000000/spi@b00/flash@0/partitions
[ 0.649213] OF: Bad cell count for /palmbus@10000000/spi@b00/flash@0/partitions

@plan44
Copy link
Contributor

plan44 commented Apr 8, 2024

You are right, this is not related.

What this warning means is that in that hardware design, the SPI flash address mode is not resetted when a unexpected SoC hard-reset occors, rendering the device unbootable. This is the case for the Omega2(S)+ (but not for the Omega2(S)). The "hack" consists of reducing the time where a hard-reset would do any harm to the absolute minimum possible, so almost (but not 100%) eliminating the real world chance of a boot failure.

This "hack" has been upstreamed and is now a standard feature of the linux kernel, which can be enabled in the device tree (and is, for the Omega2). However the original authors chose to treat it as a not-fully-ok condition and thus emit this long verbose and sometimes confusing warning. That's why I apply this patch which reduces the warning to a single line in my Omega2 builds - there's no point in being that alarmistic about a hardware condition nobody (except a yet to become Omega3, maybe? 😉) can do anything against.

@plan44
Copy link
Contributor

plan44 commented Apr 8, 2024

Build fails if package "onion-dt-overlay-sw-spi" is enabled to include in the build. Is this expected?

What kind of error do you get?

@kishangondaliya
Copy link

cp -fpR /build/build_dir/target-mipsel_24kc_musl/root-ramips /build/build_dir/target-mipsel_24kc_musl/root.orig-ramips
mkdir: cannot create directory '/sys/kernel/config/device-tree': Permission denied
/build/build_dir/target-mipsel_24kc_musl/root-ramips/usr/lib/opkg/info/onion-dt-overlay-sw-spi.postinst-pkg: line 5: /sys/kernel/config/device-tree/overlays/sw-spi/dtbo: No such file or directory
postinst script ./usr/lib/opkg/info/onion-dt-overlay-sw-spi.postinst has failed with exit code 1
make[2]: *** [package/Makefile:74: package/install] Error 1

@plan44
Copy link
Contributor

plan44 commented Apr 15, 2024

I'm not 100% sure I understand the makefile of onion-dt-overlay, but it seems to me it tries to activate the overlay in the postinst script of the package already - probably the idea behind doing this is that the overlay gets active right after package installation, not only after next reboot (when the /lib/preinit/90_apply_dt_overlays will activate all overlays found in /lib/firmware/device-tree/overlays/).

Thing is, if you select a package for being pre-installed in the image (selecting * in menuconfig, not M), the "installation" will happen on the build system, so the package scripts need to be prepared for that by checking IPKG_INSTROOT.

Looking at the makefile lines 71-74, it seems to me that check is in place.

But your build error message show that the build tries to create a /sys/kernel/config/device-tree directory - which fails because on your desktop as a non-root user you certainly don't have permission to install device tree overlays ;-)

So I guess that part of the makefile, lines 71-74 contain a bug, only I don't see it - maybe @greenbreakfast does?

@greenbreakfast
Copy link
Contributor Author

greenbreakfast commented Apr 15, 2024

There's a few reasons why this might be happening:

  1. The software spi device tree overlay package is still in testing/development - see Change selected GPIOs for software SPI DT overlay package #77
  2. There's some issue in the onion-dt-overlay's (rather aggressive) post-install script as @plan44 pointed out

Some history on the dt overlay: Originally the dt overlay needed a reboot to become active 8139af2, but it was changed soon after so that dto's become active right after installation: 8139af2

We will look at completing the sw spi dto package and hopefully that fixes the issue.
In the meantime, @kishangondaliya can you please create a Bug Report new issue so we have a way to track the problem you're seeing?

@kishangondaliya
Copy link

@plan44 yes, that's what I mentioned, if we select the package to be pre-installed, it will create an issue as the postinit script will try to execute in the build system and try to create "/sys" directory, which is not a correct approach.
From the above conversation, I can see testing is done by installing the package after booting the base image and not with the entire image build which included this package.

My approach would be to give a warning to a user in the postinit script that a reboot is required after installation if the package is installed after booting else if the package is in the firmware itself, no postinit is required.

@greenbreakfast Sure, I'll create a Bug report for the same.

P.S. I'm porting everything on OpenWRT 23 and already have a stable build (at least for my purpose). Let me know if I can contribute anything here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants