Merge work from other contributors

[RichardFevrier]

Thank you for all the work you have already made 🎉

I was wondering if you would consider merging the work from @cooper151288 and @WinkelCode since they both produced a lot of work too 🙏

[WinkelCode]

Hi,

I'm the person who made the RPM/akmod and AKMS (Alpine Linux) scripts (https://github.com/WinkelCode/it87)

I did the project in early 2022 to get it to work on Fedora Silverblue (which it does last time I checked), and also a bunch of additional scripting, which may or may not be unnecessarily complicated.

Basically the idea of my scripts is to make installation of the module easy without actually having it in the distro's repos, which I don't know if there is enough interest in.

My development on these scripts kind of stalled in a weird spot, I have this stupidly over the top script for RPM, and this lighter one for Alpine. Before I stopped working in it, I was looking into "synchronizing" both (maybe the DKMS one too) scripts to be more similar, possibly just merging them into one. I was also looking into refactoring the documentation (the README mainly) to be more approachable.

I'll take this issue as a sign to finally finish the project and submit a pull request here once I'm done (changes to documentation optional). My script for RPM actually doesn't need to be alongside the source files, it can grab them from a given repository, so I could alternatively split it off entirely.

[RichardFevrier]

Hi @WinkelCode,

Thank you for your detailed answer!

My daily driver is Fedora Silverblue this is the reason that I found all the work you have done. I also found this repo where I think @frankcrawford tries to stay up to date and in conformity with the official version.

Trying to regroup all of your efforts, that is the reason that I have opened this issue in the first place 🙂

[frankcrawford]

Folks,I’m currently on holidays, and away from my systems, but when I get back I am keen to look at this.Foe reference, I am currently trying to get a lot of the recent driver updates into the kernel tree, but am also willing to add other things into my repo.FrankSent from my iPadOn 15 Nov 2022, at 4:46 am, Richard Février @.***> wrote: Hi @WinkelCode, Thank you for your detailed answer! My daily driver is Fedora Silverblue this is the reason that I found all the work you have done. I also found this repo where I think @frankcrawford tries to stay up to date and in conformity with the official version. Trying to regroup all of your efforts, that is the reason that I have opened this issue in the first place 🙂

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

[frankcrawford]

@RichardFevrier Okay, I'm back from holidays now, and can spend a bit of time looking at your suggestions, and I'm fine with it, although obviously need the assistance of the others.

Firstly @WinkelCode I'm fine to add you mods in, especially as it looks like your changes are slightly orthogonal to mine, in that I haven't really spent much time on it, and happy to get something better in for it. So, I guess what I am saying is raise a PR and I'll be happy to merge it in.

As for the code from @cooper151288 I'd be happy to merge any changes, but I don't see much aside from the README file. If there is any other practical changes let me know.

[cooper151288]

I'll briefly mention what I intended to do with my fork, which fell by the wayside before any real development could take place:

Main purpose was to add more features to the driver, namely the closed-loop fan control.

I've tested this function on a system with an IT8728F chip, so if you want advice as to what registers to poke to get it working, I can help with that.

You can safely disregard the changes on my fork if you aren't aiming to add this feature.

On Mon, 21 Nov 2022, 11:27 Frank Crawford, @.***> wrote:

@RichardFevrier https://github.com/RichardFevrier Okay, I'm back from holidays now, and can spend a bit of time looking at your suggestions, and I'm fine with it, although obviously need the assistance of the others.

Firstly @WinkelCode https://github.com/WinkelCode I'm fine to add you mods in, especially as it looks like your changes are slightly orthogonal to mine, in that I haven't really spent much time on it, and happy to get something better in for it. So, I guess what I am saying is raise a PR and I'll be happy to merge it in.

As for the code from @cooper151288 https://github.com/cooper151288 I'd be happy to merge any changes, but I don't see much aside from the README file. If there is any other practical changes let me know.

— Reply to this email directly, view it on GitHub https://github.com/frankcrawford/it87/issues/4#issuecomment-1321908896, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD5PMVT67EM2SF7QT3WT7F3WJNMBJANCNFSM6AAAAAAR74PGHQ . You are receiving this because you were mentioned.Message ID: @.***>

[frankcrawford]

@cooper151288 for the moment, I may leave it aside, as I try to get the drive updated in the current kernel with all the existing changes.

However, I am interested in getting it added in a future version, so I may check what changes are required.

Thanks.

[frankcrawford]

@WinkelCode just out of interest, has there been any updates on your AKMS work?

[WinkelCode]

@WinkelCode just out of interest, has there been any updates on your AKMS work?

