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

feat: add ST7796 kernel module (fbtft) #7543

Merged
merged 2 commits into from
Dec 7, 2024

Conversation

redrathnure
Copy link
Contributor

@redrathnure redrathnure commented Dec 6, 2024

Description

Add support for ST7796 based LCD module (connected via SPI interface). One of real device example is MKS PI-TS35 display.

How Has This Been Tested?

The changes was tested on MKS PI-TS35 display together with MKS-PI board as part of custom Armbian image. Touch screen uses an ads7846 module.

  • MKS-PI + MKS PI-TS35

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings

@github-actions github-actions bot added size/medium PR with more then 50 and less then 250 lines Needs review Seeking for review Hardware Hardware related like kernel, U-Boot, ... Patches Patches related to kernel, U-Boot, ... labels Dec 6, 2024
@paolosabatino
Copy link
Contributor

Nice, the driver looks like ok, there's a typo though in the heading comment:

 * FB driver for the ILI9341 LCD display controller

Anyway, this should be introduced for rockchip64 edge kernel (6.12) too, otherwise it will get cut off during next current kernel bump.

@redrathnure
Copy link
Contributor Author

redrathnure commented Dec 7, 2024

this should be introduced for rockchip64 edge kernel (6.12) too

Not sure I got this part. Now the patch file in 6.6 folder and 6.12 has a link to it. Do you mean it's pbetter to put a copy of the patch instead symlink?

(Typo was fixed)

@redrathnure redrathnure force-pushed the feat-add-fb_st7796-kmod branch from 83eb214 to 059919d Compare December 7, 2024 18:04
Add support for ST7796 based LCD module (connected via SPI interface). One of real device example is [MKS PI-TS35 display](https://github.com/makerbase-mks/MKS-TFT-Hardware/tree/master/MKS%20PI-TS35).
@redrathnure redrathnure force-pushed the feat-add-fb_st7796-kmod branch from 059919d to 556333d Compare December 7, 2024 18:05
@paolosabatino
Copy link
Contributor

this should be introduced for rockchip64 edge kernel (6.12) too

Not sure I got this part. Now the patch file in 6.6 folder and 6.12 has a link to it. Do you mean it's pbetter to put a copy of the patch instead symlink?

(Typo was fixed)

Ah sorry, didn't notice the symlink in 6.12 and 6.9 directories; Despite I see the symlink being done elsewhere, TBH I would discourage doing so: at one point in time, 6.6 kernel will be dismissed and its patch directory removed. Possible symlinks pointing to 6.6 patch in other patch archives (in your case 6.9 and 6.12) will break. I don't like the idea that removing the 6.6 patch archive directory may affect other patch archives.

@github-actions github-actions bot added size/large PR with 250 lines or more and removed size/medium PR with more then 50 and less then 250 lines labels Dec 7, 2024
@redrathnure
Copy link
Contributor Author

I don't like the idea that removing the 6.6 patch archive directory may affect other patch archives.

Done, symlinks were replaced by copies.

Copy link
Contributor

@paolosabatino paolosabatino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patience, approved!

@paolosabatino paolosabatino merged commit a15a461 into armbian:main Dec 7, 2024
@pykpkg47
Copy link
Contributor

pykpkg47 commented Dec 7, 2024

this commit is breaking git clone see workflow errors https://github.com/armbian/build/actions/runs/12216820229
unable to create symlink patch/kernel/archive/rockchip64-6.12/general-st7796-driver.patch
please revert as soon as possible because automation like https://github.com/armbian/build/blob/main/action.yml
clone the repo first and then switch to selected branch which is broken with this

@redrathnure
Copy link
Contributor Author

this commit is breaking git clone see workflow errors https://github.com/armbian/build/actions/runs/12216820229
unable to create symlink patch/kernel/archive/rockchip64-6.12/general-st7796-driver.patch

Maintainers sync
unable to create symlink patch/kernel/archive/rockchip64-6.12/general-st7796-driver.patch: File name too long

How to fix it? Is it possible to reset master for -2 commits back and then reopen a PR?

@rpardini
Copy link
Member

rpardini commented Dec 8, 2024

Oh wow. How this came to be is beyond me. So the first commit added symlinks, and the second one changed it's contents to a copy of the symlinked, but apparently failed to change the mode of the file; thus the patch contents (which is very large) is interpreted by git as the "symlink target" leading to "file name too long".

Fact that GitHub hides the details makes it not easy at all.

Was @redrathnure using Windows or core.symlinks=off to do this?

To fix this we need force-push powers to main. @igorpecovnik ? (We should drop those 2 commits, then re-do this PR with out the symlinking).

@igorpecovnik
Copy link
Member

We should drop those 2 commits, then re-do this PR with out the symlinking

Hmm, clean checkout failed ... need to figure out how to deal with this, before making more damages.

remote: Enumerating objects: 132023, done.
remote: Counting objects: 100% (1056/1056), done.
remote: Compressing objects: 100% (617/617), done.
remote: Total 132023 (delta 650), reused 745 (delta 432), pack-reused 130967 (from 1)
Receiving objects: 100% (132023/132023), 644.04 MiB | 7.11 MiB/s, done.
Resolving deltas: 100% (89455/89455), done.
error: unable to create symlink patch/kernel/archive/rockchip64-6.12/general-st7796-driver.patch: File name too long
error: unable to create symlink patch/kernel/archive/rockchip64-6.9/general-st7796-driver.patch: File name too long
fatal: unable to checkout working tree
warning: Clone succeeded, but checkout failed.
You can inspect what was checked out with 'git status'
and retry with 'git restore --source=HEAD :/'

@igorpecovnik
Copy link
Member

done.

@redrathnure
Copy link
Contributor Author

A version without symlinks magic: #7550

@paolosabatino
Copy link
Contributor

I apologize for the mess my suggestion caused 😔

@igorpecovnik
Copy link
Member

No worries, we learned something new.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Hardware Hardware related like kernel, U-Boot, ... Needs review Seeking for review Patches Patches related to kernel, U-Boot, ... size/large PR with 250 lines or more
Development

Successfully merging this pull request may close these issues.

5 participants