GNU bug report logs - #51663
[PATCH] gnu: u-boot: Fix rk3399 boot from emmc.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 51663 in the body.
You can then email your comments to 51663 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Pierre Langlois <pierre.langlois <at> gmx.com>
To: guix-patches <at> gnu.org
Cc: Vagrant Cascadian <vagrant <at> debian.org>
Subject: [PATCH] gnu: u-boot: Fix rk3399 boot from emmc.
Date: Sun, 07 Nov 2021 14:34:09 +0000

[Message part 1 (text/plain, inline)]

Hi Guix!

I'm afraid the last u-boot update broke booting the rockpro64 and
pinebook-pro from the eMMC card :-/.  Going through the u-boot ML, I saw
that it was reported [0], and this fix [1] should be applied.

So here's a patch!  I tested it on on both the pinebook-pro and
rockpro64.

OK to apply?

Thanks,
Pierre

[0]: https://lists.denx.de/pipermail/u-boot/2021-November/466329.html
[1]: https://patchwork.ozlabs.org/project/uboot/patch/20211101044347.17822-1-yifeng.zhao <at> rock-chips.com/

[signature.asc (application/pgp-signature, inline)]

[0001-gnu-u-boot-Fix-rk3399-boot-from-emmc.patch (text/x-patch, inline)]

From 4dea90a439f749f9f54407603c01eabf9900a75e Mon Sep 17 00:00:00 2001
From: Pierre Langlois <pierre.langlois <at> gmx.com>
Date: Fri, 5 Nov 2021 20:38:45 +0000
Subject: [PATCH] gnu: u-boot: Fix rk3399 boot from emmc.

* gnu/packages/bootloaders.scm (%u-boot-rk3399-enable-emmc-phy-patch): New
variable.
(u-boot)[origin]: Register it.
* gnu/packages/patches/u-boot-rk3399-enable-emmc-phy.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
---
 gnu/local.mk                                  |  1 +
 gnu/packages/bootloaders.scm                  |  8 +++++-
 .../u-boot-rk3399-enable-emmc-phy.patch       | 26 +++++++++++++++++++
 3 files changed, 34 insertions(+), 1 deletion(-)
 create mode 100644 gnu/packages/patches/u-boot-rk3399-enable-emmc-phy.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index 05258ac054..0f98d6ce56 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1842,6 +1842,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/u-boot-rockchip-inno-usb.patch		\
   %D%/packages/patches/u-boot-sifive-prevent-reloc-initrd-fdt.patch	\
   %D%/packages/patches/u-boot-riscv64-fix-extlinux.patch	\
+  %D%/packages/patches/u-boot-rk3399-enable-emmc-phy.patch	\
   %D%/packages/patches/ucx-tcp-iface-ioctl.patch		\
   %D%/packages/patches/ungoogled-chromium-extension-search-path.patch	\
   %D%/packages/patches/ungoogled-chromium-ffmpeg-compat.patch	\
diff --git a/gnu/packages/bootloaders.scm b/gnu/packages/bootloaders.scm
index 706ddf0207..c93af2241f 100644
--- a/gnu/packages/bootloaders.scm
+++ b/gnu/packages/bootloaders.scm
@@ -504,6 +504,11 @@ (define %u-boot-allow-disabling-openssl-patch
   ;; https://lists.denx.de/pipermail/u-boot/2021-October/462728.html
   (search-patch "u-boot-allow-disabling-openssl.patch"))