Sorry, stuff got in the way and then I got kind of sidetracked. I'm back working on it as I'm writing this.

[WinkelCode]

@frankcrawford Would you say that there is a good possibility that this out-of-tree module will still be useful in a few months? I've been thinking about just straight up writing the CI/CD stuff to automatically build and put RPM/DEB/APK packages in the repo's releases.

Also, my scripts currently default to creating these entries:

# /etc/depmod.d/it87.conf
# Slightly different for AKMS, etc.
override it87 * kernel/extra

# /etc/modules-load.d/it87.conf
it87

# /etc/modprobe.d/it87.conf
options it87 ignore_resource_conflict

Especially the ignore_resource_conflict is needed on my Gigabyte B550 Vision D's IT8688E chip, is that a good default to have, or should we handle it differently? I guess I could make it build two different packages, or an extra package?

Edit 2: I've pretty much decided I'm going to pursue the CI route. However, the question whether ignore_resource_conflict should be a default is still open, I am not sure how problematic it could be? I could just make an extra package that simply enables it, what do you guys think?

[frankcrawford]

@WinkelCode yes, I plan to keep this going for a while, and even is I update the in-tree module, this will always have a few items not accepted there.

As for ignore_resource_conflict it is pretty much essential for all recent Gigabyte motherboards. If you do some simple tests for me, I can add it into the DMI match so it is automatically set (something that will never be in the in-tree version).

Technically, we should not just set it, as there are cases where ignoring ACPI conflicts can cause issues, but I'm pretty sure that won't happen with this particular one, as Gigabyte have done nothing with it.

[WinkelCode]

Good news: I am pretty much done. Unfortunately it took much longer than I had hoped, mostly because I kept running into incredibly annoying and obscure issues which kept killing my motivation, but I managed to either fix or work around most of them.

Quick rundown:

• packagetool.sh: Builds apk, rpm and deb packages in a container, then makes the respective dynamic module builder build them, checks if they were built (modprobeing the file), then uninstalls the package to make sure the module is properly removed (I added that one because Alpine's apk wouldn't tell AKMS to uninstall it).
• Reusable GitHub Actions workflow for packagetool with build caching.
• Spec files for RPM, APKBUILD and AKMBUILD for Alpine. Alpine packages are extracted to additionally deliver package-less files for manual installation in build artifact (no such capability with akmods).
• Alpine and RPM use the following structure: Userland package with depmod (override built in module) and modload (always load module) configuration, package for the dynamic build system (AKMS/akmods), and a ignore_resource_conflict package with the self-explanatory modprobe entry. Alpine also has a separate 'doc' package as is convention. I made sure to always also include the LICENSE file along the source code (I added the GPLv2 license to the repo, as was already specified in the source code).
• Debian is unchanged as I see it is already being maintained, might benefit from some config files from the userland packages, but I am done with learning new package systems for now 😅
• Added an option to the Makefile to specify the module version as an argument

Notes:

• I am not 100% sure on the compliance with conventions regarding the RPM and Alpine packaging, but it's not going into the repos so it doesn't matter too much.
• The tool handles the current commit and current date as required, but i haven't implemented anything for "proper" versions. I am under the impression that while two tags exist, this fork was never properly versioned.
• The kernel module gets the same version format as the package.
• Podman should "just work" with root permissions (the module building during testing usually needs special privileges).
• Docker needs to have the buildx thingy enabled so it can use the type of caching we need for CI. I could add an option so it also works without buildx, but supposedly that's depreciated and it doesn't support caching (idk what's going on over there exactly).
• Fedora Immutable users should be able to just clone the repo, run packagetool.sh --container_runtime=podman --package_system=rpm and grab the .rpms from the .release directory.

Right now I am double checking stuff and finishing up, I'll have to test the packages on my bare metal machine, and make sure the package build files can be correctly used without the packagetool.sh wrapper.

[frankcrawford]

@WinkelCode

Thanks for that. Once you are happy, lets roll it into this repo.

As you noted, it is unlikely anything here will be picked up directly by any of the distributions, so something that works and is close to their standards is good enough.

I'll probably run up an Alpine VM sometime just to make sure it installs, but won't have any hardware running Alpine to test, so I am relying on you (and the wider community) to note any issues.

And again, thanks for the work.

Regards Frank

[WinkelCode]

I created a draft pull request here: https://github.com/frankcrawford/it87/pull/9

Packaging is done but I need help making sure the README is up-to-date with the current status of the module.

[frankcrawford]

@WinkelCode how is this going? I haven't really looked at it lately, but it seems to not have changed since May.