diff mbox series

[bug#51663] gnu: u-boot: Fix rk3399 boot from emmc.

Message ID 87a6igav2l.fsf@gmx.com
State Accepted
Headers show
Series [bug#51663] gnu: u-boot: Fix rk3399 boot from emmc. | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

Pierre Langlois Nov. 7, 2021, 2:34 p.m. UTC
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@rock-chips.com/

Comments

Mathieu Othacehe Nov. 12, 2021, 4:07 p.m. UTC | #1
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
Vagrant Cascadian Nov. 12, 2021, 6:26 p.m. UTC | #2
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@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
Pierre Langlois Nov. 13, 2021, 1:35 p.m. UTC | #3
Hi,

Vagrant Cascadian <vagrant@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@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
Pierre Langlois Nov. 27, 2021, 3:04 p.m. UTC | #4
Hi Mathieu,

Mathieu Othacehe <othacehe@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
Pierre Langlois Nov. 27, 2021, 3:06 p.m. UTC | #5
Pierre Langlois <pierre.langlois@gmx.com> writes:

> [[PGP Signed Part:Undecided]]
> Hi,
>
> Vagrant Cascadian <vagrant@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@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
diff mbox series

Patch

From 4dea90a439f749f9f54407603c01eabf9900a75e Mon Sep 17 00:00:00 2001
From: Pierre Langlois <pierre.langlois@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@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