+(define %u-boot-rk3399-enable-emmc-phy-patch
+  ;; Fix emmc boot on rockpro64 and pinebook-pro.
+  ;; https://lists.denx.de/pipermail/u-boot/2021-November/466329.html
+  (search-patch "u-boot-rk3399-enable-emmc-phy.patch"))
+
 (define u-boot
   (package
     (name "u-boot")
@@ -512,7 +517,8 @@ (define u-boot
 	      (patches
                (list %u-boot-rockchip-inno-usb-patch
                      %u-boot-allow-disabling-openssl-patch
-                     %u-boot-sifive-prevent-relocating-initrd-fdt))
+                     %u-boot-sifive-prevent-relocating-initrd-fdt
+                     %u-boot-rk3399-enable-emmc-phy-patch))
               (method url-fetch)
               (uri (string-append
                     "https://ftp.denx.de/pub/u-boot/"
diff --git a/gnu/packages/patches/u-boot-rk3399-enable-emmc-phy.patch b/gnu/packages/patches/u-boot-rk3399-enable-emmc-phy.patch
new file mode 100644
index 0000000000..f14a9ce104
--- /dev/null
+++ b/gnu/packages/patches/u-boot-rk3399-enable-emmc-phy.patch
@@ -0,0 +1,26 @@
+adapting commit ac804143cf ("mmc: rockchip_sdhci: add phy and clock
+config for rk3399") to fix the issue "Not found emmc phy device".
+
+Signed-off-by: Yifeng Zhao <yifeng.zhao <at> rock-chips.com>
+---
+
+ arch/arm/dts/rk3399-u-boot.dtsi | 4 ++++
+ 1 file changed, 4 insertions(+)
+
+diff --git a/arch/arm/dts/rk3399-u-boot.dtsi b/arch/arm/dts/rk3399-u-boot.dtsi
+index 73922c328a..716b9a433a 100644
+--- a/arch/arm/dts/rk3399-u-boot.dtsi
++++ b/arch/arm/dts/rk3399-u-boot.dtsi
+@@ -88,6 +88,10 @@
+ 	u-boot,dm-pre-reloc;
+ };
+
++&emmc_phy {
++	u-boot,dm-pre-reloc;
++};
++
+ &grf {
+ 	u-boot,dm-pre-reloc;
+ };
+--
+2.17.1
--
2.33.1

Message #8 received at 51663 <at> debbugs.gnu.org (full text, mbox):

From: Mathieu Othacehe <othacehe <at> gnu.org>
To: Pierre Langlois <pierre.langlois <at> gmx.com>
Cc: Vagrant Cascadian <vagrant <at> debian.org>, 51663 <at> debbugs.gnu.org
Subject: Re: bug#51663: [PATCH] gnu: u-boot: Fix rk3399 boot from emmc.
Date: Fri, 12 Nov 2021 16:07:52 +0000

Hello Pierre,

> +                     %u-boot-rk3399-enable-emmc-phy-patch))

You could add a comment to indicate that this can probably be removed
when updating u-boot.

Otherwise, looks fine!

Thanks,

Mathieu

Message #11 received at 51663 <at> debbugs.gnu.org (full text, mbox):

From: Vagrant Cascadian <vagrant <at> debian.org>
To: Pierre Langlois <pierre.langlois <at> gmx.com>
Cc: 51663 <at> debbugs.gnu.org
Subject: Re: [PATCH] gnu: u-boot: Fix rk3399 boot from emmc.
Date: Fri, 12 Nov 2021 10:26:22 -0800

[Message part 1 (text/plain, inline)]

On 2021-11-07, Pierre Langlois wrote:
> I'm afraid the last u-boot update broke booting the rockpro64 and
> pinebook-pro from the eMMC card :-/.  Going through the u-boot ML, I saw
> that it was reported [0], and this fix [1] should be applied.
>
> So here's a patch!  I tested it on on both the pinebook-pro and
> rockpro64.
>
> OK to apply?
>
> Thanks,
> Pierre
>
> [0]: https://lists.denx.de/pipermail/u-boot/2021-November/466329.html
> [1]: https://patchwork.ozlabs.org/project/uboot/patch/20211101044347.17822-1-yifeng.zhao <at> rock-chips.com/
...
> diff --git a/gnu/packages/patches/u-boot-rk3399-enable-emmc-phy.patch b/gnu/packages/patches/u-boot-rk3399-enable-emmc-phy.patch
> new file mode 100644
> index 0000000000..f14a9ce104
> --- /dev/null
> +++ b/gnu/packages/patches/u-boot-rk3399-enable-emmc-phy.patch
> @@ -0,0 +1,26 @@
> +adapting commit ac804143cf ("mmc: rockchip_sdhci: add phy and clock
> +config for rk3399") to fix the issue "Not found emmc phy device".

I'm a little confused about this comment in the patch... is ac804143cf
the commit which fixed the issue or broke it? is there a reference to an
upstream commit and/or issue that fixes it?

Otherwise, looks good to me!

live well,
  vagrant

[signature.asc (application/pgp-signature, inline)]

Message #14 received at 51663 <at> debbugs.gnu.org (full text, mbox):

From: Pierre Langlois <pierre.langlois <at> gmx.com>
To: Vagrant Cascadian <vagrant <at> debian.org>
Cc: Pierre Langlois <pierre.langlois <at> gmx.com>, 51663 <at> debbugs.gnu.org
Subject: Re: [PATCH] gnu: u-boot: Fix rk3399 boot from emmc.
Date: Sat, 13 Nov 2021 13:35:09 +0000

[Message part 1 (text/plain, inline)]

Hi,

Vagrant Cascadian <vagrant <at> debian.org> writes:

> [[PGP Signed Part:Undecided]]
> On 2021-11-07, Pierre Langlois wrote:
>> I'm afraid the last u-boot update broke booting the rockpro64 and
>> pinebook-pro from the eMMC card :-/.  Going through the u-boot ML, I saw
>> that it was reported [0], and this fix [1] should be applied.
>>
>> So here's a patch!  I tested it on on both the pinebook-pro and
>> rockpro64.
>>
>> OK to apply?
>>
>> Thanks,
>> Pierre
>>
>> [0]: https://lists.denx.de/pipermail/u-boot/2021-November/466329.html
>> [1]: https://patchwork.ozlabs.org/project/uboot/patch/20211101044347.17822-1-yifeng.zhao <at> rock-chips.com/
> ...
>> diff --git a/gnu/packages/patches/u-boot-rk3399-enable-emmc-phy.patch b/gnu/packages/patches/u-boot-rk3399-enable-emmc-phy.patch
>> new file mode 100644
>> index 0000000000..f14a9ce104
>> --- /dev/null
>> +++ b/gnu/packages/patches/u-boot-rk3399-enable-emmc-phy.patch
>> @@ -0,0 +1,26 @@
>> +adapting commit ac804143cf ("mmc: rockchip_sdhci: add phy and clock
>> +config for rk3399") to fix the issue "Not found emmc phy device".
>
> I'm a little confused about this comment in the patch... is ac804143cf
> the commit which fixed the issue or broke it? is there a reference to an
> upstream commit and/or issue that fixes it?

Yeah it's not very clear to me either, I think ac804143cf is the commit
that introduced the regression indeed. Doing a bit of digging, I don't
think the fix was merged upstream yet though :-(.

Instead, I see Fedora opted to revert the problematic patch, along with
another dependent one. Probably before the fix was posted to the list:

  https://src.fedoraproject.org/rpms/uboot-tools/c/37df227bc0961f9f0dc4dafa9e983290dbdb2bc3
  https://bugzilla.redhat.com/show_bug.cgi?id=2014182#c3

Given it's not upstream, it could be safer to do what Fedora did and
revert a couple of patches, what do you think? The fix is from the
original author so I'm inclined to trust it, I'd be surprised if it
doesn't get merged.

Thanks,
Pierre

[signature.asc (application/pgp-signature, inline)]

Message #19 received at 51663-done <at> debbugs.gnu.org (full text, mbox):

From: Pierre Langlois <pierre.langlois <at> gmx.com>
To: Mathieu Othacehe <othacehe <at> gnu.org>
Cc: Vagrant Cascadian <vagrant <at> debian.org>,
 Pierre Langlois <pierre.langlois <at> gmx.com>, 51663-done <at> debbugs.gnu.org
Subject: Re: bug#51663: [PATCH] gnu: u-boot: Fix rk3399 boot from emmc.
Date: Sat, 27 Nov 2021 15:04:21 +0000

[Message part 1 (text/plain, inline)]

Hi Mathieu,

Mathieu Othacehe <othacehe <at> gnu.org> writes:

> Hello Pierre,
>
>> +                     %u-boot-rk3399-enable-emmc-phy-patch))
>
> You could add a comment to indicate that this can probably be removed
> when updating u-boot.

Done, the patch hasn't been picked up upstream yet AFAIK, however it was
picked up by fedora so I'm confident it's OK and it doens't just work
for me accidentally :-).

>
> Otherwise, looks fine!

Thanks! Pushed with bf1e46e959884df4bb204807efd21bbf44c5f87e

Pierre

[signature.asc (application/pgp-signature, inline)]

Message #22 received at 51663 <at> debbugs.gnu.org (full text, mbox):

From: Pierre Langlois <pierre.langlois <at> gmx.com>
To: Vagrant Cascadian <vagrant <at> debian.org>
Cc: Pierre Langlois <pierre.langlois <at> gmx.com>, 51663 <at> debbugs.gnu.org
Subject: Re: [PATCH] gnu: u-boot: Fix rk3399 boot from emmc.
Date: Sat, 27 Nov 2021 15:06:18 +0000

[Message part 1 (text/plain, inline)]

Pierre Langlois <pierre.langlois <at> gmx.com> writes:

> [[PGP Signed Part:Undecided]]
> Hi,
>
> Vagrant Cascadian <vagrant <at> debian.org> writes:
>
>> [[PGP Signed Part:Undecided]]
>> On 2021-11-07, Pierre Langlois wrote:
>>> I'm afraid the last u-boot update broke booting the rockpro64 and
>>> pinebook-pro from the eMMC card :-/.  Going through the u-boot ML, I saw
>>> that it was reported [0], and this fix [1] should be applied.
>>>
>>> So here's a patch!  I tested it on on both the pinebook-pro and
>>> rockpro64.
>>>
>>> OK to apply?
>>>
>>> Thanks,
>>> Pierre
>>>
>>> [0]: https://lists.denx.de/pipermail/u-boot/2021-November/466329.html
>>> [1]: https://patchwork.ozlabs.org/project/uboot/patch/20211101044347.17822-1-yifeng.zhao <at> rock-chips.com/
>> ...
>>> diff --git a/gnu/packages/patches/u-boot-rk3399-enable-emmc-phy.patch b/gnu/packages/patches/u-boot-rk3399-enable-emmc-phy.patch
>>> new file mode 100644
>>> index 0000000000..f14a9ce104
>>> --- /dev/null
>>> +++ b/gnu/packages/patches/u-boot-rk3399-enable-emmc-phy.patch
>>> @@ -0,0 +1,26 @@
>>> +adapting commit ac804143cf ("mmc: rockchip_sdhci: add phy and clock
>>> +config for rk3399") to fix the issue "Not found emmc phy device".
>>
>> I'm a little confused about this comment in the patch... is ac804143cf
>> the commit which fixed the issue or broke it? is there a reference to an
>> upstream commit and/or issue that fixes it?
>
> Yeah it's not very clear to me either, I think ac804143cf is the commit
> that introduced the regression indeed. Doing a bit of digging, I don't
> think the fix was merged upstream yet though :-(.
>
> Instead, I see Fedora opted to revert the problematic patch, along with
> another dependent one. Probably before the fix was posted to the list:
>
>   https://src.fedoraproject.org/rpms/uboot-tools/c/37df227bc0961f9f0dc4dafa9e983290dbdb2bc3
>   https://bugzilla.redhat.com/show_bug.cgi?id=2014182#c3
>
> Given it's not upstream, it could be safer to do what Fedora did and
> revert a couple of patches, what do you think? The fix is from the
> original author so I'm inclined to trust it, I'd be surprised if it
> doesn't get merged.

Taking another quick look today, it seems Fedora now picked up this
patch, yey!
https://src.fedoraproject.org/rpms/uboot-tools/c/9eb973e7d2ab74341c0eb7312576b6f016ec03d4?branch=rawhide

I'm more confident it's the right fix now so I've gone ahead and pushed
it.

Thanks!
Pierre

[signature.asc (application/pgp-signature, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.