help-guix
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: linux and u-boot for pinenote-rk3568


From: phodina
Subject: Re: linux and u-boot for pinenote-rk3568
Date: Fri, 01 Jul 2022 20:42:38 +0000

Hi!

> On 2022-06-30, phodina via wrote:
>
> > I'm attempting to build the Linux kernel and u-boot for ARM64 target -
> > eink tablet.
>
>
> I can infer what you're talking about from the later patches, but it
> would be good to mention what specific hardware you're talking about up
> front and/or in the subject. :)

Sorry for the confusion. I should have stated it's Pinenote eink tablet 
directly.

>
> > Unfotutnately the patches are not yet in the upstream so I had to
> > tweak little bit in the gnu/packages/linux.scm and
> > gnu/bootloader/bootloaders.scm files.
>
>
> Had to do similar things early on to get the pinebook-pro support
> available in guix before it was fully upstream... wheee!

Thanks! I do have Guix running on Pinebook Pro laptop so I'd CC you in the help 
:-)

>
>
> A partial review with comments inline follows...
>
> > Subject: [PATCH 4/7] gnu: Provide private implementation with selection of 
> > the
> > u-boot package.
> > Sent with Proton Mail secure email.

------- Original Message -------
On Thursday, June 30th, 2022 at 9:04 PM, Vagrant Cascadian <vagrant@debian.org> 
wrote:


> > * gnu/packages/bootloaders.scm: Provide private implementation of
> > make-u-boot-package and make-u-boot-sunxi64-package so that the public API
> > does not change. The private allows to specify the u-boot package.
>
>
> This seems unecessarily complicated; you could just use a different
> source for the eventual u-boot-pinenote* packages rather than renaming
> existing functions and/or adding new ones... see what
> u-boot-nintendo-nes-classic-edition does, mentioned below...

Thanks for the hint. I had a look and reimplemented the u-boot. No there is no 
need to provide private implementation!


> > From 44b1385e6dbcc79bafebddc7699ed302fcb5fe91 Mon Sep 17 00:00:00 2001
> > From: Petr Hodina phodina@protonmail.com
> > Date: Thu, 30 Jun 2022 11:20:55 +0200
> > Subject: [PATCH 6/7] gnu: Add install-pinenote-rk3568-u-boot and
> > u-boot-pinenote-rk3568-bootloader.
> >
> > * gnu/bootloader/u-boot.scm (install-pinenote-rk3568-u-boot,
> > u-boot-pinenote-rk3568-bootloader): New variables.
>
>
> The patch order is wrong, this is patch 6/7 followed by patch 5/7
> ... sometimes the order doesn't matter, but this order was actually a
> bit confusing for me. Please submit patches in the appropriate order. :)

This was done by mistake as I build the system image in the last step so the 
order didn't matter. It's fixed in the new patch set.

>
> > diff --git a/gnu/bootloader/u-boot.scm b/gnu/bootloader/u-boot.scm
> > index 6cad33b741..4410cc497a 100644
> > --- a/gnu/bootloader/u-boot.scm
> > +++ b/gnu/bootloader/u-boot.scm
>
> ...
>
> > @@ -127,6 +129,15 @@ (define install-rockpro64-rk3399-u-boot
> >
> > (define install-pinebook-pro-rk3399-u-boot install-rockpro64-rk3399-u-boot)
> >
> > +;; TODO: Supply correct offsets
> > +(define install-pinenote-rk3568-u-boot
> > + #~(lambda (bootloader root-index image)
> > + (let ((idb (string-append bootloader "/libexec/idbloader.img"))
> > + (u-boot (string-append bootloader "/libexec/u-boot.itb")))
> > + (write-file-on-device idb (stat:size (stat idb))
> > + image (* 64 512))
> > + (write-file-on-device u-boot (stat:size (stat u-boot))
> > + image (* 16384 512)))))
>
>
> Does it really use different offsets than other rockchip platforms, or
> is that just a note to confirm if it does?

The TODO note is here more as a warning as I do put it there as a check.

Slight issue here is that the ATF is not released so even though I have gone 
through some patches form downstream and applied them on the upstream I think 
it won't work.

Prefereably I'd like to use it on Quartz64 board which I now use as Guix 
Aarch64 native build machine before flashing to the Pinenote.

