[needs-packaging] U-Boot for Microchip PIC64GX

Bug #2072490 reported by Heinrich Schuchardt
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Ubuntu
Fix Released
Wishlist
Unassigned

Bug Description

We want to support the upcoming Microchip PIC64GX board. Unfortunately upstreaming of U-Boot has not started yet. Due to the number of differences between our U-Boot 2024.01 and the vendor U-Boot it is not trivial to add board support to our existing U-Boot. The best interim solution is packaging the vendor U-Boot. Once upstreaming has occurred we can sun-set the vendor U-Boot package and replace it by the u-boot-microchip package.

Changed in ubuntu:
assignee: nobody → Heinrich Schuchardt (xypron)
Revision history for this message
Heinrich Schuchardt (xypron) wrote :

u-boot-pic64gx - 20240624-0ubuntu1~ppa1 is available in ppa:xypron/u-boot-pic64gx.

summary: - Package U-Boot for Microchip PIC64GX
+ [needs-packaging] U-Boot for Microchip PIC64GX
description: updated
description: updated
Changed in ubuntu:
assignee: Heinrich Schuchardt (xypron) → nobody
tags: added: needs-packaging
Revision history for this message
Brian Murray (brian-murray) wrote :

*** This is an automated message ***

This bug is tagged needs-packaging which identifies it as a request for a new package in Ubuntu. As a part of the managing needs-packaging bug reports specification, https://wiki.ubuntu.com/QATeam/Specs/NeedsPackagingBugs, all needs-packaging bug reports have Wishlist importance. Subsequently, I'm setting this bug's status to Wishlist.

Changed in ubuntu:
importance: Undecided → Wishlist
Revision history for this message
Dave Jones (waveform) wrote :

Sponsoring for oracular; this will need an ack from an AA to enter the archive. The question then is whether to MIR this package, given the intent is obviously to include it in images. I guess that'll come down to how "official" we want those images to be (and that may come down to a question of timing given there's only a month to go before noble's .1 release).

Revision history for this message
Łukasz Zemczak (sil2100) wrote :

