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

Convert the project from using user-patches back to a tranditional patch #24

Open
ChristineBoersen opened this issue Feb 5, 2024 · 14 comments

Comments

@ChristineBoersen
Copy link

ChristineBoersen commented Feb 5, 2024

Which feature would you like to have?

Goal

  • All of the current user-patch related code moved into a normal package
  • With any luck, if community desired, get this back-ported to mainline as a possible configuration for mkspi?

Funding

  • I'm offering to write an initial version of this and assist with maintenance presuming this is low level of effort
@redrathnure
Copy link
Owner

Oh yeah, I was thinking the same thing. I would happy to do this in right way.
However I have a doubt about going back to official Armbian repo. I remember an article with pretty strict requirements to introduce new board. I am not sure it is possible for me because a) I am not affiliated with MKS and spend time very little time to this project (and on non regular basis) and b) I do not really gave access to the hardware documentation and original patches ( not sure we can do it 100% worked with publicly available bits of info)

@ChristineBoersen
Copy link
Author

Yeah..

We can still do it a "more right" way without to much trouble I believe (without going as far as being "conforming" but "in the spirit of".

I believe and was going to try to use this utility (VS Code extension Git Patch, as I believe it will allow something along the lines of

  • make a patch from loose unmatched files and/or combine patches into a single patch file.
  • This would allow the current patches you have to be "rolled up" into a single patch, and we place it in the normal patch dir, named "in the spirit" of how the rest of the patches are.

I already prototyped this once just moving the patches (ones I had from previous effort I was working independently) into the normal patch dir and almost had it working but there were some issues in how I did it (general mistake, unrelated).

Then go forward, just diff/patch like normal.

@redrathnure
Copy link
Owner

Well, perhaps we have different definition of "more right" way:) Let's discuss this a bit.

  1. I would assume that kernel patches from userpatches directory should be moved to common patches directory. Main question where is the right place? And see also the next point
  2. Now my patches kinda override Renegade board. Means if I would build two images (for Renegade and MKSPI boards) from the same sources I would get only one worked. So another one question is how to declare fully separated board but consume parches from Renegade board on the same time (I won't to rewrite everything for rockchip64 platform). This is related to kernel config files too.
  3. Perhaps it's better to have separated configuration for MKS PI/SKIPR and may be other boards.

Another one task to adapt all these point to the current project structure. I try to rebase my changes over fresh armbian/main and looks like patch locations were significantly changed.

And I am against the idea of ​​keeping all changes in single file. It's better to have a few "topic related" files rather than one single blob.

BTW are you aware about ./compile.sh ... kernel-patch command arg? It gives you possibility to change a code and then prepare proper patch file automatically. Pretty handy tool.

@ChristineBoersen
Copy link
Author

1/2 were where I stopped on my solo attempts. It was getting the timing of the patch insertion when coming from latest.

What I had working at one point before deciding to combine my efforts here was/were

  • Started from latest with clean pull
  • Pulled (from makerbase-mks) their original patches similar to what you did, minus the last spi patches
  • Where I stopped was 2-ish 3 regading getting my patches at the right spot AFTER the config changes

Coming from Renegade actually makes the most sense since that is what the HW probes think it is, and that this is really a "subvariant"

No problem about the single file, I probably should have said single "cleanup commit" irrespective of number of files.

Definitely the kernel-patch command is handy :)

@ChristineBoersen
Copy link
Author

Actually, we may want to use a newer feature from this armbian#5124

It adds (as one of the newer *-patch commands) uboot-config, This outputs a defconfig at the end, instead of a patch.

Since that's in uboot, we would then be able to take that defconfig (having the current userpatches in it), and use that defconfig for future builds instead of the normal instead of the current Renegade ( roc-cc-rk3328_defconfig ) one.

@redrathnure
Copy link
Owner

But we have no issues with u-boot, patches already in right place. Do you mean kernel-config?

@ChristineBoersen
Copy link
Author

Actually (after relooking last night and starting an out of band from armbian/build tag 23.11.2.) you are right, the defconfig file can remain untouched. Let me see where I get from this out of band attempt.

@ChristineBoersen
Copy link
Author

Ok, doing the above, I was able with exceptionally minimal changes, take 2 files from your build, put them into the appropriate place in /patch dir, on 6.1 upstream armbian/build tag 23.11.2.

I need to debug why 6.6 has a build error but it is obviously from a Realteck wifi patch not going properly which may be as easy as disabling the xtra wifi drivers.

I took things as far as communicating over CANBUS via USB patch between MCU and "pi" sides, with, klipper in usb/can bridge mode for the MCU's bin

My testing steps were to

  • Clone to empty folder upstream armbian/build tag v23.11.2
  • Copy an armbian-mkspi adjusted mkspi.conf to the same folder in armbian/build. The only adjustment is to comment one line for FS type.
####BOOTFS_TYPE="fat"

  • Copy userpatches/zz-mkspi-kernel-rockchip64-current.patch to patch/kernel/archive/rockchip64-6.1/zz-mkspi-kernel-rockchip64-current.patch

6.1 works, 6.6 doesn't build yet (wifi patch breaks things)
./compile.sh BOARD=mkspi BRANCH=current INSTALL_HEADERS=yes BUILD_KSRC=yes INSTALL_KSRC=yes EXTERNAL=yes EXTERNAL_NEW=compile BSFREEZE=yes RELEASE=bookworm DISABLE_IPV6=false KERNEL_CONFIGURE=no

This obviously is still preliminary testing, but you will note, this is only using one partition (one of the major changes) which is why the removal of the FS line is needed in the conf.

More info as I get to test more. By doing this, the only patches I don't have from your work appear to be the a display you added, and the display modes added. Both would be easier to just reapply with non-duplicate indexes.

@redrathnure
Copy link
Owner

Would you mind to open PR to show these changes? And have you consider to use latest armbian master or mater from this repo? Asking because Armbian recently changed patch dir structure.

@ChristineBoersen
Copy link
Author

The only reason I haven't proposed a PR yet is because the PR should be (techinically) be against armbian/build, since that's all I'm modifying, just adding 2-3 files from this repository. Doing this to prove how close they are, should this repository rebase back to armbian/build, add the few files to bring functionality back to where this repository is, right this minute, then maintain from there forward.

That's what I was trying to get across by "quickly" doing. Essentially putting it where it would be IF it was in upstream. I hope that made some sense. Been working too many hours this week :)

Also, for clarity I STARTED from latest github.com/armbian/build branch main (not master, that is archive) tag 23.11.2. I then made the couple of changes I mention above.

Also, Disabling EXTRAWIFI did exactly as I expected and build of 6.6 is continuing as we speak :)