>
> > From 4d53d2bdb8526f72ed6cd3183ff2a2141990c900 Mon Sep 17 00:00:00 2001
> > From: Petr Hodina phodina@protonmail.com
> > Date: Thu, 30 Jun 2022 11:19:33 +0200
> > Subject: [PATCH 5/7] gnu: Add u-boot-pinenote-rk3568.
> >
> > * gnu/packages/bootloaders.scm (u-boot-pinenote-rk3568): New variable.
> >
> > diff --git a/gnu/packages/bootloaders.scm b/gnu/packages/bootloaders.scm
> > index cab97852f1..9c266b7bed 100644
> > --- a/gnu/packages/bootloaders.scm
> > +++ b/gnu/packages/bootloaders.scm
> > @@ -834,6 +834,9 @@ (define-public u-boot-pine64-plus
> > (define-public u-boot-pine64-lts
> > (make-u-boot-sunxi64-package "pine64-lts" "aarch64-linux-gnu"))
> >
> > +(define-public u-boot-pinenote-rk3568
> > + (make-u-boot-sunxi64-package-priv "pinenote" "aarch64-linux-gnu" 
> > u-boot-pinenote))
> > +
> > (define-public u-boot-pinebook
> > (let ((base (make-u-boot-sunxi64-package "pinebook" "aarch64-linux-gnu")))
> > (package
>
>
> Definitely not u-boot-sunxi64-package. Probably more similar to the
> u-boot-pinebook-pro-rk3399 or u-boot-rock64-rk3328, which i think use
> the standard make-u-boot-package.

I've applied the change. Haven't built the downstream u-boot yet but I'll check 
the dump from Pinenote.
>
> For using an entirely different source repository for a specific
> package, see the u-boot-nintendo-nes-classic-edition and how that uses a
> different source, e.g.
>
> (define-public u-boot-nintendo-nes-classic-edition
> (let ((base (make-u-boot-package "Nintendo_NES_Classic_Edition"
> "arm-linux-gnueabihf")))
> (package
> (inherit base)
> ;; Starting with 2019.01, FEL doesn't work anymore on A33.
> (version "2018.11")
> (source (origin
> ...
>
> > From a6499c4c384dd3c0e06968cc6987da0e47af6290 Mon Sep 17 00:00:00 2001
> > From: Petr Hodina phodina@protonmail.com
> > Date: Thu, 30 Jun 2022 10:12:39 +0200
> > Subject: [PATCH 1/7] gnu: Add linux-libre-arm64-pinenote.
> >
> > * gnu/packages/linux.scm (linux-libre-arm64-pinenote): New variable.
> > * gnu/local.mk: Add patches.
> > * linux-libre-arm64-pinenote-battery-level.patch: New file.
> > * linux-libre-arm64-pinenote-defconfig.patch: New file.
> > * linux-libre-arm64-pinenote-dtsi.patch: New file.
> > * linux-libre-arm64-pinenote-ebc-patches.patch: New file.
> > * linux-libre-arm64-pinenote-touchscreen-1.patch: New file.
> > * linux-libre-arm64-pinenote-touchscreen-2.patch: New file.
> > * linux-libre-arm64-rockchip-add-hdmi-sound.patch: New file.
>
>
> I'm going to make some comments on this patch, but I get plenty confused
> with patches in gnu/packages/linux.scm as it can be a little more
> complicated than your typical packaging, so any other reviewers really
> ought to jump in too! :)

I've removed some patches as they are not for this kernel release and will need 
more polishing.

>
> > diff --git a/gnu/local.mk b/gnu/local.mk
> > index 353b91cfd2..c19f1f418b 100644
> > --- a/gnu/local.mk
> > +++ b/gnu/local.mk
> > @@ -1444,6 +1444,13 @@ dist_patch_DATA = \
> > %D%/packages/patches/linbox-fix-pkgconfig.patch \
> > %D%/packages/patches/linphone-desktop-without-sdk.patch \
> > %D%/packages/patches/linux-libre-support-for-Pinebook-Pro.patch \
> > + %D%/packages/patches/linux-libre-arm64-pinenote-touchscreen-1.patch \
> > + %D%/packages/patches/linux-libre-arm64-pinenote-touchscreen-2.patch \
> > + %D%/packages/patches/linux-libre-arm64-pinenote-battery-level.patch \
> > + %D%/packages/patches/linux-libre-arm64-rockchip-add-hdmi-sound.patch \
> > + %D%/packages/patches/linux-libre-arm64-pinenote-defconfig.patch \
> > + %D%/packages/patches/linux-libre-arm64-pinenote-dtsi.patch \
>
>
> whitespace inconsistancy on the previous few lines...

It was meant more for the process of porting rather than packing it into Guix 
atm so there inconsistencies are there unfortunately now.
>
> > diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
> > index 8f7b4f4f5b..bae8ffb959 100644
> > --- a/gnu/packages/linux.scm
> > +++ b/gnu/packages/linux.scm
> > @@ -349,6 +349,12 @@ (define (%upstream-linux-source version hash)
> > "linux-" version ".tar.xz"))
> > (sha256 hash)))
> >
> > +(define (%pinenote-linux-source version hash)
> > + (origin
> > + (method url-fetch)
> > + (uri (string-append "https://github.com/phodina/linux-pinenote/tarball/"; 
> > version))
> > + (sha256 hash)))
> > +
>
>
> I think tarballs from github are generally discouraged; although with
> the kernel that can be quite a large git checkout... but...
>
> > ;; The current "stable" kernels. That is, the most recently released major
> > ;; versions that are still supported upstream.

The tarball is smaller than the git checkout. Now there are defines for the 
exact kernel version.

> >
> > @@ -367,6 +373,14 @@ (define-public linux-libre-5.17-pristine-source
> > (%upstream-linux-source version hash)
> > deblob-scripts-5.17)))
> >
> > +(define-public linux-libre-arm64-pinenote-pristine-source
> > + (let ((version linux-libre-5.17-version)
> > + (commit "c91a48e028fe1f6a0e5748fd87c446aa7e31811b")
> > + (hash (base32 "1xwyvvps1r3zl1n9szlgrj8ylw5sgj6fr52fig9f2cc6ai331bbn")))
> > + (make-linux-libre-source version
> > + (%pinenote-linux-source commit hash)
> > + deblob-scripts-5.17)))
> > +
>
>
> Oh, wait, you do pull from git? Is the tarball used, or the git
> checkout? Seems like you shouldn't need both.

I do download the tarball just specify the exact commit rather than to say the 
name of branch as that can change and break the build due to sha has mismatch.

>
> > +(define-public linux-libre-arm64-pinenote-source
> > + (source-with-patches linux-libre-arm64-pinenote-pristine-source
> > + (list %boot-logo-patch
> > + %linux-libre-arm-export-__sync_icache_dcache-patch
> > + (search-patch
> > +"linux-libre-arm64-pinenote-touchscreen-1.patch")
> > + (search-patch
> > +"linux-libre-arm64-pinenote-touchscreen-2.patch")
> > + (search-patch
> > +"linux-libre-arm64-pinenote-battery-level.patch")
> > + (search-patch
> > +"linux-libre-arm64-rockchip-add-hdmi-sound.patch")
> > + (search-patch
> > +"linux-libre-arm64-pinenote-defconfig.patch")
> > + (search-patch
> > +"linux-libre-arm64-pinenote-dtsi.patch")
> > + (search-patch
> > +"linux-libre-arm64-pinenote-ebc-patches.patch"))))
> > +
>
>
> Maybe use:
>
> (search-patches "A.patch"
> "B.patch"
> ...
> "N.patch")
>
> Though you may have to append two lists together for that to work. As to
> how... ask someone who understands guile. :)

That's what I attempted but failed. Now it's fixed.
>
> Also wondering if there is a branch with all the patches you need
> already applied, since you're using a separate source origin already...

Well, I could apply them to the kernel fork. I'll open the issue on the github 
to discuss them.

>
> > +(define-public linux-libre-arm64-pinenote
> > + (make-linux-libre* linux-libre-version
> > + linux-libre-gnu-revision
> > + linux-libre-arm64-pinenote-source
> > + '("aarch64-linux")
> > + #:defconfig "pinenote_defconfig"
> > + #:extra-options
> > + %default-extra-linux-options))
>
>
> You might want to not assume to use the same versions as
> (linux-libre-version, linux-libre-gnu-revision), as they might change
> out from under you... those were recently updated to 5.18.x ... instead
> just pick the versions relevent to the upstream source.
>
>
> Hope that's helpful, good luck and thanks for working on adding support
> for this very interesting platform!

Thanks Vagrant for the support and help. It now builds the system image. It's 
still not something that will work but we're getting there.

In the original email I attached the build log. The deblobing the kernel 
sources still fails and therefore I commented out now the list of the blobs. 
However, I'd like remove this modification. I assume it fails due to some 
assembly file which is neccessary for the eink driver to work.

Is there a way how to exclude it? It's in assembly so the source code is there 
(not machine code but also not that readable, there are some attempts to 
replace it). Also it's little bit pointless to remove it if the device will 
loose the display :-D

Also how do I specify the system image format so I can that I get the 
bootloader, kernel, initramfs and the rootfs on separate partitions that can be 
later flashed to the board using the rkdeveloptool?

----
Petr

Attachment: v2-0003-gnu-Add-u-boot-pinenote-rk3568.patch
Description: Text Data

Attachment: v2-0005-gnu-Add-image-for-pinenote.patch
Description: Text Data

Attachment: v2-0002-gnu-u-boot-Remove-input-labels.patch
Description: Text Data

Attachment: v2-0004-gnu-Add-install-pinenote-rk3568-u-boot-and-u-boot.patch
Description: Text Data

Attachment: v2-0001-gnu-Add-linux-libre-arm64-pinenote.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]