rockPi-S patchset overwriting mainline device tree

The PR is

Could you give it a quick review and approve?

You mentioned that you have some older RockPi S boards. What rev are they? (v1.2, older?!)

Right now, I’ve got the RockPI-S build optimized for the older silicon because I’m unsure whether the new CPU operating points will work with the old.

If you could verify that the new operating points work with the old silicon at least well enough to boot, I’d prefer to default to those. Folks with the older boards would be advised to apply a user overlay to switch to the old operating points. Those old boards (before v1.3) have been out of production for some time now.

It’s done. I’ll submit a PR in the next few days, which I assume you’ll review.

I could not easily locate a prefix of patches that would yield the six month old upstream .dts file. So, I took the opportunity to review the Armbian .dts and created a single new patch on the upstream .dts that replaced the dozen or so tiny ones that had been building Armbian’s version.

Unsurprisingly, I found some cruft in the course of this, which is why I’m holding off submitting the PR.

If one were to simply delete those first Armbian patches in the sequence up until the one that resulted in the version the mainline copied, the resulting fully patched .dts should end up being identical. Would this be sufficient?

That would be how I would go about it. Those patches were a drag and drop from some Radxa repo AFAIK and have historically broken other boards and required some post-fixing already. I see them as a continued risk if not periodically reviewed/updated. I know you inherited it, and aren’t responsible for the sloppy execution.

The other boards affected are mostly not supported, Helios64 is being looked at by manofthesea, the roc-rk3328-pc is something in Oleg’s special projects kernel and not a real part of rockchip64 (unless Jock wants to maintain it for special non-media builds), meaning it can just be deleted outright. The nanoPi R4S… well hopefully littlecxm is able to clean that one up as well.

With kernel 6.1 expected to be LTS, the work to “tame Rockchip64” needs complete before that is shifted to “current” from “edge”. I assume this will occur at the next release cycle. That said it looks like that’s slated for February, so it isn’t the tightest timeline ever to adjust patches to the current “state of the kernel”, 6 weeks working time excluding the holidays.

Do I understand correctly that you intend to disable support for the Rock Pi-S if I do not refactor the patches to your liking in time for the 2023 release? When in 2023 would that likely be?

That certainly is not my first choice, and obviously not my ultimate decision as this is a benign-ish dictatorship under Igor. However my understanding was maintenance of the codebase falls under board support responsibilities, and if I have to do the refactor I can’t guarantee satisfactory results, my Rock Pi S is a pre-release and I don’t have any real world applications to test it with. I can’t let the patchset “bit rot”, and I can’t risk screwing up the refactor.

Ideally we’re pushing to have fewer and fewer patches as time goes forward on any given board, this makes the entire system easier for everyone.

[I was confused about when rk3308-rock-pi-s.dts was added. There are rk3308-rock-pi-s.dts.orig files even in the legacy kernel source directories generated by the Armbian build tool. They must have simply been the result of incremental patches ]

You seem to have created the file with the board-rockpis-0001-arm64-dts-rockchip-add-ROCK-Pi-S-DTS-support.patch on 12/13/20

[dfd5cf9692e97774f7f0bfd72227144e36f58070]

As you observed, this is a structural change that impacts many boards. If you flag it independently as a high priority task for each impacted maintainer, some may work on it – each likely coming up with different refactorings and naming conventions.

Would it not be better to provide an example refactoring for at least one such board that other maintainers can be encouraged to follow?

I agree that the Armbian Rockchip patches have become a tangled mess, however, I don’t see why this has to be cleaned up for a 2023 release. This deadline seems arbitrary. What’s driving it?

Do I understand correctly that you intend to disable support for the Rock Pi-S if I do not refactor the patches to your liking in time for the 2023 release? When in 2023 would that likely be?

In the case of , it occurs to me that the relevant .dts was only added a few months ago and appears to have been copied verbatim from Armbian at the time. If one were to simply delete those first Armbian patches in the sequence up until the one that resulted in the version the mainline copied, the resulting fully patched .dts should end up being identical. Would this be sufficient?

Yes, that is the device tree.

The official Rock Pi S device tree was added to Mainline 6 months ago, I don’t understand your determination it’s an obsolete DTS. The use of an unrelated DTS as the base for Armbian mainline builds is inappropriate and must be discontinued. This same task will be falling on all the other boards that have likewise had a device tree added to mainline since their introduction. (Helios64, NanoPi R4S, RK3328-roc-pc)

Patching the file is fine and necessary, at the moment the Armbian system is deleting the mainline device tree, substituting one of “unknown” origin and accuracy (some hacked up Radxa stuff that came in at the same time as several breaking source changes). This results in more patches and a high effort on the teams part.

Deleting the now unnecessary (and misnamed) “board-rockpis-0001-arm64-dts-rockchip-add-ROCK-Pi-S-DTS-support.patch” will result in a cascade of failures in the rest of the series that need to be adjusted for. My intention is to have all these patch problems/future maintenance issues cleaned up before the 2023 release, or disabling the boards that aren’t. I don’t have time to step through this activity before the next release when I have my own boards to look after, and would rather not disable the board. ← Why it’s high priority.