@ChristineBoersen
Copy link
Author

Ok, 6.6 compiled with no issue, I am able to talk to both sides of the board via USB->CANBUS as canbus device and printer shuts down as expected since I haven't wired my printer's heater and temp sensors yet since I have it setup with a Stealthburner head and am going to swap the head and x axis to CF before

Looks like it is time to wire my printer and take testing to the next level. Once i've proved it all talks to "stock" ish sensors, I'll hook up my EBB42, get CAN connection working there, and swap heads.

@redrathnure
Copy link
Owner

redrathnure commented Feb 17, 2024

For me it's pretty difficult to catch up what have you done. this is why I asked about PR.
Anyway a few hints about testing:

  • usually I double check TS35 screen, including possibility to flip it
  • TS35 touch (in my case I just install Klipper +Klipper Screen and check how it works)
  • Ethernet
  • UART to connect ADXL345
  • MKS IPS50 screen (proper HDMI modes)

Please pay attention to:

  • TS35 need custom kernel module (kernel src patch + changes in kernel config)
  • all these patches are not available in MKS repo. It's better to use one of branches from this repo If you need this functionality it
  • TS35 touch had (still have?) problems with 6.6 kernel

And looks like original Armbian repo restructured kernel patches recently. Seems like Renegade related stuff was moved to archive :)

@ChristineBoersen
Copy link
Author

What would make the most sense

  • Have you clone https:/github.com/armbian/build, branch main, tag 23.11.2 for your HEAD
  • I do the pull against THAT repository (essentally change my origin and pull/push to you)
  • Then we can add back secondary functionalities that I might not have moved over (my primary concern was working platform and I've been working too much on the "day" job (14+ hr days)

Normal operation of board works (i.e. run klipper, connect via CANBUS to the "printer" side)

  • Ethernet port should work since that's in the main dts changes (rk3328*)
  • TS35 "should" work as I believe that is mainlined code
  • Those 2 HDMI modes need to be added back/confirmed in hdmi.c and friends That will be a headache since the main project keeps adding so numbers will have to be moved around (If it doesn't require contiguous numbers, consider a high "magic" number so it doesn't need constant maintenance)
  • UART for ADXL345, code needs to be merged still, wasn't high on my list since I will be using the sensor on my toolhead vs the one on the board
  • Any patch I applied was directly from YOUR repo

@redrathnure
Copy link
Owner

Have you clone https:/github.com/armbian/build, branch main, tag 23.11.2 for your HEAD

Yes, I periodically sync with this repo. my armbian/main points to https:/github.com/armbian/build/main. Each feature branch is cut from my armbian/main, each release has own tag and when I sync with origin armbian, I rebase current feature branch over armbian/main. My main points to the most stable release/release tag.

So if you would nee "clean state" I would branch ether from my armbian/main or from https:/github.com/armbian/build/main

I do the pull against THAT repository (essentally change my origin and pull/push to you)

See text bellow.

Then we can add back secondary functionalities that I might not have moved over (my primary concern was working platform and I've been working too much on the "day" job (14+ hr days)

Sure, after proper testing

Normal operation of board works (i.e. run klipper, connect via CANBUS to the "printer" side)

Ethernet port should work since that's in the main dts changes (rk3328*)

OK, but keep in mind the original definition has 2 Ethernet channels. You need disable the first one and enable the second (gmac2phy if I remember correctly)

TS35 "should" work as I believe that is mainlined code

Totally wrong. Mainline code do not have support for st7796. They have something similar, but that driver cannot rotate image, which sometimes is needed. tsc2046 touchscreen is full surprises too. IRQ definition should be adjusted for kernel after 6.4. Please see changes for 0.3.3... tag.

Those 2 HDMI modes need to be added back/confirmed in hdmi.c and friends That will be a headache since the main project keeps adding so numbers will have to be moved around (If it doesn't require contiguous numbers, consider a high "magic" number so it doesn't need constant maintenance)

Yeah, for some reasons proper HDMI communication gone from 6.x kernels (5.x was able to read EDIT properly, 6.x not). SO we need this headache to keep using yet another great MKS device :)

UART for ADXL345, code needs to be merged still, wasn't high on my list since I will be using the sensor on my toolhead vs the one on the board

What do you mean? DO you have non standard pins for UART?

Any patch I applied was directly from YOUR repo

Be careful, I removed couple of patches in scope of 0.3.3... and will update DTS mapping in scope of 0.3.4 (touchscreen fix)

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

No branches or pull requests

2 participants