I have reviewed this package. It seems more or less similar to the -nezha one structurally, so that's fine. But before I accept it into Ubuntu I'd like to know a bit more about the orig tarball. Where does this tarball come from? I see the 20240624 versioning, but on the repository mentioned in debian/control (https://github.com/pic64gx/pic64gx-uboot/tags) I only see v2024.06.tar.gz. Is that this release? Why is the versioning different? How will we convert those? How can we guarantee that the release tarball is 1:1 what is officially from upstream if it's repacked and reversioned?

Changed in ubuntu:
status: New → Incomplete
Revision history for this message
Heinrich Schuchardt (xypron) wrote :

Hello Łukasz,

Thanks for reviewing.

Microchip has published https://github.com/pic64gx/pic64gx-uboot, branch pic64gx.
The last commit 39490770a78d I saw was dated 2024-06-24. By now Microchip has created a tag v2024.06 for it.

Using the tag seems looks like a better choice.

Best regards

Heinrich

Revision history for this message
Heinrich Schuchardt (xypron) wrote :

u-boot-pic64gx_2024.06-0ubuntu0.22.4.1~ppa1_all.deb is available in ppa:xypron/u-boot-pic64gx-2024.06

The orig file was created with from Microchips upstream tag with:
git archive --format=tar --prefix='u-boot-pic64gx-2024.06/' -o ../u-boot-pic64gx_2024.06.orig.tar v2024.06

Changed in ubuntu:
status: Incomplete → New
Revision history for this message
Steve Langasek (vorlon) wrote :

u-boot-pic64gx_2024.06

well, somewhat unfortunate wrt license review that this doesn't align with any of the u-boot upstream tags; I guess I'll compare with 2024.04...

The u-boot source package has in debian/copyright:

Files-Excluded:
  drivers/dma/MCD_tasks.c

u-boot-pic64gx does not. Do you know why this was excluded in the Debian u-boot package, and why it would be ok to ship it in the pic64gx build? If not, I recommend that you follow the u-boot source package's lead to avoid any doubts.

There's also an 8kloc diff between the debian/copyright in u-boot 2024.01+dfsg-1ubuntu5 and the one in u-boot-pic64gx 20240624-0ubuntu1. That is a rather significant barrier to using u-boot as a base for review of correct licensing / copyright declaration of the new package. Can you explain this difference?

Also, speaking of '20240624-0ubuntu1' - note that this is not consistent with the versioning scheme used by the u-boot source package; 20240624 sorts >> the YYYY.MM style version numbers. This will make it difficult to later build the u-boot-pic64gx binary out of the u-boot source package due to inconsistent version numbers. I suggest you avoid this now by packaging this instead as 2024.06.24-0ubuntu1.

Changed in ubuntu:
status: New → Incomplete
Revision history for this message
Heinrich Schuchardt (xypron) wrote :

Hello Steve,

thanks for reviewing.

As mentioned above I have already repackaged as u-boot-pic64gx_2024.06-0ubuntu0.22.4.1~ppa1 as Microchip created a tag v2024.06.

v2024.06 is a version number chosen by Microchip which is not related to the upstream U-Boot version numbering scheme. Actually it is based on upstream v2023.07. Once the Microchip has upstreamed the U-Boot code we will put the binaries in u-boot-microchip built from the u-boot source package.

The file drivers/dma/MCD_tasks.c was introduced in 2008 with commit 11865ea844e7 ("ColdFire: Add MCF547x_8x dma code - 1") by employees of the copyright holding company. In this commit it was already marked as GPL-2-or-later. I cannot see any tort in distributing a GPLed file in our orig.tar.xz which we do not compile. In upstream U-Boot the file was removed since due to obsolescence.

Best regards

Heinrich

Changed in ubuntu:
status: Incomplete → New
Revision history for this message
Heinrich Schuchardt (xypron) wrote (last edit ):

File drivers/dma/MCD_tasks.c was dropped in the u-boot source package due to Debian bug 750912. The file contains some instructions for the 68000 platform in hexadecimal form. See https://lists.gnu.org/archive/html/gnu-linux-libre/2014-05/msg00014.html.

We are not distributing the content of this file in 'executable form'.

Revision history for this message
Steve Langasek (vorlon) wrote :

arch/riscv/cpu/mpfs/cpu.c, arch/riscv/cpu/mpfs/dram.c, arch/riscv/cpu/pic64gx/cpu.c, arch/riscv/cpu/pic64gx/dram.c are new files in u-boot-pic64gx-20240624 listed as 'Copyright (C) 2018, Bin Meng <email address hidden>'. Please include this in debian/copyright.

arch/riscv/dts/mpfs-tysom-m.dts lists Copyright (C) 2020-2022 - Aldec which is not in debian/copyright. (It also lists Copyright (C) 2022-2023 - Conor Dooley <email address hidden> but we can presume this is incorrect and should be Microchip, which is listed in debian/copyright.)

I also see that this source builds an Arch: all binary package. The convention is that the u-boot binary packages declare the architectures that they are used on (riscv64); we don't want to install u-boot-pic64gx on an amd64 system.

Ok to leave drivers/dma/MCD_tasks.c in the source.

And diffing debian/copyright against the Debian v2023.07 package instead of the latest looks sensible, so no changes needed there.

That leaves the above three issues, plus the upstream version problem.

Revision history for this message
Heinrich Schuchardt (xypron) wrote :

u-boot-pic64gx 2024.06-0ubuntu1~ppa1 is available in ppa:xypron/u-boot-pic64gx-2024.06 and considers Steve's review remarks.

Revision history for this message
Dave Jones (waveform) wrote :

It looks like Steve's comments are addressed by the current PPA version (and I'll make a note to check that MCD_tasks.c disappears in the forthcoming merge of u-boot main!). I do note there's a few lintian errors / warnings to clean up. Most are trivial and I'm happy to just deal with them myself while sponsoring, specifically:

E: u-boot-pic64gx source: build-depends-indep-without-arch-indep

This is because, now the u-boot-pic64gx package is riscv64 only, there are no arch-indep packages left. Trivial fix: make gcc-riscv64-linux-gnu part of build-depends instead.

W: u-boot-pic64gx source: missing-license-paragraph-in-dep5-copyright gpl-2.0 [debian/copyright:126]
W: u-boot-pic64gx source: missing-license-paragraph-in-dep5-copyright gpl-2.0+ [debian/copyright:119]

Here the lines in d/copyright refer to GPL-2.0 instead of GPL-2 and GPL-2.0+ instead of GPL-2+. Trivial fix: delete .0 in each case

W: u-boot-pic64gx source: patch-file-present-but-not-mentioned-in-series [debian/patches/riscv-update-PIC64GX-model-property.patch]

This one I need a bit of guidance on. I *think* this patch is not meant to be there; if I add it to d/p/series it complains about a reversed/already applied patch so I'm guessing this patch was in an earlier bit of packaging, but was then applied upstream and so removed from d/p/series, but the patch itself didn't get removed from the directory? If so, I'm happy to just remove it while sponsoring.

Revision history for this message
Dave Jones (waveform) wrote :

The ~ppa3 version now in the same PPA addresses the concerns above, and passes lintian clean; sponsoring for oracular.

Changed in ubuntu:
status: New → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.